Merged V4.1-BUG-FIX to HEAD

43114: ALF-15986: ALF-15986: Upgrade to Bitrock 8.5.1 in order to improve font scaling and adaptive layout with Gtk
   - Helps I18N
   43131: Fix for ALF-16510 (webview link incorrect style in High Constrast theme) and also corrected background color class in Activities title in High Contrast them.
   43147: ALF-14395: AccessDeniedException Thrown When Adding ch:contentHits Aspect from CustomAspect SDK Project
      Now sets a runAs user in the execution thread (using the correct tenant)
      Also fixes a bug where the read list was being used as the write list!
   43154: Merged BRANCHES/DEV/FEATURES/CLOUD1_DISCUSSIONS to BRANCHES/DEV/V4.1-BUG-FIX:
      43145: CLOUD-889: Fix noderefs to use base name for nested replies. Convert integration test to run all tests within a tenant (ALF-16498)
   43172: Merged V3.4-BUG-FIX to V4.1-BUG-FIX
      43166: ALF-14306 Add priorities to transformers
         << incomplete - initial steps >>
         - Addition of a TransformerSelector with the same logic as was there before.
           Makes changing the selection transformers simpler in future.
         - Addition of TransformerLog that logs a single DEBUG line for each top level transformer attempt (includes no transformers available).
           Records: sourceMimetype, targetMimetype, INFO/WARN/ERROR, FileName, FileSize, TransformerName, FailureMessage, TimeTaken
           Makes adding a true transformer log in future simpler.
      43169: Merged V3.4 to V3.4-BUG-FIX
         43167: ALF-16379: Performance degradation detected in benchmark test
            - Contention found in concurrent searches
            - Due to a correction in path generation behaviour in ALF-15171, there were a lot more 'container' (path) documents in the index. This meant that searches by node ID would hit a lot of path documents as well as the node document with ISNODE=T. This meant that whatever caches were in the indexreaders would be exhausted so concurrent searches would contend with each other on cache loading.
            - Using the ALF-15077 solution, LeafScorer now directly locates 'leaf' nodes using LEAFID=<noderef> (when possible) and avoids exhausting the caches and hitting container documents.
      43171: Merged V3.4 to V3.4-BUG-FIX (RECORD ONLY)
         43168: ALF-16379: Merged PATCHES/V4.1.1 to V3.4
            42592: ALF-16332: Alternative version of AbstractWebScriptViewResolver that uses a ConcurrentHashMap and thus allows multiple views to be resolved at the same time!
   43173: Merged V3.4-BUG-FIX to V4.1-BUG-FIX (RECORD ONLY)
      43170: Merged V4.1-BUG-FIX to V3.4-BUG-FIX
         42741: Fix for ALF-16332 - Alternative version of AbstractWebScriptViewResolver that uses a ConcurrentHashMap and thus allows multiple views to be resolved at the same time!


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@43175 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

config/alfresco/content-services-context.xml

@@ -294,8 +294,15 @@
       </property>
    </bean>
 
+   <!-- Selects the transformer to be used -->
+   <bean id="transformerSelector" class="org.alfresco.repo.content.transform.TransformerSelectorImpl" />
+
    <!-- Content Transformation Regisitry -->
-   <bean id="contentTransformerRegistry" class="org.alfresco.repo.content.transform.ContentTransformerRegistry" />
+   <bean id="contentTransformerRegistry" class="org.alfresco.repo.content.transform.ContentTransformerRegistry" >
+      <constructor-arg>
+         <ref bean="transformerSelector"/>
+      </constructor-arg>
+   </bean>
 
    <!-- Transformation Debug -->
    <bean id="transformerDebug" class="org.alfresco.repo.content.transform.TransformerDebug">

source/java/org/alfresco/repo/content/transform/ContentTransformerRegistry.java

@@ -35,6 +35,9 @@ import org.apache.commons.logging.LogFactory;
  * <p>
  * The transformers themselves are used to determine the applicability
  * of a particular transformation.
+ * <p>
+ * The actual selection of a transformer is done by the injected
+ * {@link TransformerSelector}.
  * 
  * @see org.alfresco.repo.content.transform.ContentTransformer
  * 
@@ -46,11 +49,14 @@ public class ContentTransformerRegistry
     
     private List<ContentTransformer> transformers;
     
+    private final TransformerSelector transformerSelector;
+    
     /**
      * @param mimetypeMap all the mimetypes available to the system
      */
-    public ContentTransformerRegistry()
+    public ContentTransformerRegistry(TransformerSelector transformerSelector)
     {
+        this.transformerSelector = transformerSelector;
         this.transformers = new ArrayList<ContentTransformer>(10);
     }
     
