ATS-711: Update JavaDoc and format code (#217)

* Update JavaDoc and format code
* Add more tests

alfresco-transform-core-aio/alfresco-transform-core-aio-boot/src/main/java/org/alfresco/transformer/AIOController.java

@@ -87,13 +87,11 @@ public class AIOController extends AbstractTransformerController
     }
 
     @Override
-    public void processTransform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
+    public void processTransform(final File sourceFile, final File targetFile, final String sourceMimetype,
-            Map<String, String> transformOptions, Long timeout) 
+                                 final String targetMimetype, final Map<String, String> transformOptions, final Long timeout)
     {
-
         final String transform = getTransformerName(sourceFile, sourceMimetype, targetMimetype, transformOptions);
         transformInternal( transform, sourceFile, targetFile, MIMETYPE_HTML, MIMETYPE_TEXT_PLAIN, transformOptions);
-
     }
 
     // TODO ATS-713 Currently uses the Misc probeTest. The implementation will need to be changed such that the test can be selected based on the required transform
@@ -113,8 +111,6 @@ public class AIOController extends AbstractTransformerController
                 parameters.put(SOURCE_ENCODING, "UTF-8");
                 transformInternal( "misc", sourceFile, targetFile, MIMETYPE_HTML,
                             MIMETYPE_TEXT_PLAIN, parameters);
-            
-                
             }
         };
     }
@@ -138,7 +134,6 @@ public class AIOController extends AbstractTransformerController
         transformOptions.keySet().removeAll(optionsToFilter);
         transformOptions.values().removeIf(v -> v.isEmpty());
 
-
         final String targetFilename = createTargetFileName(
             sourceMultipartFile.getOriginalFilename(), targetExtension);
         getProbeTestTransform().incrementTransformerCount();
@@ -150,8 +145,6 @@ public class AIOController extends AbstractTransformerController
                 targetExtension, transformOptions);
         transformInternal(transform, sourceFile, targetFile, sourceMimetype, targetMimetype, transformOptions);
 
-
-
         final ResponseEntity<Resource> body = createAttachment(targetFilename, targetFile);
         LogEntry.setTargetSize(targetFile.length());
         long time = LogEntry.setStatusCodeAndMessage(OK.value(), "Success");
@@ -160,8 +153,8 @@ public class AIOController extends AbstractTransformerController
         return body;
     }
 
