Skip to content

[Serializer] throw more specific exceptions #21239

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
Sep 27, 2017
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
6 changes: 3 additions & 3 deletions src/Symfony/Component/Serializer/Encoder/JsonDecode.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace Symfony\Component\Serializer\Encoder;

use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotEncodableValueException;

/**
* Decodes JSON data.
Expand Down Expand Up @@ -73,7 +73,7 @@ public function __construct($associative = false, $depth = 512)
*
* @return mixed
*
* @throws UnexpectedValueException
* @throws NotEncodableValueException
*
* @see http://php.net/json_decode json_decode
*/
Expand All @@ -88,7 +88,7 @@ public function decode($data, $format, array $context = array())
$decodedData = json_decode($data, $associative, $recursionDepth, $options);

if (JSON_ERROR_NONE !== $this->lastError = json_last_error()) {
throw new UnexpectedValueException(json_last_error_msg());
throw new NotEncodableValueException(json_last_error_msg());
}

return $decodedData;
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Component/Serializer/Encoder/JsonEncode.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace Symfony\Component\Serializer\Encoder;

use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotEncodableValueException;

/**
* Encodes JSON data.
Expand Down Expand Up @@ -40,7 +40,7 @@ public function encode($data, $format, array $context = array())
$encodedJson = json_encode($data, $context['json_encode_options']);

if (JSON_ERROR_NONE !== $this->lastError = json_last_error()) {
throw new UnexpectedValueException(json_last_error_msg());
throw new NotEncodableValueException(json_last_error_msg());
}

return $encodedJson;
Expand Down
14 changes: 7 additions & 7 deletions src/Symfony/Component/Serializer/Encoder/XmlEncoder.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

namespace Symfony\Component\Serializer\Encoder;

use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotEncodableValueException;

/**
* Encodes XML data.
Expand Down Expand Up @@ -78,7 +78,7 @@ public function encode($data, $format, array $context = array())
public function decode($data, $format, array $context = array())
{
if ('' === trim($data)) {
throw new UnexpectedValueException('Invalid XML data, it can not be empty.');
throw new NotEncodableValueException('Invalid XML data, it can not be empty.');
}

$internalErrors = libxml_use_internal_errors(true);
Expand All @@ -94,13 +94,13 @@ public function decode($data, $format, array $context = array())
if ($error = libxml_get_last_error()) {
libxml_clear_errors();

throw new UnexpectedValueException($error->message);
throw new NotEncodableValueException($error->message);
}

$rootNode = null;
foreach ($dom->childNodes as $child) {
if (XML_DOCUMENT_TYPE_NODE === $child->nodeType) {
throw new UnexpectedValueException('Document types are not allowed.');
throw new NotEncodableValueException('Document types are not allowed.');
}
if (!$rootNode && XML_PI_NODE !== $child->nodeType) {
$rootNode = $child;
Expand Down Expand Up @@ -380,7 +380,7 @@ private function parseXmlValue(\DOMNode $node, array $context = array())
*
* @return bool
*
* @throws UnexpectedValueException
* @throws NotEncodableValueException
*/
private function buildXml(\DOMNode $parentNode, $data, $xmlRootNodeName = null)
{
Expand Down Expand Up @@ -437,7 +437,7 @@ private function buildXml(\DOMNode $parentNode, $data, $xmlRootNodeName = null)
return $this->appendNode($parentNode, $data, 'data');
}

throw new UnexpectedValueException(sprintf('An unexpected value could not be serialized: %s', var_export($data, true)));
throw new NotEncodableValueException(sprintf('An unexpected value could not be serialized: %s', var_export($data, true)));
}

/**
Expand Down Expand Up @@ -485,7 +485,7 @@ private function needsCdataWrapping($val)
*
* @return bool
*
* @throws UnexpectedValueException
* @throws NotEncodableValueException
*/
private function selectNodeType(\DOMNode $node, $val)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?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;

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
*/
class NotEncodableValueException extends UnexpectedValueException
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?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;

/**
* @author Christian Flothmann <christian.flothmann@sensiolabs.de>
*/
class NotNormalizableValueException extends UnexpectedValueException
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use Symfony\Component\Serializer\Encoder\JsonEncoder;
use Symfony\Component\Serializer\Exception\ExtraAttributesException;
use Symfony\Component\Serializer\Exception\LogicException;
use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface;
use Symfony\Component\PropertyInfo\Type;
use Symfony\Component\Serializer\Mapping\AttributeMetadataInterface;
Expand Down Expand Up @@ -200,7 +200,7 @@ public function denormalize($data, $class, $format = null, array $context = arra
try {
$this->setAttributeValue($object, $attribute, $value, $format, $context);
} catch (InvalidArgumentException $e) {
throw new UnexpectedValueException($e->getMessage(), $e->getCode(), $e);
throw new NotNormalizableValueException($e->getMessage(), $e->getCode(), $e);
}
}

Expand Down Expand Up @@ -233,7 +233,7 @@ abstract protected function setAttributeValue($object, $attribute, $value, $form
*
* @return mixed
*
* @throws UnexpectedValueException
* @throws NotNormalizableValueException
* @throws LogicException
*/
private function validateAndDenormalize($currentClass, $attribute, $data, $format, array $context)
Expand Down Expand Up @@ -292,7 +292,7 @@ private function validateAndDenormalize($currentClass, $attribute, $data, $forma
return $data;
}

throw new UnexpectedValueException(sprintf('The type of the "%s" attribute for class "%s" must be one of "%s" ("%s" given).', $attribute, $currentClass, implode('", "', array_keys($expectedTypes)), gettype($data)));
throw new NotNormalizableValueException(sprintf('The type of the "%s" attribute for class "%s" must be one of "%s" ("%s" given).', $attribute, $currentClass, implode('", "', array_keys($expectedTypes)), gettype($data)));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

use Symfony\Component\Serializer\Exception\BadMethodCallException;
use Symfony\Component\Serializer\Exception\InvalidArgumentException;
use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
use Symfony\Component\Serializer\SerializerAwareInterface;
use Symfony\Component\Serializer\SerializerInterface;

Expand All @@ -33,6 +33,8 @@ class ArrayDenormalizer implements DenormalizerInterface, SerializerAwareInterfa

/**
* {@inheritdoc}
*
* @throws NotNormalizableValueException
*/
public function denormalize($data, $class, $format = null, array $context = array())
{
Expand All @@ -52,7 +54,7 @@ public function denormalize($data, $class, $format = null, array $context = arra
$builtinType = isset($context['key_type']) ? $context['key_type']->getBuiltinType() : null;
foreach ($data as $key => $value) {
if (null !== $builtinType && !call_user_func('is_'.$builtinType, $key)) {
throw new UnexpectedValueException(sprintf('The type of the key "%s" must be "%s" ("%s" given).', $key, $builtinType, gettype($key)));
throw new NotNormalizableValueException(sprintf('The type of the key "%s" must be "%s" ("%s" given).', $key, $builtinType, gettype($key)));
}

$data[$key] = $serializer->denormalize($value, $class, $format, $context);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesser;
use Symfony\Component\HttpFoundation\File\MimeType\MimeTypeGuesserInterface;
use Symfony\Component\Serializer\Exception\InvalidArgumentException;
use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;

/**
* Normalizes an {@see \SplFileInfo} object to a data URI.
Expand Down Expand Up @@ -85,11 +85,14 @@ public function supportsNormalization($data, $format = null)
* Regex adapted from Brian Grinstead code.
*
* @see https://gist.github.com/bgrins/6194623
*
* @throws InvalidArgumentException
* @throws NotNormalizableValueException
*/
public function denormalize($data, $class, $format = null, array $context = array())
{
if (!preg_match('/^data:([a-z0-9][a-z0-9\!\#\$\&\-\^\_\+\.]{0,126}\/[a-z0-9][a-z0-9\!\#\$\&\-\^\_\+\.]{0,126}(;[a-z0-9\-]+\=[a-z0-9\-]+)?)?(;base64)?,[a-z0-9\!\$\&\\\'\,\(\)\*\+\,\;\=\-\.\_\~\:\@\/\?\%\s]*\s*$/i', $data)) {
throw new UnexpectedValueException('The provided "data:" URI is not valid.');
throw new NotNormalizableValueException('The provided "data:" URI is not valid.');
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO this example here should actually continue to throw the old exception. This is decodeable, but just not valid in the domain. I guess this example would be a 422 instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

But the same is true for the other places where we throw a NotNormalizableValueException in this PR, isn't it?

}

try {
Expand All @@ -102,7 +105,7 @@ public function denormalize($data, $class, $format = null, array $context = arra
return new \SplFileObject($data);
}
} catch (\RuntimeException $exception) {
throw new UnexpectedValueException($exception->getMessage(), $exception->getCode(), $exception);
throw new NotNormalizableValueException($exception->getMessage(), $exception->getCode(), $exception);
}

throw new InvalidArgumentException(sprintf('The class parameter "%s" is not supported. It must be one of "SplFileInfo", "SplFileObject" or "Symfony\Component\HttpFoundation\File\File".', $class));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
namespace Symfony\Component\Serializer\Normalizer;

use Symfony\Component\Serializer\Exception\InvalidArgumentException;
use Symfony\Component\Serializer\Exception\UnexpectedValueException;
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;

/**
* Normalizes an object implementing the {@see \DateTimeInterface} to a date string.
Expand Down Expand Up @@ -79,7 +79,7 @@ public function supportsNormalization($data, $format = null)
/**
* {@inheritdoc}
*
* @throws UnexpectedValueException
* @throws NotNormalizableValueException
*/
public function denormalize($data, $class, $format = null, array $context = array())
{
Expand All @@ -100,7 +100,7 @@ public function denormalize($data, $class, $format = null, array $context = arra

$dateTimeErrors = \DateTime::class === $class ? \DateTime::getLastErrors() : \DateTimeImmutable::getLastErrors();

throw new UnexpectedValueException(sprintf(
throw new NotNormalizableValueException(sprintf(
Copy link
Member

@dunglas dunglas Jan 11, 2017

Choose a reason for hiding this comment

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

Maybe can we set a specific exception code depending of the cause to be able to easily detect if it's an invalid data: URI, or an invalid date and so on?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have thought about that too. The reason I did not add it is consistency. We would only be able to add codes for Symfony's own normalizer. However, it's quite common to add your own normalizers in libraries/projects. As a consequence exception codes would only be useful when they are issued by built-in normalizers. I don't really like that. What do you think?

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 maybe reserve a range for app codes?

'Parsing datetime string "%s" using format "%s" resulted in %d errors:'."\n".'%s',
$data,
$dateTimeFormat,
Expand All @@ -112,7 +112,7 @@ public function denormalize($data, $class, $format = null, array $context = arra
try {
return \DateTime::class === $class ? new \DateTime($data, $timezone) : new \DateTimeImmutable($data, $timezone);
} catch (\Exception $e) {
throw new UnexpectedValueException($e->getMessage(), $e->getCode(), $e);
throw new NotNormalizableValueException($e->getMessage(), $e->getCode(), $e);
}
}

Expand Down
15 changes: 9 additions & 6 deletions src/Symfony/Component/Serializer/Serializer.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
use Symfony\Component\Serializer\Encoder\ChainEncoder;
use Symfony\Component\Serializer\Encoder\EncoderInterface;
use Symfony\Component\Serializer\Encoder\DecoderInterface;
use Symfony\Component\Serializer\Exception\NotEncodableValueException;
use Symfony\Component\Serializer\Exception\NotNormalizableValueException;
use Symfony\Component\Serializer\Normalizer\DenormalizerAwareInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use Symfony\Component\Serializer\Exception\LogicException;
use Symfony\Component\Serializer\Exception\UnexpectedValueException;

/**
* Serializer serializes and deserializes data.
Expand Down Expand Up @@ -108,7 +109,7 @@ public function __construct(array $normalizers = array(), array $encoders = arra
final public function serialize($data, $format, array $context = array())
{
if (!$this->supportsEncoding($format)) {
throw new UnexpectedValueException(sprintf('Serialization for the format %s is not supported', $format));
throw new NotEncodableValueException(sprintf('Serialization for the format %s is not supported', $format));
}

if ($this->encoder->needsNormalization($format)) {
Expand All @@ -124,7 +125,7 @@ final public function serialize($data, $format, array $context = array())
final public function deserialize($data, $type, $format, array $context = array())
{
if (!$this->supportsDecoding($format)) {
throw new UnexpectedValueException(sprintf('Deserialization for the format %s is not supported', $format));
throw new NotEncodableValueException(sprintf('Deserialization for the format %s is not supported', $format));
}

$data = $this->decode($data, $format, $context);
Expand Down Expand Up @@ -160,14 +161,16 @@ public function normalize($data, $format = null, array $context = array())
throw new LogicException('You must register at least one normalizer to be able to normalize objects.');
}

throw new UnexpectedValueException(sprintf('Could not normalize object of type %s, no supporting normalizer found.', get_class($data)));
throw new NotNormalizableValueException(sprintf('Could not normalize object of type %s, no supporting normalizer found.', get_class($data)));
}

throw new UnexpectedValueException(sprintf('An unexpected value could not be normalized: %s', var_export($data, true)));
throw new NotNormalizableValueException(sprintf('An unexpected value could not be normalized: %s', var_export($data, true)));
}

/**
* {@inheritdoc}
*
* @throws NotNormalizableValueException
*/
public function denormalize($data, $type, $format = null, array $context = array())
{
Expand All @@ -179,7 +182,7 @@ public function denormalize($data, $type, $format = null, array $context = array
return $normalizer->denormalize($data, $type, $format, $context);
}

throw new UnexpectedValueException(sprintf('Could not denormalize object of type %s, no supporting normalizer found.', $type));
throw new NotNormalizableValueException(sprintf('Could not denormalize object of type %s, no supporting normalizer found.', $type));
}

/**
Expand Down