> On Jun 3, 2020, at 10:14 PM, sha.ji...@oracle.com wrote:
>
> Hi Max,
>
> On 2020/6/3 17:17, Weijun Wang wrote:
>> TimeStampCheck.java:
>>
>> - Can you please inline all those private getXyz() calls in Interceptor into
>> getRespParam()? I would like to see all these customizations in one place.
>> Just add a blank line in between and it will be clean enough for me.
> OK
>
>>
>> TsaParam.java:
>>
>> - Please group the fields into different area. Looks like some are fields in
>> TSTInfo, and some are server internals.
> One usage of this class is carrying all parameters delivered by HTTP
> request. Do you mean they should be grouped in different classes?
No. Just add some comment lines inside the class and separate them into
different groups.
>
>>
>> The overall design is good, but I have a feeling that the interface and
>> implementation can be further separated. I can see that there are a lot of
>> methods and classes that will not be touched by TimeStampCheck at all.
>> Should these things be taken out to be another implementation which is
>> parallel to the one in TimeStampCheck? Maybe TsaHandler::createSigner will
>> be abstract.
> All RespInterceptor methods are used by TimeStampCheck.
> DefaultRespInterceptor is used by standalone TSA server. In this style, no
> more RespInterceptor implementation is needed.
See below.
>
>>
>> There are quite some public constructors in the classes. Are they meant to
>> be called by a user? Or by a child class? Or just internal?
> The public constructors in TsaServer can be called by applications for
> making standalone servers.
How about the other classes?
>
>>
>> Also, for those protected methods (Ex: parseRequestParam and createResponse
>> in TsaSigner), do you meant to override it in the future?
> Possibly.
> For example, a sub parseRequestParam() may need to parse extensions from
> request. A test may directly twist the byte[] returned by createResponse(),
> or even an overridden createResponse() just returns an empty byte[].
Can we make them internal at the moment until someone really need to extend it?
>
>>
>> Can you give examples on how to use this server? Will it always use
>> DefaultRespInterceptor? If so, will the default impl in RespInterceptor.java
>> ever be used?
> I'll use this server as standalone-like service. The custom TSA fields
> just be delivered by HTTP request query string. I won't provide a custom
> RespInterceptor, and DefaultRespInterceptor is enough to me.
You can put DefaultRespInterceptor in the standalone server.
If you change TsaSigner.java
80 public TsaSigner(SignerEntry signerEntry, byte[] requestData,
81 TsaParam param) {
82 this(signerEntry, requestData,
83 new DefaultRespInterceptor<TsaParam>(param));
to use the even more "default" RespInterceptor, i.e. new RespInterceptor(){},
is it still useable?
Thanks,
Max
>
> Best regards,
> John Jiang
>
