Enable Brotli decompression

[zakkak]

I tried adding a test that gets an encrypted text and sends it encrypted to the server for decryption as in #43392 with no success (I couldn't get the test properly set up, not that it decompression fails). Manually running the reproducer from #43392 confirms the fix, but we should have a test in Quarkus IT. @Karm do you think you can help with this given your prior experience?

Note/Thoughts: Zstd and Snappy (cc @gastaldi) remain unsupported. Since we already support snappy in Kafka, and there is quarkus-snappy we should probably support it in vertx as well.

Fixes #43392

[geoand]

Are we waiting for something for this one or should it go in?

[Karm]

@geoand I'll add a test case, hopefully today. Could be a separate PR, could be this one...

[geoand]

Thanks for the update. I'll ket @zakkak comment on his preference

[zakkak]

Given that adding a test is in @Karm's immediate plans I prefer waiting for it so that we can backport the PR as a whole (including tests) in one go.

[Karm]

@zakkak @cescoffier I added a Decompressor tests in commit 1aa2223

It works fine with HotSpot, client sends compressed PUT and POST to various endpoints and Server decompresses and either sends back or compresses again and sends back, client decompresses and checks. All fine.

Native testing fails

 ./mvnw clean verify -f integration-tests -pl  vertx-http-compressors/app,vertx-http-compressors/all,vertx-http-compressors/some -Dnative

Native test flow fails for a peculiar reason:

java.lang.UnsatisfiedLinkError: 'java.nio.ByteBuffer com.aayushatharva.brotli4j.encoder.EncoderJNI.nativeCreate(long[])'
        at com.aayushatharva.brotli4j.encoder.EncoderJNI.nativeCreate(Native Method)
        at com.aayushatharva.brotli4j.encoder.EncoderJNI.access$200(EncoderJNI.java:17)
        at com.aayushatharva.brotli4j.encoder.EncoderJNI$Wrapper.<init>(EncoderJNI.java:90)
        at com.aayushatharva.brotli4j.encoder.Encoder.<init>(Encoder.java:48)
        at com.aayushatharva.brotli4j.encoder.BrotliOutputStream.<init>(BrotliOutputStream.java:36)
        at com.aayushatharva.brotli4j.encoder.BrotliOutputStream.<init>(BrotliOutputStream.java:48)
        at com.aayushatharva.brotli4j.encoder.BrotliOutputStream.<init>(BrotliOutputStream.java:58)
        at io.quarkus.compressors.it.Testflow.compress(Testflow.java:153)
        at io.quarkus.compressors.it.Testflow.runDecompressorsTest(Testflow.java:125)
        at io.quarkus.compressors.it.RESTEndpointsTest.testDecompressors(RESTEndpointsTest.java:120)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at io.quarkus.test.junit.QuarkusTestExtension.interceptTestTemplateMethod(QuarkusTestExtension.java:856)

I originally thought that for some reason the test code itself is baked in the executable, but it is not the case.

So far it escapes me what happens that when running the integration test native, the test code, i.e. what is typed in the Junit, cannot find the Brotli4j .so :-(

[Karm]

For the record, doing this doesn't help the native integration test:

--- a/integration-tests/vertx-http-compressors/app/pom.xml
+++ b/integration-tests/vertx-http-compressors/app/pom.xml
@@ -21,6 +21,11 @@
             <artifactId>quarkus-junit5</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.aayushatharva.brotli4j</groupId>
+            <artifactId>brotli4j</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-rest</artifactId>

and neither does this:

--- a/integration-tests/vertx-http-compressors/all/pom.xml
+++ b/integration-tests/vertx-http-compressors/all/pom.xml
@@ -29,6 +29,11 @@
             <type>test-jar</type>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.aayushatharva.brotli4j</groupId>
+            <artifactId>brotli4j</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>io.vertx</groupId>
             <artifactId>vertx-web-client</artifactId>

[Karm]

I'll revisit it later...

[Karm]

The CI test result is apparently misleading again 😔 I haven't fixed the Native test yet. It passed for me locally only with HotSpot.

[Karm]

@zakkak @cescoffier
This "fixes" the native test:

--- a/integration-tests/vertx-http-compressors/app/src/test/java/io/quarkus/compressors/it/Testflow.java
+++ b/integration-tests/vertx-http-compressors/app/src/test/java/io/quarkus/compressors/it/Testflow.java
@@ -149,6 +149,7 @@ public static Buffer compress(String algorithm, String payload) {
             }
             return Buffer.buffer(byteStream.toByteArray());
         } else if ("br".equalsIgnoreCase(algorithm)) {
+            System.load("/tmp/brotli/lib/linux-x86_64/libbrotli.so");
             try (BrotliOutputStream brotliStream = new BrotliOutputStream(byteStream)) {
                 brotliStream.write(payload.getBytes(StandardCharsets.UTF_8));
             } catch (IOException e) {

Do you think it's some kind of classloading issue that manifests only when running with -Dnative, the way the all and some apps depend on the app module?

[Karm]

I have a fix, gonna push later.
One needs to call Brotli4jLoader.ensureAvailability(); to load the .so. Obviously... The fact that you don't have to do it in HotSpot testing as QuarkusTest is that you run it all in the same instance and the brotli is laoded already by Q.

[Karm]

@zakkak @cescoffier
✔️ It's fixed now. Both HotSpot and Native works. The reason really was that when running as QuarkusTest, Brotli was already loaded for you by Q, whereas when running as IntegrationTest, nobody loaded Brotli for you so the test must load it.

❌ Why the CI was all green here?
It's yet another case of a giant false negative, the test was collapsing as ERROR in native, but the way Maven is set up in this CI somehow reports it as "Flaky" and makes the overall status green.

See https://karms.biz/pastebin/errors-labeled-as-flakes.txt

I'll open a separate issue for it.

[Karm]

I added tests and it's all good now. Thanks.

[Karm]

#44422

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 4174cce.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

---

Flaky tests - Develocity

⚙️ Native Tests - HTTP

📦 integration-tests/rest-client-reactive

✖ io.quarkus.it.rest.client.BasicTestIT.shouldCreateClientSpans - History

• expected: <1> but was: <2> - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: expected: <1> but was: <2>
	at io.quarkus.it.rest.client.BasicTest.shouldCreateClientSpans(BasicTest.java:216)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:805)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

[zakkak]

Thank you @Karm !!!