Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CAM-10771 Improve BPMN Parser Exceptions #333

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1337,7 +1337,7 @@ protected void validateOutgoingFlows(ActivityImpl activity) {
for (PvmTransition transition : activity.getOutgoingTransitions()) {
if (transition.getId() == null) {
addError("Sequence flow with sourceRef='" + activity.getId() + "' must have an id, activity with id '" + activity.getId() + "' uses 'asyncAfter'.",
null);
activity.getId(), activity.getId() );
}
}
}
Expand All @@ -1347,13 +1347,13 @@ public void validateExclusiveGateway(ActivityImpl activity) {
if (activity.getOutgoingTransitions().size() == 0) {
// TODO: double check if this is valid (I think in Activiti yes, since we
// need start events we will need an end event as well)
addError("Exclusive Gateway '" + activity.getId() + "' has no outgoing sequence flows.", null);
addError("Exclusive Gateway '" + activity.getId() + "' has no outgoing sequence flows.", activity.getId());
} else if (activity.getOutgoingTransitions().size() == 1) {
PvmTransition flow = activity.getOutgoingTransitions().get(0);
Condition condition = (Condition) flow.getProperty(BpmnParse.PROPERTYNAME_CONDITION);
if (condition != null) {
addError("Exclusive Gateway '" + activity.getId() + "' has only one outgoing sequence flow ('" + flow.getId()
+ "'). This is not allowed to have a condition.", null);
+ "'). This is not allowed to have a condition.", activity.getId() , flow.getId());
}
} else {
String defaultSequenceFlow = (String) activity.getProperty("default");
Expand All @@ -1370,7 +1370,7 @@ public void validateExclusiveGateway(ActivityImpl activity) {
}
if (hasConditon && isDefaultFlow) {
addError("Exclusive Gateway '" + activity.getId() + "' has outgoing sequence flow '" + flow.getId()
+ "' which is the default flow but has a condition too.", null);
+ "' which is the default flow but has a condition too.", activity.getId() , flow.getId());
}
}
if (hasDefaultFlow || flowsWithoutCondition.size() > 1) {
Expand All @@ -1380,7 +1380,7 @@ public void validateExclusiveGateway(ActivityImpl activity) {
for (PvmTransition flow : flowsWithoutCondition) {
addError(
"Exclusive Gateway '" + activity.getId() + "' has outgoing sequence flow '" + flow.getId() + "' without condition which is not the default flow.",
null);
activity.getId() , flow.getId());
}
} else if (flowsWithoutCondition.size() == 1) {
// Havinf no default and exactly one flow without condition this is
Expand All @@ -1389,7 +1389,7 @@ public void validateExclusiveGateway(ActivityImpl activity) {
addWarning(
"Exclusive Gateway '" + activity.getId() + "' has outgoing sequence flow '" + flow.getId()
+ "' without condition which is not the default flow. We assume it to be the default flow, but it is bad modeling practice, better set the default flow in your gateway.",
null);
activity.getId() , flow.getId());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@

/**
* @author Tom Baeyens
* @author Falko Menge
*/
public class Parse extends DefaultHandler {

Expand Down Expand Up @@ -170,6 +171,10 @@ public void addError(String errorMessage, Element element) {
errors.add(new Problem(errorMessage, name, element));
}

public void addError(String errorMessage, String... elementIds) {
errors.add(new Problem(errorMessage, name, elementIds));
}

public void addError(BpmnParseException e) {
errors.add(new Problem(e, name));
}
Expand All @@ -186,6 +191,10 @@ public void addWarning(String errorMessage, Element element) {
warnings.add(new Problem(errorMessage, name, element));
}

public void addWarning(String errorMessage, String... elementIds) {
warnings.add(new Problem(errorMessage, name, elementIds));
}

public boolean hasWarnings() {
return warnings != null && !warnings.isEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,27 @@
*/
package org.camunda.bpm.engine.impl.util.xml;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Set;

import org.camunda.bpm.engine.BpmnParseException;
import org.xml.sax.SAXParseException;


/**
* @author Tom Baeyens
* @author Joram Barrez
* @author Falko Menge
*/
public class Problem {

protected String errorMessage;
protected String resource;
protected int line;
protected int column;
private Set<String> elementIds = new HashSet<String>();

public Problem(SAXParseException e, String resource) {
concatenateErrorMessages(e);
Expand All @@ -44,6 +51,18 @@ public Problem(String errorMessage, String resourceName, Element element) {
if (element!=null) {
this.line = element.getLine();
this.column = element.getColumn();
String id = element.attribute("id");
if (id != null && id.length() > 0) {
this.elementIds.add(id);
}
}
}

public Problem(String errorMessage, String resourceName, String[] elementIds) {
this.errorMessage = errorMessage;
this.resource = resourceName;
if (elementIds != null) {
this.elementIds.addAll(new HashSet<String>(Arrays.asList(elementIds)));
}
}

Expand All @@ -70,6 +89,29 @@ protected void concatenateErrorMessages(Throwable throwable) {
}

public String toString() {
return errorMessage+(resource!=null ? " | "+resource : "")+" | line "+line+" | column "+column;
StringBuilder string = new StringBuilder(errorMessage);
if (resource != null) {
string.append(" | " + resource);
}
if (line > 0) {
string.append(" | line " + line);
}
if (column > 0) {
string.append(" | column " + column);
}
if (elementIds.size() > 0) {
string.append(" | element");
if (elementIds.size() > 1) {
string.append('s');
}
string.append(' ');
for (Iterator iterator = elementIds.iterator(); iterator.hasNext();) {
string.append(iterator.next());
if (iterator.hasNext()) {
string.append(',');
}
}
}
return string.toString();
}
}