diff --git a/src/component/parser/xml/BpmnXmlParser.ts b/src/component/parser/xml/BpmnXmlParser.ts index b60aaeeda1..de97d04c51 100644 --- a/src/component/parser/xml/BpmnXmlParser.ts +++ b/src/component/parser/xml/BpmnXmlParser.ts @@ -32,6 +32,15 @@ const entitiesReplacements: Replacement[] = [ { regex: /&(quot|#34|#x22);/g, val: '"' }, ]; +const nodesWithNumericAttributes = new Set( + ['BPMNShape.Bounds', 'BPMNShape.BPMNLabel.Bounds', 'BPMNEdge.BPMNLabel.Bounds', 'BPMNEdge.waypoint'].map(element => `definitions.BPMNDiagram.BPMNPlane.${element}`), +); +const numericAttributes = new Set(['x', 'y', 'width', 'height']); + +const isNumeric = (attributeName: string, nodePath: string): boolean => { + return nodesWithNumericAttributes.has(nodePath) && numericAttributes.has(attributeName); +}; + /** * @internal */ @@ -46,10 +55,28 @@ export default class BpmnXmlParser { attributeNamePrefix: '', // default to '@_' removeNSPrefix: true, ignoreAttributes: false, - parseAttributeValue: true, // ensure numbers are parsed as number, not as string - // entities management - processEntities: false, // If you don't have entities in your XML document then it is recommended to disable it for better performance. - attributeValueProcessor: (_name: string, value: string) => { + + /** + * Ensure numbers and booleans are parsed with their related type and not as string. + * See https://github.com/NaturalIntelligence/fast-xml-parser/blob/v4.3.4/docs/v4/2.XMLparseOptions.md#parseattributevalue + */ + parseAttributeValue: true, + + /** + * Entities management. The recommendation is: "If you don't have entities in your XML document then it is recommended to disable it for better performance." + * See https://github.com/NaturalIntelligence/fast-xml-parser/blob/v4.3.4/docs/v4/2.XMLparseOptions.md#processentities + */ + processEntities: false, + + // See https://github.com/NaturalIntelligence/fast-xml-parser/blob/v4.3.4/docs/v4/2.XMLparseOptions.md#attributevalueprocessor + attributeValueProcessor: (name: string, value: string, nodePath: string): unknown => { + if (isNumeric(name, nodePath)) { + // The strnum lib used by fast-xml-parser is not able to parse all numbers + // The only available options are https://github.com/NaturalIntelligence/fast-xml-parser/blob/v4.3.4/docs/v4/2.XMLparseOptions.md#numberparseoptions + // This is a fix for https://github.com/process-analytics/bpmn-visualization-js/issues/2857 + return Number(value); + } + return this.processAttribute(value); }, }; diff --git a/test/fixtures/bpmn/xml-parsing/itp-commerce_Vizi-Modeler-for-Microsoft-Visio_7.7151.18707_bug-2857.bpmn b/test/fixtures/bpmn/xml-parsing/itp-commerce_Vizi-Modeler-for-Microsoft-Visio_7.7151.18707_bug-2857.bpmn new file mode 100644 index 0000000000..9b9fa6a6ad --- /dev/null +++ b/test/fixtures/bpmn/xml-parsing/itp-commerce_Vizi-Modeler-for-Microsoft-Visio_7.7151.18707_bug-2857.bpmn @@ -0,0 +1,170 @@ + + + + + + _0a5cddd1-ba79-4a79-a523-f865393d895c + + + _0a5cddd1-ba79-4a79-a523-f865393d895c + _07921065-0775-4224-bbe2-248230479a18 + _316925d8-db99-456f-8fab-b4b06e8ea139 + + + _316925d8-db99-456f-8fab-b4b06e8ea139 + _003a851a-471f-4b88-b386-9e9538b4c12c + _782252f0-e833-421c-a958-c3da4dc791a1 + + + _07921065-0775-4224-bbe2-248230479a18 + _1a8fd619-0ffc-4e68-88b0-eb09e2bca9b4 + _1edba66a-70a3-4a01-b084-886e3a1eb2ed + + + _782252f0-e833-421c-a958-c3da4dc791a1 + + + _003a851a-471f-4b88-b386-9e9538b4c12c + + + _1a8fd619-0ffc-4e68-88b0-eb09e2bca9b4 + + + _1edba66a-70a3-4a01-b084-886e3a1eb2ed + + + + + test='Does it move - no' + + + test='Does it move - no' + + + test='Should it move + yes' + + + + test='Should it move – no' + + + test='Should it move – yes' + + + test='Should it move + yes' + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/fixtures/bpmn/xml-parsing/special/simple-start-task-end_large_numbers_and_large_decimals.bpmn b/test/fixtures/bpmn/xml-parsing/special/simple-start-task-end_large_numbers_and_large_decimals.bpmn index 798a421c37..1b344199db 100644 --- a/test/fixtures/bpmn/xml-parsing/special/simple-start-task-end_large_numbers_and_large_decimals.bpmn +++ b/test/fixtures/bpmn/xml-parsing/special/simple-start-task-end_large_numbers_and_large_decimals.bpmn @@ -16,21 +16,27 @@ - - + + + + + - - + + - + - + + + + diff --git a/test/integration/mxGraph.model.bpmn.elements.test.ts b/test/integration/mxGraph.model.bpmn.elements.test.ts index 81403184f0..fb838a7a8e 100644 --- a/test/integration/mxGraph.model.bpmn.elements.test.ts +++ b/test/integration/mxGraph.model.bpmn.elements.test.ts @@ -36,7 +36,7 @@ import { ShapeBpmnMarkerKind, ShapeBpmnSubProcessKind, } from '@lib/bpmn-visualization'; -import { mxgraph, mxConstants, mxPoint } from '@lib/component/mxgraph/initializer'; +import { mxConstants, mxgraph, mxPoint } from '@lib/component/mxgraph/initializer'; import { readFileSync } from '@test/shared/file-helper'; const mxGeometry = mxgraph.mxGeometry; @@ -44,8 +44,10 @@ const mxGeometry = mxgraph.mxGeometry; describe('mxGraph model - BPMN elements', () => { describe('BPMN elements should be available in the mxGraph model', () => { describe('Diagram with all the kind of elements', () => { - // load BPMN - bpmnVisualization.load(readFileSync('../fixtures/bpmn/model-complete-semantic.bpmn')); + beforeAll(() => { + // load BPMN + bpmnVisualization.load(readFileSync('../fixtures/bpmn/model-complete-semantic.bpmn')); + }); const expectedBoldFont = { isBold: true, @@ -1517,8 +1519,7 @@ describe('mxGraph model - BPMN elements', () => { it('Diagram with a not displayed pool (without shape) with elements', () => { // load BPMN - const bpmnDiagramToFilter = readFileSync('../fixtures/bpmn/bpmn-rendering/pools.04.not.displayed.with.elements.bpmn'); - bpmnVisualization.load(bpmnDiagramToFilter); + bpmnVisualization.load(readFileSync('../fixtures/bpmn/bpmn-rendering/pools.04.not.displayed.with.elements.bpmn')); expectPoolsInModel(0); @@ -1678,28 +1679,29 @@ describe('mxGraph model - BPMN elements', () => { it('Parse a diagram with large numbers and large decimals', () => { bpmnVisualization.load(readFileSync('../fixtures/bpmn/xml-parsing/special/simple-start-task-end_large_numbers_and_large_decimals.bpmn')); + const startEvent1Geometry = new mxGeometry( + 156.100_010_002_564_63, + 81.345_000_000_000_01, // 81.345000000000009 in the diagram + // eslint-disable-next-line @typescript-eslint/no-loss-of-precision + 36.000_345_000_100_000_2, // 36.0003450001000002 in the diagram + 36.000_000_100_354_96, + ); + startEvent1Geometry.offset = new mxPoint(1.899_989_997_435_369_6, 42.654_999_999_999_99); expect('StartEvent_1').toBeCellWithParentAndGeometry({ parentId: defaultParentId, - geometry: new mxGeometry( - 156.100_01, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - '81.3450000000000090', // 81.345000000000009 in the diagram - '36.0003450001000002', - 36.000_000_1, - ), + geometry: startEvent1Geometry, }); expect('Activity_1').toBeCellWithParentAndGeometry({ parentId: defaultParentId, - geometry: new mxGeometry(250, 59, 100, 80), + geometry: new mxGeometry(250, 59.795_444_2, 100.678_942_1, 80), }); - const geometry = new mxGeometry(412, 81, 36, 36); - geometry.offset = new mxPoint(4.16e25, 1.240_000_000_03e29); + const endEvent1Geometry = new mxGeometry(412, 81, 36, 36); + endEvent1Geometry.offset = new mxPoint(4.16e25, 1.240_000_000_03e29); expect('EndEvent_1').toBeCellWithParentAndGeometry({ parentId: defaultParentId, - geometry: geometry, + geometry: endEvent1Geometry, }); }); @@ -1709,10 +1711,8 @@ describe('mxGraph model - BPMN elements', () => { expect('Activity_1').toBeCellWithParentAndGeometry({ parentId: defaultParentId, geometry: new mxGeometry( - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore malformed source, conversion result - 'not_a_number0', // from 'not_a_number' - 'not a number too0', // from 'not a number too' + Number.NaN, // from 'not_a_number' + Number.NaN, // from 'not a number too' -100, -80, ), diff --git a/test/unit/component/parser/xml/BpmnXmlParser.00.special.parsing.cases.test.ts b/test/unit/component/parser/xml/BpmnXmlParser.00.special.parsing.cases.test.ts index 0b77da87ca..ed0d7541c0 100644 --- a/test/unit/component/parser/xml/BpmnXmlParser.00.special.parsing.cases.test.ts +++ b/test/unit/component/parser/xml/BpmnXmlParser.00.special.parsing.cases.test.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import type { BPMNDiagram, BPMNLabel, BPMNShape } from '@lib/model/bpmn/json/bpmndi'; +import type { BPMNDiagram, BPMNEdge, BPMNLabel, BPMNShape } from '@lib/model/bpmn/json/bpmndi'; import BpmnXmlParser from '@lib/component/parser/xml/BpmnXmlParser'; import Bounds from '@lib/model/bpmn/internal/Bounds'; @@ -38,18 +38,58 @@ describe('Special parsing cases', () => { const shapes = bpmnDiagram.BPMNPlane.BPMNShape as BPMNShape[]; const getShape = (id: string): BPMNShape => shapes.find(s => s.id == id); - // string instead of number - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore width and y are parsed as string. They have too many decimals - expect(getShape('BPMNShape_StartEvent_1').Bounds).toEqual(new Bounds(156.100_01, '81.345000000000009', '36.0003450001000002', 36.000_000_1)); + const edges = bpmnDiagram.BPMNPlane.BPMNEdge as BPMNEdge[]; + const getEdge = (id: string): BPMNEdge => edges.find(s => s.id == id); - expect(getShape('BPMNShape_Activity_1').Bounds).toEqual(new Bounds(250, 59, 100, 80)); + expect(getShape('BPMNShape_StartEvent_1').Bounds).toEqual( + new Bounds( + 156.100_010_002_564_63, // 156.10001000256464316843136874561354684 in the diagram + 81.345_000_000_000_01, // 81.345000000000009 in the diagram + 36.000_345_000_1, // 36.0003450001000002 in the diagram + 36.000_000_100_354_96, // 36.00000010035496143139997251548445 in the diagram + ), + ); + + const activity1 = getShape('BPMNShape_Activity_1'); + // standard numbers for bounds + expect(activity1.Bounds).toEqual(new Bounds(250, 59.795_444_2, 100.678_942_1, 80)); + // large number decimals for label bounds + expect((activity1.BPMNLabel as BPMNLabel).Bounds).toEqual( + new Bounds( + 251.546_168_735_168_75, // 251.546168735168735133580035789 in the diagram + 62.535_763_100_004_69, // 62.5357631000046898412244767058 in the diagram + 33.659_851_435_460_055, // 33.659851435460054800548744548 in the diagram + 18.245_131_658_435_167, // '18.245131658435165843221640005446841658416841 in the diagram + ), + ); // large numbers use scientific notation or converted as string const endEventShape = getShape('BPMNShape_EndEvent_1'); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore width converted to string to not get a truncated number at runtime - expect((endEventShape.BPMNLabel as BPMNLabel).Bounds).toEqual(new Bounds(4.16e25, 1.240_000_000_03e29, '20000000000000000009', 1.4e21)); + expect((endEventShape.BPMNLabel as BPMNLabel).Bounds).toEqual(new Bounds(4.16e25, 1.240_000_000_03e29, 20_000_000_000_000_000_000, 1.4e21)); + + // label bounds of edge + const edge1 = getEdge('BPMNEdge_Flow_1'); + expect((edge1.BPMNLabel as BPMNLabel).Bounds).toEqual( + new Bounds( + 258.654_687_421_687_64, // 258.6546874216876435469813 in the diagram + 94.549_684_316_846_52, // 94.549684316846518435138654654687 in the diagram + 53.126_461_365_874_65, // 53.1264613658746467887414 in the diagram + 84.431_876_431_357_68, // 84.4318764313576846543564684651454646 in the diagram + ), + ); + const edge1Waypoints = edge1.waypoint; + expect(edge1Waypoints).toHaveLength(2); + expect(edge1Waypoints[0]).toEqual({ x: 192, y: 99.4 }); + expect(edge1Waypoints[1]).toEqual({ x: 250.12, y: 99.9246 }); + + // waypoints of edge + const edge2Waypoints = getEdge('BPMNEdge_Flow_2').waypoint; + expect(edge2Waypoints).toHaveLength(2); + expect(edge2Waypoints[0]).toEqual({ + x: 350.010_000_000_545_46, // 350.010000000545455749847855625445 in the diagram + y: 99.000_000_000_048_54, // 99.000000000048548464923279646 in the diagram + }); + expect(edge2Waypoints[1]).toEqual({ x: 412.658, y: 99.12 }); }); it('Parse a diagram with numbers not parsable as number', () => { @@ -60,9 +100,14 @@ describe('Special parsing cases', () => { const getShape = (id: string): BPMNShape => shapes.find(s => s.id == id); // x and y values are string instead of number in the source diagram - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore x and y are parsed as string as defined in the BPMN diagram - expect(getShape('BPMNShape_Activity_1').Bounds).toEqual(new Bounds('not_a_number', 'not a number too', -100, -80)); + expect(getShape('BPMNShape_Activity_1').Bounds).toEqual( + new Bounds( + Number.NaN, // 'not_a_number' in the diagram + Number.NaN, // 'not a number too' in the diagram + -100, + -80, + ), + ); }); it('Parse a diagram with entities in the name attributes', () => { diff --git a/test/unit/component/parser/xml/BpmnXmlParser.itp-commerce.7-7151-18707.test.ts b/test/unit/component/parser/xml/BpmnXmlParser.itp-commerce.7-7151-18707.test.ts new file mode 100644 index 0000000000..33d7fbf974 --- /dev/null +++ b/test/unit/component/parser/xml/BpmnXmlParser.itp-commerce.7-7151-18707.test.ts @@ -0,0 +1,56 @@ +/* +Copyright 2024 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 type { TProcess } from '@lib/model/bpmn/json/baseElement/rootElement/rootElement'; +import type { BPMNDiagram, BPMNEdge, BPMNShape } from '@lib/model/bpmn/json/bpmndi'; + +import BpmnXmlParser from '@lib/component/parser/xml/BpmnXmlParser'; +import { readFileSync } from '@test/shared/file-helper'; + +describe('parse bpmn as xml for ipt-commerce Vizi Modeler for Microsoft Visio 7.7151.18707', () => { + it('bpmn with process with extension, ensure elements are present', () => { + const a20Process = readFileSync('../fixtures/bpmn/xml-parsing/itp-commerce_Vizi-Modeler-for-Microsoft-Visio_7.7151.18707_bug-2857.bpmn'); + + const json = new BpmnXmlParser().parse(a20Process); + + const process: TProcess = json.definitions.process as TProcess; + expect(process.task).toHaveLength(4); + expect(process.exclusiveGateway).toHaveLength(3); + expect(process.sequenceFlow).toHaveLength(7); + + const bpmnPlane = (json.definitions.BPMNDiagram as BPMNDiagram).BPMNPlane; + const shapes = bpmnPlane.BPMNShape as BPMNShape[]; + expect(shapes).toHaveLength(8); + const edges = bpmnPlane.BPMNEdge as BPMNEdge[]; + expect(edges).toHaveLength(7); + + // Ensure that coordinates are parsed correctly as number. Fix for https://github.com/process-analytics/bpmn-visualization-js/issues/2857 + const shapeBounds = shapes.find(shape => shape.id === '_BA5FD27D-30DD-4F4F-93B3-A76CC5C4D0D2').Bounds; + expect(shapeBounds).toEqual({ + x: 415.275_590_551_181_1, + y: 82.204_724_409_448_89, + width: 17.007_874_015_748_033, + height: 17.007_874_015_748_033, + }); + + const edgeWaypoints = edges.find(edge => edge.id === '_7A3B4F0A-FB98-41F2-A77E-DE47B57C3C28').waypoint; + expect(edgeWaypoints).toEqual([ + { x: 445.039_370_078_740_21, y: 182.480_314_960_629_93 }, + { x: 577.559_055_118_110_2, y: 182.480_314_960_629_93 }, + { x: 577.559_055_118_110_2, y: 239.173_228_346_456_77 }, + ]); + }); +});