From 18231cce25b42038bdd82a0493c7e3551b3a76a4 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 11:45:19 +0530 Subject: [PATCH 01/18] Move Swagger documentation from URLconf to the endpoint --- .../catalog/api/examples/__init__.py | 1 + .../catalog/api/examples/image_requests.py | 5 ++ .../catalog/api/views/image_views.py | 39 +++++++++---- .../catalog/{urls.py => urls/__init__.py} | 57 ------------------- 4 files changed, 35 insertions(+), 67 deletions(-) rename openverse-api/catalog/{urls.py => urls/__init__.py} (83%) diff --git a/openverse-api/catalog/api/examples/__init__.py b/openverse-api/catalog/api/examples/__init__.py index fd9b16c52..8b2acc008 100644 --- a/openverse-api/catalog/api/examples/__init__.py +++ b/openverse-api/catalog/api/examples/__init__.py @@ -18,6 +18,7 @@ recommendations_images_read_curl, image_detail_curl, image_stats_curl, + report_image_curl, ) from catalog.api.examples.image_responses import ( image_search_200_example, diff --git a/openverse-api/catalog/api/examples/image_requests.py b/openverse-api/catalog/api/examples/image_requests.py index eaa2a8306..aa7d7236b 100644 --- a/openverse-api/catalog/api/examples/image_requests.py +++ b/openverse-api/catalog/api/examples/image_requests.py @@ -41,3 +41,8 @@ # Get the statistics for image sources curl -H "Authorization: Bearer DLBYIcfnKfolaXKcmMC8RIDCavc2hW" http://api.openverse.engineering/v1/images/stats """ # noqa + +report_image_curl = """ +# Report an issue about image ID 7c829a03-fb24-4b57-9b03-65f43ed19395 +curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer DLBYIcfnKfolaXKcmMC8RIDCavc2hW" -d '{"reason": "mature", "identifier": "7c829a03-fb24-4b57-9b03-65f43ed19395", "description": "This image contains sensitive content"}' https://api.openverse.engineering/v1/images/7c829a03-fb24-4b57-9b03-65f43ed19395/report +""" # noqa \ No newline at end of file diff --git a/openverse-api/catalog/api/views/image_views.py b/openverse-api/catalog/api/views/image_views.py index f148fbee9..e17b862c5 100644 --- a/openverse-api/catalog/api/views/image_views.py +++ b/openverse-api/catalog/api/views/image_views.py @@ -29,6 +29,8 @@ oembed_list_404_example, image_stats_curl, image_stats_200_example, + report_image_curl, + images_report_create_201_example, ) from catalog.api.models import Image, ImageReport from catalog.api.serializers.error_serializers import ( @@ -49,6 +51,7 @@ from catalog.api.utils.exceptions import input_error_response from catalog.api.utils.watermark import watermark from catalog.api.views.media_views import ( + refer_sample, RESULTS, RESULT_COUNT, PAGE_COUNT, @@ -367,22 +370,38 @@ def get(self, request): class ReportImageView(CreateAPIView): - """ - images_report_create - - images_report_create is an API endpoint to report an issue about a - specified image ID to Openverse. + report_image_description = f""" +images_report_create is an API endpoint to report an issue about a specified +image ID to Openverse. + +By using this endpoint, you can report an image if it infringes copyright, +contains mature or sensitive content and others. - By using this endpoint, you can report an image if it infringes copyright, - contains mature or sensitive content and others. +{refer_sample}""" # noqa - You can refer to Bash's Request Samples for example on how to use - this endpoint. - """ # noqa swagger_schema = CustomAutoSchema queryset = ImageReport.objects.all() serializer_class = ReportImageSerializer + @swagger_auto_schema(operation_id='images_report_create', + operation_description=report_image_description, + query_serializer=ReportImageSerializer, + responses={ + "201": openapi.Response( + description="OK", + examples=images_report_create_201_example, + schema=ReportImageSerializer + ) + }, + code_examples=[ + { + 'lang': 'Bash', + 'source': report_image_curl, + } + ]) + def post(self, request, *args, **kwargs): + return super(ReportImageView, self).post(request, *args, **kwargs) + class ImageStats(MediaStats): image_stats_description = f""" diff --git a/openverse-api/catalog/urls.py b/openverse-api/catalog/urls/__init__.py similarity index 83% rename from openverse-api/catalog/urls.py rename to openverse-api/catalog/urls/__init__.py index ff2c9d13e..10d5a52b6 100644 --- a/openverse-api/catalog/urls.py +++ b/openverse-api/catalog/urls/__init__.py @@ -190,63 +190,6 @@ permission_classes=(rest_framework.permissions.AllowAny,), ) -report_image_bash = \ - """ - # Report an issue about image ID (7c829a03-fb24-4b57-9b03-65f43ed19395) - curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer DLBYIcfnKfolaXKcmMC8RIDCavc2hW" -d '{"reason": "mature", "identifier": "7c829a03-fb24-4b57-9b03-65f43ed19395", "description": "This image contains sensitive content"}' https://api.openverse.engineering/v1/images/7c829a03-fb24-4b57-9b03-65f43ed19395/report - """ # noqa - -report_image_request = openapi.Schema( - type=openapi.TYPE_OBJECT, - required=['reason', 'identifier'], - properties={ - 'reason': openapi.Schema( - title="Reason", - type=openapi.TYPE_STRING, - enum=["mature", "dmca", "other"], - max_length=20, - description="The reason to report image to Openverse." - ), - 'identifier': openapi.Schema( - title="Identifier", - type=openapi.TYPE_STRING, - format=openapi.FORMAT_UUID, - description="The ID for image to be reported." - ), - 'description': openapi.Schema( - title="Description", - type=openapi.TYPE_STRING, - max_length=500, - nullable=True, - description="The explanation on why image is being reported." - ) - }, - example={ - "reason": "mature", - "identifier": "7c829a03-fb24-4b57-9b03-65f43ed19395", - "description": "This image contains sensitive content" - } -) - -decorated_report_image_view = \ - swagger_auto_schema( - method='post', - responses={ - "201": openapi.Response( - description="OK", - examples=images_report_create_201_example, - schema=ReportImageSerializer - ) - }, - request_body=report_image_request, - code_examples=[ - { - 'lang': 'Bash', - 'source': report_image_bash - } - ] - )(ReportImageView.as_view()) - versioned_paths = [ path('', schema_view.with_ui('redoc', cache_timeout=None), name='root'), path('auth_tokens/register', Register.as_view(), name='register'), From 68cbd29b59dd347757cab8479eec31fed360bafa Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 11:47:24 +0530 Subject: [PATCH 02/18] Use Django `settings` module properly --- openverse-api/catalog/urls/__init__.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 10d5a52b6..6ed6fa150 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -14,6 +14,7 @@ 2. Add a URL to urlpatterns: path('blog/', include('blog.urls')) """ import rest_framework.permissions +from django.conf import settings from django.conf.urls import include from django.contrib import admin from django.urls import path, re_path @@ -176,7 +177,7 @@ schema_view = get_schema_view( openapi.Info( title="Openverse API", - default_version=API_VERSION, + default_version=settings.API_VERSION, description=description, contact=openapi.Contact(email="zack.krida@automattic.com"), license=openapi.License(name="MIT License", url=license_url), @@ -252,7 +253,7 @@ path('thumbs/', ProxiedImage.as_view(), name='thumbs'), path('oembed', OembedView.as_view(), name='oembed') ] -if WATERMARK_ENABLED: +if settings.WATERMARK_ENABLED: versioned_paths.append( path('watermark/', Watermark.as_view()) ) From 02a40ac89df32371448a3accb0a9d24ea0189ee4 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 11:49:18 +0530 Subject: [PATCH 03/18] Disable documentation caching for dev environments --- openverse-api/catalog/urls/__init__.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 6ed6fa150..c2b897791 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -191,6 +191,8 @@ permission_classes=(rest_framework.permissions.AllowAny,), ) +cache_timeout = 0 if settings.DEBUG else 15 + versioned_paths = [ path('', schema_view.with_ui('redoc', cache_timeout=None), name='root'), path('auth_tokens/register', Register.as_view(), name='register'), @@ -262,19 +264,29 @@ path('', RedirectView.as_view(url='/v1')), path('admin/', admin.site.urls), re_path('healthcheck', HealthCheck.as_view()), + + # Swagger documentation re_path( r'^swagger(?P\.json|\.yaml)$', - schema_view.without_ui(cache_timeout=None), name='schema-json' + schema_view.without_ui(cache_timeout=None), + name='schema-json' ), re_path( r'^swagger/$', - schema_view.with_ui('swagger', cache_timeout=15), + schema_view.with_ui('swagger', cache_timeout=cache_timeout), name='schema-swagger-ui' ), re_path( r'^redoc/$', - schema_view.with_ui('redoc', cache_timeout=15), + schema_view.with_ui('redoc', cache_timeout=cache_timeout), name='schema-redoc' ), + re_path( + r'^v1/$', + schema_view.with_ui('redoc', cache_timeout=cache_timeout), + name='root' + ), + + # API path('v1/', include(versioned_paths)) ] From 4a40dba37600f89107814b3ea06302124f859222 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 11:49:58 +0530 Subject: [PATCH 04/18] Replace hardcoded path with name resolution --- openverse-api/catalog/urls/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index c2b897791..7f2ef162b 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -261,7 +261,7 @@ ) urlpatterns = [ - path('', RedirectView.as_view(url='/v1')), + path('', RedirectView.as_view(pattern_name='root')), path('admin/', admin.site.urls), re_path('healthcheck', HealthCheck.as_view()), From 2023e445a35b014ade0f6b3cc69ad75f89ba2321 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 11:51:03 +0530 Subject: [PATCH 05/18] Break URL config into smaller submodules --- openverse-api/catalog/urls/__init__.py | 76 +++++------------------ openverse-api/catalog/urls/audio.py | 31 +++++++++ openverse-api/catalog/urls/auth_tokens.py | 23 +++++++ openverse-api/catalog/urls/images.py | 37 +++++++++++ 4 files changed, 106 insertions(+), 61 deletions(-) create mode 100644 openverse-api/catalog/urls/audio.py create mode 100644 openverse-api/catalog/urls/auth_tokens.py create mode 100644 openverse-api/catalog/urls/images.py diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 7f2ef162b..68c6d4586 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -20,21 +20,16 @@ from django.urls import path, re_path from django.views.generic import RedirectView from drf_yasg import openapi -from drf_yasg.utils import swagger_auto_schema from drf_yasg.views import get_schema_view -from catalog.api.examples import images_report_create_201_example -from catalog.api.serializers.image_serializers import \ - ReportImageSerializer -from catalog.api.views.audio_views import SearchAudio, AudioDetail, \ - RelatedAudio, AudioStats -from catalog.api.views.image_views import SearchImages, ImageDetail, \ - Watermark, RelatedImage, OembedView, ReportImageView, ImageStats +from catalog.api.views.image_views import Watermark, OembedView from catalog.api.views.link_views import CreateShortenedLink, \ ResolveShortenedLink -from catalog.api.views.site_views import HealthCheck, Register, CheckRates, \ - VerifyEmail, ProxiedImage -from catalog.settings import API_VERSION, WATERMARK_ENABLED +from catalog.api.views.site_views import HealthCheck, CheckRates, ProxiedImage + +from catalog.urls.auth_tokens import urlpatterns as auth_tokens_patterns +from catalog.urls.audio import urlpatterns as audio_patterns +from catalog.urls.images import urlpatterns as images_patterns description = """ # Introduction @@ -194,66 +189,25 @@ cache_timeout = 0 if settings.DEBUG else 15 versioned_paths = [ - path('', schema_view.with_ui('redoc', cache_timeout=None), name='root'), - path('auth_tokens/register', Register.as_view(), name='register'), path('rate_limit', CheckRates.as_view(), name='key_info'), - path( - 'auth_tokens/verify/', - VerifyEmail.as_view(), - name='verify-email' - ), - re_path( - r'auth_tokens/', - include('oauth2_provider.urls', namespace='oauth2_provider') - ), + path('auth_tokens/', include(auth_tokens_patterns)), - path( - 'audio/stats', - AudioStats.as_view(), - name='audio-stats' - ), - path( - 'audio/', - AudioDetail.as_view(), - name='audio-detail' - ), - re_path('audio', SearchAudio.as_view(), name='audio'), - path( - 'recommendations/audio/', - RelatedAudio.as_view(), - name='related-audio' - ), + # Audio + path('audio/', include(audio_patterns)), + # Images + path('images/', include(images_patterns)), + path('thumbs/', ProxiedImage.as_view(), name='thumbs'), + path('oembed', OembedView.as_view(), name='oembed'), + + # Deprecated path( - 'images/stats', - ImageStats.as_view(), - name='image-stats' - ), - path( - 'images/', - ImageDetail.as_view(), - name='image-detail' - ), - path( - 'images//report', - decorated_report_image_view, - name='report-image' - ), - path( - 'recommendations/images/', - RelatedImage.as_view(), - name='related-images' - ), - re_path('images', SearchImages.as_view(), name='images'), - path( # Deprecated 'sources', RedirectView.as_view(pattern_name='image-stats', permanent=True), name='about-image' ), path('link', CreateShortenedLink.as_view(), name='make-link'), path('link/', ResolveShortenedLink.as_view(), name='resolve'), - path('thumbs/', ProxiedImage.as_view(), name='thumbs'), - path('oembed', OembedView.as_view(), name='oembed') ] if settings.WATERMARK_ENABLED: versioned_paths.append( diff --git a/openverse-api/catalog/urls/audio.py b/openverse-api/catalog/urls/audio.py new file mode 100644 index 000000000..e6c4d7eaf --- /dev/null +++ b/openverse-api/catalog/urls/audio.py @@ -0,0 +1,31 @@ +from django.urls import path + +from catalog.api.views.audio_views import ( + SearchAudio, + AudioDetail, + RelatedAudio, + AudioStats, +) + +urlpatterns = [ + path( + 'stats', + AudioStats.as_view(), + name='audio-stats' + ), + path( + 'recommendations/', + RelatedAudio.as_view(), + name='related-audio' + ), + path( + '', + AudioDetail.as_view(), + name='audio-detail' + ), + path( + '', + SearchAudio.as_view(), + name='audio' + ), +] diff --git a/openverse-api/catalog/urls/auth_tokens.py b/openverse-api/catalog/urls/auth_tokens.py new file mode 100644 index 000000000..34518f5e8 --- /dev/null +++ b/openverse-api/catalog/urls/auth_tokens.py @@ -0,0 +1,23 @@ +from django.urls import path, include + +from catalog.api.views.site_views import ( + Register, + VerifyEmail, +) + +urlpatterns = [ + path( + 'register', + Register.as_view(), + name='register' + ), + path( + 'verify/', + VerifyEmail.as_view(), + name='verify-email' + ), + path( + '', + include('oauth2_provider.urls', namespace='oauth2_provider') + ), +] \ No newline at end of file diff --git a/openverse-api/catalog/urls/images.py b/openverse-api/catalog/urls/images.py new file mode 100644 index 000000000..38496563a --- /dev/null +++ b/openverse-api/catalog/urls/images.py @@ -0,0 +1,37 @@ +from django.urls import path + +from catalog.api.views.image_views import ( + SearchImages, + ImageDetail, + RelatedImage, + ImageStats, + ReportImageView, +) + +urlpatterns = [ + path( + 'stats', + ImageStats.as_view(), + name='image-stats' + ), + path( + 'recommendations/', + RelatedImage.as_view(), + name='related-images' + ), + path( + '', + ImageDetail.as_view(), + name='image-detail' + ), + path( + '/report', + ReportImageView.as_view(), + name='report-image' + ), + path( + '', + SearchImages.as_view(), + name='images' + ), +] From 0f544baf8e88ad80a8fcd5ea199e81da9a742ec1 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 11:51:30 +0530 Subject: [PATCH 06/18] Replace `re_path` with `path` unless necessary --- openverse-api/catalog/urls/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 68c6d4586..41b1a9ae3 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -217,7 +217,7 @@ urlpatterns = [ path('', RedirectView.as_view(pattern_name='root')), path('admin/', admin.site.urls), - re_path('healthcheck', HealthCheck.as_view()), + path('healthcheck', HealthCheck.as_view()), # Swagger documentation re_path( @@ -242,5 +242,5 @@ ), # API - path('v1/', include(versioned_paths)) + path('v1/', include(versioned_paths)), ] From b8044fff9044602322b9b6ef84495ccd06942610 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 12:21:16 +0530 Subject: [PATCH 07/18] Update path names to match new URL pattern --- openverse-api/catalog/api/serializers/audio_serializers.py | 2 +- openverse-api/catalog/api/serializers/image_serializers.py | 2 +- openverse-api/catalog/urls/audio.py | 2 +- openverse-api/catalog/urls/images.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openverse-api/catalog/api/serializers/audio_serializers.py b/openverse-api/catalog/api/serializers/audio_serializers.py index 1dd6d9410..782c4bc80 100644 --- a/openverse-api/catalog/api/serializers/audio_serializers.py +++ b/openverse-api/catalog/api/serializers/audio_serializers.py @@ -135,7 +135,7 @@ class AudioSerializer(MediaSerializer): help_text="A direct link to the detail view of this audio file." ) related_url = serializers.HyperlinkedIdentityField( - view_name='related-audio', + view_name='audio-related', lookup_field='identifier', read_only=True, help_text="A link to an endpoint that provides similar audio files." diff --git a/openverse-api/catalog/api/serializers/image_serializers.py b/openverse-api/catalog/api/serializers/image_serializers.py index c5929616b..780565785 100644 --- a/openverse-api/catalog/api/serializers/image_serializers.py +++ b/openverse-api/catalog/api/serializers/image_serializers.py @@ -117,7 +117,7 @@ class ImageSerializer(MediaSerializer): help_text="A direct link to the detail view of this image." ) related_url = serializers.HyperlinkedIdentityField( - view_name='related-images', + view_name='image-related', lookup_field='identifier', read_only=True, help_text="A link to an endpoint that provides similar images." diff --git a/openverse-api/catalog/urls/audio.py b/openverse-api/catalog/urls/audio.py index e6c4d7eaf..28f81d768 100644 --- a/openverse-api/catalog/urls/audio.py +++ b/openverse-api/catalog/urls/audio.py @@ -16,7 +16,7 @@ path( 'recommendations/', RelatedAudio.as_view(), - name='related-audio' + name='audio-related' ), path( '', diff --git a/openverse-api/catalog/urls/images.py b/openverse-api/catalog/urls/images.py index 38496563a..8517ea075 100644 --- a/openverse-api/catalog/urls/images.py +++ b/openverse-api/catalog/urls/images.py @@ -17,7 +17,7 @@ path( 'recommendations/', RelatedImage.as_view(), - name='related-images' + name='image-related' ), path( '', From e06059c38668d33484b6bf10d92bff24b7584328 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 12:21:30 +0530 Subject: [PATCH 08/18] Add backwards compatible redirects --- openverse-api/catalog/urls/__init__.py | 5 +++++ openverse-api/test/v1_integration_test.py | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 41b1a9ae3..5f4aefcb4 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -206,6 +206,11 @@ RedirectView.as_view(pattern_name='image-stats', permanent=True), name='about-image' ), + path( + 'recommendations/images/', + RedirectView.as_view(pattern_name='image-related', permanent=True), + name='related-images', + ), path('link', CreateShortenedLink.as_view(), name='make-link'), path('link/', ResolveShortenedLink.as_view(), name='resolve'), ] diff --git a/openverse-api/test/v1_integration_test.py b/openverse-api/test/v1_integration_test.py index e84dd4f8e..226ed0e36 100644 --- a/openverse-api/test/v1_integration_test.py +++ b/openverse-api/test/v1_integration_test.py @@ -67,6 +67,17 @@ def test_old_stats_endpoint(): assert response.headers.get('Location') == '/v1/images/stats' +def test_old_related_images_endpoint(): + response = requests.get( + f'{API_URL}/v1/recommendations/images/xyz', + allow_redirects=False, + verify=False + ) + assert response.status_code == 301 + assert response.is_permanent_redirect + assert response.headers.get('Location') == '/v1/images/recommendations/xyz' + + @pytest.mark.skip(reason="Disabled feature") @pytest.fixture def test_list_create(image_fixture): From 995d925b4dfcf03a249b2c8e3baf824153d995e1 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 12:33:07 +0530 Subject: [PATCH 09/18] Make recommendations view a subpath of detail view --- openverse-api/catalog/urls/audio.py | 10 +++++----- openverse-api/catalog/urls/images.py | 10 +++++----- openverse-api/test/v1_integration_test.py | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/openverse-api/catalog/urls/audio.py b/openverse-api/catalog/urls/audio.py index 28f81d768..1f2893e38 100644 --- a/openverse-api/catalog/urls/audio.py +++ b/openverse-api/catalog/urls/audio.py @@ -13,16 +13,16 @@ AudioStats.as_view(), name='audio-stats' ), - path( - 'recommendations/', - RelatedAudio.as_view(), - name='audio-related' - ), path( '', AudioDetail.as_view(), name='audio-detail' ), + path( + '/recommendations', + RelatedAudio.as_view(), + name='audio-related' + ), path( '', SearchAudio.as_view(), diff --git a/openverse-api/catalog/urls/images.py b/openverse-api/catalog/urls/images.py index 8517ea075..6e43d4754 100644 --- a/openverse-api/catalog/urls/images.py +++ b/openverse-api/catalog/urls/images.py @@ -14,16 +14,16 @@ ImageStats.as_view(), name='image-stats' ), - path( - 'recommendations/', - RelatedImage.as_view(), - name='image-related' - ), path( '', ImageDetail.as_view(), name='image-detail' ), + path( + '/recommendations', + RelatedImage.as_view(), + name='image-related' + ), path( '/report', ReportImageView.as_view(), diff --git a/openverse-api/test/v1_integration_test.py b/openverse-api/test/v1_integration_test.py index 226ed0e36..48da62659 100644 --- a/openverse-api/test/v1_integration_test.py +++ b/openverse-api/test/v1_integration_test.py @@ -75,7 +75,7 @@ def test_old_related_images_endpoint(): ) assert response.status_code == 301 assert response.is_permanent_redirect - assert response.headers.get('Location') == '/v1/images/recommendations/xyz' + assert response.headers.get('Location') == '/v1/images/xyz/recommendations' @pytest.mark.skip(reason="Disabled feature") From 25089d3dfff79aeea37cc38070afc57a3b249b64 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 12:50:44 +0530 Subject: [PATCH 10/18] Place `ProxiedImage` with other image endpoints --- .../catalog/api/views/image_views.py | 45 ++++++++++++++++ openverse-api/catalog/api/views/site_views.py | 54 +------------------ 2 files changed, 47 insertions(+), 52 deletions(-) diff --git a/openverse-api/catalog/api/views/image_views.py b/openverse-api/catalog/api/views/image_views.py index e17b862c5..ce6d6d3e0 100644 --- a/openverse-api/catalog/api/views/image_views.py +++ b/openverse-api/catalog/api/views/image_views.py @@ -5,6 +5,7 @@ import piexif import requests from PIL import Image as img +from django.conf import settings from django.http.response import HttpResponse, FileResponse from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema @@ -13,6 +14,8 @@ from rest_framework.response import Response from rest_framework.reverse import reverse from rest_framework.views import APIView +from urllib.error import HTTPError +from urllib.request import urlopen import catalog.api.controllers.search_controller as search_controller from catalog.api.examples import ( @@ -46,8 +49,10 @@ OembedSerializer, OembedResponseSerializer, AboutImageSerializer, + ProxiedImageSerializer, ) from catalog.api.utils import ccrel +from catalog.api.utils.throttle import OneThousandPerMinute from catalog.api.utils.exceptions import input_error_response from catalog.api.utils.watermark import watermark from catalog.api.views.media_views import ( @@ -429,3 +434,43 @@ class ImageStats(MediaStats): ]) def get(self, request, format=None): return self._get(request, 'image') + + +class ProxiedImage(APIView): + """ + Return the thumb of an image. + """ + + lookup_field = 'identifier' + queryset = Image.objects.all() + throttle_classes = [OneThousandPerMinute] + swagger_schema = None + + def get(self, request, identifier, format=None): + serialized = ProxiedImageSerializer(data=request.data) + serialized.is_valid() + try: + image = Image.objects.get(identifier=identifier) + except Image.DoesNotExist: + return Response(status=404, data='Not Found') + + if serialized.data['full_size']: + proxy_upstream = f'{settings.THUMBNAIL_PROXY_URL}/{image.url}' + else: + proxy_upstream = f'{settings.THUMBNAIL_PROXY_URL}/{settings.THUMBNAIL_WIDTH_PX}' \ + f',fit/{image.url}' + try: + upstream_response = urlopen(proxy_upstream) + status = upstream_response.status + content_type = upstream_response.headers.get('Content-Type') + except HTTPError: + log.info('Failed to render thumbnail: ', exc_info=True) + return HttpResponse(status=500) + + response = HttpResponse( + upstream_response.read(), + status=status, + content_type=content_type + ) + + return response diff --git a/openverse-api/catalog/api/views/site_views.py b/openverse-api/catalog/api/views/site_views.py index dcc581727..246615a2e 100644 --- a/openverse-api/catalog/api/views/site_views.py +++ b/openverse-api/catalog/api/views/site_views.py @@ -1,8 +1,6 @@ import logging as log import secrets import smtplib -from urllib.error import HTTPError -from urllib.request import urlopen from django.core.mail import send_mail from rest_framework.response import Response from rest_framework.reverse import reverse @@ -14,20 +12,11 @@ ForbiddenErrorSerializer, InternalServerErrorSerializer, ) -from catalog.api.serializers.image_serializers import ProxiedImageSerializer from drf_yasg.utils import swagger_auto_schema -from catalog.api.models import ( - Image, - ThrottledApplication, - OAuth2Verification, -) -from catalog.api.utils.throttle import ( - TenPerDay, OnePerSecond, OneThousandPerMinute -) +from catalog.api.models import ThrottledApplication, OAuth2Verification +from catalog.api.utils.throttle import TenPerDay, OnePerSecond from catalog.api.utils.oauth2_helper import get_token_info -from catalog.settings import THUMBNAIL_PROXY_URL, THUMBNAIL_WIDTH_PX from django.core.cache import cache -from django.http import HttpResponse from drf_yasg import openapi from catalog.example_responses import ( register_api_oauth2_201_example, @@ -328,42 +317,3 @@ def get(self, request, format=None): } return Response(status=200, data=response_data) - -class ProxiedImage(APIView): - """ - Return the thumb of an image. - """ - - lookup_field = 'identifier' - queryset = Image.objects.all() - throttle_classes = [OneThousandPerMinute] - swagger_schema = None - - def get(self, request, identifier, format=None): - serialized = ProxiedImageSerializer(data=request.data) - serialized.is_valid() - try: - image = Image.objects.get(identifier=identifier) - except Image.DoesNotExist: - return Response(status=404, data='Not Found') - - if serialized.data['full_size']: - proxy_upstream = f'{THUMBNAIL_PROXY_URL}/{image.url}' - else: - proxy_upstream = f'{THUMBNAIL_PROXY_URL}/{THUMBNAIL_WIDTH_PX}'\ - f',fit/{image.url}' - try: - upstream_response = urlopen(proxy_upstream) - status = upstream_response.status - content_type = upstream_response.headers.get('Content-Type') - except HTTPError: - log.info('Failed to render thumbnail: ', exc_info=True) - return HttpResponse(status=500) - - response = HttpResponse( - upstream_response.read(), - status=status, - content_type=content_type - ) - - return response From eb3956e432b8b0c9ce21a80057753dbc0c8d4af6 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 12:51:21 +0530 Subject: [PATCH 11/18] Add '/thumb' as a subpath of the image detail view --- openverse-api/catalog/api/serializers/image_serializers.py | 2 +- openverse-api/catalog/api/views/image_views.py | 4 ++-- openverse-api/catalog/urls/images.py | 6 ++++++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/openverse-api/catalog/api/serializers/image_serializers.py b/openverse-api/catalog/api/serializers/image_serializers.py index 780565785..b4a9380a6 100644 --- a/openverse-api/catalog/api/serializers/image_serializers.py +++ b/openverse-api/catalog/api/serializers/image_serializers.py @@ -126,7 +126,7 @@ class ImageSerializer(MediaSerializer): def get_thumbnail(self, obj): request = self.context['request'] host = request.get_host() - path = reverse('thumbs', kwargs={'identifier': obj.identifier}) + path = reverse('image-thumb', kwargs={'identifier': obj.identifier}) return f'https://{host}{path}' diff --git a/openverse-api/catalog/api/views/image_views.py b/openverse-api/catalog/api/views/image_views.py index ce6d6d3e0..5b8734961 100644 --- a/openverse-api/catalog/api/views/image_views.py +++ b/openverse-api/catalog/api/views/image_views.py @@ -6,13 +6,13 @@ import requests from PIL import Image as img from django.conf import settings +from django.urls import reverse from django.http.response import HttpResponse, FileResponse from drf_yasg import openapi from drf_yasg.utils import swagger_auto_schema from rest_framework import status from rest_framework.generics import GenericAPIView, CreateAPIView from rest_framework.response import Response -from rest_framework.reverse import reverse from rest_framework.views import APIView from urllib.error import HTTPError from urllib.request import urlopen @@ -220,7 +220,7 @@ def get(self, request, identifier, format=None): # Proxy insecure HTTP images at full resolution. if 'http://' in resp.data[search_controller.URL]: secure = request.build_absolute_uri( - reverse('thumbs', [identifier]) + reverse('image-thumb', kwargs={'identifier': identifier}) ) secure += '?full_size=True' resp.data[search_controller.URL] = secure diff --git a/openverse-api/catalog/urls/images.py b/openverse-api/catalog/urls/images.py index 6e43d4754..eac870e12 100644 --- a/openverse-api/catalog/urls/images.py +++ b/openverse-api/catalog/urls/images.py @@ -6,6 +6,7 @@ RelatedImage, ImageStats, ReportImageView, + ProxiedImage, ) urlpatterns = [ @@ -19,6 +20,11 @@ ImageDetail.as_view(), name='image-detail' ), + path( + '/thumb', + ProxiedImage.as_view(), + name='image-thumb' + ), path( '/recommendations', RelatedImage.as_view(), From f58ef8b00b21c635778582da602b2bc0401bf478 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 12:53:12 +0530 Subject: [PATCH 12/18] Deprecate the old thumbnail endpoint --- openverse-api/catalog/urls/__init__.py | 8 ++++++-- openverse-api/test/v1_integration_test.py | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 5f4aefcb4..c4e9b7ace 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -25,7 +25,7 @@ from catalog.api.views.image_views import Watermark, OembedView from catalog.api.views.link_views import CreateShortenedLink, \ ResolveShortenedLink -from catalog.api.views.site_views import HealthCheck, CheckRates, ProxiedImage +from catalog.api.views.site_views import HealthCheck, CheckRates from catalog.urls.auth_tokens import urlpatterns as auth_tokens_patterns from catalog.urls.audio import urlpatterns as audio_patterns @@ -197,7 +197,6 @@ # Images path('images/', include(images_patterns)), - path('thumbs/', ProxiedImage.as_view(), name='thumbs'), path('oembed', OembedView.as_view(), name='oembed'), # Deprecated @@ -211,6 +210,11 @@ RedirectView.as_view(pattern_name='image-related', permanent=True), name='related-images', ), + path( + 'thumbs/', + RedirectView.as_view(pattern_name='image-thumb', permanent=True), + name='thumbs' + ), path('link', CreateShortenedLink.as_view(), name='make-link'), path('link/', ResolveShortenedLink.as_view(), name='resolve'), ] diff --git a/openverse-api/test/v1_integration_test.py b/openverse-api/test/v1_integration_test.py index 48da62659..5232ddc0a 100644 --- a/openverse-api/test/v1_integration_test.py +++ b/openverse-api/test/v1_integration_test.py @@ -78,6 +78,17 @@ def test_old_related_images_endpoint(): assert response.headers.get('Location') == '/v1/images/xyz/recommendations' +def test_old_thumbs_endpoint(): + response = requests.get( + f'{API_URL}/v1/thumbs/xyz', + allow_redirects=False, + verify=False + ) + assert response.status_code == 301 + assert response.is_permanent_redirect + assert response.headers.get('Location') == '/v1/images/xyz/thumb' + + @pytest.mark.skip(reason="Disabled feature") @pytest.fixture def test_list_create(image_fixture): From 0a682bffb757ddb5ec1948e3256630486a725cf2 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 13:07:03 +0530 Subject: [PATCH 13/18] Separate tests for backwards compatibility into their own suite --- openverse-api/test/backwards_compat_test.py | 44 +++++++++++++++++++++ openverse-api/test/v1_integration_test.py | 33 ---------------- 2 files changed, 44 insertions(+), 33 deletions(-) create mode 100644 openverse-api/test/backwards_compat_test.py diff --git a/openverse-api/test/backwards_compat_test.py b/openverse-api/test/backwards_compat_test.py new file mode 100644 index 000000000..fe7d3c791 --- /dev/null +++ b/openverse-api/test/backwards_compat_test.py @@ -0,0 +1,44 @@ +""" +This file ensures that deprecated URLs are redirected to their updated paths and +not left to rot. + +Can be used to verify a live deployment is functioning as designed. +Run with the `pytest -s` command from this directory. +""" + +import requests + +from test.constants import API_URL + + +def test_old_stats_endpoint(): + response = requests.get( + f'{API_URL}/v1/sources?type=images', + allow_redirects=False, + verify=False + ) + assert response.status_code == 301 + assert response.is_permanent_redirect + assert response.headers.get('Location') == '/v1/images/stats' + + +def test_old_related_images_endpoint(): + response = requests.get( + f'{API_URL}/v1/recommendations/images/xyz', + allow_redirects=False, + verify=False + ) + assert response.status_code == 301 + assert response.is_permanent_redirect + assert response.headers.get('Location') == '/v1/images/xyz/recommendations' + + +def test_old_thumbs_endpoint(): + response = requests.get( + f'{API_URL}/v1/thumbs/xyz', + allow_redirects=False, + verify=False + ) + assert response.status_code == 301 + assert response.is_permanent_redirect + assert response.headers.get('Location') == '/v1/images/xyz/thumb' \ No newline at end of file diff --git a/openverse-api/test/v1_integration_test.py b/openverse-api/test/v1_integration_test.py index 5232ddc0a..f137567b6 100644 --- a/openverse-api/test/v1_integration_test.py +++ b/openverse-api/test/v1_integration_test.py @@ -56,39 +56,6 @@ def test_link_shortener_resolve(link_shortener_fixture): assert response.status_code == 301 -def test_old_stats_endpoint(): - response = requests.get( - f'{API_URL}/v1/sources?type=images', - allow_redirects=False, - verify=False - ) - assert response.status_code == 301 - assert response.is_permanent_redirect - assert response.headers.get('Location') == '/v1/images/stats' - - -def test_old_related_images_endpoint(): - response = requests.get( - f'{API_URL}/v1/recommendations/images/xyz', - allow_redirects=False, - verify=False - ) - assert response.status_code == 301 - assert response.is_permanent_redirect - assert response.headers.get('Location') == '/v1/images/xyz/recommendations' - - -def test_old_thumbs_endpoint(): - response = requests.get( - f'{API_URL}/v1/thumbs/xyz', - allow_redirects=False, - verify=False - ) - assert response.status_code == 301 - assert response.is_permanent_redirect - assert response.headers.get('Location') == '/v1/images/xyz/thumb' - - @pytest.mark.skip(reason="Disabled feature") @pytest.fixture def test_list_create(image_fixture): From 0f3a1af6fa704e0714a2651d288de1256871d3ee Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 14:26:33 +0530 Subject: [PATCH 14/18] Fix code style problems caught during lint --- openverse-api/catalog/api/examples/image_requests.py | 2 +- openverse-api/catalog/api/views/image_views.py | 3 ++- openverse-api/catalog/api/views/site_views.py | 1 - openverse-api/catalog/urls/auth_tokens.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/openverse-api/catalog/api/examples/image_requests.py b/openverse-api/catalog/api/examples/image_requests.py index aa7d7236b..2cb651fd0 100644 --- a/openverse-api/catalog/api/examples/image_requests.py +++ b/openverse-api/catalog/api/examples/image_requests.py @@ -45,4 +45,4 @@ report_image_curl = """ # Report an issue about image ID 7c829a03-fb24-4b57-9b03-65f43ed19395 curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer DLBYIcfnKfolaXKcmMC8RIDCavc2hW" -d '{"reason": "mature", "identifier": "7c829a03-fb24-4b57-9b03-65f43ed19395", "description": "This image contains sensitive content"}' https://api.openverse.engineering/v1/images/7c829a03-fb24-4b57-9b03-65f43ed19395/report -""" # noqa \ No newline at end of file +""" # noqa diff --git a/openverse-api/catalog/api/views/image_views.py b/openverse-api/catalog/api/views/image_views.py index 5b8734961..e017289cb 100644 --- a/openverse-api/catalog/api/views/image_views.py +++ b/openverse-api/catalog/api/views/image_views.py @@ -457,7 +457,8 @@ def get(self, request, identifier, format=None): if serialized.data['full_size']: proxy_upstream = f'{settings.THUMBNAIL_PROXY_URL}/{image.url}' else: - proxy_upstream = f'{settings.THUMBNAIL_PROXY_URL}/{settings.THUMBNAIL_WIDTH_PX}' \ + proxy_upstream = f'{settings.THUMBNAIL_PROXY_URL}/' \ + f'{settings.THUMBNAIL_WIDTH_PX}' \ f',fit/{image.url}' try: upstream_response = urlopen(proxy_upstream) diff --git a/openverse-api/catalog/api/views/site_views.py b/openverse-api/catalog/api/views/site_views.py index 246615a2e..80a1e55ba 100644 --- a/openverse-api/catalog/api/views/site_views.py +++ b/openverse-api/catalog/api/views/site_views.py @@ -316,4 +316,3 @@ def get(self, request, format=None): 'verified': verified } return Response(status=200, data=response_data) - diff --git a/openverse-api/catalog/urls/auth_tokens.py b/openverse-api/catalog/urls/auth_tokens.py index 34518f5e8..d0bd4e99b 100644 --- a/openverse-api/catalog/urls/auth_tokens.py +++ b/openverse-api/catalog/urls/auth_tokens.py @@ -20,4 +20,4 @@ '', include('oauth2_provider.urls', namespace='oauth2_provider') ), -] \ No newline at end of file +] From 77378ad17829d47e6ac8b19db3507dd594b321b1 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 20:17:31 +0530 Subject: [PATCH 15/18] Discontinue link shortening and resolve functionality --- .../catalog/api/utils/status_code_view.py | 27 ++++++++ openverse-api/catalog/api/views/link_views.py | 69 ------------------- openverse-api/catalog/urls/__init__.py | 17 +++-- openverse-api/test/v1_integration_test.py | 23 ++----- 4 files changed, 47 insertions(+), 89 deletions(-) create mode 100644 openverse-api/catalog/api/utils/status_code_view.py delete mode 100644 openverse-api/catalog/api/views/link_views.py diff --git a/openverse-api/catalog/api/utils/status_code_view.py b/openverse-api/catalog/api/utils/status_code_view.py new file mode 100644 index 000000000..544d9c6a2 --- /dev/null +++ b/openverse-api/catalog/api/utils/status_code_view.py @@ -0,0 +1,27 @@ +from django.http import JsonResponse +from django.utils.decorators import method_decorator +from django.views.decorators.csrf import csrf_exempt +from django.views.generic import View + + +def get_status_code_view(data, status_code=200): + """ + Get a class-based that returns the given data and status code on all HTTP + methods. Useful for blanket discontinuation of API endpoints. + + :param data: the dictionary to serialize as the JSON response + :param status_code: the status code of the returned response + :return: the class based view that returns the same response for all methods + """ + + @method_decorator(csrf_exempt, name='dispatch') + class StatusCodeView(View): + status = status_code + + def dispatch(self, request, *args, **kwargs): + return JsonResponse( + status=self.status, + data=data, + ) + + return StatusCodeView diff --git a/openverse-api/catalog/api/views/link_views.py b/openverse-api/catalog/api/views/link_views.py deleted file mode 100644 index b249ade05..000000000 --- a/openverse-api/catalog/api/views/link_views.py +++ /dev/null @@ -1,69 +0,0 @@ -from django.http import HttpResponsePermanentRedirect -from catalog.api.models import ShortenedLink -from rest_framework.generics import GenericAPIView -from rest_framework.views import APIView -from rest_framework.decorators import throttle_classes -from catalog.api.utils.throttle import PostRequestThrottler -from catalog.api.serializers.link_serializers import ShortenedLinkSerializer -from catalog.api.models import ShortenedLink -from catalog import settings -from rest_framework.response import Response -from rest_framework import serializers -from drf_yasg.utils import swagger_auto_schema - - -class _LinkCreatedResponse(serializers.Serializer): - shortened_url = serializers.URLField() - - -class CreateShortenedLink(GenericAPIView): - serializer_class = ShortenedLinkSerializer - swagger_schema = None - - @throttle_classes([PostRequestThrottler]) - def post(self, request, format=None): - """ Create a shortened URL. Only domains within the CC Catalog platform - will be accepted. The `full_url` must be a whitelisted endpoint.""" - full_url = request.data['full_url'] - serialized = ShortenedLinkSerializer(data={'full_url': full_url}) - if not serialized.is_valid(): - return Response( - status=400, - data=serialized.errors - ) - - try: - existing_path = ShortenedLink \ - .objects \ - .get(full_url=full_url) \ - .shortened_path - shortened_url = settings.ROOT_SHORTENING_URL + '/' + existing_path - except ShortenedLink.DoesNotExist: - shortened_path = serialized.save() - shortened_url = settings.ROOT_SHORTENING_URL + '/' + shortened_path - - return Response( - status=200, - data={ - 'shortened_url': shortened_url - } - ) - - -class ResolveShortenedLink(APIView): - swagger_schema = None - - def get(self, request, path, format=None): - """ - Given a shortened URL path, such as 'zb3k0', resolve the full URL - and redirect the caller. - """ - try: - link_instance = ShortenedLink.objects.get(shortened_path=path) - except ShortenedLink.DoesNotExist: - return Response( - status=404, - data='Not Found' - ) - full_url = link_instance.full_url - return HttpResponsePermanentRedirect(full_url) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index c4e9b7ace..7338efa67 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -23,9 +23,8 @@ from drf_yasg.views import get_schema_view from catalog.api.views.image_views import Watermark, OembedView -from catalog.api.views.link_views import CreateShortenedLink, \ - ResolveShortenedLink from catalog.api.views.site_views import HealthCheck, CheckRates +from catalog.api.utils.status_code_view import get_status_code_view from catalog.urls.auth_tokens import urlpatterns as auth_tokens_patterns from catalog.urls.audio import urlpatterns as audio_patterns @@ -188,6 +187,11 @@ cache_timeout = 0 if settings.DEBUG else 15 +discontinuation_message = { + 'error': 'Gone', + 'reason': 'This API endpoint has been discontinued.' +} + versioned_paths = [ path('rate_limit', CheckRates.as_view(), name='key_info'), path('auth_tokens/', include(auth_tokens_patterns)), @@ -215,8 +219,13 @@ RedirectView.as_view(pattern_name='image-thumb', permanent=True), name='thumbs' ), - path('link', CreateShortenedLink.as_view(), name='make-link'), - path('link/', ResolveShortenedLink.as_view(), name='resolve'), + + # Discontinued + re_path( + r'^link/', + get_status_code_view(discontinuation_message, 410).as_view(), + name='make-link' + ), ] if settings.WATERMARK_ENABLED: versioned_paths.append( diff --git a/openverse-api/test/v1_integration_test.py b/openverse-api/test/v1_integration_test.py index f137567b6..2c224c672 100644 --- a/openverse-api/test/v1_integration_test.py +++ b/openverse-api/test/v1_integration_test.py @@ -36,24 +36,15 @@ def test_image_thumb(image_fixture): assert thumbnail_response.headers["Content-Type"].startswith("image/") -@pytest.fixture -def link_shortener_fixture(image_fixture): - link_to_shorten = image_fixture['results'][0]['detail_url'] - payload = {"full_url": link_to_shorten} - response = requests.post(f'{API_URL}/v1/link', json=payload, verify=False) - assert response.status_code == 200 - return json.loads(response.text) - - -def test_link_shortener_create(link_shortener_fixture): - assert 'shortened_url' in link_shortener_fixture +def test_link_shortener_create(): + payload = {'full_url': 'abcd'} + response = requests.post(f'{API_URL}/v1/link/', json=payload, verify=False) + assert response.status_code == 410 -def test_link_shortener_resolve(link_shortener_fixture): - path = link_shortener_fixture['shortened_url'].split('/')[-1] - response = requests.get(f'{API_URL}/v1/link/{path}', allow_redirects=False, - verify=False) - assert response.status_code == 301 +def test_link_shortener_resolve(): + response = requests.get(f'{API_URL}/v1/link/abc', verify=False) + assert response.status_code == 410 @pytest.mark.skip(reason="Disabled feature") From 1603c23b7a5bd6ec845add6a0d11151a6e1c3e9a Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 20:42:24 +0530 Subject: [PATCH 16/18] Move oEmbed endpoint under images --- openverse-api/catalog/urls/__init__.py | 6 ++- openverse-api/test/image_integration_test.py | 40 +++++++++++++++++++- openverse-api/test/v1_integration_test.py | 31 --------------- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 7338efa67..165ac0f0d 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -201,7 +201,6 @@ # Images path('images/', include(images_patterns)), - path('oembed', OembedView.as_view(), name='oembed'), # Deprecated path( @@ -214,6 +213,11 @@ RedirectView.as_view(pattern_name='image-related', permanent=True), name='related-images', ), + path( + 'oembed', + RedirectView.as_view(pattern_name='image-oembed', query_string=True, permanent=True), + name='oembed' + ), path( 'thumbs/', RedirectView.as_view(pattern_name='image-thumb', permanent=True), diff --git a/openverse-api/test/image_integration_test.py b/openverse-api/test/image_integration_test.py index e0a10747b..6a3b4dcdc 100644 --- a/openverse-api/test/image_integration_test.py +++ b/openverse-api/test/image_integration_test.py @@ -4,9 +4,10 @@ """ import json - +from urllib.parse import urlencode import pytest import requests +import xml.etree.ElementTree as ET from test.constants import API_URL from test.media_integration import ( @@ -50,3 +51,40 @@ def test_image_detail(image_fixture): def test_image_stats(): stats('images', 'image_count') + + +def test_oembed_endpoint_for_json(): + params = { + 'url': 'https://any.domain/any/path/29cb352c-60c1-41d8-bfa1-7d6f7d955f63', + # 'format': 'json' is the default + } + response = requests.get( + f'{API_URL}/v1/images/oembed?{urlencode(params)}', + verify=False + ) + assert response.status_code == 200 + assert response.headers['Content-Type'] == "application/json" + + parsed = response.json() + assert parsed['width'] == 1276 + assert parsed['height'] == 1536 + assert parsed['license_url'] == 'https://creativecommons.org/licenses/by-nc-nd/4.0/' + + +def test_oembed_endpoint_for_xml(): + params = { + 'url': 'https://any.domain/any/path/29cb352c-60c1-41d8-bfa1-7d6f7d955f63', + 'format': 'xml' + } + response = requests.get( + f'{API_URL}/v1/images/oembed?{urlencode(params)}', + verify=False + ) + assert response.status_code == 200 + assert response.headers['Content-Type'] == "application/xml; charset=utf-8" + + response_body_as_xml = ET.fromstring(response.content) + xml_tree = ET.ElementTree(response_body_as_xml) + assert xml_tree.find("width").text == '1276' + assert xml_tree.find("height").text == '1536' + assert xml_tree.find("license_url").text == 'https://creativecommons.org/licenses/by-nc-nd/4.0/' diff --git a/openverse-api/test/v1_integration_test.py b/openverse-api/test/v1_integration_test.py index 2c224c672..59e8479c9 100644 --- a/openverse-api/test/v1_integration_test.py +++ b/openverse-api/test/v1_integration_test.py @@ -9,7 +9,6 @@ import uuid import time import catalog.settings -import xml.etree.ElementTree as ET from django.db.models import Max from django.urls import reverse @@ -458,36 +457,6 @@ def test_related_image_search_page_consistency( assert len(related['results']) == 10 -def test_oembed_endpoint_for_json(): - response = requests.get( - f'{API_URL}/v1/oembed?url=https%3A//' - 'search.creativecommons.org/photos/' - '29cb352c-60c1-41d8-bfa1-7d6f7d955f63' - ) - assert response.status_code == 200 - assert response.headers['Content-Type'] == "application/json" - parsed = response.json() - assert parsed['width'] == 1276 - assert parsed['height'] == 1536 - assert parsed['license_url'] == 'https://creativecommons.org/licenses/by-nc-nd/4.0/' - - -def test_oembed_endpoint_for_xml(): - response = requests.get( - f'{API_URL}/v1/oembed?url=https%3A//' - 'search.creativecommons.org/photos/' - '29cb352c-60c1-41d8-bfa1-7d6f7d955f63' - '&format=xml' - ) - assert response.status_code == 200 - assert response.headers['Content-Type'] == "application/xml; charset=utf-8" - response_body_as_xml = ET.fromstring(response.content) - xml_tree = ET.ElementTree(response_body_as_xml) - assert xml_tree.find("width").text == '1276' - assert xml_tree.find("height").text == '1536' - assert xml_tree.find("license_url").text == 'https://creativecommons.org/licenses/by-nc-nd/4.0/' - - def test_report_endpoint(): identifier = 'dac5f6b0-e07a-44a0-a444-7f43d71f9beb' payload = { From db1e4ab79ecc1bb9c725cffdf822d4386a53bd78 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Wed, 11 Aug 2021 20:43:08 +0530 Subject: [PATCH 17/18] Retain backwards compatibility for old oEmbed endpoint --- openverse-api/catalog/urls/__init__.py | 6 +++++- openverse-api/catalog/urls/images.py | 6 ++++++ openverse-api/test/backwards_compat_test.py | 11 +++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/openverse-api/catalog/urls/__init__.py b/openverse-api/catalog/urls/__init__.py index 165ac0f0d..64e624689 100644 --- a/openverse-api/catalog/urls/__init__.py +++ b/openverse-api/catalog/urls/__init__.py @@ -215,7 +215,11 @@ ), path( 'oembed', - RedirectView.as_view(pattern_name='image-oembed', query_string=True, permanent=True), + RedirectView.as_view( + pattern_name='image-oembed', + query_string=True, + permanent=True + ), name='oembed' ), path( diff --git a/openverse-api/catalog/urls/images.py b/openverse-api/catalog/urls/images.py index eac870e12..4595a86ee 100644 --- a/openverse-api/catalog/urls/images.py +++ b/openverse-api/catalog/urls/images.py @@ -7,6 +7,7 @@ ImageStats, ReportImageView, ProxiedImage, + OembedView, ) urlpatterns = [ @@ -15,6 +16,11 @@ ImageStats.as_view(), name='image-stats' ), + path( + 'oembed', + OembedView.as_view(), + name='image-oembed' + ), path( '', ImageDetail.as_view(), diff --git a/openverse-api/test/backwards_compat_test.py b/openverse-api/test/backwards_compat_test.py index fe7d3c791..e09ee4f35 100644 --- a/openverse-api/test/backwards_compat_test.py +++ b/openverse-api/test/backwards_compat_test.py @@ -33,6 +33,17 @@ def test_old_related_images_endpoint(): assert response.headers.get('Location') == '/v1/images/xyz/recommendations' +def test_old_oembed_endpoint(): + response = requests.get( + f'{API_URL}/v1/oembed?key=value', + allow_redirects=False, + verify=False + ) + assert response.status_code == 301 + assert response.is_permanent_redirect + assert response.headers.get('Location') == '/v1/images/oembed?key=value' + + def test_old_thumbs_endpoint(): response = requests.get( f'{API_URL}/v1/thumbs/xyz', From 80fc446eaa19489a2f50f2aa6d9cfead6f516918 Mon Sep 17 00:00:00 2001 From: Dhruv Bhanushali Date: Mon, 23 Aug 2021 16:12:24 +0530 Subject: [PATCH 18/18] Add a missing word in the documentation --- openverse-api/catalog/api/utils/status_code_view.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openverse-api/catalog/api/utils/status_code_view.py b/openverse-api/catalog/api/utils/status_code_view.py index 544d9c6a2..73642ef2e 100644 --- a/openverse-api/catalog/api/utils/status_code_view.py +++ b/openverse-api/catalog/api/utils/status_code_view.py @@ -6,8 +6,8 @@ def get_status_code_view(data, status_code=200): """ - Get a class-based that returns the given data and status code on all HTTP - methods. Useful for blanket discontinuation of API endpoints. + Get a class-based view that returns the given data and status code on all + HTTP methods. Useful for blanket discontinuation of API endpoints. :param data: the dictionary to serialize as the JSON response :param status_code: the status code of the returned response