Repository: beetbox/beets

Issue:
discogs: Parse IDs from new release URL format
<!--
Describe your problem, feature request, or discussion topic here.

Please fill out this and the "Setup" section below and remember to include
enough detail so that other people can reproduce the problem.
-->

### Problem

- With some files located in directory `at` with no tags
- With discogs plugin enabled

Running this command in verbose (`-vv`) mode:

```sh
$ beet -vv import at
```
Then entering `i` to enter release ID, for example: https://www.discogs.com/release/18377836-Aphex-Twin-Come-To-Daddy

Led to this problem:

```
Enter release ID: https://www.discogs.com/release/18377836-Aphex-Twin-Come-To-Daddy
Tagging  - 
Searching for album ID: https://www.discogs.com/release/18377836-Aphex-Twin-Come-To-Daddy
Requesting MusicBrainz release https://www.discogs.com/release/18377836-Aphex-Twin-Come-To-Daddy
Invalid MBID (https://www.discogs.com/release/18377836-Aphex-Twin-Come-To-Daddy).
discogs: Searching for release https://www.discogs.com/release/18377836-Aphex-Twin-Come-To-Daddy
Evaluating 0 candidates.
Sending event: before_choose_candidate
No matching release found for 4 tracks.
For help, see: https://beets.readthedocs.org/en/latest/faq.html#nomatch
```

Here's a link to the music files that trigger the bug (if relevant): N/A - any music files will do

Here is some verbose logging while beets was starting up:

```
 0.87
Sending event: albuminfo_received
Candidate: Skrewdriver - Boots And Braces (1654191)
Computing track assignment...
...done.
Success. Distance: 0.85
Sending event: albuminfo_received
Candidate: Saint Etienne - Continental (1778268)
Computing track assignment...
...done.
Success. Distance: 0.88
Sending event: albuminfo_received
Candidate: X-102 - Titan EP (1393615)
Computing track assignment...
...done.
Success. Distance: 0.83
Evaluating 5 candidates.
```
Note that the `search` option worked. 

