Adjust dummy persistence spi semantics towards proton spi semantics when

bucket doesn't exist:
  setActiveState(), put(), remove() creates bucket if it doesn't already exist.

4 files changed, 96 insertions, 36 deletions

diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
index d2e6ec2716e..495522d3cf5 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
@@ -2283,33 +2283,81 @@ TEST_F(ConformanceTest, resource_usage)
 }
 
 void
-ConformanceTest::test_empty_bucket_info(bool bucket_exists)
+ConformanceTest::test_empty_bucket_info(bool bucket_exists, bool active)
 {
     document::TestDocMan testDocMan;
     _factory->clear();
     PersistenceProviderUP spi(getSpi(*_factory, testDocMan));
     Context context(Priority(0), Trace::TraceLevel(0));
     Bucket bucket(makeSpiBucket(BucketId(8, 0x01)));
+    spi->setClusterState(makeBucketSpace(), createClusterState());
     if (bucket_exists) {
         spi->createBucket(bucket, context);
     }
+    if (active) {
+        spi->setActiveState(bucket, BucketInfo::ACTIVE);
+    }
     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());
+    EXPECT_EQ(active, info_result.getBucketInfo().isActive());
 }
 
 TEST_F(ConformanceTest, test_empty_bucket_gives_empty_bucket_info)
 {
-    test_empty_bucket_info(true);
+    test_empty_bucket_info(true, false);
 }
 
 TEST_F(ConformanceTest, test_missing_bucket_gives_empty_bucket_info)
 {
-    test_empty_bucket_info(false);
+    test_empty_bucket_info(false, false);
+}
+
+TEST_F(ConformanceTest, test_empty_bucket_can_be_activated)
+{
+    test_empty_bucket_info(true, true);
+}
+
+TEST_F(ConformanceTest, test_missing_bucket_can_be_activated)
+{
+    test_empty_bucket_info(false, true);
+}
+
+TEST_F(ConformanceTest, test_put_to_missing_bucket)
+{
+    document::TestDocMan testDocMan;
+    _factory->clear();
+    PersistenceProviderUP spi(getSpi(*_factory, testDocMan));
+    Context context(Priority(0), Trace::TraceLevel(0));
+    Bucket bucket(makeSpiBucket(BucketId(8, 0x01)));
+    std::shared_ptr<Document> doc1 = testDocMan.createRandomDocumentAtLocation(0x01, 1);
+    auto put_result = spi->put(bucket, Timestamp(1), doc1, context);
+    EXPECT_TRUE(!put_result.hasError());
+    auto info_result = spi->getBucketInfo(bucket);
+    EXPECT_TRUE(!info_result.hasError());
+    EXPECT_NE(0u, info_result.getBucketInfo().getChecksum().getValue());
+    EXPECT_EQ(1u, info_result.getBucketInfo().getEntryCount());
+    EXPECT_EQ(1u, info_result.getBucketInfo().getDocumentCount());
+}
+
+TEST_F(ConformanceTest, test_remove_to_missing_bucket)
+{
+    document::TestDocMan testDocMan;
+    _factory->clear();
+    PersistenceProviderUP spi(getSpi(*_factory, testDocMan));
+    Context context(Priority(0), Trace::TraceLevel(0));
+    Bucket bucket(makeSpiBucket(BucketId(8, 0x01)));
+    std::shared_ptr<Document> doc1 = testDocMan.createRandomDocumentAtLocation(0x01, 1);
+    auto remove_result = spi->remove(bucket, Timestamp(1), doc1->getId(), context);
+    EXPECT_TRUE(!remove_result.hasError());
+    auto info_result = spi->getBucketInfo(bucket);
+    EXPECT_TRUE(!info_result.hasError());
+    EXPECT_EQ(0u, info_result.getBucketInfo().getChecksum().getValue());
+    EXPECT_EQ(1u, info_result.getBucketInfo().getEntryCount());
+    EXPECT_EQ(0u, info_result.getBucketInfo().getDocumentCount());
 }
 
 TEST_F(ConformanceTest, detectAndTestOptionalBehavior)
diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.h b/persistence/src/vespa/persistence/conformancetest/conformancetest.h
index 2ee2526c2dd..5c3f20b1900 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.h
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.h
@@ -153,7 +153,7 @@ protected:
 
     void test_iterate_empty_or_missing_bucket(bool bucket_exists);
 
