From c663c6888e5321ada30ee55b7e3d4a09f9c8d0a3 Mon Sep 17 00:00:00 2001 From: Belal Ansari Date: Wed, 22 Apr 2026 19:51:29 +0530 Subject: [PATCH] ACS-10652 fixing Test Failures in ACS (#1234) --- .../registry/CombinedTransformConfig.java | 158 ++++++++++++++---- .../transform/registry/DeferredOverride.java | 49 ++++++ .../registry/CombinedTransformConfigTest.java | 21 +++ .../OverrideTransformConfigTests.java | 102 +++++++++-- 4 files changed, 288 insertions(+), 42 deletions(-) create mode 100644 model/src/main/java/org/alfresco/transform/registry/DeferredOverride.java diff --git a/model/src/main/java/org/alfresco/transform/registry/CombinedTransformConfig.java b/model/src/main/java/org/alfresco/transform/registry/CombinedTransformConfig.java index 56128bcb..0b1f35a6 100644 --- a/model/src/main/java/org/alfresco/transform/registry/CombinedTransformConfig.java +++ b/model/src/main/java/org/alfresco/transform/registry/CombinedTransformConfig.java @@ -30,11 +30,14 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.StringJoiner; import java.util.function.Function; import java.util.stream.Collectors; +import org.springframework.util.CollectionUtils; + import org.alfresco.transform.config.AddSupported; import org.alfresco.transform.config.OverrideSupported; import org.alfresco.transform.config.RemoveSupported; @@ -71,6 +74,7 @@ public class CombinedTransformConfig private final Map> combinedTransformOptions = new HashMap<>(); private List> combinedTransformers = new ArrayList<>(); private final Defaults defaults = new Defaults(); + private final List deferredOverrides = new ArrayList<>(); public static void combineAndRegister(TransformConfig transformConfig, String readFrom, String baseUrl, AbstractTransformRegistry registry) @@ -86,6 +90,7 @@ public class CombinedTransformConfig combinedTransformOptions.clear(); combinedTransformers.clear(); defaults.clear(); + deferredOverrides.clear(); } public void addTransformConfig(List> transformConfigList, AbstractTransformRegistry registry) @@ -101,7 +106,7 @@ public class CombinedTransformConfig removeSupported(transformConfig.getRemoveSupported(), readFrom, registry); addSupported(transformConfig.getAddSupported(), readFrom, registry); - overrideSupported(transformConfig.getOverrideSupported(), readFrom, registry); + storeOverridesForDeferredProcessing(transformConfig.getOverrideSupported(), readFrom); // Add transform options and transformers from the new transformConfig transformConfig.getTransformOptions().forEach(combinedTransformOptions::put); @@ -218,23 +223,83 @@ public class CombinedTransformConfig })); } - private void overrideSupported(Set overrideSupportedSet, String readFrom, AbstractTransformRegistry registry) + /** + * Store overrides for deferred processing. Overrides are applied AFTER wildcard generation in the combineTransformerConfig() method, ensuring that pipeline transformers have their supportedSourceAndTargetList populated before overrides are applied. + * + * @param overrideSupportedSet + * the set of overrides to store + * @param readFrom + * where the overrides were read from + */ + private void storeOverridesForDeferredProcessing(Set overrideSupportedSet, String readFrom) { - processSupported(overrideSupportedSet, readFrom, registry, "overrideSupported", - (leftOver, overrideSupported) -> combinedTransformers.stream().map(Origin::get).filter(transformer -> transformer.getTransformerName().equals(overrideSupported.getTransformerName())).forEach(transformerWithName -> { - Set supportedSourceAndTargetList = transformerWithName.getSupportedSourceAndTargetList(); - SupportedSourceAndTarget existingSupported = getExistingSupported( - supportedSourceAndTargetList, - overrideSupported.getSourceMediaType(), overrideSupported.getTargetMediaType()); - if (existingSupported != null) - { - supportedSourceAndTargetList.remove(existingSupported); - existingSupported.setMaxSourceSizeBytes(overrideSupported.getMaxSourceSizeBytes()); - existingSupported.setPriority(overrideSupported.getPriority()); - supportedSourceAndTargetList.add(existingSupported); - leftOver.remove(overrideSupported); - } - })); + if (!CollectionUtils.isEmpty(overrideSupportedSet)) + { + overrideSupportedSet.forEach(override -> deferredOverrides.add(new DeferredOverride(override, readFrom))); + } + } + + /** + * Apply all stored overrides AFTER wildcard generation. This ensures that pipeline and failover transformers have their supportedSourceAndTargetList populated before overrides are applied. + * + * @param registry + * used for logging + */ + private void applyDeferredOverrides(AbstractTransformRegistry registry) + { + if (CollectionUtils.isEmpty(deferredOverrides)) + { + return; + } + + Map> leftoverBySource = new HashMap<>(); + for (DeferredOverride deferredOverride : deferredOverrides) + { + OverrideSupported override = deferredOverride.getOverrideSupported(); + String readFrom = deferredOverride.getReadFrom(); + + List matchedTransformers = combinedTransformers.stream() + .map(Origin::get) + .filter(transformer -> transformer.getTransformerName().equals(override.getTransformerName())) + .collect(Collectors.toList()); + if (matchedTransformers.isEmpty()) + { + leftoverBySource.computeIfAbsent(readFrom, k -> new HashSet<>()).add(override); + continue; + } + if (matchedTransformers.size() > 1) + { + throw new IllegalStateException("Multiple transformers found for " + readFrom + " with name: " + override.getTransformerName() + ". This should not be possible as removeInvalidTransformers should have removed duplicates."); + } + + Set supportedList = matchedTransformers.get(0).getSupportedSourceAndTargetList(); + Optional existingSupportedOpt = supportedList.stream() + .filter(supported -> supported.getSourceMediaType().equals(override.getSourceMediaType()) && + supported.getTargetMediaType().equals(override.getTargetMediaType())) + .findFirst(); + + if (existingSupportedOpt.isPresent()) + { + SupportedSourceAndTarget existingSupported = existingSupportedOpt.get(); + supportedList.remove(existingSupported); + if (override.getMaxSourceSizeBytes() != null) + { + existingSupported.setMaxSourceSizeBytes(override.getMaxSourceSizeBytes()); + } + if (override.getPriority() != null) + { + existingSupported.setPriority(override.getPriority()); + } + supportedList.add(existingSupported); + } + else + { + leftoverBySource.computeIfAbsent(readFrom, k -> new HashSet<>()).add(override); + } + } + // Warn about overrides that didn't match anything + leftoverBySource.forEach((readFrom, leftOvers) -> logWarn(leftOvers, readFrom, registry, "overrideSupported")); + deferredOverrides.clear(); } private SupportedSourceAndTarget getExistingSupported(Set supportedSourceAndTargetList, @@ -250,8 +315,9 @@ public class CombinedTransformConfig { removeInvalidTransformers(registry); sortTransformers(registry); - applyDefaults(); addWildcardSupportedSourceAndTarget(registry); + applyDefaults(); + applyDeferredOverrides(registry); removePipelinesWithUnsupportedTransforms(registry); setCoreVersionOnCombinedMultiStepTransformers(); } @@ -584,28 +650,51 @@ public class CombinedTransformConfig } /** - * Applies priority and size defaults. Must be called before {@link #addWildcardSupportedSourceAndTarget(AbstractTransformRegistry)} as it uses the priority value. + * Applies priority and size defaults to a SupportedSourceAndTarget entry. + */ + private SupportedSourceAndTarget applyDefaultsToSupportedSourceAndTarget( + SupportedSourceAndTarget supportedSourceAndTarget, + String transformerName, + Set supportedDefaultTransformerNames, + Defaults defaults) + { + Integer priority = supportedSourceAndTarget.getPriority(); + Long maxSourceSizeBytes = supportedSourceAndTarget.getMaxSourceSizeBytes(); + String sourceMediaType = supportedSourceAndTarget.getSourceMediaType(); + if (defaults.valuesUnset(priority, maxSourceSizeBytes)) + { + supportedSourceAndTarget.setPriority(defaults.getPriority(transformerName, sourceMediaType, priority)); + supportedSourceAndTarget.setMaxSourceSizeBytes(defaults.getMaxSourceSizeBytes(transformerName, sourceMediaType, maxSourceSizeBytes)); + } + if (supportedDefaultTransformerNames.contains(transformerName)) + { + supportedSourceAndTarget.setPriority(defaults.getPriority(transformerName, sourceMediaType, null)); + supportedSourceAndTarget.setMaxSourceSizeBytes(defaults.getMaxSourceSizeBytes(transformerName, sourceMediaType, null)); + } + return supportedSourceAndTarget; + } + + /** + * Applies priority and size defaults to supported source/target entries. + *

+ * Previously, this method was called before {@link #addWildcardSupportedSourceAndTarget(AbstractTransformRegistry)} because it relied on the priority value. As of MNT-25426, it is now called after wildcard generation, ensuring that pipeline transformers also receive the correct defaults. */ private void applyDefaults() { + Set supportedDefaultTransformerNames = defaults.getSupportedDefaults() + .stream() + .map(SupportedDefaults::getTransformerName) + .collect(toSet()); + combinedTransformers.stream() .map(Origin::get) .forEach(transformer -> { transformer.setSupportedSourceAndTargetList( - transformer.getSupportedSourceAndTargetList().stream().map(supportedSourceAndTarget -> { - Integer priority = supportedSourceAndTarget.getPriority(); - Long maxSourceSizeBytes = supportedSourceAndTarget.getMaxSourceSizeBytes(); - if (defaults.valuesUnset(priority, maxSourceSizeBytes)) - { - String transformerName = transformer.getTransformerName(); - String sourceMediaType = supportedSourceAndTarget.getSourceMediaType(); - supportedSourceAndTarget.setPriority(defaults.getPriority(transformerName, sourceMediaType, priority)); - supportedSourceAndTarget.setMaxSourceSizeBytes(defaults.getMaxSourceSizeBytes(transformerName, sourceMediaType, maxSourceSizeBytes)); - } - return supportedSourceAndTarget; - }).collect(toSet())); + transformer.getSupportedSourceAndTargetList() + .stream() + .map(supportedSourceAndTarget -> applyDefaultsToSupportedSourceAndTarget(supportedSourceAndTarget, transformer.getTransformerName(), supportedDefaultTransformerNames, defaults)) + .collect(toSet())); }); - defaults.clear(); } @@ -865,4 +954,9 @@ public class CombinedTransformConfig { return combinedTransformers.stream().collect(Collectors.toMap(origin -> origin.get().getTransformerName(), origin -> origin)); } + + List getDeferredOverrides() + { + return deferredOverrides; + } } diff --git a/model/src/main/java/org/alfresco/transform/registry/DeferredOverride.java b/model/src/main/java/org/alfresco/transform/registry/DeferredOverride.java new file mode 100644 index 00000000..bcf1eb74 --- /dev/null +++ b/model/src/main/java/org/alfresco/transform/registry/DeferredOverride.java @@ -0,0 +1,49 @@ +/* + * #%L + * Alfresco Transform Model + * %% + * Copyright (C) 2026 Alfresco Software Limited + * %% + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Lesser Public License for more details. + * + * You should have received a copy of the GNU General Lesser Public + * License along with this program. If not, see + * . + * #L% + */ +package org.alfresco.transform.registry; + +import org.alfresco.transform.config.OverrideSupported; + +/** + * Holds override information for deferred processing after wildcard generation. + */ +class DeferredOverride +{ + private final OverrideSupported overrideSupported; + private final String readFrom; + + public DeferredOverride(OverrideSupported overrideSupported, String readFrom) + { + this.overrideSupported = overrideSupported; + this.readFrom = readFrom; + } + + public OverrideSupported getOverrideSupported() + { + return overrideSupported; + } + + public String getReadFrom() + { + return readFrom; + } +} diff --git a/model/src/test/java/org/alfresco/transform/registry/CombinedTransformConfigTest.java b/model/src/test/java/org/alfresco/transform/registry/CombinedTransformConfigTest.java index 2f6b019c..c037d206 100644 --- a/model/src/test/java/org/alfresco/transform/registry/CombinedTransformConfigTest.java +++ b/model/src/test/java/org/alfresco/transform/registry/CombinedTransformConfigTest.java @@ -39,6 +39,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import org.junit.jupiter.api.Test; +import org.alfresco.transform.config.OverrideSupported; import org.alfresco.transform.config.SupportedSourceAndTarget; import org.alfresco.transform.config.TransformConfig; import org.alfresco.transform.config.TransformStep; @@ -300,6 +301,26 @@ public class CombinedTransformConfigTest assertEquals(0, config.buildTransformConfig().getTransformOptions().size()); } + @Test + public void testClearAlsoRemovesDeferredOverrides() + { + // Add a config with an overrideSupported entry to populate deferredOverrides + TransformConfig overrideConfig = TransformConfig.builder() + .withOverrideSupported(ImmutableSet.of( + OverrideSupported.builder() + .withTransformerName("pipeline1") + .withSourceMediaType("mimetype/a") + .withTargetMediaType("mimetype/b") + .withPriority(99) + .build())) + .build(); + config.addTransformConfig(overrideConfig, READ_FROM_B, BASE_URL_B, registry); + + assertEquals(1, config.getDeferredOverrides().size()); + config.clear(); + assertEquals(0, config.getDeferredOverrides().size()); + } + @Test public void testCombineTransformerConfigNoOp() { diff --git a/model/src/test/java/org/alfresco/transform/registry/OverrideTransformConfigTests.java b/model/src/test/java/org/alfresco/transform/registry/OverrideTransformConfigTests.java index 14050189..e0c731d5 100644 --- a/model/src/test/java/org/alfresco/transform/registry/OverrideTransformConfigTests.java +++ b/model/src/test/java/org/alfresco/transform/registry/OverrideTransformConfigTests.java @@ -25,6 +25,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.HashSet; +import java.util.List; import java.util.Set; import com.google.common.collect.ImmutableList; @@ -37,6 +38,7 @@ import org.alfresco.transform.config.RemoveSupported; import org.alfresco.transform.config.SupportedDefaults; import org.alfresco.transform.config.SupportedSourceAndTarget; import org.alfresco.transform.config.TransformConfig; +import org.alfresco.transform.config.TransformStep; import org.alfresco.transform.config.Transformer; /** @@ -54,10 +56,12 @@ public class OverrideTransformConfigTests .withTargetMediaType("mimetype/b") .build(); - private final SupportedSourceAndTarget supported_A2B__40 = SupportedSourceAndTarget.builder() + // Override result: priority overridden to 40; maxSourceSizeBytes not in override, retained as -1 (unlimited default from original) + private final SupportedSourceAndTarget supported_A2B_default_40 = SupportedSourceAndTarget.builder() .withSourceMediaType("mimetype/a") .withTargetMediaType("mimetype/b") .withPriority(40) + .withMaxSourceSizeBytes(-1L) .build(); private final SupportedSourceAndTarget supported_C2D = SupportedSourceAndTarget.builder() @@ -79,10 +83,12 @@ public class OverrideTransformConfigTests .withPriority(23) .build(); - private final SupportedSourceAndTarget supported_X2Y_200 = SupportedSourceAndTarget.builder() + // Override result: maxSourceSizeBytes overridden to 200; priority not in override, retained as 23 from original entry + private final SupportedSourceAndTarget supported_X2Y_200_23 = SupportedSourceAndTarget.builder() .withSourceMediaType("mimetype/x") .withTargetMediaType("mimetype/y") .withMaxSourceSizeBytes(200L) + .withPriority(23) .build(); private final TransformConfig transformConfig_A2B_X2Y_100_23 = TransformConfig.builder() @@ -399,13 +405,13 @@ public class OverrideTransformConfigTests .withSourceMediaType("mimetype/c") .withTargetMediaType("mimetype/d") .build(), - OverrideSupported.builder() // size default -> 200 and priority default -> 100 + OverrideSupported.builder() // priority: Default -> 40, maxSourceSizeBytes: not in override, retained as -1 (unlimited default) .withTransformerName("1") .withSourceMediaType("mimetype/a") .withTargetMediaType("mimetype/b") .withPriority(40) .build(), - OverrideSupported.builder() // size 100 -> 200 and change priority to default + OverrideSupported.builder() // maxSourceSizeBytes: 100 -> 200, priority: not in override, retained as 23 from original entry .withTransformerName("1") .withSourceMediaType("mimetype/x") .withTargetMediaType("mimetype/y") @@ -416,7 +422,7 @@ public class OverrideTransformConfigTests .withSourceMediaType("mimetype/a") .withTargetMediaType("mimetype/d") .build())) - // OverrideSupported values with missing fields are defaults, so no test values here + // overrideSupported uses patch semantics: fields not specified in the override are retained from the existing entry .build(); String expectedWarnMessage = "Unable to process \"overrideSupported\": [" + @@ -424,14 +430,90 @@ public class OverrideTransformConfigTests "{\"transformerName\": \"bad\", \"sourceMediaType\": \"mimetype/a\", \"targetMediaType\": \"mimetype/d\"}]. " + "Read from readFromB"; ImmutableSet expectedSupported = ImmutableSet.of( - supported_X2Y_200, - supported_A2B__40); + supported_X2Y_200_23, + supported_A2B_default_40); String expectedToString = "[" + - "{\"sourceMediaType\": \"mimetype/a\", \"targetMediaType\": \"mimetype/b\", \"priority\": \"40\"}, " + - "{\"sourceMediaType\": \"mimetype/x\", \"targetMediaType\": \"mimetype/y\", \"maxSourceSizeBytes\": \"200\"}" + + "{\"sourceMediaType\": \"mimetype/a\", \"targetMediaType\": \"mimetype/b\", \"maxSourceSizeBytes\": \"-1\", \"priority\": \"40\"}, " + + "{\"sourceMediaType\": \"mimetype/x\", \"targetMediaType\": \"mimetype/y\", \"maxSourceSizeBytes\": \"200\", \"priority\": \"23\"}" + "]"; - addTransformConfig(secondConfig, expectedWarnMessage, expectedSupported, expectedToString); + config.addTransformConfig(secondConfig, READ_FROM_B, BASE_URL_B, registry); + config.combineTransformerConfig(registry); + + assertEquals(1, registry.warnMessages.size()); + assertEquals(expectedWarnMessage, registry.warnMessages.get(0)); + + Set supportedSourceAndTargetList = config.buildTransformConfig().getTransformers().get(0).getSupportedSourceAndTargetList(); + assertEquals(expectedSupported, supportedSourceAndTargetList); + assertEquals(expectedToString, supportedSourceAndTargetList.toString()); + } + + @Test + public void testDeferredOverrideForPipelineTransformer() + { + // Add step transformers first + Transformer step1 = Transformer.builder() + .withTransformerName("step1") + .withSupportedSourceAndTargetList(Set.of( + SupportedSourceAndTarget.builder() + .withSourceMediaType("mimetype/document") + .withTargetMediaType("mimetype/pdf") + .build())) + .build(); + + Transformer step2 = Transformer.builder() + .withTransformerName("step2") + .withSupportedSourceAndTargetList(Set.of( + SupportedSourceAndTarget.builder() + .withSourceMediaType("mimetype/pdf") + .withTargetMediaType("mimetype/image") + .build())) + .build(); + + // Add pipeline transformer + Transformer pipelineTransformer = Transformer.builder() + .withTransformerName("pipeline1") + .withTransformerPipeline(List.of( + new TransformStep("step1", "mimetype/pdf"), + new TransformStep("step2", null))) + .build(); + + TransformConfig pipelineConfig = TransformConfig.builder() + .withTransformers(List.of(step1, step2, pipelineTransformer)) + .build(); + + config.addTransformConfig(pipelineConfig, READ_FROM_A, BASE_URL_A, registry); + + // Add override for pipeline transformer + OverrideSupported override = OverrideSupported.builder() + .withTransformerName("pipeline1") + .withSourceMediaType("mimetype/document") + .withTargetMediaType("mimetype/image") + .withPriority(40) + .build(); + + TransformConfig overrideConfig = TransformConfig.builder() + .withOverrideSupported(Set.of(override)) + .build(); + + config.addTransformConfig(overrideConfig, READ_FROM_B, BASE_URL_B, registry); + + // Combine configs + config.combineTransformerConfig(registry); + + // Assert override applied + List transformers = config.buildTransformConfig().getTransformers(); + assertTrue(!transformers.isEmpty(), "Pipeline transformer should exist after valid setup"); + Set supportedList = transformers.stream() + .filter(t -> "pipeline1".equals(t.getTransformerName())) + .findFirst() + .orElseThrow() + .getSupportedSourceAndTargetList(); + + boolean found = supportedList.stream().anyMatch(s -> "mimetype/document".equals(s.getSourceMediaType()) && + "mimetype/image".equals(s.getTargetMediaType()) && + s.getPriority() == 40); + assertTrue(found, "Deferred override for pipeline transformer should be applied after wildcard generation"); } private void addTransformConfig_A2B_X2Y_100_23()