Object around user permissions dictionary

Question

I want to expose permissions property on ORM User model:

PERMISSIONS_MAP= {
 'admin': ['can_delete'],
}
class DatabaseModel:
 pass
class User(DatabaseModel):
 def __init__(self, role):
 self.role = role
 
 @property
 def permissions(self):
 return role_permissions.list(self.role)
class RolePermissions:
 def __init__(self, permissions_map: dict = PERMISSIONS_MAP):
 self.permissions_map = permissions_map
 def list(self, role):
 print(f'listing permissions')
 return [p for p in self.permissions_map.get(role, [])]
role_permissions = RolePermissions()
admin = User('admin')
assert admin.permissions == ['can_delete']
assert admin.permissions == ['can_delete'] # here the permissions will be recalculated, but ideally shouldn't
guest = User('guest')
assert guest.permissions == []

But I don't like 2 things:

1. Return value of permissions property will be calculated each time the property is called:

I can easily alleviate that by switching property with functools.cached_property decorator.

However, I would like to instead improve the design. Would changing to RolePermissions(self.role).list() from role_permissions.list(self.role) be a move into a right direction? Where do I go from there to mitigate unwanted recalculations?

2. User class is coupled to user_permissions. I could solve it through dependency injection, yet it feels cumbersome to make the permissions property accept UserPermissions object. Is there a better way to decouple this?

Any other suggestions are of course welcome!

• Reinderien: Does this use SQLAlchemy?
• barciewicz: @Reinderien: nope, Mongoengine, a wrapper around pymongo(MongoDB). It is similar however (was modeled after DjangoORM)

Answer 1

PERMISSIONS_MAP should probably be a map of enums to enum sets, not strings to string lists. Stringly-typed data are not a good idea.

Add PEP484 type hints to your function signatures.

permissions being a property is fine. You're concerned about recalculation, but

1. once this code is refactored and the list comprehension goes away, the amount of "recalculation" will be trivial, and
2. whereas you could add an lru_cache, that is not called-for here.

Do not use mutable structures like dictionaries for argument defaults. Usual workaround is to default to None and then fill in with a dictionary in function body.

Do not call a method list since that's a built-in.

Your list comprehension should go away. You can just delete the entire RolePermissions class, and in User,

@property
def permissions(self) -> set[Permission]:
 return PERMISSIONS_MAP[self.role]

The get is problematic because it hides a situation where a user has an invalid role not present in PERMISSIONS_MAP, so don't do that.

• barciewicz: Thanks! I admit to oversimplifying my example, in a more complete version USER_PERMISSIONS is indeed a mapping of enums, hence the list comprehension (it extracts enum values). So if I decided to keep the RolePermissions class (as a wrapper for the mapping with as_list method exposed), do you have any opinion on point 2. in my question? How can I (should I?) get rid of the coupling between role_permissions and User?
• Reinderien: I don't see that coupling as much of a concern. More concerning are lack of type hints, and a singleton permissions class that doesn't deserve to be a class.