refactor: drop trailing colon from redis key namespaces; tighten test fixture

Author: beekld

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

@@ -14,14 +14,14 @@ namespace {
 
 // Schema strings from the LaunchDarkly Big Segments spec; must match what
 // Relay writes byte-for-byte.
-constexpr char kIncludeKeyPrefix[] = "big_segment_include:";
-constexpr char kExcludeKeyPrefix[] = "big_segment_exclude:";
+//
+// Membership keys are three-part: {user-prefix}:{namespace}:{context-hash},
+// where each membership state has its own namespace. Sync-time is two-part:
+// {user-prefix}:big_segments_synchronized_on.
+constexpr char kIncludeKeyNamespace[] = "big_segment_include";
+constexpr char kExcludeKeyNamespace[] = "big_segment_exclude";
 constexpr char kSyncTimeKey[] = "big_segments_synchronized_on";
 
-std::string PrefixedKey(std::string const& prefix, std::string const& base) {
-    return prefix + ":" + base;
-}
-
 }  // namespace
 
 tl::expected<std::unique_ptr<RedisBigSegmentStore>, std::string>
@@ -40,16 +40,16 @@ RedisBigSegmentStore::RedisBigSegmentStore(
     std::string prefix)
     : redis_(std::move(redis)),
       prefix_(std::move(prefix)),
-      sync_time_key_(PrefixedKey(prefix_, kSyncTimeKey)) {}
+      sync_time_key_(prefix_ + ":" + kSyncTimeKey) {}
 
 RedisBigSegmentStore::~RedisBigSegmentStore() = default;
 
 IBigSegmentStore::GetMembershipResult RedisBigSegmentStore::GetMembership(
     std::string const& context_hash) const {
     std::string const include_key =
-        PrefixedKey(prefix_, kIncludeKeyPrefix + context_hash);
+        prefix_ + ":" + kIncludeKeyNamespace + ":" + context_hash;
     std::string const exclude_key =
-        PrefixedKey(prefix_, kExcludeKeyPrefix + context_hash);
+        prefix_ + ":" + kExcludeKeyNamespace + ":" + context_hash;
 
     std::vector<std::string> included;
     std::vector<std::string> excluded;

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

@@ -33,7 +33,13 @@ class RedisBigSegmentTests : public ::testing::Test {
 
     void AddIncludes(std::string const& context_hash,
                      std::vector<std::string> const& refs) {
-        auto const key = prefix_ + ":big_segment_include:" + context_hash;
+        AddIncludesUnderPrefix(prefix_, context_hash, refs);
+    }
+
+    void AddIncludesUnderPrefix(std::string const& prefix,
+                                std::string const& context_hash,
+                                std::vector<std::string> const& refs) {
+        auto const key = prefix + ":big_segment_include:" + context_hash;
         for (auto const& ref : refs) {
             client_.sadd(key, ref);
         }
@@ -48,8 +54,11 @@ class RedisBigSegmentTests : public ::testing::Test {
     }
 
     void SetSyncTime(std::int64_t millis) {
-        client_.set(prefix_ + ":big_segments_synchronized_on",
-                    std::to_string(millis));
+        SetSyncTimeRaw(std::to_string(millis));
+    }
+
+    void SetSyncTimeRaw(std::string const& value) {
+        client_.set(prefix_ + ":big_segments_synchronized_on", value);
     }
 
    protected:
@@ -58,8 +67,6 @@ class RedisBigSegmentTests : public ::testing::Test {
    private:
     std::string const uri_;
     std::string const prefix_;
-
-   protected:
     sw::redis::Redis client_;
 };
 
@@ -113,8 +120,7 @@ TEST_F(RedisBigSegmentTests, GetMembershipInclusionWinsOverExclusion) {
 
 TEST_F(RedisBigSegmentTests, GetMembershipIsPrefixScoped) {
     // Write under a different prefix; same-named test store should not see it.
-    auto const other_prefix_key = "otherprefix:big_segment_include:alice";
-    client_.sadd(other_prefix_key, "seg1.g1");
+    AddIncludesUnderPrefix("otherprefix", "alice", {"seg1.g1"});
 
     auto const result = store_->GetMembership("alice");
     ASSERT_TRUE(result);
@@ -132,8 +138,7 @@ TEST_F(RedisBigSegmentTests, GetMetadataReturnsSyncTime) {
 }
 
 TEST_F(RedisBigSegmentTests, GetMetadataRejectsMalformedSyncTime) {
-    client_.set(std::string("testprefix:big_segments_synchronized_on"),
-                "not-a-number");
+    SetSyncTimeRaw("not-a-number");
 
     auto const result = store_->GetMetadata();
     ASSERT_FALSE(result);

libs/server-sdk/include/launchdarkly/server_side/integrations/big_segments/big_segment_store_types.hpp

@@ -50,8 +50,6 @@ class Membership {
     [[nodiscard]] std::optional<bool> CheckMembership(
         std::string const& segment_ref) const;
 
-    Membership() = default;
-
    private:
     explicit Membership(std::unordered_map<std::string, bool> entries);
 

libs/server-sdk/tests/big_segment_store_types_test.cpp

@@ -37,8 +37,3 @@ TEST(MembershipTests, DifferentGenerationsAreDistinct) {
     ASSERT_EQ(m.CheckMembership("seg.g1"), false);
     ASSERT_EQ(m.CheckMembership("seg.g2"), true);
 }
-
-TEST(MembershipTests, DefaultConstructedIsEmpty) {
-    Membership const m;
-    ASSERT_FALSE(m.CheckMembership("seg.g1").has_value());
-}