From 3bb58f83fcbc4463f7ea8cf42a7213aa2c8a8ab1 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Mon, 12 Jan 2026 10:23:31 +0100 Subject: [PATCH] refactor: extract shared DB upgrade logic and encapsulate keyboard layout state - Extract IndexedDB schema upgrade logic to shared db-upgrade.ts to prevent maintenance drift between OperationLogStoreService and ArchiveStoreService - Create KeyboardLayoutService to encapsulate global mutable keyboard layout state, improving testability and SSR compatibility - Maintain backwards compatibility with existing tests via fallback to deprecated userKbLayout when service is not connected --- src/app/app.component.ts | 8 +- .../keyboard-layout.service.ts | 75 +++++++++++++++++++ .../persistence/archive-store.service.ts | 56 ++------------ src/app/op-log/persistence/db-upgrade.ts | 64 ++++++++++++++++ .../operation-log-store.service.ts | 39 +--------- src/app/util/check-key-combo.ts | 72 +++++++++++------- 6 files changed, 200 insertions(+), 114 deletions(-) create mode 100644 src/app/core/keyboard-layout/keyboard-layout.service.ts create mode 100644 src/app/op-log/persistence/db-upgrade.ts diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 79eeb3371..efcc623f4 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -66,7 +66,8 @@ import { WorkContextThemeCfg } from './features/work-context/work-context.model' import { isInputElement } from './util/dom-element'; import { MobileBottomNavComponent } from './core-ui/mobile-bottom-nav/mobile-bottom-nav.component'; import { StartupService } from './core/startup/startup.service'; -import { saveUserKbLayout } from './util/check-key-combo'; +import { KeyboardLayoutService } from './core/keyboard-layout/keyboard-layout.service'; +import { setKeyboardLayoutService } from './util/check-key-combo'; const w = window as Window & { productivityTips?: string[][]; randomIndex?: number }; const productivityTip: string[] | undefined = @@ -129,6 +130,7 @@ export class AppComponent implements OnDestroy, AfterViewInit { private _ngZone = inject(NgZone); private _document = inject(DOCUMENT, { optional: true }); private _startupService = inject(StartupService); + private _keyboardLayoutService = inject(KeyboardLayoutService); readonly syncTriggerService = inject(SyncTriggerService); readonly imexMetaService = inject(ImexViewService); @@ -228,7 +230,9 @@ export class AppComponent implements OnDestroy, AfterViewInit { }); // ! For keyboard shortcuts to work correctly with any layouts (QWERTZ/AZERTY/etc) - user's keyboard layout must be presaved - saveUserKbLayout(); + // Connect the service to the utility functions and save the layout + setKeyboardLayoutService(this._keyboardLayoutService); + this._keyboardLayoutService.saveUserLayout(); } skipInitialSync(): void { diff --git a/src/app/core/keyboard-layout/keyboard-layout.service.ts b/src/app/core/keyboard-layout/keyboard-layout.service.ts new file mode 100644 index 000000000..0c2d30f80 --- /dev/null +++ b/src/app/core/keyboard-layout/keyboard-layout.service.ts @@ -0,0 +1,75 @@ +import { Injectable } from '@angular/core'; + +export interface NavigatorWithKeyboard { + keyboard?: NavigatorKeyboard; +} + +export interface NavigatorKeyboard { + getLayoutMap: () => Promise>; +} + +/** + * A Map where keys are string representations of key codes, + * and values are the corresponding characters or symbols for this layout. + * + * See https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap#browser_compatibility + */ +export type KeyboardLayout = Map; + +/** + * Service that manages the user's keyboard layout mapping. + * + * Encapsulates the keyboard layout state to avoid global mutable state, + * which caused issues in parallel tests and SSR scenarios. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap + */ +@Injectable({ + providedIn: 'root', +}) +export class KeyboardLayoutService { + private _layout: KeyboardLayout = new Map(); + + /** + * Gets the current keyboard layout map. + * Returns an empty map if the layout hasn't been initialized. + */ + get layout(): KeyboardLayout { + return this._layout; + } + + /** + * Saves the user's keyboard layout mapping from the browser's Keyboard API. + * Should be called once during app initialization. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap + */ + async saveUserLayout(): Promise { + // If browser doesn't support keyboard API + if (!('keyboard' in navigator)) return; + + const keyboard = (navigator as NavigatorWithKeyboard).keyboard; + if (!keyboard) return; + + const kbLayout = await keyboard.getLayoutMap(); + this._layout.clear(); + kbLayout.forEach((value, key) => this._layout.set(key, value)); + } + + /** + * Clears the keyboard layout. Used for testing. + * @internal + */ + clear(): void { + this._layout.clear(); + } + + /** + * Sets the keyboard layout directly. Used for testing. + * @internal + */ + setLayout(layout: KeyboardLayout): void { + this._layout.clear(); + layout.forEach((value, key) => this._layout.set(key, value)); + } +} diff --git a/src/app/op-log/persistence/archive-store.service.ts b/src/app/op-log/persistence/archive-store.service.ts index 2428bac95..ca515a357 100644 --- a/src/app/op-log/persistence/archive-store.service.ts +++ b/src/app/op-log/persistence/archive-store.service.ts @@ -1,13 +1,8 @@ import { Injectable } from '@angular/core'; import { IDBPDatabase, openDB } from 'idb'; import { ArchiveModel } from '../../features/time-tracking/time-tracking.model'; -import { - DB_NAME, - DB_VERSION, - STORE_NAMES, - SINGLETON_KEY, - OPS_INDEXES, -} from './db-keys.const'; +import { DB_NAME, DB_VERSION, STORE_NAMES, SINGLETON_KEY } from './db-keys.const'; +import { runDbUpgrade } from './db-upgrade'; /** * Entry stored in archive_young or archive_old object stores. @@ -62,52 +57,15 @@ export class ArchiveStoreService { /** * Opens the SUP_OPS database for archive operations. * - * IMPORTANT: This includes the COMPLETE upgrade callback to ensure ALL stores - * are created if ArchiveStoreService opens the database before OperationLogStoreService. - * IndexedDB only runs ONE upgrade callback per version transition, so whichever - * service opens the DB first MUST create all stores. - * - * This is critical for test environments where browser state may be cleared between - * test runs, causing a fresh database to be created by whichever service initializes first. + * Uses shared runDbUpgrade() to ensure ALL stores are created if ArchiveStoreService + * opens the database before OperationLogStoreService. IndexedDB only runs ONE upgrade + * callback per version transition, so whichever service opens the DB first MUST create + * all stores. */ private async _init(): Promise { this._db = await openDB(DB_NAME, DB_VERSION, { upgrade: (db, oldVersion, _newVersion, transaction) => { - // IMPORTANT: This must mirror OperationLogStoreService.init() upgrade logic exactly! - // If this service opens the DB first, it must create ALL stores. - - // Version 1: Create initial stores - if (oldVersion < 1) { - const opStore = db.createObjectStore(STORE_NAMES.OPS, { - keyPath: 'seq', - autoIncrement: true, - }); - opStore.createIndex(OPS_INDEXES.BY_ID, 'op.id', { unique: true }); - opStore.createIndex(OPS_INDEXES.BY_SYNCED_AT, 'syncedAt'); - - db.createObjectStore(STORE_NAMES.STATE_CACHE, { keyPath: 'id' }); - db.createObjectStore(STORE_NAMES.IMPORT_BACKUP, { keyPath: 'id' }); - } - - // Version 2: Add vector_clock store for atomic writes - if (oldVersion < 2) { - db.createObjectStore(STORE_NAMES.VECTOR_CLOCK); - } - - // Version 3: Add compound index for efficient source+status queries - if (oldVersion < 3) { - const opStore = transaction.objectStore(STORE_NAMES.OPS); - opStore.createIndex(OPS_INDEXES.BY_SOURCE_AND_STATUS, [ - 'source', - 'applicationStatus', - ]); - } - - // Version 4: Add archive stores for archiveYoung and archiveOld - if (oldVersion < 4) { - db.createObjectStore(STORE_NAMES.ARCHIVE_YOUNG, { keyPath: 'id' }); - db.createObjectStore(STORE_NAMES.ARCHIVE_OLD, { keyPath: 'id' }); - } + runDbUpgrade(db, oldVersion, transaction); }, }); } diff --git a/src/app/op-log/persistence/db-upgrade.ts b/src/app/op-log/persistence/db-upgrade.ts new file mode 100644 index 000000000..1e101b87d --- /dev/null +++ b/src/app/op-log/persistence/db-upgrade.ts @@ -0,0 +1,64 @@ +/** + * Shared IndexedDB upgrade logic for SUP_OPS database. + * + * CRITICAL: This upgrade function MUST be used by ALL services that open the SUP_OPS database. + * IndexedDB only runs ONE upgrade callback per version transition - whichever service opens + * the database first MUST create ALL stores. + * + * This shared function ensures schema consistency regardless of service initialization order. + */ + +import { IDBPDatabase, IDBPTransaction } from 'idb'; +import { STORE_NAMES, OPS_INDEXES } from './db-keys.const'; + +/** + * Performs the database upgrade for SUP_OPS. + * Called from openDB's upgrade callback in both OperationLogStoreService and ArchiveStoreService. + * + * @param db The IDBPDatabase instance + * @param oldVersion The previous version (0 for new databases) + * @param transaction The upgrade transaction (needed to access existing stores) + */ +export const runDbUpgrade = ( + db: IDBPDatabase, + oldVersion: number, + transaction: IDBPTransaction, +): void => { + // Version 1: Create initial stores + if (oldVersion < 1) { + const opStore = db.createObjectStore(STORE_NAMES.OPS, { + keyPath: 'seq', + autoIncrement: true, + }); + opStore.createIndex(OPS_INDEXES.BY_ID, 'op.id', { unique: true }); + opStore.createIndex(OPS_INDEXES.BY_SYNCED_AT, 'syncedAt'); + + db.createObjectStore(STORE_NAMES.STATE_CACHE, { keyPath: 'id' }); + db.createObjectStore(STORE_NAMES.IMPORT_BACKUP, { keyPath: 'id' }); + } + + // Version 2: Add vector_clock store for atomic writes + // This consolidates the vector clock from pf.META_MODEL into SUP_OPS + // to enable single-transaction writes (op + vector clock together) + if (oldVersion < 2) { + db.createObjectStore(STORE_NAMES.VECTOR_CLOCK); + } + + // Version 3: Add compound index for efficient source+status queries + // PERF: Enables O(results) queries for getPendingRemoteOps/getFailedRemoteOps + // instead of O(all ops) full table scan + if (oldVersion < 3) { + const opStore = transaction.objectStore(STORE_NAMES.OPS); + opStore.createIndex(OPS_INDEXES.BY_SOURCE_AND_STATUS, [ + 'source', + 'applicationStatus', + ]); + } + + // Version 4: Add archive stores for archiveYoung and archiveOld + // Consolidates archive data from legacy 'pf' database into SUP_OPS + if (oldVersion < 4) { + db.createObjectStore(STORE_NAMES.ARCHIVE_YOUNG, { keyPath: 'id' }); + db.createObjectStore(STORE_NAMES.ARCHIVE_OLD, { keyPath: 'id' }); + } +}; diff --git a/src/app/op-log/persistence/operation-log-store.service.ts b/src/app/op-log/persistence/operation-log-store.service.ts index 9716b28c4..8c233adda 100644 --- a/src/app/op-log/persistence/operation-log-store.service.ts +++ b/src/app/op-log/persistence/operation-log-store.service.ts @@ -22,6 +22,7 @@ import { BACKUP_KEY, OPS_INDEXES, } from './db-keys.const'; +import { runDbUpgrade } from './db-upgrade'; /** * Vector clock entry stored in the vector_clock object store. @@ -172,43 +173,7 @@ export class OperationLogStoreService { async init(): Promise { this._db = await openDB(DB_NAME, DB_VERSION, { upgrade: (db, oldVersion, _newVersion, transaction) => { - // Version 1: Create initial stores - if (oldVersion < 1) { - const opStore = db.createObjectStore(STORE_NAMES.OPS, { - keyPath: 'seq', - autoIncrement: true, - }); - opStore.createIndex(OPS_INDEXES.BY_ID, 'op.id', { unique: true }); - opStore.createIndex(OPS_INDEXES.BY_SYNCED_AT, 'syncedAt'); - - db.createObjectStore(STORE_NAMES.STATE_CACHE, { keyPath: 'id' }); - db.createObjectStore(STORE_NAMES.IMPORT_BACKUP, { keyPath: 'id' }); - } - - // Version 2: Add vector_clock store for atomic writes - // This consolidates the vector clock from pf.META_MODEL into SUP_OPS - // to enable single-transaction writes (op + vector clock together) - if (oldVersion < 2) { - db.createObjectStore(STORE_NAMES.VECTOR_CLOCK); - } - - // Version 3: Add compound index for efficient source+status queries - // PERF: Enables O(results) queries for getPendingRemoteOps/getFailedRemoteOps - // instead of O(all ops) full table scan - if (oldVersion < 3) { - const opStore = transaction.objectStore(STORE_NAMES.OPS); - opStore.createIndex(OPS_INDEXES.BY_SOURCE_AND_STATUS, [ - 'source', - 'applicationStatus', - ]); - } - - // Version 4: Add archive stores for archiveYoung and archiveOld - // Consolidates archive data from legacy 'pf' database into SUP_OPS - if (oldVersion < 4) { - db.createObjectStore(STORE_NAMES.ARCHIVE_YOUNG, { keyPath: 'id' }); - db.createObjectStore(STORE_NAMES.ARCHIVE_OLD, { keyPath: 'id' }); - } + runDbUpgrade(db, oldVersion, transaction); }, }); } diff --git a/src/app/util/check-key-combo.ts b/src/app/util/check-key-combo.ts index b6107dd32..0333bc0b3 100644 --- a/src/app/util/check-key-combo.ts +++ b/src/app/util/check-key-combo.ts @@ -1,18 +1,12 @@ -export interface NavigatorWithKeyboard { - keyboard?: NavigatorKeyboard; -} +import { + KeyboardLayoutService, + KeyboardLayout, + NavigatorWithKeyboard, + NavigatorKeyboard, +} from '../core/keyboard-layout/keyboard-layout.service'; -export interface NavigatorKeyboard { - getLayoutMap: () => Promise>; -} - -/** - * A Map where keys are string representations of key codes, - * and values are the corresponding characters or symbols for this layout. - * - * See https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap#browser_compatibility - */ -export type KeyboardLayout = Map; +// Re-export types for backwards compatibility +export type { KeyboardLayout, NavigatorWithKeyboard, NavigatorKeyboard }; // Just an alias for better readability export const KEYS = { @@ -26,18 +20,42 @@ export const KEYS = { }, } as const; -/** Saved user keyboard layout */ +/** + * Module-level reference to the keyboard layout service. + * Initialized lazily when first accessed. + */ +let _keyboardLayoutService: KeyboardLayoutService | null = null; + +/** + * Sets the keyboard layout service instance. + * Called from app initialization to connect the service to these utility functions. + */ +export const setKeyboardLayoutService = (service: KeyboardLayoutService): void => { + _keyboardLayoutService = service; +}; + +/** + * @deprecated Use KeyboardLayoutService.layout instead. + * Provided for backwards compatibility with tests. + */ export const userKbLayout: KeyboardLayout = new Map(); -/** Try to save user kayboard layout mapping - https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap */ +/** + * @deprecated Use KeyboardLayoutService.saveUserLayout() instead. + * Provided for backwards compatibility. + */ export const saveUserKbLayout = async (): Promise => { - // If browser doesn't support keyboard API https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap - if (!('keyboard' in navigator)) return; - - const keyboard = navigator.keyboard as NavigatorKeyboard; - const kbLayout = await keyboard.getLayoutMap(); - userKbLayout.clear(); - kbLayout.forEach((value, key) => userKbLayout.set(key, value)); + if (_keyboardLayoutService) { + await _keyboardLayoutService.saveUserLayout(); + } else { + // Fallback for when service is not available (e.g., tests without DI) + if (!('keyboard' in navigator)) return; + const keyboard = (navigator as NavigatorWithKeyboard).keyboard; + if (!keyboard) return; + const kbLayout = await keyboard.getLayoutMap(); + userKbLayout.clear(); + kbLayout.forEach((value, key) => userKbLayout.set(key, value)); + } }; /** @@ -80,9 +98,11 @@ export const prepareKeyCode = (code: KeyboardEvent['code']): string => { }, }; - // Try to use user kayboard layout mapping - https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap - if (!rules.codeMapping[code] && userKbLayout.size) { - const foundKey = userKbLayout.get(code); + // Try to use user keyboard layout mapping - https://developer.mozilla.org/en-US/docs/Web/API/KeyboardLayoutMap + // Use service layout if available, fall back to deprecated userKbLayout for backwards compat + const layout = _keyboardLayoutService?.layout ?? userKbLayout; + if (!rules.codeMapping[code] && layout.size) { + const foundKey = layout.get(code); if (foundKey) code = foundKey.toUpperCase(); }