Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implemented a Provider structure for Files, Cache, and Database adapters. Added a new memory cache implementation. #291

Closed
wants to merge 14 commits into from
Closed

Implemented a Provider structure for Files, Cache, and Database adapters. Added a new memory cache implementation. #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
fc9b4c4
Added the memory cache and tests.
Feb 7, 2016
ce40215
Implemented the new cache/provider to replace the old cache.js. All t…
Feb 7, 2016
94835a3
Added default TTL
Feb 8, 2016
127bdb7
Gave up my reservations about relying on Node module caching to enfor…
Feb 9, 2016
c853957
Clean up, added documentation. Moved resolveAdapter to the provider
Feb 9, 2016
ac6b6df
Moved the requirement statement to the top of the file
Feb 9, 2016
91feeac
Merged in the changes to DatabaseProvider and FilesProvider. Updated …
Feb 9, 2016
9488a9e
Rebased and moved to the new structure
Feb 9, 2016
8d8a01c
Fixed the pathing issues with the new structure
Feb 9, 2016
6b29cbc
Updated to ES6
Feb 9, 2016
9405e35
Removed deprecated FilesAdapter and DatabaseAdapter
Feb 9, 2016
cf0247f
Prevent crashing by adding default args
Feb 9, 2016
20b7835
Updated bin/parse-server to wrap the options as an app { ... app: { o…
Feb 9, 2016
309775d
Decoupled providers and default adapters.
Feb 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 14 additions & 12 deletions bin/parse-server
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,27 @@ if (process.env.PARSE_SERVER_OPTIONS) {
options = JSON.parse(process.env.PARSE_SERVER_OPTIONS);

} else {

options.databaseURI = process.env.PARSE_SERVER_DATABASE_URI;
options.cloud = process.env.PARSE_SERVER_CLOUD_CODE_MAIN;
options.collectionPrefix = process.env.PARSE_SERVER_COLLECTION_PREFIX;
options.app = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this breaking change on the options?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a separate PR probably. I wanted to separate the application configuration from the server configuration (see args.app in index.js), so that I could pass args.app to a constructor: new ParseApp(args.app);. Basically just namespacing the app options. This would likely change to an array when we implement multiple apps.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I have  {applications: [] }

Then ignore all other keys in configuration, in the sense that all apps should and could be configured differently (DB provider, keys, adapters, cloud code etc...)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the singleton providers (mail, cache, database, files) would need to have a map of app:adapter. I started implementing it and decided it would be better to get this merged first. The DatabaseProvider already kind of implements this, but I want to refactor it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not get too opinionated on general configuration vs per app configuration. If designed properly, no matter how many app run on the server it should work. We should not have singletons as we create new instances of the ParseServer, it doesn't make any sense that on server configuration interfere with another.

Today, the multiple server is not supported because of the singletons everywhere, which are more bad design decisions we need to turn right don't you think?

I mean, when developing apps, I just want to quickly update my json configuration with a new set APP keys and get running, like we did back in the parse days :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I've refactored all the singletons out in a fork of mine, but it's a pretty major revision. I've harped on it in a few issues, but never gotten any traction. Basically you have to pass the context down via the Express req object. So if you need to access the MailProvider you have to do something like req.Parse.Server.MailProvider.getAdapter(). This is also cool because you can write custom middleware when running ParseServer as an express plugin. getAdapter() would become getAdapter(appId) for all the service providers.

I'm not sure the singletons are blocking multiple servers though. Once we map applications to adapters then I think we're clear to run multiple servers each with independent adapter configurations for the same application. I was strongly against singletons, but I started to look at the Mongoose source and figured if Mongoose is using them, then we're probably safe.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a CLI and Docker has some good opportunities to basically bootstrap a Parse server stack with just a 1 liner: docker run parse-server:latest --config '{ databaseURI, appId, masterKey }'.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or npm install -g parse-server && parse-server --config path/to/config.json

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh, singletons have given me a real pain to make the multiple apps work correctly, the first in line being Parse, because everything is asynchronous, you can't guarantee that your singleton didn't change it's state between 2 callbacks :)

options.app.databaseURI = process.env.PARSE_SERVER_DATABASE_URI;
options.app.cloud = process.env.PARSE_SERVER_CLOUD_CODE_MAIN;
options.app.collectionPrefix = process.env.PARSE_SERVER_COLLECTION_PREFIX;

// Keys and App ID
options.appId = process.env.PARSE_SERVER_APPLICATION_ID;
options.clientKey = process.env.PARSE_SERVER_CLIENT_KEY;
options.restAPIKey = process.env.PARSE_SERVER_REST_API_KEY;
options.dotNetKey = process.env.PARSE_SERVER_DOTNET_KEY;
options.javascriptKey = process.env.PARSE_SERVER_JAVASCRIPT_KEY;
options.masterKey = process.env.PARSE_SERVER_MASTER_KEY;
options.fileKey = process.env.PARSE_SERVER_FILE_KEY;
options.app.appId = process.env.PARSE_SERVER_APPLICATION_ID;
options.app.clientKey = process.env.PARSE_SERVER_CLIENT_KEY;
options.app.restAPIKey = process.env.PARSE_SERVER_REST_API_KEY;
options.app.dotNetKey = process.env.PARSE_SERVER_DOTNET_KEY;
options.app.javascriptKey = process.env.PARSE_SERVER_JAVASCRIPT_KEY;
options.app.dotNetKey = process.env.PARSE_SERVER_DOTNET_KEY;
options.app.masterKey = process.env.PARSE_SERVER_MASTER_KEY;
options.app.fileKey = process.env.PARSE_SERVER_FILE_KEY;

// Comma separated list of facebook app ids
var facebookAppIds = process.env.PARSE_SERVER_FACEBOOK_APP_IDS;

if (facebookAppIds) {
facebookAppIds = facebookAppIds.split(",");
options.facebookAppIds = facebookAppIds;
options.app.facebookAppIds = facebookAppIds;
}
}

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"codecov": "^1.0.1",
"deep-diff": "^0.3.3",
"jasmine": "^2.3.2",
"lodash": "^4.2.1",
"mongodb-runner": "^3.1.15"
},
"scripts": {
Expand Down
4 changes: 2 additions & 2 deletions spec/ParseAPI.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// A bunch of different tests are in here - it isn't very thematic.
// It would probably be better to refactor them into different files.

var DatabaseAdapter = require('../src/DatabaseAdapter');
var DatabaseProvider = require('../src/classes/DatabaseProvider').default;
var request = require('request');

describe('miscellaneous', function() {
Expand Down Expand Up @@ -358,7 +358,7 @@ describe('miscellaneous', function() {
obj.set('foo', 'bar');
return obj.save();
}).then(() => {
var db = DatabaseAdapter.getDatabaseConnection(appId);
var db = DatabaseProvider.getDatabaseConnection(appId);
return db.mongoFind('TestObject', {}, {});
}).then((results) => {
expect(results.length).toEqual(1);
Expand Down
5 changes: 2 additions & 3 deletions spec/ParseInstallation.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@
// Ported from installation_collection_test.go

var auth = require('../src/Auth');
var cache = require('../src/cache');
var Config = require('../src/Config');
var DatabaseAdapter = require('../src/DatabaseAdapter');
var DatabaseProvider = require('../src/classes/DatabaseProvider').default;
var Parse = require('parse/node').Parse;
var rest = require('../src/rest');

var config = new Config('test');
var database = DatabaseAdapter.getDatabaseConnection('test');
var database = DatabaseProvider.getDatabaseConnection('test');

describe('Installations', () => {

Expand Down
5 changes: 2 additions & 3 deletions spec/RestCreate.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
// These tests check the "create" functionality of the REST API.
var auth = require('../src/Auth');
var cache = require('../src/cache');
var Config = require('../src/Config');
var DatabaseAdapter = require('../src/DatabaseAdapter');
var DatabaseProvider = require('../src/classes/DatabaseProvider').default;
var Parse = require('parse/node').Parse;
var rest = require('../src/rest');
var request = require('request');

var config = new Config('test');
var database = DatabaseAdapter.getDatabaseConnection('test');
var database = DatabaseProvider.getDatabaseConnection('test');

describe('rest create', () => {
it('handles _id', (done) => {
Expand Down
1 change: 0 additions & 1 deletion spec/RestQuery.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// These tests check the "find" functionality of the REST API.
var auth = require('../src/Auth');
var cache = require('../src/cache');
var Config = require('../src/Config');
var rest = require('../src/rest');

Expand Down
Loading