Partial fix for issue #7698 - TlsManager support for client-side #7699
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oracle-contributor-agreement
bot
added
the
OCA Verified
tomas-langer
previously requested changes
Oct 4, 2023
| private TlsManagerCache() { | ||
| } | ||
|
|
||
| static TlsManager getOrCreate(Object configBean, |
There was a problem hiding this comment.
The signature should be:
static <T> TlsManager getOrCreate(T configBean, Function<T, TlsManager> creator)
if this would be the way to go, as that allows you to use a method referenc for the creator
| * @param creator the creator to apply if not already in cache, which takes the config bean instance | ||
| * @return the tls manager instance from cache, defaulting to creation from the {@code creator} if not in cache | ||
| */ | ||
| static TlsManager getOrCreate(Config config, |
There was a problem hiding this comment.
why is there a name and config parameter, when theser are not used in the code? Please remove them
Also update the signature to what I wrote above
There was a problem hiding this comment.
I was thinking we might need them for future use. I'll remove them but at least consider that.
danielkec
requested changes
Oct 5, 2023
common/tls/src/main/java/io/helidon/common/tls/spi/TlsManagerProvider.java
Show resolved
Hide resolved
common/tls/src/main/java/io/helidon/common/tls/spi/TlsManagerProvider.java
Outdated
Show resolved
Hide resolved
danielkec
approved these changes
Oct 9, 2023
dalexandrov
pushed a commit
to dalexandrov/helidon
that referenced
this pull request
Oct 17, 2023
…-side (helidon-io#7699) * Partial fix for issue helidon-io#7698 - Support for caching managers when the config is the same.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This provides an enhancement whereas on the same JVM, using the same config bean, you will get the same
TlsManagerinstance. This is to prevent a proliferation of manager instances in client-side usages - which will officially be coming in a separator PR later.Base issue is #7698