Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

THRIFT-5693 - fix bug in serialization of enum default values #2783

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

THRIFT-5693 - fix bug in serialization of enum default values #2783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/cpp/src/thrift/generate/t_py_generator.cc
View file
Open in desktop
Expand Up @@ -2287,7 +2287,7 @@ void t_py_generator::generate_deserialize_field(ostream& out,
out << endl;
} else if (type->is_enum()) {
if (gen_enum_) {
indent(out) << name << " = " << type_name(type) << "(iprot.readI32()).name";
indent(out) << name << " = " << type_name(type) << "(iprot.readI32()).value";
} else {
indent(out) << name << " = iprot.readI32()";
}
Expand Down Expand Up @@ -2477,7 +2477,7 @@ void t_py_generator::generate_serialize_field(ostream& out, t_field* tfield, str
}
} else if (type->is_enum()) {
if (gen_enum_){
out << "writeI32(" << type_name(type) << "[" << name << "].value)";
out << "writeI32(" << type_name(type) << "(" << name << ").value)";
} else {
out << "writeI32(" << name << ")";
}
Expand Down
4 changes: 4 additions & 0 deletions test/ThriftTest.thrift
View file
Open in desktop
Expand Up @@ -420,6 +420,10 @@ struct OptionalSetDefaultTest {
1: optional set<string> with_default = [ "test" ]
}

struct OptionalEnum {
1: optional Numberz e = Numberz.FIVE;
}

struct OptionalBinary {
1: optional set<binary> bin_set = {}
2: optional map<binary,i32> bin_map = {}
Expand Down
11 changes: 11 additions & 0 deletions test/py/SerializationTest.py
View file
Open in desktop
Expand Up @@ -33,6 +33,7 @@
VersioningTestV2,
Xtruct,
Xtruct2,
OptionalEnum,
)

from Recursive.ttypes import RecTree
Expand Down Expand Up @@ -193,6 +194,8 @@ def setUp(self):
]
)

self.optional_enum = OptionalEnum()

def _serialize(self, obj):
trans = TTransport.TMemoryBuffer()
prot = self.protocol_factory.getProtocol(trans)
Expand Down Expand Up @@ -351,6 +354,12 @@ def testRecVector(self):
out_list = self._collapseLinkedList(cur_list)
self.assertEqual(golden_list, out_list)

def testDefaultEnum(self):
"""Ensure default enum values are serializable"""
obj = self._deserialize(OptionalEnum, self._serialize(self.optional_enum))
self.assertEquals(obj, self.optional_enum)


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excessive blank lines. I think the general python style is 2 blank lines between classes/top level stuff and one blank line between functions inside the same class. here you are making it 3 blank lines between classes.


class NormalBinaryTest(AbstractTest):
protocol_factory = TBinaryProtocol.TBinaryProtocolFactory()
Expand Down Expand Up @@ -451,6 +460,8 @@ def _enumerate_enum(enum_class):
self.assertEquals(obj, objcopy)




Comment on lines +463 to +464
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, you are making it 4 blank lines between the class above and suite.

def suite():
suite = unittest.TestSuite()
loader = unittest.TestLoader()
Expand Down
4 changes: 4 additions & 0 deletions test/v0.16/ThriftTest.thrift
View file
Open in desktop
Expand Up @@ -416,3 +416,7 @@ struct OptionalBinary {
1: optional set<binary> bin_set = {}
2: optional map<binary,i32> bin_map = {}
}

struct OptionalEnum {
1: optional Numberz e = Numberz.FIVE;
}