Add an access-tracking cache to be used in rules

[J-N-K]

trafficstars

Closes #2084 Closes #2881 Closes #2943 Depends on #2886

This implements option 1 that was discussed in #2084 (which seems to be what most comments prefer). It is remotely based on the code here: https://github.com/openhab/openhab-addons/blob/4f4dfcca20f39c2d8ed059b67e1dba09c9c83957/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/SharedCache.java.

Two separate key-value-caches are implemented:

1. privateCache: It is created per scriptIdentifier and can only be accessed from the same script. It is not cleared between two subsequent runs of a rule but removed when the script is unloaded. This can be used e.g. to store timers.

2. sharedCache: It is shared between all scripts. Every access to a certain key in the cache is tracked. The key is removed from the map if all scripts that ever accessed (put or get) that key have been unloaded.

Since script unloading is tracked by the framework nothing special needs to be done in the script itself. That makes it usable also in UI rules, where scriptUnloaded is not available.

~~I am currently investigating if I can auto-cancel ScheduledFutures and Timer when a key is removed (i.e. the last script that used the key was unloaded).~~ ScheduledFuture<?> and Timer objects are cancelled when the script is unloaded in a private cache or if the last accessor was removed in the shared cache. They are not cancelled on remove because I might be that the script wants to use it or take special actions before cancellation.

@jpg0: do you think the preset name cache should be changed, so that we do not run into conflicts? I changed the case of the sharedcache to be more consistent with the camel-case we use in general. As an alternative, I could implement sharedcache as an alias to sharedCache and log a warning if that is used instead of the new name. The old name could then be removed in a later version.

Signed-off-by: Jan N. Klug [email protected]

[jpg0]

@J-N-K tbh I have not been very careful with naming in the past because:

• I personally always explicitly import everything as I've had huge issues in the past with these implicit things clashing
• There has not been much care taken here so far in naming - for example event is surely one of the most used variable names possible!
• Any add-on can just insert it's own new preset with whatever name it chooses at whatever time it chooses and cause clashes

