IGNITE-17016 Fixed an issue that could lead to skipping 3rd party cac…

…he store while executing REST API GET command. Fixes apache#10525
QPC-github · Feb 16, 2023 · 4496972 · 4496972
1 parent 4284a50
commit 4496972
Show file tree

Hide file tree

Showing 2 changed files with 127 additions and 1 deletion.
diff --git a/...t/java/org/apache/ignite/internal/processors/rest/JettyRestProcessorAbstractSelfTest.java b/...t/java/org/apache/ignite/internal/processors/rest/JettyRestProcessorAbstractSelfTest.java
@@ -40,6 +40,10 @@
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
+import javax.cache.Cache;
+import javax.cache.configuration.FactoryBuilder;
+import javax.cache.integration.CacheLoaderException;
+import javax.cache.integration.CacheWriterException;
 import com.fasterxml.jackson.annotation.JsonInclude;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.JsonNode;
@@ -53,6 +57,7 @@
 import org.apache.ignite.cache.query.SqlFieldsQuery;
 import org.apache.ignite.cache.query.SqlQuery;
 import org.apache.ignite.cache.query.annotations.QuerySqlField;
+import org.apache.ignite.cache.store.CacheStoreAdapter;
 import org.apache.ignite.cluster.ClusterNode;
 import org.apache.ignite.cluster.ClusterState;
 import org.apache.ignite.configuration.CacheConfiguration;
@@ -107,6 +112,12 @@ public abstract class JettyRestProcessorAbstractSelfTest extends JettyRestProces
     /** */
     private static boolean memoryMetricsEnabled;
 
+    /** */
+    protected static final String CASHE_STORE_ENABLED_CACHE_NAME = "cache-store-enabled-cache";
+
+    /** */
+    protected static volatile Map<String, String> thirdPartyStore;
+
     /** */
     @Parameterized.Parameter
     public boolean useBinaryArrays;
