Meteor-Community-Packages / meteor-roles

Authorization package for Meteor, compatible with built-in accounts packages
http://meteor-community-packages.github.io/meteor-roles/
MIT License
921 stars 166 forks source link

Can we avoid using the function _assureConsistency? #269

Closed SimonSimCity closed 4 years ago

SimonSimCity commented 5 years ago

The function _assureConsistency() in code states that it's used internally after complicated changes. This mainly means that instead of covering all edge-cases, we basically throw all inherited roles away and build up the structure again.

This might work well for small role-bases, but creates a lot of database-requests in my case, where users have document-based roles (including many permissions - both called roles in the vocabulary of this package) and often up to 15 roles per document.

Over here it now takes minutes until a role is removed from a user because it calls _assureConsistency().

Let's not talk about that this package stores all the roles in the user-document, which also is something I have to attack at a certain point in time.

Wouldn't it make more sense to recreate the edge-cases, get them off the table? Maybe leave this method in for internal reset-function - but this shouldn't be called by other functionality e.g. removeRolesFromParent() or (more important to me) removeUsersFromRoles().

mitar commented 5 years ago

Let's not talk about that this package stores all the roles in the user-document, which also is something I have to attack at a certain point in time.

My take is that you should not be using this package for the document-level permissions, but only for collection-level permissions. This is how I am doing.

Wouldn't it make more sense to recreate the edge-cases, get them off the table?

I think it was not possible to do so without transactions. I think that now that MongoDB has transactions we might be able to change this approach.

SimonSimCity commented 4 years ago

The method has been dropped and performance has been gained in v3.