azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Protobuf: fixed a bug in serialization/deserialization of repeated fields (now non-packed fields serialized properly)

Browse Source
This commit is contained in:
dsavvinov
2016-07-21 14:27:12 +03:00
parent 10216da2f9
commit 2520afd3aa
5 changed files with 116 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
proto/compiler/google/src/google/protobuf/compiler/kotlin/protoc
BIN
View File
Binary file not shown.
proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_class_generator.cc
+18 -3
View File
@@ -324,7 +324,7 @@ void ClassGenerator::generateParseMethods(io::Printer *printer) const {
printer->Indent();
// read while we won't exceed expected amount of bytes
printer->Print("while(getSize() < expectedSize) {\n");
printer->Print("while(getSizeNoTag() < expectedSize) {\n");
printer->Indent();
printer->Print("parseFieldFrom(input)\n");
@@ -335,7 +335,7 @@ void ClassGenerator::generateParseMethods(io::Printer *printer) const {
// check if we have read more than expected
vars["dollar"] = "$";
printer->Print(vars,
"if (getSize() > expectedSize) { "
"if (getSizeNoTag() > expectedSize) { "
"throw InvalidProtocolBufferException(\"Error: expected size of message $dollar$expectedSize, but have read at least $dollar${getSize()}\") "
"}\n");
@@ -359,12 +359,27 @@ void ClassGenerator::generateParseMethods(io::Printer *printer) const {
}
void ClassGenerator::generateGetSizeMethod(io::Printer *printer) const {
// getSize(): Int
printer->Print("fun getSize(): Int {\n");
printer->Indent();
printer->Print("var size = 0\n");
for (int i = 0; i < properties.size(); ++i) {
properties[i]->generateSizeEstimationCode(printer, "size");
properties[i]->generateSizeEstimationCode(printer, "size", /* noTag = */ false);
}
printer->Print("return size\n");
printer->Outdent();
printer->Print("}\n");
// getSizeNoTag(): Int
printer->Print("fun getSizeNoTag(): Int {\n");
printer->Indent();
printer->Print("var size = 0\n");
for (int i = 0; i < properties.size(); ++i) {
properties[i]->generateSizeEstimationCode(printer, "size", /* noTag = */ true);
}
printer->Print("return size\n");
proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.cc
+93 -22
View File
@@ -58,12 +58,18 @@ FieldGenerator::FieldGenerator(FieldDescriptor const * descriptor, ClassGenerato
, protoLabel(descriptor->label())
{ }
void FieldGenerator::generateSerializationForRepeated(io::Printer * printer, bool isRead, bool noTag) const {
void FieldGenerator::generateSerializationForPacked(io::Printer *printer, bool isRead, bool noTag) const {
map <string, string> vars;
vars["fieldNumber"] = std::to_string(getFieldNumber());
vars["builderType"] = getUnderlyingTypeGenerator().getFullType();
vars["initValue"] = getUnderlyingTypeGenerator().getInitValue();
vars["fieldName"] = simpleName;
bool isPrimitive = descriptor->type() != FieldDescriptor::TYPE_BYTES &&
descriptor->type() != FieldDescriptor::TYPE_MESSAGE &&
descriptor->type() != FieldDescriptor::TYPE_STRING &&
descriptor->type() != FieldDescriptor::TYPE_ENUM;
if (isRead) {
if (!noTag) {
printer->Print(vars, "val tag = input.readTag($fieldNumber$, WireType.LENGTH_DELIMITED)\n");
@@ -94,15 +100,9 @@ void FieldGenerator::generateSerializationForRepeated(io::Printer * printer, boo
singleFieldGen.simpleName = "tmp";
singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL;
// Note that primitive types are packed by default in proto3, i.e. they are should be written without tag
bool isPrimitive = descriptor->type() != FieldDescriptor::TYPE_BYTES &&
descriptor->type() != FieldDescriptor::TYPE_MESSAGE &&
descriptor->type() != FieldDescriptor::TYPE_STRING &&
descriptor->type() != FieldDescriptor::TYPE_ENUM;
singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ isPrimitive);
singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ true);
singleFieldGen.generateSizeEstimationCode(printer, /* varName = */
"readSize"); // add size of current element to total size
"readSize"); // add size of current element to total size //TODO: think is it's ok that we estimate size with tag here
printer->Print(vars, "$fieldName$.add(tmp)\n");
@@ -137,12 +137,6 @@ void FieldGenerator::generateSerializationForRepeated(io::Printer * printer, boo
singleFieldGen.simpleName = "item";
singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL;
// TODO: maybe refactor this in name_resolving or separate method at least
bool isPrimitive = descriptor->type() != FieldDescriptor::TYPE_BYTES &&
descriptor->type() != FieldDescriptor::TYPE_MESSAGE &&
descriptor->type() != FieldDescriptor::TYPE_STRING &&
descriptor->type() != FieldDescriptor::TYPE_ENUM;
singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ isPrimitive);
printer->Outdent(); // for-loop
@@ -153,6 +147,72 @@ void FieldGenerator::generateSerializationForRepeated(io::Printer * printer, boo
}
}
void FieldGenerator::generateSerializationForRepeated(io::Printer *printer, bool isRead, bool noTag) const {
map <string, string> vars;
vars["fieldNumber"] = std::to_string(getFieldNumber());
vars["builderType"] = getUnderlyingTypeGenerator().getFullType();
vars["initValue"] = getUnderlyingTypeGenerator().getInitValue();
vars["fieldName"] = simpleName;
if (isRead) {
printer->Print("var readSize = 0\n");
/* hack: copy current FieldGenerator and change label to OPTIONAL. Also change name to
name of iterator in for-loop.
This will allow to re-use this function for generating serialization code for elements of array.
More importantly, this will care about nested types too.
Efficiently, it inlines serialization code for all underlying types.
This hack isn't necessary from the architectural point of view and could be safely
removed as soon as target code will support inheritance and interfaces.
(then writing CodedOutputStream.writeMessage will be possible).
*/
FieldGenerator singleFieldGen = getUnderlyingTypeGenerator();
/* Another dirty hack here: create tmp variable of a given type and read it from input stream
then add that tmp var into list.
This is made because simple recursive call will generate code that tries to array[i].mergeFrom().
This is incorrect because array has old size, while 'i' iterates over new size, which can lead
to ArrayOutOfIndex errors.
*/
printer->Print(vars, "var tmp: $builderType$ = $initValue$\n");
singleFieldGen.simpleName = "tmp";
singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL;
singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ true);
singleFieldGen.generateSizeEstimationCode(printer, /* varName = */
"readSize", /* noTag = */ true); // add size of current element to total size. Note that
printer->Print(vars, "$fieldName$.add(tmp)\n");
}
else {
/**
* Protobuf format:
* - Check if size of array is > 0, because empty repeated fields shouldn't appear in message
* - Write tag explicitly
* - Write length as int32 (note that tag shouldn't be added)
* - Write all repeated elements via recursive call (for primitive types without tags)
*/
printer->Print(vars, "if ($fieldName$.size > 0) {\n");
printer->Indent();
// all elements
printer->Print(vars, "for (item in $fieldName$) {\n");
printer->Indent();
// hack: see above
FieldGenerator singleFieldGen = FieldGenerator(descriptor, enclosingClass, nameResolver);
singleFieldGen.simpleName = "item";
singleFieldGen.protoLabel = FieldDescriptor::LABEL_OPTIONAL;
singleFieldGen.generateSerializationCode(printer, isRead, /* noTag = */ true);
printer->Outdent(); // for-loop
printer->Print("}\n");
printer->Outdent(); // if-clause
printer->Print("}\n");
}
}
void FieldGenerator::generateSerializationForEnums(io::Printer * printer, bool isRead, bool noTag) const {
map <string, string> vars;
vars["converter"] = getEnumFromIntConverter();
@@ -205,7 +265,7 @@ void FieldGenerator::generateSerializationForMessages(io::Printer * printer, boo
"$fieldName$.mergeFromWithSize(input, expectedSize)\n");
// check that actual size equal to expected size
printer->Print(vars, "if (expectedSize != $fieldName$.getSize()) { "
printer->Print(vars, "if (expectedSize != $fieldName$.getSizeNoTag()) { "
"throw InvalidProtocolBufferException ("
"\"Expected size $dollar${expectedSize} got $dollar${$fieldName$.getSize()}"
"\") }\n");
@@ -252,7 +312,16 @@ void FieldGenerator::generateSerializationCode(io::Printer *printer, bool isRead
/* Try to generate syntax for serialization of repeated fields.
* Note that it should be first check because of Google's FieldDescriptor structure */
if (getProtoLabel() == FieldDescriptor::LABEL_REPEATED) {
generateSerializationForRepeated(printer, isRead, noTag);
bool isPrimitive = descriptor->type() != FieldDescriptor::TYPE_BYTES &&
descriptor->type() != FieldDescriptor::TYPE_MESSAGE &&
descriptor->type() != FieldDescriptor::TYPE_STRING &&
descriptor->type() != FieldDescriptor::TYPE_ENUM;
if (isPrimitive) {
generateSerializationForPacked(printer, isRead, noTag);
}
else {
generateSerializationForRepeated(printer, isRead, noTag);
}
return;
}
@@ -392,11 +461,13 @@ void FieldGenerator::generateSizeEstimationCode(io::Printer *printer, string var
if (getProtoType() == FieldDescriptor::TYPE_MESSAGE) {
// don't forget about tag and length annotation
printer->Print(vars, "$varName$ += $fieldName$.getSize()"
" + "
"WireFormat.getTagSize($fieldNumber$, WireType.LENGTH_DELIMITED)"
" + "
"WireFormat.getVarint32Size($fieldName$.getSize())\n"
);
"\n");
if (!noTag) {
printer->Print(vars, "$varName$ += "
"WireFormat.getTagSize($fieldNumber$, WireType.LENGTH_DELIMITED)"
" + "
"WireFormat.getVarint32Size($fieldName$.getSize())\n");
}
return;
}
proto/compiler/google/src/google/protobuf/compiler/kotlin/src/kotlin_field_generator.h
+1
View File
@@ -27,6 +27,7 @@ private:
void generateSetter(io::Printer * printer) const;
void generateRepeatedMethods(io::Printer * printer, bool isBuilder) const;
void generateSerializationForPacked (io::Printer * printer, bool isRead, bool noTag) const;
void generateSerializationForRepeated (io::Printer * printer, bool isRead, bool noTag) const;
void generateSerializationForEnums (io::Printer * printer, bool isRead, bool noTag) const;
void generateSerializationForMessages (io::Printer * printer, bool isRead, bool noTag) const;
proto/compiler/src/WireFormat.kt
+4
View File
@@ -104,11 +104,15 @@ object WireFormat {
}
fun getStringSize(fieldNumber: Int, value: String): Int {
if (value.length == 0)
return 0
val encodedStringSize = value.toByteArray(Charsets.UTF_8).size //TODO: not sure if it's the best way to do it
return encodedStringSize + getTagSize(fieldNumber, WireType.LENGTH_DELIMITED) + getVarint32Size(encodedStringSize)
}
fun getBytesSize(fieldNumber: Int, value: ByteArray): Int {
if (value.size == 0)
return 0
return value.size + getTagSize(fieldNumber, WireType.LENGTH_DELIMITED) + getVarint32Size(value.size)
}
}