-    private void debugLogTransform(String message, String sourceMimetype, String targetMimetype, String targetExtension,
+    private void debugLogTransform(final String message, final String sourceMimetype, final String targetMimetype,
-                                   Map<String, String> transformOptions) {
+                                   final String targetExtension, final Map<String, String> transformOptions) {
         if (logger.isDebugEnabled())
         {
             logger.debug(
@@ -173,29 +166,19 @@ public class AIOController extends AbstractTransformerController
     @Override
     public ResponseEntity<TransformConfig> info()
     {
-        TransformConfig transformConfig = new TransformConfig();
         logger.info("GET Transform Config.");
-        try
+        TransformConfig transformConfig = transformRegistry.getTransformConfig();
-        {
-            transformConfig = transformRegistry.getTransformConfig();
-        }
-        catch (Exception e)
-        {
-            throw new TransformException(INTERNAL_SERVER_ERROR.value(), e.getMessage(), e);
-        }
-
         return new ResponseEntity<>(transformConfig, OK);
     }
 
-    protected void transformInternal(String transformName, File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
+    protected void transformInternal(final String transformName, final File sourceFile, final File targetFile,
-                      Map<String, String> transformOptions)
+                                     final String sourceMimetype, final String targetMimetype,
+                                     final Map<String, String> transformOptions)
     {
         logger.debug("Processing request with: sourceFile '{}', targetFile '{}', transformOptions" +
                 " '{}', timeout {} ms", sourceFile, targetFile, transformOptions);
 
         Transformer transformer = transformRegistry.getByTransformName(transformName);
-
-
         if (transformer == null)
         {
             new TransformException(INTERNAL_SERVER_ERROR.value(), "No transformer mapping for - transform:"
@@ -204,7 +187,7 @@ public class AIOController extends AbstractTransformerController
 
         if (logger.isDebugEnabled())
         {
-            logger.debug("Performing transform '{}' using {}", transformName, transformer.getClass().getSimpleName());
+            logger.debug("Performing transform '{}' using {}", transformName, transformer.getTransformerId());
         }
 
         try
@@ -227,6 +210,5 @@ public class AIOController extends AbstractTransformerController
             throw new TransformException(INTERNAL_SERVER_ERROR.value(), "Failed transform - transform:"
                     + transformName + " sourceMimetype:" + sourceMimetype + " targetMimetype:" + targetMimetype);
         }
-
     }
 }

alfresco-transform-core-aio/alfresco-transform-core-aio-boot/src/main/java/org/alfresco/transformer/AIOCustomConfig.java

@@ -55,5 +55,4 @@ public class AIOCustomConfig
         aioTransformRegistry.registerTransformer(new PdfRendererAdapter());
         return aioTransformRegistry;
     }
-
 }

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/AIOTransformRegistry.java

@@ -50,15 +50,13 @@ import java.util.Set;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.springframework.http.HttpStatus.BAD_REQUEST;
 
-/***
+/**
- *
+ * AIOTransformRegistry manages all of the sub transformers registered to it and provides aggregated TransformConfig.
- * @author eknizat
  */
 public class AIOTransformRegistry extends AbstractTransformRegistry
 {
     private static final Logger log = LoggerFactory.getLogger(AIOTransformRegistry.class);
 
-
     private static final String ENGINE_CONFIG_LOCATION_POSTFIX = "_engine_config.json";
 
     private TransformConfig aggregatedConfig = new TransformConfig();
@@ -69,39 +67,35 @@ public class AIOTransformRegistry extends AbstractTransformRegistry
 
     private ObjectMapper jsonObjectMapper = new ObjectMapper();
 
-
+    //Represents the mapping between a transform and a transformer, multiple mappings can point to the same transformer.
-    /**
-     * Represents the mapping between a transform and a transformer, multiple mappings can point to the same transformer.
-     */
     private Map<String, Transformer> transformerTransformMapping = new HashMap();
 
-
     /**
      * The registration will go through all supported sub transformers and map them to the transformer implementation.
-     *
+     * @param transformer The transformer implementation, this could be a single transformer
-     * @param transformer The transformer implementation,
+     *                    or a transformer managing multiple sub transformers. The transformer's configuration file will
-     *                    this could be a transformer managing multiple sub transformers.
+     *                    be read based on the {@link Transformer#getTransformerId()} value.
      * @throws Exception Exception is thrown if a mapping for a transformer name already exists.
      */
-    public void registerTransformer(Transformer transformer) throws Exception
+    public void registerTransformer(final Transformer transformer) throws Exception
     {
+        // Load config for the transformer
         String location = getTransformConfigLocation(transformer);
         TransformConfig transformConfig = loadTransformConfig(location);
 
-        for (org.alfresco.transform.client.model.config.Transformer transformerConfig
+        // Map all of the transforms defined in the config to this Transformer implementation
-                : transformConfig.getTransformers())
+        for (org.alfresco.transform.client.model.config.Transformer transformerConfig : transformConfig.getTransformers())
         {
             String transformerName = transformerConfig.getTransformerName();
             if (transformerTransformMapping.containsKey(transformerName))
             {
                 throw new Exception("Transformer name " + transformerName + " is already registered.");
             }
-
             transformerTransformMapping.put(transformerName, transformer);
             log.debug("Registered transformer with name: '{}'.", transformerName);
         }
 
-        // add to data
+        // Add the new transformer configuration to the aggregate config
         aggregatedConfig.getTransformers().addAll(transformConfig.getTransformers());
         aggregatedConfig.getTransformOptions().putAll(transformConfig.getTransformOptions());
         registerAll(transformConfig, location, location);
@@ -109,28 +103,31 @@ public class AIOTransformRegistry extends AbstractTransformRegistry
 
     /**
      *
-     * @param transformName - the transform name used in mapping...
+     * @param transformName The transform name as it appears in TransformConfig.
-     * @return
+     * @return The transformer implementation mapped to the transform name.
      */
-    public Transformer getByTransformName(String transformName)
+    public Transformer getByTransformName(final String transformName)
     {
         return getTransformerTransformMapping().get(transformName);
     }
 
-    public TransformConfig getTransformConfig() throws Exception
+    /**
+     *
+     * @return The aggregated config of all the registered transformers
+     */
+    public TransformConfig getTransformConfig()
     {
         return aggregatedConfig;
     }
 
-    protected String getTransformConfigLocation(Transformer transformer)
+    protected String getTransformConfigLocation(final Transformer transformer)
     {
         String location = transformer.getTransformerId() + ENGINE_CONFIG_LOCATION_POSTFIX;
         return location;
     }
 
-    protected TransformConfig loadTransformConfig(String name) throws Exception
+    protected TransformConfig loadTransformConfig(final String name) throws Exception
     {
-
         if (getClass().getClassLoader().getResource(name) == null)
         {
             throw new Exception("Configuration '" + name + "' does not exist on the classpath.");

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/AllInOneTransformer.java

@@ -1,97 +0,0 @@
-/*
- * #%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 <http://www.gnu.org/licenses/>.
- * #L%
- */
-package org.alfresco.transformer.transformers;
-
-import org.alfresco.transformer.AIOTransformRegistry;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.File;
-import java.util.Map;
-
-/**
- * Top level transformer managing multiple sub transformers.
- *
- * @author eknizat
- */
-public class AllInOneTransformer implements Transformer
-{
-
-    private static final Logger logger = LoggerFactory.getLogger(AllInOneTransformer.class);
-
-    /**
-     * Represents the mapping between a transform and a transformer, multiple mappings can point to the same transformer.
-     */
-    AIOTransformRegistry transformRegistry = new AIOTransformRegistry();
-
-    public AllInOneTransformer()
-    {
-
-    }
-
-    public void addTransformer(Transformer transformer) throws Exception
-    {
-        transformRegistry.registerTransformer(transformer);
-    }
-
-    @Override
-    public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
-                          Map<String, String> transformOptions) throws Exception
-    {
-        String transformName = transformOptions.get(TRANSFORM_NAME_PARAMETER);
-        Transformer transformer = transformRegistry.getByTransformName(transformName);
-
-        if (transformer == null)
-        {
-            throw new Exception("No transformer mapping for : transform:" + transformName + " sourceMimetype:"
-                    + sourceMimetype + " targetMimetype:" + targetMimetype);
-        }
-
-        if (logger.isDebugEnabled())
-        {
-            logger.debug("Performing transform '{}' using {}", transformName, transformer.getClass().getSimpleName());
-        }
-        transformer.transform(sourceFile, targetFile, sourceMimetype, targetMimetype, transformOptions);
-    }
-
-    @Override
-    public String getTransformerId()
-    {
-        return "all-in-one";
-    }
-
-
-    public AIOTransformRegistry getTransformRegistry()
-    {
-        return transformRegistry;
-    }
-
-    public void setTransformRegistry(AIOTransformRegistry transformRegistry)
-    {
-        this.transformRegistry = transformRegistry;
-    }
-}

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/ImageMagickAdapter.java

@@ -59,15 +59,14 @@ public class ImageMagickAdapter implements Transformer
     private static String ID = "imagemagick";
     private ImageMagickCommandExecutor commandExecutor;
 
-    public ImageMagickAdapter() throws Exception 
+    public ImageMagickAdapter()
     {
-        super();
         commandExecutor = new ImageMagickCommandExecutor();
     }
 
     @Override
     public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
-            Map<String, String> transformOptions) throws Exception 
+            Map<String, String> transformOptions)
     {
         
         final String options = ImageMagickOptionsBuilder

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/LibreOfficeAdapter.java

@@ -36,15 +36,14 @@ public class LibreOfficeAdapter implements Transformer
     private static String ID = "libreoffice";
     private LibreOfficeJavaExecutor javaExecutor;
 
-    public LibreOfficeAdapter() throws Exception
+    public LibreOfficeAdapter()
     {
-        super();
         javaExecutor = new LibreOfficeJavaExecutor();
     }
     
     @Override
 	public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
-            Map<String, String> transformOptions) throws Exception 
+            Map<String, String> transformOptions)
     {
         javaExecutor.call(sourceFile, targetFile);
     }

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/MiscAdapter.java

@@ -34,16 +34,14 @@ public class MiscAdapter implements Transformer
     private static final String ID = "misc";
     private SelectingTransformer miscSelectingTransformer;
 
-
+    public MiscAdapter()
-    public MiscAdapter() throws Exception
     {
-        super();
         miscSelectingTransformer = new SelectingTransformer();
     }
 
     @Override
     public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype, Map<String,
-            String> transformOptions) throws Exception
+            String> transformOptions)
     {
         String transformerName = transformOptions.get(TRANSFORM_NAME_PARAMETER);
         miscSelectingTransformer.transform(transformerName, sourceFile, targetFile,

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/PdfRendererAdapter.java

@@ -46,15 +46,14 @@ public class PdfRendererAdapter implements Transformer
     private static String ID = "pdfrenderer";
     private PdfRendererCommandExecutor pdfExecutor;
 
-    public PdfRendererAdapter() throws Exception
+    public PdfRendererAdapter()
     {
-        super();
         pdfExecutor = new PdfRendererCommandExecutor();
     }
 
 	@Override
 	public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
-            Map<String, String> transformOptions) throws Exception 
+            Map<String, String> transformOptions)
     {
        
         final String options = PdfRendererOptionsBuilder

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/TikaAdapter.java

@@ -44,12 +44,12 @@ public class TikaAdapter implements Transformer
 
     public TikaAdapter() throws Exception
     {
-        super();
         tikaJavaExecutor = new TikaJavaExecutor();
     }
 
     @Override
-    public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype, Map<String, String> transformOptions) throws Exception
+    public void transform(File sourceFile, File targetFile, String sourceMimetype, String targetMimetype,
+                          Map<String, String> transformOptions) throws Exception
     {
         final String transform = transformOptions.get(TRANSFORM_NAME_PARAMETER);
 

alfresco-transform-core-aio/alfresco-transform-core-aio/src/main/java/org/alfresco/transformer/transformers/Transformer.java

@@ -26,21 +26,16 @@
  */
 package org.alfresco.transformer.transformers;
 
-
-import org.alfresco.transform.client.model.config.TransformConfig;
-
 import java.io.File;
 import java.util.Map;
 
 
 /**
- * Interface for transformers which can perform transformations and specify their own supported configuration.
+ * Interface for transformers used with {@link org.alfresco.transformer.AIOTransformRegistry}.
  */
 public interface Transformer
 {
-    /**
+    // Additional parameter used by transformers like {@link TikaAdapter} and {@link MiscAdapter}
-     * Controllers pass this as an additional parameter..
-     */
     String TRANSFORM_NAME_PARAMETER = "alfresco.transform-name-parameter";
 
     /**
@@ -54,13 +49,11 @@ public interface Transformer
     void transform(File sourceFile, File targetFile, String sourceMimetype,
                           String targetMimetype, Map<String, String> transformOptions) throws Exception;
 
-
     /**
-     * @return Supported config for the transformer implementation.
+     * @return A unique transformer id,
      *
      */
     String getTransformerId();
-
 }
 
 

alfresco-transform-core-aio/alfresco-transform-core-aio/src/test/java/org/alfresco/transformer/transformers/AIOTransformRegistryTest.java

@@ -58,15 +58,15 @@ public class AIOTransformRegistryTest
     String SOURCE_ENCODING = "sourceEncoding";
     String TARGET_ENCODING = "targetEncoding";
 
-    AIOTransformRegistry aioTransformer = new AIOTransformRegistry();
+    AIOTransformRegistry aioTransformerRegistry = new AIOTransformRegistry();
     ObjectMapper objectMapper = new ObjectMapper();
 
 
     @Before
     public void before() throws Exception
     {
-        aioTransformer.registerTransformer(new MiscAdapter());
+        aioTransformerRegistry.registerTransformer(new MiscAdapter());
-        aioTransformer.registerTransformer(new TikaAdapter());
+        aioTransformerRegistry.registerTransformer(new TikaAdapter());
 
     }
 
@@ -103,9 +103,9 @@ public class AIOTransformRegistryTest
         // check correct number of transformers
         assertEquals("Number of expected transformers",
                 miscConfig.getTransformers().size() + tikaConfig.getTransformers().size(),
-                aioTransformer.getTransformConfig().getTransformers().size());
+                aioTransformerRegistry.getTransformConfig().getTransformers().size());
 
