Skip to content

chore: refactor setting of auto-populated field in request #17004

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

Draft
parthea wants to merge 4 commits into
base: repro-508597813
Choose a base branch
from
Draft

chore: refactor setting of auto-populated field in request #17004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0223a0d
chore: refactor setting of auto-populated field in request
parthea May 8, 2026
41536e7
address feedback
parthea May 8, 2026
e29016f
wip
parthea May 8, 2026
fbd8387
wip
parthea May 8, 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
10 changes: 5 additions & 5 deletions ...erator/gapic/templates/%namespace/%name_%version/%sub/services/%service/_shared_macros.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
#}

{% macro auto_populate_uuid4_fields(api, method) %}
{% macro auto_populate_uuid4_fields(api, method, is_async=False) %}
{#
Automatically populate UUID4 fields according to
https://google.aip.dev/client-libraries/4235 when the
Expand All @@ -27,12 +27,12 @@
{% with method_settings = api.all_method_settings.get(method.meta.address.proto) %}
{% if method_settings is not none %}
{% for auto_populated_field in method_settings.auto_populated_fields %}
{% if method.input.fields[auto_populated_field].proto3_optional %}
if '{{ auto_populated_field }}' not in request:
{% set is_proto3_optional = method.input.fields[auto_populated_field].proto3_optional %}
{% if is_async %}
self._client._setup_request_id(request, '{{ auto_populated_field }}', {{ is_proto3_optional }})
{% else %}
if not request.{{ auto_populated_field }}:
self._setup_request_id(request, '{{ auto_populated_field }}', {{ is_proto3_optional }})
{% endif %}
request.{{ auto_populated_field }} = str(uuid.uuid4())
{% endfor %}
{% endif %}{# if method_settings is not none #}
{% endwith %}{# method_settings #}
Expand Down
2 changes: 1 addition & 1 deletion ...rator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ class {{ service.async_client_name }}:

{{ shared_macros.create_metadata(method) }}
{{ shared_macros.add_api_version_header_to_metadata(service.version) }}
{{ shared_macros.auto_populate_uuid4_fields(api, method) }}
{{ shared_macros.auto_populate_uuid4_fields(api, method, is_async=True) }}

# Validate the universe domain.
self._client._validate_universe_domain()
Expand Down
32 changes: 32 additions & 0 deletions ...c-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,38 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
# NOTE (b/349488459): universe validation is disabled until further notice.
return True

{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
@staticmethod
def _setup_request_id(request, field_name: str, is_proto3_optional: bool):
"""Populate a UUID4 field in the request if it is not already set.

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.
"""
if isinstance(request, dict):
if is_proto3_optional:
if field_name not in request:
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 field_name not in request:
setattr(request, field_name, str(uuid.uuid4()))
else:
if not getattr(request, field_name):
setattr(request, field_name, str(uuid.uuid4()))
Comment on lines +474 to +485
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

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

high

The implementation of _setup_request_id has two significant issues:

  1. Correctness (Dict support): The docstring states that request can be a dict, but setattr and getattr will raise an AttributeError when called on a dictionary.
  2. Correctness (Pure Protobuf support): For pure protobuf messages (non-proto-plus), the check field_name not in request will raise a TypeError because protobuf messages are not iterable.

To support both dictionary and message types (including pure protobuf and proto-plus), the logic should be more robust.

        if isinstance(request, dict):
            if is_proto3_optional:
                if field_name not in request:
                    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 field_name not in request:
                    setattr(request, field_name, str(uuid.uuid4()))
        else:
            if not getattr(request, field_name):
                setattr(request, field_name, str(uuid.uuid4()))

{% endif %}

def _add_cred_info_for_auth_errors(
self,
error: core_exceptions.GoogleAPICallError
Expand Down
50 changes: 50 additions & 0 deletions .../gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,56 @@ def test__add_cred_info_for_auth_errors_no_get_cred_info(error_code):
client._add_cred_info_for_auth_errors(error)
assert error.details == []

{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
def test__setup_request_id():
class MockRequest:
def __init__(self, **kwargs):
for k, v in kwargs.items():
setattr(self, k, v)
def __contains__(self, key):
return hasattr(self, key)

# Test with proto3 optional field not in request
request = MockRequest()
{{ service.client_name }}._setup_request_id(request, "request_id", True)
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request.request_id)

# Test with proto3 optional field already in request
request = MockRequest(request_id="already_set")
{{ service.client_name }}._setup_request_id(request, "request_id", True)
assert request.request_id == "already_set"

# Test with non-proto3 optional field empty
request = MockRequest(request_id="")
{{ service.client_name }}._setup_request_id(request, "request_id", False)
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request.request_id)

# Test with non-proto3 optional field already set
request = MockRequest(request_id="already_set")
{{ service.client_name }}._setup_request_id(request, "request_id", False)
assert request.request_id == "already_set"

# Test with dict and proto3 optional field not in request
request = {}
{{ service.client_name }}._setup_request_id(request, "request_id", True)
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request["request_id"])

# Test with dict and proto3 optional field already in request
request = {"request_id": "already_set"}
{{ service.client_name }}._setup_request_id(request, "request_id", True)
assert request["request_id"] == "already_set"

# Test with dict and non-proto3 optional field empty
request = {"request_id": ""}
{{ service.client_name }}._setup_request_id(request, "request_id", False)
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request["request_id"])

# Test with dict and non-proto3 optional field already set
request = {"request_id": "already_set"}
{{ service.client_name }}._setup_request_id(request, "request_id", False)
assert request["request_id"] == "already_set"
{% endif %}

@pytest.mark.parametrize("client_class,transport_name", [
{% if 'grpc' in opts.transport %}
({{ service.client_name }}, "grpc"),
Expand Down
9 changes: 3 additions & 6 deletions .../google/cloud/storagebatchoperations_v1/services/storage_batch_operations/async_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,7 @@ async def sample_create_job():
)),
)

if not request.request_id:
request.request_id = str(uuid.uuid4())
self._client._setup_request_id(request, 'request_id', False)

# Validate the universe domain.
self._client._validate_universe_domain()
Expand Down Expand Up @@ -728,8 +727,7 @@ async def sample_delete_job():
)),
)

