Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#33426 closed Bug (fixed)

ResolverMatch repr is incorrect for Class Based Views

Reported by: Keryn Knight Owned by: Keryn Knight
Component: Core (URLs) Version: 4.0
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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 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 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 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.

Change History (5)

comment:1 by Mariusz Felisiak, 3 years ago

Has patch: set
Owner: changed from nobody to Keryn Knight
Severity: NormalRelease blocker
Status: newassigned
Triage Stage: UnreviewedAccepted

Thanks for the report!

Regression in 7c08f26bf0439c1ed593b51b51ad847f7e262bc1.

Version 0, edited 3 years ago by Mariusz Felisiak (next)

comment:2 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:3 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In f4b06a3:

Fixed #33426 -- Fixed ResolverMatch.repr_() for class-based views.

Regression in 7c08f26bf0439c1ed593b51b51ad847f7e262bc1.

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In c8a6bf95:

[4.0.x] Fixed #33426 -- Fixed ResolverMatch.repr_() for class-based views.

Regression in 7c08f26bf0439c1ed593b51b51ad847f7e262bc1.

Backport of f4b06a3cc1e54888ff86f36a1f9a3ddf21292314 from main

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In d05ab13c:

Refs #33426 -- Simplified technical_404_response() with ResolverMatch._func_path.

Note: See TracTickets for help on using tickets.
Back to Top