Add dynamic scripting-language transformation service #3487

jimtng · 2023-03-26T12:24:31Z

See previous discussions in #3465 and #3476
Resolve #3465
Potentially fix #3521

Documentation: openhab/openhab-docs#2042
Breaking change notice: openhab/openhab-distro#1514

Why?

A simpler syntax for file-based transformations, reduces potential errors
Transformation script files will have their own proper extension that corresponds to its actual language, e.g. .js, .rb, .py, etc. This is a much more elegant way than using .script which is very confusing especially when utilising multiple languages.
Simplified UI when choosing script based profile transformations. Users only need to select the language when creating the transformation, and not having to choose the language again each and every time they need to use the transformation. This saves a tremendous amount of effort when dealing with many item-to-channel links.
Removes the potential language mismatch that was previously possible especially through the UI
The UI will only show the transformations in the relevant language chosen, unlike previously where there was no way to identify the actual language of a .script transformation file.
Continued support / backwards compatibility, JS() transformation will continue to work in openHAB 4.0

SCRIPT transformation itself is removed completely.
Instead, it creates the transformation for whatever language is registered in the system:

DSL
JS
RB
GROOVY
PY

It will only show the ones that are actually installed. Here, I have Ruby, Jython and JS Scripting installed and Groovy not installed, so GROOVY is not showing up on the list

Also note that it only shows the scripts for the selected language. Above shows only Ruby scripts, and below, only JS scripts:

The profile URI for Ruby transformation is profile:transform:RB which corresponds to the .rb extension
The profile URI for JS transformation is profile:transform:JS which corresponds to the .js extension
... and so on.

Furthermore, the configuration options can also be localised, although I can't seem to get anything (including MAP, REGEX, etc) to show any language other than English on my system, even after changing my language regional setting in openhab.

It should be compatible with the old JS transformation too. Here's my sitemap:

		Switch item=Black_Alert label="Black Alert [JS(bro.js):%s]"

bro.js:

(function (i) {
  return i + " Bro!";
})(input)

Even inline JS transformation works like it used to:

		Switch item=Black_Alert label="Black Alert [JS(| input + \" - Inline Bro!\"):%s]"

And now Ruby inline transformation works too:

		Switch item=Black_Alert label="Black Alert [RB(| \" #{input} - Ruby Inline Bro!\"):%s]"

And here's the result of the REST rest/transformation/services

[
  "JSONPATH",
  "REGEX",
  "MAP",
  "DSL",
  "RB",
  "JS",
  "PY"
]

And the result of rest/profile-types (with other profile entries removed for brevity)

[
  {
    "uid": "transform:RB",
    "label": "RB",
    "kind": "STATE",
    "supportedItemTypes": []
  },
  {
    "uid": "transform:JS",
    "label": "JS",
    "kind": "STATE",
    "supportedItemTypes": []
  },
  {
    "uid": "transform:PY",
    "label": "PY",
    "kind": "STATE",
    "supportedItemTypes": []
  },
  {
    "uid": "transform:DSL",
    "label": "DSL",
    "kind": "STATE",
    "supportedItemTypes": []
  }
]

J-N-K · 2023-03-26T15:54:54Z

I'll have a deeper look later but two things that I found immediately:

You should use a factory component for generating the transformation services (have a look at ManagedUIComponentProvider and UIComponentRegistryFactoryImpl). You can then inject the TransformationRegistry and the ScriptEngineManager directly to the transformation service
There should be a better label than "RB" for the transformation
What do you intend to do with the different config descriptions? At first glance it looks unnecessary complex
Why do you use the factory to get the transformation services to the profile factory? You could also inject them directly.

J-N-K · 2023-03-26T17:12:15Z

Thinking a bit more about it: If you inject the TransformationService to the ProfileFactory you can even make it the profile factory for all TransformationService and move it to org.openhab.transformation.

The config description provider should then be part of the ScriptTransformationService, so the service provides the description (like the XMLs are provided by the respective transformation bundles).

jimtng · 2023-03-26T23:13:47Z

Thanks for the feedback. I'm learning about everything as I go. I'll look into your other suggestions. In the mean time:

