[automation] Some classes are internal but exposed "accidentally"

[J-N-K]

I'm currently working on a JSR223 compatible Java scripting addon.

The core exposes adds a binding scriptExtension or events (and probably more that I didn't encounter yet) to the ScriptEngine. Unfortunately the classes for these bindings (ScriptExtensionManagerWrapper and ScriptBusEvent) are in internal bundles: org.openhab.core.automation.module.script.internal and org.openhab.core.automation.module.script.internal.defaultscope and are therefore not available in the classloader and the script loading fails (internal bundles are not exported).

Would it make sense to move these to public packages? My guess would be that this is not a breaking change since they can't be accessed at the moment anyway.

[kaikreuzer]

Objects of those classes are put into the script engine scope and are then normally accessible (and resolvable) - this works for all the existing scripting languages afaik.
Could you elaborate why the situation is differently in your case?

[J-N-K]

Sure. The issue is that the used implementation of Java scripting (https://github.com/eobermuhlner/java-scriptengine) relies on the compilation of the script. This is done via the compiler obtained with ToolProvider.getCompiler().

Scripts are classes implementing Runnable, where the actual code is placed in the run method. The objects from the script engine scope are injected into the script via fields of the class (e.g. public ItemRegistry ir;) before the script is executed.

When compiling the script, all classes used have to be present on the classpath and during instantiation of the class object, the classloader has to be able to resolve the classes, otherwise a CNFE is thrown.

[kaikreuzer]

Ok, thanks for the infos - looks like an interesting script engine.

I guess it should be ok to move the affected classes to an exported package. Would you like to create a PR for it?

[J-N-K]

I wonder if we should really expose the classes themselves or if we should add an interface. The reason is that some of the methods (e.g. the constructor of ScriptBusEvent or ScriptThingActions.addThingActions are package private (which makes sense since they should not be used in scripts or outside of core), they are used by the DefaultScriptScopeProvider (obviously in the same package). IMO the DefaultScriptScopeProvider should not be exported, which requires to change the methods mentionded before to public.

[lolodomo]

Is it a concurrent approach to https://github.com/seaside1/jrule ?
https://community.openhab.org/t/jrule-java-rules-in-openhab-3-alpha/121441

[J-N-K]

Originally I started with the code presented there. But unfortunately it has some limitations:

• UI defined rules not possible
• thing actions cannot be used in the way other JSR 223 languages do
• type-safety (one of the great advantages of Java over other languages) cannot be ensured because everything is a String
• triggers/conditions are not fully supported

Therefore I decided to retry my approach from a year ago or so with the library mentioned above and finally succeeded. Biggest issue was that I missed the "Dynamic-Import: *" instruction, which I found in the groovy JSR 223 addon.

So: it's a different approach.

[lolodomo]

Ok, thank you for the explanation. I am interested to test such rule engine.

[kaikreuzer]

I wonder if we should really expose the classes themselves or if we should add an interface.

I'd agree that this would be the cleaner approach as it decouples the implementation. So we could rename the existing classes to *Impl and keep them in the internal package and introduce according interfaces for them in a public package.

[J-N-K]

I'll check how that works and provide a PR.