Skip to content

Commit

Permalink
Bugfix: Uncaught error causing rendering error when referencing an ar…
Browse files Browse the repository at this point in the history
…tifact on a non-existent route.

PiperOrigin-RevId: 579796465
  • Loading branch information
Googler authored and copybara-github committed Nov 7, 2023
1 parent 0a47c6a commit 69c55b2
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 10 deletions.
142 changes: 142 additions & 0 deletions src/app/demo_page/demo_datasets/artifact_in_nested_loop.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/**
* @license
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* @fileoverview This file contains all the fake data json payloads for the
* various testing scenarios
*/

import {DagNodeSkeleton, DagSkeleton, repeatedMetaNodes, StateTable} from '../../node_spec';


const entrypointMeta = {
state: 'NO_STATE_RUNTIME',
subType: 'model',
icon: {
name: 'artifact-model',
iconset: 'cloud_ai',
size: 'large',
},
};

const iterationMeta = {
state: 'TIMEOUT',
hasControlNode: true,
treatAsLoop: true,
icon: {
name: 'group-iterative',
iconset: 'cloud_ai',
size: 'medium',
},
};

const artifactRefsMeta = {
state: 'NO_STATE_RUNTIME',
artifactRefs: [
{id: 'Entry point', path: []},
{id: 'Entry point', path: ['Iteration', 'it-1']},
{id: 'Not-existing node', path: ['Not-existing path']},
],
};

/**
* A graph dataset containing artifact referencing to nodes inside a multi level
* iteration. Contains 3 references: to the entry node, to the entry node under
* the level 1 iteration's first run and to a not-existent node (which should
* not appear). Can be used by `DagNode.createFromSkeleton()` to generate a
* GraphSpec.
*/
export const fakeGraph: DagSkeleton = {
state: {
'Entry point': entrypointMeta,
'Iteration': {
...iterationMeta,
description: 'Level 1',
selectedLoopId: 'it-1',
groupMeta: repeatedMetaNodes(
['it-1', 'it-2'],
i => ({
state: 'SUCCEEDED',
displayName: `Iteration ${i.slice(3)}`,
groupMeta: {
'Entry point': entrypointMeta,
'Iteration': {
...iterationMeta,
description: 'Level 2',
selectedLoopId: 'it-2',
groupMeta: repeatedMetaNodes(
['it-1', 'it-2'], i => ({
state: 'SUCCEEDED',
displayName: `Iteration ${i.slice(3)}`,
groupMeta: {
'Artifact refs': artifactRefsMeta,
},
}))
},
},
})),
},
} as StateTable,
skeleton: [
{
id: 'Entry point',
type: 'artifact',
next: [
{
id: 'Iteration',
type: 'group',
definition: Array.from(
{length: 2}, (e, i) => ({
id: `it-${i + 1}`,
type: 'group',
definition: [
{
id: 'Entry point',
type: 'artifact',
next: [
{
id: 'Iteration',
type: 'group',
definition: Array.from(
{length: 2}, (e, i) => ({
id: `it-${i + 1}`,
type: 'group',
definition: [
{
id: 'Artifact refs',
type: 'execution',
},
]
}))
},
{
id: 'Execution',
type: 'execution',
}
]
},
]
}))
},
{
id: 'Execution',
type: 'execution',
}
]
} as DagNodeSkeleton,
],
};
9 changes: 7 additions & 2 deletions src/app/demo_page/demo_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {fakeGraph as exampleDagDemo} from '../test_resources/fake_data';
import {DagTheme, DagToolbarModule} from '../toolbar';
import {UserConfig} from '../user_config.service';

