[WIP] Experiment deterministic resource names

.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

.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}"
+}

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 {}

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 %}

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]
             )
         )
 

packages/gapic-generator/gapic/schema/wrappers.py

@@ -2240,6 +2240,46 @@ 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:
@@ -2278,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_
@@ -2294,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(
@@ -2315,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]:

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)

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.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 {}

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 %}

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

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