From 6288aa3fbbeeed14fdaa97bf62bee0953943ad53 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 5 Aug 2026 04:50:30 +0000 Subject: [PATCH 1/7] fix(api-core): use truthiness check in setup_request_id to support proto-plus messages --- packages/google-api-core/google/api_core/gapic_v1/requests.py | 2 +- packages/google-api-core/tests/unit/gapic/test_requests.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index 76f5e916716d..8ce97c9cffa7 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -65,7 +65,7 @@ def setup_request_id( setattr(request, field_name, str(uuid.uuid4())) except (AttributeError, ValueError): # Proto-plus messages or other objects - if getattr(request, field_name, None) is None: + if not getattr(request, field_name, None): setattr(request, field_name, str(uuid.uuid4())) else: if not getattr(request, field_name, None): diff --git a/packages/google-api-core/tests/unit/gapic/test_requests.py b/packages/google-api-core/tests/unit/gapic/test_requests.py index a69bff5e16d7..71c7ce6d5225 100644 --- a/packages/google-api-core/tests/unit/gapic/test_requests.py +++ b/packages/google-api-core/tests/unit/gapic/test_requests.py @@ -53,7 +53,6 @@ def HasField(self, key): # MockRequest cases (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 cases @@ -65,7 +64,6 @@ def HasField(self, key): ({}, 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"), @@ -75,7 +73,6 @@ def HasField(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", @@ -84,7 +81,6 @@ def HasField(self, key): "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", From 18914477715fec2ca138602a82e2e2fb6323e001 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 5 Aug 2026 18:11:41 +0000 Subject: [PATCH 2/7] update goldens & inspect proto-plus underlying protobuf for request_id auto-population --- .../%name_%version/%sub/_compat.py.j2 | 50 ++++++++++--------- .../%name_%version/%sub/test_compat.py.j2 | 34 +++++++++++++ .../storagebatchoperations_v1/_compat.py | 48 ++++++++++-------- .../storagebatchoperations_v1/test_compat.py | 34 +++++++++++++ .../google/api_core/gapic_v1/requests.py | 50 +++++++++++-------- .../tests/unit/gapic/test_requests.py | 29 +++++++++++ 6 files changed, 179 insertions(+), 66 deletions(-) diff --git a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 index fba8615921af..0b5fab8360d8 100644 --- a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 +++ b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 @@ -24,10 +24,6 @@ from typing import Any, Dict, List, Optional, Tuple from typing import Union {% endif %} -{% if has_auto_populated_fields %} -import google.protobuf.message - -{% endif %} from google.api_core import path_template from google.api_core.universe import EmptyUniverseError from google.auth.exceptions import MutualTLSChannelError @@ -170,7 +166,7 @@ def get_universe_domain( {% if has_auto_populated_fields %} def setup_request_id( - request: Union[google.protobuf.message.Message, dict, None], + request: Union[Any, dict, None], field_name: str, is_proto3_optional: bool, ) -> None: @@ -184,7 +180,7 @@ def setup_request_id( resources). Args: - request (Union[google.protobuf.message.Message, dict]): The + request (Union[Any, dict, None]): The request object. field_name (str): The name of the field to populate. is_proto3_optional (bool): Whether the field is proto3 optional. @@ -192,26 +188,34 @@ def setup_request_id( if request is None: return + 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())) + should_populate = field_name not in request or request[field_name] is None + else: + should_populate = not request.get(field_name) else: - if not getattr(request, field_name, None): - setattr(request, field_name, str(uuid.uuid4())) + is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") + if is_proto3_optional: + if is_proto_plus: + try: + should_populate = not request._pb.HasField(field_name) + except ValueError: + should_populate = getattr(request, field_name, None) is None + else: + try: + should_populate = not request.HasField(field_name) + except (AttributeError, ValueError): + should_populate = getattr(request, field_name, None) is None + else: + should_populate = not getattr(request, field_name, None) + + 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 %} diff --git a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 index 8a923aeeef83..189502640377 100644 --- a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 +++ b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 @@ -261,6 +261,16 @@ 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") @@ -268,6 +278,16 @@ class MockValueErrorRequest: 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( @@ -275,14 +295,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"), @@ -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", diff --git a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py index ffa58f4f04eb..0df7c0d7859c 100755 --- a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py +++ b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py @@ -22,8 +22,6 @@ from typing import Any, Dict, List, Optional, Tuple from typing import Union -import google.protobuf.message - from google.api_core import path_template from google.api_core.universe import EmptyUniverseError from google.auth.exceptions import MutualTLSChannelError @@ -165,7 +163,7 @@ def get_universe_domain( def setup_request_id( - request: Union[google.protobuf.message.Message, dict, None], + request: Union[Any, dict, None], field_name: str, is_proto3_optional: bool, ) -> None: @@ -179,7 +177,7 @@ def setup_request_id( resources). Args: - request (Union[google.protobuf.message.Message, dict]): The + request (Union[Any, dict, None]): The request object. field_name (str): The name of the field to populate. is_proto3_optional (bool): Whether the field is proto3 optional. @@ -187,26 +185,34 @@ def setup_request_id( if request is None: return + 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())) + should_populate = field_name not in request or request[field_name] is None + else: + should_populate = not request.get(field_name) else: - if not getattr(request, field_name, None): - setattr(request, field_name, str(uuid.uuid4())) + is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") + if is_proto3_optional: + if is_proto_plus: + try: + should_populate = not request._pb.HasField(field_name) + except ValueError: + should_populate = getattr(request, field_name, None) is None + else: + try: + should_populate = not request.HasField(field_name) + except (AttributeError, ValueError): + should_populate = getattr(request, field_name, None) is None + else: + should_populate = not getattr(request, field_name, None) + + 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( diff --git a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py index 0fb9cbb72fd5..339e328027fd 100755 --- a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py +++ b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py @@ -260,6 +260,16 @@ 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") @@ -267,6 +277,16 @@ def HasField(self, key): 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( @@ -274,14 +294,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"), @@ -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", diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index 8ce97c9cffa7..378910932975 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -22,13 +22,11 @@ """ import uuid -from typing import Union - -import google.protobuf.message +from typing import Any, Union def setup_request_id( - request: Union[google.protobuf.message.Message, dict, None], + request: Union[Any, dict, None], field_name: str, is_proto3_optional: bool, ) -> None: @@ -42,7 +40,7 @@ def setup_request_id( resources). Args: - request (Union[google.protobuf.message.Message, dict]): The + request (Union[Any, dict, None]): The request object. field_name (str): The name of the field to populate. is_proto3_optional (bool): Whether the field is proto3 optional. @@ -50,23 +48,31 @@ def setup_request_id( if request is None: return + 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())) + should_populate = field_name not in request or request[field_name] is None + else: + should_populate = not request.get(field_name) else: - if not getattr(request, field_name, None): - setattr(request, field_name, str(uuid.uuid4())) + is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") + if is_proto3_optional: + if is_proto_plus: + try: + should_populate = not request._pb.HasField(field_name) + except ValueError: + should_populate = getattr(request, field_name, None) is None + else: + try: + should_populate = not request.HasField(field_name) + except (AttributeError, ValueError): + should_populate = getattr(request, field_name, None) is None + else: + should_populate = not getattr(request, field_name, None) + + if should_populate: + generated_id = str(uuid.uuid4()) + if isinstance(request, dict): + request[field_name] = generated_id + else: + setattr(request, field_name, generated_id) diff --git a/packages/google-api-core/tests/unit/gapic/test_requests.py b/packages/google-api-core/tests/unit/gapic/test_requests.py index 71c7ce6d5225..d5e0957786ef 100644 --- a/packages/google-api-core/tests/unit/gapic/test_requests.py +++ b/packages/google-api-core/tests/unit/gapic/test_requests.py @@ -37,11 +37,25 @@ 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") +class MockProtoPlusValueErrorRequest: + def __init__(self, **kwargs): + self._pb = MockValueErrorRequest() + for k, v in kwargs.items(): + setattr(self, k, v) + + # --- Parameterized Test --- 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}" @@ -53,17 +67,25 @@ def HasField(self, key): # MockRequest cases (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 cases (MockProtoRequest(), True, "uuid"), (MockProtoRequest(request_id="already_set"), True, "already_set"), + (MockProtoRequest(request_id=""), True, ""), + # MockProtoPlusRequest cases + (MockProtoPlusRequest(), True, "uuid"), + (MockProtoPlusRequest(request_id="already_set"), True, "already_set"), + (MockProtoPlusRequest(request_id=""), True, ""), # ValueError case (MockValueErrorRequest(), True, "uuid"), + (MockProtoPlusValueErrorRequest(), True, "uuid"), # Dict cases ({}, 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"), @@ -73,14 +95,21 @@ def HasField(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", From 7daaa1940df255a1f63e63887f923fbbb038442c Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 5 Aug 2026 20:40:57 +0000 Subject: [PATCH 3/7] update goldens --- .../%name_%version/%sub/_compat.py.j2 | 17 +++++++++++++++++ .../cloud/storagebatchoperations_v1/_compat.py | 17 +++++++++++++++++ .../google/api_core/gapic_v1/requests.py | 17 +++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 index 0b5fab8360d8..3be859b4b689 100644 --- a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 +++ b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 @@ -188,28 +188,45 @@ def setup_request_id( if request is None: return + # 1. Evaluate whether the field is considered "unset" and needs 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: + # AIP-4235: For dictionaries with optional fields, only populate + # if the key is missing or the value is explicitly None. should_populate = field_name not in request or request[field_name] is None else: + # For non-optional dict fields, populate on any missing or falsy value. should_populate = not request.get(field_name) else: + # Check if this is a proto-plus wrapper object containing an underlying + # protobuf message (`._pb`). is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") if is_proto3_optional: if is_proto_plus: try: + # Ask the underlying protobuf message if the field was explicitly set. should_populate = not request._pb.HasField(field_name) except ValueError: + # Fallback for non-optional fields or non-presence primitives. should_populate = getattr(request, field_name, None) is None else: try: + # For pure protobuf messages, use HasField directly. should_populate = not request.HasField(field_name) except (AttributeError, ValueError): + # For standard Python objects or mocks, only populate if strictly None. should_populate = getattr(request, field_name, None) is None else: + # For non-optional fields on objects, populate on any falsy value (None or ""). should_populate = not getattr(request, field_name, None) + # 2. Consolidate mutation to a single, clean DRY block. if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): diff --git a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py index 0df7c0d7859c..47cb364e6668 100755 --- a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py +++ b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py @@ -185,28 +185,45 @@ def setup_request_id( if request is None: return + # 1. Evaluate whether the field is considered "unset" and needs 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: + # AIP-4235: For dictionaries with optional fields, only populate + # if the key is missing or the value is explicitly None. should_populate = field_name not in request or request[field_name] is None else: + # For non-optional dict fields, populate on any missing or falsy value. should_populate = not request.get(field_name) else: + # Check if this is a proto-plus wrapper object containing an underlying + # protobuf message (`._pb`). is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") if is_proto3_optional: if is_proto_plus: try: + # Ask the underlying protobuf message if the field was explicitly set. should_populate = not request._pb.HasField(field_name) except ValueError: + # Fallback for non-optional fields or non-presence primitives. should_populate = getattr(request, field_name, None) is None else: try: + # For pure protobuf messages, use HasField directly. should_populate = not request.HasField(field_name) except (AttributeError, ValueError): + # For standard Python objects or mocks, only populate if strictly None. should_populate = getattr(request, field_name, None) is None else: + # For non-optional fields on objects, populate on any falsy value (None or ""). should_populate = not getattr(request, field_name, None) + # 2. Consolidate mutation to a single, clean DRY block. if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index 378910932975..f6611b719655 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -48,28 +48,45 @@ def setup_request_id( if request is None: return + # 1. Evaluate whether the field is considered "unset" and needs 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: + # AIP-4235: For dictionaries with optional fields, only populate + # if the key is missing or the value is explicitly None. should_populate = field_name not in request or request[field_name] is None else: + # For non-optional dict fields, populate on any missing or falsy value. should_populate = not request.get(field_name) else: + # Check if this is a proto-plus wrapper object containing an underlying + # protobuf message (`._pb`). is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") if is_proto3_optional: if is_proto_plus: try: + # Ask the underlying protobuf message if the field was explicitly set. should_populate = not request._pb.HasField(field_name) except ValueError: + # Fallback for non-optional fields or non-presence primitives. should_populate = getattr(request, field_name, None) is None else: try: + # For pure protobuf messages, use HasField directly. should_populate = not request.HasField(field_name) except (AttributeError, ValueError): + # For standard Python objects or mocks, only populate if strictly None. should_populate = getattr(request, field_name, None) is None else: + # For non-optional fields on objects, populate on any falsy value (None or ""). should_populate = not getattr(request, field_name, None) + # 2. Consolidate mutation to a single, clean DRY block. if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): From 6e9daa53c1565bed953e7e2992cc8a6ae2186cdb Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 5 Aug 2026 21:04:49 +0000 Subject: [PATCH 4/7] update goldens --- .../%name_%version/%sub/_compat.py.j2 | 57 +++++++++++------- .../storagebatchoperations_v1/_compat.py | 57 +++++++++++------- .../google/api_core/gapic_v1/requests.py | 58 ++++++++++++------- 3 files changed, 109 insertions(+), 63 deletions(-) diff --git a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 index 3be859b4b689..0422be9491ba 100644 --- a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 +++ b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 @@ -17,11 +17,15 @@ 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 +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 @@ -166,7 +170,7 @@ def get_universe_domain( {% if has_auto_populated_fields %} def setup_request_id( - request: Union[Any, dict, None], + request: Union[google.protobuf.message.Message, "proto.Message", dict, None], field_name: str, is_proto3_optional: bool, ) -> None: @@ -180,15 +184,19 @@ def setup_request_id( resources). Args: - request (Union[Any, dict, None]): 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 - # 1. Evaluate whether the field is considered "unset" and needs population. + # 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`) @@ -197,36 +205,43 @@ def setup_request_id( should_populate = False if isinstance(request, dict): if is_proto3_optional: - # AIP-4235: For dictionaries with optional fields, only populate - # if the key is missing or the value is explicitly None. + # 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: - # For non-optional dict fields, populate on any missing or falsy value. + # 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: - # Check if this is a proto-plus wrapper object containing an underlying - # protobuf message (`._pb`). - is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") + # 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: + 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: - # Ask the underlying protobuf message if the field was explicitly set. - should_populate = not request._pb.HasField(field_name) + should_populate = not pb_msg.HasField(field_name) except ValueError: - # Fallback for non-optional fields or non-presence primitives. + # `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: - # For pure protobuf messages, use HasField directly. should_populate = not request.HasField(field_name) except (AttributeError, ValueError): - # For standard Python objects or mocks, only populate if strictly None. + # Fall back for objects/mocks that do not implement `HasField` or where `HasField` fails. should_populate = getattr(request, field_name, None) is None else: - # For non-optional fields on objects, populate on any falsy value (None or ""). + # 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) - # 2. Consolidate mutation to a single, clean DRY block. + # Consolidate mutation to a single, clean DRY block. if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): diff --git a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py index 47cb364e6668..ad9a16eacf3b 100755 --- a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py +++ b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py @@ -18,9 +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 + +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 @@ -163,7 +167,7 @@ def get_universe_domain( def setup_request_id( - request: Union[Any, dict, None], + request: Union[google.protobuf.message.Message, "proto.Message", dict, None], field_name: str, is_proto3_optional: bool, ) -> None: @@ -177,15 +181,19 @@ def setup_request_id( resources). Args: - request (Union[Any, dict, None]): 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 - # 1. Evaluate whether the field is considered "unset" and needs population. + # 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`) @@ -194,36 +202,43 @@ def setup_request_id( should_populate = False if isinstance(request, dict): if is_proto3_optional: - # AIP-4235: For dictionaries with optional fields, only populate - # if the key is missing or the value is explicitly None. + # 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: - # For non-optional dict fields, populate on any missing or falsy value. + # 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: - # Check if this is a proto-plus wrapper object containing an underlying - # protobuf message (`._pb`). - is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") + # 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: + 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: - # Ask the underlying protobuf message if the field was explicitly set. - should_populate = not request._pb.HasField(field_name) + should_populate = not pb_msg.HasField(field_name) except ValueError: - # Fallback for non-optional fields or non-presence primitives. + # `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: - # For pure protobuf messages, use HasField directly. should_populate = not request.HasField(field_name) except (AttributeError, ValueError): - # For standard Python objects or mocks, only populate if strictly None. + # Fall back for objects/mocks that do not implement `HasField` or where `HasField` fails. should_populate = getattr(request, field_name, None) is None else: - # For non-optional fields on objects, populate on any falsy value (None or ""). + # 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) - # 2. Consolidate mutation to a single, clean DRY block. + # Consolidate mutation to a single, clean DRY block. if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index f6611b719655..146a809609d3 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -22,11 +22,16 @@ """ import uuid -from typing import Any, Union +from typing import TYPE_CHECKING, Any, Union + +import google.protobuf.message + +if TYPE_CHECKING: # pragma: NO COVER + import proto # type: ignore[import-untyped] def setup_request_id( - request: Union[Any, dict, None], + request: Union[google.protobuf.message.Message, "proto.Message", dict, None], field_name: str, is_proto3_optional: bool, ) -> None: @@ -40,15 +45,19 @@ def setup_request_id( resources). Args: - request (Union[Any, dict, None]): 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 - # 1. Evaluate whether the field is considered "unset" and needs population. + # 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`) @@ -57,36 +66,43 @@ def setup_request_id( should_populate = False if isinstance(request, dict): if is_proto3_optional: - # AIP-4235: For dictionaries with optional fields, only populate - # if the key is missing or the value is explicitly None. + # 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: - # For non-optional dict fields, populate on any missing or falsy value. + # 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: - # Check if this is a proto-plus wrapper object containing an underlying - # protobuf message (`._pb`). - is_proto_plus = hasattr(request, "_pb") and hasattr(request._pb, "HasField") + # 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: + 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: - # Ask the underlying protobuf message if the field was explicitly set. - should_populate = not request._pb.HasField(field_name) + should_populate = not pb_msg.HasField(field_name) except ValueError: - # Fallback for non-optional fields or non-presence primitives. + # `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: - # For pure protobuf messages, use HasField directly. should_populate = not request.HasField(field_name) except (AttributeError, ValueError): - # For standard Python objects or mocks, only populate if strictly None. + # Fall back for objects/mocks that do not implement `HasField` or where `HasField` fails. should_populate = getattr(request, field_name, None) is None else: - # For non-optional fields on objects, populate on any falsy value (None or ""). + # 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) - # 2. Consolidate mutation to a single, clean DRY block. + # Consolidate mutation to a single, clean DRY block. if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): From c6bae7bbdfefba2e9056f29e1c766bd74079b916 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 5 Aug 2026 21:14:17 +0000 Subject: [PATCH 5/7] remove unsued import --- packages/google-api-core/google/api_core/gapic_v1/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index 146a809609d3..eedabfa56842 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -22,7 +22,7 @@ """ import uuid -from typing import TYPE_CHECKING, Any, Union +from typing import TYPE_CHECKING, Union import google.protobuf.message From 526ff10569cd6debec78eb903c3fa675e6063bf6 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Wed, 5 Aug 2026 23:42:41 +0000 Subject: [PATCH 6/7] update goldens --- .../%name_%version/%sub/_compat.py.j2 | 34 ++++++------------- .../%name_%version/%sub/test_compat.py.j2 | 12 ------- .../storagebatchoperations_v1/_compat.py | 34 ++++++------------- .../storagebatchoperations_v1/test_compat.py | 12 ------- .../google/api_core/gapic_v1/requests.py | 34 ++++++------------- 5 files changed, 33 insertions(+), 93 deletions(-) diff --git a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 index 0422be9491ba..8b81732192d7 100644 --- a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 +++ b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/_compat.py.j2 @@ -215,33 +215,21 @@ def setup_request_id( # Auto-populate if the key is missing, None, or falsy (e.g., empty string ''). should_populate = not request.get(field_name) else: - # 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") + # Case 2: Object request (proto-plus wrapper or pure protobuf message). 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 + # 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 2c: Object request without explicit presence (`is_proto3_optional=False`). + # 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 getattr(request, field_name, None) + should_populate = not bool(getattr(request, field_name, False)) - # Consolidate mutation to a single, clean DRY block. + # If the field was found to be empty, set random id if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): diff --git a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 index 189502640377..d972c5d812d5 100644 --- a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 +++ b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_compat.py.j2 @@ -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): @@ -267,17 +264,11 @@ class MockProtoPlusRequest: 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): @@ -285,9 +276,6 @@ class MockProtoPlusValueErrorRequest: 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( diff --git a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py index ad9a16eacf3b..7f7a2a0a1cf6 100755 --- a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py +++ b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/_compat.py @@ -212,33 +212,21 @@ def setup_request_id( # Auto-populate if the key is missing, None, or falsy (e.g., empty string ''). should_populate = not request.get(field_name) else: - # 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") + # Case 2: Object request (proto-plus wrapper or pure protobuf message). 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 + # 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 2c: Object request without explicit presence (`is_proto3_optional=False`). + # 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 getattr(request, field_name, None) + should_populate = not bool(getattr(request, field_name, False)) - # Consolidate mutation to a single, clean DRY block. + # If the field was found to be empty, set random id if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): diff --git a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py index 339e328027fd..4a3854581747 100755 --- a/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py +++ b/packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_compat.py @@ -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): @@ -266,17 +263,11 @@ def __init__(self, **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): @@ -284,9 +275,6 @@ def __init__(self, **kwargs): 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( diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index eedabfa56842..3dd0316bdf1e 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -76,33 +76,21 @@ def setup_request_id( # Auto-populate if the key is missing, None, or falsy (e.g., empty string ''). should_populate = not request.get(field_name) else: - # 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") + # Case 2: Object request (proto-plus wrapper or pure protobuf message). 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 + # 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 2c: Object request without explicit presence (`is_proto3_optional=False`). + # 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 getattr(request, field_name, None) + should_populate = not bool(getattr(request, field_name, False)) - # Consolidate mutation to a single, clean DRY block. + # If the field was found to be empty, set random id if should_populate: generated_id = str(uuid.uuid4()) if isinstance(request, dict): From 9a526824487af77ba7b0886cd9779a94f95d70c8 Mon Sep 17 00:00:00 2001 From: Heba Alazzeh Date: Thu, 6 Aug 2026 00:04:48 +0000 Subject: [PATCH 7/7] update comments --- .../google-api-core/google/api_core/gapic_v1/requests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/google-api-core/google/api_core/gapic_v1/requests.py b/packages/google-api-core/google/api_core/gapic_v1/requests.py index 3dd0316bdf1e..876f40aaa7b4 100644 --- a/packages/google-api-core/google/api_core/gapic_v1/requests.py +++ b/packages/google-api-core/google/api_core/gapic_v1/requests.py @@ -76,8 +76,9 @@ def setup_request_id( # Auto-populate if the key is missing, None, or falsy (e.g., empty string ''). should_populate = not request.get(field_name) else: - # Case 2: Object request (proto-plus wrapper or pure protobuf message). if is_proto3_optional: + # Case 2a: Proto request with explicit presence (`is_proto3_optional=True`) + # (proto-plus wrapper or pure protobuf message). # Extract the protobuf from proto-plus if wrapped. pure_pb: google.protobuf.message.Message = getattr(request, "_pb", request) try: @@ -86,7 +87,7 @@ def setup_request_id( # 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`). + # Case 2b: Proto 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))