Skip to content

Commit a8cc8d8

Browse files
hebaalazzehhebaalazzeh
committed
feat: delegate request-id setup to public method_helpers
Updates generator templates and goldens to import public method_helpers from google-api-core gapic_v1 and call setup_request_id helper. Removes duplicate setup_request_id test logic from generated client unit tests.
1 parent db040bb commit a8cc8d8

5 files changed

Lines changed: 25 additions & 203 deletions

File tree

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,6 @@ import logging as std_logging
88
from collections import OrderedDict
99
import re
1010
from typing import Dict, Callable, Mapping, MutableMapping, MutableSequence, Optional, {% if service.any_server_streaming %}AsyncIterable, Awaitable, {% endif %}{% if service.any_client_streaming %}AsyncIterator, {% endif %}Sequence, Tuple, Type, Union
11-
{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
12-
import uuid
13-
{% endif %}
1411
{% if service.any_deprecated %}
1512
import warnings
1613
{% endif %}

‎packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2‎

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,14 @@
66
{% import "%namespace/%name_%version/%sub/services/%service/_client_macros.j2" as macros %}
77
{% import "%namespace/%name_%version/%sub/services/%service/_shared_macros.j2" as shared_macros %}
88

9+
{% set has_auto_populated_fields = namespace(value=false) %}
10+
{% for method in service.methods.values() %}
11+
{% set method_settings = api.all_method_settings.get(method.meta.address.proto) %}
12+
{% if method_settings and method_settings.auto_populated_fields %}
13+
{% set has_auto_populated_fields.value = true %}
14+
{% endif %}
15+
{% endfor %}
16+
917
from collections import OrderedDict
1018
{% if service.any_extended_operations_methods %}
1119
import functools
@@ -16,9 +24,6 @@ import logging as std_logging
1624
import os
1725
import re
1826
from typing import Dict, Callable, Mapping, MutableMapping, MutableSequence, Optional, {% if service.any_server_streaming %}Iterable, {% endif %}{% if service.any_client_streaming %}Iterator, {% endif %}Sequence, Tuple, Type, Union, cast
19-
{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
20-
import uuid
21-
{% endif %}
2227
import warnings
2328

2429
{% set package_path = api.naming.module_namespace|join('.') + "." + api.naming.versioned_module_name %}
@@ -30,6 +35,9 @@ from google.api_core import exceptions as core_exceptions
3035
from google.api_core import extended_operation
3136
{% endif %}
3237
from google.api_core import gapic_v1
38+
{% if has_auto_populated_fields.value %}
39+
from google.api_core.gapic_v1 import method_helpers
40+
{% endif %}
3341
from google.api_core import retry as retries
3442
from google.auth import credentials as ga_credentials # type: ignore
3543
from google.auth.transport import mtls # type: ignore
@@ -453,7 +461,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
453461
# NOTE (b/349488459): universe validation is disabled until further notice.
454462
return True
455463

456-
{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
464+
{% if has_auto_populated_fields.value %}
457465
@staticmethod
458466
def _setup_request_id(request, field_name: str, is_proto3_optional: bool):
459467
"""Populate a UUID4 field in the request if it is not already set.
@@ -463,26 +471,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta):
463471
field_name (str): The name of the field to populate.
464472
is_proto3_optional (bool): Whether the field is proto3 optional.
465473
"""
466-
if isinstance(request, dict):
467-
if is_proto3_optional:
468-
if field_name not in request:
469-
request[field_name] = str(uuid.uuid4())
470-
elif not request.get(field_name):
471-
request[field_name] = str(uuid.uuid4())
472-
return
473-
474-
if is_proto3_optional:
475-
try:
476-
# Pure protobuf messages
477-
if not request.HasField(field_name):
478-
setattr(request, field_name, str(uuid.uuid4()))
479-
except (AttributeError, ValueError):
480-
# Proto-plus messages or other objects
481-
if field_name not in request:
482-
setattr(request, field_name, str(uuid.uuid4()))
483-
else:
484-
if not getattr(request, field_name):
485-
setattr(request, field_name, str(uuid.uuid4()))
474+
method_helpers.setup_request_id(request, field_name, is_proto3_optional)
486475
{% endif %}
487476

488477
def _add_cred_info_for_auth_errors(

‎packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2‎

Lines changed: 10 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,17 @@
66
{% import "tests/unit/gapic/%name_%version/%sub/test_macros.j2" as test_macros %}
77
{% import "%namespace/%name_%version/%sub/services/%service/_shared_macros.j2" as shared_macros %}
88

9+
{% set has_auto_populated_fields = namespace(value=false) %}
10+
{% for method in service.methods.values() %}
11+
{% set method_settings = api.all_method_settings.get(method.meta.address.proto) %}
12+
{% if method_settings and method_settings.auto_populated_fields %}
13+
{% set has_auto_populated_fields.value = true %}
14+
{% endif %}
15+
{% endfor %}
16+
917
import os
1018
import asyncio
11-
{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
19+
{% if has_auto_populated_fields.value %}
1220
import re
1321
{% endif %}
1422
from unittest import mock
@@ -107,7 +115,7 @@ CRED_INFO_JSON = {
107115
"principal": "service-account@example.com",
108116
}
109117
CRED_INFO_STRING = json.dumps(CRED_INFO_JSON)
110-
{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
118+
{% if has_auto_populated_fields.value %}
111119
_UUID4_RE = re.compile(r"{{ uuid4_re }}")
112120
{% endif %}
113121

@@ -434,84 +442,6 @@ def test__add_cred_info_for_auth_errors_no_get_cred_info(error_code):
434442
client._add_cred_info_for_auth_errors(error)
435443
assert error.details == []
436444

437-
{% if api.all_method_settings.values()|map(attribute="auto_populated_fields", default=[])|select|list %}
438-
def test__setup_request_id():
439-
class MockRequest:
440-
def __init__(self, **kwargs):
441-
for k, v in kwargs.items():
442-
setattr(self, k, v)
443-
def __contains__(self, key):
444-
return hasattr(self, key)
445-
446-
class MockProtoRequest:
447-
def __init__(self, **kwargs):
448-
for k, v in kwargs.items():
449-
setattr(self, k, v)
450-
def HasField(self, key):
451-
return hasattr(self, key)
452-
453-
# Test with proto3 optional field not in request
454-
request = MockRequest()
455-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
456-
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request.request_id)
457-
458-
# Test with proto3 optional field already in request
459-
request = MockRequest(request_id="already_set")
460-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
461-
assert request.request_id == "already_set"
462-
463-
# Test with non-proto3 optional field empty
464-
request = MockRequest(request_id="")
465-
{{ service.client_name }}._setup_request_id(request, "request_id", False)
466-
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request.request_id)
467-
468-
# Test with non-proto3 optional field already set
469-
request = MockRequest(request_id="already_set")
470-
{{ service.client_name }}._setup_request_id(request, "request_id", False)
471-
assert request.request_id == "already_set"
472-
473-
# Test with proto3 optional field not in request (MockProtoRequest)
474-
request = MockProtoRequest()
475-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
476-
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request.request_id)
477-
478-
# Test with proto3 optional field already in request (MockProtoRequest)
479-
request = MockProtoRequest(request_id="already_set")
480-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
481-
assert request.request_id == "already_set"
482-
483-
# Test with ValueError
484-
class MockValueErrorRequest:
485-
def HasField(self, key):
486-
raise ValueError("Mismatched field")
487-
def __contains__(self, key):
488-
return hasattr(self, key)
489-
490-
request = MockValueErrorRequest()
491-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
492-
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request.request_id)
493-
494-
# Test with dict and proto3 optional field not in request
495-
request = {}
496-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
497-
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request["request_id"])
498-
499-
# Test with dict and proto3 optional field already in request
500-
request = {"request_id": "already_set"}
501-
{{ service.client_name }}._setup_request_id(request, "request_id", True)
502-
assert request["request_id"] == "already_set"
503-
504-
# Test with dict and non-proto3 optional field empty
505-
request = {"request_id": ""}
506-
{{ service.client_name }}._setup_request_id(request, "request_id", False)
507-
assert re.match(r"{{ test_macros.get_uuid4_re() }}", request["request_id"])
508-
509-
# Test with dict and non-proto3 optional field already set
510-
request = {"request_id": "already_set"}
511-
{{ service.client_name }}._setup_request_id(request, "request_id", False)
512-
assert request["request_id"] == "already_set"
513-
514-
{% endif %}
515445
@pytest.mark.parametrize("client_class,transport_name", [
516446
{% if 'grpc' in opts.transport %}
517447
({{ service.client_name }}, "grpc"),

‎packages/gapic-generator/tests/integration/goldens/storagebatchoperations/google/cloud/storagebatchoperations_v1/services/storage_batch_operations/client.py‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
from google.api_core import client_options as client_options_lib
2929
from google.api_core import exceptions as core_exceptions
3030
from google.api_core import gapic_v1
31+
from google.api_core.gapic_v1 import method_helpers
3132
from google.api_core import retry as retries
3233
from google.auth import credentials as ga_credentials # type: ignore
3334
from google.auth.transport import mtls # type: ignore
@@ -480,26 +481,7 @@ def _setup_request_id(request, field_name: str, is_proto3_optional: bool):
480481
field_name (str): The name of the field to populate.
481482
is_proto3_optional (bool): Whether the field is proto3 optional.
482483
"""
483-
if isinstance(request, dict):
484-
if is_proto3_optional:
485-
if field_name not in request:
486-
request[field_name] = str(uuid.uuid4())
487-
elif not request.get(field_name):
488-
request[field_name] = str(uuid.uuid4())
489-
return
490-
491-
if is_proto3_optional:
492-
try:
493-
# Pure protobuf messages
494-
if not request.HasField(field_name):
495-
setattr(request, field_name, str(uuid.uuid4()))
496-
except (AttributeError, ValueError):
497-
# Proto-plus messages or other objects
498-
if field_name not in request:
499-
setattr(request, field_name, str(uuid.uuid4()))
500-
else:
501-
if not getattr(request, field_name):
502-
setattr(request, field_name, str(uuid.uuid4()))
484+
method_helpers.setup_request_id(request, field_name, is_proto3_optional)
503485

504486
def _add_cred_info_for_auth_errors(
505487
self,

‎packages/gapic-generator/tests/integration/goldens/storagebatchoperations/tests/unit/gapic/storagebatchoperations_v1/test_storage_batch_operations.py‎

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -351,82 +351,6 @@ def test__add_cred_info_for_auth_errors_no_get_cred_info(error_code):
351351
client._add_cred_info_for_auth_errors(error)
352352
assert error.details == []
353353

354-
def test__setup_request_id():
355-
class MockRequest:
356-
def __init__(self, **kwargs):
357-
for k, v in kwargs.items():
358-
setattr(self, k, v)
359-
def __contains__(self, key):
360-
return hasattr(self, key)
361-
362-
class MockProtoRequest:
363-
def __init__(self, **kwargs):
364-
for k, v in kwargs.items():
365-
setattr(self, k, v)
366-
def HasField(self, key):
367-
return hasattr(self, key)
368-
369-
# Test with proto3 optional field not in request
370-
request = MockRequest()
371-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
372-
assert re.match(r"[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}", request.request_id)
373-
374-
# Test with proto3 optional field already in request
375-
request = MockRequest(request_id="already_set")
376-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
377-
assert request.request_id == "already_set"
378-
379-
# Test with non-proto3 optional field empty
380-
request = MockRequest(request_id="")
381-
StorageBatchOperationsClient._setup_request_id(request, "request_id", False)
382-
assert re.match(r"[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}", request.request_id)
383-
384-
# Test with non-proto3 optional field already set
385-
request = MockRequest(request_id="already_set")
386-
StorageBatchOperationsClient._setup_request_id(request, "request_id", False)
387-
assert request.request_id == "already_set"
388-
389-
# Test with proto3 optional field not in request (MockProtoRequest)
390-
request = MockProtoRequest()
391-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
392-
assert re.match(r"[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}", request.request_id)
393-
394-
# Test with proto3 optional field already in request (MockProtoRequest)
395-
request = MockProtoRequest(request_id="already_set")
396-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
397-
assert request.request_id == "already_set"
398-
399-
# Test with ValueError
400-
class MockValueErrorRequest:
401-
def HasField(self, key):
402-
raise ValueError("Mismatched field")
403-
def __contains__(self, key):
404-
return hasattr(self, key)
405-
406-
request = MockValueErrorRequest()
407-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
408-
assert re.match(r"[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}", request.request_id)
409-
410-
# Test with dict and proto3 optional field not in request
411-
request = {}
412-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
413-
assert re.match(r"[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}", request["request_id"])
414-
415-
# Test with dict and proto3 optional field already in request
416-
request = {"request_id": "already_set"}
417-
StorageBatchOperationsClient._setup_request_id(request, "request_id", True)
418-
assert request["request_id"] == "already_set"
419-
420-
# Test with dict and non-proto3 optional field empty
421-
request = {"request_id": ""}
422-
StorageBatchOperationsClient._setup_request_id(request, "request_id", False)
423-
assert re.match(r"[a-f0-9]{8}-?[a-f0-9]{4}-?4[a-f0-9]{3}-?[89ab][a-f0-9]{3}-?[a-f0-9]{12}", request["request_id"])
424-
425-
# Test with dict and non-proto3 optional field already set
426-
request = {"request_id": "already_set"}
427-
StorageBatchOperationsClient._setup_request_id(request, "request_id", False)
428-
assert request["request_id"] == "already_set"
429-
430354
@pytest.mark.parametrize("client_class,transport_name", [
431355
(StorageBatchOperationsClient, "grpc"),
432356
(StorageBatchOperationsAsyncClient, "grpc_asyncio"),

0 commit comments

Comments
 (0)