fix(modeling): unclaim ID on shape/connection delete

* unclaim ID of deleted shapes and connections
* unclaim ID of referenced processes when deleting participant
* this does NOT ensure we unclaim IDs of elements like DI, events, errors, ...

Related to camunda/camunda-modeler#1503
Related to #608

lib/features/modeling/behavior/UnclaimIdBehavior.js

@@ -1,33 +1,56 @@
-import {
-  forEach
-} from 'min-dash';
-
 import inherits from 'inherits';
 
 import CommandInterceptor from 'diagram-js/lib/command/CommandInterceptor';
 
+import { is } from '../../../util/ModelUtil';
+import { isExpanded } from '../../../util/DiUtil';
+import { isLabel } from '../../../util/LabelUtil';
+
 
 /**
  * Unclaims model IDs on element deletion.
  *
- * @param {EventBus} eventBus
+ * @param {Canvas} canvas
+ * @param {Injector} injector
+ * @param {Moddle} moddle
  * @param {Modeling} modeling
  */
-export default function UnclaimIdBehavior(eventBus, modeling) {
+export default function UnclaimIdBehavior(canvas, injector, moddle, modeling) {
+  injector.invoke(CommandInterceptor, this);
 
-  CommandInterceptor.call(this, eventBus);
-
-  this.preExecute('elements.delete', function(event) {
+  this.preExecute('shape.delete', function(event) {
     var context = event.context,
-        elements = context.elements;
+        shape = context.shape,
+        shapeBo = shape.businessObject;
 
-    forEach(elements, function(element) {
-      modeling.unclaimId(element.businessObject.id, element.businessObject);
-    });
+    if (isLabel(shape)) {
+      return;
+    }
 
+    if (is(shape, 'bpmn:Participant') && isExpanded(shape)) {
+      moddle.ids.unclaim(shapeBo.processRef.id);
+    }
+
+    modeling.unclaimId(shapeBo.id, shapeBo);
+  });
+
+
+  this.preExecute('connection.delete', function(event) {
+    var context = event.context,
+        connection = context.connection,
+        connectionBo = connection.businessObject;
+
+    modeling.unclaimId(connectionBo.id, connectionBo);
+  });
+
+  this.preExecute('canvas.updateRoot', function() {
+    var rootElement = canvas.getRootElement(),
+        rootElementBo = rootElement.businessObject;
+
+    moddle.ids.unclaim(rootElementBo.id);
   });
 }
 
 inherits(UnclaimIdBehavior, CommandInterceptor);
 
-UnclaimIdBehavior.$inject = [ 'eventBus', 'modeling' ];
+UnclaimIdBehavior.$inject = [ 'canvas', 'injector', 'moddle', 'modeling' ];

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

@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<bpmn:definitions 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_1fc1ugd" targetNamespace="http://bpmn.io/schema/bpmn" exporter="Camunda Modeler" exporterVersion="3.3.5">
+  <bpmn:collaboration id="Collaboration_1">
+    <bpmn:participant id="Participant_1" processRef="Process_1" />
+  </bpmn:collaboration>
+  <bpmn:process id="Process_1" isExecutable="true">
+    <bpmn:startEvent id="StartEvent_1">
+      <bpmn:outgoing>SequenceFlow_1</bpmn:outgoing>
+    </bpmn:startEvent>
+    <bpmn:endEvent id="EndEvent_1">
+      <bpmn:incoming>SequenceFlow_1</bpmn:incoming>
+    </bpmn:endEvent>
+    <bpmn:sequenceFlow id="SequenceFlow_1" sourceRef="StartEvent_1" targetRef="EndEvent_1" />
+  </bpmn:process>
+  <bpmndi:BPMNDiagram id="BPMNDiagram_1">
+    <bpmndi:BPMNPlane id="BPMNPlane_1" bpmnElement="Collaboration_1">
+      <bpmndi:BPMNShape id="Participant_0bwhkbn_di" bpmnElement="Participant_1" isHorizontal="true">
+        <dc:Bounds x="-200" y="-25" width="600" height="250" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="StartEvent_1l3vzgi_di" bpmnElement="StartEvent_1">
+        <dc:Bounds x="-98" y="82" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNShape id="EndEvent_1nvhsm6_di" bpmnElement="EndEvent_1">
+        <dc:Bounds x="-8" y="82" width="36" height="36" />
+      </bpmndi:BPMNShape>
+      <bpmndi:BPMNEdge id="SequenceFlow_12bzkef_di" bpmnElement="SequenceFlow_1">
+        <di:waypoint x="-62" y="100" />
+        <di:waypoint x="-8" y="100" />
+      </bpmndi:BPMNEdge>
+    </bpmndi:BPMNPlane>
+  </bpmndi:BPMNDiagram>
+</bpmn:definitions>

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

@@ -0,0 +1,85 @@
+import {
+  bootstrapModeler,
+  inject
+} from 'test/TestHelper';
+
+import coreModule from 'lib/core';
+import modelingModule from 'lib/features/modeling';
+
+
+describe('features/modeling - unclaim id', function() {
+
+  var testModules = [ coreModule, modelingModule ];
+
+  var diagramXML = require('./UnclaimIdBehaviorSpec.bpmn');
+
+  beforeEach(bootstrapModeler(diagramXML, { modules: testModules }));
+
+
+  it('should unclaim ID of shape', inject(function(elementRegistry, moddle, modeling) {
+
+    // given
+    var startEvent = elementRegistry.get('StartEvent_1');
+
+    // when
+    modeling.removeElements([ startEvent ]);
+
+    // then
+    expect(moddle.ids.assigned('StartEvent_1')).to.be.false;
+  }));
+
+
+  it('should unclaim ID of process', inject(function(elementRegistry, moddle, modeling) {
+
+    // given
+    var participant = elementRegistry.get('Participant_1');
+
+    // when
+    modeling.removeElements([ participant ]);
+
+    // then
+    expect(moddle.ids.assigned('Process_1')).to.be.false;
+  }));
+
+
+  it('should unclaim ID of connection', inject(function(elementRegistry, moddle, modeling) {
+
+    // given
+    var sequenceFlow = elementRegistry.get('SequenceFlow_1');
+
+    // when
+    modeling.removeElements([ sequenceFlow ]);
+
+    // then
+    expect(moddle.ids.assigned('SequenceFlow_1')).to.be.false;
+  }));
+
+
+  it('should unclaim ID of children', inject(function(elementRegistry, moddle, modeling) {
+
+    // given
+    var participant = elementRegistry.get('Participant_1');
+
+    // when
+    modeling.removeElements([ participant ]);
+
+    // then
+    expect(moddle.ids.assigned('StartEvent_1')).to.be.false;
+    expect(moddle.ids.assigned('SequenceFlow_1')).to.be.false;
+    expect(moddle.ids.assigned('EndEvent_1')).to.be.false;
+  }));
+
+
+  it('should unclaim ID of root', inject(function(elementRegistry, moddle, modeling) {
+
+    // given
+    var participant = elementRegistry.get('Participant_1');
+
+    // when
+    modeling.removeElements([ participant ]);
+
+    // then
+    expect(moddle.ids.assigned('Collaboration_1')).to.be.false;
+  }));
+
+});