There should be a better label than "RB" for the transformation

I was planning to do this later on because at the moment I just wanted to make them the same as any other transformation. I agree that a longer / more descriptive name is better.

What do you intend to do with the different config descriptions? At first glance it looks unnecessary complex

The idea is to keep a static xml for profile:transform:SCRIPT inside script-profile.xml which can then get translated to other locales (via the crowdin facility), and then just duplicate it (at runtime) for all other profile:transform:XX. This way we don't end up duplicating the translation effort for other scripting-languages, and also makes adding new scripting-languages easy without having to keep specific translation for each scripting-language.

Is there a better / simpler mechanism to achieve this?

jimtng · 2023-03-27T03:46:13Z

Thinking a bit more about it: If you inject the TransformationService to the ProfileFactory you can even make it the profile factory for all TransformationService and move it to org.openhab.transformation.

Are you referring to the actual org.openhab.core.thing.profiles.ProfileFactory? This doesn't make sense to me because not all profiles use a transformation service.

Or are you saying that we should create a TransformationProfileFactory and deduplicate it from all the other transformation addons?

Why do you use the factory to get the transformation services to the profile factory? You could also inject them directly.

I don't know how to inject them directly when the target filter is dynamic depending on the scriptType.

scriptTransformationFactory.getTransformationService() doesn't actually create a transformation service on demand. It is merely a helper to find the correct transformation service based on scriptType. I could also get the service using bundleContext.getService with the appropriate filter, but the scriptTransformationFactory is already there and it is also needed for getting the list of scriptTypes.

J-N-K · 2023-03-27T06:45:23Z

I meant a TransformationProfilerFactory. This would reduce code in the transformation add-ons and is very similar to what @kaikreuzer suggested in #2872. If you go that route, you can inject all TransformationServices without filter.

The ScriptTransformationFactory already creates the services (l. 85), I don't think you need the .getTransformationService method at all if you inject them into the profile factory. I think you can use ServiceReference<TransformationService> there instead of TransformationService and then extract the properties from the service reference.

jimtng · 2023-03-27T11:23:44Z

florian-h05 · 2023-03-27T12:32:40Z

Looks good 👍

What happens if you have both Nashorn and Graal installed?

jimtng · 2023-03-27T12:38:51Z

What happens if you have both Nashorn and Graal installed?

jimtng · 2023-03-27T14:05:48Z

I meant a TransformationProfilerFactory. This would reduce code in the transformation add-ons and is very similar to what @kaikreuzer suggested in #2872. If you go that route, you can inject all TransformationServices without filter.

I've just looked into this. Although it appears do-able, it would require some changes / refactoring that would affect many things both in core and in addons which could open a new can of worms. For example, the VATTransformation has a different / custom profile class implementation. It should be tackled in a separate PR.

J-N-K · 2023-03-27T14:34:34Z

Fine with me. Can you push your latest code?

jimtng · 2023-03-27T16:45:30Z

I've incorporated most of your suggestions, except:

Why do you use the factory to get the transformation services to the profile factory? You could also inject them directly.

I can inject all the transformation services using dynamic reference, and keep track of them in a list / map of some sort, but that seems like a lot of duplicate extra work when it's already being kept track of by the ScriptTransformationServiceFactory. Is there a reason why the profilefactory needs to keep its own reference independently?

J-N-K · 2023-03-27T21:21:54Z

Factories should not be used as registries, this is mixing concepts. Please have a look at jimtng#1. IMO also the ConfigDescriptionProvider should be moved to the ScriptTransformationService, it should be possible to create the ConfigDescription in the constructor and then just return that (List.of(configDescription) in getConfigDescriptions and configDescription or null in getConfigDescription).

jimtng · 2023-03-27T23:42:31Z

Thanks for the help, @J-N-K!

One last thing, not sure if it was by intention or not, but you omitted the profile ID and just used the language name in the label. It does look rather nice, but wondering whether we should add the ID back in to look like JS - ECMAScript (ECMAScript 262 Edition 11), NASHORNJS - ECMAScript (ECMA - 262 Edition 5.1) etc. ?

