Skip to content

fix(api-core): use truthiness check in setup_request_id to support proto-plus messages #18000

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

Merged
hebaalazzeh merged 7 commits into from
Aug 6, 2026
Merged

fix(api-core): use truthiness check in setup_request_id to support proto-plus messages #18000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6288aa3
fix(api-core): use truthiness check in setup_request_id to support pr…
hebaalazzeh Aug 5, 2026
1891447
update goldens & inspect proto-plus underlying protobuf for request_i…
hebaalazzeh Aug 5, 2026
7daaa19
update goldens
hebaalazzeh Aug 5, 2026
6e9daa5
update goldens
hebaalazzeh Aug 5, 2026
c6bae7b
remove unsued import
hebaalazzeh Aug 5, 2026
526ff10
update goldens
hebaalazzeh Aug 5, 2026
9a52682
update comments
hebaalazzeh Aug 6, 2026
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
78 changes: 51 additions & 27 deletions packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ import os
import json
{% if has_auto_populated_fields %}
import uuid
import google.protobuf.message
{% endif %}

from typing import Any, Dict, List, Optional, Tuple
{% if has_auto_populated_fields %}
from typing import Union
{% endif %}

{% if has_auto_populated_fields %}
import google.protobuf.message
from typing import TYPE_CHECKING, Union

if TYPE_CHECKING: # pragma: NO COVER
import proto # type: ignore[import-untyped]
{% endif %}

