Yet Another FileAdapter: Google Cloud Storage

[asciimike]

Tossing my hat in the ring for building another FileAdapter, this time GCSAdapter.js for integrating Google Cloud Storage

Like all the other file adapters, you can create one like so:

var GCSAdapter = require('parse-server').GCSAdapter;
...
var api = new ParseServer({
  ...
  filesAdapter: new GCSAdapter(
    "<gcp_project_id>",
    "<path/to/keyfile>",
    "<gcp_bucket_name>",
    {
      bucketPrefix: <String>,
      directAccess: <Boolean>
    }
  )
});

Re: #545 I'm happy to wait on a redesign if that's where we're going. Agree that it's a bit hard to handle these currently, and the dependencies on AWS, Azure, and GCP Node modules is getting a little ridiculous...

On the subject of unit tests--I went ahead and modified helpers.js locally to point to GCS and tested these locally, with 21/21 passing. I also created and added a ParseFile+GCS.spec.js, which contains tests for Parse Server proxied files as well as public readable files. Note that these tests require a GCP Project, keyfile, and GCS Bucket.

I'm putting some thought into mocking out the various providers to avoid all the issues with having sets of keys for each service, as I'm sure the team doesn't want to maintain a bunch of various keys to various cloud platforms. I also have some thoughts on creating cloud provider config settings to encapsulate the AWS, Azure, GCP, etc. keys that I'd be happy to talk to someone about.

[facebook-github-bot]

@mcdonamp updated the pull request.

[asciimike]

Still plagued by the flaky Geopoint test in #572 , but other tests will work.

Also, if people are annoyed by the fact that the tests still show up (and are just listed as pending), I thought of two quick solutions:

1. Create a spec/optional directory, which is excluded from the jasmine.json spec_files glob. This would require no changes to the present jasmine.json, since directories aren't recursed into currently.
2. Change the glob to !(*+*).spec.js which ignores any foo+bar.spec.js, meaning that any "extension" specs such as GCS, S3, etc. would take the form of test+extension.spec.js and would thus be ignored unless a developer explicitly calls npm test spec/test+extension.spec.js.

Either solution here allows us to get rid of 3e41a21 since the tests wouldn't be run unless a developer explicitly wanted to run it.

Also, I'd love to be able to contribute to the Wiki to clean up some of the Files docs: basically just add a section for each provider, as well as an overview of what's going on. What's the best way to contribute back to the Wiki (given how PRs against it aren't straightforward)?

[flovilmart]

@mcdonamp we were discussing @nlutsenko test factories for those adapter. Those factories would allow you to keep the adapter on your repo, and use travis with your encrypted keys. this way we'll ensure the compatibility with parse-server without burdening the core repository with zillions of implementations. What do you think?

[otymartin]

@mcdonamp Hey - parse newbie talking. What does this mean? I deployed parse server to google app engine using mongolab. Does that mean I can use google datastore as an alternative to mongolab using this adapter? Preferably, I'd like to do that to keep all my backend activity within a single platform.

[flovilmart]

@otymartin it means you can use Google Cloud Storage, to store and serve the PFFiles instead of the default GridFS or Amazon S3. For Now, you cant use GCP NoSQL database engine as some more changes are required to this project.
You can still host your database on GCP, but you would have to manage it manually.

[asciimike]

@otymartin @flovilmart beat me to the punch by seconds! As he said, this means that you can store your Parse Files (user generated content like images, videos, audio, etc.) in Google Cloud Storage, Google's Petabyte static storage system. This is distinct from something like Cloud Datastore, which is Google's Hosted NoSQL offering. Currently Parse Server is very mongo specific and making it fully database agnostic is going to take a lot of time, though #698 is a good step in that direction.

[asciimike]

@flovilmart totally agree with this idea, though I've got some concerns that it'll raise the barrier to entry for creating such extensions, especially for new devs. Maybe creating a Parse Server Extension repo with a template (basically the helpers.js and jasmine.json) that folks can clone and easily add to. The other concern I have is discoverability, though I assume that could be taken care of relatively easily on the wiki (by adding an extensions page) or by the individual cloud providers who curate their extensions.

What can I do to help this move forward? Some potential AIs I see here are:

1. Reconfigure this adapter as a separate node module
2. Rip out all provider specific code and packages

FWIW, if folks don't care about running their own, I'd probably be OK owning a parse-server-file-extensions module that bundles the adapters for S3, Azure, and GCS as well as handles testing, etc. Thus the imports end up looking like:

var S3Adapter = require('parse-server-file-extensions').S3Adapter(config);
var AzureBlobAdapter = require('parse-server-file-extensions').AzureBlobAdapter(config);
var GCSAdapter = require('parse-server-file-extensions').GCSAdapter(config);

This might also make config a little easier, since we could try to offer a single shared config file for all these (though TBH, not sure why anyone would be using multiple file storage providers).

[flovilmart]

We're having pretty intense discussions around that. My main concern is testability and stability.
those providers require API keys, and we can't afford to have jasmine ignores all the way in the repo for those adapter.

I'm refactoring the FIlesController tests now into a factory, and I uncovered 3 major issues with S3Adapter as it's completely untested.

The test factories are design to be easy to use:

var TestsFactory = require("./FilesControllerTestsFactory");


// Small additional tests to improve overall coverage
describe("FilesController",()=>{
  TestsFactory(new GridStoreAdapter());
  if (process.env.S3_ACCESS_KEY && process.env.S3_SECRET_KEY) {
     TestsFactory(new S3Adapter(process.env.S3_ACCESS_KEY, process.env.S3_SECRET_KEY, 'parse.server.tests')); 
  } else {
    console.warn("cannot test S3Adapter");
  }
});
``` 

[asciimike]

Cool--I figured, as I'm sure it's a contentious issue. Happy to weigh in if you'd like another opinion; otherwise, happy to stay out as I'm sure there are already plenty of folks more knowledgeable than me working to solve it.

I found a few issues with mine that were teased out in testing, so luckily I fixed them before they saw the light of day.

I thought about switching off the presence of env variables as an option as well, not sure why I didn't put it down... I'm a fan of this strategy, as well as of splitting these tests (and the adapters) out into a separate repo.

Not sure if the best strategy is to merge all of these adapters in and then cut them all out at the same time and provide a major version bump, or just herd cats to pull the appropriate PRs somewhere else. Either way, let me know what you need in the way of help this moving forward!

[asciimike]

@flovilmart I assume we're waiting on #718 to align these and then merge?

I'm happy to open an issue on the topic of File Adapter refactoring, potentially to a separate repo, if that will provide a place for us to consolidate our thoughts and gather broader community feedback.

[flovilmart]

@mcdonamp waiting for #738 as it has just the test factories for the FileAdapters. I managed to get the testing going for external adapter for the push adapter in https://github.com/flovilmart/parse-server-urban-airship. That involves a submodule and weird stuff.

The debate is open on the question of whether we should add those adapters in the core repo or not :)

[facebook-github-bot]

@mcdonamp updated the pull request.

[asciimike]

@flovilmart ok, got this back in line with the other adapters and the new stuff in FilesController (which is nice stuff--super slick!). Everything should be good to go here, tests pass (plus my other tests, which I have removed in favor of the FilesController).

Also, of note: I chose not to implement createBucket(). Generally I don't like create lots of buckets without the user fully realizing, as I think that a dev should know exactly what bucket they're writing to (especially since they're paying for it--one have to go to a console somewhere and enter a credit card, creating a bucket shouldn't be too difficult after that). Secondly, I think that the createBucket() calls on deleteFile() and getFileData() are extraneous. If the bucket doesn't exist and is automatically created, immediately calling delete or get isn't going to do anything, as the file can't exist (unless there's some fantastic race condition going on that somehow ends up uploading a file in between the create request and the subsequent request).

[facebook-github-bot]

@mcdonamp updated the pull request.

[flovilmart]

@mcdonamp no problem! You need a rebase and I believe we're good to go!

[asciimike]

Ok, I think I may update to what's in #833 to get the proper loading from the environment, then I'll run another quick set of tests and get it in.

[facebook-github-bot]

@mcdonamp updated the pull request.

[codecov-io]

Current coverage is 90.10%

Merging #708 into master will decrease coverage by -1.62% as of 03ae193

@@            master   #708   diff @@
=====================================
  Files           71     72     +1
  Stmts         4108   4164    +56
  Branches       849    863    +14
  Methods          0      0       
=====================================
- Hit           3768   3752    -16
  Partial         10     10       
- Missed         330    402    +72

Review entire Coverage Diff as of 03ae193

Powered by Codecov. Updated on successful CI builds.

[facebook-github-bot]

@mcdonamp updated the pull request.

[facebook-github-bot]

@mcdonamp updated the pull request.

[asciimike]

@flovilmart ok, this should be good to go. Lots of whitespace changes (looks like my sublime config likes that ;)

Also updated the README for both, and all tests are passing. Thanks for all the hard work and guidance here. I'm also happy to help the folks working on #545 and #716 if they need to know next steps.

[flovilmart]

Great you added those too!