[Pki-devel] [PATCH] 11 Added generics (part 1).

[Endi Sukma Dewata]

New patch attached (#11-2).

On 1/16/2012 4:26 PM, John Magne wrote:
>> The following construct below originally looked odd, but upon further review online,
>> it appears that this is a generic method where the "<V>" at the front denotes a type.
>> This is legitimate but it looks like this getExtDataInHashtable is used extensively, it
>> would be nice to know if we've done some simple testing to see if this works as expected.
>>
>> -    public Hashtable getExtDataInHashtable(String key);
>> +    public<V>   Hashtable<String, V>   getExtDataInHashtable(String key);
>
> As suggested by Adam last week, I replaced this with:
>
>     public Hashtable<String, Object>  getExtDataInHashtable(String key);
>
> OK, looks good, although is there a chance that the previous "V" could be anything, but where "Object" limits us to things based on "Object"? I suppose Eclipse would have complained if this was an issue.

I think originally the Hashtable may contain String or Object values. 
But after closer inspection, it looks like the Hashtable only accepts 
store String values. So I'm changing the code again to:

   public Hashtable<String, String> getExtDataInHashtable(String key);

Some validation codes become redundant because of this, we can clean it 
up in a separate patch.

> Also, I grepped around and found the following in RequestTest.java
>
> Hashtable<String, ?>  out = request.getExtDataInHashtable("topkey");
>
> Should that have triggered a warning or something?

The wildcards (?) are now changed to String.

>> Otherwise I think that it comes down to how much simple testing have we done to make
>> sure extensions still work as expected? I noticed the unit testing and that's great.
>
> I did a smoke test and didn't find any problems, but I'm not sure how to
> test specific changes that were made in this patch. Do you have any
> suggestions?
>
> As for testing, I think the best that could be done would be to go to the CA's EE interface and try
> a subset of the different certificate enrollment profiles. Some of the profiles require cert requests in a
> PKCS#10 format and some don't. The requests have to be parsed and this would exercise things.

I did some tests again using CRMF and I didn't see any problems.

> Also, it would be nice to inspect the pretty print results of some of these enrollments to see if any extensions
> print out to the screen in an obvious incorrect fashion. If the smoke test spoken of was basically this, great.

I checked the extensions output in the Agent interface and didn't see 
anything out of ordinary.

-- 
Endi S. Dewata
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dogtag-edewata-0011-2-Added-generics-part-1.patch
Type: text/x-patch
Size: 536997 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20120117/51cb3947/attachment.bin>