Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear ThreadLocal request atttribute after response is processed #7

Merged
merged 8 commits into from
Aug 10, 2015
Merged

Clear ThreadLocal request atttribute after response is processed #7

merged 8 commits into from
Aug 10, 2015

Conversation

lucaswiman
Copy link

@jedie This pull request updates the ThreadLocal middleware so that the thread local reference to the request object is cleared when the response is processed by the middleware.

Issue Description

This is unlikely to matter in practice since a server will overwrite the previous _thread_locals.request when it receives a new request.

The use case this is intended to fix is:

  • A function f() is used on both the front end and back end of an application.
  • It collects metadata when used on the front end, for example to record an audit log of user actions. If get_current_{request,user} is called, it creates a model entry with a foreign key referencing the change the user made.
  • When used on the backend, no model entry M is created, or the User foreign key is NULL.
  • A test suite runs:
    • It first tests the front-end audit logging, using a simulated request from django's test client, which sets the _thread_locals.request variable. The User object is created as part of the test, then destroyed when the database transaction is rolled back by the tearDown method.
    • It subsequently runs a test of the back end functionality, which attempts to save an audit log of the action from the user on the previous test. Since this user no longer exists, a database integrity error is raised.

Changes

Testing

I added two views to the example app. One that returns the GET parameters from the current request, and one that simply raises an exception. I added tests that assert:

  • The current GET parameters are actually returned (i.e. that get_current_request actually returns the current request). This passed on the master branch, and is just included for completeness.
  • When a successful request is executed, the request reference is cleared. This failed using the code on the master branch.
  • When an unsuccessful request is executed (in the raise_exception view), the request reference is cleared. This failed using the code on the master branch.

Thanks for maintaining this library!

@jedie
Copy link
Owner

jedie commented Aug 8, 2015

Looks great!

Please add you to AUTHORS...

@lucaswiman
Copy link
Author

Added to AUTHORS in c60c9c8. Should I update the changelog as well?

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.13% when pulling c60c9c8 on lucaswiman:master into d3f784f on jedie:master.

jedie added a commit that referenced this pull request Aug 10, 2015
Clear ThreadLocal request atttribute after response is processed

Thanks!
@jedie jedie merged commit bb84dbf into jedie:master Aug 10, 2015
@jedie
Copy link
Owner

jedie commented Aug 10, 2015

Should I update the changelog as well?

I will do this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants