Fix cached documents not being fully deleted in Anonymous Mode

raucao · raucao · commit c561870a6435 · 2025-06-13T19:15:23.000+04:00
fixes #1204
diff --git a/src/baseclient.ts b/src/baseclient.ts
@@ -624,17 +624,16 @@ export class BaseClient {
    * @example
    * client.remove('path/to/object').then(() => console.log('item deleted'));
    */
-  // TODO add real return type
   // TODO Don't return the RemoteResponse directly, handle response properly
-  remove (path: string): Promise<unknown> {
+  async remove (path: string): Promise<QueuedRequestResponse> {
     if (typeof path !== 'string') {
       return Promise.reject('Argument \'path\' of baseClient.remove must be a string');
     }
     if (!this.storage.access.checkPathPermission(this.makePath(path), 'rw')) {
       console.warn('WARNING: Removing a document to which only read access (\'r\') was claimed');
     }
 
-    return this.storage.delete(this.makePath(path));
+    return this.storage.delete(this.makePath(path), this.storage.connected);
   }
 
   /**
diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts
@@ -227,7 +227,7 @@ abstract class CachingLayer {
     return this._updateNodes(paths, _processNodes);
   }
 
-  async delete (path: string): Promise<QueuedRequestResponse> {
+  async delete (path: string, remoteConnected: boolean): Promise<QueuedRequestResponse> {
     const paths = pathsFromRoot(path);
 
     return this._updateNodes(paths, function (nodePaths, nodes) {
@@ -245,7 +245,7 @@ abstract class CachingLayer {
           // Document
           previous = getLatest(node);
           node.local = {
-            body: false,
+            body: remoteConnected ? false : undefined,
             previousBody: (previous ? previous.body : undefined),
             previousContentType: (previous ? previous.contentType : undefined),
           };
@@ -371,8 +371,13 @@ abstract class CachingLayer {
               newContentType: node.local.contentType
             });
           }
-          delete node.local.previousBody;
-          delete node.local.previousContentType;
+          if (node.local.body === undefined) {
+            // no remote connected, remove deleted node from cache immediately
+            nodes[path] = undefined;
+          } else {
+            delete node.local.previousBody;
+            delete node.local.previousContentType;
+          }
         }
       }
 
diff --git a/src/syncedgetputdelete.ts b/src/syncedgetputdelete.ts
@@ -42,11 +42,11 @@ const SyncedGetPutDelete = {
     }
   },
 
-  'delete': function (path: string): Promise<QueuedRequestResponse | RemoteResponse> {
+  'delete': function (path: string, remoteConnected: boolean): Promise<QueuedRequestResponse | RemoteResponse> {
     if (this.local) {
-      return this.local.delete(path);
+      return this.local.delete(path, remoteConnected);
     } else {
-      return SyncedGetPutDelete._wrapBusyDone.call(this, this.remote.delete(path));
+      return SyncedGetPutDelete._wrapBusyDone.call(this, this.remote.delete(path, remoteConnected));
     }
   },
 
diff --git a/test/unit/cachinglayer-suite.js b/test/unit/cachinglayer-suite.js
@@ -211,7 +211,7 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'],
             test.assertAnd(nodes, {
               '/foo': undefined
             }, 'second pass');
-            nodes['/foo'] = {local: {some: 'data'}};
+            nodes['/foo'] = {local: {body: 'data'}};
             jobTwoCbCalled = true;
             return nodes;
           }).then(function() {
@@ -235,10 +235,10 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'],
 
             test.assertAnd(nodes, {
               '/foo': {
-                local: {some: 'data'}
+                local: {body: 'data'}
               }
             }, 'third pass');
-            nodes['/foo'] = {local: {some: 'other data'}};
+            nodes['/foo'] = {local: {body: 'other data'}};
             jobThreeCbCalled = true;
             return nodes;
           }).then(function() {
diff --git a/test/unit/cachinglayer.test.mjs b/test/unit/cachinglayer.test.mjs
@@ -96,6 +96,115 @@ describe("CachingLayer", function() {
     });
   });
 
+  describe("#delete", function() {
+    describe("when connected", function() {
+      beforeEach(async function() {
+        this.rs.remote.connected = true;
+        this.rs.remote.online = true;
+        this.rs.caching.enable("/foo/");
+
+        await this.rs.local.setNodes({
+          "/foo/": {
+            path: "/foo/",
+            common: {
+              itemsMap: {
+                "one": true,
+                "two": true
+              },
+              contentType: "application/ld+json",
+              timestamp: new Date().getTime(),
+              revision: "oldie-but-goodie"
+            }
+          },
+          "/foo/one": {
+            path: "/foo/one",
+            common: {
+              body: "some data",
+              contentType: "text/plain",
+              timestamp: new Date().getTime(),
+              revision: "123456"
+            }
+          },
+          "/foo/two": {
+            path: "/foo/two",
+            common: {
+              body: "some other data",
+              contentType: "text/plain",
+              timestamp: new Date().getTime(),
+              revision: "abcdef"
+            }
+          }
+        });
+
+        await this.rs.local.delete('/foo/one', this.rs.remote.connected);
+      });
+
+      it("marks the node for deletion", async function() {
+        const nodes = await this.rs.local.getNodes(["/foo/", "/foo/one"]);
+        const folder = nodes["/foo/"];
+        const node = nodes["/foo/one"];
+
+        expect(Object.keys(folder.common.itemsMap)).to.deep.equal(["one", "two"]);
+        expect(Object.keys(folder.local.itemsMap)).to.deep.equal(["two"]);
+        expect(node.local.body).to.be.false;
+        expect(node.push.body).to.be.false;
+      });
+    });
+
+    describe("when disconnected", function() {
+      beforeEach(async function() {
+        this.rs.remote.connected = false;
+        this.rs.remote.online = false;
+        this.rs.caching.enable("/foo/");
+
+        await this.rs.local.setNodes({
+          "/foo/": {
+            path: "/foo/",
+            local: {
+              itemsMap: {
+                "one": true,
+                "two": true
+              },
+              contentType: "application/ld+json",
+              timestamp: new Date().getTime(),
+              revision: "oldie-but-goodie"
+            }
+          },
+          "/foo/one": {
+            path: "/foo/one",
+            local: {
+              body: "some data",
+              contentType: "text/plain",
+              timestamp: new Date().getTime(),
+              revision: "123456"
+            }
+          },
+          "/foo/two": {
+            path: "/foo/two",
+            local: {
+              body: "some other data",
+              contentType: "text/plain",
+              timestamp: new Date().getTime(),
+              revision: "abcdef"
+            }
+          }
+        });
+
+        await this.rs.local.delete('/foo/one', this.rs.remote.connected);
+      });
+
+      it("deletes the node immediately", async function() {
+        const nodes = await this.rs.local.getNodes(["/foo/", "/foo/one", "/foo/two"]);
+        const folder = nodes["/foo/"];
+
+        expect(folder.common).to.be.undefined;
+        expect(Object.keys(folder.local.itemsMap)).to.deep.equal(["two"]);
+        expect(nodes["/foo/one"]).to.be.undefined;
+        expect(nodes["/foo/two"].local.revision).to.equal("abcdef");
+      });
+    });
+  });
+
   describe("#_emitChangeEvents", function() {
     it("broadcasts the change to other browser tabs", function(done) {
       this.rs.local.broadcastChannel = {
diff --git a/test/unit/inmemorycaching-suite.js b/test/unit/inmemorycaching-suite.js
@@ -299,7 +299,7 @@ define(['./build/config', './build/inmemorystorage'], function (config, InMemory
 
             test.assertAnd(Object.keys(storage), storageKeys);
 
-            return env.ims.delete('/foo/bar/baz').then(function (r) {
+            return env.ims.delete('/foo/bar/baz', true).then(function (r) {
               test.assertAnd(r.statusCode, 200, 'Wrong statusCode: '+r.statusCode); //TODO belongs in separate test
               test.assertAnd(getLatest(storage['/foo/bar/baz']), undefined);
               test.assertAnd(getLatest(storage['/foo/bar/']).itemsMap, {});
@@ -327,7 +327,7 @@ define(['./build/config', './build/inmemorystorage'], function (config, InMemory
               });
             });
 
-            return env.ims.delete('/foo/bar/baz');
+            return env.ims.delete('/foo/bar/baz', true);
           });
         }
       },
@@ -340,7 +340,7 @@ define(['./build/config', './build/inmemorystorage'], function (config, InMemory
 
           return env.ims.put('/foo/bar/baz', 'bla', 'text/plain', true, 'a1b2c3').then(function () {
             return env.ims.put('/foo/baz', 'bla', 'text/plain', true, 'a1b2c3').then(function () {
-              return env.ims.delete('/foo/bar/baz').then(function (status) {
+              return env.ims.delete('/foo/bar/baz', true).then(function (status) {
                 test.assertAnd(getLatest(storage['/']).itemsMap, { 'foo/': true });
                 test.assertAnd(getLatest(storage['/foo/']).itemsMap, { 'baz': true });
                 test.assertAnd(getLatest(storage['/foo/baz']).body, 'bla');
@@ -357,7 +357,7 @@ define(['./build/config', './build/inmemorystorage'], function (config, InMemory
         run: function (env, test) {
           return env.ims.put('/foo/bar/baz', 'bla', 'text/plain', 'a1b2c3').then(function () {
             return env.ims.put('/foo/baz', 'bla', 'text/plain', 'a1b2c3').then(function () {
-              return env.ims.delete('/foo/bar/baz').then(function (status) {
+              return env.ims.delete('/foo/bar/baz', true).then(function (status) {
                 test.assert(env.ims._storage['/'].local.itemsMap, {'foo/': true});
               });
             });

Original file line number	Diff line number	Diff line change
`@@ -624,17 +624,16 @@ export class BaseClient {`
`624`	`624`	`* @example`
`625`	`625`	`* client.remove('path/to/object').then(() => console.log('item deleted'));`
`626`	`626`	`*/`
`627`		`- // TODO add real return type`
`628`	`627`	`// TODO Don't return the RemoteResponse directly, handle response properly`
`629`		`- remove (path: string): Promise<unknown> {`
	`628`	`+ async remove (path: string): Promise<QueuedRequestResponse> {`
`630`	`629`	`if (typeof path !== 'string') {`
`631`	`630`	`return Promise.reject('Argument \'path\' of baseClient.remove must be a string');`
`632`	`631`	`}`
`633`	`632`	`if (!this.storage.access.checkPathPermission(this.makePath(path), 'rw')) {`
`634`	`633`	`console.warn('WARNING: Removing a document to which only read access (\'r\') was claimed');`
`635`	`634`	`}`
`636`	`635`
`637`		`- return this.storage.delete(this.makePath(path));`
	`636`	`+ return this.storage.delete(this.makePath(path), this.storage.connected);`
`638`	`637`	`}`
`639`	`638`
`640`	`639`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -42,11 +42,11 @@ const SyncedGetPutDelete = {`
`42`	`42`	`}`
`43`	`43`	`},`
`44`	`44`
`45`		`- 'delete': function (path: string): Promise<QueuedRequestResponse \| RemoteResponse> {`
	`45`	`+ 'delete': function (path: string, remoteConnected: boolean): Promise<QueuedRequestResponse \| RemoteResponse> {`
`46`	`46`	`if (this.local) {`
`47`		`- return this.local.delete(path);`
	`47`	`+ return this.local.delete(path, remoteConnected);`
`48`	`48`	`} else {`
`49`		`- return SyncedGetPutDelete._wrapBusyDone.call(this, this.remote.delete(path));`
	`49`	`+ return SyncedGetPutDelete._wrapBusyDone.call(this, this.remote.delete(path, remoteConnected));`
`50`	`50`	`}`
`51`	`51`	`},`
`52`	`52`