```
Enter search, enter Id, aBort, eDit, edit Candidates? e
Artist: Aphex Twin
Album: Come to daddy
Tagging  - 
No album ID found.
Search terms: Aphex Twin - Come to daddy
Album might be VA: False
Searching for MusicBrainz releases with: {'release': 'come to daddy', 'artist': 'aphex twin', 'tracks': '4'}
Requesting MusicBrainz release f7a09aff-6a8c-49c1-b8fb-4d2e93bebd6b
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
primary MB release type: single
Sending event: mb_album_extract
Sending event: albuminfo_received
Candidate: Aphex Twin - Come to Daddy (f7a09aff-6a8c-49c1-b8fb-4d2e93bebd6b)
Computing track assignment...
...done.
Success. Distance: 0.76
Requesting MusicBrainz release 9197ad55-4c0a-3cb3-b190-e1f9062b867c
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
primary MB release type: single
Sending event: mb_album_extract
Sending event: albuminfo_received
Candidate: Aphex Twin - Come to Daddy (9197ad55-4c0a-3cb3-b190-e1f9062b867c)
Computing track assignment...
...done.
Success. Distance: 0.76
Requesting MusicBrainz release 32ad4a8c-cd44-3637-ac39-3479d7be8fb2
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
primary MB release type: ep
Sending event: mb_album_extract
Sending event: albuminfo_received
Candidate: Aphex Twin - Come to Daddy (32ad4a8c-cd44-3637-ac39-3479d7be8fb2)
Computing track assignment...
...done.
Success. Distance: 0.81
Requesting MusicBrainz release c8d680de-3bbd-3546-bf0f-ac151e079c1b
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
primary MB release type: ep
Sending event: mb_album_extract
Sending event: albuminfo_received
Candidate: Aphex Twin - Come to Daddy (c8d680de-3bbd-3546-bf0f-ac151e079c1b)
Computing track assignment...
...done.
Success. Distance: 0.81
Requesting MusicBrainz release 0931f506-5138-3c27-b561-fc95f0d813a9
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
Sending event: mb_track_extract
primary MB release type: ep
Sending event: mb_album_extract
Sending event: albuminfo_received
Candidate: Aphex Twin - Come to Daddy (0931f506-5138-3c27-b561-fc95f0d813a9)
Computing track assignment...
...done.
Success. Distance: 0.81
discogs: Searching for master release 27457
discogs: hit rate limit, waiting for 1.8842348098754882 seconds
discogs: Searching for master release 27457
discogs: hit rate limit, waiting for 2.157538557052612 seconds
discogs: Searching for master release 27457
discogs: hit rate limit, waiting for 2.085971975326538 seconds
discogs: Searching for master release 27457
discogs: hit rate limit, waiting for 1.992177629470825 seconds
discogs: Searching for master release 27457
discogs: hit rate limit, waiting for 2.0841585636138915 seconds
Sending event: albuminfo_received
Candidate: Aphex Twin - Come To Daddy (3662)
Computing track assignment...
...done.
Success. Distance: 0.69
Sending event: albuminfo_received
Candidate: Aphex Twin - Come To Daddy (3903368)
Computing track assignment...
...done.
Success. Distance: 0.69
Sending event: albuminfo_received
Candidate: Aphex Twin - Come To Daddy (6157)
Computing track assignment...
...done.
Success. Distance: 0.78
Sending event: albuminfo_received
Candidate: Aphex Twin - Come To Daddy (52731)
Computing track assignment...
...done.
Success. Distance: 0.78
Sending event: albuminfo_received
Candidate: Aphex Twin - Come To Daddy (10818482)
Computing track assignment...
...done.
Success. Distance: 0.69
Evaluating 10 candidates.
Sending event: before_choose_candidate
Finding tags for album " - ".
Candidates:
1. Aphex Twin - Come To Daddy (31.2%) (tracks, album, artist, ...) (Discogs, Vinyl, 1997, UK, Warp Records, WAP94)
2. Aphex Twin - Come To Daddy (31.2%) (tracks, album, artist, ...) (Discogs, Vinyl, 2012, UK, Warp Records, WAP94)
3. Aphex Twin - Come To Daddy (31.2%) (tracks, album, artist, ...) (Discogs, Vinyl, 2017, UK, Warp Records, WAP94)
4. Aphex Twin - Come to Daddy (23.8%) (tracks, album, artist) (CD, 1997, GB, Warp, WAP94CD)
5. Aphex Twin - Come to Daddy (23.8%) (tracks, album, artist) (12" Vinyl, 1997, GB, Warp, WAP94)
6. Aphex Twin - Come To Daddy (22.1%) (tracks, missing tracks, album, ...) (Discogs, CD, 1997, UK, Warp Records, WAP94CDX)
7. Aphex Twin - Come To Daddy (22.1%) (tracks, missing tracks, album, ...) (Discogs, CD, 1997, US, Sire, 31001)
8. Aphex Twin - Come to Daddy (18.9%) (tracks, missing tracks, album, ...) (CD, 1997, DE, Rough Trade, RTD126.3383.2)
9. Aphex Twin - Come to Daddy (18.9%) (tracks, missing tracks, album, ...) (CD, 1997, JP, Sire Records, WPCR-1603)
10. Aphex Twin - Come to Daddy (18.9%) (tracks, missing tracks, album, ...) (CD, 1997, US, Sire Records, 31001)
```

### Setup

* OS: PopOS
* Python version: 3.9.5
* beets version: 1.5.0
* Turning off plugins made problem go away (yes/no): no

<!--
You can turn off plugins temporarily by passing --plugins= on the command line:

$ beet --plugins= version
-->

My configuration (output of `beet config`) is:

