Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASSETS-33747 named smartcrop in image core v3 component #2676

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5013f08
ASSETS-33747 named smartcrop in image core v3 component powered by fa…
mohiaror Feb 19, 2024
2fa74c7
ASSETS-33747 named smartcrop in image core v3 component powered by fa…
mohiaror Feb 19, 2024
59483dd
@trivial fix linting errors.
mohiaror Feb 19, 2024
7a91999
Show smartcrop renditions group even if DM is disabled but polaris is…
mohiaror Feb 21, 2024
db2c096
Update code to also show dynamicmedia group when dialog is loaded wit…
mohiaror Feb 21, 2024
c0e7c27
Make dependency on clientbuilderfactory optional and check for null. …
mohiaror Feb 21, 2024
030d3c7
ASSETS-33747 named smartcrop in image core v3 component powered by fa…
mohiaror Feb 25, 2024
09bb73f
Merge remote-tracking branch 'upstream/main' into issues/ASSETS-33747
mohiaror Feb 25, 2024
2a459f0
Minor bug fixes.
mohiaror Feb 29, 2024
81b271c
Fix test cases.
mohiaror Mar 1, 2024
ba0ad23
Merge branch 'main' into issues/ASSETS-33747
LSantha Mar 1, 2024
74f67c2
@trivial Fix existing test cases.
mohiaror Mar 4, 2024
1c208e5
Merge remote-tracking branch 'upstream/main' into issues/ASSETS-33747
mohiaror Mar 4, 2024
3d052c5
ASSETS-33747 named smartcrop in image core v3 component
mohiaror Mar 8, 2024
428751c
Merge remote-tracking branch 'upstream/main' into issues/ASSETS-33747
mohiaror Mar 8, 2024
675de49
ASSETS-33747 named smartcrop in image core v3 component
mohiaror Mar 10, 2024
680286c
@trivial Hide the smartcrop option if remote asset does not have smar…
mohiaror Mar 11, 2024
1e5a442
ASSETS-33747 named smartcrop in image core v3 component
mohiaror Mar 11, 2024
28361c5
@trivial Move the code to show smartcrop dropdown only after fetching…
mohiaror Mar 12, 2024
1566ce3
@trivial Fix eslint error.
mohiaror Mar 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -16,13 +16,24 @@
package com.adobe.cq.wcm.core.components.internal.models.v3;

import java.awt.*;
import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Scanner;

import javax.annotation.PostConstruct;

import org.apache.commons.lang3.StringUtils;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.StatusLine;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.osgi.services.HttpClientBuilderFactory;
import org.apache.http.util.EntityUtils;
import org.apache.sling.api.SlingHttpServletRequest;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ValueMap;
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored to remove the deprecated dependencies.

import org.json.JSONObject;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -67,6 +80,10 @@ public class ImageImpl extends com.adobe.cq.wcm.core.components.internal.models.
@OSGiService
@Optional
private NextGenDynamicMediaConfig nextGenDynamicMediaConfig;

@OSGiService
@Optional
private HttpClientBuilderFactory clientBuilderFactory;

private boolean imageLinkHidden = false;

Expand All @@ -77,6 +94,9 @@ public class ImageImpl extends com.adobe.cq.wcm.core.components.internal.models.
private Dimension dimension;

private boolean ngdmImage = false;
private CloseableHttpClient client;
private static final String PATH_PLACEHOLDER_ASSET_ID = "{asset-id}";
private String metadataDeliveryEndpoint;

