ASSETS-33747 named smartcrop in image core v3 component #2676
Conversation
…stly Instead of fastly smartcrops, expect named smartcrops in delivery URLs.
…stly Add editor code to render image through delivery URL and support smartcrop.
| @@ -32,6 +43,8 @@ | |||
| import org.apache.sling.models.annotations.injectorspecific.OSGiService; | |||
| import org.jetbrains.annotations.NotNull; | |||
| import org.jetbrains.annotations.Nullable; | |||
| import org.json.JSONException; | |||
There was a problem hiding this comment.
You should no longer use those deprecated dependencies (https://javadoc.io/doc/com.adobe.aem/uber-jar/latest/org/json/package-summary.html), but rather javax.json
There was a problem hiding this comment.
Refactored to remove the deprecated dependencies.
| StatusLine statusLine = response.getStatusLine(); | ||
| int statusCode = statusLine.getStatusCode(); | ||
| if (statusCode == HttpStatus.SC_OK) { | ||
| HttpEntity e = response.getEntity(); |
There was a problem hiding this comment.
You should always close the response. Better use the execute method taking a ResponseHandler (https://hc.apache.org/httpcomponents-client-4.5.x/current/httpclient/apidocs/org/apache/http/client/HttpClient.html#execute(org.apache.http.client.methods.HttpUriRequest,%20org.apache.http.client.ResponseHandler)) and directly act on the returned input stream.
There was a problem hiding this comment.
@kwin Thanks. Incorporated the feedback in new commit.
…This is to avoid rewriting/fixing all Image test cases.
mohiaror
changed the title
| @@ -135,6 +155,37 @@ public String getSrcset() { | |||
| // in case of dm image and auto smartcrop the srcset needs to generated client side | |||
| if (dmImage && StringUtils.equals(smartCropRendition, SMART_CROP_AUTO)) { | |||
| srcSet = EMPTY_PIXEL; | |||
| } else if (ngdmImage && StringUtils.equals(smartCropRendition, SMART_CROP_AUTO) && client != null) { | |||
| String endPointUrl = "https://" + nextGenDynamicMediaConfig.getRepositoryId() + metadataDeliveryEndpoint; | |||
There was a problem hiding this comment.
Please move this to a separate method to facilitate unit-testing!
There was a problem hiding this comment.
Moved it into a separate function.
Review comments. Add a separate test case for smartcrop. Add a failing testcase for auto smartcrop.
Incorporate review feedback.
Add test cases for srcset and responsehandler code.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
============================================
- Coverage 87.17% 87.15% -0.03%
- Complexity 2668 2673 +5
============================================
Files 233 234 +1
Lines 7128 7161 +33
Branches 1090 1095 +5
============================================
+ Hits 6214 6241 +27
- Misses 363 365 +2
- Partials 551 555 +4 ☔ View full report in Codecov by Sentry. |
|
Closing this PR in favor of #2698 |
Instead of fastly smartcrops, expect named smartcrops in delivery URLs.
Fixes #1, Fixes #2