Marketplace versioning with embedded resource

I found the method in the UI widget handler that determines if a widget add-on is “installed”:

    @Override
    public boolean isInstalled(String id) {
        return widgetRegistry.getAll().stream().anyMatch(w -> w.hasTag(id));
    }

So, it’s that simple - that tag is the whole “coupling”. You can add a “marketplace tag” to any widget you have, and as long as it matches a “real marketplace widget” this widget will become “installed”, and uninstalling it will thus delete your widget. In the same manner, all you have to do to “make it your own” is to delete the marketplace tag.

It gets more complicated for rule templates because they don’t actually do anything by themselves. They have to be applied and configured to create an instance based on it. If the rule template is upgraded, should the rules created from the old version be modified or removed?

Or just store a “modified” flag with the widget.

I don’t like that idea at all.

Given that OH itself uses GitHub and as far as I can tell the vast majority of those who have posted this far are using GitHub, I suspect banning GitHub is not acceptable. I personally would be more than a little miffed.

If it’s good for the goose, it’s good for the gander.

:+1

That won’t work for rule templates though because there is no “rule templates builder” under developer tools (one of the things I had on my wish list for 5). There is no place to edit the template itself so no place to remove the tag. And there’s no way to add a template except through the marketplace so what would that even mean to remove the tag?

Of course the API or editing the jsondb is always possible.

I’ll address the other points when I’ve gathered some more information, but I think you misunderstood my suggestion here. I was only suggesting to “ban” github.com on “resource URLs” for the marketplace. The thing is that the github.com URLs never link to the file itself, they link to a web page showing the file in the browser. raw.githubusercontent.com on the other hand, links to a “raw” version of the file, that means, only the YAML, JSON or whatever content the file has. This is the version OH needs to load, the other one is a lot of HTML with the content deep down in the “node tree”.

I think I found a way to handle this though. If all that’s needed is to append ?raw=true if the address is github.com, it should be possible to handle. Github will then automatically redirect to the corresponding “raw” link. Some “finesse” is needed though, because currently the logic is quite “dumb” and only checks that the URL ends with .yaml or .json. That means that if people actually post a link using ?raw=true, OH will fail to recognize is as a “valid resource”. The URL needs to be parsed so that path and parameters separated before the “extension check” is done, and then, if “host” is github.com, raw=true could be added as an URL parameter.

I looked briefly at this last night, but I know so little about it and didn’t find any obvious ways to do it. Too much validation can be an enemy too, makinig things fail because of small syntax deviations. But, it doesn’t look like Jackson itself (the YAML parser) offers any more validation than that it errors out if it can’t parse the content.

I don’t think I’m the right person to look at this, considering how little I know about YAML and what Java components for dealing with YAML exists. Maybe this should become a GitHub issue?

I made this little “hack” in the Community Marketplace “resource URL processing”:

    private URI processResourceURL(String url) {
        URI uri;
        try {
            uri = new URI(url);
            if (uri.getFragment() != null) {
                throw new IllegalArgumentException("Fragment not allowed in resource URLs: " + uri.getFragment());
            }
            String host = uri.getHost();
            if (host == null) {
                throw new IllegalArgumentException("Missing host in resource URL: " + url);
            }
            String query, path;
            switch (host) {
                case "github.com":
                case "www.github.com":
                    // Modify GitHub URLs to use their "raw" version if necessary
                    path = uri.getPath();
                    if (path != null && path.contains("/blob/")) {
                        query = uri.getQuery();
                        if (query == null) {
                            return new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), "raw=true", null);
                        }
                        if (!query.contains("raw=true")) {
                            String[] parts = query.split("&");
                            String[] newParts = new String[parts.length + 1];
                            System.arraycopy(parts, 0, newParts, 0, parts.length);
                            newParts[parts.length] = "raw=true";
                            return new URI(uri.getScheme(), uri.getAuthority(), uri.getPath(), String.join("&", newParts), null);
                        }
                    }
            }
            return uri;
        } catch (URISyntaxException e) {
            throw new IllegalArgumentException("Invalid URL: " + url, e);
        }
    }

There might be some “loopholes” in it, making a proper URL query parameter parser handling URL encoding and the “full syntax” was a bit too much here. If there is a OH dependency with a “proper URL/URI parser” included already, it could be used to make it more robust, but I didn’t find anything with a little probing.

