diff --git a/libs/common/src/platform/abstractions/config/config-api.service.abstraction.ts b/libs/common/src/platform/abstractions/config/config-api.service.abstraction.ts index 3c191f59ccc..0460e8c715f 100644 --- a/libs/common/src/platform/abstractions/config/config-api.service.abstraction.ts +++ b/libs/common/src/platform/abstractions/config/config-api.service.abstraction.ts @@ -5,5 +5,5 @@ export abstract class ConfigApiServiceAbstraction { /** * Fetches the server configuration for the given user. If no user is provided, the configuration will not contain user-specific context. */ - abstract get(userId: UserId | undefined): Promise; + abstract get(userId: UserId | null): Promise; } diff --git a/libs/common/src/platform/abstractions/environment.service.ts b/libs/common/src/platform/abstractions/environment.service.ts index b8931656848..86a0fbea242 100644 --- a/libs/common/src/platform/abstractions/environment.service.ts +++ b/libs/common/src/platform/abstractions/environment.service.ts @@ -95,6 +95,13 @@ export interface Environment { */ export abstract class EnvironmentService { abstract environment$: Observable; + + /** + * The environment stored in global state, when a user signs in the state stored here will become + * their user environment. + */ + abstract globalEnvironment$: Observable; + abstract cloudWebVaultUrl$: Observable; /** @@ -125,12 +132,12 @@ export abstract class EnvironmentService { * @param userId - The user id to set the cloud web vault app URL for. If null or undefined the global environment is set. * @param region - The region of the cloud web vault app. */ - abstract setCloudRegion(userId: UserId, region: Region): Promise; + abstract setCloudRegion(userId: UserId | null, region: Region): Promise; /** * Get the environment from state. Useful if you need to get the environment for another user. */ - abstract getEnvironment$(userId: UserId): Observable; + abstract getEnvironment$(userId: UserId): Observable; /** * @deprecated Use {@link getEnvironment$} instead. diff --git a/libs/common/src/platform/services/config/config-api.service.ts b/libs/common/src/platform/services/config/config-api.service.ts index f283410acea..b7ecb9c8712 100644 --- a/libs/common/src/platform/services/config/config-api.service.ts +++ b/libs/common/src/platform/services/config/config-api.service.ts @@ -10,7 +10,7 @@ export class ConfigApiService implements ConfigApiServiceAbstraction { private tokenService: TokenService, ) {} - async get(userId: UserId | undefined): Promise { + async get(userId: UserId | null): Promise { // Authentication adds extra context to config responses, if the user has an access token, we want to use it // We don't particularly care about ensuring the token is valid and not expired, just that it exists const authed: boolean = diff --git a/libs/common/src/platform/services/config/config.service.spec.ts b/libs/common/src/platform/services/config/config.service.spec.ts index ea3b56a32f1..e8a1872c4c1 100644 --- a/libs/common/src/platform/services/config/config.service.spec.ts +++ b/libs/common/src/platform/services/config/config.service.spec.ts @@ -10,9 +10,9 @@ import { FakeGlobalState, FakeSingleUserState, FakeStateProvider, - awaitAsync, mockAccountServiceWith, } from "../../../../spec"; +import { Matrix } from "../../../../spec/matrix"; import { subscribeTo } from "../../../../spec/observable-tracker"; import { AuthService } from "../../../auth/abstractions/auth.service"; import { AuthenticationStatus } from "../../../auth/enums/authentication-status"; @@ -74,7 +74,8 @@ describe("ConfigService", () => { }); beforeEach(() => { - environmentService.environment$ = environmentSubject; + Matrix.autoMockMethod(environmentService.getEnvironment$, () => environmentSubject); + environmentService.globalEnvironment$ = environmentSubject; sut = new DefaultConfigService( configApiService, environmentService, @@ -98,9 +99,12 @@ describe("ConfigService", () => { : serverConfigFactory(activeApiUrl + userId, tooOld); const globalStored = configStateDescription === "missing" - ? {} + ? { + [activeApiUrl]: null, + } : { [activeApiUrl]: serverConfigFactory(activeApiUrl, tooOld), + [activeApiUrl + "0"]: serverConfigFactory(activeApiUrl + userId, tooOld), }; beforeEach(() => { @@ -108,11 +112,6 @@ describe("ConfigService", () => { userState.nextState(userStored); }); - // sanity check - test("authed and unauthorized state are different", () => { - expect(globalStored[activeApiUrl]).not.toEqual(userStored); - }); - describe("fail to fetch", () => { beforeEach(() => { configApiService.get.mockRejectedValue(new Error("Unable to fetch")); @@ -178,6 +177,7 @@ describe("ConfigService", () => { beforeEach(() => { globalState.stateSubject.next(globalStored); userState.nextState(userStored); + Matrix.autoMockMethod(environmentService.getEnvironment$, () => environmentSubject); }); it("does not fetch from server", async () => { await firstValueFrom(sut.serverConfig$); @@ -189,21 +189,13 @@ describe("ConfigService", () => { 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); - }); }); }); }); it("gets global config when there is an locked active user", async () => { await accountService.switchAccount(userId); - environmentService.environment$ = of(environmentFactory(activeApiUrl)); + environmentService.globalEnvironment$ = of(environmentFactory(activeApiUrl)); globalState.stateSubject.next({ [activeApiUrl]: serverConfigFactory(activeApiUrl + "global"), @@ -236,7 +228,8 @@ describe("ConfigService", () => { beforeEach(() => { environmentSubject = new Subject(); - environmentService.environment$ = environmentSubject; + environmentService.globalEnvironment$ = environmentSubject; + Matrix.autoMockMethod(environmentService.getEnvironment$, () => environmentSubject); sut = new DefaultConfigService( configApiService, environmentService, @@ -327,7 +320,8 @@ describe("ConfigService", () => { beforeEach(async () => { const config = serverConfigFactory("existing-data", tooOld); - environmentService.environment$ = environmentSubject; + environmentService.globalEnvironment$ = environmentSubject; + Matrix.autoMockMethod(environmentService.getEnvironment$, () => environmentSubject); globalState.stateSubject.next({ [apiUrl(0)]: config }); userState.stateSubject.next({ diff --git a/libs/common/src/platform/services/config/default-config.service.ts b/libs/common/src/platform/services/config/default-config.service.ts index 33f86d30885..4297b65cb60 100644 --- a/libs/common/src/platform/services/config/default-config.service.ts +++ b/libs/common/src/platform/services/config/default-config.service.ts @@ -1,17 +1,18 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { combineLatest, + distinctUntilChanged, firstValueFrom, map, mergeWith, NEVER, Observable, of, - shareReplay, + ReplaySubject, + share, Subject, switchMap, tap, + timer, } from "rxjs"; import { SemVer } from "semver"; @@ -50,11 +51,15 @@ export const GLOBAL_SERVER_CONFIGURATIONS = KeyDefinition.record { + return previous.getApiUrl() === current.getApiUrl(); +}; + // FIXME: currently we are limited to api requests for active users. Update to accept a UserId and APIUrl once ApiService supports it. export class DefaultConfigService implements ConfigService { - private failedFetchFallbackSubject = new Subject(); + private failedFetchFallbackSubject = new Subject(); - serverConfig$: Observable; + serverConfig$: Observable; serverSettings$: Observable; @@ -67,25 +72,51 @@ export class DefaultConfigService implements ConfigService { private stateProvider: StateProvider, private authService: AuthService, ) { - const userId$ = this.stateProvider.activeUserId$; - const authStatus$ = userId$.pipe( - switchMap((userId) => (userId == null ? of(null) : this.authService.authStatusFor$(userId))), + const globalConfig$ = this.environmentService.globalEnvironment$.pipe( + distinctUntilChanged(environmentComparer), + switchMap((environment) => + this.globalConfigFor$(environment.getApiUrl()).pipe( + map((config) => { + return [config, null as UserId | null, environment] as const; + }), + ), + ), ); - this.serverConfig$ = combineLatest([ - userId$, - this.environmentService.environment$, - authStatus$, - ]).pipe( - switchMap(([userId, environment, authStatus]) => { - if (userId == null || authStatus !== AuthenticationStatus.Unlocked) { - return this.globalConfigFor$(environment.getApiUrl()).pipe( - map((config) => [config, null, environment] as const), - ); + this.serverConfig$ = this.stateProvider.activeUserId$.pipe( + distinctUntilChanged(), + switchMap((userId) => { + if (userId == null) { + // Global + return globalConfig$; } - return this.userConfigFor$(userId).pipe( - map((config) => [config, userId, environment] as const), + return this.authService.authStatusFor$(userId).pipe( + map((authStatus) => authStatus === AuthenticationStatus.Unlocked), + distinctUntilChanged(), + switchMap((isUnlocked) => { + if (!isUnlocked) { + return globalConfig$; + } + + return combineLatest([ + this.environmentService + .getEnvironment$(userId) + .pipe(distinctUntilChanged(environmentComparer)), + this.userConfigFor$(userId), + ]).pipe( + switchMap(([environment, config]) => { + if (config == null) { + // If the user doesn't have any config yet, use the global config for that url as the fallback + return this.globalConfigFor$(environment.getApiUrl()).pipe( + map((globalConfig) => [globalConfig, userId, environment] as const), + ); + } + + return of([config, userId, environment] as const); + }), + ); + }), ); }), tap(async (rec) => { @@ -106,7 +137,7 @@ export class DefaultConfigService implements ConfigService { }), // If fetch fails, we'll emit on this subject to fallback to the existing config mergeWith(this.failedFetchFallbackSubject), - shareReplay({ refCount: true, bufferSize: 1 }), + share({ connector: () => new ReplaySubject(1), resetOnRefCountZero: () => timer(1000) }), ); this.cloudRegion$ = this.serverConfig$.pipe( @@ -155,8 +186,8 @@ export class DefaultConfigService implements ConfigService { // Updates the on-disk configuration with a newly retrieved configuration private async renewConfig( - existingConfig: ServerConfig, - userId: UserId, + existingConfig: ServerConfig | null, + userId: UserId | null, environment: Environment, ): Promise { try { @@ -164,9 +195,7 @@ export class DefaultConfigService implements ConfigService { // somewhat quickly even though it may not be accurate, we won't cancel the HTTP request // though so that hopefully it can have finished and hydrated a more accurate value. const handle = setTimeout(() => { - this.logService.info( - "Self-host environment did not respond in time, emitting previous config.", - ); + this.logService.info("Environment did not respond in time, emitting previous config."); this.failedFetchFallbackSubject.next(existingConfig); }, SLOW_EMISSION_GUARD); const response = await this.configApiService.get(userId); @@ -199,13 +228,13 @@ export class DefaultConfigService implements ConfigService { } } - private globalConfigFor$(apiUrl: string): Observable { + private globalConfigFor$(apiUrl: string): Observable { return this.stateProvider .getGlobal(GLOBAL_SERVER_CONFIGURATIONS) - .state$.pipe(map((configs) => configs?.[apiUrl])); + .state$.pipe(map((configs) => configs?.[apiUrl] ?? null)); } - private userConfigFor$(userId: UserId): Observable { + private userConfigFor$(userId: UserId): Observable { return this.stateProvider.getUser(userId, USER_SERVER_CONFIG).state$; } } diff --git a/libs/common/src/platform/services/default-environment.service.ts b/libs/common/src/platform/services/default-environment.service.ts index df55693ba0b..4a1af68505a 100644 --- a/libs/common/src/platform/services/default-environment.service.ts +++ b/libs/common/src/platform/services/default-environment.service.ts @@ -133,6 +133,7 @@ export class DefaultEnvironmentService implements EnvironmentService { ); environment$: Observable; + globalEnvironment$: Observable; cloudWebVaultUrl$: Observable; constructor( @@ -148,6 +149,10 @@ export class DefaultEnvironmentService implements EnvironmentService { distinctUntilChanged((oldUserId: UserId, newUserId: UserId) => oldUserId == newUserId), ); + this.globalEnvironment$ = this.stateProvider + .getGlobal(GLOBAL_ENVIRONMENT_KEY) + .state$.pipe(map((state) => this.buildEnvironment(state?.region, state?.urls))); + this.environment$ = account$.pipe( switchMap((userId) => { const t = userId @@ -263,7 +268,7 @@ export class DefaultEnvironmentService implements EnvironmentService { return new SelfHostedEnvironment(urls); } - async setCloudRegion(userId: UserId, region: CloudRegion) { + async setCloudRegion(userId: UserId | null, region: CloudRegion) { if (userId == null) { await this.globalCloudRegionState.update(() => region); } else { @@ -271,7 +276,7 @@ export class DefaultEnvironmentService implements EnvironmentService { } } - getEnvironment$(userId: UserId): Observable { + getEnvironment$(userId: UserId): Observable { return this.stateProvider.getUser(userId, USER_ENVIRONMENT_KEY).state$.pipe( map((state) => { return this.buildEnvironment(state?.region, state?.urls);