From 7d29e85869f26ba0b4043f160bcfc04d96b95d04 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 23 Jul 2024 18:02:15 +0000 Subject: [PATCH 1/6] fix: construct messages with nested struct --- proto/marshal/rules/message.py | 3 ++- tests/test_marshal_types_struct.py | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/proto/marshal/rules/message.py b/proto/marshal/rules/message.py index c865b99d..c0725b30 100644 --- a/proto/marshal/rules/message.py +++ b/proto/marshal/rules/message.py @@ -34,7 +34,8 @@ def to_proto(self, value): try: # Try the fast path first. return self._descriptor(**value) - except TypeError as ex: + except (TypeError, ValueError) as ex: + # except TypeError as ex: # If we have a type error, # try the slow path in case the error # was an int64/string issue diff --git a/tests/test_marshal_types_struct.py b/tests/test_marshal_types_struct.py index 914730c5..6ae65737 100644 --- a/tests/test_marshal_types_struct.py +++ b/tests/test_marshal_types_struct.py @@ -243,3 +243,26 @@ class Foo(proto.Message): detached["bacon"] = True foo.value = detached assert foo.value == {"foo": "bar", "bacon": True} + + +def test_struct_nested(): + class Foo(proto.Message): + struct_field: struct_pb2.Struct = proto.Field( + proto.MESSAGE, + number=1, + message=struct_pb2.Struct, + ) + + class Bar(proto.Message): + foo_field: Foo = proto.Field( + proto.MESSAGE, + number=1, + message=Foo, + ) + + foo = Foo({"struct_field": {"foo": "bagel"}}) + assert foo.struct_field == {"foo": "bagel"} + + bar = Bar({"foo_field": {"struct_field": {"foo": "cheese"}}}) + assert bar.foo_field == Foo({"struct_field": {"foo": "cheese"}}) + assert bar.foo_field.struct_field == {"foo": "cheese"} From f51724effd0ae59623da8c5b4c51231c6bed0ae3 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 23 Jul 2024 18:06:31 +0000 Subject: [PATCH 2/6] fix lint issues --- tests/test_marshal_types_struct.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_marshal_types_struct.py b/tests/test_marshal_types_struct.py index 6ae65737..8ca2cde8 100644 --- a/tests/test_marshal_types_struct.py +++ b/tests/test_marshal_types_struct.py @@ -248,21 +248,21 @@ class Foo(proto.Message): def test_struct_nested(): class Foo(proto.Message): struct_field: struct_pb2.Struct = proto.Field( - proto.MESSAGE, - number=1, - message=struct_pb2.Struct, - ) + proto.MESSAGE, + number=1, + message=struct_pb2.Struct, + ) class Bar(proto.Message): foo_field: Foo = proto.Field( - proto.MESSAGE, - number=1, - message=Foo, - ) - + proto.MESSAGE, + number=1, + message=Foo, + ) + foo = Foo({"struct_field": {"foo": "bagel"}}) assert foo.struct_field == {"foo": "bagel"} - + bar = Bar({"foo_field": {"struct_field": {"foo": "cheese"}}}) assert bar.foo_field == Foo({"struct_field": {"foo": "cheese"}}) assert bar.foo_field.struct_field == {"foo": "cheese"} From 503d7f0e2415ad1e135e4ac491447b8ebef80c25 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 23 Jul 2024 18:08:51 +0000 Subject: [PATCH 3/6] some cleanup --- proto/marshal/rules/message.py | 1 - 1 file changed, 1 deletion(-) diff --git a/proto/marshal/rules/message.py b/proto/marshal/rules/message.py index c0725b30..ac25b95a 100644 --- a/proto/marshal/rules/message.py +++ b/proto/marshal/rules/message.py @@ -35,7 +35,6 @@ def to_proto(self, value): # Try the fast path first. return self._descriptor(**value) except (TypeError, ValueError) as ex: - # except TypeError as ex: # If we have a type error, # try the slow path in case the error # was an int64/string issue From bdb940bf34f7cda90c1fd367dd5fb277c518e65e Mon Sep 17 00:00:00 2001 From: ohmayr Date: Wed, 24 Jul 2024 18:23:53 +0000 Subject: [PATCH 4/6] remove code for handling underscore keys --- proto/message.py | 31 +------------------------------ 1 file changed, 1 insertion(+), 30 deletions(-) diff --git a/proto/message.py b/proto/message.py index 08d71658..989c1cd3 100644 --- a/proto/message.py +++ b/proto/message.py @@ -725,36 +725,7 @@ def __init__( "Unknown field for {}: {}".format(self.__class__.__name__, key) ) - 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): - if _upb: - # In UPB, pb_type is MessageMeta which doesn't expose attrs like it used to in Python/CPP. - keys_to_update = [ - item - for item in value - if item not in pb_type.DESCRIPTOR.fields_by_name - and f"{item}_" in pb_type.DESCRIPTOR.fields_by_name - ] - else: - keys_to_update = [ - item - for item in value - if not hasattr(pb_type, item) - and hasattr(pb_type, f"{item}_") - ] - for item in keys_to_update: - value[f"{item}_"] = value.pop(item) - - pb_value = marshal.to_proto(pb_type, value) + pb_value = marshal.to_proto(pb_type, value) if pb_value is not None: params[key] = pb_value From 9694d1e3345777846fa1878fe13dcffc53aa66ab Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 30 Jul 2024 16:55:12 +0000 Subject: [PATCH 5/6] update commit message --- proto/marshal/rules/message.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/proto/marshal/rules/message.py b/proto/marshal/rules/message.py index ac25b95a..cdf9c1e7 100644 --- a/proto/marshal/rules/message.py +++ b/proto/marshal/rules/message.py @@ -35,9 +35,13 @@ def to_proto(self, value): # Try the fast path first. return self._descriptor(**value) except (TypeError, ValueError) as ex: - # If we have a type error, + # If we have a TypeError or Valueerror, # try the slow path in case the error - # was an int64/string issue + # was: + # - an int64/string issue. + # - a missing key issue in case a key only exists with a `_` suffix. + # See related issue: https://github.com/googleapis/python-api-core/issues/227. + # - a missing key issue due to nested struct. See: b/321905145. return self._wrapper(value)._pb return value From 7a199ed233cb64bb8a1b632b056f22198b6d9ea7 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Tue, 30 Jul 2024 17:00:05 +0000 Subject: [PATCH 6/6] fix style --- proto/marshal/rules/message.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proto/marshal/rules/message.py b/proto/marshal/rules/message.py index cdf9c1e7..479a2d95 100644 --- a/proto/marshal/rules/message.py +++ b/proto/marshal/rules/message.py @@ -37,7 +37,7 @@ def to_proto(self, value): except (TypeError, ValueError) as ex: # If we have a TypeError or Valueerror, # try the slow path in case the error - # was: + # was: # - an int64/string issue. # - a missing key issue in case a key only exists with a `_` suffix. # See related issue: https://github.com/googleapis/python-api-core/issues/227.