Merge pull request #5753 from vespa-engine/toregge/remove-for-unknown-document-type-is-an-error

Remove for an unknown document type is an error.

6 files changed, 13 insertions, 43 deletions

diff --git a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp
index 34de8a2e6a3..c333b8943e2 100644
--- a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp
+++ b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp
@@ -127,16 +127,6 @@ TEST_F("require that handler snapshot can be retrieved for given bucket space",
     TEST_DO(assertSnapshot({}, f.map.getHandlerSnapshot(space_null)));
 }
 
-TEST_F("require that handler snapshot can be retrieved for given document type (in bucket space)", Fixture)
-{
-    // Note: Document id doesn't contain document type -> all handlers returned
-    TEST_DO(assertSnapshot({handler_a, handler_b},
-                           f.map.getHandlerSnapshot(space_1, DocumentId("userdoc:namespace:1234:namespace"))));
-    TEST_DO(assertSnapshot({handler_a},
-                           f.map.getHandlerSnapshot(space_1, DocumentId("id:namespace:a::doc1"))));
-    EXPECT_TRUE(f.map.getHandlerSnapshot(space_1, DocumentId("id:namespace:c::doc2")).get() == nullptr);
-}
-
 TEST_MAIN()
 {
     TEST_RUN_ALL();
diff --git a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
index a016a56edf7..9262f9a7b6f 100644
--- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
+++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
@@ -565,12 +565,15 @@ TEST_F("require that removes are routed to handlers", SimpleFixture)
     assertHandler(bucket0, tstamp0, docId0, f.hset.handler1);
     assertHandler(bucket0, tstamp0, docId0, f.hset.handler2);
     EXPECT_FALSE(rr.wasFound());
+    EXPECT_TRUE(rr.hasError());
+    EXPECT_EQUAL(Result(Result::PERMANENT_ERROR, "No handler for document type 'type3'"), rr);
 
     f.hset.handler1.setExistingTimestamp(tstamp2);
     rr = f.engine.remove(bucket1, tstamp1, docId1, context);
     assertHandler(bucket1, tstamp1, docId1, f.hset.handler1);
     assertHandler(bucket0, tstamp0, docId0, f.hset.handler2);
     EXPECT_TRUE(rr.wasFound());
+    EXPECT_FALSE(rr.hasError());
 
     f.hset.handler1.setExistingTimestamp(tstamp0);
     f.hset.handler2.setExistingTimestamp(tstamp3);
@@ -578,12 +581,14 @@ TEST_F("require that removes are routed to handlers", SimpleFixture)
     assertHandler(bucket1, tstamp1, docId1, f.hset.handler1);
     assertHandler(bucket1, tstamp1, docId2, f.hset.handler2);
     EXPECT_TRUE(rr.wasFound());
+    EXPECT_FALSE(rr.hasError());
 
     f.hset.handler2.setExistingTimestamp(tstamp0);
     rr = f.engine.remove(bucket1, tstamp1, docId2, context);
     assertHandler(bucket1, tstamp1, docId1, f.hset.handler1);
     assertHandler(bucket1, tstamp1, docId2, f.hset.handler2);
     EXPECT_FALSE(rr.wasFound());
+    EXPECT_FALSE(rr.hasError());
 }
 
 
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.cpp
index cf7d027f244..feea15ddadd 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.cpp
@@ -94,18 +94,4 @@ public:
 
 }
 
-HandlerSnapshot::UP
-PersistenceHandlerMap::getHandlerSnapshot(document::BucketSpace bucketSpace,
-                                          const document::DocumentId &id) const
-{
-    if (!id.hasDocType()) {
-        return getHandlerSnapshot(bucketSpace);
-    }
-    IPersistenceHandler::SP handler = getHandler(bucketSpace, DocTypeName(id.getDocType()));
-    if (!handler.get()) {
-        return HandlerSnapshot::UP();
-    }
-    return std::make_unique<HandlerSnapshot>(SequenceOfOne::make(handler.get()), 1);
-}
-
 }
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.h
index 8a852066284..003c378d6a7 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistence_handler_map.h
@@ -62,8 +62,6 @@ public:
                                     const DocTypeName &docType) const;
     HandlerSnapshot::UP getHandlerSnapshot() const;
     HandlerSnapshot::UP getHandlerSnapshot(document::BucketSpace bucketSpace) const;
-    HandlerSnapshot::UP getHandlerSnapshot(document::BucketSpace bucketSpace,
-                                           const document::DocumentId &id) const;
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
index 0950a596f5b..056497d137e 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.cpp
@@ -173,13 +173,6 @@ PersistenceEngine::getHandlerSnapshot(document::BucketSpace bucketSpace) const
     return _handlers.getHandlerSnapshot(bucketSpace);
 }
 
-PersistenceEngine::HandlerSnapshot::UP
-PersistenceEngine::getHandlerSnapshot(document::BucketSpace bucketSpace, const DocumentId &id) const
-{
-    std::lock_guard<std::mutex> guard(_lock);
-    return _handlers.getHandlerSnapshot(bucketSpace, id);
-}
-
 PersistenceEngine::PersistenceEngine(IPersistenceEngineOwner &owner, const IResourceWriteFilter &writeFilter,
                                      ssize_t defaultSerializedSize, bool ignoreMaxBytes)
     : AbstractPersistenceProvider(),
@@ -356,17 +349,17 @@ PersistenceEngine::RemoveResult
 PersistenceEngine::remove(const Bucket& b, Timestamp t, const DocumentId& did, Context&)
 {
     std::shared_lock<std::shared_timed_mutex> rguard(_rwMutex);
+    assert(did.hasDocType());
+    DocTypeName docType(did.getDocType());
     LOG(spam, "remove(%s, %" PRIu64 ", \"%s\")", b.toString().c_str(),
         static_cast<uint64_t>(t.getValue()), did.toString().c_str());
-    HandlerSnapshot::UP snap = getHandlerSnapshot(b.getBucketSpace(), did);
-    if (!snap) {
-        return RemoveResult(false);
-    }
-    TransportLatch latch(snap->size());
-    for (; snap->handlers().valid(); snap->handlers().next()) {
-        IPersistenceHandler *handler = snap->handlers().get();
-        handler->handleRemove(feedtoken::make(latch), b, t, did);
+    IPersistenceHandler::SP handler = getHandler(b.getBucketSpace(), docType);
+    if (!handler) {
+        return RemoveResult(Result::PERMANENT_ERROR,
+                            make_string("No handler for document type '%s'", docType.toString().c_str()));
     }
+    TransportLatch latch(1);
+    handler->handleRemove(feedtoken::make(latch), b, t, did);
     latch.await();
     return latch.getRemoveResult();
 }
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h
index a9c15e6f2b9..f0e7b7057de 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/persistenceengine.h
@@ -84,8 +84,6 @@ private:
                                        const DocTypeName &docType) const;
     HandlerSnapshot::UP getHandlerSnapshot() const;
     HandlerSnapshot::UP getHandlerSnapshot(document::BucketSpace bucketSpace) const;
-    HandlerSnapshot::UP getHandlerSnapshot(document::BucketSpace bucketSpace,
-                                           const document::DocumentId &docId) const;
 
     void saveClusterState(BucketSpace bucketSpace, const ClusterState &calc);
     ClusterState::SP savedClusterState(BucketSpace bucketSpace) const;