feat(modeling): repair broken connection dockings on import

Closes #479

lib/features/modeling/behavior/ImportDockingFix.js

@@ -0,0 +1,105 @@
+'use strict';
+
+var getMid = require('diagram-js/lib/layout/LayoutUtil').getMid;
+
+
+/**
+ * Fix broken dockings after DI imports.
+ *
+ * @param {EventBus} eventBus
+ */
+function ImportDockingFix(eventBus) {
+
+  function adjustDocking(startPoint, nextPoint, elementMid) {
+
+    var elementTop = {
+      x: elementMid.x,
+      y: elementMid.y - 50
+    };
+
+    var elementLeft = {
+      x: elementMid.x - 50,
+      y: elementMid.y
+    };
+
+    var verticalIntersect = lineIntersect(startPoint, nextPoint, elementMid, elementTop),
+        horizontalIntersect = lineIntersect(startPoint, nextPoint, elementMid, elementLeft);
+
+    // original is horizontal or vertical center cross intersection
+    var centerIntersect;
+
+    if (verticalIntersect && horizontalIntersect) {
+      if (getDistance(verticalIntersect, elementMid) > getDistance(horizontalIntersect, elementMid)) {
+        centerIntersect = horizontalIntersect;
+      } else {
+        centerIntersect = verticalIntersect;
+      }
+    } else {
+      centerIntersect = verticalIntersect || horizontalIntersect;
+    }
+
+    startPoint.original = centerIntersect;
+  }
+
+  function fixDockings(connection) {
+    var waypoints = connection.waypoints;
+
+    adjustDocking(
+      waypoints[0],
+      waypoints[1],
+      getMid(connection.source)
+    );
+
+    adjustDocking(
+      waypoints[waypoints.length - 1],
+      waypoints[waypoints.length - 2],
+      getMid(connection.target)
+    );
+  }
+
+  eventBus.on('bpmnElement.added', function(e) {
+
+    var element = e.element;
+
+    if (element.waypoints) {
+      fixDockings(element);
+    }
+  });
+}
+
+ImportDockingFix.$inject = [ 'eventBus' ];
+
+module.exports = ImportDockingFix;
+
+
+/////// helpers //////////////////////////////////
+
+function lineIntersect(l1s, l1e, l2s, l2e) {
+  // if the lines intersect, the result contains the x and y of the
+  // intersection (treating the lines as infinite) and booleans for
+  // whether line segment 1 or line segment 2 contain the point
+  var denominator, a, b, c, numerator;
+
+  denominator = ((l2e.y - l2s.y) * (l1e.x - l1s.x)) - ((l2e.x - l2s.x) * (l1e.y - l1s.y));
+
+  if (denominator == 0) {
+    return null;
+  }
+
+  a = l1s.y - l2s.y;
+  b = l1s.x - l2s.x;
+  numerator = ((l2e.x - l2s.x) * a) - ((l2e.y - l2s.y) * b);
+
+  c = numerator / denominator;
+
+  // if we cast these lines infinitely in
+  // both directions, they intersect here
+  return {
+    x: Math.round(l1s.x + (c * (l1e.x - l1s.x))),
+    y: Math.round(l1s.y + (c * (l1e.y - l1s.y)))
+  };
+}
+
+function getDistance(p1, p2) {
+  return Math.sqrt(Math.pow(p1.x - p2.x, 2) + Math.pow(p1.y - p2.y, 2));
+}

lib/features/modeling/behavior/index.js

