Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Huge performance improvement on roles queries #1383

Merged
merged 1 commit into from
Apr 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 25 additions & 25 deletions spec/ParseRole.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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();
});
Expand Down Expand Up @@ -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
});
})
});
Expand All @@ -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();
})

Expand All @@ -259,7 +257,6 @@ describe('Parse Role testing', () => {
done();
});
}, (e) => {
console.log(e);
fail('should not have errored');
});
});
Expand Down Expand Up @@ -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();
})
});

});

113 changes: 51 additions & 62 deletions src/Auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down