Bogus Code in NodeImpl configureNode(...)
-----------------------------------------
Key: TUSCANY-2989
URL: https://issues.apache.org/jira/browse/TUSCANY-2989
Project: Tuscany
Issue Type: Bug
Components: Java SCA Core Runtime
Affects Versions: Java-SCA-2.0
Reporter: Mike Edwards
Assignee: Mike Edwards
Fix For: Java-SCA-2.0
The NodeImpl configureNode(...) method contains some bogus code that fails when
a Node is configured to run a named Composite with multiple contributions. The
current code ends up resolving the given Composite multiple times, once per
contribution - even though it is present in only one contribution. The result
is a NullPointerException when an attempt is made to resolve the composite in a
contribution to which it does not belong.
The original testcase which revealed the problem is the OASIS SCA Assembly
testcase ASM_8001_Testcase, which uses a pair of contributions - "ASM_8001" and
"General" and runs the composite Test_ASM_8001.composite.
The bogus code in question is this section:
// Find the composite in the given contributions
boolean found = false;
Artifact compositeFile = contributionFactory.createArtifact();
compositeFile.setUnresolved(true);
compositeFile.setURI(composite.getURI());
for (Contribution contribution: workspace.getContributions()) {
ModelResolver resolver = contribution.getModelResolver();
// for (Artifact artifact : contribution.getArtifacts()){
// logger.log(Level.INFO,"artifact - " + artifact.getURI());
// }
Artifact resolvedArtifact = resolver.resolveModel(Artifact.class,
compositeFile);
// if (!resolvedArtifact.isUnresolved() &&
resolvedArtifact.getModel() instanceof Composite) {
if (!composite.isUnresolved()) {
// The composite content was passed into the node and read
into a composite model,
// don't use the composite found in the contribution, use
that composite, but just resolve
// it within the context of the contribution
compositeProcessor.resolve(composite, resolver);
} else {
// Use the resolved composite we've found in the
contribution
composite = (Composite)resolvedArtifact.getModel();
}
found = true;
// break;
// }
}
// if (!found) {
// throw new IllegalArgumentException("Composite not found: " +
composite.getURI());
// }
Note the amount of commenting out here. Looks as if this code was modified
without enough care.
As written, this code tries to resolve the composite in EVERY contribution
given to the node - notice that the for loop is over all contributions.
So, assume that the composite exists in the first contribution in the list.
First time through the for loop, resolvedArtifact gets set to the resolved
artifact for the composite that we're after - and given that the original
composite we were trying to find was unresolved, we take the then clause of the
if statement and end up setting the composite field to the resolved model from
the resolvedArtifact. (This is correct and is what we want)
The second time through the loop, the composite is already resolved and we go
to the first part of the if statement, where we attempt to resolve the
composite in the context of the second contribution. Note the lack of a check
on the resolvedArtifact from the resolver.resolveModel() method - this would
have told us that the composite can't be resolved in this contribution. When
compositeProcessor.resolve(...) is called for an artifact that isn't in the
contribution at all it fails, with a null pointer exception.
So:
1) The loop should stop as soon as the composite is found in a contribution
2) There should be a check to see if the returned resolvedArtifact was actually
found in the contribution - if not, there is nothing to do.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
