Eliminate ApplyBucketDiffEntryResult.

1 files changed, 39 insertions, 21 deletions

diff --git a/storage/src/tests/persistence/apply_bucket_diff_state_test.cpp b/storage/src/tests/persistence/apply_bucket_diff_state_test.cpp
index 6e1e62d2080..fe77477fa77 100644
--- a/storage/src/tests/persistence/apply_bucket_diff_state_test.cpp
+++ b/storage/src/tests/persistence/apply_bucket_diff_state_test.cpp
@@ -1,6 +1,5 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <vespa/storage/persistence/apply_bucket_diff_entry_result.h>
 #include <vespa/storage/persistence/apply_bucket_diff_state.h>
 #include <vespa/storage/persistence/merge_bucket_info_syncer.h>
 #include <vespa/document/base/documentid.h>
@@ -26,38 +25,44 @@ document::Bucket dummy_document_bucket(makeDocumentBucket(document::BucketId(0,
 class DummyMergeBucketInfoSyncer : public MergeBucketInfoSyncer
 {
     uint32_t& _sync_count;
+    vespalib::string _fail;
 public:
     DummyMergeBucketInfoSyncer(uint32_t& sync_count)
         : MergeBucketInfoSyncer(),
-          _sync_count(sync_count)
+          _sync_count(sync_count),
+          _fail()
     {
     }
+    ~DummyMergeBucketInfoSyncer();
     void sync_bucket_info(const spi::Bucket& bucket) const override {
         EXPECT_EQ(bucket, spi::Bucket(dummy_document_bucket));
         ++_sync_count;
+        if (!_fail.empty()) {
+            throw std::runtime_error(_fail);
+        }
     }
+    void set_fail(vespalib::string fail) { _fail = std::move(fail); }
 };
 
-ApplyBucketDiffEntryResult
-make_result(spi::Result &spi_result, const DocumentId &doc_id)
+DummyMergeBucketInfoSyncer::~DummyMergeBucketInfoSyncer() = default;
+
+void
+make_result(ApplyBucketDiffState& state, spi::Result &spi_result, const DocumentId &doc_id)
 {
-    std::promise<std::unique_ptr<spi::Result>> result_promise;
-    result_promise.set_value(std::make_unique<spi::Result>(spi_result));
-    spi::Bucket bucket(makeDocumentBucket(bucket_id_factory.getBucketId(doc_id)));
-    return ApplyBucketDiffEntryResult(result_promise.get_future(), bucket, doc_id, test_op);
+    state.on_entry_complete(std::make_unique<spi::Result>(spi_result), doc_id, test_op);
 }
 
 void push_ok(ApplyBucketDiffState &state)
 {
-    state.push_back(make_result(spi_result_ok, DocumentId("id::test::0")));
-    state.push_back(make_result(spi_result_ok, DocumentId("id::test::1")));
+    make_result(state, spi_result_ok, DocumentId("id::test::0"));
+    make_result(state, spi_result_ok, DocumentId("id::test::1"));
 }
 
 void push_bad(ApplyBucketDiffState &state)
 {
-    state.push_back(make_result(spi_result_ok, DocumentId("id::test::0")));
-    state.push_back(make_result(spi_result_fail, DocumentId("id::test::1")));
-    state.push_back(make_result(spi_result_fail, DocumentId("id::test::2")));
+    make_result(state, spi_result_ok, DocumentId("id::test::0"));
+    make_result(state, spi_result_fail, DocumentId("id::test::1"));
+    make_result(state, spi_result_fail, DocumentId("id::test::2"));
 }
 
 }
@@ -75,15 +80,19 @@ public:
     {
     }
 
+    ~ApplyBucketDiffStateTestBase();
+
     std::unique_ptr<ApplyBucketDiffState> make_state() {
         return std::make_unique<ApplyBucketDiffState>(syncer, spi::Bucket(dummy_document_bucket));
     }
 };
 
+ApplyBucketDiffStateTestBase::~ApplyBucketDiffStateTestBase() = default;
+
 class ApplyBucketDiffStateTest : public ApplyBucketDiffStateTestBase
 {
 public:
-    std::unique_ptr<ApplyBucketDiffState> state;
+    std::shared_ptr<ApplyBucketDiffState> state;
 
     ApplyBucketDiffStateTest()
         : ApplyBucketDiffStateTestBase(),
@@ -95,13 +104,14 @@ public:
         state = make_state();
     }
 
+    void check_failure(std::string expected) {
+        auto future = state->get_future();
+        state.reset();
+        std::string fail_message = future.get();
+        EXPECT_EQ(expected, fail_message);
+    }
     void check_failure() {
-        try {
-            state->check();
-            FAIL() << "Failed to throw exception for failed result";
-        } catch (std::exception &e) {
-            EXPECT_EQ("Failed put for id::test::1 in Bucket(0xeb4700c03842cac4): Result(5, write blocked)", std::string(e.what()));
-        }
+        check_failure("Failed put for id::test::1 in Bucket(0x0000000000000010): Result(5, write blocked)");
     }
 
 };
@@ -109,7 +119,7 @@ public:
 TEST_F(ApplyBucketDiffStateTest, ok_results_can_be_checked)
 {
     push_ok(*state);
-    state->check();
+    check_failure("");
 }
 
 TEST_F(ApplyBucketDiffStateTest, failed_result_errors_ignored)
@@ -146,4 +156,12 @@ TEST_F(ApplyBucketDiffStateTest, explicit_sync_bucket_info_works)
     EXPECT_EQ(1, sync_count);
 }
 
+TEST_F(ApplyBucketDiffStateTest, failed_sync_bucket_info_is_detected)
+{
+    vespalib::string fail("sync bucket failed");
+    syncer.set_fail(fail);
+    state->mark_stale_bucket_info();
+    check_failure(fail);
+}
+
 }