From 1c2eca05602757ec70da83485ed134efc104db00 Mon Sep 17 00:00:00 2001 From: Kutu Date: Sat, 30 Sep 2023 23:09:48 +0200 Subject: [PATCH 1/3] Add tests for captions --- tests/api/test_media_retrieval.py | 98 +++++++++++++++++++++++++++++-- tests/mocks/media_retrieval.py | 50 +++++++++++++--- 2 files changed, 133 insertions(+), 15 deletions(-) diff --git a/tests/api/test_media_retrieval.py b/tests/api/test_media_retrieval.py index 214e963..e6708b0 100644 --- a/tests/api/test_media_retrieval.py +++ b/tests/api/test_media_retrieval.py @@ -2,6 +2,8 @@ from pathlib import Path from typing import Any +import pytest +from _pytest.fixtures import FixtureRequest from responses import Response import responses from knuckles import Subsonic @@ -31,7 +33,7 @@ def test_download_with_a_given_filename( song["id"], tmp_path / download_metadata.output_filename ) - # Check if the file data has been unaltered + # Check if the file data has been altered with open(tmp_path / download_metadata.output_filename, "r") as file: assert placeholder_data == file.read() @@ -51,7 +53,7 @@ def test_download_without_a_given_filename( download_path = subsonic.media_retrieval.download(song["id"], tmp_path) - # Check if the file data has been unaltered + # Check if the file data has been altered with open(tmp_path / download_metadata.default_filename, "r") as file: assert placeholder_data == file.read() @@ -65,6 +67,90 @@ def test_hls(subsonic: Subsonic, song: dict[str, Any]) -> None: assert parse.parse_qs(stream_url.query)["id"][0] == song["id"] +def test_get_captions_with_a_given_filename( + subsonic: Subsonic, + mock_get_captions_vtt: Response, + tmp_path: Path, + placeholder_data: str, + song: dict[str, Any], + vtt_metadata: FileMetadata, +): + responses.add(mock_get_captions_vtt) + + download_path = subsonic.media_retrieval.get_captions( + song["id"], tmp_path / vtt_metadata.output_filename + ) + + # Check if the file data has been altered + with open(tmp_path / vtt_metadata.output_filename, "r") as file: + assert placeholder_data == file.read() + + assert download_path == tmp_path / vtt_metadata.output_filename + + +@pytest.mark.parametrize( + "mock, metadata", + [ + ("mock_get_captions_vtt", "vtt_metadata"), + ("mock_get_captions_srt", "srt_metadata"), + ], +) +def test_get_captions_without_a_given_filename( + request: FixtureRequest, + subsonic: Subsonic, + mock: str, + tmp_path: Path, + placeholder_data: str, + song: dict[str, Any], + metadata: str, +): + # Retrieve the mocks dynamically as their tests are equal + get_mock: Response = request.getfixturevalue(mock) + get_metadata: FileMetadata = request.getfixturevalue(metadata) + + responses.add(get_mock) + + download_path = subsonic.media_retrieval.get_captions(song["id"], tmp_path) + + # Check if the file data has been altered + with open(tmp_path / get_metadata.default_filename, "r") as file: + assert placeholder_data == file.read() + + assert download_path == tmp_path / get_metadata.default_filename + + +@pytest.mark.parametrize( + "mock, metadata, file_format", + [ + ("mock_get_captions_vtt", "vtt_metadata", SubtitlesFormat.vtt), + ("mock_get_captions_srt", "srt_metadata", SubtitlesFormat.srt), + ], +) +def test_get_captions_with_a_preferred_file_format( + request: FixtureRequest, + subsonic: Subsonic, + mock: str, + tmp_path: Path, + placeholder_data: str, + song: dict[str, Any], + metadata: str, + file_format: SubtitlesFormat, +): + # Retrieve the mocks dynamically as their tests are equal + get_mock: Response = request.getfixturevalue(mock) + get_metadata: FileMetadata = request.getfixturevalue(metadata) + + responses.add(get_mock) + + download_path = subsonic.media_retrieval.download(song["id"], tmp_path, file_format) + + # Check if the file data has been altered + with open(tmp_path / get_metadata.default_filename, "r") as file: + assert placeholder_data == file.read() + + assert download_path == tmp_path / get_metadata.default_filename + + @responses.activate def test_get_cover_art_with_a_given_filename( subsonic: Subsonic, @@ -81,7 +167,7 @@ def test_get_cover_art_with_a_given_filename( song["coverArt"], tmp_path / cover_art_metadata.output_filename, cover_art_size ) - # Check if the file data has been unaltered + # Check if the file data has been altered with open(tmp_path / cover_art_metadata.output_filename, "r") as file: assert placeholder_data == file.read() @@ -104,7 +190,7 @@ def test_get_cover_art_without_a_given_filename( song["coverArt"], tmp_path, cover_art_size ) - # Check if the file data has been unaltered + # Check if the file data has been altered with open(tmp_path / cover_art_metadata.default_filename, "r") as file: assert placeholder_data == file.read() @@ -126,7 +212,7 @@ def test_get_avatar_with_a_given_filename( username, tmp_path / avatar_metadata.output_filename ) - # Check if the file data has been unaltered + # Check if the file data has been altered with open(tmp_path / avatar_metadata.output_filename, "r") as file: assert placeholder_data == file.read() @@ -146,7 +232,7 @@ def test_get_avatar_without_a_given_filename( download_path = subsonic.media_retrieval.get_avatar(username, tmp_path) - # Check if the file data has been unaltered + # Check if the file data has been altered with open(tmp_path / avatar_metadata.default_filename, "r") as file: assert placeholder_data == file.read() diff --git a/tests/mocks/media_retrieval.py b/tests/mocks/media_retrieval.py index 05834b1..feed66f 100644 --- a/tests/mocks/media_retrieval.py +++ b/tests/mocks/media_retrieval.py @@ -1,3 +1,4 @@ +from collections import namedtuple from dataclasses import dataclass from pathlib import Path from typing import Any, Protocol @@ -52,15 +53,9 @@ def inner( return inner -@dataclass -class FileMetadata: - # The default filename that Knuckles should use if no custom one is provided - default_filename: str - - # The custom filename given to Knuckles - output_filename: str - - content_type: str +FileMetadata = namedtuple( + "FileMetadata", ["default_filename", "output_filename", "content_type"] +) @pytest.fixture @@ -85,6 +80,43 @@ def mock_download( ) +@pytest.fixture +def srt_metadata() -> FileMetadata: + # This MIME TYPE is not approved by the IANA + return FileMetadata("default.srt", "output.srt", "application/x-subrip") + + +@pytest.fixture +def vtt_metadata() -> FileMetadata: + return FileMetadata("default.vtt", "output.vtt", "text/vtt") + + +@pytest.fixture +def mock_get_captions_srt( + mock_download_file_generator: MockDownload, + song: dict[str, Any], + srt_metadata: FileMetadata, +) -> Response: + return mock_download_file_generator( + "getCaptions", + {"id": song["id"]}, + srt_metadata.content_type, + ) + + +@pytest.fixture +def mock_get_captions_vtt( + mock_download_file_generator: MockDownload, + song: dict[str, Any], + vtt_metadata: FileMetadata, +) -> Response: + return mock_download_file_generator( + "getCaptions", + {"id": song["coverArt"]}, + vtt_metadata.content_type, + ) + + @pytest.fixture def cover_art_metadata(song: dict[str, Any]) -> FileMetadata: return FileMetadata(f"{song['coverArt']}.png", "output.png", "image/png") From 8e711f438f93b49cc41a169230ac48aac1f47461 Mon Sep 17 00:00:00 2001 From: Kutu Date: Sat, 30 Sep 2023 23:59:58 +0200 Subject: [PATCH 2/3] Implement captions downloading --- src/knuckles/media_annotation.py | 1 - src/knuckles/media_retrieval.py | 34 +++++++++++++++++++++-- tests/api/test_media_retrieval.py | 14 +++++++--- tests/mocks/media_retrieval.py | 45 ++++++++++++++++++++++++------- 4 files changed, 77 insertions(+), 17 deletions(-) diff --git a/src/knuckles/media_annotation.py b/src/knuckles/media_annotation.py index 1f48fa6..2a65136 100644 --- a/src/knuckles/media_annotation.py +++ b/src/knuckles/media_annotation.py @@ -1,6 +1,5 @@ from datetime import datetime from typing import TYPE_CHECKING - from .api import Api from .exceptions import InvalidRatingNumber diff --git a/src/knuckles/media_retrieval.py b/src/knuckles/media_retrieval.py index 15b17a1..96ab02a 100644 --- a/src/knuckles/media_retrieval.py +++ b/src/knuckles/media_retrieval.py @@ -1,3 +1,4 @@ +from enum import Enum from mimetypes import guess_extension from pathlib import Path from typing import Any @@ -8,6 +9,11 @@ from .api import Api +class SubtitlesFileFormat(Enum): + VTT = "vtt" + SRT = "srt" + + class MediaRetrieval: """Class that contains all the methods needed to interact with the media retrieval calls in the Subsonic API. @@ -95,8 +101,32 @@ def hls(self, id: str) -> str: return self._generate_url("hls.m3u8", {"id": id}) - def get_captions(self) -> None: - ... + def get_captions( + self, + id: str, + file_or_directory_path: Path, + subtitles_file_format: SubtitlesFileFormat = SubtitlesFileFormat.VTT, + ) -> Path: + # Check if the given file format is a valid one + SubtitlesFileFormat(subtitles_file_format.value) + + response = self.api.raw_request( + "getCaptions", + {"id": id, "format": subtitles_file_format.value}, + ) + + mime_type = response.headers["content-type"].partition(";")[0].strip() + + # As application/x-subrip is not a valid MIME TYPE a manual check is done + file_extension: str | None + if mime_type == "application/x-subrip": + file_extension = ".srt" + else: + file_extension = guess_extension(mime_type) + + filename = id + file_extension if file_extension else id + + return self._download_file(response, file_or_directory_path, filename) def get_cover_art( self, id: str, file_or_directory_path: Path, size: int | None = None diff --git a/tests/api/test_media_retrieval.py b/tests/api/test_media_retrieval.py index e6708b0..442aab7 100644 --- a/tests/api/test_media_retrieval.py +++ b/tests/api/test_media_retrieval.py @@ -8,6 +8,7 @@ import responses from knuckles import Subsonic +from knuckles.media_retrieval import SubtitlesFileFormat from tests.mocks.media_retrieval import FileMetadata @@ -67,6 +68,7 @@ def test_hls(subsonic: Subsonic, song: dict[str, Any]) -> None: assert parse.parse_qs(stream_url.query)["id"][0] == song["id"] +@responses.activate def test_get_captions_with_a_given_filename( subsonic: Subsonic, mock_get_captions_vtt: Response, @@ -88,6 +90,7 @@ def test_get_captions_with_a_given_filename( assert download_path == tmp_path / vtt_metadata.output_filename +@responses.activate @pytest.mark.parametrize( "mock, metadata", [ @@ -119,11 +122,12 @@ def test_get_captions_without_a_given_filename( assert download_path == tmp_path / get_metadata.default_filename +@responses.activate @pytest.mark.parametrize( "mock, metadata, file_format", [ - ("mock_get_captions_vtt", "vtt_metadata", SubtitlesFormat.vtt), - ("mock_get_captions_srt", "srt_metadata", SubtitlesFormat.srt), + ("mock_get_captions_prefer_vtt", "vtt_metadata", SubtitlesFileFormat.VTT), + ("mock_get_captions_prefer_srt", "srt_metadata", SubtitlesFileFormat.SRT), ], ) def test_get_captions_with_a_preferred_file_format( @@ -134,7 +138,7 @@ def test_get_captions_with_a_preferred_file_format( placeholder_data: str, song: dict[str, Any], metadata: str, - file_format: SubtitlesFormat, + file_format: SubtitlesFileFormat, ): # Retrieve the mocks dynamically as their tests are equal get_mock: Response = request.getfixturevalue(mock) @@ -142,7 +146,9 @@ def test_get_captions_with_a_preferred_file_format( responses.add(get_mock) - download_path = subsonic.media_retrieval.download(song["id"], tmp_path, file_format) + download_path = subsonic.media_retrieval.get_captions( + song["id"], tmp_path, file_format + ) # Check if the file data has been altered with open(tmp_path / get_metadata.default_filename, "r") as file: diff --git a/tests/mocks/media_retrieval.py b/tests/mocks/media_retrieval.py index feed66f..beba8a2 100644 --- a/tests/mocks/media_retrieval.py +++ b/tests/mocks/media_retrieval.py @@ -1,5 +1,4 @@ from collections import namedtuple -from dataclasses import dataclass from pathlib import Path from typing import Any, Protocol @@ -81,14 +80,40 @@ def mock_download( @pytest.fixture -def srt_metadata() -> FileMetadata: - # This MIME TYPE is not approved by the IANA - return FileMetadata("default.srt", "output.srt", "application/x-subrip") +def vtt_metadata(song: dict[str, Any]) -> FileMetadata: + return FileMetadata(f"{song['id']}.vtt", "output.vtt", "text/vtt") @pytest.fixture -def vtt_metadata() -> FileMetadata: - return FileMetadata("default.vtt", "output.vtt", "text/vtt") +def mock_get_captions_vtt( + mock_download_file_generator: MockDownload, + song: dict[str, Any], + vtt_metadata: FileMetadata, +) -> Response: + return mock_download_file_generator( + "getCaptions", + {"id": song["id"]}, + vtt_metadata.content_type, + ) + + +@pytest.fixture +def mock_get_captions_prefer_vtt( + mock_download_file_generator: MockDownload, + song: dict[str, Any], + vtt_metadata: FileMetadata, +) -> Response: + return mock_download_file_generator( + "getCaptions", + {"id": song["id"], "format": "vtt"}, + vtt_metadata.content_type, + ) + + +@pytest.fixture +def srt_metadata(song: dict[str, Any]) -> FileMetadata: + # This MIME TYPE is not approved by the IANA + return FileMetadata(f"{song['id']}.srt", "output.srt", "application/x-subrip") @pytest.fixture @@ -105,15 +130,15 @@ def mock_get_captions_srt( @pytest.fixture -def mock_get_captions_vtt( +def mock_get_captions_prefer_srt( mock_download_file_generator: MockDownload, song: dict[str, Any], - vtt_metadata: FileMetadata, + srt_metadata: FileMetadata, ) -> Response: return mock_download_file_generator( "getCaptions", - {"id": song["coverArt"]}, - vtt_metadata.content_type, + {"id": song["id"], "format": "srt"}, + srt_metadata.content_type, ) From 7e778589636094d3eccd855516ed7e2eb51a7816 Mon Sep 17 00:00:00 2001 From: Kutu Date: Sun, 1 Oct 2023 00:01:52 +0200 Subject: [PATCH 3/3] Clean if statement --- src/knuckles/media_retrieval.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/knuckles/media_retrieval.py b/src/knuckles/media_retrieval.py index 96ab02a..437d10c 100644 --- a/src/knuckles/media_retrieval.py +++ b/src/knuckles/media_retrieval.py @@ -118,11 +118,10 @@ def get_captions( mime_type = response.headers["content-type"].partition(";")[0].strip() # As application/x-subrip is not a valid MIME TYPE a manual check is done - file_extension: str | None - if mime_type == "application/x-subrip": - file_extension = ".srt" - else: + if not mime_type == "application/x-subrip": file_extension = guess_extension(mime_type) + else: + file_extension = ".srt" filename = id + file_extension if file_extension else id