ISPN-5335 ConfigurationBuilder with custom interceptor leads to share…

…d interceptor

core/src/main/java/org/infinispan/configuration/cache/InterceptorConfiguration.java

@@ -4,6 +4,7 @@
 import org.infinispan.commons.configuration.attributes.Attribute;
 import org.infinispan.commons.configuration.attributes.AttributeDefinition;
 import org.infinispan.commons.configuration.attributes.AttributeSet;
+import org.infinispan.commons.util.Util;
 import org.infinispan.interceptors.base.CommandInterceptor;
 
 /**
@@ -33,16 +34,18 @@ public static enum Position {
    public static final AttributeDefinition<Class> AFTER = AttributeDefinition.builder("after", null, Class.class).immutable().build();
    public static final AttributeDefinition<Class> BEFORE = AttributeDefinition.builder("before", null, Class.class).immutable().build();
    public static final AttributeDefinition<CommandInterceptor> INTERCEPTOR = AttributeDefinition.builder("interceptor", null, CommandInterceptor.class).immutable().build();
+   public static final AttributeDefinition<Class> INTERCEPTOR_CLASS = AttributeDefinition.builder("interceptorClass", null, Class.class).immutable().build();
    public static final AttributeDefinition<Integer> INDEX = AttributeDefinition.builder("index", -1).immutable().build();
 
    public static AttributeSet attributeDefinitionSet() {
-      return new AttributeSet(InterceptorConfiguration.class, AbstractTypedPropertiesConfiguration.attributeSet(), POSITION, AFTER, BEFORE, INTERCEPTOR, INDEX);
+      return new AttributeSet(InterceptorConfiguration.class, AbstractTypedPropertiesConfiguration.attributeSet(), POSITION, AFTER, BEFORE, INTERCEPTOR, INTERCEPTOR_CLASS, INDEX);
    }
 
    private final Attribute<Position> position;
    private final Attribute<Class> after;
    private final Attribute<Class> before;
    private final Attribute<CommandInterceptor> interceptor;
+   private final Attribute<Class> interceptorClass;
    private final Attribute<Integer> index;
 
    InterceptorConfiguration(AttributeSet attributes) {
@@ -51,6 +54,7 @@ public static AttributeSet attributeDefinitionSet() {
       after = attributes.attribute(AFTER);
       before = attributes.attribute(BEFORE);
       interceptor = attributes.attribute(INTERCEPTOR);
+      interceptorClass = attributes.attribute(INTERCEPTOR_CLASS);
       index = attributes.attribute(INDEX);
    }
 
@@ -65,7 +69,15 @@ public Class<? extends CommandInterceptor> before() {
    }
 
    public CommandInterceptor interceptor() {
-      return interceptor.get();
+      if (interceptor.isNull()) {
+         return (CommandInterceptor) Util.getInstance(interceptorClass.get());
+      } else {
+         return interceptor.get();
+      }
+   }
+
+   public Class<? extends CommandInterceptor> interceptorClass() {
+      return interceptorClass.get();
    }
 
    public int index() {

core/src/main/java/org/infinispan/configuration/cache/InterceptorConfigurationBuilder.java

@@ -1,21 +1,22 @@
 package org.infinispan.configuration.cache;
 
-import static org.infinispan.commons.configuration.AbstractTypedPropertiesConfiguration.PROPERTIES;
-import static org.infinispan.configuration.cache.InterceptorConfiguration.*;
-
-import java.util.Properties;
-
 import org.infinispan.commons.CacheConfigurationException;
 import org.infinispan.commons.configuration.Builder;
+import org.infinispan.commons.configuration.attributes.Attribute;
 import org.infinispan.commons.configuration.attributes.AttributeSet;
 import org.infinispan.commons.util.TypedProperties;
-import org.infinispan.configuration.cache.InterceptorConfiguration.Position;
+import org.infinispan.configuration.cache.InterceptorConfiguration.*;
 import org.infinispan.configuration.global.GlobalConfiguration;
 import org.infinispan.interceptors.base.BaseCustomInterceptor;
 import org.infinispan.interceptors.base.CommandInterceptor;
 import org.infinispan.util.logging.Log;
 import org.infinispan.util.logging.LogFactory;
 
+import java.util.Properties;
+
+import static org.infinispan.commons.configuration.AbstractTypedPropertiesConfiguration.PROPERTIES;
+import static org.infinispan.configuration.cache.InterceptorConfiguration.*;
+
 /**
  * This builder defines details of a specific custom interceptor.
  */
@@ -52,8 +53,19 @@ public InterceptorConfigurationBuilder before(Class<? extends CommandInterceptor
       return this;
    }
 
