Marketplace versioning with embedded resource

It’s only JSON at the moment, but it should still be somewhat useful for testing as you can just run it through an online converter. But, I do understand that this is an undesirable step. I’m trying to get some YAML support working at this moment, but the support for multiple objects in an array is currently pushing my completely lacking YAML knowledge over the edge. Is it correct that an array in YAML should be basically just like in JSON, making a pair of outer [] and then just comma separate the elements?

Anyway, the array with multiple rule templates already seems to work fine in JSON.

I’ll look some more then, ysc:washing_machine_alert seems to be JS at least.

Which seems to be already implemented :confused:

It can’t be that you miss these things, GitHub will automatically alert me of all changes to any issues or PR’s that I’m somehow involved with - and I think that’s with the default alert settings. Could it be that things are implemented and integrated, but that the issues are never closed or updated accordingly? If so, that is somewhat problematic I would say.

YAML does lists I know but the list has to be under something. I don’t think YAML supports starting off with just a list without a starting element. But my knowledge of YAML primarily comes from Ansible, so maybe that’s not a universal issue but just how Ansible works.

There was some work on adding YAML support for loading anything from file but it was dropped because there was too much arguing going on with the PR. It was a case of the perfect becoming the enemy of the good.

I am aware that one can create rules as json in files and indeed you can have more than one rule per file using an array. Back when the marketplace first started I even tried to publish a rule template to the marketplace using a very tediously hand coded JSON version of the MQTT Event Bus rule templates so I could have the one post to the marketplace with all three rule templates. It didn’t work, and I was told it couldn’t work without changes to core IIRC. That’s why you see three MQTT Event Bus postings for OH 3 on the marketplace. I worked around it for OH 4 by taking advantage of new ways to detect how the rule was triggered and new features added to the binding which let me do everything in one rule instead of three.

If it doesn’t do any validation, doesn’t support YAML, and apparently was never tested, I’d hardly call it already implemented. I suspect what happened was generic support for that was added for files in general but no one ever did anything with rule templates.

There are lots and lots of issues. And if this was implemented as part of a fix to some other PR or issue and my issue was never mentioned, how would I know unless I read and understood the code for each and every PR. I ain’t got time for that.

What you are seeing might also have been implemented before I opened my issues.

Or I might be misremebering and I only had conversations with the developers on other PRs and never opened an issue of my own. I can’t seem to find them. I’ll open new ones.

For my testing purposes here, I’ve simply run the YAML rule templates through this: Transform YAML into JSON - Online YAML Tools
..and then pasted the JSON to a file. It seems to import just fine, they show up as templates when I want to create a new rule. I then took two of these “converts” and surrounded with [] and separated with a comma and put into one file, and both show up in OH.

I didn’t say it was a working implementation, but somebody clearly made this and it got merged, even if untested. And no, it’s not “general support”, it’s specifically made for rule templates.

I don’t mean that those opening issues should read all PR’s “just in case” whatever the issue is about is handled. I mean that PRs, when merged, should to the extent possible, close all issues they resolve. Yeah, it might be some manual job for those handling the PR, but that’s where the responsibility should lie IMO. That’s how I’ve been used to working in other projects.

That would be particularly tragic.. it seems to be quite old indeed, as the current code stems from this commit: https://github.com/openhab/openhab-core/commit/a37cceab6767313e1b99643739bdc450428dcbde

…which means AFAIU that it originally comes from ESH (I’ve looked up stuff in the old archived ESH repo in the past, but it’s hardly worth it here just to find the date).

That must at the very least mean that nobody has ever considered the issue enough to check what’s already there..?

There’s no need, I think I’m close to figuring this out.

No, mine are Blockly templates

By the way, I made some very rudimentary validation in my commit if you look at TemplateFileProviderWatcher.java. I didn’t know exactly what to require, but this could easily be extended. I only check this:

    protected void validateObject(RuleTemplate template) throws ValidationException {
        String s;
        if ((s = template.getUID()) == null || s.isBlank()) {
            throw new ValidationException(ObjectType.TEMPLATE, null, "UID cannot be blank");
        }
        if ((s = template.getLabel()) == null || s.isBlank()) {
            throw new ValidationException(ObjectType.TEMPLATE, template.getUID(), "Label cannot be blank");
        }
        if (template.getModules(Module.class).isEmpty()) {
            throw new ValidationException(ObjectType.TEMPLATE, template.getUID(), "There must be at least one module");
        }
    }

..but it should at least be enough that “complete garbage” won’t be accepted.

ESH and OH 2 had a different marketplace which relied on a service provided by the Eclipse Foundation. When OH moved off the Eclipse project and merged ESH with OH, for a time there was no marketplace at all. Then support for using the forum as a marketplace was added after a time.

If this code was written back in the ESH days, it was written for a different marketplace and that marketplace did not support anything but bindings. No UI widgets (MainUI didn’t exist), no Block libraries (Blockly wasn’t an option) and no rule templates.

It may not do any validation because validation was done before it got posted to the marketplace.

I’m talking exclusively about the file based rule templates implementation now, it’s not really related to the marketplace. Still, the file based rule template code seems to stem from ESH.

Which is weird because there was no such thing as a rule template in ESH. It’s likely the start of an implementation that was never finished or something like that.

Ok, I just had to look it up. It comes from here (Nov 21, 2016):

https://github.com/eclipse-archived/smarthome/commit/442794d51e694ba1043dafe3cf452e23ed3ba3dc

The templates being talked about there are something internal to OH and not referring to rule templates. They might be something that could be reused to support rule templates but that was not their original intent.

It requires quite some “research” to get the overview over everything that has happened to related code over these 9 years, but I can say that I’ve seen that “template” and “rule template” seems to be used somewhat interchangeably in the Java code. I think that “template” might mean something slightly more generic, but I’m not quite sure. Regardless, it’s this very code that can read rule templates with my little bugfix :wink:

I’ll add that it seems like I’m on the verge of getting it to accept a “root YAML array” too, I just need to find a way to “reverse” the parser if the first element isn’t an array start element. GSON has a nice “peek” feature where you can peek at the next element without actually moving the parser ahead. That lets you make decisions like if you want to parse what’s comming as an array or as a single object, but it doesn’t look like Jackson’s (YAML) parser has this - at least I haven’t found it yet. But, I think I can get around that with using mark/reset on the underlying stream - I just need to figure out the details.

Once that is done, I think that YAML should work as well, and the parser is “generic” within automation, so it might actually handle other types of content too. It won’t have an impact outside of “automation” though.

Can I just say, I left this thread 3 days ago but you guys are so loquacious I now have a new novel to read :joy:

(I will read it)

1 Like

I bear a lot of responsibiliy for this. Nobody believes me, but I actually try to be brief. The success rate on the other hand…

I think I’ve got the YAML parser:

https://github.com/Nadahar/openhab-core/commit/5cf80397e59b8df5d3a82981b309150b7eae646b

It does parse my “trial YAMLs” at least, and they show up in OH like any other rule templates. This YAML parser is just for files though, not the marketplace, so it should probably be enabled there too..

I don’t know how to extensively test this to make sure it’s solid though… any ideas?

edit: These GitHub links will stop working once I’ve rebased, which I do quite a lot, so they won’t work probably sooner rather than later. But, the gist of the YAML parser is this:

    @Override
    public Set<Template> parse(InputStreamReader reader) throws ParsingException {
        try {
            Set<Template> templates = new HashSet<>();
            JsonNode rootNode = yamlMapper.readTree(reader);
            if (rootNode.isArray()) {
                List<RuleTemplateDTO> templateDtos = yamlMapper.convertValue(rootNode, new TypeReference<List<RuleTemplateDTO>>(){});
                for (RuleTemplateDTO templateDTO : templateDtos) {
                    templates.add(RuleTemplateDTOMapper.map(templateDTO));
                }
            } else {
                RuleTemplateDTO templateDto = yamlMapper.convertValue(rootNode, new TypeReference<RuleTemplateDTO>(){});
                templates.add(RuleTemplateDTOMapper.map(templateDto));
            }
            return templates;
        } catch (Exception e) {
            throw new ParsingException(new ParsingNestedException(ParsingNestedException.TEMPLATE, null, e));
        } finally {
            try {
                reader.close();
            } catch (IOException e) {
            }
        }
    }

Regarding the definition of “managed”, I found this on the “top level” ManagedProvider interface:

/**
 * The {@link ManagedProvider} is a specific {@link Provider} that enables to
 * add, remove and update elements at runtime.
 *
 * @author Dennis Nobel - Initial contribution
 *
 * @param <E>
 *            type of the element
 * @param <K>
 *            type of the element key
 */

Even though I’d say that “managed” is wide open for interpretation, that’s closer to how I have interpreted it. It isn’t related to how it’s stored (JSONDB, files or otherwise), but to whether it can be edited/managed from OH itself. If you go by that definition, there’s nothing that prevents “non-managed” storage in JSONDB, or “managed” storage in files, databases etc - as long as “write methods” are available.

I’ve managed to refactor the marketplace rule template provider to use the “same parsing” as the file based one, so that now it’s possible to have more than one rule template in one marketplace add-on as well, in both JSON and YAML format (they just have to be “in an array”).

I’ve modified my “test bundle” marketplace entry to include a YAML containing two rule templates, and it installs seemingly correctly and they both show up in the UI.

I think I should split all these “rule template fixes” out from the rest of the marketplace work, so that it can become a separate PR. There’s no reason that I can see that these have to go together, or that the rule template stuff should have to wait for the marketplace stuff to be completed.

2 Likes

As far as I can tell, this is ready and working, so I’ve prepared a branch with only the rules template fixes here: