You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by sg...@apache.org on 2020/02/05 09:50:22 UTC

[freemarker-generator] branch FREEMARKER-129 updated: FREEMARKER-129 Refactor source code

This is an automated email from the ASF dual-hosted git repository.

sgoeschl pushed a commit to branch FREEMARKER-129
in repository https://gitbox.apache.org/repos/asf/freemarker-generator.git


The following commit(s) were added to refs/heads/FREEMARKER-129 by this push:
     new bb365d1  FREEMARKER-129 Refactor source code
bb365d1 is described below

commit bb365d10135161e2c614dbe1a790e53d2afced2b
Author: Siegfried Goeschl <si...@gmail.com>
AuthorDate: Wed Feb 5 10:50:08 2020 +0100

    FREEMARKER-129 Refactor source code
---
 .../generator/base/tools/ToolsFactory.java         |  61 +++++------
 .../generator/base}/util/LocaleUtils.java          |   2 +-
 .../base}/util/NonClosableWriterWrapper.java       |   6 +-
 .../generator/document/DocumentsSupplierTest.java  |   6 ++
 .../generator/tools/ToolsFactoryTest.java          | 115 +++++++++++++++++++++
 .../cli/impl/TemplateDirectorySupplier.java        |   2 +-
 .../generator/cli/impl/ToolsSupplier.java          |  47 +--------
 .../freemarker/generator/cli/model/Settings.java   |   4 +-
 8 files changed, 159 insertions(+), 84 deletions(-)

diff --git a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java b/freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/tools/ToolsFactory.java
similarity index 61%
copy from freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java
copy to freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/tools/ToolsFactory.java
index e6752ac..29aa7e8 100644
--- a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java
+++ b/freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/tools/ToolsFactory.java
@@ -14,49 +14,24 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.freemarker.generator.cli.impl;
-
-import freemarker.template.utility.ClassUtil;
+package org.apache.freemarker.generator.base.tools;
 
 import java.lang.reflect.Constructor;
 import java.util.Map;
-import java.util.Properties;
-import java.util.function.Supplier;
 
 import static java.util.Arrays.stream;
