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

@@ -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,19 +802,9 @@ 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`);
-        }
-    }
 };
 
 /**

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;
+                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}!`);
         });

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]);
     },
 
     /**

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;