[REST] New API for conversion between file formats

[lolodomo]

Related to #4585

This PR only supports management of things.
It supports DSL and YAML file formats.

A first new API (POST /file-format/create) allows to create a file format (DSL or YAML) from a JSON object.
A second new API (POST /file-format/parse) allows to parse a file format (DSL or YAML) to a JSON object.

These 2 APIs should help Main UI displaying DSL and YAML file formats for things.

[lolodomo]

First API:

Second API:

[jimtng]

I understand that you haven't implemented Items here yet. But I'm wondering how will we request it to convert JSON Item to Yaml? Currently I see /create accepts "text/vnd.openhab.dsl.thing", "application/yaml". application/yaml will return Thing.

The same goes for /transform.

Will we use different end points for item creation / transformation?

[jimtng]

As I understand it:

• /create converts JSON -> DSL or YAML
• /transform converts DSL or YAML -> DSL, YAML, or JSON

Why not remove /create and make /transform able to accept JSON as well?

Bonus points that It could perform a round trip as a way to validate data (whichever the format) when source format == target format. If the source data is correct, return it, otherwise return an error.

Also this way we can have /transform-thing and /transform-item

[spacemanspiff2007]

There was a rather lengthy discussion in #4585 how the api should look like.
In the end the consensus was to provide a parse and create because this makes the most sense and is also the only way to properly and correctly annotate the api. Also it prevents common pitfalls which exist when directly translating between file formats.

So it was

• /create which does Json Core DTOs -> *.things DSL, *.items DSL, YAML.
  The accept header defines the output format
• /parse which does *.things DSL, *.items DSL, YAML -> Json Core DTOs.
  The content-type header defines the input format

@lolodomo also wanted to provide convenience to serialize existing objects more easily, that's why there was also a
/existing which does Existing Registry Obj -> *.things DSL, *.items DSL, YAML. The content-type header defines the input format

---

@lolodomo
Your proposed a /transform api which is different.
Did you find a way to work around the technical limitations and issues mentioned in #4585.
What made you change your mind to go directly for a /transform instead of the discussed /parse ?
I was under the impression you also thought the above suggestion is a good idea and the desired api. Is this not the case?

@jimtng
Have you read all of #4585 ?
Imho everything was already clear and discussed and I'd rather not want to start discussing from scratch again.

[lolodomo]

I understand that you haven't implemented Items here yet. But I'm wondering how will we request it to convert JSON Item to Yaml? Currently I see /create accepts "text/vnd.openhab.dsl.thing", "application/yaml". application/yaml will return Thing.

The same goes for /transform.

Will we use different end points for item creation / transformation?

No, this will be the same. I will just add support for items.
When using "application/json" or "application/yaml" as output, it will consider what you have at input to transform it at output. As an example, if your input is a YAML containing items and things, the output will contain items and things if supported by the selected output format.

[lolodomo]

s I understand it:

* /create converts JSON -> DSL or YAML

* /transform converts DSL or YAML -> DSL, YAML, or JSON

Why not remove /create and make /transform able to accept JSON as well?

Very good question, I was sure someone will ask it :)
To be honest, this was my intention to have a unique /transform endpoint for any conversion. At a certain point of the discussion, it was also the dream of either @spacemanspiff2007 or @mherwege .
The only technical problem I encountered is that I did not succeed to define an input parameter that is either of type String for DSL or YAML and FileFormatDTO type for JSON. If I use String type for my parameter, this leads to an error when I provide JSON because there is an automatic try to parse the provided JSON to the target type and it fails if target is String. I also tried to use Object as parameter type but with no success. I also tried to define two parameters, one for YAML/DSL as String type, one for JSON as FileFoirmatDTO type but this is probably not allowed to have both request bodies for a POST request as Swagger systematically shows only one of both.
After all these failures, the only remaining option was to have two different endpoints.

[lolodomo]

Bonus points that It could perform a round trip as a way to validate data (whichever the format) when source format == target format. If the source data is correct, return it, otherwise return an error.

