diff options
author | Feng Xiao <xfxyjwf@gmail.com> | 2014-11-10 17:34:54 -0800 |
---|---|---|
committer | Feng Xiao <xfxyjwf@gmail.com> | 2014-11-10 17:34:54 -0800 |
commit | 6ef984af4b0c63c1c33127a12dcfc8e6359f0c9e (patch) | |
tree | d17c61ff9f3ae28224fbddac6d26bfc59e2cf755 /python/google | |
parent | baca1a8a1aa180c42de6278d3b8286c4496c6a10 (diff) | |
download | protobuf-6ef984af4b0c63c1c33127a12dcfc8e6359f0c9e.tar.gz protobuf-6ef984af4b0c63c1c33127a12dcfc8e6359f0c9e.tar.bz2 protobuf-6ef984af4b0c63c1c33127a12dcfc8e6359f0c9e.zip |
Down-integrate from internal code base.
Diffstat (limited to 'python/google')
44 files changed, 1220 insertions, 594 deletions
diff --git a/python/google/protobuf/descriptor.py b/python/google/protobuf/descriptor.py index 6da8bb0b..af571b7c 100755 --- a/python/google/protobuf/descriptor.py +++ b/python/google/protobuf/descriptor.py @@ -841,9 +841,10 @@ def MakeDescriptor(desc_proto, package='', build_file_if_cpp=True): field_proto.number, field_proto.type, FieldDescriptor.ProtoTypeToCppProtoType(field_proto.type), field_proto.label, None, nested_desc, enum_desc, None, False, None, - has_default_value=False) + options=field_proto.options, has_default_value=False) fields.append(field) desc_name = '.'.join(full_message_name) return Descriptor(desc_proto.name, desc_name, None, None, fields, - nested_types.values(), enum_types.values(), []) + nested_types.values(), enum_types.values(), [], + options=desc_proto.options) diff --git a/python/google/protobuf/descriptor_database.py b/python/google/protobuf/descriptor_database.py index 55fb8c70..b10021e9 100644 --- a/python/google/protobuf/descriptor_database.py +++ b/python/google/protobuf/descriptor_database.py @@ -133,5 +133,5 @@ def _ExtractSymbols(desc_proto, package): for nested_type in desc_proto.nested_type: for symbol in _ExtractSymbols(nested_type, message_name): yield symbol - for enum_type in desc_proto.enum_type: - yield '.'.join((message_name, enum_type.name)) + for enum_type in desc_proto.enum_type: + yield '.'.join((message_name, enum_type.name)) diff --git a/python/google/protobuf/descriptor_pool.py b/python/google/protobuf/descriptor_pool.py index cf234cfa..bcac513a 100644 --- a/python/google/protobuf/descriptor_pool.py +++ b/python/google/protobuf/descriptor_pool.py @@ -554,7 +554,7 @@ class DescriptorPool(object): field_desc.default_value = field_proto.default_value.lower() == 'true' elif field_proto.type == descriptor.FieldDescriptor.TYPE_ENUM: field_desc.default_value = field_desc.enum_type.values_by_name[ - field_proto.default_value].index + field_proto.default_value].number elif field_proto.type == descriptor.FieldDescriptor.TYPE_BYTES: field_desc.default_value = text_encoding.CUnescape( field_proto.default_value) diff --git a/python/google/protobuf/internal/descriptor_database_test.py b/python/google/protobuf/internal/descriptor_database_test.py index fc65b69a..8970f5c2 100644 --- a/python/google/protobuf/internal/descriptor_database_test.py +++ b/python/google/protobuf/internal/descriptor_database_test.py @@ -58,6 +58,8 @@ class DescriptorDatabaseTest(basetest.TestCase): 'google.protobuf.python.internal.Factory2Enum')) self.assertEquals(file_desc_proto, db.FindFileContainingSymbol( 'google.protobuf.python.internal.Factory2Message.NestedFactory2Enum')) + self.assertEquals(file_desc_proto, db.FindFileContainingSymbol( + 'google.protobuf.python.internal.MessageWithNestedEnumOnly.NestedEnum')) if __name__ == '__main__': basetest.main() diff --git a/python/google/protobuf/internal/descriptor_pool_test.py b/python/google/protobuf/internal/descriptor_pool_test.py index d2f85579..11ef61c5 100644 --- a/python/google/protobuf/internal/descriptor_pool_test.py +++ b/python/google/protobuf/internal/descriptor_pool_test.py @@ -48,6 +48,7 @@ from google.protobuf.internal import factory_test2_pb2 from google.protobuf import descriptor from google.protobuf import descriptor_database from google.protobuf import descriptor_pool +from google.protobuf import symbol_database class DescriptorPoolTest(basetest.TestCase): @@ -237,6 +238,32 @@ class DescriptorPoolTest(basetest.TestCase): TEST2_FILE.CheckFile(self, self.pool) + def testEnumDefaultValue(self): + """Test the default value of enums which don't start at zero.""" + def _CheckDefaultValue(file_descriptor): + default_value = (file_descriptor + .message_types_by_name['DescriptorPoolTest1'] + .fields_by_name['nested_enum'] + .default_value) + self.assertEqual(default_value, + descriptor_pool_test1_pb2.DescriptorPoolTest1.BETA) + # First check what the generated descriptor contains. + _CheckDefaultValue(descriptor_pool_test1_pb2.DESCRIPTOR) + # Then check the generated pool. Normally this is the same descriptor. + file_descriptor = symbol_database.Default().pool.FindFileByName( + 'google/protobuf/internal/descriptor_pool_test1.proto') + self.assertIs(file_descriptor, descriptor_pool_test1_pb2.DESCRIPTOR) + _CheckDefaultValue(file_descriptor) + + # Then check the dynamic pool and its internal DescriptorDatabase. + descriptor_proto = descriptor_pb2.FileDescriptorProto.FromString( + descriptor_pool_test1_pb2.DESCRIPTOR.serialized_pb) + self.pool.Add(descriptor_proto) + # And do the same check as above + file_descriptor = self.pool.FindFileByName( + 'google/protobuf/internal/descriptor_pool_test1.proto') + _CheckDefaultValue(file_descriptor) + class ProtoFile(object): @@ -328,7 +355,7 @@ class EnumField(object): test.assertEqual(descriptor.FieldDescriptor.CPPTYPE_ENUM, field_desc.cpp_type) test.assertTrue(field_desc.has_default_value) - test.assertEqual(enum_desc.values_by_name[self.default_value].index, + test.assertEqual(enum_desc.values_by_name[self.default_value].number, field_desc.default_value) test.assertEqual(msg_desc, field_desc.containing_type) test.assertEqual(enum_desc, field_desc.enum_type) diff --git a/python/google/protobuf/internal/descriptor_pool_test1.proto b/python/google/protobuf/internal/descriptor_pool_test1.proto index 6dfe4ef3..00816b78 100644 --- a/python/google/protobuf/internal/descriptor_pool_test1.proto +++ b/python/google/protobuf/internal/descriptor_pool_test1.proto @@ -28,6 +28,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto2"; + package google.protobuf.python.internal; diff --git a/python/google/protobuf/internal/descriptor_pool_test2.proto b/python/google/protobuf/internal/descriptor_pool_test2.proto index fbc84382..e3fa660c 100644 --- a/python/google/protobuf/internal/descriptor_pool_test2.proto +++ b/python/google/protobuf/internal/descriptor_pool_test2.proto @@ -28,6 +28,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto2"; + package google.protobuf.python.internal; import "google/protobuf/internal/descriptor_pool_test1.proto"; diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index b3777e39..3924f21a 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -665,5 +665,15 @@ class MakeDescriptorTest(basetest.TestCase): descriptor.FieldDescriptor.CPPTYPE_UINT64) + def testMakeDescriptorWithOptions(self): + descriptor_proto = descriptor_pb2.DescriptorProto() + aggregate_message = unittest_custom_options_pb2.AggregateMessage + aggregate_message.DESCRIPTOR.CopyToProto(descriptor_proto) + reformed_descriptor = descriptor.MakeDescriptor(descriptor_proto) + + options = reformed_descriptor.GetOptions() + self.assertEquals(101, + options.Extensions[unittest_custom_options_pb2.msgopt].i) + if __name__ == '__main__': basetest.main() diff --git a/python/google/protobuf/internal/factory_test1.proto b/python/google/protobuf/internal/factory_test1.proto index 9f5a3919..d2fbbeec 100644 --- a/python/google/protobuf/internal/factory_test1.proto +++ b/python/google/protobuf/internal/factory_test1.proto @@ -30,6 +30,7 @@ // Author: matthewtoia@google.com (Matt Toia) +syntax = "proto2"; package google.protobuf.python.internal; diff --git a/python/google/protobuf/internal/factory_test2.proto b/python/google/protobuf/internal/factory_test2.proto index 27feb6ce..bb1b54ad 100644 --- a/python/google/protobuf/internal/factory_test2.proto +++ b/python/google/protobuf/internal/factory_test2.proto @@ -30,6 +30,7 @@ // Author: matthewtoia@google.com (Matt Toia) +syntax = "proto2"; package google.protobuf.python.internal; @@ -87,6 +88,12 @@ message LoopMessage { optional Factory2Message loop = 1; } +message MessageWithNestedEnumOnly { + enum NestedEnum { + NESTED_MESSAGE_ENUM_0 = 0; + } +} + extend Factory1Message { optional string another_field = 1002; } diff --git a/python/google/protobuf/internal/generator_test.py b/python/google/protobuf/internal/generator_test.py index 422fa9a6..14b05cca 100755 --- a/python/google/protobuf/internal/generator_test.py +++ b/python/google/protobuf/internal/generator_test.py @@ -35,7 +35,7 @@ # indirect testing of the protocol compiler output. """Unittest that directly tests the output of the pure-Python protocol -compiler. See //google/protobuf/reflection_test.py for a test which +compiler. See //google/protobuf/internal/reflection_test.py for a test which further ensures that we can use Python protocol message objects as we expect. """ @@ -281,6 +281,8 @@ class GeneratorTest(basetest.TestCase): "baz") self.assertEqual(message.Extensions[test_bad_identifiers_pb2.service], "qux") + self.assertEqual(message.Extensions[test_bad_identifiers_pb2.class_], + "Foo") def testOneof(self): desc = unittest_pb2.TestAllTypes.DESCRIPTOR diff --git a/python/google/protobuf/internal/import_test_package/BUILD b/python/google/protobuf/internal/import_test_package/BUILD new file mode 100644 index 00000000..90e59505 --- /dev/null +++ b/python/google/protobuf/internal/import_test_package/BUILD @@ -0,0 +1,27 @@ +# Description: +# An example package that contains nested protos that are imported from +# __init__.py. See testPackageInitializationImport in reflection_test.py for +# details. + +package( + default_visibility = ["//net/proto2/python/internal:__pkg__"], +) + +proto_library( + name = "inner_proto", + srcs = ["inner.proto"], + py_api_version = 2, +) + +proto_library( + name = "outer_proto", + srcs = ["outer.proto"], + py_api_version = 2, + deps = [":inner_proto"], +) + +py_library( + name = "import_test_package", + srcs = ["__init__.py"], + deps = [":outer_proto"], +) diff --git a/python/google/protobuf/internal/import_test_package/__init__.py b/python/google/protobuf/internal/import_test_package/__init__.py new file mode 100644 index 00000000..5121dd0e --- /dev/null +++ b/python/google/protobuf/internal/import_test_package/__init__.py @@ -0,0 +1,33 @@ +# Protocol Buffers - Google's data interchange format +# Copyright 2008 Google Inc. All rights reserved. +# https://developers.google.com/protocol-buffers/ +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Sample module importing a nested proto from itself.""" + +from google.protobuf.internal.import_test_package import outer_pb2 as myproto diff --git a/python/google/protobuf/internal/import_test_package/inner.proto b/python/google/protobuf/internal/import_test_package/inner.proto new file mode 100644 index 00000000..2887c123 --- /dev/null +++ b/python/google/protobuf/internal/import_test_package/inner.proto @@ -0,0 +1,37 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto2"; + +package google.protobuf.python.internal.import_test_package; + +message Inner { + optional int32 value = 1 [default = 57]; +} diff --git a/python/google/protobuf/internal/import_test_package/outer.proto b/python/google/protobuf/internal/import_test_package/outer.proto new file mode 100644 index 00000000..a27fb5c8 --- /dev/null +++ b/python/google/protobuf/internal/import_test_package/outer.proto @@ -0,0 +1,39 @@ +// Protocol Buffers - Google's data interchange format +// Copyright 2008 Google Inc. All rights reserved. +// https://developers.google.com/protocol-buffers/ +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto2"; + +package google.protobuf.python.internal.import_test_package; + +import "google/protobuf/internal/import_test_package/inner.proto"; + +message Outer { + optional Inner inner = 1; +} diff --git a/python/google/protobuf/internal/message_test.py b/python/google/protobuf/internal/message_test.py index 48b7ffd4..42e2ad7e 100755 --- a/python/google/protobuf/internal/message_test.py +++ b/python/google/protobuf/internal/message_test.py @@ -337,6 +337,20 @@ class MessageTest(basetest.TestCase): empty.ParseFromString(populated.SerializeToString()) self.assertEqual(str(empty), '') + def testRepeatedNestedFieldIteration(self): + msg = unittest_pb2.TestAllTypes() + msg.repeated_nested_message.add(bb=1) + msg.repeated_nested_message.add(bb=2) + msg.repeated_nested_message.add(bb=3) + msg.repeated_nested_message.add(bb=4) + + self.assertEquals([1, 2, 3, 4], + [m.bb for m in msg.repeated_nested_message]) + self.assertEquals([4, 3, 2, 1], + [m.bb for m in reversed(msg.repeated_nested_message)]) + self.assertEquals([4, 3, 2, 1], + [m.bb for m in msg.repeated_nested_message[::-1]]) + def testSortingRepeatedScalarFieldsDefaultComparator(self): """Check some different types with the default comparator.""" message = unittest_pb2.TestAllTypes() @@ -641,6 +655,32 @@ class MessageTest(basetest.TestCase): m2.ParseFromString(m.SerializeToString()) self.assertEqual('oneof_uint32', m2.WhichOneof('oneof_field')) + def testOneofCopyFrom(self): + m = unittest_pb2.TestAllTypes() + m.oneof_uint32 = 11 + m2 = unittest_pb2.TestAllTypes() + m2.CopyFrom(m) + self.assertEqual('oneof_uint32', m2.WhichOneof('oneof_field')) + + def testOneofNestedMergeFrom(self): + m = unittest_pb2.NestedTestAllTypes() + m.payload.oneof_uint32 = 11 + m2 = unittest_pb2.NestedTestAllTypes() + m2.payload.oneof_bytes = b'bb' + m2.child.payload.oneof_bytes = b'bb' + m2.MergeFrom(m) + self.assertEqual('oneof_uint32', m2.payload.WhichOneof('oneof_field')) + self.assertEqual('oneof_bytes', m2.child.payload.WhichOneof('oneof_field')) + + def testOneofClear(self): + m = unittest_pb2.TestAllTypes() + m.oneof_uint32 = 11 + m.Clear() + self.assertIsNone(m.WhichOneof('oneof_field')) + m.oneof_bytes = b'bb' + self.assertTrue(m.HasField('oneof_field')) + + def testSortEmptyRepeatedCompositeContainer(self): """Exercise a scenario that has led to segfaults in the past. """ diff --git a/python/google/protobuf/internal/missing_enum_values.proto b/python/google/protobuf/internal/missing_enum_values.proto index e90f0cd3..161fc5e1 100644 --- a/python/google/protobuf/internal/missing_enum_values.proto +++ b/python/google/protobuf/internal/missing_enum_values.proto @@ -28,6 +28,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto2"; + package google.protobuf.python.internal; message TestEnumValues { diff --git a/python/google/protobuf/internal/more_extensions.proto b/python/google/protobuf/internal/more_extensions.proto index c04e597f..78f14673 100644 --- a/python/google/protobuf/internal/more_extensions.proto +++ b/python/google/protobuf/internal/more_extensions.proto @@ -30,6 +30,7 @@ // Author: robinson@google.com (Will Robinson) +syntax = "proto2"; package google.protobuf.internal; diff --git a/python/google/protobuf/internal/more_extensions_dynamic.proto b/python/google/protobuf/internal/more_extensions_dynamic.proto index 88bd9c1b..11f85ef6 100644 --- a/python/google/protobuf/internal/more_extensions_dynamic.proto +++ b/python/google/protobuf/internal/more_extensions_dynamic.proto @@ -34,6 +34,7 @@ // generated C++ type is available for the extendee, but the extension is // defined in a file whose C++ type is not in the binary. +syntax = "proto2"; import "google/protobuf/internal/more_extensions.proto"; diff --git a/python/google/protobuf/internal/more_messages.proto b/python/google/protobuf/internal/more_messages.proto index 61db66c5..2c6ab9ef 100644 --- a/python/google/protobuf/internal/more_messages.proto +++ b/python/google/protobuf/internal/more_messages.proto @@ -30,6 +30,7 @@ // Author: robinson@google.com (Will Robinson) +syntax = "proto2"; package google.protobuf.internal; diff --git a/python/google/protobuf/internal/proto_builder_test.py b/python/google/protobuf/internal/proto_builder_test.py new file mode 100644 index 00000000..c74db7e7 --- /dev/null +++ b/python/google/protobuf/internal/proto_builder_test.py @@ -0,0 +1,77 @@ +#! /usr/bin/python +# +# Protocol Buffers - Google's data interchange format +# Copyright 2008 Google Inc. All rights reserved. +# https://developers.google.com/protocol-buffers/ +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Tests for google.protobuf.proto_builder.""" + +from google.apputils import basetest + +from google.protobuf import descriptor_pb2 +from google.protobuf import descriptor_pool +from google.protobuf import proto_builder +from google.protobuf import text_format + + +class ProtoBuilderTest(basetest.TestCase): + + def setUp(self): + self._fields = { + 'foo': descriptor_pb2.FieldDescriptorProto.TYPE_INT64, + 'bar': descriptor_pb2.FieldDescriptorProto.TYPE_STRING, + } + + def testMakeSimpleProtoClass(self): + """Test that we can create a proto class.""" + proto_cls = proto_builder.MakeSimpleProtoClass( + self._fields, + full_name='net.proto2.python.public.proto_builder_test.Test') + proto = proto_cls() + proto.foo = 12345 + proto.bar = 'asdf' + self.assertMultiLineEqual( + 'bar: "asdf"\nfoo: 12345\n', text_format.MessageToString(proto)) + + def testMakeSameProtoClassTwice(self): + """Test that the DescriptorPool is used.""" + pool = descriptor_pool.DescriptorPool() + proto_cls1 = proto_builder.MakeSimpleProtoClass( + self._fields, + full_name='net.proto2.python.public.proto_builder_test.Test', + pool=pool) + proto_cls2 = proto_builder.MakeSimpleProtoClass( + self._fields, + full_name='net.proto2.python.public.proto_builder_test.Test', + pool=pool) + self.assertIs(proto_cls1.DESCRIPTOR, proto_cls2.DESCRIPTOR) + + +if __name__ == '__main__': + basetest.main() diff --git a/python/google/protobuf/internal/python_message.py b/python/google/protobuf/internal/python_message.py index a5c26f45..6fda6ae0 100755 --- a/python/google/protobuf/internal/python_message.py +++ b/python/google/protobuf/internal/python_message.py @@ -306,6 +306,17 @@ def _DefaultValueConstructorForField(field): return MakeScalarDefault +def _ReraiseTypeErrorWithFieldName(message_name, field_name): + """Re-raise the currently-handled TypeError with the field name added.""" + exc = sys.exc_info()[1] + if len(exc.args) == 1 and type(exc) is TypeError: + # simple TypeError; add field name to exception message + exc = TypeError('%s for field %s.%s' % (str(exc), message_name, field_name)) + + # re-raise possibly-amended exception with original traceback: + raise type(exc), exc, sys.exc_info()[2] + + def _AddInitMethod(message_descriptor, cls): """Adds an __init__ method to cls.""" fields = message_descriptor.fields @@ -338,10 +349,16 @@ def _AddInitMethod(message_descriptor, cls): self._fields[field] = copy elif field.cpp_type == _FieldDescriptor.CPPTYPE_MESSAGE: copy = field._default_constructor(self) - copy.MergeFrom(field_value) + try: + copy.MergeFrom(field_value) + except TypeError: + _ReraiseTypeErrorWithFieldName(message_descriptor.name, field_name) self._fields[field] = copy else: - setattr(self, field_name, field_value) + try: + setattr(self, field_name, field_value) + except TypeError: + _ReraiseTypeErrorWithFieldName(message_descriptor.name, field_name) init.__module__ = None init.__doc__ = None @@ -691,6 +708,7 @@ def _AddClearMethod(message_descriptor, cls): # Clear fields. self._fields = {} self._unknown_fields = () + self._oneofs = {} self._Modified() cls.Clear = Clear @@ -993,6 +1011,8 @@ def _AddMergeFromMethod(cls): field_value.MergeFrom(value) else: self._fields[field] = value + if field.containing_oneof: + self._UpdateOneofState(field) if msg._unknown_fields: if not self._unknown_fields: diff --git a/python/google/protobuf/internal/reflection_test.py b/python/google/protobuf/internal/reflection_test.py index d59815d0..6b24b092 100755 --- a/python/google/protobuf/internal/reflection_test.py +++ b/python/google/protobuf/internal/reflection_test.py @@ -35,8 +35,6 @@ pure-Python protocol compiler. """ -__author__ = 'robinson@google.com (Will Robinson)' - import copy import gc import operator @@ -1252,15 +1250,18 @@ class ReflectionTest(basetest.TestCase): # Try something that *is* an extension handle, just not for # this message... - unknown_handle = more_extensions_pb2.optional_int_extension - self.assertRaises(KeyError, extendee_proto.HasExtension, - unknown_handle) - self.assertRaises(KeyError, extendee_proto.ClearExtension, - unknown_handle) - self.assertRaises(KeyError, extendee_proto.Extensions.__getitem__, - unknown_handle) - self.assertRaises(KeyError, extendee_proto.Extensions.__setitem__, - unknown_handle, 5) + for unknown_handle in (more_extensions_pb2.optional_int_extension, + more_extensions_pb2.optional_message_extension, + more_extensions_pb2.repeated_int_extension, + more_extensions_pb2.repeated_message_extension): + self.assertRaises(KeyError, extendee_proto.HasExtension, + unknown_handle) + self.assertRaises(KeyError, extendee_proto.ClearExtension, + unknown_handle) + self.assertRaises(KeyError, extendee_proto.Extensions.__getitem__, + unknown_handle) + self.assertRaises(KeyError, extendee_proto.Extensions.__setitem__, + unknown_handle, 5) # Try call HasExtension() with a valid handle, but for a # *repeated* field. (Just as with non-extension repeated @@ -1669,16 +1670,15 @@ class ReflectionTest(basetest.TestCase): proto.optional_string = str('Testing') self.assertEqual(proto.optional_string, unicode('Testing')) - # Try to assign a 'str' value which contains bytes that aren't 7-bit ASCII. + # Try to assign a 'bytes' object which contains non-UTF-8. self.assertRaises(ValueError, setattr, proto, 'optional_string', b'a\x80a') - if str is bytes: # PY2 - # Assign a 'str' object which contains a UTF-8 encoded string. - self.assertRaises(ValueError, - setattr, proto, 'optional_string', 'Тест') - else: - proto.optional_string = 'Тест' - # No exception thrown. + # No exception: Assign already encoded UTF-8 bytes to a string field. + utf8_bytes = u'Тест'.encode('utf-8') + proto.optional_string = utf8_bytes + # No exception: Assign the a non-ascii unicode object. + proto.optional_string = u'Тест' + # No exception thrown (normal str assignment containing ASCII). proto.optional_string = 'abc' def testStringUTF8Serialization(self): @@ -1774,6 +1774,24 @@ class ReflectionTest(basetest.TestCase): proto.optionalgroup.SetInParent() self.assertTrue(proto.HasField('optionalgroup')) + def testPackageInitializationImport(self): + """Test that we can import nested messages from their __init__.py. + + Such setup is not trivial since at the time of processing of __init__.py one + can't refer to its submodules by name in code, so expressions like + google.protobuf.internal.import_test_package.inner_pb2 + don't work. They do work in imports, so we have assign an alias at import + and then use that alias in generated code. + """ + # We import here since it's the import that used to fail, and we want + # the failure to have the right context. + # pylint: disable=g-import-not-at-top + from google.protobuf.internal import import_test_package + # pylint: enable=g-import-not-at-top + msg = import_test_package.myproto.Outer() + # Just check the default value. + self.assertEqual(57, msg.inner.value) + # Since we had so many tests for protocol buffer equality, we broke these out # into separate TestCase classes. @@ -2802,6 +2820,9 @@ class OptionsTest(basetest.TestCase): class ClassAPITest(basetest.TestCase): + @basetest.unittest.skipIf( + api_implementation.Type() == 'cpp' and api_implementation.Version() == 2, + 'C++ implementation requires a call to MakeDescriptor()') def testMakeClassWithNestedDescriptor(self): leaf_desc = descriptor.Descriptor('leaf', 'package.parent.child.leaf', '', containing_type=None, fields=[], diff --git a/python/google/protobuf/internal/test_bad_identifiers.proto b/python/google/protobuf/internal/test_bad_identifiers.proto index 9eb18cb0..29fa38a2 100644 --- a/python/google/protobuf/internal/test_bad_identifiers.proto +++ b/python/google/protobuf/internal/test_bad_identifiers.proto @@ -30,6 +30,7 @@ // Author: kenton@google.com (Kenton Varda) +syntax = "proto2"; package protobuf_unittest; @@ -39,13 +40,15 @@ message TestBadIdentifiers { extensions 100 to max; } -// Make sure these reasonable extension names don't conflict with internal -// variables. extend TestBadIdentifiers { + // Make sure these reasonable extension names don't conflict with internal + // variables. optional string message = 100 [default="foo"]; optional string descriptor = 101 [default="bar"]; optional string reflection = 102 [default="baz"]; optional string service = 103 [default="qux"]; + // And Python keywords. + optional string class = 104 [default="Foo"]; } message AnotherMessage {} diff --git a/python/google/protobuf/internal/text_format_test.py b/python/google/protobuf/internal/text_format_test.py index b0a3a5f7..55e3c2c8 100755 --- a/python/google/protobuf/internal/text_format_test.py +++ b/python/google/protobuf/internal/text_format_test.py @@ -69,13 +69,18 @@ class TextFormatTest(basetest.TestCase): message.my_string = '115' message.my_int = 101 message.my_float = 111 + message.optional_nested_message.oo = 0 + message.optional_nested_message.bb = 1 self.CompareToGoldenText( self.RemoveRedundantZeros(text_format.MessageToString( message, use_index_order=True)), - 'my_string: \"115\"\nmy_int: 101\nmy_float: 111\n') + 'my_string: \"115\"\nmy_int: 101\nmy_float: 111\n' + 'optional_nested_message {\n oo: 0\n bb: 1\n}\n') self.CompareToGoldenText( self.RemoveRedundantZeros(text_format.MessageToString( - message)), 'my_int: 101\nmy_string: \"115\"\nmy_float: 111\n') + message)), + 'my_int: 101\nmy_string: \"115\"\nmy_float: 111\n' + 'optional_nested_message {\n bb: 1\n oo: 0\n}\n') def testPrintAllExtensions(self): message = unittest_pb2.TestAllExtensions() @@ -511,7 +516,7 @@ class TextFormatTest(basetest.TestCase): message.repeated_string[4]) self.assertEqual(SLASH + 'x20', message.repeated_string[5]) - def testMergeRepeatedScalars(self): + def testMergeDuplicateScalars(self): message = unittest_pb2.TestAllTypes() text = ('optional_int32: 42 ' 'optional_int32: 67') @@ -519,7 +524,7 @@ class TextFormatTest(basetest.TestCase): self.assertIs(r, message) self.assertEqual(67, message.optional_int32) - def testParseRepeatedScalars(self): + def testParseDuplicateScalars(self): message = unittest_pb2.TestAllTypes() text = ('optional_int32: 42 ' 'optional_int32: 67') @@ -529,7 +534,7 @@ class TextFormatTest(basetest.TestCase): 'have multiple "optional_int32" fields.'), text_format.Parse, text, message) - def testMergeRepeatedNestedMessageScalars(self): + def testMergeDuplicateNestedMessageScalars(self): message = unittest_pb2.TestAllTypes() text = ('optional_nested_message { bb: 1 } ' 'optional_nested_message { bb: 2 }') @@ -537,7 +542,7 @@ class TextFormatTest(basetest.TestCase): self.assertTrue(r is message) self.assertEqual(2, message.optional_nested_message.bb) - def testParseRepeatedNestedMessageScalars(self): + def testParseDuplicateNestedMessageScalars(self): message = unittest_pb2.TestAllTypes() text = ('optional_nested_message { bb: 1 } ' 'optional_nested_message { bb: 2 }') @@ -547,7 +552,7 @@ class TextFormatTest(basetest.TestCase): 'should not have multiple "bb" fields.'), text_format.Parse, text, message) - def testMergeRepeatedExtensionScalars(self): + def testMergeDuplicateExtensionScalars(self): message = unittest_pb2.TestAllExtensions() text = ('[protobuf_unittest.optional_int32_extension]: 42 ' '[protobuf_unittest.optional_int32_extension]: 67') @@ -556,7 +561,7 @@ class TextFormatTest(basetest.TestCase): 67, message.Extensions[unittest_pb2.optional_int32_extension]) - def testParseRepeatedExtensionScalars(self): + def testParseDuplicateExtensionScalars(self): message = unittest_pb2.TestAllExtensions() text = ('[protobuf_unittest.optional_int32_extension]: 42 ' '[protobuf_unittest.optional_int32_extension]: 67') diff --git a/python/google/protobuf/internal/type_checkers.py b/python/google/protobuf/internal/type_checkers.py index 56d26460..118725da 100755 --- a/python/google/protobuf/internal/type_checkers.py +++ b/python/google/protobuf/internal/type_checkers.py @@ -154,14 +154,13 @@ class UnicodeValueChecker(object): (proposed_value, type(proposed_value), (bytes, unicode))) raise TypeError(message) - # If the value is of type 'bytes' make sure that it is in 7-bit ASCII - # encoding. + # If the value is of type 'bytes' make sure that it is valid UTF-8 data. if isinstance(proposed_value, bytes): try: - proposed_value = proposed_value.decode('ascii') + proposed_value = proposed_value.decode('utf-8') except UnicodeDecodeError: - raise ValueError('%.1024r has type bytes, but isn\'t in 7-bit ASCII ' - 'encoding. Non-ASCII strings must be converted to ' + raise ValueError('%.1024r has type bytes, but isn\'t valid UTF-8 ' + 'encoding. Non-UTF-8 strings must be converted to ' 'unicode objects before being added.' % (proposed_value)) return proposed_value diff --git a/python/google/protobuf/internal/unknown_fields_test.py b/python/google/protobuf/internal/unknown_fields_test.py index 71775609..a4dc1f7c 100755 --- a/python/google/protobuf/internal/unknown_fields_test.py +++ b/python/google/protobuf/internal/unknown_fields_test.py @@ -38,12 +38,16 @@ __author__ = 'bohdank@google.com (Bohdan Koval)' from google.apputils import basetest from google.protobuf import unittest_mset_pb2 from google.protobuf import unittest_pb2 +from google.protobuf.internal import api_implementation from google.protobuf.internal import encoder from google.protobuf.internal import missing_enum_values_pb2 from google.protobuf.internal import test_util from google.protobuf.internal import type_checkers +@basetest.unittest.skipIf( + api_implementation.Type() == 'cpp' and api_implementation.Version() == 2, + 'C++ implementation does not expose unknown fields to Python') class UnknownFieldsTest(basetest.TestCase): def setUp(self): @@ -175,7 +179,10 @@ class UnknownFieldsTest(basetest.TestCase): self.assertNotEqual(self.empty_message, message) -class UnknownFieldsTest(basetest.TestCase): +@basetest.unittest.skipIf( + api_implementation.Type() == 'cpp' and api_implementation.Version() == 2, + 'C++ implementation does not expose unknown fields to Python') +class UnknownEnumValuesTest(basetest.TestCase): def setUp(self): self.descriptor = missing_enum_values_pb2.TestEnumValues.DESCRIPTOR diff --git a/python/google/protobuf/message.py b/python/google/protobuf/message.py index c186452a..88ed9f4c 100755 --- a/python/google/protobuf/message.py +++ b/python/google/protobuf/message.py @@ -233,12 +233,21 @@ class Message(object): raise NotImplementedError def HasField(self, field_name): - """Checks if a certain field is set for the message. Note if the - field_name is not defined in the message descriptor, ValueError will be - raised.""" + """Checks if a certain field is set for the message, or if any field inside + a oneof group is set. Note that if the field_name is not defined in the + message descriptor, ValueError will be raised.""" raise NotImplementedError def ClearField(self, field_name): + """Clears the contents of a given field, or the field set inside a oneof + group. If the name neither refers to a defined field or oneof group, + ValueError is raised.""" + raise NotImplementedError + + def WhichOneof(self, oneof_group): + """Returns the name of the field that is set inside a oneof group, or + None if no field is set. If no group with the given name exists, ValueError + will be raised.""" raise NotImplementedError def HasExtension(self, extension_handle): diff --git a/python/google/protobuf/proto_builder.py b/python/google/protobuf/proto_builder.py new file mode 100644 index 00000000..1fa28f1a --- /dev/null +++ b/python/google/protobuf/proto_builder.py @@ -0,0 +1,98 @@ +# Protocol Buffers - Google's data interchange format +# Copyright 2008 Google Inc. All rights reserved. +# https://developers.google.com/protocol-buffers/ +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Dynamic Protobuf class creator.""" + +import hashlib +import os + +from google.protobuf import descriptor_pb2 +from google.protobuf import message_factory + + +def _GetMessageFromFactory(factory, full_name): + """Get a proto class from the MessageFactory by name. + + Args: + factory: a MessageFactory instance. + full_name: str, the fully qualified name of the proto type. + Returns: + a class, for the type identified by full_name. + Raises: + KeyError, if the proto is not found in the factory's descriptor pool. + """ + proto_descriptor = factory.pool.FindMessageTypeByName(full_name) + proto_cls = factory.GetPrototype(proto_descriptor) + return proto_cls + + +def MakeSimpleProtoClass(fields, full_name, pool=None): + """Create a Protobuf class whose fields are basic types. + + Note: this doesn't validate field names! + + Args: + fields: dict of {name: field_type} mappings for each field in the proto. + full_name: str, the fully-qualified name of the proto type. + pool: optional DescriptorPool instance. + Returns: + a class, the new protobuf class with a FileDescriptor. + """ + factory = message_factory.MessageFactory(pool=pool) + try: + proto_cls = _GetMessageFromFactory(factory, full_name) + return proto_cls + except KeyError: + # The factory's DescriptorPool doesn't know about this class yet. + pass + + # Use a consistent file name that is unlikely to conflict with any imported + # proto files. + fields_hash = hashlib.sha1() + for f_name, f_type in sorted(fields.items()): + fields_hash.update(f_name.encode('utf8')) + fields_hash.update(str(f_type).encode('utf8')) + proto_file_name = fields_hash.hexdigest() + '.proto' + + package, name = full_name.rsplit('.', 1) + file_proto = descriptor_pb2.FileDescriptorProto() + file_proto.name = os.path.join(package.replace('.', '/'), proto_file_name) + file_proto.package = package + desc_proto = file_proto.message_type.add() + desc_proto.name = name + for f_number, (f_name, f_type) in enumerate(sorted(fields.items()), 1): + field_proto = desc_proto.field.add() + field_proto.name = f_name + field_proto.number = f_number + field_proto.label = descriptor_pb2.FieldDescriptorProto.LABEL_OPTIONAL + field_proto.type = f_type + + factory.pool.Add(file_proto) + return _GetMessageFromFactory(factory, full_name) diff --git a/python/google/protobuf/pyext/cpp_message.py b/python/google/protobuf/pyext/cpp_message.py index dcf34a02..037bb72c 100644 --- a/python/google/protobuf/pyext/cpp_message.py +++ b/python/google/protobuf/pyext/cpp_message.py @@ -53,9 +53,5 @@ def NewMessage(bases, message_descriptor, dictionary): def InitMessage(message_descriptor, cls): - """Constructs a new message instance (called before instance's __init__).""" - - def SubInit(self, **kwargs): - super(cls, self).__init__(message_descriptor, **kwargs) - cls.__init__ = SubInit + """Finalizes the creation of a message class.""" cls.AddDescriptors(message_descriptor) diff --git a/python/google/protobuf/pyext/descriptor.cc b/python/google/protobuf/pyext/descriptor.cc index 3f7be73c..55bb0b72 100644 --- a/python/google/protobuf/pyext/descriptor.cc +++ b/python/google/protobuf/pyext/descriptor.cc @@ -35,6 +35,7 @@ #include <google/protobuf/descriptor.pb.h> #include <google/protobuf/pyext/descriptor.h> +#include <google/protobuf/pyext/message.h> #include <google/protobuf/pyext/scoped_pyobject_ptr.h> #define C(str) const_cast<char*>(str) @@ -46,7 +47,7 @@ #error "Python 3.0 - 3.2 are not supported." #else #define PyString_AsString(ob) \ - (PyUnicode_Check(ob)? PyUnicode_AsUTF8(ob): PyBytes_AS_STRING(ob)) + (PyUnicode_Check(ob)? PyUnicode_AsUTF8(ob): PyBytes_AsString(ob)) #endif #endif @@ -65,10 +66,80 @@ namespace python { static google::protobuf::DescriptorPool* g_descriptor_pool = NULL; +namespace cmessage_descriptor { + +static void Dealloc(CMessageDescriptor* self) { + Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self)); +} + +static PyObject* GetFullName(CMessageDescriptor* self, void *closure) { + return PyString_FromStringAndSize( + self->descriptor->full_name().c_str(), + self->descriptor->full_name().size()); +} + +static PyObject* GetName(CMessageDescriptor *self, void *closure) { + return PyString_FromStringAndSize( + self->descriptor->name().c_str(), + self->descriptor->name().size()); +} + +static PyGetSetDef Getters[] = { + { C("full_name"), (getter)GetFullName, NULL, "Full name", NULL}, + { C("name"), (getter)GetName, NULL, "Unqualified name", NULL}, + {NULL} +}; + +} // namespace cmessage_descriptor + +PyTypeObject CMessageDescriptor_Type = { + PyVarObject_HEAD_INIT(&PyType_Type, 0) + C("google.protobuf.internal." + "_net_proto2___python." + "CMessageDescriptor"), // tp_name + sizeof(CMessageDescriptor), // tp_basicsize + 0, // tp_itemsize + (destructor)cmessage_descriptor::Dealloc, // tp_dealloc + 0, // tp_print + 0, // tp_getattr + 0, // tp_setattr + 0, // tp_compare + 0, // tp_repr + 0, // tp_as_number + 0, // tp_as_sequence + 0, // tp_as_mapping + 0, // tp_hash + 0, // tp_call + 0, // tp_str + 0, // tp_getattro + 0, // tp_setattro + 0, // tp_as_buffer + Py_TPFLAGS_DEFAULT, // tp_flags + C("A Message Descriptor"), // tp_doc + 0, // tp_traverse + 0, // tp_clear + 0, // tp_richcompare + 0, // tp_weaklistoffset + 0, // tp_iter + 0, // tp_iternext + 0, // tp_methods + 0, // tp_members + cmessage_descriptor::Getters, // tp_getset + 0, // tp_base + 0, // tp_dict + 0, // tp_descr_get + 0, // tp_descr_set + 0, // tp_dictoffset + 0, // tp_init + PyType_GenericAlloc, // tp_alloc + PyType_GenericNew, // tp_new + PyObject_Del, // tp_free +}; + + namespace cfield_descriptor { static void Dealloc(CFieldDescriptor* self) { - Py_CLEAR(self->descriptor_field); Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self)); } @@ -98,7 +169,7 @@ static PyObject* GetID(CFieldDescriptor *self, void *closure) { static PyGetSetDef Getters[] = { { C("full_name"), (getter)GetFullName, NULL, "Full name", NULL}, - { C("name"), (getter)GetName, NULL, "last name", NULL}, + { C("name"), (getter)GetName, NULL, "Unqualified name", NULL}, { C("cpp_type"), (getter)GetCppType, NULL, "C++ Type", NULL}, { C("label"), (getter)GetLabel, NULL, "Label", NULL}, { C("id"), (getter)GetID, NULL, "ID", NULL}, @@ -151,13 +222,56 @@ PyTypeObject CFieldDescriptor_Type = { PyObject_Del, // tp_free }; + namespace cdescriptor_pool { -static void Dealloc(CDescriptorPool* self) { +PyDescriptorPool* NewDescriptorPool() { + PyDescriptorPool* cdescriptor_pool = PyObject_New( + PyDescriptorPool, &PyDescriptorPool_Type); + if (cdescriptor_pool == NULL) { + return NULL; + } + + // Build a DescriptorPool for messages only declared in Python libraries. + // generated_pool() contains all messages linked in C++ libraries, and is used + // as underlay. + cdescriptor_pool->pool = new google::protobuf::DescriptorPool( + google::protobuf::DescriptorPool::generated_pool()); + + // TODO(amauryfa): Rewrite the SymbolDatabase in C so that it uses the same + // storage. + cdescriptor_pool->classes_by_descriptor = + new PyDescriptorPool::ClassesByMessageMap(); + + return cdescriptor_pool; +} + +static void Dealloc(PyDescriptorPool* self) { + for (auto it : (*self->classes_by_descriptor)) { + Py_DECREF(it.second); + } + delete self->classes_by_descriptor; Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self)); } -static PyObject* NewCDescriptor( +const google::protobuf::Descriptor* FindMessageTypeByName(PyDescriptorPool* self, + const string& name) { + return self->pool->FindMessageTypeByName(name); +} + +static PyObject* NewCMessageDescriptor( + const google::protobuf::Descriptor* message_descriptor) { + CMessageDescriptor* cmessage_descriptor = PyObject_New( + CMessageDescriptor, &CMessageDescriptor_Type); + if (cmessage_descriptor == NULL) { + return NULL; + } + cmessage_descriptor->descriptor = message_descriptor; + + return reinterpret_cast<PyObject*>(cmessage_descriptor); +} + +static PyObject* NewCFieldDescriptor( const google::protobuf::FieldDescriptor* field_descriptor) { CFieldDescriptor* cfield_descriptor = PyObject_New( CFieldDescriptor, &CFieldDescriptor_Type); @@ -165,12 +279,61 @@ static PyObject* NewCDescriptor( return NULL; } cfield_descriptor->descriptor = field_descriptor; - cfield_descriptor->descriptor_field = NULL; return reinterpret_cast<PyObject*>(cfield_descriptor); } -PyObject* FindFieldByName(CDescriptorPool* self, PyObject* name) { +// Add a message class to our database. +const google::protobuf::Descriptor* RegisterMessageClass( + PyDescriptorPool* self, PyObject *message_class, PyObject* descriptor) { + ScopedPyObjectPtr full_message_name( + PyObject_GetAttrString(descriptor, "full_name")); + const char* full_name = PyString_AsString(full_message_name); + if (full_name == NULL) { + return NULL; + } + const Descriptor *message_descriptor = + self->pool->FindMessageTypeByName(full_name); + if (!message_descriptor) { + PyErr_Format(PyExc_TypeError, "Could not find C++ descriptor for '%s'", + full_name); + return NULL; + } + Py_INCREF(message_class); + auto ret = self->classes_by_descriptor->insert( + make_pair(message_descriptor, message_class)); + if (!ret.second) { + // Update case: DECREF the previous value. + Py_DECREF(ret.first->second); + ret.first->second = message_class; + } + + // Also add the C++ descriptor to the Python descriptor class. + ScopedPyObjectPtr cdescriptor(NewCMessageDescriptor(message_descriptor)); + if (cdescriptor == NULL) { + return NULL; + } + if (PyObject_SetAttrString( + descriptor, "_cdescriptor", cdescriptor) < 0) { + return NULL; + } + return message_descriptor; +} + +// Retrieve the message class added to our database. +PyObject *GetMessageClass(PyDescriptorPool* self, + const Descriptor *message_descriptor) { + auto ret = self->classes_by_descriptor->find(message_descriptor); + if (ret == self->classes_by_descriptor->end()) { + PyErr_Format(PyExc_TypeError, "No message class registered for '%s'", + message_descriptor->full_name().c_str()); + return NULL; + } else { + return ret->second; + } +} + +PyObject* FindFieldByName(PyDescriptorPool* self, PyObject* name) { const char* full_field_name = PyString_AsString(name); if (full_field_name == NULL) { return NULL; @@ -186,10 +349,10 @@ PyObject* FindFieldByName(CDescriptorPool* self, PyObject* name) { return NULL; } - return NewCDescriptor(field_descriptor); + return NewCFieldDescriptor(field_descriptor); } -PyObject* FindExtensionByName(CDescriptorPool* self, PyObject* arg) { +PyObject* FindExtensionByName(PyDescriptorPool* self, PyObject* arg) { const char* full_field_name = PyString_AsString(arg); if (full_field_name == NULL) { return NULL; @@ -203,7 +366,7 @@ PyObject* FindExtensionByName(CDescriptorPool* self, PyObject* arg) { return NULL; } - return NewCDescriptor(field_descriptor); + return NewCFieldDescriptor(field_descriptor); } static PyMethodDef Methods[] = { @@ -220,12 +383,12 @@ static PyMethodDef Methods[] = { } // namespace cdescriptor_pool -PyTypeObject CDescriptorPool_Type = { +PyTypeObject PyDescriptorPool_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) C("google.protobuf.internal." "_net_proto2___python." "CFieldDescriptor"), // tp_name - sizeof(CDescriptorPool), // tp_basicsize + sizeof(PyDescriptorPool), // tp_basicsize 0, // tp_itemsize (destructor)cdescriptor_pool::Dealloc, // tp_dealloc 0, // tp_print @@ -259,29 +422,11 @@ PyTypeObject CDescriptorPool_Type = { 0, // tp_descr_set 0, // tp_dictoffset 0, // tp_init - PyType_GenericAlloc, // tp_alloc - PyType_GenericNew, // tp_new + 0, // tp_alloc + 0, // tp_new PyObject_Del, // tp_free }; -google::protobuf::DescriptorPool* GetDescriptorPool() { - if (g_descriptor_pool == NULL) { - g_descriptor_pool = new google::protobuf::DescriptorPool( - google::protobuf::DescriptorPool::generated_pool()); - } - return g_descriptor_pool; -} - -PyObject* Python_NewCDescriptorPool(PyObject* ignored, PyObject* args) { - CDescriptorPool* cdescriptor_pool = PyObject_New( - CDescriptorPool, &CDescriptorPool_Type); - if (cdescriptor_pool == NULL) { - return NULL; - } - cdescriptor_pool->pool = GetDescriptorPool(); - return reinterpret_cast<PyObject*>(cdescriptor_pool); -} - // Collects errors that occur during proto file building to allow them to be // propagated in the python exception instead of only living in ERROR logs. @@ -321,6 +466,8 @@ PyObject* Python_BuildFile(PyObject* ignored, PyObject* arg) { return NULL; } + // If the file was already part of a C++ library, all its descriptors are in + // the underlying pool. No need to do anything else. if (google::protobuf::DescriptorPool::generated_pool()->FindFileByName( file_proto.name()) != NULL) { Py_RETURN_NONE; @@ -328,8 +475,8 @@ PyObject* Python_BuildFile(PyObject* ignored, PyObject* arg) { BuildFileErrorCollector error_collector; const google::protobuf::FileDescriptor* descriptor = - GetDescriptorPool()->BuildFileCollectingErrors(file_proto, - &error_collector); + GetDescriptorPool()->pool->BuildFileCollectingErrors(file_proto, + &error_collector); if (descriptor == NULL) { PyErr_Format(PyExc_TypeError, "Couldn't build proto file into descriptor pool!\n%s", @@ -341,12 +488,13 @@ PyObject* Python_BuildFile(PyObject* ignored, PyObject* arg) { } bool InitDescriptor() { - CFieldDescriptor_Type.tp_new = PyType_GenericNew; + if (PyType_Ready(&CMessageDescriptor_Type) < 0) + return false; if (PyType_Ready(&CFieldDescriptor_Type) < 0) return false; - CDescriptorPool_Type.tp_new = PyType_GenericNew; - if (PyType_Ready(&CDescriptorPool_Type) < 0) + PyDescriptorPool_Type.tp_new = PyType_GenericNew; + if (PyType_Ready(&PyDescriptorPool_Type) < 0) return false; return true; diff --git a/python/google/protobuf/pyext/descriptor.h b/python/google/protobuf/pyext/descriptor.h index ae7a1b9c..9e5957b5 100644 --- a/python/google/protobuf/pyext/descriptor.h +++ b/python/google/protobuf/pyext/descriptor.h @@ -36,6 +36,8 @@ #include <Python.h> #include <structmember.h> +#include <google/protobuf/stubs/hash.h> + #include <google/protobuf/descriptor.h> #if PY_VERSION_HEX < 0x02050000 && !defined(PY_SSIZE_T_MIN) @@ -48,46 +50,90 @@ namespace google { namespace protobuf { namespace python { +typedef struct CMessageDescriptor { + PyObject_HEAD + + // The proto2 descriptor that this object represents. + const google::protobuf::Descriptor* descriptor; +} CMessageDescriptor; + + typedef struct CFieldDescriptor { PyObject_HEAD // The proto2 descriptor that this object represents. const google::protobuf::FieldDescriptor* descriptor; - - // Reference to the original field object in the Python DESCRIPTOR. - PyObject* descriptor_field; } CFieldDescriptor; -typedef struct { + +// Wraps operations to the global DescriptorPool which contains information +// about all messages and fields. +// +// There is normally one pool per process. We make it a Python object only +// because it contains many Python references. +// TODO(amauryfa): See whether such objects can appear in reference cycles, and +// consider adding support for the cyclic GC. +// +// "Methods" that interacts with this DescriptorPool are in the cdescriptor_pool +// namespace. +typedef struct PyDescriptorPool { PyObject_HEAD - const google::protobuf::DescriptorPool* pool; -} CDescriptorPool; + google::protobuf::DescriptorPool* pool; + // Make our own mapping to retrieve Python classes from C++ descriptors. + // + // Descriptor pointers stored here are owned by the DescriptorPool above. + // Python references to classes are owned by this PyDescriptorPool. + typedef hash_map<const Descriptor *, PyObject *> ClassesByMessageMap; + ClassesByMessageMap *classes_by_descriptor; +} PyDescriptorPool; + + +extern PyTypeObject CMessageDescriptor_Type; extern PyTypeObject CFieldDescriptor_Type; -extern PyTypeObject CDescriptorPool_Type; +extern PyTypeObject PyDescriptorPool_Type; + namespace cdescriptor_pool { +// Builds a new DescriptorPool. Normally called only once per process. +PyDescriptorPool* NewDescriptorPool(); + +// Looks up a message by name. +// Returns a message Descriptor, or NULL if not found. +const google::protobuf::Descriptor* FindMessageTypeByName(PyDescriptorPool* self, + const string& name); + +// Registers a new Python class for the given message descriptor. +// Returns the message Descriptor. +// On error, returns NULL with a Python exception set. +const google::protobuf::Descriptor* RegisterMessageClass( + PyDescriptorPool* self, PyObject *message_class, PyObject *descriptor); + +// Retrieves the Python class registered with the given message descriptor. +// +// Returns a *borrowed* reference if found, otherwise returns NULL with an +// exception set. +PyObject *GetMessageClass(PyDescriptorPool* self, + const Descriptor *message_descriptor); + // Looks up a field by name. Returns a CDescriptor corresponding to // the field on success, or NULL on failure. // // Returns a new reference. -PyObject* FindFieldByName(CDescriptorPool* self, PyObject* name); +PyObject* FindFieldByName(PyDescriptorPool* self, PyObject* name); // Looks up an extension by name. Returns a CDescriptor corresponding // to the field on success, or NULL on failure. // // Returns a new reference. -PyObject* FindExtensionByName(CDescriptorPool* self, PyObject* arg); - +PyObject* FindExtensionByName(PyDescriptorPool* self, PyObject* arg); } // namespace cdescriptor_pool -PyObject* Python_NewCDescriptorPool(PyObject* ignored, PyObject* args); PyObject* Python_BuildFile(PyObject* ignored, PyObject* args); bool InitDescriptor(); -google::protobuf::DescriptorPool* GetDescriptorPool(); } // namespace python } // namespace protobuf diff --git a/python/google/protobuf/pyext/extension_dict.cc b/python/google/protobuf/pyext/extension_dict.cc index 3861c794..d83b57d5 100644 --- a/python/google/protobuf/pyext/extension_dict.cc +++ b/python/google/protobuf/pyext/extension_dict.cc @@ -62,22 +62,6 @@ static google::protobuf::Message* GetMessage(ExtensionDict* self) { } } -CFieldDescriptor* InternalGetCDescriptorFromExtension(PyObject* extension) { - PyObject* cdescriptor = PyObject_GetAttrString(extension, "_cdescriptor"); - if (cdescriptor == NULL) { - PyErr_SetString(PyExc_KeyError, "Unregistered extension."); - return NULL; - } - if (!PyObject_TypeCheck(cdescriptor, &CFieldDescriptor_Type)) { - PyErr_SetString(PyExc_TypeError, "Not a CFieldDescriptor"); - Py_DECREF(cdescriptor); - return NULL; - } - CFieldDescriptor* descriptor = - reinterpret_cast<CFieldDescriptor*>(cdescriptor); - return descriptor; -} - PyObject* len(ExtensionDict* self) { #if PY_MAJOR_VERSION >= 3 return PyLong_FromLong(PyDict_Size(self->values)); @@ -118,16 +102,15 @@ int ReleaseExtension(ExtensionDict* self, } PyObject* subscript(ExtensionDict* self, PyObject* key) { - CFieldDescriptor* cdescriptor = InternalGetCDescriptorFromExtension( - key); - if (cdescriptor == NULL) { + const google::protobuf::FieldDescriptor* descriptor = + cmessage::GetExtensionDescriptor(key); + if (descriptor == NULL) { return NULL; } - ScopedPyObjectPtr py_cdescriptor(reinterpret_cast<PyObject*>(cdescriptor)); - const google::protobuf::FieldDescriptor* descriptor = cdescriptor->descriptor; - if (descriptor == NULL) { + if (!CheckFieldBelongsToMessage(descriptor, self->parent->message)) { return NULL; } + if (descriptor->label() != FieldDescriptor::LABEL_REPEATED && descriptor->cpp_type() != FieldDescriptor::CPPTYPE_MESSAGE) { return cmessage::InternalGetScalar(self->parent, descriptor); @@ -142,7 +125,7 @@ PyObject* subscript(ExtensionDict* self, PyObject* key) { if (descriptor->label() != FieldDescriptor::LABEL_REPEATED && descriptor->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { PyObject* sub_message = cmessage::InternalGetSubMessage( - self->parent, cdescriptor); + self->parent, descriptor); if (sub_message == NULL) { return NULL; } @@ -152,33 +135,21 @@ PyObject* subscript(ExtensionDict* self, PyObject* key) { if (descriptor->label() == FieldDescriptor::LABEL_REPEATED) { if (descriptor->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { - // COPIED - PyObject* py_container = PyObject_CallObject( - reinterpret_cast<PyObject*>(&RepeatedCompositeContainer_Type), - NULL); + PyObject *message_class = cdescriptor_pool::GetMessageClass( + GetDescriptorPool(), descriptor->message_type()); + if (message_class == NULL) { + return NULL; + } + PyObject* py_container = repeated_composite_container::NewContainer( + self->parent, descriptor, message_class); if (py_container == NULL) { return NULL; } - RepeatedCompositeContainer* container = - reinterpret_cast<RepeatedCompositeContainer*>(py_container); - PyObject* field = cdescriptor->descriptor_field; - PyObject* message_type = PyObject_GetAttrString(field, "message_type"); - PyObject* concrete_class = PyObject_GetAttrString(message_type, - "_concrete_class"); - container->owner = self->owner; - container->parent = self->parent; - container->message = self->parent->message; - container->parent_field = cdescriptor; - container->subclass_init = concrete_class; - Py_DECREF(message_type); PyDict_SetItem(self->values, key, py_container); return py_container; } else { - // COPIED - ScopedPyObjectPtr init_args(PyTuple_Pack(2, self->parent, cdescriptor)); - PyObject* py_container = PyObject_CallObject( - reinterpret_cast<PyObject*>(&RepeatedScalarContainer_Type), - init_args); + PyObject* py_container = repeated_scalar_container::NewContainer( + self->parent, descriptor); if (py_container == NULL) { return NULL; } @@ -191,13 +162,15 @@ PyObject* subscript(ExtensionDict* self, PyObject* key) { } int ass_subscript(ExtensionDict* self, PyObject* key, PyObject* value) { - CFieldDescriptor* cdescriptor = InternalGetCDescriptorFromExtension( - key); - if (cdescriptor == NULL) { + const google::protobuf::FieldDescriptor* descriptor = + cmessage::GetExtensionDescriptor(key); + if (descriptor == NULL) { + return -1; + } + if (!CheckFieldBelongsToMessage(descriptor, self->parent->message)) { return -1; } - ScopedPyObjectPtr py_cdescriptor(reinterpret_cast<PyObject*>(cdescriptor)); - const google::protobuf::FieldDescriptor* descriptor = cdescriptor->descriptor; + if (descriptor->label() != FieldDescriptor::LABEL_OPTIONAL || descriptor->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE) { PyErr_SetString(PyExc_TypeError, "Extension is repeated and/or composite " @@ -214,20 +187,18 @@ int ass_subscript(ExtensionDict* self, PyObject* key, PyObject* value) { } PyObject* ClearExtension(ExtensionDict* self, PyObject* extension) { - CFieldDescriptor* cdescriptor = InternalGetCDescriptorFromExtension( - extension); - if (cdescriptor == NULL) { + const google::protobuf::FieldDescriptor* descriptor = + cmessage::GetExtensionDescriptor(extension); + if (descriptor == NULL) { return NULL; } - ScopedPyObjectPtr py_cdescriptor(reinterpret_cast<PyObject*>(cdescriptor)); PyObject* value = PyDict_GetItem(self->values, extension); if (value != NULL) { - if (ReleaseExtension(self, value, cdescriptor->descriptor) < 0) { + if (ReleaseExtension(self, value, descriptor) < 0) { return NULL; } } - if (cmessage::ClearFieldByDescriptor(self->parent, - cdescriptor->descriptor) == NULL) { + if (cmessage::ClearFieldByDescriptor(self->parent, descriptor) == NULL) { return NULL; } if (PyDict_DelItem(self->values, extension) < 0) { @@ -237,14 +208,12 @@ PyObject* ClearExtension(ExtensionDict* self, PyObject* extension) { } PyObject* HasExtension(ExtensionDict* self, PyObject* extension) { - CFieldDescriptor* cdescriptor = InternalGetCDescriptorFromExtension( - extension); - if (cdescriptor == NULL) { + const google::protobuf::FieldDescriptor* descriptor = + cmessage::GetExtensionDescriptor(extension); + if (descriptor == NULL) { return NULL; } - ScopedPyObjectPtr py_cdescriptor(reinterpret_cast<PyObject*>(cdescriptor)); - PyObject* result = cmessage::HasFieldByDescriptor( - self->parent, cdescriptor->descriptor); + PyObject* result = cmessage::HasFieldByDescriptor(self->parent, descriptor); return result; } @@ -263,11 +232,18 @@ PyObject* _FindExtensionByName(ExtensionDict* self, PyObject* name) { } } -int init(ExtensionDict* self, PyObject* args, PyObject* kwargs) { - self->parent = NULL; - self->message = NULL; +ExtensionDict* NewExtensionDict(CMessage *parent) { + ExtensionDict* self = reinterpret_cast<ExtensionDict*>( + PyType_GenericAlloc(&ExtensionDict_Type, 0)); + if (self == NULL) { + return NULL; + } + + self->parent = parent; // Store a borrowed reference. + self->message = parent->message; + self->owner = parent->owner; self->values = PyDict_New(); - return 0; + return self; } void dealloc(ExtensionDict* self) { @@ -330,7 +306,7 @@ PyTypeObject ExtensionDict_Type = { 0, // tp_descr_get 0, // tp_descr_set 0, // tp_dictoffset - (initproc)extension_dict::init, // tp_init + 0, // tp_init }; } // namespace python diff --git a/python/google/protobuf/pyext/extension_dict.h b/python/google/protobuf/pyext/extension_dict.h index 13c874a4..47625e23 100644 --- a/python/google/protobuf/pyext/extension_dict.h +++ b/python/google/protobuf/pyext/extension_dict.h @@ -53,13 +53,26 @@ using internal::shared_ptr; namespace python { struct CMessage; -struct CFieldDescriptor; typedef struct ExtensionDict { PyObject_HEAD; + + // This is the top-level C++ Message object that owns the whole + // proto tree. Every Python container class holds a + // reference to it in order to keep it alive as long as there's a + // Python object that references any part of the tree. shared_ptr<Message> owner; + + // Weak reference to parent message. Used to make sure + // the parent is writable when an extension field is modified. CMessage* parent; + + // Pointer to the C++ Message that this ExtensionDict extends. + // Not owned by us. Message* message; + + // A dict of child messages, indexed by Extension descriptors. + // Similar to CMessage::composite_fields. PyObject* values; } ExtensionDict; @@ -67,11 +80,8 @@ extern PyTypeObject ExtensionDict_Type; namespace extension_dict { -// Gets the _cdescriptor reference to a CFieldDescriptor object given a -// python descriptor object. -// -// Returns a new reference. -CFieldDescriptor* InternalGetCDescriptorFromExtension(PyObject* extension); +// Builds an Extensions dict for a specific message. +ExtensionDict* NewExtensionDict(CMessage *parent); // Gets the number of extension values in this ExtensionDict as a python object. // diff --git a/python/google/protobuf/pyext/message.cc b/python/google/protobuf/pyext/message.cc index 9fb7083f..cd956e0e 100644 --- a/python/google/protobuf/pyext/message.cc +++ b/python/google/protobuf/pyext/message.cc @@ -71,7 +71,7 @@ #error "Python 3.0 - 3.2 are not supported." #else #define PyString_AsString(ob) \ - (PyUnicode_Check(ob)? PyUnicode_AsUTF8(ob): PyBytes_AS_STRING(ob)) + (PyUnicode_Check(ob)? PyUnicode_AsUTF8(ob): PyBytes_AsString(ob)) #endif #endif @@ -81,7 +81,9 @@ namespace python { // Forward declarations namespace cmessage { -static PyObject* GetDescriptor(CMessage* self, PyObject* name); +static const google::protobuf::FieldDescriptor* GetFieldDescriptor( + CMessage* self, PyObject* name); +static const google::protobuf::Descriptor* GetMessageDescriptor(PyTypeObject* cls); static string GetMessageName(CMessage* self); int InternalReleaseFieldByDescriptor( const google::protobuf::FieldDescriptor* field_descriptor, @@ -147,12 +149,15 @@ int ForEachCompositeField(CMessage* self, Visitor visitor) { PyObject* key; PyObject* field; + // Never use self->message in this function, it may be already freed. + const google::protobuf::Descriptor* message_descriptor = + cmessage::GetMessageDescriptor(Py_TYPE(self)); + // Visit normal fields. while (PyDict_Next(self->composite_fields, &pos, &key, &field)) { - PyObject* cdescriptor = cmessage::GetDescriptor(self, key); - if (cdescriptor != NULL) { - const google::protobuf::FieldDescriptor* descriptor = - reinterpret_cast<CFieldDescriptor*>(cdescriptor)->descriptor; + const google::protobuf::FieldDescriptor* descriptor = + message_descriptor->FindFieldByName(PyString_AsString(key)); + if (descriptor != NULL) { if (VisitCompositeField(descriptor, field, visitor) == -1) return -1; } @@ -161,11 +166,11 @@ int ForEachCompositeField(CMessage* self, Visitor visitor) { // Visit extension fields. if (self->extensions != NULL) { while (PyDict_Next(self->extensions->values, &pos, &key, &field)) { - CFieldDescriptor* cdescriptor = - extension_dict::InternalGetCDescriptorFromExtension(key); - if (cdescriptor == NULL) + const google::protobuf::FieldDescriptor* descriptor = + cmessage::GetExtensionDescriptor(key); + if (descriptor == NULL) return -1; - if (VisitCompositeField(cdescriptor->descriptor, field, visitor) == -1) + if (VisitCompositeField(descriptor, field, visitor) == -1) return -1; } } @@ -191,18 +196,19 @@ PyObject* PickleError_class; // Constant PyString values used for GetAttr/GetItem. static PyObject* kDESCRIPTOR; -static PyObject* k__descriptors; +static PyObject* k_cdescriptor; static PyObject* kfull_name; static PyObject* kname; -static PyObject* kmessage_type; -static PyObject* kis_extendable; static PyObject* kextensions_by_name; static PyObject* k_extensions_by_name; static PyObject* k_extensions_by_number; -static PyObject* k_concrete_class; static PyObject* kfields_by_name; -static CDescriptorPool* descriptor_pool; +static PyDescriptorPool* descriptor_pool; + +PyDescriptorPool* GetDescriptorPool() { + return descriptor_pool; +} /* Is 64bit */ void FormatTypeError(PyObject* arg, char* expected_types) { @@ -313,12 +319,12 @@ bool CheckAndSetString( } if (PyBytes_Check(arg)) { - PyObject* unicode = PyUnicode_FromEncodedObject(arg, "ascii", NULL); + PyObject* unicode = PyUnicode_FromEncodedObject(arg, "utf-8", NULL); if (unicode == NULL) { PyObject* repr = PyObject_Repr(arg); PyErr_Format(PyExc_ValueError, - "%s has type str, but isn't in 7-bit ASCII " - "encoding. Non-ASCII strings must be converted to " + "%s has type str, but isn't valid UTF-8 " + "encoding. Non-UTF-8 strings must be converted to " "unicode objects before being added.", PyString_AsString(repr)); Py_DECREF(repr); @@ -335,12 +341,9 @@ bool CheckAndSetString( PyObject* encoded_string = NULL; if (descriptor->type() == google::protobuf::FieldDescriptor::TYPE_STRING) { if (PyBytes_Check(arg)) { -#if PY_MAJOR_VERSION < 3 - encoded_string = PyString_AsEncodedObject(arg, "utf-8", NULL); -#else + // The bytes were already validated as correctly encoded UTF-8 above. encoded_string = arg; // Already encoded. Py_INCREF(encoded_string); -#endif } else { encoded_string = PyUnicode_AsEncodedObject(arg, "utf-8", NULL); } @@ -391,6 +394,17 @@ PyObject* ToStringObject( return result; } +bool CheckFieldBelongsToMessage(const google::protobuf::FieldDescriptor* field_descriptor, + const google::protobuf::Message* message) { + if (message->GetDescriptor() == field_descriptor->containing_type()) { + return true; + } + PyErr_Format(PyExc_KeyError, "Field '%s' does not belong to message '%s'", + field_descriptor->full_name().c_str(), + message->GetDescriptor()->full_name().c_str()); + return false; +} + google::protobuf::DynamicMessageFactory* global_message_factory; namespace cmessage { @@ -489,7 +503,7 @@ int AssureWritable(CMessage* self) { google::protobuf::Message* parent_message = self->parent->message; google::protobuf::Message* mutable_message = GetMutableMessage( self->parent, - self->parent_field->descriptor); + self->parent_field_descriptor); if (mutable_message == NULL) { return -1; } @@ -512,26 +526,61 @@ int AssureWritable(CMessage* self) { // --- Globals: -static PyObject* GetDescriptor(CMessage* self, PyObject* name) { - PyObject* descriptors = - PyDict_GetItem(Py_TYPE(self)->tp_dict, k__descriptors); - if (descriptors == NULL) { - PyErr_SetString(PyExc_TypeError, "No __descriptors"); +// Retrieve the C++ Descriptor of a message class. +// On error, returns NULL with an exception set. +static const google::protobuf::Descriptor* GetMessageDescriptor(PyTypeObject* cls) { + ScopedPyObjectPtr descriptor(PyObject_GetAttr( + reinterpret_cast<PyObject*>(cls), kDESCRIPTOR)); + if (descriptor == NULL) { + PyErr_SetString(PyExc_TypeError, "Message class has no DESCRIPTOR"); + return NULL; + } + ScopedPyObjectPtr cdescriptor(PyObject_GetAttr(descriptor, k_cdescriptor)); + if (cdescriptor == NULL) { + PyErr_SetString(PyExc_TypeError, "Unregistered message."); + return NULL; + } + if (!PyObject_TypeCheck(cdescriptor, &CMessageDescriptor_Type)) { + PyErr_SetString(PyExc_TypeError, "Not a CMessageDescriptor"); return NULL; } + return reinterpret_cast<CMessageDescriptor*>(cdescriptor.get())->descriptor; +} - return PyDict_GetItem(descriptors, name); +// Retrieve a C++ FieldDescriptor for a message attribute. +// The C++ message must be valid. +// TODO(amauryfa): This function should stay internal, because exception +// handling is not consistent. +static const google::protobuf::FieldDescriptor* GetFieldDescriptor( + CMessage* self, PyObject* name) { + const google::protobuf::Descriptor *message_descriptor = self->message->GetDescriptor(); + const char* field_name = PyString_AsString(name); + if (field_name == NULL) { + return NULL; + } + const google::protobuf::FieldDescriptor *field_descriptor = + message_descriptor->FindFieldByName(field_name); + if (field_descriptor == NULL) { + // Note: No exception is set! + return NULL; + } + return field_descriptor; } -static const google::protobuf::Message* CreateMessage(const char* message_type) { - string message_name(message_type); - const google::protobuf::Descriptor* descriptor = - GetDescriptorPool()->FindMessageTypeByName(message_name); - if (descriptor == NULL) { - PyErr_SetString(PyExc_TypeError, message_type); +// Retrieve a C++ FieldDescriptor for an extension handle. +const google::protobuf::FieldDescriptor* GetExtensionDescriptor(PyObject* extension) { + ScopedPyObjectPtr cdescriptor( + PyObject_GetAttrString(extension, "_cdescriptor")); + if (cdescriptor == NULL) { + PyErr_SetString(PyExc_KeyError, "Unregistered extension."); + return NULL; + } + if (!PyObject_TypeCheck(cdescriptor, &CFieldDescriptor_Type)) { + PyErr_SetString(PyExc_TypeError, "Not a CFieldDescriptor"); + Py_DECREF(cdescriptor); return NULL; } - return global_message_factory->GetPrototype(descriptor); + return reinterpret_cast<CFieldDescriptor*>(cdescriptor.get())->descriptor; } // If cmessage_list is not NULL, this function releases values into the @@ -627,39 +676,8 @@ int InternalDeleteRepeatedField( return 0; } -int InitAttributes(CMessage* self, PyObject* arg, PyObject* kwargs) { - ScopedPyObjectPtr descriptor; - if (arg == NULL) { - descriptor.reset( - PyObject_GetAttr(reinterpret_cast<PyObject*>(self), kDESCRIPTOR)); - if (descriptor == NULL) { - return NULL; - } - } else { - descriptor.reset(arg); - descriptor.inc(); - } - ScopedPyObjectPtr is_extendable(PyObject_GetAttr(descriptor, kis_extendable)); - if (is_extendable == NULL) { - return NULL; - } - int retcode = PyObject_IsTrue(is_extendable); - if (retcode == -1) { - return NULL; - } - if (retcode) { - PyObject* py_extension_dict = PyObject_CallObject( - reinterpret_cast<PyObject*>(&ExtensionDict_Type), NULL); - if (py_extension_dict == NULL) { - return NULL; - } - ExtensionDict* extension_dict = reinterpret_cast<ExtensionDict*>( - py_extension_dict); - extension_dict->parent = self; - extension_dict->message = self->message; - self->extensions = extension_dict; - } - +// Initializes fields of a message. Used in constructors. +int InitAttributes(CMessage* self, PyObject* kwargs) { if (kwargs == NULL) { return 0; } @@ -672,14 +690,12 @@ int InitAttributes(CMessage* self, PyObject* arg, PyObject* kwargs) { PyErr_SetString(PyExc_ValueError, "Field name must be a string"); return -1; } - PyObject* py_cdescriptor = GetDescriptor(self, name); - if (py_cdescriptor == NULL) { + const google::protobuf::FieldDescriptor* descriptor = GetFieldDescriptor(self, name); + if (descriptor == NULL) { PyErr_Format(PyExc_ValueError, "Protocol message has no \"%s\" field.", PyString_AsString(name)); return -1; } - const google::protobuf::FieldDescriptor* descriptor = - reinterpret_cast<CFieldDescriptor*>(py_cdescriptor)->descriptor; if (descriptor->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED) { ScopedPyObjectPtr container(GetAttr(self, name)); if (container == NULL) { @@ -719,15 +735,19 @@ int InitAttributes(CMessage* self, PyObject* arg, PyObject* kwargs) { return 0; } -static PyObject* New(PyTypeObject* type, PyObject* args, PyObject* kwargs) { - CMessage* self = reinterpret_cast<CMessage*>(type->tp_alloc(type, 0)); +// Allocates an incomplete Python Message: the caller must fill self->message, +// self->owner and eventually self->parent. +CMessage* NewEmptyMessage(PyObject* type, + const google::protobuf::Descriptor *descriptor) { + CMessage* self = reinterpret_cast<CMessage*>( + PyType_GenericAlloc(reinterpret_cast<PyTypeObject*>(type), 0)); if (self == NULL) { return NULL; } self->message = NULL; self->parent = NULL; - self->parent_field = NULL; + self->parent_field_descriptor = NULL; self->read_only = false; self->extensions = NULL; @@ -735,43 +755,58 @@ static PyObject* New(PyTypeObject* type, PyObject* args, PyObject* kwargs) { if (self->composite_fields == NULL) { return NULL; } - return reinterpret_cast<PyObject*>(self); -} - -PyObject* NewEmpty(PyObject* type) { - return New(reinterpret_cast<PyTypeObject*>(type), NULL, NULL); -} -static int Init(CMessage* self, PyObject* args, PyObject* kwargs) { - if (kwargs == NULL) { - // TODO(anuraag): Set error - return -1; + // If there are extension_ranges, the message is "extendable". Allocate a + // dictionary to store the extension fields. + if (descriptor->extension_range_count() > 0) { + // TODO(amauryfa): Delay the construction of this dict until extensions are + // really used on the object. + ExtensionDict* extension_dict = extension_dict::NewExtensionDict(self); + if (extension_dict == NULL) { + return NULL; + } + self->extensions = extension_dict; } - PyObject* descriptor = PyTuple_GetItem(args, 0); - if (descriptor == NULL || PyTuple_Size(args) != 1) { - PyErr_SetString(PyExc_ValueError, "args must contain one arg: descriptor"); - return -1; - } + return self; +} - ScopedPyObjectPtr py_message_type(PyObject_GetAttr(descriptor, kfull_name)); - if (py_message_type == NULL) { - return -1; +// The __new__ method of Message classes. +// Creates a new C++ message and takes ownership. +static PyObject* New(PyTypeObject* type, + PyObject* unused_args, PyObject* unused_kwargs) { + // Retrieve the message descriptor and the default instance (=prototype). + const google::protobuf::Descriptor* message_descriptor = GetMessageDescriptor(type); + if (message_descriptor == NULL) { + return NULL; } - - const char* message_type = PyString_AsString(py_message_type.get()); - const google::protobuf::Message* message = CreateMessage(message_type); - if (message == NULL) { - return -1; + const google::protobuf::Message* default_message = + global_message_factory->GetPrototype(message_descriptor); + if (default_message == NULL) { + PyErr_SetString(PyExc_TypeError, message_descriptor->full_name().c_str()); + return NULL; } - self->message = message->New(); + CMessage* self = NewEmptyMessage(reinterpret_cast<PyObject*>(type), + message_descriptor); + if (self == NULL) { + return NULL; + } + self->message = default_message->New(); self->owner.reset(self->message); - if (InitAttributes(self, descriptor, kwargs) < 0) { + return reinterpret_cast<PyObject*>(self); +} + +// The __init__ method of Message classes. +// It initializes fields from keywords passed to the constructor. +static int Init(CMessage* self, PyObject* args, PyObject* kwargs) { + if (PyTuple_Size(args) != 0) { + PyErr_SetString(PyExc_TypeError, "No positional arguments allowed"); return -1; } - return 0; + + return InitAttributes(self, kwargs); } // --------------------------------------------------------------------- @@ -853,9 +888,7 @@ PyObject* IsInitialized(CMessage* self, PyObject* args) { PyObject* HasFieldByDescriptor( CMessage* self, const google::protobuf::FieldDescriptor* field_descriptor) { google::protobuf::Message* message = self->message; - if (!FIELD_BELONGS_TO_MESSAGE(field_descriptor, message)) { - PyErr_SetString(PyExc_KeyError, - "Field does not belong to message!"); + if (!CheckFieldBelongsToMessage(field_descriptor, message)) { return NULL; } if (field_descriptor->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED) { @@ -1048,7 +1081,7 @@ int ReleaseSubMessage(google::protobuf::Message* message, child_cmessage->message = released_message.get(); child_cmessage->owner.swap(released_message); child_cmessage->parent = NULL; - child_cmessage->parent_field = NULL; + child_cmessage->parent_field_descriptor = NULL; child_cmessage->read_only = false; return ForEachCompositeField(child_cmessage, SetOwnerVisitor(child_cmessage->owner)); @@ -1090,10 +1123,8 @@ int InternalReleaseFieldByDescriptor( int InternalReleaseField(CMessage* self, PyObject* composite_field, PyObject* name) { - PyObject* cdescriptor = GetDescriptor(self, name); - if (cdescriptor != NULL) { - const google::protobuf::FieldDescriptor* descriptor = - reinterpret_cast<CFieldDescriptor*>(cdescriptor)->descriptor; + const google::protobuf::FieldDescriptor* descriptor = GetFieldDescriptor(self, name); + if (descriptor != NULL) { return InternalReleaseFieldByDescriptor( descriptor, composite_field, self->message); } @@ -1104,9 +1135,7 @@ int InternalReleaseField(CMessage* self, PyObject* composite_field, PyObject* ClearFieldByDescriptor( CMessage* self, const google::protobuf::FieldDescriptor* descriptor) { - if (!FIELD_BELONGS_TO_MESSAGE(descriptor, self->message)) { - PyErr_SetString(PyExc_KeyError, - "Field does not belong to message!"); + if (!CheckFieldBelongsToMessage(descriptor, self->message)) { return NULL; } AssureWritable(self); @@ -1177,15 +1206,10 @@ PyObject* Clear(CMessage* self) { // fields have been released. if (self->extensions != NULL) { Py_CLEAR(self->extensions); - PyObject* py_extension_dict = PyObject_CallObject( - reinterpret_cast<PyObject*>(&ExtensionDict_Type), NULL); - if (py_extension_dict == NULL) { + ExtensionDict* extension_dict = extension_dict::NewExtensionDict(self); + if (extension_dict == NULL) { return NULL; } - ExtensionDict* extension_dict = reinterpret_cast<ExtensionDict*>( - py_extension_dict); - extension_dict->parent = self; - extension_dict->message = self->message; self->extensions = extension_dict; } PyDict_Clear(self->composite_fields); @@ -1196,8 +1220,8 @@ PyObject* Clear(CMessage* self) { // --------------------------------------------------------------------- static string GetMessageName(CMessage* self) { - if (self->parent_field != NULL) { - return self->parent_field->descriptor->full_name(); + if (self->parent_field_descriptor != NULL) { + return self->parent_field_descriptor->full_name(); } else { return self->message->GetDescriptor()->full_name(); } @@ -1219,7 +1243,7 @@ static PyObject* SerializeToString(CMessage* self, PyObject* args) { return NULL; } PyErr_Format(EncodeError_class, "Message %s is missing required fields: %s", - GetMessageName(self).c_str(), PyString_AsString(joined.get())); + GetMessageName(self).c_str(), PyString_AsString(joined)); return NULL; } int size = self->message->ByteSize(); @@ -1361,7 +1385,7 @@ static PyObject* MergeFromString(CMessage* self, PyObject* arg) { AssureWritable(self); google::protobuf::io::CodedInputStream input( reinterpret_cast<const uint8*>(data), data_length); - input.SetExtensionRegistry(GetDescriptorPool(), global_message_factory); + input.SetExtensionRegistry(descriptor_pool->pool, global_message_factory); bool success = self->message->MergePartialFromCodedStream(&input); if (success) { return PyInt_FromLong(input.CurrentPosition()); @@ -1421,15 +1445,11 @@ static PyObject* RegisterExtension(PyObject* cls, return NULL; } - CFieldDescriptor* cdescriptor = - extension_dict::InternalGetCDescriptorFromExtension(extension_handle); - ScopedPyObjectPtr py_cdescriptor(reinterpret_cast<PyObject*>(cdescriptor)); - if (cdescriptor == NULL) { + const google::protobuf::FieldDescriptor* descriptor = + GetExtensionDescriptor(extension_handle); + if (descriptor == NULL) { return NULL; } - Py_INCREF(extension_handle); - cdescriptor->descriptor_field = extension_handle; - const google::protobuf::FieldDescriptor* descriptor = cdescriptor->descriptor; // Check if it's a message set if (descriptor->is_extension() && descriptor->containing_type()->options().message_set_wire_format() && @@ -1608,8 +1628,14 @@ static PyObject* RichCompare(CMessage* self, PyObject* other, int opid) { } if (opid == Py_EQ || opid == Py_NE) { ScopedPyObjectPtr self_fields(ListFields(self)); + if (!self_fields) { + return NULL; + } ScopedPyObjectPtr other_fields(ListFields( reinterpret_cast<CMessage*>(other))); + if (!other_fields) { + return NULL; + } return PyObject_RichCompare(self_fields, other_fields, opid); } else { Py_INCREF(Py_NotImplemented); @@ -1623,9 +1649,7 @@ PyObject* InternalGetScalar( google::protobuf::Message* message = self->message; const google::protobuf::Reflection* reflection = message->GetReflection(); - if (!FIELD_BELONGS_TO_MESSAGE(field_descriptor, message)) { - PyErr_SetString( - PyExc_KeyError, "Field does not belong to message!"); + if (!CheckFieldBelongsToMessage(field_descriptor, message)) { return NULL; } @@ -1701,43 +1725,31 @@ PyObject* InternalGetScalar( return result; } -PyObject* InternalGetSubMessage(CMessage* self, - CFieldDescriptor* cfield_descriptor) { - PyObject* field = cfield_descriptor->descriptor_field; - ScopedPyObjectPtr message_type(PyObject_GetAttr(field, kmessage_type)); - if (message_type == NULL) { - return NULL; - } - ScopedPyObjectPtr concrete_class( - PyObject_GetAttr(message_type, k_concrete_class)); - if (concrete_class == NULL) { +PyObject* InternalGetSubMessage( + CMessage* self, const google::protobuf::FieldDescriptor* field_descriptor) { + const google::protobuf::Reflection* reflection = self->message->GetReflection(); + const google::protobuf::Message& sub_message = reflection->GetMessage( + *self->message, field_descriptor, global_message_factory); + + PyObject *message_class = cdescriptor_pool::GetMessageClass( + descriptor_pool, field_descriptor->message_type()); + if (message_class == NULL) { return NULL; } - PyObject* py_cmsg = cmessage::NewEmpty(concrete_class); - if (py_cmsg == NULL) { + + CMessage* cmsg = cmessage::NewEmptyMessage(message_class, + sub_message.GetDescriptor()); + if (cmsg == NULL) { return NULL; } - if (!PyObject_TypeCheck(py_cmsg, &CMessage_Type)) { - PyErr_SetString(PyExc_TypeError, "Not a CMessage!"); - } - CMessage* cmsg = reinterpret_cast<CMessage*>(py_cmsg); - const google::protobuf::FieldDescriptor* field_descriptor = - cfield_descriptor->descriptor; - const google::protobuf::Reflection* reflection = self->message->GetReflection(); - const google::protobuf::Message& sub_message = reflection->GetMessage( - *self->message, field_descriptor, global_message_factory); cmsg->owner = self->owner; cmsg->parent = self; - cmsg->parent_field = cfield_descriptor; + cmsg->parent_field_descriptor = field_descriptor; cmsg->read_only = !reflection->HasField(*self->message, field_descriptor); cmsg->message = const_cast<google::protobuf::Message*>(&sub_message); - if (InitAttributes(cmsg, NULL, NULL) < 0) { - Py_DECREF(py_cmsg); - return NULL; - } - return py_cmsg; + return reinterpret_cast<PyObject*>(cmsg); } int InternalSetScalar( @@ -1747,9 +1759,7 @@ int InternalSetScalar( google::protobuf::Message* message = self->message; const google::protobuf::Reflection* reflection = message->GetReflection(); - if (!FIELD_BELONGS_TO_MESSAGE(field_descriptor, message)) { - PyErr_SetString( - PyExc_KeyError, "Field does not belong to message!"); + if (!CheckFieldBelongsToMessage(field_descriptor, message)) { return -1; } @@ -1838,25 +1848,35 @@ PyObject* FromString(PyTypeObject* cls, PyObject* serialized) { return NULL; } - if (InitAttributes(cmsg, NULL, NULL) < 0) { - Py_DECREF(py_cmsg); - return NULL; - } return py_cmsg; } + +// Finalize the creation of the Message class. +// Called from its metaclass: GeneratedProtocolMessageType.__init__(). static PyObject* AddDescriptors(PyTypeObject* cls, PyObject* descriptor) { - if (PyObject_SetAttr(reinterpret_cast<PyObject*>(cls), - k_extensions_by_name, PyDict_New()) < 0) { - return NULL; - } - if (PyObject_SetAttr(reinterpret_cast<PyObject*>(cls), - k_extensions_by_number, PyDict_New()) < 0) { + const google::protobuf::Descriptor* message_descriptor = + cdescriptor_pool::RegisterMessageClass( + descriptor_pool, reinterpret_cast<PyObject*>(cls), descriptor); + if (message_descriptor == NULL) { return NULL; } - ScopedPyObjectPtr field_descriptors(PyDict_New()); + // If there are extension_ranges, the message is "extendable", and extension + // classes will register themselves in this class. + if (message_descriptor->extension_range_count() > 0) { + ScopedPyObjectPtr by_name(PyDict_New()); + if (PyObject_SetAttr(reinterpret_cast<PyObject*>(cls), + k_extensions_by_name, by_name) < 0) { + return NULL; + } + ScopedPyObjectPtr by_number(PyDict_New()); + if (PyObject_SetAttr(reinterpret_cast<PyObject*>(cls), + k_extensions_by_number, by_number) < 0) { + return NULL; + } + } ScopedPyObjectPtr fields(PyObject_GetAttrString(descriptor, "fields")); if (fields == NULL) { @@ -1878,19 +1898,14 @@ static PyObject* AddDescriptors(PyTypeObject* cls, return NULL; } - PyObject* field_descriptor = - cdescriptor_pool::FindFieldByName(descriptor_pool, full_field_name); + ScopedPyObjectPtr field_descriptor( + cdescriptor_pool::FindFieldByName(descriptor_pool, full_field_name)); if (field_descriptor == NULL) { PyErr_SetString(PyExc_TypeError, "Couldn't find field"); return NULL; } - Py_INCREF(field); CFieldDescriptor* cfield_descriptor = reinterpret_cast<CFieldDescriptor*>( - field_descriptor); - cfield_descriptor->descriptor_field = field; - if (PyDict_SetItem(field_descriptors, field_name, field_descriptor) < 0) { - return NULL; - } + field_descriptor.get()); // The FieldDescriptor's name field might either be of type bytes or // of type unicode, depending on whether the FieldDescriptor was @@ -1919,8 +1934,6 @@ static PyObject* AddDescriptors(PyTypeObject* cls, } } - PyDict_SetItem(cls->tp_dict, k__descriptors, field_descriptors); - // Enum Values ScopedPyObjectPtr enum_types(PyObject_GetAttrString(descriptor, "enum_types")); @@ -1994,15 +2007,11 @@ static PyObject* AddDescriptors(PyTypeObject* cls, extension_name, extension_field) == -1) { return NULL; } - ScopedPyObjectPtr py_cfield_descriptor( - PyObject_GetAttrString(extension_field, "_cdescriptor")); - if (py_cfield_descriptor == NULL) { + const google::protobuf::FieldDescriptor* field_descriptor = + GetExtensionDescriptor(extension_field); + if (field_descriptor == NULL) { return NULL; } - CFieldDescriptor* cfield_descriptor = - reinterpret_cast<CFieldDescriptor*>(py_cfield_descriptor.get()); - Py_INCREF(extension_field); - cfield_descriptor->descriptor_field = extension_field; ScopedPyObjectPtr field_name_upcased( PyObject_CallMethod(extension_name, "upper", NULL)); @@ -2015,13 +2024,12 @@ static PyObject* AddDescriptors(PyTypeObject* cls, return NULL; } ScopedPyObjectPtr number(PyInt_FromLong( - cfield_descriptor->descriptor->number())); + field_descriptor->number())); if (number == NULL) { return NULL; } if (PyObject_SetAttr(reinterpret_cast<PyObject*>(cls), - field_number_name, PyInt_FromLong( - cfield_descriptor->descriptor->number())) == -1) { + field_number_name, number) == -1) { return NULL; } } @@ -2039,10 +2047,6 @@ PyObject* DeepCopy(CMessage* self, PyObject* arg) { Py_DECREF(clone); return NULL; } - if (InitAttributes(reinterpret_cast<CMessage*>(clone), NULL, NULL) < 0) { - Py_DECREF(clone); - return NULL; - } if (MergeFrom(reinterpret_cast<CMessage*>(clone), reinterpret_cast<PyObject*>(self)) == NULL) { Py_DECREF(clone); @@ -2202,48 +2206,30 @@ PyObject* GetAttr(CMessage* self, PyObject* name) { return value; } - PyObject* descriptor = GetDescriptor(self, name); - if (descriptor != NULL) { - CFieldDescriptor* cdescriptor = - reinterpret_cast<CFieldDescriptor*>(descriptor); - const google::protobuf::FieldDescriptor* field_descriptor = cdescriptor->descriptor; + const google::protobuf::FieldDescriptor* field_descriptor = GetFieldDescriptor( + self, name); + if (field_descriptor != NULL) { if (field_descriptor->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED) { if (field_descriptor->cpp_type() == google::protobuf::FieldDescriptor::CPPTYPE_MESSAGE) { - PyObject* py_container = PyObject_CallObject( - reinterpret_cast<PyObject*>(&RepeatedCompositeContainer_Type), - NULL); - if (py_container == NULL) { + PyObject *message_class = cdescriptor_pool::GetMessageClass( + descriptor_pool, field_descriptor->message_type()); + if (message_class == NULL) { return NULL; } - RepeatedCompositeContainer* container = - reinterpret_cast<RepeatedCompositeContainer*>(py_container); - PyObject* field = cdescriptor->descriptor_field; - PyObject* message_type = PyObject_GetAttr(field, kmessage_type); - if (message_type == NULL) { - return NULL; - } - PyObject* concrete_class = - PyObject_GetAttr(message_type, k_concrete_class); - if (concrete_class == NULL) { + PyObject* py_container = repeated_composite_container::NewContainer( + self, field_descriptor, message_class); + if (py_container == NULL) { return NULL; } - container->parent = self; - container->parent_field = cdescriptor; - container->message = self->message; - container->owner = self->owner; - container->subclass_init = concrete_class; - Py_DECREF(message_type); if (PyDict_SetItem(self->composite_fields, name, py_container) < 0) { Py_DECREF(py_container); return NULL; } return py_container; } else { - ScopedPyObjectPtr init_args(PyTuple_Pack(2, self, cdescriptor)); - PyObject* py_container = PyObject_CallObject( - reinterpret_cast<PyObject*>(&RepeatedScalarContainer_Type), - init_args); + PyObject* py_container = repeated_scalar_container::NewContainer( + self, field_descriptor); if (py_container == NULL) { return NULL; } @@ -2256,7 +2242,7 @@ PyObject* GetAttr(CMessage* self, PyObject* name) { } else { if (field_descriptor->cpp_type() == google::protobuf::FieldDescriptor::CPPTYPE_MESSAGE) { - PyObject* sub_message = InternalGetSubMessage(self, cdescriptor); + PyObject* sub_message = InternalGetSubMessage(self, field_descriptor); if (PyDict_SetItem(self->composite_fields, name, sub_message) < 0) { Py_DECREF(sub_message); return NULL; @@ -2278,12 +2264,10 @@ int SetAttr(CMessage* self, PyObject* name, PyObject* value) { return -1; } - PyObject* descriptor = GetDescriptor(self, name); - if (descriptor != NULL) { + const google::protobuf::FieldDescriptor* field_descriptor = + GetFieldDescriptor(self, name); + if (field_descriptor != NULL) { AssureWritable(self); - CFieldDescriptor* cdescriptor = - reinterpret_cast<CFieldDescriptor*>(descriptor); - const google::protobuf::FieldDescriptor* field_descriptor = cdescriptor->descriptor; if (field_descriptor->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED) { PyErr_Format(PyExc_AttributeError, "Assignment not allowed to repeated " "field \"%s\" in protocol message object.", @@ -2401,22 +2385,18 @@ void InitGlobals() { kuint64max_py = PyLong_FromUnsignedLongLong(kuint64max); kDESCRIPTOR = PyString_FromString("DESCRIPTOR"); - k__descriptors = PyString_FromString("__descriptors"); + k_cdescriptor = PyString_FromString("_cdescriptor"); kfull_name = PyString_FromString("full_name"); - kis_extendable = PyString_FromString("is_extendable"); kextensions_by_name = PyString_FromString("extensions_by_name"); k_extensions_by_name = PyString_FromString("_extensions_by_name"); k_extensions_by_number = PyString_FromString("_extensions_by_number"); - k_concrete_class = PyString_FromString("_concrete_class"); - kmessage_type = PyString_FromString("message_type"); kname = PyString_FromString("name"); kfields_by_name = PyString_FromString("fields_by_name"); - global_message_factory = new DynamicMessageFactory(GetDescriptorPool()); - global_message_factory->SetDelegateToGeneratedFactory(true); + descriptor_pool = cdescriptor_pool::NewDescriptorPool(); - descriptor_pool = reinterpret_cast<google::protobuf::python::CDescriptorPool*>( - Python_NewCDescriptorPool(NULL, NULL)); + global_message_factory = new DynamicMessageFactory(descriptor_pool->pool); + global_message_factory->SetDelegateToGeneratedFactory(true); } bool InitProto2MessageModule(PyObject *m) { @@ -2427,19 +2407,32 @@ bool InitProto2MessageModule(PyObject *m) { return false; } - // All three of these are actually set elsewhere, directly onto the child - // protocol buffer message class, but set them here as well to document that - // subclasses need to set these. + // DESCRIPTOR is set on each protocol buffer message class elsewhere, but set + // it here as well to document that subclasses need to set it. PyDict_SetItem(google::protobuf::python::CMessage_Type.tp_dict, kDESCRIPTOR, Py_None); - PyDict_SetItem(google::protobuf::python::CMessage_Type.tp_dict, - k_extensions_by_name, Py_None); - PyDict_SetItem(google::protobuf::python::CMessage_Type.tp_dict, - k_extensions_by_number, Py_None); + // Subclasses with message extensions will override _extensions_by_name and + // _extensions_by_number with fresh mutable dictionaries in AddDescriptors. + // All other classes can share this same immutable mapping. + ScopedPyObjectPtr empty_dict(PyDict_New()); + if (empty_dict == NULL) { + return false; + } + ScopedPyObjectPtr immutable_dict(PyDictProxy_New(empty_dict)); + if (immutable_dict == NULL) { + return false; + } + if (PyDict_SetItem(google::protobuf::python::CMessage_Type.tp_dict, + k_extensions_by_name, immutable_dict) < 0) { + return false; + } + if (PyDict_SetItem(google::protobuf::python::CMessage_Type.tp_dict, + k_extensions_by_number, immutable_dict) < 0) { + return false; + } PyModule_AddObject(m, "Message", reinterpret_cast<PyObject*>( &google::protobuf::python::CMessage_Type)); - google::protobuf::python::RepeatedScalarContainer_Type.tp_new = PyType_GenericNew; google::protobuf::python::RepeatedScalarContainer_Type.tp_hash = PyObject_HashNotImplemented; if (PyType_Ready(&google::protobuf::python::RepeatedScalarContainer_Type) < 0) { @@ -2450,7 +2443,6 @@ bool InitProto2MessageModule(PyObject *m) { reinterpret_cast<PyObject*>( &google::protobuf::python::RepeatedScalarContainer_Type)); - google::protobuf::python::RepeatedCompositeContainer_Type.tp_new = PyType_GenericNew; google::protobuf::python::RepeatedCompositeContainer_Type.tp_hash = PyObject_HashNotImplemented; if (PyType_Ready(&google::protobuf::python::RepeatedCompositeContainer_Type) < 0) { @@ -2462,7 +2454,6 @@ bool InitProto2MessageModule(PyObject *m) { reinterpret_cast<PyObject*>( &google::protobuf::python::RepeatedCompositeContainer_Type)); - google::protobuf::python::ExtensionDict_Type.tp_new = PyType_GenericNew; google::protobuf::python::ExtensionDict_Type.tp_hash = PyObject_HashNotImplemented; if (PyType_Ready(&google::protobuf::python::ExtensionDict_Type) < 0) { return false; diff --git a/python/google/protobuf/pyext/message.h b/python/google/protobuf/pyext/message.h index 9f4978f4..73e5a7d3 100644 --- a/python/google/protobuf/pyext/message.h +++ b/python/google/protobuf/pyext/message.h @@ -49,12 +49,13 @@ namespace protobuf { class Message; class Reflection; class FieldDescriptor; +class Descriptor; using internal::shared_ptr; namespace python { -struct CFieldDescriptor; +struct PyDescriptorPool; struct ExtensionDict; typedef struct CMessage { @@ -79,13 +80,11 @@ typedef struct CMessage { // to use this pointer will result in a crash. struct CMessage* parent; - // Weak reference to the parent's descriptor that describes this submessage. + // Pointer to the parent's descriptor that describes this submessage. // Used together with the parent's message when making a default message // instance mutable. - // TODO(anuraag): With a bit of work on the Python/C++ layer, it should be - // possible to make this a direct pointer to a C++ FieldDescriptor, this would - // be easier if this implementation replaces upstream. - CFieldDescriptor* parent_field; + // The pointer is owned by the global DescriptorPool. + const google::protobuf::FieldDescriptor* parent_field_descriptor; // Pointer to the C++ Message object for this CMessage. The // CMessage does not own this pointer. @@ -113,8 +112,11 @@ extern PyTypeObject CMessage_Type; namespace cmessage { -// Create a new empty message that can be populated by the parent. -PyObject* NewEmpty(PyObject* type); +// Internal function to create a new empty Message Python object, but with empty +// pointers to the C++ objects. +// The caller must fill self->message, self->owner and eventually self->parent. +CMessage* NewEmptyMessage(PyObject* type, + const google::protobuf::Descriptor* descriptor); // Release a submessage from its proto tree, making it a new top-level messgae. // A new message will be created if this is a read-only default instance. @@ -124,12 +126,16 @@ int ReleaseSubMessage(google::protobuf::Message* message, const google::protobuf::FieldDescriptor* field_descriptor, CMessage* child_cmessage); +// Retrieves the C++ descriptor of a Python Extension descriptor. +// On error, return NULL with an exception set. +const google::protobuf::FieldDescriptor* GetExtensionDescriptor(PyObject* extension); + // Initializes a new CMessage instance for a submessage. Only called once per // submessage as the result is cached in composite_fields. // // Corresponds to reflection api method GetMessage. -PyObject* InternalGetSubMessage(CMessage* self, - CFieldDescriptor* cfield_descriptor); +PyObject* InternalGetSubMessage( + CMessage* self, const google::protobuf::FieldDescriptor* field_descriptor); // Deletes a range of C++ submessages in a repeated field (following a // removal in a RepeatedCompositeContainer). @@ -190,10 +196,8 @@ PyObject* HasFieldByDescriptor( // Corresponds to reflection api method HasField. PyObject* HasField(CMessage* self, PyObject* arg); -// Initializes constants/enum values on a message. This is called by -// RepeatedCompositeContainer and ExtensionDict after calling the constructor. -// TODO(anuraag): Make it always called from within the constructor since it can -int InitAttributes(CMessage* self, PyObject* descriptor, PyObject* kwargs); +// Initializes values of fields on a newly constructed message. +int InitAttributes(CMessage* self, PyObject* kwargs); PyObject* MergeFrom(CMessage* self, PyObject* arg); @@ -218,12 +222,14 @@ int AssureWritable(CMessage* self); } // namespace cmessage + +// Retrieve the global descriptor pool owned by the _message module. +PyDescriptorPool* GetDescriptorPool(); + + /* Is 64bit */ #define IS_64BIT (SIZEOF_LONG == 8) -#define FIELD_BELONGS_TO_MESSAGE(field_descriptor, message) \ - ((message)->GetDescriptor() == (field_descriptor)->containing_type()) - #define FIELD_IS_REPEATED(field_descriptor) \ ((field_descriptor)->label() == google::protobuf::FieldDescriptor::LABEL_REPEATED) @@ -296,6 +302,11 @@ bool CheckAndSetString( PyObject* ToStringObject( const google::protobuf::FieldDescriptor* descriptor, string value); +// Check if the passed field descriptor belongs to the given message. +// If not, return false and set a Python exception (a KeyError) +bool CheckFieldBelongsToMessage(const google::protobuf::FieldDescriptor* field_descriptor, + const google::protobuf::Message* message); + extern PyObject* PickleError_class; } // namespace python diff --git a/python/google/protobuf/pyext/proto2_api_test.proto b/python/google/protobuf/pyext/proto2_api_test.proto index 72c31b1c..18aecfb7 100644 --- a/python/google/protobuf/pyext/proto2_api_test.proto +++ b/python/google/protobuf/pyext/proto2_api_test.proto @@ -28,6 +28,8 @@ // (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +syntax = "proto2"; + import "google/protobuf/internal/cpp/proto1_api_test.proto"; package google.protobuf.python.internal; diff --git a/python/google/protobuf/pyext/python.proto b/python/google/protobuf/pyext/python.proto index d47d402c..cce645d7 100644 --- a/python/google/protobuf/pyext/python.proto +++ b/python/google/protobuf/pyext/python.proto @@ -33,6 +33,7 @@ // These message definitions are used to exercises known corner cases // in the C++ implementation of the Python API. +syntax = "proto2"; package google.protobuf.python.internal; @@ -63,4 +64,5 @@ message TestAllExtensions { extend TestAllExtensions { optional TestAllTypes.NestedMessage optional_nested_message_extension = 1; + repeated TestAllTypes.NestedMessage repeated_nested_message_extension = 2; } diff --git a/python/google/protobuf/pyext/repeated_composite_container.cc b/python/google/protobuf/pyext/repeated_composite_container.cc index 5c05b3d8..36fe86ae 100644 --- a/python/google/protobuf/pyext/repeated_composite_container.cc +++ b/python/google/protobuf/pyext/repeated_composite_container.cc @@ -65,14 +65,14 @@ namespace repeated_composite_container { #define GOOGLE_CHECK_ATTACHED(self) \ do { \ GOOGLE_CHECK_NOTNULL((self)->message); \ - GOOGLE_CHECK_NOTNULL((self)->parent_field); \ + GOOGLE_CHECK_NOTNULL((self)->parent_field_descriptor); \ } while (0); #define GOOGLE_CHECK_RELEASED(self) \ do { \ GOOGLE_CHECK((self)->owner.get() == NULL); \ GOOGLE_CHECK((self)->message == NULL); \ - GOOGLE_CHECK((self)->parent_field == NULL); \ + GOOGLE_CHECK((self)->parent_field_descriptor == NULL); \ GOOGLE_CHECK((self)->parent == NULL); \ } while (0); @@ -122,7 +122,7 @@ static int InternalQuickSort(RepeatedCompositeContainer* self, google::protobuf::Message* message = self->message; const google::protobuf::Reflection* reflection = message->GetReflection(); - const google::protobuf::FieldDescriptor* descriptor = self->parent_field->descriptor; + const google::protobuf::FieldDescriptor* descriptor = self->parent_field_descriptor; Py_ssize_t left; Py_ssize_t right; @@ -202,7 +202,7 @@ static Py_ssize_t Length(RepeatedCompositeContainer* self) { google::protobuf::Message* message = self->message; if (message != NULL) { return message->GetReflection()->FieldSize(*message, - self->parent_field->descriptor); + self->parent_field_descriptor); } else { // The container has been released (i.e. by a call to Clear() or // ClearField() on the parent) and thus there's no message. @@ -225,19 +225,19 @@ static int UpdateChildMessages(RepeatedCompositeContainer* self) { const google::protobuf::Reflection* reflection = message->GetReflection(); for (Py_ssize_t i = child_length; i < message_length; ++i) { const Message& sub_message = reflection->GetRepeatedMessage( - *(self->message), self->parent_field->descriptor, i); - ScopedPyObjectPtr py_cmsg(cmessage::NewEmpty(self->subclass_init)); - if (py_cmsg == NULL) { + *(self->message), self->parent_field_descriptor, i); + CMessage* cmsg = cmessage::NewEmptyMessage(self->subclass_init, + sub_message.GetDescriptor()); + ScopedPyObjectPtr py_cmsg(reinterpret_cast<PyObject*>(cmsg)); + if (cmsg == NULL) { return -1; } - CMessage* cmsg = reinterpret_cast<CMessage*>(py_cmsg.get()); cmsg->owner = self->owner; cmsg->message = const_cast<google::protobuf::Message*>(&sub_message); cmsg->parent = self->parent; - if (cmessage::InitAttributes(cmsg, NULL, NULL) < 0) { + if (PyList_Append(self->child_messages, py_cmsg) < 0) { return -1; } - PyList_Append(self->child_messages, py_cmsg); } return 0; } @@ -258,23 +258,25 @@ static PyObject* AddToAttached(RepeatedCompositeContainer* self, google::protobuf::Message* message = self->message; google::protobuf::Message* sub_message = message->GetReflection()->AddMessage(message, - self->parent_field->descriptor); - PyObject* py_cmsg = cmessage::NewEmpty(self->subclass_init); - if (py_cmsg == NULL) { + self->parent_field_descriptor); + CMessage* cmsg = cmessage::NewEmptyMessage(self->subclass_init, + sub_message->GetDescriptor()); + if (cmsg == NULL) return NULL; - } - CMessage* cmsg = reinterpret_cast<CMessage*>(py_cmsg); cmsg->owner = self->owner; cmsg->message = sub_message; cmsg->parent = self->parent; - // cmessage::InitAttributes must be called after cmsg->message has - // been set. - if (cmessage::InitAttributes(cmsg, NULL, kwargs) < 0) { + if (cmessage::InitAttributes(cmsg, kwargs) < 0) { + Py_DECREF(cmsg); + return NULL; + } + + PyObject* py_cmsg = reinterpret_cast<PyObject*>(cmsg); + if (PyList_Append(self->child_messages, py_cmsg) < 0) { Py_DECREF(py_cmsg); return NULL; } - PyList_Append(self->child_messages, py_cmsg); return py_cmsg; } @@ -283,20 +285,16 @@ static PyObject* AddToReleased(RepeatedCompositeContainer* self, PyObject* kwargs) { GOOGLE_CHECK_RELEASED(self); - // Create the CMessage - PyObject* py_cmsg = PyObject_CallObject(self->subclass_init, NULL); + // Create a new Message detached from the rest. + PyObject* py_cmsg = PyEval_CallObjectWithKeywords( + self->subclass_init, NULL, kwargs); if (py_cmsg == NULL) return NULL; - CMessage* cmsg = reinterpret_cast<CMessage*>(py_cmsg); - if (cmessage::InitAttributes(cmsg, NULL, kwargs) < 0) { + + if (PyList_Append(self->child_messages, py_cmsg) < 0) { Py_DECREF(py_cmsg); return NULL; } - - // The Message got created by the call to subclass_init above and - // it set self->owner to the newly allocated message. - - PyList_Append(self->child_messages, py_cmsg); return py_cmsg; } @@ -354,35 +352,9 @@ PyObject* Subscript(RepeatedCompositeContainer* self, PyObject* slice) { if (UpdateChildMessages(self) < 0) { return NULL; } - Py_ssize_t from; - Py_ssize_t to; - Py_ssize_t step; - Py_ssize_t length = Length(self); - Py_ssize_t slicelength; - if (PySlice_Check(slice)) { -#if PY_MAJOR_VERSION >= 3 - if (PySlice_GetIndicesEx(slice, -#else - if (PySlice_GetIndicesEx(reinterpret_cast<PySliceObject*>(slice), -#endif - length, &from, &to, &step, &slicelength) == -1) { - return NULL; - } - return PyList_GetSlice(self->child_messages, from, to); - } else if (PyInt_Check(slice) || PyLong_Check(slice)) { - from = to = PyLong_AsLong(slice); - if (from < 0) { - from = to = length + from; - } - PyObject* result = PyList_GetItem(self->child_messages, from); - if (result == NULL) { - return NULL; - } - Py_INCREF(result); - return result; - } - PyErr_SetString(PyExc_TypeError, "index must be an integer or slice"); - return NULL; + // Just forward the call to the subscript-handling function of the + // list containing the child messages. + return PyObject_GetItem(self->child_messages, slice); } int AssignSubscript(RepeatedCompositeContainer* self, @@ -399,7 +371,7 @@ int AssignSubscript(RepeatedCompositeContainer* self, // Delete from the underlying Message, if any. if (self->message != NULL) { if (cmessage::InternalDeleteRepeatedField(self->message, - self->parent_field->descriptor, + self->parent_field_descriptor, slice, self->child_messages) < 0) { return -1; @@ -512,7 +484,7 @@ static PyObject* SortAttached(RepeatedCompositeContainer* self, if (reverse) { google::protobuf::Message* message = self->message; const google::protobuf::Reflection* reflection = message->GetReflection(); - const google::protobuf::FieldDescriptor* descriptor = self->parent_field->descriptor; + const google::protobuf::FieldDescriptor* descriptor = self->parent_field_descriptor; // Reverse the Message array. for (int i = 0; i < length / 2; ++i) @@ -554,8 +526,9 @@ static PyObject* Sort(RepeatedCompositeContainer* self, } } - if (UpdateChildMessages(self) < 0) + if (UpdateChildMessages(self) < 0) { return NULL; + } if (self->message == NULL) { return SortReleased(self, args, kwds); } else { @@ -617,7 +590,7 @@ void ReleaseLastTo(const FieldDescriptor* field, shared_ptr<Message> released_message( ReleaseLast(field, cmessage->message->GetDescriptor(), message)); cmessage->parent = NULL; - cmessage->parent_field = NULL; + cmessage->parent_field_descriptor = NULL; cmessage->message = released_message.get(); cmessage->read_only = false; cmessage::SetOwner(cmessage, released_message); @@ -633,7 +606,7 @@ int Release(RepeatedCompositeContainer* self) { } Message* message = self->message; - const FieldDescriptor* field = self->parent_field->descriptor; + const FieldDescriptor* field = self->parent_field_descriptor; // The reflection API only lets us release the last message in a // repeated field. Therefore we iterate through the children @@ -648,7 +621,7 @@ int Release(RepeatedCompositeContainer* self) { // Detach from containing message. self->parent = NULL; - self->parent_field = NULL; + self->parent_field_descriptor = NULL; self->message = NULL; self->owner.reset(); @@ -670,22 +643,40 @@ int SetOwner(RepeatedCompositeContainer* self, return 0; } -static int Init(RepeatedCompositeContainer* self, - PyObject* args, - PyObject* kwargs) { - self->message = NULL; - self->parent = NULL; - self->parent_field = NULL; - self->subclass_init = NULL; +// The private constructor of RepeatedCompositeContainer objects. +PyObject *NewContainer( + CMessage* parent, + const google::protobuf::FieldDescriptor* parent_field_descriptor, + PyObject *concrete_class) { + if (!CheckFieldBelongsToMessage(parent_field_descriptor, parent->message)) { + return NULL; + } + + RepeatedCompositeContainer* self = + reinterpret_cast<RepeatedCompositeContainer*>( + PyType_GenericAlloc(&RepeatedCompositeContainer_Type, 0)); + if (self == NULL) { + return NULL; + } + + self->message = parent->message; + self->parent = parent; + self->parent_field_descriptor = parent_field_descriptor; + self->owner = parent->owner; + Py_INCREF(concrete_class); + self->subclass_init = concrete_class; self->child_messages = PyList_New(0); - return 0; + + return reinterpret_cast<PyObject*>(self); } static void Dealloc(RepeatedCompositeContainer* self) { Py_CLEAR(self->child_messages); + Py_CLEAR(self->subclass_init); // TODO(tibell): Do we need to call delete on these objects to make // sure their destructors are called? self->owner.reset(); + Py_TYPE(self)->tp_free(reinterpret_cast<PyObject*>(self)); } @@ -755,7 +746,7 @@ PyTypeObject RepeatedCompositeContainer_Type = { 0, // tp_descr_get 0, // tp_descr_set 0, // tp_dictoffset - (initproc)repeated_composite_container::Init, // tp_init + 0, // tp_init }; } // namespace python diff --git a/python/google/protobuf/pyext/repeated_composite_container.h b/python/google/protobuf/pyext/repeated_composite_container.h index 898ef5a7..a76a5d6a 100644 --- a/python/google/protobuf/pyext/repeated_composite_container.h +++ b/python/google/protobuf/pyext/repeated_composite_container.h @@ -55,7 +55,6 @@ using internal::shared_ptr; namespace python { struct CMessage; -struct CFieldDescriptor; // A RepeatedCompositeContainer can be in one of two states: attached // or released. @@ -66,7 +65,7 @@ struct CFieldDescriptor; // 'child_messages' are owner by the 'owner'. // // When in the released state 'message', 'owner', 'parent', and -// 'parent_field' are NULL. +// 'parent_field_descriptor' are NULL. typedef struct RepeatedCompositeContainer { PyObject_HEAD; @@ -82,7 +81,8 @@ typedef struct RepeatedCompositeContainer { CMessage* parent; // A descriptor used to modify the underlying 'message'. - CFieldDescriptor* parent_field; + // The pointer is owned by the global DescriptorPool. + const google::protobuf::FieldDescriptor* parent_field_descriptor; // Pointer to the C++ Message that contains this container. The // RepeatedCompositeContainer does not own this pointer. @@ -102,6 +102,13 @@ extern PyTypeObject RepeatedCompositeContainer_Type; namespace repeated_composite_container { +// Builds a RepeatedCompositeContainer object, from a parent message and a +// field descriptor. +PyObject *NewContainer( + CMessage* parent, + const google::protobuf::FieldDescriptor* parent_field_descriptor, + PyObject *concrete_class); + // Returns the number of items in this repeated composite container. static Py_ssize_t Length(RepeatedCompositeContainer* self); diff --git a/python/google/protobuf/pyext/repeated_scalar_container.cc b/python/google/protobuf/pyext/repeated_scalar_container.cc index e627d37d..49d23fd6 100644 --- a/python/google/protobuf/pyext/repeated_scalar_container.cc +++ b/python/google/protobuf/pyext/repeated_scalar_container.cc @@ -52,7 +52,7 @@ #error "Python 3.0 - 3.2 are not supported." #else #define PyString_AsString(ob) \ - (PyUnicode_Check(ob)? PyUnicode_AsUTF8(ob): PyBytes_AS_STRING(ob)) + (PyUnicode_Check(ob)? PyUnicode_AsUTF8(ob): PyBytes_AsString(ob)) #endif #endif @@ -67,7 +67,7 @@ namespace repeated_scalar_container { static int InternalAssignRepeatedField( RepeatedScalarContainer* self, PyObject* list) { self->message->GetReflection()->ClearField(self->message, - self->parent_field->descriptor); + self->parent_field_descriptor); for (Py_ssize_t i = 0; i < PyList_GET_SIZE(list); ++i) { PyObject* value = PyList_GET_ITEM(list, i); if (Append(self, value) == NULL) { @@ -80,7 +80,7 @@ static int InternalAssignRepeatedField( static Py_ssize_t Len(RepeatedScalarContainer* self) { google::protobuf::Message* message = self->message; return message->GetReflection()->FieldSize(*message, - self->parent_field->descriptor); + self->parent_field_descriptor); } static int AssignItem(RepeatedScalarContainer* self, @@ -89,12 +89,7 @@ static int AssignItem(RepeatedScalarContainer* self, cmessage::AssureWritable(self->parent); google::protobuf::Message* message = self->message; const google::protobuf::FieldDescriptor* field_descriptor = - self->parent_field->descriptor; - if (!FIELD_BELONGS_TO_MESSAGE(field_descriptor, message)) { - PyErr_SetString( - PyExc_KeyError, "Field does not belong to message!"); - return -1; - } + self->parent_field_descriptor; const google::protobuf::Reflection* reflection = message->GetReflection(); int field_size = reflection->FieldSize(*message, field_descriptor); @@ -175,7 +170,7 @@ static int AssignItem(RepeatedScalarContainer* self, ScopedPyObjectPtr s(PyObject_Str(arg)); if (s != NULL) { PyErr_Format(PyExc_ValueError, "Unknown enum value: %s", - PyString_AsString(s.get())); + PyString_AsString(s)); } return -1; } @@ -193,7 +188,7 @@ static int AssignItem(RepeatedScalarContainer* self, static PyObject* Item(RepeatedScalarContainer* self, Py_ssize_t index) { google::protobuf::Message* message = self->message; const google::protobuf::FieldDescriptor* field_descriptor = - self->parent_field->descriptor; + self->parent_field_descriptor; const google::protobuf::Reflection* reflection = message->GetReflection(); int field_size = reflection->FieldSize(*message, field_descriptor); @@ -358,13 +353,7 @@ PyObject* Append(RepeatedScalarContainer* self, PyObject* item) { cmessage::AssureWritable(self->parent); google::protobuf::Message* message = self->message; const google::protobuf::FieldDescriptor* field_descriptor = - self->parent_field->descriptor; - - if (!FIELD_BELONGS_TO_MESSAGE(field_descriptor, message)) { - PyErr_SetString( - PyExc_KeyError, "Field does not belong to message!"); - return NULL; - } + self->parent_field_descriptor; const google::protobuf::Reflection* reflection = message->GetReflection(); switch (field_descriptor->cpp_type()) { @@ -422,7 +411,7 @@ PyObject* Append(RepeatedScalarContainer* self, PyObject* item) { ScopedPyObjectPtr s(PyObject_Str(item)); if (s != NULL) { PyErr_Format(PyExc_ValueError, "Unknown enum value: %s", - PyString_AsString(s.get())); + PyString_AsString(s)); } return NULL; } @@ -451,7 +440,7 @@ static int AssSubscript(RepeatedScalarContainer* self, cmessage::AssureWritable(self->parent); google::protobuf::Message* message = self->message; const google::protobuf::FieldDescriptor* field_descriptor = - self->parent_field->descriptor; + self->parent_field_descriptor; #if PY_MAJOR_VERSION < 3 if (PyInt_Check(slice)) { @@ -638,47 +627,25 @@ static PyObject* Sort(RepeatedScalarContainer* self, Py_RETURN_NONE; } -static int Init(RepeatedScalarContainer* self, - PyObject* args, - PyObject* kwargs) { - PyObject* py_parent; - PyObject* py_parent_field; - if (!PyArg_UnpackTuple(args, "__init__()", 2, 2, &py_parent, - &py_parent_field)) { - return -1; - } - - if (!PyObject_TypeCheck(py_parent, &CMessage_Type)) { - PyErr_Format(PyExc_TypeError, - "expect %s, but got %s", - CMessage_Type.tp_name, - Py_TYPE(py_parent)->tp_name); - return -1; +// The private constructor of RepeatedScalarContainer objects. +PyObject *NewContainer( + CMessage* parent, const google::protobuf::FieldDescriptor* parent_field_descriptor) { + if (!CheckFieldBelongsToMessage(parent_field_descriptor, parent->message)) { + return NULL; } - if (!PyObject_TypeCheck(py_parent_field, &CFieldDescriptor_Type)) { - PyErr_Format(PyExc_TypeError, - "expect %s, but got %s", - CFieldDescriptor_Type.tp_name, - Py_TYPE(py_parent_field)->tp_name); - return -1; + RepeatedScalarContainer* self = reinterpret_cast<RepeatedScalarContainer*>( + PyType_GenericAlloc(&RepeatedScalarContainer_Type, 0)); + if (self == NULL) { + return NULL; } - CMessage* cmessage = reinterpret_cast<CMessage*>(py_parent); - CFieldDescriptor* cdescriptor = reinterpret_cast<CFieldDescriptor*>( - py_parent_field); - - if (!FIELD_BELONGS_TO_MESSAGE(cdescriptor->descriptor, cmessage->message)) { - PyErr_SetString( - PyExc_KeyError, "Field does not belong to message!"); - return -1; - } + self->message = parent->message; + self->parent = parent; + self->parent_field_descriptor = parent_field_descriptor; + self->owner = parent->owner; - self->message = cmessage->message; - self->parent = cmessage; - self->parent_field = cdescriptor; - self->owner = cmessage->owner; - return 0; + return reinterpret_cast<PyObject*>(self); } // Initializes the underlying Message object of "to" so it becomes a new parent @@ -699,10 +666,7 @@ static int InitializeAndCopyToParentContainer( google::protobuf::Message* new_message = global_message_factory->GetPrototype( from->message->GetDescriptor())->New(); to->parent = NULL; - // TODO(anuraag): Document why it's OK to hang on to parent_field, - // even though it's a weak reference. It ought to be enough to - // hold on to the FieldDescriptor only. - to->parent_field = from->parent_field; + to->parent_field_descriptor = from->parent_field_descriptor; to->message = new_message; to->owner.reset(new_message); if (InternalAssignRepeatedField(to, values) < 0) { @@ -716,23 +680,17 @@ int Release(RepeatedScalarContainer* self) { } PyObject* DeepCopy(RepeatedScalarContainer* self, PyObject* arg) { - ScopedPyObjectPtr init_args( - PyTuple_Pack(2, self->parent, self->parent_field)); - PyObject* clone = PyObject_CallObject( - reinterpret_cast<PyObject*>(&RepeatedScalarContainer_Type), init_args); + RepeatedScalarContainer* clone = reinterpret_cast<RepeatedScalarContainer*>( + PyType_GenericAlloc(&RepeatedScalarContainer_Type, 0)); if (clone == NULL) { return NULL; } - if (!PyObject_TypeCheck(clone, &RepeatedScalarContainer_Type)) { - Py_DECREF(clone); - return NULL; - } - if (InitializeAndCopyToParentContainer( - self, reinterpret_cast<RepeatedScalarContainer*>(clone)) < 0) { + + if (InitializeAndCopyToParentContainer(self, clone) < 0) { Py_DECREF(clone); return NULL; } - return clone; + return reinterpret_cast<PyObject*>(clone); } static void Dealloc(RepeatedScalarContainer* self) { @@ -817,7 +775,7 @@ PyTypeObject RepeatedScalarContainer_Type = { 0, // tp_descr_get 0, // tp_descr_set 0, // tp_dictoffset - (initproc)repeated_scalar_container::Init, // tp_init + 0, // tp_init }; } // namespace python diff --git a/python/google/protobuf/pyext/repeated_scalar_container.h b/python/google/protobuf/pyext/repeated_scalar_container.h index 69d15d5c..513bfe48 100644 --- a/python/google/protobuf/pyext/repeated_scalar_container.h +++ b/python/google/protobuf/pyext/repeated_scalar_container.h @@ -41,6 +41,7 @@ #include <google/protobuf/stubs/shared_ptr.h> #endif +#include <google/protobuf/descriptor.h> namespace google { namespace protobuf { @@ -51,7 +52,6 @@ using internal::shared_ptr; namespace python { -struct CFieldDescriptor; struct CMessage; typedef struct RepeatedScalarContainer { @@ -73,16 +73,22 @@ typedef struct RepeatedScalarContainer { // modifying the container. CMessage* parent; - // Weak reference to the parent's descriptor that describes this + // Pointer to the parent's descriptor that describes this // field. Used together with the parent's message when making a // default message instance mutable. - CFieldDescriptor* parent_field; + // The pointer is owned by the global DescriptorPool. + const google::protobuf::FieldDescriptor* parent_field_descriptor; } RepeatedScalarContainer; extern PyTypeObject RepeatedScalarContainer_Type; namespace repeated_scalar_container { +// Builds a RepeatedScalarContainer object, from a parent message and a +// field descriptor. +extern PyObject *NewContainer( + CMessage* parent, const google::protobuf::FieldDescriptor* parent_field_descriptor); + // Appends the scalar 'item' to the end of the container 'self'. // // Returns None if successful; returns NULL and sets an exception if diff --git a/python/google/protobuf/pyext/scoped_pyobject_ptr.h b/python/google/protobuf/pyext/scoped_pyobject_ptr.h index 9f337c3c..fefebb6a 100644 --- a/python/google/protobuf/pyext/scoped_pyobject_ptr.h +++ b/python/google/protobuf/pyext/scoped_pyobject_ptr.h @@ -33,6 +33,8 @@ #ifndef GOOGLE_PROTOBUF_PYTHON_CPP_SCOPED_PYOBJECT_PTR_H__ #define GOOGLE_PROTOBUF_PYTHON_CPP_SCOPED_PYOBJECT_PTR_H__ +#include <google/protobuf/stubs/common.h> + #include <Python.h> namespace google { diff --git a/python/google/protobuf/text_format.py b/python/google/protobuf/text_format.py index 2429fa59..b2f3d32c 100755 --- a/python/google/protobuf/text_format.py +++ b/python/google/protobuf/text_format.py @@ -112,15 +112,17 @@ def PrintMessage(message, out, indent=0, as_utf8=False, as_one_line=False, for element in value: PrintField(field, element, out, indent, as_utf8, as_one_line, pointy_brackets=pointy_brackets, + use_index_order=use_index_order, float_format=float_format) else: PrintField(field, value, out, indent, as_utf8, as_one_line, pointy_brackets=pointy_brackets, + use_index_order=use_index_order, float_format=float_format) def PrintField(field, value, out, indent=0, as_utf8=False, as_one_line=False, - pointy_brackets=False, float_format=None): + pointy_brackets=False, use_index_order=False, float_format=None): """Print a single field name/value pair. For repeated fields, the value should be a single element.""" @@ -148,6 +150,7 @@ def PrintField(field, value, out, indent=0, as_utf8=False, as_one_line=False, PrintFieldValue(field, value, out, indent, as_utf8, as_one_line, pointy_brackets=pointy_brackets, + use_index_order=use_index_order, float_format=float_format) if as_one_line: out.write(' ') @@ -157,6 +160,7 @@ def PrintField(field, value, out, indent=0, as_utf8=False, as_one_line=False, def PrintFieldValue(field, value, out, indent=0, as_utf8=False, as_one_line=False, pointy_brackets=False, + use_index_order=False, float_format=None): """Print a single field value (not including name). For repeated fields, the value should be a single element.""" @@ -173,12 +177,14 @@ def PrintFieldValue(field, value, out, indent=0, as_utf8=False, out.write(' %s ' % openb) PrintMessage(value, out, indent, as_utf8, as_one_line, pointy_brackets=pointy_brackets, + use_index_order=use_index_order, float_format=float_format) out.write(closeb) else: out.write(' %s\n' % openb) PrintMessage(value, out, indent + 2, as_utf8, as_one_line, pointy_brackets=pointy_brackets, + use_index_order=use_index_order, float_format=float_format) out.write(' ' * indent + closeb) elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_ENUM: |