Fix local itemsMap being inconsistent after changes on both ends

This can cause documents to either not be deleted at all when they
should be, but at the least for the parent folder's `local` itemsMap to
become inconsistent and potentially persist forever, because it can
never be the same as the remote itemsMap again (which is the criterium
for removing it).

Author: raucao

src/sync.ts

@@ -469,14 +469,28 @@ export class Sync {
       if (node.common.itemsMap) {
         for (const itemName in node.common.itemsMap) {
           if (!node.local.itemsMap[itemName]) {
-            // Indicates the node is either newly being fetched
-            // has been deleted locally (whether or not leading to conflict);
-            // before listing it in local listings, check if a local deletion
-            // exists.
+            // Indicates the node is either newly being fetched, or
+            // has been deleted locally (whether or not leading to
+            // conflict); before listing it in local listings, check
+            // if a local deletion exists.
             node.local.itemsMap[itemName] = false;
           }
         }
 
+        for (const itemName in node.local.itemsMap) {
+          if (!node.common.itemsMap[itemName]) {
+            // When an item appears in a folder's local itemsMap, but
+            // not in remote/common, it may or may not have been
+            // changed or deleted locally. The local itemsMap may
+            // only contain it, beause the item existed when
+            // *another* local item was changed, so we need to make
+            // sure that it's checked/processed again, so it will be
+            // deleted if there's no local change waiting to be
+            // pushed out.
+            this.addTask(node.path+itemName);
+          }
+        }
+
         if (equal(node.local.itemsMap, node.common.itemsMap)) {
           delete node.local;
         }
@@ -745,10 +759,10 @@ export class Sync {
     }
 
     const nodes = await this.rs.local.getNodes(paths);
+    const parentNode: RSNode = nodes[parentPath];
+    const missingChildren = {};
     let node: RSNode = nodes[path];
-    let parentNode: RSNode = nodes[parentPath];
     let itemName: string;
-    const missingChildren = {};
 
     function collectMissingChildren (folder): void {
       if (folder && folder.itemsMap) {
@@ -787,7 +801,15 @@ export class Sync {
       if (parentNode && parentNode.local && parentNode.local.itemsMap) {
         itemName = path.substring(parentPath.length);
 
-        parentNode.local.itemsMap[itemName] = true;
+        if (bodyOrItemsMap !== false) {
+          parentNode.local.itemsMap[itemName] = true;
+        } else {
+          if (parentNode.local.itemsMap[itemName]) {
+            // node is 404 on remote, can safely be removed from
+            // parent's local itemsMap now
+            delete parentNode.local.itemsMap[itemName];
+          }
+        }
 
         if (equal(parentNode.local.itemsMap, parentNode.common.itemsMap)) {
           delete parentNode.local;

test/unit/sync.test.mjs

@@ -689,6 +689,85 @@ describe("Sync", function() {
       });
     });
 
+    describe("Documents deleted both locally and on remote", function() {
+      beforeEach(async function() {
+        // three and four deleted on remote, three also deleted locally
+
+        const newRemoteMap = {
+          "one": { "ETag": "one" },
+          "two": { "ETag": "two" }
+        };
+
+        this.rs.local.setNodes({
+          "/foo/": {
+            path: "/foo/",
+            common: {
+              itemsMap: { "one": true, "two": true, "three": true, "four": true },
+              revision: "remotefolderrevision",
+              timestamp: 1397210425598,
+            },
+            local: {
+              itemsMap: { "one": true, "two": true, "four": true },
+              revision: "localfolderrevision",
+              timestamp: 1397210425612
+            }
+          },
+          "/foo/one": {
+            path: "/foo/one",
+            common: {
+              body: { foo: "one" },
+              contentType: "application/json",
+              revision: "one",
+              timestamp: 1234567891000
+            }
+          },
+          "/foo/two": {
+            path: "/foo/two",
+            local: {
+              body: { foo: "two" },
+              contentType: "application/json",
+              revision: "two",
+              timestamp: 1234567891000
+            }
+          },
+          "/foo/four": {
+            path: "/foo/four",
+            common: {
+              body: { foo: "four" },
+              contentType: "application/json",
+              revision: "four",
+              timestamp: 1234567891000
+            }
+          }
+        });
+
+        await this.rs.sync.handleResponse('/foo/', 'get', {
+          statusCode: 200, body: newRemoteMap,
+          contentType: 'application/ld+json',
+          revision: 'newfolderrevision'
+        });
+
+        this.nodes = await this.rs.local.getNodes([
+          "/foo/", "/foo/one", "/foo/two", "/foo/three", "/foo/four"
+        ]);
+      });
+
+      it("removes the document from the parent node's common itemsMap", async function() {
+        expect(this.nodes["/foo/"].common.itemsMap).to.deep.equal({
+          "one": true, "two": true
+        });
+      });
+
+      it("removes the parent node's local and remote itemsMap", function() {
+        expect(this.nodes["/foo/"].local).to.be.undefined;
+        expect(this.nodes["/foo/"].remote).to.be.undefined;
+      });
+
+      it("removes the deleted node from the cache", function() {
+        expect(this.nodes["/foo/four"]).to.be.undefined;
+      });
+    });
+
     describe("PUT without conflict", function() {
       beforeEach(async function() {
         this.rs.local.setNodes({
@@ -872,6 +951,86 @@ describe("Sync", function() {
     });
   });
 
+  describe("#autoMergeFolder", function() {
+    describe("documents updated on both sides", function() {
+      beforeEach(function() {
+        this.addTask = sinon.spy(this.rs.sync, "addTask");
+
+        this.nodeBefore = {
+          "path": "/foo/",
+          "remote": {
+            "revision": "incomingrevision",
+            "timestamp": 1750232323004,
+            "itemsMap": {
+              "1750232100702": true, // added remotely
+              "1750232294620": true, // no change
+            }
+          },
+          "common": {
+            "revision": "oldrevision",
+            "timestamp": 1750232313004,
+            "itemsMap": {
+              "1750232294620": true,
+            }
+          },
+          "local": {
+            "revision": "localrevision",
+            "timestamp": 1750232095577,
+            "itemsMap": {
+              "1750232294620": true,
+              "1750232283970": true, // uncertain state
+              "1750233526039": true, // added locally
+            }
+          }
+        };
+
+        this.nodeAfter = this.rs.sync.autoMergeFolder(this.nodeBefore);
+      });
+
+      it("adds new documents to the local itemsMap as uncached", function() {
+        expect(this.nodeAfter.local.itemsMap).to.deep.equal({
+          "1750232100702": false, // marked for fetch
+          "1750232294620": true,
+          "1750232283970": true,
+          "1750233526039": true
+        });
+      });
+
+      it("adds sync tasks for local items missing in incoming itemsMap", function() {
+        expect(this.addTask.callCount).to.equal(2);
+        expect(this.addTask.getCall(0).args[0]).to.equal("/foo/1750232283970");
+        expect(this.addTask.getCall(1).args[0]).to.equal("/foo/1750233526039");
+      });
+    });
+
+    describe("on an empty node", function() {
+      it("removes a remote version if it has a null revision", async function() {
+        const node = {
+          path: "foo", common: {}, remote: { revision: null }
+        };
+
+        expect(this.rs.sync.autoMergeDocument(node)).to
+          .deep.equal({ path: "foo", common: {} });
+      });
+    });
+
+    it("merges mutual deletions", function() {
+      const node = {
+        "path": "/myfavoritedrinks/b",
+        "common": { "timestamp": 1405488508303 },
+        "local":  { "body": false, "timestamp": 1405488515881 },
+        "remote": { "body": false, "timestamp": 1405488740722 }
+      };
+      const localAndRemoteRemoved = {
+        "path": "/myfavoritedrinks/b",
+        "common": { "timestamp": 1405488508303 }
+      };
+
+      expect(this.rs.sync.autoMergeDocument(node)).to
+        .deep.equal(localAndRemoteRemoved);
+    });
+  });
+
   describe("#autoMerge", function() {
     describe("new node", function() {
       beforeEach(function() {