Adjust dummy persistence spi semantics towards proton spi semantics when

bucket doesn't exist:
 * getBucketInfo() returns success with empty bucket info
 * createIterator() returns success
 * iterate() returns empty complete result.

4 files changed, 60 insertions, 11 deletions

diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
index c5f0e60a43a..d2e6ec2716e 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
@@ -1444,14 +1444,17 @@ TEST_F(ConformanceTest, testIterateAlreadyCompleted)
     spi->destroyIterator(iter.getIteratorId(), context);
 }
 
-TEST_F(ConformanceTest, testIterateEmptyBucket)
+void
+ConformanceTest::test_iterate_empty_or_missing_bucket(bool bucket_exists)
 {
     document::TestDocMan testDocMan;
     _factory->clear();
     PersistenceProviderUP spi(getSpi(*_factory, testDocMan));
     Context context(Priority(0), Trace::TraceLevel(0));
     Bucket b(makeSpiBucket(BucketId(8, 0x1)));
-    spi->createBucket(b, context);
+    if (bucket_exists) {
+        spi->createBucket(b, context);
+    }
     Selection sel(createSelection(""));
 
     CreateIteratorResult iter(createIterator(*spi, b, sel));
@@ -1464,6 +1467,16 @@ TEST_F(ConformanceTest, testIterateEmptyBucket)
     spi->destroyIterator(iter.getIteratorId(), context);
 }
 
+TEST_F(ConformanceTest, test_iterate_empty_bucket)
+{
+    test_iterate_empty_or_missing_bucket(true);
+}
+
+TEST_F(ConformanceTest, test_iterate_missing_bucket)
+{
+    test_iterate_empty_or_missing_bucket(false);
+}
+
 TEST_F(ConformanceTest, testDeleteBucket)
 {
     document::TestDocMan testDocMan;
@@ -2269,6 +2282,36 @@ TEST_F(ConformanceTest, resource_usage)
     EXPECT_EQ(0.4, resource_usage_listener.get_usage().get_memory_usage());
 }
 
+void
+ConformanceTest::test_empty_bucket_info(bool bucket_exists)
+{
+    document::TestDocMan testDocMan;
+    _factory->clear();
+    PersistenceProviderUP spi(getSpi(*_factory, testDocMan));
+    Context context(Priority(0), Trace::TraceLevel(0));
+    Bucket bucket(makeSpiBucket(BucketId(8, 0x01)));
+    if (bucket_exists) {
+        spi->createBucket(bucket, context);
+    }
+    auto info_result = spi->getBucketInfo(bucket);
+    EXPECT_TRUE(!info_result.hasError());
+    EXPECT_EQ(0u, info_result.getBucketInfo().getChecksum().getValue());
+    EXPECT_EQ(0u, info_result.getBucketInfo().getEntryCount());
+    EXPECT_EQ(0u, info_result.getBucketInfo().getDocumentCount());
+    EXPECT_TRUE(info_result.getBucketInfo().isReady());
+    EXPECT_FALSE(info_result.getBucketInfo().isActive());
+}
+
+TEST_F(ConformanceTest, test_empty_bucket_gives_empty_bucket_info)
+{
+    test_empty_bucket_info(true);
+}
+
+TEST_F(ConformanceTest, test_missing_bucket_gives_empty_bucket_info)
+{
+    test_empty_bucket_info(false);
+}
+
 TEST_F(ConformanceTest, detectAndTestOptionalBehavior)
 {
     // Report if implementation supports setting bucket size info.
diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.h b/persistence/src/vespa/persistence/conformancetest/conformancetest.h
index a55461bca80..2ee2526c2dd 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.h
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.h
@@ -151,6 +151,10 @@ protected:
             const Bucket& target,
             document::TestDocMan& testDocMan);
 
+    void test_iterate_empty_or_missing_bucket(bool bucket_exists);
+
+    void test_empty_bucket_info(bool bucket_exists);
+
     ConformanceTest();
     ConformanceTest(const std::string &docType);
 };
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index 74fef13f141..71c12ab0b2f 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -393,7 +393,8 @@ DummyPersistence::getBucketInfo(const Bucket& b) const
     if (!bc.get()) {
         LOG(debug, "getBucketInfo(%s) : (bucket not found)",
             b.toString().c_str());
-        return BucketInfoResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
+        BucketInfo info(BucketChecksum(0), 0, 0, 0, 0);
+        return BucketInfoResult(info);
     }
 
     BucketInfo info((*bc)->getBucketInfo());
@@ -538,9 +539,6 @@ DummyPersistence::createIterator(const Bucket &b, FieldSetSP fs, const Selection
         }
     }
     BucketContentGuard::UP bc(acquireBucketWithLock(b, LockMode::Shared));
-    if (!bc.get()) {
-        return CreateIteratorResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
-    }
 
     Iterator* it;
     IteratorId id;
@@ -556,6 +554,10 @@ DummyPersistence::createIterator(const Bucket &b, FieldSetSP fs, const Selection
     }
     // Memory pointed to by 'it' should now be valid from here on out
 
+    if (!bc.get()) {
+        // Bucket not found.
+        return CreateIteratorResult(id);
+    }
     it->_fieldSet = std::move(fs);
     const BucketContent::GidMapType& gidMap((*bc)->_gidMap);
 
@@ -627,7 +629,7 @@ DummyPersistence::iterate(IteratorId id, uint64_t maxByteSize, Context& ctx) con
     BucketContentGuard::UP bc(acquireBucketWithLock(it->_bucket, LockMode::Shared));
     if (!bc.get()) {
         ctx.trace(9, "finished iterate(); bucket not found");
-        return IterateResult(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found");
+        return IterateResult(std::vector<DocEntry::UP>(), true);
     }
     LOG(debug, "Iterator %" PRIu64 " acquired bucket lock", uint64_t(id));
 
diff --git a/storage/src/tests/persistence/filestorage/sanitycheckeddeletetest.cpp b/storage/src/tests/persistence/filestorage/sanitycheckeddeletetest.cpp
index ef71f0ae5f0..588b390cd5f 100644
--- a/storage/src/tests/persistence/filestorage/sanitycheckeddeletetest.cpp
+++ b/storage/src/tests/persistence/filestorage/sanitycheckeddeletetest.cpp
@@ -84,10 +84,10 @@ TEST_F(SanityCheckedDeleteTest, differing_document_sizes_not_considered_out_of_s
 
     c.top.sendDown(delete_cmd);
     c.top.waitForMessages(1, MSG_WAIT_TIME);
-    // Bucket should now well and truly be gone. Will trigger a getBucketInfo error response.
-    spi::BucketInfoResult info_post_delete(
-            _node->getPersistenceProvider().getBucketInfo(spiBucket));
-    ASSERT_TRUE(info_post_delete.hasError()) << info_post_delete.getErrorMessage();
+    auto reply = c.top.getAndRemoveMessage(api::MessageType::DELETEBUCKET_REPLY);
+    auto delete_reply = std::dynamic_pointer_cast<api::DeleteBucketReply>(reply);
+    ASSERT_TRUE(delete_reply);
+    ASSERT_TRUE(delete_reply->getResult().success());
 }
 
 } // namespace storage