fix: add field headers for other http verbs (#443)

Closes #401
googleapis · Jun 9, 2020 · f8e9712 · f8e9712
1 parent decae37
commit f8e9712
Show file tree

Hide file tree

Showing 6 changed files with 65 additions and 27 deletions.
diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2
@@ -333,7 +333,9 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
         metadata = tuple(metadata) + (
             gapic_v1.routing_header.to_grpc_metadata((
                 {%- for field_header in method.field_headers %}
+                {%- if not method.client_streaming %}
                 ('{{ field_header }}', request.{{ field_header }}),
+                {%- endif %}
                 {%- endfor %}
             )),
         )

diff --git a/gapic/ads-templates/tests/unit/%name_%version/%sub/test_%service.py.j2 b/gapic/ads-templates/tests/unit/%name_%version/%sub/test_%service.py.j2
@@ -258,25 +258,33 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
     {% endfor %}
     {% endif %}
 
-{% if method.field_headers %}
+{% if method.field_headers and not method.client_streaming %}
 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 %}
-    )
+    request = {{ method.input.ident }}()
+
+    {%- for field_header in method.field_headers %}
+    request.{{ 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:
+        {% 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 %}
         client.{{ method.name|snake_case }}(request)
 
         # Establish that the underlying gRPC stub method was called.
@@ -471,7 +479,6 @@ def test_{{ method.name|snake_case }}_raw_page_lro():
 
 {% endfor -%} {#- method in methods #}
 
-
 def test_credentials_transport_error():
     # It is an error to provide credentials and a transport instance.
     transport = transports.{{ service.name }}GrpcTransport(
@@ -689,12 +696,12 @@ def test_{{ service.name|snake_case }}_grpc_lro_client():
 {% for message in service.resource_messages -%}
 {% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") -%}
 def test_{{ message.resource_type|snake_case }}_path():
-  {% for arg in message.resource_path_args -%}
-  {{ arg }} = "{{ molluscs.next() }}"
-  {% endfor %}
-  expected = "{{ message.resource_path }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %})
-  actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }})
-  assert expected == actual
+    {% for arg in message.resource_path_args -%}
+    {{ arg }} = "{{ molluscs.next() }}"
+    {% endfor %}
+    expected = "{{ message.resource_path }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %})
+    actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }})
+    assert expected == actual
 
 
 def test_parse_{{ message.resource_type|snake_case }}_path():

diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py
@@ -625,9 +625,19 @@ def client_output(self):
     def field_headers(self) -> Sequence[str]:
         """Return the field headers defined for this method."""
         http = self.options.Extensions[annotations_pb2.http]
-        if http.get:
-            return tuple(re.findall(r'\{([a-z][\w\d_.]+)=', http.get))
-        return ()
+
+        pattern = re.compile(r'\{([a-z][\w\d_.]+)=')
+
+        potential_verbs = [
+            http.get,
+            http.put,
+            http.post,
+            http.delete,
+            http.patch,
+            http.custom.path,
+        ]
+
+        return next((tuple(pattern.findall(verb)) for verb in potential_verbs if verb), ())
 
     @utils.cached_property
     def flattened_fields(self) -> Mapping[str, Field]:

diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2
@@ -333,7 +333,9 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
         metadata = tuple(metadata) + (
             gapic_v1.routing_header.to_grpc_metadata((
                 {%- for field_header in method.field_headers %}
+                {%- if not method.client_streaming %}
                 ('{{ field_header }}', request.{{ field_header }}),
+                {%- endif %}
                 {%- endfor %}
             )),
         )

diff --git a/gapic/templates/tests/unit/%name_%version/%sub/test_%service.py.j2 b/gapic/templates/tests/unit/%name_%version/%sub/test_%service.py.j2
@@ -258,25 +258,33 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
     {% endfor %}
     {% endif %}
 
-{% if method.field_headers %}
+{% if method.field_headers and not method.client_streaming %}
 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 %}
-    )
+    request = {{ method.input.ident }}()
+
+    {%- for field_header in method.field_headers %}
+    request.{{ 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:
+        {% 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 %}
         client.{{ method.name|snake_case }}(request)
 
         # Establish that the underlying gRPC stub method was called.

diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py
@@ -205,9 +205,18 @@ def test_method_field_headers_none():
 
 
 def test_method_field_headers_present():
-    http_rule = http_pb2.HttpRule(get='/v1/{parent=projects/*}/topics')
-    method = make_method('DoSomething', http_rule=http_rule)
-    assert method.field_headers == ('parent',)
+    verbs = [
+        'get',
+        'put',
+        'post',
+        'delete',
+        'patch',
+    ]
+
+    for v in verbs:
+        rule = http_pb2.HttpRule(**{v: '/v1/{parent=projects/*}/topics'})
+        method = make_method('DoSomething', http_rule=rule)
+        assert method.field_headers == ('parent',)
 
 
 def test_method_idempotent_yes():