Skip to content

Commit c3ad8ea

Browse files
vam-googleparthea
andauthored
feat: Make grpc transcode logic work in terms of protobuf python objects (#428)
* feat: Make grpc transcode logic work in terms of protobuf python objects (for context: [gRPC Transcoding](https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L44)) Previously it worked on dictionaries only, but that causes problems. In GAPIC the dictionaries are created through the same logic as JSON (there is no better built-in way), thus applying [protobuf json mapping](https://developers.google.com/protocol-buffers/docs/proto3#json) conversion logic in the process. Unfortunately converting a protobuf object to a dictionary and to JSON, although similar, are not the same thing. Specifically the `Timestamp`, `Duration`, `FieldMask`, `uint64`, `int64`, and `*Value` protobuf messages are converted to strings for JSON (instead of being properly converted to dicts for most of those types, and `int64/uint64` converted to `int` respectively). As a result a rountrip that GAPIC was relying on (protobuf object -> dict -> transcode -> protobuf object) did not work properly. For example, when converted to dictionary, every int64 field would be converted to `string` (because it is what proto-JSON mapping spec requires), but later, when we need to rebuild a message from a transcoded dictionary that would fail with the following error: ``` TypeError: '0' has type str, but expected one of: int ``` Note, `*Rules` thing from proto-plus does not help, becuase the type may happen inside common native protobuf stub messsages (like `google.type.Money`), fields of which are outside of scope of the proto-plus custom conversion logic. Also, this change greatly simplifies the procedure of transcodding, eliminating multiple conversion steps (to and from dictionaries multiple times) making the whole logic significanly more efficient (python gapics are nutoriously known to be slow due to proto-plus stuff, so efficiency is important) and robust (JSON conversion logic does not interfere anymore with pure protobuf objects grpc transcoding) * reformat code using black * reformat code according to flake8 Co-authored-by: Anthonios Partheniou <[email protected]>
1 parent 7371d02 commit c3ad8ea

File tree

2 files changed

+325
-43
lines changed

2 files changed

+325
-43
lines changed

google/api_core/path_template.py

+40-20
Original file line numberDiff line numberDiff line change
@@ -176,18 +176,20 @@ def get_field(request, field):
176176
"""Get the value of a field from a given dictionary.
177177
178178
Args:
179-
request (dict): A dictionary object.
179+
request (dict | Message): A dictionary or a Message object.
180180
field (str): The key to the request in dot notation.
181181
182182
Returns:
183183
The value of the field.
184184
"""
185185
parts = field.split(".")
186186
value = request
187+
187188
for part in parts:
188189
if not isinstance(value, dict):
189-
return
190-
value = value.get(part)
190+
value = getattr(value, part, None)
191+
else:
192+
value = value.get(part)
191193
if isinstance(value, dict):
192194
return
193195
return value
@@ -197,19 +199,27 @@ def delete_field(request, field):
197199
"""Delete the value of a field from a given dictionary.
198200
199201
Args:
200-
request (dict): A dictionary object.
202+
request (dict | Message): A dictionary object or a Message.
201203
field (str): The key to the request in dot notation.
202204
"""
203205
parts = deque(field.split("."))
204206
while len(parts) > 1:
205-
if not isinstance(request, dict):
206-
return
207207
part = parts.popleft()
208-
request = request.get(part)
208+
if not isinstance(request, dict):
209+
if hasattr(request, part):
210+
request = getattr(request, part, None)
211+
else:
212+
return
213+
else:
214+
request = request.get(part)
209215
part = parts.popleft()
210216
if not isinstance(request, dict):
211-
return
212-
request.pop(part, None)
217+
if hasattr(request, part):
218+
request.ClearField(part)
219+
else:
220+
return
221+
else:
222+
request.pop(part, None)
213223

214224

215225
def validate(tmpl, path):
@@ -237,7 +247,7 @@ def validate(tmpl, path):
237247
return True if re.match(pattern, path) is not None else False
238248

239249

240-
def transcode(http_options, **request_kwargs):
250+
def transcode(http_options, message=None, **request_kwargs):
241251
"""Transcodes a grpc request pattern into a proper HTTP request following the rules outlined here,
242252
https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L44-L312
243253
@@ -248,18 +258,20 @@ def transcode(http_options, **request_kwargs):
248258
'body' (str): The body field name (optional)
249259
(This is a simplified representation of the proto option `google.api.http`)
250260
261+
message (Message) : A request object (optional)
251262
request_kwargs (dict) : A dict representing the request object
252263
253264
Returns:
254265
dict: The transcoded request with these keys,
255266
'method' (str) : The http method
256267
'uri' (str) : The expanded uri
257-
'body' (dict) : A dict representing the body (optional)
258-
'query_params' (dict) : A dict mapping query parameter variables and values
268+
'body' (dict | Message) : A dict or a Message representing the body (optional)
269+
'query_params' (dict | Message) : A dict or Message mapping query parameter variables and values
259270
260271
Raises:
261272
ValueError: If the request does not match the given template.
262273
"""
274+
transcoded_value = message or request_kwargs
263275
for http_option in http_options:
264276
request = {}
265277

@@ -268,27 +280,35 @@ def transcode(http_options, **request_kwargs):
268280
path_fields = [
269281
match.group("name") for match in _VARIABLE_RE.finditer(uri_template)
270282
]
271-
path_args = {field: get_field(request_kwargs, field) for field in path_fields}
283+
path_args = {field: get_field(transcoded_value, field) for field in path_fields}
272284
request["uri"] = expand(uri_template, **path_args)
273-
# Remove fields used in uri path from request
274-
leftovers = copy.deepcopy(request_kwargs)
275-
for path_field in path_fields:
276-
delete_field(leftovers, path_field)
277285

278286
if not validate(uri_template, request["uri"]) or not all(path_args.values()):
279287
continue
280288

289+
# Remove fields used in uri path from request
290+
leftovers = copy.deepcopy(transcoded_value)
291+
for path_field in path_fields:
292+
delete_field(leftovers, path_field)
293+
281294
# Assign body and query params
282295
body = http_option.get("body")
283296

284297
if body:
285298
if body == "*":
286299
request["body"] = leftovers
287-
request["query_params"] = {}
300+
if message:
301+
request["query_params"] = message.__class__()
302+
else:
303+
request["query_params"] = {}
288304
else:
289305
try:
290-
request["body"] = leftovers.pop(body)
291-
except KeyError:
306+
if message:
307+
request["body"] = getattr(leftovers, body)
308+
delete_field(leftovers, body)
309+
else:
310+
request["body"] = leftovers.pop(body)
311+
except (KeyError, AttributeError):
292312
continue
293313
request["query_params"] = leftovers
294314
else:

0 commit comments

Comments
 (0)