From 468949b8990c9060f2880e86b193828ecf8615f3 Mon Sep 17 00:00:00 2001 From: Chris Meyers Date: Mon, 8 Apr 2024 10:39:22 -0400 Subject: [PATCH] Remove uneeded drf_reverse overwrite * `drf_reverse()` was introduced here https://github.com/ansible/awx/commit/1a75b1836e3416260961e72e17c7736d6b43e0b3 * There is a comment about monkey patching. I can't find the monkey patch it is referencing. * AWX `drf_reverse()` is a copy paste of this https://github.com/encode/django-rest-framework/blob/master/rest_framework/reverse.py#L32 * The only difference is DRF's version calls `preserve_builtin_query_params()` * `preserve_builtin_query_params()` only does something if `api_settings.URL_FORMAT_OVERRIDE` is defined. * We don't use `REST_FRAMEWORK.URL_FORMAT_OVERRIDE` --- awx/api/versioning.py | 25 +++++---------------- awx/main/tests/functional/api/test_auth.py | 2 +- awx/main/tests/functional/api/test_oauth.py | 4 +++- 3 files changed, 9 insertions(+), 22 deletions(-) diff --git a/awx/api/versioning.py b/awx/api/versioning.py index f6778b60d2fa..09da66b62011 100644 --- a/awx/api/versioning.py +++ b/awx/api/versioning.py @@ -2,9 +2,8 @@ # All Rights Reserved. from django.conf import settings -from django.urls import NoReverseMatch -from rest_framework.reverse import _reverse +from rest_framework.reverse import reverse as drf_reverse from rest_framework.versioning import URLPathVersioning as BaseVersioning @@ -15,25 +14,9 @@ def is_optional_api_urlpattern_prefix_request(request): return False -def drf_reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra): - """ - Copy and monkey-patch `rest_framework.reverse.reverse` to prevent adding unwarranted - query string parameters. - """ - scheme = getattr(request, 'versioning_scheme', None) - if scheme is not None: - try: - url = scheme.reverse(viewname, args, kwargs, request, format, **extra) - except NoReverseMatch: - # In case the versioning scheme reversal fails, fallback to the - # default implementation - url = _reverse(viewname, args, kwargs, request, format, **extra) - else: - url = _reverse(viewname, args, kwargs, request, format, **extra) - +def transform_optional_api_urlpattern_prefix_url(request, url): if is_optional_api_urlpattern_prefix_request(request): url = url.replace('/api', f"/api/{settings.OPTIONAL_API_URLPATTERN_PREFIX}") - return url @@ -46,7 +29,9 @@ def reverse(viewname, args=None, kwargs=None, request=None, format=None, **extra kwargs = {} if 'version' not in kwargs: kwargs['version'] = settings.REST_FRAMEWORK['DEFAULT_VERSION'] - return drf_reverse(viewname, args, kwargs, request, format, **extra) + url = drf_reverse(viewname, args, kwargs, request, format, **extra) + + return transform_optional_api_urlpattern_prefix_url(request, url) class URLPathVersioning(BaseVersioning): diff --git a/awx/main/tests/functional/api/test_auth.py b/awx/main/tests/functional/api/test_auth.py index d9ac588de323..7ecfe9de9582 100644 --- a/awx/main/tests/functional/api/test_auth.py +++ b/awx/main/tests/functional/api/test_auth.py @@ -6,7 +6,7 @@ from rest_framework.test import APIRequestFactory from awx.api.generics import LoggedLoginView -from awx.api.versioning import drf_reverse +from rest_framework.reverse import reverse as drf_reverse @pytest.mark.django_db diff --git a/awx/main/tests/functional/api/test_oauth.py b/awx/main/tests/functional/api/test_oauth.py index 4387f06b9caa..e95d2cdc4abf 100644 --- a/awx/main/tests/functional/api/test_oauth.py +++ b/awx/main/tests/functional/api/test_oauth.py @@ -8,8 +8,10 @@ from django.test.utils import override_settings from django.utils.encoding import smart_str, smart_bytes +from rest_framework.reverse import reverse as drf_reverse + from awx.main.utils.encryption import decrypt_value, get_encryption_key -from awx.api.versioning import reverse, drf_reverse +from awx.api.versioning import reverse from awx.main.models.oauth import OAuth2Application as Application, OAuth2AccessToken as AccessToken from awx.main.tests.functional import immediate_on_commit from awx.sso.models import UserEnterpriseAuth