From 72512587223de32aa28ada8ef1a07e44da114eb9 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Fri, 16 May 2025 20:52:13 +0000 Subject: [PATCH 1/5] feat: Proto-plus modifications for enforcing strict oneofs --- .../admin_v2/overlay/types/oneof_message.py | 107 ++++++++++++ .../bigtable/admin_v2/types/oneof_message.py | 107 ++++++++++++ google/cloud/bigtable/admin_v2/types/table.py | 4 +- owlbot.py | 14 ++ tests/unit/admin_overlay/my_oneof_message.py | 45 +++++ .../unit/admin_overlay/test_oneof_message.py | 156 ++++++++++++++++++ 6 files changed, 431 insertions(+), 2 deletions(-) create mode 100644 google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py create mode 100644 google/cloud/bigtable/admin_v2/types/oneof_message.py create mode 100644 tests/unit/admin_overlay/my_oneof_message.py create mode 100644 tests/unit/admin_overlay/test_oneof_message.py diff --git a/google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py b/google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py new file mode 100644 index 000000000..3ed38b2ff --- /dev/null +++ b/google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py @@ -0,0 +1,107 @@ +# -*- coding: utf-8 -*- +# Copyright 2025 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 +# +# http://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.abc +import proto + + +class OneofMessage(proto.Message): + def _get_oneof_field_from_key(self, key): + """Given a field name, return the corresponding oneof associated with it. If it doesn't exist, return None.""" + + oneof_type = None + + try: + oneof_type = self._meta.fields[key].oneof + except KeyError: + # Underscores may be appended to field names + # that collide with python or proto-plus keywords. + # In case a key only exists with a `_` suffix, coerce the key + # to include the `_` suffix. It's not possible to + # natively define the same field with a trailing underscore in protobuf. + # See related issue + # https://github.com/googleapis/python-api-core/issues/227 + if f"{key}_" in self._meta.fields: + key = f"{key}_" + oneof_type = self._meta.fields[key].oneof + + return oneof_type + + def __init__( + self, + mapping=None, + *, + ignore_unknown_fields=False, + **kwargs, + ): + # We accept several things for `mapping`: + # * An instance of this class. + # * An instance of the underlying protobuf descriptor class. + # * A dict + # * Nothing (keyword arguments only). + # + # + # Check for oneofs collisions in the parameters provided. Extract a set of + # all fields that are set from the mappings + kwargs combined. + mapping_fields = set(kwargs.keys()) + + if mapping is None: + pass + elif isinstance(mapping, collections.abc.Mapping): + mapping_fields.update(mapping.keys()) + elif isinstance(mapping, self._meta.pb): + mapping_fields.update(field.name for field, _ in mapping.ListFields()) + elif isinstance(mapping, type(self)): + mapping_fields.update(field.name for field, _ in mapping._pb.ListFields()) + else: + # Sanity check: Did we get something not a map? Error if so. + raise TypeError( + "Invalid constructor input for %s: %r" + % ( + self.__class__.__name__, + mapping, + ) + ) + + oneofs = set() + + for field in mapping_fields: + oneof_field = self._get_oneof_field_from_key(field) + if oneof_field is not None: + if oneof_field in oneofs: + raise ValueError( + "Invalid constructor input for %s: Multiple fields defined for oneof %s" + % (self.__class__.__name__, oneof_field) + ) + else: + oneofs.add(oneof_field) + + super().__init__(mapping, ignore_unknown_fields=ignore_unknown_fields, **kwargs) + + def __setattr__(self, key, value): + # Oneof check: Only set the value of an existing oneof field + # if the field being overridden is the same as the field already set + # for the oneof. + oneof = self._get_oneof_field_from_key(key) + if ( + oneof is not None + and self._pb.HasField(oneof) + and self._pb.WhichOneof(oneof) != key + ): + raise ValueError( + "Overriding the field set for oneof %s with a different field %s" + % (oneof, key) + ) + super().__setattr__(key, value) diff --git a/google/cloud/bigtable/admin_v2/types/oneof_message.py b/google/cloud/bigtable/admin_v2/types/oneof_message.py new file mode 100644 index 000000000..3ed38b2ff --- /dev/null +++ b/google/cloud/bigtable/admin_v2/types/oneof_message.py @@ -0,0 +1,107 @@ +# -*- coding: utf-8 -*- +# Copyright 2025 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 +# +# http://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.abc +import proto + + +class OneofMessage(proto.Message): + def _get_oneof_field_from_key(self, key): + """Given a field name, return the corresponding oneof associated with it. If it doesn't exist, return None.""" + + oneof_type = None + + try: + oneof_type = self._meta.fields[key].oneof + except KeyError: + # Underscores may be appended to field names + # that collide with python or proto-plus keywords. + # In case a key only exists with a `_` suffix, coerce the key + # to include the `_` suffix. It's not possible to + # natively define the same field with a trailing underscore in protobuf. + # See related issue + # https://github.com/googleapis/python-api-core/issues/227 + if f"{key}_" in self._meta.fields: + key = f"{key}_" + oneof_type = self._meta.fields[key].oneof + + return oneof_type + + def __init__( + self, + mapping=None, + *, + ignore_unknown_fields=False, + **kwargs, + ): + # We accept several things for `mapping`: + # * An instance of this class. + # * An instance of the underlying protobuf descriptor class. + # * A dict + # * Nothing (keyword arguments only). + # + # + # Check for oneofs collisions in the parameters provided. Extract a set of + # all fields that are set from the mappings + kwargs combined. + mapping_fields = set(kwargs.keys()) + + if mapping is None: + pass + elif isinstance(mapping, collections.abc.Mapping): + mapping_fields.update(mapping.keys()) + elif isinstance(mapping, self._meta.pb): + mapping_fields.update(field.name for field, _ in mapping.ListFields()) + elif isinstance(mapping, type(self)): + mapping_fields.update(field.name for field, _ in mapping._pb.ListFields()) + else: + # Sanity check: Did we get something not a map? Error if so. + raise TypeError( + "Invalid constructor input for %s: %r" + % ( + self.__class__.__name__, + mapping, + ) + ) + + oneofs = set() + + for field in mapping_fields: + oneof_field = self._get_oneof_field_from_key(field) + if oneof_field is not None: + if oneof_field in oneofs: + raise ValueError( + "Invalid constructor input for %s: Multiple fields defined for oneof %s" + % (self.__class__.__name__, oneof_field) + ) + else: + oneofs.add(oneof_field) + + super().__init__(mapping, ignore_unknown_fields=ignore_unknown_fields, **kwargs) + + def __setattr__(self, key, value): + # Oneof check: Only set the value of an existing oneof field + # if the field being overridden is the same as the field already set + # for the oneof. + oneof = self._get_oneof_field_from_key(key) + if ( + oneof is not None + and self._pb.HasField(oneof) + and self._pb.WhichOneof(oneof) != key + ): + raise ValueError( + "Overriding the field set for oneof %s with a different field %s" + % (oneof, key) + ) + super().__setattr__(key, value) diff --git a/google/cloud/bigtable/admin_v2/types/table.py b/google/cloud/bigtable/admin_v2/types/table.py index 8705b731b..112c1e284 100644 --- a/google/cloud/bigtable/admin_v2/types/table.py +++ b/google/cloud/bigtable/admin_v2/types/table.py @@ -19,7 +19,7 @@ import proto # type: ignore -from google.cloud.bigtable.admin_v2.types import types +from google.cloud.bigtable.admin_v2.types import types, oneof_message from google.protobuf import duration_pb2 # type: ignore from google.protobuf import timestamp_pb2 # type: ignore from google.rpc import status_pb2 # type: ignore @@ -584,7 +584,7 @@ class ColumnFamily(proto.Message): ) -class GcRule(proto.Message): +class GcRule(oneof_message.OneofMessage): r"""Rule for determining which cells to delete during garbage collection. diff --git a/owlbot.py b/owlbot.py index 8f8de3024..19b5af8c9 100644 --- a/owlbot.py +++ b/owlbot.py @@ -238,4 +238,18 @@ def add_overlay_to_init_py(init_py_location, import_statements): """ ) +# Add the oneof_message import into table.py for GcRule +s.replace( + "google/cloud/bigtable/admin_v2/types/table.py", + r"from google\.cloud\.bigtable\.admin_v2\.types import types", + "from google.cloud.bigtable.admin_v2.types import types, oneof_message", +) + +# Re-subclass GcRule in table.py +s.replace( + "google/cloud/bigtable/admin_v2/types/table.py", + r"class GcRule\(proto\.Message\)\:", + "class GcRule(oneof_message.OneofMessage):", +) + s.shell.run(["nox", "-s", "blacken"], hide_output=False) diff --git a/tests/unit/admin_overlay/my_oneof_message.py b/tests/unit/admin_overlay/my_oneof_message.py new file mode 100644 index 000000000..08058b512 --- /dev/null +++ b/tests/unit/admin_overlay/my_oneof_message.py @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- +# Copyright 2025 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 +# +# http://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 proto + +from google.cloud.bigtable.admin_v2.types import oneof_message + +__protobuf__ = proto.module( + package="test.oneof.v1", + manifest={ + "MyOneofMessage", + }, +) + + +# Foo and Bar belong to oneof foobar, and baz is independent. +class MyOneofMessage(oneof_message.OneofMessage): + foo: int = proto.Field( + proto.INT32, + number=1, + oneof="foobar", + ) + + bar: int = proto.Field( + proto.INT32, + number=2, + oneof="foobar", + ) + + baz: int = proto.Field( + proto.INT32, + number=3, + ) diff --git a/tests/unit/admin_overlay/test_oneof_message.py b/tests/unit/admin_overlay/test_oneof_message.py new file mode 100644 index 000000000..58b641bb0 --- /dev/null +++ b/tests/unit/admin_overlay/test_oneof_message.py @@ -0,0 +1,156 @@ +# -*- coding: utf-8 -*- +# Copyright 2025 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 +# +# http://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. +# +from google.cloud.bigtable.admin_v2.types import GcRule +from google.protobuf import duration_pb2 + +import my_oneof_message + +import unittest + +# The following proto bytestring was constructed running printproto in +# text-to-binary mode on the following textproto for GcRule: +# +# intersection { +# rules { +# max_num_versions: 1234 +# } +# rules { +# max_age { +# seconds: 12345 +# } +# } +# } +GCRULE_RAW_PROTO_BYTESTRING = b"\x1a\x0c\n\x03\x08\xd2\t\n\x05\x12\x03\x08\xb9`" +INITIAL_VALUE = 123 +FINAL_VALUE = 456 + + +class TestGenericOneofMessage(unittest.TestCase): + def setUp(self): + self.default_msg = my_oneof_message.MyOneofMessage() + self.foo_msg = my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE) + + def test_setattr_oneof_no_conflict(self): + self.default_msg.foo = INITIAL_VALUE + self.default_msg.baz = INITIAL_VALUE + self.assertEqual(self.default_msg.foo, INITIAL_VALUE) + self.assertEqual(self.default_msg.baz, INITIAL_VALUE) + self.assertFalse(self.default_msg.bar) + + def test_setattr_oneof_conflict(self): + with self.assertRaises(ValueError): + self.foo_msg.bar = INITIAL_VALUE + self.assertEqual(self.foo_msg.foo, INITIAL_VALUE) + self.assertFalse(self.foo_msg.bar) + + self.default_msg.bar = INITIAL_VALUE + with self.assertRaises(ValueError): + self.default_msg.foo = INITIAL_VALUE + self.assertEqual(self.default_msg.bar, INITIAL_VALUE) + self.assertFalse(self.default_msg.foo) + + def test_setattr_oneof_same_oneof_field(self): + self.foo_msg.foo = FINAL_VALUE + self.assertEqual(self.foo_msg.foo, FINAL_VALUE) + self.assertFalse(self.foo_msg.bar) + + self.default_msg.foo = INITIAL_VALUE + self.default_msg.foo = FINAL_VALUE + self.assertEqual(self.default_msg.foo, FINAL_VALUE) + + def test_setattr_oneof_delattr(self): + del self.foo_msg.foo + self.foo_msg.bar = INITIAL_VALUE + self.assertEqual(self.foo_msg.bar, INITIAL_VALUE) + self.assertFalse(self.foo_msg.foo) + + def test_init_oneof_conflict(self): + with self.assertRaises(ValueError): + my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE, bar=INITIAL_VALUE) + + with self.assertRaises(ValueError): + my_oneof_message.MyOneofMessage( + { + "foo": INITIAL_VALUE, + "bar": INITIAL_VALUE, + } + ) + + with self.assertRaises(ValueError): + my_oneof_message.MyOneofMessage(self.foo_msg._pb, bar=INITIAL_VALUE) + + with self.assertRaises(ValueError): + my_oneof_message.MyOneofMessage(self.foo_msg, bar=INITIAL_VALUE) + + def test_init_oneof_no_conflict(self): + msg = my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE, baz=INITIAL_VALUE) + self.assertEqual(msg.foo, INITIAL_VALUE) + self.assertEqual(msg.baz, INITIAL_VALUE) + self.assertFalse(msg.bar) + + msg = my_oneof_message.MyOneofMessage( + { + "foo": INITIAL_VALUE, + "baz": INITIAL_VALUE, + } + ) + self.assertEqual(msg.foo, INITIAL_VALUE) + self.assertEqual(msg.baz, INITIAL_VALUE) + self.assertFalse(msg.bar) + + msg = my_oneof_message.MyOneofMessage(self.foo_msg, baz=INITIAL_VALUE) + self.assertEqual(msg.foo, INITIAL_VALUE) + self.assertEqual(msg.baz, INITIAL_VALUE) + self.assertFalse(msg.bar) + + msg = my_oneof_message.MyOneofMessage(self.foo_msg._pb, baz=INITIAL_VALUE) + self.assertEqual(msg.foo, INITIAL_VALUE) + self.assertEqual(msg.baz, INITIAL_VALUE) + self.assertFalse(msg.bar) + + def test_init_kwargs_override_same_field_oneof(self): + # Kwargs take precedence over mapping, and this should be OK + msg = my_oneof_message.MyOneofMessage( + { + "foo": INITIAL_VALUE, + }, + foo=FINAL_VALUE, + ) + self.assertEqual(msg.foo, FINAL_VALUE) + + msg = my_oneof_message.MyOneofMessage(self.foo_msg, foo=FINAL_VALUE) + self.assertEqual(msg.foo, FINAL_VALUE) + + msg = my_oneof_message.MyOneofMessage(self.foo_msg._pb, foo=FINAL_VALUE) + self.assertEqual(msg.foo, FINAL_VALUE) + + +class TestGcRule(unittest.TestCase): + def test_serialize_deserialize(self): + test = GcRule( + intersection=GcRule.Intersection( + rules=[ + GcRule(max_num_versions=1234), + GcRule(max_age=duration_pb2.Duration(seconds=12345)), + ] + ) + ) + self.assertEqual(GcRule.serialize(test), GCRULE_RAW_PROTO_BYTESTRING) + self.assertEqual(GcRule.deserialize(GCRULE_RAW_PROTO_BYTESTRING), test) + + +if __name__ == "__main__": + unittest.main() From 7e34d3c8a446dd51b700cac743be35422556888b Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Mon, 16 Jun 2025 18:28:24 +0000 Subject: [PATCH 2/5] Added template directory + changed unit tests to pytest --- gapic_templates/README.rst | 9 + .../admin_v2/types/oneof_message.py.j2 | 0 google/cloud/bigtable/admin_v2/types/table.py | 3 +- owlbot.py | 22 +- .../unit/admin_overlay/test_oneof_message.py | 229 +++++++++--------- 5 files changed, 148 insertions(+), 115 deletions(-) create mode 100644 gapic_templates/README.rst rename google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py => gapic_templates/admin_v2/types/oneof_message.py.j2 (100%) diff --git a/gapic_templates/README.rst b/gapic_templates/README.rst new file mode 100644 index 000000000..1261a096c --- /dev/null +++ b/gapic_templates/README.rst @@ -0,0 +1,9 @@ +GAPIC Templates +=============== + +This directory is intended for inserting handwritten files +into autogenerated directories (see `owlbot.py`). In particular, +it is needed for inserting the definition of `OneofMessage` into +`google/cloud/bigtable/admin_v2/types` to prevent circular import +issues with having something in that directory import from +`google/cloud/bigtable/admin_v2/overlay`. diff --git a/google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py b/gapic_templates/admin_v2/types/oneof_message.py.j2 similarity index 100% rename from google/cloud/bigtable/admin_v2/overlay/types/oneof_message.py rename to gapic_templates/admin_v2/types/oneof_message.py.j2 diff --git a/google/cloud/bigtable/admin_v2/types/table.py b/google/cloud/bigtable/admin_v2/types/table.py index 112c1e284..2478f5601 100644 --- a/google/cloud/bigtable/admin_v2/types/table.py +++ b/google/cloud/bigtable/admin_v2/types/table.py @@ -19,7 +19,8 @@ import proto # type: ignore -from google.cloud.bigtable.admin_v2.types import types, oneof_message +from google.cloud.bigtable.admin_v2.types import types +from google.cloud.bigtable.admin_v2.types import oneof_message from google.protobuf import duration_pb2 # type: ignore from google.protobuf import timestamp_pb2 # type: ignore from google.rpc import status_pb2 # type: ignore diff --git a/owlbot.py b/owlbot.py index 19b5af8c9..dfc716fee 100644 --- a/owlbot.py +++ b/owlbot.py @@ -20,8 +20,9 @@ from typing import List, Optional import synthtool as s -from synthtool import gcp +from synthtool import gcp, _tracked_paths from synthtool.languages import python +from synthtool.sources import templates common = gcp.CommonTemplates() @@ -238,11 +239,26 @@ def add_overlay_to_init_py(init_py_location, import_statements): """ ) +# Oneofs work: + +# Move the definition of a oneof message into the autogenerated types +# directory. This is needed to prevent circular import issues in admin_v2/overlay. +# This can also be used to insert other files into other autogenerated directories. +gapic_templates = templates.TemplateGroup("gapic_templates") +_tracked_paths.add(gapic_templates.dir) +gapic_templated_files = gapic_templates.render() +s.move( + gapic_templated_files, + destination="google/cloud/bigtable", + excludes=["README.rst"], +) + # Add the oneof_message import into table.py for GcRule s.replace( "google/cloud/bigtable/admin_v2/types/table.py", - r"from google\.cloud\.bigtable\.admin_v2\.types import types", - "from google.cloud.bigtable.admin_v2.types import types, oneof_message", + r"^(from google\.cloud\.bigtable\.admin_v2\.types import .+)$", + r"""\1 +from google.cloud.bigtable.admin_v2.types import oneof_message""", ) # Re-subclass GcRule in table.py diff --git a/tests/unit/admin_overlay/test_oneof_message.py b/tests/unit/admin_overlay/test_oneof_message.py index 58b641bb0..b93da21d8 100644 --- a/tests/unit/admin_overlay/test_oneof_message.py +++ b/tests/unit/admin_overlay/test_oneof_message.py @@ -18,7 +18,8 @@ import my_oneof_message -import unittest +import pytest + # The following proto bytestring was constructed running printproto in # text-to-binary mode on the following textproto for GcRule: @@ -37,120 +38,126 @@ INITIAL_VALUE = 123 FINAL_VALUE = 456 +@pytest.fixture +def default_msg(): + return my_oneof_message.MyOneofMessage() -class TestGenericOneofMessage(unittest.TestCase): - def setUp(self): - self.default_msg = my_oneof_message.MyOneofMessage() - self.foo_msg = my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE) - - def test_setattr_oneof_no_conflict(self): - self.default_msg.foo = INITIAL_VALUE - self.default_msg.baz = INITIAL_VALUE - self.assertEqual(self.default_msg.foo, INITIAL_VALUE) - self.assertEqual(self.default_msg.baz, INITIAL_VALUE) - self.assertFalse(self.default_msg.bar) - - def test_setattr_oneof_conflict(self): - with self.assertRaises(ValueError): - self.foo_msg.bar = INITIAL_VALUE - self.assertEqual(self.foo_msg.foo, INITIAL_VALUE) - self.assertFalse(self.foo_msg.bar) - - self.default_msg.bar = INITIAL_VALUE - with self.assertRaises(ValueError): - self.default_msg.foo = INITIAL_VALUE - self.assertEqual(self.default_msg.bar, INITIAL_VALUE) - self.assertFalse(self.default_msg.foo) - - def test_setattr_oneof_same_oneof_field(self): - self.foo_msg.foo = FINAL_VALUE - self.assertEqual(self.foo_msg.foo, FINAL_VALUE) - self.assertFalse(self.foo_msg.bar) - - self.default_msg.foo = INITIAL_VALUE - self.default_msg.foo = FINAL_VALUE - self.assertEqual(self.default_msg.foo, FINAL_VALUE) - - def test_setattr_oneof_delattr(self): - del self.foo_msg.foo - self.foo_msg.bar = INITIAL_VALUE - self.assertEqual(self.foo_msg.bar, INITIAL_VALUE) - self.assertFalse(self.foo_msg.foo) - - def test_init_oneof_conflict(self): - with self.assertRaises(ValueError): - my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE, bar=INITIAL_VALUE) - - with self.assertRaises(ValueError): - my_oneof_message.MyOneofMessage( - { - "foo": INITIAL_VALUE, - "bar": INITIAL_VALUE, - } - ) - - with self.assertRaises(ValueError): - my_oneof_message.MyOneofMessage(self.foo_msg._pb, bar=INITIAL_VALUE) - - with self.assertRaises(ValueError): - my_oneof_message.MyOneofMessage(self.foo_msg, bar=INITIAL_VALUE) - - def test_init_oneof_no_conflict(self): - msg = my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE, baz=INITIAL_VALUE) - self.assertEqual(msg.foo, INITIAL_VALUE) - self.assertEqual(msg.baz, INITIAL_VALUE) - self.assertFalse(msg.bar) - - msg = my_oneof_message.MyOneofMessage( - { - "foo": INITIAL_VALUE, - "baz": INITIAL_VALUE, - } - ) - self.assertEqual(msg.foo, INITIAL_VALUE) - self.assertEqual(msg.baz, INITIAL_VALUE) - self.assertFalse(msg.bar) - - msg = my_oneof_message.MyOneofMessage(self.foo_msg, baz=INITIAL_VALUE) - self.assertEqual(msg.foo, INITIAL_VALUE) - self.assertEqual(msg.baz, INITIAL_VALUE) - self.assertFalse(msg.bar) - - msg = my_oneof_message.MyOneofMessage(self.foo_msg._pb, baz=INITIAL_VALUE) - self.assertEqual(msg.foo, INITIAL_VALUE) - self.assertEqual(msg.baz, INITIAL_VALUE) - self.assertFalse(msg.bar) - - def test_init_kwargs_override_same_field_oneof(self): - # Kwargs take precedence over mapping, and this should be OK - msg = my_oneof_message.MyOneofMessage( - { - "foo": INITIAL_VALUE, - }, - foo=FINAL_VALUE, - ) - self.assertEqual(msg.foo, FINAL_VALUE) - msg = my_oneof_message.MyOneofMessage(self.foo_msg, foo=FINAL_VALUE) - self.assertEqual(msg.foo, FINAL_VALUE) +@pytest.fixture +def foo_msg(): + return my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE) - msg = my_oneof_message.MyOneofMessage(self.foo_msg._pb, foo=FINAL_VALUE) - self.assertEqual(msg.foo, FINAL_VALUE) +def test_oneof_message_setattr_oneof_no_conflict(default_msg): + default_msg.foo = INITIAL_VALUE + default_msg.baz = INITIAL_VALUE + assert default_msg.foo == INITIAL_VALUE + assert default_msg.baz == INITIAL_VALUE + assert not default_msg.bar + + +def test_oneof_message_setattr_conflict(default_msg, foo_msg): + with pytest.raises(ValueError): + foo_msg.bar = INITIAL_VALUE + assert foo_msg.foo == INITIAL_VALUE + assert not foo_msg.bar + + default_msg.bar = INITIAL_VALUE + with pytest.raises(ValueError): + default_msg.foo = INITIAL_VALUE + assert default_msg.bar == INITIAL_VALUE + assert not default_msg.foo -class TestGcRule(unittest.TestCase): - def test_serialize_deserialize(self): - test = GcRule( - intersection=GcRule.Intersection( - rules=[ - GcRule(max_num_versions=1234), - GcRule(max_age=duration_pb2.Duration(seconds=12345)), - ] - ) - ) - self.assertEqual(GcRule.serialize(test), GCRULE_RAW_PROTO_BYTESTRING) - self.assertEqual(GcRule.deserialize(GCRULE_RAW_PROTO_BYTESTRING), test) +def test_oneof_message_setattr_oneof_same_oneof_field(default_msg, foo_msg): + foo_msg.foo = FINAL_VALUE + assert foo_msg.foo == FINAL_VALUE + assert not foo_msg.bar -if __name__ == "__main__": - unittest.main() + default_msg.bar = INITIAL_VALUE + default_msg.bar = FINAL_VALUE + assert default_msg.bar == FINAL_VALUE + assert not default_msg.foo + + +def test_oneof_message_setattr_oneof_delattr(foo_msg): + del foo_msg.foo + foo_msg.bar = INITIAL_VALUE + assert foo_msg.bar == INITIAL_VALUE + assert not foo_msg.foo + + +def test_oneof_message_init_oneof_conflict(foo_msg): + with pytest.raises(ValueError): + my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE, bar=INITIAL_VALUE) + + with pytest.raises(ValueError): + my_oneof_message.MyOneofMessage( + { + "foo": INITIAL_VALUE, + "bar": INITIAL_VALUE, + } + ) + + with pytest.raises(ValueError): + my_oneof_message.MyOneofMessage(foo_msg._pb, bar=INITIAL_VALUE) + + with pytest.raises(ValueError): + my_oneof_message.MyOneofMessage(foo_msg, bar=INITIAL_VALUE) + + +def test_oneof_message_init_oneof_no_conflict(foo_msg): + msg = my_oneof_message.MyOneofMessage(foo=INITIAL_VALUE, baz=INITIAL_VALUE) + assert msg.foo == INITIAL_VALUE + assert msg.baz == INITIAL_VALUE + assert not msg.bar + + msg = my_oneof_message.MyOneofMessage( + { + "foo": INITIAL_VALUE, + "baz": INITIAL_VALUE, + } + ) + assert msg.foo == INITIAL_VALUE + assert msg.baz == INITIAL_VALUE + assert not msg.bar + + msg = my_oneof_message.MyOneofMessage(foo_msg, baz=INITIAL_VALUE) + assert msg.foo == INITIAL_VALUE + assert msg.baz == INITIAL_VALUE + assert not msg.bar + + msg = my_oneof_message.MyOneofMessage(foo_msg._pb, baz=INITIAL_VALUE) + assert msg.foo == INITIAL_VALUE + assert msg.baz == INITIAL_VALUE + assert not msg.bar + + +def test_oneof_message_init_kwargs_override_same_field_oneof(foo_msg): + # Kwargs take precedence over mapping, and this should be OK + msg = my_oneof_message.MyOneofMessage( + { + "foo": INITIAL_VALUE, + }, + foo=FINAL_VALUE, + ) + assert msg.foo == FINAL_VALUE + + msg = my_oneof_message.MyOneofMessage(foo_msg, foo=FINAL_VALUE) + assert msg.foo == FINAL_VALUE + + msg = my_oneof_message.MyOneofMessage(foo_msg._pb, foo=FINAL_VALUE) + assert msg.foo == FINAL_VALUE + + +def test_gcrule_serialize_deserialize(): + test = GcRule( + intersection=GcRule.Intersection( + rules=[ + GcRule(max_num_versions=1234), + GcRule(max_age=duration_pb2.Duration(seconds=12345)), + ] + ) + ) + assert GcRule.serialize(test) == GCRULE_RAW_PROTO_BYTESTRING + assert GcRule.deserialize(GCRULE_RAW_PROTO_BYTESTRING) == test From 2c38e4e095e8d2ecf4d8931e6b6093761c316859 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Mon, 16 Jun 2025 18:35:17 +0000 Subject: [PATCH 3/5] Finished README --- gapic_templates/README.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gapic_templates/README.rst b/gapic_templates/README.rst index 1261a096c..7e27d4572 100644 --- a/gapic_templates/README.rst +++ b/gapic_templates/README.rst @@ -7,3 +7,11 @@ it is needed for inserting the definition of `OneofMessage` into `google/cloud/bigtable/admin_v2/types` to prevent circular import issues with having something in that directory import from `google/cloud/bigtable/admin_v2/overlay`. + + +Usage +----- + +The contents of this directory will be copied in to `google/cloud/bigtable`. +As such, create subdirectories in this directory to mirror the final location +under `google/cloud/bigtable` that your file will be under. From 46f1562ebbe3ac3e3fcbd37362161ef04370c3e7 Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Mon, 16 Jun 2025 18:36:58 +0000 Subject: [PATCH 4/5] linting --- tests/unit/admin_overlay/test_oneof_message.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/admin_overlay/test_oneof_message.py b/tests/unit/admin_overlay/test_oneof_message.py index b93da21d8..20c5d6b5f 100644 --- a/tests/unit/admin_overlay/test_oneof_message.py +++ b/tests/unit/admin_overlay/test_oneof_message.py @@ -38,6 +38,7 @@ INITIAL_VALUE = 123 FINAL_VALUE = 456 + @pytest.fixture def default_msg(): return my_oneof_message.MyOneofMessage() @@ -66,7 +67,7 @@ def test_oneof_message_setattr_conflict(default_msg, foo_msg): with pytest.raises(ValueError): default_msg.foo = INITIAL_VALUE assert default_msg.bar == INITIAL_VALUE - assert not default_msg.foo + assert not default_msg.foo def test_oneof_message_setattr_oneof_same_oneof_field(default_msg, foo_msg): From 9f2c9dce392badd7b7c1a00fc47190f87951de0b Mon Sep 17 00:00:00 2001 From: Kevin Zheng Date: Tue, 17 Jun 2025 14:39:02 +0000 Subject: [PATCH 5/5] Added source of truth comment --- gapic_templates/admin_v2/types/oneof_message.py.j2 | 4 ++++ google/cloud/bigtable/admin_v2/types/oneof_message.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/gapic_templates/admin_v2/types/oneof_message.py.j2 b/gapic_templates/admin_v2/types/oneof_message.py.j2 index 3ed38b2ff..0bb7cf551 100644 --- a/gapic_templates/admin_v2/types/oneof_message.py.j2 +++ b/gapic_templates/admin_v2/types/oneof_message.py.j2 @@ -13,6 +13,10 @@ # See the License for the specific language governing permissions and # limitations under the License. # +# +# AUTOGENERATED FILE: DO NOT EDIT. The source of truth for this file is under +# the gapic_templates directory, not this one. + import collections.abc import proto diff --git a/google/cloud/bigtable/admin_v2/types/oneof_message.py b/google/cloud/bigtable/admin_v2/types/oneof_message.py index 3ed38b2ff..0bb7cf551 100644 --- a/google/cloud/bigtable/admin_v2/types/oneof_message.py +++ b/google/cloud/bigtable/admin_v2/types/oneof_message.py @@ -13,6 +13,10 @@ # See the License for the specific language governing permissions and # limitations under the License. # +# +# AUTOGENERATED FILE: DO NOT EDIT. The source of truth for this file is under +# the gapic_templates directory, not this one. + import collections.abc import proto