From dddb11ff404ab4cfc7b33fe34efb4aa049e1a2d6 Mon Sep 17 00:00:00 2001
From: Carlos Valente <34649812+cpvalente@users.noreply.github.com>
Date: Fri, 22 Mar 2024 13:21:49 +0100
Subject: [PATCH] refactor: granular updates to rundown (#848)

---
 .../__tests__/rundownCacheUtils.test.ts       | 68 ++++++++++++++++++-
 .../services/rundown-service/rundownCache.ts  | 25 +++++--
 .../rundown-service/rundownCacheUtils.ts      | 45 +++++++++++-
 3 files changed, 131 insertions(+), 7 deletions(-)

diff --git a/apps/server/src/services/rundown-service/__tests__/rundownCacheUtils.test.ts b/apps/server/src/services/rundown-service/__tests__/rundownCacheUtils.test.ts
index 2366dc39c..5dbcc6a34 100644
--- a/apps/server/src/services/rundown-service/__tests__/rundownCacheUtils.test.ts
+++ b/apps/server/src/services/rundown-service/__tests__/rundownCacheUtils.test.ts
@@ -1,5 +1,20 @@
-import { CustomFields, OntimeEvent, OntimeRundown, SupportedEvent } from 'ontime-types';
-import { addToCustomAssignment, getLink, handleCustomField, handleLink } from '../rundownCacheUtils.js';
+import {
+  CustomFields,
+  EndAction,
+  OntimeEvent,
+  OntimeRundown,
+  SupportedEvent,
+  TimeStrategy,
+  TimerType,
+} from 'ontime-types';
+import {
+  addToCustomAssignment,
+  getLink,
+  handleCustomField,
+  handleLink,
+  hasChanges,
+  isDataStale,
+} from '../rundownCacheUtils.js';
 
 describe('getLink()', () => {
   it('should return null if there is no link', () => {
@@ -187,3 +202,52 @@ describe('handleCustomField()', () => {
     });
   });
 });
+
+describe('isDataStale()', () => {
+  it('is stale if data contains timers', () => {
+    const needsRecompute = [
+      { timeStart: 10 },
+      { timeEnd: 10 },
+      { duration: 10 },
+      { linkStart: '1' },
+      { timerStrategy: TimeStrategy.LockDuration },
+    ];
+
+    for (const testCase of needsRecompute) {
+      expect(isDataStale(testCase)).toBe(true);
+    }
+    expect.assertions(needsRecompute.length);
+  });
+
+  it('is not stale if data contains auxiliary dataset', () => {
+    expect(
+      isDataStale({
+        cue: 'cue',
+        title: 'title',
+        note: 'note',
+        endAction: EndAction.LoadNext,
+        timerType: TimerType.Clock,
+        isPublic: false,
+        colour: 'colour',
+        timeWarning: 1,
+        timeDanger: 2,
+        custom: {
+          lighting: { value: '3' },
+        },
+      }),
+    ).toBe(false);
+  });
+});
+
+describe('hasChanges()', () => {
+  it('identifies objects with new values', () => {
+    const newEvent = { id: '1', title: 'new-title' } as OntimeEvent;
+    const existing = { id: '1', cue: 'cue', title: 'title' } as OntimeEvent;
+    expect(hasChanges(existing, newEvent)).toBe(true);
+  });
+  it('identifies objects with all same values', () => {
+    const newEvent = { id: '1', title: 'title' } as OntimeEvent;
+    const existing = { id: '1', cue: 'cue', title: 'title' } as OntimeEvent;
+    expect(hasChanges(existing, newEvent)).toBe(false);
+  });
+});
diff --git a/apps/server/src/services/rundown-service/rundownCache.ts b/apps/server/src/services/rundown-service/rundownCache.ts
index 79bba59b6..e9815cab5 100644
--- a/apps/server/src/services/rundown-service/rundownCache.ts
+++ b/apps/server/src/services/rundown-service/rundownCache.ts
@@ -15,7 +15,7 @@ import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { createPatch } from '../../utils/parser.js';
 import { getTotalDuration } from '../timerUtils.js';
 import { apply } from './delayUtils.js';
-import { handleCustomField, handleLink } from './rundownCacheUtils.js';
+import { handleCustomField, handleLink, hasChanges, isDataStale } from './rundownCacheUtils.js';
 
 type EventID = string;
 type NormalisedRundown = Record<EventID, OntimeRundownEntry>;
@@ -225,20 +225,25 @@ type MutatingFn<T extends object> = (params: MutationParams<T>) => MutatingRetur
  */
 export function mutateCache<T extends object>(mutation: MutatingFn<T>) {
   async function scopedMutation(params: T) {
+    /**
+     * Marking the data set as stale
+     * doing it before calling the mutation, gives the function a chance
+     * to prevent recalculation by setting stale = false
+     */
+    isStale = true;
+
     const { newEvent, newRundown } = mutation({ ...params, persistedRundown });
 
     revision = revision + 1;
-    isStale = true;
     persistedRundown = newRundown;
 
     // schedule a non priority cache update
     setImmediate(() => {
       console.time('rundownCache__init');
-      generate();
+      get();
       console.timeEnd('rundownCache__init');
     });
 
-    // TODO: should we throttle this?
     // defer writing to the database
     setImmediate(() => {
       DataProvider.setRundown(persistedRundown);
@@ -302,11 +307,23 @@ export function edit({ persistedRundown, eventId, patch }: EditArgs): Required<M
   }
 
   const eventInMemory = persistedRundown[indexAt];
+  if (!hasChanges(eventInMemory, patch)) {
+    isStale = false;
+    return;
+  }
+
   const newEvent = makeEvent(eventInMemory, patch);
 
   const newRundown = [...persistedRundown];
   newRundown[indexAt] = newEvent;
 
+  const makeStale = isDataStale(patch);
+
+  if (!makeStale) {
+    rundown[newEvent.id] = newEvent;
+  }
+
+  isStale = makeStale;
   return { newRundown, newEvent };
 }
 
diff --git a/apps/server/src/services/rundown-service/rundownCacheUtils.ts b/apps/server/src/services/rundown-service/rundownCacheUtils.ts
index 0fe4c4793..e38604fb3 100644
--- a/apps/server/src/services/rundown-service/rundownCacheUtils.ts
+++ b/apps/server/src/services/rundown-service/rundownCacheUtils.ts
@@ -1,4 +1,12 @@
-import { OntimeEvent, isOntimeEvent, OntimeRundown, CustomFieldLabel, CustomFields } from 'ontime-types';
+import {
+  OntimeEvent,
+  isOntimeEvent,
+  OntimeRundown,
+  CustomFieldLabel,
+  CustomFields,
+  OntimeRundownEntry,
+  OntimeBaseEvent,
+} from 'ontime-types';
 import { getLinkedTimes } from 'ontime-utils';
 
 /**
@@ -90,3 +98,38 @@ export function handleCustomField(
     }
   }
 }
+
+/** List of event properties which do not need the rundown to be regenerated */
+enum regenerateWhitelist {
+  'id',
+  'cue',
+  'title',
+  'note',
+  'endAction',
+  'timerType',
+  'isPublic',
+  'colour',
+  'timeWarning',
+  'timeDanger',
+  'custom',
+}
+
+/**
+ * given a patch, returns whether all keys are whitelisted
+ * @param path
+ */
+export function isDataStale(patch: Partial<OntimeRundownEntry>): boolean {
+  return Object.keys(patch).some((key) => !(key in regenerateWhitelist));
+}
+
+/**
+ * Given an event and a patch to that event checks whether there are actual changes to the dataset
+ * @param existingEvent
+ * @param newEvent
+ * @returns
+ */
+export function hasChanges<T extends OntimeBaseEvent>(existingEvent: T, newEvent: Partial<T>): boolean {
+  return Object.keys(newEvent).some(
+    (key) => !Object.hasOwn(existingEvent, key) || existingEvent[key] !== newEvent[key],
+  );
+}