@@ -8,6 +8,7 @@ module.exports = {
     'createParticipantBehavior',
     'dataInputAssociationBehavior',
     'deleteLaneBehavior',
+    'importDockingFix',
     'labelBehavior',
     'modelingFeedback',
     'removeParticipantBehavior',
@@ -27,6 +28,7 @@ module.exports = {
   createParticipantBehavior: [ 'type', require('./CreateParticipantBehavior') ],
   dataInputAssociationBehavior: [ 'type', require('./DataInputAssociationBehavior') ],
   deleteLaneBehavior: [ 'type', require('./DeleteLaneBehavior') ],
+  importDockingFix: [ 'type', require('./ImportDockingFix') ],
   labelBehavior: [ 'type', require('./LabelBehavior') ],
   modelingFeedback: [ 'type', require('./ModelingFeedback') ],
   removeParticipantBehavior: [ 'type', require('./RemoveParticipantBehavior') ],

test/spec/features/modeling/MoveConnectionSpec.js

@@ -29,7 +29,7 @@ describe('features/modeling - move connection', function() {
       // when
       modeling.moveConnection(sequenceFlowConnection, { x: 20, y: 10 });
 
-      var waypoints = [
+      var expectedWaypoints = [
         { x: 598, y: 351 },
         { x: 954, y: 351 },
         { x: 954, y: 446 },
@@ -39,10 +39,10 @@ describe('features/modeling - move connection', function() {
       // then
 
       // expect cropped connection
-      expect(sequenceFlowConnection.waypoints).eql(waypoints);
+      expect(sequenceFlowConnection).to.have.waypoints(expectedWaypoints);
 
       // expect cropped waypoints in di
-      var diWaypoints = bpmnFactory.createDiWaypoints(waypoints);
+      var diWaypoints = bpmnFactory.createDiWaypoints(expectedWaypoints);
 
       expect(sequenceFlow.di.waypoint).eql(diWaypoints);
     }));

test/spec/features/modeling/behavior/CreateOnFlowBehaviorSpec.js

@@ -4,6 +4,8 @@ require('../../../../TestHelper');
 
 /* global inject, bootstrapModeler */
 
+var flatten = require('lodash/array/flatten');
+
 var modelingModule = require('../../../../../lib/features/modeling');
 
 
@@ -40,10 +42,12 @@ describe('modeling/behavior - drop on connection', function() {
           sequenceFlow = elementRegistry.get('SequenceFlow'),
           task = elementRegistry.get('Task');
 
-      var position = { x: 340, y: 120 }; // first bendpoint
+      var originalWaypoints = sequenceFlow.waypoints;
+
+      var dropPosition = { x: 340, y: 120 }; // first bendpoint
 
       // when
-      var newShape = modeling.createShape(intermediateThrowEvent, position, sequenceFlow);
+      var newShape = modeling.createShape(intermediateThrowEvent, dropPosition, sequenceFlow);
 
       // then
 
@@ -62,16 +66,19 @@ describe('modeling/behavior - drop on connection', function() {
       expect(task.incoming[0]).to.equal(newShape.outgoing[0]);
 
       // split target at insertion point
-      expect(sequenceFlow.waypoints).eql([
+      expect(sequenceFlow).to.have.waypoints(flatten([
-        { original: { x: 209, y: 120 }, x: 209, y: 120 },
+        originalWaypoints.slice(0, 1),
-        { original: { x: 340, y: 120 }, x: 322, y: 120 }
+        { x: 322, y: 120 }
-      ]);
+      ]));
 
-      expect(targetConnection.waypoints).eql([
+      expect(sequenceFlow).to.have.endDocking(dropPosition);
-        { original: { x: 340, y: 120 }, x: 340, y: 138 },
+
-        { x: 340, y: 299 },
+      expect(targetConnection).to.have.waypoints(flatten([
-        { original: { x: 502, y: 299 }, x: 502, y: 299 }
+        { x: 340, y: 138 },
-      ]);
+        originalWaypoints.slice(2)
+      ]));
+
+      expect(targetConnection).to.have.startDocking(dropPosition);
     }));
 
 
@@ -81,11 +88,12 @@ describe('modeling/behavior - drop on connection', function() {
       var endEventShape = elementFactory.createShape({ type: 'bpmn:EndEvent' });
 
       var sequenceFlow = elementRegistry.get('SequenceFlow');
+      var originalWaypoints = sequenceFlow.waypoints;
 
-      var position = { x: 340, y: 120 }; // first bendpoint
+      var dropPosition = { x: 340, y: 120 }; // first bendpoint
 
       // when
-      var newShape = modeling.createShape(endEventShape, position, sequenceFlow);
+      var newShape = modeling.createShape(endEventShape, dropPosition, sequenceFlow);
 
       // then
 
@@ -97,10 +105,10 @@ describe('modeling/behavior - drop on connection', function() {
       expect(newShape.outgoing.length).to.equal(0);
 
       // split target at insertion point
-      expect(sequenceFlow.waypoints).eql([
+      expect(sequenceFlow).to.have.waypoints(flatten([
-        { original: { x: 209, y: 120 }, x: 209, y: 120 },
+        originalWaypoints.slice(0, 1),
-        { original: { x: 340, y: 120 }, x: 322, y: 120 }
+        { x: 322, y: 120 }
-      ]);
+      ]));
     }));
 
 
@@ -110,11 +118,12 @@ describe('modeling/behavior - drop on connection', function() {
       var startEventShape = elementFactory.createShape({ type: 'bpmn:StartEvent' });
 
       var sequenceFlow = elementRegistry.get('SequenceFlow');
+      var originalWaypoints = sequenceFlow.waypoints;
 
-      var position = { x: 340, y: 120 }; // first bendpoint
+      var dropPosition = { x: 340, y: 120 }; // first bendpoint
 
       // when
-      var newShape = modeling.createShape(startEventShape, position, sequenceFlow);
+      var newShape = modeling.createShape(startEventShape, dropPosition, sequenceFlow);
 
       // then
 
@@ -126,12 +135,12 @@ describe('modeling/behavior - drop on connection', function() {
       expect(newShape.outgoing[0]).to.eql(sequenceFlow);
 
       // split target at insertion point
-      expect(sequenceFlow.waypoints).eql([
+      expect(sequenceFlow).to.have.waypoints(flatten([
-        { original: { x: 340, y: 120 }, x: 340, y: 138 },
+        { x: 340, y: 138 },
-        { x: 340, y: 299 },
+        originalWaypoints.slice(2)
-        { original: { x: 502, y: 299 }, x: 502, y: 299 }
+      ]));
-      ]);
     }));
+
   });
 
 
@@ -144,13 +153,13 @@ describe('modeling/behavior - drop on connection', function() {
 
       var sequenceFlow = elementRegistry.get('SequenceFlow');
 
-      var position = { x: 340, y: 120 }; // first bendpoint
+      var dropPosition = { x: 340, y: 120 }; // first bendpoint
 
       // when
       var canDrop = rules.allowed('shape.create', {
         shape: participantShape,
         parent: sequenceFlow,
-        position: position
+        dropPosition: dropPosition
       });
 
       // then
@@ -159,4 +168,4 @@ describe('modeling/behavior - drop on connection', function() {
 
   });
 
-});
+});

test/spec/features/modeling/behavior/ImportDockingFix.bpmn

@@ -0,0 +1,61 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:bpmn="http://www.omg.org/spec/BPMN/20100524/MODEL" xmlns:bpmndi="http://www.omg.org/spec/BPMN/20100524/DI" xmlns:dc="http://www.omg.org/spec/DD/20100524/DC" xmlns:di="http://www.omg.org/spec/DD/20100524/DI" id="Definitions_1" targetNamespace="http://bpmn.io/schema/bpmn">
+  <bpmn:process id="Process_1" isExecutable="false">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>SequenceFlow</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:task id="Task_A" name="A">
+      <bpmn:incoming>SequenceFlow</bpmn:incoming>
+      <bpmn:property id="Property_09ddda9" name="__targetRef_placeholder" />
+      <bpmn:dataInputAssociation id="DataAssociation_1">
+        <bpmn:sourceRef>DataStoreReference</bpmn:sourceRef>
+        <bpmn:targetRef>Property_09ddda9</bpmn:targetRef>
+      </bpmn:dataInputAssociation>
+    </bpmn:task>
+    <bpmn:sequenceFlow id="SequenceFlow" sourceRef="StartEvent_1" targetRef="Task_A" />
+    <bpmn:dataStoreReference id="DataStoreReference" />
+    <bpmn:task id="Task_B">
+      <bpmn:property id="Property_0vybrii" name="__targetRef_placeholder" />
+      <bpmn:dataInputAssociation id="DataAssociation_2">
+        <bpmn:sourceRef>DataStoreReference</bpmn:sourceRef>
+        <bpmn:targetRef>Property_0vybrii</bpmn:targetRef>
+      </bpmn:dataInputAssociation>
+    </bpmn:task>
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Process_1">
+      <bpmndi:BPMNShape id="_BPMNShape_StartEvent_2" bpmnElement="StartEvent_1">
+        <dc:Bounds x="173" y="102" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="Task_A_di" bpmnElement="Task_A">
+        <dc:Bounds x="278" y="80" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <!-- off by 10px (x) -->
+      <bpmndi:BPMNEdge id="SequenceFlow_di" bpmnElement="SequenceFlow">
+        <di:waypoint xsi:type="dc:Point" x="209" y="120" />
+        <di:waypoint xsi:type="dc:Point" x="268" y="120" />
+        <bpmndi:BPMNLabel>
+          <dc:Bounds x="198.5" y="95" width="90" height="20" />
+        </bpmndi:BPMNLabel>
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNShape id="DataStoreReference_di" bpmnElement="DataStoreReference">
+        <dc:Bounds x="412" y="344" width="50" height="50" />
+        <bpmndi:BPMNLabel>
+          <dc:Bounds x="392" y="394" width="90" height="20" />
+        </bpmndi:BPMNLabel>
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="DataAssociation_1_di" bpmnElement="DataAssociation_1">
+        <di:waypoint xsi:type="dc:Point" x="426" y="344" />
+        <di:waypoint xsi:type="dc:Point" x="346" y="160" />
+      </bpmndi:BPMNEdge>
+      <bpmndi:BPMNShape id="Task_B_di" bpmnElement="Task_B">
+        <dc:Bounds x="584" y="329" width="100" height="80" />
+      </bpmndi:BPMNShape>
+      <!-- off by 10px (x) -->
+      <bpmndi:BPMNEdge id="DataAssociation_2_di" bpmnElement="DataAssociation_2">
+        <di:waypoint xsi:type="dc:Point" x="462" y="369" />
+        <di:waypoint xsi:type="dc:Point" x="574" y="369" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>

test/spec/features/modeling/behavior/ImportDockingFixSpec.js

@@ -0,0 +1,37 @@
+'use strict';
+
+require('../../../../TestHelper');
+
+/* global bootstrapModeler, inject */
+
+var modelingModule = require('../../../../../lib/features/modeling'),
+    coreModule = require('../../../../../lib/core');
+
+
+describe('features/modeling/behavior - ImportDockingFix', function() {
+
+  var diagramXML = require('./ImportDockingFix.bpmn');
+
+  beforeEach(bootstrapModeler(diagramXML, {
+    modules: [
+      coreModule,
+      modelingModule
+    ]
+  }));
+
+
+  it('should correct dockings on import', inject(function(elementRegistry, modeling) {
+
+    // when
+    var sequenceFlowConnection = elementRegistry.get('SequenceFlow'),
+        associationConnection = elementRegistry.get('DataAssociation_1');
+
+    // then
+    expect(sequenceFlowConnection).to.have.startDocking({ x: 191, y: 120 });
+    expect(sequenceFlowConnection).to.have.endDocking({ x: 328, y: 120 });
+
+    expect(associationConnection).to.have.startDocking({ x: 437, y: 369 });
+    expect(associationConnection).to.have.endDocking({ x: 328, y: 119 });
+  }));
+
+});

test/spec/features/modeling/layout/LayoutAssociationSpec.js

@@ -7,20 +7,26 @@ require('../../../../TestHelper');
 var modelingModule = require('../../../../../lib/features/modeling'),
     coreModule = require('../../../../../lib/core');
 
+
 describe('features/modeling - layout association', function() {
 
   var diagramXML = require('../../../../fixtures/bpmn/basic.bpmn');
 
-  var testModules = [ coreModule, modelingModule ];
+  beforeEach(bootstrapModeler(diagramXML, {
+    modules: [
+      coreModule,
+      modelingModule
+    ]
+  }));
+
 
   var rootShape;
 
-  beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
-
   beforeEach(inject(function(canvas) {
     rootShape = canvas.getRootElement();
   }));
 
+
   it('should layout straight after TextAnnotation creation', inject(function(elementRegistry, modeling) {
 
     // given

test/spec/features/modeling/layout/LayoutConnectionSpec.js

@@ -26,6 +26,8 @@ describe('features/modeling - layout connection', function() {
       var sequenceFlowConnection = elementRegistry.get('SequenceFlow_1'),
           sequenceFlow = sequenceFlowConnection.businessObject;
 
+      var expectedWaypoints = sequenceFlowConnection.waypoints;
+
       // when
       modeling.layoutConnection(sequenceFlowConnection);
 
@@ -34,17 +36,10 @@ describe('features/modeling - layout connection', function() {
       // expect cropped, repaired connection
       // that was not actually modified
 
-      var waypoints = [
+      expect(sequenceFlowConnection.waypoints).to.eql(expectedWaypoints);
-        { original: { x: 578, y: 341 }, x: 578, y: 341 },
-        { x: 934, y: 341 },
-        { x: 934, y: 436 },
-        { original: { x: 832, y: 436 }, x: 832, y: 436 }
-      ];
-
-      expect(sequenceFlowConnection.waypoints).eql(waypoints);
 
       // expect cropped waypoints in di
-      var diWaypoints = bpmnFactory.createDiWaypoints(waypoints);
+      var diWaypoints = bpmnFactory.createDiWaypoints(expectedWaypoints);
 
       expect(sequenceFlow.di.waypoint).eql(diWaypoints);
     }));

test/spec/features/modeling/layout/LayoutDataAssociationSpec.js

@@ -13,18 +13,23 @@ describe('features/modeling - layout data association', function() {
 
   var diagramXML = require('../../../../fixtures/bpmn/basic.bpmn');
 
-  var testModules = [ coreModule, modelingModule ];
+  beforeEach(bootstrapModeler(diagramXML, {
+    modules: [
+      coreModule,
+      modelingModule
+    ]
+  }));
+
 
   var rootShape,
       taskShape;
 
-  beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
-
   beforeEach(inject(function(elementRegistry, canvas) {
     rootShape = canvas.getRootElement();
     taskShape = elementRegistry.get('Task_1');
   }));
 
+
   it('should layout straight after DataObjectReference creation', inject(function(modeling) {
 
     // when

test/spec/features/modeling/layout/LayoutSequenceFlowSpec.js

@@ -161,11 +161,11 @@ describe('features/modeling - layout', function() {
         reconnectEnd(connection, 'ExclusiveGateway_1', newDocking);
 
         // then
-        expect(connection.waypoints).to.eql([
+        expect(connection).to.have.waypoints([
-          { original:{ x:382,y:241 },x:382,y:241 },
+          { x: 382, y: 241 },
-          { x:559,y:241 },
+          { x: 559, y: 241 },
-          { x:559,y:138 },
+          { x: 559, y: 138 },
-          { original:{ x:660,y:280 },x:660,y:280 }
+          { x: 660, y: 280 }
         ]);
       }));
 
@@ -180,10 +180,11 @@ describe('features/modeling - layout', function() {
         move('ServiceTask_1', delta);
 
         // then
-        expect(connection.waypoints).to.eql([{ original:{ x:382,y:241 },x:382,y:241 },
+        expect(connection).to.have.waypoints([
-          { x:559,y:241 },
+          { x: 382, y: 241 },
-          { x:559,y:158 },
+          { x: 559, y: 241 },
-          { original:{ x:598,y:158 },x:598,y:158 }
+          { x: 559, y: 158 },
+          { x: 598, y: 158 }
         ]);
       }));
 
@@ -198,11 +199,11 @@ describe('features/modeling - layout', function() {
         move('Task_1', delta);
 
         // then
-        expect(connection.waypoints).to.eql([
+        expect(connection).to.have.waypoints([
-          { original:{ x:352,y:261 },x:352,y:261 },
+          { x: 352, y: 261 },
-          { x:559,y:261 },
+          { x: 559, y: 261 },
-          { x:559,y:138 },
+          { x: 559, y: 138 },
-          { original:{ x:628,y:138 },x:628,y:138 }
+          { x: 628, y: 138 }
         ]);
       }));