Add control to deserialization of null values

[fghirlanda]

Q	A
Bug report?	no
Feature request?	yes
BC Break report?	no
RFC?	no

Given a class which has a collection or an array, like for instance:

   /**
     * @var ArrayCollection
     * @JMS\Type("ArrayCollection<MyObject>")
     */
    protected $collection;

    public function __construct()
    {
        $this->collection = new ArrayCollection();
    }

and serialized data such as ['collection' => null], what currently happens when the data is deserialized, is that the property $collection becomes null rather than, for instance, an empty collection.

It would be nice to be able to control the value which is being set when a null input value is found.
Some possible solutions :

• use the DeserializationContext to determine whether to set the value to null or not
• delegate the decision to the handlers
• use the annotations (either per class or property) to specify callbacks or default values to use in this case, or do that per object type

Please let us know your thoughts and we would like to contribute.

Thank you!

[goetas]

I guess here a custom setter here should already solve the issue

/**
 * @var ArrayCollection
 * @JMS\Type("ArrayCollection<MyObject>")
 * @Accessor(setter="setCollection")
 */
protected $collection;

public function __construct()
{
    $this->collection = new ArrayCollection();
}
public function setCollection($value)
{
    $this->collection = $value instanceof ArrayCollection? $value : new ArrayCollection();
}

Am I missing something?

[fghirlanda]

@goetas , thank you for your reply.

Yes, that would obviously work, but it would mean a check in each setter for each single variable which is of that type(s).

[goetas]

use the DeserializationContext to determine whether to set the value to null or not
delegate the decision to the handlers

Delegating it to handlers technically might work, but will make more complex the handler logic as they will have to handle explicitly the null/not-null values (by some configuration that will need to be exposed somehow)

use the annotations (either per class or property) to specify callbacks or default values to use in this case, or do that per object type

callbacks are exactly what is my solution proposed before. default values will not work as they need some logic (constructor args as example) and to do so we will need some code to be somewhere... that will end-up in a callback...

[fghirlanda]

The difference is that using a callback this way we would need as many callbacks as properties of that type.
A callback that would return an empty ArrayCollection (for instance) on the other hand could be shared among all the properties who need it.

Default values can already be set via the constructor setting the ObjectConstructor in the SerializerBuilder.
Being then simply able to skip overwriting them with null values would allow us to keep the default values.

[goetas]

hmm... something as

@Accessor(setter="expr(service('my_setter_service').myMethod(value))")

might be a nice idea. what do you think?

[fghirlanda]

To be very honest this seems to me like a bit of misuse of the @Accessor annotation.
I would rather introduce a new annotation or, just for the sake of simplicity, add a flag in the DeserializationContext, similar to what is already available when serializing.
Neither of the solutions would introduce breaking changes.

[goetas]

What do you want to add in the DeserializationContext ? probably I'm not understanding your solution as inside it you have to encode some logic (and DeserializationContext is not meant for it)

Can you make me an example?

[fghirlanda]

What currently happens in the GenericDeserializationVisitor, in visitProperty is this:

$v = $data[$name] !== null ? $this->navigator->accept($data[$name], $metadata->type, $context) : null;
$this->accessor->setValue($this->currentObject, $v, $metadata);

That is, if the provided data is null, the value is always set to null.

The point is that in that method we have access to the Context.
So, assuming we add a flag like skipDeserializeNull (obviously defaulted to false), we could do this when $v is null:

if ($context->skipDeserializeNull() === false) {
            $this->accessor->setValue($this->currentObject, $v, $metadata);
}

This way, if we have already set default values via the constructor, they would be untouched.

What do you think?

[goetas]

This way, if we have already set default values via the constructor, they would be untouched.

sorry to disappoint you but the constructor is never called by the jms serializer. the object instantiation is done via reflection

[goetas]

see http://www.doctrine-project.org/2010/03/21/doctrine-2-give-me-my-constructor-back.html

[fghirlanda]

I know the constructor is not called, but in the SerializerBuilder it is possible to set the ObjectConstructor. and what we currently inject is a SimpleObjectConstructor which calls the constructor.

[goetas]

Calling the constructor in the serialization process is a bad idea. the constructor should be called only in your business logic

[fghirlanda]

The constructor is called in our implementation of the ObjectConstructorInterface, maybe my explanation was not clear :)

[goetas]

skipDeserializeNull looks a bad idea to me as it requires other components to be in place to work (your custom constructor in this case).

expression language inside a custom accessor might look an abuse but imo has less side-effects (it will work just as a callback before the "set")

[fghirlanda]

