Skip to content

Commit 565c059

Browse files
NotBobTheBuilderNotBobTheBuilder
committed
Allow disabling workaround for fixed MongoDB bug
1 parent b804245 commit 565c059

File tree

7 files changed

+196
-90
lines changed

7 files changed

+196
-90
lines changed

‎spec/DatabaseController.spec.js

Lines changed: 102 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3,61 +3,118 @@ const validateQuery = DatabaseController._validateQuery;
33

44
describe('DatabaseController', function() {
55
describe('validateQuery', function() {
6-
it('should restructure simple cases of SERVER-13732', done => {
7-
const query = {
8-
$or: [{ a: 1 }, { a: 2 }],
9-
_rperm: { $in: ['a', 'b'] },
10-
foo: 3,
11-
};
12-
validateQuery(query);
13-
expect(query).toEqual({
14-
$or: [
15-
{ a: 1, _rperm: { $in: ['a', 'b'] }, foo: 3 },
16-
{ a: 2, _rperm: { $in: ['a', 'b'] }, foo: 3 },
17-
],
18-
});
19-
done();
20-
});
6+
describe('with skipMongoDBServer13732Workaround disabled (the default)', function() {
7+
it('should restructure simple cases of SERVER-13732', done => {
8+
const query = {
9+
$or: [{ a: 1 }, { a: 2 }],
10+
_rperm: { $in: ['a', 'b'] },
11+
foo: 3,
12+
};
13+
validateQuery(query, true);
14+
expect(query).toEqual({
15+
$or: [
16+
{ a: 1, _rperm: { $in: ['a', 'b'] }, foo: 3 },
17+
{ a: 2, _rperm: { $in: ['a', 'b'] }, foo: 3 },
18+
],
19+
});
20+
done();
21+
});
2122

22-
it('should not restructure SERVER-13732 queries with $nears', done => {
23-
let query = { $or: [{ a: 1 }, { b: 1 }], c: { $nearSphere: {} } };
24-
validateQuery(query);
25-
expect(query).toEqual({
26-
$or: [{ a: 1 }, { b: 1 }],
27-
c: { $nearSphere: {} },
23+
it('should not restructure SERVER-13732 queries with $nears', done => {
24+
let query = { $or: [{ a: 1 }, { b: 1 }], c: { $nearSphere: {} } };
25+
validateQuery(query, true);
26+
expect(query).toEqual({
27+
$or: [{ a: 1 }, { b: 1 }],
28+
c: { $nearSphere: {} },
29+
});
30+
query = { $or: [{ a: 1 }, { b: 1 }], c: { $near: {} } };
31+
validateQuery(query, true);
32+
expect(query).toEqual({ $or: [{ a: 1 }, { b: 1 }], c: { $near: {} } });
33+
done();
2834
});
2935

30-
query = { $or: [{ a: 1 }, { b: 1 }], c: { $near: {} } };
31-
validateQuery(query);
32-
expect(query).toEqual({ $or: [{ a: 1 }, { b: 1 }], c: { $near: {} } });
36+
it('should push refactored keys down a tree for SERVER-13732', done => {
37+
const query = {
38+
a: 1,
39+
$or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }],
40+
};
41+
validateQuery(query, true);
42+
expect(query).toEqual({
43+
$or: [
44+
{ $or: [{ b: 1, a: 1 }, { b: 2, a: 1 }] },
45+
{ $or: [{ c: 1, a: 1 }, { c: 2, a: 1 }] },
46+
],
47+
});
48+
done();
49+
});
3350

34-
done();
51+
it('should reject invalid queries', done => {
52+
expect(() => validateQuery({ $or: { a: 1 } }, true)).toThrow();
53+
done();
54+
});
55+
56+
it('should accept valid queries', done => {
57+
expect(() =>
58+
validateQuery({ $or: [{ a: 1 }, { b: 2 }] }, true)
59+
).not.toThrow();
60+
done();
61+
});
3562
});
3663

37-
it('should push refactored keys down a tree for SERVER-13732', done => {
38-
const query = {
39-
a: 1,
40-
$or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }],
41-
};
42-
validateQuery(query);
43-
expect(query).toEqual({
44-
$or: [
45-
{ $or: [{ b: 1, a: 1 }, { b: 2, a: 1 }] },
46-
{ $or: [{ c: 1, a: 1 }, { c: 2, a: 1 }] },
47-
],
64+
describe('with skipMongoDBServer13732Workaround enabled', function() {
65+
it('should not restructure simple cases of SERVER-13732', done => {
66+
const query = {
67+
$or: [{ a: 1 }, { a: 2 }],
68+
_rperm: { $in: ['a', 'b'] },
69+
foo: 3,
70+
};
71+
validateQuery(query, false);
72+
expect(query).toEqual({
73+
$or: [{ a: 1 }, { a: 2 }],
74+
_rperm: { $in: ['a', 'b'] },
75+
foo: 3,
76+
});
77+
done();
4878
});
4979

50-
done();
51-
});
80+
it('should not restructure SERVER-13732 queries with $nears', done => {
81+
let query = { $or: [{ a: 1 }, { b: 1 }], c: { $nearSphere: {} } };
82+
validateQuery(query, false);
83+
expect(query).toEqual({
84+
$or: [{ a: 1 }, { b: 1 }],
85+
c: { $nearSphere: {} },
86+
});
87+
query = { $or: [{ a: 1 }, { b: 1 }], c: { $near: {} } };
88+
validateQuery(query, false);
89+
expect(query).toEqual({ $or: [{ a: 1 }, { b: 1 }], c: { $near: {} } });
90+
done();
91+
});
5292

53-
it('should reject invalid queries', done => {
54-
expect(() => validateQuery({ $or: { a: 1 } })).toThrow();
55-
done();
56-
});
93+
it('should not push refactored keys down a tree for SERVER-13732', done => {
94+
const query = {
95+
a: 1,
96+
$or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }],
97+
};
98+
validateQuery(query, false);
99+
expect(query).toEqual({
100+
a: 1,
101+
$or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }],
102+
});
103+
104+
done();
105+
});
57106

58-
it('should accept valid queries', done => {
59-
expect(() => validateQuery({ $or: [{ a: 1 }, { b: 2 }] })).not.toThrow();
60-
done();
107+
it('should reject invalid queries', done => {
108+
expect(() => validateQuery({ $or: { a: 1 } }, false)).toThrow();
109+
done();
110+
});
111+
112+
it('should accept valid queries', done => {
113+
expect(() =>
114+
validateQuery({ $or: [{ a: 1 }, { b: 2 }] }, false)
115+
).not.toThrow();
116+
done();
117+
});
61118
});
62119
});
63120
});

