Skip to content

Commit

Permalink
Do not attempt to fetch lyrics with empty data
Browse files Browse the repository at this point in the history
Modified `search_pairs` function in `lyrics.py` to:

* Firstly strip each of `artist`, `artist_sort` and `title` fields
* Only generate alternatives if both `artist` and `title` are not empty
* Ensure that `artist_sort` is not empty and not equal to artist (ignoring
  case) before appending it to the artists

Extended tests to cover the changes.
  • Loading branch information
snejus committed Oct 3, 2024
1 parent 6ef353d commit 273e39b
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 12 deletions.
10 changes: 8 additions & 2 deletions beetsplug/lyrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,13 @@ def generate_alternatives(string, patterns):
alternatives.append(match.group(1))
return alternatives

title, artist, artist_sort = item.title, item.artist, item.artist_sort
title, artist, artist_sort = (
item.title.strip(),
item.artist.strip(),
item.artist_sort.strip(),
)
if not title or not artist:
return ()

patterns = [
# Remove any featuring artists from the artists name
Expand All @@ -172,7 +178,7 @@ def generate_alternatives(string, patterns):
artists = generate_alternatives(artist, patterns)
# Use the artist_sort as fallback only if it differs from artist to avoid
# repeated remote requests with the same search terms
if artist != artist_sort:
if artist_sort and artist.lower() != artist_sort.lower():
artists.append(artist_sort)

patterns = [
Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ Bug fixes:
configuration for each test case. This fixes the issue where some tests
failed because they read developer's local lyrics configuration.
:bug:`5133`
* :doc:`plugins/lyrics`: Do not attempt to search for lyrics if either the
artist or title is missing and ignore ``artist_sort`` value if it is empty.
:bug:`2635`

For packagers:

Expand Down
35 changes: 25 additions & 10 deletions test/plugins/test_lyrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,36 @@ def xfail_on_ci(msg: str) -> pytest.MarkDecorator:


class TestLyricsUtils:
unexpected_empty_artist = pytest.mark.xfail(
reason="Empty artist '' should not be present"
@pytest.mark.parametrize(
"artist, title",
[
("Artist", ""),
("", "Title"),
(" ", ""),
("", " "),
("", ""),
],
)
def test_search_empty(self, artist, title):
actual_pairs = lyrics.search_pairs(Item(artist=artist, title=title))

assert not list(actual_pairs)

@pytest.mark.parametrize(
"artist, artist_sort, expected_extra_artists",
[
_p("Alice ft. Bob", "", ["Alice"], marks=unexpected_empty_artist),
_p("Alice feat Bob", "", ["Alice"], marks=unexpected_empty_artist),
_p("Alice feat. Bob", "", ["Alice"], marks=unexpected_empty_artist),
_p("Alice feats Bob", "", [], marks=unexpected_empty_artist),
_p("Alice featuring Bob", "", ["Alice"], marks=unexpected_empty_artist),
_p("Alice & Bob", "", ["Alice"], marks=unexpected_empty_artist),
_p("Alice and Bob", "", ["Alice"], marks=unexpected_empty_artist),
_p("Alice", "", [], marks=unexpected_empty_artist),
("Alice ft. Bob", "", ["Alice"]),
("Alice feat Bob", "", ["Alice"]),
("Alice feat. Bob", "", ["Alice"]),
("Alice feats Bob", "", []),
("Alice featuring Bob", "", ["Alice"]),
("Alice & Bob", "", ["Alice"]),
("Alice and Bob", "", ["Alice"]),
("Alice", "", []),
("Alice", "Alice", []),
("Alice", "alice", []),
("Alice", "alice ", []),
("Alice", "Alice A", ["Alice A"]),
("CHVRCHΞS", "CHVRCHES", ["CHVRCHES"]),
("横山克", "Masaru Yokoyama", ["Masaru Yokoyama"]),
],
Expand Down

0 comments on commit 273e39b

Please sign in to comment.