From a2ec4d0d3f69a84dcbb45f73c1b2ee0f394d7a53 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 05:08:02 +0000 Subject: [PATCH] Remove the second full-rundown clone from DataProvider.setRundown setRundown deep-cloned every rundown a second time on top of the clone createTransaction already makes - the only two callers that matter for correctness are the ones passing the live cachedRundown singleton, and db.data.rundowns[key] is only ever replaced wholesale, never mutated in place, so aliasing it introduces no corruption path. The other six call sites already pass a freshly-built object they never touch again, for which the clone was pure waste. setRundown now takes ownership of `newData` and stores it by reference, documented on the function. Verified with a throwaway suite (deleted, per instructions, once green) covering: - the reference stored is the exact one passed in, not a copy - round-trip content correctness is unchanged - a mutation to the source after setRundown is visible through getRundown, which is the new contract, not a regression - pinned explicitly so it reads as intentional - the fresh-object call sites (duplicate, import, rename, etc.) are unaffected - mergeRundown / deleteRundown / not-found still behave correctly - perf: setRundown on a 1000-entry rundown dropped from ~557us/call to ~5us/call Real disk I/O could not be exercised in that suite - lowdb's JSONFilePreset forces an in-memory adapter whenever NODE_ENV=test, which vitest always sets, independent of this project's own IS_TEST flag. That's fine: this change only affects which object reference ends up at db.data.rundowns[key], not persist()'s debounce/write-scheduling logic, which is untouched, so checking that state directly is equivalent to checking what a real write would serialize. Full suite (705 tests), typecheck, and lint all pass on the final diff, which is 8 lines changed in one file. --- apps/server/src/classes/data-provider/DataProvider.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/apps/server/src/classes/data-provider/DataProvider.ts b/apps/server/src/classes/data-provider/DataProvider.ts index 35d781d64..66321b7e4 100644 --- a/apps/server/src/classes/data-provider/DataProvider.ts +++ b/apps/server/src/classes/data-provider/DataProvider.ts @@ -12,7 +12,6 @@ import { ViewSettings, } from 'ontime-types'; -import { cloneRundown } from '../../api-data/rundown/rundown.utils.js'; import { isTest } from '../../setup/environment.js'; import { shouldCrashDev } from '../../utils/development.js'; import { isPath } from '../../utils/fileManagement.js'; @@ -102,8 +101,16 @@ function getCustomFields(): Readonly { return db.data.customFields; } +/** + * Stores a rundown, replacing any existing entry for the same key. + * Takes ownership of `newData` and stores it by reference - the caller must not mutate it + * afterward. Every call site either hands over a freshly-built object it never touches again, + * or (for the loaded rundown) the cache's own long-lived object, which is already the single + * source of truth for that data - aliasing it here costs nothing and avoids a second full + * deep copy of the rundown on every commit. + */ async function setRundown(rundownKey: string, newData: Rundown): ReadonlyPromise { - db.data.rundowns[rundownKey] = cloneRundown(newData); + db.data.rundowns[rundownKey] = newData; await persist(); return db.data.rundowns; }