Skip to content

Commit 5cbe8b2

Browse files
authored
fix: Fix rest transport logic (#1039)
* fix: Fix rest transport logic This includes 1) Do not include asyncio tests in the generated tests, because rest transport does not have asynio client. 2) Generate body field mock values for generated tests (otherwise grpc transcodding logic would fail). 3) Make `always_use_jwt_access=True` default for rest clients (grpc already does that) to match expected calls in generated tests. 4) Fix mypy errors for `AuthorizedSession` by ignoring it 5) Include operations_v1 conditionally, only if the client has lro There are few more fixes left, which are expected to be fixed in separate PRs. 1) `message->to_dict->message` roundrtip problem for int64 types is expected to be fixed by googleapis/proto-plus-python#267 2) builtins conflicts (`license_` vs `license` as field name) is expected to be fixed by a TBD PR * fix integration tests
1 parent c9cb6c7 commit 5cbe8b2

File tree

5 files changed

+74
-12
lines changed

5 files changed

+74
-12
lines changed

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,9 +925,21 @@ def query_params(self) -> Set[str]:
925925

926926
return set(self.input.fields) - params
927927

928+
@property
929+
def body_fields(self) -> Mapping[str, Field]:
930+
bindings = self.http_options
931+
if bindings and bindings[0].body and bindings[0].body != "*":
932+
return self._fields_mapping([bindings[0].body])
933+
return {}
934+
928935
# TODO(yon-mg): refactor as there may be more than one method signature
929936
@utils.cached_property
930937
def flattened_fields(self) -> Mapping[str, Field]:
938+
signatures = self.options.Extensions[client_pb2.method_signature]
939+
return self._fields_mapping(signatures)
940+
941+
# TODO(yon-mg): refactor as there may be more than one method signature
942+
def _fields_mapping(self, signatures) -> Mapping[str, Field]:
931943
"""Return the signature defined for this method."""
932944
cross_pkg_request = self.input.ident.package != self.ident.package
933945

@@ -946,7 +958,6 @@ def filter_fields(sig: str) -> Iterable[Tuple[str, Field]]:
946958

947959
yield name, field
948960

949-
signatures = self.options.Extensions[client_pb2.method_signature]
950961
answer: Dict[str, Field] = collections.OrderedDict(
951962
name_and_field
952963
for sig in signatures

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -306,9 +306,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
306306
client_cert_source_for_mtls=client_cert_source_func,
307307
quota_project_id=client_options.quota_project_id,
308308
client_info=client_info,
309-
{% if "grpc" in opts.transport %}
310309
always_use_jwt_access=True,
311-
{% endif %}
312310
)
313311

314312

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
1-
from google.auth.transport.requests import AuthorizedSession
1+
from google.auth.transport.requests import AuthorizedSession # type: ignore
22
import json # type: ignore
33
import grpc # type: ignore
4-
from google.auth.transport.grpc import SslCredentials # type: ignore
5-
from google.auth import credentials as ga_credentials # type: ignore
4+
from google.auth.transport.grpc import SslCredentials # type: ignore
5+
from google.auth import credentials as ga_credentials # type: ignore
66
from google.api_core import exceptions as core_exceptions # type: ignore
7-
from google.api_core import retry as retries # type: ignore
8-
from google.api_core import rest_helpers # type: ignore
9-
from google.api_core import path_template # type: ignore
10-
from google.api_core import gapic_v1 # type: ignore
7+
from google.api_core import retry as retries # type: ignore
8+
from google.api_core import rest_helpers # type: ignore
9+
from google.api_core import path_template # type: ignore
10+
from google.api_core import gapic_v1 # type: ignore
11+
{% if service.has_lro %}
1112
from google.api_core import operations_v1
13+
{% endif %}
1214
from requests import __version__ as requests_version
1315
from typing import Callable, Dict, Optional, Sequence, Tuple, Union
1416
import warnings

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,14 @@ def test_{{ method.name|snake_case }}_rest(transport: str = 'rest', request_type
11061106
)
11071107

11081108
# send a request that will satisfy transcoding
1109-
request = request_type({{ method.http_options[0].sample_request}})
1109+
request_init = {{ method.http_options[0].sample_request}}
1110+
{% for field in method.body_fields.values() %}
1111+
{% if not field.oneof or field.proto3_optional %}
1112+
{# ignore oneof fields that might conflict with sample_request #}
1113+
request_init["{{ field.name }}"] = {{ field.mock_value }}
1114+
{% endif %}
1115+
{% endfor %}
1116+
request = request_type(request_init)
11101117
{% if method.client_streaming %}
11111118
requests = [request]
11121119
{% endif %}
@@ -2419,6 +2426,7 @@ async def test_test_iam_permissions_from_dict_async():
24192426

24202427
{% endif %}
24212428

2429+
{% if 'grpc' in opts.transport %}
24222430
@pytest.mark.asyncio
24232431
async def test_transport_close_async():
24242432
client = {{ service.async_client_name }}(
@@ -2429,6 +2437,7 @@ async def test_transport_close_async():
24292437
async with client:
24302438
close.assert_not_called()
24312439
close.assert_called_once()
2440+
{% endif %}
24322441

24332442
def test_transport_close():
24342443
transports = {

packages/gapic-generator/tests/unit/schema/wrappers/test_method.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,35 @@ def test_method_path_params_no_http_rule():
330330
assert method.path_params == []
331331

332332

333+
def test_body_fields():
334+
http_rule = http_pb2.HttpRule(
335+
post='/v1/{arms_shape=arms/*}/squids',
336+
body='mantle'
337+
)
338+
339+
mantle_stuff = make_field(name='mantle_stuff', type=9)
340+
message = make_message('Mantle', fields=(mantle_stuff,))
341+
mantle = make_field('mantle', type=11, type_name='Mantle', message=message)
342+
arms_shape = make_field('arms_shape', type=9)
343+
input_message = make_message('Squid', fields=(mantle, arms_shape))
344+
method = make_method(
345+
'PutSquid', input_message=input_message, http_rule=http_rule)
346+
assert set(method.body_fields) == {'mantle'}
347+
mock_value = method.body_fields['mantle'].mock_value
348+
assert mock_value == "baz.Mantle(mantle_stuff='mantle_stuff_value')"
349+
350+
351+
def test_body_fields_no_body():
352+
http_rule = http_pb2.HttpRule(
353+
post='/v1/{arms_shape=arms/*}/squids',
354+
)
355+
356+
method = make_method(
357+
'PutSquid', http_rule=http_rule)
358+
359+
assert not method.body_fields
360+
361+
333362
def test_method_http_options():
334363
verbs = [
335364
'get',
@@ -363,7 +392,7 @@ def test_method_http_options_no_http_rule():
363392
assert method.path_params == []
364393

365394

366-
def test_method_http_options_body():
395+
def test_method_http_options_body_star():
367396
http_rule = http_pb2.HttpRule(
368397
post='/v1/{parent=projects/*}/topics',
369398
body='*'
@@ -376,6 +405,19 @@ def test_method_http_options_body():
376405
}]
377406

378407

408+
def test_method_http_options_body_field():
409+
http_rule = http_pb2.HttpRule(
410+
post='/v1/{parent=projects/*}/topics',
411+
body='body_field'
412+
)
413+
method = make_method('DoSomething', http_rule=http_rule)
414+
assert [dataclasses.asdict(http) for http in method.http_options] == [{
415+
'method': 'post',
416+
'uri': '/v1/{parent=projects/*}/topics',
417+
'body': 'body_field'
418+
}]
419+
420+
379421
def test_method_http_options_additional_bindings():
380422
http_rule = http_pb2.HttpRule(
381423
post='/v1/{parent=projects/*}/topics',

0 commit comments

Comments
 (0)