fix: give FDv1 adapter a private status manager

Author: beekld

libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.cpp

@@ -121,12 +121,11 @@ std::string const& FDv1AdapterSynchronizer::ConvertingDestination::Identity()
 
 // ----- FDv1AdapterSynchronizer -----
 
-FDv1AdapterSynchronizer::FDv1AdapterSynchronizer(
-    std::unique_ptr<data_interfaces::IDataSynchronizer> fdv1_source,
-    data_components::DataSourceStatusManager* status_manager)
+FDv1AdapterSynchronizer::FDv1AdapterSynchronizer(SourceBuilder source_builder)
     : state_(std::make_shared<State>(close_promise_.GetFuture())),
       destination_(std::make_unique<ConvertingDestination>(state_)),
-      status_manager_(status_manager),
+      status_manager_(
+          std::make_unique<data_components::DataSourceStatusManager>()),
       status_subscription_(status_manager_->OnDataSourceStatusChange(
           [state = state_](DataSourceStatus status) {
               auto error = status.LastError();
@@ -148,7 +147,7 @@ FDv1AdapterSynchronizer::FDv1AdapterSynchronizer(
                       break;
               }
           })),
-      fdv1_source_(std::move(fdv1_source)) {}
+      fdv1_source_(source_builder(*status_manager_)) {}
 
 FDv1AdapterSynchronizer::~FDv1AdapterSynchronizer() {
     Close();

libs/server-sdk/src/data_systems/fdv2/fdv1_adapter_synchronizer.hpp

@@ -9,6 +9,7 @@
 #include <launchdarkly/connection.hpp>
 
 #include <deque>
+#include <functional>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -34,17 +35,17 @@ namespace launchdarkly::server_side::data_systems {
 class FDv1AdapterSynchronizer final
     : public data_interfaces::IFDv2Synchronizer {
    public:
+    using SourceBuilder =
+        std::function<std::unique_ptr<data_interfaces::IDataSynchronizer>(
+            data_components::DataSourceStatusManager&)>;
+
     /**
-     * @param fdv1_source The wrapped source. Must have been constructed
-     *                    with status_manager as its status sink so that
-     *                    state changes flow back into this adapter.
-     * @param status_manager Non-owning. The caller retains ownership and
-     *                       must keep it alive for the lifetime of the
-     *                       wrapped source and this adapter.
+     * @param source_builder Called once during construction with the
+     *                       adapter's status manager. Returns the wrapped
+     *                       FDv1 source, which must be constructed against
+     *                       the provided manager as its status sink.
      */
-    FDv1AdapterSynchronizer(
-        std::unique_ptr<data_interfaces::IDataSynchronizer> fdv1_source,
-        data_components::DataSourceStatusManager* status_manager);
+    explicit FDv1AdapterSynchronizer(SourceBuilder source_builder);
 
     ~FDv1AdapterSynchronizer() override;
 
@@ -111,10 +112,8 @@ class FDv1AdapterSynchronizer final
     std::shared_ptr<State> const state_;
     std::unique_ptr<ConvertingDestination> const destination_;
 
-    // Non-owning. The caller must keep this alive for the lifetime of
-    // the wrapped source, which holds a reference to it for status
-    // reporting.
-    data_components::DataSourceStatusManager* const status_manager_;
+    std::unique_ptr<data_components::DataSourceStatusManager> const
+        status_manager_;
     std::unique_ptr<IConnection> const status_subscription_;
 
     std::unique_ptr<data_interfaces::IDataSynchronizer> const fdv1_source_;

libs/server-sdk/tests/fdv1_adapter_synchronizer_test.cpp

@@ -47,41 +47,53 @@ class MockFDv1Source final : public IDataSynchronizer {
     bool bootstrap_was_null = false;
 };
 
+// Returns a SourceBuilder closure that constructs a MockFDv1Source. The
+// resulting source and the adapter's internal status manager are exposed
+// through the out parameters; pass nullptr for either to skip.
+FDv1AdapterSynchronizer::SourceBuilder MakeMockBuilder(
+    MockFDv1Source** out_source = nullptr,
+    DataSourceStatusManager** out_sm = nullptr) {
+    return [out_source, out_sm](DataSourceStatusManager& sm) {
+        if (out_sm) {
+            *out_sm = &sm;
+        }
+        auto source = std::make_unique<MockFDv1Source>(sm);
+        if (out_source) {
+            *out_source = source.get();
+        }
+        return source;
+    };
+}
+
 }  // namespace
 
 TEST(FDv1AdapterSynchronizerTest, FirstNextStartsFDv1Source) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     auto future = adapter.Next(data_model::Selector{});
 
-    EXPECT_EQ(1, source_ptr->start_count);
-    EXPECT_TRUE(source_ptr->bootstrap_was_null);
+    EXPECT_EQ(1, source->start_count);
+    EXPECT_TRUE(source->bootstrap_was_null);
     EXPECT_FALSE(future.IsFinished());
 }
 
 TEST(FDv1AdapterSynchronizerTest, SecondNextDoesNotRestartSource) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     auto first = adapter.Next(data_model::Selector{});
