<fix>: remove ASTReference impl in CompiledVisitor

<fix>: remove all logger abstraction
<feat>: add testcases

Author: xiaoma20082008

velocity-engine-core/README.md

@@ -1,7 +1,7 @@
 ```
         +----------+                             |
         |          |  nowContext  +---------+    |
-        |          | -----------> | Context | <--+ run
+        |          | -----------> | Context | <--+ render
    init |          |              +---------+    |
   ----->| Velocity |                             v
         |          |  getTemplate  +------------------+

velocity-engine-core/pom.xml

@@ -295,6 +295,12 @@
     </build>
 
     <dependencies>
+        <dependency>
+            <groupId>org.javassist</groupId>
+            <artifactId>javassist</artifactId>
+            <version>3.28.0-GA</version>
+            <scope>provided</scope>
+        </dependency>
         <dependency>
             <groupId>org.apache.commons</groupId>
             <artifactId>commons-lang3</artifactId>

velocity-engine-core/src/main/java/org/apache/velocity/app/VelocityEngine.java

@@ -19,6 +19,10 @@
  * under the License.
  */
 
+import java.io.Reader;
+import java.io.Writer;
+import java.util.Objects;
+import java.util.Properties;
 import org.apache.velocity.Template;
 import org.apache.velocity.context.Context;
 import org.apache.velocity.exception.MethodInvocationException;
@@ -28,10 +32,6 @@
 import org.apache.velocity.runtime.RuntimeInstance;
 import org.slf4j.Logger;
 
-import java.io.Reader;
-import java.io.Writer;
-import java.util.Properties;
-
 /**
  * <p>
  * This class provides a separate new-able instance of the
@@ -51,14 +51,19 @@
  */
 public class VelocityEngine implements RuntimeConstants
 {
-    private RuntimeInstance ri = new RuntimeInstance();
+    private final RuntimeInstance ri;
 
     /**
      *  Init-less CTOR
      */
     public VelocityEngine()
     {
-        // do nothing
+        this(new RuntimeInstance());
+    }
+
+    public VelocityEngine(RuntimeInstance ri) // for testable
+    {
+        this.ri = Objects.requireNonNull(ri, "RuntimeInstance is null");
     }
 
     /**
@@ -68,6 +73,7 @@ public VelocityEngine()
      */
     public VelocityEngine(String propsFilename)
     {
+        this();
         ri.setProperties(propsFilename);
     }
 
@@ -77,6 +83,7 @@ public VelocityEngine(String propsFilename)
      */
     public VelocityEngine(Properties p)
     {
+        this();
         ri.setProperties(p);
     }
 

velocity-engine-core/src/main/java/org/apache/velocity/spi/Loader.java

@@ -20,18 +20,11 @@
  */
 
 import java.io.IOException;
-import java.util.List;
 import java.util.Locale;
 import org.apache.velocity.api.Resource;
 
 public interface Loader {
 
-  /**
-   * @param suffix resource suffix
-   * @return resource names.
-   */
-  List<String> list(String suffix) throws IOException;
-
   boolean exists(String name, Locale locale);
 
   Resource load(String name, Locale locale, String encoding) throws IOException;

velocity-engine-core/src/main/java/org/apache/velocity/spi/Logger.java

@@ -22,12 +22,17 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import org.apache.velocity.spi.Cache;
 
 public class MapCache<K, V> implements Cache<K, V> {
 
-  private ConcurrentMap<K, V> map;
+  private final ConcurrentMap<K, V> map;
+
+  public MapCache() {
+    this.map = new ConcurrentHashMap<>();
+  }
 
   @Override
   public int size() {
@@ -51,61 +56,61 @@ public boolean containsValue(Object value) {
 
   @Override
   public V get(Object key) {
-    return null;
+    return map.get(key);
   }
 
   @Override
   public V put(K key, V value) {
-    return null;
+    return map.put(key, value);
   }
 
   @Override
   public V remove(Object key) {
-    return null;
+    return map.remove(key);
   }
 
   @Override
   public void putAll(Map<? extends K, ? extends V> m) {
-
+    map.putAll(m);
   }
 
   @Override
   public void clear() {
-
+    map.clear();
   }
 
   @Override
   public Set<K> keySet() {
-    return null;
+    return map.keySet();
   }
 
   @Override
   public Collection<V> values() {
-    return null;
+    return map.values();
   }
 
   @Override
   public Set<Entry<K, V>> entrySet() {
-    return null;
+    return map.entrySet();
   }
 
   @Override
   public V putIfAbsent(K key, V value) {
-    return null;
+    return map.putIfAbsent(key, value);
   }
 
   @Override
   public boolean remove(Object key, Object value) {
-    return false;
+    return map.remove(key, value);
   }
 
   @Override
   public boolean replace(K key, V oldValue, V newValue) {
-    return false;
+    return map.replace(key, oldValue, newValue);
   }
 
   @Override
   public V replace(K key, V value) {
-    return null;
+    return map.replace(key, value);
   }
 }

velocity-engine-core/src/main/java/org/apache/velocity/spi/caches/MapCache.java

@@ -29,14 +29,15 @@
 import org.apache.velocity.exception.VelocityException;
 import org.apache.velocity.spi.Cache;
 import org.apache.velocity.spi.Compiler;
-import org.apache.velocity.spi.Logger;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public abstract class BaseCompiler implements Compiler {
 
   private static final Pattern PACKAGE_PATTERN = Pattern.compile("package\\s+([_a-zA-Z][_a-zA-Z0-9\\.]*);");
-  private static final Pattern CLASS_PATTERN = Pattern.compile("class\\s+([_a-zA-Z][_a-zA-Z0-9]*)\\s+");
+  private static final Pattern CLASS_PATTERN = Pattern.compile("public\\s+class\\s+([_a-zA-Z][_a-zA-Z0-9]*)\\s+");
 
-  protected Logger logger;
+  protected final Logger logger = LoggerFactory.getLogger(getClass());
   protected Cache<String, Object> cache;
   protected File compileDirectory;
   private volatile boolean first = true;
@@ -80,10 +81,6 @@ public Class<?> compile(String code) throws VelocityException {
 
   protected abstract Class<?> doCompile(String name, String source) throws Exception;
 
-  public void setLogger(Logger logger) {
-    this.logger = logger;
-  }
-
   public void setCache(Cache<String, Object> cache) {
     this.cache = cache;
   }
@@ -102,7 +99,7 @@ protected void saveBytecode(String name, byte[] bytecode) {
         if (first) {
           first = false;
           if (logger != null && logger.isInfoEnabled()) {
-            logger.info("Compile httl template classes to directory " + compileDirectory.getAbsolutePath());
+            logger.info("Compile template classes to directory " + compileDirectory.getAbsolutePath());
           }
         }
       } catch (IOException e) {

velocity-engine-core/src/main/java/org/apache/velocity/spi/compilers/BaseCompiler.java

@@ -65,7 +65,7 @@ public JavacCompiler() {
     compiler = ToolProvider.getSystemJavaCompiler();
     if (compiler == null) {
       throw new IllegalStateException(
-          "Can not get system java compiler. Please run with JDK (NOT JVM), or configure the httl.properties: compiler=httl.spi.compilers.JavassistCompiler, and add javassist.jar.");
+          "Can not get system java compiler.");
     }
     diagnosticCollector = new DiagnosticCollector<JavaFileObject>();
     standardJavaFileManager = compiler.getStandardFileManager(diagnosticCollector, null, null);

velocity-engine-core/src/main/java/org/apache/velocity/spi/compilers/JavacCompiler.java

@@ -19,10 +19,12 @@
  * under the License.
  */
 
+import javassist.ClassPool;
+
 public class JavassistCompiler extends BaseCompiler {
 
   @Override
   protected Class<?> doCompile(String name, String source) throws Exception {
-    return null;
+    return ClassPool.getDefault().makeClass(source).toClass();
   }
 }