[Django] #28999: URL Reverse does not work for CBV (Class Based Views)
Django
-----------------------------------------+---------------------------------
Reporter: airstandley | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 2.0
Severity: Normal | Keywords: url reverse cbv
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+---------------------------------
This is primarily an issue with CBV {{{get_view()}}} method.
However this affects basically all url reverse resolution as urls.base
{{{reverse()}}} -> urls.resolvers {{{get_resolver()}}} -> urls.resolvers
**RegexURLResolver** -> utils.datastructures **MultiValueDict** -> dict()
{{{__getitem__()}}} which uses the key's hash value.
I discovered this while attempting to reverse a class based view. No
matter what I tried I could not get a reverse url. Quite a bit of testing
and digging later, the issue appears to be that {{{as_view()}} creates a
new view function on every call, and every one of these functions has a
unique hash. This means that using the result of one call to "as_view" as
the key in **MultiValueDict** results in a value you can not retrieve with
a subsequent call.
{{{
from testapp.views import MyCBView
from django.utils.datastructures import MultiValueDict,
MultiValueDictKeyError
lookups = MultiValueDict()
first_call = MyCBView.as_view()
second_call = MyCBView.as_view()
# Does Not Work
lookups[first_call] = "Test Retrieval"
try:
test = lookups[second_call]
except MultiValueDictKeyError:
print("Lookup Failed {} != {}".format(first_call.__hash__(),
second_call.__hash__()))
# Works
test = lookups[first_call]
print("Lookup Succeeded test={}".format(test))
}}}
I am fairly certain that it is not intended (and certainly not documented)
that you must store the return of "as_view", and use that stored value in
your urlconf, if you ever want to be able to reverse using a CBV instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/28999>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: airstandley | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 2.0
Keywords: url reverse cbv | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I'm not sure if that's worth trying to fix (besides documenting that it
doesn't work) -- is there a reason your prefer reversing that way as
opposed to using a URL name?
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:1>
Django
Reporter: airstandley | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 2.0
Keywords: url reverse cbv | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by airstandley):
I think it may be possible to fix by simply caching the view function when
it is created and from then on returning the cache, but I haven't yet
looked into how this would effect the initwargs.
Hmm, I'm not sure how to best describe my preference. It's part of a
larger experimentation with the framework.
I'm trying to experiment with using {{{get_absolute_url}}} on my models,
and I have CBV DetailView classes for my models. Firstly it seems round-
about to use a URL name that represents a url->view mapping when I know
the view I want. The View Class for a given Model is a core relationship
that should never change, if someone uses my Models they should want to
use my Views for those model. The URL name "should" not change, but it is
perfectly reasonable someone may want to use my models and my views
without using my urlconf. The url schema (an thus URL name) could be
considered a separate matter to the models/views so I would like to de-
couple these things in code for my Models/Views.
I hope that makes sense?
Secondly I would ideally like to find a way to create a 'reverse' that
find the url regardless of the namespacing. (So a user of my app would not
have to update my models.py if they wanted to include my urlconf under a
namespace). Obviously this won't work with URL names, but since every
function should have a unique hash, I believe I can make it work using
view functions... But that is currently impossible for CBV because of this
issue.
I'm probably trying to be far to fancy for my own good here, but that is
the reason for my preference.
I admit, it is likely that for 99% of people, documenting that it doesn't
work would probably be a satisfactory fix.
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:2>
Django
Reporter: airstandley | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 2.0
Keywords: url reverse cbv | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by airstandley):
So I think I have a possible fix, but I'm not yet set up for contributing
to Django so I haven't run the test suite on it. Preliminary testing
appeared to work.
I would be interested in hearing others thoughts before investing more
time into this.
{{{
@classonlymethod
def as_view(cls, **initkwargs):
"""
Main entry point for a request-response process.
"""
for key in initkwargs:
if key in cls.http_method_names:
raise TypeError("You tried to pass in the %s method name
as a "
"keyword argument to %s(). Don't do that."
% (key, cls.__name__))
if not hasattr(cls, key):
raise TypeError("%s() received an invalid keyword %r.
as_view "
"only accepts arguments that are already "
"attributes of the class." %
(cls.__name__, key))
class callable_view(object):
# Instance of callable_view is callable and acts as a view
function
def __call__(self, request, *args, **kwargs):
self = cls(**initkwargs)
if hasattr(self, 'get') and not hasattr(self, 'head'):
self.head = self.get
self.request = request
self.args = args
self.kwargs = kwargs
return self.dispatch(request, *args, **kwargs)
def __hash__(self):
return hash(cls) # Class' 'view function' hash is defined
by the Class
view = callable_view()
view.view_class = cls
view.view_initkwargs = initkwargs
# take name and docstring from class
update_wrapper(view, cls, updated=())
# and possible attributes set by decorators
# like csrf_exempt from dispatch
update_wrapper(view, cls.dispatch, assigned=())
return view
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:3>
Django
Reporter: airstandley | Owner: nobody
Type: Bug | Status: new
Component: Generic views | Version: 2.0
Keywords: url reverse cbv | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Marten Kenbeek):
Replying to [comment:3 airstandley]:
> So I think I have a possible fix, but I'm not yet set up for
contributing to Django so I haven't run the test suite on it. Preliminary
testing appeared to work.
>
> I would be interested in hearing others thoughts before investing more
time into this.
>
> <snip>
The `callable_view` would compare equal regardless of its `initkwargs`, I
don't think that's the behaviour we'd want. We can compare the initkwargs
as well, but that becomes kinda iffy when more complex arguments don't
necessarily compare equal, in which case we get the same problem but in a
less obvious way that's harder to debug.
I'd much rather document the pattern of calling `.as_view()` once in
`views.py` and using the result as if it were a function-based view (if
that's not yet documented), e.g.:
{{{
class MyDetailView(DetailView):
...
my_detail_view = MyDetailView.as_view()
}}}
Then you can use `my_detail_view` as if it were a function-based view, and
reversing would work as expected. I think that should solve your usecase
as well without requiring changes to the way `.as_view()` and `reverse()`
work.
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:4>
Django
instance
--------------------------------------+------------------------------------
Reporter: airstandley | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Keywords: url reverse cbv | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* component: Generic views => Documentation
* stage: Unreviewed => Accepted
* type: Bug => Cleanup/optimization
Comment:
I don't think the pattern that Marten described is documented.
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:5>
Django
instance
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by airstandley):
Replying to [comment:4 Marten Kenbeek]:
> The `callable_view` would compare equal regardless of its `initkwargs`,
I don't think that's the behaviour we'd want.
>
> <snip>
Alright, thanks for the insight. I was not entirely sure what the
intention of the `initkwargs` was. If the intent is that a CBV will be
used more than once with different `initkwargs` then I can not think of
any approach with `callable_view` that would work.
The latest docs suggest using `CBV.as_view()` in the urlconf
`urlpatterns`. Would you change all of those references or add an addendum
stating the downside of that approach and documenting the pattern that
Marten described?
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:6>
Django
instance
--------------------------------------+------------------------------------
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham):
I would add a note and not change existing examples. I think reversing by
instance isn't a common need.
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:7>
Django
instance
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Clifford Gama):
* owner: nobody => Clifford Gama
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:8>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Clifford Gama):
Comment:
[https://github.com/django/django/pull/18688 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:9>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Sarah Boyce):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:10>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Clifford Gama):
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:11>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Sarah Boyce):
* needs_better_patch: 0 => 1
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:12>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Clifford Gama):
* needs_better_patch: 1 => 0
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:13>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Cleanup/optimization | Status: assigned
Component: Documentation | Version: 2.0
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sarah Boyce):
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:14>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Component: Documentation | Version: 2.0
Severity: Normal | Resolution: fixed
| checkin
Easy pickings: 0 | UI/UX: 0
Changes (by Sarah Boyce <42296566+sarahboyce@…>):
* resolution: => fixed
* status: assigned => closed
Comment:
In [changeset:"4d11ea1ef01eba14b3a48a727f07f723f782fd84" 4d11ea1e]:
{{{#!CommitTicketReference repository=""
revision="4d11ea1ef01eba14b3a48a727f07f723f782fd84"
Fixed #28999 -- Documented how to reverse a class-based view by instance.
Co-authored-by: Sarah Boyce <42296566+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:16>
Django
instance
-------------------------------------+-------------------------------------
Reporter: Andrew Standley | Owner: Clifford
Type: | Gama
Severity: Normal | Resolution:
| checkin
Easy pickings: 0 | UI/UX: 0
Comment (by Sarah Boyce <42296566+sarahboyce@…>):
In [changeset:"be138f32ed32a4bf3e62305145423285e462c853" be138f32]:
{{{#!CommitTicketReference repository=""
revision="be138f32ed32a4bf3e62305145423285e462c853"
Refs #28999 -- Added tests for reversing a class-based view by instance.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/28999#comment:15>