-    source_ptr->destination_->Init(data_model::SDKDataSet{});
+    source->destination_->Init(data_model::SDKDataSet{});
     auto result = first.WaitForResult(1s);
     ASSERT_TRUE(result.has_value());
     adapter.Next(data_model::Selector{});
 
-    EXPECT_EQ(1, source_ptr->start_count);
+    EXPECT_EQ(1, source->start_count);
 }
 
 TEST(FDv1AdapterSynchronizerTest, FDv1InitProducesFullChangeSet) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     auto future = adapter.Next(data_model::Selector{});
 
@@ -90,7 +102,7 @@ TEST(FDv1AdapterSynchronizerTest, FDv1InitProducesFullChangeSet) {
     flag.key = "flagA";
     flag.version = 1;
     data_set.flags.emplace("flagA", data_model::FlagDescriptor(flag));
-    source_ptr->destination_->Init(std::move(data_set));
+    source->destination_->Init(std::move(data_set));
 
     auto result = future.WaitForResult(1s);
     ASSERT_TRUE(result.has_value());
@@ -104,17 +116,15 @@ TEST(FDv1AdapterSynchronizerTest, FDv1InitProducesFullChangeSet) {
 }
 
 TEST(FDv1AdapterSynchronizerTest, FDv1UpsertProducesPartialChangeSet) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     // Flag upsert.
     auto flag_future = adapter.Next(data_model::Selector{});
     data_model::Flag flag;
     flag.key = "flagA";
     flag.version = 2;
-    source_ptr->destination_->Upsert("flagA", data_model::FlagDescriptor(flag));
+    source->destination_->Upsert("flagA", data_model::FlagDescriptor(flag));
 
     auto flag_result = flag_future.WaitForResult(1s);
     ASSERT_TRUE(flag_result.has_value());
@@ -131,8 +141,7 @@ TEST(FDv1AdapterSynchronizerTest, FDv1UpsertProducesPartialChangeSet) {
     data_model::Segment seg;
     seg.key = "segA";
     seg.version = 3;
-    source_ptr->destination_->Upsert("segA",
-                                     data_model::SegmentDescriptor(seg));
+    source->destination_->Upsert("segA", data_model::SegmentDescriptor(seg));
 
     auto seg_result = seg_future.WaitForResult(1s);
     ASSERT_TRUE(seg_result.has_value());
@@ -146,9 +155,7 @@ TEST(FDv1AdapterSynchronizerTest, FDv1UpsertProducesPartialChangeSet) {
 }
 
 TEST(FDv1AdapterSynchronizerTest, ClosePendingNextReturnsShutdown) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder());
 
     auto future = adapter.Next(data_model::Selector{});
     EXPECT_FALSE(future.IsFinished());
@@ -162,48 +169,42 @@ TEST(FDv1AdapterSynchronizerTest, ClosePendingNextReturnsShutdown) {
 }
 
 TEST(FDv1AdapterSynchronizerTest, CloseShutsDownStartedFDv1Source) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     adapter.Next(data_model::Selector{});
     adapter.Close();
 
-    EXPECT_EQ(1, source_ptr->shutdown_count);
+    EXPECT_EQ(1, source->shutdown_count);
 }
 
 TEST(FDv1AdapterSynchronizerTest, CloseWithoutStartDoesNotShutDownFDv1Source) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     // No Next() call — FDv1 source was never started.
     adapter.Close();
 
