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

Make GlobalConfig work like parse.com #1210

Merged
merged 4 commits into from
Mar 27, 2016
Merged

Make GlobalConfig work like parse.com #1210

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
114f7d1
Make global config visible from parse-dashboard when it's enabled
framp Mar 26, 2016
94b10de
Fixed config upsert implementation to handle nested object and __op:D…
framp Mar 27, 2016
6b9fd16
Added merging in Config with scoped set
framp Mar 27, 2016
236c7d1
Refactor code
framp Mar 27, 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
10 changes: 6 additions & 4 deletions src/Routers/FeaturesRouter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@ import { version } from '../../package.json';
import PromiseRouter from '../PromiseRouter';
import * as middleware from "../middlewares";

const isGlobalConfigEnabled = !!(process.env.PARSE_EXPERIMENTAL_CONFIG_ENABLED || process.env.TESTING)

export class FeaturesRouter extends PromiseRouter {
mountRoutes() {
this.route('GET','/serverInfo', middleware.promiseEnforceMasterKeyAccess, req => {
const features = {
globalConfig: {
create: false,
read: false,
update: false,
delete: false,
create: isGlobalConfigEnabled,
read: isGlobalConfigEnabled,
update: isGlobalConfigEnabled,
delete: isGlobalConfigEnabled,
},
hooks: {
create: false,
Expand Down
15 changes: 13 additions & 2 deletions src/Routers/GlobalConfigRouter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,19 @@ export class GlobalConfigRouter extends PromiseRouter {

updateGlobalConfig(req) {
return req.config.database.adaptiveCollection('_GlobalConfig')
.then(coll => coll.upsertOne({ _id: 1 }, { $set: req.body }))
.then(() => ({ response: { result: true } }));
.then(coll => coll.find({ '_id': 1 }, { limit: 1 }))
.then(results => {
const previousConfig = results && results[0] && results[0].params || {};
Copy link
Contributor

Choose a reason for hiding this comment

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

You should do it in one op, and use $unset when op delete is set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@flovilmart:
The main problem is that I can't merge nested documents with $set afaik.
The whole params object get replaced with the new one.

{"params": {"a":"b"}}
In mongodb:
{"params": {"a":"a", "b":"b"}}
Result:
{"params": {"a":"b"}}
Expected:
{"params": {"a":"b", "b": "b"}}

Copy link
Contributor

Choose a reason for hiding this comment

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

Shoot... It's been poorly designed... Can you try with dot notation? I believe that works also $unset supports dot notation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. Dot notation works and some params manipulation works, awesome!

const newConfig = Object.assign({}, previousConfig, req.body.params);
for (var key in newConfig) {
if (newConfig[key] && newConfig[key].__op && newConfig[key].__op === "Delete") {
delete newConfig[key];
}
}
return req.config.database.adaptiveCollection('_GlobalConfig')
.then(coll => coll.upsertOne({ _id: 1 }, { $set: { params: newConfig } }))
.then(() => ({ response: { result: true } }));
})
}

mountRoutes() {
Expand Down