QQE 378 HTTP module / REST use-cases - Development topic compression Brotli4j

[jcarranzan]

Links

Jira : https://issues.redhat.com/browse/QQE-378

Quarkus documentation:

• Quarkus - http-reference#http-compression
• Quarkus - reactive-routes#http-compressio
• Vert.x - http_compression
• Vert.x - Handling compressed body
• Vert.x - http_compression_algorithms

Reminder for considerable topics

• Make sure you have considered the following areas when preparing the test plan:

  • Logging
  • Tracing
  • Metrics
  • Security
  • OpenAPI
  • Data sources
  • Frontend
  • Qute

[michalvavrik]

I could go on, but it's a draft so I just left few notes. Cheers :-)

[michalvavrik]

Hello @jcarranzan , I'm not going to continue discussion in individual comment threads because there seems to be something I don't link about this TP in general, not particular lines. You say:

As far as I know, Quarkus itself is not capable of managing brotli4J but yes gzip. For that reason, vert.x configuration HTTP server options become relevant in this context.

I don't know where these information comes form, but it doesn't seem to be correct.

Well, quarkus options like quarkus.http.enable-compression and quarkus.http.enable-decompression works by default for gzip so for brotli4J we need both options enabled and use Vert.x HTTP server options in some way (as you mentioned above) or directly the Brotli4J library API.

I gave you link for the quarkus.http.enable-compression already: https://quarkus.io/guides/all-config#quarkus-vertx-http_quarkus-http-enable-compression

and it says: Note that the compression format (e.g., gzip) must be specified in the Content-Encoding header in the request. which means you just set content-encoding to the br and that is it.

Please consider rewriting this TP so that we actually test Quarkus support to Brotli4j compression and not Vert.x support. I think we should use Quarkus constructs, that is we set these configuration properties (enable compression) we use annotations and headers. If that is not sufficient, then please consider opening bug in Quarkus. Thank you

BTW when looking into this, I also mentioned netty extensions handles brotli4j already, so you don't need to add it and it should work in native.

[jcarranzan]

Hi @michalvavrik , my comments about this PR came from the fact that last week I was experimenting with the code and the documentation. Indeed, there isn't much information about Brotli4J compression in the Quarkus docs, and I couldn't find much in the Quarkus core code either, except that the Netty extensions handle Brotli4J. Therefore, in Vert.x, it's also handled with custom HTTP server configuration, as I did a draft PR here (draft).

That being said, I pushed some classes with the basic Quarkus setup to my branch to see if I could implement Brotli4J compression, as you suggested, but I wasn't successful. So, I would appreciate it if you could take a look and see if you notice anything that I might be missing. (branch--> https://github.com/jcarranzan/quarkus-test-suite/tree/brotli4j-coverage-3.8). Thanks

[michalvavrik]

Hi @michalvavrik , my comments about this PR came from the fact that last week I was experimenting with the code and the documentation. Indeed, there isn't much information about Brotli4J compression in the Quarkus docs, and I couldn't find much in the Quarkus core code either, except that the Netty extensions handle Brotli4J. Therefore, in Vert.x, it's also handled with custom HTTP server configuration, as I did a draft PR here (quarkus-qe/quarkus-test-suite#1717.

Tried it. Using io.vertx.core.http.HttpServerOptions#addCompressor seems only way. The Content-Encoding is only for decompression, my fault. The Accept-Encoding can only use to signal client support for existing compressors.

So to conclude this TP, I suggest:

• Can you add to the test plan choosing between gzip and br by that header if that makes sense to you.
• Consider creating documentation issue in the Quarkus that shows how to add support for other than default compressor? I re-read all the related docs and I don't think it's user friendly. Choice is yours though.
• We already have compressed / decompressed annotations tested somewhere, can you also add it to the TP for Brotli4j?
• Explain in the TP why up to 8192 magical number is tested. I stick to my previous comments, some PR header does not establish a contract and we shouldn't follow it. What users need to know should be found in the docs.

Please answer following questions:

• https://issues.redhat.com/browse/QQE-378 only speaks about compression, isn't it strange to test compression support for Brotli4j and not decompression?
• https://issues.redhat.com/browse/QQE-378 speaks about other compressors like Snappy, TP should explain why chose to test Brotli4j and not others.
• Why isn't OpenShift relevant for this testing?

[michalvavrik]

In regards of the 8192, can you please check whether raising https://quarkus.io/guides/all-config#quarkus-vertx-http_quarkus-http-limits-max-chunk-size helps? If not, there should be opened documentation issue linked with this TP. Thank you

[jcarranzan]

Please answer following questions:

* https://issues.redhat.com/browse/QQE-378 only speaks about compression, isn't it strange to test compression support for Brotli4j and not decompression? 

Good point, now I've added some lines related to decompression also.

* https://issues.redhat.com/browse/QQE-378 speaks about other compressors like Snappy, TP should explain why chose to test Brotli4j and not others.

I've added it also.

* Why isn't OpenShift relevant for this testing? 

Yes, it's, added.
Thanks.