[Django] #33426: ResolverMatch repr is incorrect for Class Based Views

12 views
Skip to first unread message

Django

unread,
Jan 10, 2022, 7:56:55 AM1/10/22
to django-...@googlegroups.com
#33426: ResolverMatch repr is incorrect for Class Based Views
----------------------------------------+------------------------
Reporter: Keryn Knight | Owner: nobody
Type: Bug | Status: new
Component: Core (URLs) | Version: 4.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------+------------------------
The following test applies cleanly to 3.2.9, and AFAIK would apply roughly
correctly all the way back to when CBVs were introduced (I can't easily
test without going back to a super old Python and finding the test cases,
which have moved around):
{{{
"""
add to class: tests.urlpatterns_reverse.tests.ResolverMatchTests
"""
@override_settings(ROOT_URLCONF='urlpatterns_reverse.reverse_lazy_urls')
def test_classbased_repr(self):
self.assertEqual(
repr(resolve('/redirect/')),
"ResolverMatch(func=urlpatterns_reverse.views.LazyRedirectView, "
"args=(), kwargs={}, url_name=None, app_names=[], "
"namespaces=[], route=redirect/)",
)
}}}

The `_func_path` as AFAIK always been a representation to the fully
qualified dotted callable where possible, that is for a CBV it's the CBV
module + the class name.

As of 4.0, the `_func_path` has become `urlpatterns_reverse.views.view`, I
believe because of #32260 removing the use of `update_wrapper` and
intentionally not setting the `__name__` and `__qualname__` in favour
using the `view_class` attribute, as per the comment ''view_class should
be used to robustly determine the name of the view'' (see
[https://github.com/django/django/pull/14124 Pull Request])

Unfortunately I think that means the detection of class based views in
`ResolverMatch` no longer works correctly, and this can probably only be
resolved by making `ResolverMatch` CBV aware again by embedding detection
of `view_class` therein.

Noted it as a question in [https://github.com/django/django/pull/15286
this PR] for ticket #33396, but hoisting it here properly to be considered
separately.

The fix appears to ostensibly the same as for #33425 (see
[https://github.com/django/django/pull/15292 PR])
{{{
class ResolverMatch:
def __init__(...):
# ...
if hasattr(func, 'view_class'):
func = func.view_class
if not hasattr(func, '__name__'):
# ...
}}}

I have a branch which I shall push shortly to confirm it works generally.

--
Ticket URL: <https://code.djangoproject.com/ticket/33426>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jan 10, 2022, 9:23:14 AM1/10/22
to django-...@googlegroups.com
#33426: ResolverMatch repr is incorrect for Class Based Views
---------------------------------+----------------------------------------
Reporter: Keryn Knight | Owner: Keryn Knight
Type: Bug | Status: assigned

Component: Core (URLs) | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------
Changes (by Mariusz Felisiak):

* owner: nobody => Keryn Knight
* status: new => assigned
* has_patch: 0 => 1
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted


Comment:

Thanks for the report!

Regression in 7c08f26bf0439c1ed593b51b51ad847f7e262bc1.

--
Ticket URL: <https://code.djangoproject.com/ticket/33426#comment:1>

Django

unread,
Jan 10, 2022, 11:34:20 AM1/10/22
to django-...@googlegroups.com
#33426: ResolverMatch repr is incorrect for Class Based Views
-------------------------------------+-------------------------------------

Reporter: Keryn Knight | Owner: Keryn
| Knight
Type: Bug | Status: assigned
Component: Core (URLs) | Version: 4.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/33426#comment:2>

Django

unread,
Jan 10, 2022, 12:40:09 PM1/10/22
to django-...@googlegroups.com
#33426: ResolverMatch repr is incorrect for Class Based Views
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Keryn
| Knight
Type: Bug | Status: closed

Component: Core (URLs) | Version: 4.0
Severity: Release blocker | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"f4b06a3cc1e54888ff86f36a1f9a3ddf21292314" f4b06a3]:
{{{
#!CommitTicketReference repository=""
revision="f4b06a3cc1e54888ff86f36a1f9a3ddf21292314"
Fixed #33426 -- Fixed ResolverMatch.__repr_() for class-based views.

Regression in 7c08f26bf0439c1ed593b51b51ad847f7e262bc1.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33426#comment:3>

Django

unread,
Jan 10, 2022, 12:40:25 PM1/10/22
to django-...@googlegroups.com
#33426: ResolverMatch repr is incorrect for Class Based Views
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Keryn
| Knight
Type: Bug | Status: closed
Component: Core (URLs) | Version: 4.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"c8a6bf951b5ad34df852eea6e63a6ad6a0aecd87" c8a6bf95]:
{{{
#!CommitTicketReference repository=""
revision="c8a6bf951b5ad34df852eea6e63a6ad6a0aecd87"
[4.0.x] Fixed #33426 -- Fixed ResolverMatch.__repr_() for class-based
views.

Regression in 7c08f26bf0439c1ed593b51b51ad847f7e262bc1.

Backport of f4b06a3cc1e54888ff86f36a1f9a3ddf21292314 from main
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33426#comment:4>

Django

unread,
Jan 12, 2022, 11:57:04 AM1/12/22
to django-...@googlegroups.com
#33426: ResolverMatch repr is incorrect for Class Based Views
-------------------------------------+-------------------------------------
Reporter: Keryn Knight | Owner: Keryn
| Knight
Type: Bug | Status: closed
Component: Core (URLs) | Version: 4.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"d05ab13c56849ed09d02c1f188b7f47f0c090a2a" d05ab13c]:
{{{
#!CommitTicketReference repository=""
revision="d05ab13c56849ed09d02c1f188b7f47f0c090a2a"
Refs #33426 -- Simplified technical_404_response() with
ResolverMatch._func_path.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/33426#comment:5>

Reply all
Reply to author
Forward
0 new messages