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

Open
hebaalazzeh wants to merge 5 commits into
base: main
Choose a base branch
from
Open

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
5 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
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
90 changes: 63 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,70 @@ 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, pure protobuf message, or mock/dict-like object).
pb_msg = getattr(request, "_pb", None)
is_proto_plus = pb_msg is not None and hasattr(pb_msg, "HasField")
if is_proto3_optional:
if is_proto_plus and pb_msg is not None:
# Case 2a: Proto-plus message with explicit presence.
# `proto.Message` instances wrap an underlying C++/Python protobuf message in `._pb`.
# We check `pb_msg.HasField(field_name)` to determine if the field was set by the user.
try:
should_populate = not pb_msg.HasField(field_name)
except ValueError:
# `HasField` raises ValueError if the field does not support presence (e.g., non-optional field).
# Fall back to checking if the attribute value is explicitly None.
should_populate = getattr(request, field_name, None) is None
else:
# Case 2b: Pure protobuf message or custom object with explicit presence.
try:
should_populate = not request.HasField(field_name)
except (AttributeError, ValueError):
# Fall back for objects/mocks that do not implement `HasField` or where `HasField` fails.
should_populate = getattr(request, field_name, None) is None
else:
# Case 2c: Object request without explicit presence (`is_proto3_optional=False`).
# Auto-populate if the field value is falsy (None or empty string '').
should_populate = not getattr(request, field_name, None)

# Consolidate mutation to a single, clean DRY block.
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
34 changes: 34 additions & 0 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 @@ -261,28 +261,55 @@ 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)

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


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)

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

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}"

@pytest.mark.parametrize(
"request_obj, is_proto3_optional, expected",
[
(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 +318,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
86 changes: 62 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,70 @@ 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, pure protobuf message, or mock/dict-like object).
pb_msg = getattr(request, "_pb", None)
is_proto_plus = pb_msg is not None and hasattr(pb_msg, "HasField")
if is_proto3_optional:
if is_proto_plus and pb_msg is not None:
# Case 2a: Proto-plus message with explicit presence.
# `proto.Message` instances wrap an underlying C++/Python protobuf message in `._pb`.
# We check `pb_msg.HasField(field_name)` to determine if the field was set by the user.
try:
should_populate = not pb_msg.HasField(field_name)
except ValueError:
# `HasField` raises ValueError if the field does not support presence (e.g., non-optional field).
# Fall back to checking if the attribute value is explicitly None.
should_populate = getattr(request, field_name, None) is None
else:
# Case 2b: Pure protobuf message or custom object with explicit presence.
try:
should_populate = not request.HasField(field_name)
except (AttributeError, ValueError):
# Fall back for objects/mocks that do not implement `HasField` or where `HasField` fails.
should_populate = getattr(request, field_name, None) is None
else:
# Case 2c: Object request without explicit presence (`is_proto3_optional=False`).
# Auto-populate if the field value is falsy (None or empty string '').
should_populate = not getattr(request, field_name, None)

# Consolidate mutation to a single, clean DRY block.
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
34 changes: 34 additions & 0 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 @@ -260,28 +260,55 @@ 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)

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


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)

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

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}"

@pytest.mark.parametrize(
"request_obj, is_proto3_optional, expected",
[
(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 +317,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