velocity-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nbu...@apache.org
Subject svn commit: r539268 - in /velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools: ToolInfo.java config/ToolConfiguration.java
Date Fri, 18 May 2007 05:47:24 GMT
Author: nbubna
Date: Thu May 17 22:47:23 2007
New Revision: 539268

URL: http://svn.apache.org/viewvc?view=rev&rev=539268
Log:
refactor and enhance tool validation to catch more problems earlier and with better error
reporting

Modified:
    velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/ToolInfo.java
    velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/config/ToolConfiguration.java

Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/ToolInfo.java
URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/ToolInfo.java?view=diff&rev=539268&r1=539267&r2=539268
==============================================================================
--- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/ToolInfo.java (original)
+++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/ToolInfo.java Thu
May 17 22:47:23 2007
@@ -76,19 +76,17 @@
      */
     public void setClass(Class clazz)
     {
-        try
+        if (clazz == null)
         {
-            // make sure we can get an instance
-            // make sure we can get an instance
-            clazz.newInstance();
-
-            // ok, we'll accept this one
-            this.clazz = clazz;
-        }
-        catch (Throwable t)
-        {
-            throw new IllegalArgumentException("Could not create an instance of "+clazz,
t);
+            throw new NullPointerException("Tool class must not be null");
         }
+        this.clazz = clazz;
+
+        //NOTE: we used to check here that we could get an instance of
+        //      the tool class, but that's been moved to ToolConfiguration
+        //      in order to fail as earlier as possible.  most people won't
+        //      manually create ToolInfo.  if they do and we can't get an
+        //      instance, they should be capable of figuring out what happened
 
         // search for a configure(Map params) method in the class
         try
@@ -304,15 +302,13 @@
         }
         catch (IllegalAccessException iae)
         {
-            String msg = "Unable to invoke " +
-                         method + " on " + tool;
+            String msg = "Unable to invoke " + method + " on " + tool;
             // restricting access to this method by this class ist verboten
             throw new IllegalStateException(msg, iae);
         }
         catch (InvocationTargetException ite)
         {
-            String msg = "Exception when invoking " +
-                         method + " on " + tool;
+            String msg = "Exception when invoking " + method + " on " + tool;
             // convert to a runtime exception, and re-throw
             throw new RuntimeException(msg, ite.getCause());
         }

Modified: velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/config/ToolConfiguration.java
URL: http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/config/ToolConfiguration.java?view=diff&rev=539268&r1=539267&r2=539268
==============================================================================
--- velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/config/ToolConfiguration.java
(original)
+++ velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/config/ToolConfiguration.java
Thu May 17 22:47:23 2007
@@ -33,9 +33,15 @@
  */
 public class ToolConfiguration extends Configuration
 {
+    private enum Status {
+        VALID, OLD, NONE, MISSING, UNSUPPORTED, UNINSTANTIABLE;
+    }
+
     private String key;
     private String classname;
     private String restrictTo;
+    private Status status;
+    private Throwable problem;
 
     public void setKey(String key)
     {
@@ -48,12 +54,13 @@
      */
     public void setClass(Class clazz)
     {
-        this.classname = clazz.getName();
+        setClassname(clazz.getName());
     }
 
     public void setClassname(String classname)
     {
         this.classname = classname;
+        this.status = null;
     }
 
     public void setRestrictTo(String path)
@@ -125,45 +132,81 @@
         }
     }
 
-    private enum Status {
-        VALID, OLD, NONE, MISSING, UNSUPPORTED;
-    }
-
     private final Status getStatus()
     {
-        if (getClassname() == null)
+        if (this.status == null)
         {
-            return Status.NONE;
-        }
+            if (getClassname() == null)
+            {
+                this.status = Status.NONE;
+            }
 
-        // check for mere presence of init() or configure()
-        try
-        {
-            Class clazz = Utils.getClass(getClassname());
+            // check for mere presence of init() or configure()
+            try
+            {
+                // make sure the classname resolves to a class we have
+                Class clazz = Utils.getClass(getClassname());
+
+                // try hard to ensure we have all necessary supporting classes
+                digForDependencies(clazz);
 
-            Method init = clazz.getMethod("init", new Class[]{ Object.class });
+                // create an instance to make sure we can do that
+                clazz.newInstance();
 
-            // if init is deprecated, then we'll consider it a
-            // new tool with BC support, not an old tool
-            Deprecated bc = init.getAnnotation(Deprecated.class);
-            if (bc == null)
+                // check for an init method
+                Method init =
+                    clazz.getMethod("init", new Class[]{ Object.class });
+
+                // if init is deprecated, then we'll consider it a
+                // new tool with BC support, not an old tool
+                Deprecated bc = init.getAnnotation(Deprecated.class);
+                if (bc == null)
+                {
+                    this.status = Status.OLD;
+                    this.problem = null;
+                }
+                else
+                {
+                    this.status = Status.VALID;
+                    this.problem = null;
+                }
+            }
+            catch (NoSuchMethodException nsme)
             {
-                return Status.OLD;
+                // ignore this
+                this.status = Status.VALID;
+                this.problem = null;
+            }
+            catch (ClassNotFoundException cnfe)
+            {
+                this.status = Status.MISSING;
+                this.problem = cnfe;
+            }
+            catch (NoClassDefFoundError ncdfe)
+            {
+                this.status = Status.UNSUPPORTED;
+                this.problem = ncdfe;
+            }
+            catch (Throwable t)
+            {
+                // for all other problems...
+                this.status = Status.UNINSTANTIABLE;
+                this.problem = t;
             }
         }
-        catch (NoSuchMethodException nsme)
-        {
-            // ignore this
-        }
-        catch (ClassNotFoundException cnfe)
-        {
-            return Status.MISSING;
-        }
-        catch (NoClassDefFoundError ncdfe)
+        return this.status;
+    }
+
+    private void digForDependencies(Class clazz)
+    {
+        clazz.getDeclaredMethods();
+        clazz.getDeclaredFields();
+
+        Class superClass = clazz.getSuperclass();
+        if (superClass != null)
         {
-            return Status.UNSUPPORTED;
+            digForDependencies(superClass);
         }
-        return Status.VALID;
     }
 
     public String getRestrictTo()
@@ -201,9 +244,14 @@
             case NONE:
                 return "No classname set for: "+this;
             case MISSING:
-                return "Couldn't find tool class in the classpath for: "+this;
+                return "Couldn't find tool class in the classpath for: "+this+
+                       "("+this.problem+")";
             case UNSUPPORTED:
-                return "Couldn't find necessary supporting classes for: "+this;
+                return "Couldn't find necessary supporting classes for: "+this+
+                       "("+this.problem+")";
+            case UNINSTANTIABLE:
+                return "Couldn't instantiate instance of tool for: "+this+
+                       "("+this.problem+")";
             default:
                 return "";
         }
@@ -254,6 +302,10 @@
                     break;
                 case UNSUPPORTED:
                     out.append("Unsupported ");
+                    break;
+                case UNINSTANTIABLE:
+                    out.append("Unusable ");
+                    break;
             }
             out.append("Tool '");
             out.append(getKey());



Mime
View raw message