From fa8c902e2b13e615a9adbf5893acd94a437cdd4d Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Mon, 30 Apr 2018 19:15:11 -0400 Subject: [PATCH 01/13] #4678: Converting strings to Date when schema.type is Date within aggregate function --- .../Storage/Mongo/MongoStorageAdapter.js | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 3c543090f5..60f322282f 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -570,12 +570,28 @@ export class MongoStorageAdapter implements StorageAdapter { const transformMatch = { [`_p_${field}`] : `${schema.fields[field].targetClass}$${stage.$match[field]}` }; stage.$match = transformMatch; } + else if (schema.fields[field] && schema.fields[field].type === 'Date') { + const transformMatch = { [`${field}`]: new Date(stage.$match[field]) }; + stage.$match = transformMatch; + } if (field === 'objectId') { const transformMatch = Object.assign({}, stage.$match); transformMatch._id = stage.$match[field]; delete transformMatch.objectId; stage.$match = transformMatch; } + else if (field === 'createdAt') { + const transformMatch = Object.assign({}, stage.$match); + transformMatch._created_at = stage.$match[field]; + delete transformMatch.createdAt; + stage.$match = transformMatch; + } + else if (field === 'updatedAt') { + const transformMatch = Object.assign({}, stage.$match); + transformMatch._updated_at = stage.$match[field]; + delete transformMatch.updatedAt; + stage.$match = transformMatch; + } } } return stage; From 9a12b7c04187c379de415517dc25f8b0073e9279 Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Mon, 30 Apr 2018 19:51:31 -0400 Subject: [PATCH 02/13] Added test cases to test new date match aggregate query --- spec/ParseQuery.Aggregate.spec.js | 54 +++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index c698395085..1e278bc1d7 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -474,6 +474,60 @@ describe('Parse.Query Aggregate testing', () => { }); }); + it('match date query - createdAt', (done) => { + const obj1 = new TestObject(); + const obj2 = new TestObject(); + + Parse.Object.saveAll([obj1, obj2]).then(() => { + const now = new Date(); + const today = new Date(now.getFullYear(), now.getMonth(), now.getDate()); + const pipeline = [ + { match: { 'createdAt': today } } + ]; + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(2); + done(); + }); + }); + + it('match date query - updatedAt', (done) => { + const obj1 = new TestObject(); + const obj2 = new TestObject(); + + Parse.Object.saveAll([obj1, obj2]).then(() => { + const now = new Date(); + const today = new Date(now.getFullYear(), now.getMonth(), now.getDate()); + const pipeline = [ + { match: { 'updatedAt': today } } + ]; + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(2); + done(); + }); + }); + + it('match date query - empty', (done) => { + const obj1 = new TestObject(); + const obj2 = new TestObject(); + + Parse.Object.saveAll([obj1, obj2]).then(() => { + const now = new Date(); + const future = new Date(now.getFullYear(), now.getMonth()+1, now.getDate()); + const pipeline = [ + { match: { 'createdAt': future } } + ]; + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(0); + done(); + }); + }); + it('project query', (done) => { const options = Object.assign({}, masterKeyOptions, { body: { From 044453a85b2180137db19b4962cd0eaa08c4cfd3 Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Tue, 1 May 2018 18:38:27 -0400 Subject: [PATCH 03/13] Added function to parse match aggregate arguments and convert necessary values to Date objects --- spec/ParseQuery.Aggregate.spec.js | 6 +- .../Storage/Mongo/MongoStorageAdapter.js | 78 ++++++++++++------- 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 1e278bc1d7..4cc70170b5 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -482,7 +482,7 @@ describe('Parse.Query Aggregate testing', () => { const now = new Date(); const today = new Date(now.getFullYear(), now.getMonth(), now.getDate()); const pipeline = [ - { match: { 'createdAt': today } } + { match: { 'createdAt': { $gte: today } } } ]; const query = new Parse.Query(TestObject); return query.aggregate(pipeline); @@ -500,7 +500,7 @@ describe('Parse.Query Aggregate testing', () => { const now = new Date(); const today = new Date(now.getFullYear(), now.getMonth(), now.getDate()); const pipeline = [ - { match: { 'updatedAt': today } } + { match: { 'updatedAt': { $gte: today } } } ]; const query = new Parse.Query(TestObject); return query.aggregate(pipeline); @@ -516,7 +516,7 @@ describe('Parse.Query Aggregate testing', () => { Parse.Object.saveAll([obj1, obj2]).then(() => { const now = new Date(); - const future = new Date(now.getFullYear(), now.getMonth()+1, now.getDate()); + const future = new Date(now.getFullYear(), now.getMonth() + 1, now.getDate()); const pipeline = [ { match: { 'createdAt': future } } ]; diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 60f322282f..014208b24c 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -565,34 +565,7 @@ export class MongoStorageAdapter implements StorageAdapter { } } if (stage.$match) { - for (const field in stage.$match) { - if (schema.fields[field] && schema.fields[field].type === 'Pointer') { - const transformMatch = { [`_p_${field}`] : `${schema.fields[field].targetClass}$${stage.$match[field]}` }; - stage.$match = transformMatch; - } - else if (schema.fields[field] && schema.fields[field].type === 'Date') { - const transformMatch = { [`${field}`]: new Date(stage.$match[field]) }; - stage.$match = transformMatch; - } - if (field === 'objectId') { - const transformMatch = Object.assign({}, stage.$match); - transformMatch._id = stage.$match[field]; - delete transformMatch.objectId; - stage.$match = transformMatch; - } - else if (field === 'createdAt') { - const transformMatch = Object.assign({}, stage.$match); - transformMatch._created_at = stage.$match[field]; - delete transformMatch.createdAt; - stage.$match = transformMatch; - } - else if (field === 'updatedAt') { - const transformMatch = Object.assign({}, stage.$match); - transformMatch._updated_at = stage.$match[field]; - delete transformMatch.updatedAt; - stage.$match = transformMatch; - } - } + this._parseAggregateArgs(schema, stage.$match); } return stage; }); @@ -624,6 +597,55 @@ export class MongoStorageAdapter implements StorageAdapter { .catch(err => this.handleError(err)); } + _parseAggregateArgs(schema: any, pipeline: any): any { + var rtnval = {}; + if (Array.isArray(pipeline)) { + rtnval = []; + for (const field in pipeline) { + rtnval.push(this._parseAggregateArgs(schema, pipeline[field])); + } + } + else if (typeof pipeline === 'object') { + for (const field in pipeline) { + if (schema.fields[field] && schema.fields[field].type === 'Pointer') { + rtnval[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; + } + else if (schema.fields[field] && schema.fields[field].type === 'Date') { + rtnval[field] = this._convertToDate(pipeline[field]); + } + else { + rtnval[field] = this._parseAggregateArgs(schema, pipeline[field]); + } + + if (field === 'objectId') { + rtnval['_id'] = rtnval[field]; + delete rtnval[field]; + } + else if (field === 'createdAt') { + rtnval['_created_at'] = rtnval[field]; + delete rtnval[field]; + } + else if (field === 'updatedAt') { + rtnval['_updated_at'] = rtnval[field]; + delete rtnval[field]; + } + } + } + return rtnval; + } + + _convertToDate(value: any): any { + if (typeof value === 'string') { + return new Date(value); + } + + var rtnval = {} + for (const field in value) { + rtnval[field] = this._convertToDate(value[field]) + } + return rtnval; + } + _parseReadPreference(readPreference: ?string): ?string { switch (readPreference) { case 'PRIMARY': From 17147114c4f5de7f7e7257a983b65de592ee4b6d Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Tue, 1 May 2018 20:04:50 -0400 Subject: [PATCH 04/13] Added missing return value --- src/Adapters/Storage/Mongo/MongoStorageAdapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 014208b24c..df312b1a1f 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -565,7 +565,7 @@ export class MongoStorageAdapter implements StorageAdapter { } } if (stage.$match) { - this._parseAggregateArgs(schema, stage.$match); + stage.$match = this._parseAggregateArgs(schema, stage.$match); } return stage; }); From d922a7ab77ca906fa068a9acad441040480df839 Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Fri, 4 May 2018 11:24:21 -0400 Subject: [PATCH 05/13] Improved code quality based on suggestions and figured out why tests were failing --- spec/ParseQuery.Aggregate.spec.js | 6 ++- .../Storage/Mongo/MongoStorageAdapter.js | 47 +++++++++++++------ 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 4cc70170b5..ec21f1a7a5 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -487,7 +487,8 @@ describe('Parse.Query Aggregate testing', () => { const query = new Parse.Query(TestObject); return query.aggregate(pipeline); }).then((results) => { - expect(results.length).toEqual(2); + // Four objects were created initially, we added two more. + expect(results.length).toEqual(6); done(); }); }); @@ -505,7 +506,8 @@ describe('Parse.Query Aggregate testing', () => { const query = new Parse.Query(TestObject); return query.aggregate(pipeline); }).then((results) => { - expect(results.length).toEqual(2); + // Four objects were added initially, we added two more. + expect(results.length).toEqual(6); done(); }); }); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 0685465fa7..239b3cb8a0 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -597,49 +597,66 @@ export class MongoStorageAdapter implements StorageAdapter { .catch(err => this.handleError(err)); } + // This function will recursively traverse the pipeline and convert any Pointer or Date columns. + // If we detect a pointer column we will rename the column being queried for to match the column + // in the database. We also modify the value to what we expect the value to be in the database + // as well. + // For dates, the driver expects a Date object, but we have a string coming in. So we'll convert + // the string to a Date so the driver can perform the necessary comparison. + // + // The goal of this method is to look for the "leaves" of the pipeline and determine if it needs + // to be converted. The pipeline can have a few different forms. For more details, see: + // https://docs.mongodb.com/manual/reference/operator/aggregation/ + // + // If the pipeline is an array, it means we are probably parsing an '$and' or '$or' operator. In + // that case we need to loop through all of it's children to find the columns being operated on. + // If the pipeline is an object, then we'll loop through the keys checking to see if the key name + // matches one of the schema columns. If it does match a column and the column is a Pointer or + // a Date, then we'll convert the value as described above. + // + // As much as I hate recursion...this seemed like a good fit for it. We're essentially traversing + // down a tree to find a "leaf node" and checking to see if it needs to be converted. _parseAggregateArgs(schema: any, pipeline: any): any { - var rtnval = {}; if (Array.isArray(pipeline)) { - rtnval = []; - for (const field in pipeline) { - rtnval.push(this._parseAggregateArgs(schema, pipeline[field])); - } + return pipeline.map((value) => this._parseAggregateArgs(schema, value)); } else if (typeof pipeline === 'object') { + const rtnval = {}; for (const field in pipeline) { if (schema.fields[field] && schema.fields[field].type === 'Pointer') { rtnval[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; - } - else if (schema.fields[field] && schema.fields[field].type === 'Date') { + } else if (schema.fields[field] && schema.fields[field].type === 'Date') { rtnval[field] = this._convertToDate(pipeline[field]); - } - else { + } else { rtnval[field] = this._parseAggregateArgs(schema, pipeline[field]); } if (field === 'objectId') { rtnval['_id'] = rtnval[field]; delete rtnval[field]; - } - else if (field === 'createdAt') { + } else if (field === 'createdAt') { rtnval['_created_at'] = rtnval[field]; delete rtnval[field]; - } - else if (field === 'updatedAt') { + } else if (field === 'updatedAt') { rtnval['_updated_at'] = rtnval[field]; delete rtnval[field]; } } + return rtnval; } - return rtnval; + return pipeline; } + // This function will attempt to convert the provided value to a Date object. Since this is part + // of an aggregation pipeline, the value can either be a string or it can be another object with + // an operator in it (like $gt, $lt, etc). Because of this I felt it was easier to make this a + // recursive method to traverse down to the "leaf node" which is going to be the string. _convertToDate(value: any): any { if (typeof value === 'string') { return new Date(value); } - var rtnval = {} + const rtnval = {} for (const field in value) { rtnval[field] = this._convertToDate(value[field]) } From 7261ba022a9dd9f6e8676bf6a0fd2a178f0a34c6 Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Sat, 19 May 2018 14:25:26 -0400 Subject: [PATCH 06/13] Added tests from @dplewis --- spec/ParseQuery.Aggregate.spec.js | 101 ++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index ec21f1a7a5..1d5fb900c0 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -164,6 +164,32 @@ describe('Parse.Query Aggregate testing', () => { }); }); + it('group by date object transform', (done) => { + const obj1 = new TestObject(); + const obj2 = new TestObject(); + const obj3 = new TestObject(); + const pipeline = [{ + group: { + objectId: { day: { $dayOfMonth: "$updatedAt" }, month: { $month: "$createdAt" }, year: { $year: "$createdAt" } }, + count: { $sum: 1 } + } + }]; + Parse.Object.saveAll([obj1, obj2, obj3]).then(() => { + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + const createdAt = new Date(obj1.createdAt); + expect(results[0].objectId.day).toEqual(createdAt.getUTCDate()); + expect(results[0].objectId.month).toEqual(createdAt.getMonth() + 1); + expect(results[0].objectId.year).toEqual(createdAt.getUTCFullYear()); + done(); + }).catch((error) => { + console.log(error); + console.log('error'); + done(); + }); + }); + it_exclude_dbs(['postgres'])('cannot group by date field (excluding createdAt and updatedAt)', (done) => { const obj1 = new TestObject({ dateField: new Date(1990, 11, 1) }); const obj2 = new TestObject({ dateField: new Date(1990, 5, 1) }); @@ -339,6 +365,27 @@ describe('Parse.Query Aggregate testing', () => { }).catch(done.fail); }); + it('match comparison date query', (done) => { + const today = new Date(); + const yesterday = new Date(); + const tomorrow = new Date(); + yesterday.setDate(today.getDate() - 1); + tomorrow.setDate(today.getDate() + 1); + const obj1 = new TestObject({ dateField: yesterday }); + const obj2 = new TestObject({ dateField: today }); + const obj3 = new TestObject({ dateField: tomorrow }); + const pipeline = [ + { match: { dateField: { $lt: tomorrow } } } + ]; + Parse.Object.saveAll([obj1, obj2, obj3]).then(() => { + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toBe(2); + done(); + }); + }); + it('match comparison query', (done) => { const options = Object.assign({}, masterKeyOptions, { body: { @@ -474,6 +521,17 @@ describe('Parse.Query Aggregate testing', () => { }); }); + it('match exists query', (done) => { + const pipeline = [ + { match: { score: { $exists: true } } } + ]; + const query = new Parse.Query(TestObject); + query.aggregate(pipeline).then((results) => { + expect(results.length).toEqual(4); + done(); + }); + }); + it('match date query - createdAt', (done) => { const obj1 = new TestObject(); const obj2 = new TestObject(); @@ -530,6 +588,29 @@ describe('Parse.Query Aggregate testing', () => { }); }); + it_exclude_dbs(['postgres'])('match pointer with operator query', (done) => { + const pointer = new PointerObject(); + + const obj1 = new TestObject({ pointer }); + const obj2 = new TestObject({ pointer }); + const obj3 = new TestObject(); + + Parse.Object.saveAll([pointer, obj1, obj2, obj3]).then(() => { + const pipeline = [ + { match: { pointer: { $exists: true } } } + ]; + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(2); + expect(results[0].pointer.objectId).toEqual(pointer.id); + expect(results[1].pointer.objectId).toEqual(pointer.id); + expect(results.some(result => result.objectId === obj1.id)).toEqual(true); + expect(results.some(result => result.objectId === obj2.id)).toEqual(true); + done(); + }); + }); + it('project query', (done) => { const options = Object.assign({}, masterKeyOptions, { body: { @@ -568,6 +649,26 @@ describe('Parse.Query Aggregate testing', () => { }).catch(done.fail); }); + it('project pointer query', (done) => { + const pointer = new PointerObject(); + const obj = new TestObject({ pointer, name: 'hello' }); + + obj.save().then(() => { + const pipeline = [ + { match: { objectId: obj.id } }, + { project: { pointer: 1, name: 1, createdAt: 1 } } + ]; + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(1); + expect(results[0].name).toEqual('hello'); + expect(results[0].createdAt).not.toBe(undefined); + expect(results[0].pointer.objectId).toEqual(pointer.id); + done(); + }); + }); + it('project with group query', (done) => { const options = Object.assign({}, masterKeyOptions, { body: { From beb54bbb532b4aa518ca6345273a18b0c950a26b Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Sat, 19 May 2018 14:25:50 -0400 Subject: [PATCH 07/13] Supporting project aggregation as well as exists operator --- .../Storage/Mongo/MongoStorageAdapter.js | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 239b3cb8a0..ea4d4fe5dc 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -567,6 +567,9 @@ export class MongoStorageAdapter implements StorageAdapter { if (stage.$match) { stage.$match = this._parseAggregateArgs(schema, stage.$match); } + if (stage.$project) { + stage.$project = this._parseAggregateProjectArgs(schema, stage.$project); + } return stage; }); readPreference = this._parseReadPreference(readPreference); @@ -624,7 +627,15 @@ export class MongoStorageAdapter implements StorageAdapter { const rtnval = {}; for (const field in pipeline) { if (schema.fields[field] && schema.fields[field].type === 'Pointer') { - rtnval[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; + if (typeof pipeline[field] === 'object') { + // + // Pass objects down to MongoDB...this is more than likely an $exists operator. + // + rtnval[`_p_${field}`] = pipeline[field]; + } + else { + rtnval[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; + } } else if (schema.fields[field] && schema.fields[field].type === 'Date') { rtnval[field] = this._convertToDate(pipeline[field]); } else { @@ -647,6 +658,39 @@ export class MongoStorageAdapter implements StorageAdapter { return pipeline; } + // This function is slightly different than the one above. Rather than trying to combine these + // two functions and making the code even harder to understand, I decided to split it up. The + // difference with this function is we are not transforming the values, only the keys of the + // pipeline. + _parseAggregateProjectArgs(schema: any, pipeline: any): any { + if (Array.isArray(pipeline)) { + return pipeline.map((value) => this._parseAggregateProjectArgs(schema, value)); + } + else if (typeof pipeline === 'object') { + const rtnval = {}; + for (const field in pipeline) { + if (schema.fields[field] && schema.fields[field].type === 'Pointer') { + rtnval[`_p_${field}`] = pipeline[field]; + } else { + rtnval[field] = this._parseAggregateArgs(schema, pipeline[field]); + } + + if (field === 'objectId') { + rtnval['_id'] = rtnval[field]; + delete rtnval[field]; + } else if (field === 'createdAt') { + rtnval['_created_at'] = rtnval[field]; + delete rtnval[field]; + } else if (field === 'updatedAt') { + rtnval['_updated_at'] = rtnval[field]; + delete rtnval[field]; + } + } + return rtnval; + } + return pipeline; + } + // This function will attempt to convert the provided value to a Date object. Since this is part // of an aggregation pipeline, the value can either be a string or it can be another object with // an operator in it (like $gt, $lt, etc). Because of this I felt it was easier to make this a From abf42d963a7e0197885fe3e7d47ec448bc5af390 Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Tue, 22 May 2018 20:08:47 -0400 Subject: [PATCH 08/13] Excluding exists match for postgres --- spec/ParseQuery.Aggregate.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 1d5fb900c0..174fc1a098 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -521,7 +521,7 @@ describe('Parse.Query Aggregate testing', () => { }); }); - it('match exists query', (done) => { + it_exclude_dbs(['postgres'])('match exists query', (done) => { const pipeline = [ { match: { score: { $exists: true } } } ]; From da3afbbfcc9e690dc245924959a23133e7c40e6c Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Mon, 18 Jun 2018 21:04:29 -0400 Subject: [PATCH 09/13] Handling the $group operator similar to $match and $project --- .../Storage/Mongo/MongoStorageAdapter.js | 44 ++++++++++++++++--- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index bab9059f4d..2a0f80c812 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -557,11 +557,10 @@ export class MongoStorageAdapter implements StorageAdapter { aggregate(className: string, schema: any, pipeline: any, readPreference: ?string) { let isPointerField = false; pipeline = pipeline.map((stage) => { - if (stage.$group && stage.$group._id && (typeof stage.$group._id === 'string')) { - const field = stage.$group._id.substring(1); - if (schema.fields[field] && schema.fields[field].type === 'Pointer') { + if (stage.$group) { + stage.$group = this._parseAggregateGroupArgs(schema, stage.$group); + if (stage.$group._id && (typeof stage.$group._id === 'string') && stage.$group._id.indexOf('$_p_') >= 0) { isPointerField = true; - stage.$group._id = `$_p_${field}`; } } if (stage.$match) { @@ -632,8 +631,7 @@ export class MongoStorageAdapter implements StorageAdapter { // Pass objects down to MongoDB...this is more than likely an $exists operator. // rtnval[`_p_${field}`] = pipeline[field]; - } - else { + } else { rtnval[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; } } else if (schema.fields[field] && schema.fields[field].type === 'Date') { @@ -671,7 +669,7 @@ export class MongoStorageAdapter implements StorageAdapter { for (const field in pipeline) { if (schema.fields[field] && schema.fields[field].type === 'Pointer') { rtnval[`_p_${field}`] = pipeline[field]; - } else { + } else { rtnval[field] = this._parseAggregateArgs(schema, pipeline[field]); } @@ -691,6 +689,38 @@ export class MongoStorageAdapter implements StorageAdapter { return pipeline; } + // This function is slightly different than the two above. MongoDB $group aggregate looks like: + // { $group: { _id: , : { : }, ... } } + // The could be a column name, prefixed with the '$' character. We'll look for + // these and check to see if it is a 'Pointer' or if it's one of createdAt, + // updatedAt or objectId and change it accordingly. + _parseAggregateGroupArgs(schema: any, pipeline: any): any { + if (Array.isArray(pipeline)) { + return pipeline.map((value) => this._parseAggregateGroupArgs(schema, value)); + } + else if (typeof pipeline === 'object') { + const rtnval = {}; + for (const field in pipeline) { + rtnval[field] = this._parseAggregateGroupArgs(schema, pipeline[field]); + } + return rtnval; + } + else if (typeof pipeline === 'string') { + const field = pipeline.substring(1); + if (schema.fields[field] && schema.fields[field].type === 'Pointer') { + return `$_p_${field}`; + } + if (field === 'objectId') { + return '$_id'; + } else if (field == 'createdAt') { + return '$_created_at'; + } else if (field == 'updatedAt') { + return '$_updated_at'; + } + } + return pipeline; + } + // This function will attempt to convert the provided value to a Date object. Since this is part // of an aggregation pipeline, the value can either be a string or it can be another object with // an operator in it (like $gt, $lt, etc). Because of this I felt it was easier to make this a From a7c428d35be42a1c56b9d7669e7ecfd61d77ed3f Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Wed, 20 Jun 2018 17:54:36 -0400 Subject: [PATCH 10/13] Added more tests for better code coverage --- spec/ParseQuery.Aggregate.spec.js | 98 ++++++++++++++++++- .../Storage/Mongo/MongoStorageAdapter.js | 8 +- 2 files changed, 100 insertions(+), 6 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 174fc1a098..8ced66f6b0 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -183,9 +183,101 @@ describe('Parse.Query Aggregate testing', () => { expect(results[0].objectId.month).toEqual(createdAt.getMonth() + 1); expect(results[0].objectId.year).toEqual(createdAt.getUTCFullYear()); done(); - }).catch((error) => { - console.log(error); - console.log('error'); + }); + }); + + it('group and multiply transform', (done) => { + const obj1 = new TestObject({ name: 'item a', quantity: 2, price: 10 }); + const obj2 = new TestObject({ name: 'item b', quantity: 5, price: 5 }); + const pipeline = [{ + group: { + objectId: null, + total: { $sum: { $multiply: [ '$quantity', '$price' ] } } + } + }]; + Parse.Object.saveAll([obj1, obj2]).then(() => { + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(1); + expect(results[0].total).toEqual(45); + done(); + }); + }); + + it('project and multiply transform', (done) => { + const obj1 = new TestObject({ name: 'item a', quantity: 2, price: 10 }); + const obj2 = new TestObject({ name: 'item b', quantity: 5, price: 5 }); + const pipeline = [ + { + match: { quantity: { $exists: true } } + }, + { + project: { + name: 1, + total: { $multiply: [ '$quantity', '$price' ] } + } + } + ]; + Parse.Object.saveAll([obj1, obj2]).then(() => { + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(2); + if (results[0].name === 'item a') { + expect(results[0].total).toEqual(20); + expect(results[1].total).toEqual(25); + } + else { + expect(results[0].total).toEqual(25); + expect(results[1].total).toEqual(20); + } + done(); + }); + }); + + it('project without objectId transform', (done) => { + const obj1 = new TestObject({ name: 'item a', quantity: 2, price: 10 }); + const obj2 = new TestObject({ name: 'item b', quantity: 5, price: 5 }); + const pipeline = [ + { + match: { quantity: { $exists: true } } + }, + { + project: { + objectId: 0, + total: { $multiply: [ '$quantity', '$price' ] } + } + }, + { + sort: { total: 1 } + } + ]; + Parse.Object.saveAll([obj1, obj2]).then(() => { + const query = new Parse.Query(TestObject); + return query.aggregate(pipeline); + }).then((results) => { + expect(results.length).toEqual(2); + expect(results[0].total).toEqual(20); + expect(results[0].objectId).toEqual(undefined); + expect(results[1].total).toEqual(25); + expect(results[1].objectId).toEqual(undefined); + done(); + }); + }); + + it('project updatedAt only transform', (done) => { + const pipeline = [{ + project: { objectId: 0, updatedAt: 1 } + }]; + const query = new Parse.Query(TestObject); + query.aggregate(pipeline).then((results) => { + expect(results.length).toEqual(4); + for (let i = 0; i < results.length; i++) { + const item = results[i]; + expect(item.hasOwnProperty('updatedAt')).toEqual(true); + expect(item.hasOwnProperty('objectId')).toEqual(false); + } done(); }); }); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 2a0f80c812..bfd3ec5005 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -662,6 +662,10 @@ export class MongoStorageAdapter implements StorageAdapter { // pipeline. _parseAggregateProjectArgs(schema: any, pipeline: any): any { if (Array.isArray(pipeline)) { + // + // For $project aggregations, I don't think they can ever start with an array...so this + // might be dead code. + // return pipeline.map((value) => this._parseAggregateProjectArgs(schema, value)); } else if (typeof pipeline === 'object') { @@ -710,9 +714,7 @@ export class MongoStorageAdapter implements StorageAdapter { if (schema.fields[field] && schema.fields[field].type === 'Pointer') { return `$_p_${field}`; } - if (field === 'objectId') { - return '$_id'; - } else if (field == 'createdAt') { + if (field == 'createdAt') { return '$_created_at'; } else if (field == 'updatedAt') { return '$_updated_at'; From cc250791f44244fbf0f9edbcc465bc30bd860525 Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Wed, 20 Jun 2018 19:39:00 -0400 Subject: [PATCH 11/13] Excluding certain tests from being run on postgres --- spec/ParseQuery.Aggregate.spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 8ced66f6b0..6f03fadadb 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -186,7 +186,7 @@ describe('Parse.Query Aggregate testing', () => { }); }); - it('group and multiply transform', (done) => { + it_exclude_dbs(['postgres'])('group and multiply transform', (done) => { const obj1 = new TestObject({ name: 'item a', quantity: 2, price: 10 }); const obj2 = new TestObject({ name: 'item b', quantity: 5, price: 5 }); const pipeline = [{ @@ -205,7 +205,7 @@ describe('Parse.Query Aggregate testing', () => { }); }); - it('project and multiply transform', (done) => { + it_exclude_dbs(['postgres'])('project and multiply transform', (done) => { const obj1 = new TestObject({ name: 'item a', quantity: 2, price: 10 }); const obj2 = new TestObject({ name: 'item b', quantity: 5, price: 5 }); const pipeline = [ @@ -236,7 +236,7 @@ describe('Parse.Query Aggregate testing', () => { }); }); - it('project without objectId transform', (done) => { + it_exclude_dbs(['postgres'])('project without objectId transform', (done) => { const obj1 = new TestObject({ name: 'item a', quantity: 2, price: 10 }); const obj2 = new TestObject({ name: 'item b', quantity: 5, price: 5 }); const pipeline = [ From 1a51a822eac15d30c9f5a140d856c66decc27d3e Mon Sep 17 00:00:00 2001 From: Christopher Bland Date: Wed, 20 Jun 2018 20:31:56 -0400 Subject: [PATCH 12/13] Excluding one more test from postgres --- spec/ParseQuery.Aggregate.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index 6f03fadadb..7c064c8059 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -266,7 +266,7 @@ describe('Parse.Query Aggregate testing', () => { }); }); - it('project updatedAt only transform', (done) => { + it_exclude_dbs(['postgres'])('project updatedAt only transform', (done) => { const pipeline = [{ project: { objectId: 0, updatedAt: 1 } }]; From af531c13e0eb01a98d9aebd5354676a3328cc9c7 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Sat, 23 Jun 2018 21:45:54 -0500 Subject: [PATCH 13/13] clean up --- .../Storage/Mongo/MongoStorageAdapter.js | 94 ++++++++----------- 1 file changed, 39 insertions(+), 55 deletions(-) diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index bfd3ec5005..0ec3ae5a82 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -621,37 +621,34 @@ export class MongoStorageAdapter implements StorageAdapter { _parseAggregateArgs(schema: any, pipeline: any): any { if (Array.isArray(pipeline)) { return pipeline.map((value) => this._parseAggregateArgs(schema, value)); - } - else if (typeof pipeline === 'object') { - const rtnval = {}; + } else if (typeof pipeline === 'object') { + const returnValue = {}; for (const field in pipeline) { if (schema.fields[field] && schema.fields[field].type === 'Pointer') { if (typeof pipeline[field] === 'object') { - // // Pass objects down to MongoDB...this is more than likely an $exists operator. - // - rtnval[`_p_${field}`] = pipeline[field]; + returnValue[`_p_${field}`] = pipeline[field]; } else { - rtnval[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; + returnValue[`_p_${field}`] = `${schema.fields[field].targetClass}$${pipeline[field]}`; } } else if (schema.fields[field] && schema.fields[field].type === 'Date') { - rtnval[field] = this._convertToDate(pipeline[field]); + returnValue[field] = this._convertToDate(pipeline[field]); } else { - rtnval[field] = this._parseAggregateArgs(schema, pipeline[field]); + returnValue[field] = this._parseAggregateArgs(schema, pipeline[field]); } if (field === 'objectId') { - rtnval['_id'] = rtnval[field]; - delete rtnval[field]; + returnValue['_id'] = returnValue[field]; + delete returnValue[field]; } else if (field === 'createdAt') { - rtnval['_created_at'] = rtnval[field]; - delete rtnval[field]; + returnValue['_created_at'] = returnValue[field]; + delete returnValue[field]; } else if (field === 'updatedAt') { - rtnval['_updated_at'] = rtnval[field]; - delete rtnval[field]; + returnValue['_updated_at'] = returnValue[field]; + delete returnValue[field]; } } - return rtnval; + return returnValue; } return pipeline; } @@ -661,36 +658,26 @@ export class MongoStorageAdapter implements StorageAdapter { // difference with this function is we are not transforming the values, only the keys of the // pipeline. _parseAggregateProjectArgs(schema: any, pipeline: any): any { - if (Array.isArray(pipeline)) { - // - // For $project aggregations, I don't think they can ever start with an array...so this - // might be dead code. - // - return pipeline.map((value) => this._parseAggregateProjectArgs(schema, value)); - } - else if (typeof pipeline === 'object') { - const rtnval = {}; - for (const field in pipeline) { - if (schema.fields[field] && schema.fields[field].type === 'Pointer') { - rtnval[`_p_${field}`] = pipeline[field]; - } else { - rtnval[field] = this._parseAggregateArgs(schema, pipeline[field]); - } + const returnValue = {}; + for (const field in pipeline) { + if (schema.fields[field] && schema.fields[field].type === 'Pointer') { + returnValue[`_p_${field}`] = pipeline[field]; + } else { + returnValue[field] = this._parseAggregateArgs(schema, pipeline[field]); + } - if (field === 'objectId') { - rtnval['_id'] = rtnval[field]; - delete rtnval[field]; - } else if (field === 'createdAt') { - rtnval['_created_at'] = rtnval[field]; - delete rtnval[field]; - } else if (field === 'updatedAt') { - rtnval['_updated_at'] = rtnval[field]; - delete rtnval[field]; - } + if (field === 'objectId') { + returnValue['_id'] = returnValue[field]; + delete returnValue[field]; + } else if (field === 'createdAt') { + returnValue['_created_at'] = returnValue[field]; + delete returnValue[field]; + } else if (field === 'updatedAt') { + returnValue['_updated_at'] = returnValue[field]; + delete returnValue[field]; } - return rtnval; } - return pipeline; + return returnValue; } // This function is slightly different than the two above. MongoDB $group aggregate looks like: @@ -701,20 +688,17 @@ export class MongoStorageAdapter implements StorageAdapter { _parseAggregateGroupArgs(schema: any, pipeline: any): any { if (Array.isArray(pipeline)) { return pipeline.map((value) => this._parseAggregateGroupArgs(schema, value)); - } - else if (typeof pipeline === 'object') { - const rtnval = {}; + } else if (typeof pipeline === 'object') { + const returnValue = {}; for (const field in pipeline) { - rtnval[field] = this._parseAggregateGroupArgs(schema, pipeline[field]); + returnValue[field] = this._parseAggregateGroupArgs(schema, pipeline[field]); } - return rtnval; - } - else if (typeof pipeline === 'string') { + return returnValue; + } else if (typeof pipeline === 'string') { const field = pipeline.substring(1); if (schema.fields[field] && schema.fields[field].type === 'Pointer') { return `$_p_${field}`; - } - if (field == 'createdAt') { + } else if (field == 'createdAt') { return '$_created_at'; } else if (field == 'updatedAt') { return '$_updated_at'; @@ -732,11 +716,11 @@ export class MongoStorageAdapter implements StorageAdapter { return new Date(value); } - const rtnval = {} + const returnValue = {} for (const field in value) { - rtnval[field] = this._convertToDate(value[field]) + returnValue[field] = this._convertToDate(value[field]) } - return rtnval; + return returnValue; } _parseReadPreference(readPreference: ?string): ?string {