Skip to content

Commit 8d1fa77

Browse files
koonpengkoonpeng
committed
fix several edge validation errors
Signed-off-by: Teo Koon Peng <[email protected]>
1 parent b3b2acc commit 8d1fa77

File tree

3 files changed

+432
-70
lines changed

3 files changed

+432
-70
lines changed

‎diagram-editor/frontend/diagram-editor.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ import {
4949
} from './nodes';
5050
import { useTemplates } from './templates-provider';
5151
import { autoLayout } from './utils/auto-layout';
52-
import { checkValidEdgeSimple, getValidEdgeTypes } from './utils/connection';
52+
import { getValidEdgeTypes, validateEdgeSimple } from './utils/connection';
5353
import { exhaustiveCheck } from './utils/exhaustive-check';
5454
import { exportTemplate } from './utils/export-diagram';
5555
import { calculateScopeBounds, LAYOUT_OPTIONS } from './utils/layout';
@@ -551,7 +551,7 @@ function DiagramEditor() {
551551
data: defaultEdgeData(validEdges[0]),
552552
} as DiagramEditorEdge;
553553

554-
const validationResult = checkValidEdgeSimple(
554+
const validationResult = validateEdgeSimple(
555555
newEdge,
556556
reactFlowInstance.current,
557557
);

‎diagram-editor/frontend/utils/connection.test.ts

Lines changed: 338 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,338 @@
1+
import {
2+
createBufferKeyEdge,
3+
createBufferSeqEdge,
4+
createDefaultEdge,
5+
createForkResultErrEdge,
6+
createForkResultOkEdge,
7+
type DiagramEditorEdge,
8+
} from '../edges';
9+
import {
10+
createOperationNode,
11+
createTerminateNode,
12+
type DiagramEditorNode,
13+
} from '../nodes';
14+
import {
15+
getValidEdgeTypes,
16+
type NodesAndEdgesAccessor,
17+
validateEdgeQuick,
18+
validateEdgeSimple,
19+
} from './connection';
20+
import { ROOT_NAMESPACE } from './namespace';
21+
22+
class MockReactFlowAccessor implements NodesAndEdgesAccessor {
23+
nodesMap: Record<string, DiagramEditorNode>;
24+
edgesMap: Record<string, DiagramEditorEdge>;
25+
26+
constructor(nodes: DiagramEditorNode[], edges: DiagramEditorEdge[]) {
27+
this.nodesMap = nodes.reduce(
28+
(map, node) => {
29+
map[node.id] = node;
30+
return map;
31+
},
32+
{} as Record<string, DiagramEditorNode>,
33+
);
34+
this.edgesMap = edges.reduce(
35+
(map, edge) => {
36+
map[edge.id] = edge;
37+
return map;
38+
},
39+
{} as Record<string, DiagramEditorEdge>,
40+
);
41+
}
42+
43+
getNode(id: string): DiagramEditorNode | undefined {
44+
return this.nodesMap[id];
45+
}
46+
47+
getNodes(): DiagramEditorNode[] {
48+
return Object.values(this.nodesMap);
49+
}
50+
51+
getEdges(): DiagramEditorEdge[] {
52+
return Object.values(this.edgesMap);
53+
}
54+
}
55+
56+
describe('validate edges', () => {
57+
test('buffer->node is invalid', () => {
58+
const sourceNode = createOperationNode(
59+
ROOT_NAMESPACE,
60+
undefined,
61+
{ x: 0, y: 0 },
62+
{ type: 'buffer' },
63+
'test_op_buffer',
64+
);
65+
const targetNode = createOperationNode(
66+
ROOT_NAMESPACE,
67+
undefined,
68+
{ x: 0, y: 0 },
69+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
70+
'test_op_node',
71+
);
72+
73+
const validEdges = getValidEdgeTypes(sourceNode, targetNode);
74+
expect(validEdges.length).toBe(0);
75+
76+
const edge = createDefaultEdge(sourceNode.id, targetNode.id);
77+
const reactFlow = new MockReactFlowAccessor(
78+
[sourceNode, targetNode],
79+
[edge],
80+
);
81+
const result = validateEdgeQuick(edge, reactFlow);
82+
expect(result.valid).toBe(false);
83+
});
84+
85+
test('buffer->join is valid only for buffer edges', () => {
86+
const sourceNode = createOperationNode(
87+
ROOT_NAMESPACE,
88+
undefined,
89+
{ x: 0, y: 0 },
90+
{ type: 'buffer' },
91+
'test_op_buffer',
92+
);
93+
const targetNode = createOperationNode(
94+
ROOT_NAMESPACE,
95+
undefined,
96+
{ x: 0, y: 0 },
97+
{
98+
type: 'join',
99+
buffers: [],
100+
next: { builtin: 'dispose' },
101+
},
102+
'test_op_join',
103+
);
104+
105+
const validEdges = getValidEdgeTypes(sourceNode, targetNode);
106+
expect(validEdges.length).toBe(2);
107+
expect(validEdges).toContain('bufferKey');
108+
expect(validEdges).toContain('bufferSeq');
109+
110+
{
111+
const edge = createBufferSeqEdge(sourceNode.id, targetNode.id, {
112+
seq: 0,
113+
});
114+
const reactFlow = new MockReactFlowAccessor(
115+
[sourceNode, targetNode],
116+
[edge],
117+
);
118+
const result = validateEdgeQuick(edge, reactFlow);
119+
expect(result.valid).toBe(true);
120+
}
121+
122+
{
123+
const edge = createBufferKeyEdge(sourceNode.id, targetNode.id, {
124+
key: 'test',
125+
});
126+
const reactFlow = new MockReactFlowAccessor(
127+
[sourceNode, targetNode],
128+
[edge],
129+
);
130+
const result = validateEdgeQuick(edge, reactFlow);
131+
expect(result.valid).toBe(true);
132+
}
133+
134+
{
135+
const edge = createDefaultEdge(sourceNode.id, targetNode.id);
136+
const reactFlow = new MockReactFlowAccessor(
137+
[sourceNode, targetNode],
138+
[edge],
139+
);
140+
const result = validateEdgeQuick(edge, reactFlow);
141+
expect(result.valid).toBe(false);
142+
}
143+
});
144+
145+
test('node->buffer_access and buffer->buffer_access are valid', () => {
146+
const nodeNode = createOperationNode(
147+
ROOT_NAMESPACE,
148+
undefined,
149+
{ x: 0, y: 0 },
150+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
151+
'test_op_node',
152+
);
153+
const bufferNode = createOperationNode(
154+
ROOT_NAMESPACE,
155+
undefined,
156+
{ x: 0, y: 0 },
157+
{ type: 'buffer' },
158+
'test_op_buffer',
159+
);
160+
const bufferAccessNode = createOperationNode(
161+
ROOT_NAMESPACE,
162+
undefined,
163+
{ x: 0, y: 0 },
164+
{ type: 'buffer_access', buffers: [], next: { builtin: 'dispose' } },
165+
'test_op_buffer_access',
166+
);
167+
168+
{
169+
const validEdges = getValidEdgeTypes(nodeNode, bufferAccessNode);
170+
expect(validEdges.length).toBe(1);
171+
expect(validEdges).toContain('default');
172+
}
173+
{
174+
const validEdges = getValidEdgeTypes(bufferNode, bufferAccessNode);
175+
expect(validEdges.length).toBe(2);
176+
expect(validEdges).toContain('bufferKey');
177+
expect(validEdges).toContain('bufferSeq');
178+
}
179+
});
180+
181+
test('node->join and buffer->join are valid', () => {
182+
const nodeNode = createOperationNode(
183+
ROOT_NAMESPACE,
184+
undefined,
185+
{ x: 0, y: 0 },
186+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
187+
'test_op_node',
188+
);
189+
const bufferNode = createOperationNode(
190+
ROOT_NAMESPACE,
191+
undefined,
192+
{ x: 0, y: 0 },
193+
{ type: 'buffer' },
194+
'test_op_buffer',
195+
);
196+
const joinNode = createOperationNode(
197+
ROOT_NAMESPACE,
198+
undefined,
199+
{ x: 0, y: 0 },
200+
{ type: 'join', buffers: [], next: { builtin: 'dispose' } },
201+
'test_op_join',
202+
);
203+
const serializedJoinNode = createOperationNode(
204+
ROOT_NAMESPACE,
205+
undefined,
206+
{ x: 0, y: 0 },
207+
{ type: 'serialized_join', buffers: [], next: { builtin: 'dispose' } },
208+
'test_op_serialized_join',
209+
);
210+
211+
for (const targetNode of [joinNode, serializedJoinNode]) {
212+
{
213+
const validEdges = getValidEdgeTypes(nodeNode, targetNode);
214+
expect(validEdges.length).toBe(1);
215+
expect(validEdges).toContain('default');
216+
}
217+
{
218+
const validEdges = getValidEdgeTypes(bufferNode, targetNode);
219+
expect(validEdges.length).toBe(2);
220+
expect(validEdges).toContain('bufferKey');
221+
expect(validEdges).toContain('bufferSeq');
222+
}
223+
}
224+
});
225+
226+
test('node operation only allows 1 output', () => {
227+
const nodeNode = createOperationNode(
228+
ROOT_NAMESPACE,
229+
undefined,
230+
{ x: 0, y: 0 },
231+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
232+
'test_op_node',
233+
);
234+
const forkCloneNode = createOperationNode(
235+
ROOT_NAMESPACE,
236+
undefined,
237+
{ x: 0, y: 0 },
238+
{ type: 'fork_clone', next: [] },
239+
'test_fork_clone',
240+
);
241+
const forkCloneNode2 = createOperationNode(
242+
ROOT_NAMESPACE,
243+
undefined,
244+
{ x: 0, y: 0 },
245+
{ type: 'fork_clone', next: [] },
246+
'test_fork_clone2',
247+
);
248+
249+
const existingEdge = createDefaultEdge(nodeNode.id, forkCloneNode.id);
250+
const reactFlow = new MockReactFlowAccessor(
251+
[nodeNode, forkCloneNode],
252+
[existingEdge],
253+
);
254+
{
255+
const result = validateEdgeSimple(existingEdge, reactFlow);
256+
expect(result.valid).toBe(true);
257+
}
258+
{
259+
const newEdge = createDefaultEdge(nodeNode.id, forkCloneNode2.id);
260+
const result = validateEdgeSimple(newEdge, reactFlow);
261+
expect(result.valid).toBe(false);
262+
}
263+
});
264+
265+
test('fork clone operation allows multiple outputs', () => {
266+
const forkCloneNode = createOperationNode(
267+
ROOT_NAMESPACE,
268+
undefined,
269+
{ x: 0, y: 0 },
270+
{ type: 'fork_clone', next: [] },
271+
'test_fork_clone',
272+
);
273+
const terminateNode = createTerminateNode(ROOT_NAMESPACE, { x: 0, y: 0 });
274+
275+
const edges = [
276+
createDefaultEdge(forkCloneNode.id, terminateNode.id),
277+
createDefaultEdge(forkCloneNode.id, terminateNode.id),
278+
];
279+
const reactFlow = new MockReactFlowAccessor(
280+
[forkCloneNode, terminateNode],
281+
edges,
282+
);
283+
284+
{
285+
const newEdge = createDefaultEdge(forkCloneNode.id, terminateNode.id);
286+
const result = validateEdgeSimple(newEdge, reactFlow);
287+
expect(result.valid).toBe(true);
288+
}
289+
});
290+
291+
test('fork result operation only allows 2 outputs', () => {
292+
const forkResultNode = createOperationNode(
293+
ROOT_NAMESPACE,
294+
undefined,
295+
{ x: 0, y: 0 },
296+
{
297+
type: 'fork_result',
298+
ok: { builtin: 'dispose' },
299+
err: { builtin: 'dispose' },
300+
},
301+
'test_fork_result',
302+
);
303+
const terminateNode = createTerminateNode(ROOT_NAMESPACE, { x: 0, y: 0 });
304+
305+
{
306+
const existingEdges = [
307+
createForkResultOkEdge(forkResultNode.id, terminateNode.id),
308+
];
309+
const reactFlow = new MockReactFlowAccessor(
310+
[forkResultNode, terminateNode],
311+
existingEdges,
312+
);
313+
const newEdge = createForkResultErrEdge(
314+
forkResultNode.id,
315+
terminateNode.id,
316+
);
317+
const result = validateEdgeSimple(newEdge, reactFlow);
318+
expect(result.valid).toBe(true);
319+
}
320+
321+
{
322+
const existingEdges = [
323+
createForkResultOkEdge(forkResultNode.id, terminateNode.id),
324+
createForkResultErrEdge(forkResultNode.id, terminateNode.id),
325+
];
326+
const reactFlow = new MockReactFlowAccessor(
327+
[forkResultNode, terminateNode],
328+
existingEdges,
329+
);
330+
const newEdge = createForkResultErrEdge(
331+
forkResultNode.id,
332+
terminateNode.id,
333+
);
334+
const result = validateEdgeSimple(newEdge, reactFlow);
335+
expect(result.valid).toBe(false);
336+
}
337+
});
338+
});

0 commit comments

Comments
 (0)