@florian-h05 WDYT?

This is how it looks now:

jimtng · 2023-03-28T00:42:33Z

IMO also the ConfigDescriptionProvider should be moved to the ScriptTransformationService, it should be possible to create the ConfigDescription in the constructor and then just return that (List.of(configDescription) in getConfigDescriptions and configDescription or null in getConfigDescription).

It's a very nice idea. I tried it, but I got stuck because I don't get the locale in the constructor.

florian-h05 · 2023-03-28T10:46:43Z

wondering whether we should add the ID back in to look like JS - ECMAScript (ECMAScript 262 Edition 11), NASHORNJS - ECMAScript (ECMA - 262 Edition 5.1) etc. ?

Looks very good with JS and NASHORNJS 👍
Thanks!

I will adjust the WebUI PR openhab/openhab-webui#1448 as soon as this gets merged.

jimtng · 2023-03-28T10:52:32Z

Looks very good with JS and NASHORNJS

OK I'll add the transformation ID back in a few minutes.

jimtng · 2023-03-28T11:03:49Z

This is how it will look - with the profile sorting PR #3491 included. Kind of looking a bit all over the place.

Just a thought: should we group the SCRIPT ones together (once the profile list is sorted)? example:

Bearing in mind that most people probably only have DSL and one other scripting language installed...

J-N-K

Otherwise LGTM.

...g.openhab.core.transform/src/main/java/org/openhab/core/transform/TransformationService.java

...ipt/src/main/java/org/openhab/core/automation/module/script/ScriptTransformationService.java

J-N-K · 2023-03-28T17:54:25Z

.../main/java/org/openhab/core/automation/module/script/ScriptTransformationServiceFactory.java

+            Dictionary<String, Object> properties = new Hashtable<>();
+            String name = type.toUpperCase();
+            properties.put(TransformationService.SERVICE_PROPERTY_NAME, name);
+            properties.put(TransformationService.SERVICE_PROPERTY_LABEL, name + " - " + languageName);


I would prefer SCRIPT Rule DSL (v1) or SCRIPT ECMAScript 262 Edition. This also groups them nicely with your other PR.

Yes this looks nicer! changed as suggested

I’d like to have the JS and NASHORNJS names still there, because they are mich easier to differ then the ECMAScript Versions, but I am fine with putting a SCRIPT in front of them.

That makes the label very long and duplicates the information for all other languages. Can we modify what is returned by the script engine so it returns „ECMAScript (Nashorn, v5)“ and „ECMAScript (Graal, v11)“ or similar?

I don’t think this is possible, because at least I’ve never noticed GraalJS providing the information that it is Graal.

However looking at the UI for creating new scripts, I think it would be fine if the config description contains the MIME type and UI can display the MIME type. And don’t change the label here.

Alternatively we change the corresponding graaljs and nashorn bundles to somehow return those values, but that involves creating a proxy javax.script.ScriptEngineFactory unless there's a better way to do that.

Both GraalJS and NashornJS addons already have their own ScriptEngineFactory with method overrides to provide e.g. additional MIME types. See https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.automation.jsscriptingnashorn/src/main/java/org/openhab/automation/jsscriptingnashorn/internal/NashornScriptEngineFactory.java and https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java.

Therefore let‘s just override the getLanguageName method there and use the languageName as in the current state of this PR. Therefore this PR should be finished.

Using getEngineName

Using getEngineName IMO is too long. I‘d go for the languageName and overwrite the getLanguageName methods in the JS bundles.

Using getEngineName IMO is too long. I‘d go for the languageName and overwrite the getLanguageName methods in the JS bundles.

I agree. If this is our decision, then there are no further changes needed for this PR?

It will look like this now, before changes in the JS bundles

Looks good 👍

From my POV, this PR is finished.

This replaced SCRIPT transformation with one specific to each language e.g. JS, RB, GROOVY, etc. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

This preserves the scriptType case instead of forcing it to lowercase Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Signed-off-by: Jan N. Klug <github@klug.nrw>

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

jimtng · 2023-04-07T22:36:35Z

Rebased and merge conflicts resolved