@@ -123,6 +134,8 @@ public static Iterable<Object[]> useBinaryArrays() {
         System.setProperty(IGNITE_MARSHALLER_BLACKLIST, path);
         System.setProperty(IGNITE_USE_BINARY_ARRAYS, Boolean.toString(useBinaryArrays));
 
+        thirdPartyStore = new ConcurrentHashMap<>();
+
         super.beforeTestsStarted();
 
         initCache();
@@ -134,6 +147,8 @@ public static Iterable<Object[]> useBinaryArrays() {
         System.clearProperty(IGNITE_USE_BINARY_ARRAYS);
 
         super.afterTestsStopped();
+
+        thirdPartyStore = null;
     }
 
     /** {@inheritDoc} */
@@ -143,6 +158,7 @@ public static Iterable<Object[]> useBinaryArrays() {
         grid(0).cluster().state(ACTIVE);
 
         grid(0).cache(DEFAULT_CACHE_NAME).removeAll();
+        grid(0).cache(CASHE_STORE_ENABLED_CACHE_NAME).removeAll();
 
         if (memoryMetricsEnabled) {
             memoryMetricsEnabled = false;
@@ -319,6 +335,43 @@ public void testGet() throws Exception {
         assertCacheOperation(ret, "getVal");
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testGetSkipStoreStore() throws Exception {
+        String key = "skipStoreTestKey";
+        String val = "skipStoreTestValue";
+
+        assertNull(
+                "The value should be empty before the test.",
+                grid(0).cache(CASHE_STORE_ENABLED_CACHE_NAME).get(key));
+
+        thirdPartyStore.put(key, val);
+
+        String skipStoreRet = content(
+                CASHE_STORE_ENABLED_CACHE_NAME,
+                GridRestCommand.CACHE_GET,
+                "key",
+                key,
+                "cacheFlags",
+                "1");
+
+        info("Get command result: " + skipStoreRet);
+
+        assertCacheOperation(skipStoreRet, null);
+
+        String ret = content(
+                CASHE_STORE_ENABLED_CACHE_NAME,
+                GridRestCommand.CACHE_GET,
+                "key",
+                key);
+
+        info("Get command result: " + ret);
+
+        assertCacheOperation(ret, val);
+    }
+
     /**
      * @param json JSON to check.
      * @param p Person to compare with.
@@ -1299,6 +1352,46 @@ public void testPut() throws Exception {
         assertCacheOperation(ret, true);
     }
 
+    /**
+     * @throws Exception If failed.
+     */
+    @Test
+    public void testPutSkipStore() throws Exception {
+        String key = "testPutSkipStoreKey";
+        String val1 = "testPutSkipStoreValue1";
+        String val2 = "testPutSkipStoreValue2";
+
+        String skipStoreRet = content(
+                CASHE_STORE_ENABLED_CACHE_NAME,
+                GridRestCommand.CACHE_PUT,
+                "key",
+                key,
+                "val",
+                val1,
+                "cacheFlags",
+                "1");
+
+        info("Put command result: " + skipStoreRet);
+
+        assertCacheOperation(skipStoreRet, true);
+
+        assertNull("Third party cache store should be skipped.", thirdPartyStore.get(key));
+
+        String ret = content(
+                CASHE_STORE_ENABLED_CACHE_NAME,
+                GridRestCommand.CACHE_PUT,
+                "key",
+                key,
+                "val",
+                val2);
+
+        info("Put command result: " + ret);
+
+        assertCacheOperation(ret, true);
+
+        assertEquals("Third party cache store should not be skipped.", val2, thirdPartyStore.get(key));
+    }
+
     /**
      * @throws Exception If failed.
      */
@@ -3531,6 +3624,21 @@ private static class NodeIdFilter implements IgnitePredicate<ClusterNode> {
 
         cfg.setDataStorageConfiguration(dsCfg);
 
+        int sz = cfg.getCacheConfiguration().length;
+
+        CacheConfiguration[] cacheCfgs = Arrays.copyOf(cfg.getCacheConfiguration(), sz + 1);
+
+        CacheConfiguration<String, String> cacheCfg = new CacheConfiguration<>(CASHE_STORE_ENABLED_CACHE_NAME);
+        cacheCfg.setCopyOnRead(false)
+                .setCacheMode(CacheMode.REPLICATED)
+                .setReadThrough(true)
+                .setWriteThrough(true)
+                .setCacheStoreFactory(FactoryBuilder.factoryOf(JettyRestProcessorUnsignedSelfTest.TestStore.class));
+
+        cacheCfgs[sz] = cacheCfg;
+
+        cfg.setCacheConfiguration(cacheCfgs);
+
         return cfg;
     }
 
@@ -3621,4 +3729,22 @@ private void changeClusterState(
 
         checkState(success ? newState : curState);
     }
+
+    /** Test 3rd party cache store. */
+    public static class TestStore extends CacheStoreAdapter<String, String> {
+        /** {@inheritDoc} */
+        @Override public String load(String key) throws CacheLoaderException {
+            return thirdPartyStore.get(key);
+        }
+
+        /** {@inheritDoc} */
+        @Override public void write(Cache.Entry<? extends String, ? extends String> entry) throws CacheWriterException {
+            thirdPartyStore.put(entry.getKey(), entry.getValue());
+        }
+
+        /** {@inheritDoc} */
+        @Override public void delete(Object key) throws CacheWriterException {
+            thirdPartyStore.remove(key);
+        }
+    }
 }
diff --git a/...es/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java b/...es/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheProxyImpl.java
@@ -263,7 +263,7 @@ public IgniteInternalCache<K, V> delegate() {
             return new GridCacheProxyImpl<>(ctx, delegate,
                 opCtx != null ? opCtx.setSkipStore(skipStore) :
                     new CacheOperationContext(
-                        true,
+                        skipStore,
                         false,
                         null,
                         false,