From 33ed1a41c0bf5f262bde2315296ca5a9adc96e20 Mon Sep 17 00:00:00 2001 From: huntj88 Date: Thu, 17 Aug 2023 17:47:03 -0500 Subject: [PATCH] Fixed getClientInfo receiving wrong format from android (#705) Co-authored-by: James Hunt --- packages/live-share/package.json | 4 +- packages/live-share/src/LiveShareRuntime.ts | 3 +- .../BackwardsCompatibilityHostDecorator.ts | 43 ++------------- .../src/internals/BaseHostDecorator.ts | 53 ++++++++++++++++++ .../src/internals/FormatFixHostDecorator.ts | 55 +++++++++++++++++++ .../src/internals/LiveShareHostDecorator.ts | 49 ++--------------- packages/live-share/src/internals/index.ts | 1 + .../test/FormatFixHostDecorator.spec.ts | 53 ++++++++++++++++++ 8 files changed, 175 insertions(+), 86 deletions(-) create mode 100644 packages/live-share/src/internals/BaseHostDecorator.ts create mode 100644 packages/live-share/src/internals/FormatFixHostDecorator.ts create mode 100644 packages/live-share/src/internals/test/FormatFixHostDecorator.spec.ts diff --git a/packages/live-share/package.json b/packages/live-share/package.json index e35007c36..507fceb90 100644 --- a/packages/live-share/package.json +++ b/packages/live-share/package.json @@ -9,8 +9,8 @@ "scripts": { "clean": "npx shx rm -rf bin", "build": "tsc -p tsconfig.build.json", - "test": "ts-mocha --config ./src/test/.mocharc.js src/**/*.spec.ts --timeout 10000", - "test:debug": "ts-mocha --config ./src/test/.mocharc.js src/**/*.spec.ts --inspect-brk", + "test": "ts-mocha --config ./src/test/.mocharc.js src --recursive --extension .spec.ts --timeout 10000", + "test:debug": "ts-mocha --config ./src/test/.mocharc.js src --recursive --extension .spec.ts --inspect-brk", "test:coverage": "nyc --reporter=html --reporter=text --reporter=text-summary npm test", "test:race": "node test-for-race-conditions.js" }, diff --git a/packages/live-share/src/LiveShareRuntime.ts b/packages/live-share/src/LiveShareRuntime.ts index 501cfc9f5..8e8328f04 100644 --- a/packages/live-share/src/LiveShareRuntime.ts +++ b/packages/live-share/src/LiveShareRuntime.ts @@ -14,6 +14,7 @@ import { RoleVerifier, isTimestampProvider, LiveObjectManager, + FormatFixHostDecorator, } from "./internals"; import { IAzureAudience } from "@fluidframework/azure-client"; @@ -46,7 +47,7 @@ export class LiveShareRuntime { // LiveShareHostDecorator is used as a thin caching layer for some host APIs. this._host = decorate ? new BackwardsCompatibilityHostDecorator( - new LiveShareHostDecorator(host) + new LiveShareHostDecorator(new FormatFixHostDecorator(host)) ) : host; this._timestampProvider = timestampProvider diff --git a/packages/live-share/src/internals/BackwardsCompatibilityHostDecorator.ts b/packages/live-share/src/internals/BackwardsCompatibilityHostDecorator.ts index 0bb67a175..111b71e67 100644 --- a/packages/live-share/src/internals/BackwardsCompatibilityHostDecorator.ts +++ b/packages/live-share/src/internals/BackwardsCompatibilityHostDecorator.ts @@ -3,14 +3,8 @@ * Licensed under the Microsoft Live Share SDK License. */ -import { - ILiveShareHost, - IFluidTenantInfo, - IFluidContainerInfo, - INtpTimeInfo, - UserMeetingRole, - IClientInfo, -} from "../interfaces"; +import { ILiveShareHost, UserMeetingRole, IClientInfo } from "../interfaces"; +import { BaseHostDecorator } from "./BaseHostDecorator"; import { RequestCache } from "./RequestCache"; import { isErrorLike, @@ -43,7 +37,7 @@ export interface BackwardsCompatibilityGetClientInfoRetrySchedule { * * For internal use only. Will be deleted. */ -export class BackwardsCompatibilityHostDecorator implements ILiveShareHost { +export class BackwardsCompatibilityHostDecorator extends BaseHostDecorator { private readonly _userRolesRequestCache: RequestCache = new RequestCache(CACHE_LIFETIME); @@ -57,34 +51,13 @@ export class BackwardsCompatibilityHostDecorator implements ILiveShareHost { * _host would be `BackwardsCompatibilityHostDecorator` decorator: `new BackwardsCompatibilityHostDecorator(new LiveShareHostDecorator(teamsJsHost))` */ constructor( - private readonly _host: ILiveShareHost & + readonly _host: ILiveShareHost & BackwardsCompatibilityGetClientInfoRetrySchedule ) { + super(_host); this.warmupCheckGetClientInfoExists(); } - public getFluidTenantInfo(): Promise { - return this._host.getFluidTenantInfo(); - } - - public getFluidToken(containerId?: string): Promise { - return this._host.getFluidToken(containerId); - } - - public getFluidContainerId(): Promise { - return this._host.getFluidContainerId(); - } - - public setFluidContainerId( - containerId: string - ): Promise { - return this._host.setFluidContainerId(containerId); - } - - public getNtpTime(): Promise { - return this._host.getNtpTime(); - } - /** * @deprecated */ @@ -193,12 +166,6 @@ export class BackwardsCompatibilityHostDecorator implements ILiveShareHost { return await this.getClientInfo(clientId); } - public async registerClientId( - clientId: string - ): Promise { - return this._host.registerClientId(clientId); - } - private warmupCheckGetClientInfoExists() { if (this._getClientInfoExists) return; diff --git a/packages/live-share/src/internals/BaseHostDecorator.ts b/packages/live-share/src/internals/BaseHostDecorator.ts new file mode 100644 index 000000000..8b15aca77 --- /dev/null +++ b/packages/live-share/src/internals/BaseHostDecorator.ts @@ -0,0 +1,53 @@ +import { + ILiveShareHost, + IFluidTenantInfo, + IFluidContainerInfo, + INtpTimeInfo, + UserMeetingRole, + IClientInfo, +} from "../interfaces"; + +export class BaseHostDecorator implements ILiveShareHost { + /** + * @hidden + */ + constructor(readonly _host: ILiveShareHost) {} + + public async registerClientId( + clientId: string + ): Promise { + return this._host.registerClientId(clientId); + } + public async getClientRoles( + clientId: string + ): Promise { + return this._host.getClientRoles(clientId); + } + public async getClientInfo( + clientId: string + ): Promise { + return this._host.getClientInfo(clientId); + } + + public getFluidTenantInfo(): Promise { + return this._host.getFluidTenantInfo(); + } + + public getFluidToken(containerId?: string): Promise { + return this._host.getFluidToken(containerId); + } + + public getFluidContainerId(): Promise { + return this._host.getFluidContainerId(); + } + + public setFluidContainerId( + containerId: string + ): Promise { + return this._host.setFluidContainerId(containerId); + } + + public getNtpTime(): Promise { + return this._host.getNtpTime(); + } +} diff --git a/packages/live-share/src/internals/FormatFixHostDecorator.ts b/packages/live-share/src/internals/FormatFixHostDecorator.ts new file mode 100644 index 000000000..5637f5c07 --- /dev/null +++ b/packages/live-share/src/internals/FormatFixHostDecorator.ts @@ -0,0 +1,55 @@ +import { UserMeetingRole, IClientInfo } from "../interfaces"; +import { BaseHostDecorator } from "./BaseHostDecorator"; + +import { isClientRolesResponse } from "./type-guards"; + +/** + * @hidden + */ +export class FormatFixHostDecorator extends BaseHostDecorator { + public async getClientInfo( + clientId: string + ): Promise { + const clientInfo = await this._host.getClientInfo(clientId); + if (isAndroidClientInfoBugFormat(clientInfo)) { + return { + userId: clientInfo.lock, + roles: clientInfo._loadStates, + displayName: clientInfo.internalState, + }; + } + return clientInfo; + } +} + +/** + * @hidden + */ +function isAndroidClientInfoBugFormat( + value: any +): value is AndroidClientInfoBugFormat { + return ( + typeof value?.lock === "string" && + isClientRolesResponse(value?._loadStates) && + (typeof value?.internalState === "string" || + value?.internalState === undefined) + ); +} + +/** + * @hidden + */ +interface AndroidClientInfoBugFormat { + /** + * The user identifier that corresponds to the provided client identifier. + */ + lock: string; + /** + * List of roles of the user. + */ + _loadStates: UserMeetingRole[]; + /** + * Optional. The display name for the user. + */ + internalState?: string; +} diff --git a/packages/live-share/src/internals/LiveShareHostDecorator.ts b/packages/live-share/src/internals/LiveShareHostDecorator.ts index 6ccdb374d..4c642d34f 100644 --- a/packages/live-share/src/internals/LiveShareHostDecorator.ts +++ b/packages/live-share/src/internals/LiveShareHostDecorator.ts @@ -3,15 +3,9 @@ * Licensed under the Microsoft Live Share SDK License. */ -import { - ILiveShareHost, - IFluidTenantInfo, - IFluidContainerInfo, - INtpTimeInfo, - UserMeetingRole, - IClientInfo, -} from "../interfaces"; +import { UserMeetingRole, IClientInfo } from "../interfaces"; import { BackwardsCompatibilityGetClientInfoRetrySchedule } from "./BackwardsCompatibilityHostDecorator"; +import { BaseHostDecorator } from "./BaseHostDecorator"; import { RequestCache } from "./RequestCache"; import { isErrorLike, @@ -29,7 +23,8 @@ const CACHE_LIFETIME = 4 * 1000; * Live Share Host decorator used to reduce rapid duplicate requests. */ export class LiveShareHostDecorator - implements ILiveShareHost, BackwardsCompatibilityGetClientInfoRetrySchedule + extends BaseHostDecorator + implements BackwardsCompatibilityGetClientInfoRetrySchedule { private readonly _registerRequestCache: RequestCache = new RequestCache(CACHE_LIFETIME); @@ -37,42 +32,6 @@ export class LiveShareHostDecorator IClientInfo | undefined > = new RequestCache(CACHE_LIFETIME); - /** - * @hidden - */ - constructor(private readonly _host: ILiveShareHost) {} - - public getFluidTenantInfo(): Promise { - return this._host.getFluidTenantInfo(); - } - - public getFluidToken(containerId?: string): Promise { - return this._host.getFluidToken(containerId); - } - - public getFluidContainerId(): Promise { - return this._host.getFluidContainerId(); - } - - public setFluidContainerId( - containerId: string - ): Promise { - return this._host.setFluidContainerId(containerId); - } - - public getNtpTime(): Promise { - return this._host.getNtpTime(); - } - - /** - * @deprecated - */ - public async getClientRoles( - clientId: string - ): Promise { - return this._host.getClientRoles(clientId); - } - public async getClientInfo( clientId: string, lateFinish?: () => void, // TODO: delete this and below (not a breaking change to remove, see InternalDontUseGetClientInfoRetryPolyfill) diff --git a/packages/live-share/src/internals/index.ts b/packages/live-share/src/internals/index.ts index b6abef5bc..6892862e5 100644 --- a/packages/live-share/src/internals/index.ts +++ b/packages/live-share/src/internals/index.ts @@ -15,3 +15,4 @@ export * from "./type-guards"; export * from "./internal-interfaces"; export * from "./LiveObjectManager"; export * from "./ContainerSynchronizer"; +export * from "./FormatFixHostDecorator"; diff --git a/packages/live-share/src/internals/test/FormatFixHostDecorator.spec.ts b/packages/live-share/src/internals/test/FormatFixHostDecorator.spec.ts new file mode 100644 index 000000000..b2bd3f3bd --- /dev/null +++ b/packages/live-share/src/internals/test/FormatFixHostDecorator.spec.ts @@ -0,0 +1,53 @@ +import { ILiveShareHost, UserMeetingRole, IClientInfo } from "../../interfaces"; +import { strict as assert } from "assert"; +import { FormatFixHostDecorator } from ".."; + +class HostWithAndroidBug { + constructor(private result: unknown) {} + public async getClientInfo( + clientId: string + ): Promise { + return this.result as IClientInfo; + } +} + +describe("FormatFixHostDecorator tests", function () { + it("should map android bug format to expected format", async () => { + const hostResponse = { + lock: "userId", + _loadStates: [UserMeetingRole.organizer], + internalState: "displayName", + }; + const host = new HostWithAndroidBug( + hostResponse + ) as unknown as ILiveShareHost; + const hostWithMapper = new FormatFixHostDecorator(host); + const expectedResult = { + userId: "userId", + roles: [UserMeetingRole.organizer], + displayName: "displayName", + }; + const result = await hostWithMapper.getClientInfo("test"); + assert( + JSON.stringify(result) === JSON.stringify(expectedResult), + `unexpected result: ${JSON.stringify(result)}` + ); + }); + + it("should not map if already expected format", async () => { + const hostResponse = { + userId: "userId", + roles: [UserMeetingRole.organizer], + displayName: "displayName", + }; + const host = new HostWithAndroidBug( + hostResponse + ) as unknown as ILiveShareHost; + const hostWithMapper = new FormatFixHostDecorator(host); + const result = await hostWithMapper.getClientInfo("test"); + assert( + JSON.stringify(result) === JSON.stringify(hostResponse), + `unexpected result: ${JSON.stringify(result)}` + ); + }); +});