flexible include mechanism

[plata]

Remove the fixed 4 level namespace and replace it by a flexible slug include mechanism.

How it could work: The ìd in the script.json becomes a slug, e.g. dotnet40 becomes engines/wine/verbs/dotnet40. The include changes from include(["Engines", "Wine", "Verbs", "dotnet40"]) to include("engines/wine/verbs/dotnet40"). A method getAll could return a list of ScriptDTO for a given slug, e.g. getAll("applications/games") would return applications/games/aoeII/steam, applications/games/uplay/online etc.

Pros:

• Removes the fixed 4 levels (e.g. strange paths like Engines/Wine/Engine/Object).
• Removes the artificial assignment of scripts to ApplicationDTO and ScriptDTO (why is "Verbs" an application?).
• Could allow to run a script via "Run a script" button (see PhoenicisOrg/POL-POM-5 #1179).

Cons:

• For application, the valid hierarchy of category/application/script is encoded in the slug instead of a class hierarchy. We could solve this by keeping the 4 level namespace for applications only but it's probably not a good idea to differentiate between scripts in the repository.

[madoar]

How/where would we use the new getAll method? I think this suggestion is a good idea.

[plata]

It would be used e.g. in the applications tab to show the applications for the different categories.

[madoar]

To load scripts with a slug input via include, we could use the following implementation:

    @Override
    public void injectInto(NashornEngine nashornEngine) {
        // a map referencing the slugs to their corresponding script file
        final Map<String, URI> knownScripts = loadAllScripts();

        nashornEngine.put("include", (Consumer<ScriptObjectMirror>) args -> {
            final String slug = args.to(String.class);

            URI scriptUri = knownScripts.get(slug);

            // I guess we would need to add the next method to scriptFetcher
            final String script = scriptFetcher.getScript(scriptUri);
            if (script == null) {
                throwException(new ScriptException(slug + " is not found"));
            } else {
                nashornEngine.eval("//# sourceURL=" + slug + "\n" + script,
                        this::throwException);
            }
        }, this::throwException);
    }

For more details see #519.

[plata]

How about using reverse-DNS like so: vendor.type.name

For example, this would give the following IDs/slugs:

• org.phoenicis.apps.games.aoe2.steam
• org.phoenicis.engines.wine.verbs.vcrun2010

but also possible:

• com.plata.apps.games.csgo.steam

include would be:

include("org.phoenicis.engines.wine.verbs.vcrun2010")

The apps tab would show everything in *.*.apps.**, similarly for verbs etc.

Internally, the whole repository hierarchy would be replaced by a map like outlined by @madoar above.

@qparis @madoar what's your opinion about this?

[qparis]

I see one drawback: we cannot identify that your Csgo script corresponds to mine

[plata]

You mean if you would structure your repository differently, like fr.qparis.apps.games.shooters.csgo.steam? That's correct but actually it's not a new problem. For example, with the current implementation I could use "Counter Strike:GO" and you could use "CS GO" and you have the same issue.

[qparis]

Yes of course if we do not agree on the name we always have the issue. However here it is a bit different. Even if our names perfectly match, we won’t be able to tell that it is the same game. Imagine when we have a working website that presents the different versions of the scripts.

Do we really need name spacing?

[plata]

Even if our names perfectly match, we won’t be able to tell that it is the same game.

Why? We need to merge repositories like we do now. So the merge would always ignore the vendor. So fr.qparis.apps.games.csgo.steam → apps.games.csgo.steam, same for com.plata.apps.games.csgo.steam. I don't really see a difference to the current implementation.

Do we really need name spacing?

We have this already. Just that it's "applications/games/csgo" right now. With all the strange consequences like "wine/verbs" is an application and "wine/verbs/vcrun2010" is a script. In general, reverse-DNS is a commonly used scheme which is familiar for many people and - more importantly - has proven that it works. So why come up with something custom?

[qparis]

If we merge the vendor, it would work.

However, I’m not sure that it is a priority to change this mechanism. I agree that it would be cleaner, but I’m not sure it would bring a really huge improvement

[madoar]

I'm fine with adding a vendor. The question is if we need to do it explicitly? I mean doesn't the repository itself kind of represent the vendor? Do we need to give it an explicit name?

[plata]

The main question is if we want to change our include system to work with IDs instead of the current repository hierarchy.

In my opinion, we should start stabilizing our APIs rather sooner than later.

[qparis]

It's ok if the API is not stable. It will always evolve. When we implement this feature, we can still make the old include mechanism backward compatible so no hurry :)

We should focus on what adds value to our end user

[qparis]

Sorry for the close, bad click

[plata]

We should focus on what adds value to our end user

I do not agree to this. Yes, the API will always evolve but central parts like the include mechanism or the way we call scripts from Java should settle. I really wouldn't want to be backwards compatible for this stuff and it will be a pain to change the more scripts we have.

[qparis]

I'm not sure it will be a pain: we can decide that include(["a", "b", "c"]) means include("org.phoenicis.a.b.c") right?

[plata]

In this case, yes. I was referring more to the general issue of stabilizing the API.

In any case, I think we can decide if we want to change the include system or not regardless if it will be done now or in one year.

[qparis]

I think we should keep this in mind and do it when we find a reason to do it (like fix a bug, add a new feature easily, etc...)

[plata]

Would you consider "make it easier for people who want to provide scripts to understand includes" a valid reason? Currently understanding includes seems to be difficult (there's an issue regarding this).

[qparis]

It is a perfectly valid reason, and if you want to do it, feel free to :) (After all, this is the spirit of opensource project). However, I'm not 100% sure that the cost-benefices ratio is huge

[madoar]

My personal problem with our current include scheme is the part where you need to find out the correct path leading to the verb/utility you want to use. Instead of following the folder path you need to look in every json file on the way which is not really intuitive. It would be nice if we could improve this.

[qparis]

Maybe we could also have a loader that loads all verb at once

[madoar]

This would just reduce the number of necessary includes but not the problem of finding the right include path, right? In addition we do not only need to include the verbs but also engine settings (e.g. windowsVersion), utility functions (e.g. filesystem) and the quick scripts.

[qparis]

It would just make things easier globally (one include for all wine engine), but it does not solve the problem you are mentioning. However, it can be a pretty good deal

[madoar]

That is true. It should also be quite easy to implement. We just need to add a new script containing the concrete verb includes.

[plata]

I will try to do something about this issue. I hope that I can find some time during the next days...