mirror of
https://github.com/bitwarden/browser
synced 2025-12-22 19:23:52 +00:00
Ps/pm 5965/better config polling (#8325)
* Create tracker that can await until expected observables are received. * Test dates are almost equal * Remove unused class method * Allow for updating active account in accout service fake * Correct observable tracker behavior Clarify documentation * Transition config service to state provider Updates the config fetching behavior to be lazy and ensure that any emitted value has been updated if older than a configurable value (statically compiled). If desired, config fetching can be ensured fresh through an async. * Update calls to config service in DI and bootstrapping * Migrate account server configs * Fix global config fetching * Test migration rollback * Adhere to implementation naming convention * Adhere to abstract class naming convention * Complete config abstraction rename * Remove unnecessary cli config service * Fix builds * Validate observable does not complete * Use token service to determine authed or unauthed config pull * Remove superfluous factory config * Name describe blocks after the thing they test * Remove implementation documentation Unfortunately the experience when linking to external documentation is quite poor. Instead of following the link and retrieving docs, you get a link that can be clicked to take you out of context to the docs. No link _does_ retrieve docs, but lacks indication in the implementation that documentation exists at all. On the balance, removing the link is the better experience. * Fix storybook
This commit is contained in:
Matt Gibson
GitHub
@@ -1,200 +1,264 @@
|
||||
import { MockProxy, mock } from "jest-mock-extended";
|
||||
import { ReplaySubject, skip, take } from "rxjs";
|
||||
/**
|
||||
* need to update test environment so structuredClone works appropriately
|
||||
* @jest-environment ../../libs/shared/test.environment.ts
|
||||
*/
|
||||
|
||||
import { FakeStateProvider, mockAccountServiceWith } from "../../../../spec";
|
||||
import { AuthService } from "../../../auth/abstractions/auth.service";
|
||||
import { AuthenticationStatus } from "../../../auth/enums/authentication-status";
|
||||
import { mock } from "jest-mock-extended";
|
||||
import { Subject, firstValueFrom, of } from "rxjs";
|
||||
|
||||
import {
|
||||
FakeGlobalState,
|
||||
FakeSingleUserState,
|
||||
FakeStateProvider,
|
||||
awaitAsync,
|
||||
mockAccountServiceWith,
|
||||
} from "../../../../spec";
|
||||
import { subscribeTo } from "../../../../spec/observable-tracker";
|
||||
import { UserId } from "../../../types/guid";
|
||||
import { ConfigApiServiceAbstraction } from "../../abstractions/config/config-api.service.abstraction";
|
||||
import { ServerConfig } from "../../abstractions/config/server-config";
|
||||
import { Environment, EnvironmentService } from "../../abstractions/environment.service";
|
||||
import { LogService } from "../../abstractions/log.service";
|
||||
import { StateService } from "../../abstractions/state.service";
|
||||
import { Utils } from "../../misc/utils";
|
||||
import { ServerConfigData } from "../../models/data/server-config.data";
|
||||
import {
|
||||
EnvironmentServerConfigResponse,
|
||||
ServerConfigResponse,
|
||||
ThirdPartyServerConfigResponse,
|
||||
} from "../../models/response/server-config.response";
|
||||
import { StateProvider } from "../../state";
|
||||
|
||||
import { ConfigService } from "./config.service";
|
||||
import {
|
||||
ApiUrl,
|
||||
DefaultConfigService,
|
||||
RETRIEVAL_INTERVAL,
|
||||
GLOBAL_SERVER_CONFIGURATIONS,
|
||||
USER_SERVER_CONFIG,
|
||||
} from "./default-config.service";
|
||||
|
||||
describe("ConfigService", () => {
|
||||
let stateService: MockProxy<StateService>;
|
||||
let configApiService: MockProxy<ConfigApiServiceAbstraction>;
|
||||
let authService: MockProxy<AuthService>;
|
||||
let environmentService: MockProxy<EnvironmentService>;
|
||||
let logService: MockProxy<LogService>;
|
||||
let replaySubject: ReplaySubject<Environment>;
|
||||
let stateProvider: StateProvider;
|
||||
|
||||
let serverResponseCount: number; // increments to track distinct responses received from server
|
||||
|
||||
// Observables will start emitting as soon as this is created, so only create it
|
||||
// after everything is mocked
|
||||
const configServiceFactory = () => {
|
||||
const configService = new ConfigService(
|
||||
stateService,
|
||||
configApiService,
|
||||
authService,
|
||||
environmentService,
|
||||
logService,
|
||||
stateProvider,
|
||||
);
|
||||
configService.init();
|
||||
return configService;
|
||||
};
|
||||
const configApiService = mock<ConfigApiServiceAbstraction>();
|
||||
const environmentService = mock<EnvironmentService>();
|
||||
const logService = mock<LogService>();
|
||||
let stateProvider: FakeStateProvider;
|
||||
let globalState: FakeGlobalState<Record<ApiUrl, ServerConfig>>;
|
||||
let userState: FakeSingleUserState<ServerConfig>;
|
||||
const activeApiUrl = apiUrl(0);
|
||||
const userId = "userId" as UserId;
|
||||
const accountService = mockAccountServiceWith(userId);
|
||||
const tooOld = new Date(Date.now() - 1.1 * RETRIEVAL_INTERVAL);
|
||||
|
||||
beforeEach(() => {
|
||||
stateService = mock();
|
||||
configApiService = mock();
|
||||
authService = mock();
|
||||
environmentService = mock();
|
||||
logService = mock();
|
||||
replaySubject = new ReplaySubject<Environment>(1);
|
||||
const accountService = mockAccountServiceWith("0" as UserId);
|
||||
stateProvider = new FakeStateProvider(accountService);
|
||||
|
||||
environmentService.environment$ = replaySubject.asObservable();
|
||||
|
||||
serverResponseCount = 1;
|
||||
configApiService.get.mockImplementation(() =>
|
||||
Promise.resolve(serverConfigResponseFactory("server" + serverResponseCount++)),
|
||||
);
|
||||
|
||||
jest.useFakeTimers();
|
||||
globalState = stateProvider.global.getFake(GLOBAL_SERVER_CONFIGURATIONS);
|
||||
userState = stateProvider.singleUser.getFake(userId, USER_SERVER_CONFIG);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.useRealTimers();
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
it("Uses storage as fallback", (done) => {
|
||||
const storedConfigData = serverConfigDataFactory("storedConfig");
|
||||
stateService.getServerConfig.mockResolvedValueOnce(storedConfigData);
|
||||
describe.each([null, userId])("active user: %s", (activeUserId) => {
|
||||
let sut: DefaultConfigService;
|
||||
|
||||
configApiService.get.mockRejectedValueOnce(new Error("Unable to fetch"));
|
||||
|
||||
const configService = configServiceFactory();
|
||||
|
||||
configService.serverConfig$.pipe(take(1)).subscribe((config) => {
|
||||
expect(config).toEqual(new ServerConfig(storedConfigData));
|
||||
expect(stateService.getServerConfig).toHaveBeenCalledTimes(1);
|
||||
expect(stateService.setServerConfig).not.toHaveBeenCalled();
|
||||
done();
|
||||
beforeAll(async () => {
|
||||
await accountService.switchAccount(activeUserId);
|
||||
});
|
||||
|
||||
configService.triggerServerConfigFetch();
|
||||
});
|
||||
|
||||
it("Stream does not error out if fetch fails", (done) => {
|
||||
const storedConfigData = serverConfigDataFactory("storedConfig");
|
||||
stateService.getServerConfig.mockResolvedValueOnce(storedConfigData);
|
||||
|
||||
const configService = configServiceFactory();
|
||||
|
||||
configService.serverConfig$.pipe(skip(1), take(1)).subscribe((config) => {
|
||||
try {
|
||||
expect(config.gitHash).toEqual("server1");
|
||||
done();
|
||||
} catch (e) {
|
||||
done(e);
|
||||
}
|
||||
});
|
||||
|
||||
configApiService.get.mockRejectedValueOnce(new Error("Unable to fetch"));
|
||||
configService.triggerServerConfigFetch();
|
||||
|
||||
configApiService.get.mockResolvedValueOnce(serverConfigResponseFactory("server1"));
|
||||
configService.triggerServerConfigFetch();
|
||||
});
|
||||
|
||||
describe("Fetches config from server", () => {
|
||||
beforeEach(() => {
|
||||
stateService.getServerConfig.mockResolvedValueOnce(null);
|
||||
environmentService.environment$ = of(environmentFactory(activeApiUrl));
|
||||
sut = new DefaultConfigService(
|
||||
configApiService,
|
||||
environmentService,
|
||||
logService,
|
||||
stateProvider,
|
||||
);
|
||||
});
|
||||
|
||||
it.each<number | jest.DoneCallback>([1, 2, 3])(
|
||||
"after %p hour/s",
|
||||
(hours: number, done: jest.DoneCallback) => {
|
||||
const configService = configServiceFactory();
|
||||
describe("serverConfig$", () => {
|
||||
it.each([{}, null])("handles null stored state", async (globalTestState) => {
|
||||
globalState.stateSubject.next(globalTestState);
|
||||
userState.nextState(null);
|
||||
await expect(firstValueFrom(sut.serverConfig$)).resolves.not.toThrow();
|
||||
});
|
||||
|
||||
// skip previous hours (if any)
|
||||
configService.serverConfig$.pipe(skip(hours - 1), take(1)).subscribe((config) => {
|
||||
try {
|
||||
expect(config.gitHash).toEqual("server" + hours);
|
||||
expect(configApiService.get).toHaveBeenCalledTimes(hours);
|
||||
done();
|
||||
} catch (e) {
|
||||
done(e);
|
||||
}
|
||||
describe.each(["stale", "missing"])("%s config", (configStateDescription) => {
|
||||
const userStored =
|
||||
configStateDescription === "missing"
|
||||
? null
|
||||
: serverConfigFactory(activeApiUrl + userId, tooOld);
|
||||
const globalStored =
|
||||
configStateDescription === "missing"
|
||||
? {}
|
||||
: {
|
||||
[activeApiUrl]: serverConfigFactory(activeApiUrl, tooOld),
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
globalState.stateSubject.next(globalStored);
|
||||
userState.nextState(userStored);
|
||||
});
|
||||
|
||||
const oneHourInMs = 1000 * 3600;
|
||||
jest.advanceTimersByTime(oneHourInMs * hours + 1);
|
||||
},
|
||||
);
|
||||
// sanity check
|
||||
test("authed and unauthorized state are different", () => {
|
||||
expect(globalStored[activeApiUrl]).not.toEqual(userStored);
|
||||
});
|
||||
|
||||
it("when environment URLs change", (done) => {
|
||||
const configService = configServiceFactory();
|
||||
describe("fail to fetch", () => {
|
||||
beforeEach(() => {
|
||||
configApiService.get.mockRejectedValue(new Error("Unable to fetch"));
|
||||
});
|
||||
|
||||
configService.serverConfig$.pipe(take(1)).subscribe((config) => {
|
||||
try {
|
||||
expect(config.gitHash).toEqual("server1");
|
||||
done();
|
||||
} catch (e) {
|
||||
done(e);
|
||||
}
|
||||
it("uses storage as fallback", async () => {
|
||||
const actual = await firstValueFrom(sut.serverConfig$);
|
||||
expect(actual).toEqual(activeUserId ? userStored : globalStored[activeApiUrl]);
|
||||
expect(configApiService.get).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("does not error out when fetch fails", async () => {
|
||||
await expect(firstValueFrom(sut.serverConfig$)).resolves.not.toThrow();
|
||||
expect(configApiService.get).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("logs an error when unable to fetch", async () => {
|
||||
await firstValueFrom(sut.serverConfig$);
|
||||
|
||||
expect(logService.error).toHaveBeenCalledWith(
|
||||
`Unable to fetch ServerConfig from ${activeApiUrl}: Unable to fetch`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("fetch success", () => {
|
||||
const response = serverConfigResponseFactory();
|
||||
const newConfig = new ServerConfig(new ServerConfigData(response));
|
||||
|
||||
it("should be a new config", async () => {
|
||||
expect(newConfig).not.toEqual(activeUserId ? userStored : globalStored[activeApiUrl]);
|
||||
});
|
||||
|
||||
it("fetches config from server when it's older than an hour", async () => {
|
||||
await firstValueFrom(sut.serverConfig$);
|
||||
|
||||
expect(configApiService.get).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("returns the updated config", async () => {
|
||||
configApiService.get.mockResolvedValue(response);
|
||||
|
||||
const actual = await firstValueFrom(sut.serverConfig$);
|
||||
|
||||
// This is the time the response is converted to a config
|
||||
expect(actual.utcDate).toAlmostEqual(newConfig.utcDate, 1000);
|
||||
delete actual.utcDate;
|
||||
delete newConfig.utcDate;
|
||||
|
||||
expect(actual).toEqual(newConfig);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
replaySubject.next(null);
|
||||
});
|
||||
describe("fresh configuration", () => {
|
||||
const userStored = serverConfigFactory(activeApiUrl + userId);
|
||||
const globalStored = {
|
||||
[activeApiUrl]: serverConfigFactory(activeApiUrl),
|
||||
};
|
||||
beforeEach(() => {
|
||||
globalState.stateSubject.next(globalStored);
|
||||
userState.nextState(userStored);
|
||||
});
|
||||
it("does not fetch from server", async () => {
|
||||
await firstValueFrom(sut.serverConfig$);
|
||||
|
||||
it("when triggerServerConfigFetch() is called", (done) => {
|
||||
const configService = configServiceFactory();
|
||||
expect(configApiService.get).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
configService.serverConfig$.pipe(take(1)).subscribe((config) => {
|
||||
try {
|
||||
expect(config.gitHash).toEqual("server1");
|
||||
done();
|
||||
} catch (e) {
|
||||
done(e);
|
||||
}
|
||||
it("uses stored value", async () => {
|
||||
const actual = await firstValueFrom(sut.serverConfig$);
|
||||
expect(actual).toEqual(activeUserId ? userStored : globalStored[activeApiUrl]);
|
||||
});
|
||||
|
||||
it("does not complete after emit", async () => {
|
||||
const emissions = [];
|
||||
const subscription = sut.serverConfig$.subscribe((v) => emissions.push(v));
|
||||
await awaitAsync();
|
||||
expect(emissions.length).toBe(1);
|
||||
expect(subscription.closed).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
configService.triggerServerConfigFetch();
|
||||
});
|
||||
});
|
||||
|
||||
it("Saves server config to storage when the user is logged in", (done) => {
|
||||
stateService.getServerConfig.mockResolvedValueOnce(null);
|
||||
authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Locked);
|
||||
const configService = configServiceFactory();
|
||||
describe("environment change", () => {
|
||||
let sut: DefaultConfigService;
|
||||
let environmentSubject: Subject<Environment>;
|
||||
|
||||
configService.serverConfig$.pipe(take(1)).subscribe(() => {
|
||||
try {
|
||||
expect(stateService.setServerConfig).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ gitHash: "server1" }),
|
||||
);
|
||||
done();
|
||||
} catch (e) {
|
||||
done(e);
|
||||
}
|
||||
beforeAll(async () => {
|
||||
// updating environment with an active account is undefined behavior
|
||||
await accountService.switchAccount(null);
|
||||
});
|
||||
|
||||
configService.triggerServerConfigFetch();
|
||||
beforeEach(() => {
|
||||
environmentSubject = new Subject<Environment>();
|
||||
environmentService.environment$ = environmentSubject;
|
||||
sut = new DefaultConfigService(
|
||||
configApiService,
|
||||
environmentService,
|
||||
logService,
|
||||
stateProvider,
|
||||
);
|
||||
});
|
||||
|
||||
describe("serverConfig$", () => {
|
||||
it("emits a new config when the environment changes", async () => {
|
||||
const globalStored = {
|
||||
[apiUrl(0)]: serverConfigFactory(apiUrl(0)),
|
||||
[apiUrl(1)]: serverConfigFactory(apiUrl(1)),
|
||||
};
|
||||
globalState.stateSubject.next(globalStored);
|
||||
|
||||
const spy = subscribeTo(sut.serverConfig$);
|
||||
|
||||
environmentSubject.next(environmentFactory(apiUrl(0)));
|
||||
environmentSubject.next(environmentFactory(apiUrl(1)));
|
||||
|
||||
const expected = [globalStored[apiUrl(0)], globalStored[apiUrl(1)]];
|
||||
|
||||
const actual = await spy.pauseUntilReceived(2);
|
||||
expect(actual.length).toBe(2);
|
||||
|
||||
// validate dates this is done separately because the dates are created when ServerConfig is initialized
|
||||
expect(actual[0].utcDate).toAlmostEqual(expected[0].utcDate, 1000);
|
||||
expect(actual[1].utcDate).toAlmostEqual(expected[1].utcDate, 1000);
|
||||
delete actual[0].utcDate;
|
||||
delete actual[1].utcDate;
|
||||
delete expected[0].utcDate;
|
||||
delete expected[1].utcDate;
|
||||
|
||||
expect(actual).toEqual(expected);
|
||||
spy.unsubscribe();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
function serverConfigDataFactory(gitHash: string) {
|
||||
return new ServerConfigData(serverConfigResponseFactory(gitHash));
|
||||
function apiUrl(count: number) {
|
||||
return `https://api${count}.test.com`;
|
||||
}
|
||||
|
||||
function serverConfigResponseFactory(gitHash: string) {
|
||||
function serverConfigFactory(hash: string, date: Date = new Date()) {
|
||||
const config = new ServerConfig(serverConfigDataFactory(hash));
|
||||
config.utcDate = date;
|
||||
return config;
|
||||
}
|
||||
|
||||
function serverConfigDataFactory(hash?: string) {
|
||||
return new ServerConfigData(serverConfigResponseFactory(hash));
|
||||
}
|
||||
|
||||
function serverConfigResponseFactory(hash?: string) {
|
||||
return new ServerConfigResponse({
|
||||
version: "myConfigVersion",
|
||||
gitHash: gitHash,
|
||||
gitHash: hash ?? Utils.newGuid(), // Use optional git hash to store uniqueness value
|
||||
server: new ThirdPartyServerConfigResponse({
|
||||
name: "myThirdPartyServer",
|
||||
url: "www.example.com",
|
||||
@@ -209,3 +273,9 @@ function serverConfigResponseFactory(gitHash: string) {
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function environmentFactory(apiUrl: string) {
|
||||
return {
|
||||
getApiUrl: () => apiUrl,
|
||||
} as Environment;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user