@@ -108,67 +114,7 @@ public class ContentTransformerRegistry
     public List<ContentTransformer> getActiveTransformers(String sourceMimetype, long sourceSize, String targetMimetype, TransformationOptions options)
     {
         // Get the list of transformers
-        List<ContentTransformer> transformers = findTransformers(sourceMimetype, sourceSize, targetMimetype, options);
+        List<ContentTransformer> transformers = transformerSelector.selectTransformers(this.transformers, sourceMimetype, sourceSize, targetMimetype, options);
-        final Map<ContentTransformer,Long> activeTransformers = new HashMap<ContentTransformer, Long>();
-        
-        // identify the performance of all the transformers
-         for (ContentTransformer transformer : transformers)
-        {
-            long transformationTime = transformer.getTransformationTime();
-            activeTransformers.put(transformer, transformationTime);
-        }
-         
-        // sort by performance (quicker is "better")
-        List<ContentTransformer> sorted = new ArrayList<ContentTransformer>(activeTransformers.keySet());
-        Collections.sort(sorted, new Comparator<ContentTransformer>() {
-
-            @Override
-            public int compare(ContentTransformer a, ContentTransformer b)
-            {
-                return activeTransformers.get(a).compareTo(activeTransformers.get(b));
-            }
-            
-        });
-        
-        // All done
-        return sorted;
-    }
-    
-    /**
-     * Gets all transformers, of equal reliability, that can perform the requested transformation.
-     * 
-     * @return Returns best transformer for the translation - null if all
-     *      score 0.0 on reliability
-     */
-    private List<ContentTransformer> findTransformers(String sourceMimetype, long sourceSize, String targetMimetype, TransformationOptions options)
-    {
-        List<ContentTransformer> transformers = new ArrayList<ContentTransformer>(2);
-        boolean foundExplicit = false;
-        
-        // loop through transformers
-        for (ContentTransformer transformer : this.transformers)
-        {
-            if (transformer.isTransformable(sourceMimetype, sourceSize, targetMimetype, options) == true)
-            {
-                if (transformer.isExplicitTransformation(sourceMimetype, targetMimetype, options) == true)
-                {
-                    if (foundExplicit == false)
-                    {
-                        transformers.clear();
-                        foundExplicit = true;
-                    }
-                    transformers.add(transformer);
-                }
-                else
-                {
-                    if (foundExplicit == false)
-                    {
-                        transformers.add(transformer);
-                    }
-                }
-            }
-        }
-        // done
         if (logger.isDebugEnabled())
         {
             logger.debug("Searched for transformer: \n" +

source/java/org/alfresco/repo/content/transform/ContentTransformerRegistryTest.java

@@ -70,7 +70,7 @@ public class ContentTransformerRegistryTest extends AbstractContentTransformerTe
             bytes[i] = (byte)i;
         }
         // create the dummyRegistry
-        dummyRegistry = new ContentTransformerRegistry();
+        dummyRegistry = new ContentTransformerRegistry(new TransformerSelectorImpl());
         // create some dummy transformers for reliability tests
         new DummyTransformer(mimetypeService, transformerDebug, dummyRegistry, A, B, 10L);
         new DummyTransformer(mimetypeService, transformerDebug, dummyRegistry, A, B, 10L);

source/java/org/alfresco/repo/content/transform/TransformerDebug.java

@@ -27,7 +27,6 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.alfresco.model.ContentModel;
-import org.alfresco.service.cmr.repository.InvalidNodeRefException;
 import org.alfresco.service.cmr.repository.MimetypeService;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
@@ -57,6 +56,8 @@ import org.apache.commons.logging.LogFactory;
 public class TransformerDebug
 {
     private static final Log logger = LogFactory.getLog(TransformerDebug.class);
+    private static final TransformerLog info = new TransformerLog();
+    private static final String NO_TRANSFORMERS = "No transformers";
 
     private enum Call
     {
@@ -119,16 +120,19 @@ public class TransformerDebug
         private Call callType;
         private int childId;
         private Set<UnavailableTransformer> unavailableTransformers;
-// See debug(String, Throwable) as to why this is commented out
+        private String failureReason;
-//      private Throwable lastThrowable;
+        private long sourceSize;
+        private String transformerName;
         
-        private Frame(Frame parent, String fromUrl, String sourceMimetype, String targetMimetype,
+        private Frame(Frame parent, String transformerName, String fromUrl, String sourceMimetype, String targetMimetype,
-                TransformationOptions options, Call pushCall, boolean origDebugOutput)
+                long sourceSize, TransformationOptions options, Call pushCall, boolean origDebugOutput)
         {
             this.id = parent == null ? -1 : ++parent.childId;
             this.fromUrl = fromUrl;
+            this.transformerName = transformerName;
             this.sourceMimetype = sourceMimetype;
             this.targetMimetype = targetMimetype;
+            this.sourceSize = sourceSize;
             this.options = options;
             this.callType = pushCall;
             this.origDebugOutput = origDebugOutput;
@@ -143,18 +147,48 @@ public class TransformerDebug
             }
             return id;
         }
+        
+        private void setFailureReason(String failureReason)
+        {
+            this.failureReason = failureReason;
+        }
+
+        private String getFailureReason()
+        {
+            return failureReason;
+        }
+
+        private void setSourceSize(long sourceSize)
+        {
+            this.sourceSize = sourceSize;
+        }
+
+        public long getSourceSize()
+        {
+            return sourceSize;
+        }
+
+        private void setTransformerName(String transformerName)
+        {
+            this.transformerName = transformerName;
+        }
+
+        public String getTransformerName()
+        {
+            return transformerName;
+        }
     }
     
     private class UnavailableTransformer
     {
         private final String name;
-        private final String reason;
+        private final long maxSourceSizeKBytes;
         private final transient boolean debug;
         
-        UnavailableTransformer(String name, String reason, boolean debug)
+        UnavailableTransformer(String name, long maxSourceSizeKBytes, boolean debug)
         {
             this.name = name;
-            this.reason = reason;
+            this.maxSourceSizeKBytes = maxSourceSizeKBytes;
             this.debug = debug;
         }
         
@@ -162,7 +196,7 @@ public class TransformerDebug
         public int hashCode()
         {
             int hashCode = 37 * name.hashCode();
-            hashCode += 37 * reason.hashCode();
+            hashCode += 37 * maxSourceSizeKBytes;
             return hashCode;
         }
 
@@ -178,7 +212,7 @@ public class TransformerDebug
                 UnavailableTransformer that = (UnavailableTransformer) obj;
                 return
                     EqualsHelper.nullSafeEquals(name, that.name) &&
-                    EqualsHelper.nullSafeEquals(reason, that.reason);
+                    maxSourceSizeKBytes == that.maxSourceSizeKBytes;
             }
             else
             {
@@ -248,7 +282,7 @@ public class TransformerDebug
         }
     }
     
-    private void push(String name, String fromUrl, String sourceMimetype, String targetMimetype,
+    private void push(String transformerName, String fromUrl, String sourceMimetype, String targetMimetype,
             long sourceSize, TransformationOptions options, Call callType)
     {
         Deque<Frame> ourStack = ThreadInfo.getStack();
@@ -256,18 +290,20 @@ public class TransformerDebug
 
         if (callType == Call.TRANSFORM && frame != null && frame.callType == Call.AVAILABLE)
         {
+            frame.setTransformerName(transformerName);
+            frame.setSourceSize(sourceSize);
             frame.callType = Call.AVAILABLE_AND_TRANSFORM;
         }
 
         // Create a new frame. Logging level is set to trace if the file size is 0
         boolean origDebugOutput = ThreadInfo.setDebugOutput(ThreadInfo.getDebugOutput() && sourceSize != 0);
-        frame = new Frame(frame, fromUrl, sourceMimetype, targetMimetype, options, callType, origDebugOutput);
+        frame = new Frame(frame, transformerName, fromUrl, sourceMimetype, targetMimetype, sourceSize, options, callType, origDebugOutput);
         ourStack.push(frame);
             
         if (callType == Call.TRANSFORM)
         {
             // Log the basic info about this transformation
-            logBasicDetails(frame, sourceSize, name, (ourStack.size() == 1));
+            logBasicDetails(frame, sourceSize, transformerName, (ourStack.size() == 1));
         }
     }
     
@@ -288,20 +324,12 @@ public class TransformerDebug
                 String name = (!isTransformableStack.isEmpty())
                     ? isTransformableStack.getFirst()
                     : getName(transformer);
-                String reason = "> "+fileSize(maxSourceSizeKBytes*1024);
                 boolean debug = (maxSourceSizeKBytes != 0);
-                if (ourStack.size() == 1)
-                {
                 if (frame.unavailableTransformers == null)
                 {
                     frame.unavailableTransformers = new HashSet<UnavailableTransformer>();
                 }
-                    frame.unavailableTransformers.add(new UnavailableTransformer(name, reason, debug));
+                frame.unavailableTransformers.add(new UnavailableTransformer(name, maxSourceSizeKBytes, debug));
-                }
-                else
-                {
-                    log("-- " + name + ' ' + reason, debug);
-                }
             }
         }
     }
