Skip to content

Commit

Permalink
Fix reemitter not being correctly wired on user objects created in st…
Browse files Browse the repository at this point in the history
…orage 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
  • Loading branch information
MidhunSureshR authored Oct 27, 2023
1 parent ce7b7bf commit 88d066a
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 22 deletions.
31 changes: 31 additions & 0 deletions spec/integ/matrix-client-event-emitter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,37 @@ describe("MatrixClient events", function () {
expect(fired).toBe(true);
});

it("should emit User events when presence data is absent in first sync", async () => {
const MODIFIED_SYNC_DATA: any = structuredClone(SYNC_DATA);
delete MODIFIED_SYNC_DATA["presence"];
const MODIFIED_NEXT_SYNC_DATA: any = structuredClone(NEXT_SYNC_DATA);
MODIFIED_NEXT_SYNC_DATA.presence = {
events: [
utils.mkPresence({
user: "@foo:bar",
name: "Foo Bar",
presence: "online",
}),
],
};
httpBackend!.when("GET", "/sync").respond(200, MODIFIED_SYNC_DATA);
httpBackend!.when("GET", "/sync").respond(200, MODIFIED_NEXT_SYNC_DATA);
let fired = false;
client!.on(UserEvent.Presence, function (event, user) {
fired = true;
expect(user).toBeTruthy();
expect(event).toBeTruthy();
if (!user || !event) {
return;
}
expect(event.event).toEqual(MODIFIED_NEXT_SYNC_DATA.presence.events[0]);
expect(user.presence).toEqual(MODIFIED_NEXT_SYNC_DATA.presence.events[0]?.content?.presence);
});
client!.startClient();
await httpBackend!.flushAllExpected();
expect(fired).toBe(true);
});

it("should emit Room events", function () {
httpBackend!.when("GET", "/sync").respond(200, SYNC_DATA);
httpBackend!.when("GET", "/sync").respond(200, NEXT_SYNC_DATA);
Expand Down
1 change: 1 addition & 0 deletions spec/unit/event-mapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe("eventMapperFor", function () {
getRoom(roomId: string): Room | null {
return rooms.find((r) => r.roomId === roomId) ?? null;
},
setUserCreator(_) {},
} as IStore,
scheduler: {
setProcessFunction: jest.fn(),
Expand Down
1 change: 1 addition & 0 deletions spec/unit/matrix-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ describe("MatrixClient", function () {
"storeFilter",
"startup",
"deleteAllData",
"setUserCreator",
] as const
).reduce((r, k) => {
r[k] = jest.fn();
Expand Down
59 changes: 58 additions & 1 deletion spec/unit/stores/indexeddb.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ limitations under the License.

import "fake-indexeddb/auto";
import "jest-localstorage-mock";
import { IDBFactory } from "fake-indexeddb";

import { IndexedDBStore, IStateEventWithRoomId, MemoryStore } from "../../../src";
import { IndexedDBStore, IStateEventWithRoomId, MemoryStore, User, UserEvent } from "../../../src";
import { emitPromise } from "../../test-utils/test-utils";
import { LocalIndexedDBStoreBackend } from "../../../src/store/indexeddb-local-backend";
import { defer } from "../../../src/utils";
Expand Down Expand Up @@ -76,6 +77,62 @@ describe("IndexedDBStore", () => {
expect(await store.getOutOfBandMembers(roomId)).toHaveLength(2);
});

it("Should load presence events on startup", async () => {
// 1. Create idb database
const indexedDB = new IDBFactory();
const setupDefer = defer<Event>();
const req = indexedDB.open("matrix-js-sdk:db3", 1);
let db: IDBDatabase;
req.onupgradeneeded = () => {
db = req.result;
db.createObjectStore("users", { keyPath: ["userId"] });
db.createObjectStore("accountData", { keyPath: ["type"] });
db.createObjectStore("sync", { keyPath: ["clobber"] });
};
req.onsuccess = setupDefer.resolve;
await setupDefer.promise;

// 2. Fill in user presence data
const writeDefer = defer<Event>();
const transaction = db!.transaction(["users"], "readwrite");
const objectStore = transaction.objectStore("users");
const request = objectStore.put({
userId: "@alice:matrix.org",
event: {
content: {
presence: "online",
},
sender: "@alice:matrix.org",
type: "m.presence",
},
});
request.onsuccess = writeDefer.resolve;
await writeDefer.promise;

// 3. Close database
req.result.close();

// 2. Check if the code loads presence events
const store = new IndexedDBStore({
indexedDB: indexedDB,
dbName: "db3",
localStorage,
});
let userCreated = false;
let presenceEventEmitted = false;
store.setUserCreator((id: string) => {
userCreated = true;
const user = new User(id);
user.on(UserEvent.Presence, () => {
presenceEventEmitted = true;
});
return user;
});
await store.startup();
expect(userCreated).toBe(true);
expect(presenceEventEmitted).toBe(true);
});

it("should use MemoryStore methods for pending events if no localStorage", async () => {
jest.spyOn(MemoryStore.prototype, "setPendingEvents");
jest.spyOn(MemoryStore.prototype, "getPendingEvents");
Expand Down
1 change: 1 addition & 0 deletions src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa

this.usingExternalCrypto = opts.usingExternalCrypto ?? false;
this.store = opts.store || new StubStore();
this.store.setUserCreator((userId) => User.createUser(userId, this));
this.deviceId = opts.deviceId || null;
this.sessionId = randomString(10);

Expand Down
21 changes: 21 additions & 0 deletions src/models/user.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { MatrixClient } from "../matrix";
import { MatrixEvent } from "./event";
import { TypedEventEmitter } from "./typed-event-emitter";

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

/**
* Construct a new User whose events will also emit on MatrixClient.
* A User must have an ID and can optionally have extra information associated with it.
* @param userId - Required. The ID of this user.
* @param client - An instance of MatrixClient object
* @returns User object with reEmitter setup on client
*/
public static createUser(userId: string, client: MatrixClient): User {
const user = new User(userId);
client.reEmitter.reEmit(user, [
UserEvent.AvatarUrl,
UserEvent.DisplayName,
UserEvent.Presence,
UserEvent.CurrentlyActive,
UserEvent.LastPresenceTs,
]);
return user;
}

/**
* Update this User with the given presence event. May fire "User.presence",
* "User.avatarUrl" and/or "User.displayName" if this event updates this user's
Expand Down
8 changes: 8 additions & 0 deletions src/store/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export interface ISavedSync {
accountData: IMinimalEvent[];
}

export type UserCreator = (userId: string) => User;

/**
* A store for most of the data js-sdk needs to store, apart from crypto data
*/
Expand Down Expand Up @@ -62,6 +64,12 @@ export interface IStore {
*/
storeRoom(room: Room): void;

/**
* Set the user creator which is used for creating User objects
* @param creator - A callback that accepts an user-id and returns an User object
*/
setUserCreator(creator: UserCreator): void;

/**
* Retrieve a room by its' room ID.
* @param roomId - The room ID.
Expand Down
6 changes: 4 additions & 2 deletions src/store/indexeddb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ limitations under the License.
import { MemoryStore, IOpts as IBaseOpts } from "./memory";
import { LocalIndexedDBStoreBackend } from "./indexeddb-local-backend";
import { RemoteIndexedDBStoreBackend } from "./indexeddb-remote-backend";
import { User } from "../models/user";
import { IEvent, MatrixEvent } from "../models/event";
import { logger } from "../logger";
import { ISavedSync } from "./index";
Expand Down Expand Up @@ -140,7 +139,10 @@ export class IndexedDBStore extends MemoryStore {
.then((userPresenceEvents) => {
logger.log(`IndexedDBStore.startup: processing presence events`);
userPresenceEvents.forEach(([userId, rawEvent]) => {
const u = new User(userId);
if (!this.createUser) {
throw new Error("createUser is undefined, it should be set with setUserCreator()!");
}
const u = this.createUser(userId);
if (rawEvent) {
u.setPresenceEvent(new MatrixEvent(rawEvent));
}
Expand Down
9 changes: 7 additions & 2 deletions src/store/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { IEvent, MatrixEvent } from "../models/event";
import { RoomState, RoomStateEvent } from "../models/room-state";
import { RoomMember } from "../models/room-member";
import { Filter } from "../filter";
import { ISavedSync, IStore } from "./index";
import { ISavedSync, IStore, UserCreator } from "./index";
import { RoomSummary } from "../models/room-summary";
import { ISyncResponse } from "../sync-accumulator";
import { IStateEventWithRoomId } from "../@types/search";
Expand Down Expand Up @@ -63,6 +63,7 @@ export class MemoryStore implements IStore {
private clientOptions?: IStoredClientOpts;
private pendingToDeviceBatches: IndexedToDeviceBatch[] = [];
private nextToDeviceBatchId = 0;
protected createUser?: UserCreator;

/**
* Construct a new in-memory data store for the Matrix Client.
Expand Down Expand Up @@ -108,6 +109,10 @@ export class MemoryStore implements IStore {
});
}

public setUserCreator(creator: UserCreator): void {
this.createUser = creator;
}

/**
* Called when a room member in a room being tracked by this store has been
* updated.
Expand All @@ -119,7 +124,7 @@ export class MemoryStore implements IStore {
return;
}

const user = this.users[member.userId] || new User(member.userId);
const user = this.users[member.userId] || this.createUser?.(member.userId);
if (member.name) {
user.setDisplayName(member.name);
if (member.events.member) {
Expand Down
9 changes: 8 additions & 1 deletion src/store/stub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { Room } from "../models/room";
import { User } from "../models/user";
import { IEvent, MatrixEvent } from "../models/event";
import { Filter } from "../filter";
import { ISavedSync, IStore } from "./index";
import { ISavedSync, IStore, UserCreator } from "./index";
import { RoomSummary } from "../models/room-summary";
import { ISyncResponse } from "../sync-accumulator";
import { IStateEventWithRoomId } from "../@types/search";
Expand Down Expand Up @@ -117,6 +117,13 @@ export class StubStore implements IStore {
return [];
}

/**
* No-op.
*/
public setUserCreator(creator: UserCreator): void {
return;
}

/**
* Store events for a room.
* @param room - The room to store events for.
Expand Down
20 changes: 4 additions & 16 deletions src/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ limitations under the License.
import { Optional } from "matrix-events-sdk";

import type { SyncCryptoCallbacks } from "./common-crypto/CryptoBackend";
import { User, UserEvent } from "./models/user";
import { User } from "./models/user";
import { NotificationCountType, Room, RoomEvent } from "./models/room";
import { deepCopy, defer, IDeferred, noUnsafeEventProps, promiseMapSeries, unsafeProp } from "./utils";
import { Filter } from "./filter";
Expand Down Expand Up @@ -431,7 +431,7 @@ export class SyncApi {
if (user) {
user.setPresenceEvent(presenceEvent);
} else {
user = createNewUser(client, presenceEvent.getContent().user_id);
user = User.createUser(presenceEvent.getContent().user_id, client);
user.setPresenceEvent(presenceEvent);
client.store.storeUser(user);
}
Expand Down Expand Up @@ -530,7 +530,7 @@ export class SyncApi {
if (user) {
user.setPresenceEvent(presenceEvent);
} else {
user = createNewUser(this.client, presenceEvent.getContent().user_id);
user = User.createUser(presenceEvent.getContent().user_id, this.client);
user.setPresenceEvent(presenceEvent);
this.client.store.storeUser(user);
}
Expand Down Expand Up @@ -1150,7 +1150,7 @@ export class SyncApi {
if (user) {
user.setPresenceEvent(presenceEvent);
} else {
user = createNewUser(client, presenceEvent.getSender()!);
user = User.createUser(presenceEvent.getSender()!, client);
user.setPresenceEvent(presenceEvent);
client.store.storeUser(user);
}
Expand Down Expand Up @@ -1893,18 +1893,6 @@ export class SyncApi {
};
}

function createNewUser(client: MatrixClient, userId: string): User {
const user = new User(userId);
client.reEmitter.reEmit(user, [
UserEvent.AvatarUrl,
UserEvent.DisplayName,
UserEvent.Presence,
UserEvent.CurrentlyActive,
UserEvent.LastPresenceTs,
]);
return user;
}

// /!\ This function is not intended for public use! It's only exported from
// here in order to share some common logic with sliding-sync-sdk.ts.
export function _createAndReEmitRoom(client: MatrixClient, roomId: string, opts: Partial<IStoredClientOpts>): Room {
Expand Down

0 comments on commit 88d066a

Please sign in to comment.