From 09d771906fe5c4e7f243cd75c5a8a63e6d55f77b Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Tue, 9 Mar 2021 09:16:29 +0100 Subject: [PATCH 1/2] PICARD-2135: Fixed overwriting and deleting WOAR frame --- picard/formats/id3.py | 3 ++- test/formats/test_id3.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index b47a8ae31..10b0493ee 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -489,6 +489,7 @@ class ID3File(File): else: tags.add(id3.WCOP(url=values[0])) elif frameid == 'WOAR' and valid_urls: + tags.delall('WOAR') for url in values: tags.add(id3.WOAR(url=url)) elif frameid.startswith('T') or frameid == 'MVNM': @@ -580,7 +581,7 @@ class ID3File(File): if frame.FrameID == 'POPM' and frame.email == user_email: del tags[key] elif real_name in self.__translate: - del tags[real_name] + tags.delall(real_name) elif name.lower() in self.__rtranslate_freetext_ci: delall_ci(tags, 'TXXX:' + self.__rtranslate_freetext_ci[name.lower()]) elif real_name in self.__translate_freetext: diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 6b5291ea1..17a08d13b 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -454,6 +454,28 @@ class CommonId3Tests: self.assertIn('http://example.com/1', loaded_licenses) self.assertIn('http://example.com/2', loaded_licenses) + @skipUnlessTestfile + def test_woar_not_duplicated(self): + metadata = Metadata({ + 'website': 'http://example.com/1' + }) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertEqual(metadata['website'], loaded_metadata['website']) + metadata['website'] = 'http://example.com/2' + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertEqual(metadata['website'], loaded_metadata['website']) + + @skipUnlessTestfile + def test_woar_delete(self): + metadata = Metadata({ + 'website': 'http://example.com/1' + }) + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertEqual(metadata['website'], loaded_metadata['website']) + del metadata['website'] + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertNotIn('website', loaded_metadata) + class MP3Test(CommonId3Tests.Id3TestCase): testfile = 'test.mp3' From dc1f46946ebe2e6795ab6b160a4f604347147e1c Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Tue, 9 Mar 2021 10:30:10 +0100 Subject: [PATCH 2/2] PICARD-2135: Fixed overwriting and deleting license tags to ID3 --- picard/formats/id3.py | 5 +++++ test/formats/test_id3.py | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/picard/formats/id3.py b/picard/formats/id3.py index 10b0493ee..130896aa9 100644 --- a/picard/formats/id3.py +++ b/picard/formats/id3.py @@ -485,8 +485,10 @@ class ID3File(File): if frameid == 'WCOP': # Only add WCOP if there is only one license URL, otherwise use TXXX:LICENSE if len(values) > 1 or not valid_urls: + tags.delall('WCOP') tags.add(self.build_TXXX(encoding, self.__rtranslate_freetext[name], values)) else: + tags.delall('TXXX:' + self.__rtranslate_freetext[name]) tags.add(id3.WCOP(url=values[0])) elif frameid == 'WOAR' and valid_urls: tags.delall('WOAR') @@ -575,6 +577,9 @@ class ID3File(File): for key, frame in list(tags.items()): if frame.FrameID == 'UFID' and frame.owner == 'http://musicbrainz.org': del tags[key] + elif name == 'license': + tags.delall(real_name) + tags.delall('TXXX:' + self.__rtranslate_freetext[name]) elif real_name == 'POPM': user_email = config.setting['rating_user_email'] for key, frame in list(tags.items()): diff --git a/test/formats/test_id3.py b/test/formats/test_id3.py index 17a08d13b..8e065a355 100644 --- a/test/formats/test_id3.py +++ b/test/formats/test_id3.py @@ -454,6 +454,46 @@ class CommonId3Tests: self.assertIn('http://example.com/1', loaded_licenses) self.assertIn('http://example.com/2', loaded_licenses) + @skipUnlessTestfile + def test_license_upgrade_wcop(self): + tags = mutagen.id3.ID3Tags() + tags.add(mutagen.id3.WCOP(url='http://example.com/1')) + save_raw(self.filename, tags) + metadata = load_metadata(self.filename) + self.assertEqual('http://example.com/1', metadata['license']) + metadata.add('license', 'http://example.com/2') + save_metadata(self.filename, metadata) + raw_metadata = load_raw(self.filename) + self.assertNotIn('WCOP', raw_metadata) + loaded_licenses = [url for url in raw_metadata['TXXX:LICENSE']] + self.assertEqual(['http://example.com/1', 'http://example.com/2'], loaded_licenses) + + @skipUnlessTestfile + def test_license_downgrade_wcop(self): + tags = mutagen.id3.ID3Tags() + licenses = ['http://example.com/1', 'http://example.com/2'] + tags.add(mutagen.id3.TXXX(desc='LICENSE', text=licenses)) + save_raw(self.filename, tags) + raw_metadata = load_raw(self.filename) + metadata = load_metadata(self.filename) + self.assertEqual(licenses, metadata.getall('license')) + metadata['license'] = 'http://example.com/1' + save_metadata(self.filename, metadata) + raw_metadata = load_raw(self.filename) + self.assertEqual('http://example.com/1', raw_metadata['WCOP']) + self.assertNotIn('TXXX:LICENSE', raw_metadata) + + @skipUnlessTestfile + def test_license_delete(self): + tags = mutagen.id3.ID3Tags() + tags.add(mutagen.id3.WCOP(url='http://example.com/1')) + tags.add(mutagen.id3.TXXX(desc='LICENSE', text='http://example.com/2')) + save_raw(self.filename, tags) + metadata = load_metadata(self.filename) + del metadata['license'] + loaded_metadata = save_and_load_metadata(self.filename, metadata) + self.assertNotIn('license', loaded_metadata) + @skipUnlessTestfile def test_woar_not_duplicated(self): metadata = Metadata({