Repository: SINTEF/dlite (language: python)

Issue:
Adding options to the Python plugin from_bytes() and to_bytes() methods
# Description
Adding options to the Python plugin from_bytes() and to_bytes() methods.

Note, that this PR will not make old Python plugins with `to_bytes()` and/or `from_bytes()` methods without the `options` argument stop working.

## Type of change
- [ ] Bug fix & code cleanup
- [x] New feature
- [ ] Documentation update
- [ ] Test update

## Checklist for the reviewer
This checklist should be used as a help for the reviewer.

- [ ] Is the change limited to one issue?
- [ ] Does this PR close the issue?
- [ ] Is the code easy to read and understand?
- [ ] Do all new feature have an accompanying new test?
- [ ] Has the documentation been updated as necessary?



Discussion / hints:

Fix patch:
diff --git a/bindings/python/dlite-entity.i b/bindings/python/dlite-entity.i
index 80d35ae30..23b78a51f 100644
--- a/bindings/python/dlite-entity.i
+++ b/bindings/python/dlite-entity.i
@@ -430,11 +430,11 @@ Call signatures:
       *LEN = 0;
       return;
     }
-    if (!(buf = malloc(n))) {
+    if (!(buf = malloc(n+1))) {
       dlite_err(dliteMemoryError, "allocation failure");
       return;
     }
-    if ((m = dlite_instance_memsave(driver, buf, n, $self, options)) < 0)
+    if ((m = dlite_instance_memsave(driver, buf, n+1, $self, options)) < 0)
       return;
     assert (m == n);
     *ARGOUT_BYTES = buf;
diff --git a/src/dlite-misc.c b/src/dlite-misc.c
index ffb67a389..ba075c2a5 100644
--- a/src/dlite-misc.c
+++ b/src/dlite-misc.c
@@ -217,7 +217,7 @@ int dlite_option_parse(char *options, DLiteOpt *opts, int modify)
     }
     if (!opts[i].key) {
       int len = strcspn(p, "=;&#");
-      return errx(1, "unknown option key: '%.*s'", len, p);
+      return errx(dliteValueError, "unknown option key: '%.*s'", len, p);
     }
   }
   return 0;
diff --git a/src/utils/jstore.c b/src/utils/jstore.c
index 29819af18..fc83b14c5 100644
--- a/src/utils/jstore.c
+++ b/src/utils/jstore.c
@@ -226,26 +226,34 @@ int jstore_update_from_jsmn(JStore *js, const char *src, jsmntok_t *tok)
   return 0;
 }
 
-/* Update JSON store with values from file `filename`.
+/* Update JSON store with values from string `buf`.
    Return non-zero on error. */
-int jstore_update_from_file(JStore *js, const char *filename)
+int jstore_update_from_string(JStore *js, const char *buf, int len)
 {
   jsmn_parser parser;
   jsmntok_t *tokens=NULL;
   unsigned int ntokens=0;
   int r, stat;
-  char *buf;
-  if (!(buf = jstore_readfile(filename)))
-    return err(1, "error reading JSON file \"%s\"", filename);
   jsmn_init(&parser);
-  r = jsmn_parse_alloc(&parser, buf, strlen(buf), &tokens, &ntokens);
+  r = jsmn_parse_alloc(&parser, buf, len, &tokens, &ntokens);
   if (r < 0) {
-    free(buf);
-    return err(1, "error parsing JSON file \"%s\": %s",
-               filename, jsmn_strerror(r));
+    return err(1, "error parsing JSON buffer \"%.70s\": %s",
+               buf, jsmn_strerror(r));
   }
   stat = jstore_update_from_jsmn(js, buf, tokens);
   free(tokens);
+  return stat;
+}
+
+/* Update JSON store with values from file `filename`.
+   Return non-zero on error. */
+int jstore_update_from_file(JStore *js, const char *filename)
+{
+  int stat;
+  char *buf;
+  if (!(buf = jstore_readfile(filename)))
+    return err(1, "error reading JSON file \"%s\"", filename);
+  stat = jstore_update_from_string(js, buf, strlen(buf));
   free(buf);
   return stat;
 }
diff --git a/src/utils/jstore.h b/src/utils/jstore.h
index e907157b1..5b51a6aa5 100644
--- a/src/utils/jstore.h
+++ b/src/utils/jstore.h
@@ -102,8 +102,12 @@ int jstore_update(JStore *js, JStore *other);
     be an object. Return non-zero on error. */
 int jstore_update_from_jsmn(JStore *js, const char *src, jsmntok_t *tok);
 
