feat: support for proto3 optional fields (#519)

Fields marked as 'optional' in proto files are given an 'optional=True' parameter in their constructor. Bumps required version of proto-plus in gapic surface to 1.1.0 Includes minor cleanups in template code that refers to oneofs.
googleapis · Jul 13, 2020 · 1aa729c · 1aa729c
1 parent 12e9bee
commit 1aa729c
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 13 deletions.
diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/pagers.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/services/%service/pagers.py.j2
@@ -38,7 +38,7 @@ class {{ method.name }}Pager:
             method: Callable[..., {{ method.output.ident }}],
             request: {{ method.input.ident }},
             response: {{ method.output.ident }},
-            metadata: Sequence[Tuple[str, str]] = ())):
+            metadata: Sequence[Tuple[str, str]] = ()):
         """Instantiate the pager.
 
         Args:

diff --git a/gapic/ads-templates/%namespace/%name/%version/%sub/types/_message.py.j2 b/gapic/ads-templates/%namespace/%name/%version/%sub/types/_message.py.j2
@@ -42,8 +42,7 @@ class {{ message.name }}({{ p }}.Message):
     {% endwith -%}
     {% else -%}
     {{ field.name }} = {{ p }}.{% if field.repeated %}Repeated{% endif %}Field(
-        {{- p }}.{{ field.proto_type }}, number={{ field.number }}
-    {% if field.oneof %}, oneof='{{ field.oneof }}'{% endif %}
+        {{- p }}.{{ field.proto_type }}, number={{ field.number }}{% if field.proto3_optional %}, optional=True{% elif field.oneof %}, oneof='{{ field.oneof }}'{% endif %}
     {%- if field.enum or field.message %},
         {{ field.proto_type.lower() }}={{ field.type.ident.rel(message.ident) }},
     {% endif %})

diff --git a/gapic/ads-templates/setup.py.j2 b/gapic/ads-templates/setup.py.j2
@@ -19,7 +19,7 @@ setuptools.setup(
         'google-api-core >= 1.17.0, < 2.0.0dev',
         'googleapis-common-protos >= 1.5.8',
         'grpcio >= 1.10.0',
-        'proto-plus >= 0.4.0',
+        'proto-plus >= 1.1.0',
     {%- if api.requires_package(('google', 'iam', 'v1')) %}
         'grpc-google-iam-v1',
     {%- endif %}

diff --git a/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/gapic/ads-templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
@@ -221,9 +221,9 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
         call.return_value = iter([{{ method.output.ident }}()])
         {% else -%}
         call.return_value = {{ method.output.ident }}(
-            {%- for field in method.output.fields.values() | rejectattr('message') %}{%- for field in method.output.fields.values() | rejectattr('message')%}{% if not field.oneof or field.proto3_optional %}
+            {%- for field in method.output.fields.values() | rejectattr('message')%}{% if not field.oneof or field.proto3_optional %}
             {{ field.name }}={{ field.mock_value }},
-            {%- endfor %}
+            {% endif %}{%- endfor %}
             {#- This is a hack to only pick one field  #}
             {%- for oneof_fields in method.output.oneof_fields().values() %}
             {% with field = oneof_fields[0] %}
@@ -257,14 +257,15 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
         assert isinstance(message, {{ method.output.ident }})
     {% else -%}
     assert isinstance(response, {{ method.client_output.ident }})
-    {% for field in method.output.fields.values() | rejectattr('message') -%}
+    {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not field.oneof or field.proto3_optional %}
     {% if field.field_pb.type in [1, 2] -%} {# Use approx eq for floats -#}
     assert math.isclose(response.{{ field.name }}, {{ field.mock_value }}, rel_tol=1e-6)
     {% elif field.field_pb.type == 8 -%} {# Use 'is' for bools #}
     assert response.{{ field.name }} is {{ field.mock_value }}
     {% else -%}
     assert response.{{ field.name }} == {{ field.mock_value }}
     {% endif -%}
+    {% endif -%} {# end oneof/optional #}
     {% endfor %}
     {% endif %}
 

diff --git a/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2
@@ -42,8 +42,7 @@ class {{ message.name }}({{ p }}.Message):
     {% endwith -%}
     {% else -%}                 {# field.map #}
     {{ field.name }} = {{ p }}.{% if field.repeated %}Repeated{% endif %}Field(
-        {{- p }}.{{ field.proto_type }}, number={{ field.number }}
-    {% if field.oneof %}, oneof='{{ field.oneof }}'{% endif %}
+        {{- p }}.{{ field.proto_type }}, number={{ field.number }}{% if field.proto3_optional %}, optional=True{% elif field.oneof %}, oneof='{{ field.oneof }}'{% endif %}
     {%- if field.enum or field.message %},
         {{ field.proto_type.lower() }}={{ field.type.ident.rel(message.ident) }},
     {% endif %})                {# enum or message #}

diff --git a/gapic/templates/setup.py.j2 b/gapic/templates/setup.py.j2
@@ -18,7 +18,7 @@ setuptools.setup(
     install_requires=(
         'google-api-core[grpc] >= 1.21.0, < 2.0.0dev',
         'libcst >= 0.2.5',
-        'proto-plus >= 0.4.0',
+        'proto-plus >= 1.1.0',
     {%- if api.requires_package(('google', 'iam', 'v1')) %}
         'grpc-google-iam-v1',
     {%- endif %}

diff --git a/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2 b/gapic/templates/tests/unit/gapic/%name_%version/%sub/test_%service.py.j2
@@ -325,7 +325,7 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'):
         assert isinstance(message, {{ method.output.ident }})
     {% else -%}
     assert isinstance(response, {{ method.client_output.ident }})
-    {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not (field.oneof and not field.proto3_optional) %}
+    {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not field.oneof or field.proto3_optional %}
     {% if field.field_pb.type in [1, 2] -%} {# Use approx eq for floats -#}
     assert math.isclose(response.{{ field.name }}, {{ field.mock_value }}, rel_tol=1e-6)
     {% elif field.field_pb.type == 8 -%} {# Use 'is' for bools #}
@@ -376,7 +376,7 @@ async def test_{{ method.name|snake_case }}_async(transport: str = 'grpc_asyncio
             {%- else -%}
                 grpc_helpers_async.FakeStreamUnaryCall
             {%- endif -%}({{ method.output.ident }}(
-            {%- for field in method.output.fields.values() | rejectattr('message') %}{% if not (field.oneof and not field.proto3_optional) %}
+            {%- for field in method.output.fields.values() | rejectattr('message') %}{% if not field.oneof or field.proto3_optional %}
             {{ field.name }}={{ field.mock_value }},
             {%- endif %}
             {%- endfor %}
@@ -409,7 +409,7 @@ async def test_{{ method.name|snake_case }}_async(transport: str = 'grpc_asyncio
     assert isinstance(message, {{ method.output.ident }})
     {% else -%}
     assert isinstance(response, {{ method.client_output_async.ident }})
-    {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not (field.oneof and not field.proto3_optional) %}
+    {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not field.oneof or field.proto3_optional %}
     {% if field.field_pb.type in [1, 2] -%} {# Use approx eq for floats -#}
     assert math.isclose(response.{{ field.name }}, {{ field.mock_value }}, rel_tol=1e-6)
     {% elif field.field_pb.type == 8 -%} {# Use 'is' for bools #}