ASGIRequest compatible middleware

[an7oine]

Super call handles MiddlewareMixin._async_check() too.

[codecov[bot]]

Codecov Report

Merging #126 (71ddd5e) into master (fb3008e) will not change coverage. The diff coverage is 100.00%.

:exclamation: Current head 71ddd5e differs from pull request most recent head 7cb561c. Consider uploading reports for the commit 7cb561c to get more accurate results

@@            Coverage Diff            @@
##            master      #126   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          391       399    +8     
  Branches        66        67    +1     
=========================================
+ Hits           391       399    +8     

Impacted Files	Coverage Δ
django_hosts/middleware.py	100.00% <100.00%> (ø)
django_hosts/__init__.py	100.00% <0.00%> (ø)
django_hosts/defaults.py	100.00% <0.00%> (ø)
django_hosts/resolvers.py	100.00% <0.00%> (ø)
django_hosts/templatetags/hosts.py	100.00% <0.00%> (ø)
django_hosts/templatetags/hosts_override.py	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update fb3008e...7cb561c. Read the comment docs.

[an7oine]

Crucial part is self._async_check(). This could be done here if supercall is a problem.

[LeMinaw]

I'll investigate a bit on this in the next few days, there is still something very unclear about this get_response parameter to me.

Perhaps in the meantime, it would be great to provide some testcase(s?) involving ASGIRequest / async capability?

[an7oine]

Are we getting close to a merge? Ability to install from Pypi in ASGI production would be nice.

[ddabble]

@LeMinaw

Looking at the source for MiddlewareMixin init method, it seems the reason no supercall is made ATM is the parent class does not support None as a get_response argument (which is set by the subclass constructor as a default value).

It seems like back when the get_response argument was added (in this commit), it pretty much copied the code in the parent class' __init__() method, and so maybe not calling the __init__() method was an oversight instead of a deliberate choice to allow a None value..? 🤔 Regardless, as Django's code for MiddlewareMixin stands today, it has potential for producing an error if get_response is None (both in __call__() and __acall__()), which is not expected behavior of a child class. Also, I'm fairly sure (my knowledge on Django middleware is also limited 😅) that the get_response argument is only provided by Django itself when initializing the middleware chain - with no practical way of changing this behavior - and that it's not reasonable to assume that users of this library will somehow try to pass None. If this is not the case, however, we should probably just add a line to the changelog saying that the get_response argument doesn't accept a None value 🙂

[ddabble]

Current release 5.0 is incompatible with channels >= 3.0.0 (as stated in the updated changelog), as the middleware classes are not correctly marked as async-capable, which this PR fixes (by calling MiddlewareMixin.__init__()), so it would be nice to have this PR merged and version 5.1 released 🙂

Do you have any input on this, @timgraham?

[ddabble]

I squashed all 6 commits into 1 (203cf3bd5f1faa7b697e622f553c6e8a051152cf), as they all contained tightly connected changes 🙂

@timgraham Is this more like what you had in mind?

[ddabble]

There doesn't seem to be much activity from any other experienced contributors here, so would you have time to review the changes I made that you requested, @timgraham? 🙂