From 2672939ce7e8a834b64b56769532a596cd838ade Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Sun, 12 Aug 2018 12:41:40 -0500 Subject: [PATCH 1/3] Add pipeline key to Aggregate --- spec/ParseQuery.Aggregate.spec.js | 21 +++++++++++++++++++++ src/Routers/AggregateRouter.js | 11 ++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index f325e03a43..d934a2d402 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -100,6 +100,27 @@ describe('Parse.Query Aggregate testing', () => { }).catch(done.fail); }); + it('group by field with pipeline key', (done) => { + const options = Object.assign({}, masterKeyOptions, { + body: { + pipeline: { + group: { objectId: '$name' }, + } + } + }); + rp.get(Parse.serverURL + '/aggregate/TestObject', options) + .then((resp) => { + expect(resp.results.length).toBe(3); + expect(resp.results[0].hasOwnProperty('objectId')).toBe(true); + expect(resp.results[1].hasOwnProperty('objectId')).toBe(true); + expect(resp.results[2].hasOwnProperty('objectId')).toBe(true); + expect(resp.results[0].objectId).not.toBe(undefined); + expect(resp.results[1].objectId).not.toBe(undefined); + expect(resp.results[2].objectId).not.toBe(undefined); + done(); + }).catch(done.fail); + }); + it('group by empty object', (done) => { const obj = new TestObject(); const pipeline = [{ diff --git a/src/Routers/AggregateRouter.js b/src/Routers/AggregateRouter.js index c99436f380..f51a0483a0 100644 --- a/src/Routers/AggregateRouter.js +++ b/src/Routers/AggregateRouter.js @@ -4,7 +4,7 @@ import * as middleware from '../middlewares'; import Parse from 'parse/node'; import UsersRouter from './UsersRouter'; -const BASE_KEYS = ['where', 'distinct']; +const BASE_KEYS = ['where', 'distinct', 'pipeline']; const PIPELINE_KEYS = [ 'addFields', @@ -41,17 +41,18 @@ export class AggregateRouter extends ClassesRouter { handleFind(req) { const body = Object.assign(req.body, ClassesRouter.JSONFromQuery(req.query)); const options = {}; + const data = body.pipeline || body; let pipeline = []; - if (Array.isArray(body)) { - pipeline = body.map((stage) => { + if (Array.isArray(data)) { + pipeline = data.map((stage) => { const stageName = Object.keys(stage)[0]; return this.transformStage(stageName, stage); }); } else { const stages = []; - for (const stageName in body) { - stages.push(this.transformStage(stageName, body)); + for (const stageName in data) { + stages.push(this.transformStage(stageName, data)); } pipeline = stages; } From 30dd020f4c3767a2792699ab5861201c67b8f714 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Sun, 12 Aug 2018 17:10:04 -0500 Subject: [PATCH 2/3] clean up --- spec/ParseQuery.Aggregate.spec.js | 21 +++++------ src/Routers/AggregateRouter.js | 60 ++++++++++++++++++++++--------- 2 files changed, 52 insertions(+), 29 deletions(-) diff --git a/spec/ParseQuery.Aggregate.spec.js b/spec/ParseQuery.Aggregate.spec.js index d934a2d402..377d4d96a2 100644 --- a/spec/ParseQuery.Aggregate.spec.js +++ b/spec/ParseQuery.Aggregate.spec.js @@ -100,7 +100,7 @@ describe('Parse.Query Aggregate testing', () => { }).catch(done.fail); }); - it('group by field with pipeline key', (done) => { + it('group by pipeline operator', async () => { const options = Object.assign({}, masterKeyOptions, { body: { pipeline: { @@ -108,17 +108,14 @@ describe('Parse.Query Aggregate testing', () => { } } }); - rp.get(Parse.serverURL + '/aggregate/TestObject', options) - .then((resp) => { - expect(resp.results.length).toBe(3); - expect(resp.results[0].hasOwnProperty('objectId')).toBe(true); - expect(resp.results[1].hasOwnProperty('objectId')).toBe(true); - expect(resp.results[2].hasOwnProperty('objectId')).toBe(true); - expect(resp.results[0].objectId).not.toBe(undefined); - expect(resp.results[1].objectId).not.toBe(undefined); - expect(resp.results[2].objectId).not.toBe(undefined); - done(); - }).catch(done.fail); + const resp = await rp.get(Parse.serverURL + '/aggregate/TestObject', options); + expect(resp.results.length).toBe(3); + expect(resp.results[0].hasOwnProperty('objectId')).toBe(true); + expect(resp.results[1].hasOwnProperty('objectId')).toBe(true); + expect(resp.results[2].hasOwnProperty('objectId')).toBe(true); + expect(resp.results[0].objectId).not.toBe(undefined); + expect(resp.results[1].objectId).not.toBe(undefined); + expect(resp.results[2].objectId).not.toBe(undefined); }); it('group by empty object', (done) => { diff --git a/src/Routers/AggregateRouter.js b/src/Routers/AggregateRouter.js index f51a0483a0..aa575fe232 100644 --- a/src/Routers/AggregateRouter.js +++ b/src/Routers/AggregateRouter.js @@ -41,25 +41,10 @@ export class AggregateRouter extends ClassesRouter { handleFind(req) { const body = Object.assign(req.body, ClassesRouter.JSONFromQuery(req.query)); const options = {}; - const data = body.pipeline || body; - let pipeline = []; - - if (Array.isArray(data)) { - pipeline = data.map((stage) => { - const stageName = Object.keys(stage)[0]; - return this.transformStage(stageName, stage); - }); - } else { - const stages = []; - for (const stageName in data) { - stages.push(this.transformStage(stageName, data)); - } - pipeline = stages; - } if (body.distinct) { options.distinct = String(body.distinct); } - options.pipeline = pipeline; + options.pipeline = AggregateRouter.getPipeline(body); if (typeof body.where === 'string') { body.where = JSON.parse(body.where); } @@ -73,7 +58,48 @@ export class AggregateRouter extends ClassesRouter { }); } - transformStage(stageName, stage) { + /* Builds a pipeline from the body. Originally the body could be passed as a single object, + * and now we support many options + * + * Array + * + * body: [{ + * group: { objectId: '$name' }, + * }] + * + * Object + * + * body: { + * group: { objectId: '$name' }, + * } + * + * + * Pipeline Operator with an Array or an Object + * + * body: { + * pipeline: { + * group: { objectId: '$name' }, + * } + * } + * + */ + static getPipeline(body) { + let pipeline = body.pipeline || body; + + if (!Array.isArray(pipeline)) { + pipeline = Object.keys(pipeline).map((key) => { return { [key]: pipeline[key] } }); + } + + return pipeline.map((stage) => { + const keys = Object.keys(stage); + if (keys.length != 1) { + throw new Error(`Pipeline stages should only have one key found ${keys.join(' ')}`); + } + return AggregateRouter.transformStage(keys[0], stage); + }); + } + + static transformStage(stageName, stage) { if (ALLOWED_KEYS.indexOf(stageName) === -1) { throw new Parse.Error( Parse.Error.INVALID_QUERY, From 417b43e3eb0b7a7c7f3aba68b938d3681d3df669 Mon Sep 17 00:00:00 2001 From: Diamond Lewis Date: Sun, 12 Aug 2018 18:30:29 -0500 Subject: [PATCH 3/3] unit tests --- CHANGELOG.md | 5 ++- spec/AggregateRouter.spec.js | 69 ++++++++++++++++++++++++++++++++++ src/Routers/AggregateRouter.js | 2 +- 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 spec/AggregateRouter.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index fa8d1061b9..51b8dd144d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,13 @@ ### master [Full Changelog](https://github.com/parse-community/parse-server/compare/2.8.4...master) +#### Improvements: +* Adds Pipeline Operator to Aggregate Router + ### 2.8.4 [Full Changelog](https://github.com/parse-community/parse-server/compare/2.8.3...2.8.4) -#### Improvements +#### Improvements: * Adds ability to forward errors to express handler (#4697) * Adds ability to increment the push badge with an arbitrary value (#4889) * Adds ability to preserve the file names when uploading (#4915) diff --git a/spec/AggregateRouter.spec.js b/spec/AggregateRouter.spec.js new file mode 100644 index 0000000000..4e751d85a4 --- /dev/null +++ b/spec/AggregateRouter.spec.js @@ -0,0 +1,69 @@ +const AggregateRouter = require('../lib/Routers/AggregateRouter').AggregateRouter; + +describe('AggregateRouter', () => { + it('get pipeline from Array', () => { + const body = [{ + group: { objectId: {} } + }]; + const expected = [ { $group: { _id: {} } } ]; + const result = AggregateRouter.getPipeline(body); + expect(result).toEqual(expected); + }); + + it('get pipeline from Object', () => { + const body = { + group: { objectId: {} } + }; + const expected = [ { $group: { _id: {} } } ]; + const result = AggregateRouter.getPipeline(body); + expect(result).toEqual(expected); + }); + + it('get pipeline from Pipeline Operator (Array)', () => { + const body = { + pipeline: [{ + group: { objectId: {} } + }] + }; + const expected = [ { $group: { _id: {} } } ]; + const result = AggregateRouter.getPipeline(body); + expect(result).toEqual(expected); + }); + + it('get pipeline from Pipeline Operator (Object)', () => { + const body = { + pipeline: { + group: { objectId: {} } + } + }; + const expected = [ { $group: { _id: {} } } ]; + const result = AggregateRouter.getPipeline(body); + expect(result).toEqual(expected); + }); + + it('get pipeline fails multiple keys in Array stage ', () => { + const body = [{ + group: { objectId: {} }, + match: { name: 'Test' }, + }]; + try { + AggregateRouter.getPipeline(body); + } catch (e) { + expect(e.message).toBe('Pipeline stages should only have one key found group, match'); + } + }); + + it('get pipeline fails multiple keys in Pipeline Operator Array stage ', () => { + const body = { + pipeline: [{ + group: { objectId: {} }, + match: { name: 'Test' }, + }] + }; + try { + AggregateRouter.getPipeline(body); + } catch (e) { + expect(e.message).toBe('Pipeline stages should only have one key found group, match'); + } + }); +}); diff --git a/src/Routers/AggregateRouter.js b/src/Routers/AggregateRouter.js index aa575fe232..b72640048a 100644 --- a/src/Routers/AggregateRouter.js +++ b/src/Routers/AggregateRouter.js @@ -93,7 +93,7 @@ export class AggregateRouter extends ClassesRouter { return pipeline.map((stage) => { const keys = Object.keys(stage); if (keys.length != 1) { - throw new Error(`Pipeline stages should only have one key found ${keys.join(' ')}`); + throw new Error(`Pipeline stages should only have one key found ${keys.join(', ')}`); } return AggregateRouter.transformStage(keys[0], stage); });