import {fakeGraph as artifactInNestedLoopDemo} from './demo_datasets/artifact_in_nested_loop';
import {fakeGraph as recursiveGraphDemo} from './demo_datasets/recursive_graph';
import {DagDatasetSettings} from './demo_datasets/shared';
import {fakeGraph as singleNodeDemo} from './demo_datasets/single_node';
Expand All @@ -47,13 +48,17 @@ interface Options<T> {

const LOCAL_STORAGE_KEY = 'workflow_graph_user_config';

const DEFAULT_DATASET = 'Default Dataset';

const datasets: Options<GraphSpec> = {
'Default Dataset':
DagNode.createFromSkeleton(exampleDagDemo.skeleton, exampleDagDemo.state),
'Single Node':
DagNode.createFromSkeleton(singleNodeDemo.skeleton, singleNodeDemo.state),
'Recursive Graph': DagNode.createFromSkeleton(
recursiveGraphDemo.skeleton, recursiveGraphDemo.state),
'Artifact in nested loop': DagNode.createFromSkeleton(
artifactInNestedLoopDemo.skeleton, artifactInNestedLoopDemo.state),
};

const datasetOptions: Options<DagDatasetSettings> = {
Expand Down Expand Up @@ -225,7 +230,7 @@ export class DagDemoPage {
destroy = new Subject<void>();

constructor(public dialog: MatDialog) {
this.setCurrDataset('Default Dataset', true);
this.setCurrDataset(DEFAULT_DATASET, true);
this.resetAll();
}

Expand Down Expand Up @@ -457,7 +462,7 @@ export class DagDemoPage {
if (!this.dagRef || !this.currDataset) return;
let group: GraphSpec = this.currDataset;
if (parent) {
group = DagGroup.navigatePath(group, parent.path);
group = DagGroup.navigatePath(group, parent.path)!;
}
const nodeIds = new Set(group.nodes.map(node => node.id));
let newId = '';
Expand Down
16 changes: 10 additions & 6 deletions src/app/directed_acyclic_graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -559,11 +559,15 @@ export class DirectedAcyclicGraph implements AfterViewInit, OnInit, OnDestroy {
}

/** Resolved a `NodeRef` to its actual node (be it a group or a node) */
resolveReference(ref: NodeRef): DagNode|DagGroup {
const destLoc = DagGroup.navigatePath(this as GraphSpec, ref.path);
const {nodes, edges, groups} = destLoc;
const nodeMap = DagNode.createNodeMap(nodes, edges, groups);
return nodeMap.nodes[ref.id]?.node || nodeMap.groups[ref.id]?.group;
resolveReference(ref: NodeRef): DagNode|DagGroup|undefined {
try {
const destLoc = DagGroup.navigatePath(this as GraphSpec, ref.path);
const {nodes, edges, groups} = destLoc;
const nodeMap = DagNode.createNodeMap(nodes, edges, groups);
return nodeMap.nodes[ref.id]?.node || nodeMap.groups[ref.id]?.group;
} catch {
return;
}
}

async sleep(time: number) {
Expand Down Expand Up @@ -647,7 +651,7 @@ export class DirectedAcyclicGraph implements AfterViewInit, OnInit, OnDestroy {
* offset_
*/
getNodeOffset(ref: NodeRef) {
const node = this.resolveReference(ref);
const node = this.resolveReference(ref)!;
const offset: Point = {x: 0, y: 0};
let groups = this.groups;
for (const segment of ref.path) {
Expand Down
2 changes: 1 addition & 1 deletion src/app/directed_acyclic_graph_raw.ng.html
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@
[style.height.px]="(group.height! - group.padY!)"
>
<header
*ngIf="group.hasControlNode"
*ngIf="group.hasControlNode && getControlNodeFor(group)"
class="control-node-scaffold fade-in"
[style.width.px]="getControlNodeFor(group).width"
[style.height.px]="getControlNodeFor(group).height"
Expand Down
2 changes: 1 addition & 1 deletion src/app/directed_acyclic_graph_raw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ export class DagRaw implements DoCheck, OnInit, OnDestroy {
@Input() noEmptySpaceAlloc = false;
@Output() groupIterationChanged = new EventEmitter<GroupIterationRecord>();

@Input() resolveReference?: (ref: NodeRef) => DagNode | DagGroup;
@Input() resolveReference?: (ref: NodeRef) => DagNode | DagGroup | undefined;

// DAG Props Converted (for interaction with Renderer)
@Output() graphResize = new EventEmitter<GraphDims>();
Expand Down

0 comments on commit 69c55b2

Please sign in to comment.