diff options
71 files changed, 449 insertions, 431 deletions
diff --git a/config/src/vespa/config/common/configparser.h b/config/src/vespa/config/common/configparser.h index 42ee684eb59..48db4b8e16a 100644 --- a/config/src/vespa/config/common/configparser.h +++ b/config/src/vespa/config/common/configparser.h @@ -17,14 +17,17 @@ public: class Cfg { public: Cfg(const std::vector<vespalib::string> & v) - : _cfg(&v[0]), _sz(v.size()) + : _cfg(v.empty() ? nullptr : &v[0]), + _sz(v.size()) { } Cfg(const std::vector<vespalib::string, vespalib::allocator_large<vespalib::string>> & v) : - _cfg(&v[0]), + _cfg(v.empty() ? nullptr : &v[0]), _sz(v.size()) { } - size_t size() const { return _sz; } - const vespalib::string & operator[] (size_t idx) const { return _cfg[idx]; } + size_t size() const noexcept { return _sz; } + const vespalib::string & operator[] (size_t idx) const noexcept { + return _cfg[idx]; + } private: const vespalib::string * _cfg; size_t _sz; diff --git a/configserver/src/main/sh/start-configserver b/configserver/src/main/sh/start-configserver index 317af4b2fea..216eefec7a0 100755 --- a/configserver/src/main/sh/start-configserver +++ b/configserver/src/main/sh/start-configserver @@ -157,7 +157,7 @@ jvmargs="$baseuserargs $serveruserargs" export LD_PRELOAD=${VESPA_HOME}/lib64/vespa/malloc/libvespamalloc.so -printenv > $cfpfile +vespa-run-as-vespa-user sh -c "printenv > $cfpfile" fixddir $bundlecachedir vespa-run-as-vespa-user vespa-runserver -s configserver -r 30 -p $pidfile -- \ diff --git a/document/src/main/java/com/yahoo/document/DocumentTypeManagerConfigurer.java b/document/src/main/java/com/yahoo/document/DocumentTypeManagerConfigurer.java index 662fe8665f3..183f92d543c 100644 --- a/document/src/main/java/com/yahoo/document/DocumentTypeManagerConfigurer.java +++ b/document/src/main/java/com/yahoo/document/DocumentTypeManagerConfigurer.java @@ -450,8 +450,9 @@ public class DocumentTypeManagerConfigurer implements ConfigSubscriber.SingleSub void createEmptyStructs() { String docName = docTypeConfig.name(); for (var typeconf : docTypeConfig.structtype()) { - if (usev8geopositions && isPositionStruct(typeconf)) { - addNewType(typeconf.idx(), new GeoPosType(8)); + if (isPositionStruct(typeconf)) { + int geoVersion = usev8geopositions ? 8 : 7; + addNewType(typeconf.idx(), new GeoPosType(geoVersion)); } else { addNewType(typeconf.idx(), new StructDataType(typeconf.name())); } @@ -526,7 +527,7 @@ public class DocumentTypeManagerConfigurer implements ConfigSubscriber.SingleSub } void fillStructs() { for (var structCfg : docTypeConfig.structtype()) { - if (usev8geopositions && isPositionStruct(structCfg)) { + if (isPositionStruct(structCfg)) { continue; } int idx = structCfg.idx(); diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp index 5c263daaa31..a447df1044e 100644 --- a/document/src/tests/documentselectparsertest.cpp +++ b/document/src/tests/documentselectparsertest.cpp @@ -137,11 +137,11 @@ DocumentUpdate::SP DocumentSelectParserTest::createUpdate( const std::string& hstr) { const DocumentType* type = _repo->getDocumentType(doctype); - DocumentUpdate::SP doc(new DocumentUpdate(*_repo, *type, DocumentId(id))); + auto doc = std::make_shared<DocumentUpdate>(*_repo, *type, DocumentId(id)); doc->addUpdate(FieldUpdate(doc->getType().getField("headerval")) - .addUpdate(AssignValueUpdate(IntFieldValue(hint)))); + .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(hint)))); doc->addUpdate(FieldUpdate(doc->getType().getField("hstringval")) - .addUpdate(AssignValueUpdate(StringFieldValue(hstr)))); + .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue(hstr)))); return doc; } diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index e1e86bca671..beb94959e73 100644 --- a/document/src/tests/documentupdatetestcase.cpp +++ b/document/src/tests/documentupdatetestcase.cpp @@ -126,14 +126,14 @@ TEST(DocumentUpdateTest, testSimpleUsage) testRoundtripSerialize(RemoveValueUpdate(IntFieldValue(1)), *arrayType); FieldUpdate fieldUpdate(docType->getField("intf")); - fieldUpdate.addUpdate(AssignValueUpdate(IntFieldValue(1))); + fieldUpdate.addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(1))); nbostream stream = serialize(fieldUpdate); FieldUpdate fieldUpdateCopy(repo, *docType, stream); EXPECT_EQ(fieldUpdate, fieldUpdateCopy); // Test that a document update can be serialized DocumentUpdate docUpdate(repo, *docType, DocumentId("id:ns:test::1")); - docUpdate.addUpdate(fieldUpdateCopy); + docUpdate.addUpdate(std::move(fieldUpdateCopy)); nbostream docBuf = serializeHEAD(docUpdate); auto docUpdateCopy(DocumentUpdate::createHEAD(repo, docBuf)); @@ -150,7 +150,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) { Document updated(doc); DocumentUpdate upd(repo, *docType, DocumentId("id:ns:test::1")); - upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(ClearValueUpdate())); + upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(std::make_unique<ClearValueUpdate>())); upd.applyTo(updated); EXPECT_NE(doc, updated); EXPECT_FALSE(updated.getValue("intf")); @@ -158,7 +158,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) { Document updated(doc); DocumentUpdate upd(repo, *docType, DocumentId("id:ns:test::1")); - upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(AssignValueUpdate(IntFieldValue(15)))); + upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(15)))); upd.applyTo(updated); EXPECT_NE(doc, updated); EXPECT_EQ(15, updated.getValue("intf")->getAsInt()); @@ -166,7 +166,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) { Document updated(doc); DocumentUpdate upd(repo, *docType, DocumentId("id:ns:test::1")); - upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 15))); + upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 15))); upd.applyTo(updated); EXPECT_NE(doc, updated); EXPECT_EQ(20, updated.getValue("intf")->getAsInt()); @@ -174,7 +174,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) { Document updated(doc); DocumentUpdate upd(repo, *docType, DocumentId("id:ns:test::1")); - upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(AddValueUpdate(IntFieldValue(4)))); + upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(4)))); upd.applyTo(updated); EXPECT_NE(doc, updated); std::unique_ptr<ArrayFieldValue> val(dynamic_cast<ArrayFieldValue*>(updated.getValue("intarr").release())); @@ -184,7 +184,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) { Document updated(doc); DocumentUpdate upd(repo, *docType, DocumentId("id:ns:test::1")); - upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(RemoveValueUpdate(IntFieldValue(3)))); + upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(3)))); upd.applyTo(updated); EXPECT_NE(doc, updated); std::unique_ptr<ArrayFieldValue> val(dynamic_cast<ArrayFieldValue*>(updated.getValue("intarr").release())); @@ -195,7 +195,7 @@ TEST(DocumentUpdateTest, testSimpleUsage) Document updated(doc); DocumentUpdate upd(repo, *docType, DocumentId("id:ns:test::1")); upd.addUpdate(FieldUpdate(docType->getField("bytef")) - .addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 15))); + .addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 15))); upd.applyTo(updated); EXPECT_NE(doc, updated); EXPECT_EQ(15, (int) updated.getValue("bytef")->getAsByte()); @@ -212,7 +212,7 @@ TEST(DocumentUpdateTest, testClearField) // Apply an update. DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(AssignValueUpdate())) + .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(std::make_unique<AssignValueUpdate>())) .applyTo(*doc); EXPECT_FALSE(doc->getValue("headerval")); } @@ -227,7 +227,7 @@ TEST(DocumentUpdateTest, testUpdateApplySingleValue) // Apply an update. DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(AssignValueUpdate(IntFieldValue(9)))) + .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(9)))) .applyTo(*doc); EXPECT_EQ(9, doc->getValue("headerval")->getAsInt()); } @@ -245,7 +245,7 @@ TEST(DocumentUpdateTest, testUpdateArray) myarray.add(StringFieldValue("bar")); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(AssignValueUpdate(myarray))) + .addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(std::make_unique<AssignValueUpdate>(myarray))) .applyTo(*doc); auto fval1(doc->getAs<ArrayFieldValue>(doc->getField("tags"))); ASSERT_EQ((size_t) 2, fval1->size()); @@ -255,8 +255,8 @@ TEST(DocumentUpdateTest, testUpdateArray) // Append array field DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(doc->getField("tags")) - .addUpdate(AddValueUpdate(StringFieldValue("another"))) - .addUpdate(AddValueUpdate(StringFieldValue("tag")))) + .addUpdate(std::make_unique<AddValueUpdate>(StringFieldValue("another"))) + .addUpdate(std::make_unique<AddValueUpdate>(StringFieldValue("tag")))) .applyTo(*doc); std::unique_ptr<ArrayFieldValue> fval2(doc->getAs<ArrayFieldValue>(doc->getField("tags"))); @@ -270,15 +270,15 @@ TEST(DocumentUpdateTest, testUpdateArray) ASSERT_THROW( DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(doc->getField("tags")) - .addUpdate(AssignValueUpdate(StringFieldValue("THROW MEH!")))) + .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue("THROW MEH!")))) .applyTo(*doc), std::exception) << "Expected exception when assigning a string value to an array field."; // Remove array field. DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(doc->getField("tags")) - .addUpdate(RemoveValueUpdate(StringFieldValue("foo"))) - .addUpdate(RemoveValueUpdate(StringFieldValue("tag")))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("foo"))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("tag")))) .applyTo(*doc); auto fval3(doc->getAs<ArrayFieldValue>(doc->getField("tags"))); ASSERT_EQ((size_t) 2, fval3->size()); @@ -292,11 +292,25 @@ TEST(DocumentUpdateTest, testUpdateArray) ASSERT_THROW( DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(doc->getField("tags")) - .addUpdate(RemoveValueUpdate(myarray2))) + .addUpdate(std::make_unique<RemoveValueUpdate>(myarray2))) .applyTo(*doc), std::exception) << "Expected exception when removing an array from a string array."; } +std::unique_ptr<ValueUpdate> +createAddUpdate(vespalib::stringref key, int weight) { + auto upd = std::make_unique<AddValueUpdate>(StringFieldValue(key)); + upd->setWeight(weight); + return upd; +} + +std::unique_ptr<ValueUpdate> +createAddUpdate(int key, int weight) { + auto upd = std::make_unique<AddValueUpdate>(IntFieldValue(key)); + upd->setWeight(weight); + return upd; +} + TEST(DocumentUpdateTest, testUpdateWeightedSet) { // Create a test document @@ -310,7 +324,7 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) wset.add(StringFieldValue("foo"), 3); wset.add(StringFieldValue("bar"), 14); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(field).addUpdate(AssignValueUpdate(wset))) + .addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(wset))) .applyTo(*doc); auto fval1(doc->getAs<WeightedSetFieldValue>(field)); ASSERT_EQ((size_t) 2, fval1->size()); @@ -327,7 +341,7 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) wset2.add(StringFieldValue("bar"), 24); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field) - .addUpdate(AssignValueUpdate(wset2))) + .addUpdate(std::make_unique<AssignValueUpdate>(wset2))) .applyTo(*doc); auto fval2(doc->getAs<WeightedSetFieldValue>(field)); ASSERT_EQ((size_t) 2, fval2->size()); @@ -341,8 +355,8 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) // Append weighted field DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field) - .addUpdate(AddValueUpdate(StringFieldValue("foo")).setWeight(3)) - .addUpdate(AddValueUpdate(StringFieldValue("too")).setWeight(14))) + .addUpdate(createAddUpdate("foo", 3)) + .addUpdate(createAddUpdate("too", 14))) .applyTo(*doc); std::unique_ptr<WeightedSetFieldValue> fval3(doc->getAs<WeightedSetFieldValue>(field)); @@ -357,8 +371,8 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) // Remove weighted field DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field) - .addUpdate(RemoveValueUpdate(StringFieldValue("foo"))) - .addUpdate(RemoveValueUpdate(StringFieldValue("too")))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("foo"))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("too")))) .applyTo(*doc); auto fval4(doc->getAs<WeightedSetFieldValue>(field)); ASSERT_EQ((size_t) 1, fval4->size()); @@ -405,8 +419,8 @@ WeightedSetAutoCreateFixture::WeightedSetAutoCreateFixture() update(repo, *docType, DocumentId("id:ns:test::1")) { update.addUpdate(FieldUpdate(field) - .addUpdate(MapValueUpdate(StringFieldValue("foo"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 1)))); + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("foo"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 1)))); } } // anon ns @@ -443,8 +457,8 @@ TEST(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) { WeightedSetAutoCreateFixture fixture; fixture.update.addUpdate(FieldUpdate(fixture.field) - .addUpdate(MapValueUpdate(StringFieldValue("baz"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 0)))); + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("baz"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 0)))); fixture.applyUpdateToDocument(); @@ -525,20 +539,20 @@ TEST(DocumentUpdateTest, testGenerateSerializedFile) const DocumentType *type(repo.getDocumentType("serializetest")); DocumentUpdate upd(repo, *type, DocumentId("id:ns:serializetest::update")); upd.addUpdate(FieldUpdate(type->getField("intfield")) - .addUpdate(AssignValueUpdate(IntFieldValue(4)))); + .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(4)))); upd.addUpdate(FieldUpdate(type->getField("floatfield")) - .addUpdate(AssignValueUpdate(FloatFieldValue(1.00f)))); + .addUpdate(std::make_unique<AssignValueUpdate>(FloatFieldValue(1.00f)))); upd.addUpdate(FieldUpdate(type->getField("arrayoffloatfield")) - .addUpdate(AddValueUpdate(FloatFieldValue(5.00f))) - .addUpdate(AddValueUpdate(FloatFieldValue(4.23f))) - .addUpdate(AddValueUpdate(FloatFieldValue(-1.00f)))); + .addUpdate(std::make_unique<AddValueUpdate>(FloatFieldValue(5.00f))) + .addUpdate(std::make_unique<AddValueUpdate>(FloatFieldValue(4.23f))) + .addUpdate(std::make_unique<AddValueUpdate>(FloatFieldValue(-1.00f)))); upd.addUpdate(FieldUpdate(type->getField("intfield")) - .addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 3))); + .addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 3))); upd.addUpdate(FieldUpdate(type->getField("wsfield")) - .addUpdate(MapValueUpdate(StringFieldValue("foo"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 2))) - .addUpdate(MapValueUpdate(StringFieldValue("foo"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Mul, 2)))); + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("foo"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 2))) + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("foo"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Mul, 2)))); nbostream buf(serializeHEAD(upd)); writeBufferToFile(buf, "data/serializeupdatecpp.dat"); } @@ -555,7 +569,7 @@ TEST(DocumentUpdateTest, testSetBadFieldTypes) DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); ASSERT_THROW( update.addUpdate(FieldUpdate(doc->getField("headerval")) - .addUpdate(AssignValueUpdate(FloatFieldValue(4.00f)))), + .addUpdate(std::make_unique<AssignValueUpdate>(FloatFieldValue(4.00f)))), std::exception) << "Expected exception when adding a float to an int field."; update.applyTo(*doc); @@ -572,7 +586,7 @@ TEST(DocumentUpdateTest, testUpdateApplyNoParams) EXPECT_EQ((document::FieldValue*)nullptr, doc->getValue(doc->getField("tags")).get()); DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); - update.addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(AssignValueUpdate())); + update.addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(std::make_unique<AssignValueUpdate>())); update.applyTo(*doc); @@ -590,7 +604,7 @@ TEST(DocumentUpdateTest, testUpdateApplyNoArrayValues) // Assign array field with no array values = empty array DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); update.addUpdate(FieldUpdate(field) - .addUpdate(AssignValueUpdate(ArrayFieldValue(field.getDataType())))); + .addUpdate(std::make_unique<AssignValueUpdate>(ArrayFieldValue(field.getDataType())))); update.applyTo(*doc); @@ -610,7 +624,7 @@ TEST(DocumentUpdateTest, testUpdateArrayEmptyParamValue) // Assign array field with no array values = empty array. DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); - update.addUpdate(FieldUpdate(field).addUpdate(AssignValueUpdate(ArrayFieldValue(field.getDataType())))); + update.addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(ArrayFieldValue(field.getDataType())))); update.applyTo(*doc); // Verify that the field was set in the document. @@ -620,7 +634,7 @@ TEST(DocumentUpdateTest, testUpdateArrayEmptyParamValue) // Remove array field. DocumentUpdate update2(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); - update2.addUpdate(FieldUpdate(field).addUpdate(ClearValueUpdate())); + update2.addUpdate(FieldUpdate(field).addUpdate(std::make_unique<ClearValueUpdate>())); update2.applyTo(*doc); // Verify that the field was cleared in the document. @@ -638,7 +652,7 @@ TEST(DocumentUpdateTest, testUpdateWeightedSetEmptyParamValue) // Assign weighted set with no items = empty set. DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); - update.addUpdate(FieldUpdate(field).addUpdate(AssignValueUpdate(WeightedSetFieldValue(field.getDataType())))); + update.addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(WeightedSetFieldValue(field.getDataType())))); update.applyTo(*doc); // Verify that the field was set in the document. @@ -648,7 +662,7 @@ TEST(DocumentUpdateTest, testUpdateWeightedSetEmptyParamValue) // Remove weighted set field. DocumentUpdate update2(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); - update2.addUpdate(FieldUpdate(field).addUpdate(ClearValueUpdate())); + update2.addUpdate(FieldUpdate(field).addUpdate(std::make_unique<ClearValueUpdate>())); update2.applyTo(*doc); // Verify that the field was cleared in the document. @@ -668,8 +682,8 @@ TEST(DocumentUpdateTest, testUpdateArrayWrongSubtype) DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); ASSERT_THROW( update.addUpdate(FieldUpdate(field) - .addUpdate(AddValueUpdate(IntFieldValue(123))) - .addUpdate(AddValueUpdate(IntFieldValue(456)))), + .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(123))) + .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(456)))), std::exception) << "Expected exception when adding wrong type."; // Apply update @@ -692,8 +706,8 @@ TEST(DocumentUpdateTest, testUpdateWeightedSetWrongSubtype) DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); ASSERT_THROW( update.addUpdate(FieldUpdate(field) - .addUpdate(AddValueUpdate(IntFieldValue(123)).setWeight(1000)) - .addUpdate(AddValueUpdate(IntFieldValue(456)).setWeight(2000))), + .addUpdate(createAddUpdate(123, 1000)) + .addUpdate(createAddUpdate(456, 2000))), std::exception) << "Expected exception when adding wrong type."; // Apply update @@ -718,8 +732,8 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(MapValueUpdate(StringFieldValue("banana"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 1.0)))) + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("banana"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); std::unique_ptr<WeightedSetFieldValue> fv1 = doc->getAs<WeightedSetFieldValue>(field1); @@ -727,19 +741,19 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field2) - .addUpdate(MapValueUpdate(StringFieldValue("banana"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 1.0)))) + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("banana"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); auto fv2 = doc->getAs<WeightedSetFieldValue>(field2); EXPECT_EQ(1, fv2->size()); EXPECT_EQ(fv1->find(StringFieldValue("apple")), fv1->end()); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(field1).addUpdate(ClearValueUpdate())) + .addUpdate(FieldUpdate(field1).addUpdate(std::make_unique<ClearValueUpdate>())) .applyTo(*doc); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(field1).addUpdate(AddValueUpdate(StringFieldValue("apple")).setWeight(1))) + .addUpdate(FieldUpdate(field1).addUpdate(createAddUpdate("apple", 1))) .applyTo(*doc); auto fval3(doc->getAs<WeightedSetFieldValue>(field1)); @@ -747,7 +761,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) EXPECT_EQ(1, fval3->get(StringFieldValue("apple"))); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(field2).addUpdate(AddValueUpdate(StringFieldValue("apple")).setWeight(1))) + .addUpdate(FieldUpdate(field2).addUpdate(createAddUpdate("apple", 1))) .applyTo(*doc); auto fval3b(doc->getAs<WeightedSetFieldValue>(field2)); @@ -756,8 +770,8 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(MapValueUpdate(StringFieldValue("apple"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Sub, 1.0)))) + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("apple"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Sub, 1.0)))) .applyTo(*doc); auto fv3 = doc->getAs<WeightedSetFieldValue>(field1); @@ -766,8 +780,8 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field2) - .addUpdate(MapValueUpdate(StringFieldValue("apple"), - ArithmeticValueUpdate(ArithmeticValueUpdate::Sub, 1.0)))) + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("apple"), + std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Sub, 1.0)))) .applyTo(*doc); auto fv4 = doc->getAs<WeightedSetFieldValue>(field2); @@ -851,9 +865,9 @@ struct TensorUpdateFixture { .add({{"x", "b"}}, 3)); } - void applyUpdate(const ValueUpdate &update) { + void applyUpdate(std::unique_ptr<ValueUpdate> update) { DocumentUpdate docUpdate(docMan.getTypeRepo(), *emptyDoc->getDataType(), emptyDoc->getId()); - docUpdate.addUpdate(FieldUpdate(docUpdate.getType().getField(fieldName)).addUpdate(update)); + docUpdate.addUpdate(FieldUpdate(docUpdate.getType().getField(fieldName)).addUpdate(std::move(update))); docUpdate.applyTo(updatedDoc); } @@ -887,23 +901,23 @@ struct TensorUpdateFixture { } void assertApplyUpdate(const TensorSpec &initialTensor, - const ValueUpdate &update, + std::unique_ptr<ValueUpdate> update, const TensorSpec &expTensor) { setTensor(initialTensor); - applyUpdate(update); + applyUpdate(std::move(update)); assertDocumentUpdated(); assertTensor(expTensor); } - void assertApplyUpdateNonExisting(const ValueUpdate &update, + void assertApplyUpdateNonExisting(std::unique_ptr<ValueUpdate> update, const TensorSpec &expTensor) { - applyUpdate(update); + applyUpdate(std::move(update)); assertDocumentUpdated(); assertTensor(expTensor); } - void assertApplyUpdateNonExisting(const ValueUpdate &update) { - applyUpdate(update); + void assertApplyUpdateNonExisting(std::unique_ptr<ValueUpdate> update) { + applyUpdate(std::move(update)); assertDocumentUpdated(); assertTensorNull(); } @@ -927,7 +941,7 @@ TEST(DocumentUpdateTest, tensor_assign_update_can_be_applied) { TensorUpdateFixture f; auto newTensor = f.makeBaselineTensor(); - f.applyUpdate(AssignValueUpdate(*newTensor)); + f.applyUpdate(std::make_unique<AssignValueUpdate>(*newTensor)); f.assertDocumentUpdated(); f.assertTensor(*newTensor); } @@ -936,7 +950,7 @@ TEST(DocumentUpdateTest, tensor_clear_update_can_be_applied) { TensorUpdateFixture f; f.setTensor(*f.makeBaselineTensor()); - f.applyUpdate(ClearValueUpdate()); + f.applyUpdate(std::make_unique<ClearValueUpdate>()); f.assertDocumentNotUpdated(); EXPECT_FALSE(f.getTensor()); } @@ -947,7 +961,7 @@ TEST(DocumentUpdateTest, tensor_add_update_can_be_applied) f.assertApplyUpdate(f.spec().add({{"x", "a"}}, 2) .add({{"x", "b"}}, 3), - TensorAddUpdate(f.makeTensor(f.spec().add({{"x", "b"}}, 5) + std::make_unique<TensorAddUpdate>(f.makeTensor(f.spec().add({{"x", "b"}}, 5) .add({{"x", "c"}}, 7))), f.spec().add({{"x", "a"}}, 2) @@ -958,7 +972,7 @@ TEST(DocumentUpdateTest, tensor_add_update_can_be_applied) TEST(DocumentUpdateTest, tensor_add_update_can_be_applied_to_nonexisting_tensor) { TensorUpdateFixture f; - f.assertApplyUpdateNonExisting(TensorAddUpdate(f.makeTensor(f.spec().add({{"x", "b"}}, 5) + f.assertApplyUpdateNonExisting(std::make_unique<TensorAddUpdate>(f.makeTensor(f.spec().add({{"x", "b"}}, 5) .add({{"x", "c"}}, 7))), f.spec().add({{"x", "b"}}, 5) @@ -971,7 +985,7 @@ TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied) f.assertApplyUpdate(f.spec().add({{"x", "a"}}, 2) .add({{"x", "b"}}, 3), - TensorRemoveUpdate(f.makeTensor(f.spec().add({{"x", "b"}}, 1))), + std::make_unique<TensorRemoveUpdate>(f.makeTensor(f.spec().add({{"x", "b"}}, 1))), f.spec().add({{"x", "a"}}, 2)); } @@ -979,7 +993,7 @@ TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied) TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied_to_nonexisting_tensor) { TensorUpdateFixture f; - f.assertApplyUpdateNonExisting(TensorRemoveUpdate(f.makeTensor(f.spec().add({{"x", "b"}}, 1)))); + f.assertApplyUpdateNonExisting(std::make_unique<TensorRemoveUpdate>(f.makeTensor(f.spec().add({{"x", "b"}}, 1)))); } TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied) @@ -989,20 +1003,20 @@ TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied) .add({{"x", "b"}}, 3); f.assertApplyUpdate(baseLine, - TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, + std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::REPLACE, f.makeTensor(f.spec().add({{"x", "b"}}, 5) .add({{"x", "c"}}, 7))), f.spec().add({{"x", "a"}}, 2) .add({{"x", "b"}}, 5)); f.assertApplyUpdate(baseLine, - TensorModifyUpdate(TensorModifyUpdate::Operation::ADD, + std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::ADD, f.makeTensor(f.spec().add({{"x", "b"}}, 5))), f.spec().add({{"x", "a"}}, 2) .add({{"x", "b"}}, 8)); f.assertApplyUpdate(baseLine, - TensorModifyUpdate(TensorModifyUpdate::Operation::MULTIPLY, + std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::MULTIPLY, f.makeTensor(f.spec().add({{"x", "b"}}, 5))), f.spec().add({{"x", "a"}}, 2) .add({{"x", "b"}}, 15)); @@ -1011,7 +1025,7 @@ TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied) TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied_to_nonexisting_tensor) { TensorUpdateFixture f; - f.assertApplyUpdateNonExisting(TensorModifyUpdate(TensorModifyUpdate::Operation::ADD, + f.assertApplyUpdateNonExisting(std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::ADD, f.makeTensor(f.spec().add({{"x", "b"}}, 5)))); } @@ -1148,13 +1162,13 @@ struct TensorUpdateSerializeFixture { (*repo, docType, DocumentId("id:test:test::0")); result->addUpdate(FieldUpdate(getField("sparse_tensor")) - .addUpdate(AssignValueUpdate(*makeTensor())) - .addUpdate(TensorAddUpdate(makeTensor())) - .addUpdate(TensorRemoveUpdate(makeTensor()))); + .addUpdate(std::make_unique<AssignValueUpdate>(*makeTensor())) + .addUpdate(std::make_unique<TensorAddUpdate>(makeTensor())) + .addUpdate(std::make_unique<TensorRemoveUpdate>(makeTensor()))); result->addUpdate(FieldUpdate(getField("dense_tensor")) - .addUpdate(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, makeTensor())) - .addUpdate(TensorModifyUpdate(TensorModifyUpdate::Operation::ADD, makeTensor())) - .addUpdate(TensorModifyUpdate(TensorModifyUpdate::Operation::MULTIPLY, makeTensor()))); + .addUpdate(std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::REPLACE, makeTensor())) + .addUpdate(std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::ADD, makeTensor())) + .addUpdate(std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::MULTIPLY, makeTensor()))); return result; } @@ -1233,10 +1247,10 @@ CreateIfNonExistentFixture::~CreateIfNonExistentFixture() = default; CreateIfNonExistentFixture::CreateIfNonExistentFixture() : docMan(), document(docMan.createDocument()), - update(new DocumentUpdate(docMan.getTypeRepo(), *document->getDataType(), document->getId())) + update(std::make_unique<DocumentUpdate>(docMan.getTypeRepo(), *document->getDataType(), document->getId())) { update->addUpdate(FieldUpdate(document->getField("headerval")) - .addUpdate(AssignValueUpdate(IntFieldValue(1)))); + .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(1)))); update->setCreateIfNonExistent(true); } @@ -1268,8 +1282,8 @@ ArrayUpdateFixture::ArrayUpdateFixture() { update = std::make_unique<DocumentUpdate>(doc_man.getTypeRepo(), *doc->getDataType(), doc->getId()); update->addUpdate(FieldUpdate(array_field) - .addUpdate(MapValueUpdate(IntFieldValue(1), - AssignValueUpdate(StringFieldValue("bar"))))); + .addUpdate(std::make_unique<MapValueUpdate>(IntFieldValue(1), + std::make_unique<AssignValueUpdate>(StringFieldValue("bar"))))); } ArrayUpdateFixture::~ArrayUpdateFixture() = default; diff --git a/document/src/tests/feed_reject_helper_test.cpp b/document/src/tests/feed_reject_helper_test.cpp index bb7faade164..6649c9c0208 100644 --- a/document/src/tests/feed_reject_helper_test.cpp +++ b/document/src/tests/feed_reject_helper_test.cpp @@ -60,7 +60,7 @@ TEST(DocumentRejectTest, requireThatClearRemoveTensorRemoveAndArtithmeticUpdates TEST(DocumentRejectTest, requireThatAddMapTensorModifyAndTensorAddUpdatesWillBeRejected) { EXPECT_TRUE(FeedRejectHelper::mustReject(AddValueUpdate(IntFieldValue()))); - EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(IntFieldValue(), ClearValueUpdate()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(IntFieldValue(), std::make_unique<ClearValueUpdate>()))); EXPECT_TRUE(FeedRejectHelper::mustReject(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, std::make_unique<TensorFieldValue>()))); EXPECT_TRUE(FeedRejectHelper::mustReject(TensorAddUpdate(std::make_unique<TensorFieldValue>()))); diff --git a/document/src/tests/fieldpathupdatetestcase.cpp b/document/src/tests/fieldpathupdatetestcase.cpp index 3d41a5bcacd..17b84ecc180 100644 --- a/document/src/tests/fieldpathupdatetestcase.cpp +++ b/document/src/tests/fieldpathupdatetestcase.cpp @@ -182,8 +182,9 @@ void testSerialize(const DocumentTypeRepo& repo, const DocumentUpdate& a) { } EXPECT_EQ(a, *b); } catch (std::exception& e) { - std::cerr << "Failed while testing document field path update:\n" - << a.toString(true) << "\n"; + std::cerr << "Failed while testing document field path update:\n"; + a.print(std::cerr, true, ""); + std::cerr << std::endl; throw; } } diff --git a/document/src/tests/testxml.cpp b/document/src/tests/testxml.cpp index 2fdcee029c8..978ab572214 100644 --- a/document/src/tests/testxml.cpp +++ b/document/src/tests/testxml.cpp @@ -22,9 +22,7 @@ namespace { Document::UP createTestDocument(const DocumentTypeRepo& repo) { const DocumentType* type(repo.getDocumentType("testdoc")); - Document::UP - doc(new Document(*type, - DocumentId("id:ns:testdoc::crawler/http://www.ntnu.no/"))); + auto doc = std::make_unique<Document>(*type,DocumentId("id:ns:testdoc::crawler/http://www.ntnu.no/")); doc->setRepo(repo); std::string s("humlepungens buffer"); ByteBuffer bb(s.c_str(), s.size()); @@ -47,8 +45,7 @@ Document::UP createTestDocument(const DocumentTypeRepo& repo) val.add(rawVal3); doc->setValue(doc->getField("rawarrayattr"), val); - Document::UP doc2(new Document(*type, DocumentId( - "id:ns:testdoc::crawler/http://www.ntnu.no/2"))); + auto doc2 = std::make_unique<Document>(*type, DocumentId("id:ns:testdoc::crawler/http://www.ntnu.no/2")); doc2->setValue(doc2->getField("stringattr"), StringFieldValue("tjo hei paa du")); doc->setValue(doc->getField("docfield"), *doc2); @@ -61,19 +58,18 @@ createTestDocumentUpdate(const DocumentTypeRepo& repo) const DocumentType* type(repo.getDocumentType("testdoc")); DocumentId id("id:ns:testdoc::crawler/http://www.ntnu.no/"); - DocumentUpdate::UP up(new DocumentUpdate(repo, *type, id)); + auto up = std::make_unique<DocumentUpdate>(repo, *type, id); up->addUpdate(FieldUpdate(type->getField("intattr")) - .addUpdate(AssignValueUpdate(IntFieldValue(7)))); + .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(7)))); up->addUpdate(FieldUpdate(type->getField("stringattr")) - .addUpdate(AssignValueUpdate( - StringFieldValue("New value")))); + .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue("New value")))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) - .addUpdate(AddValueUpdate(IntFieldValue(123))) - .addUpdate(AddValueUpdate(IntFieldValue(456)))); + .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(123))) + .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(456)))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) - .addUpdate(RemoveValueUpdate(IntFieldValue(123))) - .addUpdate(RemoveValueUpdate(IntFieldValue(456))) - .addUpdate(RemoveValueUpdate(IntFieldValue(789)))); + .addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(123))) + .addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(456))) + .addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(789)))); return up; } diff --git a/document/src/vespa/document/base/forcelink.cpp b/document/src/vespa/document/base/forcelink.cpp index 0462f22784c..500b5cf7fa5 100644 --- a/document/src/vespa/document/base/forcelink.cpp +++ b/document/src/vespa/document/base/forcelink.cpp @@ -14,7 +14,7 @@ ForceLink::ForceLink(void) DocumentType type("foo", 1); Document document(type, DocumentId("doc:ns:bar")); DocumentUpdate documentUpdate; - MapValueUpdate mapValueUpdate(IntFieldValue(3), ClearValueUpdate()); + MapValueUpdate mapValueUpdate(IntFieldValue(3), std::make_unique<ClearValueUpdate>()); AddValueUpdate addValueUpdate(IntFieldValue(3)); RemoveValueUpdate removeValueUpdate(IntFieldValue(3)); AssignValueUpdate assignValueUpdate(IntFieldValue(3)); diff --git a/document/src/vespa/document/base/idstring.cpp b/document/src/vespa/document/base/idstring.cpp index a364ffd90ce..f3e41981c8e 100644 --- a/document/src/vespa/document/base/idstring.cpp +++ b/document/src/vespa/document/base/idstring.cpp @@ -110,13 +110,27 @@ fmemchr(const char * s, const char * e) #endif } +namespace { + +// Avoid issues with primitive alignment when reading from buffer. +// Requires caller to ensure buffer is big enough to read from. +template <typename T> +inline T read_unaligned(const char* buf) noexcept +{ + T tmp; + memcpy(&tmp, buf, sizeof(T)); + return tmp; +} + +} + void verifyIdString(const char * id, size_t sz_) { if (sz_ > 4) { - if (_G_id.as16 == *reinterpret_cast<const uint16_t *>(id) && id[2] == ':') { + if ((_G_id.as16 == read_unaligned<uint16_t>(id)) && (id[2] == ':')) { return; - } else if ((sz_ == 6) && (_G_null.as32 == *reinterpret_cast<const uint32_t *>(id)) && (id[4] == ':') && (id[5] == ':')) { + } else if ((sz_ == 6) && (_G_null.as32 == read_unaligned<uint32_t>(id)) && (id[4] == ':') && (id[5] == ':')) { reportNoId(id); } else if (sz_ > 8) { reportNoSchemeSeparator(id); diff --git a/document/src/vespa/document/update/addvalueupdate.h b/document/src/vespa/document/update/addvalueupdate.h index 0707e575e50..ee7f6fbcdf2 100644 --- a/document/src/vespa/document/update/addvalueupdate.h +++ b/document/src/vespa/document/update/addvalueupdate.h @@ -67,7 +67,6 @@ public: void printXml(XmlOutputStream& xos) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream & buffer) override; - AddValueUpdate* clone() const override { return new AddValueUpdate(*this); } DECLARE_IDENTIFIABLE(AddValueUpdate); }; diff --git a/document/src/vespa/document/update/arithmeticvalueupdate.h b/document/src/vespa/document/update/arithmeticvalueupdate.h index f7673adf9c4..cb86528fce5 100644 --- a/document/src/vespa/document/update/arithmeticvalueupdate.h +++ b/document/src/vespa/document/update/arithmeticvalueupdate.h @@ -91,7 +91,6 @@ public: void printXml(XmlOutputStream& xos) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream & buffer) override; - ArithmeticValueUpdate* clone() const override { return new ArithmeticValueUpdate(*this); } DECLARE_IDENTIFIABLE(ArithmeticValueUpdate); }; diff --git a/document/src/vespa/document/update/assignvalueupdate.h b/document/src/vespa/document/update/assignvalueupdate.h index c877236199c..e829e80da45 100644 --- a/document/src/vespa/document/update/assignvalueupdate.h +++ b/document/src/vespa/document/update/assignvalueupdate.h @@ -43,7 +43,6 @@ public: void printXml(XmlOutputStream& xos) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream & buffer) override; - AssignValueUpdate* clone() const override { return new AssignValueUpdate(*this); } DECLARE_IDENTIFIABLE(AssignValueUpdate); }; diff --git a/document/src/vespa/document/update/clearvalueupdate.h b/document/src/vespa/document/update/clearvalueupdate.h index 49aa958c103..6e0d800dd73 100644 --- a/document/src/vespa/document/update/clearvalueupdate.h +++ b/document/src/vespa/document/update/clearvalueupdate.h @@ -25,7 +25,6 @@ public: void printXml(XmlOutputStream& xos) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream& buffer) override; - ClearValueUpdate* clone() const override { return new ClearValueUpdate(*this); } DECLARE_IDENTIFIABLE(ClearValueUpdate); }; diff --git a/document/src/vespa/document/update/documentupdate.cpp b/document/src/vespa/document/update/documentupdate.cpp index fde1ab70e11..89e275bf92d 100644 --- a/document/src/vespa/document/update/documentupdate.cpp +++ b/document/src/vespa/document/update/documentupdate.cpp @@ -11,7 +11,7 @@ #include <vespa/document/datatype/documenttype.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/vespalib/util/xmlstream.h> -#include <ostream> +#include <sstream> using vespalib::IllegalArgumentException; using vespalib::IllegalStateException; @@ -122,9 +122,9 @@ void DocumentUpdate::ensureDeserialized() const { } DocumentUpdate& -DocumentUpdate::addUpdate(const FieldUpdate& update) { +DocumentUpdate::addUpdate(FieldUpdate &&update) { ensureDeserialized(); - _updates.push_back(update); + _updates.push_back(std::move(update)); reserialize(); return *this; } @@ -336,6 +336,15 @@ DocumentUpdate::reserialize() _needHardReserialize = false; } +std::string +DocumentUpdate::toXml(const std::string& indent) const +{ + std::ostringstream ost; + XmlOutputStream xos(ost, indent); + printXml(xos); + return ost.str(); +} + std::ostream & operator<<(std::ostream &out, const DocumentUpdate &update) { diff --git a/document/src/vespa/document/update/documentupdate.h b/document/src/vespa/document/update/documentupdate.h index c7ebe913d79..1afadff2827 100644 --- a/document/src/vespa/document/update/documentupdate.h +++ b/document/src/vespa/document/update/documentupdate.h @@ -40,7 +40,7 @@ class VespaDocumentSerializer; * path updates was added, and a new serialization format was * introduced while keeping the old one. */ -class DocumentUpdate final : public Printable, public vespalib::xml::XmlSerializable +class DocumentUpdate { public: using UP = std::unique_ptr<DocumentUpdate>; @@ -70,7 +70,9 @@ public: DocumentUpdate(const DocumentUpdate &) = delete; DocumentUpdate & operator = (const DocumentUpdate &) = delete; - ~DocumentUpdate() override; + DocumentUpdate(DocumentUpdate &&) = delete; + DocumentUpdate & operator = (DocumentUpdate &&) = delete; + ~DocumentUpdate(); bool operator==(const DocumentUpdate&) const; bool operator!=(const DocumentUpdate & rhs) const { return ! (*this == rhs); } @@ -85,7 +87,7 @@ public: */ void applyTo(Document& doc) const; - DocumentUpdate& addUpdate(const FieldUpdate& update); + DocumentUpdate& addUpdate(FieldUpdate && update); DocumentUpdate& addFieldPathUpdate(const FieldPathUpdate::CP& update); /** @return The list of updates. */ @@ -98,9 +100,8 @@ public: /** @return The type of document this update is for. */ const DocumentType& getType() const; - void print(std::ostream& out, bool verbose, const std::string& indent) const override; + void serializeHEAD(vespalib::nbostream &stream) const; - void printXml(XmlOutputStream&) const override; /** * Sets whether this update should create the document it updates if that document does not exist. @@ -115,6 +116,10 @@ public: int serializeFlags(int size_) const; + // Only used for debugging + void print(std::ostream& out, bool verbose, const std::string& indent) const; + void printXml(XmlOutputStream&) const; + std::string toXml(const std::string& indent) const; private: DocumentId _documentId; // The ID of the document to update. const DataType *_type; // The type of document this update is for. diff --git a/document/src/vespa/document/update/fieldupdate.cpp b/document/src/vespa/document/update/fieldupdate.cpp index ff60c3b7994..f21be067a4a 100644 --- a/document/src/vespa/document/update/fieldupdate.cpp +++ b/document/src/vespa/document/update/fieldupdate.cpp @@ -12,8 +12,7 @@ namespace document { using vespalib::nbostream; FieldUpdate::FieldUpdate(const Field& field) - : Printable(), - _field(field), + : _field(field), _updates() { } @@ -29,19 +28,17 @@ int readInt(nbostream & stream) { } FieldUpdate::FieldUpdate(const DocumentTypeRepo& repo, const DataType & type, nbostream & stream) - : Printable(), - _field(type.getField(readInt(stream))), + : _field(type.getField(readInt(stream))), _updates() { int numUpdates = readInt(stream); _updates.reserve(numUpdates); const DataType& dataType = _field.getDataType(); for(int i(0); i < numUpdates; i++) { - _updates.emplace_back(ValueUpdate::createInstance(repo, dataType, stream).release()); + _updates.emplace_back(ValueUpdate::createInstance(repo, dataType, stream)); } } -FieldUpdate::FieldUpdate(const FieldUpdate &) = default; FieldUpdate::~FieldUpdate() = default; bool @@ -55,6 +52,20 @@ FieldUpdate::operator==(const FieldUpdate& other) const return true; } + +FieldUpdate& +FieldUpdate::addUpdate(std::unique_ptr<ValueUpdate> update) & { + update->checkCompatibility(_field); // May throw exception. + _updates.push_back(std::move(update)); + return *this; +} + +FieldUpdate&& +FieldUpdate::addUpdate(std::unique_ptr<ValueUpdate> update) && { + addUpdate(std::move(update)); + return std::move(*this); +} + void FieldUpdate::printXml(XmlOutputStream& xos) const { diff --git a/document/src/vespa/document/update/fieldupdate.h b/document/src/vespa/document/update/fieldupdate.h index e364e5db5fd..e8e83ab3e48 100644 --- a/document/src/vespa/document/update/fieldupdate.h +++ b/document/src/vespa/document/update/fieldupdate.h @@ -21,24 +21,19 @@ namespace document { class Document; class DocumentType; -class FieldUpdate : public vespalib::Identifiable, - public Printable, - public vespalib::xml::XmlSerializable +class FieldUpdate { - Field _field; - std::vector<ValueUpdate::CP> _updates; - using nbostream = vespalib::nbostream; - public: - typedef vespalib::CloneablePtr<FieldUpdate> CP; + using nbostream = vespalib::nbostream; + using ValueUpdates = std::vector<std::unique_ptr<ValueUpdate>>; using XmlOutputStream = vespalib::xml::XmlOutputStream; FieldUpdate(const Field& field); - FieldUpdate(const FieldUpdate &); + FieldUpdate(const FieldUpdate &) = delete; FieldUpdate & operator = (const FieldUpdate &) = delete; FieldUpdate(FieldUpdate &&) = default; FieldUpdate & operator = (FieldUpdate &&) = default; - ~FieldUpdate() override; + ~FieldUpdate(); /** * This is a convenience function to construct a field update directly from @@ -58,23 +53,20 @@ public: * @param update A pointer to the value update to add to this. * @return A pointer to this. */ - FieldUpdate& addUpdate(const ValueUpdate& update) { - update.checkCompatibility(_field); // May throw exception. - _updates.push_back(ValueUpdate::CP(update.clone())); - return *this; - } + FieldUpdate& addUpdate(std::unique_ptr<ValueUpdate> update) &; + FieldUpdate&& addUpdate(std::unique_ptr<ValueUpdate> update) &&; const ValueUpdate& operator[](int index) const { return *_updates[index]; } ValueUpdate& operator[](int index) { return *_updates[index]; } size_t size() const { return _updates.size(); } /** @return The non-modifieable list of value updates to perform. */ - const std::vector<ValueUpdate::CP>& getUpdates() const { return _updates; } + const ValueUpdates & getUpdates() const { return _updates; } const Field& getField() const { return _field; } void applyTo(Document& doc) const; - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - void printXml(XmlOutputStream&) const override; + void print(std::ostream& out, bool verbose, const std::string& indent) const; + void printXml(XmlOutputStream&) const; /** * Deserializes the given stream into an instance of an update object. @@ -84,7 +76,9 @@ public: * @param buffer The stream that contains the serialized update object. */ void deserialize(const DocumentTypeRepo& repo, const DocumentType& type, nbostream& stream); - +private: + Field _field; + ValueUpdates _updates; }; } // document diff --git a/document/src/vespa/document/update/mapvalueupdate.cpp b/document/src/vespa/document/update/mapvalueupdate.cpp index 48e52a9d2cc..c3d9cff571b 100644 --- a/document/src/vespa/document/update/mapvalueupdate.cpp +++ b/document/src/vespa/document/update/mapvalueupdate.cpp @@ -18,14 +18,24 @@ namespace document { IMPLEMENT_IDENTIFIABLE(MapValueUpdate, ValueUpdate); -MapValueUpdate::MapValueUpdate(const FieldValue& key, const ValueUpdate& update) +MapValueUpdate::MapValueUpdate(const FieldValue& key, std::unique_ptr<ValueUpdate> update) : ValueUpdate(), _key(key.clone()), - _update(update.clone()) + _update(std::move(update)) {} -MapValueUpdate::MapValueUpdate(const MapValueUpdate &) = default; -MapValueUpdate & MapValueUpdate::operator = (const MapValueUpdate &) = default; +MapValueUpdate::MapValueUpdate(const MapValueUpdate &) + : ValueUpdate(), + _key(), + _update() +{ + abort(); // TODO Will never be called, remove +} +MapValueUpdate & +MapValueUpdate::operator = (const MapValueUpdate &) { + abort(); // TODO Will never be called, remove + return *this; +} MapValueUpdate::~MapValueUpdate() = default; bool @@ -122,7 +132,9 @@ MapValueUpdate::printXml(XmlOutputStream& xos) const { xos << XmlTag("map") << XmlTag("value") << *_key << XmlEndTag() - << XmlTag("update") << *_update << XmlEndTag() + << XmlTag("update"); + _update->printXml(xos); + xos << XmlEndTag() << XmlEndTag(); } diff --git a/document/src/vespa/document/update/mapvalueupdate.h b/document/src/vespa/document/update/mapvalueupdate.h index 8ec57473aa5..722255dd8d6 100644 --- a/document/src/vespa/document/update/mapvalueupdate.h +++ b/document/src/vespa/document/update/mapvalueupdate.h @@ -17,15 +17,6 @@ namespace document { class MapValueUpdate : public ValueUpdate { - FieldValue::CP _key; // The field value this update is mapping to. - ValueUpdate::CP _update; //The update to apply to the value member of this. - - // Used by ValueUpdate's static factory function - // Private because it generates an invalid object. - friend class ValueUpdate; - MapValueUpdate() : ValueUpdate(), _key(), _update() {} - - ACCEPT_UPDATE_VISITOR; public: /** @@ -35,13 +26,13 @@ public: * @param key The identifier of the field value to be updated. * @param update The update to map to apply to the field value of this. */ - MapValueUpdate(const FieldValue& key, const ValueUpdate& update); + MapValueUpdate(const FieldValue& key, std::unique_ptr<ValueUpdate> update); MapValueUpdate(const MapValueUpdate &); MapValueUpdate & operator = (const MapValueUpdate &); MapValueUpdate(MapValueUpdate &&) = default; MapValueUpdate & operator = (MapValueUpdate &&) = default; - ~MapValueUpdate(); + ~MapValueUpdate() override; bool operator==(const ValueUpdate& other) const override; @@ -52,24 +43,13 @@ public: ValueUpdate& getUpdate() { return *_update; } /** - * Sets the identifier of the field value to update. - * - * @param key The field value identifier. - * @return A pointer to this. - */ - MapValueUpdate& setKey(const FieldValue& key) { - _key.reset(key.clone()); - return *this; - } - - /** * Sets the update to apply to the value update of this. * * @param update The value update. * @return A pointer to this. */ - MapValueUpdate& setUpdate(const ValueUpdate& update) { - _update.reset(update.clone()); + MapValueUpdate& setUpdate(std::unique_ptr<ValueUpdate> update) { + _update = std::move(update); return *this; } @@ -78,10 +58,18 @@ public: void printXml(XmlOutputStream& xos) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream& buffer) override; - MapValueUpdate* clone() const override { return new MapValueUpdate(*this); } DECLARE_IDENTIFIABLE(MapValueUpdate); +private: + std::unique_ptr<FieldValue> _key; // The field value this update is mapping to. + std::unique_ptr<ValueUpdate> _update; //The update to apply to the value member of this. + + // Used by ValueUpdate's static factory function + // Private because it generates an invalid object. + friend class ValueUpdate; + MapValueUpdate() : ValueUpdate(), _key(), _update() {} + ACCEPT_UPDATE_VISITOR; }; } diff --git a/document/src/vespa/document/update/removevalueupdate.h b/document/src/vespa/document/update/removevalueupdate.h index 7fc1ee4a4e1..0eea8f69da7 100644 --- a/document/src/vespa/document/update/removevalueupdate.h +++ b/document/src/vespa/document/update/removevalueupdate.h @@ -35,23 +35,11 @@ public: */ const FieldValue& getKey() const { return *_key; } - /** - * Sets the field value to remove during this update. - * - * @param The new field value. - * @return A pointer to this. - */ - RemoveValueUpdate& setKey(const FieldValue& key) { - _key.reset(key.clone()); - return *this; - } - void checkCompatibility(const Field& field) const override; bool applyTo(FieldValue& value) const override; void printXml(XmlOutputStream& xos) const override; void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream& buffer) override; - RemoveValueUpdate* clone() const override { return new RemoveValueUpdate(*this); } DECLARE_IDENTIFIABLE(RemoveValueUpdate); diff --git a/document/src/vespa/document/update/tensor_add_update.cpp b/document/src/vespa/document/update/tensor_add_update.cpp index 1f8aed2d8b4..2d6b6e1d658 100644 --- a/document/src/vespa/document/update/tensor_add_update.cpp +++ b/document/src/vespa/document/update/tensor_add_update.cpp @@ -145,10 +145,4 @@ TensorAddUpdate::deserialize(const DocumentTypeRepo &repo, const DataType &type, deserializer.read(*_tensor); } -TensorAddUpdate* -TensorAddUpdate::clone() const -{ - return new TensorAddUpdate(*this); -} - } diff --git a/document/src/vespa/document/update/tensor_add_update.h b/document/src/vespa/document/update/tensor_add_update.h index 259226b380c..7f2228bff41 100644 --- a/document/src/vespa/document/update/tensor_add_update.h +++ b/document/src/vespa/document/update/tensor_add_update.h @@ -35,7 +35,6 @@ public: void printXml(XmlOutputStream &xos) const override; void print(std::ostream &out, bool verbose, const std::string &indent) const override; void deserialize(const DocumentTypeRepo &repo, const DataType &type, nbostream &stream) override; - TensorAddUpdate* clone() const override; DECLARE_IDENTIFIABLE(TensorAddUpdate); }; diff --git a/document/src/vespa/document/update/tensor_modify_update.cpp b/document/src/vespa/document/update/tensor_modify_update.cpp index 49ea57f28c1..77fd5059b0c 100644 --- a/document/src/vespa/document/update/tensor_modify_update.cpp +++ b/document/src/vespa/document/update/tensor_modify_update.cpp @@ -253,10 +253,4 @@ TensorModifyUpdate::deserialize(const DocumentTypeRepo &repo, const DataType &ty verifyCellsTensorIsSparse(_tensor->getAsTensorPtr()); } -TensorModifyUpdate* -TensorModifyUpdate::clone() const -{ - return new TensorModifyUpdate(*this); -} - } diff --git a/document/src/vespa/document/update/tensor_modify_update.h b/document/src/vespa/document/update/tensor_modify_update.h index b721ec70a0b..b6d339a36cf 100644 --- a/document/src/vespa/document/update/tensor_modify_update.h +++ b/document/src/vespa/document/update/tensor_modify_update.h @@ -49,7 +49,6 @@ public: void printXml(XmlOutputStream &xos) const override; void print(std::ostream &out, bool verbose, const std::string &indent) const override; void deserialize(const DocumentTypeRepo &repo, const DataType &type, nbostream &stream) override; - TensorModifyUpdate* clone() const override; DECLARE_IDENTIFIABLE(TensorModifyUpdate); }; diff --git a/document/src/vespa/document/update/tensor_remove_update.cpp b/document/src/vespa/document/update/tensor_remove_update.cpp index f2d11ef8234..69b8b898ec5 100644 --- a/document/src/vespa/document/update/tensor_remove_update.cpp +++ b/document/src/vespa/document/update/tensor_remove_update.cpp @@ -200,10 +200,4 @@ TensorRemoveUpdate::deserialize(const DocumentTypeRepo &repo, const DataType &ty _tensor->assignDeserialized(std::move(tensor)); } -TensorRemoveUpdate * -TensorRemoveUpdate::clone() const -{ - return new TensorRemoveUpdate(*this); -} - } diff --git a/document/src/vespa/document/update/tensor_remove_update.h b/document/src/vespa/document/update/tensor_remove_update.h index bd2817899f5..9b4ea17c4d9 100644 --- a/document/src/vespa/document/update/tensor_remove_update.h +++ b/document/src/vespa/document/update/tensor_remove_update.h @@ -40,7 +40,6 @@ public: void printXml(XmlOutputStream &xos) const override; void print(std::ostream &out, bool verbose, const std::string &indent) const override; void deserialize(const DocumentTypeRepo &repo, const DataType &type, nbostream &stream) override; - TensorRemoveUpdate* clone() const override; DECLARE_IDENTIFIABLE(TensorRemoveUpdate); }; diff --git a/document/src/vespa/document/update/valueupdate.cpp b/document/src/vespa/document/update/valueupdate.cpp index 8a023fb9ef2..8fc85e8858a 100644 --- a/document/src/vespa/document/update/valueupdate.cpp +++ b/document/src/vespa/document/update/valueupdate.cpp @@ -26,4 +26,10 @@ ValueUpdate::createInstance(const DocumentTypeRepo& repo, const DataType& type, } } +std::ostream& +operator<<(std::ostream& out, const ValueUpdate& p) { + p.print(out, false, ""); + return out; +} + } diff --git a/document/src/vespa/document/update/valueupdate.h b/document/src/vespa/document/update/valueupdate.h index ec903c1adc1..0b52fe46ac9 100644 --- a/document/src/vespa/document/update/valueupdate.h +++ b/document/src/vespa/document/update/valueupdate.h @@ -19,10 +19,9 @@ #pragma once #include "updatevisitor.h" -#include <vespa/document/util/printable.h> #include <vespa/document/util/identifiableid.h> #include <vespa/vespalib/objects/nbostream.h> -#include <vespa/vespalib/util/xmlserializable.h> +#include <vespa/vespalib/util/xmlstream.h> namespace document { @@ -31,14 +30,11 @@ class Field; class FieldValue; class DataType; -class ValueUpdate : public vespalib::Identifiable, - public Printable, - public vespalib::xml::XmlSerializable +class ValueUpdate : public vespalib::Identifiable { protected: using nbostream = vespalib::nbostream; public: - using CP = vespalib::CloneablePtr<ValueUpdate>; using XmlOutputStream = vespalib::xml::XmlOutputStream; /** @@ -62,11 +58,6 @@ public: TensorRemoveUpdate = IDENTIFIABLE_CLASSID(TensorRemoveUpdate) }; - ValueUpdate() - : Printable(), XmlSerializable() {} - - virtual ~ValueUpdate() = default; - virtual bool operator==(const ValueUpdate&) const = 0; bool operator != (const ValueUpdate & rhs) const { return ! (*this == rhs); } @@ -85,8 +76,6 @@ public: */ virtual bool applyTo(FieldValue& value) const = 0; - virtual ValueUpdate* clone() const = 0; - /** * Deserializes the given stream into an instance of an update object. * @@ -105,8 +94,13 @@ public: */ virtual void accept(UpdateVisitor &visitor) const = 0; + virtual void print(std::ostream& out, bool verbose, const std::string& indent) const = 0; + virtual void printXml(XmlOutputStream& out) const = 0; + DECLARE_IDENTIFIABLE_ABSTRACT(ValueUpdate); }; +std::ostream& operator<<(std::ostream& out, const ValueUpdate & p); + } diff --git a/document/src/vespa/document/util/printable.cpp b/document/src/vespa/document/util/printable.cpp index 8d9135af285..c8043bfce04 100644 --- a/document/src/vespa/document/util/printable.cpp +++ b/document/src/vespa/document/util/printable.cpp @@ -23,7 +23,7 @@ void Printable::print(std::ostream& out, const std::string& indent) const { } std::ostream& operator<<(std::ostream& out, const Printable& p) { - p.print(out); + p.print(out, false, ""); return out; } diff --git a/fnet/src/vespa/fnet/databuffer.cpp b/fnet/src/vespa/fnet/databuffer.cpp index f6a7e1c25e5..7820ae2dfcf 100644 --- a/fnet/src/vespa/fnet/databuffer.cpp +++ b/fnet/src/vespa/fnet/databuffer.cpp @@ -94,7 +94,9 @@ FNET_DataBuffer::Pack(uint32_t needbytes) bufsize *= 2; Alloc newBuf(Alloc::alloc(bufsize)); - memcpy(newBuf.get(), _datapt, GetDataLen()); + if (_datapt != nullptr) { + memcpy(newBuf.get(), _datapt, GetDataLen()); + } _ownedBuf.swap(newBuf); _bufstart = static_cast<char *>(_ownedBuf.get()); _freept = _bufstart + GetDataLen(); diff --git a/fnet/src/vespa/fnet/frt/values.cpp b/fnet/src/vespa/fnet/frt/values.cpp index dedefd64323..85979e2af41 100644 --- a/fnet/src/vespa/fnet/frt/values.cpp +++ b/fnet/src/vespa/fnet/frt/values.cpp @@ -984,7 +984,7 @@ FRT_Values::DecodeBig(FNET_DataBuffer *src, uint32_t len) } if (len != 0) goto error; - if (strncmp(typeString, _typeString, numValues) != 0) goto error; + if ((numValues > 0) && strncmp(typeString, _typeString, numValues) != 0) goto error; return true; error: @@ -1385,6 +1385,9 @@ FRT_Values::EncodeBig(FNET_DataBuffer *dst) const char *p = _typeString; dst->WriteInt32Fast(numValues); + if (numValues == 0) { + return; // p may be nullptr, don't try to write what does not exist. + } dst->WriteBytesFast(p, numValues); for (uint32_t i = 0; i < numValues; i++, p++) { diff --git a/parent/pom.xml b/parent/pom.xml index c16f812f80c..c53e9aa1f0e 100644 --- a/parent/pom.xml +++ b/parent/pom.xml @@ -92,14 +92,6 @@ <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-dependency-plugin</artifactId> <version>${maven-dependency-plugin.version}</version> - <dependencies> - <!-- TODO: remove when upgrading to 3.1.2 --> - <dependency> - <groupId>org.apache.maven.shared</groupId> - <artifactId>maven-dependency-analyzer</artifactId> - <version>1.11.1</version> - </dependency> - </dependencies> </plugin> <plugin> <groupId>org.apache.maven.plugins</groupId> diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index 95d7822e17c..2a5b4b48cfa 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -665,8 +665,7 @@ TEST_F(ConformanceTest, testPutOlderDocumentVersion) EXPECT_TRUE(info2.getUsedSize() >= info1.getDocumentSize()); } - GetResult gr = spi->get(bucket, document::AllFields(), doc1->getId(), - context); + GetResult gr = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, gr.getErrorCode()); EXPECT_EQ(Timestamp(5), gr.getTimestamp()); @@ -684,8 +683,7 @@ TEST_F(ConformanceTest, testPutDuplicate) Bucket bucket(makeSpiBucket(BucketId(8, 0x01))); Document::SP doc1 = testDocMan.createRandomDocumentAtLocation(0x01, 1); spi->createBucket(bucket, context); - EXPECT_EQ(Result(), - spi->put(bucket, Timestamp(3), doc1, context)); + EXPECT_EQ(Result(), spi->put(bucket, Timestamp(3), doc1, context)); BucketChecksum checksum; { @@ -693,8 +691,7 @@ TEST_F(ConformanceTest, testPutDuplicate) EXPECT_EQ(1, (int)info.getDocumentCount()); checksum = info.getChecksum(); } - EXPECT_EQ(Result(), - spi->put(bucket, Timestamp(3), doc1, context)); + EXPECT_EQ(Result(), spi->put(bucket, Timestamp(3), doc1, context)); { const BucketInfo info = spi->getBucketInfo(bucket).getBucketInfo(); @@ -730,10 +727,7 @@ TEST_F(ConformanceTest, testRemove) } // Add a remove entry - RemoveResult result2 = spi->remove(bucket, - Timestamp(5), - doc1->getId(), - context); + RemoveResult result2 = spi->remove(bucket, Timestamp(5), doc1->getId(), context); { const BucketInfo info = spi->getBucketInfo(bucket).getBucketInfo(); @@ -753,10 +747,7 @@ TEST_F(ConformanceTest, testRemove) } // Result tagged as document not found - RemoveResult result3 = spi->remove(bucket, - Timestamp(7), - doc1->getId(), - context); + RemoveResult result3 = spi->remove(bucket, Timestamp(7), doc1->getId(), context); { const BucketInfo info = spi->getBucketInfo(bucket).getBucketInfo(); @@ -769,10 +760,7 @@ TEST_F(ConformanceTest, testRemove) EXPECT_TRUE(!result4.hasError()); - RemoveResult result5 = spi->remove(bucket, - Timestamp(9), - doc1->getId(), - context); + RemoveResult result5 = spi->remove(bucket, Timestamp(9), doc1->getId(), context); { const BucketInfo info = spi->getBucketInfo(bucket).getBucketInfo(); @@ -782,10 +770,7 @@ TEST_F(ConformanceTest, testRemove) EXPECT_TRUE(!result5.hasError()); } - GetResult getResult = spi->get(bucket, - document::AllFields(), - doc1->getId(), - context); + GetResult getResult = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, getResult.getErrorCode()); EXPECT_EQ(Timestamp(9), getResult.getTimestamp()); @@ -843,10 +828,7 @@ TEST_F(ConformanceTest, testRemoveMerge) // Remove a document that does not exist { - RemoveResult removeResult = spi->remove(bucket, - Timestamp(10), - removeId, - context); + RemoveResult removeResult = spi->remove(bucket, Timestamp(10), removeId, context); EXPECT_EQ(Result::ErrorType::NONE, removeResult.getErrorCode()); EXPECT_EQ(false, removeResult.wasFound()); } @@ -869,10 +851,7 @@ TEST_F(ConformanceTest, testRemoveMerge) } // Add a _newer_ remove for the same document ID we already removed { - RemoveResult removeResult = spi->remove(bucket, - Timestamp(11), - removeId, - context); + RemoveResult removeResult = spi->remove(bucket, Timestamp(11), removeId, context); EXPECT_EQ(Result::ErrorType::NONE, removeResult.getErrorCode()); EXPECT_EQ(false, removeResult.wasFound()); } @@ -896,10 +875,7 @@ TEST_F(ConformanceTest, testRemoveMerge) // It may or may not be present in a subsequent iteration, but the // newest timestamp must still be present. { - RemoveResult removeResult = spi->remove(bucket, - Timestamp(7), - removeId, - context); + RemoveResult removeResult = spi->remove(bucket, Timestamp(7), removeId, context); EXPECT_EQ(Result::ErrorType::NONE, removeResult.getErrorCode()); EXPECT_EQ(false, removeResult.wasFound()); } @@ -934,32 +910,24 @@ TEST_F(ConformanceTest, testUpdate) const document::DocumentType *docType( testDocMan.getTypeRepo().getDocumentType("testdoctype1")); document::DocumentUpdate::SP update(new DocumentUpdate(testDocMan.getTypeRepo(), *docType, doc1->getId())); - std::shared_ptr<document::AssignValueUpdate> assignUpdate(new document::AssignValueUpdate(document::IntFieldValue(42))); - document::FieldUpdate fieldUpdate(docType->getField("headerval")); - fieldUpdate.addUpdate(*assignUpdate); - update->addUpdate(fieldUpdate); + update->addUpdate(document::FieldUpdate(docType->getField("headerval")).addUpdate(std::make_unique<document::AssignValueUpdate>(document::IntFieldValue(42)))); { - UpdateResult result = spi->update(bucket, Timestamp(3), update, - context); + UpdateResult result = spi->update(bucket, Timestamp(3), update, context); EXPECT_EQ(Result(), Result(result)); EXPECT_EQ(Timestamp(0), result.getExistingTimestamp()); } spi->put(bucket, Timestamp(3), doc1, context); { - UpdateResult result = spi->update(bucket, Timestamp(4), update, - context); + UpdateResult result = spi->update(bucket, Timestamp(4), update, context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(3), result.getExistingTimestamp()); } { - GetResult result = spi->get(bucket, - document::AllFields(), - doc1->getId(), - context); + GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(4), result.getTimestamp()); @@ -972,10 +940,7 @@ TEST_F(ConformanceTest, testUpdate) spi->remove(bucket, Timestamp(5), doc1->getId(), context); { - GetResult result = spi->get(bucket, - document::AllFields(), - doc1->getId(), - context); + GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(5), result.getTimestamp()); @@ -984,8 +949,7 @@ TEST_F(ConformanceTest, testUpdate) } { - UpdateResult result = spi->update(bucket, Timestamp(6), update, - context); + UpdateResult result = spi->update(bucket, Timestamp(6), update, context); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(0), result.getExistingTimestamp()); diff --git a/screwdriver.yaml b/screwdriver.yaml index b23dfc50e70..9570991ff26 100644 --- a/screwdriver.yaml +++ b/screwdriver.yaml @@ -183,6 +183,8 @@ jobs: - *save-cache publish-release: + image: docker.io/vespaengine/vespa-build-centos-stream8:latest + annotations: screwdriver.cd/cpu: 7 screwdriver.cd/ram: 16 diff --git a/searchcore/src/apps/vespa-transactionlog-inspect/vespa-transactionlog-inspect.cpp b/searchcore/src/apps/vespa-transactionlog-inspect/vespa-transactionlog-inspect.cpp index f00702f2785..4d7f32241f9 100644 --- a/searchcore/src/apps/vespa-transactionlog-inspect/vespa-transactionlog-inspect.cpp +++ b/searchcore/src/apps/vespa-transactionlog-inspect/vespa-transactionlog-inspect.cpp @@ -232,9 +232,14 @@ private: toPrint.printXml(out); std::cout << std::endl; } + void printXml(const document::DocumentUpdate &toPrint) { + vespalib::xml::XmlOutputStream out(std::cout); + toPrint.printXml(out); + std::cout << std::endl; + } - void printText(const document::Printable &toPrint) { - toPrint.print(std::cout, _verbose); + void printText(const document::DocumentUpdate &toPrint) { + toPrint.print(std::cout, _verbose, ""); std::cout << std::endl; } @@ -265,7 +270,7 @@ public: } void replay(const UpdateOperation &op) override { print(op); - if (op.getUpdate().get() != NULL) { + if (op.getUpdate()) { if (_printXml) { printXml(*op.getUpdate()); } else { diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp index fdf75bbd726..51bc60d3783 100644 --- a/searchcore/src/tests/proton/attribute/attribute_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp @@ -483,9 +483,9 @@ TEST_F(AttributeWriterTest, handles_update) const document::DocumentType &dt(idb.getDocumentType()); DocumentUpdate upd(*idb.getDocumentTypeRepo(), dt, DocumentId("id:ns:searchdocument::1")); upd.addUpdate(FieldUpdate(upd.getType().getField("a1")) - .addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 5))); + .addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 5))); upd.addUpdate(FieldUpdate(upd.getType().getField("a2")) - .addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 10))); + .addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 10))); DummyFieldUpdateCallback onUpdate; update(2, upd, 1, onUpdate); @@ -528,7 +528,7 @@ TEST_F(AttributeWriterTest, handles_predicate_update) DocumentUpdate upd(*idb.getDocumentTypeRepo(), dt, DocumentId("id:ns:searchdocument::1")); PredicateFieldValue new_value(builder.feature("foo").value("bar").build()); upd.addUpdate(FieldUpdate(upd.getType().getField("a1")) - .addUpdate(AssignValueUpdate(new_value))); + .addUpdate(std::make_unique<AssignValueUpdate>(new_value))); PredicateIndex &index = static_cast<PredicateAttribute &>(*a1).getIndex(); EXPECT_EQ(1u, index.getZeroConstraintDocs().size()); @@ -729,7 +729,7 @@ TEST_F(AttributeWriterTest, handles_tensor_assign_update) TensorFieldValue new_value(xySparseTensorDataType); new_value = SimpleValue::from_value(*new_tensor); upd.addUpdate(FieldUpdate(upd.getType().getField("a1")) - .addUpdate(AssignValueUpdate(new_value))); + .addUpdate(std::make_unique<AssignValueUpdate>(new_value))); DummyFieldUpdateCallback onUpdate; update(2, upd, 1, onUpdate); EXPECT_EQ(2u, a1->getNumDocs()); @@ -938,7 +938,7 @@ public: TensorDataType tensor_type(vespalib::eval::ValueType::from_spec(dense_tensor)); TensorFieldValue tensor_value(tensor_type); tensor_value= SimpleValue::from_value(*tensor); - upd->addUpdate(FieldUpdate(upd->getType().getField("a1")).addUpdate(AssignValueUpdate(tensor_value))); + upd->addUpdate(FieldUpdate(upd->getType().getField("a1")).addUpdate(std::make_unique<AssignValueUpdate>(tensor_value))); return upd; } void expect_shared_executor_tasks(size_t exp_accepted_tasks) { diff --git a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp index 924abc81711..20584a2a1fb 100644 --- a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp +++ b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp @@ -97,29 +97,28 @@ struct Fixture { { } - void applyValueUpdate(AttributeVector & vec, uint32_t docId, const ValueUpdate & upd) { + void applyValueUpdate(AttributeVector & vec, uint32_t docId, std::unique_ptr<ValueUpdate> upd) { FieldUpdate fupd(docType->getField(vec.getName())); - fupd.addUpdate(upd); + fupd.addUpdate(std::move(upd)); search::AttributeUpdater::handleUpdate(vec, docId, fupd); vec.commit(); } void applyArrayUpdates(AttributeVector & vec, const FieldValue & assign, const FieldValue & first, const FieldValue & second) { - applyValueUpdate(vec, 0, AssignValueUpdate(assign)); - applyValueUpdate(vec, 1, AddValueUpdate(second)); - applyValueUpdate(vec, 2, RemoveValueUpdate(first)); - applyValueUpdate(vec, 3, ClearValueUpdate()); + applyValueUpdate(vec, 0, std::make_unique<AssignValueUpdate>(assign)); + applyValueUpdate(vec, 1, std::make_unique<AddValueUpdate>(second)); + applyValueUpdate(vec, 2, std::make_unique<RemoveValueUpdate>(first)); + applyValueUpdate(vec, 3, std::make_unique<ClearValueUpdate>()); } void applyWeightedSetUpdates(AttributeVector & vec, const FieldValue & assign, const FieldValue & first, const FieldValue & second) { - applyValueUpdate(vec, 0, AssignValueUpdate(assign)); - applyValueUpdate(vec, 1, AddValueUpdate(second, 20)); - applyValueUpdate(vec, 2, RemoveValueUpdate(first)); - applyValueUpdate(vec, 3, ClearValueUpdate()); - ArithmeticValueUpdate arithmetic(ArithmeticValueUpdate::Add, 10); - applyValueUpdate(vec, 4, MapValueUpdate(first, arithmetic)); + applyValueUpdate(vec, 0, std::make_unique<AssignValueUpdate>(assign)); + applyValueUpdate(vec, 1, std::make_unique<AddValueUpdate>(second, 20)); + applyValueUpdate(vec, 2, std::make_unique<RemoveValueUpdate>(first)); + applyValueUpdate(vec, 3, std::make_unique<ClearValueUpdate>()); + applyValueUpdate(vec, 4, std::make_unique<MapValueUpdate>(first, std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 10))); } }; @@ -207,9 +206,9 @@ TEST_F("require that single attributes are updated", Fixture) AttributePtr vec = create<int32_t, IntegerAttribute>(3, 32, 0, "in1/int", Config(bt, ct)); - f.applyValueUpdate(*vec, 0, AssignValueUpdate(IntFieldValue(64))); - f.applyValueUpdate(*vec, 1, ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 10)); - f.applyValueUpdate(*vec, 2, ClearValueUpdate()); + f.applyValueUpdate(*vec, 0, std::make_unique<AssignValueUpdate>(IntFieldValue(64))); + f.applyValueUpdate(*vec, 1, std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 10)); + f.applyValueUpdate(*vec, 2, std::make_unique<ClearValueUpdate>()); EXPECT_EQUAL(3u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector<WeightedInt>{WeightedInt(64)})); EXPECT_TRUE(check(vec, 1, std::vector<WeightedInt>{WeightedInt(42)})); @@ -221,9 +220,9 @@ TEST_F("require that single attributes are updated", Fixture) "in1/float", Config(bt, ct)); - f.applyValueUpdate(*vec, 0, AssignValueUpdate(FloatFieldValue(77.7f))); - f.applyValueUpdate(*vec, 1, ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 10)); - f.applyValueUpdate(*vec, 2, ClearValueUpdate()); + f.applyValueUpdate(*vec, 0, std::make_unique<AssignValueUpdate>(FloatFieldValue(77.7f))); + f.applyValueUpdate(*vec, 1, std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 10)); + f.applyValueUpdate(*vec, 2, std::make_unique<ClearValueUpdate>()); EXPECT_EQUAL(3u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector<WeightedFloat>{WeightedFloat(77.7f)})); EXPECT_TRUE(check(vec, 1, std::vector<WeightedFloat>{WeightedFloat(65.5f)})); @@ -235,8 +234,8 @@ TEST_F("require that single attributes are updated", Fixture) "in1/string", Config(bt, ct)); - f.applyValueUpdate(*vec, 0, AssignValueUpdate(StringFieldValue("second"))); - f.applyValueUpdate(*vec, 2, ClearValueUpdate()); + f.applyValueUpdate(*vec, 0, std::make_unique<AssignValueUpdate>(StringFieldValue("second"))); + f.applyValueUpdate(*vec, 2, std::make_unique<ClearValueUpdate>()); EXPECT_EQUAL(3u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector<WeightedString>{WeightedString("second")})); EXPECT_TRUE(check(vec, 1, std::vector<WeightedString>{WeightedString("first")})); @@ -255,8 +254,8 @@ TEST_F("require that single attributes are updated", Fixture) asReferenceAttribute(*vec).update(docId, toGid(doc1)); } vec->commit(); - f.applyValueUpdate(*vec, 0, AssignValueUpdate(ReferenceFieldValue(dynamic_cast<const ReferenceDataType &>(f.docType->getField("ref").getDataType()), DocumentId(doc2)))); - f.applyValueUpdate(*vec, 2, ClearValueUpdate()); + f.applyValueUpdate(*vec, 0, std::make_unique<AssignValueUpdate>(ReferenceFieldValue(dynamic_cast<const ReferenceDataType &>(f.docType->getField("ref").getDataType()), DocumentId(doc2)))); + f.applyValueUpdate(*vec, 2, std::make_unique<ClearValueUpdate>()); EXPECT_EQUAL(3u, vec->getNumDocs()); TEST_DO(assertRef(*vec, doc2, 0)); TEST_DO(assertRef(*vec, doc1, 1)); @@ -453,7 +452,7 @@ TEST_F("require that tensor modify update is applied", { f.setTensor(TensorSpec(f.type).add({{"x", 0}}, 3).add({{"x", 1}}, 5)); f.applyValueUpdate(*f.attribute, 1, - TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, + std::make_unique<TensorModifyUpdate>(TensorModifyUpdate::Operation::REPLACE, makeTensorFieldValue(TensorSpec("tensor(x{})").add({{"x", "0"}}, 7)))); f.assertTensor(TensorSpec(f.type).add({{"x", 0}}, 7).add({{"x", 1}}, 5)); } @@ -463,7 +462,7 @@ TEST_F("require that tensor add update is applied", { f.setTensor(TensorSpec(f.type).add({{"x", "a"}}, 2)); f.applyValueUpdate(*f.attribute, 1, - TensorAddUpdate(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "a"}}, 3)))); + std::make_unique<TensorAddUpdate>(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "a"}}, 3)))); f.assertTensor(TensorSpec(f.type).add({{"x", "a"}}, 3)); } @@ -471,7 +470,7 @@ TEST_F("require that tensor add update to non-existing tensor creates empty tens TensorFixture<SerializedFastValueAttribute>("tensor(x{})", "sparse_tensor")) { f.applyValueUpdate(*f.attribute, 1, - TensorAddUpdate(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "a"}}, 3)))); + std::make_unique<TensorAddUpdate>(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "a"}}, 3)))); f.assertTensor(TensorSpec(f.type).add({{"x", "a"}}, 3)); } @@ -480,7 +479,7 @@ TEST_F("require that tensor remove update is applied", { f.setTensor(TensorSpec(f.type).add({{"x", "a"}}, 2).add({{"x", "b"}}, 3)); f.applyValueUpdate(*f.attribute, 1, - TensorRemoveUpdate(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "b"}}, 1)))); + std::make_unique<TensorRemoveUpdate>(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "b"}}, 1)))); f.assertTensor(TensorSpec(f.type).add({{"x", "a"}}, 2)); } diff --git a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp index 4040d69270f..6cd92dea516 100644 --- a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp @@ -345,10 +345,7 @@ struct UpdateContext { } else { fieldValue->assign(document::StringFieldValue("new value")); } - document::AssignValueUpdate assignValueUpdate(*fieldValue); - document::FieldUpdate fieldUpdate(field); - fieldUpdate.addUpdate(assignValueUpdate); - update->addUpdate(fieldUpdate); + update->addUpdate(document::FieldUpdate(field).addUpdate(std::make_unique<document::AssignValueUpdate>(*fieldValue))); } }; @@ -773,11 +770,11 @@ TEST_F("require that update with a fieldpath update will be rejected", SchemaCon TEST_F("require that all value updates will be inspected before rejected", SchemaContext) { const DocumentType *docType = f.getRepo()->getDocumentType(f.getDocType().getName()); auto docUpdate = std::make_unique<DocumentUpdate>(*f.getRepo(), *docType, DocumentId("id:ns:" + docType->getName() + "::1")); - docUpdate->addUpdate(FieldUpdate(docType->getField("i1")).addUpdate(ClearValueUpdate())); + docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique<ClearValueUpdate>()))); EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); - docUpdate->addUpdate(FieldUpdate(docType->getField("i1")).addUpdate(ClearValueUpdate())); + docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique<ClearValueUpdate>()))); EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); - docUpdate->addUpdate(FieldUpdate(docType->getField("i1")).addUpdate(AssignValueUpdate(StringFieldValue()))); + docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue())))); EXPECT_TRUE(FeedRejectHelper::mustReject(*docUpdate)); } diff --git a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp b/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp index d79b46b2e08..40332157a8d 100644 --- a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp +++ b/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp @@ -16,7 +16,6 @@ #include <vespa/searchlib/query/base.h> #include <persistence/spi/types.h> #include <vespa/document/base/documentid.h> -#include <vespa/document/config/documenttypes_config_fwd.h> #include <vespa/document/datatype/datatype.h> #include <vespa/document/fieldvalue/document.h> #include <vespa/document/update/documentupdate.h> @@ -123,7 +122,7 @@ public: auto makeUpdate() { auto upd(std::make_shared<DocumentUpdate>(*_repo, _docType, docId)); upd->addUpdate(FieldUpdate(upd->getType().getField("string")). - addUpdate(AssignValueUpdate(StringFieldValue("newval")))); + addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue("newval")))); return upd; } auto makeDoc() { diff --git a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp index 95593307ba2..7f35e05dfb6 100644 --- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp +++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp @@ -25,6 +25,8 @@ using document::BucketSpace; using document::Document; using document::DocumentId; using document::DocumentType; +using document::DocumentTypeRepo; +using document::DocumentUpdate; using document::test::makeBucketSpace; using search::DocumentMetaData; using storage::spi::Bucket; @@ -56,19 +58,19 @@ createDocType(const vespalib::string &name, int32_t id) } -document::Document::SP +Document::SP createDoc(const DocumentType &docType, const DocumentId &docId) { - return std::make_shared<document::Document>(docType, docId); + return std::make_shared<Document>(docType, docId); } -document::DocumentUpdate::SP +DocumentUpdate::SP createUpd(const DocumentType& docType, const DocumentId &docId) { - static std::vector<std::unique_ptr<document::DocumentTypeRepo>> repoList; - repoList.emplace_back(std::make_unique<document::DocumentTypeRepo>(docType)); - return std::make_shared<document::DocumentUpdate>(*repoList.back(), docType, docId); + static std::vector<std::unique_ptr<DocumentTypeRepo>> repoList; + repoList.emplace_back(std::make_unique<DocumentTypeRepo>(docType)); + return std::make_shared<DocumentUpdate>(*repoList.back(), docType, docId); } storage::spi::ClusterState @@ -105,14 +107,14 @@ createClusterState(const storage::lib::State& nodeState = storage::lib::State::U struct MyDocumentRetriever : DocumentRetrieverBaseForTest { - document::DocumentTypeRepo repo; + DocumentTypeRepo repo; const Document *document; Timestamp timestamp; DocumentId &last_doc_id; MyDocumentRetriever(const Document *d, Timestamp ts, DocumentId &last_id) : repo(), document(d), timestamp(ts), last_doc_id(last_id) {} - const document::DocumentTypeRepo &getDocumentTypeRepo() const override { + const DocumentTypeRepo &getDocumentTypeRepo() const override { return repo; } void getBucketMetaData(const storage::spi::Bucket &, search::DocumentMetaData::Vector &v) const override { @@ -123,11 +125,11 @@ struct MyDocumentRetriever : DocumentRetrieverBaseForTest { DocumentMetaData getDocumentMetaData(const DocumentId &id) const override { last_doc_id = id; if (document != nullptr) { - return DocumentMetaData(1, timestamp, document::BucketId(1), document->getId().getGlobalId()); + return DocumentMetaData(1, timestamp, BucketId(1), document->getId().getGlobalId()); } return DocumentMetaData(); } - document::Document::UP getFullDocument(search::DocumentIdT) const override { + Document::UP getFullDocument(search::DocumentIdT) const override { if (document != nullptr) { return Document::UP(document->clone()); } @@ -271,7 +273,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer { resultHandler.handle(BucketIdListResult()); } - void handlePopulateActiveBuckets(document::BucketId::List buckets, IGenericResultHandler &resultHandler) override { + void handlePopulateActiveBuckets(BucketId::List buckets, IGenericResultHandler &resultHandler) override { (void) buckets; resultHandler.handle(Result()); } @@ -317,10 +319,10 @@ DocumentId docId3("id:type3:type3::1"); Document::SP doc1(createDoc(type1, docId1)); Document::SP doc2(createDoc(type2, docId2)); Document::SP doc3(createDoc(type3, docId3)); -document::DocumentUpdate::SP upd1(createUpd(type1, docId1)); -document::DocumentUpdate::SP upd2(createUpd(type2, docId2)); -document::DocumentUpdate::SP upd3(createUpd(type3, docId3)); -document::DocumentUpdate::SP bad_id_upd(createUpd(type1, docId2)); +DocumentUpdate::SP upd1(createUpd(type1, docId1)); +DocumentUpdate::SP upd2(createUpd(type2, docId2)); +DocumentUpdate::SP upd3(createUpd(type3, docId3)); +DocumentUpdate::SP bad_id_upd(createUpd(type1, docId2)); BucketId bckId1(1); BucketId bckId2(2); BucketId bckId3(3); @@ -518,10 +520,8 @@ TEST_F("require that update is rejected if resource limit is reached", SimpleFix DocumentType type(createDocType("type_with_one_string", 1)); document::Field field("string", 1, *document::DataType::STRING); type.addField(field); - document::DocumentUpdate::SP upd = createUpd(type, docId1); - document::FieldUpdate fUpd(field); - fUpd.addUpdate(document::AssignValueUpdate(document::StringFieldValue("new value"))); - upd->addUpdate(fUpd); + DocumentUpdate::SP upd = createUpd(type, docId1); + upd->addUpdate(std::move(document::FieldUpdate(field).addUpdate(std::make_unique<document::AssignValueUpdate>(document::StringFieldValue("new value"))))); EXPECT_EQUAL( Result(Result::ErrorType::RESOURCE_EXHAUSTED, diff --git a/searchcore/src/vespa/searchcore/bmcluster/bm_feed.cpp b/searchcore/src/vespa/searchcore/bmcluster/bm_feed.cpp index 56cea8db741..2dcf18f9da6 100644 --- a/searchcore/src/vespa/searchcore/bmcluster/bm_feed.cpp +++ b/searchcore/src/vespa/searchcore/bmcluster/bm_feed.cpp @@ -1,17 +1,14 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "bm_feed.h" -#include "avg_sampler.h" #include "bm_feed_operation.h" #include "bm_feed_params.h" #include "bm_range.h" #include "bucket_selector.h" -#include "pending_tracker.h" #include "i_bm_feed_handler.h" #include <vespa/document/base/documentid.h> #include <vespa/document/bucket/bucketid.h> #include <vespa/document/datatype/documenttype.h> -#include <vespa/document/fieldset/fieldsets.h> #include <vespa/document/fieldvalue/document.h> #include <vespa/document/fieldvalue/intfieldvalue.h> #include <vespa/document/repo/documenttyperepo.h> @@ -74,7 +71,7 @@ BmFeed::make_document_update(uint32_t n, uint32_t i) const { auto id = make_document_id(n, i); auto document_update = std::make_unique<DocumentUpdate>(*_repo, *_document_type, id); - document_update->addUpdate(FieldUpdate(_field).addUpdate(AssignValueUpdate(IntFieldValue(15)))); + document_update->addUpdate(FieldUpdate(_field).addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(15)))); return document_update; } diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h index 16763ccdc9f..87176721681 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h @@ -135,7 +135,7 @@ private: MetaDataView make_meta_data_view() { return vespalib::ConstArrayRef(&_metaDataStore[0], getCommittedDocIdLimit()); } UnboundMetaDataView acquire_unbound_meta_data_view() const noexcept { return &_metaDataStore.acquire_elem_ref(0); } - UnboundMetaDataView get_unbound_meta_data_view() const noexcept { return &_metaDataStore[0]; } + UnboundMetaDataView get_unbound_meta_data_view() const noexcept { return &_metaDataStore.get_elem_ref(0); } // Called from writer only public: typedef TreeType::Iterator Iterator; diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index 73dd7d2f776..ea8a1649789 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -1514,11 +1514,11 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 7u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 0u); - EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Add, 10)))); - EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 10)))); - EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, ArithVU(ArithVU::Mul, 10)))); - EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, ArithVU(ArithVU::Div, 10)))); - EXPECT_TRUE(ptr->apply(6, MapVU(initFieldValue, AssignValueUpdate(IntFieldValue(70))))); + EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, std::make_unique<ArithVU>(ArithVU::Add, 10)))); + EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, std::make_unique<ArithVU>(ArithVU::Sub, 10)))); + EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, std::make_unique<ArithVU>(ArithVU::Mul, 10)))); + EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, std::make_unique<ArithVU>(ArithVU::Div, 10)))); + EXPECT_TRUE(ptr->apply(6, MapVU(initFieldValue, std::make_unique<AssignValueUpdate>(IntFieldValue(70))))); ptr->commit(); EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 12u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 5u); @@ -1536,7 +1536,7 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(buf[0].getWeight(), 70); // removeifzero - EXPECT_TRUE(ptr->apply(4, MapVU(initFieldValue, ArithVU(ArithVU::Sub, 100)))); + EXPECT_TRUE(ptr->apply(4, MapVU(initFieldValue, std::make_unique<ArithVU>(ArithVU::Sub, 100)))); ptr->commit(); if (removeIfZero) { EXPECT_EQUAL(ptr->get(4, &buf[0], 2), uint32_t(0)); @@ -1548,7 +1548,7 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 6u); // createifnonexistant - EXPECT_TRUE(ptr->apply(5, MapVU(nonExistant, ArithVU(ArithVU::Add, 10)))); + EXPECT_TRUE(ptr->apply(5, MapVU(nonExistant, std::make_unique<ArithVU>(ArithVU::Add, 10)))); ptr->commit(); if (createIfNonExistant) { EXPECT_EQUAL(ptr->get(5, &buf[0], 2), uint32_t(2)); @@ -1568,7 +1568,7 @@ AttributeTest::testMapValueUpdate(const AttributePtr & ptr, BufferType initValue EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 15u); ASSERT_TRUE(vec.append(0, initValue.getValue(), 12345)); EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); - EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, ArithVU(ArithVU::Div, 0)))); + EXPECT_TRUE(ptr->apply(0, MapVU(initFieldValue, std::make_unique<ArithVU>(ArithVU::Div, 0)))); EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 7u); ptr->commit(); diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp index 11ea58bc124..eebef46284b 100644 --- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp +++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping_base.cpp @@ -25,10 +25,11 @@ MultiValueMappingBase::~MultiValueMappingBase() = default; MultiValueMappingBase::RefCopyVector MultiValueMappingBase::getRefCopy(uint32_t size) const { assert(size <= _indices.size()); + auto* indices = &_indices.get_elem_ref(0); // Called from writer only RefCopyVector result; result.reserve(size); for (uint32_t lid = 0; lid < size; ++lid) { - result.push_back(_indices[lid].load_relaxed()); + result.push_back(indices[lid].load_relaxed()); } return result; } diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h index 0d2931a1f04..40823faff8a 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.h @@ -63,11 +63,12 @@ public: * The pointer is only guaranteed to be valid for as long as you hold the attribute guard. **/ MinFeatureHandle getMinFeatureVector() const { - return MinFeatureHandle(&_min_feature[0], getNumDocs()); + const auto* min_feature_vector = &_min_feature.acquire_elem_ref(0); + return MinFeatureHandle(min_feature_vector, getNumDocs()); } const IntervalRange * getIntervalRangeVector() const { - return &_interval_range_vector[0]; + return &_interval_range_vector.acquire_elem_ref(0); } IntervalRange getMaxIntervalRange() const { diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp index a9743990626..2d8d4c4858b 100644 --- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp @@ -337,10 +337,11 @@ ReferenceAttribute::IndicesCopyVector ReferenceAttribute::getIndicesCopy(uint32_t size) const { assert(size <= _indices.size()); + auto* indices = &_indices.get_elem_ref(0); // Called from writer only IndicesCopyVector result; result.reserve(size); for (uint32_t i = 0; i < size; ++i) { - result.push_back(_indices[i].load_relaxed()); + result.push_back(indices[i].load_relaxed()); } return result; } diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp index b5de652fbe2..b671a23bbe7 100644 --- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp @@ -41,10 +41,11 @@ SingleValueEnumAttributeBase::EnumIndexCopyVector SingleValueEnumAttributeBase::getIndicesCopy(uint32_t size) const { assert(size <= _enumIndices.size()); + auto* enum_indices = &_enumIndices.get_elem_ref(0); // Called from writer only EnumIndexCopyVector result; result.reserve(size); for (uint32_t lid = 0; lid < size; ++lid) { - result.push_back(_enumIndices[lid].load_relaxed()); + result.push_back(enum_indices[lid].load_relaxed()); } return result; } diff --git a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp index d03f995f99e..39986c43ac5 100644 --- a/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp +++ b/searchlib/src/vespa/searchlib/common/condensedbitvectors.cpp @@ -71,7 +71,7 @@ private: } bool get(Key key, uint32_t index) const override { assert(key < getKeyCapacity()); - return (_v[index] & (B << key)) != 0; + return (_v.acquire_elem_ref(index) & (B << key)) != 0; } size_t getKeyCapacity() const override { return sizeof(T)*8; } @@ -89,7 +89,7 @@ CondensedBitVectorT<T>::computeCountVector(T mask, CountVector & cv, F func) con size_t i(0); const size_t UNROLL = 2; uint8_t *d = &cv[0]; - const T *v = &_v[0]; + const T *v = &_v.acquire_elem_ref(0); for (const size_t m(cv.size() - (UNROLL - 1)); i < m; i+=UNROLL) { for (size_t j(0); j < UNROLL; j++) { func(d[i+j], countBits(v[i+j] & mask)); @@ -103,8 +103,9 @@ template <typename F> void CondensedBitVectorT<T>::computeTail(T mask, CountVector & cv, F func, size_t i) const { + auto* v = &_v.acquire_elem_ref(0); for (; i < cv.size(); i++) { - func(cv[i], countBits(_v[i] & mask)); + func(cv[i], countBits(v[i] & mask)); } } diff --git a/searchlib/src/vespa/searchlib/docstore/idatastore.h b/searchlib/src/vespa/searchlib/docstore/idatastore.h index fc0eae1d15e..442614dfdae 100644 --- a/searchlib/src/vespa/searchlib/docstore/idatastore.h +++ b/searchlib/src/vespa/searchlib/docstore/idatastore.h @@ -7,6 +7,7 @@ #include <vespa/vespalib/stllike/string.h> #include <vespa/vespalib/util/memoryusage.h> #include <vespa/vespalib/util/time.h> +#include <atomic> #include <vector> namespace vespalib { class DataBuffer; } @@ -172,7 +173,7 @@ public: * Get the number of entries (including removed IDs * or gaps in the local ID sequence) in the data store. */ - uint32_t getDocIdLimit() const { return _docIdLimit; } + uint32_t getDocIdLimit() const { return _docIdLimit.load(std::memory_order_acquire); } /** * Returns the name of the base directory where the data file is stored. @@ -181,16 +182,16 @@ public: protected: void setDocIdLimit(uint32_t docIdLimit) { - _docIdLimit = docIdLimit; + _docIdLimit.store(docIdLimit, std::memory_order_release); } void updateDocIdLimit(uint32_t docIdLimit) { - if (docIdLimit > _docIdLimit) { + if (docIdLimit > _docIdLimit.load(std::memory_order_relaxed)) { setDocIdLimit(docIdLimit); } } private: - uint32_t _docIdLimit; + std::atomic<uint32_t> _docIdLimit; vespalib::string _dirName; }; diff --git a/searchlib/src/vespa/searchlib/predicate/simple_index.h b/searchlib/src/vespa/searchlib/predicate/simple_index.h index 25ba76ce7c4..df3d0686e82 100644 --- a/searchlib/src/vespa/searchlib/predicate/simple_index.h +++ b/searchlib/src/vespa/searchlib/predicate/simple_index.h @@ -88,8 +88,10 @@ public: PostingVectorIterator(const PostingVectorIterator&) = default; PostingVectorIterator& operator=(const PostingVectorIterator&) = default; - explicit PostingVectorIterator(const PostingVector & vector, size_t size) : - _vector(&vector[0]), _size(size) { + explicit PostingVectorIterator(const PostingVector & vector, size_t size) + : _vector(&vector.acquire_elem_ref(0)), + _size(size) + { assert(_size <= vector.size()); linearSeek(1); } diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h index 726abb8141d..06482c075cb 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_graph.h @@ -56,7 +56,7 @@ struct HnswGraph { void trim_node_refs_size(); NodeRef get_node_ref(uint32_t docid) const { - return node_refs[docid].load_relaxed(); + return node_refs.get_elem_ref(docid).load_relaxed(); // Called from writer only } NodeRef acquire_node_ref(uint32_t docid) const { diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp index 112396fd3fb..89f54eacc09 100644 --- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp +++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp @@ -269,10 +269,11 @@ TensorAttribute::getRefCopy() const { uint32_t size = getCommittedDocIdLimit(); assert(size <= _refVector.size()); + auto* ref_vector = &_refVector.get_elem_ref(0); // Called from writer only RefCopyVector result; result.reserve(size); for (uint32_t i = 0; i < size; ++i) { - result.push_back(_refVector[i].load_relaxed()); + result.push_back(ref_vector[i].load_relaxed()); } return result; } diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp index 27855091257..0e5372043ab 100644 --- a/storage/src/tests/distributor/externaloperationhandlertest.cpp +++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp @@ -7,7 +7,6 @@ #include <vespa/document/update/assignvalueupdate.h> #include <vespa/document/update/documentupdate.h> #include <vespa/document/fieldvalue/stringfieldvalue.h> -#include <vespa/document/datatype/documenttype.h> #include <vespa/storage/common/reindexing_constants.h> #include <vespa/storage/distributor/top_level_distributor.h> #include <vespa/storage/distributor/distributor_bucket_space.h> @@ -22,6 +21,9 @@ using document::test::makeDocumentBucket; using document::DocumentId; +using document::FieldUpdate; +using document::StringFieldValue; +using document::AssignValueUpdate; using namespace ::testing; namespace storage::distributor { @@ -595,9 +597,7 @@ TEST_F(ExternalOperationHandlerTest, non_trivial_updates_are_rejected_if_feed_is auto cmd = makeUpdateCommand("testdoctype1", "id:foo:testdoctype1::foo"); const auto* doc_type = _testDocMan.getTypeRepo().getDocumentType("testdoctype1"); - document::FieldUpdate upd(doc_type->getField("title")); - upd.addUpdate(document::AssignValueUpdate(document::StringFieldValue("new value"))); - cmd->getUpdate()->addUpdate(upd); + cmd->getUpdate()->addUpdate(FieldUpdate(doc_type->getField("title")).addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue("new value")))); ASSERT_NO_FATAL_FAILURE(start_operation_verify_rejected(std::move(cmd))); EXPECT_EQ("ReturnCode(NO_SPACE, External feed is blocked due to resource exhaustion: full disk)", diff --git a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp index fae2a3d0982..0e3ce55856b 100644 --- a/storage/src/tests/distributor/twophaseupdateoperationtest.cpp +++ b/storage/src/tests/distributor/twophaseupdateoperationtest.cpp @@ -295,9 +295,7 @@ TwoPhaseUpdateOperationTest::sendUpdate(const std::string& bucketState, update = std::make_shared<document::DocumentUpdate>( *_repo, *_doc_type, document::DocumentId("id:ns:" + _doc_type->getName() + "::1")); - document::FieldUpdate fup(_doc_type->getField("headerval")); - fup.addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 10)); - update->addUpdate(fup); + update->addUpdate(FieldUpdate(_doc_type->getField("headerval")).addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 10))); } else { // Create an update to a different doctype than the one returned as // part of the Get. Just a sneaky way to force an eval error. @@ -305,9 +303,7 @@ TwoPhaseUpdateOperationTest::sendUpdate(const std::string& bucketState, update = std::make_shared<document::DocumentUpdate>( *_repo, *badDocType, document::DocumentId("id:ns:" + _doc_type->getName() + "::1")); - document::FieldUpdate fup(badDocType->getField("onlyinchild")); - fup.addUpdate(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 10)); - update->addUpdate(fup); + update->addUpdate(FieldUpdate(badDocType->getField("onlyinchild")).addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 10))); } update->setCreateIfNonExistent(options._createIfNonExistent); diff --git a/storage/src/tests/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp index 6974319dc72..3812e7b85c9 100644 --- a/storage/src/tests/persistence/persistencetestutils.cpp +++ b/storage/src/tests/persistence/persistencetestutils.cpp @@ -225,10 +225,7 @@ PersistenceTestUtils::createBodyUpdate(const document::DocumentId& docId, const { const DocumentType* docType(getTypeRepo()->getDocumentType("testdoctype1")); auto update = std::make_shared<document::DocumentUpdate>(*getTypeRepo(), *docType, docId); - auto assignUpdate = std::make_shared<document::AssignValueUpdate>(updateValue); - document::FieldUpdate fieldUpdate(docType->getField("content")); - fieldUpdate.addUpdate(*assignUpdate); - update->addUpdate(fieldUpdate); + update->addUpdate(document::FieldUpdate(docType->getField("content")).addUpdate(std::make_unique<document::AssignValueUpdate>(updateValue))); return update; } @@ -237,10 +234,7 @@ PersistenceTestUtils::createHeaderUpdate(const document::DocumentId& docId, cons { const DocumentType* docType(getTypeRepo()->getDocumentType("testdoctype1")); auto update = std::make_shared<document::DocumentUpdate>(*getTypeRepo(), *docType, docId); - auto assignUpdate = std::make_shared<document::AssignValueUpdate>(updateValue); - document::FieldUpdate fieldUpdate(docType->getField("headerval")); - fieldUpdate.addUpdate(*assignUpdate); - update->addUpdate(fieldUpdate); + update->addUpdate(document::FieldUpdate(docType->getField("headerval")).addUpdate(std::make_unique<document::AssignValueUpdate>(updateValue))); return update; } diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp index 816fd7871d3..6e5fc491941 100644 --- a/storage/src/tests/persistence/testandsettest.cpp +++ b/storage/src/tests/persistence/testandsettest.cpp @@ -7,7 +7,6 @@ #include <vespa/document/fieldvalue/fieldvalues.h> #include <vespa/document/update/documentupdate.h> #include <vespa/document/update/assignvalueupdate.h> -#include <vespa/document/datatype/documenttype.h> #include <vespa/document/fieldset/fieldsets.h> #include <vespa/persistence/spi/test.h> #include <vespa/persistence/spi/persistenceprovider.h> @@ -160,9 +159,7 @@ std::shared_ptr<api::UpdateCommand> TestAndSetTest::conditional_update_test(bool createIfMissing, api::Timestamp updateTimestamp) { auto docUpdate = std::make_shared<document::DocumentUpdate>(_env->_testDocMan.getTypeRepo(), testDoc->getType(), testDocId); - auto fieldUpdate = document::FieldUpdate(testDoc->getField("content")); - fieldUpdate.addUpdate(document::AssignValueUpdate(NEW_CONTENT)); - docUpdate->addUpdate(fieldUpdate); + docUpdate->addUpdate(document::FieldUpdate(testDoc->getField("content")).addUpdate(std::make_unique<document::AssignValueUpdate>(NEW_CONTENT))); docUpdate->setCreateIfNonExistent(createIfMissing); auto updateUp = std::make_unique<api::UpdateCommand>(BUCKET, docUpdate, updateTimestamp); diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp index b927c045f51..40969455d68 100644 --- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp +++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp @@ -34,6 +34,11 @@ using document::Document; using document::DocumentId; using document::DocumentType; using document::DocumentTypeRepo; +using document::FieldUpdate; +using document::FieldPathUpdate; +using document::AssignValueUpdate; +using document::IntFieldValue; +using document::RemoveFieldPathUpdate; using document::test::makeDocumentBucket; using document::test::makeBucketSpace; using storage::lib::ClusterState; @@ -245,13 +250,9 @@ TEST_P(StorageProtocolTest, response_metadata_is_propagated) { TEST_P(StorageProtocolTest, update) { auto update = std::make_shared<document::DocumentUpdate>( _docMan.getTypeRepo(), *_testDoc->getDataType(), _testDoc->getId()); - auto assignUpdate = std::make_shared<document::AssignValueUpdate>(document::IntFieldValue(17)); - document::FieldUpdate fieldUpdate(_testDoc->getField("headerval")); - fieldUpdate.addUpdate(*assignUpdate); - update->addUpdate(fieldUpdate); + update->addUpdate(FieldUpdate(_testDoc->getField("headerval")).addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(17)))); - update->addFieldPathUpdate(document::FieldPathUpdate::CP( - new document::RemoveFieldPathUpdate("headerval", "testdoctype1.headerval > 0"))); + update->addFieldPathUpdate(FieldPathUpdate::CP(new RemoveFieldPathUpdate("headerval", "testdoctype1.headerval > 0"))); auto cmd = std::make_shared<UpdateCommand>(_bucket, update, 14); EXPECT_EQ(Timestamp(0), cmd->getOldTimestamp()); diff --git a/vespa-feed-client-api/abi-spec.json b/vespa-feed-client-api/abi-spec.json index 16e532a2c9a..5bd0acf82d3 100644 --- a/vespa-feed-client-api/abi-spec.json +++ b/vespa-feed-client-api/abi-spec.json @@ -140,6 +140,7 @@ "public abstract ai.vespa.feed.client.FeedClientBuilder setCaCertificatesFile(java.nio.file.Path)", "public abstract ai.vespa.feed.client.FeedClientBuilder setCaCertificates(java.util.Collection)", "public abstract ai.vespa.feed.client.FeedClientBuilder setEndpointUris(java.util.List)", + "public abstract ai.vespa.feed.client.FeedClientBuilder setProxy(java.net.URI)", "public abstract ai.vespa.feed.client.FeedClient build()" ], "fields": [ diff --git a/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java b/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java index 95c9b2c95fe..7ec5fbb02b7 100644 --- a/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java +++ b/vespa-feed-client-api/src/main/java/ai/vespa/feed/client/FeedClientBuilder.java @@ -109,6 +109,9 @@ public interface FeedClientBuilder { /** Overrides endpoint URIs for this client */ FeedClientBuilder setEndpointUris(List<URI> endpoints); + /** Specify HTTP(S) proxy for all endpoints */ + FeedClientBuilder setProxy(URI uri); + /** Constructs instance of {@link FeedClient} from builder configuration */ FeedClient build(); diff --git a/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliArguments.java b/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliArguments.java index ac859359bfa..e024f961e26 100644 --- a/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliArguments.java +++ b/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliArguments.java @@ -55,6 +55,7 @@ class CliArguments { private static final String VERSION_OPTION = "version"; private static final String STDIN_OPTION = "stdin"; private static final String DOOM_OPTION = "max-failure-seconds"; + private static final String PROXY_OPTION = "proxy"; private final CommandLine arguments; @@ -165,6 +166,16 @@ class CliArguments { boolean dryrunEnabled() { return has(DRYRUN_OPTION); } + Optional<URI> proxy() throws CliArgumentsException { + try { + URL url = (URL) arguments.getParsedOptionValue(PROXY_OPTION); + if (url == null) return Optional.empty(); + return Optional.of(url.toURI()); + } catch (ParseException | URISyntaxException e) { + throw new CliArgumentsException("Invalid proxy: " + e.getMessage(), e); + } + } + private OptionalInt intValue(String option) throws CliArgumentsException { try { Number number = (Number) arguments.getParsedOptionValue(option); @@ -310,6 +321,12 @@ class CliArguments { .addOption(Option.builder() .longOpt(SHOW_ALL_OPTION) .desc("Print the result of every feed operation") + .build()) + .addOption(Option.builder() + .longOpt(PROXY_OPTION) + .desc("URI to proxy endpoint") + .hasArg() + .type(URL.class) .build()); } diff --git a/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliClient.java b/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliClient.java index 5e904b37588..68b9cf6af0e 100644 --- a/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliClient.java +++ b/vespa-feed-client-cli/src/main/java/ai/vespa/feed/client/impl/CliClient.java @@ -138,6 +138,7 @@ public class CliClient { builder.setDryrun(cliArgs.dryrunEnabled()); cliArgs.doomSeconds().ifPresent(doom -> builder.setCircuitBreaker(new GracePeriodCircuitBreaker(Duration.ofSeconds(10), Duration.ofSeconds(doom)))); + cliArgs.proxy().ifPresent(builder::setProxy); return builder.build(); } diff --git a/vespa-feed-client-cli/src/test/java/ai/vespa/feed/client/impl/CliArgumentsTest.java b/vespa-feed-client-cli/src/test/java/ai/vespa/feed/client/impl/CliArgumentsTest.java index fe3dc465814..201a85ed09d 100644 --- a/vespa-feed-client-cli/src/test/java/ai/vespa/feed/client/impl/CliArgumentsTest.java +++ b/vespa-feed-client-cli/src/test/java/ai/vespa/feed/client/impl/CliArgumentsTest.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.feed.client.impl; -import ai.vespa.feed.client.impl.CliArguments; import org.junit.jupiter.api.Test; import java.io.ByteArrayOutputStream; @@ -11,7 +10,10 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.time.Duration; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * @author bjorncs @@ -26,7 +28,7 @@ class CliArgumentsTest { "--ca-certificates=ca-certs.pem", "--disable-ssl-hostname-verification", "--header=\"My-Header: my-value\"", "--header", "Another-Header: another-value", "--benchmark", "--route=myroute", "--timeout=0.125", "--trace=9", "--verbose", "--silent", - "--show-errors", "--show-all", "--max-failure-seconds=30"}); + "--show-errors", "--show-all", "--max-failure-seconds=30", "--proxy", "https://myproxy:1234"}); assertEquals(URI.create("https://vespa.ai:4443/"), args.endpoint()); assertEquals(Paths.get("feed.json"), args.inputFile().get()); assertEquals(10, args.connections().getAsInt()); @@ -49,6 +51,7 @@ class CliArgumentsTest { assertTrue(args.showErrors()); assertTrue(args.showSuccesses()); assertFalse(args.showProgress()); + assertEquals(URI.create("https://myproxy:1234"), args.proxy().orElse(null)); } @Test diff --git a/vespa-feed-client-cli/src/test/resources/help.txt b/vespa-feed-client-cli/src/test/resources/help.txt index 323206ab128..66d7c3521c2 100644 --- a/vespa-feed-client-cli/src/test/resources/help.txt +++ b/vespa-feed-client-cli/src/test/resources/help.txt @@ -25,6 +25,7 @@ Vespa feed client streams per HTTP/2 connection --private-key <arg> Path to PEM/PKCS#8 encoded private key file + --proxy <arg> URI to proxy endpoint --route <arg> Target Vespa route for feed operations --show-all Print the result of every feed diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java index b51210d22ea..62cd56f21ce 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/ApacheCluster.java @@ -10,15 +10,13 @@ import org.apache.hc.client5.http.impl.async.HttpAsyncClients; import org.apache.hc.client5.http.ssl.ClientTlsStrategyBuilder; import org.apache.hc.core5.concurrent.FutureCallback; import org.apache.hc.core5.http.ContentType; +import org.apache.hc.core5.http.HttpHost; import org.apache.hc.core5.http.message.BasicHeader; import org.apache.hc.core5.http2.config.H2Config; import org.apache.hc.core5.net.URIAuthority; import org.apache.hc.core5.reactor.IOReactorConfig; -import org.apache.hc.core5.util.Timeout; - -import org.apache.hc.core5.function.Factory; import org.apache.hc.core5.reactor.ssl.TlsDetails; -import javax.net.ssl.SSLEngine; +import org.apache.hc.core5.util.Timeout; import javax.net.ssl.SSLContext; import java.io.IOException; @@ -45,11 +43,7 @@ class ApacheCluster implements Cluster { private final List<Endpoint> endpoints = new ArrayList<>(); private final List<BasicHeader> defaultHeaders = Arrays.asList(new BasicHeader("User-Agent", String.format("vespa-feed-client/%s", Vespa.VERSION)), new BasicHeader("Vespa-Client-Version", Vespa.VERSION)); - private final RequestConfig defaultConfig = RequestConfig.custom() - .setConnectTimeout(Timeout.ofSeconds(10)) - .setConnectionRequestTimeout(Timeout.DISABLED) - .setResponseTimeout(Timeout.ofSeconds(190)) - .build(); + private final RequestConfig requestConfig; private final ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(t -> new Thread(t, "request-timeout-thread")); @@ -57,6 +51,7 @@ class ApacheCluster implements Cluster { for (URI endpoint : builder.endpoints) for (int i = 0; i < builder.connectionsPerEndpoint; i++) endpoints.add(new Endpoint(createHttpClient(builder), endpoint)); + this.requestConfig = createRequestConfig(builder); } @Override @@ -75,7 +70,7 @@ class ApacheCluster implements Cluster { SimpleHttpRequest request = new SimpleHttpRequest(wrapped.method(), wrapped.path()); request.setScheme(endpoint.url.getScheme()); request.setAuthority(new URIAuthority(endpoint.url.getHost(), portOf(endpoint.url))); - request.setConfig(defaultConfig); + request.setConfig(requestConfig); defaultHeaders.forEach(request::setHeader); wrapped.headers().forEach((name, value) -> request.setHeader(name, value.get())); if (wrapped.body() != null) @@ -162,6 +157,15 @@ class ApacheCluster implements Cluster { : url.getPort(); } + private static RequestConfig createRequestConfig(FeedClientBuilderImpl b) { + RequestConfig.Builder builder = RequestConfig.custom() + .setConnectTimeout(Timeout.ofSeconds(10)) + .setConnectionRequestTimeout(Timeout.DISABLED) + .setResponseTimeout(Timeout.ofSeconds(190)); + if (b.proxy != null) builder.setProxy(new HttpHost(b.proxy.getScheme(), b.proxy.getHost(), b.proxy.getPort())); + return builder.build(); + } + private static class ApacheHttpResponse implements HttpResponse { private final SimpleHttpResponse wrapped; diff --git a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java index 7dafeb0b541..134ad464618 100644 --- a/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java +++ b/vespa-feed-client/src/main/java/ai/vespa/feed/client/impl/FeedClientBuilderImpl.java @@ -19,7 +19,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.function.Supplier; import static java.util.Objects.requireNonNull; @@ -50,7 +49,7 @@ public class FeedClientBuilderImpl implements FeedClientBuilder { Collection<X509Certificate> caCertificates; boolean benchmark = true; boolean dryrun = false; - + URI proxy; public FeedClientBuilderImpl() { @@ -188,6 +187,8 @@ public class FeedClientBuilderImpl implements FeedClientBuilder { return this; } + @Override public FeedClientBuilder setProxy(URI uri) { this.proxy = uri; return this; } + /** Constructs instance of {@link ai.vespa.feed.client.FeedClient} from builder configuration */ @Override public FeedClient build() { diff --git a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/VespaRecordWriter.java b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/VespaRecordWriter.java index c381ec87492..6d6c3789835 100644 --- a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/VespaRecordWriter.java +++ b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/VespaRecordWriter.java @@ -130,6 +130,12 @@ public class VespaRecordWriter extends RecordWriter<Object, Object> { .setMaxStreamPerConnection(streamsPerConnection) .setDryrun(config.dryrun()) .setRetryStrategy(retryStrategy(config)); + if (config.proxyHost() != null) { + URI proxyUri = URI.create(String.format( + "%s://%s:%d", config.proxyScheme(), config.proxyHost(), config.proxyPort())); + log.info("Using proxy " + proxyUri); + feedClientBuilder.setProxy(proxyUri); + } onFeedClientInitialization(feedClientBuilder); return feedClientBuilder.build(); diff --git a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/util/VespaConfiguration.java b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/util/VespaConfiguration.java index 1421a3fcd43..715546fe6fe 100644 --- a/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/util/VespaConfiguration.java +++ b/vespa-hadoop/src/main/java/com/yahoo/vespa/hadoop/mapreduce/util/VespaConfiguration.java @@ -16,6 +16,7 @@ public class VespaConfiguration { public static final String USE_SSL = "vespa.feed.ssl"; public static final String PROXY_HOST = "vespa.feed.proxy.host"; public static final String PROXY_PORT = "vespa.feed.proxy.port"; + public static final String PROXY_SCHEME = "vespa.feed.proxy.scheme"; public static final String DRYRUN = "vespa.feed.dryrun"; public static final String USE_COMPRESSION = "vespa.feed.usecompression"; public static final String DATA_FORMAT = "vespa.feed.data.format"; @@ -71,6 +72,13 @@ public class VespaConfiguration { } + public String proxyScheme() { + String raw = getString(PROXY_SCHEME); + if (raw == null) return "http"; + return raw; + } + + public boolean dryrun() { return getBoolean(DRYRUN, false); } @@ -186,6 +194,7 @@ public class VespaConfiguration { sb.append(USE_SSL + ": " + useSSL().map(Object::toString).orElse("<empty>") + "\n"); sb.append(PROXY_HOST + ": " + proxyHost() + "\n"); sb.append(PROXY_PORT + ": " + proxyPort() + "\n"); + sb.append(PROXY_SCHEME + ": " + proxyScheme() + "\n"); sb.append(DRYRUN + ": " + dryrun() +"\n"); sb.append(USE_COMPRESSION + ": " + useCompression() +"\n"); sb.append(DATA_FORMAT + ": " + dataFormat() +"\n"); diff --git a/vespalib/src/vespa/vespalib/net/server_socket.h b/vespalib/src/vespa/vespalib/net/server_socket.h index 5dd59bf6559..09dd61a4c5b 100644 --- a/vespalib/src/vespa/vespalib/net/server_socket.h +++ b/vespalib/src/vespa/vespalib/net/server_socket.h @@ -20,7 +20,7 @@ private: void cleanup(); public: - ServerSocket() : _handle(), _path() {} + ServerSocket() : _handle(), _path(), _blocking(false), _shutdown(false) {} explicit ServerSocket(const SocketSpec &spec); explicit ServerSocket(const vespalib::string &spec); explicit ServerSocket(int port); diff --git a/vespalib/src/vespa/vespalib/util/classname.cpp b/vespalib/src/vespa/vespalib/util/classname.cpp index f1dd9de6686..8e9fa6b7041 100644 --- a/vespalib/src/vespa/vespalib/util/classname.cpp +++ b/vespalib/src/vespa/vespalib/util/classname.cpp @@ -8,7 +8,10 @@ namespace vespalib { string demangle(const char * native) { int status = 0; size_t size = 0; - char *unmangled = abi::__cxa_demangle(native, 0, &size, &status); + char *unmangled = abi::__cxa_demangle(native, nullptr, &size, &status); + if (unmangled == nullptr) { + return ""; // Demangling failed for some reason. TODO return `native` instead? + } string result(unmangled); free(unmangled); return result; diff --git a/vespalib/src/vespa/vespalib/util/rcuvector.h b/vespalib/src/vespa/vespalib/util/rcuvector.h index d2d0a946b91..fd0a7de441a 100644 --- a/vespalib/src/vespa/vespalib/util/rcuvector.h +++ b/vespalib/src/vespa/vespalib/util/rcuvector.h @@ -120,7 +120,6 @@ public: size_t capacity() const { return _data.capacity(); } void clear() { _data.clear(); } T & operator[](size_t i) { return _data[i]; } - const T & operator[](size_t i) const { return _data[i]; } /* * Readers holding a generation guard can call acquire_elem_ref(i) * to get a const reference to element i. Array bound must be handled @@ -128,6 +127,8 @@ public: */ const T& acquire_elem_ref(size_t i) const noexcept { return *(_vector_start.load(std::memory_order_acquire) + i); } + const T& get_elem_ref(size_t i) const noexcept { return _data[i]; } // Called from writer only + void reset(); void shrink(size_t newSize) __attribute__((noinline)); void replaceVector(ArrayType replacement); |