I think the above will work in the majority of cases though, you’d probably need some really awkwardly formatted URL to get in trouble.

The result of above is that now the above mentioned widget installs instead of making everything crash, so it’s at least an improvement:

Absolutely complicated yes. I don’t fully know how templates and rules relate/work yet, but looking in the JSONDB after making a test rule, it seems like the rule is forever “tied” to the template:

  "test1": {
    "class": "org.openhab.core.automation.dto.RuleDTO",
    "value": {
      "triggers": [],
      "conditions": [],
      "actions": [],
      "configuration": {
        "fixedTargetTime": "00:21"
      },
      "configDescriptions": [],
      "templateUID": "ysc:simulate_sunrise",
      "uid": "test1",
      "name": "test1",
      "tags": [],
      "visibility": "VISIBLE"
    }
  }

So, upgrading it would potentially break or change existing rules. That’s not good unless you actually want that because of a bug or similar.

That said, my suggestion was that maybe add-ons should only ever be automatically upgraded during OH upgrade if the existing add-on was no longer compatible with the new OH version. First of all, how likely is it that rule templates break in future versions? And, if they do, those rules will break anyway, they might have a bigger chance to “survive” if the template is updated than if it’s not.

edit: To avoid “erroneous deprecation” of add-ons, I’ve added support for “open ranges” in my implementation. That means that you can define e.g [4.3;) which means that this works for 4.3.0 and later. I think that having to edit the add-ons to have an upper limit if they “break” in the future is likely to cause less disruption than it is to having add-on authors constantly remember to bump the upper limit before new versions are released. It will depend on the add-on of course, bundles are more likely to break.

This is the JavaDocs for rule templates:

/**
 * This class is used to define {@code Rule Templates} which are shared combination of ready to use modules, which can
 * be configured to produce {@link Rule} instances.
 * <p>
 * The {@link RuleTemplate}s can be used by any creator of Rules, but they can be modified only by its creator. The
 * template modification is done by updating the {@link RuleTemplate}.
 * <p>
 * Templates can have {@code tags} - non-hierarchical keywords or terms for describing them.
 */

It’s very explicit that the templates can only be modified by their creator. I don’t know the ideology behind creating the templates, if this is an “important point” - but it could explain why there is no UI editor.

edit: Maybe I misinterpreted the intent of the JavaDocs, and that it is just a way to describe “the concept”.

Either way, after looking a little bit at rule templates, I can only find 4 “providers”:

  • TemplateFileProvider
  • TemplateResourceBundleProvider
  • CommandlineTemplateProvider
  • MarketplaceRuleTemplateProvider

So, it seems that rule templates can be provisioned both as files, bundles, from the marketplace and “from the command line” whatever that means. The JavaDocs for CommandlineTemplateProvider states:

/**
 * This class is implementation of {@link TemplateProvider}. It extends functionality of {@link AbstractCommandProvider}
 * <p>
 * It is responsible for execution of {@link AutomationCommandsPluggable}, corresponding to the {@link RuleTemplate}s:
 * <ul>
 * <li>imports the {@link RuleTemplate}s from local files or from URL resources
 * <li>provides functionality for persistence of the {@link RuleTemplate}s
 * <li>removes the {@link RuleTemplate}s and their persistence
 * </ul>
 */

That’s a digression though, it might just be I who don’t understand exactly what it does, and it certainly isn’t important in this context.

What is important is that there’s no ManagedRuleTemplateProvider. So, I think there’s more missing that just the UI to edit rule templates, “managed” rule templates doesn’t seem to exist. It might not be that hard to create, but it would be needed in addition to an editor in the UI. Following “the logic” of other types of add-ons, only the “managed” ones would be editable. All others would be write protected (with a padlock). The exception to this “rule” seems to be UI widgets and block libraries. For those, the marketplace “installs” them as “managed” add-ons, so that they are editable.

I would say, even though that’s probably not popular, that it would be better if this were consistent and that UI widgets and block libraries didn’t “share space” with the managed ones. Instead, I think it should be a button one could click to copy any such “element” (don’t know exactly what to call it, anything that can be stored as “managed”) from non-managed storage to managed (with a different UID). That way, there would be consistency across the different concepts, and it would be clear to the user what is “UI manageable” and not, without any blurry lines or misunderstandings.

