From 3ea8150e41f5b6652eda8992984f8ae44f064d6d Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Sat, 12 Feb 2022 02:41:11 +0000 Subject: [PATCH 1/4] fix: mitigate collisions in field names --- proto/message.py | 10 ++++++ tests/test_fields_mitigate_collision.py | 41 +++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 tests/test_fields_mitigate_collision.py diff --git a/proto/message.py b/proto/message.py index 5135a542..a348c3d4 100644 --- a/proto/message.py +++ b/proto/message.py @@ -524,6 +524,16 @@ def __init__( # coerced. marshal = self._meta.marshal for key, value in mapping.items(): + # 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 + # natively define the same field with a trailing underscore in protobuf. + # See related issue + # https://github.com/googleapis/python-api-core/issues/227 + if key not in self._meta.fields and f"{key}_" in self._meta.fields: + key = f"{key}_" + try: pb_type = self._meta.fields[key].pb_type except KeyError: diff --git a/tests/test_fields_mitigate_collision.py b/tests/test_fields_mitigate_collision.py new file mode 100644 index 00000000..f9d39af8 --- /dev/null +++ b/tests/test_fields_mitigate_collision.py @@ -0,0 +1,41 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import proto + + +# 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 +# natively define the same field with a trailing underscore in protobuf. +# See related issue +# https://github.com/googleapis/python-api-core/issues/227 +def test_fields_mitigate_collision(): + class TestMessage(proto.Message): + spam_ = proto.Field(proto.STRING, number=1) + eggs = proto.Field(proto.STRING, number=2) + + obj = TestMessage(spam_="has_spam") + obj.eggs = "has_eggs" + assert obj.spam_ == "has_spam" + + modified_obj = TestMessage( + { + "spam": "has_spam", + "eggs": "has_eggs" + } + ) + + assert modified_obj.spam_ == "has_spam" From 2a9f06a31b3b4fbcf7e0687103f28215bcd87a0d Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Sat, 12 Feb 2022 02:48:50 +0000 Subject: [PATCH 2/4] lint --- tests/test_fields_mitigate_collision.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/test_fields_mitigate_collision.py b/tests/test_fields_mitigate_collision.py index f9d39af8..7830fcd9 100644 --- a/tests/test_fields_mitigate_collision.py +++ b/tests/test_fields_mitigate_collision.py @@ -31,11 +31,5 @@ class TestMessage(proto.Message): obj.eggs = "has_eggs" assert obj.spam_ == "has_spam" - modified_obj = TestMessage( - { - "spam": "has_spam", - "eggs": "has_eggs" - } - ) - + modified_obj = TestMessage({"spam": "has_spam", "eggs": "has_eggs"}) assert modified_obj.spam_ == "has_spam" From 4de2462f29edf80fe28a12a1957290d27de7165b Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Sat, 12 Feb 2022 02:51:49 +0000 Subject: [PATCH 3/4] add comment --- tests/test_fields_mitigate_collision.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_fields_mitigate_collision.py b/tests/test_fields_mitigate_collision.py index 7830fcd9..a28d393e 100644 --- a/tests/test_fields_mitigate_collision.py +++ b/tests/test_fields_mitigate_collision.py @@ -31,5 +31,6 @@ class TestMessage(proto.Message): 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" From 485bec26e189f497e2344ed596c1bac01bdf65cc Mon Sep 17 00:00:00 2001 From: Anthonios Partheniou Date: Mon, 14 Feb 2022 19:46:33 +0000 Subject: [PATCH 4/4] address review feedback --- proto/message.py | 33 +++++++++++++------------ tests/test_fields_mitigate_collision.py | 2 +- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/proto/message.py b/proto/message.py index a348c3d4..7293fdb5 100644 --- a/proto/message.py +++ b/proto/message.py @@ -524,25 +524,26 @@ def __init__( # coerced. marshal = self._meta.marshal for key, value in mapping.items(): - # 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 - # natively define the same field with a trailing underscore in protobuf. - # See related issue - # https://github.com/googleapis/python-api-core/issues/227 - if key not in self._meta.fields and f"{key}_" in self._meta.fields: - key = f"{key}_" - try: pb_type = self._meta.fields[key].pb_type except KeyError: - if ignore_unknown_fields: - continue - - raise ValueError( - "Unknown field for {}: {}".format(self.__class__.__name__, key) - ) + # 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 + # 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: + if ignore_unknown_fields: + continue + + raise ValueError( + "Unknown field for {}: {}".format(self.__class__.__name__, key) + ) pb_value = marshal.to_proto(pb_type, value) if pb_value is not None: diff --git a/tests/test_fields_mitigate_collision.py b/tests/test_fields_mitigate_collision.py index a28d393e..0dca71df 100644 --- a/tests/test_fields_mitigate_collision.py +++ b/tests/test_fields_mitigate_collision.py @@ -1,4 +1,4 @@ -# Copyright 2018 Google LLC +# Copyright 2022 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License.