Optimizing pointer CLP query decoration done by DatabaseController#addPointerPermissions

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### master
 [Full Changelog](https://github.com/parse-community/parse-server/compare/4.2.0...master)
+- FIX: Optimize query decoration based on pointer CLPs by looking at the class schema to determine the field's type.
 
 ### 4.2.0
 [Full Changelog](https://github.com/parse-community/parse-server/compare/4.1.0...4.2.0)

spec/DatabaseController.spec.js

@@ -1,9 +1,9 @@
 const DatabaseController = require('../lib/Controllers/DatabaseController.js');
 const validateQuery = DatabaseController._validateQuery;
 
-describe('DatabaseController', function() {
-  describe('validateQuery', function() {
-    it('should not restructure simple cases of SERVER-13732', done => {
+describe('DatabaseController', function () {
+  describe('validateQuery', function () {
+    it('should not restructure simple cases of SERVER-13732', (done) => {
       const query = {
         $or: [{ a: 1 }, { a: 2 }],
         _rperm: { $in: ['a', 'b'] },
@@ -18,7 +18,7 @@ describe('DatabaseController', function() {
       done();
     });
 
-    it('should not restructure SERVER-13732 queries with $nears', done => {
+    it('should not restructure SERVER-13732 queries with $nears', (done) => {
       let query = { $or: [{ a: 1 }, { b: 1 }], c: { $nearSphere: {} } };
       validateQuery(query);
       expect(query).toEqual({
@@ -31,7 +31,7 @@ describe('DatabaseController', function() {
       done();
     });
 
-    it('should not push refactored keys down a tree for SERVER-13732', done => {
+    it('should not push refactored keys down a tree for SERVER-13732', (done) => {
       const query = {
         a: 1,
         $or: [{ $or: [{ b: 1 }, { b: 2 }] }, { $or: [{ c: 1 }, { c: 2 }] }],
@@ -45,14 +45,274 @@ describe('DatabaseController', function() {
       done();
     });
 
-    it('should reject invalid queries', done => {
+    it('should reject invalid queries', (done) => {
       expect(() => validateQuery({ $or: { a: 1 } })).toThrow();
       done();
     });
 
-    it('should accept valid queries', done => {
+    it('should accept valid queries', (done) => {
       expect(() => validateQuery({ $or: [{ a: 1 }, { b: 2 }] })).not.toThrow();
       done();
     });
   });
+
+  describe('addPointerPermissions', function () {
+    const CLASS_NAME = 'Foo';
+    const USER_ID = 'userId';
+    const ACL_GROUP = [USER_ID];
+    const OPERATION = 'find';
+
+    const databaseController = new DatabaseController();
+    const schemaController = jasmine.createSpyObj('SchemaController', [
+      'testPermissionsForClassName',
+      'getClassLevelPermissions',
+      'getExpectedType',
+    ]);
+
+    it('should not decorate query if no pointer CLPs are present', (done) => {
+      const clp = buildCLP();
+      const query = { a: 'b' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(true);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+
+      const output = databaseController.addPointerPermissions(
+        schemaController,
+        CLASS_NAME,
+        OPERATION,
+        query,
+        ACL_GROUP
+      );
+
+      expect(output).toEqual({ ...query });
+
+      done();
+    });
+
+    it('should decorate query if a pointer CLP entry is present', (done) => {
+      const clp = buildCLP(['user']);
+      const query = { a: 'b' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(false);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'user')
+        .and.returnValue({ type: 'Pointer' });
+
+      const output = databaseController.addPointerPermissions(
+        schemaController,
+        CLASS_NAME,
+        OPERATION,
+        query,
+        ACL_GROUP
+      );
+
+      expect(output).toEqual({ ...query, user: createUserPointer(USER_ID) });
+
+      done();
+    });
+
+    it('should decorate query if an array CLP entry is present', (done) => {
+      const clp = buildCLP(['users']);
+      const query = { a: 'b' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(false);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'users')
+        .and.returnValue({ type: 'Array' });
+
+      const output = databaseController.addPointerPermissions(
+        schemaController,
+        CLASS_NAME,
+        OPERATION,
+        query,
+        ACL_GROUP
+      );
+
+      expect(output).toEqual({
+        ...query,
+        users: { $all: [createUserPointer(USER_ID)] },
+      });
+
+      done();
+    });
+
+    it('should decorate query if an object CLP entry is present', (done) => {
+      const clp = buildCLP(['user']);
+      const query = { a: 'b' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(false);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'user')
+        .and.returnValue({ type: 'Object' });
+
+      const output = databaseController.addPointerPermissions(
+        schemaController,
+        CLASS_NAME,
+        OPERATION,
+        query,
+        ACL_GROUP
+      );
+
+      expect(output).toEqual({
+        ...query,
+        user: createUserPointer(USER_ID),
+      });
+
+      done();
+    });
+
+    it('should decorate query if a pointer CLP is present and the same field is part of the query', (done) => {
+      const clp = buildCLP(['user']);
+      const query = { a: 'b', user: 'a' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(false);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'user')
+        .and.returnValue({ type: 'Pointer' });
+
+      const output = databaseController.addPointerPermissions(
+        schemaController,
+        CLASS_NAME,
+        OPERATION,
+        query,
+        ACL_GROUP
+      );
+
+      expect(output).toEqual({
+        $and: [{ user: createUserPointer(USER_ID) }, { ...query }],
+      });
+
+      done();
+    });
+
+    it('should transform the query to an $or query if multiple array/pointer CLPs are present', (done) => {
+      const clp = buildCLP(['user', 'users', 'userObject']);
+      const query = { a: 'b' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(false);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'user')
+        .and.returnValue({ type: 'Pointer' });
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'users')
+        .and.returnValue({ type: 'Array' });
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'userObject')
+        .and.returnValue({ type: 'Object' });
+
+      const output = databaseController.addPointerPermissions(
+        schemaController,
+        CLASS_NAME,
+        OPERATION,
+        query,
+        ACL_GROUP
+      );
+
+      expect(output).toEqual({
+        $or: [
+          { ...query, user: createUserPointer(USER_ID) },
+          { ...query, users: { $all: [createUserPointer(USER_ID)] } },
+          { ...query, userObject: createUserPointer(USER_ID) },
+        ],
+      });
+
+      done();
+    });
+
+    it('should throw an error if for some unexpected reason the property specified in the CLP is neither a pointer nor an array', (done) => {
+      const clp = buildCLP(['user']);
+      const query = { a: 'b' };
+
+      schemaController.testPermissionsForClassName
+        .withArgs(CLASS_NAME, ACL_GROUP, OPERATION)
+        .and.returnValue(false);
+      schemaController.getClassLevelPermissions
+        .withArgs(CLASS_NAME)
+        .and.returnValue(clp);
+      schemaController.getExpectedType
+        .withArgs(CLASS_NAME, 'user')
+        .and.returnValue({ type: 'Number' });
+
+      expect(() => {
+        databaseController.addPointerPermissions(
+          schemaController,
+          CLASS_NAME,
+          OPERATION,
+          query,
+          ACL_GROUP
+        );
+      }).toThrow(
+        Error(
+          `An unexpected condition occurred when resolving pointer permissions: ${CLASS_NAME} user`
+        )
+      );
+
+      done();
+    });
+  });
 });
+
+function buildCLP(pointerNames) {
+  const OPERATIONS = [
+    'count',
+    'find',
+    'get',
+    'create',
+    'update',
+    'delete',
+    'addField',
+  ];
+
+  const clp = OPERATIONS.reduce((acc, op) => {
+    acc[op] = {};
+
+    if (pointerNames && pointerNames.length) {
+      acc[op].pointerFields = pointerNames;
+    }
+
+    return acc;
+  }, {});
+
+  clp.protectedFields = {};
+  clp.writeUserFields = [];
+  clp.readUserFields = [];
+
+  return clp;
+}
+
+function createUserPointer(userId) {
+  return {
+    __type: 'Pointer',
+    className: '_User',
+    objectId: userId,
+  };
+}

src/Controllers/DatabaseController.js

@@ -1570,23 +1570,42 @@ class DatabaseController {
         objectId: userId,
       };
 
-      const ors = permFields.flatMap((key) => {
-        // constraint for single pointer setup
-        const q = {
-          [key]: userPointer,
-        };
-        // constraint for users-array setup
-        const qa = {
-          [key]: { $all: [userPointer] },
-        };
+      const queries = permFields.map((key) => {
+        const fieldDescriptor = schema.getExpectedType(className, key);
+        const fieldType =
+          fieldDescriptor &&
+          typeof fieldDescriptor === 'object' &&
+          Object.prototype.hasOwnProperty.call(fieldDescriptor, 'type')
+            ? fieldDescriptor.type
+            : null;
+
+        let queryClause;
+
+        if (fieldType === 'Pointer') {
+          // constraint for single pointer setup
+          queryClause = { [key]: userPointer };
+        } else if (fieldType === 'Array') {
+          // constraint for users-array setup
+          queryClause = { [key]: { $all: [userPointer] } };
+        } else if (fieldType === 'Object') {
+          // constraint for object setup
+          queryClause = { [key]: userPointer };
+        } else {
+          // This means that there is a CLP field of an unexpected type. This condition should not happen, which is
+          // why is being treated as an error.
+          throw Error(
+            `An unexpected condition occurred when resolving pointer permissions: ${className} ${key}`
+          );
+        }
         // if we already have a constraint on the key, use the $and
         if (Object.prototype.hasOwnProperty.call(query, key)) {
-          return [{ $and: [q, query] }, { $and: [qa, query] }];
+          return { $and: [queryClause, query] };
         }
         // otherwise just add the constaint
-        return [Object.assign({}, query, q), Object.assign({}, query, qa)];
+        return Object.assign({}, query, queryClause);
       });
-      return { $or: ors };
+
+      return queries.length === 1 ? queries[0] : { $or: queries };
     } else {
       return query;
     }