From 1cbae6a24587e1757035b852ce31a5b86e194b69 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 25 Jun 2020 15:41:55 -0700 Subject: [PATCH 01/25] fix: add oneof fields to genereated protoplus init --- .../%namespace/%name_%version/%sub/types/_message.py.j2 | 1 + 1 file changed, 1 insertion(+) diff --git a/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 index e9586108c0..e71ea0f70f 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 @@ -43,6 +43,7 @@ class {{ message.name }}({{ p }}.Message): {% else -%} {{ field.name }} = {{ p }}.{% if field.repeated %}Repeated{% endif %}Field( {{- p }}.{{ field.proto_type }}, number={{ field.number }} + {% if field.oneof %}, oneof='{{ field.oneof }}'{% endif %} {%- if field.enum or field.message %}, {{ field.proto_type.lower() }}={{ field.type.ident.rel(message.ident) }}, {% endif %}) From f104ba0eafa97f5a1d68faeaeb9f4e33ff97b7bf Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 25 Jun 2020 16:58:35 -0700 Subject: [PATCH 02/25] Add a one of field to the Field dataclass --- gapic/schema/wrappers.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 5a632fcc02..edce4ca763 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -54,6 +54,8 @@ class Field: meta: metadata.Metadata = dataclasses.field( default_factory=metadata.Metadata, ) + oneof:bool = False + def __getattr__(self, name): return getattr(self.field_pb, name) From 1f8ee9cffa28c19c7fa771c07d5cbe3f6985fe04 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 25 Jun 2020 17:03:33 -0700 Subject: [PATCH 03/25] style --- gapic/schema/wrappers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index edce4ca763..434c5c8e63 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -54,7 +54,7 @@ class Field: meta: metadata.Metadata = dataclasses.field( default_factory=metadata.Metadata, ) - oneof:bool = False + oneof: bool = False def __getattr__(self, name): From 7438862c3381aa33bd2c490bfd39180933ee0a41 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 25 Jun 2020 17:11:39 -0700 Subject: [PATCH 04/25] style --- gapic/schema/wrappers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 434c5c8e63..2e95b9cf9c 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -56,7 +56,6 @@ class Field: ) oneof: bool = False - def __getattr__(self, name): return getattr(self.field_pb, name) From 593f64775e7bde0e4c52a897e5e7500d4c4de44e Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Thu, 25 Jun 2020 23:11:44 -0700 Subject: [PATCH 05/25] str type for oneof --- gapic/schema/wrappers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 2e95b9cf9c..04d062ad7a 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -54,7 +54,7 @@ class Field: meta: metadata.Metadata = dataclasses.field( default_factory=metadata.Metadata, ) - oneof: bool = False + oneof: str = None def __getattr__(self, name): return getattr(self.field_pb, name) From 114a277f63656a2bea5007edfe677a40ca71cffe Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 26 Jun 2020 09:59:48 -0700 Subject: [PATCH 06/25] add field oneof to ads templates --- .../%namespace/%name/%version/%sub/types/_message.py.j2 | 1 + 1 file changed, 1 insertion(+) 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 index a8119827b8..15bf3ea4ab 100644 --- a/gapic/ads-templates/%namespace/%name/%version/%sub/types/_message.py.j2 +++ b/gapic/ads-templates/%namespace/%name/%version/%sub/types/_message.py.j2 @@ -43,6 +43,7 @@ class {{ message.name }}({{ p }}.Message): {% else -%} {{ field.name }} = {{ p }}.{% if field.repeated %}Repeated{% endif %}Field( {{- p }}.{{ field.proto_type }}, number={{ field.number }} + {% if field.oneof %}, oneof='{{ field.oneof }}'{% endif %} {%- if field.enum or field.message %}, {{ field.proto_type.lower() }}={{ field.type.ident.rel(message.ident) }}, {% endif %}) From 589e914cadac3ddcf4d6e81dc9c6c52cad83cf55 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Fri, 26 Jun 2020 10:04:18 -0700 Subject: [PATCH 07/25] mypy: specify oneof as optional --- gapic/schema/wrappers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 04d062ad7a..a4add5084f 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -54,7 +54,7 @@ class Field: meta: metadata.Metadata = dataclasses.field( default_factory=metadata.Metadata, ) - oneof: str = None + oneof: Optional[str] = None def __getattr__(self, name): return getattr(self.field_pb, name) From c912534a8920020e4dd94bf9b13840d3a81f817b Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 29 Jun 2020 17:04:29 -0700 Subject: [PATCH 08/25] add oneofs to schema --- gapic/schema/api.py | 49 ++++++++++++++++++++++++++++++++++++---- gapic/schema/wrappers.py | 10 ++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index cc9b9cf1f0..5066af1bd0 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -556,14 +556,43 @@ def _load_children(self, answer[wrapped.name] = wrapped return answer + def _get_oneofs(self, + oneof_pbs: Sequence[descriptor_pb2.OneofDescriptorProto], + address: metadata.Address, path: Tuple[int, ...], + ) -> Dict[str, wrappers.Oneof]: + """Return a dictionary of wrapped oneofs for the given message. + + Args: + oneof_fields (Sequence[~.descriptor_pb2.OneofDescriptorProto]): A + sequence of protobuf field objects. + address (~.metadata.Address): An address object denoting the + location of these oneofs. + path (Tuple[int]): The source location path thus far, as + understood by ``SourceCodeInfo.Location``. + + Returns: + Mapping[str, ~.wrappers.Oneof]: A ordered mapping of + :class:`~.wrappers.Oneof` objects. + """ + # Iterate over the oneofs and collect them into a dictionary. + answer: Dict[str, wrappers.Oneof] = collections.OrderedDict() + for oneof_pb, i in zip(oneof_pbs, range(0, sys.maxsize)): + answer[oneof_pb.name] = wrappers.Oneof( + oneof_pb=oneof_pb, + ) + + # Done; return the answer. + return answer + def _get_fields(self, field_pbs: Sequence[descriptor_pb2.FieldDescriptorProto], address: metadata.Address, path: Tuple[int, ...], + oneofs = None ) -> Dict[str, wrappers.Field]: """Return a dictionary of wrapped fields for the given message. Args: - fields (Sequence[~.descriptor_pb2.FieldDescriptorProto]): A + field_pbs (Sequence[~.descriptor_pb2.FieldDescriptorProto]): A sequence of protobuf field objects. address (~.metadata.Address): An address object denoting the location of these fields. @@ -586,14 +615,19 @@ def _get_fields(self, # `_load_message` method. answer: Dict[str, wrappers.Field] = collections.OrderedDict() for field_pb, i in zip(field_pbs, range(0, sys.maxsize)): + oneof_name = None + if getattr(field_pb, 'oneof_index', -1) >= 0 and oneofs: + oneof_name = list(oneofs.keys())[field_pb.oneof_index] + answer[field_pb.name] = wrappers.Field( field_pb=field_pb, enum=self.api_enums.get(field_pb.type_name.lstrip('.')), message=self.api_messages.get(field_pb.type_name.lstrip('.')), - meta=metadata.Metadata( + metaexit=metadata.Metadata( address=address.child(field_pb.name, path + (i,)), documentation=self.docs.get(path + (i,), self.EMPTY), ), + oneof=oneof_name, ) # Done; return the answer. @@ -779,19 +813,25 @@ def _load_message(self, loader=self._load_message, path=path + (3,), ) - # self._load_children(message.oneof_decl, loader=self._load_field, - # address=nested_addr, info=info.get(8, {})) + + oneofs = self._get_oneofs( + message_pb.oneof_decl, + address=address, + path=path + (8,), + ) # Create a dictionary of all the fields for this message. fields = self._get_fields( message_pb.field, address=address, path=path + (2,), + oneofs=oneofs, ) fields.update(self._get_fields( message_pb.extension, address=address, path=path + (6,), + oneofs=oneofs, )) # Create a message correspoding to this descriptor. @@ -804,6 +844,7 @@ def _load_message(self, address=address, documentation=self.docs.get(path, self.EMPTY), ), + oneofs=oneofs, ) return self.proto_messages[address.proto] diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index a4add5084f..7c6c5ac187 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -207,6 +207,15 @@ def with_context(self, *, collisions: FrozenSet[str]) -> 'Field': ) +@dataclasses.dataclass(frozen=True) +class Oneof: + """Description of a field.""" + oneof_pb: descriptor_pb2.OneofDescriptorProto + + def __getattr__(self, name): + return getattr(self.oneof_pb, name) + + @dataclasses.dataclass(frozen=True) class MessageType: """Description of a message (defined with the ``message`` keyword).""" @@ -221,6 +230,7 @@ class MessageType: meta: metadata.Metadata = dataclasses.field( default_factory=metadata.Metadata, ) + oneofs: Optional[List[str]] = None def __getattr__(self, name): return getattr(self.message_pb, name) From afa357835521ecfa32aab0f4038a71c42a04afd8 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 29 Jun 2020 17:14:05 -0700 Subject: [PATCH 09/25] exit oops --- gapic/schema/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 5066af1bd0..f8dae18418 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -623,7 +623,7 @@ def _get_fields(self, field_pb=field_pb, enum=self.api_enums.get(field_pb.type_name.lstrip('.')), message=self.api_messages.get(field_pb.type_name.lstrip('.')), - metaexit=metadata.Metadata( + meta=metadata.Metadata( address=address.child(field_pb.name, path + (i,)), documentation=self.docs.get(path + (i,), self.EMPTY), ), From 9a5e6c511ed176ed46695b7228b6a1e0d1d8f387 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Mon, 29 Jun 2020 19:23:28 -0700 Subject: [PATCH 10/25] mypy and lint fixes --- gapic/schema/api.py | 4 ++-- gapic/schema/wrappers.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index f8dae18418..bfce262c99 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -587,7 +587,7 @@ def _get_oneofs(self, def _get_fields(self, field_pbs: Sequence[descriptor_pb2.FieldDescriptorProto], address: metadata.Address, path: Tuple[int, ...], - oneofs = None + oneofs=None ) -> Dict[str, wrappers.Field]: """Return a dictionary of wrapped fields for the given message. @@ -617,7 +617,7 @@ def _get_fields(self, for field_pb, i in zip(field_pbs, range(0, sys.maxsize)): oneof_name = None if getattr(field_pb, 'oneof_index', -1) >= 0 and oneofs: - oneof_name = list(oneofs.keys())[field_pb.oneof_index] + oneof_name = list(oneofs.keys())[field_pb.oneof_index] answer[field_pb.name] = wrappers.Field( field_pb=field_pb, diff --git a/gapic/schema/wrappers.py b/gapic/schema/wrappers.py index 7c6c5ac187..34d68655db 100644 --- a/gapic/schema/wrappers.py +++ b/gapic/schema/wrappers.py @@ -230,7 +230,7 @@ class MessageType: meta: metadata.Metadata = dataclasses.field( default_factory=metadata.Metadata, ) - oneofs: Optional[List[str]] = None + oneofs: Optional[Mapping[str, 'Oneof']] = None def __getattr__(self, name): return getattr(self.message_pb, name) From cf31e2a643475e0ee03d8237c1d1317657cfb1b8 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Tue, 30 Jun 2020 12:40:13 -0700 Subject: [PATCH 11/25] add tests for oneofs --- test_utils/test_utils.py | 13 +++++++- tests/unit/schema/test_api.py | 39 ++++++++++++++++++++++++ tests/unit/schema/wrappers/test_field.py | 7 +++++ 3 files changed, 58 insertions(+), 1 deletion(-) diff --git a/test_utils/test_utils.py b/test_utils/test_utils.py index 697d08e8df..89fd735142 100644 --- a/test_utils/test_utils.py +++ b/test_utils/test_utils.py @@ -200,6 +200,7 @@ def make_field( message: wrappers.MessageType = None, enum: wrappers.EnumType = None, meta: metadata.Metadata = None, + oneof: str = None, **kwargs ) -> wrappers.Field: T = desc.FieldDescriptorProto.Type @@ -223,11 +224,13 @@ def make_field( number=number, **kwargs ) + return wrappers.Field( field_pb=field_pb, enum=enum, message=message, meta=meta or metadata.Metadata(), + oneof=oneof, ) @@ -322,20 +325,28 @@ def make_enum_pb2( def make_message_pb2( name: str, fields: tuple = (), + oneof_decl: tuple = (), **kwargs ) -> desc.DescriptorProto: - return desc.DescriptorProto(name=name, field=fields, **kwargs) + return desc.DescriptorProto(name=name, field=fields, oneof_decl=oneof_decl, **kwargs) def make_field_pb2(name: str, number: int, type: int = 11, # 11 == message type_name: str = None, + oneof_index: int = None ) -> desc.FieldDescriptorProto: return desc.FieldDescriptorProto( name=name, number=number, type=type, type_name=type_name, + oneof_index=oneof_index, + ) + +def make_oneof_pb2(name: str) -> desc.OneofDescriptorProto: + return desc.OneofDescriptorProto( + name=name, ) diff --git a/tests/unit/schema/test_api.py b/tests/unit/schema/test_api.py index 8dc1760cd8..fb98f4bb6e 100644 --- a/tests/unit/schema/test_api.py +++ b/tests/unit/schema/test_api.py @@ -34,6 +34,7 @@ make_file_pb2, make_message_pb2, make_naming, + make_oneof_pb2, ) @@ -238,6 +239,44 @@ def test_proto_keyword_fname(): 'class__.proto', } +def test_proto_oneof(): + # Put together a couple of minimal protos. + fd = ( + make_file_pb2( + name='dep.proto', + package='google.dep', + messages=(make_message_pb2(name='ImportedMessage', fields=()),), + ), + make_file_pb2( + name='foo.proto', + package='google.example.v1', + messages=( + make_message_pb2(name='Foo', fields=()), + make_message_pb2( + name='Bar', + fields=( + make_field_pb2(name='imported_message', number=1, + type_name='.google.dep.ImportedMessage', + oneof_index=0), + make_field_pb2( + name='primitive', number=2, type=1, oneof_index=0), + ), + oneof_decl=( + make_oneof_pb2(name="value_type"), + ) + ) + ) + ) + ) + + # Create an API with those protos. + api_schema = api.API.build(fd, package='google.example.v1') + proto = api_schema.protos['foo.proto'] + assert proto.names == {'imported_message', 'Bar', 'primitive', 'Foo'} + oneofs = proto.messages["google.example.v1.Bar"].oneofs + assert len(oneofs) == 1 + assert "value_type" in oneofs.keys() + def test_proto_names_import_collision(): # Put together a couple of minimal protos. diff --git a/tests/unit/schema/wrappers/test_field.py b/tests/unit/schema/wrappers/test_field.py index 7104c735be..3cdcaf9bcf 100644 --- a/tests/unit/schema/wrappers/test_field.py +++ b/tests/unit/schema/wrappers/test_field.py @@ -153,6 +153,13 @@ def test_mock_value_int(): assert field.mock_value == '728' +def test_oneof(): + REP = descriptor_pb2.FieldDescriptorProto.Label.Value('LABEL_REPEATED') + + field = make_field(oneof="oneof_name") + assert field.oneof == "oneof_name" + + def test_mock_value_float(): field = make_field(name='foo_bar', type='TYPE_DOUBLE') assert field.mock_value == '0.728' From 3b90426576f6dc02f82d1ae8a2392d3166637604 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Tue, 30 Jun 2020 12:41:42 -0700 Subject: [PATCH 12/25] lint --- tests/unit/schema/test_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/schema/test_api.py b/tests/unit/schema/test_api.py index fb98f4bb6e..b3f023054c 100644 --- a/tests/unit/schema/test_api.py +++ b/tests/unit/schema/test_api.py @@ -239,6 +239,7 @@ def test_proto_keyword_fname(): 'class__.proto', } + def test_proto_oneof(): # Put together a couple of minimal protos. fd = ( From 70cfe159776aacd4b47b49b60f516e2a7f7845e6 Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Tue, 30 Jun 2020 12:50:32 -0700 Subject: [PATCH 13/25] typing and test --- gapic/schema/api.py | 11 ++++--- tests/unit/schema/wrappers/test_oneof.py | 37 ++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 tests/unit/schema/wrappers/test_oneof.py diff --git a/gapic/schema/api.py b/gapic/schema/api.py index bfce262c99..3422f45d80 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -575,11 +575,10 @@ def _get_oneofs(self, :class:`~.wrappers.Oneof` objects. """ # Iterate over the oneofs and collect them into a dictionary. - answer: Dict[str, wrappers.Oneof] = collections.OrderedDict() - for oneof_pb, i in zip(oneof_pbs, range(0, sys.maxsize)): - answer[oneof_pb.name] = wrappers.Oneof( - oneof_pb=oneof_pb, - ) + answer = collections.OrderedDict( + (oneof_pb.name, wrappers.Oneof(oneof_pb=oneof_pb)) + for i, oneof_pb in enumerate(oneof_pbs) + ) # Done; return the answer. return answer @@ -587,7 +586,7 @@ def _get_oneofs(self, def _get_fields(self, field_pbs: Sequence[descriptor_pb2.FieldDescriptorProto], address: metadata.Address, path: Tuple[int, ...], - oneofs=None + oneofs: Optional[Dict[str, wrappers.Oneof]]=None ) -> Dict[str, wrappers.Field]: """Return a dictionary of wrapped fields for the given message. diff --git a/tests/unit/schema/wrappers/test_oneof.py b/tests/unit/schema/wrappers/test_oneof.py new file mode 100644 index 0000000000..d772846e12 --- /dev/null +++ b/tests/unit/schema/wrappers/test_oneof.py @@ -0,0 +1,37 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import collections + +import pytest + +from google.api import field_behavior_pb2 +from google.protobuf import descriptor_pb2 + +from gapic.schema import metadata +from gapic.schema import wrappers + +from test_utils.test_utils import ( + #make_field, + #make_message, + make_oneof_pb2, +) + + +def test_wrapped_oneof(): + oneof_pb = make_oneof_pb2("oneof_name") + wrapped = wrappers.Oneof(oneof_pb=oneof_pb) + + assert wrapped.oneof_pb == oneof_pb + assert wrapped.name == oneof_pb.name From 58d3f97a807699363ad7e79dc08ebfdb994d2afd Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Tue, 30 Jun 2020 12:53:33 -0700 Subject: [PATCH 14/25] style --- gapic/schema/api.py | 2 +- tests/unit/schema/wrappers/test_oneof.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 3422f45d80..3da8c2cb0c 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -586,7 +586,7 @@ def _get_oneofs(self, def _get_fields(self, field_pbs: Sequence[descriptor_pb2.FieldDescriptorProto], address: metadata.Address, path: Tuple[int, ...], - oneofs: Optional[Dict[str, wrappers.Oneof]]=None + oneofs: Optional[Dict[str, wrappers.Oneof]] = None ) -> Dict[str, wrappers.Field]: """Return a dictionary of wrapped fields for the given message. diff --git a/tests/unit/schema/wrappers/test_oneof.py b/tests/unit/schema/wrappers/test_oneof.py index d772846e12..90fe2546ce 100644 --- a/tests/unit/schema/wrappers/test_oneof.py +++ b/tests/unit/schema/wrappers/test_oneof.py @@ -23,8 +23,6 @@ from gapic.schema import wrappers from test_utils.test_utils import ( - #make_field, - #make_message, make_oneof_pb2, ) From b44317f504735049868d74facf97c278c1ecc13a Mon Sep 17 00:00:00 2001 From: Chris Wilcox Date: Tue, 30 Jun 2020 12:57:36 -0700 Subject: [PATCH 15/25] whitespace --- gapic/schema/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 3da8c2cb0c..6c666d6980 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -586,7 +586,7 @@ def _get_oneofs(self, def _get_fields(self, field_pbs: Sequence[descriptor_pb2.FieldDescriptorProto], address: metadata.Address, path: Tuple[int, ...], - oneofs: Optional[Dict[str, wrappers.Oneof]] = None + oneofs: Optional[Dict[str, wrappers.Oneof]] = None ) -> Dict[str, wrappers.Field]: """Return a dictionary of wrapped fields for the given message. From 374bfbe05563560d31a93874f3bbd56b989c4337 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Mon, 6 Jul 2020 14:02:37 -0700 Subject: [PATCH 16/25] Add an nth utility --- gapic/utils/__init__.py | 2 ++ gapic/utils/code.py | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/gapic/utils/__init__.py b/gapic/utils/__init__.py index 315c575e2b..905fcbdec2 100644 --- a/gapic/utils/__init__.py +++ b/gapic/utils/__init__.py @@ -15,6 +15,7 @@ from gapic.utils.cache import cached_property from gapic.utils.case import to_snake_case from gapic.utils.code import empty +from gapic.utils.code import nth from gapic.utils.code import partition from gapic.utils.doc import doc from gapic.utils.filename import to_valid_filename @@ -29,6 +30,7 @@ 'cached_property', 'doc', 'empty', + 'nth', 'partition', 'RESERVED_NAMES', 'rst', diff --git a/gapic/utils/code.py b/gapic/utils/code.py index 27458a999c..00b7d4d170 100644 --- a/gapic/utils/code.py +++ b/gapic/utils/code.py @@ -12,8 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import (Callable, Iterable, List, Tuple, TypeVar) - +from typing import (Callable, Iterable, List, Optional, Tuple, TypeVar) +import itertools def empty(content: str) -> bool: """Return True if this file has no Python statements, False otherwise. @@ -50,3 +50,15 @@ def partition(predicate: Callable[[T], bool], # Returns trueList, falseList return results[1], results[0] + + +def nth(iterable: Iterable[T], n: int, default: Optional[T]=None) -> Optional[T]: + """Return the nth element of an iterable or a default value. + + Args + iterable: (Iterable(T)): An iterable on any type. + n (int): The 'index' of the lement to retrieve. + default: (Optional(T)): An optional default elemnt if the iterable has + fewer than n elements. + """ + return next(itertools.islice(iterable, n, None), default) From a329bd09d3a042474ea0b5b9e3dbe83004dc4643 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Mon, 6 Jul 2020 14:03:08 -0700 Subject: [PATCH 17/25] Tweak api.py to use nth --- gapic/schema/api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 6c666d6980..a94f1c1fef 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -34,6 +34,7 @@ from gapic.schema import wrappers from gapic.schema import naming as api_naming from gapic.utils import cached_property +from gapic.utils import nth from gapic.utils import to_snake_case from gapic.utils import RESERVED_NAMES @@ -614,9 +615,8 @@ def _get_fields(self, # `_load_message` method. answer: Dict[str, wrappers.Field] = collections.OrderedDict() for field_pb, i in zip(field_pbs, range(0, sys.maxsize)): - oneof_name = None - if getattr(field_pb, 'oneof_index', -1) >= 0 and oneofs: - oneof_name = list(oneofs.keys())[field_pb.oneof_index] + oneofs_exist = oneofs and getattr(field_pb, 'oneof_index', -1) > 0 + oneof_name = nth(oneofs, field_pb.oneof_index) if oneofs_exist else None answer[field_pb.name] = wrappers.Field( field_pb=field_pb, @@ -816,7 +816,7 @@ def _load_message(self, oneofs = self._get_oneofs( message_pb.oneof_decl, address=address, - path=path + (8,), + path=path + (7,), ) # Create a dictionary of all the fields for this message. From cbf73a3b6e20e6771c14373652784f69db47e274 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Mon, 6 Jul 2020 14:03:30 -0700 Subject: [PATCH 18/25] Templates change for oneofs (NOT ADS TEMPLATES) --- gapic/templates/noxfile.py.j2 | 2 +- .../gapic/%name_%version/%sub/test_%service.py.j2 | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/gapic/templates/noxfile.py.j2 b/gapic/templates/noxfile.py.j2 index d31a325e2f..5fde488f00 100644 --- a/gapic/templates/noxfile.py.j2 +++ b/gapic/templates/noxfile.py.j2 @@ -20,7 +20,7 @@ def unit(session): '--cov-config=.coveragerc', '--cov-report=term', '--cov-report=html', - os.path.join('tests', 'unit', '{{ api.naming.versioned_module_name }}'), + os.path.join('tests', 'unit',) ) 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 index ecfe6af704..45355276d9 100644 --- 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 @@ -285,9 +285,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 and not field.proto3_optional) %} {{ field.name }}={{ field.mock_value }}, - {%- endfor %} + {% endif %}{%- endfor %} ) {% endif -%} {% if method.client_streaming %} @@ -315,7 +315,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') -%} + {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not (field.oneof and not 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 #} @@ -323,6 +323,7 @@ def test_{{ method.name|snake_case }}(transport: str = 'grpc'): {% else -%} assert response.{{ field.name }} == {{ field.mock_value }} {% endif -%} + {% endif -%} {# end oneof/optional #} {% endfor %} {% endif %} @@ -365,8 +366,9 @@ 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') %} + {%- for field in method.output.fields.values() | rejectattr('message') %}{% if not (field.oneof and not field.proto3_optional) %} {{ field.name }}={{ field.mock_value }}, + {%- endif %} {%- endfor %} )) {% endif -%} @@ -397,7 +399,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') -%} + {% for field in method.output.fields.values() | rejectattr('message') -%}{% if not (field.oneof and not 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 #} @@ -405,6 +407,7 @@ async def test_{{ method.name|snake_case }}_async(transport: str = 'grpc_asyncio {% else -%} assert response.{{ field.name }} == {{ field.mock_value }} {% endif -%} + {% endif -%} {# oneof/optional #} {% endfor %} {% endif %} From 5f861d3ddd239afaa749c0b6ffe4148f10874f5a Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 13:25:51 -0700 Subject: [PATCH 19/25] Test nth --- gapic/schema/api.py | 2 +- tests/unit/utils/test_code.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index a94f1c1fef..8ec70f422b 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -614,7 +614,7 @@ def _get_fields(self, # first) and this will be None. This case is addressed in the # `_load_message` method. answer: Dict[str, wrappers.Field] = collections.OrderedDict() - for field_pb, i in zip(field_pbs, range(0, sys.maxsize)): + for i, field_pb in enumerate(field_pbs): oneofs_exist = oneofs and getattr(field_pb, 'oneof_index', -1) > 0 oneof_name = nth(oneofs, field_pb.oneof_index) if oneofs_exist else None diff --git a/tests/unit/utils/test_code.py b/tests/unit/utils/test_code.py index 1069443f7b..b0106073d5 100644 --- a/tests/unit/utils/test_code.py +++ b/tests/unit/utils/test_code.py @@ -34,3 +34,11 @@ def test_empty_whitespace_comments(): def test_empty_code(): assert not code.empty('import this') + +def test_nth(): + # list + assert code.nth([i*i for i in range(20)], 4) == 16 + # generator + assert code.nth((i*i for i in range(20)), 4) == 16 + # default + assert code.nth((i*i for i in range(20)), 30, 2112) == 2112 From 64ec32b4b2072198a72e41319b23f73dd340b25c Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 13:27:39 -0700 Subject: [PATCH 20/25] Style check --- gapic/schema/api.py | 3 ++- gapic/utils/code.py | 3 ++- tests/unit/utils/test_code.py | 6 +++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 8ec70f422b..5823787f59 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -616,7 +616,8 @@ def _get_fields(self, answer: Dict[str, wrappers.Field] = collections.OrderedDict() for i, field_pb in enumerate(field_pbs): oneofs_exist = oneofs and getattr(field_pb, 'oneof_index', -1) > 0 - oneof_name = nth(oneofs, field_pb.oneof_index) if oneofs_exist else None + oneof_name = nth(oneofs, + field_pb.oneof_index) if oneofs_exist else None answer[field_pb.name] = wrappers.Field( field_pb=field_pb, diff --git a/gapic/utils/code.py b/gapic/utils/code.py index 00b7d4d170..2c8d869392 100644 --- a/gapic/utils/code.py +++ b/gapic/utils/code.py @@ -15,6 +15,7 @@ from typing import (Callable, Iterable, List, Optional, Tuple, TypeVar) import itertools + def empty(content: str) -> bool: """Return True if this file has no Python statements, False otherwise. @@ -52,7 +53,7 @@ def partition(predicate: Callable[[T], bool], return results[1], results[0] -def nth(iterable: Iterable[T], n: int, default: Optional[T]=None) -> Optional[T]: +def nth(iterable: Iterable[T], n: int, default: Optional[T] = None) -> Optional[T]: """Return the nth element of an iterable or a default value. Args diff --git a/tests/unit/utils/test_code.py b/tests/unit/utils/test_code.py index b0106073d5..fe77d4314a 100644 --- a/tests/unit/utils/test_code.py +++ b/tests/unit/utils/test_code.py @@ -37,8 +37,8 @@ def test_empty_code(): def test_nth(): # list - assert code.nth([i*i for i in range(20)], 4) == 16 + assert code.nth([i * i for i in range(20)], 4) == 16 # generator - assert code.nth((i*i for i in range(20)), 4) == 16 + assert code.nth((i * i for i in range(20)), 4) == 16 # default - assert code.nth((i*i for i in range(20)), 30, 2112) == 2112 + assert code.nth((i * i for i in range(20)), 30, 2112) == 2112 From 07709f871fe0b02f3f7e29738a7a5b65ae773f02 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 13:36:34 -0700 Subject: [PATCH 21/25] Tweak --- gapic/schema/api.py | 8 +++++--- tests/unit/utils/test_code.py | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 5823787f59..d22dbe4980 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -615,9 +615,11 @@ def _get_fields(self, # `_load_message` method. answer: Dict[str, wrappers.Field] = collections.OrderedDict() for i, field_pb in enumerate(field_pbs): - oneofs_exist = oneofs and getattr(field_pb, 'oneof_index', -1) > 0 - oneof_name = nth(oneofs, - field_pb.oneof_index) if oneofs_exist else None + oneof_name = nth( + iter(oneofs or {}), + field_pb.oneof_index if getattr(field_pb, 'oneof_index', -1) > 0 else None, + None + ) answer[field_pb.name] = wrappers.Field( field_pb=field_pb, diff --git a/tests/unit/utils/test_code.py b/tests/unit/utils/test_code.py index fe77d4314a..5f18679d6f 100644 --- a/tests/unit/utils/test_code.py +++ b/tests/unit/utils/test_code.py @@ -35,6 +35,7 @@ def test_empty_whitespace_comments(): def test_empty_code(): assert not code.empty('import this') + def test_nth(): # list assert code.nth([i * i for i in range(20)], 4) == 16 From aa7ceb625826c029bf10c30c704e1cc8e2044bc5 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 13:39:34 -0700 Subject: [PATCH 22/25] Thing --- gapic/schema/api.py | 6 +++++- gapic/utils/code.py | 8 ++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index d22dbe4980..30dbac3943 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -617,7 +617,11 @@ def _get_fields(self, for i, field_pb in enumerate(field_pbs): oneof_name = nth( iter(oneofs or {}), - field_pb.oneof_index if getattr(field_pb, 'oneof_index', -1) > 0 else None, + field_pb.oneof_index if getattr( + field_pb, + 'oneof_index', + -1 + ) > 0 else None, None ) diff --git a/gapic/utils/code.py b/gapic/utils/code.py index 2c8d869392..87603e215e 100644 --- a/gapic/utils/code.py +++ b/gapic/utils/code.py @@ -53,13 +53,13 @@ def partition(predicate: Callable[[T], bool], return results[1], results[0] -def nth(iterable: Iterable[T], n: int, default: Optional[T] = None) -> Optional[T]: +def nth(iterable: Iterable[T], n: Optional[int] = None, default: Optional[T] = None) -> Optional[T]: """Return the nth element of an iterable or a default value. Args - iterable: (Iterable(T)): An iterable on any type. - n (int): The 'index' of the lement to retrieve. - default: (Optional(T)): An optional default elemnt if the iterable has + iterable (Iterable(T)): An iterable on any type. + n (Optional(int)): The 'index' of the lement to retrieve. + default (Optional(T)): An optional default elemnt if the iterable has fewer than n elements. """ return next(itertools.islice(iterable, n, None), default) From ec65b735c827668d89ade8ee43fbcff757d0a313 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 14:31:34 -0700 Subject: [PATCH 23/25] Thing in thing exception --- gapic/schema/api.py | 14 +++++--------- .../%name_%version/%sub/types/_message.py.j2 | 10 +++++----- gapic/utils/code.py | 4 ++-- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 30dbac3943..67efad53c5 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -615,16 +615,12 @@ def _get_fields(self, # `_load_message` method. answer: Dict[str, wrappers.Field] = collections.OrderedDict() for i, field_pb in enumerate(field_pbs): + is_oneof = oneofs and field_pb.oneof_index > 0 oneof_name = nth( - iter(oneofs or {}), - field_pb.oneof_index if getattr( - field_pb, - 'oneof_index', - -1 - ) > 0 else None, - None - ) - + iter(oneofs), + field_pb.oneof_index + ) if is_oneof else None + answer[field_pb.name] = wrappers.Field( field_pb=field_pb, enum=self.api_enums.get(field_pb.type_name.lstrip('.')), diff --git a/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 b/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 index e71ea0f70f..5a1eb5fcc6 100644 --- a/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 +++ b/gapic/templates/%namespace/%name_%version/%sub/types/_message.py.j2 @@ -38,15 +38,15 @@ class {{ message.name }}({{ p }}.Message): {{- p }}.{{ key_field.proto_type }}, {{ p }}.{{ value_field.proto_type }}, number={{ field.number }} {%- if value_field.enum or value_field.message %}, {{ value_field.proto_type.lower() }}={{ value_field.type.ident.rel(message.ident) }}, - {% endif %}) + {% endif %}) {# enum or message#} {% endwith -%} - {% else -%} + {% 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 %} {%- if field.enum or field.message %}, {{ field.proto_type.lower() }}={{ field.type.ident.rel(message.ident) }}, - {% endif %}) - {% endif -%} - {% endfor -%} + {% endif %}) {# enum or message #} + {% endif -%} {# field.map #} + {% endfor -%} {# for field in message.fields.values#} {{ '\n\n' }} diff --git a/gapic/utils/code.py b/gapic/utils/code.py index 87603e215e..15f327983c 100644 --- a/gapic/utils/code.py +++ b/gapic/utils/code.py @@ -53,12 +53,12 @@ def partition(predicate: Callable[[T], bool], return results[1], results[0] -def nth(iterable: Iterable[T], n: Optional[int] = None, default: Optional[T] = None) -> Optional[T]: +def nth(iterable: Iterable[T], n: int, default: Optional[T] = None) -> Optional[T]: """Return the nth element of an iterable or a default value. Args iterable (Iterable(T)): An iterable on any type. - n (Optional(int)): The 'index' of the lement to retrieve. + n (int): The 'index' of the lement to retrieve. default (Optional(T)): An optional default elemnt if the iterable has fewer than n elements. """ From 05b05d55c6be82d35130709cce344283196354ca Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 14:34:42 -0700 Subject: [PATCH 24/25] Whitespace --- gapic/schema/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index 67efad53c5..e75258bc06 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -620,7 +620,7 @@ def _get_fields(self, iter(oneofs), field_pb.oneof_index ) if is_oneof else None - + answer[field_pb.name] = wrappers.Field( field_pb=field_pb, enum=self.api_enums.get(field_pb.type_name.lstrip('.')), From e590edb99452ce4d65bd0aac4613bef457a95a13 Mon Sep 17 00:00:00 2001 From: Dov Shlachter Date: Tue, 7 Jul 2020 14:35:21 -0700 Subject: [PATCH 25/25] Fighting the type system --- gapic/schema/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gapic/schema/api.py b/gapic/schema/api.py index e75258bc06..df3e1daa8e 100644 --- a/gapic/schema/api.py +++ b/gapic/schema/api.py @@ -617,7 +617,7 @@ def _get_fields(self, for i, field_pb in enumerate(field_pbs): is_oneof = oneofs and field_pb.oneof_index > 0 oneof_name = nth( - iter(oneofs), + (oneofs or {}).keys(), field_pb.oneof_index ) if is_oneof else None