Bug 352525

Comment 1 Tobias Liefke 2011-07-25 17:53:46 EDT

There are four problems, which lead to this behaviour:

1. As result of #120110 the BPELActivityDeserializer.unmarshall is called two times ("run the deserializer again so it can resolve references to objects that were not fully realized in pass 1"). I think, a deserializer should only be called twice if it signals that it needs a second pass.
Example:

class ... implements BPELActivityDeserializer {
  public Activity unmarshall(... BPELReader bpelReader) {
    ...
    bpelReader.add2ndPass(this);
  }
}

2. In ExtensionSampleActivityDeserializer (line 152) the child activity of the structured extension activity is always created, no matter if the extension activity has already the correct child activity. bpelReader.xml2Activity should should only get called if the activityElement is not the one from the current child activity:
  activityElement = (Element) childElements.item(i);
  if (sa.getActivity() == null 
      || sa.getActivity().getElement() != activityElement) {
    Activity childActivity = bpelReader.xml2Activity(activityElement);
    if (childActivity != null) {
      sa.setActivity(childActivity);
    }
  }

3. The WST Implementation of the DOM model(org.eclipse.wst.xml.core.internal.document.NodeContainer) doesn't check in its replaceChild method, if the old child is the same as the new child:
public Node replaceChild(Node newChild, Node oldChild) throws DOMException {
  ...
  if (oldChild == null)
    return newChild;
  if (newChild != null)
    insertBefore(newChild, oldChild);
  return removeChild(oldChild);
}

The result is, that the child is removed if the new child is the same as the old one. For a quick fix we should check that in ReconcilationHelper.replaceChild (line 270):
  if (newElement.getElement() != oldElement.getElement()) {
    parseElement.replaceChild(newElement.getElement(), oldElement.getElement());
  }

But we should really create a bug report for WST, too.

4. After the element is removed from its parent, the WSDLElement recieves the notification of the "setActivity" and tries to adapt the content. As getAdoptionParentNode(EReference) returns "getElement()" even for the ExtensionActivityImpl, it adds the "new" element to that parent element. From my point of view, ExtensionActivityImpl should overwrite getAdoptionParentNode and return the first child element.

Every single solution of these problems will solve the original problem, but I think we should fix all of them, as every single one can lead to other problems as well.

Comment 2 Tobias Liefke 2011-07-29 09:38:28 EDT

Created attachment 200593 [details]
Proposed patch

I submitted a bug report for the 3rd problem to the WTP team (#353407) and attached the workaround here. I didn't set up GIT for my IDE up to now, so it may take a while until it is added to the repository.

We should really discuss my solutions for the three other problems as well.

Comment 3 Tobias Liefke 2011-07-29 09:40:26 EDT

Decreased importance to "normal" - because a solution exists.