I did misunderstand, but I’m not sure there is anything that can be done about it from a Discourse perspective. But you can throw up an error log after attempting to install it with a meaningful and actionable message “you must link to the raw file…”.

They already do fail for small syntax deviations, and often without error message or with error messages that are not meaningful. I’m helping someone right now deal with this.

Couldn’t hurt.

I’m not sure that you mean by that. The rule created from a template gets a marketplace tag and there’s a new “configuration” section at the top of the rule showing what you set as properties when you created the rule but that’s it. You can remove and readd the template and it will not do anything to the instantiated rule(s).

To upgrade to a new rule template you currently must:

  1. remove the rule template
  2. refresh the page (not sure if this is still required)
  3. add the rule template
  4. navigate to a rule created from the rule template
  5. open the code tab and copy the configuration section (if there are more than one or two parameters) and paste it somewhere
  6. delete the rule
  7. create a new rule and choose the rule template and use the configurations from 5 to fill in the properties
  8. save

It sucks. But as with the UI widgets, you can remove the marketplace tag.

Currently, that isn’t the case. Once the rule is created and saved, it stands on its own and continues to exist independent of the rule template.

What happens is the rule template marks in the code where each property should go with {{propName}}. When the rule is saved, those get literally replaced with a String find and replace with the value entered for that property. That’s it. At that point the rule exists on it’s own independently.

It wholly depends on whether there are breaking changes in the language the rule template was written in and the template was not defensively coded to account for that. I can’t say which languages are more or less likely to have a breaking change from one version of OH to the next.

Agreed. I think the best way to handle that though is not even attempt to do it automatically. Add a button to a rule created from a template to “sync with template”. That will read the configuration section and reapply the properties to what ever the current template happens to be. If it was updated then the rule will be updated.

This might break if a new required property was added to the template though. That would need to be handled somehow.

But by giving the end user this responsibility, it also lets one independently update the template. Because templates are developed on their own schedule, waiting for a full OH upgrade isn’t good enough.

Agreed.

I’m not sure what that even means in the context of the JavaDocs. What’s the “creator” in this context? Is that documenting a technical limitation or a philosophical statement? Are they talking about the marketplace posting?

From an end user’s perspective, once a rule is instantiated from a template, they can edit it all they want. JavaDoc would preclude the ability to create an editor under Developer Tools. There isn’t one because no one has volunteered to build it. But as far as I know there is no active hostility to that idea in the first place.

I made a remedy for this that I think shouldn’t have too many side-effects.

Yeah ok, I haven’t found a YAML validator to validate it anyway, although I’m sure they exist. But, even though they do fail now, installing them doesn’t fail. If validation failed, it would be natural to abort the installation (to prevent crashes etc. from processing garbage), which might be potentially annoying if the error was minor. But, it’s probably not an important point.

Ah, thanks for the insight. I hadn’t understood that separate rules were actually “compiled” using the templates, I just saw that it simply stored a “configuration” of the template together with a link to the template in JSONDB. But, from your description, it sounds like there’s more too it.

I saw a comment in the JavaDocs somewhere that there was an “update” method that must be called to update the rules if a template had been updates, to it should be possible to make the process somewhat smoother than what you describe. But, I still haven’t grasped exactly how this is done (in Java/Core).

:+1: That’s what I called “compiled” above. So, an independent rules is created, the template is just used during the creation.

I tested that, and nothing showed up under “rules”, but I now see that there’s an error in my log that I hadn’t provided a mandatory configuration, so that’s probably what prevented the rule from being created - which led to to drawing the wrong conclusion.

Yeah, I didn’t quite take into account that they are written in e.g JavaScript. That probably makes breaking a more likely event, at least with major releases.

I’m not exactly sure either, it’s certainly not a technical limitation, it seems to be an attempt to document the “concept” of rule templates. It’s not related to the marketplace, this is documented on “the top level rule template interface”. But, I’m not sure it’s important. I think maybe I “misread” is sligtly, that it was only an attempt to separate the concept of “a template” from the concept of “a rule”.

Regarding rule templates, see the edit to my previous post who elaborate the topic further.

