fix: resolve unit test failure caused by differences in protobuf runt…

…imes (#1749)

Co-authored-by: Victor Chudnovsky <vchudnov@google.com>

gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2

@@ -1014,6 +1014,58 @@ def test_{{ method.name|snake_case }}_rest(request_type):
     {% if not field.oneof or field.proto3_optional %}
     {# ignore oneof fields that might conflict with sample_request #}
     request_init["{{ field.name }}"] = {{ field.merged_mock_value(method.http_options[0].sample_request(method).get(field.name)) }}
+    # The version of a generated dependency at test runtime may differ from the version used during generation.
+    # Delete any fields which are not present in the current runtime dependency
+    # See https://github.com/googleapis/gapic-generator-python/issues/1748
+
+    # Determine if the message type is proto-plus or protobuf
+    is_message_proto_plus_type = not hasattr({{ method.input.ident }}.meta.fields["{{ field.name }}"].message, "DESCRIPTOR")
+
+    if is_message_proto_plus_type:
+        message_fields = {{ method.input.ident }}.meta.fields["{{ field.name }}"].message.meta.fields
+    else:
+        message_fields = {{ method.input.ident }}.meta.fields["{{ field.name }}"].message.DESCRIPTOR.fields
+
+    subfields_not_in_runtime = []
+
+    # Get all subfields for the message
+    runtime_nested_fields = [
+        (field.name, subfield.name)
+        for field in message_fields
+        if hasattr(field, "message_type") and field.message_type
+        for subfield in field.message_type.fields
+    ]
+
+    # For each item in the sample request, create a list of sub fields which are not present at runtime
+    for field, value in request_init["{{ field.name }}"].items():
+        result = None
+        is_repeated = False
+        # For repeated fields
+        if isinstance(value, list) and len(value):
+            is_repeated = True
+            result = value[0]
+        # For fields where the type is another message
+        if isinstance(value, dict):
+            result = value
+
+        if result and hasattr(result, "keys"):
+            for subfield in result.keys():
+                if (field, subfield) not in runtime_nested_fields:
+                    subfields_not_in_runtime.append(
+                        {"field": field, "subfield": subfield, "is_repeated": is_repeated}
+                    )
+
+    # Remove fields from the sample request which are not present in the runtime version of the dependency
+    for subfield_to_delete in subfields_not_in_runtime:
+        field = subfield_to_delete.get("field")
+        field_repeated = subfield_to_delete.get("is_repeated")
+        subfield = subfield_to_delete.get("subfield")
+        if subfield:
+            if field_repeated:
+                for i in range(0, len(request_init["{{ field.name }}"][field])):
+                    del request_init["{{ field.name }}"][field][i][subfield]
+            else:
+                del request_init["{{ field.name }}"][field][subfield]
     {% endif %}
     {% endfor %}
     request = request_type(request_init)
@@ -1048,15 +1100,22 @@ def test_{{ method.name|snake_case }}_rest(request_type):
         # Wrap the value into a proper Response obj
         response_value = Response()
         response_value.status_code = 200
-    {% if method.void %}
+        {% if method.void %}
         json_return_value = ''
         {% elif method.lro %}
         json_return_value = json_format.MessageToJson(return_value)
-    {% elif method.server_streaming %}
-        json_return_value = "[{}]".format({{ method.output.ident }}.to_json(return_value))
-    {% else %}
-        json_return_value = {{ method.output.ident }}.to_json(return_value)
-    {% endif %}
+        {% else %}
+
+        {% if method.output.ident.is_proto_plus_type %}
+        # Convert return value to protobuf type
+        return_value = {{ method.output.ident }}.pb(return_value)
+        {% endif %}
+        json_return_value = json_format.MessageToJson(return_value)
+        {% endif %}
+        {% if method.server_streaming %}
+        json_return_value = "[{}]".format(json_return_value)
+        {% endif %}
+
         response_value._content = json_return_value.encode('UTF-8')
         req.return_value = response_value
         {% if method.client_streaming %}
@@ -1220,11 +1279,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
             {% else %}
 
             {% if method.output.ident.is_proto_plus_type %}
-            pb_return_value = {{ method.output.ident }}.pb(return_value)
-            {% else %}
-            pb_return_value = return_value
+            # Convert return value to protobuf type
+            return_value = {{ method.output.ident }}.pb(return_value)
             {% endif %}
-            json_return_value = json_format.MessageToJson(pb_return_value)
+            json_return_value = json_format.MessageToJson(return_value)
             {% endif %}
             {% if method.server_streaming %}
             json_return_value = "[{}]".format(json_return_value)
@@ -1345,12 +1403,6 @@ def test_{{ method_name }}_rest_bad_request(transport: str = 'rest', request_typ
 
     # send a request that will satisfy transcoding
     request_init = {{ method.http_options[0].sample_request(method) }}
-    {% for field in method.body_fields.values() %}
-    {% if not field.oneof or field.proto3_optional %}
-    {# ignore oneof fields that might conflict with sample_request #}
-    request_init["{{ field.name }}"] = {{ field.merged_mock_value(method.http_options[0].sample_request(method).get(field.name)) }}
-    {% endif %}
-    {% endfor %}
     request = request_type(request_init)
     {% if method.client_streaming %}
     requests = [request]
@@ -1391,16 +1443,21 @@ def test_{{ method.name|snake_case }}_rest_flattened():
         # Wrap the value into a proper Response obj
         response_value = Response()
         response_value.status_code = 200
-    {% if method.void %}
+        {% if method.void %}
         json_return_value = ''
         {% elif method.lro %}
         json_return_value = json_format.MessageToJson(return_value)
-    {% elif method.server_streaming %}
-        json_return_value = "[{}]".format({{ method.output.ident }}.to_json(return_value))
-    {% else %}
-        json_return_value = {{ method.output.ident }}.to_json(return_value)
-    {% endif %}
+        {% else %}
 
+        {% if method.output.ident.is_proto_plus_type %}
+        # Convert return value to protobuf type
+        return_value = {{ method.output.ident }}.pb(return_value)
+        {% endif %}
+        json_return_value = json_format.MessageToJson(return_value)
+        {% endif %}
+        {% if method.server_streaming %}
+        json_return_value = "[{}]".format(json_return_value)
+        {% endif %}
         response_value._content = json_return_value.encode('UTF-8')
         req.return_value = response_value
 

gapic/templates/setup.py.j2

@@ -30,6 +30,7 @@ dependencies = [
     "google-api-core[grpc] >= 1.34.0, <3.0.0dev,!=2.0.*,!=2.1.*,!=2.2.*,!=2.3.*,!=2.4.*,!=2.5.*,!=2.6.*,!=2.7.*,!=2.8.*,!=2.9.*,!=2.10.*",
     "proto-plus >= 1.22.0, <2.0.0dev",
     "proto-plus >= 1.22.2, <2.0.0dev; python_version>='3.11'",
+    {# Explicitly exclude protobuf versions mentioned in https://cloud.google.com/support/bulletins#GCP-2022-019 #}
     "protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5",
     {% for package_tuple, package_info in pypi_packages.items() %}
     {# Quick check to make sure the package is different from this setup.py #}

gapic/templates/tests/unit/gapic/%name_%version/%sub/test_macros.j2

@@ -887,6 +887,58 @@ def test_{{ method_name }}_rest(request_type):
     {% if not field.oneof or field.proto3_optional %}
     {# ignore oneof fields that might conflict with sample_request #}
     request_init["{{ field.name }}"] = {{ field.merged_mock_value(method.http_options[0].sample_request(method).get(field.name)) }}
+    # The version of a generated dependency at test runtime may differ from the version used during generation.
+    # Delete any fields which are not present in the current runtime dependency
+    # See https://github.com/googleapis/gapic-generator-python/issues/1748
+
+    # Determine if the message type is proto-plus or protobuf
+    is_message_proto_plus_type = not hasattr({{ method.input.ident }}.meta.fields["{{ field.name }}"].message, "DESCRIPTOR")
+
+    if is_message_proto_plus_type:
+        message_fields = {{ method.input.ident }}.meta.fields["{{ field.name }}"].message.meta.fields
+    else:
+        message_fields = {{ method.input.ident }}.meta.fields["{{ field.name }}"].message.DESCRIPTOR.fields
+
+    subfields_not_in_runtime = []
+
+    # Get all subfields for the message
+    runtime_nested_fields = [
+        (field.name, subfield.name)
+        for field in message_fields
+        if hasattr(field, "message_type") and field.message_type
+        for subfield in field.message_type.fields
+    ]
+
+    # For each item in the sample request, create a list of sub fields which are not present at runtime
+    for field, value in request_init["{{ field.name }}"].items():
+        result = None
+        is_repeated = False
+        # For repeated fields
+        if isinstance(value, list) and len(value):
+            is_repeated = True
+            result = value[0]
+        # For fields where the type is another message
+        if isinstance(value, dict):
+            result = value
+
+        if result and hasattr(result, "keys"):
+            for subfield in result.keys():
+                if (field, subfield) not in runtime_nested_fields:
+                    subfields_not_in_runtime.append(
+                        {"field": field, "subfield": subfield, "is_repeated": is_repeated}
+                    )
+
+    # Remove fields from the sample request which are not present in the runtime version of the dependency
+    for subfield_to_delete in subfields_not_in_runtime:
+        field = subfield_to_delete.get("field")
+        field_repeated = subfield_to_delete.get("is_repeated")
+        subfield = subfield_to_delete.get("subfield")
+        if subfield:
+            if field_repeated:
+                for i in range(0, len(request_init["{{ field.name }}"][field])):
+                    del request_init["{{ field.name }}"][field][i][subfield]
+            else:
+                del request_init["{{ field.name }}"][field][subfield]
     {% endif %}
     {% endfor %}
     request = request_type(**request_init)
@@ -942,11 +994,10 @@ def test_{{ method_name }}_rest(request_type):
         json_return_value = json_format.MessageToJson(return_value)
         {% else %}
         {% if method.output.ident.is_proto_plus_type %}
-        pb_return_value = {{ method.output.ident }}.pb(return_value)
-        {% else %}
-        pb_return_value = return_value
+        # Convert return value to protobuf type
+        return_value = {{ method.output.ident }}.pb(return_value)
         {% endif %}
-        json_return_value = json_format.MessageToJson(pb_return_value)
+        json_return_value = json_format.MessageToJson(return_value)
         {% endif %}
 
         {% if method.server_streaming %}
@@ -1116,11 +1167,10 @@ def test_{{ method_name }}_rest_required_fields(request_type={{ method.input.ide
             {% else %}
 
             {% if method.output.ident.is_proto_plus_type %}
-            pb_return_value = {{ method.output.ident }}.pb(return_value)
-            {% else %}
-            pb_return_value = return_value
+            # Convert return value to protobuf type
+            return_value = {{ method.output.ident }}.pb(return_value)
             {% endif %}
-            json_return_value = json_format.MessageToJson(pb_return_value)
+            json_return_value = json_format.MessageToJson(return_value)
             {% endif %}
             {% if method.server_streaming %}
             json_return_value = "[{}]".format(json_return_value)
@@ -1242,12 +1292,6 @@ def test_{{ method_name }}_rest_bad_request(transport: str = 'rest', request_typ
 
     # send a request that will satisfy transcoding
     request_init = {{ method.http_options[0].sample_request(method) }}
-    {% for field in method.body_fields.values() %}
-    {% if not field.oneof or field.proto3_optional %}
-    {# ignore oneof fields that might conflict with sample_request #}
-    request_init["{{ field.name }}"] = {{ field.merged_mock_value(method.http_options[0].sample_request(method).get(field.name)) }}
-    {% endif %}
-    {% endfor %}
     request = request_type(**request_init)
     {% if method.client_streaming %}
     requests = [request]
@@ -1308,11 +1352,10 @@ def test_{{ method_name }}_rest_flattened():
         json_return_value = json_format.MessageToJson(return_value)
         {% else %}
         {% if method.output.ident.is_proto_plus_type %}
-        pb_return_value = {{ method.output.ident }}.pb(return_value)
-        {% else %}
-        pb_return_value = return_value
+        # Convert return value to protobuf type
+        return_value = {{ method.output.ident }}.pb(return_value)
         {% endif %}
-        json_return_value = json_format.MessageToJson(pb_return_value)
+        json_return_value = json_format.MessageToJson(return_value)
         {% endif %}
         {% if method.server_streaming %}
         json_return_value = "[{}]".format(json_return_value)

noxfile.py

@@ -110,9 +110,20 @@ def __call__(self, frag):
             )
 
             # Install the generated fragment library.
-            # Note: install into the tempdir to prevent issues
-            # with running pip concurrently.
-            self.session.install(tmp_dir, "-e", ".", "-t", tmp_dir, "-qqq")
+            if self.use_ads_templates:
+                self.session.install(tmp_dir, "-e", ".", "-qqq")
+            else:
+                # Use the constraints file for the specific python runtime version.
+                # We do this to make sure that we're testing against the lowest
+                # supported version of a dependency.
+                # This is needed to recreate the issue reported in
+                # https://github.com/googleapis/gapic-generator-python/issues/1748
+                # The ads templates do not have constraints files.
+                constraints_path = str(
+                f"{tmp_dir}/testing/constraints-{self.session.python}.txt"
+                )
+                self.session.install(tmp_dir, "-e", ".", "-qqq", "-r", constraints_path)
+
             # Run the fragment's generated unit tests.
             # Don't bother parallelizing them: we already parallelize
             # the fragments, and there usually aren't too many tests per fragment.

setup.py

@@ -25,12 +25,14 @@
 version = "1.11.5"
 release_status = "Development Status :: 5 - Production/Stable"
 dependencies = [
+    # Ensure that the lower bounds of these dependencies match what we have in the
+    # templated setup.py.j2: https://github.com/googleapis/gapic-generator-python/blob/main/gapic/templates/setup.py.j2
     "click >= 6.7",
-    "google-api-core >= 2.8.0",
+    "google-api-core[grpc] >= 1.34.0, <3.0.0dev,!=2.0.*,!=2.1.*,!=2.2.*,!=2.3.*,!=2.4.*,!=2.5.*,!=2.6.*,!=2.7.*,!=2.8.*,!=2.9.*,!=2.10.*",
     "googleapis-common-protos >= 1.55.0",
     "grpcio >= 1.24.3",
     "jinja2 >= 2.10",
-    "protobuf >= 3.18.0, < 5.0.0dev",
+    "protobuf>=3.19.5,<5.0.0dev,!=3.20.0,!=3.20.1,!=4.21.0,!=4.21.1,!=4.21.2,!=4.21.3,!=4.21.4,!=4.21.5",
     "pypandoc >= 1.4",
     "PyYAML >= 5.1.1",
     "grpc-google-iam-v1 >= 0.12.4, < 1.0.0dev",

tests/fragments/google/README.rst

@@ -0,0 +1,3 @@
+The protos in this folder were copied directly from `googleapis/googleapis`_ and are needed for the purposes of running fragment tests.
+
+.. _googleapis/googleapis: https://github.com/googleapis/googleapis/tree/master/google

tests/fragments/test_google_protobuf_type.proto

@@ -0,0 +1,55 @@
+// Copyright (C) 2023  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
+//
+//     http://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.
+
+
+// The purpose of this fragment test is to test for an issue found in the generated
+// client for `google/api/servicemanagement/v1` where the version of a generated
+// dependency (google.protobuf.type) at runtime differs from the version used during
+// generation. See https://github.com/googleapis/gapic-generator-python/issues/1748.
+
+syntax = "proto3";
+
+package google.fragment;
+
+import "google/api/annotations.proto";
+import "google/api/client.proto";
+import "google/api/field_behavior.proto";
+import "google/protobuf/struct.proto";
+import "google/protobuf/type.proto";
+
+service MyServiceWithProtobufType {
+  option (google.api.default_host) = "my.example.com";
+
+  rpc MyMethod(MethodRequestWithProtobufType)
+      returns (MethodResponseWithProtobufType) {
+    option (google.api.http) = {
+      post: "/v1/services/{service_name}/configs"
+      body: "test_message"
+    };
+    option (google.api.method_signature) = "service_name,test_message";
+  }
+}
+
+message MethodRequestWithProtobufType {
+  string service_name = 1 [(google.api.field_behavior) = REQUIRED];
+  TestMessage test_message = 2 [(google.api.field_behavior) = REQUIRED];
+}
+
+message TestMessage {
+  repeated google.protobuf.Type types = 2 [(google.api.field_behavior) = REQUIRED];
+}
+
+message MethodResponseWithProtobufType {
+  google.protobuf.Value result = 1;
+}