[jsscripting] Reimplement timer creation method of `ScriptExecution` #13695

florian-h05 · 2022-11-11T15:12:36Z

Description

This PR reimplements the openHAB Core API for creating timers to make this also threadsafe.

As discussed on the forum https://community.openhab.org/t/js-scripting-why-are-we-deprecated-createtimer/140748, the setTimeout and setInterval methods do not provide the functionality that advanced users need, and to avoid that those users and users coming from Rules DSL leave JS Scripting because of this missing API, we have to provide thread-safe access to it.

Furthermore, JSScriptServiceUtil was introduced to provide easy access from to script services provided by core. This replaces the usage of the JSScriptServiceUtil from the Rules DSL parts of core and therefore reduces dependency on a part that might become an addon in the future.

openhab-js is also upgraded to 2.1.1.

Testing

Use the following file-based script to test:

const { time} = require('openhab');

// Note that all timers are scheduled to nearly the same time, so without the synchronization mechanism in place, this would lead to a multi-thread exception
ThreadsafeTimers.createTimer(time.toZDT(), () => {
    console.info("Hello world from createTimer without identifier")
});

ThreadsafeTimers.createTimer('createTimer', time.toZDT(), () => {
    console.info("Hello world from createTimer with identifier")
});

…ead of in an extra methods Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…ript services Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

… for thread-safety Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 · 2022-11-11T15:13:13Z

@rkoshak
PR is now open.

@jlaur @jpg0 Could you please give your feedback on this?

openhab-bot · 2022-11-11T15:15:07Z

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

https://community.openhab.org/t/js-scripting-why-are-we-deprecated-createtimer/140748/18

...ng/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java

...ting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptServiceUtil.java

...ng/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 · 2022-11-12T14:19:08Z

@jpg0 I addressed your review, could you please re-review?

digitaldan

LGTM

jpg0

Only issue I have is with the use of OSGi. If it's a common pattern in openHAB I'm not concerned about it though.

...scripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java

…th an injection mechanism Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

