Skip to content

Commit 6fc78e5

Browse files
authored
feat: add --resource-name-alias flag to resolve namespace collisions (#16769)
Introduces the `--resource-name-alias` CLI flag to allow API owners to safely disambiguate fully qualified resource paths that flatten to identical method names in the generated client. The flag accepts mappings in the format `resource.path/Name:AliasName`. The configuration is parsed natively into the compiler Options and injected into the schema models during construction, avoiding global state mutations. Example usage in BUILD.bazel: ``` --resource-name-alias=ces.googleapis.com/Tool:CesTool ``` Towards: b/505425328
1 parent 6224fb6 commit 6fc78e5

6 files changed

Lines changed: 191 additions & 3 deletions

File tree

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,6 +1689,7 @@ def _load_message(
16891689
documentation=self.docs.get(path, self.EMPTY),
16901690
),
16911691
oneofs=oneofs,
1692+
resource_name_aliases=self.opts.resource_name_aliases,
16921693
)
16931694
return self.proto_messages[address.proto]
16941695

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,7 @@ class MessageType:
520520
default_factory=metadata.Metadata,
521521
)
522522
oneofs: Optional[Mapping[str, "Oneof"]] = None
523+
resource_name_aliases: Mapping[str, str] = dataclasses.field(default_factory=dict)
523524

524525
def __getattr__(self, name):
525526
return getattr(self.message_pb, name)
@@ -688,7 +689,12 @@ def resource_path(self) -> Optional[str]:
688689
@property
689690
def resource_type(self) -> Optional[str]:
690691
resource = self.options.Extensions[resource_pb2.resource]
691-
return resource.type[resource.type.find("/") + 1 :] if resource else None
692+
if not resource.type:
693+
return None
694+
695+
default_type = resource.type[resource.type.find("/") + 1 :]
696+
697+
return self.resource_name_aliases.get(resource.type, default_type)
692698

693699
@property
694700
def resource_type_full_path(self) -> Optional[str]:
@@ -2323,6 +2329,29 @@ def gen_indirect_resources_used(message):
23232329
key=lambda m: m.resource_type_full_path or m.name
23242330
)
23252331

2332+
# Fail-fast collision detection
2333+
seen_types: Dict[str, "MessageType"] = {}
2334+
for msg in sorted_messages:
2335+
res_type = msg.resource_type
2336+
if not res_type:
2337+
# Fail fast if a resource is missing type
2338+
raise ValueError(
2339+
f"\n\nFatal: Message '{msg.name}' defines a resource pattern but is missing a resource type. "
2340+
f"This violates AIP-123 (https://google.aip.dev/123). Please define a 'type' in the google.api.resource option."
2341+
)
2342+
2343+
if res_type in seen_types:
2344+
incumbent = seen_types[res_type]
2345+
raise ValueError(
2346+
f"\n\nFatal: Namespace collision detected for resource type '{res_type}'.\n"
2347+
f"Resources '{incumbent.resource_type_full_path}' and '{msg.resource_type_full_path}' "
2348+
f"both flatten to the exact same method name.\n"
2349+
f"To protect backward compatibility, explicitly alias one of these using "
2350+
f"the `--resource-name-alias` CLI parameter.\n"
2351+
f"Example: --resource-name-alias={msg.resource_type_full_path}:CustomName\n"
2352+
)
2353+
seen_types[res_type] = msg
2354+
23262355
return tuple(sorted_messages)
23272356

23282357
@utils.cached_property

packages/gapic-generator/gapic/utils/options.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ class Options:
5151
rest_numeric_enums: bool = False
5252
proto_plus_deps: Tuple[str, ...] = dataclasses.field(default=("",))
5353
gapic_version: str = "0.0.0"
54+
resource_name_aliases: Dict[str, str] = dataclasses.field(default_factory=dict)
5455

5556
# Class constants
5657
PYTHON_GAPIC_PREFIX: str = "python-gapic-"
@@ -72,7 +73,11 @@ class Options:
7273
# proto plus dependencies delineated by '+'
7374
# For example, 'google.cloud.api.v1+google.cloud.anotherapi.v2'
7475
"proto-plus-deps",
75-
"gapic-version", # A version string following https://peps.python.org/pep-0440
76+
"gapic-version", # A version string following https://peps.python.org/pep-0440,
77+
# Resolves method name collisions by mapping a fully qualified
78+
# resource path to a custom TitleCase alias.
79+
# Format: resource.path/Name:AliasName
80+
"resource-name-alias",
7681
)
7782
)
7883

