Repository: SINTEF/dlite (language: python)

Issue:
Recover from invalid storage in storage path
If a file in DLITE_STORAGES (dlite.storage_path in Python) has a unknown file extension and cannot be loaded, the search stops with an error instead of continue to see if there is a storage that can be read and contain the instance asked for.
Index plugins
Plugins should be indexed in a separate text file to facilitate proper look-ups for DLite.

Currently DLite searches through all plugin files within the registered plugin directories resulting in import errors if plugins depend on packages that are not installed.
This is the case, even if a particular "problematic" plugin is not the intended target for a certain operation.

Suggestion:
Have a dedicated index file, e.g., a YAML file, in the root of each plugin folder that DLite can visit to look up the plugins available in the current folder.


Discussion / hints:

Currently "problematic plugins" are ignored with a warning. 

I am not sure if it is more user friendly to add a text file listing available plugins that the user will have to maintain?

Fix patch:
diff --git a/src/dlite-entity.c b/src/dlite-entity.c
index 66b19ee22..ddc0f460f 100644
--- a/src/dlite-entity.c
+++ b/src/dlite-entity.c
@@ -716,7 +716,13 @@ DLiteInstance *dlite_instance_get(const char *id)
 
     /* Set read-only as default mode (all drivers should support this) */
     if (!options) options = "mode=r";
