Since this change to the library is likely to be backported, it should be listed as a breaking change in the announcement of OH 5.2.1 I think.
Ok - but the “actual break” was with 5.2.0. But, I guess the most important factor is that the users are notified somewhere.
No - but they should enable passing complex data structures into the shared cache.
I don’t think there is much to test, we pretty exactly know what to put in the shared cache and what not … all JS objects shouldn’t go there, Java objects are “fine”. Converting everything to Java types through javaify() should be fine.
As Blockly dictates how passed in data is accessed, it is enough to adjust the code generation to automatically javaify and jsify.
That’s a difficulty …
I think shared cache should require explicit jsify and javaify.
It can be done comparable to the event object conversion (see openhab-addons/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java at 0cc33a0f62d3ce8d8e47c1e79142f690e7b49af8 · openhab/openhab-addons · GitHub), but it can break user scripts as it would change ctx.
Maybe @rlkoshak can assess how much ctx is in use and for what … depending on that we can go for automatic jsify of ctx.
Just had a look, even without callinf jsify on the receiving side, you can access array entries like in JS and reference Map members using .key syntax like in JS. So Graal makes the Java types look like JS, however that illusion breaks when working with java.time.* types.
For most cases, we should be fine without jsify … that being said, we can still think about automatic jsify for the called rule.
My view is that:
For “normal” operation, both javaify() and jsify() should be a manual operation that the user can choose to use. Therefore, they should probably be dedicated Blockly blocks, to give Blockly users the same possibilities/options.
In the two situations where we know that sending JS objects is “fatal”, we should implicitly use javaify(). Remember, it doesn’t do anything to primitives or objects that already are Java objects - it only changes anything if JS objects are found. And, it doesn’t change the “source” - the argument sent is still untouched, in its original form. What it does is to make a copy that it modifies and passes on/returns.
The sending script is unlikely to have any use for the “javaified” version, and if it does, it can invoke it manually in advance. Running javaify() implicitly on an already “javaified” structure wouldn’t change it at all.
When returned, things are different. By doing implicit jsify(), you take away information that cannot be restored on the other side. In particular, this applies if you deal with a Java map that you wish to keep as a Java map, because you use it with API calls where it is desirable to retain the original map, untouched. The only way to allow the script access to this object, is to make jsify() explicit, which is why I’m advocating for doing just that.
The “core issue” is that JS objects are converted to Java maps when “javafying”, and we have no way to differentiate what was originally a Java map from what was originally a JS object. Because they end up being identical, we have no way to “correctly” convert them back to their original state, so “everything” will end up being jsified to a JS object. Which I suspect will be fine in most situations, but not aways.
That’s why I think the best solution is: Normally, explicit both ways. For running other rules and the shared cache, where JS objects will break, implicit javaify() is preferable, to avoid exceptions. To retain information, manual jsify() is the best solution also in the situations where javaify() is implicit.
Given that Java arrays and Java Maps can be used like their JS counterparts, users won’t notice the difference as long as they only have primitives inside the original JS arrays and objects that have become Java arrays and Maps.
It depends on how it changes ctx, and really implicitly calling javaify is already breaking the ctx since what goes in isn’t what comes out and requires changes on the callee side to deal with it.
That’s the whole reason why I’d prefer to find a way to call jsify implicitly too before it gets to the called script.
And indeed I was thinking of the event Object conversion. It could dump the ctx and call jsify on all the Java stuff it finds there. Even better is if there is a little book keeping added to only call jsify on those Objects which were converted to Java through javaify.
I can’t comment too much on how much ctx is used as opposed to just pulling the variable out of thin air (i.e. assuming the variable exists in the called rule). I do know that Blockly uses it almost exclusively so any Blockly rule that gets called from another is using ctx.
I would expect that those scripts that just use the variable will become broken because that Object will remain Java. But I think it would be an improvement overall to provide an actual source for the variables (i.e. ctx) rather than just expecting the variables to somehow magically be there.
I feel like I might be rambling, does this make sense?
What about stuff like forEach, map, accessing an element of an Array using [ ] notation, etc.? That’s often where it breaks down I think. At least it used to. Stuff may have changed since the last I had to deal with this.
What does jsify do to those Java Objects that were Java in the original?
Nothing unless they happen to be Maps. But, that is the problem, since JS objects → Java Map in javaify(), you can’t tell what were originally Java maps from what were originally JS objects when you want to convert back. So, jsify() will convert them to JS Objects, which might invalidate them for further API use - all depending on the details.
In the event object conversion, I think calling jsify() is fine, because the original is still retained in the raw property, isn’t it?
We could make sure to document this either way. It’s not going to be a common issue but it’s going to hit someone somewhere.
Yes, openhab-js always preserves the original in a “raw” property. The JS versions will be copies from the raw, not replacements.
In those cases I think calling jsify() implicitly should be completely fine, I thought it was about running the whole “raw” ctx object through jsify().
edit: For shared cache retrieval, there is no “raw” fallback, but perhaps an additional argument could be added to get() (and any other methods that performs a “get”). This would be a boolean parameter jsify, which defaulted to true. That way, it would automatically be jsified by default, but it would still be possible to disable it in cases where the jsification caused problems?
Blockly uses ctx for passed in information and ctx.ruleUID. That’s it AFAIK.
The ctx.ruleUID syntax works both with a Java Map and a JS object, so the type of ctx doesn’t matter here for the ruleUID.
For other stuff, I guess Blockly does only handle JS types, so it would benefit from automatic jsify() (or does it itself).
I don’t know how to implement that through book keeping, that would need to happen across all script engines and somehow identify an object. A (really dirty) alternative would be to extend the Java types we use in Javaify and have something like a JSScriptingArrayList. We could then detect if something was JS and converted to Java.
It accesses them from ctx and not from thin-air.
Array notation works fine, map and forEach also work fine. To be fair, those also exist in Java.
Even reduce works, which is nothing that ArrayList actually procides.
I think the point is that there’s no clean way to do this, and attempting it would only get us deeper into mess. I see endless problems trying to get that to work universally, and even if you somehow “tainted” the Java map you sent, there’s a lot of Map.of() and Map.copyOf() in the Java code that doesn’t guarantee that what you get back is actually the original instance.
I think that’s right but would need to survey all the get blocks to make sure. That makes sense though.
As far as I know, Rules DSL, jRuby, and Python just work with Java Objects natively. It’s only JS that does this conversion to JS. So the book keeping may only need to be handled by and for JS rules. That makes it a little simpler and you could do the book keeping inside the ctx itself.
So javaify would be called on everything with a record of what actually got converted from JS to Java. On the callee side if it’s DSL, jRuby, Python, et al they get the javaify’ed Java Objects. They can’t really do anything with a JS Object anyway most likely. However, if the callee is a JS, we use that bookkeeping stored in the ctx to jsify those Objects that were originally JS to begin with. This would be done in the same function or a function called from the function where the JS event Object is built and it populates a copy of the ctx into the event Object (event.ctx perhaps).
That’s my thought process anyway.
Blockly does but in the UI JS scripts the varialbes are just there. One does not need to go through ctx to get them. Just reference them by name.
That’s what I mean by “thin air”.
I think maybe my explanations aren’t getting through, so I made a PR to show what I mean. Seeing that the raw object wasn’t “raw” surprised me, so I just took at guess there, but my idea is to jsify() everything that isn’t inside raw automatically, and let raw remain just that.
Prelimiinary testing with openhab-js 5.20 results:
-
Passing the array of Item names seems to work just fine. I didn’t do extenstive testing but at least a forEach worked on the other end without a call to jsify.
-
Passing the array of JS Item Objects did not work though. I didn’t expect them to but wanted to see what would happen. Unfortunately the error is inscrutable. When I attempt to do anything with the passed in Item Object it says:
2026-07-16 15:32:07.556 [ERROR] [ion.MasterBedroomSensors_Temperature] - Error running rule new-sensor-status-proc
Error: Functions aren't allowed
Because I know what is going on, I know what that means. But I expect many users will be completely baffled but this error.
If this could be detected in the calling rule and the error expanded to soemthing like “passing Objects with functions to another rule are not allowed” along with a line number and the name of the parameter containing the Object with functions, that would be ideal. It could even be a warning at this point in the execution flow as no error will be generated in the callee rule unless the user attempts to use that variable.
If this cannot be detected until we are in the callee rule, we need to provide more information. The name of the ctx variable, the line number, some sort of context to tell the user where to look. Right now all I know is there is some error somewhere in the script but my first though would be “since when can I not define a function in a script?”
Especially when this error occurs for users on rules that have worked for a long time it’s going to take them quite a bit of looking to discovery where the error is comming from.
Note, I’ve no problem not supporting Objects with functions here. I can’t imagine many are doing this and if they are, they should be using a library anyway. It’s a reasonable restriction.
I just wrote that error message “as a suggestion”, and no better suggestion came up. The text itself can easily be changed, but: There’s no way to detect “objects with functions”, javaify() is recursive, so the function can be the very “top” argument, or it can be buried deep inside a structure, and can be an array element as well as an object member. To be able to “figure this out”, you’d need to look at the stack the moment the error is generated. I don’t know what is and isn’t possible in JS in that regard, but it certainly looks complicated to try to make a sensible error by inspecting the stack where it originated.
When it comes to line number etc., I don’t think that’s possible. The code is no longer in “source form” when it’s executed, it is compiled bytecode. There won’t be any references to where in the source file that piece of code came from, that I can think of.
Maybe @florian-h05 has some ideas about how to make the error “more accurate”, but here and now, I can’t really see what we can do except to change the text itself.
I’m not quite in the loop here, is JS Item objects something fundamentally different from Java Item objects? Why are there functions in them?
So we can have stuff like MyItem.quantityState or MyItem.sendCommandIfDifferent("ON") etc. So that we don’t have to worry about “is this a Java Object or a JS Object” inside our rules. So we can use Item Objects with third party node JS modules if it’s needed.
The raw Java Item Object is in there too in MyItem.rawItem if one ever really needs it. But otherwise, just about everything you deal with in a JS script is going to be a native JS Object. That’s why that method to create the event Object that we’ve been referencing above exists.
The JS Item seems like just a wrapper, that could easily be unwrapped in javaify() and then rewrapped in jsify(). I just didn’t know about it, so I didn’t handle it.
Are there other such “wrapper objects” for OH classes? If so, they all need to be handled in both methods.
That won’t solve the problem with the “imprecise error message” itself, but it might make it a lot less likely that users encounter it without knowingly trying to send functions.
Things and Actions at a minimum. The event Object of course. Those are the ones I can think of off the top of my head. Pretty much any class that includes a raw property of some sort is going to be a wrapper.
But I’m not certain passing a Java Item Object is a great idea anyway. It used to be the case that if you use an Item Object as a key in a map (this was a very long time ago) eventually the Object would get refreshed and it’s hash would change and you’d lose the entry. I would be concerned that passing an Item Object like that between contexts might become problematic. It might become disconnected from the event bus (e.g. state won’t update, can’t command or update any more) or something like that. It definitely would need to be tested.