django-rest-framework-condition
django-rest-framework-condition copied to clipboard
update decorator to not create decorated functions per request
While the existing implementation is simple it: creates a decorated function per request, calls it, and then will discard it. Since the functionality is straightforward and also encapsulated fairly well in Django, this PR modifies the decorator to copy the original Django decorator, and calls the same helper functions where possible.
Because the functionality is no longer simply wrapping Django's internal implementation, but is still mostly the same, there have been tests added to compare the requests generated by Django vs requests generated by Django Request Framework/this library.
Finally, in order to be less confusing to library writers the call signature of etag_func and last_modified_func has been updated to match Django Request Framework and will pass a DRF Request object instead
of Django's native Request object:
# from the original signature
def original_etag_func(wsgi_request, *args, **kwargs): pass
# to the updated signature
def updated_etag_func(drf_self, drf_request, *args, **kwargs): pass
In order to ease the transition between the two signatures, there has been a flag "use_self" added to the decorators, and a warning will be emitted if use_self has not been set to True.
Codecov Report
Merging #10 into master will decrease coverage by
7.14%. The diff coverage is91.17%.
@@ Coverage Diff @@
## master #10 +/- ##
===========================================
- Coverage 100.00% 92.85% -7.15%
===========================================
Files 2 2
Lines 21 42 +21
===========================================
+ Hits 21 39 +18
- Misses 0 3 +3
| Impacted Files | Coverage Δ | |
|---|---|---|
| rest_framework_condition/decorators.py | 92.30% <91.17%> (-7.70%) |
:arrow_down: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 13c076f...7fe7e94. Read the comment docs.
@jozo not sure if you have time to look over this PR?
Thank you for your PR @terencehonles . I'm wondering what is the benefit of this PR. Is it performance? If so, how much it improves it? Does it outweigh simplicity of the current implementation?
Well the bigger/biggest benefit is if you need to use the request object that DRF provides you can actually use it. From there you can use request.parser_context to get the view or other things that are passed via the context. This allows conditions that look at what type of API request is being made and makes more sense as this is condition support for DRF not just Django.
As far as performance I figured this was likely just overlooked, but I don't expect Django's conditions expected to be used this way.