[jboss-jira] [JBoss JIRA] Closed: (JBPM-988) JpdlXmlReader fails to readProcessDefinition if process contains nodes with same name inside different containers

JpdlXmlReader fails to readProcessDefinition if process contains nodes with same name inside different containers ----------------------------------------------------------------------------------------------------------------- Key: JBPM-988 URL: http://jira.jboss.com/jira/browse/JBPM-988 Project: JBoss jBPM Issue Type: Bug Components: Core Engine Affects Versions: jBPM jPDL 3.2 Reporter: Olga Ivanova Assigned To: Tom Baeyens Fix For: jBPM jPDL 3.2.2 Attachments: TestJpdlXmlReader.java JpdlXmlReader incorrectly handles nodes that belong to super state container. While reading node, it first calls node.setName and only after that adds node to corresponding node container. Method setName is not simply getter, it also updates corresponding container with new node name. For node that belongs to super state, container is not yet set, so node updates it's name inside process definition (which is default container for all nodes), if there are several nodes with same name that belongs to different containers and if between processing them there are no process definition (root) nodes, exception is thrown when setting name to second node, that such node already exists: java.lang.IllegalArgumentException: couldn't set name 's' on node 'State(c70f32)'cause the process definition of this node has already another node with the same name at org.jbpm.graph.def.Node.setName(Node.java:426) at org.jbpm.jpdl.xml.JpdlXmlReader.readNode(JpdlXmlReader.java:488) UnitTest attached. This problem could either be solved by changing JpdlXmlReader.readNode method, and putting nodeCollection.addNode at the beginning of the method: public void readNode(Element nodeElement, Node node, NodeCollection nodeCollection) { nodeCollection.addNode(node); // get the node name String name = nodeElement.attributeValue("name"); if (name != null) { node.setName(name); ..... Another solution is to change Node.setName method, so it will first check if node already exists in container node collection (hasNode(name)), and only in this case update it's name there: public void setName(String name) { if (isDifferent(this.name, name)) { String oldName = this.name; if (superState!=null && superState.hasNode(oldName)) { if ( superState.hasNode(name) ) { throw new IllegalArgumentException("couldn't set name '"+name+"' on node '"+this+"'cause the superState of this node has already another child node with the same name"); } Map nodes = superState.getNodesMap(); nodes.remove(oldName); nodes.put(name,this); } else if (processDefinition!=null && processDefinition.hasNode(oldName)) { if ( processDefinition.hasNode(name) ) { throw new IllegalArgumentException("couldn't set name '"+name+"' on node '"+this+"'cause the process definition of this node has already another node with the same name"); } Map nodeMap = processDefinition.getNodesMap(); nodeMap.remove(oldName); nodeMap.put(name,this); } this.name = name; } } I would vote for second solution, as is does not change any logic, it simply does not updates something that does not require update anyway.