Bug 536186 (RHQ-563) - DeployPackagesResponse should let me change results
Summary: DeployPackagesResponse should let me change results
Keywords:
Status: CLOSED NEXTRELEASE
Alias: RHQ-563
Product: RHQ Project
Classification: Other
Component: Content
Version: 1.0
Hardware: All
OS: All
medium
medium
Target Milestone: ---
: ---
Assignee: Jason Dobies
QA Contact:
URL: http://jira.rhq-project.org/browse/RH...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-06-11 04:40 UTC by John Mazzitelli
Modified: 2008-06-11 13:45 UTC (History)
0 users

Fixed In Version: 1.0.1
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed:
Embargoed:


Attachments (Terms of Use)

Description John Mazzitelli 2008-06-11 04:40:00 UTC
DeployPackagesResponse takes a result in the constructor. But there is no setter.  So I can't create one initally, set the result to SUCCESS and later if I determine an error occurred, I can't flip it to FAILURE.  There *are* setters to set the error message - so it would make sense to let me change the results. It would make using the response object easier. Here's how I think you shoudl be able to use this object - notice I build it up as I go along iteratng thru the packages to be deployed:

DeployPackagesResponse r = new DeployPackagesResponse(key, SUCCESS);
for each package to deploy()
{
    try {
            ... deploy package....
           r.addPackageResponse(...);
    }
    catch (Exception e) {
       r.setErrorMessageWithThrowable(e);
        r.setResult(FAILURE); // see below- shoudln't really need to do this anyway)
    }
}

another alternative is to have the setErrorMessage methods implicitly change the result to FAILURE. In fact, it should do this anyway.  If I create a response with SUCCESS but later set an error message, it doesn't make sense that the result remain SUCCESS.  Setting an error message should implicitly assume the result was a FAILURE. If it works like this, I don't see a setter to set the results (though it might still make the api nicer).

I actually don't see how you easily work with this response object as it is now.  You can't set a bulk collection of individual package responses (i.e. this is missing an API like "addAllPackageResponses()", so how do you easily build up a DeployPackagesResponse without forcing to know what the end result is before hand (which you need because you can only set the result via the constructor).

Comment 1 John Mazzitelli 2008-06-11 04:48:18 UTC
ugh - same problem with DeployIndividualPackageResponse.

This API is much harder to work with than it should be.

Need to do one or both of:

1) provide a setContentResponseResult() method
2) have setErrorMessage methods implicitly set the result to FAILURE


Comment 2 John Mazzitelli 2008-06-11 04:58:13 UTC
same problems with RemovePackageResponse/RemoveIndividualPackageResponse

Comment 3 Jason Dobies 2008-06-11 13:45:11 UTC
r960 - DeployPackagesResponse.java, DeployIndividualPackageResponse.java, RemoveIndividualPackageResponse.java, RemovePackagesResponse.java
- Loosened up result code setting restrictions by adding setter and constructor that doesn't require it.

Comment 4 Red Hat Bugzilla 2009-11-10 21:11:59 UTC
This bug was previously known as http://jira.rhq-project.org/browse/RHQ-563



Note You need to log in before you can comment on or make changes to this bug.