@@ -315,18 +343,25 @@ public class TransformerDebug
         {
             Deque<Frame> ourStack = ThreadInfo.getStack();
             Frame frame = ourStack.peek();
+            boolean firstLevel = ourStack.size() == 1;
 
             // Override setDebugOutput(false) to allow debug when there are transformers but they are all unavailable
             // Note once turned on we don't turn it off again.
-            if (transformers.size() == 0 &&
+            if (transformers.size() == 0)
-                frame.unavailableTransformers != null &&
+            {
-                frame.unavailableTransformers.size() != 0) {
+                frame.setFailureReason(NO_TRANSFORMERS);
+                if (frame.unavailableTransformers != null &&
+                    frame.unavailableTransformers.size() != 0)
+                {
                     ThreadInfo.setDebugOutput(true);
                 }
+            }
+            frame.setSourceSize(sourceSize);
+            
             // Log the basic info about this transformation
             logBasicDetails(frame, sourceSize,
                     calledFrom + ((transformers.size() == 0) ? " NO transformers" : ""),
-                    (ourStack.size() == 1));
+                    firstLevel);
 
             // Report available and unavailable transformers
             char c = 'a';
@@ -346,8 +381,8 @@ public class TransformerDebug
                 for (UnavailableTransformer unavailable: frame.unavailableTransformers)
                 {
                     int pad = longestNameLength - unavailable.name.length();
-                    log("--" + (c++) + ") " + unavailable.name + spaces(pad+1) + unavailable.reason,
+                    String reason = "> "+fileSize(unavailable.maxSourceSizeKBytes*1024);
-                        unavailable.debug);
+                    log("--" + (c++) + ") " + unavailable.name + spaces(pad+1) + reason, unavailable.debug);
                 }
             }
         }
