diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2017-03-28 17:16:54 +0200 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2017-03-28 17:16:54 +0200 |
commit | c4abdb06a60852ac00c5920eff34aeefb3fc00e3 (patch) | |
tree | d354b356360bad13013b78d648dde152bea9abd2 | |
parent | daa0a43e5e8629b63be12b5205411d926d1bab06 (diff) |
Make struct serialization/deserialization symmetric even when empty struct.
5 files changed, 66 insertions, 47 deletions
diff --git a/document/src/tests/structfieldvaluetest.cpp b/document/src/tests/structfieldvaluetest.cpp index c1bf53d5322..b1c68662553 100644 --- a/document/src/tests/structfieldvaluetest.cpp +++ b/document/src/tests/structfieldvaluetest.cpp @@ -1,6 +1,5 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#include <vespa/fastos/fastos.h> #include <vespa/document/fieldvalue/fieldvalues.h> #include <vespa/document/repo/configbuilder.h> #include <vespa/document/serialization/vespadocumentdeserializer.h> @@ -16,13 +15,18 @@ using document::config_builder::Map; namespace document { struct StructFieldValueTest : public CppUnit::TestFixture { + DocumentTypeRepo doc_repo; + StructFieldValueTest(); void setUp() {} void tearDown() {} void testStruct(); + void testEmptyStruct(); CPPUNIT_TEST_SUITE(StructFieldValueTest); CPPUNIT_TEST(testStruct); + CPPUNIT_TEST(testEmptyStruct); + CPPUNIT_TEST_SUITE_END(); }; @@ -38,18 +42,44 @@ void deserialize(const ByteBuffer &buffer, T &value, const FixedTypeRepo &repo) VespaDocumentDeserializer deserializer(repo, stream, version); deserializer.read(value); } -} // namespace -void StructFieldValueTest::testStruct() -{ +config_builder::DocumenttypesConfigBuilderHelper +createBuilder() { config_builder::DocumenttypesConfigBuilderHelper builder; builder.document(42, "test", Struct("test.header") - .addField("int", DataType::T_INT) - .addField("long", DataType::T_LONG) - .addField("content", DataType::T_STRING), + .addField("int", DataType::T_INT) + .addField("long", DataType::T_LONG) + .addField("content", DataType::T_STRING), Struct("test.body")); - DocumentTypeRepo doc_repo(builder.config()); + return builder; +} + +} // namespace + +StructFieldValueTest::StructFieldValueTest() + : doc_repo(createBuilder().config()) +{} + +void StructFieldValueTest::testEmptyStruct() +{ + FixedTypeRepo repo(doc_repo, *doc_repo.getDocumentType(42)); + const DataType &type = *repo.getDataType("test.header"); + StructFieldValue value(type); + + // Serialize & equality + std::unique_ptr<ByteBuffer> buffer(value.serialize()); + buffer->flip(); + + CPPUNIT_ASSERT_EQUAL(buffer->getLength(), buffer->getLimit()); + StructFieldValue value2(type); + + deserialize(*buffer, value2, repo); + CPPUNIT_ASSERT(value == value2); +} + +void StructFieldValueTest::testStruct() +{ const DocumentType *doc_type = doc_repo.getDocumentType(42); CPPUNIT_ASSERT(doc_type); FixedTypeRepo repo(doc_repo, *doc_type); diff --git a/document/src/vespa/document/fieldvalue/structfieldvalue.h b/document/src/vespa/document/fieldvalue/structfieldvalue.h index bc5f1087f5a..4ddd93469a5 100644 --- a/document/src/vespa/document/fieldvalue/structfieldvalue.h +++ b/document/src/vespa/document/fieldvalue/structfieldvalue.h @@ -1,18 +1,18 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + /** * \class document::StructFieldValue * \ingroup fieldvalue * * \brief Stores a set of predefined field <-> fieldvalue mappings. */ -#pragma once - // Not strictly needed to include exceptions.h, but to avoid clients needing - // to to catch FieldNotFoundException -#include <vespa/document/datatype/structdatatype.h> -#include <vespa/document/fieldvalue/structuredfieldvalue.h> +#include "structuredfieldvalue.h" +#include "serializablearray.h" #include <vespa/document/util/compressionconfig.h> -#include <vespa/document/fieldvalue/serializablearray.h> +#include <vespa/document/datatype/structdatatype.h> #include <vector> namespace document { diff --git a/document/src/vespa/document/serialization/vespadocumentdeserializer.h b/document/src/vespa/document/serialization/vespadocumentdeserializer.h index b4725327ec4..3cc0092802b 100644 --- a/document/src/vespa/document/serialization/vespadocumentdeserializer.h +++ b/document/src/vespa/document/serialization/vespadocumentdeserializer.h @@ -2,9 +2,9 @@ #pragma once -#include <memory> #include <vespa/document/fieldvalue/fieldvaluevisitor.h> #include <vespa/document/repo/fixedtyperepo.h> +#include <memory> namespace vespalib { class nbostream; } diff --git a/document/src/vespa/document/serialization/vespadocumentserializer.cpp b/document/src/vespa/document/serialization/vespadocumentserializer.cpp index d4d3856eb7a..2fcec61fe60 100644 --- a/document/src/vespa/document/serialization/vespadocumentserializer.cpp +++ b/document/src/vespa/document/serialization/vespadocumentserializer.cpp @@ -25,8 +25,7 @@ #include <vespa/vespalib/data/slime/binary_format.h> #include <vespa/vespalib/objects/nbostream.h> #include <vespa/eval/tensor/serialization/typed_binary_format.h> -#include <utility> -#include <vector> + using std::make_pair; using std::pair; @@ -259,21 +258,25 @@ bool bigEnough(size_t size, const CompressionConfig &config) return (size >= config.minSize); } -void compressStream(const CompressionConfig &config, nbostream &stream, +vespalib::ConstBufferRef +compressStream(const CompressionConfig &config, nbostream &stream, vespalib::DataBuffer & compressed_data) { + vespalib::ConstBufferRef buf(stream.c_str(), stream.size()); if (config.useCompression() && bigEnough(stream.size(), config)) { CompressionConfig::Type compressedType = compress(config, vespalib::ConstBufferRef(stream.c_str(), stream.size()), compressed_data, false); if (compressedType != config.type || ! compressionSufficient(config, stream.size(), compressed_data.getDataLen())) { compressed_data.clear(); + } else { + buf = vespalib::ConstBufferRef(compressed_data.getData(), compressed_data.getDataLen()); } } + return buf; } -void putFieldInfo(nbostream &output, - const vector<pair<uint32_t, uint32_t> > &field_info) { +void putFieldInfo(nbostream &output, const vector<pair<uint32_t, uint32_t> > &field_info) { putInt1_4Bytes(output, field_info.size()); for (size_t i = 0; i < field_info.size(); ++i) { putInt1_4Bytes(output, field_info[i].first); @@ -345,29 +348,23 @@ void VespaDocumentSerializer::write(const StructFieldValue &value, nbostream value_stream; vector<pair<uint32_t, uint32_t> > field_info; serializeFields(value, value_stream, field_info, fieldSet); - if (field_info.empty()) { - return; - } const CompressionConfig &comp_config = value.getCompressionConfig(); vespalib::DataBuffer compressed_data; - compressStream(comp_config, value_stream, compressed_data); - - if (compressed_data.getDataLen() == 0) { - uint8_t comp_type = comp_config.type == CompressionConfig::NONE ? - CompressionConfig::NONE : - CompressionConfig::UNCOMPRESSABLE; - _stream << static_cast<uint32_t>(value_stream.size()); - _stream << comp_type; - putFieldInfo(_stream, field_info); - _stream.write(value_stream.c_str(), value_stream.size()); - } else { - _stream << static_cast<uint32_t>(compressed_data.getDataLen()); - _stream << static_cast<uint8_t>(comp_config.type); + vespalib::ConstBufferRef toSerialize = compressStream(comp_config, value_stream, compressed_data); + + uint8_t comp_type = (compressed_data.getDataLen() == 0) + ? (comp_config.type == CompressionConfig::NONE + ? CompressionConfig::NONE + : CompressionConfig::UNCOMPRESSABLE) + : comp_config.type; + _stream << static_cast<uint32_t>(toSerialize.size()); + _stream << comp_type; + if (compressed_data.getDataLen() != 0) { putInt2_4_8Bytes(_stream, value_stream.size()); - putFieldInfo(_stream, field_info); - _stream.write(compressed_data.getData(), compressed_data.getDataLen()); } + putFieldInfo(_stream, field_info); + _stream.write(toSerialize.c_str(), toSerialize.size()); } void VespaDocumentSerializer::write(const WeightedSetFieldValue &value) { diff --git a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_0.cpp b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_0.cpp index 334816fe93c..f4f6616cd66 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_0.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/protocolserialization5_0.cpp @@ -4,17 +4,9 @@ #include "serializationhelper.h" #include "storagecommand.h" #include "storagereply.h" -#include "storageprotocol.h" -#include <vespa/messagebus/blob.h> -#include <vespa/messagebus/blobref.h> -#include <vespa/storageapi/messageapi/storagemessage.h> -#include <vespa/storageapi/message/bucket.h> #include <vespa/storageapi/message/bucketsplitting.h> -#include <vespa/storageapi/message/persistence.h> #include <vespa/storageapi/message/multioperation.h> -#include <vespa/vespalib/util/growablebytebuffer.h> -#include <vespa/document/select/orderingspecification.h> -#include <vespa/storageapi/messageapi/returncode.h> + namespace storage { namespace mbusprot { |