From 59f082d9d4ee1bc18b7738477fe8c3642b735974 Mon Sep 17 00:00:00 2001 From: alandavis Date: Thu, 8 Sep 2022 18:38:42 +0100 Subject: [PATCH] Batch 4 of changes from of review comments - done for now --- .../transform/base/TransformController.java | 4 ++-- .../transform/base/probes/ProbeTransform.java | 9 ++++----- .../base/registry/CustomTransformers.java | 16 +++++++-------- .../base/registry/TransformRegistry.java | 20 +++++++++++-------- .../base/transform/TransformHandler.java | 5 +---- .../base/transform/TransformManagerImpl.java | 4 ++-- .../transform/base/AbstractBaseTest.java | 18 ++++++++++++++++- .../base/TransformControllerTest.java | 2 +- model/pom.xml | 2 +- 9 files changed, 48 insertions(+), 32 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 7310a1e6..9c30c41c 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 @@ -233,7 +233,7 @@ public class TransformController public String ready(HttpServletRequest request) { // An alternative without transforms might be: ((TransformRegistry)transformRegistry).isReadyForTransformRequests(); - return getProbeTransform().doTransformOrNothing(request, false, transformHandler); + return getProbeTransform().doTransformOrNothing(false, transformHandler); } /** @@ -243,7 +243,7 @@ public class TransformController @ResponseBody public String live(HttpServletRequest request) { - return getProbeTransform().doTransformOrNothing(request, true, transformHandler); + return getProbeTransform().doTransformOrNothing(true, transformHandler); } public ProbeTransform getProbeTransform() 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 1bbca158..7e75151a 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 @@ -32,7 +32,6 @@ import org.alfresco.transform.common.TransformException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import javax.servlet.http.HttpServletRequest; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -121,7 +120,7 @@ public class ProbeTransform this.sourceFilename = sourceFilename; this.sourceMimetype = sourceMimetype; this.targetMimetype = targetMimetype; - this.transformOptions = new HashMap(transformOptions); + this.transformOptions = new HashMap<>(transformOptions); minExpectedLength = Math.max(0, expectedLength - plusOrMinus); maxExpectedLength = expectedLength + plusOrMinus; @@ -168,7 +167,7 @@ public class ProbeTransform } // We don't want to be doing test transforms every few seconds, but do want frequent live probes. - public String doTransformOrNothing(HttpServletRequest request, boolean isLiveProbe, TransformHandler transformHandler) + public String doTransformOrNothing(boolean isLiveProbe, TransformHandler transformHandler) { // If not initialised OR it is a live probe and we are scheduled to to do a test transform. probeCount++; @@ -215,7 +214,7 @@ public class ProbeTransform transformHandler.handleProbeRequest(sourceMimetype, targetMimetype, transformOptions, sourceFile, targetFile, this); long time = System.currentTimeMillis() - start; String message = "Transform " + time + "ms"; - checkTargetFile(targetFile, isLiveProbe, message); + checkTargetFile(targetFile, isLiveProbe); recordTransformTime(time); calculateMaxTime(time, isLiveProbe); @@ -312,7 +311,7 @@ public class ProbeTransform } } - private void checkTargetFile(File targetFile, boolean isLiveProbe, String message) + private void checkTargetFile(File targetFile, boolean isLiveProbe) { String probeMessage = getProbeMessage(isLiveProbe); if (!targetFile.exists() || !targetFile.isFile()) diff --git a/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java b/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java index 36e951cf..bd1678cf 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/registry/CustomTransformers.java @@ -45,22 +45,22 @@ public class CustomTransformers private static final Logger logger = LoggerFactory.getLogger(CustomTransformers.class); @Autowired(required = false) - private List customTransformers; + private List customTransformerList; private final Map customTransformersByName = new HashMap<>(); @PostConstruct private void initCustomTransformersByName() { - if (customTransformers != null) + if (customTransformerList != null) { - customTransformers.forEach(customTransformer -> + customTransformerList.forEach(customTransformer -> customTransformersByName.put(customTransformer.getTransformerName(), customTransformer)); - List nonNullTransformerNames = customTransformers.stream() - .map(CustomTransformer::getTransformerName) - .filter(Objects::nonNull) - .collect(Collectors.toList()); + List nonNullTransformerNames = customTransformerList.stream() + .map(CustomTransformer::getTransformerName) + .filter(Objects::nonNull) + .collect(Collectors.toList()); if (!nonNullTransformerNames.isEmpty()) { @@ -87,6 +87,6 @@ public class CustomTransformers public List toList() { - return customTransformers; + return customTransformerList; } } diff --git a/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java b/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java index c2253801..2aa05661 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/registry/TransformRegistry.java @@ -88,7 +88,7 @@ public class TransformRegistry extends AbstractTransformRegistry private boolean isTRouter; // Not autowired - avoids a circular reference in the router - initialised on startup event - private List customTransformers; + private List customTransformerList; private int previousLogMessageHashCode; @@ -127,6 +127,11 @@ public class TransformRegistry extends AbstractTransformRegistry { this.transformerByNameMap = transformerByNameMap; } + + public int getTransformCount() + { + return transformCount; + } } private Data data = new Data(); @@ -152,7 +157,7 @@ public class TransformRegistry extends AbstractTransformRegistry backoff = @Backoff(delayExpression = "#{${transform.engine.config.retry.timeout} * 1000}")) void initRegistryOnAppStartup(final ContextRefreshedEvent event) { - customTransformers = event.getApplicationContext().getBean(CustomTransformers.class).toList(); + customTransformerList = event.getApplicationContext().getBean(CustomTransformers.class).toList(); retrieveConfig(); } @@ -196,17 +201,16 @@ public class TransformRegistry extends AbstractTransformRegistry Map> transformerByNameMap = combinedTransformConfig.getTransformerByNameMap(); concurrentUpdate(combinedTransformConfig, uncombinedTransformConfig, transformConfig, transformerByNameMap); - logTransformers(uncombinedTransformConfig, combinedTransformConfig, transformerByNameMap); + logTransformers(uncombinedTransformConfig, transformerByNameMap); } - private void logTransformers(TransformConfig uncombinedTransformConfig, CombinedTransformConfig combinedTransformConfig, - Map> transformerByNameMap) + private void logTransformers(TransformConfig uncombinedTransformConfig, Map> transformerByNameMap) { if (logger.isInfoEnabled()) { - Set customTransformerNames = new HashSet(customTransformers == null + Set customTransformerNames = new HashSet<>(customTransformerList == null ? Collections.emptySet() - : customTransformers.stream().map(CustomTransformer::getTransformerName).collect(Collectors.toSet())); + : customTransformerList.stream().map(CustomTransformer::getTransformerName).collect(Collectors.toSet())); List nonNullTransformerNames = uncombinedTransformConfig.getTransformers().stream() .map(Transformer::getTransformerName) .filter(Objects::nonNull) @@ -214,7 +218,7 @@ public class TransformRegistry extends AbstractTransformRegistry ArrayList logMessages = new ArrayList<>(); if (!nonNullTransformerNames.isEmpty()) { - logMessages.add("Transformers (" + nonNullTransformerNames.size() + "):"); + logMessages.add("Transformers (" + nonNullTransformerNames.size() + ") Transforms (" + getData().getTransformCount()+ "):"); nonNullTransformerNames .stream() .sorted(String.CASE_INSENSITIVE_ORDER) 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 12692b10..67a1eb19 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 @@ -85,13 +85,10 @@ import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; @Component public class TransformHandler { - private static final Logger logger = LoggerFactory.getLogger(TransformHandler.class); private static final String FAILED_WRITING_TO_SFS = "Failed writing to SFS"; - @Autowired(required = false) - private List transformEngines; @Autowired(required = false) private CustomTransformers customTransformers; @Autowired @@ -267,7 +264,7 @@ public class TransformHandler reply.setErrorDetails(messageWithCause("Transform failed", e)); transformerDebug.logFailure(reply); - logger.trace("Transform failed. Sending " + reply, e); + logger.trace("Transform failed. Sending {}", reply, e); transformReplySender.send(replyToQueue, reply); } diff --git a/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformManagerImpl.java b/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformManagerImpl.java index 47efa34f..db910521 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformManagerImpl.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/transform/TransformManagerImpl.java @@ -202,7 +202,7 @@ public class TransformManagerImpl implements TransformManager { if (sourceFile != null && !sourceFile.delete()) { - logger.error("Failed to delete temporary source file "+sourceFile.getPath()); + logger.error("Failed to delete temporary source file {}", sourceFile.getPath()); } outputStreamLengthRecorder = null; sourceFile = null; @@ -214,7 +214,7 @@ public class TransformManagerImpl implements TransformManager { if (!keepTargetFile && targetFile != null && !targetFile.delete()) { - logger.error("Failed to delete temporary target file "+targetFile.getPath()); + logger.error("Failed to delete temporary target file {}", targetFile.getPath()); } targetFile = null; createTargetFileCalled = false; 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 2e83e765..88251244 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 @@ -61,6 +61,8 @@ import java.nio.file.Files; import java.util.HashMap; import java.util.Map; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.alfresco.transform.common.RequestParamMap.DIRECT_ACCESS_URL; @@ -284,6 +286,20 @@ public abstract class AbstractBaseTest .build(); } + public static void resetProbeForTesting(ProbeTransform probe) + { + ReflectionTestUtils.setField(probe, "probeCount", 0); + ReflectionTestUtils.setField(probe, "transCount", 0); + ReflectionTestUtils.setField(probe, "normalTime", 0); + ReflectionTestUtils.setField(probe, "maxTime", Long.MAX_VALUE); + ReflectionTestUtils.setField(probe, "nextTransformTime", 0); + + ((AtomicBoolean)ReflectionTestUtils.getField(probe, "initialised")).set(false); + ((AtomicBoolean)ReflectionTestUtils.getField(probe, "readySent")).set(false); + ((AtomicLong)ReflectionTestUtils.getField(probe, "transformCount")).set(0); + ((AtomicBoolean)ReflectionTestUtils.getField(probe, "die")).set(false); + } + @Test public void simpleTransformTest() throws Exception { @@ -325,7 +341,7 @@ public abstract class AbstractBaseTest public void calculateMaxTime() throws Exception { ProbeTransform probeTransform = controller.getProbeTransform(); - probeTransform.resetForTesting(); + resetProbeForTesting(probeTransform); probeTransform.setLivenessPercent(110); long[][] values = new long[][]{ 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 4643daa0..fad89bce 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.getProbeTransform().resetForTesting(); + AbstractBaseTest.resetProbeForTesting(transformController.getProbeTransform()); } @Test diff --git a/model/pom.xml b/model/pom.xml index fd09d775..7a702718 100644 --- a/model/pom.xml +++ b/model/pom.xml @@ -1,4 +1,4 @@ - +j 4.0.0 - Model