@@ -476,29 +511,91 @@ public class TransformerDebug
         if (!ourStack.isEmpty())
         {
             Frame frame = ourStack.peek();
+
             if ((frame.callType == callType) ||
                 (frame.callType == Call.AVAILABLE_AND_TRANSFORM && callType == Call.AVAILABLE))
             {
-                if (!suppressFinish && (ourStack.size() == 1 || logger.isTraceEnabled()))
+                int size = ourStack.size();
+                String ms = ms(System.currentTimeMillis() - frame.start);
+
+                logInfo(frame, size, ms);
+                
+                boolean firstLevel = size == 1;
+                if (!suppressFinish && (firstLevel || logger.isTraceEnabled()))
                 {
-                    boolean topFrame = ourStack.size() == 1;
+                    log("Finished in " + ms +
-                    log("Finished in " +
-                        ms(System.currentTimeMillis() - frame.start) +
                         (frame.callType == Call.AVAILABLE ? " Transformer NOT called" : "") +
-                        (topFrame ? "\n" : ""), 
+                        (firstLevel ? "\n" : ""), 
-                        topFrame);
+                        firstLevel);
                 }
                 
                 setDebugOutput(frame.origDebugOutput);
                 ourStack.pop();
+            }
+        }
+    }
 
-// See debug(String, Throwable) as to why this is commented out
+    private void logInfo(Frame frame, int size, String ms)
-//                if (ourStack.size() >= 1)
+    {
-//                {
+        if (info.isDebugEnabled())
-//                    ourStack.peek().lastThrowable = frame.lastThrowable;
+        {
-//                }
+            String failureReason = frame.getFailureReason();
+            boolean firstLevel = size == 1;
+            String sourceExt = getMimetypeExt(frame.sourceMimetype);
+            String targetExt = getMimetypeExt(frame.targetMimetype);
+            String fileName = getFileName(frame.options, firstLevel, frame.sourceSize);
+            long sourceSize = frame.getSourceSize();
+            String transformerName = frame.getTransformerName();
+            String level = null;
+            boolean debug = false;
+            if (NO_TRANSFORMERS.equals(failureReason))
+            {
+                debug = firstLevel;
+                level = "INFO";
+                failureReason = NO_TRANSFORMERS;
+                if (frame.unavailableTransformers != null)
+                {
+                    level = "WARN";
+                    long smallestMaxSourceSizeKBytes = Long.MAX_VALUE;
+                    for (UnavailableTransformer unavailable: frame.unavailableTransformers)
+                    {
+                        if (smallestMaxSourceSizeKBytes > unavailable.maxSourceSizeKBytes)
+                        {
+                            smallestMaxSourceSizeKBytes = unavailable.maxSourceSizeKBytes;
                         }
                     }
+                    failureReason = "No transformers as file is > "+fileSize(smallestMaxSourceSizeKBytes*1024);
+                }
+            }
+            else if (frame.callType == Call.TRANSFORM)
+            {
+                level = failureReason == null || failureReason.length() == 0 ? "INFO" : "ERROR";
+                
+                // Use TRACE logging for all but the first TRANSFORM
+                debug = size == 1 || (size == 2 && ThreadInfo.getStack().peekLast().callType != Call.TRANSFORM);
+            }
+            
+            if (level != null)
+            {
+                infoLog(getReference(debug), sourceExt, targetExt, level, fileName, sourceSize, transformerName, failureReason, ms, debug);
+            }
+        }
+    }
+    
+    private void infoLog(String reference, String sourceExt, String targetExt, String level, String fileName,
+            long sourceSize, String transformerName, String failureReason, String ms, boolean debug)
+    {
+        String message =
+                "  "+reference +
+                sourceExt +
+                targetExt +
+                (level == null ? "" : level+' ') +
+                (fileName == null ? "" : fileName) +
+                (sourceSize >= 0 ? ' '+fileSize(sourceSize) : "") +
+                (transformerName == null ? "" : ' '+transformerName) +
+                (failureReason == null ? "" : ' '+failureReason) +
+                ' '+ms;
+        info.log(message,  debug);
     }
 
     /**
@@ -507,7 +604,7 @@ public class TransformerDebug
     public boolean isEnabled()
     {
         // Don't check ThreadInfo.getDebugOutput() as availableTransformers() may upgrade from trace to debug.
-        return logger.isDebugEnabled();
+        return logger.isDebugEnabled() || info.isDebugEnabled();
     }
     
     /**
@@ -546,39 +643,31 @@ public class TransformerDebug
         {
             log(message + ' ' + t.getMessage());
             
-//            // Generally the full stack is not needed as transformer
+            Deque<Frame> ourStack = ThreadInfo.getStack();
-//            // Exceptions get logged as a Error higher up, so including
+            if (!ourStack.isEmpty())
-//            // the stack trace has been found not to be needed. Keeping
+            {
-//            // the following code and code that sets lastThrowable just
+                Frame frame = ourStack.peek();
-//            // in case we need it after all.
+                frame.setFailureReason(message +' '+ getRootCauseMessage(t));
-//
+            }
-//            Frame frame = ThreadInfo.getStack().peek();
-//            boolean newThrowable = isNewThrowable(frame.lastThrowable, t);
-//            frame.lastThrowable = t;
-//
-//            if (newThrowable)
-//            {
-//                log(message, t, true);
-//            }
-//            else
-//            {
-//                log(message + ' ' + t.getMessage());
-//            }
         }
     }
 
-//    private boolean isNewThrowable(Throwable lastThrowable, Throwable t)
+    private String getRootCauseMessage(Throwable t)
-//    {
+    {
-//        while (t != null)
+        Throwable cause = t;
-//        {
+        while (cause != null)
-//            if (lastThrowable == t)
+        {
-//            {
+            t = cause;
-//                return false;
+            cause = t.getCause();
-//            }
+        }
-//            t = t.getCause();
+        
-//        }
+        String message = t.getMessage();
-//        return true;
+        if (message == null || message.length() == 0)
-//    }
+        {
+            message = t.getClass().getSimpleName();
+        }
+        return message;
+    }
 
     private void log(String message)
     {
@@ -592,13 +681,13 @@ public class TransformerDebug
     
     private void log(String message, Throwable t, boolean debug)
     {
-        if (debug && ThreadInfo.getDebugOutput())
+        if (debug && ThreadInfo.getDebugOutput() && logger.isDebugEnabled())
         {
-            logger.debug(getReference()+message, t);
+            logger.debug(getReference(false)+message, t);
         }
         else if (logger.isTraceEnabled())
         {
-            logger.trace(getReference()+message, t);
+            logger.trace(getReference(false)+message, t);
         }
     }
 
@@ -609,19 +698,15 @@ public class TransformerDebug
      */
     public <T extends Throwable> T setCause(T t)
     {
-// See debug(String, Throwable) as to why this is commented out
-//        if (isEnabled())
-//        {
-//            Deque<Frame> ourStack = ThreadInfo.getStack();
-//            if (!ourStack.isEmpty())
-//            {
-//                ourStack.peek().lastThrowable = t;
-//            }
-//        }
         return t;
     }
     
-    private String getReference()
+    /**
+     * Returns a N.N.N style reference to the transformation.
+     * @param firstLevelOnly indicates if only the top level should be included.
+     * @return a padded (fixed length) reference.
+     */
+    private String getReference(boolean firstLevelOnly)
     {
         StringBuilder sb = new StringBuilder("");
         Frame frame = null;
@@ -634,6 +719,10 @@ public class TransformerDebug
             {
                 sb.append(frame.getId());
                 lengthOfFirstId = sb.length();
+                if (firstLevelOnly)
+                {
+                    break;
+                }
             }
             else
             {
@@ -770,3 +859,30 @@ public class TransformerDebug
         return sb.toString();
     }
 }
+
+class TransformerLog
+{
+    private static final Log logger = LogFactory.getLog(TransformerLog.class);
+
+    void log(String message, boolean debug)
+    {
+        if (debug)
+        {
+            logger.debug(message);
+        }
+        else
+        {
+            logger.trace(message);
+        }
+    }
+
+    public boolean isDebugEnabled()
+    {
+        return logger.isDebugEnabled();
+    }
+
+    public boolean isTraceEnabled()
+    {
+        return logger.isTraceEnabled();
+    }
+}

source/java/org/alfresco/repo/content/transform/TransformerSelector.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2005-2012 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco 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.
+ *
+ * Alfresco 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.content.transform;
+
+import java.util.List;
+
+import org.alfresco.service.cmr.repository.TransformationOptions;
+
+/**
+ * Selects a transformer from a supplied list of transformers that appear
+ * able to handle a given transformation.
+ * 
+ * @author Alan Davis
+ */
+public interface TransformerSelector
+{
+    /**
+     * Returns a sorted list of transformers that identifies the order in which transformers
+     * should be tried.
+     * @param transformers an unordered list of all transformers. This
+     *        list may be modified.
+     * @param sourceMimetype
+     * @param sourceSize
+     * @param targetMimetype
+     * @param options transformation options
+     * @return a sorted list of transformers, with the best one first.
+     */
+    List<ContentTransformer> selectTransformers(List<ContentTransformer> transformers,
+            String sourceMimetype, long sourceSize, String targetMimetype,
+            TransformationOptions options);
+}

