From d1e811cef400a6f4645bfb7cbe69311a714f070a Mon Sep 17 00:00:00 2001 From: alandavis Date: Thu, 11 Aug 2022 08:35:47 +0100 Subject: [PATCH] Move transformEngine from TransformHandler to transformControl as that is a better fit Also reduces the number of reads of the engine_config.json --- .../transform/base/TransformController.java | 42 ++++++++++++--- .../base/messaging/QueueTransformService.java | 6 +-- .../transform/base/probes/ProbeTransform.java | 2 +- .../base/transform/ProcessHandler.java | 8 +-- .../base/transform/TransformHandler.java | 54 +++---------------- .../transform/base/AbstractBaseTest.java | 2 +- .../base/TransformControllerTest.java | 4 +- .../messaging/QueueTransformServiceTest.java | 24 ++++----- .../base/transform/FragmentHandlerTest.java | 5 +- 9 files changed, 69 insertions(+), 78 deletions(-) diff --git a/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java b/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java index ea9841bc..69ed2f1f 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/TransformController.java @@ -28,6 +28,7 @@ package org.alfresco.transform.base; import org.alfresco.transform.base.html.OptionLister; import org.alfresco.transform.base.logging.LogEntry; +import org.alfresco.transform.base.probes.ProbeTransform; import org.alfresco.transform.base.registry.TransformRegistry; import org.alfresco.transform.base.transform.TransformHandler; import org.alfresco.transform.client.model.TransformReply; @@ -58,11 +59,13 @@ import org.springframework.web.multipart.MultipartFile; import org.springframework.web.servlet.ModelAndView; import javax.annotation.PostConstruct; +import javax.jms.Destination; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.util.Arrays; import java.util.Collection; +import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -111,9 +114,28 @@ public class TransformController TransformEngine transformEngine; @PostConstruct - private void init() + private void initTransformEngine() { - transformEngine = transformHandler.getTransformEngine(); + if (transformEngines != null) + { + transformEngine = getTransformEngine(); + logger.info("TransformEngine: " + transformEngine.getTransformEngineName()); + transformEngines.stream() + .filter(te -> te != transformEngine) + .sorted(Comparator.comparing(TransformEngine::getTransformEngineName)) + .map(transformEngine -> " "+transformEngine.getTransformEngineName()).forEach(logger::info); + } + } + + private TransformEngine getTransformEngine() + { + // 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)); } @EventListener(ApplicationReadyEvent.class) @@ -206,7 +228,7 @@ public class TransformController @ResponseBody public String ready(HttpServletRequest request) { - return transformHandler.probe(request, false); + return getProbeTransform().doTransformOrNothing(request, false, transformHandler); } /** @@ -216,7 +238,12 @@ public class TransformController @ResponseBody public String live(HttpServletRequest request) { - return transformHandler.probe(request, true); + return getProbeTransform().doTransformOrNothing(request, true, transformHandler); + } + + public ProbeTransform getProbeTransform() + { + return transformEngine.getProbeTransform(); } @GetMapping(value = ENDPOINT_TRANSFORM_CONFIG) @@ -234,9 +261,10 @@ public class TransformController @PostMapping(value = ENDPOINT_TRANSFORM, produces = APPLICATION_JSON_VALUE) @ResponseBody public ResponseEntity transform(@RequestBody TransformRequest request, - @RequestParam(value = "timeout", required = false) Long timeout) + @RequestParam(value = "timeout", required = false) Long timeout, + @RequestParam(value = "replyToQueue", required = false) Destination replyToQueue) { - TransformReply reply = transformHandler.handleMessageRequest(request, timeout, null); + TransformReply reply = transformHandler.handleMessageRequest(request, timeout, replyToQueue, getProbeTransform()); return new ResponseEntity<>(reply, HttpStatus.valueOf(reply.getStatus())); } @@ -249,7 +277,7 @@ public class TransformController @RequestParam Map requestParameters) { return transformHandler.handleHttpRequest(request, sourceMultipartFile, sourceMimetype, - targetMimetype, requestParameters); + targetMimetype, requestParameters, getProbeTransform()); } // Used the t-engine's simple html test UI. diff --git a/engines/base/src/main/java/org/alfresco/transform/base/messaging/QueueTransformService.java b/engines/base/src/main/java/org/alfresco/transform/base/messaging/QueueTransformService.java index 9c296ff5..2cfcabad 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/messaging/QueueTransformService.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/messaging/QueueTransformService.java @@ -26,7 +26,7 @@ */ package org.alfresco.transform.base.messaging; -import org.alfresco.transform.base.transform.TransformHandler; +import org.alfresco.transform.base.TransformController; import org.alfresco.transform.client.model.TransformReply; import org.alfresco.transform.client.model.TransformRequest; import org.alfresco.transform.common.TransformException; @@ -63,7 +63,7 @@ public class QueueTransformService private static final Logger logger = LoggerFactory.getLogger(QueueTransformService.class); @Autowired - private TransformHandler transformHandler; + private TransformController transformController; @Autowired private TransformMessageConverter transformMessageConverter; @Autowired @@ -123,7 +123,7 @@ public class QueueTransformService return; } - transformHandler.handleMessageRequest(transformRequest.get(), null, replyToQueue); + transformController.transform(transformRequest.get(), null, replyToQueue); } /** diff --git a/engines/base/src/main/java/org/alfresco/transform/base/probes/ProbeTransform.java b/engines/base/src/main/java/org/alfresco/transform/base/probes/ProbeTransform.java index bff18096..2d5dccb5 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/probes/ProbeTransform.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/probes/ProbeTransform.java @@ -212,7 +212,7 @@ public class ProbeTransform File sourceFile = getSourceFile(isLiveProbe); File targetFile = getTargetFile(); - transformHandler.handleProbRequest(sourceMimetype, targetMimetype, transformOptions, sourceFile, targetFile); + transformHandler.handleProbRequest(sourceMimetype, targetMimetype, transformOptions, sourceFile, targetFile, this); long time = System.currentTimeMillis() - start; String message = "Transform " + time + "ms"; checkTargetFile(targetFile, isLiveProbe, message); diff --git a/engines/base/src/main/java/org/alfresco/transform/base/transform/ProcessHandler.java b/engines/base/src/main/java/org/alfresco/transform/base/transform/ProcessHandler.java index 62c45da4..27142953 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/transform/ProcessHandler.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/transform/ProcessHandler.java @@ -59,10 +59,10 @@ import static org.springframework.http.HttpStatus.OK; /** * Provides the transform logic common to http (upload/download), message and probe requests. See - * {@link TransformHandler#handleHttpRequest(HttpServletRequest, MultipartFile, String, String, Map)}, - * {@link TransformHandler#handleMessageRequest(TransformRequest, Long, Destination)} and - * {@link TransformHandler#handleProbRequest(String, String, Map, File, File)}. Note the handing of transform requests - * via a message queue is the same as via the {@link TransformController#transform(TransformRequest, Long)}. + * {@link TransformHandler#handleHttpRequest(HttpServletRequest, MultipartFile, String, String, Map, ProbeTransform)}, + * {@link TransformHandler#handleMessageRequest(TransformRequest, Long, Destination, ProbeTransform)} and + * {@link TransformHandler#handleProbRequest(String, String, Map, File, File, ProbeTransform)}. Note the handing of transform requests + * via a message queue is the same as via the {@link TransformController#transform(TransformRequest, Long, Destination)}. */ abstract class ProcessHandler extends FragmentHandler { diff --git a/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformHandler.java b/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformHandler.java index ac893e1e..4721cb2d 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformHandler.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformHandler.java @@ -106,29 +106,9 @@ public class TransformHandler private TransformerDebug transformerDebug; private final AtomicInteger httpRequestCount = new AtomicInteger(1); - private TransformEngine transformEngine; private final Map customTransformersByName = new HashMap<>(); @PostConstruct - private void init() - { - initTransformEngine(); - initCustomTransformersByName(); - } - - private void initTransformEngine() - { - if (transformEngines != null) - { - transformEngine = getTransformEngine(); - logger.info("TransformEngine: " + transformEngine.getTransformEngineName()); - transformEngines.stream() - .filter(te -> te != transformEngine) - .sorted(Comparator.comparing(TransformEngine::getTransformEngineName)) - .map(transformEngine -> " "+transformEngine.getTransformEngineName()).forEach(logger::info); - } - } - private void initCustomTransformersByName() { if (customTransformers != null) @@ -143,31 +123,15 @@ public class TransformHandler } } - public TransformEngine getTransformEngine() - { - // 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 transformEngine.getProbeTransform(); - } - public ResponseEntity handleHttpRequest(HttpServletRequest request, MultipartFile sourceMultipartFile, String sourceMimetype, String targetMimetype, - Map requestParameters) + Map requestParameters, ProbeTransform probeTransform) { AtomicReference> responseEntity = new AtomicReference<>(); new ProcessHandler(sourceMimetype, targetMimetype, requestParameters, "e" + httpRequestCount.getAndIncrement(), transformRegistry, - transformerDebug, getProbeTransform(), customTransformersByName) + transformerDebug, probeTransform, customTransformersByName) { @Override protected void init() throws IOException @@ -207,11 +171,11 @@ public class TransformHandler } public void handleProbRequest(String sourceMimetype, String targetMimetype, Map transformOptions, - File sourceFile, File targetFile) + File sourceFile, File targetFile, ProbeTransform probeTransform) { new ProcessHandler(sourceMimetype, targetMimetype, transformOptions, "p" + httpRequestCount.getAndIncrement(), transformRegistry, - transformerDebug, getProbeTransform(), customTransformersByName) + transformerDebug, probeTransform, customTransformersByName) { @Override protected void init() throws IOException @@ -242,12 +206,13 @@ public class TransformHandler }.handleTransformRequest(); } - public TransformReply handleMessageRequest(TransformRequest request, Long timeout, Destination replyToQueue) + public TransformReply handleMessageRequest(TransformRequest request, Long timeout, Destination replyToQueue, + ProbeTransform probeTransform) { TransformReply reply = createBasicTransformReply(request); new ProcessHandler(request.getSourceMediaType(), request.getTargetMediaType(), request.getTransformRequestOptions(),"unset", transformRegistry, - transformerDebug, getProbeTransform(), customTransformersByName) + transformerDebug, probeTransform, customTransformersByName) { @Override protected void init() throws IOException @@ -304,11 +269,6 @@ 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); diff --git a/engines/base/src/test/java/org/alfresco/transform/base/AbstractBaseTest.java b/engines/base/src/test/java/org/alfresco/transform/base/AbstractBaseTest.java index b6a15942..af3b4dfa 100644 --- a/engines/base/src/test/java/org/alfresco/transform/base/AbstractBaseTest.java +++ b/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.transformHandler.getProbeTransform(); + ProbeTransform probeTransform = controller.getProbeTransform(); probeTransform.resetForTesting(); probeTransform.setLivenessPercent(110); diff --git a/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java b/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java index 650a01a1..61ee8515 100644 --- a/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java +++ b/engines/base/src/test/java/org/alfresco/transform/base/TransformControllerTest.java @@ -131,7 +131,7 @@ public class TransformControllerTest static void resetProbeForTesting(TransformController transformController) { - transformController.transformHandler.getProbeTransform().resetForTesting(); + transformController.getProbeTransform().resetForTesting(); } @Test @@ -446,7 +446,7 @@ public class TransformControllerTest SOURCE_MIMETYPE, MIMETYPE_TEXT_PLAIN, TARGET_MIMETYPE, MIMETYPE_PDF, PAGE_REQUEST_PARAM, "1", - SOURCE_ENCODING, "UTF-8"))); + SOURCE_ENCODING, "UTF-8")), any()); } finally { diff --git a/engines/base/src/test/java/org/alfresco/transform/base/messaging/QueueTransformServiceTest.java b/engines/base/src/test/java/org/alfresco/transform/base/messaging/QueueTransformServiceTest.java index be31d970..45022a73 100644 --- a/engines/base/src/test/java/org/alfresco/transform/base/messaging/QueueTransformServiceTest.java +++ b/engines/base/src/test/java/org/alfresco/transform/base/messaging/QueueTransformServiceTest.java @@ -27,7 +27,7 @@ package org.alfresco.transform.base.messaging; -import org.alfresco.transform.base.transform.TransformHandler; +import org.alfresco.transform.base.TransformController; import org.alfresco.transform.client.model.TransformReply; import org.alfresco.transform.client.model.TransformRequest; import org.apache.activemq.command.ActiveMQObjectMessage; @@ -56,7 +56,7 @@ import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; public class QueueTransformServiceTest { @Mock - private TransformHandler transformHandler; + private TransformController transformController; @Mock private TransformMessageConverter transformMessageConverter; @Mock @@ -76,7 +76,7 @@ public class QueueTransformServiceTest { queueTransformService.receive(null); - verifyNoMoreInteractions(transformHandler); + verifyNoMoreInteractions(transformController); verifyNoMoreInteractions(transformMessageConverter); verifyNoMoreInteractions(transformReplySender); } @@ -86,7 +86,7 @@ public class QueueTransformServiceTest { queueTransformService.receive(new ActiveMQObjectMessage()); - verifyNoMoreInteractions(transformHandler); + verifyNoMoreInteractions(transformController); verifyNoMoreInteractions(transformMessageConverter); verifyNoMoreInteractions(transformReplySender); } @@ -114,7 +114,7 @@ public class QueueTransformServiceTest verify(transformMessageConverter).fromMessage(msg); verify(transformReplySender).send(destination, reply, msg.getCorrelationId()); - verifyNoMoreInteractions(transformHandler); + verifyNoMoreInteractions(transformController); } @Test @@ -141,7 +141,7 @@ public class QueueTransformServiceTest verify(transformMessageConverter).fromMessage(msg); verify(transformReplySender).send(destination, reply, msg.getCorrelationId()); - verifyNoMoreInteractions(transformHandler); + verifyNoMoreInteractions(transformController); } @Test @@ -168,7 +168,7 @@ public class QueueTransformServiceTest verify(transformMessageConverter).fromMessage(msg); verify(transformReplySender).send(destination, reply, msg.getCorrelationId()); - verifyNoMoreInteractions(transformHandler); + verifyNoMoreInteractions(transformController); } @Test @@ -186,12 +186,12 @@ public class QueueTransformServiceTest doReturn(request).when(transformMessageConverter).fromMessage(msg); doAnswer(invocation -> {transformReplySender.send(destination, reply); return null;}) - .when(transformHandler).handleMessageRequest(request, null, destination); + .when(transformController).transform(request, null, destination); queueTransformService.receive(msg); verify(transformMessageConverter).fromMessage(msg); - verify(transformHandler).handleMessageRequest(request, null, destination); + verify(transformController).transform(request, null, destination); verify(transformReplySender).send(destination, reply); } @@ -204,7 +204,7 @@ public class QueueTransformServiceTest queueTransformService.receive(msg); - verifyNoMoreInteractions(transformHandler); + verifyNoMoreInteractions(transformController); verifyNoMoreInteractions(transformMessageConverter); verifyNoMoreInteractions(transformReplySender); } @@ -227,12 +227,12 @@ public class QueueTransformServiceTest doReturn(request).when(transformMessageConverter).fromMessage(msg); doAnswer(invocation -> {transformReplySender.send(destination, reply); return null;}) - .when(transformHandler).handleMessageRequest(request, null, destination); + .when(transformController).transform(request, null, destination); queueTransformService.receive(msg); verify(transformMessageConverter).fromMessage(msg); - verify(transformHandler).handleMessageRequest(request, null, destination); + verify(transformController).transform(request, null, destination); verify(transformReplySender).send(destination, reply); } } diff --git a/engines/base/src/test/java/org/alfresco/transform/base/transform/FragmentHandlerTest.java b/engines/base/src/test/java/org/alfresco/transform/base/transform/FragmentHandlerTest.java index 4226682f..74bcda1e 100644 --- a/engines/base/src/test/java/org/alfresco/transform/base/transform/FragmentHandlerTest.java +++ b/engines/base/src/test/java/org/alfresco/transform/base/transform/FragmentHandlerTest.java @@ -33,6 +33,7 @@ import org.alfresco.transform.base.fakes.FakeTransformerFragments; import org.alfresco.transform.base.messaging.TransformReplySender; import org.alfresco.transform.base.model.FileRefEntity; import org.alfresco.transform.base.model.FileRefResponse; +import org.alfresco.transform.base.probes.ProbeTransform; import org.alfresco.transform.client.model.TransformReply; import org.alfresco.transform.client.model.TransformRequest; import org.apache.commons.lang3.tuple.Pair; @@ -89,6 +90,8 @@ public class FragmentHandlerTest protected AlfrescoSharedFileStoreClient fakeSfsClient; @MockBean private TransformReplySender transformReplySender; + @MockBean + private ProbeTransform probeTransform; private void assertFragments(String sourceText, String expectedError, List expectedLines) { @@ -127,7 +130,7 @@ public class FragmentHandlerTest .withSourceSize(32L) .withInternalContextForTransformEngineTests() .build(); - transformHandler.handleMessageRequest(request, Long.MAX_VALUE, null); + transformHandler.handleMessageRequest(request, Long.MAX_VALUE, null, probeTransform); TransformReply lastReply = replies.get(replies.size() - 1).getRight(); String errorDetails = lastReply.getErrorDetails();