Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: disambiguate field headers whose names are reserved python words #1178

Merged
merged 5 commits into from
Feb 12, 2022
Merged

fix: disambiguate field headers whose names are reserved python words #1178

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7fd8460
fix: disambiguate field headers whose names are reserved python words
parthea Feb 2, 2022
e534e8a
Merge branch 'main' into disambiguate-field-headers
parthea Feb 2, 2022
6bb69b8
Merge branch 'main' into disambiguate-field-headers
parthea Feb 12, 2022
412127a
Add FieldHeader class
parthea Feb 12, 2022
97a4dce
update tests and templates
parthea Feb 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
)),
Expand Down
6 changes: 3 additions & 3 deletions gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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']
Expand Down Expand Up @@ -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 %}
)),
Expand Down
16 changes: 13 additions & 3 deletions gapic/schema/wrappers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,8 +965,13 @@ 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[dict]:
parthea marked this conversation as resolved.
Show resolved Hide resolved
"""Return the field headers defined for this method."""

def disambiguate_field_headers(input_str: str) -> str:
"""Used to prevent collisions with python keywords"""
return input_str + "_" if input_str in utils.RESERVED_NAMES else input_str

http = self.options.Extensions[annotations_pb2.http]

pattern = re.compile(r'\{([a-z][\w\d_.]+)=')
Expand All @@ -979,8 +984,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({"raw": field_header, "disambiguated": disambiguate_field_headers(field_header)}
for field_header in pattern.findall(verb))
for verb in potential_verbs
if verb
)
return next(field_headers, ())

@property
def http_options(self) -> List[HttpRule]:
Expand Down
2 changes: 1 addition & 1 deletion gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
)),
Expand Down
2 changes: 1 addition & 1 deletion gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,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 %}
)),
Expand Down
10 changes: 5 additions & 5 deletions gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,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.
Expand Down Expand Up @@ -795,7 +795,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']
Expand All @@ -812,7 +812,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.
Expand Down Expand Up @@ -841,7 +841,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']
Expand Down Expand Up @@ -1090,7 +1090,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 %}
)),
Expand Down
9 changes: 8 additions & 1 deletion tests/unit/schema/wrappers/test_method.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,14 @@ 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 == (
{'raw': 'parent', '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 == (
{'raw': 'object', 'disambiguated': 'object_'},)


def test_method_http_opt():
Expand Down