From 8976efd142145482d61d83755db2fab0a8b626a6 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 28 Mar 2022 09:57:50 +0000 Subject: Avoid Identifiable for ValueUpdate. It complicates without bringing much useful. --- document/src/tests/documentupdatetestcase.cpp | 2 +- .../serialization/vespadocumentserializer.cpp | 18 +++--- .../src/vespa/document/update/addvalueupdate.cpp | 6 +- .../src/vespa/document/update/addvalueupdate.h | 6 +- .../document/update/arithmeticvalueupdate.cpp | 4 +- .../vespa/document/update/arithmeticvalueupdate.h | 8 +-- .../vespa/document/update/assignvalueupdate.cpp | 11 ++-- .../src/vespa/document/update/assignvalueupdate.h | 6 +- .../src/vespa/document/update/clearvalueupdate.cpp | 4 +- .../src/vespa/document/update/clearvalueupdate.h | 8 +-- .../src/vespa/document/update/mapvalueupdate.cpp | 8 +-- .../src/vespa/document/update/mapvalueupdate.h | 5 +- .../vespa/document/update/removevalueupdate.cpp | 6 +- .../src/vespa/document/update/removevalueupdate.h | 9 +-- .../vespa/document/update/tensor_add_update.cpp | 16 +++-- .../src/vespa/document/update/tensor_add_update.h | 5 +- .../vespa/document/update/tensor_modify_update.cpp | 16 +++-- .../vespa/document/update/tensor_modify_update.h | 5 +- .../vespa/document/update/tensor_remove_update.cpp | 16 +++-- .../vespa/document/update/tensor_remove_update.h | 5 +- document/src/vespa/document/update/valueupdate.cpp | 75 ++++++++++++++++++---- document/src/vespa/document/update/valueupdate.h | 22 ++++--- .../src/vespa/document/util/feed_reject_helper.cpp | 4 +- 23 files changed, 154 insertions(+), 111 deletions(-) (limited to 'document') diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index beb94959e73..93ab1c370c5 100644 --- a/document/src/tests/documentupdatetestcase.cpp +++ b/document/src/tests/documentupdatetestcase.cpp @@ -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]; diff --git a/document/src/vespa/document/serialization/vespadocumentserializer.cpp b/document/src/vespa/document/serialization/vespadocumentserializer.cpp index b6a7dabd87d..c156594f1f0 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 << ValueUpdate::Remove; write(value.getKey()); } @@ -385,7 +385,7 @@ VespaDocumentSerializer::write(const RemoveValueUpdate &value) void VespaDocumentSerializer::write(const AddValueUpdate &value) { - _stream << AddValueUpdate::classId; + _stream << ValueUpdate::Add; write(value.getValue()); _stream << static_cast(value.getWeight()); } @@ -393,7 +393,7 @@ VespaDocumentSerializer::write(const AddValueUpdate &value) void VespaDocumentSerializer::write(const ArithmeticValueUpdate &value) { - _stream << ArithmeticValueUpdate::classId; + _stream << ValueUpdate::Arithmetic; _stream << static_cast(value.getOperator()); _stream << static_cast(value.getOperand()); } @@ -401,7 +401,7 @@ VespaDocumentSerializer::write(const ArithmeticValueUpdate &value) void VespaDocumentSerializer::write(const AssignValueUpdate &value) { - _stream << AssignValueUpdate::classId; + _stream << ValueUpdate::Assign; if (value.hasValue()) { _stream << static_cast(CONTENT_HASVALUE); write(value.getValue()); @@ -414,12 +414,12 @@ void VespaDocumentSerializer::write(const ClearValueUpdate &value) { (void) value; - _stream << ClearValueUpdate::classId; + _stream << ValueUpdate::Clear; } void VespaDocumentSerializer::write(const MapValueUpdate &value) { - _stream << MapValueUpdate::classId; + _stream << 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 << ValueUpdate::TensorModify; _stream << static_cast(value.getOperation()); write(value.getTensor()); } @@ -493,7 +493,7 @@ VespaDocumentSerializer::visit(const TensorModifyUpdate &value) void VespaDocumentSerializer::write(const TensorAddUpdate &value) { - _stream << TensorAddUpdate::classId; + _stream << ValueUpdate::TensorAdd; write(value.getTensor()); } @@ -506,7 +506,7 @@ VespaDocumentSerializer::visit(const TensorAddUpdate &value) void VespaDocumentSerializer::write(const TensorRemoveUpdate &value) { - _stream << TensorRemoveUpdate::classId; + _stream << 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..3b593d3e9b2 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(), + : ValueUpdate(Add), _value(value.clone()), _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(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..2ab7a98e842 100644 --- a/document/src/vespa/document/update/addvalueupdate.h +++ b/document/src/vespa/document/update/addvalueupdate.h @@ -12,14 +12,14 @@ namespace document { -class AddValueUpdate : public ValueUpdate { +class AddValueUpdate final : public ValueUpdate { FieldValue::CP _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(0), _weight(1) {} ACCEPT_UPDATE_VISITOR; public: typedef std::unique_ptr UP; @@ -67,8 +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; - - 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(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..c82a5b4c338 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 { @@ -31,7 +31,7 @@ private: // Private because it generates an invalid object. friend class ValueUpdate; ArithmeticValueUpdate() - : ValueUpdate(), + : ValueUpdate(Arithmetic), _operator(MAX_NUM_OPERATORS), _operand(0.0) {} @@ -46,7 +46,7 @@ public: * @param opn The operand for the operation. */ ArithmeticValueUpdate(Operator opt, double opn) - : ValueUpdate(), + : ValueUpdate(Arithmetic), _operator(opt), _operand(opn) {} @@ -91,8 +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; - - DECLARE_IDENTIFIABLE(ArithmeticValueUpdate); }; } // document diff --git a/document/src/vespa/document/update/assignvalueupdate.cpp b/document/src/vespa/document/update/assignvalueupdate.cpp index cf2321dbf56..363eff92bf0 100644 --- a/document/src/vespa/document/update/assignvalueupdate.cpp +++ b/document/src/vespa/document/update/assignvalueupdate.cpp @@ -16,12 +16,13 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(AssignValueUpdate, ValueUpdate); - -AssignValueUpdate::AssignValueUpdate() = default; +AssignValueUpdate::AssignValueUpdate() + : ValueUpdate(Assign), + _value() +{} AssignValueUpdate::AssignValueUpdate(const FieldValue& value) - : ValueUpdate(), + : ValueUpdate(Assign), _value(value.clone()) { } @@ -33,7 +34,7 @@ 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(other)); return _value == o._value; } diff --git a/document/src/vespa/document/update/assignvalueupdate.h b/document/src/vespa/document/update/assignvalueupdate.h index e829e80da45..9df2084ef97 100644 --- a/document/src/vespa/document/update/assignvalueupdate.h +++ b/document/src/vespa/document/update/assignvalueupdate.h @@ -16,15 +16,13 @@ namespace document { -class AssignValueUpdate : public ValueUpdate { +class AssignValueUpdate final : public ValueUpdate { FieldValue::CP _value; ACCEPT_UPDATE_VISITOR; public: typedef std::unique_ptr UP; - AssignValueUpdate(); - AssignValueUpdate(const FieldValue& value); ~AssignValueUpdate() override; @@ -43,8 +41,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..2a68814f8b0 100644 --- a/document/src/vespa/document/update/clearvalueupdate.h +++ b/document/src/vespa/document/update/clearvalueupdate.h @@ -11,12 +11,12 @@ namespace document { -class ClearValueUpdate : public ValueUpdate { +class ClearValueUpdate final : public ValueUpdate { ACCEPT_UPDATE_VISITOR; public: typedef std::unique_ptr UP; - ClearValueUpdate() : ValueUpdate() {} - ClearValueUpdate(const ClearValueUpdate& update) : ValueUpdate(update) {} + ClearValueUpdate(const ClearValueUpdate& update) = default; + ClearValueUpdate() : ValueUpdate(Clear) {} ClearValueUpdate &operator=(const ClearValueUpdate &rhs) = default; bool operator==(const ValueUpdate& other) const override; @@ -25,8 +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; - - DECLARE_IDENTIFIABLE(ClearValueUpdate); }; } diff --git a/document/src/vespa/document/update/mapvalueupdate.cpp b/document/src/vespa/document/update/mapvalueupdate.cpp index c3d9cff571b..93f2029b92f 100644 --- a/document/src/vespa/document/update/mapvalueupdate.cpp +++ b/document/src/vespa/document/update/mapvalueupdate.cpp @@ -16,16 +16,14 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(MapValueUpdate, ValueUpdate); - MapValueUpdate::MapValueUpdate(const FieldValue& key, std::unique_ptr update) - : ValueUpdate(), + : ValueUpdate(Map), _key(key.clone()), _update(std::move(update)) {} MapValueUpdate::MapValueUpdate(const MapValueUpdate &) - : ValueUpdate(), + : ValueUpdate(Map), _key(), _update() { @@ -41,7 +39,7 @@ 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(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..93d6c844899 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: /** @@ -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 _key; // The field value this update is mapping to. std::unique_ptr _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..4789df8d7ab 100644 --- a/document/src/vespa/document/update/removevalueupdate.cpp +++ b/document/src/vespa/document/update/removevalueupdate.cpp @@ -16,10 +16,8 @@ using namespace vespalib::xml; namespace document { -IMPLEMENT_IDENTIFIABLE(RemoveValueUpdate, ValueUpdate); - RemoveValueUpdate::RemoveValueUpdate(const FieldValue& key) - : ValueUpdate(), + : ValueUpdate(Remove), _key(key.clone()) {} @@ -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(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..0c57817ce33 100644 --- a/document/src/vespa/document/update/removevalueupdate.h +++ b/document/src/vespa/document/update/removevalueupdate.h @@ -10,12 +10,12 @@ namespace document { -class RemoveValueUpdate : public ValueUpdate { +class RemoveValueUpdate final : public ValueUpdate { FieldValue::CP _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 UP; @@ -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..49c73722147 100644 --- a/document/src/vespa/document/update/tensor_add_update.cpp +++ b/document/src/vespa/document/update/tensor_add_update.cpp @@ -22,20 +22,24 @@ 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()) + : ValueUpdate(rhs), + TensorUpdate(rhs), + _tensor(rhs._tensor->clone()) { } TensorAddUpdate::TensorAddUpdate(std::unique_ptr &&tensor) - : _tensor(std::move(tensor)) + : ValueUpdate(TensorAdd), + TensorUpdate(), + _tensor(std::move(tensor)) { } @@ -58,7 +62,7 @@ TensorAddUpdate::operator=(TensorAddUpdate &&rhs) bool TensorAddUpdate::operator==(const ValueUpdate &other) const { - if (other.getClass().id() != TensorAddUpdate::classId) { + if (other.getType() != TensorAdd) { return false; } const TensorAddUpdate& o(static_cast(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..7e62f2db71a 100644 --- a/document/src/vespa/document/update/tensor_add_update.h +++ b/document/src/vespa/document/update/tensor_add_update.h @@ -14,9 +14,10 @@ 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 _tensor; + friend ValueUpdate; TensorAddUpdate(); TensorAddUpdate(const TensorAddUpdate &rhs); ACCEPT_UPDATE_VISITOR; @@ -35,8 +36,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..6abed60af17 100644 --- a/document/src/vespa/document/update/tensor_modify_update.cpp +++ b/document/src/vespa/document/update/tensor_modify_update.cpp @@ -82,17 +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), + : ValueUpdate(rhs), + TensorUpdate(), + _operation(rhs._operation), _tensorType(std::make_unique(*rhs._tensorType)), _tensor(static_cast(_tensorType->createFieldValue().release())) { @@ -100,7 +102,9 @@ TensorModifyUpdate::TensorModifyUpdate(const TensorModifyUpdate &rhs) } TensorModifyUpdate::TensorModifyUpdate(Operation operation, std::unique_ptr tensor) - : _operation(operation), + : ValueUpdate(TensorModify), + TensorUpdate(), + _operation(operation), _tensorType(std::make_unique(dynamic_cast(*tensor->getDataType()))), _tensor(static_cast(_tensorType->createFieldValue().release())) { @@ -134,7 +138,7 @@ TensorModifyUpdate::operator=(TensorModifyUpdate &&rhs) bool TensorModifyUpdate::operator==(const ValueUpdate &other) const { - if (other.getClass().id() != TensorModifyUpdate::classId) { + if (other.getType() != TensorModify) { return false; } const TensorModifyUpdate& o(static_cast(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..9fb69ec5a13 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,6 +30,7 @@ private: std::unique_ptr _tensorType; std::unique_ptr _tensor; + friend ValueUpdate; TensorModifyUpdate(); TensorModifyUpdate(const TensorModifyUpdate &rhs); ACCEPT_UPDATE_VISITOR; @@ -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..4bfb6bc6b18 100644 --- a/document/src/vespa/document/update/tensor_remove_update.cpp +++ b/document/src/vespa/document/update/tensor_remove_update.cpp @@ -38,22 +38,26 @@ 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(*rhs._tensorType)), + : ValueUpdate(rhs), + TensorUpdate(rhs), + _tensorType(std::make_unique(*rhs._tensorType)), _tensor(rhs._tensor->clone()) { } TensorRemoveUpdate::TensorRemoveUpdate(std::unique_ptr tensor) - : _tensorType(std::make_unique(dynamic_cast(*tensor->getDataType()))), + : ValueUpdate(TensorRemove), + TensorUpdate(), + _tensorType(std::make_unique(dynamic_cast(*tensor->getDataType()))), _tensor(static_cast(_tensorType->createFieldValue().release())) { *_tensor = *tensor; @@ -84,7 +88,7 @@ TensorRemoveUpdate::operator=(TensorRemoveUpdate &&rhs) bool TensorRemoveUpdate::operator==(const ValueUpdate &other) const { - if (other.getClass().id() != TensorRemoveUpdate::classId) { + if (other.getType() != TensorRemove) { return false; } const TensorRemoveUpdate& o(static_cast(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..00c886fa41f 100644 --- a/document/src/vespa/document/update/tensor_remove_update.h +++ b/document/src/vespa/document/update/tensor_remove_update.h @@ -16,11 +16,12 @@ 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 _tensorType; std::unique_ptr _tensor; + friend ValueUpdate; TensorRemoveUpdate(); TensorRemoveUpdate(const TensorRemoveUpdate &rhs); ACCEPT_UPDATE_VISITOR; @@ -40,8 +41,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..4af61178a79 100644 --- a/document/src/vespa/document/update/valueupdate.cpp +++ b/document/src/vespa/document/update/valueupdate.cpp @@ -1,12 +1,69 @@ // 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 #include 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"; + } + abort(); +} + +std::unique_ptr +ValueUpdate::create(ValueUpdateType type) { + switch (type) { + case Add: + return std::unique_ptr(new AddValueUpdate()); + case Assign: + return std::make_unique(); + case Arithmetic: + return std::unique_ptr(new ArithmeticValueUpdate()); + case Clear: + return std::make_unique(); + case Remove: + return std::unique_ptr(new RemoveValueUpdate()); + case Map: + return std::unique_ptr( new MapValueUpdate()); + case TensorAdd: + return std::unique_ptr( new TensorAddUpdate()); + case TensorModify: + return std::unique_ptr( new TensorModifyUpdate()); + case TensorRemove: + return std::unique_ptr( new TensorRemoveUpdate()); + } + throw std::runtime_error(vespalib::make_string("Could not find a class for classId %d(%x)", type, type)); +} std::unique_ptr ValueUpdate::createInstance(const DocumentTypeRepo& repo, const DataType& type, nbostream & stream) @@ -14,16 +71,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 update(static_cast(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 update = create(static_cast(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(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 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: { -- cgit v1.2.3 From 1cd2c6a3ba9e392c00b91bfe3cf1215a80c6679f Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Mon, 28 Mar 2022 11:43:52 +0000 Subject: Remove copy constructors. --- document/src/tests/documentselectparsertest.cpp | 4 +- document/src/tests/documentupdatetestcase.cpp | 119 ++++++++-------- document/src/tests/feed_reject_helper_test.cpp | 10 +- document/src/tests/testxml.cpp | 14 +- document/src/vespa/document/base/forcelink.cpp | 8 +- .../src/vespa/document/update/addvalueupdate.cpp | 4 +- .../src/vespa/document/update/addvalueupdate.h | 21 +-- .../vespa/document/update/arithmeticvalueupdate.h | 10 +- .../vespa/document/update/assignvalueupdate.cpp | 10 +- .../src/vespa/document/update/assignvalueupdate.h | 7 +- .../src/vespa/document/update/clearvalueupdate.h | 5 +- .../src/vespa/document/update/mapvalueupdate.cpp | 16 +-- .../src/vespa/document/update/mapvalueupdate.h | 6 +- .../vespa/document/update/removevalueupdate.cpp | 4 +- .../src/vespa/document/update/removevalueupdate.h | 10 +- .../vespa/document/update/tensor_add_update.cpp | 21 --- .../src/vespa/document/update/tensor_add_update.h | 5 +- .../vespa/document/update/tensor_modify_update.cpp | 32 ----- .../vespa/document/update/tensor_modify_update.h | 6 +- .../vespa/document/update/tensor_remove_update.cpp | 28 ---- .../vespa/document/update/tensor_remove_update.h | 5 +- .../conformancetest/conformancetest.cpp | 154 ++++++--------------- .../src/tests/proton/attribute/attribute_test.cpp | 15 +- .../attribute_updater/attribute_updater_test.cpp | 138 ++++++++---------- .../documentdb/feedhandler/feedhandler_test.cpp | 4 +- .../proton/feedoperation/feedoperation_test.cpp | 2 +- .../persistenceengine/persistenceengine_test.cpp | 2 +- .../src/vespa/searchcore/bmcluster/bm_feed.cpp | 2 +- searchlib/src/tests/attribute/attribute_test.cpp | 19 ++- .../distributor/externaloperationhandlertest.cpp | 21 +-- .../src/tests/persistence/persistencetestutils.cpp | 8 +- .../src/tests/persistence/persistencetestutils.h | 67 +++------ storage/src/tests/persistence/testandsettest.cpp | 11 +- .../src/tests/mbusprot/storageprotocoltest.cpp | 5 +- 34 files changed, 281 insertions(+), 512 deletions(-) (limited to 'document') diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp index a447df1044e..70ebd9bd7a2 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(*_repo, *type, DocumentId(id)); doc->addUpdate(FieldUpdate(doc->getType().getField("headerval")) - .addUpdate(std::make_unique(IntFieldValue(hint)))); + .addUpdate(std::make_unique(std::make_unique(hint)))); doc->addUpdate(FieldUpdate(doc->getType().getField("hstringval")) - .addUpdate(std::make_unique(StringFieldValue(hstr)))); + .addUpdate(std::make_unique(std::make_unique(hstr)))); return doc; } diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index 93ab1c370c5..27a9791b95c 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(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(1), 4), *arrayType); + testRoundtripSerialize(RemoveValueUpdate(std::make_unique(1)), *arrayType); FieldUpdate fieldUpdate(docType->getField("intf")); - fieldUpdate.addUpdate(std::make_unique(IntFieldValue(1))); + fieldUpdate.addUpdate(std::make_unique(std::make_unique(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(IntFieldValue(15)))); + upd.addUpdate(FieldUpdate(docType->getField("intf")).addUpdate(std::make_unique(std::make_unique(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(IntFieldValue(4)))); + upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(std::make_unique(std::make_unique(4)))); upd.applyTo(updated); EXPECT_NE(doc, updated); std::unique_ptr val(dynamic_cast(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(IntFieldValue(3)))); + upd.addUpdate(FieldUpdate(docType->getField("intarr")).addUpdate(std::make_unique(std::make_unique(3)))); upd.applyTo(updated); EXPECT_NE(doc, updated); std::unique_ptr val(dynamic_cast(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(IntFieldValue(9)))) + .addUpdate(FieldUpdate(doc->getField("headerval")).addUpdate(std::make_unique(std::make_unique(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(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(myarray))) + .addUpdate(FieldUpdate(doc->getField("tags")).addUpdate(std::make_unique(std::move(myarray)))) .applyTo(*doc); auto fval1(doc->getAs(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(StringFieldValue("another"))) - .addUpdate(std::make_unique(StringFieldValue("tag")))) + .addUpdate(std::make_unique(std::make_unique("another"))) + .addUpdate(std::make_unique(std::make_unique("tag")))) .applyTo(*doc); std::unique_ptr fval2(doc->getAs(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(StringFieldValue("THROW MEH!")))) + .addUpdate(std::make_unique(std::make_unique("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(StringFieldValue("foo"))) - .addUpdate(std::make_unique(StringFieldValue("tag")))) + .addUpdate(std::make_unique(std::make_unique("foo"))) + .addUpdate(std::make_unique(std::make_unique("tag")))) .applyTo(*doc); auto fval3(doc->getAs(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(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(myarray2))) + .addUpdate(std::make_unique(std::move(myarray2)))) .applyTo(*doc), std::exception) << "Expected exception when removing an array from a string array."; } std::unique_ptr createAddUpdate(vespalib::stringref key, int weight) { - auto upd = std::make_unique(StringFieldValue(key)); + auto upd = std::make_unique(std::make_unique(key)); upd->setWeight(weight); return upd; } std::unique_ptr createAddUpdate(int key, int weight) { - auto upd = std::make_unique(IntFieldValue(key)); + auto upd = std::make_unique(std::make_unique(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(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(wset))) + .addUpdate(FieldUpdate(field).addUpdate(std::make_unique(std::move(wset)))) .applyTo(*doc); auto fval1(doc->getAs(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(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(wset2))) + .addUpdate(std::make_unique(std::move(wset2)))) .applyTo(*doc); auto fval2(doc->getAs(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(StringFieldValue("foo"))) - .addUpdate(std::make_unique(StringFieldValue("too")))) + .addUpdate(std::make_unique(std::make_unique("foo"))) + .addUpdate(std::make_unique(std::make_unique("too")))) .applyTo(*doc); auto fval4(doc->getAs(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(StringFieldValue("foo"), + .addUpdate(std::make_unique(std::make_unique("foo"), std::make_unique(ArithmeticValueUpdate::Add, 1)))); } } // anon ns @@ -457,7 +457,7 @@ TEST(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) { WeightedSetAutoCreateFixture fixture; fixture.update.addUpdate(FieldUpdate(fixture.field) - .addUpdate(std::make_unique(StringFieldValue("baz"), + .addUpdate(std::make_unique(std::make_unique("baz"), std::make_unique(ArithmeticValueUpdate::Add, 0)))); fixture.applyUpdateToDocument(); @@ -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(IntFieldValue(4)))); + .addUpdate(std::make_unique(std::make_unique(4)))); upd.addUpdate(FieldUpdate(type->getField("floatfield")) - .addUpdate(std::make_unique(FloatFieldValue(1.00f)))); + .addUpdate(std::make_unique(std::make_unique(1.00f)))); upd.addUpdate(FieldUpdate(type->getField("arrayoffloatfield")) - .addUpdate(std::make_unique(FloatFieldValue(5.00f))) - .addUpdate(std::make_unique(FloatFieldValue(4.23f))) - .addUpdate(std::make_unique(FloatFieldValue(-1.00f)))); + .addUpdate(std::make_unique(std::make_unique(5.00f))) + .addUpdate(std::make_unique(std::make_unique(4.23f))) + .addUpdate(std::make_unique(std::make_unique(-1.00f)))); upd.addUpdate(FieldUpdate(type->getField("intfield")) .addUpdate(std::make_unique(ArithmeticValueUpdate::Add, 3))); upd.addUpdate(FieldUpdate(type->getField("wsfield")) - .addUpdate(std::make_unique(StringFieldValue("foo"), + .addUpdate(std::make_unique(std::make_unique("foo"), std::make_unique(ArithmeticValueUpdate::Add, 2))) - .addUpdate(std::make_unique(StringFieldValue("foo"), + .addUpdate(std::make_unique(std::make_unique("foo"), std::make_unique(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(FloatFieldValue(4.00f)))), + .addUpdate(std::make_unique(std::make_unique(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(ArrayFieldValue(field.getDataType())))); + .addUpdate(std::make_unique(std::make_unique(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(ArrayFieldValue(field.getDataType())))); + update.addUpdate(FieldUpdate(field).addUpdate(std::make_unique(std::make_unique(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(WeightedSetFieldValue(field.getDataType())))); + update.addUpdate(FieldUpdate(field).addUpdate(std::make_unique(std::make_unique(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(IntFieldValue(123))) - .addUpdate(std::make_unique(IntFieldValue(456)))), + .addUpdate(std::make_unique(std::make_unique(123))) + .addUpdate(std::make_unique(std::make_unique(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(StringFieldValue("banana"), + .addUpdate(std::make_unique(std::make_unique("banana"), std::make_unique(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); std::unique_ptr fv1 = @@ -741,7 +741,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field2) - .addUpdate(std::make_unique(StringFieldValue("banana"), + .addUpdate(std::make_unique(std::make_unique("banana"), std::make_unique(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); auto fv2 = doc->getAs(field2); @@ -770,7 +770,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(std::make_unique(StringFieldValue("apple"), + .addUpdate(std::make_unique(std::make_unique("apple"), std::make_unique(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(StringFieldValue("apple"), + .addUpdate(std::make_unique(std::make_unique("apple"), std::make_unique(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(*newTensor)); + f.applyUpdate(std::make_unique(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(*makeTensor())) + .addUpdate(std::make_unique(makeTensor())) .addUpdate(std::make_unique(makeTensor())) .addUpdate(std::make_unique(makeTensor()))); result->addUpdate(FieldUpdate(getField("dense_tensor")) @@ -1250,7 +1249,7 @@ CreateIfNonExistentFixture::CreateIfNonExistentFixture() update(std::make_unique(docMan.getTypeRepo(), *document->getDataType(), document->getId())) { update->addUpdate(FieldUpdate(document->getField("headerval")) - .addUpdate(std::make_unique(IntFieldValue(1)))); + .addUpdate(std::make_unique(std::make_unique(1)))); update->setCreateIfNonExistent(true); } @@ -1282,8 +1281,8 @@ ArrayUpdateFixture::ArrayUpdateFixture() { update = std::make_unique(doc_man.getTypeRepo(), *doc->getDataType(), doc->getId()); update->addUpdate(FieldUpdate(array_field) - .addUpdate(std::make_unique(IntFieldValue(1), - std::make_unique(StringFieldValue("bar"))))); + .addUpdate(std::make_unique(std::make_unique(1), + std::make_unique(std::make_unique("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..278e89b15a3 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(std::make_unique()))); EXPECT_FALSE(FeedRejectHelper::mustReject(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 5.0))); EXPECT_FALSE(FeedRejectHelper::mustReject(TensorRemoveUpdate(std::make_unique()))); } TEST(DocumentRejectTest, requireThatAddMapTensorModifyAndTensorAddUpdatesWillBeRejected) { - EXPECT_TRUE(FeedRejectHelper::mustReject(AddValueUpdate(IntFieldValue()))); - EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(IntFieldValue(), std::make_unique()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(AddValueUpdate(std::make_unique()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(MapValueUpdate(std::make_unique(), std::make_unique()))); EXPECT_TRUE(FeedRejectHelper::mustReject(TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE, std::make_unique()))); EXPECT_TRUE(FeedRejectHelper::mustReject(TensorAddUpdate(std::make_unique()))); } TEST(DocumentRejectTest, requireThatAssignUpdatesWillBeRejectedBasedOnTheirContent) { - EXPECT_FALSE(FeedRejectHelper::mustReject(AssignValueUpdate(IntFieldValue()))); - EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(StringFieldValue()))); + EXPECT_FALSE(FeedRejectHelper::mustReject(AssignValueUpdate(std::make_unique()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(std::make_unique()))); } } diff --git a/document/src/tests/testxml.cpp b/document/src/tests/testxml.cpp index 978ab572214..998d41480f3 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(repo, *type, id); up->addUpdate(FieldUpdate(type->getField("intattr")) - .addUpdate(std::make_unique(IntFieldValue(7)))); + .addUpdate(std::make_unique(std::make_unique(7)))); up->addUpdate(FieldUpdate(type->getField("stringattr")) - .addUpdate(std::make_unique(StringFieldValue("New value")))); + .addUpdate(std::make_unique(std::make_unique("New value")))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) - .addUpdate(std::make_unique(IntFieldValue(123))) - .addUpdate(std::make_unique(IntFieldValue(456)))); + .addUpdate(std::make_unique(std::make_unique(123))) + .addUpdate(std::make_unique(std::make_unique(456)))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) - .addUpdate(std::make_unique(IntFieldValue(123))) - .addUpdate(std::make_unique(IntFieldValue(456))) - .addUpdate(std::make_unique(IntFieldValue(789)))); + .addUpdate(std::make_unique(std::make_unique(123))) + .addUpdate(std::make_unique(std::make_unique(456))) + .addUpdate(std::make_unique(std::make_unique(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()); - AddValueUpdate addValueUpdate(IntFieldValue(3)); - RemoveValueUpdate removeValueUpdate(IntFieldValue(3)); - AssignValueUpdate assignValueUpdate(IntFieldValue(3)); + MapValueUpdate mapValueUpdate(std::make_unique(3), std::make_unique()); + AddValueUpdate addValueUpdate(std::make_unique(3)); + RemoveValueUpdate removeValueUpdate(std::make_unique(3)); + AssignValueUpdate assignValueUpdate(std::make_unique(3)); ClearValueUpdate clearValueUpdate; ArithmeticValueUpdate arithmeticValueUpdate(ArithmeticValueUpdate::Add, 3); } diff --git a/document/src/vespa/document/update/addvalueupdate.cpp b/document/src/vespa/document/update/addvalueupdate.cpp index 3b593d3e9b2..102f8ceebbe 100644 --- a/document/src/vespa/document/update/addvalueupdate.cpp +++ b/document/src/vespa/document/update/addvalueupdate.cpp @@ -18,9 +18,9 @@ using namespace vespalib::xml; namespace document { -AddValueUpdate:: AddValueUpdate(const FieldValue& value, int weight) +AddValueUpdate:: AddValueUpdate(std::unique_ptr value, int weight) : ValueUpdate(Add), - _value(value.clone()), + _value(std::move(value)), _weight(weight) {} diff --git a/document/src/vespa/document/update/addvalueupdate.h b/document/src/vespa/document/update/addvalueupdate.h index 2ab7a98e842..f891c368f96 100644 --- a/document/src/vespa/document/update/addvalueupdate.h +++ b/document/src/vespa/document/update/addvalueupdate.h @@ -13,24 +13,24 @@ namespace document { class AddValueUpdate final : public ValueUpdate { - FieldValue::CP _value; // The field value to add by this update. + std::unique_ptr _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(Add), _value(0), _weight(1) {} + AddValueUpdate() : ValueUpdate(Add), _value(), _weight(1) {} ACCEPT_UPDATE_VISITOR; public: - typedef std::unique_ptr 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 value, int weight = 1); + AddValueUpdate(const AddValueUpdate &) = delete; + AddValueUpdate & operator =(const AddValueUpdate &) = delete; ~AddValueUpdate(); bool operator==(const ValueUpdate& other) const override; @@ -41,17 +41,6 @@ public: /** @return The weight to assign to the value of this. */ 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. * diff --git a/document/src/vespa/document/update/arithmeticvalueupdate.h b/document/src/vespa/document/update/arithmeticvalueupdate.h index c82a5b4c338..7651c838458 100644 --- a/document/src/vespa/document/update/arithmeticvalueupdate.h +++ b/document/src/vespa/document/update/arithmeticvalueupdate.h @@ -25,7 +25,7 @@ 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. @@ -50,12 +50,8 @@ public: _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; diff --git a/document/src/vespa/document/update/assignvalueupdate.cpp b/document/src/vespa/document/update/assignvalueupdate.cpp index 363eff92bf0..32bded8cfd7 100644 --- a/document/src/vespa/document/update/assignvalueupdate.cpp +++ b/document/src/vespa/document/update/assignvalueupdate.cpp @@ -21,9 +21,9 @@ AssignValueUpdate::AssignValueUpdate() _value() {} -AssignValueUpdate::AssignValueUpdate(const FieldValue& value) +AssignValueUpdate::AssignValueUpdate(std::unique_ptr value) : ValueUpdate(Assign), - _value(value.clone()) + _value(std::move(value)) { } AssignValueUpdate::~AssignValueUpdate() = default; @@ -36,7 +36,11 @@ AssignValueUpdate::operator==(const ValueUpdate& other) const { if (other.getType() != Assign) return false; const AssignValueUpdate& o(static_cast(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 9df2084ef97..1e006d3baed 100644 --- a/document/src/vespa/document/update/assignvalueupdate.h +++ b/document/src/vespa/document/update/assignvalueupdate.h @@ -17,13 +17,14 @@ namespace document { class AssignValueUpdate final : public ValueUpdate { - FieldValue::CP _value; + std::unique_ptr _value; ACCEPT_UPDATE_VISITOR; public: - typedef std::unique_ptr UP; AssignValueUpdate(); - AssignValueUpdate(const FieldValue& value); + AssignValueUpdate(std::unique_ptr value); + AssignValueUpdate(const AssignValueUpdate& value) = delete; + AssignValueUpdate & operator=(const AssignValueUpdate& value) = delete; ~AssignValueUpdate() override; bool operator==(const ValueUpdate& other) const override; diff --git a/document/src/vespa/document/update/clearvalueupdate.h b/document/src/vespa/document/update/clearvalueupdate.h index 2a68814f8b0..770f0a91d07 100644 --- a/document/src/vespa/document/update/clearvalueupdate.h +++ b/document/src/vespa/document/update/clearvalueupdate.h @@ -14,10 +14,9 @@ namespace document { class ClearValueUpdate final : public ValueUpdate { ACCEPT_UPDATE_VISITOR; public: - typedef std::unique_ptr UP; - ClearValueUpdate(const ClearValueUpdate& update) = default; + ClearValueUpdate(const ClearValueUpdate& update) = delete; + ClearValueUpdate &operator=(const ClearValueUpdate &rhs) = delete; ClearValueUpdate() : ValueUpdate(Clear) {} - ClearValueUpdate &operator=(const ClearValueUpdate &rhs) = default; bool operator==(const ValueUpdate& other) const override; void checkCompatibility(const Field& field) const override; diff --git a/document/src/vespa/document/update/mapvalueupdate.cpp b/document/src/vespa/document/update/mapvalueupdate.cpp index 93f2029b92f..47346133e17 100644 --- a/document/src/vespa/document/update/mapvalueupdate.cpp +++ b/document/src/vespa/document/update/mapvalueupdate.cpp @@ -16,24 +16,12 @@ using namespace vespalib::xml; namespace document { -MapValueUpdate::MapValueUpdate(const FieldValue& key, std::unique_ptr update) +MapValueUpdate::MapValueUpdate(std::unique_ptr key, std::unique_ptr update) : ValueUpdate(Map), - _key(key.clone()), + _key(std::move(key)), _update(std::move(update)) {} -MapValueUpdate::MapValueUpdate(const MapValueUpdate &) - : ValueUpdate(Map), - _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 diff --git a/document/src/vespa/document/update/mapvalueupdate.h b/document/src/vespa/document/update/mapvalueupdate.h index 93d6c844899..dfbb880f422 100644 --- a/document/src/vespa/document/update/mapvalueupdate.h +++ b/document/src/vespa/document/update/mapvalueupdate.h @@ -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 update); - MapValueUpdate(const MapValueUpdate &); - MapValueUpdate & operator = (const MapValueUpdate &); + MapValueUpdate(std::unique_ptr key, std::unique_ptr update); + MapValueUpdate(const MapValueUpdate &) = delete; + MapValueUpdate & operator = (const MapValueUpdate &) = delete; MapValueUpdate(MapValueUpdate &&) = default; MapValueUpdate & operator = (MapValueUpdate &&) = default; diff --git a/document/src/vespa/document/update/removevalueupdate.cpp b/document/src/vespa/document/update/removevalueupdate.cpp index 4789df8d7ab..3c381148e94 100644 --- a/document/src/vespa/document/update/removevalueupdate.cpp +++ b/document/src/vespa/document/update/removevalueupdate.cpp @@ -16,9 +16,9 @@ using namespace vespalib::xml; namespace document { -RemoveValueUpdate::RemoveValueUpdate(const FieldValue& key) +RemoveValueUpdate::RemoveValueUpdate(std::unique_ptr key) : ValueUpdate(Remove), - _key(key.clone()) + _key(std::move(key)) {} RemoveValueUpdate::~RemoveValueUpdate() = default; diff --git a/document/src/vespa/document/update/removevalueupdate.h b/document/src/vespa/document/update/removevalueupdate.h index 0c57817ce33..cfce9b5234a 100644 --- a/document/src/vespa/document/update/removevalueupdate.h +++ b/document/src/vespa/document/update/removevalueupdate.h @@ -11,21 +11,21 @@ namespace document { class RemoveValueUpdate final : public ValueUpdate { - FieldValue::CP _key; // The field value to remove by this update. + std::unique_ptr _key; // The field value to remove by this update. ACCEPT_UPDATE_VISITOR; friend ValueUpdate; RemoveValueUpdate() : ValueUpdate(Remove), _key() {} public: - typedef std::unique_ptr 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(); + RemoveValueUpdate(std::unique_ptr key); + RemoveValueUpdate(const RemoveValueUpdate &) = delete; + RemoveValueUpdate & operator=(const RemoveValueUpdate &) = delete; + ~RemoveValueUpdate() override; bool operator==(const ValueUpdate& other) const override; diff --git a/document/src/vespa/document/update/tensor_add_update.cpp b/document/src/vespa/document/update/tensor_add_update.cpp index 49c73722147..b3bb4d7623d 100644 --- a/document/src/vespa/document/update/tensor_add_update.cpp +++ b/document/src/vespa/document/update/tensor_add_update.cpp @@ -29,13 +29,6 @@ TensorAddUpdate::TensorAddUpdate() { } -TensorAddUpdate::TensorAddUpdate(const TensorAddUpdate &rhs) - : ValueUpdate(rhs), - TensorUpdate(rhs), - _tensor(rhs._tensor->clone()) -{ -} - TensorAddUpdate::TensorAddUpdate(std::unique_ptr &&tensor) : ValueUpdate(TensorAdd), TensorUpdate(), @@ -45,20 +38,6 @@ TensorAddUpdate::TensorAddUpdate(std::unique_ptr &&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 { diff --git a/document/src/vespa/document/update/tensor_add_update.h b/document/src/vespa/document/update/tensor_add_update.h index 7e62f2db71a..00346b4e723 100644 --- a/document/src/vespa/document/update/tensor_add_update.h +++ b/document/src/vespa/document/update/tensor_add_update.h @@ -19,13 +19,12 @@ class TensorAddUpdate final : public ValueUpdate, public TensorUpdate { friend ValueUpdate; TensorAddUpdate(); - TensorAddUpdate(const TensorAddUpdate &rhs); ACCEPT_UPDATE_VISITOR; public: TensorAddUpdate(std::unique_ptr &&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; diff --git a/document/src/vespa/document/update/tensor_modify_update.cpp b/document/src/vespa/document/update/tensor_modify_update.cpp index 6abed60af17..92b2a8672c3 100644 --- a/document/src/vespa/document/update/tensor_modify_update.cpp +++ b/document/src/vespa/document/update/tensor_modify_update.cpp @@ -91,16 +91,6 @@ TensorModifyUpdate::TensorModifyUpdate() { } -TensorModifyUpdate::TensorModifyUpdate(const TensorModifyUpdate &rhs) - : ValueUpdate(rhs), - TensorUpdate(), - _operation(rhs._operation), - _tensorType(std::make_unique(*rhs._tensorType)), - _tensor(static_cast(_tensorType->createFieldValue().release())) -{ - *_tensor = *rhs._tensor; -} - TensorModifyUpdate::TensorModifyUpdate(Operation operation, std::unique_ptr tensor) : ValueUpdate(TensorModify), TensorUpdate(), @@ -113,28 +103,6 @@ TensorModifyUpdate::TensorModifyUpdate(Operation operation, std::unique_ptr(*rhs._tensorType); - _tensor.reset(dynamic_cast(_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 { diff --git a/document/src/vespa/document/update/tensor_modify_update.h b/document/src/vespa/document/update/tensor_modify_update.h index 9fb69ec5a13..9386b4f8a1b 100644 --- a/document/src/vespa/document/update/tensor_modify_update.h +++ b/document/src/vespa/document/update/tensor_modify_update.h @@ -32,13 +32,13 @@ private: friend ValueUpdate; TensorModifyUpdate(); - TensorModifyUpdate(const TensorModifyUpdate &rhs); ACCEPT_UPDATE_VISITOR; public: TensorModifyUpdate(Operation operation, std::unique_ptr 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; } diff --git a/document/src/vespa/document/update/tensor_remove_update.cpp b/document/src/vespa/document/update/tensor_remove_update.cpp index 4bfb6bc6b18..74c0cbe6c63 100644 --- a/document/src/vespa/document/update/tensor_remove_update.cpp +++ b/document/src/vespa/document/update/tensor_remove_update.cpp @@ -46,14 +46,6 @@ TensorRemoveUpdate::TensorRemoveUpdate() { } -TensorRemoveUpdate::TensorRemoveUpdate(const TensorRemoveUpdate &rhs) - : ValueUpdate(rhs), - TensorUpdate(rhs), - _tensorType(std::make_unique(*rhs._tensorType)), - _tensor(rhs._tensor->clone()) -{ -} - TensorRemoveUpdate::TensorRemoveUpdate(std::unique_ptr tensor) : ValueUpdate(TensorRemove), TensorUpdate(), @@ -65,26 +57,6 @@ TensorRemoveUpdate::TensorRemoveUpdate(std::unique_ptr tensor) TensorRemoveUpdate::~TensorRemoveUpdate() = default; -TensorRemoveUpdate & -TensorRemoveUpdate::operator=(const TensorRemoveUpdate &rhs) -{ - if (&rhs != this) { - _tensor.reset(); - _tensorType = std::make_unique(*rhs._tensorType); - _tensor.reset(static_cast(_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 { diff --git a/document/src/vespa/document/update/tensor_remove_update.h b/document/src/vespa/document/update/tensor_remove_update.h index 00c886fa41f..06b0c512b4b 100644 --- a/document/src/vespa/document/update/tensor_remove_update.h +++ b/document/src/vespa/document/update/tensor_remove_update.h @@ -23,14 +23,13 @@ private: friend ValueUpdate; TensorRemoveUpdate(); - TensorRemoveUpdate(const TensorRemoveUpdate &rhs); ACCEPT_UPDATE_VISITOR; public: TensorRemoveUpdate(std::unique_ptr 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 applyTo(const vespalib::eval::Value &tensor) const; std::unique_ptr apply_to(const Value &tensor, diff --git a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp index 2a5b4b48cfa..6f05a1a7b79 100644 --- a/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp +++ b/persistence/src/vespa/persistence/conformancetest/conformancetest.cpp @@ -29,6 +29,9 @@ using document::BucketId; using document::BucketSpace; using document::test::makeBucketSpace; +using document::FieldUpdate; +using document::AssignValueUpdate; +using document::IntFieldValue; using storage::spi::test::makeSpiBucket; using storage::spi::test::cloneDocEntry; @@ -92,11 +95,7 @@ createClusterState(const lib::State& nodeState = lib::State::UP) storage::lib::ClusterState cstate; StorDistributionConfigBuilder dc; - cstate.setNodeState(Node(NodeType::STORAGE, 0), - NodeState(NodeType::STORAGE, - nodeState, - "dummy desc", - 1.0)); + cstate.setNodeState(Node(NodeType::STORAGE, 0), NodeState(NodeType::STORAGE, nodeState, "dummy desc", 1.0)); cstate.setClusterState(State::UP); dc.redundancy = 1; dc.readyCopies = 1; @@ -120,8 +119,7 @@ struct DocAndTimestamp DocAndTimestamp(const Document::SP& docptr, spi::Timestamp ts) : doc(docptr), timestamp(ts) - { - } + { } }; /** @@ -135,9 +133,7 @@ struct Chunk struct DocEntryIndirectTimestampComparator { - bool operator()(const DocEntry::UP& e1, - const DocEntry::UP& e2) const - { + bool operator()(const DocEntry::UP& e1, const DocEntry::UP& e2) const { return e1->getTimestamp() < e2->getTimestamp(); } }; @@ -210,12 +206,7 @@ iterateBucket(PersistenceProvider& spi, Selection sel(docSel); Context context(Priority(0), Trace::TraceLevel(0)); - CreateIteratorResult iter = spi.createIterator( - bucket, - std::make_shared(), - sel, - versions, - context); + CreateIteratorResult iter = spi.createIterator(bucket, std::make_shared(), sel, versions, context); EXPECT_EQ(Result::ErrorType::NONE, iter.getErrorCode()); @@ -340,17 +331,9 @@ TEST_F(ConformanceTest, testBasics) Document::SP doc1 = testDocMan.createRandomDocumentAtLocation(0x01, 1); Document::SP doc2 = testDocMan.createRandomDocumentAtLocation(0x01, 2); spi->createBucket(bucket, context); - EXPECT_EQ( - Result(), - Result(spi->put(bucket, Timestamp(1), doc1, context))); - - EXPECT_EQ( - Result(), - Result(spi->put(bucket, Timestamp(2), doc2, context))); - - EXPECT_EQ( - Result(), - Result(spi->remove(bucket, Timestamp(3), doc1->getId(), context))); + EXPECT_EQ(Result(), Result(spi->put(bucket, Timestamp(1), doc1, context))); + EXPECT_EQ(Result(), Result(spi->put(bucket, Timestamp(2), doc2, context))); + EXPECT_EQ(Result(), Result(spi->remove(bucket, Timestamp(3), doc1->getId(), context))); // Iterate first without removes, then with. for (int iterPass = 0; iterPass < 2; ++iterPass) { @@ -369,9 +352,7 @@ TEST_F(ConformanceTest, testBasics) EXPECT_EQ(Result(), Result(iter)); - IterateResult result = - spi->iterate(iter.getIteratorId(), - std::numeric_limits().max(), context); + IterateResult result = spi->iterate(iter.getIteratorId(), std::numeric_limits().max(), context); EXPECT_EQ(Result(), Result(result)); EXPECT_TRUE(result.isCompleted()); @@ -614,8 +595,7 @@ TEST_F(ConformanceTest, testPutNewDocumentVersion) EXPECT_TRUE(info.getUsedSize() >= info.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(4), gr.getTimestamp()); @@ -660,8 +640,7 @@ TEST_F(ConformanceTest, testPutOlderDocumentVersion) EXPECT_EQ(1, (int)info2.getDocumentCount()); EXPECT_TRUE(info2.getEntryCount() >= info1.getDocumentCount()); EXPECT_EQ(info1.getChecksum(), info2.getChecksum()); - EXPECT_EQ(info1.getDocumentSize(), - info2.getDocumentSize()); + EXPECT_EQ(info1.getDocumentSize(), info2.getDocumentSize()); EXPECT_TRUE(info2.getUsedSize() >= info1.getDocumentSize()); } @@ -907,10 +886,9 @@ TEST_F(ConformanceTest, testUpdate) Bucket bucket(makeSpiBucket(BucketId(8, 0x01))); spi->createBucket(bucket, context); - const document::DocumentType *docType( - testDocMan.getTypeRepo().getDocumentType("testdoctype1")); + const document::DocumentType *docType(testDocMan.getTypeRepo().getDocumentType("testdoctype1")); document::DocumentUpdate::SP update(new DocumentUpdate(testDocMan.getTypeRepo(), *docType, doc1->getId())); - update->addUpdate(document::FieldUpdate(docType->getField("headerval")).addUpdate(std::make_unique(document::IntFieldValue(42)))); + update->addUpdate(FieldUpdate(docType->getField("headerval")).addUpdate(std::make_unique(std::make_unique(42)))); { UpdateResult result = spi->update(bucket, Timestamp(3), update, context); @@ -932,9 +910,7 @@ TEST_F(ConformanceTest, testUpdate) EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(4), result.getTimestamp()); EXPECT_FALSE(result.is_tombstone()); - EXPECT_EQ(document::IntFieldValue(42), - static_cast( - *result.getDocument().getValue("headerval"))); + EXPECT_EQ(IntFieldValue(42), static_cast(*result.getDocument().getValue("headerval"))); } spi->remove(bucket, Timestamp(5), doc1->getId(), context); @@ -977,9 +953,7 @@ TEST_F(ConformanceTest, testUpdate) EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); EXPECT_EQ(Timestamp(7), result.getTimestamp()); EXPECT_FALSE(result.is_tombstone()); - EXPECT_EQ(document::IntFieldValue(42), - reinterpret_cast( - *result.getDocument().getValue("headerval"))); + EXPECT_EQ(IntFieldValue(42), reinterpret_cast(*result.getDocument().getValue("headerval"))); } } @@ -995,8 +969,7 @@ TEST_F(ConformanceTest, testGet) spi->createBucket(bucket, 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(0), result.getTimestamp()); @@ -1006,8 +979,7 @@ TEST_F(ConformanceTest, testGet) spi->put(bucket, Timestamp(3), doc1, context); { - GetResult result = spi->get(bucket, document::AllFields(), - doc1->getId(), context); + GetResult result = spi->get(bucket, document::AllFields(), doc1->getId(), context); EXPECT_EQ(*doc1, result.getDocument()); EXPECT_EQ(Timestamp(3), result.getTimestamp()); EXPECT_FALSE(result.is_tombstone()); @@ -1016,8 +988,7 @@ TEST_F(ConformanceTest, testGet) spi->remove(bucket, Timestamp(4), 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(4), result.getTimestamp()); @@ -1035,8 +1006,7 @@ TEST_F(ConformanceTest, testIterateCreateIterator) Bucket b(makeSpiBucket(BucketId(8, 0x1))); spi->createBucket(b, context); - spi::CreateIteratorResult result( - createIterator(*spi, b, createSelection(""))); + spi::CreateIteratorResult result(createIterator(*spi, b, createSelection(""))); EXPECT_EQ(Result::ErrorType::NONE, result.getErrorCode()); // Iterator ID 0 means invalid iterator, so cannot be returned // from a successful createIterator call. @@ -1075,8 +1045,7 @@ TEST_F(ConformanceTest, testIterateDestroyIterator) } { - Result destroyResult( - spi->destroyIterator(iter.getIteratorId(), context)); + Result destroyResult(spi->destroyIterator(iter.getIteratorId(), context)); EXPECT_TRUE(!destroyResult.hasError()); } // Iteration should now fail @@ -1085,8 +1054,7 @@ TEST_F(ConformanceTest, testIterateDestroyIterator) EXPECT_EQ(Result::ErrorType::PERMANENT_ERROR, result.getErrorCode()); } { - Result destroyResult( - spi->destroyIterator(iter.getIteratorId(), context)); + Result destroyResult(spi->destroyIterator(iter.getIteratorId(), context)); EXPECT_TRUE(!destroyResult.hasError()); } } @@ -1124,7 +1092,7 @@ TEST_F(ConformanceTest, testIterateAllDocsNewestVersionOnly) for (size_t i = 0; i < docs.size(); ++i) { Document::SP newDoc(docs[i].doc->clone()); Timestamp newTimestamp(2000 + i); - newDoc->setValue("headerval", document::IntFieldValue(5678 + i)); + newDoc->setValue("headerval", IntFieldValue(5678 + i)); spi->put(b, newTimestamp, newDoc, context); newDocs.push_back(DocAndTimestamp(newDoc, newTimestamp)); } @@ -1166,8 +1134,7 @@ TEST_F(ConformanceTest, testMaxByteSize) Bucket b(makeSpiBucket(BucketId(8, 0x1))); spi->createBucket(b, context); - std::vector docs( - feedDocs(*spi, testDocMan, b, 100, 4_Ki, 4096)); + std::vector docs(feedDocs(*spi, testDocMan, b, 100, 4_Ki, 4096)); Selection sel(createSelection("")); CreateIteratorResult iter(createIterator(*spi, b, sel)); @@ -1198,14 +1165,11 @@ TEST_F(ConformanceTest, testIterateMatchTimestampRange) for (uint32_t i = 0; i < 99; i++) { Timestamp timestamp(1000 + i); - document::Document::SP doc( - testDocMan.createRandomDocumentAtLocation( - 1, timestamp, 110, 110)); + document::Document::SP doc(testDocMan.createRandomDocumentAtLocation(1, timestamp, 110, 110)); spi->put(b, timestamp, doc, context); if (timestamp >= fromTimestamp && timestamp <= toTimestamp) { - docsToVisit.push_back( - DocAndTimestamp(doc, Timestamp(1000 + i))); + docsToVisit.push_back(DocAndTimestamp(doc, Timestamp(1000 + i))); } } @@ -1236,14 +1200,11 @@ TEST_F(ConformanceTest, testIterateExplicitTimestampSubset) for (uint32_t i = 0; i < 99; i++) { Timestamp timestamp(1000 + i); - document::Document::SP doc( - testDocMan.createRandomDocumentAtLocation( - 1, timestamp, 110, 110)); + document::Document::SP doc(testDocMan.createRandomDocumentAtLocation(1, timestamp, 110, 110)); spi->put(b, timestamp, doc, context); if (timestamp % 3 == 0) { - docsToVisit.push_back( - DocAndTimestamp(doc, Timestamp(1000 + i))); + docsToVisit.push_back(DocAndTimestamp(doc, Timestamp(1000 + i))); timestampsToVisit.push_back(Timestamp(timestamp)); } } @@ -1332,9 +1293,8 @@ TEST_F(ConformanceTest, testIterateMatchSelection) std::vector docsToVisit; for (uint32_t i = 0; i < 99; i++) { - document::Document::SP doc(testDocMan.createRandomDocumentAtLocation( - 1, 1000 + i, 110, 110)); - doc->setValue("headerval", document::IntFieldValue(i)); + document::Document::SP doc(testDocMan.createRandomDocumentAtLocation(1, 1000 + i, 110, 110)); + doc->setValue("headerval", IntFieldValue(i)); spi->put(b, Timestamp(1000 + i), doc, context); if ((i % 3) == 0) { @@ -1503,10 +1463,7 @@ testDeleteBucketPostCondition(const PersistenceProvider &spi, { Context context(Priority(0), Trace::TraceLevel(0)); { - 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(0), result.getTimestamp()); @@ -1633,31 +1590,23 @@ testSplitTargetExistsPostCondition(const PersistenceProvider &spi, const Bucket &bucketC, document::TestDocMan &testDocMan) { - EXPECT_EQ(10, (int)spi.getBucketInfo(bucketA).getBucketInfo(). - getDocumentCount()); - EXPECT_EQ(15, (int)spi.getBucketInfo(bucketB).getBucketInfo(). - getDocumentCount()); + EXPECT_EQ(10, (int)spi.getBucketInfo(bucketA).getBucketInfo().getDocumentCount()); + EXPECT_EQ(15, (int)spi.getBucketInfo(bucketB).getBucketInfo().getDocumentCount()); document::AllFields fs; Context context(Priority(0), Trace::TraceLevel(0)); for (uint32_t i = 0; i < 10; ++i) { Document::UP doc1 = testDocMan.createRandomDocumentAtLocation(0x02, i); - EXPECT_TRUE( - spi.get(bucketA, fs, doc1->getId(), context).hasDocument()); - EXPECT_TRUE( - !spi.get(bucketC, fs, doc1->getId(), context).hasDocument()); - EXPECT_TRUE( - !spi.get(bucketB, fs, doc1->getId(), context).hasDocument()); + EXPECT_TRUE(spi.get(bucketA, fs, doc1->getId(), context).hasDocument()); + EXPECT_FALSE(spi.get(bucketC, fs, doc1->getId(), context).hasDocument()); + EXPECT_FALSE(spi.get(bucketB, fs, doc1->getId(), context).hasDocument()); } for (uint32_t i = 10; i < 25; ++i) { Document::UP doc1 = testDocMan.createRandomDocumentAtLocation(0x06, i); - EXPECT_TRUE( - spi.get(bucketB, fs, doc1->getId(), context).hasDocument()); - EXPECT_TRUE( - !spi.get(bucketA, fs, doc1->getId(), context).hasDocument()); - EXPECT_TRUE( - !spi.get(bucketC, fs, doc1->getId(), context).hasDocument()); + EXPECT_TRUE(spi.get(bucketB, fs, doc1->getId(), context).hasDocument()); + EXPECT_FALSE(spi.get(bucketA, fs, doc1->getId(), context).hasDocument()); + EXPECT_FALSE(spi.get(bucketC, fs, doc1->getId(), context).hasDocument()); } } @@ -1722,16 +1671,12 @@ ConformanceTest::createAndPopulateJoinSourceBuckets( spi.createBucket(source2, context); for (uint32_t i = 0; i < 10; ++i) { - Document::SP doc( - testDocMan.createRandomDocumentAtLocation( - source1.getBucketId().getId(), i)); + Document::SP doc(testDocMan.createRandomDocumentAtLocation(source1.getBucketId().getId(), i)); spi.put(source1, Timestamp(i + 1), doc, context); } for (uint32_t i = 10; i < 20; ++i) { - Document::SP doc( - testDocMan.createRandomDocumentAtLocation( - source2.getBucketId().getId(), i)); + Document::SP doc(testDocMan.createRandomDocumentAtLocation(source2.getBucketId().getId(), i)); spi.put(source2, Timestamp(i + 1), doc, context); } } @@ -1788,9 +1733,7 @@ testJoinNormalCasePostCondition(const PersistenceProvider &spi, document::AllFields fs; Context context(Priority(0), Trace::TraceLevel(0)); for (uint32_t i = 0; i < 10; ++i) { - Document::UP doc( - testDocMan.createRandomDocumentAtLocation( - bucketA.getBucketId().getId(), i)); + Document::UP doc(testDocMan.createRandomDocumentAtLocation(bucketA.getBucketId().getId(), i)); EXPECT_TRUE(spi.get(bucketC, fs, doc->getId(), context).hasDocument()); EXPECT_TRUE(!spi.get(bucketA, fs, doc->getId(), context).hasDocument()); } @@ -2017,8 +1960,7 @@ TEST_F(ConformanceTest, testJoinSameSourceBucketsTargetExists) populateBucket(target, *spi, context, 10, 20, testDocMan); spi->join(source, source, target, context); - testJoinSameSourceBucketsTargetExistsPostCondition( - *spi, source, target, testDocMan); + testJoinSameSourceBucketsTargetExistsPostCondition(*spi, source, target, testDocMan); if (_factory->hasPersistence()) { spi.reset(); document::TestDocMan testDocMan2; @@ -2057,12 +1999,8 @@ TEST_F(ConformanceTest, testBucketActivation) // Add and remove a document, so document goes to zero, to check that // active state isn't cleared then. Document::SP doc1 = testDocMan.createRandomDocumentAtLocation(0x01, 1); - EXPECT_EQ( - Result(), - Result(spi->put(bucket, Timestamp(1), doc1, context))); - EXPECT_EQ( - Result(), - Result(spi->remove(bucket, Timestamp(5), doc1->getId(), context))); + EXPECT_EQ(Result(), Result(spi->put(bucket, Timestamp(1), doc1, context))); + EXPECT_EQ(Result(), Result(spi->remove(bucket, Timestamp(5), doc1->getId(), context))); EXPECT_TRUE(spi->getBucketInfo(bucket).getBucketInfo().isActive()); // Setting node down should clear active flag. diff --git a/searchcore/src/tests/proton/attribute/attribute_test.cpp b/searchcore/src/tests/proton/attribute/attribute_test.cpp index 51bc60d3783..de6b923bdc1 100644 --- a/searchcore/src/tests/proton/attribute/attribute_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_test.cpp @@ -526,9 +526,8 @@ TEST_F(AttributeWriterTest, handles_predicate_update) const document::DocumentType &dt(idb.getDocumentType()); 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(std::make_unique(new_value))); + .addUpdate(std::make_unique(std::make_unique(builder.feature("foo").value("bar").build())))); PredicateIndex &index = static_cast(*a1).getIndex(); EXPECT_EQ(1u, index.getZeroConstraintDocs().size()); @@ -726,10 +725,10 @@ TEST_F(AttributeWriterTest, handles_tensor_assign_update) auto new_tensor = make_tensor(TensorSpec(sparse_tensor) .add({{"x", "8"}, {"y", "9"}}, 11)); TensorDataType xySparseTensorDataType(vespalib::eval::ValueType::from_spec(sparse_tensor)); - TensorFieldValue new_value(xySparseTensorDataType); - new_value = SimpleValue::from_value(*new_tensor); + auto new_value = std::make_unique(xySparseTensorDataType); + *new_value = SimpleValue::from_value(*new_tensor); upd.addUpdate(FieldUpdate(upd.getType().getField("a1")) - .addUpdate(std::make_unique(new_value))); + .addUpdate(std::make_unique(std::move(new_value)))); DummyFieldUpdateCallback onUpdate; update(2, upd, 1, onUpdate); EXPECT_EQ(2u, a1->getNumDocs()); @@ -936,9 +935,9 @@ public: builder.getDocumentType(), DocumentId(doc_id)); 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(std::make_unique(tensor_value))); + auto tensor_value = std::make_unique(tensor_type); + *tensor_value = SimpleValue::from_value(*tensor); + upd->addUpdate(FieldUpdate(upd->getType().getField("a1")).addUpdate(std::make_unique(std::move(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 20584a2a1fb..d2897216ea2 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 @@ -104,21 +104,21 @@ struct Fixture { vec.commit(); } - void applyArrayUpdates(AttributeVector & vec, const FieldValue & assign, - const FieldValue & first, const FieldValue & second) { - applyValueUpdate(vec, 0, std::make_unique(assign)); - applyValueUpdate(vec, 1, std::make_unique(second)); - applyValueUpdate(vec, 2, std::make_unique(first)); + void applyArrayUpdates(AttributeVector & vec, std::unique_ptr assign, + std::unique_ptr first, std::unique_ptr second) { + applyValueUpdate(vec, 0, std::make_unique(std::move(assign))); + applyValueUpdate(vec, 1, std::make_unique(std::move(second))); + applyValueUpdate(vec, 2, std::make_unique(std::move(first))); applyValueUpdate(vec, 3, std::make_unique()); } - void applyWeightedSetUpdates(AttributeVector & vec, const FieldValue & assign, - const FieldValue & first, const FieldValue & second) { - applyValueUpdate(vec, 0, std::make_unique(assign)); - applyValueUpdate(vec, 1, std::make_unique(second, 20)); - applyValueUpdate(vec, 2, std::make_unique(first)); + void applyWeightedSetUpdates(AttributeVector & vec, std::unique_ptr assign, + std::unique_ptr first, std::unique_ptr copyOfFirst, std::unique_ptr second) { + applyValueUpdate(vec, 0, std::make_unique(std::move(assign))); + applyValueUpdate(vec, 1, std::make_unique(std::move(second), 20)); + applyValueUpdate(vec, 2, std::make_unique(std::move(first))); applyValueUpdate(vec, 3, std::make_unique()); - applyValueUpdate(vec, 4, std::make_unique(first, std::make_unique(ArithmeticValueUpdate::Add, 10))); + applyValueUpdate(vec, 4, std::make_unique(std::move(copyOfFirst), std::make_unique(ArithmeticValueUpdate::Add, 10))); } }; @@ -203,10 +203,8 @@ TEST_F("require that single attributes are updated", Fixture) CollectionType ct(CollectionType::SINGLE); { BasicType bt(BasicType::INT32); - AttributePtr vec = create(3, 32, 0, - "in1/int", - Config(bt, ct)); - f.applyValueUpdate(*vec, 0, std::make_unique(IntFieldValue(64))); + AttributePtr vec = create(3, 32, 0, "in1/int", Config(bt, ct)); + f.applyValueUpdate(*vec, 0, std::make_unique(std::make_unique(64))); f.applyValueUpdate(*vec, 1, std::make_unique(ArithmeticValueUpdate::Add, 10)); f.applyValueUpdate(*vec, 2, std::make_unique()); EXPECT_EQUAL(3u, vec->getNumDocs()); @@ -216,11 +214,8 @@ TEST_F("require that single attributes are updated", Fixture) } { BasicType bt(BasicType::FLOAT); - AttributePtr vec = create(3, 55.5f, 0, - "in1/float", - Config(bt, - ct)); - f.applyValueUpdate(*vec, 0, std::make_unique(FloatFieldValue(77.7f))); + AttributePtr vec = create(3, 55.5f, 0, "in1/float",Config(bt, ct)); + f.applyValueUpdate(*vec, 0, std::make_unique(std::make_unique(77.7f))); f.applyValueUpdate(*vec, 1, std::make_unique(ArithmeticValueUpdate::Add, 10)); f.applyValueUpdate(*vec, 2, std::make_unique()); EXPECT_EQUAL(3u, vec->getNumDocs()); @@ -230,11 +225,8 @@ TEST_F("require that single attributes are updated", Fixture) } { BasicType bt(BasicType::STRING); - AttributePtr vec = create(3, "first", 0, - "in1/string", - Config(bt, - ct)); - f.applyValueUpdate(*vec, 0, std::make_unique(StringFieldValue("second"))); + AttributePtr vec = create(3, "first", 0, "in1/string",Config(bt, ct)); + f.applyValueUpdate(*vec, 0, std::make_unique(std::make_unique("second"))); f.applyValueUpdate(*vec, 2, std::make_unique()); EXPECT_EQUAL(3u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedString("second")})); @@ -254,7 +246,7 @@ TEST_F("require that single attributes are updated", Fixture) asReferenceAttribute(*vec).update(docId, toGid(doc1)); } vec->commit(); - f.applyValueUpdate(*vec, 0, std::make_unique(ReferenceFieldValue(dynamic_cast(f.docType->getField("ref").getDataType()), DocumentId(doc2)))); + f.applyValueUpdate(*vec, 0, std::make_unique(std::make_unique(dynamic_cast(f.docType->getField("ref").getDataType()), DocumentId(doc2)))); f.applyValueUpdate(*vec, 2, std::make_unique()); EXPECT_EQUAL(3u, vec->getNumDocs()); TEST_DO(assertRef(*vec, doc2, 0)); @@ -268,14 +260,12 @@ TEST_F("require that array attributes are updated", Fixture) CollectionType ct(CollectionType::ARRAY); { BasicType bt(BasicType::INT32); - AttributePtr vec = create(5, 32, 1, - "in1/aint", - Config(bt, ct)); - IntFieldValue first(32); - IntFieldValue second(64); - ArrayFieldValue assign(f.docType->getField("aint").getDataType()); - assign.add(second); - f.applyArrayUpdates(*vec, assign, first, second); + AttributePtr vec = create(5, 32, 1, "in1/aint", Config(bt, ct)); + auto first = std::make_unique(32); + auto second = std::make_unique(64); + auto assign = std::make_unique(f.docType->getField("aint").getDataType()); + assign->add(*second); + f.applyArrayUpdates(*vec, std::move(assign), std::move(first), std::move(second)); EXPECT_EQUAL(5u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedInt(64)})); @@ -286,15 +276,12 @@ TEST_F("require that array attributes are updated", Fixture) } { BasicType bt(BasicType::FLOAT); - AttributePtr vec = create(5, 55.5f, 1, - "in1/afloat", - Config(bt, - ct)); - FloatFieldValue first(55.5f); - FloatFieldValue second(77.7f); - ArrayFieldValue assign(f.docType->getField("afloat").getDataType()); - assign.add(second); - f.applyArrayUpdates(*vec, assign, first, second); + AttributePtr vec = create(5, 55.5f, 1, "in1/afloat", Config(bt, ct)); + auto first = std::make_unique(55.5f); + auto second = std::make_unique(77.7f); + auto assign = std::make_unique(f.docType->getField("afloat").getDataType()); + assign->add(*second); + f.applyArrayUpdates(*vec, std::move(assign), std::move(first), std::move(second)); EXPECT_EQUAL(5u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedFloat(77.7f)})); @@ -305,14 +292,12 @@ TEST_F("require that array attributes are updated", Fixture) } { BasicType bt(BasicType::STRING); - AttributePtr vec = create(5, "first", 1, - "in1/astring", - Config(bt, ct)); - StringFieldValue first("first"); - StringFieldValue second("second"); - ArrayFieldValue assign(f.docType->getField("astring").getDataType()); - assign.add(second); - f.applyArrayUpdates(*vec, assign, first, second); + AttributePtr vec = create(5, "first", 1, "in1/astring", Config(bt, ct)); + auto first = std::make_unique("first"); + auto second = std::make_unique("second"); + auto assign = std::make_unique(f.docType->getField("astring").getDataType()); + assign->add(*second); + f.applyArrayUpdates(*vec, std::move(assign), std::move(first), std::move(second)); EXPECT_EQUAL(5u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedString("second")})); @@ -328,15 +313,13 @@ TEST_F("require that weighted set attributes are updated", Fixture) CollectionType ct(CollectionType::WSET); { BasicType bt(BasicType::INT32); - AttributePtr vec = create(5, 32, 100, - "in1/wsint", - Config(bt, ct)); - IntFieldValue first(32); - IntFieldValue second(64); - WeightedSetFieldValue - assign(f.docType->getField("wsint").getDataType()); - assign.add(second, 20); - f.applyWeightedSetUpdates(*vec, assign, first, second); + AttributePtr vec = create(5, 32, 100, "in1/wsint", Config(bt, ct)); + auto first = std::make_unique(32); + auto copyOfFirst = std::make_unique(32); + auto second = std::make_unique(64); + auto assign = std::make_unique(f.docType->getField("wsint").getDataType()); + assign->add(*second, 20); + f.applyWeightedSetUpdates(*vec, std::move(assign), std::move(first), std::move(copyOfFirst), std::move(second)); EXPECT_EQUAL(5u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedInt(64, 20)})); @@ -347,16 +330,13 @@ TEST_F("require that weighted set attributes are updated", Fixture) } { BasicType bt(BasicType::FLOAT); - AttributePtr vec = create(5, 55.5f, 100, - "in1/wsfloat", - Config(bt, - ct)); - FloatFieldValue first(55.5f); - FloatFieldValue second(77.7f); - WeightedSetFieldValue - assign(f.docType->getField("wsfloat").getDataType()); - assign.add(second, 20); - f.applyWeightedSetUpdates(*vec, assign, first, second); + AttributePtr vec = create(5, 55.5f, 100, "in1/wsfloat", Config(bt, ct)); + auto first = std::make_unique(55.5f); + auto copyOfFirst = std::make_unique(55.5f); + auto second = std::make_unique(77.7f); + auto assign = std::make_unique(f.docType->getField("wsfloat").getDataType()); + assign->add(*second, 20); + f.applyWeightedSetUpdates(*vec, std::move(assign), std::move(first), std::move(copyOfFirst), std::move(second)); EXPECT_EQUAL(5u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedFloat(77.7f, 20)})); @@ -367,17 +347,13 @@ TEST_F("require that weighted set attributes are updated", Fixture) } { BasicType bt(BasicType::STRING); - AttributePtr vec = create(5, "first", - 100, - "in1/wsstring", - Config(bt, - ct)); - StringFieldValue first("first"); - StringFieldValue second("second"); - WeightedSetFieldValue - assign(f.docType->getField("wsstring").getDataType()); - assign.add(second, 20); - f.applyWeightedSetUpdates(*vec, assign, first, second); + AttributePtr vec = create(5, "first", 100, "in1/wsstring", Config(bt, ct)); + auto first = std::make_unique("first"); + auto copyOfFirst = std::make_unique("first"); + auto second = std::make_unique("second"); + auto assign = std::make_unique(f.docType->getField("wsstring").getDataType()); + assign->add(*second, 20); + f.applyWeightedSetUpdates(*vec, std::move(assign), std::move(first), std::move(copyOfFirst), std::move(second)); EXPECT_EQUAL(5u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedString("second", 20)})); diff --git a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp index 6cd92dea516..94595f0efc7 100644 --- a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp @@ -345,7 +345,7 @@ struct UpdateContext { } else { fieldValue->assign(document::StringFieldValue("new value")); } - update->addUpdate(document::FieldUpdate(field).addUpdate(std::make_unique(*fieldValue))); + update->addUpdate(document::FieldUpdate(field).addUpdate(std::make_unique(std::move(fieldValue)))); } }; @@ -774,7 +774,7 @@ TEST_F("require that all value updates will be inspected before rejected", Schem EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique()))); EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); - docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique(StringFieldValue())))); + docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique(std::make_unique())))); 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 40332157a8d..dea3a3e69a6 100644 --- a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp +++ b/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp @@ -122,7 +122,7 @@ public: auto makeUpdate() { auto upd(std::make_shared(*_repo, _docType, docId)); upd->addUpdate(FieldUpdate(upd->getType().getField("string")). - addUpdate(std::make_unique(StringFieldValue("newval")))); + addUpdate(std::make_unique(std::make_unique("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 7f35e05dfb6..6fb7a35fffe 100644 --- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp +++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp @@ -521,7 +521,7 @@ TEST_F("require that update is rejected if resource limit is reached", SimpleFix document::Field field("string", 1, *document::DataType::STRING); type.addField(field); DocumentUpdate::SP upd = createUpd(type, docId1); - upd->addUpdate(std::move(document::FieldUpdate(field).addUpdate(std::make_unique(document::StringFieldValue("new value"))))); + upd->addUpdate(std::move(document::FieldUpdate(field).addUpdate(std::make_unique(std::make_unique("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 2dcf18f9da6..0856bad0035 100644 --- a/searchcore/src/vespa/searchcore/bmcluster/bm_feed.cpp +++ b/searchcore/src/vespa/searchcore/bmcluster/bm_feed.cpp @@ -71,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(*_repo, *_document_type, id); - document_update->addUpdate(FieldUpdate(_field).addUpdate(std::make_unique(IntFieldValue(15)))); + document_update->addUpdate(FieldUpdate(_field).addUpdate(std::make_unique(std::make_unique(15)))); return document_update; } diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index ea8a1649789..cb51487abdd 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, std::make_unique(ArithVU::Add, 10)))); - EXPECT_TRUE(ptr->apply(1, MapVU(initFieldValue, std::make_unique(ArithVU::Sub, 10)))); - EXPECT_TRUE(ptr->apply(2, MapVU(initFieldValue, std::make_unique(ArithVU::Mul, 10)))); - EXPECT_TRUE(ptr->apply(3, MapVU(initFieldValue, std::make_unique(ArithVU::Div, 10)))); - EXPECT_TRUE(ptr->apply(6, MapVU(initFieldValue, std::make_unique(IntFieldValue(70))))); + EXPECT_TRUE(ptr->apply(0, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(ArithVU::Add, 10)))); + EXPECT_TRUE(ptr->apply(1, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(ArithVU::Sub, 10)))); + EXPECT_TRUE(ptr->apply(2, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(ArithVU::Mul, 10)))); + EXPECT_TRUE(ptr->apply(3, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(ArithVU::Div, 10)))); + EXPECT_TRUE(ptr->apply(6, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(std::make_unique(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, std::make_unique(ArithVU::Sub, 100)))); + EXPECT_TRUE(ptr->apply(4, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(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, std::make_unique(ArithVU::Add, 10)))); + EXPECT_TRUE(ptr->apply(5, MapVU(std::unique_ptr(nonExistant.clone()), std::make_unique(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, std::make_unique(ArithVU::Div, 0)))); + EXPECT_TRUE(ptr->apply(0, MapVU(std::unique_ptr(initFieldValue.clone()), std::make_unique(ArithVU::Div, 0)))); EXPECT_EQUAL(ptr->getStatus().getUpdateCount(), 16u); EXPECT_EQUAL(ptr->getStatus().getNonIdempotentUpdateCount(), 7u); ptr->commit(); @@ -1585,8 +1585,7 @@ AttributeTest::testMapValueUpdate() (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), IntFieldValue(32), false, false); } { // remove if zero - AttributePtr ptr = createAttribute("wsint32", Config(BasicType::INT32, - CollectionType(CollectionType::WSET, true, false))); + AttributePtr ptr = createAttribute("wsint32", Config(BasicType::INT32, CollectionType(CollectionType::WSET, true, false))); testMapValueUpdate (ptr, AttributeVector::WeightedInt(64, 1), IntFieldValue(64), IntFieldValue(32), true, false); } diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp index 0e5372043ab..a6e2b9a2632 100644 --- a/storage/src/tests/distributor/externaloperationhandlertest.cpp +++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp @@ -32,28 +32,22 @@ struct ExternalOperationHandlerTest : Test, DistributorStripeTestUtil { document::TestDocMan _testDocMan; document::BucketId findNonOwnedUserBucketInState(vespalib::stringref state); - document::BucketId findOwned1stNotOwned2ndInStates( - vespalib::stringref state1, - vespalib::stringref state2); + document::BucketId findOwned1stNotOwned2ndInStates(vespalib::stringref state1, vespalib::stringref state2); std::shared_ptr makeGetCommandForUser(uint64_t id) const; std::shared_ptr makeGetCommand(const vespalib::string& id) const; - std::shared_ptr makeUpdateCommand(const vespalib::string& doc_type, - const vespalib::string& id) const; + std::shared_ptr makeUpdateCommand(const vespalib::string& doc_type, const vespalib::string& id) const; std::shared_ptr makeUpdateCommand() const; std::shared_ptr makeUpdateCommandForUser(uint64_t id) const; - std::shared_ptr makePutCommand(const vespalib::string& doc_type, - const vespalib::string& id) const; + std::shared_ptr makePutCommand(const vespalib::string& doc_type, const vespalib::string& id) const; std::shared_ptr makeRemoveCommand(const vespalib::string& id) const; void verify_busy_bounced_due_to_no_active_state(std::shared_ptr cmd); - void start_operation_verify_not_rejected(std::shared_ptr cmd, - Operation::SP& out_generated); + void start_operation_verify_not_rejected(std::shared_ptr cmd, Operation::SP& out_generated); void start_operation_verify_rejected(std::shared_ptr cmd); - int64_t safe_time_not_reached_metric_count( - const PersistenceOperationMetricSet & metrics) const { + int64_t safe_time_not_reached_metric_count(const PersistenceOperationMetricSet & metrics) const { return metrics.failures.safe_time_not_reached.getLongValue("count"); } @@ -61,8 +55,7 @@ struct ExternalOperationHandlerTest : Test, DistributorStripeTestUtil { return metrics.failures.safe_time_not_reached.getLongValue("count"); } - int64_t concurrent_mutatations_metric_count( - const PersistenceOperationMetricSet& metrics) const { + int64_t concurrent_mutatations_metric_count(const PersistenceOperationMetricSet& metrics) const { return metrics.failures.concurrent_mutations.getLongValue("count"); } @@ -597,7 +590,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"); - cmd->getUpdate()->addUpdate(FieldUpdate(doc_type->getField("title")).addUpdate(std::make_unique(StringFieldValue("new value")))); + cmd->getUpdate()->addUpdate(FieldUpdate(doc_type->getField("title")).addUpdate(std::make_unique(std::make_unique("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/persistence/persistencetestutils.cpp b/storage/src/tests/persistence/persistencetestutils.cpp index 3812e7b85c9..b0efed07fab 100644 --- a/storage/src/tests/persistence/persistencetestutils.cpp +++ b/storage/src/tests/persistence/persistencetestutils.cpp @@ -221,20 +221,20 @@ PersistenceTestUtils::doGetOnDisk(const document::BucketId& bucketId, const docu } document::DocumentUpdate::SP -PersistenceTestUtils::createBodyUpdate(const document::DocumentId& docId, const document::FieldValue& updateValue) +PersistenceTestUtils::createBodyUpdate(const document::DocumentId& docId, std::unique_ptr updateValue) { const DocumentType* docType(getTypeRepo()->getDocumentType("testdoctype1")); auto update = std::make_shared(*getTypeRepo(), *docType, docId); - update->addUpdate(document::FieldUpdate(docType->getField("content")).addUpdate(std::make_unique(updateValue))); + update->addUpdate(document::FieldUpdate(docType->getField("content")).addUpdate(std::make_unique(std::move(updateValue)))); return update; } document::DocumentUpdate::SP -PersistenceTestUtils::createHeaderUpdate(const document::DocumentId& docId, const document::FieldValue& updateValue) +PersistenceTestUtils::createHeaderUpdate(const document::DocumentId& docId, std::unique_ptr updateValue) { const DocumentType* docType(getTypeRepo()->getDocumentType("testdoctype1")); auto update = std::make_shared(*getTypeRepo(), *docType, docId); - update->addUpdate(document::FieldUpdate(docType->getField("headerval")).addUpdate(std::make_unique(updateValue))); + update->addUpdate(document::FieldUpdate(docType->getField("headerval")).addUpdate(std::make_unique(std::move(updateValue)))); return update; } diff --git a/storage/src/tests/persistence/persistencetestutils.h b/storage/src/tests/persistence/persistencetestutils.h index 4bbff9bb2ca..94ae7b9fb53 100644 --- a/storage/src/tests/persistence/persistencetestutils.h +++ b/storage/src/tests/persistence/persistencetestutils.h @@ -163,44 +163,23 @@ public: /** Returns the document that was inserted. */ - document::Document::SP doPutOnDisk( - uint32_t location, - spi::Timestamp timestamp, - uint32_t minSize = 0, - uint32_t maxSize = 128); - - document::Document::SP doPut( - uint32_t location, - spi::Timestamp timestamp, - uint32_t minSize = 0, - uint32_t maxSize = 128) - { return doPutOnDisk(location, timestamp, minSize, maxSize); } + document::Document::SP doPutOnDisk(uint32_t location, spi::Timestamp timestamp, uint32_t minSize = 0, uint32_t maxSize = 128); + + document::Document::SP doPut(uint32_t location, spi::Timestamp timestamp, uint32_t minSize = 0, uint32_t maxSize = 128) { + return doPutOnDisk(location, timestamp, minSize, maxSize); + } /** Returns the new doccount if document was removed, or -1 if not found. */ - bool doRemoveOnDisk( - const document::BucketId& bid, - const document::DocumentId& id, - spi::Timestamp timestamp, - bool persistRemove); - - bool doRemove( - const document::BucketId& bid, - const document::DocumentId& id, - spi::Timestamp timestamp, - bool persistRemove) { + bool doRemoveOnDisk(const document::BucketId& bid, const document::DocumentId& id, spi::Timestamp timestamp, bool persistRemove); + bool doRemove(const document::BucketId& bid, const document::DocumentId& id, spi::Timestamp timestamp, bool persistRemove) { return doRemoveOnDisk(bid, id, timestamp, persistRemove); } - bool doUnrevertableRemoveOnDisk(const document::BucketId& bid, - const document::DocumentId& id, - spi::Timestamp timestamp); + bool doUnrevertableRemoveOnDisk(const document::BucketId& bid, const document::DocumentId& id, spi::Timestamp timestamp); - bool doUnrevertableRemove(const document::BucketId& bid, - const document::DocumentId& id, - spi::Timestamp timestamp) - { + bool doUnrevertableRemove(const document::BucketId& bid, const document::DocumentId& id, spi::Timestamp timestamp) { return doUnrevertableRemoveOnDisk(bid, id, timestamp); } @@ -211,25 +190,19 @@ public: * @unrevertableRemove If set, instead of adding put, turn put to remove. * @usedBits Generate bucket to use from docid using this amount of bits. */ - void doRemove(const document::DocumentId& id, spi::Timestamp, - bool unrevertableRemove = false, uint16_t usedBits = 16); + void doRemove(const document::DocumentId& id, spi::Timestamp, bool unrevertableRemove = false, uint16_t usedBits = 16); - spi::GetResult doGetOnDisk( - const document::BucketId& bucketId, - const document::DocumentId& docId); + spi::GetResult doGetOnDisk(const document::BucketId& bucketId, const document::DocumentId& docId); - spi::GetResult doGet( - const document::BucketId& bucketId, - const document::DocumentId& docId) - { return doGetOnDisk(bucketId, docId); } + spi::GetResult doGet(const document::BucketId& bucketId, const document::DocumentId& docId) { + return doGetOnDisk(bucketId, docId); + } - std::shared_ptr createBodyUpdate( - const document::DocumentId& id, - const document::FieldValue& updateValue); + std::shared_ptr + createBodyUpdate(const document::DocumentId& id, std::unique_ptr updateValue); - std::shared_ptr createHeaderUpdate( - const document::DocumentId& id, - const document::FieldValue& updateValue); + std::shared_ptr + createHeaderUpdate(const document::DocumentId& id, std::unique_ptr updateValue); uint16_t getDiskFromBucketDatabaseIfUnset(const document::Bucket &); @@ -241,9 +214,7 @@ public: */ void doPut(const document::Document::SP& doc, spi::Timestamp, uint16_t usedBits = 16); - void doPut(const document::Document::SP& doc, - document::BucketId bid, - spi::Timestamp time); + void doPut(const document::Document::SP& doc, document::BucketId bid, spi::Timestamp time); spi::UpdateResult doUpdate(document::BucketId bid, const std::shared_ptr& update, diff --git a/storage/src/tests/persistence/testandsettest.cpp b/storage/src/tests/persistence/testandsettest.cpp index 6e5fc491941..3245bd78a07 100644 --- a/storage/src/tests/persistence/testandsettest.cpp +++ b/storage/src/tests/persistence/testandsettest.cpp @@ -18,6 +18,7 @@ using std::shared_ptr; using storage::spi::test::makeSpiBucket; using document::test::makeDocumentBucket; +using document::StringFieldValue; using namespace ::testing; namespace storage { @@ -28,10 +29,10 @@ struct TestAndSetTest : PersistenceTestUtils { static constexpr int RANDOM_SEED = 1234; const document::BucketId BUCKET_ID{16, 4}; - const document::StringFieldValue MISMATCHING_HEADER{"Definitely nothing about loud canines"}; - const document::StringFieldValue MATCHING_HEADER{"Some string with woofy dog as a substring"}; - const document::StringFieldValue OLD_CONTENT{"Some old content"}; - const document::StringFieldValue NEW_CONTENT{"Freshly pressed and squeezed content"}; + const StringFieldValue MISMATCHING_HEADER{"Definitely nothing about loud canines"}; + const StringFieldValue MATCHING_HEADER{"Some string with woofy dog as a substring"}; + const StringFieldValue OLD_CONTENT{"Some old content"}; + const StringFieldValue NEW_CONTENT{"Freshly pressed and squeezed content"}; const document::Bucket BUCKET = makeDocumentBucket(BUCKET_ID); unique_ptr persistenceHandler; @@ -159,7 +160,7 @@ std::shared_ptr TestAndSetTest::conditional_update_test(bool createIfMissing, api::Timestamp updateTimestamp) { auto docUpdate = std::make_shared(_env->_testDocMan.getTypeRepo(), testDoc->getType(), testDocId); - docUpdate->addUpdate(document::FieldUpdate(testDoc->getField("content")).addUpdate(std::make_unique(NEW_CONTENT))); + docUpdate->addUpdate(document::FieldUpdate(testDoc->getField("content")).addUpdate(std::make_unique(std::make_unique(NEW_CONTENT)))); docUpdate->setCreateIfNonExistent(createIfMissing); auto updateUp = std::make_unique(BUCKET, docUpdate, updateTimestamp); diff --git a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp index 40969455d68..5e773be42b6 100644 --- a/storageapi/src/tests/mbusprot/storageprotocoltest.cpp +++ b/storageapi/src/tests/mbusprot/storageprotocoltest.cpp @@ -248,9 +248,8 @@ TEST_P(StorageProtocolTest, response_metadata_is_propagated) { } TEST_P(StorageProtocolTest, update) { - auto update = std::make_shared( - _docMan.getTypeRepo(), *_testDoc->getDataType(), _testDoc->getId()); - update->addUpdate(FieldUpdate(_testDoc->getField("headerval")).addUpdate(std::make_unique(IntFieldValue(17)))); + auto update = std::make_shared(_docMan.getTypeRepo(), *_testDoc->getDataType(), _testDoc->getId()); + update->addUpdate(FieldUpdate(_testDoc->getField("headerval")).addUpdate(std::make_unique(std::make_unique(17)))); update->addFieldPathUpdate(FieldPathUpdate::CP(new RemoveFieldPathUpdate("headerval", "testdoctype1.headerval > 0"))); -- cgit v1.2.3 From 11f15ddddb70427d1b571d76ca6ef734a1f15a0f Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 29 Mar 2022 13:51:55 +0000 Subject: Minor simplifications after PR feedback. --- document/src/tests/documentselectparsertest.cpp | 2 +- document/src/tests/documentupdatetestcase.cpp | 34 +++++++++++----------- document/src/tests/feed_reject_helper_test.cpp | 4 +-- document/src/tests/testxml.cpp | 2 +- .../src/vespa/document/update/assignvalueupdate.h | 2 +- .../src/vespa/document/update/removevalueupdate.h | 2 +- .../vespa/document/update/tensor_add_update.cpp | 2 +- .../src/vespa/document/update/tensor_add_update.h | 2 +- .../vespa/document/update/tensor_remove_update.cpp | 3 +- .../vespa/document/update/tensor_remove_update.h | 2 +- document/src/vespa/document/update/valueupdate.cpp | 6 ++-- .../attribute_updater/attribute_updater_test.cpp | 12 ++++---- .../documentdb/feedhandler/feedhandler_test.cpp | 2 +- .../proton/feedoperation/feedoperation_test.cpp | 2 +- .../distributor/externaloperationhandlertest.cpp | 2 +- 15 files changed, 40 insertions(+), 39 deletions(-) (limited to 'document') diff --git a/document/src/tests/documentselectparsertest.cpp b/document/src/tests/documentselectparsertest.cpp index 70ebd9bd7a2..6644fec2da0 100644 --- a/document/src/tests/documentselectparsertest.cpp +++ b/document/src/tests/documentselectparsertest.cpp @@ -141,7 +141,7 @@ DocumentUpdate::SP DocumentSelectParserTest::createUpdate( doc->addUpdate(FieldUpdate(doc->getType().getField("headerval")) .addUpdate(std::make_unique(std::make_unique(hint)))); doc->addUpdate(FieldUpdate(doc->getType().getField("hstringval")) - .addUpdate(std::make_unique(std::make_unique(hstr)))); + .addUpdate(std::make_unique(StringFieldValue::make(hstr)))); return doc; } diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp index 27a9791b95c..40f398ee93e 100644 --- a/document/src/tests/documentupdatetestcase.cpp +++ b/document/src/tests/documentupdatetestcase.cpp @@ -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(std::make_unique("another"))) - .addUpdate(std::make_unique(std::make_unique("tag")))) + .addUpdate(std::make_unique(StringFieldValue::make("another"))) + .addUpdate(std::make_unique(StringFieldValue::make("tag")))) .applyTo(*doc); std::unique_ptr fval2(doc->getAs(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(std::make_unique("THROW MEH!")))) + .addUpdate(std::make_unique(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(std::make_unique("foo"))) - .addUpdate(std::make_unique(std::make_unique("tag")))) + .addUpdate(std::make_unique(StringFieldValue::make("foo"))) + .addUpdate(std::make_unique(StringFieldValue::make("tag")))) .applyTo(*doc); auto fval3(doc->getAs(doc->getField("tags"))); ASSERT_EQ((size_t) 2, fval3->size()); @@ -299,7 +299,7 @@ TEST(DocumentUpdateTest, testUpdateArray) std::unique_ptr createAddUpdate(vespalib::stringref key, int weight) { - auto upd = std::make_unique(std::make_unique(key)); + auto upd = std::make_unique(StringFieldValue::make(key)); upd->setWeight(weight); return upd; } @@ -371,8 +371,8 @@ TEST(DocumentUpdateTest, testUpdateWeightedSet) // Remove weighted field DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field) - .addUpdate(std::make_unique(std::make_unique("foo"))) - .addUpdate(std::make_unique(std::make_unique("too")))) + .addUpdate(std::make_unique(StringFieldValue::make("foo"))) + .addUpdate(std::make_unique(StringFieldValue::make("too")))) .applyTo(*doc); auto fval4(doc->getAs(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(std::make_unique("foo"), + .addUpdate(std::make_unique(StringFieldValue::make("foo"), std::make_unique(ArithmeticValueUpdate::Add, 1)))); } } // anon ns @@ -457,7 +457,7 @@ TEST(DocumentUpdateTest, testIncrementWithZeroResultWeightIsRemoved) { WeightedSetAutoCreateFixture fixture; fixture.update.addUpdate(FieldUpdate(fixture.field) - .addUpdate(std::make_unique(std::make_unique("baz"), + .addUpdate(std::make_unique(StringFieldValue::make("baz"), std::make_unique(ArithmeticValueUpdate::Add, 0)))); fixture.applyUpdateToDocument(); @@ -549,9 +549,9 @@ TEST(DocumentUpdateTest, testGenerateSerializedFile) upd.addUpdate(FieldUpdate(type->getField("intfield")) .addUpdate(std::make_unique(ArithmeticValueUpdate::Add, 3))); upd.addUpdate(FieldUpdate(type->getField("wsfield")) - .addUpdate(std::make_unique(std::make_unique("foo"), + .addUpdate(std::make_unique(StringFieldValue::make("foo"), std::make_unique(ArithmeticValueUpdate::Add, 2))) - .addUpdate(std::make_unique(std::make_unique("foo"), + .addUpdate(std::make_unique(StringFieldValue::make("foo"), std::make_unique(ArithmeticValueUpdate::Mul, 2)))); nbostream buf(serializeHEAD(upd)); writeBufferToFile(buf, "data/serializeupdatecpp.dat"); @@ -732,7 +732,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(std::make_unique(std::make_unique("banana"), + .addUpdate(std::make_unique(StringFieldValue::make("banana"), std::make_unique(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); std::unique_ptr fv1 = @@ -741,7 +741,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field2) - .addUpdate(std::make_unique(std::make_unique("banana"), + .addUpdate(std::make_unique(StringFieldValue::make("banana"), std::make_unique(ArithmeticValueUpdate::Add, 1.0)))) .applyTo(*doc); auto fv2 = doc->getAs(field2); @@ -770,7 +770,7 @@ TEST(DocumentUpdateTest, testMapValueUpdate) DocumentUpdate(docMan.getTypeRepo(), *doc->getDataType(), doc->getId()) .addUpdate(FieldUpdate(field1) - .addUpdate(std::make_unique(std::make_unique("apple"), + .addUpdate(std::make_unique(StringFieldValue::make("apple"), std::make_unique(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(std::make_unique("apple"), + .addUpdate(std::make_unique(StringFieldValue::make("apple"), std::make_unique(ArithmeticValueUpdate::Sub, 1.0)))) .applyTo(*doc); @@ -1282,7 +1282,7 @@ ArrayUpdateFixture::ArrayUpdateFixture() update = std::make_unique(doc_man.getTypeRepo(), *doc->getDataType(), doc->getId()); update->addUpdate(FieldUpdate(array_field) .addUpdate(std::make_unique(std::make_unique(1), - std::make_unique(std::make_unique("bar"))))); + std::make_unique(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 278e89b15a3..fd217601b6c 100644 --- a/document/src/tests/feed_reject_helper_test.cpp +++ b/document/src/tests/feed_reject_helper_test.cpp @@ -53,7 +53,7 @@ TEST(DocumentRejectTest, requireThatFixedSizeFieldValuesAreDetected) { TEST(DocumentRejectTest, requireThatClearRemoveTensorRemoveAndArtithmeticUpdatesIgnoreFeedRejection) { EXPECT_FALSE(FeedRejectHelper::mustReject(ClearValueUpdate())); - EXPECT_FALSE(FeedRejectHelper::mustReject(RemoveValueUpdate(std::make_unique()))); + EXPECT_FALSE(FeedRejectHelper::mustReject(RemoveValueUpdate(StringFieldValue::make()))); EXPECT_FALSE(FeedRejectHelper::mustReject(ArithmeticValueUpdate(ArithmeticValueUpdate::Add, 5.0))); EXPECT_FALSE(FeedRejectHelper::mustReject(TensorRemoveUpdate(std::make_unique()))); } @@ -68,7 +68,7 @@ TEST(DocumentRejectTest, requireThatAddMapTensorModifyAndTensorAddUpdatesWillBeR TEST(DocumentRejectTest, requireThatAssignUpdatesWillBeRejectedBasedOnTheirContent) { EXPECT_FALSE(FeedRejectHelper::mustReject(AssignValueUpdate(std::make_unique()))); - EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(std::make_unique()))); + EXPECT_TRUE(FeedRejectHelper::mustReject(AssignValueUpdate(StringFieldValue::make()))); } } diff --git a/document/src/tests/testxml.cpp b/document/src/tests/testxml.cpp index 998d41480f3..5f194661fe5 100644 --- a/document/src/tests/testxml.cpp +++ b/document/src/tests/testxml.cpp @@ -62,7 +62,7 @@ createTestDocumentUpdate(const DocumentTypeRepo& repo) up->addUpdate(FieldUpdate(type->getField("intattr")) .addUpdate(std::make_unique(std::make_unique(7)))); up->addUpdate(FieldUpdate(type->getField("stringattr")) - .addUpdate(std::make_unique(std::make_unique("New value")))); + .addUpdate(std::make_unique(StringFieldValue::make("New value")))); up->addUpdate(FieldUpdate(type->getField("arrayattr")) .addUpdate(std::make_unique(std::make_unique(123))) .addUpdate(std::make_unique(std::make_unique(456)))); diff --git a/document/src/vespa/document/update/assignvalueupdate.h b/document/src/vespa/document/update/assignvalueupdate.h index 1e006d3baed..071dcc2236d 100644 --- a/document/src/vespa/document/update/assignvalueupdate.h +++ b/document/src/vespa/document/update/assignvalueupdate.h @@ -22,7 +22,7 @@ class AssignValueUpdate final : public ValueUpdate { ACCEPT_UPDATE_VISITOR; public: AssignValueUpdate(); - AssignValueUpdate(std::unique_ptr value); + explicit AssignValueUpdate(std::unique_ptr value); AssignValueUpdate(const AssignValueUpdate& value) = delete; AssignValueUpdate & operator=(const AssignValueUpdate& value) = delete; ~AssignValueUpdate() override; diff --git a/document/src/vespa/document/update/removevalueupdate.h b/document/src/vespa/document/update/removevalueupdate.h index cfce9b5234a..02f8204de23 100644 --- a/document/src/vespa/document/update/removevalueupdate.h +++ b/document/src/vespa/document/update/removevalueupdate.h @@ -22,7 +22,7 @@ public: * * @param value The identifier of the field value to update. */ - RemoveValueUpdate(std::unique_ptr key); + explicit RemoveValueUpdate(std::unique_ptr key); RemoveValueUpdate(const RemoveValueUpdate &) = delete; RemoveValueUpdate & operator=(const RemoveValueUpdate &) = delete; ~RemoveValueUpdate() override; diff --git a/document/src/vespa/document/update/tensor_add_update.cpp b/document/src/vespa/document/update/tensor_add_update.cpp index b3bb4d7623d..4110a94693f 100644 --- a/document/src/vespa/document/update/tensor_add_update.cpp +++ b/document/src/vespa/document/update/tensor_add_update.cpp @@ -29,7 +29,7 @@ TensorAddUpdate::TensorAddUpdate() { } -TensorAddUpdate::TensorAddUpdate(std::unique_ptr &&tensor) +TensorAddUpdate::TensorAddUpdate(std::unique_ptr tensor) : ValueUpdate(TensorAdd), TensorUpdate(), _tensor(std::move(tensor)) diff --git a/document/src/vespa/document/update/tensor_add_update.h b/document/src/vespa/document/update/tensor_add_update.h index 00346b4e723..20e231b4294 100644 --- a/document/src/vespa/document/update/tensor_add_update.h +++ b/document/src/vespa/document/update/tensor_add_update.h @@ -21,7 +21,7 @@ class TensorAddUpdate final : public ValueUpdate, public TensorUpdate { TensorAddUpdate(); ACCEPT_UPDATE_VISITOR; public: - TensorAddUpdate(std::unique_ptr &&tensor); + explicit TensorAddUpdate(std::unique_ptr tensor); TensorAddUpdate(const TensorAddUpdate &rhs) = delete; TensorAddUpdate &operator=(const TensorAddUpdate &rhs) = delete; ~TensorAddUpdate() override; diff --git a/document/src/vespa/document/update/tensor_remove_update.cpp b/document/src/vespa/document/update/tensor_remove_update.cpp index 74c0cbe6c63..25af29ce6b8 100644 --- a/document/src/vespa/document/update/tensor_remove_update.cpp +++ b/document/src/vespa/document/update/tensor_remove_update.cpp @@ -50,9 +50,8 @@ TensorRemoveUpdate::TensorRemoveUpdate(std::unique_ptr tensor) : ValueUpdate(TensorRemove), TensorUpdate(), _tensorType(std::make_unique(dynamic_cast(*tensor->getDataType()))), - _tensor(static_cast(_tensorType->createFieldValue().release())) + _tensor(std::move(tensor)) { - *_tensor = *tensor; } TensorRemoveUpdate::~TensorRemoveUpdate() = default; diff --git a/document/src/vespa/document/update/tensor_remove_update.h b/document/src/vespa/document/update/tensor_remove_update.h index 06b0c512b4b..ca908fc75fc 100644 --- a/document/src/vespa/document/update/tensor_remove_update.h +++ b/document/src/vespa/document/update/tensor_remove_update.h @@ -26,7 +26,7 @@ private: ACCEPT_UPDATE_VISITOR; public: - TensorRemoveUpdate(std::unique_ptr tensor); + explicit TensorRemoveUpdate(std::unique_ptr tensor); TensorRemoveUpdate(const TensorRemoveUpdate &rhs) = delete; TensorRemoveUpdate &operator=(const TensorRemoveUpdate &rhs) = delete; ~TensorRemoveUpdate() override; diff --git a/document/src/vespa/document/update/valueupdate.cpp b/document/src/vespa/document/update/valueupdate.cpp index 4af61178a79..50866311518 100644 --- a/document/src/vespa/document/update/valueupdate.cpp +++ b/document/src/vespa/document/update/valueupdate.cpp @@ -36,8 +36,9 @@ ValueUpdate::className() const noexcept { return "TensorModifyUpdate"; case TensorRemove: return "TensorRemoveUpdate"; + default: + abort(); } - abort(); } std::unique_ptr @@ -61,8 +62,9 @@ ValueUpdate::create(ValueUpdateType type) { return std::unique_ptr( new TensorModifyUpdate()); case TensorRemove: return std::unique_ptr( new TensorRemoveUpdate()); + default: + throw std::runtime_error(vespalib::make_string("Could not find a class for classId %d(%x)", type, type)); } - throw std::runtime_error(vespalib::make_string("Could not find a class for classId %d(%x)", type, type)); } std::unique_ptr 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 d2897216ea2..8fa5c2994b0 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 @@ -226,7 +226,7 @@ TEST_F("require that single attributes are updated", Fixture) { BasicType bt(BasicType::STRING); AttributePtr vec = create(3, "first", 0, "in1/string",Config(bt, ct)); - f.applyValueUpdate(*vec, 0, std::make_unique(std::make_unique("second"))); + f.applyValueUpdate(*vec, 0, std::make_unique(StringFieldValue::make("second"))); f.applyValueUpdate(*vec, 2, std::make_unique()); EXPECT_EQUAL(3u, vec->getNumDocs()); EXPECT_TRUE(check(vec, 0, std::vector{WeightedString("second")})); @@ -293,8 +293,8 @@ TEST_F("require that array attributes are updated", Fixture) { BasicType bt(BasicType::STRING); AttributePtr vec = create(5, "first", 1, "in1/astring", Config(bt, ct)); - auto first = std::make_unique("first"); - auto second = std::make_unique("second"); + auto first = StringFieldValue::make("first"); + auto second = StringFieldValue::make("second"); auto assign = std::make_unique(f.docType->getField("astring").getDataType()); assign->add(*second); f.applyArrayUpdates(*vec, std::move(assign), std::move(first), std::move(second)); @@ -348,9 +348,9 @@ TEST_F("require that weighted set attributes are updated", Fixture) { BasicType bt(BasicType::STRING); AttributePtr vec = create(5, "first", 100, "in1/wsstring", Config(bt, ct)); - auto first = std::make_unique("first"); - auto copyOfFirst = std::make_unique("first"); - auto second = std::make_unique("second"); + auto first = StringFieldValue::make("first"); + auto copyOfFirst = StringFieldValue::make("first"); + auto second = StringFieldValue::make("second"); auto assign = std::make_unique(f.docType->getField("wsstring").getDataType()); assign->add(*second, 20); f.applyWeightedSetUpdates(*vec, std::move(assign), std::move(first), std::move(copyOfFirst), std::move(second)); diff --git a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp index 94595f0efc7..8b20d448a2a 100644 --- a/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedhandler/feedhandler_test.cpp @@ -774,7 +774,7 @@ TEST_F("require that all value updates will be inspected before rejected", Schem EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique()))); EXPECT_FALSE(FeedRejectHelper::mustReject(*docUpdate)); - docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique(std::make_unique())))); + docUpdate->addUpdate(std::move(FieldUpdate(docType->getField("i1")).addUpdate(std::make_unique(StringFieldValue::make())))); 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 dea3a3e69a6..039524a237c 100644 --- a/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp +++ b/searchcore/src/tests/proton/feedoperation/feedoperation_test.cpp @@ -122,7 +122,7 @@ public: auto makeUpdate() { auto upd(std::make_shared(*_repo, _docType, docId)); upd->addUpdate(FieldUpdate(upd->getType().getField("string")). - addUpdate(std::make_unique(std::make_unique("newval")))); + addUpdate(std::make_unique(StringFieldValue::make("newval")))); return upd; } auto makeDoc() { diff --git a/storage/src/tests/distributor/externaloperationhandlertest.cpp b/storage/src/tests/distributor/externaloperationhandlertest.cpp index a6e2b9a2632..fa17d6e1eac 100644 --- a/storage/src/tests/distributor/externaloperationhandlertest.cpp +++ b/storage/src/tests/distributor/externaloperationhandlertest.cpp @@ -590,7 +590,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"); - cmd->getUpdate()->addUpdate(FieldUpdate(doc_type->getField("title")).addUpdate(std::make_unique(std::make_unique("new value")))); + cmd->getUpdate()->addUpdate(FieldUpdate(doc_type->getField("title")).addUpdate(std::make_unique(StringFieldValue::make("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)", -- cgit v1.2.3 From 2d197543db1491993c194c29be88a5ad33c383fb Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Tue, 29 Mar 2022 13:55:28 +0000 Subject: Ensure correct type is used for serialisation. --- .../document/serialization/vespadocumentserializer.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'document') diff --git a/document/src/vespa/document/serialization/vespadocumentserializer.cpp b/document/src/vespa/document/serialization/vespadocumentserializer.cpp index c156594f1f0..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 << ValueUpdate::Remove; + _stream << uint32_t(ValueUpdate::Remove); write(value.getKey()); } @@ -385,7 +385,7 @@ VespaDocumentSerializer::write(const RemoveValueUpdate &value) void VespaDocumentSerializer::write(const AddValueUpdate &value) { - _stream << ValueUpdate::Add; + _stream << uint32_t(ValueUpdate::Add); write(value.getValue()); _stream << static_cast(value.getWeight()); } @@ -393,7 +393,7 @@ VespaDocumentSerializer::write(const AddValueUpdate &value) void VespaDocumentSerializer::write(const ArithmeticValueUpdate &value) { - _stream << ValueUpdate::Arithmetic; + _stream << uint32_t(ValueUpdate::Arithmetic); _stream << static_cast(value.getOperator()); _stream << static_cast(value.getOperand()); } @@ -401,7 +401,7 @@ VespaDocumentSerializer::write(const ArithmeticValueUpdate &value) void VespaDocumentSerializer::write(const AssignValueUpdate &value) { - _stream << ValueUpdate::Assign; + _stream << uint32_t(ValueUpdate::Assign); if (value.hasValue()) { _stream << static_cast(CONTENT_HASVALUE); write(value.getValue()); @@ -414,12 +414,12 @@ void VespaDocumentSerializer::write(const ClearValueUpdate &value) { (void) value; - _stream << ValueUpdate::Clear; + _stream << uint32_t(ValueUpdate::Clear); } void VespaDocumentSerializer::write(const MapValueUpdate &value) { - _stream << ValueUpdate::Map; + _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 << ValueUpdate::TensorModify; + _stream << uint32_t(ValueUpdate::TensorModify); _stream << static_cast(value.getOperation()); write(value.getTensor()); } @@ -493,7 +493,7 @@ VespaDocumentSerializer::visit(const TensorModifyUpdate &value) void VespaDocumentSerializer::write(const TensorAddUpdate &value) { - _stream << ValueUpdate::TensorAdd; + _stream << uint32_t(ValueUpdate::TensorAdd); write(value.getTensor()); } @@ -506,7 +506,7 @@ VespaDocumentSerializer::visit(const TensorAddUpdate &value) void VespaDocumentSerializer::write(const TensorRemoveUpdate &value) { - _stream << ValueUpdate::TensorRemove; + _stream << uint32_t(ValueUpdate::TensorRemove); write(value.getTensor()); } -- cgit v1.2.3