From f8ea1702f8933324d0db21ddc81f026da41fe0b8 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 31 Aug 2018 14:40:56 +0200 Subject: [PATCH 1/6] store support for removing out of band members for a room --- src/store/indexeddb-local-backend.js | 76 ++++++++++++++++++++++++--- src/store/indexeddb-remote-backend.js | 4 ++ src/store/indexeddb-store-worker.js | 3 ++ src/store/indexeddb.js | 4 ++ 4 files changed, 79 insertions(+), 8 deletions(-) diff --git a/src/store/indexeddb-local-backend.js b/src/store/indexeddb-local-backend.js index b24bcbdbe..4df9dc0af 100644 --- a/src/store/indexeddb-local-backend.js +++ b/src/store/indexeddb-local-backend.js @@ -71,7 +71,7 @@ function selectQuery(store, keyRange, resultMapper) { }); } -function promiseifyTxn(txn) { +function txnAsPromise(txn) { return new Promise((resolve, reject) => { txn.oncomplete = function(event) { resolve(event); @@ -82,7 +82,7 @@ function promiseifyTxn(txn) { }); } -function promiseifyRequest(req) { +function reqAsEventPromise(req) { return new Promise((resolve, reject) => { req.onsuccess = function(event) { resolve(event); @@ -93,6 +93,17 @@ function promiseifyRequest(req) { }); } +function reqAsPromise(req) { + return new Promise((resolve, reject) => { + req.onsuccess = () => resolve(req); + req.onerror = (err) => reject(err); + }); +} + +function reqAsCursorPromise(req) { + return reqAsEventPromise(req).then((event) => event.target.result); +} + /** * Does the actual reading from and writing to the indexeddb * @@ -159,7 +170,7 @@ LocalIndexedDBStoreBackend.prototype = { console.log( `LocalIndexedDBStoreBackend.connect: awaiting connection...`, ); - return promiseifyRequest(req).then((ev) => { + return reqAsEventPromise(req).then((ev) => { console.log( `LocalIndexedDBStoreBackend.connect: connected`, ); @@ -265,7 +276,7 @@ LocalIndexedDBStoreBackend.prototype = { const tx = this.db.transaction(["oob_membership_events"], "readwrite"); const store = tx.objectStore("oob_membership_events"); const eventPuts = membershipEvents.map((e) => { - const putPromise = promiseifyRequest(store.put(e)); + const putPromise = reqAsEventPromise(store.put(e)); // ignoring the result makes sure we discard the IDB success event // ASAP, and not create a potentially big array containing them // unneccesarily later on by calling Promise.all. @@ -281,7 +292,7 @@ LocalIndexedDBStoreBackend.prototype = { oob_written: true, state_key: 0, }; - const markerPut = promiseifyRequest(store.put(markerObject)); + const markerPut = reqAsEventPromise(store.put(markerObject)); const allPuts = eventPuts.concat(markerPut); // ignore the empty array Promise.all creates // as this method should just resolve @@ -292,6 +303,55 @@ LocalIndexedDBStoreBackend.prototype = { }); }, + clearOutOfBandMembers: async function(roomId) { + // the approach to delete all members for a room + // is to get the min and max state key from the index + // for that room, and then delete between those + // keys in the store. + // this should be way faster than deleting every member + // individually for a large room. + const readTx = this.db.transaction( + ["oob_membership_events"], + "readonly"); + const store = readTx.objectStore("oob_membership_events"); + const roomIndex = store.index("room"); + const roomRange = IDBKeyRange.only(roomId); + + const indexCount = (await reqAsPromise(roomIndex.count(roomRange))).result; + + const minStateKeyProm = reqAsCursorPromise( + roomIndex.openKeyCursor(roomRange, "next"), + ).then((cursor) => cursor && cursor.primaryKey[1]); + const maxStateKeyProm = reqAsCursorPromise( + roomIndex.openKeyCursor(roomRange, "prev"), + ).then((cursor) => cursor && cursor.primaryKey[1]); + const [minStateKey, maxStateKey] = await Promise.all( + [minStateKeyProm, maxStateKeyProm]); + + const writeTx = this.db.transaction( + ["oob_membership_events"], + "readwrite"); + const writeStore = writeTx.objectStore("oob_membership_events"); + const membersKeyRange = IDBKeyRange.bound( + [roomId, minStateKey], + [roomId, maxStateKey], + ); + const count = + (await reqAsPromise(writeStore.count(membersKeyRange))).result; + + // Leaving this for now to make sure + if (count !== indexCount) { + console.error(`not deleting all members, ` + + `oob_membership_events and its index room ` + + `dont seem to have the same key order`); + } + + console.log(`LL: Deleting ${count} users + marker for ` + + `room ${roomId}, with key range:`, + [roomId, minStateKey], [roomId, maxStateKey]); + await reqAsPromise(writeStore.delete(membersKeyRange)); + }, + /** * Clear the entire database. This should be used when logging out of a client * to prevent mixing data between accounts. @@ -389,7 +449,7 @@ LocalIndexedDBStoreBackend.prototype = { roomsData: roomsData, groupsData: groupsData, }); // put == UPSERT - return promiseifyTxn(txn); + return txnAsPromise(txn); }); }, @@ -406,7 +466,7 @@ LocalIndexedDBStoreBackend.prototype = { for (let i = 0; i < accountData.length; i++) { store.put(accountData[i]); // put == UPSERT } - return promiseifyTxn(txn); + return txnAsPromise(txn); }); }, @@ -428,7 +488,7 @@ LocalIndexedDBStoreBackend.prototype = { event: tuple[1], }); // put == UPSERT } - return promiseifyTxn(txn); + return txnAsPromise(txn); }); }, diff --git a/src/store/indexeddb-remote-backend.js b/src/store/indexeddb-remote-backend.js index 7a58ded1b..85f07f86b 100644 --- a/src/store/indexeddb-remote-backend.js +++ b/src/store/indexeddb-remote-backend.js @@ -110,6 +110,10 @@ RemoteIndexedDBStoreBackend.prototype = { return this._doCmd('setOutOfBandMembers', [roomId, membershipEvents]); }, + clearOutOfBandMembers: function(roomId) { + return this._doCmd('clearOutOfBandMembers', [roomId]); + }, + /** * Load all user presence events from the database. This is not cached. * @return {Promise} A list of presence events in their raw form. diff --git a/src/store/indexeddb-store-worker.js b/src/store/indexeddb-store-worker.js index 021156afc..adfc3535b 100644 --- a/src/store/indexeddb-store-worker.js +++ b/src/store/indexeddb-store-worker.js @@ -95,6 +95,9 @@ class IndexedDBStoreWorker { case 'getOutOfBandMembers': prom = this.backend.getOutOfBandMembers(msg.args[0]); break; + case 'clearOutOfBandMembers': + prom = this.backend.clearOutOfBandMembers(msg.args[0]); + break; case 'setOutOfBandMembers': prom = this.backend.setOutOfBandMembers(msg.args[0], msg.args[1]); break; diff --git a/src/store/indexeddb.js b/src/store/indexeddb.js index 5e027537e..0de57c89a 100644 --- a/src/store/indexeddb.js +++ b/src/store/indexeddb.js @@ -242,4 +242,8 @@ IndexedDBStore.prototype.setOutOfBandMembers = function(roomId, membershipEvents return this.backend.setOutOfBandMembers(roomId, membershipEvents); }; +IndexedDBStore.prototype.clearOutOfBandMembers = function(roomId) { + return this.backend.clearOutOfBandMembers(roomId); +}; + module.exports.IndexedDBStore = IndexedDBStore; From 7258fe4e5c16ed889296dbe00d0d2694fdc4b98c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 31 Aug 2018 14:41:25 +0200 Subject: [PATCH 2/6] clear out of band members in store when leaving room --- src/models/room.js | 23 +++++++++++++++++++++++ src/store/stub.js | 4 ++++ src/sync.js | 1 + 3 files changed, 28 insertions(+) diff --git a/src/models/room.js b/src/models/room.js index 705e86bfc..2d253d64e 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -457,6 +457,29 @@ Room.prototype.loadMembersIfNeeded = function() { return this._membersPromise; }; +/** + * Removes the lazily loaded members from storage if needed + */ +Room.prototype.clearLoadedMembersIfNeeded = async function() { + if (this._opts.lazyLoadMembers && this._membersPromise) { + await this.loadMembersIfNeeded(); + this._membersPromise = null; + await this._client.store.clearOutOfBandMembers(this.roomId); + } +}; + +/** + * called when sync receives this room in the leave section + * to do cleanup after leaving a room. Possibly called multiple times. + */ +Room.prototype.onLeft = function() { + this.clearLoadedMembersIfNeeded().catch((err) => { + console.error(`error after clearing loaded members from ` + + `room ${this.roomId} after leaving`); + console.dir(err); + }); +}; + /** * Reset the live timeline of all timelineSets, and start new ones. * diff --git a/src/store/stub.js b/src/store/stub.js index 9c628b287..d0c2cabc5 100644 --- a/src/store/stub.js +++ b/src/store/stub.js @@ -272,6 +272,10 @@ StubStore.prototype = { setOutOfBandMembers: function() { return Promise.resolve(); }, + + clearOutOfBandMembers: function() { + return Promise.resolve(); + }, }; /** Stub Store class. */ diff --git a/src/sync.js b/src/sync.js index 4aa2d2482..b500ff316 100644 --- a/src/sync.js +++ b/src/sync.js @@ -1106,6 +1106,7 @@ SyncApi.prototype._processSyncResponse = async function( leaveRooms.forEach(function(leaveObj) { const room = leaveObj.room; room.setSyncedMembership("leave"); + room.onLeft(); const stateEvents = self._mapSyncEventsFormat(leaveObj.state, room); From 9b1926f902d751172d505eec5946e872b4288d2c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 31 Aug 2018 16:11:37 +0200 Subject: [PATCH 3/6] also clear out lazy loaded members from storage --- src/models/room-state.js | 16 ++++++++++++++++ src/models/room.js | 1 + 2 files changed, 17 insertions(+) diff --git a/src/models/room-state.js b/src/models/room-state.js index fb049daa9..bb8e93fb9 100644 --- a/src/models/room-state.js +++ b/src/models/room-state.js @@ -443,6 +443,22 @@ RoomState.prototype.markOutOfBandMembersFailed = function() { this._oobMemberFlags.status = OOB_STATUS_NOTSTARTED; }; +/** + * Clears the loaded out-of-band members + */ +RoomState.prototype.clearOutOfBandMembers = function() { + let count = 0; + Object.keys(this.members).forEach((userId) => { + const member = this.members[userId]; + if (member.isOutOfBand()) { + ++count; + delete this.members[userId]; + } + }); + console.log(`LL: RoomState removed ${count} members...`); + this._oobMemberFlags.status = OOB_STATUS_NOTSTARTED; +}; + /** * Sets the loaded out-of-band members. * @param {MatrixEvent[]} stateEvents array of membership state events diff --git a/src/models/room.js b/src/models/room.js index 2d253d64e..552806238 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -465,6 +465,7 @@ Room.prototype.clearLoadedMembersIfNeeded = async function() { await this.loadMembersIfNeeded(); this._membersPromise = null; await this._client.store.clearOutOfBandMembers(this.roomId); + this.currentState.clearOutOfBandMembers(); } }; From f30136dba3978ec5adaadafa0435373b0683811b Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 31 Aug 2018 16:12:54 +0200 Subject: [PATCH 4/6] only clear promise at the end to avoid race between load and clear members --- src/models/room.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/models/room.js b/src/models/room.js index 552806238..19f4a7df6 100644 --- a/src/models/room.js +++ b/src/models/room.js @@ -463,9 +463,9 @@ Room.prototype.loadMembersIfNeeded = function() { Room.prototype.clearLoadedMembersIfNeeded = async function() { if (this._opts.lazyLoadMembers && this._membersPromise) { await this.loadMembersIfNeeded(); - this._membersPromise = null; await this._client.store.clearOutOfBandMembers(this.roomId); this.currentState.clearOutOfBandMembers(); + this._membersPromise = null; } }; From ebc162e3d88ded04c6ed2ae206ea76132917c9b3 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Fri, 31 Aug 2018 16:13:34 +0200 Subject: [PATCH 5/6] do onLeft (which clears the LL members) as late as possible to avoid chance that something might call loadMembersIfNeeded on the room and load them back again. --- src/sync.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sync.js b/src/sync.js index b500ff316..dda0984e7 100644 --- a/src/sync.js +++ b/src/sync.js @@ -1106,7 +1106,6 @@ SyncApi.prototype._processSyncResponse = async function( leaveRooms.forEach(function(leaveObj) { const room = leaveObj.room; room.setSyncedMembership("leave"); - room.onLeft(); const stateEvents = self._mapSyncEventsFormat(leaveObj.state, room); @@ -1135,6 +1134,8 @@ SyncApi.prototype._processSyncResponse = async function( accountDataEvents.forEach(function(e) { client.emit("event", e); }); + + room.onLeft(); }); // update the notification timeline, if appropriate. From 3bed5969bfa8df7a22abb58e1868c5ed39d50adb Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Mon, 3 Sep 2018 10:27:00 +0200 Subject: [PATCH 6/6] remove count logging, approach confirmed to work and be according to idb spec --- src/store/indexeddb-local-backend.js | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/store/indexeddb-local-backend.js b/src/store/indexeddb-local-backend.js index 4df9dc0af..4bcbbe6e2 100644 --- a/src/store/indexeddb-local-backend.js +++ b/src/store/indexeddb-local-backend.js @@ -317,8 +317,6 @@ LocalIndexedDBStoreBackend.prototype = { const roomIndex = store.index("room"); const roomRange = IDBKeyRange.only(roomId); - const indexCount = (await reqAsPromise(roomIndex.count(roomRange))).result; - const minStateKeyProm = reqAsCursorPromise( roomIndex.openKeyCursor(roomRange, "next"), ).then((cursor) => cursor && cursor.primaryKey[1]); @@ -336,17 +334,8 @@ LocalIndexedDBStoreBackend.prototype = { [roomId, minStateKey], [roomId, maxStateKey], ); - const count = - (await reqAsPromise(writeStore.count(membersKeyRange))).result; - // Leaving this for now to make sure - if (count !== indexCount) { - console.error(`not deleting all members, ` + - `oob_membership_events and its index room ` + - `dont seem to have the same key order`); - } - - console.log(`LL: Deleting ${count} users + marker for ` + + console.log(`LL: Deleting all users + marker in storage for ` + `room ${roomId}, with key range:`, [roomId, minStateKey], [roomId, maxStateKey]); await reqAsPromise(writeStore.delete(membersKeyRange));