+   /**
+    * Class of the new custom interceptor to add to the configuration.
+    * @param interceptorClass an instance of {@link CommandInterceptor}
+    */
+   public InterceptorConfigurationBuilder interceptorClass(Class<? extends CommandInterceptor> interceptorClass) {
+      attributes.attribute(INTERCEPTOR_CLASS).set(interceptorClass);
+      return this;
+   }
+
    /**
     * An instance of the new custom interceptor to add to the configuration.
+    * Warning: if you use this configuration for multiple caches, the interceptor instance will
+    * be shared, which will corrupt interceptor stack. Use {@link #interceptorClass} instead.
     * @param interceptor an instance of {@link CommandInterceptor}
     */
    public InterceptorConfigurationBuilder interceptor(CommandInterceptor interceptor) {
@@ -122,6 +134,28 @@ public InterceptorConfigurationBuilder removeProperty(String key) {
 
    @Override
    public void validate() {
+      Attribute<Class> interceptorClassAttribute = attributes.attribute(INTERCEPTOR_CLASS);
+      Attribute<CommandInterceptor> interceptorAttribute = attributes.attribute(INTERCEPTOR);
+
+
+      if (!interceptorClassAttribute.isNull() && !interceptorAttribute.isNull()) {
+         throw log.interceptorClassAndInstanceDefined(interceptorClassAttribute.get().getName(), interceptorAttribute.get().toString());
+      } else if (interceptorClassAttribute.isNull() && interceptorAttribute.isNull()) {
+         throw log.customInterceptorMissingClass();
+      }
+      Class<? extends CommandInterceptor> interceptorClass = interceptorClassAttribute.get();
+      if (interceptorClass == null) {
+         interceptorClass = interceptorAttribute.get().getClass();
+      }
+
+      if (!BaseCustomInterceptor.class.isAssignableFrom(interceptorClass)) {
+         final String className = interceptorClass.getName();
+         //Suppress noisy warnings if the interceptor is one of our own (like one of those from Query):
+         if (! className.startsWith("org.infinispan.")) {
+            log.suggestCustomInterceptorInheritance(className);
+         }
+      }
+
       // Make sure more than one 'position' isn't picked.
       int positions = 0;
 
@@ -130,25 +164,13 @@ public void validate() {
       if (attributes.attribute(INDEX).get() > -1) positions++;
       if (attributes.attribute(POSITION).isModified()) positions++;
 
-      CommandInterceptor interceptor = attributes.attribute(INTERCEPTOR).get();
       switch (positions) {
          case 0:
-            throw log.missingCustomInterceptorPosition(interceptor.getClass().getName());
+            throw log.missingCustomInterceptorPosition(interceptorClass.getName());
          case 1:
             break;
          default:
-            throw log.multipleCustomInterceptorPositions(interceptor.getClass().getName());
-      }
-
-      if (interceptor == null) {
-         throw log.customInterceptorMissingClass();
-      }
-      else if (!(interceptor instanceof BaseCustomInterceptor)) {
-         final String className = interceptor.getClass().getName();
-         //Suppress noisy warnings if the interceptor is one of our own (like one of those from Query):
-         if (! className.startsWith("org.infinispan.")) {
-            log.suggestCustomInterceptorInheritance(className);
-         }
+            throw log.multipleCustomInterceptorPositions(interceptorClass.getName());
       }
    }
 

core/src/main/java/org/infinispan/configuration/parsing/Parser70.java

@@ -6,6 +6,7 @@
 import org.infinispan.commons.executors.BlockingThreadPoolExecutorFactory;
 import org.infinispan.commons.executors.CachedThreadPoolExecutorFactory;
 import org.infinispan.commons.executors.ScheduledThreadPoolExecutorFactory;
+import org.infinispan.commons.executors.ThreadPoolExecutorFactory;
 import org.infinispan.commons.marshall.AdvancedExternalizer;
 import org.infinispan.commons.marshall.Marshaller;
 import org.infinispan.commons.util.Util;
@@ -16,7 +17,6 @@
 import org.infinispan.configuration.global.ShutdownHookBehavior;
 import org.infinispan.configuration.global.ThreadPoolConfiguration;
 import org.infinispan.configuration.global.ThreadPoolConfigurationBuilder;
-import org.infinispan.commons.executors.ThreadPoolExecutorFactory;
 import org.infinispan.configuration.global.TransportConfigurationBuilder;
 import org.infinispan.container.DataContainer;
 import org.infinispan.distribution.ch.ConsistentHashFactory;
@@ -35,7 +35,8 @@
 import org.infinispan.security.impl.ClusterRoleMapper;
 import org.infinispan.security.impl.CommonNameRoleMapper;
 import org.infinispan.security.impl.IdentityRoleMapper;
-import org.infinispan.transaction.*;
+import org.infinispan.transaction.LockingMode;
+import org.infinispan.transaction.TransactionProtocol;
 import org.infinispan.transaction.lookup.TransactionManagerLookup;
 import org.infinispan.util.concurrent.IsolationLevel;
 import org.infinispan.util.logging.Log;
@@ -44,7 +45,6 @@
 
 import javax.xml.stream.XMLStreamConstants;
 import javax.xml.stream.XMLStreamException;
-
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -1242,7 +1242,7 @@ private void parseInterceptor(XMLExtendedStreamReader reader, ConfigurationBuild
                interceptorBuilder.before(Util.<CommandInterceptor>loadClass(value, holder.getClassLoader()));
                break;
             case CLASS:
-               interceptorBuilder.interceptor(Util.<CommandInterceptor>getInstance(value, holder.getClassLoader()));
+               interceptorBuilder.interceptorClass(Util.<CommandInterceptor>loadClass(value, holder.getClassLoader()));
                break;
             case INDEX:
                interceptorBuilder.index(Integer.parseInt(value));

core/src/main/java/org/infinispan/configuration/parsing/Parser71.java

@@ -6,6 +6,7 @@
 import org.infinispan.commons.executors.BlockingThreadPoolExecutorFactory;
 import org.infinispan.commons.executors.CachedThreadPoolExecutorFactory;
 import org.infinispan.commons.executors.ScheduledThreadPoolExecutorFactory;
+import org.infinispan.commons.executors.ThreadPoolExecutorFactory;
 import org.infinispan.commons.marshall.AdvancedExternalizer;
 import org.infinispan.commons.marshall.Marshaller;
 import org.infinispan.commons.util.Util;
@@ -16,7 +17,6 @@
 import org.infinispan.configuration.global.ShutdownHookBehavior;
 import org.infinispan.configuration.global.ThreadPoolConfiguration;
 import org.infinispan.configuration.global.ThreadPoolConfigurationBuilder;
-import org.infinispan.commons.executors.ThreadPoolExecutorFactory;
 import org.infinispan.configuration.global.TransportConfigurationBuilder;
 import org.infinispan.container.DataContainer;
 import org.infinispan.distribution.ch.ConsistentHashFactory;
@@ -35,7 +35,8 @@
 import org.infinispan.security.impl.ClusterRoleMapper;
 import org.infinispan.security.impl.CommonNameRoleMapper;
 import org.infinispan.security.impl.IdentityRoleMapper;
-import org.infinispan.transaction.*;
+import org.infinispan.transaction.LockingMode;
+import org.infinispan.transaction.TransactionProtocol;
 import org.infinispan.transaction.lookup.TransactionManagerLookup;
 import org.infinispan.util.concurrent.IsolationLevel;
 import org.infinispan.util.logging.Log;
@@ -1242,7 +1243,7 @@ private void parseInterceptor(XMLExtendedStreamReader reader, ConfigurationBuild
                interceptorBuilder.before(Util.<CommandInterceptor>loadClass(value, holder.getClassLoader()));
                break;
             case CLASS:
-               interceptorBuilder.interceptor(Util.<CommandInterceptor>getInstance(value, holder.getClassLoader()));
+               interceptorBuilder.interceptorClass(Util.<CommandInterceptor>loadClass(value, holder.getClassLoader()));
                break;
             case INDEX:
                interceptorBuilder.index(Integer.parseInt(value));

core/src/main/java/org/infinispan/configuration/parsing/Parser72.java

@@ -6,6 +6,7 @@
 import org.infinispan.commons.executors.BlockingThreadPoolExecutorFactory;
 import org.infinispan.commons.executors.CachedThreadPoolExecutorFactory;
 import org.infinispan.commons.executors.ScheduledThreadPoolExecutorFactory;
+import org.infinispan.commons.executors.ThreadPoolExecutorFactory;
 import org.infinispan.commons.marshall.AdvancedExternalizer;
 import org.infinispan.commons.marshall.Marshaller;
 import org.infinispan.commons.util.Util;
@@ -16,7 +17,6 @@
 import org.infinispan.configuration.global.ShutdownHookBehavior;
 import org.infinispan.configuration.global.ThreadPoolConfiguration;
 import org.infinispan.configuration.global.ThreadPoolConfigurationBuilder;
-import org.infinispan.commons.executors.ThreadPoolExecutorFactory;
 import org.infinispan.configuration.global.TransportConfigurationBuilder;
 import org.infinispan.container.DataContainer;
 import org.infinispan.distribution.ch.ConsistentHashFactory;
@@ -35,7 +35,8 @@
 import org.infinispan.security.impl.ClusterRoleMapper;
 import org.infinispan.security.impl.CommonNameRoleMapper;
 import org.infinispan.security.impl.IdentityRoleMapper;
-import org.infinispan.transaction.*;
+import org.infinispan.transaction.LockingMode;
+import org.infinispan.transaction.TransactionProtocol;
 import org.infinispan.transaction.lookup.TransactionManagerLookup;
 import org.infinispan.util.concurrent.IsolationLevel;
 import org.infinispan.util.logging.Log;
@@ -1248,7 +1249,7 @@ private void parseInterceptor(XMLExtendedStreamReader reader, ConfigurationBuild
                interceptorBuilder.before(Util.<CommandInterceptor>loadClass(value, holder.getClassLoader()));
                break;
             case CLASS:
-               interceptorBuilder.interceptor(Util.<CommandInterceptor>getInstance(value, holder.getClassLoader()));
+               interceptorBuilder.interceptorClass(Util.<CommandInterceptor>loadClass(value, holder.getClassLoader()));
                break;
             case INDEX:
                interceptorBuilder.index(Integer.parseInt(value));

core/src/main/java/org/infinispan/interceptors/base/BaseCustomInterceptor.java

@@ -27,6 +27,11 @@ public class BaseCustomInterceptor extends CommandInterceptor {
 
    @Inject
    private void setup(Cache<?, ?> cache, EmbeddedCacheManager embeddedCacheManager) {
+      if (this.cache != null) {
+         // see https://issues.jboss.org/browse/ISPN-5335
+         throw new IllegalStateException("Setting up the interceptor second time;" +
+               "this could be caused by the same instance of interceptor used by several caches.");
+      }
       this.cache = cache;
       this.embeddedCacheManager = embeddedCacheManager;
    }

core/src/main/java/org/infinispan/util/logging/Log.java

@@ -1,33 +1,5 @@
 package org.infinispan.util.logging;
 
-import static org.jboss.logging.Logger.Level.DEBUG;
-import static org.jboss.logging.Logger.Level.ERROR;
-import static org.jboss.logging.Logger.Level.FATAL;
-import static org.jboss.logging.Logger.Level.INFO;
-import static org.jboss.logging.Logger.Level.TRACE;
-import static org.jboss.logging.Logger.Level.WARN;
-
-import java.io.File;
-import java.io.IOException;
-import java.lang.reflect.Method;
-import java.net.URL;
-import java.nio.channels.FileChannel;
-import java.security.Permission;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.UUID;
-import java.util.concurrent.CountDownLatch;
-
-import javax.management.InstanceAlreadyExistsException;
-import javax.management.MBeanRegistrationException;
-import javax.management.ObjectName;
-import javax.naming.NamingException;
-import javax.transaction.Synchronization;
-import javax.transaction.TransactionManager;
-import javax.transaction.xa.XAException;
-import javax.xml.namespace.QName;
-
 import org.infinispan.IllegalLifecycleStateException;
 import org.infinispan.commands.ReplicableCommand;
 import org.infinispan.commands.tx.PrepareCommand;
@@ -61,6 +33,28 @@
 import org.jboss.logging.annotations.MessageLogger;
 import org.jgroups.View;
 
+import javax.management.InstanceAlreadyExistsException;
+import javax.management.MBeanRegistrationException;
+import javax.management.ObjectName;
+import javax.naming.NamingException;
+import javax.transaction.Synchronization;
+import javax.transaction.TransactionManager;
+import javax.transaction.xa.XAException;
+import javax.xml.namespace.QName;
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Method;
+import java.net.URL;
+import java.nio.channels.FileChannel;
+import java.security.Permission;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+
+import static org.jboss.logging.Logger.Level.*;
+
 /**
  * Infinispan's log abstraction layer on top of JBoss Logging.
  * <p/>
@@ -1299,4 +1293,7 @@ void asyncStoreShutdownTimeoutTooHigh(long configuredAsyncStopTimeout,
 
    @Message(value = "Use of the replication queue is only allowed with an ASYNCHRONOUS cluster mode.", id = 353)
    CacheConfigurationException replicationQueueOnlyForAsyncCaches();
+
+   @Message(value = "Cannot define both interceptor class (%s) and interceptor instance (%s)", id = 354)
+   CacheConfigurationException interceptorClassAndInstanceDefined(String customInterceptorClassName, String customInterceptor);
 }