From 54a7b07e55361ecff2cd3ab576c51c1563e9e50e Mon Sep 17 00:00:00 2001 From: Cezar Leahu Date: Tue, 11 Dec 2018 16:02:04 +0000 Subject: [PATCH] ATS-213 : Transformers should have unique option name --- .../AlfrescoPdfRendererController.java | 70 +++-------- .../alfresco/transformer/OptionsBuilder.java | 110 ++++++++++++++++++ .../resources/templates/transformForm.html | 4 +- .../AlfrescoPdfRendererControllerTest.java | 18 +-- .../alfresco/transformer/OptionsBuilder.java | 2 +- .../ImageMagickControllerTest.java | 5 +- alfresco-transformer-base/README.md | 12 +- .../executors/AbstractCommandExecutor.java | 25 ++-- 8 files changed, 159 insertions(+), 87 deletions(-) create mode 100644 alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/OptionsBuilder.java diff --git a/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/AlfrescoPdfRendererController.java b/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/AlfrescoPdfRendererController.java index 060fcec5..f3ba15e0 100644 --- a/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/AlfrescoPdfRendererController.java +++ b/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/AlfrescoPdfRendererController.java @@ -21,7 +21,6 @@ import static org.springframework.http.HttpStatus.OK; import java.io.File; import java.util.Arrays; import java.util.Map; -import java.util.StringJoiner; import javax.servlet.http.HttpServletRequest; @@ -110,25 +109,14 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" + " '{}', timeout {} ms", sourceFile, targetFile, transformOptions, timeout); - String page = transformOptions.get("page"); - Integer pageOption = page == null ? null : Integer.parseInt(page); - - String width = transformOptions.get("width"); - Integer widthOption = width == null ? null : Integer.parseInt(width); - - String height = transformOptions.get("height"); - Integer heightOption = height == null ? null : Integer.parseInt(height); - - String allowEnlargement = transformOptions.get("allowEnlargement"); - Boolean allowEnlargementOption = - allowEnlargement == null ? null : Boolean.parseBoolean(allowEnlargement); - - String maintainAspectRatio = transformOptions.get("maintainAspectRatio"); - Boolean maintainAspectRatioOption = - maintainAspectRatio == null ? null : Boolean.parseBoolean(maintainAspectRatio); - - String options = buildTransformOptions(pageOption, widthOption, heightOption, - allowEnlargementOption, maintainAspectRatioOption); + final String options = OptionsBuilder + .builder() + .withPage(transformOptions.get("page")) + .withWidth(transformOptions.get("width")) + .withHeight(transformOptions.get("height")) + .withAllowPdfEnlargement(transformOptions.get("allowPdfEnlargement")) + .withMaintainPdfAspectRatio(transformOptions.get("maintainPdfAspectRatio")) + .build(); commandExecutor.run(options, sourceFile, targetFile, timeout); } @@ -144,8 +132,8 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController @RequestParam(value = "page", required = false) Integer page, @RequestParam(value = "width", required = false) Integer width, @RequestParam(value = "height", required = false) Integer height, - @RequestParam(value = "allowEnlargement", required = false) Boolean allowEnlargement, - @RequestParam(value = "maintainAspectRatio", required = false) Boolean maintainAspectRatio) + @RequestParam(value = "allowPdfEnlargement", required = false) Boolean allowPdfEnlargement, + @RequestParam(value = "maintainPdfAspectRatio", required = false) Boolean maintainPdfAspectRatio) { String targetFilename = createTargetFileName(sourceMultipartFile.getOriginalFilename(), targetExtension); getProbeTestTransform().incrementTransformerCount(); @@ -153,8 +141,15 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController File targetFile = createTargetFile(request, targetFilename); // Both files are deleted by TransformInterceptor.afterCompletion - String options = buildTransformOptions(page, width, height, allowEnlargement, - maintainAspectRatio); + final String options = OptionsBuilder + .builder() + .withPage(page) + .withWidth(width) + .withHeight(height) + .withAllowPdfEnlargement(allowPdfEnlargement) + .withMaintainPdfAspectRatio(maintainPdfAspectRatio) + .build(); + commandExecutor.run(options, sourceFile, targetFile, timeout); final ResponseEntity body = createAttachment(targetFilename, targetFile); @@ -164,31 +159,4 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController getProbeTestTransform().recordTransformTime(time); return body; } - - private static String buildTransformOptions(Integer page, Integer width, Integer height, Boolean - allowEnlargement, Boolean maintainAspectRatio) - { - StringJoiner args = new StringJoiner(" "); - if (width != null && width >= 0) - { - args.add("--width=" + width); - } - if (height != null && height >= 0) - { - args.add("--height=" + height); - } - if (allowEnlargement != null && allowEnlargement) - { - args.add("--allow-enlargement"); - } - if (maintainAspectRatio != null && maintainAspectRatio) - { - args.add("--maintain-aspect-ratio"); - } - if (page != null && page >= 0) - { - args.add("--page=" + page); - } - return args.toString(); - } } diff --git a/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/OptionsBuilder.java b/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/OptionsBuilder.java new file mode 100644 index 00000000..3f69349c --- /dev/null +++ b/alfresco-docker-alfresco-pdf-renderer/src/main/java/org/alfresco/transformer/OptionsBuilder.java @@ -0,0 +1,110 @@ +package org.alfresco.transformer; + +import static org.alfresco.transformer.util.Util.stringToBoolean; +import static org.alfresco.transformer.util.Util.stringToInteger; + +import java.util.StringJoiner; + +/** + * PdfRenderer options builder. + * + * @author Cezar Leahu + */ +final class OptionsBuilder +{ + private Integer page; + private Integer width; + private Integer height; + private Boolean allowPdfEnlargement; + private Boolean maintainPdfAspectRatio; + + private OptionsBuilder() + { + } + + public OptionsBuilder withPage(final String page) + { + return withPage(stringToInteger(page)); + } + + public OptionsBuilder withPage(final Integer page) + { + this.page = page; + return this; + } + + public OptionsBuilder withWidth(final String width) + { + return withWidth(stringToInteger(width)); + } + + public OptionsBuilder withWidth(final Integer width) + { + this.width = width; + return this; + } + + public OptionsBuilder withHeight(final String height) + { + return withHeight(stringToInteger(height)); + } + + public OptionsBuilder withHeight(final Integer height) + { + this.height = height; + return this; + } + + public OptionsBuilder withAllowPdfEnlargement(final String allowPdfEnlargement) + { + return withAllowPdfEnlargement(stringToBoolean(allowPdfEnlargement)); + } + + public OptionsBuilder withAllowPdfEnlargement(final Boolean allowPdfEnlargement) + { + this.allowPdfEnlargement = allowPdfEnlargement; + return this; + } + + public OptionsBuilder withMaintainPdfAspectRatio(final String maintainPdfAspectRatio) + { + return withMaintainPdfAspectRatio(stringToBoolean(maintainPdfAspectRatio)); + } + + public OptionsBuilder withMaintainPdfAspectRatio(final Boolean maintainPdfAspectRatio) + { + this.maintainPdfAspectRatio = maintainPdfAspectRatio; + return this; + } + + public String build() + { + StringJoiner args = new StringJoiner(" "); + if (width != null && width >= 0) + { + args.add("--width=" + width); + } + if (height != null && height >= 0) + { + args.add("--height=" + height); + } + if (allowPdfEnlargement != null && allowPdfEnlargement) + { + args.add("--allow-enlargement"); + } + if (maintainPdfAspectRatio != null && maintainPdfAspectRatio) + { + args.add("--maintain-aspect-ratio"); + } + if (page != null && page >= 0) + { + args.add("--page=" + page); + } + return args.toString(); + } + + public static OptionsBuilder builder() + { + return new OptionsBuilder(); + } +} diff --git a/alfresco-docker-alfresco-pdf-renderer/src/main/resources/templates/transformForm.html b/alfresco-docker-alfresco-pdf-renderer/src/main/resources/templates/transformForm.html index 3d16a701..250adbf1 100644 --- a/alfresco-docker-alfresco-pdf-renderer/src/main/resources/templates/transformForm.html +++ b/alfresco-docker-alfresco-pdf-renderer/src/main/resources/templates/transformForm.html @@ -14,8 +14,8 @@
width
height
-
allowEnlargement
-
maintainAspectRatio
+
allowPdfEnlargement
+
maintainPdfAspectRatio
diff --git a/alfresco-docker-alfresco-pdf-renderer/src/test/java/org/alfresco/transformer/AlfrescoPdfRendererControllerTest.java b/alfresco-docker-alfresco-pdf-renderer/src/test/java/org/alfresco/transformer/AlfrescoPdfRendererControllerTest.java index 51f50afe..ca20982f 100644 --- a/alfresco-docker-alfresco-pdf-renderer/src/test/java/org/alfresco/transformer/AlfrescoPdfRendererControllerTest.java +++ b/alfresco-docker-alfresco-pdf-renderer/src/test/java/org/alfresco/transformer/AlfrescoPdfRendererControllerTest.java @@ -52,6 +52,7 @@ import org.alfresco.transformer.executors.PdfRendererCommandExecutor; import org.alfresco.transformer.model.FileRefEntity; import org.alfresco.transformer.model.FileRefResponse; import org.alfresco.util.exec.RuntimeExec; +import org.alfresco.util.exec.RuntimeExec.ExecutionResult; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -67,6 +68,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.util.StringUtils; @@ -79,7 +81,7 @@ import org.springframework.util.StringUtils; public class AlfrescoPdfRendererControllerTest extends AbstractTransformerControllerTest { @Mock - private RuntimeExec.ExecutionResult mockExecutionResult; + private ExecutionResult mockExecutionResult; @Mock private RuntimeExec mockTransformCommand; @@ -96,9 +98,9 @@ public class AlfrescoPdfRendererControllerTest extends AbstractTransformerContro @Before public void before() throws IOException { - commandExecutor.setTransformCommand(mockTransformCommand); - commandExecutor.setCheckCommand(mockCheckCommand); - + ReflectionTestUtils.setField(commandExecutor, "transformCommand", mockTransformCommand); + ReflectionTestUtils.setField(commandExecutor, "checkCommand", mockCheckCommand); + mockTransformCommand("pdf", "png", "application/pdf", true); } @@ -190,8 +192,8 @@ public class AlfrescoPdfRendererControllerTest extends AbstractTransformerContro .param("width", "321") .param("height", "654") - .param("allowEnlargement", "true") - .param("maintainAspectRatio", "true")) + .param("allowPdfEnlargement", "true") + .param("maintainPdfAspectRatio", "true")) .andExpect(status().is(OK.value())) .andExpect(content().bytes(expectedTargetFileBytes)) @@ -210,8 +212,8 @@ public class AlfrescoPdfRendererControllerTest extends AbstractTransformerContro .param("width", "321") .param("height", "654") - .param("allowEnlargement", "false") - .param("maintainAspectRatio", "false")) + .param("allowPdfEnlargement", "false") + .param("maintainPdfAspectRatio", "false")) .andExpect(status().is(OK.value())) .andExpect(content().bytes(expectedTargetFileBytes)) diff --git a/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/OptionsBuilder.java b/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/OptionsBuilder.java index 589c4b69..d9ff7864 100644 --- a/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/OptionsBuilder.java +++ b/alfresco-docker-imagemagick/src/main/java/org/alfresco/transformer/OptionsBuilder.java @@ -15,7 +15,7 @@ import org.alfresco.transformer.exceptions.TransformException; * * @author Cezar Leahu */ -public final class OptionsBuilder +final class OptionsBuilder { private static final List GRAVITY_VALUES = asList("North", "NorthEast", "East", "SouthEast", "South", "SouthWest", "West", "NorthWest", "Center"); diff --git a/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java b/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java index 80ee9779..7cea6fb7 100644 --- a/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java +++ b/alfresco-docker-imagemagick/src/test/java/org/alfresco/transformer/ImageMagickControllerTest.java @@ -67,6 +67,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.util.StringUtils; @@ -96,8 +97,8 @@ public class ImageMagickControllerTest extends AbstractTransformerControllerTest @Before public void before() throws IOException { - commandExecutor.setTransformCommand(mockTransformCommand); - commandExecutor.setCheckCommand(mockCheckCommand); + ReflectionTestUtils.setField(commandExecutor, "transformCommand", mockTransformCommand); + ReflectionTestUtils.setField(commandExecutor, "checkCommand", mockCheckCommand); mockTransformCommand("jpg", "png", "image/jpg", true); } diff --git a/alfresco-transformer-base/README.md b/alfresco-transformer-base/README.md index ad538e07..12165544 100644 --- a/alfresco-transformer-base/README.md +++ b/alfresco-transformer-base/README.md @@ -28,8 +28,8 @@ src/main/java/org/alfresco/transformer/Application.java
targetFilename *
width
height
-
allowEnlargement
-
maintainAspectRatio
+
allowPdfEnlargement
+
maintainPdfAspectRatio
page
timeout
@@ -59,8 +59,8 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController @RequestParam("targetFilename") String targetFilename, @RequestParam(value = "width", required = false) Integer width, @RequestParam(value = "height", required = false) Integer height, - @RequestParam(value = "allowEnlargement", required = false) Boolean allowEnlargement, - @RequestParam(value = "maintainAspectRatio", required = false) Boolean maintainAspectRatio, + @RequestParam(value = "allowPdfEnlargement", required = false) Boolean allowPdfEnlargement, + @RequestParam(value = "maintainPdfAspectRatio", required = false) Boolean maintainPdfAspectRatio, @RequestParam(value = "page", required = false) Integer page, @RequestParam(value = "timeout", required = false) Long timeout) { @@ -79,11 +79,11 @@ public class AlfrescoPdfRendererController extends AbstractTransformerController { args.add("--height=" + height); } - if (allowEnlargement != null && allowEnlargement) + if (allowPdfEnlargement != null && allowPdfEnlargement) { args.add("--allow-enlargement"); } - if (maintainAspectRatio != null && maintainAspectRatio) + if (maintainPdfAspectRatio != null && maintainPdfAspectRatio) { args.add("--maintain-aspect-ratio"); } diff --git a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/executors/AbstractCommandExecutor.java b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/executors/AbstractCommandExecutor.java index ee54490b..f889cfba 100644 --- a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/executors/AbstractCommandExecutor.java +++ b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/executors/AbstractCommandExecutor.java @@ -8,32 +8,24 @@ import java.util.Map; import org.alfresco.transformer.exceptions.TransformException; import org.alfresco.util.exec.RuntimeExec; +import org.alfresco.util.exec.RuntimeExec.ExecutionResult; /** */ public abstract class AbstractCommandExecutor implements CommandExecutor { - private RuntimeExec transformCommand = createTransformCommand(); - private RuntimeExec checkCommand = createCheckCommand(); + private final RuntimeExec transformCommand = createTransformCommand(); + private final RuntimeExec checkCommand = createCheckCommand(); protected abstract RuntimeExec createTransformCommand(); protected abstract RuntimeExec createCheckCommand(); - // todo remove these setters and and make the fields final - public void setTransformCommand(RuntimeExec re) { - transformCommand = re; - } - - public void setCheckCommand(RuntimeExec re) { - checkCommand = re; - } - @Override public void run(Map properties, File targetFile, Long timeout) { timeout = timeout != null && timeout > 0 ? timeout : 0; - RuntimeExec.ExecutionResult result = transformCommand.execute(properties, timeout); + final ExecutionResult result = transformCommand.execute(properties, timeout); if (result.getExitValue() != 0 && result.getStdErr() != null && result.getStdErr().length() > 0) { @@ -51,24 +43,23 @@ public abstract class AbstractCommandExecutor implements CommandExecutor @Override public String version() { - String version = "Version not checked"; if (checkCommand != null) { - RuntimeExec.ExecutionResult result = checkCommand.execute(); + final ExecutionResult result = checkCommand.execute(); if (result.getExitValue() != 0 && result.getStdErr() != null && result.getStdErr().length() > 0) { throw new TransformException(INTERNAL_SERVER_ERROR.value(), "Transformer version check exit code was not 0: \n" + result); } - version = result.getStdOut().trim(); + final String version = result.getStdOut().trim(); if (version.isEmpty()) { throw new TransformException(INTERNAL_SERVER_ERROR.value(), "Transformer version check failed to create any output"); } + return version; } - - return version; + return "Version not checked"; } }