BUGFIX: Correctly handle empty property elements in node import

[zickgraf]

What I did

Made sure that properties with an empty value are actually set during the import.

How I did it

$reader->isEmptyElement seems to only work for self-closing tags (for example, see relatedDocuments in the tests), but not for tags with empty content (for example, see subpageLayout in the tests). To work around this, also test for isset($properties[$currentProperty]) when matching a suitable END_ELEMENT.

How to verify it

Import a Sites.xml with empty properties and run node:repair. There should be no missing default values.

Checklist

• [x] Code follows the PSR-2 coding style
• [x] Tests have been created, run and adjusted as needed
• [x] The PR is created against the lowest maintained branch

[mhsdesign]

The lowest maintained branch is Neos 5.3 ^^

[zickgraf]

The lowest maintained branch is Neos 5.3 ^^

Ah, thanks for the hint. I got confused by the note at the end of the readme, which should probably only apply to security fixes. I have now changed the base to 5.3. (Since I cannot change the source of the PR, I have pushed 5.3 to my 4.3 branch).

[zickgraf]

I have rebased the PR onto 7.3, since this is now the lowest maintained branch for bugfixes (if I see this correctly). Could you consider this for merge before the next LTS? The fix seems simple and always having to take special care of empty properties during export and import is quite a nuisance.

[kdambekalns]

Looks good at first sight, will test this…

One thought already: What about an array/string having null as default value? For a string that export/import would be lossy (no null equivalent in the XML), but simply putting an empty string in, could be "wrong". And for arrays it's the same, I just don't know off the top of my head if an empty array is exported as such.

[zickgraf]

Looks good at first sight, will test this…

Thanks!

One thought already: What about an array/string having null as default value? For a string that export/import would be lossy (no null equivalent in the XML), but simply putting an empty string in, could be "wrong". And for arrays it's the same, I just don't know off the top of my head if an empty array is exported as such.

I just checked: array/strings having null as default value do not appear in the XML at all and thus are not affected by this PR.

[zickgraf]

Thanks for you review @kdambekalns!

Could this this PR be considered for merge before the upcoming LTS?

[zickgraf]

Thanks for the additional reviews and the merge! This will make upgrading to 8.3 via export and import much easier for us :-)