From 18f055bf46d06bc25eca958f395c75fc613f5a33 Mon Sep 17 00:00:00 2001 From: Denis Ungureanu Date: Mon, 20 Aug 2018 15:41:34 +0300 Subject: [PATCH 1/6] ATS-68 : ATS-16: Fix error status code mapping for expected invalid requests - validated the transformRequest on '/transform' endpoint --- .../AbstractTransformerController.java | 25 +++++++++++++++++++ .../transformer/WebApplicationConfig.java | 9 ++++++- pom.xml | 2 +- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java index f041e82e..5c02e5c6 100644 --- a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java +++ b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java @@ -39,12 +39,14 @@ import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.StringJoiner; +import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.alfresco.transform.client.model.TransformReply; import org.alfresco.transform.client.model.TransformRequest; +import org.alfresco.transform.client.model.TransformRequestValidator; import org.alfresco.transformer.model.FileRefResponse; import org.alfresco.util.TempFileProvider; import org.alfresco.util.exec.RuntimeExec; @@ -59,6 +61,8 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.ui.Model; import org.springframework.util.StringUtils; +import org.springframework.validation.DirectFieldBindingResult; +import org.springframework.validation.Errors; import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; @@ -108,6 +112,9 @@ public abstract class AbstractTransformerController @Autowired private AlfrescoSharedFileStoreClient alfrescoSharedFileStoreClient; + @Autowired + private TransformRequestValidator transformRequestValidator; + protected static Log logger; protected RuntimeExec transformCommand; @@ -158,6 +165,17 @@ public abstract class AbstractTransformerController transformReply.setSchema(transformRequest.getSchema()); transformReply.setClientData(transformRequest.getClientData()); + Errors errors = validateTransformRequest(transformRequest); + if (!errors.getAllErrors().isEmpty()) + { + transformReply.setStatus(HttpStatus.BAD_REQUEST.value()); + transformReply.setErrorDetails(errors.getAllErrors().stream().map(Object::toString) + .collect(Collectors.joining(", "))); + + return new ResponseEntity<>(transformReply, + HttpStatus.valueOf(transformReply.getStatus())); + } + // Load the source file File sourceFile; try @@ -253,6 +271,13 @@ public abstract class AbstractTransformerController return new ResponseEntity<>(transformReply, HttpStatus.valueOf(transformReply.getStatus())); } + private Errors validateTransformRequest(TransformRequest transformRequest) + { + DirectFieldBindingResult errors = new DirectFieldBindingResult(transformRequest, "request"); + transformRequestValidator.validate(transformRequest, errors); + return errors; + } + protected abstract void processTransform(File sourceFile, File targetFile, Map transformOptions, Long timeout); diff --git a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/WebApplicationConfig.java b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/WebApplicationConfig.java index 720dd662..1e9ca806 100644 --- a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/WebApplicationConfig.java +++ b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/WebApplicationConfig.java @@ -30,13 +30,14 @@ import org.springframework.context.annotation.Configuration; import org.springframework.web.client.RestTemplate; import org.springframework.web.servlet.config.annotation.InterceptorRegistry; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; +import org.alfresco.transform.client.model.TransformRequestValidator; @Configuration public class WebApplicationConfig extends WebMvcConfigurerAdapter { @Override public void addInterceptors(InterceptorRegistry registry) { - registry.addInterceptor(transformInterceptor()).addPathPatterns("/transform", "/live", "/ready");; + registry.addInterceptor(transformInterceptor()).addPathPatterns("/transform", "/live", "/ready"); } @Bean @@ -55,4 +56,10 @@ public class WebApplicationConfig extends WebMvcConfigurerAdapter { public AlfrescoSharedFileStoreClient alfrescoSharedFileStoreClient(){ return new AlfrescoSharedFileStoreClient(); } + + @Bean + public TransformRequestValidator transformRequestValidator() + { + return new TransformRequestValidator(); + } } diff --git a/pom.xml b/pom.xml index 9e25a262..95a61a7d 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ 3.0.1.1 1.2.3 ${project.version} - 0.0.4 + 0.0.5-SNAPSHOT From 0917cc520cf6664aed0805b303b2fa0a41450bd7 Mon Sep 17 00:00:00 2001 From: Denis Ungureanu Date: Mon, 20 Aug 2018 16:05:07 +0300 Subject: [PATCH 2/6] ATS-68 : ATS-16: Fix error status code mapping for expected invalid requests - updated transform-data-model dependency version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 95a61a7d..bf9b4dc8 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ 3.0.1.1 1.2.3 ${project.version} - 0.0.5-SNAPSHOT + 0.0.5 From 22573da79ccdbcb7d6c12d82e8565e56eef0dedd Mon Sep 17 00:00:00 2001 From: Denis Ungureanu Date: Mon, 20 Aug 2018 21:34:20 +0300 Subject: [PATCH 3/6] ATS-68 : ATS-16: Fix error status code mapping for expected invalid requests - updated transform-data-model dependency version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index bf9b4dc8..d0d015fa 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ 3.0.1.1 1.2.3 ${project.version} - 0.0.5 + 0.0.6 From 9fdafcb60f7d0b54713384f610c45a334ed153a2 Mon Sep 17 00:00:00 2001 From: Denis Ungureanu Date: Tue, 21 Aug 2018 14:05:31 +0300 Subject: [PATCH 4/6] ATS-68 : ATS-16: Fix error status code mapping for expected invalid requests - updated tests - added negative test for 400 reply --- .../AlfrescoPdfRendererControllerTest.java | 3 +++ .../ImageMagickControllerTest.java | 3 +++ .../LibreOfficeControllerTest.java | 3 +++ .../transformer/TikaControllerTest.java | 3 +++ .../AbstractTransformerControllerTest.java | 25 +++++++++++++++---- 5 files changed, 32 insertions(+), 5 deletions(-) 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 e65aad03..9340cd3b 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 @@ -35,6 +35,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.http.MediaType; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; @@ -102,5 +103,7 @@ public class AlfrescoPdfRendererControllerTest extends AbstractTransformerContro { transformRequest.setSourceExtension("pdf"); transformRequest.setTargetExtension("png"); + transformRequest.setSourceMediaType(MediaType.APPLICATION_PDF_VALUE); + transformRequest.setTargetMediaType(MediaType.IMAGE_PNG_VALUE); } } 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 2b69ff71..71e61fd2 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 @@ -37,6 +37,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.http.MediaType; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; @@ -176,5 +177,7 @@ public class ImageMagickControllerTest extends AbstractTransformerControllerTest { transformRequest.setSourceExtension("png"); transformRequest.setTargetExtension("png"); + transformRequest.setSourceMediaType(MediaType.IMAGE_PNG_VALUE); + transformRequest.setTargetMediaType(MediaType.IMAGE_PNG_VALUE); } } diff --git a/alfresco-docker-libreoffice/src/test/java/org/alfresco/transformer/LibreOfficeControllerTest.java b/alfresco-docker-libreoffice/src/test/java/org/alfresco/transformer/LibreOfficeControllerTest.java index 08c67b72..cab9ebea 100644 --- a/alfresco-docker-libreoffice/src/test/java/org/alfresco/transformer/LibreOfficeControllerTest.java +++ b/alfresco-docker-libreoffice/src/test/java/org/alfresco/transformer/LibreOfficeControllerTest.java @@ -48,6 +48,7 @@ import org.junit.runner.RunWith; import org.mockito.stubbing.Answer; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; @@ -133,5 +134,7 @@ public class LibreOfficeControllerTest extends AbstractTransformerControllerTest { transformRequest.setSourceExtension("doc"); transformRequest.setTargetExtension("pdf"); + transformRequest.setSourceMediaType("application/msword"); + transformRequest.setTargetMediaType(MediaType.IMAGE_PNG_VALUE); } } diff --git a/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java b/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java index 27d0e572..3e37ebb7 100644 --- a/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java +++ b/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java @@ -67,6 +67,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.SpyBean; +import org.springframework.http.MediaType; import org.springframework.mock.web.MockMultipartFile; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.test.web.servlet.MvcResult; @@ -398,6 +399,8 @@ public class TikaControllerTest extends AbstractTransformerControllerTest { transformRequest.setSourceExtension(sourceExtension); transformRequest.setTargetExtension(targetExtension); + transformRequest.setSourceMediaType(MediaType.APPLICATION_PDF_VALUE); + transformRequest.setTargetMediaType(MediaType.TEXT_PLAIN_VALUE); transformRequest.getTransformationRequestOptions().put("transform", "PdfBox"); transformRequest.getTransformationRequestOptions().put("targetMimetype", "text/plain"); transformRequest.getTransformationRequestOptions().put("targetEncoding", "UTF-8"); diff --git a/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java b/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java index ee67c2ae..bf12a790 100644 --- a/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java +++ b/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java @@ -408,15 +408,10 @@ public abstract class AbstractTransformerControllerTest transformRequest.setSchema(1); transformRequest.setClientData("Alfresco Digital Business Platform"); transformRequest.setTransformationRequestOptions(new HashMap<>()); - transformRequest.setSourceReference(sourceFileRef); transformRequest.setSourceExtension(sourceExtension); - // TODO: ATS-53 - transformRequest.setSourceMediaType("TODO"); transformRequest.setSourceSize(sourceFile.length()); - transformRequest.setTargetExtension(targetExtension); - transformRequest.setTargetMediaType("TODO"); // HTTP Request HttpHeaders headers = new HttpHeaders(); @@ -447,5 +442,25 @@ public abstract class AbstractTransformerControllerTest assertEquals(transformRequest.getSchema(), transformReply.getSchema()); } + @Test + public void testEmptyPojoTransform() throws Exception + { + // Transformation Request POJO + TransformRequest transformRequest = new TransformRequest(); + + // Serialize and call the transformer + String tr = objectMapper.writeValueAsString(transformRequest); + String transformationReplyAsString = mockMvc.perform(MockMvcRequestBuilders.post("/transform") + .header(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE) + .header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE).content(tr)) + .andExpect(status().is(HttpStatus.BAD_REQUEST.value())) + .andReturn().getResponse().getContentAsString(); + + TransformReply transformReply = objectMapper.readValue(transformationReplyAsString, TransformReply.class); + + // Assert the reply + assertEquals(HttpStatus.BAD_REQUEST.value(), transformReply.getStatus()); + } + protected abstract void updateTransformRequestWithSpecificOptions(TransformRequest transformRequest); } From 8b9593b2bdc6a557eab9b8410785f60f7f465a86 Mon Sep 17 00:00:00 2001 From: Denis Ungureanu Date: Tue, 21 Aug 2018 14:43:39 +0300 Subject: [PATCH 5/6] ATS-68 : ATS-16: Fix error status code mapping for expected invalid requests - used MediaType.TEXT_PLAIN_VALUE in test instead of "text/plain" --- .../test/java/org/alfresco/transformer/TikaControllerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java b/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java index 3e37ebb7..ed30fcc4 100644 --- a/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java +++ b/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java @@ -402,7 +402,7 @@ public class TikaControllerTest extends AbstractTransformerControllerTest transformRequest.setSourceMediaType(MediaType.APPLICATION_PDF_VALUE); transformRequest.setTargetMediaType(MediaType.TEXT_PLAIN_VALUE); transformRequest.getTransformationRequestOptions().put("transform", "PdfBox"); - transformRequest.getTransformationRequestOptions().put("targetMimetype", "text/plain"); + transformRequest.getTransformationRequestOptions().put("targetMimetype", MediaType.TEXT_PLAIN_VALUE); transformRequest.getTransformationRequestOptions().put("targetEncoding", "UTF-8"); } } From dbedbce8c61ca61a2b4b469ebb85e72f78e9432c Mon Sep 17 00:00:00 2001 From: Denis Ungureanu Date: Tue, 21 Aug 2018 15:11:22 +0300 Subject: [PATCH 6/6] ATS-68 : ATS-16: Fix error status code mapping for expected invalid requests - fixed code after updating the transform-data-model version (ATS-70) --- .../java/org/alfresco/transformer/TikaControllerTest.java | 6 +++--- .../alfresco/transformer/AbstractTransformerController.java | 2 +- .../transformer/AbstractTransformerControllerTest.java | 2 +- pom.xml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java b/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java index ed30fcc4..412be7ea 100644 --- a/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java +++ b/alfresco-docker-tika/src/test/java/org/alfresco/transformer/TikaControllerTest.java @@ -401,8 +401,8 @@ public class TikaControllerTest extends AbstractTransformerControllerTest transformRequest.setTargetExtension(targetExtension); transformRequest.setSourceMediaType(MediaType.APPLICATION_PDF_VALUE); transformRequest.setTargetMediaType(MediaType.TEXT_PLAIN_VALUE); - transformRequest.getTransformationRequestOptions().put("transform", "PdfBox"); - transformRequest.getTransformationRequestOptions().put("targetMimetype", MediaType.TEXT_PLAIN_VALUE); - transformRequest.getTransformationRequestOptions().put("targetEncoding", "UTF-8"); + transformRequest.getTransformRequestOptions().put("transform", "PdfBox"); + transformRequest.getTransformRequestOptions().put("targetMimetype", MediaType.TEXT_PLAIN_VALUE); + transformRequest.getTransformRequestOptions().put("targetEncoding", "UTF-8"); } } diff --git a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java index 5c02e5c6..64e1222f 100644 --- a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java +++ b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java @@ -215,7 +215,7 @@ public abstract class AbstractTransformerController try { processTransform(sourceFile, targetFile, - transformRequest.getTransformationRequestOptions(), timeout); + transformRequest.getTransformRequestOptions(), timeout); } catch (TransformException te) { diff --git a/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java b/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java index bf12a790..c58f893f 100644 --- a/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java +++ b/alfresco-transformer-base/src/test/java/org/alfresco/transformer/AbstractTransformerControllerTest.java @@ -407,7 +407,7 @@ public abstract class AbstractTransformerControllerTest transformRequest.setRequestId("1"); transformRequest.setSchema(1); transformRequest.setClientData("Alfresco Digital Business Platform"); - transformRequest.setTransformationRequestOptions(new HashMap<>()); + transformRequest.setTransformRequestOptions(new HashMap<>()); transformRequest.setSourceReference(sourceFileRef); transformRequest.setSourceExtension(sourceExtension); transformRequest.setSourceSize(sourceFile.length()); diff --git a/pom.xml b/pom.xml index d0d015fa..e6ee8e61 100644 --- a/pom.xml +++ b/pom.xml @@ -24,7 +24,7 @@ 3.0.1.1 1.2.3 ${project.version} - 0.0.6 + 0.0.7