[Django] #30101: Recommended middleware syntax fails for some testing cases (when not using Client)
Django
Client)
-------------------------------------+-------------------------------------
Reporter: tdiam | Owner: nobody
Type: | Status: new
Uncategorized |
Component: | Version: 2.1
Documentation | Keywords: middleware, unit
Severity: Normal | test, requestfactory, request,
Triage Stage: | layering
Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
It is common for developers to prefer RequestFactory over Client in unit
testing, or any similar module that mocks requests without processing the
URLs or applying the enabled middleware. So, when a unit test needs to
test a view while using a middleware, the easiest way to do so is:
{{{
#!python
from django.test import RequestFactory
from .views import MyView
from .middleware import MyMiddleware
request = RequestFactory().get('/')
view = MyMiddleware(MyView)
response = view(request)
}}}
Also, it may be needed to test views that accept arguments (e.g. captured
URL parameters), which would probably look like this:
{{{
#!python
request = RequestFactory().get('/articles/4/')
response = MyView(request, pk=4)
}}}
The problem arises when we both use test views that accept parameters and
a custom middleware. The current documentation offers examples for writing
such a middleware:
{{{
#!python
def MyMiddleware(get_response):
# One-time configuration and initialization.
def middleware(request):
# Code to be executed for each request before
# the view (and later middleware) are called.
response = get_response(request)
# Code to be executed for each request/response after
# the view is called.
return response
return middleware
}}}
Given the above tests, one would expect this one to work as well as
expected:
{{{
#!python
request = RequestFactory().get('/articles/4/')
view = MyMiddleware(MyView)
response = view(request, pk=4)
}}}
However this is not the case, since the `middleware` function expects only
a single positional argument and not the keyword argument `pk`, thus
raising a TypeError.
The attached files recommend a more flexible (albeit less simple) syntax
and data flow for custom middleware that would prevent inexperienced
developers from getting stuck here.
--
Ticket URL: <https://code.djangoproject.com/ticket/30101>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Client)
-------------------------------------+-------------------------------------
Reporter: tdiam | Owner: nobody
Component: Documentation | Version: 2.1
Severity: Normal | Resolution:
Keywords: middleware, unit | Triage Stage:
test, requestfactory, request, | Unreviewed
layering |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* Attachment "middleware_syntax_in_docs.diff" added.
Patch for documentation
Django
Client)
-------------------------------------+-------------------------------------
Reporter: tdiam | Owner: nobody
Component: Documentation | Version: 2.1
Severity: Normal | Resolution:
Keywords: middleware, unit | Triage Stage:
test, requestfactory, request, | Unreviewed
layering |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
response = get_response(request)
return response
return middleware
}}}
Given the above tests, one would expect this one to work as expected too:
{{{
#!python
request = RequestFactory().get('/articles/4/')
view = MyMiddleware(MyView)
response = view(request, pk=4)
}}}
However this is not the case, since the `middleware` function expects only
a single positional argument and not the keyword argument `pk`, thus
raising a TypeError.
The attached files recommend a more flexible (albeit less simple) syntax
and data flow for custom middleware that would prevent inexperienced
developers from getting stuck here.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30101#comment:1>
Django
Client)
-------------------------------------+-------------------------------------
Diamantidis |
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 2.1
Severity: Normal | Resolution:
test, requestfactory, request, |
layering |
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/30101#comment:2>
Django
Client)
-------------------------------------+-------------------------------------
Diamantidis |
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 2.1
Severity: Normal | Resolution:
Keywords: middleware, unit | Triage Stage: Accepted
test, requestfactory, request, |
layering |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Nasir Hussain):
Replying to [comment:1 Theodore Diamantidis]:
Should I create a PR with the patch in the documentation you provided?
I don't know what's the next steps if someone uploads a patch file too.
--
Ticket URL: <https://code.djangoproject.com/ticket/30101#comment:3>
Django
Client)
-------------------------------------+-------------------------------------
Diamantidis |
Cleanup/optimization |
Component: Documentation | Version: 2.1
Keywords: middleware, unit | Triage Stage: Accepted
test, requestfactory, request, |
layering |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
This usage isn't correct:
{{{
view = MyMiddleware(MyView)
}}}
It treats views and middlewares as if they were equivalent, which they are
not.
Django's `BaseHandler` provides a `_get_response()` method that uses the
`URLConf ` and `URLResolver` to determine the callback (i.e. view
function) and parameters to pass to the view.
([https://github.com/django/django/blob/d02b2aa11e5b6c351a9a2c0673c23569889f90d6/django/core/handlers/base.py#L100-L101
django/core/handlers/base.py].)
It's this `_get_response()` that sits at the center of the ''Middleware
Onion™''. You can't just pass in a view.
To simulate the full dispatch you'd need to do something like this, as a
minimum:
{{{
request = RequestFactory().get('/articles/4/')
def _get_response(request):
return MyView(request, pk=4)
middleware_chain = MyMiddleware(_get_response)
}}}
(Lots of ways we might make that better...)
I've thought about adding something on this to the
[https://docs.djangoproject.com/en/2.1/topics/http/middleware/ middleware
usage guide] but, by the time you're jumping through these hoops, I think
the best advice is to **use the provided test client** and let Django
handle the mapping between your middleware and views.
--
Ticket URL: <https://code.djangoproject.com/ticket/30101#comment:4>