I was not yet sure what to do when input media and output media are identical. I applied the general transformation process leading to an output that can be different from the input. But it can be changed to simply return as output what was provided as input.

[lolodomo]

Also this way we can have /transform-thing and /transform-item

As explained in a previous message just before, we don't need to distinguish things and items. The code is considering what it receives as input and what output media is requested and then builds the output.
If the input is a DSL things, the requested JSON or YAML output will then only contains things.
If the input is a JSON containing items and things, the requested YAML output will then contain items and things.

[lolodomo]

@lolodomo
Your proposed a /transform api which is different.
Did you find a way to work around the technical limitations and issues mentioned in #4585.
What made you change your mind to go directly for a /transform instead of the discussed /parse ?
I was under the impression you also thought the above suggestion is a good idea and the desired api. Is this not the case?

Yes, I found a way to make the transformation API almost fully generic. It was a wish of you or @mherwege I believe. I think it is really fantastic to have an API that can directly converts from DSL to YAML or YAML to DSL.
I let you read my other messages, I encountered a technical difficulty and I was constrained to keep the create API.

My idea was a unique generic API being able to convert from JSON/DSL/YAML to JSON/DSL/YAML. It currently only covers DSL/YAML to JSON/DSL/YAML for a technical reason.

What I wanted to remove is the fact that the APIs either takes a JSON as input or a JSON as output. I think it is good to be able to really convert from a file format to another one, DSL to YAML for example. That is the reason why I renamed parse into transform as it is not only a parser but also a generator. Parser + generator = transform.

[lolodomo]

In fact, I have an option to remove the create API and have the transform API supporting also JSON as input, I can just provide a JSON example for the JSON content type and so receives the JSON in a String parameter (like for DSL and YAML) and then parses myself the JSON in a FileFormatDTO object (instead of being done automatically). I will apply this change, it's worth the effort.

[jimtng]

The only technical problem I encountered is that I did not succeed to define an input parameter that is either of type String for DSL or YAML and FileFormatDTO type for JSON. If I use String type for my parameter, this leads to an error when I provide JSON because there is an automatic try to parse the provided JSON to the target type and it fails if target is String. I also tried to use Object as parameter type but with no success. I also tried to define two parameters, one for YAML/DSL as String type, one for JSON as FileFoirmatDTO type but this is probably not allowed to have both request bodies for a POST request as Swagger systematically shows only one of both.
After all these failures, the only remaining option was to have two different endpoints.

Thanks for the explanation.

As explained in a previous message just before, we don't need to distinguish things and items. The code is considering what it receives as input and what output media is requested and then builds the output.
If the input is a DSL things, the requested JSON or YAML output will then only contains things.
If the input is a JSON containing items and things, the requested YAML output will then contain items and things.

Interesting! So the YAML input must contain either things: or items:, just like how the output from /file-format/things/ now, right?

In the UI (thing-details page), the version: and things: will need to be stripped off before displaying it and added before sending to the API.

Also, since the UID cannot be changed anyway, maybe we'll strip that off before displaying in the UI too, so that there is no leading spaces in the display.

i.e.

version: 2
things:
  thing:uid:here:xxx:
    thing_attrib: xx
    more_attrib: yy
    channels:
      channel1:
        xx: yy

will be displayed as:

thing_attrib: xx
more_attrib: yy
channels:
  channel1:
    xx: yy

In fact, I have an option to remove the create API and have the transform API supporting also JSON as input, I can just provide a JSON example for the JSON content type and so receives the JSON in a String parameter (like for DSL and YAML) and then parses myself the JSON in a FileFormatDTO object (instead of being done automatically). I will apply this change, it's worth the effort.

That's awesome!

Did you consider the name convert instead of transform? I think the term convert is more familiar, e.g. "I'm going to convert this JPG to PNG" instead of "I'm going to transform this JPG to PNG".

[spacemanspiff2007]

At a certain point of the discussion, it was also the dream of either @spacemanspiff2007 or @mherwege .

This was @mherwege .
We discussed it a bit but I think the general direction was not to do this but always parse to / transform from json DTOs.
I'm strictly against a generic translation endpoint which does translation between dialects.
There were several reasons and I'll try to summarize them again.

