node-casbin icon indicating copy to clipboard operation
node-casbin copied to clipboard

coreEnforcer.buildRoleLinksInternal calls model.buildRoleLinks(rmMap) on values of rmMap

Open richard-sim opened this issue 4 years ago • 5 comments

https://github.com/casbin/node-casbin/blob/f859d105b31668a93f965f1e2d6c3c5cb7f22f3d/src/coreEnforcer.ts#L367

Due to a refactor ~6 months ago, model.buildRoleLinks(rmMap) is called within a loop over rmMap.values() in coreEnforcer.buildRoleLinksInternal(). Not only is this inefficient, but if rmMap is empty then model.buildRoleLinks() will never be called.

richard-sim avatar Nov 03 '21 16:11 richard-sim

@Gabriel-403 @Zxilly @kingiw @nodece

casbin-bot avatar Nov 03 '21 16:11 casbin-bot

@richard-sim do you have any suggestions on how it can be improved?

Shivansh-yadav13 avatar Mar 06 '22 16:03 Shivansh-yadav13

@fabian4 @nodece

hsluoyz avatar Mar 06 '22 16:03 hsluoyz

@Shivansh-yadav13: It's been a while since I opened this issue, but IIRC it should not be in the loop at all (as it was prior to the refactor that I referred to).

richard-sim avatar Mar 07 '22 04:03 richard-sim

Not only is this inefficient, but if rmMap is empty then model.buildRoleLinks() will never be called.

but we will have Default Role Manager? if we use role definition in the model

Shivansh-yadav13 avatar Mar 08 '22 10:03 Shivansh-yadav13