-        List<String> actualTransformerNames = aioTransformer.getTransformConfig().getTransformers()
+        List<String> actualTransformerNames = aioTransformerRegistry.getTransformConfig().getTransformers()
                 .stream().map(t -> t.getTransformerName()).collect(Collectors.toList());
         // check all transformers are there
         for(String transformNames : expectedTransformNames)
@@ -116,9 +116,9 @@ public class AIOTransformRegistryTest
         // check correct number of options
         assertEquals("Number of expected transformers",
                 miscConfig.getTransformOptions().size() + tikaConfig.getTransformOptions().size(),
-                aioTransformer.getTransformConfig().getTransformOptions().size());
+                aioTransformerRegistry.getTransformConfig().getTransformOptions().size());
 
-        Set<String> actualOptionNames = aioTransformer.getTransformConfig().getTransformOptions().keySet();
+        Set<String> actualOptionNames = aioTransformerRegistry.getTransformConfig().getTransformOptions().keySet();
 
         // check all options are there
         for (String optionName : expectedTransformOptionNames)
@@ -127,7 +127,33 @@ public class AIOTransformRegistryTest
         }
     }
 
-    // Test copied from Misc (HtmlParserContentTransformerTest) See ATS-712 aioTransformer - html
+    @Test
+    public void testTransformerMapping()
+    {
+        List<String> tikaTransforms = Arrays.asList("Archive", "OutlookMsg", "PdfBox", "Office", "Poi", "OOXML", "TikaAuto", "TextMining");
+        List<String> miscTransforms = Arrays.asList("html", "string", "appleIWorks", "textToPdf", "rfc822");
+
+        for (String transform : tikaTransforms)
+        {
+            String actualId = aioTransformerRegistry.getByTransformName(transform).getTransformerId();
+            assertEquals("Wrong mapping for transform "+transform, "tika", actualId);
+        }
+
+        for (String transform : miscTransforms)
+        {
+            String actualId = aioTransformerRegistry.getByTransformName(transform).getTransformerId();
+            assertEquals("Wrong mapping for transform "+transform, "misc", actualId);
+        }
+    }
+
+    @Test(expected = Exception.class)
+    public void testDuplicateTransformsException() throws Exception
+    {
+        // The Misc transformers are already registered
+        aioTransformerRegistry.registerTransformer(new MiscAdapter());
+    }
+
+    // Test copied from Misc (HtmlParserContentTransformerTest) See ATS-712 aioTransformerRegistry - html
     @Test
     public void testMiscHtml() throws Exception
     {
@@ -157,7 +183,7 @@ public class AIOTransformRegistryTest
             Map<String, String> parameters = new HashMap<>();
             parameters.put(SOURCE_ENCODING, "ISO-8859-1");
             parameters.put(TRANSFORM_NAME_PARAMETER, "html");
-            Transformer transformer = aioTransformer.getByTransformName("html");
+            Transformer transformer = aioTransformerRegistry.getByTransformName("html");
             transformer.transform(tmpS, tmpD, SOURCE_MIMETYPE, TARGET_MIMETYPE, parameters);
 
             assertEquals(expected, readFromFile(tmpD, "UTF-8"));
@@ -227,7 +253,7 @@ public class AIOTransformRegistryTest
         }
     }
 
