[jrubyscripting] Wrong content type

[ghys]

The content type of the JRuby script engine seems to be wrong:

Expected Behavior

There isn't a IANA-assigned media type (formerly MIME type) but in the UI I originally used application/x-ruby to activate the Ruby mode of the editor, there is a conversion method here: https://github.com/openhab/openhab-webui/blob/fb426697e95cb06ef4f94f05fee6750303aee90e/bundles/org.openhab.ui/web/src/components/config/controls/script-editor.vue#L169-L178 Note that I added the current rb as well but it would probably be better to use a proper media type.

Current Behavior

The current content type rb doesn't follow the standard type/subtype format.

Possible Solution

Change it to application/x-ruby. Jython uses application/python (probably should have been application/x-python as you're not supposed to invent new types without x-, they have to be registered at https://www.iana.org/assignments/media-types/media-types.xhtml). Groovy uses application/x-groovy.

Steps to Reproduce (for Bugs)

1. Install the JRuby add-on
2. Create a script or script action in a rule

Your Environment

OH 3.2 RC1

[kaikreuzer]

Shall we consider this critical and fix it @ghys?

[ghys]

I have no decision power over here, but if it's quick and easy enough, I'd advise to do it, yes - if we decide to change it later it would mean users would need to reconfigure their existing rules.

[kaikreuzer]

Do you know exactly what would need to be done?

[digitaldan]

The Handler Factory needs to return the mime type(s) it supports through an interface if i remember correctly, should not be a tough fix

[ghys]

Based on what's done for JSScripting here:

https://github.com/openhab/openhab-addons/blob/22b9be1390a8e46c6bbbeed67a807d813e8e645e/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java#L44-L64

I'm assuming something similar has to be done here:

https://github.com/openhab/openhab-addons/blob/22b9be1390a8e46c6bbbeed67a807d813e8e645e/bundles/org.openhab.automation.jrubyscripting/src/main/java/org/openhab/automation/jrubyscripting/internal/JRubyScriptEngineFactory.java#L89-L91

[digitaldan]

Correct , we would want to either replace, or add to this line

https://github.com/openhab/openhab-addons/blob/fa92077d95c3aae64c48931bce082bb7a6bd6201/bundles/org.openhab.automation.jrubyscripting/src/main/java/org/openhab/automation/jrubyscripting/internal/JRubyScriptEngineFactory.java#L54

an additional mime type

[kaikreuzer]

So it is actually the JRuby factory itself that delivers the wrong mime-types? Well, if they do not make sense for us, we can override them - I just hope it won't have any other side-effects?

[ghys]

Maybe what's because of the order here? https://github.com/openhab/openhab-addons/blob/22b9be1390a8e46c6bbbeed67a807d813e8e645e/bundles/org.openhab.automation.jrubyscripting/src/main/java/org/openhab/automation/jrubyscripting/internal/JRubyScriptEngineFactory.java#L54-L56 i.e. the first script type would be what ends up being the content type advertised, so

.concat(factory.getMimeTypes().stream(), factory.getExtensions().stream())

(just speculating)

[ghys]

The JRuby engine factory seems to use application/x-ruby as its MIME type - it seems the planets aligned on this one ;)

https://github.com/jruby/jruby/blob/a309a88614916621de4cc5dc3693f279dae58d0c/core/src/main/java/org/jruby/embed/jsr223/JRubyEngineFactory.java#L68

[digitaldan]

.e. the first script type would be what ends up being the content type advertised, so

Yep that sounds right maybe @boc-tothefuture is monitoring and can chime in ?

[ghys]

Seems the order has no importance in fact:

https://github.com/openhab/openhab-core/blob/79edf2b9e643ad802c31b2e5f54edb93534277fd/bundles/org.openhab.core.model.script.runtime/src/org/openhab/core/model/script/runtime/internal/engine/DSLScriptEngineFactory.java#L50-L53

https://github.com/openhab/openhab-core/blob/79edf2b9e643ad802c31b2e5f54edb93534277fd/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/NashornScriptEngineFactory.java#L53-L54

[ghys]

I think I nailed it down to:

https://github.com/openhab/openhab-core/blob/eb3f1e92cc02869af0d96ad3a67f0471cc380355/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/provider/ScriptModuleTypeProvider.java#L177-L181

    private String getPreferredMimeType(ScriptEngineFactory factory) {
        List<String> mimeTypes = new ArrayList<>(factory.getScriptTypes());
        mimeTypes.removeIf(mimeType -> !mimeType.contains("application") || mimeType.contains("x-"));
        return mimeTypes.isEmpty() ? factory.getScriptTypes().get(0) : mimeTypes.get(0);
    }

Because application/x-ruby contains x- it is removed from the list of preferred mime types and therefore only the extension rb remains. Actually contrary to what I said in the OP Groovy has the problem too:

[kaikreuzer]

Looks as if this line is in already since April 2019... It isn't really clear to me why those entries are excluded, though.

[ghys]

Maybe the Nashorn script engine factory exposes several MIME types including application/javascript and another with x- and this was the way to choose the right one.

[ghys]

https://docs.oracle.com/en/java/javase/11/docs/api/jdk.scripting.nashorn/jdk/nashorn/api/scripting/NashornScriptEngineFactory.html

JSR-223 compliant script engine factory for Nashorn. The engine answers for: names "nashorn", "Nashorn", "js", "JS", "JavaScript", "javascript", "ECMAScript", and "ecmascript"; MIME types "application/javascript", "application/ecmascript", "text/javascript", and "text/ecmascript"; as well as for the extension "js".

So I can't find a reason either why those entries with x- are removed.

[ghys]

Ok it's because of Jython...

https://github.com/jython/jython/blob/0a58cc26566d2b2334e80b2b3f2f42f6c738db2d/src/org/python/jsr223/PyScriptEngineFactory.java#L88-L91

    public List<String> getMimeTypes() {
        return Collections.unmodifiableList(Arrays.asList(
                "text/python", "application/python", "text/x-python", "application/x-python"));
    }

[kaikreuzer]

Right, that would also explain why it was added by Scott... So maybe we should just exclude "text/x-python" and "application/x-python" in that line then?

[ghys]

As I said above I believe application/x-python is actually more correct ;)

Jython uses application/python (probably should have been application/x-python as you're not supposed to invent new types without x-, they have to be registered at https://www.iana.org/assignments/media-types/media-types.xhtml).

(to be perfectly complete, even using x- to avoid the registration process has been deprecated since 2012 with RFC 6648, see also RFC 6838 §3.4)

Maybe existing modules configured with application/python would not have to be changed either, because the Jython engine would still answer for that script type.

So I would exclude application/python (the UI would need to be changed as well) but other opinions are welcome!

[kaikreuzer]

Fine for me. Shall we try to squeeze this in tonight? It's probably helpful as fixing it later on will be a breaking change for existing scripts...

[ghys]

I created https://github.com/openhab/openhab-core/pull/2629 and https://github.com/openhab/openhab-webui/pull/1248 - completely untested though!

[boc-tothefuture]

Reading through here, thanks for the mention @digitaldan - looks like there is an issue in how the core is processing the returned mime-types and not something JRuby binding is doing, is that correct? Happy to adjust the binding if we need to.

[kaikreuzer]

Thanks @ghys - I have merged both and launched a new build. Let's test the result together briefly!

[kaikreuzer]

Time for testing: https://ci.openhab.org/job/openHAB3-Distribution/2648/

[kaikreuzer]

This looks quite good now:

[ghys]

Great! I'll assume the editor does the syntax highlighting for all these languages as well.

[kaikreuzer]

Yes, syntax highlighting seems to work as well. 👍

[boc-tothefuture]

Can this be closed?