You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by mb...@apache.org on 2013/04/25 18:36:59 UTC

svn commit: r1475844 - in /commons/sandbox/weaver/trunk: modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/ processor/src/main/java/org/apache/commons/weaver/utils/

Author: mbenson
Date: Thu Apr 25 16:36:58 2013
New Revision: 1475844

URL: http://svn.apache.org/r1475844
Log:
refactor classPool creation out for potential reuse; refactor Privilizer to be built with a Builder thus allowing AccessLevel to be configured without subclassing or proliferation of user-accessible constructor args; Privilizer no longer abstract

Modified:
    commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/AccessLevel.java
    commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/Privilizer.java
    commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/PrivilizerWeaver.java
    commons/sandbox/weaver/trunk/processor/src/main/java/org/apache/commons/weaver/utils/Assistant.java

Modified: commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/AccessLevel.java
URL: http://svn.apache.org/viewvc/commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/AccessLevel.java?rev=1475844&r1=1475843&r2=1475844&view=diff
==============================================================================
--- commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/AccessLevel.java (original)
+++ commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/AccessLevel.java Thu Apr 25 16:36:58 2013
@@ -53,4 +53,13 @@ public enum AccessLevel {
     public String toString() {
         return name().toLowerCase(Locale.US);
     }
+
+    /**
+     * Get the {@link AccessLevel} value that should be used as a default.
+     * 
+     * @return {@link AccessLevel#PRIVATE}
+     */
+    public static AccessLevel defaultValue() {
+        return AccessLevel.PRIVATE;
+    }
 }

Modified: commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/Privilizer.java
URL: http://svn.apache.org/viewvc/commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/Privilizer.java?rev=1475844&r1=1475843&r2=1475844&view=diff
==============================================================================
--- commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/Privilizer.java (original)
+++ commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/Privilizer.java Thu Apr 25 16:36:58 2013
@@ -63,7 +63,7 @@ import org.apache.commons.weaver.utils.B
  * @see Privileged
  * @see Privilizing
  */
