Skip to content

Commit

Permalink
Do not reject enum values that are not listed in the proto file
Browse files Browse the repository at this point in the history
According to the language guide [1]:

    During deserialization, unrecognized enum values will be preserved
    in the message, though how this is represented when the message is
    deserialized is language-dependent.

We can store any integer in the underlying field, because in Cython,
enum and int are the same.

Fixes appnexus#149.
  • Loading branch information
thirtythreeforty committed Feb 27, 2020
1 parent 154ccd4 commit 1304330
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
22 changes: 2 additions & 20 deletions pyrobuf/protobuf/templates/proto_pyx.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,7 @@ cdef class {{ message.full_name }}:
{%- endif %}
for val in value:
{%- if field.type == 'enum' and field.enum_def != None %}
{%- for value, enum_field in field.enum_def.fields.items() %}
{%- if loop.index == 1 %}
if val == {{ value }}:
{%- else %}
elif val == {{ value }}:
{%- endif %}
self._{{ field.name }}.append(_{{ enum_field.full_name }})
{% endfor %}
else:
raise ValueError("{} not a valid value for enum {{ field.enum_name }}".format(val))
self._{{ field.name }}.append(val)
{%- elif field.type == 'string' and version_major == 2 %}
if isinstance(val, unicode):
list.append(self._{{ field.name }}, val)
Expand All @@ -234,16 +225,7 @@ cdef class {{ message.full_name }}:
{%- endif %}
{%- else %}
{%- if field.type == 'enum' and field.enum_def != None %}
{%- for value, enum_field in field.enum_def.fields.items() %}
{%- if loop.index == 1 %}
if value == {{ value }}:
{%- else %}
elif value == {{ value }}:
{%- endif %}
self._{{ field.name }} = _{{ enum_field.full_name }}
{% endfor %}
else:
raise ValueError("{} not a valid value for enum {{ field.enum_name }}".format(value))
self._{{ field.name }} = value
{%- elif field.type == 'string' and version_major == 2 %}
if isinstance(value, unicode):
self._{{ field.name }} = value
Expand Down
1 change: 1 addition & 0 deletions tests/proto/test_repeated_enum.proto
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ message TestRepeatedEnum {
TEST_ENUM_FIELD_2 = 2;
}
repeated EnumField list_enum = 1;
optional EnumField single_enum = 2;
}
21 changes: 21 additions & 0 deletions tests/test_repeated_enum.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,33 @@ def test_repeated_enum_serde(self):
message1.list_enum.append(2)
message1.list_enum.append(0)
message1.list_enum.append(1)
# Must be able to handle arbitrary integers in enums:
message1.list_enum.append(42)
# ... but not arbitrary values:
with self.assertRaises(TypeError):
message1.list_enum.append('not an int')

buf = message1.SerializeToString()
message2.ParseFromString(buf)

for i in range(len(message1.list_enum)):
self.assertEqual(message1.list_enum[i], message2.list_enum[i])

def test_single_enum_serde(self):
message1 = TestRepeatedEnum()
message2 = TestRepeatedEnum()

message1.single_enum = 1
# Must be able to handle arbitrary integers in enums:
message1.single_enum = 42
# ... but not arbitrary values:
with self.assertRaises(TypeError):
message1.single_enum = 'not an int'

buf = message1.SerializeToString()
message2.ParseFromString(buf)

self.assertEqual(message1.single_enum, message2.single_enum)

if __name__ == "__main__":
unittest.main()

0 comments on commit 1304330

Please sign in to comment.