1. It's not possible to properly document the api.
2. It makes error handling unnecessarily hard
3. Sometimes requires to necessarily drop data and thus is not round trip save which silently (!) leads to loss of data.
4. Leads to unexpected behavior

Examples:

1. Lack of proper documentation

Output: Items-DSL
Input can only be yaml and Json-DTO. Things-DSL may not be selected as input format.
Yaml should also only allow data in an items key, over providing e.g. with things entries should raise an error.
So the input schema in the docs should either be:

{
   "items": ["List of ItemDtos"],
   "links": ["List of LinkDtos"]
}

if input json is selected or

items:
   Item1: ...
   Item2: ...

if input yaml is selected

As you can see the required input schema is now dependent on the desired output (items-dsl) and the selected input (json or yaml).
It's not possible to document that in the swagger ui because input schema has two dependencies.
Also there are input/output combinations which do not work at all.

2. There multiple errors which can occur:

• parsing the input format
• over providing data
• missing required data
• error generating the target format

All these possible error cases have to be properly reported in an api response.

When serializing from/to json dtos it's easy:
When the data matches the input schema the output file can almost always be generated. All these cases don't need to be differentiated. It's also easy to return a proper error message because the input schema is well defined.
Parsing to JsonDTOs always works, there is no over providing or missing data. Errors can be directly reported in case the input is malformed.

3. Not round trip safe
   yaml -> things dsl -> yaml is not guaranteed to be the same which is very unexpected

It's unacceptable for the transformation API to guess if it's okay to drop data or not.
This decision has always to be done by the caller

4. Main use cases is not a simple translation but merging or aggregating data

e.g. things-dsl + items-dsl -> yaml or things-DTO + items-DTO -> yaml.
It's not worth to live with all shortcomings when for a proper aggregation one has to use the json-DTOs anyway

---

The generic translation endpoint is very complex and leads to strange edge cases and behavior.
Additionally the API can not be properly documented which is not acceptable.
Making two requests (format -> JsonDTO, JsonDTO -> format) is very cheap, even more when this is something that does only happen after manual interaction.
The logical thing is to provide a clean API which works consistently, logically and reproducible.
That's only possible with the intermediate step with JsonDTOs and thus that should be what will be implemented.

[lolodomo]

In fact, I have an option to remove the create API and have the transform API supporting also JSON as input, I can just provide a JSON example for the JSON content type and so receives the JSON in a String parameter (like for DSL and YAML) and then parses myself the JSON in a FileFormatDTO object (instead of being done automatically). I will apply this change, it's worth the effort.

Unfortunately, still does not work.
We have org.openhab.core.io.rest.core.internal.MediaTypeExtension detecting any value provided as JSON to inject it into the taget Java object. Very practical in general but problematic in my use case.

[lolodomo]

... strictly against ...
... unacceptable ...

Please come back to a more reasonable level of discussion ... or all of this will end up exactly like the previous time you had this way of discussing with @J-N-K , that is to nowhere!

The logical thing is to provide a clean API which works consistently, logically and reproducible.
That's only possible with the intermediate step with JsonDTOs and thus that should be what will be implemented.

I understand some of your points but restricting features just for a question of API documentation, it is a shame. I like the idea to have a direct conversion between DSL and YAML. Just my opinion.

Let's first listen at other opinions. @jimtng @mherwege and others.

[jimtng]

The only use case I'm aware of right now is to convert json object from mainui (which is retrieved from the rest api too) into yaml (and maybe dsl) and back so that what we see in the "CODE" tab in Thing-details and Item Details match the same format as the Yaml file format. I'm not sure what the application for DSL <=> YAML would be for, but maybe it can be used to offer a tool to convert user's existing DSL to YAML.

The API in this PR would provide these capabilities.

Making two requests (format -> JsonDTO, JsonDTO -> format) is very cheap

What's the difference between that and the server simply doing the intermediate step for you?

