Skip to content

[Serializer] Give access to the context to support* methods #19371

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.

Sign up for GitHub

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
wants to merge 6 commits into from
Closed

[Serializer] Give access to the context to support* methods #19371

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7a4e9ec
[Serializer] Give access to the context to support* methods
dunglas Jul 16, 2016
1de7609
Remove commented code
dunglas Oct 25, 2016
f6d4479
Revert "Remove commented code"
dunglas Jan 28, 2017
3bf4bf1
Fix CS
dunglas Jan 28, 2017
f70762a
Better deprecations
dunglas Jan 28, 2017
661005e
Remove useless checks
dunglas Feb 13, 2017
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
15 changes: 9 additions & 6 deletions src/Symfony/Component/Serializer/Encoder/ChainDecoder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
* @final since version 3.3.
*/
class ChainDecoder implements DecoderInterface
class ChainDecoder implements DecoderInterface /*, ContextAwareDecoderInterface*/
{
protected $decoders = array();
protected $decoderByFormat = array();
Expand All @@ -37,16 +37,18 @@ public function __construct(array $decoders = array())
*/
final public function decode($data, $format, array $context = array())
{
return $this->getDecoder($format)->decode($data, $format, $context);
return $this->getDecoder($format, $context)->decode($data, $format, $context);
}

/**
* {@inheritdoc}
*/
public function supportsDecoding($format)
public function supportsDecoding($format/*, array $context = array()*/)
Copy link
Member

Choose a reason for hiding this comment

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

We can't do this, uncommenting in 4.0: that would break code without any warning beforehand.
I'd say:

  • either we uncomment now (playing scratchy with our BC policy, but is it likely that this class could be extended?)
  • or trigger a deprecation when the argument is not defined on the derivated class (but look at the boiler plate to do this :) )

Copy link
Member Author

Choose a reason for hiding this comment

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

It's is very unlikely that some extended this class (I never seen this at least).

Copy link
Contributor

Choose a reason for hiding this comment

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

Or third option, deprecate extending this class?

Copy link
Member

@nicolas-grekas nicolas-grekas Sep 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I'd personally be ok for 1. and uncomment all the currently commented code

Copy link
Member Author

Choose a reason for hiding this comment

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

if we uncomment this code, we can also remove the trick with func_num_args().

Copy link
Member

Choose a reason for hiding this comment

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

and remove the double interface yes :)

{
$context = func_num_args() > 1 ? func_get_arg(1) : array();

try {
$this->getDecoder($format);
$this->getDecoder($format, $context);
} catch (RuntimeException $e) {
return false;
}
Expand All @@ -58,12 +60,13 @@ public function supportsDecoding($format)
* Gets the decoder supporting the format.
*
* @param string $format
* @param array $context
*
* @return DecoderInterface
*
* @throws RuntimeException If no decoder is found.
*/
private function getDecoder($format)
private function getDecoder($format, array $context)
{
if (isset($this->decoderByFormat[$format])
&& isset($this->decoders[$this->decoderByFormat[$format]])
Expand All @@ -72,7 +75,7 @@ private function getDecoder($format)
}

foreach ($this->decoders as $i => $decoder) {
if ($decoder->supportsDecoding($format)) {
if ($decoder->supportsDecoding($format, $context)) {
$this->decoderByFormat[$format] = $i;

return $decoder;
Expand Down
23 changes: 14 additions & 9 deletions src/Symfony/Component/Serializer/Encoder/ChainEncoder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
* @final since version 3.3.
*/
class ChainEncoder implements EncoderInterface
class ChainEncoder implements EncoderInterface /*, ContextAwareEncoderInterface*/
{
protected $encoders = array();
protected $encoderByFormat = array();
Expand All @@ -37,16 +37,18 @@ public function __construct(array $encoders = array())
*/
final public function encode($data, $format, array $context = array())
{
return $this->getEncoder($format)->encode($data, $format, $context);
return $this->getEncoder($format, $context)->encode($data, $format, $context);
}

/**
* {@inheritdoc}
*/
public function supportsEncoding($format)
public function supportsEncoding($format/*, array $context = array()*/)
{
$context = func_num_args() > 1 ? func_get_arg(1) : array();

try {
$this->getEncoder($format);
$this->getEncoder($format, $context);
} catch (RuntimeException $e) {
return false;
}
Expand All @@ -58,19 +60,21 @@ public function supportsEncoding($format)
* Checks whether the normalization is needed for the given format.
*
* @param string $format
* @param array $context
*
* @return bool
*/
public function needsNormalization($format)
public function needsNormalization($format/*, array $context = array()*/)
{
$encoder = $this->getEncoder($format);
$context = func_num_args() > 1 ? func_get_arg(1) : array();
$encoder = $this->getEncoder($format, $context);

if (!$encoder instanceof NormalizationAwareInterface) {
return true;
}

if ($encoder instanceof self) {
return $encoder->needsNormalization($format);
return $encoder->needsNormalization($format, $context);
}

return false;
Expand All @@ -80,12 +84,13 @@ public function needsNormalization($format)
* Gets the encoder supporting the format.
*
* @param string $format
* @param array $context
*
* @return EncoderInterface
*
* @throws RuntimeException if no encoder is found
*/
private function getEncoder($format)
private function getEncoder($format, array $context)
{
if (isset($this->encoderByFormat[$format])
&& isset($this->encoders[$this->encoderByFormat[$format]])
Expand All @@ -94,7 +99,7 @@ private function getEncoder($format)
}

foreach ($this->encoders as $i => $encoder) {
if ($encoder->supportsEncoding($format)) {
if ($encoder->supportsEncoding($format, $context)) {
$this->encoderByFormat[$format] = $i;

return $encoder;
Expand Down
27 changes: 27 additions & 0 deletions src/Symfony/Component/Serializer/Encoder/ContextAwareDecoderInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?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\Encoder;

/**
* Adds the support of an extra $context parameter for the supportsDecoding method.
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
interface ContextAwareDecoderInterface extends DecoderInterface
Copy link
Contributor

@ogizanagi ogizanagi Sep 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think it can work by extending the old interface or am I missing something ? 😕

Copy link
Member Author

Choose a reason for hiding this comment

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

It works if the argument is optional: https://3v4l.org/UiviL
I wasn't aware of that (thanks @nicolas-grekas for the hint) but it's practical here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great :)

{
/**
* {@inheritdoc}
*
* @param array $context options that decoders have access to
*/
public function supportsDecoding($format, array $context = array());
}
27 changes: 27 additions & 0 deletions src/Symfony/Component/Serializer/Encoder/ContextAwareEncoderInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?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\Encoder;

/**
* Adds the support of an extra $context parameter for the supportsEncoding method.
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
interface ContextAwareEncoderInterface extends EncoderInterface
{
/**
* {@inheritdoc}
*
* @param array $context options that encoders have access to
*/
public function supportsEncoding($format, array $context = array());
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ private function validateAndDenormalize($currentClass, $attribute, $data, $forma
throw new LogicException(sprintf('Cannot denormalize attribute "%s" for class "%s" because injected serializer is not a denormalizer', $attribute, $class));
}

if ($this->serializer->supportsDenormalization($data, $class, $format)) {
if ($this->serializer->supportsDenormalization($data, $class, $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.

@dunglas Can you check that my conflict resolution is the right one. If not, can you fix it directly on master? Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

@fabpot fixed in b5d9b3b.

Copy link
Member

Choose a reason for hiding this comment

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

thanks

return $this->serializer->denormalize($data, $class, $format, $context);
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/Symfony/Component/Serializer/Normalizer/ArrayDenormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
* Denormalizes arrays of objects.
*
* @author Alexander M. Turek <me@derrabus.de>
*
* @final since version 3.3.
*/
class ArrayDenormalizer implements DenormalizerInterface, SerializerAwareInterface
{
Expand Down Expand Up @@ -64,10 +66,12 @@ public function denormalize($data, $class, $format = null, array $context = arra
/**
* {@inheritdoc}
*/
public function supportsDenormalization($data, $type, $format = null)
public function supportsDenormalization($data, $type, $format = null/*, array $context = array()*/)
{
$context = func_num_args() > 3 ? func_get_arg(3) : array();
Copy link
Member

Choose a reason for hiding this comment

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

Why not uncommenting the argument? (Same at other places)

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as before, adding a new argument to a public method isn't allowed by our BC policy.


return substr($type, -2) === '[]'
&& $this->serializer->supportsDenormalization($data, substr($type, 0, -2), $format);
&& $this->serializer->supportsDenormalization($data, substr($type, 0, -2), $format, $context);
}

/**
Expand Down
27 changes: 27 additions & 0 deletions src/Symfony/Component/Serializer/Normalizer/ContextAwareDenormalizerInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?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\Normalizer;

/**
* Adds the support of an extra $context parameter for the supportsDenormalization method.
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
interface ContextAwareDenormalizerInterface extends DenormalizerInterface
{
/**
* {@inheritdoc}
*
* @param array $context options that denormalizers have access to
*/
public function supportsDenormalization($data, $type, $format = null, array $context = array());
}
27 changes: 27 additions & 0 deletions src/Symfony/Component/Serializer/Normalizer/ContextAwareNormalizerInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?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\Normalizer;

/**
* Adds the support of an extra $context parameter for the supportsNormalization method.
*
* @author Kévin Dunglas <dunglas@gmail.com>
*/
interface ContextAwareNormalizerInterface extends NormalizerInterface
{
/**
* {@inheritdoc}
*
* @param array $context options that normalizers have access to
*/
public function supportsNormalization($data, $format = null, array $context = array());
}
Loading