if not request.request_id:
request.request_id = str(uuid.uuid4())
self._client._setup_request_id(request, 'request_id', False)

# Validate the universe domain.
self._client._validate_universe_domain()
Expand Down Expand Up @@ -831,8 +829,7 @@ async def sample_cancel_job():
)),
)

if not request.request_id:
request.request_id = str(uuid.uuid4())
self._client._setup_request_id(request, 'request_id', False)

# Validate the universe domain.
self._client._validate_universe_domain()
Expand Down
25 changes: 19 additions & 6 deletions ...ations/google/cloud/storagebatchoperations_v1/services/storage_batch_operations/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,22 @@ def _validate_universe_domain(self):
# NOTE (b/349488459): universe validation is disabled until further notice.
return True

@staticmethod
def _setup_request_id(request, field_name: str, is_proto3_optional: bool):
"""Populate a UUID4 field in the request if it is not already set.

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.
"""
if is_proto3_optional:
if field_name not in request:
setattr(request, field_name, str(uuid.uuid4()))
else:
if not getattr(request, field_name):
setattr(request, field_name, str(uuid.uuid4()))

def _add_cred_info_for_auth_errors(
self,
error: core_exceptions.GoogleAPICallError
Expand Down Expand Up @@ -1005,8 +1021,7 @@ def sample_create_job():
)),
)

if not request.request_id:
request.request_id = str(uuid.uuid4())
self._setup_request_id(request, 'request_id', False)

# Validate the universe domain.
self._validate_universe_domain()
Expand Down Expand Up @@ -1111,8 +1126,7 @@ def sample_delete_job():
)),
)

if not request.request_id:
request.request_id = str(uuid.uuid4())
self._setup_request_id(request, 'request_id', False)

# Validate the universe domain.
self._validate_universe_domain()
Expand Down Expand Up @@ -1213,8 +1227,7 @@ def sample_cancel_job():
)),
)

if not request.request_id:
request.request_id = str(uuid.uuid4())
self._setup_request_id(request, 'request_id', False)

# Validate the universe domain.
self._validate_universe_domain()
Expand Down
Loading