At the end of the day, there is one ultimate format: The Java Thing object itself. Any other format would be created in such a way that there is no data loss to/from that format.

Additionally the API can not be properly documented which is not acceptable.

Can you explain what documentation problem exists in the implementation from this PR?

[mherwege]

I see multiple use cases:

1. Endpoint for the UI to generate the code view from the UI javascript object representation (which closely matches the REST API JSON, but there sometimes is some extra logic in the UI, but that can be handled in the UI). And with these endpoints it will allow to not only show YAML but also DSL, a user choice. The YAML and DSL will also convert to the JSON notation. The generated code will use the same structure as file based configurations and that is a big plus. It also avoids requiring conversion logic in the UI. JSON to/from YAML is easy in the UI, but generates a format mimicking the JSON structure, not the agreed format (or we would have to maintain that specific conversion in the UI). Note that the UI currently does not have conversion from DSL to JSON (except for sitemaps, but that has not been handled yet). For this case, you cannot assume the code view is all code needed for file based configuration of the same object. The view is in context of the UI. If you copy in file based config here, you may lose some of the copied in YAML or DSL as it is out of context for the UI. Switching from a code view to UI view and back to code view would make you lose that extra config, but I think that’s acceptable.
2. Convert text based configuration to managed configuration. This is essentially converting YAML or DSL to JSON. The one thing to be aware about in this case is that comments in the files cannot be represented in the JSON, so will be lost. Apart from that, there should be no data loss. This is the import/export functions in the UI.
3. Convert from file based YAML to file based DSL or the other way around. While internally converting to JSON (or 2 calls to the REST endpoint, first to JSON and then the other format) would keep the content, comments will be lost. If there is direct conversion, there is an opportunity to keep comments. I don’t know if the current code achieves that.

I am largely indifferent to the final REST structure, but based on the comments issue, I do think having a possibility for a direct conversion makes sense (even if we do not achieve keeping the comments just yet).

I also think that if there is a way to limit the number of endpoints and still keep the documentation properly represented, we should strive for that. But I do respect @lolodomo when he does not find a good solution for that to use multiple.

[lolodomo]

If there is direct conversion, there is an opportunity to keep comments. I don’t know if the current code achieves that.

No

I am largely indifferent to the final REST structure, but based on the comments issue, I do think having a possibility for a direct conversion makes sense (even if we do not achieve keeping the comments just yet).

I also think that if there is a way to limit the number of endpoints and still keep the documentation properly represented, we should strive for that. But I do respect @lolodomo when he does not find a good solution for that to use multiple.

You all don't see an immediate interest to have a conversion between DSL and YAML file formats and the current identified use cases are currently with Main UI and will require the REST DTO object either as input or output. So let make @spacemanspiff2007 happy again and let's restrict the APIs to always go through a DTO object.

[lolodomo]

In the UI (thing-details page), the version: and things: will need to be stripped off before displaying it and added before sending to the API.

Also, since the UID cannot be changed anyway, maybe we'll strip that off before displaying in the UI too, so that there is no leading spaces in the display.

I did not see this message.

I don't know if this is a good idea to strip the code, in that case we will not show exactly the file format. The idea is to have the ability to copy from the code tab, and in that case it is better to have the full syntax.
But maybe Main UI can provide a toggle between the full syntax and the stripped syntax ? The stripped syntax would be defaulted.
WDYT ?

[jimtng]

I don't know if this is a good idea to strip the code, in that case we will not show exactly the file format. The idea is to have the ability to copy from the code tab, and in that case it is better to have the full syntax.
But maybe Main UI can provide a toggle between the full syntax and the stripped syntax ? The stripped syntax would be defaulted.
WDYT ?

Hmm, maybe it's simplest to keep it as is?

It's just awkward to see things: and thinguid: there, which kind of makes you think you could paste multiple Things in there. Then the UI needs to detect this case.

On the other hand, I do see your point about being able to copy paste to/from the file directly.

Perhaps @merwege or @florian-h05 have a better idea on how to do this.

[lolodomo]

It's just awkward to see things: and thinguid: there, which kind of makes you think you could paste multiple Things in there. Then the UI needs to detect this case.

I agree with you.

We have pros and cons for the 2 options.

[mherwege]

Hmm, maybe it's simplest to keep it as is?
It's just awkward to see things: and thinguid: there, which kind of makes you think you could paste multiple Things in there. Then the UI needs to detect this case.
On the other hand, I do see your point about being able to copy paste to/from the file directly.

I do think the UI editor should show only the context of what is being edited or viewed. In my view, that does mean stripping the higher level wrappers. At the moment, for things, there is only a per thing editor. I could imagine a code view on the thing list level where all things are shown, so that would include the higher level structure. But it wouldn’t be editable (or it would require considerable UI changes).

for items, there is a similar consideration. The current item edit code view only shows the item data, and not the channels and links. They are not editable in that place. If we start showing them, we create the expectation they are editable. Maybe we should therefore have a non-editable code view on the item level (not the item edit level) that does show these. And then again on the item list level showing the code for all items (non-editable). These non-editable views could even include the file-defined items and things.

For me, in summary, when it is editable, it should match the context of the object being edited, nothing else. I can imagine non-editable views on a higher level showing the more complete definition.

This is only my opinion, open for anything.

[lolodomo]

for items, there is a similar consideration. The current item edit code view only shows the item data, and not the channels and links. They are not editable in that place. If we start showing them, we create the expectation they are editable. Maybe we should therefore have a non-editable code view on the item level (not the item edit level) that does show these. And then again on the item list level showing the code for all items (non-editable). These non-editable views could even include the file-defined items and things.

Note that for items I could also include a parameter in the API to hide or not the metadata and channel links.
The same thing I did in this PR for things with channels.

For me, in summary, when it is editable, it should match the context of the object being edited, nothing else. I can imagine non-editable views on a higher level showing the more complete definition.

This is only my opinion, open for anything.

But it makes sense.

[lolodomo]

Change done.
My two first messages were updated (new screen captures).

[lolodomo]

When the API returns status 400, the errors are provided as a string separated by a carriage return. Maybe it would be better to return errors as a JSON ?

[mherwege]

When the API returns status 400, the errors are provided as a string separated by a carriage return. Maybe it would be better to return errors as a JSON ?

If we only have a message, there is not much value in returning it in a JSON. It becomes interesting when we have line number (and potentially position) as a field as well, because the editor could point at the line and position then.

[lolodomo]

The first part until " to YamlThingDTO: " is generated by us, the next part is th eerror message from the library.
We are providing the full current YAML block to help the user identifying the element that is wrong. It could be enough to just provide the element ID ? It would make the error message shorter. Something like: could not parse element "id" to YamlThingDTO: Unrecognized field ...
It would be possible with the Map version which is the only one that will finally remain.
WDYT ?

https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/YamlModelRepositoryImpl.java#L750

Note that the part "at [Source: UNKNOWN; byte offset: #UNKNOWN] " has been removed in a recent merged PR.

[jimtng]

It could be enough to just provide the element ID

yes please.

FYI this is what I'm working on:

If those [x,y] are consistent, I can extract them and use them as line/column numbers

[jimtng]

Sometimes the error requires it to be displayed as monospaced text (much like the code fence here)

Like this one for example

Failed to process model: while scanning a simple key
 in 'reader', line 5, column 5:
        config
        ^
could not find expected ':'
 in 'reader', line 6, column 18:
          geolocation: "-asdfasdff5"
                     ^

 at [Source: (ByteArrayInputStream); line: 4, column: 27]

But sometimes doing so would look awkward.

Also the at [Source: (ByteArrayInputStream); line: 4, column: 27] seems useless for the user and would be confusing. They'd ask: "What is ByteArrayInputStream, and what line 4 column 27 is it referring to?"
Sorry if that's the one already removed in a recent PR.

[jimtng]

So right now, if there's a caret character ^ in the error message, I'll display it using <pre>. Otherwise, it'll be displayed normally

[jimtng]

It could be enough to just provide the element ID

yes please.

FYI this is what I'm working on:

If those `[x,y]` are consistent, I can extract them and use them as line/column numbers

It seems that in this case, we should just display the first one or two messages? All I did was remove the colon in the Channels:. Otherwise the list of errors is very long.

[jimtng]

I did some cleaning up in javascript, removing all text up to ... YamlThingDTO: and some parts of the messages after that.

So I think this can be cleaned up on the UI side and we can iterate more quickly that way.

[lolodomo]

I pushed the change of the error message.

[jimtng]

The updated error message:

I think it would be nicer to remove everything up to and including YamlThingDTO:. Here's what it looks like (stripped off in JS)

[lolodomo]

We can't remove it in general as it is more than useful when parsing a file containing several things.
You are in a particular case where you manage a unique thing. We could remove the initial part in that case.

Regarding the final part "(through reference chain ...)", I am not sure it has any value ? I could also filter it if you agree.

[jimtng]

You are in a particular case where you manage a unique thing. We could remove the initial part in that case.

Shall we just remove it on the JS side, as demonstrated here?

Regarding the final part "(through reference chain ...)", I am not sure it has any value ? I could also filter it if you agree.

Yes I agree.

[lolodomo]

You are in a particular case where you manage a unique thing. We could remove the initial part in that case.

Shall we just remove it on the JS side, as demonstrated here?

Yes, could be better.

[lolodomo]

Regarding the final part "(through reference chain ...)", I am not sure it has any value ? I could also filter it if you agree.

Yes I agree.

Done

[jimtng]

I am getting an error when trying to define a channel description.

Unrecognized field "description", not marked as ignorable (6 known properties: "config", "itemDimension", "kind", "type", "itemType", "label"])

It should be supported:

The old structure:

UID: mqtt:topic:ebfc89d27f
label: aaaaa Generic MQTT Thing
thingTypeUID: mqtt:topic
configuration: {}
bridgeUID: mqtt:broker:mosquitto
channels:
  - id: sdfasf
    channelTypeUID: mqtt:string
    label: label
    description: desc
    configuration: {}

[lolodomo]

The old structure:

UID: mqtt:topic:ebfc89d27f
label: aaaaa Generic MQTT Thing
thingTypeUID: mqtt:topic
configuration: {}
bridgeUID: mqtt:broker:mosquitto
channels:
  - id: sdfasf
    channelTypeUID: mqtt:string
    label: label
    description: desc
    configuration: {}

Rather something to discuss outside this particular PR. The description was not something you can provide with the DSL syntax.
You are showing I believe the JSON from a managed thing ? I guess this description here is a copy of the channel type description.
It is probably easy to add.
To be discussed in #3666

[jimtng]

Rather something to discuss outside this particular PR. The description was not something you can provide with the DSL syntax.
You are showing I believe the JSON from a managed thing ? I guess this description here is a copy of the channel type description.
It is probably easy to add.
To be discussed in #3666

I presume you aren't going to include it in this PR? If so, I will remove it from the UI for now. But this will mean a regression in the UI - with users able to edit them before but now won't be able to.

Even though DSL doesn't support this syntax, the description won't be lost, due to the way the round-trip (parse - create) is done in the UI. So right now, even though user can't see/edit it in Code (YAML / DSL), the description is still there when they switch out of Code and into Channel editor, and they can still edit it there.

[lolodomo]

I presume you aren't going to include it in this PR? If so, I will remove it from the UI for now. But this will mean a regression in the UI - with users able to edit them before but now won't be able to.

I will create a new PR as it is a change in the YAML thing syntax.

[jimtng]

I presume you aren't going to include it in this PR? If so, I will remove it from the UI for now. But this will mean a regression in the UI - with users able to edit them before but now won't be able to.

I will create a new PR as it is a change in the YAML thing syntax.

Does it mean that this PR can be merged as is, and when this other PR is merged, it will automatically allow description?

[lolodomo]

Does it mean that this PR can be merged as is, and when this other PR is merged, it will automatically allow description?

Yes

[lolodomo]

I am closing this PR, a new PR #4793 covers its scope.