search

hasgeek / coaster

Common patterns for Flask apps
http://coaster.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
69 stars 13 forks source link

Make `roles_for` use a cache #453

Open jace opened 5 months ago

jace commented 5 months ago

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 commented 5 months ago

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.