From 66a91984956d057a98d5ab6b2421834f3e2a8238 Mon Sep 17 00:00:00 2001 From: chrismclarke Date: Fri, 22 Nov 2024 11:02:02 -0800 Subject: [PATCH] refactor: app config service override hierarchy --- .../services/template-metadata.service.ts | 7 +++ .../app-config/app-config.service.spec.ts | 41 ++++++++++------ .../services/app-config/app-config.service.ts | 49 ++++++++++++------- src/app/shared/services/skin/skin.service.ts | 2 +- 4 files changed, 64 insertions(+), 35 deletions(-) diff --git a/src/app/shared/components/template/services/template-metadata.service.ts b/src/app/shared/components/template/services/template-metadata.service.ts index a62b7ba523..49836c3bc0 100644 --- a/src/app/shared/components/template/services/template-metadata.service.ts +++ b/src/app/shared/components/template/services/template-metadata.service.ts @@ -6,6 +6,7 @@ import { Router } from "@angular/router"; import { toSignal } from "@angular/core/rxjs-interop"; import { ngRouterMergedSnapshot$ } from "src/app/shared/utils/angular.utils"; import { isEqual } from "packages/shared/src/utils/object-utils"; +import { AppConfigService } from "src/app/shared/services/app-config/app-config.service"; /** * Service responsible for handling metadata of the current top-level template, @@ -26,6 +27,7 @@ export class TemplateMetadataService extends SyncServiceBase { constructor( private templateService: TemplateService, + private appConfigService: AppConfigService, private router: Router ) { super("TemplateMetadata"); @@ -42,5 +44,10 @@ export class TemplateMetadataService extends SyncServiceBase { }, { allowSignalWrites: true } ); + // apply any template-specific appConfig overrides on change + effect(() => { + const templateAppConfig = this.parameterList().app_config; + this.appConfigService.setAppConfig(templateAppConfig, "template"); + }); } } diff --git a/src/app/shared/services/app-config/app-config.service.spec.ts b/src/app/shared/services/app-config/app-config.service.spec.ts index 2b73be5c0f..d21985ac90 100644 --- a/src/app/shared/services/app-config/app-config.service.spec.ts +++ b/src/app/shared/services/app-config/app-config.service.spec.ts @@ -3,13 +3,9 @@ import { TestBed } from "@angular/core/testing"; import { AppConfigService } from "./app-config.service"; import { BehaviorSubject } from "rxjs/internal/BehaviorSubject"; import { IAppConfig } from "../../model"; -import { signal } from "@angular/core"; +import { signal, WritableSignal } from "@angular/core"; import { DeploymentService } from "../deployment/deployment.service"; -import { - getDefaultAppConfig, - IAppConfigOverride, - IDeploymentRuntimeConfig, -} from "packages/data-models"; +import { IAppConfigOverride, IDeploymentRuntimeConfig } from "packages/data-models"; import { deepMergeObjects } from "../../utils"; import { firstValueFrom } from "rxjs/internal/firstValueFrom"; import { MockDeploymentService } from "../deployment/deployment.service.spec"; @@ -46,6 +42,7 @@ const MOCK_DEPLOYMENT_CONFIG: Partial = { */ describe("AppConfigService", () => { let service: AppConfigService; + let appConfigSetSpy: jasmine.Spy>; beforeEach(() => { TestBed.configureTestingModule({ @@ -54,24 +51,20 @@ describe("AppConfigService", () => { ], }); service = TestBed.inject(AppConfigService); + appConfigSetSpy = spyOn(service.appConfig, "set").and.callThrough(); }); it("applies default config overrides on init", () => { - expect(service.appConfig().APP_HEADER_DEFAULTS.title).toEqual( - getDefaultAppConfig().APP_HEADER_DEFAULTS.title - ); + expect(service.appConfig().APP_HEADER_DEFAULTS.title).toEqual("App"); }); it("applies deployment-specific config overrides on init", () => { expect(service.appConfig().APP_FOOTER_DEFAULTS.templateName).toEqual("mock_footer"); }); - it("applies overrides to app config", () => { - service.setAppConfig({ APP_HEADER_DEFAULTS: { title: "updated" } }); - expect(service.appConfig().APP_HEADER_DEFAULTS).toEqual({ - ...getDefaultAppConfig().APP_HEADER_DEFAULTS, - title: "updated", - }); + it("applies skin-level overrides to app config", () => { + service.setAppConfig({ APP_HEADER_DEFAULTS: { title: "updated" } }, "skin"); + expect(service.appConfig().APP_HEADER_DEFAULTS.title).toEqual("updated"); // also ensure doesn't unset default deployment expect(service.appConfig().APP_FOOTER_DEFAULTS.templateName).toEqual("mock_footer"); }); @@ -80,7 +73,23 @@ describe("AppConfigService", () => { firstValueFrom(service.changes$).then((v) => { expect(v).toEqual({ APP_HEADER_DEFAULTS: { title: "partial changes" } }); }); + service.setAppConfig({ APP_HEADER_DEFAULTS: { title: "partial changes" } }, "skin"); + expect(appConfigSetSpy).toHaveBeenCalledTimes(1); + }); - service.setAppConfig({ APP_HEADER_DEFAULTS: { title: "partial changes" } }); + it("ignores lower-order updates when higher order exists", async () => { + service.setAppConfig({ APP_FOOTER_DEFAULTS: { templateName: "template_footer" } }, "template"); + expect(service.appConfig().APP_FOOTER_DEFAULTS.templateName).toEqual("template_footer"); + service.setAppConfig({ APP_FOOTER_DEFAULTS: { templateName: "skin_footer" } }, "skin"); + expect(service.appConfig().APP_FOOTER_DEFAULTS.templateName).toEqual("template_footer"); + // the second service set should not trigger any changes to appConfig signal (or observable) + expect(appConfigSetSpy).toHaveBeenCalledTimes(1); + }); + + it("reverts to initial config values when template override removed", async () => { + service.setAppConfig({ APP_FOOTER_DEFAULTS: { templateName: "template_footer" } }, "template"); + expect(service.appConfig().APP_FOOTER_DEFAULTS.templateName).toEqual("template_footer"); + service.setAppConfig({}, "template"); + expect(service.appConfig().APP_FOOTER_DEFAULTS.templateName).toEqual("mock_footer"); }); }); diff --git a/src/app/shared/services/app-config/app-config.service.ts b/src/app/shared/services/app-config/app-config.service.ts index 1028ebca04..6942ca9186 100644 --- a/src/app/shared/services/app-config/app-config.service.ts +++ b/src/app/shared/services/app-config/app-config.service.ts @@ -8,32 +8,36 @@ import { Observable } from "rxjs"; import { DeploymentService } from "../deployment/deployment.service"; import { updateRoutingDefaults } from "./app-config.utils"; import { Router } from "@angular/router"; +import { isEqual } from "packages/shared/src/utils/object-utils"; + +/** Config overrides can come from a variety of sources with orders of hierarchy */ +const APP_CONFIG_OVERRIDE_ORDER = { + default: 0, + deployment: 1, + skin: 2, + template: 3, +}; +type IAppConfigOverrideSource = keyof typeof APP_CONFIG_OVERRIDE_ORDER; @Injectable({ providedIn: "root", }) export class AppConfigService extends SyncServiceBase { - /** - * Initial config is generated by merging default app config with deployment-specific overrides - * It is accessed via a read-only getter to avoid update from methods - **/ - private readonly initialConfig: IAppConfig = deepMergeObjects( - getDefaultAppConfig(), - this.deploymentService.config.app_config - ); - /** Signal representation of current appConfig value */ - public appConfig = signal(this.initialConfig); + public appConfig = signal(undefined); /** * @deprecated - prefer use of config signal and computed/effect bindings * List of constants provided by data-models combined with deployment-specific overrides and skin-specific overrides **/ - public appConfig$ = new BehaviorSubject(this.initialConfig); + public appConfig$ = new BehaviorSubject(undefined); /** Tracking observable of deep changes to app config, exposed in `changes` public method */ private appConfigChanges$: Observable>; + /** Array of all applied config overrides. Array position represents override hierarchy order (0-3) */ + private configOverrides: IAppConfigOverride[] = []; + /** * @deprecated - prefer use of config signal and computed/effect bindings * @@ -63,20 +67,29 @@ export class AppConfigService extends SyncServiceBase { this.initialise(); } - /** When service initialises load initial config to trigger any side-effects */ + /** When service initialises load config defaults and deployment to trigger any side-effects */ private initialise() { - this.setAppConfig(this.initialConfig); + this.setAppConfig(getDefaultAppConfig(), "default"); + this.setAppConfig(this.deploymentService.config.app_config, "deployment"); } /** * Generate a complete app config by deep-merging app config overrides * with the initial config */ - public setAppConfig(overrides: IAppConfigOverride = {}) { - const mergedConfig = deepMergeObjects({} as IAppConfig, this.initialConfig, overrides); - this.handleConfigSideEffects(overrides, mergedConfig); - this.appConfig.set(mergedConfig); - this.appConfig$.next(mergedConfig); + public setAppConfig(overrides: IAppConfigOverride = {}, source: IAppConfigOverrideSource) { + const overrideIndex = APP_CONFIG_OVERRIDE_ORDER[source]; + // replace any overrides at the existing level (e.g. skin or template) + this.configOverrides[overrideIndex] = overrides; + // merge all levels of override, with higher order levels merged on top of lower + const mergedConfig = deepMergeObjects({} as IAppConfig, ...this.configOverrides); + + // trigger change effects only if config changed + if (!isEqual(this.appConfig(), mergedConfig)) { + this.handleConfigSideEffects(overrides, mergedConfig); + this.appConfig.set(mergedConfig); + this.appConfig$.next(mergedConfig); + } } private handleConfigSideEffects(overrides: IAppConfigOverride = {}, config: IAppConfig) { diff --git a/src/app/shared/services/skin/skin.service.ts b/src/app/shared/services/skin/skin.service.ts index 1356b3ed93..6a31601f6b 100644 --- a/src/app/shared/services/skin/skin.service.ts +++ b/src/app/shared/services/skin/skin.service.ts @@ -51,7 +51,7 @@ export class SkinService extends SyncServiceBase { const override = this.generateOverrideConfig(targetSkin); const revert = this.generateRevertConfig(targetSkin); console.log("[SKIN] SET", { targetSkin, override, revert }); - this.appConfigService.setAppConfig(override); + this.appConfigService.setAppConfig(override, "skin"); this.revertOverride = revert; this.currentSkin = targetSkin;