BUGFIX: Neos Ui JSON serializable property values

[mhsdesign]

This bugfix will make use of the \JsonSerializable interface instead directly when serializing properties for the neos ui.

With neos/flow-development-collection#2762 native support for vo's in flow_json_array was introduced.

That also allows value object support for node properties, as they can be stored directly the node properties flow_json_array.

Unfortunately the property serialisation for the NeosUi does NOT use the expected \JsonSerializable::jsonSerialize but the property mapper and the ArrayFromObjectConverter for object types to get an array that will be json encoded.

This works mostly fine but in some cases it fails:

• when your fromArray fields and property names values dont match
• when a "object" subtypes the object mapper is no convertable like the GuzzleHttp\Psr7\Uri:

Too few arguments to function GuzzleHttp\Psr7\Uri::isAbsolute(), 0 passed in /core/neos-manufacture-highest/Packages/Framework/Neos.Utility.ObjectHandling/Classes/ObjectAccess.php on line 151 and exactly 1 expected

72 GuzzleHttp\Psr7\Uri::isAbsolute()
71 Neos\Utility\ObjectAccess::getPropertyInternal(GuzzleHttp\Psr7\Uri, "absolute", false, true)
70 Neos\Utility\ObjectAccess::getGettableProperties(GuzzleHttp\Psr7\Uri)
69 Neos\Flow\Property\TypeConverter\ArrayFromObjectConverter_Original::getSourceChildPropertiesToBeConverted(GuzzleHttp\Psr7\Uri)

Current workarounds are aop:
https://github.com/sitegeist/Sitegeist.InspectorGadget/blob/78f5f4a206287b1c4bedf5cb88582ed51cb4a311/Classes/Infrastructure/NodeInfo/NodeInfoPostProcessingAspect.php#L17
Or to use a dumb property mapper:
https://github.com/sitegeist/Sitegeist.Archaeopteryx/blob/9322b9cb8e4824bcaf7aaa247c23b1244a2f1167/Classes/LinkToArrayForNeosUiConverter.php#L12C16-L12C78

Upgrade instructions

Review instructions

Checklist

• Code follows the PSR-2 coding style
• Tests have been created, run and adjusted as needed
• The PR is created against the lowest maintained branch
• Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
• Reviewer - The first section explains the change briefly for change-logs
• Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

[mhsdesign]

i will move this over to the neos ui instead and target 9.0

for now one needs to implement an toArray type converter for the value object: https://github.com/sitegeist/Sitegeist.Archaeopteryx/blob/9322b9cb8e4824bcaf7aaa247c23b1244a2f1167/Classes/LinkToArrayForNeosUiConverter.php

[mhsdesign]

Edit: As this service still resides in Neos 8.3 in Neos.Neos and will only be moved in Neos 9 to the ui, i were able to fix it here, even-though its considered ui matter.

Once this is merged i will open a dedicated pr in Neos.Ui

[kitsunet]

Wait why do we need to move it to the UI? Just because the UI is the only place using it? Seems like a very sensible thing to have in any neos also for custom caching or node relation implementations?

[mhsdesign]

Actually it resides in Neos 9 in the ui already: https://github.com/neos/neos-ui/blob/d581ba73e41d335cb7cb6dc443de9e7dfe1783ba/Classes/Domain/Service/NodePropertyConverterService.php#L218

Just because the UI is the only place using it?
yes its a big blob of super ugly ugly conversion that we will get rid of when we use a better serialisation format that is possibly understood by the cr directly.

... i like to have this code encapsulated in the Neos ui.

THIS IS DIRTY CODE. In the longer run, the neos UI should work DIRECTLY with serialized properties instead of the objects.

no one shall use this code ^^

[mhsdesign]

Seems like a very sensible thing to have in any neos also for custom caching or node relation implementations?

im not entirely sure i get you point and understand ...

[kitsunet]

I thought the whole property (un)serialize thing could be helpful outside of UI context as well but if we strive to use the serialized properties directly I guess that is fine too... so nvm

[mficzel]

Fine by 👀, 💬 , and me knowing the topic beforehand

[kitsunet]

I think this needs a lot of dedicated serialize/deserialize tests and also a before after comparison, because I am not sure about some cases people could already have right now (DateTime being one of those). Also we force DateTime to be in UTC (or rather server TZ) atm, so this would no longer be the case.

[kitsunet]

Nod, nod this seems sensible then :)