You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tuscany.apache.org by "Mike Edwards (JIRA)" <de...@tuscany.apache.org> on 2009/04/26 17:15:31 UTC
[jira] Created: (TUSCANY-2989) Bogus Code in NodeImpl
configureNode(...)
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.
[jira] Closed: (TUSCANY-2989) Bogus Code in NodeImpl
configureNode(...)
Posted by "Mike Edwards (JIRA)" <de...@tuscany.apache.org>.
[ https://issues.apache.org/jira/browse/TUSCANY-2989?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mike Edwards closed TUSCANY-2989.
---------------------------------
Closing resolved issues
> 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.
[jira] Commented: (TUSCANY-2989) Bogus Code in NodeImpl
configureNode(...)
Posted by "ant elder (JIRA)" <de...@tuscany.apache.org>.
[ https://issues.apache.org/jira/browse/TUSCANY-2989?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12703008#action_12703008 ]
ant elder commented on TUSCANY-2989:
------------------------------------
See http://apache.markmail.org/message/jcscho3frvslpm44. Would be good to have the ASM_8001 testcase part of the build.
> 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.
[jira] Resolved: (TUSCANY-2989) Bogus Code in NodeImpl
configureNode(...)
Posted by "Mike Edwards (JIRA)" <de...@tuscany.apache.org>.
[ https://issues.apache.org/jira/browse/TUSCANY-2989?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Mike Edwards resolved TUSCANY-2989.
-----------------------------------
Resolution: Fixed
Resolved in 765417
> 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.