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!

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.