fix: construct messages with nested struct (#479)

* fix: construct messages with nested struct

* fix lint issues

* some cleanup

* remove code for handling underscore keys

* update commit message

* fix style

---------

Co-authored-by: Anthonios Partheniou <[email protected]>

Author: ohmayr

proto/marshal/rules/message.py

@@ -34,10 +34,14 @@ def to_proto(self, value):
             try:
                 # Try the fast path first.
                 return self._descriptor(**value)
-            except TypeError as ex:
-                # If we have a type error,
+            except (TypeError, ValueError) as ex:
+                # 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
 

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

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"}