…raalJSScriptEngineFactory` Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

jlaur · 2022-11-12T21:01:54Z

@kaikreuzer - it seems that @jpg0 is missing as member of the addons contributors group, could you add him?

* minor improvements * some more Signed-off-by: Jan N. Klug <github@klug.nrw>

…y of `JSRuntimeFeatures` Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 · 2022-11-12T21:55:15Z

FYI: DCO is failing because of @J-N-K GitHub username is not his real name, which he used to signoff.

florian-h05 · 2022-11-20T16:53:38Z

@jlaur This is ready for your review and merging now.

FYI: digitaldan and jpg0 approved at some point, after that I’ve changed the implementation of JSScriptServiceUtil (as jpg0 and J-N-K asked me to). J-N-K gave his approval in #13695 (comment).
After all that reviews, I only upgraded the included openhab-js version and removed some unused code.

jlaur

Thanks! Only a small simplification/optimization suggested.

...ng/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 · 2022-11-20T20:25:39Z

@jlaur I addressed your review, let's merge.

jlaur

LGTM

florian-h05 · 2022-11-20T20:39:04Z

@jlaur All checks have passed.

openhab-bot · 2022-11-20T22:42:40Z

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

https://community.openhab.org/t/js-scripting-why-are-we-deprecated-createtimer/140748/22

@J-N-K

…penhab#13695) * [jsscripting] Refactor ThreadsafeTimers to create futures inline instead of in an extra methods * [jsscripting] Introduce utility class for providing easy access to script services * [jsscripting] Reimplement timer creation methods from ScriptExecution for thread-safety * [jsscripting] Add missing JavaDoc for reimplement timer creation methods * [jsscripting] Remove the future from the map when setTimeout expires * [jsscripting] Rename `GraalJSScriptServiceUtil` to `JSScriptServiceUtil` * [jsscripting] Remove the `createTimerWithArgument` method * [jsscripting] Replace the OSGi workaround of `JSScriptServiceUtil` with an injection mechanism * [jsscripting] Use constructor to inject `JSScriptServiceUtil` into `GraalJSScriptEngineFactory` * [jsscripting] Minor improvements by @J-N-K (#1) * [jsscripting] Minor changes related to last commit to keep flexibility of `JSRuntimeFeatures` * [jsscripting] Upgrade openhab-js to v2.1.1 * [jsscripting] Remove unused code Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jan N. Klug <github@klug.nrw> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

openhab-bot · 2023-01-01T09:20:20Z

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

https://community.openhab.org/t/typeerror-timer-reschedule-is-not-a-function/141518/10

@J-N-K

…penhab#13695) * [jsscripting] Refactor ThreadsafeTimers to create futures inline instead of in an extra methods * [jsscripting] Introduce utility class for providing easy access to script services * [jsscripting] Reimplement timer creation methods from ScriptExecution for thread-safety * [jsscripting] Add missing JavaDoc for reimplement timer creation methods * [jsscripting] Remove the future from the map when setTimeout expires * [jsscripting] Rename `GraalJSScriptServiceUtil` to `JSScriptServiceUtil` * [jsscripting] Remove the `createTimerWithArgument` method * [jsscripting] Replace the OSGi workaround of `JSScriptServiceUtil` with an injection mechanism * [jsscripting] Use constructor to inject `JSScriptServiceUtil` into `GraalJSScriptEngineFactory` * [jsscripting] Minor improvements by @J-N-K (openhab#1) * [jsscripting] Minor changes related to last commit to keep flexibility of `JSRuntimeFeatures` * [jsscripting] Upgrade openhab-js to v2.1.1 * [jsscripting] Remove unused code Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jan N. Klug <github@klug.nrw>

@J-N-K

…penhab#13695) * [jsscripting] Refactor ThreadsafeTimers to create futures inline instead of in an extra methods * [jsscripting] Introduce utility class for providing easy access to script services * [jsscripting] Reimplement timer creation methods from ScriptExecution for thread-safety * [jsscripting] Add missing JavaDoc for reimplement timer creation methods * [jsscripting] Remove the future from the map when setTimeout expires * [jsscripting] Rename `GraalJSScriptServiceUtil` to `JSScriptServiceUtil` * [jsscripting] Remove the `createTimerWithArgument` method * [jsscripting] Replace the OSGi workaround of `JSScriptServiceUtil` with an injection mechanism * [jsscripting] Use constructor to inject `JSScriptServiceUtil` into `GraalJSScriptEngineFactory` * [jsscripting] Minor improvements by @J-N-K (#1) * [jsscripting] Minor changes related to last commit to keep flexibility of `JSRuntimeFeatures` * [jsscripting] Upgrade openhab-js to v2.1.1 * [jsscripting] Remove unused code Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jan N. Klug <github@klug.nrw>

@J-N-K

…penhab#13695) * [jsscripting] Refactor ThreadsafeTimers to create futures inline instead of in an extra methods * [jsscripting] Introduce utility class for providing easy access to script services * [jsscripting] Reimplement timer creation methods from ScriptExecution for thread-safety * [jsscripting] Add missing JavaDoc for reimplement timer creation methods * [jsscripting] Remove the future from the map when setTimeout expires * [jsscripting] Rename `GraalJSScriptServiceUtil` to `JSScriptServiceUtil` * [jsscripting] Remove the `createTimerWithArgument` method * [jsscripting] Replace the OSGi workaround of `JSScriptServiceUtil` with an injection mechanism * [jsscripting] Use constructor to inject `JSScriptServiceUtil` into `GraalJSScriptEngineFactory` * [jsscripting] Minor improvements by @J-N-K (#1) * [jsscripting] Minor changes related to last commit to keep flexibility of `JSRuntimeFeatures` * [jsscripting] Upgrade openhab-js to v2.1.1 * [jsscripting] Remove unused code Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jan N. Klug <github@klug.nrw>

@J-N-K

…penhab#13695) * [jsscripting] Refactor ThreadsafeTimers to create futures inline instead of in an extra methods * [jsscripting] Introduce utility class for providing easy access to script services * [jsscripting] Reimplement timer creation methods from ScriptExecution for thread-safety * [jsscripting] Add missing JavaDoc for reimplement timer creation methods * [jsscripting] Remove the future from the map when setTimeout expires * [jsscripting] Rename `GraalJSScriptServiceUtil` to `JSScriptServiceUtil` * [jsscripting] Remove the `createTimerWithArgument` method * [jsscripting] Replace the OSGi workaround of `JSScriptServiceUtil` with an injection mechanism * [jsscripting] Use constructor to inject `JSScriptServiceUtil` into `GraalJSScriptEngineFactory` * [jsscripting] Minor improvements by @J-N-K (#1) * [jsscripting] Minor changes related to last commit to keep flexibility of `JSRuntimeFeatures` * [jsscripting] Upgrade openhab-js to v2.1.1 * [jsscripting] Remove unused code Signed-off-by: Florian Hotze <florianh_dev@icloud.com> Co-authored-by: Jan N. Klug <github@klug.nrw> Signed-off-by: Andras Uhrin <andras.uhrin@gmail.com>

florian-h05 added 4 commits November 10, 2022 22:49

[jsscripting] Refactor ThreadsafeTimers to create futures inline inst…

1a0d7d0

…ead of in an extra methods Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Introduce utility class for providing easy access to sc…

689553e

…ript services Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Reimplement timer creation methods from ScriptExecution…

e458f89

… for thread-safety Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Add missing JavaDoc for reimplement timer creation methods

d3a99c6

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 added enhancement An enhancement or new feature for an existing add-on awaiting other PR Depends on another PR labels Nov 11, 2022

florian-h05 mentioned this pull request Nov 11, 2022

Refactor ScriptExecution to plain Java ScriptExtension openhab/openhab-core#3155

Merged

florian-h05 requested a review from jlaur November 11, 2022 15:15

jlaur requested review from a team and digitaldan and removed request for jlaur November 11, 2022 15:27

digitaldan reviewed Nov 11, 2022

View reviewed changes

...ng/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java Show resolved Hide resolved

jpg0 reviewed Nov 11, 2022

View reviewed changes

florian-h05 added 3 commits November 11, 2022 22:21

[jsscripting] Remove the future from the map when setTimeout expires

838d0e7

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Rename GraalJSScriptServiceUtil to JSScriptServiceUtil

e0a2053

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

[jsscripting] Remove the createTimerWithArgument method

d61a625

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 mentioned this pull request Nov 12, 2022

[actions] Don't deprecate ScriptExecution.createTimer openhab/openhab-js#171

Merged

digitaldan approved these changes Nov 12, 2022

View reviewed changes

jpg0 approved these changes Nov 12, 2022

View reviewed changes

...scripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java Outdated Show resolved Hide resolved

[jsscripting] Replace the OSGi workaround of JSScriptServiceUtil wi…

fb0ca2d

…th an injection mechanism Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 force-pushed the jsscripting-reimplement-createtimer branch from 21c04be to fb0ca2d Compare November 12, 2022 19:26

[jsscripting] Use constructor to inject JSScriptServiceUtil into `G…

