Comments (10)
If you anticipate your predicate to be used with less arguments than required (and you should), then you should handle the case accordingly inside the predicate. What could the value be for the second argument of your predicate if the rule is tested with one argument, if not None? If the condition your predicate tests cannot be tested with less arguments, then check for that and return True or False as appropriate. It all depends on what conditions predicates test, on how you break down the problem you're trying to solve.
Predicates should generally be simple one-liners that are then ANDed, ORed, etc, with other predicates to check more complex conditions. You can mix predicates with different number of arguments each, it's ok, each one will receive the appropriate arguments, depending both on how many arguments it accepts and how many arguments were given in the test_rule
call -- whatever is less.
A simple trick I'm often using is to AND my predicate with another that simply checks for a condition that my predicate likes to take for granted. This allows you to keep predicates simple and reusable. For example, assume that you need to check whether a user can send an invite. Assume your predicate expects the first argument to be a proper User instance, and that this instance is properly associated with a Profile instance. Here's how I'd write my rule:
# helper predicate
@predicate
def has_profile(obj):
return obj and hasattr(obj, 'get_profile') and obj.get_profile() is not None
# condition predicate
@predicate
def has_invites_left(obj):
return obj.get_profile().number_of_invites_left > 0
add_rule('can_invite', has_profile & has_invites_left)
In this case, if has_profile
returns False, has_invites_left
will not be called -- i.e. normal boolean short-circuiting behaviour. You can similarly apply this to check for None arguments etc.
If I'm not helping you, write up!
from django-rules.
I think that the issue is more about that e.g. an exception should be raised, if a call to a predicate does not provide all expected (keyword) arguments.
if I defined my predicate to accept only two positional arguments and the caller gives only one argument, my predicate will be called with None for the second argument
from django-rules.
Limiting the way test_rule is called would limit the potential to combine predicates, since you'd only be able to combine predicates that accept the same number of arguments.
The thing I tried to allude to with my previous comment was that testing a rule with fewer arguments (or simply passing None for any of them) is fine and should be anticipated. The predicate (or a combination of predicates) is the best place to handle this case.
When testing a rule, you are essentially offloading work. Why check for "obj is not None" before calling test_rule? What if you test the same rule in 13 different places in your code? Handling the case centrally with a predicate is the best option in my opinion. You may even choose to add a predicate that raises an exception for being given less arguments if you like ;)
from django-rules.
Hi, @dfunckt thank you for your answer.
I find your approach very interesting and I will definitly use it.
@blueyed understood correctly what I implied with my question. I think raising an exception would be more expected. At least this is what I felt when I started to play with django-rules
.
Let me introduce you how I see the predicates handling:
First we should honour predicate signature, if there is 2 positional argument expected then we can not call the predicate if we have only one argument under the hood. This is provided out of the box by python
We should not try to be smarter than this (*args, **kwargs)
.
In order to achieve this, we could register as many as predicates we need under the same name.
As an eaxmple speaks better than words, here you go.
@rules.predicate
def are_equal(a, b):
return a == b
@are_equal.derivate # the predicate decorate another predicate. I know the name `derivate` sucks :)
def is_equal(a):
return a == 'a'
add_rule('can_proceed', are_equal)
assert test_rule('can_proceed', 'a', 'a') # are_equal predicate is called
assert test_rule('can_proceed', 'a') # is_equal predicate is called
test_rule('can_proceed') # KeyError, TypeError, PredicateNotFoundError, ... we can decide later which one
In this example I do not extend the complexity of adding rule for every combination of arguments (there is only one call of add_rule()
).
And still we have a fair guarantee that the predicates will behave as expected. As if there was no 'rulesets' in between.
That said, I didn't started to develop a POC, thus I do not know yet if it is really feasible. But at least it shows how it will be handled by the user.
How does it sounds ?
from django-rules.
Hi Nicolas, here's how you can do this now:
@predicate
def are_equal(a, b):
return a == b
@predicate
def is_equal(a):
return a == 'a'
@predicate
def equal(*args):
if len(args) == 1:
return is_equal(*args)
elif len(args) == 2:
return are_equal(*args)
else:
return False # or raise TypeError()
add_rule('can_proceed', equal)
I agree that it can be useful to factor out equal
or similar functions as helpers that come with rules to do these kind of things. I can't think of other use cases though, nor what the API should be.
I'm sure I don't quite like predicates decorating each other though (i.e. @are_equal.derive). I'd prefer a separate function that takes predicates as its arguments, does its thing and returns a new predicate, similar to what is_group_member
does -- it's more explicit and allows the predicates to be used in other rules too.
I'm also not convinced that changing test_rule
to enforce the correct number of arguments is a good choice -- certainly it shouldn't be the default, but I wouldn't mind a helper that does this should you like to do this.
test_rule
should be, in my opinion, totally ignorant of what the underlying predicates need to do their work, so that it can be used from anywhere with ease (imagine testing a rule in a Django template). This kind of logic should be placed inside predicates, or handled by composing a bunch of them.
But, like I said, we could improve rules as you suggest to allow more complex compositions of predicates. I'd also love to see any other use cases. Any ideas? Anyone?
Thanks!
from django-rules.
@dfunckt If I register a predicate with (*args)
as a signature, then Predicate.num_args == 0
.
So your example is not yet supported.
You are right for predicate that decorate other predicates, it is not the most elegant solution. What you suggest is better.
It test_rule should be ignorant of what underlying predicates need, to do their work, the best is to give the arguments as they come. You capture them with (*args, **kw)
and you pass them as they are.
We can reduce them safely if the predicate accept 0 or 1 argument, but extending them is more complex.
What we could do, instead of defaulting to None
, is to use a marker object. A singleton instance that tells the predicate he can ignore this argument.
from rules import NOT_SET
@rules.predicate
def are_equal(a, b):
if b is NOT_SET: # Here I'm sure that the caller didn't pass the b argument
return a == 'a'
else:
return a == b
from django-rules.
The reason Predicate.test
is defined as test(obj=None, target=None)
is to limit the scope of rules and enforce a consistent API between predicates and callers.
Your idea about the marker is however interesting, and I can see it can be useful to be able to distinguish between None
and "Not Given". I guess it's our final option if we rule out raising exceptions for testing with less arguments. Also, this change is backwards incompatible, but I guess we can allow ourselves this flexibility since the project is still so new.
I don't really mind changing Predicate.test
and test_rule
to allow passing arbitrary arguments. The only problem I see is that it would limit our ability to extend the API (for example, I think it would be useful to start passing predicates a context
dict that can be used to keep state between predicates during a single test_rule
invocation). A solution would be to extend test_rule to accept _args, but not *_kwargs.
Regarding the example, you're right, the example should be like this:
@predicate
def equal(a=None, b=None):
if target is None:
return is_equal(a)
else:
return are_equal(a, b)
from django-rules.
You are right, exclusive usage of (*args, **kwargs)
will prevent us to extend predicate API because there might be conflicts. so let's forget about it for now.
Regarding NOT_GIVEN
marker topic.
We might break current implementations if people starts to wrote in their predicate if target is not None:
.
But like you said:
the project is still so new
so let's do it ! 😈
I can submit a PR if there is concensus from the involved paticipants.
from django-rules.
Sure, please go ahead. Let's have a PR and we'll take it from there. Thanks!
from django-rules.
This issue is now resolved by merging #15
from django-rules.
Related Issues (20)
- Many reader of an object HOT 3
- is this repository receiving updates? HOT 1
- Are the predicates defined on a model called automatically?
- 'permission_required' fails on anonymous view callbacks HOT 5
- Pass Payload into Predicate for POST HOT 2
- Django Rules with Non Auth User Model
- Support Django 4.0 HOT 5
- Alternative to `RulesModelBase` in DRF to manage dependency on this library. HOT 8
- Using `AND` (`&`) with a predicate that returns `None` incorrectly returns `True` HOT 5
- Remove Python 2 code
- Django: how does one know which permission failed on a particular request? HOT 1
- Django rules with abstract base class throw error after addition
- Predicate parameters HOT 2
- How do you map predicates to objects and/or users? HOT 1
- AttributeError: 'NoneType' when trying to access the admin panel view list HOT 2
- Passing the view's request or extra arguments to a predicate.
- Consider cutting a new release? HOT 7
- Error: displaying objects even the user hasn't the correct permissions. HOT 1
- How to correctly use asymetric mixed permissions?
- Broken link in documentation HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from django-rules.