Use full YouTube URLs instead of just video IDs

Use full YouTube URLs instead of just video IDs as the path params for
the `/video/` route.

Add a new `YouTubeService` that handles parsing YouTube URLs in order to
extract the video IDs from them. In the future this same
`YoutubeService` will also be used to get video transcripts from the
YouTube API.

This commit also adds a new `YOUTUBE_CAPTIONS` setting and disables the
`/video` route (HTTP Unauthorized) is this setting isn't enabled.

`get_url_details.py` is also turned into a service (`URLDetailsService`)
because it now depends on two other services (and because previously it
was a random top-level file).

Also, HTTP caching headers are added to the `/video` route.

tests/conftest.py

@@ -17,6 +17,7 @@ def pyramid_settings():
         "checkmate_ignore_reasons": None,
         "checkmate_allow_all": False,
         "enable_front_page": True,
+        "youtube_captions": True,
         "data_directory": "tests/data_directory",
         "dev": False,
     }

tests/unit/conftest.py

@@ -7,6 +7,7 @@
 
 from tests.unit.services import *  # pylint: disable=wildcard-import,unused-wildcard-import
 from via.checkmate import ViaCheckmateClient
+from via.resources import QueryURLResource
 from via.views import add_routes
 
 
@@ -48,3 +49,13 @@ def pyramid_request(
     pyramid_request.checkmate.check_url.return_value = None
 
     return pyramid_request
+
+
+@pytest.fixture
+def call_view(pyramid_request):
+    def call_view(url="http://example.com/name.pdf", params=None, view=None):
+        pyramid_request.params = dict(params or {}, url=url)
+        context = QueryURLResource(pyramid_request)
+        return view(context, pyramid_request)
+
+    return call_view

tests/unit/services.py

@@ -7,7 +7,9 @@
     HTTPService,
     PDFURLBuilder,
     SecureLinkService,
+    URLDetailsService,
     ViaClientService,
+    YoutubeService,
 )
 
 
@@ -27,6 +29,16 @@ def via_client_service(mock_service):
     return mock_service(ViaClientService)
 
 
+@pytest.fixture
+def url_details_service(mock_service):
+    return mock_service(URLDetailsService)
+
+
+@pytest.fixture
+def youtube_service(mock_service):
+    return mock_service(YoutubeService)
+
+
 @pytest.fixture
 def google_drive_api(mock_service):
     return mock_service(GoogleDriveAPI)

tests/unit/via/get_url_test.py → tests/unit/via/services/url_details_test.py

@@ -5,7 +5,7 @@
 from h_matchers import Any
 from requests import Response
 
-from via.get_url import get_url_details
+from via.services.url_details import URLDetailsService, factory
 
 
 class TestGetURLDetails:
@@ -26,6 +26,7 @@ def test_it_calls_get_for_normal_urls(
         mime_type,
         status_code,
         http_service,
+        svc,
     ):
         if content_type:
             response.headers = {"Content-Type": content_type}