c35ba71

…raalJSScriptEngineFactory` Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

J-N-K and others added 2 commits November 12, 2022 22:13

[jsscripting] Minor improvements by @J-N-K (#1)

417267c

* minor improvements * some more Signed-off-by: Jan N. Klug <github@klug.nrw>

[jsscripting] Minor changes related to last commit to keep flexibilit…

77bc18a

…y of `JSRuntimeFeatures` Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 requested a review from J-N-K November 12, 2022 22:16

florian-h05 removed the awaiting other PR Depends on another PR label Nov 19, 2022

florian-h05 changed the title ~~[jsscripting] Reimplement timer creation methods of ScriptExecution~~ [WIP] [jsscripting] Reimplement timer creation methods of ScriptExecution Nov 19, 2022

J-N-K closed this Nov 20, 2022

J-N-K reopened this Nov 20, 2022

florian-h05 force-pushed the jsscripting-reimplement-createtimer branch from 78a41e0 to b447b57 Compare November 20, 2022 10:34

florian-h05 added the awaiting other PR Depends on another PR label Nov 20, 2022

florian-h05 changed the title ~~[WIP] [jsscripting] Reimplement timer creation methods of ScriptExecution~~ [jsscripting] Reimplement timer creation methods of ScriptExecution Nov 20, 2022

jlaur added rebuild Triggers Jenkins PR build and removed awaiting other PR Depends on another PR rebuild Triggers Jenkins PR build labels Nov 20, 2022

jlaur requested changes Nov 20, 2022

View reviewed changes

...ng/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java Outdated Show resolved Hide resolved

[jsscripting] Address review

cdbc7b0

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

florian-h05 requested a review from jlaur November 20, 2022 20:24

florian-h05 changed the title ~~[jsscripting] Reimplement timer creation methods of ScriptExecution~~ [jsscripting] Reimplement timer creation method of ScriptExecution Nov 20, 2022

jlaur approved these changes Nov 20, 2022

View reviewed changes

jlaur merged commit bfff07b into openhab:main Nov 20, 2022

jlaur added this to the 3.4 milestone Nov 20, 2022

florian-h05 deleted the jsscripting-reimplement-createtimer branch November 20, 2022 21:12

This was referenced Nov 29, 2022

[jsscripting] Refactor dependency tracking #13756

Merged

[helpers.js] The centralized function that creates timers should not use createTimerWithArgument rkoshak/openhab-rules-tools#75

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jsscripting] Reimplement timer creation method of `ScriptExecution` #13695

[jsscripting] Reimplement timer creation method of `ScriptExecution` #13695

florian-h05 commented Nov 11, 2022 •

edited

Loading

florian-h05 commented Nov 11, 2022

openhab-bot commented Nov 11, 2022

florian-h05 commented Nov 12, 2022 •

edited

Loading

digitaldan left a comment

jpg0 left a comment

jlaur commented Nov 12, 2022

florian-h05 commented Nov 12, 2022

florian-h05 commented Nov 20, 2022

jlaur left a comment

florian-h05 commented Nov 20, 2022 •

edited

Loading

jlaur left a comment

florian-h05 commented Nov 20, 2022

openhab-bot commented Nov 20, 2022

openhab-bot commented Jan 1, 2023

[jsscripting] Reimplement timer creation method of ScriptExecution #13695

[jsscripting] Reimplement timer creation method of ScriptExecution #13695

Conversation

florian-h05 commented Nov 11, 2022 • edited Loading

Description

Testing

florian-h05 commented Nov 11, 2022

openhab-bot commented Nov 11, 2022

florian-h05 commented Nov 12, 2022 • edited Loading

digitaldan left a comment

Choose a reason for hiding this comment

jpg0 left a comment

Choose a reason for hiding this comment

jlaur commented Nov 12, 2022

florian-h05 commented Nov 12, 2022

florian-h05 commented Nov 20, 2022

jlaur left a comment

Choose a reason for hiding this comment

florian-h05 commented Nov 20, 2022 • edited Loading

jlaur left a comment

Choose a reason for hiding this comment

florian-h05 commented Nov 20, 2022

openhab-bot commented Nov 20, 2022

openhab-bot commented Jan 1, 2023

[jsscripting] Reimplement timer creation method of `ScriptExecution` #13695

[jsscripting] Reimplement timer creation method of `ScriptExecution` #13695

florian-h05 commented Nov 11, 2022 •

edited

Loading

florian-h05 commented Nov 12, 2022 •

edited

Loading

florian-h05 commented Nov 20, 2022 •

edited

Loading