Skip to content

Commit

Permalink
Lazy role lookup performance optimizations (#420)
Browse files Browse the repository at this point in the history
* Perf: `LazyRoleSet.__bool__` more performant
* Bugfix: `RoleAccessProxy.__attr_available` accidentally forced enumeration of all roles
* Bugfix: `RoleAccessProxy`'s getitem/setitem/getattr/setattr also accidentally forced enumeration of all roles when raising an exception
  • Loading branch information
jace committed Oct 27, 2023
1 parent e6d19a9 commit e5c8656
Showing 1 changed file with 19 additions and 9 deletions.
28 changes: 19 additions & 9 deletions src/coaster/sqlalchemy/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ def __init__(
self._scanned_granted_by: t.Set[str] = set() # Contains relattr

def __repr__(self) -> str: # pragma: no cover
return f'LazyRoleSet({self.obj}, {self.actor})'
return f'LazyRoleSet({self.obj!r}, {self.actor!r}, {self._present!r})'

def _from_iterable( # pylint: disable=arguments-differ
self, it: t.Iterator[str]
Expand Down Expand Up @@ -508,7 +508,9 @@ def __len__(self) -> int:

def __bool__(self) -> bool:
# Make bool() faster than len() by using the cache first
return bool(self._present) or bool(self._contents())
return bool(self._present) or any(
self._role_is_present(role) for role in self.obj.__roles__
)

def __eq__(self, other: t.Any) -> bool:
if isinstance(other, LazyRoleSet):
Expand Down Expand Up @@ -851,9 +853,17 @@ def __attr_available(
for role, roledict in self._obj.__roles__.items()
if attr in roledict.get(action, ())
}
if (
isinstance(self._roles, LazyRoleSet) and self._roles.has_any(granting_roles)
) or (self._roles & granting_roles):
if not granting_roles:
# No role grants access to this attribute for the given action
absent.add(attr)
return False
if isinstance(self._roles, LazyRoleSet):
# If we have a LazyRoleSet, use its `has_any` method for lazy testing
# (test for overlap in present roles, then test for grant of other roles)
if self._roles.has_any(granting_roles):
present.add(attr)
return True
elif self._roles & granting_roles:
present.add(attr)
return True
absent.add(attr)
Expand Down Expand Up @@ -886,7 +896,7 @@ def __getattr__(self, attr: str) -> t.Any:
return self.__get_processed_attr(attr)
raise AttributeError(
f"{self._obj.__class__.__qualname__}.{attr};"
f" current roles {set(self.current_roles)!r}"
f" current roles {self.current_roles!r}"
)

def __setattr__(self, attr: str, value: t.Any) -> None:
Expand All @@ -895,7 +905,7 @@ def __setattr__(self, attr: str, value: t.Any) -> None:
return setattr(self._obj, attr, value)
raise AttributeError(
f"{self._obj.__class__.__qualname__}.{attr};"
f" current roles {set(self.current_roles)!r}"
f" current roles {self.current_roles!r}"
)

def __getitem__(self, key: str) -> t.Any:
Expand All @@ -904,7 +914,7 @@ def __getitem__(self, key: str) -> t.Any:
return self.__get_processed_attr(key)
raise KeyError(
f"{self._obj.__class__.__qualname__}.{key};"
f" current roles {set(self.current_roles)!r}"
f" current roles {self.current_roles!r}"
)

def __len__(self) -> int:
Expand All @@ -919,7 +929,7 @@ def __setitem__(self, key: str, value: str) -> None:
return setattr(self._obj, key, value)
raise KeyError(
f"{self._obj.__class__.__qualname__}.{key};"
f" current roles {set(self.current_roles)!r}"
f" current roles {self.current_roles!r}"
)

def __iter__(self) -> t.Iterator[str]:
Expand Down

0 comments on commit e5c8656

Please sign in to comment.