Skip to content

Conversation

@dilyanpalauzov
Copy link

For .java files in the unnamed package (no package …; line) public for methods and constructors does not need to be specified anymore. Ctors and methods are called, even if they are explicitly private.

For .java files with package …; line still only public constructors and methods (main, eval, scriptLoaded, scriptUnloaded) are considered.

The error message

2025-10-15 18:46:25.568 [ERROR] [ipt.internal.ScriptEngineManagerImpl] - Error during evaluation of script '/etc/openhab/automation/jsr223/m.java': Index 0 out of bounds for length 0

means that no constructor can be accessed outside the package. (package x; and then all ctors are private or protected; in Java223CompiledScript compiledClass.getConstructors() returned zero constructors).

This could be improved:

[ERROR] [ipt.internal.ScriptEngineManagerImpl] - Error during evaluation of script '/etc/openhab/automation/jsr223/m.java': cannot execute: M doesn't have a method named eval/main/run, or a RunScript annotated method

to say conditionally if it does not have a main method or a public main method.

@dalgwen
Copy link
Owner

dalgwen commented Oct 16, 2025

For .java files in the unnamed package (no package …; line) public for methods and constructors does not need to be specified anymore. Ctors and methods are called, even if they are explicitly private.

Do you refer to JEP495 ? If so:
It's for java24, and we still use java21. What is the behaviour of our current code (without this PR) with java24 ? Will calling such "non-specified/default accessor" method/constructor work out of the box ? We may not need those modifications when openHAB switch to java24+

@dalgwen
Copy link
Owner

dalgwen commented Oct 16, 2025

2025-10-15 18:46:25.568 [ERROR] [ipt.internal.ScriptEngineManagerImpl] - Error during evaluation of script '/etc/openhab/automation/jsr223/m.java': Index 0 out of bounds for length 0

How did you get this error message ?
With or without this PR content ?

@dilyanpalauzov
Copy link
Author

This changeset makes the changes done herein in README.md possible.

@dalgwen
Copy link
Owner

dalgwen commented Oct 16, 2025

OK, so no link with JEP495.

Then, unless we are talking about java 24+, it kinda seems against current Java version philosophy.
And why did you allow this for the unnamed package only ?

And whatever your answer is, I think it introduces at least one issue:
If there is no package declaration, then your modification uses getDeclaredMethod(s) instead of getMethod(s).
But getDeclaredMethod(s) doesn't return inherited method, contrary to getMethod(s).
So if we want to use getDeclaredMethod(s), we have to enhance it by also looking in super class.

For reference, I had the same kind of issue with field injection. I had to design a recursive method getFieldDeep in BindingInjector, because I also wanted to use getDeclaredField instead of getField in order to get private/protected member.

@dilyanpalauzov
Copy link
Author

The purpose and inspiration of this change is to free users from the obligation to write public before each method/constructor, they want to expose to Java223. In favour of against Java philosophy it makes things simpler, with no practical disadvantages. If you think of Java223 running in the unnamed package, and the jsr223/.java files are also in the unnamed package, then the packages are the same and no public before methods and constructors will be necessary.

I changed the error “Index 0 out of bounds for length 0”, which appeared only after this change was applied. (Without this change, the error was present, but completely differently spelled). Now for automation/jsr223/n.java

package Z;
public class N {
  private N() {};
  public Object main() {
    return null;
  }
}

the logged error is:

[INFO ] [ort.loader.AbstractScriptFileWatcher] - (Re-)Loading script '/etc/openhab/automation/jsr223/n.java'
[ERROR] [va223.internal.Java223CompiledScript] - Exception during evaluation of a java223 script: No public constructor in Z.N
org.openhab.automation.java223.common.Java223Exception: No public constructor in Z.N
        at org.openhab.automation.java223.internal.Java223CompiledScript.construct(Java223CompiledScript.java:147) ~[?:?]
        at org.openhab.automation.java223.internal.Java223CompiledScript.eval(Java223CompiledScript.java:102) ~[?:?]
        at javax.script.CompiledScript.eval(CompiledScript.java:93) ~[java.scripting:?]
        at ch.obermuhlner.scriptengine.java.JavaScriptEngine.eval(JavaScriptEngine.java:179) ~[?:?]
        at ch.obermuhlner.scriptengine.java.JavaScriptEngine.eval(JavaScriptEngine.java:167) ~[?:?]
        at ch.obermuhlner.scriptengine.java.JavaScriptEngine.eval(JavaScriptEngine.java:157) ~[?:?]
        at ch.obermuhlner.scriptengine.java.JavaScriptEngine.eval(JavaScriptEngine.java:152) ~[?:?]
        at org.openhab.core.automation.module.script.internal.ScriptEngineManagerImpl.loadScript(ScriptEngineManagerImpl.java:165) ~[?:?]
        at org.openhab.core.automation.module.script.rulesupport.loader.AbstractScriptFileWatcher.createAndLoad(AbstractScriptFileWatcher.java:335) ~[?:?]
        at org.openhab.core.automation.module.script.rulesupport.loader.AbstractScriptFileWatcher.lambda$13(AbstractScriptFileWatcher.java:309) ~[?:?]
        at java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1804) ~[?:?]
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:572) ~[?:?]
        at java.util.concurrent.FutureTask.run(FutureTask.java:317) ~[?:?]
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) ~[?:?]
        at java.lang.Thread.run(Thread.java:1583) [?:?]
[ERROR] [ipt.internal.ScriptEngineManagerImpl] - Error during evaluation of script '/etc/openhab/automation/jsr223/n.java': No public constructor in Z.N
[WARN ] [ort.loader.AbstractScriptFileWatcher] - Script loading error, ignoring file '/etc/openhab/automation/jsr223/n.java'

Notice above, how “No public constructor in Z.N” appears three times.

Whether this should be altered only for the unnamed package or always, is a matter of opinion, and can be changed. I think in most cases nobody will create packages, so it does not really matter. And those who want to create packages, must be explicit whether to be public or not.

Having “Simple Source Files and Instance Main Methods (JEP459)”, or “Compact Source Files and Instance Main Methods (JEP 512)” (writing methods without class wrapper for simple programs) would be nice for Java223. JEP459 is however only for main method and this change is for many methods and constructors. This change is however similar to the idea of JEP459.

openHAB 5 requires Java 21, I use Java 21, and if there is something Java 24 related, then it is pure coincidence. The most essential thing done here are invocations of .trySetAccessible(); and I do not thing that with Java 24 .trySetAccessible(); will be significantly different

… I think it introduces at least one issue:
If there is no package declaration, then your modification uses getDeclaredMethod(s) instead of getMethod(s).
But getDeclaredMethod(s) doesn't return inherited method, contrary to getMethod(s).
So if we want to use getDeclaredMethod(s), we have to enhance it by also looking in super class.

getDeclaredMethod() has an advantage over getMethod(), which is that the returned methods do not have to have explicitly public.

In Java223ScriptEngine.invokeFunction() (only used for scriptLoaded and scriptUnloaded) I added a fallback to getMethod() if getDeclaredMethod() does not return anything.

This change is however not done for finding the runnable function (main, eval) in Java223Strategy.execute(Object, Map<String, Object>). The problem is that extra magic there will make Java223Strategy.execute(Object, Map) slower. The other problem is that the runnable method is discovered on every call of execute() rather than when the .java file is compiled. E.g. when this sitemap is loaded:

sitemap a label="A" {
  Text item=x label="UU [JAVA(|return \"ABC\";):%s]"
}

and the value of x is changed many times, then Java223ScriptEngine.compile() is executed once, but many times Java223Strategy.execute(Object, Map) searches for the runnable method, even if all the times the result is the same.

Another simplifications, which can be made for files loaded from automation/jsr223/.java, there should be no error if no runnable methods (eval, main) are contained. The .java file can do all its program logic in the constructor, or in scriptLoaded() and if these methods are implemented, there is no need for a runnable method. openHAB does only eval(String) files loaded from automation/jsr223, as these are run once, so there is no need to optimize the process by compiling these. But for transformations, UI rule bodies (I guess) the code is first compile()d and then eval()uated. That said, if openHAB does not call .compile() then Java223 should not log “Error during evaluation of script '/etc/openhab/automation/jsr223/n.java': cannot execute: N doesn't have a method named eval/main/run, or a RunScript annotated method”.

Shall I leave here for now Java223Strategy.execute(Object, Map<String, Object>) unchanged, so that the rest can make progess and then public will be redundant only for constructors and scriptLoaded/scriptUnloaded?

@dilyanpalauzov dilyanpalauzov force-pushed the j233_mtds_deflt_visibility branch 2 times, most recently from 32cf5d6 to cbbd11a Compare October 16, 2025 19:28
@dilyanpalauzov dilyanpalauzov force-pushed the j233_mtds_deflt_visibility branch from cbbd11a to de41a12 Compare October 16, 2025 19:28
@dilyanpalauzov
Copy link
Author

The current change breaks the "@runscript" and "@rule" annotations for unnamed packages. As the changed implementation does not use getMethod() it skips Java223Script.internalParseRules and then @Rule annotations are not handled.

@dilyanpalauzov dilyanpalauzov marked this pull request as draft October 17, 2025 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants