Use Java 11 compatible sslSocketFactory setter #9
Conversation
jroitgrund
force-pushed
the
java-11-compatibility
branch
from
f474959 to
e5c3630
Compare
|
Woops, hadn't noticed #8 ! Mine builds though. |
| public HttpPollingResource(Optional<SSLSocketFactory> socketFactory, Collection<String> pollRequests, | ||
| int numAttempts, long intervalMillis, int connectionTimeoutMillis, int readTimeoutMillis) { | ||
| public HttpPollingResource( | ||
| Optional<SslParameters> sslParameters, |
There was a problem hiding this comment.
This SSLSocketFactory -> SslParameters change broke Java API and means automatic PRs don't go in without manual attention :/
do you mind if I bring back the backcompat thing?
There was a problem hiding this comment.
My bad, I didn't realize that the constructor was also public - I thought only the static factory was.
There was a problem hiding this comment.
The problem is the two constructors (Optional<SslParameters> and Optional`) can't be overloaded, because they have the same erasure. To reintroduce the old constructor, we'd have to break people who moved to the new constructor. Happy to raise a PR to do this if you think it's better.
There was a problem hiding this comment.
Yep I think bringing back the old constructor is the right approach here. There are tons of usages of 0.4.0 but since 0.5.0 has only been released for a couple of days (and didn't auto upgrade) I think it's ok to call this a 'fix'.
| this.x509TrustManager = x509TrustManager; | ||
| } | ||
|
|
||
| private static SslParameters of(SSLSocketFactory sslSocketFactory) { |
There was a problem hiding this comment.
why is this one private but the other public?
3 participants
Fixes the following breakage on consumers: