From ceb67600b931f11c83c60094f72e1b7f970b5538 Mon Sep 17 00:00:00 2001 From: meyerd Date: Thu, 16 Feb 2012 14:54:36 +0000 Subject: [PATCH] ACT-1099 Parsing results in exception when using text annotation connected with association --- .../engine/impl/bpmn/parser/BpmnParse.java | 27 +++++++--- .../engine/impl/util/xml/Element.java | 10 ++++ .../engine/test/bpmn/parse/BpmnParseTest.java | 5 ++ ...ElementsForUnknownModelElements.bpmn20.xml | 49 +++++++++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 modules/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/parse/BpmnParseTest.testParseDiagramInterchangeElementsForUnknownModelElements.bpmn20.xml diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/BpmnParse.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/BpmnParse.java index 2e936e2f29..6bc1d327e3 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/BpmnParse.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/bpmn/parser/BpmnParse.java @@ -151,6 +151,10 @@ public class BpmnParse extends Parse { /** A map for storing sequence flow based on their id during parsing. */ protected Map sequenceFlows; + + /** A list of all element IDs. This allows us to parse only what we actually support but + * still validate the references among elements we do not support. */ + protected List elementIds = new ArrayList(); /** * Mapping containing values stored during the first phase of parsing since @@ -225,6 +229,7 @@ public class BpmnParse extends Parse { * Parses the 'definitions' root element */ protected void parseRootElement() { + collectElementIds(); parseDefinitionsAttributes(); parseImports(); parseItemDefinitions(); @@ -243,6 +248,10 @@ public class BpmnParse extends Parse { } } + protected void collectElementIds() { + rootElement.collectIds(elementIds); + } + protected void parseDefinitionsAttributes() { String typeLanguage = rootElement.attribute("typeLanguage"); String expressionLanguage = rootElement.attribute("expressionLanguage"); @@ -598,7 +607,14 @@ public class BpmnParse extends Parse { ActivityImpl sourceActivity = parentScope.findActivity(sourceRef); ActivityImpl targetActivity = parentScope.findActivity(targetRef); - if(sourceActivity != null && targetActivity != null) { + // an association may reference elements that are not parsed as activities (like for instance + // test annotations so do not throw an exception if source sourceActivity or targetActivity are null) + // However, we make sure they reference 'something': + if(sourceActivity == null && !elementIds.contains(sourceRef)) { + addError("Invalid reference sourceRef '"+sourceRef+"' of association element ", associationElement); + } else if(targetRef == null && !elementIds.contains(targetRef)) { + addError("Invalid reference targetRef '"+targetRef+"' of association element ", associationElement); + } else { if(sourceActivity.getProperty("type").equals("compensationBoundaryCatch")) { Object isForCompensation = targetActivity.getProperty(PROPERTYNAME_IS_FOR_COMPENSATION); if(isForCompensation == null || !(Boolean) isForCompensation) { @@ -608,12 +624,7 @@ public class BpmnParse extends Parse { compensatedActivity.setProperty(PROPERTYNAME_COMPENSATION_HANDLER_ID, targetActivity.getId()); } } - }else if(sourceActivity == null) { - addError("invalid sourceRef '"+sourceRef+"' in association", associationElement); - }else if(targetActivity == null) { - addError("invalid targetRef '"+targetRef+"' in association", associationElement); } - } } @@ -2773,7 +2784,7 @@ public class BpmnParse extends Parse { if (isExpanded != null) { activity.setProperty(PROPERTYNAME_ISEXPANDED, parseBooleanAttribute(isExpanded)); } - } else { + } else if(!elementIds.contains(activityId)) { // it might not be an activity but it might still reference 'something' addError("Invalid reference in 'bpmnElement' attribute, activity " + activityId + "not found", bpmnShapeElement); } } else { @@ -2797,7 +2808,7 @@ public class BpmnParse extends Parse { } else { addError("Minimum 2 waypoint elements must be definted for a 'BPMNEdge'", bpmnEdgeElement); } - } else { + } else if(!elementIds.contains(sequenceFlowId)) { // it might not be a sequenceFlow but it might still reference 'something' addError("Invalid reference in 'bpmnElement' attribute, sequenceFlow " + sequenceFlowId + "not found", bpmnEdgeElement); } } else { diff --git a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/util/xml/Element.java b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/util/xml/Element.java index a9ea054fba..ae8e9190c2 100644 --- a/modules/activiti-engine/src/main/java/org/activiti/engine/impl/util/xml/Element.java +++ b/modules/activiti-engine/src/main/java/org/activiti/engine/impl/util/xml/Element.java @@ -169,4 +169,14 @@ public class Element { public String getText() { return text.toString(); } + + /** + * allows to recursively collect the ids of all elements in the tree. + */ + public void collectIds(List ids) { + ids.add(attribute("id")); + for (Element child : elements) { + child.collectIds(ids); + } + } } diff --git a/modules/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/parse/BpmnParseTest.java b/modules/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/parse/BpmnParseTest.java index 4a99d2c6c1..549b4a1970 100644 --- a/modules/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/parse/BpmnParseTest.java +++ b/modules/activiti-engine/src/test/java/org/activiti/engine/test/bpmn/parse/BpmnParseTest.java @@ -124,6 +124,11 @@ public class BpmnParseTest extends PluggableActivitiTestCase { } } + @Deployment + public void testParseDiagramInterchangeElementsForUnknownModelElements() { + + } + protected void assertActivityBounds(ActivityImpl activity, int x, int y, int width, int height) { assertEquals(x, activity.getX()); assertEquals(y, activity.getY()); diff --git a/modules/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/parse/BpmnParseTest.testParseDiagramInterchangeElementsForUnknownModelElements.bpmn20.xml b/modules/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/parse/BpmnParseTest.testParseDiagramInterchangeElementsForUnknownModelElements.bpmn20.xml new file mode 100644 index 0000000000..0f899cac5b --- /dev/null +++ b/modules/activiti-engine/src/test/resources/org/activiti/engine/test/bpmn/parse/BpmnParseTest.testParseDiagramInterchangeElementsForUnknownModelElements.bpmn20.xml @@ -0,0 +1,49 @@ + + + + + sequenceFlow_14 + + + sequenceFlow_14 + sequenceFlow_15 + + + sequenceFlow_15 + + + + + + Some Text + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file -- GitLab