admin: use BitHandler instead of bitor operator #103
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@ortholeviator can you fix the linter error? |
|
Also, can you explain why the change is correct, perhaps in the commit message? |
|
@timabbott Somehow I didn't thought there would be a linter, especially since the tests had passed in my local machine. I'll make sure it is fixed for all other pull requests. |
`FieldListFilter.__init__` populates `used_parameters` as follows.
for p in self.expected_parameters():
if p in params:
value = params.pop(p)
self.used_parameters[p] = prepare_lookup_value(p, value)
`FieldListFilter.expected_parameters` simply returns a singleton list
containing `self.lookup_kwarg`, which is simply a copy of `field_path`.
`used_parameters` therefore has at most one entry with key set to
`field_path` and value from the parameters, since `prepare_lookup_value`
returns the value as-is unless the key ends with either `'__in'` or
`'__isnull'`.
`BitHandler` "represents an array of bits, each as a `Bit` object."
It is one of two classes (the other being `Bit`) that is accepted by the
specialized `exact` lookup. Here, we would only like to filter out
records which has all of the bits in the given integer parameter set.
The lookup uses the value in the given `BitHandler` as a mask, in the
form `field & mask = mask`.
The keys are not used here, since they are simply used for named bit
enumeration and manipulation. This is justified for the following
reasons:
1. The user may supply an integer with multiple bits set which `Bit`
cannot handle.
2. The class is unlikely to change to adopt a strict behavior to filter out anything not in `_keys`
3. This commit only aims to fix the bug and it would be out of the scope.
Later version may make the `keys` parameter of `BitHandler optional
or introduce some new wrapper object or a custom lookup for achieving
this.
This is a bit more efficient implementation compared to the intended one
since instead of doing `field = field | mask`, it instead chooses the
straightforward approach of `field & mask = mask`. SQL databases are
more likely to recognize the last pattern for optimization, although they
should in all way be equivalent.
Plus, empty keyword arguments for `filter` essentially means a no-op, so
in this case we just return `queryset` directly since they are meant to
be immutable anyway.
Fixes disqus#61, disqus#64, and disqus#89
ortholeviator
force-pushed
the
fix/admin-bitor
branch
from
13064ee to
4588fd5
Compare
|
@timabbott I'll happily introduce some other lookup mechanism if the fix indeed seems inelegant. |
This was referenced Apr 6, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #61, #64, and #89 in BitFieldListFilter, with a more efficient implementation.