From 954579036a7769f947336a8e207c16709bb0eb17 Mon Sep 17 00:00:00 2001 From: alandavis Date: Wed, 27 Jul 2022 10:10:37 +0100 Subject: [PATCH] Save point: [skip ci] * libreoffice --- .../transform/aio/AIOImageMagickTest.java | 19 --- .../transform/aio/AIOLibreOfficeTest.java | 11 -- .../transform/aio/AIOPdfRendererTest.java | 21 --- .../transform/base/TransformHandler.java | 2 +- .../transform/base/TransformRegistryImpl.java | 1 - .../base/executors/JavaExecutor.java | 40 ++++++ .../transform/base/AbstractBaseTest.java | 65 ++++------ .../imagemagick/ImageMagickTest.java | 120 +++++++++--------- .../transformers/LibreOfficeTransformer.java | 9 +- .../libreoffice/LibreOfficeTest.java | 81 ++++++------ .../org/alfresco/transform/misc/MiscTest.java | 11 +- .../pdfrenderer/PdfRendererTest.java | 99 +++++++-------- .../org/alfresco/transform/tika/TikaTest.java | 25 +--- .../registry/AbstractTransformRegistry.java | 10 +- .../registry/TransformRegistryHelper.java | 8 +- 15 files changed, 234 insertions(+), 288 deletions(-) create mode 100644 engines/base/src/main/java/org/alfresco/transform/base/executors/JavaExecutor.java diff --git a/engines/aio/src/test/java/org/alfresco/transform/aio/AIOImageMagickTest.java b/engines/aio/src/test/java/org/alfresco/transform/aio/AIOImageMagickTest.java index 1887e10f..922554fa 100644 --- a/engines/aio/src/test/java/org/alfresco/transform/aio/AIOImageMagickTest.java +++ b/engines/aio/src/test/java/org/alfresco/transform/aio/AIOImageMagickTest.java @@ -57,25 +57,6 @@ public class AIOImageMagickTest extends ImageMagickTest // mockTransformCommand("jpg", "png", "image/jpeg", true); } - @Override - protected MockHttpServletRequestBuilder mockMvcRequest(String url, MockMultipartFile sourceFile, - String... params) - { - final MockHttpServletRequestBuilder builder = super.mockMvcRequest(url, sourceFile, params) - .param("targetMimetype", targetMimetype) - .param("sourceMimetype", sourceMimetype); - - return builder; - } - -// @Test -// @Override -// public void noTargetFileTest() -// { -// // Ignore the test in super class as the AIO transforms will not be selected . -// // It is the mock that returns a zero length file for other transformers, when we supply an invalid targetExtension. -// } -// // @Test // @Override // public void testGetTransformConfigInfo() diff --git a/engines/aio/src/test/java/org/alfresco/transform/aio/AIOLibreOfficeTest.java b/engines/aio/src/test/java/org/alfresco/transform/aio/AIOLibreOfficeTest.java index 82fa9369..829de6bf 100644 --- a/engines/aio/src/test/java/org/alfresco/transform/aio/AIOLibreOfficeTest.java +++ b/engines/aio/src/test/java/org/alfresco/transform/aio/AIOLibreOfficeTest.java @@ -49,17 +49,6 @@ public class AIOLibreOfficeTest extends LibreOfficeTest // // No need to set the transform registry to the controller as it is @Autowired in // } - @Override - protected MockHttpServletRequestBuilder mockMvcRequest(String url, MockMultipartFile sourceFile, - String... params) - { - final MockHttpServletRequestBuilder builder = super.mockMvcRequest(url, sourceFile, params) - .param("targetMimetype", targetMimetype) - .param("sourceMimetype", sourceMimetype); - - return builder; - } - // @Test // @Override // public void testGetTransformConfigInfo() diff --git a/engines/aio/src/test/java/org/alfresco/transform/aio/AIOPdfRendererTest.java b/engines/aio/src/test/java/org/alfresco/transform/aio/AIOPdfRendererTest.java index 0f486d23..404a5796 100644 --- a/engines/aio/src/test/java/org/alfresco/transform/aio/AIOPdfRendererTest.java +++ b/engines/aio/src/test/java/org/alfresco/transform/aio/AIOPdfRendererTest.java @@ -40,27 +40,6 @@ public class AIOPdfRendererTest extends PdfRendererTest { @Autowired AbstractTransformRegistry transformRegistry; - @Override - protected void setFields() - { -// ReflectionTestUtils.setField(commandExecutor, "transformCommand", mockTransformCommand); -// ReflectionTestUtils.setField(commandExecutor, "checkCommand", mockCheckCommand); -// //Need to wire in the mocked commandExecutor into the controller... -// Map transformers = transformRegistry.getTransformerEngineMapping(); -// transformers.replace("pdfrenderer", commandExecutor); - } - - @Override - protected MockHttpServletRequestBuilder mockMvcRequest(String url, MockMultipartFile sourceFile, - String... params) - { - final MockHttpServletRequestBuilder builder = super.mockMvcRequest(url, sourceFile, params) - .param("targetMimetype", targetMimetype) - .param("sourceMimetype", sourceMimetype); - - return builder; - } - // @Test // @Override // public void testGetTransformConfigInfo() diff --git a/engines/base/src/main/java/org/alfresco/transform/base/TransformHandler.java b/engines/base/src/main/java/org/alfresco/transform/base/TransformHandler.java index d057ab42..8b5fb75d 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/TransformHandler.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/TransformHandler.java @@ -516,7 +516,7 @@ public class TransformHandler throw new TransformException(BAD_REQUEST, "No transforms were able to handle the request: "+ sourceMimetype+" -> "+targetMimetype+transformOptions.entrySet().stream() .map(entry -> entry.getKey()+"="+entry.getValue()) - .collect(Collectors.joining(", ", " with ", ""))); + .collect(Collectors.joining(", ", " ", ""))); } return transformerName; } diff --git a/engines/base/src/main/java/org/alfresco/transform/base/TransformRegistryImpl.java b/engines/base/src/main/java/org/alfresco/transform/base/TransformRegistryImpl.java index 4d6a02e2..97b793d7 100644 --- a/engines/base/src/main/java/org/alfresco/transform/base/TransformRegistryImpl.java +++ b/engines/base/src/main/java/org/alfresco/transform/base/TransformRegistryImpl.java @@ -32,7 +32,6 @@ import org.alfresco.transform.registry.TransformCache; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import javax.annotation.PostConstruct; import java.util.Comparator; diff --git a/engines/base/src/main/java/org/alfresco/transform/base/executors/JavaExecutor.java b/engines/base/src/main/java/org/alfresco/transform/base/executors/JavaExecutor.java new file mode 100644 index 00000000..63b9f682 --- /dev/null +++ b/engines/base/src/main/java/org/alfresco/transform/base/executors/JavaExecutor.java @@ -0,0 +1,40 @@ +/* + * #%L + * Alfresco Transform Core + * %% + * Copyright (C) 2005 - 2020 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * - + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * - + * 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 . + * #L% + */ +package org.alfresco.transform.base.executors; + +import java.io.File; + +/** + * Basic interface for executing transformations inside Java/JVM. + * + * @author Cezar Leahu + * @author adavis + */ +public interface JavaExecutor +{ + void call(File sourceFile, File targetFile, String... args) throws Exception; +} 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 ad4b4e67..5b7fe36d 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 @@ -28,6 +28,8 @@ package org.alfresco.transform.base; import com.fasterxml.jackson.databind.ObjectMapper; import org.alfresco.transform.base.clients.AlfrescoSharedFileStoreClient; +import org.alfresco.transform.base.executors.CommandExecutor; +import org.alfresco.transform.base.executors.RuntimeExec; import org.alfresco.transform.base.model.FileRefEntity; import org.alfresco.transform.base.model.FileRefResponse; import org.alfresco.transform.base.probes.ProbeTransform; @@ -42,6 +44,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.mock.web.MockMultipartFile; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.ResultActions; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; @@ -68,7 +71,6 @@ import static org.springframework.http.HttpHeaders.ACCEPT; import static org.springframework.http.HttpHeaders.CONTENT_TYPE; import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.CREATED; -import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header; @@ -86,6 +88,8 @@ public abstract class AbstractBaseTest @TempDir public File tempDir; + @Autowired + protected TransformHandler transformHandler; @Autowired protected TransformController controller; @@ -125,43 +129,32 @@ public abstract class AbstractBaseTest protected byte[] expectedTargetFileBytes; // Called by sub class + private CommandExecutor commandExecutor; + private RuntimeExec origTransformCommand; + private RuntimeExec origCheckCommand; + + protected void setMockExternalCommandsOnTransformer(CommandExecutor commandExecutor, RuntimeExec mockTransformCommand, + RuntimeExec mockCheckCommand) + { + this.commandExecutor = commandExecutor; + origTransformCommand = (RuntimeExec) ReflectionTestUtils.getField(commandExecutor, "transformCommand"); + origCheckCommand = (RuntimeExec) ReflectionTestUtils.getField(commandExecutor, "transformCommand"); + ReflectionTestUtils.setField(commandExecutor, "transformCommand", mockTransformCommand); + ReflectionTestUtils.setField(commandExecutor, "checkCommand", mockCheckCommand); + } + + protected void resetExternalCommandsOnTransformer() + { + ReflectionTestUtils.setField(commandExecutor, "transformCommand", origTransformCommand); + ReflectionTestUtils.setField(commandExecutor, "checkCommand", origCheckCommand); + } + protected abstract void mockTransformCommand(String sourceExtension, String targetExtension, String sourceMimetype, boolean readTargetFileBytes) throws IOException; - protected ProbeTransform getProbeTestTransform() - { - return controller.probeTransform; - } - protected abstract void updateTransformRequestWithSpecificOptions(TransformRequest transformRequest); -// static void assertConfig(String url, String expectedTransformers, String expectedOptions, -// MockMvc mockMvc, ObjectMapper objectMapper) throws Exception -// { -// TransformConfig config = objectMapper.readValue( -// mockMvc.perform(MockMvcRequestBuilders.get(url)) -// .andExpect(status().isOk()) -// .andExpect(header().string(CONTENT_TYPE, APPLICATION_JSON_VALUE)) -// .andReturn() -// .getResponse() -// .getContentAsString(), TransformConfig.class); -// -// // Gets a list of transformerNames,coreVersion,optionNames -// assertEquals(expectedTransformers, -// config.getTransformers().stream() -// .map(t -> t.getTransformerName()+"," -// +t.getCoreVersion()+"," -// +t.getTransformOptions().stream().sorted().collect(Collectors.joining(","))) -// .sorted() -// .collect(Collectors.joining("\n"))); -// -// assertEquals(expectedOptions, -// config.getTransformOptions().keySet().stream() -// .sorted() -// .collect(Collectors.joining(","))); -// } -// /** * This method ends up being the core of the mock. * It copies content from an existing file in the resources folder to the desired location @@ -301,14 +294,6 @@ public abstract class AbstractBaseTest } @Test - public void noTargetFileTest() throws Exception - { - mockMvc.perform(mockMvcRequest(ENDPOINT_TRANSFORM, sourceFile, "targetExtension", "xxx")) - .andExpect(status().is(INTERNAL_SERVER_ERROR.value())); - } - - @Test - // Looks dangerous but is okay as we only use the final filename public void dotDotSourceFilenameTest() throws Exception { sourceFile = new MockMultipartFile("file", "../quick." + sourceExtension, sourceMimetype, sourceFileBytes); diff --git a/engines/imagemagick/src/test/java/org/alfresco/transform/imagemagick/ImageMagickTest.java b/engines/imagemagick/src/test/java/org/alfresco/transform/imagemagick/ImageMagickTest.java index e4ca697d..03b70450 100644 --- a/engines/imagemagick/src/test/java/org/alfresco/transform/imagemagick/ImageMagickTest.java +++ b/engines/imagemagick/src/test/java/org/alfresco/transform/imagemagick/ImageMagickTest.java @@ -26,6 +26,38 @@ */ package org.alfresco.transform.imagemagick; +import org.alfresco.transform.base.AbstractBaseTest; +import org.alfresco.transform.base.executors.RuntimeExec; +import org.alfresco.transform.base.executors.RuntimeExec.ExecutionResult; +import org.alfresco.transform.base.model.FileRefEntity; +import org.alfresco.transform.base.model.FileRefResponse; +import org.alfresco.transform.client.model.TransformReply; +import org.alfresco.transform.client.model.TransformRequest; +import org.alfresco.transform.imagemagick.transformers.ImageMagickTransformer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mock; +import org.mockito.stubbing.Answer; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.Resource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.ResponseEntity; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.Map; +import java.util.UUID; + import static org.alfresco.transform.common.RequestParamMap.ENDPOINT_TRANSFORM; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -44,103 +76,68 @@ import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; import static org.springframework.http.MediaType.IMAGE_PNG_VALUE; 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.request; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.util.StringUtils.getFilenameExtension; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.util.Arrays; -import java.util.Map; -import java.util.UUID; - -import javax.annotation.PostConstruct; - -import org.alfresco.transform.client.model.TransformReply; -import org.alfresco.transform.client.model.TransformRequest; -import org.alfresco.transform.imagemagick.transformers.ImageMagickTransformer; -import org.alfresco.transform.base.AbstractBaseTest; -import org.alfresco.transform.base.executors.RuntimeExec; -import org.alfresco.transform.base.executors.RuntimeExec.ExecutionResult; -import org.alfresco.transform.base.model.FileRefEntity; -import org.alfresco.transform.base.model.FileRefResponse; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; -import org.mockito.Mock; -import org.mockito.stubbing.Answer; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.core.io.FileSystemResource; -import org.springframework.core.io.Resource; -import org.springframework.http.HttpHeaders; -import org.springframework.http.ResponseEntity; -import org.springframework.mock.web.MockMultipartFile; -import org.springframework.test.util.ReflectionTestUtils; -import org.springframework.test.web.servlet.MvcResult; -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; - /** - * Test ImageMagick. + * Test ImageMagick with mocked external command. */ public class ImageMagickTest extends AbstractBaseTest { - private static final String ENGINE_CONFIG_NAME = "imagemagick_engine_config.json"; + private static String PREFIX_IMAGE = "image/"; - private String PREFIX_IMAGE = "image/"; + @Autowired + private ImageMagickTransformer imageMagickTransformer; @Mock protected ExecutionResult mockExecutionResult; - @Mock protected RuntimeExec mockTransformCommand; - @Mock protected RuntimeExec mockCheckCommand; - @Value("${transform.core.imagemagick.exe}") protected String EXE; - @Value("${transform.core.imagemagick.dyn}") protected String DYN; - @Value("${transform.core.imagemagick.root}") protected String ROOT; - @Value("${transform.core.imagemagick.coders}") protected String CODERS; - @Value("${transform.core.imagemagick.config}") protected String CONFIG; - protected ImageMagickTransformer commandExecutor; - - @PostConstruct - private void init() - { - commandExecutor = new ImageMagickTransformer(); - } - @BeforeEach public void before() throws IOException { - ReflectionTestUtils.setField(commandExecutor, "transformCommand", mockTransformCommand); - ReflectionTestUtils.setField(commandExecutor, "checkCommand", mockCheckCommand); - ReflectionTestUtils.setField(controller, "commandExecutor", commandExecutor); + setMockExternalCommandsOnTransformer(imageMagickTransformer, mockTransformCommand, mockCheckCommand); + mockTransformCommand("jpg", "png", "image/jpeg", true); + } - mockTransformCommand("jpg", "png", "image/jpg", true); + @AfterEach + public void after() + { + resetExternalCommandsOnTransformer(); } @Override - protected void mockTransformCommand(String sourceExtension, + protected MockHttpServletRequestBuilder mockMvcRequest(String url, MockMultipartFile sourceFile, + String... params) + { + final MockHttpServletRequestBuilder builder = super.mockMvcRequest(url, sourceFile, params) + .param("targetMimetype", targetMimetype) + .param("sourceMimetype", sourceMimetype); + return builder; + } + + @Override + public void mockTransformCommand(String sourceExtension, String targetExtension, String sourceMimetype, boolean readTargetFileBytes) throws IOException { this.sourceExtension = sourceExtension; this.targetExtension = targetExtension; this.sourceMimetype = sourceMimetype; - this.targetMimetype = PREFIX_IMAGE + targetExtension; + this.targetMimetype = PREFIX_IMAGE + ("jpg".equals(targetExtension) ? "jpeg" : targetExtension); expectedOptions = null; expectedSourceSuffix = null; @@ -160,7 +157,8 @@ public class ImageMagickTest extends AbstractBaseTest assertNotNull(actualSource); assertNotNull(actualTarget); - if (expectedSourceSuffix != null) { + if (expectedSourceSuffix != null) + { assertTrue(actualSource.endsWith(expectedSourceSuffix), "The source file \"" + actualSource + "\" should have ended in \"" + expectedSourceSuffix + "\""); actualSource = actualSource.substring(0, actualSource.length() - expectedSourceSuffix.length()); @@ -350,8 +348,8 @@ public class ImageMagickTest extends AbstractBaseTest mockMvc.perform(mockMvcRequest(ENDPOINT_TRANSFORM, sourceFile, "targetExtension", "xxx")) .andExpect(status().is(BAD_REQUEST.value())) - .andExpect( - status().reason(containsString("Transformer exit code was not 0: \nSTDERR"))); + .andExpect(status() + .reason(containsString("Transformer exit code was not 0: \nSTDERR"))); } @Test diff --git a/engines/libreoffice/src/main/java/org/alfresco/transform/libreoffice/transformers/LibreOfficeTransformer.java b/engines/libreoffice/src/main/java/org/alfresco/transform/libreoffice/transformers/LibreOfficeTransformer.java index 403a03b8..d0eebe3a 100644 --- a/engines/libreoffice/src/main/java/org/alfresco/transform/libreoffice/transformers/LibreOfficeTransformer.java +++ b/engines/libreoffice/src/main/java/org/alfresco/transform/libreoffice/transformers/LibreOfficeTransformer.java @@ -29,6 +29,7 @@ package org.alfresco.transform.libreoffice.transformers; import com.fasterxml.jackson.databind.ObjectMapper; import com.sun.star.task.ErrorCodeIOException; import org.alfresco.transform.base.TransformManager; +import org.alfresco.transform.base.executors.JavaExecutor; import org.alfresco.transform.base.util.CustomTransformerFileAdaptor; import org.alfresco.transform.common.TransformException; import org.apache.commons.lang3.StringUtils; @@ -59,7 +60,7 @@ import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; * transformation logic in the same JVM (check the {@link JodConverter} implementation). */ @Component -public class LibreOfficeTransformer implements CustomTransformerFileAdaptor +public class LibreOfficeTransformer implements JavaExecutor, CustomTransformerFileAdaptor { private static final Logger logger = LoggerFactory.getLogger(LibreOfficeTransformer.class); @@ -133,6 +134,12 @@ public class LibreOfficeTransformer implements CustomTransformerFileAdaptor @Override public void transform(String sourceMimetype, String targetMimetype, Map transformOptions, File sourceFile, File targetFile) + { + call(sourceFile, targetFile); + } + + @Override + public void call(File sourceFile, File targetFile, String... args) { try { diff --git a/engines/libreoffice/src/test/java/org/alfresco/transform/libreoffice/LibreOfficeTest.java b/engines/libreoffice/src/test/java/org/alfresco/transform/libreoffice/LibreOfficeTest.java index 368b2cdd..eb8bd4ad 100644 --- a/engines/libreoffice/src/test/java/org/alfresco/transform/libreoffice/LibreOfficeTest.java +++ b/engines/libreoffice/src/test/java/org/alfresco/transform/libreoffice/LibreOfficeTest.java @@ -38,6 +38,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; import static org.springframework.http.HttpHeaders.ACCEPT; import static org.springframework.http.HttpHeaders.CONTENT_DISPOSITION; @@ -52,10 +53,10 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.util.Arrays; +import java.util.Map; import java.util.UUID; -import javax.annotation.PostConstruct; - +import org.alfresco.transform.base.CustomTransformer; import org.alfresco.transform.client.model.TransformReply; import org.alfresco.transform.client.model.TransformRequest; import org.alfresco.transform.libreoffice.transformers.LibreOfficeTransformer; @@ -64,9 +65,11 @@ import org.alfresco.transform.base.executors.RuntimeExec.ExecutionResult; import org.alfresco.transform.base.model.FileRefEntity; import org.alfresco.transform.base.model.FileRefResponse; import org.artofsolving.jodconverter.office.OfficeException; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; +import org.mockito.Spy; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.io.FileSystemResource; @@ -75,56 +78,47 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; /** - * Test LibreOffice. + * Test LibreOffice with mocked external command. */ public class LibreOfficeTest extends AbstractBaseTest { - protected String targetMimetype = MIMETYPE_PDF; + protected static String targetMimetype = MIMETYPE_PDF; + @Autowired + private LibreOfficeTransformer libreOfficeTransformer; + + @Spy + private LibreOfficeTransformer spyLibreOfficeTransformer; @Mock protected ExecutionResult mockExecutionResult; @Value("${transform.core.libreoffice.path}") private String execPath; - @Value("${transform.core.libreoffice.maxTasksPerProcess}") private String maxTasksPerProcess; - @Value("${transform.core.libreoffice.timeout}") private String timeout; - @Value("${transform.core.libreoffice.portNumbers}") private String portNumbers; - @Value("${transform.core.libreoffice.templateProfileDir}") private String templateProfileDir; - @Value("${transform.core.libreoffice.isEnabled}") private String isEnabled; - - @Autowired - private LibreOfficeTransformer libreOfficeTransformer; - - protected LibreOfficeTransformer javaExecutor; - - @PostConstruct - private void init() - { -// javaExecutor = Mockito.spy(new LibreOfficeTransformer(execPath, maxTasksPerProcess, timeout, portNumbers, templateProfileDir, isEnabled)); - } @BeforeEach public void before() throws IOException { + var customTransformersByName = (Map) ReflectionTestUtils.getField(transformHandler, "customTransformersByName"); + customTransformersByName.put("libreoffice", spyLibreOfficeTransformer); + sourceExtension = "doc"; targetExtension = "pdf"; sourceMimetype = "application/msword"; -// setJavaExecutor(controller, javaExecutor); - // The following is based on super.mockTransformCommand(...) // This is because LibreOffice used JodConverter rather than a RuntimeExec @@ -158,14 +152,24 @@ public class LibreOfficeTest extends AbstractBaseTest assertTrue(Arrays.equals(sourceFileBytes, actualSourceFileBytes), "Source file is not the same"); return null; - }).when(javaExecutor).convert(any(), any()); + }).when(spyLibreOfficeTransformer).convert(any(), any()); } - -// protected void setJavaExecutor(TransformerController controller, LibreOfficeTransformer javaExecutor) -// { -// ReflectionTestUtils.setField(controller, "javaExecutor", javaExecutor); -// } + @AfterEach + public void after() throws IOException + { + var customTransformersByName = (Map) ReflectionTestUtils.getField(transformHandler, "customTransformersByName"); + customTransformersByName.put("libreoffice", libreOfficeTransformer); + } + + @Override + protected MockHttpServletRequestBuilder mockMvcRequest(String url, MockMultipartFile sourceFile, String... params) + { + final MockHttpServletRequestBuilder builder = super.mockMvcRequest(url, sourceFile, params) + .param("targetMimetype", targetMimetype) + .param("sourceMimetype", sourceMimetype); + return builder; + } @Override protected void mockTransformCommand(String sourceExtension, String targetExtension, @@ -177,7 +181,7 @@ public class LibreOfficeTest extends AbstractBaseTest @Test public void badExitCodeTest() throws Exception { - doThrow(OfficeException.class).when(javaExecutor).convert(any(), any()); + doThrow(OfficeException.class).when(spyLibreOfficeTransformer).convert(any(), any()); mockMvc .perform(MockMvcRequestBuilders @@ -289,33 +293,34 @@ public class LibreOfficeTest extends AbstractBaseTest @Test public void testInvalidExecutorMaxTasksPerProcess() { - testInvalidValue("maxTasksPerProcess", "INVALID", maxTasksPerProcess, - "LibreOfficeTransformer MAX_TASKS_PER_PROCESS must have a numeric value"); + testInvalidValue("maxTasksPerProcess", "INVALID", + "LibreOfficeTransformer LIBREOFFICE_MAX_TASKS_PER_PROCESS must have a numeric value"); } @Test public void testInvalidExecutorTimeout() { - testInvalidValue("timeout", "INVALID", timeout, - "LibreOfficeTransformer TIMEOUT must have a numeric value"); + testInvalidValue("timeout", "INVALID", + "LibreOfficeTransformer LIBREOFFICE_TIMEOUT must have a numeric value"); } @Test public void testInvalidExecutorPortNumbers() { - testInvalidValue("portNumbers", null, portNumbers, - "LibreOfficeTransformer PORT variable cannot be null or empty"); + testInvalidValue("portNumbers", null, + "LibreOfficeTransformer LIBREOFFICE_PORT_NUMBERS variable cannot be null or empty"); } @Test public void testInvalidExecutorIsEnabled() { - testInvalidValue("isEnabled", "INVALID", isEnabled, - "LibreOfficeTransformer IS_ENABLED variable must be set to true/false"); + testInvalidValue("isEnabled", "INVALID", + "LibreOfficeTransformer LIBREOFFICE_IS_ENABLED variable must be set to true/false"); } - private void testInvalidValue(String fieldName, String invalidValue, String validValue, String expectedErrorMessage) + private void testInvalidValue(String fieldName, String invalidValue, String expectedErrorMessage) { + String validValue = (String)ReflectionTestUtils.getField(libreOfficeTransformer, fieldName); String errorMessage = ""; try { diff --git a/engines/misc/src/test/java/org/alfresco/transform/misc/MiscTest.java b/engines/misc/src/test/java/org/alfresco/transform/misc/MiscTest.java index c43c0e7f..9362b8f8 100644 --- a/engines/misc/src/test/java/org/alfresco/transform/misc/MiscTest.java +++ b/engines/misc/src/test/java/org/alfresco/transform/misc/MiscTest.java @@ -59,7 +59,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; /** - * Test Misc. + * Test Misc. Includes calling the 3rd party libraries. */ public class MiscTest extends AbstractBaseTest { @@ -84,6 +84,7 @@ public class MiscTest extends AbstractBaseTest protected void mockTransformCommand(String sourceExtension, String targetExtension, String sourceMimetype, boolean readTargetFileBytes) { + // Misc transform is not mocked. It is run for real. } @Override @@ -109,14 +110,6 @@ public class MiscTest extends AbstractBaseTest return builder; } - @Test - @Override - public void noTargetFileTest() - { - // Ignore the test in super class as the Misc transforms are real rather than mocked up. - // It is the mock that returns a zero length file for other transformers, when we supply an invalid targetExtension. - } - /** * Test transforming a valid eml file to text */ diff --git a/engines/pdfrenderer/src/test/java/org/alfresco/transform/pdfrenderer/PdfRendererTest.java b/engines/pdfrenderer/src/test/java/org/alfresco/transform/pdfrenderer/PdfRendererTest.java index 5695202d..5661cb1f 100644 --- a/engines/pdfrenderer/src/test/java/org/alfresco/transform/pdfrenderer/PdfRendererTest.java +++ b/engines/pdfrenderer/src/test/java/org/alfresco/transform/pdfrenderer/PdfRendererTest.java @@ -26,6 +26,36 @@ */ package org.alfresco.transform.pdfrenderer; +import org.alfresco.transform.base.AbstractBaseTest; +import org.alfresco.transform.base.executors.RuntimeExec; +import org.alfresco.transform.base.executors.RuntimeExec.ExecutionResult; +import org.alfresco.transform.base.model.FileRefEntity; +import org.alfresco.transform.base.model.FileRefResponse; +import org.alfresco.transform.client.model.TransformReply; +import org.alfresco.transform.client.model.TransformRequest; +import org.alfresco.transform.pdfrenderer.transformers.PdfRendererTransformer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.stubbing.Answer; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.core.io.FileSystemResource; +import org.springframework.core.io.Resource; +import org.springframework.http.HttpHeaders; +import org.springframework.http.ResponseEntity; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.Map; +import java.util.UUID; + import static org.alfresco.transform.common.RequestParamMap.ENDPOINT_TRANSFORM; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -45,81 +75,46 @@ import static org.springframework.http.MediaType.APPLICATION_PDF_VALUE; import static org.springframework.http.MediaType.IMAGE_PNG_VALUE; 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.request; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import static org.springframework.util.StringUtils.getFilenameExtension; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.util.Arrays; -import java.util.Map; -import java.util.UUID; - -import javax.annotation.PostConstruct; - -import org.alfresco.transform.client.model.TransformReply; -import org.alfresco.transform.client.model.TransformRequest; -import org.alfresco.transform.base.AbstractBaseTest; -import org.alfresco.transform.pdfrenderer.transformers.PdfRendererTransformer; -import org.alfresco.transform.base.executors.RuntimeExec; -import org.alfresco.transform.base.executors.RuntimeExec.ExecutionResult; -import org.alfresco.transform.base.model.FileRefEntity; -import org.alfresco.transform.base.model.FileRefResponse; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.mockito.Mock; -import org.mockito.stubbing.Answer; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.core.io.FileSystemResource; -import org.springframework.core.io.Resource; -import org.springframework.http.HttpHeaders; -import org.springframework.http.ResponseEntity; -import org.springframework.mock.web.MockMultipartFile; -import org.springframework.test.util.ReflectionTestUtils; -import org.springframework.test.web.servlet.MvcResult; -import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; - /** - * Test PdfRenderer. + * Test PdfRenderer with mocked external command. */ public class PdfRendererTest extends AbstractBaseTest { - private static final String ENGINE_CONFIG_NAME = "pdfrenderer_engine_config.json"; - + @Autowired + private PdfRendererTransformer pdfRendererTransformer; @Mock private ExecutionResult mockExecutionResult; - @Mock protected RuntimeExec mockTransformCommand; - @Mock protected RuntimeExec mockCheckCommand; - @Value("${transform.core.pdfrenderer.exe}") protected String execPath; - protected PdfRendererTransformer commandExecutor; - - @PostConstruct - private void init() - { - commandExecutor = new PdfRendererTransformer(); - } - @BeforeEach public void before() throws IOException { - setFields(); - + setMockExternalCommandsOnTransformer(pdfRendererTransformer, mockTransformCommand, mockCheckCommand); mockTransformCommand("pdf", "png", APPLICATION_PDF_VALUE, true); } - protected void setFields() + @AfterEach + public void after() { - ReflectionTestUtils.setField(commandExecutor, "transformCommand", mockTransformCommand); - ReflectionTestUtils.setField(commandExecutor, "checkCommand", mockCheckCommand); - ReflectionTestUtils.setField(controller, "commandExecutor", commandExecutor); + resetExternalCommandsOnTransformer(); + } + + @Override + protected MockHttpServletRequestBuilder mockMvcRequest(String url, MockMultipartFile sourceFile, + String... params) + { + final MockHttpServletRequestBuilder builder = super.mockMvcRequest(url, sourceFile, params) + .param("targetMimetype", targetMimetype) + .param("sourceMimetype", sourceMimetype); + return builder; } @Override diff --git a/engines/tika/src/test/java/org/alfresco/transform/tika/TikaTest.java b/engines/tika/src/test/java/org/alfresco/transform/tika/TikaTest.java index 491a84e4..473ac350 100644 --- a/engines/tika/src/test/java/org/alfresco/transform/tika/TikaTest.java +++ b/engines/tika/src/test/java/org/alfresco/transform/tika/TikaTest.java @@ -46,7 +46,6 @@ import org.springframework.test.web.servlet.MvcResult; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; -import javax.servlet.http.HttpServletRequest; import java.io.ByteArrayInputStream; import java.io.File; import java.io.IOException; @@ -109,7 +108,6 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. */ public class TikaTest extends AbstractBaseTest { - private static final String ENGINE_CONFIG_NAME = "tika_engine_config.json"; private static final String EXPECTED_XHTML_CONTENT_CONTAINS = "