@@ -188,6 +193,35 @@ def tweak_path(p):
188193
if len(proto_plus_deps):
189194
proto_plus_deps = tuple(proto_plus_deps[0].split("+"))
190195

196+
# Parse the resource name aliases dictionary (Format: "path/to/Resource:AliasName")
197+
resource_name_aliases = {}
198+
raw_aliases = opts.pop("resource-name-alias", [])
199+
200+
# Parse explicitly and safely
201+
for mapping in raw_aliases:
202+
if not mapping:
203+
# We only need to check `not mapping` because the top-level
204+
# opt_string parser already stripped trailing whitespaces
205+
continue
206+
207+
try:
208+
# split(":", 1) ensures we only split on the FIRST colon
209+
res_path, alias_name = mapping.split(":", 1)
210+
211+
clean_path = res_path.strip()
212+
clean_alias = alias_name.strip()
213+
214+
if not clean_path or not clean_alias:
215+
raise ValueError()
216+
217+
resource_name_aliases[clean_path] = clean_alias
218+
219+
except ValueError:
220+
warnings.warn(
221+
f"Ignored malformed resource-name-alias: '{mapping}'. "
222+
"Expected format is 'resource.path/Name:AliasName'."
223+
)
224+
191225
answer = Options(
192226
name=opts.pop("name", [""]).pop(),
193227
namespace=tuple(opts.pop("namespace", [])),
@@ -210,6 +244,7 @@ def tweak_path(p):
210244
rest_numeric_enums=rest_numeric_enums,
211245
proto_plus_deps=proto_plus_deps,
212246
gapic_version=opts.pop("gapic-version", ["0.0.0"]).pop(),
247+
resource_name_aliases=resource_name_aliases,
213248
)
214249

215250
# Note: if we ever need to recursively check directories for sample

packages/gapic-generator/tests/unit/generator/test_options.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,3 +254,29 @@ def test_options_proto_plus_deps():
254254
"google.apps.script.type.slides",
255255
"google.apps.script.type",
256256
)
257+
258+
259+
def test_options_resource_name_aliases():
260+
with mock.patch.object(warnings, "warn") as warn:
261+
opts = Options.build(
262+
"resource-name-alias=ces.googleapis.com/Tool:CesTool,"
263+
"resource-name-alias=workspace.googleapis.com/Tool:WorkspaceTool,"
264+
"resource-name-alias=bad_string_without_colon,"
265+
"resource-name-alias=:MissingPath,"
266+
"resource-name-alias=MissingAlias:,"
267+
"resource-name-alias=" # Covers empty string ("")
268+
)
269+
270+
# Verify the dictionary is perfectly formed
271+
expected = {
272+
"ces.googleapis.com/Tool": "CesTool",
273+
"workspace.googleapis.com/Tool": "WorkspaceTool",
274+
}
275+
assert opts.resource_name_aliases == expected
276+
277+
# We expect exactly 3 warnings:
278+
# 1. bad_string
279+
# 2. :MissingPath
280+
# 3. MissingAlias:
281+
# (The empty ' ' string safely 'continues' without warning, as intended)
282+
assert warn.call_count == 3

packages/gapic-generator/tests/unit/schema/wrappers/test_message.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,44 @@ def test_extended_operation_request_response_fields():
473473