kaikreuzer

Thanks!
Since @J-N-K already reviewed and approved it, I only had a glance over the code without actively testing it, but what I saw looks fine.

Closes #1844. * Allows to copy the UID. * Sorts the transformation types in the "by types" list alphabetically and makes the segment labels uppercase to match the transformation service names. * Adjusts to recent core change openhab/openhab-core#3487: removes the script language selection and updates the editor mode and code snippet injection. * Extends language support and refactors the translate mode logic in `script-editor.vue`. * Fixes the clear label button not displayed in create mode and displayed for non-editable transformations. * Shows a tip on how to use the transformation for Item states. * Fixes failing and not required API request after transformation creation. * Removes possibility to select and attempt to delete not-editable transformations, which is not possible and failed. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

openhab-bot · 2023-04-17T14:21:09Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-0-milestone-discussion/145133/186

openhab-bot · 2023-04-21T14:37:47Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/preparation-for-oh-4-0-how-to-find-my-js-transformations/146233/9

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

openhab-bot · 2023-04-27T15:45:13Z

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/problem-with-graaljs-script-transformation-in-things-file/145188/11

* Add dynamic scripting language transformation service This replaced SCRIPT transformation with one specific to each language e.g. JS, RB, GROOVY, etc. Co-authored-by: Jan N. Klug <github@klug.nrw> Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au> GitOrigin-RevId: fbaf992

jimtng requested a review from a team as a code owner March 26, 2023 12:24

jimtng force-pushed the polyglot-transformation branch 3 times, most recently from 1bbc292 to fa3d60b Compare March 26, 2023 12:49

J-N-K mentioned this pull request Mar 26, 2023

Add a transformations menu and editor openhab/openhab-webui#1448

Merged

jimtng mentioned this pull request Mar 26, 2023

Support a cleaner syntax for SCRIPT transformation #3476

Closed

jimtng changed the title ~~Add dynamic scripting language transformation service~~ Add dynamic scripting-language transformation service Mar 26, 2023

jimtng force-pushed the polyglot-transformation branch 2 times, most recently from 4b03a35 to dd98143 Compare March 27, 2023 17:33

jimtng force-pushed the polyglot-transformation branch from e9f79b3 to d63c4b0 Compare March 28, 2023 00:43

jimtng mentioned this pull request Mar 28, 2023

Document the new dynamic scripting-language transformation openhab/openhab-docs#2042

Merged

jimtng force-pushed the polyglot-transformation branch from 7206c5d to c197d79 Compare March 28, 2023 10:43

J-N-K requested changes Mar 28, 2023

View reviewed changes

jimtng and others added 12 commits April 7, 2023 23:02

Add dynamic scripting language transformation service

bc4f81c

This replaced SCRIPT transformation with one specific to each language e.g. JS, RB, GROOVY, etc. Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

DRY and optimize lookups

ed799f9

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Add language name label, refactor using component factory

f519f91

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Add a special property to hold the scriptType

970dfc4

This preserves the scriptType case instead of forcing it to lowercase Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

refactoring

7864968

Signed-off-by: Jan N. Klug <github@klug.nrw>

Fix the service id key

614ba0d

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Move ConfigDescription to ScriptTransformationService

f2e6e27

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

simplify uri check

34c05d6

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Add the profile ID to the label as prefix to the language name

083c6b6

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Make constants static

117fb41

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Implement requested changes

e34b22c

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

Use the engineFactory as the hash key

3592c8e

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

jimtng force-pushed the polyglot-transformation branch from 7990a14 to 3592c8e Compare April 7, 2023 13:03

resolving merge conflict

02b521b

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>

jimtng force-pushed the polyglot-transformation branch from 20cf167 to 02b521b Compare April 7, 2023 13:12

kaikreuzer approved these changes Apr 12, 2023

View reviewed changes

kaikreuzer merged commit fbaf992 into openhab:main Apr 12, 2023

kaikreuzer added the enhancement An enhancement or new feature of the Core label Apr 12, 2023

kaikreuzer added this to the 4.0 milestone Apr 12, 2023

jimtng deleted the polyglot-transformation branch April 13, 2023 23:13

florian-h05 mentioned this pull request Apr 16, 2023

Transformations: Various enhancements & fixes openhab/openhab-webui#1845

Merged

florian-h05 added a commit to florian-h05/openhab-js that referenced this pull request Apr 24, 2023

README: Update transformation docs for recent core changes

55bfb4a

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

SteWers mentioned this pull request May 8, 2023

Explain upgrade tool openhab/openhab-docs#2073

Merged

jimtng mentioned this pull request Jul 22, 2023

[jrubyscripting] Update README openhab/openhab-addons#15287

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dynamic scripting-language transformation service #3487

Add dynamic scripting-language transformation service #3487

jimtng commented Mar 26, 2023 •

edited

Loading

J-N-K commented Mar 26, 2023

J-N-K commented Mar 26, 2023 •

edited

Loading

jimtng commented Mar 26, 2023 •

edited

Loading

jimtng commented Mar 27, 2023

J-N-K commented Mar 27, 2023

jimtng commented Mar 27, 2023

florian-h05 commented Mar 27, 2023

jimtng commented Mar 27, 2023

jimtng commented Mar 27, 2023 •

edited

Loading

J-N-K commented Mar 27, 2023

jimtng commented Mar 27, 2023

J-N-K commented Mar 27, 2023

jimtng commented Mar 27, 2023 •

edited

Loading

jimtng commented Mar 28, 2023

florian-h05 commented Mar 28, 2023 •

edited

Loading

jimtng commented Mar 28, 2023

jimtng commented Mar 28, 2023 •

edited

Loading

J-N-K left a comment

J-N-K Mar 28, 2023

jimtng Mar 28, 2023

florian-h05 Mar 28, 2023

J-N-K Mar 28, 2023

florian-h05 Mar 28, 2023

florian-h05 Mar 28, 2023 •

edited

Loading

jimtng Mar 28, 2023

florian-h05 Mar 28, 2023

jimtng Mar 28, 2023 •

edited

Loading

florian-h05 Mar 28, 2023

jimtng commented Apr 7, 2023

kaikreuzer left a comment

openhab-bot commented Apr 17, 2023

openhab-bot commented Apr 21, 2023

openhab-bot commented Apr 27, 2023

Add dynamic scripting-language transformation service #3487

Add dynamic scripting-language transformation service #3487

Conversation

jimtng commented Mar 26, 2023 • edited Loading

J-N-K commented Mar 26, 2023

J-N-K commented Mar 26, 2023 • edited Loading

jimtng commented Mar 26, 2023 • edited Loading

jimtng commented Mar 27, 2023

J-N-K commented Mar 27, 2023

jimtng commented Mar 27, 2023

florian-h05 commented Mar 27, 2023

jimtng commented Mar 27, 2023

jimtng commented Mar 27, 2023 • edited Loading

J-N-K commented Mar 27, 2023

jimtng commented Mar 27, 2023

J-N-K commented Mar 27, 2023

jimtng commented Mar 27, 2023 • edited Loading

jimtng commented Mar 28, 2023

florian-h05 commented Mar 28, 2023 • edited Loading

jimtng commented Mar 28, 2023

jimtng commented Mar 28, 2023 • edited Loading

J-N-K left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

florian-h05 Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimtng Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimtng commented Apr 7, 2023

kaikreuzer left a comment

Choose a reason for hiding this comment

openhab-bot commented Apr 17, 2023

openhab-bot commented Apr 21, 2023

openhab-bot commented Apr 27, 2023

jimtng commented Mar 26, 2023 •

edited

Loading

J-N-K commented Mar 26, 2023 •

edited

Loading

jimtng commented Mar 26, 2023 •

edited

Loading

jimtng commented Mar 27, 2023 •

edited

Loading

jimtng commented Mar 27, 2023 •

edited

Loading

florian-h05 commented Mar 28, 2023 •

edited

Loading

jimtng commented Mar 28, 2023 •

edited

Loading

florian-h05 Mar 28, 2023 •

edited

Loading

jimtng Mar 28, 2023 •

edited

Loading