ISPN-7286: Cache stores should declare whether they can be shared (Am…

…endment).
infinispan · Jan 23, 2017 · e53db36 · e53db36
1 parent 5be1424
commit e53db36
Show file tree

Hide file tree

Showing 33 changed files with 101 additions and 45 deletions.
diff --git a/core/src/main/java/org/infinispan/configuration/cache/AbstractStoreConfigurationBuilder.java b/core/src/main/java/org/infinispan/configuration/cache/AbstractStoreConfigurationBuilder.java
@@ -13,6 +13,7 @@
 
 import org.infinispan.commons.CacheConfigurationException;
 import org.infinispan.commons.configuration.Builder;
+import org.infinispan.commons.configuration.ConfigurationFor;
 import org.infinispan.commons.configuration.attributes.AttributeSet;
 import org.infinispan.commons.persistence.Store;
 import org.infinispan.commons.util.ReflectionUtil;
@@ -190,14 +191,19 @@ public void validate() {
       boolean transactional = attributes.attribute(TRANSACTIONAL).get();
       ConfigurationBuilder builder = getBuilder();
 
-      Class storeClazz = attributes.getKlass();
-      if (storeClazz != null && storeClazz.isAnnotationPresent(Store.class)) {
-         Store storeProps = (Store) storeClazz.getAnnotation(Store.class);
-         if (!storeProps.shared() && shared) {
-            throw log.nonSharedStoreConfiguredAsShared(attributes.getName());
+      Class configKlass = attributes.getKlass();
+      if (configKlass != null && configKlass.isAnnotationPresent(ConfigurationFor.class)) {
+         Class storeKlass = ((ConfigurationFor) configKlass.getAnnotation(ConfigurationFor.class)).value();
+         if (storeKlass.isAnnotationPresent(Store.class)) {
+            Store storeProps = (Store) storeKlass.getAnnotation(Store.class);
+            if (!storeProps.shared() && shared) {
+               throw log.nonSharedStoreConfiguredAsShared(storeKlass.getSimpleName());
+            }
+         } else {
+            log.warnStoreAnnotationMissing(storeKlass.getSimpleName());
          }
       } else {
-         log.warnStoreAnnotationMissing(attributes.getName());
+         log.warnConfigurationForAnnotationMissing(attributes.getName());
       }
 
       if (!shared && !fetchPersistentState && !purgeOnStartup

diff --git a/...src/main/java/org/infinispan/configuration/cache/SingleFileStoreConfigurationBuilder.java b/...src/main/java/org/infinispan/configuration/cache/SingleFileStoreConfigurationBuilder.java
@@ -5,6 +5,8 @@
 import static org.infinispan.configuration.cache.SingleFileStoreConfiguration.MAX_ENTRIES;
 
 import org.infinispan.commons.configuration.Builder;
+import org.infinispan.commons.configuration.attributes.AttributeSet;
+
 /**
  * Single file cache store configuration builder.
  *
@@ -14,9 +16,12 @@
 public class SingleFileStoreConfigurationBuilder
       extends AbstractStoreConfigurationBuilder<SingleFileStoreConfiguration, SingleFileStoreConfigurationBuilder> {
 
-
    public SingleFileStoreConfigurationBuilder(PersistenceConfigurationBuilder builder) {
-      super(builder, SingleFileStoreConfiguration.attributeDefinitionSet());
+      this(builder, SingleFileStoreConfiguration.attributeDefinitionSet());
+   }
+
+   public SingleFileStoreConfigurationBuilder(PersistenceConfigurationBuilder builder, AttributeSet attributeSet) {
+      super(builder, attributeSet);
    }
 
    @Override

diff --git a/core/src/main/java/org/infinispan/persistence/file/SingleFileStore.java b/core/src/main/java/org/infinispan/persistence/file/SingleFileStore.java
@@ -21,6 +21,7 @@
 
 import org.infinispan.commons.configuration.ConfiguredBy;
 import org.infinispan.commons.io.ByteBufferFactory;
+import org.infinispan.commons.persistence.Store;
 import org.infinispan.commons.util.CollectionFactory;
 import org.infinispan.configuration.cache.SingleFileStoreConfiguration;
 import org.infinispan.executors.ExecutorAllCompletionService;
@@ -64,6 +65,7 @@
  * @author Mircea Markus
  * @since 6.0
  */
+@Store
 @ConfiguredBy(SingleFileStoreConfiguration.class)
 public class SingleFileStore<K, V> implements AdvancedLoadWriteStore<K, V> {
    private static final Log log = LogFactory.getLog(SingleFileStore.class);

diff --git a/core/src/main/java/org/infinispan/util/logging/Log.java b/core/src/main/java/org/infinispan/util/logging/Log.java
@@ -1485,7 +1485,7 @@ TimeoutException timeoutWaitingForView(int expectedViewId, int currentViewId,
    CacheConfigurationException nonSharedStoreConfiguredAsShared(String storeType);
 
    @LogMessage(level = WARN)
-   @Message(value = "Unable to validate all properties of %s's configuration as the @Store attribute is missing", id = 431)
+   @Message(value = "Unable to validate %s's configuration as the @Store annotation is missing", id = 431)
    void warnStoreAnnotationMissing(String name);
 
    @Message(value = "Missing configuration for default cache '%s' declared on container", id = 432)
@@ -1504,4 +1504,8 @@ TimeoutException timeoutWaitingForView(int expectedViewId, int currentViewId,
 
    @Message(value = "Cache '%s' has been requested, but no cache configuration exists with that name and no default cache has been set for this container", id = 436)
    CacheConfigurationException noSuchCacheConfiguration(String name);
+
+   @LogMessage(level = WARN)
+   @Message(value = "Unable to validate %s with the implementing store as the @ConfigurationFor annotation is missing", id = 437)
+   void warnConfigurationForAnnotationMissing(String name);
 }
diff --git a/core/src/test/java/org/infinispan/configuration/parsing/UnifiedXmlFileParsingTest.java b/core/src/test/java/org/infinispan/configuration/parsing/UnifiedXmlFileParsingTest.java
@@ -279,7 +279,7 @@ private static void configurationCheck70(EmbeddedCacheManager cm) {
       assertFalse(fileStore.singletonStore().enabled());
       assertFalse(fileStore.purgeOnStartup());
       assertTrue(fileStore.preload());
-      assertTrue(fileStore.shared());
+      assertFalse(fileStore.shared());
       assertEquals(2048, fileStore.async().modificationQueueSize());
       assertEquals(1, fileStore.async().threadPoolSize());
       assertEquals(Index.NONE, c.indexing().index());

diff --git a/core/src/test/java/org/infinispan/persistence/StoreConfigurationValidationTest.java b/core/src/test/java/org/infinispan/persistence/StoreConfigurationValidationTest.java
@@ -83,7 +83,7 @@ public NonSharedDummyInMemoryStore() {
    static class NonSharedDummyStoreConfiguration extends DummyInMemoryStoreConfiguration {
 
       public static AttributeSet attributeDefinitionSet() {
-         return new AttributeSet(NonSharedDummyInMemoryStore.class, DummyInMemoryStoreConfiguration.attributeDefinitionSet());
+         return new AttributeSet(NonSharedDummyStoreConfiguration.class, DummyInMemoryStoreConfiguration.attributeDefinitionSet());
       }
 
       NonSharedDummyStoreConfiguration(AttributeSet attributes, AsyncStoreConfiguration async, SingletonStoreConfiguration singletonStore) {

diff --git a/core/src/test/java/org/infinispan/persistence/support/BatchAsyncStoreTest.java b/core/src/test/java/org/infinispan/persistence/support/BatchAsyncStoreTest.java
@@ -46,7 +46,7 @@ private void enableTestJdbcStorage(ConfigurationBuilder configuration) throws Ex
             .passivation(false)
             .addSingleFileStore()
                .preload(false)
-               .shared(true)
+               .shared(false)
                .location(tmpDirectory)
                .async()
                   .enable()

diff --git a/...rc/test/java/org/infinispan/statetransfer/StateTransferFileCacheLoaderFunctionalTest.java b/...rc/test/java/org/infinispan/statetransfer/StateTransferFileCacheLoaderFunctionalTest.java
@@ -12,13 +12,22 @@
 import java.util.concurrent.Future;
 
 import org.infinispan.Cache;
+import org.infinispan.commons.configuration.BuiltBy;
+import org.infinispan.commons.configuration.ConfigurationFor;
+import org.infinispan.commons.configuration.attributes.AttributeSet;
+import org.infinispan.commons.persistence.Store;
 import org.infinispan.commons.util.Util;
+import org.infinispan.configuration.cache.AsyncStoreConfiguration;
 import org.infinispan.configuration.cache.CacheMode;
 import org.infinispan.configuration.cache.ConfigurationBuilder;
+import org.infinispan.configuration.cache.PersistenceConfigurationBuilder;
+import org.infinispan.configuration.cache.SingleFileStoreConfiguration;
 import org.infinispan.configuration.cache.SingleFileStoreConfigurationBuilder;
+import org.infinispan.configuration.cache.SingletonStoreConfiguration;
 import org.infinispan.context.Flag;
 import org.infinispan.manager.CacheContainer;
 import org.infinispan.manager.EmbeddedCacheManager;
+import org.infinispan.persistence.file.SingleFileStore;
 import org.infinispan.test.MultipleCacheManagersTest;
 import org.infinispan.test.TestingUtil;
 import org.infinispan.transaction.LockingMode;
@@ -86,7 +95,7 @@ protected void createCacheManagers() throws Throwable {
    protected EmbeddedCacheManager createCacheManager(String tmpDirectory) {
       configurationBuilder.persistence().clearStores();
 
-      SingleFileStoreConfigurationBuilder fcsBuilder = configurationBuilder.persistence().addSingleFileStore();
+      SharedSingleFileStoreConfigurationBulder fcsBuilder = configurationBuilder.persistence().addStore(SharedSingleFileStoreConfigurationBulder.class);
       fcsBuilder
             .fetchPersistentState(true)
             .purgeOnStartup(false)
@@ -255,4 +264,32 @@ public Void call() throws Exception {
          if (cm40 != null) cm40.stop();
       }
    }
+
+   // Create shared version of file stores so that validation does not fail, but the logic for this test still works
+   @Store(shared = true)
+   public static class SharedSingleFileStore extends SingleFileStore {}
+
+   @BuiltBy(SharedSingleFileStoreConfigurationBulder.class)
+   @ConfigurationFor(SharedSingleFileStore.class)
+   public static class SharedSingleFileStoreConfiguration extends SingleFileStoreConfiguration {
+
+      public static AttributeSet attributeDefinitionSet() {
+         return new AttributeSet(SharedSingleFileStoreConfiguration.class, SingleFileStoreConfiguration.attributeDefinitionSet());
+      }
+
+      SharedSingleFileStoreConfiguration(AttributeSet attributes, AsyncStoreConfiguration async, SingletonStoreConfiguration singletonStore) {
+         super(attributes, async, singletonStore);
+      }
+   }
+
+   public static class SharedSingleFileStoreConfigurationBulder extends SingleFileStoreConfigurationBuilder {
+      public SharedSingleFileStoreConfigurationBulder(PersistenceConfigurationBuilder builder) {
+         super(builder, SharedSingleFileStoreConfiguration.attributeDefinitionSet());
+      }
+
+      @Override
+      public SingleFileStoreConfiguration create() {
+         return new SharedSingleFileStoreConfiguration(attributes.protect(), async.create(), singletonStore.create());
+      }
+   }
 }
diff --git a/core/src/test/resources/configs/unified/7.0.xml b/core/src/test/resources/configs/unified/7.0.xml
@@ -32,7 +32,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/core/src/test/resources/configs/unified/7.1.xml b/core/src/test/resources/configs/unified/7.1.xml
@@ -32,7 +32,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/core/src/test/resources/configs/unified/7.2.xml b/core/src/test/resources/configs/unified/7.2.xml
@@ -33,7 +33,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/core/src/test/resources/configs/unified/8.0.xml b/core/src/test/resources/configs/unified/8.0.xml
@@ -45,7 +45,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>
@@ -184,7 +184,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/core/src/test/resources/configs/unified/8.1.xml b/core/src/test/resources/configs/unified/8.1.xml
@@ -49,7 +49,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>
@@ -188,7 +188,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/core/src/test/resources/configs/unified/8.2.xml b/core/src/test/resources/configs/unified/8.2.xml
@@ -50,7 +50,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>
@@ -190,7 +190,7 @@
          <eviction max-entries="20000" strategy="LIRS"/>
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind flush-lock-timeout="2" modification-queue-size="2048" shutdown-timeout="20000" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/core/src/test/resources/configs/unified/9.0.xml b/core/src/test/resources/configs/unified/9.0.xml
@@ -47,7 +47,7 @@
          <transaction mode="FULL_XA" stop-timeout="60000" locking="OPTIMISTIC" transaction-manager-lookup="org.infinispan.transaction.lookup.JBossStandaloneJTAManagerLookup" complete-timeout="34000" reaper-interval="35000" auto-commit="true" />
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind modification-queue-size="2048" thread-pool-size="1" />
             </file-store>
          </persistence>
@@ -213,7 +213,7 @@
          <transaction mode="FULL_XA" stop-timeout="60000" locking="OPTIMISTIC" transaction-manager-lookup="org.infinispan.transaction.lookup.JBossStandaloneJTAManagerLookup" complete-timeout="34000" reaper-interval="35000" auto-commit="true" />
          <expiration interval="10000" lifespan="10" max-idle="10"/>
          <persistence passivation="false">
-            <file-store path="path" relative-to="jboss.server.temp.dir" shared="true" singleton="false" fetch-state="false" preload="true" purge="false">
+            <file-store path="path" relative-to="jboss.server.temp.dir" shared="false" singleton="false" fetch-state="false" preload="true" purge="false">
                <write-behind modification-queue-size="2048" thread-pool-size="1" />
             </file-store>
          </persistence>

diff --git a/documentation/src/main/asciidoc/upgrading/upgrading.asciidoc b/documentation/src/main/asciidoc/upgrading/upgrading.asciidoc
@@ -103,8 +103,9 @@ store. See link:../user_guide/persistence_jdbc.adoc#JDBC_Migrator[JDBC Migrator
 === @Store Annotation Introduced
 A new annotation, `@Store`, has been added for persistence stores. This allows a store's properties to be
 explicitly defined and validated against the provided store configuration.  Existing stores should be updated to use this
-annotation where possible.  If no annotation is present your store will continue to function as before, albeit with a warning that
-additional store validation cannot be completed.
+annotation and the store's configuration class should also declare the `@ConfigurationFor` annotation.  If neither of these
+annotations are present on the store or configuration class, then a your store will continue to function as before, albeit
+with a warning that additional store validation cannot be completed.
 
 
 == Upgrading from 8.1 to 8.2

diff --git a/documentation/src/main/asciidoc/user_guide/persistence_custom.adoc b/documentation/src/main/asciidoc/user_guide/persistence_custom.adoc
@@ -21,7 +21,8 @@ As an optional step, you should add the following annotations to your configurat
 as adding `@ConfiguredBy` to your store implementation class.  These additional annotations will ensure that your custom
 configuration builder is used to parse your store configuration from xml. If these annotations are not added, then the
 `CustomStoreConfigurationBuilder` will be used to parse the common store attributes defined in `AbstractStoreConfiguration`
-and any additional elements will be ignored.
+and any additional elements will be ignored. If a store and its configuration do not declare the `@Store` and `@ConfigurationFor`
+annotations respectively, a warning message will be logged upon cache initialisation.
 
 
 4. Add your custom store to your cache's configuration:

diff --git a/...ce/leveldb/src/test/java/org/infinispan/persistence/leveldb/config/ConfigurationTest.java b/...ce/leveldb/src/test/java/org/infinispan/persistence/leveldb/config/ConfigurationTest.java
@@ -79,7 +79,7 @@ public void testXmlConfig() throws IOException {
 
       // check generic store attributes
       StoreConfiguration cacheLoaderConfig = cacheConfig.persistence().stores().get(0);
-      assertTrue(cacheLoaderConfig.shared());
+      assertFalse(cacheLoaderConfig.shared());
       assertTrue(cacheLoaderConfig.preload());
       assertTrue(cacheLoaderConfig instanceof LevelDBStoreConfiguration);
 

diff --git a/persistence/leveldb/src/test/resources/config/leveldb-config-auto.xml b/persistence/leveldb/src/test/resources/config/leveldb-config-auto.xml
@@ -4,7 +4,7 @@
    <cache-container>
       <local-cache name="testCache">
          <persistence passivation="false">
-            <leveldb-store path="/tmp/leveldb/52/data" shared="true" preload="true">
+            <leveldb-store path="/tmp/leveldb/52/data" shared="false" preload="true">
                <expiration path="/tmp/leveldb/52/expired"/>
                <implementation/>
             </leveldb-store>

diff --git a/persistence/leveldb/src/test/resources/config/leveldb-config-java.xml b/persistence/leveldb/src/test/resources/config/leveldb-config-java.xml
@@ -4,7 +4,7 @@
    <cache-container>
       <local-cache name="testCache">
          <persistence passivation="false">
-            <leveldb-store path="/tmp/leveldb/53/data" shared="true" preload="true">
+            <leveldb-store path="/tmp/leveldb/53/data" shared="false" preload="true">
                <expiration path="/tmp/leveldb/53/expired"/>
                <compression type="NONE" />
                <implementation type="JAVA" />