[rest] Add semantic tag registry + API to manage user tags #3646

lolodomo · 2023-06-07T18:19:53Z

Related to #3619

New registry for semantic tags.
New default semantic tags provider for all built-in semantic tags.
New managed provider to add/remove/update user semantic tags.
Storage of user semantic tags in a JSON DB file.
New REST API to add/remove/update user tags in the semantic model.
New REST API to get a sub-tree of the semantic tags.

Semantic tag class annotations are removed.

Semantic tag classes are now created at runtime.
Classes Locations, Equipments, Points and Properties are removed

Static methods SemanticTags.add removed
The adding of semantic tag classes is now managed only by the tag registry.

Avoids calling static method SemanticTags.getById when possible

SemanticsMetadataProvider service now requires semanticTagRegistry to start.

Signed-off-by: Laurent Garnier lg.hc@free.fr

lolodomo · 2023-06-07T18:27:44Z

@J-N-K : I am sorry, I was constrained to create a new PR, the previous PR #3636 was no more updating with my last commits.

The last change is the full removal of the tag class annotation (TagInfo). It should answer to @splatch comments (in the other PR) about TagInfo.

lolodomo · 2023-06-07T18:29:57Z

Remains one comment from @splatch I have not yet answered:

Custom tags will not be properly recognized by SemanticsMetadataProvider, if they are registered later. There is fair chance that they will work on random basis, depending on initialization order of elements which register tags at runtime.

lolodomo · 2023-06-08T20:52:59Z

I have now considered most of @splatch comments done in the old PR, the most important being the removal of static methods in SemanticTags that allowed to add tag class.
It should be ready for a new review.

lolodomo · 2023-06-09T06:43:53Z

A final effort would be required to remove the last remaining static stuff in SemanticsTat class.
I will have a look this evening.

lolodomo · 2023-06-10T04:28:39Z

A final effort would be required to remove the last remaining static stuff in SemanticsTat class. I will have a look this evening.

Done.
It is not possible to remove all static methods. For example, they are used for rules.

Related to openhab#3619 New registry for semantic tags. New default semantic tags provider for all built-in semantic tags. New managed provider to add/remove/update user semantic tags. Storage of user semantic tags in a JSON DB file. New REST API to add/remove/update user tags in the semantic model. New REST API to get a sub-tree of the semantic tags. Semantic tag class annotations are removed. Semantic tag classes are now created at runtime. Classes Locations, Equipments, Points and Properties are removed Static methods SemanticTags.add removed The adding of semantic tag classes is now managed only by the tag registry. Avoids calling static method SemanticTags.getById when possible SemanticsMetadataProvider service now requires semanticTagRegistry to start. Signed-off-by: Laurent Garnier <lg.hc@free.fr>

lolodomo · 2023-06-10T05:17:42Z

@J-N-K : I squashed all my commits. It is in my opinion now finished and ready for a review.

@J-N-K : did you start implementing a config file provider based on YAML ? You said you were interested to work on that. If not, I will start the implementation. This will be of course in another PR.

lolodomo · 2023-06-10T05:23:58Z

Custom tags will not be properly recognized by SemanticsMetadataProvider, if they are registered later. There is fair chance that they will work on random basis, depending on initialization order of elements which register tags at runtime.

It is now OK because SemanticsMetadataProvider is now started only when the semantic tag registry is started. I added an OISGi dependency. So we are sure that all default tags and all managed tags are there when this service is started.
Later, when a config file provider will be implemented, we will have to take care again about that.

J-N-K · 2023-06-10T06:53:34Z

@lolodomo See main...J-N-K:openhab-core:yaml It is more a proof-of-concept than a full implementation, but I think that's the way to do it.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

lolodomo · 2023-06-10T12:44:21Z

@lolodomo See main...J-N-K:openhab-core:yaml It is more a proof-of-concept than a full implementation, but I think that's the way to do it.

Ok, thank you, I will do it, your example will really help me, in particular the watch service stuff.

....core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/tag/TagResource.java

...st.core/src/main/java/org/openhab/core/io/rest/core/internal/tag/EnrichedSemanticTagDTO.java

...ore.semantics/src/main/java/org/openhab/core/semantics/internal/SemanticTagRegistryImpl.java

...nhab.core.semantics/src/main/java/org/openhab/core/semantics/ManagedSemanticTagProvider.java

J-N-K · 2023-06-10T17:53:06Z

...ore.semantics/src/main/java/org/openhab/core/semantics/model/DefaultSemanticTagProvider.java

+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.core.semantics.model;


I think you should move this to the internal (or semantics) package, because that's where the managed provider is.

I asked myself the question.
As it is a generated file, isn't it important to put it in a separate package to clearly identify it ?

In other bundles we use src/generated/java/org/... for that.

Can you please tell me where I can find an example ?
I will have to add the generated path in the build path.

