Skip to content

PropertyAccessor don't evaluate properly when 1:M relations and make use of CollectionType #29447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
maikelohcfg opened this issue Dec 3, 2018 · 3 comments

Comments

@maikelohcfg
Copy link

maikelohcfg commented Dec 3, 2018

Symfony version(s) affected: 3.4.19

Description
Class Symfony\Component\PropertyAccess\PropertyAccessor on method getWriteAccessInfo don't properly handle condition evaluation for call method findAdderAndRemover on line 730.

How to reproduce
I am working on some project and make use of one to many relation with a form collection type for the M side of the relation and following Symfony Documentation declared method addPropertyName and removePropertyName properly. Everything was working good until updated to version 3.4.19 through composer. Now I have to make setPropertyName method private because if it is public it gain preference over addPropertyName on condition evaluation so the owning side of the relation never keep null on M side of the relation

`
private function getWriteAccessInfo($class, $property, $value)
{
$key = str_replace('\', '.', $class).'..'.$property;

    if (isset($this->writePropertyCache[$key])) {
        return $this->writePropertyCache[$key];
    }

    if ($this->cacheItemPool) {
        $item = $this->cacheItemPool->getItem(self::CACHE_PREFIX_WRITE.rawurlencode($key));
        if ($item->isHit()) {
            return $this->writePropertyCache[$key] = $item->get();
        }
    }

    $access = array();

    $reflClass = new \ReflectionClass($class);
    $access[self::ACCESS_HAS_PROPERTY] = $reflClass->hasProperty($property);
    $camelized = $this->camelize($property);
    $singulars = (array) Inflector::singularize($camelized);

    if (!isset($access[self::ACCESS_TYPE])) {
        $setter = 'set'.$camelized;
        $getsetter = lcfirst($camelized); // jQuery style, e.g. read: last(), write: last($item)

        if ($this->isMethodAccessible($reflClass, $setter, 1)) {
            $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_METHOD;
            $access[self::ACCESS_NAME] = $setter;
        } elseif ($this->isMethodAccessible($reflClass, $getsetter, 1)) {
            $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_METHOD;
            $access[self::ACCESS_NAME] = $getsetter;
        } elseif ($this->isMethodAccessible($reflClass, '__set', 2)) {
            $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_PROPERTY;
            $access[self::ACCESS_NAME] = $property;
        } elseif ($access[self::ACCESS_HAS_PROPERTY] && $reflClass->getProperty($property)->isPublic()) {
            $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_PROPERTY;
            $access[self::ACCESS_NAME] = $property;
        } elseif ($this->magicCall && $this->isMethodAccessible($reflClass, '__call', 2)) {
            // we call the getter and hope the __call do the job
            $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_MAGIC;
            $access[self::ACCESS_NAME] = $setter;
        } elseif (null !== $methods = $this->findAdderAndRemover($reflClass, $singulars)) {
            if (\is_array($value) || $value instanceof \Traversable) {
                $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_ADDER_AND_REMOVER;
                $access[self::ACCESS_ADDER] = $methods[0];
                $access[self::ACCESS_REMOVER] = $methods[1];
            } else {
                $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
                $access[self::ACCESS_NAME] = sprintf(
                    'The property "%s" in class "%s" can be defined with the methods "%s()" but '.
                    'the new value must be an array or an instance of \Traversable, '.
                    '"%s" given.',
                    $property,
                    $reflClass->name,
                    implode('()", "', $methods),
                    \is_object($value) ? \get_class($value) : \gettype($value)
                );
            }
        } else {
            $access[self::ACCESS_TYPE] = self::ACCESS_TYPE_NOT_FOUND;
            $access[self::ACCESS_NAME] = sprintf(
                'Neither the property "%s" nor one of the methods %s"%s()", "%s()", '.
                '"__set()" or "__call()" exist and have public access in class "%s".',
                $property,
                implode('', array_map(function ($singular) {
                    return '"add'.$singular.'()"/"remove'.$singular.'()", ';
                }, $singulars)),
                $setter,
                $getsetter,
                $reflClass->name
            );
        }
    }

    
    if (isset($item)) {
        $this->cacheItemPool->save($item->set($access));
    }

    return $this->writePropertyCache[$key] = $access;
}`

Possible Solution
I want to make pull request to fix this, but I see on master branch you already have a pre call to findAdderAndRemover and give preference over set as should be. But on composer again try to update and still get buggy code, wich force me to make setter private on entities to surround bug and keep working 1:M relation with collection type.

Maybe you need to update the composer package to some more recent commit.

@nicolas-grekas
Copy link
Member

Isn't this fixed by #29355 already?

@maikelohcfg
Copy link
Author

Yes it does, I see the code con master branch, but again when I try to update through composer it seem that composer package for 3.4.19 don't have included the commit.

@xabbuh
Copy link
Member

xabbuh commented Dec 3, 2018

The fix is not released yet. You either have to downgrade to 3.4.18 for now or update to 3.4@dev.

@xabbuh xabbuh closed this as completed Dec 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants