From 25f3dfbb75f466a0b77181f208573ed844bece86 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 14:22:59 +0200 Subject: [PATCH 1/3] fix failing test --- src/matrix/room/RoomSummary.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/matrix/room/RoomSummary.js b/src/matrix/room/RoomSummary.js index 36c7d203..b3758d23 100644 --- a/src/matrix/room/RoomSummary.js +++ b/src/matrix/room/RoomSummary.js @@ -263,7 +263,7 @@ export function tests() { "membership trigger change": function(assert) { const summary = new RoomSummary("id"); let written = false; - const changes = summary.writeSync({}, "join", {roomSummary: {set: () => { written = true; }}}); + const changes = summary.writeSync({}, "join", false, false, {roomSummary: {set: () => { written = true; }}}); assert(changes); assert(written); assert.equal(changes.membership, "join"); From 14b27f81fe08746e3912a47a57031839f83e959d Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 14:28:40 +0200 Subject: [PATCH 2/3] store session values as individual values in store so we don't have to write the whole object every time something changes we'll use this to store the olm account --- src/matrix/Session.js | 61 +++++++++++-------- src/matrix/storage/idb/schema.js | 22 ++++++- src/matrix/storage/idb/stores/SessionStore.js | 12 ++-- 3 files changed, 61 insertions(+), 34 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index 7a7dea52..da560303 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -24,7 +24,8 @@ export class Session { constructor({storage, hsApi, sessionInfo}) { this._storage = storage; this._hsApi = hsApi; - this._session = null; + this._syncToken = null; + this._syncFilterId = null; this._sessionInfo = sessionInfo; this._rooms = new ObservableMap(); this._sendScheduler = new SendScheduler({hsApi, backoff: new RateLimitingBackoff()}); @@ -42,11 +43,12 @@ export class Session { this._storage.storeNames.pendingEvents, ]); // restore session object - this._session = await txn.session.get(); - if (!this._session) { - this._session = {}; - return; - } + const [syncToken, syncFilterId] = await Promise.all([ + txn.session.get("syncToken"), + txn.session.get("syncFilterId") + ]); + this._syncToken = syncToken; + this._syncFilterId = syncFilterId; const pendingEventsByRoomId = await this._getPendingEventsByRoom(txn); // load rooms const rooms = await txn.roomSummary.getAll(); @@ -70,11 +72,9 @@ export class Session { const txn = await this._storage.readWriteTxn([ this._storage.storeNames.session ]); - const newSessionData = Object.assign({}, this._session, {serverVersions: lastVersionResponse}); - txn.session.set(newSessionData); + txn.session.set("serverVersions", lastVersionResponse); // TODO: what can we do if this throws? await txn.complete(); - this._session = newSessionData; } this._sendScheduler.start(); @@ -115,27 +115,28 @@ export class Session { } writeSync(syncToken, syncFilterId, accountData, txn) { - if (syncToken !== this._session.syncToken) { - // don't modify this._session because transaction might still fail - const newSessionData = Object.assign({}, this._session, {syncToken, syncFilterId}); - txn.session.set(newSessionData); - return newSessionData; + if (syncToken !== this._syncToken) { + // don't modify `this` because transaction might still fail + txn.session.set("syncToken", syncToken); + txn.session.set("syncFilterId", syncFilterId); + return {syncToken, syncFilterId}; } } - afterSync(newSessionData) { - if (newSessionData) { + afterSync(changes) { + if (changes) { // sync transaction succeeded, modify object state now - this._session = newSessionData; + this._syncToken = changes.syncToken; + this._syncFilterId = changes.syncFilterId; } } get syncToken() { - return this._session.syncToken; + return this._syncToken; } get syncFilterId() { - return this._session.syncFilterId; + return this._syncFilterId; } get user() { @@ -149,8 +150,8 @@ export function tests() { readTxn() { return Promise.resolve({ session: { - get() { - return Promise.resolve(Object.assign({}, session)); + get(key) { + return Promise.resolve(session[key]); } }, pendingEvents: { @@ -176,18 +177,24 @@ export function tests() { syncFilterId: 5, }), sessionInfo: {userId: ""}}); await session.load(); - let txnSetCalled = false; + let syncTokenSet = false; + let syncFilterIdSet = false; const syncTxn = { session: { - set({syncToken, syncFilterId}) { - txnSetCalled = true; - assert.equal(syncToken, "b"); - assert.equal(syncFilterId, 6); + set(key, value) { + if (key === "syncToken") { + assert.equal(value, "b"); + syncTokenSet = true; + } else if (key === "syncFilterId") { + assert.equal(value, 6); + syncFilterIdSet = true; + } } } }; const newSessionData = session.writeSync("b", 6, {}, syncTxn); - assert(txnSetCalled); + assert(syncTokenSet); + assert(syncFilterIdSet); assert.equal(session.syncToken, "a"); assert.equal(session.syncFilterId, 5); session.afterSync(newSessionData); diff --git a/src/matrix/storage/idb/schema.js b/src/matrix/storage/idb/schema.js index 21a108c8..8efe7c5f 100644 --- a/src/matrix/storage/idb/schema.js +++ b/src/matrix/storage/idb/schema.js @@ -1,12 +1,14 @@ -import {iterateCursor} from "./utils.js"; +import {iterateCursor, reqAsPromise} from "./utils.js"; import {RoomMember, EVENT_TYPE as MEMBER_EVENT_TYPE} from "../../room/members/RoomMember.js"; import {RoomMemberStore} from "./stores/RoomMemberStore.js"; +import {SessionStore} from "./stores/SessionStore.js"; // FUNCTIONS SHOULD ONLY BE APPENDED!! // the index in the array is the database version export const schema = [ createInitialStores, createMemberStore, + migrateSession, ]; // TODO: how to deal with git merge conflicts of this array? @@ -44,3 +46,21 @@ async function createMemberStore(db, txn) { } }); } + +async function migrateSession(db, txn) { + const session = txn.objectStore("session"); + try { + const PRE_MIGRATION_KEY = 1; + const entry = await reqAsPromise(session.get(PRE_MIGRATION_KEY)); + if (entry) { + session.delete(PRE_MIGRATION_KEY); + const store = new SessionStore(session); + for (const [key, value] of Object.entries(entry.value)) { + store.set(key, value); + } + } + } catch (err) { + txn.abort(); + console.error("could not migrate session", err.stack); + } +} diff --git a/src/matrix/storage/idb/stores/SessionStore.js b/src/matrix/storage/idb/stores/SessionStore.js index e26de3b4..2e74b9df 100644 --- a/src/matrix/storage/idb/stores/SessionStore.js +++ b/src/matrix/storage/idb/stores/SessionStore.js @@ -35,14 +35,14 @@ export class SessionStore { this._sessionStore = sessionStore; } - async get() { - const session = await this._sessionStore.selectFirst(IDBKeyRange.only(1)); - if (session) { - return session.value; + async get(key) { + const entry = await this._sessionStore.get(key); + if (entry) { + return entry.value; } } - set(session) { - return this._sessionStore.put({key: 1, value: session}); + set(key, value) { + return this._sessionStore.put({key, value}); } } From 09a018ade15961a3e479077afde7f1cffe7d84e5 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Thu, 27 Aug 2020 14:36:50 +0200 Subject: [PATCH 3/3] store sync token and filter id under the same key in session as they are updated together --- src/matrix/Session.js | 48 ++++++++++++-------------------- src/matrix/storage/idb/schema.js | 6 ++-- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/src/matrix/Session.js b/src/matrix/Session.js index da560303..b477c3d9 100644 --- a/src/matrix/Session.js +++ b/src/matrix/Session.js @@ -24,8 +24,7 @@ export class Session { constructor({storage, hsApi, sessionInfo}) { this._storage = storage; this._hsApi = hsApi; - this._syncToken = null; - this._syncFilterId = null; + this._syncInfo = null; this._sessionInfo = sessionInfo; this._rooms = new ObservableMap(); this._sendScheduler = new SendScheduler({hsApi, backoff: new RateLimitingBackoff()}); @@ -43,12 +42,7 @@ export class Session { this._storage.storeNames.pendingEvents, ]); // restore session object - const [syncToken, syncFilterId] = await Promise.all([ - txn.session.get("syncToken"), - txn.session.get("syncFilterId") - ]); - this._syncToken = syncToken; - this._syncFilterId = syncFilterId; + this._syncInfo = await txn.session.get("sync"); const pendingEventsByRoomId = await this._getPendingEventsByRoom(txn); // load rooms const rooms = await txn.roomSummary.getAll(); @@ -115,28 +109,27 @@ export class Session { } writeSync(syncToken, syncFilterId, accountData, txn) { - if (syncToken !== this._syncToken) { + if (syncToken !== this.syncToken) { + const syncInfo = {token: syncToken, filterId: syncFilterId}; // don't modify `this` because transaction might still fail - txn.session.set("syncToken", syncToken); - txn.session.set("syncFilterId", syncFilterId); - return {syncToken, syncFilterId}; + txn.session.set("sync", syncInfo); + return syncInfo; } } - afterSync(changes) { - if (changes) { + afterSync(syncInfo) { + if (syncInfo) { // sync transaction succeeded, modify object state now - this._syncToken = changes.syncToken; - this._syncFilterId = changes.syncFilterId; + this._syncInfo = syncInfo; } } get syncToken() { - return this._syncToken; + return this._syncInfo?.token; } get syncFilterId() { - return this._syncFilterId; + return this._syncInfo?.filterId; } get user() { @@ -173,28 +166,23 @@ export function tests() { return { "session data is not modified until after sync": async (assert) => { const session = new Session({storage: createStorageMock({ - syncToken: "a", - syncFilterId: 5, + sync: {token: "a", filterId: 5} }), sessionInfo: {userId: ""}}); await session.load(); - let syncTokenSet = false; - let syncFilterIdSet = false; + let syncSet = false; const syncTxn = { session: { set(key, value) { - if (key === "syncToken") { - assert.equal(value, "b"); - syncTokenSet = true; - } else if (key === "syncFilterId") { - assert.equal(value, 6); - syncFilterIdSet = true; + if (key === "sync") { + assert.equal(value.token, "b"); + assert.equal(value.filterId, 6); + syncSet = true; } } } }; const newSessionData = session.writeSync("b", 6, {}, syncTxn); - assert(syncTokenSet); - assert(syncFilterIdSet); + assert(syncSet); assert.equal(session.syncToken, "a"); assert.equal(session.syncFilterId, 5); session.afterSync(newSessionData); diff --git a/src/matrix/storage/idb/schema.js b/src/matrix/storage/idb/schema.js index 8efe7c5f..0ab5707a 100644 --- a/src/matrix/storage/idb/schema.js +++ b/src/matrix/storage/idb/schema.js @@ -54,10 +54,10 @@ async function migrateSession(db, txn) { const entry = await reqAsPromise(session.get(PRE_MIGRATION_KEY)); if (entry) { session.delete(PRE_MIGRATION_KEY); + const {syncToken, syncFilterId, serverVersions} = entry.value; const store = new SessionStore(session); - for (const [key, value] of Object.entries(entry.value)) { - store.set(key, value); - } + store.set("sync", {token: syncToken, filterId: syncFilterId}); + store.set("serverVersions", serverVersions); } } catch (err) { txn.abort();