RFR: 8284694: Avoid evaluating SSLAlgorithmConstraints twice [v3]

[Daniel Jeliński]

On Tue, 19 Apr 2022 14:23:07 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Daniel Jeliński has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - add more factory methods, update copyright
>>  - return DEFAULT also when user constraints are null
>
> src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java line 94:
> 
>> 92:      * @return a SSLAlgorithmConstraints instance
>> 93:      */
>> 94:     static AlgorithmConstraints forSocket(SSLSocket socket,
> 
> I like the idea to use a static method for the construction.  What do you think if the same coding style is used for other constructors, by making them private and add forSocket() methods accordingly?  For example, changing the following constructor to a private method.
> 
> SSLAlgorithmConstraints(SSLEngine engine, String[] supportedAlgorithms,
>             boolean withDefaultCertPathConstraints) {

it won't change the performance in any way - when `supportedAlgorithms` are set, we always need to create a new object. But you're right, it's inconsistent to offer both constructors and factory methods in the same class. Changed.

> src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java line 118:
> 
>> 116:         if (userSpecifiedConstraints == null) {
>> 117:             return withDefaultCertPathConstraints ? DEFAULT : DEFAULT_SSL_ONLY;
>> 118:         }
> 
> It looks like a partial duplicate of nullIfDefault().  What do you think if  merging the logic into nullIfDefault()?  Or even merging nullIfDefault() logic into the getUserSpecifiedConstraints() method?  New parameters may be required for the getUserSpecifiedConstraints() methods.

I'm not a big fan of modifying `getUserSpecifiedConstraints`; that method has 3 `return` clauses. But you're right, there was some code duplication that is removed now.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8199