```yaml
plugins: edit discogs inline info badfiles missing embedart fetchart
directory: /mnt/nfs/Backup/media/vinyl
library: /mnt/nfs/Backup/media/beets-db/beets-vinyl.db

import:
    move: no
    copy: yes
    timid: yes

paths:
    default: $albumartist/$album%aunique{}/%if{$multidisc,CD$disc/}$track $artist - $title
    comp: Compilations/$album%aunique{}/%if{$multidisc,CD$disc/}$track $artist - $title
    singleton: Singles/$artist - $title
item_fields:
    multidisc: 1 if disctotal > 1 and media == 'CD' else 0
original_date: yes
threaded: yes
fetchart:
    cover_names: cover
    minwidth: 500
    store_source: yes
    auto: yes
    maxwidth: 0
    quality: 0
    max_filesize: 0
    enforce_ratio: no
    cautious: no
    sources:
    - filesystem
    - coverart
    - itunes
    - amazon
    - albumart
    google_key: REDACTED
    google_engine: 001442825323518660753:hrh5ch1gjzm
    fanarttv_key: REDACTED
    lastfm_key: REDACTED
    high_resolution: no
embedart:
    maxwidth: 0
    auto: yes
    compare_threshold: 0
    ifempty: no
    remove_art_file: no
    quality: 0
pathfields: {}
album_fields: {}
missing:
    count: no
    total: no
    album: no
edit:
    albumfields: album albumartist
    itemfields: track title artist album
    ignore_fields: id path
discogs:
    apikey: REDACTED
    apisecret: REDACTED
    tokenfile: discogs_token.json
    source_weight: 0.5
    user_token: REDACTED
    separator: ', '
    index_tracks: no
```

I just tried setting my own `discogs.apikey` and `discogs.apisecret` and I received the same `hit rate limit` output as above. 

Might be worth mentioning that this was working 2 days ago, I hadn't changed anything in my setup between then. My `requirements.txt` file is:

```
audioread==2.1.9
beautifulsoup4==4.9.3
beetcamp==0.8.0
beets==1.5.0
cached-property==1.5.2
certifi==2020.12.5
chardet==4.0.0
confuse==1.4.0
discogs-client==2.3.0
idna==2.10
isodate==0.6.0
jellyfish==0.8.2
mediafile==0.6.0
munkres==1.1.4
musicbrainzngs==0.7.1
mutagen==1.45.1
oauthlib==3.1.0
packaging==20.9
Pillow==8.1.0
pyacoustid==1.2.0
pycountry==20.7.3
pyparsing==2.4.7
PyYAML==5.4.1
requests==2.25.1
requests-oauthlib==1.3.0
six==1.15.0
soupsieve==2.2
Unidecode==1.2.0
urllib3==1.26.3
```


Discussion:

Patch:
diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py
index 76036f1be..90486850a 100644
--- a/beetsplug/discogs.py
+++ b/beetsplug/discogs.py
@@ -197,6 +197,31 @@ class DiscogsPlugin(BeetsPlugin):
             self._log.debug('Connection error in album search', exc_info=True)
             return []
 