-import static java.util.Objects.requireNonNull;
-import static java.util.stream.Collectors.toMap;
-
-public class ToolsSupplier implements Supplier<Map<String, Object>> {
 
-    private static final String FREEMARKER_TOOLS_PREFIX = "freemarker.tools.";
-
-    private final Properties configuration;
-    private final Map<String, Object> settings;
+public class ToolsFactory {
 
     /**
-     * Constructor.
+     * Checks if the given class can be loaded from the class loader.
      *
-     * @param configuration Containing "freemarker.tools.XXX=class"
-     * @param settings      Passed to the instantiated tools
+     * @param clazzName Class to instantiate
+     * @return true if loaded
      */
-    public ToolsSupplier(Properties configuration, Map<String, Object> settings) {
-        this.configuration = requireNonNull(configuration);
-        this.settings = requireNonNull(settings);
-    }
-
-    @Override
-    public Map<String, Object> get() {
-        return configuration.stringPropertyNames().stream()
-                .filter(name -> name.startsWith(FREEMARKER_TOOLS_PREFIX))
-                .filter(name -> toolClassCanBeLoaded(configuration.getProperty(name)))
-                .collect(toMap(ToolsSupplier::toolName, name -> createTool(configuration.getProperty(name), settings)));
-    }
-
-    private boolean toolClassCanBeLoaded(String clazzName) {
+    public static boolean exists(String clazzName) {
         try {
-            ClassUtil.forName(clazzName);
-            return true;
+            return forName(clazzName) != null;
         } catch (NoClassDefFoundError | ClassNotFoundException e) {
             return false;
         }
@@ -70,7 +45,7 @@ public class ToolsSupplier implements Supplier<Map<String, Object>> {
      * @param settings  Settings used to configure the tool
      * @return Tool instance
      */
-    private static Object createTool(String clazzName, Map<String, Object> settings) {
+    public static Object create(String clazzName, Map<String, Object> settings) {
         try {
             final Class<?> clazz = Class.forName(clazzName);
             final Constructor<?>[] constructors = clazz.getConstructors();
@@ -96,7 +71,23 @@ public class ToolsSupplier implements Supplier<Map<String, Object>> {
                 .orElse(null);
     }
 
-    private static String toolName(String propertyName) {
-        return propertyName.substring(FREEMARKER_TOOLS_PREFIX.length());
+    /**
+     * Similar to {@link Class#forName(java.lang.String)}, but attempts to load
+     * through the thread context class loader. Only if thread context class
+     * loader is inaccessible, or it can't find the class will it attempt to
+     * fall back to the class loader that loads the FreeMarker classes.
+     */
+    private static Class<?> forName(String className) throws ClassNotFoundException {
+        try {
+            final ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+            if (contextClassLoader != null) {  // not null: we don't want to fall back to the bootstrap class loader
+                return Class.forName(className, true, contextClassLoader);
+            }
+        } catch (ClassNotFoundException | SecurityException e) {
+            ;// Intentionally ignored
+        }
+
+        // Fall back to the defining class loader of the FreeMarker classes
+        return Class.forName(className);
     }
 }
diff --git a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/util/LocaleUtils.java b/freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/util/LocaleUtils.java
similarity index 95%
rename from freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/util/LocaleUtils.java
rename to freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/util/LocaleUtils.java
index ff00548..e7a3fc2 100644
--- a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/util/LocaleUtils.java
+++ b/freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/util/LocaleUtils.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.freemarker.generator.cli.util;
+package org.apache.freemarker.generator.base.util;
 
 import org.apache.freemarker.generator.base.util.StringUtils;
 
diff --git a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/util/NonClosableWriterWrapper.java b/freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/util/NonClosableWriterWrapper.java
similarity index 95%
rename from freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/util/NonClosableWriterWrapper.java
rename to freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/util/NonClosableWriterWrapper.java
index 6e3f5ec..e2637b7 100644
--- a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/util/NonClosableWriterWrapper.java
+++ b/freemarker-generator-base/src/main/java/org/apache/freemarker/generator/base/util/NonClosableWriterWrapper.java
@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.freemarker.generator.cli.util;
+package org.apache.freemarker.generator.base.util;
 
 import java.io.IOException;
 import java.io.Writer;
@@ -89,4 +89,8 @@ public class NonClosableWriterWrapper extends Writer {
     public String toString() {
         return delegate.toString();
     }
+
+    public Writer getDelegate() {
+        return delegate;
+    }
 }
diff --git a/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/document/DocumentsSupplierTest.java b/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/document/DocumentsSupplierTest.java
index b052e0d..16385b2 100644
--- a/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/document/DocumentsSupplierTest.java
+++ b/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/document/DocumentsSupplierTest.java
@@ -84,6 +84,12 @@ public class DocumentsSupplierTest {
         assertEquals(0, supplier("/does-not-exist", MATCH_ALL).get().size());
     }
 
+    @Test
+    public void shouldAllowDuplicateFiles() {
+        final List<String> sources = Arrays.asList("pom.xml", "pom.xml");
+        assertEquals(2, supplier(sources, "*.xml").get().size());
+    }
+
     private static DocumentsSupplier supplier(String directory, String include) {
         return new DocumentsSupplier(singletonList(directory), include, Charset.defaultCharset());
     }
diff --git a/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/tools/ToolsFactoryTest.java b/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/tools/ToolsFactoryTest.java
new file mode 100644
index 0000000..91d04a6
--- /dev/null
+++ b/freemarker-generator-base/src/test/java/org/apache/freemarker/generator/tools/ToolsFactoryTest.java
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.freemarker.generator.tools;
+
+import org.apache.freemarker.generator.base.tools.ToolsFactory;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+public class ToolsFactoryTest {
+
+    private final Map<String, Object> settings = new HashMap<>();
+
+    @Test
+    public void shouldCreateToolWithDefaultConstructor() {
+        final Object tool = ToolsFactory.create(ToolWithDefaultConstructor.class.getName(), settings);
+
+        assertNotNull(tool);
+        assertEquals("ToolWithDefaultConstructor", tool.toString());
+    }
+
+    @Test
+    public void shouldCreateToolWithMapConstructor() {
+        final Object tool = ToolsFactory.create(ToolWithMapConstructor.class.getName(), settings);
+
+        assertNotNull(tool);
+        assertEquals("ToolWithMapConstructor", tool.toString());
+        assertNotNull(((ToolWithMapConstructor) tool).getSettings());
+    }
+
+    @Test
+    public void shouldFavourMapOverDefaultConstructorOver() {
+        final Object tool = ToolsFactory.create(ToolWithDefaultAndMapConstructor.class.getName(), settings);
+
+        assertNotNull(tool);
+        assertEquals("ToolWithDefaultAndMapConstructor", tool.toString());
+        assertNotNull(((ToolWithDefaultAndMapConstructor) tool).getSettings());
+    }
+
+    @Test(expected = RuntimeException.class)
+    public void shouldThrowRuntimeExceptionForUnknownClass() {
+        ToolsFactory.create("does-not-exist", settings);
+    }
+
+
+    public static final class ToolWithDefaultConstructor {
+
+        public ToolWithDefaultConstructor() {
+        }
+
+        @Override
+        public String toString() {
+            return "ToolWithDefaultConstructor";
+        }
+    }
+
+    public static final class ToolWithMapConstructor {
+
+        private final Map<String, Object> settings;
+
+        public ToolWithMapConstructor(Map<String, Object> settings) {
+            this.settings = settings;
+        }
+
+        Map<String, Object> getSettings() {
+            return settings;
+        }
+
+        @Override
+        public String toString() {
+            return "ToolWithMapConstructor";
+        }
+    }
+
+    public static final class ToolWithDefaultAndMapConstructor {
+
+        private final Map<String, Object> settings;
+
+        public ToolWithDefaultAndMapConstructor() {
+            this.settings = null;
+        }
+
+        public ToolWithDefaultAndMapConstructor(Map<String, Object> settings) {
+            this.settings = settings;
+        }
+
+        Map<String, Object> getSettings() {
+            return settings;
+        }
+
+        @Override
+        public String toString() {
+            return "ToolWithDefaultAndMapConstructor";
+        }
+    }
+
+}
diff --git a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/TemplateDirectorySupplier.java b/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/TemplateDirectorySupplier.java
index e029226..7a580e1 100644
--- a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/TemplateDirectorySupplier.java
+++ b/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/TemplateDirectorySupplier.java
@@ -37,7 +37,7 @@ public class TemplateDirectorySupplier implements Supplier<List<File>> {
 
     private static final String UNDEFINED = "__undefined__";
 
-    /** Installation directory of "freemarkr-cli" */
+    /** Installation directory of "freemarker-cli" */
     private static final String APP_HOME = "app.home";
 
     /** Current working directory */
diff --git a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java b/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java
index e6752ac..d76e5ef 100644
--- a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java
+++ b/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/impl/ToolsSupplier.java
@@ -16,14 +16,12 @@
  */
 package org.apache.freemarker.generator.cli.impl;
 
-import freemarker.template.utility.ClassUtil;
+import org.apache.freemarker.generator.base.tools.ToolsFactory;
 
-import java.lang.reflect.Constructor;
 import java.util.Map;
 import java.util.Properties;
 import java.util.function.Supplier;
 
-import static java.util.Arrays.stream;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toMap;
 
@@ -50,50 +48,11 @@ public class ToolsSupplier implements Supplier<Map<String, Object>> {
         return configuration.stringPropertyNames().stream()
                 .filter(name -> name.startsWith(FREEMARKER_TOOLS_PREFIX))
                 .filter(name -> toolClassCanBeLoaded(configuration.getProperty(name)))
-                .collect(toMap(ToolsSupplier::toolName, name -> createTool(configuration.getProperty(name), settings)));
+                .collect(toMap(ToolsSupplier::toolName, name -> ToolsFactory.create(configuration.getProperty(name), settings)));
     }
 
     private boolean toolClassCanBeLoaded(String clazzName) {
-        try {
-            ClassUtil.forName(clazzName);
-            return true;
-        } catch (NoClassDefFoundError | ClassNotFoundException e) {
-            return false;
-        }
-    }
-
-    /**
-     * Create a tool instance either using single argument constructor taking a map or
-     * the default constructor.
-     *
-     * @param clazzName Class to instantiate
-     * @param settings  Settings used to configure the tool
-     * @return Tool instance
-     */
-    private static Object createTool(String clazzName, Map<String, Object> settings) {
-        try {
-            final Class<?> clazz = Class.forName(clazzName);
-            final Constructor<?>[] constructors = clazz.getConstructors();
-            final Constructor<?> constructorWithSettings = findSingleParameterConstructor(constructors, Map.class);
-            final Constructor<?> defaultConstructor = findDefaultConstructor(constructors);
-            return constructorWithSettings != null ? constructorWithSettings.newInstance(settings) : defaultConstructor.newInstance();
-        } catch (Exception e) {
-            throw new RuntimeException("Failed to create tool: " + clazzName, e);
-        }
-    }
-
-    private static Constructor<?> findSingleParameterConstructor(Constructor<?>[] constructors, Class<?> parameterClazz) {
-        return stream(constructors)
-                .filter(c -> c.getParameterCount() == 1 && c.getParameterTypes()[0].equals(parameterClazz))
-                .findFirst()
-                .orElse(null);
-    }
-
-    private static Constructor<?> findDefaultConstructor(Constructor<?>[] constructors) {
-        return stream(constructors)
-                .filter(c -> c.getParameterCount() == 0)
-                .findFirst()
-                .orElse(null);
+        return ToolsFactory.exists(clazzName);
     }
 
     private static String toolName(String propertyName) {
diff --git a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/model/Settings.java b/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/model/Settings.java
index 9a6a421..84b99be 100644
--- a/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/model/Settings.java
+++ b/freemarker-generator-cli/src/main/java/org/apache/freemarker/generator/cli/model/Settings.java
@@ -16,8 +16,8 @@
  */
 package org.apache.freemarker.generator.cli.model;
 
-import org.apache.freemarker.generator.cli.util.LocaleUtils;
-import org.apache.freemarker.generator.cli.util.NonClosableWriterWrapper;
+import org.apache.freemarker.generator.base.util.LocaleUtils;
+import org.apache.freemarker.generator.base.util.NonClosableWriterWrapper;
 
 import java.io.File;
 import java.io.Writer;