RFR[15] JDK-8245134: test/lib/jdk/test/lib/security/KeyStoreUtils.java should allow to specify aliases

Valerie Peng valerie.peng at oracle.com
Tue May 26 19:07:28 UTC 2020


Looks good. Cleaner and shorter this way~

Thanks,
Valerie
On 5/21/2020 8:18 PM, sha.jiang at oracle.com wrote:
>
> Hi Valerie,
> Thanks for your clarification!
>
> Could you please review this updated webrev:
> http://cr.openjdk.java.net/~jjiang/8245134/webrev.01/
>
> Best regards,
> John Jiang
>
> On 2020/5/22 06:08, Valerie Peng wrote:
>>
>>
>> On 5/20/2020 7:38 PM, sha.jiang at oracle.com wrote:
>>>
>>>> - line 176, maybe it's better to just supply the type, clearer and 
>>>> less dependency.
>>>>
>>> Not get this point.
>>> Could you please describe some more?
>>
>> I mean to change line 176 as below (see the added text in blue):
>>
>> 176 return createTrustStore(DEFAULT_TYPE, certStrs, null);
>>
>>
>>>> With this extra aliases argument, number of createTrustStore(...) 
>>>> methods doubled from 2 to 4, number of createKeyStore(...) methods 
>>>> also doubled from 4 to 8. Isn't it a bit much to have 8 methods 
>>>> doing the same thing? Especially in the case of 
>>>> createKeyStore(...), quite a few of them have long list of 
>>>> arguments with the same type, combining this with the large number 
>>>> of methods, it can get confusing on which method is called. How 
>>>> often do you think the aliases are supplied? Maybe we only add 
>>>> methods which will be used instead of adding all possible combinations.
>>>>
>>> I'll remove 4 createKeyStore(...) methods (like the below one) that 
>>> have long list of arguments.
>>> createKeyStore(String type, String[] keyAlgos,
>>>             String[] keyStrs, String[] passwords, String[] certStrs,
>>>             String[] aliases)
>>> It looks no test is using them. My tests also won't use them.
>>
>> Sounds good.
>>
>> Thanks,
>> Valerie
>>>
>>> Best regards,
>>> John Jiang
>>>>
>>>> Thanks,
>>>> Valerie
>>>> On 5/15/2020 11:40 PM, sha.jiang at oracle.com wrote:
>>>>> Hi,
>>>>> This patch adds some new createTrustStore() and createKeyStore() 
>>>>> methods to test
>>>>> lib class jdk.test.lib.security.KeyStoreUtils.
>>>>> These new methods allow to pass trust/key store aliases in.
>>>>>
>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8245134
>>>>> Webrev: http://cr.openjdk.java.net/~jjiang/8245134/webrev.00/
>>>>>
>>>>> Best regards,
>>>>> John Jiang
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20200526/013b8b65/attachment.htm>


More information about the security-dev mailing list