mirror of
https://github.com/fergalmoran/picard.git
synced 2026-01-06 08:34:01 +00:00
Metadata.update(): fix a corner case leading to inconsistent data
```python
m = Metadata(tag1='a', tag2='b')
del m['tag1']
```
```
m store: {'tag2': ['b']}
m deleted: {'tag1'}
```
```python
m2 = Metadata(tag1='c', tag2='d')
del m2['tag2']
```
```
m2 store: {'tag2': ['b']}
m2 deleted: {'tag1'}
```
```python
m.update(m2)
```
```
m store: {'tag1': ['c']}
m deleted: {'tag1', 'tag2'} <---- this was incorrect, and untested case
```
This patch fixes it, and results to:
```
m store: {'tag1': ['c']}
m deleted: {'tag2'}
```
This commit is contained in:
@@ -285,20 +285,23 @@ class Metadata(MutableMapping):
|
||||
self.update(other)
|
||||
|
||||
def update(self, *args, **kwargs):
|
||||
if len(args) == 1 and isinstance(args[0], self.__class__):
|
||||
one_arg = len(args) == 1
|
||||
if one_arg and isinstance(args[0], self.__class__):
|
||||
# update from Metadata object
|
||||
other = args[0]
|
||||
for k, v in other._store.items():
|
||||
self._store[k] = v[:]
|
||||
|
||||
for k, v in other.rawitems():
|
||||
self.set(k, v[:])
|
||||
|
||||
for tag in other.deleted_tags:
|
||||
del self[tag]
|
||||
|
||||
if other.images:
|
||||
self.images = other.images[:]
|
||||
if other.length:
|
||||
self.length = other.length
|
||||
|
||||
# Remove deleted tags from UI on save
|
||||
for tag in other.deleted_tags:
|
||||
del self[tag]
|
||||
elif len(args) == 1 and isinstance(args[0], MutableMapping):
|
||||
elif one_arg and isinstance(args[0], MutableMapping):
|
||||
# update from MutableMapping (ie. dict)
|
||||
for k, v in args[0].items():
|
||||
self[k] = v
|
||||
|
||||
@@ -299,3 +299,16 @@ class MetadataTest(PicardTestCase):
|
||||
self.assertEqual(m['tag2'], 'b')
|
||||
m.update(tag2='')
|
||||
self.assertIn('tag2', m.deleted_tags)
|
||||
|
||||
def test_metadata_mapping_update_kw_del(self):
|
||||
m = Metadata(tag1='a', tag2='b')
|
||||
del m['tag1']
|
||||
|
||||
m2 = Metadata(tag1='c', tag2='d')
|
||||
del m2['tag2']
|
||||
|
||||
m.update(m2)
|
||||
self.assertEqual(m['tag1'], 'c')
|
||||
self.assertNotIn('tag2', m)
|
||||
self.assertNotIn('tag1', m.deleted_tags)
|
||||
self.assertIn('tag2', m.deleted_tags)
|
||||
|
||||
Reference in New Issue
Block a user