diff --git a/picard/mbjson.py b/picard/mbjson.py index cb8a96389..be8380b9d 100644 --- a/picard/mbjson.py +++ b/picard/mbjson.py @@ -27,7 +27,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -from collections import namedtuple +from types import SimpleNamespace from picard import log from picard.config import get_config @@ -205,38 +205,58 @@ def _relations_to_metadata_target_type_series(relation, m, context): entity = context.entity series = relation['series'] var_prefix = f'~{entity}_' if entity else '~' - m.add(f'{var_prefix}series', series['name']) - m.add(f'{var_prefix}seriesid', series['id']) - m.add(f'{var_prefix}seriescomment', series['disambiguation']) - m.add(f'{var_prefix}seriesnumber', relation['attribute-values'].get('number', '')) + name = f'{var_prefix}series' + mbid = f'{var_prefix}seriesid' + comment = f'{var_prefix}seriescomment' + number = f'{var_prefix}seriesnumber' + if not context.metadata_was_cleared['series']: + # Clear related metadata first to prevent accumulation + # of identical value, see PICARD-2700 issue + m.unset(name) + m.unset(mbid) + m.unset(comment) + m.unset(number) + # That's to ensure it is done only once + context.metadata_was_cleared['series'] = True + m.add(name, series['name']) + m.add(mbid, series['id']) + m.add(comment, series['disambiguation']) + m.add(number, relation['attribute-values'].get('number', '')) + + +class RelFunc(SimpleNamespace): + clear_metadata_first = False + func = None _RELATIONS_TO_METADATA_TARGET_TYPE_FUNC = { - 'artist': _relations_to_metadata_target_type_artist, - 'series': _relations_to_metadata_target_type_series, - 'url': _relations_to_metadata_target_type_url, - 'work': _relations_to_metadata_target_type_work, + 'artist': RelFunc(func=_relations_to_metadata_target_type_artist), + 'series': RelFunc( + func=_relations_to_metadata_target_type_series, + clear_metadata_first=True + ), + 'url': RelFunc(func=_relations_to_metadata_target_type_url), + 'work': RelFunc(func=_relations_to_metadata_target_type_work), } -TargetTypeFuncContext = namedtuple( - 'TargetTypeFuncContext', - 'config entity instrumental use_credited_as use_instrument_credits' -) - - def _relations_to_metadata(relations, m, instrumental=False, config=None, entity=None): config = config or get_config() - context = TargetTypeFuncContext( - config, - entity, - instrumental, - not config.setting['standardize_artists'], - not config.setting['standardize_instruments'], + context = SimpleNamespace( + config=config, + entity=entity, + instrumental=instrumental, + use_credited_as=not config.setting['standardize_artists'], + use_instrument_credits=not config.setting['standardize_instruments'], + metadata_was_cleared=dict(), ) for relation in relations: - if relation['target-type'] in _RELATIONS_TO_METADATA_TARGET_TYPE_FUNC: - _RELATIONS_TO_METADATA_TARGET_TYPE_FUNC[relation['target-type']](relation, m, context) + target = relation['target-type'] + if target in _RELATIONS_TO_METADATA_TARGET_TYPE_FUNC: + relfunc = _RELATIONS_TO_METADATA_TARGET_TYPE_FUNC[target] + if target not in context.metadata_was_cleared: + context.metadata_was_cleared[target] = not relfunc.clear_metadata_first + relfunc.func(relation, m, context) def _locales_from_aliases(aliases): diff --git a/test/test_mbjson.py b/test/test_mbjson.py index eefc641ee..f466fc545 100644 --- a/test/test_mbjson.py +++ b/test/test_mbjson.py @@ -206,6 +206,46 @@ class ReleaseTest(MBJSONTest): ]) self.assertEqual(m.getall('~releasegroup_seriesnumber'), ['15', '291']) + def test_release_group_rels_double(self): + m = Metadata() + release_group_to_metadata(self.json_doc['release-group'], m) + + # load it twice and check for duplicates + release_group_to_metadata(self.json_doc['release-group'], m) + self.assertEqual(m.getall('~releasegroup_series'), [ + "Absolute Radio's The 100 Collection", + '1001 Albums You Must Hear Before You Die' + ]) + self.assertEqual(m.getall('~releasegroup_seriesid'), [ + '4bf41050-6fa9-41a6-8398-15bdab4b0352', + '4bc2a338-e1d8-4546-8a61-640da8aaf888' + ]) + self.assertEqual(m.getall('~releasegroup_seriescomment'), [ + '2005 edition' + ]) + self.assertEqual(m.getall('~releasegroup_seriesnumber'), ['15', '291']) + + def test_release_group_rels_removed(self): + m = Metadata() + release_group_to_metadata(self.json_doc['release-group'], m) + + # remove one of the series from original metadata + for i, rel in enumerate(self.json_doc['release-group']['relations']): + if not rel['type'] == 'part of': + continue + if rel['series']['name'] == '1001 Albums You Must Hear Before You Die': + del self.json_doc['release-group']['relations'][i] + break + release_group_to_metadata(self.json_doc['release-group'], m) + self.assertEqual(m.getall('~releasegroup_series'), [ + "Absolute Radio's The 100 Collection", + ]) + self.assertEqual(m.getall('~releasegroup_seriesid'), [ + '4bf41050-6fa9-41a6-8398-15bdab4b0352', + ]) + self.assertEqual(m.getall('~releasegroup_seriescomment'), []) + self.assertEqual(m.getall('~releasegroup_seriesnumber'), ['15']) + class NullReleaseTest(MBJSONTest):