Skip to content

Commit

Permalink
Fixed getClientInfo receiving wrong format from android (#705)
Browse files Browse the repository at this point in the history
Co-authored-by: James Hunt <[email protected]>
  • Loading branch information
huntj88 and James Hunt committed Aug 17, 2023
1 parent e17b1fe commit 33ed1a4
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 86 deletions.
4 changes: 2 additions & 2 deletions packages/live-share/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
3 changes: 2 additions & 1 deletion packages/live-share/src/LiveShareRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
RoleVerifier,
isTimestampProvider,
LiveObjectManager,
FormatFixHostDecorator,
} from "./internals";
import { IAzureAudience } from "@fluidframework/azure-client";

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<UserMeetingRole[]> =
new RequestCache(CACHE_LIFETIME);

Expand All @@ -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<IFluidTenantInfo> {
return this._host.getFluidTenantInfo();
}

public getFluidToken(containerId?: string): Promise<string> {
return this._host.getFluidToken(containerId);
}

public getFluidContainerId(): Promise<IFluidContainerInfo> {
return this._host.getFluidContainerId();
}

public setFluidContainerId(
containerId: string
): Promise<IFluidContainerInfo> {
return this._host.setFluidContainerId(containerId);
}

public getNtpTime(): Promise<INtpTimeInfo> {
return this._host.getNtpTime();
}

/**
* @deprecated
*/
Expand Down Expand Up @@ -193,12 +166,6 @@ export class BackwardsCompatibilityHostDecorator implements ILiveShareHost {
return await this.getClientInfo(clientId);
}

public async registerClientId(
clientId: string
): Promise<UserMeetingRole[]> {
return this._host.registerClientId(clientId);
}

private warmupCheckGetClientInfoExists() {
if (this._getClientInfoExists) return;

Expand Down
53 changes: 53 additions & 0 deletions packages/live-share/src/internals/BaseHostDecorator.ts
Original file line number Diff line number Diff line change
@@ -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<UserMeetingRole[]> {
return this._host.registerClientId(clientId);
}
public async getClientRoles(
clientId: string
): Promise<UserMeetingRole[] | undefined> {
return this._host.getClientRoles(clientId);
}
public async getClientInfo(
clientId: string
): Promise<IClientInfo | undefined> {
return this._host.getClientInfo(clientId);
}

public getFluidTenantInfo(): Promise<IFluidTenantInfo> {
return this._host.getFluidTenantInfo();
}

public getFluidToken(containerId?: string): Promise<string> {
return this._host.getFluidToken(containerId);
}

public getFluidContainerId(): Promise<IFluidContainerInfo> {
return this._host.getFluidContainerId();
}

public setFluidContainerId(
containerId: string
): Promise<IFluidContainerInfo> {
return this._host.setFluidContainerId(containerId);
}

public getNtpTime(): Promise<INtpTimeInfo> {
return this._host.getNtpTime();
}
}
55 changes: 55 additions & 0 deletions packages/live-share/src/internals/FormatFixHostDecorator.ts
Original file line number Diff line number Diff line change
@@ -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<IClientInfo | undefined> {
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;
}
49 changes: 4 additions & 45 deletions packages/live-share/src/internals/LiveShareHostDecorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -29,50 +23,15 @@ 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<UserMeetingRole[]> =
new RequestCache(CACHE_LIFETIME);
private readonly _userInfoRequestCache: RequestCache<
IClientInfo | undefined
> = new RequestCache(CACHE_LIFETIME);

/**
* @hidden
*/
constructor(private readonly _host: ILiveShareHost) {}

public getFluidTenantInfo(): Promise<IFluidTenantInfo> {
return this._host.getFluidTenantInfo();
}

public getFluidToken(containerId?: string): Promise<string> {
return this._host.getFluidToken(containerId);
}

public getFluidContainerId(): Promise<IFluidContainerInfo> {
return this._host.getFluidContainerId();
}

public setFluidContainerId(
containerId: string
): Promise<IFluidContainerInfo> {
return this._host.setFluidContainerId(containerId);
}

public getNtpTime(): Promise<INtpTimeInfo> {
return this._host.getNtpTime();
}

/**
* @deprecated
*/
public async getClientRoles(
clientId: string
): Promise<UserMeetingRole[] | undefined> {
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)
Expand Down
1 change: 1 addition & 0 deletions packages/live-share/src/internals/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@ export * from "./type-guards";
export * from "./internal-interfaces";
export * from "./LiveObjectManager";
export * from "./ContainerSynchronizer";
export * from "./FormatFixHostDecorator";
Original file line number Diff line number Diff line change
@@ -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<IClientInfo | undefined> {
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)}`
);
});
});

0 comments on commit 33ed1a4

Please sign in to comment.