As you are probably aware I have been against auto-importing these symbols due to these issues - however as both (auto-import and common names) things have already happened, my view now is that to remediate the problem we now should try to be clear to the user when a clash actually occurs (most likely by hooking into the JS engine) if it becomes a problem. (This hasn't been done.)

As for the immediate concern, I appreciate the concern of clashing with existing scripts but also fear needing to come up with more and more obscure preset names as we introduce additional ones. Not sure where we should draw the line though.

[J-N-K]

@openhab/core-maintainers As this is a much requested feature it would be great if someone could find the time to review. About half of the changed lines are tests.

[openhab-bot]

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

https://community.openhab.org/t/motion-detected-light-on-turn-off-after-3-minutes-but-only-between-8pm-5am/141212/31

[jimtng]

Could this make it into 3.4?

[florian-h05]

This is also on our "wishlist" at JS Scripting, as well as it is closing 3 issues at once.

@openhab/core-maintainers Would be great if it could make it into 3.4.0.

[rkoshak]

I'll chime in. From the end user's perspective, this will be very important to universally support many use cases that are only possible through sharing variables between script actions and conditions or preserving variables between runs of a single script action/condition.

I've been anticipating this PR's merge for some time. It would be awesome if we could get it into 3.4 release.

[jimtng]

Who do we need to ping? Perhaps this is overlooked?

[florian-h05]

I already pinged the core maintainers, if we want to, we can also explicitely ping kaikreuzer, cweitkamp or wborn.

[wborn]

Yes it will be a nice addition and it's not that much code too. :slightly_smiling_face:

sharedCache: It is shared between all scripts.

I think it is truly shared between all scripts so you can also use it between different scripting engines, right? Might be handy if you decide to migrate from one engine to another one day.

[J-N-K]

It seems that there is an issue with using this new feature in JSScripting as well as DSL rules. @florian-h05, do you know how to import additional presets? According to the JSR223 documentation this should work via scriptExtension.importPreset but scriptExtension (or its alias se) seems to be unavailable in JSScripting. In Groovy the following works:

import org.openhab.core.automation.*
import org.openhab.core.automation.module.script.rulesupport.shared.simple.*
import org.openhab.core.config.core.Configuration

scriptExtension.importPreset("RuleSupport")
scriptExtension.importPreset("RuleSimple")
scriptExtension.importPreset("cache")

def sRule = new SimpleRule() {
    Object execute(Action module, Map<String, ?> inputs) {
	    sharedCache.put("x", "y")
    }
}
sRule.setTriggers([
    TriggerBuilder.create()
        .withId("aTimerTrigger")
        .withTypeUID("timer.GenericCronTrigger")
        .withConfiguration(new Configuration([cronExpression: "0 * * * * ?"]))
        .build()
    ])

automationManager.addRule(sRule)

[jpg0]

@J-N-K you can reference it like this:

const { cache } = require('@runtime/Defaults');

(assuming that it's in the default set of presets)

[florian-h05]

@J-N-K AFAIK, you are right that scriptExtension is not available, but reading throught https://github.com/openhab/openhab-addons/blob/622654ff1d020454c60980ccfc500905f7f2c068/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/ScriptExtensionModuleProvider.java, I think it should be possible to import the cache using require('@runtime/cache') (as it is not in the default preset, but in the cache presets).

const { sharedCache, privateCache } = require('@runtime/cache');

[J-N-K]

Thanks, I can confirm that

var { sharedCache } = require("@runtime/cache")

console.log(sharedCache.get("x"))

successfully logs y which is the value set in the Groovy script. So it works across languages, just DSL needs to be figured out. I'm currently not sure how to implement the privateCache, the sharedCache seems to be possible.

[J-N-K]

I also found a way to make the cache available in DSL. It feels a bit like an ugly hack, so I would prefer to do that in another PR, as it‘ll most likely be a longer review process.

[florian-h05]

So this is finished and ready for review now?

[J-N-K]

Yes, it‘s also rebased and confirmed working with latest changes to automation.

[wborn]

Is the plan now also to remove the existing SharedCache from the jsscripting add-on to prevent "user cache confusion"?

[florian-h05]

Is the plan now also to remove the existing SharedCache from the jsscripting add-on to prevent "user cache confusion"?

Yes, but the coexistence should not lead to real user confusion, as the SharedCache is not documented directly and only exposed through the helper library. I just need to figure out how to handle both the shared and the private cache in the JavaScript library whilst not breaking existing installations. I‘ll think about this and probably ping a few people to get their opinion.

[florian-h05]

@J-N-K Does the cancellation of timers work for you in JS Scripting?

I am using the following script, and I can edit it and save it, openHAB reloads the script, but the timer runs (you need to have a look at the log timestamps, there should run no timer 15 seconds after the first reload of the script):

const { privateCache } = require('@runtime/cache');
const { actions, time } = require('openhab');

console.log(privateCache.get('timer'));

const timer = actions.ScriptExecution.createTimer(time.toZDT().plusSeconds(15), () => {
  console.warn('Timer ran.');
})

privateCache.put('timer', timer);

Log:

19:35:58.228 [INFO ] [port.loader.AbstractScriptFileWatcher] - Loading script '/etc/openhab/automation/js/test.js'
19:35:59.221 [INFO ] [penhab.automation.script.file.test.js] - null
19:36:06.185 [INFO ] [port.loader.AbstractScriptFileWatcher] - Loading script '/etc/openhab/automation/js/test.js'
19:36:07.172 [INFO ] [penhab.automation.script.file.test.js] - null
19:36:14.223 [WARN ] [penhab.automation.script.file.test.js] - Timer ran.
19:36:22.172 [WARN ] [penhab.automation.script.file.test.js] - Timer ran.

[J-N-K]

Can't work, because the import is wrong.