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

[John Magne]

Thanks Endi:

----- Original Message -----
From: "Endi Sukma Dewata" <edewata at redhat.com>
To: "John Magne" <jmagne at redhat.com>
Cc: pki-devel at redhat.com
Sent: Monday, January 16, 2012 1:46:45 PM
Subject: Re: [Pki-devel] [PATCH] 11 Added generics (part 1).

New patch attached. It has been rebased against the latest.

On 1/14/2012 12:12 AM, 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.

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?



> Somehow in rifling through 10,000 lines I picked out this below:
>
> CertificatePoliciesExtDefault.java
>
> -            Vector infos = null;
> +            Vector<CertificatePolicyInfo>  infos;
>
> Looks like we didn't set info to null here:

OK

I think the following code will set the infos, or set it to null if 
there's an exception, so the null initialization is not necessary.

> 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.

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.

-- 
Endi S. Dewata


> ----- Original Message -----
> From: "Endi Sukma Dewata"<edewata at redhat.com>
> To: pki-devel at redhat.com
> Sent: Friday, January 13, 2012 7:53:22 AM
> Subject: [Pki-devel] [PATCH] 11 Added generics (part 1).
>
> This patch is based on Adam's patch. It brings down the warnings
> from 6137 to 4656.
>
> Ticket #2
>
> http://fedorapeople.org/gitweb?p=edewata/public_git/pki.git;a=commitdiff;h=0cf280a1c37d89ae4501a766f38275be0fcb7408