Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4080 #4085

Merged
merged 9 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 34 additions & 9 deletions beetsplug/discogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,32 @@ def candidates(self, items, artist, album, va_likely, extra_tags=None):
self._log.debug('Connection error in album search', exc_info=True)
return []

@staticmethod
def extract_release_id_regex(album_id):
wisp3rwind marked this conversation as resolved.
Show resolved Hide resolved
"""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:
break
if not match:
return None
return int(match.group('id'))
wisp3rwind marked this conversation as resolved.
Show resolved Hide resolved

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.
Expand All @@ -205,15 +231,13 @@ def album_for_id(self, album_id):
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')
Expand All @@ -226,7 +250,8 @@ def album_for_id(self, album_id):
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)

Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
-----------------------

Expand Down
37 changes: 28 additions & 9 deletions test/test_discogs.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,16 @@ def test_parse_position(self):
"""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()
Expand Down Expand Up @@ -355,9 +355,28 @@ def test_parse_release_without_required_fields(self):
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)
wisp3rwind marked this conversation as resolved.
Show resolved Hide resolved
if not match:
match = ''
self.assertEqual(match, expected)


def suite():
return unittest.TestLoader().loadTestsFromName(__name__)


if __name__ == '__main__':
unittest.main(defaultTest='suite')