Skip to content

[Serializer] Associative arrays with mixed keys can't be denormalised #51751

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

Closed
melya opened this issue Sep 26, 2023 · 1 comment
Closed

[Serializer] Associative arrays with mixed keys can't be denormalised #51751

melya opened this issue Sep 26, 2023 · 1 comment

Comments

@melya
Copy link
Contributor

melya commented Sep 26, 2023

Symfony version(s) affected

6.2.12

Description

Hi!

PHP automatically converts numeric strings used as array keys to integers for optimization purposes.
Therefore, scenario below is not supported.
As symfony "normalizes" <string|int, \DummyInner> to simple \DummyInner[] and takes first type only at

https://github.com/symfony/serializer/blob/6.3/Normalizer/AbstractObjectNormalizer.php#L502-L508

The exception message looks like

Fatal error: Uncaught Symfony\Component\Serializer\Exception\NotNormalizableValueException: The type of the key "str" must be "string" ("int" given). in symfony-serializer-demo/vendor/symfony/serializer/Exception/NotNormalizableValueException.php on line 31

How to reproduce

<?php

declare(strict_types=1);

use Symfony\Component\PropertyInfo\Extractor\PhpDocExtractor;
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
use Symfony\Component\Serializer\Normalizer\ArrayDenormalizer;
use Symfony\Component\Serializer\Normalizer\PropertyNormalizer;
use \Symfony\Component\Serializer\Serializer;

require_once "vendor/autoload.php";

class DummyInner {
    public function __construct(public readonly bool $value)
    {
    }
}

class DummyCollection {
    public function __construct(
        /** @var array<string|int, \DummyInner> */
        public readonly array $fields
    ) {
    }
}

$data = [
    "fields" => [
        "str" => ["value" => true],
        "123" => ["value" => false],
        "0"   => ["value" => false],
    ]
];


$extractor  = new PropertyInfoExtractor([], [new PhpDocExtractor(), new ReflectionExtractor()]);
$serializer = new Serializer(
    [
        new ArrayDenormalizer(),
        new PropertyNormalizer(null, null, $extractor),
    ]
);

$actual   = $serializer->denormalize($data, DummyCollection::class);
$expected = new DummyCollection([
    "str" => new DummyInner(true),
    "123" => new DummyInner(false),
    "0"   => new DummyInner(false),
]);


var_dump($actual == $expected);

Possible Solution

Since the union types aren't supported yet (and we don't know when and if) maybe we could have a workaround for this ?

Not sure if this is the right place for workaround, but at least it works on my scenario

https://github.com/symfony/serializer/blob/6.3/Normalizer/ArrayDenormalizer.php#L47-L75

and we can add "auto-conversion" like

public function denormalize(mixed $data, string $type, string $format = null, array $context = []): array
{
    if (null === $this->denormalizer) {
        throw new BadMethodCallException('Please set a denormalizer before calling denormalize()!');
    }
    if (!\is_array($data)) {
        throw NotNormalizableValueException::createForUnexpectedDataType(sprintf('Data expected to be "%s", "%s" given.', $type, get_debug_type($data)), $data, [Type::BUILTIN_TYPE_ARRAY], $context['deserialization_path'] ?? null);
    }
    if (!str_ends_with($type, '[]')) {
        throw new InvalidArgumentException('Unsupported class: '.$type);
    }

    $type = substr($type, 0, -2);

    $builtinTypes = array_map(static function (Type $keyType) {
        return $keyType->getBuiltinType();
    }, \is_array($keyType = $context['key_type'] ?? []) ? $keyType : [$keyType]);

    foreach ($data as $key => $value) {
        $subContext = $context;
        $subContext['deserialization_path'] = ($context['deserialization_path'] ?? false) ? sprintf('%s[%s]', $context['deserialization_path'], $key) : "[$key]";

+        if ($builtinType === "string" && \is_numeric($key)) {
+            $key = (string)$key;
+        }

        $this->validateKeyType($builtinTypes, $key, $subContext['deserialization_path']);

        $data[$key] = $this->denormalizer->denormalize($value, $type, $format, $subContext);
    }

    return $data;
}
@melya
Copy link
Contributor Author

melya commented Sep 26, 2023

Already fixed in #50933
Closing

@melya melya closed this as completed Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants