Skip to content

[Serializer] Allow to use easily static constructors #19137

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
Jun 29, 2016
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
25 changes: 23 additions & 2 deletions src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,23 @@ protected function prepareForDenormalization($data)
return (array) $data;
}

/**
* Returns the method to use to construct an object. This method must be either
* the object constructor or static.
*
* @param array $data
* @param string $class
* @param array $context
* @param \ReflectionClass $reflectionClass
* @param array|bool $allowedAttributes
*
* @return \ReflectionMethod|null
*/
protected function getConstructor(array &$data, $class, array &$context, \ReflectionClass $reflectionClass, $allowedAttributes)
{
return $reflectionClass->getConstructor();
}

/**
* Instantiates an object using constructor parameters when needed.
*
Expand Down Expand Up @@ -282,7 +299,7 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
return $object;
}

$constructor = $reflectionClass->getConstructor();
$constructor = $this->getConstructor($data, $class, $context, $reflectionClass, $allowedAttributes);
if ($constructor) {
$constructorParameters = $constructor->getParameters();

Expand Down Expand Up @@ -318,7 +335,11 @@ protected function instantiateObject(array &$data, $class, array &$context, \Ref
}
}

return $reflectionClass->newInstanceArgs($params);
if ($constructor->isConstructor()) {
Copy link
Contributor

@ro0NL ro0NL Jun 26, 2016

Choose a reason for hiding this comment

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

This is breaking and should also be called if $constructor is null. However that throws an exception if the class has no constructor, so what about calling newInstanceWithoutConstructor then?, being the absolute fallback.

Copy link
Member

Choose a reason for hiding this comment

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

if $constructor is null, this code wouldn't be reached because of the if statement on line 303.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hehe. Totally overlooked indenting.

return $reflectionClass->newInstanceArgs($params);
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't any sense for this else.

Copy link
Contributor Author

@GuilhemN GuilhemN Jun 28, 2016

Choose a reason for hiding this comment

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

IMO it's harder to understand this 2 lines without it.

return $constructor->invokeArgs(null, $params);
}
}

return new $class();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?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 StaticConstructorDummy
{
public $foo;
public $bar;
public $quz;

public static function create($foo)
{
$dummy = new self();
$dummy->quz = $foo;

return $dummy;
}

private function __construct()
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<?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\Normalizer\ObjectNormalizer;

/**
* @author Guilhem N. <egetick@gmail.com>
*/
class StaticConstructorNormalizer extends ObjectNormalizer
{
/**
* {@inheritdoc}
*/
protected function getConstructor(array &$data, $class, array &$context, \ReflectionClass $reflectionClass, $allowedAttributes)
{
if (is_a($class, StaticConstructorDummy::class, true)) {
return new \ReflectionMethod($class, 'create');
}

return parent::getConstructor($data, $class, $context, $reflectionClass, $allowedAttributes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use Symfony\Component\Serializer\Normalizer\ObjectNormalizer;
use Symfony\Component\Serializer\Tests\Fixtures\AbstractNormalizerDummy;
use Symfony\Component\Serializer\Tests\Fixtures\ProxyDummy;
use Symfony\Component\Serializer\Tests\Fixtures\StaticConstructorDummy;
use Symfony\Component\Serializer\Tests\Fixtures\StaticConstructorNormalizer;

/**
* Provides a dummy Normalizer which extends the AbstractNormalizer.
Expand Down Expand Up @@ -103,4 +105,14 @@ public function testObjectToPopulateWithProxy()

$this->assertSame('bar', $proxyDummy->getFoo());
}

public function testObjectWithStaticConstructor()
{
$normalizer = new StaticConstructorNormalizer();
$dummy = $normalizer->denormalize(array('foo' => 'baz'), StaticConstructorDummy::class);

$this->assertInstanceOf(StaticConstructorDummy::class, $dummy);
$this->assertEquals('baz', $dummy->quz);
$this->assertNull($dummy->foo);
}
}