Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Don't store 'edge without bpmn element' in the BpmnModel #1113

Merged
merged 1 commit into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions src/component/parser/json/converter/DiagramConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,28 @@ export default class DiagramConverter {
}

private deserializeEdges(edges: BPMNEdge | BPMNEdge[]): Edge[] {
return ensureIsArray(edges).map(edge => {
const flow =
this.convertedElements.findSequenceFlow(edge.bpmnElement) ||
this.convertedElements.findMessageFlow(edge.bpmnElement) ||
this.convertedElements.findAssociationFlow(edge.bpmnElement);
const waypoints = this.deserializeWaypoints(edge.waypoint);
const label = this.deserializeLabel(edge.BPMNLabel, edge.id);

// TODO Remove messageVisibleKind conversion type when we merge/simplify internal model with BPMN json model
const messageVisibleKind = edge.messageVisibleKind ? ((edge.messageVisibleKind as unknown) as MessageVisibleKind) : MessageVisibleKind.NONE;

return new Edge(edge.id, flow, waypoints, label, messageVisibleKind);
});
return ensureIsArray(edges)
.map(edge => {
const flow =
this.convertedElements.findSequenceFlow(edge.bpmnElement) ||
this.convertedElements.findMessageFlow(edge.bpmnElement) ||
this.convertedElements.findAssociationFlow(edge.bpmnElement);

if (!flow) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

// TODO error management
console.warn('Edge json deserialization: unable to find bpmn element with id %s', edge.bpmnElement);
return;
}

const waypoints = this.deserializeWaypoints(edge.waypoint);
const label = this.deserializeLabel(edge.BPMNLabel, edge.id);

// TODO Remove messageVisibleKind conversion type when we merge/simplify internal model with BPMN json model
const messageVisibleKind = edge.messageVisibleKind ? ((edge.messageVisibleKind as unknown) as MessageVisibleKind) : MessageVisibleKind.NONE;

return new Edge(edge.id, flow, waypoints, label, messageVisibleKind);
})
.filter(edge => edge);
}

private deserializeWaypoints(waypoints: Point[]): Waypoint[] {
Expand Down
49 changes: 49 additions & 0 deletions test/unit/component/parser/json/BpmnJsonParser.edge.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/**
* Copyright 2021 Bonitasoft S.A.
*
* 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.
*/
import { parseJsonAndExpectOnlyEdges } from './JsonTestUtils';

describe('parse bpmn as json for edges', () => {
jest.spyOn(console, 'warn');

afterEach(() => {
jest.clearAllMocks();
});

// this also covers unsupported bpmn element types that are then not retrieved during the parsing
it('should not convert as Edge without related BPMN element', () => {
console.warn = jest.fn();
const json = {
definitions: {
targetNamespace: '',
process: '',
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'edge-bpmnElement-unknown',
waypoint: [{ x: 10, y: 10 }],
},
},
},
},
};

parseJsonAndExpectOnlyEdges(json, 0);
expect(console.warn).toHaveBeenCalledWith('Edge json deserialization: unable to find bpmn element with id %s', 'edge-bpmnElement-unknown');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,20 @@ describe('parse bpmn as json for label bounds', () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id',
Expand All @@ -127,13 +134,20 @@ describe('parse bpmn as json for label bounds', () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id',
Expand Down
69 changes: 60 additions & 9 deletions test/unit/component/parser/json/BpmnJsonParser.label.font.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,24 @@ describe('parse bpmn as json for label font', () => {
},
);

it("should convert as Edge with Font, when a BPMNEdge (who references a BPMNLabelStyle with font) is an attribute (as object) of 'BPMNPlane' (as object)", () => {
it("should convert as Edge with Font, when a BPMNEdge (which references a BPMNLabelStyle with font) is an attribute (as object) of 'BPMNPlane' (as object)", () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef',
targetRef: 'targetRef',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id',
Expand Down Expand Up @@ -174,21 +181,36 @@ describe('parse bpmn as json for label font', () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: [
{
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
{
id: 'sequenceFlow_id_1',
sourceRef: 'sourceRef_1',
targetRef: 'targetRef_1',
},
],
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: [
{
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
labelStyle: 'style_id_1',
},
},
{
id: 'BPMNEdge_id_1',
bpmnElement: 'sequenceFlow_id_1',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
labelStyle: 'style_id_1',
Expand Down Expand Up @@ -280,18 +302,32 @@ describe('parse bpmn as json for label font', () => {
expect(model.flowNodes[1].label).toBeUndefined();
});

it("should convert as Edge[] without Font, when BPMNEdges (who reference a BPMNLabelStyle) are an attribute (as array) of 'BPMNPlane' (as object) & BPMNLabelStyle (with font with/without all attributes) is an attribute (as array) of 'BPMNDiagram' (as object)", () => {
it("should convert as Edge[] without Font, when BPMNEdges (which reference a BPMNLabelStyle) are an attribute (as array) of 'BPMNPlane' (as object) & BPMNLabelStyle (with font with/without all attributes) is an attribute (as array) of 'BPMNDiagram' (as object)", () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: [
{
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
{
id: 'sequenceFlow_id_1',
sourceRef: 'sourceRef_1',
targetRef: 'targetRef_1',
},
],
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: [
{
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id_1',
Expand All @@ -300,6 +336,7 @@ describe('parse bpmn as json for label font', () => {
},
{
id: 'BPMNEdge_id_1',
bpmnElement: 'sequenceFlow_id_1',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id_2',
Expand Down Expand Up @@ -372,17 +409,24 @@ describe('parse bpmn as json for label font', () => {
expect(model.flowNodes[0].label).toBeUndefined();
});

it("should convert as Edge without Font, when a BPMNEdge (who references a BPMNLabelStyle without font) is an attribute (as object) of 'BPMNPlane' (as object)", () => {
it("should convert as Edge without Font, when a BPMNEdge (which references a BPMNLabelStyle without font) is an attribute (as object) of 'BPMNPlane' (as object)", () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id',
Expand Down Expand Up @@ -437,18 +481,25 @@ describe('parse bpmn as json for label font', () => {
expect(console.warn).toHaveBeenCalledWith('Unable to assign font from style %s to shape/edge %s', 'non-existing_style_id', 'BPMNShape_id_0');
});

it("should convert as Edge without Font, when a BPMNEdge (who references a non-existing BPMNLabelStyle) is an attribute (as object) of 'BPMNPlane' (as object)", () => {
it("should convert as Edge without Font, when a BPMNEdge (which references a non-existing BPMNLabelStyle) is an attribute (as object) of 'BPMNPlane' (as object)", () => {
console.warn = jest.fn();
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: 'label_id',
Expand Down
27 changes: 24 additions & 3 deletions test/unit/component/parser/json/BpmnJsonParser.label.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,20 @@ describe('parse bpmn as json for label font', () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: '',
},
Expand Down Expand Up @@ -123,13 +130,20 @@ describe('parse bpmn as json for label font', () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
BPMNLabel: {
id: '',
Expand Down Expand Up @@ -178,13 +192,20 @@ describe('parse bpmn as json for label font', () => {
const json = {
definitions: {
targetNamespace: '',
process: '',
process: {
sequenceFlow: {
id: 'sequenceFlow_id_0',
sourceRef: 'sourceRef_0',
targetRef: 'targetRef_0',
},
},
BPMNDiagram: {
id: 'BpmnDiagram_1',
BPMNPlane: {
id: 'BpmnPlane_1',
BPMNEdge: {
id: 'BPMNEdge_id_0',
bpmnElement: 'sequenceFlow_id_0',
waypoint: [{ x: 10, y: 10 }],
},
},
Expand Down