‎src/Config.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ export class Config {
3434
);
3535
config.database = new DatabaseController(
3636
cacheInfo.databaseController.adapter,
37-
schemaCache
37+
schemaCache,
38+
cacheInfo.skipMongoDBServer13732Workaround
3839
);
3940
} else {
4041
config[key] = cacheInfo[key];

‎src/Controllers/DatabaseController.js

Lines changed: 78 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -69,48 +69,73 @@ const isSpecialQueryKey = key => {
6969
return specialQuerykeys.indexOf(key) >= 0;
7070
};
7171

72-
const validateQuery = (query: any): void => {
72+
const validateQuery = (
73+
query: any,
74+
skipMongoDBServer13732Workaround: boolean
75+
): void => {
7376
if (query.ACL) {
7477
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot query on ACL.');
7578
}
7679

7780
if (query.$or) {
7881
if (query.$or instanceof Array) {
79-
query.$or.forEach(validateQuery);
80-
81-
/* In MongoDB, $or queries which are not alone at the top level of the
82-
* query can not make efficient use of indexes due to a long standing
83-
* bug known as SERVER-13732.
84-
*
85-
* This block restructures queries in which $or is not the sole top
86-
* level element by moving all other top-level predicates inside every
87-
* subdocument of the $or predicate, allowing MongoDB's query planner
88-
* to make full use of the most relevant indexes.
89-
*
90-
* EG: {$or: [{a: 1}, {a: 2}], b: 2}
91-
* Becomes: {$or: [{a: 1, b: 2}, {a: 2, b: 2}]}
92-
*
93-
* The only exceptions are $near and $nearSphere operators, which are
94-
* constrained to only 1 operator per query. As a result, these ops
95-
* remain at the top level
96-
*
97-
* https://jira.mongodb.org/browse/SERVER-13732
98-
* https://github.com/parse-community/parse-server/issues/3767
99-
*/
100-
Object.keys(query).forEach(key => {
101-
const noCollisions = !query.$or.some(subq => subq.hasOwnProperty(key));
102-
let hasNears = false;
103-
if (query[key] != null && typeof query[key] == 'object') {
104-
hasNears = '$near' in query[key] || '$nearSphere' in query[key];
105-
}
106-
if (key != '$or' && noCollisions && !hasNears) {
107-
query.$or.forEach(subquery => {
108-
subquery[key] = query[key];
109-
});
110-
delete query[key];
111-
}
112-
});
113-
query.$or.forEach(validateQuery);
82+
query.$or.forEach(el =>
83+
validateQuery(el, skipMongoDBServer13732Workaround)
84+
);
85+
86+
if (!skipMongoDBServer13732Workaround) {
87+
/* In MongoDB 3.2 & 3.4, $or queries which are not alone at the top
88+
* level of the query can not make efficient use of indexes due to a
89+
* long standing bug known as SERVER-13732.
90+
*
91+
* This bug was fixed in MongoDB version 3.6.
92+
*
93+
* For versions pre-3.6, the below logic produces a substantial
94+
* performance improvement inside the database by avoiding the bug.
95+
*
96+
* For versions 3.6 and above, there is no performance improvement and
97+
* the logic is unnecessary. Some query patterns are even slowed by
98+
* the below logic, due to the bug having been fixed and better
99+
* query plans being chosen.
100+
*
101+
* When versions before 3.4 are no longer supported by this project,
102+
* this logic, and the accompanying `skipMongoDBServer13732Workaround`
103+
* flag, can be removed.
104+
*
105+
* This block restructures queries in which $or is not the sole top
106+
* level element by moving all other top-level predicates inside every
107+
* subdocument of the $or predicate, allowing MongoDB's query planner
108+
* to make full use of the most relevant indexes.
109+
*
110+
* EG: {$or: [{a: 1}, {a: 2}], b: 2}
111+
* Becomes: {$or: [{a: 1, b: 2}, {a: 2, b: 2}]}
112+
*
113+
* The only exceptions are $near and $nearSphere operators, which are
114+
* constrained to only 1 operator per query. As a result, these ops
115+
* remain at the top level
116+
*
117+
* https://jira.mongodb.org/browse/SERVER-13732
118+
* https://github.com/parse-community/parse-server/issues/3767
119+
*/
120+
Object.keys(query).forEach(key => {
121+
const noCollisions = !query.$or.some(subq =>
122+
subq.hasOwnProperty(key)
123+
);
124+
let hasNears = false;
125+
if (query[key] != null && typeof query[key] == 'object') {
126+
hasNears = '$near' in query[key] || '$nearSphere' in query[key];
127+
}
128+
if (key != '$or' && noCollisions && !hasNears) {
129+
query.$or.forEach(subquery => {
130+
subquery[key] = query[key];
131+
});
132+
delete query[key];
133+
}
134+
});
135+
query.$or.forEach(el =>
136+
validateQuery(el, skipMongoDBServer13732Workaround)
137+
);
138+
}
114139
} else {
115140
throw new Parse.Error(
116141
Parse.Error.INVALID_QUERY,
@@ -121,7 +146,9 @@ const validateQuery = (query: any): void => {
121146

122147
if (query.$and) {
123148
if (query.$and instanceof Array) {
124-
query.$and.forEach(validateQuery);
149+
query.$and.forEach(el =>
150+
validateQuery(el, skipMongoDBServer13732Workaround)
151+
);
125152
} else {
126153
throw new Parse.Error(
127154
Parse.Error.INVALID_QUERY,
@@ -132,7 +159,9 @@ const validateQuery = (query: any): void => {
132159

133160
if (query.$nor) {
134161
if (query.$nor instanceof Array && query.$nor.length > 0) {
135-
query.$nor.forEach(validateQuery);
162+
query.$nor.forEach(el =>
163+
validateQuery(el, skipMongoDBServer13732Workaround)
164+
);
136165
} else {
137166
throw new Parse.Error(
138167
Parse.Error.INVALID_QUERY,
@@ -381,14 +410,20 @@ class DatabaseController {
381410
adapter: StorageAdapter;
382411
schemaCache: any;
383412
schemaPromise: ?Promise<SchemaController.SchemaController>;
413+
skipMongoDBServer13732Workaround: boolean;
384414

385-
constructor(adapter: StorageAdapter, schemaCache: any) {
415+
constructor(
416+
adapter: StorageAdapter,
417+
schemaCache: any,
418+
skipMongoDBServer13732Workaround: boolean
419+
) {
386420
this.adapter = adapter;
387421
this.schemaCache = schemaCache;
388422
// We don't want a mutable this.schema, because then you could have
389423
// one request that uses different schemas for different parts of
390424
// it. Instead, use loadSchema to get a schema.
391425
this.schemaPromise = null;
426+
this.skipMongoDBServer13732Workaround = skipMongoDBServer13732Workaround;
392427
}
393428

394429
collectionExists(className: string): Promise<boolean> {
@@ -512,7 +547,7 @@ class DatabaseController {
512547
if (acl) {
513548
query = addWriteACL(query, acl);
514549
}
515-
validateQuery(query);
550+
validateQuery(query, this.skipMongoDBServer13732Workaround);
516551
return schemaController
517552
.getOneSchema(className, true)
518553
.catch(error => {
@@ -782,7 +817,7 @@ class DatabaseController {
782817
if (acl) {
783818
query = addWriteACL(query, acl);
784819
}
785-
validateQuery(query);
820+
validateQuery(query, this.skipMongoDBServer13732Workaround);
786821
return schemaController
787822
.getOneSchema(className)
788823
.catch(error => {
@@ -1274,7 +1309,7 @@ class DatabaseController {
12741309
query = addReadACL(query, aclGroup);
12751310
}
12761311
}
1277-
validateQuery(query);
1312+
validateQuery(query, this.skipMongoDBServer13732Workaround);
12781313
if (count) {
12791314
if (!classExists) {
12801315
return 0;
@@ -1539,7 +1574,7 @@ class DatabaseController {
15391574
]);
15401575
}
15411576

1542-
static _validateQuery: any => void;
1577+
static _validateQuery: (any, boolean) => void;
15431578
}
15441579

15451580
module.exports = DatabaseController;

‎src/Controllers/index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ export function getDatabaseController(
147147
const {
148148
databaseURI,
149149
databaseOptions,
150+
skipMongoDBServer13732Workaround,
150151
collectionPrefix,
151152
schemaCacheTTL,
152153
enableSingleSchemaCache,
@@ -170,7 +171,8 @@ export function getDatabaseController(
170171
}
171172
return new DatabaseController(
172173
databaseAdapter,
173-
new SchemaCache(cacheController, schemaCacheTTL, enableSingleSchemaCache)
174+
new SchemaCache(cacheController, schemaCacheTTL, enableSingleSchemaCache),
175+
skipMongoDBServer13732Workaround
174176
);
175177
}
176178

‎src/Options/Definitions.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,12 @@ module.exports.ParseServerOptions = {
320320
help: 'Disables console output',
321321
action: parsers.booleanParser,
322322
},
323+
skipMongoDBServer13732Workaround: {
324+
env: 'PARSE_SKIP_MONGODB_SERVER_13732_WORKAROUND',
325+
help: 'Circumvent Parse workaround for historical MongoDB bug SERVER-13732',
326+
action: parsers.booleanParser,
327+
default: false,
328+
},
323329
startLiveQueryServer: {
324330
env: 'PARSE_SERVER_START_LIVE_QUERY_SERVER',
325331
help: 'Starts the liveQuery server',

0 commit comments

Comments
 (0)