Skip to content

Commit

Permalink
Remove MediaViewSet's lookup regex (#5273)
Browse files Browse the repository at this point in the history
Co-authored-by: Dhruv Bhanushali <[email protected]>
  • Loading branch information
krysal and dhruvkb authored Dec 16, 2024
1 parent af38397 commit 3fa8f30
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 21 deletions.
1 change: 1 addition & 0 deletions api/api/docs/audio_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
201: (AudioReportRequestSerializer, audio_complain_201_example),
400: (ValidationError, None),
401: (AuthenticationFailed, None),
404: (NotFound, None),
},
eg=[audio_complain_curl],
)
Expand Down
7 changes: 3 additions & 4 deletions api/api/docs/image_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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],
)
Expand All @@ -119,6 +117,7 @@
201: (ImageReportRequestSerializer, image_complain_201_example),
401: (AuthenticationFailed, None),
400: (ValidationError, None),
404: (NotFound, None),
},
eg=[image_complain_curl],
)
Expand All @@ -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],
)
Expand Down
2 changes: 0 additions & 2 deletions api/api/examples/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions api/api/examples/audio_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,5 +69,6 @@
# Get the waveform of audio ID {identifier}
curl \\
{auth} \\
-H "Content-Type: application/json" \\
"{ORIGIN}/v1/audio/{identifier}/waveform/"
"""
3 changes: 0 additions & 3 deletions api/api/examples/image_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@
],
}

image_related_404_example = {"detail": "An internal server error occurred."}

image_oembed_200_example = {
"version": "1.0",
"type": "photo",
Expand All @@ -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 = {
Expand Down
5 changes: 1 addition & 4 deletions api/api/views/media_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 1 addition & 3 deletions api/conf/settings/sentry.py
Original file line number Diff line number Diff line change
Expand Up @@ -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="")
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion api/conf/urls/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions api/pdm.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions api/test/integration/test_media_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,7 @@ def test_search_refuses_invalid_categories(
@pytest.mark.parametrize(
"bad_uuid",
[
"000000000000000000000000000000000000",
"123456789123456789123456789123456789",
"12345678-1234-5678-1234-1234567891234",
"abcd",
Expand Down Expand Up @@ -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"]
Expand Down
26 changes: 25 additions & 1 deletion api/test/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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],
)

0 comments on commit 3fa8f30

Please sign in to comment.