@@ -36,7 +37,7 @@ def test_it_calls_get_for_normal_urls(
 
         url = "http://example.com"
 
-        result = get_url_details(http_service, url, headers=sentinel.headers)
+        result = svc.get_url_details(url, headers=sentinel.headers)
 
         assert result == (mime_type, status_code)
         http_service.get.assert_called_once_with(
@@ -50,27 +51,53 @@ def test_it_calls_get_for_normal_urls(
 
     @pytest.mark.usefixtures("response")
     def test_it_modifies_headers(
-        self, clean_headers, add_request_headers, http_service
+        self, clean_headers, add_request_headers, http_service, svc
     ):
-        get_url_details(
-            http_service, url="http://example.com", headers={"X-Pre-Existing": 1}
-        )
+        svc.get_url_details(url="http://example.com", headers={"X-Pre-Existing": 1})
 
         _args, kwargs = http_service.get.call_args
 
         clean_headers.assert_called_once_with({"X-Pre-Existing": 1})
         add_request_headers.assert_called_once_with(clean_headers.return_value)
         assert kwargs["headers"] == add_request_headers.return_value
 
-    def test_it_assumes_pdf_with_a_google_drive_url(self, http_service, GoogleDriveAPI):
+    def test_it_assumes_pdf_with_a_google_drive_url(
+        self, http_service, GoogleDriveAPI, svc
+    ):
         GoogleDriveAPI.parse_file_url.return_value = {"file_id": "FILE_ID"}
 
-        result = get_url_details(http_service, sentinel.google_drive_url)
+        result = svc.get_url_details(sentinel.google_drive_url)
 
         assert result == ("application/pdf", 200)
         GoogleDriveAPI.parse_file_url.assert_called_once_with(sentinel.google_drive_url)
         http_service.get.assert_not_called()
 
+    def test_it_returns_video_for_youtube_url(
+        self, http_service, youtube_service, GoogleDriveAPI, svc
+    ):
+        GoogleDriveAPI.parse_file_url.return_value = {}
+        youtube_service.parse_url.return_value = "VIDEO_ID"
+        youtube_service.enabled = True
+
+        result = svc.get_url_details(sentinel.youtube_url)
+
+        assert result == ("video/x-youtube", 200)
+        youtube_service.parse_url.assert_called_once_with(sentinel.youtube_url)
+        http_service.get.assert_not_called()
+
+    @pytest.mark.usefixtures("response")
+    def test_it_when_youtube_disabled(self, youtube_service, GoogleDriveAPI, svc):
+        GoogleDriveAPI.parse_file_url.return_value = {}
+        youtube_service.enabled = False
+
+        svc.get_url_details(sentinel.youtube_url)
+
+        youtube_service.parse_url.assert_not_called()
+
+    @pytest.fixture
+    def svc(self, http_service, youtube_service):
+        return URLDetailsService(http_service, youtube_service)
+
     @pytest.fixture
     def response(self, http_service):
         response = Response()
@@ -83,12 +110,24 @@ def response(self, http_service):
 
     @pytest.fixture
     def GoogleDriveAPI(self, patch):
-        return patch("via.get_url.GoogleDriveAPI", return_value={})
+        return patch("via.services.url_details.GoogleDriveAPI", return_value={})
+
+    @pytest.fixture(autouse=True)
+    def youtube_service(self, youtube_service):
+        youtube_service.enabled = False
+        return youtube_service
 
     @pytest.fixture(autouse=True)
     def add_request_headers(self, patch):
-        return patch("via.get_url.add_request_headers", return_value={})
+        return patch("via.services.url_details.add_request_headers", return_value={})
 
     @pytest.fixture(autouse=True)
     def clean_headers(self, patch):
-        return patch("via.get_url.clean_headers", return_value={})
+        return patch("via.services.url_details.clean_headers", return_value={})
+
+
+@pytest.mark.usefixtures("http_service", "youtube_service")
+def test_factory(pyramid_request):
+    svc = factory(sentinel.context, pyramid_request)
+
+    assert isinstance(svc, URLDetailsService)

tests/unit/via/services/via_client_test.py

@@ -6,17 +6,6 @@
 
 
 class TestViaClientService:
-    @pytest.mark.parametrize(
-        "mime_type,is_pdf",
-        [
-            ("application/x-pdf", True),
-            ("application/pdf", True),
-            ("text/html", False),
-        ],
-    )
-    def test_is_pdf(self, mime_type, is_pdf, svc):
-        assert svc.is_pdf(mime_type) == is_pdf
-
     @pytest.mark.parametrize(
         "mime_type,expected_content_type",
         [

tests/unit/via/services/youtube_test.py

@@ -0,0 +1,33 @@
+from unittest.mock import sentinel
+
+import pytest
+
+from via.services.youtube import YoutubeService, factory
+
+
+class TestYoutubeService:
+    @pytest.mark.parametrize(
+        "url,video_id",
+        [
+            ("not_an_url", None),
+            ("https://notyoutube:1000000", None),
+            ("https://notyoutube.com", None),
+            ("https://youtube.com", None),
+            ("https://youtube.com?param=nope", None),
+            ("https://youtube.com?v=", None),
+            ("https://www.youtube.com/watch?v=VIDEO_ID", "VIDEO_ID"),
+            ("https://www.youtube.com/watch?v=VIDEO_ID&t=14s", "VIDEO_ID"),
+        ],
+    )
+    def test_parse_file_url(self, url, video_id):
+        assert video_id == YoutubeService.parse_url(url)
+
+    @pytest.mark.parametrize("enabled,expected", [(True, True), (False, False)])
+    def test_enabled(self, enabled, expected):
+        assert YoutubeService(enabled).enabled == expected
+
+
+def test_factory(pyramid_request):
+    svc = factory(sentinel.context, pyramid_request)
+
+    assert isinstance(svc, YoutubeService)

tests/unit/via/views/proxy_test.py

@@ -8,19 +8,18 @@
 
 class TestProxy:
     def test_it(
-        self,
-        context,
-        pyramid_request,
-        get_url_details,
-        via_client_service,
-        http_service,
+        self, context, pyramid_request, url_details_service, via_client_service
     ):
+        url_details_service.get_url_details.return_value = (
+            sentinel.mime_type,
+            sentinel.status_code,
+        )
         url = context.url_from_path.return_value = "/https://example.org?a=1"
 
         result = proxy(context, pyramid_request)
 
         pyramid_request.checkmate.raise_if_blocked.assert_called_once_with(url)
-        get_url_details.assert_called_once_with(http_service, url)
+        url_details_service.get_url_details.assert_called_once_with(url)
         via_client_service.url_for.assert_called_once_with(
             url, sentinel.mime_type, pyramid_request.params
         )
@@ -29,9 +28,3 @@ def test_it(
     @pytest.fixture
     def context(self):
         return create_autospec(PathURLResource, spec_set=True, instance=True)
-
-    @pytest.fixture(autouse=True)
-    def get_url_details(self, patch):
-        get_url_details = patch("via.views.proxy.get_url_details")
-        get_url_details.return_value = sentinel.mime_type, sentinel.status_code
-        return get_url_details

tests/unit/via/views/route_by_content_test.py

@@ -13,37 +13,39 @@ class TestRouteByContent:
     @pytest.mark.parametrize(
         "content_type,status_code,expected_cache_control_header",
         [
-            ("PDF", 200, "public, max-age=300, stale-while-revalidate=86400"),
-            ("HTML", 200, "public, max-age=60, stale-while-revalidate=86400"),
-            ("HTML", 401, "public, max-age=60, stale-while-revalidate=86400"),
-            ("HTML", 404, "public, max-age=60, stale-while-revalidate=86400"),
-            ("HTML", 500, "no-cache"),
-            ("HTML", 501, "no-cache"),
+            ("pdf", 200, "public, max-age=300, stale-while-revalidate=86400"),
+            ("html", 200, "public, max-age=60, stale-while-revalidate=86400"),
+            ("html", 401, "public, max-age=60, stale-while-revalidate=86400"),
+            ("html", 404, "public, max-age=60, stale-while-revalidate=86400"),
+            ("html", 500, "no-cache"),
+            ("html", 501, "no-cache"),
         ],
     )
     def test_it(
         self,
         content_type,
         context,
         expected_cache_control_header,
-        get_url_details,
+        url_details_service,
         pyramid_request,
         status_code,
         via_client_service,
-        http_service,
     ):
         pyramid_request.params = {"url": sentinel.url, "foo": "bar"}
-        get_url_details.return_value = (sentinel.mime_type, status_code)
-        via_client_service.is_pdf.return_value = content_type == "PDF"
+        url_details_service.get_url_details.return_value = (
+            sentinel.mime_type,
+            status_code,
+        )
+        via_client_service.content_type.return_value = content_type
 
         response = route_by_content(context, pyramid_request)
 
         url = context.url_from_query.return_value
         pyramid_request.checkmate.raise_if_blocked.assert_called_once_with(url)
-        get_url_details.assert_called_once_with(
-            http_service, url, pyramid_request.headers
+        url_details_service.get_url_details.assert_called_once_with(
+            url, pyramid_request.headers
         )
-        via_client_service.is_pdf.assert_called_once_with(sentinel.mime_type)
+        via_client_service.content_type.assert_called_once_with(sentinel.mime_type)
         via_client_service.url_for.assert_called_once_with(
             url, sentinel.mime_type, {"foo": "bar"}
         )
@@ -57,7 +59,3 @@ def test_it(
     @pytest.fixture
     def context(self):
         return create_autospec(QueryURLResource, spec_set=True, instance=True)
-
-    @pytest.fixture(autouse=True)
-    def get_url_details(self, patch):
-        return patch("via.views.route_by_content.get_url_details")

tests/unit/via/views/view_pdf_test.py

@@ -4,7 +4,6 @@
 from h_matchers import Any
 from pyramid.httpexceptions import HTTPNoContent
 
-from via.resources import QueryURLResource
 from via.views.view_pdf import proxy_google_drive_file, proxy_python_pdf, view_pdf
 
 
@@ -16,7 +15,7 @@
 )
 class TestViewPDF:
     def test_it(self, call_view, pyramid_request, pyramid_settings, Configuration):
-        response = call_view("http://example.com/foo.pdf")
+        response = call_view("http://example.com/foo.pdf", view=view_pdf)
 
         Configuration.extract_from_params.assert_called_once_with(
             pyramid_request.params
@@ -36,7 +35,7 @@ def test_it_builds_the_url(
     ):
         google_drive_api.parse_file_url.return_value = None
 
-        call_view("https://example.com/foo/bar.pdf?q=s")
+        call_view("https://example.com/foo/bar.pdf?q=s", view=view_pdf)
 
         pdf_url_builder_service.get_pdf_url.assert_called_once_with(
             "https://example.com/foo/bar.pdf?q=s"
@@ -138,13 +137,3 @@ def test_it_can_stream_an_empty_iterator(self, http_service, call_view):
         response = call_view("https://one-drive.com", view=proxy_python_pdf)
 
         assert isinstance(response, HTTPNoContent)
-
-
-@pytest.fixture
-def call_view(pyramid_request):
-    def call_view(url="http://example.com/name.pdf", params=None, view=view_pdf):
-        pyramid_request.params = dict(params or {}, url=url)
-        context = QueryURLResource(pyramid_request)
-        return view(context, pyramid_request)
-
-    return call_view

tests/unit/via/views/view_video_test.py

@@ -1,15 +1,14 @@
 from unittest.mock import sentinel
 
 import pytest
+from pyramid.httpexceptions import HTTPUnauthorized
 
 from via.views.view_video import view_video
 
 
 class TestViewVideo:
-    def test_it(self, pyramid_request, Configuration):
-        pyramid_request.matchdict["id"] = "abcdef"
-
-        response = view_video(pyramid_request)
+    def test_it(self, Configuration, call_view, youtube_service):
+        response = call_view(view=view_video, url="https://youtube.com")
 
         assert response["client_embed_url"] == "http://hypothes.is/embed.js"
         assert (
@@ -28,7 +27,15 @@ def test_it(self, pyramid_request, Configuration):
                 },
             ],
         }
-        assert response["video_id"] == "abcdef"
+        youtube_service.parse_url.assert_called_once_with("https://youtube.com")
+        assert response["video_id"] == youtube_service.parse_url.return_value
+
+    def test_it_when_disabled(self, youtube_service, call_view):
+        youtube_service.enabled = False
+
+        response = call_view(view=view_video, url="https://youtube.com")
+
+        assert isinstance(response, HTTPUnauthorized)
 
     @pytest.fixture
     def Configuration(self, patch):