Issues with ManagedMetadataProvider and type conversions in JSR223 scripts

[ccutrer]

The gist of this issue is that if you add some metadata to an item from a JSR223 script, it gets added to the MetadataRegistry exactly as it was given to MetadataRegistry.add. This is a problem, because after restarting openHAB, it will get added to the MetadataRegistry as it gets deserialized from storage. In the most innocuous scenario, this means an Integer value in the configuration will get changed to a Float afterwards. There are more convoluted (but real-life) scenarios where you can put other objects into the configuration map (such as a Ruby Symbol) that seems to work just fine, until openHAB restarts, and it's round tripped through serialization and comes back completely broken.

So... the first, and hopefully simpler fix, would be for AbstractManagedProvider to call notifyListenersAboutAddedElement in add (or notifyListenersAboutUpdatedElement in update) not with element that was passed in, but instead with a serialization round-tripped version from storage. Seems simple enough, except that Storage.put has no way of providing that, and doing a get right after the put seems liable to introduce race conditions, but maybe not? And if it does, then Storage needs to be extended to have a method that can return the round-tripped value you just put in, as well as the previous value. Seems non-trivial.

The deeper feature request would be to allow JsonStorage to have additional types added to its entityMapper to be able to handle types that Gson can't natively handle. For example, registering Ruby's Symbol to at the least just convert it to a String before serialization, or better to fully round-trip. Obviously metadata such as this would only be useful to the caller that would know how to interpret it. But that's the story for any type of metadata.

I wanted to get feedback before I write any PRs addressing any of these issues.

[jimtng]

This issue has been causing me grief for a long time, so I'd love to have a solution. I've experienced mysterious randomly occurring, data-dependent errors because of this. Thanks for looking into this!

[J-N-K]

This is indeed not only a problem for metadata, but also for thing configuration. We have fixed that by normalizing configuration before passing it to the thing handler but this is not possible for metadata - we don't have a config description that shows what is the correct way to serialize/de-serialize the value.

In fact the correct solution would IMO be to refactor the whole storage code to store value and type instead of value only. But this is a big refactoring which might blow up lots of systems, so for sure needs a good migration path and also intensive testing.

[splatch]

Config descriptors are single level, so they work with scalar values. This is suboptimal for most complex scenarios, but on other hand keep definition of configs and their processing reasonable and manageable. WRT custom namespaces I made a wish to bind it with optional config descriptors, mainly to render UI forms, looking at this issue it seems to be relevant also in other places and managed metadata provider itself.