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

Make roles_for use a cache #453

Open
jace opened this issue Mar 19, 2024 · 1 comment
Open

Make roles_for use a cache #453

jace opened this issue Mar 19, 2024 · 1 comment

Comments

@jace
Copy link
Member

jace commented Mar 19, 2024

RoleMixin.roles_for is a method that returns a new instance of LazyRoleSet on each call. This has exposed an inefficiency with recursive calls in ConditionalRole introduced in #451.

RoleMixin.current_roles meanwhile writes a cache entry to flask.g, without support for cache invalidation, leaving that as a gotcha for the developer.

Both these can be solved by turning roles_for into a non-data descriptor that's stored on the instance and tracks the LazyRoleSet instance for every actor it's been called with. It could also support the Mapping protocol (obj.roles_for[actor]) to make the cache more apparent, although this excludes the (as yet unused) anchors parameter.

Since subclasses may override roles_for, (a) the descriptor may need a new name, or (b) RoleMixin.__init_subclass__ should specifically rewrap the method or (c) raise a depreciation error.

@jace
Copy link
Member Author

jace commented Mar 19, 2024

The roles_for cache should use a weak-key dict so that an actor that's no longer needed will also drop associated LazyRoleSet instances.

For cache invalidation: del obj.roles_for will drop the cache for all actors. del obj.roles_for[actor] will drop for a specific actor. Neither will be typically required as obj itself is not cached between requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant