Skip to content

[Serializer] Handle circular references #12098

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

Merged
merged 1 commit into from
Oct 20, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?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\Exception;

/**
* CircularReferenceException
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class CircularReferenceException extends RuntimeException
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Serializer\Normalizer;

use Symfony\Component\Serializer\Exception\CircularReferenceException;
use Symfony\Component\Serializer\Exception\InvalidArgumentException;
use Symfony\Component\Serializer\Exception\RuntimeException;

Expand All @@ -33,13 +34,50 @@
* takes place.
*
* @author Nils Adermann <naderman@naderman.de>
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class GetSetMethodNormalizer extends SerializerAwareNormalizer implements NormalizerInterface, DenormalizerInterface
{
protected $circularReferenceLimit = 1;
protected $circularReferenceHandler;
protected $callbacks = array();
protected $ignoredAttributes = array();
protected $camelizedAttributes = array();

/**
* Set circular reference limit.
*
* @param $circularReferenceLimit limit of iterations for the same object
*
* @return self
*/
public function setCircularReferenceLimit($circularReferenceLimit)
{
$this->circularReferenceLimit = $circularReferenceLimit;

return $this;
}

/**
* Set circular reference handler.
*
* @param callable $circularReferenceHandler
*
* @return self
*
* @throws InvalidArgumentException
*/
public function setCircularReferenceHandler($circularReferenceHandler)
{
if (!is_callable($circularReferenceHandler)) {
throw new InvalidArgumentException('The given circular reference handler is not callable.');
}

$this->circularReferenceHandler = $circularReferenceHandler;

return $this;
}

/**
* Set normalization callbacks.
*
Expand Down Expand Up @@ -94,6 +132,24 @@ public function setCamelizedAttributes(array $camelizedAttributes)
*/
public function normalize($object, $format = null, array $context = array())
{
$objectHash = spl_object_hash($object);

if (isset($context['circular_reference_limit'][$objectHash])) {
if ($context['circular_reference_limit'][$objectHash] >= $this->circularReferenceLimit) {
unset($context['circular_reference_limit'][$objectHash]);

if ($this->circularReferenceHandler) {
return call_user_func($this->circularReferenceHandler, $object);
}

throw new CircularReferenceException(sprintf('A circular reference has been detected (configured limit: %d).', $this->circularReferenceLimit));
}

$context['circular_reference_limit'][$objectHash]++;
} else {
$context['circular_reference_limit'][$objectHash] = 1;
}

$reflectionObject = new \ReflectionObject($object);
$reflectionMethods = $reflectionObject->getMethods(\ReflectionMethod::IS_PUBLIC);

Expand All @@ -114,7 +170,8 @@ public function normalize($object, $format = null, array $context = array())
if (!$this->serializer instanceof NormalizerInterface) {
throw new \LogicException(sprintf('Cannot normalize attribute "%s" because injected serializer is not a normalizer', $attributeName));
}
$attributeValue = $this->serializer->normalize($attributeValue, $format);

$attributeValue = $this->serializer->normalize($attributeValue, $format, $context);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing the $context['circular_reference_limit'] array by an SplObjectStorage should prevent the many copies that currently occurs along the recursive walk.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, sorry, what I just said is wrong: an SplObjectStorage object would share the state between different branches of the recursive tree, which is not what you want I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the commit I just pushed? It should work and reduce the memory usage?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add a test that checks what happens when you serialize a structure that has many times the same object at sibling positions (more than circularReferenceLimit times)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas Good catch.
I've switched back to an array implementation and added a test for siblings.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, sorry for the misleading comment, at least we gained a test :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're welcome. It could have been a good improvement!

}

$attributes[$attributeName] = $attributeValue;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?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;

/**
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class CircularReferenceDummy
{
public function getMe()
{
return $this;
}
}
56 changes: 56 additions & 0 deletions src/Symfony/Component/Serializer/Tests/Fixtures/SiblingHolder.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?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;

/**
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class SiblingHolder
{
private $sibling0;
private $sibling1;
private $sibling2;

public function __construct()
{
$sibling = new Sibling();
$this->sibling0 = $sibling;
$this->sibling1 = $sibling;
$this->sibling2 = $sibling;
}

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

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

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

/**
* @author Kévin Dunglas <dunglas@gmail.com>
*/
class Sibling
{
public function getCoopTilleuls()
{
return 'Les-Tilleuls.coop';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@
namespace Symfony\Component\Serializer\Tests\Normalizer;

use Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer;
use Symfony\Component\Serializer\Serializer;
use Symfony\Component\Serializer\SerializerInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\Tests\Fixtures\CircularReferenceDummy;
use Symfony\Component\Serializer\Tests\Fixtures\SiblingHolder;

class GetSetMethodNormalizerTest extends \PHPUnit_Framework_TestCase
{
Expand Down Expand Up @@ -271,6 +274,49 @@ public function testUnableToNormalizeObjectAttribute()

$this->normalizer->normalize($obj, 'any');
}

/**
* @expectedException \Symfony\Component\Serializer\Exception\CircularReferenceException
*/
public function testUnableToNormalizeCircularReference()
{
$serializer = new Serializer(array($this->normalizer));
$this->normalizer->setSerializer($serializer);
$this->normalizer->setCircularReferenceLimit(2);

$obj = new CircularReferenceDummy();

$this->normalizer->normalize($obj);
}

public function testSiblingReference()
{
$serializer = new Serializer(array($this->normalizer));
$this->normalizer->setSerializer($serializer);

$siblingHolder = new SiblingHolder();

$expected = array(
'sibling0' => array('coopTilleuls' => 'Les-Tilleuls.coop'),
'sibling1' => array('coopTilleuls' => 'Les-Tilleuls.coop'),
'sibling2' => array('coopTilleuls' => 'Les-Tilleuls.coop'),
);
$this->assertEquals($expected, $this->normalizer->normalize($siblingHolder));
}

public function testCircularReferenceHandler()
{
$serializer = new Serializer(array($this->normalizer));
$this->normalizer->setSerializer($serializer);
$this->normalizer->setCircularReferenceHandler(function ($obj) {
return get_class($obj);
});

$obj = new CircularReferenceDummy();

$expected = array('me' => 'Symfony\Component\Serializer\Tests\Fixtures\CircularReferenceDummy');
$this->assertEquals($expected, $this->normalizer->normalize($obj));
}
}

class GetSetDummy
Expand Down