Ensure required response metadata is propagated to Reply instance

3 files changed, 62 insertions, 5 deletions

diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp
index 8690d89e12b..b1a754bbbab 100644
--- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp
+++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp
@@ -108,7 +108,7 @@ std::string version_as_gtest_string(TestParamInfo<vespalib::Version> info) {
 // TODO replace with INSTANTIATE_TEST_SUITE_P on newer gtest versions
 INSTANTIATE_TEST_CASE_P(MultiVersionTest, StorageProtocolTest,
                         Values(vespalib::Version(6, 240, 0),
-                               vespalib::Version(7, 40, 5)),
+                               vespalib::Version(7, 41, 19)),
                         version_as_gtest_string);
 
 namespace {
@@ -191,7 +191,58 @@ TEST_P(StorageProtocolTest, response_without_remapped_bucket_preserves_original_
     EXPECT_FALSE(reply2->hasBeenRemapped());
     EXPECT_EQ(_bucket_id, reply2->getBucketId());
     EXPECT_EQ(document::BucketId(), reply2->getOriginalBucketId());
+}
+
+TEST_P(StorageProtocolTest, invalid_bucket_info_is_propagated) {
+    auto cmd = std::make_shared<PutCommand>(_bucket, _testDoc, 14);
+    auto cmd2 = copyCommand(cmd);
+    auto reply = std::make_shared<PutReply>(*cmd2);
+    BucketInfo invalid_info;
+    EXPECT_FALSE(invalid_info.valid());
+    reply->setBucketInfo(invalid_info);
+    auto reply2 = copyReply(reply);
+
+    EXPECT_EQ(invalid_info, reply2->getBucketInfo());
+    EXPECT_FALSE(reply2->getBucketInfo().valid());
+}
+
+TEST_P(StorageProtocolTest, all_zero_bucket_info_is_propagated) {
+    auto cmd = std::make_shared<PutCommand>(_bucket, _testDoc, 14);
+    auto cmd2 = copyCommand(cmd);
+    auto reply = std::make_shared<PutReply>(*cmd2);
+    BucketInfo zero_info(0, 0, 0, 0, 0, false, false, 0);
+    reply->setBucketInfo(zero_info);
+    auto reply2 = copyReply(reply);
 
+    EXPECT_EQ(zero_info, reply2->getBucketInfo());
+}
+
+TEST_P(StorageProtocolTest, request_metadata_is_propagated) {
+    auto cmd = std::make_shared<PutCommand>(_bucket, _testDoc, 14);
+    cmd->forceMsgId(12345);
+    cmd->setPriority(50);
+    cmd->setLoadType(_loadTypes["foo"]);
+    cmd->setSourceIndex(321);
+    auto cmd2 = copyCommand(cmd);
+    EXPECT_EQ(12345, cmd2->getMsgId());
+    EXPECT_EQ(50, cmd2->getPriority());
+    EXPECT_EQ(_loadTypes["foo"].getId(), cmd2->getLoadType().getId());
+    EXPECT_EQ(321, cmd2->getSourceIndex());
+}
+
+TEST_P(StorageProtocolTest, response_metadata_is_propagated) {
+    auto cmd = std::make_shared<PutCommand>(_bucket, _testDoc, 14);
+    auto cmd2 = copyCommand(cmd);
+    auto reply = std::make_shared<PutReply>(*cmd2);
+    reply->forceMsgId(1234);
+    reply->setPriority(101);
+    ReturnCode result(ReturnCode::TEST_AND_SET_CONDITION_FAILED, "foo is not bar");
+    reply->setResult(result);
+
+    auto reply2 = copyReply(reply);
+    EXPECT_EQ(result, reply2->getResult());
+    EXPECT_EQ(1234, reply->getMsgId());
+    EXPECT_EQ(101, reply->getPriority());
 }
 
 TEST_P(StorageProtocolTest, update) {
diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp
index ca77977046c..d0446f52893 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp
+++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization7.cpp
@@ -279,6 +279,13 @@ public:
         }
     }
 
+    void transfer_meta_information_to(api::StorageReply& dest) {
+        dest.forceMsgId(_hdr.message_id());
+        dest.setPriority(static_cast<uint8_t>(_hdr.priority()));
+        dest.setResult(api::ReturnCode(static_cast<api::ReturnCode::Result>(_hdr.return_code_id()),
+                                       _hdr.return_code_message()));
+    }
+
     ProtobufType& response() noexcept { return *_proto_obj; }
     const ProtobufType& response() const noexcept { return *_proto_obj; }
 };
@@ -314,6 +321,7 @@ ProtocolSerialization7::decode_response(document::ByteBuffer& in_buf, Func&& f)
     ResponseDecoder<ProtobufType> dec(in_buf);
     const auto& res = dec.response();
     auto reply = f(res);
+    dec.transfer_meta_information_to(*reply);
     return reply;
 }
 
@@ -374,9 +382,7 @@ std::unique_ptr<api::StorageReply>
 ProtocolSerialization7::decode_bucket_info_response(document::ByteBuffer& in_buf, Func&& f) const {
     return decode_bucket_response<ProtobufType>(in_buf, [&](const ProtobufType& res) {
         auto reply = f(res);
-        if (res.has_bucket_info()) {
-            reply->setBucketInfo(get_bucket_info(res.bucket_info()));
-        }
+        reply->setBucketInfo(get_bucket_info(res.bucket_info())); // If not present, default of all zeroes is correct
         return reply;
     });
 }
diff --git a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp
index 7bc6333762b..33fcc29db11 100644
--- a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp
+++ b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp
@@ -34,7 +34,7 @@ StorageProtocol::createPolicy(const mbus::string&, const mbus::string&) const
 }
 
 namespace {
-    vespalib::Version version7_0(7, 40, 5);
+    vespalib::Version version7_0(7, 41, 19);
     vespalib::Version version6_0(6, 240, 0);
     vespalib::Version version5_2(5, 93, 30);
     vespalib::Version version5_1(5, 1, 0);