from google.api_core import path_template
from google.api_core.universe import EmptyUniverseError
from google.auth.exceptions import MutualTLSChannelError
Expand Down Expand Up @@ -170,7 +170,7 @@ def get_universe_domain(
{% if has_auto_populated_fields %}

def setup_request_id(
request: Union[google.protobuf.message.Message, dict, None],
request: Union[google.protobuf.message.Message, "proto.Message", dict, None],
field_name: str,
is_proto3_optional: bool,
) -> None:
Expand All @@ -184,34 +184,58 @@ def setup_request_id(
resources).

Args:
request (Union[google.protobuf.message.Message, dict]): The
request object.
field_name (str): The name of the field to populate.
is_proto3_optional (bool): Whether the field is proto3 optional.
request (Union[google.protobuf.message.Message, proto.Message, dict, None]): The
request object or dictionary.
field_name (str): The name of the field to populate (e.g., "request_id").
is_proto3_optional (bool): Whether the field supports explicit presence
(defined with `optional` in proto3 syntax). When True, empty strings ("")
are preserved as explicit user input per AIP-4235, and UUID auto-population
occurs only if the field is unset. When False, any empty or falsy value is
populated with a UUID.
"""
if request is None:
return

# Evaluate whether the field is considered "unset" and needs auto-population.
#
# According to AIP-4235, optional request ID fields must be populated
# if and only if they have explicit presence (`is_proto3_optional=True`)
# and were not set by the user (i.e. unset). Explicitly provided empty
# strings ('') must be preserved when `is_proto3_optional=True`.
should_populate = False
if isinstance(request, dict):

@daniel-sanche daniel-sanche Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more comments would be helpful here. There are a lot of nested cases, it's hard to follow

Maybe this should even be broken into multiple helpers

@hebaalazzeh hebaalazzeh Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added more comments!

if is_proto3_optional:
if field_name not in request or request[field_name] is None:
request[field_name] = str(uuid.uuid4())
elif not request.get(field_name):
request[field_name] = str(uuid.uuid4())
return

if is_proto3_optional:
try:
# Pure protobuf messages
if not request.HasField(field_name):
setattr(request, field_name, str(uuid.uuid4()))
except (AttributeError, ValueError):
# Proto-plus messages or other objects
if not getattr(request, field_name, None):
setattr(request, field_name, str(uuid.uuid4()))
# Case 1a: Dictionary request with explicit presence (`is_proto3_optional=True`).
# Per AIP-4235, auto-populate only if the key is completely missing from
# the dictionary or its value is explicitly set to None.
# An explicit empty string ('') must NOT be overwritten.
should_populate = field_name not in request or request[field_name] is None
else:
# Case 1b: Dictionary request without explicit presence (`is_proto3_optional=False`).
# Auto-populate if the key is missing, None, or falsy (e.g., empty string '').
should_populate = not request.get(field_name)
else:
if not getattr(request, field_name, None):
setattr(request, field_name, str(uuid.uuid4()))
# Case 2: Object request (proto-plus wrapper or pure protobuf message).
if is_proto3_optional:
# Extract the protobuf from proto-plus if wrapped.
pure_pb: google.protobuf.message.Message = getattr(request, "_pb", request)
try:
should_populate = not pure_pb.HasField(field_name)
except (AttributeError, ValueError):
# Fall back if `HasField` fails or is unsupported.
should_populate = getattr(pure_pb, field_name, None) is None
else:
# Case 2b: Object request without explicit presence (`is_proto3_optional=False`).
# Auto-populate if the field value is falsy (None or empty string '').
should_populate = not bool(getattr(request, field_name, False))

# If the field was found to be empty, set random id
if should_populate:
generated_id = str(uuid.uuid4())
if isinstance(request, dict):
request[field_name] = generated_id
else:
setattr(request, field_name, generated_id)

{% endif %}

Expand Down
32 changes: 27 additions & 5 deletions ...es/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,6 @@ class MockRequest:
for k, v in kwargs.items():
setattr(self, k, v)

def __contains__(self, key):
return hasattr(self, key)


class MockProtoRequest:
def __init__(self, **kwargs):
Expand All @@ -261,12 +258,23 @@ class MockProtoRequest:
return hasattr(self, key)


class MockProtoPlusRequest:
def __init__(self, **kwargs):
self._pb = MockProtoRequest(**kwargs)
for k, v in kwargs.items():
setattr(self, k, v)


class MockValueErrorRequest:
def HasField(self, key):
raise ValueError("Mismatched field")

def __contains__(self, key):
return hasattr(self, key)

class MockProtoPlusValueErrorRequest:
def __init__(self, **kwargs):
self._pb = MockValueErrorRequest()
for k, v in kwargs.items():
setattr(self, k, v)

UUID_REGEX = r"[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}"

Expand All @@ -275,14 +283,21 @@ UUID_REGEX = r"[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{1
[
(MockRequest(), True, "uuid"),
(MockRequest(request_id="already_set"), True, "already_set"),
(MockRequest(request_id=""), True, ""),
(MockRequest(request_id=""), False, "uuid"),
(MockRequest(request_id="already_set"), False, "already_set"),
(MockProtoRequest(), True, "uuid"),
(MockProtoRequest(request_id="already_set"), True, "already_set"),
(MockProtoRequest(request_id=""), True, ""),
(MockProtoPlusRequest(), True, "uuid"),
(MockProtoPlusRequest(request_id="already_set"), True, "already_set"),
(MockProtoPlusRequest(request_id=""), True, ""),
(MockValueErrorRequest(), True, "uuid"),
(MockProtoPlusValueErrorRequest(), True, "uuid"),
({}, True, "uuid"),
({"request_id": None}, True, "uuid"),
({"request_id": "already_set"}, True, "already_set"),
({"request_id": ""}, True, ""),
({"request_id": ""}, False, "uuid"),
({"request_id": None}, False, "uuid"),
({"request_id": "already_set"}, False, "already_set"),
Expand All @@ -291,14 +306,21 @@ UUID_REGEX = r"[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{1
ids=[
"proto3_optional_not_in_request",
"proto3_optional_already_in_request",
"proto3_optional_explicit_empty",
"non_proto3_optional_empty",
"non_proto3_optional_already_set",
"proto3_optional_not_in_request_proto",
"proto3_optional_already_in_request_proto",
"proto3_optional_explicit_empty_proto",
"proto3_optional_not_in_request_proto_plus",
"proto3_optional_already_in_request_proto_plus",
"proto3_optional_explicit_empty_proto_plus",
"value_error_fallback",
"proto3_optional_value_error_fallback_proto_plus",
"dict_proto3_optional_not_in_request",
"dict_proto3_optional_value_none",
"dict_proto3_optional_already_in_request",
"dict_proto3_optional_explicit_empty",
"dict_non_proto3_optional_empty",
"dict_non_proto3_optional_value_none",
"dict_non_proto3_optional_already_set",
Expand Down
74 changes: 50 additions & 24 deletions ...egration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
import os
import json
import uuid
import google.protobuf.message

from typing import Any, Dict, List, Optional, Tuple
from typing import Union
from typing import TYPE_CHECKING, Union

import google.protobuf.message
if TYPE_CHECKING: # pragma: NO COVER
import proto # type: ignore[import-untyped]

from google.api_core import path_template
from google.api_core.universe import EmptyUniverseError
Expand Down Expand Up @@ -165,7 +167,7 @@ def get_universe_domain(


def setup_request_id(
request: Union[google.protobuf.message.Message, dict, None],
request: Union[google.protobuf.message.Message, "proto.Message", dict, None],
field_name: str,
is_proto3_optional: bool,
) -> None:
Expand All @@ -179,34 +181,58 @@ def setup_request_id(
resources).

Args:
request (Union[google.protobuf.message.Message, dict]): The
request object.
field_name (str): The name of the field to populate.
is_proto3_optional (bool): Whether the field is proto3 optional.
request (Union[google.protobuf.message.Message, proto.Message, dict, None]): The
request object or dictionary.
field_name (str): The name of the field to populate (e.g., "request_id").
is_proto3_optional (bool): Whether the field supports explicit presence
(defined with `optional` in proto3 syntax). When True, empty strings ("")
are preserved as explicit user input per AIP-4235, and UUID auto-population
occurs only if the field is unset. When False, any empty or falsy value is
populated with a UUID.
"""
if request is None:
return

# Evaluate whether the field is considered "unset" and needs auto-population.
#
# According to AIP-4235, optional request ID fields must be populated
# if and only if they have explicit presence (`is_proto3_optional=True`)
# and were not set by the user (i.e. unset). Explicitly provided empty
# strings ('') must be preserved when `is_proto3_optional=True`.
should_populate = False
if isinstance(request, dict):
if is_proto3_optional:
if field_name not in request or request[field_name] is None:
request[field_name] = str(uuid.uuid4())
elif not request.get(field_name):
request[field_name] = str(uuid.uuid4())
return

if is_proto3_optional:
try:
# Pure protobuf messages
if not request.HasField(field_name):
setattr(request, field_name, str(uuid.uuid4()))
except (AttributeError, ValueError):
# Proto-plus messages or other objects
if not getattr(request, field_name, None):
setattr(request, field_name, str(uuid.uuid4()))
# Case 1a: Dictionary request with explicit presence (`is_proto3_optional=True`).
# Per AIP-4235, auto-populate only if the key is completely missing from
# the dictionary or its value is explicitly set to None.
# An explicit empty string ('') must NOT be overwritten.
should_populate = field_name not in request or request[field_name] is None
else:
# Case 1b: Dictionary request without explicit presence (`is_proto3_optional=False`).
# Auto-populate if the key is missing, None, or falsy (e.g., empty string '').
should_populate = not request.get(field_name)
else:
if not getattr(request, field_name, None):
setattr(request, field_name, str(uuid.uuid4()))
# Case 2: Object request (proto-plus wrapper or pure protobuf message).
if is_proto3_optional:
# Extract the protobuf from proto-plus if wrapped.
pure_pb: google.protobuf.message.Message = getattr(request, "_pb", request)
try:
should_populate = not pure_pb.HasField(field_name)
except (AttributeError, ValueError):
# Fall back if `HasField` fails or is unsupported.
should_populate = getattr(pure_pb, field_name, None) is None
else:
# Case 2b: Object request without explicit presence (`is_proto3_optional=False`).
# Auto-populate if the field value is falsy (None or empty string '').
should_populate = not bool(getattr(request, field_name, False))

# If the field was found to be empty, set random id
if should_populate:
generated_id = str(uuid.uuid4())
if isinstance(request, dict):
request[field_name] = generated_id
else:
setattr(request, field_name, generated_id)


def transcode_request(
Expand Down
32 changes: 27 additions & 5 deletions .../goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,9 +247,6 @@ def __init__(self, **kwargs):
for k, v in kwargs.items():
setattr(self, k, v)

def __contains__(self, key):
return hasattr(self, key)


class MockProtoRequest:
def __init__(self, **kwargs):
Expand All @@ -260,12 +257,23 @@ def HasField(self, key):
return hasattr(self, key)


class MockProtoPlusRequest:
def __init__(self, **kwargs):
self._pb = MockProtoRequest(**kwargs)
for k, v in kwargs.items():
setattr(self, k, v)


class MockValueErrorRequest:
def HasField(self, key):
raise ValueError("Mismatched field")

def __contains__(self, key):
return hasattr(self, key)

class MockProtoPlusValueErrorRequest:
def __init__(self, **kwargs):
self._pb = MockValueErrorRequest()
for k, v in kwargs.items():
setattr(self, k, v)

UUID_REGEX = r"[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}"

Expand All @@ -274,14 +282,21 @@ def __contains__(self, key):
[
(MockRequest(), True, "uuid"),
(MockRequest(request_id="already_set"), True, "already_set"),
(MockRequest(request_id=""), True, ""),
(MockRequest(request_id=""), False, "uuid"),
(MockRequest(request_id="already_set"), False, "already_set"),
(MockProtoRequest(), True, "uuid"),
(MockProtoRequest(request_id="already_set"), True, "already_set"),
(MockProtoRequest(request_id=""), True, ""),
(MockProtoPlusRequest(), True, "uuid"),
(MockProtoPlusRequest(request_id="already_set"), True, "already_set"),
(MockProtoPlusRequest(request_id=""), True, ""),
(MockValueErrorRequest(), True, "uuid"),
(MockProtoPlusValueErrorRequest(), True, "uuid"),
({}, True, "uuid"),
({"request_id": None}, True, "uuid"),
({"request_id": "already_set"}, True, "already_set"),
({"request_id": ""}, True, ""),
({"request_id": ""}, False, "uuid"),
({"request_id": None}, False, "uuid"),
({"request_id": "already_set"}, False, "already_set"),
Expand All @@ -290,14 +305,21 @@ def __contains__(self, key):
ids=[
"proto3_optional_not_in_request",
"proto3_optional_already_in_request",
"proto3_optional_explicit_empty",
"non_proto3_optional_empty",
"non_proto3_optional_already_set",
"proto3_optional_not_in_request_proto",
"proto3_optional_already_in_request_proto",
"proto3_optional_explicit_empty_proto",
"proto3_optional_not_in_request_proto_plus",
"proto3_optional_already_in_request_proto_plus",
"proto3_optional_explicit_empty_proto_plus",
"value_error_fallback",
"proto3_optional_value_error_fallback_proto_plus",
"dict_proto3_optional_not_in_request",
"dict_proto3_optional_value_none",
"dict_proto3_optional_already_in_request",
"dict_proto3_optional_explicit_empty",
"dict_non_proto3_optional_empty",
"dict_non_proto3_optional_value_none",
"dict_non_proto3_optional_already_set",
Expand Down
Loading
Loading