source/java/org/alfresco/repo/content/transform/TransformerSelectorImpl.java

@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2005-2012 Alfresco Software Limited.
+ *
+ * This file is part of Alfresco
+ *
+ * Alfresco 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.
+ *
+ * Alfresco 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.alfresco.repo.content.transform;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.alfresco.service.cmr.repository.TransformationOptions;
+
+/**
+ * Implementation of a transformer selector that matches the code that was in place
+ * before a selector was introduced. It is expected that this code will be replaced.
+ * 
+ * @author Alan Davis
+ */
+public class TransformerSelectorImpl implements TransformerSelector
+{
+
+    @Override
+    public List<ContentTransformer> selectTransformers(List<ContentTransformer> transformers,
+            String sourceMimetype, long sourceSize, String targetMimetype,
+            TransformationOptions options)
+    {
+        transformers = findTransformers(transformers, sourceMimetype, sourceSize, targetMimetype, options);
+        transformers = discardNonExplicitTransformers(transformers, sourceMimetype, sourceSize, targetMimetype, options);
+        transformers = sortTransformers(transformers, sourceMimetype, sourceSize, targetMimetype, options);
+        return transformers;
+    }
+
+    /**
+     * Reduces the list of transformers down to only those capable of doing the transformation.
+     */
+    private List<ContentTransformer> findTransformers(List<ContentTransformer> allTransformers, String sourceMimetype,
+            long sourceSize, String targetMimetype, TransformationOptions options)
+    {
+        List<ContentTransformer> transformers = new ArrayList<ContentTransformer>(2);
+        
+        for (ContentTransformer transformer : allTransformers)
+        {
+            if (transformer.isTransformable(sourceMimetype, sourceSize, targetMimetype, options) == true)
+            {
+                transformers.add(transformer);
+            }
+        }
+        return transformers;
+    }
+    
+    /**
+     * Discards non explicit transformers if there are any explicit ones.
+     */
+    private List<ContentTransformer> discardNonExplicitTransformers(List<ContentTransformer> allTransformers, String sourceMimetype,
+            long sourceSize, String targetMimetype, TransformationOptions options)
+    {
+        List<ContentTransformer> transformers = new ArrayList<ContentTransformer>(2);
+        boolean foundExplicit = false;
+        
+        for (ContentTransformer transformer : allTransformers)
+        {
+            if (transformer.isExplicitTransformation(sourceMimetype, targetMimetype, options) == true)
+            {
+                if (foundExplicit == false)
+                {
+                    transformers.clear();
+                    foundExplicit = true;
+                }
+                transformers.add(transformer);
+            }
+            else
+            {
+                if (foundExplicit == false)
+                {
+                    transformers.add(transformer);
+                }
+            }
+        }
+        return transformers;
+    }
+
+    // sort by performance (quicker is "better")
+    private List<ContentTransformer> sortTransformers(List<ContentTransformer> transformers,
+            String sourceMimetype, long sourceSize, String targetMimetype,
+            TransformationOptions options)
+    {
+        final Map<ContentTransformer,Long> activeTransformers = new HashMap<ContentTransformer, Long>();
+        for (ContentTransformer transformer : transformers)
+        {
+            long transformationTime = transformer.getTransformationTime();
+            activeTransformers.put(transformer, transformationTime);
+        }
+         
+        List<ContentTransformer> sorted = new ArrayList<ContentTransformer>(activeTransformers.keySet());
+        Collections.sort(sorted, new Comparator<ContentTransformer>() {
+            @Override
+            public int compare(ContentTransformer a, ContentTransformer b)
+            {
+                return activeTransformers.get(a).compareTo(activeTransformers.get(b));
+            }
+        });
+        return sorted;
+    }
+}

source/java/org/alfresco/repo/discussion/DiscussionServiceImpl.java

@@ -584,7 +584,7 @@ public class DiscussionServiceImpl implements DiscussionService
        NodeBackedEntity node = results.getPage().get(0);
        
        // Wrap and return
-       return buildPost(node.getNodeRef(), topic, node.getName(), null);
+       return buildPost(tenantService.getBaseName(node.getNodeRef()), topic, node.getName(), null);
     }
 
    
@@ -820,7 +820,7 @@ public class DiscussionServiceImpl implements DiscussionService
       Map<Long,NodeRef> idToNodeRef = new HashMap<Long, NodeRef>();
       for(NodeWithTargetsEntity e : results.getPage())
       {
-         idToNodeRef.put(e.getId(), e.getNodeRef());
+         idToNodeRef.put(e.getId(), tenantService.getBaseName(e.getNodeRef()));
       }
       
       Map<NodeRef,List<NodeWithTargetsEntity>> idToReplies = new HashMap<NodeRef, List<NodeWithTargetsEntity>>();
