-
Notifications
You must be signed in to change notification settings - Fork 78
fix: Adjust Field Names in URI Templates #1041
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| # Copyright 2021 Google LLC | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| from gapic.utils.reserved_names import RESERVED_NAMES | ||
| from google.api_core import path_template # type: ignore | ||
|
|
||
|
|
||
| def convert_uri_fieldnames(uri: str) -> str: | ||
| """Modify field names in uri_templates to avoid reserved names. | ||
|
|
||
| Args: | ||
| uri: a uri template, optionally containing field references in {} braces. | ||
|
|
||
| Returns: | ||
| the uri with any field names modified if they conflict with | ||
| reserved names. | ||
| """ | ||
|
|
||
| def _fix_name_segment(name_seg: str) -> str: | ||
| return name_seg + "_" if name_seg in RESERVED_NAMES else name_seg | ||
|
|
||
| def _fix_field_path(field_path: str) -> str: | ||
| return ".".join( | ||
| (_fix_name_segment(name_seg) for name_seg in field_path.split("."))) | ||
|
|
||
| last = 0 | ||
| pieces = [] | ||
| for match in path_template._VARIABLE_RE.finditer(uri): | ||
| start_pos, end_pos = match.span("name") | ||
| if start_pos == end_pos: | ||
| continue | ||
| pieces.append(uri[last:start_pos]) | ||
| fixed_field_path = _fix_field_path(uri[start_pos:end_pos]) | ||
| pieces.append(fixed_field_path) | ||
| last = end_pos | ||
| pieces.append(uri[last:]) | ||
|
|
||
| return "".join(pieces) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -452,6 +452,19 @@ def test_method_http_options_additional_bindings(): | |
| }] | ||
|
|
||
|
|
||
| def test_method_http_options_reserved_name_in_url(): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we also add the short template format ( In general, please treat the short format as a first-class citizen for all grpc transcodding work, because this is the only format used in compute, which is the most importantlibrary relying on rest format.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have that form in the unit test test_uri_conv.py. I can add other names.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, the test there already includes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is |
||
| http_rule = http_pb2.HttpRule( | ||
| post='/v1/license/{license=lic/*}', | ||
| body='*' | ||
| ) | ||
| method = make_method('DoSomething', http_rule=http_rule) | ||
| assert [dataclasses.asdict(http) for http in method.http_options] == [{ | ||
| 'method': 'post', | ||
| 'uri': '/v1/license/{license_=lic/*}', | ||
| 'body': '*' | ||
| }] | ||
|
|
||
|
|
||
| def test_method_http_options_generate_sample(): | ||
| http_rule = http_pb2.HttpRule( | ||
| get='/v1/{resource.id=projects/*/regions/*/id/**}/stuff', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| # Copyright 2021 Google LLC | ||
| # | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # you may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # https://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
|
|
||
| from unittest import mock | ||
|
|
||
| import pypandoc | ||
|
|
||
| from gapic import utils | ||
|
|
||
|
|
||
| def test_convert_uri_fieldname(): | ||
| uri = "abc/*/license/{license}/{xyz.reversed=reversed/*}" | ||
| expected_uri = "abc/*/license/{license_}/{xyz.reversed_=reversed/*}" | ||
| assert utils.convert_uri_fieldnames(uri) == expected_uri | ||
|
|
||
|
|
||
| def test_convert_uri_fieldname_no_fields(): | ||
| uri = "abc/license" | ||
| assert utils.convert_uri_fieldnames(uri) == uri | ||
|
|
||
|
|
||
| def test_convert_uri_fieldname_no_reserved_names(): | ||
| uri = "abc/*/books/{book}/{xyz.chapter=page/*}" | ||
| assert utils.convert_uri_fieldnames(uri) == uri |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can
_VARIABLE_REbe made public ingoogle-api-core? Otherwise someone might assume it's only used ingoogle-api-coreand make a change that breaks the generator.Slightly lower priority since it's in the generator code and not in generated library code.