From 7ed2c262e26746b85104e318b534e04325967653 Mon Sep 17 00:00:00 2001 From: Brent Shaffer Date: Tue, 31 May 2022 15:19:28 -0700 Subject: [PATCH] feat: [PHP] add previous classname to descriptor pool --- php/composer.json | 5 +- php/ext/google/protobuf/def.c | 18 +++++-- php/ext/google/protobuf/names.c | 38 +++++++++++---- php/ext/google/protobuf/names.h | 2 +- php/ext/google/protobuf/protobuf.c | 9 +++- .../Google/Protobuf/Internal/Descriptor.php | 16 ++++++- .../Protobuf/Internal/DescriptorPool.php | 1 + .../Protobuf/Internal/EnumDescriptor.php | 3 +- php/src/Google/Protobuf/Internal/GPBUtil.php | 47 ++++++++++++++++++- php/tests/PreviouslyGeneratedClassTest.php | 18 +++++++ .../TestPreviouslyUnreservedMessage.php | 28 +++++++++++ .../generated_previous/Previous/readonly.php | 31 ++++++++++++ .../test_previously_unreserved_message.proto | 5 ++ 13 files changed, 199 insertions(+), 22 deletions(-) create mode 100644 php/tests/PreviouslyGeneratedClassTest.php create mode 100644 php/tests/generated_previous/GPBMetadata/ProtoPrevious/TestPreviouslyUnreservedMessage.php create mode 100644 php/tests/generated_previous/Previous/readonly.php create mode 100644 php/tests/proto_previous/test_previously_unreserved_message.proto diff --git a/php/composer.json b/php/composer.json index f712f0ebb9..001a120023 100644 --- a/php/composer.json +++ b/php/composer.json @@ -20,7 +20,10 @@ "autoload-dev": { "psr-4": { "": "tmp" - } + }, + "classmap": [ + "tests/generated_previous" + ] }, "scripts": { "test_c": "./generate_test_protos.sh && ./tests/compile_extension.sh && php -dextension=ext/google/protobuf/modules/protobuf.so vendor/bin/phpunit --bootstrap tests/force_c_ext.php tests", diff --git a/php/ext/google/protobuf/def.c b/php/ext/google/protobuf/def.c index dfb96f283f..8be2b92c51 100644 --- a/php/ext/google/protobuf/def.c +++ b/php/ext/google/protobuf/def.c @@ -162,7 +162,7 @@ static void EnumDescriptor_FromEnumDef(zval *val, const upb_EnumDef *m) { ZVAL_NULL(val); } else { char *classname = - GetPhpClassname(upb_EnumDef_File(m), upb_EnumDef_FullName(m)); + GetPhpClassname(upb_EnumDef_File(m), upb_EnumDef_FullName(m), false); zend_string *str = zend_string_init(classname, strlen(classname), 0); zend_class_entry *ce = zend_lookup_class(str); // May autoload the class. @@ -500,14 +500,22 @@ static void Descriptor_destructor(zend_object* obj) { static zend_class_entry *Descriptor_GetGeneratedClass(const upb_MessageDef *m) { char *classname = - GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m)); + GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), false); zend_string *str = zend_string_init(classname, strlen(classname), 0); zend_class_entry *ce = zend_lookup_class(str); // May autoload the class. - zend_string_release (str); - if (!ce) { - zend_error(E_ERROR, "Couldn't load generated class %s", classname); + char *classname2 = + GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), true); + str = zend_string_init(classname2, strlen(classname2), 0); + ce = zend_lookup_class(str); // May autoload the class. + + zend_string_release (str); + free(classname2); + + if (!ce) { + zend_error(E_ERROR, "Couldn't load generated class %s", classname); + } } free(classname); diff --git a/php/ext/google/protobuf/names.c b/php/ext/google/protobuf/names.c index a2988816ed..b304d92310 100644 --- a/php/ext/google/protobuf/names.c +++ b/php/ext/google/protobuf/names.c @@ -89,6 +89,9 @@ const char *const kReservedNames[] = { "string", "true", "false", "null", "void", "iterable", NULL}; +const char *const kPreviouslyUnreservedNames[] = { + "readonly", NULL}; + bool is_reserved_name(const char* name) { int i; for (i = 0; kReservedNames[i]; i++) { @@ -99,6 +102,16 @@ bool is_reserved_name(const char* name) { return false; } +bool is_previously_unreserved_name(const char* name) { + int i; + for (i = 0; kPreviouslyUnreservedNames[i]; i++) { + if (strcmp(kPreviouslyUnreservedNames[i], name) == 0) { + return true; + } + } + return false; +} + static char nolocale_tolower(char ch) { if (ch >= 'A' && ch <= 'Z') { return ch - ('A' - 'a'); @@ -115,7 +128,7 @@ static char nolocale_toupper(char ch) { } } -static bool is_reserved(const char *segment, int length) { +static bool is_reserved(const char *segment, int length, bool previous) { bool result; char* lower = calloc(1, length + 1); memcpy(lower, segment, length); @@ -126,6 +139,9 @@ static bool is_reserved(const char *segment, int length) { } lower[length] = 0; result = is_reserved_name(lower); + if (result && previous && is_previously_unreserved_name(lower)) { + result = false; + } free(lower); return result; } @@ -133,11 +149,12 @@ static bool is_reserved(const char *segment, int length) { static void fill_prefix(const char *segment, int length, const char *prefix_given, const char *package_name, - stringsink *classname) { + stringsink *classname, + bool previous) { if (prefix_given != NULL && strcmp(prefix_given, "") != 0) { stringsink_string(classname, prefix_given, strlen(prefix_given)); } else { - if (is_reserved(segment, length)) { + if (is_reserved(segment, length, previous)) { if (package_name != NULL && strcmp("google.protobuf", package_name) == 0) { stringsink_string(classname, "GPB", 3); @@ -160,7 +177,7 @@ static void fill_segment(const char *segment, int length, } static void fill_namespace(const char *package, const char *php_namespace, - stringsink *classname) { + stringsink *classname, bool previous) { if (php_namespace != NULL) { if (strlen(php_namespace) != 0) { stringsink_string(classname, php_namespace, strlen(php_namespace)); @@ -174,7 +191,7 @@ static void fill_namespace(const char *package, const char *php_namespace, while (j < package_len && package[j] != '.') { j++; } - fill_prefix(package + i, j - i, "", package, classname); + fill_prefix(package + i, j - i, "", package, classname, previous); fill_segment(package + i, j - i, classname, true); stringsink_string(classname, "\\", 1); i = j + 1; @@ -185,7 +202,8 @@ static void fill_namespace(const char *package, const char *php_namespace, static void fill_classname(const char *fullname, const char *package, const char *prefix, - stringsink *classname) { + stringsink *classname, + bool previous) { int classname_start = 0; if (package != NULL) { size_t package_len = strlen(package); @@ -199,7 +217,7 @@ static void fill_classname(const char *fullname, while (j < fullname_len && fullname[j] != '.') { j++; } - fill_prefix(fullname + i, j - i, prefix, package, classname); + fill_prefix(fullname + i, j - i, prefix, package, classname, previous); fill_segment(fullname + i, j - i, classname, false); if (j != fullname_len) { stringsink_string(classname, "\\", 1); @@ -215,7 +233,7 @@ char *str_view_dup(upb_StringView str) { return ret; } -char *GetPhpClassname(const upb_FileDef *file, const char *fullname) { +char *GetPhpClassname(const upb_FileDef *file, const char *fullname, bool previous) { // Prepend '.' to package name to make it absolute. In the 5 additional // bytes allocated, one for '.', one for trailing 0, and 3 for 'GPB' if // given message is google.protobuf.Empty. @@ -234,8 +252,8 @@ char *GetPhpClassname(const upb_FileDef *file, const char *fullname) { stringsink namesink; stringsink_init(&namesink); - fill_namespace(package, php_namespace, &namesink); - fill_classname(fullname, package, prefix, &namesink); + fill_namespace(package, php_namespace, &namesink, previous); + fill_classname(fullname, package, prefix, &namesink, previous); stringsink_string(&namesink, "\0", 1); ret = strdup(namesink.ptr); stringsink_uninit(&namesink); diff --git a/php/ext/google/protobuf/names.h b/php/ext/google/protobuf/names.h index 86af799ac0..52911c319e 100644 --- a/php/ext/google/protobuf/names.h +++ b/php/ext/google/protobuf/names.h @@ -35,6 +35,6 @@ // Translates a protobuf symbol name (eg. foo.bar.Baz) into a PHP class name // (eg. \Foo\Bar\Baz). -char *GetPhpClassname(const upb_FileDef *file, const char *fullname); +char *GetPhpClassname(const upb_FileDef *file, const char *fullname, bool previous); #endif // PHP_PROTOBUF_NAMES_H_ diff --git a/php/ext/google/protobuf/protobuf.c b/php/ext/google/protobuf/protobuf.c index c786b6eca0..268108654a 100644 --- a/php/ext/google/protobuf/protobuf.c +++ b/php/ext/google/protobuf/protobuf.c @@ -242,13 +242,18 @@ bool ObjCache_Get(const void *upb_obj, zval *val) { // ----------------------------------------------------------------------------- void NameMap_AddMessage(const upb_MessageDef *m) { - char *k = GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m)); + char *k = GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), false); zend_hash_str_add_ptr(&PROTOBUF_G(name_msg_cache), k, strlen(k), (void*)m); + char *k2 = GetPhpClassname(upb_MessageDef_File(m), upb_MessageDef_FullName(m), true); + if (strcmp(k, k2) != 0) { + zend_hash_str_add_ptr(&PROTOBUF_G(name_msg_cache), k2, strlen(k2), (void*)m); + } free(k); + free(k2); } void NameMap_AddEnum(const upb_EnumDef *e) { - char *k = GetPhpClassname(upb_EnumDef_File(e), upb_EnumDef_FullName(e)); + char *k = GetPhpClassname(upb_EnumDef_File(e), upb_EnumDef_FullName(e), false); zend_hash_str_add_ptr(&PROTOBUF_G(name_enum_cache), k, strlen(k), (void*)e); free(k); } diff --git a/php/src/Google/Protobuf/Internal/Descriptor.php b/php/src/Google/Protobuf/Internal/Descriptor.php index a7f80d534e..b7d59cbf9b 100644 --- a/php/src/Google/Protobuf/Internal/Descriptor.php +++ b/php/src/Google/Protobuf/Internal/Descriptor.php @@ -45,6 +45,7 @@ class Descriptor private $enum_type = []; private $klass; private $legacy_klass; + private $previous_klass; private $options; private $oneof_decl = []; @@ -162,6 +163,16 @@ class Descriptor return $this->legacy_klass; } + public function setPreviouslyReservedClass($klass) + { + $this->previous_klass = $klass; + } + + public function getPreviouslyReservedClass() + { + return $this->previous_klass; + } + public function setOptions($options) { $this->options = $options; @@ -179,6 +190,7 @@ class Descriptor $message_name_without_package = ""; $classname = ""; $legacy_classname = ""; + $previous_classname = ""; $fullname = ""; GPBUtil::getFullClassName( $proto, @@ -187,10 +199,12 @@ class Descriptor $message_name_without_package, $classname, $legacy_classname, - $fullname); + $fullname, + $previous_classname); $desc->setFullName($fullname); $desc->setClass($classname); $desc->setLegacyClass($legacy_classname); + $desc->setPreviouslyReservedClass($previous_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 1468a02380..9709a1c80e 100644 --- a/php/src/Google/Protobuf/Internal/DescriptorPool.php +++ b/php/src/Google/Protobuf/Internal/DescriptorPool.php @@ -96,6 +96,7 @@ class DescriptorPool $descriptor->getClass(); $this->class_to_desc[$descriptor->getClass()] = $descriptor; $this->class_to_desc[$descriptor->getLegacyClass()] = $descriptor; + $this->class_to_desc[$descriptor->getPreviouslyReservedClass()] = $descriptor; foreach ($descriptor->getNestedType() as $nested_type) { $this->addDescriptor($nested_type); } diff --git a/php/src/Google/Protobuf/Internal/EnumDescriptor.php b/php/src/Google/Protobuf/Internal/EnumDescriptor.php index 7af4f84012..383f53b13f 100644 --- a/php/src/Google/Protobuf/Internal/EnumDescriptor.php +++ b/php/src/Google/Protobuf/Internal/EnumDescriptor.php @@ -101,7 +101,8 @@ class EnumDescriptor $enum_name_without_package, $classname, $legacy_classname, - $fullname); + $fullname, + $unused_previous_classname); $desc->setFullName($fullname); $desc->setClass($classname); $desc->setLegacyClass($legacy_classname); diff --git a/php/src/Google/Protobuf/Internal/GPBUtil.php b/php/src/Google/Protobuf/Internal/GPBUtil.php index d7f2faaf6c..0e5cd2a7e6 100644 --- a/php/src/Google/Protobuf/Internal/GPBUtil.php +++ b/php/src/Google/Protobuf/Internal/GPBUtil.php @@ -304,6 +304,27 @@ class GPBUtil return ""; } + public static function getPreviouslyUnreservedClassNamePrefix( + $classname, + $file_proto) + { + $previously_unreserved_words = array( + "readonly"=>0 + ); + + if (array_key_exists(strtolower($classname), $previously_unreserved_words)) { + $option = $file_proto->getOptions(); + $prefix = is_null($option) ? "" : $option->getPhpClassPrefix(); + if ($prefix !== "") { + return $prefix; + } + + return ""; + } + + return self::getClassNamePrefix($classname, $file_proto); + } + public static function getLegacyClassNameWithoutPackage( $name, $file_proto) @@ -323,6 +344,17 @@ class GPBUtil return implode('\\', $parts); } + public static function getPreviouslyUnreservedClassNameWithoutPackage( + $name, + $file_proto) + { + $parts = explode('.', $name); + foreach ($parts as $i => $part) { + $parts[$i] = static::getPreviouslyUnreservedClassNamePrefix($parts[$i], $file_proto) . $parts[$i]; + } + return implode('\\', $parts); + } + public static function getFullClassName( $proto, $containing, @@ -330,7 +362,8 @@ class GPBUtil &$message_name_without_package, &$classname, &$legacy_classname, - &$fullname) + &$fullname, + &$previous_classname) { // Full name needs to start with '.'. $message_name_without_package = $proto->getName(); @@ -351,6 +384,9 @@ class GPBUtil $legacy_class_name_without_package = static::getLegacyClassNameWithoutPackage( $message_name_without_package, $file_proto); + $previous_class_name_without_package = + static::getPreviouslyUnreservedClassNameWithoutPackage( + $message_name_without_package, $file_proto); $option = $file_proto->getOptions(); if (!is_null($option) && $option->hasPhpNamespace()) { @@ -359,10 +395,13 @@ class GPBUtil $classname = $namespace . "\\" . $class_name_without_package; $legacy_classname = $namespace . "\\" . $legacy_class_name_without_package; + $previous_classname = + $namespace . "\\" . $previous_class_name_without_package; return; } else { $classname = $class_name_without_package; $legacy_classname = $legacy_class_name_without_package; + $previous_classname = $previous_class_name_without_package; return; } } @@ -370,6 +409,7 @@ class GPBUtil if ($package === "") { $classname = $class_name_without_package; $legacy_classname = $legacy_class_name_without_package; + $previous_classname = $previous_class_name_without_package; } else { $parts = array_map('ucwords', explode('.', $package)); foreach ($parts as $i => $part) { @@ -382,6 +422,11 @@ class GPBUtil $legacy_classname = implode('\\', array_map('ucwords', explode('.', $package))). "\\".$legacy_class_name_without_package; + $previous_classname = + implode('\\', array_map('ucwords', explode('.', $package))). + "\\".self::getPreviouslyUnreservedClassNamePrefix( + $previous_class_name_without_package, $file_proto). + $previous_class_name_without_package; } } diff --git a/php/tests/PreviouslyGeneratedClassTest.php b/php/tests/PreviouslyGeneratedClassTest.php new file mode 100644 index 0000000000..077c84ff13 --- /dev/null +++ b/php/tests/PreviouslyGeneratedClassTest.php @@ -0,0 +1,18 @@ +assertTrue(true); + } +} diff --git a/php/tests/generated_previous/GPBMetadata/ProtoPrevious/TestPreviouslyUnreservedMessage.php b/php/tests/generated_previous/GPBMetadata/ProtoPrevious/TestPreviouslyUnreservedMessage.php new file mode 100644 index 0000000000..d2120f9b6b --- /dev/null +++ b/php/tests/generated_previous/GPBMetadata/ProtoPrevious/TestPreviouslyUnreservedMessage.php @@ -0,0 +1,28 @@ +internalAddGeneratedFile( + ' +W +7proto_previous/test_previously_unreserved_message.protoprevious" + +readonlybproto3' + , true); + + static::$is_initialized = true; + } +} + diff --git a/php/tests/generated_previous/Previous/readonly.php b/php/tests/generated_previous/Previous/readonly.php new file mode 100644 index 0000000000..013f29354c --- /dev/null +++ b/php/tests/generated_previous/Previous/readonly.php @@ -0,0 +1,31 @@ +previous.readonly + */ +class readonly extends \Google\Protobuf\Internal\Message +{ + + /** + * Constructor. + * + * @param array $data { + * Optional. Data for populating the Message object. + * + * } + */ + public function __construct($data = NULL) { + \GPBMetadata\ProtoPrevious\TestPreviouslyUnreservedMessage::initOnce(); + parent::__construct($data); + } + +} + diff --git a/php/tests/proto_previous/test_previously_unreserved_message.proto b/php/tests/proto_previous/test_previously_unreserved_message.proto new file mode 100644 index 0000000000..1b4b0b7978 --- /dev/null +++ b/php/tests/proto_previous/test_previously_unreserved_message.proto @@ -0,0 +1,5 @@ +syntax = "proto3"; + +package previous; + +message readonly {} \ No newline at end of file