[
https://issues.apache.org/jira/browse/GEODE-10414?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17606478#comment-17606478
]
ASF GitHub Bot commented on GEODE-10414:
----------------------------------------
albertogpz commented on code in PR #984:
URL: https://github.com/apache/geode-native/pull/984#discussion_r973963090
##########
cppcache/integration/test/RegionPutTest.cpp:
##########
@@ -0,0 +1,294 @@
+/*
+ * 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 <gmock/gmock.h>
+
+#include <chrono>
+#include <future>
+#include <iostream>
+#include <random>
+#include <thread>
+
+#include <gtest/gtest.h>
+
+#include <geode/Cache.hpp>
+#include <geode/CacheTransactionManager.hpp>
+#include <geode/PoolManager.hpp>
+#include <geode/RegionFactory.hpp>
+#include <geode/RegionShortcut.hpp>
+
+#include "CacheRegionHelper.hpp"
+#include "framework/Cluster.h"
+#include "framework/Framework.h"
+#include "framework/Gfsh.h"
+
+namespace {
+
+using apache::geode::client::Cache;
+using apache::geode::client::Cacheable;
+using apache::geode::client::CacheableKey;
+using apache::geode::client::CacheableString;
+using apache::geode::client::CommitConflictException;
+using apache::geode::client::Pool;
+using apache::geode::client::Region;
+using apache::geode::client::RegionShortcut;
+
+using std::chrono::minutes;
+
+using ::testing::Eq;
+using ::testing::IsNull;
+using ::testing::NotNull;
+
+Cache createCache() {
+ using apache::geode::client::CacheFactory;
+
+ auto cache = CacheFactory()
+ .set("log-level", "debug")
+ .set("statistic-sampling-enabled", "false")
+ .create();
+
+ return cache;
+}
+
+std::shared_ptr<Pool> createPool(Cluster& cluster, Cache& cache) {
+ auto poolFactory = cache.getPoolManager().createFactory();
+ cluster.applyLocators(poolFactory);
+ return poolFactory.create("default");
+}
+
+std::shared_ptr<Region> setupRegion(Cache& cache,
+ const std::shared_ptr<Pool>& pool) {
+ auto region = cache.createRegionFactory(RegionShortcut::PROXY)
+ .setPoolName(pool->getName())
+ .create("region");
+
+ return region;
+}
+
+std::shared_ptr<Region> setupCachingRegion(Cache& cache,
+ const std::shared_ptr<Pool>& pool) {
+ auto region = cache.createRegionFactory(RegionShortcut::CACHING_PROXY)
+ .setPoolName(pool->getName())
+ .create("region");
+
+ return region;
+}
+
+TEST(RegionPutTest, testPutIfAbsentNotExistingEntry) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = createCache();
+ auto pool = createPool(cluster, cache);
+ auto region = setupRegion(cache, pool);
+ auto key = CacheableKey::create("key-1");
+ auto value = CacheableString::create("value");
+
+ EXPECT_THAT(region->putIfAbsent(key, value), IsNull());
+
+ auto retrieved = region->get(key);
+ EXPECT_THAT(retrieved, NotNull());
+
+ auto converted = std::dynamic_pointer_cast<CacheableString>(retrieved);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+}
+
+TEST(RegionPutTest, testPutIfAbsentNotExistingEntryWithCaching) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = createCache();
+ auto pool = createPool(cluster, cache);
+ auto region = setupCachingRegion(cache, pool);
+ auto key = CacheableKey::create("key-1");
+ auto value = CacheableString::create("value");
+
+ EXPECT_THAT(region->putIfAbsent(key, value), IsNull());
+
+ auto retrieved = region->get(key);
+ EXPECT_THAT(retrieved, NotNull());
+
+ auto converted = std::dynamic_pointer_cast<CacheableString>(retrieved);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+
+ // Verify cached value matches expected
+ auto entry = region->getEntry(key);
+ EXPECT_THAT(entry, NotNull());
+
+ retrieved = entry->getValue();
+ EXPECT_THAT(converted, NotNull());
+
+ converted = std::dynamic_pointer_cast<CacheableString>(retrieved);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+}
+
+TEST(RegionPutTest, testPutIfAbsentExistingEntry) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = createCache();
+ auto pool = createPool(cluster, cache);
+ auto region = setupRegion(cache, pool);
+ auto key = CacheableKey::create("key-1");
+
+ EXPECT_NO_THROW(region->put(key, CacheableString::create("previous-value")));
+
+ auto value = std::dynamic_pointer_cast<CacheableString>(region->get(key));
+
+ auto previous =
+ region->putIfAbsent(key, CacheableString::create("new-value"));
+ EXPECT_THAT(previous, NotNull());
+
+ auto converted = std::dynamic_pointer_cast<CacheableString>(previous);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+}
+
+TEST(RegionPutTest, testPutIfAbsentExistingEntryCaching) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = createCache();
+ auto pool = createPool(cluster, cache);
+ auto region = setupCachingRegion(cache, pool);
+ auto key = CacheableKey::create("key-1");
+
+ EXPECT_NO_THROW(region->put(key, CacheableString::create("previous-value")));
+
+ auto value = std::dynamic_pointer_cast<CacheableString>(region->get(key));
+
+ auto previous =
+ region->putIfAbsent(key, CacheableString::create("new-value"));
+ EXPECT_THAT(previous, NotNull());
+
+ auto converted = std::dynamic_pointer_cast<CacheableString>(previous);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+
+ // Verify cached value matches expected
+ auto entry = region->getEntry(key);
+ EXPECT_THAT(entry, NotNull());
+
+ auto retrieved = entry->getValue();
+ EXPECT_THAT(converted, NotNull());
+
+ converted = std::dynamic_pointer_cast<CacheableString>(retrieved);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+}
+
+TEST(RegionPutTest, testPutIfAbsentWIthinTxAndOutOfTx) {
+ Cluster cluster{LocatorCount{1}, ServerCount{1}};
+
+ cluster.start();
+
+ cluster.getGfsh()
+ .create()
+ .region()
+ .withName("region")
+ .withType("REPLICATE")
+ .execute();
+
+ auto cache = createCache();
+ auto pool = createPool(cluster, cache);
+ auto region = setupRegion(cache, pool);
+ auto key = CacheableKey::create("key-1");
+ auto value = CacheableString::create("value");
+ auto txMgr = cache.getCacheTransactionManager();
+
+ txMgr->begin();
+ EXPECT_THAT(region->putIfAbsent(key, value), IsNull());
+
+ auto retrieved = region->get(key);
+ EXPECT_THAT(retrieved, NotNull());
+
+ auto converted = std::dynamic_pointer_cast<CacheableString>(retrieved);
+ EXPECT_THAT(converted, NotNull());
+ EXPECT_THAT(converted->toString(), Eq(value->toString()));
+ auto& txId = txMgr->suspend();
+
+ EXPECT_THAT(region->putIfAbsent(key, value), IsNull());
+
+ txMgr->resume(txId);
+ EXPECT_THROW(txMgr->commit(), CommitConflictException);
+}
+
+TEST(RegionPutTest, testPutIfAbsentExistingEntryTX) {
Review Comment:
What is the difference between this test case and
`testPutIfAbsentExistingEntryCaching()`?
Why is there a TX suffix in the name?
##########
cppcache/src/LocalRegion.cpp:
##########
@@ -367,11 +372,27 @@ void LocalRegion::put(const
std::shared_ptr<CacheableKey>& key,
throwExceptionIfError("Region::put", err);
}
+std::shared_ptr<Serializable> LocalRegion::putIfAbsent(
+ const std::shared_ptr<CacheableKey>& key,
+ const std::shared_ptr<Serializable>& value,
+ const std::shared_ptr<Serializable>& cbArg) {
+ auto opTime = startStatOpTime();
+ std::shared_ptr<Serializable> retValue;
+ std::shared_ptr<VersionTag> versionTag;
+ GfErrType err = putIfAbsentImpl(key, value, cbArg, retValue, -1,
Review Comment:
Is the Impl suffix a common convention in C++ to name methods that implement
the logic of public methods?
Given that it is a convention in Java to name classes that implement an
interface, I would change the name to something else to avoid misunderstandings.
> Add putIfAbsent method to region interfaces
> -------------------------------------------
>
> Key: GEODE-10414
> URL: https://issues.apache.org/jira/browse/GEODE-10414
> Project: Geode
> Issue Type: New Feature
> Components: native client
> Reporter: Mario Salazar de Torres
> Assignee: Mario Salazar de Torres
> Priority: Major
> Labels: pull-request-available
>
> *AS A* geode-native contributor
> *I WANT TO* have putIfAbsent region method implemented
> *SO THAT* I can atomically put entries only if they don't previously, exist
> and also, to align it with the Java API.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
