From 343848f9fff53b4a3a7e763c63ea18170b6b670e Mon Sep 17 00:00:00 2001 From: Jesper Friis Date: Sun, 7 Jul 2024 11:32:51 +0200 Subject: [PATCH] Added support for `to_bytes()` and `from_bytes()` in the json storage plugin (#881) Added support for to_bytes() and from_bytes() in the json storage plugin. Other changes: * Add more throughout testing of to_bytes() * Fixed a bug in the to_bytes() method - now it also returns the last character * Invalid storage plugin option now result in DLiteValueError (should we have a dedicated error for that?) This PR open for simplifying the implementation of asdict() method and the dlite.utils.instance_from_dict() function. They can now be implemented with to_bytes() and from_bytes() together with json.loads() and json.dumps(), respectively. --------- Co-authored-by: Francesca L. Bleken <48128015+francescalb@users.noreply.github.com> --- bindings/python/dlite-entity.i | 4 +- bindings/python/tests/test_storage.py | 43 ++++++- src/dlite-misc.c | 2 +- src/tests/test_json.c | 4 +- src/tests/test_misc.c | 3 +- src/utils/jstore.c | 26 ++-- src/utils/jstore.h | 6 +- storages/json/dlite-json-storage.c | 163 +++++++++++++++++++------- 8 files changed, 194 insertions(+), 57 deletions(-) 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/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/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/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); } 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 */