Skip to content

Commit 88d066a

Browse files
MidhunSureshRMidhunSureshR
authored
Fix reemitter not being correctly wired on user objects created in storage classes (#3796)
* Fix issue * Fix jest test * Fix even more jest failures * Fix formatting * Add a test * Write test for older code * Fix lint * Rename method * Make ctor deprecated
1 parent ce7b7bf commit 88d066a

11 files changed

+144
-22
lines changed

‎spec/integ/matrix-client-event-emitter.spec.ts

+31
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,37 @@ describe("MatrixClient events", function () {
194194
expect(fired).toBe(true);
195195
});
196196

197+
it("should emit User events when presence data is absent in first sync", async () => {
198+
const MODIFIED_SYNC_DATA: any = structuredClone(SYNC_DATA);
199+
delete MODIFIED_SYNC_DATA["presence"];
200+
const MODIFIED_NEXT_SYNC_DATA: any = structuredClone(NEXT_SYNC_DATA);
201+
MODIFIED_NEXT_SYNC_DATA.presence = {
202+
events: [
203+
utils.mkPresence({
204+
user: "@foo:bar",
205+
name: "Foo Bar",
206+
presence: "online",
207+
}),
208+
],
209+
};
210+
httpBackend!.when("GET", "/sync").respond(200, MODIFIED_SYNC_DATA);
211+
httpBackend!.when("GET", "/sync").respond(200, MODIFIED_NEXT_SYNC_DATA);
212+
let fired = false;
213+
client!.on(UserEvent.Presence, function (event, user) {
214+
fired = true;
215+
expect(user).toBeTruthy();
216+
expect(event).toBeTruthy();
217+
if (!user || !event) {
218+
return;
219+
}
220+
expect(event.event).toEqual(MODIFIED_NEXT_SYNC_DATA.presence.events[0]);
221+
expect(user.presence).toEqual(MODIFIED_NEXT_SYNC_DATA.presence.events[0]?.content?.presence);
222+
});
223+
client!.startClient();
224+
await httpBackend!.flushAllExpected();
225+
expect(fired).toBe(true);
226+
});
227+
197228
it("should emit Room events", function () {
198229
httpBackend!.when("GET", "/sync").respond(200, SYNC_DATA);
199230
httpBackend!.when("GET", "/sync").respond(200, NEXT_SYNC_DATA);

‎spec/unit/event-mapper.spec.ts

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ describe("eventMapperFor", function () {
3434
getRoom(roomId: string): Room | null {
3535
return rooms.find((r) => r.roomId === roomId) ?? null;
3636
},
37+
setUserCreator(_) {},
3738
} as IStore,
3839
scheduler: {
3940
setProcessFunction: jest.fn(),

‎spec/unit/matrix-client.spec.ts

+1
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ describe("MatrixClient", function () {
328328
"storeFilter",
329329
"startup",
330330
"deleteAllData",
331+
"setUserCreator",
331332
] as const
332333
).reduce((r, k) => {
333334
r[k] = jest.fn();

‎spec/unit/stores/indexeddb.spec.ts

+58-1
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ limitations under the License.
1616

1717
import "fake-indexeddb/auto";
1818
import "jest-localstorage-mock";
19+
import { IDBFactory } from "fake-indexeddb";
1920

20-
import { IndexedDBStore, IStateEventWithRoomId, MemoryStore } from "../../../src";
21+
import { IndexedDBStore, IStateEventWithRoomId, MemoryStore, User, UserEvent } from "../../../src";
2122
import { emitPromise } from "../../test-utils/test-utils";
2223
import { LocalIndexedDBStoreBackend } from "../../../src/store/indexeddb-local-backend";
2324
import { defer } from "../../../src/utils";
@@ -76,6 +77,62 @@ describe("IndexedDBStore", () => {
7677
expect(await store.getOutOfBandMembers(roomId)).toHaveLength(2);
7778
});
7879

80+
it("Should load presence events on startup", async () => {
81+
// 1. Create idb database
82+
const indexedDB = new IDBFactory();
83+
const setupDefer = defer<Event>();
84+
const req = indexedDB.open("matrix-js-sdk:db3", 1);
85+
let db: IDBDatabase;
86+
req.onupgradeneeded = () => {
87+
db = req.result;
88+
db.createObjectStore("users", { keyPath: ["userId"] });
89+
db.createObjectStore("accountData", { keyPath: ["type"] });
90+
db.createObjectStore("sync", { keyPath: ["clobber"] });
91+
};
92+
req.onsuccess = setupDefer.resolve;
93+
await setupDefer.promise;
94+
95+
// 2. Fill in user presence data
96+
const writeDefer = defer<Event>();
97+
const transaction = db!.transaction(["users"], "readwrite");
98+
const objectStore = transaction.objectStore("users");
99+
const request = objectStore.put({
100+
userId: "@alice:matrix.org",
101+
event: {
102+
content: {
103+
presence: "online",
104+
},
105+
sender: "@alice:matrix.org",
106+
type: "m.presence",
107+
},
108+
});
109+
request.onsuccess = writeDefer.resolve;
110+
await writeDefer.promise;
111+
112+
// 3. Close database
113+
req.result.close();
114+
115+
// 2. Check if the code loads presence events
116+
const store = new IndexedDBStore({
117+
indexedDB: indexedDB,
118+
dbName: "db3",
119+
localStorage,
120+
});
121+
let userCreated = false;
122+
let presenceEventEmitted = false;
123+
store.setUserCreator((id: string) => {
124+
userCreated = true;
125+
const user = new User(id);
126+
user.on(UserEvent.Presence, () => {
127+
presenceEventEmitted = true;
128+
});
129+
return user;
130+
});
131+
await store.startup();
132+
expect(userCreated).toBe(true);
133+
expect(presenceEventEmitted).toBe(true);
134+
});
135+
79136
it("should use MemoryStore methods for pending events if no localStorage", async () => {
80137
jest.spyOn(MemoryStore.prototype, "setPendingEvents");
81138
jest.spyOn(MemoryStore.prototype, "getPendingEvents");

‎src/client.ts

+1
Original file line numberDiff line numberDiff line change
@@ -1342,6 +1342,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
13421342

13431343
this.usingExternalCrypto = opts.usingExternalCrypto ?? false;
13441344
this.store = opts.store || new StubStore();
1345+
this.store.setUserCreator((userId) => User.createUser(userId, this));
13451346
this.deviceId = opts.deviceId || null;
13461347
this.sessionId = randomString(10);
13471348

‎src/models/user.ts

+21
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17+
import { MatrixClient } from "../matrix";
1718
import { MatrixEvent } from "./event";
1819
import { TypedEventEmitter } from "./typed-event-emitter";
1920

@@ -152,6 +153,7 @@ export class User extends TypedEventEmitter<UserEvent, UserEventHandlerMap> {
152153
/**
153154
* Construct a new User. A User must have an ID and can optionally have extra information associated with it.
154155
* @param userId - Required. The ID of this user.
156+
* @deprecated use `User.createUser`
155157
*/
156158
public constructor(public readonly userId: string) {
157159
super();
@@ -160,6 +162,25 @@ export class User extends TypedEventEmitter<UserEvent, UserEventHandlerMap> {
160162
this.updateModifiedTime();
161163
}
162164

165+
/**
166+
* Construct a new User whose events will also emit on MatrixClient.
167+
* A User must have an ID and can optionally have extra information associated with it.
168+
* @param userId - Required. The ID of this user.
169+
* @param client - An instance of MatrixClient object
170+
* @returns User object with reEmitter setup on client
171+
*/
172+
public static createUser(userId: string, client: MatrixClient): User {
173+
const user = new User(userId);
174+
client.reEmitter.reEmit(user, [
175+
UserEvent.AvatarUrl,
176+
UserEvent.DisplayName,
177+
UserEvent.Presence,
178+
UserEvent.CurrentlyActive,
179+
UserEvent.LastPresenceTs,
180+
]);
181+
return user;
182+
}
183+
163184
/**
164185
* Update this User with the given presence event. May fire "User.presence",
165186
* "User.avatarUrl" and/or "User.displayName" if this event updates this user's

‎src/store/index.ts

+8
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ export interface ISavedSync {
3232
accountData: IMinimalEvent[];
3333
}
3434

35+
export type UserCreator = (userId: string) => User;
36+
3537
/**
3638
* A store for most of the data js-sdk needs to store, apart from crypto data
3739
*/
@@ -62,6 +64,12 @@ export interface IStore {
6264
*/
6365
storeRoom(room: Room): void;
6466

67+
/**
68+
* Set the user creator which is used for creating User objects
69+
* @param creator - A callback that accepts an user-id and returns an User object
70+
*/
71+
setUserCreator(creator: UserCreator): void;
72+
6573
/**
6674
* Retrieve a room by its' room ID.
6775
* @param roomId - The room ID.

‎src/store/indexeddb.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ limitations under the License.
1919
import { MemoryStore, IOpts as IBaseOpts } from "./memory";
2020
import { LocalIndexedDBStoreBackend } from "./indexeddb-local-backend";
2121
import { RemoteIndexedDBStoreBackend } from "./indexeddb-remote-backend";
22-
import { User } from "../models/user";
2322
import { IEvent, MatrixEvent } from "../models/event";
2423
import { logger } from "../logger";
2524
import { ISavedSync } from "./index";
@@ -140,7 +139,10 @@ export class IndexedDBStore extends MemoryStore {
140139
.then((userPresenceEvents) => {
141140
logger.log(`IndexedDBStore.startup: processing presence events`);
142141
userPresenceEvents.forEach(([userId, rawEvent]) => {
143-
const u = new User(userId);
142+
if (!this.createUser) {
143+
throw new Error("createUser is undefined, it should be set with setUserCreator()!");
144+
}
145+
const u = this.createUser(userId);
144146
if (rawEvent) {
145147
u.setPresenceEvent(new MatrixEvent(rawEvent));
146148
}

‎src/store/memory.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { IEvent, MatrixEvent } from "../models/event";
2525
import { RoomState, RoomStateEvent } from "../models/room-state";
2626
import { RoomMember } from "../models/room-member";
2727
import { Filter } from "../filter";
28-
import { ISavedSync, IStore } from "./index";
28+
import { ISavedSync, IStore, UserCreator } from "./index";
2929
import { RoomSummary } from "../models/room-summary";
3030
import { ISyncResponse } from "../sync-accumulator";
3131
import { IStateEventWithRoomId } from "../@types/search";
@@ -63,6 +63,7 @@ export class MemoryStore implements IStore {
6363
private clientOptions?: IStoredClientOpts;
6464
private pendingToDeviceBatches: IndexedToDeviceBatch[] = [];
6565
private nextToDeviceBatchId = 0;
66+
protected createUser?: UserCreator;
6667

6768
/**
6869
* Construct a new in-memory data store for the Matrix Client.
@@ -108,6 +109,10 @@ export class MemoryStore implements IStore {
108109
});
109110
}
110111

112+
public setUserCreator(creator: UserCreator): void {
113+
this.createUser = creator;
114+
}
115+
111116
/**
112117
* Called when a room member in a room being tracked by this store has been
113118
* updated.
@@ -119,7 +124,7 @@ export class MemoryStore implements IStore {
119124
return;
120125
}
121126

122-
const user = this.users[member.userId] || new User(member.userId);
127+
const user = this.users[member.userId] || this.createUser?.(member.userId);
123128
if (member.name) {
124129
user.setDisplayName(member.name);
125130
if (member.events.member) {

‎src/store/stub.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { Room } from "../models/room";
2323
import { User } from "../models/user";
2424
import { IEvent, MatrixEvent } from "../models/event";
2525
import { Filter } from "../filter";
26-
import { ISavedSync, IStore } from "./index";
26+
import { ISavedSync, IStore, UserCreator } from "./index";
2727
import { RoomSummary } from "../models/room-summary";
2828
import { ISyncResponse } from "../sync-accumulator";
2929
import { IStateEventWithRoomId } from "../@types/search";
@@ -117,6 +117,13 @@ export class StubStore implements IStore {
117117
return [];
118118
}
119119

120+
/**
121+
* No-op.
122+
*/
123+
public setUserCreator(creator: UserCreator): void {
124+
return;
125+
}
126+
120127
/**
121128
* Store events for a room.
122129
* @param room - The room to store events for.

‎src/sync.ts

+4-16
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ limitations under the License.
2626
import { Optional } from "matrix-events-sdk";
2727

2828
import type { SyncCryptoCallbacks } from "./common-crypto/CryptoBackend";
29-
import { User, UserEvent } from "./models/user";
29+
import { User } from "./models/user";
3030
import { NotificationCountType, Room, RoomEvent } from "./models/room";
3131
import { deepCopy, defer, IDeferred, noUnsafeEventProps, promiseMapSeries, unsafeProp } from "./utils";
3232
import { Filter } from "./filter";
@@ -431,7 +431,7 @@ export class SyncApi {
431431
if (user) {
432432
user.setPresenceEvent(presenceEvent);
433433
} else {
434-
user = createNewUser(client, presenceEvent.getContent().user_id);
434+
user = User.createUser(presenceEvent.getContent().user_id, client);
435435
user.setPresenceEvent(presenceEvent);
436436
client.store.storeUser(user);
437437
}
@@ -530,7 +530,7 @@ export class SyncApi {
530530
if (user) {
531531
user.setPresenceEvent(presenceEvent);
532532
} else {
533-
user = createNewUser(this.client, presenceEvent.getContent().user_id);
533+
user = User.createUser(presenceEvent.getContent().user_id, this.client);
534534
user.setPresenceEvent(presenceEvent);
535535
this.client.store.storeUser(user);
536536
}
@@ -1150,7 +1150,7 @@ export class SyncApi {
11501150
if (user) {
11511151
user.setPresenceEvent(presenceEvent);
11521152
} else {
1153-
user = createNewUser(client, presenceEvent.getSender()!);
1153+
user = User.createUser(presenceEvent.getSender()!, client);
11541154
user.setPresenceEvent(presenceEvent);
11551155
client.store.storeUser(user);
11561156
}
@@ -1893,18 +1893,6 @@ export class SyncApi {
18931893
};
18941894
}
18951895

1896-
function createNewUser(client: MatrixClient, userId: string): User {
1897-
const user = new User(userId);
1898-
client.reEmitter.reEmit(user, [
1899-
UserEvent.AvatarUrl,
1900-
UserEvent.DisplayName,
1901-
UserEvent.Presence,
1902-
UserEvent.CurrentlyActive,
1903-
UserEvent.LastPresenceTs,
1904-
]);
1905-
return user;
1906-
}
1907-
19081896
// /!\ This function is not intended for public use! It's only exported from
19091897
// here in order to share some common logic with sliding-sync-sdk.ts.
19101898
export function _createAndReEmitRoom(client: MatrixClient, roomId: string, opts: Partial<IStoredClientOpts>): Room {

0 commit comments

Comments
 (0)