From 273e39bfbccec62042f0f5ed508b06bcca2f8ade Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=A0ar=C5=ABnas=20Nejus?= <snejus@protonmail.com>
Date: Thu, 3 Oct 2024 14:15:56 +0100
Subject: [PATCH] Do not attempt to fetch lyrics with empty data

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.
---
 beetsplug/lyrics.py         | 10 ++++++++--
 docs/changelog.rst          |  3 +++
 test/plugins/test_lyrics.py | 35 +++++++++++++++++++++++++----------
 3 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/beetsplug/lyrics.py b/beetsplug/lyrics.py
index f218be53f0..9558ef08cd 100644
--- a/beetsplug/lyrics.py
+++ b/beetsplug/lyrics.py
@@ -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
@@ -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 = [
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 82eb9f28e1..9c3d4a22a4 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -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:
 
diff --git a/test/plugins/test_lyrics.py b/test/plugins/test_lyrics.py
index 63c94cc08a..af924961fd 100644
--- a/test/plugins/test_lyrics.py
+++ b/test/plugins/test_lyrics.py
@@ -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"]),
         ],