fixes

ist-dresden · Mar 14, 2024 · 68df6dd · 68df6dd
1 parent d2b43d1
commit 68df6dd
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 60 deletions.
diff --git a/...c/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoPageTranslateServiceImpl.java b/...c/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoPageTranslateServiceImpl.java
@@ -25,7 +25,6 @@
 import org.apache.sling.api.resource.ValueMap;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
-import org.osgi.service.component.annotations.ReferenceCardinality;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -62,9 +61,6 @@ public class AutoPageTranslateServiceImpl implements AutoPageTranslateService {
     @Reference
     protected AutoTranslateConfigService autoTranslateConfigService;
 
-    @Reference(cardinality = ReferenceCardinality.OPTIONAL)
-    protected volatile AutoTranslateService translateService;
-
     @Reference
     protected LiveRelationshipManager liveRelationshipManager;
 
@@ -301,60 +297,61 @@ protected boolean collectPropertiesToTranslate(
         }
         String sourcePath = relationship.getSourcePath();
         Resource sourceResource = resource.getResourceResolver().getResource(sourcePath);
-        if (sourceResource != null && translateService != null
-                && autoTranslateConfigService.isTranslatableResource(sourceResource)) {
-            ValueMap sourceValueMap = sourceResource.getValueMap();
-            ModifiableValueMap targetValueMap = requireNonNull(resource.adaptTo(ModifiableValueMap.class));
-            for (String key : autoTranslateConfigService.translateableAttributes(sourceResource)) {
-                if (AITranslatePropertyWrapper.isAiTranslateProperty(key)) {
-                    continue;
-                }
-                stats.translateableProperties++;
-                AITranslatePropertyWrapper targetWrapper = new AITranslatePropertyWrapper(sourceValueMap, targetValueMap, key);
-                boolean isCancelled = relationship.getStatus() != null && (
-                        relationship.getStatus().isCancelled() ||
-                                relationship.getStatus().getCanceledProperties().contains(key)
-                );
-
-                // we will translate except if the property is cancelled and we don't want to touch cancelled properties,
-                // or if we have a current translation.
-
-                if (isCancelled && !translationParameters.translateWhenChanged) {
-                    continue; // don't touch cancelled properties
-                }
-
-                if (targetWrapper.isOriginalAsWhenLastTranslating()) {
-                    // shortcut: we have a recent translation already
-                    targetWrapper.setCurrentValue(targetWrapper.getTranslatedCopy());
-                    changed = changed || !StringUtils.equals(targetWrapper.getTranslatedCopy(), targetWrapper.getOriginalCopy());
-                    continue;
-                }
+        if (sourceResource == null || !autoTranslateConfigService.isTranslatableResource(sourceResource)) {
+            if (sourceResource == null) {
+                LOG.info("No source resource found - translation not touching {}", resource.getPath());
+                // that can happen for new resources in the live copy. Unfortunately it's not really clear if we should
+                // try to translate that or not - we'll probably learn about that in practice.
+            }
+            return false;
+        }
+        ValueMap sourceValueMap = sourceResource.getValueMap();
+        ModifiableValueMap targetValueMap = requireNonNull(resource.adaptTo(ModifiableValueMap.class));
+        for (String key : autoTranslateConfigService.translateableAttributes(sourceResource)) {
+            if (AITranslatePropertyWrapper.isAiTranslateProperty(key)) {
+                continue;
+            }
+            stats.translateableProperties++;
+            AITranslatePropertyWrapper targetWrapper = new AITranslatePropertyWrapper(sourceValueMap, targetValueMap, key);
+            boolean isCancelled = relationship.getStatus() != null && (
+                    relationship.getStatus().isCancelled() ||
+                            relationship.getStatus().getCanceledProperties().contains(key)
+            );
+
+            // we will translate except if the property is cancelled and we don't want to touch cancelled properties,
+            // or if we have a current translation.
+
+            if (isCancelled && !translationParameters.translateWhenChanged) {
+                continue; // don't touch cancelled properties
+            }
 
-                if (isCancelled && targetWrapper.hasSavedTranslation()
-                        && !StringUtils.equals(targetWrapper.getTranslatedCopy(), targetWrapper.getCurrentValue())
-                        && !StringUtils.equals(targetWrapper.getOriginal(), targetWrapper.getCurrentValue())) {
-                    // = translateWhenChanged override; save manual change. We also exclude the phase during rollout
-                    // where the property is reset to the original value and we have to restore the translation.
-                    LOG.info("Re-translating {} in {} despite manual change", key, resource.getPath());
-                    targetWrapper.saveManualChange();
-                    stats.modifiedButRetranslatedProperties++;
-                }
+            if (targetWrapper.isOriginalAsWhenLastTranslating()) {
+                // shortcut: we have a recent translation already
+                targetWrapper.setCurrentValue(targetWrapper.getTranslatedCopy());
+                changed = changed || !StringUtils.equals(targetWrapper.getTranslatedCopy(), targetWrapper.getOriginalCopy());
+                continue;
+            }
 
-                if (targetWrapper.hasSavedTranslation()) {
-                    stats.retranslatedProperties++;
-                }
+            if (isCancelled && targetWrapper.hasSavedTranslation()
+                    && !StringUtils.equals(targetWrapper.getTranslatedCopy(), targetWrapper.getCurrentValue())
+                    && !StringUtils.equals(targetWrapper.getOriginal(), targetWrapper.getCurrentValue())) {
+                // = translateWhenChanged override; save manual change. We also exclude the phase during rollout
+                // where the property is reset to the original value and we have to restore the translation.
+                LOG.info("Re-translating {} in {} despite manual change", key, resource.getPath());
+                targetWrapper.saveManualChange();
+                stats.modifiedButRetranslatedProperties++;
+            }
 
-                LOG.debug("Re-translating {} in {}", key, resource.getPath());
-                PropertyToTranslate propertyToTranslate = new PropertyToTranslate();
-                propertyToTranslate.sourceResource = sourceResource;
-                propertyToTranslate.targetResource = resource;
-                propertyToTranslate.propertyName = key;
-                propertiesToTranslate.add(propertyToTranslate);
+            if (targetWrapper.hasSavedTranslation()) {
+                stats.retranslatedProperties++;
             }
-        } else {
-            LOG.info("No source resource found - translation not touching {}", resource.getPath());
-            // that can happen for new resources in the live copy. Unfortunately it's not really clear if we should
-            // try to translate that or not - we'll probably learn about that in practice.
+
+            LOG.debug("Re-translating {} in {}", key, resource.getPath());
+            PropertyToTranslate propertyToTranslate = new PropertyToTranslate();
+            propertyToTranslate.sourceResource = sourceResource;
+            propertyToTranslate.targetResource = resource;
+            propertyToTranslate.propertyName = key;
+            propertiesToTranslate.add(propertyToTranslate);
         }
         for (Resource child : resource.getChildren()) {
             if (!PATTERN_IGNORED_SUBNODE_NAMES.matcher(child.getName()).matches()) {

diff --git a/aem/core/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateConfig.java b/aem/core/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateConfig.java
@@ -37,4 +37,9 @@
                     "heuristics / standard attributes. Same syntax as allowed attributes.")
     String[] deniedAttributesRegexes() default {};
 
+    @AttributeDefinition(name = "Ignore Assets except Content Fragments",
+            description = "If true, assets are ignored for translation, except for content fragments. " +
+                    "(Otherwise the translator would translate metadata of images and videos.)")
+    boolean ignoreAssetsExceptContentFragments() default true;
+
 }
diff --git a/...main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateConfigServiceImpl.java b/...main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateConfigServiceImpl.java
@@ -117,6 +117,11 @@ public boolean isTranslatableResource(@Nullable final Resource resource) {
             return false;
         }
         final String resourceType = resource.getResourceType();
+        if (resourceType.equals("dam:AssetContent") && config.ignoreAssetsExceptContentFragments()
+                && !Boolean.TRUE.equals(resource.getValueMap().get("contentFragment", Boolean.class))){
+            LOG.debug("Ignoring asset that is not content fragment: ", resource.getPath());
+            return false;
+        }
         for (final Pattern pattern : deniedResourceTypes) {
             if (pattern.matcher(resourceType).matches()) {
                 return false;

diff --git a/...core/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateRunModel.java b/...core/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateRunModel.java
@@ -1,5 +1,7 @@
 package com.composum.ai.aem.core.impl.autotranslate;
 
+import javax.annotation.Nullable;
+
 import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.SlingHttpServletRequest;
 import org.apache.sling.api.request.RequestPathInfo;
@@ -30,6 +32,7 @@ public void checkDisabled() {
         }
     }
 
+    @Nullable
     public AutoTranslateService.TranslationRun getModel() {
         RequestPathInfo requestPathInfo = request.getRequestPathInfo();
         String suffix = requestPathInfo.getSuffix();
@@ -40,7 +43,7 @@ public AutoTranslateService.TranslationRun getModel() {
         return autoTranslateService.getTranslationRuns().stream()
                 .filter(run -> id.equals(run.id))
                 .findFirst()
-                .orElseThrow(() -> new IllegalArgumentException("no such run: " + id));
+                .orElse(null);
     }
 
     public String cancel() {

diff --git a/aem/core/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateService.java b/aem/core/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateService.java
@@ -1,10 +1,14 @@
 package com.composum.ai.aem.core.impl.autotranslate;
 
+import static org.apache.commons.lang3.StringUtils.startsWith;
+
 import java.util.List;
+import java.util.regex.Pattern;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.resource.LoginException;
 import org.apache.sling.api.resource.PersistenceException;
 import org.apache.sling.api.resource.Resource;
@@ -91,9 +95,23 @@ public String toString() {
     }
 
     static abstract class TranslationPage {
+        private final static Pattern IMAGE_VIDEO_PATTERN =
+                Pattern.compile("\\.(png|jpg|jpeg|gif|svg|mp3|mov|mp4)(/|$)", Pattern.CASE_INSENSITIVE);
+
         public String pagePath;
         public String status;
         public AutoPageTranslateService.Stats stats;
+
+        public String editorUrl() {
+            if (startsWith(pagePath,  "/content/dam")) {
+                if (IMAGE_VIDEO_PATTERN.matcher(pagePath).find()) {
+                    return "/assetdetails.html" + pagePath;
+                }
+                return "/editor.html" + pagePath;
+            } else {
+                return "/editor.html" + pagePath + ".html";
+            }
+        }
     }
 
 }
diff --git a/...e/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateServiceImpl.java b/...e/src/main/java/com/composum/ai/aem/core/impl/autotranslate/AutoTranslateServiceImpl.java
@@ -136,7 +136,7 @@ public TranslationRun startTranslation(
                 .map(r -> new TranslationPageImpl(r.getPath()))
                 .collect(Collectors.toList());
         run.waituntil = System.currentTimeMillis() + 1000; // when triggered during live copy creation.
-        run.status = "scheduled";
+        run.status = "queued";
         run.user = resourceResolver.getUserID();
         run.configuration = configuration;
         stateService.getTranslationRuns().add(run);
@@ -281,6 +281,7 @@ public static class TranslationPageImpl extends TranslationPage {
         public TranslationPageImpl(String resourcePath) {
             this.resourcePath = resourcePath;
             pagePath = ResourceUtil.getParent(resourcePath); // remove jcr:content
+            this.status = "queued";
         }
     }
 

diff --git a/aem/ui.apps/src/main/content/jcr_root/apps/composum-ai/components/autotranslate/run/run.html b/aem/ui.apps/src/main/content/jcr_root/apps/composum-ai/components/autotranslate/run/run.html
@@ -16,9 +16,15 @@
                     <sly data-sly-use.runmodel="com.composum.ai.aem.core.impl.autotranslate.AutoTranslateRunModel"
                          data-sly-set.model="${runmodel.model}">
                         <section class="coral-Form-fieldset"><h2
-                                    class="coral-Form-fieldset-legend">Translation run details</h2>
+                                    class="coral-Form-fieldset-legend">Translation run details for ${model.rootPath}</h2>
                             <div class="coral-Form coral-Form--vertical foundation-form">
                                 <input type="hidden" name="_charset_" value="utf-8">
+                                <coral-alert variant="error" data-sly-test="${!model}">
+                                    <coral-alert-header>EXPIRED</coral-alert-header>
+                                    <coral-alert-content>
+                                        The data about this translation run is no longer available.
+                                    </coral-alert-content>
+                                </coral-alert>
                                 <span class="foundation-field-readonly coral-Form-fieldwrapper">
                                     <span class="coral-Form-field foundation-layout-util-breakword">
                                         <strong>Status:</strong> ${model.status}
@@ -48,6 +54,7 @@
                                        the number of paths that were found and relocated to a copy with the appropriate language
                                        versus the total number of paths found in the page.
                                        For modified pages a link into the editor is provided.
+                                       Please refresh this page to update the status.
                                        </span>
                                </span>
                                 <div class="coral-Form-fieldwrapper">
@@ -77,10 +84,11 @@
                                                 </sly>
                                             </td>
                                             <td is="coral-table-cell">
-                                                <a data-sly-unwrap="${!item.stats.hasChanges}"
+                                                <a
                                                    target="_blank"
-                                                   href="/editor.html${item.pagePath}.html">${item.pagePath}
-                                                </a></td>
+                                                   href="${item.editorUrl}">${item.pagePath}
+                                                </a>
+                                            </td>
                                         </tr>
                                         </tbody>
                                     </table>