fix: disambiguate field headers whose names are reserved python words (…

…#1178) * fix: disambiguate field headers whose names are reserved python words * Add FieldHeader class * update tests and templates
googleapis · Feb 12, 2022 · 98aa690 · 98aa690
1 parent 246bfe2
commit 98aa690
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 15 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
@@ -452,7 +452,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
             gapic_v1.routing_header.to_grpc_metadata((
                 {% for field_header in method.field_headers %}
                 {% if not method.client_streaming %}
-                ("{{ field_header }}", request.{{ field_header }}),
+                ("{{ field_header.raw }}", request.{{ field_header.disambiguated }}),
                 {% endif %}
                 {% endfor %}
             )),

diff --git a/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
@@ -595,7 +595,7 @@ def test_{{ method_name }}_field_headers():
     request = {{ method.input.ident }}()
 
     {% for field_header in method.field_headers %}
-    request.{{ field_header }} = '{{ field_header }}/value'
+    request.{{ field_header.disambiguated }} = '{{ field_header.raw }}/value'
     {% endfor %}
 
     # Mock the actual call within the gRPC stub, and fake the request.
@@ -623,7 +623,7 @@ def test_{{ method_name }}_field_headers():
     assert (
         'x-goog-request-params',
         '{% for field_header in method.field_headers -%}
-        {{ field_header }}={{ field_header }}/value
+        {{ field_header.raw }}={{ field_header.raw }}/value
         {%- if not loop.last %}&{% endif %}
         {%- endfor -%}',
     ) in kw['metadata']
@@ -783,7 +783,7 @@ def test_{{ method_name }}_pager(transport_name: str = "grpc"):
             gapic_v1.routing_header.to_grpc_metadata((
                 {% for field_header in method.field_headers %}
                 {% if not method.client_streaming %}
-                ('{{ field_header }}', ''),
+                ('{{ field_header.raw }}', ''),
                 {% endif %}
                 {% endfor %}
             )),

diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py
@@ -332,6 +332,15 @@ def with_context(
         )
 
 
+@dataclasses.dataclass(frozen=True)
+class FieldHeader:
+    raw: str
+
+    @property
+    def disambiguated(self) -> str:
+        return self.raw + "_" if self.raw in utils.RESERVED_NAMES else self.raw
+
+
 @dataclasses.dataclass(frozen=True)
 class Oneof:
     """Description of a field."""
@@ -1070,8 +1079,9 @@ def is_deprecated(self) -> bool:
     #               e.g. doesn't work with basic case of gRPC transcoding
 
     @property
-    def field_headers(self) -> Sequence[str]:
+    def field_headers(self) -> Sequence[FieldHeader]:
         """Return the field headers defined for this method."""
+
         http = self.options.Extensions[annotations_pb2.http]
 
         pattern = re.compile(r'\{([a-z][\w\d_.]+)=')
@@ -1084,8 +1094,13 @@ def field_headers(self) -> Sequence[str]:
             http.patch,
             http.custom.path,
         ]
-
-        return next((tuple(pattern.findall(verb)) for verb in potential_verbs if verb), ())
+        field_headers = (
+            tuple(FieldHeader(field_header)
+                  for field_header in pattern.findall(verb))
+            for verb in potential_verbs
+            if verb
+        )
+        return next(field_headers, ())
 
     @property
     def explicit_routing(self):

diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2
@@ -327,7 +327,7 @@ class {{ service.async_client_name }}:
             gapic_v1.routing_header.to_grpc_metadata((
                 {% for field_header in method.field_headers %}
                 {% if not method.client_streaming %}
-                ("{{ field_header }}", request.{{ field_header }}),
+                ("{{ field_header.raw }}", request.{{ field_header.disambiguated }}),
                 {% endif %}
                 {% endfor %}
             )),

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
@@ -509,7 +509,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
             gapic_v1.routing_header.to_grpc_metadata((
                 {% for field_header in method.field_headers %}
                 {% if not method.client_streaming %}
-                ("{{ field_header }}", request.{{ field_header }}),
+                ("{{ field_header.raw }}", request.{{ field_header.disambiguated }}),
                 {% endif %}
                 {% endfor %}
             )),

diff --git a/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
@@ -805,7 +805,7 @@ def test_{{ method_name }}_field_headers():
     request = {{ method.input.ident }}()
 
     {% for field_header in method.field_headers %}
-    request.{{ field_header }} = '{{ field_header }}/value'
+    request.{{ field_header.disambiguated }} = '{{ field_header.raw }}/value'
     {% endfor %}
 
     # Mock the actual call within the gRPC stub, and fake the request.
@@ -833,7 +833,7 @@ def test_{{ method_name }}_field_headers():
     assert (
         'x-goog-request-params',
         '{% for field_header in method.field_headers -%}
-        {{ field_header }}={{ field_header }}/value
+        {{ field_header.raw }}={{ field_header.raw }}/value
         {%- if not loop.last %}&{% endif %}
         {%- endfor -%}',
     ) in kw['metadata']
@@ -850,7 +850,7 @@ async def test_{{ method_name }}_field_headers_async():
     request = {{ method.input.ident }}()
 
     {% for field_header in method.field_headers %}
-    request.{{ field_header }} = '{{ field_header }}/value'
+    request.{{ field_header.disambiguated }} = '{{ field_header.raw }}/value'
     {% endfor %}
 
     # Mock the actual call within the gRPC stub, and fake the request.
@@ -879,7 +879,7 @@ async def test_{{ method_name }}_field_headers_async():
     assert (
         'x-goog-request-params',
         '{% for field_header in method.field_headers -%}
-        {{ field_header }}={{ field_header }}/value
+        {{ field_header.raw }}={{ field_header.raw }}/value
         {%- if not loop.last %}&{% endif %}
         {%- endfor -%}',
     ) in kw['metadata']
@@ -1128,7 +1128,7 @@ def test_{{ method_name }}_pager(transport_name: str = "grpc"):
             gapic_v1.routing_header.to_grpc_metadata((
                 {% for field_header in method.field_headers %}
                 {% if not method.client_streaming %}
-                ('{{ field_header }}', ''),
+                ('{{ field_header.raw }}', ''),
                 {% endif %}
                 {% endfor %}
             )),

diff --git a/tests/unit/schema/wrappers/test_method.py b/tests/unit/schema/wrappers/test_method.py
@@ -287,7 +287,16 @@ def test_method_field_headers_present():
     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',)
+        assert method.field_headers == (wrappers.FieldHeader('parent'),)
+        assert method.field_headers[0].raw == 'parent'
+        assert method.field_headers[0].disambiguated == 'parent'
+
+        # test that reserved keyword in field header is disambiguated
+        rule = http_pb2.HttpRule(**{v: '/v1/{object=objects/*}/topics'})
+        method = make_method('DoSomething', http_rule=rule)
+        assert method.field_headers == (wrappers.FieldHeader('object'),)
+        assert method.field_headers[0].raw == 'object'
+        assert method.field_headers[0].disambiguated == 'object_'
 
 
 def test_method_routing_rule():