-    EXPECT_EQ(0, source_ptr->start_count);
-    EXPECT_EQ(0, source_ptr->shutdown_count);
+    EXPECT_EQ(0, source->start_count);
+    EXPECT_EQ(0, source->shutdown_count);
 }
 
 TEST(FDv1AdapterSynchronizerTest, QueuedResultsDrainInFifoOrder) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    auto* source_ptr = source.get();
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    MockFDv1Source* source = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(&source));
 
     // Start the source by satisfying one Next() with an Init.
     auto first = adapter.Next(data_model::Selector{});
-    source_ptr->destination_->Init(data_model::SDKDataSet{});
+    source->destination_->Init(data_model::SDKDataSet{});
     first.WaitForResult(1s);
 
     // Two upserts queue with no Next() in flight.
     data_model::Flag flag_a;
     flag_a.key = "a";
     data_model::Flag flag_b;
     flag_b.key = "b";
-    source_ptr->destination_->Upsert("a", data_model::FlagDescriptor(flag_a));
-    source_ptr->destination_->Upsert("b", data_model::FlagDescriptor(flag_b));
+    source->destination_->Upsert("a", data_model::FlagDescriptor(flag_a));
+    source->destination_->Upsert("b", data_model::FlagDescriptor(flag_b));
 
     // Drain in FIFO order.
     auto r1 = adapter.Next(data_model::Selector{}).WaitForResult(1s);
@@ -221,15 +222,14 @@ TEST(FDv1AdapterSynchronizerTest, QueuedResultsDrainInFifoOrder) {
 }
 
 TEST(FDv1AdapterSynchronizerTest, InterruptedStatusProducesInterruptedResult) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    DataSourceStatusManager* source_manager = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(nullptr, &source_manager));
 
     // kInterrupted from kInitializing stays kInitializing; drive past first.
-    status_manager.SetState(DataSourceStatus::DataSourceState::kValid);
+    source_manager->SetState(DataSourceStatus::DataSourceState::kValid);
 
     auto future = adapter.Next(data_model::Selector{});
-    status_manager.SetState(
+    source_manager->SetState(
         DataSourceStatus::DataSourceState::kInterrupted,
         DataSourceStatus::ErrorInfo::ErrorKind::kNetworkError, "boom");
 
@@ -243,12 +243,11 @@ TEST(FDv1AdapterSynchronizerTest, InterruptedStatusProducesInterruptedResult) {
 }
 
 TEST(FDv1AdapterSynchronizerTest, OffStatusProducesTerminalErrorResult) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    DataSourceStatusManager* source_manager = nullptr;
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder(nullptr, &source_manager));
 
     auto future = adapter.Next(data_model::Selector{});
-    status_manager.SetState(
+    source_manager->SetState(
         DataSourceStatus::DataSourceState::kOff,
         DataSourceStatus::ErrorInfo::ErrorKind::kErrorResponse, "401");
 
@@ -262,9 +261,7 @@ TEST(FDv1AdapterSynchronizerTest, OffStatusProducesTerminalErrorResult) {
 }
 
 TEST(FDv1AdapterSynchronizerTest, NextAfterCloseReturnsShutdown) {
-    DataSourceStatusManager status_manager;
-    auto source = std::make_unique<MockFDv1Source>(status_manager);
-    FDv1AdapterSynchronizer adapter(std::move(source), &status_manager);
+    FDv1AdapterSynchronizer adapter(MakeMockBuilder());
 
     adapter.Close();
     auto future = adapter.Next(data_model::Selector{});