This is an automated email from the ASF dual-hosted git repository.
bbender pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new 9870644 GEODE-5626: Fix crash in register all keys (#351)
9870644 is described below
commit 9870644656ef61239244a43e65aaadc922a858c2
Author: Blake Bender <[email protected]>
AuthorDate: Mon Sep 17 07:48:15 2018 -0700
GEODE-5626: Fix crash in register all keys (#351)
* GEODE-5626: Reject getInitialValues=true in registerAllKeys when region
is not caching.
- Refactor Cluster class so we can apply locators separately.
- Clean up server directories at test startup.
- Add test cases for caching proxy and proxy regions with
getInitialValues set to true
- remove extraneous comment and empty doc comments
- throw exception if gfsh exits non-zero
- use relative path when deleting server directory
Co-authored-by: Ivan Godwin <[email protected]>
---
cppcache/integration-test-2/CMakeLists.txt | 1 +
cppcache/integration-test-2/RegisterKeysTest.cpp | 191 +++++++++++++++++++++
cppcache/integration-test-2/framework/Cluster.h | 30 ++--
.../integration-test-2/framework/GfshExecute.cpp | 3 +
.../integration-test-2/framework/GfshExecute.h | 23 ++-
cppcache/src/ThinClientRegion.cpp | 19 +-
6 files changed, 252 insertions(+), 15 deletions(-)
diff --git a/cppcache/integration-test-2/CMakeLists.txt
b/cppcache/integration-test-2/CMakeLists.txt
index 198c704..8e0233c 100644
--- a/cppcache/integration-test-2/CMakeLists.txt
+++ b/cppcache/integration-test-2/CMakeLists.txt
@@ -27,6 +27,7 @@ add_executable(integration-test-2
framework/GfshExecute.h
RegionPutGetAllTest.cpp
PdxInstanceTest.cpp
+ RegisterKeysTest.cpp
StructTest.cpp
EnableChunkHandlerThreadTest.cpp
)
diff --git a/cppcache/integration-test-2/RegisterKeysTest.cpp
b/cppcache/integration-test-2/RegisterKeysTest.cpp
new file mode 100644
index 0000000..1dc3333
--- /dev/null
+++ b/cppcache/integration-test-2/RegisterKeysTest.cpp
@@ -0,0 +1,191 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more *
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <iostream>
+
+#include <gtest/gtest.h>
+
+#include <geode/CacheFactory.hpp>
+#include <geode/Cache.hpp>
+#include <geode/PoolManager.hpp>
+#include <geode/RegionFactory.hpp>
+#include <geode/RegionShortcut.hpp>
+
+#include "framework/Framework.h"
+#include "framework/Gfsh.h"
+#include "framework/Cluster.h"
+
+namespace {
+
+using namespace apache::geode::client;
+
+apache::geode::client::Cache createTestCache() {
+ CacheFactory cacheFactory;
+ return cacheFactory.set("log-level", "none")
+ .set("statistic-sampling-enabled", "false")
+ .create();
+}
+
+std::shared_ptr<Region> setupCachingProxyRegion(Cache& cache) {
+ auto region = cache.createRegionFactory(RegionShortcut::CACHING_PROXY)
+ .setPoolName("default")
+ .create("region");
+
+ return region;
+}
+
+std::shared_ptr<Region> setupProxyRegion(Cache& cache) {
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName("default")
+ .create("region");
+
+ return region;
+}
+
+TEST(RegisterKeysTest, RegisterAllWithCachingRegion) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ {
+ auto cache = createTestCache();
+ auto poolFactory =
+ cache.getPoolManager().createFactory().setSubscriptionEnabled(true);
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+ auto region = setupCachingProxyRegion(cache);
+
+ region->put("one", std::make_shared<CacheableInt16>(1));
+ region->put("two", std::make_shared<CacheableInt16>(2));
+ region->put("three", std::make_shared<CacheableInt16>(3));
+ }
+
+ {
+ auto cache2 = createTestCache();
+ auto poolFactory =
+ cache2.getPoolManager().createFactory().setSubscriptionEnabled(true);
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+ auto region2 = setupCachingProxyRegion(cache2);
+
+ auto&& entryBefore = region2->getEntry("one");
+ ASSERT_EQ(entryBefore, nullptr);
+
+ // 2nd parameter is getInitialValues, default is false, which leads to the
+ // first update notification being passed in with a NULL "oldValue". Set
it
+ // to true here, and verify the entry retrieved is valid, i.e. the initial
+ // value was retrieved.
+ region2->registerAllKeys(false, true);
+
+ auto&& uncastedEntry = region2->getEntry("one");
+ auto&& entryAfter =
+ std::dynamic_pointer_cast<CacheableInt16>(uncastedEntry->getValue());
+ ASSERT_NE(entryAfter, nullptr);
+ ASSERT_EQ(entryAfter->value(), 1);
+ }
+}
+
+TEST(RegisterKeysTest, RegisterAnyWithCachingRegion) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ {
+ auto cache = createTestCache();
+ auto poolFactory =
+ cache.getPoolManager().createFactory().setSubscriptionEnabled(true);
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+ auto region = setupCachingProxyRegion(cache);
+
+ region->put("one", std::make_shared<CacheableInt16>(1));
+ region->put("two", std::make_shared<CacheableInt16>(2));
+ region->put("three", std::make_shared<CacheableInt16>(3));
+
+ cache.close();
+ }
+
+ {
+ auto cache2 = createTestCache();
+ auto poolFactory =
+ cache2.getPoolManager().createFactory().setSubscriptionEnabled(true);
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+ auto region2 = setupCachingProxyRegion(cache2);
+ std::vector<std::shared_ptr<CacheableKey> > keys;
+ keys.push_back(std::make_shared<CacheableString>("one"));
+
+ auto&& entryBefore = region2->getEntry("one");
+ ASSERT_EQ(entryBefore, nullptr);
+
+ region2->registerKeys(keys, false, true);
+
+ auto&& entryAfterGet = std::dynamic_pointer_cast<CacheableInt16>(
+ region2->getEntry("one")->getValue());
+ ASSERT_NE(entryAfterGet, nullptr);
+ ASSERT_EQ(entryAfterGet->value(), 1);
+ }
+}
+
+TEST(RegisterKeysTest, RegisterAllWithProxyRegion) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+ auto cache = createTestCache();
+ auto poolFactory =
+ cache.getPoolManager().createFactory().setSubscriptionEnabled(true);
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+ auto region = setupProxyRegion(cache);
+
+ EXPECT_THROW(region->registerAllKeys(false, true), IllegalStateException);
+ cache.close();
+}
+
+TEST(RegisterKeysTest, RegisterAnyWithProxyRegion) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+ auto cache = createTestCache();
+ auto poolFactory =
+ cache.getPoolManager().createFactory().setSubscriptionEnabled(true);
+ cluster.applyLocators(poolFactory);
+ poolFactory.create("default");
+ auto region = setupProxyRegion(cache);
+ std::vector<std::shared_ptr<CacheableKey> > keys;
+ keys.push_back(std::make_shared<CacheableInt16>(2));
+
+ EXPECT_THROW(region->registerKeys(keys, false, true), IllegalStateException);
+ cache.close();
+}
+
+} // namespace
diff --git a/cppcache/integration-test-2/framework/Cluster.h
b/cppcache/integration-test-2/framework/Cluster.h
index fb5871e..37be2e6 100644
--- a/cppcache/integration-test-2/framework/Cluster.h
+++ b/cppcache/integration-test-2/framework/Cluster.h
@@ -182,6 +182,7 @@ class Cluster {
initialServers_(initialServers.get()) {
jmxManagerPort_ = Framework::getAvailablePort();
+ removeServerDirectory();
start();
}
@@ -208,33 +209,38 @@ class Cluster {
void stop();
+ void removeServerDirectory() {
+ boost::filesystem::path serverDir = boost::filesystem::relative(name_);
+ boost::filesystem::remove_all(serverDir);
+ }
+
+ apache::geode::client::Cache createCache() { return createCache({}); }
apache::geode::client::Cache createCache(const
std::unordered_map<std::string, std::string>& properties) {
using apache::geode::client::CacheFactory;
CacheFactory cacheFactory;
- for (auto&& property : properties) {
+ for (auto &&property : properties) {
cacheFactory.set(property.first, property.second);
}
- auto cache = cacheFactory
- .set("log-level", "none")
- .set("statistic-sampling-enabled", "false")
- .create();
+ auto cache = cacheFactory.set("log-level", "none")
+ .set("statistic-sampling-enabled", "false")
+ .create();
auto poolFactory = cache.getPoolManager().createFactory();
- for (const auto &locator : locators_) {
- poolFactory.addLocator(locator.getAdddress().address,
- locator.getAdddress().port);
- poolFactory.create("default");
- }
+ applyLocators(poolFactory);
+ poolFactory.create("default");
return cache;
}
- apache::geode::client::Cache createCache() {
- return createCache({});
+ void applyLocators(apache::geode::client::PoolFactory &poolFactory) {
+ for (const auto &locator : locators_) {
+ poolFactory.addLocator(locator.getAdddress().address,
+ locator.getAdddress().port);
+ }
}
Gfsh &getGfsh() noexcept { return gfsh_; }
diff --git a/cppcache/integration-test-2/framework/GfshExecute.cpp
b/cppcache/integration-test-2/framework/GfshExecute.cpp
index 8a569e9..b9d8f69 100644
--- a/cppcache/integration-test-2/framework/GfshExecute.cpp
+++ b/cppcache/integration-test-2/framework/GfshExecute.cpp
@@ -67,6 +67,9 @@ void GfshExecute::execute(const std::string &command) {
auto exit_code = gfsh.exit_code();
BOOST_LOG_TRIVIAL(debug) << "Gfsh::execute: exit:" << exit_code;
+ if (exit_code) {
+ throw new GfshExecuteException("gfsh error", exit_code);
+ }
extractConnectionCommand(command);
}
diff --git a/cppcache/integration-test-2/framework/GfshExecute.h
b/cppcache/integration-test-2/framework/GfshExecute.h
index d44cccc..d812c41 100644
--- a/cppcache/integration-test-2/framework/GfshExecute.h
+++ b/cppcache/integration-test-2/framework/GfshExecute.h
@@ -20,10 +20,12 @@
#ifndef INTEGRATION_TEST_FRAMEWORK_GFSHEXECUTE_H
#define INTEGRATION_TEST_FRAMEWORK_GFSHEXECUTE_H
-#include <string>
-#include <iostream>
#include <algorithm>
+#include <iostream>
#include <regex>
+#include <string>
+
+#include <geode/Exception.hpp>
#pragma error_messages(off, oklambdaretmulti, wvarhidemem, \
w_constexprnonlitret, explctspectypename)
@@ -40,6 +42,23 @@ bool starts_with(const _T &input, const _T &match) {
std::equal(std::begin(match), std::end(match), std::begin(input));
}
+
+ class GfshExecuteException : public apache::geode::client::Exception {
+public:
+ GfshExecuteException(std::string message, int returnCode) :
+ apache::geode::client::Exception(message),
+ returnCode_(returnCode) {}
+ ~GfshExecuteException() noexcept override {}
+ std::string getName() const override {
+ return "GfshExecuteException";
+ }
+ int getGfshReturnCode() {
+ return returnCode_;
+ }
+private:
+ int returnCode_;
+};
+
class GfshExecute : public Gfsh {
public:
GfshExecute() = default;
diff --git a/cppcache/src/ThinClientRegion.cpp
b/cppcache/src/ThinClientRegion.cpp
index 74c467a..2629685 100644
--- a/cppcache/src/ThinClientRegion.cpp
+++ b/cppcache/src/ThinClientRegion.cpp
@@ -417,9 +417,18 @@ void ThinClientRegion::registerKeys(
"Durable flag only applicable for "
"durable clients");
}
+ if (getInitialValues && !m_regionAttributes.getCachingEnabled()) {
+ LOGERROR(
+ "Register keys getInitialValues flag is only applicable for caching"
+ "clients");
+ throw IllegalStateException(
+ "getInitialValues flag only applicable for caching clients");
+ }
InterestResultPolicy interestPolicy = InterestResultPolicy::NONE;
- if (getInitialValues) interestPolicy = InterestResultPolicy::KEYS_VALUES;
+ if (getInitialValues) {
+ interestPolicy = InterestResultPolicy::KEYS_VALUES;
+ }
LOGDEBUG("ThinClientRegion::registerKeys : interestpolicy is %d",
interestPolicy.ordinal);
@@ -489,6 +498,14 @@ void ThinClientRegion::registerAllKeys(bool isDurable,
bool getInitialValues,
"Durable flag only applicable for durable clients");
}
+ if (getInitialValues && !m_regionAttributes.getCachingEnabled()) {
+ LOGERROR(
+ "Register all keys getInitialValues flag is only applicable for
caching"
+ "clients");
+ throw IllegalStateException(
+ "getInitialValues flag only applicable for caching clients");
+ }
+
InterestResultPolicy interestPolicy = InterestResultPolicy::NONE;
if (getInitialValues) {
interestPolicy = InterestResultPolicy::KEYS_VALUES;