@@ -869,7 +869,7 @@ public class DiscussionServiceImpl implements DiscussionService
          {
             for (NodeWithTargetsEntity entity : replies)
             {
-               calculateRepliesPreLoad(entity.getNodeRef(), preLoad, idToReplies, levels-1);
+               calculateRepliesPreLoad(tenantService.getBaseName(entity.getNodeRef()), preLoad, idToReplies, levels-1);
             }
          }
       }
@@ -885,7 +885,7 @@ public class DiscussionServiceImpl implements DiscussionService
          {
             for (NodeWithTargetsEntity entity : replyEntities)
             {
-               PostInfo replyPost = buildPost(entity.getNodeRef(), post.getTopic(), entity.getName(), null);
+               PostInfo replyPost = buildPost(tenantService.getBaseName(entity.getNodeRef()), post.getTopic(), entity.getName(), null);
                replies.add(wrap(replyPost, idToReplies, levels-1));
             }
          }

source/java/org/alfresco/repo/discussion/DiscussionServiceImplTest.java

@@ -94,16 +94,13 @@ public class DiscussionServiceImplTest
     private static TaggingService               TAGGING_SERVICE;
     private static TenantAdminService           TENANT_ADMIN_SERVICE;
     
-    private static final String TEST_USER = DiscussionServiceImplTest.class.getSimpleName() + "_testuser";
-    private static final String ADMIN_USER = AuthenticationUtil.getAdminUserName();
-    
     private static final String TENANT_DOMAIN = (DiscussionServiceImplTest.class.getSimpleName() + "Tenant").toLowerCase();
-    private static final String TENANT_ADMIN_USER = ADMIN_USER + "@" + TENANT_DOMAIN;
+
-    private static final String TENANT_TEST_USER = TEST_USER + "@" + TENANT_DOMAIN;
+    private static final String TEST_USER = DiscussionServiceImplTest.class.getSimpleName() + "_testuser@" + TENANT_DOMAIN;
+    private static final String ADMIN_USER = AuthenticationUtil.getAdminUserName() + "@" + TENANT_DOMAIN;
 
     private static SiteInfo DISCUSSION_SITE;
     private static SiteInfo ALTERNATE_DISCUSSION_SITE;
