Test remove by gid for nonexisting gid and for gid with tombstone.

3 files changed, 31 insertions, 3 deletions

diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
index 0c46e1269d9..f08d8ef2344 100644
--- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
+++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp
@@ -43,6 +43,13 @@ using DocEntryList = std::vector<DocEntry::UP>;
 
 namespace {
 
+template <typename T>
+std::shared_ptr<T>
+as_sp(std::unique_ptr<T> value)
+{
+    return value;
+}
+
 std::unique_ptr<PersistenceProvider>
 getSpi(ConformanceTest::PersistenceFactory &factory, const document::TestDocMan &testDocMan) {
     PersistenceProviderUP result(factory.getPersistenceImplementation(
@@ -916,11 +923,15 @@ TEST_F(ConformanceTest, testRemoveByGid)
     Context context(Priority(0), Trace::TraceLevel(0));
 
     Bucket bucket(makeSpiBucket(BucketId(8, 0x01)));
-    std::shared_ptr<Document> doc1 = testDocMan.createRandomDocumentAtLocation(0x01, 1);
-    std::shared_ptr<Document> doc2 = testDocMan.createRandomDocumentAtLocation(0x01, 2);
+    auto doc1 = as_sp(testDocMan.createRandomDocumentAtLocation(0x01, 1));
+    auto doc2 = as_sp(testDocMan.createRandomDocumentAtLocation(0x01, 2));
+    auto doc3 = as_sp(testDocMan.createRandomDocumentAtLocation(0x01, 3));
+    auto doc4 = as_sp(testDocMan.createRandomDocumentAtLocation(0x01, 4));
     spi->createBucket(bucket);
     EXPECT_EQ(Result(), Result(spi->put(bucket, Timestamp(11), doc1)));
     EXPECT_EQ(Result(), Result(spi->put(bucket, Timestamp(12), doc2)));
+    EXPECT_EQ(Result(), Result(spi->put(bucket, Timestamp(13), doc3)));
+    EXPECT_EQ(Result(), Result(spi->remove(bucket, Timestamp(14), doc3->getId())));
     auto info = spi->getBucketInfo(bucket).getBucketInfo();
     EXPECT_EQ(2, info.getDocumentCount());
     std::vector<DocTypeGidAndTimestamp> ids;
@@ -928,6 +939,11 @@ TEST_F(ConformanceTest, testRemoveByGid)
     assert_remove_by_gid(*spi, bucket, ids, 0, 2, "ignored removebygid");
     ids.back().timestamp = Timestamp(11);
     assert_remove_by_gid(*spi, bucket, ids, 1, 1, "removebygid");
+    ids.back().timestamp = Timestamp(15);
+    ids.back().gid = doc3->getId().getGlobalId();
+    assert_remove_by_gid(*spi, bucket, ids, 0, 1, "already removed removebygid");
+    ids.back().gid = doc4->getId().getGlobalId();
+    assert_remove_by_gid(*spi, bucket, ids, 0, 1, "not found removebygid");
     if (_factory->hasPersistence()) {
         spi.reset();
         document::TestDocMan testDocMan2;
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
index 0ea872a54a5..d656ff3f9e6 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.cpp
@@ -292,6 +292,17 @@ BucketContent::eraseEntry(Timestamp t) {
     }
 }
 
+void
+BucketContent::eraseEntries(const GlobalId& gid) {
+    auto gid_it = _gidMap.find(gid);
+    if (gid_it != _gidMap.end()) {
+        _gidMap.erase(gid_it);
+        auto it = std::remove_if(_entries.begin(), _entries.end(), [&gid](auto& e) { return e.gid == gid; });
+        _entries.erase(it, _entries.end());
+        _outdatedInfo = true;
+    }
+}
+
 DummyPersistence::DummyPersistence(const std::shared_ptr<const document::DocumentTypeRepo> &repo)
     : _initialized(false),
       _repo(repo),
@@ -559,7 +570,7 @@ DummyPersistence::removeByGidAsync(const Bucket& b, std::vector<spi::DocTypeGidA
         DocEntry::SP entry((*bc)->getEntry(gid));
         if (entry && entry->getTimestamp() <= t) {
             numRemoves += entry->isRemove() ? 0 : 1;
-            (*bc)->eraseEntry(entry->getTimestamp());
+            (*bc)->eraseEntries(gid);
         }
     }
     bc.reset();
diff --git a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
index f7f58612b8d..58e31ff1bdf 100644
--- a/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
+++ b/persistence/src/vespa/persistence/dummyimpl/dummypersistence.h
@@ -77,6 +77,7 @@ struct BucketContent {
     std::shared_ptr<DocEntry> getEntry(const GlobalId& gid) const;
     DocEntry::SP getEntry(Timestamp) const;
     void eraseEntry(Timestamp t);
+    void eraseEntries(const GlobalId& git);
     void setActive(bool active = true) {
         _active = active;
         _info = BucketInfo(_info.getChecksum(),