Skip to content

Commit

Permalink
[BUGFIX] Avoid symfony/property-access in getGettablePropertyNames()
Browse files Browse the repository at this point in the history
ObjectAccess::getGettablePropertyNames() has quite the history by now.
It used to be quite simple, an is_callable() check for getters/hassers
and issers of objects. That didn't account for methods with mandatory
method arguments which was fixed by using reflection. Because runtime
reflection is slow, the usage of cached reflection (ClassSchema) had
been introduced. But, during that change, symfony/property-access had
also been introduces, which contradicts the idea of performance gain
because:

- symfony/property-access also uses uncached reflection
- symfony/property-access actually calls the accessors under test

As both is undesirable, the usage of symfony/property-access has been
removed again.

Releases: main, 12.4, 11.5
Resolves: #101176
Change-Id: I2bc796ebeaf2f1357fd3154b711910c6f553f4e4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/79675
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
  • Loading branch information
alexanderschnitzler authored and bmack committed Jul 4, 2023
1 parent c5fff77 commit 2289ae2
Showing 1 changed file with 53 additions and 18 deletions.
71 changes: 53 additions & 18 deletions typo3/sysext/extbase/Classes/Reflection/ObjectAccess.php
Expand Up @@ -209,37 +209,72 @@ public static function getGettablePropertyNames(object $object): array
$classSchema = GeneralUtility::makeInstance(ReflectionService::class)
->getClassSchema($object);

$accessor = self::createAccessor();
$propertyNames = array_keys($classSchema->getProperties());
$accessiblePropertyNames = array_filter(
$propertyNames,
static fn (string $propertyName): bool => $accessor->isReadable($object, $propertyName)
);

foreach ($classSchema->getMethods() as $methodName => $methodDefinition) {
if (!$methodDefinition->isPublic()) {
$accessiblePropertyNames = [];
foreach ($classSchema->getProperties() as $propertyName => $propertyDefinition) {
if ($propertyDefinition->isPublic()) {
$accessiblePropertyNames[] = $propertyName;
continue;
}

foreach ($methodDefinition->getParameters() as $methodParam) {
if (!$methodParam->isOptional()) {
continue 2;
$accessors = [
'get' . ucfirst($propertyName),
'has' . ucfirst($propertyName),
'is' . ucfirst($propertyName),
];

foreach ($accessors as $accessor) {
if (!$classSchema->hasMethod($accessor)) {
continue;
}

if (!$classSchema->getMethod($accessor)->isPublic()) {
continue;
}

foreach ($classSchema->getMethod($accessor)->getParameters() as $methodParam) {
if (!$methodParam->isOptional()) {
continue 2;
}
}

if (!is_callable([$object, $accessor])) {
continue;
}

$accessiblePropertyNames[] = $propertyName;
}
}

// Fallback mechanism to not break former behaviour
//
// todo: Checking accessor methods of virtual(non-existing) properties should be removed (breaking) in
// upcoming versions. It was an unintentionally added "feature" in the past. It contradicts the method
// name "getGettablePropertyNames".
foreach ($classSchema->getMethods() as $methodName => $methodDefinition) {
$propertyName = null;
if (str_starts_with($methodName, 'get') || str_starts_with($methodName, 'has')) {
$propertyName = lcfirst(substr($methodName, 3));
}

if (str_starts_with($methodName, 'is')) {
$propertyName = lcfirst(substr($methodName, 2));
}

if (str_starts_with($methodName, 'get')) {
$accessiblePropertyNames[] = lcfirst(substr($methodName, 3));
if ($propertyName === null) {
continue;
}

if (str_starts_with($methodName, 'has')) {
$accessiblePropertyNames[] = lcfirst(substr($methodName, 3));
if (!$methodDefinition->isPublic()) {
continue;
}

if (str_starts_with($methodName, 'is')) {
$accessiblePropertyNames[] = lcfirst(substr($methodName, 2));
foreach ($methodDefinition->getParameters() as $methodParam) {
if (!$methodParam->isOptional()) {
continue 2;
}
}

$accessiblePropertyNames[] = $propertyName;
}

$accessiblePropertyNames = array_unique($accessiblePropertyNames);
Expand Down

0 comments on commit 2289ae2

Please sign in to comment.