From 65c6c7c6ebf8dcc0702f4622712c58b1082de30f Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 14 Apr 2020 11:28:17 -0700 Subject: [PATCH 1/3] Test and impl for resource path parsing methods in generated clients Given a fully qualified resource path, it is sometimes desirable to parse out the component segments. This change adds generated methods to do this parsing to gapic client classes, accompanying generated unit tests, logic in the wrapper schema to support this feature, and generator unit tests for the schema logic. --- gapic/schema/wrappers.py | 34 ++++++++++++++- .../%sub/services/%service/client.py.j2 | 5 +++ .../%name_%version/%sub/test_%service.py.j2 | 27 ++++++++---- tests/unit/schema/wrappers/test_message.py | 41 +++++++++++++++++++ 4 files changed, 98 insertions(+), 9 deletions(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 09744ac3d9..97c29275e4 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -209,6 +209,10 @@ def with_context(self, *, collisions: FrozenSet[str]) -> 'Field': @dataclasses.dataclass(frozen=True) class MessageType: """Description of a message (defined with the ``message`` keyword).""" + # Class attributes + PATH_ARG_RE = re.compile(r'\{([a-zA-Z0-9_-]+)\}') + + # Instance attributes message_pb: descriptor_pb2.DescriptorProto fields: Mapping[str, Field] nested_enums: Mapping[str, 'EnumType'] @@ -278,8 +282,34 @@ def resource_type(self) -> Optional[str]: @property def resource_path_args(self) -> Sequence[str]: - path_arg_re = re.compile(r'\{([a-zA-Z0-9_-]+)\}') - return path_arg_re.findall(self.resource_path or '') + return self.PATH_ARG_RE.findall(self.resource_path or '') + + @utils.cached_property + def path_regex_str(self) -> str: + def gen_component_re(m: re.Match) -> str: + # We can't just use (?P[^/]+) because segments may be + # separated by delimiters other than '/'. + # Multiple delimiter characters within one schema are allowed, e.g. + # as/{a}-{b}/cs/{c}%{d}_{e} + # This is discouraged but permitted by AIP4231 + return "(?P<{name}>.+?)".format(name=m.groups()[0]) + + # The indirection here is a little confusing: + # we're using the resource path template as the base of a regex, + # with each resource ID segment being captured by a regex. + # E.g., the path schema + # kingdoms/{kingdom}/phyla/{phylum} + # becomes the regex + # ^kingdoms/(?P.+?)/phyla/(?P.+?)$ + parsing_regex_str = ( + "^" + + self.PATH_ARG_RE.sub( + gen_component_re, + self.resource_path or '' + ) + + "$" + ) + return parsing_regex_str def get_field(self, *field_path: str, collisions: FrozenSet[str] = frozenset()) -> Field: diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index 5c5fd5157b..52319e0b1e 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -120,6 +120,11 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): """Return a fully-qualified {{ message.resource_type|snake_case }} string.""" return "{{ message.resource_path }}".format({% for arg in message.resource_path_args %}{{ arg }}={{ arg }}, {% endfor %}) + + def parse_{{ message.resource_type|snake_case }}_path(path: str) -> Dict[str,str]: + """Parse 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 {} {% endfor %} def __init__(self, *, diff --git a/gapic/templates/tests/unit/%name_%version/%sub/test_%service.py.j2 b/gapic/templates/tests/unit/%name_%version/%sub/test_%service.py.j2 index 4e3e89a329..aac51fe791 100644 --- a/gapic/templates/tests/unit/%name_%version/%sub/test_%service.py.j2 +++ b/gapic/templates/tests/unit/%name_%version/%sub/test_%service.py.j2 @@ -692,14 +692,27 @@ def test_{{ service.name|snake_case }}_grpc_lro_client(): {% endif -%} {% for message in service.resource_messages -%} -{% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle") -%} +{% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") -%} def test_{{ message.resource_type|snake_case }}_path(): - {% for arg in message.resource_path_args -%} - {{ arg }} = "{{ molluscs.next() }}" - {% endfor %} - expected = "{{ message.resource_path }}".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(", ") }}) - assert expected == actual + {% for arg in message.resource_path_args -%} + {{ arg }} = "{{ molluscs.next() }}" + {% endfor %} + expected = "{{ message.resource_path }}".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(", ") }}) + assert expected == actual + + +def test_parse_{{ message.resource_type|snake_case }}_path(): + expected = { + {% for arg in message.resource_path_args -%} + "{{ arg }}": "{{ molluscs.next() }}", + {% endfor %} + } + path = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path(**expected) + + # Check that the path construction is reversible. + actual = {{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path(path) + assert expected == actual {% endwith -%} {% endfor -%} diff --git a/tests/unit/schema/wrappers/test_message.py b/tests/unit/schema/wrappers/test_message.py index 99b0751e05..7ae95d0299 100644 --- a/tests/unit/schema/wrappers/test_message.py +++ b/tests/unit/schema/wrappers/test_message.py @@ -13,6 +13,7 @@ # limitations under the License. import collections +import re from typing import Sequence, Tuple import pytest @@ -181,6 +182,46 @@ def test_resource_path(): assert message.resource_type == "Class" +def test_parse_resource_path(): + options = descriptor_pb2.MessageOptions() + resource = options.Extensions[resource_pb2.resource] + resource.pattern.append( + "kingdoms/{kingdom}/phyla/{phylum}/classes/{klass}" + ) + resource.type = "taxonomy.biology.com/Klass" + message = make_message('Klass', options=options) + + # Plausible resource ID path + path = "kingdoms/animalia/phyla/mollusca/classes/cephalopoda" + expected = { + 'kingdom': 'animalia', + 'phylum': 'mollusca', + 'klass': 'cephalopoda', + } + actual = re.match(message.path_regex_str, path).groupdict() + + assert expected == actual + + options2 = descriptor_pb2.MessageOptions() + resource2 = options2.Extensions[resource_pb2.resource] + resource2.pattern.append( + "kingdoms-{kingdom}_{phylum}#classes%{klass}" + ) + resource2.type = "taxonomy.biology.com/Klass" + message2 = make_message('Klass', options=options2) + + # Plausible resource ID path from a non-standard schema + path2 = "kingdoms-Animalia/_Mollusca~#classes%Cephalopoda" + expected2 = { + 'kingdom': 'Animalia/', + 'phylum': 'Mollusca~', + 'klass': 'Cephalopoda', + } + actual2 = re.match(message2.path_regex_str, path2).groupdict() + + assert expected2 == actual2 + + def test_field_map(): # Create an Entry message. entry_msg = make_message( From 0bf275fa3e52edec3bcc4620afe91ac3b848cc62 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Wed, 15 Apr 2020 10:36:38 -0700 Subject: [PATCH 2/3] Fix for python 3.6 --- gapic/schema/wrappers.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 97c29275e4..1c0ae4d85a 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -286,14 +286,6 @@ def resource_path_args(self) -> Sequence[str]: @utils.cached_property def path_regex_str(self) -> str: - def gen_component_re(m: re.Match) -> str: - # We can't just use (?P[^/]+) because segments may be - # separated by delimiters other than '/'. - # Multiple delimiter characters within one schema are allowed, e.g. - # as/{a}-{b}/cs/{c}%{d}_{e} - # This is discouraged but permitted by AIP4231 - return "(?P<{name}>.+?)".format(name=m.groups()[0]) - # The indirection here is a little confusing: # we're using the resource path template as the base of a regex, # with each resource ID segment being captured by a regex. @@ -304,7 +296,13 @@ def gen_component_re(m: re.Match) -> str: parsing_regex_str = ( "^" + self.PATH_ARG_RE.sub( - gen_component_re, + # We can't just use (?P[^/]+) because segments may be + # separated by delimiters other than '/'. + # Multiple delimiter characters within one schema are allowed, + # e.g. + # as/{a}-{b}/cs/{c}%{d}_{e} + # This is discouraged but permitted by AIP4231 + lambda m: "(?P<{name}>.+?)".format(name=m.groups()[0]), self.resource_path or '' ) + "$" From c52d9521d9e8c690762a3ec95a6d11ee2c29617e Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Wed, 15 Apr 2020 10:45:32 -0700 Subject: [PATCH 3/3] Add path parsing tests to ads templates --- .../%version/%sub/services/%service/client.py.j2 | 6 ++++++ .../unit/%name_%version/%sub/test_%service.py.j2 | 15 ++++++++++++++- .../%sub/services/%service/client.py.j2 | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 index 7f5cf6b73f..ff7e2c4d48 100644 --- a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 +++ b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/client.py.j2 @@ -120,6 +120,12 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): """Return a fully-qualified {{ message.resource_type|snake_case }} string.""" return "{{ message.resource_path }}".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]: + """Parse 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 {} {% endfor %} def __init__(self, *, diff --git a/gapic/ads-templates/tests/unit/%name_%version/%sub/test_%service.py.j2 b/gapic/ads-templates/tests/unit/%name_%version/%sub/test_%service.py.j2 index 32e66ba8ad..5cd0019b59 100644 --- a/gapic/ads-templates/tests/unit/%name_%version/%sub/test_%service.py.j2 +++ b/gapic/ads-templates/tests/unit/%name_%version/%sub/test_%service.py.j2 @@ -693,7 +693,7 @@ def test_{{ service.name|snake_case }}_grpc_lro_client(): {% endif -%} {% for message in service.resource_messages -%} -{% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle") -%} +{% with molluscs = cycler("squid", "clam", "whelk", "octopus", "oyster", "nudibranch", "cuttlefish", "mussel", "winkle", "nautilus", "scallop", "abalone") -%} def test_{{ message.resource_type|snake_case }}_path(): {% for arg in message.resource_path_args -%} {{ arg }} = "{{ molluscs.next() }}" @@ -702,6 +702,19 @@ def test_{{ message.resource_type|snake_case }}_path(): actual = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path({{message.resource_path_args|join(", ") }}) assert expected == actual + +def test_parse_{{ message.resource_type|snake_case }}_path(): + expected = { + {% for arg in message.resource_path_args -%} + "{{ arg }}": "{{ molluscs.next() }}", + {% endfor %} + } + path = {{ service.client_name }}.{{ message.resource_type|snake_case }}_path(**expected) + + # Check that the path construction is reversible. + actual = {{ service.client_name }}.parse_{{ message.resource_type|snake_case }}_path(path) + assert expected == actual + {% endwith -%} {% endfor -%} diff --git a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 index 52319e0b1e..4099fe5b10 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/services/%service/client.py.j2 @@ -121,6 +121,7 @@ class {{ service.client_name }}(metaclass={{ service.client_name }}Meta): return "{{ message.resource_path }}".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]: """Parse a {{ message.resource_type|snake_case }} path into its component segments.""" m = re.match(r"{{ message.path_regex_str }}", path)