Skip to content

Commit d23e22e

Browse files
koonpengkoonpeng
committed
fix more validation bugs
Signed-off-by: Teo Koon Peng <[email protected]>
1 parent 1549978 commit d23e22e

File tree

3 files changed

+146
-67
lines changed

3 files changed

+146
-67
lines changed

‎diagram-editor/frontend/edges/index.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -49,22 +49,22 @@ export type DefaultEdgeData = Record<string, never>;
4949
export type DefaultEdge = Edge<DefaultEdgeData, 'default'>;
5050

5151
type EdgeMapping = {
52-
default: { comp: DefaultEdge; data: DefaultEdgeData };
53-
unzip: { comp: UnzipEdge; data: UnzipEdgeData };
54-
forkResultOk: { comp: ForkResultOkEdge; data: ForkResultOkEdgeData };
55-
forkResultErr: { comp: ForkResultErrEdge; data: ForkResultErrEdgeData };
56-
splitKey: { comp: SplitKeyEdge; data: SplitKeyEdgeData };
57-
splitSeq: { comp: SplitSeqEdge; data: SplitSeqEdgeData };
58-
splitRemaining: { comp: SplitRemainingEdge; data: SplitRemainingEdgeData };
59-
bufferKey: { comp: BufferKeyEdge; data: BufferKeyEdgeData };
60-
bufferSeq: { comp: BufferSeqEdge; data: BufferSeqEdgeData };
61-
streamOut: { comp: StreamOutEdge; data: StreamOutEdgeData };
52+
default: { edge: DefaultEdge; data: DefaultEdgeData };
53+
unzip: { edge: UnzipEdge; data: UnzipEdgeData };
54+
forkResultOk: { edge: ForkResultOkEdge; data: ForkResultOkEdgeData };
55+
forkResultErr: { edge: ForkResultErrEdge; data: ForkResultErrEdgeData };
56+
splitKey: { edge: SplitKeyEdge; data: SplitKeyEdgeData };
57+
splitSeq: { edge: SplitSeqEdge; data: SplitSeqEdgeData };
58+
splitRemaining: { edge: SplitRemainingEdge; data: SplitRemainingEdgeData };
59+
bufferKey: { edge: BufferKeyEdge; data: BufferKeyEdgeData };
60+
bufferSeq: { edge: BufferSeqEdge; data: BufferSeqEdgeData };
61+
streamOut: { edge: StreamOutEdge; data: StreamOutEdgeData };
6262
};
6363

6464
export type EdgeTypes = keyof EdgeMapping;
6565

6666
export type DiagramEditorEdge<T extends EdgeTypes = EdgeTypes> =
67-
EdgeMapping[T]['comp'];
67+
EdgeMapping[T]['edge'];
6868

6969
export type EdgeData<T extends EdgeTypes = EdgeTypes> = EdgeMapping[T]['data'];
7070

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

Lines changed: 133 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import {
88
} from '../edges';
99
import {
1010
createOperationNode,
11+
createSectionBufferNode,
12+
createSectionInputNode,
13+
createSectionOutputNode,
1114
createTerminateNode,
1215
type DiagramEditorNode,
1316
} from '../nodes';
@@ -54,43 +57,22 @@ class MockReactFlowAccessor implements NodesAndEdgesAccessor {
5457
}
5558

5659
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(
60+
test('"buffer" can only connect to operations that accepts a buffer', () => {
61+
const node = createOperationNode(
6662
ROOT_NAMESPACE,
6763
undefined,
6864
{ x: 0, y: 0 },
6965
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
7066
'test_op_node',
7167
);
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(
68+
const buffer = createOperationNode(
8769
ROOT_NAMESPACE,
8870
undefined,
8971
{ x: 0, y: 0 },
9072
{ type: 'buffer' },
9173
'test_op_buffer',
9274
);
93-
const targetNode = createOperationNode(
75+
const join = createOperationNode(
9476
ROOT_NAMESPACE,
9577
undefined,
9678
{ x: 0, y: 0 },
@@ -102,47 +84,45 @@ describe('validate edges', () => {
10284
'test_op_join',
10385
);
10486

105-
const validEdges = getValidEdgeTypes(sourceNode, targetNode);
106-
expect(validEdges.length).toBe(2);
107-
expect(validEdges).toContain('bufferKey');
108-
expect(validEdges).toContain('bufferSeq');
87+
{
88+
// "node" does not accept buffer
89+
const validEdges = getValidEdgeTypes(buffer, node);
90+
expect(validEdges.length).toBe(0);
91+
92+
// "buffer" does not output data ("default" edge)
93+
const edge = createDefaultEdge(buffer.id, join.id);
94+
const reactFlow = new MockReactFlowAccessor([buffer, join], [edge]);
95+
const result = validateEdgeQuick(edge, reactFlow);
96+
expect(result.valid).toBe(false);
97+
}
98+
99+
{
100+
const validEdges = getValidEdgeTypes(buffer, join);
101+
expect(validEdges.length).toBe(2);
102+
expect(validEdges).toContain('bufferKey');
103+
expect(validEdges).toContain('bufferSeq');
104+
}
109105

110106
{
111-
const edge = createBufferSeqEdge(sourceNode.id, targetNode.id, {
107+
const edge = createBufferSeqEdge(buffer.id, join.id, {
112108
seq: 0,
113109
});
114-
const reactFlow = new MockReactFlowAccessor(
115-
[sourceNode, targetNode],
116-
[edge],
117-
);
110+
const reactFlow = new MockReactFlowAccessor([buffer, join], [edge]);
118111
const result = validateEdgeQuick(edge, reactFlow);
119112
expect(result.valid).toBe(true);
120113
}
121114

122115
{
123-
const edge = createBufferKeyEdge(sourceNode.id, targetNode.id, {
116+
const edge = createBufferKeyEdge(buffer.id, join.id, {
124117
key: 'test',
125118
});
126-
const reactFlow = new MockReactFlowAccessor(
127-
[sourceNode, targetNode],
128-
[edge],
129-
);
119+
const reactFlow = new MockReactFlowAccessor([buffer, join], [edge]);
130120
const result = validateEdgeQuick(edge, reactFlow);
131121
expect(result.valid).toBe(true);
132122
}
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-
}
143123
});
144124

145-
test('node->buffer_access and buffer->buffer_access are valid', () => {
125+
test('"buffer_access" accepts both data and buffer edges', () => {
146126
const nodeNode = createOperationNode(
147127
ROOT_NAMESPACE,
148128
undefined,
@@ -178,7 +158,7 @@ describe('validate edges', () => {
178158
}
179159
});
180160

181-
test('node->join and buffer->join are valid', () => {
161+
test('"join" node accepts both data and buffer edges', () => {
182162
const nodeNode = createOperationNode(
183163
ROOT_NAMESPACE,
184164
undefined,
@@ -223,7 +203,106 @@ describe('validate edges', () => {
223203
}
224204
});
225205

226-
test('node operation only allows 1 output', () => {
206+
test('"sectionInput" can only connect to operations that accepts data', () => {
207+
const sectionInput = createSectionInputNode(
208+
'test_section_input',
209+
'test_section_input',
210+
{ x: 0, y: 0 },
211+
);
212+
const node = createOperationNode(
213+
ROOT_NAMESPACE,
214+
undefined,
215+
{ x: 0, y: 0 },
216+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
217+
'test_op_node',
218+
);
219+
const listen = createOperationNode(
220+
ROOT_NAMESPACE,
221+
undefined,
222+
{ x: 0, y: 0 },
223+
{ type: 'listen', buffers: [], next: { builtin: 'dispose' } },
224+
'test_op_listen',
225+
);
226+
227+
{
228+
const validEdges = getValidEdgeTypes(sectionInput, node);
229+
expect(validEdges.length).toBe(1);
230+
expect(validEdges).toContain('default');
231+
}
232+
233+
{
234+
const validEdges = getValidEdgeTypes(sectionInput, listen);
235+
expect(validEdges.length).toBe(0);
236+
}
237+
});
238+
239+
test('"sectionBuffer" can only connect to operations that accepts buffer', () => {
240+
const sectionBuffer = createSectionBufferNode(
241+
'test_section_buffer',
242+
'test_section_buffer',
243+
{ x: 0, y: 0 },
244+
);
245+
const node = createOperationNode(
246+
ROOT_NAMESPACE,
247+
undefined,
248+
{ x: 0, y: 0 },
249+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
250+
'test_op_node',
251+
);
252+
const listen = createOperationNode(
253+
ROOT_NAMESPACE,
254+
undefined,
255+
{ x: 0, y: 0 },
256+
{ type: 'listen', buffers: [], next: { builtin: 'dispose' } },
257+
'test_op_listen',
258+
);
259+
260+
{
261+
const validEdges = getValidEdgeTypes(sectionBuffer, node);
262+
expect(validEdges.length).toBe(0);
263+
}
264+
265+
{
266+
const validEdges = getValidEdgeTypes(sectionBuffer, listen);
267+
expect(validEdges.length).toBe(2);
268+
expect(validEdges).toContain('bufferKey');
269+
expect(validEdges).toContain('bufferSeq');
270+
}
271+
});
272+
273+
test('"sectionOutput" only accepts data edges', () => {
274+
const sectionOutput = createSectionOutputNode('test_section_output', {
275+
x: 0,
276+
y: 0,
277+
});
278+
const node = createOperationNode(
279+
ROOT_NAMESPACE,
280+
undefined,
281+
{ x: 0, y: 0 },
282+
{ type: 'node', builder: 'test_builder', next: { builtin: 'dispose' } },
283+
'test_op_node',
284+
);
285+
const buffer = createOperationNode(
286+
ROOT_NAMESPACE,
287+
undefined,
288+
{ x: 0, y: 0 },
289+
{ type: 'buffer', buffers: [] },
290+
'test_op_buffer',
291+
);
292+
293+
{
294+
const validEdges = getValidEdgeTypes(node, sectionOutput);
295+
expect(validEdges.length).toBe(1);
296+
expect(validEdges).toContain('default');
297+
}
298+
299+
{
300+
const validEdges = getValidEdgeTypes(buffer, sectionOutput);
301+
expect(validEdges.length).toBe(0);
302+
}
303+
});
304+
305+
test('"node" operation only allows 1 output', () => {
227306
const nodeNode = createOperationNode(
228307
ROOT_NAMESPACE,
229308
undefined,
@@ -262,7 +341,7 @@ describe('validate edges', () => {
262341
}
263342
});
264343

265-
test('fork clone operation allows multiple outputs', () => {
344+
test('"fork_clone" operation allows multiple outputs', () => {
266345
const forkCloneNode = createOperationNode(
267346
ROOT_NAMESPACE,
268347
undefined,
@@ -288,7 +367,7 @@ describe('validate edges', () => {
288367
}
289368
});
290369

291-
test('fork result operation only allows 2 outputs', () => {
370+
test('"fork_result" operation only allows 2 outputs', () => {
292371
const forkResultNode = createOperationNode(
293372
ROOT_NAMESPACE,
294373
undefined,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ const ALLOWED_OUTPUT_EDGES: Record<NodeTypes, EdgeTypes[]> = {
3434
section: ['default'],
3535
sectionInput: ['default'],
3636
sectionOutput: [],
37-
sectionBuffer: ['default'],
37+
sectionBuffer: ['bufferKey', 'bufferSeq'],
3838
serialized_join: ['default'],
3939
split: ['splitKey', 'splitSeq', 'splitRemaining'],
4040
start: ['default'],
@@ -50,7 +50,7 @@ const ALLOWED_INPUT_EDGE_CATEGORIES: Record<NodeTypes, EdgeCategory[]> = {
5050
fork_clone: [EdgeCategory.Data],
5151
fork_result: [EdgeCategory.Data],
5252
join: [EdgeCategory.Buffer, EdgeCategory.Data],
53-
listen: [EdgeCategory.Data, EdgeCategory.Buffer],
53+
listen: [EdgeCategory.Buffer],
5454
node: [EdgeCategory.Data],
5555
scope: [EdgeCategory.Data],
5656
section: [EdgeCategory.Data],

0 commit comments

Comments
 (0)