In truth, all rule templates are managed. There is no way to get a rule template into your OH config except by installing it from the marketplace. There is no file I can create locally and test things out before I create a public posting in the marketplace. There is no builder.

But “managed” really just means “saved in JSONDB and not defined via separate files; i.e. OH manages the storage of the config” and by that definition, all rule templates are managed. It’s impossible to create a non-managed rule template.

If you wanted to create a file base rule template that would be pretty cool. But for now, it’s that which is lacking, not the managed side. There’s nothing to mark it as managed because that would be redundant. If it’s a rule template it is by definition managed.

Rule templates should work the same as UI Widgets currently do.

Right now the lines are pretty clear. If it’s in a file under $OH_CONF it’s unmanaged, and you can’t change it through the UI. Everything else is managed.

I don’t think that’s correct, but I must do more investigation to figure this out. But, to me, it seems that you can indeed created files, in some format, and they will be picked up as “non-managed” rule templates. I’ll have to get back when I’ve studied the details further. But it’s not as black and white that it’s “either file based or managed”. At least the code in Core is much more general than that, you can in principle have an unlimited amount of different providers for many things. Transformations for example are stored in the JSONDB separate from the “managed” transformations that you can edit from the UI. That’s why they will have a padlock on them if you look at them from the UI. They simply install to different “paths” in the JSONDB.

This isn’t about technical limitations, it has to do with how it’s designed. It might be that different implementers have had slightly different ideas/philosophies, which is why things don’t fully line up.

Nope, I opened an issue to add that but it was never picked up. Unless someone added it without mentioning my issue and I missed it (I usually watch all the PRs that get applied to core and the UIs) there is no way to load a rule template from a file.

I’d be very happy to hear otherwise.

Right but my assertion is the transformations shouldn’t have the padlock in the first place.

It is implemented as it is, but I think that it’s wrong and inconsistent. There is no reason to prevent end users from modifying what they’ve installed for transformations but not for Rule Templates, UI Widgets, and Block Libraries.

It’s indeed there, but it doesn’t look like it’s ever been tested. I’ve fixed one bug so far, which makes it import “something”. I don’t know how to write rule templates, so I guess I will have to copy/paste some from the marketplace into a file.

It seems like the expected format is JSON, and I think it’s parsed just like those from the marketplace - so the syntax should be the same. The location for these files is conf/automation/templates. However, because of the bug I fixed, it fails to pick them up because it doesn’t look in this folder but in the “root application folder”, despite watching the correct folder. So, when the OS tells it “a new file named x.json has been created” in the watched (conf/automation/templates), it tries to read the file from a completely different folder, and fails with that the file isn’t readable (obviously because it doesn’t exist). Nothing is logged, so it just fails silently.

Further testing is needed, there could be more bugs here, I’m not really sure if handling of modified files has been implemented. It kind of seems to just handle creation and deletion, but maybe the “creation process” also works for modified files.

The bugfix I’ve done so far is this, and at least this makes it do something:

 .../provider/file/AutomationWatchService.java      | 23 +++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/provider/file/AutomationWatchService.java b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/provider/file/AutomationWatchService.java
index 42e3539bf..092a4b6df 100644
--- a/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/provider/file/AutomationWatchService.java
+++ b/bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/internal/provider/file/AutomationWatchService.java
@@ -12,11 +12,14 @@
  */
 package org.openhab.core.automation.internal.provider.file;
 
