[Pki-devel] [PATCH] 26 CLI - Cert Request Review and Cert Request Approve Implementation

[Endi Sukma Dewata]

On 7/24/2012 4:05 PM, Abhishek Koneru wrote:
> Please review the patch with implementations for review and approval of
> a cert request using CLI.

I have not been able to run it successfully since I'm still working on 
auth, but I have some comments below. I'll test this once I have a 
working environment.

1. The CertRequestApproveCLI.printHelp() generates a message like this:

     usage: cert -U <uri> -d <Certificate database directory>
     -w <password> -n <cert> request-approve <request id>

The command name is split into "cert ... request-approve". It's supposed 
to show "cert-request-approve". Also it's not necessary to show the -U, 
-d, -w, -n options because they are part of the MainCLI's options. You 
can view those options by executing "pki" without any parameters.

2. Same as #1 for CertRequestReviewCLI.printHelp().

3. In CAEnrollProfile.java there are some "Check Point <n>" debug 
messages. I'd say we should either replace them with messages that 
explain what the code is doing or just remove them.

4. In CertRequestResource.java the MediaType.TEXT_XML is actually not 
needed. We should use MediaType.APPLICATION_XML for XML content type.

5. The CertRequestResourceService.assignRequest() returns an empty 
CertRequestInfos. It doesn't seem to be necessary, but I haven't tested it.

6. In CertRequestResourceService.java:163 the original exception should 
be attached to the new exception as follows:

     throw new CMSException("Problem approving request in CertRequestRes
urce.assignRequest!", e);

7. In CertRequestResourceService.java:165 the debug message 
"RequestNotFound" below is not necessary because the toString() will 
include the exception name in the output. It should be just:

     CMS.debug(e);

-- 
Endi S. Dewata