From 84ee2f30fc67638326a308dd0c25255bcc8dfabd Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 6 Apr 2026 22:38:44 +0000 Subject: [PATCH 01/14] wip --- .../gapic-generator/gapic/schema/wrappers.py | 45 +++++++++++++++++++ .../%sub/services/%service/client.py.j2 | 9 ++-- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/packages/gapic-generator/gapic/schema/wrappers.py b/packages/gapic-generator/gapic/schema/wrappers.py index f654d0a82d18..dcaf493378c4 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -2240,6 +2240,51 @@ def oauth_scopes(self) -> Sequence[str]: for i in self.options.Extensions[client_pb2.oauth_scopes].split(",") if i ) + + @utils.cached_property + def deterministic_resource_messages(self) -> Sequence[MessageType]: + """Returns a sorted sequence of resource messages to guarantee deterministic output order.""" + return tuple( + sorted( + self.resource_messages, + key=lambda r: r.resource_type_full_path or r.name + ) + ) + + @utils.cached_property + def resource_path_method_names(self) -> Dict[str, str]: + """Returns a mapping of resource_type_full_path to its disambiguated Python method base name. + + e.g., 'ces.googleapis.com/Tool' -> 'ces_tool' + 'dialogflow.googleapis.com/Tool' -> 'dialogflow_tool' + """ + import collections + from gapic import utils + + # 1. Count occurrences of the short resource type (e.g., 'Tool') across ALL resources + type_counts = collections.Counter( + r.resource_type for r in self.resource_messages_dict.values() if r.resource_type + ) + + method_names = {} + for full_path, resource in self.resource_messages_dict.items(): + if not resource.resource_type: + continue + + # Standard base name (e.g., "Tool" -> "tool") + base_name = utils.to_snake_case(resource.resource_type) + + # 2. Collision detection: If multiple resources share the same short type name + if type_counts[resource.resource_type] > 1: + # Extract the domain and prepend it (e.g., 'ces' from 'ces.googleapis.com/Tool') + domain = full_path.split(".")[0] + # Replace dashes with underscores for valid python method names (e.g., 'foo-bar' -> 'foo_bar') + domain_prefix = domain.replace("-", "_") + base_name = f"{domain_prefix}_{base_name}" + + method_names[full_path] = base_name + + return method_names @property def module_name(self) -> str: 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..5acabdfead18 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,15 +262,16 @@ 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.deterministic_resource_messages %} + {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} + @staticmethod - def {{ message.resource_type|snake_case }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: + def {{ method_base_name }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: """Returns a fully-qualified {{ message.resource_type|snake_case }} string.""" return "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) - @staticmethod - def parse_{{ message.resource_type|snake_case }}_path(path: str) -> Dict[str,str]: + def parse_{{ method_base_name }}_path(path: str) -> Dict[str,str]: """Parses a {{ message.resource_type|snake_case }} path into its component segments.""" m = re.match(r"{{ message.path_regex_str }}", path) return m.groupdict() if m else {} From c5aa36da49e0130bbe01c986bafc97a96fb595b4 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 6 Apr 2026 22:41:01 +0000 Subject: [PATCH 02/14] wip --- .generator/requirements.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From 627e377d522a3bc67dc56e2c20939f573cb93142 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 6 Apr 2026 23:08:12 +0000 Subject: [PATCH 03/14] wip --- .librarian/generate-request.json | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 .librarian/generate-request.json 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 From 19bc3e30266b1cc8e832784234709dc3248019b7 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 6 Apr 2026 23:08:55 +0000 Subject: [PATCH 04/14] wip --- .../gapic-generator/gapic/schema/wrappers.py | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/packages/gapic-generator/gapic/schema/wrappers.py b/packages/gapic-generator/gapic/schema/wrappers.py index dcaf493378c4..ed7d63c9b831 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -2253,15 +2253,10 @@ def deterministic_resource_messages(self) -> Sequence[MessageType]: @utils.cached_property def resource_path_method_names(self) -> Dict[str, str]: - """Returns a mapping of resource_type_full_path to its disambiguated Python method base name. - - e.g., 'ces.googleapis.com/Tool' -> 'ces_tool' - 'dialogflow.googleapis.com/Tool' -> 'dialogflow_tool' - """ + """Returns a mapping of resource_type_full_path to its disambiguated Python method base name.""" import collections from gapic import utils - # 1. Count occurrences of the short resource type (e.g., 'Tool') across ALL resources type_counts = collections.Counter( r.resource_type for r in self.resource_messages_dict.values() if r.resource_type ) @@ -2271,16 +2266,16 @@ def resource_path_method_names(self) -> Dict[str, str]: if not resource.resource_type: continue - # Standard base name (e.g., "Tool" -> "tool") base_name = utils.to_snake_case(resource.resource_type) - # 2. Collision detection: If multiple resources share the same short type name + # Collision detection if type_counts[resource.resource_type] > 1: - # Extract the domain and prepend it (e.g., 'ces' from 'ces.googleapis.com/Tool') domain = full_path.split(".")[0] - # Replace dashes with underscores for valid python method names (e.g., 'foo-bar' -> 'foo_bar') - domain_prefix = domain.replace("-", "_") - base_name = f"{domain_prefix}_{base_name}" + + # THE MAGIC FIX: Only prefix if the resource belongs to a foreign API + if domain != self.shortname: + domain_prefix = domain.replace("-", "_") + base_name = f"{domain_prefix}_{base_name}" method_names[full_path] = base_name From 380ce06cb53585ff1dbd8a1dd5928dfac72cc27a Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 6 Apr 2026 23:28:40 +0000 Subject: [PATCH 05/14] wip --- packages/gapic-generator/gapic/schema/api.py | 10 ++++++---- .../gapic-generator/gapic/schema/wrappers.py | 19 ++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/gapic-generator/gapic/schema/api.py b/packages/gapic-generator/gapic/schema/api.py index 4ee478a64398..d01dcfb561b9 100644 --- a/packages/gapic-generator/gapic/schema/api.py +++ b/packages/gapic-generator/gapic/schema/api.py @@ -157,7 +157,7 @@ def messages(self) -> Mapping[str, wrappers.MessageType]: @cached_property def resource_messages(self) -> Mapping[str, wrappers.MessageType]: - """Return the file level resources of the proto.""" + """Return the file level resources of the proto in a deterministic order.""" file_resource_messages = ( (res.type, wrappers.CommonResource.build(res).message_type) for res in self.file_pb2.options.Extensions[ @@ -169,10 +169,12 @@ def resource_messages(self) -> Mapping[str, wrappers.MessageType]: for msg in self.messages.values() if msg.options.Extensions[resource_pb2.resource].type ) + + # Sort the chained generator by the resource path (item[0]) to ensure determinism 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 ed7d63c9b831..283e9434e8ce 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -2318,14 +2318,13 @@ 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 in a deterministic order.""" def gen_resources(message): if message.resource_path: yield message - for type_ in message.recursive_field_types: if type_.resource_path: yield type_ @@ -2334,14 +2333,12 @@ def gen_indirect_resources_used(message): for field in message.recursive_resource_fields: resource = field.options.Extensions[resource_pb2.resource_reference] resource_type = resource.type or resource.child_type - # The resource may not be visible if the resource type is one of - # the common_resources (see the class var in class definition) - # or if it's something unhelpful like '*'. resource = self.visible_resources.get(resource_type) if resource: yield resource - return frozenset( + # Collect all resources into an unordered set to remove duplicates + unsorted_resources = set( msg for method in self.methods.values() for msg in chain( @@ -2355,6 +2352,14 @@ def gen_indirect_resources_used(message): ), ) ) + + # Return them deterministically sorted by their full path + return tuple( + sorted( + unsorted_resources, + key=lambda r: r.resource_type_full_path or r.name + ) + ) @utils.cached_property def resource_messages_dict(self) -> Dict[str, MessageType]: From b69d1e444cf613c72c2bc94c2ad8ab6d80df4d2d Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 6 Apr 2026 23:56:40 +0000 Subject: [PATCH 06/14] wip --- .../%sub/services/%service/client.py.j2 | 7 ++- .../%name_%version/%sub/test_%service.py.j2 | 13 +++-- .../%sub/services/%service/async_client.py.j2 | 7 ++- .../%sub/services/%service/client.py.j2 | 2 +- .../%name_%version/%sub/test_%service.py.j2 | 13 +++-- .../tests/unit/schema/test_api.py | 13 ++++- .../unit/schema/wrappers/test_service.py | 57 ++++++++++++++++--- 7 files changed, 84 insertions(+), 28 deletions(-) diff --git a/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 b/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 index d76aa97d064b..d61dc41d2283 100644 --- a/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 +++ b/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 @@ -199,15 +199,16 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): """ self.transport.close() - {% for message in service.resource_messages|sort(attribute="resource_type") %} + {% for message in service.resource_messages %} + {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} @staticmethod - def {{ message.resource_type|snake_case }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: + def {{ method_base_name }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: """Returns a fully-qualified {{ message.resource_type|snake_case }} string.""" return "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) @staticmethod - def parse_{{ message.resource_type|snake_case }}_path(path: str) -> Dict[str,str]: + def parse_{{ method_base_name }}_path(path: str) -> Dict[str,str]: """Parses a {{ message.resource_type|snake_case }} path into its component segments.""" m = re.match(r"{{ message.path_regex_str }}", path) return m.groupdict() if m else {} diff --git a/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 index 39d5fa7d35e7..d7a8d33a31ee 100644 --- a/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 +++ b/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 @@ -2382,26 +2382,27 @@ def test_{{ service.name|snake_case }}_grpc_lro_client(): {% endif %} {# if grpc in opts #} {% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") %} -{% for message in service.resource_messages|sort(attribute="resource_type") %} -def test_{{ message.resource_type|snake_case }}_path(): +{% for message in service.resource_messages %} +{% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} +def test_{{ method_base_name }}_path(): {% for arg in message.resource_path_args %} {{ arg }} = "{{ molluscs.next() }}" {% endfor %} expected = "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) - actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }}) + actual = {{ service.client_name }}.{{ method_base_name }}_path({{message.resource_path_args|join(", ") }}) assert expected == actual -def test_parse_{{ message.resource_type|snake_case }}_path(): +def test_parse_{{ method_base_name }}_path(): expected = { {% for arg in message.resource_path_args %} "{{ arg }}": "{{ molluscs.next() }}", {% endfor %} } - path = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path(**expected) + path = {{ service.client_name }}.{{ method_base_name }}_path(**expected) # Check that the path construction is reversible. - actual = {{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path(path) + actual = {{ service.client_name }}.parse_{{ method_base_name }}_path(path) assert expected == actual {% endfor %} 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..4090f17e01a5 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,9 +76,10 @@ 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") %} - {{ 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) + {% for message in service.resource_messages %} + {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} + {{ method_base_name }}_path = staticmethod({{ service.client_name }}.{{ method_base_name }}_path) + parse_{{ method_base_name }}_path = staticmethod({{ service.client_name }}.parse_{{ method_base_name }}_path) {% endfor %} {% for resource_msg in service.common_resources.values()|sort(attribute="type_name") %} common_{{ resource_msg.message_type.resource_type|snake_case }}_path = staticmethod({{ service.client_name }}.common_{{ resource_msg.message_type.resource_type|snake_case }}_path) 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 5acabdfead18..963326a3ae31 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.deterministic_resource_messages %} + {% for message in service.resource_messages %} {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} @staticmethod diff --git a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 index 0a48dd6e1887..280cd77096e8 100644 --- a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 +++ b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 @@ -1788,26 +1788,27 @@ def test_{{ service.name|snake_case }}_grpc_lro_async_client(): {% endif %} {# if grpc in opts #} {% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") %} -{% for message in service.resource_messages|sort(attribute="resource_type") %} -def test_{{ message.resource_type|snake_case }}_path(): +{% for message in service.resource_messages %} +{% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} +def test_{{ method_base_name }}_path(): {% for arg in message.resource_path_args %} {{ arg }} = "{{ molluscs.next() }}" {% endfor %} expected = "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) - actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }}) + actual = {{ service.client_name }}.{{ method_base_name }}_path({{message.resource_path_args|join(", ") }}) assert expected == actual -def test_parse_{{ message.resource_type|snake_case }}_path(): +def test_parse_{{ method_base_name }}_path(): expected = { {% for arg in message.resource_path_args %} "{{ arg }}": "{{ molluscs.next() }}", {% endfor %} } - path = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path(**expected) + path = {{ service.client_name }}.{{ method_base_name }}_path(**expected) # Check that the path construction is reversible. - actual = {{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path(path) + actual = {{ service.client_name }}.parse_{{ method_base_name }}_path(path) assert expected == actual {% endfor %} diff --git a/packages/gapic-generator/tests/unit/schema/test_api.py b/packages/gapic-generator/tests/unit/schema/test_api.py index 9d2dab2ec6a1..f5b96caa4cb5 100644 --- a/packages/gapic-generator/tests/unit/schema/test_api.py +++ b/packages/gapic-generator/tests/unit/schema/test_api.py @@ -1767,7 +1767,14 @@ 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()) + + # Update test to expect the new deterministically sorted tuple + expected = tuple( + sorted( + expected.values(), + key=lambda r: r.resource_type_full_path or r.name + ) + ) actual = service.resource_messages assert actual == expected @@ -1822,7 +1829,9 @@ 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"]} + + # Update test to expect a tuple instead of a set + expected = tuple([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..5720ece09d12 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, - clam_resource, - whelk_resource, - squamosa_message, - } + expected = tuple( + sorted( + [squid_resource, clam_resource, whelk_resource, squamosa_message], + key=lambda r: r.resource_type_full_path or r.name + ) + ) actual = service.resource_messages assert expected == actual @@ -557,11 +557,54 @@ def test_resource_response(): ), ) - expected = {squid_resource, clam_resource} + expected = tuple( + sorted( + [squid_resource, clam_resource], + key=lambda r: r.resource_type_full_path or r.name + ) + ) actual = mollusc_service.resource_messages assert expected == actual +def test_service_resource_path_method_names_collision(): + # Setup mock resources with a collision on "Tool" + native_opts = descriptor_pb2.MessageOptions() + native_opts.Extensions[resource_pb2.resource].type = "dialogflow.googleapis.com/Tool" + native_tool = make_message("Tool", options=native_opts) + + foreign_opts = descriptor_pb2.MessageOptions() + foreign_opts.Extensions[resource_pb2.resource].type = "ces.googleapis.com/Tool" + foreign_tool = make_message("Tool", options=foreign_opts) + + service = make_service( + name="DialogflowService", + host="dialogflow.googleapis.com", # This sets the native domain shortname + methods=( + make_method( + "DoTool", + input_message=make_message( + "DoToolRequest", + fields=( + make_field("native", message=native_tool), + make_field("foreign", message=foreign_tool), + ) + ) + ), + ) + ) + + # 1. Test Determinism (Sorting by full path instead of unordered frozenset) + resources = service.resource_messages + assert resources[0].resource_type_full_path == "ces.googleapis.com/Tool" + assert resources[1].resource_type_full_path == "dialogflow.googleapis.com/Tool" + + # 2. Test Collision Resolution (Prefixing only the foreign domain) + method_names = service.resource_path_method_names + assert method_names["ces.googleapis.com/Tool"] == "ces_tool" + assert method_names["dialogflow.googleapis.com/Tool"] == "tool" + + def test_operation_polling_method(): T = descriptor_pb2.FieldDescriptorProto.Type From 8249847f050c7fca47b69717810f6a73ac058a29 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:01:40 +0000 Subject: [PATCH 07/14] revert changes --- .../%sub/services/%service/client.py.j2 | 7 +-- .../%sub/services/%service/async_client.py.j2 | 7 +-- .../%sub/services/%service/client.py.j2 | 9 ++- .../tests/unit/schema/test_api.py | 13 +---- .../unit/schema/wrappers/test_service.py | 57 +++---------------- 5 files changed, 19 insertions(+), 74 deletions(-) diff --git a/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 b/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 index d61dc41d2283..d76aa97d064b 100644 --- a/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 +++ b/packages/gapic-generator/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 @@ -199,16 +199,15 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): """ self.transport.close() - {% for message in service.resource_messages %} - {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} + {% for message in service.resource_messages|sort(attribute="resource_type") %} @staticmethod - def {{ method_base_name }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: + 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.""" return "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) @staticmethod - def parse_{{ method_base_name }}_path(path: str) -> Dict[str,str]: + def parse_{{ message.resource_type|snake_case }}_path(path: str) -> Dict[str,str]: """Parses a {{ message.resource_type|snake_case }} path into its component segments.""" m = re.match(r"{{ message.path_regex_str }}", path) return m.groupdict() if m else {} 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 4090f17e01a5..f89b25de271a 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,10 +76,9 @@ 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 %} - {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} - {{ method_base_name }}_path = staticmethod({{ service.client_name }}.{{ method_base_name }}_path) - parse_{{ method_base_name }}_path = staticmethod({{ service.client_name }}.parse_{{ method_base_name }}_path) + {% for message in service.resource_messages|sort(attribute="resource_type") %} + {{ 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") %} common_{{ resource_msg.message_type.resource_type|snake_case }}_path = staticmethod({{ service.client_name }}.common_{{ resource_msg.message_type.resource_type|snake_case }}_path) 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 963326a3ae31..f8056c05c7fd 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,16 +262,15 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): return self._transport - {% for message in service.resource_messages %} - {% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} - + {% for message in service.resource_messages|sort(attribute="resource_type") %} @staticmethod - def {{ method_base_name }}_path({% for arg in message.resource_path_args %}{{ arg }}: str,{% endfor %}) -> str: + 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.""" return "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) + @staticmethod - def parse_{{ method_base_name }}_path(path: str) -> Dict[str,str]: + def parse_{{ message.resource_type|snake_case }}_path(path: str) -> Dict[str,str]: """Parses a {{ message.resource_type|snake_case }} path into its component segments.""" m = re.match(r"{{ message.path_regex_str }}", path) return m.groupdict() if m else {} diff --git a/packages/gapic-generator/tests/unit/schema/test_api.py b/packages/gapic-generator/tests/unit/schema/test_api.py index f5b96caa4cb5..9d2dab2ec6a1 100644 --- a/packages/gapic-generator/tests/unit/schema/test_api.py +++ b/packages/gapic-generator/tests/unit/schema/test_api.py @@ -1767,14 +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") - - # Update test to expect the new deterministically sorted tuple - expected = tuple( - sorted( - expected.values(), - key=lambda r: r.resource_type_full_path or r.name - ) - ) + expected = frozenset(expected.values()) actual = service.resource_messages assert actual == expected @@ -1829,9 +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") - - # Update test to expect a tuple instead of a set - expected = tuple([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 5720ece09d12..eb54577b2e51 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 = tuple( - sorted( - [squid_resource, clam_resource, whelk_resource, squamosa_message], - key=lambda r: r.resource_type_full_path or r.name - ) - ) + expected = { + squid_resource, + clam_resource, + whelk_resource, + squamosa_message, + } actual = service.resource_messages assert expected == actual @@ -557,54 +557,11 @@ def test_resource_response(): ), ) - expected = tuple( - sorted( - [squid_resource, clam_resource], - key=lambda r: r.resource_type_full_path or r.name - ) - ) + expected = {squid_resource, clam_resource} actual = mollusc_service.resource_messages assert expected == actual -def test_service_resource_path_method_names_collision(): - # Setup mock resources with a collision on "Tool" - native_opts = descriptor_pb2.MessageOptions() - native_opts.Extensions[resource_pb2.resource].type = "dialogflow.googleapis.com/Tool" - native_tool = make_message("Tool", options=native_opts) - - foreign_opts = descriptor_pb2.MessageOptions() - foreign_opts.Extensions[resource_pb2.resource].type = "ces.googleapis.com/Tool" - foreign_tool = make_message("Tool", options=foreign_opts) - - service = make_service( - name="DialogflowService", - host="dialogflow.googleapis.com", # This sets the native domain shortname - methods=( - make_method( - "DoTool", - input_message=make_message( - "DoToolRequest", - fields=( - make_field("native", message=native_tool), - make_field("foreign", message=foreign_tool), - ) - ) - ), - ) - ) - - # 1. Test Determinism (Sorting by full path instead of unordered frozenset) - resources = service.resource_messages - assert resources[0].resource_type_full_path == "ces.googleapis.com/Tool" - assert resources[1].resource_type_full_path == "dialogflow.googleapis.com/Tool" - - # 2. Test Collision Resolution (Prefixing only the foreign domain) - method_names = service.resource_path_method_names - assert method_names["ces.googleapis.com/Tool"] == "ces_tool" - assert method_names["dialogflow.googleapis.com/Tool"] == "tool" - - def test_operation_polling_method(): T = descriptor_pb2.FieldDescriptorProto.Type From 840b26522ea5ed41350a44276e9b597e92923a67 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:06:10 +0000 Subject: [PATCH 08/14] revert changes --- packages/gapic-generator/gapic/schema/api.py | 10 ++-- .../gapic-generator/gapic/schema/wrappers.py | 59 +++---------------- 2 files changed, 11 insertions(+), 58 deletions(-) diff --git a/packages/gapic-generator/gapic/schema/api.py b/packages/gapic-generator/gapic/schema/api.py index d01dcfb561b9..4ee478a64398 100644 --- a/packages/gapic-generator/gapic/schema/api.py +++ b/packages/gapic-generator/gapic/schema/api.py @@ -157,7 +157,7 @@ def messages(self) -> Mapping[str, wrappers.MessageType]: @cached_property def resource_messages(self) -> Mapping[str, wrappers.MessageType]: - """Return the file level resources of the proto in a deterministic order.""" + """Return the file level resources of the proto.""" file_resource_messages = ( (res.type, wrappers.CommonResource.build(res).message_type) for res in self.file_pb2.options.Extensions[ @@ -169,12 +169,10 @@ def resource_messages(self) -> Mapping[str, wrappers.MessageType]: for msg in self.messages.values() if msg.options.Extensions[resource_pb2.resource].type ) - - # Sort the chained generator by the resource path (item[0]) to ensure determinism return collections.OrderedDict( - sorted( - itertools.chain(file_resource_messages, resource_messages), - key=lambda item: item[0] + itertools.chain( + file_resource_messages, + resource_messages, ) ) diff --git a/packages/gapic-generator/gapic/schema/wrappers.py b/packages/gapic-generator/gapic/schema/wrappers.py index 283e9434e8ce..f654d0a82d18 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -2240,46 +2240,6 @@ def oauth_scopes(self) -> Sequence[str]: for i in self.options.Extensions[client_pb2.oauth_scopes].split(",") if i ) - - @utils.cached_property - def deterministic_resource_messages(self) -> Sequence[MessageType]: - """Returns a sorted sequence of resource messages to guarantee deterministic output order.""" - return tuple( - sorted( - self.resource_messages, - key=lambda r: r.resource_type_full_path or r.name - ) - ) - - @utils.cached_property - def resource_path_method_names(self) -> Dict[str, str]: - """Returns a mapping of resource_type_full_path to its disambiguated Python method base name.""" - import collections - from gapic import utils - - type_counts = collections.Counter( - r.resource_type for r in self.resource_messages_dict.values() if r.resource_type - ) - - method_names = {} - for full_path, resource in self.resource_messages_dict.items(): - if not resource.resource_type: - continue - - base_name = utils.to_snake_case(resource.resource_type) - - # Collision detection - if type_counts[resource.resource_type] > 1: - domain = full_path.split(".")[0] - - # THE MAGIC FIX: Only prefix if the resource belongs to a foreign API - if domain != self.shortname: - domain_prefix = domain.replace("-", "_") - base_name = f"{domain_prefix}_{base_name}" - - method_names[full_path] = base_name - - return method_names @property def module_name(self) -> str: @@ -2318,13 +2278,14 @@ def names(self) -> FrozenSet[str]: return frozenset(answer) @utils.cached_property - def resource_messages(self) -> Sequence[MessageType]: + def resource_messages(self) -> FrozenSet[MessageType]: """Returns all the resource message types used in all - request and response fields in the service in a deterministic order.""" + request and response fields in the service.""" def gen_resources(message): if message.resource_path: yield message + for type_ in message.recursive_field_types: if type_.resource_path: yield type_ @@ -2333,12 +2294,14 @@ def gen_indirect_resources_used(message): for field in message.recursive_resource_fields: resource = field.options.Extensions[resource_pb2.resource_reference] resource_type = resource.type or resource.child_type + # The resource may not be visible if the resource type is one of + # the common_resources (see the class var in class definition) + # or if it's something unhelpful like '*'. resource = self.visible_resources.get(resource_type) if resource: yield resource - # Collect all resources into an unordered set to remove duplicates - unsorted_resources = set( + return frozenset( msg for method in self.methods.values() for msg in chain( @@ -2352,14 +2315,6 @@ def gen_indirect_resources_used(message): ), ) ) - - # Return them deterministically sorted by their full path - return tuple( - sorted( - unsorted_resources, - key=lambda r: r.resource_type_full_path or r.name - ) - ) @utils.cached_property def resource_messages_dict(self) -> Dict[str, MessageType]: From 42e05279c15ada700a5f1b2a85f455d9b85c930a Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:20:58 +0000 Subject: [PATCH 09/14] resolve undeterministic generation issue --- packages/gapic-generator/gapic/schema/api.py | 9 ++++++--- packages/gapic-generator/gapic/schema/wrappers.py | 13 ++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) 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..e5fb7c12aaa2 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -2278,9 +2278,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 +2301,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 +2316,13 @@ def gen_indirect_resources_used(message): ) ) + return tuple( + sorted( + unique_messages, + key=lambda m: m.resource_type_full_path or m.name + ) + ) + @utils.cached_property def resource_messages_dict(self) -> Dict[str, MessageType]: """Returns a dict from resource reference to From 5849780ed1aaab0c3d939fac3c2d66cdf4c332ac Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:21:50 +0000 Subject: [PATCH 10/14] revert changes --- .../gapic/%name_%version/%sub/test_%service.py.j2 | 13 ++++++------- .../gapic/%name_%version/%sub/test_%service.py.j2 | 13 ++++++------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 index d7a8d33a31ee..39d5fa7d35e7 100644 --- a/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 +++ b/packages/gapic-generator/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 @@ -2382,27 +2382,26 @@ def test_{{ service.name|snake_case }}_grpc_lro_client(): {% endif %} {# if grpc in opts #} {% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") %} -{% for message in service.resource_messages %} -{% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} -def test_{{ method_base_name }}_path(): +{% for message in service.resource_messages|sort(attribute="resource_type") %} +def test_{{ message.resource_type|snake_case }}_path(): {% for arg in message.resource_path_args %} {{ arg }} = "{{ molluscs.next() }}" {% endfor %} expected = "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) - actual = {{ service.client_name }}.{{ method_base_name }}_path({{message.resource_path_args|join(", ") }}) + actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }}) assert expected == actual -def test_parse_{{ method_base_name }}_path(): +def test_parse_{{ message.resource_type|snake_case }}_path(): expected = { {% for arg in message.resource_path_args %} "{{ arg }}": "{{ molluscs.next() }}", {% endfor %} } - path = {{ service.client_name }}.{{ method_base_name }}_path(**expected) + path = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path(**expected) # Check that the path construction is reversible. - actual = {{ service.client_name }}.parse_{{ method_base_name }}_path(path) + actual = {{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path(path) assert expected == actual {% endfor %} diff --git a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 index 280cd77096e8..0a48dd6e1887 100644 --- a/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 +++ b/packages/gapic-generator/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 @@ -1788,27 +1788,26 @@ def test_{{ service.name|snake_case }}_grpc_lro_async_client(): {% endif %} {# if grpc in opts #} {% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") %} -{% for message in service.resource_messages %} -{% set method_base_name = service.resource_path_method_names[message.resource_type_full_path] %} -def test_{{ method_base_name }}_path(): +{% for message in service.resource_messages|sort(attribute="resource_type") %} +def test_{{ message.resource_type|snake_case }}_path(): {% for arg in message.resource_path_args %} {{ arg }} = "{{ molluscs.next() }}" {% endfor %} expected = "{{ message.resource_path_formatted }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) - actual = {{ service.client_name }}.{{ method_base_name }}_path({{message.resource_path_args|join(", ") }}) + actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }}) assert expected == actual -def test_parse_{{ method_base_name }}_path(): +def test_parse_{{ message.resource_type|snake_case }}_path(): expected = { {% for arg in message.resource_path_args %} "{{ arg }}": "{{ molluscs.next() }}", {% endfor %} } - path = {{ service.client_name }}.{{ method_base_name }}_path(**expected) + path = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path(**expected) # Check that the path construction is reversible. - actual = {{ service.client_name }}.parse_{{ method_base_name }}_path(path) + actual = {{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path(path) assert expected == actual {% endfor %} From 3551fc7b0de81008e8f3a23f4be280fc021479db Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:29:30 +0000 Subject: [PATCH 11/14] update tests --- .../gapic-generator/tests/unit/schema/test_api.py | 14 +++++++------- .../tests/unit/schema/wrappers/test_service.py | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/gapic-generator/tests/unit/schema/test_api.py b/packages/gapic-generator/tests/unit/schema/test_api.py index 9d2dab2ec6a1..92a4ed1a6536 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, ), ) @@ -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 From 46146dff662769d87929785e6f36176e9a601ffa Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:35:51 +0000 Subject: [PATCH 12/14] update tests --- packages/gapic-generator/tests/unit/schema/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gapic-generator/tests/unit/schema/test_api.py b/packages/gapic-generator/tests/unit/schema/test_api.py index 92a4ed1a6536..e2e25738b5a2 100644 --- a/packages/gapic-generator/tests/unit/schema/test_api.py +++ b/packages/gapic-generator/tests/unit/schema/test_api.py @@ -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 From 80d0dd9afb882a6f09990c973d814d12903baf69 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 00:38:50 +0000 Subject: [PATCH 13/14] update tests --- packages/gapic-generator/tests/unit/schema/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gapic-generator/tests/unit/schema/test_api.py b/packages/gapic-generator/tests/unit/schema/test_api.py index e2e25738b5a2..9d94e8251523 100644 --- a/packages/gapic-generator/tests/unit/schema/test_api.py +++ b/packages/gapic-generator/tests/unit/schema/test_api.py @@ -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 From 4f36ef0d8c89aa0e75224b528d461ac81934a94e Mon Sep 17 00:00:00 2001 From: ohmayr Date: Thu, 9 Apr 2026 01:33:29 +0000 Subject: [PATCH 14/14] resolve naming collisions --- .../gapic-generator/gapic/schema/wrappers.py | 28 ++++++++- .../unit/schema/wrappers/test_message.py | 63 ++++++++++++++++++- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/packages/gapic-generator/gapic/schema/wrappers.py b/packages/gapic-generator/gapic/schema/wrappers.py index e5fb7c12aaa2..6801baf00a3d 100644 --- a/packages/gapic-generator/gapic/schema/wrappers.py +++ b/packages/gapic-generator/gapic/schema/wrappers.py @@ -685,10 +685,36 @@ def resource_path(self) -> Optional[str]: If there are multiple paths, returns the first one.""" return next(iter(self.options.Extensions[resource_pb2.resource].pattern), None) + def _apply_domain_heuristic(self, raw_type: str) -> str: + """Determines if a resource is foreign and adds a prefix to prevent + [no-redef] AST collisions.""" + if not raw_type: + return "" + + if "/" not in raw_type: + return raw_type + + # Extract the root domain and final resource name, bypassing any nested paths. + # (e.g., "ces.googleapis.com/Project/Location/Tool" -> "ces.googleapis.com" and "Tool") + resource_parts = raw_type.split('/') + domain, short_name = resource_parts[0], resource_parts[-1] + domain_prefix = domain.split('.', 1)[0] + + try: + native_package = self.meta.address.package + # 2. If the domain prefix isn't natively in the package namespace, it's foreign + if domain_prefix and native_package and domain_prefix not in native_package: + return f"{domain_prefix}_{short_name}" + except (AttributeError, TypeError): + # 3. Safe fallback if meta, address, or package are missing/None on this wrapper + pass + + return short_name + @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 + return self._apply_domain_heuristic(resource.type) if resource else None @property def resource_type_full_path(self) -> Optional[str]: diff --git a/packages/gapic-generator/tests/unit/schema/wrappers/test_message.py b/packages/gapic-generator/tests/unit/schema/wrappers/test_message.py index a13f62c02b9b..22c865b4dd27 100644 --- a/packages/gapic-generator/tests/unit/schema/wrappers/test_message.py +++ b/packages/gapic-generator/tests/unit/schema/wrappers/test_message.py @@ -188,7 +188,7 @@ def test_resource_path(): resource.pattern.append("kingdoms/{kingdom}/phyla/{phylum}/classes/{klass}") resource.pattern.append("kingdoms/{kingdom}/divisions/{division}/classes/{klass}") resource.type = "taxonomy.biology.com/Class" - message = make_message("Squid", options=options) + message = make_message("Squid", options=options, package="taxonomy.biology.v1") assert message.resource_path == "kingdoms/{kingdom}/phyla/{phylum}/classes/{klass}" assert message.resource_path_args == ["kingdom", "phylum", "klass"] @@ -201,7 +201,7 @@ def test_resource_path_with_wildcard(): resource.pattern.append("kingdoms/{kingdom}/phyla/{phylum}/classes/{klass=**}") resource.pattern.append("kingdoms/{kingdom}/divisions/{division}/classes/{klass}") resource.type = "taxonomy.biology.com/Class" - message = make_message("Squid", options=options) + message = make_message("Squid", options=options, package="taxonomy.biology.v1") assert ( message.resource_path == "kingdoms/{kingdom}/phyla/{phylum}/classes/{klass=**}" @@ -230,7 +230,7 @@ def test_resource_path_pure_wildcard(): resource = options.Extensions[resource_pb2.resource] resource.pattern.append("*") resource.type = "taxonomy.biology.com/Class" - message = make_message("Squid", options=options) + message = make_message("Squid", options=options, package="taxonomy.biology.v1") # Pure wildcard resource names do not really help construct resources # but they are a part of the spec so we need to support them, which means at @@ -473,3 +473,60 @@ def test_extended_operation_request_response_fields(): actual = poll_request.extended_operation_response_fields assert actual == expected + + +@pytest.mark.parametrize( + "raw_type, package_tuple, expected", + [ + # 1. Empty or malformed inputs + ("", ("google", "cloud", "dialogflow", "v2"), ""), + ("Tool", ("google", "cloud", "dialogflow", "v2"), "Tool"), + + # 2. Native Resources (Prefix 'dialogflow' IS in the package tuple) + ("dialogflow.googleapis.com/Tool", ("google", "cloud", "dialogflow", "v2"), "Tool"), + ("dialogflow.googleapis.com/Project/Location/Tool", ("google", "cloud", "dialogflow", "v2"), "Tool"), + + # 3. Foreign Resources (Prefix 'ces' is NOT in the package tuple) + ("ces.googleapis.com/Tool", ("google", "cloud", "dialogflow", "v2"), "ces_Tool"), + ("ces.googleapis.com/Project/Location/Tool", ("google", "cloud", "dialogflow", "v2"), "ces_Tool"), + ] +) +def test_apply_domain_heuristic(raw_type, package_tuple, expected): + meta = metadata.Metadata( + address=metadata.Address( + name="TestMessage", + package=package_tuple, + module="test", + ) + ) + message = make_message("TestMessage", meta=meta) + + actual = message._apply_domain_heuristic(raw_type) + assert actual == expected + + +def test_apply_domain_heuristic_none_package(): + """Test the EAFP failsafe if package is explicitly None (TypeError).""" + meta = metadata.Metadata( + address=metadata.Address( + name="TestMessage", + package=None, + module="test", + ) + ) + message = make_message("TestMessage", meta=meta) + + actual = message._apply_domain_heuristic("ces.googleapis.com/Tool") + assert actual == "Tool" + + +def test_apply_domain_heuristic_missing_meta(): + """Test the EAFP failsafe if meta or address are missing (AttributeError).""" + # To test the AttributeError fallback cleanly without mocks, we can just + # pass a bare-bones Python class to the unbound method. This perfectly + # proves the try/except block safely handles totally missing attributes. + class EmptyMessage: + pass + + actual = wrappers.MessageType._apply_domain_heuristic(EmptyMessage(), "ces.googleapis.com/Tool") + assert actual == "Tool"