Merge lp:~jderose/dmedia/import-update into lp:dmedia

[Jason Gerard DeRose]

This switches the importer to use Database.update() instead of Database.save(), which fixes an import failure that can occur when importing duplicate files (files already in Dmedia).

For details, see:
https://bugs.launchpad.net/dmedia/+bug/1856111

[David Jordan (dmj726)]

Looks good. Thanks for finding this issue and documenting to problem and solution. Approved!

review: Approve

=== modified file 'dmedia/importer.py'
--- dmedia/importer.py	2014-04-19 22:37:14 +0000
+++ dmedia/importer.py	2019-12-12 05:25:21 +0000
@@ -43,7 +43,7 @@
 from dmedia.util import get_project_db
 from dmedia.units import bytes10
 from dmedia import workers, schema
-from dmedia.metastore import MetaStore, create_stored, merge_stored
+from dmedia.metastore import MetaStore, create_stored, update_duplicate_file
 from dmedia.extractor import extract, merge_thumbnail
 
 
@@ -270,10 +270,7 @@
             stored = create_stored(ch.id, *filestores)
             try:
                 doc = self.db.get(ch.id)
-                doc['origin'] = 'user'
-                doc['atime'] = int(timestamp)
-                merge_stored(doc['stored'], stored)
-                self.db.save(doc)
+                self.db.update(update_duplicate_file, doc, timestamp, stored)
                 yield ('duplicate', file, ch)
             except NotFound:
                 doc = schema.create_file(timestamp, ch, stored)

=== modified file 'dmedia/metastore.py'
--- dmedia/metastore.py	2019-12-11 18:43:39 +0000
+++ dmedia/metastore.py	2019-12-12 05:25:21 +0000
@@ -378,6 +378,16 @@
     doc['bytes_avail'] = bytes_avail
 
 
+# See https://bugs.launchpad.net/dmedia/+bug/1856111
+def update_duplicate_file(doc, timestamp, stored):
+    """
+    Used by importer.ImportWorker.
+    """
+    doc['origin'] = 'user'
+    doc['atime'] = int(timestamp)
+    merge_stored(get_dict(doc, 'stored'), stored)
+
+
 def relink_iter(fs, count=50):
     buf = []
     for st in fs:

=== modified file 'dmedia/tests/test_metastore.py'
--- dmedia/tests/test_metastore.py	2019-12-11 18:43:39 +0000
+++ dmedia/tests/test_metastore.py	2019-12-12 05:25:21 +0000
@@ -1453,6 +1453,75 @@
             }
         )
 
+    def test_update_duplicate_file(self):
+        doc = {}
+        stored = {}
+        self.assertIsNone(metastore.update_duplicate_file(doc, 12345.6, stored))
+        self.assertEqual(doc,
+            {
+                'origin': 'user',
+                'atime': 12345,
+                'stored': {},
+            }
+        )
+        self.assertIsNot(doc['stored'], stored)
+
+        id1 = random_id()
+        id2 = random_id()
+        id3 = random_id()
+        old = {
+            id1: {
+                'copies': 2,
+                'mtime': 1234,
+                'verified': 2345,
+            },
+            id2: {
+                'should_be': 'ignored',
+                'copies': 2,
+                'mtime': 3456,
+                'verified': 4567,
+            },
+        }
+        new = {
+            id2: {
+                'copies': 1,
+                'mtime': 5678,
+            },
+            id3: {
+                'copies': 1,
+                'mtime': 6789,
+            },
+        }
+        doc = {
+            'origin': 'strange',
+            'atime': 9876,
+            'stored': old,
+        }
+        self.assertIsNone(metastore.update_duplicate_file(doc, 12345.6, new))
+        self.assertEqual(doc,
+            {
+                'origin': 'user',
+                'atime': 12345,
+                'stored': {
+                    id1: {  # Should be unmodified old:
+                        'copies': 2,
+                        'mtime': 1234,
+                        'verified': 2345,
+                    },
+                    id2: {  # Should be updated with new, plus 'verified' poped:
+                        'should_be': 'ignored',
+                        'copies': 1,
+                        'mtime': 5678,
+                    },
+                    id3: {  # Should be from new:
+                        'copies': 1,
+                        'mtime': 6789,
+                    },  
+                }
+            }
+        )
+        self.assertIs(doc['stored'], old)
+
     def test_relink_iter(self):
         fs = TempFileStore()