pivotal-jbarrett commented on a change in pull request #894:
URL: https://github.com/apache/geode-native/pull/894#discussion_r747955671



##########
File path: cppcache/integration/test/RegisterKeysTest.cpp
##########
@@ -575,4 +581,257 @@ TEST(RegisterKeysTest, RegisterAnyWithProxyRegion) {
   cache.close();
 }
 
+apache::geode::client::Cache createCache() {
+  return apache::geode::client::CacheFactory()
+      .set("log-level", "none")
+      .set("statistic-sampling-enabled", "false")
+      .create();
+}
+
+std::shared_ptr<apache::geode::client::Pool> createPool(
+    Cluster& cluster, apache::geode::client::Cache& cache) {
+  auto poolFactory = cache.getPoolManager().createFactory();
+  cluster.applyLocators(poolFactory);
+  poolFactory.setSubscriptionEnabled(true);  // Per the customer.
+  return poolFactory.create("default");
+}
+
+std::shared_ptr<apache::geode::client::Region> setupRegion(
+    apache::geode::client::Cache& cache,
+    const std::shared_ptr<apache::geode::client::Pool>& pool) {
+  auto region =
+      cache
+          .createRegionFactory(apache::geode::client::RegionShortcut::
+                                   CACHING_PROXY)  // Per the customer.
+          .setPoolName(pool->getName())
+          .create("region");
+
+  return region;
+}
+
+TEST(RegisterKeysTest, DontReceiveValues) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+  cluster.start();
+
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("PARTITION")
+      .execute();
+
+  auto cache1 = createCache();
+  auto pool1 = createPool(cluster, cache1);
+  auto region1 = setupRegion(cache1, pool1);
+  auto attrMutator = region1->getAttributesMutator();
+
+  // put key/value pairs in the region using cache2

Review comment:
       If a block of code needs a comment it should be refactored to be 
readable or extracted into a method with a descriptive name. This test is 
getting pretty long.

##########
File path: cppcache/integration/test/RegisterKeysTest.cpp
##########
@@ -575,4 +581,257 @@ TEST(RegisterKeysTest, RegisterAnyWithProxyRegion) {
   cache.close();
 }
 
+apache::geode::client::Cache createCache() {
+  return apache::geode::client::CacheFactory()
+      .set("log-level", "none")
+      .set("statistic-sampling-enabled", "false")
+      .create();
+}
+
+std::shared_ptr<apache::geode::client::Pool> createPool(
+    Cluster& cluster, apache::geode::client::Cache& cache) {
+  auto poolFactory = cache.getPoolManager().createFactory();
+  cluster.applyLocators(poolFactory);
+  poolFactory.setSubscriptionEnabled(true);  // Per the customer.
+  return poolFactory.create("default");
+}
+
+std::shared_ptr<apache::geode::client::Region> setupRegion(
+    apache::geode::client::Cache& cache,
+    const std::shared_ptr<apache::geode::client::Pool>& pool) {
+  auto region =
+      cache
+          .createRegionFactory(apache::geode::client::RegionShortcut::
+                                   CACHING_PROXY)  // Per the customer.
+          .setPoolName(pool->getName())
+          .create("region");
+
+  return region;
+}
+
+TEST(RegisterKeysTest, DontReceiveValues) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+  cluster.start();
+
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("PARTITION")
+      .execute();
+
+  auto cache1 = createCache();
+  auto pool1 = createPool(cluster, cache1);
+  auto region1 = setupRegion(cache1, pool1);
+  auto attrMutator = region1->getAttributesMutator();
+
+  // put key/value pairs in the region using cache2
+
+  auto cache2 = createCache();
+  auto pool2 = createPool(cluster, cache2);
+  auto region2 = setupRegion(cache2, pool2);
+
+  const int NUMKEYS = 100;
+  for (int i = 0; i < NUMKEYS; i++) {
+    region2->put(apache::geode::client::CacheableInt32::create(i),
+                 apache::geode::client::CacheableInt32::create(i));
+  }
+
+  // register for all keys, but don't get any data
+
+  region1->registerAllKeys(false, false, false);
+
+  // verify we didn't get any data
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    auto hasKey =
+        region1->containsKey(apache::geode::client::CacheableInt32::create(i));
+    EXPECT_FALSE(hasKey);
+
+    auto hasValue = region1->containsValueForKey(
+        apache::geode::client::CacheableInt32::create(i));
+    EXPECT_FALSE(hasValue);
+  }
+
+  // load cache1 with data
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    auto value = 
region1->get(apache::geode::client::CacheableInt32::create(i));
+  }
+
+  // put new values in the region using cache2
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    region2->put(apache::geode::client::CacheableInt32::create(i),
+                 apache::geode::client::CacheableInt32::create(i + 1000));
+  }
+
+  // wait for cache1 invalidates due to receiveValues=false, and verify
+
+  std::this_thread::sleep_for(5000ms);
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    auto hasKey =
+        region1->containsKey(apache::geode::client::CacheableInt32::create(i));
+    EXPECT_TRUE(hasKey);
+
+    auto hasValue = region1->containsValueForKey(
+        apache::geode::client::CacheableInt32::create(i));
+    EXPECT_FALSE(hasValue);
+  }
+}
+
+TEST(RegisterKeysTest, ReceiveValuesLocalInvalidate) {
+  Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+  cluster.start();
+
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("PARTITION")
+      .execute();
+
+  auto cache1 = createCache();
+  auto pool1 = createPool(cluster, cache1);
+  auto region1 = setupRegion(cache1, pool1);
+  auto attrMutator = region1->getAttributesMutator();
+
+  // put key/value pairs in the region using cache2
+
+  auto cache2 = createCache();
+  auto pool2 = createPool(cluster, cache2);
+  auto region2 = setupRegion(cache2, pool2);
+
+  const int NUMKEYS = 100;
+  for (int i = 0; i < NUMKEYS; i++) {
+    region2->put(apache::geode::client::CacheableInt32::create(i),
+                 apache::geode::client::CacheableInt32::create(i));
+  }
+
+  // register for all keys and get data now and future updates
+
+  region1->registerAllKeys(false, true, true);
+
+  // verify we now have data in cache1
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    auto hasKey =
+        region1->containsKey(apache::geode::client::CacheableInt32::create(i));
+    EXPECT_TRUE(hasKey);
+
+    auto hasValue = region1->containsValueForKey(
+        apache::geode::client::CacheableInt32::create(i));
+    EXPECT_TRUE(hasValue);
+  }
+
+  // invalidate data in cache1
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    region1->localInvalidate(apache::geode::client::CacheableInt32::create(i));
+  }
+
+  // verify cache1 data no longer valid
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    auto hasKey =
+        region1->containsKey(apache::geode::client::CacheableInt32::create(i));
+    EXPECT_TRUE(hasKey);
+
+    auto hasValue = region1->containsValueForKey(
+        apache::geode::client::CacheableInt32::create(i));
+    EXPECT_FALSE(hasValue);
+  }
+
+  // put new values in the region using cache2
+
+  for (int i = 0; i < NUMKEYS; i++) {
+    region2->put(apache::geode::client::CacheableInt32::create(i),
+                 apache::geode::client::CacheableInt32::create(i + 2000));
+  }
+
+  // wait for the new values in cache1 due to receiveValues = true, and verify
+
+  std::this_thread::sleep_for(5000ms);

Review comment:
       A test should not have sleeps. Updates arriving at the other cache 
should trigger a cache listener. You can use that trigger to release this 
thread to expect the values. Alternatively this thread can await/loop until the 
assertions are true or a timeout expires.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to