+    @staticmethod
+    def extract_release_id_regex(album_id):
+        """Returns the Discogs_id or None."""
+        # Discogs-IDs are simple integers. In order to avoid confusion with
+        # other metadata plugins, we only look for very specific formats of the
+        # input string:
+        # - plain integer, optionally wrapped in brackets and prefixed by an
+        #   'r', as this is how discogs displays the release ID on its webpage.
+        # - legacy url format: discogs.com/<name of release>/release/<id>
+        # - current url format: discogs.com/release/<id>-<name of release>
+        # See #291, #4080 and #4085 for the discussions leading up to these
+        # patterns.
+        # Regex has been tested here https://regex101.com/r/wyLdB4/2
+
+        for pattern in [
+                r'^\[?r?(?P<id>\d+)\]?$',
+                r'discogs\.com/release/(?P<id>\d+)-',
+                r'discogs\.com/[^/]+/release/(?P<id>\d+)',
+        ]:
+            match = re.search(pattern, album_id)
+            if match:
+                return int(match.group('id'))
+
+        return None
+
     def album_for_id(self, album_id):
         """Fetches an album by its Discogs ID and returns an AlbumInfo object
         or None if the album is not found.
@@ -205,15 +230,13 @@ class DiscogsPlugin(BeetsPlugin):
             return
 
         self._log.debug('Searching for release {0}', album_id)
-        # Discogs-IDs are simple integers. We only look for those at the end
-        # of an input string as to avoid confusion with other metadata plugins.
-        # An optional bracket can follow the integer, as this is how discogs
-        # displays the release ID on its webpage.
-        match = re.search(r'(^|\[*r|discogs\.com/.+/release/)(\d+)($|\])',
-                          album_id)
-        if not match:
+
+        discogs_id = self.extract_release_id_regex(album_id)
+
+        if not discogs_id:
             return None
-        result = Release(self.discogs_client, {'id': int(match.group(2))})
+
+        result = Release(self.discogs_client, {'id': discogs_id})
         # Try to obtain title to verify that we indeed have a valid Release
         try:
             getattr(result, 'title')
@@ -226,7 +249,8 @@ class DiscogsPlugin(BeetsPlugin):
                     return self.album_for_id(album_id)
             return None
         except CONNECTION_ERRORS:
-            self._log.debug('Connection error in album lookup', exc_info=True)
+            self._log.debug('Connection error in album lookup',
+                            exc_info=True)
             return None
         return self.get_album_info(result)
 
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 9581c820f..8bad04728 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -39,6 +39,9 @@ Bug fixes:
 * :doc:`/plugins/lyrics`: Fix crash bug when beautifulsoup4 is not installed.
   :bug:`4027`
 
+* :doc:`/plugins/discogs`: Adapt regex to new URL format .
+  :bug: `4080`
+
 1.5.0 (August 19, 2021)
 -----------------------
 


Test patch:
diff --git a/test/test_discogs.py b/test/test_discogs.py
index 8f1b07721..4e62e7124 100644
--- a/test/test_discogs.py
+++ b/test/test_discogs.py
@@ -172,16 +172,16 @@ class DGAlbumInfoTest(_common.TestCase):
         """Test the conversion of discogs `position` to medium, medium_index
         and subtrack_index."""
         # List of tuples (discogs_position, (medium, medium_index, subindex)
-        positions = [('1',       (None,   '1',  None)),
-                     ('A12',     ('A',    '12', None)),
-                     ('12-34',   ('12-',  '34', None)),
-                     ('CD1-1',   ('CD1-', '1',  None)),
-                     ('1.12',    (None,   '1',  '12')),
-                     ('12.a',    (None,   '12', 'A')),
-                     ('12.34',   (None,   '12', '34')),
-                     ('1ab',     (None,   '1',  'AB')),
+        positions = [('1', (None, '1', None)),
+                     ('A12', ('A', '12', None)),
+                     ('12-34', ('12-', '34', None)),
+                     ('CD1-1', ('CD1-', '1', None)),
+                     ('1.12', (None, '1', '12')),
+                     ('12.a', (None, '12', 'A')),
+                     ('12.34', (None, '12', '34')),
+                     ('1ab', (None, '1', 'AB')),
                      # Non-standard
-                     ('IV',      ('IV',   None, None)),
+                     ('IV', ('IV', None, None)),
                      ]
 
         d = DiscogsPlugin()
@@ -355,9 +355,28 @@ class DGAlbumInfoTest(_common.TestCase):
         self.assertEqual(d, None)
         self.assertIn('Release does not contain the required fields', logs[0])
 
+    def test_album_for_id(self):
+        """Test parsing for a valid Discogs release_id"""
+        test_patterns = [('http://www.discogs.com/G%C3%BCnther-Lause-Meru-Ep/release/4354798', 4354798),  # NOQA E501
+                         ('http://www.discogs.com/release/4354798-G%C3%BCnther-Lause-Meru-Ep', 4354798),  # NOQA E501
+                         ('http://www.discogs.com/G%C3%BCnther-4354798Lause-Meru-Ep/release/4354798', 4354798),  # NOQA E501
+                         ('http://www.discogs.com/release/4354798-G%C3%BCnther-4354798Lause-Meru-Ep/', 4354798),  # NOQA E501
+                         ('[r4354798]', 4354798),
+                         ('r4354798', 4354798),
+                         ('4354798', 4354798),
+                         ('yet-another-metadata-provider.org/foo/12345', ''),
+                         ('005b84a0-ecd6-39f1-b2f6-6eb48756b268', ''),
+                         ]
+        for test_pattern, expected in test_patterns:
+            match = DiscogsPlugin.extract_release_id_regex(test_pattern)
+            if not match:
+                match = ''
+            self.assertEqual(match, expected)
+
 
 def suite():
     return unittest.TestLoader().loadTestsFromName(__name__)
 
+
 if __name__ == '__main__':
     unittest.main(defaultTest='suite')