-    // Test copied from Misc (TextToPdfContentTransformerTest) See ATS-712 aioTransformer - pdf
+    // Test copied from Misc (TextToPdfContentTransformerTest) See ATS-712 aioTransformerRegistry - pdf
     @Test
     public void testMiscPdf() throws Exception
     {
@@ -268,7 +294,7 @@ public class AIOTransformRegistryTest
         Map<String, String> parameters = new HashMap<>();
         parameters.put(PAGE_LIMIT, pageLimit);
         parameters.put(TRANSFORM_NAME_PARAMETER, "textToPdf");
-        Transformer transformer = aioTransformer.getByTransformName("textToPdf");
+        Transformer transformer = aioTransformerRegistry.getByTransformName("textToPdf");
         transformer.transform(sourceFile, targetFile, "text/plain", "application/pdf", parameters);
 
         // Read back in the PDF and check it

alfresco-transformer-base/src/main/java/org/alfresco/transformer/AbstractTransformerController.java

@@ -115,7 +115,6 @@ public abstract class AbstractTransformerController implements TransformControll
     @GetMapping(value = "/transform/config")
     public ResponseEntity<TransformConfig> info()
     {
-        // TODO - This cast should not be here
         logger.info("GET Transform Config.");
         final TransformConfig transformConfig =
             ((TransformRegistryImpl) transformRegistry).getTransformConfig();