-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
Added settings manager #1645
Merged
drew-gross
merged 9 commits into
parse-community:settings-endpoint
from
mamaso:settings-endpoint-no-cache
May 13, 2016
Merged
Added settings manager #1645
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
83151b7
Added settings manager
bda6744
Clarity improvements & test fixes for settings manager
b4fea9e
Updated cli-defs, features router, removed global test object
f2f8c79
Fixed cli definitions syntax error
34f2394
Fixed formatting issues
6a428ec
Removed ENABLE_CONFIG_CHANGES env var & improved doc
bf45772
Disable config changes for most tests
f6a8950
test fixes
92dd715
Correct handling of verbose
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,210 @@ | ||
var request = require('request'); | ||
var deepcopy = require('deepcopy'); | ||
var DatabaseAdapter = require('../src/DatabaseAdapter'); | ||
var database = DatabaseAdapter.getDatabaseConnection('test', 'test_'); | ||
var settingsCollection = '_ServerSettings'; | ||
var logger = require('../src/logger').default; | ||
|
||
var configuration; | ||
|
||
describe('Persistent Settings', () => { | ||
beforeEach((done) => { | ||
configuration = deepcopy(defaultConfiguration); | ||
configuration.verbose = true; | ||
configuration.enableConfigChanges = true; | ||
newServer().then(done); | ||
}); | ||
|
||
describe('Upon Initialization', () => { | ||
it('should persist settings', (done) => { | ||
configuration.clientKey = 'local'; | ||
|
||
newServer() | ||
.then(getPersisted) | ||
.then(persisted => { | ||
expect(persisted.clientKey).toEqual('local'); | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should only load mutable settings from database', (done) => { | ||
configuration.clientKey = 'local'; // defined | ||
|
||
updatePersisted({ logLevel: 'info', clientKey: 'persisted' }) | ||
.then(newServer) | ||
.then(_ => { | ||
var config = parseServerObject.config; | ||
expect(config.logLevel).toEqual('info'); // not locked or defined, so updated | ||
expect(config.clientKey).toEqual('local'); // configuration defined, therefore not updated | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('overwrites defined settings if lockDefinedSettings is false', (done) => { | ||
configuration.clientKey = 'local'; | ||
configuration.lockDefinedSettings = false; | ||
|
||
updatePersisted({ clientKey: 'persisted' }) | ||
.then(newServer) | ||
.then(_ => { | ||
var config = parseServerObject.config; | ||
expect(config.clientKey).toEqual('persisted'); // defined setting was updated | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
}); | ||
|
||
describe('Settings Router', () => { | ||
it('should provide error on post if config changes disabled', (done) => { | ||
configuration.enableConfigChanges = false; | ||
newServer() | ||
.then(endpoint.get) | ||
.then(res => expect(res.res.statusCode).toBe(200)) | ||
.then(_ => endpoint.post({ clientKey: 'causesError' })) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(403); | ||
expect(res.body.error).toBe('Server config changes are disabled'); | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should run setting callbacks such as configureLogger', (done) => { | ||
endpoint.post({ logLevel: 'silly' }) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(200); | ||
expect(res.body.logLevel).toBe('silly'); | ||
expect(logger.transports['parse-server'].level).toBe('silly'); | ||
}) | ||
.then(endpoint.get) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(200); | ||
expect(res.body.logLevel).toBe('silly'); | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should not set defined setting', (done) => { | ||
endpoint.post({ clientKey: 'alreadyDefined' }) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(200); | ||
expect(res.body.clientKey).toBeUndefined(); | ||
}) | ||
.then(endpoint.get) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(200); | ||
expect(res.body.clientKey).toBe(configuration.clientKey); | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should not allow access without masterKey', (done) => { | ||
var invalidHeaders = { | ||
'X-Parse-Application-Id': 'test', | ||
'X-Parse-Master-Key': 'invalid' | ||
}; | ||
|
||
endpoint.post({ logLevel: 'silly' }, invalidHeaders) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(403); | ||
expect(res.body.error).toBe('unauthorized'); | ||
}) | ||
.then(_ => endpoint.get(invalidHeaders)) | ||
.then(res => { | ||
expect(res.res.statusCode).toBe(403); | ||
expect(res.body.error).toBe('unauthorized'); | ||
}) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should expose non-existant settings as null', (done) => { | ||
delete configuration.clientKey; | ||
|
||
database.deleteEverything() | ||
.then(newServer) | ||
.then(endpoint.get) | ||
.then(res => expect(res.body.clientKey).toBe(null)) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should fetch database values', (done) => { | ||
delete configuration.clientKey; | ||
|
||
database.deleteEverything() | ||
.then(newServer) | ||
.then(endpoint.get) | ||
.then(res => expect(res.body.clientKey).toBe(null)) | ||
.then(_ => updatePersisted({ clientKey: 'persisted' })) | ||
.then(endpoint.get) | ||
.then(res => expect(res.body.clientKey).toBe('persisted')) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
|
||
it('should only return modified values', (done) => { | ||
// info is default log level | ||
var currentLogLevel; | ||
endpoint.get() | ||
.then(res => currentLogLevel = res.body.logLevel) | ||
.then(_ => endpoint.post({ logLevel: currentLogLevel })) | ||
.then(res => expect(res.body.logLevel).toBeUndefined) | ||
.then(done) | ||
.catch(done.fail); | ||
}); | ||
}); | ||
}); | ||
|
||
function newServer() { | ||
setServerConfiguration(deepcopy(configuration)); | ||
return parseServerObject.config.settingsInitialized; | ||
} | ||
|
||
function updatePersisted(settings) { | ||
settings.applicationId = configuration.appId; | ||
return parseServerObject.config.settingsInitialized | ||
.then(_ => database.adaptiveCollection(settingsCollection)) | ||
.then(coll => coll.upsertOne({ applicationId: configuration.appId }, { $set: settings })) | ||
.then(_ => undefined); | ||
} | ||
|
||
function getPersisted() { | ||
return parseServerObject.config.settingsInitialized | ||
.then(_ => database.mongoFind(settingsCollection, {}, {})) | ||
.then(results => results && results.length && results[0]); | ||
} | ||
|
||
var settingsUrl = 'http://localhost:8378/1/settings'; | ||
var defaultHeaders = { | ||
'X-Parse-Application-Id': 'test', | ||
'X-Parse-Master-Key': 'test' | ||
}; | ||
|
||
var req = (method, headers, body) => new Promise((resolve, reject) => { | ||
request[method]({ | ||
url: settingsUrl, | ||
json: body, | ||
headers: headers || defaultHeaders | ||
}, (err, res, body) => { | ||
if (err) { | ||
reject(err); | ||
} else { | ||
if (typeof body === 'string') body = JSON.parse(body); | ||
resolve({ | ||
res: res, | ||
body: body | ||
}); | ||
} | ||
}); | ||
}); | ||
|
||
var endpoint = { | ||
get: headers => req('get', headers), | ||
post: (body, headers) => req('post', headers, body) | ||
} |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you adjust these tests to not need global variables please? I'd suggest fetching everything you need for tests from the HTTP api rather than looking at the object itself. Then it's more of an end-to-end test as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is an ugly side effect of the pull/update/push promise behavior that happens in the constructor.
Adjusted so that setServerConfiguration returns the constructed parse server object instead of using a new global, is that a fair compromise?
The tests are split into two groups: testing ParseServer constructor behavior & testing the settings router e2e. Testing the constructor behavior benefits from having the constructed Parse Server object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that seems like a good solution.