474474
actual = poll_request.extended_operation_response_fields
475475
assert actual == expected
476+
477+
478+
def test_message_type_resource_type_with_alias():
479+
# 1. Cleanly mock the options
480+
opts = descriptor_pb2.MessageOptions()
481+
opts.Extensions[resource_pb2.resource].type = "ces.googleapis.com/Tool"
482+
483+
msg_pb = descriptor_pb2.DescriptorProto(name="MyMessage", options=opts)
484+
485+
# 2. Explicitly instantiate MessageType here instead
486+
# of using `make_message` to inject 'resource_name_aliases' field.
487+
message = wrappers.MessageType(
488+
message_pb=msg_pb,
489+
fields={},
490+
nested_enums={},
491+
nested_messages={},
492+
resource_name_aliases={"ces.googleapis.com/Tool": "CesTool"},
493+
)
494+
495+
# 3. Verify it intercepted the default Protobuf logic
496+
# and returned the alias
497+
assert message.resource_type == "CesTool"
498+
499+
500+
def test_message_type_resource_type_without_alias():
501+
# 1. Cleanly mock the options
502+
opts = descriptor_pb2.MessageOptions()
503+
opts.Extensions[resource_pb2.resource].type = "ces.googleapis.com/Tool"
504+
msg_pb = descriptor_pb2.DescriptorProto(name="MyMessage", options=opts)
505+
506+
# 2. Instantiate WITHOUT any aliases matching this resource
507+
message = wrappers.MessageType(
508+
message_pb=msg_pb,
509+
fields={},
510+
nested_enums={},
511+
nested_messages={},
512+
resource_name_aliases={"some.other/Resource": "OtherAlias"},
513+
)
514+
515+
# 3. Verify it falls back to the default type ("Tool")
516+
assert message.resource_type == "Tool"

packages/gapic-generator/tests/unit/schema/wrappers/test_service.py

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,16 @@
1414

1515
import collections
1616
import itertools
17+
import pytest
1718
import typing
1819

1920
from google.api import resource_pb2
2021
from google.cloud import extended_operations_pb2 as ex_ops_pb2
2122
from google.protobuf import descriptor_pb2
2223

2324
from gapic.schema import imp
24-
from gapic.schema.wrappers import CommonResource
25+
from gapic.schema.wrappers import CommonResource, MessageType
26+
from gapic.schema import wrappers
2527

2628
from test_utils.test_utils import (
2729
get_method,
@@ -693,3 +695,57 @@ def test_extended_operations_lro_detection():
693695
# because Service objects can't perform the lookup.
694696
# Instead we kick that can to the API object and make it do the lookup and verification.
695697
assert lro.operation_service == "CustomOperations"
698+
699+
700+
def test_resource_messages_throws_informative_collision_error():
701+
# 1. Create options with explicit colliding resource types AND valid patterns
702+
opts_1 = descriptor_pb2.MessageOptions()
703+
opts_1.Extensions[resource_pb2.resource].type = "ces.googleapis.com/Tool"
704+
opts_1.Extensions[resource_pb2.resource].pattern.append("ces/{ces}/tool")
705+
706+
opts_2 = descriptor_pb2.MessageOptions()
707+
opts_2.Extensions[resource_pb2.resource].type = "workspace.googleapis.com/Tool"
708+
opts_2.Extensions[resource_pb2.resource].pattern.append("workspaces/{workspace}/tool")
709+
710+
# 2. Use the test helpers to build the messages
711+
msg_1 = make_message("MessageOne", options=opts_1)
712+
msg_2 = make_message("MessageTwo", options=opts_2)
713+
714+
# 3. Build the service with BOTH methods (so the property loops)
715+
# AND visible_resources (so the internal lookups succeed).
716+
service = make_service(
717+
name="MyService",
718+
methods=(
719+
make_method("GetToolOne", input_message=msg_1),
720+
make_method("GetToolTwo", input_message=msg_2),
721+
),
722+
visible_resources={
723+
"ces.googleapis.com/Tool": msg_1,
724+
"workspace.googleapis.com/Tool": msg_2,
725+
}
726+
)
727+
728+
# 4. Assert that asking the service for its resources throws the custom error
729+
expected_error_regex = r"(?s)Namespace collision detected.*--resource-name-alias"
730+
731+
with pytest.raises(ValueError, match=expected_error_regex):
732+
_ = service.resource_messages
733+
734+
735+
def test_resource_messages_raises_on_malformed_typeless_resource():
736+
# 1. Create a malformed resource: it has a pattern, but no type!
737+
opts = descriptor_pb2.MessageOptions()
738+
opts.Extensions[resource_pb2.resource].pattern.append("workspaces/{workspace}")
739+
740+
malformed_msg = make_message("MalformedMessage", options=opts)
741+
742+
service = make_service(
743+
name="MyService",
744+
methods=(
745+
make_method("DoThing", input_message=malformed_msg),
746+
)
747+
)
748+
749+
# 2. Trigger the property and expect it to fail fast with the AIP-123 URL
750+
with pytest.raises(ValueError, match="https://google.aip.dev/123"):
751+
_ = service.resource_messages

0 commit comments

Comments
 (0)