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..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 @@ -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 @@ -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: @@ -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): 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 %} 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..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): @@ -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}" @@ -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"), @@ -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", 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..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 @@ -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 @@ -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: @@ -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( 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..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): @@ -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}" @@ -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"), @@ -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", 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..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 @@ -22,13 +22,16 @@ """ import uuid -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] + 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: @@ -42,31 +45,56 @@ 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 getattr(request, field_name, None) is 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())) + 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: + 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: 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)) + + # 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) 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..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}" @@ -59,8 +73,14 @@ def HasField(self, key): # 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"), @@ -80,7 +100,12 @@ def HasField(self, key): "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",