Route document removes only to the recipient(s) handling the document type (as given in the document id).

For legacy document ids (without document type) we still use all the routes.

4 files changed, 123 insertions, 0 deletions

diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentRouteSelectorPolicy.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentRouteSelectorPolicy.java
index e313763fd0e..29a9548579e 100755
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentRouteSelectorPolicy.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/DocumentRouteSelectorPolicy.java
@@ -145,6 +145,14 @@ public class DocumentRouteSelectorPolicy
         case DocumentProtocol.MESSAGE_UPDATEDOCUMENT:
             return selector.accepts(((UpdateDocumentMessage)msg).getDocumentUpdate()) != Result.FALSE;
 
+        case DocumentProtocol.MESSAGE_REMOVEDOCUMENT: {
+            RemoveDocumentMessage removeMsg = (RemoveDocumentMessage)msg;
+            if (removeMsg.getDocumentId().hasDocType()) {
+                return selector.accepts(removeMsg.getDocumentRemove()) != Result.FALSE;
+            } else {
+                return true;
+            }
+        }
 
         case DocumentProtocol.MESSAGE_BATCHDOCUMENTUPDATE:
             BatchDocumentUpdateMessage bdu = (BatchDocumentUpdateMessage)msg;
diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestCase.java
index 45409877285..9db4d1c0d2e 100755
--- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestCase.java
+++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/test/PolicyTestCase.java
@@ -357,6 +357,50 @@ public class PolicyTestCase {
     }
 
     @Test
