From 5e11bf735e498f093812e472ef8dd2cda4cebfaa Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 31 Jul 2018 16:56:18 +0200 Subject: [PATCH] store OOB status along with members, to avoid unneccesary fetching for some small rooms, it is possible that calling /members would not yield any previously unknown members, as they were all recently active. This would be the case for most DMs. For these rooms, we'd end up with 0 OOB members after lazy loading them, so when getting them out of storage we need a way to distuinguist this case from never having lazy loaded the members of the room at all. We store a marker object in the same store and return [] or null accordingly. This way the /members don't get fetched a second time. --- src/client.js | 16 ++----- src/store/indexeddb-local-backend.js | 67 +++++++++++++++++++++++---- src/store/indexeddb-remote-backend.js | 19 +++++++- src/store/indexeddb.js | 19 +++++++- 4 files changed, 94 insertions(+), 27 deletions(-) diff --git a/src/client.js b/src/client.js index 977755734..9365b54a0 100644 --- a/src/client.js +++ b/src/client.js @@ -762,7 +762,7 @@ MatrixClient.prototype._loadMembers = async function(room) { // were the members loaded from the server? let fromServer = false; let rawMembersEvents = await this.store.getOutOfBandMembers(roomId); - if (rawMembersEvents.length == 0) { + if (rawMembersEvents === null) { fromServer = true; const lastEventId = room.getLastEventId(); const response = await this.members(roomId, "join", "leave", lastEventId); @@ -802,18 +802,8 @@ MatrixClient.prototype.loadRoomMembersIfNeeded = async function(roomId) { const rawMembersEvents = room.currentState.getMembers() .filter((m) => m.isOutOfBand()) .map((m) => m.events.member.event); - // TODO: probably need a way to mark a room as lazy loaded - // even though we didn't store any members, as we'll just - // lazy loaded the room in every session. This is a likely - // scenario for DM's where all the members would likely - // be known without lazy loading. - if (rawMembersEvents.length) { - console.log(`LL: telling backend to store ${rawMembersEvents.length} members`); - await this.store.setOutOfBandMembers(roomId, rawMembersEvents); - } - else { - console.log(`LL: no members needed to be stored`); - } + console.log(`LL: telling backend to store ${rawMembersEvents.length} members`); + await this.store.setOutOfBandMembers(roomId, rawMembersEvents); } }; diff --git a/src/store/indexeddb-local-backend.js b/src/store/indexeddb-local-backend.js index 1cca9f3e2..2b97e0553 100644 --- a/src/store/indexeddb-local-backend.js +++ b/src/store/indexeddb-local-backend.js @@ -34,7 +34,10 @@ function createDatabase(db) { } function upgradeSchemaV2(db) { - const oobMembersStore = db.createObjectStore("oob_membership_events", { keyPath: ["room_id", "state_key"]}); + const oobMembersStore = db.createObjectStore( + "oob_membership_events", { + keyPath: ["room_id", "state_key"], + }); oobMembersStore.createIndex("room", "room_id"); } @@ -195,6 +198,13 @@ LocalIndexedDBStoreBackend.prototype = { }); }, + /** + * Returns the out-of-band membership events for this room that + * were previously loaded. + * @param {string} roomId + * @returns {event[]} the events, potentially an empty array if OOB loading didn't yield any new members + * @returns {null} in case the members for this room haven't been stored yet + */ getOutOfBandMembers: function(roomId) { return new Promise((resolve, reject) =>{ const tx = this.db.transaction(["oob_membership_events"], "readonly"); @@ -204,36 +214,73 @@ LocalIndexedDBStoreBackend.prototype = { const request = roomIndex.openCursor(range); const membershipEvents = []; + // did we encounter the oob_written marker object + // amongst the results? That means OOB member + // loading already happened for this room + // but there were no members to persist as they + // were all known already + let oobWritten = false; + request.onsuccess = (event) => { const cursor = event.target.result; if (!cursor) { + // Unknown room + if (!membershipEvents.length && !oobWritten) { + return resolve(null); + } return resolve(membershipEvents); } const record = cursor.value; - membershipEvents.push(record); + if (record.oob_written) { + oobWritten = true; + } else { + membershipEvents.push(record); + } cursor.continue(); }; request.onerror = (err) => { reject(err); }; - }).then((membershipEvents) => { - console.log(`LL: got ${membershipEvents.length} membershipEvents from storage for room ${roomId} ...`); - return membershipEvents; + }).then((events) => { + console.log(`LL: got ${events && events.length}` + + ` membershipEvents from storage for room ${roomId} ...`); + return events; }); }, + /** + * Stores the out-of-band membership events for this room. Note that + * it still makes sense to store an empty array as the OOB status for the room is + * marked as fetched, and getOutOfBandMembers will return an empty array instead of null + * @param {string} roomId + * @param {event[]} membershipEvents the membership events to store + * @returns {Promise} when all members have been stored + */ setOutOfBandMembers: function(roomId, membershipEvents) { - console.log(`LL: backend about to store ${membershipEvents.length} members for ${roomId}`); - function ignoreResult() {}; + console.log(`LL: backend about to store ${membershipEvents.length}` + + ` members for ${roomId}`); + function ignoreResult() {} // run everything in a promise so anything that throws will reject return new Promise((resolve) =>{ const tx = this.db.transaction(["oob_membership_events"], "readwrite"); const store = tx.objectStore("oob_membership_events"); - const puts = membershipEvents.map((e) => { - let putPromise = promiseifyRequest(store.put(e)); + const eventPuts = membershipEvents.map((e) => { + const putPromise = promiseifyRequest(store.put(e)); return putPromise.then(ignoreResult); }); - resolve(Promise.all(puts).then(ignoreResult)); + // aside from all the events, we also write a marker object to the store + // to mark the fact that OOB members have been written for this room. + // It's possible that 0 members need to be written as all where previously know + // but we still need to know whether to return null or [] from getOutOfBandMembers + // where null means out of band members haven't been stored yet for this room + const markerObject = { + room_id: roomId, + oob_written: true, + state_key: 0, + }; + const markerPut = promiseifyRequest(store.put(markerObject)); + const allPuts = eventPuts.concat(markerPut); + resolve(Promise.all(allPuts).then(ignoreResult)); }).then(() => { console.log(`LL: backend done storing for ${roomId}!`); }); diff --git a/src/store/indexeddb-remote-backend.js b/src/store/indexeddb-remote-backend.js index 1594eef26..7a58ded1b 100644 --- a/src/store/indexeddb-remote-backend.js +++ b/src/store/indexeddb-remote-backend.js @@ -87,12 +87,27 @@ RemoteIndexedDBStoreBackend.prototype = { return this._doCmd('syncToDatabase', [users]); }, + /** + * Returns the out-of-band membership events for this room that + * were previously loaded. + * @param {string} roomId + * @returns {event[]} the events, potentially an empty array if OOB loading didn't yield any new members + * @returns {null} in case the members for this room haven't been stored yet + */ getOutOfBandMembers: function(roomId) { return this._doCmd('getOutOfBandMembers', [roomId]); }, - setOutOfBandMembers: function(roomId, members) { - return this._doCmd('setOutOfBandMembers', [roomId, members]); + /** + * Stores the out-of-band membership events for this room. Note that + * it still makes sense to store an empty array as the OOB status for the room is + * marked as fetched, and getOutOfBandMembers will return an empty array instead of null + * @param {string} roomId + * @param {event[]} membershipEvents the membership events to store + * @returns {Promise} when all members have been stored + */ + setOutOfBandMembers: function(roomId, membershipEvents) { + return this._doCmd('setOutOfBandMembers', [roomId, membershipEvents]); }, /** diff --git a/src/store/indexeddb.js b/src/store/indexeddb.js index 0086eb531..5e027537e 100644 --- a/src/store/indexeddb.js +++ b/src/store/indexeddb.js @@ -219,12 +219,27 @@ IndexedDBStore.prototype.setSyncData = function(syncData) { return this.backend.setSyncData(syncData); }; +/** + * Returns the out-of-band membership events for this room that + * were previously loaded. + * @param {string} roomId + * @returns {event[]} the events, potentially an empty array if OOB loading didn't yield any new members + * @returns {null} in case the members for this room haven't been stored yet + */ IndexedDBStore.prototype.getOutOfBandMembers = function(roomId) { return this.backend.getOutOfBandMembers(roomId); }; -IndexedDBStore.prototype.setOutOfBandMembers = function(roomId, members) { - return this.backend.setOutOfBandMembers(roomId, members); +/** + * Stores the out-of-band membership events for this room. Note that + * it still makes sense to store an empty array as the OOB status for the room is + * marked as fetched, and getOutOfBandMembers will return an empty array instead of null + * @param {string} roomId + * @param {event[]} membershipEvents the membership events to store + * @returns {Promise} when all members have been stored + */ +IndexedDBStore.prototype.setOutOfBandMembers = function(roomId, membershipEvents) { + return this.backend.setOutOfBandMembers(roomId, membershipEvents); }; module.exports.IndexedDBStore = IndexedDBStore;