Skip to content

Commit

Permalink
fix: fix resource path args for paths with =** (#1089)
Browse files Browse the repository at this point in the history
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
};
```
  • Loading branch information
busunkim96 authored Nov 18, 2021
1 parent 7ea436f commit 309cc66
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 5 deletions.
7 changes: 6 additions & 1 deletion gapic/samplegen/samplegen.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,)

Expand Down
3 changes: 2 additions & 1 deletion gapic/schema/wrappers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 16 additions & 3 deletions tests/snippetgen/snippets.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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];
Expand Down
21 changes: 21 additions & 0 deletions tests/unit/schema/wrappers/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit 309cc66

Please sign in to comment.