Skip to content
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

Redundant denormalize call in SerializerTrait #127

Open
Aweptimum opened this issue Aug 28, 2023 · 4 comments
Open

Redundant denormalize call in SerializerTrait #127

Aweptimum opened this issue Aug 28, 2023 · 4 comments

Comments

@Aweptimum
Copy link

Aweptimum commented Aug 28, 2023

I've been trying to debug the deserialization from a database of a vector object. While stepping through a unit test, I noticed that the property of another entity typed as a json_document column in the test was being treated as an array of objects. The requested type inside the Serializer is of App\MyDTO[], but the #type key is set to App\MyDTO in the database. Still, it is somehow being deserialized properly as a single object, while each element in my vector object's array is being treated as a vector object and failing.

I tracked down the appended [] to line 82 in the SerializerTrait

https://github.com/dunglas/doctrine-json-odm/blob/89b01483c4f6e7a5e25fc2ea18b2fe46574617ae/src/SerializerTrait.php#L70C5-L94C6

(not sure why the embedded snippet won't render)

The extra call to $this->denormalize on 82 supplies $data back to itself, but the #type key has been unset. So in that recursive denormalize call, it skips down to the is_iterable check, which evaluates to true because $data is an array. It then appends [] to the $type and delegates to the Symfony Serializer. It, in turn, delegates to the ArrayDenormalizer because [] has been appended.

I'm not sure why the data at times comes back fine as a single object and others do not.

I think removing line 82 altogether would solve the issue - it seems like parent::denormalize would be all that is needed to correctly return the denormalized data. As a test, I commented out 82 in my project, and the deserialization process actually works now for my vector type while also making less calls to other normalizers.

@Aweptimum
Copy link
Author

I forked it, made the change, and ran the test suite. It seems removing it breaks the ::nestedObjects tests as well as enums:

There were 3 failures:

1) Dunglas\DoctrineJsonOdm\Tests\FunctionalTest::testNestedObjects
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
         'key' => 'parent'
         'value' => Array (
             0 => Array (
-                0 => Dunglas\DoctrineJsonOdm\Tests\Fixtures\TestBundle\Document\Attribute Object (...)
+                0 => Array (...)
             )
         )
     )
 )

doctrine-json-odm-redundant\tests\FunctionalTest.php:117

2) Dunglas\DoctrineJsonOdm\Tests\FunctionalTest::testStoreAndRetrieveEnum
Failed asserting that Array &0 (
    '#type' => 'Dunglas\DoctrineJsonOdm\Tests\Fixtures\TestBundle\Enum\InputMode'
    '#scalar' => 'email'
) is identical to an object of class "Dunglas\DoctrineJsonOdm\Tests\Fixtures\TestBundle\Enum\InputMode".

doctrine-json-odm-redundant\tests\FunctionalTest.php:219

3) Dunglas\DoctrineJsonOdm\Tests\SerializerTest::testNestedObjects
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
         'key' => 'parent'
         'value' => Array (
             0 => Array (
-                0 => Dunglas\DoctrineJsonOdm\Tests\Fixtures\TestBundle\Document\Attribute Object (...)
+                0 => Array (...)
             )
         )
     )
 )
doctrine-json-odm-redundant\tests\SerializerTest.php:88

@Aweptimum
Copy link
Author

Aweptimum commented Oct 27, 2023

I had a moment of realization - the class I'm trying to serialize implements the \Iterator interface and I think this makes it pass the is_iterable check in SerializerTrait->denormalize when it shouldn't. I added a test case in #128 that demonstrates the issue

@Arkemlar
Copy link

Well, this thing brokes my wishes to use that library, so sad.

@Aweptimum
Copy link
Author

Aweptimum commented Aug 26, 2024

@Arkemlar you can create a custom normalizer for your iterable type
In my Symfony 6.2 project it looked roughly like this:

class VectorNormalizer implements NormalizerInterface
{
    function supportsNormalization($object)
    {
        return $object instance of Vector;
    }
    
    function normalize($object)
    {
        return $object->getArray();
    }
}

You don't need a custom denormalizer so long as normalize returns data in the format the constructor expects (an array in my case)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants