diff --git a/src/camera-manager/manager.ts b/src/camera-manager/manager.ts index 4879914f..bbd303fe 100644 --- a/src/camera-manager/manager.ts +++ b/src/camera-manager/manager.ts @@ -222,8 +222,8 @@ export class CameraManager { camerasConfig.some((config) => hasAutoTriggers(config)) ) { // ... then we need to populate the entity cache by fetching all entities - // from Home Assistant. Do this once upfront, to avoid each camera doing - // it. + // from Home Assistant. Attempt to do this once upfront, to avoid each + // camera doing needing to fetch entity state. await this._api.getEntityRegistryManager().fetchEntityList(hass); } diff --git a/src/card-controller/controller.ts b/src/card-controller/controller.ts index 02ce4624..d0425ffe 100644 --- a/src/card-controller/controller.ts +++ b/src/card-controller/controller.ts @@ -2,6 +2,10 @@ import { LovelaceCardEditor } from '@dermotduffy/custom-card-helpers'; import { ReactiveController } from 'lit'; import { CameraManager } from '../camera-manager/manager'; import { FrigateCardConfig } from '../config/types'; +import { + createDeviceRegistryCache, + DeviceRegistryManager, +} from '../utils/ha/registry/device'; import { createEntityRegistryCache, EntityRegistryManager, @@ -90,6 +94,9 @@ export class CardController { // These properties may be used in the construction of 'managers' (and should // be created first). + protected _deviceRegistryManager = new DeviceRegistryManager( + createDeviceRegistryCache(), + ); protected _entityRegistryManager = new EntityRegistryManager( createEntityRegistryCache(), ); @@ -178,6 +185,10 @@ export class CardController return this._defaultManager; } + public getDeviceRegistryManager(): DeviceRegistryManager { + return this._deviceRegistryManager; + } + public getDownloadManager(): DownloadManager { return this._downloadManager; } diff --git a/src/card.ts b/src/card.ts index c8a65072..b17461a8 100644 --- a/src/card.ts +++ b/src/card.ts @@ -400,6 +400,7 @@ class FrigateCard extends LitElement { .triggeredCameraIDs=${this._config?.view.triggers.show_trigger_status ? this._controller.getTriggersManager().getTriggeredCameraIDs() : undefined} + .deviceRegistryManager=${this._controller.getDeviceRegistryManager()} > ${ // Keep message rendering to last to show messages that may have been diff --git a/src/components/diagnostics.ts b/src/components/diagnostics.ts index fd049c28..b44469d3 100644 --- a/src/components/diagnostics.ts +++ b/src/components/diagnostics.ts @@ -6,12 +6,16 @@ import { localize } from '../localize/localize'; import basicBlockStyle from '../scss/basic-block.scss'; import { Diagnostics, getDiagnostics } from '../utils/diagnostics'; import { renderMessage } from './message'; +import { DeviceRegistryManager } from '../utils/ha/registry/device'; @customElement('frigate-card-diagnostics') export class FrigateCardDiagnostics extends LitElement { @property({ attribute: false }) public hass?: HomeAssistant; + @property({ attribute: false }) + public deviceRegistryManager?: DeviceRegistryManager; + @property({ attribute: false }) public rawConfig?: RawFrigateCardConfig; @@ -19,7 +23,11 @@ export class FrigateCardDiagnostics extends LitElement { protected _diagnostics: Diagnostics | null = null; protected async _fetchDiagnostics(): Promise { - this._diagnostics = await getDiagnostics(this.hass, this.rawConfig); + this._diagnostics = await getDiagnostics( + this.hass, + this.deviceRegistryManager, + this.rawConfig, + ); } protected shouldUpdate(): boolean { diff --git a/src/components/views.ts b/src/components/views.ts index 8d735574..1921ce31 100644 --- a/src/components/views.ts +++ b/src/components/views.ts @@ -19,9 +19,11 @@ import { } from '../config/types.js'; import viewsStyle from '../scss/views.scss'; import { ExtendedHomeAssistant } from '../types.js'; +import { DeviceRegistryManager } from '../utils/ha/registry/device/index.js'; import { ResolvedMediaCache } from '../utils/ha/resolved-media.js'; -// As a special case: Diagnostics is not dynamically loaded in case something goes wrong. +// As a special case: The diagnostics view is not dynamically loaded in case +// something goes wrong. import './diagnostics.js'; @customElement('frigate-card-views') @@ -62,6 +64,9 @@ export class FrigateCardViews extends LitElement { @property({ attribute: false }) public triggeredCameraIDs?: Set; + @property({ attribute: false }) + public deviceRegistryManager?: DeviceRegistryManager; + protected willUpdate(changedProps: PropertyValues): void { if (changedProps.has('viewManagerEpoch') || changedProps.has('config')) { const view = this.viewManagerEpoch?.manager.getView(); @@ -215,6 +220,7 @@ export class FrigateCardViews extends LitElement { ? html` ` : ``} diff --git a/src/utils/diagnostics.ts b/src/utils/diagnostics.ts index 05706f4a..9c1defa8 100644 --- a/src/utils/diagnostics.ts +++ b/src/utils/diagnostics.ts @@ -2,8 +2,7 @@ import { HomeAssistant } from '@dermotduffy/custom-card-helpers'; import pkg from '../../package.json'; import { RawFrigateCardConfig } from '../config/types'; import { getLanguage } from '../localize/localize'; -import { getAllDevices } from './ha/registry/device'; -import { DeviceList } from './ha/registry/device/types'; +import { DeviceRegistryManager } from './ha/registry/device'; type FrigateVersions = Record; @@ -44,20 +43,19 @@ export interface Diagnostics { export const getDiagnostics = async ( hass?: HomeAssistant, + deviceRegistryManager?: DeviceRegistryManager, rawConfig?: RawFrigateCardConfig, ): Promise => { - let devices: DeviceList | undefined = []; - if (hass) { - try { - devices = await getAllDevices(hass); - } catch (e) { - // Pass. This is optional. - } - } - // Get the Frigate devices in order to extract the Frigate integration and // server version numbers. - const frigateDevices = devices?.filter((device) => device.manufacturer === 'Frigate'); + const frigateDevices = + hass && deviceRegistryManager + ? await deviceRegistryManager.getMatchingDevices( + hass, + (device) => device.manufacturer === 'Frigate', + ) + : []; + const frigateVersionMap: Map = new Map(); frigateDevices?.forEach((device) => { device.config_entries.forEach((configEntry) => { diff --git a/src/utils/ha/registry/device/index.ts b/src/utils/ha/registry/device/index.ts index 54d39f94..ac3eb783 100644 --- a/src/utils/ha/registry/device/index.ts +++ b/src/utils/ha/registry/device/index.ts @@ -1,14 +1,54 @@ import { HomeAssistant } from '@dermotduffy/custom-card-helpers'; import { homeAssistantWSRequest } from '../..'; -import { DeviceList, deviceListSchema } from './types'; +import { errorToConsole } from '../../../basic'; +import { RegistryCache } from '../cache'; +import { Device, DeviceList, deviceListSchema } from './types'; -/** - * Get a list of all entities from the entity registry. May throw. - * @param hass The Home Assistant object. - * @returns An entity list object. - */ -export const getAllDevices = async (hass: HomeAssistant): Promise => { - return await homeAssistantWSRequest(hass, deviceListSchema, { - type: 'config/device_registry/list', - }); +export const createDeviceRegistryCache = (): RegistryCache => { + return new RegistryCache((device) => device.id); }; + +export class DeviceRegistryManager { + protected _cache: RegistryCache; + protected _fetchedDeviceList = false; + + constructor(cache: RegistryCache) { + this._cache = cache; + } + + public async getDevice(hass: HomeAssistant, deviceID: string): Promise { + if (this._cache.has(deviceID)) { + return this._cache.get(deviceID); + } + + // There is currently no way to fetch a single device. + await this._fetchDeviceList(hass); + return this._cache.get(deviceID) ?? null; + } + + public async getMatchingDevices( + hass: HomeAssistant, + func: (arg: Device) => boolean, + ): Promise { + await this._fetchDeviceList(hass); + return this._cache.getMatches(func); + } + + protected async _fetchDeviceList(hass: HomeAssistant): Promise { + if (this._fetchedDeviceList) { + return; + } + + let deviceList: DeviceList | null = null; + try { + deviceList = await homeAssistantWSRequest(hass, deviceListSchema, { + type: 'config/device_registry/list', + }); + } catch (e) { + errorToConsole(e as Error); + return; + } + this._cache.add(deviceList); + this._fetchedDeviceList = true; + } +} diff --git a/src/utils/ha/registry/entity/index.ts b/src/utils/ha/registry/entity/index.ts index 191cd243..d7a18274 100644 --- a/src/utils/ha/registry/entity/index.ts +++ b/src/utils/ha/registry/entity/index.ts @@ -1,7 +1,8 @@ import { HomeAssistant } from '@dermotduffy/custom-card-helpers'; import { homeAssistantWSRequest } from '../..'; -import { Entity, EntityList, entitySchema, entityListSchema } from './types.js'; +import { errorToConsole } from '../../../basic'; import { RegistryCache } from '../cache'; +import { Entity, EntityList, entityListSchema, entitySchema } from './types.js'; export const createEntityRegistryCache = (): RegistryCache => { return new RegistryCache((entity) => entity.entity_id); @@ -31,7 +32,8 @@ export class EntityRegistryManager { type: 'config/entity_registry/get', entity_id: entityID, }); - } catch { + } catch (e) { + errorToConsole(e as Error); return null; } this._cache.add(entity); @@ -68,9 +70,16 @@ export class EntityRegistryManager { if (this._fetchedEntityList) { return; } - const entityList = await homeAssistantWSRequest(hass, entityListSchema, { - type: 'config/entity_registry/list', - }); + + let entityList: EntityList | null = null; + try { + entityList = await homeAssistantWSRequest(hass, entityListSchema, { + type: 'config/entity_registry/list', + }); + } catch (e) { + errorToConsole(e as Error); + return; + } this._cache.add(entityList); this._fetchedEntityList = true; } diff --git a/tests/card-controller/controller.test.ts b/tests/card-controller/controller.test.ts index d5268e4f..ee93f7e1 100644 --- a/tests/card-controller/controller.test.ts +++ b/tests/card-controller/controller.test.ts @@ -28,6 +28,7 @@ import { StyleManager } from '../../src/card-controller/style-manager'; import { TriggersManager } from '../../src/card-controller/triggers-manager'; import { ViewManager } from '../../src/card-controller/view/view-manager'; import { FrigateCardEditor } from '../../src/editor'; +import { DeviceRegistryManager } from '../../src/utils/ha/registry/device'; import { EntityRegistryManager } from '../../src/utils/ha/registry/entity'; import { ResolvedMediaCache } from '../../src/utils/ha/resolved-media'; @@ -55,6 +56,7 @@ vi.mock('../../src/card-controller/status-bar-item-manager'); vi.mock('../../src/card-controller/style-manager'); vi.mock('../../src/card-controller/triggers-manager'); vi.mock('../../src/card-controller/view/view-manager'); +vi.mock('../../src/utils/ha/registry/device'); vi.mock('../../src/utils/ha/registry/entity'); vi.mock('../../src/utils/ha/resolved-media'); @@ -149,6 +151,12 @@ describe('CardController', () => { ); }); + it('getDeviceRegistryManager', () => { + expect(createController().getDeviceRegistryManager()).toBe( + vi.mocked(DeviceRegistryManager).mock.instances[0], + ); + }); + it('getDownloadManager', () => { expect(createController().getDownloadManager()).toBe( vi.mocked(DownloadManager).mock.instances[0], diff --git a/tests/test-utils.ts b/tests/test-utils.ts index b66acbcb..33c6a525 100644 --- a/tests/test-utils.ts +++ b/tests/test-utils.ts @@ -55,6 +55,7 @@ import { } from '../src/config/types'; import { CapabilitiesRaw, ExtendedHomeAssistant, MediaLoadedInfo } from '../src/types'; import { HassStateDifference } from '../src/utils/ha'; +import { Device } from '../src/utils/ha/registry/device/types'; import { EntityRegistryManager } from '../src/utils/ha/registry/entity'; import { Entity } from '../src/utils/ha/registry/entity/types'; import { ViewMedia, ViewMediaType } from '../src/view/media'; @@ -128,6 +129,15 @@ export const createUser = (user?: Partial): CurrentUser => ({ ...user, }); +export const createRegistryDevice = (device?: Partial): Device => { + return { + id: device?.id ?? 'id', + model: device?.model ?? null, + config_entries: device?.config_entries ?? [], + manufacturer: device?.manufacturer ?? null, + }; +}; + export const createRegistryEntity = (entity?: Partial): Entity => { return { config_entry_id: entity?.config_entry_id ?? null, diff --git a/tests/utils/diagnostics.test.ts b/tests/utils/diagnostics.test.ts index 5befe19c..8f351f27 100644 --- a/tests/utils/diagnostics.test.ts +++ b/tests/utils/diagnostics.test.ts @@ -1,9 +1,10 @@ import { HassConfig } from 'home-assistant-js-websocket'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { mock } from 'vitest-mock-extended'; import { getLanguage } from '../../src/localize/localize'; import { getDiagnostics, getReleaseVersion } from '../../src/utils/diagnostics.js'; -import { getAllDevices } from '../../src/utils/ha/registry/device/index.js'; -import { createHASS } from '../test-utils'; +import { DeviceRegistryManager } from '../../src/utils/ha/registry/device'; +import { createHASS, createRegistryDevice } from '../test-utils'; vi.mock('../../package.json', () => ({ default: { @@ -34,18 +35,6 @@ describe('getDiagnostics', () => { vi.mocked(getLanguage).mockReturnValue('en'); vi.stubGlobal('navigator', { userAgent: 'FrigateCardTest/1.0' }); - - vi.mocked(getAllDevices).mockResolvedValue([ - { - id: 'id', - model: '4.0.0/0.13.0-aded314', - config_entries: [ - 'ac4e79d258449a83bc0cf6d47a021c46', - 'b03e70c659d58ae2ce7f2dc76fed2929', - ], - manufacturer: 'Frigate', - }, - ]); }); afterEach(() => { @@ -54,8 +43,27 @@ describe('getDiagnostics', () => { }); it('should fetch diagnostics', async () => { + const deviceRegistryManager = mock(); + deviceRegistryManager.getMatchingDevices.mockResolvedValue([ + createRegistryDevice({ + id: 'id1', + model: '4.0.0/0.13.0-aded314', + config_entries: ['ac4e79d258449a83bc0cf6d47a021c46'], + }), + createRegistryDevice({ + id: 'id2', + model: '4.0.0/0.13.0-aded314', + config_entries: ['b03e70c659d58ae2ce7f2dc76fed2929'], + }), + createRegistryDevice({ + id: 'no-model', + model: null, + config_entries: ['b03e70c659d58ae2ce7f2dc76fed2920'], + }), + ]); + expect( - await getDiagnostics(hass, { + await getDiagnostics(hass, deviceRegistryManager, { cameras: [{ camera_entity: 'camera.office' }], }), ).toEqual({ @@ -80,6 +88,27 @@ describe('getDiagnostics', () => { }); }); + it('should use correct device registry matcher', async () => { + const deviceRegistryManager = mock(); + deviceRegistryManager.getMatchingDevices.mockResolvedValue([]); + + await getDiagnostics(hass, deviceRegistryManager, { + cameras: [{ camera_entity: 'camera.office' }], + }); + + // Verify the matcher passed into the deviceRegistryManager correctly filters + // Frigate cameras. + const matcher = deviceRegistryManager.getMatchingDevices.mock.calls[0][1]; + expect(matcher(createRegistryDevice())).toBe(false); + expect( + matcher( + createRegistryDevice({ + manufacturer: 'Frigate', + }), + ), + ).toBe(true); + }); + it('should fetch diagnostics without hass or config', async () => { expect(await getDiagnostics()).toEqual({ browser: 'FrigateCardTest/1.0', @@ -96,37 +125,10 @@ describe('getDiagnostics', () => { }); it('should fetch diagnostics without device model', async () => { - vi.mocked(getAllDevices).mockResolvedValue([ - { - id: 'id', - model: null, - config_entries: [ - 'ac4e79d258449a83bc0cf6d47a021c46', - 'b03e70c659d58ae2ce7f2dc76fed2929', - ], - manufacturer: 'Frigate', - }, - ]); + const deviceRegistryManager = mock(); + deviceRegistryManager.getMatchingDevices.mockResolvedValue([]); - expect(await getDiagnostics(hass)).toEqual({ - browser: 'FrigateCardTest/1.0', - card_version: '__FRIGATE_CARD_RELEASE_VERSION__', - git: { - build_date: 'Tue, 19 Sep 2023 04:59:27 GMT', - commit_date: 'Wed, 6 Sep 2023 21:27:28 -0700', - hash: 'g4cf13b1', - }, - ha_version: '2023.9.0', - date: now, - lang: 'en', - timezone: expect.anything(), - }); - }); - - it('should fetch diagnostics if getAllDevices errors', async () => { - vi.mocked(getAllDevices).mockRejectedValue(new Error()); - - expect(await getDiagnostics(hass)).toEqual({ + expect(await getDiagnostics(hass, deviceRegistryManager)).toEqual({ browser: 'FrigateCardTest/1.0', card_version: '__FRIGATE_CARD_RELEASE_VERSION__', git: { diff --git a/tests/utils/ha/registry/device/index.test.ts b/tests/utils/ha/registry/device/index.test.ts new file mode 100644 index 00000000..38a37f39 --- /dev/null +++ b/tests/utils/ha/registry/device/index.test.ts @@ -0,0 +1,73 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { homeAssistantWSRequest } from '../../../../../src/utils/ha'; +import { createHASS, createRegistryDevice } from '../../../../test-utils.js'; +import { + createDeviceRegistryCache, + DeviceRegistryManager, +} from '../../../../../src/utils/ha/registry/device'; + +vi.mock('../../../../../src/utils/ha'); +vi.spyOn(global.console, 'warn').mockImplementation(() => true); + +describe('DeviceRegistryManager', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + describe('getDevice', () => { + it('should not fetch when cached', async () => { + const cache = createDeviceRegistryCache(); + const testDevice = createRegistryDevice({ id: 'test' }); + + cache.add(testDevice); + + const manager = new DeviceRegistryManager(cache); + expect(await manager.getDevice(createHASS(), 'test')).toEqual(testDevice); + + expect(homeAssistantWSRequest).not.toHaveBeenCalled(); + }); + + it('should fetch and cache when not cached', async () => { + const testDevice = createRegistryDevice({ id: 'test' }); + + const manager = new DeviceRegistryManager(createDeviceRegistryCache()); + vi.mocked(homeAssistantWSRequest).mockResolvedValueOnce([testDevice]); + + expect(await manager.getDevice(createHASS(), 'test')).toEqual(testDevice); + expect(homeAssistantWSRequest).toBeCalledTimes(1); + + expect(await manager.getDevice(createHASS(), 'test')).toEqual(testDevice); + expect(homeAssistantWSRequest).toBeCalledTimes(1); + + expect(await manager.getDevice(createHASS(), 'missing')).toBeNull(); + + // The fetch call is called exactly once. + expect(homeAssistantWSRequest).toBeCalledTimes(1); + }); + + it('should return null when fetch fails', async () => { + vi.mocked(homeAssistantWSRequest).mockRejectedValueOnce(new Error('Fetch error')); + + const manager = new DeviceRegistryManager(createDeviceRegistryCache()); + expect(await manager.getDevice(createHASS(), 'test')).toBeNull(); + + vi.mocked(expect(console.warn)).toBeCalledWith('Fetch error'); + }); + }); + + it('getMatchingDevices', async () => { + const matchingDevice = createRegistryDevice({ id: 'matching' }); + const notMatchingDevice = createRegistryDevice({ id: 'not-matching' }); + const hass = createHASS(); + + vi.mocked(homeAssistantWSRequest).mockResolvedValueOnce([ + matchingDevice, + notMatchingDevice, + ]); + + const manager = new DeviceRegistryManager(createDeviceRegistryCache()); + expect( + await manager.getMatchingDevices(hass, (entity) => entity.id == 'matching'), + ).toEqual([matchingDevice]); + }); +}); diff --git a/tests/utils/ha/registry/entity/index.test.ts b/tests/utils/ha/registry/entity/index.test.ts index 0f143fb2..966b8cdd 100644 --- a/tests/utils/ha/registry/entity/index.test.ts +++ b/tests/utils/ha/registry/entity/index.test.ts @@ -7,6 +7,7 @@ import { import { createHASS, createRegistryEntity } from '../../../../test-utils.js'; vi.mock('../../../../../src/utils/ha'); +vi.spyOn(global.console, 'warn').mockImplementation(() => true); describe('EntityRegistryManager', () => { afterEach(() => { @@ -44,10 +45,12 @@ describe('EntityRegistryManager', () => { const manager = new EntityRegistryManager(createEntityRegistryCache()); expect(await manager.getEntity(createHASS(), 'missing')).toBeNull(); + + vi.mocked(expect(console.warn)).toBeCalledWith('Not found'); }); }); - it('getEntities', () => { + it('getEntities', async () => { const cachedEntity = createRegistryEntity({ entity_id: 'cached' }); const notCachedEntity = createRegistryEntity({ entity_id: 'not-cached' }); @@ -59,34 +62,53 @@ describe('EntityRegistryManager', () => { vi.mocked(homeAssistantWSRequest).mockRejectedValueOnce(new Error('Not found')); expect( - manager.getEntities(createHASS(), ['cached', 'not-cached', 'missing']), - ).resolves.toEqual( + await manager.getEntities(createHASS(), ['cached', 'not-cached', 'missing']), + ).toEqual( new Map([ ['cached', cachedEntity], ['not-cached', notCachedEntity], ]), ); + + vi.mocked(expect(console.warn)).toBeCalledWith('Not found'); }); - it('fetchEntityList', async () => { - const hass = createHASS(); - const entity = createRegistryEntity({ entity_id: 'cached' }); - vi.mocked(homeAssistantWSRequest).mockResolvedValueOnce([entity]); + describe('fetchEntityList', async () => { + it('should fetch entire entity list once', async () => { + const hass = createHASS(); + const entity = createRegistryEntity({ entity_id: 'cached' }); + vi.mocked(homeAssistantWSRequest).mockResolvedValueOnce([entity]); - const manager = new EntityRegistryManager(createEntityRegistryCache()); + const manager = new EntityRegistryManager(createEntityRegistryCache()); - await manager.fetchEntityList(hass); + await manager.fetchEntityList(hass); - expect(homeAssistantWSRequest).toBeCalledTimes(1); - expect(homeAssistantWSRequest).toBeCalledWith(expect.anything(), expect.anything(), { - type: 'config/entity_registry/list', + expect(homeAssistantWSRequest).toBeCalledTimes(1); + expect(homeAssistantWSRequest).toBeCalledWith( + expect.anything(), + expect.anything(), + { + type: 'config/entity_registry/list', + }, + ); + + expect(await manager.getEntity(hass, 'cached')).toEqual(entity); + expect(homeAssistantWSRequest).toBeCalledTimes(1); + + await manager.fetchEntityList(hass); + expect(homeAssistantWSRequest).toBeCalledTimes(1); }); - expect(await manager.getEntity(hass, 'cached')).toEqual(entity); - expect(homeAssistantWSRequest).toBeCalledTimes(1); + it('should log to console on error', async () => { + const hass = createHASS(); + vi.mocked(homeAssistantWSRequest).mockRejectedValueOnce(new Error('Fetch error')); - await manager.fetchEntityList(hass); - expect(homeAssistantWSRequest).toBeCalledTimes(1); + const manager = new EntityRegistryManager(createEntityRegistryCache()); + + await manager.fetchEntityList(hass); + + vi.mocked(expect(console.warn)).toBeCalledWith('Fetch error'); + }); }); it('getMatchingEntities', async () => { diff --git a/vite.config.ts b/vite.config.ts index e85dffcd..78d0127b 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -25,7 +25,7 @@ const FULL_COVERAGE_FILES_RELATIVE = [ 'utils/download.ts', 'utils/embla/**/*.ts', 'utils/endpoint.ts', - 'utils/ha/registry/entity/**/*.ts', + 'utils/ha/registry/**/*.ts', 'utils/ha/types.ts', 'utils/initializer.ts', 'utils/interaction-mode.ts',