I don't understand how having more options can be a problem :)
The fact that you can use it in conjunction with a custom constructor (something which is already in place) does not mean you have to.

Besides, I think the skipDeserializeNull flag is actually useful on its own, because the same problem arises if you have properties with a default value, such as:

class MyClass{
    /**
     * @var int
     * @JMS\Type("int")
     */
    protected $myProperty = 123;
}

[goetas]

The problem is the delicate balance between features and stability, obtained via options and constraints... but i see your point here

@schmittjoh what is your opinion in this case?

[InfopactMLoos]

There is a much cleaner solution (in my opinion). The real problem is that the serializer skips the constructor because it uses reflection to set properties. Using the context 'target' we can set a empty object as a starting point which ofcourse goes through the constructor during instantiation. Then when the serializer fills the object it will ignore the null value and keep the objects original constructor value.

• Create InitializedObjectConstructor class

<?php

/*
 * Copyright 2016 Johannes M. Schmitt <[email protected]>
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

namespace MyNamespace\Serializer;

use JMS\Serializer\VisitorInterface;
use JMS\Serializer\Metadata\ClassMetadata;
use JMS\Serializer\DeserializationContext;
use JMS\Serializer\Construction\ObjectConstructorInterface;

/**
 * Object constructor that allows deserialization into already constructed
 * objects passed through the deserialization context
 */
class InitializedObjectConstructor implements ObjectConstructorInterface
{
    private $fallbackConstructor;

    /**
     * Constructor.
     *
     * @param ObjectConstructorInterface $fallbackConstructor Fallback object constructor
     */
    public function __construct(ObjectConstructorInterface $fallbackConstructor)
    {
        $this->fallbackConstructor = $fallbackConstructor;
    }

    /**
     * {@inheritdoc}
     */
    public function construct(VisitorInterface $visitor, ClassMetadata $metadata, $data, array $type, DeserializationContext $context)
    {
        if ($context->attributes->containsKey('target') && $context->getDepth() === 1) {
            return $context->attributes->get('target')->get();
        }

        return $this->fallbackConstructor->construct($visitor, $metadata, $data, $type, $context);
    }

}

• Configure and use serializer

use JMS\Serializer\SerializerBuilder;
use JMS\Serializer\Construction\UnserializeObjectConstructor;
use JMS\Serializer\DeserializationContext;
use MyNamespace\InitializedObjectConstructor;

$fallback = new UnserializeObjectConstructor();
$objectConstructor = new InitializedObjectConstructor($fallback);
$serializer = SerializerBuilder::create()
                ->setObjectConstructor($objectConstructor)
                ->build();
 
$context = DeserializationContext::create();
$context->attributes->set('target', new MyClass());
$object = $serializer->deserialize($requestBody, MyClass, 'json', $context);

edited, excuse my late-night-writing forgetfulness, I have added the missing configuration and custom class

[etki]

It is quite funny that deserializing array of nulls has opposite effect (nulls are never preserved):

$serializer = \JMS\Serializer\SerializerBuilder::create()->build();
var_dump($serializer->fromArray(['item' => null], 'array<string, stdClass>'));

array(1) {
  'item' =>
  class stdClass#53 (0) {
  }
}

As you probably already have guessed, my needs are straight opposite to that (i would like to preserve nulls)

[goetas]

fixed in 94c319d

[guilliamxavier]

@goetas Please can you explain how your commit, which only affects Serialization, is supposed to have fixed the present issue, which is all about Deserialization? For instance, the code of @etki just above (version 1.10.0 at the time) is still giving the same result today in version 2.3.0

[goetas]

Hmm, you might be right. can #1005 be a solution for it?

[guilliamxavier]

@goetas Thanks for your work on this lib =) #1005 indeed seems related (but I can't speak for others); for example, it would give

$serializer = \JMS\Serializer\SerializerBuilder::create()->build();
var_dump($serializer->fromArray(['item' => null], 'array<string, stdClass>'));

array(1) {
  'item' =>
  NULL
}

and

$serializer = \JMS\Serializer\SerializerBuilder::create()->build();
$dCtx = \JMS\Serializer\DeserializationContext::create()->setDeserializeNull(true);
var_dump($serializer->fromArray(['item' => null], 'array<string, stdClass>', $dCtx));

array(1) {
  'item' =>
  class stdClass#53 (0) {
  }
}

(don't know for the ArrayCollection though)

[goetas]

Not sure it #1005 is exactly about that.
I do not have a need of #1005 but if you want to work on it, feel free to continue on #1005 and if the result will lead somewhere, we can add it to the jms core