From 781fdf4fdc45c17b5c37a94e7e0b8ec588ea1218 Mon Sep 17 00:00:00 2001 From: Kerry Date: Fri, 8 Apr 2022 10:50:06 +0200 Subject: [PATCH] Live location sharing - update beacon_info implementation to latest MSC (#2281) * remove M_BEACON_INFO_VARIABLE Signed-off-by: Kerry Archibald * create beacon_info events with non-variable event type Signed-off-by: Kerry Archibald * remove isBeaconInfoEventType Signed-off-by: Kerry Archibald * refer to msc3673 instead of msc3489 Signed-off-by: Kerry Archibald * remove event type suffix Signed-off-by: Kerry Archibald * update beacon identifier to use state key Signed-off-by: Kerry Archibald * fix beacon spec Signed-off-by: Kerry Archibald * fix room-state tests Signed-off-by: Kerry Archibald * add beacon identifier Signed-off-by: Kerry Archibald * dont allow update to older beacon event Signed-off-by: Kerry Archibald * lint Signed-off-by: Kerry Archibald * unnest beacon_info content Signed-off-by: Kerry Archibald * lint Signed-off-by: Kerry Archibald * check redaction event id Signed-off-by: Kerry Archibald --- spec/test-utils/beacon.ts | 5 ++- spec/unit/content-helpers.spec.ts | 9 ++--- spec/unit/matrix-client.spec.ts | 10 ++--- spec/unit/models/beacon.spec.ts | 63 +++++++++++++++++-------------- spec/unit/room-state.spec.js | 30 +++++++-------- src/@types/beacon.ts | 28 +++++--------- src/client.ts | 15 ++------ src/content-helpers.ts | 18 ++++----- src/models/beacon.ts | 21 +++++++---- src/models/room-state.ts | 26 ++++++++----- 10 files changed, 108 insertions(+), 117 deletions(-) diff --git a/spec/test-utils/beacon.ts b/spec/test-utils/beacon.ts index 84fe41cdf27..0823cca0c72 100644 --- a/spec/test-utils/beacon.ts +++ b/spec/test-utils/beacon.ts @@ -42,7 +42,6 @@ export const makeBeaconInfoEvent = ( roomId: string, contentProps: Partial = {}, eventId?: string, - eventTypeSuffix?: string, ): MatrixEvent => { const { timeout, isLive, description, assetType, @@ -51,12 +50,14 @@ export const makeBeaconInfoEvent = ( ...contentProps, }; const event = new MatrixEvent({ - type: `${M_BEACON_INFO.name}.${sender}.${eventTypeSuffix || Date.now()}`, + type: M_BEACON_INFO.name, room_id: roomId, state_key: sender, content: makeBeaconInfoContent(timeout, isLive, description, assetType), }); + event.event.origin_server_ts = Date.now(); + // live beacons use the beacon_info event id // set or default this event.replaceLocalEventId(eventId || `$${Math.random()}-${Math.random()}`); diff --git a/spec/unit/content-helpers.spec.ts b/spec/unit/content-helpers.spec.ts index 3430bf4c2c1..71b7344ed6a 100644 --- a/spec/unit/content-helpers.spec.ts +++ b/spec/unit/content-helpers.spec.ts @@ -16,7 +16,6 @@ limitations under the License. import { REFERENCE_RELATION } from "matrix-events-sdk"; -import { M_BEACON_INFO } from "../../src/@types/beacon"; import { LocationAssetType, M_ASSET, M_LOCATION, M_TIMESTAMP } from "../../src/@types/location"; import { makeBeaconContent, makeBeaconInfoContent } from "../../src/content-helpers"; @@ -36,11 +35,9 @@ describe('Beacon content helpers', () => { 'nice beacon_info', LocationAssetType.Pin, )).toEqual({ - [M_BEACON_INFO.name]: { - description: 'nice beacon_info', - timeout: 1234, - live: true, - }, + description: 'nice beacon_info', + timeout: 1234, + live: true, [M_TIMESTAMP.name]: mockDateNow, [M_ASSET.name]: { type: LocationAssetType.Pin, diff --git a/spec/unit/matrix-client.spec.ts b/spec/unit/matrix-client.spec.ts index b363daa2326..b9b0081135e 100644 --- a/spec/unit/matrix-client.spec.ts +++ b/spec/unit/matrix-client.spec.ts @@ -999,10 +999,10 @@ describe("MatrixClient", function() { }); it("creates new beacon info", async () => { - await client.unstable_createLiveBeacon(roomId, content, '123'); + await client.unstable_createLiveBeacon(roomId, content); // event type combined - const expectedEventType = `${M_BEACON_INFO.name}.${userId}.123`; + const expectedEventType = M_BEACON_INFO.name; const [callback, method, path, queryParams, requestContent] = client.http.authedRequest.mock.calls[0]; expect(callback).toBeFalsy(); expect(method).toBe('PUT'); @@ -1015,15 +1015,13 @@ describe("MatrixClient", function() { }); it("updates beacon info with specific event type", async () => { - const eventType = `${M_BEACON_INFO.name}.${userId}.456`; - - await client.unstable_setLiveBeacon(roomId, eventType, content); + await client.unstable_setLiveBeacon(roomId, content); // event type combined const [, , path, , requestContent] = client.http.authedRequest.mock.calls[0]; expect(path).toEqual( `/rooms/${encodeURIComponent(roomId)}/state/` + - `${encodeURIComponent(eventType)}/${encodeURIComponent(userId)}`, + `${encodeURIComponent(M_BEACON_INFO.name)}/${encodeURIComponent(userId)}`, ); expect(requestContent).toEqual(content); }); diff --git a/spec/unit/models/beacon.spec.ts b/spec/unit/models/beacon.spec.ts index 0e39e7c6961..a037d905dd7 100644 --- a/spec/unit/models/beacon.spec.ts +++ b/spec/unit/models/beacon.spec.ts @@ -14,11 +14,8 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EventType } from "../../../src"; -import { M_BEACON_INFO } from "../../../src/@types/beacon"; import { isTimestampInDuration, - isBeaconInfoEventType, Beacon, BeaconEvent, } from "../../../src/models/beacon"; @@ -57,27 +54,9 @@ describe('Beacon', () => { }); }); - describe('isBeaconInfoEventType', () => { - it.each([ - EventType.CallAnswer, - `prefix.${M_BEACON_INFO.name}`, - `prefix.${M_BEACON_INFO.altName}`, - ])('returns false for %s', (type) => { - expect(isBeaconInfoEventType(type)).toBe(false); - }); - - it.each([ - M_BEACON_INFO.name, - M_BEACON_INFO.altName, - `${M_BEACON_INFO.name}.@test:server.org.12345`, - `${M_BEACON_INFO.altName}.@test:server.org.12345`, - ])('returns true for %s', (type) => { - expect(isBeaconInfoEventType(type)).toBe(true); - }); - }); - describe('Beacon', () => { const userId = '@user:server.org'; + const userId2 = '@user2:server.org'; const roomId = '$room:server.org'; // 14.03.2022 16:15 const now = 1647270879403; @@ -88,6 +67,7 @@ describe('Beacon', () => { // without timeout of 3 hours let liveBeaconEvent; let notLiveBeaconEvent; + let user2BeaconEvent; const advanceDateAndTime = (ms: number) => { // bc liveness check uses Date.now we have to advance this mock @@ -107,14 +87,21 @@ describe('Beacon', () => { isLive: true, }, '$live123', - '$live123', ); notLiveBeaconEvent = makeBeaconInfoEvent( userId, roomId, { timeout: HOUR_MS * 3, isLive: false }, '$dead123', - '$dead123', + ); + user2BeaconEvent = makeBeaconInfoEvent( + userId2, + roomId, + { + timeout: HOUR_MS * 3, + isLive: true, + }, + '$user2live123', ); // back to now @@ -133,7 +120,7 @@ describe('Beacon', () => { expect(beacon.isLive).toEqual(true); expect(beacon.beaconInfoOwner).toEqual(userId); expect(beacon.beaconInfoEventType).toEqual(liveBeaconEvent.getType()); - expect(beacon.identifier).toEqual(liveBeaconEvent.getType()); + expect(beacon.identifier).toEqual(`${roomId}_${userId}`); expect(beacon.beaconInfo).toBeTruthy(); }); @@ -171,8 +158,27 @@ describe('Beacon', () => { expect(beacon.beaconInfoId).toEqual(liveBeaconEvent.getId()); - expect(() => beacon.update(notLiveBeaconEvent)).toThrow(); - expect(beacon.isLive).toEqual(true); + expect(() => beacon.update(user2BeaconEvent)).toThrow(); + // didnt update + expect(beacon.identifier).toEqual(`${roomId}_${userId}`); + }); + + it('does not update with an older event', () => { + const beacon = new Beacon(liveBeaconEvent); + const emitSpy = jest.spyOn(beacon, 'emit').mockClear(); + expect(beacon.beaconInfoId).toEqual(liveBeaconEvent.getId()); + + const oldUpdateEvent = makeBeaconInfoEvent( + userId, + roomId, + ); + // less than the original event + oldUpdateEvent.event.origin_server_ts = liveBeaconEvent.event.origin_server_ts - 1000; + + beacon.update(oldUpdateEvent); + // didnt update + expect(emitSpy).not.toHaveBeenCalled(); + expect(beacon.beaconInfoId).toEqual(liveBeaconEvent.getId()); }); it('updates event', () => { @@ -182,7 +188,7 @@ describe('Beacon', () => { expect(beacon.isLive).toEqual(true); const updatedBeaconEvent = makeBeaconInfoEvent( - userId, roomId, { timeout: HOUR_MS * 3, isLive: false }, '$live123', '$live123'); + userId, roomId, { timeout: HOUR_MS * 3, isLive: false }, '$live123'); beacon.update(updatedBeaconEvent); expect(beacon.isLive).toEqual(false); @@ -200,7 +206,6 @@ describe('Beacon', () => { roomId, { timeout: HOUR_MS * 3, isLive: false }, beacon.beaconInfoId, - '$live123', ); beacon.update(updatedBeaconEvent); diff --git a/spec/unit/room-state.spec.js b/spec/unit/room-state.spec.js index fa00d21fc9a..a96ff989c45 100644 --- a/spec/unit/room-state.spec.js +++ b/spec/unit/room-state.spec.js @@ -2,7 +2,7 @@ import * as utils from "../test-utils/test-utils"; import { makeBeaconInfoEvent } from "../test-utils/beacon"; import { filterEmitCallsByEventType } from "../test-utils/emitter"; import { RoomState, RoomStateEvent } from "../../src/models/room-state"; -import { BeaconEvent } from "../../src/models/beacon"; +import { BeaconEvent, getBeaconInfoIdentifier } from "../../src/models/beacon"; describe("RoomState", function() { const roomId = "!foo:bar"; @@ -260,7 +260,7 @@ describe("RoomState", function() { state.setStateEvents([beaconEvent]); expect(state.beacons.size).toEqual(1); - const beaconInstance = state.beacons.get(beaconEvent.getType()); + const beaconInstance = state.beacons.get(`${roomId}_${userA}`); expect(beaconInstance).toBeTruthy(); expect(emitSpy).toHaveBeenCalledWith(BeaconEvent.New, beaconEvent, beaconInstance); }); @@ -275,49 +275,49 @@ describe("RoomState", function() { // no beacon added expect(state.beacons.size).toEqual(0); - expect(state.beacons.get(redactedBeaconEvent.getType)).toBeFalsy(); + expect(state.beacons.get(getBeaconInfoIdentifier(redactedBeaconEvent))).toBeFalsy(); // no new beacon emit expect(filterEmitCallsByEventType(BeaconEvent.New, emitSpy).length).toBeFalsy(); }); it('updates existing beacon info events in state', () => { const beaconId = '$beacon1'; - const beaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId, beaconId); - const updatedBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: false }, beaconId, beaconId); + const beaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId); + const updatedBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: false }, beaconId); state.setStateEvents([beaconEvent]); - const beaconInstance = state.beacons.get(beaconEvent.getType()); + const beaconInstance = state.beacons.get(getBeaconInfoIdentifier(beaconEvent)); expect(beaconInstance.isLive).toEqual(true); state.setStateEvents([updatedBeaconEvent]); // same Beacon - expect(state.beacons.get(beaconEvent.getType())).toBe(beaconInstance); + expect(state.beacons.get(getBeaconInfoIdentifier(beaconEvent))).toBe(beaconInstance); // updated liveness - expect(state.beacons.get(beaconEvent.getType()).isLive).toEqual(false); + expect(state.beacons.get(getBeaconInfoIdentifier(beaconEvent)).isLive).toEqual(false); }); it('destroys and removes redacted beacon events', () => { const beaconId = '$beacon1'; - const beaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId, beaconId); - const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId, beaconId); - const redactionEvent = { event: { type: 'm.room.redaction' } }; + const beaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId); + const redactedBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, beaconId); + const redactionEvent = { event: { type: 'm.room.redaction', redacts: beaconEvent.getId() } }; redactedBeaconEvent.makeRedacted(redactionEvent); state.setStateEvents([beaconEvent]); - const beaconInstance = state.beacons.get(beaconEvent.getType()); + const beaconInstance = state.beacons.get(getBeaconInfoIdentifier(beaconEvent)); const destroySpy = jest.spyOn(beaconInstance, 'destroy'); expect(beaconInstance.isLive).toEqual(true); state.setStateEvents([redactedBeaconEvent]); expect(destroySpy).toHaveBeenCalled(); - expect(state.beacons.get(beaconEvent.getType())).toBe(undefined); + expect(state.beacons.get(getBeaconInfoIdentifier(beaconEvent))).toBe(undefined); }); it('updates live beacon ids once after setting state events', () => { - const liveBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, '$beacon1', '$beacon1'); - const deadBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: false }, '$beacon2', '$beacon2'); + const liveBeaconEvent = makeBeaconInfoEvent(userA, roomId, { isLive: true }, '$beacon1'); + const deadBeaconEvent = makeBeaconInfoEvent(userB, roomId, { isLive: false }, '$beacon2'); const emitSpy = jest.spyOn(state, 'emit'); diff --git a/src/@types/beacon.ts b/src/@types/beacon.ts index adf033daa24..6da17061e61 100644 --- a/src/@types/beacon.ts +++ b/src/@types/beacon.ts @@ -14,14 +14,14 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { EitherAnd, RELATES_TO_RELATIONSHIP, REFERENCE_RELATION } from "matrix-events-sdk"; +import { RELATES_TO_RELATIONSHIP, REFERENCE_RELATION } from "matrix-events-sdk"; import { UnstableValue } from "../NamespacedValue"; import { MAssetEvent, MLocationEvent, MTimestampEvent } from "./location"; /** - * Beacon info and beacon event types as described in MSC3489 - * https://github.com/matrix-org/matrix-spec-proposals/pull/3489 + * Beacon info and beacon event types as described in MSC3672 + * https://github.com/matrix-org/matrix-spec-proposals/pull/3672 */ /** @@ -60,16 +60,11 @@ import { MAssetEvent, MLocationEvent, MTimestampEvent } from "./location"; * } */ -/** - * Variable event type for m.beacon_info - */ -export const M_BEACON_INFO_VARIABLE = new UnstableValue("m.beacon_info.*", "org.matrix.msc3489.beacon_info.*"); - /** * Non-variable type for m.beacon_info event content */ -export const M_BEACON_INFO = new UnstableValue("m.beacon_info", "org.matrix.msc3489.beacon_info"); -export const M_BEACON = new UnstableValue("m.beacon", "org.matrix.msc3489.beacon"); +export const M_BEACON_INFO = new UnstableValue("m.beacon_info", "org.matrix.msc3672.beacon_info"); +export const M_BEACON = new UnstableValue("m.beacon", "org.matrix.msc3672.beacon"); export type MBeaconInfoContent = { description?: string; @@ -80,16 +75,11 @@ export type MBeaconInfoContent = { live?: boolean; }; -export type MBeaconInfoEvent = EitherAnd< - { [M_BEACON_INFO.name]: MBeaconInfoContent }, - { [M_BEACON_INFO.altName]: MBeaconInfoContent } ->; - /** * m.beacon_info Event example from the spec - * https://github.com/matrix-org/matrix-spec-proposals/pull/3489 + * https://github.com/matrix-org/matrix-spec-proposals/pull/3672 * { - "type": "m.beacon_info.@matthew:matrix.org.1", + "type": "m.beacon_info", "state_key": "@matthew:matrix.org", "content": { "m.beacon_info": { @@ -108,7 +98,7 @@ export type MBeaconInfoEvent = EitherAnd< * m.beacon_info.* event content */ export type MBeaconInfoEventContent = & - MBeaconInfoEvent & + MBeaconInfoContent & // creation timestamp of the beacon on the client MTimestampEvent & // the type of asset being tracked as per MSC3488 @@ -116,7 +106,7 @@ export type MBeaconInfoEventContent = & /** * m.beacon event example - * https://github.com/matrix-org/matrix-spec-proposals/pull/3489 + * https://github.com/matrix-org/matrix-spec-proposals/pull/3672 * * { "type": "m.beacon", diff --git a/src/client.ts b/src/client.ts index 982feb2bf3c..1ae008dba2a 100644 --- a/src/client.ts +++ b/src/client.ts @@ -180,7 +180,7 @@ import { MediaHandler } from "./webrtc/mediaHandler"; import { IRefreshTokenResponse } from "./@types/auth"; import { TypedEventEmitter } from "./models/typed-event-emitter"; import { Thread, THREAD_RELATION_TYPE } from "./models/thread"; -import { MBeaconInfoEventContent, M_BEACON_INFO_VARIABLE } from "./@types/beacon"; +import { MBeaconInfoEventContent, M_BEACON_INFO } from "./@types/beacon"; export type Store = IStore; export type SessionStore = WebStorageSessionStore; @@ -3649,39 +3649,30 @@ export class MatrixClient extends TypedEventEmitter ({ - [M_BEACON_INFO.name]: { - description, - timeout, - live: isLive, - }, + description, + timeout, + live: isLive, [M_TIMESTAMP.name]: timestamp || Date.now(), [M_ASSET.name]: { type: assetType ?? LocationAssetType.Self, @@ -227,7 +225,7 @@ export type BeaconInfoState = MBeaconInfoContent & { * Flatten beacon info event content */ export const parseBeaconInfoContent = (content: MBeaconInfoEventContent): BeaconInfoState => { - const { description, timeout, live } = M_BEACON_INFO.findIn(content); + const { description, timeout, live } = content; const { type: assetType } = M_ASSET.findIn(content); const timestamp = M_TIMESTAMP.findIn(content); @@ -243,14 +241,14 @@ export const parseBeaconInfoContent = (content: MBeaconInfoEventContent): Beacon export type MakeBeaconContent = ( uri: string, timestamp: number, - beaconInfoId: string, + beaconInfoEventId: string, description?: string, ) => MBeaconEventContent; export const makeBeaconContent: MakeBeaconContent = ( uri, timestamp, - beaconInfoId, + beaconInfoEventId, description, ) => ({ [M_LOCATION.name]: { @@ -260,6 +258,6 @@ export const makeBeaconContent: MakeBeaconContent = ( [M_TIMESTAMP.name]: timestamp, "m.relates_to": { rel_type: REFERENCE_RELATION.name, - event_id: beaconInfoId, + event_id: beaconInfoEventId, }, }); diff --git a/src/models/beacon.ts b/src/models/beacon.ts index 329fc04e6bd..70cb67f7a13 100644 --- a/src/models/beacon.ts +++ b/src/models/beacon.ts @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { M_BEACON_INFO } from "../@types/beacon"; import { BeaconInfoState, parseBeaconInfoContent } from "../content-helpers"; import { MatrixEvent } from "../matrix"; import { TypedEventEmitter } from "./typed-event-emitter"; @@ -38,11 +37,13 @@ export const isTimestampInDuration = ( timestamp: number, ): boolean => timestamp >= startTimestamp && startTimestamp + durationMs >= timestamp; -export const isBeaconInfoEventType = (type: string) => - type.startsWith(M_BEACON_INFO.name) || - type.startsWith(M_BEACON_INFO.altName); +// beacon info events are uniquely identified by +// `_` +export type BeaconIdentifier = string; +export const getBeaconInfoIdentifier = (event: MatrixEvent): BeaconIdentifier => + `${event.getRoomId()}_${event.getStateKey()}`; -// https://github.com/matrix-org/matrix-spec-proposals/pull/3489 +// https://github.com/matrix-org/matrix-spec-proposals/pull/3672 export class Beacon extends TypedEventEmitter, BeaconEventHandlerMap> { public readonly roomId: string; private _beaconInfo: BeaconInfoState; @@ -61,8 +62,8 @@ export class Beacon extends TypedEventEmitter public events = new Map>(); // Map> public paginationToken: string = null; - public readonly beacons = new Map(); - private liveBeaconIds: string[] = []; + public readonly beacons = new Map(); + private liveBeaconIds: BeaconIdentifier[] = []; /** * Construct room state. @@ -330,7 +333,7 @@ export class RoomState extends TypedEventEmitter return; } - if (isBeaconInfoEventType(event.getType())) { + if (M_BEACON_INFO.matches(event.getType())) { this.setBeacon(event); } @@ -437,12 +440,15 @@ export class RoomState extends TypedEventEmitter * @experimental */ private setBeacon(event: MatrixEvent): void { - if (this.beacons.has(event.getType())) { - const beacon = this.beacons.get(event.getType()); + const beaconIdentifier = getBeaconInfoIdentifier(event); + if (this.beacons.has(beaconIdentifier)) { + const beacon = this.beacons.get(beaconIdentifier); if (event.isRedacted()) { - beacon.destroy(); - this.beacons.delete(event.getType()); + if (beacon.beaconInfoId === event.getRedactionEvent()?.['redacts']) { + beacon.destroy(); + this.beacons.delete(beaconIdentifier); + } return; } @@ -464,7 +470,7 @@ export class RoomState extends TypedEventEmitter this.emit(BeaconEvent.New, event, beacon); beacon.on(BeaconEvent.LivenessChange, this.onBeaconLivenessChange.bind(this)); - this.beacons.set(beacon.beaconInfoEventType, beacon); + this.beacons.set(beacon.identifier, beacon); } /** @@ -477,7 +483,7 @@ export class RoomState extends TypedEventEmitter const prevHasLiveBeacons = !!this.liveBeaconIds?.length; this.liveBeaconIds = Array.from(this.beacons.values()) .filter(beacon => beacon.isLive) - .map(beacon => beacon.beaconInfoId); + .map(beacon => beacon.identifier); const hasLiveBeacons = !!this.liveBeaconIds.length; if (prevHasLiveBeacons !== hasLiveBeacons) {