Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6fc78e5

Browse filesBrowse files
ohmayrohmayr
authored
feat: add --resource-name-alias flag to resolve namespace collisions (googleapis#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
Copy full SHA for 6fc78e5

6 files changed

+191-3Lines changed: 191 additions & 3 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

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

Copy file name to clipboardExpand all lines: packages/gapic-generator/gapic/schema/api.py
+1Lines 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

Collapse file

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

Copy file name to clipboardExpand all lines: packages/gapic-generator/gapic/schema/wrappers.py
+30-1Lines 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
Collapse file

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

Copy file name to clipboardExpand all lines: packages/gapic-generator/gapic/utils/options.py
+36-1Lines 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
Collapse file

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

Copy file name to clipboardExpand all lines: packages/gapic-generator/tests/unit/generator/test_options.py
+26Lines 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
Collapse file

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

Copy file name to clipboardExpand all lines: packages/gapic-generator/tests/unit/schema/wrappers/test_message.py
+41Lines 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"
Collapse file

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

Copy file name to clipboardExpand all lines: packages/gapic-generator/tests/unit/schema/wrappers/test_service.py
+57-1Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.