Cleanup JavassistUtils

Add explicit assumption that asCtClass() is synchronize and follow
that to AbstractStreamWriterGenerator synchronization.

Change-Id: I0f9141d535d370c0285aa6d5e43f45ab4bd0a1eb
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java
index c76bb43de5d4f05804c73efabc6f020b78544b21..bb9a7f176c8c771e9d82df35b2e39550e1250bcc 100644 (file)
--- a/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java
+++ b/binding/mdsal-binding-dom-codec/src/main/java/org/opendaylight/mdsal/binding/dom/codec/gen/impl/AbstractStreamWriterGenerator.java
@@ -7,7 +7,9 @@
  */
 package org.opendaylight.mdsal.binding.dom.codec.gen.impl;
 
-import com.google.common.base.Preconditions;
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
+
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
@@ -72,13 +74,15 @@ abstract class AbstractStreamWriterGenerator extends AbstractGenerator implement
     }
 
     protected AbstractStreamWriterGenerator(final JavassistUtils utils) {
-        this.javassist = Preconditions.checkNotNull(utils, "JavassistUtils instance is required.");
-        this.serializeArguments = new CtClass[] {
-                javassist.asCtClass(DataObjectSerializerRegistry.class),
-                javassist.asCtClass(DataObject.class),
-                javassist.asCtClass(BindingStreamEventWriter.class),
-        };
-        javassist.appendClassLoaderIfMissing(DataObjectSerializerPrototype.class.getClassLoader());
+        this.javassist = requireNonNull(utils, "JavassistUtils instance is required.");
+        synchronized (javassist) {
+            this.serializeArguments = new CtClass[] {
+                    javassist.asCtClass(DataObjectSerializerRegistry.class),
+                    javassist.asCtClass(DataObject.class),
+                    javassist.asCtClass(BindingStreamEventWriter.class),
+            };
+            javassist.appendClassLoaderIfMissing(DataObjectSerializerPrototype.class.getClassLoader());
+        }
         this.implementations = CacheBuilder.newBuilder().weakKeys().build(new SerializerImplementationLoader());
     }
 
@@ -110,8 +114,8 @@ abstract class AbstractStreamWriterGenerator extends AbstractGenerator implement
         @Override
         @SuppressWarnings("unchecked")
         public DataObjectSerializerImplementation load(final Class<?> type) throws Exception {
-            Preconditions.checkArgument(BindingReflections.isBindingClass(type));
-            Preconditions.checkArgument(DataContainer.class.isAssignableFrom(type),
+            checkArgument(BindingReflections.isBindingClass(type));
+            checkArgument(DataContainer.class.isAssignableFrom(type),
                 "DataContainer is not assingnable from %s from classloader %s.", type, type.getClassLoader());
 
             final String serializerName = getSerializerName(type);
@@ -221,7 +225,7 @@ abstract class AbstractStreamWriterGenerator extends AbstractGenerator implement
                     // The prototype is not visible, so we need to take care of that
                     cls.setModifiers(Modifier.setPublic(cls.getModifiers()));
                 });
-        } catch (final NotFoundException e) {
+        } catch (NotFoundException | CannotCompileException e) {
             LOG.error("Failed to instatiate serializer {}", source, e);
             throw new LinkageError("Unexpected instantation problem: serializer prototype not found", e);
         }

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/ClassCustomizer.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/ClassCustomizer.java
index 40cf3c6c89f1d85bb252582f07a0900e3eec4c08..20a6e4af711c392c60e571e46670979ed5341dd2 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/ClassCustomizer.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/ClassCustomizer.java
@@ -8,19 +8,22 @@
 package org.opendaylight.mdsal.binding.generator.util;
 
 import com.google.common.annotations.Beta;
-
+import javassist.CannotCompileException;
 import javassist.CtClass;
+import javassist.NotFoundException;
 
 /**
  * Interface allowing customization of classes after loading.
  */
 @Beta
+@FunctionalInterface
 public interface ClassCustomizer {
     /**
      * Customize a class.
      *
      * @param cls Class to be customized
-     * @throws Exception when a problem ensues.
+     * @throws CannotCompileException when a javassist error occurs
+     * @throws NotFoundException when a javassist error occurs
      */
-    void customizeClass(CtClass cls) throws Exception;
+    void customizeClass(CtClass cls) throws CannotCompileException, NotFoundException;
 }

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtils.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtils.java
index 4bb02c0ddcc721e0f231a915a03fb40a46514fa6..8e37d9b3a8b1e4987bc5f089b41422f28fc9992c 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtils.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtils.java
@@ -7,8 +7,10 @@
  */
 package org.opendaylight.mdsal.binding.generator.util;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
+
 import com.google.common.annotations.Beta;
-import com.google.common.base.Preconditions;
 import java.util.Collection;
 import java.util.Map;
 import java.util.WeakHashMap;
@@ -22,6 +24,7 @@ import javassist.CtMethod;
 import javassist.LoaderClassPath;
 import javassist.Modifier;
 import javassist.NotFoundException;
