Improving template automatic resolution exception handling #304

butterfly-api/src/main/java/com/paypal/butterfly/api/ButterflyFacade.java

@@ -7,6 +7,7 @@
 
 import java.io.File;
 import java.util.List;
+import java.util.Optional;
 import java.util.Properties;
 import java.util.concurrent.CompletableFuture;
 
@@ -37,17 +38,24 @@ public interface ButterflyFacade {
      * transformation template resolution is performed by each registered
      * Extension class. Based on the application folder, and its content, each
      * registered extension might decide which transformation template should be used
-     * to transform it. Only one can be chosen. If no template applies,
-     * if more than one applies (from different extensions), or if no
-     * extension has been registered, a {@link TemplateResolutionException} is
-     * thrown explaining the reason why no template could be chosen.
+     * to transform it. Only one can be chosen. These are the possible resolution results:
+     * <ol>
+     *     <li>Empty optional is returned: if no transformation template is resolved (unless if extension(s) evaluated the application as invalid,
+     *     then an {@link TemplateResolutionException} is thrown, as explained below in details)</li>
+     *     <li>An optional with {@link TransformationTemplate} class is returned: if only one transformation template is resolved</li>
+     *     <li>A {@link TemplateResolutionException} exception is thrown: if one of the following cases is true (call {@link TemplateResolutionException#getMessage()} for details):
+     *     <ol>
+     *         <li>If there are no extensions registered</li>
+     *         <li>Multiple transformation templates are resolved</li>
+     *         <li>If no transformation template is resolved, but one or more extensions recognized the application type, but determined it to be invalid for transformation</li>
+     *     </ol>
+     * </ol>
      *
      * @param applicationFolder the folder where the code of the application to be transformed is
-     * @return the chosen transformation template
-     * @throws TemplateResolutionException if no template could be resolved, more than one was resolved
-     * (from different extensions), or if no extension has been registered
+     * @return see above
+     * @throws TemplateResolutionException see above
      */
-    Class<? extends TransformationTemplate> automaticResolution(File applicationFolder) throws TemplateResolutionException;
+    Optional<Class<? extends TransformationTemplate>> automaticResolution(File applicationFolder) throws TemplateResolutionException;
 
     /**
      * Creates and returns a new {@link Configuration} object

butterfly-cli/src/main/java/com/paypal/butterfly/cli/ButterflyCliRunner.java

@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.io.StringReader;
 import java.util.List;
+import java.util.Optional;
 import java.util.Properties;
 
 /**
@@ -132,11 +133,12 @@ ButterflyCliRun run(File butterflyHome, String butterflyBanner) {
             logger.info("Transformation template associated with shortcut {}: {}", shortcut, templateClass.getName());
         } else {
             try {
-                templateClass = butterflyFacade.automaticResolution(applicationFolder);
-                if (templateClass == null) {
+                Optional<Class<? extends TransformationTemplate>> resolution = butterflyFacade.automaticResolution(applicationFolder);
+                if (!resolution.isPresent()) {
                     registerError(run, "No transformation template could be resolved for this application. Specify it explicitly using option -t or -s.");
                     return run;
                 }
+                templateClass = resolution.get();
                 logger.info("Transformation template automatically resolved");
             } catch (TemplateResolutionException e) {
                 registerError(run, e.getMessage());

butterfly-cli/src/test/java/com/test/SampleExtension1.java

@@ -4,6 +4,7 @@
 import com.paypal.butterfly.extensions.api.TransformationTemplate;
 
 import java.io.File;
+import java.util.Optional;
 
 /**
  * @author facarvalho
@@ -26,8 +27,8 @@ public String getVersion() {
     }
 
     @Override
-    public Class<? extends TransformationTemplate> automaticResolution(File applicationFolder) {
-        return SampleTemplate.class;
+    public Optional<Class<? extends TransformationTemplate>> automaticResolution(File applicationFolder) {
+        return Optional.of(SampleTemplate.class);
     }
 
 }

butterfly-cli/src/test/java/com/test/SampleExtension2.java

@@ -4,6 +4,7 @@
 import com.paypal.butterfly.extensions.api.TransformationTemplate;
 
 import java.io.File;
+import java.util.Optional;
 
 /**
  * @author facarvalho
@@ -25,8 +26,8 @@ public String getVersion() {
     }
 
     @Override
-    public Class<? extends TransformationTemplate> automaticResolution(File applicationFolder) {
-        return null;
+    public Optional<Class<? extends TransformationTemplate>> automaticResolution(File applicationFolder) {
+        return Optional.empty();
     }
 
 }

butterfly-core/src/main/java/com/paypal/butterfly/core/ButterflyFacadeImpl.java

@@ -11,10 +11,7 @@
 import org.springframework.stereotype.Component;
 
 import java.io.File;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Properties;
-import java.util.Set;
+import java.util.*;
 import java.util.concurrent.CompletableFuture;
 
 /**
@@ -49,29 +46,38 @@ public List<Extension> getExtensions() {
     }
 
     @Override
-    public Class<? extends TransformationTemplate> automaticResolution(File applicationFolder) throws TemplateResolutionException {
+    public Optional<Class<? extends TransformationTemplate>> automaticResolution(File applicationFolder) throws TemplateResolutionException {
         if (extensionRegistry.getExtensions().isEmpty()) {
             throw new TemplateResolutionException("No Butterfly extension has been registered");
         }
 
         Set<Class<? extends TransformationTemplate>> resolvedTemplates = new HashSet<>();
-        Class<? extends TransformationTemplate> t;
+        Optional<Class<? extends TransformationTemplate>> t;
+        Map<String, TemplateResolutionException> extensionsResolutionExceptions = new HashMap();
 
         for (Extension extension : extensionRegistry.getExtensions()) {
-            t = extension.automaticResolution(applicationFolder);
-            if (t != null) {
-                resolvedTemplates.add(t);
+            try {
+                t = extension.automaticResolution(applicationFolder);
+                if (t.isPresent()) {
+                    resolvedTemplates.add(t.get());
+                }
+            } catch (TemplateResolutionException e) {
+                extensionsResolutionExceptions.put(extension.getClass().getName(), e);
             }
         }
 
-        if (resolvedTemplates.size() == 0) {
-            throw new TemplateResolutionException("No transformation template could be resolved");
-        }
         if (resolvedTemplates.size() == 1) {
-            return (Class<? extends TransformationTemplate>) resolvedTemplates.toArray()[0];
+            return Optional.of((Class<? extends TransformationTemplate>) resolvedTemplates.toArray()[0]);
         }
-
-        throw new TemplateResolutionException("More than one transformation template was resolved, they are: " + resolvedTemplates);
+        if (resolvedTemplates.size() > 1) {
+            throw new TemplateResolutionException("More than one transformation template was resolved, they are: " + resolvedTemplates);
+        }
+        if (extensionsResolutionExceptions.size() == 1) {
+            throw (TemplateResolutionException) extensionsResolutionExceptions.values().toArray()[0];
+        } else if (extensionsResolutionExceptions.size() > 1) {
+            throw new TemplateResolutionException("No transformation template could be resolved. However, more than one extension recognized the application type, but considered them invalid. See the following map, whose key is an extension class, and value is the reason why application failed validation: " + extensionsResolutionExceptions);
+        }
+        return Optional.empty();
     }
 
     @Override

butterfly-extensions-api/src/main/java/com/paypal/butterfly/extensions/api/Extension.java

@@ -13,6 +13,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 /**
  * A Butterfly third-party extension. It provides custom
@@ -67,14 +68,24 @@ public final List<Class<? extends TransformationTemplate>> getTemplateClasses()
      * transformation template resolution is actually performed by each registered
      * Extension class. Based on the application folder, and its content, each
      * registered extension might decide which transformation template should be used
-     * to transform it. Only one or none can be chosen. If no one applies, null is
-     * returned.
+     * to transform it. These are the possible resolution results:
+     * <ol>
+     *     <li>Empty optional is returned: if application type is not recognized as a known and supported type by the extension</li>
+     *     <li>An optional with {@link TransformationTemplate} class is returned: if application type is recognized and application is valid</li>
+     *     <li>A {@link TemplateResolutionException} exception is thrown: if the application type is recognized as a known and supported type
+     *     (based on most of its folders and files structure and content), however, the extension identifies it as invalid for specific reasons,
+     *     (for missing a required file for example, having an invalid property version, etc). Call {@link TemplateResolutionException#getMessage()} for details.
+     * </ol>
+     * <br>
+     * Notice the difference between "not recognized" and "invalid" can be vague and arbitrary.
+     * It is entirely up to the extension to define its own criteria and communicate it with the application owners.
      *
      * @param applicationFolder the folder where the code of the application to be transformed is
-     * @return the chosen transformation template class, or null, if no one applies
+     * @return see above
+     * @throws TemplateResolutionException see above
      */
-    public  Class<? extends TransformationTemplate> automaticResolution(File applicationFolder) {
-        return null;
+    public Optional<Class<? extends TransformationTemplate>> automaticResolution(File applicationFolder) throws TemplateResolutionException {
+        return Optional.empty();
     }
 
     /**

butterfly-extensions-api/src/test/java/com/paypal/butterfly/extensions/api/ExtensionTest.java

@@ -9,6 +9,7 @@
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.util.Optional;
 
 import static org.testng.Assert.*;
 
@@ -48,7 +49,7 @@ public void templatesTest() {
     }
 
     @Test
-    public void automaticResolutionTest() throws URISyntaxException {
+    public void automaticResolutionTest() throws URISyntaxException, TemplateResolutionException {
         File appFolder = new File(getClass().getResource("/test-app").toURI());
 
         Extension extension = new Extension() {
@@ -58,19 +59,19 @@ public void automaticResolutionTest() throws URISyntaxException {
             public String getVersion() {return null;}
 
             @Override
-            public Class<? extends TransformationTemplate> automaticResolution(File applicationFolder) {
-                return (Class<? extends TransformationTemplate>) getTemplateClasses().get(0);
+            public Optional<Class<? extends TransformationTemplate>> automaticResolution(File applicationFolder) {
+                return Optional.of((Class<? extends TransformationTemplate>) getTemplateClasses().get(0));
             }
         };
         extension.add(SampleTransformationTemplate2.class);
 
-        assertEquals(extension.automaticResolution(appFolder), SampleTransformationTemplate2.class);
+        assertEquals(extension.automaticResolution(appFolder).get(), SampleTransformationTemplate2.class);
     }
 
     @Test
-    public void notSupportedAutomaticResolutionTest() throws URISyntaxException {
+    public void notSupportedAutomaticResolutionTest() throws URISyntaxException, TemplateResolutionException {
         File appFolder = new File(getClass().getResource("/test-app").toURI());
-        assertNull(new SampleExtension().automaticResolution(appFolder));
+        assertFalse(new SampleExtension().automaticResolution(appFolder).isPresent());
     }
 
     @Test

extensions-catalog/butterfly-springboot-extension/src/main/java/com/paypal/butterfly/extensions/springboot/ButterflySpringBootExtension.java

@@ -1,6 +1,7 @@
 package com.paypal.butterfly.extensions.springboot;
 
 import java.io.File;
+import java.util.Optional;
 
 import com.paypal.butterfly.extensions.api.Extension;
 import com.paypal.butterfly.extensions.api.TransformationTemplate;
@@ -28,9 +29,9 @@ public String getVersion() {
     }
 
     @Override
-    public Class<? extends TransformationTemplate> automaticResolution(File file) {
+    public Optional<Class<? extends TransformationTemplate>> automaticResolution(File file) {
         // TODO
-        return null;
+        return Optional.empty();
     }
 
 }

extensions-catalog/butterfly-springboot-extension/src/test/java/com/paypal/butterfly/extensions/springboot/ButterflySpringBootExtensionTest.java

@@ -17,7 +17,7 @@ public void test() {
 
         assertEquals(butterflySpringBootExtension.getDescription(), "Butterfly Spring Boot extension");
         assertEquals(butterflySpringBootExtension.getVersion(), "1.0.0");
-        assertNull(butterflySpringBootExtension.automaticResolution(null));
+        assertFalse(butterflySpringBootExtension.automaticResolution(null).isPresent());
         assertNotNull(butterflySpringBootExtension.getTemplateClasses());
         assertEquals(butterflySpringBootExtension.getTemplateClasses().size(), 2);
         assertTrue(butterflySpringBootExtension.getTemplateClasses().contains(JavaEEToSpringBoot.class));