Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove MediaViewSet's lookup regex #5273

Merged
merged 5 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -46,10 +46,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],
)
Loading