+import javax.annotation.concurrent.GuardedBy;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -31,13 +34,13 @@ import org.slf4j.LoggerFactory;
  */
 public final class JavassistUtils {
     private static final Logger LOG = LoggerFactory.getLogger(JavassistUtils.class);
-
     private static final Map<ClassPool, JavassistUtils> INSTANCES = new WeakHashMap<>();
+
     private final Map<ClassLoader, ClassPath> loaderClassPaths = new WeakHashMap<>();
     private final ClassPool classPool;
 
     private JavassistUtils(final ClassPool pool) {
-        classPool = Preconditions.checkNotNull(pool);
+        classPool = requireNonNull(pool);
     }
 
     /**
@@ -49,7 +52,7 @@ public final class JavassistUtils {
      * @return shared utility instance for specified pool
      */
     public static synchronized JavassistUtils forClassPool(final ClassPool pool) {
-        JavassistUtils ret = INSTANCES.get(Preconditions.checkNotNull(pool));
+        JavassistUtils ret = INSTANCES.get(requireNonNull(pool));
         if (ret == null) {
             ret = new JavassistUtils(pool);
             INSTANCES.put(pool, ret);
@@ -67,7 +70,8 @@ public final class JavassistUtils {
     }
 
     public void method(final CtClass it, final Class<?> returnType, final String name,
-            final Collection<? extends Class<?>> parameters, final MethodGenerator function1) throws CannotCompileException {
+            final Collection<? extends Class<?>> parameters, final MethodGenerator function1)
+                    throws CannotCompileException {
         final CtClass[] pa = new CtClass[parameters.size()];
 
         int i = 0;
@@ -89,7 +93,8 @@ public final class JavassistUtils {
         it.addMethod(method);
     }
 
-    public void implementMethodsFrom(final CtClass target, final CtClass source, final MethodGenerator function1) throws CannotCompileException {
+    public void implementMethodsFrom(final CtClass target, final CtClass source, final MethodGenerator function1)
+            throws CannotCompileException {
         for (CtMethod method : source.getMethods()) {
             if (method.getDeclaringClass() == source) {
                 CtMethod redeclaredMethod = new CtMethod(method, target, null);
@@ -105,7 +110,8 @@ public final class JavassistUtils {
         return target;
     }
 
-    public CtClass createClass(final String fqn, final CtClass superInterface, final ClassGenerator cls) throws CannotCompileException {
+    public CtClass createClass(final String fqn, final CtClass superInterface, final ClassGenerator cls)
+            throws CannotCompileException {
         CtClass target = classPool.makeClass(fqn);
         implementsType(target, superInterface);
         cls.process(target);
@@ -113,8 +119,8 @@ public final class JavassistUtils {
     }
 
     /**
-     * Instantiate a new class based on a prototype. The class is set to automatically
-     * prune.
+     * Instantiate a new class based on a prototype. The class is set to automatically prune. The {@code customizer}
+     * is guaranteed to run with this object locked.
      *
      * @param prototype Prototype class fully qualified name
      * @param fqn Target class fully qualified name
@@ -123,25 +129,31 @@ public final class JavassistUtils {
      * @throws NotFoundException when the prototype class is not found
      */
     @Beta
-    public synchronized CtClass instantiatePrototype(final String prototype, final String fqn, final ClassCustomizer customizer) throws NotFoundException {
+    public synchronized CtClass instantiatePrototype(final String prototype, final String fqn,
+            final ClassCustomizer customizer) throws CannotCompileException, NotFoundException {
         final CtClass result = classPool.getAndRename(prototype, fqn);
         try {
             customizer.customizeClass(result);
+        } catch (CannotCompileException | NotFoundException e) {
+            result.detach();
+            throw e;
         } catch (Exception e) {
             LOG.warn("Failed to customize {} from prototype {}", fqn, prototype, e);
             result.detach();
-            throw new IllegalStateException(String.format("Failed to instantiate prototype %s as %s", prototype, fqn), e);
+            throw new IllegalStateException(String.format("Failed to instantiate prototype %s as %s", prototype, fqn),
+                e);
         }
 
         result.stopPruning(false);
         return result;
     }
 
-    public void implementsType(final CtClass it, final CtClass supertype) {
-        Preconditions.checkArgument(supertype.isInterface(), "Supertype must be interface");
+    private static void implementsType(final CtClass it, final CtClass supertype) {
+        checkArgument(supertype.isInterface(), "Supertype %s is not an interface", supertype);
         it.addInterface(supertype);
     }
 
+    @GuardedBy("this")
     public CtClass asCtClass(final Class<?> class1) {
         return get(this.classPool, class1);
     }

diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtilsTest.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtilsTest.java
index 461af632bb48d3e3797ac0228444ffcad70b3738..e1f8a16283597fbc9be14191484649d28bfba91a 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtilsTest.java
+++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/util/JavassistUtilsTest.java
@@ -15,20 +15,18 @@ import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
 
 import com.google.common.collect.ImmutableList;
+import javassist.CannotCompileException;
 import javassist.ClassPool;
 import javassist.CtClass;
 import javassist.CtField;
+import javassist.NotFoundException;
 import javassist.bytecode.AccessFlag;
 import org.junit.Test;
-import org.opendaylight.mdsal.binding.generator.util.ClassCustomizer;
-import org.opendaylight.mdsal.binding.generator.util.ClassGenerator;
-import org.opendaylight.mdsal.binding.generator.util.JavassistUtils;
-import org.opendaylight.mdsal.binding.generator.util.MethodGenerator;
 
 public class JavassistUtilsTest {
 
     @Test
-    public void forClassPool() throws Exception {
+    public void forClassPool() throws CannotCompileException, NotFoundException {
         final JavassistUtils javassistUtils = JavassistUtils.forClassPool(ClassPool.getDefault());
         final ClassGenerator classGenerator = mock(ClassGenerator.class);
         doNothing().when(classGenerator).process(any());