Re-add original flattening semantics (#256)

Not a strict readition: flattened fields can be non-primitives.

Author: software-dov

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

@@ -545,26 +545,14 @@ def field_headers(self) -> Sequence[str]:
     @utils.cached_property
     def flattened_fields(self) -> Mapping[str, Field]:
         """Return the signature defined for this method."""
-        answer: Dict[str, Field] = collections.OrderedDict()
         signatures = self.options.Extensions[client_pb2.method_signature]
 
-        # Iterate over each signature and add the appropriate fields.
-        for sig in signatures:
-            # Get all of the individual fields.
-            fields = collections.OrderedDict([
-                (f.strip(), self.input.get_field(*f.strip().split('.')))
-                for f in sig.split(',')
-            ])
-
-            # Sanity check: If any fields contain a message, we ignore the
-            # entire signature.
-            if any([i.message for i in fields.values()]):
-                continue
-
-            # Add the fields to the answer.
-            answer.update(fields)
+        answer: Dict[str, Field] = collections.OrderedDict(
+            (f.strip(), self.input.get_field(*f.strip().split('.')))
+            for sig in signatures
+            for f in sig.split(',')
+        )
 
-        # Done; return the flattened fields
         return answer
 
     @utils.cached_property

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

@@ -126,14 +126,11 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
 
     {% for method in service.methods.values() -%}
     def {{ method.name|snake_case }}(self,
-            {% for field in method.legacy_flattened_fields.values() -%}
-            {% if field.required -%}
-            {{ field.name }}: {{ field.ident }},
-            {% else -%}
+            request: {{ method.input.ident }} = None,
+            *,
+            {% for field in method.flattened_fields.values() -%}
             {{ field.name }}: {{ field.ident }} = None,
-            {% endif -%}
             {% endfor -%}
-            *,
             retry: retries.Retry = gapic_v1.method.DEFAULT,
             timeout: float = None,
             metadata: Sequence[Tuple[str, str]] = (),
@@ -164,13 +161,20 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
         {%- endif %}
         """
         # Create or coerce a protobuf request object.
-        {% if method.legacy_flattened_fields -%}
+        {% if method.flattened_fields -%}
+        # Sanity check: If we got a request object, we should *not* have
+        # gotten any keyword arguments that map to the request.
+        if request is not None and any([{{ method.flattened_fields.values()|join(', ', attribute='name') }}]):
+            raise ValueError('If the `request` argument is set, then none of '
+                             'the individual field arguments should be set.')
+
         # If we have keyword arguments corresponding to fields on the
         # request, apply these.
         {% endif -%}
-        request = {{ method.input.ident }}()
-        {%- for field in method.legacy_flattened_fields.values() %}
-        request.{{ field.name }} = {{ field.name }}
+        request = {{ method.input.ident }}(request)
+        {%- for key, field in method.flattened_fields.items() %}
+        if {{ field.name }} is not None:
+            request.{{ key }} = {{ field.name }}
         {%- endfor %}
 
         # Wrap the RPC method; this adds retry and timeout information,

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

@@ -71,6 +71,10 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
         transport=transport,
     )
 
+    # Everything is optional in proto3 as far as the runtime is concerned,
+    # and we are mocking out the actual API, so just send an empty request.
+    request = {{ method.input.ident }}()
+
     # Mock the actual call within the gRPC stub, and fake the request.
     with mock.patch.object(
             type(client._transport.{{ method.name|snake_case }}),
@@ -89,18 +93,12 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
             {%- endfor %}
         )
         {% endif -%}
-        # Everything is optional in proto3 as far as the runtime is concerned,
-        # and we are mocking out the actual API, so just send an empty request.
-        response = client.{{ method.name|snake_case }}(
-            {% for field in method.legacy_flattened_fields.values() -%}
-            {{ field.name }}=None,
-            {% endfor -%}
-        )
-
+        response = client.{{ method.name|snake_case }}(request)
+        
         # Establish that the underlying gRPC stub method was called.
         assert len(call.mock_calls) == 1
         _, args, _ = call.mock_calls[0]
-        assert args[0] == {{ method.input.ident }}()
+        assert args[0] == request
 
     # Establish that the response is the type that we expect.
     {% if method.void -%}
@@ -121,29 +119,26 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
 def test_{{ method.name|snake_case }}_field_headers():
     client = {{ service.client_name }}(
         credentials=credentials.AnonymousCredentials(),
+  )
+
+    # Any value that is part of the HTTP/1.1 URI should be sent as
+    # a field header. Set these to a non-empty value.
+    request = {{ method.input.ident }}(
+        {%- for field_header in method.field_headers %}
+        {{ field_header }}='{{ field_header }}/value',
+        {%- endfor %}
     )
 
     # Mock the actual call within the gRPC stub, and fake the request.
     with mock.patch.object(
             type(client._transport.{{ method.name|snake_case }}),
             '__call__') as call:
         call.return_value = {{ method.output.ident }}()
-        # Any value that is part of the HTTP/1.1 URI should be sent as
-        # a field header. Set these to a non-empty value.
-        response = client.{{ method.name|snake_case }}(
-        {%- for field_header in method.field_headers %}
-            {{ field_header }}='{{ field_header }}/value',
-        {%- endfor %}
-        )
-
+        response = client.{{ method.name|snake_case }}(request)
+        
         # Establish that the underlying gRPC stub method was called.
         assert len(call.mock_calls) == 1
         _, args, _ = call.mock_calls[0]
-        request = {{ method.input.ident }}(
-        {%- for field_header in method.field_headers %}
-            {{ field_header }}='{{ field_header }}/value',
-        {%- endfor %}
-        )
         assert args[0] == request
 
     # Establish that the field header was sent.
@@ -155,63 +150,60 @@ def test_{{ method.name|snake_case }}_field_headers():
         {%- if not loop.last %}&{% endif -%}
         {%- endfor %}',
     ) in kw['metadata']
-{% endif %} {#- method.field_headers #}
-
-{# NOTE: the backwards compatibility requirements of legacy flattening make these tests no longer relevant #}
-{# If the legacy flattening is ever abandoned, these tests or something like them will be needed again. #}
-{#  #}
-{# {% if method.flattened_fields %} #}
-{# def test_{{ method.name|snake_case }}_flattened(): #}
-{#     client = {{ service.client_name }}( #}
-{#         credentials=credentials.AnonymousCredentials(), #}
-{#     ) #}
-
-{#     # Mock the actual call within the gRPC stub, and fake the request. #}
-{#     with mock.patch.object( #}
-{#             type(client._transport.{{ method.name|snake_case }}), #}
-{#             '__call__') as call: #}
-{#         # Designate an appropriate return value for the call. #}
-{#         {% if method.void -%} #}
-{#         call.return_value = None #}
-{#         {% elif method.lro -%} #}
-{#         call.return_value = operations_pb2.Operation(name='operations/op') #}
-{#         {% elif method.server_streaming -%} #}
-{#         call.return_value = iter([{{ method.output.ident }}()]) #}
-{#         {% else -%} #}
-{#         call.return_value = {{ method.output.ident }}() #}
-{#         {% endif %} #}
-{#         # Call the method with a truthy value for each flattened field, #}
-{#         # using the keyword arguments to the method. #}
-{#         response = client.{{ method.name|snake_case }}( #}
-{#             {%- for key, field in method.flattened_fields.items() %} #}
-{#             {{ field.name }}={{ field.mock_value }}, #}
-{#             {%- endfor %} #}
-{#         ) #}
-
-{#         # Establish that the underlying call was made with the expected #}
-{#         # request object values. #}
-{#         assert len(call.mock_calls) == 1 #}
-{#         _, args, _ = call.mock_calls[0] #}
-{#         {% for key, field in method.flattened_fields.items() -%} #}
-{#         assert args[0].{{ key }} == {{ field.mock_value }} #}
-{#         {% endfor %} #}
-
-
-{# def test_{{ method.name|snake_case }}_flattened_error(): #}
-{#     client = {{ service.client_name }}( #}
-{#         credentials=credentials.AnonymousCredentials(), #}
-{#     ) #}
-
-{#     # Attempting to call a method with both a request object and flattened #}
-{#     # fields is an error. #}
-{#     with pytest.raises(ValueError): #}
-{#         client.{{ method.name|snake_case }}( #}
-{#             {{ method.input.ident }}(), #}
-{#             {%- for key, field in method.flattened_fields.items() %} #}
-{#             {{ field.name }}={{ field.mock_value }}, #}
-{#             {%- endfor %} #}
-{#         ) #}
-{# {% endif %} {\#- method.flattened_fields #\} #}
+{% endif %}
+
+{% if method.flattened_fields %} 
+def test_{{ method.name|snake_case }}_flattened(): 
+    client = {{ service.client_name }}( 
+        credentials=credentials.AnonymousCredentials(), 
+    ) 
+
+    # Mock the actual call within the gRPC stub, and fake the request. 
+    with mock.patch.object( 
+            type(client._transport.{{ method.name|snake_case }}), 
+            '__call__') as call: 
+        # Designate an appropriate return value for the call. 
+        {% if method.void -%} 
+        call.return_value = None 
+        {% elif method.lro -%} 
+        call.return_value = operations_pb2.Operation(name='operations/op') 
+        {% elif method.server_streaming -%} 
+        call.return_value = iter([{{ method.output.ident }}()]) 
+        {% else -%} 
+        call.return_value = {{ method.output.ident }}() 
+        {% endif %} 
+        # Call the method with a truthy value for each flattened field, 
+        # using the keyword arguments to the method. 
+        response = client.{{ method.name|snake_case }}( 
+            {%- for field in method.flattened_fields.values() %} 
+            {{ field.name }}={{ field.mock_value }}, 
+            {%- endfor %} 
+        ) 
+
+        # Establish that the underlying call was made with the expected 
+        # request object values. 
+        assert len(call.mock_calls) == 1 
+        _, args, _ = call.mock_calls[0] 
+        {% for key, field in method.flattened_fields.items() -%}
+        assert args[0].{{ key }} == {{ field.mock_value }} 
+        {% endfor %} 
+
+
+def test_{{ method.name|snake_case }}_flattened_error(): 
+    client = {{ service.client_name }}( 
+        credentials=credentials.AnonymousCredentials(), 
+    ) 
+
+    # Attempting to call a method with both a request object and flattened 
+    # fields is an error. 
+    with pytest.raises(ValueError): 
+        client.{{ method.name|snake_case }}( 
+            {{ method.input.ident }}(), 
+            {%- for field in method.flattened_fields.values() %} 
+            {{ field.name }}={{ field.mock_value }}, 
+            {%- endfor %} 
+        ) 
+{% endif %}
 
 
 {% if method.paged_result_field %}
@@ -252,7 +244,9 @@ def test_{{ method.name|snake_case }}_pager():
             ),
             RuntimeError,
         )
-        results = [i for i in client.{{ method.name|snake_case }}({% for field in method.legacy_flattened_fields.values() if field.required -%}None,{% endfor -%})]
+        results = [i for i in client.{{ method.name|snake_case }}(
+            request={},
+        )]
         assert len(results) == 6
         assert all([isinstance(i, {{ method.paged_result_field.message.ident }})
                     for i in results])
@@ -294,7 +288,7 @@ def test_{{ method.name|snake_case }}_pages():
             ),
             RuntimeError,
         )
-        pages = list(client.{{ method.name|snake_case }}({% for field in method.legacy_flattened_fields.values() if field.required -%}None,{% endfor -%}).pages)
+        pages = list(client.{{ method.name|snake_case }}(request={}).pages)
         for page, token in zip(pages, ['abc','def','ghi', '']):
             assert page.raw_page.next_page_token == token
 {% elif method.lro and "next_page_token" in method.lro.response_type.fields.keys() %}
@@ -352,7 +346,7 @@ def test_{{ service.name|snake_case }}_base_transport():
     )
     for method in methods:
         with pytest.raises(NotImplementedError):
-            getattr(transport, method)()
+            getattr(transport, method)(request=object())
 
     {% if service.has_lro -%}
     # Additionally, the LRO client (a property) should

packages/gapic-generator/tests/system/test_grpc_lro.py

@@ -18,11 +18,11 @@
 
 
 def test_lro(echo):
-    future = echo.wait(
-        end_time=datetime.now(tz=timezone.utc) + timedelta(seconds=1),
-        success={
+    future = echo.wait({
+        'end_time': datetime.now(tz=timezone.utc) + timedelta(seconds=1),
+        'success': {
             'content': 'The hail in Wales falls mainly on the snails...eventually.'
-        }
+        }}
     )
     response = future.result()
     assert isinstance(response, showcase_v1beta1.WaitResponse)

packages/gapic-generator/tests/system/test_grpc_streams.py

@@ -15,9 +15,9 @@
 
 def test_unary_stream(echo):
     content = 'The hail in Wales falls mainly on the snails.'
-    responses = echo.expand(
-        content=content,
-    )
+    responses = echo.expand({
+        'content': content,
+    })
 
     # Consume the response and ensure it matches what we expect.
     # with pytest.raises(exceptions.NotFound) as exc:

packages/gapic-generator/tests/system/test_grpc_unary.py

@@ -20,28 +20,28 @@
 from google import showcase
 
 
-def test_unary(echo):
-    content = 'The hail in Wales falls mainly on the snails.'
-    response = echo.echo(
-        content=content,
-    )
-    assert response.content == content
+def test_unary_with_request_object(echo):
+    response = echo.echo(showcase.EchoRequest(
+        content='The hail in Wales falls mainly on the snails.',
+    ))
+    assert response.content == 'The hail in Wales falls mainly on the snails.'
 
 
-def test_unary_positional(echo):
-    content = 'The hail in Wales falls mainly on the snails.'
-    response = echo.echo(content,)
-    assert response.content == content
+def test_unary_with_dict(echo):
+    response = echo.echo({
+        'content': 'The hail in Wales falls mainly on the snails.',
+    })
+    assert response.content == 'The hail in Wales falls mainly on the snails.'
 
 
 def test_unary_error(echo):
     message = 'Bad things! Bad things!'
     with pytest.raises(exceptions.InvalidArgument) as exc:
-        echo.echo(
-            error={
+        echo.echo({
+            'error': {
                 'code': code_pb2.Code.Value('INVALID_ARGUMENT'),
                 'message': message,
             },
-        )
+        })
         assert exc.value.code == 400
         assert exc.value.message == message

packages/gapic-generator/tests/system/test_pagination.py

@@ -17,21 +17,21 @@
 
 def test_pagination(echo):
     text = 'The hail in Wales falls mainly on the snails.'
-    results = [i for i in echo.paged_expand(
-        content=text,
-        page_size=3,
-    )]
+    results = [i for i in echo.paged_expand({
+        'content': text,
+        'page_size': 3,
+    })]
     assert len(results) == 9
     assert results == [showcase.EchoResponse(content=i)
                        for i in text.split(' ')]
 
 
 def test_pagination_pages(echo):
     text = "The hail in Wales falls mainly on the snails."
-    page_results = list(echo.paged_expand(
-        content=text,
-        page_size=3,
-    ).pages)
+    page_results = list(echo.paged_expand({
+        'content': text,
+        'page_size': 3,
+    }).pages)
 
     assert len(page_results) == 3
     assert not page_results[-1].next_page_token