diff --git a/picard/file.py b/picard/file.py index c3a15c37e..b2b4ecb7f 100644 --- a/picard/file.py +++ b/picard/file.py @@ -652,25 +652,35 @@ class File(QtCore.QObject, Item): def is_saved(self): return self.similarity == 1.0 and self.state == File.NORMAL + def _tags_to_update(self, ignored_tags): + for name in set(self.metadata) | set(self.orig_metadata): + if name.startswith('~'): + continue + if not self.supports_tag(name): + continue + if name in ignored_tags: + continue + yield name + def update(self, signal=True): - metadata = self.metadata - names = set(metadata) | set(self.orig_metadata) config = get_config() clear_existing_tags = config.setting["clear_existing_tags"] - ignored_tags = config.setting["compare_ignore_tags"] - for name in names: - if (not name.startswith('~') and self.supports_tag(name) - and name not in ignored_tags): - new_values = metadata.getall(name) - if not (new_values or clear_existing_tags - or name in metadata.deleted_tags): - continue - orig_values = self.orig_metadata.getall(name) - if orig_values != new_values: - self.similarity = self.orig_metadata.compare(metadata, ignored_tags) - if self.state == File.NORMAL: - self.state = File.CHANGED - break + ignored_tags = set(config.setting["compare_ignore_tags"]) + + for name in self._tags_to_update(ignored_tags): + new_values = self.metadata.getall(name) + if not ( + new_values + or clear_existing_tags + or name in self.metadata.deleted_tags + ): + continue + orig_values = self.orig_metadata.getall(name) + if orig_values != new_values: + self.similarity = self.orig_metadata.compare(self.metadata, ignored_tags) + if self.state == File.NORMAL: + self.state = File.CHANGED + break else: if (self.metadata.images and self.orig_metadata.images != self.metadata.images): diff --git a/test/test_file.py b/test/test_file.py index 68f41a054..fa50bab60 100644 --- a/test/test_file.py +++ b/test/test_file.py @@ -23,10 +23,12 @@ import os import re +from types import GeneratorType import unittest from unittest.mock import MagicMock from test.picardtestcase import PicardTestCase +from test.test_coverart_image import create_image from picard import config from picard.const.sys import ( @@ -402,3 +404,191 @@ class FileAdditionalFilesPatternsTest(PicardTestCase): (re.compile('(?s:.*\\.jpg)\\Z', re.IGNORECASE), False), } self.assertEqual(File._compile_move_additional_files_pattern(pattern), expected) + + +class FileUpdateTest(PicardTestCase): + + def setUp(self): + super().setUp() + self.file = File('/somepath/somefile.mp3') + self.INVALIDSIMVAL = 666 + self.file.similarity = self.INVALIDSIMVAL # to check if changed or not + self.file.supports_tag = lambda x: False if x.startswith('unsupported') else True + self.set_config_values({ + 'clear_existing_tags': False, + 'compare_ignore_tags': [], + 'enabled_plugins': [], + }) + + def test_same_image(self): + image = create_image(b'a') + self.file.metadata.images = [image] + self.file.orig_metadata.images = [image] + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) # it should be modified + self.assertEqual(self.file.state, File.NORMAL) + + def test_same_image_pending(self): + image = create_image(b'a') + self.file.metadata.images = [image] + self.file.orig_metadata.images = [image] + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.PENDING) + + def test_same_image_changed_state(self): + image = create_image(b'a') + self.file.metadata.images = [image] + self.file.orig_metadata.images = [image] + self.file.state = File.CHANGED + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.NORMAL) + + def test_changed_image(self): + old_image = create_image(b'a') + new_image = create_image(b'b') + self.file.metadata.images = [new_image] + self.file.orig_metadata.images = [old_image] + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, self.INVALIDSIMVAL) # it shouldbn't be modified + self.assertEqual(self.file.state, File.CHANGED) + + def test_signal(self): + # just for coverage + self.file.update(signal=True) + self.assertEqual(self.file.metadata, Metadata()) + self.assertEqual(self.file.orig_metadata, Metadata()) + + def test_tags_to_update(self): + self.file.orig_metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + 'ignoreme_old': 'a', + '~ignoreme_old': 'b', + 'unsupported_old': 'c', + }) + self.file.metadata = Metadata({ + 'artist': 'someartist', + 'ignoreme_new': 'd', + '~ignoreme_new': 'e', + 'unsupported_new': 'f', + }) + + ignore_tags = {'ignoreme_old', 'ignoreme_new'} + + expected = {'album', 'title', 'artist'} + result = self.file._tags_to_update(ignore_tags) + self.assertIsInstance(result, GeneratorType) + self.assertEqual(set(result), expected) + + def test_unchanged_metadata(self): + self.file.orig_metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + self.file.metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.NORMAL) + + def test_changed_metadata(self): + self.file.orig_metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + self.file.metadata = Metadata({ + 'album': 'somealbum2', + 'title': 'sometitle2', + }) + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertLess(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.CHANGED) + + def test_changed_metadata_pending(self): + self.file.orig_metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + self.file.metadata = Metadata({ + 'album': 'somealbum2', + 'title': 'sometitle2', + }) + + self.file.update(signal=False) + self.assertLess(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.PENDING) # it shouldn't be modified + + def test_clear_existing(self): + self.file.orig_metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + self.file.metadata = Metadata() + self.file.state = File.NORMAL + + config.setting["clear_existing_tags"] = True + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 0.0) + self.assertEqual(self.file.state, File.CHANGED) + + def test_no_new_metadata(self): + self.file.orig_metadata = Metadata({ + 'album': 'somealbum', + 'title': 'sometitle', + }) + self.file.metadata = Metadata() + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.NORMAL) + + def test_tilde_tag(self): + self.file.orig_metadata = Metadata() + self.file.metadata = Metadata({ + '~tag': 'value' + }) + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.NORMAL) + + def test_ignored_tag(self): + self.file.orig_metadata = Metadata() + self.file.metadata = Metadata({ + 'tag': 'value' + }) + self.file.state = File.NORMAL + + config.setting["compare_ignore_tags"] = ['tag'] + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.NORMAL) + + def test_unsupported_tag(self): + self.file.orig_metadata = Metadata() + self.file.metadata = Metadata({ + 'unsupported': 'value' + }) + self.file.state = File.NORMAL + + self.file.update(signal=False) + self.assertEqual(self.file.similarity, 1.0) + self.assertEqual(self.file.state, File.NORMAL)