mirror of
https://github.com/Alfresco/alfresco-community-repo.git
synced 2025-10-08 14:51:49 +00:00
Merged 5.1.N (5.1.1) to HEAD (5.2)
123502 adavis: Merged 5.0.N (5.0.4) to 5.1.N (5.1.1) 123484 adavis: MNT-14295 TransformerConfigLimits Behavior Differs on Java 7 vs 8 - Fixed TransformerPropertyNameExtractor so that it discarded limit or max properties if its sibling max or limit value is supplied and has a lower value (i.e. it should be used). - Cleaned up TransformerConfigLimits.setTransformationLimitsFromProperties - This has been a problem since 4.2. - The problem only came to light with the switch to Java 8, as the order in which transformer properties are loaded changed. They were being loaded in MapEntry order. The problem did not show up in the unit tests, because they too were impacted by the MapEntry order and ironically loaded the properties in the same order. - Added unit tests that are supply both max an limit values. - Enhanced the debug and toString methods in order to trace the problem. git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@123711 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
This commit is contained in:
@@ -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<K1, K2, V>
|
||||
|
||||
map.put(key2, t);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString()
|
||||
{
|
||||
StringBuilder sb = new StringBuilder();
|
||||
for (Entry<K1, Map<K2, V>> outerEntry: mapMap.entrySet())
|
||||
{
|
||||
for (Entry<K2, V> 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();
|
||||
}
|
||||
}
|
||||
|
@@ -27,10 +27,12 @@ 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;
|
||||
import org.alfresco.service.cmr.repository.TransformationOptionLimits;
|
||||
import org.alfresco.service.cmr.repository.TransformationOptionPair;
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.apache.commons.logging.LogFactory;
|
||||
|
||||
@@ -43,12 +45,14 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor
|
||||
{
|
||||
private static Log logger = LogFactory.getLog(TransformerConfigLimits.class);
|
||||
|
||||
private static final String NOT_THROWN_MESSAGE = "Both max and limit should not be set.";
|
||||
|
||||
// Map using use, transformer, source mimetype and target mimetype to a set of limits.
|
||||
// Entries higher up the hierarchy are added so that values may be defaulted down.
|
||||
// Entries are added lower down the hierarchy when a search takes place.
|
||||
private Map<String, Map<String, DoubleMap<String, String, TransformationOptionLimits>>> 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<String> uses;
|
||||
|
||||
public TransformerConfigLimits(TransformerProperties transformerProperties, MimetypeService mimetypeService)
|
||||
@@ -57,7 +61,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,27 +83,63 @@ 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<TransformerSourceTargetSuffixValue> properties = getPropertiesForUse(use, allUseMap);
|
||||
|
||||
// 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 property1: properties)
|
||||
{
|
||||
int origLevel = getLevel(property.transformerName, property.sourceMimetype);
|
||||
int origLevel = getLevel(property1.transformerName, property1.sourceMimetype);
|
||||
if (pass == origLevel)
|
||||
{
|
||||
String transformerName = (property.transformerName == null)
|
||||
? DEFAULT_TRANSFORMER : property.transformerName;
|
||||
logger.debug(property1);
|
||||
String transformerName = (property1.transformerName == null)
|
||||
? DEFAULT_TRANSFORMER : property1.transformerName;
|
||||
limits = getOrCreateTransformerOptionLimits(transformerName,
|
||||
property.sourceMimetype, property.targetMimetype, use);
|
||||
setTransformationLimitsFromProperties(limits, property.value, property.suffix);
|
||||
debug("V", transformerName, property.sourceMimetype, property.targetMimetype, use, limits);
|
||||
property1.sourceMimetype, property1.targetMimetype, use);
|
||||
setTransformationLimitsFromProperties(limits, property1.value, property1.suffix);
|
||||
debug("V", transformerName, property1.sourceMimetype, property1.targetMimetype, use, limits);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
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<TransformerSourceTargetSuffixValue> 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)
|
||||
{
|
||||
int origLevel = getLevel(property.transformerName, property.sourceMimetype);
|
||||
if (pass == origLevel)
|
||||
{
|
||||
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 +245,29 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor
|
||||
return limits;
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public String toString()
|
||||
{
|
||||
StringBuilder sb = new StringBuilder();
|
||||
for (Entry<String, Map<String, DoubleMap<String, String, TransformationOptionLimits>>> useEntry: limitsMap.entrySet())
|
||||
{
|
||||
for (Entry<String, DoubleMap<String, String, TransformationOptionLimits>> 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.
|
||||
@@ -253,43 +316,50 @@ public class TransformerConfigLimits extends TransformerPropertyNameExtractor
|
||||
return level;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets a TransformationOptionLimits value. This may be a size in K bytes,
|
||||
* a time in ms or number of pages. It may also be a maximum or limit value.
|
||||
* When the maximum and limit are both set and the same, the maximum value
|
||||
* wins.
|
||||
*/
|
||||
private void setTransformationLimitsFromProperties(TransformationOptionLimits limits,
|
||||
String value, String suffix)
|
||||
{
|
||||
long l = Long.parseLong(value);
|
||||
if (suffix == TransformerConfig.MAX_SOURCE_SIZE_K_BYTES)
|
||||
long newValue = Long.parseLong(value);
|
||||
TransformationOptionPair optionPair =
|
||||
suffix == TransformerConfig.MAX_SOURCE_SIZE_K_BYTES ||
|
||||
suffix == TransformerConfig.READ_LIMIT_K_BYTES
|
||||
? limits.getKBytesPair()
|
||||
: suffix == TransformerConfig.TIMEOUT_MS ||
|
||||
suffix == TransformerConfig.READ_LIMIT_TIME_MS
|
||||
? limits.getTimePair()
|
||||
: limits.getPagesPair();
|
||||
|
||||
// If max rather than limit value
|
||||
if (suffix == TransformerConfig.MAX_SOURCE_SIZE_K_BYTES ||
|
||||
suffix == TransformerConfig.TIMEOUT_MS ||
|
||||
suffix == TransformerConfig.MAX_PAGES)
|
||||
{
|
||||
limits.setReadLimitKBytes(-1);
|
||||
limits.setMaxSourceSizeKBytes(l);
|
||||
long limit = optionPair.getLimit();
|
||||
if (limit < 0 || limit >= newValue)
|
||||
{
|
||||
optionPair.setLimit(-1, NOT_THROWN_MESSAGE);
|
||||
optionPair.setMax(newValue, NOT_THROWN_MESSAGE);
|
||||
}
|
||||
}
|
||||
else if (suffix == TransformerConfig.TIMEOUT_MS)
|
||||
else
|
||||
{
|
||||
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);
|
||||
long max = optionPair.getMax();
|
||||
if (max < 0 || max > newValue)
|
||||
{
|
||||
optionPair.setMax(-1, NOT_THROWN_MESSAGE);
|
||||
optionPair.setLimit(newValue, NOT_THROWN_MESSAGE);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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 +382,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);
|
||||
}
|
||||
}
|
||||
|
@@ -197,10 +197,14 @@ public abstract class TransformerPropertyNameExtractor
|
||||
}
|
||||
|
||||
/**
|
||||
* Optionally adds a new TransformerSourceTargetValue. If the supplied value is constructed from
|
||||
* from a mimetypes property a new value is always added and will replace any existing value. If
|
||||
* from an extensions property the value is only added if there is not a current value.
|
||||
* In other words properties that include mimetypes win out over those that use extensions.
|
||||
* Optionally adds a new TransformerSourceTargetValue and discards some when
|
||||
* there is another 'better' property.<p>
|
||||
*
|
||||
* If there is a property for the max value and another for the limit the lower
|
||||
* one wins. If equal, the max value wins.<p>
|
||||
*
|
||||
* If the supplied value is constructed from a mimetypes value it wins over one
|
||||
* constructed from an extensions property.
|
||||
*/
|
||||
private void addTransformerSourceTargetValue(
|
||||
Map<TransformerSourceTargetSuffixKey, TransformerSourceTargetSuffixValue> transformerSourceTargetSuffixValues,
|
||||
@@ -213,7 +217,35 @@ public abstract class TransformerPropertyNameExtractor
|
||||
|
||||
if (mimetypeProperty || !transformerSourceTargetSuffixValues.containsKey(key))
|
||||
{
|
||||
transformerSourceTargetSuffixValues.put(key, transformerSourceTargetSuffixValue);
|
||||
TransformerSourceTargetSuffixKey siblingKey = transformerSourceTargetSuffixValue.getSiblingKey();
|
||||
TransformerSourceTargetSuffixValue sibling = transformerSourceTargetSuffixValues.get(siblingKey);
|
||||
|
||||
boolean doAdd = true;
|
||||
if (sibling != null)
|
||||
{
|
||||
doAdd = false;
|
||||
long newValue = Long.parseLong(value);
|
||||
if (newValue > 0)
|
||||
{
|
||||
long oldValue = Long.parseLong(sibling.value);
|
||||
|
||||
// If max rather than limit value
|
||||
boolean isMax =
|
||||
suffix == TransformerConfig.MAX_SOURCE_SIZE_K_BYTES ||
|
||||
suffix == TransformerConfig.TIMEOUT_MS ||
|
||||
suffix == TransformerConfig.MAX_PAGES;
|
||||
if (oldValue < 0 || (isMax && oldValue >= newValue) || (!isMax && oldValue > newValue))
|
||||
{
|
||||
transformerSourceTargetSuffixValues.remove(siblingKey);
|
||||
doAdd = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (doAdd)
|
||||
{
|
||||
transformerSourceTargetSuffixValues.put(key, transformerSourceTargetSuffixValue);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -348,7 +380,7 @@ class TransformerSourceTargetSuffixKey
|
||||
? ""
|
||||
: TransformerConfig.EXTENSIONS+sourceExt+'.'+targetExt)+
|
||||
suffix+
|
||||
(use == null ? "" : TransformerConfig.USE + use);
|
||||
(use == null || ANY.equals(use) ? "" : TransformerConfig.USE + use);
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -435,6 +467,30 @@ class TransformerSourceTargetSuffixValue extends TransformerSourceTargetSuffixKe
|
||||
return new TransformerSourceTargetSuffixKey(transformerName, sourceExt, targetExt, suffix, use);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the key of the sibling property for the max value if this is the limit value and one
|
||||
* for the limit value if the max value.
|
||||
*/
|
||||
public TransformerSourceTargetSuffixKey getSiblingKey()
|
||||
{
|
||||
String siblingSuffix =
|
||||
suffix == TransformerConfig.MAX_SOURCE_SIZE_K_BYTES
|
||||
? TransformerConfig.READ_LIMIT_K_BYTES
|
||||
: suffix == TransformerConfig.READ_LIMIT_K_BYTES
|
||||
? TransformerConfig.MAX_SOURCE_SIZE_K_BYTES
|
||||
|
||||
: suffix == TransformerConfig.TIMEOUT_MS
|
||||
? TransformerConfig.READ_LIMIT_TIME_MS
|
||||
: suffix == TransformerConfig.READ_LIMIT_TIME_MS
|
||||
? TransformerConfig.TIMEOUT_MS
|
||||
|
||||
: suffix == TransformerConfig.MAX_PAGES
|
||||
? TransformerConfig.PAGE_LIMIT
|
||||
: TransformerConfig.MAX_PAGES;
|
||||
|
||||
return new TransformerSourceTargetSuffixKey(transformerName, sourceExt, targetExt, siblingSuffix, use);
|
||||
}
|
||||
|
||||
public String toString()
|
||||
{
|
||||
return super.toString()+'='+value;
|
||||
|
@@ -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();
|
||||
}
|
||||
|
||||
/**
|
||||
|
Reference in New Issue
Block a user