Move /members fetching to room as getEncryptionTargetMembers needs it

spec/unit/room.spec.js

@@ -469,7 +469,7 @@ describe("Room", function() {
 
     describe("compareEventOrdering", function() {
         beforeEach(function() {
-            room = new Room(roomId, null, {timelineSupport: true});
+            room = new Room(roomId, null, null, null, {timelineSupport: true});
         });
 
         const events = [

src/client.js

@@ -769,56 +769,6 @@ MatrixClient.prototype.getRoom = function(roomId) {
     return this.store.getRoom(roomId);
 };
 
-MatrixClient.prototype._loadMembers = async function(room) {
-    const roomId = room.roomId;
-    // were the members loaded from the server?
-    let fromServer = false;
-    let rawMembersEvents = await this.store.getOutOfBandMembers(roomId);
-    if (rawMembersEvents === null) {
-        fromServer = true;
-        const lastEventId = room.getLastEventId();
-        const response = await this.members(roomId, "join", "leave", lastEventId);
-        rawMembersEvents = response.chunk;
-        console.log(`LL: got ${rawMembersEvents.length} members from server`);
-    }
-    const memberEvents = rawMembersEvents.map(this.getEventMapper());
-    return {memberEvents, fromServer};
-};
-
-/**
- * Preloads the member list for the given room id,
- * in case lazy loading of memberships is in use.
- * @param {string} roomId The room ID
- */
-MatrixClient.prototype.loadRoomMembersIfNeeded = async function(roomId) {
-    const room = this.getRoom(roomId);
-    if (!room || !room.needsOutOfBandMembers()) {
-        return;
-    }
-    // intercept whether we need to store oob members afterwards
-    let membersNeedStoring = false;
-    // Note that we don't await _loadMembers here first.
-    // setLazyLoadedMembers sets a flag before it awaits the promise passed in
-    // to avoid a race when calling membersNeedLoading/loadOutOfBandMembers
-    // in fast succession, before the first promise resolves.
-    const membersPromise = this._loadMembers(room)
-        .then(({memberEvents, fromServer}) => {
-            membersNeedStoring = fromServer;
-            return memberEvents;
-        });
-    await room.loadOutOfBandMembers(membersPromise);
-    // if loadOutOfBandMembers throws, this wont be called
-    // but that's fine as we don't want to store members
-    // that caused an error.
-    if (membersNeedStoring) {
-        const rawMembersEvents = room.currentState.getMembers()
-            .filter((m) => m.isOutOfBand())
-            .map((m) => m.events.member.event);
-        console.log(`LL: telling backend to store ${rawMembersEvents.length} members`);
-        await this.store.setOutOfBandMembers(roomId, rawMembersEvents);
-    }
-};
-
 /**
  * Retrieve all known rooms.
  * @return {Room[]} A list of rooms, or an empty list if there is no data store.

src/models/room.js

@@ -179,6 +179,11 @@ function Room(roomId, myUserId, opts) {
     this._oobMembersPromise = null;
     if (this._opts.lazyLoadMembers) {
         this._oobMembersPromise = new utils.Deferred();
+    this._client = client;
+    if (!this._opts.lazyLoadMembers) {
+        this._membersPromise = Promise.resolve();
+    } else {
+        this._membersPromise = null;
     }
 }
 
@@ -275,41 +280,79 @@ Room.prototype.setSyncedMembership = function(membership) {
     this._syncedMembership = membership;
 };
 
-/**
- * Get the out-of-band members loading state, whether loading is needed or not.
- * Note that loading might be in progress and hence isn't needed.
- * @return {bool} whether or not the members of this room need to be loaded
- */
-Room.prototype.needsOutOfBandMembers = function() {
-    return this._opts.lazyLoadMembers &&
-        this.currentState.needsOutOfBandMembers();
+Room.prototype._loadMembersFromServer = async function() {
+    const queryString = utils.encodeParams({
+        membership: "join",
+        not_membership: "leave",
+        at: this.getLastEventId(),
+    });
+    const path = utils.encodeUri("/rooms/$roomId/members?" + queryString,
+        {$roomId: this.roomId});
+    const http = this._client._http;
+    const response = await http.authedRequest(callback, "GET", path);
+    return response.chunk;
+};
+
+
+Room.prototype._loadMembers = async function() {
+    // were the members loaded from the server?
+    let fromServer = false;
+    let rawMembersEvents = 
+        await this._client.store.getOutOfBandMembers(this.roomId);
+    if (rawMembersEvents === null) {
+        fromServer = true;
+        rawMembersEvents = await this._loadMembersFromServer();
+        console.log(`LL: got ${rawMembersEvents.length}` +
+            `members from server for room ${this.roomId}`);
+    }
+    const memberEvents = rawMembersEvents.map(this._client.getEventMapper());
+    return {memberEvents, fromServer};
 };
 
 /**
- * Loads the out-of-band members from the promise passed in
- * @param {Promise} eventsPromise promise that resolves to an array with membership MatrixEvents for the members
+ * Preloads the member list in case lazy loading
+ * of memberships is in use. Can be called multiple times,
+ * it will only preload once.
+ * @return {Promise} when preloading is done and
+ * accessing the members on the room will take
+ * all members in the room into account
  */
-Room.prototype.loadOutOfBandMembers = function(eventsPromise) {
-    if (!this.needsOutOfBandMembers()) {
-        return Promise.resolve();
+Room.prototype.loadMembersIfNeeded = function() {
+    if (this._membersPromise) {
+        return this._membersPromise;
     }
+
+    // mark the state so that incoming messages while
+    // the request is in flight get marked as superseding
+    // the OOB members
     this.currentState.markOutOfBandMembersStarted();
 
-    // store the promise that already updated the room state
-    // to ensure that happens first
-    const updatedRoomStatePromise = eventsPromise.then((events) => {
-        this.currentState.setOutOfBandMembers(events);
-    }, (err) => {
-        this.currentState.markOutOfBandMembersFailed();
-        throw err;  //rethrow so calling code is aware operation failed
+    const promise = this._loadMembers().then(({memberEvents, fromServer}) => {
+        this.currentState.setOutOfBandMembers(memberEvents);
+        if (fromServer) {
+            const oobMembers = this.currentState.getMembers()
+                .filter((m) => m.isOutOfBand())
+                .map((m) => m.events.member.event);
+            console.log(`LL: telling store to write ${oobEvents.length}`
+                + ` members for room ${this.roomId}`);
+            const store = this._client.store;
+            return store.setOutOfBandMembers(roomId, oobMembers)
+                // swallow any IDB error as we don't want to fail
+                // because of this
+                .catch((err) => {
+                    console.log("LL: storing OOB room members failed, oh well",
+                        err);
                 });
-    // resolve the Deferred with the pending updated room promise,
-    // this will signal OOB members are now available to
-    // dependant code like getEncryptionTargetMembers
-    if (this._oobMembersPromise) {
-        this._oobMembersPromise.resolve(updatedRoomStatePromise);
         }
-    return updatedRoomStatePromise;
+    }).catch((err) => {
+        // allow retries on fail
+        this._membersPromise = null;
+        this.currentState.markOutOfBandMembersFailed()
+        throw err;
+    });
+
+    this._membersPromise = promise;
+    return this._membersPromise;
 };
 
 /**

src/sync.js

@@ -113,7 +113,7 @@ function SyncApi(client, opts) {
  */
 SyncApi.prototype.createRoom = function(roomId) {
     const client = this.client;
-    const room = new Room(roomId, client.getUserId(), {
+    const room = new Room(roomId, client, client.getUserId(), {
         lazyLoadMembers: this.opts.lazyLoadMembers,
         pendingEventOrdering: this.opts.pendingEventOrdering,
         timelineSupport: client.timelineSupport,