From e06471603f8f1fd6be3bdce7cd6f90d273a28fbb Mon Sep 17 00:00:00 2001 From: Arthur Cinader <700572+acinader@users.noreply.github.com> Date: Thu, 7 Jun 2018 15:47:18 -0700 Subject: [PATCH] Don't error when attempting to sort on an object field (#4806) * add failing test to demonstrate that you can't sort on a field in an object. * Only validate the base of the field name. * fix test name * Only test sort for mongo. * pg order by nested object * level 2 test * Factor out operation to get a field's base name. Add comment. * tweak comment wording so it wont make my grammar teacher angry. --- spec/ParseQuery.spec.js | 84 +++++++++++++++++++ .../Postgres/PostgresStorageAdapter.js | 5 +- src/Controllers/DatabaseController.js | 17 +++- 3 files changed, 101 insertions(+), 5 deletions(-) diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 1a755d5a..8aee36a5 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -1538,6 +1538,90 @@ describe('Parse.Query testing', () => { }); }); + it('can order on an object string field', function (done) { + const testSet = [ + { sortField: { value: "Z" } }, + { sortField: { value: "A" } }, + { sortField: { value: "M" } }, + ]; + + const objects = testSet.map(e => new Parse.Object('Test', e)); + Parse.Object.saveAll(objects) + .then(() => new Parse.Query('Test').addDescending('sortField.value').first()) + .then((result) => { + expect(result.get('sortField').value).toBe("Z"); + return new Parse.Query('Test').addAscending('sortField.value').first() + }) + .then((result) => { + expect(result.get('sortField').value).toBe("A"); + done(); + }) + .catch(done.fail); + }); + + it('can order on an object string field (level 2)', function (done) { + const testSet = [ + { sortField: { value: { field: "Z" } } }, + { sortField: { value: { field: "A" } } }, + { sortField: { value: { field: "M" } } }, + ]; + + const objects = testSet.map(e => new Parse.Object('Test', e)); + Parse.Object.saveAll(objects) + .then(() => new Parse.Query('Test').addDescending('sortField.value.field').first()) + .then((result) => { + expect(result.get('sortField').value.field).toBe("Z"); + return new Parse.Query('Test').addAscending('sortField.value.field').first() + }) + .then((result) => { + expect(result.get('sortField').value.field).toBe("A"); + done(); + }) + .catch(done.fail); + }); + + it('can order on an object number field', function (done) { + const testSet = [ + { sortField: { value: 10 } }, + { sortField: { value: 1 } }, + { sortField: { value: 5 } }, + ]; + + const objects = testSet.map(e => new Parse.Object('Test', e)); + Parse.Object.saveAll(objects) + .then(() => new Parse.Query('Test').addDescending('sortField.value').first()) + .then((result) => { + expect(result.get('sortField').value).toBe(10); + return new Parse.Query('Test').addAscending('sortField.value').first() + }) + .then((result) => { + expect(result.get('sortField').value).toBe(1); + done(); + }) + .catch(done.fail); + }); + + it('can order on an object number field (level 2)', function (done) { + const testSet = [ + { sortField: { value: { field: 10 } } }, + { sortField: { value: { field: 1 } } }, + { sortField: { value: { field: 5 } } }, + ]; + + const objects = testSet.map(e => new Parse.Object('Test', e)); + Parse.Object.saveAll(objects) + .then(() => new Parse.Query('Test').addDescending('sortField.value.field').first()) + .then((result) => { + expect(result.get('sortField').value.field).toBe(10); + return new Parse.Query('Test').addAscending('sortField.value.field').first() + }) + .then((result) => { + expect(result.get('sortField').value.field).toBe(1); + done(); + }) + .catch(done.fail); + }); + it("order by ascending number then descending string", function(done) { const strings = ["a", "b", "c", "d"]; const makeBoxedNumber = function(num, i) { diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js index 0b9ed5ec..ce950f41 100644 --- a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js +++ b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js @@ -1397,11 +1397,12 @@ export class PostgresStorageAdapter implements StorageAdapter { if (sort) { const sortCopy: any = sort; const sorting = Object.keys(sort).map((key) => { + const transformKey = transformDotFieldToComponents(key).join('->'); // Using $idx pattern gives: non-integer constant in ORDER BY if (sortCopy[key] === 1) { - return `"${key}" ASC`; + return `${transformKey} ASC`; } - return `"${key}" DESC`; + return `${transformKey} DESC`; }).join(); sortPattern = sort !== undefined && Object.keys(sort).length > 0 ? `ORDER BY ${sorting}` : ''; } diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 91c12ed5..2272c64b 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -294,6 +294,16 @@ const untransformObjectACL = ({_rperm, _wperm, ...output}) => { return output; } +/** + * When querying, the fieldName may be compound, extract the base fieldName + * `temperature.celsius` becomes `temperature` + * @param {string} fieldName that may be a compound field name + * @returns {string} the basename of the field + */ +const getBaseFieldName = (fieldName: string): string => { + return fieldName.split('.')[0] +} + const relationSchema = { fields: { relatedId: { type: 'String' }, owningId: { type: 'String' } } }; class DatabaseController { @@ -411,8 +421,8 @@ class DatabaseController { if (fieldName.match(/^authData\.([a-zA-Z0-9_]+)\.id$/)) { throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid field name for update: ${fieldName}`); } - fieldName = fieldName.split('.')[0]; - if (!SchemaController.fieldNameIsValid(fieldName) && !isSpecialUpdateKey(fieldName)) { + const baseFieldName = getBaseFieldName(fieldName); + if (!SchemaController.fieldNameIsValid(baseFieldName) && !isSpecialUpdateKey(baseFieldName)) { throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid field name for update: ${fieldName}`); } }); @@ -900,7 +910,8 @@ class DatabaseController { if (fieldName.match(/^authData\.([a-zA-Z0-9_]+)\.id$/)) { throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Cannot sort by ${fieldName}`); } - if (!SchemaController.fieldNameIsValid(fieldName)) { + const baseFieldName = getBaseFieldName(fieldName); + if (!SchemaController.fieldNameIsValid(baseFieldName)) { throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid field name: ${fieldName}.`); } });