From 3fa8f30461b6e6841ac57eb72a9d345b92b5b2d5 Mon Sep 17 00:00:00 2001 From: Krystle Salazar Date: Mon, 16 Dec 2024 11:25:42 -0400 Subject: [PATCH] Remove MediaViewSet's lookup regex (#5273) Co-authored-by: Dhruv Bhanushali --- api/api/docs/audio_docs.py | 1 + api/api/docs/image_docs.py | 7 +++-- api/api/examples/__init__.py | 2 -- api/api/examples/audio_requests.py | 1 + api/api/examples/image_responses.py | 3 --- api/api/views/media_views.py | 5 +--- api/conf/settings/sentry.py | 4 +-- api/conf/urls/__init__.py | 2 +- api/pdm.lock | 6 ++--- .../integration/test_media_integration.py | 17 ++++++++++++ api/test/test_schema.py | 26 ++++++++++++++++++- 11 files changed, 53 insertions(+), 21 deletions(-) diff --git a/api/api/docs/audio_docs.py b/api/api/docs/audio_docs.py index 4b4c721ef30..44dd1710003 100644 --- a/api/api/docs/audio_docs.py +++ b/api/api/docs/audio_docs.py @@ -116,6 +116,7 @@ 201: (AudioReportRequestSerializer, audio_complain_201_example), 400: (ValidationError, None), 401: (AuthenticationFailed, None), + 404: (NotFound, None), }, eg=[audio_complain_curl], ) diff --git a/api/api/docs/image_docs.py b/api/api/docs/image_docs.py index e0146b18854..b200120049c 100644 --- a/api/api/docs/image_docs.py +++ b/api/api/docs/image_docs.py @@ -21,10 +21,8 @@ image_detail_404_example, image_detail_curl, image_oembed_200_example, - image_oembed_404_example, image_oembed_curl, image_related_200_example, - image_related_404_example, image_related_curl, image_search_200_example, image_search_400_example, @@ -109,7 +107,7 @@ res={ 200: (ImageSerializer(many=True), image_related_200_example["results"][0]), 401: (AuthenticationFailed, None), - 404: (NotFound, image_related_404_example), + 404: (NotFound, None), }, eg=[image_related_curl], ) @@ -119,6 +117,7 @@ 201: (ImageReportRequestSerializer, image_complain_201_example), 401: (AuthenticationFailed, None), 400: (ValidationError, None), + 404: (NotFound, None), }, eg=[image_complain_curl], ) @@ -138,7 +137,7 @@ 200: (OembedSerializer, image_oembed_200_example), 400: (ValidationError, image_oembed_400_example), 401: (AuthenticationFailed, None), - 404: (NotFound, image_oembed_404_example), + 404: (NotFound, None), }, eg=[image_oembed_curl], ) diff --git a/api/api/examples/__init__.py b/api/api/examples/__init__.py index 99c932c9893..0b54525bcdc 100644 --- a/api/api/examples/__init__.py +++ b/api/api/examples/__init__.py @@ -33,9 +33,7 @@ image_detail_200_example, image_detail_404_example, image_oembed_200_example, - image_oembed_404_example, image_related_200_example, - image_related_404_example, image_search_200_example, image_search_400_example, image_stats_200_example, diff --git a/api/api/examples/audio_requests.py b/api/api/examples/audio_requests.py index ed623adc85d..a3b7bd7c283 100644 --- a/api/api/examples/audio_requests.py +++ b/api/api/examples/audio_requests.py @@ -69,5 +69,6 @@ # Get the waveform of audio ID {identifier} curl \\ {auth} \\ + -H "Content-Type: application/json" \\ "{ORIGIN}/v1/audio/{identifier}/waveform/" """ diff --git a/api/api/examples/image_responses.py b/api/api/examples/image_responses.py index 4205eadabe1..cd0fb5bca43 100644 --- a/api/api/examples/image_responses.py +++ b/api/api/examples/image_responses.py @@ -121,8 +121,6 @@ ], } -image_related_404_example = {"detail": "An internal server error occurred."} - image_oembed_200_example = { "version": "1.0", "type": "photo", @@ -134,7 +132,6 @@ "license_url": "https://creativecommons.org/publicdomain/zero/1.0/", } -image_oembed_404_example = {"detail": "An internal server error occurred."} image_oembed_400_example = {"detail": {"url": ["Could not parse identifier from URL."]}} image_complain_201_example = { diff --git a/api/api/views/media_views.py b/api/api/views/media_views.py index c66870a79fa..50e651ee042 100644 --- a/api/api/views/media_views.py +++ b/api/api/views/media_views.py @@ -45,10 +45,7 @@ class MediaViewSet(AsyncViewSetMixin, AsyncAPIView, ReadOnlyModelViewSet): view_is_async = True lookup_field = "identifier" - # TODO: https://github.com/encode/django-rest-framework/pull/6789 - lookup_value_regex = ( - r"[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}" - ) + lookup_value_converter = "uuid" pagination_class = StandardPagination diff --git a/api/conf/settings/sentry.py b/api/conf/settings/sentry.py index 06d390a9b5b..1eb908dd297 100644 --- a/api/conf/settings/sentry.py +++ b/api/conf/settings/sentry.py @@ -4,6 +4,7 @@ from sentry_sdk.integrations.logging import LoggingIntegration, ignore_logger from conf.settings.base import ENVIRONMENT +from conf.settings.security import DEBUG SENTRY_DSN = config("SENTRY_DSN", default="") @@ -13,9 +14,6 @@ "SENTRY_PROFILES_SAMPLE_RATE", default=0, cast=float ) -# SECURITY WARNING: don't run with debug turned on in production! -DEBUG = config("DJANGO_DEBUG_ENABLED", default=False, cast=bool) - INTEGRATIONS = [ DjangoIntegration(), # This prevents two errors from being sent to Sentry (one with the correct diff --git a/api/conf/urls/__init__.py b/api/conf/urls/__init__.py index dd015e7a4a7..282e14e25b8 100644 --- a/api/conf/urls/__init__.py +++ b/api/conf/urls/__init__.py @@ -24,7 +24,7 @@ path("", include(deprecations_urlpatterns)), # Deprecated, redirects to new URL ] -router = SimpleRouter() +router = SimpleRouter(use_regex_path=False) router.register("audio", AudioViewSet, basename="audio") router.register("images", ImageViewSet, basename="image") versioned_paths += router.urls diff --git a/api/pdm.lock b/api/pdm.lock index a31f54debeb..97f661c30a1 100644 --- a/api/pdm.lock +++ b/api/pdm.lock @@ -1759,7 +1759,7 @@ files = [ [[package]] name = "schemathesis" -version = "3.38.9" +version = "3.38.10" requires_python = ">=3.8" summary = "Property-based testing framework for Open API and GraphQL based apps" groups = ["test"] @@ -1788,8 +1788,8 @@ dependencies = [ "yarl<2.0,>=1.5", ] files = [ - {file = "schemathesis-3.38.9-py3-none-any.whl", hash = "sha256:13a9a103cd1641891f933afb324583d69b6234f8aa050ec5af18a1e9692e73d7"}, - {file = "schemathesis-3.38.9.tar.gz", hash = "sha256:d652200f85abe64ebb417b1630c44300510bd3ded3936975b67951f37c49ee0f"}, + {file = "schemathesis-3.38.10-py3-none-any.whl", hash = "sha256:32e2aaabf57e4e8327f11c6e7b8dd2f6e41b436f3d4eae35c17109c770d358d4"}, + {file = "schemathesis-3.38.10.tar.gz", hash = "sha256:01cdd4f313d0b1a8625604cc3bf99ec09147a5668771c8e9bfc1f97990bba2da"}, ] [[package]] diff --git a/api/test/integration/test_media_integration.py b/api/test/integration/test_media_integration.py index 08646a11e45..56d0cec04ae 100644 --- a/api/test/integration/test_media_integration.py +++ b/api/test/integration/test_media_integration.py @@ -304,6 +304,7 @@ def test_search_refuses_invalid_categories( @pytest.mark.parametrize( "bad_uuid", [ + "000000000000000000000000000000000000", "123456789123456789123456789123456789", "12345678-1234-5678-1234-1234567891234", "abcd", @@ -383,6 +384,22 @@ def test_detail_view_contains_sensitivity_info(sensitive_result, api_client): ################ +@pytest.mark.parametrize( + "bad_uuid", + [ + "000000000000000000000000000000000000", + "123456789123456789123456789123456789", + "12345678-1234-5678-1234-1234567891234", + "abcd", + ], +) +def test_related_view_for_invalid_uuids_returns_not_found( + media_type_config: MediaTypeConfig, bad_uuid: str, api_client +): + res = api_client.get(f"/v1/{media_type_config.url_prefix}/{bad_uuid}/related/") + assert res.status_code == 404 + + def test_related_view_has_no_pagination(related_results): _, _, data = related_results results = data["results"] diff --git a/api/test/test_schema.py b/api/test/test_schema.py index 4951a57f5d4..4b2fe45fcad 100644 --- a/api/test/test_schema.py +++ b/api/test/test_schema.py @@ -3,11 +3,32 @@ from django.conf import settings import schemathesis +from schemathesis.checks import content_type_conformance schema = schemathesis.from_uri(f"{settings.CANONICAL_ORIGIN}/v1/schema/") +def status_code_aware_content_type_conformance(ctx, res, case): + """ + Skip Content-Type conformance check when status code is 404. + + A status code of 404 can come from two sources: + - Django sees the incoming URL and cannot map it to any endpoint. + This returns an HTML response. + - DRF gets the request but cannot map any object to the identifier. + This returns a content-negotiated HTML/JSON response. + + Since the end user will likely not be using the data in the 404 + response anyway, we don't need the Content-Type to be validated. + """ + + if res.status_code == 404: + return + + return content_type_conformance(ctx, res, case) + + # The null-bytes Bearer tokens are skipped. # The pattern identifies tests with headers that are acceptable, # by only allowing authorization headers that use characters valid for @@ -30,4 +51,7 @@ def test_schema(case: schemathesis.Case): # from schemathesis's implementation of `parameterize`. return - case.call_and_validate() + case.call_and_validate( + excluded_checks=[content_type_conformance], + additional_checks=[status_code_aware_content_type_conformance], + )