-    void test_empty_bucket_info(bool bucket_exists);
+    void test_empty_bucket_info(bool bucket_exists, bool active);
 
     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 71c12ab0b2f..c7365f39f02 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -376,6 +376,10 @@ DummyPersistence::setActiveStateAsync(const Bucket& b, BucketInfo::ActiveState n
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
 
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
+    if (!bc) {
+        internal_create_bucket(b);
+        bc = acquireBucketWithLock(b);
+    }
     if ( ! bc ) {
         onComplete->onComplete(std::make_unique<BucketInfoResult>(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found"));
     } else {
@@ -412,26 +416,25 @@ DummyPersistence::putAsync(const Bucket& b, Timestamp t, Document::SP doc, Conte
         b.toString().c_str(), uint64_t(t), doc->getId().toString().c_str());
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
-    if (!bc.get()) {
+    while (!bc) {
+        internal_create_bucket(b);
+        bc = acquireBucketWithLock(b);
+    }
+    DocEntry::SP existing = (*bc)->getEntry(t);
+    if (existing) {
         bc.reset();
-        onComplete->onComplete(std::make_unique<BucketInfoResult>(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found"));
-    } else {
-        DocEntry::SP existing = (*bc)->getEntry(t);
-        if (existing) {
-            bc.reset();
-            if (doc->getId() == *existing->getDocumentId()) {
-                onComplete->onComplete(std::make_unique<Result>());
-            } else {
-                onComplete->onComplete(std::make_unique<Result>(Result::ErrorType::TIMESTAMP_EXISTS,
-                                                                "Timestamp already existed"));
-            }
-        } else {
-            LOG(spam, "Inserting document %s", doc->toString(true).c_str());
-            auto entry = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone()));
-            (*bc)->insert(std::move(entry));
-            bc.reset();
+        if (doc->getId() == *existing->getDocumentId()) {
             onComplete->onComplete(std::make_unique<Result>());
+        } else {
+            onComplete->onComplete(std::make_unique<Result>(Result::ErrorType::TIMESTAMP_EXISTS,
+                                                            "Timestamp already existed"));
         }
+    } else {
+        LOG(spam, "Inserting document %s", doc->toString(true).c_str());
+        auto entry = std::make_unique<DocEntry>(t, NONE, Document::UP(doc->clone()));
+        (*bc)->insert(std::move(entry));
+        bc.reset();
+        onComplete->onComplete(std::make_unique<Result>());
     }
 }
 
@@ -478,21 +481,20 @@ DummyPersistence::removeAsync(const Bucket& b, Timestamp t, const DocumentId& di
     assert(b.getBucketSpace() == FixedBucketSpaces::default_space());
 
     BucketContentGuard::UP bc(acquireBucketWithLock(b));
-    if ( ! bc ) {
-        bc.reset();
-        onComplete->onComplete(std::make_unique<RemoveResult>(Result::ErrorType::TRANSIENT_ERROR, "Bucket not found"));
-    } else {
-        DocEntry::SP entry((*bc)->getEntry(did));
-        bool foundPut(entry.get() && !entry->isRemove());
-        auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, did);
-
-        if ((*bc)->hasTimestamp(t)) {
-            (*bc)->eraseEntry(t);
-        }
-        (*bc)->insert(std::move(remEntry));
-        bc.reset();
-        onComplete->onComplete(std::make_unique<RemoveResult>(foundPut));
+    while (!bc) {
+        internal_create_bucket(b);
+        bc = acquireBucketWithLock(b);
+    }
+    DocEntry::SP entry((*bc)->getEntry(did));
+    bool foundPut(entry.get() && !entry->isRemove());
+    auto remEntry = std::make_unique<DocEntry>(t, REMOVE_ENTRY, did);
+    
+    if ((*bc)->hasTimestamp(t)) {
+        (*bc)->eraseEntry(t);
     }
+    (*bc)->insert(std::move(remEntry));
+    bc.reset();
+    onComplete->onComplete(std::make_unique<RemoveResult>(foundPut));
 }
 
 GetResult
@@ -928,6 +930,15 @@ DummyPersistence::releaseBucketNoLock(const BucketContent& bc, LockMode lock_mod
     }
 }
 
+void
+DummyPersistence::internal_create_bucket(const Bucket& b)
+{
+    std::lock_guard lock(_monitor);
+    if (_content.find(b) == _content.end()) {
+        _content[b] = std::make_shared<BucketContent>();
+    }
+}
+
 }
 
 VESPALIB_HASH_MAP_INSTANTIATE_H(storage::spi::Bucket, std::shared_ptr<storage::spi::dummy::BucketContent>, document::BucketId::hash)
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
index a25bf6b8a8e..4e0d088d5d6 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
@@ -203,6 +203,7 @@ private:
     // Const since funcs only alter mutable field in BucketContent
     BucketContentGuard::UP acquireBucketWithLock(const Bucket& b, LockMode lock_mode = LockMode::Exclusive) const;
     void releaseBucketNoLock(const BucketContent& bc, LockMode lock_mode = LockMode::Exclusive) const noexcept;
+    void internal_create_bucket(const Bucket &b);
 
     mutable bool _initialized;
     std::shared_ptr<const document::DocumentTypeRepo> _repo;