Re: [Pki-devel] PATCH 0029 - RESTful servlet to configure system in a single servlet
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