@PostConstruct
protected void initModel() {
Expand Down Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to a separate method to facilitate unit-testing!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it into a separate function.

HttpGet get = new HttpGet(endPointUrl);
get.setHeader("X-Adobe-Accept-Experimental", "1");
try {
HttpResponse response = client.execute(get);
StatusLine statusLine = response.getStatusLine();
int statusCode = statusLine.getStatusCode();
if (statusCode == HttpStatus.SC_OK) {
HttpEntity e = response.getEntity();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwin Thanks. Incorporated the feedback in new commit.

String entity = EntityUtils.toString(e);
JSONObject metadata = new JSONObject(entity);
JSONObject repositoryMetadata = metadata.getJSONObject("repositoryMetadata");
JSONObject smartCrops = repositoryMetadata.getJSONObject("smartcrops");
Iterator<String> smartCropsNames = smartCrops.keys();
String[] ngdmSrcsetArray = new String[smartCrops.length()];
int i = 0;
while (smartCropsNames.hasNext()) {
String namedSmartCrop = smartCropsNames.next();
if (srcUritemplate.contains("=" + URI_WIDTH_PLACEHOLDER)) {
ngdmSrcsetArray[i] =
srcUritemplate.replace("width={.width}", String.format("smartcrop=%s", namedSmartCrop)) + " " + ((JSONObject)smartCrops.get(namedSmartCrop)).get("width") + "w";
i++;
}
}
srcSet = StringUtils.join(ngdmSrcsetArray, ',');

}
} catch (IOException | JSONException e) {
LOGGER.warn("Couldn't generate srcset for remote asset");
}
} else {
for (int i = 0; i < widthsArray.length; i++) {
if (srcUritemplate.contains("=" + URI_WIDTH_PLACEHOLDER)) {
Expand Down Expand Up @@ -271,18 +322,26 @@ private void initNextGenerationDynamicMedia() {
initResource();
properties = resource.getValueMap();
String fileReference = properties.get("fileReference", String.class);
String smartCrop = properties.get("smartCrop", String.class);
String smartCrop = properties.get("smartCropRendition", String.class);
if (isNgdmImageReference(fileReference)) {
int width = currentStyle.get(PN_DESIGN_RESIZE_WIDTH, DEFAULT_NGDM_ASSET_WIDTH);
NextGenDMImageURIBuilder builder = new NextGenDMImageURIBuilder(nextGenDynamicMediaConfig, fileReference)
.withPreferWebp(true)
.withWidth(width);
if(StringUtils.isNotEmpty(smartCrop)) {
if(StringUtils.isNotEmpty(smartCrop) && !StringUtils.equals(smartCrop, SMART_CROP_AUTO)) {
builder.withSmartCrop(smartCrop);
}
src = builder.build();
ngdmImage = true;
hasContent = true;
if (clientBuilderFactory != null) {
client = clientBuilderFactory.newBuilder().build();
}
metadataDeliveryEndpoint = nextGenDynamicMediaConfig.getAssetMetadataPath();
Scanner scanner = new Scanner(fileReference);
scanner.useDelimiter("/");
String assetId = scanner.next();
metadataDeliveryEndpoint = metadataDeliveryEndpoint.replace(PATH_PLACEHOLDER_ASSET_ID, assetId);
}
}

Expand Down
Expand Up @@ -36,7 +36,7 @@ public class NextGenDMImageURIBuilder {

private NextGenDynamicMediaConfig config;
private String fileReference;
private String smartCropAspectRatio;
private String smartCropName;
private int width = DEFAULT_NGDM_ASSET_WIDTH;

private int height;
Expand All @@ -48,11 +48,11 @@ public NextGenDMImageURIBuilder(NextGenDynamicMediaConfig config, String fileRef
}

/**
* Smart Crop aspect ratio string.
* @param smartCropAspectRatio - a string in "width:height" format;
* Smart Crop name.
* @param smartCropName - a string denoting the name of the smartcrop;
*/
public NextGenDMImageURIBuilder withSmartCrop(String smartCropAspectRatio) {
this.smartCropAspectRatio = smartCropAspectRatio;
public NextGenDMImageURIBuilder withSmartCrop(String smartCropName) {
this.smartCropName = smartCropName;
return this;
}

Expand Down Expand Up @@ -113,12 +113,8 @@ public String build() {
if(this.preferWebp) {
params.put("preferwebp", "true");
}
if (StringUtils.isNotEmpty(this.smartCropAspectRatio)) {
if (isValidSmartCrop(this.smartCropAspectRatio)) {
params.put("crop", String.format("%s,smart", this.smartCropAspectRatio));
} else {
LOGGER.info("Invalid smartCrop value at {}", this.smartCropAspectRatio);
}
if (StringUtils.isNotEmpty(this.smartCropName)) {
params.put("smartcrop", this.smartCropName);
}
if(params.size() > 0) {
uriBuilder.append("?");
Expand All @@ -133,18 +129,4 @@ public String build() {
LOGGER.info("Invalid fileReference or NGDMConfig. fileReference = {}", this.fileReference);
return null;
}

private boolean isValidSmartCrop(String smartCropStr) {
String[] crops = smartCropStr.split(":");
if (crops.length == 2) {
try {
Integer.parseInt(crops[0]);
Integer.parseInt(crops[1]);
return true;
} catch (NumberFormatException ex) {
return false;
}
}
return false;
}
}
Expand Up @@ -70,7 +70,7 @@ private void initModel() {
Resource component = request.getResourceResolver().getResource(componentPath);
ValueMap properties = component.getValueMap();
String fileReference = properties.get("fileReference", String.class);
String smartCrop = properties.get("smartCrop", String.class);
String smartCrop = properties.get("smartCropRendition", String.class);
ValueMap configs = resource.getValueMap();
int width = configs.get("width", 480);
int height = configs.get("height", 480);
Expand Down
Expand Up @@ -53,6 +53,7 @@ protected void setUp() {
MockNextGenDynamicMediaConfig config = new MockNextGenDynamicMediaConfig();
config.setEnabled(true);
config.setRepositoryId("testrepo");
config.setAssetMetadataPath("/adobe/assets/{asset-id}/metadata");
context.registerInjectActivateService(config);
}

Expand Down
Expand Up @@ -671,6 +671,7 @@ void testNgdmImage() {
MockNextGenDynamicMediaConfig config = new MockNextGenDynamicMediaConfig();
config.setEnabled(true);
config.setRepositoryId("testrepo");
config.setAssetMetadataPath("/adobe/assets/{asset-id}/metadata");
context.registerInjectActivateService(config);

Image image = getImageUnderTest(NGDM_IMAGE1_PATH);
Expand All @@ -683,6 +684,7 @@ void testNgdmImageWithResizeWidth() {
MockNextGenDynamicMediaConfig config = new MockNextGenDynamicMediaConfig();
config.setEnabled(true);
config.setRepositoryId("testrepo");
config.setAssetMetadataPath("/adobe/assets/{asset-id}/metadata");
context.registerInjectActivateService(config);
context.contentPolicyMapping(resourceType, PN_DESIGN_RESIZE_WIDTH, 800);

Expand Down
Expand Up @@ -83,7 +83,7 @@ public void testThumbnailSrc() {
assertTrue(nextGenDMThumbnail.getSrc().contains(BASE_URI));
assertTrue(nextGenDMThumbnail.getSrc().contains("width=260"));
assertTrue(nextGenDMThumbnail.getSrc().contains("height=260"));
assertTrue(nextGenDMThumbnail.getSrc().contains("crop=2:3,smart"));
assertTrue(nextGenDMThumbnail.getSrc().contains("smartcrop=Medium"));
}

@Test
Expand Down
Expand Up @@ -85,17 +85,9 @@ public void testUriWithCustomPreferWebp() {

@Test
public void testUriWithSmartCrop() {
uriBuilder.withSmartCrop("2:3");
uriBuilder.withSmartCrop("Medium");
String uri = uriBuilder.build();
assertTrue(uri.contains("crop=2:3,smart"));
}


@Test
public void testUriWithInvalidSmartCrop() {
uriBuilder.withSmartCrop("x:y");
String uri = uriBuilder.build();
assertEquals(baseUri, uri);
assertTrue(uri.contains("smartcrop=Medium"));
}

@Test
Expand Down
@@ -1,8 +1,9 @@
{
"id": "image-042059c802",
"src": "https://testrepo/adobe/dynamicmedia/deliver/urn:aaid:aem:e82c3c87-1453-48f5-844b-1822fb610911/cutfruits.png?width=640&preferwebp=true&crop=2:3,smart",
"src": "https://testrepo/adobe/dynamicmedia/deliver/urn:aaid:aem:e82c3c87-1453-48f5-844b-1822fb610911/cutfruits.png?width=640&preferwebp=true&smartcrop=Medium",
"lazyEnabled": true,
"sizes": "",
"smartCropRendition": "Medium",
"dataLayer": {
"image-042059c802": {
"@type": "core/wcm/components/image/v3/image"
Expand Down
@@ -1,6 +1,7 @@
{
"id": "image-042059c802",
"lazyEnabled": true,
"smartCropRendition": "Medium",
"dataLayer": {
"image-042059c802": {
"@type": "core/wcm/components/image/v3/image"
Expand Down
@@ -1,7 +1,8 @@
{
"id": "image-042059c802",
"src": "https://testrepo/adobe/dynamicmedia/deliver/urn:aaid:aem:e82c3c87-1453-48f5-844b-1822fb610911/cutfruits.png?width=800&preferwebp=true&crop=2:3,smart",
"src": "https://testrepo/adobe/dynamicmedia/deliver/urn:aaid:aem:e82c3c87-1453-48f5-844b-1822fb610911/cutfruits.png?width=800&preferwebp=true&smartcrop=Medium",
"lazyEnabled": true,
"smartCropRendition": "Medium",
"sizes": "",
"dataLayer": {
"image-042059c802": {
Expand Down
2 changes: 1 addition & 1 deletion bundles/core/src/test/resources/image/v3/test-content.json
Expand Up @@ -957,7 +957,7 @@
"jcr:createdBy": "admin",
"sling:resourceType": "core/wcm/components/image/v3/image",
"fileReference": "/urn:aaid:aem:e82c3c87-1453-48f5-844b-1822fb610911/cutfruits.png",
"smartCrop": "2:3"
"smartCropRendition": "Medium"
}
}
}
Expand Down
Expand Up @@ -51,9 +51,6 @@
text="Enable DM features"
uncheckedValue="false"
value="{Boolean}true">
<granite:rendercondition jcr:primaryType="nt:unstructured"
sling:resourceType="granite/ui/components/coral/foundation/renderconditions/feature"
feature="com.adobe.dam.asset.scene7.feature.flag"/>
</enableDmFeatures>
<enableAssetDelivery
jcr:primaryType="nt:unstructured"
Expand Down