search

rkoshak / openhab-rules-tools

Library functions, classes, and examples to reuse in the development of new Rules.
Other
64 stars 23 forks source link

[helpers.js] Small improvement suggestion: check if items with metadata exist #109

Closed Huib0513 closed 1 year ago

Huib0513 commented 1 year ago

In 'helpers.js' the function 'checkGrpAndMetadata' does not verify if any items with metadata in the specified namespace actually exist.

rkoshak commented 1 year ago

I purposely do not test for that in checkGrpAndMetadata because I reuse that function across multiple rule templates and the lack of metadata isn't an error in all those rule templates (e.g. Threshold Alert). So testing that the metadata exists where it's required is implemented in the Rule Template itself.

In the Time Based State Machine, case, that's done in the validateItemConfig function which should throw an error if the Item doesn't have the defined namespace. 

var validateItemConfig = (itemName) => {
  const md = items[itemName].getMetadata()[NAMESPACE];

  if(!md) {
    throw itemName + ' does not have ' + NAMESPACE + ' metadata!';
  }

It might be that the !md isn't evaluating to true when the metadata doesn't exist, but then I'd expect it to throw a malformed exception on the next test, since non-existing metadata shouldn't have a value either.

  if(!md.value) {
    throw itemName + ' has malformed ' + NAMESPACE + ' metadata, no value found!';
  }

I just set the metadata namespace property on my instance of the rule template to a non-existing and I did in fact get an exception as expected:

2023-10-16 13:50:37.711 [INFO ] [omation.rules_tools.TimeStateMachine] - All etod-dne Items are configured correctly
2023-10-16 13:50:37.864 [WARN ] [ore.internal.scheduler.SchedulerImpl] - Scheduled job 'ui.time-state-machine.debounce' failed and stopped
org.graalvm.polyglot.PolyglotException: TypeError: Cannot read property "configuration" from undefined
        at <js>.:=>(<eval>:84) ~[?:?]
        at <js>.getItemsOfType(<eval>:84) ~[?:?]
        at <js>.validateAllConfigs(<eval>:121) ~[?:?]
        at <js>.createTimersGenerator(<eval>:202) ~[?:?]
        at <js>.:=>(/openhab/conf/automation/js/node_modules/openhab_rules_tools/timerMgr.js:66) ~[?:?]
        at com.oracle.truffle.polyglot.PolyglotFunctionProxyHandler.invoke(PolyglotFunctionProxyHandler.java:154) ~[bundleFile:?]
        at jdk.proxy1.$Proxy726.run(Unknown Source) ~[?:?]
        at org.openhab.automation.jsscripting.internal.threading.ThreadsafeTimers.lambda$0(ThreadsafeTimers.java:85) ~[bundleFile:?]
        at org.openhab.core.internal.scheduler.SchedulerImpl.lambda$12(SchedulerImpl.java:189) ~[?:?]
        at org.openhab.core.internal.scheduler.SchedulerImpl.lambda$1(SchedulerImpl.java:88) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
        at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
        at java.lang.Thread.run(Thread.java:833) [?:?]

Clearly I need to fix that function because it should have thrown a different exception but that doesn't explain why it didn't for you. Are you certain you are using the latest version? At the top of the Script Action there is a comment. It should read

// Version 0.4

If it's not there or it reads something else, you have a old version of the template and need to remove and re-add it and reinstantiate the rule (sadly that's the only way to upgrade a rule for now).

Stay tuned for version 0.5 which will fix that function so it correctly detects the missing item metadata.

rkoshak commented 1 year ago

A new version of the rule template has been posted to the marketplace which tests that all members of the Group have metadata and tells you which ones don't. I'm going to close this as completed with that change.

Huib0513 commented 1 year ago

Thanks for the clear explanation! I did not consider reuse of the function. I did see the exception from the scheduled job, but it took me quite a while to find the typo that caused it. The update of the rule template seems like the best solution, thank you for the trouble!