Fix layer-update for differing last elements (#131)

* Fix layer-update for differing last elements

* Move path_diff into helper function and add test

* Use fs_path to determine write_base

---------

Co-authored-by: Viktor Dick <[email protected]>
Co-authored-by: Alexander Rolfes <[email protected]>

CHANGELOG

@@ -1,3 +1,7 @@
+23.1.2+dev.1
+  * Fix layer-update for cases where the last element in both checksum files is
+    not the same.
+
 23.1.2
   * Add warning for custom layer paths shadowing changes in layer-update
   * Panic before deleting the top-level acl_users

perfact/zodbsync/commands/layer_update.py

@@ -4,7 +4,7 @@
 import shutil
 
 from ..subcommand import SubCommand
-# from ..helpers import hashdir
+from ..helpers import path_diff
 
 
 class LayerUpdate(SubCommand):
@@ -49,34 +49,18 @@ def run(self):
                 os.path.join(self.sync.base_dir, '.layer-checksums', ident),
                 os.path.join(layer_paths[ident], '.checksums')
             ))
-            old = self.read_checksums(fnames[-1][0])
-            new = self.read_checksums(fnames[-1][1])
-            oldidx = 0
-            newidx = 0
-            # Iterate through results, which are ordered by path. Add any
-            # deviation to paths
-            while oldidx < len(old) or newidx < len(new):
-                if old[oldidx] == new[newidx]:
-                    oldidx += 1
-                    newidx += 1
-                    continue
-                oldpath = old[oldidx][0]
-                newpath = new[newidx][0]
-                if oldpath <= newpath:
-                    paths.add(oldpath)
-                    oldidx += 1
-                    continue
-                if newpath <= oldpath:
-                    paths.add(newpath)
-                    newidx += 1
+            paths.update(path_diff(
+                self.read_checksums(fnames[-1][0]),
+                self.read_checksums(fnames[-1][1]),
+            ))
 
         if not paths:
             return
-
-        self._playback_paths(sorted(paths))
+        paths = sorted(paths)
+        self._playback_paths(paths)
 
         if not self.args.dry_run:
-            self.sync.record(sorted(paths), recurse=False, skip_errors=True,
+            self.sync.record(paths, recurse=False, skip_errors=True,
                              ignore_removed=True)
             for path in paths:
                 if self.sync.fs_pathinfo(path)['layeridx'] == 0:

perfact/zodbsync/helpers.py

@@ -247,3 +247,33 @@ def db_modtime(context):  # pragma: no cover
     Allow access to a modtime for the whole ZODB.
     """
     return context._p_jar._db.lastTransaction()
+
+
+def path_diff(old, new):
+    """
+    For two lists of tuples (path, checksum) that are ordered by path, return
+    the set of all paths that differ, i.e. either are only present in one of
+    the lists or have a different checksum.
+    """
+    result = set()
+    oldidx = 0
+    newidx = 0
+    # Iterate through results, which are ordered by path. Add any
+    # deviation to paths
+    while oldidx < len(old) and newidx < len(new):
+        if old[oldidx] == new[newidx]:
+            oldidx += 1
+            newidx += 1
+            continue
+        oldpath = old[oldidx][0]
+        newpath = new[newidx][0]
+        if oldpath <= newpath:
+            result.add(oldpath)
+            oldidx += 1
+            continue
+        if newpath <= oldpath:
+            result.add(newpath)
+            newidx += 1
+    result.update([row[0] for row in old[oldidx:]])
+    result.update([row[0] for row in new[newidx:]])
+    return result

perfact/zodbsync/tests/test_helpers.py

@@ -123,3 +123,20 @@ def test_literal_eval():
     assert helpers.literal_eval("-True") == -1
     with pytest.raises(Exception):
         helpers.literal_eval('f(1)')
+
+
+def test_path_diff():
+    """Check that path_diff also handles cases where the last element is not
+    the same in both lists."""
+    old = [
+        ('Abc', '1234'),
+        ('Def', 'afaf'),
+        ('Xyz', 'yzyz'),
+    ]
+    new = [
+        ('Abc', '1234'),
+        ('Def', 'axax'),
+        ('Yyy', 'yzyz'),
+    ]
+    result = helpers.path_diff(old, new)
+    assert result == {'Def', 'Xyz', 'Yyy'}

perfact/zodbsync/zodbsync.py

@@ -477,7 +477,7 @@ def fs_write(self, path, data):
         if old_data != new_data:
             # Path in top layer, might be different than the one where we read
             # the content
-            write_base = os.path.join(self.app_dir, path.lstrip('/'))
+            write_base = self.fs_path(path)
             os.makedirs(write_base, exist_ok=True)
 
             self.logger.debug("Will write %d bytes of metadata" % len(fmt))