Merge pull request #2991 from S7evinK/s7evink/slidingsync-unsub

sliding sync: Fix issue where no unsubs are sent when switching rooms

spec/integ/sliding-sync.spec.ts

@@ -1418,6 +1418,102 @@ describe("SlidingSync", () => {
             await httpBackend!.flushAllExpected();
             slidingSync.stop();
         });
+
+        it("should not be possible to add/modify an already added custom subscription", async () => {
+            const slidingSync = new SlidingSync(proxyBaseUrl, [], defaultSub, client!, 1);
+            slidingSync.addCustomSubscription(customSubName1, customSub1);
+            slidingSync.addCustomSubscription(customSubName1, customSub2);
+            slidingSync.useCustomSubscription(roomA, customSubName1);
+            slidingSync.modifyRoomSubscriptions(new Set<string>([roomA]));
+
+            httpBackend!
+                .when("POST", syncUrl)
+                .check(function (req) {
+                    const body = req.data;
+                    logger.log("custom subs", body);
+                    expect(body.room_subscriptions).toBeTruthy();
+                    expect(body.room_subscriptions[roomA]).toEqual(customSub1);
+                })
+                .respond(200, {
+                    pos: "b",
+                    lists: [],
+                    extensions: {},
+                    rooms: {},
+                });
+            slidingSync.start();
+            await httpBackend!.flushAllExpected();
+            slidingSync.stop();
+        });
+
+        it("should change the custom subscription if they are different", async () => {
+            const slidingSync = new SlidingSync(proxyBaseUrl, [], defaultSub, client!, 1);
+            slidingSync.addCustomSubscription(customSubName1, customSub1);
+            slidingSync.addCustomSubscription(customSubName2, customSub2);
+            slidingSync.useCustomSubscription(roomA, customSubName1);
+            slidingSync.modifyRoomSubscriptions(new Set<string>([roomA]));
+
+            httpBackend!
+                .when("POST", syncUrl)
+                .check(function (req) {
+                    const body = req.data;
+                    logger.log("custom subs", body);
+                    expect(body.room_subscriptions).toBeTruthy();
+                    expect(body.room_subscriptions[roomA]).toEqual(customSub1);
+                    expect(body.unsubscribe_rooms).toBeUndefined();
+                })
+                .respond(200, {
+                    pos: "b",
+                    lists: [],
+                    extensions: {},
+                    rooms: {},
+                });
+            slidingSync.start();
+            await httpBackend!.flushAllExpected();
+
+            // using the same subscription doesn't unsub nor changes subscriptions
+            slidingSync.useCustomSubscription(roomA, customSubName1);
+            slidingSync.modifyRoomSubscriptions(new Set<string>([roomA]));
+
+            httpBackend!
+                .when("POST", syncUrl)
+                .check(function (req) {
+                    const body = req.data;
+                    logger.log("custom subs", body);
+                    expect(body.room_subscriptions).toBeUndefined();
+                    expect(body.unsubscribe_rooms).toBeUndefined();
+                })
+                .respond(200, {
+                    pos: "b",
+                    lists: [],
+                    extensions: {},
+                    rooms: {},
+                });
+            slidingSync.start();
+            await httpBackend!.flushAllExpected();
+
+            // Changing the subscription works
+            slidingSync.useCustomSubscription(roomA, customSubName2);
+            slidingSync.modifyRoomSubscriptions(new Set<string>([roomA]));
+
+            httpBackend!
+                .when("POST", syncUrl)
+                .check(function (req) {
+                    const body = req.data;
+                    logger.log("custom subs", body);
+                    expect(body.room_subscriptions).toBeTruthy();
+                    expect(body.room_subscriptions[roomA]).toEqual(customSub2);
+                    expect(body.unsubscribe_rooms).toBeUndefined();
+                })
+                .respond(200, {
+                    pos: "b",
+                    lists: [],
+                    extensions: {},
+                    rooms: {},
+                });
+            slidingSync.start();
+            await httpBackend!.flushAllExpected();
+            slidingSync.stop();
+        });
     });
 
     describe("extensions", () => {

src/sliding-sync.ts

@@ -397,6 +397,10 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
      * @param sub - The subscription information.
      */
     public addCustomSubscription(name: string, sub: MSC3575RoomSubscription): void {
+        if (this.customSubscriptions.has(name)) {
+            logger.warn(`addCustomSubscription: ${name} already exists as a custom subscription, ignoring.`);
+            return;
+        }
         this.customSubscriptions.set(name, sub);
     }
 
@@ -408,6 +412,11 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
      * will be used.
      */
     public useCustomSubscription(roomId: string, name: string): void {
+        // We already know about this custom subscription, as it is immutable,
+        // we don't need to unconfirm the subscription.
+        if (this.roomIdToCustomSubscription.get(roomId) === name) {
+            return;
+        }
         this.roomIdToCustomSubscription.set(roomId, name);
         // unconfirm this subscription so a resend() will send it up afresh.
         this.confirmedRoomSubscriptions.delete(roomId);