-public abstract class Privilizer {
+public class Privilizer {
     /**
      * Simple interface to abstract the act of saving modifications to a class.
      */
@@ -81,25 +81,32 @@ public abstract class Privilizer {
         NEVER,
 
         /**
-         * Weaves such that the check for an active {@link SecurityManager} is
-         * done once only.
+         * Weaves such that the check for an active {@link SecurityManager} is done once only.
          */
         ON_INIT(generateName("hasSecurityManager")),
 
         /**
-         * Weaves such that the check for an active {@link SecurityManager} is
-         * done for each {@link Privileged} method execution.
+         * Weaves such that the check for an active {@link SecurityManager} is done for each {@link Privileged} method
+         * execution.
          */
         DYNAMIC(HAS_SECURITY_MANAGER_CONDITION),
 
         /**
-         * Weaves such that {@link Privileged} methods are always executed as
-         * such.
+         * Weaves such that {@link Privileged} methods are always executed as such.
          */
         ALWAYS;
 
         private final String condition;
 
+        /**
+         * Get the {@link Policy} value that should be used as a default.
+         * 
+         * @return {@link Policy#DYNAMIC}
+         */
+        public static Policy defaultValue() {
+            return DYNAMIC;
+        }
+
         private Policy() {
             this(null);
         }
@@ -113,6 +120,36 @@ public abstract class Privilizer {
         }
     }
 
+    /**
+     * Privilizer builder.
+     */
+    public static class Builder {
+        private final ClassPool classPool;
+        private final ModifiedClassWriter modifiedClassWriter;
+        private Policy policy = Policy.defaultValue();
+        private AccessLevel targetAccessLevel = AccessLevel.defaultValue();
+
+        public Builder(ClassPool classPool, ModifiedClassWriter modifiedClassWriter) {
+            super();
+            this.classPool = classPool;
+            this.modifiedClassWriter = modifiedClassWriter;
+        }
+
+        public Builder withPolicy(Policy policy) {
+            this.policy = policy;
+            return this;
+        }
+
+        public Builder withTargetAccessLevel(AccessLevel targetAccessLevel) {
+            this.targetAccessLevel = targetAccessLevel;
+            return this;
+        }
+
+        public Privilizer build() {
+            return new Privilizer(classPool, modifiedClassWriter, policy, targetAccessLevel);
+        }
+    }
+
     protected static final String POLICY_NAME = "policyName";
 
     private static final String ACTION_SUFFIX = "_ACTION";
@@ -174,23 +211,22 @@ public abstract class Privilizer {
     private final ClassPool classPool;
     private final ModifiedClassWriter modifiedClassWriter;
     private final Assistant assistant;
+    private final AccessLevel targetAccessLevel;
 
     private boolean settingsReported;
 
-    public Privilizer(ClassPool classPool, ModifiedClassWriter modifiedClassWriter) {
-        this(classPool, modifiedClassWriter, Policy.DYNAMIC);
-    }
-
-    public Privilizer(ClassPool classPool, ModifiedClassWriter modifiedClassWriter, Policy policy) {
-        this.policy = Validate.notNull(policy, "policy");
+    private Privilizer(ClassPool classPool, ModifiedClassWriter modifiedClassWriter, Policy policy,
+        AccessLevel targetAccessLevel) {
         this.classPool = Validate.notNull(classPool, "classPool");
         this.modifiedClassWriter = Validate.notNull(modifiedClassWriter, "modifiedClassWriter");
+        this.policy = Validate.notNull(policy, "policy");
+        this.targetAccessLevel = Validate.notNull(targetAccessLevel, "targetAccessLevel");
         this.assistant = new Assistant(classPool, "__privilizer_");
     }
 
     /**
-     * Weave the specified class. Handles all {@link Privileged} methods as well
-     * as calls described by {@code privilizing}.
+     * Weave the specified class. Handles all {@link Privileged} methods as well as calls described by
+     * {@code privilizing}.
      * 
      * @param privilizing
      * 
@@ -339,17 +375,13 @@ public abstract class Privilizer {
     /*
      * This design is almost certainly suboptimal. Basically, we have:
      * 
-     * for a declared method, look for calls to blueprint methods for each
-     * blueprint method, copy it when copying, inspect blueprint method's code
-     * and recursively copy in methods from the source class of *that particular
-     * method* because otherwise CtNewMethod will do it for us and we'll miss
-     * our window of opportunity now that we have a copied blueprint method,
-     * inspect it for blueprint calls from other classes and do this whole thing
-     * recursively.
+     * for a declared method, look for calls to blueprint methods for each blueprint method, copy it when copying,
+     * inspect blueprint method's code and recursively copy in methods from the source class of *that particular method*
+     * because otherwise CtNewMethod will do it for us and we'll miss our window of opportunity now that we have a
+     * copied blueprint method, inspect it for blueprint calls from other classes and do this whole thing recursively.
      * 
-     * It would *seem* that we could combine the recursion/copying of methods
-     * from all blueprint classes but I can't get my head around it right now.
-     * -MJB
+     * It would *seem* that we could combine the recursion/copying of methods from all blueprint classes but I can't get
+     * my head around it right now. -MJB
      */
     private CtMethod copyBlueprintTo(final CtClass target, final String toName, final CtMethod method,
         final CallTo[] blueprintCalls) throws ClassNotFoundException, NotFoundException, IOException,
@@ -560,10 +592,6 @@ public abstract class Privilizer {
         log.info(String.format(message, args));
     }
 
-    protected AccessLevel getTargetAccessLevel() {
-        return AccessLevel.PRIVATE;
-    }
-
     private CtClass createAction(CtClass type, CtMethod impl, Class<?> iface) throws NotFoundException,
         CannotCompileException, IOException {
         final boolean exc = impl.getExceptionTypes().length > 0;
@@ -660,9 +688,9 @@ public abstract class Privilizer {
     private boolean weave(CtClass type, CtMethod method) throws ClassNotFoundException, CannotCompileException,
         NotFoundException, IOException, IllegalAccessException {
         final AccessLevel accessLevel = AccessLevel.of(method.getModifiers());
-        if (getTargetAccessLevel().compareTo(accessLevel) > 0) {
+        if (targetAccessLevel.compareTo(accessLevel) > 0) {
             throw new IllegalAccessException("Method " + type.getName() + "#" + toString(method)
-                + " must have maximum access level '" + getTargetAccessLevel() + "' but is defined wider ('"
+                + " must have maximum access level '" + targetAccessLevel + "' but is defined wider ('"
                 + accessLevel + "')");
         }
         if (AccessLevel.PACKAGE.compareTo(accessLevel) > 0) {

Modified: commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/PrivilizerWeaver.java
URL: http://svn.apache.org/viewvc/commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/PrivilizerWeaver.java?rev=1475844&r1=1475843&r2=1475844&view=diff
==============================================================================
--- commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/PrivilizerWeaver.java (original)
+++ commons/sandbox/weaver/trunk/modules/privilizer/weaver/src/main/java/org/apache/commons/weaver/privilizer/PrivilizerWeaver.java Thu Apr 25 16:36:58 2013
@@ -25,17 +25,15 @@ import java.util.Properties;
 import javassist.CannotCompileException;
 import javassist.ClassPool;
 import javassist.CtClass;
-import javassist.LoaderClassPath;
-import javassist.NotFoundException;
 
 import org.apache.commons.lang3.StringUtils;
-import org.apache.commons.lang3.Validate;
 import org.apache.commons.weaver.model.ScanRequest;
 import org.apache.commons.weaver.model.ScanResult;
 import org.apache.commons.weaver.model.WeavableClass;
 import org.apache.commons.weaver.model.WeaveInterest;
 import org.apache.commons.weaver.privilizer.Privilizer.ModifiedClassWriter;
 import org.apache.commons.weaver.spi.Weaver;
+import org.apache.commons.weaver.utils.Assistant;
 import org.apache.commons.weaver.utils.URLArray;
 
 /**
@@ -48,53 +46,12 @@ public class PrivilizerWeaver implements
     public static final String CONFIG_ACCESS_LEVEL = CONFIG_WEAVER + "accessLevel";
     public static final String CONFIG_POLICY = CONFIG_WEAVER + "policy";
 
-    private static ClassPool createClassPool(ClassLoader classpath, File target) {
-        final ClassPool result = new ClassPool();
-        try {
-            result.appendClassPath(validTarget(target).getAbsolutePath());
-            result.appendClassPath(new LoaderClassPath(classpath));
-            result.appendPathList(System.getProperty("java.class.path"));
-        } catch (final NotFoundException e) {
-            throw new RuntimeException(e);
-        }
-        return result;
-    }
-
-    private static File validTarget(File target) {
-        Validate.notNull(target, "target");
-        Validate.isTrue(target.isDirectory(), "not a directory");
-        return target;
-    }
-
     private Privilizer privilizer;
-    private Privilizer.Policy policy;
-    private AccessLevel targetAccessLevel;
 
     @Override
     public void configure(final List<String> classPath, final File target, final Properties config) {
         final URLClassLoader urlClassLoader = new URLClassLoader(URLArray.fromPaths(classPath));
-
-        final String accessLevel = config.getProperty(CONFIG_ACCESS_LEVEL);
-        if (StringUtils.isNotEmpty(accessLevel)) {
-            targetAccessLevel = AccessLevel.valueOf(accessLevel);
-        }
-
-        class ConfiguredPrivilizer extends Privilizer {
-
-            ConfiguredPrivilizer(ClassPool classPool, ModifiedClassWriter modifiedClassWriter, Policy policy) {
-                super(classPool, modifiedClassWriter, policy);
-            }
-
-            ConfiguredPrivilizer(ClassPool classPool, ModifiedClassWriter modifiedClassWriter) {
-                super(classPool, modifiedClassWriter);
-            }
-
-            @Override
-            protected AccessLevel getTargetAccessLevel() {
-                return targetAccessLevel == null ? super.getTargetAccessLevel() : targetAccessLevel;
-            }
-        }
-        final ClassPool classPool = createClassPool(urlClassLoader, target);
+        final ClassPool classPool = Assistant.createClassPool(urlClassLoader, target);
         final ModifiedClassWriter modifiedClassWriter = new ModifiedClassWriter() {
 
             @Override
@@ -103,14 +60,20 @@ public class PrivilizerWeaver implements
             }
         };
 
+        final Privilizer.Builder builder = new Privilizer.Builder(classPool, modifiedClassWriter);
+
+        final String accessLevel = config.getProperty(CONFIG_ACCESS_LEVEL);
+        if (StringUtils.isNotEmpty(accessLevel)) {
+            builder.withTargetAccessLevel(AccessLevel.valueOf(accessLevel));
+        }
+
         final String policyConfig = config.getProperty(CONFIG_POLICY);
 
         if (StringUtils.isNotEmpty(policyConfig)) {
-            policy = Privilizer.Policy.valueOf(policyConfig);
-            privilizer = new ConfiguredPrivilizer(classPool, modifiedClassWriter, policy);
-        } else {
-            privilizer = new ConfiguredPrivilizer(classPool, modifiedClassWriter);
+            builder.withPolicy(Privilizer.Policy.valueOf(policyConfig));
         }
+
+        privilizer = builder.build();
     }
 
     @Override

Modified: commons/sandbox/weaver/trunk/processor/src/main/java/org/apache/commons/weaver/utils/Assistant.java
URL: http://svn.apache.org/viewvc/commons/sandbox/weaver/trunk/processor/src/main/java/org/apache/commons/weaver/utils/Assistant.java?rev=1475844&r1=1475843&r2=1475844&view=diff
==============================================================================
--- commons/sandbox/weaver/trunk/processor/src/main/java/org/apache/commons/weaver/utils/Assistant.java (original)
+++ commons/sandbox/weaver/trunk/processor/src/main/java/org/apache/commons/weaver/utils/Assistant.java Thu Apr 25 16:36:58 2013
@@ -15,6 +15,7 @@
  */
 package org.apache.commons.weaver.utils;
 
+import java.io.File;
 import java.lang.reflect.Field;
 import java.lang.reflect.Modifier;
 import java.util.HashSet;
@@ -26,6 +27,7 @@ import javassist.CtClass;
 import javassist.CtField;
 import javassist.CtMethod;
 import javassist.CtNewMethod;
+import javassist.LoaderClassPath;
 import javassist.NotFoundException;
 
 import org.apache.commons.lang3.Validate;
@@ -61,6 +63,29 @@ public class Assistant {
         }
     }
 
+    /**
+     * Create a {@link ClassPool} including the target directory and an associated classpath.
+     * 
+     * @param classpath
+     * @param target
+     * @return ClassPool
+     */
+    public static ClassPool createClassPool(ClassLoader classpath, File target) {
+        Validate.notNull(classpath, "classpath");
+        Validate.notNull(target, "target");
+        Validate.isTrue(target.isDirectory(), "not a directory");
+
+        final ClassPool result = new ClassPool();
+        try {
+            result.appendClassPath(target.getAbsolutePath());
+            result.appendClassPath(new LoaderClassPath(classpath));
+            result.appendPathList(System.getProperty("java.class.path"));
+        } catch (final NotFoundException e) {
+            throw new RuntimeException(e);
+        }
+        return result;
+    }
+
     private final ClassPool classPool;
     private final String prefix;