diff options
author | Tor Brede Vekterli <vekterli@yahooinc.com> | 2022-01-19 12:39:18 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@yahooinc.com> | 2022-01-19 12:39:18 +0000 |
commit | efe929581809e5a1e4db895e08e22ad07ae10757 (patch) | |
tree | c06df4c16b5113c369f3a97025a02a9e7c525273 | |
parent | 8ca62417e9c3b1bb2887226054069fb4c346c835 (diff) |
Expicitly reject "none"-fieldset for external visitors
It does not make sense for a client to run a visitor that inherently
cannot return any data at all. Explicitly check for, and reject, `[none]`
fieldsets and emit a hopefully helpful error message to use `[id]`
instead.
3 files changed, 28 insertions, 0 deletions
diff --git a/storage/src/tests/distributor/visitoroperationtest.cpp b/storage/src/tests/distributor/visitoroperationtest.cpp index ea99186a434..6c597b620dd 100644 --- a/storage/src/tests/distributor/visitoroperationtest.cpp +++ b/storage/src/tests/distributor/visitoroperationtest.cpp @@ -252,6 +252,20 @@ TEST_F(VisitorOperationTest, no_bucket) { runEmptyVisitor(msg)); } +TEST_F(VisitorOperationTest, none_fieldset_is_rejected) { + enable_cluster_state("distributor:1 storage:1"); + auto msg = std::make_shared<api::CreateVisitorCommand>( + makeBucketSpace(), "dumpvisitor", "instance", ""); + msg->addBucketToBeVisited(document::BucketId(16, 1)); + msg->addBucketToBeVisited(nullId); + msg->setFieldSet("[none]"); + + EXPECT_EQ("CreateVisitorReply(last=BucketId(0x0000000000000000)) " + "ReturnCode(ILLEGAL_PARAMETERS, Field set '[none]' is not supported " + "for external visitor operations. Use '[id]' to return documents with no fields set.)", + runEmptyVisitor(msg)); +} + TEST_F(VisitorOperationTest, only_super_bucket_and_progress_allowed) { enable_cluster_state("distributor:1 storage:1"); diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp index 42411d53d52..5abaad6ef9f 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "visitoroperation.h" +#include <vespa/document/fieldset/fieldsets.h> #include <vespa/storage/common/reindexing_constants.h> #include <vespa/storage/storageserver/storagemetricsset.h> #include <vespa/storage/distributor/top_level_distributor.h> @@ -347,6 +348,17 @@ VisitorOperation::verifyOperationSentToCorrectDistributor() verifyDistributorOwnsBucket(_superBucket.bid); } +void +VisitorOperation::verify_fieldset_makes_sense_for_visiting() +{ + if (_msg->getFieldSet() == document::NoFields::NAME) { + throw VisitorVerificationException( + api::ReturnCode::ILLEGAL_PARAMETERS, + "Field set '[none]' is not supported for external visitor operations. " + "Use '[id]' to return documents with no fields set."); + } +} + bool VisitorOperation::verifyCreateVisitorCommand(DistributorStripeMessageSender& sender) { @@ -354,6 +366,7 @@ VisitorOperation::verifyCreateVisitorCommand(DistributorStripeMessageSender& sen verifyOperationContainsBuckets(); verifyOperationHasSuperbucketAndProgress(); verifyOperationSentToCorrectDistributor(); + verify_fieldset_makes_sense_for_visiting(); // TODO wrap and test if (is_read_for_write() && (_msg->getMaxBucketsPerVisitor() != 1)) { throw VisitorVerificationException( diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h index c38c463a313..ce10ea87c11 100644 --- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h @@ -106,6 +106,7 @@ private: void verifyOperationContainsBuckets(); void verifyOperationHasSuperbucketAndProgress(); void verifyOperationSentToCorrectDistributor(); + void verify_fieldset_makes_sense_for_visiting(); bool verifyCreateVisitorCommand(DistributorStripeMessageSender& sender); bool pickBucketsToVisit(const std::vector<BucketDatabase::Entry>& buckets); bool expandBucketContaining(); |