From e3f80efd1a76d3fb2a009dfc1ec713d1921d3ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 4 Jun 2025 17:24:01 +0400 Subject: [PATCH 01/47] WIP docs, async --- src/indexeddb.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/indexeddb.ts b/src/indexeddb.ts index 278ed3328..6bb5e9957 100644 --- a/src/indexeddb.ts +++ b/src/indexeddb.ts @@ -121,7 +121,7 @@ class IndexedDB extends CachingLayer { return nodes; }); } else { - return Promise.resolve(fromCache); + return fromCache; } } @@ -133,7 +133,6 @@ class IndexedDB extends CachingLayer { this.changesQueued[i] = nodes[i] || false; } this.maybeFlush(); - return Promise.resolve(); } /** @@ -167,7 +166,9 @@ class IndexedDB extends CachingLayer { } /** - * TODO: Document + * Retrieve nodes from the database + * + * @internal */ getNodesFromDb (paths: string[]): Promise { return new Promise((resolve, reject) => { @@ -193,12 +194,13 @@ class IndexedDB extends CachingLayer { reject('get transaction error/abort'); this.getsRunning--; }; - }); } /** - * TODO: Document + * Store nodes in the database + * + * @internal */ async setNodesInDb (nodes: { [key: string]: unknown }): Promise { return new Promise((resolve, reject) => { @@ -209,7 +211,7 @@ class IndexedDB extends CachingLayer { this.putsRunning++; - log('[IndexedDB] Starting put', nodes, this.putsRunning); + log('[IndexedDB] Starting puts', nodes, this.putsRunning); for (const path in nodes) { const node = nodes[path]; @@ -232,7 +234,7 @@ class IndexedDB extends CachingLayer { transaction.oncomplete = () => { this.putsRunning--; - log('[IndexedDB] Finished put', nodes, this.putsRunning, (new Date().getTime() - startTime) + 'ms'); + log('[IndexedDB] Finished puts', nodes, this.putsRunning, (new Date().getTime() - startTime) + 'ms'); resolve(); }; From 2c97b61aa82e7fd3367001ce22346543f31a54da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 15:50:32 +0400 Subject: [PATCH 02/47] Speed up specs, clear localStorage where used --- test/helpers/memoryStorage.mjs | 1 - test/unit/authorize.test.mjs | 12 ++---------- test/unit/discover.test.mjs | 16 ++++++++++++---- test/unit/dropbox.test.mjs | 4 ++++ test/unit/remotestorage.test.mjs | 27 ++++++++++----------------- test/unit/requests.test.mjs | 30 ++++++++++++++++++++++-------- 6 files changed, 50 insertions(+), 40 deletions(-) diff --git a/test/helpers/memoryStorage.mjs b/test/helpers/memoryStorage.mjs index c68b5286b..bff3c89e0 100644 --- a/test/helpers/memoryStorage.mjs +++ b/test/helpers/memoryStorage.mjs @@ -37,7 +37,6 @@ export class MemoryStorage { clear() { this.map.clear(); } - } let localStorage, sessionStorage; diff --git a/test/unit/authorize.test.mjs b/test/unit/authorize.test.mjs index 82834e7d3..47cfa697d 100644 --- a/test/unit/authorize.test.mjs +++ b/test/unit/authorize.test.mjs @@ -46,22 +46,14 @@ globalThis.localStorageAvailable = localStorageAvailable; globalThis["sessionStorage"] = sessionStorage; describe("Authorize", () => { - const sandbox = sinon.createSandbox(); - beforeEach( () => { - localStorage.removeItem('remotestorage:backend'); - localStorage.removeItem(WIRECLIENT_SETTINGS_KEY); - localStorage.removeItem(DISCOVER_SETTINGS_KEY); - localStorage.removeItem('remotestorage:api-keys'); - sessionStorage.removeItem('remotestorage:codeVerifier'); - sessionStorage.removeItem('remotestorage:state'); - globalThis.document.location.href = 'https://foo/bar'; }); afterEach(() => { fetchMock.reset(); - sandbox.restore(); + localStorage.clear(); + sessionStorage.clear(); }); describe("#authorize", () => { diff --git a/test/unit/discover.test.mjs b/test/unit/discover.test.mjs index ad38364c8..c3ea49da4 100644 --- a/test/unit/discover.test.mjs +++ b/test/unit/discover.test.mjs @@ -5,6 +5,7 @@ import fetchMock from 'fetch-mock'; import { localStorage } from '../helpers/memoryStorage.mjs'; +import config from "../../build/config.js"; import Discover from '../../build/discover.js'; chai.use(chaiAsPromised); @@ -76,6 +77,16 @@ const jrdJimbo = { }; describe('Webfinger discovery', () => { + before(() => { + config.requestTimeout = 20; + config.discoveryTimeout = 20; + }); + + after(() => { + localStorage.clear(); + config.requestTimout = 30000; + config.discoveryTimeout = 5000; + }); describe('successful lookup', () => { before(() => { @@ -103,10 +114,7 @@ describe('Webfinger discovery', () => { }); }); - after(() => { - localStorage.removeItem('remotestorage:discover'); - fetchMock.reset(); - }); + after(() => fetchMock.reset()); }); describe('record missing', () => { diff --git a/test/unit/dropbox.test.mjs b/test/unit/dropbox.test.mjs index 277327bfd..d93796338 100644 --- a/test/unit/dropbox.test.mjs +++ b/test/unit/dropbox.test.mjs @@ -70,6 +70,10 @@ describe('Dropbox backend', () => { sandbox.restore(); }); + after(() => { + localStorage.clear(); + }); + describe("infrastructure", () => { it("has a function `stopWaitingForToken`", () => { expect(typeof dropbox.stopWaitingForToken).to.equal('function'); diff --git a/test/unit/remotestorage.test.mjs b/test/unit/remotestorage.test.mjs index 8edd2910d..387efb8f6 100644 --- a/test/unit/remotestorage.test.mjs +++ b/test/unit/remotestorage.test.mjs @@ -25,17 +25,18 @@ class FakeRemote { applyMixins(FakeRemote, [ EventHandling ]); describe("RemoteStorage", function() { - const sandbox = sinon.createSandbox(); + beforeEach(function() { + this.rs = new RemoteStorage({ cache: false }); + }); afterEach(function() { - sandbox.restore(); + this.rs.disconnect(); + this.rs = undefined; + fetchMock.reset(); + sinon.reset(); }); describe('#addModule', function() { - beforeEach(function() { - this.rs = new RemoteStorage({ cache: false }); - }); - it('creates a module', function() { this.rs.addModule({ name: 'foo', builder: function() { return { exports: { it: 'worked' } }; @@ -77,7 +78,7 @@ describe("RemoteStorage", function() { beforeEach(function() { this.rs = new RemoteStorage({ cache: false, - discoveryTimeout: 500 + discoveryTimeout: 10 }); }); @@ -144,18 +145,12 @@ describe("RemoteStorage", function() { this.dropboxRsInit = Dropbox._rs_init; }); - beforeEach(function() { - this.rs = new RemoteStorage({ cache: false }); - }); - afterEach(function() { Dropbox._rs_init = this.dropboxRsInit; }); it("initializes the configured backend when it's not initialized yet", function(done) { - Dropbox._rs_init = function() { - done(); - }; + Dropbox._rs_init = function() { done(); }; this.rs.setApiKeys({ dropbox: 'testkey' }); }); @@ -163,9 +158,7 @@ describe("RemoteStorage", function() { it("reinitializes the configured backend when the key changed", function(done) { this.rs.apiKeys.dropbox = { appKey: 'old key' }; - Dropbox._rs_init = function() { - done(); - }; + Dropbox._rs_init = function() { done(); }; this.rs.setApiKeys({ dropbox: 'new key' }); }); diff --git a/test/unit/requests.test.mjs b/test/unit/requests.test.mjs index 550e63558..dc9045b2b 100644 --- a/test/unit/requests.test.mjs +++ b/test/unit/requests.test.mjs @@ -12,28 +12,42 @@ describe("request helpers", () => { describe("requestWithTimeout", () => { const originalTimeout = config.requestTimeout; - beforeEach(() => { - fetchMock.reset(); + before(() => { + config.requestTimeout = 20; }); - afterEach(() => { + after(() => { config.requestTimeout = originalTimeout; }); + afterEach(() => { + fetchMock.reset(); + }); + it("aborts requests if they don't resolve by the configured timeout", async () => { - config.requestTimeout = 20; const URL = 'https://example.edu/'; - fetchMock.mock({name: 'getFile', url: URL}, {status: 200, body: "Hello"}, {delay: 10_000}); - await expect(requestWithTimeout('GET', URL, {})).to.be.rejectedWith(/timeout/); + fetchMock.mock( + { name: 'getFile', url: URL }, + { status: 200, body: "Hello" }, + { delay: 30 } + ); + + await expect(requestWithTimeout('GET', URL, {})).to + .be.rejectedWith(/timeout/); }); it("fulfills requests, when they return before timeout", async () => { const URL = 'https://example.io/'; const BODY = 'Goodbye!'; - fetchMock.mock({name: 'getFile', url: URL}, {status: 200, body: BODY}); - await expect(requestWithTimeout('GET', URL, {})).to.eventually.be.an('object').with.property('response', BODY); + fetchMock.mock( + { name: 'getFile', url: URL }, + { status: 200, body: BODY } + ); + + await expect(requestWithTimeout('GET', URL, {})).to + .eventually.be.an('object').with.property('response', BODY); }); }); }); From 60373af6d078f020680ab9b2a245e2238938af31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 15:51:15 +0400 Subject: [PATCH 03/47] Use setItem for localStorage Missed this one in #1330 --- src/wireclient.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wireclient.ts b/src/wireclient.ts index 50fe8b438..f77caf3cd 100644 --- a/src/wireclient.ts +++ b/src/wireclient.ts @@ -304,13 +304,13 @@ class WireClient extends RemoteBase implements Remote { this.connected = false; } if (hasLocalStorage) { - localStorage[SETTINGS_KEY] = JSON.stringify({ + localStorage.setItem(SETTINGS_KEY, JSON.stringify({ userAddress: this.userAddress, href: this.href, storageApi: this.storageApi, token: this.token, properties: this.properties - }); + })); } } From 0a3cfd2c345028d7b5dfe3e16c3797cfb71f30a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 15:52:04 +0400 Subject: [PATCH 04/47] Use null value instead of relying on parse exception `getItem` returns `null` (whereas `localStorage[SETTINGS_KEY]`) used to return `undefined`. --- src/discover.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/discover.ts b/src/discover.ts index 2f89bada8..1cab27c59 100644 --- a/src/discover.ts +++ b/src/discover.ts @@ -90,12 +90,8 @@ Discover.DiscoveryError.prototype.constructor = Discover.DiscoveryError; Discover._rs_init = function (/*remoteStorage*/): void { hasLocalStorage = localStorageAvailable(); if (hasLocalStorage) { - try { - const settings = JSON.parse(localStorage.getItem(SETTINGS_KEY)); - cachedInfo = settings.cache; - } catch(e) { - /* empty */ - } + const settings = JSON.parse(localStorage.getItem(SETTINGS_KEY)); + if (settings) { cachedInfo = settings.cache; } } }; From de9cce4779323718258c216b38b40cf2d7286e7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 4 Jun 2025 18:25:56 +0400 Subject: [PATCH 05/47] WIP Port Sync tests to Mocha --- test/unit/remotestorage.test.mjs | 29 +++++++ test/unit/sync-suite.js | 135 ------------------------------- test/unit/sync.test.mjs | 70 +++++++++++++++- 3 files changed, 96 insertions(+), 138 deletions(-) diff --git a/test/unit/remotestorage.test.mjs b/test/unit/remotestorage.test.mjs index 387efb8f6..c312386e3 100644 --- a/test/unit/remotestorage.test.mjs +++ b/test/unit/remotestorage.test.mjs @@ -198,4 +198,33 @@ describe("RemoteStorage", function() { // expect(this.rs.googledrive.clientId).to.be.null; }); }); + + describe("#getSyncInterval", function() { + it("returns the configured sync interval", function() { + expect(this.rs.getSyncInterval()).to.equal(10000); + }); + }); + + describe("#setSyncInterval", function() { + before(function() { + this.rs = new RemoteStorage({ cache: false }); + }); + + it("sets the sync interval to the given value", function() { + this.rs.setSyncInterval(2000); + expect(this.rs.getSyncInterval()).to.equal(2000); + }); + + it("expects a number", function() { + expect(() => this.rs.setSyncInterval('60000')).to.throw(/not a valid sync interval/); + }); + + it("must more than (or equal to) 2 seconds", function() { + expect(() => this.rs.setSyncInterval(1000)).to.throw(/not a valid sync interval/); + }); + + it("must be less than (or equal to) 1 hour", function() { + expect(() => this.rs.setSyncInterval(3600001)).to.throw(/not a valid sync interval/); + }); + }); }); diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index 7f6c093e4..7f6aee388 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -53,34 +53,6 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require }, tests: [ - { - desc: "getParentPath works correctly", - run: function(env,test){ - test.assertAnd(env.rs.sync.getParentPath('/a'), '/'); - test.assertAnd(env.rs.sync.getParentPath('/a/'), '/'); - test.assertAnd(env.rs.sync.getParentPath('/a/b'), '/a/'); - test.assertAnd(env.rs.sync.getParentPath('/a/b/'), '/a/'); - test.assertAnd(env.rs.sync.getParentPath('/a/b/c'), '/a/b/'); - test.assertAnd(env.rs.sync.getParentPath('/a/b/c/'), '/a/b/'); - test.done(); - } - }, - - { - desc: "sync() returns immediately if not connected", - run: function(env,test){ - var failed = false; - env.rs.remote.connected = false; - env.rs.on('sync-done', function(){ - failed = true; - }); - - env.rs.sync.sync().then(function(){ - test.assert(failed, false); - }); - } - }, - { desc : "Sync adapter sets and removes all event listeners", run : function(env, test) { @@ -105,113 +77,6 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require } }, - { - desc : "Sync adapter starts syncing on connect", - run : function(env, test) { - env.rs.startSync = function() { - test.done(); - } - - Sync._rs_init(env.rs); - - env.rs._emit('connected'); - } - }, - - { - desc : "Sync adapter connect handler removes itself after the first call", - run : function(env, test) { - env.rs.startSync = function() { - test.assert(env.rs._handlers['connected'].length, 0, "connect handler still exists"); - } - - Sync._rs_init(env.rs); - - env.rs._emit('connected'); - } - }, - - { - desc : "Custom connected event handlers get called after Sync adapter removed its own handler", - run : function(env, test) { - Sync._rs_init(env.rs); - - env.rs.on('connected', function() { - test.done(); - }); - - env.rs._emit('connected'); - } - }, - - { - desc : "Sync adapter removes itself from remoteStorage instance after cleanup", - run : function(env, test) { - test.assertAnd(typeof env.rs.sync, "object", "sync is not defined"); - - Sync._rs_cleanup(env.rs); - - test.assert(typeof env.rs.sync, "undefined", "sync is still defined after cleanup"); - } - }, - - { - desc: "Default sync interval", - run: function(env, test) { - test.assert(env.rs.getSyncInterval(), 10000); - } - }, - - { - desc: "Update sync interval", - run: function(env, test) { - env.rs.setSyncInterval(2000); - test.assert(env.rs.getSyncInterval(), 2000); - } - }, - - { - desc: "Setting a wrong sync interval throws an error", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - try { - env.rs.setSyncInterval('60000'); - test.result(false, "setSyncInterval() didn't fail"); - } catch(e) { - test.result(true); - } - } - }, - - { - desc: "Setting a sync interval too short throws an error", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - try { - env.rs.setSyncInterval(1999); - test.result(false, "setSyncInterval() didn't fail"); - } catch(e) { - test.result(true); - } - } - }, - - { - desc: "Setting a sync interval too long throws an error", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - try { - env.rs.setSyncInterval(3600001); - test.result(false, "setSyncInterval() didn't fail"); - } catch(e) { - test.result(true); - } - } - }, - { desc: "Sync calls doTasks, and goes to collectTasks only if necessary", run: function(env, test) { diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index b88a40f3c..08e4f81a4 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -7,11 +7,11 @@ import { RemoteStorage } from '../../build/remotestorage.js'; import { Sync } from '../../build/sync.js'; describe("Sync", function() { - const sandbox = sinon.createSandbox(); - beforeEach(function(done) { this.rs = new RemoteStorage(); + this.rs.on('features-loaded', () => { + this.rs._handlers['connected'] = []; this.rs.local = new InMemoryStorage(); this.rs.sync = new Sync(this.rs); this.rs.sync.doTasks = () => { return true; }; @@ -20,7 +20,71 @@ describe("Sync", function() { }); afterEach(function() { - sandbox.restore(); + if (this.rs.sync) { Sync._rs_cleanup(this.rs); } + this.rs = undefined; + sinon.reset(); + }); + + describe(".rs_init", function() { + it("starts syncing on connect", function(done) { + let startSyncCalled = 0; + this.rs.startSync = () => { + startSyncCalled++; + if (startSyncCalled === 1) { done(); } + }; + + Sync._rs_init(this.rs); + this.rs._emit('connected'); + }); + + it("removes the 'connected' handler when it's called", function() { + Sync._rs_init(this.rs); + this.rs._emit('connected'); + expect(this.rs._handlers['connected'].length).to.equal(0); + }); + + it("doesn't interfere with custom 'connected' handlers", function(done) { + this.rs.on('connected', done); + Sync._rs_init(this.rs); + this.rs._emit('connected'); + }); + }); + + describe(".rs_cleanup", function() { + it("adapter removes itself from RS instance", async function() { + expect(typeof this.rs.sync).to.equal("object"); + Sync._rs_cleanup(this.rs); + expect(typeof this.rs.sync).to.equal("undefined"); + }); + }); + + describe("#getParentPath", function() { + it("returns the correct values", async function() { + const paths = { + '/a': '/', + '/a/': '/', + '/a/b': '/a/', + '/a/b/': '/a/', + '/a/b/c': '/a/b/', + '/a/b/c/': '/a/b/' + }; + + for (const path in paths) { + expect(this.rs.sync.getParentPath(path)).to.equal(paths[path], `The parent path of ${path} should be ${paths[path]}`); + } + }); + }); + + describe("#sync", function() { + it("returns immediately when not connected", async function() { + let syncDone = false; + this.rs.remote.connected = false; + this.rs.on('sync-done', () => { syncDone = true; }); + + await this.rs.sync.sync().then(() => { + expect(syncDone).to.be.false; + }); + }); }); describe("#finishTask", function() { From e5c5f77854520045d6691cb77c3bc1696a5460f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 18:07:13 +0400 Subject: [PATCH 06/47] Use correct type for RS class in Sync, fix type errors --- src/cachinglayer.ts | 11 +- src/remote.ts | 19 ++- src/sync.ts | 228 +++++++++++++++++--------------- test/unit/cachinglayer-suite.js | 8 +- test/unit/sync-suite.js | 6 +- 5 files changed, 146 insertions(+), 126 deletions(-) diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts index 1d63adb1e..88f1bf6a3 100644 --- a/src/cachinglayer.ts +++ b/src/cachinglayer.ts @@ -267,7 +267,7 @@ abstract class CachingLayer { const node = nodes[nodePath]; if (node && node.common && node.local) { - this._emitChange({ + this.emitChange({ path: node.path, origin: 'local', oldValue: (node.local.body === false ? undefined : node.local.body), @@ -281,7 +281,10 @@ abstract class CachingLayer { }); } - private _emitChange(obj: ChangeObj): void { + /** + * Emit a change event + */ + emitChange(obj: ChangeObj): void { if (config.changeEvents[obj.origin]) { this._emit('change', obj); } @@ -294,7 +297,7 @@ abstract class CachingLayer { if (isDocument(node.path)) { const latest = getLatest(node); if (latest) { - this._emitChange({ + this.emitChange({ path: node.path, origin: 'local', oldValue: undefined, @@ -403,7 +406,7 @@ abstract class CachingLayer { private _emitChangeEvents(events: RSEvent[]) { for (let i = 0, len = events.length; i < len; i++) { - this._emitChange(events[i]); + this.emitChange(events[i]); if (this.diffHandler) { this.diffHandler(events[i].path); } diff --git a/src/remote.ts b/src/remote.ts index 303a420b9..e3d6c4d9b 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -57,11 +57,11 @@ export class RemoteBase extends EventHandling { export interface RemoteSettings { userAddress?: string; - href?: string; // remoteStorage server's base URL - storageApi?: string; // spec version - token?: string | false; // OAuth2 access token - refreshToken?: string; // OAuth2 refresh token - tokenType?: string; // type of access token; usually 'bearer' + href?: string; // remoteStorage server's base URL + storageApi?: string; // spec version + token?: string | false; // OAuth2 access token + refreshToken?: string; // OAuth2 refresh token + tokenType?: string; // type of access token; usually 'bearer' properties?: object; } @@ -129,6 +129,13 @@ export interface Remote { */ clientId?: string; + /** + * OAuth2 access token + * + * @internal + */ + token?: string | false; + /** * OAuth2 PKCE * @@ -149,7 +156,7 @@ export interface Remote { stopWaitingForToken (): void; - get (path: string, options: { ifMatch?: string; ifNoneMatch?: string }): Promise; + get (path: string, options?: { ifMatch?: string; ifNoneMatch?: string }): Promise; put (path: string, body: XMLHttpRequestBodyInit, contentType: string, options: { ifMatch?: string; ifNoneMatch?: string }): Promise diff --git a/src/sync.ts b/src/sync.ts index ee1d736c4..fb9ab3039 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -14,6 +14,7 @@ import { isDocument, pathsFromRoot } from './util'; +import type RemoteStorage from './remotestorage'; let syncCycleCb, syncOnConnect; @@ -98,8 +99,7 @@ function handleVisibility (env, rs): void { * the `markChildren` function). **/ export class Sync { - // TODO remove when RS is defined, or if unnecessary - rs: { [propName: string]: any }; + rs: RemoteStorage; numThreads: number; done: boolean; @@ -111,7 +111,7 @@ export class Sync { _timeStarted: object; _finishedTasks: Array = []; - constructor (remoteStorage: object) { + constructor (remoteStorage: RemoteStorage) { this.rs = remoteStorage; this._tasks = {}; @@ -352,6 +352,9 @@ export class Sync { .catch((e: Error) => { throw e; }); } + /** + * TODO document + **/ public flush (nodes: RSNodes): RSNodes { for (const path in nodes) { // Strategy is 'FLUSH' and no local changes exist @@ -364,6 +367,11 @@ export class Sync { return nodes; } + /** + * Sync one path + * + * @internal + **/ public doTask (path: string): object { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; @@ -452,7 +460,7 @@ export class Sync { // keep/revert: log('[Sync] Emitting keep/revert'); - this.rs.local._emitChange({ + this.rs.local.emitChange({ origin: 'conflict', path: node.path, oldValue: node.local.body, @@ -500,7 +508,7 @@ export class Sync { newContentType: node.remote.contentType }; if (change.oldValue || change.newValue) { - this.rs.local._emitChange(change); + this.rs.local.emitChange(change); } if (!node.remote.body) { // no remote, so delete/don't create @@ -514,7 +522,7 @@ export class Sync { } } else { if (node.common.body) { - this.rs.local._emitChange({ + this.rs.local.emitChange({ origin: 'remote', path: node.path, oldValue: node.common.body, @@ -541,6 +549,9 @@ export class Sync { }); } + /** + * TODO document + **/ public async markChildren (path, itemsMap, changedNodes: RSNodes, missingChildren): Promise { const paths = []; const meta = {}; @@ -554,130 +565,128 @@ export class Sync { paths.push(path+childName); } - return this.rs.local.getNodes(paths).then((nodes: RSNodes) => { - let cachingStrategy; - let node; - - for (const nodePath in nodes) { - node = nodes[nodePath]; - - if (meta[nodePath]) { - if (node && node.common) { - if (nodeChanged(node, meta[nodePath].ETag)) { - changedNodes[nodePath] = deepClone(node); - changedNodes[nodePath].remote = { - revision: meta[nodePath].ETag, + const nodes = await this.rs.local.getNodes(paths); + + let cachingStrategy; + let node; + + for (const nodePath in nodes) { + node = nodes[nodePath]; + + if (meta[nodePath]) { + if (node && node.common) { + if (nodeChanged(node, meta[nodePath].ETag)) { + changedNodes[nodePath] = deepClone(node); + changedNodes[nodePath].remote = { + revision: meta[nodePath].ETag, + timestamp: this.now() + }; + changedNodes[nodePath] = this.autoMerge(changedNodes[nodePath]); + } + } else { + cachingStrategy = this.rs.caching.checkPath(nodePath); + if (cachingStrategy === 'ALL') { + changedNodes[nodePath] = { + path: nodePath, + common: { timestamp: this.now() - }; - changedNodes[nodePath] = this.autoMerge(changedNodes[nodePath]); - } - } else { - cachingStrategy = this.rs.caching.checkPath(nodePath); - if (cachingStrategy === 'ALL') { - changedNodes[nodePath] = { - path: nodePath, - common: { - timestamp: this.now() - }, - remote: { - revision: meta[nodePath].ETag, - timestamp: this.now() - } - }; - } + }, + remote: { + revision: meta[nodePath].ETag, + timestamp: this.now() + } + }; } + } - if (changedNodes[nodePath] && meta[nodePath]['Content-Type']) { - changedNodes[nodePath].remote.contentType = meta[nodePath]['Content-Type']; - } + if (changedNodes[nodePath] && meta[nodePath]['Content-Type']) { + changedNodes[nodePath].remote.contentType = meta[nodePath]['Content-Type']; + } - if (changedNodes[nodePath] && meta[nodePath]['Content-Length']) { - changedNodes[nodePath].remote.contentLength = meta[nodePath]['Content-Length']; - } - } else if (missingChildren[nodePath.substring(path.length)] && node && node.common) { - if (node.common.itemsMap) { - for (const commonItem in node.common.itemsMap) { - recurse[nodePath+commonItem] = true; - } + if (changedNodes[nodePath] && meta[nodePath]['Content-Length']) { + changedNodes[nodePath].remote.contentLength = meta[nodePath]['Content-Length']; + } + } else if (missingChildren[nodePath.substring(path.length)] && node && node.common) { + if (node.common.itemsMap) { + for (const commonItem in node.common.itemsMap) { + recurse[nodePath+commonItem] = true; } + } - if (node.local && node.local.itemsMap) { - for (const localItem in node.local.itemsMap) { - recurse[nodePath+localItem] = true; - } + if (node.local && node.local.itemsMap) { + for (const localItem in node.local.itemsMap) { + recurse[nodePath+localItem] = true; } + } - if (node.remote || isFolder(nodePath)) { - changedNodes[nodePath] = undefined; - } else { - changedNodes[nodePath] = this.autoMerge(node); - - if (typeof changedNodes[nodePath] === 'undefined') { - const parentPath = this.getParentPath(nodePath); - const parentNode = changedNodes[parentPath]; - const itemName = nodePath.substring(path.length); - if (parentNode && parentNode.local) { - delete parentNode.local.itemsMap[itemName]; - - if (equal(parentNode.local.itemsMap, parentNode.common.itemsMap)) { - delete parentNode.local; - } + if (node.remote || isFolder(nodePath)) { + changedNodes[nodePath] = undefined; + } else { + changedNodes[nodePath] = this.autoMerge(node); + + if (typeof changedNodes[nodePath] === 'undefined') { + const parentPath = this.getParentPath(nodePath); + const parentNode = changedNodes[parentPath]; + const itemName = nodePath.substring(path.length); + if (parentNode && parentNode.local) { + delete parentNode.local.itemsMap[itemName]; + + if (equal(parentNode.local.itemsMap, parentNode.common.itemsMap)) { + delete parentNode.local; } } } } } + } - return this.deleteRemoteTrees(Object.keys(recurse), changedNodes) - .then(changedObjs2 => { - return this.rs.local.setNodes(this.flush(changedObjs2)); - }); - }); + const changedNodes2 = await this.deleteRemoteTrees(Object.keys(recurse), changedNodes); + if (changedNodes2) { + await this.rs.local.setNodes(this.flush(changedNodes2)); + } } - public async deleteRemoteTrees (paths: Array, changedNodes: RSNodes): Promise { - if (paths.length === 0) { - return Promise.resolve(changedNodes); - } + /** + * TODO document + **/ + public async deleteRemoteTrees (paths: Array, changedNodes: RSNodes): Promise { + if (paths.length === 0) { return changedNodes; } - return this.rs.local.getNodes(paths).then(async (nodes: RSNodes) => { - const subPaths = {}; + const nodes = await this.rs.local.getNodes(paths); + const subPaths = {}; - function collectSubPaths (folder, path: string): void { - if (folder && folder.itemsMap) { - for (const itemName in folder.itemsMap) { - subPaths[path+itemName] = true; - } + function collectSubPaths (folder, path: string): void { + if (folder && folder.itemsMap) { + for (const itemName in folder.itemsMap) { + subPaths[path+itemName] = true; } } + } - for (const path in nodes) { - const node = nodes[path]; - - // TODO Why check for the node here? I don't think this check ever applies - if (!node) { continue; } + for (const path in nodes) { + const node = nodes[path]; + if (!node) { continue; } - if (isFolder(path)) { - collectSubPaths(node.common, path); - collectSubPaths(node.local, path); - } else { - if (node.common && typeof(node.common.body) !== undefined) { - changedNodes[path] = deepClone(node); - changedNodes[path].remote = { - body: false, - timestamp: this.now() - }; - changedNodes[path] = this.autoMerge(changedNodes[path]); - } + if (isFolder(path)) { + collectSubPaths(node.common, path); + collectSubPaths(node.local, path); + } else { + if (node.common && typeof(node.common.body) !== undefined) { + changedNodes[path] = deepClone(node); + changedNodes[path].remote = { + body: false, + timestamp: this.now() + }; + changedNodes[path] = this.autoMerge(changedNodes[path]); } } + } - // Recurse whole tree depth levels at once: - return this.deleteRemoteTrees(Object.keys(subPaths), changedNodes) - .then(changedNodes2 => { - return this.rs.local.setNodes(this.flush(changedNodes2)); - }); - }); + // Recurse whole tree depth levels at once: + const changedNodes2 = await this.deleteRemoteTrees(Object.keys(subPaths), changedNodes); + if (changedNodes2) { + await this.rs.local.setNodes(this.flush(changedNodes2)); + } } public async completeFetch (path: string, bodyOrItemsMap: object, contentType: string, revision: string): Promise { @@ -991,6 +1000,9 @@ export class Sync { }); } + /** + * TODO document + **/ public doTasks (): boolean { let numToHave: number, numAdded = 0, path: string; if (this.rs.remote.connected) { @@ -1044,9 +1056,9 @@ export class Sync { } /** - * Method: sync + * Start a sync procedure **/ - public sync (): Promise { + public async sync (): Promise { this.done = false; if (!this.doTasks()) { @@ -1060,8 +1072,6 @@ export class Sync { log('[Sync] Sync error', e); throw new Error('Local cache unavailable'); }); - } else { - return Promise.resolve(); } } diff --git a/test/unit/cachinglayer-suite.js b/test/unit/cachinglayer-suite.js index 86cc1c056..680828930 100644 --- a/test/unit/cachinglayer-suite.js +++ b/test/unit/cachinglayer-suite.js @@ -134,7 +134,7 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'], }, { - desc: "_emitChange emits change events", + desc: "emitChange emits change events", run: function(env, test) { var changeEvent = { path: '/foo', @@ -145,12 +145,12 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'], test.assert(event, changeEvent); }); - env.ims._emitChange(changeEvent); + env.ims.emitChange(changeEvent); } }, { - desc: "_emitChange doesn't emit events that are not enabled", + desc: "emitChange doesn't emit events that are not enabled", run: function(env, test) { var changeEvent = { path: '/foo', @@ -163,7 +163,7 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'], test.result(false, 'change event should not have been fired'); }); - env.ims._emitChange(changeEvent); + env.ims.emitChange(changeEvent); setTimeout(function() { test.done(); diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index 7f6aee388..d1b364580 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -944,7 +944,7 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require }; var otherDone = false; - env.rs.sync.rs.local._emitChange = function(changeEvent) { + env.rs.sync.rs.local.emitChange = function(changeEvent) { test.assertAnd(changeEvent, { origin: 'remote', path: 'foo', @@ -978,7 +978,7 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require remote: { body: false, revision: 'null' } }; var otherDone = false; - env.rs.sync.rs.local._emitChange = function(obj) { + env.rs.sync.rs.local.emitChange = function(obj) { test.assertAnd(obj, { origin: 'remote', path: 'foo', @@ -1011,7 +1011,7 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require common: {}, remote: { body: false, revision: 'null' } }; - env.rs.sync.rs.local._emitChange = function(obj) { + env.rs.sync.rs.local.emitChange = function(obj) { test.result(false, 'should not have emitted '+JSON.stringify(obj)); }; var result = env.rs.sync.autoMerge(node); From 3fad53a74b734fde065141267e04e8b02fb5059b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 18:54:33 +0400 Subject: [PATCH 07/47] WIP Types --- src/sync.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index fb9ab3039..8448d8ba0 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -105,11 +105,14 @@ export class Sync { done: boolean; stopped: boolean; + _tasks: { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [key: string]: ((...args: any[]) => any)[]; + }; // TODO define in more detail - _tasks: object; _running: object; _timeStarted: object; - _finishedTasks: Array = []; + _finishedTasks: SyncTask[] = []; constructor (remoteStorage: RemoteStorage) { this.rs = remoteStorage; From c1494b60107e1578dec679191731136950a9d265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 18:58:28 +0400 Subject: [PATCH 08/47] Remove unused argument and code path --- src/sync.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 8448d8ba0..93ff032fd 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -156,8 +156,7 @@ export class Sync { }); } - // FIXME force02 sounds like rs spec 02, thus could be removed - public corruptServerItemsMap (itemsMap, force02?: boolean): boolean { + public corruptServerItemsMap (itemsMap): boolean { if ((typeof(itemsMap) !== 'object') || (Array.isArray(itemsMap))) { return true; } @@ -179,14 +178,6 @@ export class Sync { if (itemName.indexOf('/') !== -1) { return true; } - if (force02) { - if (typeof(item['Content-Type']) !== 'string') { - return true; - } - if (typeof(item['Content-Length']) !== 'number') { - return true; - } - } } } From b22dc5961988332dcf6dfa6c7a4f8bb5aa37c281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 18:58:58 +0400 Subject: [PATCH 09/47] WIP docs, async --- src/sync.ts | 54 +++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 93ff032fd..88a2c542b 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -136,11 +136,17 @@ export class Sync { this.addEvents(['done', 'req-done']); } - public now (): number { + /** + * Return current time + **/ + now (): number { return new Date().getTime(); } - public queueGetRequest (path: string): object { + /** + * TODO document + **/ + public async queueGetRequest (path: string): Promise { return new Promise((resolve, reject) => { if (!this.rs.remote.connected) { reject('cannot fulfill maxAge requirement - remote is not connected'); @@ -223,6 +229,9 @@ export class Sync { return Object.getOwnPropertyNames(this._tasks).length > 0; } + /** + * TODO document + **/ public async collectDiffTasks (): Promise { let num = 0; @@ -347,7 +356,7 @@ export class Sync { } /** - * TODO document + * Flush nodes from cache after sync to remote **/ public flush (nodes: RSNodes): RSNodes { for (const path in nodes) { @@ -363,8 +372,6 @@ export class Sync { /** * Sync one path - * - * @internal **/ public doTask (path: string): object { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { @@ -422,6 +429,9 @@ export class Sync { }); } + /** + * TODO document + **/ public autoMergeFolder (node: RSNode): RSNode { if (node.remote.itemsMap) { node.common = node.remote; @@ -446,6 +456,9 @@ export class Sync { return node; } + /** + * TODO document + **/ public autoMergeDocument (node: RSNode): RSNode { if (hasNoRemoteChanges(node)) { node = mergeMutualDeletion(node); @@ -477,6 +490,9 @@ export class Sync { return node; } + /** + * TODO document + **/ public autoMerge (node: RSNode): RSNode { if (node.remote) { if (node.local) { @@ -643,7 +659,7 @@ export class Sync { /** * TODO document **/ - public async deleteRemoteTrees (paths: Array, changedNodes: RSNodes): Promise { + public async deleteRemoteTrees (paths: string[], changedNodes: RSNodes): Promise { if (paths.length === 0) { return changedNodes; } const nodes = await this.rs.local.getNodes(paths); @@ -683,8 +699,11 @@ export class Sync { } } + /** + * TODO document + **/ public async completeFetch (path: string, bodyOrItemsMap: object, contentType: string, revision: string): Promise { - let paths: Array; + let paths: string[]; let parentPath: string; const pathsFromRootArr = pathsFromRoot(path); @@ -754,6 +773,9 @@ export class Sync { }); } + /** + * TODO document + **/ public async completePush (path: string, action, conflict, revision: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; @@ -804,6 +826,9 @@ export class Sync { }); } + /** + * TODO document + **/ public async dealWithFailure (path: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { if (nodes[path]) { @@ -847,6 +872,9 @@ export class Sync { } } + /** + * TODO document + **/ public async handleGetResponse (path: string, status: ResponseStatus, bodyOrItemsMap, contentType: string, revision: string): Promise { if (status.notFound) { if (isFolder(path)) { @@ -909,6 +937,9 @@ export class Sync { } } + /** + * TODO document + **/ public finishTask (task: SyncTask, queueTask = true): void | Promise { if (task.action === undefined) { delete this._running[task.path]; @@ -1026,6 +1057,9 @@ export class Sync { return (numAdded >= numToAdd); } + /** + * TODO document + **/ public async collectTasks (alsoCheckRefresh?: boolean): Promise { if (this.hasTasks() || this.stopped) { return Promise.resolve(); @@ -1040,7 +1074,11 @@ export class Sync { }, function (err) { throw err; }); } - public addTask (path: string, cb?): void { + /** + * TODO document + **/ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + public addTask (path: string, cb?: (...args: any[]) => any): void { if (!this._tasks[path]) { this._tasks[path] = []; } From 70b352b417fe28b01d300af32e2b899f5e8aba23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 5 Jun 2025 19:37:57 +0400 Subject: [PATCH 10/47] Rename sync setup functions for clarity --- src/remotestorage.ts | 16 +++++++--------- src/sync.ts | 19 ++++++++----------- test/unit/remotestorage-suite.js | 12 ++++++------ test/unit/sync-suite.js | 24 ------------------------ test/unit/sync.test.mjs | 11 +++++++++++ 5 files changed, 32 insertions(+), 50 deletions(-) diff --git a/src/remotestorage.ts b/src/remotestorage.ts index d264a677e..0792e71d7 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -1090,19 +1090,19 @@ export class RemoteStorage { } /** - * TODO: document + * Add a handler to schedule periodic sync if sync enabled + * * @internal */ - syncCycle (): void { + setupSyncCycle (): void { if (!this.sync || this.sync.stopped) { return; } + log('[Sync] Setting up sync cycle'); this.on('sync-done', (): void => { - // FIXME Re-enable when modules are all imports - // log('[Sync] Sync done. Setting timer to', this.getCurrentSyncInterval()); + log('[Sync] Sync done. Setting timer to', this.getCurrentSyncInterval()); if (this.sync && !this.sync.stopped) { if (this._syncTimer) { clearTimeout(this._syncTimer); - this._syncTimer = undefined; } this._syncTimer = setTimeout(this.sync.sync.bind(this.sync), this.getCurrentSyncInterval()); } @@ -1141,14 +1141,12 @@ export class RemoteStorage { this._syncTimer = undefined; if (this.sync) { - // FIXME Re-enable when modules are all imports - // log('[Sync] Stopping sync'); + log('[Sync] Stopping sync'); this.sync.stopped = true; } else { // The sync class has not been initialized yet, so we make sure it will // not start the syncing process as soon as it's initialized. - // FIXME Re-enable when modules are all imports - // log('[Sync] Will instantiate sync stopped'); + log('[Sync] Will instantiate sync stopped'); this.syncStopped = true; } } diff --git a/src/sync.ts b/src/sync.ts index 88a2c542b..9fb20b987 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -16,7 +16,7 @@ import { } from './util'; import type RemoteStorage from './remotestorage'; -let syncCycleCb, syncOnConnect; +let setupSync, syncOnConnect; interface ResponseStatus { statusCode: string | number; @@ -1107,11 +1107,9 @@ export class Sync { } } - static _rs_init (remoteStorage): void { - syncCycleCb = function (): void { + static _rs_init (remoteStorage: RemoteStorage): void { + setupSync = function (): void { // if (!config.cache) return false - log('[Sync] syncCycleCb calling syncCycle'); - const env = new Env(); if (env.isBrowser()) { handleVisibility(env, remoteStorage); } @@ -1120,14 +1118,13 @@ export class Sync { remoteStorage.sync = new Sync(remoteStorage); if (remoteStorage.syncStopped) { - log('[Sync] Instantiating sync stopped'); + log('[Sync] Initializing with sync stopped'); remoteStorage.sync.stopped = true; delete remoteStorage.syncStopped; } } - log('[Sync] syncCycleCb calling syncCycle'); - remoteStorage.syncCycle(); + remoteStorage.setupSyncCycle(); }; syncOnConnect = function (): void { @@ -1135,13 +1132,13 @@ export class Sync { remoteStorage.startSync(); }; - remoteStorage.on('ready', syncCycleCb); + remoteStorage.on('ready', setupSync); remoteStorage.on('connected', syncOnConnect); } - static _rs_cleanup (remoteStorage): void { + static _rs_cleanup (remoteStorage: RemoteStorage): void { remoteStorage.stopSync(); - remoteStorage.removeEventListener('ready', syncCycleCb); + remoteStorage.removeEventListener('ready', setupSync); remoteStorage.removeEventListener('connected', syncOnConnect); remoteStorage.sync = undefined; diff --git a/test/unit/remotestorage-suite.js b/test/unit/remotestorage-suite.js index a20298f15..880f00535 100644 --- a/test/unit/remotestorage-suite.js +++ b/test/unit/remotestorage-suite.js @@ -441,19 +441,19 @@ define(['require', 'tv4', './build/eventhandling', './build/util'], }, { - desc: "#syncCycle registers an event handler to schedule periodic sync", + desc: "#setupSyncCycle registers an event handler to schedule periodic sync", run: function (env, test) { env.rs.sync = { sync: function(){} }; - env.rs.syncCycle(); + env.rs.setupSyncCycle(); test.assert(env.rs._handlers["sync-done"].length, 1); } }, { - desc: "#syncCycle does not register any event handlers when there is no sync instance", + desc: "#setupSyncCycle does not register any event handlers when there is no sync instance", run: function (env, test) { - env.rs.syncCycle(); + env.rs.setupSyncCycle(); test.assert(env.rs._handlers["sync-done"].length, 0); } @@ -463,7 +463,7 @@ define(['require', 'tv4', './build/eventhandling', './build/util'], desc: "sync-done handler does not reschedule a new sync when sync is stopped", run: function (env, test) { env.rs.sync = { sync: function() {} }; - env.rs.syncCycle(); + env.rs.setupSyncCycle(); env.rs.sync.stopped = true; @@ -477,7 +477,7 @@ define(['require', 'tv4', './build/eventhandling', './build/util'], desc: "sync-done handler does not reschedule a new sync when there is no sync instance", run: function (env, test) { env.rs.sync = { sync: function() {} }; - env.rs.syncCycle(); + env.rs.setupSyncCycle(); env.rs.sync = undefined; diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index d1b364580..f554f67aa 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -53,30 +53,6 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require }, tests: [ - { - desc : "Sync adapter sets and removes all event listeners", - run : function(env, test) { - function allHandlers() { - var handlers = env.rs._handlers; - var l = 0; - for (var k in handlers) { - l += handlers[k].length; - } - return l; - } - - test.assertAnd(allHandlers(), 0, "before init found "+allHandlers()+" handlers"); - - Sync._rs_init(env.rs); - test.assertAnd(allHandlers(), 2, "after init found "+allHandlers()+" handlers"); - - Sync._rs_cleanup(env.rs); - test.assertAnd(allHandlers(), 0, "after cleanup found "+allHandlers()+" handlers"); - - test.done(); - } - }, - { desc: "Sync calls doTasks, and goes to collectTasks only if necessary", run: function(env, test) { diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 08e4f81a4..69dab6033 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -26,6 +26,17 @@ describe("Sync", function() { }); describe(".rs_init", function() { + it("sets up sync when RS instance is ready", function(done) { + let setupSyncCycleCalled = 0; + this.rs.setupSyncCycle = () => { + setupSyncCycleCalled++; + if (setupSyncCycleCalled === 1) { done(); } + }; + + Sync._rs_init(this.rs); + this.rs._emit('ready'); + }); + it("starts syncing on connect", function(done) { let startSyncCalled = 0; this.rs.startSync = () => { From afaa82ae68f7e7564a9927e88a076492394ab286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 12:57:44 +0400 Subject: [PATCH 11/47] Refactor, document doTasks; port tests for sync() --- src/sync.ts | 62 +++++++++++++++++++++++++---------------- test/unit/sync-suite.js | 40 -------------------------- test/unit/sync.test.mjs | 38 +++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 64 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 9fb20b987..00a3dd14b 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1026,34 +1026,50 @@ export class Sync { } /** - * TODO document + * Determine how many tasks we want to have **/ - public doTasks (): boolean { - let numToHave: number, numAdded = 0, path: string; - if (this.rs.remote.connected) { - if (this.rs.remote.online) { - numToHave = this.numThreads; - } else { - numToHave = 1; - } + tasksWanted (): number { + if (!this.rs.remote.connected) { + // Nothing to sync if no remote connected + return 0; + } + + if (this.rs.remote.online) { + // Run as many tasks as threads are available/configured + return this.numThreads; } else { - numToHave = 0; + // Only run 1 task when we're offline + return 1; } + } + + /** + * Check if more tasks can be queued, and start running + * tasks + * + * @returns {Boolean} `true` when all tasks have been + * run or there's nothing to do, `false` + * if we could or want to run more + **/ + doTasks (): boolean { + const numToHave = this.tasksWanted(); const numToAdd = numToHave - Object.getOwnPropertyNames(this._running).length; - if (numToAdd <= 0) { - return true; - } + + if (numToAdd <= 0) { return true; } + + let numAdded: number = 0; + let path: string; + for (path in this._tasks) { if (!this._running[path]) { this._timeStarted[path] = this.now(); this._running[path] = this.doTask(path); this._running[path].then(this.finishTask.bind(this)); numAdded++; - if (numAdded >= numToAdd) { - return true; - } + if (numAdded >= numToAdd) { break; } } } + return (numAdded >= numToAdd); } @@ -1094,16 +1110,14 @@ export class Sync { this.done = false; if (!this.doTasks()) { - return this.collectTasks().then(() => { - try { - this.doTasks(); - } catch(e) { - log('[Sync] doTasks error', e); - } - }, function (e) { + try { + await this.collectTasks(); + } catch (e) { log('[Sync] Sync error', e); throw new Error('Local cache unavailable'); - }); + }; + + this.doTasks(); } } diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index f554f67aa..ed15a2f2e 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -53,46 +53,6 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require }, tests: [ - { - desc: "Sync calls doTasks, and goes to collectTasks only if necessary", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - var doTasksCalled = 0, collectTasksCalled = 0, addTaskCalled = 0, - tmpDoTasks = env.rs.sync.doTasks, - tmpFindTasks = env.rs.sync.collectTasks, - tmpAddTasks = env.rs.sync.addTasks; - - env.rs.sync.doTasks = function() { - doTasksCalled++; - if (addTaskCalled) { - return true; - } else { - return false; - } - }; - env.rs.sync.collectTasks = function() { - collectTasksCalled++; - return Promise.resolve(); - }; - env.rs.sync.addTask = function() { - addTaskCalled++; - }; - env.rs.sync.sync().then(function() { - test.assertAnd(doTasksCalled, 2); - test.assertAnd(collectTasksCalled, 1); - env.rs.sync.addTask('/foo', function() {}); - return env.rs.sync.sync(); - }).then(function() { - test.assertAnd(doTasksCalled, 3); - test.assertAnd(collectTasksCalled, 1); - env.rs.sync.doTasks = tmpDoTasks; - env.rs.sync.collectTasks = tmpFindTasks; - env.rs.sync.addTasks = tmpAddTasks; - test.done(); - }); - } - }, { desc: "collectTasks calls collectDiffTasks and goes to collectRefreshTasks only if necessary", run: function(env, test) { diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 69dab6033..127105887 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -96,6 +96,44 @@ describe("Sync", function() { expect(syncDone).to.be.false; }); }); + + describe("with no need to sync", function() { + beforeEach(function() { + this.spies = { + doTasks: sinon.stub(this.rs.sync, 'doTasks').returns(true), + collectTasks: sinon.spy(this.rs.sync, 'collectTasks') + }; + }); + + it("does not call #collectTasks()", async function() { + await this.rs.sync.sync(); + expect(this.spies.collectTasks.callCount).to.equal(0); + }); + + it("calls #doTasks() once", async function() { + await this.rs.sync.sync(); + expect(this.spies.doTasks.callCount).to.equal(1); + }); + }); + + describe("with sync desired but not enough tasks queued", function() { + beforeEach(function() { + this.spies = { + doTasks: sinon.stub(this.rs.sync, 'doTasks').returns(false), + collectTasks: sinon.spy(this.rs.sync, 'collectTasks') + }; + }); + + it("calls #collectTasks()", async function() { + await this.rs.sync.sync(); + expect(this.spies.collectTasks.callCount).to.equal(1); + }); + + it("calls #doTasks() twice", async function() { + await this.rs.sync.sync(); + expect(this.spies.doTasks.callCount).to.equal(2); + }); + }); }); describe("#finishTask", function() { From 8733e236c7f309efbfcbbf394207cad30d056646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 15:24:54 +0400 Subject: [PATCH 12/47] Refactor, document task collect functions; port tests --- src/sync.ts | 44 ++++++------- test/helpers/fake-access.mjs | 40 ++++++++++++ test/unit/sync-suite.js | 96 ---------------------------- test/unit/sync.test.mjs | 118 +++++++++++++++++++++++++++++++++++ 4 files changed, 180 insertions(+), 118 deletions(-) create mode 100644 test/helpers/fake-access.mjs diff --git a/src/sync.ts b/src/sync.ts index 00a3dd14b..5c9b98207 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -230,9 +230,9 @@ export class Sync { } /** - * TODO document + * Collect sync tasks for changed nodes **/ - public async collectDiffTasks (): Promise { + async collectDiffTasks (): Promise { let num = 0; return this.rs.local.forAllNodes((node: RSNode) => { @@ -244,7 +244,8 @@ export class Sync { this.addTask(node.path); num++; } - } else if (this.needsFetch(node) && this.rs.access.checkPathPermission(node.path, 'r')) { + } else if (this.needsFetch(node) && + this.rs.access.checkPathPermission(node.path, 'r')) { this.addTask(node.path); num++; } else if (isDocument(node.path) && this.needsPush(node) && @@ -253,8 +254,7 @@ export class Sync { num++; } }) - .then((): number => num) - .catch(e => { throw e; }); + .then((): number => num); } public inConflict (node: RSNode): boolean { @@ -335,8 +335,11 @@ export class Sync { } } - public async collectRefreshTasks (): Promise { - return this.rs.local.forAllNodes((node: RSNode) => { + /** + * Collect tasks to refresh highest outdated folder in tree + **/ + async collectRefreshTasks (): Promise { + await this.rs.local.forAllNodes((node: RSNode) => { let parentPath: string; if (this.needsRefresh(node)) { try { @@ -350,9 +353,9 @@ export class Sync { this.addTask(node.path); } } - }) - .then(() => this.deleteChildPathsFromTasks()) - .catch((e: Error) => { throw e; }); + }); + + this.deleteChildPathsFromTasks(); } /** @@ -1074,20 +1077,17 @@ export class Sync { } /** - * TODO document + * Collect any potential sync tasks if none are queued **/ - public async collectTasks (alsoCheckRefresh?: boolean): Promise { - if (this.hasTasks() || this.stopped) { - return Promise.resolve(); - } + async collectTasks (alsoCheckRefresh: boolean = true): Promise { + if (this.hasTasks() || this.stopped) { return; } - return this.collectDiffTasks().then(numDiffs => { - if (numDiffs || alsoCheckRefresh === false) { - return Promise.resolve(); - } else { - return this.collectRefreshTasks(); - } - }, function (err) { throw err; }); + const numDiffs = await this.collectDiffTasks(); + if (numDiffs > 0) { return; } + + if (alsoCheckRefresh) { + return this.collectRefreshTasks(); + } } /** diff --git a/test/helpers/fake-access.mjs b/test/helpers/fake-access.mjs new file mode 100644 index 000000000..bead815bf --- /dev/null +++ b/test/helpers/fake-access.mjs @@ -0,0 +1,40 @@ +class FakeAccess { + constructor() { + this._data = {}; + } + + set (moduleName, value) { + this._data[moduleName] = value; + } + + get (moduleName) { + return this._data[moduleName]; + } + + checkPathPermission (path, mode) { + if (path.substring(0, '/foo/'.length) === '/foo/') { + return true; + } + if (path.substring(0, '/read/access/'.length) === '/read/access/' && mode === 'r') { + return true; + } + if (path.substring(0, '/write/access/'.length) === '/write/access/') { + return true; + } + if (path.substring(0, '/readings/'.length) === '/readings/' && mode === 'r') { + return true; + } + if (path.substring(0, '/public/readings/'.length) === '/public/readings/' && mode === 'r') { + return true; + } + if (path.substring(0, '/writings/'.length) === '/writings/') { + return true; + } + if (path.substring(0, '/public/writings/'.length) === '/public/writings/') { + return true; + } + return false; + } +} + +export default FakeAccess; diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index ed15a2f2e..78bc283d9 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -53,102 +53,6 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require }, tests: [ - { - desc: "collectTasks calls collectDiffTasks and goes to collectRefreshTasks only if necessary", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - var collectDiffTasksCalled = 0, collectRefreshTasksCalled = 0, - tmpCheckDiffs = env.rs.sync.collectDiffTasks, - tmpCheckRefresh = env.rs.sync.collectRefreshTasks, - haveDiffs = 0; - env.rs.sync.collectDiffTasks = function() { - collectDiffTasksCalled++; - return Promise.resolve(haveDiffs); - }; - env.rs.sync.collectRefreshTasks = function() { - collectRefreshTasksCalled++; - return Promise.resolve([]); - }; - env.rs.sync.collectTasks().then(function() { - test.assertAnd(collectDiffTasksCalled, 1); - test.assertAnd(collectRefreshTasksCalled, 1); - haveDiffs = 1; - return env.rs.sync.collectTasks(); - }).then(function() { - test.assertAnd(collectDiffTasksCalled, 2); - test.assertAnd(collectRefreshTasksCalled, 1); - env.rs.sync.collectDiffTasks = tmpCheckDiffs; - env.rs.sync.collectRefreshTasks = tmpCheckRefresh; - test.done(); - }); - } - }, - { - desc: "collectRefreshTasks gives preference to caching parent", - run: function(env, test) { - var tmpForAllNodes = env.rs.local.forAllNodes; - var tmpNow = env.rs.sync.now; - var fakeCallback = function() {}; - - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - - env.rs.sync.addTask( - '/foo/ba/and/then/some/sub/path', - fakeCallback // should be passed to the ancestor when overruled - ); - env.rs.sync.now = function() { - return 1234568654321; - }; - - env.rs.local.forAllNodes = function(cb) { - cb({ - path: '/foo/ba/and/then/some/sub/path', //should be overruled by ancestor /foo/ba/ - common: { - body: 'off', - contentType: 'cT', - timestamp: 1234567890123 - } - }); - cb({ - path: '/foo/ba/', //should retrieve /foo/ to get its new revision - common: { - body: 'off', - contentType: 'cT', - timestamp: 1234567890124 - } - }); - cb({ - path: '/read/access/', // should retrieve - common: { - body: 'off', - contentType: 'cT', - timestamp: 1234567890124 - } - }); - cb({ - path: '/no/access/', // no access - common: { - body: 'off', - contentType: 'cT', - timestamp: 1234567890124 - } - }); - return Promise.resolve(); - }; - - env.rs.sync.collectRefreshTasks().then(function() { - test.assertAnd(env.rs.sync._tasks, { - '/foo/': [fakeCallback], // inherited from task '/foo/ba/and/then/some/sub/path' - '/read/access/': [] - }); - env.rs.local.forAllNodes = tmpForAllNodes; - env.rs.sync.now = tmpNow; - test.done(); - }); - } - }, { desc: "go through the request-queue with 4-8 requests at a time", run: function(env, test) { diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 127105887..50d553112 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -5,10 +5,16 @@ import sinon from 'sinon'; import InMemoryStorage from '../../build/inmemorystorage.js'; import { RemoteStorage } from '../../build/remotestorage.js'; import { Sync } from '../../build/sync.js'; +import FakeAccess from '../helpers/fake-access.mjs'; describe("Sync", function() { beforeEach(function(done) { this.rs = new RemoteStorage(); + Object.defineProperty(this.rs, 'access', { + value: new FakeAccess(), + writable: true, + configurable: true + }); this.rs.on('features-loaded', () => { this.rs._handlers['connected'] = []; @@ -136,6 +142,118 @@ describe("Sync", function() { }); }); + describe("#collectTasks", function() { + beforeEach(function() { + this.spies = { + collectDiffTasks: sinon.spy(this.rs.sync, 'collectDiffTasks'), + collectRefreshTasks: sinon.spy(this.rs.sync, 'collectRefreshTasks') + }; + }); + + describe("with tasks queued", function() { + beforeEach(function() { + this.rs.sync._tasks = { "/foo/bar": [] }; + }); + + it("returns immediately", async function() { + await this.rs.sync.collectTasks(); + expect(this.spies.collectDiffTasks.callCount).to.equal(0); + expect(this.spies.collectRefreshTasks.callCount).to.equal(0); + }); + }); + + describe("when sync is stopped", function() { + beforeEach(function() { + this.rs.sync.stopped = true; + }); + + it("returns immediately", async function() { + await this.rs.sync.collectTasks(); + expect(this.spies.collectDiffTasks.callCount).to.equal(0); + expect(this.spies.collectRefreshTasks.callCount).to.equal(0); + }); + }); + + describe("with diffs found", function() { + beforeEach(function() { + sinon.restore(); + this.spies = { + collectDiffTasks: sinon.stub(this.rs.sync, 'collectDiffTasks').returns(1), + collectRefreshTasks: sinon.spy(this.rs.sync, 'collectRefreshTasks') + }; + }); + + it("calls #collectDiffTasks()", async function() { + await this.rs.sync.collectTasks(); + expect(this.spies.collectDiffTasks.callCount).to.equal(1); + }); + + it("does not call #collectRefreshTasks()", async function() { + await this.rs.sync.collectTasks(); + expect(this.spies.collectRefreshTasks.callCount).to.equal(0); + }); + }); + + describe("with no diffs found", function() { + beforeEach(function() { + sinon.restore(); + this.spies = { + collectDiffTasks: sinon.stub(this.rs.sync, 'collectDiffTasks').returns(0), + collectRefreshTasks: sinon.spy(this.rs.sync, 'collectRefreshTasks') + }; + }); + + it("calls #collectRefreshTasks()", async function() { + await this.rs.sync.collectTasks(); + expect(this.spies.collectRefreshTasks.callCount).to.equal(1); + }); + + it("does not call #collectRefreshTasks() when `alsoCheckRefresh` is set to `false`", async function() { + await this.rs.sync.collectTasks(false); + expect(this.spies.collectRefreshTasks.callCount).to.equal(0); + }); + }); + }); + + describe("#collectRefreshTasks", function() { + beforeEach(function() { + this.fakeCallback = function() {}; + this.rs.sync.addTask( + '/foo/bar/and/then/some', this.fakeCallback + ); + this.rs.sync.now = () => 1234568654321; + + this.rs.local.forAllNodes = async function(cb) { + cb({ + path: '/foo/bar/and/then/some', //should be overruled by ancestor /foo/ba/ + common: { body: 'off', contentType: 'cT', timestamp: 1234567890123 } + }); + cb({ + path: '/foo/bar/', //should retrieve /foo/ to get its new revision + common: { body: 'off', contentType: 'cT', timestamp: 1234567890124 } + }); + cb({ + path: '/read/access/', // should retrieve + common: { body: 'off', contentType: 'cT', timestamp: 1234567890124 } + }); + cb({ + path: '/no/access/', // no access + common: { body: 'off', contentType: 'cT', timestamp: 1234567890124 } + }); + }; + + }); + + it("gives preference to parent folders", async function() { + await this.rs.sync.collectRefreshTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ + '/foo/': [this.fakeCallback], // inherited from task '/foo/bar/and/then/some' + '/read/access/': [] + }); + }); + }); + describe("#finishTask", function() { beforeEach(function() { this.tasks = { From 2498392f8ff3e777575e18e0d18a9d5e2a5219bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 15:25:53 +0400 Subject: [PATCH 13/47] Use Object.keys --- src/sync.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 5c9b98207..fb1c01f6d 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -225,8 +225,8 @@ export class Sync { (node.push && this.corruptRevision(node.push))); } - public hasTasks (): boolean { - return Object.getOwnPropertyNames(this._tasks).length > 0; + hasTasks (): boolean { + return Object.keys(this._tasks).length > 0; } /** @@ -993,7 +993,7 @@ export class Sync { await this.collectTasks(false).then(() => { // See if there are any more tasks that are not refresh tasks if (!this.hasTasks() || this.stopped) { - log('[Sync] Sync is done! Reschedule?', Object.getOwnPropertyNames(this._tasks).length, this.stopped); + log('[Sync] Sync is done! Reschedule?', Object.keys(this._tasks).length, this.stopped); if (!this.done) { this.done = true; this.rs._emit('sync-done', { completed: true }); @@ -1056,7 +1056,7 @@ export class Sync { **/ doTasks (): boolean { const numToHave = this.tasksWanted(); - const numToAdd = numToHave - Object.getOwnPropertyNames(this._running).length; + const numToAdd = numToHave - Object.keys(this._running).length; if (numToAdd <= 0) { return true; } From 1c596c574a63f31d860896f13d16c7f5484f4641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 15:26:04 +0400 Subject: [PATCH 14/47] No explicitly public functions if not used outside of class --- src/sync.ts | 60 ++++++++++++++++++++++++++--------------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index fb1c01f6d..d4a71e7cb 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -146,7 +146,7 @@ export class Sync { /** * TODO document **/ - public async queueGetRequest (path: string): Promise { + async queueGetRequest (path: string): Promise { return new Promise((resolve, reject) => { if (!this.rs.remote.connected) { reject('cannot fulfill maxAge requirement - remote is not connected'); @@ -162,7 +162,7 @@ export class Sync { }); } - public corruptServerItemsMap (itemsMap): boolean { + corruptServerItemsMap (itemsMap): boolean { if ((typeof(itemsMap) !== 'object') || (Array.isArray(itemsMap))) { return true; } @@ -190,7 +190,7 @@ export class Sync { return false; } - public corruptItemsMap (itemsMap): boolean { + corruptItemsMap (itemsMap): boolean { if ((typeof(itemsMap) !== 'object') || (Array.isArray(itemsMap))) { return true; } @@ -204,7 +204,7 @@ export class Sync { return false; } - public corruptRevision (rev): boolean { + corruptRevision (rev): boolean { return ((typeof(rev) !== 'object') || (Array.isArray(rev)) || (rev.revision && typeof(rev.revision) !== 'string') || @@ -215,7 +215,7 @@ export class Sync { (rev.itemsMap && this.corruptItemsMap(rev.itemsMap))); } - public isCorrupt (node: RSNode): boolean { + isCorrupt (node: RSNode): boolean { return ((typeof(node) !== 'object') || (Array.isArray(node)) || (typeof(node.path) !== 'string') || @@ -257,12 +257,12 @@ export class Sync { .then((): number => num); } - public inConflict (node: RSNode): boolean { + inConflict (node: RSNode): boolean { return (node.local && node.remote && (node.remote.body !== undefined || node.remote.itemsMap)); } - public needsRefresh (node: RSNode): boolean { + needsRefresh (node: RSNode): boolean { if (node.common) { if (!node.common.timestamp) { return true; @@ -272,7 +272,7 @@ export class Sync { return false; } - public needsFetch (node: RSNode): boolean { + needsFetch (node: RSNode): boolean { if (this.inConflict(node)) { return true; } @@ -289,7 +289,7 @@ export class Sync { return false; } - public needsPush (node: RSNode): boolean { + needsPush (node: RSNode): boolean { if (this.inConflict(node)) { return false; } @@ -298,15 +298,15 @@ export class Sync { } } - public needsRemotePut (node: RSNode): boolean { + needsRemotePut (node: RSNode): boolean { return node.local && node.local.body; } - public needsRemoteDelete (node: RSNode): boolean { + needsRemoteDelete (node: RSNode): boolean { return node.local && node.local.body === false; } - public getParentPath (path: string): string { + getParentPath (path: string): string { const parts = path.match(/^(.*\/)([^\/]+\/?)$/); if (parts) { @@ -316,7 +316,7 @@ export class Sync { } } - public deleteChildPathsFromTasks (): void { + deleteChildPathsFromTasks (): void { for (const path in this._tasks) { const paths = pathsFromRoot(path); @@ -361,7 +361,7 @@ export class Sync { /** * Flush nodes from cache after sync to remote **/ - public flush (nodes: RSNodes): RSNodes { + flush (nodes: RSNodes): RSNodes { for (const path in nodes) { // Strategy is 'FLUSH' and no local changes exist if (this.rs.caching.checkPath(path) === 'FLUSH' && @@ -376,7 +376,7 @@ export class Sync { /** * Sync one path **/ - public doTask (path: string): object { + doTask (path: string): object { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; // First fetch: @@ -435,7 +435,7 @@ export class Sync { /** * TODO document **/ - public autoMergeFolder (node: RSNode): RSNode { + autoMergeFolder (node: RSNode): RSNode { if (node.remote.itemsMap) { node.common = node.remote; delete node.remote; @@ -462,7 +462,7 @@ export class Sync { /** * TODO document **/ - public autoMergeDocument (node: RSNode): RSNode { + autoMergeDocument (node: RSNode): RSNode { if (hasNoRemoteChanges(node)) { node = mergeMutualDeletion(node); delete node.remote; @@ -496,7 +496,7 @@ export class Sync { /** * TODO document **/ - public autoMerge (node: RSNode): RSNode { + autoMerge (node: RSNode): RSNode { if (node.remote) { if (node.local) { if (isFolder(node.path)) { @@ -551,7 +551,7 @@ export class Sync { return node; } - public async updateCommonTimestamp (path: string, revision: string): Promise { + async updateCommonTimestamp (path: string, revision: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { if (nodes[path] && nodes[path].common && @@ -565,7 +565,7 @@ export class Sync { /** * TODO document **/ - public async markChildren (path, itemsMap, changedNodes: RSNodes, missingChildren): Promise { + async markChildren (path, itemsMap, changedNodes: RSNodes, missingChildren): Promise { const paths = []; const meta = {}; const recurse = {}; @@ -662,7 +662,7 @@ export class Sync { /** * TODO document **/ - public async deleteRemoteTrees (paths: string[], changedNodes: RSNodes): Promise { + async deleteRemoteTrees (paths: string[], changedNodes: RSNodes): Promise { if (paths.length === 0) { return changedNodes; } const nodes = await this.rs.local.getNodes(paths); @@ -705,7 +705,7 @@ export class Sync { /** * TODO document **/ - public async completeFetch (path: string, bodyOrItemsMap: object, contentType: string, revision: string): Promise { + async completeFetch (path: string, bodyOrItemsMap: object, contentType: string, revision: string): Promise { let paths: string[]; let parentPath: string; const pathsFromRootArr = pathsFromRoot(path); @@ -779,7 +779,7 @@ export class Sync { /** * TODO document **/ - public async completePush (path: string, action, conflict, revision: string): Promise { + async completePush (path: string, action, conflict, revision: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; @@ -832,7 +832,7 @@ export class Sync { /** * TODO document **/ - public async dealWithFailure (path: string): Promise { + async dealWithFailure (path: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { if (nodes[path]) { delete nodes[path].push; @@ -841,7 +841,7 @@ export class Sync { }); } - public interpretStatus (statusCode: string | number): ResponseStatus { + interpretStatus (statusCode: string | number): ResponseStatus { const status: ResponseStatus = { statusCode: statusCode, successful: undefined, @@ -878,7 +878,7 @@ export class Sync { /** * TODO document **/ - public async handleGetResponse (path: string, status: ResponseStatus, bodyOrItemsMap, contentType: string, revision: string): Promise { + async handleGetResponse (path: string, status: ResponseStatus, bodyOrItemsMap, contentType: string, revision: string): Promise { if (status.notFound) { if (isFolder(path)) { bodyOrItemsMap = {}; @@ -909,7 +909,7 @@ export class Sync { } } - public handleResponse (path: string, action, r): Promise { + handleResponse (path: string, action, r): Promise { const status = this.interpretStatus(r.statusCode); if (status.successful) { @@ -943,7 +943,7 @@ export class Sync { /** * TODO document **/ - public finishTask (task: SyncTask, queueTask = true): void | Promise { + finishTask (task: SyncTask, queueTask = true): void | Promise { if (task.action === undefined) { delete this._running[task.path]; return; @@ -1091,10 +1091,10 @@ export class Sync { } /** - * TODO document + * Add a sync task for the given path **/ // eslint-disable-next-line @typescript-eslint/no-explicit-any - public addTask (path: string, cb?: (...args: any[]) => any): void { + addTask (path: string, cb?: (...args: any[]) => any): void { if (!this._tasks[path]) { this._tasks[path] = []; } From f9e307ff23b6937fb76aabcf760cb7cafbf5d575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 16:28:40 +0400 Subject: [PATCH 15/47] Port more tests --- test/unit/sync-suite.js | 134 ---------------------------------------- test/unit/sync.test.mjs | 77 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 134 deletions(-) diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index 78bc283d9..67853bba3 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -53,140 +53,6 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require }, tests: [ - { - desc: "go through the request-queue with 4-8 requests at a time", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - var tmpDoTask = env.rs.sync.doTask; - env.rs.sync.doTask = function() { - return Promise.resolve({ - action: undefined, - promise: Promise.resolve() - }); - }; - env.rs.sync.numThreads = 5; - env.rs.sync.rs.remote.connected = true; - env.rs.remote.online = true; - env.rs.sync._tasks = { - '/foo1/': true, - '/foo2/': true, - '/foo3': true, - '/foo4/': true, - '/foo/5': true, - '/foo/6/': true, - '/foo7/': true, - '/foo8': true, - '/fo/o/9/': true - }; - env.rs.sync._running = {}; - env.rs.sync.doTasks(); - test.assertAnd(env.rs.sync._tasks, { - '/foo1/': true, - '/foo2/': true, - '/foo3': true, - '/foo4/': true, - '/foo/5': true, - '/foo/6/': true, - '/foo7/': true, - '/foo8': true, - '/fo/o/9/': true - }); - test.assertAnd(Object.getOwnPropertyNames(env.rs.sync._running).sort(), [ - '/foo1/', - '/foo2/', - '/foo3', - '/foo4/', - '/foo/5' - ].sort()); - test.done(); - env.rs.sync.doTask = tmpDoTask; - } - }, - - { - desc: "sync will attempt only one request, at low frequency, when not online", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - var tmpDoTask = env.rs.sync.doTask; - env.rs.sync.doTask = function() { - return Promise.resolve({ - action: undefined, - promise: Promise.resolve() - }); - }; - env.rs.sync.numThreads = 5; - env.rs.sync.rs.remote.connected = true; - env.rs.sync.rs.remote.online = false; - env.rs.sync._tasks = { - '/foo1/': true, - '/foo2/': true, - '/foo3': true, - '/foo4/': true, - '/foo/5': true, - '/foo/6/': true, - '/foo7/': true, - '/foo8': true, - '/fo/o/9/': true - }; - env.rs.sync._running = {}; - env.rs.sync.doTasks(); - test.assertAnd(env.rs.sync._tasks, { - '/foo1/': true, - '/foo2/': true, - '/foo3': true, - '/foo4/': true, - '/foo/5': true, - '/foo/6/': true, - '/foo7/': true, - '/foo8': true, - '/fo/o/9/': true - }); - test.assertAnd(Object.getOwnPropertyNames(env.rs.sync._running).sort(), [ - '/foo1/' - ]); - test.done(); - env.rs.sync.doTask = tmpDoTask; - } - }, - - { - desc: "sync will not attempt any requests when not connected", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - env.rs.sync.numThreads = 5; - env.rs.remote.connected = false; - env.rs.sync._tasks = { - '/foo1/': true, - '/foo2/': true, - '/foo3': true, - '/foo4/': true, - '/foo/5': true, - '/foo/6/': true, - '/foo7/': true, - '/foo8': true, - '/fo/o/9/': true - }; - env.rs.sync._running = {}; - env.rs.sync.doTasks(); - test.assertAnd(env.rs.sync._tasks, { - '/foo1/': true, - '/foo2/': true, - '/foo3': true, - '/foo4/': true, - '/foo/5': true, - '/foo/6/': true, - '/foo7/': true, - '/foo8': true, - '/fo/o/9/': true - }); - test.assertAnd(env.rs.sync._running, {}); - test.done(); - } - }, - { desc: "sync will stop the current task cycle on timeout", run: function(env, test) { diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 50d553112..96c7c50b7 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -9,6 +9,7 @@ import FakeAccess from '../helpers/fake-access.mjs'; describe("Sync", function() { beforeEach(function(done) { + this.original = {}; this.rs = new RemoteStorage(); Object.defineProperty(this.rs, 'access', { value: new FakeAccess(), @@ -19,7 +20,9 @@ describe("Sync", function() { this.rs.on('features-loaded', () => { this.rs._handlers['connected'] = []; this.rs.local = new InMemoryStorage(); + this.rs.syncStopped = true; this.rs.sync = new Sync(this.rs); + this.original.doTasks = this.rs.sync.doTasks; this.rs.sync.doTasks = () => { return true; }; done(); }); @@ -254,6 +257,80 @@ describe("Sync", function() { }); }); + describe("#doTasks", function() { + beforeEach(function() { + this.rs.sync.doTasks = this.original.doTasks; + }); + + describe("when not connected", function() { + beforeEach(function() { + this.rs.remote.connected = false; + this.rs.sync._tasks = { '/foo1/': [] }; + }); + + it("does not attempt any requests", async function() { + this.rs.sync.doTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ '/foo1/': [] }); + expect(Object.keys(this.rs.sync._running).length).to.equal(0); + }); + }); + + describe("when offline", function() { + beforeEach(function() { + this.rs.remote.connected = true; + this.rs.remote.online = false; + this.rs.sync.doTask = async function() { + return { action: undefined, promise: Promise.resolve() }; + }; + this.rs.sync._tasks = { + '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], + '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [] + }; + }); + + it("attempts only one request, at low frequency", async function() { + this.rs.sync.doTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ + '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], + '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [] + }); + expect(Object.keys(this.rs.sync._running)).to.deep.equal([ + '/foo1/' + ]); + }); + }); + + describe("normal operation", function() { + beforeEach(function() { + this.rs.remote.connected = true; + this.rs.remote.online = true; + this.rs.sync.numThreads = 5; + + this.rs.sync.doTask = async function() { + return { action: undefined, promise: Promise.resolve() }; + }; + this.rs.sync._tasks = { + '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], + '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [] + }; + }); + + it("attempts requests according to the number of threads configured", async function() { + this.rs.sync.doTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ + '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], + '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [], + }); + expect(Object.keys(this.rs.sync._running)).to.deep.equal([ + '/foo1/', '/foo2/', '/foo3', '/foo4/', '/foo/5' + ]); + }); + }); + }); + describe("#finishTask", function() { beforeEach(function() { this.tasks = { From 4724fcbc6842bf747d9cecb33156c0115c29cc53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 17:39:45 +0400 Subject: [PATCH 16/47] Types and docs for Sync --- src/cachinglayer.ts | 30 ++++---- src/interfaces/queued_request_response.ts | 2 +- src/interfaces/rs_node.ts | 24 +++++- src/sync.ts | 91 ++++++++++++++--------- 4 files changed, 96 insertions(+), 51 deletions(-) diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts index 88f1bf6a3..d158635c2 100644 --- a/src/cachinglayer.ts +++ b/src/cachinglayer.ts @@ -1,7 +1,7 @@ import type { ChangeObj } from './interfaces/change_obj'; import type { QueuedRequestResponse } from './interfaces/queued_request_response'; import type { RSEvent } from './interfaces/rs_event'; -import type { RSNode, RSNodes, ProcessNodes } from './interfaces/rs_node'; +import type { RSItem, RSNode, RSNodes, ProcessNodes } from './interfaces/rs_node'; import EventHandling from './eventhandling'; import config from './config'; import log from './log'; @@ -14,7 +14,7 @@ import { pathsFromRoot } from './util'; -function getLatest (node: RSNode): any { +function getLatest (node: RSNode): RSItem { if (typeof (node) !== 'object' || typeof (node.path) !== 'string') { return; } @@ -138,15 +138,15 @@ abstract class CachingLayer { if (typeof (maxAge) === 'number') { return this.getNodes(pathsFromRoot(path)) .then((objs) => { - const node: RSNode = getLatest(objs[path]); + const item: RSItem = getLatest(objs[path]); if (isOutdated(objs, maxAge)) { return queueGetRequest(path); - } else if (node) { + } else if (item) { return { statusCode: 200, - body: node.body || node.itemsMap, - contentType: node.contentType + body: item.body || item.itemsMap, + contentType: item.contentType }; } else { return { statusCode: 404 }; @@ -155,21 +155,21 @@ abstract class CachingLayer { } else { return this.getNodes([path]) .then((objs) => { - const node: RSNode = getLatest(objs[path]); + const item: RSItem = getLatest(objs[path]); - if (node) { + if (item) { if (isFolder(path)) { - for (const i in node.itemsMap) { + for (const i in item.itemsMap) { // the hasOwnProperty check here is only because our jshint settings require it: - if (node.itemsMap.hasOwnProperty(i) && node.itemsMap[i] === false) { - delete node.itemsMap[i]; + if (item.itemsMap.hasOwnProperty(i) && item.itemsMap[i] === false) { + delete item.itemsMap[i]; } } } return { statusCode: 200, - body: node.body || node.itemsMap, - contentType: node.contentType + body: item.body || item.itemsMap, + contentType: item.contentType }; } else { return {statusCode: 404}; @@ -178,7 +178,7 @@ abstract class CachingLayer { } } - async put (path: string, body: unknown, contentType: string): Promise { + async put (path: string, body: string, contentType: string): Promise { const paths = pathsFromRoot(path); function _processNodes(nodePaths: string[], nodes: RSNodes): RSNodes { @@ -186,7 +186,7 @@ abstract class CachingLayer { for (let i = 0, len = nodePaths.length; i < len; i++) { const nodePath = nodePaths[i]; let node = nodes[nodePath]; - let previous: RSNode; + let previous: RSItem; if (!node) { nodes[nodePath] = node = makeNode(nodePath); diff --git a/src/interfaces/queued_request_response.ts b/src/interfaces/queued_request_response.ts index 7f80701b4..db5555c43 100644 --- a/src/interfaces/queued_request_response.ts +++ b/src/interfaces/queued_request_response.ts @@ -1,6 +1,6 @@ export type QueuedRequestResponse = { statusCode: number; - body?: object; + body?: string | object; contentType?: string; revision?: string; }; diff --git a/src/interfaces/rs_node.ts b/src/interfaces/rs_node.ts index 2f11a0c26..07cffe90c 100644 --- a/src/interfaces/rs_node.ts +++ b/src/interfaces/rs_node.ts @@ -1,5 +1,27 @@ +export type RSItem = { + body?: string | object | false; + contentType?: string; + contentLength?: number; + revision?: string; + timestamp?: number; + itemsMap?: { + [key: string]: any; + }; + previousBody?: string | object | false; + previousContentType?: string; +} + export type RSNode = { - [key: string]: any; + path: string; + body?: string | object | false; + contentType?: string; + itemsMap?: { + [key: string]: any; + }; + common?: RSItem; + local?: RSItem; + remote?: RSItem; + push?: RSItem; }; export type RSNodes = { diff --git a/src/sync.ts b/src/sync.ts index d4a71e7cb..335622dc9 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1,4 +1,6 @@ +import type RemoteStorage from './remotestorage'; import type { RSNode, RSNodes } from './interfaces/rs_node'; +import type { QueuedRequestResponse } from './interfaces/queued_request_response'; import config from './config'; import Env from './env'; import EventHandling from './eventhandling'; @@ -14,7 +16,6 @@ import { isDocument, pathsFromRoot } from './util'; -import type RemoteStorage from './remotestorage'; let setupSync, syncOnConnect; @@ -29,7 +30,7 @@ interface ResponseStatus { } interface SyncTask { - action: any; + action: string; path: string; promise: Promise; } @@ -48,7 +49,7 @@ function isStaleChild (node: RSNode): boolean { } function hasCommonRevision (node: RSNode): boolean { - return node.common && node.common.revision; + return !!node.common && !!node.common.revision; } function hasNoRemoteChanges (node: RSNode): boolean { @@ -101,28 +102,44 @@ function handleVisibility (env, rs): void { export class Sync { rs: RemoteStorage; - numThreads: number; + /** + * Maximum number of parallel requests to execute + **/ + numThreads: number = 10; + + /** + * Sync done? `false` when periodic sync is currently running + **/ done: boolean; + + /** + * Sync stopped entirely + **/ stopped: boolean; - _tasks: { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - [key: string]: ((...args: any[]) => any)[]; - }; - // TODO define in more detail - _running: object; - _timeStarted: object; + /** + * Paths queued for sync, sometimes with callbacks + **/ + _tasks: { [key: string]: Array<() => void>; } = {}; + + /** + * Promises of currently running sync tasks per path + **/ + _running: { [key: string]: Promise; } = {}; + + /** + * Start times of current sync per path + **/ + _timeStarted: { [key: string]: number; } = {}; + + /** + * TODO document + **/ _finishedTasks: SyncTask[] = []; constructor (remoteStorage: RemoteStorage) { this.rs = remoteStorage; - this._tasks = {}; - this._running = {}; - this._timeStarted = {}; - - this.numThreads = 10; - this.rs.local.onDiff(path => { this.addTask(path); this.doTasks(); @@ -144,9 +161,11 @@ export class Sync { } /** - * TODO document + * When getting a path from the caching layer, this function might be handed + * in to first check if it was updated on the remote, in order to fulfill a + * maxAge requirement **/ - async queueGetRequest (path: string): Promise { + async queueGetRequest (path: string): Promise { return new Promise((resolve, reject) => { if (!this.rs.remote.connected) { reject('cannot fulfill maxAge requirement - remote is not connected'); @@ -258,8 +277,8 @@ export class Sync { } inConflict (node: RSNode): boolean { - return (node.local && node.remote && - (node.remote.body !== undefined || node.remote.itemsMap)); + return (!!node.local && !!node.remote && + (node.remote.body !== undefined || !!node.remote.itemsMap)); } needsRefresh (node: RSNode): boolean { @@ -299,7 +318,7 @@ export class Sync { } needsRemotePut (node: RSNode): boolean { - return node.local && node.local.body; + return node.local && typeof(node.local.body) === "string"; } needsRemoteDelete (node: RSNode): boolean { @@ -376,7 +395,7 @@ export class Sync { /** * Sync one path **/ - doTask (path: string): object { + async doTask (path: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; // First fetch: @@ -402,7 +421,12 @@ export class Sync { } return taskFor('put', path, - this.rs.remote.put(path, node.push.body, node.push.contentType, options) + this.rs.remote.put( + path, + node.push.body as string, + node.push.contentType, + options + ) ); }); } @@ -833,12 +857,12 @@ export class Sync { * TODO document **/ async dealWithFailure (path: string): Promise { - return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { - if (nodes[path]) { - delete nodes[path].push; - return this.rs.local.setNodes(this.flush(nodes)); - } - }); + const nodes = await this.rs.local.getNodes([path]); + + if (nodes[path]) { + delete nodes[path].push; + return this.rs.local.setNodes(this.flush(nodes)); + } } interpretStatus (statusCode: string | number): ResponseStatus { @@ -943,7 +967,7 @@ export class Sync { /** * TODO document **/ - finishTask (task: SyncTask, queueTask = true): void | Promise { + finishTask (task: SyncTask, queueTask: boolean = true): void | Promise { if (task.action === undefined) { delete this._running[task.path]; return; @@ -1066,8 +1090,7 @@ export class Sync { for (path in this._tasks) { if (!this._running[path]) { this._timeStarted[path] = this.now(); - this._running[path] = this.doTask(path); - this._running[path].then(this.finishTask.bind(this)); + this._running[path] = this.doTask(path).then(this.finishTask.bind(this)); numAdded++; if (numAdded >= numToAdd) { break; } } @@ -1094,7 +1117,7 @@ export class Sync { * Add a sync task for the given path **/ // eslint-disable-next-line @typescript-eslint/no-explicit-any - addTask (path: string, cb?: (...args: any[]) => any): void { + addTask (path: string, cb?: () => void): void { if (!this._tasks[path]) { this._tasks[path] = []; } From 823e28f940507db1e814039d185861ccaae51595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 18:03:30 +0400 Subject: [PATCH 17/47] Formatting --- src/sync.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 335622dc9..7f9b1c8c4 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -45,7 +45,8 @@ function nodeChanged (node: RSNode, etag: string): boolean { } function isStaleChild (node: RSNode): boolean { - return node.remote && node.remote.revision && !node.remote.itemsMap && !node.remote.body; + return !!node.remote && !!node.remote.revision && + !node.remote.itemsMap && !node.remote.body; } function hasCommonRevision (node: RSNode): boolean { @@ -58,8 +59,8 @@ function hasNoRemoteChanges (node: RSNode): boolean { return false; } return (node.common.body === undefined && node.remote.body === false) || - (node.remote.body === node.common.body && - node.remote.contentType === node.common.contentType); + (node.remote.body === node.common.body && + node.remote.contentType === node.common.contentType); } function mergeMutualDeletion (node: RSNode): RSNode { @@ -793,10 +794,7 @@ export class Sync { nodes[path] = this.autoMerge(node); - return { - toBeSaved: nodes, - missingChildren: missingChildren - }; + return { toBeSaved: nodes, missingChildren }; }); } From f50278df858cff6edeaeaffd7adca49fd4c5bf73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 19:10:43 +0400 Subject: [PATCH 18/47] Add 'sync-started' event closes #1173 --- src/remotestorage.ts | 6 +++++- src/sync.ts | 10 +++++++--- test/unit/sync-suite.js | 2 +- test/unit/sync.test.mjs | 22 ++++++++++++++++++++++ test/unit/versioning-suite.js | 2 +- 5 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/remotestorage.ts b/src/remotestorage.ts index 0792e71d7..7707e097c 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -208,6 +208,10 @@ enum ApiKeyType { * * Emitted when a network request completes * + * ### `sync-started` + * + * Emitted when a sync procedure has started. + * * ### `sync-req-done` * * Emitted when a single sync request has finished. Callback functions @@ -370,7 +374,7 @@ export class RemoteStorage { this.addEvents([ 'ready', 'authing', 'connecting', 'connected', 'disconnected', 'not-connected', 'conflict', 'error', 'features-loaded', - 'sync-interval-change', 'sync-req-done', 'sync-done', + 'sync-interval-change', 'sync-started', 'sync-req-done', 'sync-done', 'wire-busy', 'wire-done', 'network-offline', 'network-online' ]); diff --git a/src/sync.ts b/src/sync.ts index 7f9b1c8c4..9f2f42743 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1079,11 +1079,15 @@ export class Sync { doTasks (): boolean { const numToHave = this.tasksWanted(); const numToAdd = numToHave - Object.keys(this._running).length; - if (numToAdd <= 0) { return true; } - let numAdded: number = 0; - let path: string; + // `this.done` is `true` for immediate sync and `false` for + // periodic sync + if (this.hasTasks() && !this.done) { + this.rs._emit('sync-started'); + } + + let numAdded: number = 0, path: string; for (path in this._tasks) { if (!this._running[path]) { diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js index 67853bba3..45185357f 100644 --- a/test/unit/sync-suite.js +++ b/test/unit/sync-suite.js @@ -35,7 +35,7 @@ define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require beforeEach: function(env, test) { env.rs = new RemoteStorage(); - env.rs.addEvents(['sync-req-done', 'sync-done', 'ready', 'connected', 'sync-interval-change', 'error']); + env.rs.addEvents(['sync-started', 'sync-req-done', 'sync-done', 'ready', 'connected', 'sync-interval-change', 'error']); env.rs.local = new InMemoryStorage(); env.rs.remote = new FakeRemote(); env.rs.access = new FakeAccess(); diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 96c7c50b7..c9b83e98d 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -97,11 +97,14 @@ describe("Sync", function() { describe("#sync", function() { it("returns immediately when not connected", async function() { + let syncStarted = false; let syncDone = false; this.rs.remote.connected = false; + this.rs.on('sync-started', () => { syncStarted = true; }); this.rs.on('sync-done', () => { syncDone = true; }); await this.rs.sync.sync().then(() => { + expect(syncStarted).to.be.false; expect(syncDone).to.be.false; }); }); @@ -123,6 +126,13 @@ describe("Sync", function() { await this.rs.sync.sync(); expect(this.spies.doTasks.callCount).to.equal(1); }); + + it("does not emit 'sync-started'", async function() { + let syncStarted = false; + this.rs.on('sync-started', () => { syncStarted = true; }); + await this.rs.sync.sync(); + expect(syncStarted).to.be.false; + }); }); describe("with sync desired but not enough tasks queued", function() { @@ -142,6 +152,13 @@ describe("Sync", function() { await this.rs.sync.sync(); expect(this.spies.doTasks.callCount).to.equal(2); }); + + it("does not emit 'sync-started'", async function() { + let syncStarted = false; + this.rs.on('sync-started', () => { syncStarted = true; }); + await this.rs.sync.sync(); + expect(syncStarted).to.be.false; + }); }); }); @@ -317,6 +334,11 @@ describe("Sync", function() { }; }); + it("emits 'sync-started'", async function(done) { + this.rs.on('sync-started', () => { done(); }); + this.rs.sync.doTasks(); + }); + it("attempts requests according to the number of threads configured", async function() { this.rs.sync.doTasks(); diff --git a/test/unit/versioning-suite.js b/test/unit/versioning-suite.js index 5cc4fa4c5..f32fa4c97 100644 --- a/test/unit/versioning-suite.js +++ b/test/unit/versioning-suite.js @@ -358,7 +358,7 @@ define(['./build/config', './build/eventhandling', './build/inmemorystorage', beforeEach: function(env, test){ env.rs = new RemoteStorage(); - env.rs.addEvents(['sync-req-done', 'sync-done', 'ready', 'error']); + env.rs.addEvents(['sync-started', 'sync-req-done', 'sync-done', 'ready', 'error']); env.rs.local = new InMemoryStorage(); env.rs.remote = new FakeRemote(); env.rs.access = new FakeAccess(); From c6ea79cf6fcb61237f32381e4e0dda97fa72da18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 6 Jun 2025 19:39:54 +0400 Subject: [PATCH 19/47] Finish porting sync tests --- .eslintrc.js | 2 +- src/sync.ts | 2 +- test/unit/cachinglayer.test.mjs | 98 ++++ test/unit/sync-suite.js | 943 -------------------------------- test/unit/sync.test.mjs | 896 +++++++++++++++++++++++++++--- 5 files changed, 926 insertions(+), 1015 deletions(-) create mode 100644 test/unit/cachinglayer.test.mjs delete mode 100644 test/unit/sync-suite.js diff --git a/.eslintrc.js b/.eslintrc.js index ba48a5d2d..9fcc48dae 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -92,7 +92,7 @@ module.exports = { ], "overrides": [ { - "files": ["*.test.ts"], + "files": ["*.test.mjs"], "rules": { "max-statements": 0, "@typescript-eslint/no-empty-function": 0, diff --git a/src/sync.ts b/src/sync.ts index 9f2f42743..863e6a169 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -340,7 +340,7 @@ export class Sync { for (const path in this._tasks) { const paths = pathsFromRoot(path); - for (let i=1; i { + this.rs._handlers['connected'] = []; + this.rs.syncStopped = true; + this.rs.sync = new Sync(this.rs); + done(); + }); + }); + + afterEach(function() { + if (this.rs.sync) { Sync._rs_cleanup(this.rs); } + this.rs = undefined; + }); + + describe("#get", function() { + beforeEach(async function() { + this.rs.remote.connected = true; + this.rs.remote.online = true; + this.rs.caching.enable("/foo/"); + + fetchMock.mock( + { url: "end:/foo/"}, + { status: 200, body: {}, headers: { 'Content-Type': 'application/ld+json', 'ETag': '"new-and-fresh"' } } + ); + fetchMock.mock( + { url: "end:/foo/one"}, + { status: 200, body: 'body', headers: { 'Content-Type': 'text/plain', 'ETag': '"brandnew-and-fresh"' } } + ); + fetchMock.mock( + { url: "end:/foo/two"}, + { status: 200, body: 'ohai', headers: { 'Content-Type': 'text/plain', 'ETag': '"brandnew-and-even-fresher"' } } + ); + + await this.rs.local.setNodes({ + '/foo/': { + path: '/foo/', + common: { + itemsMap: {}, + contentType: 'application/ld+json', + timestamp: new Date().getTime() - 60000, + revision: 'oldie-but-goodie' + } + }, + '/foo/one': { + path: '/foo/one', + common: { + body: 'old data', + contentType: 'text/plain', + timestamp: new Date().getTime() - 60000, + revision: 'oldie-but-goodie' + } + } + }); + }); + + afterEach(() => { + fetchMock.reset(); + }); + + it("resolves with the local data when younger than maxAge", async function() { + const res = await this.rs.local.get('/foo/one', 120000, this.rs.sync.queueGetRequest.bind(this.rs.sync)); + expect(res).to.deep.equal({ + statusCode: 200, body: 'old data', contentType: 'text/plain' + }); + }); + + it("resolves with updated data when cache older than maxAge", async function() { + const res = await this.rs.local.get('/foo/one', 5, this.rs.sync.queueGetRequest.bind(this.rs.sync)); + expect(res).to.deep.equal({ + statusCode: 200, body: 'body', contentType: 'text/plain' + }); + }); + + it("resolves with data from remote when no local data cached", async function() { + const res = await this.rs.local.get('/foo/two', 5, this.rs.sync.queueGetRequest.bind(this.rs.sync)); + expect(res).to.deep.equal({ + statusCode: 200, body: 'ohai', contentType: 'text/plain' + }); + }); + }); +}); diff --git a/test/unit/sync-suite.js b/test/unit/sync-suite.js deleted file mode 100644 index 45185357f..000000000 --- a/test/unit/sync-suite.js +++ /dev/null @@ -1,943 +0,0 @@ -if (typeof(define) !== 'function') { - var define = require('amdefine'); -} - -define(['./build/util', 'require', 'test/helpers/mocks'], function(util, require, mocks) { - var suites = []; - - suites.push({ - name: "Sync Suite", - desc: "testing the sync adapter instance", - - setup: function(env, test){ - mocks.defineMocks(env); - - global.Authorize = require('./build/authorize'); - global.UnauthorizedError = require('./build/unauthorized-error'); - global.config = require('./build/config'); - global.EventHandling = require('./build/eventhandling').EventHandling; - global.Sync = require('./build/sync').Sync; - global.InMemoryStorage = require('./build/inmemorystorage'); - - class RemoteStorage { static log () {} } - util.applyMixins(RemoteStorage, [EventHandling]); - global.RemoteStorage = RemoteStorage; - - var RS = require('./build/remotestorage').RemoteStorage; - RemoteStorage.prototype.stopSync = RS.prototype.stopSync; - RemoteStorage.prototype.startSync = RS.prototype.startSync; - RemoteStorage.prototype.getSyncInterval = RS.prototype.getSyncInterval; - RemoteStorage.prototype.setSyncInterval = RS.prototype.setSyncInterval; - RemoteStorage.prototype.isValidInterval = RS.prototype.isValidInterval; - - test.done(); - }, - - beforeEach: function(env, test) { - env.rs = new RemoteStorage(); - env.rs.addEvents(['sync-started', 'sync-req-done', 'sync-done', 'ready', 'connected', 'sync-interval-change', 'error']); - env.rs.local = new InMemoryStorage(); - env.rs.remote = new FakeRemote(); - env.rs.access = new FakeAccess(); - env.rs.caching = new FakeCaching(); - env.rs.sync = new Sync(env.rs); - global.remoteStorage = env.rs; - - env.rs.sync.numThreads = 5; - env.rs.remote.connected = true; - env.rs.remote.online = true; - env.rs.sync._tasks = {}; - env.rs.sync._running = {}; - - test.done(); - }, - - tests: [ - { - desc: "sync will stop the current task cycle on timeout", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - - env.rs.caching._responses['/foo1/'] = 'ALL'; - env.rs.caching._responses['/foo2/'] = 'ALL'; - env.rs.remote._responses[['get', '/foo1/' ]] = {statusCode: 'timeout'}; - env.rs.remote._responses[['get', '/foo2/' ]] = {statusCode: 200}; - - env.rs.sync.numThreads = 1; - env.rs.remote.connected = true; - env.rs.sync._tasks = { - '/foo1/': true, - '/foo2/': true - }; - env.rs.sync._running = {}; - - env.rs.on('sync-done', function() { - test.assertAnd(env.rs.sync._running, {}); - test.assertAnd(env.rs.sync._tasks, { - '/foo1/': true, - '/foo2/': true - }); - test.done(); - }); - - env.rs.sync.doTasks(); - } - }, - - { - desc: "collectDiffTasks will not enqueue requests outside the access scope", - run: function(env, test) { - env.rs.sync.numThreads = 5; - env.rs.remote.connected = true; - env.rs.remote.online = true; - env.rs.local.setNodes({ - '/foo/bar': { - path: '/foo/bar', - common: { body: 'asdf', contentType: 'qwer', revision: '987', timestamp: 1234567890123 }, - local: { body: false, timestamp: 1234567891000 } - }, - '/public/nothings/bar': { - path: '/public/nothings/bar', - common: { revision: '987', timestamp: 1234567890123 }, - local: { body: 'asdf', contentType: 'qwer', timestamp: 1234567891000 } - } - }).then(function() { - env.rs.sync.collectDiffTasks(); - test.assertAnd(env.rs.sync._tasks, {'/foo/bar': []}); - //env.rs.sync.on('done', function() { - test.done(); - //}); - }); - } - }, - - { - desc: "collectDiffTasks retrieves body and Content-Type when a new remote revision is set inside rw access scope", - run: function(env, test) { - env.rs.local.setNodes({ - '/nothings/bar': { - path: '/nothings/bar', - common: { body: 'asdf', contentType: 'qwer', revision: '987', timestamp: 1234567890123 }, - remote: { revision: '900' } - }, - '/public/writings/bar': { - path: '/public/writings/bar', - common: { revision: '987', timestamp: 1234567890123 }, - remote: { revision: 'a' } - } - }).then(function() { - return env.rs.sync.collectDiffTasks(); - }).then(function() { - test.assertAnd(env.rs.sync._tasks, { - '/public/writings/bar': [] - }); - //env.rs.sync.on('done', function() { - test.done(); - //}); - }); - } - }, - - { - desc: "sync will discard corrupt cache nodes but try to retrieve them if node.path is readable", - run: function(env, test) { - env.rs.access.set('writings', 'r'); - env.rs.access.set('writings', 'rw'); - env.rs.local.setNodes({ - '/writings/bar': { - path: '/writings/bar', - common: { body: function() {}, contentType: 3, revision: '987', timestamp: 1234567890123 }, - remote: { revision: 'yes' }, - push: 'no' - }, - '/writings/baz': { - common: { body: function() {}, contentType: 3, revision: '987', timestamp: 1234567890123 }, - remote: { revision: 'yes' }, - push: 'no' - }, - '/writings/baf': { - path: '/writings/baf', - remote: { revision: 'yes' } - } - }).then(function() { - return env.rs.sync.collectDiffTasks(); - }).then(function(num) { - test.assertAnd(num, 2); - test.assertAnd(env.rs.sync._tasks, { - '/writings/bar': [], - '/writings/baf': [] - }); - test.done(); - }); - } - }, - - { - desc: "sync will reject its promise if the cache is not available", - run: function(env, test) { - var tmp = env.rs.forAllNodes; - env.rs.local.forAllNodes = function(cb) { - return Promise.reject('i am broken, deal with it!'); - }; - env.rs.sync.sync().then(function() { - test.result(false, 'sync was supposed to reject its promise'); - }, function(err) { - test.assertAnd(err, new Error('local cache unavailable')); - test.done(); - }); - env.rs.forAllNodes = tmp; - } - }, - - { - desc: "sync will fulfill its promise as long as the cache is available", - run: function(env, test) { - env.rs.sync.sync().then(function() { - test.done(); - }, function(err) { - test.result(false, 'sync was supposed to fulfill its promise'); - }); - } - }, - - { - desc: "get with maxAge requirement is rejected if remote is not connected", - run: function(env, test) { - env.rs.remote.connected = false; - return env.rs.local.get('asdf', 2, env.rs.sync.queueGetRequest.bind(env.rs.sync)).then(function() { - test.result(false, 'should have been rejected'); - }, function(err) { - test.done(); - }); - } - }, - - { - desc: "get with maxAge requirement is rejected if remote is not online", - willFail: true, - run: function (env, test) { - env.rs.remote.online = false; - return env.rs.local.get('asdf', 2, env.rs.sync.queueGetRequest.bind(env.rs.sync)); - } - }, - - { - desc: "get with maxAge fetches from remote when no local data exists", - run: function (env, test) { - env.rs.caching._responses['/'] = 'ALL'; - env.rs.caching._responses['/foo'] = 'ALL'; - env.rs.remote._responses[['get', '/foo' ]] = {statusCode: 200, body: 'body', contentType: 'text/plain', revision: 'revision'}; - env.rs.remote.connected = true; - - return env.rs.local.get('/foo', 5, env.rs.sync.queueGetRequest.bind(env.rs.sync)).then(function (r) { - test.assertAnd(r.statusCode, 200); - test.assertAnd(r.body, 'body'); - test.assertAnd(r.contentType, 'text/plain'); - test.done(); - }); - } - }, - - { - desc: "get with maxAge fetches from remote when local data is too old", - run: function (env, test) { - env.rs.caching._responses['/'] = 'ALL'; - env.rs.caching._responses['/foo'] = 'ALL'; - env.rs.remote._responses[['get', '/foo' ]] = {statusCode: 200, body: 'body', contentType: 'text/plain', revision: 'revision'}; - env.rs.remote.connected = true; - - return env.rs.local.setNodes({ - '/foo': { - path: '/foo', - common: { - body: 'old data', - contentType: 'text/html', - timestamp: new Date().getTime() - 60000 - } - } - }).then(function () { - env.rs.local.get('/foo', 5, env.rs.sync.queueGetRequest.bind(env.rs.sync)).then(function (r) { - test.assertAnd(r.statusCode, 200); - test.assertAnd(r.body, 'body'); - test.assertAnd(r.contentType, 'text/plain'); - test.done(); - }); - }); - } - }, - - { - desc: "get with maxAge returns local data when it's not outdated", - run: function (env, test) { - return env.rs.local.setNodes({ - '/foo': { - path: '/foo', - common: { - body: 'old data', - contentType: 'text/html', - timestamp: new Date().getTime() - 60000 - } - } - }).then(function () { - env.rs.local.get('/foo', 120000, env.rs.sync.queueGetRequest.bind(env.rs.sync)).then(function (r) { - test.assertAnd(r.statusCode, 200); - test.assertAnd(r.body, 'old data'); - test.assertAnd(r.contentType, 'text/html'); - test.done(); - }); - }); - } - }, - - { - desc: "completePush for put without conflict updates 'common', removes 'local' and 'push' from node", - run: function(env, test) { - env.rs.caching._responses['/foo/bar'] = 'ALL'; - - env.rs.local.setNodes({ - '/foo/bar': { - path: '/foo/bar', - local: { - body: {foo: 'bar'}, - contentType: 'application/json', - timestamp: 1234567891000 - }, - push: { - body: {foo: 'bar'}, - contentType: 'application/json', - timestamp: 1234567891234 - } - } - }).then(function() { - return env.rs.sync.completePush('/foo/bar', 'put', false, '12345'); - }).then(function() { - env.rs.local.getNodes(['/foo/bar']).then(function(nodes) { - var node = nodes['/foo/bar']; - test.assertAnd(node.common.body, {foo: 'bar'}); - test.assertAnd(node.common.contentType, 'application/json'); - test.assertTypeAnd(node.local, 'undefined'); - test.assertType(node.remote, 'undefined'); - }); - }); - } - }, - - { - desc: "fetching a new document deletes the local itemsMap from parent folder when there are no other pending changes", - run: function(env, test) { - env.rs.caching._responses['/foo/'] = 'ALL'; - env.rs.caching._responses['/foo/new'] = 'ALL'; - env.rs.local.setNodes({ - '/foo/': { - path: '/foo/', - common: { - itemsMap: { - 'bar': true, - 'new': true - }, - revision: 'remotefolderrevision', - timestamp: 1397210425598, - }, - local: { - itemsMap: { - 'bar': true, - 'new': false - }, - revision: 'localfolderrevision', - timestamp: 1397210425612 - } - }, - '/foo/bar': { - path: '/foo/bar', - common: { - body: { foo: 'bar' }, - contentType: 'application/json', - revision: 'docrevision', - timestamp: 1234567891000 - } - } - }).then(function() { - return env.rs.sync.handleResponse('/foo/new', 'get', {statusCode: 200, body: { foo: 'new' }, contentType: 'application/json', revision: 'newrevision'}); - }).then(function() { - env.rs.local.getNodes(['/foo/']).then(function(nodes) { - var parentNode = nodes['/foo/']; - test.assertAnd(parentNode.common.itemsMap, { 'bar': true, 'new': true }); - test.assertTypeAnd(parentNode.local, 'undefined'); - test.assertType(parentNode.remote, 'undefined'); - }); - }, test.fail).catch(test.fail); - } - }, - - { - desc: "fetching a new document keeps the local itemsMap from parent folder when there are other pending changes", - run: function(env, test) { - env.rs.caching._responses['/foo/'] = 'ALL'; - env.rs.caching._responses['/foo/new'] = 'ALL'; - - env.rs.local.setNodes({ - '/foo/': { - path: '/foo/', - common: { - itemsMap: { - 'bar': true, - 'new': true, - 'othernew': true - }, - revision: 'remotefolderrevision', - timestamp: 1397210425598, - }, - local: { - itemsMap: { - 'bar': true, - 'new': false, - 'othernew': false - }, - revision: 'localfolderrevision', - timestamp: 1397210425612 - } - }, - '/foo/bar': { - path: '/foo/bar', - common: { - body: { foo: 'bar' }, - contentType: 'application/json', - revision: 'docrevision', - timestamp: 1234567891000 - } - } - }).then(function() { - return env.rs.sync.handleResponse('/foo/new', 'get', {statusCode: 200, body: { foo: 'new' }, contentType: 'application/json', revision: 'newrevision'}); - }).then(function() { - env.rs.local.getNodes(['/foo/']).then(function(nodes) { - var parentNode = nodes['/foo/']; - test.assertAnd(parentNode.common.itemsMap, { 'bar': true, 'new': true, 'othernew': true }); - test.assertAnd(parentNode.local.itemsMap, { 'bar': true, 'new': true, 'othernew': false }); - test.assertType(parentNode.remote, 'undefined'); - }); - }, test.fail).catch(test.fail); - } - }, - - { - desc: "when a document has been deleted remotely, it's removed from local itemsMap", - run: function(env, test) { - env.rs.caching._responses['/foo/'] = 'ALL'; - env.rs.caching._responses['/foo/new'] = 'ALL'; - env.rs.caching._responses['/foo/old'] = 'ALL'; - - var newItemsMap = { - 'bar': { 'ETag': 'bardocrevision' }, - 'new': { 'ETag': 'newdocrevision' } - }; - - env.rs.local.setNodes({ - '/foo/': { - path: '/foo/', - common: { - itemsMap: { - 'bar': true, - 'old': true, - }, - revision: 'remotefolderrevision', - timestamp: 1397210425598, - }, - local: { - itemsMap: { - 'bar': true, - 'old': true, - 'new': true - }, - revision: 'localfolderrevision', - timestamp: 1397210425612 - } - }, - '/foo/bar': { - path: '/foo/bar', - common: { - body: { foo: 'bar' }, - contentType: 'application/json', - revision: 'bardocrevision', - timestamp: 1234567891000 - } - }, - '/foo/old': { - path: '/foo/old', - common: { - body: { foo: 'old' }, - contentType: 'application/json', - revision: 'olddocrevision', - timestamp: 1234567891000 - } - }, - '/foo/new': { - path: '/foo/new', - local: { - body: { foo: 'new' }, - contentType: 'application/json', - timestamp: 1234567891000 - } - } - }).then(function() { - return env.rs.sync.handleResponse('/foo/', 'get', {statusCode: 200, body: newItemsMap, contentType: 'application/json', revision: 'newfolderrevision'}); - }).then(function() { - env.rs.local.getNodes(['/foo/', '/foo/old']).then(function(nodes) { - var parentNode = nodes['/foo/']; - - test.assertAnd(parentNode.common.itemsMap, { 'bar': true, 'new': true }); - test.assertTypeAnd(parentNode.local, 'undefined'); - test.assertTypeAnd(parentNode.remote, 'undefined'); - test.assertType(nodes['/foo/old'], 'undefined'); - }); - }, test.fail).catch(test.fail); - } - }, - - { - desc: "Setting a wrong (string) sync interval throws an error", - run: function(env, test) { - try { - env.rs.setSyncInterval('60000'); - test.result(false, "setSyncInterval() didn't fail"); - } catch(e) { - test.result(true); - } - } - }, - - { - desc: "Setting a wrong (small) sync interval throws an error", - run: function(env, test) { - try { - env.rs.setSyncInterval(10); - test.result(false, "setSyncInterval() didn't fail"); - } catch(e) { - test.result(true); - } - } - }, - - { - desc: "handleResponse emits Unauthorized error for status 401", - run: function(env, test) { - env.rs.on('error', function(err) { - if (err instanceof UnauthorizedError) { - test.result(true, "handleResponse() emitted Unauthorized error"); - } else { - test.result(false); - } - }); - return env.rs.sync.handleResponse(undefined, undefined, {statusCode: 401}).catch(function(){}); - } - }, - - { - desc: "handleResponse emits an error for unhandled status codes", - run: function(env, test) { - var errorEmitted, errorThrown; - - env.rs.on('error', function(err) { - if (err instanceof Error) { - test.assertAnd(err.message, 'HTTP response code 418 received.'); - errorEmitted = true; - if (errorThrown) { - test.done(); - } - } else { - test.result(false); - } - }); - - return env.rs.sync.handleResponse(undefined, undefined, {statusCode: 418}).then(function() { - test.result(false); - }, function(error) { - test.assertAnd(error.message, 'HTTP response code 418 received.'); - errorThrown = true; - if (errorEmitted) { - test.done(); - } - }); - } - }, - - { - desc: "deleteRemoteTrees returns a promise", - run: function(env, test) { - env.rs.sync.deleteRemoteTrees([], {changed: 'nodes'}).then(function(ret1) { - test.assertAnd(ret1, {changed: 'nodes'}); - env.rs.sync.deleteRemoteTrees(['foo'], {}).then(function(ret2) { - test.assertAnd(ret2, undefined); - test.done(); - }); - }); - } - }, - - { - desc: "autoMergeDocument leaves a remote version in place even if it has only a revision", - run: function(env, test) { - var node = { - path: 'foo', - common: { body: 'foo', contentType: 'bloo', revision: 'common' }, - local: { body: 'floo', contentType: 'blaloo' }, - remote: { revision: 'conflict' } - }; - var result = env.rs.sync.autoMergeDocument(node); - test.assertAnd(result, node); - test.done(); - } - }, - - { - desc: "autoMergeDocument on an empty node removes a remote version if it has a null revision", - run: function(env, test) { - var node = { - path: 'foo', - common: {}, - remote: { revision: null } - }; - var remoteRemoved = { - path: 'foo', - common: {} - }; - var result = env.rs.sync.autoMergeDocument(node); - test.assertAnd(result, remoteRemoved); - test.done(); - } - }, - - { - desc: "autoMergeDocument merges mutual deletions (#737)", - run: function(env, test) { - var node = { - "path": "/myfavoritedrinks/b", - "common": { - "timestamp": 1405488508303 - }, - "local": { - "body": false, - "timestamp": 1405488515881 - }, - "remote": { - "body": false, - "timestamp": 1405488740722 - } - }; - var localAndRemoteRemoved = { - "path": "/myfavoritedrinks/b", - "common": { - "timestamp": 1405488508303 - } - }; - var result = env.rs.sync.autoMergeDocument(node); - test.assertAnd(result, localAndRemoteRemoved); - test.done(); - } - }, - - { - desc: "autoMerge auto-merges and sends out a change event if a node changed", - run: function(env, test) { - var node = { - path: 'foo', - common: { body: 'old value', contentType: 'old content-type', revision: 'common' }, - remote: { body: 'new value', contentType: 'new content-type', revision: 'remote' } - }; - var merged = { - path: 'foo', - common: { body: 'new value', contentType: 'new content-type', revision: 'remote' } - }; - var otherDone = false; - - env.rs.sync.rs.local.emitChange = function(changeEvent) { - test.assertAnd(changeEvent, { - origin: 'remote', - path: 'foo', - newValue: 'new value', - oldValue: 'old value', - newContentType: 'new content-type', - oldContentType: 'old content-type' - }); - if (otherDone) { - test.done(); - } else { - otherDone = true; - } - }; - var result = env.rs.sync.autoMerge(node); - test.assertAnd(result, merged); - if (otherDone) { - test.done(); - } else { - otherDone = true; - } - } - }, - - { - desc: "autoMerge removes the whole node on 404 and sends out a change event if a node existed before", - run: function(env, test) { - var node = { - path: 'foo', - common: { body: 'foo', contentType: 'bloo', revision: 'common' }, - remote: { body: false, revision: 'null' } - }; - var otherDone = false; - env.rs.sync.rs.local.emitChange = function(obj) { - test.assertAnd(obj, { - origin: 'remote', - path: 'foo', - oldValue: 'foo', - newValue: undefined, - oldContentType: 'bloo', - newContentType: undefined - }); - if (otherDone) { - test.done(); - } else { - otherDone = true; - } - }; - var result = env.rs.sync.autoMerge(node); - test.assertAnd(result, undefined); - if (otherDone) { - test.done(); - } else { - otherDone = true; - } - } - }, - - { - desc: "autoMerge doesn't send out a change event on 404 if a node didn't exist before", - run: function(env, test) { - var node = { - path: 'foo', - common: {}, - remote: { body: false, revision: 'null' } - }; - env.rs.sync.rs.local.emitChange = function(obj) { - test.result(false, 'should not have emitted '+JSON.stringify(obj)); - }; - var result = env.rs.sync.autoMerge(node); - test.assertAnd(result, undefined); - setTimeout(function() { - test.done(); - }, 100); - } - }, - - { - desc: "completePush of a conflict sets revision to the incoming revision, or to 'conflict' if null", - run: function(env, test) { - var getNodes = env.rs.sync.rs.local.getNodes, - setNodes = env.rs.sync.rs.local.setNodes; - env.rs.caching._responses['foo'] = 'ALL'; - env.rs.sync.rs.local.getNodes = function(paths) { - test.assertAnd(paths, ['foo']); - return Promise.resolve({ - foo: { - path: 'foo', - common: { body: 'foo', contentType: 'bloo', revision: 'common' }, - local: { body: 'floo', contentType: 'blaloo' }, - push: { body: 'floo', contentType: 'blaloo' } - } - }); - }; - env.rs.sync.rs.local.setNodes = function(nodes) { - test.assert(nodes, { - foo: { - path: 'foo', - common: { body: 'foo', contentType: 'bloo', revision: 'common' }, - local: { body: 'floo', contentType: 'blaloo' }, - remote: { revision: '123', timestamp: 1234567890123 } - } - }); - setTimeout(function() { - env.rs.sync.getNodes = getNodes; - env.rs.sync.setNodes = setNodes; - test.done(); - }, 0); - }; - env.rs.sync.now = function() { return 1234567890123; }; - env.rs.sync.completePush('foo', 'put', true, '123'); - } - }, - - { - desc: "sync multiple new documents under the same folder while there are local changes should merge folder without missing any document", - run: function(env, test) { - test.assertAnd(env.rs.sync._tasks, {}); - test.assertAnd(env.rs.sync._running, {}); - var tmpDoTask = env.rs.sync.doTask; - - env.rs.sync.doTask = function(path) { - return Promise.resolve({ - action: 'get', - path, - promise: Promise.resolve({ - statusCode: 200, - body: path, - contentType: 'text/plain', - revision: path - }) - }); - }; - - env.rs.sync.rs.remote.connected = true; - env.rs.remote.online = true; - env.rs.sync._tasks = { - '/foo/2': true, - '/foo/3': true, - '/foo/4': true, - '/foo/5': true, - '/foo/6': true, - '/foo/7': true, - '/foo/8': true, - '/foo/9': true, - '/foo/10': true, - '/foo/11': true, - }; - env.rs.sync._running = {}; - env.rs.sync.doTasks(); - test.assertAnd(env.rs.sync._tasks, { - '/foo/2': true, - '/foo/3': true, - '/foo/4': true, - '/foo/5': true, - '/foo/6': true, - '/foo/7': true, - '/foo/8': true, - '/foo/9': true, - '/foo/10': true, - '/foo/11': true, - }); - test.assertAnd(Object.getOwnPropertyNames(env.rs.sync._running).sort(), [ - '/foo/2', - '/foo/3', - '/foo/4', - '/foo/5', - '/foo/6', - ].sort()); - env.rs.caching._responses['/'] = 'ALL'; - env.rs.caching._responses['/foo/'] = 'ALL'; - env.rs.caching._responses['/foo/1'] = 'ALL'; - for (const t of Object.keys(env.rs.sync._tasks)) { - env.rs.caching._responses[t] = 'ALL'; - } - env.rs.local.setNodes({ - '/foo/': { - path: '/foo/', - common: { - itemsMap: { - '1': true, - }, - revision: 'remotefolderrevision', - timestamp: 1397210425598, - }, - local: { - itemsMap: { - '1': true - }, - revision: 'localfolderrevision', - timestamp: 1397210425612 - } - }, - '/foo/1': { - path: '/foo/1', - local: { body: '/foo/1', contentType: 'test/plain', timestamp: 1234567891000 } - } - }).then(() => { - setTimeout(function () { - env.rs.local.getNodes(['/foo/']).then(function(nodes) { - var node = nodes['/foo/']; - test.assertAnd(node.local.itemsMap, { - '1': true, - '2': true, - '3': true, - '4': true, - '5': true, - '6': true, - '7': true, - '8': true, - '9': true, - '10': true, - '11': true, - }); - env.rs.sync.doTask = tmpDoTask; - test.done() - }); - }, 100) - }); - } - }, - - { - desc: "calling handleResponse multiple times without waiting for it to return should only update index for latest change", - run: function(env, test) { - env.rs.caching._responses['/'] = 'ALL'; - env.rs.caching._responses['/foo/'] = 'ALL'; - env.rs.caching._responses['/foo/2'] = 'ALL'; - env.rs.caching._responses['/foo/3'] = 'ALL'; - - const nodesFetched = [ - { - path: '/foo/2', - action: 'get', - response: {statusCode: 200, body: { foo: 'new' }, contentType: 'application/json', revision: 'newrevision'}, - }, - { - path: '/foo/3', - action: 'get', - response: {statusCode: 200, body: { foo: 'new' }, contentType: 'application/json', revision: 'newrevision'}, - } - ] - - env.rs.local.setNodes({ - '/foo/': { - path: '/foo/', - common: { - itemsMap: { - '1': false, - }, - revision: 'remotefolderrevision', - timestamp: 1397210425598, - }, - local: { - itemsMap: { - '1': true - }, - revision: 'localfolderrevision', - timestamp: 1397210425612 - } - }, - '/foo/1': { - path: '/foo/1', - local: { body: {asdf: 'asdf'}, contentType: 'application/json', timestamp: 1234567891000 } - } - }).then(function() { - const promises = [] - for (const node of nodesFetched){ - promises.push(env.rs.sync.handleResponse(node.path, node.action, node.response)) - } - return Promise.all(promises) - }).then(function(){ - return env.rs.local.getNodes(['/foo/', '/foo/1', '/foo/2', '/foo/3']) - }).then(function(nodes) { - var parentNode = nodes['/foo/']; - test.assertAnd(parentNode.local.itemsMap, { - '1': true, - '3': true - }); - test.assertType(nodes['/foo/2'], 'object'); - test.assertType(nodes['/foo/3'], 'object'); - test.done() - }); - } - }, - ] - }); - - return suites; -}); diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index c9b83e98d..faeaed1d0 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -1,24 +1,28 @@ -import 'mocha'; -import { expect } from 'chai'; -import sinon from 'sinon'; +import "mocha"; +import chai, { expect } from "chai"; +import chaiAsPromised from "chai-as-promised"; +import sinon from "sinon"; -import InMemoryStorage from '../../build/inmemorystorage.js'; -import { RemoteStorage } from '../../build/remotestorage.js'; -import { Sync } from '../../build/sync.js'; -import FakeAccess from '../helpers/fake-access.mjs'; +import InMemoryStorage from "../../build/inmemorystorage.js"; +import { RemoteStorage } from "../../build/remotestorage.js"; +import { Sync } from "../../build/sync.js"; +import FakeAccess from "../helpers/fake-access.mjs"; +import UnauthorizedError from "./build/unauthorized-error.js"; + +chai.use(chaiAsPromised); describe("Sync", function() { beforeEach(function(done) { this.original = {}; this.rs = new RemoteStorage(); - Object.defineProperty(this.rs, 'access', { + Object.defineProperty(this.rs, "access", { value: new FakeAccess(), writable: true, configurable: true }); - this.rs.on('features-loaded', () => { - this.rs._handlers['connected'] = []; + this.rs.on("features-loaded", () => { + this.rs._handlers["connected"] = []; this.rs.local = new InMemoryStorage(); this.rs.syncStopped = true; this.rs.sync = new Sync(this.rs); @@ -43,7 +47,7 @@ describe("Sync", function() { }; Sync._rs_init(this.rs); - this.rs._emit('ready'); + this.rs._emit("ready"); }); it("starts syncing on connect", function(done) { @@ -54,19 +58,19 @@ describe("Sync", function() { }; Sync._rs_init(this.rs); - this.rs._emit('connected'); + this.rs._emit("connected"); }); it("removes the 'connected' handler when it's called", function() { Sync._rs_init(this.rs); - this.rs._emit('connected'); - expect(this.rs._handlers['connected'].length).to.equal(0); + this.rs._emit("connected"); + expect(this.rs._handlers["connected"].length).to.equal(0); }); it("doesn't interfere with custom 'connected' handlers", function(done) { - this.rs.on('connected', done); + this.rs.on("connected", done); Sync._rs_init(this.rs); - this.rs._emit('connected'); + this.rs._emit("connected"); }); }); @@ -81,12 +85,12 @@ describe("Sync", function() { describe("#getParentPath", function() { it("returns the correct values", async function() { const paths = { - '/a': '/', - '/a/': '/', - '/a/b': '/a/', - '/a/b/': '/a/', - '/a/b/c': '/a/b/', - '/a/b/c/': '/a/b/' + "/a": "/", + "/a/": "/", + "/a/b": "/a/", + "/a/b/": "/a/", + "/a/b/c": "/a/b/", + "/a/b/c/": "/a/b/" }; for (const path in paths) { @@ -100,8 +104,8 @@ describe("Sync", function() { let syncStarted = false; let syncDone = false; this.rs.remote.connected = false; - this.rs.on('sync-started', () => { syncStarted = true; }); - this.rs.on('sync-done', () => { syncDone = true; }); + this.rs.on("sync-started", () => { syncStarted = true; }); + this.rs.on("sync-done", () => { syncDone = true; }); await this.rs.sync.sync().then(() => { expect(syncStarted).to.be.false; @@ -112,8 +116,8 @@ describe("Sync", function() { describe("with no need to sync", function() { beforeEach(function() { this.spies = { - doTasks: sinon.stub(this.rs.sync, 'doTasks').returns(true), - collectTasks: sinon.spy(this.rs.sync, 'collectTasks') + doTasks: sinon.stub(this.rs.sync, "doTasks").returns(true), + collectTasks: sinon.spy(this.rs.sync, "collectTasks") }; }); @@ -129,7 +133,7 @@ describe("Sync", function() { it("does not emit 'sync-started'", async function() { let syncStarted = false; - this.rs.on('sync-started', () => { syncStarted = true; }); + this.rs.on("sync-started", () => { syncStarted = true; }); await this.rs.sync.sync(); expect(syncStarted).to.be.false; }); @@ -138,8 +142,8 @@ describe("Sync", function() { describe("with sync desired but not enough tasks queued", function() { beforeEach(function() { this.spies = { - doTasks: sinon.stub(this.rs.sync, 'doTasks').returns(false), - collectTasks: sinon.spy(this.rs.sync, 'collectTasks') + doTasks: sinon.stub(this.rs.sync, "doTasks").returns(false), + collectTasks: sinon.spy(this.rs.sync, "collectTasks") }; }); @@ -155,18 +159,32 @@ describe("Sync", function() { it("does not emit 'sync-started'", async function() { let syncStarted = false; - this.rs.on('sync-started', () => { syncStarted = true; }); + this.rs.on("sync-started", () => { syncStarted = true; }); await this.rs.sync.sync(); expect(syncStarted).to.be.false; }); }); + + describe("when the cache back-end is erroring", function() { + beforeEach(function() { + this.rs.sync.doTasks = () => false; // trigger collectTasks + this.rs.local.forAllNodes = async function() { + throw new Error("I am broken, deal with it!"); + }; + }); + + it("rejects with error", async function() { + await expect(this.rs.sync.sync()).to.eventually + .be.rejectedWith(/cache unavailable/); + }); + }); }); describe("#collectTasks", function() { beforeEach(function() { this.spies = { - collectDiffTasks: sinon.spy(this.rs.sync, 'collectDiffTasks'), - collectRefreshTasks: sinon.spy(this.rs.sync, 'collectRefreshTasks') + collectDiffTasks: sinon.spy(this.rs.sync, "collectDiffTasks"), + collectRefreshTasks: sinon.spy(this.rs.sync, "collectRefreshTasks") }; }); @@ -198,8 +216,8 @@ describe("Sync", function() { beforeEach(function() { sinon.restore(); this.spies = { - collectDiffTasks: sinon.stub(this.rs.sync, 'collectDiffTasks').returns(1), - collectRefreshTasks: sinon.spy(this.rs.sync, 'collectRefreshTasks') + collectDiffTasks: sinon.stub(this.rs.sync, "collectDiffTasks").returns(1), + collectRefreshTasks: sinon.spy(this.rs.sync, "collectRefreshTasks") }; }); @@ -218,8 +236,8 @@ describe("Sync", function() { beforeEach(function() { sinon.restore(); this.spies = { - collectDiffTasks: sinon.stub(this.rs.sync, 'collectDiffTasks').returns(0), - collectRefreshTasks: sinon.spy(this.rs.sync, 'collectRefreshTasks') + collectDiffTasks: sinon.stub(this.rs.sync, "collectDiffTasks").returns(0), + collectRefreshTasks: sinon.spy(this.rs.sync, "collectRefreshTasks") }; }); @@ -239,26 +257,26 @@ describe("Sync", function() { beforeEach(function() { this.fakeCallback = function() {}; this.rs.sync.addTask( - '/foo/bar/and/then/some', this.fakeCallback + "/foo/bar/and/then/some", this.fakeCallback ); this.rs.sync.now = () => 1234568654321; this.rs.local.forAllNodes = async function(cb) { cb({ - path: '/foo/bar/and/then/some', //should be overruled by ancestor /foo/ba/ - common: { body: 'off', contentType: 'cT', timestamp: 1234567890123 } + path: "/foo/bar/and/then/some", //should be overruled by ancestor /foo/ba/ + common: { body: "off", contentType: "cT", timestamp: 1234567890123 } }); cb({ - path: '/foo/bar/', //should retrieve /foo/ to get its new revision - common: { body: 'off', contentType: 'cT', timestamp: 1234567890124 } + path: "/foo/bar/", //should retrieve /foo/ to get its new revision + common: { body: "off", contentType: "cT", timestamp: 1234567890124 } }); cb({ - path: '/read/access/', // should retrieve - common: { body: 'off', contentType: 'cT', timestamp: 1234567890124 } + path: "/read/access/", // should retrieve + common: { body: "off", contentType: "cT", timestamp: 1234567890124 } }); cb({ - path: '/no/access/', // no access - common: { body: 'off', contentType: 'cT', timestamp: 1234567890124 } + path: "/no/access/", // no access + common: { body: "off", contentType: "cT", timestamp: 1234567890124 } }); }; @@ -268,8 +286,74 @@ describe("Sync", function() { await this.rs.sync.collectRefreshTasks(); expect(this.rs.sync._tasks).to.deep.equal({ - '/foo/': [this.fakeCallback], // inherited from task '/foo/bar/and/then/some' - '/read/access/': [] + "/foo/": [this.fakeCallback], // inherited from task '/foo/bar/and/then/some' + "/read/access/": [] + }); + }); + }); + + describe("#collectDiffTasks", function() { + beforeEach(function() { + }); + + describe("", function() { + beforeEach(function() { + }); + }); + + + it("does not enqueue tasks outside permitted access scopes", async function() { + await this.rs.local.setNodes({ + "/foo/bar": { + path: "/foo/bar", + common: { body: "asdf", contentType: "qwer", revision: "987", timestamp: 1234567890123 }, + local: { body: false, timestamp: 1234567891000 } + }, + "/public/nothings/bar": { + path: "/public/nothings/bar", + common: { revision: "987", timestamp: 1234567890123 }, + local: { body: "asdf", contentType: "qwer", timestamp: 1234567891000 } + } + }); + await this.rs.sync.collectDiffTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ + "/foo/bar": [] + }); + }); + + it("enqueues a task when a new remote revision has been set", async function() { + await this.rs.local.setNodes({ + "/public/writings/bar": { + path: "/public/writings/bar", + common: { revision: "987", timestamp: 1234567890123 }, + remote: { revision: "a" } + } + }); + await this.rs.sync.collectDiffTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ + "/public/writings/bar": [] + }); + }); + + it("enqueues tasks for corrupt cache nodes with a readable path", async function() { + await this.rs.local.setNodes({ + "/writings/baz": { + // corrupt, but no path + common: { body: "foo", contentType: "text/plain", revision: "123456abcdef", timestamp: 1234567890123 }, + remote: { revision: "yes" }, + push: "no" + }, + "/writings/baf": { + path: "/writings/baf", + remote: { revision: "yes" } + } + }); + await this.rs.sync.collectDiffTasks(); + + expect(this.rs.sync._tasks).to.deep.equal({ + "/writings/baf": [] }); }); }); @@ -282,13 +366,13 @@ describe("Sync", function() { describe("when not connected", function() { beforeEach(function() { this.rs.remote.connected = false; - this.rs.sync._tasks = { '/foo1/': [] }; + this.rs.sync._tasks = { "/foo1/": [] }; }); it("does not attempt any requests", async function() { this.rs.sync.doTasks(); - expect(this.rs.sync._tasks).to.deep.equal({ '/foo1/': [] }); + expect(this.rs.sync._tasks).to.deep.equal({ "/foo1/": [] }); expect(Object.keys(this.rs.sync._running).length).to.equal(0); }); }); @@ -301,8 +385,8 @@ describe("Sync", function() { return { action: undefined, promise: Promise.resolve() }; }; this.rs.sync._tasks = { - '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], - '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [] + "/foo1/": [], "/foo2/": [], "/foo3": [], "/foo4/": [], + "/foo/5": [], "/foo/6/": [], "/foo7/": [], "/foo8": [] }; }); @@ -310,11 +394,11 @@ describe("Sync", function() { this.rs.sync.doTasks(); expect(this.rs.sync._tasks).to.deep.equal({ - '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], - '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [] + "/foo1/": [], "/foo2/": [], "/foo3": [], "/foo4/": [], + "/foo/5": [], "/foo/6/": [], "/foo7/": [], "/foo8": [] }); expect(Object.keys(this.rs.sync._running)).to.deep.equal([ - '/foo1/' + "/foo1/" ]); }); }); @@ -329,13 +413,13 @@ describe("Sync", function() { return { action: undefined, promise: Promise.resolve() }; }; this.rs.sync._tasks = { - '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], - '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [] + "/foo1/": [], "/foo2/": [], "/foo3": [], "/foo4/": [], + "/foo/5": [], "/foo/6/": [], "/foo7/": [], "/foo8": [] }; }); - it("emits 'sync-started'", async function(done) { - this.rs.on('sync-started', () => { done(); }); + it("emits 'sync-started'", function(done) { + this.rs.on("sync-started", () => { done(); }); this.rs.sync.doTasks(); }); @@ -343,13 +427,522 @@ describe("Sync", function() { this.rs.sync.doTasks(); expect(this.rs.sync._tasks).to.deep.equal({ - '/foo1/': [], '/foo2/': [], '/foo3': [], '/foo4/': [], - '/foo/5': [], '/foo/6/': [], '/foo7/': [], '/foo8': [], + "/foo1/": [], "/foo2/": [], "/foo3": [], "/foo4/": [], + "/foo/5": [], "/foo/6/": [], "/foo7/": [], "/foo8": [], }); expect(Object.keys(this.rs.sync._running)).to.deep.equal([ - '/foo1/', '/foo2/', '/foo3', '/foo4/', '/foo/5' + "/foo1/", "/foo2/", "/foo3", "/foo4/", "/foo/5" + ]); + }); + }); + }); + + describe("#handleResponse", function() { + describe("Fetching a new document", function() { + describe("with no pending changes in parent folder", function() { + beforeEach(async function() { + await this.rs.local.setNodes({ + "/foo/": { + path: "/foo/", + common: { + itemsMap: { "bar": true, "new": true }, + revision: "remotefolderrevision", + timestamp: 1397210425598, + }, + local: { + itemsMap: { "bar": true, "new": false }, + revision: "localfolderrevision", + timestamp: 1397210425612 + } + }, + "/foo/bar": { + path: "/foo/bar", + common: { + body: { foo: "bar" }, + contentType: "application/json", + revision: "docrevision", + timestamp: 1234567891000 + } + } + }); + + await this.rs.sync.handleResponse("/foo/new", "get", { + statusCode: 200, body: { foo: "new" }, + contentType: "application/json", + revision: "newrevision" + }); + + const nodes = await this.rs.local.getNodes(["/foo/"]); + this.parentNode = nodes["/foo/"]; + }); + + it("updates common itemsMap of parent folder", async function() { + expect(this.parentNode.common.itemsMap).to.deep.equal({ + "bar": true, "new": true + }); + }); + + it("deletes local and remote itemsMap from parent folder", async function() { + expect(this.parentNode.local).to.be.undefined; + expect(this.parentNode.remote).to.be.undefined; + }); + }); + + describe("with other pending changes in parent folder", function() { + beforeEach(async function() { + await this.rs.local.setNodes({ + "/foo/": { + path: "/foo/", + common: { + itemsMap: { "bar": true, "new": true, "othernew": true }, + revision: "remotefolderrevision", + timestamp: 1397210425598, + }, + local: { + itemsMap: { "bar": true, "new": false, "othernew": false }, + revision: "localfolderrevision", + timestamp: 1397210425612 + } + }, + "/foo/bar": { + path: "/foo/bar", + common: { + body: { foo: "bar" }, + contentType: "application/json", + revision: "docrevision", + timestamp: 1234567891000 + } + } + }); + + await this.rs.sync.handleResponse("/foo/new", "get", { + statusCode: 200, body: { foo: "new" }, + contentType: "application/json", + revision: "newrevision" + }); + + const nodes = await this.rs.local.getNodes(["/foo/"]); + this.parentNode = nodes["/foo/"]; + }); + + it("updates common itemsMap of parent folder", async function() { + expect(this.parentNode.common.itemsMap).to.deep.equal({ + "bar": true, "new": true, "othernew": true + }); + }); + + it("keeps local itemsMap of parent folder", async function() { + expect(this.parentNode.local.itemsMap).to.deep.equal({ + "bar": true, "new": true, "othernew": false + }); + }); + + it("deletes remote itemsMap from parent folder", async function() { + expect(this.parentNode.remote).to.be.undefined; + }); + }); + + describe("called multiple times without waiting", function() { + beforeEach(async function() { + this.nodesFetched = [ + { + path: "/foo/2", action: "get", + response: { + statusCode: 200, body: { foo: "new" }, + contentType: "application/json", + revision: "newrevision2" + }, + }, + { + path: "/foo/3", action: "get", + response: { + statusCode: 200, body: { foo: "new" }, + contentType: "application/json", + revision: "newrevision3" + }, + } + ]; + + await this.rs.local.setNodes({ + "/foo/": { + path: "/foo/", + common: { + itemsMap: { "1": false }, + revision: "remotefolderrevision", + timestamp: 1397210425598, + }, + local: { + itemsMap: { "1": true }, + revision: "localfolderrevision", + timestamp: 1397210425612 + } + }, + "/foo/1": { + path: "/foo/1", + local: { + body: { asdf: "asdf" }, + contentType: "application/json", + timestamp: 1234567891000 + } + } + }); + + const promises = []; + for (const res of this.nodesFetched){ + promises.push(this.rs.sync.handleResponse( + res.path, res.action, res.response + )); + } + await Promise.all(promises); + + this.nodes = await this.rs.local.getNodes([ + "/foo/", "/foo/1", "/foo/2", "/foo/3" + ]); + }); + + it("only updates local index for the latest change", function() { + expect(this.nodes["/foo/"].local.itemsMap).to.deep.equal({ + "1": true, "3": true // does not include "2" + }); + }); + + it("caches the fetched nodes", function() { + expect(this.nodes["/foo/2"]).to.be.an('object'); + expect(this.nodes["/foo/3"]).to.be.an('object'); + }); + }); + }); + + describe("Document deleted on remote", function() { + beforeEach(async function() { + const newItemsMap = { + "bar": { "ETag": "bardocrevision" }, + "new": { "ETag": "newdocrevision" } + }; + + this.rs.local.setNodes({ + "/foo/": { + path: "/foo/", + common: { + itemsMap: { "bar": true, "old": true, }, + revision: "remotefolderrevision", + timestamp: 1397210425598, + }, + local: { + itemsMap: { "bar": true, "old": true, "new": true }, + revision: "localfolderrevision", + timestamp: 1397210425612 + } + }, + "/foo/bar": { + path: "/foo/bar", + common: { + body: { foo: "bar" }, + contentType: "application/json", + revision: "bardocrevision", + timestamp: 1234567891000 + } + }, + "/foo/old": { + path: "/foo/old", + common: { + body: { foo: "old" }, + contentType: "application/json", + revision: "olddocrevision", + timestamp: 1234567891000 + } + }, + "/foo/new": { + path: "/foo/new", + local: { + body: { foo: "new" }, + contentType: "application/json", + timestamp: 1234567891000 + } + } + }); + + await this.rs.sync.handleResponse('/foo/', 'get', { + statusCode: 200, body: newItemsMap, + contentType: 'application/json', + revision: 'newfolderrevision' + }); + + this.nodes = await this.rs.local.getNodes([ + "/foo/", "/foo/old" ]); }); + + it("removes the document from the parent node's common itemsMap", async function() { + expect(this.nodes["/foo/"].common.itemsMap).to.deep.equal({ + "bar": true, "new": 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/old"]).to.be.undefined; + }); + }); + + describe("PUT without conflict", function() { + beforeEach(async function() { + this.rs.local.setNodes({ + "/foo/bar": { + path: "/foo/bar", + local: { + body: { foo: "bar" }, + contentType: "application/json", + timestamp: 1234567891000 + }, + push: { + body: { foo: "bar" }, + contentType: "application/json", + timestamp: 1234567891234 + } + } + }); + + await this.rs.sync.handleResponse("/foo/bar", "put", { + statusCode: 201, revision: "newrevision" + }); + + const nodes = await this.rs.local.getNodes(["/foo/bar"]); + this.node = nodes["/foo/bar"]; + }); + + it("updates 'common'", async function() { + expect(this.node.common.body).to.deep.equal({foo: "bar"}); + expect(this.node.common.contentType).to.equal("application/json"); + }); + + it("removes 'local' and 'push' from node", function() { + expect(this.node.local).to.be.undefined; + expect(this.node.push).to.be.undefined; + }); + }); + + describe("PUT with conflict", function() { + beforeEach(async function() { + this.rs.local.setNodes({ + "/foo/bar": { + path: "/foo/bar", + common: { + body: "foo", contentType: "bloo", + revision: "common" + }, + local: { body: "floo", contentType: "blaloo" }, + push: { body: "floo", contentType: "blaloo" } + } + }); + + this.rs.sync.now = function() { return 1234567890123; }; + + await this.rs.sync.handleResponse("/foo/bar", "put", { + statusCode: 412, revision: "updated-elsewhere" + }); + + const nodes = await this.rs.local.getNodes(["/foo/bar"]); + this.node = nodes["/foo/bar"]; + }); + + it("does not update local and known common data", function() { + expect(this.node.common).to.deep.equal({ + body: "foo", contentType: "bloo", revision: "common" + }); + expect(this.node.local).to.deep.equal({ + body: "floo", contentType: "blaloo" + }); + }); + + it("sets the remote revision and timestamp", function() { + expect(this.node.remote).to.deep.equal({ + revision: "updated-elsewhere", timestamp: 1234567890123 + }); + }); + + it("removes the node's push data", function() { + expect(this.node.push).to.be.undefined; + }); + }); + + describe("401 response", function() { + it("emits Unauthorized error", function(done) { + this.rs.on("error", function(err) { + if (err instanceof UnauthorizedError) { done(); } + }); + + this.rs.sync.handleResponse( + undefined, undefined, { statusCode: 401 } + ); + }); + }); + + describe("Unknown response", function() { + it("emits an error", function(done) { + this.rs.on("error", function(err) { + if (err instanceof Error) { + expect(err.message).to.equal("HTTP response code 418 received."); + done(); + } + }); + + this.rs.sync.handleResponse( + undefined, undefined, { statusCode: 418 } + ); + }); + }); + }); + + describe("#autoMergeDocument", function() { + describe("when remote only has a revision", function() { + it("returns the node as is", function() { + const node = { + path: "foo", + common: { body: "foo", contentType: "bloo", revision: "common" }, + local: { body: "floo", contentType: "blaloo" }, + remote: { revision: "updated-elsewhere" } + }; + + expect(this.rs.sync.autoMergeDocument(node)).to.equal(node); + }); + }); + + 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("when node was updated", function() { + beforeEach(function() { + this.node = { + path: "foo", + common: { body: "old value", contentType: "old content-type", revision: "common" }, + remote: { body: "new value", contentType: "new content-type", revision: "remote" } + }; + }); + + it("emits a 'change' event", function(done) { + this.rs.local.emitChange = function(changeEvent) { + expect(changeEvent).to.deep.equal({ + origin: "remote", + path: "foo", + newValue: "new value", + oldValue: "old value", + newContentType: "new content-type", + oldContentType: "old content-type" + }); + done(); + }; + + this.rs.sync.autoMerge(this.node); + }); + + it("merges the node items", function() { + expect(this.rs.sync.autoMerge(this.node)).to.deep.equal({ + path: "foo", + common: { body: "new value", contentType: "new content-type", revision: "remote" } + }); + }); + }); + + describe("when node was deleted", function() { + describe("with node cached before", function() { + beforeEach(function() { + this.node = { + path: "foo", + common: { body: "foo", contentType: "bloo", revision: "common" }, + remote: { body: false, revision: "null" } + }; + }); + + it("emits a change event", function(done) { + this.rs.local.emitChange = function(changeEvent) { + expect(changeEvent).to.deep.equal({ + origin: "remote", + path: "foo", + oldValue: "foo", + newValue: undefined, + oldContentType: "bloo", + newContentType: undefined + }); + done(); + }; + + this.rs.sync.autoMerge(this.node); + }); + + it("returns undefined", function() { + expect(this.rs.sync.autoMerge(this.node)).to.be.undefined; + }); + }); + + describe("with node not cached before", function() { + beforeEach(function() { + this.node = { + path: "foo", + common: {}, + remote: { body: false, revision: "null" } + }; + }); + + it("does not emit a change event", function() { + const emitChange = sinon.spy(this.rs.local, "emitChange"); + + this.rs.sync.autoMerge(this.node); + + expect(emitChange.called).to.be.false; + }); + + it("returns undefined", function() { + expect(this.rs.sync.autoMerge(this.node)).to.be.undefined; + }); + }); + }); + }); + + describe("#markRemoteDeletions", function() { + describe("with empty paths", function() { + it("returns the changed nodes", async function() { + const changedNodes = { changed: 'nodes' }; + const res = await this.rs.sync.markRemoteDeletions([], changedNodes); + expect(res).to.equal(changedNodes); + }); + }); + + describe("with paths given", function() { + it("returns undefined", async function() { + const res = await this.rs.sync.markRemoteDeletions(['foo'], {}); + expect(res).to.be.undefined; + }); }); }); @@ -371,9 +964,28 @@ describe("Sync", function() { "/example/two": { "action": "get", "path": "/example/two", + "promise": new Promise(resolve => { + resolve({ + "statusCode": 200, + "body": "two", + "contentType": "text/plain; charset=UTF-8", + "revision": "123456abcdef" + }); + }) + }, + "/example/server-error": { + "action": "get", + "path": "/example/server-error", "promise": new Promise(resolve => { resolve({ "statusCode": 500 }); }) + }, + "/example/timeout": { + "action": "get", + "path": "/example/timeout", + "promise": new Promise(resolve => { + resolve({ "statusCode": "timeout" }); + }) } }; @@ -382,13 +994,27 @@ describe("Sync", function() { describe("successfully completed", function() { it("emits 'sync-req-done' with the number of remaining tasks", async function() { - const rsEmit = sinon.spy(this.rs, '_emit'); + const rsEmit = sinon.spy(this.rs, "_emit"); await this.rs.sync.finishTask(this.tasks["/example/one"], false); expect(rsEmit.callCount).to.equal(1); - expect(rsEmit.getCall(0).args[0]).to.equal('sync-req-done'); - expect(rsEmit.getCall(0).args[1]).to.have.property('tasksRemaining', 1); + expect(rsEmit.getCall(0).args[0]).to.equal("sync-req-done"); + expect(rsEmit.getCall(0).args[1]).to.have.property("tasksRemaining", 3); + }); + + it("removes the task from _running tasks", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + + expect(Object.keys(this.rs.sync._running)).to.deep.equal([ + "/example/two", "/example/server-error", "/example/timeout" + ]); + }); + + it("removes the task start time from _timeStarted", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + + expect(Object.keys(this.rs.sync._timeStarted)).to.not.include("/example/one"); }); describe("last task", function() { @@ -416,28 +1042,158 @@ describe("Sync", function() { describe("failed to complete", function() { it("emits 'sync-req-done' with the number of remaining tasks", async function() { const rsEmit = sinon.spy(this.rs, '_emit'); - - await this.rs.sync.finishTask(this.tasks["/example/two"], false); + await this.rs.sync.finishTask(this.tasks["/example/server-error"], false); expect(rsEmit.callCount).to.equal(3); // 'error', 'sync-req-done', 'sync-done' expect(rsEmit.getCall(1).args[0]).to.equal('sync-req-done'); - expect(rsEmit.getCall(1).args[1]).to.have.property('tasksRemaining', 2); + expect(rsEmit.getCall(1).args[1]).to.have.property('tasksRemaining', 4); }); it("marks the sync as done", async function() { - await this.rs.sync.finishTask(this.tasks["/example/two"], false); + await this.rs.sync.finishTask(this.tasks["/example/server-error"], false); expect(this.rs.sync.done).to.be.true; }); it("emits 'sync-done' with negative 'completed' status", async function() { const rsEmit = sinon.spy(this.rs, '_emit'); - - await this.rs.sync.finishTask(this.tasks["/example/two"], false); + await this.rs.sync.finishTask(this.tasks["/example/server-error"], false); expect(rsEmit.getCall(2).args[0]).to.equal('sync-done'); expect(rsEmit.getCall(2).args[1]).to.have.property('completed', false); }); + + it("stops the current task cycle on server error", async function() { + await this.rs.sync.finishTask(this.tasks["/example/two"], false); + await this.rs.sync.finishTask(this.tasks["/example/server-error"], false); + expect(Object.keys(this.rs.sync._tasks)).to.deep.equal([ + "/example/one", + "/example/server-error", + "/example/timeout" + ]); + }); + + it("stops the current task cycle on timeout", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + await this.rs.sync.finishTask(this.tasks["/example/timeout"], false); + expect(Object.keys(this.rs.sync._tasks)).to.deep.equal([ + "/example/two", + "/example/server-error", + "/example/timeout" + ]); + }); + }); + }); + + describe("#queueGetRequest", function() { + describe("normal operation", function() { + beforeEach(async function() { + this.rs.remote.connected = true; + this.rs.remote.online = true; + this.rs.caching.enable("/foo/"); + this.rs.local.get = async function(path) { + if (path === "/foo/one") { return "dummy response"; } + }; + this.rs.sync.doTasks = function() { + // Execute callback for our queued task + this._tasks["/foo/one"][0](); + }.bind(this.rs.sync); + + }); + + it("adds a task for the path and resolves with local data when task is finished", async function() { + const res = await this.rs.sync.queueGetRequest("/foo/one"); + expect(res).to.equal("dummy response"); + }); + }); + + describe("when not connected", function() { + beforeEach(function() { + this.rs.remote.connected = false; + }); + + it("get with maxAge requirement is rejected", async function() { + await expect(this.rs.sync.queueGetRequest("/example/one")).to + .eventually.be.rejectedWith(/remote is not connected/); + }); + }); + + describe("when not connected", function() { + beforeEach(function() { + this.rs.remote.connected = true; + this.rs.remote.online = false; + }); + + it("get with maxAge requirement is rejected", async function() { + await expect(this.rs.sync.queueGetRequest("/example/one")).to + .eventually.be.rejectedWith(/remote is not online/); + }); }); }); + describe("Edge cases", function() { + describe("Syncing multiple new documents in the same folder while there are local changes", function() { + beforeEach(function(done) { + this.rs.remote.connected = true; + this.rs.remote.online = true; + this.rs.sync.doTasks = this.original.doTasks; + this.rs.sync.doTask = async function(path) { + return { + action: 'get', + path, + promise: Promise.resolve({ + statusCode: 200, + body: path, + contentType: 'text/plain', + revision: path + }) + }; + }; + this.rs.sync._tasks = { + '/foo/2': [], '/foo/3': [], '/foo/4': [], '/foo/5': [], + '/foo/6': [], '/foo/7': [], '/foo/8': [], '/foo/9': [], + '/foo/10': [], '/foo/11': [], + }; + this.rs.local.setNodes({ + '/foo/': { + path: '/foo/', + common: { + itemsMap: { '1': true, }, + revision: 'remotefolderrevision', + timestamp: 1397210425598, + }, + local: { + itemsMap: { '1': true }, + revision: 'localfolderrevision', + timestamp: 1397210425612 + } + }, + '/foo/1': { + path: '/foo/1', + local: { body: '/foo/1', contentType: 'test/plain', timestamp: 1234567891000 } + } + }); + + this.rs.on('sync-done', () => done()); + this.rs.sync.doTasks(); + }); + + it("merges the folder without missing any documents", async function() { + const nodes = await this.rs.local.getNodes(['/foo/']); + const node = nodes['/foo/']; + expect(node.local.itemsMap).to.deep.equal({ + '1': true, + '2': true, + '3': true, + '4': true, + '5': true, + '6': true, + '7': true, + '8': true, + '9': true, + '10': true, + '11': true, + }); + }); + }); + }); }); From a60d5bd0b41570ddad2996472348057944fc14dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sat, 7 Jun 2025 11:54:56 +0400 Subject: [PATCH 20/47] Refactor finishTask --- src/sync.ts | 132 +++++++++++++++++++++------------------- test/unit/sync.test.mjs | 44 ++++++++++++-- 2 files changed, 111 insertions(+), 65 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 863e6a169..10b3212b5 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -134,7 +134,7 @@ export class Sync { _timeStarted: { [key: string]: number; } = {}; /** - * TODO document + * Holds finished tasks for orderly processing **/ _finishedTasks: SyncTask[] = []; @@ -931,7 +931,7 @@ export class Sync { } } - handleResponse (path: string, action, r): Promise { + async handleResponse (path: string, action, r): Promise { const status = this.interpretStatus(r.statusCode); if (status.successful) { @@ -963,9 +963,9 @@ export class Sync { } /** - * TODO document + * Execute/finish running tasks, one at a time **/ - finishTask (task: SyncTask, queueTask: boolean = true): void | Promise { + async finishTask (task: SyncTask, queueTask: boolean = true): Promise { if (task.action === undefined) { delete this._running[task.path]; return; @@ -982,72 +982,82 @@ export class Sync { log("[Sync] run task:", task.path); - return task.promise - .then(res => { - return this.handleResponse(task.path, task.action, res); - }, err => { - log('[Sync] wireclient rejects its promise!', task.path, task.action, err); - return this.handleResponse(task.path, task.action, { statusCode: 'offline' }); - }) - .then(async (completed) => { - this._finishedTasks.shift(); - delete this._timeStarted[task.path]; - delete this._running[task.path]; - - if (completed) { - if (this._tasks[task.path]) { - for (let i=0; i < this._tasks[task.path].length; i++) { - this._tasks[task.path][i](); - } - delete this._tasks[task.path]; - } - } - - this.rs._emit('sync-req-done', { - tasksRemaining: Object.keys(this._tasks).length - }); + let res; + try { + res = await task.promise; + } catch (err) { + log('[Sync] wire client rejects its promise', task.path, task.action, err); + res = { statusCode: 'offline' }; + } - if (this._finishedTasks.length > 0) { - this.finishTask(this._finishedTasks[0], false); - return; - } + try { + const completed = await this.handleResponse(task.path, task.action, res); + this.finishSuccessfulTask(task, completed); + } catch (err) { + this.finishUnsuccessfulTask(task, err); + } + } - await this.collectTasks(false).then(() => { - // See if there are any more tasks that are not refresh tasks - if (!this.hasTasks() || this.stopped) { - log('[Sync] Sync is done! Reschedule?', Object.keys(this._tasks).length, this.stopped); - if (!this.done) { - this.done = true; - this.rs._emit('sync-done', { completed: true }); - } - } else { - // Use a 10ms timeout to let the JavaScript runtime catch its breath - // (and hopefully force an IndexedDB auto-commit?), and also to cause - // the threads to get staggered and get a good spread over time: - setTimeout(() => { this.doTasks(); }, 10); - } - }); - }, err => { - log('[Sync] Error', err); + async finishSuccessfulTask (task: SyncTask, completed: boolean): Promise { + this._finishedTasks.shift(); + delete this._timeStarted[task.path]; + delete this._running[task.path]; - this._finishedTasks.shift(); - delete this._timeStarted[task.path]; - delete this._running[task.path]; + if (completed) { + if (this._tasks[task.path]) { + for (let i=0; i < this._tasks[task.path].length; i++) { + this._tasks[task.path][i](); + } + delete this._tasks[task.path]; + } + } - this.rs._emit('sync-req-done', { - tasksRemaining: Object.keys(this._tasks).length - }); + this.rs._emit('sync-req-done', { + tasksRemaining: Object.keys(this._tasks).length + }); - if (this._finishedTasks.length > 0) { - this.finishTask(this._finishedTasks[0], false); - return; - } + if (this._finishedTasks.length > 0) { + await this.finishTask(this._finishedTasks[0], false); + return; + } + await this.collectTasks(false).then(() => { + // See if there are any more tasks that are not refresh tasks + if (!this.hasTasks() || this.stopped) { + log('[Sync] Sync is done! Reschedule?', Object.keys(this._tasks).length, this.stopped); if (!this.done) { this.done = true; - this.rs._emit('sync-done', { completed: false }); + this.rs._emit('sync-done', { completed: true }); } - }); + } else { + // Use a 10ms timeout to let the JavaScript runtime catch its breath + // (and hopefully force an IndexedDB auto-commit?), and also to cause + // the threads to get staggered and get a good spread over time: + setTimeout(() => { this.doTasks(); }, 10); + } + }); + } + + async finishUnsuccessfulTask (task: SyncTask, err: Error): Promise { + log('[Sync] Error', err); + + this._finishedTasks.shift(); + delete this._timeStarted[task.path]; + delete this._running[task.path]; + + this.rs._emit('sync-req-done', { + tasksRemaining: Object.keys(this._tasks).length + }); + + if (this._finishedTasks.length > 0) { + await this.finishTask(this._finishedTasks[0], false); + return; + } + + if (!this.done) { + this.done = true; + this.rs._emit('sync-done', { completed: false }); + } } /** diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index faeaed1d0..45e4a66e8 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -989,7 +989,11 @@ describe("Sync", function() { } }; - this.rs.sync._tasks = this.tasks; + Object.keys(this.tasks).map(path => { + this.rs.sync._tasks[path] = []; + this.rs.sync._timeStarted[path] = this.rs.sync.now(); + this.rs.sync._running[path] = Promise.resolve(this.tasks[path]); + }); }); describe("successfully completed", function() { @@ -1017,8 +1021,23 @@ describe("Sync", function() { expect(Object.keys(this.rs.sync._timeStarted)).to.not.include("/example/one"); }); + it("removes the task from _running tasks", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + + expect(Object.keys(this.rs.sync._running)).to.deep.equal([ + '/example/two', '/example/server-error', '/example/timeout' + ]); + }); + + it("removes the task start time from _timeStarted", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + + expect(Object.keys(this.rs.sync._timeStarted)).to.not.include('/example/one'); + }); + describe("last task", function() { beforeEach(function() { + this.rs.sync.collectTasks = async () => {}; this.rs.sync._tasks = { "/example/one": this.tasks["/example/one"] }; }); @@ -1040,13 +1059,31 @@ describe("Sync", function() { }); describe("failed to complete", function() { + beforeEach(async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + }); + + it("removes the task from _running tasks", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + + expect(Object.keys(this.rs.sync._running)).to.deep.equal([ + '/example/two', '/example/server-error', '/example/timeout' + ]); + }); + + it("removes the task start time from _timeStarted", async function() { + await this.rs.sync.finishTask(this.tasks["/example/one"], false); + + expect(Object.keys(this.rs.sync._timeStarted)).to.not.include('/example/one'); + }); + it("emits 'sync-req-done' with the number of remaining tasks", async function() { const rsEmit = sinon.spy(this.rs, '_emit'); await this.rs.sync.finishTask(this.tasks["/example/server-error"], false); expect(rsEmit.callCount).to.equal(3); // 'error', 'sync-req-done', 'sync-done' expect(rsEmit.getCall(1).args[0]).to.equal('sync-req-done'); - expect(rsEmit.getCall(1).args[1]).to.have.property('tasksRemaining', 4); + expect(rsEmit.getCall(1).args[1]).to.have.property('tasksRemaining', 3); }); it("marks the sync as done", async function() { @@ -1063,10 +1100,9 @@ describe("Sync", function() { }); it("stops the current task cycle on server error", async function() { - await this.rs.sync.finishTask(this.tasks["/example/two"], false); await this.rs.sync.finishTask(this.tasks["/example/server-error"], false); expect(Object.keys(this.rs.sync._tasks)).to.deep.equal([ - "/example/one", + "/example/two", "/example/server-error", "/example/timeout" ]); From 26cb53cc4f2d6bf4ee14c3a47124c63f5c44888b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sat, 7 Jun 2025 16:40:39 +0400 Subject: [PATCH 21/47] Remove "legacy nodes" entirely --- src/cachinglayer.ts | 32 ++------------------------------ src/indexeddb.ts | 2 +- src/inmemorystorage.ts | 2 +- src/interfaces/rs_node.ts | 9 +-------- src/localstorage.ts | 5 ++--- test/unit/cachinglayer-suite.js | 7 ------- 6 files changed, 7 insertions(+), 50 deletions(-) diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts index d158635c2..8fa8dde3d 100644 --- a/src/cachinglayer.ts +++ b/src/cachinglayer.ts @@ -37,16 +37,6 @@ function getLatest (node: RSNode): RSItem { if (node.common && node.common.body && node.common.contentType) { return node.common; } - // Migration code! Once all apps use at least this version of the lib, we - // can publish clean-up code that migrates over any old-format data, and - // stop supporting it. For now, new apps will support data in both - // formats, thanks to this: - if (node.body && node.contentType) { - return { - body: node.body, - contentType: node.contentType - }; - } } } @@ -55,7 +45,9 @@ function isOutdated (nodes: RSNodes, maxAge: number): boolean { if (nodes[path] && nodes[path].remote) { return true; } + const nodeVersion = getLatest(nodes[path]); + if (nodeVersion && nodeVersion.timestamp && (new Date().getTime()) - nodeVersion.timestamp <= maxAge) { return false; } else if (!nodeVersion) { @@ -317,26 +309,6 @@ abstract class CachingLayer { this.diffHandler = diffHandler; } - migrate(node: RSNode): RSNode { - if (typeof (node) === 'object' && !node.common) { - node.common = {}; - if (typeof (node.path) === 'string') { - if (node.path.substr(-1) === '/' && typeof (node.body) === 'object') { - node.common.itemsMap = node.body; - } - } else { - //save legacy content of document node as local version - if (!node.local) { - node.local = {}; - } - node.local.body = node.body; - node.local.contentType = node.contentType; - } - } - return node; - } - - private _updateNodes(paths: string[], _processNodes: ProcessNodes): Promise { return new Promise((resolve, reject) => { this._doUpdateNodes(paths, _processNodes, { diff --git a/src/indexeddb.ts b/src/indexeddb.ts index 6bb5e9957..893525ff5 100644 --- a/src/indexeddb.ts +++ b/src/indexeddb.ts @@ -288,7 +288,7 @@ class IndexedDB extends CachingLayer { const cursor = evt.target.result; if (cursor) { - cb(this.migrate(cursor.value)); + cb(cursor.value); cursor.continue(); } else { resolve(); diff --git a/src/inmemorystorage.ts b/src/inmemorystorage.ts index 243e1205b..b9a0aa4a7 100644 --- a/src/inmemorystorage.ts +++ b/src/inmemorystorage.ts @@ -46,7 +46,7 @@ class InMemoryStorage extends CachingLayer { forAllNodes(cb: (node: RSNode) => void): Promise { for (const path in this._storage) { - cb(this.migrate(this._storage[path])); + cb(this._storage[path]); } return Promise.resolve(); } diff --git a/src/interfaces/rs_node.ts b/src/interfaces/rs_node.ts index 07cffe90c..c719e7721 100644 --- a/src/interfaces/rs_node.ts +++ b/src/interfaces/rs_node.ts @@ -4,20 +4,13 @@ export type RSItem = { contentLength?: number; revision?: string; timestamp?: number; - itemsMap?: { - [key: string]: any; - }; + itemsMap?: { [key: string]: any; }; previousBody?: string | object | false; previousContentType?: string; } export type RSNode = { path: string; - body?: string | object | false; - contentType?: string; - itemsMap?: { - [key: string]: any; - }; common?: RSItem; local?: RSItem; remote?: RSItem; diff --git a/src/localstorage.ts b/src/localstorage.ts index 70f2aacb6..62e50affe 100644 --- a/src/localstorage.ts +++ b/src/localstorage.ts @@ -26,7 +26,7 @@ class LocalStorage extends CachingLayer { this.addEvents(['change', 'local-events-done']); } - // TODO fix this + // TODO use correct types diffHandler(...args: any[]): void { return; } @@ -61,8 +61,7 @@ class LocalStorage extends CachingLayer { for (let i = 0, len = localStorage.length; i < len; i++) { if (isNodeKey(localStorage.key(i))) { try { - // NOTE: this is coming from caching layer todo fix via interface or similar - node = this.migrate(JSON.parse(localStorage.getItem(localStorage.key(i)))); + node = JSON.parse(localStorage.getItem(localStorage.key(i))); } catch (e) { node = undefined; } diff --git a/test/unit/cachinglayer-suite.js b/test/unit/cachinglayer-suite.js index 680828930..f6870c3b9 100644 --- a/test/unit/cachinglayer-suite.js +++ b/test/unit/cachinglayer-suite.js @@ -49,11 +49,6 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'], push: { foo: 'bar' }, remote: { foo: 'bar' } }, - legacyNode = { - path: '/foo', - body: 'asdf', - contentType: 'text/plain' - }; deletedLocalNode = { path: '/a/b', local: { body: false }, @@ -68,8 +63,6 @@ define(['require', './build/util', './build/config', './build/inmemorystorage'], test.assertAnd(getLatest(localNode).contentType, 'c'); test.assertAnd(getLatest(commonNode).body, 'b'); test.assertAnd(getLatest(commonNode).contentType, 'c'); - test.assertAnd(getLatest(legacyNode).body, 'asdf'); - test.assertAnd(getLatest(legacyNode).contentType, 'text/plain'); test.assertAnd(getLatest(deletedLocalNode), undefined); test.done(); } From 004f607d5aec1731ad1452ef30511bce688900cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sat, 7 Jun 2025 18:35:59 +0400 Subject: [PATCH 22/47] More types and docs for Sync --- src/cachinglayer.ts | 1 - src/sync.ts | 90 +++++++++++++++++++++++---------------------- 2 files changed, 47 insertions(+), 44 deletions(-) diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts index 8fa8dde3d..c05e6b80b 100644 --- a/src/cachinglayer.ts +++ b/src/cachinglayer.ts @@ -251,7 +251,6 @@ abstract class CachingLayer { } flush(path: string): unknown { - return this._getAllDescendentPaths(path).then((paths: string[]) => { return this.getNodes(paths); }).then((nodes: RSNodes) => { diff --git a/src/sync.ts b/src/sync.ts index 10b3212b5..dd0d97f2f 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1,6 +1,7 @@ import type RemoteStorage from './remotestorage'; -import type { RSNode, RSNodes } from './interfaces/rs_node'; +import type { RSItem, RSNode, RSNodes } from './interfaces/rs_node'; import type { QueuedRequestResponse } from './interfaces/queued_request_response'; +import type { RemoteResponse } from './remote'; import config from './config'; import Env from './env'; import EventHandling from './eventhandling'; @@ -30,12 +31,12 @@ interface ResponseStatus { } interface SyncTask { - action: string; + action: "get" | "put" | "delete"; path: string; - promise: Promise; + promise: Promise; } -function taskFor (action, path: string, promise: Promise): SyncTask { +function taskFor (action: SyncTask["action"], path: SyncTask["path"], promise: SyncTask["promise"]): SyncTask { return { action, path, promise }; } @@ -99,43 +100,43 @@ function handleVisibility (env, rs): void { * It does this using requests to documents and folders. Whenever a folder GET * comes in, it gives information about all the documents it contains (this is * the `markChildren` function). - **/ + */ export class Sync { rs: RemoteStorage; /** * Maximum number of parallel requests to execute - **/ + */ numThreads: number = 10; /** * Sync done? `false` when periodic sync is currently running - **/ + */ done: boolean; /** * Sync stopped entirely - **/ + */ stopped: boolean; /** * Paths queued for sync, sometimes with callbacks - **/ + */ _tasks: { [key: string]: Array<() => void>; } = {}; /** * Promises of currently running sync tasks per path - **/ + */ _running: { [key: string]: Promise; } = {}; /** * Start times of current sync per path - **/ + */ _timeStarted: { [key: string]: number; } = {}; /** * Holds finished tasks for orderly processing - **/ + */ _finishedTasks: SyncTask[] = []; constructor (remoteStorage: RemoteStorage) { @@ -156,7 +157,7 @@ export class Sync { /** * Return current time - **/ + */ now (): number { return new Date().getTime(); } @@ -165,7 +166,7 @@ export class Sync { * When getting a path from the caching layer, this function might be handed * in to first check if it was updated on the remote, in order to fulfill a * maxAge requirement - **/ + */ async queueGetRequest (path: string): Promise { return new Promise((resolve, reject) => { if (!this.rs.remote.connected) { @@ -251,7 +252,7 @@ export class Sync { /** * Collect sync tasks for changed nodes - **/ + */ async collectDiffTasks (): Promise { let num = 0; @@ -357,7 +358,7 @@ export class Sync { /** * Collect tasks to refresh highest outdated folder in tree - **/ + */ async collectRefreshTasks (): Promise { await this.rs.local.forAllNodes((node: RSNode) => { let parentPath: string; @@ -380,7 +381,7 @@ export class Sync { /** * Flush nodes from cache after sync to remote - **/ + */ flush (nodes: RSNodes): RSNodes { for (const path in nodes) { // Strategy is 'FLUSH' and no local changes exist @@ -395,7 +396,7 @@ export class Sync { /** * Sync one path - **/ + */ async doTask (path: string): Promise { return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; @@ -459,7 +460,7 @@ export class Sync { /** * TODO document - **/ + */ autoMergeFolder (node: RSNode): RSNode { if (node.remote.itemsMap) { node.common = node.remote; @@ -486,7 +487,7 @@ export class Sync { /** * TODO document - **/ + */ autoMergeDocument (node: RSNode): RSNode { if (hasNoRemoteChanges(node)) { node = mergeMutualDeletion(node); @@ -520,7 +521,7 @@ export class Sync { /** * TODO document - **/ + */ autoMerge (node: RSNode): RSNode { if (node.remote) { if (node.local) { @@ -589,7 +590,7 @@ export class Sync { /** * TODO document - **/ + */ async markChildren (path, itemsMap, changedNodes: RSNodes, missingChildren): Promise { const paths = []; const meta = {}; @@ -686,7 +687,7 @@ export class Sync { /** * TODO document - **/ + */ async deleteRemoteTrees (paths: string[], changedNodes: RSNodes): Promise { if (paths.length === 0) { return changedNodes; } @@ -729,8 +730,8 @@ export class Sync { /** * TODO document - **/ - async completeFetch (path: string, bodyOrItemsMap: object, contentType: string, revision: string): Promise { + */ + async completeFetch (path: string, bodyOrItemsMap: RSItem["body"], contentType: string, revision: string): Promise { let paths: string[]; let parentPath: string; const pathsFromRootArr = pathsFromRoot(path); @@ -775,7 +776,7 @@ export class Sync { collectMissingChildren(node.remote); node.remote.itemsMap = {}; - for (itemName in bodyOrItemsMap) { + for (itemName in bodyOrItemsMap as object) { node.remote.itemsMap[itemName] = true; } } else { @@ -799,11 +800,11 @@ export class Sync { } /** - * TODO document - **/ - async completePush (path: string, action, conflict, revision: string): Promise { + * Handle successful PUT or DELETE request + */ return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { const node = nodes[path]; + async completePush (path: string, action: "put" | "delete", conflict: boolean, revision: string): Promise { if (!node.push) { this.stopped = true; @@ -853,7 +854,7 @@ export class Sync { /** * TODO document - **/ + */ async dealWithFailure (path: string): Promise { const nodes = await this.rs.local.getNodes([path]); @@ -898,9 +899,9 @@ export class Sync { } /** - * TODO document - **/ - async handleGetResponse (path: string, status: ResponseStatus, bodyOrItemsMap, contentType: string, revision: string): Promise { + * Handle successful GET request + */ + async handleGetResponse (path: string, status: ResponseStatus, bodyOrItemsMap: RSItem["body"], contentType: string, revision: string): Promise { if (status.notFound) { if (isFolder(path)) { bodyOrItemsMap = {}; @@ -931,7 +932,10 @@ export class Sync { } } - async handleResponse (path: string, action, r): Promise { + /** + * Handle response of executed request + */ + async handleResponse (path: string, action: SyncTask["action"], r: RemoteResponse): Promise { const status = this.interpretStatus(r.statusCode); if (status.successful) { @@ -964,7 +968,7 @@ export class Sync { /** * Execute/finish running tasks, one at a time - **/ + */ async finishTask (task: SyncTask, queueTask: boolean = true): Promise { if (task.action === undefined) { delete this._running[task.path]; @@ -1062,7 +1066,7 @@ export class Sync { /** * Determine how many tasks we want to have - **/ + */ tasksWanted (): number { if (!this.rs.remote.connected) { // Nothing to sync if no remote connected @@ -1082,10 +1086,10 @@ export class Sync { * Check if more tasks can be queued, and start running * tasks * - * @returns {Boolean} `true` when all tasks have been - * run or there's nothing to do, `false` - * if we could or want to run more - **/ + * @returns {Boolean} `true` when all tasks have been started or + * there's nothing to do, `false` if we could + * or want to run more + */ doTasks (): boolean { const numToHave = this.tasksWanted(); const numToAdd = numToHave - Object.keys(this._running).length; @@ -1113,7 +1117,7 @@ export class Sync { /** * Collect any potential sync tasks if none are queued - **/ + */ async collectTasks (alsoCheckRefresh: boolean = true): Promise { if (this.hasTasks() || this.stopped) { return; } @@ -1127,7 +1131,7 @@ export class Sync { /** * Add a sync task for the given path - **/ + */ // eslint-disable-next-line @typescript-eslint/no-explicit-any addTask (path: string, cb?: () => void): void { if (!this._tasks[path]) { @@ -1140,7 +1144,7 @@ export class Sync { /** * Start a sync procedure - **/ + */ public async sync (): Promise { this.done = false; From 39e06d641484ff0e7b120066279f77407480fea5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 8 Jun 2025 11:04:47 +0400 Subject: [PATCH 23/47] Use async/await --- src/sync.ts | 69 ++++++++++++++++++++++++++--------------------------- 1 file changed, 34 insertions(+), 35 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index dd0d97f2f..64b238896 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -802,54 +802,53 @@ export class Sync { /** * Handle successful PUT or DELETE request */ - return this.rs.local.getNodes([path]).then((nodes: RSNodes) => { - const node = nodes[path]; async completePush (path: string, action: "put" | "delete", conflict: boolean, revision: string): Promise { + const nodes = await this.rs.local.getNodes([path]); + const node = nodes[path]; - if (!node.push) { - this.stopped = true; - throw new Error('completePush called but no push version!'); - } + if (!node.push) { + this.stopped = true; + throw new Error('completePush called but no push version!'); + } - if (conflict) { - log('[Sync] We have a conflict'); + if (conflict) { + log('[Sync] We have a conflict'); - if (!node.remote || node.remote.revision !== revision) { - node.remote = { - revision: revision || 'conflict', - timestamp: this.now() - }; - delete node.push; - } - - nodes[path] = this.autoMerge(node); - } else { - node.common = { - revision: revision, + if (!node.remote || node.remote.revision !== revision) { + node.remote = { + revision: revision || 'conflict', timestamp: this.now() }; + delete node.push; + } + + nodes[path] = this.autoMerge(node); + } else { + node.common = { + revision: revision, + timestamp: this.now() + }; - if (action === 'put') { - node.common.body = node.push.body; - node.common.contentType = node.push.contentType; + if (action === 'put') { + node.common.body = node.push.body; + node.common.contentType = node.push.contentType; - if (equal(node.local.body, node.push.body) && - node.local.contentType === node.push.contentType) { - delete node.local; - } + if (equal(node.local.body, node.push.body) && + node.local.contentType === node.push.contentType) { + delete node.local; + } + delete node.push; + } else if (action === 'delete') { + if (node.local.body === false) { // No new local changes since push; flush it. + nodes[path] = undefined; + } else { delete node.push; - } else if (action === 'delete') { - if (node.local.body === false) { // No new local changes since push; flush it. - nodes[path] = undefined; - } else { - delete node.push; - } } } + } - return this.rs.local.setNodes(this.flush(nodes)); - }); + await this.rs.local.setNodes(this.flush(nodes)); } /** From ad636f950b34342f98b17e945b7853e72741315b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 8 Jun 2025 12:30:32 +0400 Subject: [PATCH 24/47] Refactor handleGetResponse --- src/sync.ts | 36 ++++++++++++++---------------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 64b238896..681430058 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -902,33 +902,25 @@ export class Sync { */ async handleGetResponse (path: string, status: ResponseStatus, bodyOrItemsMap: RSItem["body"], contentType: string, revision: string): Promise { if (status.notFound) { - if (isFolder(path)) { - bodyOrItemsMap = {}; - } else { - bodyOrItemsMap = false; - } + bodyOrItemsMap = isFolder(path) ? {} : false; } if (status.changed) { - return this.completeFetch(path, bodyOrItemsMap, contentType, revision) - .then(dataFromFetch => { - if (isFolder(path)) { - if (this.corruptServerItemsMap(bodyOrItemsMap)) { - log('[Sync] WARNING: Discarding corrupt folder description from server for ' + path); - return false; - } else { - return this.markChildren(path, bodyOrItemsMap, dataFromFetch.toBeSaved, dataFromFetch.missingChildren) - .then(() => { return true; }); - } - } else { - return this.rs.local.setNodes(this.flush(dataFromFetch.toBeSaved)) - .then(() => { return true; }); - } - }); + const data = await this.completeFetch(path, bodyOrItemsMap, contentType, revision); + + if (isFolder(path)) { + if (this.corruptServerItemsMap(bodyOrItemsMap)) { + log('[Sync] WARNING: Discarding corrupt folder description from server for ' + path); + return false; + } + await this.markChildren(path, bodyOrItemsMap, data.toBeSaved, data.missingChildren); + } else { + await this.rs.local.setNodes(this.flush(data.toBeSaved)); + } } else { - return this.updateCommonTimestamp(path, revision) - .then(() => { return true; }); + await this.updateCommonTimestamp(path, revision); } + return true; } /** From 119f384e00f5ea8b8f3a14e318de4562daa72c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 8 Jun 2025 15:12:12 +0400 Subject: [PATCH 25/47] More types and docs for Sync --- src/sync.ts | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 681430058..ea1aa26df 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -459,7 +459,7 @@ export class Sync { } /** - * TODO document + * Merge/process folder node items after updates from remote */ autoMergeFolder (node: RSNode): RSNode { if (node.remote.itemsMap) { @@ -486,7 +486,7 @@ export class Sync { } /** - * TODO document + * Merge/process document node items after updates from remote */ autoMergeDocument (node: RSNode): RSNode { if (hasNoRemoteChanges(node)) { @@ -520,7 +520,7 @@ export class Sync { } /** - * TODO document + * Merge/process node items after various updates from remote */ autoMerge (node: RSNode): RSNode { if (node.remote) { @@ -589,9 +589,10 @@ export class Sync { } /** - * TODO document + * After successful GET of a folder, mark its children/items for + * changes and further processing */ - async markChildren (path, itemsMap, changedNodes: RSNodes, missingChildren): Promise { + async markChildren (path: string, itemsMap: RSItem["itemsMap"], changedNodes: RSNodes, missingChildren: { [key: string]: boolean }): Promise { const paths = []; const meta = {}; const recurse = {}; @@ -686,7 +687,8 @@ export class Sync { } /** - * TODO document + * Recursively process paths to mark documents as remotely deleted + * where applicable */ async deleteRemoteTrees (paths: string[], changedNodes: RSNodes): Promise { if (paths.length === 0) { return changedNodes; } @@ -694,7 +696,7 @@ export class Sync { const nodes = await this.rs.local.getNodes(paths); const subPaths = {}; - function collectSubPaths (folder, path: string): void { + function collectSubPaths (folder: RSItem, path: string): void { if (folder && folder.itemsMap) { for (const itemName in folder.itemsMap) { subPaths[path+itemName] = true; @@ -729,7 +731,7 @@ export class Sync { } /** - * TODO document + * Complete a successful GET request */ async completeFetch (path: string, bodyOrItemsMap: RSItem["body"], contentType: string, revision: string): Promise { let paths: string[]; @@ -776,7 +778,7 @@ export class Sync { collectMissingChildren(node.remote); node.remote.itemsMap = {}; - for (itemName in bodyOrItemsMap as object) { + for (itemName in bodyOrItemsMap as RSItem["itemsMap"]) { node.remote.itemsMap[itemName] = true; } } else { @@ -852,7 +854,7 @@ export class Sync { } /** - * TODO document + * Remove push item from cached nodes that failed to sync */ async dealWithFailure (path: string): Promise { const nodes = await this.rs.local.getNodes([path]); @@ -913,7 +915,7 @@ export class Sync { log('[Sync] WARNING: Discarding corrupt folder description from server for ' + path); return false; } - await this.markChildren(path, bodyOrItemsMap, data.toBeSaved, data.missingChildren); + await this.markChildren(path, bodyOrItemsMap as RSItem["itemsMap"], data.toBeSaved, data.missingChildren); } else { await this.rs.local.setNodes(this.flush(data.toBeSaved)); } From 0bf6ef62b9634002e2321a80e4ffa3194d3519cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 8 Jun 2025 15:12:36 +0400 Subject: [PATCH 26/47] Refactor autoMerge --- src/sync.ts | 77 +++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index ea1aa26df..d678f4664 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -523,43 +523,7 @@ export class Sync { * Merge/process node items after various updates from remote */ autoMerge (node: RSNode): RSNode { - if (node.remote) { - if (node.local) { - if (isFolder(node.path)) { - return this.autoMergeFolder(node); - } else { - return this.autoMergeDocument(node); - } - } else { // no local changes - if (isFolder(node.path)) { - if (node.remote.itemsMap !== undefined) { - node.common = node.remote; - delete node.remote; - } - } else { - if (node.remote.body !== undefined) { - const change = { - origin: 'remote', - path: node.path, - oldValue: (node.common.body === false ? undefined : node.common.body), - newValue: (node.remote.body === false ? undefined : node.remote.body), - oldContentType: node.common.contentType, - newContentType: node.remote.contentType - }; - if (change.oldValue || change.newValue) { - this.rs.local.emitChange(change); - } - - if (!node.remote.body) { // no remote, so delete/don't create - return; - } - - node.common = node.remote; - delete node.remote; - } - } - } - } else { + if (!node.remote) { if (node.common.body) { this.rs.local.emitChange({ origin: 'remote', @@ -570,8 +534,45 @@ export class Sync { newContentType: undefined }); } + return; + } - return undefined; + // Local changes + if (node.local) { + if (isFolder(node.path)) { + return this.autoMergeFolder(node); + } else { + return this.autoMergeDocument(node); + } + } + + if (isFolder(node.path)) { + if (node.remote.itemsMap !== undefined) { + node.common = node.remote; + delete node.remote; + } + } else { + if (node.remote.body !== undefined) { + const change = { + origin: 'remote', + path: node.path, + oldValue: (node.common.body === false ? undefined : node.common.body), + newValue: (node.remote.body === false ? undefined : node.remote.body), + oldContentType: node.common.contentType, + newContentType: node.remote.contentType + }; + + if (change.oldValue || change.newValue) { + this.rs.local.emitChange(change); + } + + if (!node.remote.body) { // no remote, so delete/don't create + return; + } + + node.common = node.remote; + delete node.remote; + } } return node; From 57b60543e6ab606b6a47bd29b34cb023e84f4bac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Sun, 8 Jun 2025 16:50:30 +0400 Subject: [PATCH 27/47] Improve function name --- src/sync.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index d678f4664..0fd0bf943 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -681,7 +681,7 @@ export class Sync { } } - const changedNodes2 = await this.deleteRemoteTrees(Object.keys(recurse), changedNodes); + const changedNodes2 = await this.markRemoteDeletions(Object.keys(recurse), changedNodes); if (changedNodes2) { await this.rs.local.setNodes(this.flush(changedNodes2)); } @@ -691,7 +691,7 @@ export class Sync { * Recursively process paths to mark documents as remotely deleted * where applicable */ - async deleteRemoteTrees (paths: string[], changedNodes: RSNodes): Promise { + async markRemoteDeletions (paths: string[], changedNodes: RSNodes): Promise { if (paths.length === 0) { return changedNodes; } const nodes = await this.rs.local.getNodes(paths); @@ -725,7 +725,7 @@ export class Sync { } // Recurse whole tree depth levels at once: - const changedNodes2 = await this.deleteRemoteTrees(Object.keys(subPaths), changedNodes); + const changedNodes2 = await this.markRemoteDeletions(Object.keys(subPaths), changedNodes); if (changedNodes2) { await this.rs.local.setNodes(this.flush(changedNodes2)); } From 9f38689e4916b3cfe5051ef8f6b41450a47cd95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 08:30:10 +0400 Subject: [PATCH 28/47] Add comment --- src/sync.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync.ts b/src/sync.ts index 0fd0bf943..dbf195dca 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -425,7 +425,7 @@ export class Sync { return taskFor('put', path, this.rs.remote.put( path, - node.push.body as string, + node.push.body as string, // TODO string | ArrayBuffer? node.push.contentType, options ) From 8905c264e44e1636a59419bb6b3e864eaf9cefd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 13:29:53 +0400 Subject: [PATCH 29/47] Document function --- src/remotestorage.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/remotestorage.ts b/src/remotestorage.ts index 7707e097c..d214a8eeb 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -433,10 +433,9 @@ export class RemoteStorage { // load all features and emit `ready` this._init(); - /** - * TODO: document - */ this.fireInitial = function () { + // When caching is turned on, emit change events with origin "local" for + // all cached documents if (this.local) { setTimeout(this.local.fireInitial.bind(this.local), 0); } From 77aa057b49137cd28caf92f4975de213aefe2755 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 13:35:59 +0400 Subject: [PATCH 30/47] Remove unused static property --- src/remotestorage.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/remotestorage.ts b/src/remotestorage.ts index d214a8eeb..79a854376 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -452,14 +452,6 @@ export class RemoteStorage { return this.remote.connected; } - /** - * FIXME: Instead of doing this, would be better to only - * export setAuthURL / getAuthURL from RemoteStorage prototype - * - * @ignore - */ - static Authorize = Authorize; - static SyncError = SyncError; static Unauthorized = UnauthorizedError; static DiscoveryError = Discover.DiscoveryError; From 95d06aa3eefe349bd59c0aa74313d0dbcfa19b9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 13:37:05 +0400 Subject: [PATCH 31/47] Clean up access and caching initialization Use _rs_init instead of custom prototype properties --- src/access.ts | 12 ++++---- src/authorize.ts | 2 +- src/baseclient.ts | 2 +- src/caching.ts | 7 +++-- src/remotestorage.ts | 50 ++++++--------------------------- test/unit/cachinglayer.test.mjs | 8 ++---- test/unit/sync.test.mjs | 9 +++--- 7 files changed, 28 insertions(+), 62 deletions(-) diff --git a/src/access.ts b/src/access.ts index 3245048f4..3bbcfde9b 100644 --- a/src/access.ts +++ b/src/access.ts @@ -1,3 +1,5 @@ +import type RemoteStorage from "./remotestorage"; + export type AccessMode = 'r' | 'rw'; export type AccessScope = string; @@ -22,11 +24,6 @@ export class Access { rootPaths: string[]; storageType: string; - // TODO create custom type for init function - static _rs_init(): void { - return; - } - constructor() { this.reset(); } @@ -197,6 +194,11 @@ export class Access { setStorageType (type: string): void { this.storageType = type; } + + static _rs_init(remoteStorage: RemoteStorage): void { + remoteStorage.access = new Access(); + return; + } } export default Access; diff --git a/src/authorize.ts b/src/authorize.ts index 241952bb8..fb065b2b7 100644 --- a/src/authorize.ts +++ b/src/authorize.ts @@ -1,5 +1,5 @@ +import type RemoteStorage from './remotestorage'; import log from './log'; -import RemoteStorage from './remotestorage'; import { localStorageAvailable, globalContext, toBase64 } from './util'; import UnauthorizedError from './unauthorized-error'; import { EventHandler } from './eventhandling'; diff --git a/src/baseclient.ts b/src/baseclient.ts index af624ce76..6e549c941 100644 --- a/src/baseclient.ts +++ b/src/baseclient.ts @@ -1,4 +1,5 @@ import tv4 from 'tv4'; +import type RemoteStorage from './remotestorage'; import type { JsonSchemas } from './interfaces/json_schema'; import type { ChangeObj } from './interfaces/change_obj'; import type { QueuedRequestResponse } from './interfaces/queued_request_response'; @@ -7,7 +8,6 @@ import SchemaNotFound from './schema-not-found-error'; import EventHandling from './eventhandling'; import config from './config'; import { applyMixins, cleanPath, isFolder } from './util'; -import RemoteStorage from './remotestorage'; function getModuleNameFromBase(path: string): string { const parts = path.split('/'); diff --git a/src/caching.ts b/src/caching.ts index db10cca25..795097979 100644 --- a/src/caching.ts +++ b/src/caching.ts @@ -1,7 +1,7 @@ -import { containingFolder, isFolder } from './util'; -import log from './log'; import type Access from './access'; import type RemoteStorage from './remotestorage'; +import { containingFolder, isFolder } from './util'; +import log from './log'; /** * @class @@ -182,7 +182,8 @@ export class Caching { * * @internal **/ - static _rs_init (/*remoteStorage*/): void { + static _rs_init (remoteStorage: RemoteStorage): void { + remoteStorage.caching = new Caching(remoteStorage); return; } } diff --git a/src/remotestorage.ts b/src/remotestorage.ts index 79a854376..19c19b3bb 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -295,14 +295,19 @@ export class RemoteStorage { apiKeys: {googledrive?: {clientId: string}; dropbox?: {appKey: string}} = {}; /** + * Managing claimed access scopes */ access: Access; + /** + * Managing cache settings */ - sync: Sync; + caching: Caching; + /** + * @internal */ - caching: Caching; + sync: Sync; /** * @internal @@ -345,7 +350,7 @@ export class RemoteStorage { * Access to the local caching backend used. Usually either a * `RemoteStorage.IndexedDB` or `RemoteStorage.LocalStorage` instance. * - * Not available, when caching is turned off. + * Not available when caching is turned off. * * @internal */ @@ -1225,45 +1230,6 @@ export class RemoteStorage { } } -/** - * @property access - * - * Tracking claimed access scopes. A instance. -*/ -Object.defineProperty(RemoteStorage.prototype, 'access', { - get: function() { - const access = new Access(); - Object.defineProperty(this, 'access', { - value: access - }); - return access; - }, - configurable: true -}); - -// TODO Clean up/harmonize how modules are loaded and/or document this architecture properly -// -// At this point the remoteStorage object has not been created yet. -// Only its prototype exists so far, so we define a self-constructing -// property on there: - -/** - * Property: caching - * - * Caching settings. A instance. - */ -// FIXME Was in rs_init of Caching but don't want to require RemoteStorage from there. -Object.defineProperty(RemoteStorage.prototype, 'caching', { - configurable: true, - get: function () { - const caching = new Caching(this); - Object.defineProperty(this, 'caching', { - value: caching - }); - return caching; - } -}); - export interface RemoteStorage extends EventHandling {} applyMixins(RemoteStorage, [EventHandling]); diff --git a/test/unit/cachinglayer.test.mjs b/test/unit/cachinglayer.test.mjs index 0befd0255..1e7d11a01 100644 --- a/test/unit/cachinglayer.test.mjs +++ b/test/unit/cachinglayer.test.mjs @@ -3,6 +3,7 @@ import { expect } from 'chai'; import fetchMock from 'fetch-mock'; import { RemoteStorage } from '../../build/remotestorage.js'; +import { Caching } from "../../build/caching.js"; import { Sync } from '../../build/sync.js'; import FakeAccess from '../helpers/fake-access.mjs'; @@ -10,14 +11,11 @@ describe("CachingLayer", function() { beforeEach(function(done) { this.original = {}; this.rs = new RemoteStorage(); - Object.defineProperty(this.rs, 'access', { - value: new FakeAccess(), - writable: true, - configurable: true - }); this.rs.on('features-loaded', () => { this.rs._handlers['connected'] = []; + this.rs.access = new FakeAccess(); + Caching._rs_init(this.rs); // needs FakeAccess, too this.rs.syncStopped = true; this.rs.sync = new Sync(this.rs); done(); diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 45e4a66e8..772553627 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -5,6 +5,7 @@ import sinon from "sinon"; import InMemoryStorage from "../../build/inmemorystorage.js"; import { RemoteStorage } from "../../build/remotestorage.js"; +import { Caching } from "../../build/caching.js"; import { Sync } from "../../build/sync.js"; import FakeAccess from "../helpers/fake-access.mjs"; import UnauthorizedError from "./build/unauthorized-error.js"; @@ -15,13 +16,11 @@ describe("Sync", function() { beforeEach(function(done) { this.original = {}; this.rs = new RemoteStorage(); - Object.defineProperty(this.rs, "access", { - value: new FakeAccess(), - writable: true, - configurable: true - }); this.rs.on("features-loaded", () => { + this.rs.access = new FakeAccess(); + Caching._rs_init(this.rs); + this.rs._handlers["connected"] = []; this.rs.local = new InMemoryStorage(); this.rs.syncStopped = true; From e704370da2e31d6589e45528c8db3347e581b067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 14:55:07 +0400 Subject: [PATCH 32/47] Don't log full error for Sync errors Only log the message as debug log, without the stack trace refs #839 --- src/sync.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync.ts b/src/sync.ts index dbf195dca..edcdcce87 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1037,7 +1037,7 @@ export class Sync { } async finishUnsuccessfulTask (task: SyncTask, err: Error): Promise { - log('[Sync] Error', err); + log('[Sync]', err.message); this._finishedTasks.shift(); delete this._timeStarted[task.path]; From 8e3bb2782767afbe9cc975cc083055ae1f837910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 15:27:11 +0400 Subject: [PATCH 33/47] Partially revert 95d06aa3 In order for devs not having to wait feature loading before managing access and caching settings, we need those classes to be instantiated onthe prototype already --- src/access.ts | 5 +---- src/caching.ts | 7 +++---- src/remotestorage.ts | 21 +++++++++++++++++++++ test/unit/cachinglayer.test.mjs | 8 +++++--- test/unit/sync.test.mjs | 9 +++++---- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/access.ts b/src/access.ts index 3bbcfde9b..f53ec9343 100644 --- a/src/access.ts +++ b/src/access.ts @@ -1,5 +1,3 @@ -import type RemoteStorage from "./remotestorage"; - export type AccessMode = 'r' | 'rw'; export type AccessScope = string; @@ -195,8 +193,7 @@ export class Access { this.storageType = type; } - static _rs_init(remoteStorage: RemoteStorage): void { - remoteStorage.access = new Access(); + static _rs_init(): void { return; } } diff --git a/src/caching.ts b/src/caching.ts index 795097979..db10cca25 100644 --- a/src/caching.ts +++ b/src/caching.ts @@ -1,7 +1,7 @@ -import type Access from './access'; -import type RemoteStorage from './remotestorage'; import { containingFolder, isFolder } from './util'; import log from './log'; +import type Access from './access'; +import type RemoteStorage from './remotestorage'; /** * @class @@ -182,8 +182,7 @@ export class Caching { * * @internal **/ - static _rs_init (remoteStorage: RemoteStorage): void { - remoteStorage.caching = new Caching(remoteStorage); + static _rs_init (/*remoteStorage*/): void { return; } } diff --git a/src/remotestorage.ts b/src/remotestorage.ts index 19c19b3bb..cf22e7d7e 100644 --- a/src/remotestorage.ts +++ b/src/remotestorage.ts @@ -1230,6 +1230,27 @@ export class RemoteStorage { } } +// At this point the remoteStorage object has not been created yet. Only +// its prototype exists so far, so we define self-constructing properties on +// it, in order for devs not having to wait for feature loading before managing +// access and caching settings +Object.defineProperty(RemoteStorage.prototype, 'access', { + configurable: true, + get: function() { + const access = new Access(); + Object.defineProperty(this, 'access', { value: access }); + return access; + }, +}); +Object.defineProperty(RemoteStorage.prototype, 'caching', { + configurable: true, + get: function () { + const caching = new Caching(this); + Object.defineProperty(this, 'caching', { value: caching }); + return caching; + } +}); + export interface RemoteStorage extends EventHandling {} applyMixins(RemoteStorage, [EventHandling]); diff --git a/test/unit/cachinglayer.test.mjs b/test/unit/cachinglayer.test.mjs index 1e7d11a01..0befd0255 100644 --- a/test/unit/cachinglayer.test.mjs +++ b/test/unit/cachinglayer.test.mjs @@ -3,7 +3,6 @@ import { expect } from 'chai'; import fetchMock from 'fetch-mock'; import { RemoteStorage } from '../../build/remotestorage.js'; -import { Caching } from "../../build/caching.js"; import { Sync } from '../../build/sync.js'; import FakeAccess from '../helpers/fake-access.mjs'; @@ -11,11 +10,14 @@ describe("CachingLayer", function() { beforeEach(function(done) { this.original = {}; this.rs = new RemoteStorage(); + Object.defineProperty(this.rs, 'access', { + value: new FakeAccess(), + writable: true, + configurable: true + }); this.rs.on('features-loaded', () => { this.rs._handlers['connected'] = []; - this.rs.access = new FakeAccess(); - Caching._rs_init(this.rs); // needs FakeAccess, too this.rs.syncStopped = true; this.rs.sync = new Sync(this.rs); done(); diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 772553627..45e4a66e8 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -5,7 +5,6 @@ import sinon from "sinon"; import InMemoryStorage from "../../build/inmemorystorage.js"; import { RemoteStorage } from "../../build/remotestorage.js"; -import { Caching } from "../../build/caching.js"; import { Sync } from "../../build/sync.js"; import FakeAccess from "../helpers/fake-access.mjs"; import UnauthorizedError from "./build/unauthorized-error.js"; @@ -16,11 +15,13 @@ describe("Sync", function() { beforeEach(function(done) { this.original = {}; this.rs = new RemoteStorage(); + Object.defineProperty(this.rs, "access", { + value: new FakeAccess(), + writable: true, + configurable: true + }); this.rs.on("features-loaded", () => { - this.rs.access = new FakeAccess(); - Caching._rs_init(this.rs); - this.rs._handlers["connected"] = []; this.rs.local = new InMemoryStorage(); this.rs.syncStopped = true; From deb348c49ff25583a4e5e9e7faef65cb4a12de5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 17:48:24 +0400 Subject: [PATCH 34/47] Broadcast change events to other tabs --- src/cachinglayer.ts | 28 ++++++++++++++++++++++++++-- test/unit/cachinglayer.test.mjs | 22 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts index c05e6b80b..5825cee76 100644 --- a/src/cachinglayer.ts +++ b/src/cachinglayer.ts @@ -2,6 +2,7 @@ import type { ChangeObj } from './interfaces/change_obj'; import type { QueuedRequestResponse } from './interfaces/queued_request_response'; import type { RSEvent } from './interfaces/rs_event'; import type { RSItem, RSNode, RSNodes, ProcessNodes } from './interfaces/rs_node'; +import Env from './env'; import EventHandling from './eventhandling'; import config from './config'; import log from './log'; @@ -9,6 +10,7 @@ import { applyMixins, deepClone, equal, + globalContext, isDocument, isFolder, pathsFromRoot @@ -120,6 +122,21 @@ abstract class CachingLayer { abstract setNodes(nodes: RSNodes): Promise; + /** + * Broadcast channel, used to inform other tabs about change events + */ + broadcastChannel: BroadcastChannel; + + constructor () { + const env = new Env(); + if (env.isBrowser() && !!globalContext["BroadcastChannel"]) { + this.broadcastChannel = new BroadcastChannel('remotestorage:changes'); + // Listen for change events from other tabs, and re-emit here + this.broadcastChannel.onmessage = (event: MessageEvent) => { + this.emitChange(event.data); + }; + } + } // -------------------------------------------------- @@ -377,9 +394,16 @@ abstract class CachingLayer { private _emitChangeEvents(events: RSEvent[]) { for (let i = 0, len = events.length; i < len; i++) { - this.emitChange(events[i]); + const change = events[i]; + + this.emitChange(change); + if (this.diffHandler) { - this.diffHandler(events[i].path); + this.diffHandler(change.path); + } + + if (!!this.broadcastChannel && change.origin === "window") { + this.broadcastChannel.postMessage(change); // Broadcast to other tabs } } } diff --git a/test/unit/cachinglayer.test.mjs b/test/unit/cachinglayer.test.mjs index 0befd0255..d3d0ac31f 100644 --- a/test/unit/cachinglayer.test.mjs +++ b/test/unit/cachinglayer.test.mjs @@ -95,4 +95,26 @@ describe("CachingLayer", function() { }); }); }); + + describe("#_emitChangeEvents", function() { + it("broadcasts the change to other browser tabs", function(done) { + this.rs.local.broadcastChannel = { + postMessage: (change) => { + expect(change.newValue).to.equal("bar"); + done(); + } + }; + + this.rs.local._emitChangeEvents([ + { + path: "/foo/bar", + origin: 'window', + oldValue: "foo", + newValue: "bar", + oldContentType: "text/plain", + newContentType: "text/plain" + } + ]); + }); + }); }); From 305b19e7ea1efad9141142c45fc877f6b1b2df5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 9 Jun 2025 21:11:01 +0400 Subject: [PATCH 35/47] Refactor permissions for FakeAccess --- test/helpers/fake-access.mjs | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/test/helpers/fake-access.mjs b/test/helpers/fake-access.mjs index bead815bf..0ca860cbd 100644 --- a/test/helpers/fake-access.mjs +++ b/test/helpers/fake-access.mjs @@ -11,27 +11,21 @@ class FakeAccess { return this._data[moduleName]; } - checkPathPermission (path, mode) { - if (path.substring(0, '/foo/'.length) === '/foo/') { - return true; - } - if (path.substring(0, '/read/access/'.length) === '/read/access/' && mode === 'r') { - return true; - } - if (path.substring(0, '/write/access/'.length) === '/write/access/') { - return true; - } - if (path.substring(0, '/readings/'.length) === '/readings/' && mode === 'r') { - return true; - } - if (path.substring(0, '/public/readings/'.length) === '/public/readings/' && mode === 'r') { - return true; - } - if (path.substring(0, '/writings/'.length) === '/writings/') { - return true; - } - if (path.substring(0, '/public/writings/'.length) === '/public/writings/') { - return true; + checkPathPermission(path, mode) { + const permissions = new Map([ + ['/foo/', 'rw'], + ['/read/access/', 'r'], + ['/write/access/', 'rw'], + ['/readings/', 'r'], + ['/public/readings/', 'r'], + ['/writings/', 'rw'], + ['/public/writings/', 'rw'] + ]); + + for (const [prefix, presetMode] of permissions) { + if (path.startsWith(prefix) && (!mode || presetMode.startsWith(mode))) { + return true; + } } return false; } From 1a8d3f9aa06e71cf34c73d653741f5f3691ee678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 13 Jun 2025 11:23:46 +0400 Subject: [PATCH 36/47] Types --- src/cachinglayer.ts | 43 +++++++++++++++++++-------------------- src/remote.ts | 4 ++-- src/syncedgetputdelete.ts | 11 ++++++---- src/wireclient.ts | 2 +- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/cachinglayer.ts b/src/cachinglayer.ts index 5825cee76..013b53bf1 100644 --- a/src/cachinglayer.ts +++ b/src/cachinglayer.ts @@ -187,7 +187,7 @@ abstract class CachingLayer { } } - async put (path: string, body: string, contentType: string): Promise { + async put (path: string, body: string, contentType: string): Promise { const paths = pathsFromRoot(path); function _processNodes(nodePaths: string[], nodes: RSNodes): RSNodes { @@ -227,7 +227,7 @@ abstract class CachingLayer { return this._updateNodes(paths, _processNodes); } - delete (path: string): unknown { + async delete (path: string): Promise { const paths = pathsFromRoot(path); return this._updateNodes(paths, function (nodePaths, nodes) { @@ -263,11 +263,12 @@ abstract class CachingLayer { } } } + return nodes; }); } - flush(path: string): unknown { + flush(path: string): Promise { return this._getAllDescendentPaths(path).then((paths: string[]) => { return this.getNodes(paths); }).then((nodes: RSNodes) => { @@ -325,7 +326,7 @@ abstract class CachingLayer { this.diffHandler = diffHandler; } - private _updateNodes(paths: string[], _processNodes: ProcessNodes): Promise { + private _updateNodes(paths: string[], _processNodes: ProcessNodes): Promise { return new Promise((resolve, reject) => { this._doUpdateNodes(paths, _processNodes, { resolve: resolve, @@ -334,7 +335,7 @@ abstract class CachingLayer { }); } - private _doUpdateNodes(paths: string[], _processNodes: ProcessNodes, promise) { + private async _doUpdateNodes(paths: string[], _processNodes: ProcessNodes, promise): Promise { if (this._updateNodesRunning) { this._updateNodesQueued.push({ paths: paths, @@ -342,11 +343,11 @@ abstract class CachingLayer { promise: promise }); return; - } else { - this._updateNodesRunning = true; } + this._updateNodesRunning = true; - this.getNodes(paths).then((nodes) => { + try { + let nodes = await this.getNodes(paths); const existingNodes = deepClone(nodes); const changeEvents = []; @@ -375,21 +376,19 @@ abstract class CachingLayer { } } - this.setNodes(nodes).then(() => { - this._emitChangeEvents(changeEvents); - promise.resolve({statusCode: 200}); - }); - }).then(() => { - return Promise.resolve(); - }, (err) => { + await this.setNodes(nodes); + this._emitChangeEvents(changeEvents); + + promise.resolve({ statusCode: 200 }); + } catch (err) { promise.reject(err); - }).then(() => { - this._updateNodesRunning = false; - const nextJob = this._updateNodesQueued.shift(); - if (nextJob) { - this._doUpdateNodes(nextJob.paths, nextJob.cb, nextJob.promise); - } - }); + } + + this._updateNodesRunning = false; + const nextJob = this._updateNodesQueued.shift(); + if (nextJob) { + await this._doUpdateNodes(nextJob.paths, nextJob.cb, nextJob.promise); + } } private _emitChangeEvents(events: RSEvent[]) { diff --git a/src/remote.ts b/src/remote.ts index e3d6c4d9b..c684ebf44 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -67,9 +67,9 @@ export interface RemoteSettings { export interface RemoteResponse { statusCode: number; - revision?: string; + body?: string | { [key: string]: any; }; contentType?: string; - body?: any; + revision?: string; } /** diff --git a/src/syncedgetputdelete.ts b/src/syncedgetputdelete.ts index 20f6650d4..c5d504072 100644 --- a/src/syncedgetputdelete.ts +++ b/src/syncedgetputdelete.ts @@ -1,3 +1,5 @@ +import type { QueuedRequestResponse } from './interfaces/queued_request_response'; +import type { RemoteResponse } from './remote'; import log from './log'; function shareFirst(path: string): boolean { @@ -16,7 +18,7 @@ function defaultMaxAge(context): false | number { } const SyncedGetPutDelete = { - get: function (path: string, maxAge: undefined | false | number): Promise { + get: function (path: string, maxAge: undefined | false | number): Promise { if (!this.local) { return this.remote.get(path); } else { @@ -29,7 +31,7 @@ const SyncedGetPutDelete = { } }, - put: function (path: string, body: unknown, contentType: string): Promise { + put: function (path: string, body: unknown, contentType: string): Promise { if (shareFirst.bind(this)(path)) { return SyncedGetPutDelete._wrapBusyDone.call(this, this.remote.put(path, body, contentType)); } @@ -40,7 +42,7 @@ const SyncedGetPutDelete = { } }, - 'delete': function (path: string): Promise { + 'delete': function (path: string): Promise { if (this.local) { return this.local.delete(path); } else { @@ -48,8 +50,9 @@ const SyncedGetPutDelete = { } }, - _wrapBusyDone: async function (result: Promise): Promise { + _wrapBusyDone: async function (result: Promise): Promise { this._emit('wire-busy'); + return result.then((r) => { this._emit('wire-done', { success: true }); return Promise.resolve(r); diff --git a/src/wireclient.ts b/src/wireclient.ts index f77caf3cd..28efac452 100644 --- a/src/wireclient.ts +++ b/src/wireclient.ts @@ -339,7 +339,7 @@ class WireClient extends RemoteBase implements Remote { let itemsMap = {}; if (typeof (r.body) !== 'undefined') { try { - r.body = JSON.parse(r.body); + r.body = JSON.parse(r.body as string); } catch (e) { return Promise.reject('Folder description at ' + this.href + cleanPath(path) + ' is not JSON'); } From 4ee34316dbee2b9f8521d05d04bb628141519c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 13 Jun 2025 14:58:14 +0400 Subject: [PATCH 37/47] Add tests for merging nodes with empty bodies --- test/unit/sync.test.mjs | 105 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 3 deletions(-) diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 45e4a66e8..5c6ff1069 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -840,8 +840,74 @@ describe("Sync", function() { }); }); - describe("#autoMerge", function() { - describe("when node was updated", function() { + describe.only("#autoMerge", function() { + describe("new node", function() { + beforeEach(function() { + this.node = { + path: "foo", + common: {}, + remote: { body: "new value", contentType: "new content-type", revision: "remote" } + }; + }); + + it("emits a 'change' event", function(done) { + this.rs.local.emitChange = function(changeEvent) { + expect(changeEvent).to.deep.equal({ + origin: "remote", + path: "foo", + newValue: "new value", + oldValue: undefined, + newContentType: "new content-type", + oldContentType: undefined + }); + done(); + }; + + this.rs.sync.autoMerge(this.node); + }); + + it("merges the node items", function() { + expect(this.rs.sync.autoMerge(this.node)).to.deep.equal({ + path: "foo", + common: { body: "new value", contentType: "new content-type", revision: "remote" } + }); + }); + + describe("with zero-length body", function() { + beforeEach(function() { + this.node = { + path: "foo", + common: {}, + remote: { body: "", contentType: "new content-type", revision: "remote" } + }; + }); + + it("emits a 'change' event", function(done) { + this.rs.local.emitChange = function(changeEvent) { + expect(changeEvent).to.deep.equal({ + origin: "remote", + path: "foo", + newValue: "", + oldValue: undefined, + newContentType: "new content-type", + oldContentType: undefined + }); + done(); + }; + + this.rs.sync.autoMerge(this.node); + }); + + it("merges the node items", function() { + expect(this.rs.sync.autoMerge(this.node)).to.deep.equal({ + path: "foo", + common: { body: "", contentType: "new content-type", revision: "remote" } + }); + }); + }); + }); + + describe("updated node", function() { beforeEach(function() { this.node = { path: "foo", @@ -872,9 +938,42 @@ describe("Sync", function() { common: { body: "new value", contentType: "new content-type", revision: "remote" } }); }); + + describe("with zero-length body", function() { + beforeEach(function() { + this.node = { + path: "foo", + common: { body: "old value", contentType: "old content-type", revision: "common" }, + remote: { body: "", contentType: "new content-type", revision: "remote" } + }; + }); + + it("emits a 'change' event", function(done) { + this.rs.local.emitChange = function(changeEvent) { + expect(changeEvent).to.deep.equal({ + origin: "remote", + path: "foo", + newValue: "", + oldValue: "old value", + newContentType: "new content-type", + oldContentType: "old content-type" + }); + done(); + }; + + this.rs.sync.autoMerge(this.node); + }); + + it("merges the node items", function() { + expect(this.rs.sync.autoMerge(this.node)).to.deep.equal({ + path: "foo", + common: { body: "", contentType: "new content-type", revision: "remote" } + }); + }); + }); }); - describe("when node was deleted", function() { + describe("deleted node", function() { describe("with node cached before", function() { beforeEach(function() { this.node = { From ebcb9e12ee5a961288c5f73d83060bbfd527560c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 13 Jun 2025 15:16:07 +0400 Subject: [PATCH 38/47] Fix handling of empty nodes refs #1335 --- src/sync.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index edcdcce87..64e2a44e4 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -507,10 +507,10 @@ export class Sync { lastCommonContentType: node.common.contentType }); - if (node.remote.body) { - node.common = node.remote; - } else { + if (node.remote.body === false) { node.common = {}; + } else { + node.common = node.remote; } delete node.remote; delete node.local; @@ -562,12 +562,12 @@ export class Sync { newContentType: node.remote.contentType }; - if (change.oldValue || change.newValue) { + if (change.oldValue !== undefined || change.newValue !== undefined) { this.rs.local.emitChange(change); } - if (!node.remote.body) { // no remote, so delete/don't create - return; + if (node.remote.body === false) { + return; // no remote, so delete } node.common = node.remote; @@ -762,7 +762,7 @@ export class Sync { } } - if (typeof(node) !== 'object' || + if (typeof(node) !== 'object' || node.path !== path || typeof(node.common) !== 'object') { node = { path: path, common: {} }; From 5c1dac7c1a83aa63f9d320fabc5d688f0f17204b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 13 Jun 2025 16:44:57 +0400 Subject: [PATCH 39/47] Re-enable all tests Accidentally left a `.only` in the last commit --- test/unit/sync.test.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 5c6ff1069..68405cb96 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -840,7 +840,7 @@ describe("Sync", function() { }); }); - describe.only("#autoMerge", function() { + describe("#autoMerge", function() { describe("new node", function() { beforeEach(function() { this.node = { From c561870a64355f36455888cd7120a3ece8febdaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Fri, 13 Jun 2025 19:15:23 +0400 Subject: [PATCH 40/47] Fix cached documents not being fully deleted in Anonymous Mode fixes #1204 --- src/baseclient.ts | 5 +- src/cachinglayer.ts | 13 ++-- src/syncedgetputdelete.ts | 6 +- test/unit/cachinglayer-suite.js | 6 +- test/unit/cachinglayer.test.mjs | 109 +++++++++++++++++++++++++++++ test/unit/inmemorycaching-suite.js | 8 +-- 6 files changed, 130 insertions(+), 17 deletions(-) diff --git a/src/baseclient.ts b/src/baseclient.ts index 6e549c941..14d8bb49e 100644 --- a/src/baseclient.ts +++ b/src/baseclient.ts @@ -624,9 +624,8 @@ 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 { + async remove (path: string): Promise { if (typeof path !== 'string') { return Promise.reject('Argument \'path\' of baseClient.remove must be a string'); } @@ -634,7 +633,7 @@ export class BaseClient { 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 index 013b53bf1..d554ef82f 100644 --- 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 { + async delete (path: string, remoteConnected: boolean): Promise { 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 index c5d504072..89bd338f4 100644 --- a/src/syncedgetputdelete.ts +++ b/src/syncedgetputdelete.ts @@ -42,11 +42,11 @@ const SyncedGetPutDelete = { } }, - 'delete': function (path: string): Promise { + 'delete': function (path: string, remoteConnected: boolean): Promise { 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 index f6870c3b9..88f670772 100644 --- 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 index d3d0ac31f..b23603349 100644 --- 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 index a2b70ee7d..4d7950390 100644 --- 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}); }); }); From 0482c78ea30ec9a18055a838819d5f45a301f474 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 16 Jun 2025 13:22:40 +0400 Subject: [PATCH 41/47] Remove unnecessary context for event handler calls --- src/eventhandling.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/eventhandling.ts b/src/eventhandling.ts index d168011fa..d3e1c552a 100644 --- a/src/eventhandling.ts +++ b/src/eventhandling.ts @@ -76,7 +76,7 @@ export class EventHandling { _emit (eventName: string, ...args: unknown[]): void { this._validateEvent(eventName); this._handlers[eventName].slice().forEach((handler) => { - handler.apply(this, args); + handler(...args); }); } From 16fbeb4aec48f05142175d5d5be44a7d23bdfb7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Mon, 16 Jun 2025 13:22:57 +0400 Subject: [PATCH 42/47] Emit conflict events asynchronously This way, apps can use these the same way as they can use other change events. --- src/baseclient.ts | 23 +++++++++-------------- src/sync.ts | 18 ++++++++---------- test/unit/sync.test.mjs | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 24 deletions(-) diff --git a/src/baseclient.ts b/src/baseclient.ts index 14d8bb49e..cb9af420a 100644 --- a/src/baseclient.ts +++ b/src/baseclient.ts @@ -147,8 +147,8 @@ function getModuleNameFromBase(path: string): string { * during sync. * * > [!NOTE] - * > Automatically receiving remote changes depends on the {@link caching!Caching} settings - * > for your module/paths. + * > Automatically receiving remote changes depends on the + * > {@link caching!Caching caching} settings for your module/paths. * * ### `window` * @@ -180,13 +180,13 @@ function getModuleNameFromBase(path: string): string { * } * ``` * - * But when this change is pushed out by asynchronous synchronization, this change - * may be rejected by the server, if the remote version has in the meantime changed - * from `white` to for instance `red`; this will then lead to a change event with - * origin `conflict` (usually a few seconds after the event with origin `window`, - * if you have those activated). Note that since you already changed it from - * `white` to `blue` in the local version a few seconds ago, `oldValue` is now - * your local value of `blue`: + * However, when this change is pushed out by the sync process, it will be + * rejected by the server, if the remote version has changed in the meantime, + * for example from `white` to `red`. This will lead to a change event with + * origin `conflict`, usually a few seconds after the event with origin + * `window`. Note that since you already changed it from `white` to `blue` in + * the local version a few seconds ago, `oldValue` is now your local value of + * `blue`: * * ```js * { @@ -212,11 +212,6 @@ function getModuleNameFromBase(path: string): string { * * If there is an algorithm to merge the differences between local and remote * versions of the data, conflicts may be automatically resolved. - * {@link storeObject} or {@link storeFile} must not be called synchronously from - * the change event handler, nor by chaining Promises. {@link storeObject} or - * {@link storeFile} must not be called until the next iteration of the JavaScript - * Task Queue, using for example - * [`setTimeout()`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout). * * If no algorithm exists, conflict resolution typically involves displaying local * and remote versions to the user, and having the user merge them, or choose diff --git a/src/sync.ts b/src/sync.ts index 64e2a44e4..1c70c0fd7 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -493,17 +493,15 @@ export class Sync { node = mergeMutualDeletion(node); delete node.remote; } else if (node.remote.body !== undefined) { - // keep/revert: - log('[Sync] Emitting keep/revert'); - - this.rs.local.emitChange({ - origin: 'conflict', - path: node.path, - oldValue: node.local.body, - newValue: node.remote.body, + log('[Sync] Emitting conflict event'); + setTimeout(this.rs.local.emitChange.bind(this.rs.local), 10, { + origin: 'conflict', + path: node.path, + oldValue: node.local.body, + newValue: node.remote.body, lastCommonValue: node.common.body, - oldContentType: node.local.contentType, - newContentType: node.remote.contentType, + oldContentType: node.local.contentType, + newContentType: node.remote.contentType, lastCommonContentType: node.common.contentType }); diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index 68405cb96..f4579bbe5 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -838,6 +838,38 @@ describe("Sync", function() { expect(this.rs.sync.autoMergeDocument(node)).to .deep.equal(localAndRemoteRemoved); }); + + describe("when node was also changed on remote", function() { + beforeEach(function() { + this.emitChange = sinon.spy(this.rs.local, "emitChange"); + + this.rs.sync.autoMergeDocument({ + path: "foo", + common: { body: "foo", contentType: "bloo", revision: "common" }, + local: { body: "floo", contentType: "blaloo" }, + remote: { body: "florb", revision: "updated-elsewhere" } + }); + }); + + it("asynchronously emits a conflict event", function(done) { + expect(this.emitChange.called).to.be.false; + + setTimeout(() => { + expect(this.emitChange.called).to.be.true; + expect(this.emitChange.getCall(0).firstArg).to.deep.equal({ + origin: "conflict", + path: "foo", + oldValue: "floo", + newValue: "florb", + lastCommonValue: "foo", + oldContentType: "blaloo", + newContentType: undefined, + lastCommonContentType: "bloo" + }); + done(); + }, 20); + }); + }); }); describe("#autoMerge", function() { From fb6bba88544722b346dcecf3993a6fb44a990666 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 18 Jun 2025 12:29:02 +0400 Subject: [PATCH 43/47] Use async/await --- src/sync.ts | 84 ++++++++++++++++++++++++++--------------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 1c70c0fd7..35f0f1baf 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -744,60 +744,60 @@ export class Sync { paths = [path, parentPath]; } - return this.rs.local.getNodes(paths).then((nodes: RSNodes) => { - let itemName: string; - let node: RSNode = nodes[path]; - let parentNode: RSNode; - const missingChildren = {}; - - function collectMissingChildren (folder): void { - if (folder && folder.itemsMap) { - for (itemName in folder.itemsMap) { - if (!bodyOrItemsMap[itemName]) { - missingChildren[itemName] = true; - } + const nodes = await this.rs.local.getNodes(paths); + let node: RSNode = nodes[path]; + let parentNode: RSNode = nodes[parentPath]; + let itemName: string; + const missingChildren = {}; + + function collectMissingChildren (folder): void { + if (folder && folder.itemsMap) { + for (itemName in folder.itemsMap) { + if (!bodyOrItemsMap[itemName]) { + missingChildren[itemName] = true; } } } + } + + if (typeof(node) !== 'object' || + node.path !== path || + typeof(node.common) !== 'object') { + node = { path: path, common: {} }; + nodes[path] = node; + } + + node.remote = { + revision: revision, + timestamp: this.now() + }; + + if (isFolder(path)) { + collectMissingChildren(node.common); + collectMissingChildren(node.remote); - if (typeof(node) !== 'object' || - node.path !== path || - typeof(node.common) !== 'object') { - node = { path: path, common: {} }; - nodes[path] = node; + node.remote.itemsMap = {}; + for (itemName in bodyOrItemsMap as RSItem["itemsMap"]) { + node.remote.itemsMap[itemName] = true; } + } else { + node.remote.body = bodyOrItemsMap; + node.remote.contentType = contentType; - node.remote = { - revision: revision, - timestamp: this.now() - }; + if (parentNode && parentNode.local && parentNode.local.itemsMap) { + itemName = path.substring(parentPath.length); - if (isFolder(path)) { - collectMissingChildren(node.common); - collectMissingChildren(node.remote); + parentNode.local.itemsMap[itemName] = true; - node.remote.itemsMap = {}; - for (itemName in bodyOrItemsMap as RSItem["itemsMap"]) { - node.remote.itemsMap[itemName] = true; - } - } else { - node.remote.body = bodyOrItemsMap; - node.remote.contentType = contentType; - - parentNode = nodes[parentPath]; - if (parentNode && parentNode.local && parentNode.local.itemsMap) { - itemName = path.substring(parentPath.length); - parentNode.local.itemsMap[itemName] = true; - if (equal(parentNode.local.itemsMap, parentNode.common.itemsMap)) { - delete parentNode.local; - } + if (equal(parentNode.local.itemsMap, parentNode.common.itemsMap)) { + delete parentNode.local; } } + } - nodes[path] = this.autoMerge(node); + nodes[path] = this.autoMerge(node); - return { toBeSaved: nodes, missingChildren }; - }); + return { toBeSaved: nodes, missingChildren }; } /** From 098962db95c072a631e297dfa5df06a1c9b0a1c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 18 Jun 2025 12:34:25 +0400 Subject: [PATCH 44/47] 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). --- src/sync.ts | 36 +++++++-- test/unit/sync.test.mjs | 159 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 7 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 35f0f1baf..0483206d3 100644 --- a/src/sync.ts +++ b/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; diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index f4579bbe5..b272c5404 100644 --- a/test/unit/sync.test.mjs +++ b/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() { From d04f33480b8190f3aa6662e06ced0dbc91896198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Wed, 18 Jun 2025 14:28:51 +0400 Subject: [PATCH 45/47] Do not emit conflict events for mutual deletions No changes can be lost when documents are deleted on both ends --- src/sync.ts | 26 ++++++++++++--------- test/unit/sync.test.mjs | 50 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 0483206d3..6e80d6c53 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -507,17 +507,21 @@ export class Sync { node = mergeMutualDeletion(node); delete node.remote; } else if (node.remote.body !== undefined) { - log('[Sync] Emitting conflict event'); - setTimeout(this.rs.local.emitChange.bind(this.rs.local), 10, { - origin: 'conflict', - path: node.path, - oldValue: node.local.body, - newValue: node.remote.body, - lastCommonValue: node.common.body, - oldContentType: node.local.contentType, - newContentType: node.remote.contentType, - lastCommonContentType: node.common.contentType - }); + if (node.remote.body === false && node.local?.body === false) { + // Deleted on both sides, nothing to do + } else { + log('[Sync] Emitting conflict event'); + setTimeout(this.rs.local.emitChange.bind(this.rs.local), 10, { + origin: 'conflict', + path: node.path, + oldValue: node.local.body, + newValue: node.remote.body, + lastCommonValue: node.common.body, + oldContentType: node.local.contentType, + newContentType: node.remote.contentType, + lastCommonContentType: node.common.contentType + }); + } if (node.remote.body === false) { node.common = {}; diff --git a/test/unit/sync.test.mjs b/test/unit/sync.test.mjs index b272c5404..386bd6ad1 100644 --- a/test/unit/sync.test.mjs +++ b/test/unit/sync.test.mjs @@ -949,6 +949,56 @@ describe("Sync", function() { }, 20); }); }); + + describe("when node was also deleted on remote", function() { + beforeEach(function() { + this.emitChange = sinon.spy(this.rs.local, "emitChange"); + + this.rs.sync.autoMergeDocument({ + path: "foo", + common: { body: "foo", contentType: "bloo", revision: "common" }, + local: { body: false }, + remote: { body: false } + }); + }); + + it("does not emit a conflict event", function(done) { + setTimeout(() => { + expect(this.emitChange.called).to.be.false; + done(); + }, 20); + }); + }); + + describe("when node was changed on remote, but deleted locally", function() { + beforeEach(function() { + this.emitChange = sinon.spy(this.rs.local, "emitChange"); + + this.rs.sync.autoMergeDocument({ + path: "foo", + remote: { body: "bar", contentType: "text/plain", revision: "newrev"}, + common: { body: "foo", contentType: "text/plain", revision: "common" }, + local: { body: false } + }); + }); + + it("emits a conflict event", function(done) { + setTimeout(() => { + expect(this.emitChange.called).to.be.true; + expect(this.emitChange.getCall(0).firstArg).to.deep.equal({ + origin: "conflict", + path: "foo", + oldValue: false, + newValue: "bar", + lastCommonValue: "foo", + oldContentType: undefined, + newContentType: "text/plain", + lastCommonContentType: "text/plain" + }); + done(); + }, 20); + }); + }); }); describe("#autoMergeFolder", function() { From 6afc445871536b71bcdd023d6c70c45f675be07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= Date: Thu, 19 Jun 2025 11:42:26 +0400 Subject: [PATCH 46/47] Always emit `sync-done` when done It doesn't matter if a client received `sync-started` or not. When there are no more tasks to run after any task has finished successfully, the sync is done. Also removes a superfluous second log statement for when sync is done. --- src/sync.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/sync.ts b/src/sync.ts index 6e80d6c53..047ca05ec 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -1046,11 +1046,8 @@ export class Sync { await this.collectTasks(false).then(() => { // See if there are any more tasks that are not refresh tasks if (!this.hasTasks() || this.stopped) { - log('[Sync] Sync is done! Reschedule?', Object.keys(this._tasks).length, this.stopped); - if (!this.done) { - this.done = true; - this.rs._emit('sync-done', { completed: true }); - } + if (!this.done) { this.done = true; } + this.rs._emit('sync-done', { completed: true }); } else { // Use a 10ms timeout to let the JavaScript runtime catch its breath // (and hopefully force an IndexedDB auto-commit?), and also to cause From cc87eddf5e9b1cd238931f71cf0a0b412968de40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A2u=20Cao?= <842+raucao@users.noreply.github.com> Date: Tue, 12 Aug 2025 14:53:24 +0200 Subject: [PATCH 47/47] Fix typeof statement Co-authored-by: Garret Alfert --- src/sync.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sync.ts b/src/sync.ts index 047ca05ec..c2d696cf3 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -729,7 +729,7 @@ export class Sync { collectSubPaths(node.common, path); collectSubPaths(node.local, path); } else { - if (node.common && typeof(node.common.body) !== undefined) { + if (node.common && typeof(node.common.body) !== 'undefined') { changedNodes[path] = deepClone(node); changedNodes[path].remote = { body: false,