You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by mm...@apache.org on 2021/05/14 17:03:16 UTC
[geode-native] branch develop updated: GEODE-9249: remove(key,
value) bug
This is an automated email from the ASF dual-hosted git repository.
mmartell 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 86c0046 GEODE-9249: remove(key, value) bug
86c0046 is described below
commit 86c00466257120e1cf3eb042ede866a3828e7626
Author: Michael Martell <mm...@pivotal.io>
AuthorDate: Fri May 14 10:03:04 2021 -0700
GEODE-9249: remove(key, value) bug
* Added a new CACHING_PROXY test suite
* Fix the bug in TcrMessageDestroy
* Add isUserNullValue to TcrMessageDestroy CTOR, to allow both types of Destroy messages
* Add support for creating a Cacheable from a nullptr
---
cppcache/include/geode/CacheableBuiltins.hpp | 5 +
cppcache/integration/test/CMakeLists.txt | 1 +
cppcache/integration/test/CachingProxyTest.cpp | 174 +++++++++++++++++++++++++
cppcache/src/TcrMessage.cpp | 4 +-
cppcache/src/TcrMessage.hpp | 1 +
cppcache/src/ThinClientRegion.cpp | 8 +-
cppcache/test/TcrMessageTest.cpp | 8 +-
7 files changed, 191 insertions(+), 10 deletions(-)
diff --git a/cppcache/include/geode/CacheableBuiltins.hpp b/cppcache/include/geode/CacheableBuiltins.hpp
index ec81932..206fdd4 100644
--- a/cppcache/include/geode/CacheableBuiltins.hpp
+++ b/cppcache/include/geode/CacheableBuiltins.hpp
@@ -55,6 +55,11 @@ inline std::shared_ptr<Cacheable> Serializable::create(bool value) {
return CacheableBoolean::create(value);
}
+template <>
+inline std::shared_ptr<Cacheable> Serializable::create(std::nullptr_t) {
+ return static_cast<std::shared_ptr<Cacheable>>(nullptr);
+}
+
/**
* An immutable wrapper for byte that can serve as
* a distributable key object for caching.
diff --git a/cppcache/integration/test/CMakeLists.txt b/cppcache/integration/test/CMakeLists.txt
index d3363e5..62f0391 100644
--- a/cppcache/integration/test/CMakeLists.txt
+++ b/cppcache/integration/test/CMakeLists.txt
@@ -17,6 +17,7 @@ add_executable(cpp-integration-test
AuthInitializeTest.cpp
BasicIPv6Test.cpp
CacheWriterTest.cpp
+ CachingProxyTest.cpp
CacheXmlTest.cpp
CleanIdleConnections.cpp
ClientTransactionXATest.cpp
diff --git a/cppcache/integration/test/CachingProxyTest.cpp b/cppcache/integration/test/CachingProxyTest.cpp
new file mode 100644
index 0000000..f6f0d83
--- /dev/null
+++ b/cppcache/integration/test/CachingProxyTest.cpp
@@ -0,0 +1,174 @@
+/*
+ * 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 <list>
+#include <thread>
+
+#include <gtest/gtest.h>
+
+#include <geode/Cache.hpp>
+#include <geode/CacheFactory.hpp>
+#include <geode/RegionFactory.hpp>
+#include <geode/RegionShortcut.hpp>
+#include <geode/Serializable.hpp>
+
+#include "framework/Cluster.h"
+
+using apache::geode::client::Cache;
+using apache::geode::client::Cacheable;
+using apache::geode::client::CacheableString;
+using apache::geode::client::CacheFactory;
+using apache::geode::client::Region;
+using apache::geode::client::RegionShortcut;
+using apache::geode::client::Serializable;
+
+TEST(CachingProxyTest, LocalRemoveAfterLocalInvalidate) {
+ Cluster cluster = Cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ auto cache = CacheFactory().create();
+
+ cache.getPoolManager()
+ .createFactory()
+ .addLocator("localhost", cluster.getLocatorPort())
+ .create("pool");
+
+ std::string key = std::string("scharles");
+ std::string value = std::string("Sylvia Charles");
+
+ auto region = cache.createRegionFactory(RegionShortcut::CACHING_PROXY)
+ .setPoolName("pool")
+ .create("region");
+
+ region->put(key, value);
+
+ auto user = region->get(key);
+
+ region->localInvalidate(key);
+
+ auto resultLocalRemove = region->localRemove(key, value);
+ ASSERT_FALSE(resultLocalRemove);
+
+ resultLocalRemove = region->localRemove(key, nullptr);
+ ASSERT_TRUE(resultLocalRemove);
+}
+
+TEST(CachingProxyTest, RemoveAfterInvalidate) {
+ Cluster cluster = Cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ auto cache = CacheFactory().create();
+
+ cache.getPoolManager()
+ .createFactory()
+ .addLocator("localhost", cluster.getLocatorPort())
+ .create("pool");
+
+ std::string key = std::string("scharles");
+ std::string value = std::string("Sylvia Charles");
+
+ auto region = cache.createRegionFactory(RegionShortcut::CACHING_PROXY)
+ .setPoolName("pool")
+ .create("region");
+
+ region->put(key, value);
+
+ region->invalidate(key);
+
+ auto resultRemove = region->remove(key, value);
+ ASSERT_FALSE(resultRemove);
+
+ resultRemove = region->remove(key, nullptr);
+ ASSERT_TRUE(resultRemove);
+}
+
+TEST(CachingProxyTest, RemoveAfterLocalInvalidate) {
+ Cluster cluster = Cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ auto cache = CacheFactory().create();
+
+ cache.getPoolManager()
+ .createFactory()
+ .addLocator("localhost", cluster.getLocatorPort())
+ .create("pool");
+
+ std::string key = std::string("scharles");
+ std::string value = std::string("Sylvia Charles");
+
+ auto region = cache.createRegionFactory(RegionShortcut::CACHING_PROXY)
+ .setPoolName("pool")
+ .create("region");
+
+ region->put(key, value);
+
+ region->localInvalidate(key);
+
+ auto resultRemove = region->remove(key, nullptr);
+ ASSERT_FALSE(resultRemove);
+
+ auto user = region->get(key);
+ ASSERT_EQ(std::dynamic_pointer_cast<CacheableString>(user)->value(), value);
+}
+
+TEST(CachingProxyTest, Remove) {
+ Cluster cluster = Cluster{LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("PARTITION")
+ .execute();
+
+ auto cache = CacheFactory().create();
+
+ cache.getPoolManager()
+ .createFactory()
+ .addLocator("localhost", cluster.getLocatorPort())
+ .create("pool");
+
+ std::string key = std::string("scharles");
+ std::string value = std::string("Sylvia Charles");
+
+ auto region = cache.createRegionFactory(RegionShortcut::CACHING_PROXY)
+ .setPoolName("pool")
+ .create("region");
+
+ region->put(key, value);
+
+ auto resultRemove = region->remove(key);
+ ASSERT_TRUE(resultRemove);
+}
diff --git a/cppcache/src/TcrMessage.cpp b/cppcache/src/TcrMessage.cpp
index 6b30c4a..5e18c08 100644
--- a/cppcache/src/TcrMessage.cpp
+++ b/cppcache/src/TcrMessage.cpp
@@ -1930,7 +1930,7 @@ TcrMessageInvalidate::TcrMessageInvalidate(
TcrMessageDestroy::TcrMessageDestroy(
DataOutput* dataOutput, const Region* region,
const std::shared_ptr<CacheableKey>& key,
- const std::shared_ptr<Cacheable>& value,
+ const std::shared_ptr<Cacheable>& value, bool isUserNullValue,
const std::shared_ptr<Serializable>& aCallbackArgument,
ThinClientBaseDM* connectionDM) {
m_request.reset(dataOutput);
@@ -1953,7 +1953,7 @@ TcrMessageDestroy::TcrMessageDestroy(
"key passed to the constructor can't be nullptr");
}
- if (value != nullptr) {
+ if (value != nullptr || isUserNullValue) {
numOfParts += 2; // for GFE Destroy65.java
writeHeader(TcrMessage::DESTROY, numOfParts);
writeRegionPart(m_regionName);
diff --git a/cppcache/src/TcrMessage.hpp b/cppcache/src/TcrMessage.hpp
index 1107b24..9641444 100644
--- a/cppcache/src/TcrMessage.hpp
+++ b/cppcache/src/TcrMessage.hpp
@@ -552,6 +552,7 @@ class TcrMessageDestroy : public TcrMessage {
TcrMessageDestroy(DataOutput* dataOutput, const Region* region,
const std::shared_ptr<CacheableKey>& key,
const std::shared_ptr<Cacheable>& value,
+ bool isUserNullValue,
const std::shared_ptr<Serializable>& aCallbackArgument,
ThinClientBaseDM* connectionDM = nullptr);
diff --git a/cppcache/src/ThinClientRegion.cpp b/cppcache/src/ThinClientRegion.cpp
index 55940f1..23230f4 100644
--- a/cppcache/src/ThinClientRegion.cpp
+++ b/cppcache/src/ThinClientRegion.cpp
@@ -1022,7 +1022,7 @@ GfErrType ThinClientRegion::destroyNoThrow_remote(
// do TCR destroy
TcrMessageDestroy request(new DataOutput(m_cacheImpl->createDataOutput()),
- this, keyPtr, nullptr, aCallbackArgument,
+ this, keyPtr, nullptr, false, aCallbackArgument,
m_tcrdm.get());
TcrMessageReply reply(true, m_tcrdm.get());
err = m_tcrdm->sendSyncRequest(request, reply);
@@ -1066,8 +1066,8 @@ GfErrType ThinClientRegion::removeNoThrow_remote(
// do TCR remove
TcrMessageDestroy request(new DataOutput(m_cacheImpl->createDataOutput()),
- this, keyPtr, cvalue, aCallbackArgument,
- m_tcrdm.get());
+ this, keyPtr, cvalue, cvalue == nullptr,
+ aCallbackArgument, m_tcrdm.get());
TcrMessageReply reply(true, m_tcrdm.get());
err = m_tcrdm->sendSyncRequest(request, reply);
if (err != GF_NOERR) {
@@ -1109,7 +1109,7 @@ GfErrType ThinClientRegion::removeNoThrowEX_remote(
// do TCR remove
TcrMessageDestroy request(new DataOutput(m_cacheImpl->createDataOutput()),
- this, keyPtr, nullptr, aCallbackArgument,
+ this, keyPtr, nullptr, false, aCallbackArgument,
m_tcrdm.get());
TcrMessageReply reply(true, m_tcrdm.get());
err = m_tcrdm->sendSyncRequest(request, reply);
diff --git a/cppcache/test/TcrMessageTest.cpp b/cppcache/test/TcrMessageTest.cpp
index 741d88b..ad74626 100644
--- a/cppcache/test/TcrMessageTest.cpp
+++ b/cppcache/test/TcrMessageTest.cpp
@@ -264,16 +264,16 @@ TEST_F(TcrMessageTest, testConstructor2WithDestroy) {
TcrMessageDestroy message(
new DataOutputUnderTest(), static_cast<const Region *>(nullptr),
CacheableString::create("mykey"),
- static_cast<const std::shared_ptr<CacheableKey>>(nullptr),
+ static_cast<const std::shared_ptr<CacheableKey>>(nullptr), true,
static_cast<const std::shared_ptr<Serializable>>(nullptr),
static_cast<ThinClientBaseDM *>(nullptr));
EXPECT_EQ(TcrMessage::DESTROY, message.getMessageType());
EXPECT_MESSAGE_EQ(
- "000000090000004800000005FFFFFFFF000000001300494E56414C49445F524547494F4E"
- "5F4E414D4500000008015700056D796B6579000000010129000000010129000000120003"
- "000000000000000103\\h{16}",
+ "000000090000004900000005FFFFFFFF000000001300494E56414C49445F524547494F4E"
+ "5F4E414D4500000008015700056D796B6579000000010129000000020137080000001200"
+ "03000000000000000103\\h{16}",
message);
}