Numerous small cleanups (#328)

Add paginated result to a method's refererred to types
Remove empty request construction for cross package requests with no flattened fields

Author: software-dov

packages/gapic-generator/gapic/schema/wrappers.py

@@ -653,6 +653,11 @@ def _ref_types(self, use_legacy: bool = False) -> Sequence[Union[MessageType, En
             answer.append(self.lro.response_type)
             answer.append(self.lro.metadata_type)
 
+        # If this message paginates its responses, it is possible
+        # that the individual result messages reside in a different module.
+        if self.paged_result_field:
+            answer.append(self.paged_result_field.message)
+
         # Done; return the answer.
         return tuple(answer)
 

packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2

@@ -195,16 +195,18 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
         # so it must be constructed via keyword expansion.
         if isinstance(request, dict):
             request = {{ method.input.ident }}(**request)
+        {% if method.flattened_fields -%}{# Cross-package req and flattened fields #}
         elif not request:
             request = {{ method.input.ident }}()
+        {% endif -%}{# Cross-package req and flattened fields #}
         {%- else %}
         request = {{ method.input.ident }}(request)
         {% endif %} {# different request package #}
 
-        # If we have keyword arguments corresponding to fields on the
-        # request, apply these.
         {#- Vanilla python protobuf wrapper types cannot _set_ repeated fields #}
         {%- for key, field in method.flattened_fields.items() if not(field.repeated and method.input.ident.package != method.ident.package) %}
+        # If we have keyword arguments corresponding to fields on the
+        # request, apply these.
         if {{ field.name }} is not None:
             request.{{ key }} = {{ field.name }}
         {%- endfor %}

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

@@ -294,8 +294,8 @@ def test_{{ method.name|snake_case }}_pager():
             request={},
         )]
         assert len(results) == 6
-        assert all([isinstance(i, {{ method.paged_result_field.message.ident }})
-                    for i in results])
+        assert all(isinstance(i, {{ method.paged_result_field.message.ident }})
+                   for i in results)
 
 def test_{{ method.name|snake_case }}_pages():
     client = {{ service.client_name }}(

packages/gapic-generator/tests/unit/schema/wrappers/test_method.py

@@ -113,6 +113,40 @@ def test_method_paged_result_field_no_page_field():
     assert method.paged_result_field is None
 
 
+def test_method_paged_result_ref_types():
+    input_msg = make_message(
+        name='ListSquidsRequest',
+        fields=(
+            make_field(name='parent', type=9),      # str
+            make_field(name='page_size', type=5),   # int
+            make_field(name='page_token', type=9),  # str
+        ),
+        module='squid',
+    )
+    mollusc_msg = make_message('Mollusc', module='mollusc')
+    output_msg = make_message(
+        name='ListMolluscsResponse',
+        fields=(
+            make_field(name='molluscs', message=mollusc_msg, repeated=True),
+            make_field(name='next_page_token', type=9)
+        ),
+        module='mollusc'
+    )
+    method = make_method(
+        'ListSquids',
+        input_message=input_msg,
+        output_message=output_msg,
+        module='squid'
+    )
+
+    ref_type_names = {t.name for t in method.ref_types}
+    assert ref_type_names == {
+        'ListSquidsRequest',
+        'ListSquidsPager',
+        'Mollusc',
+    }
+
+
 def test_method_field_headers_none():
     method = make_method('DoSomething')
     assert isinstance(method.field_headers, collections.abc.Sequence)