From a3a65b320d6bde3bae6c367ae83c265006cbf46b Mon Sep 17 00:00:00 2001 From: Adam Date: Mon, 24 Jul 2017 22:41:09 +0200 Subject: Fix issue #1745 - javascript allow dot in filename --- src/google/protobuf/compiler/js/js_generator.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index 7c63e58f..97b88890 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -193,6 +193,7 @@ string ModuleAlias(const string& filename) { string basename = StripProto(filename); StripString(&basename, "-", '$'); StripString(&basename, "/", '_'); + StripString(&basename, “.”, '_'); return basename + "_pb"; } -- cgit v1.2.3 From 2b6db00de1276a1d71722e69b1f8b9ecb65abc96 Mon Sep 17 00:00:00 2001 From: Adam Date: Mon, 24 Jul 2017 23:00:49 +0200 Subject: Fix quotation marks --- src/google/protobuf/compiler/js/js_generator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc index 97b88890..2d86d1c8 100755 --- a/src/google/protobuf/compiler/js/js_generator.cc +++ b/src/google/protobuf/compiler/js/js_generator.cc @@ -193,7 +193,7 @@ string ModuleAlias(const string& filename) { string basename = StripProto(filename); StripString(&basename, "-", '$'); StripString(&basename, "/", '_'); - StripString(&basename, “.”, '_'); + StripString(&basename, ".", '_'); return basename + "_pb"; } -- cgit v1.2.3 From 21800ff84ff0b68833e321e02c3c50f83047fd92 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Jul 2017 15:41:31 -0400 Subject: Add a objc_class_prefix to test_messages_proto3.proto. Both test_messages_proto3.proto & test_messages_proto2.proto define message ForeignMessage {...} and enum ForeignEnum {...} but since objc doesn't use the proto package in the naming, these end up conflicting. Adding the objc_class_prefix option to the proto3 file ensure the generated objc types are all unique. --- conformance/conformance_objc.m | 6 +++--- src/google/protobuf/test_messages_proto3.proto | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/conformance/conformance_objc.m b/conformance/conformance_objc.m index ba1c946f..a5012ec4 100644 --- a/conformance/conformance_objc.m +++ b/conformance/conformance_objc.m @@ -63,7 +63,7 @@ static NSData *CheckedReadDataOfLength(NSFileHandle *handle, NSUInteger numBytes static ConformanceResponse *DoTest(ConformanceRequest *request) { ConformanceResponse *response = [ConformanceResponse message]; - TestAllTypesProto3 *testMessage = nil; + Proto3TestAllTypesProto3 *testMessage = nil; switch (request.payloadOneOfCase) { case ConformanceRequest_Payload_OneOfCase_GPBUnsetOneOfCase: @@ -73,8 +73,8 @@ static ConformanceResponse *DoTest(ConformanceRequest *request) { case ConformanceRequest_Payload_OneOfCase_ProtobufPayload: { if ([request.messageType isEqualToString:@"protobuf_test_messages.proto3.TestAllTypesProto3"]) { NSError *error = nil; - testMessage = [TestAllTypesProto3 parseFromData:request.protobufPayload - error:&error]; + testMessage = [Proto3TestAllTypesProto3 parseFromData:request.protobufPayload + error:&error]; if (!testMessage) { response.parseError = [NSString stringWithFormat:@"Parse error: %@", error]; diff --git a/src/google/protobuf/test_messages_proto3.proto b/src/google/protobuf/test_messages_proto3.proto index abf73427..84b9da99 100644 --- a/src/google/protobuf/test_messages_proto3.proto +++ b/src/google/protobuf/test_messages_proto3.proto @@ -39,6 +39,7 @@ syntax = "proto3"; package protobuf_test_messages.proto3; option java_package = "com.google.protobuf_test_messages.proto3"; +option objc_class_prefix = "Proto3"; // This is the default, but we specify it here explicitly. option optimize_for = SPEED; -- cgit v1.2.3 From 9fd5e59c97db498afd34e7a12dabed2915fb2205 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Jul 2017 15:45:23 -0400 Subject: Generate the proto2 test file and link it in for ObjC. --- conformance/Makefile.am | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/conformance/Makefile.am b/conformance/Makefile.am index 9fad5409..f7ce053b 100644 --- a/conformance/Makefile.am +++ b/conformance/Makefile.am @@ -2,12 +2,12 @@ conformance_protoc_inputs = \ conformance.proto \ - $(top_srcdir)/src/google/protobuf/test_messages_proto3.proto - -# proto2 input files, should be separated with proto3, as we -# can't generate proto2 files for ruby, php and objc + $(top_srcdir)/src/google/protobuf/test_messages_proto3.proto + +# proto2 input files, should be separated with proto3, as we +# can't generate proto2 files for ruby, php and objc conformance_proto2_protoc_inputs = \ - $(top_srcdir)/src/google/protobuf/test_messages_proto2.proto + $(top_srcdir)/src/google/protobuf/test_messages_proto2.proto well_known_type_protoc_inputs = \ $(top_srcdir)/src/google/protobuf/any.proto \ @@ -86,6 +86,8 @@ other_language_protoc_outputs = \ google/protobuf/struct.pb.h \ google/protobuf/struct.rb \ google/protobuf/struct_pb2.py \ + google/protobuf/TestMessagesProto2.pbobjc.h \ + google/protobuf/TestMessagesProto2.pbobjc.m \ google/protobuf/TestMessagesProto3.pbobjc.h \ google/protobuf/TestMessagesProto3.pbobjc.m \ google/protobuf/test_messages_proto3.pb.cc \ @@ -228,7 +230,7 @@ if OBJC_CONFORMANCE_TEST bin_PROGRAMS += conformance-objc conformance_objc_SOURCES = conformance_objc.m ../objectivec/GPBProtocolBuffers.m -nodist_conformance_objc_SOURCES = Conformance.pbobjc.m google/protobuf/TestMessagesProto3.pbobjc.m +nodist_conformance_objc_SOURCES = Conformance.pbobjc.m google/protobuf/TestMessagesProto2.pbobjc.m google/protobuf/TestMessagesProto3.pbobjc.m # On travis, the build fails without the isysroot because whatever system # headers are being found don't include generics support for # NSArray/NSDictionary, the only guess is their image at one time had an odd @@ -237,7 +239,7 @@ conformance_objc_CPPFLAGS = -I$(top_srcdir)/objectivec -isysroot `xcrun --sdk ma conformance_objc_LDFLAGS = -framework Foundation # Explicit dep beacuse BUILT_SOURCES are only done before a "make all/check" # so a direct "make test_objc" could fail if parallel enough. -conformance_objc-conformance_objc.$(OBJEXT): Conformance.pbobjc.h google/protobuf/TestMessagesProto3.pbobjc.h +conformance_objc-conformance_objc.$(OBJEXT): Conformance.pbobjc.h google/protobuf/TestMessagesProto2.pbobjc.h google/protobuf/TestMessagesProto3.pbobjc.h endif @@ -253,7 +255,7 @@ if USE_EXTERNAL_PROTOC # Some implementations include pre-generated versions of well-known types. protoc_middleman: $(conformance_protoc_inputs) $(conformance_proto2_protoc_inputs) $(well_known_type_protoc_inputs) google-protobuf $(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --ruby_out=. --objc_out=. --python_out=. --php_out=. --js_out=import_style=commonjs,binary:. $(conformance_protoc_inputs) - $(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --python_out=. --js_out=import_style=commonjs,binary:. $(conformance_proto2_protoc_inputs) + $(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --objc_out=. --python_out=. --js_out=import_style=commonjs,binary:. $(conformance_proto2_protoc_inputs) $(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --ruby_out=. --python_out=. --php_out=. --js_out=import_style=commonjs,binary:google-protobuf $(well_known_type_protoc_inputs) ## $(PROTOC) -I$(srcdir) -I$(top_srcdir) --java_out=lite:lite $(conformance_protoc_inputs) $(well_known_type_protoc_inputs) touch protoc_middleman @@ -265,7 +267,7 @@ else # building out-of-tree. protoc_middleman: $(top_srcdir)/src/protoc$(EXEEXT) $(conformance_protoc_inputs) $(conformance_proto2_protoc_inputs) $(well_known_type_protoc_inputs) google-protobuf oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --ruby_out=$$oldpwd --objc_out=$$oldpwd --python_out=$$oldpwd --php_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd $(conformance_protoc_inputs) ) - oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --python_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd $(conformance_proto2_protoc_inputs) ) + oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --objc_out=. --python_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd $(conformance_proto2_protoc_inputs) ) oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --ruby_out=$$oldpwd --python_out=$$oldpwd --php_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd/google-protobuf $(well_known_type_protoc_inputs) ) ## @mkdir -p lite ## oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --java_out=lite:$$oldpwd/lite $(conformance_protoc_inputs) $(well_known_type_protoc_inputs) ) -- cgit v1.2.3 From c2831a346c02678c751cdb8fb98387c773dc08a4 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Jul 2017 15:58:07 -0400 Subject: Add the proto2 message conformance support for ObjC. --- conformance/conformance_objc.m | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/conformance/conformance_objc.m b/conformance/conformance_objc.m index a5012ec4..6023fc78 100644 --- a/conformance/conformance_objc.m +++ b/conformance/conformance_objc.m @@ -31,6 +31,7 @@ #import #import "Conformance.pbobjc.h" +#import "google/protobuf/TestMessagesProto2.pbobjc.h" #import "google/protobuf/TestMessagesProto3.pbobjc.h" static void Die(NSString *format, ...) __dead2; @@ -63,7 +64,7 @@ static NSData *CheckedReadDataOfLength(NSFileHandle *handle, NSUInteger numBytes static ConformanceResponse *DoTest(ConformanceRequest *request) { ConformanceResponse *response = [ConformanceResponse message]; - Proto3TestAllTypesProto3 *testMessage = nil; + GPBMessage *testMessage = nil; switch (request.payloadOneOfCase) { case ConformanceRequest_Payload_OneOfCase_GPBUnsetOneOfCase: @@ -71,20 +72,21 @@ static ConformanceResponse *DoTest(ConformanceRequest *request) { break; case ConformanceRequest_Payload_OneOfCase_ProtobufPayload: { - if ([request.messageType isEqualToString:@"protobuf_test_messages.proto3.TestAllTypesProto3"]) { + Class msgClass = nil; + if ([request.messageType isEqual:@"protobuf_test_messages.proto3.TestAllTypesProto3"]) { + msgClass = [Proto3TestAllTypesProto3 class]; + } else if ([request.messageType isEqual:@"protobuf_test_messages.proto2.TestAllTypesProto2"]) { + msgClass = [TestAllTypesProto2 class]; + } else { + Die(@"Protobuf request doesn't have specific payload type"); + } + if (msgClass) { NSError *error = nil; - testMessage = [Proto3TestAllTypesProto3 parseFromData:request.protobufPayload - error:&error]; + testMessage = [msgClass parseFromData:request.protobufPayload error:&error]; if (!testMessage) { response.parseError = [NSString stringWithFormat:@"Parse error: %@", error]; } - } else if ([request.messageType isEqualToString:@"protobuf_test_messages.proto2.TestAllTypesProto2"]) { - response.skipped = @"ObjC doesn't support proto2"; - break; - } else { - Die(@"Protobuf request doesn't have specific payload type"); - break; } break; } -- cgit v1.2.3 From 3caf9fd00a84d71c63c9fb873eb9befd70400153 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Jul 2017 16:24:05 -0400 Subject: Review feedback. - Better error message for unknown messageType. - Remove unneeded if. --- conformance/conformance_objc.m | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/conformance/conformance_objc.m b/conformance/conformance_objc.m index 6023fc78..84a43811 100644 --- a/conformance/conformance_objc.m +++ b/conformance/conformance_objc.m @@ -78,15 +78,13 @@ static ConformanceResponse *DoTest(ConformanceRequest *request) { } else if ([request.messageType isEqual:@"protobuf_test_messages.proto2.TestAllTypesProto2"]) { msgClass = [TestAllTypesProto2 class]; } else { - Die(@"Protobuf request doesn't have specific payload type"); + Die(@"Protobuf request had an unknown message_type: %@", request.messageType); } - if (msgClass) { - NSError *error = nil; - testMessage = [msgClass parseFromData:request.protobufPayload error:&error]; - if (!testMessage) { - response.parseError = - [NSString stringWithFormat:@"Parse error: %@", error]; - } + NSError *error = nil; + testMessage = [msgClass parseFromData:request.protobufPayload error:&error]; + if (!testMessage) { + response.parseError = + [NSString stringWithFormat:@"Parse error: %@", error]; } break; } -- cgit v1.2.3 From 9a4692d8af01697571c0fa33b17223df79423992 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Wed, 26 Jul 2017 16:44:42 -0400 Subject: Update the comment on the message_type to cover what it should be. --- conformance/conformance.proto | 4 +++- csharp/src/Google.Protobuf.Conformance/Conformance.cs | 4 +++- csharp/src/Google.Protobuf.Test/TestProtos/TestMessagesProto3.cs | 6 +++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/conformance/conformance.proto b/conformance/conformance.proto index 10e5d34e..525140e9 100644 --- a/conformance/conformance.proto +++ b/conformance/conformance.proto @@ -78,7 +78,9 @@ message ConformanceRequest { // Which format should the testee serialize its message to? WireFormat requested_output_format = 3; - // should be set to either "proto2" or "proto3" + // The full name for the test message to use; for the moment, either: + // protobuf_test_messages.proto3.TestAllTypesProto3 or + // protobuf_test_messages.proto2.TestAllTypesProto2. string message_type = 4; } diff --git a/csharp/src/Google.Protobuf.Conformance/Conformance.cs b/csharp/src/Google.Protobuf.Conformance/Conformance.cs index e1fbb350..394607b9 100644 --- a/csharp/src/Google.Protobuf.Conformance/Conformance.cs +++ b/csharp/src/Google.Protobuf.Conformance/Conformance.cs @@ -142,7 +142,9 @@ namespace Conformance { public const int MessageTypeFieldNumber = 4; private string messageType_ = ""; /// - /// should be set to either "proto2" or "proto3" + /// The full name for the test message to use; for the moment, either: + /// protobuf_test_messages.proto3.TestAllTypesProto3 or + /// protobuf_test_messages.proto2.TestAllTypesProto2. /// [global::System.Diagnostics.DebuggerNonUserCodeAttribute] public string MessageType { diff --git a/csharp/src/Google.Protobuf.Test/TestProtos/TestMessagesProto3.cs b/csharp/src/Google.Protobuf.Test/TestProtos/TestMessagesProto3.cs index 076afb3b..6607be7a 100644 --- a/csharp/src/Google.Protobuf.Test/TestProtos/TestMessagesProto3.cs +++ b/csharp/src/Google.Protobuf.Test/TestProtos/TestMessagesProto3.cs @@ -198,9 +198,9 @@ namespace ProtobufTestMessages.Proto3 { "AjgBIjkKCk5lc3RlZEVudW0SBwoDRk9PEAASBwoDQkFSEAESBwoDQkFaEAIS", "EAoDTkVHEP///////////wFCDQoLb25lb2ZfZmllbGQiGwoORm9yZWlnbk1l", "c3NhZ2USCQoBYxgBIAEoBSpACgtGb3JlaWduRW51bRIPCgtGT1JFSUdOX0ZP", - "TxAAEg8KC0ZPUkVJR05fQkFSEAESDwoLRk9SRUlHTl9CQVoQAkIvCihjb20u", - "Z29vZ2xlLnByb3RvYnVmX3Rlc3RfbWVzc2FnZXMucHJvdG8zSAH4AQFiBnBy", - "b3RvMw==")); + "TxAAEg8KC0ZPUkVJR05fQkFSEAESDwoLRk9SRUlHTl9CQVoQAkI4Cihjb20u", + "Z29vZ2xlLnByb3RvYnVmX3Rlc3RfbWVzc2FnZXMucHJvdG8zSAH4AQGiAgZQ", + "cm90bzNiBnByb3RvMw==")); descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData, new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.AnyReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.DurationReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.FieldMaskReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.StructReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.TimestampReflection.Descriptor, global::Google.Protobuf.WellKnownTypes.WrappersReflection.Descriptor, }, new pbr::GeneratedClrTypeInfo(new[] {typeof(global::ProtobufTestMessages.Proto3.ForeignEnum), }, new pbr::GeneratedClrTypeInfo[] { -- cgit v1.2.3 From 547d76ed8e23275d1acadb92d0ea891562bf55e0 Mon Sep 17 00:00:00 2001 From: king6cong Date: Mon, 31 Jul 2017 14:45:57 +0800 Subject: Add classpath for java example Makefile --- examples/Makefile | 2 +- examples/README.txt | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/examples/Makefile b/examples/Makefile index 51f13426..d16bb56d 100644 --- a/examples/Makefile +++ b/examples/Makefile @@ -51,7 +51,7 @@ list_people_gotest: list_people.go list_people_go go test list_people.go list_people_test.go javac_middleman: AddPerson.java ListPeople.java protoc_middleman - javac AddPerson.java ListPeople.java com/example/tutorial/AddressBookProtos.java + javac -cp ../java/core/target/*.jar AddPerson.java ListPeople.java com/example/tutorial/AddressBookProtos.java @touch javac_middleman add_person_java: javac_middleman diff --git a/examples/README.txt b/examples/README.txt index b33f8414..2e4f6e4e 100644 --- a/examples/README.txt +++ b/examples/README.txt @@ -28,6 +28,13 @@ These examples are part of the Protocol Buffers tutorial, located at: "-lpthread" from the linker commands (perhaps replacing it with something else). We didn't do this automatically because we wanted to keep the example simple. +## Java ## + +protobuf-java-*.jar can be generated by: + cd ../java + mvn package +and will be used by "make java" + ## Go ## The Go example requires a plugin to the protocol buffer compiler, so it is not -- cgit v1.2.3 From d32f5b4de3501e1bf3c0bc98d8e74219d47ebe69 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 1 Aug 2017 09:42:46 -0700 Subject: Removes unnecessary pass-by-references in PHP internal classes (#3433) --- php/src/Google/Protobuf/Internal/DescriptorPool.php | 14 +++++++------- php/src/Google/Protobuf/Internal/MapEntry.php | 4 ++-- php/src/Google/Protobuf/Internal/OneofDescriptor.php | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/php/src/Google/Protobuf/Internal/DescriptorPool.php b/php/src/Google/Protobuf/Internal/DescriptorPool.php index 2c00dfb6..65d1a884 100644 --- a/php/src/Google/Protobuf/Internal/DescriptorPool.php +++ b/php/src/Google/Protobuf/Internal/DescriptorPool.php @@ -61,17 +61,17 @@ class DescriptorPool $files->mergeFromString($data); $file = FileDescriptor::buildFromProto($files->getFile()[0]); - foreach ($file->getMessageType() as &$desc) { + foreach ($file->getMessageType() as $desc) { $this->addDescriptor($desc); } unset($desc); - foreach ($file->getEnumType() as &$desc) { + foreach ($file->getEnumType() as $desc) { $this->addEnumDescriptor($desc); } unset($desc); - foreach ($file->getMessageType() as &$desc) { + foreach ($file->getMessageType() as $desc) { $this->crossLink($desc); } unset($desc); @@ -129,9 +129,9 @@ class DescriptorPool return $this->class_to_enum_desc[$klass]; } - private function crossLink(&$desc) + private function crossLink(Descriptor $desc) { - foreach ($desc->getField() as &$field) { + foreach ($desc->getField() as $field) { switch ($field->getType()) { case GPBType::MESSAGE: $proto = $field->getMessageType(); @@ -149,7 +149,7 @@ class DescriptorPool } unset($field); - foreach ($desc->getNestedType() as &$nested_type) { + foreach ($desc->getNestedType() as $nested_type) { $this->crossLink($nested_type); } unset($nested_type); @@ -157,7 +157,7 @@ class DescriptorPool public function finish() { - foreach ($this->class_to_desc as $klass => &$desc) { + foreach ($this->class_to_desc as $klass => $desc) { $this->crossLink($desc); } unset($desc); diff --git a/php/src/Google/Protobuf/Internal/MapEntry.php b/php/src/Google/Protobuf/Internal/MapEntry.php index 926645e1..9c32f1ea 100644 --- a/php/src/Google/Protobuf/Internal/MapEntry.php +++ b/php/src/Google/Protobuf/Internal/MapEntry.php @@ -39,7 +39,7 @@ class MapEntry extends Message public $key; public $value; - public function setKey(&$key) { + public function setKey($key) { $this->key = $key; } @@ -47,7 +47,7 @@ class MapEntry extends Message return $this->key; } - public function setValue(&$value) { + public function setValue($value) { $this->value = $value; } diff --git a/php/src/Google/Protobuf/Internal/OneofDescriptor.php b/php/src/Google/Protobuf/Internal/OneofDescriptor.php index 04988737..57961f39 100644 --- a/php/src/Google/Protobuf/Internal/OneofDescriptor.php +++ b/php/src/Google/Protobuf/Internal/OneofDescriptor.php @@ -48,7 +48,7 @@ class OneofDescriptor return $this->name; } - public function addField(&$field) + public function addField(Descriptor $field) { $this->fields[] = $field; } -- cgit v1.2.3 From be73938d72c91ff0075b1ed683f07922ad5801b0 Mon Sep 17 00:00:00 2001 From: Tony Wong Date: Wed, 2 Aug 2017 05:22:47 +0900 Subject: Change divideInt64ToInt32 to static (#3436) divideInt64ToInt32 is called statically from protobuf/php/src/Google/Protobuf/Internal/CodedOutputStream.php (the only reference) This causes fatal error in PHP 7.1 (32-bit only because 64-bit doesn't use this function) --- php/src/Google/Protobuf/Internal/GPBUtil.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/php/src/Google/Protobuf/Internal/GPBUtil.php b/php/src/Google/Protobuf/Internal/GPBUtil.php index 22ad27f9..6fe36068 100644 --- a/php/src/Google/Protobuf/Internal/GPBUtil.php +++ b/php/src/Google/Protobuf/Internal/GPBUtil.php @@ -38,7 +38,7 @@ use Google\Protobuf\Internal\MapField; class GPBUtil { - public function divideInt64ToInt32($value, &$high, &$low, $trim = false) + public static function divideInt64ToInt32($value, &$high, &$low, $trim = false) { $isNeg = (bccomp($value, 0) < 0); if ($isNeg) { -- cgit v1.2.3 From c15a3269f9dea81db5a45b527cd8699f11b03f0b Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Wed, 2 Aug 2017 07:42:27 -0700 Subject: Expose descriptor API in php c extension (#3422) --- Makefile.am | 2 + php/ext/google/protobuf/def.c | 481 ++++++++++++++++++++++++++++++++- php/ext/google/protobuf/protobuf.c | 31 ++- php/ext/google/protobuf/protobuf.h | 63 ++++- php/tests/descriptors_test.php | 243 +++++++++++++++++ php/tests/proto/test_descriptors.proto | 35 +++ php/tests/test.sh | 2 +- tests.sh | 11 +- 8 files changed, 841 insertions(+), 27 deletions(-) create mode 100644 php/tests/descriptors_test.php create mode 100644 php/tests/proto/test_descriptors.proto diff --git a/Makefile.am b/Makefile.am index 7ffb202e..6279b2de 100644 --- a/Makefile.am +++ b/Makefile.am @@ -658,6 +658,7 @@ php_EXTRA_DIST= \ php/tests/array_test.php \ php/tests/autoload.php \ php/tests/compatibility_test.sh \ + php/tests/descriptors_test.php \ php/tests/encode_decode_test.php \ php/tests/gdb_test.sh \ php/tests/generated_class_test.php \ @@ -666,6 +667,7 @@ php_EXTRA_DIST= \ php/tests/map_field_test.php \ php/tests/memory_leak_test.php \ php/tests/php_implementation_test.php \ + php/tests/proto/test_descriptors.proto \ php/tests/proto/test_empty_php_namespace.proto \ php/tests/proto/test_import_descriptor_proto.proto \ php/tests/proto/test_include.proto \ diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index 332616b2..ae17455c 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -37,12 +37,27 @@ const int kReservedNamesSize = 3; static void descriptor_init_c_instance(Descriptor* intern TSRMLS_DC); static void descriptor_free_c(Descriptor* object TSRMLS_DC); +static void field_descriptor_init_c_instance(FieldDescriptor* intern TSRMLS_DC); +static void field_descriptor_free_c(FieldDescriptor* object TSRMLS_DC); + static void enum_descriptor_init_c_instance(EnumDescriptor* intern TSRMLS_DC); static void enum_descriptor_free_c(EnumDescriptor* object TSRMLS_DC); +static void enum_value_descriptor_init_c_instance( + EnumValueDescriptor *intern TSRMLS_DC); +static void enum_value_descriptor_free_c(EnumValueDescriptor *object TSRMLS_DC); + static void descriptor_pool_free_c(DescriptorPool* object TSRMLS_DC); static void descriptor_pool_init_c_instance(DescriptorPool* pool TSRMLS_DC); +static void internal_descriptor_pool_free_c( + InternalDescriptorPool *object TSRMLS_DC); +static void internal_descriptor_pool_init_c_instance( + InternalDescriptorPool *pool TSRMLS_DC); + +static void oneof_descriptor_free_c(Oneof* object TSRMLS_DC); +static void oneof_descriptor_init_c_instance(Oneof* pool TSRMLS_DC); + // ----------------------------------------------------------------------------- // Common Utilities // ----------------------------------------------------------------------------- @@ -169,10 +184,15 @@ void gpb_type_init(TSRMLS_D) { // ----------------------------------------------------------------------------- static zend_function_entry descriptor_methods[] = { + PHP_ME(Descriptor, getFullName, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Descriptor, getField, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Descriptor, getFieldCount, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Descriptor, getOneofDecl, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Descriptor, getOneofDeclCount, NULL, ZEND_ACC_PUBLIC) ZEND_FE_END }; -DEFINE_CLASS(Descriptor, descriptor, "Google\\Protobuf\\Internal\\Descriptor"); +DEFINE_CLASS(Descriptor, descriptor, "Google\\Protobuf\\Descriptor"); static void descriptor_free_c(Descriptor *self TSRMLS_DC) { if (self->layout) { @@ -203,7 +223,6 @@ static void descriptor_free_c(Descriptor *self TSRMLS_DC) { } static void descriptor_init_c_instance(Descriptor *desc TSRMLS_DC) { - // zend_object_std_init(&desc->std, descriptor_type TSRMLS_CC); desc->msgdef = NULL; desc->layout = NULL; desc->klass = NULL; @@ -215,30 +234,207 @@ static void descriptor_init_c_instance(Descriptor *desc TSRMLS_DC) { desc->json_serialize_handlers_preserve = NULL; } +PHP_METHOD(Descriptor, getFullName) { + Descriptor *intern = UNBOX(Descriptor, getThis()); + const char* fullname = upb_msgdef_fullname(intern->msgdef); + PHP_PROTO_RETVAL_STRINGL(fullname, strlen(fullname), 1); +} + +PHP_METHOD(Descriptor, getField) { + long index; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &index) == + FAILURE) { + zend_error(E_USER_ERROR, "Expect integer for index.\n"); + return; + } + + Descriptor *intern = UNBOX(Descriptor, getThis()); + int field_num = upb_msgdef_numfields(intern->msgdef); + if (index < 0 || index >= field_num) { + zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); + return; + } + + upb_msg_field_iter iter; + int i; + for(upb_msg_field_begin(&iter, intern->msgdef), i = 0; + !upb_msg_field_done(&iter) && i < index; + upb_msg_field_next(&iter), i++); + const upb_fielddef *field = upb_msg_iter_field(&iter); + + PHP_PROTO_HASHTABLE_VALUE field_hashtable_value = get_def_obj(field); + if (field_hashtable_value == NULL) { +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(field_hashtable_value); + ZVAL_OBJ(field_hashtable_value, field_descriptor_type->create_object( + field_descriptor_type TSRMLS_CC)); +#else + field_hashtable_value = + field_descriptor_type->create_object(field_descriptor_type TSRMLS_CC); +#endif + FieldDescriptor *field_php = + UNBOX_HASHTABLE_VALUE(FieldDescriptor, field_hashtable_value); + field_php->fielddef = field; + add_def_obj(field, field_hashtable_value); + } + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(field_hashtable_value, 1, 0); +#else + ++GC_REFCOUNT(field_hashtable_value); + RETURN_OBJ(field_hashtable_value); +#endif +} + +PHP_METHOD(Descriptor, getFieldCount) { + Descriptor *intern = UNBOX(Descriptor, getThis()); + RETURN_LONG(upb_msgdef_numfields(intern->msgdef)); +} + +PHP_METHOD(Descriptor, getOneofDecl) { + long index; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &index) == + FAILURE) { + zend_error(E_USER_ERROR, "Expect integer for index.\n"); + return; + } + + Descriptor *intern = UNBOX(Descriptor, getThis()); + int field_num = upb_msgdef_numoneofs(intern->msgdef); + if (index < 0 || index >= field_num) { + zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); + return; + } + + upb_msg_oneof_iter iter; + int i; + for(upb_msg_oneof_begin(&iter, intern->msgdef), i = 0; + !upb_msg_oneof_done(&iter) && i < index; + upb_msg_oneof_next(&iter), i++); + upb_oneofdef *oneof = upb_msg_iter_oneof(&iter); + + ZVAL_OBJ(return_value, oneof_descriptor_type->create_object( + oneof_descriptor_type TSRMLS_CC)); + Oneof *oneof_php = UNBOX(Oneof, return_value); + oneof_php->oneofdef = oneof; +} + +PHP_METHOD(Descriptor, getOneofDeclCount) { + Descriptor *intern = UNBOX(Descriptor, getThis()); + RETURN_LONG(upb_msgdef_numoneofs(intern->msgdef)); +} + // ----------------------------------------------------------------------------- // EnumDescriptor // ----------------------------------------------------------------------------- static zend_function_entry enum_descriptor_methods[] = { + PHP_ME(EnumDescriptor, getValue, NULL, ZEND_ACC_PUBLIC) + PHP_ME(EnumDescriptor, getValueCount, NULL, ZEND_ACC_PUBLIC) ZEND_FE_END }; DEFINE_CLASS(EnumDescriptor, enum_descriptor, - "Google\\Protobuf\\Internal\\EnumDescriptor"); + "Google\\Protobuf\\EnumDescriptor"); static void enum_descriptor_free_c(EnumDescriptor *self TSRMLS_DC) { } static void enum_descriptor_init_c_instance(EnumDescriptor *self TSRMLS_DC) { - // zend_object_std_init(&self->std, enum_descriptor_type TSRMLS_CC); self->enumdef = NULL; self->klass = NULL; } +PHP_METHOD(EnumDescriptor, getValue) { + long index; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &index) == + FAILURE) { + zend_error(E_USER_ERROR, "Expect integer for index.\n"); + return; + } + + EnumDescriptor *intern = UNBOX(EnumDescriptor, getThis()); + int field_num = upb_enumdef_numvals(intern->enumdef); + if (index < 0 || index >= field_num) { + zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); + return; + } + + upb_enum_iter iter; + int i; + for(upb_enum_begin(&iter, intern->enumdef), i = 0; + !upb_enum_done(&iter) && i < index; + upb_enum_next(&iter), i++); + + ZVAL_OBJ(return_value, enum_value_descriptor_type->create_object( + enum_value_descriptor_type TSRMLS_CC)); + EnumValueDescriptor *enum_value_php = + UNBOX(EnumValueDescriptor, return_value); + enum_value_php->name = upb_enum_iter_name(&iter); + enum_value_php->number = upb_enum_iter_number(&iter); +} + +PHP_METHOD(EnumDescriptor, getValueCount) { + EnumDescriptor *intern = UNBOX(EnumDescriptor, getThis()); + RETURN_LONG(upb_enumdef_numvals(intern->enumdef)); +} + +// ----------------------------------------------------------------------------- +// EnumValueDescriptor +// ----------------------------------------------------------------------------- + +static zend_function_entry enum_value_descriptor_methods[] = { + PHP_ME(EnumValueDescriptor, getName, NULL, ZEND_ACC_PUBLIC) + PHP_ME(EnumValueDescriptor, getNumber, NULL, ZEND_ACC_PUBLIC) + ZEND_FE_END +}; + +DEFINE_CLASS(EnumValueDescriptor, enum_value_descriptor, + "Google\\Protobuf\\EnumValueDescriptor"); + +static void enum_value_descriptor_free_c(EnumValueDescriptor *self TSRMLS_DC) { +} + +static void enum_value_descriptor_init_c_instance(EnumValueDescriptor *self TSRMLS_DC) { + self->name = NULL; + self->number = 0; +} + +PHP_METHOD(EnumValueDescriptor, getName) { + EnumValueDescriptor *intern = UNBOX(EnumValueDescriptor, getThis()); + PHP_PROTO_RETVAL_STRINGL(intern->name, strlen(intern->name), 1); +} + +PHP_METHOD(EnumValueDescriptor, getNumber) { + EnumValueDescriptor *intern = UNBOX(EnumValueDescriptor, getThis()); + RETURN_LONG(intern->number); +} + // ----------------------------------------------------------------------------- // FieldDescriptor // ----------------------------------------------------------------------------- +static zend_function_entry field_descriptor_methods[] = { + PHP_ME(FieldDescriptor, getName, NULL, ZEND_ACC_PUBLIC) + PHP_ME(FieldDescriptor, getNumber, NULL, ZEND_ACC_PUBLIC) + PHP_ME(FieldDescriptor, getLabel, NULL, ZEND_ACC_PUBLIC) + PHP_ME(FieldDescriptor, getType, NULL, ZEND_ACC_PUBLIC) + PHP_ME(FieldDescriptor, isMap, NULL, ZEND_ACC_PUBLIC) + PHP_ME(FieldDescriptor, getEnumType, NULL, ZEND_ACC_PUBLIC) + PHP_ME(FieldDescriptor, getMessageType, NULL, ZEND_ACC_PUBLIC) + ZEND_FE_END +}; + +DEFINE_CLASS(FieldDescriptor, field_descriptor, + "Google\\Protobuf\\FieldDescriptor"); + +static void field_descriptor_free_c(FieldDescriptor *self TSRMLS_DC) { +} + +static void field_descriptor_init_c_instance(FieldDescriptor *self TSRMLS_DC) { + self->fielddef = NULL; +} + upb_fieldtype_t to_fieldtype(upb_descriptortype_t type) { switch (type) { #define CASE(descriptor_type, type) \ @@ -272,6 +468,150 @@ upb_fieldtype_t to_fieldtype(upb_descriptortype_t type) { return 0; } +PHP_METHOD(FieldDescriptor, getName) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + const char* name = upb_fielddef_name(intern->fielddef); + PHP_PROTO_RETVAL_STRINGL(name, strlen(name), 1); +} + +PHP_METHOD(FieldDescriptor, getNumber) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + RETURN_LONG(upb_fielddef_number(intern->fielddef)); +} + +PHP_METHOD(FieldDescriptor, getLabel) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + RETURN_LONG(upb_fielddef_label(intern->fielddef)); +} + +PHP_METHOD(FieldDescriptor, getType) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + RETURN_LONG(upb_fielddef_descriptortype(intern->fielddef)); +} + +PHP_METHOD(FieldDescriptor, isMap) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + RETURN_BOOL(upb_fielddef_ismap(intern->fielddef)); +} + +PHP_METHOD(FieldDescriptor, getEnumType) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + const upb_enumdef *enumdef = upb_fielddef_enumsubdef(intern->fielddef); + if (enumdef == NULL) { + char error_msg[100]; + sprintf(error_msg, "Cannot get enum type for non-enum field '%s'", + upb_fielddef_name(intern->fielddef)); + zend_throw_exception(NULL, error_msg, 0 TSRMLS_CC); + return; + } + PHP_PROTO_HASHTABLE_VALUE desc = get_def_obj(enumdef); + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(desc, 1, 0); +#else + ++GC_REFCOUNT(desc); + RETURN_OBJ(desc); +#endif +} + +PHP_METHOD(FieldDescriptor, getMessageType) { + FieldDescriptor *intern = UNBOX(FieldDescriptor, getThis()); + const upb_msgdef *msgdef = upb_fielddef_msgsubdef(intern->fielddef); + if (msgdef == NULL) { + char error_msg[100]; + sprintf(error_msg, "Cannot get message type for non-message field '%s'", + upb_fielddef_name(intern->fielddef)); + zend_throw_exception(NULL, error_msg, 0 TSRMLS_CC); + return; + } + PHP_PROTO_HASHTABLE_VALUE desc = get_def_obj(msgdef); + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(desc, 1, 0); +#else + ++GC_REFCOUNT(desc); + RETURN_OBJ(desc); +#endif +} + +// ----------------------------------------------------------------------------- +// Oneof +// ----------------------------------------------------------------------------- + +static zend_function_entry oneof_descriptor_methods[] = { + PHP_ME(Oneof, getName, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Oneof, getField, NULL, ZEND_ACC_PUBLIC) + PHP_ME(Oneof, getFieldCount, NULL, ZEND_ACC_PUBLIC) + ZEND_FE_END +}; + +DEFINE_CLASS(Oneof, oneof_descriptor, + "Google\\Protobuf\\OneofDescriptor"); + +static void oneof_descriptor_free_c(Oneof *self TSRMLS_DC) { +} + +static void oneof_descriptor_init_c_instance(Oneof *self TSRMLS_DC) { + self->oneofdef = NULL; +} + +PHP_METHOD(Oneof, getName) { + Oneof *intern = UNBOX(Oneof, getThis()); + const char *name = upb_oneofdef_name(intern->oneofdef); + PHP_PROTO_RETVAL_STRINGL(name, strlen(name), 1); +} + +PHP_METHOD(Oneof, getField) { + long index; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l", &index) == + FAILURE) { + zend_error(E_USER_ERROR, "Expect integer for index.\n"); + return; + } + + Oneof *intern = UNBOX(Oneof, getThis()); + int field_num = upb_oneofdef_numfields(intern->oneofdef); + if (index < 0 || index >= field_num) { + zend_error(E_USER_ERROR, "Cannot get element at %ld.\n", index); + return; + } + + upb_oneof_iter iter; + int i; + for(upb_oneof_begin(&iter, intern->oneofdef), i = 0; + !upb_oneof_done(&iter) && i < index; + upb_oneof_next(&iter), i++); + const upb_fielddef *field = upb_oneof_iter_field(&iter); + + PHP_PROTO_HASHTABLE_VALUE field_hashtable_value = get_def_obj(field); + if (field_hashtable_value == NULL) { +#if PHP_MAJOR_VERSION < 7 + MAKE_STD_ZVAL(field_hashtable_value); + ZVAL_OBJ(field_hashtable_value, field_descriptor_type->create_object( + field_descriptor_type TSRMLS_CC)); +#else + field_hashtable_value = + field_descriptor_type->create_object(field_descriptor_type TSRMLS_CC); +#endif + FieldDescriptor *field_php = + UNBOX_HASHTABLE_VALUE(FieldDescriptor, field_hashtable_value); + field_php->fielddef = field; + add_def_obj(field, field_hashtable_value); + } + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(field_hashtable_value, 1, 0); +#else + ++GC_REFCOUNT(field_hashtable_value); + RETURN_OBJ(field_hashtable_value); +#endif +} + +PHP_METHOD(Oneof, getFieldCount) { + Oneof *intern = UNBOX(Oneof, getThis()); + RETURN_LONG(upb_oneofdef_numfields(intern->oneofdef)); +} + // ----------------------------------------------------------------------------- // DescriptorPool // ----------------------------------------------------------------------------- @@ -279,52 +619,79 @@ upb_fieldtype_t to_fieldtype(upb_descriptortype_t type) { static zend_function_entry descriptor_pool_methods[] = { PHP_ME(DescriptorPool, getGeneratedPool, NULL, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) - PHP_ME(DescriptorPool, internalAddGeneratedFile, NULL, ZEND_ACC_PUBLIC) + PHP_ME(DescriptorPool, getDescriptorByClassName, NULL, ZEND_ACC_PUBLIC) + PHP_ME(DescriptorPool, getEnumDescriptorByClassName, NULL, ZEND_ACC_PUBLIC) + ZEND_FE_END +}; + +static zend_function_entry internal_descriptor_pool_methods[] = { + PHP_ME(InternalDescriptorPool, getGeneratedPool, NULL, + ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) + PHP_ME(InternalDescriptorPool, internalAddGeneratedFile, NULL, ZEND_ACC_PUBLIC) ZEND_FE_END }; DEFINE_CLASS(DescriptorPool, descriptor_pool, + "Google\\Protobuf\\DescriptorPool"); +DEFINE_CLASS(InternalDescriptorPool, internal_descriptor_pool, "Google\\Protobuf\\Internal\\DescriptorPool"); // wrapper of generated pool #if PHP_MAJOR_VERSION < 7 zval* generated_pool_php; +zval* internal_generated_pool_php; #else zend_object *generated_pool_php; +zend_object *internal_generated_pool_php; #endif -DescriptorPool *generated_pool; // The actual generated pool +InternalDescriptorPool *generated_pool; // The actual generated pool static void init_generated_pool_once(TSRMLS_D) { - if (generated_pool_php == NULL) { + if (generated_pool == NULL) { #if PHP_MAJOR_VERSION < 7 MAKE_STD_ZVAL(generated_pool_php); + MAKE_STD_ZVAL(internal_generated_pool_php); + ZVAL_OBJ(internal_generated_pool_php, + internal_descriptor_pool_type->create_object( + internal_descriptor_pool_type TSRMLS_CC)); + generated_pool = UNBOX(InternalDescriptorPool, internal_generated_pool_php); ZVAL_OBJ(generated_pool_php, descriptor_pool_type->create_object( descriptor_pool_type TSRMLS_CC)); - generated_pool = UNBOX(DescriptorPool, generated_pool_php); #else + internal_generated_pool_php = internal_descriptor_pool_type->create_object( + internal_descriptor_pool_type TSRMLS_CC); + generated_pool = (InternalDescriptorPool *)((char *)internal_generated_pool_php - + XtOffsetOf(InternalDescriptorPool, std)); generated_pool_php = descriptor_pool_type->create_object(descriptor_pool_type TSRMLS_CC); - generated_pool = (DescriptorPool *)((char *)generated_pool_php - - XtOffsetOf(DescriptorPool, std)); #endif } } -static void descriptor_pool_init_c_instance(DescriptorPool *pool TSRMLS_DC) { - // zend_object_std_init(&pool->std, descriptor_pool_type TSRMLS_CC); +static void internal_descriptor_pool_init_c_instance( + InternalDescriptorPool *pool TSRMLS_DC) { pool->symtab = upb_symtab_new(); ALLOC_HASHTABLE(pool->pending_list); zend_hash_init(pool->pending_list, 1, NULL, ZVAL_PTR_DTOR, 0); } -static void descriptor_pool_free_c(DescriptorPool *pool TSRMLS_DC) { +static void internal_descriptor_pool_free_c( + InternalDescriptorPool *pool TSRMLS_DC) { upb_symtab_free(pool->symtab); zend_hash_destroy(pool->pending_list); FREE_HASHTABLE(pool->pending_list); } +static void descriptor_pool_init_c_instance(DescriptorPool *pool TSRMLS_DC) { + assert(generated_pool != NULL); + pool->intern = generated_pool; +} + +static void descriptor_pool_free_c(DescriptorPool *pool TSRMLS_DC) { +} + static void validate_enumdef(const upb_enumdef *enumdef) { // Verify that an entry exists with integer value 0. (This is the default // value.) @@ -358,6 +725,16 @@ PHP_METHOD(DescriptorPool, getGeneratedPool) { #endif } +PHP_METHOD(InternalDescriptorPool, getGeneratedPool) { + init_generated_pool_once(TSRMLS_C); +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(internal_generated_pool_php, 1, 0); +#else + ++GC_REFCOUNT(internal_generated_pool_php); + RETURN_OBJ(internal_generated_pool_php); +#endif +} + static void classname_no_prefix(const char *fullname, const char *package_name, char *class_name) { size_t i = 0, j; @@ -455,7 +832,7 @@ static void convert_to_class_name_inplace(const char *package, memcpy(classname + i, prefix, prefix_len); } -PHP_METHOD(DescriptorPool, internalAddGeneratedFile) { +PHP_METHOD(InternalDescriptorPool, internalAddGeneratedFile) { char *data = NULL; PHP_PROTO_SIZE data_len; upb_filedef **files; @@ -466,7 +843,7 @@ PHP_METHOD(DescriptorPool, internalAddGeneratedFile) { return; } - DescriptorPool *pool = UNBOX(DescriptorPool, getThis()); + InternalDescriptorPool *pool = UNBOX(InternalDescriptorPool, getThis()); CHECK_UPB(files = upb_loaddescriptor(data, data_len, &pool, &status), "Parse binary descriptors to internal descriptors failed"); @@ -550,3 +927,77 @@ PHP_METHOD(DescriptorPool, internalAddGeneratedFile) { upb_filedef_unref(files[0], &pool); upb_gfree(files); } + +PHP_METHOD(DescriptorPool, getDescriptorByClassName) { + DescriptorPool *public_pool = UNBOX(DescriptorPool, getThis()); + InternalDescriptorPool *pool = public_pool->intern; + + char *classname = NULL; + PHP_PROTO_SIZE classname_len; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &classname, + &classname_len) == FAILURE) { + return; + } + + PHP_PROTO_CE_DECLARE pce; + if (php_proto_zend_lookup_class(classname, classname_len, &pce) == + FAILURE) { + RETURN_NULL(); + } + + PHP_PROTO_HASHTABLE_VALUE desc = get_ce_obj(PHP_PROTO_CE_UNREF(pce)); + if (desc == NULL) { + RETURN_NULL(); + } + + zend_class_entry* instance_ce = HASHTABLE_VALUE_CE(desc); + + if (!instanceof_function(instance_ce, descriptor_type TSRMLS_CC)) { + RETURN_NULL(); + } + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(desc, 1, 0); +#else + ++GC_REFCOUNT(desc); + RETURN_OBJ(desc); +#endif +} + +PHP_METHOD(DescriptorPool, getEnumDescriptorByClassName) { + DescriptorPool *public_pool = UNBOX(DescriptorPool, getThis()); + InternalDescriptorPool *pool = public_pool->intern; + + char *classname = NULL; + PHP_PROTO_SIZE classname_len; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &classname, + &classname_len) == FAILURE) { + return; + } + + PHP_PROTO_CE_DECLARE pce; + if (php_proto_zend_lookup_class(classname, classname_len, &pce) == + FAILURE) { + RETURN_NULL(); + } + + PHP_PROTO_HASHTABLE_VALUE desc = get_ce_obj(PHP_PROTO_CE_UNREF(pce)); + if (desc == NULL) { + RETURN_NULL(); + } + + zend_class_entry* instance_ce = HASHTABLE_VALUE_CE(desc); + + if (!instanceof_function(instance_ce, enum_descriptor_type TSRMLS_CC)) { + RETURN_NULL(); + } + +#if PHP_MAJOR_VERSION < 7 + RETURN_ZVAL(desc, 1, 0); +#else + ++GC_REFCOUNT(desc); + RETURN_OBJ(desc); +#endif +} diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index 2b5b58c6..dc730030 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -63,7 +63,6 @@ static void* get_from_table(const HashTable* t, const void* def) { void** value; if (php_proto_zend_hash_index_find_mem(t, (zend_ulong)def, (void**)&value) == FAILURE) { - zend_error(E_ERROR, "PHP object not found for given definition.\n"); return NULL; } return *value; @@ -166,6 +165,7 @@ static PHP_RINIT_FUNCTION(protobuf) { generated_pool = NULL; generated_pool_php = NULL; + internal_generated_pool_php = NULL; return 0; } @@ -182,21 +182,40 @@ static PHP_RSHUTDOWN_FUNCTION(protobuf) { zval_dtor(generated_pool_php); FREE_ZVAL(generated_pool_php); } + if (internal_generated_pool_php != NULL) { + zval_dtor(internal_generated_pool_php); + FREE_ZVAL(internal_generated_pool_php); + } +#else + if (generated_pool_php != NULL) { + zval tmp; + ZVAL_OBJ(&tmp, generated_pool_php); + zval_dtor(&tmp); + } + if (internal_generated_pool_php != NULL) { + zval tmp; + ZVAL_OBJ(&tmp, internal_generated_pool_php); + zval_dtor(&tmp); + } #endif return 0; } static PHP_MINIT_FUNCTION(protobuf) { + descriptor_pool_init(TSRMLS_C); + descriptor_init(TSRMLS_C); + enum_descriptor_init(TSRMLS_C); + enum_value_descriptor_init(TSRMLS_C); + field_descriptor_init(TSRMLS_C); + gpb_type_init(TSRMLS_C); + internal_descriptor_pool_init(TSRMLS_C); map_field_init(TSRMLS_C); map_field_iter_init(TSRMLS_C); + message_init(TSRMLS_C); + oneof_descriptor_init(TSRMLS_C); repeated_field_init(TSRMLS_C); repeated_field_iter_init(TSRMLS_C); - gpb_type_init(TSRMLS_C); - message_init(TSRMLS_C); - descriptor_pool_init(TSRMLS_C); - descriptor_init(TSRMLS_C); - enum_descriptor_init(TSRMLS_C); util_init(TSRMLS_C); return 0; diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index c00cd917..1f79fe8c 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -185,6 +185,7 @@ #define HASHTABLE_VALUE_DTOR ZVAL_PTR_DTOR #define PHP_PROTO_HASHTABLE_VALUE zval* +#define HASHTABLE_VALUE_CE(val) Z_OBJCE_P(val) #define CREATE_HASHTABLE_VALUE(OBJ, WRAPPED_OBJ, OBJ_TYPE, OBJ_CLASS_ENTRY) \ OBJ_TYPE* OBJ; \ @@ -369,6 +370,7 @@ static inline int php_proto_zend_hash_get_current_data_ex(HashTable* ht, #define HASHTABLE_VALUE_DTOR php_proto_hashtable_descriptor_release #define PHP_PROTO_HASHTABLE_VALUE zend_object* +#define HASHTABLE_VALUE_CE(val) val->ce #define CREATE_HASHTABLE_VALUE(OBJ, WRAPPED_OBJ, OBJ_TYPE, OBJ_CLASS_ENTRY) \ OBJ_TYPE* OBJ; \ @@ -397,7 +399,9 @@ static inline int php_proto_zend_lookup_class( struct DescriptorPool; struct Descriptor; struct EnumDescriptor; +struct EnumValueDescriptor; struct FieldDescriptor; +struct InternalDescriptorPool; struct MessageField; struct MessageHeader; struct MessageLayout; @@ -410,7 +414,9 @@ struct Oneof; typedef struct DescriptorPool DescriptorPool; typedef struct Descriptor Descriptor; typedef struct EnumDescriptor EnumDescriptor; +typedef struct EnumValueDescriptor EnumValueDescriptor; typedef struct FieldDescriptor FieldDescriptor; +typedef struct InternalDescriptorPool InternalDescriptorPool; typedef struct MessageField MessageField; typedef struct MessageHeader MessageHeader; typedef struct MessageLayout MessageLayout; @@ -431,9 +437,12 @@ ZEND_END_MODULE_GLOBALS(protobuf) void descriptor_init(TSRMLS_D); void enum_descriptor_init(TSRMLS_D); void descriptor_pool_init(TSRMLS_D); +void internal_descriptor_pool_init(TSRMLS_D); +void field_descriptor_init(TSRMLS_D); void gpb_type_init(TSRMLS_D); void map_field_init(TSRMLS_D); void map_field_iter_init(TSRMLS_D); +void oneof_descriptor_init(TSRMLS_D); void repeated_field_init(TSRMLS_D); void repeated_field_iter_init(TSRMLS_D); void util_init(TSRMLS_D); @@ -458,22 +467,34 @@ extern zend_class_entry* repeated_field_type; // ----------------------------------------------------------------------------- PHP_PROTO_WRAP_OBJECT_START(DescriptorPool) + InternalDescriptorPool* intern; +PHP_PROTO_WRAP_OBJECT_END + +PHP_METHOD(DescriptorPool, getGeneratedPool); +PHP_METHOD(DescriptorPool, getDescriptorByClassName); +PHP_METHOD(DescriptorPool, getEnumDescriptorByClassName); + +PHP_PROTO_WRAP_OBJECT_START(InternalDescriptorPool) upb_symtab* symtab; HashTable* pending_list; PHP_PROTO_WRAP_OBJECT_END -PHP_METHOD(DescriptorPool, getGeneratedPool); -PHP_METHOD(DescriptorPool, internalAddGeneratedFile); +PHP_METHOD(InternalDescriptorPool, getGeneratedPool); +PHP_METHOD(InternalDescriptorPool, internalAddGeneratedFile); // wrapper of generated pool #if PHP_MAJOR_VERSION < 7 extern zval* generated_pool_php; +extern zval* internal_generated_pool_php; void descriptor_pool_free(void* object TSRMLS_DC); +void internal_descriptor_pool_free(void* object TSRMLS_DC); #else extern zend_object *generated_pool_php; +extern zend_object *internal_generated_pool_php; void descriptor_pool_free(zend_object* object); +void internal_descriptor_pool_free(zend_object* object); #endif -extern DescriptorPool* generated_pool; // The actual generated pool +extern InternalDescriptorPool* generated_pool; // The actual generated pool PHP_PROTO_WRAP_OBJECT_START(Descriptor) const upb_msgdef* msgdef; @@ -487,6 +508,12 @@ PHP_PROTO_WRAP_OBJECT_START(Descriptor) const upb_handlers* json_serialize_handlers_preserve; PHP_PROTO_WRAP_OBJECT_END +PHP_METHOD(Descriptor, getFullName); +PHP_METHOD(Descriptor, getField); +PHP_METHOD(Descriptor, getFieldCount); +PHP_METHOD(Descriptor, getOneofDecl); +PHP_METHOD(Descriptor, getOneofDeclCount); + extern zend_class_entry* descriptor_type; void descriptor_name_set(Descriptor *desc, const char *name); @@ -495,14 +522,36 @@ PHP_PROTO_WRAP_OBJECT_START(FieldDescriptor) const upb_fielddef* fielddef; PHP_PROTO_WRAP_OBJECT_END +PHP_METHOD(FieldDescriptor, getName); +PHP_METHOD(FieldDescriptor, getNumber); +PHP_METHOD(FieldDescriptor, getLabel); +PHP_METHOD(FieldDescriptor, getType); +PHP_METHOD(FieldDescriptor, isMap); +PHP_METHOD(FieldDescriptor, getEnumType); +PHP_METHOD(FieldDescriptor, getMessageType); + +extern zend_class_entry* field_descriptor_type; + PHP_PROTO_WRAP_OBJECT_START(EnumDescriptor) const upb_enumdef* enumdef; zend_class_entry* klass; // begins as NULL - // VALUE module; // begins as nil PHP_PROTO_WRAP_OBJECT_END +PHP_METHOD(EnumDescriptor, getValue); +PHP_METHOD(EnumDescriptor, getValueCount); + extern zend_class_entry* enum_descriptor_type; +PHP_PROTO_WRAP_OBJECT_START(EnumValueDescriptor) + const char* name; + int32_t number; +PHP_PROTO_WRAP_OBJECT_END + +PHP_METHOD(EnumValueDescriptor, getName); +PHP_METHOD(EnumValueDescriptor, getNumber); + +extern zend_class_entry* enum_value_descriptor_type; + // ----------------------------------------------------------------------------- // Message class creation. // ----------------------------------------------------------------------------- @@ -819,6 +868,12 @@ PHP_PROTO_WRAP_OBJECT_START(Oneof) char value[NATIVE_SLOT_MAX_SIZE]; PHP_PROTO_WRAP_OBJECT_END +PHP_METHOD(Oneof, getName); +PHP_METHOD(Oneof, getField); +PHP_METHOD(Oneof, getFieldCount); + +extern zend_class_entry* oneof_descriptor_type; + // Oneof case slot value to indicate that no oneof case is set. The value `0` is // safe because field numbers are used as case identifiers, and no field can // have a number of 0. diff --git a/php/tests/descriptors_test.php b/php/tests/descriptors_test.php new file mode 100644 index 00000000..c3833c24 --- /dev/null +++ b/php/tests/descriptors_test.php @@ -0,0 +1,243 @@ +getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + $this->assertInstanceOf('\Google\Protobuf\Descriptor', $desc); + + $enumDesc = $pool->getEnumDescriptorByClassName(get_class(new TestDescriptorsEnum())); + $this->assertInstanceOf('\Google\Protobuf\EnumDescriptor', $enumDesc); + } + + public function testDescriptorPoolIncorrectArgs() + { + $pool = DescriptorPool::getGeneratedPool(); + + $desc = $pool->getDescriptorByClassName('NotAClass'); + $this->assertNull($desc); + + $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsEnum())); + $this->assertNull($desc); + + $enumDesc = $pool->getEnumDescriptorByClassName(get_class(new TestDescriptorsMessage())); + $this->assertNull($enumDesc); + } + + ######################################################### + # Test descriptor. + ######################################################### + + public function testDescriptor() + { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + + $this->assertSame('descriptors.TestDescriptorsMessage', $desc->getFullName()); + + $this->assertInstanceOf('\Google\Protobuf\FieldDescriptor', $desc->getField(0)); + $this->assertSame(7, $desc->getFieldCount()); + + $this->assertInstanceOf('\Google\Protobuf\OneofDescriptor', $desc->getOneofDecl(0)); + $this->assertSame(1, $desc->getOneofDeclCount()); + } + + ######################################################### + # Test enum descriptor. + ######################################################### + + public function testEnumDescriptor() + { + // WARNINIG - we need to do this so that TestDescriptorsEnum is registered!!? + new TestDescriptorsMessage(); + + $pool = DescriptorPool::getGeneratedPool(); + + $enumDesc = $pool->getEnumDescriptorByClassName(get_class(new TestDescriptorsEnum())); + + // Build map of enum values + $enumDescMap = []; + for ($i = 0; $i < $enumDesc->getValueCount(); $i++) { + $enumValueDesc = $enumDesc->getValue($i); + $this->assertInstanceOf('\Google\Protobuf\EnumValueDescriptor', $enumValueDesc); + $enumDescMap[$enumValueDesc->getNumber()] = $enumValueDesc->getName(); + } + + $this->assertSame('ZERO', $enumDescMap[0]); + $this->assertSame('ONE', $enumDescMap[1]); + + $this->assertSame(2, $enumDesc->getValueCount()); + } + + ######################################################### + # Test field descriptor. + ######################################################### + + public function testFieldDescriptor() + { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + + $fieldDescMap = $this->buildFieldMap($desc); + + // Optional int field + $fieldDesc = $fieldDescMap[1]; + $this->assertSame('optional_int32', $fieldDesc->getName()); + $this->assertSame(1, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_OPTIONAL, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_INT32, $fieldDesc->getType()); + $this->assertFalse($fieldDesc->isMap()); + + // Optional enum field + $fieldDesc = $fieldDescMap[16]; + $this->assertSame('optional_enum', $fieldDesc->getName()); + $this->assertSame(16, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_OPTIONAL, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_ENUM, $fieldDesc->getType()); + $this->assertInstanceOf('\Google\Protobuf\EnumDescriptor', $fieldDesc->getEnumType()); + $this->assertFalse($fieldDesc->isMap()); + + // Optional message field + $fieldDesc = $fieldDescMap[17]; + $this->assertSame('optional_message', $fieldDesc->getName()); + $this->assertSame(17, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_OPTIONAL, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_MESSAGE, $fieldDesc->getType()); + $this->assertInstanceOf('\Google\Protobuf\Descriptor', $fieldDesc->getMessageType()); + $this->assertFalse($fieldDesc->isMap()); + + // Repeated int field + $fieldDesc = $fieldDescMap[31]; + $this->assertSame('repeated_int32', $fieldDesc->getName()); + $this->assertSame(31, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_REPEATED, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_INT32, $fieldDesc->getType()); + $this->assertFalse($fieldDesc->isMap()); + + // Repeated message field + $fieldDesc = $fieldDescMap[47]; + $this->assertSame('repeated_message', $fieldDesc->getName()); + $this->assertSame(47, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_REPEATED, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_MESSAGE, $fieldDesc->getType()); + $this->assertInstanceOf('\Google\Protobuf\Descriptor', $fieldDesc->getMessageType()); + $this->assertFalse($fieldDesc->isMap()); + + // Oneof int field + // Tested further in testOneofDescriptor() + $fieldDesc = $fieldDescMap[51]; + $this->assertSame('oneof_int32', $fieldDesc->getName()); + $this->assertSame(51, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_OPTIONAL, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_INT32, $fieldDesc->getType()); + $this->assertFalse($fieldDesc->isMap()); + + // Map int-enum field + $fieldDesc = $fieldDescMap[71]; + $this->assertSame('map_int32_enum', $fieldDesc->getName()); + $this->assertSame(71, $fieldDesc->getNumber()); + $this->assertSame(self::GPBLABEL_REPEATED, $fieldDesc->getLabel()); + $this->assertSame(self::GPBTYPE_MESSAGE, $fieldDesc->getType()); + $this->assertTrue($fieldDesc->isMap()); + $mapDesc = $fieldDesc->getMessageType(); + $this->assertSame('descriptors.TestDescriptorsMessage.MapInt32EnumEntry', $mapDesc->getFullName()); + $this->assertSame(self::GPBTYPE_INT32, $mapDesc->getField(0)->getType()); + $this->assertSame(self::GPBTYPE_ENUM, $mapDesc->getField(1)->getType()); + } + + /** + * @expectedException \Exception + */ + public function testFieldDescriptorEnumException() + { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + $fieldDesc = $desc->getField(0); + $fieldDesc->getEnumType(); + } + + /** + * @expectedException \Exception + */ + public function testFieldDescriptorMessageException() + { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + $fieldDesc = $desc->getField(0); + $fieldDesc->getMessageType(); + } + + ######################################################### + # Test oneof descriptor. + ######################################################### + + public function testOneofDescriptor() + { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + + $fieldDescMap = $this->buildFieldMap($desc); + $fieldDesc = $fieldDescMap[51]; + + $oneofDesc = $desc->getOneofDecl(0); + + $this->assertSame('my_oneof', $oneofDesc->getName()); + $fieldDescFromOneof = $oneofDesc->getField(0); + $this->assertSame($fieldDesc, $fieldDescFromOneof); + $this->assertSame(1, $oneofDesc->getFieldCount()); + } + + private function buildFieldMap($desc) + { + $fieldDescMap = []; + for ($i = 0; $i < $desc->getFieldCount(); $i++) { + $fieldDesc = $desc->getField($i); + $fieldDescMap[$fieldDesc->getNumber()] = $fieldDesc; + } + return $fieldDescMap; + } +} diff --git a/php/tests/proto/test_descriptors.proto b/php/tests/proto/test_descriptors.proto new file mode 100644 index 00000000..d42aec7c --- /dev/null +++ b/php/tests/proto/test_descriptors.proto @@ -0,0 +1,35 @@ +syntax = "proto3"; + +package descriptors; + +message TestDescriptorsMessage { + int32 optional_int32 = 1; + TestDescriptorsEnum optional_enum = 16; + Sub optional_message = 17; + + // Repeated + repeated int32 repeated_int32 = 31; + repeated Sub repeated_message = 47; + + oneof my_oneof { + int32 oneof_int32 = 51; + } + + map map_int32_enum = 71; + + message Sub { + int32 a = 1; + repeated int32 b = 2; + } + + enum EnumSub { + ZERO = 0; + ONE = 1; + } +} + +enum TestDescriptorsEnum { + ZERO = 0; + ONE = 1; +} + diff --git a/php/tests/test.sh b/php/tests/test.sh index b640c143..c35372d3 100755 --- a/php/tests/test.sh +++ b/php/tests/test.sh @@ -8,7 +8,7 @@ set -e phpize && ./configure CFLAGS='-g -O0' && make popd -tests=( array_test.php encode_decode_test.php generated_class_test.php generated_phpdoc_test.php map_field_test.php well_known_test.php generated_service_test.php ) +tests=( array_test.php encode_decode_test.php generated_class_test.php generated_phpdoc_test.php map_field_test.php well_known_test.php generated_service_test.php descriptors_test.php ) for t in "${tests[@]}" do diff --git a/tests.sh b/tests.sh index fa4a3479..cfc08c35 100755 --- a/tests.sh +++ b/tests.sh @@ -347,7 +347,16 @@ generate_php_test_proto() { # Generate test file rm -rf generated mkdir generated - ../../src/protoc --php_out=generated proto/test.proto proto/test_include.proto proto/test_no_namespace.proto proto/test_prefix.proto proto/test_php_namespace.proto proto/test_empty_php_namespace.proto proto/test_service.proto proto/test_service_namespace.proto + ../../src/protoc --php_out=generated \ + proto/test.proto \ + proto/test_include.proto \ + proto/test_no_namespace.proto \ + proto/test_prefix.proto \ + proto/test_php_namespace.proto \ + proto/test_empty_php_namespace.proto \ + proto/test_service.proto \ + proto/test_service_namespace.proto \ + proto/test_descriptors.proto pushd ../../src ./protoc --php_out=../php/tests/generated google/protobuf/empty.proto ./protoc --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto -- cgit v1.2.3 From 9df89ccabcf8ad7d634009a00faf0e9ba153bdb7 Mon Sep 17 00:00:00 2001 From: Ryan Gordon Date: Wed, 2 Aug 2017 07:43:27 -0700 Subject: Fixing HHVM Compatibility (#3437) --- php/src/Google/Protobuf/Internal/Message.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/php/src/Google/Protobuf/Internal/Message.php b/php/src/Google/Protobuf/Internal/Message.php index 1ecd4fa2..e1009f2f 100644 --- a/php/src/Google/Protobuf/Internal/Message.php +++ b/php/src/Google/Protobuf/Internal/Message.php @@ -841,7 +841,6 @@ class Message if (is_null($value)) { continue; } - $getter = $field->getGetter(); $key_field = $field->getMessageType()->getFieldByNumber(1); $value_field = $field->getMessageType()->getFieldByNumber(2); foreach ($value as $tmp_key => $tmp_value) { @@ -858,13 +857,12 @@ class Message $this->convertJsonValueToProtoValue( $tmp_value, $value_field); - $this->$getter()[$proto_key] = $proto_value; + self::kvUpdateHelper($field, $proto_key, $proto_value); } } else if ($field->isRepeated()) { if (is_null($value)) { continue; } - $getter = $field->getGetter(); foreach ($value as $tmp) { if (is_null($tmp)) { throw new \Exception( @@ -872,7 +870,7 @@ class Message } $proto_value = $this->convertJsonValueToProtoValue($tmp, $field); - $this->$getter()[] = $proto_value; + self::appendHelper($field, $proto_value); } } else { $setter = $field->getSetter(); -- cgit v1.2.3 From 25672c175792f707c71c9aa9fcd29cab31c757fa Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Wed, 2 Aug 2017 18:33:25 -0700 Subject: Add getClass for php Descriptor in c extension (#3443) --- php/ext/google/protobuf/def.c | 11 +++++++++++ php/ext/google/protobuf/protobuf.h | 1 + php/tests/descriptors_test.php | 5 ++++- 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index ae17455c..f885c145 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -184,6 +184,7 @@ void gpb_type_init(TSRMLS_D) { // ----------------------------------------------------------------------------- static zend_function_entry descriptor_methods[] = { + PHP_ME(Descriptor, getClass, NULL, ZEND_ACC_PUBLIC) PHP_ME(Descriptor, getFullName, NULL, ZEND_ACC_PUBLIC) PHP_ME(Descriptor, getField, NULL, ZEND_ACC_PUBLIC) PHP_ME(Descriptor, getFieldCount, NULL, ZEND_ACC_PUBLIC) @@ -234,6 +235,16 @@ static void descriptor_init_c_instance(Descriptor *desc TSRMLS_DC) { desc->json_serialize_handlers_preserve = NULL; } +PHP_METHOD(Descriptor, getClass) { + Descriptor *intern = UNBOX(Descriptor, getThis()); +#if PHP_MAJOR_VERSION < 7 + const char* classname = intern->klass->name; +#else + const char* classname = ZSTR_VAL(intern->klass->name); +#endif + PHP_PROTO_RETVAL_STRINGL(classname, strlen(classname), 1); +} + PHP_METHOD(Descriptor, getFullName) { Descriptor *intern = UNBOX(Descriptor, getThis()); const char* fullname = upb_msgdef_fullname(intern->msgdef); diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 1f79fe8c..44a4155f 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -508,6 +508,7 @@ PHP_PROTO_WRAP_OBJECT_START(Descriptor) const upb_handlers* json_serialize_handlers_preserve; PHP_PROTO_WRAP_OBJECT_END +PHP_METHOD(Descriptor, getClass); PHP_METHOD(Descriptor, getFullName); PHP_METHOD(Descriptor, getField); PHP_METHOD(Descriptor, getFieldCount); diff --git a/php/tests/descriptors_test.php b/php/tests/descriptors_test.php index c3833c24..17e8a4f2 100644 --- a/php/tests/descriptors_test.php +++ b/php/tests/descriptors_test.php @@ -75,9 +75,12 @@ class DescriptorsTest extends TestBase public function testDescriptor() { $pool = DescriptorPool::getGeneratedPool(); - $desc = $pool->getDescriptorByClassName(get_class(new TestDescriptorsMessage())); + $class = get_class(new TestDescriptorsMessage()); + $this->assertSame('Descriptors\TestDescriptorsMessage', $class); + $desc = $pool->getDescriptorByClassName($class); $this->assertSame('descriptors.TestDescriptorsMessage', $desc->getFullName()); + $this->assertSame($class, $desc->getClass()); $this->assertInstanceOf('\Google\Protobuf\FieldDescriptor', $desc->getField(0)); $this->assertSame(7, $desc->getFieldCount()); -- cgit v1.2.3 From a3e17523b4d7c0a4b73f8a17ddae7bf6c101d167 Mon Sep 17 00:00:00 2001 From: Giorgio Azzinnaro Date: Fri, 4 Aug 2017 21:19:36 +0200 Subject: Update third party addons with ProfaneDB I added my project ProfaneDB, it is a database for Protocol Buffers objects. Written in C++, it uses gRPC as an interface for other languages. It is still work in progress, but I'd love to get some feedback on it while I progress! --- docs/third_party.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/third_party.md b/docs/third_party.md index 4c9ffef3..385650df 100644 --- a/docs/third_party.md +++ b/docs/third_party.md @@ -156,3 +156,4 @@ There are miscellaneous other things you may find useful as a Protocol Buffers d * [Linter for .proto files](https://github.com/ckaznocha/protoc-gen-lint) * [Protocol Buffers Dynamic Schema - create protobuf schemas programmatically (Java)] (https://github.com/os72/protobuf-dynamic) * [Make protoc plugins in NodeJS](https://github.com/konsumer/node-protoc-plugin) +* [ProfaneDB - A Protocol Buffers database](https://profanedb.gitlab.io) -- cgit v1.2.3 From 21b0e5587c01948927ede9be789671ff116b7ad4 Mon Sep 17 00:00:00 2001 From: michaelbausor Date: Fri, 4 Aug 2017 16:35:22 -0700 Subject: Update PHP descriptors (#3391) * Add descriptors test * Update descriptors tests * Add public descriptors * Add test_desriptors.proto to test script * Update composer files * Remove references to GPBType, update tests to be compatible with c * Update for c extension compatibility * Remove nested enums for descriptor, update tests * Strip leading '.' from descriptor name * Update tests with test for getClass, fix OneofDescriptor * Add new files to Makefile.am --- Makefile.am | 9 +- composer.json | 4 +- php/composer.json | 8 +- php/phpunit.xml | 1 + php/src/Google/Protobuf/Descriptor.php | 100 ++++++++++++++++++ php/src/Google/Protobuf/DescriptorPool.php | 76 +++++++++++++ php/src/Google/Protobuf/EnumDescriptor.php | 79 ++++++++++++++ php/src/Google/Protobuf/EnumValueDescriptor.php | 64 +++++++++++ php/src/Google/Protobuf/FieldDescriptor.php | 117 +++++++++++++++++++++ php/src/Google/Protobuf/Internal/Descriptor.php | 21 +++- .../Protobuf/Internal/EnumBuilderContext.php | 4 +- .../Google/Protobuf/Internal/EnumDescriptor.php | 20 ++++ .../Protobuf/Internal/EnumValueDescriptor.php | 59 ----------- .../Google/Protobuf/Internal/FieldDescriptor.php | 6 ++ .../Protobuf/Internal/GetPublicDescriptorTrait.php | 41 ++++++++ .../Protobuf/Internal/HasPublicDescriptorTrait.php | 43 ++++++++ .../Google/Protobuf/Internal/OneofDescriptor.php | 15 ++- php/src/Google/Protobuf/OneofDescriptor.php | 75 +++++++++++++ 18 files changed, 669 insertions(+), 73 deletions(-) create mode 100644 php/src/Google/Protobuf/Descriptor.php create mode 100644 php/src/Google/Protobuf/DescriptorPool.php create mode 100644 php/src/Google/Protobuf/EnumDescriptor.php create mode 100644 php/src/Google/Protobuf/EnumValueDescriptor.php create mode 100644 php/src/Google/Protobuf/FieldDescriptor.php delete mode 100644 php/src/Google/Protobuf/Internal/EnumValueDescriptor.php create mode 100644 php/src/Google/Protobuf/Internal/GetPublicDescriptorTrait.php create mode 100644 php/src/Google/Protobuf/Internal/HasPublicDescriptorTrait.php create mode 100644 php/src/Google/Protobuf/OneofDescriptor.php diff --git a/Makefile.am b/Makefile.am index 6279b2de..fafe1d25 100644 --- a/Makefile.am +++ b/Makefile.am @@ -597,6 +597,12 @@ php_EXTRA_DIST= \ php/ext/google/protobuf/upb.c \ php/ext/google/protobuf/protobuf.c \ php/src/phpdoc.dist.xml \ + php/src/Google/Protobuf/Descriptor.php \ + php/src/Google/Protobuf/DescriptorPool.php \ + php/src/Google/Protobuf/EnumDescriptor.php \ + php/src/Google/Protobuf/EnumValueDescriptor.php \ + php/src/Google/Protobuf/FieldDescriptor.php \ + php/src/Google/Protobuf/OneofDescriptor.php \ php/src/Google/Protobuf/Internal/CodedInputStream.php \ php/src/Google/Protobuf/Internal/CodedOutputStream.php \ php/src/Google/Protobuf/Internal/DescriptorPool.php \ @@ -609,7 +615,6 @@ php_EXTRA_DIST= \ php/src/Google/Protobuf/Internal/EnumDescriptorProto.php \ php/src/Google/Protobuf/Internal/EnumOptions.php \ php/src/Google/Protobuf/Internal/EnumValueDescriptorProto.php \ - php/src/Google/Protobuf/Internal/EnumValueDescriptor.php \ php/src/Google/Protobuf/Internal/EnumValueOptions.php \ php/src/Google/Protobuf/Internal/FieldDescriptorProto_Label.php \ php/src/Google/Protobuf/Internal/FieldDescriptorProto.php \ @@ -625,6 +630,7 @@ php_EXTRA_DIST= \ php/src/Google/Protobuf/Internal/FileOptions.php \ php/src/Google/Protobuf/Internal/GeneratedCodeInfo_Annotation.php \ php/src/Google/Protobuf/Internal/GeneratedCodeInfo.php \ + php/src/Google/Protobuf/Internal/GetPublicDescriptorTrait.php \ php/src/Google/Protobuf/Internal/GPBDecodeException.php \ php/src/Google/Protobuf/Internal/GPBJsonWire.php \ php/src/Google/Protobuf/Internal/GPBLabel.php \ @@ -632,6 +638,7 @@ php_EXTRA_DIST= \ php/src/Google/Protobuf/Internal/GPBUtil.php \ php/src/Google/Protobuf/Internal/GPBWireType.php \ php/src/Google/Protobuf/Internal/GPBWire.php \ + php/src/Google/Protobuf/Internal/HasPublicDescriptorTrait.php \ php/src/Google/Protobuf/Internal/MapEntry.php \ php/src/Google/Protobuf/Internal/MapFieldIter.php \ php/src/Google/Protobuf/Internal/MapField.php \ diff --git a/composer.json b/composer.json index 80d90eab..2c64ad22 100644 --- a/composer.json +++ b/composer.json @@ -16,8 +16,8 @@ }, "autoload": { "psr-4": { - "Google\\Protobuf\\Internal\\": "php/src/Google/Protobuf/Internal", - "GPBMetadata\\Google\\Protobuf\\Internal\\": "php/src/GPBMetadata/Google/Protobuf/Internal" + "Google\\Protobuf\\": "php/src/Google/Protobuf", + "GPBMetadata\\Google\\Protobuf\\": "php/src/GPBMetadata/Google/Protobuf" } } } diff --git a/php/composer.json b/php/composer.json index 724a45dd..34e0447c 100644 --- a/php/composer.json +++ b/php/composer.json @@ -13,12 +13,8 @@ }, "autoload": { "psr-4": { - "Foo\\": "tests/generated/Foo", - "Bar\\": "tests/generated/Bar", - "Google\\Protobuf\\": "tests/generated/Google/Protobuf", - "Google\\Protobuf\\Internal\\": "src/Google/Protobuf/Internal", - "GPBMetadata\\": "tests/generated/GPBMetadata", - "GPBMetadata\\Google\\Protobuf\\Internal\\": "src/GPBMetadata/Google/Protobuf/Internal", + "Google\\Protobuf\\": "src/Google/Protobuf", + "GPBMetadata\\Google\\Protobuf\\": "src/GPBMetadata/Google/Protobuf", "": "tests/generated" } } diff --git a/php/phpunit.xml b/php/phpunit.xml index 637467be..d7077038 100644 --- a/php/phpunit.xml +++ b/php/phpunit.xml @@ -10,6 +10,7 @@ tests/generated_phpdoc_test.php tests/map_field_test.php tests/well_known_test.php + tests/descriptors_test.php tests/generated_service_test.php diff --git a/php/src/Google/Protobuf/Descriptor.php b/php/src/Google/Protobuf/Descriptor.php new file mode 100644 index 00000000..986b81e1 --- /dev/null +++ b/php/src/Google/Protobuf/Descriptor.php @@ -0,0 +1,100 @@ +internal_desc = $internal_desc; + } + + /** + * @return string Full protobuf message name + */ + public function getFullName() + { + return trim($this->internal_desc->getFullName(), "."); + } + + /** + * @return string PHP class name + */ + public function getClass() + { + return $this->internal_desc->getClass(); + } + + /** + * @param int $index Must be >= 0 and < getFieldCount() + * @return FieldDescriptor + */ + public function getField($index) + { + return $this->getPublicDescriptor($this->internal_desc->getFieldByIndex($index)); + } + + /** + * @return int Number of fields in message + */ + public function getFieldCount() + { + return count($this->internal_desc->getField()); + } + + /** + * @param int $index Must be >= 0 and < getOneofDeclCount() + * @return OneofDescriptor + */ + public function getOneofDecl($index) + { + return $this->getPublicDescriptor($this->internal_desc->getOneofDecl()[$index]); + } + + /** + * @return int Number of oneofs in message + */ + public function getOneofDeclCount() + { + return count($this->internal_desc->getOneofDecl()); + } +} diff --git a/php/src/Google/Protobuf/DescriptorPool.php b/php/src/Google/Protobuf/DescriptorPool.php new file mode 100644 index 00000000..119f0e2e --- /dev/null +++ b/php/src/Google/Protobuf/DescriptorPool.php @@ -0,0 +1,76 @@ +internal_pool = $internal_pool; + } + + /** + * @param string $className A fully qualified protobuf class name + * @return Descriptor + */ + public function getDescriptorByClassName($className) + { + $desc = $this->internal_pool->getDescriptorByClassName($className); + return is_null($desc) ? null : $desc->getPublicDescriptor(); + } + + /** + * @param string $className A fully qualified protobuf class name + * @return EnumDescriptor + */ + public function getEnumDescriptorByClassName($className) + { + $desc = $this->internal_pool->getEnumDescriptorByClassName($className); + return is_null($desc) ? null : $desc->getPublicDescriptor(); + } +} diff --git a/php/src/Google/Protobuf/EnumDescriptor.php b/php/src/Google/Protobuf/EnumDescriptor.php new file mode 100644 index 00000000..a8b56c0d --- /dev/null +++ b/php/src/Google/Protobuf/EnumDescriptor.php @@ -0,0 +1,79 @@ +internal_desc = $internal_desc; + } + + /** + * @return string Full protobuf message name + */ + public function getFullName() + { + return $this->internal_desc->getFullName(); + } + + /** + * @return string PHP class name + */ + public function getClass() + { + return $this->internal_desc->getClass(); + } + + /** + * @param int $index Must be >= 0 and < getValueCount() + * @return EnumValueDescriptor + */ + public function getValue($index) + { + return $this->internal_desc->getValueDescriptorByIndex($index); + } + + /** + * @return int Number of values in enum + */ + public function getValueCount() + { + return $this->internal_desc->getValueCount(); + } +} diff --git a/php/src/Google/Protobuf/EnumValueDescriptor.php b/php/src/Google/Protobuf/EnumValueDescriptor.php new file mode 100644 index 00000000..e76e1997 --- /dev/null +++ b/php/src/Google/Protobuf/EnumValueDescriptor.php @@ -0,0 +1,64 @@ +name = $name; + $this->number = $number; + } + + /** + * @return string + */ + public function getName() + { + return $this->name; + } + + /** + * @return int + */ + public function getNumber() + { + return $this->number; + } +} diff --git a/php/src/Google/Protobuf/FieldDescriptor.php b/php/src/Google/Protobuf/FieldDescriptor.php new file mode 100644 index 00000000..ac9271f9 --- /dev/null +++ b/php/src/Google/Protobuf/FieldDescriptor.php @@ -0,0 +1,117 @@ +internal_desc = $internal_desc; + } + + /** + * @return string Field name + */ + public function getName() + { + return $this->internal_desc->getName(); + } + + /** + * @return int Protobuf field number + */ + public function getNumber() + { + return $this->internal_desc->getNumber(); + } + + /** + * @return int + */ + public function getLabel() + { + return $this->internal_desc->getLabel(); + } + + /** + * @return int + */ + public function getType() + { + return $this->internal_desc->getType(); + } + + /** + * @return Descriptor Returns a descriptor for the field type if the field type is a message, otherwise throws \Exception + * @throws \Exception + */ + public function getMessageType() + { + if ($this->getType() == GPBType::MESSAGE) { + return $this->getPublicDescriptor($this->internal_desc->getMessageType()); + } else { + throw new \Exception("Cannot get message type for non-message field '" . $this->getName() . "'"); + } + } + + /** + * @return EnumDescriptor Returns an enum descriptor if the field type is an enum, otherwise throws \Exception + * @throws \Exception + */ + public function getEnumType() + { + if ($this->getType() == GPBType::ENUM) { + return $this->getPublicDescriptor($this->internal_desc->getEnumType()); + } else { + throw new \Exception("Cannot get enum type for non-enum field '" . $this->getName() . "'"); + } + } + + /** + * @return boolean + */ + public function isMap() + { + return $this->internal_desc->isMap(); + } +} diff --git a/php/src/Google/Protobuf/Internal/Descriptor.php b/php/src/Google/Protobuf/Internal/Descriptor.php index 44225ad2..ee3a8bde 100644 --- a/php/src/Google/Protobuf/Internal/Descriptor.php +++ b/php/src/Google/Protobuf/Internal/Descriptor.php @@ -34,17 +34,24 @@ namespace Google\Protobuf\Internal; class Descriptor { + use HasPublicDescriptorTrait; private $full_name; private $field = []; private $json_to_field = []; private $name_to_field = []; + private $index_to_field = []; private $nested_type = []; private $enum_type = []; private $klass; private $options; private $oneof_decl = []; + public function __construct() + { + $this->public_desc = new \Google\Protobuf\Descriptor($this); + } + public function addOneofDecl($oneof) { $this->oneof_decl[] = $oneof; @@ -70,6 +77,7 @@ class Descriptor $this->field[$field->getNumber()] = $field; $this->json_to_field[$field->getJsonName()] = $field; $this->name_to_field[$field->getName()] = $field; + $this->index_to_field[] = $field; } public function getField() @@ -124,6 +132,15 @@ class Descriptor } } + public function getFieldByIndex($index) + { + if (count($this->index_to_field) <= $index) { + return NULL; + } else { + return $this->index_to_field[$index]; + } + } + public function setClass($klass) { $this->klass = $klass; @@ -179,9 +196,11 @@ class Descriptor } // Handle oneof fields. + $index = 0; foreach ($proto->getOneofDecl() as $oneof_proto) { $desc->addOneofDecl( - OneofDescriptor::buildFromProto($oneof_proto, $desc)); + OneofDescriptor::buildFromProto($oneof_proto, $desc, $index)); + $index++; } return $desc; diff --git a/php/src/Google/Protobuf/Internal/EnumBuilderContext.php b/php/src/Google/Protobuf/Internal/EnumBuilderContext.php index c1dac24d..08397284 100644 --- a/php/src/Google/Protobuf/Internal/EnumBuilderContext.php +++ b/php/src/Google/Protobuf/Internal/EnumBuilderContext.php @@ -33,7 +33,7 @@ namespace Google\Protobuf\Internal; use Google\Protobuf\Internal\EnumDescriptor; -use Google\Protobuf\Internal\EnumValueDescriptor; +use Google\Protobuf\EnumValueDescriptor; class EnumBuilderContext { @@ -51,7 +51,7 @@ class EnumBuilderContext public function value($name, $number) { - $value = new EnumValueDescriptor(); + $value = new EnumValueDescriptor($name, $number); $this->descriptor->addValue($number, $value); return $this; } diff --git a/php/src/Google/Protobuf/Internal/EnumDescriptor.php b/php/src/Google/Protobuf/Internal/EnumDescriptor.php index 33a55a4a..01649fec 100644 --- a/php/src/Google/Protobuf/Internal/EnumDescriptor.php +++ b/php/src/Google/Protobuf/Internal/EnumDescriptor.php @@ -2,13 +2,22 @@ namespace Google\Protobuf\Internal; +use Google\Protobuf\EnumValueDescriptor; + class EnumDescriptor { + use HasPublicDescriptorTrait; private $klass; private $full_name; private $value; private $name_to_value; + private $value_descriptor = []; + + public function __construct() + { + $this->public_desc = new \Google\Protobuf\EnumDescriptor($this); + } public function setFullName($full_name) { @@ -24,6 +33,7 @@ class EnumDescriptor { $this->value[$number] = $value; $this->name_to_value[$value->getName()] = $value; + $this->value_descriptor[] = new EnumValueDescriptor($value->getName(), $number); } public function getValueByNumber($number) @@ -36,6 +46,16 @@ class EnumDescriptor return $this->name_to_value[$name]; } + public function getValueDescriptorByIndex($index) + { + return $this->value_descriptor[$index]; + } + + public function getValueCount() + { + return count($this->value); + } + public function setClass($klass) { $this->klass = $klass; diff --git a/php/src/Google/Protobuf/Internal/EnumValueDescriptor.php b/php/src/Google/Protobuf/Internal/EnumValueDescriptor.php deleted file mode 100644 index 549766e3..00000000 --- a/php/src/Google/Protobuf/Internal/EnumValueDescriptor.php +++ /dev/null @@ -1,59 +0,0 @@ -name = $name; - } - - public function getName() - { - return $this->name; - } - - public function setNumber($number) - { - $this->number = $number; - } - - public function getNumber() - { - return $this->number; - } -} diff --git a/php/src/Google/Protobuf/Internal/FieldDescriptor.php b/php/src/Google/Protobuf/Internal/FieldDescriptor.php index f18bf810..1443c6fd 100644 --- a/php/src/Google/Protobuf/Internal/FieldDescriptor.php +++ b/php/src/Google/Protobuf/Internal/FieldDescriptor.php @@ -34,6 +34,7 @@ namespace Google\Protobuf\Internal; class FieldDescriptor { + use HasPublicDescriptorTrait; private $name; private $json_name; @@ -48,6 +49,11 @@ class FieldDescriptor private $is_map; private $oneof_index = -1; + public function __construct() + { + $this->public_desc = new \Google\Protobuf\FieldDescriptor($this); + } + public function setOneofIndex($index) { $this->oneof_index = $index; diff --git a/php/src/Google/Protobuf/Internal/GetPublicDescriptorTrait.php b/php/src/Google/Protobuf/Internal/GetPublicDescriptorTrait.php new file mode 100644 index 00000000..d22bc305 --- /dev/null +++ b/php/src/Google/Protobuf/Internal/GetPublicDescriptorTrait.php @@ -0,0 +1,41 @@ +getPublicDescriptor(); + } +} diff --git a/php/src/Google/Protobuf/Internal/HasPublicDescriptorTrait.php b/php/src/Google/Protobuf/Internal/HasPublicDescriptorTrait.php new file mode 100644 index 00000000..ed5d1660 --- /dev/null +++ b/php/src/Google/Protobuf/Internal/HasPublicDescriptorTrait.php @@ -0,0 +1,43 @@ +public_desc; + } +} diff --git a/php/src/Google/Protobuf/Internal/OneofDescriptor.php b/php/src/Google/Protobuf/Internal/OneofDescriptor.php index 57961f39..67b107f6 100644 --- a/php/src/Google/Protobuf/Internal/OneofDescriptor.php +++ b/php/src/Google/Protobuf/Internal/OneofDescriptor.php @@ -34,10 +34,16 @@ namespace Google\Protobuf\Internal; class OneofDescriptor { + use HasPublicDescriptorTrait; private $name; private $fields; + public function __construct() + { + $this->public_desc = new \Google\Protobuf\OneofDescriptor($this); + } + public function setName($name) { $this->name = $name; @@ -48,7 +54,7 @@ class OneofDescriptor return $this->name; } - public function addField(Descriptor $field) + public function addField(FieldDescriptor $field) { $this->fields[] = $field; } @@ -58,10 +64,15 @@ class OneofDescriptor return $this->fields; } - public static function buildFromProto($oneof_proto) + public static function buildFromProto($oneof_proto, $desc, $index) { $oneof = new OneofDescriptor(); $oneof->setName($oneof_proto->getName()); + foreach ($desc->getField() as $field) { + if ($field->getOneofIndex() == $index) { + $oneof->addField($field); + } + } return $oneof; } } diff --git a/php/src/Google/Protobuf/OneofDescriptor.php b/php/src/Google/Protobuf/OneofDescriptor.php new file mode 100644 index 00000000..d9736634 --- /dev/null +++ b/php/src/Google/Protobuf/OneofDescriptor.php @@ -0,0 +1,75 @@ +internal_desc = $internal_desc; + } + + /** + * @return string The name of the oneof + */ + public function getName() + { + return $this->internal_desc->getName(); + } + + /** + * @param int $index Must be >= 0 and < getFieldCount() + * @return FieldDescriptor + */ + public function getField($index) + { + return $this->getPublicDescriptor($this->internal_desc->getFields()[$index]); + } + + /** + * @return int Number of fields in the oneof + */ + public function getFieldCount() + { + return count($this->internal_desc->getFields()); + } +} -- cgit v1.2.3 From 49b44bff2b6257a119f9c6a342d6151c736586b8 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Fri, 4 Aug 2017 16:35:49 -0700 Subject: Fix the bug in php c extension that setting one field can change another field's value. (#3455) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix the bug in php c extension that setting one field can change another field's value. The reason is that previously, in c extension, it was assumed that the order that fields were declared in php is the same as the order of fields in upb. This is not true. Now, for every field in upb, we will look up the actual property that is corresponding to the upb field. * Cleanup pull request * Fix indentation * Port to php5 * Port with php7.1 * Port to zts --- php/ext/google/protobuf/encode_decode.c | 4 +- php/ext/google/protobuf/message.c | 10 ++-- php/ext/google/protobuf/protobuf.h | 4 +- php/ext/google/protobuf/storage.c | 91 ++++++++++++++++++++++++--------- php/tests/generated_class_test.php | 13 +++++ php/tests/memory_leak_test.php | 1 + php/tests/proto/test.proto | 5 ++ 7 files changed, 95 insertions(+), 33 deletions(-) diff --git a/php/ext/google/protobuf/encode_decode.c b/php/ext/google/protobuf/encode_decode.c index 4cb8997d..89e75d6a 100644 --- a/php/ext/google/protobuf/encode_decode.c +++ b/php/ext/google/protobuf/encode_decode.c @@ -716,7 +716,7 @@ static void *oneofbytes_handler(void *closure, DEREF(message_data(msg), oneofdata->case_ofs, uint32_t) = oneofdata->oneof_case_num; DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*) = - &(msg->std.properties_table)[oneofdata->property_ofs]; + OBJ_PROP(&msg->std, oneofdata->property_ofs); return empty_php_string(DEREF( message_data(msg), oneofdata->ofs, CACHED_VALUE*)); @@ -747,7 +747,7 @@ static void* oneofsubmsg_handler(void* closure, const void* hd) { // Create new message. DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*) = - &(msg->std.properties_table)[oneofdata->property_ofs]; + OBJ_PROP(&msg->std, oneofdata->property_ofs); ZVAL_OBJ(CACHED_PTR_TO_ZVAL_PTR( DEREF(message_data(msg), oneofdata->ofs, CACHED_VALUE*)), subklass->create_object(subklass TSRMLS_CC)); diff --git a/php/ext/google/protobuf/message.c b/php/ext/google/protobuf/message.c index 254640c7..519786dd 100644 --- a/php/ext/google/protobuf/message.c +++ b/php/ext/google/protobuf/message.c @@ -172,7 +172,7 @@ static zval* message_get_property(zval* object, zval* member, int type, zend_get_property_info(Z_OBJCE_P(object), member, true TSRMLS_CC); return layout_get( self->descriptor->layout, message_data(self), field, - &Z_OBJ_P(object)->properties_table[property_info->offset] TSRMLS_CC); + OBJ_PROP(Z_OBJ_P(object), property_info->offset) TSRMLS_CC); #else property_info = zend_get_property_info(Z_OBJCE_P(object), Z_STR_P(member), true); @@ -222,7 +222,7 @@ void custom_data_init(const zend_class_entry* ce, // case a collection happens during object creation in layout_init(). intern->descriptor = desc; layout_init(desc->layout, message_data(intern), - intern->std.properties_table PHP_PROTO_TSRMLS_CC); + &intern->std PHP_PROTO_TSRMLS_CC); } void build_class_from_descriptor( @@ -265,8 +265,7 @@ PHP_METHOD(Message, clear) { zend_class_entry* ce = desc->klass; object_properties_init(&msg->std, ce); - layout_init(desc->layout, message_data(msg), - msg->std.properties_table TSRMLS_CC); + layout_init(desc->layout, message_data(msg), &msg->std TSRMLS_CC); } PHP_METHOD(Message, mergeFrom) { @@ -301,7 +300,8 @@ PHP_METHOD(Message, readOneof) { int property_cache_index = msg->descriptor->layout->fields[upb_fielddef_index(field)].cache_index; - zval* property_ptr = OBJ_PROP(Z_OBJ_P(getThis()), property_cache_index); + zval* property_ptr = CACHED_PTR_TO_ZVAL_PTR( + OBJ_PROP(Z_OBJ_P(getThis()), property_cache_index)); // Unlike singular fields, oneof fields share cached property. So we cannot // let lay_get modify the cached property. Instead, we pass in the return diff --git a/php/ext/google/protobuf/protobuf.h b/php/ext/google/protobuf/protobuf.h index 44a4155f..f9e9d229 100644 --- a/php/ext/google/protobuf/protobuf.h +++ b/php/ext/google/protobuf/protobuf.h @@ -151,7 +151,7 @@ #define PHP_PROTO_GLOBAL_UNINITIALIZED_ZVAL EG(uninitialized_zval_ptr) -#define OBJ_PROP(PROPERTIES, OFFSET) (PROPERTIES)->properties_table[OFFSET] +#define OBJ_PROP(OBJECT, OFFSET) &((OBJECT)->properties_table[OFFSET]) #define php_proto_zval_ptr_dtor(zval_ptr) \ zval_ptr_dtor(&(zval_ptr)) @@ -644,7 +644,7 @@ PHP_PROTO_WRAP_OBJECT_END MessageLayout* create_layout(const upb_msgdef* msgdef); void layout_init(MessageLayout* layout, void* storage, - CACHED_VALUE* properties_table PHP_PROTO_TSRMLS_DC); + zend_object* object PHP_PROTO_TSRMLS_DC); zval* layout_get(MessageLayout* layout, const void* storage, const upb_fielddef* field, CACHED_VALUE* cache TSRMLS_DC); void layout_set(MessageLayout* layout, MessageHeader* header, diff --git a/php/ext/google/protobuf/storage.c b/php/ext/google/protobuf/storage.c index e46dbf70..4830e15f 100644 --- a/php/ext/google/protobuf/storage.c +++ b/php/ext/google/protobuf/storage.c @@ -275,9 +275,6 @@ void native_slot_init(upb_fieldtype_t type, void* memory, CACHED_VALUE* cache) { break; case UPB_TYPE_STRING: case UPB_TYPE_BYTES: - DEREF(memory, CACHED_VALUE*) = cache; - ZVAL_EMPTY_STRING(CACHED_PTR_TO_ZVAL_PTR(cache)); - break; case UPB_TYPE_MESSAGE: DEREF(memory, CACHED_VALUE*) = cache; break; @@ -586,6 +583,8 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { upb_msg_oneof_iter oit; size_t off = 0; int i = 0; + TSRMLS_FETCH(); + Descriptor* desc = UNBOX_HASHTABLE_VALUE(Descriptor, get_def_obj(msgdef)); layout->fields = ALLOC_N(MessageField, nfields); @@ -612,7 +611,37 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { layout->fields[upb_fielddef_index(field)].offset = off; layout->fields[upb_fielddef_index(field)].case_offset = MESSAGE_FIELD_NO_CASE; - layout->fields[upb_fielddef_index(field)].cache_index = i++; + + const char* fieldname = upb_fielddef_name(field); + +#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0) + zend_class_entry* old_scope = EG(scope); + EG(scope) = desc->klass; +#else + zend_class_entry* old_scope = EG(fake_scope); + EG(fake_scope) = desc->klass; +#endif + +#if PHP_MAJOR_VERSION < 7 + zval member; + ZVAL_STRINGL(&member, fieldname, strlen(fieldname), 0); + zend_property_info* property_info = + zend_get_property_info(desc->klass, &member, true TSRMLS_CC); +#else + zend_string* member = zend_string_init(fieldname, strlen(fieldname), 1); + zend_property_info* property_info = + zend_get_property_info(desc->klass, member, true); + zend_string_release(member); +#endif + +#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0) + EG(scope) = old_scope; +#else + EG(fake_scope) = old_scope; +#endif + + layout->fields[upb_fielddef_index(field)].cache_index = + property_info->offset; off += field_size; } @@ -640,11 +669,40 @@ MessageLayout* create_layout(const upb_msgdef* msgdef) { // Align the offset . off = align_up_to( off, field_size); // Assign all fields in the oneof this same offset. + const char* oneofname = upb_oneofdef_name(oneof); for (upb_oneof_begin(&fit, oneof); !upb_oneof_done(&fit); upb_oneof_next(&fit)) { const upb_fielddef* field = upb_oneof_iter_field(&fit); layout->fields[upb_fielddef_index(field)].offset = off; - layout->fields[upb_fielddef_index(field)].cache_index = i; + +#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0) + zend_class_entry* old_scope = EG(scope); + EG(scope) = desc->klass; +#else + zend_class_entry* old_scope = EG(fake_scope); + EG(fake_scope) = desc->klass; +#endif + +#if PHP_MAJOR_VERSION < 7 + zval member; + ZVAL_STRINGL(&member, oneofname, strlen(oneofname), 0); + zend_property_info* property_info = + zend_get_property_info(desc->klass, &member, true TSRMLS_CC); +#else + zend_string* member = zend_string_init(oneofname, strlen(oneofname), 1); + zend_property_info* property_info = + zend_get_property_info(desc->klass, member, true); + zend_string_release(member); +#endif + +#if PHP_MAJOR_VERSION < 7 || (PHP_MAJOR_VERSION == 7 && PHP_MINOR_VERSION == 0) + EG(scope) = old_scope; +#else + EG(fake_scope) = old_scope; +#endif + + layout->fields[upb_fielddef_index(field)].cache_index = + property_info->offset; } i++; off += field_size; @@ -683,7 +741,7 @@ void free_layout(MessageLayout* layout) { } void layout_init(MessageLayout* layout, void* storage, - CACHED_VALUE* properties_table PHP_PROTO_TSRMLS_DC) { + zend_object* object PHP_PROTO_TSRMLS_DC) { int i; upb_msg_field_iter it; for (upb_msg_field_begin(&it, layout->msgdef), i = 0; !upb_msg_field_done(&it); @@ -692,22 +750,7 @@ void layout_init(MessageLayout* layout, void* storage, void* memory = slot_memory(layout, storage, field); uint32_t* oneof_case = slot_oneof_case(layout, storage, field); int cache_index = slot_property_cache(layout, storage, field); - CACHED_VALUE* property_ptr = &properties_table[cache_index]; - - // Clean up initial value by generated code. In the generated code of - // previous versions, each php field is given an initial value. However, the - // order to initialize these fields may not be consistent with the order of - // upb fields. - if (Z_TYPE_P(CACHED_PTR_TO_ZVAL_PTR(property_ptr)) == IS_STRING) { -#if PHP_MAJOR_VERSION < 7 - if (!IS_INTERNED(Z_STRVAL_PP(property_ptr))) { - FREE(Z_STRVAL_PP(property_ptr)); - } -#else - zend_string_release(Z_STR_P(property_ptr)); -#endif - } - ZVAL_NULL(CACHED_PTR_TO_ZVAL_PTR(property_ptr)); + CACHED_VALUE* property_ptr = OBJ_PROP(object, cache_index); if (upb_fielddef_containingoneof(field)) { memset(memory, 0, NATIVE_SLOT_MAX_SIZE); @@ -797,7 +840,7 @@ void layout_set(MessageLayout* layout, MessageHeader* header, header->descriptor->layout->fields[upb_fielddef_index(field)] .cache_index; DEREF(memory, CACHED_VALUE*) = - &(header->std.properties_table)[property_cache_index]; + OBJ_PROP(&header->std, property_cache_index); memory = DEREF(memory, CACHED_VALUE*); break; } @@ -964,7 +1007,7 @@ void layout_merge(MessageLayout* layout, MessageHeader* from, int property_cache_index = layout->fields[upb_fielddef_index(field)].cache_index; DEREF(to_memory, CACHED_VALUE*) = - &(to->std.properties_table)[property_cache_index]; + OBJ_PROP(&to->std, property_cache_index); break; } default: diff --git a/php/tests/generated_class_test.php b/php/tests/generated_class_test.php index 56e3be20..86e68683 100644 --- a/php/tests/generated_class_test.php +++ b/php/tests/generated_class_test.php @@ -13,6 +13,7 @@ use Foo\TestIncludeNamespaceMessage; use Foo\TestIncludePrefixMessage; use Foo\TestMessage; use Foo\TestMessage_Sub; +use Foo\TestReverseFieldOrder; use Php\Test\TestNamespace; class GeneratedClassTest extends TestBase @@ -702,4 +703,16 @@ class GeneratedClassTest extends TestBase $this->assertSame(1, $m->getOptionalInt32()); $this->assertSame(2, $m->getOptionalUInt32()); } + + ######################################################### + # Test Reverse Field Order. + ######################################################### + + public function testReverseFieldOrder() + { + $m = new TestReverseFieldOrder(); + $m->setB("abc"); + $this->assertSame("abc", $m->getB()); + $this->assertNotSame("abc", $m->getA()); + } } diff --git a/php/tests/memory_leak_test.php b/php/tests/memory_leak_test.php index faa1833d..a92694d0 100644 --- a/php/tests/memory_leak_test.php +++ b/php/tests/memory_leak_test.php @@ -21,6 +21,7 @@ require_once('generated/Foo/TestMessage_Sub.php'); require_once('generated/Foo/TestPackedMessage.php'); require_once('generated/Foo/TestPhpDoc.php'); require_once('generated/Foo/TestRandomFieldOrder.php'); +require_once('generated/Foo/TestReverseFieldOrder.php'); require_once('generated/Foo/TestUnpackedMessage.php'); require_once('generated/GPBMetadata/Proto/Test.php'); require_once('generated/GPBMetadata/Proto/TestEmptyPhpNamespace.php'); diff --git a/php/tests/proto/test.proto b/php/tests/proto/test.proto index d81f66f5..a90f3d1d 100644 --- a/php/tests/proto/test.proto +++ b/php/tests/proto/test.proto @@ -187,3 +187,8 @@ message TestRandomFieldOrder { int64 tag13 = 150; string tag14 = 160; } + +message TestReverseFieldOrder { + repeated int32 a = 2; + string b = 1; +} -- cgit v1.2.3 From f14703c933d04a4aac285c482bf828269bd0a151 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Fri, 4 Aug 2017 16:42:19 -0700 Subject: Update commit id in Dockerfile to reflect change in #3391 (#3459) --- jenkins/docker/Dockerfile | 2 +- jenkins/docker32/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jenkins/docker/Dockerfile b/jenkins/docker/Dockerfile index 45900262..8faba4c6 100644 --- a/jenkins/docker/Dockerfile +++ b/jenkins/docker/Dockerfile @@ -182,7 +182,7 @@ RUN cd /tmp && \ rm -rf protobuf && \ git clone https://github.com/google/protobuf.git && \ cd protobuf && \ - git reset --hard 8d97b3d8b5a33650e822460b3b561802c969e86e && \ + git reset --hard 49b44bff2b6257a119f9c6a342d6151c736586b8 && \ cd php && \ ln -sfn /usr/local/php-5.5/bin/php /usr/bin/php && \ ln -sfn /usr/local/php-5.5/bin/php-config /usr/bin/php-config && \ diff --git a/jenkins/docker32/Dockerfile b/jenkins/docker32/Dockerfile index 2c9d4165..1278889f 100644 --- a/jenkins/docker32/Dockerfile +++ b/jenkins/docker32/Dockerfile @@ -98,7 +98,7 @@ RUN composer config -g -- secure-http false RUN cd /tmp && \ git clone https://github.com/google/protobuf.git && \ cd protobuf/php && \ - git reset --hard 8d97b3d8b5a33650e822460b3b561802c969e86e && \ + git reset --hard 49b44bff2b6257a119f9c6a342d6151c736586b8 && \ ln -sfn /usr/local/php-5.5/bin/php /usr/bin/php && \ ln -sfn /usr/local/php-5.5/bin/php-config /usr/bin/php-config && \ ln -sfn /usr/local/php-5.5/bin/phpize /usr/bin/phpize && \ -- cgit v1.2.3