-    if ((s = dlite_storage_open(driver, location, options))) {
+    ErrTry:
+      s = dlite_storage_open(driver, location, options);
+    ErrCatch(dliteStorageOpenError):  // suppressed error
+    ErrCatch(dliteStorageLoadError):  // suppressed error
+      break;
+    ErrEnd;
+    if (s) {
       /* url is a storage we can open... */
       ErrTry:
         inst = _instance_load_casted(s, id, NULL, 0);
@@ -731,7 +737,12 @@ DLiteInstance *dlite_instance_get(const char *id)
         const char *path;
         while (!inst && (path = fu_globnext(fiter))) {
 	  driver = (char *)fu_fileext(path);
-	  if ((s = dlite_storage_open(driver, path, options))) {
+          ErrTry:
+            s = dlite_storage_open(driver, path, options);
+          ErrCatch(dliteStorageOpenError):  // suppressed error
+            break;
+          ErrEnd;
+          if (s) {
             ErrTry:
               inst = _instance_load_casted(s, id, NULL, 0);
             ErrCatch(dliteStorageLoadError):  // suppressed error
diff --git a/src/dlite-errors.h b/src/dlite-errors.h
index 6877b9c6b..32edac52b 100644
--- a/src/dlite-errors.h
+++ b/src/dlite-errors.h
@@ -14,10 +14,10 @@ typedef enum {
   dliteNullReferenceError,
   dliteStorageOpenError,     /*!< cannot open storage plugin */
   dliteStorageLoadError,     /*!< cannot load storage plugin */
+  dliteStorageSaveError,     /*!< cannot save storage plugin */
 
   /* Should always be the last error */
   dliteLastError
 } DLiteErrors;
 
-
 #endif  /* _DLITE_ERRORS_H */
diff --git a/src/dlite-json.c b/src/dlite-json.c
index e93059577..99bb2e132 100644
--- a/src/dlite-json.c
+++ b/src/dlite-json.c
@@ -1076,7 +1076,7 @@ DLiteJsonFormat dlite_jstore_loads(JStore *js, const char *src, int len)
   unsigned int ntokens=0;
   char uuid[DLITE_UUID_LENGTH+1], *uri=NULL;
   int r;
-  DLiteJsonFormat format=-1;
+  DLiteJsonFormat retval=-1, format;
   DLiteJsonFlag flags=0;
   char *dots = (len > 30) ? "..." : "";
 
@@ -1116,11 +1116,12 @@ DLiteJsonFormat dlite_jstore_loads(JStore *js, const char *src, int len)
       t += jsmn_count(v) + 2;
     }
   }
+  retval = format;
 
  fail:
   if (tokens) free(tokens);
   if (uri) free(uri);
-  return format;
+  return retval;
 }
 
 /*
diff --git a/src/dlite-json.h b/src/dlite-json.h
index 7a6bc4bd1..f3f9cfbb2 100644
--- a/src/dlite-json.h
+++ b/src/dlite-json.h
@@ -25,8 +25,9 @@ typedef enum {
 
 /** Enum indicating whether a JSON string is formatted as data and metadata. */
 typedef enum {
-  dliteJsonDataFormat,    /*!< Data format */
-  dliteJsonMetaFormat     /*!< Metadata format */
+  dliteJsonUnknownFormat=-1,  /*!< Unknown format */
+  dliteJsonDataFormat=0,      /*!< Data format */
+  dliteJsonMetaFormat         /*!< Metadata format */
 } DLiteJsonFormat;
 
 
diff --git a/src/dlite-storage.c b/src/dlite-storage.c
index ec07bc04e..360866620 100644
--- a/src/dlite-storage.c
+++ b/src/dlite-storage.c
@@ -88,8 +88,8 @@ DLiteStorage *dlite_storage_open(const char *driver, const char *location,
   if (!(api = dlite_storage_plugin_get(driver))) goto fail;
   if (!(s = api->open(api, location, options))) goto fail;
   s->api = api;
-  if (!(s->location = strdup(location))) FAIL(NULL);
-  if (options && !(s->options = strdup(options))) FAIL(NULL);
+  if (!(s->location = strdup(location))) FAIL("allocation failure");
+  if (options && !(s->options = strdup(options))) FAIL("allocation failure");
 
   map_init(&s->cache);
 
diff --git a/src/utils/err.h b/src/utils/err.h
index cd4807a16..8cb17cd1d 100644
--- a/src/utils/err.h
+++ b/src/utils/err.h
@@ -621,7 +621,7 @@ ErrRecord *_err_get_record();
     }                                                \
     _ft = _fallthrough;                              \
     _fallthrough = 0;                                \
-    _last = _record.eval == errval || _ft;           \
+    _last = (_record.eval == errval) || _ft;         \
     _record.handled |= _last;                        \
     if (_last) _record.state = errTryCatch;          \
     switch (_last ? 1 : 0) {                         \
diff --git a/src/utils/plugin.c b/src/utils/plugin.c
index 11689771b..77bf9aa65 100644
--- a/src/utils/plugin.c
+++ b/src/utils/plugin.c
@@ -253,10 +253,12 @@ const PluginAPI *plugin_load(PluginInfo *info, const char *name,
       handle = NULL;
     }
   }
-  if (name && emit_err)
-    errx(1, "no such api: \"%s\"", name);
-  else
+  if (name) {
+    if (emit_err) errx(1, "no such api: \"%s\"", name);
+    retval = NULL;
+  } else {
     retval = loaded_api;
+  }
  fail:
   if (!retval && handle)
     (void)dsl_close(handle);


Test patch:
diff --git a/bindings/python/tests/entities/Empty.json b/bindings/python/tests/entities/Empty.json
new file mode 100644
index 000000000..b9d1d6bea
--- /dev/null
+++ b/bindings/python/tests/entities/Empty.json
@@ -0,0 +1,5 @@
+{
+  "url": "http://onto-ns.com/meta/0.1/Empty",
+  "dimensions": {}
+  "properties": {}
+}
diff --git a/bindings/python/tests/entities/Empty2.json b/bindings/python/tests/entities/Empty2.json
new file mode 100644
index 000000000..00119b45b
--- /dev/null
+++ b/bindings/python/tests/entities/Empty2.json
@@ -0,0 +1,4 @@
+{
+  "url": "http://onto-ns.com/meta/0.1/Empty",
+  "properties": {}
+}
diff --git a/bindings/python/tests/entities/Invalid1.json b/bindings/python/tests/entities/Invalid1.json
new file mode 100644
index 000000000..8401f9f18
--- /dev/null
+++ b/bindings/python/tests/entities/Invalid1.json
@@ -0,0 +1,7 @@
+{
+  "name": "Missing version",
+  "namespace": "http://onto-ns.com/meta",
+  "meta": "http://onto-ns.com/meta/0.3/EntitySchema",
+  "dimensions": {}
+  "properties": {}
+}
diff --git a/bindings/python/tests/entities/Invalid2.json b/bindings/python/tests/entities/Invalid2.json
new file mode 100644
index 000000000..4792bc587
--- /dev/null
+++ b/bindings/python/tests/entities/Invalid2.json
@@ -0,0 +1,7 @@
+{
+  "name": "Missing namespace",
+  "version": "0.0.1",
+  "meta": "http://onto-ns.com/meta/0.3/EntitySchema",
+  "dimensions": {}
+  "properties": {}
+}
diff --git a/bindings/python/tests/entities/Invalid3.json b/bindings/python/tests/entities/Invalid3.json
new file mode 100644
index 000000000..4f91de558
--- /dev/null
+++ b/bindings/python/tests/entities/Invalid3.json
@@ -0,0 +1,5 @@
+{
+  "url": "http://onto-ns.com/meta/0.1/Invalid3",
+  "description": "Missing properties",
+  "dimensions": {}
+}
diff --git a/bindings/python/tests/entities/Invalid4.json b/bindings/python/tests/entities/Invalid4.json
new file mode 100644
index 000000000..260c8b1eb
--- /dev/null
+++ b/bindings/python/tests/entities/Invalid4.json
@@ -0,0 +1,5 @@
+{
+  "description": "Missing url",
+  "dimensions": {}
+  "properties": {}
+}
diff --git a/bindings/python/tests/entities/InvalidJson.json b/bindings/python/tests/entities/InvalidJson.json
new file mode 100644
index 000000000..ded1e02c1
--- /dev/null
+++ b/bindings/python/tests/entities/InvalidJson.json
@@ -0,0 +1,6 @@
+xx{
+  "url": "http://onto-ns.com/meta/0.1/InvalidJson",
+  "meta": "http://onto-ns.com/meta/0.3/EntitySchema",
+  "dimensions": {}
+  "properties": {}
+}
diff --git a/bindings/python/tests/entities/Person.json b/bindings/python/tests/entities/Person.json
new file mode 100644
index 000000000..7e0dab81f
--- /dev/null
+++ b/bindings/python/tests/entities/Person.json
@@ -0,0 +1,34 @@
+{
+  "name": "Person",
+  "version": "0.1",
+  "namespace": "http://onto-ns.com/meta",
+  "meta": "http://onto-ns.com/meta/0.3/EntitySchema",
+  "description": "A person.",
+  "dimensions": [
+    {
+      "name": "N",
+      "description": "Number of skills."
+    }
+  ],
+  "properties": [
+    {
+      "name": "name",
+      "type": "string",
+      "description": "Full name."
+    },
+    {
+      "name": "age",
+      "type": "float",
+      "unit": "years",
+      "description": "Age of person."
+    },
+    {
+      "name": "skills",
+      "type": "string",
+      "dims": [
+        "N"
+      ],
+      "description": "List of skills."
+    }
+  ]
+}
diff --git a/bindings/python/tests/test_storage_path.py b/bindings/python/tests/test_storage_path.py
new file mode 100644
index 000000000..2144ad5ca
--- /dev/null
+++ b/bindings/python/tests/test_storage_path.py
@@ -0,0 +1,14 @@
+#!/usr/bin/env python
+from pathlib import Path
+
+import dlite
+
+
+# Configure paths
+thisdir = Path(__file__).parent.absolute()
+
+dlite.storage_path.append(thisdir / '*.txt')
+dlite.storage_path.append(thisdir / '*.yaml')
+dlite.storage_path.append(thisdir / 'entities' / '*.json')
+Person = dlite.get_instance('http://onto-ns.com/meta/0.1/Person')
+print(Person)