Skip to content

Commit 07f7503

Browse files
authored
fix(api-core): prevent overwriting explicit empty strings for optional request_id (#17798)
### Description This PR addresses code review feedback regarding how `is_proto3_optional=True` fields are treated: - **`requests.py`**: Changed the check `if not getattr(...)` to `if getattr(...) is None` for proto-plus messages and other objects when `is_proto3_optional=True`. This prevents overwriting an explicitly set empty string (`""`) with a generated UUID. - **`test_requests.py`**: Added parameterized test cases for `is_proto3_optional=True` with explicit empty string values `""` to verify they are preserved and not overwritten.
1 parent 8f55f89 commit 07f7503

2 files changed

Lines changed: 5 additions & 1 deletion

File tree

packages/google-api-core/google/api_core/gapic_v1/requests.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ def setup_request_id(
6565
setattr(request, field_name, str(uuid.uuid4()))
6666
except (AttributeError, ValueError):
6767
# Proto-plus messages or other objects
68-
if not getattr(request, field_name, None):
68+
if getattr(request, field_name, None) is None:
6969
setattr(request, field_name, str(uuid.uuid4()))
7070
else:
7171
if not getattr(request, field_name, None):

packages/google-api-core/tests/unit/gapic/test_requests.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ def HasField(self, key):
5353
# MockRequest cases
5454
(MockRequest(), True, "uuid"),
5555
(MockRequest(request_id="already_set"), True, "already_set"),
56+
(MockRequest(request_id=""), True, ""),
5657
(MockRequest(request_id=""), False, "uuid"),
5758
(MockRequest(request_id="already_set"), False, "already_set"),
5859
# MockProtoRequest cases
@@ -64,6 +65,7 @@ def HasField(self, key):
6465
({}, True, "uuid"),
6566
({"request_id": None}, True, "uuid"),
6667
({"request_id": "already_set"}, True, "already_set"),
68+
({"request_id": ""}, True, ""),
6769
({"request_id": ""}, False, "uuid"),
6870
({"request_id": None}, False, "uuid"),
6971
({"request_id": "already_set"}, False, "already_set"),
@@ -73,6 +75,7 @@ def HasField(self, key):
7375
ids=[
7476
"proto3_optional_not_in_request",
7577
"proto3_optional_already_in_request",
78+
"proto3_optional_explicit_empty",
7679
"non_proto3_optional_empty",
7780
"non_proto3_optional_already_set",
7881
"proto3_optional_not_in_request_proto",
@@ -81,6 +84,7 @@ def HasField(self, key):
8184
"dict_proto3_optional_not_in_request",
8285
"dict_proto3_optional_value_none",
8386
"dict_proto3_optional_already_in_request",
87+
"dict_proto3_optional_explicit_empty",
8488
"dict_non_proto3_optional_empty",
8589
"dict_non_proto3_optional_value_none",
8690
"dict_non_proto3_optional_already_set",

0 commit comments

Comments
 (0)