-import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.openhab.core.service.WatchService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * This class is an implementation of {@link WatchService.WatchEventListener} which is responsible for tracking file
@@ -33,6 +36,7 @@ public class AutomationWatchService implements WatchService.WatchEventListener {
     private final WatchService watchService;
     private final Path watchingDir;
     private AbstractFileProvider provider;
+    private final Logger logger = LoggerFactory.getLogger(AutomationWatchService.class);
 
     public AutomationWatchService(AbstractFileProvider provider, WatchService watchService, String watchingDir) {
         this.watchService = watchService;
@@ -54,13 +58,18 @@ public class AutomationWatchService implements WatchService.WatchEventListener {
 
     @Override
     public void processWatchEvent(WatchService.Kind kind, Path path) {
-        File file = path.toFile();
-        if (!file.isHidden()) {
-            if (kind == WatchService.Kind.DELETE) {
-                provider.removeResources(file);
-            } else if (file.canRead()) {
-                provider.importResources(file);
+        Path fullPath = watchingDir.resolve(path);
+        try {
+            if (kind == WatchService.Kind.CREATE || kind == WatchService.Kind.MODIFY) {
+                if (!Files.isHidden(fullPath)) {
+                    provider.importResources(fullPath.toFile());
+                }
+            } else if (kind == WatchService.Kind.DELETE) {
+                provider.removeResources(fullPath.toFile());
             }
+        } catch (IOException e) {
+            logger.error("Failed to process automation watch event {} for \"{}\": {}", kind, fullPath, e.getMessage());
+            logger.trace("", e);
         }
     }
 }

I ran this rule templace through a YAML to JSON converter to get this:

{
  "uid": "rules_tools:tsm",
  "label": "Time Based State Machine",
  "description": "Creates timers to transition a state Item to a new state at defined times of day.",
  "configDescriptions": [
    {
      "name": "timeOfDay",
      "type": "TEXT",
      "context": "item",
      "filterCriteria": [
        {
          "name": "type",
          "value": "String"
        }
      ],
      "label": "Time of Day State Item",
      "required": true,
      "description": "String Item that holds the current time of day's state."
    },
    {
      "name": "timesOfDayGrp",
      "type": "TEXT",
      "context": "item",
      "filterCriteria": [
        {
          "name": "type",
          "value": "Group"
        }
      ],
      "label": "Times of Day Group",
      "required": true,
      "description": "Has as members all the DateTime Items that define time of day states."
    },
    {
      "name": "namespace",
      "type": "TEXT",
      "label": "Time of Day Namespace",
      "required": true,
      "description": "The Item metadata namespace (e.g. \"tsm\")."
    }
  ],
  "triggers": [
    {
      "id": "1",
      "configuration": {
        "groupName": "{{timesOfDayGrp}}"
      },
      "type": "core.GroupStateChangeTrigger"
    },
    {
      "id": "2",
      "configuration": {
        "startlevel": 100
      },
      "type": "core.SystemStartlevelTrigger"
    },
    {
      "id": "4",
      "configuration": {
        "time": "00:05"
      },
      "type": "timer.TimeOfDayTrigger"
    }
  ],
  "conditions": [],
  "actions": [
    {
      "inputs": {},
      "id": "3",
      "configuration": {
        "type": "application/javascript",
        "script": "// Version 1.0\nvar {TimerMgr, helpers} = require('openhab_rules_tools');\nconsole.loggerName = 'org.openhab.automation.rules_tools.TimeStateMachine';\n//osgi.getService('org.apache.karaf.log.core.LogService').setLevel(console.loggerName, 'DEBUG');\n\nhelpers.validateLibraries('4.2.0', '2.0.3');\n\nconsole.debug('Starting state machine in ten seconds...');\n\n// Properties\nvar STATE_ITEM  = \"{{timeOfDay}}\";\nvar DT_GROUP    = \"{{timesOfDayGrp}}\";\nvar DAY_TYPES   = ['custom', 'holiday', 'dayset', 'weekend', 'weekday', 'default'];\nvar NAMESPACE   = '{{namespace}}';\nvar USAGE =   'Time Based State Machine Usage:\\n'\n            + 'All date times must be a member of ' + DT_GROUP + '.\\n'\n            + 'Each member of the Group must have ' + NAMESPACE + ' Item metadata of the following format:\\n'\n            + '  .items file: ' + NAMESPACE +'=\"STATE\"[type=\"daytype\", set=\"dayset\", file=\"uri\"]\\n'\n            + \"  UI YAML: use '\" + NAMESPACE + \"' for the namespace and metadata format:\\n\"\n            + '    value: STATE\\n'\n            + '    config:\\n'\n            + '      type: daytype\\n'\n            + '      set: dayset\\n'\n            + '      file: uri\\n'\n            + 'Where \"STATE\" is the state machine state that begins at the time stored in that Item, '\n            + '\"daytype\" is one of \"default\", \"weekday\", \"weekend\", \"dayset\", \"holiday\", or \"custom\". '\n            + 'If \"dayset\" is chosen for the type, the \"set\" property is required indicating the name of the '\n            + 'custom dayset configured in Ephemeris. If \"custom\" is chosen as the type, the \"file\" property '\n            + 'is required and should be the fully qualified path the the Ephemeris XML file with the custom '\n            + 'holidays defined. The \"set\" and \"file\" properties are invalid when choosing any of the other '\n            + '\"types\".';\n\n/**\n * Validates the passed in Item has valid NAMESPACE metadata.\n *\n * @param {string} itemName name of the Item to check\n * @throws exception if the metadata doesn't exist or is invalid\n */\nvar validateItemConfig = (itemName) => {\n  const md = items[itemName].getMetadata()[NAMESPACE];\n\n  if(md.value === undefined || md.value === null || md.value === '') {\n    throw itemName + ' has malformed ' + NAMESPACE + ' metadata, no value found!';\n  }\n\n  const dayType = md.configuration['type'];\n  if(!dayType) {\n    throw itemName + ' has malformed ' + NAMESPACE + ' metadata, required \"type\" property is not found!';\n  }\n\n  if(dayType == 'dayset' && !md.configuration['set']) {\n    throw itemName + ' has malformed ' + NAMESPACE + ' metadata, type is \"dayset\" but required \"set\" property is not found!';\n  }\n\n  if(dayType == 'custom' && !md.configuration['file']) {\n    throw itemName + ' has malformed ' + NAMESPACE + ' metadata, type is \"custom\" but required \"file\" property is not found!';\n  }\n\n  if(!items[itemName].type.startsWith('DateTime')) {\n    throw itemName + ' is not a DateTime Item!';\n  }\n\n  if(items[itemName].isUninitialized) {\n    throw itemName + \" is not initialized!: \" + items[itemName].state;\n  }\n\n  console.debug(itemName+ ' is valid');\n};\n\n/**\n * Return all members of the DT_GROUP that has a \"type\" metadata configuration property that\n * matches the passed in type.\n *\n * @param {string} type the day type defined in the metadata we want to get the Items for\n * @returns {Array} all the Items with the matching type in the metadata\n */\nvar getItemsOfType = (type) => {\n  const allItems = items[DT_GROUP].members;\n  return allItems.filter( item => item.getMetadata()[NAMESPACE].configuration['type'] == type);\n};\n\n/**\n * Returns true if all the Items of the given type have a unique \"state\" value\n * in the metadata.\n *\n * @param {string} the day type\n * @returns {boolean} true if all states are unique, false otherwise\n */\nvar checkUniqueStates = (type) => {\n  const allItems = getItemsOfType(type);\n  const states = new Set(allItems.map(i => { return i.getMetadata()[NAMESPACE].value; }));\n  return !allItems.length || allItems.length == states.size;\n};\n\n/**\n * Check that all Items are configured correctly.\n */\nvar validateAllConfigs = () => {\n  console.debug('Validating Item types, Item metadata, and Group membership');\n\n  // Check that all members of the Group have metadata\n  const itemsWithMD = items[DT_GROUP].members.filter(item => item.getMetadata(NAMESPACE)).length;\n  if(itemsWithMD != items[DT_GROUP].members.length) {\n    const noMdItems = items[DT_GROUP].members.filter(item => !item.getMetadata(NAMESPACE));\n    console.warn('The following Items do not have required ' + NAMESPACE + ' metadata: ' + noMdItems.map(item => item.name).join(', '));\n    return false; // no sense on performing any additional tests\n  }\n\n  // Check each Item's metadata\n  let isGood = helpers.checkGrpAndMetadata(NAMESPACE, DT_GROUP, validateItemConfig, USAGE);\n\n  // Check the state item\n  if(!items[STATE_ITEM]){\n    console.warn('The state Item ' + STATE_ITEM + ' does not exist!');\n    isGood = false;\n  }\n\n  if(!items[STATE_ITEM].type.startsWith('String')) {\n    console.warn('The state Item ' + STATE_ITEM + ' is not a String Item!');\n    isGood = false;\n  }\n\n  // Check to see if we have a default set of Items\n  if(!getItemsOfType('default')) {\n    console.warn('There are no \"default\" day type Items defined! Make sure you have all day types covered!');\n    // we'll not invalidate if there are no \"default\" items\n  }\n\n  // Check that each data set has a unique state for each Item\n  DAY_TYPES.forEach(type => {\n    if(!checkUniqueStates(type)) {\n      console.warn('Not all the metadata values for Items of type ' + type + ' are unique!');\n      isGood = false;\n    }\n  })\n\n  // Report if all configs are good or not\n  if(isGood) {\n    console.debug('All ' + NAMESPACE + ' Items are configured correctly');\n  }\n  return isGood;\n};\n\n/**\n * Pull the set of Items for today based on Ephemeris. The Ephemeris hierarchy is\n * - custom\n * - holiday\n * - dayset\n * - weeekend\n * - weekday\n * - default\n *\n * If there are no DateTime Items defined for today's type, null is returned.\n */\nvar getTodayItems = () => {\n  // Get all the DateTime Items that might apply to today given what type of day it is\n  // For example, if it's a weekend, there will be no weekday Items pulled. Whether or not\n  // the entry in this dict has an array of Items determines whether today is of that day\n  // type.\n  const startTimes = [\n    { 'type': 'custom',  'times' : getItemsOfType('custom').filter(item => actions.Ephemeris.isBankHoliday(0, item.getMetadata()[NAMESPACE].configuration['file'])) },\n    { 'type': 'holiday', 'times' : (actions.Ephemeris.isBankHoliday()) ? getItemsOfType('holiday') : [] },\n    { 'type': 'dayset',  'times' : getItemsOfType('dayset').filter(item => actions.Ephemeris.isInDayset(items.getMetadata()[NAMESPACE].configuration['set'])) },\n    { 'type': 'weekend', 'times' : (actions.Ephemeris.isWeekend()) ? getItemsOfType('weekend') : [] },\n    { 'type': 'weekday', 'times' : (!actions.Ephemeris.isWeekend()) ? getItemsOfType('weekday') : [] },\n    { 'type': 'default', 'times' : getItemsOfType('default') }\n  ];\n\n  // Go through startTimes in order and choose the first one that has a non-empty list of Items\n  const dayStartTimes = startTimes.find(dayset => dayset.times.length);\n\n  if(dayStartTimes === null) {\n    console.warn('No DateTime Items found for today');\n    return null;\n  }\n  else {\n    console.info('Today is a ' + dayStartTimes.type + ' day.');\n    return dayStartTimes.times;\n  }\n};\n\n/**\n * Returns a function called to transition the state machine from one state to the next\n *\n * @param {string} state the new state to transition to\n * @param {function} the function that transitions the state\n */\nvar stateTransitionGenerator = (state) => {\n  return function() {\n    console.info('Transitioning Time State Machine from ' + items[STATE_ITEM].state + ' to ' + state);\n    items[STATE_ITEM].sendCommand(state);\n  }\n}\n\n/**\n * Returns a function that generates the timers for all the passed in startTimes\n *\n * @param {Array} startTimes list of today's state start times\n * @param {timerMgr.TimerMgr} timers collection of timers\n * @returns {function} called to generate the timers to transition between the states\n */\nvar createTimersGenerator = (timers) => {\n  return function() {\n\n    if(validateAllConfigs()) {\n\n      // Cancel the timers, skipping the debounce timer\n      console.debug('Cancelling existing timers');\n      timers.cancelAll();\n\n      // Get the set of Items for today's state machine\n      console.debug(\"Acquiring today's state start times\");\n      const startTimes = getTodayItems();\n\n      // Get the state and start time, sort them ignoring the date, skip the ones that have\n      // already passed and create a timer to transition for the rest.\n      console.debug('Creating timers for times that have not already passed');\n      var mapped = startTimes.map(i => { return { 'state': i.getMetadata()[NAMESPACE].value,\n                                                  'time' : time.toZDT(i.state).toToday() } });\n      mapped.sort((a,b) => {\n               if(a.time.isBefore(b.time)) return -1;\n                else if(a.time.isAfter(b.time)) return 1;\n                else return 0;\n              })\n              .filter(tod => tod.time.isAfter(time.toZDT()))\n              .forEach(tod => {\n                // TODO: see if we can move to rules instead of timers\n                console.debug('Creating timer for ' + tod.state + ' at ' + tod.time);\n                timers.check(tod.state, tod.time.toString(), stateTransitionGenerator(tod.state));\n              });\n\n      // Figure out the current time of day and move to that state if necessary\n      var beforeTimes = mapped.sort((a,b) => {\n                                if(a.time.isAfter(b.time)) return -1;\n                                else if(a.time.isBefore(b.time)) return 1;\n                                else return 0;\n                              })\n                              .filter(tod => tod.time.isBefore(time.toZDT()));\n      if(!beforeTimes.length) {\n        console.debug(\"There is no date time for today before now, we can't know what the current state is, keeping the current time of day state of \" + items[STATE_ITEM].state + \".\");\n      }\n      else {\n        const currState = beforeTimes[0].state\n        const stateItem = items[STATE_ITEM];\n        console.info('The current state is ' + currState);\n        if(stateItem.state != currState) stateItem.sendCommand(currState)\n      }\n    }\n    else {\n      console.warn('The config is not valid, cannot proceed!');\n    }\n\n  };\n};\n\nvar timers = cache.private.get('timers', () => TimerMgr());\n\n// Wait a minute after the last time the rule is triggered to make sure all Items are done changing (e.g.\n// Astro Items) before calculating the new state.\ntimers.check('debounce',\n             'PT10S',\n             createTimersGenerator(timers),\n             true,\n             () => { console.debug('Flapping detected, waiting before creating timers for today'); });\n"
      },
      "type": "script.ScriptAction"
    }
  ]
}

I then stored that in a file in the “templates” folder, and my UI now shows (ignore the empty entry, it’s from my test with an empty file):

So, it seems to be working on a fundamental level at least. I can’t get JS to work on the “demo.app”, so I can’t test anything using JS, so I can’t do any further testing that seeing that it appears. Do you know if a rules DSL rules template exists - that one I should be able to actually test?

The complete lack of validation is a problem though, it will actually “import” a blank file as a completely empty “rule template” - which doesn’t even have a name, as you can see in the screen shot above. I don’t know what rules such a validation should have, but it must be possible to come up with some criteria that decides if it’s “acceptable” or not.

It should also be noted that it seems like the file can contain a JSON array of multiple rule templates.

There’s also another folder being watched for some kind of files - I have no idea what you can put there: conf\automation\moduletypes

It doesn’t look like there are any checks that prevents multiple files from using the same UID either. I don’t know what the consequences of that is, but it kind of undermines the whole “unique” concept. They will probably “overwrite” each other somewhere, so that whatever file was processed last “wins”.

I found another bug in the “file watching” implementation that I think exist in other classes too - when a file is deleted, there is a check if the file is hidden that is applied to filter out hidden files before any action is taken.

That logic is flawed however, because on Windows the OS must access the file system to check if a file is hidden (hidden is a file attribute). Since the file has already been deleted, this check fails with “file not found” and the removal procedure is aborted. On Linux this probably works, because a “hidden file” on most Linux filesystems simply means that the file name starts with a . - so no filesystem access is required to determine this, which means it doesn’t abort the processing.

The “hidden check” is unneccesary for a “removal” even though, so it’s easy to fix by just removing the check. Nothing is done during removal if a “matching resource” isn’t found in the registry, and if a hidden file is never “registered” in the first place, it won’t be in the registry and thus it’s harmless to let it try to “unregister” it.

Except for the lack of validation (both syntax and UID) and the path bug I suggested a fix for above (I’ve updated it with the “hidden” fix), it seems like file based rule templates in JSON format is working. It would probably be nice to add a YAML option as well, which shouldn’t be too hard since the marketplace already manages to parse rule templates from YAML files.

This commit should fix file base rule templates (and module types, since it’s the same code). It should probably be separated out from the “marketplace” branch and become a PR of its own. I’m currently trying to figure out how hard it is to let it accept YAML as well.

https://github.com/Nadahar/openhab-core/commit/0b6cce82f26bb7c15219c19c4c753681b0c78b75

If JSON is the only supported format it’s next to useless anyway.

Because you cannot have newlines in a JSON element all the code gets put on one line with \n in place of the newlines. As a human that’s basically unworkable.

I think the posts by ysc and hmerk are rules DSL.

That’s another thing I opened an issue for.