ISPN-11845 SingleFileStore location validation should be skipped when…

… global state is disabled
infinispan · Jun 9, 2020 · 5ff4581 · 5ff4581
1 parent 2602136
commit 5ff4581
Show file tree

Hide file tree

Showing 20 changed files with 365 additions and 40 deletions.
diff --git a/...est/java/org/infinispan/client/hotrod/query/HotRodNonIndexedSingleFileStoreQueryTest.java b/...est/java/org/infinispan/client/hotrod/query/HotRodNonIndexedSingleFileStoreQueryTest.java
@@ -39,7 +39,9 @@ protected void configure(GlobalConfigurationBuilder builder) {
    @Override
    protected ConfigurationBuilder getConfigurationBuilder() {
       ConfigurationBuilder builder = new ConfigurationBuilder();
-      builder.persistence().addStore(SingleFileStoreConfigurationBuilder.class);
+      builder.persistence()
+            .addStore(SingleFileStoreConfigurationBuilder.class)
+            .location(tmpDirectory);
 
       // ensure the data container contains minimal data so the store will need to be accessed to get the rest
       builder.locking().concurrencyLevel(1).memory().size(1);

diff --git a/...src/main/java/org/infinispan/configuration/cache/SingleFileStoreConfigurationBuilder.java b/...src/main/java/org/infinispan/configuration/cache/SingleFileStoreConfigurationBuilder.java
@@ -11,6 +11,7 @@
 import org.infinispan.commons.configuration.attributes.AttributeSet;
 import org.infinispan.commons.configuration.elements.ElementDefinition;
 import org.infinispan.configuration.global.GlobalConfiguration;
+import org.infinispan.persistence.PersistenceUtil;
 import org.infinispan.persistence.file.SingleFileStore;
 import org.infinispan.util.logging.Log;
 
@@ -103,6 +104,7 @@ public void validate() {
 
    @Override
    public void validate(GlobalConfiguration globalConfig) {
+      PersistenceUtil.validateGlobalStateStoreLocation(globalConfig, SingleFileStore.class.getSimpleName(), attributes.attribute(LOCATION));
       super.validate(globalConfig);
    }
 

diff --git a/core/src/main/java/org/infinispan/persistence/PersistenceUtil.java b/core/src/main/java/org/infinispan/persistence/PersistenceUtil.java
@@ -13,9 +13,11 @@
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
+import org.infinispan.commons.configuration.attributes.Attribute;
 import org.infinispan.commons.time.TimeService;
 import org.infinispan.commons.util.IntSet;
 import org.infinispan.configuration.global.GlobalConfiguration;
+import org.infinispan.configuration.global.GlobalStateConfiguration;
 import org.infinispan.container.DataContainer;
 import org.infinispan.container.entries.InternalCacheEntry;
 import org.infinispan.container.impl.InternalEntryFactory;
@@ -25,6 +27,7 @@
 import org.infinispan.persistence.spi.MarshallableEntry;
 import org.infinispan.persistence.spi.NonBlockingStore;
 import org.infinispan.persistence.spi.SegmentedAdvancedLoadWriteStore;
+import org.infinispan.util.logging.Log;
 import org.reactivestreams.Publisher;
 
 import io.reactivex.rxjava3.core.Flowable;
@@ -171,11 +174,20 @@ public static Path getQualifiedLocation(GlobalConfiguration globalConfiguration,
    }
 
    public static Path getLocation(GlobalConfiguration globalConfiguration, String location) {
-      Path persistentLocation = Paths.get(globalConfiguration.globalState().persistentLocation());
-      if (location == null)
-         return persistentLocation;
+      GlobalStateConfiguration globalState = globalConfiguration.globalState();
+      Path persistentLocation = Paths.get(globalState.persistentLocation());
+      if (location == null) {
+          if (!globalState.enabled()) {
+             // Should never be reached as store builders should ensure that the locations are not null during validation.
+             throw PERSISTENCE.storeLocationRequired();
+          }
+          return persistentLocation;
+      }
 
       Path path = Paths.get(location);
+      if (!globalState.enabled()) {
+          return path;
+      }
       if (path.isAbsolute()) {
          // Ensure that the path lives under the global persistent location
          if (path.startsWith(persistentLocation)) {
@@ -186,4 +198,12 @@ public static Path getLocation(GlobalConfiguration globalConfiguration, String l
       }
       return persistentLocation.resolve(path);
    }
+
+   public static void validateGlobalStateStoreLocation(GlobalConfiguration globalConfiguration, String storeType, Attribute<?>... attributes) {
+      if (!globalConfiguration.globalState().enabled()) {
+         for (Attribute<?> attr : attributes)
+            if (attr.isNull())
+               throw Log.CONFIG.storeLocationRequired(storeType, attr.name());
+      }
+   }
 }
diff --git a/core/src/main/java/org/infinispan/util/logging/Log.java b/core/src/main/java/org/infinispan/util/logging/Log.java
@@ -2026,4 +2026,10 @@ CacheConfigurationException storeConfiguredHasBothReadAndWriteOnly(String storeC
 
    @Message(value = "Store %s cannot be configured to be transactional as it does not contain the TRANSACTIONAL characteristic", id = 597)
    CacheConfigurationException storeConfiguredTransactionalButCharacteristicNotPresent(String storeClassName);
+
+   @Message(value = "Store must specify a location when global state is disabled", id = 598)
+   CacheConfigurationException storeLocationRequired();
+
+   @Message(value = "Store '%s' must specify the '%s' attribute when global state is disabled", id = 598)
+   CacheConfigurationException storeLocationRequired(String storeType, String attributeName);
 }
diff --git a/core/src/test/java/org/infinispan/eviction/impl/EvictionWithConcurrentOperationsTest.java b/core/src/test/java/org/infinispan/eviction/impl/EvictionWithConcurrentOperationsTest.java
@@ -513,7 +513,7 @@ protected EmbeddedCacheManager createCacheManager() throws Exception {
       configureEviction(builder);
       GlobalConfigurationBuilder globalBuilder = new GlobalConfigurationBuilder().nonClusteredDefault();
       globalBuilder.serialization().addContextInitializer(new EvictionWithConcurrentOperationsSCIImpl());
-      globalBuilder.globalState().persistentLocation(PERSISTENT_LOCATION);
+      globalBuilder.globalState().enable().persistentLocation(PERSISTENT_LOCATION);
       return TestCacheManagerFactory.createCacheManager(globalBuilder, builder);
    }
 

diff --git a/...a/org/infinispan/expiration/impl/ExpirationSingleFileStoreDistListenerFunctionalTest.java b/...a/org/infinispan/expiration/impl/ExpirationSingleFileStoreDistListenerFunctionalTest.java
@@ -78,7 +78,7 @@ protected EmbeddedCacheManager createCacheManager(ConfigurationBuilder builder)
       GlobalConfigurationBuilder globalBuilder = GlobalConfigurationBuilder.defaultClusteredBuilder();
 
       // Make sure each cache writes to a different location
-      globalBuilder.globalState().persistentLocation(EXTRA_MANAGER_LOCATION);
+      globalBuilder.globalState().enable().persistentLocation(EXTRA_MANAGER_LOCATION);
 
       extraManager = createClusteredCacheManager(false, globalBuilder, builder, new TransportFlags());
       // Inject our time service into the new CacheManager as well
@@ -87,7 +87,7 @@ protected EmbeddedCacheManager createCacheManager(ConfigurationBuilder builder)
 
       globalBuilder = GlobalConfigurationBuilder.defaultClusteredBuilder();
       // Make sure each cache writes to a different location
-      globalBuilder.globalState().persistentLocation(PERSISTENT_LOCATION);
+      globalBuilder.globalState().enable().persistentLocation(PERSISTENT_LOCATION);
       EmbeddedCacheManager returned = createClusteredCacheManager(false, globalBuilder, builder, new TransportFlags());
 
       // Unfortunately we can't reinject timeservice once a cache has been started, thus we have to inject

diff --git a/core/src/test/java/org/infinispan/persistence/MultiStoresFunctionalTest.java b/core/src/test/java/org/infinispan/persistence/MultiStoresFunctionalTest.java
@@ -9,12 +9,10 @@
 import java.util.List;
 
 import org.infinispan.Cache;
-import org.infinispan.commons.test.CommonsTestingUtil;
 import org.infinispan.configuration.cache.AbstractStoreConfigurationBuilder;
 import org.infinispan.configuration.cache.CacheMode;
 import org.infinispan.configuration.cache.ConfigurationBuilder;
 import org.infinispan.configuration.cache.PersistenceConfigurationBuilder;
-import org.infinispan.configuration.global.GlobalConfigurationBuilder;
 import org.infinispan.manager.EmbeddedCacheManager;
 import org.infinispan.test.AbstractInfinispanTest;
 import org.infinispan.test.CacheManagerCallable;
@@ -51,20 +49,14 @@ protected List<ConfigurationBuilder> configs(int n, Method method) throws Except
       return r;
    }
 
-   private GlobalConfigurationBuilder globalConfig() {
-      GlobalConfigurationBuilder global = GlobalConfigurationBuilder.defaultClusteredBuilder();
-      global.globalState().persistentLocation(CommonsTestingUtil.tmpDirectory(this.getClass()));
-      return global;
-   }
-
    /**
     * when a node that persisted KEY wakes up, it can't rewrite existing value.
     */
    public void testStartStopOfBackupDoesntRewriteValue(Method m) throws Exception {
       final List<ConfigurationBuilder> configs = configs(2, m);
       withCacheManagers(new MultiCacheManagerCallable(
-            TestCacheManagerFactory.createClusteredCacheManager(globalConfig(), configs.get(0)),
-            TestCacheManagerFactory.createClusteredCacheManager(globalConfig(), configs.get(1))) {
+            TestCacheManagerFactory.createClusteredCacheManager(configs.get(0)),
+            TestCacheManagerFactory.createClusteredCacheManager(configs.get(1))) {
          @Override
          public void call() {
             final EmbeddedCacheManager cacheManager0 = cms[0];
@@ -83,7 +75,7 @@ public void call() {
             assertEquals("VALUE V2", cache0.get("KEY"));
 
             withCacheManager(new CacheManagerCallable(
-                  TestCacheManagerFactory.createClusteredCacheManager(globalConfig(), configs.get(1))) {
+                  TestCacheManagerFactory.createClusteredCacheManager(configs.get(1))) {
                @Override
                public void call() {
                   Cache<String, String> newCache = cm.getCache();
@@ -101,8 +93,8 @@ public void call() {
    public void testStartStopOfBackupResurrectsDeletedKey(Method m) throws Exception {
       final List<ConfigurationBuilder> configs = configs(2, m);
       withCacheManagers(new MultiCacheManagerCallable(
-            TestCacheManagerFactory.createClusteredCacheManager(globalConfig(), configs.get(0)),
-            TestCacheManagerFactory.createClusteredCacheManager(globalConfig(), configs.get(1))) {
+            TestCacheManagerFactory.createClusteredCacheManager(configs.get(0)),
+            TestCacheManagerFactory.createClusteredCacheManager(configs.get(1))) {
          @Override
          public void call() {
             final EmbeddedCacheManager cacheManager0 = cms[0];
@@ -123,7 +115,7 @@ public void call() {
             assertEquals(null, cache0.get("KEY2"));
 
             withCacheManager(new CacheManagerCallable(
-                  TestCacheManagerFactory.createClusteredCacheManager(globalConfig(), configs.get(1))) {
+                  TestCacheManagerFactory.createClusteredCacheManager(configs.get(1))) {
                @Override
                public void call() {
                   Cache<String, String> newCache = cm.getCache();
@@ -134,5 +126,4 @@ public void call() {
          }
       });
    }
-
 }
diff --git a/core/src/test/java/org/infinispan/persistence/file/SingleFileStoreStressTest.java b/core/src/test/java/org/infinispan/persistence/file/SingleFileStoreStressTest.java
@@ -56,7 +56,7 @@ protected void teardown() {
    protected EmbeddedCacheManager createCacheManager() throws Exception {
       location = CommonsTestingUtil.tmpDirectory(SingleFileStoreStressTest.class);
       GlobalConfigurationBuilder globalBuilder = new GlobalConfigurationBuilder().nonClusteredDefault();
-      globalBuilder.globalState().persistentLocation(location);
+      globalBuilder.globalState().enable().persistentLocation(location);
 
       ConfigurationBuilder builder = new ConfigurationBuilder();
       builder.persistence().addSingleFileStore().purgeOnStartup(true).segmented(false);

diff --git a/...test/java/org/infinispan/persistence/support/AsyncStoreWithoutEvictionFunctionalTest.java b/...test/java/org/infinispan/persistence/support/AsyncStoreWithoutEvictionFunctionalTest.java
@@ -65,7 +65,7 @@ private EmbeddedCacheManager configureCacheManager() {
 
       GlobalConfigurationBuilder glob = new GlobalConfigurationBuilder().nonClusteredDefault()
             .defaultCacheName("cache");
-      glob.globalState().persistentLocation(tmpDirectory);
+      glob.globalState().enable().persistentLocation(tmpDirectory);
       ConfigurationBuilder cacheb = new ConfigurationBuilder();
       cacheb.clustering().cacheMode(CacheMode.LOCAL)
             .transaction().transactionMode(TransactionMode.NON_TRANSACTIONAL)

diff --git a/core/src/test/java/org/infinispan/persistence/support/BatchAsyncStoreTest.java b/core/src/test/java/org/infinispan/persistence/support/BatchAsyncStoreTest.java
@@ -35,7 +35,7 @@ public BatchAsyncStoreTest() {
    @Override
    protected EmbeddedCacheManager createCacheManager() throws Exception {
       GlobalConfigurationBuilder globalBuilder = new GlobalConfigurationBuilder().nonClusteredDefault();
-      globalBuilder.globalState().persistentLocation(tmpDirectory);
+      globalBuilder.globalState().enable().persistentLocation(tmpDirectory);
 
       ConfigurationBuilder configuration = new ConfigurationBuilder();
       configuration.invocationBatching().enable();

diff --git a/core/src/test/java/org/infinispan/statetransfer/ReplStateTransferCacheLoaderTest.java b/core/src/test/java/org/infinispan/statetransfer/ReplStateTransferCacheLoaderTest.java
@@ -58,7 +58,7 @@ protected void createCacheManagers() {
             .memory().storageType(StorageType.BINARY)
             .clustering().remoteTimeout(20000)
             .stateTransfer().timeout(240000).fetchInMemoryState(false).chunkSize(10000)
-            .persistence().addSingleFileStore().location("store0");
+            .persistence().addSingleFileStore().location(new File(tmpDir, "store0").getAbsolutePath());
 
       createCluster(globalBuilder, builder, 1);
       waitForClusterToForm();
@@ -82,7 +82,7 @@ public void testStateTransfer() {
 
       log.info("Adding a new node ..");
       // make sure this node writes in a different location
-      builder.persistence().clearStores().addSingleFileStore().location("store1").fetchPersistentState(true);
+      builder.persistence().clearStores().addSingleFileStore().location(new File(tmpDir, "store0").getAbsolutePath()).fetchPersistentState(true);
 
       addClusterEnabledCacheManager(globalBuilder, builder);
       log.info("Added a new node");

diff --git a/core/src/test/resources/configs/named-cache-test.xml b/core/src/test/resources/configs/named-cache-test.xml
@@ -78,7 +78,7 @@
       </local-cache>
       <local-cache name="withEmptyStore">
          <persistence>
-            <file-store/>
+            <file-store path="/tmp/FileCacheStore-Location"/>
          </persistence>
       </local-cache>
       <local-cache name="withClusterLoader">

diff --git a/documentation/src/main/asciidoc/topics/proc_setting_persistent_location.adoc b/documentation/src/main/asciidoc/topics/proc_setting_persistent_location.adoc
@@ -12,7 +12,7 @@ share data between multiple instances, use a shared persistent location.
 {brandname} servers use the `{server_home}/server/data` directory as the global
 persistent location.
 
-If you are using {brandname} as a library embedded in custom applications, the
+If you are using {brandname} as a library embedded in custom applications and global-state is enabled, the
 global persistent location defaults to the `user.dir` system property. This
 system property typically uses the directory where your application starts. You
 should configure a global persistent location to use a suitable location.
@@ -31,10 +31,7 @@ should configure a global persistent location to use a suitable location.
 
 [source,java,options="nowrap",subs=attributes+]
 ----
-public GlobalStateConfigurationBuilder persistentLocation(String path, String relativeTo) {
-   persistentLocation.location(example, my.data);
-   return this;
-}
+new GlobalConfigurationBuilder().globalState().enable().persistentLocation("example", "my.data");
 ----
 
 .File-Based Cache Stores and Global Persistent Location
@@ -63,7 +60,7 @@ You then configure a Single File cache store that uses a path named
 In this case, the configuration results in a Single File cache store in `/tmp/example/myDataStore/myCache.dat`
 
 If you attempt to set an absolute path that resides outside the global
-persistent location, {brandname} throws the following exception:
+persistent location and global-state is enabled, {brandname} throws the following exception:
 
 ----
 ISPN000558: "The store location 'foo' is not a child of the global persistent location 'bar'"