Skip to content

Commit

Permalink
Accept item in Backend.fetch method
Browse files Browse the repository at this point in the history
Let backends to pick the attributes they need: for example, `album` and
`duration` is only relevant for the LRCLib backend.
  • Loading branch information
snejus committed Sep 20, 2024
1 parent 4d481d7 commit 1ff3ff2
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 38 deletions.
63 changes: 31 additions & 32 deletions beetsplug/lyrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,16 @@
from contextlib import contextmanager
from functools import cached_property
from html import unescape
from typing import Any, ClassVar, Iterator
from typing import TYPE_CHECKING, Any, ClassVar, Iterator
from urllib.parse import urlparse

import requests
from typing_extensions import TypedDict
from unidecode import unidecode

if TYPE_CHECKING:
from beets.library import Item, Library

try:
from bs4 import BeautifulSoup

Expand Down Expand Up @@ -269,7 +272,7 @@ def __init__(self, config, log):
self._log = log
self.config = config

def fetch(self, artist, title, album=None, length=None):
def fetch(self, item: Item) -> str | None:
raise NotImplementedError


Expand Down Expand Up @@ -318,28 +321,22 @@ def pick_lyrics(
"""
return min(data, key=lambda item: cls.get_rank(target_duration, item))

def fetch(
self,
artist: str,
title: str,
album: str | None = None,
length: float = 0.0,
) -> str | None:
"""Fetch lyrics for the given artist, title, and album."""
def fetch(self, item: Item) -> str | None:
"""Fetch lyrics for the given item from the LRCLib API."""
params = {
"artist_name": artist,
"track_name": title,
"album_name": album,
"artist_name": item.artist,
"track_name": item.title,
"album_name": item.album,
}

data: list[LRCLibItem]
if data := self.fetch_json(self.base_url, params=params):
item = self.pick_lyrics(length, data)
lyrics = self.pick_lyrics(item.length, data)

if self.config["synced"] and (synced := item["syncedLyrics"]):
if self.config["synced"] and (synced := lyrics["syncedLyrics"]):
return synced

return item["plainLyrics"]
return lyrics["plainLyrics"]

return None

Expand Down Expand Up @@ -370,8 +367,8 @@ def build_url(self, artist, title):
self._encode(title.title()),
)

def fetch(self, artist, title, album=None, length=None):
url = self.build_url(artist, title)
def fetch(self, item: Item) -> str | None:
url = self.build_url(item.artist, item.title)

html = self.fetch_text(url)
if "We detected that your IP is blocked" in html:
Expand Down Expand Up @@ -412,14 +409,15 @@ def __init__(self, config, log):
self.api_key = config["genius_api_key"].as_str()
self.headers = {"Authorization": f"Bearer {self.api_key}"}

def fetch(self, artist, title, album=None, length=None):
def fetch(self, item: Item) -> str | None:
"""Fetch lyrics from genius.com
Because genius doesn't allow accessing lyrics via the api,
we first query the api for a url matching our artist & title,
then attempt to scrape that url for the lyrics.
"""
json = self._search(artist, title)
artist = item.artist
json = self._search(artist, item.title)

# find a matching artist in the json
for hit in json["response"]["hits"]:
Expand Down Expand Up @@ -513,7 +511,8 @@ def build_url(self, artist, title):
urllib.parse.quote_plus(unidecode(artistitle))
)

def fetch(self, artist, title, album=None, length=None):
def fetch(self, item: Item) -> str | None:
artist, title = item.artist, item.title
search_results = self.fetch_text(self.build_url(title, artist))
song_page_url = self.parse_search_results(search_results)
if not song_page_url:
Expand Down Expand Up @@ -696,7 +695,8 @@ def is_page_candidate(self, url_link, url_title, title, artist):
ratio = difflib.SequenceMatcher(None, song_title, title).ratio()
return ratio >= typo_ratio

def fetch(self, artist, title, album=None, length=None):
def fetch(self, item: Item) -> str | None:
artist, title = item.artist, item.title
query = f"{artist} {title}"
url = "https://www.googleapis.com/customsearch/v1?key=%s&cx=%s&q=%s" % (
self.api_key,
Expand Down Expand Up @@ -966,7 +966,9 @@ def imported(self, session, task):
session.lib, item, False, self.config["force"]
)

def fetch_item_lyrics(self, lib, item, write, force):
def fetch_item_lyrics(
self, lib: Library, item: Item, write: bool, force: bool
) -> None:
"""Fetch and store lyrics for a single item. If ``write``, then the
lyrics will also be written to the file itself.
"""
Expand All @@ -979,10 +981,7 @@ def fetch_item_lyrics(self, lib, item, write, force):
album = item.album

Check failure on line 981 in beetsplug/lyrics.py

View workflow job for this annotation

GitHub Actions / Check linting

local variable 'album' is assigned to but never used
length = round(item.length)

Check failure on line 982 in beetsplug/lyrics.py

View workflow job for this annotation

GitHub Actions / Check linting

local variable 'length' is assigned to but never used
for artist, titles in search_pairs(item):
lyrics = [
self.get_lyrics(artist, title, album=album, length=length)
for title in titles
]
lyrics = [self.get_lyrics(item) for title in titles]
if any(lyrics):
break

Expand Down Expand Up @@ -1011,18 +1010,18 @@ def fetch_item_lyrics(self, lib, item, write, force):
item.try_write()
item.store()

def get_lyrics(self, artist, title, album=None, length=None):
def get_lyrics(self, item: Item) -> str | None:
"""Fetch lyrics, trying each source in turn. Return a string or
None if no lyrics were found.
"""
self.debug("Fetching lyrics for {} - {}", artist, title)
self.debug("Fetching lyrics for {} - {}", item.artist, item.title)
for backend in self.backends.values():
with backend.handle_request():
if lyrics := backend.fetch(
artist, title, album=album, length=length
):
if lyrics := backend.fetch(item):
return lyrics

return None

def append_translation(self, text, to_lang):
from xml.etree import ElementTree

Expand Down
13 changes: 7 additions & 6 deletions test/plugins/test_lyrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,9 @@ def test_backend_source(self, backend):
"""Test default backends with a song known to exist in respective
databases.
"""
title = "Lady Madonna"
res = backend.fetch("The Beatles", title)
assert PHRASE_BY_TITLE[title] in res.lower()
item = Item(artist="The Beatles", title="Lady Madonna")
res = backend.fetch(item)
assert PHRASE_BY_TITLE[item.title] in res.lower()


class TestLyricsPlugin(LyricsPluginMixin):
Expand All @@ -235,7 +235,7 @@ def test_error_handling(
requests_mock.get(lyrics.LRCLib.base_url, **request_kwargs)

plugin = lyrics.LyricsPlugin()
assert plugin.get_lyrics("", "") is None
assert plugin.get_lyrics(Item()) is None
assert caplog.messages
last_log = caplog.messages[-1]
assert last_log
Expand Down Expand Up @@ -422,7 +422,7 @@ def lyrics_match(duration, synced, plain):


class TestLRCLibLyrics(LyricsPluginBackendMixin):
ITEM_DURATION = 999
ITEM_DURATION = 999.0

@pytest.fixture(scope="class")
def backend_name(self):
Expand All @@ -436,7 +436,8 @@ def response_data(self):
def fetch_lyrics(self, backend, requests_mock, response_data):
requests_mock.get(backend.base_url, json=response_data)

return partial(backend.fetch, "la", "la", "la", self.ITEM_DURATION)
item = Item(length=self.ITEM_DURATION)
return partial(backend.fetch, item)

@pytest.mark.parametrize(
"response_data, expected_lyrics",
Expand Down

0 comments on commit 1ff3ff2

Please sign in to comment.