#2853 The AdaptiveImageServlet should send redirects to blobs

[joerghoh]

Q	A
Fixed Issues?	Fixes #2853
Patch: Bug Fix?
Minor: New Feature?
Major: Breaking Change?
Tests Added + Pass?	Yes
Documentation Provided	Yes (code comments and or markdown)
Any Dependency Changes?
License	Apache License, Version 2.0

The AdaptiveImageServlet always streams its result through the JVM, even if its an already existing rendition with a matching blob in the blobstore. In AEM CS can delivered more efficiently with sending a redirect to the blobstore, and then the Fastly CDN follows this redirect and pulls the data directly from the blobstore. Then AEM does not need to stream that binary through the JVM.

This patch improves the handling not to stream the binary when possible. The feature is configurable and off by default.

(A word to the tests: In JUnit5 it's not possible to parametrize entire test classes anymore, which would have been my preferred way for the tests. Instead you would have to parametrize the tests on their own. To avoid that I duplicated the tests which were failing with the new feature turned and tagged the duplicates accordingly. Now the setup() can initialize the AdaptiveImageServlet accordingly. Happy to learn about a way, which allows to run each tests with the new feature turned off and on, without annotating each test.

Existing tests were not changed, to make sure that all the changes did not affect the default behavior.

[sonarqubecloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[kwin]

Why not leveraging https://sling.apache.org/documentation/bundles/rendering-content-default-get-servlets.html#streamrendererservlet via request dispatching?

[joerghoh]

@kwin I want to get rid of that stream approach where possible, not encourage it :-)

[kwin]

@joerghoh Please read both documentation and code more thoroughly. That streamrendererservlet does the redirect exactly as you implemented (despite its name): https://github.com/apache/sling-org-apache-sling-servlets-get/blob/507a84c5cc2ede84710dd719f97bb483629b969e/src/main/java/org/apache/sling/servlets/get/impl/helpers/StreamRenderer.java#L159

[joerghoh]

@kwin Unfortunately just sending the link to the raw URL of the blob is not enough in this case.

[kwin]

To me the URI being generated in https://github.com/apache/sling-org-apache-sling-jcr-resource/blob/15bfe9a6281618e5f14419c3979194cb5d0148f0/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/BinaryDownloadUriProvider.java#L111 and exposed via ExternalizedInputStream look pretty much the same as what is done in https://github.com/adobe/aem-core-wcm-components/pull/2871/files#diff-7c9529fe2217ac11ea6e74a0d7e51371fc07d1769d73e11f21f5c1377c7ce376R698.

[joerghoh]

Yes, that's correct, I will adapt the PR accordingly. Thanks @kwin for the pointer, I was not aware that we have in Sling that code already.

Need to wait for #2875 to be merged, before I can continue here.