[Pki-devel] [PATCH] 90-2 Addressed fixes for comments on patch 90.

[Endi Sukma Dewata]

On 4/16/2014 12:43 PM, Abhishek Koneru wrote:
> Please review patch 90-2 with fixes for all comments given by Ade and
> Endi.
> Patch application order - 87,89,90,90-2

Some more comments:

1. The templates are still hardcoded in KeyTemplateFindCLI, i.e. we 
can't add a new template without changing the code. See the following code:

   switch (id) {
   case "archiveKey":
       data = ResourceMessage.unmarshall(KeyArchivalRequest.class,
           templateDir + templateName);
       template = new KeyTemplate(id, "Key archival request",
           data.getAttribute("description"));
       templates.add(template);
       break;
   case "retrieveKey":
       data = ResourceMessage.unmarshall(KeyRecoveryRequest.class,
           templateDir + templateName);
       template = new KeyTemplate(id, "Key retrieval request",
           data.getAttribute("description"));
       templates.add(template);
       break;

Notice that the above cases are identical except for the class name and 
the template name. I think we can use ResourceMessage for the class name 
since we're only using it to retrieve the attributes. For the template 
name we can put that into the template as well, or we can just remove 
the name since we already have ID and the description.

So the resulting code would be (no switch required):

       data = ResourceMessage.unmarshall(ResourceMessage.class,
           templateDir + templateName);
       template = new KeyTemplate(id, data.getAttribute("description"));
       templates.add(template);

2. Similar issue with KeyTemplateShowCLI.

3. There's an extra semicolon in KeyTemplateShowCLI.java:47.

4. In KeyTemplateShowCLI you could use try-with-resource so you don't 
need to close the stream explicitly.

   try (FileOutputStream fOS = new FileOutputStream(writeToFile)) {
       ...
   } catch (IOException e) {
       ...
   }

5. Please run source formatting on KeyClient and KeyGenerateCLI.

Once they are fixed, it's ACKed. Feel free to push. Thanks.

-- 
Endi S. Dewata