Backup_Repos/beets
1
0
Fork 0
mirror of https://github.com/beetbox/beets.git synced 2026-02-10 17:34:05 +01:00
Code Issues Projects Releases Wiki Activity

Add fetchart typing v2 (#5716)

Browse source 
Adds typings to fetchart; a revised and modernized version of
https://github.com/beetbox/beets/pull/5213.

The former pull request became stale because I did more refactoring
there, in order to express properties of the `Candidate` (whether or not
it is validated) via the type system (motivated by the type state
pattern which is more common in Rust.
The result was a little contentious, since it became a little unclear
what the purpose of the `Candidate` class should even be at that point.

I think this was a case of combining too many things into a single PR.
Thus, this version does not perform any refactoring of that extent. It
does sprinkle in a few `assert`s to make thinks clear for the type
checker.
This commit is contained in:
Benedikt 2025-05-20 11:07:22 +02:00 committed by GitHub
commit 79c87e5886
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 445 additions and 208 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
.github/workflows/ci.yaml vendored
View file

@ -33,7 +33,7 @@ jobs:
if: matrix.platform == 'ubuntu-latest'
run: |
sudo apt update
sudo apt install ffmpeg gobject-introspection libcairo2-dev libgirepository-2.0-dev pandoc
sudo apt install ffmpeg gobject-introspection libcairo2-dev libgirepository-2.0-dev pandoc imagemagick
- name: Get changed lyrics files
id: lyrics-update
@ -60,7 +60,7 @@ jobs:
env:
LYRICS_UPDATED: ${{ steps.lyrics-update.outputs.any_changed }}
run: |
poetry install --extras=autobpm --extras=lyrics --extras=docs --extras=replaygain --extras=reflink
poetry install --extras=autobpm --extras=lyrics --extras=docs --extras=replaygain --extras=reflink --extras=fetchart
poe docs
poe test-with-coverage

31
beets/test/helper.py
View file

@ -886,20 +886,43 @@ class FetchImageHelper:
def run(self, *args, **kwargs):
super().run(*args, **kwargs)
IMAGEHEADER = {
IMAGEHEADER: dict[str, bytes] = {
"image/jpeg": b"\xff\xd8\xff" + b"\x00" * 3 + b"JFIF",
"image/png": b"\211PNG\r\n\032\n",
"image/gif": b"GIF89a",
# dummy type that is definitely not a valid image content type
"image/watercolour": b"watercolour",
"text/html": (
b"<!DOCTYPE html>\n<html>\n<head>\n</head>\n"
b"<body>\n</body>\n</html>"
),
}
def mock_response(self, url, content_type="image/jpeg", file_type=None):
def mock_response(
self,
url: str,
content_type: str = "image/jpeg",
file_type: None | str = None,
) -> None:
# Potentially return a file of a type that differs from the
# server-advertised content type to mimic misbehaving servers.
if file_type is None:
file_type = content_type
try:
# imghdr reads 32 bytes
header = self.IMAGEHEADER[file_type].ljust(32, b"\x00")
except KeyError:
# If we can't return a file that looks like real file of the requested
# type, better fail the test than returning something else, which might
# violate assumption made when writing a test.
raise AssertionError(f"Mocking {file_type} responses not supported")
responses.add(
responses.GET,
url,
content_type=content_type,
# imghdr reads 32 bytes
body=self.IMAGEHEADER.get(file_type, b"").ljust(32, b"\x00"),
body=header,
)

534
beetsplug/fetchart.py
View file

File diff suppressed because it is too large Load diff

6
docs/changelog.rst
View file

@ -35,6 +35,12 @@ For packagers:
``BeetsPlugin.candidates`` method signature since it is never passed in. If
you override this method in your plugin, feel free to remove this parameter.
For plugin developers:
* The `fetchart` plugins has seen a few changes to function signatures and
source registration in the process of introducing typings to the code.
Custom art sources might need to be adapted.
Other changes:
2.3.1 (May 14, 2025)

47
test/plugins/test_art.py
View file

@ -14,8 +14,11 @@
"""Tests for the album art fetchers."""
from __future__ import annotations
import os
import shutil
from typing import TYPE_CHECKING
from unittest.mock import patch
import confuse
@ -37,6 +40,11 @@ from beetsplug import fetchart
logger = logging.getLogger("beets.test_art")
if TYPE_CHECKING:
from collections.abc import Iterator, Sequence
from beets.library import Album
class Settings:
"""Used to pass settings to the ArtSources when the plugin isn't fully
@ -48,6 +56,19 @@ class Settings:
setattr(self, k, v)
class DummyRemoteArtSource(fetchart.RemoteArtSource):
NAME = "Dummy Art Source"
ID = "dummy"
def get(
self,
album: Album,
plugin: fetchart.FetchArtPlugin,
paths: None | Sequence[bytes],
) -> Iterator[fetchart.Candidate]:
return iter(())
class UseThePlugin(CleanupModulesMixin, BeetsTestCase):
modules = (fetchart.__name__, ArtResizer.__module__)
@ -202,9 +223,11 @@ class FetchImageTest(FetchImageTestCase):
def setUp(self):
super().setUp()
self.dpath = os.path.join(self.temp_dir, b"arttest")
self.source = fetchart.RemoteArtSource(logger, self.plugin.config)
self.source = DummyRemoteArtSource(logger, self.plugin.config)
self.settings = Settings(maxwidth=0)
self.candidate = fetchart.Candidate(logger, url=self.URL)
self.candidate = fetchart.Candidate(
logger, self.source.ID, url=self.URL
)
def test_invalid_type_returns_none(self):
self.mock_response(self.URL, "image/watercolour")
@ -432,7 +455,7 @@ class ITunesStoreTest(UseThePlugin):
self.mock_response(fetchart.ITunesStore.API_URL, json)
candidate = next(self.source.get(self.album, self.settings, []))
assert candidate.url == "url_to_the_image"
assert candidate.match == fetchart.Candidate.MATCH_EXACT
assert candidate.match == fetchart.MetadataMatch.EXACT
def test_itunesstore_no_result(self):
json = '{"results": []}'
@ -471,7 +494,7 @@ class ITunesStoreTest(UseThePlugin):
self.mock_response(fetchart.ITunesStore.API_URL, json)
candidate = next(self.source.get(self.album, self.settings, []))
assert candidate.url == "url_to_the_image"
assert candidate.match == fetchart.Candidate.MATCH_FALLBACK
assert candidate.match == fetchart.MetadataMatch.FALLBACK
def test_itunesstore_returns_result_without_artwork(self):
json = """{
@ -727,7 +750,11 @@ class ArtImporterTest(UseThePlugin):
self.art_file = os.path.join(self.temp_dir, b"tmpcover.jpg")
_common.touch(self.art_file)
self.old_afa = self.plugin.art_for_album
self.afa_response = fetchart.Candidate(logger, path=self.art_file)
self.afa_response = fetchart.Candidate(
logger,
source_name="test",
path=self.art_file,
)
def art_for_album(i, p, local_only=False):
return self.afa_response
@ -814,7 +841,11 @@ class ArtImporterTest(UseThePlugin):
def test_do_not_delete_original_if_already_in_place(self):
artdest = os.path.join(os.path.dirname(self.i.path), b"cover.jpg")
shutil.copyfile(syspath(self.art_file), syspath(artdest))
self.afa_response = fetchart.Candidate(logger, path=artdest)
self.afa_response = fetchart.Candidate(
logger,
source_name="test",
path=artdest,
)
self._fetch_art(True)
def test_fetch_art_if_imported_file_deleted(self):
@ -855,7 +886,9 @@ class ArtForAlbumTest(UseThePlugin):
def fs_source_get(_self, album, settings, paths):
if paths:
yield fetchart.Candidate(logger, path=self.image_file)
yield fetchart.Candidate(
logger, source_name=_self.ID, path=self.image_file
)
fetchart.FileSystem.get = fs_source_get

31
test/plugins/test_embedart.py
View file

@ -34,8 +34,35 @@ def require_artresizer_compare(test):
def wrapper(*args, **kwargs):
if not ArtResizer.shared.can_compare:
raise unittest.SkipTest("compare not available")
else:
return test(*args, **kwargs)
# PHASH computation in ImageMagick changed at some point in an
# undocumented way. Check at a low level that comparisons of our
# fixtures give the expected results. Only then, plugin logic tests
# below are meaningful.
# cf. https://github.com/ImageMagick/ImageMagick/discussions/5191
# It would be better to investigate what exactly change in IM and
# handle that in ArtResizer.IMBackend.{can_compare,compare}.
# Skipping the tests as below is a quick fix to CI, but users may
# still see unexpected behaviour.
abbey_artpath = os.path.join(_common.RSRC, b"abbey.jpg")
abbey_similarpath = os.path.join(_common.RSRC, b"abbey-similar.jpg")
abbey_differentpath = os.path.join(_common.RSRC, b"abbey-different.jpg")
compare_threshold = 20
similar_compares_ok = ArtResizer.shared.compare(
abbey_artpath,
abbey_similarpath,
compare_threshold,
)
different_compares_ok = ArtResizer.shared.compare(
abbey_artpath,
abbey_differentpath,
compare_threshold,
)
if not similar_compares_ok or different_compares_ok:
raise unittest.SkipTest("IM version with broken compare")
return test(*args, **kwargs)
wrapper.__name__ = test.__name__
return wrapper