From 597c417dce979b70497b5e339cce463e5fd5bdfc Mon Sep 17 00:00:00 2001 From: Martin Stamm Date: Wed, 25 Aug 2021 14:49:00 +0200 Subject: [PATCH] chore: throw error when accessing DI from business object Related to https://github.com/bpmn-io/bpmn-js/issues/1472 --- lib/features/modeling/ElementFactory.js | 6 +++++ lib/import/BpmnTreeWalker.js | 7 ++++++ lib/util/CompatibilityUtil.js | 22 ++++++++++++++++++- test/spec/ModelerSpec.js | 21 ++++++++++++++++++ test/spec/ViewerSpec.js | 21 ++++++++++++++++++ .../spec/features/modeling/AppendShapeSpec.js | 8 +++---- .../features/modeling/ElementFactorySpec.js | 15 +++++++++++++ 7 files changed, 95 insertions(+), 5 deletions(-) diff --git a/lib/features/modeling/ElementFactory.js b/lib/features/modeling/ElementFactory.js index 17abb051..0b1b209d 100644 --- a/lib/features/modeling/ElementFactory.js +++ b/lib/features/modeling/ElementFactory.js @@ -22,6 +22,10 @@ import { DEFAULT_LABEL_SIZE } from '../../util/LabelUtil'; +import { + ensureCompatDiRef +} from '../../util/CompatibilityUtil'; + /** * A bpmn-aware factory for diagram-js shapes @@ -72,6 +76,8 @@ ElementFactory.prototype.createBpmnElement = function(elementType, attrs) { } businessObject = this._bpmnFactory.create(attrs.type); + + ensureCompatDiRef(businessObject); } if (!di) { diff --git a/lib/import/BpmnTreeWalker.js b/lib/import/BpmnTreeWalker.js index 4bf25ddf..214d5191 100644 --- a/lib/import/BpmnTreeWalker.js +++ b/lib/import/BpmnTreeWalker.js @@ -8,6 +8,11 @@ import { elementToString } from './Util'; +import { + ensureCompatDiRef +} from '../util/CompatibilityUtil'; + + /** * Returns true if an element has the given meta-model type * @@ -113,6 +118,8 @@ export default function BpmnTreeWalker(handler, translate) { ); } else { diMap[bpmnElement.id] = di; + + ensureCompatDiRef(bpmnElement); } } else { logError( diff --git a/lib/util/CompatibilityUtil.js b/lib/util/CompatibilityUtil.js index cee525d5..0e427027 100644 --- a/lib/util/CompatibilityUtil.js +++ b/lib/util/CompatibilityUtil.js @@ -1,4 +1,7 @@ -import { isFunction } from 'min-dash'; +import { + has, + isFunction +} from 'min-dash'; // TODO(nikku): remove with future bpmn-js version @@ -51,3 +54,20 @@ export function wrapForCompatibility(api) { } }; } + + +// TODO(nikku): remove with future bpmn-js version + +var DI_ERROR_MESSAGE = 'Tried to access di from the businessObject. The di is available through the diagram element only. For more information, see https://github.com/bpmn-io/bpmn-js/issues/1472'; + +export function ensureCompatDiRef(businessObject) { + + // bpmnElement can have multiple independent DIs + if (!has(businessObject, 'di')) { + Object.defineProperty(businessObject, 'di', { + get: function() { + throw new Error(DI_ERROR_MESSAGE); + } + }); + } +} \ No newline at end of file diff --git a/test/spec/ModelerSpec.js b/test/spec/ModelerSpec.js index 77b54e9d..1e37880f 100644 --- a/test/spec/ModelerSpec.js +++ b/test/spec/ModelerSpec.js @@ -468,6 +468,27 @@ describe('Modeler', function() { }); + it('should error when accessing from businessObject', function() { + + var xml = require('../fixtures/bpmn/simple.bpmn'); + + var modeler = new Modeler({ container: container }); + + return modeler.importXML(xml).then(function() { + + // given + var elementRegistry = modeler.get('elementRegistry'), + shape = elementRegistry.get('Task_1'); + + // then + expect(shape.di).to.exist; + expect(function() { + shape.businessObject.di; + }).to.throw(/The di is available through the diagram element only./); + }); + }); + + it('should create new diagram', function() { var modeler = new Modeler({ container: container }); return modeler.createDiagram(); diff --git a/test/spec/ViewerSpec.js b/test/spec/ViewerSpec.js index bbfc19df..fa1a3c9c 100644 --- a/test/spec/ViewerSpec.js +++ b/test/spec/ViewerSpec.js @@ -287,6 +287,27 @@ describe('Viewer', function() { }); }); + + it('should error when accessing from businessObject', function() { + + var xml = require('../fixtures/bpmn/simple.bpmn'); + + return createViewer(container, Viewer, xml).then(function(result) { + + // given + var viewer = result.viewer, + elementRegistry = viewer.get('elementRegistry'), + shape = elementRegistry.get('Task_1'); + + // then + expect(shape.di).to.exist; + + expect(function() { + shape.businessObject.di; + }).to.throw(/The di is available through the diagram element only./); + }); + }); + }); diff --git a/test/spec/features/modeling/AppendShapeSpec.js b/test/spec/features/modeling/AppendShapeSpec.js index 6bb16ea2..5ad0498f 100644 --- a/test/spec/features/modeling/AppendShapeSpec.js +++ b/test/spec/features/modeling/AppendShapeSpec.js @@ -141,7 +141,7 @@ describe('features/modeling - append shape', function() { var connection = find(subProcess.get('flowElements'), function(e) { return e.sourceRef === startEvent && e.targetRef === target; }), - connectionDi = getDi(connection); + connectionDi = getDi(elementRegistry.get(connection.id)); // when @@ -174,7 +174,7 @@ describe('features/modeling - append shape', function() { var connection = find(subProcess.get('flowElements'), function(e) { return e.sourceRef === startEvent && e.targetRef === target; }), - connectionDi = getDi(connection); + connectionDi = getDi(elementRegistry.get(connection.id)); // when commandStack.undo(); @@ -209,7 +209,7 @@ describe('features/modeling - append shape', function() { var connection = find(subProcess.get('flowElements'), function(e) { return e.sourceRef === startEvent && e.targetRef === target; }), - connectionDi = getDi(connection); + connectionDi = getDi(elementRegistry.get(connection.id)); // when commandStack.undo(); @@ -270,7 +270,7 @@ describe('features/modeling - append shape', function() { var connection = find(subProcess.get('flowElements'), function(e) { return e.sourceRef === startEvent && e.targetRef === target; }), - connectionDi = getDi(connection); + connectionDi = getDi(elementRegistry.get(connection.id)); // when commandStack.undo(); diff --git a/test/spec/features/modeling/ElementFactorySpec.js b/test/spec/features/modeling/ElementFactorySpec.js index 5d2a9bbb..df29fff0 100644 --- a/test/spec/features/modeling/ElementFactorySpec.js +++ b/test/spec/features/modeling/ElementFactorySpec.js @@ -101,6 +101,21 @@ describe('features - element factory', function() { })); + it('should error when accessing via businessObject', inject(function(elementFactory) { + + // given + var shape = elementFactory.createShape({ + type: 'bpmn:Task', + }); + + // then + expect(shape.di).to.exist; + expect(function() { + shape.businessObject.di; + }).to.throw(/The di is available through the diagram element only./); + })); + + describe('integration', function() { it('should create event definition with ID', inject(function(elementFactory) {