Skip to content

Commit 07661c4

Browse files
authored
feat!: RedisDataSource::Create should return unique_ptr instead of shared_ptr (#344)
Previously, `RedisDataSource::Create` returned an `expected<shared_ptr<RedisDataSource>, error>`. We don't need to force the user into shared ownership in the factory function - instead, return a `unique_ptr` and let them make the transformation.
1 parent 99fa7b5 commit 07661c4

File tree

3 files changed

+12
-14
lines changed

3 files changed

+12
-14
lines changed

libs/server-sdk-redis-source/include/launchdarkly/server_side/integrations/redis/redis_source.hpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ class Redis;
1616
}
1717

1818
namespace launchdarkly::server_side::integrations {
19-
2019
/**
2120
* @brief RedisDataSource represents a data source for the Server-Side SDK
2221
* backed by Redis. It is meant to be used in place of the standard
@@ -47,7 +46,7 @@ class RedisDataSource final : public ISerializedDataReader {
4746
*
4847
* @return A RedisDataSource, or an error if construction failed.
4948
*/
50-
static tl::expected<std::shared_ptr<RedisDataSource>, std::string> Create(
49+
static tl::expected<std::unique_ptr<RedisDataSource>, std::string> Create(
5150
std::string uri,
5251
std::string prefix);
5352

@@ -57,8 +56,6 @@ class RedisDataSource final : public ISerializedDataReader {
5756
[[nodiscard]] std::string const& Identity() const override;
5857
[[nodiscard]] bool Initialized() const override;
5958

60-
~RedisDataSource();
61-
6259
private:
6360
RedisDataSource(std::unique_ptr<sw::redis::Redis> redis,
6461
std::string prefix);
@@ -70,5 +67,4 @@ class RedisDataSource final : public ISerializedDataReader {
7067
std::string const inited_key_;
7168
std::unique_ptr<sw::redis::Redis> redis_;
7269
};
73-
7470
} // namespace launchdarkly::server_side::integrations

libs/server-sdk-redis-source/src/redis_source.cpp

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@
33
#include <redis++.h>
44

55
namespace launchdarkly::server_side::integrations {
6-
7-
tl::expected<std::shared_ptr<RedisDataSource>, std::string>
6+
tl::expected<std::unique_ptr<RedisDataSource>, std::string>
87
RedisDataSource::Create(std::string uri, std::string prefix) {
98
try {
10-
return std::shared_ptr<RedisDataSource>(new RedisDataSource(
9+
return std::unique_ptr<RedisDataSource>(new RedisDataSource(
1110
std::make_unique<sw::redis::Redis>(std::move(uri)),
1211
std::move(prefix)));
1312
} catch (sw::redis::Error const& e) {
@@ -26,8 +25,6 @@ RedisDataSource::RedisDataSource(std::unique_ptr<sw::redis::Redis> redis,
2625
inited_key_(prefix_ + ":$inited"),
2726
redis_(std::move(redis)) {}
2827

29-
RedisDataSource::~RedisDataSource() = default;
30-
3128
ISerializedDataReader::GetResult RedisDataSource::Get(
3229
ISerializedItemKind const& kind,
3330
std::string const& itemKey) const {
@@ -54,7 +51,6 @@ ISerializedDataReader::AllResult RedisDataSource::All(
5451
items.emplace(key, SerializedItemDescriptor::Present(0, val));
5552
}
5653
return items;
57-
5854
} catch (sw::redis::Error const& e) {
5955
return tl::make_unexpected(Error{e.what()});
6056
}
@@ -72,5 +68,4 @@ bool RedisDataSource::Initialized() const {
7268
return false;
7369
}
7470
}
75-
7671
} // namespace launchdarkly::server_side::integrations

libs/server-sdk-redis-source/tests/redis_source_test.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,13 @@ TEST_F(RedisTests, FlagAndSegmentCanCoexistWithSameKey) {
405405
serialize(boost::json::value_from(segment_in)));
406406
}
407407

408+
TEST_F(RedisTests, CanConvertRedisDataSourceToDataReader) {
409+
auto maybe_source = RedisDataSource::Create("tcp://foobar:1000", "prefix");
410+
ASSERT_TRUE(maybe_source);
411+
412+
std::shared_ptr<ISerializedDataReader> reader = std::move(*maybe_source);
413+
}
414+
408415
TEST(RedisErrorTests, InvalidURIs) {
409416
std::vector<std::string> const uris = {"nope, not a redis URI",
410417
"http://foo",
@@ -433,11 +440,11 @@ TEST(RedisErrorTests, ValidURIs) {
433440
}
434441

435442
TEST(RedisErrorTests, GetReturnsErrorAndNoExceptionThrown) {
436-
auto const maybe_source = RedisDataSource::Create(
443+
auto maybe_source = RedisDataSource::Create(
437444
"tcp://foobar:1000" /* no redis service here */, "prefix");
438445
ASSERT_TRUE(maybe_source);
439446

440-
auto const source = *maybe_source;
447+
auto const source = std::move(*maybe_source);
441448

442449
auto const get_initialized = source->Initialized();
443450
ASSERT_FALSE(get_initialized);

0 commit comments

Comments
 (0)