-    private static SiteInfo TENANT_DISCUSSION_SITE;
     private static NodeRef FORUM_NODE;
     
     /**
@@ -130,7 +127,7 @@ public class DiscussionServiceImplTest
         TAGGING_SERVICE        = (TaggingService)testContext.getBean("TaggingService");
         TENANT_ADMIN_SERVICE   = testContext.getBean("tenantAdminService", TenantAdminService.class);
 
-        createTenantAndTestSites();
+        createTenant();
         
         // Do the setup as admin
         AuthenticationUtil.setFullyAuthenticatedUser(ADMIN_USER);
@@ -391,98 +388,6 @@ public class DiscussionServiceImplTest
        }
     }
     
-    @Test
-    public void tenantCreateNewTopicAndPostAndReply() throws Exception
-    {
-        TenantUtil.runAsUserTenant(new TenantRunAsWork<Object>(){
-
-            @Override
-            public Object doWork() throws Exception
-            {
-                TopicInfo siteTopic;
-                PostInfo post;
-                PostInfo reply1;
-
-                // Nothing to start with
-                PagingResults<TopicInfo> results = DISCUSSION_SERVICE.listTopics(TENANT_DISCUSSION_SITE.getShortName(), true, new PagingRequest(10));
-                assertEquals(0, results.getPage().size());
-
-                // Create two topics, one node and one site based
-                siteTopic = DISCUSSION_SERVICE.createTopic(TENANT_DISCUSSION_SITE.getShortName(),
-                            "Site Title");
-
-                testNodesToTidy.add(siteTopic.getNodeRef());
-
-                // There are no posts to start with
-                PagingResults<PostInfo> posts = DISCUSSION_SERVICE.listPosts(siteTopic, new PagingRequest(10));
-                assertEquals(0, posts.getPage().size());      
-              
-                // The topic has no primary post
-                assertEquals(null, DISCUSSION_SERVICE.getPrimaryPost(siteTopic));
-                // Which means no recent post
-                assertEquals(null, DISCUSSION_SERVICE.getMostRecentPost(siteTopic));
-
-                // Create the first post
-                String contents = "This Is Some Content";
-                post = DISCUSSION_SERVICE.createPost(siteTopic, contents);
-              
-                // Ensure it got a NodeRef, a Name and the Topic
-                assertNotNull(post.getNodeRef());
-                assertNotNull(post.getSystemName());
-                assertEquals(siteTopic, post.getTopic());
-              
-                // Ensure it shares a name with the topic
-                assertEquals(siteTopic.getSystemName(), post.getSystemName());
-              
-                // As this is the primary post, it'll share the topic title
-                assertEquals(siteTopic.getTitle(), post.getTitle());
-              
-                // It will have contents and a creator
-                assertEquals(contents, post.getContents());
-              
-                // Fetch and check
-                post = DISCUSSION_SERVICE.getPost(siteTopic, post.getSystemName());
-                assertNotNull(post.getNodeRef());
-                assertNotNull(post.getSystemName());
-                assertEquals(siteTopic, post.getTopic());
-                assertEquals(siteTopic.getTitle(), post.getTitle());
-                assertEquals(contents, post.getContents());
-              
-                // Topic will now have a primary post
-                assertNotNull(DISCUSSION_SERVICE.getPrimaryPost(siteTopic));
-                assertEquals(post.getNodeRef(), DISCUSSION_SERVICE.getPrimaryPost(siteTopic).getNodeRef());
-              
-                // Topic will now have one post listed
-                posts = DISCUSSION_SERVICE.listPosts(siteTopic, new PagingRequest(10));
-                assertEquals(1, posts.getPage().size());      
-              
-                // Add a reply
-                String reply1Contents = "Reply Contents";
-                reply1 = DISCUSSION_SERVICE.createReply(post, reply1Contents);
-                assertNotNull(reply1.getNodeRef());
-                assertNotNull(reply1.getSystemName());
-                assertEquals(siteTopic, reply1.getTopic());
-                assertEquals(null, reply1.getTitle()); // No title by default for
-                                                       // replies
-                assertEquals(reply1Contents, reply1.getContents());
-              
-                // Fetch and check
-                PostWithReplies postWithReplies = DISCUSSION_SERVICE.listPostReplies(siteTopic, 1);
-                assertNotNull(postWithReplies);
-                assertEquals(1, postWithReplies.getReplies().size());
-                PostInfo reply2 = postWithReplies.getReplies().get(0).getPost();
-                assertNotNull(reply2.getNodeRef());
-                assertNotNull(reply2.getSystemName());
-                assertEquals(siteTopic, reply2.getTopic());
-                assertEquals(null, reply2.getTitle()); // No title by default for
-                                                       // replies
-                assertEquals(reply1Contents, reply1.getContents());
-                return null;
-            }
-            
-        }, TENANT_TEST_USER, TENANT_DOMAIN);
-    }
-
 
     @Test public void createUpdateDeleteEntries() throws Exception
     {
@@ -1951,53 +1856,6 @@ public class DiscussionServiceImplTest
          AuthenticationUtil.setFullyAuthenticatedUser(TEST_USER);
     }
     
-    private static void createTenantAndTestSites()
-    {
-        AuthenticationUtil.runAs(new RunAsWork<Object>(){
-
-            @Override
-            public Object doWork() throws Exception
-            {
-                createTenant();
-                return null;
-            }
-            
-        }, ADMIN_USER);
-        
-        createTenantUser();
-        
-        TenantUtil.runAsUserTenant(new TenantRunAsWork<Object>() {
-
-            @Override
-            public Object doWork() throws Exception
-            {
-                createTenantTestSites();
-                return null;
-            }
-        }, TENANT_TEST_USER, TENANT_DOMAIN);
-    }
-    
-    private static void createTenantTestSites() throws Exception
-    {
-        final DiscussionServiceImpl privateDiscussionService = (DiscussionServiceImpl)testContext.getBean("discussionService");
-        
-        TENANT_DISCUSSION_SITE = TRANSACTION_HELPER.doInTransaction(new RetryingTransactionHelper.RetryingTransactionCallback<SiteInfo>()
-           {
-              @Override
-              public SiteInfo execute() throws Throwable
-              {
-                  SiteInfo site = SITE_SERVICE.createSite(
-                        TEST_SITE_PREFIX, 
-                        DiscussionServiceImplTest.class.getSimpleName() + "_testSite" + System.currentTimeMillis(),
-                        "test site title", "test site description", 
-                        SiteVisibility.PUBLIC);
-                  privateDiscussionService.getSiteDiscussionsContainer(site.getShortName(), true);
-                  CLASS_TEST_NODES_TO_TIDY.add(site.getNodeRef());
-                  return site;
-              }
-         });
-    }
-
     
     private static void createTenant()
     {
@@ -2015,20 +1873,6 @@ public class DiscussionServiceImplTest
         });
     }
     
-    private static void createTenantUser()
-    {
-        TenantUtil.runAsUserTenant(new TenantRunAsWork<Object>(){
-
-            @Override
-            public Object doWork() throws Exception
-            {
-                createUser(TENANT_TEST_USER);
-                return null;
-            }
-            
-        }, TENANT_ADMIN_USER, TENANT_DOMAIN);
-    }
-    
     /**
      * By default, all tests are run as the admin user.
      */
@@ -2046,7 +1890,6 @@ public class DiscussionServiceImplTest
     {
         performDeletionOfNodes(CLASS_TEST_NODES_TO_TIDY);
         deleteUser(TEST_USER);
-        deleteUser(TENANT_TEST_USER);
         deleteTenant();
     }