-
-
Notifications
You must be signed in to change notification settings - Fork 461
[YAML] Enhance removedModel method in all providers #5341
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
The enhancement consists in just getting the UID of the elements to be removed rather than building a full object. Signed-off-by: Laurent Garnier <[email protected]>
|
@Nadahar for information as it is in response to something you reported. |
|
@lolodomo I tried to see if I could review it, but I find the streams stuff completely unreadable and can't really see what has changed (except for writing the streams slightly different). Sorry, but my review doesn't matter much anyway 😉 |
|
What about also delaying the mapping in the addedModel and updatedModel until listeners have to be notified (which would require the object)? That would avoid the mapping all together if the model is isolated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Optimizes YAML model removal handling by avoiding construction of full domain objects when removing elements, relying instead on DTO identifiers (UID/name) to find and remove existing elements from in-memory collections.
Changes:
- Update
removedModelin Thing provider to remove Things by DTOuidrather than mapping fullThingobjects. - Update
removedModelin Semantic Tag provider to remove tags by DTOuidrather than mapping fullSemanticTagobjects. - Update
removedModelin Item provider to remove items by DTOnamerather than mapping fullItemobjects.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java |
Removes Things using DTO uid directly rather than building Things for comparison. |
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/semantics/YamlSemanticTagProvider.java |
Removes SemanticTags using DTO uid directly rather than building SemanticTag instances. |
bundles/org.openhab.core.model.yaml/src/main/java/org/openhab/core/model/yaml/internal/items/YamlItemProvider.java |
Removes Items using DTO name directly rather than building Items for removal lookup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
....model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java
Outdated
Show resolved
Hide resolved
....model.yaml/src/main/java/org/openhab/core/model/yaml/internal/things/YamlThingProvider.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Ravi Nadahar <[email protected]>
Signed-off-by: Laurent Garnier <[email protected]>
The enhancement consists in just getting the UID of the elements to be removed rather than building a full object.