feat(config): improve performance by not recomputing configuration if…

… not impacted (#1088) ## Proposed change Improve performance by not recomputing configuration if not impacted ## Related issues - 🚀 Feature #1087
AmadeusITGroup · Nov 29, 2023 · 69e34ec · 69e34ec
2 parents ec086fa + 9591445
commit 69e34ec
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 33 deletions.
diff --git a/packages/@o3r/configuration/src/services/configuration/configuration.base.service.spec.ts b/packages/@o3r/configuration/src/services/configuration/configuration.base.service.spec.ts
@@ -3,7 +3,8 @@ import { getTestBed, TestBed } from '@angular/core/testing';
 import { BrowserDynamicTestingModule, platformBrowserDynamicTesting } from '@angular/platform-browser-dynamic/testing';
 import { MockStore, provideMockStore } from '@ngrx/store/testing';
 import { Configuration, CustomConfig } from '@o3r/core';
-import { ConfigurationStore, selectConfigurationEntities, upsertConfigurationEntities, upsertConfigurationEntity } from '../../stores/index';
+import { Subscription } from 'rxjs';
+import { ConfigurationStore, globalConfigurationId, selectConfigOverride, selectConfigurationEntities, upsertConfigurationEntities, upsertConfigurationEntity } from '../../stores/index';
 import { ConfigurationBaseService } from './configuration.base.service';
 
 
@@ -52,7 +53,6 @@ describe('ConfigurationBaseService', () => {
 
     service = TestBed.inject(ConfigurationBaseService);
     mockStore = TestBed.inject(MockStore);
-    mockStore.overrideSelector(selectConfigurationEntities, {});
     jest.spyOn(mockStore, 'dispatch');
   });
 
@@ -95,4 +95,75 @@ describe('ConfigurationBaseService', () => {
       })
     );
   });
+
+  describe('getConfig emissions', () => {
+    const configId = 'my-config';
+    const configInitialValue = {
+      id: configId,
+      prop: 'value1'
+    };
+    const spy = jest.fn();
+    let subscription: Subscription;
+
+    beforeEach(() => {
+      mockStore.overrideSelector(selectConfigurationEntities, { [configId]: configInitialValue });
+      subscription = service.getConfig(configId).subscribe(spy);
+      spy.mockReset();
+    });
+
+    afterAll(() => {
+      subscription?.unsubscribe();
+    });
+
+    it('should not emit if we do not change the INITIAL_CONFIG value', () => {
+      mockStore.overrideSelector(selectConfigurationEntities, {
+        [configId]: configInitialValue,
+        ANOTHER_CONFIG: {
+          id: 'ANOTHER_CONFIG',
+          prop: 'value1'
+        }
+      });
+      mockStore.refreshState();
+      expect(spy).not.toHaveBeenCalled();
+    });
+
+    it('should emit if we change the INITIAL_CONFIG value', () => {
+      mockStore.overrideSelector(selectConfigurationEntities, {
+        [configId]: { ...configInitialValue, prop: 'change' }
+      });
+      mockStore.refreshState();
+      expect(spy).toHaveBeenCalledTimes(1);
+      expect(spy).toHaveBeenCalledWith(expect.objectContaining({ prop: 'change' }));
+    });
+
+    it('should not emit if we change the global config as the INITIAL_CONFIG will override its value', () => {
+      mockStore.overrideSelector(selectConfigurationEntities, {
+        [globalConfigurationId]: {
+          id: globalConfigurationId,
+          prop: 'will not override'
+        },
+        [configId]: configInitialValue
+      });
+      mockStore.refreshState();
+      expect(spy).not.toHaveBeenCalled();
+    });
+
+    it('should emit if we change the override config', () => {
+      mockStore.overrideSelector(selectConfigOverride, { [configId]: { prop: 'change' }});
+      mockStore.refreshState();
+      expect(spy).toHaveBeenCalledTimes(1);
+      expect(spy).toHaveBeenCalledWith(expect.objectContaining({ prop: 'change' }));
+    });
+
+    it('should not emit if we change the config value when it is overriden by the override config', () => {
+      mockStore.overrideSelector(selectConfigOverride, { [configId]: { prop: 'override' }});
+      mockStore.refreshState();
+      spy.mockReset();
+      mockStore.overrideSelector(selectConfigurationEntities, {
+        [configId]: { ...configInitialValue, prop: 'change' }
+      });
+      mockStore.refreshState();
+      expect(spy).not.toHaveBeenCalled();
+    });
+  });
 });
diff --git a/packages/@o3r/configuration/src/services/configuration/configuration.base.service.ts b/packages/@o3r/configuration/src/services/configuration/configuration.base.service.ts
@@ -1,19 +1,23 @@
 import { Injectable } from '@angular/core';
 import { select, Store } from '@ngrx/store';
 import { Configuration, CustomConfig, deepFill } from '@o3r/core';
-import { combineLatest, Observable } from 'rxjs';
+import { combineLatest, Observable, of } from 'rxjs';
 import { distinctUntilChanged, map, switchMap, take } from 'rxjs/operators';
 import { ConfigOverrideStore, selectComponentOverrideConfig } from '../../stores/config-override/index';
 import {
   computeConfiguration,
   ConfigurationStore,
+  globalConfigurationId,
   selectConfigurationEntities,
+  selectConfigurationForComponent,
+  selectGlobalConfiguration,
   updateConfigurationEntity,
   upsertConfigurationEntities,
   upsertConfigurationEntity
 } from '../../stores/index';
 import { ConfigurationBaseServiceModule } from './configuration.base.module';
 
+const jsonStringifyDiff = (obj1: any, obj2: any) => JSON.stringify(obj1) === JSON.stringify(obj2);
 
 /**
  * Configuration service
@@ -34,7 +38,7 @@ export class ConfigurationBaseService {
    * @param configuration to edit/add
    * @param configurationId Configuration ID
    */
-  public upsertConfiguration<T extends Configuration>(configuration: T, configurationId = 'global') {
+  public upsertConfiguration<T extends Configuration>(configuration: T, configurationId = globalConfigurationId) {
     this.store.dispatch(upsertConfigurationEntity({id: configurationId, configuration}));
   }
 
@@ -44,7 +48,7 @@ export class ConfigurationBaseService {
    * @param configuration Partial config to edit
    * @param configurationId Configuration ID
    */
-  public updateConfiguration<T extends Partial<Configuration>>(configuration: T, configurationId = 'global') {
+  public updateConfiguration<T extends Partial<Configuration>>(configuration: T, configurationId = globalConfigurationId) {
     this.store.dispatch(updateConfigurationEntity({id: configurationId, configuration}));
   }
 
@@ -77,7 +81,7 @@ export class ConfigurationBaseService {
    * @param configurationId Configuration ID to extend
    * @param forceUpdate Force update the configuration in the store
    */
-  public extendConfiguration<T extends Configuration>(extension: T, configurationId = 'global', forceUpdate = false) {
+  public extendConfiguration<T extends Configuration>(extension: T, configurationId = globalConfigurationId, forceUpdate = false) {
     if (this.extendedConfiguration[configurationId] && !forceUpdate) {
       return;
     }
@@ -116,32 +120,13 @@ export class ConfigurationBaseService {
    * @param id Id of the component
    */
   public getConfig(id: string): Observable<any> {
-    return combineLatest([
-      this.store.pipe(
-        select(selectConfigurationEntities),
-        map((storedConfigs) => {
-          const globalConfigId = 'global';
-          const componentConfig = storedConfigs[id];
-          const globalConfig = storedConfigs[globalConfigId];
-          if (id !== globalConfigId && globalConfig) {
-            return {
-              ...globalConfig,
-              ...(componentConfig || {})
-            };
-          }
-          return componentConfig || null;
-        }),
-        distinctUntilChanged((prev, current) => JSON.stringify(prev) === JSON.stringify(current))
-      ), this.store.pipe(
-        select(selectComponentOverrideConfig(id))
-      )]
-    ).pipe(
-      map(([storeConfig, storeOverrideConfig]) => {
-        return {
-          ...storeConfig,
-          ...storeOverrideConfig
-        };
-      })
+    const globalConfig$ = this.store.pipe(select(selectGlobalConfiguration));
+    const componentConfig$ = id !== globalConfigurationId ? this.store.pipe(select(selectConfigurationForComponent({ id }))) : of({});
+    const overrideConfig$ = this.store.pipe(select(selectComponentOverrideConfig(id)));
+
+    return combineLatest([globalConfig$, componentConfig$, overrideConfig$]).pipe(
+      map(([globalConfig, componentConfig, overrideConfig]) => ({ ...globalConfig, ...componentConfig, ...overrideConfig })),
+      distinctUntilChanged(jsonStringifyDiff)
     );
   }
 }
diff --git a/packages/@o3r/configuration/src/stores/configuration/configuration.selectors.ts b/packages/@o3r/configuration/src/stores/configuration/configuration.selectors.ts
@@ -1,7 +1,7 @@
 import { createSelector } from '@ngrx/store';
 import { Configuration } from '@o3r/core';
 import { configurationAdapter } from './configuration.reducer';
-import { CONFIGURATION_STORE_NAME, ConfigurationState } from './configuration.state';
+import { CONFIGURATION_STORE_NAME, ConfigurationState, globalConfigurationId } from './configuration.state';
 
 const {selectIds, selectEntities, selectAll, selectTotal} = configurationAdapter.getSelectors();
 
@@ -35,3 +35,8 @@ export const selectConfigurationTotal = createSelector(selectConfigurationState,
  */
 export const selectConfigurationForComponent = <T extends Configuration>(props: {id: string}) =>
   createSelector(selectConfigurationEntities, (entities) => (entities?.[props.id] || {}) as Configuration as T);
+
+/**
+ * Select the global configuration
+ */
+export const selectGlobalConfiguration = createSelector(selectConfigurationForComponent({ id: globalConfigurationId }), (config) => config);
diff --git a/packages/@o3r/configuration/src/stores/configuration/configuration.state.ts b/packages/@o3r/configuration/src/stores/configuration/configuration.state.ts
@@ -26,3 +26,8 @@ export interface ConfigurationStore {
   /** Configuration state */
   [CONFIGURATION_STORE_NAME]: ConfigurationState;
 }
+
+/**
+ * ID of the global configuration
+ */
+export const globalConfigurationId = 'global';