diff --git a/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/pom.xml b/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/pom.xml index a8e2f48a..f58f191d 100644 --- a/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/pom.xml +++ b/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/pom.xml @@ -3,7 +3,7 @@ 4.0.0 alfresco-transform-core-aio-boot - Alfresco Core All in One Transformer Spring Boot + Alfresco Core All-In-One Transformer Spring Boot jar @@ -14,7 +14,7 @@ - alfresco/alfresco-transform-aio + alfresco/alfresco-transform-core-aio quay.io ${project.artifactId} @@ -38,12 +38,6 @@ test-jar test - org.springframework.boot diff --git a/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/src/main/java/org/alfresco/transformer/AIOController.java b/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/src/main/java/org/alfresco/transformer/AIOController.java index ea021dc5..578713a0 100644 --- a/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/src/main/java/org/alfresco/transformer/AIOController.java +++ b/alfresco-transform-core-aio/alfresco-transform-core-aio-boot/src/main/java/org/alfresco/transformer/AIOController.java @@ -32,19 +32,20 @@ import static org.alfresco.transformer.fs.FileManager.createAttachment; import static org.alfresco.transformer.fs.FileManager.createSourceFile; import static org.alfresco.transformer.fs.FileManager.createTargetFile; import static org.alfresco.transformer.fs.FileManager.createTargetFileName; +import static org.springframework.http.HttpStatus.BAD_REQUEST; +import static org.springframework.http.HttpStatus.INTERNAL_SERVER_ERROR; import static org.springframework.http.HttpStatus.OK; import static org.springframework.http.MediaType.MULTIPART_FORM_DATA_VALUE; import java.io.File; +import java.util.List; import java.util.Map; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.HashMap; -import java.util.Iterator; import javax.servlet.http.HttpServletRequest; +import org.alfresco.transform.exceptions.TransformException; import org.alfresco.transformer.logging.LogEntry; import org.alfresco.transformer.probes.ProbeTestTransform; import org.alfresco.transformer.transformers.AllInOneTransformer; @@ -63,19 +64,12 @@ public class AIOController extends AbstractTransformerController { private static final Logger logger = LoggerFactory.getLogger(AIOController.class); - //TODO Should these be moved to the AbstractTransformerController or are they present in the transform.client? They are used by most controllers... private static final String SOURCE_ENCODING = "sourceEncoding"; private static final String SOURCE_EXTENSION = "sourceExtension"; private static final String TARGET_EXTENSION = "targetExtension"; private static final String TARGET_MIMETYPE = "targetMimetype"; private static final String SOURCE_MIMETYPE = "sourceMimetype"; - private static final String TEST_DELAY = "testDelay"; - private static final String[] UNWANTED_OPTIONS = {SOURCE_EXTENSION, - TARGET_EXTENSION, - TARGET_MIMETYPE, - SOURCE_MIMETYPE, - TEST_DELAY - }; + private static final String TEST_DELAY = "testDelay"; @Autowired private AllInOneTransformer transformer; @@ -96,18 +90,23 @@ public class AIOController extends AbstractTransformerController public void processTransform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype, Map transformOptions, Long timeout) { - debugLogTransform("Processing transform", sourceMimetype, targetMimetype, transformOptions); + logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" + + " '{}', timeout {} ms", sourceFile, targetFile, transformOptions, timeout); + final String transform = getTransformerName(sourceFile, sourceMimetype, targetMimetype, transformOptions); transformOptions.put(AllInOneTransformer.TRANSFORM_NAME_PARAMETER, transform); - try { transformer.transform(sourceFile, targetFile, sourceMimetype, targetMimetype, transformOptions); - } - catch (Exception e) + } + catch (IllegalArgumentException e) { - logger.error(e.getMessage(), e); + throw new TransformException(BAD_REQUEST.value(), e.getMessage(), e); + } + catch (Exception e) + { + throw new TransformException(INTERNAL_SERVER_ERROR.value(), e.getMessage(), e); } @@ -136,7 +135,7 @@ public class AIOController extends AbstractTransformerController } catch(Exception e) { - logger.error(e.getMessage(), e); + throw new TransformException(INTERNAL_SERVER_ERROR.value(), e.getMessage(), e); } } @@ -146,19 +145,22 @@ public class AIOController extends AbstractTransformerController @PostMapping(value = "/transform", consumes = MULTIPART_FORM_DATA_VALUE) public ResponseEntity transform(HttpServletRequest request, @RequestParam("file") MultipartFile sourceMultipartFile, - @RequestParam(TARGET_EXTENSION) String targetExtension, - @RequestParam(TARGET_MIMETYPE) String targetMimetype, @RequestParam(SOURCE_MIMETYPE) String sourceMimetype, - @RequestParam Map transformOptions, + @RequestParam(TARGET_MIMETYPE) String targetMimetype, + @RequestParam(TARGET_EXTENSION) String targetExtension, + @RequestParam Map requestParameters, @RequestParam (value = TEST_DELAY, required = false) Long testDelay) { - // TODO - remove this logginng - debugLogTransform("Entering request with: ", sourceMimetype, targetMimetype, transformOptions); + debugLogTransform("Request parameters: ", sourceMimetype, targetMimetype, targetExtension, requestParameters); + //Remove all required parameters from request parameters to get the list of options + List optionsToFilter = Arrays.asList(SOURCE_EXTENSION, TARGET_EXTENSION, TARGET_MIMETYPE, + SOURCE_MIMETYPE, TEST_DELAY); + Map transformOptions = new HashMap<>(requestParameters); + transformOptions.keySet().removeAll(optionsToFilter); + transformOptions.values().removeIf(v -> v.isEmpty()); - //Using @RequestParam Map will gather all text params, including those specified seperately above. - removeUnwantedOptions(transformOptions, UNWANTED_OPTIONS, true); final String targetFilename = createTargetFileName( sourceMultipartFile.getOriginalFilename(), targetExtension); @@ -166,24 +168,25 @@ public class AIOController extends AbstractTransformerController final File sourceFile = createSourceFile(request, sourceMultipartFile); final File targetFile = createTargetFile(request, targetFilename); - // TODO Currently sourceMimetype and targetMimetype could be an empty string how does this affect getting the name? - // not all controllers take these from the request? Do requests intended for these transforms provide these? - final String transform = getTransformerName(sourceFile, sourceMimetype, targetMimetype, transformOptions); + final String transform = getTransformerName(sourceFile, sourceMimetype, targetMimetype, transformOptions); transformOptions.put(AllInOneTransformer.TRANSFORM_NAME_PARAMETER, transform); - // TODO - remove this logginng - debugLogTransform("After filtering props request with: ", sourceMimetype, targetMimetype, transformOptions); + debugLogTransform("Performing transform with parameters: ", sourceMimetype, targetMimetype, + targetExtension, requestParameters); try { transformer.transform(sourceFile, targetFile, sourceMimetype, targetMimetype, transformOptions); } - catch (Exception e) + catch (IllegalArgumentException e) { - logger.error(e.getMessage(), e); + throw new TransformException(BAD_REQUEST.value(), e.getMessage(), e); + } + catch (Exception e) + { + throw new TransformException(INTERNAL_SERVER_ERROR.value(), e.getMessage(), e); } - final ResponseEntity body = createAttachment(targetFilename, targetFile); LogEntry.setTargetSize(targetFile.length()); @@ -193,39 +196,13 @@ public class AIOController extends AbstractTransformerController return body; } - private void debugLogTransform(String message, String sourceMimetype, String targetMimetype, Map transformOptions) { + private void debugLogTransform(String message, String sourceMimetype, String targetMimetype, String targetExtension, + Map transformOptions) { if (logger.isDebugEnabled()) { logger.debug( - "{} : sourceMimetype: '{}', targetMimetype: '{}', transformOptions: '{}'", - message, sourceMimetype, targetMimetype, transformOptions); - } - } - - /** - * Removes entries from transformOptions that have keys that match a value - * contained in unwantedStrings. - * Entries that contain empty strings can optionally be removed. - * - * @param transformOptions - * @param unwantedStrings - * @param emptyStrings - */ - private void removeUnwantedOptions(Map transformOptions, String[] unwantedStrings, boolean emptyStrings) - { - for (Iterator> iter = transformOptions.entrySet().iterator();iter.hasNext();) - { - Map.Entry entry = iter.next(); - if (entry.getValue().isEmpty() || Arrays.asList(unwantedStrings).contains(entry.getKey())) - { - iter.remove(); - if (logger.isDebugEnabled()) - { - logger.debug("Key={} has been removed from the provided RequestParameters and it was passed value={}", - entry.getKey(), entry.getValue() - ); - } - } + "{} : sourceMimetype: '{}', targetMimetype: '{}', targetExtension: '{}', transformOptions: '{}'", + message, sourceMimetype, targetMimetype, targetExtension, transformOptions); } } } diff --git a/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/AbstractTransformer.java b/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/AbstractTransformer.java index aff9f669..ab299cbb 100644 --- a/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/AbstractTransformer.java +++ b/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/AbstractTransformer.java @@ -72,9 +72,6 @@ public abstract class AbstractTransformer implements Transformer return transformConfig; } - /* - * TODO - Override default config name by a configurable location defined by a property - */ private TransformConfig loadTransformConfig() throws Exception { String configFileName = getTransformerConfigPrefix() + TRANSFORMER_CONFIG_SUFFIX; diff --git a/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/Transformer.java b/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/Transformer.java index 0233f2fc..8faca730 100644 --- a/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/Transformer.java +++ b/alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/Transformer.java @@ -35,11 +35,6 @@ import java.util.Map; /** * Interface for transformers which can perform transformations and specify their own supported configuration. - * - * TODO - This could be implemented by each individual Transform engine in its own module - * and used by controllers for simplicity and clarity. Controllers could be made generic - * - * @author eknizat */ public interface Transformer { @@ -51,10 +46,6 @@ public interface Transformer /** * Implementation of the actual transformation. * - * - * TODO - Do we really need the sourceMimetype and targetMimetype as separate arguments? - * they could be passed in parameters with predefined keys like TRANSFORM_NAME_PARAMETER - * * @param sourceFile * @param targetFile * @param transformOptions @@ -67,8 +58,6 @@ public interface Transformer /** * @return Supported config for the transformer implementation. * - * TODO - maybe this does not have to be part of the common transform interface? - * */ TransformConfig getTransformConfig(); diff --git a/alfresco-transform-core-aio/alfresco-transform-core-aio/src/test/java/org/alfresco/transformer/transformers/AllInOneTransformerTest.java b/alfresco-transform-core-aio/alfresco-transform-core-aio/src/test/java/org/alfresco/transformer/transformers/AllInOneTransformerTest.java index d08391c7..b947f40d 100644 --- a/alfresco-transform-core-aio/alfresco-transform-core-aio/src/test/java/org/alfresco/transformer/transformers/AllInOneTransformerTest.java +++ b/alfresco-transform-core-aio/alfresco-transform-core-aio/src/test/java/org/alfresco/transformer/transformers/AllInOneTransformerTest.java @@ -80,7 +80,6 @@ public class AllInOneTransformerTest return new String(Files.readAllBytes(file.toPath()), encoding); } - // TODO - add more thorough test @Test public void testConfigAggregation() throws Exception { diff --git a/alfresco-transform-imagemagick/alfresco-transform-imagemagick-boot/pom.xml b/alfresco-transform-imagemagick/alfresco-transform-imagemagick-boot/pom.xml index f37250d5..bc4beab9 100644 --- a/alfresco-transform-imagemagick/alfresco-transform-imagemagick-boot/pom.xml +++ b/alfresco-transform-imagemagick/alfresco-transform-imagemagick-boot/pom.xml @@ -1,7 +1,7 @@ 4.0.0 alfresco-transform-imagemagick-boot - Alfresco ImageMagick Transformer SpringBoot + Alfresco ImageMagick Transformer Spring Boot jar diff --git a/alfresco-transform-misc/alfresco-transform-misc-boot/pom.xml b/alfresco-transform-misc/alfresco-transform-misc-boot/pom.xml index 1a1b6a81..d8f6e886 100644 --- a/alfresco-transform-misc/alfresco-transform-misc-boot/pom.xml +++ b/alfresco-transform-misc/alfresco-transform-misc-boot/pom.xml @@ -3,7 +3,7 @@ 4.0.0 alfresco-transform-misc-boot - Alfresco Docker Miscellaneous Transformers + Alfresco Miscellaneous Transformer Spring Boot jar diff --git a/alfresco-transform-misc/alfresco-transform-misc/pom.xml b/alfresco-transform-misc/alfresco-transform-misc/pom.xml index 9e22e212..2a57f185 100644 --- a/alfresco-transform-misc/alfresco-transform-misc/pom.xml +++ b/alfresco-transform-misc/alfresco-transform-misc/pom.xml @@ -3,7 +3,7 @@ 4.0.0 alfresco-transform-misc - Alfresco Miscellaneous Transformers + Alfresco Miscellaneous Transformer jar diff --git a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/TransformRegistryImpl.java b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/TransformRegistryImpl.java index efa3ade5..f08f4cea 100644 --- a/alfresco-transformer-base/src/main/java/org/alfresco/transformer/TransformRegistryImpl.java +++ b/alfresco-transformer-base/src/main/java/org/alfresco/transformer/TransformRegistryImpl.java @@ -40,10 +40,12 @@ import org.alfresco.transform.client.registry.TransformCache; import org.alfresco.transform.exceptions.TransformException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; import org.springframework.core.io.Resource; import com.fasterxml.jackson.databind.ObjectMapper; +import org.springframework.core.io.ResourceLoader; /** * Used by clients to work out if a transformation is supported based on the engine_config.json. @@ -52,13 +54,22 @@ public class TransformRegistryImpl extends AbstractTransformRegistry { private static final Logger log = LoggerFactory.getLogger(TransformRegistryImpl.class); - // TODO - do we really need this string? + @Autowired + ResourceLoader resourceLoader; + @Value("${transform.config.location:classpath:engine_config.json}") private String locationFromProperty; - @Value("${transform.config.location:classpath:engine_config.json}") private Resource engineConfig; + @PostConstruct + public void afterPropertiesSet() + { + engineConfig = resourceLoader.getResource(locationFromProperty); + TransformConfig transformConfig = getTransformConfig(); + registerAll(transformConfig, null, locationFromProperty); + } + // Holds the structures used by AbstractTransformRegistry to look up what is supported. // Unlike other sub classes this class does not extend Data or replace it at run time. private TransformCache data = new TransformCache(); @@ -74,17 +85,10 @@ public class TransformRegistryImpl extends AbstractTransformRegistry catch (IOException e) { throw new TransformException(INTERNAL_SERVER_ERROR.value(), - "Could not read " + engineConfig.getDescription(), e); + "Could not read " + locationFromProperty, e); } } - @PostConstruct - public void afterPropertiesSet() - { - TransformConfig transformConfig = getTransformConfig(); - registerAll(transformConfig, null, locationFromProperty); - } - @Override public TransformCache getData() {