Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

setSerialiseNull(true) + exclusion strategies still include data #852

Closed
carnage opened this issue Jan 11, 2018 · 3 comments
Closed

setSerialiseNull(true) + exclusion strategies still include data #852

carnage opened this issue Jan 11, 2018 · 3 comments
Assignees
Milestone

Comments

@carnage
Copy link

carnage commented Jan 11, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? maybe (It worked as expected on a very old version, possibly prior to 1.0.0)
RFC? no

Steps required to reproduce the problem

  1. Use an exclusion strategy to exclude an item from an array/hash
  2. Set serialise null => true
  3. Get extra null values in the array (eg item wasn't fully excluded)

Expected Result

  • Only the items the exclusion strategy allows through are in the serialised result

Actual Result

  • Extra nulls are included in the array/hash
  • (untested) I suspect the same will happen for excluded properties. eg their value will be set null instead of the key being excluded entirely.

Details

The issue occurs here:
https://github.com/schmittjoh/serializer/blob/master/src/JMS/Serializer/GenericSerializationVisitor.php#L111

When the navigator determines an item should be excluded, it returns null for that value. This works as expected if setSerialiseNull is false, however when it's true, the item gets included with a null value.

Solution

The solution here is to have the navigator throw an UnacceptableException (https://github.com/schmittjoh/serializer/blob/master/src/JMS/Serializer/GraphNavigator.php#L225) when it encounters a field which should not be included in the result. That can then be caught in the code linked above and handled eg excluded from the result.

BC preservation

As this will now change behaviour, it may be desirable to include a configuration parameter (eg setIncludeExcludedAsNull()) to enable this new behaviour, defaulting to on. However I consider this behaviour to be a bug and is inconsistent with the behaviour when setSerialiseNull is set to false so I don't think this is required

@fsevestre
Copy link

I'm having the exact same issue after upgrading from 1.1.0 to the latest version. Do you know any workaround ?

@fsevestre
Copy link

fsevestre commented Mar 9, 2018

The issue was introduced by #660 :|

@goetas
Copy link
Collaborator

goetas commented Apr 15, 2018

fixed in #895

@goetas goetas closed this as completed Apr 15, 2018
@goetas goetas added this to the v2.0 milestone Apr 16, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants