Huge performance improvement on roles queries

Author: flovilmart

spec/ParseRole.spec.js

@@ -114,7 +114,7 @@ describe('Parse Role testing', () => {
       return Parse.Object.saveAll(roles, { useMasterKey: true });
     }).then( () => {
       auth = new Auth({config: new Config("test"), isMaster: true, user: user});
-      getAllRolesSpy = spyOn(auth, "_getAllRoleNamesForId").and.callThrough();
+      getAllRolesSpy = spyOn(auth, "_getAllRolesNamesForRoleIds").and.callThrough();
 
       return auth._loadRoles();
     }).then ( (roles) => {
@@ -125,15 +125,14 @@ describe('Parse Role testing', () => {
       });
 
       // 1 Query for the initial setup
-      // 4 Queries for all the specific roles
-      // 1 Query for the final $in
-      expect(restExecute.calls.count()).toEqual(6);
+      // 1 query for the parent roles
+      expect(restExecute.calls.count()).toEqual(2);
 
-      // 4 One query for each of the roles
-      // 3 Queries for the "RootRole"
-      expect(getAllRolesSpy.calls.count()).toEqual(7);
+      // 1 call for the 1st layer of roles
+      // 1 call for the 2nd layer
+      expect(getAllRolesSpy.calls.count()).toEqual(2);
       done()
-    }).catch( () =>  {
+    }).catch( (err) =>  {
       fail("should succeed");
       done();
     });
@@ -206,11 +205,11 @@ describe('Parse Role testing', () => {
       // For each role, fetch their sibling, what they inherit
       // return with result and roleId for later comparison
       let promises = [admin, moderator, contentManager, superModerator].map((role) => {
-        return auth._getAllRoleNamesForId(role.id).then((result) => {
+        return auth._getAllRolesNamesForRoleIds([role.id]).then((result) => {
           return Parse.Promise.as({
             id: role.id,
             name: role.get('name'),
-            roleIds: result
+            roleNames: result
           });
         })
       });
@@ -219,26 +218,25 @@ describe('Parse Role testing', () => {
     }).then((results) => {
       results.forEach((result) => {
         let id = result.id;
-        let roleIds = result.roleIds;
+        let roleNames = result.roleNames;
         if (id == admin.id) {
-          expect(roleIds.length).toBe(2);
-          expect(roleIds.indexOf(moderator.id)).not.toBe(-1);
-          expect(roleIds.indexOf(contentManager.id)).not.toBe(-1);
+          expect(roleNames.length).toBe(2);
+          expect(roleNames.indexOf("Moderator")).not.toBe(-1);
+          expect(roleNames.indexOf("ContentManager")).not.toBe(-1);
         } else if (id == moderator.id) {
-          expect(roleIds.length).toBe(1);
-          expect(roleIds.indexOf(contentManager.id)).toBe(0);
+          expect(roleNames.length).toBe(1);
+          expect(roleNames.indexOf("ContentManager")).toBe(0);
         } else if (id == contentManager.id) {
-          expect(roleIds.length).toBe(0);
+          expect(roleNames.length).toBe(0);
         } else if (id == superModerator.id) {
-          expect(roleIds.length).toBe(3);
-          expect(roleIds.indexOf(moderator.id)).not.toBe(-1);
-          expect(roleIds.indexOf(contentManager.id)).not.toBe(-1);
-          expect(roleIds.indexOf(superContentManager.id)).not.toBe(-1);
+          expect(roleNames.length).toBe(3);
+          expect(roleNames.indexOf("Moderator")).not.toBe(-1);
+          expect(roleNames.indexOf("ContentManager")).not.toBe(-1);
+          expect(roleNames.indexOf("SuperContentManager")).not.toBe(-1);
         }
       });
       done();
     }).fail((err) => {
-      console.error(err);
       done();
     })
 
@@ -259,7 +257,6 @@ describe('Parse Role testing', () => {
             done();
           });
       }, (e) => {
-        console.log(e);
         fail('should not have errored');
       });
   });
@@ -338,10 +335,13 @@ describe('Parse Role testing', () => {
       fail('Customer user should not have been able to save.');
       done();
     }, (e) => {
-      expect(e.code).toEqual(101);
+      if (e) {
+        expect(e.code).toEqual(101);
+      } else {
+        fail('should return an error');
+      }
       done();
     })
   });
 
 });
-

src/Auth.js

@@ -114,30 +114,17 @@ Auth.prototype._loadRoles = function() {
       this.rolePromise = null;
       return Promise.resolve(this.userRoles);
     }
+    var rolesMap =  results.reduce((m, r) => {
+      m.names.push(r.name);
+      m.ids.push(r.objectId);
+      return m;
+    }, {ids: [], names: []});
 
-    var roleIDs = results.map(r => r.objectId);
-    var promises = [Promise.resolve(roleIDs)];
-    var queriedRoles = {};
-    for (var role of roleIDs) {
-      promises.push(this._getAllRoleNamesForId(role, queriedRoles));
-    }
-    return Promise.all(promises).then((results) => {
-      var allIDs = [];
-      for (var x of results) {
-        Array.prototype.push.apply(allIDs, x);
-      }
-      var restWhere = {
-        objectId: {
-          '$in': allIDs
-        }
-      };
-      var query = new RestQuery(this.config, master(this.config),
-                                '_Role', restWhere, {});
-      return query.execute();
-    }).then((response) => {
-      var results = response.results;
-      this.userRoles = results.map((r) => {
-        return 'role:' + r.name;
+    // run the recursive finding
+    return this._getAllRolesNamesForRoleIds(rolesMap.ids, rolesMap.names)
+    .then((roleNames) => {
+      this.userRoles = roleNames.map((r) => {
+        return 'role:' + r;
       });
       this.fetchedRoles = true;
       this.rolePromise = null;
@@ -146,50 +133,52 @@ Auth.prototype._loadRoles = function() {
   });
 };
 
-// Given a role object id, get any other roles it is part of
-Auth.prototype._getAllRoleNamesForId = function(roleID, queriedRoles = {}) {
-  // Don't need to requery this role as it is already being queried for.
-  if (queriedRoles[roleID] != null) {
-      return Promise.resolve([]);
+// Given a list of roleIds, find all the parent roles, returns a promise with all names
+Auth.prototype._getAllRolesNamesForRoleIds = function(roleIDs, names = [], queriedRoles = {}) {
+  let ins = roleIDs.filter((roleID) => {
+    return queriedRoles[roleID] !== true;
+  }).map((roleID) => {
+    // mark as queried
+    queriedRoles[roleID] = true;
+    return {
+      __type: 'Pointer',
+      className: '_Role',
+      objectId: roleID
+    }
+  });
+
+  // all roles are accounted for, return the names
+  if (ins.length == 0) {
+    return Promise.resolve([...new Set(names)]);
   }
-  queriedRoles[roleID] = true;
-  // As per documentation, a Role inherits AnotherRole
-  // if this Role is in the roles pointer of this AnotherRole
-  // Let's find all the roles where this role is in a roles relation
-  var rolePointer = {
-    __type: 'Pointer',
-    className: '_Role',
-    objectId: roleID
-  };
-  var restWhere = {
-    'roles': rolePointer
-  };
-  var query = new RestQuery(this.config, master(this.config), '_Role',
-                            restWhere, {});
+  // Build an OR query across all parentRoles
+  let restWhere;
+  if (ins.length == 1) {
+    restWhere = { 'roles': ins[0] };
+  } else {
+    restWhere = { 'roles': { '$in': ins }}
+  }
+  let query = new RestQuery(this.config, master(this.config), '_Role', restWhere, {});
   return query.execute().then((response) => {
     var results = response.results;
+    // Nothing found
     if (!results.length) {
-      return Promise.resolve([]);
+      return Promise.resolve(names);
     }
-    var roleIDs = results.map(r => r.objectId);
-
-    // we found a list of roles where the roleID
-    // is referenced in the roles relation,
-    // Get the roles where those found roles are also
-    // referenced the same way
-    var parentRolesPromises = roleIDs.map( (roleId) => {
-      return this._getAllRoleNamesForId(roleId, queriedRoles);
-    });
-    parentRolesPromises.push(Promise.resolve(roleIDs));
-    return Promise.all(parentRolesPromises);
-  }).then(function(results){
-    // Flatten
-    let roleIDs = results.reduce( (memo, result) => {
-      return memo.concat(result);
-    }, []);
-    return Promise.resolve([...new Set(roleIDs)]);
-  });
-};
+    // Map the results with all Ids and names
+    let resultMap = results.reduce((memo, role) => {
+      memo.names.push(role.name);
+      memo.ids.push(role.objectId);
+      return memo;
+    }, {ids: [], names: []});
+    // store the new found names
+    names = names.concat(resultMap.names);
+    // find the next ones, circular roles will be cut
+    return this._getAllRolesNamesForRoleIds(resultMap.ids, names, queriedRoles)
+  }).then((names) => {
+    return Promise.resolve([...new Set(names)])
+  })
+}
 
 module.exports = {
   Auth: Auth,