+    public void remove_document_messages_are_sent_to_the_route_handling_the_given_document_type() {
+        PolicyTestFrame frame = createFrameWithTwoRoutes();
+
+        frame.setMessage(createRemove("id:ns:testdoc::1"));
+        frame.assertSelect(Arrays.asList("testdoc-route"));
+
+        frame.setMessage(createRemove("id:ns:other::1"));
+        frame.assertSelect(Arrays.asList("other-route"));
+    }
+
+    @Test
+    public void remove_document_messages_with_legacy_document_ids_are_sent_to_all_routes() {
+        PolicyTestFrame frame = createFrameWithTwoRoutes();
+
+        frame.setMessage(createRemove("userdoc:testdoc:1234:1"));
+        frame.assertSelect(Arrays.asList("testdoc-route", "other-route"));
+
+        frame.setMessage(createRemove("userdoc:other:1234:1"));
+        frame.assertSelect(Arrays.asList("testdoc-route", "other-route"));
+    }
+
+    private PolicyTestFrame createFrameWithTwoRoutes() {
+        PolicyTestFrame result = new PolicyTestFrame(manager);
+        result.setHop(new HopSpec("test", createDocumentRouteSelectorConfigWithTwoRoutes())
+                .addRecipient("testdoc-route").addRecipient("other-route"));
+        return result;
+    }
+
+    private String createDocumentRouteSelectorConfigWithTwoRoutes() {
+        return "[DocumentRouteSelector:raw:" +
+                "route[2]\n" +
+                "route[0].name \"testdoc-route\"\n" +
+                "route[0].selector \"testdoc and testdoc.stringfield != '0'\"\n" +
+                "route[0].feed \"\"\n" +
+                "route[1].name \"other-route\"\n" +
+                "route[1].selector \"other and other.intfield != '0'\"\n" +
+                "route[1].feed \"\"\n]";
+    }
+
+    private RemoveDocumentMessage createRemove(String docId) {
+        return new RemoveDocumentMessage(new DocumentId(docId));
+    }
+
+    @Test
     public void testSubsetService() {
         PolicyTestFrame frame = new PolicyTestFrame("docproc/cluster.default", manager);
         frame.setMessage(new PutDocumentMessage(new DocumentPut(new DocumentPut(new Document(manager.getDocumentType("testdoc"),
diff --git a/documentapi/src/tests/policies/policies_test.cpp b/documentapi/src/tests/policies/policies_test.cpp
index 58db3079631..cc598aef786 100644
--- a/documentapi/src/tests/policies/policies_test.cpp
+++ b/documentapi/src/tests/policies/policies_test.cpp
@@ -63,6 +63,8 @@ public:
     void testAND();
     void testDocumentRouteSelector();
     void testDocumentRouteSelectorIgnore();
+    void remove_document_messages_are_sent_to_the_route_handling_the_given_document_type();
+    void remove_document_messages_with_legacy_document_ids_are_sent_to_all_routes();
     void testExternSend();
     void testExternMultipleSlobroks();
     void testLoadBalancer();
@@ -103,6 +105,8 @@ Test::Main() {
     testAND();                          TEST_FLUSH();
     testDocumentRouteSelector();        TEST_FLUSH();
     testDocumentRouteSelectorIgnore();  TEST_FLUSH();
+    remove_document_messages_are_sent_to_the_route_handling_the_given_document_type(); TEST_FLUSH();
+    remove_document_messages_with_legacy_document_ids_are_sent_to_all_routes();        TEST_FLUSH();
     testExternSend();                   TEST_FLUSH();
     testExternMultipleSlobroks();       TEST_FLUSH();
     testLoadBalancer();                 TEST_FLUSH();
@@ -673,6 +677,62 @@ Test::testDocumentRouteSelectorIgnore()
 }
 
 namespace {
+
+vespalib::string
+createDocumentRouteSelectorConfigWithTwoRoutes()
+{
+    return "[DocumentRouteSelector:raw:"
+            "route[2]\n"
+            "route[0].name \"testdoc-route\"\n"
+            "route[0].selector \"testdoc and testdoc.stringfield != '0'\"\n"
+            "route[0].feed \"\"\n"
+            "route[1].name \"other-route\"\n"
+            "route[1].selector \"other and other.intfield != '0'\"\n"
+            "route[1].feed \"\"\n]";
+}
+
+std::unique_ptr<TestFrame>
+createFrameWithTwoRoutes(DocumentTypeRepo::SP repo)
+{
+    auto result = std::make_unique<TestFrame>(repo);
+    result->setHop(mbus::HopSpec("test", createDocumentRouteSelectorConfigWithTwoRoutes())
+                           .addRecipient("testdoc-route").addRecipient("other-route"));
+    return result;
+}
+
+std::unique_ptr<RemoveDocumentMessage>
+makeRemove(vespalib::string docId)
+{
+    return std::make_unique<RemoveDocumentMessage>(DocumentId(docId));
+}
+
+}
+
+void
+Test::remove_document_messages_are_sent_to_the_route_handling_the_given_document_type()
+{
+    auto frame = createFrameWithTwoRoutes(_repo);
+
+    frame->setMessage(makeRemove("id:ns:testdoc::1"));
+    EXPECT_TRUE(frame->testSelect({"testdoc-route"}));
+
+    frame->setMessage(makeRemove("id:ns:other::1"));
+    EXPECT_TRUE(frame->testSelect({"other-route"}));
+}
+
+void
+Test::remove_document_messages_with_legacy_document_ids_are_sent_to_all_routes()
+{
+    auto frame = createFrameWithTwoRoutes(_repo);
+
+    frame->setMessage(makeRemove("userdoc:testdoc:1234:1"));
+    EXPECT_TRUE(frame->testSelect({"testdoc-route", "other-route"}));
+
+    frame->setMessage(makeRemove("userdoc:other:1234:1"));
+    EXPECT_TRUE(frame->testSelect({"testdoc-route", "other-route"}));
+}
+
+namespace {
     string getDefaultDistributionConfig(
                     uint16_t redundancy = 2, uint16_t nodeCount = 10,
                     storage::lib::Distribution::DiskDistribution distr
diff --git a/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp b/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp
index 6756f694267..c763723ada1 100644
--- a/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp
+++ b/documentapi/src/vespa/documentapi/messagebus/policies/documentrouteselectorpolicy.cpp
@@ -15,6 +15,8 @@
 #include <vespa/vespalib/util/stringfmt.h>
 
 #include <vespa/log/log.h>
+#include <vespa/documentapi/messagebus/messages/removedocumentmessage.h>
+
 LOG_SETUP(".documentrouteselectorpolicy");
 
 using document::select::Result;
@@ -129,6 +131,15 @@ DocumentRouteSelectorPolicy::select(mbus::RoutingContext &context, const vespali
     case DocumentProtocol::MESSAGE_UPDATEDOCUMENT:
         return it->second->contains(static_cast<const UpdateDocumentMessage&>(msg).getDocumentUpdate()) != Result::False;
 
+    case DocumentProtocol::MESSAGE_REMOVEDOCUMENT: {
+        const RemoveDocumentMessage &removeMsg = static_cast<const RemoveDocumentMessage &>(msg);
+        if (removeMsg.getDocumentId().hasDocType()) {
+            return it->second->contains(removeMsg.getDocumentId()) != Result::False;
+        } else {
+            return true;
+        }
+    }
+
     case DocumentProtocol::MESSAGE_MULTIOPERATION:
     {
         const MultiOperationMessage& mom = static_cast<const MultiOperationMessage&>(msg);