From 2e7a72538fbdc0b103357e482b9b68db2d9012fb Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 16 Jun 2022 11:19:33 +0200 Subject: [PATCH 1/3] remove some of the confusing time travel in beacon.spec --- spec/test-utils/beacon.ts | 11 +++++-- spec/unit/models/beacon.spec.ts | 56 ++++++++++++++++++++++----------- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/spec/test-utils/beacon.ts b/spec/test-utils/beacon.ts index 0823cca0c72..252c85c8150 100644 --- a/spec/test-utils/beacon.ts +++ b/spec/test-utils/beacon.ts @@ -27,6 +27,7 @@ type InfoContentProps = { isLive?: boolean; assetType?: LocationAssetType; description?: string; + timestamp?: number; }; const DEFAULT_INFO_CONTENT_PROPS: InfoContentProps = { timeout: 3600000, @@ -44,7 +45,11 @@ export const makeBeaconInfoEvent = ( eventId?: string, ): MatrixEvent => { const { - timeout, isLive, description, assetType, + timeout, + isLive, + description, + assetType, + timestamp, } = { ...DEFAULT_INFO_CONTENT_PROPS, ...contentProps, @@ -53,10 +58,10 @@ export const makeBeaconInfoEvent = ( type: M_BEACON_INFO.name, room_id: roomId, state_key: sender, - content: makeBeaconInfoContent(timeout, isLive, description, assetType), + content: makeBeaconInfoContent(timeout, isLive, description, assetType, timestamp), }); - event.event.origin_server_ts = Date.now(); + event.event.origin_server_ts = timestamp || Date.now(); // live beacons use the beacon_info event id // set or default this diff --git a/spec/unit/models/beacon.spec.ts b/spec/unit/models/beacon.spec.ts index 30052ae7ca1..d1f77d79ef4 100644 --- a/spec/unit/models/beacon.spec.ts +++ b/spec/unit/models/beacon.spec.ts @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { MatrixEvent } from "../../../src"; import { isTimestampInDuration, Beacon, @@ -65,9 +66,9 @@ describe('Beacon', () => { // beacon_info events // created 'an hour ago' // without timeout of 3 hours - let liveBeaconEvent; - let notLiveBeaconEvent; - let user2BeaconEvent; + let liveBeaconEvent: MatrixEvent; + let notLiveBeaconEvent: MatrixEvent; + let user2BeaconEvent: MatrixEvent; const advanceDateAndTime = (ms: number) => { // bc liveness check uses Date.now we have to advance this mock @@ -77,21 +78,24 @@ describe('Beacon', () => { }; beforeEach(() => { - // go back in time to create the beacon - jest.spyOn(global.Date, 'now').mockReturnValue(now - HOUR_MS); liveBeaconEvent = makeBeaconInfoEvent( userId, roomId, { timeout: HOUR_MS * 3, isLive: true, + timestamp: now - HOUR_MS, }, '$live123', ); notLiveBeaconEvent = makeBeaconInfoEvent( userId, roomId, - { timeout: HOUR_MS * 3, isLive: false }, + { + timeout: HOUR_MS * 3, + isLive: false, + timestamp: now - HOUR_MS, + }, '$dead123', ); user2BeaconEvent = makeBeaconInfoEvent( @@ -100,11 +104,12 @@ describe('Beacon', () => { { timeout: HOUR_MS * 3, isLive: true, + timestamp: now - HOUR_MS, }, '$user2live123', ); - // back to now + // back to 'now' jest.spyOn(global.Date, 'now').mockReturnValue(now); }); @@ -131,17 +136,32 @@ describe('Beacon', () => { }); it('returns false when beacon is expired', () => { - // time travel to beacon creation + 3 hours - jest.spyOn(global.Date, 'now').mockReturnValue(now - 3 * HOUR_MS); - const beacon = new Beacon(liveBeaconEvent); + const expiredBeaconEvent = makeBeaconInfoEvent( + userId2, + roomId, + { + timeout: HOUR_MS, + isLive: true, + timestamp: now - HOUR_MS * 2, + }, + '$user2live123', + ); + const beacon = new Beacon(expiredBeaconEvent); expect(beacon.isLive).toEqual(false); }); - it('returns false when beacon timestamp is in future', () => { - // time travel to before beacon events timestamp - // event was created now - 1 hour - jest.spyOn(global.Date, 'now').mockReturnValue(now - HOUR_MS - HOUR_MS); - const beacon = new Beacon(liveBeaconEvent); + it('returns false when beacon timestamp is in future by an hour', () => { + const beaconStartsInHour = makeBeaconInfoEvent( + userId2, + roomId, + { + timeout: HOUR_MS, + isLive: true, + timestamp: now + HOUR_MS, + }, + '$user2live123', + ); + const beacon = new Beacon(beaconStartsInHour); expect(beacon.isLive).toEqual(false); }); @@ -232,19 +252,17 @@ describe('Beacon', () => { }); it('checks liveness of beacon at expected start time', () => { - // go forward in time to make beacon with timestamp in future - jest.spyOn(global.Date, 'now').mockReturnValue(now + HOUR_MS); const futureBeaconEvent = makeBeaconInfoEvent( userId, roomId, { timeout: HOUR_MS * 3, isLive: true, + // start timestamp hour in future + timestamp: now + HOUR_MS, }, '$live123', ); - // go back to now - jest.spyOn(global.Date, 'now').mockReturnValue(now); const beacon = new Beacon(futureBeaconEvent); expect(beacon.isLive).toBeFalsy(); From 8ea18889ef2f90ad03220f83de57f00852d9be62 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 16 Jun 2022 11:35:27 +0200 Subject: [PATCH 2/3] test cases --- spec/unit/models/beacon.spec.ts | 101 ++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/spec/unit/models/beacon.spec.ts b/spec/unit/models/beacon.spec.ts index d1f77d79ef4..c68d9d803b2 100644 --- a/spec/unit/models/beacon.spec.ts +++ b/spec/unit/models/beacon.spec.ts @@ -165,6 +165,55 @@ describe('Beacon', () => { expect(beacon.isLive).toEqual(false); }); + it('returns true when beacon timestamp is one minute in the future', () => { + const beaconStartsInOneMin = makeBeaconInfoEvent( + userId2, + roomId, + { + timeout: HOUR_MS, + isLive: true, + timestamp: now + 60000, + }, + '$user2live123', + ); + const beacon = new Beacon(beaconStartsInOneMin); + expect(beacon.isLive).toEqual(true); + }); + + it('returns true when beacon timestamp is one minute before expiry', () => { + // this test case is to check the start time leniency doesn't affect + // strict expiry time checks + const expiresInOneMin = makeBeaconInfoEvent( + userId2, + roomId, + { + timeout: HOUR_MS, + isLive: true, + timestamp: now - HOUR_MS + 60000, + }, + '$user2live123', + ); + const beacon = new Beacon(expiresInOneMin); + expect(beacon.isLive).toEqual(true); + }); + + it('returns false when beacon timestamp is one minute after expiry', () => { + // this test case is to check the start time leniency doesn't affect + // strict expiry time checks + const expiredOneMinAgo = makeBeaconInfoEvent( + userId2, + roomId, + { + timeout: HOUR_MS, + isLive: true, + timestamp: now - HOUR_MS - 60000, + }, + '$user2live123', + ); + const beacon = new Beacon(expiredOneMinAgo); + expect(beacon.isLive).toEqual(false); + }); + it('returns true when beacon was created in past and not yet expired', () => { // liveBeaconEvent was created 1 hour ago const beacon = new Beacon(liveBeaconEvent); @@ -363,6 +412,58 @@ describe('Beacon', () => { expect(emitSpy).not.toHaveBeenCalled(); }); + describe('when beacon is live with a start timestamp is in the future', () => { + it('ignores locations before the beacon start timestamp', () => { + const startTimestamp = now + 60000; + const beacon = new Beacon(makeBeaconInfoEvent( + userId, + roomId, + { isLive: true, timeout: 60000, timestamp: startTimestamp }, + )); + const emitSpy = jest.spyOn(beacon, 'emit'); + + beacon.addLocations([ + // beacon has now + 60000 live period + makeBeaconEvent( + userId, + { + beaconInfoId: beacon.beaconInfoId, + // now < location timestamp < beacon timestamp + timestamp: now + 10, + }, + ), + ]); + + expect(beacon.latestLocationState).toBeFalsy(); + expect(emitSpy).not.toHaveBeenCalled(); + }); + it('sets latest location when location timestamp is after startTimestamp', () => { + const startTimestamp = now + 60000; + const beacon = new Beacon(makeBeaconInfoEvent( + userId, + roomId, + { isLive: true, timeout: 60000, timestamp: startTimestamp }, + )); + const emitSpy = jest.spyOn(beacon, 'emit'); + + beacon.addLocations([ + // beacon has now + 60000 live period + makeBeaconEvent( + userId, + { + beaconInfoId: beacon.beaconInfoId, + // now < beacon timestamp < location timestamp + timestamp: startTimestamp + 10, + }, + ), + ]); + + expect(beacon.latestLocationState).toBeTruthy(); + expect(emitSpy).toHaveBeenCalled(); + }); + }); + + it('sets latest location state to most recent location', () => { const beacon = new Beacon(makeBeaconInfoEvent(userId, roomId, { isLive: true, timeout: 60000 })); const emitSpy = jest.spyOn(beacon, 'emit'); From 0055e2238203c1556060109ff7d19a99d769ae25 Mon Sep 17 00:00:00 2001 From: Kerry Archibald Date: Thu, 16 Jun 2022 12:43:29 +0200 Subject: [PATCH 3/3] add start time leniency to beacon liveness check --- spec/unit/models/beacon.spec.ts | 13 ++++++------- src/models/beacon.ts | 10 +++++++++- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/spec/unit/models/beacon.spec.ts b/spec/unit/models/beacon.spec.ts index c68d9d803b2..7fb79fe2dce 100644 --- a/spec/unit/models/beacon.spec.ts +++ b/spec/unit/models/beacon.spec.ts @@ -421,7 +421,7 @@ describe('Beacon', () => { { isLive: true, timeout: 60000, timestamp: startTimestamp }, )); const emitSpy = jest.spyOn(beacon, 'emit'); - + beacon.addLocations([ // beacon has now + 60000 live period makeBeaconEvent( @@ -433,7 +433,7 @@ describe('Beacon', () => { }, ), ]); - + expect(beacon.latestLocationState).toBeFalsy(); expect(emitSpy).not.toHaveBeenCalled(); }); @@ -442,12 +442,12 @@ describe('Beacon', () => { const beacon = new Beacon(makeBeaconInfoEvent( userId, roomId, - { isLive: true, timeout: 60000, timestamp: startTimestamp }, + { isLive: true, timeout: 600000, timestamp: startTimestamp }, )); const emitSpy = jest.spyOn(beacon, 'emit'); - + beacon.addLocations([ - // beacon has now + 60000 live period + // beacon has now + 600000 live period makeBeaconEvent( userId, { @@ -457,13 +457,12 @@ describe('Beacon', () => { }, ), ]); - + expect(beacon.latestLocationState).toBeTruthy(); expect(emitSpy).toHaveBeenCalled(); }); }); - it('sets latest location state to most recent location', () => { const beacon = new Beacon(makeBeaconInfoEvent(userId, roomId, { isLive: true, timeout: 60000 })); const emitSpy = jest.spyOn(beacon, 'emit'); diff --git a/src/models/beacon.ts b/src/models/beacon.ts index 05562574322..ddc72fddbd8 100644 --- a/src/models/beacon.ts +++ b/src/models/beacon.ts @@ -185,8 +185,16 @@ export class Beacon extends TypedEventEmitter Date.now() ? + this._beaconInfo?.timestamp - 360000 /* 6min */ : + this._beaconInfo?.timestamp; this._isLive = this._beaconInfo?.live && - isTimestampInDuration(this._beaconInfo?.timestamp, this._beaconInfo?.timeout, Date.now()); + isTimestampInDuration(startTimestamp, this._beaconInfo?.timeout, Date.now()); if (prevLiveness !== this.isLive) { this.emit(BeaconEvent.LivenessChange, this.isLive, this);