Supporting Compression discovery with ServiceLoader. #12611
Conversation
joakime
commented
Dec 4, 2024
joakime
commented
- New Compressions class to make this easier.
+ New Compressions class to make this easier.
...ssion/jetty-compression-common/src/main/java/org/eclipse/jetty/compression/Compressions.java
Outdated
Show resolved
Hide resolved
...pression-gzip/src/main/resources/META-INF/services/org.eclipse.jetty.compression.Compression
Outdated
Show resolved
Hide resolved
...ion-zstandard/src/main/resources/META-INF/services/org.eclipse.jetty.compression.Compression
Outdated
Show resolved
Hide resolved
| /** | ||
| * The {@link Compression} implementations available via {@link java.util.ServiceLoader}. | ||
| */ | ||
| public class Compressions |
There was a problem hiding this comment.
I don't think this class is necessary.
In the proxy case, we do not want both client and server to reference the same JVM singleton, but rather have each its own instance, for example to support different configurations.
The client will look the compressions in HttpClient.doStart(), and the server could do the same in CompressionHandler.doStart().
| // Fallback to discovered encodings via the service loader instead. | ||
| if (supportedEncodings.isEmpty()) | ||
| { | ||
| TypeUtil.serviceProviderStream(ServiceLoader.load(Compression.class)).forEach( |
There was a problem hiding this comment.
Perhaps it's just simpler to use serviceStream() here.
| } | ||
| catch (Throwable e) | ||
| { | ||
| LOG.warn("Unable to get Compression", e); |
There was a problem hiding this comment.
Use debug level here, but better yet, use serviceStream() and get rid of try/catch.
| @@ -76,9 +78,6 @@ public CompressionHandler() | |||
|
|
|||
| public void addCompression(Compression compression) | |||
There was a problem hiding this comment.
Please rename this to Compression putCompression(Compression compression).
| supportedEncodings.put(compression.getEncodingName(), compression); | ||
| compression.setContainer(this); | ||
| addBean(compression); |
There was a problem hiding this comment.
| supportedEncodings.put(compression.getEncodingName(), compression); | |
| compression.setContainer(this); | |
| addBean(compression); | |
| Compression existing = supportedEncodings.put(compression.getEncodingName(), compression); | |
| compression.setContainer(this); | |
| updateBean(existing, compression, true); | |
| return existing; |
