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

is_string check on array key when serializeNulls is needed. #36

Closed
wants to merge 3 commits into from

Conversation

TheBenjiManCan
Copy link

Hi, I had a situation where I needed to serialize an Array of Arrays.

For example
[
["a","b","c"],
[4,null,5],
[1,2,3]
]

Even with serialize nulls configured, it would still strip the null out of the 2nd array.

This was due to the is_string check on the array key (in this case, the number 2) causing the if statement to be true despite the requirement to serialise nulls.

I could not think of a reason for the is_string check on the array key, so I removed it, resolving my issue.

Please advise the intended purpose of the is_string check ? And how I might modify the code to incorporate the scenario of numeric array keys ?
If you agree with what I've done, please accept.

@TheBenjiManCan
Copy link
Author

I'm going to write a unit test to demonstrate. please standby...

@TheBenjiManCan
Copy link
Author

I have committed some unit tests. You won't want to accept them as-is, but they will help describe my issue and explanations.

I have added testSerialize2DArray

    $arr = array(array('a','b','c'),array(1,null,3),array('x',4.56,'z'));

My argument is that with setSerializeNull to true, the eventual expected output should be the same as a straight json_encode($arr,0);
But the serialiser as is returns

[["a","b","c"],{"0":1,"2":3},["x",4.56,"z"]]

it should return

[["a","b","c"],[1,null,3],["x",4.56,"z"]]

By removing the is_string check on the array key in visitArray, you acheive the expected result.

Now doing this breaks the existing test testSerializeNullArray, test data defined as...

    $arr = array('foo' => 'bar', 'baz' => null, null);

The expected result is intended to be

{"foo":"bar","baz":null}

But again, with setSerializeNull to true, the eventual expected output should be the same as a straight json_encode($arr,0);
which is ....

{"foo":"bar","baz":null,"0":null}

.... and rightly so ?? that last 'null' in the test array is infact defining a 3rd element of the array, so why shouldn't it be serialized ?? Why should it be 'stripped' from the data that gets serialized through json_encode in the JsonSerializationVisitor getResult() routine ?

I welcome your feedback and advice.

@schmittjoh
Copy link
Owner

Could you clean-up the test-case and add the expected formats for XML and YAML?

Tidy up tests and confirm xml/yml outputs as requested.
@TheBenjiManCan
Copy link
Author

Hi,
As requested ..... cleaned up the test cases and added expected formats for XML and YAML.
XML behaved as expected
YAML (like JSON) stripped the array's null reference, even though it's a legitimate element of the array. So, also changed YamlSerializationVisitor.php to remove the array key is_string check.

@coelhoricardo
Copy link

We have the same prob with "is_string" here.

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

Successfully merging this pull request may close these issues.

4 participants