[Pki-devel] [PATCH] 53-2 Fixes for comments for patch 53

[Abhishek Koneru]

Please review the patch with fixes for review comments on patch 53.

On Mon, 2013-04-22 at 14:41 -0500, Endi Sukma Dewata wrote:
> On 4/19/2013 1:42 PM, Abhishek Koneru wrote:
> > Please review the patch for trac ticket 217.
> >
> > Added an additional check over the actual result of a revoke/unrevoke
> > operation.
> 
> Some comments:
> 
> 1. In CertRequestInfo the synchronized keywords don't seem to be 
> necessary. CertRequestInfo objects are created to pass REST operation 
> results to the client. They don't seem to be used by multiple threads.
> 
--Removed the synchronized keywords
> 2. In CertRequestInfoFactory the operationResult uses 'pass/fail' values 
> which are duplicated in several places. It might be better to use 
> 'success/error' to match RES_SUCCESS and RES_ERROR constants, and these 
> values can be put in constant variables in CertRequestInfo:
> 
>      public static final String REQ_SUCCESS = "success";
>      public static final String REQ_ERROR = "error";
> 
> This way we can be sure the operationResult will be used consistently.
> 
-- Added the constant variables in CertRequestInfo
> 3. In CertRequestInfoFactory the code for operationResult assignment 
> seems to be incorrect because it will assign 'fail' to completed 
> requests without error. Try cert-request-find, it will show all requests 
> for system certs as failed.
> 
-- Corrected. Null value for result is given as a SUCCESS
> 4. In CertCLI if the request status is COMPLETE but the operation result 
> is 'fail' the status will appear as REJECTED. This could be confusing 
> because the request is not actually rejected. I think we should display 
> the operation result separately from the status.
> 
-- Printing the operation result seperately.

--Abhishek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pki-akoneru-0053-2-Check-the-actual-result-of-operations-cert-revoke-un.patch
Type: text/x-patch
Size: 11272 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20130423/cd05916a/attachment.bin>