From f4907ca5d03e1e998b172d2c6e64f8bc3d467e86 Mon Sep 17 00:00:00 2001 From: Diego Moreda Date: Mon, 10 Oct 2016 19:59:42 +0200 Subject: [PATCH] discogs: handle nested subtracks, add try block Modify coalesce_tracks() in order to handle the case where subtracks are defined inside an index track (as `sub_tracks` attribute), reorganizing the if logic to avoid duplicated code. Add a try..catch block enclosing the call to clean_tracklist, as a measure for avoiding side effects (and reverting to just parsing the raw_list if there are any problems). --- beetsplug/discogs.py | 39 +++++++++++++++++++++++++-------------- test/test_discogs.py | 15 +++++++++++++++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/beetsplug/discogs.py b/beetsplug/discogs.py index 9c37578b6..1e4003c97 100644 --- a/beetsplug/discogs.py +++ b/beetsplug/discogs.py @@ -34,6 +34,7 @@ import time import json import socket import os +import traceback from string import ascii_lowercase @@ -275,7 +276,15 @@ class DiscogsPlugin(BeetsPlugin): def get_tracks(self, tracklist): """Returns a list of TrackInfo objects for a discogs tracklist. """ - clean_tracklist = self.coalesce_tracks(tracklist) + try: + clean_tracklist = self.coalesce_tracks(tracklist) + except Exception as exc: + # FIXME: this is an extra precaution for making sure there are no + # side effects after #2222. It should be removed after further + # testing. + self._log.debug(u'{}', traceback.format_exc()) + self._log.error(u'uncaught exception in coalesce_tracks: {}', exc) + clean_tracklist = tracklist tracks = [] index_tracks = {} index = 0 @@ -369,6 +378,7 @@ class DiscogsPlugin(BeetsPlugin): tracklist = [] prev_subindex = '' for track in raw_tracklist: + # Regular subtrack (track with subindex). if track['position']: _, _, subindex = self.get_track_index(track['position']) if subindex: @@ -380,20 +390,21 @@ class DiscogsPlugin(BeetsPlugin): add_merged_subtracks(tracklist, subtracks) subtracks = [track] prev_subindex = subindex.rjust(len(raw_tracklist)) - else: - # Regular track. - if subtracks: - add_merged_subtracks(tracklist, subtracks) - subtracks = [] - prev_subindex = '' - tracklist.append(track) - else: - # Index track. - if subtracks: - add_merged_subtracks(tracklist, subtracks) - subtracks = [] - prev_subindex = '' + continue + + # Index track with nested sub_tracks. + if not track['position'] and 'sub_tracks' in track: + # Append the index track, assuming it contains the track title. tracklist.append(track) + add_merged_subtracks(tracklist, track['sub_tracks']) + continue + + # Regular track or index track without nested sub_tracks. + if subtracks: + add_merged_subtracks(tracklist, subtracks) + subtracks = [] + prev_subindex = '' + tracklist.append(track) # Merge and add the remaining subtracks, if any. if subtracks: diff --git a/test/test_discogs.py b/test/test_discogs.py index be81fc9af..0dff77072 100644 --- a/test/test_discogs.py +++ b/test/test_discogs.py @@ -236,6 +236,21 @@ class DGAlbumInfoTest(_common.TestCase): self.assertEqual(len(d.tracks), 1) self.assertEqual(d.tracks[0].title, 'TRACK GROUP TITLE') + def test_parse_tracklist_subtracks_nested(self): + """Test parsing of subtracks defined inside a index track.""" + release = self._make_release_from_positions(['1', '', '3']) + # Track 2: Index track with track group title, and sub_tracks + release.data['tracklist'][1]['title'] = 'TRACK GROUP TITLE' + release.data['tracklist'][1]['sub_tracks'] = [ + self._make_track('TITLE ONE', '2.1', '01:01'), + self._make_track('TITLE TWO', '2.2', '02:02') + ] + + d = DiscogsPlugin().get_album_info(release) + self.assertEqual(d.mediums, 1) + self.assertEqual(len(d.tracks), 3) + self.assertEqual(d.tracks[1].title, 'TRACK GROUP TITLE') + def suite(): return unittest.TestLoader().loadTestsFromName(__name__)