Tidy up TransformHandler

engines/base/src/main/java/org/alfresco/transform/base/TransformController.java

@@ -104,13 +104,11 @@ public class TransformController
     private String coreVersion;
 
     TransformEngine transformEngine;
-    ProbeTransform probeTransform;
 
     @PostConstruct
     private void init()
     {
         transformEngine = transformHandler.getTransformEngine();
-        probeTransform = transformHandler.getProbeTransform();
     }
 
     @EventListener(ApplicationReadyEvent.class)
@@ -186,7 +184,7 @@ public class TransformController
     @ResponseBody
     public String ready(HttpServletRequest request)
     {
-        return probeTransform.doTransformOrNothing(request, false, transformHandler);
+        return transformHandler.probe(request, false);
     }
 
     /**
@@ -196,7 +194,7 @@ public class TransformController
     @ResponseBody
     public String live(HttpServletRequest request)
     {
-        return probeTransform.doTransformOrNothing(request, true, transformHandler);
+        return transformHandler.probe(request, true);
     }
 
     @GetMapping(value = ENDPOINT_TRANSFORM_CONFIG)
@@ -298,4 +296,9 @@ public class TransformController
         mav.setViewName("error"); // display error.html
         return mav;
     }
+
+    void resetForTesting()
+    {
+
+    }
 }

engines/base/src/main/java/org/alfresco/transform/base/transform/TransformHandler.java

@@ -65,14 +65,12 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
-import java.util.Arrays;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.stream.Collectors;
 
 import static java.util.stream.Collectors.joining;
 import static org.alfresco.transform.base.fs.FileManager.createAttachment;
@@ -80,11 +78,6 @@ import static org.alfresco.transform.base.fs.FileManager.createTargetFile;
 import static org.alfresco.transform.base.fs.FileManager.getDirectAccessUrlInputStream;
 import static org.alfresco.transform.base.fs.FileManager.getMultipartFileInputStream;
 import static org.alfresco.transform.common.RequestParamMap.DIRECT_ACCESS_URL;
-import static org.alfresco.transform.common.RequestParamMap.SOURCE_ENCODING;
-import static org.alfresco.transform.common.RequestParamMap.SOURCE_EXTENSION;
-import static org.alfresco.transform.common.RequestParamMap.SOURCE_MIMETYPE;
-import static org.alfresco.transform.common.RequestParamMap.TARGET_EXTENSION;
-import static org.alfresco.transform.common.RequestParamMap.TARGET_MIMETYPE;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;
 import static org.springframework.http.HttpStatus.CREATED;
 import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
@@ -96,8 +89,6 @@ import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
 public class TransformHandler
 {
     private static final Logger logger = LoggerFactory.getLogger(TransformHandler.class);
-    private static final List<String> NON_TRANSFORM_OPTION_REQUEST_PARAMETERS = Arrays.asList(SOURCE_EXTENSION,
-        TARGET_EXTENSION, TARGET_MIMETYPE, SOURCE_MIMETYPE, DIRECT_ACCESS_URL);
 
     @Autowired(required = false)
     private List<TransformEngine> transformEngines;
@@ -116,14 +107,12 @@ public class TransformHandler
 
     private final AtomicInteger httpRequestCount = new AtomicInteger(1);
     private TransformEngine transformEngine;
-    private ProbeTransform probeTransform;
     private final Map<String, CustomTransformer> customTransformersByName = new HashMap<>();
 
     @PostConstruct
     private void init()
     {
         initTransformEngine();
-        initProbeTestTransform();
         initCustomTransformersByName();
     }
 
@@ -131,14 +120,7 @@ public class TransformHandler
     {
         if (transformEngines != null)
         {
-            // Normally there is just one TransformEngine per t-engine, but we also want to be able to amalgamate the
+            transformEngine = getTransformEngine();
-            // CustomTransform code from many t-engines into a single t-engine. In this case, there should be a wrapper
-            // TransformEngine (it has no TransformConfig of its own).
-            transformEngine = transformEngines.stream()
-                                              .filter(transformEngine -> transformEngine.getTransformConfig() == null)
-                                              .findFirst()
-                                              .orElse(transformEngines.get(0));
-
             logger.info("TransformEngine: " + transformEngine.getTransformEngineName());
             transformEngines.stream()
                             .filter(te -> te != transformEngine)
@@ -147,14 +129,6 @@ public class TransformHandler
         }
     }
 
-    private void initProbeTestTransform()
-    {
-        if (transformEngine != null)
-        {
-            probeTransform = transformEngine.getProbeTransform();
-        }
-    }
-
     private void initCustomTransformersByName()
     {
         if (customTransformers != null)
@@ -171,17 +145,18 @@ public class TransformHandler
 
     public TransformEngine getTransformEngine()
     {
-        return transformEngine;
+        // Normally there is just one TransformEngine per t-engine, but we also want to be able to amalgamate the
+        // CustomTransform code from many t-engines into a single t-engine. In this case, there should be a wrapper
+        // TransformEngine (it has no TransformConfig of its own).
+        return transformEngines.stream()
+                               .filter(transformEngine -> transformEngine.getTransformConfig() == null)
+                               .findFirst()
+                               .orElse(transformEngines.get(0));
     }
 
     public ProbeTransform getProbeTransform()
     {
-        return probeTransform;
+        return transformEngine.getProbeTransform();
-    }
-
-    public TransformerDebug getTransformerDebug()
-    {
-        return transformerDebug;
     }
 
     public ResponseEntity<Resource> handleHttpRequest(HttpServletRequest request,
@@ -190,8 +165,9 @@ public class TransformHandler
     {
         AtomicReference<ResponseEntity<Resource>> responseEntity = new AtomicReference<>();
 
-        new TransformProcess(this, sourceMimetype, targetMimetype, requestParameters,
+        new TransformProcess(sourceMimetype, targetMimetype, requestParameters,
-            "e" + httpRequestCount.getAndIncrement())
+            "e" + httpRequestCount.getAndIncrement(), transformRegistry,
+            transformerDebug, getProbeTransform(), customTransformersByName)
         {
             @Override
             protected void init() throws IOException
@@ -233,8 +209,9 @@ public class TransformHandler
     public void handleProbRequest(String sourceMimetype, String targetMimetype, Map<String, String> transformOptions,
         File sourceFile, File targetFile)
     {
-        new TransformProcess(this, sourceMimetype, targetMimetype, transformOptions,
+        new TransformProcess(sourceMimetype, targetMimetype, transformOptions,
-            "p" + httpRequestCount.getAndIncrement())
+            "p" + httpRequestCount.getAndIncrement(), transformRegistry,
+            transformerDebug, getProbeTransform(), customTransformersByName)
         {
             @Override
             protected void init() throws IOException
@@ -268,8 +245,9 @@ public class TransformHandler
     public TransformReply handleMessageRequest(TransformRequest request, Long timeout, Destination replyToQueue)
     {
         TransformReply reply = createBasicTransformReply(request);
-        new TransformProcess(this, request.getSourceMediaType(), request.getTargetMediaType(),
+        new TransformProcess(request.getSourceMediaType(), request.getTargetMediaType(),
-            request.getTransformRequestOptions(), "unset")
+            request.getTransformRequestOptions(),"unset", transformRegistry,
+            transformerDebug, getProbeTransform(), customTransformersByName)
         {
             @Override
             protected void init() throws IOException
@@ -320,6 +298,11 @@ public class TransformHandler
         return reply;
     }
 
+    public String probe(HttpServletRequest request, boolean isLiveProbe)
+    {
+        return getProbeTransform().doTransformOrNothing(request, isLiveProbe, this);
+    }
+
     private void sendSuccessfulResponse(Long timeout, TransformReply reply, Destination replyToQueue)
     {
         logger.trace("Sending successful {}, timeout {} ms", reply, timeout);
@@ -383,14 +366,6 @@ public class TransformHandler
         request.setInternalContext(InternalContext.initialise(request.getInternalContext()));
     }
 
-    public Map<String, String> cleanTransformOptions(Map<String, String> requestParameters)
-    {
-        Map<String, String> transformOptions = new HashMap<>(requestParameters);
-        NON_TRANSFORM_OPTION_REQUEST_PARAMETERS.forEach(transformOptions.keySet()::remove);
-        transformOptions.values().removeIf(String::isEmpty);
-        return transformOptions;
-    }
-
     private InputStream getSharedFileStoreInputStream(String sourceReference)
     {
         ResponseEntity<Resource> responseEntity = alfrescoSharedFileStoreClient.retrieveFile(sourceReference);
@@ -500,43 +475,4 @@ public class TransformHandler
 
         return sb.toString();
     }
-
-    public String getTransformerName(final String sourceMimetype, long sourceSizeInBytes, final String targetMimetype,
-        final Map<String, String> transformOptions)
-    {
-        // The transformOptions always contains sourceEncoding when sent to a T-Engine, even though it should not be
-        // used to select a transformer. Similar to source and target mimetypes and extensions, but these are not
-        // passed in transformOptions.
-        String sourceEncoding = transformOptions.remove(SOURCE_ENCODING);
-        try
-        {
-            final String transformerName = transformRegistry.findTransformerName(sourceMimetype,
-                    sourceSizeInBytes, targetMimetype, transformOptions, null);
-            if (transformerName == null)
-            {
-                throw new TransformException(BAD_REQUEST, "No transforms for: "+
-                        sourceMimetype+" -> "+targetMimetype+transformOptions.entrySet().stream()
-                            .map(entry -> entry.getKey()+"="+entry.getValue())
-                            .collect(Collectors.joining(", ", " ", "")));
-            }
-            return transformerName;
-        }
-        finally
-        {
-            if (sourceEncoding != null)
-            {
-                transformOptions.put(SOURCE_ENCODING, sourceEncoding);
-            }
-        }
-    }
-
-    public CustomTransformer getCustomTransformer(String transformName)
-    {
-        CustomTransformer customTransformer = customTransformersByName.get(transformName);
-        if (customTransformer == null)
-        {
-            throw new TransformException(INTERNAL_SERVER_ERROR, "Custom Transformer "+transformName+" not found");
-        }
-        return customTransformer;
-    }
 }

engines/base/src/main/java/org/alfresco/transform/base/transform/TransformProcess.java

@@ -29,17 +29,30 @@ package org.alfresco.transform.base.transform;
 import org.alfresco.transform.base.CustomTransformer;
 import org.alfresco.transform.base.TransformController;
 import org.alfresco.transform.base.logging.LogEntry;
+import org.alfresco.transform.base.probes.ProbeTransform;
 import org.alfresco.transform.client.model.TransformRequest;
 import org.alfresco.transform.common.TransformException;
 import org.alfresco.transform.common.TransformerDebug;
+import org.alfresco.transform.registry.TransformServiceRegistry;
 import org.springframework.http.HttpStatus;
 import org.springframework.web.multipart.MultipartFile;
 
 import javax.jms.Destination;
 import javax.servlet.http.HttpServletRequest;
 import java.io.File;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collectors;
 
+import static org.alfresco.transform.common.RequestParamMap.DIRECT_ACCESS_URL;
+import static org.alfresco.transform.common.RequestParamMap.SOURCE_ENCODING;
+import static org.alfresco.transform.common.RequestParamMap.SOURCE_EXTENSION;
+import static org.alfresco.transform.common.RequestParamMap.SOURCE_MIMETYPE;
+import static org.alfresco.transform.common.RequestParamMap.TARGET_EXTENSION;
+import static org.alfresco.transform.common.RequestParamMap.TARGET_MIMETYPE;
+import static org.springframework.http.HttpStatus.BAD_REQUEST;
 import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR;
 import static org.springframework.http.HttpStatus.OK;
 
@@ -52,36 +65,53 @@ import static org.springframework.http.HttpStatus.OK;
  */
 abstract class TransformProcess extends TransformStreamHandler
 {
-    private final TransformHandler transformHandler;
+    private static final List<String> NON_TRANSFORM_OPTION_REQUEST_PARAMETERS = Arrays.asList(SOURCE_EXTENSION,
-    private final TransformerDebug transformerDebug;
+        TARGET_EXTENSION, TARGET_MIMETYPE, SOURCE_MIMETYPE, DIRECT_ACCESS_URL);
+
     protected final String sourceMimetype;
     protected final String targetMimetype;
     private final Map<String, String> transformOptions;
     protected String reference;
+    private final TransformServiceRegistry transformRegistry;
+    private final TransformerDebug transformerDebug;
+    private final ProbeTransform probeTransform;
+    private final Map<String, CustomTransformer> customTransformersByName;
 
-    TransformProcess(TransformHandler transformHandler, String sourceMimetype, String targetMimetype,
+    TransformProcess(String sourceMimetype, String targetMimetype, Map<String, String> transformOptions,
-        Map<String, String> transformOptions, String reference)
+        String reference, TransformServiceRegistry transformRegistry, TransformerDebug transformerDebug,
+        ProbeTransform probeTransform, Map<String, CustomTransformer> customTransformersByName)
     {
-        LogEntry.start();
-        this.transformHandler = transformHandler;
         this.sourceMimetype = sourceMimetype;
         this.targetMimetype = targetMimetype;
-        this.transformOptions = transformHandler.cleanTransformOptions(transformOptions);
+        this.transformOptions = cleanTransformOptions(transformOptions);
         this.reference = reference;
-        this.transformerDebug = transformHandler.getTransformerDebug();
+
-        transformHandler.getProbeTransform().incrementTransformerCount();
+        this.transformRegistry = transformRegistry;
+        this.transformerDebug = transformerDebug;
+        this.probeTransform = probeTransform;
+        this.customTransformersByName = customTransformersByName;
+    }
+
+    private static Map<String, String> cleanTransformOptions(Map<String, String> requestParameters)
+    {
+        Map<String, String> transformOptions = new HashMap<>(requestParameters);
+        NON_TRANSFORM_OPTION_REQUEST_PARAMETERS.forEach(transformOptions.keySet()::remove);
+        transformOptions.values().removeIf(String::isEmpty);
+        return transformOptions;
     }
 
     public void handleTransformRequest()
     {
+        LogEntry.start();
         transformManager.setSourceMimetype(sourceMimetype);
         transformManager.setTargetMimetype(targetMimetype);
+        probeTransform.incrementTransformerCount();
         try
         {
             init();
             long sourceSizeInBytes = getSourceSize();
-            String transformName = transformHandler.getTransformerName(sourceMimetype, sourceSizeInBytes, targetMimetype, transformOptions);
+            String transformName = getTransformerName(sourceMimetype, sourceSizeInBytes, targetMimetype, transformOptions);
-            CustomTransformer customTransformer = transformHandler.getCustomTransformer(transformName);
+            CustomTransformer customTransformer = getCustomTransformer(transformName);
             transformerDebug.pushTransform(reference, sourceMimetype, targetMimetype, sourceSizeInBytes, transformName);
             transformerDebug.logOptions(reference, transformOptions);
             handleTransform(customTransformer);
@@ -101,7 +131,7 @@ abstract class TransformProcess extends TransformStreamHandler
         finally
         {
             long time = LogEntry.getTransformDuration();
-            transformHandler.getProbeTransform().recordTransformTime(time);
+            probeTransform.recordTransformTime(time);
             transformerDebug.popTransform(reference, time);
             LogEntry.complete();
         }
@@ -137,4 +167,43 @@ abstract class TransformProcess extends TransformStreamHandler
     {
         throw new TransformException(INTERNAL_SERVER_ERROR, e.getMessage(), e);
     }
+
+    private String getTransformerName(final String sourceMimetype, long sourceSizeInBytes, final String targetMimetype,
+        final Map<String, String> transformOptions)
+    {
+        // The transformOptions always contains sourceEncoding when sent to a T-Engine, even though it should not be
+        // used to select a transformer. Similar to source and target mimetypes and extensions, but these are not
+        // passed in transformOptions.
+        String sourceEncoding = transformOptions.remove(SOURCE_ENCODING);
+        try
+        {
+            final String transformerName = transformRegistry.findTransformerName(sourceMimetype,
+                sourceSizeInBytes, targetMimetype, transformOptions, null);
+            if (transformerName == null)
+            {
+                throw new TransformException(BAD_REQUEST, "No transforms for: "+
+                                                              sourceMimetype+" -> "+targetMimetype+transformOptions.entrySet().stream()
+                                                                                                                   .map(entry -> entry.getKey()+"="+entry.getValue())
+                                                                                                                   .collect(Collectors.joining(", ", " ", "")));
+            }
+            return transformerName;
+        }
+        finally
+        {
+            if (sourceEncoding != null)
+            {
+                transformOptions.put(SOURCE_ENCODING, sourceEncoding);
+            }
+        }
+    }
+
+    private CustomTransformer getCustomTransformer(String transformName)
+    {
+        CustomTransformer customTransformer = customTransformersByName.get(transformName);
+        if (customTransformer == null)
+        {
+            throw new TransformException(INTERNAL_SERVER_ERROR, "Custom Transformer "+transformName+" not found");
+        }
+        return customTransformer;
+    }
 }

engines/base/src/test/java/org/alfresco/transform/base/AbstractBaseTest.java

@@ -327,7 +327,7 @@ public abstract class AbstractBaseTest
     @Test
     public void calculateMaxTime() throws Exception
     {
-        ProbeTransform probeTransform = controller.probeTransform;
+        ProbeTransform probeTransform = controller.transformHandler.getProbeTransform();
         probeTransform.resetForTesting();
         probeTransform.setLivenessPercent(110);
 

engines/base/src/test/java/org/alfresco/transform/base/TransformControllerAllInOneTest.java

@@ -47,6 +47,7 @@ import java.util.StringJoiner;
 
 import static org.alfresco.transform.base.TransformControllerTest.assertConfig;
 import static org.alfresco.transform.base.TransformControllerTest.getLogMessagesFor;
+import static org.alfresco.transform.base.TransformControllerTest.resetProbeForTesting;
 import static org.alfresco.transform.common.Mimetype.MIMETYPE_IMAGE_JPEG;
 import static org.alfresco.transform.common.Mimetype.MIMETYPE_PDF;
 import static org.alfresco.transform.common.Mimetype.MIMETYPE_TEXT_PLAIN;
@@ -64,7 +65,6 @@ import static org.alfresco.transform.common.RequestParamMap.SOURCE_MIMETYPE;
 import static org.alfresco.transform.common.RequestParamMap.TARGET_MIMETYPE;
 import static org.hamcrest.Matchers.containsString;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -100,7 +100,6 @@ public class TransformControllerAllInOneTest
     {
         assertEquals(FakeTransformEngineWithAllInOne.class.getSimpleName(),
                 transformController.transformEngine.getClass().getSimpleName());
-        assertNotNull(transformController.probeTransform);
     }
 
     @Test
@@ -161,7 +160,7 @@ public class TransformControllerAllInOneTest
     @Test
     public void testReadyEndpointReturnsSuccessful() throws Exception
     {
-        transformController.probeTransform.resetForTesting();
+        resetProbeForTesting(transformController);
         mockMvc.perform(MockMvcRequestBuilders.get(ENDPOINT_READY))
                .andExpect(status().isOk())
                .andExpect(content().string(containsString("Success - ")));
@@ -170,7 +169,7 @@ public class TransformControllerAllInOneTest
     @Test
     public void testLiveEndpointReturnsSuccessful() throws Exception
     {
-        transformController.probeTransform.resetForTesting();
+        resetProbeForTesting(transformController);
         mockMvc.perform(MockMvcRequestBuilders.get(ENDPOINT_LIVE))
                .andExpect(status().isOk())
                .andExpect(content().string(containsString("Success - ")));

engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java

@@ -90,7 +90,6 @@ import static org.alfresco.transform.common.RequestParamMap.SOURCE_MIMETYPE;
 import static org.alfresco.transform.common.RequestParamMap.TARGET_MIMETYPE;
 import static org.hamcrest.Matchers.containsString;
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.spy;
@@ -152,12 +151,17 @@ public class TransformControllerTest
             .body((Resource) new UrlResource(sfsRef2File.get(invocation.getArguments()[0]).toURI())));
     }
 
+
+    static void resetProbeForTesting(TransformController transformController)
+    {
+        transformController.transformHandler.getProbeTransform().resetForTesting();
+    }
+
     @Test
     public void testInitEngine() throws Exception
     {
         assertEquals(FakeTransformEngineWithTwoCustomTransformers.class.getSimpleName(),
                 transformController.transformEngine.getClass().getSimpleName());
-        assertNotNull(transformController.probeTransform);
     }
 
     @Test
@@ -232,7 +236,7 @@ public class TransformControllerTest
     @Test
     public void testReadyEndpointReturnsSuccessful() throws Exception
     {
-        transformController.probeTransform.resetForTesting();
+        resetProbeForTesting(transformController);
         mockMvc.perform(MockMvcRequestBuilders.get(ENDPOINT_READY))
                .andExpect(status().isOk())
                .andExpect(content().string(containsString("Success - ")));
@@ -241,7 +245,7 @@ public class TransformControllerTest
     @Test
     public void testLiveEndpointReturnsSuccessful() throws Exception
     {
-        transformController.probeTransform.resetForTesting();
+        resetProbeForTesting(transformController);
         mockMvc.perform(MockMvcRequestBuilders.get(ENDPOINT_LIVE))
                .andExpect(status().isOk())
                .andExpect(content().string(containsString("Success - ")));
@@ -373,9 +377,6 @@ public class TransformControllerTest
                     .param("name2", PAGE_REQUEST_PARAM).param("value2", "1")
                     .param("name3", SOURCE_ENCODING).param("value3", "UTF-8"));
 
-            // Do the dispatch, just in case not doing it leaves it in a strange state.
-//            mockMvc.perform(asyncDispatch(mvcResult));
-
             verify(spy).handleHttpRequest(any(), any(), eq(MIMETYPE_TEXT_PLAIN), eq(MIMETYPE_PDF),
                 eq(ImmutableMap.of(
                     SOURCE_MIMETYPE, MIMETYPE_TEXT_PLAIN,

engines/base/src/test/java/org/alfresco/transform/base/transform/TransformStreamHandlerTest.java

@@ -48,7 +48,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
- * Tests {@link TransformStreamHandler} and {@link TransformManagerImpl#createSourceFile()} and
+ * Tests {@link TransformStreamHandler}, {@link TransformManagerImpl#createSourceFile()} and
  * {@link TransformManagerImpl#createTargetFile()} methods.
  */
 public class TransformStreamHandlerTest