Skip to content

Commit

Permalink
fix: additional logic to mitigate collisions with reserved terms
Browse files Browse the repository at this point in the history
  • Loading branch information
parthea committed Feb 16, 2022
1 parent 71591e1 commit 949e811
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 5 deletions.
62 changes: 58 additions & 4 deletions proto/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ def __init__(
# Underscores may be appended to field names
# that collide with python or proto-plus keywords.
# In case a key only exists with a `_` suffix, coerce the key
# to include the `_` suffix. Is not possible to
# to include the `_` suffix. It's not possible to
# natively define the same field with a trailing underscore in protobuf.
# See related issue
# https://github.com/googleapis/python-api-core/issues/227
Expand All @@ -545,7 +545,27 @@ def __init__(
"Unknown field for {}: {}".format(self.__class__.__name__, key)
)

pb_value = marshal.to_proto(pb_type, value)
try:
pb_value = marshal.to_proto(pb_type, value)
except ValueError:
# Underscores may be appended to field names
# that collide with python or proto-plus keywords.
# In case a key only exists with a `_` suffix, coerce the key
# to include the `_` suffix. It's not possible to
# natively define the same field with a trailing underscore in protobuf.
# See related issue
# https://github.com/googleapis/python-api-core/issues/227
if isinstance(value, dict):
keys_to_update = []
for item in value:
if not hasattr(pb_type, item) and hasattr(pb_type, f"{item}_"):
keys_to_update.append(item)
for item in keys_to_update:
value[f"{item}_"] = value[item]
del value[item]

pb_value = marshal.to_proto(pb_type, value)

if pb_value is not None:
params[key] = pb_value

Expand Down Expand Up @@ -662,7 +682,24 @@ def __getattr__(self, key):
more details.
"""
try:
pb_type = self._meta.fields[key].pb_type
try:
pb_type = self._meta.fields[key].pb_type
except KeyError:
# Underscores may be appended to field names
# that collide with python or proto-plus keywords.
# In case a key only exists with a `_` suffix, coerce the key
# to include the `_` suffix. It's not possible to
# natively define the same field with a trailing underscore in protobuf.
# See related issue
# https://github.com/googleapis/python-api-core/issues/227
if f"{key}_" in self._meta.fields:
key = f"{key}_"
pb_type = self._meta.fields[key].pb_type
else:
raise KeyError(
"Unknown field for {}: {}".format(self.__class__.__name__, key)
)

pb_value = getattr(self._pb, key)
marshal = self._meta.marshal
return marshal.to_python(pb_type, pb_value, absent=key not in self)
Expand All @@ -685,7 +722,24 @@ def __setattr__(self, key, value):
if key[0] == "_":
return super().__setattr__(key, value)
marshal = self._meta.marshal
pb_type = self._meta.fields[key].pb_type
try:
pb_type = self._meta.fields[key].pb_type
except KeyError:
# Underscores may be appended to field names
# that collide with python or proto-plus keywords.
# In case a key only exists with a `_` suffix, coerce the key
# to include the `_` suffix. It's not possible to
# natively define the same field with a trailing underscore in protobuf.
# See related issue
# https://github.com/googleapis/python-api-core/issues/227
if f"{key}_" in self._meta.fields:
key = f"{key}_"
pb_type = self._meta.fields[key].pb_type
else:
raise KeyError(
"Unknown field for {}: {}".format(self.__class__.__name__, key)
)

pb_value = marshal.to_proto(pb_type, value)

# Clear the existing field.
Expand Down
27 changes: 26 additions & 1 deletion tests/test_fields_mitigate_collision.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# Underscores may be appended to field names
# that collide with python or proto-plus keywords.
# In case a key only exists with a `_` suffix, coerce the key
# to include the `_` suffix. Is not possible to
# to include the `_` suffix. It's not possible to
# natively define the same field with a trailing underscore in protobuf.
# See related issue
# https://github.com/googleapis/python-api-core/issues/227
Expand All @@ -27,10 +27,35 @@ class TestMessage(proto.Message):
spam_ = proto.Field(proto.STRING, number=1)
eggs = proto.Field(proto.STRING, number=2)

class TextStream(proto.Message):
text_stream = proto.Field(TestMessage, number=1)

obj = TestMessage(spam_="has_spam")
obj.eggs = "has_eggs"
assert obj.spam_ == "has_spam"

# Test that `spam` is coerced to `spam_`
modified_obj = TestMessage({"spam": "has_spam", "eggs": "has_eggs"})
assert modified_obj.spam_ == "has_spam"

# Test __setattr__ and __getattr___
modified_obj.__setattr__("spam", "no_spam")
modified_obj.__getattr__("spam") == "no_spam"

modified_obj.__setattr__("spam_", "yes_spam")
modified_obj.__getattr__("spam_") == "yes_spam"

# Try nested values
modified_obj = TextStream(text_stream=TestMessage({"spam": "has_spam", "eggs": "has_eggs"}))
assert modified_obj.text_stream.spam_ == "has_spam"

# Test __setattr__ and __getattr___
modified_obj.text_stream.__setattr__("spam", "no_spam")
modified_obj.text_stream.__getattr__("spam") == "no_spam"

modified_obj.text_stream.__setattr__("spam_", "yes_spam")
modified_obj.text_stream.__getattr__("spam_") == "yes_spam"

# Try using dict
modified_obj = TextStream(text_stream={"spam": "has_spam", "eggs": "has_eggs"})
assert modified_obj.text_stream.spam_ == "has_spam"

0 comments on commit 949e811

Please sign in to comment.