From 2f7bf9c8c557fab7ede2388fd37352fe5762fe72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julien=20Qu=C3=A9r=C3=A9?= Date: Thu, 25 Apr 2019 03:28:13 +0200 Subject: [PATCH] Fix issue on count with Geo constraints and mongo (issue #5285) (#5286) * Add a tests that fails due to issue #5285 * Make test code much simpler * Fix #5285 by rewriting query (replacing $nearSphere by $geoWithin) All credit goes to @dplewis ! * move logic to transform --- spec/ParseGeoPoint.spec.js | 50 +++++++++++++++++-- .../Storage/Mongo/MongoStorageAdapter.js | 2 +- src/Adapters/Storage/Mongo/MongoTransform.js | 37 +++++++++----- 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/spec/ParseGeoPoint.spec.js b/spec/ParseGeoPoint.spec.js index 917057755a..385609c13f 100644 --- a/spec/ParseGeoPoint.spec.js +++ b/spec/ParseGeoPoint.spec.js @@ -11,9 +11,8 @@ describe('Parse.GeoPoint testing', () => { obj.set('location', point); obj.set('name', 'Ferndale'); await obj.save(); - const results = await new Parse.Query(TestObject).find(); - equal(results.length, 1); - const pointAgain = results[0].get('location'); + const result = await new Parse.Query(TestObject).get(obj.id); + const pointAgain = result.get('location'); ok(pointAgain); equal(pointAgain.latitude, 44.0); equal(pointAgain.longitude, -11.0); @@ -727,4 +726,49 @@ describe('Parse.GeoPoint testing', () => { done(); }); }); + + it('withinKilometers supports count', async () => { + const inside = new Parse.GeoPoint(10, 10); + const outside = new Parse.GeoPoint(20, 20); + + const obj1 = new Parse.Object('TestObject', { location: inside }); + const obj2 = new Parse.Object('TestObject', { location: outside }); + + await Parse.Object.saveAll([obj1, obj2]); + + const q = new Parse.Query(TestObject).withinKilometers( + 'location', + inside, + 5 + ); + const count = await q.count(); + + equal(count, 1); + }); + + it('withinKilometers complex supports count', async () => { + const inside = new Parse.GeoPoint(10, 10); + const middle = new Parse.GeoPoint(20, 20); + const outside = new Parse.GeoPoint(30, 30); + const obj1 = new Parse.Object('TestObject', { location: inside }); + const obj2 = new Parse.Object('TestObject', { location: middle }); + const obj3 = new Parse.Object('TestObject', { location: outside }); + + await Parse.Object.saveAll([obj1, obj2, obj3]); + + const q1 = new Parse.Query(TestObject).withinKilometers( + 'location', + inside, + 5 + ); + const q2 = new Parse.Query(TestObject).withinKilometers( + 'location', + middle, + 5 + ); + const query = Parse.Query.or(q1, q2); + const count = await query.count(); + + equal(count, 2); + }); }); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index 0e4c3fe810..3168684673 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -690,7 +690,7 @@ export class MongoStorageAdapter implements StorageAdapter { readPreference = this._parseReadPreference(readPreference); return this._adaptiveCollection(className) .then(collection => - collection.count(transformWhere(className, query, schema), { + collection.count(transformWhere(className, query, schema, true), { maxTimeMS: this._maxTimeMS, readPreference, }) diff --git a/src/Adapters/Storage/Mongo/MongoTransform.js b/src/Adapters/Storage/Mongo/MongoTransform.js index 2ffefebd61..d4f9657f7f 100644 --- a/src/Adapters/Storage/Mongo/MongoTransform.js +++ b/src/Adapters/Storage/Mongo/MongoTransform.js @@ -224,7 +224,7 @@ const valueAsDate = value => { return false; }; -function transformQueryKeyValue(className, key, value, schema) { +function transformQueryKeyValue(className, key, value, schema, count = false) { switch (key) { case 'createdAt': if (valueAsDate(value)) { @@ -293,7 +293,7 @@ function transformQueryKeyValue(className, key, value, schema) { return { key: key, value: value.map(subQuery => - transformWhere(className, subQuery, schema) + transformWhere(className, subQuery, schema, count) ), }; case 'lastUsed': @@ -330,7 +330,7 @@ function transformQueryKeyValue(className, key, value, schema) { } // Handle query constraints - const transformedConstraint = transformConstraint(value, field); + const transformedConstraint = transformConstraint(value, field, count); if (transformedConstraint !== CannotTransform) { if (transformedConstraint.$text) { return { key: '$text', value: transformedConstraint.$text }; @@ -359,14 +359,15 @@ function transformQueryKeyValue(className, key, value, schema) { // Main exposed method to help run queries. // restWhere is the "where" clause in REST API form. // Returns the mongo form of the query. -function transformWhere(className, restWhere, schema) { +function transformWhere(className, restWhere, schema, count = false) { const mongoWhere = {}; for (const restKey in restWhere) { const out = transformQueryKeyValue( className, restKey, restWhere[restKey], - schema + schema, + count ); mongoWhere[out.key] = out.value; } @@ -816,7 +817,7 @@ function relativeTimeToDate(text, now = new Date()) { // If it is not a valid constraint but it could be a valid something // else, return CannotTransform. // inArray is whether this is an array field. -function transformConstraint(constraint, field) { +function transformConstraint(constraint, field, count = false) { const inArray = field && field.type && field.type === 'Array'; if (typeof constraint !== 'object' || !constraint) { return CannotTransform; @@ -1002,15 +1003,27 @@ function transformConstraint(constraint, field) { } break; } - case '$nearSphere': - var point = constraint[key]; - answer[key] = [point.longitude, point.latitude]; + case '$nearSphere': { + const point = constraint[key]; + if (count) { + answer.$geoWithin = { + $centerSphere: [ + [point.longitude, point.latitude], + constraint.$maxDistance, + ], + }; + } else { + answer[key] = [point.longitude, point.latitude]; + } break; - - case '$maxDistance': + } + case '$maxDistance': { + if (count) { + break; + } answer[key] = constraint[key]; break; - + } // The SDKs don't seem to use these but they are documented in the // REST API docs. case '$maxDistanceInRadians':