+/** Update JSON store with values from string `buf`.
+    Return non-zero on error. */
+int jstore_update_from_string(JStore *js, const char *buf, int len);
+
 /** Update JSON store with values from file `filename`.
-   Return non-zero on error. */
+    Return non-zero on error. */
 int jstore_update_from_file(JStore *js, const char *filename);
 
 /** Update `filename` from JSON store.
diff --git a/storages/json/dlite-json-storage.c b/storages/json/dlite-json-storage.c
index f39473c91..93ba76876 100644
--- a/storages/json/dlite-json-storage.c
+++ b/storages/json/dlite-json-storage.c
@@ -35,13 +35,16 @@ typedef struct {
     - 'r': if `uri` is in single-entity format
     - 'a': otherwise
 */
-static int default_mode(const char *uri)
+static int default_mode(const char *uri, const unsigned char *buf, size_t size)
 {
   int mode, stat;
   JStore *js = jstore_open();
 
   ErrTry:
-    stat = jstore_update_from_file(js, uri);
+    if (uri)
+      stat = jstore_update_from_file(js, uri);
+    else
+      stat = jstore_update_from_string(js, (const char *)buf, size);
   ErrCatch(1):
     break;
   ErrEnd;
@@ -52,36 +55,12 @@ static int default_mode(const char *uri)
 }
 
 
-/**
-  Returns an url to the metadata.
-
-  Valid `options` are:
-
-  - mode : r | w | a
-      Valid values are:
-      - r   Open existing file for read-only
-      - w   Truncate existing file or create new file
-      - a   Append to existing file or create new file (default)
-  - single : yes | no
-      Whether to write single-entity format.
-  - uri-key : yes | no
-      Whether to use URI (if it exists) as json key instead of UUID
-  - with-uuid : yes | no
-      Whether to include uuid in output.
-  - with-meta : yes | no
-      Whether to always include meta in output (even for metadata).
-  - arrays : yes | no
-      Whether to write metadata dimensions and properties as arrays.
-  - as-data : yes | no (deprecated)
-  - meta : yes | no (deprecated)
-      Whether to format output as metadata. Alias for `with-uuid`
-  - compact : yes | no (deprecated)
-      Whether to write output in compact format. Alias for `as-data`
-  - useid: translate | require | keep (deprecated)
-      How to use the ID.
+/*
+  Help function for loading json data. Either `uri` or `buf` should be given.
  */
