From 59cd841f87a24a325d2a29f0df6850f87a754615 Mon Sep 17 00:00:00 2001 From: Abel Salgado Romero Date: Thu, 25 Aug 2022 22:44:27 +0200 Subject: [PATCH] Replace reflection by direct JavaExtensionRegistry calls (#596) * Stops any possible usage of AsciidoctorJ v1.6.x (long not supported) * Fixes issue where registering a non-Processor class was ignored without throwing exception * Small improvements to JavaDocs closes #568 --- CHANGELOG.adoc | 4 + .../AsciidoctorJExtensionRegistry.java | 87 +++------- .../maven/extensions/ExtensionRegistry.java | 16 +- .../AsciidoctorJExtensionRegistryTest.java | 148 ++++++++++++++++++ 4 files changed, 181 insertions(+), 74 deletions(-) create mode 100644 asciidoctor-maven-plugin/src/test/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistryTest.java diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index a3e41153..481ed2e6 100644 --- a/CHANGELOG.adoc +++ b/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) diff --git a/asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistry.java b/asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistry.java index 4cd01c2a..c454a091 100644 --- a/asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistry.java +++ b/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 clazz; try { - clazz = (Class) 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) 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) clazz); } else if (Preprocessor.class.isAssignableFrom(clazz)) { - register(javaExtensionRegistry, "preprocessor", clazz); + javaExtensionRegistry.preprocessor((Class) clazz); } else if (Postprocessor.class.isAssignableFrom(clazz)) { - register(javaExtensionRegistry, "postprocessor", clazz); + javaExtensionRegistry.postprocessor((Class) clazz); } else if (Treeprocessor.class.isAssignableFrom(clazz)) { - register(javaExtensionRegistry, "treeprocessor", clazz); + javaExtensionRegistry.treeprocessor((Class) clazz); } else if (BlockProcessor.class.isAssignableFrom(clazz)) { if (blockName == null) { - register(javaExtensionRegistry, "block", clazz); + javaExtensionRegistry.block((Class) clazz); } else { - register(javaExtensionRegistry, "block", blockName, clazz); + javaExtensionRegistry.block(blockName, (Class) clazz); } } else if (IncludeProcessor.class.isAssignableFrom(clazz)) { - register(javaExtensionRegistry, "includeProcessor", clazz); + javaExtensionRegistry.includeProcessor((Class) clazz); } else if (BlockMacroProcessor.class.isAssignableFrom(clazz)) { if (blockName == null) { - register(javaExtensionRegistry, "blockMacro", clazz); + javaExtensionRegistry.blockMacro((Class) clazz); } else { - register(javaExtensionRegistry, "blockMacro", blockName, clazz); + javaExtensionRegistry.blockMacro(blockName, (Class) clazz); } } else if (InlineMacroProcessor.class.isAssignableFrom(clazz)) { if (blockName == null) { - register(javaExtensionRegistry, "inlineMacro", clazz); + javaExtensionRegistry.inlineMacro((Class) 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) clazz); } + } else { + throw new RuntimeException("'" + extensionClassName + "' is not a valid AsciidoctorJ processor class"); } - return true; } } diff --git a/asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/ExtensionRegistry.java b/asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/ExtensionRegistry.java index 1fe17ff1..9e5ce57b 100644 --- a/asciidoctor-maven-plugin/src/main/java/org/asciidoctor/maven/extensions/ExtensionRegistry.java +++ b/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); -} \ No newline at end of file +} diff --git a/asciidoctor-maven-plugin/src/test/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistryTest.java b/asciidoctor-maven-plugin/src/test/java/org/asciidoctor/maven/extensions/AsciidoctorJExtensionRegistryTest.java new file mode 100644 index 00000000..09d751cf --- /dev/null +++ b/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 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 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 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 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 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 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 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 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 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 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 clazz = ManpageInlineMacroProcessor.class; + final String className = clazz.getCanonicalName(); + + pluginExtensionRegistry.register(className, "block_name"); + Mockito.verify(javaExtensionRegistry).inlineMacro("block_name", clazz); + } +}