Fixed problem when serializing a multidimensional primitive array

[drieschel]

This MR fixes a problem with the serialization of an multidimensional array for a basic data type. This happened for instance by using JMS type annotation ala array<string, array<string>>.

[drieschel]

Hi @dbu,

thanks for your message. Not exactly sure what you mean with "simple assignment".

Plain php arrays are used. An example looks like [foo][]=bar. Because type is an array and sub type is an array as well generateCodeForArray is called recursively and in the next iteration subtype is an instance of Liip\MetadataParser\Metadata\PropertyTypePrimitive. This leads to that an exception is thrown with Unexpected array subtype message.

[dbu]

@drieschel do you have time to follow up on my questions?

[drieschel]

Sorry for the long delay @dbu. I reverted the switch statements. Do you think it is ready to merge now? :-)

PS: I will definitely respond faster next time.

[dbu]

re-reading what we do here, i think the fix is correct as it just simplifies the existing code and handles nested arrays that end up having a primitive type correctly.

however, i have the feeling that it would be interesting if we can detect something like array<string, array<int>> because that does not need any loops, it can just be directly assinged to a field. if the fields of the array are objects, we do need to loop, but if i understand correctly, we currently generate

$input = [
  'a' => [1, 2],
  'b' => [3, 4],
];

$output = [];
foreach ($input as $key => $element) {
  $output[$key] = $element;
}

but the code could just be $output = $input.

could you please add a test case with a nested array that verifies what we generate? and also verifies that we don't run into regressions about that in the future?

[drieschel]

@dbu Think I found the solution you were looking for. Please re-check.

[dbu]

i verified locally that the test you added fails without the fix, and looked at the generated code with your fix. seems all fine. i am merging this in #41 (keeping your authorship)

[dbu]

i tagged a release with this and other fixes and improvements: https://github.com/liip/serializer/releases/tag/2.5.0

[drieschel]

awesome, thanks a lot!

we could try to do the direct assign for n-dimensional arrays but that would require us to descend to the innermost nested array to check. good enough for now!

thanks a lot for adding the test as well.

I have an idea how to direct assign an n-dimensional array. Can try that later out. I can create another MR in case it works if you are interested in.

[dbu]

sounds awesome, sure i am interested :+1: