Replace reflection by direct JavaExtensionRegistry calls

CHANGELOG.adoc

@@ -17,6 +17,7 @@ Improvements::
 
   * Split plugin and site integration in sub-modules: asciidoctor-maven-plugin and asciidoctor-doxia-module (#595)
   * Add 'asciidoc' as valid file extension in AsciidoctorDoxiaParserModule (#595)
+  * Fix throwing an exception when registering a non Extension (#596)
 
 Build / Infrastructure::
 
@@ -25,6 +26,9 @@ Build / Infrastructure::
   * Upgrade Asciidoctorj to v2.5.4 and jRuby to v9.3.4.0 (#584)
   * Upgrade Asciidoctorj to v2.5.5 (#591)
 
+Maintenance::
+  * Replace use of reflection by direct JavaExtensionRegistry calls to register extensions (#596)
+
 Documentation::
 
   * Fix absolute path in usage example and AsciiDoc references in docs (https://github.com/MarkusTiede[@MarkusTiede])(#592)

asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistry.java

@@ -1,28 +1,13 @@
 package org.asciidoctor.maven.extensions;
 
-import org.apache.maven.plugin.MojoExecutionException;
-import org.apache.maven.plugin.MojoFailureException;
 import org.asciidoctor.Asciidoctor;
-import org.asciidoctor.extension.BlockMacroProcessor;
-import org.asciidoctor.extension.BlockProcessor;
-import org.asciidoctor.extension.DocinfoProcessor;
-import org.asciidoctor.extension.IncludeProcessor;
-import org.asciidoctor.extension.InlineMacroProcessor;
-import org.asciidoctor.extension.JavaExtensionRegistry;
-import org.asciidoctor.extension.Postprocessor;
-import org.asciidoctor.extension.Preprocessor;
-import org.asciidoctor.extension.Processor;
-import org.asciidoctor.extension.Treeprocessor;
-
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.util.Arrays;
+import org.asciidoctor.extension.*;
 
 /**
  * Class responsible for registering extensions.
- * 
+ *
  * @author abelsromero
- * */
+ */
 public class AsciidoctorJExtensionRegistry implements ExtensionRegistry {
 
     private JavaExtensionRegistry javaExtensionRegistry;
@@ -33,86 +18,52 @@ public AsciidoctorJExtensionRegistry(Asciidoctor asciidoctorInstance) {
 
     /*
      * (non-Javadoc)
-     * 
+     *
      * @see
      * org.asciidoctor.maven.processors.ProcessorRegistry#register(java.lang.String, java.lang.String)
      */
     @Override
     @SuppressWarnings("unchecked")
-    public void register(String extensionClassName, String blockName) throws MojoExecutionException {
+    public void register(String extensionClassName, String blockName) {
 
         Class<? extends Processor> clazz;
         try {
-            clazz = (Class<Processor>) Class.forName(extensionClassName);
-        } catch (ClassCastException cce) {
-            // Use RuntimeException to avoid catching, we only want the message in the Mojo
-            throw new RuntimeException("'" + extensionClassName + "' is not a valid AsciidoctorJ processor class");
+            clazz = (Class<? extends Processor>) Class.forName(extensionClassName);
         } catch (ClassNotFoundException e) {
             throw new RuntimeException("'" + extensionClassName + "' not found in classpath");
         }
 
-        // TODO: Replace with direct method calls again as soon as this project compiles against AsciidoctorJ 1.6.0
         if (DocinfoProcessor.class.isAssignableFrom(clazz)) {
-            register(javaExtensionRegistry, "docinfoProcessor", clazz);
+            javaExtensionRegistry.docinfoProcessor((Class<? extends DocinfoProcessor>) clazz);
         } else if (Preprocessor.class.isAssignableFrom(clazz)) {
-            register(javaExtensionRegistry, "preprocessor", clazz);
+            javaExtensionRegistry.preprocessor((Class<? extends Preprocessor>) clazz);
         } else if (Postprocessor.class.isAssignableFrom(clazz)) {
-            register(javaExtensionRegistry, "postprocessor", clazz);
+            javaExtensionRegistry.postprocessor((Class<? extends Postprocessor>) clazz);
         } else if (Treeprocessor.class.isAssignableFrom(clazz)) {
-            register(javaExtensionRegistry, "treeprocessor", clazz);
+            javaExtensionRegistry.treeprocessor((Class<? extends Treeprocessor>) clazz);
         } else if (BlockProcessor.class.isAssignableFrom(clazz)) {
             if (blockName == null) {
-                register(javaExtensionRegistry, "block", clazz);
+                javaExtensionRegistry.block((Class<? extends BlockProcessor>) clazz);
             } else {
-                register(javaExtensionRegistry, "block", blockName, clazz);
+                javaExtensionRegistry.block(blockName, (Class<? extends BlockProcessor>) clazz);
             }
         } else if (IncludeProcessor.class.isAssignableFrom(clazz)) {
-            register(javaExtensionRegistry, "includeProcessor", clazz);
+            javaExtensionRegistry.includeProcessor((Class<? extends IncludeProcessor>) clazz);
         } else if (BlockMacroProcessor.class.isAssignableFrom(clazz)) {
             if (blockName == null) {
-                register(javaExtensionRegistry, "blockMacro", clazz);
+                javaExtensionRegistry.blockMacro((Class<? extends BlockMacroProcessor>) clazz);
             } else {
-                register(javaExtensionRegistry, "blockMacro", blockName, clazz);
+                javaExtensionRegistry.blockMacro(blockName, (Class<? extends BlockMacroProcessor>) clazz);
             }
         } else if (InlineMacroProcessor.class.isAssignableFrom(clazz)) {
             if (blockName == null) {
-                register(javaExtensionRegistry, "inlineMacro", clazz);
+                javaExtensionRegistry.inlineMacro((Class<? extends InlineMacroProcessor>) clazz);
             } else {
-                register(javaExtensionRegistry, "inlineMacro", blockName, clazz);
-            }
-        }
-    }
-
-    private void register(Object target, String methodName, Object... args) throws MojoExecutionException {
-        for (Method method: javaExtensionRegistry.getClass().getMethods()) {
-
-            if (isMethodMatching(method, methodName, args)) {
-                try {
-                    method.invoke(target, args);
-                    return;
-                } catch (Exception e) {
-                    throw new MojoExecutionException("Unexpected exception while registering extensions", e);
-                }
-            }
-
-        }
-        throw new MojoExecutionException("Internal Error. Could not register " + methodName + " with arguments " + Arrays.asList(args));
-    }
-
-    private boolean isMethodMatching(Method method, String methodName, Object[] args) {
-        if (!method.getName().equals(methodName)) {
-            return false;
-        }
-        if (method.getParameterTypes().length != args.length) {
-            return false;
-        }
-        // Don't care for primitives here, there's no method on JavaExtensionRegistry with primitives.
-        for (int i = 0; i < method.getParameterTypes().length; i++) {
-            if (args[i] != null && !method.getParameterTypes()[i].isAssignableFrom(args[i].getClass())) {
-                return false;
+                javaExtensionRegistry.inlineMacro(blockName, (Class<? extends InlineMacroProcessor>) clazz);
             }
+        } else {
+            throw new RuntimeException("'" + extensionClassName + "' is not a valid AsciidoctorJ processor class");
         }
-        return true;
     }
 
 }

asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/ExtensionRegistry.java

@@ -1,18 +1,22 @@
 package org.asciidoctor.maven.extensions;
 
-import org.apache.maven.plugin.MojoExecutionException;
 import org.asciidoctor.extension.Processor;
 
+/**
+ * Base interface for registering AsciidoctorJ extension in the plugin.
+ *
+ * @author abelsromero
+ */
 public interface ExtensionRegistry {
 
     /**
-     * Checks if {@code extensionClassName} belongs to a valid {@link Processor}
-     * class and if it can be found in the classpath
+     * Registers an AsciidoctorJ extension by full class name.
      *
      * @param extensionClassName fully qualified name of the class implementing the extension
      * @param blockName          required when declaring
-     * @throws MojoExecutionException if extension could not be registered
+     * @throws RuntimeException if {@code extensionClassName} belongs to a valid {@link Processor},
+     *                          class, or if it can be found in the classpath
      */
-    void register(String extensionClassName, String blockName) throws MojoExecutionException;
+    void register(String extensionClassName, String blockName);
 
-}
+}

asciidoctor-maven-plugin/src/test/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistryTest.java

@@ -0,0 +1,148 @@
+package org.asciidoctor.maven.extensions;
+
+import org.asciidoctor.Asciidoctor;
+import org.asciidoctor.extension.*;
+import org.asciidoctor.maven.test.processors.*;
+import org.assertj.core.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mockito;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class AsciidoctorJExtensionRegistryTest {
+
+    private JavaExtensionRegistry javaExtensionRegistry;
+    private AsciidoctorJExtensionRegistry pluginExtensionRegistry;
+
+
+    @BeforeEach
+    void beforeEach() {
+        final Asciidoctor mockAsciidoctor = Mockito.mock(Asciidoctor.class);
+        javaExtensionRegistry = Mockito.mock(JavaExtensionRegistry.class);
+        Mockito.when(mockAsciidoctor.javaExtensionRegistry()).thenReturn(javaExtensionRegistry);
+        pluginExtensionRegistry = new AsciidoctorJExtensionRegistry(mockAsciidoctor);
+    }
+
+
+    @Test
+    void should_fail_when_not_an_extension() {
+        final String className = String.class.getCanonicalName();
+
+        Exception e = Assertions.catchException(() -> pluginExtensionRegistry.register(className, null));
+
+        assertThat(e)
+                .isInstanceOf(RuntimeException.class)
+                .hasMessage(String.format("'%s' is not a valid AsciidoctorJ processor class", className));
+    }
+
+    @Test
+    void should_fail_when_extension_class_is_not_available() {
+        final String className = "not.a.real.Class";
+
+        Exception e = Assertions.catchException(() -> pluginExtensionRegistry.register(className, null));
+
+        assertThat(e)
+                .isInstanceOf(RuntimeException.class)
+                .hasMessage(String.format("'%s' not found in classpath", className));
+    }
+
+    @Test
+    void should_register_a_DocinfoProcessor() {
+        final Class<? extends DocinfoProcessor> clazz = MetaDocinfoProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).docinfoProcessor(clazz);
+    }
+
+    @Test
+    void should_register_a_Preprocessor() {
+        final Class<? extends Preprocessor> clazz = ChangeAttributeValuePreprocessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).preprocessor(clazz);
+    }
+
+    @Test
+    void should_register_a_Postprocessor() {
+        final Class<? extends Postprocessor> clazz = DummyPostprocessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).postprocessor(clazz);
+    }
+
+    @Test
+    void should_register_a_Treeprocessor() {
+        final Class<? extends Treeprocessor> clazz = DummyTreeprocessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).treeprocessor(clazz);
+    }
+
+    @Test
+    void should_register_a_BlockProcessor() {
+        final Class<? extends BlockProcessor> clazz = YellBlockProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).block(clazz);
+    }
+
+    @Test
+    void should_register_a_BlockProcessor_with_name() {
+        final Class<? extends BlockProcessor> clazz = YellBlockProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, "block_name");
+        Mockito.verify(javaExtensionRegistry).block("block_name", clazz);
+    }
+
+    @Test
+    void should_register_a_IncludeProcessor() {
+        final Class<? extends IncludeProcessor> clazz = UriIncludeProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).includeProcessor(clazz);
+    }
+
+    @Test
+    void should_register_a_BlockMacroProcessor() {
+        final Class<? extends BlockMacroProcessor> clazz = GistBlockMacroProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).blockMacro(clazz);
+    }
+
+    @Test
+    void should_register_a_BlockMacroProcessor_with_name() {
+        final Class<? extends BlockMacroProcessor> clazz = GistBlockMacroProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, "block_name");
+        Mockito.verify(javaExtensionRegistry).blockMacro("block_name", clazz);
+    }
+
+    @Test
+    void should_register_a_InlineMacroProcessor() {
+        final Class<? extends InlineMacroProcessor> clazz = ManpageInlineMacroProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, null);
+        Mockito.verify(javaExtensionRegistry).inlineMacro(clazz);
+    }
+
+    @Test
+    void should_register_a_InlineMacroProcessor_with_name() {
+        final Class<? extends InlineMacroProcessor> clazz = ManpageInlineMacroProcessor.class;
+        final String className = clazz.getCanonicalName();
+
+        pluginExtensionRegistry.register(className, "block_name");
+        Mockito.verify(javaExtensionRegistry).inlineMacro("block_name", clazz);
+    }
+}