[Pki-devel] [PATCH] 53 Check the actual result when revoking/unrevoking a certificate in CLI. Ticket 217

Endi Sukma Dewata edewata at redhat.com
Mon Apr 22 19:41:43 UTC 2013


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.

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.

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.

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.

-- 
Endi S. Dewata




More information about the Pki-devel mailing list