Metadata class: inherit from MutableMapping instead of dict

picard/metadata.py

@@ -17,6 +17,8 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
 # USA.
+from collections.abc import MutableMapping
+
 from PyQt5.QtCore import QObject
 
 from picard import config
@@ -45,7 +47,7 @@ MULTI_VALUED_JOINER = '; '
 LENGTH_SCORE_THRES_MS = 30000
 
 
-class Metadata(dict):
+class Metadata(MutableMapping):
 
     """List of metadata items with dict-like access."""
 
@@ -59,15 +61,30 @@ class Metadata(dict):
 
     multi_valued_joiner = MULTI_VALUED_JOINER
 
-    def __init__(self):
+    def __init__(self, *args, deleted_tags=None, images=None, length=None, **kwargs):
-        super().__init__()
+        self._store = dict()
-        self.images = ImageList()
-        self.has_common_images = True
         self.deleted_tags = set()
         self.length = 0
+        self.images = ImageList()
+        self.has_common_images = True
+
+        d = dict(*args, **kwargs)
+        for k, v in d.items():
+            self[k] = v
+        if images is not None:
+            for image in images:
+                self.images.append(image)
+        if deleted_tags is not None:
+            for tag in deleted_tags:
+                del self[tag]
+        if length is not None:
+            self.length = int(length)
 
     def __bool__(self):
-        return bool(len(self) or len(self.images))
+        return bool(len(self))
+
+    def __len__(self):
+        return len(self._store) + len(self.images)
 
     def append_image(self, coverartimage):
         self.images.append(coverartimage)
@@ -268,20 +285,23 @@ class Metadata(dict):
         self.update(other)
 
     def update(self, other):
-        for key in other.keys():
+        if isinstance(other, self.__class__):
-            self.set(key, other.getall(key)[:])
+            for k, v in other._store.items():
-        if other.images:
+                self._store[k] = v[:]
-            self.images = other.images[:]
+            if other.images:
-        if other.length:
+                self.images = other.images[:]
-            self.length = other.length
+            if other.length:
+                self.length = other.length
 
-        self.deleted_tags.update(other.deleted_tags)
+            # Remove deleted tags from UI on save
-        # Remove deleted tags from UI on save
+            for tag in other.deleted_tags:
-        for tag in other.deleted_tags:
+                del self[tag]
-            self.pop(tag, None)
+        elif isinstance(other, dict):
+            for k, v in other.items():
+                self[k] = v
 
     def clear(self):
-        super().clear()
+        self._store.clear()
         self.images = ImageList()
         self.length = 0
         self.clear_deleted()
@@ -290,49 +310,62 @@ class Metadata(dict):
         self.deleted_tags = set()
 
     def getall(self, name):
-        return super().get(name, [])
+        return self._store.get(name, [])
+
+    def getraw(self, name):
+        return self._store[name]
 
     def get(self, name, default=None):
-        values = super().get(name, None)
+        values = self._store.get(name, None)
         if values:
             return self.multi_valued_joiner.join(values)
         else:
             return default
 
+    def __contains__(self, name):
+        return self._store.__contains__(name)
+
     def __getitem__(self, name):
         return self.get(name, '')
 
     def set(self, name, values):
-        super().__setitem__(name, values)
+        self._store[name] = values
-        if name in self.deleted_tags:
+        self.deleted_tags.discard(name)
-            self.deleted_tags.remove(name)
 
     def __setitem__(self, name, values):
         if not isinstance(values, list):
             values = [values]
         values = [str(value) for value in values if value]
-        if len(values):
+        if values:
             self.set(name, values)
-        elif name in self:
+        elif name in self._store:
-            self.delete(name)
+            del self[name]
+
+    def __delitem__(self, name):
+        try:
+            del self._store[name]
+        except KeyError:
+            pass
+        finally:
+            self.deleted_tags.add(name)
 
     def add(self, name, value):
         if value or value == 0:
-            self.setdefault(name, []).append(value)
+            self._store.setdefault(name, []).append(value)
-            if name in self.deleted_tags:
+            self.deleted_tags.discard(name)
-                self.deleted_tags.remove(name)
 
     def add_unique(self, name, value):
         if value not in self.getall(name):
             self.add(name, value)
 
     def delete(self, name):
