[Pki-devel] PATCH 0029 - RESTful servlet to configure system in a single servlet

[Endi Sukma Dewata]

On 4/26/2012 2:49 PM, Ade Lee wrote:
> Please review:
>
>      New RESTful servlet that does system configuration in a single servlet.
>
>      Installation code common to the panels and the installation servlet are extracted to a
>      ConfigurationUtils file.  The panel code will be cleaned up to use the code in this
>      class in a later commit.
>
>      Contains restful client and test driver code.  The test driver code should be modified
>      and placed in a junit/system test framework.  Installation has been tested to work with
>      the following installations: master CA, clone CA, KRA, OCSP, TKS, subordinate CA, CA
>      signed by external CA (parts 1 and 2).
>
>      Ticket #155
>
> Thanks,
> Ade

Here's some initial feedback:

1. The ConfigurationErrorInterceptor is basically identical to the 
existing DRMErrorInterceptor. To avoid duplicating the code for each 
test components we can move the DRMErrorInterceptor from the 
base/kra/functional into the base/test and call it CMSErrorInterceptor 
(or something like that) which then can be used by all test components. 
Or we can put it into the common package so it can be used by any 
clients including the test tools. Ideally it should be put into a 
separate jar file that contains the client libraries only.

2. Classes such as ConfigurationTest and ConfigurationUtils are defined 
as containers of static functions. It might be possible to convert these 
static functions into object methods and the parameters can be passed as 
attributes, for example:

     public class ConfigurationTest {

         String host;

         public void parse(...) {
             host = cmd.getOptionValue("h");
         }

         public static void main(String args[]) {
             ConfigurationTest test = new ConfigurationTest();
             test.parse(args);
             ConfigurationData data = test.construct();
             ...
         }
     }

3. The ConfigurationUtils.getDomainXML() returns an XML string, but 
before it's used it's always parsed into DOM. It might be better to 
modify this method to return DOM directly. Is the XML string equivalent 
to DomainInfo class? If that's the case the method can also return a 
DomainInfo directly.

4. It might be possible to convert some fields in ConfigurationData such 
as dsPort or isClone into integer or boolean.

5. In InstallTokenRequest constructors the super() invocations aren't 
necessary.

6. The SystemConfigurationResourceService contains a static variable 
mRandom. The 'static' and the 'm' prefix don't seem to be necessary.

7. The auto-format doesn't seem to be working. There are some trailing 
whitespaces too. Eclipse should have fixed it automatically.

-- 
Endi S. Dewata