Skip to content

Commit

Permalink
bug #54148 [Serializer] Fix object normalizer when properties has the…
Browse files Browse the repository at this point in the history
… same name as their accessor (NeilPeyssard)

This PR was merged into the 5.4 branch.

Discussion
----------

[Serializer] Fix object normalizer when properties has the same name as their accessor

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #54109
| License       | MIT

Hello,

This PR fix a bug in object normalization when properties has the same name as their accessor. The current behavior mess up class metadata between properties and methods so the keys in the normalized format does not match the object properties, and metadata are not correctly applied (`SerializedName` in our exemple).

This bug also affects versions 6.4 and 7+, but I'm not sure if we can merge this PR in these branches without refactoring. Let me know if another PR should be open for version 6.4 and 7.

Commits
-------

8575199f30 [Serializer] Fix object normalizer when properties has the same name as their accessor
  • Loading branch information
fabpot committed Mar 17, 2024
2 parents 3cff915 + 096cddc commit 1a23213
Show file tree
Hide file tree
Showing 6 changed files with 238 additions and 5 deletions.
2 changes: 1 addition & 1 deletion Mapping/Loader/AnnotationLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public function loadClassMetadata(ClassMetadataInterface $classMetadata)

$accessorOrMutator = preg_match('/^(get|is|has|set)(.+)$/i', $method->name, $matches);
if ($accessorOrMutator) {
$attributeName = lcfirst($matches[2]);
$attributeName = $reflectionClass->hasProperty($method->name) ? $method->name : lcfirst($matches[2]);

if (isset($attributesMetadata[$attributeName])) {
$attributeMetadata = $attributesMetadata[$attributeName];
Expand Down
16 changes: 12 additions & 4 deletions Normalizer/ObjectNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,25 @@ protected function extractAttributes(object $object, ?string $format = null, arr

if (str_starts_with($name, 'get') || str_starts_with($name, 'has')) {
// getters and hassers
$attributeName = substr($name, 3);
$attributeName = $name;

if (!$reflClass->hasProperty($attributeName)) {
$attributeName = lcfirst($attributeName);
$attributeName = substr($attributeName, 3);

if (!$reflClass->hasProperty($attributeName)) {
$attributeName = lcfirst($attributeName);
}
}
} elseif (str_starts_with($name, 'is')) {
// issers
$attributeName = substr($name, 2);
$attributeName = $name;

if (!$reflClass->hasProperty($attributeName)) {
$attributeName = lcfirst($attributeName);
$attributeName = substr($attributeName, 2);

if (!$reflClass->hasProperty($attributeName)) {
$attributeName = lcfirst($attributeName);
}
}
}

Expand Down
48 changes: 48 additions & 0 deletions Tests/Fixtures/SamePropertyAsMethodDummy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Serializer\Tests\Fixtures;

class SamePropertyAsMethodDummy
{
private $freeTrial;
private $hasSubscribe;
private $getReady;
private $isActive;

public function __construct($freeTrial, $hasSubscribe, $getReady, $isActive)
{
$this->freeTrial = $freeTrial;
$this->hasSubscribe = $hasSubscribe;
$this->getReady = $getReady;
$this->isActive = $isActive;
}

public function getFreeTrial()
{
return $this->freeTrial;
}

public function hasSubscribe()
{
return $this->hasSubscribe;
}

public function getReady()
{
return $this->getReady;
}

public function isActive()
{
return $this->isActive;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Serializer\Tests\Fixtures;

use Symfony\Component\Serializer\Annotation\SerializedName;

class SamePropertyAsMethodWithMethodSerializedNameDummy
{
private $freeTrial;
private $hasSubscribe;
private $getReady;
private $isActive;

public function __construct($freeTrial, $hasSubscribe, $getReady, $isActive)
{
$this->freeTrial = $freeTrial;
$this->hasSubscribe = $hasSubscribe;
$this->getReady = $getReady;
$this->isActive = $isActive;
}

/**
* @SerializedName("free_trial_method")
*/
public function getFreeTrial()
{
return $this->freeTrial;
}

/**
* @SerializedName("has_subscribe_method")
*/
public function hasSubscribe()
{
return $this->hasSubscribe;
}

/**
* @SerializedName("get_ready_method")
*/
public function getReady()
{
return $this->getReady;
}

/**
* @SerializedName("is_active_method")
*/
public function isActive()
{
return $this->isActive;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Serializer\Tests\Fixtures;

use Symfony\Component\Serializer\Annotation\SerializedName;

class SamePropertyAsMethodWithPropertySerializedNameDummy
{
/**
* @SerializedName("free_trial_property")
*/
private $freeTrial;

/**
* @SerializedName("has_subscribe_property")
*/
private $hasSubscribe;

/**
* @SerializedName("get_ready_property")
*/
private $getReady;

/**
* @SerializedName("is_active_property")
*/
private $isActive;

public function __construct($freeTrial, $hasSubscribe, $getReady, $isActive)
{
$this->freeTrial = $freeTrial;
$this->hasSubscribe = $hasSubscribe;
$this->getReady = $getReady;
$this->isActive = $isActive;
}

public function getFreeTrial()
{
return $this->freeTrial;
}

public function hasSubscribe()
{
return $this->hasSubscribe;
}

public function getReady()
{
return $this->getReady;
}

public function isActive()
{
return $this->isActive;
}
}
50 changes: 50 additions & 0 deletions Tests/Normalizer/ObjectNormalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@
use Symfony\Component\Serializer\Tests\Fixtures\Php74Dummy;
use Symfony\Component\Serializer\Tests\Fixtures\Php74DummyPrivate;
use Symfony\Component\Serializer\Tests\Fixtures\Php80Dummy;
use Symfony\Component\Serializer\Tests\Fixtures\SamePropertyAsMethodDummy;
use Symfony\Component\Serializer\Tests\Fixtures\SamePropertyAsMethodWithMethodSerializedNameDummy;
use Symfony\Component\Serializer\Tests\Fixtures\SamePropertyAsMethodWithPropertySerializedNameDummy;
use Symfony\Component\Serializer\Tests\Fixtures\SiblingHolder;
use Symfony\Component\Serializer\Tests\Normalizer\Features\AttributesTestTrait;
use Symfony\Component\Serializer\Tests\Normalizer\Features\CacheableObjectAttributesTestTrait;
Expand Down Expand Up @@ -869,6 +872,53 @@ public function testNormalizeStdClass()

$this->assertSame(['baz' => 'baz'], $this->normalizer->normalize($o2));
}

public function testSamePropertyAsMethod()
{
$object = new SamePropertyAsMethodDummy('free_trial', 'has_subscribe', 'get_ready', 'is_active');
$expected = [
'freeTrial' => 'free_trial',
'hasSubscribe' => 'has_subscribe',
'getReady' => 'get_ready',
'isActive' => 'is_active',
];

$this->assertSame($expected, $this->normalizer->normalize($object));
}

public function testSamePropertyAsMethodWithPropertySerializedName()
{
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
$this->normalizer = new ObjectNormalizer($classMetadataFactory, new MetadataAwareNameConverter($classMetadataFactory));
$this->normalizer->setSerializer($this->serializer);

$object = new SamePropertyAsMethodWithPropertySerializedNameDummy('free_trial', 'has_subscribe', 'get_ready', 'is_active');
$expected = [
'free_trial_property' => 'free_trial',
'has_subscribe_property' => 'has_subscribe',
'get_ready_property' => 'get_ready',
'is_active_property' => 'is_active',
];

$this->assertSame($expected, $this->normalizer->normalize($object));
}

public function testSamePropertyAsMethodWithMethodSerializedName()
{
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
$this->normalizer = new ObjectNormalizer($classMetadataFactory, new MetadataAwareNameConverter($classMetadataFactory));
$this->normalizer->setSerializer($this->serializer);

$object = new SamePropertyAsMethodWithMethodSerializedNameDummy('free_trial', 'has_subscribe', 'get_ready', 'is_active');
$expected = [
'free_trial_method' => 'free_trial',
'has_subscribe_method' => 'has_subscribe',
'get_ready_method' => 'get_ready',
'is_active_method' => 'is_active',
];

$this->assertSame($expected, $this->normalizer->normalize($object));
}
}

class ProxyObjectDummy extends ObjectDummy
Expand Down

0 comments on commit 1a23213

Please sign in to comment.