From ce044817c7ba0aea27c3fd8e496635d94d20a755 Mon Sep 17 00:00:00 2001 From: Paul Yang Date: Thu, 7 Jun 2018 18:16:44 -0700 Subject: Use legacy name in php runtime (#4741) * Use legacy name in php runtime Old generated code cannot work with new runtime, because the new runtime assumes new class name for nested message. For details see #4738. * Remove unused method --- php/src/Google/Protobuf/Internal/Descriptor.php | 14 +++++++ .../Google/Protobuf/Internal/DescriptorPool.php | 2 + .../Google/Protobuf/Internal/EnumDescriptor.php | 14 +++++++ php/src/Google/Protobuf/Internal/GPBUtil.php | 29 +++++++++++-- php/src/Google/Protobuf/Internal/MapField.php | 25 ++++++++++- php/src/Google/Protobuf/Internal/RepeatedField.php | 23 +++++++++- php/tests/compatibility_test.sh | 49 +++++++++++++++------- 7 files changed, 134 insertions(+), 22 deletions(-) diff --git a/php/src/Google/Protobuf/Internal/Descriptor.php b/php/src/Google/Protobuf/Internal/Descriptor.php index ee3a8bde..21ac5de3 100644 --- a/php/src/Google/Protobuf/Internal/Descriptor.php +++ b/php/src/Google/Protobuf/Internal/Descriptor.php @@ -44,6 +44,7 @@ class Descriptor private $nested_type = []; private $enum_type = []; private $klass; + private $legacy_klass; private $options; private $oneof_decl = []; @@ -151,6 +152,16 @@ class Descriptor return $this->klass; } + public function setLegacyClass($klass) + { + $this->legacy_klass = $klass; + } + + public function getLegacyClass() + { + return $this->legacy_klass; + } + public function setOptions($options) { $this->options = $options; @@ -167,16 +178,19 @@ class Descriptor $message_name_without_package = ""; $classname = ""; + $legacy_classname = ""; $fullname = ""; GPBUtil::getFullClassName( $proto, $containing, $file_proto, $message_name_without_package, + $legacy_classname, $classname, $fullname); $desc->setFullName($fullname); $desc->setClass($classname); + $desc->setLegacyClass($legacy_classname); $desc->setOptions($proto->getOptions()); foreach ($proto->getField() as $field_proto) { diff --git a/php/src/Google/Protobuf/Internal/DescriptorPool.php b/php/src/Google/Protobuf/Internal/DescriptorPool.php index 304c1615..9b4dcc01 100644 --- a/php/src/Google/Protobuf/Internal/DescriptorPool.php +++ b/php/src/Google/Protobuf/Internal/DescriptorPool.php @@ -92,6 +92,7 @@ class DescriptorPool $this->proto_to_class[$descriptor->getFullName()] = $descriptor->getClass(); $this->class_to_desc[$descriptor->getClass()] = $descriptor; + $this->class_to_desc[$descriptor->getLegacyClass()] = $descriptor; foreach ($descriptor->getNestedType() as $nested_type) { $this->addDescriptor($nested_type); } @@ -105,6 +106,7 @@ class DescriptorPool $this->proto_to_class[$descriptor->getFullName()] = $descriptor->getClass(); $this->class_to_enum_desc[$descriptor->getClass()] = $descriptor; + $this->class_to_enum_desc[$descriptor->getLegacyClass()] = $descriptor; } public function getDescriptorByClassName($klass) diff --git a/php/src/Google/Protobuf/Internal/EnumDescriptor.php b/php/src/Google/Protobuf/Internal/EnumDescriptor.php index 01649fec..82a42767 100644 --- a/php/src/Google/Protobuf/Internal/EnumDescriptor.php +++ b/php/src/Google/Protobuf/Internal/EnumDescriptor.php @@ -9,6 +9,7 @@ class EnumDescriptor use HasPublicDescriptorTrait; private $klass; + private $legacy_klass; private $full_name; private $value; private $name_to_value; @@ -66,12 +67,23 @@ class EnumDescriptor return $this->klass; } + public function setLegacyClass($klass) + { + $this->legacy_klass = $klass; + } + + public function getLegacyClass() + { + return $this->legacy_klass; + } + public static function buildFromProto($proto, $file_proto, $containing) { $desc = new EnumDescriptor(); $enum_name_without_package = ""; $classname = ""; + $legacy_classname = ""; $fullname = ""; GPBUtil::getFullClassName( $proto, @@ -79,9 +91,11 @@ class EnumDescriptor $file_proto, $enum_name_without_package, $classname, + $legacy_classname, $fullname); $desc->setFullName($fullname); $desc->setClass($classname); + $desc->setLegacyClass($legacy_classname); $values = $proto->getValue(); foreach ($values as $value) { $desc->addValue($value->getNumber(), $value); diff --git a/php/src/Google/Protobuf/Internal/GPBUtil.php b/php/src/Google/Protobuf/Internal/GPBUtil.php index 023b07f4..ec0bf6bd 100644 --- a/php/src/Google/Protobuf/Internal/GPBUtil.php +++ b/php/src/Google/Protobuf/Internal/GPBUtil.php @@ -215,9 +215,10 @@ class GPBUtil "Expect repeated field of different type."); } if ($var->getType() === GPBType::MESSAGE && - $var->getClass() !== $klass) { + $var->getClass() !== $klass && + $var->getLegacyClass() !== $klass) { throw new \Exception( - "Expect repeated field of different message."); + "Expect repeated field of " . $klass . "."); } return $var; } @@ -242,9 +243,10 @@ class GPBUtil throw new \Exception("Expect map field of value type."); } if ($var->getValueType() === GPBType::MESSAGE && - $var->getValueClass() !== $klass) { + $var->getValueClass() !== $klass && + $var->getLegacyValueClass() !== $klass) { throw new \Exception( - "Expect map field of different value message."); + "Expect map field of " . $klass . "."); } return $var; } @@ -299,6 +301,14 @@ class GPBUtil return ""; } + public static function getLegacyClassNameWithoutPackage( + $name, + $file_proto) + { + $classname = implode('_', explode('.', $name)); + return static::getClassNamePrefix($classname, $file_proto) . $classname; + } + public static function getClassNameWithoutPackage( $name, $file_proto) @@ -316,6 +326,7 @@ class GPBUtil $file_proto, &$message_name_without_package, &$classname, + &$legacy_classname, &$fullname) { // Full name needs to start with '.'. @@ -334,21 +345,28 @@ class GPBUtil $class_name_without_package = static::getClassNameWithoutPackage($message_name_without_package, $file_proto); + $legacy_class_name_without_package = + static::getLegacyClassNameWithoutPackage( + $message_name_without_package, $file_proto); $option = $file_proto->getOptions(); if (!is_null($option) && $option->hasPhpNamespace()) { $namespace = $option->getPhpNamespace(); if ($namespace !== "") { $classname = $namespace . "\\" . $class_name_without_package; + $legacy_classname = + $namespace . "\\" . $legacy_class_name_without_package; return; } else { $classname = $class_name_without_package; + $legacy_classname = $legacy_class_name_without_package; return; } } if ($package === "") { $classname = $class_name_without_package; + $legacy_classname = $legacy_class_name_without_package; } else { $parts = array_map('ucwords', explode('.', $package)); foreach ($parts as $i => $part) { @@ -358,6 +376,9 @@ class GPBUtil implode('\\', $parts) . "\\".self::getClassNamePrefix($class_name_without_package,$file_proto). $class_name_without_package; + $legacy_classname = + implode('\\', array_map('ucwords', explode('.', $package))). + "\\".$legacy_class_name_without_package; } } diff --git a/php/src/Google/Protobuf/Internal/MapField.php b/php/src/Google/Protobuf/Internal/MapField.php index 38736dad..f7d7b710 100644 --- a/php/src/Google/Protobuf/Internal/MapField.php +++ b/php/src/Google/Protobuf/Internal/MapField.php @@ -58,7 +58,11 @@ class MapField implements \ArrayAccess, \IteratorAggregate, \Countable /** * @ignore */ - private $value_klass; + private $klass; + /** + * @ignore + */ + private $legacy_klass; /** * Constructs an instance of MapField. @@ -75,6 +79,17 @@ class MapField implements \ArrayAccess, \IteratorAggregate, \Countable $this->key_type = $key_type; $this->value_type = $value_type; $this->klass = $klass; + + if ($this->value_type == GPBType::MESSAGE) { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName($klass); + if ($desc == NULL) { + new $klass; // No msg class instance has been created before. + $desc = $pool->getDescriptorByClassName($klass); + } + $this->klass = $desc->getClass(); + $this->legacy_klass = $desc->getLegacyClass(); + } } /** @@ -101,6 +116,14 @@ class MapField implements \ArrayAccess, \IteratorAggregate, \Countable return $this->klass; } + /** + * @ignore + */ + public function getLegacyValueClass() + { + return $this->legacy_klass; + } + /** * Return the element at the given key. * diff --git a/php/src/Google/Protobuf/Internal/RepeatedField.php b/php/src/Google/Protobuf/Internal/RepeatedField.php index 797b3b3a..e0ddef50 100644 --- a/php/src/Google/Protobuf/Internal/RepeatedField.php +++ b/php/src/Google/Protobuf/Internal/RepeatedField.php @@ -59,6 +59,10 @@ class RepeatedField implements \ArrayAccess, \IteratorAggregate, \Countable * @ignore */ private $klass; + /** + * @ignore + */ + private $legacy_klass; /** * Constructs an instance of RepeatedField. @@ -71,7 +75,16 @@ class RepeatedField implements \ArrayAccess, \IteratorAggregate, \Countable { $this->container = []; $this->type = $type; - $this->klass = $klass; + if ($this->type == GPBType::MESSAGE) { + $pool = DescriptorPool::getGeneratedPool(); + $desc = $pool->getDescriptorByClassName($klass); + if ($desc == NULL) { + new $klass; // No msg class instance has been created before. + $desc = $pool->getDescriptorByClassName($klass); + } + $this->klass = $desc->getClass(); + $this->legacy_klass = $desc->getLegacyClass(); + } } /** @@ -90,6 +103,14 @@ class RepeatedField implements \ArrayAccess, \IteratorAggregate, \Countable return $this->klass; } + /** + * @ignore + */ + public function getLegacyClass() + { + return $this->legacy_klass; + } + /** * Return the element at the given index. * diff --git a/php/tests/compatibility_test.sh b/php/tests/compatibility_test.sh index 7caa46e7..b5b255ea 100755 --- a/php/tests/compatibility_test.sh +++ b/php/tests/compatibility_test.sh @@ -2,12 +2,14 @@ function use_php() { VERSION=$1 - PHP=`which php` - PHP_CONFIG=`which php-config` - PHPIZE=`which phpize` - ln -sfn "/usr/local/php-${VERSION}/bin/php" $PHP - ln -sfn "/usr/local/php-${VERSION}/bin/php-config" $PHP_CONFIG - ln -sfn "/usr/local/php-${VERSION}/bin/phpize" $PHPIZE + + OLD_PATH=$PATH + OLD_CPLUS_INCLUDE_PATH=$CPLUS_INCLUDE_PATH + OLD_C_INCLUDE_PATH=$C_INCLUDE_PATH + + export PATH=/usr/local/php-${VERSION}/bin:$OLD_PATH + export CPLUS_INCLUDE_PATH=/usr/local/php-${VERSION}/include/php/main:/usr/local/php-${VERSION}/include/php/:$OLD_CPLUS_INCLUDE_PATH + export C_INCLUDE_PATH=/usr/local/php-${VERSION}/include/php/main:/usr/local/php-${VERSION}/include/php/:$OLD_C_INCLUDE_PATH } function generate_proto() { @@ -18,7 +20,22 @@ function generate_proto() { mkdir generated $PROTOC1 --php_out=generated proto/test_include.proto - $PROTOC2 --php_out=generated proto/test.proto proto/test_no_namespace.proto proto/test_prefix.proto + $PROTOC2 --php_out=generated \ + proto/test.proto \ + proto/test_no_namespace.proto \ + proto/test_prefix.proto \ + proto/test_php_namespace.proto \ + proto/test_empty_php_namespace.proto \ + proto/test_reserved_enum_lower.proto \ + proto/test_reserved_enum_upper.proto \ + proto/test_reserved_enum_value_lower.proto \ + proto/test_reserved_enum_value_upper.proto \ + proto/test_reserved_message_lower.proto \ + proto/test_reserved_message_upper.proto \ + proto/test_service.proto \ + proto/test_service_namespace.proto \ + proto/test_descriptors.proto + pushd ../../src $PROTOC2 --php_out=../php/tests/generated -I../php/tests -I. ../php/tests/proto/test_import_descriptor_proto.proto popd @@ -52,9 +69,9 @@ cd $(dirname $0) # The old version of protobuf that we are testing compatibility against. case "$1" in - ""|3.3.0) - OLD_VERSION=3.3.0 - OLD_VERSION_PROTOC=http://repo1.maven.org/maven2/com/google/protobuf/protoc/3.3.0/protoc-3.3.0-linux-x86_64.exe + ""|3.5.0) + OLD_VERSION=3.5.0 + OLD_VERSION_PROTOC=http://repo1.maven.org/maven2/com/google/protobuf/protoc/$OLD_VERSION/protoc-$OLD_VERSION-linux-x86_64.exe ;; *) echo "[ERROR]: Unknown version number: $1" @@ -81,7 +98,7 @@ git checkout v$OLD_VERSION popd # Build and copy the new runtime -use_php 5.5 +use_php 7.1 pushd ../ext/google/protobuf make clean || true phpize && ./configure && make @@ -99,12 +116,12 @@ chmod +x old_protoc NEW_PROTOC=`pwd`/../../src/protoc OLD_PROTOC=`pwd`/old_protoc cd protobuf/php -cp -r /usr/local/vendor-5.5 vendor -wget https://phar.phpunit.de/phpunit-4.8.0.phar -O /usr/bin/phpunit +composer install # Remove implementation detail tests. tests=( array_test.php encode_decode_test.php generated_class_test.php map_field_test.php well_known_test.php ) sed -i.bak '/php_implementation_test.php/d' phpunit.xml +sed -i.bak '/generated_phpdoc_test.php/d' phpunit.xml for t in "${tests[@]}" do remove_error_test tests/$t @@ -118,7 +135,7 @@ cd tests generate_proto $OLD_PROTOC $OLD_PROTOC ./test.sh pushd .. -phpunit +./vendor/bin/phpunit popd # Test A.2: @@ -127,7 +144,7 @@ popd generate_proto $NEW_PROTOC $OLD_PROTOC ./test.sh pushd .. -phpunit +./vendor/bin/phpunit popd # Test A.3: @@ -136,5 +153,5 @@ popd generate_proto $OLD_PROTOC $NEW_PROTOC ./test.sh pushd .. -phpunit +./vendor/bin/phpunit popd -- cgit v1.2.3