-DLiteStorage *json_open(const DLiteStoragePlugin *api, const char *uri,
-                        const char *options)
+DLiteStorage *json_loader(const DLiteStoragePlugin *api, const char *uri,
+                          const unsigned char *buf, size_t size,
+                          const char *options)
 {
   DLiteJsonStorage *s=NULL;
   DLiteStorage *retval=NULL;
@@ -92,10 +71,10 @@ DLiteStorage *json_open(const DLiteStoragePlugin *api, const char *uri,
   DLiteOpt opts[] = {
     {'m', "mode",      "", mode_descr},
     {'s', "single",    "", "Whether to write single-entity format"},
-    {'s', "uri-key",   "false", "Whether to use uri as json key"},
+    {'k', "uri-key",   "false", "Whether to use uri as json key"},
     {'u', "with-uuid", "false", "Whether to include uuid in output"},
     {'M', "with-meta", "false", "Always include meta in output"},
-    {'a', "arrays",    "true",  "Serialise metadata dimensions and properties as arrays"},
+    {'a', "arrays",    "false", "Serialise metadata dimensions and properties as arrays"},
     {'d', "as-data",   "false", "Alias for `single=false` (deprecated)"},
     {'c', "compact",   "false", "Alias for `single` (deprecated)"},
     {'U', "useid",     "",      "Unused (deprecated)"},
@@ -140,7 +119,8 @@ DLiteStorage *json_open(const DLiteStoragePlugin *api, const char *uri,
    FAILCODE(dliteMemoryError, "allocation failure");
   s->api = api;
 
-  if (!mode) mode = default_mode(uri);
+  if (!mode)
+    mode = default_mode(uri, buf, size);
   s->flags |= dliteGeneric;
   switch (mode) {
   case 'r':
@@ -175,8 +155,12 @@ DLiteStorage *json_open(const DLiteStoragePlugin *api, const char *uri,
 
   /* Load jstore if not in write mode */
   if (load) {
+    DLiteJsonFormat fmt;
     if (!(s->jstore = jstore_open())) goto fail;
-    DLiteJsonFormat fmt = dlite_jstore_loadf(s->jstore, uri);
+    if (uri)
+      fmt = dlite_jstore_loadf(s->jstore, uri);
+    else
+      fmt = dlite_jstore_loads(s->jstore, (const char *)buf, size);
     if (fmt < 0) goto fail;
     if (fmt == dliteJsonMetaFormat && mode != 'a') s->flags &= ~dliteWritable;
   }
@@ -191,6 +175,42 @@ DLiteStorage *json_open(const DLiteStoragePlugin *api, const char *uri,
 }
 
 
+
+/**
+  Returns an url to the metadata.
+
+  Valid `options` are:
+
+  - mode : r | w | a
+      Valid values are:
+      - r   Open existing file for read-only
+      - w   Truncate existing file or create new file
+      - a   Append to existing file or create new file (default)
+  - single : yes | no
+      Whether to write single-entity format.
+  - uri-key : yes | no
+      Whether to use URI (if it exists) as json key instead of UUID
+  - with-uuid : yes | no
+      Whether to include uuid in output.
+  - with-meta : yes | no
+      Whether to always include meta in output (even for metadata).
+  - arrays : yes | no
+      Whether to write metadata dimensions and properties as arrays.
+  - as-data : yes | no (deprecated)
+  - meta : yes | no (deprecated)
+      Whether to format output as metadata. Alias for `with-uuid`
+  - compact : yes | no (deprecated)
+      Whether to write output in compact format. Alias for `as-data`
+  - useid: translate | require | keep (deprecated)
+      How to use the ID.
+ */
+DLiteStorage *json_open(const DLiteStoragePlugin *api, const char *uri,
+                        const char *options)
+{
+  return json_loader(api, uri, NULL, 0, options);
+}
+
+
 /**
   Closes data handle json. Returns non-zero on error.
  */
@@ -218,9 +238,13 @@ DLiteInstance *json_load(const DLiteStorage *s, const char *id)
   char uuid[DLITE_UUID_LENGTH+1];
   int uuidver;
 
-  if (!js->jstore)
-    FAILCODE1(dliteStorageLoadError,
-              "cannot load JSON file: \"%s\"", s->location);
+  if (!js->jstore) {
+    if (s->location)
+      FAILCODE1(dliteStorageLoadError,
+                "cannot load JSON file: \"%s\"", s->location);
+    else
+      FAILCODE(dliteStorageLoadError, "cannot load JSON buffer");
+  }
 
   if (!id || !*id) {
     JStoreIter iter;
@@ -286,6 +310,65 @@ int json_save(DLiteStorage *s, const DLiteInstance *inst)
 }
 
 
+/**
+  Load instance `id` from buffer `buf` of size `size`.
+  Returns NULL on error.
+ */
+DLiteInstance *json_memload(const DLiteStoragePlugin *api,
+                            const unsigned char *buf, size_t size,
+                            const char *id, const char *options)
+{
+  DLiteStorage *s = json_loader(api, NULL, buf, size, options);
+  DLiteInstance *inst = json_load(s, id);
+  json_close(s);
+  return inst;
+}
+
+
+/**
+  Save instance `inst` to buffer `buf` of size `size`.
+
+  Returns number of bytes written to `buf` (or would have been written
+  to `buf` if `buf` is not large enough).
+  Returns a negative error code on error.
+ */
+int json_memsave(const DLiteStoragePlugin *api,
+                 unsigned char *buf, size_t size,
+                 const DLiteInstance *inst, const char *options)
+{
+  int retval=-1, indent;
+  DLiteJsonFlag flags=0;
+  DLiteOpt opts[] = {
+    {'i', "indent",    "0",     "Indentation."},
+    {'s', "single",    "",      "Whether to write in single-entity format."},
+    {'k', "uri-key",   "false", "Whether to use uri as json key."},
+    {'u', "with-uuid", "false", "Whether to include uuid in output."},
+    {'M', "with-meta", "false", "Always include meta in output."},
+    {'a', "arrays",    "false", "Serialise metadata dims and props as arrays."},
+    {'n', "no-parent", "false", "Do not write transaction parent info."},
+    {'c', "compact",   "false", "Write relations with no newline."},
+    {0, NULL, NULL, NULL}
+  };
+  char *optcopy = (options) ? strdup(options) : NULL;
+  UNUSED(api);
+  if (dlite_option_parse(optcopy, opts, 1)) goto fail;
+  indent = atoi(opts[0].value);
+  if ((*opts[1].value) ? atob(opts[1].value) : dlite_instance_is_meta(inst))
+    flags |= dliteJsonSingle;
+  if (atob(opts[2].value)) flags |= dliteJsonUriKey;
+  if (atob(opts[3].value)) flags |= dliteJsonWithUuid;
+  if (atob(opts[4].value)) flags |= dliteJsonWithMeta;
+  if (atob(opts[5].value)) flags |= dliteJsonArrays;
+  if (atob(opts[6].value)) flags |= dliteJsonNoParent;
+  if (atob(opts[7].value)) flags |= dliteJsonCompactRel;
+  retval = dlite_json_sprint((char *)buf, size, inst, indent, flags);
+ fail:
+  if (optcopy) free(optcopy);
+  return retval;
+
+}
+
+
 /**
   Creates and returns a new iterator used by dlite_json_iter_next().
 
@@ -352,8 +435,8 @@ static DLiteStoragePlugin dlite_json_plugin = {
   NULL,                     /* deleteInstance */
 
   /* In-memory API */
-  NULL,                     /* memLoadInstance */
-  NULL,                     /* memSaveInstance */
+  json_memload,             /* memLoadInstance */
+  json_memsave,             /* memSaveInstance */
 
   /* === API to deprecate === */
   NULL,                     /* getUUIDs */


Test patch:
diff --git a/bindings/python/tests/test_storage.py b/bindings/python/tests/test_storage.py
index b387fc410..64451100b 100755
--- a/bindings/python/tests/test_storage.py
+++ b/bindings/python/tests/test_storage.py
@@ -57,6 +57,47 @@
 # Test query
 
 
+# Test from_bytes()
+mturk_bytes = '''{"9b256962-8c81-45f0-949c-c9d10b44050b": {
+  "meta": "http://onto-ns.com/meta/0.1/Person",
+  "dimensions": {"N": 2},
+  "properties": {
+    "name": "Mechanical Turk",
+    "age": 83,
+    "skills": ["chess", "cheating"]
+  }
+}}'''.encode()
+mturk = dlite.Instance.from_bytes("json", mturk_bytes)
+assert mturk.name == "Mechanical Turk"
+
+# ...also test with options even though they will have not effect
+mturk2 = dlite.Instance.from_bytes("json", mturk_bytes, options="mode=r")
+assert mturk2.name == "Mechanical Turk"
+
+
+
+
+# Test to_bytes()
+assert inst.to_bytes("json").decode() == str(inst)
+assert inst.to_bytes("json", options="").decode() == str(inst)
+s0 = inst.to_bytes("json", options="").decode()
+assert s0.startswith("{\n  \"ce592ff9-")
+s1 = inst.to_bytes("json", options="indent=2").decode()
+assert s1.startswith("  {\n    \"ce592ff9-")
+s2 = inst.to_bytes("json", options="single=true").decode()
+assert s2.startswith("{\n  \"uri\":")
+s3 = inst.to_bytes("json", options="uri-key=true").decode()
+assert s3.startswith("{\n  \"my-data\":")
+s4 = inst.to_bytes("json", options="single=true;with-uuid=true").decode()
+assert s4.startswith("{\n  \"uri\": \"my-data\",\n  \"uuid\":")
+
+# FIXME: Add test for the `arrays`, `no-parent` and `compact` options.
+# Should we rename `arrays` to `soft7` for consistency with the Python API?
+
+with raises(dlite.DLiteValueError):
+    inst.to_bytes("json", options="invalid-opt=").decode()
+
+
 # Test json
 print("--- testing json")
 myentity.save(f"json://{outdir}/test_storage_myentity.json?mode=w")
@@ -154,7 +195,7 @@
     #print(bytes(bytearr).decode())
 
 with raises(dlite.DLiteError):
-    inst.to_bytes("json")
+    inst.to_bytes("hdf5")
 
 
 s = dlite.Storage("json", f"{indir}/persons.json", "mode=r")
diff --git a/src/tests/test_json.c b/src/tests/test_json.c
index cc82aeb14..e0b7a6f8a 100644
--- a/src/tests/test_json.c
+++ b/src/tests/test_json.c
@@ -73,13 +73,13 @@ MU_TEST(test_sprint)
                         dliteJsonArrays | dliteJsonSingle);
   //printf("\n--------------------------------------------------------\n");
   //printf("%s\n", buf);
-  mu_assert_int_eq(1011, m); 
+  mu_assert_int_eq(1011, m);
 
   m = dlite_json_sprint(buf, sizeof(buf), (DLiteInstance *)meta, 2,
                         dliteJsonWithUuid | dliteJsonArrays | dliteJsonSingle);
   //printf("\n--------------------------------------------------------\n");
   //printf("%s\n", buf);
-  mu_assert_int_eq(1165, m); 
+  mu_assert_int_eq(1165, m);
 
   //printf("\n========================================================\n");
 
diff --git a/src/tests/test_misc.c b/src/tests/test_misc.c
index 0bd0c121d..de7ad95a5 100644
--- a/src/tests/test_misc.c
+++ b/src/tests/test_misc.c
@@ -118,7 +118,8 @@ MU_TEST(test_option_parse)
   free(options);
 
   old = err_set_stream(NULL);
-  mu_assert_int_eq(1, dlite_option_parse("name=C;mode=append", opts, 0));
+  mu_assert_int_eq(dliteValueError,
+                   dlite_option_parse("name=C;mode=append", opts, 0));
   err_set_stream(old);
 }