diff --git a/CHANGELOG.md b/CHANGELOG.md index a6ed72a..f5f4465 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,21 @@ it no longer has. colours it with `.workflow-node-group .group-initial-mark { color: … }`. It takes 24 px of the title's room, and a frame is not made wider for its title: a long title in a narrow frame runs past it sooner. +- **A view can be of the instance, not of the definition.** A producer marks + a view it offers on a node with `of: 'instance'` (`meta.views`: + `[{ id, label, of? }]`). By name, a file answers for the definition it + holds under that name; for a unit made per instance (by a factory, with + the instance's parameters) that is another one. A view so marked is asked + of the editor's root, by the path of instances that leads to the node + (`rootNetwork` and `instancePath`, with `view`), as a drill-down into a + nested unit already falls back to; its file is asked by name only when the + root has no answer, and the reader is then told that what is shown is the + definition's (a warning, with the root's reason). The mark stays on the + view's crumb, so the trail leads back to the same view; it is acted on + only for a trail that reaches the node through an instance at every step. + It is said of a view a node offers, and acted on by the model source that + asks a sidecar for its graph: a product whose graph comes from a CLI's + plan asks by name, as before, as does any view without the mark. - **Lines of text in a node, read close in.** A producer puts lines of text in a node with `meta.lines` (`[{ text, kind? }]`, or plain strings; six at most, each cut at 48 characters): what a reader zooms in for and the @@ -634,6 +649,13 @@ it no longer has. ### Fixed +- **A view no longer falls back to the structure on a refresh.** The host's + refresh (a keystroke or a save in any source file, a run) repeats the + file, the name and the trail of what is shown, and named no view: a + synthesis view, or any other, was replaced by the unit's structure under a + breadcrumb that still said the view. The view is now taken from the + trail's last crumb when the request names none and is for that crumb's + unit. - **A wire into a closed bus of the unit's ports runs the way it does.** A bundle of the unit's ports that mixes directions (a handshake's `data` and `valid` in, its `ready` and `space` out) is drawn as an input, and every diff --git a/packages/diagram-client/src/editing-action-handlers.ts b/packages/diagram-client/src/editing-action-handlers.ts index 2cf1e16..a246e84 100644 --- a/packages/diagram-client/src/editing-action-handlers.ts +++ b/packages/diagram-client/src/editing-action-handlers.ts @@ -91,6 +91,8 @@ export namespace WorkflowOpenViewAction { workflowName: string; /** The instance the view is reached through, when through a node. */ workflowInstanceName?: string; + /** `'instance'`: the view is of that instance, not of the definition the file holds (`wf:views`' `of`). */ + viewOf?: 'instance'; } export function is(action: unknown): action is Action { @@ -947,7 +949,10 @@ export class WorkflowOpenViewActionHandler implements IActionHandler { return; } void this.actionDispatcher.dispatch( - this.navigationUi.requestView(action.uri, action.workflowName, action.workflowInstanceName, action.view, action.label) as never + this.navigationUi.requestView( + action.uri, action.workflowName, action.workflowInstanceName, action.view, action.label, + action.viewOf === 'instance' ? 'instance' : undefined + ) as never ); } } diff --git a/packages/diagram-client/src/navigation-ui.ts b/packages/diagram-client/src/navigation-ui.ts index 7d03df8..5dcd1cd 100644 --- a/packages/diagram-client/src/navigation-ui.ts +++ b/packages/diagram-client/src/navigation-ui.ts @@ -24,6 +24,8 @@ type NavigationCrumb = { /** The crumb is a view of the workflow other than its structure. */ view?: string; viewLabel?: string; + /** The view is of the instance the crumb was reached through, not of the definition its file holds. */ + viewOf?: 'instance'; }; /** What a crumb says: the instance or workflow, then the view when it is one. */ @@ -727,11 +729,16 @@ export class WorkflowNavigationUi { workflowName: string, workflowInstanceName: string | undefined, view: string, - viewLabel: string | undefined + viewLabel: string | undefined, + viewOf?: 'instance' ): RequestModelAction { const currentSourceUri = this.editorContext.sourceUri; + // The crumb keeps what the view is of, so that coming back to it by + // the trail asks for the same thing the node did. const trail = this.buildNavigationTrail(currentSourceUri, targetSourceUri, workflowName, workflowInstanceName) - .map((crumb, index, all) => (index === all.length - 1 ? { ...crumb, view, ...(viewLabel ? { viewLabel } : {}) } : crumb)); + .map((crumb, index, all) => (index === all.length - 1 + ? { ...crumb, view, ...(viewLabel ? { viewLabel } : {}), ...(viewOf ? { viewOf } : {}) } + : crumb)); this.noteNavigate(targetSourceUri, workflowName, trail); return this.requestModel(targetSourceUri, workflowName, undefined, trail); } @@ -1177,12 +1184,13 @@ export class WorkflowNavigationUi { : ''; const view = typeof (item as any)?.view === 'string' ? (item as any).view.trim() : ''; const viewLabel = typeof (item as any)?.viewLabel === 'string' ? (item as any).viewLabel.trim() : ''; + const viewOf = (item as any)?.viewOf === 'instance' ? { viewOf: 'instance' as const } : {}; if (sourceUri !== '' && workflowName !== '') { out.push({ sourceUri, workflowName, ...(workflowInstanceName !== '' ? { workflowInstanceName } : {}), - ...(view !== '' ? { view, ...(viewLabel !== '' ? { viewLabel } : {}) } : {}) + ...(view !== '' ? { view, ...(viewLabel !== '' ? { viewLabel } : {}), ...viewOf } : {}) }); } } @@ -1214,7 +1222,11 @@ export class WorkflowNavigationUi { ? { workflowInstanceName: entry.workflowInstanceName.trim() } : {}), ...(typeof entry.view === 'string' && entry.view.trim() !== '' - ? { view: entry.view.trim(), ...(entry.viewLabel?.trim() ? { viewLabel: entry.viewLabel.trim() } : {}) } + ? { + view: entry.view.trim(), + ...(entry.viewLabel?.trim() ? { viewLabel: entry.viewLabel.trim() } : {}), + ...(entry.viewOf === 'instance' ? { viewOf: 'instance' as const } : {}) + } : {}) }; const prev = normalized[normalized.length - 1]; diff --git a/packages/diagram-client/src/network-navigation-mouse-listener.ts b/packages/diagram-client/src/network-navigation-mouse-listener.ts index 254d601..706d378 100644 --- a/packages/diagram-client/src/network-navigation-mouse-listener.ts +++ b/packages/diagram-client/src/network-navigation-mouse-listener.ts @@ -109,7 +109,8 @@ export function openLeaf(target: GModelElement): Action[] { ...(typeof first.label === 'string' ? { label: first.label } : {}), uri, workflowName: displayNameFromQualifiedName(referencedName.trim()), - ...(typeof instanceName === 'string' && instanceName.trim() !== '' ? { workflowInstanceName: instanceName.trim() } : {}) + ...(typeof instanceName === 'string' && instanceName.trim() !== '' ? { workflowInstanceName: instanceName.trim() } : {}), + ...(first.of === 'instance' ? { viewOf: 'instance' } : {}) } as Action]; } if (clientBehavior().doubleClickOpensSource !== true) { diff --git a/packages/diagram-client/test/leaf-double-click-opens-source.test.ts b/packages/diagram-client/test/leaf-double-click-opens-source.test.ts index d4bfea4..9ca09ba 100644 --- a/packages/diagram-client/test/leaf-double-click-opens-source.test.ts +++ b/packages/diagram-client/test/leaf-double-click-opens-source.test.ts @@ -97,6 +97,22 @@ describe('double-click on a node that is not a network instance', () => { }]); }); + it('says so when the view is of the instance, and only then', () => { + behavior({}); + const node = (views: unknown) => leafNode({ + [WorkflowDiagramMetadata.REFERENCED_URI]: LEAF_URI, + [WorkflowDiagramMetadata.REFERENCED_ENTITY_NAME]: 'adder', + [WorkflowDiagramMetadata.VIEWS]: views, + 'wf:entityInstanceName': 'u_add' + }); + expect(doubleClick(node([{ id: 'inside', label: 'Inside', of: 'instance' }]))).toEqual([{ + kind: 'dialogram.openView', view: 'inside', label: 'Inside', uri: LEAF_URI, workflowName: 'adder', + workflowInstanceName: 'u_add', viewOf: 'instance' + }]); + expect(doubleClick(node([{ id: 'inside', of: 'definition' }]))[0].viewOf).toBeUndefined(); + expect(doubleClick(node([{ id: 'inside' }]))[0].viewOf).toBeUndefined(); + }); + it('opens a view without the source flag', () => { behavior({}); const node = leafNode({ diff --git a/packages/diagram-client/test/view-of-instance-trail.test.ts b/packages/diagram-client/test/view-of-instance-trail.test.ts new file mode 100644 index 0000000..aee8f2f --- /dev/null +++ b/packages/diagram-client/test/view-of-instance-trail.test.ts @@ -0,0 +1,93 @@ +/** + * A view of the instance it is reached through (`wf:views` entry with + * `of: 'instance'`), on the client's side of the trail. + * + * The view's crumb keeps the mark, and the trail the request carries has it, + * so the server asks for the view by the path of instances, now and whenever + * the trail leads back to the crumb. + */ +import 'reflect-metadata'; +import { afterEach, describe, expect, it } from 'vitest'; +import { WorkflowNavigationUi } from '../src/navigation-ui'; +import { WorkflowOpenViewActionHandler } from '../src/editing-action-handlers'; + +const ROOT = 'file:///w/decoder.py'; +const UNIT = 'file:///w/interpolation.py'; + +/** A navigation UI with no page behind it: what `requestView` needs and no more. */ +function navigation(): any { + const ui: any = Object.create(WorkflowNavigationUi.prototype); + ui.editorContext = { sourceUri: ROOT, diagramType: 'workflow-diagram' }; + ui.stacksBySourceUri = new Map(); + return ui; +} + +const trailOf = (action: any): any[] => JSON.parse(action.options['wf:navTrail']); + +afterEach(() => { + delete (globalThis as any).__calDiagramContext; +}); + +describe('a view of the instance it is reached through', () => { + it('is marked on its crumb, which the request carries', () => { + (globalThis as any).__calDiagramContext = { workflowName: 'decoder' }; + const action = navigation().requestView(UNIT, 'Interpolation', 'interpolation', 'actor', 'Actor', 'instance'); + + expect(action.options).toMatchObject({ sourceUri: UNIT, networkName: 'Interpolation', 'wf:view': 'actor' }); + expect(trailOf(action)).toEqual([ + { sourceUri: ROOT, workflowName: 'decoder' }, + { sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation', view: 'actor', viewLabel: 'Actor', viewOf: 'instance' } + ]); + }); + + it('ends a trail that leads to it through every instance on the way: the editor’s own stack', () => { + // The editor's document is the root; a nested unit was drilled into before. + const NESTED = 'file:///w/motion.py'; + const ui = navigation(); + ui.stacksBySourceUri.set(ROOT, [ + { sourceUri: ROOT, workflowName: 'decoder' }, + { sourceUri: NESTED, workflowName: 'motion', workflowInstanceName: 'motion_Y' } + ]); + const action = ui.requestView(UNIT, 'Interpolation', 'interpolation', 'actor', 'Actor', 'instance'); + + expect(trailOf(action)).toEqual([ + { sourceUri: ROOT, workflowName: 'decoder' }, + { sourceUri: NESTED, workflowName: 'motion', workflowInstanceName: 'motion_Y' }, + { sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation', view: 'actor', viewLabel: 'Actor', viewOf: 'instance' } + ]); + // And the stack the next request starts from is that trail, the mark with it. + expect(ui.stacksBySourceUri.get(UNIT).at(-1)).toMatchObject({ view: 'actor', viewOf: 'instance' }); + }); + + it('is not marked on the crumb of a view of the definition', () => { + (globalThis as any).__calDiagramContext = { workflowName: 'decoder' }; + const action = navigation().requestView(UNIT, 'Interpolation', 'interpolation', 'synthesis', undefined); + + expect(trailOf(action)[1]).toEqual({ sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation', view: 'synthesis' }); + }); + + it('is still marked when the trail comes back from the server', () => { + const ui = navigation(); + const sent = [ + { sourceUri: ROOT, workflowName: 'decoder' }, + { sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation', view: 'actor', viewOf: 'instance' }, + // A mark the platform does not know, and one on a crumb that is no view, are dropped. + { sourceUri: 'file:///w/other.py', workflowName: 'Other', workflowInstanceName: 'o', view: 'x', viewOf: 'something' }, + { sourceUri: 'file:///w/plain.py', workflowName: 'Plain', workflowInstanceName: 'p', viewOf: 'instance' } + ]; + expect(ui.parseTrail(JSON.stringify(sent)).map((crumb: any) => crumb.viewOf)).toEqual([undefined, 'instance', undefined, undefined]); + }); + + it('is passed on by the handler of the node’s action', () => { + const asked: unknown[][] = []; + const handler: any = new WorkflowOpenViewActionHandler(); + handler.actionDispatcher = { dispatch: async () => undefined }; + handler.navigationUi = { requestView: (...args: unknown[]) => { asked.push(args); return {}; } }; + const base = { kind: 'dialogram.openView', view: 'actor', label: 'Actor', uri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation' }; + handler.handle({ ...base, viewOf: 'instance' }); + handler.handle(base); + handler.handle({ ...base, viewOf: 'anything else' }); + + expect(asked.map(args => args[5])).toEqual(['instance', undefined, undefined]); + }); +}); diff --git a/packages/diagram-server/src/server/context-menu-item-provider.ts b/packages/diagram-server/src/server/context-menu-item-provider.ts index 6a4efbf..c187a74 100644 --- a/packages/diagram-server/src/server/context-menu-item-provider.ts +++ b/packages/diagram-server/src/server/context-menu-item-provider.ts @@ -407,7 +407,8 @@ export class WorkflowContextMenuItemProvider extends ContextMenuItemProvider { label: view.label, uri: referencedUri, workflowName: displayNameFromQualifiedName(referencedName.trim()), - ...(typeof instanceName === 'string' && instanceName.trim() !== '' ? { workflowInstanceName: instanceName.trim() } : {}) + ...(typeof instanceName === 'string' && instanceName.trim() !== '' ? { workflowInstanceName: instanceName.trim() } : {}), + ...(view.of === 'instance' ? { viewOf: 'instance' } : {}) } as any] })); } diff --git a/packages/diagram-server/src/server/graph-load-request-options.ts b/packages/diagram-server/src/server/graph-load-request-options.ts index f0d20c1..71ab493 100644 --- a/packages/diagram-server/src/server/graph-load-request-options.ts +++ b/packages/diagram-server/src/server/graph-load-request-options.ts @@ -32,12 +32,27 @@ export interface NavigationTrailEntry { /** The crumb is a view of the workflow other than its structure. */ view?: string; viewLabel?: string; + /** The view is of the instance the crumb was reached through (`UnitView.of`). */ + viewOf?: 'instance'; } /** One view a producer offers of a unit, as `meta.views` lists it. */ export interface UnitView { id: string; label: string; + /** + * `'instance'`: the view is of this instance, not of the definition its + * file holds under that name. For a unit made per instance (by a factory, + * with the instance's parameters) the two differ, and the file cannot + * answer for the instance: the view is asked of the root, by the path of + * instances that leads to it, as a drill-down into a nested unit is. + * + * It is said of a view a node offers (a view of the unit shown, offered + * on the canvas, is reached through no node), and it is acted on by a + * model source that can ask a root by a path of instances: one that + * cannot asks by name, as before. + */ + of?: 'instance'; } /** The view id on a request option or root arg, or nothing. */ @@ -45,7 +60,7 @@ export function parseViewArg(raw: unknown): string | undefined { return typeof raw === 'string' && raw.trim() !== '' ? raw.trim() : undefined; } -/** `meta.views` as the producer wrote it: `[{ id, label }]`, else nothing. */ +/** `meta.views` as the producer wrote it: `[{ id, label, of? }]`, else nothing. */ export function parseViewsMeta(raw: unknown): UnitView[] | undefined { if (!Array.isArray(raw)) { return undefined; @@ -57,7 +72,7 @@ export function parseViewsMeta(raw: unknown): UnitView[] | undefined { continue; } const label = typeof item?.label === 'string' && item.label.trim() !== '' ? item.label.trim() : id; - out.push({ id, label }); + out.push({ id, label, ...(item?.of === 'instance' ? { of: 'instance' as const } : {}) }); } return out.length > 0 ? out : undefined; } @@ -239,12 +254,13 @@ export function parseNavigationTrailArg(raw: unknown): NavigationTrailEntry[] { : ''; const view = typeof item?.view === 'string' ? item.view.trim() : ''; const viewLabel = typeof item?.viewLabel === 'string' ? item.viewLabel.trim() : ''; + const viewOf = item?.viewOf === 'instance' ? { viewOf: 'instance' as const } : {}; if (sourceUri !== '' && workflowName !== '') { out.push({ sourceUri: normalizeSourceUriKey(sourceUri), workflowName, ...(workflowInstanceName !== '' ? { workflowInstanceName } : {}), - ...(view !== '' ? { view, ...(viewLabel !== '' ? { viewLabel } : {}) } : {}) + ...(view !== '' ? { view, ...(viewLabel !== '' ? { viewLabel } : {}), ...viewOf } : {}) }); } } @@ -300,6 +316,50 @@ export function normalizeGraphLoadErrors(raw: unknown): NormalizedGraphLoadError .filter((entry): entry is NormalizedGraphLoadError => entry !== undefined); } +/** + * Whether the view a request asks for is of the instance it was reached + * through (the trail's last crumb says so, and it is that view): such a view + * is asked of the root first, by the instance path (`deriveTrailFallback`). + * + * Only when the trail is whole: every crumb under the root reached through + * an instance. As a last resort after a load that failed, a path with a step + * missing does no harm; asked first, it would lead to another instance, or + * to one of the same name higher up, and that answer would be shown. + */ +export function viewIsOfInstance(opts: Record): boolean { + const view = parseViewArg(opts[VIEW_ARG]); + const trail = parseNavigationTrailArg(opts[NAV_TRAIL_ARG]); + const last = trail[trail.length - 1]; + return view !== undefined && trail.length >= 2 && last.view === view && last.viewOf === 'instance' + && trail.slice(1).every(crumb => typeof crumb.workflowInstanceName === 'string' && crumb.workflowInstanceName.trim() !== ''); +} + +/** + * A request's options with the view its trail ends at, when the request + * names none and is for that crumb's unit. + * + * The client names the view of the trail's last crumb on every request it + * makes. The host does not: a refresh (a keystroke, a save, a run) repeats + * the file, the name and the trail, and would bring back the unit's + * structure under a breadcrumb that still says the view. The trail is what + * says where the reader is, so it settles what is shown. + */ +export function withTrailView(sourceUri: string, opts: Record): Record { + if (parseViewArg(opts[VIEW_ARG]) !== undefined) { + return opts; + } + const trail = parseNavigationTrailArg(opts[NAV_TRAIL_ARG]); + const last = trail[trail.length - 1]; + if (!last?.view || last.sourceUri !== normalizeSourceUriKey(sourceUri)) { + return opts; + } + const asked = typeof opts.networkName === 'string' ? opts.networkName.trim() : ''; + if (asked !== '' && asked !== last.workflowName) { + return opts; + } + return { ...opts, [VIEW_ARG]: last.view }; +} + /** * Derive a graph-source fallback (root workflow file + instance path) from the cross-file * breadcrumb trail when the caller didn't explicitly supply `cal:graphSourceUri`. diff --git a/packages/diagram-server/src/server/source-model-storage.ts b/packages/diagram-server/src/server/source-model-storage.ts index f46eb63..780da93 100644 --- a/packages/diagram-server/src/server/source-model-storage.ts +++ b/packages/diagram-server/src/server/source-model-storage.ts @@ -55,6 +55,7 @@ import { VIEW_ARG, parseViewArg, parseViewsMeta, + withTrailView, ROOT_WORKFLOW_ARG, INSTANCE_PATH_ARG, parseNavigationTrailArg as sharedParseNavigationTrailArg, @@ -348,7 +349,8 @@ export class WorkflowSourceModelStorage implements SourceModelStorage { } private async loadSourceModelDocument(action: RequestModelAction, sourceUri: string): Promise { - const opts = action.options ?? {}; + // A refresh by the host names no view: the trail's last crumb does. + const opts = withTrailView(sourceUri, action.options ?? {}); const workflowFilePath = URI.parse(sourceUri).fsPath; const perf = new GraphLoadPerf(path.basename(workflowFilePath)); // Acquisition (graph-acquisition strategy, model-source attempt + breadcrumb retry, CLI spawn, diff --git a/packages/diagram-server/test/open-view.test.ts b/packages/diagram-server/test/open-view.test.ts index bcec141..2202625 100644 --- a/packages/diagram-server/test/open-view.test.ts +++ b/packages/diagram-server/test/open-view.test.ts @@ -10,7 +10,8 @@ import { describe, expect, it } from 'vitest'; import { WorkflowDiagramMetadata, WorkflowDiagramTypes } from '@dialogram/shared'; import { WorkflowContextMenuItemProvider } from '../src/server/context-menu-item-provider'; import { GraphGModelSource } from '../src/model/graph-gmodel-source'; -import { parseViewsMeta } from '../src/server/graph-load-request-options'; +import { parseNavigationTrailArg, parseViewsMeta, viewIsOfInstance, withTrailView } from '../src/server/graph-load-request-options'; +import { GraphGModelSource } from '../src/model/graph-gmodel-source'; const SHOWN = 'file:///w/top.vhd'; const VIEWS = [{ id: 'synth', label: 'Synthesis View' }]; @@ -47,6 +48,15 @@ describe('a view of a node’s unit', () => { }]); }); + it('says so when the view is of the instance, not of the definition its file holds', () => { + const ofInstance = { ...node, args: { ...node.args, [WorkflowDiagramMetadata.VIEWS]: [{ id: 'inside', label: 'Inside', of: 'instance' }] } }; + const item = provider(root, ofInstance).getItems(['n'], { x: 0, y: 0 }).find((i: any) => i.id === 'dialogram.openView.inside'); + expect(item.actions).toEqual([{ + kind: 'dialogram.openView', view: 'inside', label: 'Inside', + uri: 'file:///w/adder.vhd', workflowName: 'adder', workflowInstanceName: 'u_add', viewOf: 'instance' + }]); + }); + it('is not offered on a node whose unit has none', () => { const plain = { ...node, args: { ...node.args, [WorkflowDiagramMetadata.VIEWS]: undefined } }; const ids = provider(root, plain).getItems(['n'], { x: 0, y: 0 }).map((i: any) => i.id); @@ -69,6 +79,9 @@ describe('a view of the unit shown', () => { describe('the document’s meta.views', () => { it('reaches the node as wf:views, labels defaulting to the id', () => { expect(parseViewsMeta([{ id: 'synth' }, { id: '' }, 'x'])).toEqual([{ id: 'synth', label: 'synth' }]); + // What a view is of: the instance when the producer says so, else nothing said. + expect(parseViewsMeta([{ id: 'a', of: 'instance' }, { id: 'b', of: 'definition' }, { id: 'c', of: true }])) + .toEqual([{ id: 'a', label: 'a', of: 'instance' }, { id: 'b', label: 'b' }, { id: 'c', label: 'c' }]); expect(parseViewsMeta([])).toBeUndefined(); const doc: any = { version: '1.1', @@ -84,3 +97,78 @@ describe('the document’s meta.views', () => { expect(find(model.graph ?? model)?.args[WorkflowDiagramMetadata.VIEWS]).toEqual(VIEWS); }); }); + +describe('a view of the instance, on the trail', () => { + const trail = (last: Record) => JSON.stringify([ + { sourceUri: 'file:///w/top.py', workflowName: 'top' }, + { sourceUri: 'file:///w/unit.py', workflowName: 'Unit', workflowInstanceName: 'u1', view: 'inside', ...last } + ]); + + it('is kept on its crumb, and on no crumb that is not a view', () => { + expect(parseNavigationTrailArg(trail({ viewOf: 'instance' }))[1]).toEqual({ + sourceUri: 'file:///w/unit.py', workflowName: 'Unit', workflowInstanceName: 'u1', view: 'inside', viewOf: 'instance' + }); + expect(parseNavigationTrailArg(trail({ viewOf: 'definition' }))[1].viewOf).toBeUndefined(); + expect(parseNavigationTrailArg(trail({ view: '', viewOf: 'instance' }))[1].viewOf).toBeUndefined(); + }); + + it('is what the request asks for when the last crumb is that view, reached through an instance', () => { + const opts = (last: Record, view: unknown = 'inside') => ({ 'wf:view': view, 'wf:navTrail': trail(last) }); + expect(viewIsOfInstance(opts({ viewOf: 'instance' }))).toBe(true); + expect(viewIsOfInstance(opts({}))).toBe(false); + expect(viewIsOfInstance(opts({ viewOf: 'instance' }, 'synthesis'))).toBe(false); + // No view asked for at all (an explicit `undefined` would take the default above). + expect(viewIsOfInstance(opts({ viewOf: 'instance' }, null))).toBe(false); + expect(viewIsOfInstance(opts({ viewOf: 'instance', workflowInstanceName: ' ' }))).toBe(false); + expect(viewIsOfInstance({})).toBe(false); + }); + + it('needs a whole trail: every crumb under the root reached through an instance', () => { + const through = (middle: Record) => ({ + 'wf:view': 'inside', + 'wf:navTrail': JSON.stringify([ + { sourceUri: 'file:///w/top.py', workflowName: 'top' }, + { sourceUri: 'file:///w/mid.py', workflowName: 'mid', ...middle }, + { sourceUri: 'file:///w/unit.py', workflowName: 'Unit', workflowInstanceName: 'u1', view: 'inside', viewOf: 'instance' } + ]) + }); + expect(viewIsOfInstance(through({ workflowInstanceName: 'm1' }))).toBe(true); + // The middle unit was shown by name: the path would skip it. + expect(viewIsOfInstance(through({}))).toBe(false); + // A crumb alone has no root to be asked of. + expect(viewIsOfInstance({ + 'wf:view': 'inside', + 'wf:navTrail': JSON.stringify([{ sourceUri: 'file:///w/unit.py', workflowName: 'Unit', workflowInstanceName: 'u1', view: 'inside', viewOf: 'instance' }]) + })).toBe(false); + }); + + it('settles the view of a request that names none, when the request is for the last crumb’s unit', () => { + const refresh = { networkName: 'Unit', 'wf:navTrail': trail({ viewOf: 'instance' }) }; + expect(withTrailView('file:///w/unit.py', refresh)).toEqual({ ...refresh, 'wf:view': 'inside' }); + // No name at all is the crumb's unit too. + expect(withTrailView('file:///w/unit.py', { 'wf:navTrail': trail({}) })['wf:view']).toBe('inside'); + // A view the request names is the request's; another file, another name, or a crumb that is no view, leave it be. + const named = { ...refresh, 'wf:view': 'synthesis' }; + expect(withTrailView('file:///w/unit.py', named)).toBe(named); + expect(withTrailView('file:///w/other.py', refresh)).toBe(refresh); + expect(withTrailView('file:///w/unit.py', { ...refresh, networkName: 'Other' })['wf:view']).toBeUndefined(); + expect(withTrailView('file:///w/unit.py', { networkName: 'Unit', 'wf:navTrail': trail({ view: '' }) })['wf:view']).toBeUndefined(); + expect(withTrailView('file:///w/unit.py', {})).toEqual({}); + }); + + it('reaches a node’s arguments from the document, with what it is of', () => { + const graph: any = new GraphGModelSource().transform({ + version: '1', + graph: { + id: 'root', + nodes: [{ + id: 'node:u1', kind: 'actor', label: 'u1', scope: 'scope:root', ports: [], + meta: { referencedEntityName: 'Unit', views: [{ id: 'inside', label: 'Inside', of: 'instance' }, { id: 'synth' }] } + }], + edges: [] + } + }).graph; + expect(graph.children.find((child: any) => child.id === 'u1').args[WorkflowDiagramMetadata.VIEWS]) + .toEqual([{ id: 'inside', label: 'Inside', of: 'instance' }, { id: 'synth', label: 'synth' }]); + }); +}); diff --git a/packages/extension-core/src/extension/diagram/glsp-activation.ts b/packages/extension-core/src/extension/diagram/glsp-activation.ts index 8324716..8de51c1 100644 --- a/packages/extension-core/src/extension/diagram/glsp-activation.ts +++ b/packages/extension-core/src/extension/diagram/glsp-activation.ts @@ -114,6 +114,10 @@ type NavigationTrailEntry = { sourceUri?: string; workflowName?: string; workflowInstanceName?: string; + /** A crumb that is a view of the unit: kept, so a trail rewritten here still says it. */ + view?: string; + viewLabel?: string; + viewOf?: 'instance'; }; @@ -140,11 +144,20 @@ function parseNavigationTrail(raw: unknown): NavigationTrailEntry[] { const workflowInstanceName = typeof (entry as NavigationTrailEntry & { workflowInstanceName?: string }).workflowInstanceName === 'string' ? (entry as NavigationTrailEntry & { workflowInstanceName?: string }).workflowInstanceName : undefined; + const view = typeof (entry as NavigationTrailEntry).view === 'string' ? (entry as NavigationTrailEntry).view : undefined; + const viewLabel = typeof (entry as NavigationTrailEntry).viewLabel === 'string' ? (entry as NavigationTrailEntry).viewLabel : undefined; if (sourceUri || workflowName) { out.push({ sourceUri, workflowName, - ...(workflowInstanceName ? { workflowInstanceName } : {}) + ...(workflowInstanceName ? { workflowInstanceName } : {}), + ...(view + ? { + view, + ...(viewLabel ? { viewLabel } : {}), + ...((entry as NavigationTrailEntry).viewOf === 'instance' ? { viewOf: 'instance' as const } : {}) + } + : {}) }); } } diff --git a/packages/shared/src/diagram-types.ts b/packages/shared/src/diagram-types.ts index 6aa02f4..1a5c044 100644 --- a/packages/shared/src/diagram-types.ts +++ b/packages/shared/src/diagram-types.ts @@ -141,8 +141,9 @@ export namespace WorkflowDiagramMetadata { export const IS_READ_ONLY = 'cal:readOnly'; export const IS_NETWORK_INSTANCE = 'cal:isNetworkInstance'; /** The views the producer offers of the unit behind a node besides its - * structure, `[{ id, label }]` as it listed them on `meta.views` (a leaf's - * synthesis, say). On the root: the views of the unit shown. */ + * structure, `[{ id, label, of? }]` as it listed them on `meta.views` (a + * leaf's synthesis, say; `of: 'instance'` for a view of this instance and + * not of the definition). On the root: the views of the unit shown. */ export const VIEWS = 'wf:views'; /** * Labelled values a producer states about the unit behind a node, as it diff --git a/packages/sidecar-toolkit/src/server/sidecar-model-source.ts b/packages/sidecar-toolkit/src/server/sidecar-model-source.ts index 8be2964..e68e0c5 100644 --- a/packages/sidecar-toolkit/src/server/sidecar-model-source.ts +++ b/packages/sidecar-toolkit/src/server/sidecar-model-source.ts @@ -27,6 +27,7 @@ import { normalizeGraphLoadErrors, parseStringListArg, shouldRetryTrailFallback, + viewIsOfInstance, VIEW_ARG, parseViewArg } from '@dialogram/diagram-server/server/graph-load-request-options'; @@ -123,17 +124,58 @@ export class SidecarModelSource implements DiagramModelSource { : sourceUri; const graphFilePath = URI.parse(graphSourceUri).fsPath; - let sidecarResult: { graph?: Record; error?: string; warning?: string } = - await this.tryLoadGraphFromSidecar(graphFilePath, opts); - if (!sidecarResult.graph || shouldRetryTrailFallback(sidecarResult, opts)) { + // A view of the instance it was reached through is asked of the root, + // by the path of instances: by name, the file answers for the + // definition it holds under that name, which for a unit made per + // instance is another one. Only when the root has no answer is the + // file asked, as for any view. + /** The request that asks the root, by the path of instances, for what `opts` asks a file by name. */ + const ofRoot = (via: { graphSourceUri: string; rootWorkflowName: string; instancePath: string[] }): Record => ({ + ...opts, + [GRAPH_SOURCE_URI_ARG]: via.graphSourceUri, + [ROOT_WORKFLOW_ARG]: via.rootWorkflowName, + [INSTANCE_PATH_ARG]: via.instancePath + }); + const viaInstance = viewIsOfInstance(opts) ? deriveTrailFallback(sourceUri, opts) : undefined; + let sidecarResult: { graph?: Record; error?: string; warning?: string } | undefined; + if (viaInstance) { + const instanceOpts = ofRoot(viaInstance); + const answered = await this.tryLoadGraphFromSidecar(URI.parse(viaInstance.graphSourceUri).fsPath, instanceOpts); + if (answered.graph) { + sidecarResult = answered; + resolvedOpts = instanceOpts; + } else { + // The file is asked next. Whatever it says, the reader is + // told that the root had no answer, and why: what the file + // shows is the definition's, not this instance's. + const whyNot = answered.error ?? 'the root gave no graph'; + const byName = await this.tryLoadGraphFromSidecar(graphFilePath, opts); + sidecarResult = byName.graph + ? { + ...byName, + graph: { + ...byName.graph, + errors: [ + ...normalizeGraphLoadErrors(byName.graph.errors), + { + message: `This is the view of the definition in ${workflowFilePath}, not of the instance ` + + `${viaInstance.instancePath.join(' / ')}: ${whyNot}`, + severity: 'warning', + code: 'view_of_definition', + file: workflowFilePath + } + ] + } + } + : { ...byName, error: `${byName.error ?? 'no graph'} (asked of the instance first: ${whyNot})` }; + } + } + sidecarResult ??= await this.tryLoadGraphFromSidecar(graphFilePath, opts); + // The same question is not asked twice: the path of instances was the first. + if (!viaInstance && (!sidecarResult.graph || shouldRetryTrailFallback(sidecarResult, opts))) { const retry = deriveTrailFallback(sourceUri, opts); if (retry) { - const retryOpts = { - ...opts, - [GRAPH_SOURCE_URI_ARG]: retry.graphSourceUri, - [ROOT_WORKFLOW_ARG]: retry.rootWorkflowName, - [INSTANCE_PATH_ARG]: retry.instancePath - }; + const retryOpts = ofRoot(retry); const retriedResult = await this.tryLoadGraphFromSidecar(URI.parse(retry.graphSourceUri).fsPath, retryOpts); // Preserve the original partial graph if retry fails to produce a graph. if (retriedResult.graph || !sidecarResult.graph) { diff --git a/packages/sidecar-toolkit/test/view-of-instance.test.ts b/packages/sidecar-toolkit/test/view-of-instance.test.ts new file mode 100644 index 0000000..32da8f1 --- /dev/null +++ b/packages/sidecar-toolkit/test/view-of-instance.test.ts @@ -0,0 +1,270 @@ +/** + * A view of the instance it was reached through (`meta.views` entry with + * `of: 'instance'`, carried on the trail's last crumb as `viewOf`). + * + * By name, a file answers for the definition it holds under that name. For a + * unit made per instance (by a factory, with the instance's parameters) that + * is another one. So such a view is asked of the root first, by the path of + * instances the trail records, exactly as a drill-down into a nested unit + * falls back to; the file is asked by name only when the root has no answer, + * and the same question is never asked twice. When the file answers in the + * root's place the reader is told so, since what is shown is the + * definition's. A view without the mark is asked as before. + * + * The host's refresh (a keystroke, a save) repeats the trail and names no + * view: the storage takes the view from the trail's last crumb, so the view + * stays, and stays the instance's. + */ +import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { Module } from 'node:module'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import * as vscodeMock from './vscode-mock'; +import { WorkflowSourceModelStorage } from '@dialogram/diagram-server/server/diagram-glsp-module'; +import { SidecarModelSource } from '../src/server/sidecar-model-source'; +import { type SidecarRuntimeConfig } from '../src/server/sidecar-runtime-config'; +import { NEUTRAL_CREATE_NODE_CONFIG } from './fixtures/create-node-config'; + +const CFG: SidecarRuntimeConfig = { + settingsNamespace: 'calLang', + sidecarOperationPrefix: 'calpy', + sidecarCommandSettingKey: 'calpySidecarCommand', + sidecarCommandDefault: 'calpy-sidecar', + cliCommandSettingKey: 'calpyCommand', + cliCommandDefault: 'calpy', + cliPythonModule: undefined, + operationKinds: { createEntityPort: 'op.createEntityPort', deleteEntityPort: 'op.deleteEntityPort' }, + ...NEUTRAL_CREATE_NODE_CONFIG, + acceptedOperationPrefixes: ['calpy'], + graphAcquisition: 'sidecar-export' +}; + +const ROOT = 'file:///tmp/decoder.py'; +const NESTED = 'file:///tmp/motion.py'; +const UNIT = 'file:///tmp/interpolation.py'; + +/** Root, a nested unit reached through `motion_Y`, then the view of the unit reached through `interpolation`. */ +function request(last: Record = {}, view = 'actor'): Record { + return { + networkName: 'Interpolation', + 'wf:view': view, + 'wf:navTrail': JSON.stringify([ + { sourceUri: ROOT, workflowName: 'decoder' }, + { sourceUri: NESTED, workflowName: 'motion', workflowInstanceName: 'motion_Y' }, + { sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation', view: 'actor', viewLabel: 'Actor', ...last } + ]) + }; +} + +const GRAPH = { network: 'Interpolation', nodes: [], edges: [], scopes: [] }; + +/** A model source whose sidecar answers by `answer(call number)`, and what it was asked. */ +function source(answer: (call: number) => { graph?: Record; error?: string }) { + const asked: Array<{ file: string; root?: unknown; path?: unknown; graphSource?: unknown; network?: unknown; view?: unknown }> = []; + const modelSource = new SidecarModelSource(CFG) as any; + modelSource.tryLoadGraphFromSidecar = async (file: string, opts: Record) => { + asked.push({ + file, root: opts['cal:rootWorkflow'], path: opts['cal:instancePath'], graphSource: opts['cal:graphSourceUri'], + network: opts.networkName, view: opts['wf:view'] + }); + return answer(asked.length); + }; + return { modelSource: modelSource as SidecarModelSource, asked }; +} + +// What `tryLoadGraphFromSidecar` is handed. `network` is the request's own +// name still: with a root and a path, the sidecar is not sent it (see "on the wire"). +const BY_INSTANCE = { file: '/tmp/decoder.py', root: 'decoder', path: ['motion_Y', 'interpolation'], graphSource: ROOT, network: 'Interpolation', view: 'actor' }; +const BY_NAME = { file: '/tmp/interpolation.py', root: undefined, path: undefined, graphSource: undefined, network: 'Interpolation', view: 'actor' }; + +describe('a view of the instance it was reached through', () => { + it('is asked of the root, by the path of instances, and that answer is the one shown', async () => { + const { modelSource, asked } = source(() => ({ graph: GRAPH })); + const result: any = await modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }) }); + + expect(asked).toEqual([BY_INSTANCE]); + expect(result.graph.id).toBeDefined(); + // What it was resolved with goes on with the document: its layout is the instance's. + expect(result.resolvedOptions).toMatchObject({ + 'cal:graphSourceUri': ROOT, 'cal:rootWorkflow': 'decoder', 'cal:instancePath': ['motion_Y', 'interpolation'], 'wf:view': 'actor' + }); + }); + + it('is asked of its file by name when the root has no answer, and the root is not asked again', async () => { + const { modelSource, asked } = source(call => (call === 1 ? { error: 'the root could not be elaborated' } : { graph: GRAPH })); + const result: any = await modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }) }); + + expect(asked).toEqual([BY_INSTANCE, BY_NAME]); + expect(result.graph.id).toBeDefined(); + expect(result.resolvedOptions['cal:rootWorkflow']).toBeUndefined(); + }); + + it('asks no third time when neither has an answer, and says what each said', async () => { + const { modelSource, asked } = source(call => ({ error: call === 1 ? 'instance path not found' : 'no definition under that name' })); + (modelSource as any).showGraphExportError = async () => undefined; + const result: any = await modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }) }); + + expect(asked).toEqual([BY_INSTANCE, BY_NAME]); + const messages = (result.diagnostics as Array<{ message: string }>).map(d => d.message).join(' | '); + expect(messages).toContain('no definition under that name'); + expect(messages).toContain('asked of the instance first: instance path not found'); + }); + + it('says so when its file answers in the root’s place: what is shown is the definition’s', async () => { + const { modelSource } = source(call => (call === 1 ? { error: 'the root could not be elaborated' } : { graph: GRAPH })); + const result: any = await modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }) }); + + expect(result.diagnostics).toEqual([expect.objectContaining({ + severity: 'warning', + code: 'view_of_definition', + uri: '/tmp/interpolation.py', + message: 'This is the view of the definition in /tmp/interpolation.py, not of the instance motion_Y / interpolation: ' + + 'the root could not be elaborated' + })]); + // And nothing of the kind when the root answered. + const answered: any = await source(() => ({ graph: GRAPH })).modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }) }); + expect(answered.diagnostics).toEqual([]); + }); + + it('is asked of the root for an actor of the root itself: a trail of two', async () => { + const { modelSource, asked } = source(() => ({ graph: GRAPH })); + await modelSource.getGraph(UNIT, { + requestOptions: { + networkName: 'Interpolation', + 'wf:view': 'actor', + 'wf:navTrail': JSON.stringify([ + { sourceUri: ROOT, workflowName: 'decoder' }, + { sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'merger', view: 'actor', viewOf: 'instance' } + ]) + } + }); + + expect(asked).toEqual([{ ...BY_INSTANCE, path: ['merger'] }]); + }); + + it('is asked by name when the trail has a step with no instance: the path would lead elsewhere', async () => { + const { modelSource, asked } = source(() => ({ graph: GRAPH })); + await modelSource.getGraph(UNIT, { + requestOptions: { + networkName: 'Interpolation', + 'wf:view': 'actor', + 'wf:navTrail': JSON.stringify([ + { sourceUri: ROOT, workflowName: 'decoder' }, + // Shown by name, not reached through an instance. + { sourceUri: NESTED, workflowName: 'motion' }, + { sourceUri: UNIT, workflowName: 'Interpolation', workflowInstanceName: 'interpolation', view: 'actor', viewOf: 'instance' } + ]) + } + }); + + expect(asked).toEqual([BY_NAME]); + }); +}); + +describe('the view after a refresh by the host', () => { + /** The storage with the model source behind it, its drawing stubbed out: what is asked is what is checked. */ + function storageOver(modelSource: SidecarModelSource) { + const storage = new WorkflowSourceModelStorage() as any; + storage.injectedModelSource = modelSource; + storage.storageOptions = { settingsNamespace: 'calLang', operationPrefix: 'calpy' }; + let finished: Record | undefined; + storage.finishLoadFromDoc = async (_doc: unknown, _sourceUri: string, _file: string, opts: Record) => { + finished = opts; + }; + storage.getWorkflowRelationshipInfo = async () => ({ workflowNames: [] }); + return { storage, finishedWith: () => finished }; + } + + it('is still the view, and still the instance’s: the trail’s last crumb says both', async () => { + const { modelSource, asked } = source(() => ({ graph: GRAPH })); + const { storage, finishedWith } = storageOver(modelSource); + // As the host refreshes: the file, the name, the trail, and no view. + const { 'wf:view': _view, ...refresh } = request({ viewOf: 'instance' }); + await storage.loadSourceModelDocument({ options: { ...refresh, sourceUri: UNIT, forceReloadFromDisk: true } }, UNIT); + + expect(asked).toEqual([BY_INSTANCE]); + expect(finishedWith()).toMatchObject({ 'wf:view': 'actor', 'cal:rootWorkflow': 'decoder', 'cal:instancePath': ['motion_Y', 'interpolation'] }); + }); + + it('is the structure when the trail ends at no view, or at the view of another unit', async () => { + const plain = source(() => ({ graph: GRAPH })); + const { 'wf:view': _view, ...noView } = request({ view: '' }); + await storageOver(plain.modelSource).storage.loadSourceModelDocument({ options: { ...noView, sourceUri: UNIT } }, UNIT); + expect(plain.asked.map(call => call.view)).toEqual([undefined]); + + // The request is for another name than the crumb's: the crumb does not speak for it. + const other = source(() => ({ graph: GRAPH })); + const { 'wf:view': _v, ...refresh } = request({ viewOf: 'instance' }); + await storageOver(other.modelSource).storage.loadSourceModelDocument( + { options: { ...refresh, sourceUri: UNIT, networkName: 'Another' } }, UNIT); + expect(other.asked.map(call => call.view)).toEqual([undefined]); + }); +}); + +describe('on the wire', () => { + // The real `tryLoadGraphFromSidecar` reaches VS Code by `require`, which + // the test runner's alias does not cover: answer it with the mock for + // the length of this suite (as characterization-graph-load.test.ts does). + let originalLoad: (request: string, parent: unknown, isMain: boolean) => unknown; + const cleanup: string[] = []; + beforeAll(() => { + originalLoad = (Module as any)._load; + (Module as any)._load = function patched(this: unknown, wanted: string, parent: unknown, isMain: boolean) { + return wanted === 'vscode' ? vscodeMock : originalLoad.call(this, wanted, parent, isMain); + }; + }); + afterAll(() => { + (Module as any)._load = originalLoad; + }); + afterEach(() => { + delete process.env.FAKE_SIDECAR_LOG; + for (const entry of cleanup.splice(0)) { + fs.rmSync(entry, { recursive: true, force: true }); + } + }); + + it('the sidecar is sent the root’s file, the root, the path and the view, and no name', async () => { + const log = path.join(os.tmpdir(), `view-of-instance-${process.pid}-${Math.random().toString(36).slice(2)}.log`); + cleanup.push(log); + process.env.FAKE_SIDECAR_LOG = log; + const modelSource = new SidecarModelSource({ ...CFG, sidecarCommandDefault: path.join(__dirname, 'fixtures', 'fake-sidecar.cjs') }); + await modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }) }); + + const sent = fs.readFileSync(log, 'utf8').trim().split('\n').map(line => JSON.parse(line)); + expect(sent).toHaveLength(1); + expect(sent[0].file).toBe('/tmp/decoder.py'); + expect(sent[0].args).toEqual({ rootNetwork: 'decoder', instancePath: ['motion_Y', 'interpolation'], view: 'actor' }); + }); +}); + +describe('a view without the mark', () => { + it('is asked of its file by name, as before', async () => { + const { modelSource, asked } = source(() => ({ graph: GRAPH })); + await modelSource.getGraph(UNIT, { requestOptions: request() }); + + expect(asked).toEqual([BY_NAME]); + }); + + it('still falls back to the root when its file has no answer', async () => { + const { modelSource, asked } = source(call => (call === 1 ? { error: 'not found' } : { graph: GRAPH })); + await modelSource.getGraph(UNIT, { requestOptions: request() }); + + expect(asked).toEqual([BY_NAME, BY_INSTANCE]); + }); + + it('is what a mark on another view, or on a crumb reached through no instance, comes down to', async () => { + // The request is for another view than the crumb's. + const other = source(() => ({ graph: GRAPH })); + await other.modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance' }, 'synthesis') }); + expect(other.asked.map(call => call.file)).toEqual(['/tmp/interpolation.py']); + // No instance on the last crumb: there is no path to ask by. + const direct = source(() => ({ graph: GRAPH })); + await direct.modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'instance', workflowInstanceName: '' }) }); + expect(direct.asked.map(call => call.file)).toEqual(['/tmp/interpolation.py']); + // A mark that is not the one the platform knows. + const odd = source(() => ({ graph: GRAPH })); + await odd.modelSource.getGraph(UNIT, { requestOptions: request({ viewOf: 'definition' }) }); + expect(odd.asked.map(call => call.file)).toEqual(['/tmp/interpolation.py']); + }); +});