org.openhab.core.thing would be an example. But the classes are generated on the fly during build, I think this is not the case here. So maybe we leave it in place now. In the long run (when we have a YAML based provider), we could convert the CSV to YAML and then load that immediately, when we add the YAML based provider.

...org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/SemanticTagRegistry.java

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Also remove method isRemovable from registry interface. Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

lolodomo · 2023-06-11T10:32:26Z

The build failure has apparently no link with my PR:

[ERROR] Failed to read artifact descriptor for org.openhab.tools.sat.custom-checks:findbugs:jar:0.15.0: Could not transfer artifact org.openhab.tools.sat.custom-checks:findbugs:pom:0.15.0 from/to central (https://repo1.maven.org/maven2): /home/jenkins/jenkins-agent1/maven-repositories/3/org/openhab/tools/sat/custom-checks/findbugs/0.15.0/findbugs-0.15.0.pom.part (No such file or directory) -> [Help 1]

J-N-K

In general looks good. It's not so easy to review if the behavior is always the same as before because the general concept changed. But there are a lot of tests, also for the new classes and that all looks good to me.

I have left only some minor comments and would prefer to merge ASAP.

....core.io.rest.core/src/main/java/org/openhab/core/io/rest/core/internal/tag/TagResource.java

...org.openhab.core.semantics/src/main/java/org/openhab/core/semantics/SemanticTagRegistry.java

J-N-K · 2023-06-14T18:56:27Z

...ore.semantics/src/main/java/org/openhab/core/semantics/model/DefaultSemanticTagProvider.java

+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.core.semantics.model;


org.openhab.core.thing would be an example. But the classes are generated on the fly during build, I think this is not the case here. So maybe we leave it in place now. In the long run (when we have a YAML based provider), we could convert the CSV to YAML and then load that immediately, when we add the YAML based provider.

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

lolodomo · 2023-06-14T20:18:29Z

I have left only some minor comments and would prefer to merge ASAP

I believe you can merge now ;)

splatch · 2023-06-14T20:30:06Z

@lolodomo FYI, I wish to review changes later this or next week, in case I find anything I'll try to share them as a post-portem review. Sadly I ran out of spare time to catch up with what you did.
I do belive that major problem, which was related to coordination of access to static classes is gone, thus prime risk factor in this feature is eliminated.
Thank you for your work.

lolodomo · 2023-06-15T05:35:24Z

IMHO, if this is fine for core maintainers, we should not delay the merge of this PR, considering the OH4 release planning and if we want a chance UI part to be implemented in time.

If necessary, enhancements could be provided later. One of them is the config file provider I am starting to work on and I hope I can submit another PR during the weekend or next week.

J-N-K · 2023-06-15T18:17:30Z

@lolodomo It seems there is a conflict after I merged the REST cache. Can you rebase? Thanks.

lolodomo · 2023-06-15T20:56:39Z

@lolodomo It seems there is a conflict after I merged the REST cache. Can you rebase? Thanks.

Conflict resolved.

lolodomo · 2023-06-16T14:54:58Z

Thank you @J-N-K .

Can one of you trigger a new build of core framework so that we can then rebuild my HABot PR. We need to merge it ASAP now. Until that, HABot will no more compile in next snapshots.

J-N-K · 2023-06-16T15:39:23Z

Done: https://ci.openhab.org/job/openHAB-Core/1221/

Refs openhab/openhab-core#3646. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

) * Add semantic tag registry + REST API to manage user tags Related to openhab#3619 New registry for semantic tags. New default semantic tags provider for all built-in semantic tags. New managed provider to add/remove/update user semantic tags. Storage of user semantic tags in a JSON DB file. New REST API to add/remove/update user tags in the semantic model. New REST API to get a sub-tree of the semantic tags. Semantic tag class annotations are removed. Semantic tag classes are now created at runtime. Classes Locations, Equipments, Points and Properties are removed Static methods SemanticTags.add removed The adding of semantic tag classes is now managed only by the tag registry. Avoids calling static method SemanticTags.getById when possible SemanticsMetadataProvider service now requires semanticTagRegistry to start. Signed-off-by: Laurent Garnier <lg.hc@free.fr> GitOrigin-RevId: f86635f

lolodomo requested a review from a team as a code owner June 7, 2023 18:19

lolodomo changed the title ~~Custom tagsAdd semantic tag registry + REST API to manage user tags~~ Add semantic tag registry + REST API to manage user tags Jun 8, 2023

lolodomo force-pushed the custom_tags branch from 4c01507 to 2324794 Compare June 10, 2023 05:10

lolodomo added 2 commits June 10, 2023 10:05

Add methods isEditable and isRemovable to the registry

3fe6893

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Add method removeSubTree to the registry

a49a993

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

J-N-K requested changes Jun 10, 2023

View reviewed changes

lolodomo added 5 commits June 11, 2023 09:02

Review comments: Comparator, toList, code to be moved into catch

8b0d4a5

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Review comment: method buildId

e93d7e1

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Fix findings by code analysis

f80d82b

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Review comment: remove field removable from REST API result

