Use sync/async pattern for middleware since Django 3.1

[john-parton]

Reworks the middleware so that it uses the recommended pattern in django docs for async/sync middleware.

Description

Changed middleware to use pattern from docs: https://docs.djangoproject.com/en/4.2/topics/http/middleware/#asynchronous-support

Related Issue

Motivation and Context

The current middleware is synchronous only. As Django is moving towards a more async capable stack, this puts the project inline with Django's goals.

How Has This Been Tested?

Didn't.

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• I have run the pre-commit run command to format and lint.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have added my name and/or github handle to AUTHORS.rst
• I have added my change to CHANGES.rst
• All new and existing tests passed.

[codecov]

Codecov Report

Merging #1209 (49934f4) into master (2e3e325) will decrease coverage by 0.30%.
The diff coverage is 78.94%.

@@            Coverage Diff             @@
##           master    #1209      +/-   ##
==========================================
- Coverage   97.38%   97.08%   -0.30%     
==========================================
  Files          23       23              
  Lines        1263     1271       +8     
  Branches      204      208       +4     
==========================================
+ Hits         1230     1234       +4     
- Misses         16       19       +3     
- Partials       17       18       +1     

Files Changed	Coverage Δ
simple_history/middleware.py	80.00% <78.94%> (-20.00%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[ddabble]

@john-parton Nice! What do you think about rewriting it so that HistoryRequestMiddleware remains class-based? I think that would be preferable, due to the increased flexibility class-based middleware provides, and to remove any potential confusion caused by the class-like naming of the function you committed (I do understand why you kept the name unchanged, though).

I'm planning on making it class-based myself if I don't receive any response within a week or so 🙂

[john-parton]

It's surprisingly difficult to properly subclass a middleware that offers both sync and async functionality. I think it probably just presents a foot-gun. I can't think of any use-case which subclasses such a simple middleware. Any customization I think would be done by layering a different middleware before or after it.

If you give me some actual use-case, I can refactor it, otherwise you're welcome to do so.

[ddabble]

Hm yeah, good points. I also happened to find this example, illustrating some additional downsides.

[ddabble]

#1188 has already received an approval and will probably be merged within a week or so, which will lead to conflicts in this PR. I can post a comment here when it's merged, if you'd like to resolve those conflicts; otherwise, I can just do it myself (keeping the middleware function-based) and then merge 🙂

[john-parton]

Yeah, let me know when it's merged. I can take a crack at it.

[ddabble]

@john-parton It's been merged now :)

[john-parton]

@ddabble Updated. I originally just had the logic copy/pasted in the sync and async versions, but that was nasty, so I encapsulated the setting and deleting of the property with a helper.

[ddabble]

Looks good, thank you! 😊