feat(dashboardapi): diff-skip regen via flows:started predicate (#36)
Subscribes to Node-RED's flows:started runtime event, caches the {diff}
payload on the dashboardAPI source, and short-circuits the child.register
handler when none of {dashboardAPI id, child id, grandchild ids} appears in
diff.added/changed/removed/rewired. Predicate verified by S1 spike (#32).
- src/nodeClass.js: _attachLifecycleHook subscribes, _attachCloseHandler
cleans up. No-op when RED.events isn't available (unit-test friendly).
- src/specificClass.js: subtreeChanged() predicate + subtreeIdsFor() helper.
- src/commands/handlers.js: registerChild consults predicate before composing;
logs {event:'regen-skipped', outcome:'no-diff'} when skipping.
- test/basic/slice36-diff-predicate.basic.test.js: 8 cases — null/empty diff,
affected/unaffected ids, tab-id over-triggering avoidance, grandchild
inclusion, no-grandchild case.
Cold start (no cached diff yet) always regenerates — safe default. Edge case
documented in #32: when a brand-new child is wired to a registered parent,
the new child's id is in diff.added but not yet in registeredChildren when
flows:started fires. Mitigation (b) per spike findings: one-deploy race
accepted for R&D — next deploy picks up the new registration.
Closes #36
This commit is contained in:
@@ -24,12 +24,33 @@ function resolveChildNode(childId, ctx) {
|
||||
|
||||
// On child.register: build the dashboard graph (root + direct children) and
|
||||
// emit one Grafana upsert HTTP request per dashboard on Port 0.
|
||||
//
|
||||
// Diff-skip behavior (PRD F-1, S1 spike #32): if the latest flows:started
|
||||
// payload's `diff` indicates that NEITHER the dashboardAPI itself NOR this
|
||||
// child NOR its grandchildren changed, skip composition and log no-diff. The
|
||||
// first call after startup (no cached diff yet) regenerates unconditionally.
|
||||
function registerChild(source, msg, ctx) {
|
||||
const childSource = resolveChildSource(msg.payload, ctx);
|
||||
if (!childSource?.config) {
|
||||
throw new Error('Missing or invalid child node');
|
||||
}
|
||||
|
||||
const subtreeIds = source.subtreeIdsFor(ctx.node?.id, childSource);
|
||||
const changed = source.subtreeChanged(source.lastFlowsStartedDiff, subtreeIds);
|
||||
if (!changed) {
|
||||
if (source.logger?.info) {
|
||||
source.logger.info({
|
||||
event: 'regen-skipped',
|
||||
outcome: 'no-diff',
|
||||
trigger: 'child.register',
|
||||
dashboardApiId: ctx.node?.id,
|
||||
childId: childSource?.config?.general?.id,
|
||||
subtreeSize: subtreeIds.size,
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const dashboards = source.generateDashboardsForGraph(childSource, {
|
||||
includeChildren: Boolean(msg.includeChildren ?? true),
|
||||
});
|
||||
|
||||
@@ -26,6 +26,29 @@ class nodeClass {
|
||||
|
||||
this._attachInputHandler();
|
||||
this._attachCloseHandler();
|
||||
this._attachLifecycleHook();
|
||||
}
|
||||
|
||||
// Subscribe to Node-RED's `flows:started` event to cache the deploy diff so
|
||||
// the child.register handler can decide whether *this* dashboardAPI's
|
||||
// subtree was affected. Predicate documented in Gitea issue #32 spike.
|
||||
_attachLifecycleHook() {
|
||||
if (!this.RED?.events?.on) return;
|
||||
this._flowsStartedListener = (payload) => {
|
||||
const diff = payload?.diff || null;
|
||||
this.source.lastFlowsStartedDiff = diff;
|
||||
this.source.lastFlowsStartedAt = Date.now();
|
||||
if (this.source?.logger?.debug) {
|
||||
const summary = diff
|
||||
? Object.fromEntries(
|
||||
['added', 'changed', 'removed', 'rewired', 'linked', 'flowChanged']
|
||||
.map((k) => [k, (diff[k] || []).length])
|
||||
)
|
||||
: null;
|
||||
this.source.logger.debug({ event: 'flows:started', type: payload?.type, diff: summary });
|
||||
}
|
||||
};
|
||||
this.RED.events.on('flows:started', this._flowsStartedListener);
|
||||
}
|
||||
|
||||
_buildConfig(uiConfig) {
|
||||
@@ -78,6 +101,10 @@ class nodeClass {
|
||||
|
||||
_attachCloseHandler() {
|
||||
this.node.on('close', (done) => {
|
||||
if (this._flowsStartedListener && this.RED?.events?.off) {
|
||||
this.RED.events.off('flows:started', this._flowsStartedListener);
|
||||
this._flowsStartedListener = null;
|
||||
}
|
||||
if (typeof done === 'function') done();
|
||||
});
|
||||
}
|
||||
|
||||
@@ -168,6 +168,34 @@ class DashboardApi {
|
||||
return out;
|
||||
}
|
||||
|
||||
// Predicate from Gitea issue #32 spike (S1 findings). Given the diff payload
|
||||
// from Node-RED's flows:started event and a set of node ids that constitute
|
||||
// "my subtree", decides whether the subtree changed on this deploy.
|
||||
// `null` diff (first deploy / startup) → always regen (safe default).
|
||||
subtreeChanged(diff, subtreeIds) {
|
||||
if (!diff) return true;
|
||||
const mine = new Set(subtreeIds);
|
||||
for (const field of ['added', 'changed', 'removed', 'rewired']) {
|
||||
const arr = diff[field] || [];
|
||||
if (arr.some((id) => mine.has(id))) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Collect ids that constitute "this dashboardAPI + this child + its grandchildren"
|
||||
// for the diff predicate. Pulls grandchildren via the existing extractChildren walk.
|
||||
subtreeIdsFor(dashboardApiNodeId, childSource) {
|
||||
const ids = new Set();
|
||||
if (dashboardApiNodeId) ids.add(dashboardApiNodeId);
|
||||
const childId = childSource?.config?.general?.id;
|
||||
if (childId) ids.add(childId);
|
||||
for (const { childSource: gc } of this.extractChildren(childSource)) {
|
||||
const gcId = gc?.config?.general?.id;
|
||||
if (gcId) ids.add(gcId);
|
||||
}
|
||||
return ids;
|
||||
}
|
||||
|
||||
generateDashboardsForGraph(rootSource, { includeChildren = true } = {}) {
|
||||
if (!rootSource?.config) {
|
||||
this.logger.warn('generateDashboardsForGraph skipped: root source missing config');
|
||||
|
||||
73
test/basic/slice36-diff-predicate.basic.test.js
Normal file
73
test/basic/slice36-diff-predicate.basic.test.js
Normal file
@@ -0,0 +1,73 @@
|
||||
'use strict';
|
||||
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const DashboardApi = require('../../src/specificClass.js');
|
||||
|
||||
test('subtreeChanged: null diff → always regen (safe default for cold start)', () => {
|
||||
const api = new DashboardApi({});
|
||||
assert.equal(api.subtreeChanged(null, new Set(['a', 'b'])), true);
|
||||
assert.equal(api.subtreeChanged(undefined, new Set(['a', 'b'])), true);
|
||||
});
|
||||
|
||||
test('subtreeChanged: empty diff arrays → no regen needed', () => {
|
||||
const api = new DashboardApi({});
|
||||
const diff = { added: [], changed: [], removed: [], rewired: [], linked: [], flowChanged: [] };
|
||||
assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), false);
|
||||
});
|
||||
|
||||
test('subtreeChanged: id in added → regen', () => {
|
||||
const api = new DashboardApi({});
|
||||
const diff = { added: ['x', 'b'], changed: [], removed: [], rewired: [] };
|
||||
assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), true);
|
||||
});
|
||||
|
||||
test('subtreeChanged: id in changed → regen', () => {
|
||||
const api = new DashboardApi({});
|
||||
const diff = { added: [], changed: ['a'], removed: [], rewired: [] };
|
||||
assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), true);
|
||||
});
|
||||
|
||||
test('subtreeChanged: only unrelated ids → no regen', () => {
|
||||
const api = new DashboardApi({});
|
||||
const diff = { added: ['z'], changed: ['y'], removed: ['x'], rewired: ['w'] };
|
||||
assert.equal(api.subtreeChanged(diff, new Set(['a', 'b'])), false);
|
||||
});
|
||||
|
||||
test('subtreeChanged: tab id in diff but not in subtree → no regen', () => {
|
||||
// Tab id over-triggering avoidance: when an unrelated tab changes, its
|
||||
// tab id lands in changed/added but should not affect this dashboardAPI.
|
||||
const api = new DashboardApi({});
|
||||
const diff = { added: [], changed: ['unrelated_tab'], removed: [], rewired: [] };
|
||||
assert.equal(api.subtreeChanged(diff, new Set(['dashboardApiId', 'childA'])), false);
|
||||
});
|
||||
|
||||
test('subtreeIdsFor: includes dashboardAPI id + child id + grandchild ids', () => {
|
||||
const api = new DashboardApi({});
|
||||
const grandchild = {
|
||||
config: { general: { id: 'gc-1' }, functionality: { softwareType: 'measurement' } },
|
||||
};
|
||||
const grandchildEntry = { child: grandchild, position: 'downstream', softwareType: 'measurement' };
|
||||
const child = {
|
||||
config: { general: { id: 'child-1' }, functionality: { softwareType: 'pumpingStation' } },
|
||||
childRegistrationUtils: {
|
||||
registeredChildren: new Map([['gc-1', grandchildEntry]]),
|
||||
},
|
||||
};
|
||||
const ids = api.subtreeIdsFor('dApi-1', child);
|
||||
assert.equal(ids.has('dApi-1'), true);
|
||||
assert.equal(ids.has('child-1'), true);
|
||||
assert.equal(ids.has('gc-1'), true);
|
||||
assert.equal(ids.size, 3);
|
||||
});
|
||||
|
||||
test('subtreeIdsFor: handles child with no grandchildren', () => {
|
||||
const api = new DashboardApi({});
|
||||
const child = {
|
||||
config: { general: { id: 'child-1' }, functionality: { softwareType: 'measurement' } },
|
||||
};
|
||||
const ids = api.subtreeIdsFor('dApi-1', child);
|
||||
assert.equal(ids.size, 2);
|
||||
assert.ok(ids.has('dApi-1') && ids.has('child-1'));
|
||||
});
|
||||
Reference in New Issue
Block a user