a66281c

Also remove method isRemovable from registry interface. Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Method isNewIdValid renamed into canBeAdded

226cfce

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

lolodomo force-pushed the custom_tags branch from e67cda9 to 226cfce Compare June 11, 2023 09:45

J-N-K requested changes Jun 14, 2023

View reviewed changes

lolodomo added 4 commits June 14, 2023 21:21

Review comment: fix comments

4113f90

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Avoid using method buildId when not necessary

218d578

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Review comment: remove method getTagResponse

3c8b2c6

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Fix parameter descriptions in REST API

52b2943

Signed-off-by: Laurent Garnier <lg.hc@free.fr>

J-N-K added the rebuild Triggers the Jenkins PR build label Jun 15, 2023

J-N-K approved these changes Jun 15, 2023

View reviewed changes

J-N-K added enhancement An enhancement or new feature of the Core and removed rebuild Triggers the Jenkins PR build labels Jun 15, 2023

Merge branch 'main' into custom_tags

f54ea18

J-N-K merged commit f86635f into openhab:main Jun 16, 2023

J-N-K changed the title ~~Add semantic tag registry + REST API to manage user tags~~ [rest] Add semantic tag registry + API to manage user tags Jun 16, 2023

J-N-K added this to the 4.0 milestone Jun 16, 2023

lolodomo mentioned this pull request Jun 16, 2023

Dynamically load the list of Semantic tags and store them in Vuex openhab/openhab-webui#1882

Merged

florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Jun 18, 2023

Adjust to breaking core changes

0bca358

Refs openhab/openhab-core#3646. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

lolodomo deleted the custom_tags branch January 4, 2024 23:55

florian-h05 mentioned this pull request Feb 23, 2024

Add a tags edit page openhab/openhab-webui#2397

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rest] Add semantic tag registry + API to manage user tags #3646

[rest] Add semantic tag registry + API to manage user tags #3646

lolodomo commented Jun 7, 2023 •

edited

Loading

lolodomo commented Jun 7, 2023

lolodomo commented Jun 7, 2023

lolodomo commented Jun 8, 2023

lolodomo commented Jun 9, 2023

lolodomo commented Jun 10, 2023

lolodomo commented Jun 10, 2023

lolodomo commented Jun 10, 2023

J-N-K commented Jun 10, 2023 •

edited

Loading

lolodomo commented Jun 10, 2023

J-N-K Jun 10, 2023

lolodomo Jun 10, 2023 •

edited

Loading

J-N-K Jun 11, 2023

lolodomo Jun 11, 2023

J-N-K Jun 14, 2023

lolodomo Jun 14, 2023

lolodomo commented Jun 11, 2023

J-N-K left a comment

J-N-K Jun 14, 2023

lolodomo commented Jun 14, 2023

splatch commented Jun 14, 2023

lolodomo commented Jun 15, 2023 •

edited

Loading

J-N-K commented Jun 15, 2023

lolodomo commented Jun 15, 2023

lolodomo commented Jun 16, 2023 •

edited

Loading

J-N-K commented Jun 16, 2023

[rest] Add semantic tag registry + API to manage user tags #3646

[rest] Add semantic tag registry + API to manage user tags #3646

Conversation

lolodomo commented Jun 7, 2023 • edited Loading

lolodomo commented Jun 7, 2023

lolodomo commented Jun 7, 2023

lolodomo commented Jun 8, 2023

lolodomo commented Jun 9, 2023

lolodomo commented Jun 10, 2023

lolodomo commented Jun 10, 2023

lolodomo commented Jun 10, 2023

J-N-K commented Jun 10, 2023 • edited Loading

lolodomo commented Jun 10, 2023

J-N-K Jun 10, 2023

Choose a reason for hiding this comment

lolodomo Jun 10, 2023 • edited Loading

Choose a reason for hiding this comment

J-N-K Jun 11, 2023

Choose a reason for hiding this comment

lolodomo Jun 11, 2023

Choose a reason for hiding this comment

J-N-K Jun 14, 2023

Choose a reason for hiding this comment

lolodomo Jun 14, 2023

Choose a reason for hiding this comment

lolodomo commented Jun 11, 2023

J-N-K left a comment

Choose a reason for hiding this comment

J-N-K Jun 14, 2023

Choose a reason for hiding this comment

lolodomo commented Jun 14, 2023

splatch commented Jun 14, 2023

lolodomo commented Jun 15, 2023 • edited Loading

J-N-K commented Jun 15, 2023

lolodomo commented Jun 15, 2023

lolodomo commented Jun 16, 2023 • edited Loading

J-N-K commented Jun 16, 2023

lolodomo commented Jun 7, 2023 •

edited

Loading

J-N-K commented Jun 10, 2023 •

edited

Loading

lolodomo Jun 10, 2023 •

edited

Loading

lolodomo commented Jun 15, 2023 •

edited

Loading

lolodomo commented Jun 16, 2023 •

edited

Loading