From 309cc66e880e07940866864b03c744310ef56762 Mon Sep 17 00:00:00 2001 From: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com> Date: Thu, 18 Nov 2021 11:34:33 -0700 Subject: [PATCH] fix: fix resource path args for paths with =** (#1089) Some resources have `=**` in the segment. . A segment with `=**` is not matched by the current regex: ```py Python 3.9.5 (default, Jul 27 2021, 22:06:04) [GCC 10.2.1 20210110] on linux Type "help", "copyright", "credits" or "license" for more information. >>> import re >>> pattern = re.compile(r"\{([a-zA-Z0-9_-]+)\}") >>> pattern.search("projects/{project}/metricDescriptors/{metric_descriptor=**}").groups() ('project',) ``` This pattern shows up in the some real APIs ([monitoring protos](https://github.com/googleapis/googleapis/blob/3b9c98eda3bded7bb01e2f5f5c7d20f4a5d3e121/google/monitoring/v3/metric_service.proto#L39-L46), `google/cloud/iap/v1`, `google/cloud/iap/v1beta1`, `google/cloud/recommendationengine/v1beta1`, `google/devtools/remoteworkers/v1test2`, `google/home/graph/v1`, `google/iam/v1`). `**` is mentioned in passing in [Resource Names](https://cloud.google.com/apis/design/resource_names#q_how_should_i_generate_and_parse_resource_names). I was not able to find an explanation of what wildcards are considered valid in https://google.aip.dev/122 or https://google.aip.dev/client-libraries/4231. Monitoring Proto Example: ```proto option (google.api.resource_definition) = { type: "monitoring.googleapis.com/MetricDescriptor" pattern: "projects/{project}/metricDescriptors/{metric_descriptor=**}" pattern: "organizations/{organization}/metricDescriptors/{metric_descriptor=**}" pattern: "folders/{folder}/metricDescriptors/{metric_descriptor=**}" pattern: "*" history: ORIGINALLY_SINGLE_PATTERN }; ``` --- gapic/samplegen/samplegen.py | 7 ++++++- gapic/schema/wrappers.py | 3 ++- ...llusca_v1_snippets_list_resources_async.py | 5 +++++ ...ollusca_v1_snippets_list_resources_sync.py | 5 +++++ tests/snippetgen/snippets.proto | 19 ++++++++++++++--- tests/unit/schema/wrappers/test_message.py | 21 +++++++++++++++++++ 6 files changed, 55 insertions(+), 5 deletions(-) diff --git a/gapic/samplegen/samplegen.py b/gapic/samplegen/samplegen.py index 2a7e68a1f5..ab1b817300 100644 --- a/gapic/samplegen/samplegen.py +++ b/gapic/samplegen/samplegen.py @@ -110,7 +110,7 @@ class TransformedRequest: # Resource patterns look something like # kingdom/{kingdom}/phylum/{phylum}/class/{class} - RESOURCE_RE = re.compile(r"\{([^}/]+)\}") + RESOURCE_RE = wrappers.MessageType.PATH_ARG_RE @classmethod def build( @@ -198,6 +198,11 @@ def build( raise types.NoSuchResourcePattern( f"Resource {resource_typestr} has no pattern with params: {attr_name_str}" ) + # This re-writes + # patterns like: 'projects/{project}/metricDescriptors/{metric_descriptor=**}' + # to 'projects/{project}/metricDescriptors/{metric_descriptor} + # so it can be used in sample code as an f-string. + pattern = cls.RESOURCE_RE.sub(r"{\g<1>}", pattern) return cls(base=base, body=attrs, single=None, pattern=pattern,) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index eef7b9f14c..9dabdfa4da 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -317,7 +317,8 @@ def __getattr__(self, name): class MessageType: """Description of a message (defined with the ``message`` keyword).""" # Class attributes - PATH_ARG_RE = re.compile(r'\{([a-zA-Z0-9_-]+)\}') + # https://google.aip.dev/122 + PATH_ARG_RE = re.compile(r'\{([a-zA-Z0-9_\-]+)(?:=\*\*)?\}') # Instance attributes message_pb: descriptor_pb2.DescriptorProto diff --git a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_async.py b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_async.py index 7688660298..649e56a88b 100644 --- a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_async.py +++ b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_async.py @@ -38,8 +38,13 @@ async def sample_list_resources(): part_id = "part_id_value" parent = f"items/{item_id}/parts/{part_id}" + item_id = "item_id_value" + part_id = "part_id_value" + resource_with_wildcard = f"items/{item_id}/parts/{part_id}" + request = mollusca_v1.ListResourcesRequest( parent=parent, + resource_with_wildcard=resource_with_wildcard, ) # Make the request diff --git a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_sync.py b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_sync.py index f1aea49390..8892452b84 100644 --- a/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_sync.py +++ b/tests/snippetgen/goldens/mollusca_generated_mollusca_v1_snippets_list_resources_sync.py @@ -38,8 +38,13 @@ def sample_list_resources(): part_id = "part_id_value" parent = f"items/{item_id}/parts/{part_id}" + item_id = "item_id_value" + part_id = "part_id_value" + resource_with_wildcard = f"items/{item_id}/parts/{part_id}" + request = mollusca_v1.ListResourcesRequest( parent=parent, + resource_with_wildcard=resource_with_wildcard, ) # Make the request diff --git a/tests/snippetgen/snippets.proto b/tests/snippetgen/snippets.proto index d5bbb9f78b..fbffcea258 100644 --- a/tests/snippetgen/snippets.proto +++ b/tests/snippetgen/snippets.proto @@ -67,9 +67,15 @@ message ListResourcesRequest { (google.api.resource_reference) = { type: "snippets.example.com/Resource" }]; + + string resource_with_wildcard = 2 [ + (google.api.field_behavior) = REQUIRED, + (google.api.resource_reference) = { + type: "snippets.example.com/ResourceWithWildcardSegment" + }]; - int32 page_size = 2; - string page_token = 3; + int32 page_size = 3; + string page_token = 4; } message ListResourcesResponse { @@ -91,10 +97,17 @@ message Resource { pattern: "items/{item_id}/parts/{part_id}" }; string name = 1; +} - +message ResourceWithWildcardSegment { + option (google.api.resource) = { + type: "snippets.example.com/ResourceWithWildcardSegment" + pattern: "items/{item_id}/parts/{part_id=**}" + }; + string name = 1; } + message MessageWithNesting { message NestedMessage { string required_string = 1 [(google.api.field_behavior) = REQUIRED]; diff --git a/tests/unit/schema/wrappers/test_message.py b/tests/unit/schema/wrappers/test_message.py index da21f66ebd..1519fadc67 100644 --- a/tests/unit/schema/wrappers/test_message.py +++ b/tests/unit/schema/wrappers/test_message.py @@ -201,6 +201,27 @@ def test_resource_path(): assert message.resource_type == "Class" +def test_resource_path_with_wildcard(): + options = descriptor_pb2.MessageOptions() + resource = options.Extensions[resource_pb2.resource] + 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) + + assert message.resource_path == "kingdoms/{kingdom}/phyla/{phylum}/classes/{klass=**}" + assert message.resource_path_args == ["kingdom", "phylum", "klass"] + assert message.resource_type == "Class" + assert re.match(message.path_regex_str, + "kingdoms/my-kingdom/phyla/my-phylum/classes/my-klass") + assert re.match(message.path_regex_str, + "kingdoms/my-kingdom/phyla/my-phylum/classes/my-klass/additional-segment") + assert re.match(message.path_regex_str, + "kingdoms/my-kingdom/phyla/my-phylum/classes/") is None + + def test_parse_resource_path(): options = descriptor_pb2.MessageOptions() resource = options.Extensions[resource_pb2.resource]