From 5f4c3009ec17399ab24a52a3b5bc9cbd0d9a2bf6 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Wed, 15 Apr 2020 11:59:48 -0700 Subject: [PATCH] Test and impl for resource path parsing methods in generated clients (#391) 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. --- .../%sub/services/%service/client.py.j2 | 6 +++ .../%name_%version/%sub/test_%service.py.j2 | 15 ++++++- gapic/schema/wrappers.py | 32 ++++++++++++++- .../%sub/services/%service/client.py.j2 | 6 +++ .../%name_%version/%sub/test_%service.py.j2 | 27 ++++++++---- tests/unit/schema/wrappers/test_message.py | 41 +++++++++++++++++++ 6 files changed, 117 insertions(+), 10 deletions(-) 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/schema/wrappers.py b/gapic/schema/wrappers.py index 09744ac3d9..1c0ae4d85a 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,32 @@ 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: + # 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( + # 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 '' + ) + + "$" + ) + 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..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 @@ -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/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(