diff options
Diffstat (limited to 'documentapi')
21 files changed, 168 insertions, 15 deletions
diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories80.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories80.java index 778eaeda5f0..fb78e291c7c 100644 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories80.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/RoutableFactories80.java @@ -387,6 +387,8 @@ abstract class RoutableFactories80 { if (apiMsg.getCondition().isPresent()) { builder.setCondition(toProtoTasCondition(apiMsg.getCondition())); } + builder.setCreateIfMissing(apiMsg.createIfMissing() ? DocapiFeed.UpdateDocumentRequest.CreateIfMissing.TRUE + : DocapiFeed.UpdateDocumentRequest.CreateIfMissing.FALSE); return builder.build(); }) .decoderWithRepo(DocapiFeed.UpdateDocumentRequest.parser(), (protoMsg, repo) -> { @@ -396,6 +398,8 @@ abstract class RoutableFactories80 { if (protoMsg.hasCondition()) { msg.setCondition(fromProtoTasCondition(protoMsg.getCondition())); } + // We ignore the createIfMissing field here since it can always be fetched eagerly + // from the DocumentUpdate instance itself. return msg; }) .build(); diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/UpdateDocumentMessage.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/UpdateDocumentMessage.java index 3fb14664628..b8609cd42b8 100755 --- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/UpdateDocumentMessage.java +++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/UpdateDocumentMessage.java @@ -166,4 +166,10 @@ public class UpdateDocumentMessage extends TestAndSetMessage { public void setCondition(TestAndSetCondition condition) { this.update.setCondition(condition); } + + boolean createIfMissing() { + deserialize(); + return update.getCreateIfNonExistent(); + } + } diff --git a/documentapi/src/protobuf/docapi_feed.proto b/documentapi/src/protobuf/docapi_feed.proto index 8d15fd9a536..702695ef6d8 100644 --- a/documentapi/src/protobuf/docapi_feed.proto +++ b/documentapi/src/protobuf/docapi_feed.proto @@ -39,11 +39,18 @@ message PutDocumentResponse { } message UpdateDocumentRequest { + enum CreateIfMissing { + UNSPECIFIED = 0; // Legacy fallback: must deserialize `update` to find flag value + TRUE = 1; + FALSE = 2; + } + // Note: update contains embedded document ID DocumentUpdate update = 1; TestAndSetCondition condition = 2; uint64 expected_old_timestamp = 3; uint64 force_assign_timestamp = 4; + CreateIfMissing create_if_missing = 5; } message UpdateDocumentResponse { diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages60TestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages60TestCase.java index 42f200a0b6b..803b225bb0d 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages60TestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages60TestCase.java @@ -504,6 +504,7 @@ public class Messages60TestCase extends MessagesTestBase { assertEquals(msg.getNewTimestamp(), deserializedMsg.getNewTimestamp()); assertEquals(msg.getOldTimestamp(), deserializedMsg.getOldTimestamp()); assertEquals(msg.getCondition().getSelection(), deserializedMsg.getCondition().getSelection()); + assertFalse(msg.createIfMissing()); } } } diff --git a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages80TestCase.java b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages80TestCase.java index 943d9fddb26..d6a837229a9 100644 --- a/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages80TestCase.java +++ b/documentapi/src/test/java/com/yahoo/documentapi/messagebus/protocol/Messages80TestCase.java @@ -176,8 +176,52 @@ public class Messages80TestCase extends MessagesTestBase { } class UpdateDocumentMessageTest implements RunnableTest { - @Override - public void run() { + + UpdateDocumentMessage makeUpdateWithCreateIfMissing(boolean createIfMissing) { + var docType = protocol.getDocumentTypeManager().getDocumentType("testdoc"); + var update = new DocumentUpdate(docType, new DocumentId("id:ns:testdoc::")); + update.addFieldPathUpdate(new RemoveFieldPathUpdate(docType, "intfield", "testdoc.intfield > 0")); + update.setCreateIfNonExistent(createIfMissing); + + var msg = new UpdateDocumentMessage(update); + msg.setNewTimestamp(777); + msg.setOldTimestamp(666); + msg.setCondition(new TestAndSetCondition(CONDITION_STRING)); + return msg; + } + + void testLegacyCreateIfMissingFlagCanBeDeserializedFromDocumentUpdate() { + // Legacy binary files were created _prior_ to the createIfMissing flag being + // written as part of the serialization process. + forEachLanguage((lang) -> { + var msg = (UpdateDocumentMessage) deserialize( + "UpdateDocumentMessage-legacy-no-create-if-missing", + DocumentProtocol.MESSAGE_UPDATEDOCUMENT, lang); + assertFalse(msg.createIfMissing()); + + msg = (UpdateDocumentMessage) deserialize( + "UpdateDocumentMessage-legacy-with-create-if-missing", + DocumentProtocol.MESSAGE_UPDATEDOCUMENT, lang); + assertTrue(msg.createIfMissing()); + }); + } + + void checkDeserialization(Language lang, String name, boolean expectedCreate) { + var msg = (UpdateDocumentMessage) deserialize(name, DocumentProtocol.MESSAGE_UPDATEDOCUMENT, lang); + assertEquals(expectedCreate, msg.createIfMissing()); + }; + + void testCreateIfMissingFlagIsPropagated() { + serialize("UpdateDocumentMessage-no-create-if-missing", makeUpdateWithCreateIfMissing(false)); + serialize("UpdateDocumentMessage-with-create-if-missing", makeUpdateWithCreateIfMissing(true)); + + forEachLanguage((lang) -> { + checkDeserialization(lang, "UpdateDocumentMessage-no-create-if-missing", false); + checkDeserialization(lang, "UpdateDocumentMessage-with-create-if-missing", true); + }); + } + + void testAllUpdateFieldsArePropagated() { var docType = protocol.getDocumentTypeManager().getDocumentType("testdoc"); var update = new DocumentUpdate(docType, new DocumentId("id:ns:testdoc::")); update.addFieldPathUpdate(new RemoveFieldPathUpdate(docType, "intfield", "testdoc.intfield > 0")); @@ -197,6 +241,13 @@ public class Messages80TestCase extends MessagesTestBase { assertEquals(msg.getCondition().getSelection(), deserializedMsg.getCondition().getSelection()); }); } + + @Override + public void run() { + testAllUpdateFieldsArePropagated(); + testLegacyCreateIfMissingFlagCanBeDeserializedFromDocumentUpdate(); + testCreateIfMissingFlagIsPropagated(); + } } class UpdateDocumentReplyTest implements RunnableTest { diff --git a/documentapi/src/tests/messages/messages60test.cpp b/documentapi/src/tests/messages/messages60test.cpp index 2af523ce8e9..861189a2d33 100644 --- a/documentapi/src/tests/messages/messages60test.cpp +++ b/documentapi/src/tests/messages/messages60test.cpp @@ -406,7 +406,7 @@ TEST_F(Messages60Test, testUpdateDocumentMessage) { msg.setNewTimestamp(777u); msg.setCondition(TestAndSetCondition("There's just one condition")); - EXPECT_EQ(sizeof(TestAndSetMessage) + 32, sizeof(UpdateDocumentMessage)); + EXPECT_EQ(sizeof(TestAndSetMessage) + 40, sizeof(UpdateDocumentMessage)); EXPECT_EQ(MESSAGE_BASE_LENGTH + 93u + serializedLength(msg.getCondition().getSelection()), serialize("UpdateDocumentMessage", msg)); for (uint32_t lang = 0; lang < NUM_LANGUAGES; ++lang) { diff --git a/documentapi/src/tests/messages/messages80test.cpp b/documentapi/src/tests/messages/messages80test.cpp index c61b1575dac..093e44ed6b1 100644 --- a/documentapi/src/tests/messages/messages80test.cpp +++ b/documentapi/src/tests/messages/messages80test.cpp @@ -29,7 +29,7 @@ TEST(MessagesTest, concrete_types_have_expected_sizes) { EXPECT_EQ(sizeof(PutDocumentMessage), sizeof(TestAndSetMessage) + 32); EXPECT_EQ(sizeof(WriteDocumentReply), 112u); EXPECT_EQ(sizeof(UpdateDocumentReply), 120u); - EXPECT_EQ(sizeof(UpdateDocumentMessage), sizeof(TestAndSetMessage) + 32); + EXPECT_EQ(sizeof(UpdateDocumentMessage), sizeof(TestAndSetMessage) + 40); EXPECT_EQ(sizeof(RemoveDocumentMessage), sizeof(TestAndSetMessage) + 104); EXPECT_EQ(sizeof(RemoveDocumentReply), 120u); } @@ -42,6 +42,14 @@ struct Messages80Test : MessageFixture { } void try_visitor_reply(const std::string& filename, uint32_t type); + + void check_update_create_flag(uint32_t lang, const std::string& name, bool expected_create, bool expected_cached) { + auto obj = deserialize(name, DocumentProtocol::MESSAGE_UPDATEDOCUMENT, lang); + ASSERT_TRUE(obj); + auto& msg = dynamic_cast<UpdateDocumentMessage&>(*obj); + EXPECT_EQ(msg.has_cached_create_if_missing(), expected_cached); + EXPECT_EQ(msg.create_if_missing(), expected_create); + }; }; namespace { @@ -180,6 +188,48 @@ TEST_F(Messages80Test, update_document_message) { } } +TEST_F(Messages80Test, update_create_if_missing_flag_can_be_read_from_legacy_update_propagation) { + // Legacy binary files were created _prior_ to the create_if_missing flag being + // written as part of the serialization process. + for (auto lang : languages()) { + check_update_create_flag(lang, "UpdateDocumentMessage-legacy-no-create-if-missing", false, false); + check_update_create_flag(lang, "UpdateDocumentMessage-legacy-with-create-if-missing", true, false); + } +} + +TEST_F(Messages80Test, update_create_if_missing_flag_is_propagated) { + const DocumentTypeRepo& repo = type_repo(); + const document::DocumentType& docType = *repo.getDocumentType("testdoc"); + + auto make_update_msg = [&](bool create_if_missing, bool cache_flag) { + auto doc_update = std::make_shared<document::DocumentUpdate>(repo, docType, document::DocumentId("id:ns:testdoc::")); + doc_update->addFieldPathUpdate(std::make_unique<document::RemoveFieldPathUpdate>("intfield", "testdoc.intfield > 0")); + doc_update->setCreateIfNonExistent(create_if_missing); + auto msg = std::make_shared<UpdateDocumentMessage>(std::move(doc_update)); + msg->setOldTimestamp(666u); + msg->setNewTimestamp(777u); + msg->setCondition(TestAndSetCondition("There's just one condition")); + if (cache_flag) { + msg->set_cached_create_if_missing(create_if_missing); + } + return msg; + }; + + serialize("UpdateDocumentMessage-no-create-if-missing", *make_update_msg(false, true)); + serialize("UpdateDocumentMessage-with-create-if-missing", *make_update_msg(true, true)); + + for (auto lang : languages()) { + check_update_create_flag(lang, "UpdateDocumentMessage-no-create-if-missing", false, true); + check_update_create_flag(lang, "UpdateDocumentMessage-with-create-if-missing", true, true); + } + // The Java protocol implementation always serializes with a cached create-flag, + // but the C++ side does it conditionally. So these files are only checked for C++. + serialize("UpdateDocumentMessage-no-create-if-missing-uncached", *make_update_msg(false, false)); + serialize("UpdateDocumentMessage-with-create-if-missing-uncached", *make_update_msg(true, false)); + check_update_create_flag(LANG_CPP, "UpdateDocumentMessage-no-create-if-missing-uncached", false, false); + check_update_create_flag(LANG_CPP, "UpdateDocumentMessage-with-create-if-missing-uncached", true, false); +} + TEST_F(Messages80Test, update_document_reply) { UpdateDocumentReply reply; reply.setWasFound(true); diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.cpp b/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.cpp index 3c0ffa33060..d416e587ed6 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.cpp @@ -5,6 +5,7 @@ #include <vespa/documentapi/messagebus/documentprotocol.h> #include <vespa/document/update/documentupdate.h> #include <vespa/vespalib/util/exceptions.h> +#include <cassert> namespace documentapi { @@ -12,14 +13,16 @@ UpdateDocumentMessage::UpdateDocumentMessage() : TestAndSetMessage(), _documentUpdate(), _oldTime(0), - _newTime(0) + _newTime(0), + _create_if_missing() {} UpdateDocumentMessage::UpdateDocumentMessage(document::DocumentUpdate::SP documentUpdate) : TestAndSetMessage(), _documentUpdate(), _oldTime(0), - _newTime(0) + _newTime(0), + _create_if_missing() { setDocumentUpdate(std::move(documentUpdate)); } @@ -59,4 +62,14 @@ UpdateDocumentMessage::setDocumentUpdate(document::DocumentUpdate::SP documentUp _documentUpdate = std::move(documentUpdate); } +bool +UpdateDocumentMessage::create_if_missing() const +{ + if (_create_if_missing.has_value()) { + return *_create_if_missing; + } + assert(_documentUpdate); + return _documentUpdate->getCreateIfNonExistent(); +} + } diff --git a/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.h b/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.h index 55aa0bf8ae4..e4a528dacdd 100644 --- a/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.h +++ b/documentapi/src/vespa/documentapi/messagebus/messages/updatedocumentmessage.h @@ -2,6 +2,7 @@ #pragma once #include "testandsetmessage.h" +#include <optional> namespace document { class DocumentUpdate; } @@ -10,9 +11,10 @@ namespace documentapi { class UpdateDocumentMessage : public TestAndSetMessage { private: using DocumentUpdateSP = std::shared_ptr<document::DocumentUpdate>; - DocumentUpdateSP _documentUpdate; - uint64_t _oldTime; - uint64_t _newTime; + DocumentUpdateSP _documentUpdate; + uint64_t _oldTime; + uint64_t _newTime; + std::optional<bool> _create_if_missing; protected: DocumentReply::UP doCreateReply() const override; @@ -28,21 +30,21 @@ public: * Constructs a new document message for deserialization. */ UpdateDocumentMessage(); - ~UpdateDocumentMessage(); + ~UpdateDocumentMessage() override; /** * Constructs a new document update message. * * @param documentUpdate The document update to perform. */ - UpdateDocumentMessage(DocumentUpdateSP documentUpdate); + explicit UpdateDocumentMessage(DocumentUpdateSP documentUpdate); /** * Returns the document update to perform. * * @return The update. */ - DocumentUpdateSP stealDocumentUpdate() const { return std::move(_documentUpdate); } + [[nodiscard]] DocumentUpdateSP stealDocumentUpdate() const { return std::move(_documentUpdate); } const document::DocumentUpdate & getDocumentUpdate() const { return *_documentUpdate; } /** * Sets the document update to perform. @@ -56,21 +58,21 @@ public: * * @return The document timestamp. */ - uint64_t getOldTimestamp() const { return _oldTime; } + [[nodiscard]] uint64_t getOldTimestamp() const noexcept { return _oldTime; } /** * Sets the timestamp required for this update to be applied. * * @param time The timestamp to set. */ - void setOldTimestamp(uint64_t time) { _oldTime = time; } + void setOldTimestamp(uint64_t time) noexcept { _oldTime = time; } /** * Returns the timestamp to assign to the updated document. * * @return The document timestamp. */ - uint64_t getNewTimestamp() const { return _newTime; } + [[nodiscard]] uint64_t getNewTimestamp() const noexcept { return _newTime; } /** * Sets the timestamp to assign to the updated document. @@ -79,6 +81,18 @@ public: */ void setNewTimestamp(uint64_t time) { _newTime = time; } + void set_cached_create_if_missing(bool create) noexcept { + _create_if_missing = create; + } + + [[nodiscard]] bool has_cached_create_if_missing() const noexcept { + return _create_if_missing.has_value(); + } + // Note: iff has_cached_create_if_missing() == false, this will trigger a deserialization of the + // underlying DocumentUpdate instance, which might throw an exception on deserialization failure. + // Otherwise, this is noexcept. + [[nodiscard]] bool create_if_missing() const; + bool hasSequenceId() const override; uint64_t getSequenceId() const override; uint32_t getType() const override; diff --git a/documentapi/src/vespa/documentapi/messagebus/routable_factories_8.cpp b/documentapi/src/vespa/documentapi/messagebus/routable_factories_8.cpp index f9782e1abd9..9ef54932f68 100644 --- a/documentapi/src/vespa/documentapi/messagebus/routable_factories_8.cpp +++ b/documentapi/src/vespa/documentapi/messagebus/routable_factories_8.cpp @@ -312,6 +312,10 @@ std::shared_ptr<IRoutableFactory> RoutableFactories80::update_document_message_f } dest.set_expected_old_timestamp(src.getOldTimestamp()); dest.set_force_assign_timestamp(src.getNewTimestamp()); + if (src.has_cached_create_if_missing()) { + dest.set_create_if_missing(src.create_if_missing() ? protobuf::UpdateDocumentRequest_CreateIfMissing_TRUE + : protobuf::UpdateDocumentRequest_CreateIfMissing_FALSE); + } }, [type_repo = std::move(repo)](const protobuf::UpdateDocumentRequest& src) { auto msg = std::make_unique<UpdateDocumentMessage>(); @@ -321,6 +325,9 @@ std::shared_ptr<IRoutableFactory> RoutableFactories80::update_document_message_f } msg->setOldTimestamp(src.expected_old_timestamp()); msg->setNewTimestamp(src.force_assign_timestamp()); + if (src.create_if_missing() != protobuf::UpdateDocumentRequest_CreateIfMissing_UNSPECIFIED) { + msg->set_cached_create_if_missing(src.create_if_missing() == protobuf::UpdateDocumentRequest_CreateIfMissing_TRUE); + } return msg; } ); diff --git a/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-legacy-no-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-legacy-no-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..f1ceef0e51a --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-legacy-no-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-legacy-with-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-legacy-with-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..840bd693670 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-legacy-with-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-no-create-if-missing-uncached.dat b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-no-create-if-missing-uncached.dat Binary files differnew file mode 100644 index 00000000000..f1ceef0e51a --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-no-create-if-missing-uncached.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-no-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-no-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..fc42a504f8b --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-no-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-with-create-if-missing-uncached.dat b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-with-create-if-missing-uncached.dat Binary files differnew file mode 100644 index 00000000000..840bd693670 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-with-create-if-missing-uncached.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-with-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-with-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..ea4852b2e7f --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-cpp-UpdateDocumentMessage-with-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-legacy-no-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-legacy-no-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..f1ceef0e51a --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-legacy-no-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-legacy-with-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-legacy-with-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..840bd693670 --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-legacy-with-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-no-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-no-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..fc42a504f8b --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-no-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-with-create-if-missing.dat b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-with-create-if-missing.dat Binary files differnew file mode 100644 index 00000000000..ea4852b2e7f --- /dev/null +++ b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage-with-create-if-missing.dat diff --git a/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage.dat b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage.dat Binary files differindex f1ceef0e51a..fc42a504f8b 100644 --- a/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage.dat +++ b/documentapi/test/crosslanguagefiles/8.310-java-UpdateDocumentMessage.dat |