fix: updating testing, rest-only generation, & minor bug-fixes (#716)

* fix: updating testing, rest-only generation, & minor bug-fixes

* test: test async client generation

* fix: fixed reserved keyword bug, fixed bugs in gapic tests

* fix: reverted bug causing change to , refactored template tests

* fix: return type mismatch

* fix: reserved keyword issue in

* fix: replace bad regex checks with checks against field_pb type

* Update gapic/templates/noxfile.py.j2

Co-authored-by: Dov Shlachter <dovs@google.com>
Co-authored-by: Bu Sun Kim <8822365+busunkim96@users.noreply.github.com>

gapic/generator/generator.py

@@ -59,6 +59,10 @@ def __init__(self, opts: Options) -> None:
         self._env.filters["wrap"] = utils.wrap
         self._env.filters["coerce_response_name"] = coerce_response_name
 
+        # Add tests to determine type of expressions stored in strings
+        self._env.tests["str_field_pb"] = utils.is_str_field_pb
+        self._env.tests["msg_field_pb"] = utils.is_msg_field_pb
+
         self._sample_configs = opts.sample_configs
 
     def get_response(
@@ -278,6 +282,10 @@ def _render_template(
                         or
                         ('transport' in template_name
                         and not self._is_desired_transport(template_name, opts))
+                        or
+                        # TODO(yon-mg) - remove when rest async implementation resolved
+                        # temporarily stop async client gen while rest async is unkown
+                        ('async' in template_name and 'grpc' not in opts.transport)
                 ):
                     continue
 

gapic/schema/wrappers.py

@@ -411,7 +411,9 @@ def get_field(self, *field_path: str,
         collisions = collisions or self.meta.address.collisions
 
         # Get the first field in the path.
-        cursor = self.fields[field_path[0]]
+        first_field = field_path[0]
+        cursor = self.fields[first_field +
+            ('_' if first_field in utils.RESERVED_NAMES else '')]
 
         # Base case: If this is the last field in the path, return it outright.
         if len(field_path) == 1:
@@ -805,6 +807,7 @@ def filter_fields(sig: str) -> Iterable[Tuple[str, Field]]:
                     continue
                 name = f.strip()
                 field = self.input.get_field(*name.split('.'))
+                name += '_' if field.field_pb.name in utils.RESERVED_NAMES else ''
                 if cross_pkg_request and not field.is_primitive:
                     # This is not a proto-plus wrapped message type,
                     # and setting a non-primitive field directly is verboten.

gapic/templates/%namespace/%name/__init__.py.j2

@@ -12,8 +12,10 @@ from {% if api.naming.module_namespace %}{{ api.naming.module_namespace|join('.'
         if service.meta.address.subpackage == api.subpackage_view -%}
 from {% if api.naming.module_namespace %}{{ api.naming.module_namespace|join('.') }}.{% endif -%}
     {{ api.naming.versioned_module_name }}.services.{{ service.name|snake_case }}.client import {{ service.client_name }}
+{%- if 'grpc' in opts.transport %}
 from {% if api.naming.module_namespace %}{{ api.naming.module_namespace|join('.') }}.{% endif -%}
     {{ api.naming.versioned_module_name }}.services.{{ service.name|snake_case }}.async_client import {{ service.async_client_name }}
+{%- endif %}
 {% endfor -%}
 
 {#  Import messages and enums from each proto.
@@ -50,7 +52,9 @@ __all__ = (
 {% for service in api.services.values()|sort(attribute='name')
    if service.meta.address.subpackage == api.subpackage_view -%}
 '{{ service.client_name }}',
+    {%- if 'grpc' in opts.transport %}
 '{{ service.async_client_name }}',
+    {%- endif %}
 {% endfor -%}
 {% for proto in api.protos.values()|sort(attribute='module_name')
    if proto.meta.address.subpackage == api.subpackage_view -%}

gapic/templates/%namespace/%name_%version/%sub/services/%service/__init__.py.j2

@@ -2,10 +2,14 @@
 
 {% block content %}
 from .client import {{ service.client_name }}
+{%- if 'grpc' in opts.transport %}
 from .async_client import {{ service.async_client_name }}
+{%- endif %}
 
 __all__ = (
     '{{ service.client_name }}',
+    {%- if 'grpc' in opts.transport %}
     '{{ service.async_client_name }}',
+    {%- endif %}
 )
 {% endblock %}

gapic/templates/%namespace/%name_%version/%sub/services/%service/transports/rest.py.j2

@@ -78,7 +78,13 @@ class {{ service.name }}RestTransport({{ service.name }}Transport):
                 Generally, you only need to set this if you're developing	
                 your own client library.
         """
-        super().__init__(host=host, credentials=credentials)
+        # Run the base constructor
+        # TODO(yon-mg): resolve other ctor params i.e. scopes, quota, etc.
+        super().__init__(
+            host=host,
+            credentials=credentials,
+            client_info=client_info,
+        )
         self._session = AuthorizedSession(self._credentials)
         {%- if service.has_lro %}
         self._operations_client = None
@@ -163,7 +169,7 @@ class {{ service.name }}RestTransport({{ service.name }}Transport):
         url = 'https://{host}{{ method.http_opt['url'] }}'.format(
             host=self._host,
             {%- for field in method.path_params %}
-            {{ field }}=request.{{ field }},
+            {{ field }}=request.{{ method.input.get_field(field).name }},
             {%- endfor %}
         )
 
@@ -180,10 +186,8 @@ class {{ service.name }}RestTransport({{ service.name }}Transport):
         # TODO(yon-mg): further discussion needed whether 'python truthiness' is appropriate here
         #               discards default values
         # TODO(yon-mg): add test for proper url encoded strings
-        query_params = ((k, v) for k, v in query_params.items() if v)
-        for i, (param_name, param_value) in enumerate(query_params):
-            q = '?' if i == 0 else '&'
-            url += "{q}{name}={value}".format(q=q, name=param_name, value=param_value.replace(' ', '+'))
+        query_params = ['{k}={v}'.format(k=k, v=v) for k, v in query_params.items() if v]
+        url += '?{}'.format('&'.join(query_params)).replace(' ', '+')
 
         # Send the request
         {% if not method.void %}response = {% endif %}self._session.{{ method.http_opt['verb'] }}(

gapic/templates/noxfile.py.j2

@@ -7,7 +7,7 @@ import shutil
 import nox  # type: ignore
 
 
-@nox.session(python=['3.6', '3.7'])
+@nox.session(python=['3.6', '3.7', '3.8', '3.9'])
 def unit(session):
     """Run the unit test suite."""
 
@@ -21,7 +21,7 @@ def unit(session):
         '--cov-config=.coveragerc',
         '--cov-report=term',
         '--cov-report=html',
-        os.path.join('tests', 'unit',)
+        os.path.join('tests', 'unit', ''.join(session.posargs))
     )