-        if name in self:
+        del self[name]
-            self.pop(name, None)
+
-        self.deleted_tags.add(name)
+    def __iter__(self):
+        return iter(self._store)
 
     def items(self):
-        for name, values in super().items():
+        for name, values in self._store.items():
             for value in values:
                 yield name, value
 
@@ -342,12 +375,12 @@ class Metadata(dict):
         >>> m.rawitems()
         [("key1", ["value1", "value2"]), ("key2", ["value3"])]
         """
-        return dict.items(self)
+        return self._store.items()
 
     def apply_func(self, func):
-        for key, values in self.rawitems():
+        for name, values in self.rawitems():
-            if key not in PRESERVED_TAGS:
+            if name not in PRESERVED_TAGS:
-                super().__setitem__(key, [func(value) for value in values])
+                self[name] = [func(value) for value in values]
 
     def strip_whitespace(self):
         """Strip leading/trailing whitespace.
@@ -362,6 +395,12 @@ class Metadata(dict):
         """
         self.apply_func(lambda s: s.strip())
 
+    def __repr__(self):
+        return "%s(%r, deleted_tags=%r, length=%r, images=%r)" % (self.__class__.__name__, self._store, self.deleted_tags, self.length, self.images)
+
+    def __str__(self):
+        return ("store: %r\ndeleted: %r\nimages: %r\nlength: %r" % (self._store, self.deleted_tags, self.images, self.length))
+
 
 _album_metadata_processors = PluginFunctions()
 _track_metadata_processors = PluginFunctions()

test/test_metadata.py

@@ -38,28 +38,31 @@ class MetadataTest(PicardTestCase):
         pass
 
     def test_metadata_setitem(self):
-        self.assertEqual(["single1-value"], dict.get(self.metadata, "single1"))
+        self.assertEqual(["single1-value"], self.metadata.getraw("single1"))
-        self.assertEqual(["single2-value"], dict.get(self.metadata, "single2"))
+        self.assertEqual(["single2-value"], self.metadata.getraw("single2"))
-        self.assertEqual(self.multi1, dict.get(self.metadata, "multi1"))
+        self.assertEqual(self.multi1, self.metadata.getraw("multi1"))
-        self.assertEqual(self.multi2, dict.get(self.metadata, "multi2"))
+        self.assertEqual(self.multi2, self.metadata.getraw("multi2"))
-        self.assertEqual(self.multi3, dict.get(self.metadata, "multi3"))
+        self.assertEqual(self.multi3, self.metadata.getraw("multi3"))
-        self.assertEqual(["hidden-value"], dict.get(self.metadata, "~hidden"))
+        self.assertEqual(["hidden-value"], self.metadata.getraw("~hidden"))
 
     def test_metadata_get(self):
         self.assertEqual("single1-value", self.metadata["single1"])
         self.assertEqual("single1-value", self.metadata.get("single1"))
         self.assertEqual(["single1-value"], self.metadata.getall("single1"))
+        self.assertEqual(["single1-value"], self.metadata.getraw("single1"))
 
         self.assertEqual(MULTI_VALUED_JOINER.join(self.multi1), self.metadata["multi1"])
         self.assertEqual(MULTI_VALUED_JOINER.join(self.multi1), self.metadata.get("multi1"))
         self.assertEqual(self.multi1, self.metadata.getall("multi1"))
+        self.assertEqual(self.multi1, self.metadata.getraw("multi1"))
 
         self.assertEqual("", self.metadata["nonexistent"])
         self.assertEqual(None, self.metadata.get("nonexistent"))
         self.assertEqual([], self.metadata.getall("nonexistent"))
+        self.assertRaises(KeyError, self.metadata.getraw, "nonexistent")
 
-        self.assertEqual(dict.items(self.metadata), self.metadata.rawitems())
+        self.assertEqual(self.metadata._store.items(), self.metadata.rawitems())
-        metadata_items = [(x, z) for (x, y) in dict.items(self.metadata) for z in y]
+        metadata_items = [(x, z) for (x, y) in self.metadata.rawitems() for z in y]
         self.assertEqual(metadata_items, list(self.metadata.items()))
 
     def test_metadata_delete(self):
@@ -104,12 +107,12 @@ class MetadataTest(PicardTestCase):
         self.assertEqual(self.metadata.deleted_tags, m.deleted_tags)
 
         self.metadata["old"] = "old-value"
-        for (key, value) in dict.items(self.metadata):
+        for (key, value) in self.metadata.rawitems():
             self.assertIn(key, m)
-            self.assertEqual(value, dict.get(m, key))
+            self.assertEqual(value, m.getraw(key))
-        for (key, value) in dict.items(m):
+        for (key, value) in m.rawitems():
             self.assertIn(key, self.metadata)
-            self.assertEqual(value, dict.get(self.metadata, key))
+            self.assertEqual(value, self.metadata.getraw(key))
 
     def test_metadata_clear(self):
         self.metadata.clear()
@@ -133,14 +136,6 @@ class MetadataTest(PicardTestCase):
         self.assertEqual(MULTI_VALUED_JOINER.join(map(func, self.multi1)), self.metadata.get("multi1"))
         self.assertEqual(list(map(func, self.multi1)), self.metadata.getall("multi1"))
 
-        self.assertEqual("", self.metadata["nonexistent"])
-        self.assertEqual(None, self.metadata.get("nonexistent"))
-        self.assertEqual([], self.metadata.getall("nonexistent"))
-
-        self.assertEqual(dict.items(self.metadata), self.metadata.rawitems())
-        metadata_items = [(x, z) for (x, y) in dict.items(self.metadata) for z in y]
-        self.assertEqual(metadata_items, list(self.metadata.items()))
-
     def test_length_score(self):
         results = [(20000, 0, 0.333333333333),
                    (20000, 10000, 0.666666666667),
@@ -187,3 +182,65 @@ class MetadataTest(PicardTestCase):
         m2["artist"] = "TheArtist"
         m2.delete("title")
         self.assertTrue(m1.compare(m2) < 1)
+
+    def test_metadata_mapping_init(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': ''}
+        deleted_tags = set('c')
+        m = Metadata(d, deleted_tags=deleted_tags, length=1234)
+        self.assertTrue('a' in m)
+        self.assertEqual(m.getraw('a'), ['b'])
+        self.assertEqual(m['d'], MULTI_VALUED_JOINER.join(d['d']))
+        self.assertNotIn('c', m)
+        self.assertIn('c', m.deleted_tags)
+        self.assertEqual(m.length, 1234)
+
+    def test_metadata_mapping_del(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': ''}
+        m = Metadata(d)
+        self.assertEqual(m.getraw('a'), ['b'])
+        self.assertNotIn('a', m.deleted_tags)
+        del m['a']
+        self.assertRaises(KeyError, m.getraw, 'a')
+        self.assertIn('a', m.deleted_tags)
+
+    def test_metadata_mapping_iter(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': ''}
+        m = Metadata(d)
+        l = set(m)
+        self.assertEqual(l, {'a', 'c', 'd'})
+
+    def test_metadata_mapping_keys(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': ''}
+        m = Metadata(d)
+        l = set(m.keys())
+        self.assertEqual(l, {'a', 'c', 'd'})
+
+    def test_metadata_mapping_values(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': ''}
+        m = Metadata(d)
+        l = set(m.values())
+        self.assertEqual(l, {'b', '2', 'x; y'})
+
+    def test_metadata_mapping_len(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': ''}
+        m = Metadata(d)
+        self.assertEqual(len(m), 3)
+        del m['x']
+        self.assertEqual(len(m), 3)
+        del m['c']
+        self.assertEqual(len(m), 2)
+        #TODO: test with cover art images
+
+    def test_metadata_mapping_update(self):
+        d = {'a': 'b', 'c': 2, 'd': ['x', 'y'], 'x': 'z'}
+        m = Metadata(d)
+
+        d2 = {'c': 3, 'd': ['u', 'w'], 'x': ''}
+        m2 = Metadata(d2)
+
+        m.update(d2)
+        self.assertEqual(m['a'], 'b')
+        self.assertEqual(m['c'], '3')
+        self.assertEqual(m.getraw('d'), ['u', 'w'])
+        self.assertNotIn('x', m)
+        self.assertIn('x', m.deleted_tags)