The quick brown fox jumps over the lazy dog

"; private static final String EXPECTED_TEXT_CONTENT_CONTAINS = "The quick brown fox jumps over the lazy dog"; private static final String EXPECTED_MSG_CONTENT_CONTAINS = "Recipients\n" + @@ -141,6 +139,8 @@ public class TikaTest extends AbstractBaseTest String targetExtension, String sourceMimetype, boolean readTargetFileBytes) throws IOException { + // Tika transform is not mocked. It is run for real. + this.sourceExtension = sourceExtension; this.targetExtension = targetExtension; this.sourceMimetype = sourceMimetype; @@ -151,8 +151,6 @@ public class TikaTest extends AbstractBaseTest expectedTargetFileBytes = readTargetFileBytes ? readTestFile(targetExtension) : null; sourceFile = new MockMultipartFile("file", "quick." + sourceExtension, sourceMimetype, sourceFileBytes); - // Tika transform is not mocked. It is run for real. - when(mockExecutionResult.getExitValue()).thenReturn(0); when(mockExecutionResult.getStdErr()).thenReturn("STDERROR"); when(mockExecutionResult.getStdOut()).thenReturn("STDOUT"); @@ -193,9 +191,6 @@ public class TikaTest extends AbstractBaseTest .param("sourceMimetype", sourceMimetype); } - @Mock - HttpServletRequest httpServletRequest; - @Test @Override public void simpleTransformTest() throws Exception @@ -204,14 +199,6 @@ public class TikaTest extends AbstractBaseTest super.simpleTransformTest(); } - @Test - @Override - public void noTargetFileTest() - { - // Ignore the test in super class as the Tika transforms are real rather than mocked up. - // It is the mock that returns a zero length file for other transformers, when we supply an invalid targetExtension. - } - // --- Super class tests (need modified setup) --- @Test @@ -230,14 +217,6 @@ public class TikaTest extends AbstractBaseTest super.noExtensionSourceFilenameTest(); } - @Test - @Override - public void calculateMaxTime() throws Exception - { - mockTransformCommand(PDF, TXT, MIMETYPE_PDF, true); - super.calculateMaxTime(); - } - // --- General Tika tests --- @Test diff --git a/model/src/main/java/org/alfresco/transform/registry/AbstractTransformRegistry.java b/model/src/main/java/org/alfresco/transform/registry/AbstractTransformRegistry.java index a43e8431..7a60d94b 100644 --- a/model/src/main/java/org/alfresco/transform/registry/AbstractTransformRegistry.java +++ b/model/src/main/java/org/alfresco/transform/registry/AbstractTransformRegistry.java @@ -21,16 +21,16 @@ */ package org.alfresco.transform.registry; -import static org.alfresco.transform.registry.TransformRegistryHelper.retrieveTransformListBySize; -import static org.alfresco.transform.registry.TransformRegistryHelper.lookupTransformOptions; +import org.alfresco.transform.config.CoreFunction; +import org.alfresco.transform.config.TransformOption; +import org.alfresco.transform.config.Transformer; import java.util.List; import java.util.Map; import java.util.Set; -import org.alfresco.transform.config.CoreFunction; -import org.alfresco.transform.config.TransformOption; -import org.alfresco.transform.config.Transformer; +import static org.alfresco.transform.registry.TransformRegistryHelper.lookupTransformOptions; +import static org.alfresco.transform.registry.TransformRegistryHelper.retrieveTransformListBySize; /** * Used to work out if a transformation is supported. Sub classes should implement {@link #getData()} to return an diff --git a/model/src/main/java/org/alfresco/transform/registry/TransformRegistryHelper.java b/model/src/main/java/org/alfresco/transform/registry/TransformRegistryHelper.java index 88bb07fa..3cff2c71 100644 --- a/model/src/main/java/org/alfresco/transform/registry/TransformRegistryHelper.java +++ b/model/src/main/java/org/alfresco/transform/registry/TransformRegistryHelper.java @@ -122,12 +122,8 @@ class TransformRegistryHelper throw new TransformException(BAD_REQUEST, "Null value provided for targetMimetype, please provide a value"); } - final Map> targetMap = data.retrieveTransforms( - sourceMimetype); - - final List supportedTransformList = targetMap.getOrDefault( - targetMimetype, emptyList()); - + final Map> targetMap = data.retrieveTransforms(sourceMimetype); + final List supportedTransformList = targetMap.getOrDefault(targetMimetype, emptyList()); final List transformListBySize = new ArrayList<>(); for (SupportedTransform supportedTransform : supportedTransformList)