diff --git a/source/java/org/alfresco/repo/content/transform/DoubleMap.java b/source/java/org/alfresco/repo/content/transform/DoubleMap.java index d48e5c6d1f..6c12af9d77 100644 --- a/source/java/org/alfresco/repo/content/transform/DoubleMap.java +++ b/source/java/org/alfresco/repo/content/transform/DoubleMap.java @@ -19,6 +19,7 @@ package org.alfresco.repo.content.transform; import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; /** @@ -138,4 +139,26 @@ public class DoubleMap map.put(key2, t); } + + @Override + public String toString() + { + StringBuilder sb = new StringBuilder(); + for (Entry> outerEntry: mapMap.entrySet()) + { + for (Entry innerEntry: outerEntry.getValue().entrySet()) + { + if (sb.length() > 0) + { + sb.append("\n"); + } + sb.append(outerEntry.getKey()). + append(", "). + append(innerEntry.getKey()). + append(" = "). + append(innerEntry.getValue()); + } + } + return sb.toString(); + } } diff --git a/source/java/org/alfresco/repo/content/transform/TransformerConfigLimits.java b/source/java/org/alfresco/repo/content/transform/TransformerConfigLimits.java index 0aa0246428..78c69d3c24 100644 --- a/source/java/org/alfresco/repo/content/transform/TransformerConfigLimits.java +++ b/source/java/org/alfresco/repo/content/transform/TransformerConfigLimits.java @@ -27,6 +27,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.Map; import java.util.Set; +import java.util.Map.Entry; import java.util.concurrent.ConcurrentHashMap; import org.alfresco.service.cmr.repository.MimetypeService; @@ -48,7 +49,7 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor // Entries are added lower down the hierarchy when a search takes place. private Map>> limitsMap; - // The 'use' value that had properties defined, including null for the default. + // The 'use' value that had properties defined, including ANY for the default. private Set uses; public TransformerConfigLimits(TransformerProperties transformerProperties, MimetypeService mimetypeService) @@ -57,7 +58,7 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor } /** - * Sets the transformer limits created from system properties. + * Sets the transformer limits created from properties. */ private void setLimits(TransformerProperties transformerProperties, MimetypeService mimetypeService) { @@ -79,24 +80,39 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor // Populate the limitsMap for each 'use'. for (String use: uses) { - // Add the system wide default just in case it is not included, as we always need this one - TransformationOptionLimits limits = getOrCreateTransformerOptionLimits(DEFAULT_TRANSFORMER, ANY, ANY, use); - Collection properties = getPropertiesForUse(use, allUseMap); - for (int pass=0; pass<=3; pass++) + setLimits(use, properties); + } + logger.debug(this); + } + + /** + * Sets the transformer limits for a single use from properties. Method extracted so that + * it is possible to write a simpler unit test, that changes to the order of the + * properties. The order changed between Java 6, 7 and 8, resulting in MNT-14295. The original + * outer method cannot be used as it creates the list from a map (allUseMap) that it also + * creates and the order of values from that map cannot be controlled from a test. + */ + void setLimits(String use, Collection properties) + { + // Add the system wide default just in case it is not included, as we always need this one + getOrCreateTransformerOptionLimits(DEFAULT_TRANSFORMER, ANY, ANY, use); + + TransformationOptionLimits limits; + for (int pass=0; pass<=3; pass++) + { + for (TransformerSourceTargetSuffixValue property: properties) { - for (TransformerSourceTargetSuffixValue property: properties) + int origLevel = getLevel(property.transformerName, property.sourceMimetype); + if (pass == origLevel) { - int origLevel = getLevel(property.transformerName, property.sourceMimetype); - if (pass == origLevel) - { - String transformerName = (property.transformerName == null) - ? DEFAULT_TRANSFORMER : property.transformerName; - limits = getOrCreateTransformerOptionLimits(transformerName, - property.sourceMimetype, property.targetMimetype, use); - setTransformationLimitsFromProperties(limits, property.value, property.suffix); - debug("V", transformerName, property.sourceMimetype, property.targetMimetype, use, limits); - } + logger.debug(property); + String transformerName = (property.transformerName == null) + ? DEFAULT_TRANSFORMER : property.transformerName; + limits = getOrCreateTransformerOptionLimits(transformerName, + property.sourceMimetype, property.targetMimetype, use); + setTransformationLimitsFromProperties(limits, property.value, property.suffix); + debug("V", transformerName, property.sourceMimetype, property.targetMimetype, use, limits); } } } @@ -205,6 +221,29 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor return limits; } + + @Override + public String toString() + { + StringBuilder sb = new StringBuilder(); + for (Entry>> useEntry: limitsMap.entrySet()) + { + for (Entry> transformerEntry: useEntry.getValue().entrySet()) + { + if (sb.length() > 0) + { + sb.append("\n"); + } + sb.append(useEntry.getKey()). + append(", "). + append(transformerEntry.getKey()). + append(" =>\n"). + append(transformerEntry.getValue()); + } + } + return sb.toString(); + } + /** * Creates a new TransformationOptionLimits for the use, transformer and mimetype combination, * defaulting values from lower levels. @@ -259,37 +298,32 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor long l = Long.parseLong(value); if (suffix == TransformerConfig.MAX_SOURCE_SIZE_K_BYTES) { - limits.setReadLimitKBytes(-1); limits.setMaxSourceSizeKBytes(l); } else if (suffix == TransformerConfig.TIMEOUT_MS) { - limits.setReadLimitTimeMs(-1); limits.setTimeoutMs(l); } else if (suffix == TransformerConfig.MAX_PAGES) { - limits.setPageLimit(-1); limits.setMaxPages((int)l); } else if (suffix == TransformerConfig.READ_LIMIT_K_BYTES) { - limits.setMaxSourceSizeKBytes(-1); limits.setReadLimitKBytes(l); } else if (suffix == TransformerConfig.READ_LIMIT_TIME_MS) { - limits.setTimeoutMs(-1); limits.setReadLimitTimeMs(l); } else // if (suffix == TransformerConfig.PAGE_LIMIT) { - limits.setMaxPages(-1); limits.setPageLimit((int)l); } } - private void debug(String msg, String transformerName, String sourceMimetype, String targetMimetype, String use, TransformationOptionLimits limits) + private void debug(String msg, String transformerName, String sourceMimetype, + String targetMimetype, String use, TransformationOptionLimits limits) { if (logger.isDebugEnabled()) { @@ -312,10 +346,9 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor sb.append('.'); sb.append(use); sb.append('='); - sb.append(limits.getMaxSourceSizeKBytes()); + sb.append(limits); } String line = sb.toString(); -// System.err.println(line); logger.debug(line); } } diff --git a/source/java/org/alfresco/repo/content/transform/TransformerPropertyNameExtractor.java b/source/java/org/alfresco/repo/content/transform/TransformerPropertyNameExtractor.java index 7370fa37e4..cab1556bac 100644 --- a/source/java/org/alfresco/repo/content/transform/TransformerPropertyNameExtractor.java +++ b/source/java/org/alfresco/repo/content/transform/TransformerPropertyNameExtractor.java @@ -348,7 +348,7 @@ class TransformerSourceTargetSuffixKey ? "" : TransformerConfig.EXTENSIONS+sourceExt+'.'+targetExt)+ suffix+ - (use == null ? "" : TransformerConfig.USE + use); + (use == null || ANY.equals(use) ? "" : TransformerConfig.USE + use); } @Override diff --git a/source/java/org/alfresco/service/cmr/repository/TransformationOptionLimits.java b/source/java/org/alfresco/service/cmr/repository/TransformationOptionLimits.java index 4cdcef01cd..b16b5573e0 100644 --- a/source/java/org/alfresco/service/cmr/repository/TransformationOptionLimits.java +++ b/source/java/org/alfresco/service/cmr/repository/TransformationOptionLimits.java @@ -221,7 +221,7 @@ public class TransformationOptionLimits implements Serializable kbytes.append(sb, OPT_MAX_SOURCE_SIZE_K_BYTES, OPT_READ_LIMIT_K_BYTES); pages.append(sb, OPT_MAX_PAGES, OPT_PAGE_LIMIT); sb.append("}"); - return sb.length() == 2 ? "" : sb.toString(); + return sb.length() == 2 ? "no limits" : sb.toString(); } /** diff --git a/source/test-java/org/alfresco/repo/content/transform/TransformerConfigLimitsTest.java b/source/test-java/org/alfresco/repo/content/transform/TransformerConfigLimitsTest.java index d0be79092f..661cf83680 100644 --- a/source/test-java/org/alfresco/repo/content/transform/TransformerConfigLimitsTest.java +++ b/source/test-java/org/alfresco/repo/content/transform/TransformerConfigLimitsTest.java @@ -18,10 +18,15 @@ */ package org.alfresco.repo.content.transform; +import static org.alfresco.repo.content.transform.TransformerConfig.ANY; import static org.alfresco.repo.content.transform.TransformerPropertyNameExtractorTest.mockMimetypes; import static org.alfresco.repo.content.transform.TransformerPropertyNameExtractorTest.mockProperties; import static org.junit.Assert.assertEquals; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + import org.alfresco.service.cmr.repository.MimetypeService; import org.alfresco.service.cmr.repository.TransformationOptionLimits; import org.junit.Before; @@ -61,6 +66,44 @@ public class TransformerConfigLimitsTest "text/plain", "txt"); } + private TransformerConfigLimits newTransformerConfigLimits(String[][] data) + { + // Mock up an extractor with no properties + mockProperties(transformerProperties); + TransformerConfigLimits extractor = new TransformerConfigLimits(transformerProperties, mimetypeService); + + // Work out the uses, but keep the order + List uses = new ArrayList<>(); + for (int i=0; i < data.length; i++) + { + if (!uses.contains(data[i][4])) + { + uses.add(data[i][4]); + } + } + + // Load data for each use - keep the order + for (String use: uses) + { + Collection properties = new ArrayList<>(); + for (int i=0; i < data.length; i++) + { + if (use.equals(data[i][4])) + { + properties.add(new TransformerSourceTargetSuffixValue( + data[i][0], data[i][1], data[i][2], data[i][3], data[i][4], data[i][5], + mimetypeService)); + } + } + if (properties.size() > 0) + { + extractor.setLimits(use, properties); + } + } + + return extractor; + } + @Test // A value is specified for a transformer and mimetypes public void transformerMimetypesTest() @@ -187,6 +230,58 @@ public class TransformerConfigLimitsTest assertEquals(1, txtToPngLimits.getPageLimit()); } + // MNT-14295 With Java 7 the order in which properties were supplied changed from + // what happen with Java 6 and happens with 8. When combined with a bug to do with + // always clearing the max value when setting a limit or the limit when setting + // the max value, the initial map of TransformerConfigLimits would be different. + // Java 7 was used as the runtime for 4.2 and the 5.0 but Java 8 became the default + // from 5.0.1. + // None of the other unit tests in this class failed as a none of them provided + // both max and limit values. + @Test + public void propertyOrderJava7Test() + { + // Load extractor with properties in the order seen when running Java 7 + // "content.transformer.default.timeoutMs", "120000", + // "content.transformer.default.readLimitTimeMs", "-1"); + extractor = newTransformerConfigLimits(new String[][] + { + {"transformer.default", ANY, ANY, ".timeoutMs", ANY, "120000"}, + {"transformer.default", ANY, ANY, ".readLimitTimeMs", ANY, "-1"} + }); + + TransformationOptionLimits limits = extractor.getLimits(transformer1, "text/plain", "image/png", null); + assertEquals(120000L, limits.getTimeoutMs()); + assertEquals(-1L, limits.getReadLimitTimeMs()); + } + + @Test + public void propertyOrderJava8or6Test() + { + // Load extractor with properties in the order seen when running Java 8 + // "content.transformer.default.timeoutMs", "120000", + // "content.transformer.default.readLimitTimeMs", "-1"); + extractor = newTransformerConfigLimits(new String[][] + { + {"transformer.default", ANY, ANY, ".readLimitTimeMs", ANY, "-1"}, + {"transformer.default", ANY, ANY, ".timeoutMs", ANY, "120000"} + }); + + TransformationOptionLimits limits = extractor.getLimits(transformer1, "text/plain", "image/png", null); + assertEquals(120000L, limits.getTimeoutMs()); + assertEquals(-1L, limits.getReadLimitTimeMs()); + } + + @Test(expected=IllegalArgumentException.class) + public void bothMaxAndLimitSetTest() + { + mockProperties(transformerProperties, + "content.transformer.default.readLimitTimeMs", "990000", + "content.transformer.default.timeoutMs", "120000"); + + extractor = new TransformerConfigLimits(transformerProperties, mimetypeService); + } + // --------------------------------------- @Test