diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-03-29 16:00:32 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-29 16:00:32 +0200 |
commit | dc45403e426237d940544b4929f5e9f31c259d0e (patch) | |
tree | 9e63da857906708a587b0050a679ce2a72719d28 /document | |
parent | 55a68fe347b0b551cd3421d014fabcf3be4f525d (diff) | |
parent | 2d197543db1491993c194c29be88a5ad33c383fb (diff) |
Merge pull request #21856 from vespa-engine/balder/avoid-identifiable-for-valueupdate-2
Balder/avoid identifiable for valueupdate 2
Diffstat (limited to 'document')
27 files changed, 267 insertions, 330 deletions
diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp index a447df1044e..6644fec2da0 100644 --- a/document/src/tests/documentselectparsertest.cpp +++ b/document/src/tests/documentselectparsertest.cpp @@ -139,9 +139,9 @@ DocumentUpdate::SP DocumentSelectParserTest::createUpdate( const DocumentType* type = _repo->getDocumentType(doctype); auto doc = std::make_shared<DocumentUpdate>(*_repo, *type, DocumentId(id)); doc->addUpdate(FieldUpdate(doc->getType().getField("headerval")) - .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(hint)))); + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(hint)))); doc->addUpdate(FieldUpdate(doc->getType().getField("hstringval")) - .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue(hstr)))); + .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue::make(hstr)))); return doc; } diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index beb94959e73..40f398ee93e 100644 --- a/document/src/tests/documentupdatetestcase.cpp +++ b/document/src/tests/documentupdatetestcase.cpp @@ -120,13 +120,13 @@ TEST(DocumentUpdateTest, testSimpleUsage) // Test that primitive value updates can be serialized testRoundtripSerialize(ClearValueUpdate(), *DataType::INT); - testRoundtripSerialize(AssignValueUpdate(IntFieldValue(1)), *DataType::INT); + testRoundtripSerialize(AssignValueUpdate(std::make_unique<IntFieldValue>(1)), *DataType::INT); testRoundtripSerialize(ArithmeticValueUpdate(ArithmeticValueUpdate::Div, 4.3), *DataType::FLOAT); - testRoundtripSerialize(AddValueUpdate(IntFieldValue(1), 4), *arrayType); - testRoundtripSerialize(RemoveValueUpdate(IntFieldValue(1)), *arrayType); + testRoundtripSerialize(AddValueUpdate(std::make_unique<IntFieldValue>(1), 4), *arrayType); + testRoundtripSerialize(RemoveValueUpdate(std::make_unique<IntFieldValue>(1)), *arrayType); FieldUpdate fieldUpdate(docType->getField("intf")); - fieldUpdate.addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(1))); + fieldUpdate.addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(1))); nbostream stream = serialize(fieldUpdate); FieldUpdate fieldUpdateCopy(repo, *docType, stream); EXPECT_EQ(fieldUpdate, fieldUpdateCopy); @@ -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(std::make_unique<AssignValueUpdate>(IntFieldValue(15)))); + upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(15)))); upd.applyTo(updated); EXPECT_NE(doc, updated); EXPECT_EQ(15, 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(std::make_unique<AddValueUpdate>(IntFieldValue(4)))); + upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<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(std::make_unique<RemoveValueUpdate>(IntFieldValue(3)))); + upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(std::make_unique<RemoveValueUpdate>(std::make_unique<IntFieldValue>(3)))); upd.applyTo(updated); EXPECT_NE(doc, updated); std::unique_ptr<ArrayFieldValue> val(dynamic_cast<ArrayFieldValue*>(updated.getValue("intarr").release())); @@ -227,7 +227,7 @@ TEST(DocumentUpdateTest, testUpdateApplySingleValue) // Apply an update. DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(9)))) + .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(9)))) .applyTo(*doc); EXPECT_EQ(9, doc->getValue("headerval")->getAsInt()); } @@ -240,12 +240,12 @@ TEST(DocumentUpdateTest, testUpdateArray) EXPECT_EQ((document::FieldValue*)nullptr, doc->getValue(doc->getField("tags")).get()); // Assign array field. - ArrayFieldValue myarray(doc->getType().getField("tags").getDataType()); - myarray.add(StringFieldValue("foo")); - myarray.add(StringFieldValue("bar")); + auto myarray = std::make_unique<ArrayFieldValue>(doc->getType().getField("tags").getDataType()); + myarray->add(StringFieldValue("foo")); + myarray->add(StringFieldValue("bar")); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(std::make_unique<AssignValueUpdate>(myarray))) + .addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(std::make_unique<AssignValueUpdate>(std::move(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(std::make_unique<AddValueUpdate>(StringFieldValue("another"))) - .addUpdate(std::make_unique<AddValueUpdate>(StringFieldValue("tag")))) + .addUpdate(std::make_unique<AddValueUpdate>(StringFieldValue::make("another"))) + .addUpdate(std::make_unique<AddValueUpdate>(StringFieldValue::make("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(std::make_unique<AssignValueUpdate>(StringFieldValue("THROW MEH!")))) + .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue::make("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(std::make_unique<RemoveValueUpdate>(StringFieldValue("foo"))) - .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("tag")))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue::make("foo"))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue::make("tag")))) .applyTo(*doc); auto fval3(doc->getAs<ArrayFieldValue>(doc->getField("tags"))); ASSERT_EQ((size_t) 2, fval3->size()); @@ -286,27 +286,27 @@ TEST(DocumentUpdateTest, testUpdateArray) EXPECT_EQ(std::string("another"), std::string((*fval3)[1].getAsString())); // Remove array from array. - ArrayFieldValue myarray2(doc->getType().getField("tags").getDataType()); - myarray2.add(StringFieldValue("foo")); - myarray2.add(StringFieldValue("bar")); + auto myarray2 = std::make_unique<ArrayFieldValue>(doc->getType().getField("tags").getDataType()); + myarray2->add(StringFieldValue("foo")); + myarray2->add(StringFieldValue("bar")); ASSERT_THROW( DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(doc->getField("tags")) - .addUpdate(std::make_unique<RemoveValueUpdate>(myarray2))) + .addUpdate(std::make_unique<RemoveValueUpdate>(std::move(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)); + auto upd = std::make_unique<AddValueUpdate>(StringFieldValue::make(key)); upd->setWeight(weight); return upd; } std::unique_ptr<ValueUpdate> createAddUpdate(int key, int weight) { - auto upd = std::make_unique<AddValueUpdate>(IntFieldValue(key)); + auto upd = std::make_unique<AddValueUpdate>(std::make_unique<IntFieldValue>(key)); upd->setWeight(weight); return upd; } @@ -320,11 +320,11 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) EXPECT_EQ((FieldValue*) 0, doc->getValue(field).get()); // Assign weightedset field - WeightedSetFieldValue wset(field.getDataType()); - wset.add(StringFieldValue("foo"), 3); - wset.add(StringFieldValue("bar"), 14); + auto wset =std::make_unique<WeightedSetFieldValue>(field.getDataType()); + wset->add(StringFieldValue("foo"), 3); + wset->add(StringFieldValue("bar"), 14); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) - .addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(wset))) + .addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(std::move(wset)))) .applyTo(*doc); auto fval1(doc->getAs<WeightedSetFieldValue>(field)); ASSERT_EQ((size_t) 2, fval1->size()); @@ -336,12 +336,12 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) EXPECT_EQ(14, fval1->get(StringFieldValue("bar"), 0)); // Do a second assign - WeightedSetFieldValue wset2(field.getDataType()); - wset2.add(StringFieldValue("foo"), 16); - wset2.add(StringFieldValue("bar"), 24); + auto wset2 = std::make_unique<WeightedSetFieldValue>(field.getDataType()); + wset2->add(StringFieldValue("foo"), 16); + wset2->add(StringFieldValue("bar"), 24); DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field) - .addUpdate(std::make_unique<AssignValueUpdate>(wset2))) + .addUpdate(std::make_unique<AssignValueUpdate>(std::move(wset2)))) .applyTo(*doc); auto fval2(doc->getAs<WeightedSetFieldValue>(field)); ASSERT_EQ((size_t) 2, fval2->size()); @@ -371,8 +371,8 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) // Remove weighted field DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field) - .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("foo"))) - .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue("too")))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue::make("foo"))) + .addUpdate(std::make_unique<RemoveValueUpdate>(StringFieldValue::make("too")))) .applyTo(*doc); auto fval4(doc->getAs<WeightedSetFieldValue>(field)); ASSERT_EQ((size_t) 1, fval4->size()); @@ -419,7 +419,7 @@ WeightedSetAutoCreateFixture::WeightedSetAutoCreateFixture() update(repo, *docType, DocumentId("id:ns:test::1")) { update.addUpdate(FieldUpdate(field) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("foo"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("foo"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 1)))); } } // anon ns @@ -457,7 +457,7 @@ TEST(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) { WeightedSetAutoCreateFixture fixture; fixture.update.addUpdate(FieldUpdate(fixture.field) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("baz"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("baz"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 0)))); fixture.applyUpdateToDocument(); @@ -498,7 +498,7 @@ TEST(DocumentUpdateTest, testReadSerializedFile) serValue = &serField2[0]; EXPECT_EQ(serValue->getType(), ValueUpdate::Clear); - EXPECT_TRUE(serValue->inherits(ClearValueUpdate::classId)); + EXPECT_EQ(ValueUpdate::Clear, serValue->getType()); // Verify add value update. const FieldUpdate & serField3 = upd.getUpdates()[0]; @@ -539,19 +539,19 @@ 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(std::make_unique<AssignValueUpdate>(IntFieldValue(4)))); + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(4)))); upd.addUpdate(FieldUpdate(type->getField("floatfield")) - .addUpdate(std::make_unique<AssignValueUpdate>(FloatFieldValue(1.00f)))); + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<FloatFieldValue>(1.00f)))); upd.addUpdate(FieldUpdate(type->getField("arrayoffloatfield")) - .addUpdate(std::make_unique<AddValueUpdate>(FloatFieldValue(5.00f))) - .addUpdate(std::make_unique<AddValueUpdate>(FloatFieldValue(4.23f))) - .addUpdate(std::make_unique<AddValueUpdate>(FloatFieldValue(-1.00f)))); + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<FloatFieldValue>(5.00f))) + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<FloatFieldValue>(4.23f))) + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<FloatFieldValue>(-1.00f)))); upd.addUpdate(FieldUpdate(type->getField("intfield")) .addUpdate(std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 3))); upd.addUpdate(FieldUpdate(type->getField("wsfield")) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("foo"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("foo"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 2))) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("foo"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("foo"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Mul, 2)))); nbostream buf(serializeHEAD(upd)); writeBufferToFile(buf, "data/serializeupdatecpp.dat"); @@ -569,7 +569,7 @@ TEST(DocumentUpdateTest, testSetBadFieldTypes) DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); ASSERT_THROW( update.addUpdate(FieldUpdate(doc->getField("headerval")) - .addUpdate(std::make_unique<AssignValueUpdate>(FloatFieldValue(4.00f)))), + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<FloatFieldValue>(4.00f)))), std::exception) << "Expected exception when adding a float to an int field."; update.applyTo(*doc); @@ -604,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(std::make_unique<AssignValueUpdate>(ArrayFieldValue(field.getDataType())))); + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<ArrayFieldValue>(field.getDataType())))); update.applyTo(*doc); @@ -624,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(std::make_unique<AssignValueUpdate>(ArrayFieldValue(field.getDataType())))); + update.addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<ArrayFieldValue>(field.getDataType())))); update.applyTo(*doc); // Verify that the field was set in the document. @@ -652,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(std::make_unique<AssignValueUpdate>(WeightedSetFieldValue(field.getDataType())))); + update.addUpdate(FieldUpdate(field).addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<WeightedSetFieldValue>(field.getDataType())))); update.applyTo(*doc); // Verify that the field was set in the document. @@ -682,8 +682,8 @@ TEST(DocumentUpdateTest, testUpdateArrayWrongSubtype) DocumentUpdate update(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()); ASSERT_THROW( update.addUpdate(FieldUpdate(field) - .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(123))) - .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(456)))), + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<IntFieldValue>(123))) + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<IntFieldValue>(456)))), std::exception) << "Expected exception when adding wrong type."; // Apply update @@ -732,7 +732,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("banana"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("banana"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); std::unique_ptr<WeightedSetFieldValue> fv1 = @@ -741,7 +741,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field2) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("banana"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("banana"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); auto fv2 = doc->getAs<WeightedSetFieldValue>(field2); @@ -770,7 +770,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("apple"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("apple"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Sub, 1.0)))) .applyTo(*doc); @@ -780,7 +780,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field2) - .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue("apple"), + .addUpdate(std::make_unique<MapValueUpdate>(StringFieldValue::make("apple"), std::make_unique<ArithmeticValueUpdate>(ArithmeticValueUpdate::Sub, 1.0)))) .applyTo(*doc); @@ -940,10 +940,9 @@ struct TensorUpdateFixture { TEST(DocumentUpdateTest, tensor_assign_update_can_be_applied) { TensorUpdateFixture f; - auto newTensor = f.makeBaselineTensor(); - f.applyUpdate(std::make_unique<AssignValueUpdate>(*newTensor)); + f.applyUpdate(std::make_unique<AssignValueUpdate>(f.makeBaselineTensor())); f.assertDocumentUpdated(); - f.assertTensor(*newTensor); + f.assertTensor(*f.makeBaselineTensor()); } TEST(DocumentUpdateTest, tensor_clear_update_can_be_applied) @@ -1032,7 +1031,7 @@ TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied_to_nonexisting_tens TEST(DocumentUpdateTest, tensor_assign_update_can_be_roundtrip_serialized) { TensorUpdateFixture f; - f.assertRoundtripSerialize(AssignValueUpdate(*f.makeBaselineTensor())); + f.assertRoundtripSerialize(AssignValueUpdate(f.makeBaselineTensor())); } TEST(DocumentUpdateTest, tensor_add_update_can_be_roundtrip_serialized) @@ -1162,7 +1161,7 @@ struct TensorUpdateSerializeFixture { (*repo, docType, DocumentId("id:test:test::0")); result->addUpdate(FieldUpdate(getField("sparse_tensor")) - .addUpdate(std::make_unique<AssignValueUpdate>(*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")) @@ -1250,7 +1249,7 @@ CreateIfNonExistentFixture::CreateIfNonExistentFixture() update(std::make_unique<DocumentUpdate>(docMan.getTypeRepo(), *document->getDataType(), document->getId())) { update->addUpdate(FieldUpdate(document->getField("headerval")) - .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(1)))); + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(1)))); update->setCreateIfNonExistent(true); } @@ -1282,8 +1281,8 @@ ArrayUpdateFixture::ArrayUpdateFixture() { update = std::make_unique<DocumentUpdate>(doc_man.getTypeRepo(), *doc->getDataType(), doc->getId()); update->addUpdate(FieldUpdate(array_field) - .addUpdate(std::make_unique<MapValueUpdate>(IntFieldValue(1), - std::make_unique<AssignValueUpdate>(StringFieldValue("bar"))))); + .addUpdate(std::make_unique<MapValueUpdate>(std::make_unique<IntFieldValue>(1), + std::make_unique<AssignValueUpdate>(StringFieldValue::make("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 6649c9c0208..fd217601b6c 100644 --- a/document/src/tests/feed_reject_helper_test.cpp +++ b/document/src/tests/feed_reject_helper_test.cpp @@ -53,22 +53,22 @@ TEST(DocumentRejectTest, requireThatFixedSizeFieldValuesAreDetected) { TEST(DocumentRejectTest, requireThatClearRemoveTensorRemoveAndArtithmeticUpdatesIgnoreFeedRejection) { EXPECT_FALSE(FeedRejectHelper::mustReject(ClearValueUpdate())); - EXPECT_FALSE(FeedRejectHelper::mustReject(RemoveValueUpdate(StringFieldValue()))); + EXPECT_FALSE(FeedRejectHelper::mustReject(RemoveValueUpdate(StringFieldValue::make()))); EXPECT_FALSE(FeedRejectHelper::mustReject(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 5.0))); EXPECT_FALSE(FeedRejectHelper::mustReject(TensorRemoveUpdate(std::make_unique<TensorFieldValue>()))); } TEST(DocumentRejectTest, requireThatAddMapTensorModifyAndTensorAddUpdatesWillBeRejected) { - EXPECT_TRUE(FeedRejectHelper::mustReject(AddValueUpdate(IntFieldValue()))); - EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(IntFieldValue(), std::make_unique<ClearValueUpdate>()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(AddValueUpdate(std::make_unique<IntFieldValue>()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(std::make_unique<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>()))); } TEST(DocumentRejectTest, requireThatAssignUpdatesWillBeRejectedBasedOnTheirContent) { - EXPECT_FALSE(FeedRejectHelper::mustReject(AssignValueUpdate(IntFieldValue()))); - EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(StringFieldValue()))); + EXPECT_FALSE(FeedRejectHelper::mustReject(AssignValueUpdate(std::make_unique<IntFieldValue>()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(StringFieldValue::make()))); } } diff --git a/document/src/tests/testxml.cpp b/document/src/tests/testxml.cpp index 978ab572214..5f194661fe5 100644 --- a/document/src/tests/testxml.cpp +++ b/document/src/tests/testxml.cpp @@ -60,16 +60,16 @@ createTestDocumentUpdate(const DocumentTypeRepo& repo) auto up = std::make_unique<DocumentUpdate>(repo, *type, id); up->addUpdate(FieldUpdate(type->getField("intattr")) - .addUpdate(std::make_unique<AssignValueUpdate>(IntFieldValue(7)))); + .addUpdate(std::make_unique<AssignValueUpdate>(std::make_unique<IntFieldValue>(7)))); up->addUpdate(FieldUpdate(type->getField("stringattr")) - .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue("New value")))); + .addUpdate(std::make_unique<AssignValueUpdate>(StringFieldValue::make("New value")))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) - .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(123))) - .addUpdate(std::make_unique<AddValueUpdate>(IntFieldValue(456)))); + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<IntFieldValue>(123))) + .addUpdate(std::make_unique<AddValueUpdate>(std::make_unique<IntFieldValue>(456)))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) - .addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(123))) - .addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(456))) - .addUpdate(std::make_unique<RemoveValueUpdate>(IntFieldValue(789)))); + .addUpdate(std::make_unique<RemoveValueUpdate>(std::make_unique<IntFieldValue>(123))) + .addUpdate(std::make_unique<RemoveValueUpdate>(std::make_unique<IntFieldValue>(456))) + .addUpdate(std::make_unique<RemoveValueUpdate>(std::make_unique<IntFieldValue>(789)))); return up; } diff --git a/document/src/vespa/document/base/forcelink.cpp b/document/src/vespa/document/base/forcelink.cpp index 500b5cf7fa5..cfa3354f5f2 100644 --- a/document/src/vespa/document/base/forcelink.cpp +++ b/document/src/vespa/document/base/forcelink.cpp @@ -14,10 +14,10 @@ ForceLink::ForceLink(void) DocumentType type("foo", 1); Document document(type, DocumentId("doc:ns:bar")); DocumentUpdate documentUpdate; - MapValueUpdate mapValueUpdate(IntFieldValue(3), std::make_unique<ClearValueUpdate>()); - AddValueUpdate addValueUpdate(IntFieldValue(3)); - RemoveValueUpdate removeValueUpdate(IntFieldValue(3)); - AssignValueUpdate assignValueUpdate(IntFieldValue(3)); + MapValueUpdate mapValueUpdate(std::make_unique<IntFieldValue>(3), std::make_unique<ClearValueUpdate>()); + AddValueUpdate addValueUpdate(std::make_unique<IntFieldValue>(3)); + RemoveValueUpdate removeValueUpdate(std::make_unique<IntFieldValue>(3)); + AssignValueUpdate assignValueUpdate(std::make_unique<IntFieldValue>(3)); ClearValueUpdate clearValueUpdate; ArithmeticValueUpdate arithmeticValueUpdate(ArithmeticValueUpdate::Add, 3); } diff --git a/document/src/vespa/document/serialization/vespadocumentserializer.cpp b/document/src/vespa/document/serialization/vespadocumentserializer.cpp index b6a7dabd87d..2a2b642dd48 100644 --- a/document/src/vespa/document/serialization/vespadocumentserializer.cpp +++ b/document/src/vespa/document/serialization/vespadocumentserializer.cpp @@ -377,7 +377,7 @@ VespaDocumentSerializer::write(const FieldUpdate &value) void VespaDocumentSerializer::write(const RemoveValueUpdate &value) { - _stream << RemoveValueUpdate::classId; + _stream << uint32_t(ValueUpdate::Remove); write(value.getKey()); } @@ -385,7 +385,7 @@ VespaDocumentSerializer::write(const RemoveValueUpdate &value) void VespaDocumentSerializer::write(const AddValueUpdate &value) { - _stream << AddValueUpdate::classId; + _stream << uint32_t(ValueUpdate::Add); write(value.getValue()); _stream << static_cast<int32_t>(value.getWeight()); } @@ -393,7 +393,7 @@ VespaDocumentSerializer::write(const AddValueUpdate &value) void VespaDocumentSerializer::write(const ArithmeticValueUpdate &value) { - _stream << ArithmeticValueUpdate::classId; + _stream << uint32_t(ValueUpdate::Arithmetic); _stream << static_cast<uint32_t>(value.getOperator()); _stream << static_cast<double>(value.getOperand()); } @@ -401,7 +401,7 @@ VespaDocumentSerializer::write(const ArithmeticValueUpdate &value) void VespaDocumentSerializer::write(const AssignValueUpdate &value) { - _stream << AssignValueUpdate::classId; + _stream << uint32_t(ValueUpdate::Assign); if (value.hasValue()) { _stream << static_cast<uint8_t>(CONTENT_HASVALUE); write(value.getValue()); @@ -414,12 +414,12 @@ void VespaDocumentSerializer::write(const ClearValueUpdate &value) { (void) value; - _stream << ClearValueUpdate::classId; + _stream << uint32_t(ValueUpdate::Clear); } void VespaDocumentSerializer::write(const MapValueUpdate &value) { - _stream << MapValueUpdate::classId; + _stream << uint32_t(ValueUpdate::Map); write(value.getKey()); write(value.getUpdate()); } @@ -479,7 +479,7 @@ VespaDocumentSerializer::write(const RemoveFieldPathUpdate &value) void VespaDocumentSerializer::write(const TensorModifyUpdate &value) { - _stream << TensorModifyUpdate::classId; + _stream << uint32_t(ValueUpdate::TensorModify); _stream << static_cast<uint8_t>(value.getOperation()); write(value.getTensor()); } @@ -493,7 +493,7 @@ VespaDocumentSerializer::visit(const TensorModifyUpdate &value) void VespaDocumentSerializer::write(const TensorAddUpdate &value) { - _stream << TensorAddUpdate::classId; + _stream << uint32_t(ValueUpdate::TensorAdd); write(value.getTensor()); } @@ -506,7 +506,7 @@ VespaDocumentSerializer::visit(const TensorAddUpdate &value) void VespaDocumentSerializer::write(const TensorRemoveUpdate &value) { - _stream << TensorRemoveUpdate::classId; + _stream << uint32_t(ValueUpdate::TensorRemove); write(value.getTensor()); } diff --git a/document/src/vespa/document/update/addvalueupdate.cpp b/document/src/vespa/document/update/addvalueupdate.cpp index 6f6c9a93738..102f8ceebbe 100644 --- a/document/src/vespa/document/update/addvalueupdate.cpp +++ b/document/src/vespa/document/update/addvalueupdate.cpp @@ -17,19 +17,19 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(AddValueUpdate, ValueUpdate); -AddValueUpdate:: AddValueUpdate(const FieldValue& value, int weight) - : ValueUpdate(), - _value(value.clone()), +AddValueUpdate:: AddValueUpdate(std::unique_ptr<FieldValue> value, int weight) + : ValueUpdate(Add), + _value(std::move(value)), _weight(weight) {} AddValueUpdate::~AddValueUpdate() = default; + bool AddValueUpdate::operator==(const ValueUpdate& other) const { - if (other.getClass().id() != AddValueUpdate::classId) return false; + if (other.getType() != Add) return false; const AddValueUpdate& o(static_cast<const AddValueUpdate&>(other)); if (*_value != *o._value) return false; if (_weight != o._weight) return false; diff --git a/document/src/vespa/document/update/addvalueupdate.h b/document/src/vespa/document/update/addvalueupdate.h index ee7f6fbcdf2..f891c368f96 100644 --- a/document/src/vespa/document/update/addvalueupdate.h +++ b/document/src/vespa/document/update/addvalueupdate.h @@ -12,25 +12,25 @@ namespace document { -class AddValueUpdate : public ValueUpdate { - FieldValue::CP _value; // The field value to add by this update. +class AddValueUpdate final : public ValueUpdate { + std::unique_ptr<FieldValue> _value; // The field value to add by this update. int _weight; // The weight to assign to the contained value. // Used by ValueUpdate's static factory function // Private because it generates an invalid object. friend class ValueUpdate; - AddValueUpdate() : ValueUpdate(), _value(0), _weight(1) {} + AddValueUpdate() : ValueUpdate(Add), _value(), _weight(1) {} ACCEPT_UPDATE_VISITOR; public: - typedef std::unique_ptr<AddValueUpdate> UP; - /** * The default constructor requires initial values for all member variables. * * @param value The field value to add. * @param weight The weight for the field value. */ - AddValueUpdate(const FieldValue& value, int weight = 1); + AddValueUpdate(std::unique_ptr<FieldValue> value, int weight = 1); + AddValueUpdate(const AddValueUpdate &) = delete; + AddValueUpdate & operator =(const AddValueUpdate &) = delete; ~AddValueUpdate(); bool operator==(const ValueUpdate& other) const override; @@ -42,17 +42,6 @@ public: int getWeight() const { return _weight; } /** - * Sets the field value to add during this update. - * - * @param value The new field value. - * @return A reference to this object so you can chain calls. - */ - AddValueUpdate& setValue(const FieldValue& value) { - _value.reset(value.clone()); - return *this; - } - - /** * Sets the weight to assign to the value of this. * * @return A reference to this object so you can chain calls. @@ -67,8 +56,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; - - DECLARE_IDENTIFIABLE(AddValueUpdate); }; } // document diff --git a/document/src/vespa/document/update/arithmeticvalueupdate.cpp b/document/src/vespa/document/update/arithmeticvalueupdate.cpp index e5fb5aee8af..06f90d59d85 100644 --- a/document/src/vespa/document/update/arithmeticvalueupdate.cpp +++ b/document/src/vespa/document/update/arithmeticvalueupdate.cpp @@ -12,8 +12,6 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(ArithmeticValueUpdate, ValueUpdate); - // Declare string representations for operator names. static const char * operatorName[] = { "add", "div", "mul", "sub" }; static const char * operatorNameC[] = { "Add", "Div", "Mul", "Sub" }; @@ -21,7 +19,7 @@ static const char * operatorNameC[] = { "Add", "Div", "Mul", "Sub" }; bool ArithmeticValueUpdate::operator==(const ValueUpdate& other) const { - if (other.getClass().id() != ArithmeticValueUpdate::classId) return false; + if (other.getType() != Arithmetic) return false; const ArithmeticValueUpdate& o(static_cast<const ArithmeticValueUpdate&>(other)); if (_operator != o._operator) return false; if (_operand != o._operand) return false; diff --git a/document/src/vespa/document/update/arithmeticvalueupdate.h b/document/src/vespa/document/update/arithmeticvalueupdate.h index cb86528fce5..7651c838458 100644 --- a/document/src/vespa/document/update/arithmeticvalueupdate.h +++ b/document/src/vespa/document/update/arithmeticvalueupdate.h @@ -12,7 +12,7 @@ namespace document { -class ArithmeticValueUpdate : public ValueUpdate { +class ArithmeticValueUpdate final : public ValueUpdate { public: /** Declare all types of arithmetic value updates. */ enum Operator { @@ -25,13 +25,13 @@ public: private: Operator _operator; // The operator of the arithmetic operation. - double _operand; // The operand of the arithmetic operation. + double _operand; // The operand of the arithmetic operation. // Used by ValueUpdate's static factory function // Private because it generates an invalid object. friend class ValueUpdate; ArithmeticValueUpdate() - : ValueUpdate(), + : ValueUpdate(Arithmetic), _operator(MAX_NUM_OPERATORS), _operand(0.0) {} @@ -46,16 +46,12 @@ public: * @param opn The operand for the operation. */ ArithmeticValueUpdate(Operator opt, double opn) - : ValueUpdate(), + : ValueUpdate(Arithmetic), _operator(opt), _operand(opn) {} - ArithmeticValueUpdate(const ArithmeticValueUpdate& update) - : ValueUpdate(update), - _operator(update._operator), - _operand(update._operand) {} - - ArithmeticValueUpdate &operator=(const ArithmeticValueUpdate &rhs) = default; + ArithmeticValueUpdate(const ArithmeticValueUpdate& update) = delete; + ArithmeticValueUpdate &operator=(const ArithmeticValueUpdate &rhs) = delete; bool operator==(const ValueUpdate& other) const override; @@ -91,8 +87,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; - - DECLARE_IDENTIFIABLE(ArithmeticValueUpdate); }; } // document diff --git a/document/src/vespa/document/update/assignvalueupdate.cpp b/document/src/vespa/document/update/assignvalueupdate.cpp index cf2321dbf56..32bded8cfd7 100644 --- a/document/src/vespa/document/update/assignvalueupdate.cpp +++ b/document/src/vespa/document/update/assignvalueupdate.cpp @@ -16,13 +16,14 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(AssignValueUpdate, ValueUpdate); +AssignValueUpdate::AssignValueUpdate() + : ValueUpdate(Assign), + _value() +{} -AssignValueUpdate::AssignValueUpdate() = default; - -AssignValueUpdate::AssignValueUpdate(const FieldValue& value) - : ValueUpdate(), - _value(value.clone()) +AssignValueUpdate::AssignValueUpdate(std::unique_ptr<FieldValue> value) + : ValueUpdate(Assign), + _value(std::move(value)) { } AssignValueUpdate::~AssignValueUpdate() = default; @@ -33,9 +34,13 @@ static const unsigned char CONTENT_HASVALUE = 0x01; bool AssignValueUpdate::operator==(const ValueUpdate& other) const { - if (other.getClass().id() != AssignValueUpdate::classId) return false; + if (other.getType() != Assign) return false; const AssignValueUpdate& o(static_cast<const AssignValueUpdate&>(other)); - return _value == o._value; + if (_value && o._value) { + return *_value == *o._value; + } else { + return bool(_value) == bool(o._value); + } } // Ensure that this update is compatible with given field. diff --git a/document/src/vespa/document/update/assignvalueupdate.h b/document/src/vespa/document/update/assignvalueupdate.h index e829e80da45..071dcc2236d 100644 --- a/document/src/vespa/document/update/assignvalueupdate.h +++ b/document/src/vespa/document/update/assignvalueupdate.h @@ -16,16 +16,15 @@ namespace document { -class AssignValueUpdate : public ValueUpdate { - FieldValue::CP _value; +class AssignValueUpdate final : public ValueUpdate { + std::unique_ptr<FieldValue> _value; ACCEPT_UPDATE_VISITOR; public: - typedef std::unique_ptr<AssignValueUpdate> UP; - AssignValueUpdate(); - - AssignValueUpdate(const FieldValue& value); + explicit AssignValueUpdate(std::unique_ptr<FieldValue> value); + AssignValueUpdate(const AssignValueUpdate& value) = delete; + AssignValueUpdate & operator=(const AssignValueUpdate& value) = delete; ~AssignValueUpdate() override; bool operator==(const ValueUpdate& other) const override; @@ -43,8 +42,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; - - DECLARE_IDENTIFIABLE(AssignValueUpdate); }; } diff --git a/document/src/vespa/document/update/clearvalueupdate.cpp b/document/src/vespa/document/update/clearvalueupdate.cpp index 17d7660219b..357ca8162c6 100644 --- a/document/src/vespa/document/update/clearvalueupdate.cpp +++ b/document/src/vespa/document/update/clearvalueupdate.cpp @@ -8,12 +8,10 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(ClearValueUpdate, ValueUpdate); - bool ClearValueUpdate::operator==(const ValueUpdate& other) const { - return (other.getClass().id() == ClearValueUpdate::classId); + return (other.getType() == Clear); } // Ensure that this update is compatible with given field. diff --git a/document/src/vespa/document/update/clearvalueupdate.h b/document/src/vespa/document/update/clearvalueupdate.h index 6e0d800dd73..770f0a91d07 100644 --- a/document/src/vespa/document/update/clearvalueupdate.h +++ b/document/src/vespa/document/update/clearvalueupdate.h @@ -11,13 +11,12 @@ namespace document { -class ClearValueUpdate : public ValueUpdate { +class ClearValueUpdate final : public ValueUpdate { ACCEPT_UPDATE_VISITOR; public: - typedef std::unique_ptr<ClearValueUpdate> UP; - ClearValueUpdate() : ValueUpdate() {} - ClearValueUpdate(const ClearValueUpdate& update) : ValueUpdate(update) {} - ClearValueUpdate &operator=(const ClearValueUpdate &rhs) = default; + ClearValueUpdate(const ClearValueUpdate& update) = delete; + ClearValueUpdate &operator=(const ClearValueUpdate &rhs) = delete; + ClearValueUpdate() : ValueUpdate(Clear) {} bool operator==(const ValueUpdate& other) const override; void checkCompatibility(const Field& field) const override; @@ -25,8 +24,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; - - DECLARE_IDENTIFIABLE(ClearValueUpdate); }; } diff --git a/document/src/vespa/document/update/mapvalueupdate.cpp b/document/src/vespa/document/update/mapvalueupdate.cpp index c3d9cff571b..47346133e17 100644 --- a/document/src/vespa/document/update/mapvalueupdate.cpp +++ b/document/src/vespa/document/update/mapvalueupdate.cpp @@ -16,32 +16,18 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(MapValueUpdate, ValueUpdate); - -MapValueUpdate::MapValueUpdate(const FieldValue& key, std::unique_ptr<ValueUpdate> update) - : ValueUpdate(), - _key(key.clone()), +MapValueUpdate::MapValueUpdate(std::unique_ptr<FieldValue> key, std::unique_ptr<ValueUpdate> update) + : ValueUpdate(Map), + _key(std::move(key)), _update(std::move(update)) {} -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 MapValueUpdate::operator==(const ValueUpdate& other) const { - if (other.getClass().id() != MapValueUpdate::classId) return false; + if (other.getType() != Map) return false; const MapValueUpdate& o(static_cast<const MapValueUpdate&>(other)); if (*_key != *o._key) return false; if (*_update != *o._update) return false; diff --git a/document/src/vespa/document/update/mapvalueupdate.h b/document/src/vespa/document/update/mapvalueupdate.h index 722255dd8d6..dfbb880f422 100644 --- a/document/src/vespa/document/update/mapvalueupdate.h +++ b/document/src/vespa/document/update/mapvalueupdate.h @@ -16,7 +16,7 @@ namespace document { -class MapValueUpdate : public ValueUpdate { +class MapValueUpdate final : public ValueUpdate { public: /** @@ -26,9 +26,9 @@ 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, std::unique_ptr<ValueUpdate> update); - MapValueUpdate(const MapValueUpdate &); - MapValueUpdate & operator = (const MapValueUpdate &); + MapValueUpdate(std::unique_ptr<FieldValue> key, std::unique_ptr<ValueUpdate> update); + MapValueUpdate(const MapValueUpdate &) = delete; + MapValueUpdate & operator = (const MapValueUpdate &) = delete; MapValueUpdate(MapValueUpdate &&) = default; MapValueUpdate & operator = (MapValueUpdate &&) = default; @@ -59,7 +59,6 @@ public: void print(std::ostream& out, bool verbose, const std::string& indent) const override; void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream& buffer) override; - 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. @@ -67,7 +66,7 @@ private: // Used by ValueUpdate's static factory function // Private because it generates an invalid object. friend class ValueUpdate; - MapValueUpdate() : ValueUpdate(), _key(), _update() {} + MapValueUpdate() : ValueUpdate(Map), _key(), _update() {} ACCEPT_UPDATE_VISITOR; }; diff --git a/document/src/vespa/document/update/removevalueupdate.cpp b/document/src/vespa/document/update/removevalueupdate.cpp index a61553da4d1..3c381148e94 100644 --- a/document/src/vespa/document/update/removevalueupdate.cpp +++ b/document/src/vespa/document/update/removevalueupdate.cpp @@ -16,11 +16,9 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(RemoveValueUpdate, ValueUpdate); - -RemoveValueUpdate::RemoveValueUpdate(const FieldValue& key) - : ValueUpdate(), - _key(key.clone()) +RemoveValueUpdate::RemoveValueUpdate(std::unique_ptr<FieldValue> key) + : ValueUpdate(Remove), + _key(std::move(key)) {} RemoveValueUpdate::~RemoveValueUpdate() = default; @@ -28,7 +26,7 @@ RemoveValueUpdate::~RemoveValueUpdate() = default; bool RemoveValueUpdate::operator==(const ValueUpdate& other) const { - if (other.getClass().id() != RemoveValueUpdate::classId) return false; + if (other.getType() != Remove) return false; const RemoveValueUpdate& o(static_cast<const RemoveValueUpdate&>(other)); if (*_key != *o._key) return false; return true; diff --git a/document/src/vespa/document/update/removevalueupdate.h b/document/src/vespa/document/update/removevalueupdate.h index 0eea8f69da7..02f8204de23 100644 --- a/document/src/vespa/document/update/removevalueupdate.h +++ b/document/src/vespa/document/update/removevalueupdate.h @@ -10,22 +10,22 @@ namespace document { -class RemoveValueUpdate : public ValueUpdate { - FieldValue::CP _key; // The field value to remove by this update. +class RemoveValueUpdate final : public ValueUpdate { + std::unique_ptr<FieldValue> _key; // The field value to remove by this update. - RemoveValueUpdate() : ValueUpdate(), _key() {} ACCEPT_UPDATE_VISITOR; - + friend ValueUpdate; + RemoveValueUpdate() : ValueUpdate(Remove), _key() {} public: - typedef std::unique_ptr<RemoveValueUpdate> UP; - /** * The default constructor requires initial values for all member variables. * * @param value The identifier of the field value to update. */ - RemoveValueUpdate(const FieldValue& key); - ~RemoveValueUpdate(); + explicit RemoveValueUpdate(std::unique_ptr<FieldValue> key); + RemoveValueUpdate(const RemoveValueUpdate &) = delete; + RemoveValueUpdate & operator=(const RemoveValueUpdate &) = delete; + ~RemoveValueUpdate() override; bool operator==(const ValueUpdate& other) const override; @@ -40,9 +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& buffer) override; - - 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 2d6b6e1d658..4110a94693f 100644 --- a/document/src/vespa/document/update/tensor_add_update.cpp +++ b/document/src/vespa/document/update/tensor_add_update.cpp @@ -22,43 +22,26 @@ using vespalib::eval::FastValueBuilderFactory; namespace document { -IMPLEMENT_IDENTIFIABLE(TensorAddUpdate, ValueUpdate); - TensorAddUpdate::TensorAddUpdate() - : _tensor() + : ValueUpdate(TensorAdd), + TensorUpdate(), + _tensor() { } -TensorAddUpdate::TensorAddUpdate(const TensorAddUpdate &rhs) - : _tensor(rhs._tensor->clone()) -{ -} - -TensorAddUpdate::TensorAddUpdate(std::unique_ptr<TensorFieldValue> &&tensor) - : _tensor(std::move(tensor)) +TensorAddUpdate::TensorAddUpdate(std::unique_ptr<TensorFieldValue> tensor) + : ValueUpdate(TensorAdd), + TensorUpdate(), + _tensor(std::move(tensor)) { } TensorAddUpdate::~TensorAddUpdate() = default; -TensorAddUpdate & -TensorAddUpdate::operator=(const TensorAddUpdate &rhs) -{ - _tensor.reset(rhs._tensor->clone()); - return *this; -} - -TensorAddUpdate & -TensorAddUpdate::operator=(TensorAddUpdate &&rhs) -{ - _tensor = std::move(rhs._tensor); - return *this; -} - bool TensorAddUpdate::operator==(const ValueUpdate &other) const { - if (other.getClass().id() != TensorAddUpdate::classId) { + if (other.getType() != TensorAdd) { return false; } const TensorAddUpdate& o(static_cast<const TensorAddUpdate&>(other)); diff --git a/document/src/vespa/document/update/tensor_add_update.h b/document/src/vespa/document/update/tensor_add_update.h index 7f2228bff41..20e231b4294 100644 --- a/document/src/vespa/document/update/tensor_add_update.h +++ b/document/src/vespa/document/update/tensor_add_update.h @@ -14,17 +14,17 @@ class TensorFieldValue; * * The cells to add are contained in a tensor of the same type. */ -class TensorAddUpdate : public ValueUpdate, public TensorUpdate { +class TensorAddUpdate final : public ValueUpdate, public TensorUpdate { std::unique_ptr<TensorFieldValue> _tensor; + friend ValueUpdate; TensorAddUpdate(); - TensorAddUpdate(const TensorAddUpdate &rhs); ACCEPT_UPDATE_VISITOR; public: - TensorAddUpdate(std::unique_ptr<TensorFieldValue> &&tensor); + explicit TensorAddUpdate(std::unique_ptr<TensorFieldValue> tensor); + TensorAddUpdate(const TensorAddUpdate &rhs) = delete; + TensorAddUpdate &operator=(const TensorAddUpdate &rhs) = delete; ~TensorAddUpdate() override; - TensorAddUpdate &operator=(const TensorAddUpdate &rhs); - TensorAddUpdate &operator=(TensorAddUpdate &&rhs); bool operator==(const ValueUpdate &other) const override; const TensorFieldValue &getTensor() const { return *_tensor; } void checkCompatibility(const Field &field) const override; @@ -35,8 +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; - - 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 77fd5059b0c..92b2a8672c3 100644 --- a/document/src/vespa/document/update/tensor_modify_update.cpp +++ b/document/src/vespa/document/update/tensor_modify_update.cpp @@ -82,25 +82,19 @@ convertToCompatibleType(const TensorDataType &tensorType) } -IMPLEMENT_IDENTIFIABLE(TensorModifyUpdate, ValueUpdate); - TensorModifyUpdate::TensorModifyUpdate() - : _operation(Operation::MAX_NUM_OPERATIONS), + : ValueUpdate(TensorModify), + TensorUpdate(), + _operation(Operation::MAX_NUM_OPERATIONS), _tensorType(), _tensor() { } -TensorModifyUpdate::TensorModifyUpdate(const TensorModifyUpdate &rhs) - : _operation(rhs._operation), - _tensorType(std::make_unique<TensorDataType>(*rhs._tensorType)), - _tensor(static_cast<TensorFieldValue *>(_tensorType->createFieldValue().release())) -{ - *_tensor = *rhs._tensor; -} - TensorModifyUpdate::TensorModifyUpdate(Operation operation, std::unique_ptr<TensorFieldValue> tensor) - : _operation(operation), + : ValueUpdate(TensorModify), + TensorUpdate(), + _operation(operation), _tensorType(std::make_unique<TensorDataType>(dynamic_cast<const TensorDataType &>(*tensor->getDataType()))), _tensor(static_cast<TensorFieldValue *>(_tensorType->createFieldValue().release())) { @@ -109,32 +103,10 @@ TensorModifyUpdate::TensorModifyUpdate(Operation operation, std::unique_ptr<Tens TensorModifyUpdate::~TensorModifyUpdate() = default; -TensorModifyUpdate & -TensorModifyUpdate::operator=(const TensorModifyUpdate &rhs) -{ - if (&rhs != this) { - _operation = rhs._operation; - _tensor.reset(); - _tensorType = std::make_unique<TensorDataType>(*rhs._tensorType); - _tensor.reset(dynamic_cast<TensorFieldValue *>(_tensorType->createFieldValue().release())); - *_tensor = *rhs._tensor; - } - return *this; -} - -TensorModifyUpdate & -TensorModifyUpdate::operator=(TensorModifyUpdate &&rhs) -{ - _operation = rhs._operation; - _tensorType = std::move(rhs._tensorType); - _tensor = std::move(rhs._tensor); - return *this; -} - bool TensorModifyUpdate::operator==(const ValueUpdate &other) const { - if (other.getClass().id() != TensorModifyUpdate::classId) { + if (other.getType() != TensorModify) { return false; } const TensorModifyUpdate& o(static_cast<const TensorModifyUpdate&>(other)); diff --git a/document/src/vespa/document/update/tensor_modify_update.h b/document/src/vespa/document/update/tensor_modify_update.h index b6d339a36cf..9386b4f8a1b 100644 --- a/document/src/vespa/document/update/tensor_modify_update.h +++ b/document/src/vespa/document/update/tensor_modify_update.h @@ -16,7 +16,7 @@ class TensorFieldValue; * The operand is represented as a tensor field value containing a * mapped (aka sparse) tensor. */ -class TensorModifyUpdate : public ValueUpdate, public TensorUpdate { +class TensorModifyUpdate final : public ValueUpdate, public TensorUpdate { public: /** Declare all types of tensor modify updates. */ enum class Operation { // Operation to be applied to matching tensor cells @@ -30,14 +30,15 @@ private: std::unique_ptr<const TensorDataType> _tensorType; std::unique_ptr<TensorFieldValue> _tensor; + friend ValueUpdate; TensorModifyUpdate(); - TensorModifyUpdate(const TensorModifyUpdate &rhs); ACCEPT_UPDATE_VISITOR; public: TensorModifyUpdate(Operation operation, std::unique_ptr<TensorFieldValue> tensor); + TensorModifyUpdate(const TensorModifyUpdate &rhs) = delete; + TensorModifyUpdate &operator=(const TensorModifyUpdate &rhs) = delete; ~TensorModifyUpdate() override; - TensorModifyUpdate &operator=(const TensorModifyUpdate &rhs); - TensorModifyUpdate &operator=(TensorModifyUpdate &&rhs); + bool operator==(const ValueUpdate &other) const override; Operation getOperation() const { return _operation; } const TensorFieldValue &getTensor() const { return *_tensor; } @@ -49,8 +50,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; - - 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 69b8b898ec5..25af29ce6b8 100644 --- a/document/src/vespa/document/update/tensor_remove_update.cpp +++ b/document/src/vespa/document/update/tensor_remove_update.cpp @@ -38,53 +38,28 @@ convertToCompatibleType(const TensorDataType &tensorType) } -IMPLEMENT_IDENTIFIABLE(TensorRemoveUpdate, ValueUpdate); - TensorRemoveUpdate::TensorRemoveUpdate() - : _tensorType(), + : ValueUpdate(TensorRemove), + TensorUpdate(), + _tensorType(), _tensor() { } -TensorRemoveUpdate::TensorRemoveUpdate(const TensorRemoveUpdate &rhs) - : _tensorType(std::make_unique<TensorDataType>(*rhs._tensorType)), - _tensor(rhs._tensor->clone()) -{ -} - TensorRemoveUpdate::TensorRemoveUpdate(std::unique_ptr<TensorFieldValue> tensor) - : _tensorType(std::make_unique<TensorDataType>(dynamic_cast<const TensorDataType &>(*tensor->getDataType()))), - _tensor(static_cast<TensorFieldValue *>(_tensorType->createFieldValue().release())) + : ValueUpdate(TensorRemove), + TensorUpdate(), + _tensorType(std::make_unique<TensorDataType>(dynamic_cast<const TensorDataType &>(*tensor->getDataType()))), + _tensor(std::move(tensor)) { - *_tensor = *tensor; } TensorRemoveUpdate::~TensorRemoveUpdate() = default; -TensorRemoveUpdate & -TensorRemoveUpdate::operator=(const TensorRemoveUpdate &rhs) -{ - if (&rhs != this) { - _tensor.reset(); - _tensorType = std::make_unique<TensorDataType>(*rhs._tensorType); - _tensor.reset(static_cast<TensorFieldValue *>(_tensorType->createFieldValue().release())); - *_tensor = *rhs._tensor; - } - return *this; -} - -TensorRemoveUpdate & -TensorRemoveUpdate::operator=(TensorRemoveUpdate &&rhs) -{ - _tensorType = std::move(rhs._tensorType); - _tensor = std::move(rhs._tensor); - return *this; -} - bool TensorRemoveUpdate::operator==(const ValueUpdate &other) const { - if (other.getClass().id() != TensorRemoveUpdate::classId) { + if (other.getType() != TensorRemove) { return false; } const TensorRemoveUpdate& o(static_cast<const TensorRemoveUpdate&>(other)); diff --git a/document/src/vespa/document/update/tensor_remove_update.h b/document/src/vespa/document/update/tensor_remove_update.h index 9b4ea17c4d9..ca908fc75fc 100644 --- a/document/src/vespa/document/update/tensor_remove_update.h +++ b/document/src/vespa/document/update/tensor_remove_update.h @@ -16,20 +16,20 @@ class TensorFieldValue; * The cells to remove are contained in a sparse tensor (with all mapped dimensions) where cell values are set to 1.0. * When used on a mixed tensor the entire dense sub-space (pointed to by a cell in the sparse tensor) is removed. */ -class TensorRemoveUpdate : public ValueUpdate, public TensorUpdate { +class TensorRemoveUpdate final : public ValueUpdate, public TensorUpdate { private: std::unique_ptr<const TensorDataType> _tensorType; std::unique_ptr<TensorFieldValue> _tensor; + friend ValueUpdate; TensorRemoveUpdate(); - TensorRemoveUpdate(const TensorRemoveUpdate &rhs); ACCEPT_UPDATE_VISITOR; public: - TensorRemoveUpdate(std::unique_ptr<TensorFieldValue> tensor); + explicit TensorRemoveUpdate(std::unique_ptr<TensorFieldValue> tensor); + TensorRemoveUpdate(const TensorRemoveUpdate &rhs) = delete; + TensorRemoveUpdate &operator=(const TensorRemoveUpdate &rhs) = delete; ~TensorRemoveUpdate() override; - TensorRemoveUpdate &operator=(const TensorRemoveUpdate &rhs); - TensorRemoveUpdate &operator=(TensorRemoveUpdate &&rhs); const TensorFieldValue &getTensor() const { return *_tensor; } std::unique_ptr<vespalib::eval::Value> applyTo(const vespalib::eval::Value &tensor) const; std::unique_ptr<Value> apply_to(const Value &tensor, @@ -40,8 +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; - - DECLARE_IDENTIFIABLE(TensorRemoveUpdate); }; } diff --git a/document/src/vespa/document/update/valueupdate.cpp b/document/src/vespa/document/update/valueupdate.cpp index 8fc85e8858a..50866311518 100644 --- a/document/src/vespa/document/update/valueupdate.cpp +++ b/document/src/vespa/document/update/valueupdate.cpp @@ -1,12 +1,71 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "valueupdate.h" +#include "addvalueupdate.h" +#include "assignvalueupdate.h" +#include "arithmeticvalueupdate.h" +#include "clearvalueupdate.h" +#include "removevalueupdate.h" +#include "mapvalueupdate.h" +#include "tensor_add_update.h" +#include "tensor_modify_update.h" +#include "tensor_remove_update.h" #include <vespa/vespalib/util/stringfmt.h> #include <stdexcept> namespace document { -IMPLEMENT_IDENTIFIABLE_ABSTRACT(ValueUpdate, Identifiable); +const char * +ValueUpdate::className() const noexcept { + switch (getType()) { + case Add: + return "AddValueUpdate"; + case Assign: + return "AssignValueUpdate"; + case Arithmetic: + return "ArithmeticValueUpdate"; + case Clear: + return "ClearValueUpdate"; + case Remove: + return "RemoveValueUpdate"; + case Map: + return "MapValueUpdate"; + case TensorAdd: + return "TensorAddUpdate"; + case TensorModify: + return "TensorModifyUpdate"; + case TensorRemove: + return "TensorRemoveUpdate"; + default: + abort(); + } +} + +std::unique_ptr<ValueUpdate> +ValueUpdate::create(ValueUpdateType type) { + switch (type) { + case Add: + return std::unique_ptr<AddValueUpdate>(new AddValueUpdate()); + case Assign: + return std::make_unique<AssignValueUpdate>(); + case Arithmetic: + return std::unique_ptr<ArithmeticValueUpdate>(new ArithmeticValueUpdate()); + case Clear: + return std::make_unique<ClearValueUpdate>(); + case Remove: + return std::unique_ptr<RemoveValueUpdate>(new RemoveValueUpdate()); + case Map: + return std::unique_ptr<MapValueUpdate>( new MapValueUpdate()); + case TensorAdd: + return std::unique_ptr<TensorAddUpdate>( new TensorAddUpdate()); + case TensorModify: + return std::unique_ptr<TensorModifyUpdate>( new TensorModifyUpdate()); + case TensorRemove: + return std::unique_ptr<TensorRemoveUpdate>( new TensorRemoveUpdate()); + default: + throw std::runtime_error(vespalib::make_string("Could not find a class for classId %d(%x)", type, type)); + } +} std::unique_ptr<ValueUpdate> ValueUpdate::createInstance(const DocumentTypeRepo& repo, const DataType& type, nbostream & stream) @@ -14,16 +73,12 @@ ValueUpdate::createInstance(const DocumentTypeRepo& repo, const DataType& type, int32_t classId = 0; stream >> classId; - const Identifiable::RuntimeClass * rtc(Identifiable::classFromId(classId)); - if (rtc != nullptr) { - std::unique_ptr<ValueUpdate> update(static_cast<ValueUpdate*>(rtc->create())); - /// \todo TODO (was warning): Updates are not versioned in serialization format. Will not work without altering it. - /// Should also use the serializer, not this deserialize into self. - update->deserialize(repo, type, stream); - return update; - } else { - throw std::runtime_error(vespalib::make_string("Could not find a class for classId %d(%x)", classId, classId)); - } + std::unique_ptr<ValueUpdate> update = create(static_cast<ValueUpdateType>(classId)); + + /// \todo TODO (was warning): Updates are not versioned in serialization format. Will not work without altering it. + /// Should also use the serializer, not this deserialize into self. + update->deserialize(repo, type, stream); + return update; } std::ostream& diff --git a/document/src/vespa/document/update/valueupdate.h b/document/src/vespa/document/update/valueupdate.h index 0b52fe46ac9..c97de4d54e1 100644 --- a/document/src/vespa/document/update/valueupdate.h +++ b/document/src/vespa/document/update/valueupdate.h @@ -30,7 +30,7 @@ class Field; class FieldValue; class DataType; -class ValueUpdate : public vespalib::Identifiable +class ValueUpdate { protected: using nbostream = vespalib::nbostream; @@ -53,11 +53,12 @@ public: Clear = IDENTIFIABLE_CLASSID(ClearValueUpdate), Map = IDENTIFIABLE_CLASSID(MapValueUpdate), Remove = IDENTIFIABLE_CLASSID(RemoveValueUpdate), - TensorModifyUpdate = IDENTIFIABLE_CLASSID(TensorModifyUpdate), - TensorAddUpdate = IDENTIFIABLE_CLASSID(TensorAddUpdate), - TensorRemoveUpdate = IDENTIFIABLE_CLASSID(TensorRemoveUpdate) + TensorModify = IDENTIFIABLE_CLASSID(TensorModifyUpdate), + TensorAdd = IDENTIFIABLE_CLASSID(TensorAddUpdate), + TensorRemove = IDENTIFIABLE_CLASSID(TensorRemoveUpdate) }; + virtual ~ValueUpdate() = default; virtual bool operator==(const ValueUpdate&) const = 0; bool operator != (const ValueUpdate & rhs) const { return ! (*this == rhs); } @@ -85,10 +86,8 @@ public: virtual void deserialize(const DocumentTypeRepo& repo, const DataType& type, nbostream & stream) = 0; /** @return The operation type. */ - ValueUpdateType getType() const { - return static_cast<ValueUpdateType>(getClass().id()); - } - + ValueUpdateType getType() const noexcept { return _type; } + const char * className() const noexcept; /** * Visit this fieldvalue for double dispatch. */ @@ -96,8 +95,11 @@ public: 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); +protected: + ValueUpdate(ValueUpdateType type) : _type(type) { } +private: + static std::unique_ptr<ValueUpdate> create(ValueUpdateType type); + ValueUpdateType _type; }; std::ostream& operator<<(std::ostream& out, const ValueUpdate & p); diff --git a/document/src/vespa/document/util/feed_reject_helper.cpp b/document/src/vespa/document/util/feed_reject_helper.cpp index a6829ec0c60..3dec889661b 100644 --- a/document/src/vespa/document/util/feed_reject_helper.cpp +++ b/document/src/vespa/document/util/feed_reject_helper.cpp @@ -17,8 +17,8 @@ FeedRejectHelper::mustReject(const document::ValueUpdate & valueUpdate) { using namespace document; switch (valueUpdate.getType()) { case ValueUpdate::Add: - case ValueUpdate::TensorAddUpdate: - case ValueUpdate::TensorModifyUpdate: + case ValueUpdate::TensorAdd: + case ValueUpdate::TensorModify: case ValueUpdate::Map: return true; case ValueUpdate::Assign: { |