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

Behavior serializeNull -> not always honored in 2.* (but was in 1.*) #1101

Closed
narcoticfresh opened this issue Jun 23, 2019 · 4 comments
Closed

Comments

@narcoticfresh
Copy link

narcoticfresh commented Jun 23, 2019

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

Background information

Hi guys..

Over at libgraviton/graviton#720 we are still trying to migrate from serializer 1.* to 2.*. Our project is a JSON REST backend server, so serializer output is paramount for us. We don't want to change what we output after the change to Serializer version update.

We have two main edge case requirements when rendering:

  • Don't render empty objects {}
  • Never render nulls

In Serializer 1.*, serializeNull is honored all the time - this seems to have changed in 2.*.

With 1.*, empty objects still were rendered sometimes, but we worked round that by having special Handlers that the serialize to null - and that didn't render. This doesn't work anymore in 2.*.

Steps required to reproduce the problem

As we use the Serializer in a quite complex configuration, I created a demonstration repository that shows our problem isolated.

Please clone https://github.com/narcoticfresh/serializer2-problem-testcase
Run composer install, then run php test.php

What you will see:

  • An object that has a rendered output of {"members":[{"name":"member1"},{}]} or {"members":[{"name":"member1"},null]}

Why is the empty object (or null) rendered?

Expected Result

As in serializer 1.*, we expect that

  • Not serialize empty objects
  • Not serialize nulls when serializeNull is false

So if we have this setup

  • A field has a type with a handler -> that would serialize to an empty object -> what we don't want to render
  • We have a handler in place which changes the Type to 'EmptyType' when it would be an empty object in the pre_serialize stage => EmptyType is a handler that serializes to null
  • Why is this null value rendered now in version 2.0?

Actual Result

  • In the scenario outlined, nulls are rendered
@goetas
Copy link
Collaborator

goetas commented Jun 23, 2019

Hi, Thanks for reporting this.
I think that I missed to mention this in the UPGRADING notes.
When implementing a custom handler, it should honor the serializeNull settings.

I'm talking as example this, and the 2.x or 3.x version should look like:

use JMS\Serialzier\Exception\NotAcceptableException

// ...
     function serializeExtReferenceToJson(
        JsonSerializationVisitor $visitor,
        ExtReference $extReference,
        array $type,
        Context $context
    ) {
        if (null === $extReference && !$context->shouldSerializeNull()) {
            throw new NotAcceptableException();
        } elseif (null === $extReference) {
            return $visitor->visitNull(null, $type, $context);
        }
        try {
            //return $visitor->visitString($this->converter->getUrl($extReference), $type, $context);
            return $this->converter->getUrl($extReference);
        } catch (\InvalidArgumentException $e) {
          if (!$context->shouldSerializeNull()) {
             throw new NotAcceptableException();
         } else {
            return $visitor->visitNull(null, $type, $context);
         }
        }
    }

Same check should be done for the 'EmptyType' handler.

@goetas
Copy link
Collaborator

goetas commented Jun 23, 2019

In the next days will update the UPGRADING document

@narcoticfresh
Copy link
Author

@goetas
Thanks so much for your reply! Amazing that you found our problematic Handler ;-)

Hm, OK, I see, the NotAcceptableException actually gets catched and null is not rendered indeed.. great! ;-)


If you don't mind me asking, I'm still confused how the rendering of empty arrays has changed from 1.*. In 1.*, those were always rendered (empty arrays deep in the object graph)..

In newer versions, it is a kinda mixed bag. "General" serialized empty arrays are not rendered anymore. But in the context of Handlers, it seems they still are.

I updated my example with your fix and it to render an empty array (members is now null) - diff is here - serializer now renders {"members":[]}

It's not really a bug, but the behavior is a bit inconsistent - any way possibly that members is not rendered at all if it equates to an empty array?

Or in short: Why is an array property that by default is null not generally rendered if it has no members - but an array property with a Handler (that is default null) will be rendered as empty array ([]) if it has no members?

@goetas goetas closed this as completed in 33b473d Jun 24, 2019
@goetas
Copy link
Collaborator

goetas commented Jun 24, 2019

It's not really a bug, but the behavior is a bit inconsistent - any way possibly that members is not rendered at all if it equates to an empty array?

In 2.x arrays have been refactored and the ability to explicitly decide is rendering an empty array has been added via @SkipWhenEmpty annotation.

In newer versions, it is a kinda mixed bag. "General" serialized empty arrays are not rendered anymore. But in the context of Handlers, it seems they still are.

Or in short: Why is an array property that by default is null not generally rendered if it has no members - but an array property with a Handler (that is default null) will be rendered as empty array ([]) if it has no members?

If the changes done to the array handling are inconsistent in your opinion, feel free to submit a failing test case with the expected behavior. Will be happy to check it and see what can be done to make it more consistent. 😃

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