Improve YAML model repository #4024
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
Signed-off-by: Jan N. Klug <github@klug.nrw>
This reverts commit 1c4e6c7eb2afdc55628ca327e8cd9f7453d354a7. Signed-off-by: Jan N. Klug <github@klug.nrw>
|
@wborn How do we proceed here? |
wborn
reviewed
Jan 31, 2024
| */ | ||
| @NonNullByDefault |
There was a problem hiding this comment.
This essentially a DTO. Missing elements in the data are de-serialized as null.
If we decide to make the DTO classes @NonNullByDefault we would need to either annotate all fields with @Nullable (which is the similar to not annotating the class at all) or provide default values (which doesn't make sense in most cases).
bundles/org.openhab.core.model.yaml/src/test/resources/modelFileUpdatePre.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Jan N. Klug <github@klug.nrw>
|
I did not take time to have a look. Does this PR break user YAML files containing custom tags ? |
|
No, I don't think so. The format is the same, just some more options added and code refactoring |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to #3666
This improves the existing YAML model parser and repository. In detail:
YamlFilehas been removed. This simplifies code for consumers, because they only need to implementYamlModelListenerand the DTO as inheritor ofYamlDTO.getRootNamehas been removed fromYamlModelListener. The information is now provided by the@YamlElementNameannotation on the respective DTO class.addedModel,updatedModelandremovedModelmethods inYamlModelListenerhave been changed to acceptCollection<T>instead ofCollection<? extends YamlDTO>which simplifies code in the listener.YamlModelRepositoryhas been refactored to an interface with methods for adding/updating/removing elements from a model. The implementation isYamlModelRepositoryImpl. This interface is public and can be implemented to modify an existing model or create a new one. This allows to dynamically update the content.YamlModelRepositoryImplnow parses the elements usingJsonNodeinstead of de-serializing the file directly.JsonNodes are cached by model (file) name and type. This allows to mix different inheritors ofYamlDTOin one file. Since all elements are cached asJsonNode, they can also be read before the respectiveYamlModelListeneris registered. In addition, this removes the coupling between directory and model type.YamlModelRepositoryhave been added.readOnlyflag (defaults totrue) has been added. Models without this flag (orreadOnly: true) cannot be modified. This is to prevent changing user-defined models. On deserialization comments in the file are lost, so they would be missing in an updated model.@lolodomo I would especially like to hear your comments since you did the original implementation.