From 89ddea69db82964a0b4c8a143bbb398cf2960d9d Mon Sep 17 00:00:00 2001 From: Dermot Duffy Date: Sun, 3 Mar 2024 18:22:51 -0800 Subject: [PATCH] Fix obscure config merge bug. --- src/camera-manager/manager.ts | 14 ++++++--- src/utils/basic.ts | 5 +++ tests/camera-manager/manager.test.ts | 42 ++++++++++++++++++++----- tests/utils/basic.test.ts | 47 ++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 12 deletions(-) diff --git a/src/camera-manager/manager.ts b/src/camera-manager/manager.ts index 18bf305b..82960b0f 100644 --- a/src/camera-manager/manager.ts +++ b/src/camera-manager/manager.ts @@ -1,12 +1,16 @@ import add from 'date-fns/add'; import cloneDeep from 'lodash-es/cloneDeep'; -import merge from 'lodash-es/merge.js'; import sum from 'lodash-es/sum'; import { CardCameraAPI } from '../card-controller/types.js'; import { CameraConfig, CamerasConfig, PTZAction, PTZPhase } from '../config/types.js'; import { MEDIA_CHUNK_SIZE_DEFAULT } from '../const.js'; import { localize } from '../localize/localize.js'; -import { allPromises, arrayify, setify } from '../utils/basic.js'; +import { + allPromises, + arrayify, + recursivelyMergeObjectsNotArrays, + setify, +} from '../utils/basic.js'; import { getCameraID } from '../utils/camera.js'; import { log } from '../utils/debug.js'; import { ViewMedia } from '../view/media.js'; @@ -141,7 +145,7 @@ export class CameraManager { // order, to ensure that the defaults in the cameras global config do not // override the values specified in the per-camera config. const cameras = config.cameras.map((camera) => - merge(cloneDeep(config?.cameras_global), camera), + recursivelyMergeObjectsNotArrays(cloneDeep(config?.cameras_global), camera), ); try { @@ -460,13 +464,13 @@ export class CameraManager { const latestResult = getTimeFromResults('latest'); if (latestResult) { newChunkQuery.start = latestResult; - delete(newChunkQuery.end); + delete newChunkQuery.end; } } else if (direction === 'earlier') { const earliestResult = getTimeFromResults('earliest'); if (earliestResult) { newChunkQuery.end = earliestResult; - delete(newChunkQuery.start); + delete newChunkQuery.start; } } newChunkQuery.limit = chunkSize; diff --git a/src/utils/basic.ts b/src/utils/basic.ts index 7209771a..a977e9e6 100644 --- a/src/utils/basic.ts +++ b/src/utils/basic.ts @@ -3,6 +3,7 @@ import differenceInMinutes from 'date-fns/differenceInMinutes'; import differenceInSeconds from 'date-fns/differenceInSeconds'; import format from 'date-fns/format'; import isEqual from 'lodash-es/isEqual'; +import mergeWith from 'lodash-es/mergeWith'; import { FrigateCardError } from '../types'; export type ModifyInterface = Omit & R; @@ -240,3 +241,7 @@ export const getChildrenFromElement = (parent: HTMLElement): HTMLElement[] => { : [...parent.children]; return children.filter(isHTMLElement); }; + +export const recursivelyMergeObjectsNotArrays = (src1: T, src2: T): T => { + return mergeWith({}, src1, src2, (_a, b) => (Array.isArray(b) ? b : undefined)); +}; diff --git a/tests/camera-manager/manager.test.ts b/tests/camera-manager/manager.test.ts index 642a11e2..5aa825c9 100644 --- a/tests/camera-manager/manager.test.ts +++ b/tests/camera-manager/manager.test.ts @@ -1,3 +1,4 @@ +import { HomeAssistant } from '@dermotduffy/custom-card-helpers'; import add from 'date-fns/add'; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { mock } from 'vitest-mock-extended'; @@ -29,6 +30,7 @@ import { import { sortMedia } from '../../src/camera-manager/utils'; import { CardController } from '../../src/card-controller/controller'; import { CameraConfig } from '../../src/config/types'; +import { EntityRegistryManager } from '../../src/utils/ha/entity-registry'; import { ViewMedia } from '../../src/view/media'; import { TestViewMedia, @@ -222,7 +224,7 @@ describe('CameraManager', async () => { factory?: CameraManagerEngineFactory, ): CameraManager => { const camerasConfig = cameras?.map( - (camera) => camera.config ?? createCameraConfig({ engine: 'generic' }), + (camera) => camera.config ?? createCameraConfig(baseCameraConfig), ); vi.mocked(api.getConfigManager().getConfig).mockReturnValue( createConfig({ @@ -238,12 +240,17 @@ describe('CameraManager', async () => { const engineType = camera.engineType === undefined ? Engine.Generic : camera.engineType; if (engineType) { - vi.mocked(mockEngine.createCamera).mockResolvedValueOnce( - createCamera( - camera.config ?? createCameraConfig(baseCameraConfig), - mockEngine, - camera.capabilties ?? createCameraCapabilities(), - ), + vi.mocked(mockEngine.createCamera).mockImplementationOnce( + async ( + _hass: HomeAssistant, + _entityRegistryManager: EntityRegistryManager, + cameraConfig: CameraConfig, + ): Promise => + createCamera( + cameraConfig, + mockEngine, + camera.capabilties ?? createCameraCapabilities(), + ), ); } vi.mocked(mockFactory.getEngineForCamera).mockResolvedValueOnce(engineType); @@ -484,6 +491,27 @@ describe('CameraManager', async () => { expect(manager.generateDefaultEventQueries('id')).toBeNull(); }); }); + + it('should merge defaults correctly', async () => { + const api = createCardAPI(); + vi.mocked(api.getHASSManager().getHASS).mockReturnValue(createHASS()); + + const engine = mock(); + const manager = createCameraManager(api, engine, [ + { + config: createCameraConfig({ + ...baseCameraConfig, + triggers: { + events: ['snapshots'], + }, + }), + }, + ]); + expect(await manager.initializeCamerasFromConfig()).toBeTruthy(); + expect(manager.getStore().getCamera('id')?.getConfig().triggers.events).toEqual([ + 'snapshots', + ]); + }); }); describe('should get media metadata', () => { diff --git a/tests/utils/basic.test.ts b/tests/utils/basic.test.ts index 0a58c87c..4961b777 100644 --- a/tests/utils/basic.test.ts +++ b/tests/utils/basic.test.ts @@ -18,6 +18,7 @@ import { isTruthy, isValidDate, prettifyTitle, + recursivelyMergeObjectsNotArrays, runWhenIdleIfSupported, setOrRemoveAttribute, setify, @@ -291,3 +292,49 @@ describe('getChildrenFromElement', () => { expect(getChildrenFromElement(slot)).toEqual(children); }); }); + +describe('recursivelyMergeObjectsNotArrays', () => { + it('should recursively merge objects but replace arrays', () => { + expect( + recursivelyMergeObjectsNotArrays( + { + a: { + b: { + c: 3, + d: { + e: 4, + }, + array: [1, 2, 3], + }, + other: { + field: 7, + }, + }, + }, + { + a: { + b: { + array: [4], + d: { + e: 5, + }, + }, + }, + }, + ), + ).toEqual({ + a: { + b: { + c: 3, + array: [4], + d: { + e: 5, + }, + }, + other: { + field: 7, + }, + }, + }); + }); +});