Skip to content

Commit

Permalink
feat: Make grpc transcode logic work in terms of protobuf python obje…
Browse files Browse the repository at this point in the history
…cts (#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 <partheniou@google.com>
  • Loading branch information
vam-google and parthea committed Aug 30, 2022
1 parent 7371d02 commit c3ad8ea
Show file tree
Hide file tree
Showing 2 changed files with 325 additions and 43 deletions.
60 changes: 40 additions & 20 deletions google/api_core/path_template.py
Expand Up @@ -176,18 +176,20 @@ def get_field(request, field):
"""Get the value of a field from a given dictionary.
Args:
request (dict): A dictionary object.
request (dict | Message): A dictionary or a Message object.
field (str): The key to the request in dot notation.
Returns:
The value of the field.
"""
parts = field.split(".")
value = request

for part in parts:
if not isinstance(value, dict):
return
value = value.get(part)
value = getattr(value, part, None)
else:
value = value.get(part)
if isinstance(value, dict):
return
return value
Expand All @@ -197,19 +199,27 @@ def delete_field(request, field):
"""Delete the value of a field from a given dictionary.
Args:
request (dict): A dictionary object.
request (dict | Message): A dictionary object or a Message.
field (str): The key to the request in dot notation.
"""
parts = deque(field.split("."))
while len(parts) > 1:
if not isinstance(request, dict):
return
part = parts.popleft()
request = request.get(part)
if not isinstance(request, dict):
if hasattr(request, part):
request = getattr(request, part, None)
else:
return
else:
request = request.get(part)
part = parts.popleft()
if not isinstance(request, dict):
return
request.pop(part, None)
if hasattr(request, part):
request.ClearField(part)
else:
return
else:
request.pop(part, None)


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


def transcode(http_options, **request_kwargs):
def transcode(http_options, message=None, **request_kwargs):
"""Transcodes a grpc request pattern into a proper HTTP request following the rules outlined here,
https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L44-L312
Expand All @@ -248,18 +258,20 @@ def transcode(http_options, **request_kwargs):
'body' (str): The body field name (optional)
(This is a simplified representation of the proto option `google.api.http`)
message (Message) : A request object (optional)
request_kwargs (dict) : A dict representing the request object
Returns:
dict: The transcoded request with these keys,
'method' (str) : The http method
'uri' (str) : The expanded uri
'body' (dict) : A dict representing the body (optional)
'query_params' (dict) : A dict mapping query parameter variables and values
'body' (dict | Message) : A dict or a Message representing the body (optional)
'query_params' (dict | Message) : A dict or Message mapping query parameter variables and values
Raises:
ValueError: If the request does not match the given template.
"""
transcoded_value = message or request_kwargs
for http_option in http_options:
request = {}

Expand All @@ -268,27 +280,35 @@ def transcode(http_options, **request_kwargs):
path_fields = [
match.group("name") for match in _VARIABLE_RE.finditer(uri_template)
]
path_args = {field: get_field(request_kwargs, field) for field in path_fields}
path_args = {field: get_field(transcoded_value, field) for field in path_fields}
request["uri"] = expand(uri_template, **path_args)
# Remove fields used in uri path from request
leftovers = copy.deepcopy(request_kwargs)
for path_field in path_fields:
delete_field(leftovers, path_field)

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

# Remove fields used in uri path from request
leftovers = copy.deepcopy(transcoded_value)
for path_field in path_fields:
delete_field(leftovers, path_field)

# Assign body and query params
body = http_option.get("body")

if body:
if body == "*":
request["body"] = leftovers
request["query_params"] = {}
if message:
request["query_params"] = message.__class__()
else:
request["query_params"] = {}
else:
try:
request["body"] = leftovers.pop(body)
except KeyError:
if message:
request["body"] = getattr(leftovers, body)
delete_field(leftovers, body)
else:
request["body"] = leftovers.pop(body)
except (KeyError, AttributeError):
continue
request["query_params"] = leftovers
else:
Expand Down

0 comments on commit c3ad8ea

Please sign in to comment.