daisylb / bridgekeeper

Django permissions, but with QuerySets
https://bridgekeeper.leigh.party/
MIT License
41 stars 14 forks source link

Add shorthand to permission registry #6

Open daisylb opened 6 years ago

daisylb commented 6 years ago

Currently, permission.py files are slightly more verbose than they otherwise could be, especially when permissions refer to other permissions. For example, currently, you might have:

perms['shrubberies.change_shrubbery'] = has_role('shrubber') | Relation('store', perms['shrubberies.change_store'])

The registry class should have a shorthand that cuts down on the perms boilerplate, allowing code like this:

shrubberies = perms.shorthand('shrubberies')
shrubberies.change_shrubbery = has_role('shrubber') | Relation('store', shrubberies.change_store)

This could be further shortened (at the expense of readability) by doing something like s = perms.shorthand('shrubberies') and then referring to and assigning to s.update_shrubbery.

perms.shorthand('shrubberies') would return a special namespace object whose __getattr__ and __setattr__ perform item lookups/assignments on the registry itself, with the string 'shrubberies.' prefixed.

It's tempting to make __setattr__ etc on the registry object itself behave like this, but because the registry is a subclass of dict, there are already lots of attributes and methods on it that we could potentially clash with.

We could just break backwards compatibility and make the registry itself a namespace object like what we're describing, one which creates a new child namespace upon attempting to access an unset attribute (so e.g. you can assign to perms.shrubberies.change_shrubbery without first having to assign perms.shrubberies to something). The downside of this is that it creates one extra thing that we'd need to explain to new users in our tutorial without huge benefit, whereas everyone who's going to be using this library already understands how a dict works.

Doing it this way means that it's opt-in, and using an explicit call to something (.shorthand() here) gives developers that are new to codebases that have opted-in something to go and look up in the documentation.

daisylb commented 6 years ago

A note regarding this bit:

We could just break backwards compatibility...

In general I'm too worried about compatibility in general until we hit 1.x, but compatibility breaks still mean I have to refactor the internal projects that use this library. In this case though, there's not a need to; the registry-as-namespace-object model described here could still provide a __setitem__. I'm not going to do what this paragraph describes anyway though, for the other reasons I've described above.