diff --git a/.generator/requirements.in b/.generator/requirements.in index cb9f2bad32c0..24d9cd64e588 100644 --- a/.generator/requirements.in +++ b/.generator/requirements.in @@ -1,5 +1,5 @@ click -gapic-generator==1.30.13 # https://github.com/googleapis/gapic-generator-python/releases/tag/v1.30.13 +-e /usr/local/google/home/omairn/git/googleapis/google-cloud-python/packages/gapic-generator nox starlark-pyo3>=2025.1 build diff --git a/.librarian/generate-request.json b/.librarian/generate-request.json new file mode 100644 index 000000000000..8e95ec16bdb9 --- /dev/null +++ b/.librarian/generate-request.json @@ -0,0 +1,25 @@ +{ + "id": "google-cloud-dialogflow", + "version": "2.47.0", + "apis": [ + { + "path": "google/cloud/dialogflow/v2beta1", + "service_config": "dialogflow_v2beta1.yaml" + }, + { + "path": "google/cloud/dialogflow/v2", + "service_config": "dialogflow_v2.yaml" + } + ], + "source_roots": [ + "packages/google-cloud-dialogflow" + ], + "preserve_regex": [ + "packages/google-cloud-dialogflow/CHANGELOG.md", + "docs/CHANGELOG.md" + ], + "remove_regex": [ + "packages/google-cloud-dialogflow/" + ], + "tag_format": "{id}-v{version}" +} \ No newline at end of file diff --git a/packages/gapic-generator/gapic/schema/api.py b/packages/gapic-generator/gapic/schema/api.py index 4ee478a64398..ee97ed330604 100644 --- a/packages/gapic-generator/gapic/schema/api.py +++ b/packages/gapic-generator/gapic/schema/api.py @@ -170,9 +170,12 @@ def resource_messages(self) -> Mapping[str, wrappers.MessageType]: if msg.options.Extensions[resource_pb2.resource].type ) return collections.OrderedDict( - itertools.chain( - file_resource_messages, - resource_messages, + sorted( + itertools.chain( + file_resource_messages, + resource_messages, + ), + key=lambda item: item[0] ) ) diff --git a/packages/gapic-generator/gapic/schema/wrappers.py b/packages/gapic-generator/gapic/schema/wrappers.py index f654d0a82d18..0df9f0c4819b 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -70,6 +70,7 @@ from gapic.utils import cached_proto_context from gapic.utils import uri_sample from gapic.utils import make_private +from gapic.utils import Options @dataclasses.dataclass(frozen=True) @@ -688,7 +689,16 @@ def resource_path(self) -> Optional[str]: @property def resource_type(self) -> Optional[str]: resource = self.options.Extensions[resource_pb2.resource] - return resource.type[resource.type.find("/") + 1 :] if resource else None + if not resource: + return None + + # Extract the standard short name (e.g., "Tool" from "ces.googleapis.com/Tool") + default_type = resource.type[resource.type.find("/") + 1 :] + + # Check if the CLI provided an alias for this specific resource path + aliases = getattr(Options, "resource_name_aliases_global", {}) + + return aliases.get(resource.type, default_type) @property def resource_type_full_path(self) -> Optional[str]: @@ -2278,9 +2288,9 @@ def names(self) -> FrozenSet[str]: return frozenset(answer) @utils.cached_property - def resource_messages(self) -> FrozenSet[MessageType]: + def resource_messages(self) -> Sequence['MessageType']: """Returns all the resource message types used in all - request and response fields in the service.""" + request and response fields in the service, deterministically sorted.""" def gen_resources(message): if message.resource_path: @@ -2301,7 +2311,7 @@ def gen_indirect_resources_used(message): if resource: yield resource - return frozenset( + unique_messages = frozenset( msg for method in self.methods.values() for msg in chain( @@ -2316,6 +2326,33 @@ def gen_indirect_resources_used(message): ) ) + # 1. Deterministic AST Sort to avoid potential pipeline flakiness. + sorted_messages = sorted( + unique_messages, + key=lambda m: m.resource_type_full_path or m.name + ) + + # 2. Fail-fast collision detection + seen_types = {} + for msg in sorted_messages: + res_type = msg.resource_type + if not res_type: + continue + + if res_type in seen_types: + incumbent = seen_types[res_type] + raise ValueError( + f"\n\nFatal: Namespace collision detected for resource type '{res_type}'.\n" + f"Resources '{incumbent.resource_type_full_path}' and '{msg.resource_type_full_path}' " + f"both flatten to the exact same method name.\n" + f"To protect backward compatibility, explicitly alias one of these using " + f"the `--resource-name-alias` CLI parameter.\n" + f"Example: --resource-name-alias={msg.resource_type_full_path}:CustomName\n" + ) + seen_types[res_type] = msg + + return tuple(sorted_messages) + @utils.cached_property def resource_messages_dict(self) -> Dict[str, MessageType]: """Returns a dict from resource reference to diff --git a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 index f89b25de271a..7cf8271a8040 100644 --- a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 +++ b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/async_client.py.j2 @@ -76,11 +76,11 @@ class {{ service.async_client_name }}: _DEFAULT_ENDPOINT_TEMPLATE = {{ service.client_name }}._DEFAULT_ENDPOINT_TEMPLATE _DEFAULT_UNIVERSE = {{ service.client_name }}._DEFAULT_UNIVERSE - {% for message in service.resource_messages|sort(attribute="resource_type") %} + {% for message in service.resource_messages %} {{ message.resource_type|snake_case }}_path = staticmethod({{ service.client_name }}.{{ message.resource_type|snake_case }}_path) parse_{{ message.resource_type|snake_case}}_path = staticmethod({{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path) {% endfor %} - {% for resource_msg in service.common_resources.values()|sort(attribute="type_name") %} + {% for resource_msg in service.common_resources.values() %} common_{{ resource_msg.message_type.resource_type|snake_case }}_path = staticmethod({{ service.client_name }}.common_{{ resource_msg.message_type.resource_type|snake_case }}_path) parse_common_{{ resource_msg.message_type.resource_type|snake_case }}_path = staticmethod({{ service.client_name }}.parse_common_{{ resource_msg.message_type.resource_type|snake_case }}_path) {% endfor %} diff --git a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index f8056c05c7fd..41da34929519 100644 --- a/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/packages/gapic-generator/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -262,7 +262,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): return self._transport - {% for message in service.resource_messages|sort(attribute="resource_type") %} + {% for message in service.resource_messages %} @staticmethod def {{ message.resource_type|snake_case }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: """Returns a fully-qualified {{ message.resource_type|snake_case }} string.""" diff --git a/packages/gapic-generator/gapic/utils/options.py b/packages/gapic-generator/gapic/utils/options.py index 30184f12c0a9..47af7eee1740 100644 --- a/packages/gapic-generator/gapic/utils/options.py +++ b/packages/gapic-generator/gapic/utils/options.py @@ -51,6 +51,7 @@ class Options: rest_numeric_enums: bool = False proto_plus_deps: Tuple[str, ...] = dataclasses.field(default=("",)) gapic_version: str = "0.0.0" + resource_name_aliases: Dict[str, str] = dataclasses.field(default_factory=dict) # Class constants PYTHON_GAPIC_PREFIX: str = "python-gapic-" @@ -73,6 +74,7 @@ class Options: # For example, 'google.cloud.api.v1+google.cloud.anotherapi.v2' "proto-plus-deps", "gapic-version", # A version string following https://peps.python.org/pep-0440 + "resource-name-alias", ) ) @@ -187,6 +189,39 @@ def tweak_path(p): proto_plus_deps = tuple(opts.pop("proto-plus-deps", "")) if len(proto_plus_deps): proto_plus_deps = tuple(proto_plus_deps[0].split("+")) + + # Parse the resource name aliases dictionary (Format: "path/to/Resource:AliasName") + resource_name_aliases = {} + raw_aliases = opts.pop("resource-name-alias", []) + + # Normalize: protoc can return a string (1 flag) or list (multiple flags) + if not isinstance(raw_aliases, list): + raw_aliases = [raw_aliases] + + # Parse explicitly and safely + for mapping in raw_aliases: + if not mapping or not mapping.strip(): + continue + + try: + # split(":", 1) ensures we only split on the FIRST colon + res_path, alias_name = mapping.split(":", 1) + + clean_path = res_path.strip() + clean_alias = alias_name.strip() + + if not clean_path or not clean_alias: + raise ValueError() + + resource_name_aliases[clean_path] = clean_alias + + except ValueError: + warnings.warn( + f"Ignored malformed resource-name-alias: '{mapping}'. " + "Expected format is 'resource.path/Name:AliasName'." + ) + + Options.resource_name_aliases_global = resource_name_aliases answer = Options( name=opts.pop("name", [""]).pop(), @@ -210,6 +245,7 @@ def tweak_path(p): rest_numeric_enums=rest_numeric_enums, proto_plus_deps=proto_plus_deps, gapic_version=opts.pop("gapic-version", ["0.0.0"]).pop(), + resource_name_aliases=resource_name_aliases, ) # Note: if we ever need to recursively check directories for sample diff --git a/packages/gapic-generator/tests/unit/schema/test_api.py b/packages/gapic-generator/tests/unit/schema/test_api.py index 9d2dab2ec6a1..9d94e8251523 100644 --- a/packages/gapic-generator/tests/unit/schema/test_api.py +++ b/packages/gapic-generator/tests/unit/schema/test_api.py @@ -1740,17 +1740,17 @@ def test_file_level_resources(): expected = collections.OrderedDict( ( ( - "nomenclature.linnaen.com/Species", + "nomenclature.linnaen.com/Phylum", wrappers.CommonResource( - type_name="nomenclature.linnaen.com/Species", - pattern="families/{family}/genera/{genus}/species/{species}", + type_name="nomenclature.linnaen.com/Phylum", + pattern="kingdoms/{kingdom}/phyla/{phylum}", ).message_type, ), ( - "nomenclature.linnaen.com/Phylum", + "nomenclature.linnaen.com/Species", wrappers.CommonResource( - type_name="nomenclature.linnaen.com/Phylum", - pattern="kingdoms/{kingdom}/phyla/{phylum}", + type_name="nomenclature.linnaen.com/Species", + pattern="families/{family}/genera/{genus}/species/{species}", ).message_type, ), ) @@ -1767,7 +1767,7 @@ def test_file_level_resources(): # The service doesn't own any method that owns a message that references # Phylum, so the service doesn't count it among its resource messages. expected.pop("nomenclature.linnaen.com/Phylum") - expected = frozenset(expected.values()) + expected = tuple(expected.values()) actual = service.resource_messages assert actual == expected @@ -1822,7 +1822,7 @@ def test_resources_referenced_but_not_typed(reference_attr="type"): name_resource_opts.child_type = species_resource_opts.type api_schema = api.API.build([fdp], package="nomenclature.linneaen.v1") - expected = {api_schema.messages["nomenclature.linneaen.v1.Species"]} + expected = (api_schema.messages["nomenclature.linneaen.v1.Species"],) actual = api_schema.services[ "nomenclature.linneaen.v1.SpeciesService" ].resource_messages diff --git a/packages/gapic-generator/tests/unit/schema/wrappers/test_service.py b/packages/gapic-generator/tests/unit/schema/wrappers/test_service.py index eb54577b2e51..04f7ac7308ea 100644 --- a/packages/gapic-generator/tests/unit/schema/wrappers/test_service.py +++ b/packages/gapic-generator/tests/unit/schema/wrappers/test_service.py @@ -267,12 +267,12 @@ def test_resource_messages(): ), ) - expected = { - squid_resource, + expected = ( clam_resource, - whelk_resource, squamosa_message, - } + squid_resource, + whelk_resource, + ) actual = service.resource_messages assert expected == actual @@ -557,7 +557,7 @@ def test_resource_response(): ), ) - expected = {squid_resource, clam_resource} + expected = (clam_resource, squid_resource) actual = mollusc_service.resource_messages assert expected == actual