From 59dc0505a4518a7625c916796864413dae1909b5 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 29 Dec 2015 10:55:31 -0800 Subject: [PATCH 01/27] libkvs/treeobj: add internal RFC 11 treeobj lib Add class for manipulating json objects that conform to RFC 11. These are internal functions for now. --- src/common/libkvs/Makefile.am | 4 +- src/common/libkvs/treeobj.c | 450 ++++++++++++++++++++++++++++++++++ src/common/libkvs/treeobj.h | 98 ++++++++ 3 files changed, 551 insertions(+), 1 deletion(-) create mode 100644 src/common/libkvs/treeobj.c create mode 100644 src/common/libkvs/treeobj.h diff --git a/src/common/libkvs/Makefile.am b/src/common/libkvs/Makefile.am index a3dcfb8a92b7..7cfba3ab8d03 100644 --- a/src/common/libkvs/Makefile.am +++ b/src/common/libkvs/Makefile.am @@ -21,7 +21,9 @@ libkvs_la_SOURCES = \ jansson_dirent.h \ kvs_commit.c \ kvs_txn.c \ - kvs_txn_private.h + kvs_txn_private.h \ + treeobj.h \ + treeobj.c fluxcoreinclude_HEADERS = \ kvs.h \ diff --git a/src/common/libkvs/treeobj.c b/src/common/libkvs/treeobj.c new file mode 100644 index 000000000000..8a01db6daeb0 --- /dev/null +++ b/src/common/libkvs/treeobj.c @@ -0,0 +1,450 @@ +/*****************************************************************************\ + * Copyright (c) 2016 Lawrence Livermore National Security, LLC. Produced at + * the Lawrence Livermore National Laboratory (cf, AUTHORS, DISCLAIMER.LLNS). + * LLNL-CODE-658032 All rights reserved. + * + * This file is part of the Flux resource manager framework. + * For details, see https://github.com/flux-framework. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the license, or (at your option) + * any later version. + * + * Flux is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the IMPLIED WARRANTY OF MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the terms and conditions of the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. + * See also: http://www.gnu.org/licenses/ +\*****************************************************************************/ + +/* See RFC 11 */ + +#if HAVE_CONFIG_H +#include "config.h" +#endif +#include +#include +#include + +#include "treeobj.h" +#include "src/common/libutil/blobref.h" +#include "src/common/libutil/base64.h" + +static const int treeobj_version = 1; + +static int treeobj_unpack (json_t *obj, const char **typep, json_t **datap) +{ + json_t *data; + int version; + const char *type; + if (!obj || json_unpack (obj, "{s:i s:s s:o !}", + "ver", &version, + "type", &type, + "data", &data) < 0 + || version != treeobj_version) { + errno = EINVAL; + return -1; + } + if (typep) + *typep = type; + if (datap) + *datap = data; + return 0; +} + +int treeobj_validate (json_t *obj) +{ + json_t *o, *data; + const char *type; + + if (treeobj_unpack (obj, &type, &data) < 0) + goto inval; + if (!strcmp (type, "valref") || !strcmp (type, "dirref")) { + int i, len; + if (!json_is_array (data)) + goto inval; + len = json_array_size (data); + if (len == 0) + goto inval; + json_array_foreach (data, i, o) { + if (blobref_validate (json_string_value (o)) < 0) + goto inval; + } + } + else if (!strcmp (type, "dir")) { + const char *key; + if (!json_is_object (data)) + goto inval; + json_object_foreach (data, key, o) { + if (treeobj_validate (o) < 0) + goto inval; + } + } + else if (!strcmp (type, "symlink")) { + if (!json_is_string (data)) + goto inval; + } + else if (!strcmp (type, "val")) { + /* is base64, should always be a string */ + if (!json_is_string (data)) + goto inval; + } + else + goto inval; + return 0; +inval: + errno = EINVAL; + return -1; +} + +const char *treeobj_get_type (json_t *obj) +{ + const char *type; + if (treeobj_unpack (obj, &type, NULL) < 0) + return NULL; + return type; +} + +bool treeobj_is_symlink (json_t *obj) +{ + const char *type = treeobj_get_type (obj); + return type && !strcmp (type, "symlink"); +} + +bool treeobj_is_val (json_t *obj) +{ + const char *type = treeobj_get_type (obj); + return type && !strcmp (type, "val"); +} + +bool treeobj_is_valref (json_t *obj) +{ + const char *type = treeobj_get_type (obj); + return type && !strcmp (type, "valref"); +} + +bool treeobj_is_dir (json_t *obj) +{ + const char *type = treeobj_get_type (obj); + return type && !strcmp (type, "dir"); +} + +bool treeobj_is_dirref (json_t *obj) +{ + const char *type = treeobj_get_type (obj); + return type && !strcmp (type, "dirref"); +} + +json_t *treeobj_get_data (json_t *obj) +{ + json_t *data; + if (treeobj_unpack (obj, NULL, &data) < 0) + return NULL; + return data; +} + +int treeobj_decode_val (json_t *obj, void **dp, int *lp) +{ + const char *type, *xdatastr; + json_t *xdata; + int len, xlen; + char *data; + + if (treeobj_unpack (obj, &type, &xdata) < 0 + || strcmp (type, "val") != 0) { + errno = EINVAL; + return -1; + } + xdatastr = json_string_value (xdata); + xlen = strlen (xdatastr); + len = base64_decode_length (xlen); + if (len > 1) { + if (!(data = malloc (len))) { + errno = ENOMEM; + return -1; + } + if (base64_decode_block (data, &len, xdatastr, xlen) < 0) { + free (data); + errno = EINVAL; + return -1; + } + } + else { + len = 0; + data = NULL; + } + if (lp) + *lp = len; + if (dp) + *dp = data; + else + free (data); + return 0; +} + +int treeobj_get_count (json_t *obj) +{ + json_t *data; + const char *type; + int count = -1; + + if (treeobj_unpack (obj, &type, &data) < 0) + goto done; + if (!strcmp (type, "valref") || !strcmp (type, "dirref")) { + count = json_array_size (data); + } + else if (!strcmp (type, "dir")) { + count = json_object_size (data); + } + else if (!strcmp (type, "symlink") || !strcmp (type, "val")) { + count = 1; + } else { + errno = EINVAL; // unknown type + return -1; + } +done: + return count; +} + +json_t *treeobj_get_entry (json_t *obj, const char *name) +{ + const char *type; + json_t *data, *obj2; + + if (treeobj_unpack (obj, &type, &data) < 0 + || strcmp (type, "dir") != 0) { + errno = EINVAL; + return NULL; + } + if (!(obj2 = json_object_get (data, name))) { + errno = ENOENT; + return NULL; + } + return obj2; +} + +int treeobj_delete_entry (json_t *obj, const char *name) +{ + const char *type; + json_t *data; + + if (treeobj_unpack (obj, &type, &data) < 0 + || strcmp (type, "dir") != 0) { + errno = EINVAL; + return -1; + } + if (json_object_del (data, name) < 0) { + errno = ENOENT; + return -1; + } + return 0; +} + +int treeobj_insert_entry (json_t *obj, const char *name, json_t *obj2) +{ + const char *type; + json_t *data; + + if (!name || !obj2 || treeobj_unpack (obj, &type, &data) < 0 + || strcmp (type, "dir") != 0 + || treeobj_validate (obj2) < 0) { + errno = EINVAL; + return -1; + } + if (json_object_set (data, name, obj2) < 0) { + errno = EINVAL; + return -1; + } + return 0; +} + +int treeobj_append_blobref (json_t *obj, const char *blobref) +{ + const char *type; + json_t *data, *o; + int rc = -1; + + if (!blobref || blobref_validate (blobref) < 0 + || treeobj_unpack (obj, &type, &data) < 0 + || (strcmp (type, "dirref") != 0 + && strcmp (type, "valref") != 0)) { + errno = EINVAL; + goto done; + } + if (!(o = json_string (blobref))) { + errno = ENOMEM; + goto done; + } + if (json_array_append_new (data, o) < 0) { + json_decref (o); + errno = ENOMEM; + goto done; + } + rc = 0; +done: + return rc; +} + +const char *treeobj_get_blobref (json_t *obj, int index) +{ + json_t *data, *o; + const char *type, *blobref = NULL; + + if (treeobj_unpack (obj, &type, &data) < 0 + || (strcmp (type, "dirref") != 0 + && strcmp (type, "valref") != 0)) { + errno = EINVAL; + goto done; + } + if (!(o = json_array_get (data, index))) { + errno = EINVAL; + goto done; + } + blobref = json_string_value (o); +done: + return blobref; +} + +json_t *treeobj_create_dir (void) +{ + json_t *obj; + + if (!(obj = json_pack ("{s:i s:s s:{}}", "ver", treeobj_version, + "type", "dir", + "data"))) { + errno = ENOMEM; + return NULL; + } + return obj; +} + +json_t *treeobj_create_symlink (const char *target) +{ + json_t *obj; + + if (!(obj = json_pack ("{s:i s:s s:s}", "ver", treeobj_version, + "type", "symlink", + "data", target))) { + errno = ENOMEM; + return NULL; + } + return obj; +} + +json_t *treeobj_create_val (const void *data, int len) +{ + int xlen; + char *xdata; + json_t *obj = NULL; + + xlen = base64_encode_length (len); + if (!(xdata = malloc (xlen))) { + errno = ENOMEM; + goto done; + } + base64_encode_block (xdata, &xlen, data, len); + if (!(obj = json_pack ("{s:i s:s s:s}", "ver", treeobj_version, + "type", "val", + "data", xdata))) { + errno = ENOMEM; + goto done; + } +done: + free (xdata); + return obj; +} + +json_t *treeobj_create_valref (const char *blobref) +{ + json_t *obj; + + if (blobref) + obj = json_pack ("{s:i s:s s:[s]}", "ver", treeobj_version, + "type", "valref", + "data", blobref); + else + obj = json_pack ("{s:i s:s s:[]}", "ver", treeobj_version, + "type", "valref", + "data"); + if (!obj) { + errno = ENOMEM; + return NULL; + } + return obj; +} + +json_t *treeobj_create_dirref (const char *blobref) +{ + json_t *obj; + + if (blobref) + obj = json_pack ("{s:i s:s s:[s]}", "ver", treeobj_version, + "type", "dirref", + "data", blobref); + else + obj = json_pack ("{s:i s:s s:[]}", "ver", treeobj_version, + "type", "dirref", + "data"); + if (!obj) { + errno = ENOMEM; + return NULL; + } + return obj; +} + +json_t *treeobj_create_valref_buf (const char *hashtype, int maxblob, + void *data, int len) +{ + json_t *valref = NULL; + char blobref[BLOBREF_MAX_STRING_SIZE]; + int blob_len; + + if (!(valref = treeobj_create_valref (NULL))) + goto error; + while (len >= 0) { // N.B. handle zero-length blob + blob_len = len; + if (maxblob > 0 && len > maxblob) + blob_len = maxblob; + if (blobref_hash (hashtype, data, blob_len, + blobref, sizeof (blobref)) < 0) + goto error; + if (treeobj_append_blobref (valref, blobref) < 0) + goto error; + len -= blob_len; + data += blob_len; + if (len == 0) + break; + } + return valref; +error: + json_decref (valref); + return NULL; +} + +json_t *treeobj_decode (const char *buf) +{ + json_t *obj = NULL; + if (!(obj = json_loads (buf, 0, NULL)) + || treeobj_validate (obj) < 0) { + errno = EPROTO; + goto error; + } + return obj; +error: + json_decref (obj); + return NULL; +} + +char *treeobj_encode (json_t *obj) +{ + return json_dumps (obj, JSON_COMPACT|JSON_SORT_KEYS); +} + +/* + * vi:tabstop=4 shiftwidth=4 expandtab + */ diff --git a/src/common/libkvs/treeobj.h b/src/common/libkvs/treeobj.h new file mode 100644 index 000000000000..e1bba86a1d58 --- /dev/null +++ b/src/common/libkvs/treeobj.h @@ -0,0 +1,98 @@ +#ifndef _FLUX_KVS_TREEOBJ_H +#define _FLUX_KVS_TREEOBJ_H + +#include +#include + +/* See RFC 11 */ + +/* Create a treeobj + * valref, dirref: if blobref is NULL, treeobj_append_blobref() + * must be called before object is valid. + * val: copies argument (caller retains ownership) + * Return JSON object on success, NULL on failure with errno set. + */ +json_t *treeobj_create_symlink (const char *target); +json_t *treeobj_create_val (const void *data, int len); +json_t *treeobj_create_valref (const char *blobref); +json_t *treeobj_create_dir (void); +json_t *treeobj_create_dirref (const char *blobref); + +/* Validate treeobj, recursively. + * Return 0 if valid, -1 with errno = EINVAL if invalid. + */ +int treeobj_validate (json_t *obj); + +/* get type (RFC 11 defined strings or NULL on error with errno set). + */ +const char *treeobj_get_type (json_t *obj); + +/* Test for a particular type + */ +bool treeobj_is_symlink (json_t *obj); +bool treeobj_is_val (json_t *obj); +bool treeobj_is_valref (json_t *obj); +bool treeobj_is_dir (json_t *obj); +bool treeobj_is_dirref (json_t *obj); + +/* get type-specific value. + * For dirref/valref, this is an array of blobrefs. + * For directory, this is dictionary of treeobjs + * For symlink, this is a string. + * For val this is string containing base64-encoded data. + * Return JSON object on success, NULL on error with errno = EINVAL. + * The returned object is owned by 'obj' and must not be destroyed. + */ +json_t *treeobj_get_data (json_t *obj); + +/* get decoded val data. + * Caller must free returned data. + */ +int treeobj_decode_val (json_t *obj, void **data, int *len); + +/* get type-specific count. + * For dirref/valref, this is the number of blobrefs. + * For directory, this is number of entries + * For symlink or val, this is 1. + * Return count on success, -1 on error with errno = EINVAL. + */ +int treeobj_get_count (json_t *obj); + +/* get/add/remove directory entry + * Get returns JSON object (owned by 'obj', do not destory), NULL on error. + * insert takes a reference on 'obj2' (caller retains ownership). + * insert/delete return 0 on success, -1 on error with errno set. + */ +json_t *treeobj_get_entry (json_t *obj, const char *name); +int treeobj_insert_entry (json_t *obj, const char *name, json_t *obj2); +int treeobj_delete_entry (json_t *obj, const char *name); + +/* add blobref to dirref,valref object. + * Return 0 on success, -1 on failure with errno set. + */ +int treeobj_append_blobref (json_t *obj, const char *blobref); + +/* get blobref entry at 'index'. + * Return blobref on success, NULL on failure with errno set. + */ +const char *treeobj_get_blobref (json_t *obj, int index); + +/* Create valref that refers to 'data', a blob of 'len' bytes using + * 'hashtype' hash algorithm (e.g. "sha1"). If 'maxblob' > 0, split the + * blob into maxblob size chunks. + */ +json_t *treeobj_create_valref_buf (const char *hashtype, int maxblob, + void *data, int len); + +/* Convert a treeobj to/from string. + * The return value of treeobj_decode must be destroyed with json_decref(). + * The return value of treeobj_encode must be destroyed with free(). + */ +json_t *treeobj_decode (const char *buf); +char *treeobj_encode (json_t *obj); + +#endif /* !_FLUX_KVS_TREEOBJ_H */ + +/* + * vi:tabstop=4 shiftwidth=4 expandtab + */ From 553deac8f862ef997e45833d9a1f680bf5ea3034 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 29 Dec 2015 11:13:51 -0800 Subject: [PATCH 02/27] test/treeobj: add unit tests for libkvs/treeobj --- src/common/libkvs/Makefile.am | 7 +- src/common/libkvs/test/treeobj.c | 464 +++++++++++++++++++++++++++++++ 2 files changed, 470 insertions(+), 1 deletion(-) create mode 100644 src/common/libkvs/test/treeobj.c diff --git a/src/common/libkvs/Makefile.am b/src/common/libkvs/Makefile.am index 7cfba3ab8d03..7150a5ef6b70 100644 --- a/src/common/libkvs/Makefile.am +++ b/src/common/libkvs/Makefile.am @@ -38,7 +38,8 @@ TESTS = \ test_jansson_dirent.t \ test_kvs_txn.t \ test_kvs_lookup.t \ - test_kvs_dir.t + test_kvs_dir.t \ + test_treeobj.t check_PROGRAMS = \ $(TESTS) @@ -73,3 +74,7 @@ test_kvs_lookup_t_LDADD = $(test_ldadd) $(LIBDL) test_kvs_dir_t_SOURCES = test/kvs_dir.c test_kvs_dir_t_CPPFLAGS = $(test_cppflags) test_kvs_dir_t_LDADD = $(test_ldadd) $(LIBDL) + +test_treeobj_t_SOURCES = test/treeobj.c +test_treeobj_t_CPPFLAGS = $(test_cppflags) +test_treeobj_t_LDADD = $(test_ldadd) $(LIBDL) diff --git a/src/common/libkvs/test/treeobj.c b/src/common/libkvs/test/treeobj.c new file mode 100644 index 000000000000..3c135e5a7e8e --- /dev/null +++ b/src/common/libkvs/test/treeobj.c @@ -0,0 +1,464 @@ +#include +#include + +#include "src/common/libkvs/treeobj.h" + +#include "src/common/libtap/tap.h" +#include "src/common/libutil/xzmalloc.h" +#include "src/common/libutil/sha1.h" +#include "src/common/libutil/blobref.h" + +const int large_dir_entries = 5000; +json_t *create_large_dir (void) +{ + int i; + char name[256]; + json_t *ent, *dir = treeobj_create_dir (); + if (!dir) + return NULL; + for (i = 0; i < large_dir_entries; i++) { + snprintf (name, sizeof (name), "entry-%.10d", i); + if (!(ent = treeobj_create_symlink ("a.b.c.d")) + || treeobj_insert_entry (dir, name, ent) < 0) { + json_decref (dir); + return NULL; + } + } + return dir; +} + +void diag_json (json_t *o) +{ + char *s = json_dumps (o, JSON_INDENT(4)); + diag ("%s", s ? s : "nil"); + free (s); +} + +void test_codec (void) +{ + json_t *cpy, *dir = create_large_dir (); + char *s, *p; + + if (!dir) + BAIL_OUT ("could not create %d-entry dir", large_dir_entries); + s = treeobj_encode (dir); + ok (s != NULL, + "encoded %d-entry dir", large_dir_entries); + if (!s) + BAIL_OUT ("could not encode %d-entry dir", large_dir_entries); + ok ((cpy = treeobj_decode (s)) != NULL, + "decoded %d-entry dir", large_dir_entries); + if (!cpy) + diag ("%m"); + if (!cpy) + BAIL_OUT ("could not continue"); + p = treeobj_encode (cpy); + ok (p != NULL, + "re-encoded %d-entry dir", large_dir_entries); + ok (strcmp (p, s) == 0, + "and they match"); + free (p); + + free (s); + json_decref (cpy); + json_decref (dir); +} + +const char *blobrefs[] = { + "sha1-508259c0f7fd50e47716b50ad1f0fc6ed46017f9", + "sha1-ded5ba42480fe75dcebba1ce068489ff7be2186a", + "sha1-da39a3ee5e6b4b0d3255bfef95601890afd80709", +}; + +void test_valref (void) +{ + json_t *valref; + const char *blobref; + json_t *val; + + ok ((valref = treeobj_create_valref (NULL)) != NULL, + "treeobj_create_valref with no blobrefs works"); + errno = 0; + ok (treeobj_validate (valref) < 0 && errno == EINVAL, + "treeobj_validate rejects valref with no blobrefs"); + ok (treeobj_is_valref (valref), + "treeobj_is_valref returns true"); + ok ((val = treeobj_get_data (valref)) != NULL && json_is_array (val), + "treeobj_get_data returns JSON_ARRAY type"); + errno = 0; + ok (treeobj_get_blobref (valref, 0) == NULL && errno == EINVAL, + "treeobj_get_blobref [0] fails with EINVAL"); + ok (treeobj_append_blobref (valref, "foo") < 0 && errno == EINVAL, + "treeobj_append_blobref returns EINVAL on bad blobref"); + ok (treeobj_append_blobref (valref, blobrefs[0]) == 0, + "treeobj_append_blobref works"); + ok (treeobj_validate (valref) == 0, + "treeobj_validate likes valref now"); + ok (treeobj_get_count (valref) == 1, + "treeobj_get_count returns 1"); + blobref = treeobj_get_blobref (valref, 0); + ok (blobref != NULL && !strcmp (blobref, blobrefs[0]), + "treeobj_get_blobref [0] returns expected blobref"); + ok (treeobj_append_blobref (valref, blobrefs[1]) == 0, + "treeobj_append_blobref works on 2nd blobref"); + ok (treeobj_get_count (valref) == 2, + "treeobj_get_count returns 1"); + blobref = treeobj_get_blobref (valref, 0); + ok (blobref != NULL && !strcmp (blobref, blobrefs[0]), + "treeobj_get_blobref [0] still returns expected blobref"); + blobref = treeobj_get_blobref (valref, 1); + ok (blobref != NULL && !strcmp (blobref, blobrefs[1]), + "treeobj_get_blobref [1] returns expected blobref"); + diag_json (valref); + json_decref (valref); + + ok ((valref = treeobj_create_valref (blobrefs[0])) != NULL, + "treeobj_create_valref works with blobref arg"); + ok (treeobj_validate (valref) == 0, + "treeobj_validate likes valref"); + ok (treeobj_get_count (valref) == 1, + "treeobj_get_count returns 1"); + blobref = treeobj_get_blobref (valref, 0); + ok (blobref != NULL && !strcmp (blobref, blobrefs[0]), + "treeobj_get_blobref [0] returns expected blobref"); + diag_json (valref); + json_decref (valref); + + char buf[1024]; + const char *blobref2; + int i; + memset (buf, 'L', sizeof (buf)); + ok ((valref = treeobj_create_valref_buf ("sha1", 256, buf, sizeof (buf))) != NULL, + "treeobj_create_valref_buf works on 1024 byte blob"); + diag_json (valref); + ok (treeobj_get_count (valref) == 4, + "and maxblob 256 split blob into 4 blobrefs"); + blobref = treeobj_get_blobref (valref, 0); + for (i = 1; i < 4; i++) { + blobref2 = treeobj_get_blobref (valref, i); + if (!blobref || !blobref2 || strcmp (blobref, blobref2) != 0) + break; + } + ok (i == 4, + "and the four blobrefs are identical"); + json_decref (valref); + + ok ((valref = treeobj_create_valref_buf ("sha256", 0, NULL, 0)) != NULL, + "treeobj_create_valref_buf works on empty buf"); + diag_json (valref); + ok (treeobj_get_count (valref) == 1, + "and valref contains one blobref"); + json_decref (valref); +} + +void test_val (void) +{ + json_t *val, *val2; + char buf[32]; + char *outbuf; + int outlen; + + memset (buf, 'x', sizeof (buf)); + + ok ((val = treeobj_create_val (buf, sizeof (buf))) != NULL, + "treeobj_create_val works"); + diag_json (val); + ok (treeobj_is_val (val), + "treeobj_is_value returns true"); + ok (treeobj_get_count (val) == 1, + "treeobj_get_count returns 1"); + ok (treeobj_decode_val (val, (void **)&outbuf, &outlen) == 0, + "treeobj_decode_val works"); + ok (outlen == sizeof (buf), + "and returned size same as input"); + ok (memcmp (buf, outbuf, outlen) == 0, + "and returned data same as input"); + free (outbuf); + ok (treeobj_decode_val (val, (void **)&outbuf, NULL) == 0, + "treeobj_decode_val works w/o len input"); + free (outbuf); + ok (treeobj_decode_val (val, NULL, &outlen) == 0, + "treeobj_decode_val works w/o data pointer input"); + ok (outlen == sizeof (buf), + "and returned size same as input"); + + ok ((val2 = treeobj_create_val (NULL, 0)) != NULL, + "treeobj_create_val NULL, 0 works"); + diag_json (val2); + ok (treeobj_decode_val (val2, (void **)&outbuf, &outlen) == 0, + "treeobj_decode_val works"); + ok (outlen == 0, + "and returned size = 0"); + ok (outbuf == NULL, + "and returned data = NULL"); + + json_decref (val); + json_decref (val2); +} + +void test_dirref (void) +{ + json_t *dirref; + const char *blobref; + json_t *val; + + ok ((dirref = treeobj_create_dirref (NULL)) != NULL, + "treeobj_create_dirref with no blobrefs works"); + errno = 0; + ok (treeobj_validate (dirref) < 0 && errno == EINVAL, + "treeobj_validate rejects dirref with no blobrefs"); + ok (treeobj_is_dirref (dirref), + "treeobj_is_dirref returns true"); + ok ((val = treeobj_get_data (dirref)) != NULL && json_is_array (val), + "treeobj_get_data returns JSON_ARRAY type"); + errno = 0; + ok (treeobj_get_blobref (dirref, 0) == NULL && errno == EINVAL, + "treeobj_get_blobref [0] fails with EINVAL"); + ok (treeobj_append_blobref (dirref, "foo") < 0 && errno == EINVAL, + "treeobj_append_blobref returns EINVAL on bad blobref"); + ok (treeobj_append_blobref (dirref, blobrefs[0]) == 0, + "treeobj_append_blobref works"); + ok (treeobj_validate (dirref) == 0, + "treeobj_validate likes dirref now"); + ok (treeobj_get_count (dirref) == 1, + "treeobj_get_count returns 1"); + blobref = treeobj_get_blobref (dirref, 0); + ok (blobref != NULL && !strcmp (blobref, blobrefs[0]), + "treeobj_get_blobref [0] returns expected blobref"); + ok (treeobj_append_blobref (dirref, blobrefs[1]) == 0, + "treeobj_append_blobref works on 2nd blobref"); + ok (treeobj_get_count (dirref) == 2, + "treeobj_get_count returns 1"); + blobref = treeobj_get_blobref (dirref, 0); + ok (blobref != NULL && !strcmp (blobref, blobrefs[0]), + "treeobj_get_blobref [0] still returns expected blobref"); + blobref = treeobj_get_blobref (dirref, 1); + ok (blobref != NULL && !strcmp (blobref, blobrefs[1]), + "treeobj_get_blobref [1] returns expected blobref"); + diag_json (dirref); + json_decref (dirref); + + ok ((dirref = treeobj_create_dirref (blobrefs[0])) != NULL, + "treeobj_create_dirref works with blobref arg"); + ok (treeobj_validate (dirref) == 0, + "treeobj_validate likes dirref"); + ok (treeobj_get_count (dirref) == 1, + "treeobj_get_count returns 1"); + blobref = treeobj_get_blobref (dirref, 0); + ok (blobref != NULL && !strcmp (blobref, blobrefs[0]), + "treeobj_get_blobref [0] returns expected blobref"); + + diag_json (dirref); + json_decref (dirref); +} + +void test_dir (void) +{ + json_t *dir; + json_t *str = NULL, *val1 = NULL; + json_t *i = NULL, *val2 = NULL; + json_t *nil = NULL, *val3 = NULL; + json_t *val; + + /* create couple test values */ + val1 = treeobj_create_val ("foo", 4); + val2 = treeobj_create_val ("42", 3); + val3 = treeobj_create_val (NULL, 0); + if (!val1 || !val2 || !val3) + BAIL_OUT ("can't continue without test values"); + + ok ((dir = treeobj_create_dir ()) != NULL, + "treeobj_create_dir works"); + ok (treeobj_validate (dir) == 0, + "treeobj_validate likes empty dir"); + ok (treeobj_is_dir (dir), + "treeobj_is_dir returns true"); + ok ((val = treeobj_get_data (dir)) != NULL && json_is_object (val), + "treeobj_get_data returns JSON_OBJECT type"); + + ok (treeobj_get_count (dir) == 0, + "treeobj_get_count returns 0"); + ok (treeobj_insert_entry (dir, "foo", val1) == 0 + && treeobj_get_count (dir) == 1 + && treeobj_get_entry (dir, "foo") == val1, + "treeobj_insert_entry works"); + ok (treeobj_insert_entry (dir, "bar", val1) == 0 + && treeobj_get_count (dir) == 2 + && treeobj_get_entry (dir, "bar") == val1, + "treeobj_insert_entry same value different key works"); + ok (treeobj_insert_entry (dir, "bar", val2) == 0 + && treeobj_get_count (dir) == 2 + && treeobj_get_entry (dir, "foo") == val1 + && treeobj_get_entry (dir, "bar") == val2, + "treeobj_insert_entry same key replaces entry"); + ok (treeobj_delete_entry (dir, "bar") == 0 + && treeobj_get_count (dir) == 1, + "treeobj_delete_entry works"); + ok (treeobj_insert_entry (dir, "nil", val3) == 0 + && treeobj_get_count (dir) == 2 + && treeobj_get_entry (dir, "nil") == val3, + "treeobj_insert_entry accepts json_null value"); + ok (treeobj_validate (dir) == 0, + "treeobj_validate likes populated dir"); + + errno = 0; + ok (treeobj_get_entry (val1, "foo") == NULL && errno == EINVAL, + "treeobj_get_entry fails with EINVAL on non-dir treeobj"); + errno = 0; + ok (treeobj_delete_entry (val1, "foo") < 0 && errno == EINVAL, + "treeobj_delete_entry fails with EINVAL on non-dir treeobj"); + errno = 0; + ok (treeobj_insert_entry (val1, "foo", val1) < 0 && errno == EINVAL, + "treeobj_insert_entry fails with EINVAL on non-dir treeobj"); + errno = 0; + ok (treeobj_insert_entry (dir, NULL, val1) < 0 && errno == EINVAL, + "treeobj_insert_entry fails with EINVAL on NULL key"); + errno = 0; + ok (treeobj_insert_entry (dir, "baz", NULL) < 0 && errno == EINVAL, + "treeobj_insert_entry fails with EINVAL on NULL value"); + errno = 0; + ok (treeobj_get_entry (dir, "noexist") == NULL && errno == ENOENT, + "treeobj_get_entry fails with ENOENT on unknown key"); + errno = 0; + ok (treeobj_delete_entry (dir, "noexist") < 0 && errno == ENOENT, + "treeobj_delete_entry fails with ENOENT on unknown key"); + + json_decref (str); + json_decref (val1); + + json_decref (i); + json_decref (val2); + + json_decref (nil); + json_decref (val3); + + diag_json (dir); + json_decref (dir); +} + +void test_symlink (void) +{ + json_t *o, *data; + + o = treeobj_create_symlink ("a.b.c"); + ok (o != NULL, + "treeobj_create_symlink works"); + diag_json (o); + ok (treeobj_is_symlink (o), + "treeobj_is_symlink returns true"); + ok ((data = treeobj_get_data (o)) != NULL && json_is_string (data), + "treobj_get_data returned string"); + ok (!strcmp (json_string_value (data), "a.b.c"), + "and string has right content"); + json_decref (o); +} + +void test_corner_cases (void) +{ + json_t *val, *valref, *dir, *symlink; + json_t *array, *object; + char *outbuf; + int outlen; + + val = treeobj_create_val ("a", 1); + if (!val) + BAIL_OUT ("can't continue without test value"); + + ok (treeobj_append_blobref (val, blobrefs[0]) < 0 && errno == EINVAL, + "treeobj_append_blobref returns EINVAL on bad treeobj"); + + ok (treeobj_get_blobref (val, 0) == NULL && errno == EINVAL, + "treeobj_get_blobref returns EINVAL on bad treeobj"); + + /* Modify val to have bad type */ + json_object_set_new (val, "type", json_string ("foo")); + + ok (treeobj_validate (val) < 0 && errno == EINVAL, + "treeobj_validate detects invalid type"); + + ok (treeobj_get_count (val) < 0 && errno == EINVAL, + "treeobj_get_count detects invalid type"); + + ok (treeobj_decode (treeobj_encode (val)) == NULL && errno == EPROTO, + "treeobj_decode returns EPROTO on bad treeobj"); + + json_decref (val); + + valref = treeobj_create_valref (NULL); + if (!valref) + BAIL_OUT ("can't continue without test value"); + + ok (treeobj_validate (valref) < 0 && errno == EINVAL, + "treeobj_validate detects no valid blobref"); + + /* Modify valref to have bad blobref */ + array = json_array (); + json_array_set_new (array, 0, json_string ("sha1-foo")); + json_object_set_new (valref, "data", array); + + ok (treeobj_validate (valref) < 0 && errno == EINVAL, + "treeobj_validate detects bad ref in valref"); + + json_object_set_new (valref, "data", json_string ("not-array")); + + ok (treeobj_validate (valref) < 0 && errno == EINVAL, + "treeobj_validate detects bad data in valref"); + + json_decref (valref); + + dir = treeobj_create_dir (); + if (!dir) + BAIL_OUT ("can't continue without test value"); + + ok (treeobj_decode_val (dir, (void **)&outbuf, &outlen) < 0 + && errno == EINVAL, + "treeobj_decode_val returns EINVAL on non-val treeobj"); + + /* Modify valref to have bad blobref */ + object = json_object (); + json_object_set_new (object, "a", json_string ("foo")); + json_object_set_new (dir, "data", object); + + ok (treeobj_validate (dir) < 0 && errno == EINVAL, + "treeobj_validate detects bad treeobj in dir"); + + /* Modify dir to have bad data */ + json_object_set_new (dir, "data", json_integer (42)); + + ok (treeobj_validate (dir) < 0 && errno == EINVAL, + "treeobj_validate detects bad data in dir"); + + json_decref (dir); + + symlink = treeobj_create_symlink ("some-string"); + if (!symlink) + BAIL_OUT ("can't continue without test value"); + + /* Modify symlink to have bad data */ + json_object_set_new (symlink, "data", json_integer (42)); + + ok (treeobj_validate (symlink) < 0 && errno == EINVAL, + "treeobj_validate detects bad data in symlink"); + + json_decref (symlink); +} + +int main(int argc, char** argv) +{ + plan (NO_PLAN); + + test_valref (); + test_val (); + test_dirref (); + test_dir (); + test_symlink (); + test_corner_cases (); + + test_codec (); + + done_testing(); +} + +/* + * vi:tabstop=4 shiftwidth=4 expandtab + */ From c810bd5bfaf21b47dd0a1884ff1729ed735a81ee Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 3 Aug 2017 09:26:14 -0700 Subject: [PATCH 03/27] libkvs/txn: switch to use RFC 11 metadata --- src/common/libkvs/kvs_txn.c | 82 ++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 32 deletions(-) diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c index 56b3effb7bd4..763bc32c7de2 100644 --- a/src/common/libkvs/kvs_txn.c +++ b/src/common/libkvs/kvs_txn.c @@ -30,7 +30,7 @@ #include #include "kvs_txn_private.h" -#include "jansson_dirent.h" +#include "treeobj.h" struct flux_kvs_txn { json_t *ops; @@ -85,7 +85,7 @@ static int validate_op (json_t *op) if (json_unpack (op, "{s:n}", "dirent") == 0) ; // unlink sets dirent NULL else if (json_unpack (op, "{s:o}", "dirent", &dirent) == 0) { - if (j_dirent_validate (dirent) < 0) + if (treeobj_validate (dirent) < 0) goto error; } else goto error; @@ -98,7 +98,7 @@ static int validate_op (json_t *op) int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags, const char *key, const char *json_str) { - json_t *val; + json_t *dirent; json_t *op = NULL; if (!txn || !key) { @@ -109,18 +109,19 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags, goto error; if (!json_str) return flux_kvs_txn_unlink (txn, flags, key); - if (!(val = json_loads (json_str, JSON_DECODE_ANY, NULL))) { - errno = EINVAL; - goto error; + if ((flags & FLUX_KVS_TREEOBJ)) { + if (!(dirent = json_loads (json_str, JSON_DECODE_ANY, NULL))) { + errno = EINVAL; + goto error; + } } - if ((flags & FLUX_KVS_TREEOBJ)) - op = json_pack ("{s:s s:o}", "key", key, - "dirent", val); - else - op = json_pack ("{s:s s:{s:o}}", "key", key, - "dirent", "FILEVAL", val); - if (!op) { - json_decref (val); + else { + if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1))) + goto error; + } + if (!(op = json_pack ("{s:s s:o}", "key", key, + "dirent", dirent))) { + json_decref (dirent); errno = ENOMEM; goto error; } @@ -142,30 +143,41 @@ int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags, const char *key, const char *fmt, ...) { va_list ap; - json_t *val; - json_t *op = NULL; + json_t *val, *op, *dirent; if (!txn || !key | !fmt) { errno = EINVAL; goto error; } - if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0) - goto error; va_start (ap, fmt); val = json_vpack_ex (NULL, 0, fmt, ap); va_end (ap); + if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0) + goto error; if (!val) { errno = EINVAL; goto error; } - if ((flags & FLUX_KVS_TREEOBJ)) - op = json_pack ("{s:s s:o}", "key", key, - "dirent", val); - else - op = json_pack ("{s:s s:{s:o}}", "key", key, - "dirent", "FILEVAL", val); - if (!op) { + if ((flags & FLUX_KVS_TREEOBJ)) { + dirent = val; + } + else { + char *s; + if (!(s = json_dumps (val, JSON_ENCODE_ANY))) { + json_decref (val); + errno = ENOMEM; + goto error; + } json_decref (val); + if (!(dirent = treeobj_create_val (s, strlen (s) + 1))) { + free (s); + goto error; + } + free (s); + } + if (!(op = json_pack ("{s:s s:o}", "key", key, + "dirent", dirent))) { + json_decref (dirent); errno = ENOMEM; goto error; } @@ -186,7 +198,7 @@ int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags, int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags, const char *key) { - json_t *op; + json_t *op, *dirent; if (!txn || !key) { errno = EINVAL; @@ -194,8 +206,11 @@ int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags, } if (validate_flags (flags, 0) < 0) goto error; - if (!(op = json_pack ("{s:s s:{s:{}}}", "key", key, - "dirent", "DIRVAL"))) { + if (!(dirent = treeobj_create_dir ())) + goto error; + if (!(op = json_pack ("{s:s s:o}", "key", key, + "dirent", dirent))) { + json_decref (dirent); errno = ENOMEM; goto error; } @@ -246,7 +261,7 @@ int flux_kvs_txn_unlink (flux_kvs_txn_t *txn, int flags, int flux_kvs_txn_symlink (flux_kvs_txn_t *txn, int flags, const char *key, const char *target) { - json_t *op; + json_t *op, *dirent; if (!txn || !key | !target) { errno = EINVAL; @@ -254,9 +269,12 @@ int flux_kvs_txn_symlink (flux_kvs_txn_t *txn, int flags, } if (validate_flags (flags, 0) < 0) goto error; - if (!(op = json_pack ("{s:s s:{s:s}}", "key", key, - "dirent", "LINKVAL", target))) { - errno = EINVAL; + if (!(dirent = treeobj_create_symlink (target))) + goto error; + if (!(op = json_pack ("{s:s s:o}", "key", key, + "dirent", dirent))) { + json_decref (dirent); + errno = ENOMEM; goto error; } if (validate_op (op) < 0) { From 9f5ee4d83f76ee5d4c7b123718bbde8552eba677 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 3 Aug 2017 09:26:31 -0700 Subject: [PATCH 04/27] test/kvs_txn: switch to use RFC 11 metadata Additional test coverage also added. --- src/common/libkvs/kvs_txn.c | 10 ++ src/common/libkvs/test/kvs_txn.c | 221 +++++++++++++++++++++++-------- 2 files changed, 175 insertions(+), 56 deletions(-) diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c index 763bc32c7de2..b5f3d2101620 100644 --- a/src/common/libkvs/kvs_txn.c +++ b/src/common/libkvs/kvs_txn.c @@ -116,6 +116,16 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags, } } else { + json_t *test; + + /* User must pass in valid json object str, otherwise they + * should use flux_kvs_txn_pack() or flux_kvs_txn_put_raw() + */ + if (!(test = json_loads (json_str, JSON_DECODE_ANY, NULL))) { + errno = EINVAL; + goto done; + } + json_decref (test); if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1))) goto error; } diff --git a/src/common/libkvs/test/kvs_txn.c b/src/common/libkvs/test/kvs_txn.c index 34c4cd05bfa3..742f6a8fe078 100644 --- a/src/common/libkvs/test/kvs_txn.c +++ b/src/common/libkvs/test/kvs_txn.c @@ -4,9 +4,12 @@ #include #include +#include #include "kvs_txn.h" #include "kvs_txn_private.h" +#include "treeobj.h" + #include "src/common/libtap/tap.h" void jdiag (json_t *o) @@ -16,89 +19,195 @@ void jdiag (json_t *o) free (tmp); } +int check_int_value (json_t *dirent, int expected) +{ + char *data; + int rc, len, i; + json_t *val; + + if (treeobj_decode_val (dirent, (void **)&data, &len) < 0) { + diag ("%s: initial base64 decode failed", __FUNCTION__); + return -1; + } + if (len == 0 || data[len - 1] != '\0') { + diag ("%s: data not null terminated", __FUNCTION__); + free (data); + return -1; + } + if (!(val = json_loads (data, JSON_DECODE_ANY, NULL))) { + diag ("%s: couldn't decode JSON", __FUNCTION__); + free (data); + return -1; + } + rc = json_unpack (val, "i", &i); + free (data); + if (rc < 0) { + diag ("%s: couldn't find requested JSON value", __FUNCTION__); + json_decref (val); + return -1; + } + json_decref (val); + if (i != expected) { + diag ("%s: expected %d received %d", __FUNCTION__, expected, i); + return -1; + } + return 0; +} + +int check_string_value (json_t *dirent, const char *expected) +{ + char *data; + int rc, len; + const char *s; + json_t *val; + + if (treeobj_decode_val (dirent, (void **)&data, &len) < 0) { + diag ("%s: initial base64 decode failed", __FUNCTION__); + return -1; + } + if (len == 0 || data[len - 1] != '\0') { + diag ("%s: data not null terminated", __FUNCTION__); + free (data); + return -1; + } + if (!(val = json_loads (data, JSON_DECODE_ANY, NULL))) { + diag ("%s: couldn't decode JSON", __FUNCTION__); + free (data); + return -1; + } + rc = json_unpack (val, "s", &s); + free (data); + if (rc < 0) { + diag ("%s: couldn't find requested JSON value", __FUNCTION__); + json_decref (val); + return -1; + } + if (strcmp (expected, s) != 0) { + diag ("%s: expected %s received %s", __FUNCTION__, expected, s); + json_decref (val); + return -1; + } + json_decref (val); + return 0; +} + void basic (void) { flux_kvs_txn_t *txn; - int i, rc; - const char *key, *s; - json_t *entry; + int rc; + const char *key; + json_t *entry, *dirent; - /* create transaction and add two "put" ops + /* Create a transaction */ txn = flux_kvs_txn_create (); ok (txn != NULL, "flux_kvs_txn_create works"); - rc = flux_kvs_txn_pack (txn, 0, "foo.bar.baz", "i", 42); // 1 + rc = flux_kvs_txn_pack (txn, 0, "foo.bar.baz", "i", 42); ok (rc == 0, - "flux_kvs_txn_pack(i) works"); - rc = flux_kvs_txn_pack (txn, 0, "foo.bar.bleep", "s", "foo"); // 2 + "1: flux_kvs_txn_pack(i) works"); + rc = flux_kvs_txn_pack (txn, 0, "foo.bar.bleep", "s", "foo"); ok (rc == 0, - "flux_kvs_txn_pack(s) works"); - - /* verify first two ops + "2: flux_kvs_txn_pack(s) works"); + rc = flux_kvs_txn_unlink (txn, 0, "a"); + ok (rc == 0, + "3: flux_kvs_txn_unlink works"); + rc = flux_kvs_txn_mkdir (txn, 0, "b.b.b"); + ok (rc == 0, + "4: flux_kvs_txn_mkdir works"); + rc = flux_kvs_txn_symlink (txn, 0, "c.c.c", "b.b.b"); + ok (rc == 0, + "5: flux_kvs_txn_symlink works"); + rc = flux_kvs_txn_put (txn, 0, "d.d.d", "43"); + ok (rc == 0, + "6: flux_kvs_txn_put(i) works"); + rc = flux_kvs_txn_put (txn, 0, "e", NULL); + ok (rc == 0, + "7: flux_kvs_txn_put(NULL) works"); + errno = 0; + rc = flux_kvs_txn_put (txn, 0, "f", ""); + ok (rc < 0 && errno == EINVAL, + "error: flux_kvs_txn_put(empty string) fails with EINVAL"); + errno = 0; + rc = flux_kvs_txn_pack (txn, 0xFFFF, "foo.bar.blorp", "s", "foo"); + ok (rc < 0 && errno == EINVAL, + "error: flux_kvs_txn_pack(bad flags) fails with EINVAL"); + errno = 0; + rc = flux_kvs_txn_mkdir (txn, 0xFFFF, "b.b.b"); + ok (rc < 0 && errno == EINVAL, + "error: flux_kvs_txn_mkdir works"); + errno = 0; + rc = flux_kvs_txn_put (txn, 0xFFFF, "f", "42"); + ok (rc < 0 && errno == EINVAL, + "error: flux_kvs_txn_put(bad flags) fails with EINVAL"); + + /* Verify transaction contents */ ok (txn_get (txn, TXN_GET_FIRST, &entry) == 0 && entry != NULL, - "op-1: retrieved"); - jdiag (entry); - rc = json_unpack (entry, "{s:s s:{s:i}}", "key", &key, - "dirent", "FILEVAL", &i); - diag ("rc = %d" ,rc); - ok (rc == 0 && !strcmp (key, "foo.bar.baz") && i == 42, - "op-1: put foo.bar.baz = 42"); + "1: retrieved"); + ok (json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent) == 0, + "1: unpacked operation"); + ok (!strcmp (key, "foo.bar.baz") + && check_int_value (dirent, 42) == 0, + "1: put foo.bar.baz = 42"); + ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0, - "op-2: retrieved"); + "2: retrieved"); jdiag (entry); - rc = json_unpack (entry, "{s:s s:{s:s}}", "key", &key, - "dirent", "FILEVAL", &s); - ok (rc == 0 && !strcmp (key, "foo.bar.bleep") && !strcmp (s, "foo"), - "op-2: put foo.bar.baz = \"foo\""); - ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry == NULL, - "op-3: NULL"); + ok (json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent) == 0, + "2: unpacked operation"); + ok (!strcmp (key, "foo.bar.bleep") + && check_string_value (dirent, "foo") == 0, + "2: put foo.bar.baz = \"foo\""); - /* add one of each of the other types of ops - */ - rc = flux_kvs_txn_unlink (txn, 0, "a"); // 3 - ok (rc == 0, - "flux_kvs_txn_unlink works"); - rc = flux_kvs_txn_mkdir (txn, 0, "b.b.b"); // 4 - ok (rc == 0, - "flux_kvs_txn_mkdir works"); - rc = flux_kvs_txn_symlink (txn, 0, "c.c.c", "b.b.b"); // 5 - ok (rc == 0, - "flux_kvs_txn_symlink works"); - - /* verify ops - */ - ok (txn_get (txn, TXN_GET_FIRST, NULL) == 0, - "op-1: skip"); - ok (txn_get (txn, TXN_GET_NEXT, NULL) == 0, - "op-2: skip"); ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry != NULL, - "op-3: retrieved"); + "3: retrieved"); jdiag (entry); rc = json_unpack (entry, "{s:s s:n}", "key", &key, "dirent"); - ok (rc == 0 && !strcmp (key, "a"), - "op-3: unlink a"); + "3: unlink a"); + ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry != NULL, - "op-4: retrieved"); + "4: retrieved"); jdiag (entry); - rc = json_unpack (entry, "{s:s s:{s:{}}}", "key", &key, - "dirent", "DIRVAL"); + rc = json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent); + ok (rc == 0 && !strcmp (key, "b.b.b") + && treeobj_is_dir (dirent) && treeobj_get_count (dirent) == 0, + "4: mkdir b.b.b"); - ok (rc == 0 && !strcmp (key, "b.b.b"), - "op-4: mkdir b.b.b"); ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry != NULL, - "op-5: retrieved"); + "5: retrieved"); jdiag (entry); - rc = json_unpack (entry, "{s:s s:{s:s}}", "key", &key, - "dirent", "LINKVAL", &s); - ok (rc == 0 && !strcmp (key, "c.c.c") && !strcmp (s, "b.b.b"), - "op-5: symlink c.c.c b.b.b"); + rc = json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent); + ok (rc == 0 && !strcmp (key, "c.c.c") && treeobj_is_symlink (dirent) + && !strcmp (json_string_value (treeobj_get_data (dirent)), "b.b.b"), + "5: symlink c.c.c b.b.b"); + + ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry != NULL, + "6: retrieved"); + ok (json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent) == 0, + "6: unpacked operation"); + ok (!strcmp (key, "d.d.d") + && check_int_value (dirent, 43) == 0, + "6: put foo.bar.baz = 43"); + + ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry != NULL, + "7: retrieved"); + jdiag (entry); + rc = json_unpack (entry, "{s:s s:n}", "key", &key, + "dirent"); + ok (rc == 0 && !strcmp (key, "e"), + "7: unlink e"); + ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry == NULL, - "op-6: NULL"); + "8: NULL - end of transaction"); flux_kvs_txn_destroy (txn); From d61419748c1b5172a4f5039a9bbd38ed31e22df9 Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 3 Aug 2017 14:38:21 -0700 Subject: [PATCH 05/27] libkvs/txn: add flux_kvs_txn_put_raw() Add an interface for putting raw data into the KVS. Take the opportunity to factor some common code out of all the txn operations. --- src/common/libkvs/kvs_txn.c | 182 ++++++++++++++++++++---------------- src/common/libkvs/kvs_txn.h | 3 + 2 files changed, 103 insertions(+), 82 deletions(-) diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c index b5f3d2101620..8084e2e37ee9 100644 --- a/src/common/libkvs/kvs_txn.c +++ b/src/common/libkvs/kvs_txn.c @@ -95,20 +95,78 @@ static int validate_op (json_t *op) return -1; } +/* Add an operation on dirent to the transaction. + * Takes a reference on dirent so caller retains ownership. + */ +static int flux_kvs_txn_put_treeobj (flux_kvs_txn_t *txn, int flags, + const char *key, json_t *dirent) +{ + json_t *op; + + if (!dirent) + op = json_pack ("{s:s s:n}", "key", key, "dirent"); + else + op = json_pack ("{s:s s:O}", "key", key, "dirent", dirent); + if (!op) { + errno = ENOMEM; + goto error; + } + if (validate_op (op) < 0) + goto error; + if (json_array_append_new (txn->ops, op) < 0) { + json_decref (op); + errno = ENOMEM; + goto error; + } + return 0; +error: + return -1; +} + +int flux_kvs_txn_put_raw (flux_kvs_txn_t *txn, int flags, + const char *key, void *data, int len) +{ + json_t *dirent = NULL; + int saved_errno; + + if (!txn || !key) { + errno = EINVAL; + goto error; + } + if (validate_flags (flags, 0) < 0) + goto error; + if (!(dirent = treeobj_create_val (data, len))) + goto error; + if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0) + goto error; + json_decref (dirent); + return 0; + +error: + saved_errno = errno; + json_decref (dirent); + errno = saved_errno; + return -1; +} + int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags, const char *key, const char *json_str) { - json_t *dirent; - json_t *op = NULL; + json_t *dirent = NULL; + int saved_errno; if (!txn || !key) { errno = EINVAL; goto error; } - if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0) - goto error; if (!json_str) return flux_kvs_txn_unlink (txn, flags, key); + if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0) + goto error; + /* If TREEOBJ flag, decoded json_str *is* the dirent. + * Its validity will be validated by put_treeobj(). + * Otherwise, create a 'val' dirent, treating json_str as opaque data. + */ if ((flags & FLUX_KVS_TREEOBJ)) { if (!(dirent = json_loads (json_str, JSON_DECODE_ANY, NULL))) { errno = EINVAL; @@ -123,29 +181,21 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags, */ if (!(test = json_loads (json_str, JSON_DECODE_ANY, NULL))) { errno = EINVAL; - goto done; + goto error; } json_decref (test); if (!(dirent = treeobj_create_val (json_str, strlen (json_str) + 1))) goto error; } - if (!(op = json_pack ("{s:s s:o}", "key", key, - "dirent", dirent))) { - json_decref (dirent); - errno = ENOMEM; + if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0) goto error; - } - if (validate_op (op) < 0) { - json_decref (op); - goto error; - } - if (json_array_append_new (txn->ops, op) < 0) { - json_decref (op); - errno = ENOMEM; - goto error; - } + json_decref (dirent); return 0; + error: + saved_errno = errno; + json_decref (dirent); + errno = saved_errno; return -1; } @@ -153,29 +203,35 @@ int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags, const char *key, const char *fmt, ...) { va_list ap; - json_t *val, *op, *dirent; + json_t *val, *dirent = NULL; + int saved_errno; if (!txn || !key | !fmt) { errno = EINVAL; goto error; } + if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0) + goto error; va_start (ap, fmt); val = json_vpack_ex (NULL, 0, fmt, ap); va_end (ap); - if (validate_flags (flags, FLUX_KVS_TREEOBJ) < 0) - goto error; if (!val) { errno = EINVAL; goto error; } + /* If TREEOBJ flag, the json object *is* the dirent. + * Its validity will be validated by put_treeobj(). + * Otherwise, create a 'val' dirent, treating encoded json object + * as opaque data. + */ if ((flags & FLUX_KVS_TREEOBJ)) { dirent = val; } else { char *s; if (!(s = json_dumps (val, JSON_ENCODE_ANY))) { - json_decref (val); errno = ENOMEM; + json_decref (val); goto error; } json_decref (val); @@ -185,30 +241,23 @@ int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags, } free (s); } - if (!(op = json_pack ("{s:s s:o}", "key", key, - "dirent", dirent))) { - json_decref (dirent); - errno = ENOMEM; - goto error; - } - if (validate_op (op) < 0) { - json_decref (op); - goto error; - } - if (json_array_append_new (txn->ops, op) < 0) { - json_decref (op); - errno = ENOMEM; + if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0) goto error; - } + json_decref (dirent); return 0; + error: + saved_errno = errno; + json_decref (dirent); + errno = saved_errno; return -1; } int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags, const char *key) { - json_t *op, *dirent; + json_t *dirent = NULL; + int saved_errno; if (!txn || !key) { errno = EINVAL; @@ -218,51 +267,29 @@ int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags, goto error; if (!(dirent = treeobj_create_dir ())) goto error; - if (!(op = json_pack ("{s:s s:o}", "key", key, - "dirent", dirent))) { - json_decref (dirent); - errno = ENOMEM; - goto error; - } - if (validate_op (op) < 0) { - json_decref (op); - goto error; - } - if (json_array_append_new (txn->ops, op) < 0) { - json_decref (op); - errno = ENOMEM; + if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0) goto error; - } + json_decref (dirent); return 0; + error: + saved_errno = errno; + json_decref (dirent); + errno = saved_errno; return -1; } int flux_kvs_txn_unlink (flux_kvs_txn_t *txn, int flags, const char *key) { - json_t *op; - if (!txn || !key) { errno = EINVAL; goto error; } if (validate_flags (flags, 0) < 0) goto error; - if (!(op = json_pack ("{s:s s:n}", "key", key, - "dirent"))) { - errno = ENOMEM; - goto error; - } - if (validate_op (op) < 0) { - json_decref (op); - goto error; - } - if (json_array_append_new (txn->ops, op) < 0) { - json_decref (op); - errno = ENOMEM; + if (flux_kvs_txn_put_treeobj (txn, flags, key, NULL) < 0) goto error; - } return 0; error: return -1; @@ -271,7 +298,8 @@ int flux_kvs_txn_unlink (flux_kvs_txn_t *txn, int flags, int flux_kvs_txn_symlink (flux_kvs_txn_t *txn, int flags, const char *key, const char *target) { - json_t *op, *dirent; + json_t *dirent = NULL; + int saved_errno; if (!txn || !key | !target) { errno = EINVAL; @@ -281,23 +309,13 @@ int flux_kvs_txn_symlink (flux_kvs_txn_t *txn, int flags, goto error; if (!(dirent = treeobj_create_symlink (target))) goto error; - if (!(op = json_pack ("{s:s s:o}", "key", key, - "dirent", dirent))) { - json_decref (dirent); - errno = ENOMEM; - goto error; - } - if (validate_op (op) < 0) { - json_decref (op); + if (flux_kvs_txn_put_treeobj (txn, flags, key, dirent) < 0) goto error; - } - if (json_array_append_new (txn->ops, op) < 0) { - json_decref (op); - errno = ENOMEM; - goto error; - } return 0; error: + saved_errno = errno; + json_decref (dirent); + errno = saved_errno; return -1; } diff --git a/src/common/libkvs/kvs_txn.h b/src/common/libkvs/kvs_txn.h index 7db567f97654..49b372091e42 100644 --- a/src/common/libkvs/kvs_txn.h +++ b/src/common/libkvs/kvs_txn.h @@ -12,6 +12,9 @@ int flux_kvs_txn_put (flux_kvs_txn_t *txn, int flags, int flux_kvs_txn_pack (flux_kvs_txn_t *txn, int flags, const char *key, const char *fmt, ...); +int flux_kvs_txn_put_raw (flux_kvs_txn_t *txn, int flags, + const char *key, void *data, int len); + int flux_kvs_txn_mkdir (flux_kvs_txn_t *txn, int flags, const char *key); From 7384fad961a83b29a9c59416418f2e99c949ad4d Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Thu, 3 Aug 2017 14:39:26 -0700 Subject: [PATCH 06/27] test/kvs_txn: Add tests for new raw interface --- src/common/libkvs/test/kvs_txn.c | 77 +++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) diff --git a/src/common/libkvs/test/kvs_txn.c b/src/common/libkvs/test/kvs_txn.c index 742f6a8fe078..e370a7fb5931 100644 --- a/src/common/libkvs/test/kvs_txn.c +++ b/src/common/libkvs/test/kvs_txn.c @@ -6,7 +6,7 @@ #include #include -#include "kvs_txn.h" +#include "kvs.h" #include "kvs_txn_private.h" #include "treeobj.h" @@ -19,6 +19,9 @@ void jdiag (json_t *o) free (tmp); } +/* Decode a 'val' containing base64 encoded JSON. + * Extract a number, and compare it to 'expected'. + */ int check_int_value (json_t *dirent, int expected) { char *data; @@ -54,6 +57,9 @@ int check_int_value (json_t *dirent, int expected) return 0; } +/* Decode a 'val' containing base64 encoded JSON. + * Extract a string, and compare it to 'expected'. + */ int check_string_value (json_t *dirent, const char *expected) { char *data; @@ -210,7 +216,75 @@ void basic (void) "8: NULL - end of transaction"); flux_kvs_txn_destroy (txn); +} + +void test_raw_values (void) +{ + flux_kvs_txn_t *txn; + char buf[13], *nbuf; + json_t *entry, *dirent; + const char *key; + int nlen; + + memset (buf, 'c', sizeof (buf)); + + txn = flux_kvs_txn_create (); + ok (txn != NULL, + "flux_kvs_txn_create works"); + + /* Try some bad params + */ + errno = 0; + ok (flux_kvs_txn_put_raw (txn, FLUX_KVS_TREEOBJ, "a.b.c", + buf, sizeof (buf)) < 0 + && errno == EINVAL, + "flux_kvs_txn_put_raw fails with EINVAL when fed TREEOBJ flag"); + errno = 0; + ok (flux_kvs_txn_put_raw (txn, 0, NULL, buf, sizeof (buf)) < 0 + && errno == EINVAL, + "flux_kvs_txn_put_raw fails with EINVAL when fed NULL key"); + /* Put an empty buffer. + */ + ok (flux_kvs_txn_put_raw (txn, 0, "a.a.a", NULL, 0) == 0, + "flux_kvs_txn_put_raw works on empty buffer"); + /* Put some data + */ + ok (flux_kvs_txn_put_raw (txn, 0, "a.b.c", buf, sizeof (buf)) == 0, + "flux_kvs_txn_put_raw works with data"); + /* Get first. + */ + ok (txn_get (txn, TXN_GET_FIRST, &entry) == 0 && entry != NULL, + "retreived 1st op from txn"); + jdiag (entry); + ok (json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent) == 0, + "decoded op to get dirent"); + nbuf = buf; + nlen = sizeof (buf); + ok (treeobj_decode_val (dirent, (void **)&nbuf, &nlen) == 0, + "retrieved buffer from dirent"); + ok (nlen == 0, + "and it is size of zero"); + ok (nbuf == NULL, + "and buffer is NULL"); + + /* Get 2nd + */ + ok (txn_get (txn, TXN_GET_NEXT, &entry) == 0 && entry != NULL, + "retreived 2nd op from txn"); + jdiag (entry); + ok (json_unpack (entry, "{s:s s:o}", "key", &key, + "dirent", &dirent) == 0, + "decoded op to get dirent"); + ok (treeobj_decode_val (dirent, (void **)&nbuf, &nlen) == 0, + "retrieved buffer from dirent"); + ok (nlen == sizeof (buf), + "and it is the correct size"); + ok (memcmp (nbuf, buf, nlen) == 0, + "and it is the correct content"); + free (nbuf); + flux_kvs_txn_destroy (txn); } int main (int argc, char *argv[]) @@ -219,6 +293,7 @@ int main (int argc, char *argv[]) plan (NO_PLAN); basic (); + test_raw_values (); done_testing(); return (0); From e0f595c1fc0355a8f7978eb5677ec95f2d5e2f3e Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 8 Aug 2017 09:43:25 -0700 Subject: [PATCH 07/27] libkvs/txn: add big explanatory comment Add large description of overall functionality to file. --- src/common/libkvs/kvs_txn.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/common/libkvs/kvs_txn.c b/src/common/libkvs/kvs_txn.c index 8084e2e37ee9..c34a0645a81d 100644 --- a/src/common/libkvs/kvs_txn.c +++ b/src/common/libkvs/kvs_txn.c @@ -32,6 +32,30 @@ #include "kvs_txn_private.h" #include "treeobj.h" +/* A transaction is an ordered list of operations. + * Each operation contains a key and a "dirent" (RFC 11 tree object). + * The operation assigns a new dirent to the key. A NULL dirent removes + * the key. A commit operation accepts a transaction and applies the + * whole thing, in order. If any operation fails, the transaction is + * not finalized, thus either all or none of the operations are applied. + * + * Raw versus JSON values: + * All values are base64 encoded per RFC 11, even values that + * are themselves JSON. This is a change from the original design, + * which stored only JSON values. The new design stores only untyped + * opaque data. Because of this history, and because it offers convenience + * for KVS use cases, we still have flux_kvs_txn_put() which accepts encoded + * JSON, and flux_kvs_txn_pack() which builds a JSON object, then encodes it. + * The encoded JSON is converted internally to a string, that is encoded as + * base64 and becomes the raw value (with terminating NULL). + * An exception is if the FLUX_KVS_TREEOBJ flag is set - then the encoded + * or built JSON object is interpreted directly as an RFC 11 tree object. + * + * NULL or empty values: + * A zero length raw value is considered valid. + * A NULL JSON string passed to flux_kvs_txn_put() is interpreted as an unlink. + * A NULL format string passed to flux_kvs_txn_pack() is invalid. + */ struct flux_kvs_txn { json_t *ops; int cursor; From 5d84f9ee02556035e129d94a402c6c4cce4b6c7c Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Fri, 4 Aug 2017 16:04:08 -0700 Subject: [PATCH 08/27] libkvs/lookup: switch to use RFC 11 metadata --- src/common/libkvs/kvs_lookup.c | 166 ++++++++++++++++++++++++++++----- 1 file changed, 142 insertions(+), 24 deletions(-) diff --git a/src/common/libkvs/kvs_lookup.c b/src/common/libkvs/kvs_lookup.c index 100764f08b4d..b045b9f34400 100644 --- a/src/common/libkvs/kvs_lookup.c +++ b/src/common/libkvs/kvs_lookup.c @@ -33,6 +33,40 @@ #include #include "kvs_lookup.h" +#include "treeobj.h" + +struct lookup_ctx { + int flags; + json_t *treeobj; + char *treeobj_str; // json_dumps of tree object returned from lookup + void *val_data; // result of base64 decode of val object data + int val_len; + bool val_valid; + json_t *val_obj; +}; + +static const char *auxkey = "flux::lookup_ctx"; + +static void free_ctx (struct lookup_ctx *ctx) +{ + if (ctx) { + free (ctx->treeobj_str); + free (ctx->val_data); + json_decref (ctx->val_obj); + free (ctx); + } +} + +static struct lookup_ctx *alloc_ctx (void) +{ + struct lookup_ctx *ctx; + + if (!(ctx = calloc (1, sizeof (*ctx)))) { + errno = ENOMEM; + return NULL; + } + return ctx; +} static int validate_lookup_flags (int flags) { @@ -50,13 +84,28 @@ static int validate_lookup_flags (int flags) flux_future_t *flux_kvs_lookup (flux_t *h, int flags, const char *key) { + struct lookup_ctx *ctx; + flux_future_t *f; + if (!h || !key || strlen (key) == 0 || validate_lookup_flags (flags) < 0) { errno = EINVAL; return NULL; } - return flux_rpc_pack (h, "kvs.get", FLUX_NODEID_ANY, 0, "{s:s s:i}", - "key", key, - "flags", flags); + if (!(ctx = alloc_ctx ())) + return NULL; + ctx->flags = flags; + if (!(f = flux_rpc_pack (h, "kvs.get", FLUX_NODEID_ANY, 0, "{s:s s:i}", + "key", key, + "flags", flags))) { + free_ctx (ctx); + return NULL; + } + if (flux_future_aux_set (f, auxkey, ctx, (flux_free_f)free_ctx) < 0) { + free_ctx (ctx); + flux_future_destroy (f); + return NULL; + } + return f; } flux_future_t *flux_kvs_lookupat (flux_t *h, int flags, const char *key, @@ -64,23 +113,40 @@ flux_future_t *flux_kvs_lookupat (flux_t *h, int flags, const char *key, { flux_future_t *f; json_t *obj = NULL; + struct lookup_ctx *ctx; if (!h || !key || strlen (key) == 0 || validate_lookup_flags (flags) < 0) { errno = EINVAL; return NULL; } + if (!(ctx = alloc_ctx ())) + return NULL; + ctx->flags = flags; if (!treeobj) { - f = flux_kvs_lookup (h, flags, key); + if (!(f = flux_kvs_lookup (h, flags, key))) { + free_ctx (ctx); + return NULL; + } } else { if (!(obj = json_loads (treeobj, 0, NULL))) { errno = EINVAL; return NULL; } - f = flux_rpc_pack (h, "kvs.get", FLUX_NODEID_ANY, 0, "{s:s s:i s:O}", - "key", key, - "flags", flags, - "rootdir", obj); + if (!(f = flux_rpc_pack (h, "kvs.get", FLUX_NODEID_ANY, 0, + "{s:s s:i s:O}", "key", key, + "flags", flags, + "rootdir", obj))) { + free_ctx (ctx); + json_decref (obj); + return NULL; + } + } + if (flux_future_aux_set (f, auxkey, ctx, (flux_free_f)free_ctx) < 0) { + free_ctx (ctx); + json_decref (obj); + flux_future_destroy (f); + return NULL; } json_decref (obj); return f; @@ -88,39 +154,91 @@ flux_future_t *flux_kvs_lookupat (flux_t *h, int flags, const char *key, int flux_kvs_lookup_get (flux_future_t *f, const char **json_str) { - const char *auxkey = "flux::kvs_valstr"; - json_t *obj; - char *s; + struct lookup_ctx *ctx; - if (!(s = flux_future_aux_get (f, auxkey))) { - if (flux_rpc_get_unpack (f, "{s:o}", "val", &obj) < 0) + if (!(ctx = flux_future_aux_get (f, auxkey))) { + errno = EINVAL; + return -1; + } + if (!(ctx->treeobj)) { + if (flux_rpc_get_unpack (f, "{s:o}", "val", &ctx->treeobj) < 0) return -1; - if (!(s = json_dumps (obj, JSON_COMPACT|JSON_ENCODE_ANY))) { + } + /* If TREEOBJ or READDIR flags, val is a tree object. + * Re-encode as a string and return. + */ + if ((ctx->flags & FLUX_KVS_TREEOBJ) || (ctx->flags & FLUX_KVS_READDIR)) { + if (!ctx->treeobj_str) { + size_t flags = JSON_ENCODE_ANY | JSON_COMPACT | JSON_SORT_KEYS; + if (!(ctx->treeobj_str = json_dumps (ctx->treeobj, flags))) { + errno = EINVAL; + return -1; + } + } + if (json_str) + *json_str = ctx->treeobj_str; + } + /* If SYMLINK flag, extract link target from symlink object + * and return it directly. + */ + else if ((ctx->flags & FLUX_KVS_READLINK)) { + if (!treeobj_is_symlink (ctx->treeobj)) { errno = EINVAL; return -1; } - if (flux_future_aux_set (f, auxkey, s, free) < 0) { - free (s); - errno = ENOMEM; - return -1; + if (json_str) { + json_t *str = treeobj_get_data (ctx->treeobj); + const char *s = json_string_value (str); + if (!s) { + errno = EINVAL; + return -1; + } + *json_str = s; + } + } + /* No flags, val is a 'val' object. + * Decide the data and return it as a string if it is properly terminated. + */ + else { + if (!ctx->val_valid) { + if (treeobj_decode_val (ctx->treeobj, &ctx->val_data, + &ctx->val_len) < 0) + return -1; + ctx->val_valid = true; + char *s = ctx->val_data; + if (ctx->val_len < 1 || s[ctx->val_len - 1] != '\0') { + errno = EINVAL; + return -1; + } + if (json_str) + *json_str = s; } } - if (json_str) - *json_str = s; return 0; } int flux_kvs_lookup_get_unpack (flux_future_t *f, const char *fmt, ...) { + struct lookup_ctx *ctx; va_list ap; - json_t *obj; int rc; - if (flux_rpc_get_unpack (f, "{s:o}", "val", &obj) < 0) + if (!(ctx = flux_future_aux_get (f, auxkey))) { + errno = EINVAL; return -1; + } + if (!ctx->val_obj) { + const char *json_str; + if (flux_kvs_lookup_get (f, &json_str) < 0) + return -1; + if (!(ctx->val_obj = json_loads (json_str, JSON_DECODE_ANY, NULL))) { + errno = EINVAL; + return -1; + } + } va_start (ap, fmt); - if ((rc = json_vunpack_ex (obj, NULL, 0, fmt, ap) < 0)) - errno = EPROTO; + if ((rc = json_vunpack_ex (ctx->val_obj, NULL, 0, fmt, ap) < 0)) + errno = EINVAL; va_end (ap); return rc; From f3135fed181d6161f12c20aea51845af1e68e3ed Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 8 Aug 2017 10:26:10 -0700 Subject: [PATCH 09/27] libkvs/dir: switch to use RFC 11 metadata --- src/common/libkvs/kvs_dir.c | 54 +++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/common/libkvs/kvs_dir.c b/src/common/libkvs/kvs_dir.c index e0ad750e5ef2..8ba29c68cb88 100644 --- a/src/common/libkvs/kvs_dir.c +++ b/src/common/libkvs/kvs_dir.c @@ -32,6 +32,7 @@ #include #include +#include "treeobj.h" struct kvsdir_struct { flux_t *handle; @@ -43,7 +44,7 @@ struct kvsdir_struct { }; struct kvsdir_iterator_struct { - json_t *dirobj; + json_t *dirdata; void *iter; }; @@ -54,11 +55,13 @@ void kvsdir_incref (kvsdir_t *dir) void kvsdir_destroy (kvsdir_t *dir) { - if (--dir->usecount == 0) { + if (dir && --dir->usecount == 0) { + int saved_errno = errno; free (dir->rootref); free (dir->key); json_decref (dir->dirobj); free (dir); + errno = saved_errno; } } @@ -76,23 +79,30 @@ kvsdir_t *kvsdir_create (flux_t *handle, const char *rootref, return NULL; } if (!(dir = calloc (1, sizeof (*dir)))) - goto nomem; + goto error; dir->handle = handle; if (rootref) { if (!(dir->rootref = strdup (rootref))) - goto nomem; + goto error; } if (!(dir->key = strdup (key))) - goto nomem; - if (!(dir->dirobj = json_loads (json_str, 0, NULL))) - goto nomem; + goto error; + if (!(dir->dirobj = json_loads (json_str, 0, NULL))) { + errno = EINVAL; + goto error; + } + if (treeobj_validate (dir->dirobj) < 0) + goto error; + if (!treeobj_is_dir (dir->dirobj)) { + errno = EINVAL; + goto error; + } dir->usecount = 1; return dir; -nomem: +error: kvsdir_destroy (dir); - errno = ENOMEM; return NULL; } @@ -109,7 +119,7 @@ const char *kvsdir_tostring (kvsdir_t *dir) int kvsdir_get_size (kvsdir_t *dir) { - return json_object_size (dir->dirobj); + return treeobj_get_count (dir->dirobj); } const char *kvsdir_key (kvsdir_t *dir) @@ -139,20 +149,18 @@ kvsitr_t *kvsitr_create (kvsdir_t *dir) kvsitr_t *itr; if (!(itr = calloc (1, sizeof (*itr)))) - goto nomem; - itr->dirobj = dir->dirobj; - itr->iter = json_object_iter (itr->dirobj); - + goto error; + itr->dirdata = treeobj_get_data (dir->dirobj); + itr->iter = json_object_iter (itr->dirdata); return itr; -nomem: +error: kvsitr_destroy (itr); - errno = ENOMEM; return NULL; } void kvsitr_rewind (kvsitr_t *itr) { - itr->iter = json_object_iter (itr->dirobj); + itr->iter = json_object_iter (itr->dirdata); } const char *kvsitr_next (kvsitr_t *itr) @@ -161,24 +169,24 @@ const char *kvsitr_next (kvsitr_t *itr) if (itr->iter) { name = json_object_iter_key (itr->iter); - itr->iter = json_object_iter_next (itr->dirobj, itr->iter); + itr->iter = json_object_iter_next (itr->dirdata, itr->iter); } return name; } bool kvsdir_exists (kvsdir_t *dir, const char *name) { - if (json_object_get (dir->dirobj, name)) + if (treeobj_get_entry (dir->dirobj, name)) return true; return false; } bool kvsdir_isdir (kvsdir_t *dir, const char *name) { - json_t *obj = json_object_get (dir->dirobj, name); + json_t *obj = treeobj_get_entry (dir->dirobj, name); if (obj) { - if (json_object_get (obj, "DIRREF") || json_object_get (obj, "DIRVAL")) + if (treeobj_is_dir (obj) || treeobj_is_dirref (obj)) return true; } return false; @@ -186,10 +194,10 @@ bool kvsdir_isdir (kvsdir_t *dir, const char *name) bool kvsdir_issymlink (kvsdir_t *dir, const char *name) { - json_t *obj = json_object_get (dir->dirobj, name); + json_t *obj = treeobj_get_entry (dir->dirobj, name); if (obj) { - if (json_object_get (obj, "LINKVAL")) + if (treeobj_is_symlink (obj)) return true; } return false; From 5b29a5e25b9a52b4e811722363ea296e6a1195fa Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 8 Aug 2017 11:14:34 -0700 Subject: [PATCH 10/27] test/kvs_dir: increase testing coverage --- src/common/libkvs/test/kvs_dir.c | 177 ++++++++++++++++++++++++++++++- 1 file changed, 175 insertions(+), 2 deletions(-) diff --git a/src/common/libkvs/test/kvs_dir.c b/src/common/libkvs/test/kvs_dir.c index da6aff1c0b7a..cc51f5704d76 100644 --- a/src/common/libkvs/test/kvs_dir.c +++ b/src/common/libkvs/test/kvs_dir.c @@ -2,21 +2,193 @@ #include "config.h" #endif +#include #include #include +#include "kvs.h" +#include "kvs_txn_private.h" +#include "treeobj.h" + #include "src/common/libflux/flux.h" #include "kvs_dir.h" #include "src/common/libtap/tap.h" -void test_error (void) +void jdiag (json_t *o) +{ + char *tmp = json_dumps (o, JSON_COMPACT); + diag ("%s", tmp); + free (tmp); +} + +void test_empty (void) { kvsdir_t *dir; + kvsitr_t *itr; + json_t *o; + char *s; + const char *key; errno = 0; dir = kvsdir_create (NULL, NULL, NULL, NULL); ok (dir == NULL && errno == EINVAL, "kvsdir_create with all NULL args fails with EINVAL"); + + errno = 0; + dir = kvsdir_create (NULL, NULL, "foo", "{}"); + ok (dir == NULL && errno == EINVAL, + "kvsdir_create with empty JSON objects fails with EINVAL"); + + errno = 0; + dir = kvsdir_create (NULL, NULL, "foo", "foo"); + ok (dir == NULL && errno == EINVAL, + "kvsdir_create with bad JSON objects fails with EINVAL"); + + errno = 0; + dir = kvsdir_create (NULL, NULL, "foo", + "{\"data\":\"MQA=\",\"type\":\"FOO\",\"ver\":1}"); + ok (dir == NULL && errno == EINVAL, + "kvsdir_create with invalid treeobj fails with EINVAL"); + + errno = 0; + dir = kvsdir_create (NULL, NULL, "foo", + "{\"data\":\"MQA=\",\"type\":\"val\",\"ver\":1}"); + ok (dir == NULL && errno == EINVAL, + "kvsdir_create with non-dir treeobj fails with EINVAL"); + + if (!(o = treeobj_create_dir ())) + BAIL_OUT ("treeobj_create_dir failed"); + if (!(s = json_dumps (o, JSON_COMPACT))) + BAIL_OUT ("json_dumps failed on new treeobj"); + dir = kvsdir_create (NULL, NULL, "foo", s); + free (s); + + ok (dir != NULL, + "kvsdir_create with empty directory works"); + diag ("%s", kvsdir_tostring (dir)); + + ok (!kvsdir_exists (dir, "noexist"), + "kvsdir_exists on nonexistent key returns false"); + ok (!kvsdir_isdir (dir, "noexist"), + "kvsdir_isdir on nonexistent key returns false"); + ok (!kvsdir_issymlink (dir, "noexist"), + "kvsdir_issymlink on nonexistent key returns false"); + + key = kvsdir_key (dir); + ok (key != NULL && !strcmp (key, "foo"), + "kvsdir_key returns the key we put in"); + key = kvsdir_key_at (dir, "a.b.c"); + ok (key != NULL && !strcmp (key, "foo.a.b.c"), + "kvsdir_key_at a.b.c returns foo.a.b.c"); + ok (kvsdir_handle (dir) == NULL, + "kvsdir_handle returns NULL since that's what we put in"); + ok (kvsdir_rootref (dir) == NULL, + "kvsdir_rootref returns NULL since that's what we put in"); + ok (kvsdir_get_size (dir) == 0, + "kvsdir_get_size returns zero"); + + itr = kvsitr_create (dir); + ok (itr != NULL, + "kvsitr_create works"); + ok (kvsitr_next (itr) == NULL, + "kvsitr_next returns NULL on first call"); + ok (kvsitr_next (itr) == NULL, + "kvsitr_next returns NULL on second call"); + kvsitr_rewind (itr); + ok (kvsitr_next (itr) == NULL, + "kvsitr_next returns NULL after rewind"); + kvsitr_destroy (itr); + + kvsdir_destroy (dir); +} + +void test_full (void) +{ + kvsdir_t *dir; + kvsitr_t *itr; + json_t *o, *dirent; + char *s; + + if (!(o = treeobj_create_dir ())) + BAIL_OUT ("treeobj_create_dir failed"); + if (!(dirent = treeobj_create_symlink ("a.b.c"))) + BAIL_OUT ("treeobj_create_symlink failed"); + if (treeobj_insert_entry (o, "foo", dirent) < 0) + BAIL_OUT ("treeobj_insert_entry failed"); + json_decref (dirent); + if (!(dirent = treeobj_create_val ("xxxx", 4))) + BAIL_OUT ("treeobj_create_val failed"); + if (treeobj_insert_entry (o, "bar", dirent) < 0) + BAIL_OUT ("treeobj_insert_entry failed"); + json_decref (dirent); + if (!(dirent = treeobj_create_dir ())) + BAIL_OUT ("treeobj_create_dir failed"); + if (treeobj_insert_entry (o, "baz", dirent) < 0) + BAIL_OUT ("treeobj_insert_entry failed"); + json_decref (dirent); + + if (!(s = json_dumps (o, JSON_COMPACT))) + BAIL_OUT ("json_dumps failed on new treeobj"); + dir = kvsdir_create (NULL, NULL, "foo", s); + free (s); + ok (dir != NULL, + "kvsdir_create works"); + diag ("%s", kvsdir_tostring (dir)); + + ok (!kvsdir_exists (dir, "noexist"), + "kvsdir_exists on nonexistent key returns false"); + ok (kvsdir_exists (dir, "foo"), + "kvsdir_exists on existing symlink returns true"); + ok (kvsdir_exists (dir, "bar"), + "kvsdir_exists on existing val returns true"); + ok (kvsdir_exists (dir, "baz"), + "kvsdir_exists on existing dir returns true"); + + ok (!kvsdir_isdir (dir, "noexist"), + "kvsdir_isdir on nonexistent key returns false"); + ok (!kvsdir_isdir (dir, "foo"), + "kvsdir_isdir on existing symlink returns false"); + ok (!kvsdir_isdir (dir, "bar"), + "kvsdir_isdir on existing val returns false"); + ok (kvsdir_isdir (dir, "baz"), + "kvsdir_isdir on existing symlink returns true"); + + ok (!kvsdir_issymlink (dir, "noexist"), + "kvsdir_issymlink on nonexistent key returns false"); + ok (kvsdir_issymlink (dir, "foo"), + "kvsdir_issymlink on existing symlink returns true"); + ok (!kvsdir_issymlink (dir, "bar"), + "kvsdir_issymlink on existing val returns false"); + ok (!kvsdir_issymlink (dir, "baz"), + "kvsdir_issymlink on existing dir returns false"); + + ok (kvsdir_get_size (dir) == 3, + "kvsdir_get_size returns 3"); + + itr = kvsitr_create (dir); + ok (itr != NULL, + "kvsitr_create works"); + ok (kvsitr_next (itr) != NULL, + "kvsitr_next returns non-NULL on first call"); + ok (kvsitr_next (itr) != NULL, + "kvsitr_next returns non-NULL on second call"); + ok (kvsitr_next (itr) != NULL, + "kvsitr_next returns non-NULL on third call"); + ok (kvsitr_next (itr) == NULL, + "kvsitr_next returns NULL on fourth call"); + + kvsitr_rewind (itr); + ok (kvsitr_next (itr) != NULL, + "kvsitr_next returns non-NULL after rewind"); + ok (kvsitr_next (itr) != NULL, + "kvsitr_next returns non-NULL on second call"); + ok (kvsitr_next (itr) != NULL, + "kvsitr_next returns non-NULL on third call"); + ok (kvsitr_next (itr) == NULL, + "kvsitr_next returns NULL on fourth call"); + kvsitr_destroy (itr); + + kvsdir_destroy (dir); } int main (int argc, char *argv[]) @@ -24,7 +196,8 @@ int main (int argc, char *argv[]) plan (NO_PLAN); - test_error (); + test_empty (); + test_full (); done_testing(); return (0); From 83219e84f46e9afeacbeab10e4618f4f3e0e597e Mon Sep 17 00:00:00 2001 From: Jim Garlick Date: Tue, 15 Aug 2017 16:10:43 -0700 Subject: [PATCH 11/27] libkvs/watch: switch to use RFC 11 metadata --- src/common/libkvs/kvs_watch.c | 90 ++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 17 deletions(-) diff --git a/src/common/libkvs/kvs_watch.c b/src/common/libkvs/kvs_watch.c index fe03b5533783..596764ef0de3 100644 --- a/src/common/libkvs/kvs_watch.c +++ b/src/common/libkvs/kvs_watch.c @@ -29,6 +29,8 @@ #include #include +#include "treeobj.h" + typedef enum { WATCH_JSONSTR, WATCH_DIR, } watch_type_t; @@ -50,6 +52,7 @@ typedef struct { static void watch_response_cb (flux_t *h, flux_msg_handler_t *w, const flux_msg_t *msg, void *arg); +static int decode_val_object (json_t *val, char **json_str); static void freectx (kvs_watch_ctx_t *ctx) { @@ -212,15 +215,12 @@ static void watch_response_cb (flux_t *h, flux_msg_handler_t *w, goto done; if (!(wp = lookup_watcher (h, matchtag))) goto done; - if (flux_response_decode (msg, NULL, NULL) < 0) + if (flux_response_decode (msg, NULL, NULL) < 0) // handle error response goto done; if (flux_msg_unpack (msg, "{s:o}", "val", &val) < 0) goto done; - if (json_typeof (val) != JSON_NULL - && !(json_str = json_dumps (val, JSON_ENCODE_ANY))) { - errno = EPROTO; + if (decode_val_object (val, &json_str) < 0) goto done; - } if (dispatch_watch (h, wp, json_str) < 0) flux_reactor_stop_error (flux_get_reactor (h)); done: @@ -255,23 +255,62 @@ static flux_future_t *kvs_watch_rpc (flux_t *h, const char *key, return NULL; } -/* json_str set to copy of value (caller must free) */ -static int kvs_watch_rpc_get (flux_future_t *f, char **json_str) +/* val will be one of three things: + * 1) JSON_NULL, set json_str to string-encoded object (NULL) + * 2) RFC 11 dir object, set json_str to string-encoded object + * 3) RFC 11 val object, unbase64, verify NULL termination, set json_str + * The caller must free returned json_str (if any) + */ +static int decode_val_object (json_t *val, char **json_str) { - json_t *val; + char *s; + int len; - if (flux_rpc_get_unpack (f, "{s:o}", "val", &val) < 0) - return -1; - if (json_str) { - char *s = NULL; - if (json_typeof (val) != JSON_NULL - && !(s = json_dumps (val, JSON_ENCODE_ANY))) { + assert(json_str != NULL); + + if (json_typeof (val) == JSON_NULL) { + s = NULL; + } + else if (treeobj_is_dir (val)) { + if (treeobj_validate (val) < 0) + goto error; + if (!(s = json_dumps (val, JSON_ENCODE_ANY))) { errno = EPROTO; - return -1; + goto error; } - *json_str = s; } + else if (treeobj_is_val (val)) { + if (treeobj_validate (val) < 0) + goto error; + if (treeobj_decode_val (val, (void **)&s, &len) < 0) + goto error; + if (s[len - 1] != '\0') { + free (s); + errno = EPROTO; + goto error; + } + } + else { + errno = EPROTO; + goto error; + } + *json_str = s; return 0; +error: + return -1; +} + +static int kvs_watch_rpc_get (flux_future_t *f, char **json_str) +{ + json_t *val; + + if (flux_rpc_get_unpack (f, "{s:o}", "val", &val) < 0) + goto error; + if (decode_val_object (val, json_str) < 0) + goto error; + return 0; +error: + return -1; } static int kvs_watch_rpc_get_matchtag (flux_future_t *f, uint32_t *matchtag) @@ -288,19 +327,32 @@ static int kvs_watch_rpc_get_matchtag (flux_future_t *f, uint32_t *matchtag) return 0; } +/* N.B. somewhat more complicated than it should be (temporarily): + * val_in may be NULL, in which case we send a json NULL value in the RPC. + * Or val_in is an encoded JSON value, so enclose it in an RFC 11 'val' object. + */ int kvs_watch_once (flux_t *h, const char *key, char **valp) { char *val_in = NULL; char *val_out; flux_future_t *f = NULL; int rc = -1; + json_t *xval_obj = NULL; // the RFC 11 'val' object + char *xval_str = NULL; // the RFC 11 'val' object, encoded as a string + int saved_errno; if (!h || !key || !valp) { errno = EINVAL; goto done; } val_in = *valp; - if (!(f = kvs_watch_rpc (h, key, val_in, KVS_WATCH_ONCE))) + if (val_in) { + if (!(xval_obj = treeobj_create_val (val_in, strlen (val_in) + 1))) + goto done; + if (!(xval_str = treeobj_encode (xval_obj))) + goto done; + } + if (!(f = kvs_watch_rpc (h, key, xval_str, KVS_WATCH_ONCE))) goto done; if (kvs_watch_rpc_get (f, &val_out) < 0) goto done; @@ -308,7 +360,11 @@ int kvs_watch_once (flux_t *h, const char *key, char **valp) *valp = val_out; rc = 0; done: + saved_errno = errno; + free (xval_str); + json_decref (xval_obj); flux_future_destroy (f); + errno = saved_errno; return rc; } From 6326e9c0d9537214469cae533bbb4d104f90d6dc Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 3 Aug 2017 17:23:37 -0700 Subject: [PATCH 12/27] common/libkvs: Add treeobj_create_val_base64() Add treeobj_create_val_base64(), which will be useful when the data is already in base64 format. Adjust treeobj_create_val() to use treeobj_create_val_base64(). Add unit tests. --- src/common/libkvs/test/treeobj.c | 28 ++++++++++++++++++++++++++++ src/common/libkvs/treeobj.c | 21 +++++++++++++++++++-- src/common/libkvs/treeobj.h | 4 +++- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/src/common/libkvs/test/treeobj.c b/src/common/libkvs/test/treeobj.c index 3c135e5a7e8e..47bdc4878a9e 100644 --- a/src/common/libkvs/test/treeobj.c +++ b/src/common/libkvs/test/treeobj.c @@ -196,6 +196,33 @@ void test_val (void) json_decref (val2); } +void test_val_base64 (void) +{ + json_t *val; + char *base64 = "NDI="; /* 42 w/o ending NUL byte*/ + char *outbuf; + int outlen; + + ok ((val = treeobj_create_val_base64 (base64)) != NULL, + "treeobj_create_val_base64 works"); + diag_json (val); + ok (treeobj_is_val (val), + "treeobj_is_value returns true"); + ok (treeobj_decode_val (val, (void **)&outbuf, &outlen) == 0, + "treeobj_decode_val works"); + ok (outlen == 2, + "and returned correct size"); + ok (memcmp ("42", outbuf, 2) == 0, + "and returned correct data"); + free (outbuf); + + errno = 0; + ok (treeobj_create_val_base64 (NULL) == NULL && errno == EINVAL, + "treeobj_create_val_base64 NULL fails "); + + json_decref (val); +} + void test_dirref (void) { json_t *dirref; @@ -449,6 +476,7 @@ int main(int argc, char** argv) test_valref (); test_val (); + test_val_base64 (); test_dirref (); test_dir (); test_symlink (); diff --git a/src/common/libkvs/treeobj.c b/src/common/libkvs/treeobj.c index 8a01db6daeb0..7966d82a6fda 100644 --- a/src/common/libkvs/treeobj.c +++ b/src/common/libkvs/treeobj.c @@ -348,14 +348,31 @@ json_t *treeobj_create_val (const void *data, int len) goto done; } base64_encode_block (xdata, &xlen, data, len); + + if (!(obj = treeobj_create_val_base64 (xdata))) + goto done; + +done: + free (xdata); + return obj; +} + +json_t *treeobj_create_val_base64 (const char *data) +{ + json_t *obj = NULL; + + if (!data) { + errno = EINVAL; + return NULL; + } + if (!(obj = json_pack ("{s:i s:s s:s}", "ver", treeobj_version, "type", "val", - "data", xdata))) { + "data", data))) { errno = ENOMEM; goto done; } done: - free (xdata); return obj; } diff --git a/src/common/libkvs/treeobj.h b/src/common/libkvs/treeobj.h index e1bba86a1d58..da99054a0da4 100644 --- a/src/common/libkvs/treeobj.h +++ b/src/common/libkvs/treeobj.h @@ -9,11 +9,13 @@ /* Create a treeobj * valref, dirref: if blobref is NULL, treeobj_append_blobref() * must be called before object is valid. - * val: copies argument (caller retains ownership) + * val & val_base64: copies argument (caller retains ownership) + * val_base64: user supplies base64 string * Return JSON object on success, NULL on failure with errno set. */ json_t *treeobj_create_symlink (const char *target); json_t *treeobj_create_val (const void *data, int len); +json_t *treeobj_create_val_base64 (const char *data); json_t *treeobj_create_valref (const char *blobref); json_t *treeobj_create_dir (void); json_t *treeobj_create_dirref (const char *blobref); From 6dfb88c5e4e6876dd4dab0d4b107a33a6ef28fbe Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 3 Aug 2017 11:06:30 -0700 Subject: [PATCH 13/27] modules/kvs: Update internal lookup API to RFC 11 --- src/modules/kvs/lookup.c | 125 +++++++++++++++++++++++++++++---------- 1 file changed, 95 insertions(+), 30 deletions(-) diff --git a/src/modules/kvs/lookup.c b/src/modules/kvs/lookup.c index e1a477cad996..74f3bd6abdf1 100644 --- a/src/modules/kvs/lookup.c +++ b/src/modules/kvs/lookup.c @@ -38,7 +38,7 @@ #include "src/common/libutil/blobref.h" #include "src/common/libutil/log.h" -#include "src/common/libkvs/jansson_dirent.h" +#include "src/common/libkvs/treeobj.h" #include "cache.h" @@ -209,7 +209,7 @@ static walk_level_t *walk_levels_push (lookup_t *lh, */ static bool walk (lookup_t *lh) { - json_t *dir, *ref, *link; + json_t *dir; walk_level_t *wl = NULL; char *pathcomp; @@ -220,8 +220,25 @@ static bool walk (lookup_t *lh) /* Get directory of dirent */ - if ((ref = json_object_get (wl->dirent, "DIRREF"))) { - const char *refstr = json_string_value (ref); + if (treeobj_is_dirref (wl->dirent)) { + const char *refstr; + int refcount; + + if ((refcount = treeobj_get_count (wl->dirent)) < 0) { + lh->errnum = errno; + goto error; + } + + if (refcount != 1) { + log_msg ("invalid dirref count: %d", refcount); + lh->errnum = EPERM; + goto error; + } + + if (!(refstr = treeobj_get_blobref (wl->dirent, 0))) { + lh->errnum = errno; + goto error; + } if (!(dir = cache_lookup_and_get_json (lh->cache, refstr, @@ -231,8 +248,8 @@ static bool walk (lookup_t *lh) } } else { /* Unexpected dirent type */ - if (json_object_get (wl->dirent, "FILEREF") - || json_object_get (wl->dirent, "FILEVAL")) { + if (treeobj_is_valref (wl->dirent) + || treeobj_is_val (wl->dirent)) { /* don't return ENOENT or ENOTDIR, error to be * determined by caller */ goto error; @@ -250,14 +267,27 @@ static bool walk (lookup_t *lh) /* Get directory reference of path component from directory */ - if (!(wl->dirent = json_object_get (dir, pathcomp))) - /* not necessarily ENOENT, let caller decide */ + if (!(wl->dirent = treeobj_get_entry (dir, pathcomp))) { + /* if entry does not exist, not necessarily ENOENT error, + * let caller decide. If error not ENOENT, return to + * caller. */ + if (errno != ENOENT) + lh->errnum = errno; goto error; + } /* Resolve dirent if it is a link */ - if ((link = json_object_get (wl->dirent, "LINKVAL"))) { - const char *linkstr = json_string_value (link); + if (treeobj_is_symlink (wl->dirent)) { + json_t *link; + const char *linkstr; + + if (!(link = treeobj_get_data (wl->dirent))) { + lh->errnum = errno; + goto error; + } + + linkstr = json_string_value (link); /* Follow link if in middle of path or if end of path, * flags say we can follow it @@ -369,7 +399,7 @@ lookup_t *lookup_create (struct cache *cache, lh->missing_ref = NULL; lh->errnum = 0; - if (!(lh->root_dirent = j_dirent_create ("DIRREF", lh->root_ref))) { + if (!(lh->root_dirent = treeobj_create_dirref (lh->root_ref))) { saved_errno = errno; goto cleanup; } @@ -519,8 +549,10 @@ int lookup_set_aux_data (lookup_t *lh, void *data) { bool lookup (lookup_t *lh) { - json_t *vp, *valtmp = NULL; + json_t *valtmp = NULL; const char *reftmp; + const char *strtmp; + int refcount; if (!lh || lh->magic != LOOKUP_MAGIC) { errno = EINVAL; @@ -533,8 +565,7 @@ bool lookup (lookup_t *lh) /* special case root */ if (!strcmp (lh->path, ".")) { if ((lh->flags & FLUX_KVS_TREEOBJ)) { - char *tmprootref = (char *)lh->root_dir; - if (!(lh->val = j_dirent_create ("DIRREF", tmprootref))) { + if (!(lh->val = treeobj_create_dirref (lh->root_dir))) { lh->errnum = errno; goto done; } @@ -576,7 +607,7 @@ bool lookup (lookup_t *lh) goto done; } - if ((vp = json_object_get (lh->wdirent, "DIRREF"))) { + if (treeobj_is_dirref (lh->wdirent)) { if ((lh->flags & FLUX_KVS_READLINK)) { lh->errnum = EINVAL; goto done; @@ -585,7 +616,19 @@ bool lookup (lookup_t *lh) lh->errnum = EISDIR; goto done; } - reftmp = json_string_value (vp); + if ((refcount = treeobj_get_count (lh->wdirent)) < 0) { + lh->errnum = errno; + goto done; + } + if (refcount != 1) { + log_msg ("invalid dirref count: %d", refcount); + lh->errnum = EPERM; + goto done; + } + if (!(reftmp = treeobj_get_blobref (lh->wdirent, 0))) { + lh->errnum = errno; + goto done; + } valtmp = cache_lookup_and_get_json (lh->cache, reftmp, lh->current_epoch); @@ -593,11 +636,13 @@ bool lookup (lookup_t *lh) lh->missing_ref = reftmp; goto stall; } - if (!(lh->val = json_copy (valtmp))) { - lh->errnum = ENOMEM; + if (!treeobj_is_dir (valtmp)) { + /* dirref points to not dir */ + lh->errnum = EPERM; goto done; } - } else if ((vp = json_object_get (lh->wdirent, "FILEREF"))) { + lh->val = json_incref (valtmp); + } else if (treeobj_is_valref (lh->wdirent)) { if ((lh->flags & FLUX_KVS_READLINK)) { lh->errnum = EINVAL; goto done; @@ -606,7 +651,19 @@ bool lookup (lookup_t *lh) lh->errnum = ENOTDIR; goto done; } - reftmp = json_string_value (vp); + if ((refcount = treeobj_get_count (lh->wdirent)) < 0) { + lh->errnum = errno; + goto done; + } + if (refcount != 1) { + log_msg ("invalid valref count: %d", refcount); + lh->errnum = EPERM; + goto done; + } + if (!(reftmp = treeobj_get_blobref (lh->wdirent, 0))) { + lh->errnum = errno; + goto done; + } valtmp = cache_lookup_and_get_json (lh->cache, reftmp, lh->current_epoch); @@ -614,8 +671,19 @@ bool lookup (lookup_t *lh) lh->missing_ref = reftmp; goto stall; } - lh->val = json_incref (valtmp); - } else if ((vp = json_object_get (lh->wdirent, "DIRVAL"))) { + if (!json_is_string (valtmp)) { + /* valref points to non-string */ + lh->errnum = EPERM; + goto done; + } + + /* Place base64 opaque data into treeobj val object */ + strtmp = json_string_value (valtmp); + if (!(lh->val = treeobj_create_val_base64 (strtmp))) { + lh->errnum = errno; + goto done; + } + } else if (treeobj_is_dir (lh->wdirent)) { if ((lh->flags & FLUX_KVS_READLINK)) { lh->errnum = EINVAL; goto done; @@ -624,11 +692,8 @@ bool lookup (lookup_t *lh) lh->errnum = EISDIR; goto done; } - if (!(lh->val = json_copy (vp))) { - lh->errnum = ENOMEM; - goto done; - } - } else if ((vp = json_object_get (lh->wdirent, "FILEVAL"))) { + lh->val = json_incref (lh->wdirent); + } else if (treeobj_is_val (lh->wdirent)) { if ((lh->flags & FLUX_KVS_READLINK)) { lh->errnum = EINVAL; goto done; @@ -637,8 +702,8 @@ bool lookup (lookup_t *lh) lh->errnum = ENOTDIR; goto done; } - lh->val = json_incref (vp); - } else if ((vp = json_object_get (lh->wdirent, "LINKVAL"))) { + lh->val = json_incref (lh->wdirent); + } else if (treeobj_is_symlink (lh->wdirent)) { /* this should be "impossible" */ if (!(lh->flags & FLUX_KVS_READLINK)) { lh->errnum = EPROTO; @@ -648,7 +713,7 @@ bool lookup (lookup_t *lh) lh->errnum = ENOTDIR; goto done; } - lh->val = json_incref (vp); + lh->val = json_incref (lh->wdirent); } else { char *s = json_dumps (lh->wdirent, 0); log_msg ("%s: corrupt dirent: %s", __FUNCTION__, s); From b0dcc4c434e4b71b5f42b77d37f507794c370f6d Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 3 Aug 2017 12:06:08 -0700 Subject: [PATCH 14/27] modules/kvs/test: Update lookup tests to RFC 11 Of particular note, treeobj library requires that blobrefs be "real" blobrefs, not made up ones (i.e. "root-ref" or "dir-ref"). So a fair amount of the code changes are to also ensure blobrefs are now "real" and generated based on actual objects. --- src/modules/kvs/test/lookup.c | 956 ++++++++++++++++++---------------- 1 file changed, 512 insertions(+), 444 deletions(-) diff --git a/src/modules/kvs/test/lookup.c b/src/modules/kvs/test/lookup.c index c2c580b0aee5..9dc5f8f30bb3 100644 --- a/src/modules/kvs/test/lookup.c +++ b/src/modules/kvs/test/lookup.c @@ -5,9 +5,12 @@ #include #include "src/common/libtap/tap.h" -#include "src/common/libkvs/jansson_dirent.h" +#include "src/common/libkvs/treeobj.h" #include "src/modules/kvs/cache.h" #include "src/modules/kvs/lookup.h" +#include "src/modules/kvs/kvs_util.h" +#include "src/modules/kvs/types.h" +#include "src/common/libutil/base64.h" void basic_api (void) { @@ -181,6 +184,21 @@ void basic_api_errors (void) cache_destroy (cache); } +json_t *get_json_base64_string (const char *s) +{ + int len, xlen; + char *xdata; + json_t *rv; + + len = strlen (s); + xlen = base64_encode_length (len); + xdata = malloc (xlen); + base64_encode_block (xdata, &xlen, s, len); + rv = json_string (xdata); + free (xdata); + return rv; +} + void check_common (lookup_t *lh, bool lookup_result, int get_errnum_result, @@ -273,25 +291,26 @@ void lookup_root (void) { json_t *test; struct cache *cache; lookup_t *lh; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); /* This cache is * - * root-ref - * { "dir" : { "DIRREF" : "dir-ref" } } + * root_ref + * treeobj dir, no entries */ - root = json_object (); - json_object_set_new (root, "dir", j_dirent_create ("DIRREF", "dir-ref")); - cache_insert (cache, "root-ref", cache_entry_create (root)); + root = treeobj_create_dir (); + kvs_util_json_hash ("sha1", root, root_ref); + cache_insert (cache, root_ref, cache_entry_create (root)); /* flags = 0, should error EISDIR */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, ".", 0)) != NULL, "lookup_create on root, no flags, works"); @@ -300,8 +319,8 @@ void lookup_root (void) { /* flags = FLUX_KVS_READDIR, should succeed */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, ".", FLUX_KVS_READDIR)) != NULL, "lookup_create on root w/ flag = FLUX_KVS_READDIR, works"); @@ -310,12 +329,12 @@ void lookup_root (void) { /* flags = FLUX_KVS_TREEOBJ, should succeed */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, ".", FLUX_KVS_TREEOBJ)) != NULL, "lookup_create on root w/ flag = FLUX_KVS_TREEOBJ, works"); - test = j_dirent_create ("DIRREF", "root-ref"); + test = treeobj_create_dirref (root_ref); check (lh, true, 0, test, NULL, "root w/ FLUX_KVS_TREEOBJ"); json_decref (test); @@ -326,162 +345,168 @@ void lookup_root (void) { void lookup_basic (void) { json_t *root; json_t *dirref; - json_t *dirval; - json_t *linkval; + json_t *dir; + json_t *opaque_data; json_t *test; struct cache *cache; lookup_t *lh; + href_t valref_ref; + href_t dirref_ref; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); /* This cache is * - * root-ref - * { "dir" : { "DIRREF" : "dir-ref" } } + * valref_ref + * "abcd" * - * dir-ref - * { "fileval" : { "FILEVAL" : 42 } - * "file" : { "FILEREF" : "file-ref" } - * "dirval" : { "DIRVAL" : { "foo" : { "FILEVAL" : 43 } } } - * "linkval" : { "LINKVAL" : "baz" } } + * dirref_ref + * "valref" : valref to valref_ref + * "val" : val to "foo" + * "dir" : dir w/ "val" : val to "bar" + * "symlink" : symlink to "baz" * - * file-ref - * { 44 } + * root_ref + * "dirref" : dirref to dirref_ref */ - root = json_object (); - json_object_set_new (root, "dir", j_dirent_create ("DIRREF", "dir-ref")); - cache_insert (cache, "root-ref", cache_entry_create (root)); + opaque_data = get_json_base64_string ("abcd"); + kvs_util_json_hash ("sha1", opaque_data, valref_ref); + cache_insert (cache, valref_ref, cache_entry_create (opaque_data)); - dirval = json_object (); - json_object_set_new (dirval, "foo", j_dirent_create ("FILEVAL", json_integer (43))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("bar", 3)); - linkval = j_dirent_create ("LINKVAL", json_string ("baz")); + dirref = treeobj_create_dir (); + treeobj_insert_entry (dirref, "valref", treeobj_create_valref (valref_ref)); + treeobj_insert_entry (dirref, "val", treeobj_create_val ("foo", 3)); + treeobj_insert_entry (dirref, "dir", dir); + treeobj_insert_entry (dirref, "symlink", treeobj_create_symlink ("baz")); - dirref = json_object (); - json_object_set_new (dirref, "fileval", j_dirent_create ("FILEVAL", json_integer (42))); - json_object_set_new (dirref, "file", j_dirent_create ("FILEREF", "file-ref")); - json_object_set_new (dirref, "dirval", j_dirent_create ("DIRVAL", dirval)); - json_object_set_new (dirref, "linkval", linkval); + kvs_util_json_hash ("sha1", dirref, dirref_ref); + cache_insert (cache, dirref_ref, cache_entry_create (dirref)); - cache_insert (cache, "dir-ref", cache_entry_create (dirref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dirref", treeobj_create_dirref (dirref_ref)); - cache_insert (cache, "file-ref", cache_entry_create (json_integer (44))); + kvs_util_json_hash ("sha1", root, root_ref); + cache_insert (cache, root_ref, cache_entry_create (root)); - /* lookup dir value */ + /* lookup dir via dirref */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir", + root_ref, + root_ref, + "dirref", FLUX_KVS_READDIR)) != NULL, - "lookup_create on path dir"); - check (lh, true, 0, dirref, NULL, "lookup dir"); + "lookup_create on path dirref"); + check (lh, true, 0, dirref, NULL, "lookup dirref"); - /* lookup file value */ + /* lookup value via valref */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.file", + root_ref, + root_ref, + "dirref.valref", 0)) != NULL, - "lookup_create on path dir.file"); - test = json_integer (44); - check (lh, true, 0, test, NULL, "lookup dir.file"); + "lookup_create on path dirref.valref"); + test = treeobj_create_val ("abcd", 4); + check (lh, true, 0, test, NULL, "lookup dirref.valref"); json_decref (test); - /* lookup fileval value */ + /* lookup value via val */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.fileval", + root_ref, + root_ref, + "dirref.val", 0)) != NULL, - "lookup_create on path dir.fileval"); - test = json_integer (42); - check (lh, true, 0, test, NULL, "lookup dir.fileval"); + "lookup_create on path dirref.val"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "lookup dirref.val"); json_decref (test); - /* lookup dirval value */ + /* lookup dir via dir */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.dirval", + root_ref, + root_ref, + "dirref.dir", FLUX_KVS_READDIR)) != NULL, - "lookup_create on path dir.dirval"); - check (lh, true, 0, dirval, NULL, "lookup dir.dirval"); + "lookup_create on path dirref.dir"); + check (lh, true, 0, dir, NULL, "lookup dirref.dir"); - /* lookup linkval value */ + /* lookup symlink */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.linkval", + root_ref, + root_ref, + "dirref.symlink", FLUX_KVS_READLINK)) != NULL, - "lookup_create on path dir.linkval"); - test = json_string ("baz"); - check (lh, true, 0, test, NULL, "lookup dir.linkval"); + "lookup_create on path dirref.symlink"); + test = treeobj_create_symlink ("baz"); + check (lh, true, 0, test, NULL, "lookup dirref.symlink"); json_decref (test); - /* lookup dir treeobj */ + /* lookup dirref treeobj */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir", + root_ref, + root_ref, + "dirref", FLUX_KVS_TREEOBJ)) != NULL, - "lookup_create on path dir (treeobj)"); - test = j_dirent_create ("DIRREF", "dir-ref"); - check (lh, true, 0, test, NULL, "lookup dir treeobj"); + "lookup_create on path dirref (treeobj)"); + test = treeobj_create_dirref (dirref_ref); + check (lh, true, 0, test, NULL, "lookup dirref treeobj"); json_decref (test); - /* lookup file treeobj */ + /* lookup valref treeobj */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.file", + root_ref, + root_ref, + "dirref.valref", FLUX_KVS_TREEOBJ)) != NULL, - "lookup_create on path dir.file (treeobj)"); - test = j_dirent_create ("FILEREF", "file-ref"); - check (lh, true, 0, test, NULL, "lookup dir.file treeobj"); + "lookup_create on path dirref.valref (treeobj)"); + test = treeobj_create_valref (valref_ref); + check (lh, true, 0, test, NULL, "lookup dirref.valref treeobj"); json_decref (test); - /* lookup fileval treeobj */ + /* lookup val treeobj */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.fileval", + root_ref, + root_ref, + "dirref.val", FLUX_KVS_TREEOBJ)) != NULL, - "lookup_create on path dir.fileval (treeobj)"); - test = j_dirent_create ("FILEVAL", json_integer (42)); - check (lh, true, 0, test, NULL, "lookup dir.fileval treeobj"); + "lookup_create on path dirref.val (treeobj)"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "lookup dirref.val treeobj"); json_decref (test); - /* lookup dirval treeobj */ + /* lookup dir treeobj */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.dirval", + root_ref, + root_ref, + "dirref.dir", FLUX_KVS_TREEOBJ)) != NULL, - "lookup_create on path dir.dirval (treeobj)"); - test = j_dirent_create ("DIRVAL", dirval); - check (lh, true, 0, test, NULL, "lookup dir.dirval treeobj"); - json_decref (test); + "lookup_create on path dirref.dir (treeobj)"); + check (lh, true, 0, dir, NULL, "lookup dirref.dir treeobj"); - /* lookup linkval treeobj */ + /* lookup symlink treeobj */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir.linkval", + root_ref, + root_ref, + "dirref.symlink", FLUX_KVS_TREEOBJ)) != NULL, - "lookup_create on path dir.linkval (treeobj)"); - check (lh, true, 0, linkval, NULL, "lookup dir.linkval treeobj"); + "lookup_create on path dirref.symlink (treeobj)"); + test = treeobj_create_symlink ("baz"); + check (lh, true, 0, test, NULL, "lookup dirref.symlink treeobj"); + json_decref (test); cache_destroy (cache); } @@ -489,88 +514,110 @@ void lookup_basic (void) { /* lookup tests reach an error or "non-good" result */ void lookup_errors (void) { json_t *root; - json_t *dirval; + json_t *dirref; + json_t *dir; + json_t *opaque_data; struct cache *cache; lookup_t *lh; + href_t dirref_ref; + href_t valref_ref; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); /* This cache is * - * root-ref - * { "dirref" : { "DIRREF" : "dirref-ref" }, - * "fileref" : { "FILEREF" : "fileref-ref" } - * "dirval" : { "DIRVAL" : { "foo" : { "FILEVAL" : 42 } } } - * "fileval" : { "FILEVAL" : 42 } - * "linkval" : { "LINKVAL" : "linkvalstr" } - * "linkval1" : { "LINKVAL" : "linkval2" } - * "linkval2" : { "LINKVAL" : "linkval1" } } + * valref_ref + * "abcd" + * + * dirref_ref + * "val" : val to "bar" + * + * root_ref + * "symlink" : symlink to "symlinkstr" + * "symlink1" : symlink to "symlink2" + * "symlink2" : symlink to "symlink1" + * "val" : val to "foo" + * "valref" : valref to valref_ref + * "dirref" : dirref to dirref_ref + * "dir" : dir w/ "val" : val to "baz" + * */ - dirval = json_object (); - json_object_set_new (dirval, "foo", j_dirent_create ("FILEVAL", json_integer (42))); + opaque_data = get_json_base64_string ("abcd"); + kvs_util_json_hash ("sha1", opaque_data, valref_ref); + cache_insert (cache, valref_ref, cache_entry_create (opaque_data)); + + dirref = treeobj_create_dir (); + treeobj_insert_entry (dirref, "val", treeobj_create_val ("bar", 3)); + kvs_util_json_hash ("sha1", dirref, dirref_ref); + cache_insert (cache, dirref_ref, cache_entry_create (dirref)); - root = json_object (); - json_object_set_new (root, "dirref", j_dirent_create ("DIRREF", "dirref-ref")); - json_object_set_new (root, "fileref", j_dirent_create ("FILEREF", "fileref-ref")); - json_object_set_new (root, "dirval", j_dirent_create ("DIRVAL", dirval)); - json_object_set_new (root, "fileval", j_dirent_create ("FILEVAL", json_integer (42))); - json_object_set_new (root, "linkval", j_dirent_create ("LINKVAL", json_string ("linkvalstr"))); - json_object_set_new (root, "linkval1", j_dirent_create ("LINKVAL", json_string ("linkval2"))); - json_object_set_new (root, "linkval2", j_dirent_create ("LINKVAL", json_string ("linkval1"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("baz", 3)); - cache_insert (cache, "root-ref", cache_entry_create (root)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "symlink", treeobj_create_symlink ("symlinkstr")); + treeobj_insert_entry (root, "symlink1", treeobj_create_symlink ("symlink2")); + treeobj_insert_entry (root, "symlink2", treeobj_create_symlink ("symlink1")); + treeobj_insert_entry (root, "val", treeobj_create_val ("foo", 3)); + treeobj_insert_entry (root, "valref", treeobj_create_valref (valref_ref)); + treeobj_insert_entry (root, "dirref", treeobj_create_dirref (dirref_ref)); + treeobj_insert_entry (root, "dir", dir); + + kvs_util_json_hash ("sha1", root, root_ref); + cache_insert (cache, root_ref, cache_entry_create (root)); /* Lookup non-existent field. Not ENOENT - caller of lookup * decides what to do with entry not found */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, "foo", 0)) != NULL, "lookup_create on bad path in path"); check (lh, true, 0, NULL, NULL, "lookup bad path"); - /* Lookup path w/ fileval in middle, Not ENOENT - caller of lookup + /* Lookup path w/ val in middle, Not ENOENT - caller of lookup * decides what to do with entry not found */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "fileval.foo", + root_ref, + root_ref, + "val.foo", 0)) != NULL, - "lookup_create on fileval in path"); - check (lh, true, 0, NULL, NULL, "lookup fileval in path"); + "lookup_create on val in path"); + check (lh, true, 0, NULL, NULL, "lookup val in path"); - /* Lookup path w/ fileref in middle, Not ENOENT - caller of lookup + /* Lookup path w/ valref in middle, Not ENOENT - caller of lookup * decides what to do with entry not found */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "fileref.foo", + root_ref, + root_ref, + "valref.foo", 0)) != NULL, - "lookup_create on fileref in path"); - check (lh, true, 0, NULL, NULL, "lookup fileref in path"); + "lookup_create on valref in path"); + check (lh, true, 0, NULL, NULL, "lookup valref in path"); - /* Lookup path w/ dirval in middle, should get EPERM */ + /* Lookup path w/ dir in middle, should get EPERM */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dirval.foo", + root_ref, + root_ref, + "dir.foo", 0)) != NULL, - "lookup_create on dirval in path"); - check (lh, true, EPERM, NULL, NULL, "lookup dirval in path"); + "lookup_create on dir in path"); + check (lh, true, EPERM, NULL, NULL, "lookup dir in path"); /* Lookup path w/ infinite link loop, should get ELOOP */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "linkval1", + root_ref, + root_ref, + "symlink1", 0)) != NULL, "lookup_create on link loop"); check (lh, true, ELOOP, NULL, NULL, "lookup infinite links"); @@ -578,92 +625,92 @@ void lookup_errors (void) { /* Lookup a dirref, but expecting a link, should get EINVAL. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, "dirref", FLUX_KVS_READLINK)) != NULL, "lookup_create on dirref"); check (lh, true, EINVAL, NULL, NULL, "lookup dirref, expecting link"); - /* Lookup a dirval, but expecting a link, should get EINVAL. */ + /* Lookup a dir, but expecting a link, should get EINVAL. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dirval", + root_ref, + root_ref, + "dir", FLUX_KVS_READLINK)) != NULL, - "lookup_create on dirval"); - check (lh, true, EINVAL, NULL, NULL, "lookup dirval, expecting link"); + "lookup_create on dir"); + check (lh, true, EINVAL, NULL, NULL, "lookup dir, expecting link"); - /* Lookup a fileref, but expecting a link, should get EINVAL. */ + /* Lookup a valref, but expecting a link, should get EINVAL. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "fileref", + root_ref, + root_ref, + "valref", FLUX_KVS_READLINK)) != NULL, - "lookup_create on fileref"); - check (lh, true, EINVAL, NULL, NULL, "lookup fileref, expecting link"); + "lookup_create on valref"); + check (lh, true, EINVAL, NULL, NULL, "lookup valref, expecting link"); - /* Lookup a fileval, but expecting a link, should get EINVAL. */ + /* Lookup a val, but expecting a link, should get EINVAL. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "fileval", + root_ref, + root_ref, + "val", FLUX_KVS_READLINK)) != NULL, - "lookup_create on fileval"); - check (lh, true, EINVAL, NULL, NULL, "lookup fileval, expecting link"); + "lookup_create on val"); + check (lh, true, EINVAL, NULL, NULL, "lookup val, expecting link"); /* Lookup a dirref, but don't expect a dir, should get EISDIR. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, "dirref", 0)) != NULL, "lookup_create on dirref"); check (lh, true, EISDIR, NULL, NULL, "lookup dirref, not expecting dirref"); - /* Lookup a dirval, but don't expect a dir, should get EISDIR. */ + /* Lookup a dir, but don't expect a dir, should get EISDIR. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dirval", + root_ref, + root_ref, + "dir", 0)) != NULL, - "lookup_create on dirval"); - check (lh, true, EISDIR, NULL, NULL, "lookup dirval, not expecting dirval"); + "lookup_create on dir"); + check (lh, true, EISDIR, NULL, NULL, "lookup dir, not expecting dir"); - /* Lookup a fileref, but expecting a dir, should get ENOTDIR. */ + /* Lookup a valref, but expecting a dir, should get ENOTDIR. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "fileref", + root_ref, + root_ref, + "valref", FLUX_KVS_READDIR)) != NULL, - "lookup_create on fileref"); - check (lh, true, ENOTDIR, NULL, NULL, "lookup fileref, expecting dir"); + "lookup_create on valref"); + check (lh, true, ENOTDIR, NULL, NULL, "lookup valref, expecting dir"); - /* Lookup a fileval, but expecting a dir, should get ENOTDIR. */ + /* Lookup a val, but expecting a dir, should get ENOTDIR. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "fileval", + root_ref, + root_ref, + "val", FLUX_KVS_READDIR)) != NULL, - "lookup_create on fileval"); - check (lh, true, ENOTDIR, NULL, NULL, "lookup fileval, expecting dir"); + "lookup_create on val"); + check (lh, true, ENOTDIR, NULL, NULL, "lookup val, expecting dir"); - /* Lookup a linkval, but expecting a dir, should get ENOTDIR. */ + /* Lookup a symlink, but expecting a dir, should get ENOTDIR. */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "linkval", + root_ref, + root_ref, + "symlink", FLUX_KVS_READLINK | FLUX_KVS_READDIR)) != NULL, - "lookup_create on linkval"); - check (lh, true, ENOTDIR, NULL, NULL, "lookup linkval, expecting dir"); + "lookup_create on symlink"); + check (lh, true, ENOTDIR, NULL, NULL, "lookup symlink, expecting dir"); cache_destroy (cache); } @@ -671,202 +718,208 @@ void lookup_errors (void) { /* lookup link tests */ void lookup_links (void) { json_t *root; - json_t *dir1ref; - json_t *dir2ref; - json_t *dir3ref; - json_t *dirval; - json_t *linkval; + json_t *dirref1; + json_t *dirref2; + json_t *dirref3; + json_t *dir; + json_t *opaque_data; json_t *test; struct cache *cache; lookup_t *lh; + href_t valref_ref; + href_t dirref3_ref; + href_t dirref2_ref; + href_t dirref1_ref; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); /* This cache is * - * root-ref - * { "dir1" : { "DIRREF" : "dir1-ref" } - * "dir2" : { "DIRREF" : "dir2-ref" } } + * opaque_data + * "abcd" * - * dir1-ref - * { "link2dir" : { "LINKVAL" : "dir2" } - * "link2fileval" : { "LINKVAL" : "dir2.fileval" } - * "link2file" : { "LINKVAL" : "dir2.file" } - * "link2dirval" : { "LINKVAL" : "dir2.dirval" } - * "link2linkval" : { "LINKVAL" : "dir2.linkval" } } + * dirref3_ref + * "val" : val to "baz" * - * dir2-ref - * { "fileval" : { "FILEVAL" : 42 } - * "file" : { "FILEREF" : "file-ref" } - * "dirval" : { "DIRVAL" : { "foo" : { "FILEVAL" : 43 } } } - * "dir" : { "DIRREF" : "dir3-ref" } - * "linkval" : { "LINKVAL" : "dir2.fileval" } } + * dirref2_ref + * "val" : val to "foo" + * "valref" : valref to valref_ref + * "dir" : dir w/ "val" : val to "bar" + * "dirref" : dirref to dirref3_ref + * "symlink" : symlink to "dirref2.val" * - * dir3-ref - * { "fileval" : { "FILEVAL" : 44 } } + * dirref1_ref + * "link2dirref" : symlink to "dirref2" + * "link2val" : symlink to "dirref2.val" + * "link2valref" : symlink to "dirref2.valref" + * "link2dir" : symlink to "dirref2.dir" + * "link2symlink" : symlink to "dirref2.symlink" * - * file-ref - * { 45 } + * root_ref + * "dirref1" : dirref to "dirref1_ref + * "dirref2" : dirref to "dirref2_ref */ - root = json_object (); - json_object_set_new (root, "dir1", j_dirent_create ("DIRREF", "dir1-ref")); - json_object_set_new (root, "dir2", j_dirent_create ("DIRREF", "dir2-ref")); - cache_insert (cache, "root-ref", cache_entry_create (root)); - - dir1ref = json_object (); - json_object_set_new (dir1ref, "link2dir", j_dirent_create ("LINKVAL", json_string ("dir2"))); - json_object_set_new (dir1ref, "link2fileval", j_dirent_create ("LINKVAL", json_string ("dir2.fileval"))); - json_object_set_new (dir1ref, "link2file", j_dirent_create ("LINKVAL", json_string ("dir2.file"))); - json_object_set_new (dir1ref, "link2dirval", j_dirent_create ("LINKVAL", json_string ("dir2.dirval"))); - json_object_set_new (dir1ref, "link2linkval", j_dirent_create ("LINKVAL", json_string ("dir2.linkval"))); - - cache_insert (cache, "dir1-ref", cache_entry_create (dir1ref)); - - dirval = json_object (); - json_object_set_new (dirval, "foo", j_dirent_create ("FILEVAL", json_integer (43))); - - linkval = j_dirent_create ("LINKVAL", json_string ("dir2.fileval")); - - dir2ref = json_object (); - json_object_set_new (dir2ref, "fileval", j_dirent_create ("FILEVAL", json_integer (42))); - json_object_set_new (dir2ref, "file", j_dirent_create ("FILEREF", "file-ref")); - json_object_set_new (dir2ref, "dirval", j_dirent_create ("DIRVAL", dirval)); - json_object_set_new (dir2ref, "dir", j_dirent_create ("DIRREF", "dir3-ref")); - json_object_set_new (dir2ref, "linkval", linkval); - - cache_insert (cache, "dir2-ref", cache_entry_create (dir2ref)); - - dir3ref = json_object (); - json_object_set_new (dir3ref, "fileval", j_dirent_create ("FILEVAL", json_integer (44))); - - cache_insert (cache, "dir3-ref", cache_entry_create (dir3ref)); - - cache_insert (cache, "file-ref", cache_entry_create (json_integer (45))); - - /* lookup fileval, follow two links */ + opaque_data = get_json_base64_string ("abcd"); + kvs_util_json_hash ("sha1", opaque_data, valref_ref); + cache_insert (cache, valref_ref, cache_entry_create (opaque_data)); + + dirref3 = treeobj_create_dir (); + treeobj_insert_entry (dirref3, "val", treeobj_create_val ("baz", 3)); + kvs_util_json_hash ("sha1", dirref3, dirref3_ref); + cache_insert (cache, dirref3_ref, cache_entry_create (dirref3)); + + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("bar", 3)); + + dirref2 = treeobj_create_dir (); + treeobj_insert_entry (dirref2, "val", treeobj_create_val ("foo", 3)); + treeobj_insert_entry (dirref2, "valref", treeobj_create_valref (valref_ref)); + treeobj_insert_entry (dirref2, "dir", dir); + treeobj_insert_entry (dirref2, "dirref", treeobj_create_dirref (dirref3_ref)); + treeobj_insert_entry (dirref2, "symlink", treeobj_create_symlink ("dirref2.val")); + kvs_util_json_hash ("sha1", dirref2, dirref2_ref); + cache_insert (cache, dirref2_ref, cache_entry_create (dirref2)); + + dirref1 = treeobj_create_dir (); + treeobj_insert_entry (dirref1, "link2dirref", treeobj_create_symlink ("dirref2")); + treeobj_insert_entry (dirref1, "link2val", treeobj_create_symlink ("dirref2.val")); + treeobj_insert_entry (dirref1, "link2valref", treeobj_create_symlink ("dirref2.valref")); + treeobj_insert_entry (dirref1, "link2dir", treeobj_create_symlink ("dirref2.dir")); + treeobj_insert_entry (dirref1, "link2symlink", treeobj_create_symlink ("dirref2.symlink")); + kvs_util_json_hash ("sha1", dirref1, dirref1_ref); + cache_insert (cache, dirref1_ref, cache_entry_create (dirref1)); + + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dirref1", treeobj_create_dirref (dirref1_ref)); + treeobj_insert_entry (root, "dirref2", treeobj_create_dirref (dirref2_ref)); + kvs_util_json_hash ("sha1", root, root_ref); + cache_insert (cache, root_ref, cache_entry_create (root)); + + /* lookup val, follow two links */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir.linkval", + root_ref, + root_ref, + "dirref1.link2dirref.symlink", 0)) != NULL, - "lookup_create link to fileval via two links"); - test = json_integer (42); - check (lh, true, 0, test, NULL, "fileval via two links"); + "lookup_create link to val via two links"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "val via two links"); json_decref (test); - /* lookup fileval, link is middle of path */ + /* lookup val, link is middle of path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir.fileval", + root_ref, + root_ref, + "dirref1.link2dirref.val", 0)) != NULL, - "lookup_create link to fileval"); - test = json_integer (42); - check (lh, true, 0, test, NULL, "dir1.link2dir.fileval"); + "lookup_create link to val"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "dirref1.link2dirref.val"); json_decref (test); - /* lookup file, link is middle of path */ + /* lookup valref, link is middle of path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir.file", + root_ref, + root_ref, + "dirref1.link2dirref.valref", 0)) != NULL, - "lookup_create link to file"); - test = json_integer (45); - check (lh, true, 0, test, NULL, "dir1.link2dir.file"); + "lookup_create link to valref"); + test = treeobj_create_val ("abcd", 4); + check (lh, true, 0, test, NULL, "dirref1.link2dirref.valref"); json_decref (test); - /* lookup dirval, link is middle of path */ + /* lookup dir, link is middle of path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir.dirval", + root_ref, + root_ref, + "dirref1.link2dirref.dir", FLUX_KVS_READDIR)) != NULL, - "lookup_create link to dirval"); - check (lh, true, 0, dirval, NULL, "dir1.link2dir.dirval"); + "lookup_create link to dir"); + check (lh, true, 0, dir, NULL, "dirref1.link2dirref.dir"); - /* lookup dir, link is middle of path */ + /* lookup dirref, link is middle of path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir.dir", + root_ref, + root_ref, + "dirref1.link2dirref.dirref", FLUX_KVS_READDIR)) != NULL, - "lookup_create link to dir"); - check (lh, true, 0, dir3ref, NULL, "dir1.link2dir.dir"); + "lookup_create link to dirref"); + check (lh, true, 0, dirref3, NULL, "dirref1.link2dirref.dirref"); - /* lookup linkval, link is middle of path */ + /* lookup symlink, link is middle of path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir.linkval", + root_ref, + root_ref, + "dirref1.link2dirref.symlink", FLUX_KVS_READLINK)) != NULL, - "lookup_create link to linkval"); - test = json_string ("dir2.fileval"); - check (lh, true, 0, test, NULL, "dir1.link2dir.linkval"); + "lookup_create link to symlink"); + test = treeobj_create_symlink ("dirref2.val"); + check (lh, true, 0, test, NULL, "dirref1.link2dirref.symlink"); json_decref (test); - /* lookup fileval, link is last part in path */ + /* lookup val, link is last part in path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2fileval", + root_ref, + root_ref, + "dirref1.link2val", 0)) != NULL, - "lookup_create link to fileval (last part path)"); - test = json_integer (42); - check (lh, true, 0, test, NULL, "dir1.link2fileval"); + "lookup_create link to val (last part path)"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "dirref1.link2val"); json_decref (test); - /* lookup file, link is last part in path */ + /* lookup valref, link is last part in path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2file", + root_ref, + root_ref, + "dirref1.link2valref", 0)) != NULL, - "lookup_create link to file (last part path)"); - test = json_integer (45); - check (lh, true, 0, test, NULL, "dir1.link2file"); + "lookup_create link to valref (last part path)"); + test = treeobj_create_val ("abcd", 4); + check (lh, true, 0, test, NULL, "dirref1.link2valref"); json_decref (test); - /* lookup dirval, link is last part in path */ + /* lookup dir, link is last part in path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dirval", + root_ref, + root_ref, + "dirref1.link2dir", FLUX_KVS_READDIR)) != NULL, - "lookup_create link to dirval (last part path)"); - check (lh, true, 0, dirval, NULL, "dir1.link2dirval"); + "lookup_create link to dir (last part path)"); + check (lh, true, 0, dir, NULL, "dirref1.link2dir"); - /* lookup dir, link is last part in path */ + /* lookup dirref, link is last part in path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2dir", + root_ref, + root_ref, + "dirref1.link2dirref", FLUX_KVS_READDIR)) != NULL, - "lookup_create link to dir (last part path)"); - check (lh, true, 0, dir2ref, NULL, "dir1.link2dir"); + "lookup_create link to dirref (last part path)"); + check (lh, true, 0, dirref2, NULL, "dirref1.link2dirref"); - /* lookup linkval, link is last part in path */ + /* lookup symlink, link is last part in path */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.link2linkval", + root_ref, + root_ref, + "dirref1.link2symlink", FLUX_KVS_READLINK)) != NULL, - "lookup_create link to linkval (last part path)"); - test = json_string ("dir2.linkval"); - check (lh, true, 0, test, NULL, "dir1.link2linkval"); + "lookup_create link to symlink (last part path)"); + test = treeobj_create_symlink ("dirref2.symlink"); + check (lh, true, 0, test, NULL, "dirref1.link2symlink"); json_decref (test); cache_destroy (cache); @@ -875,63 +928,68 @@ void lookup_links (void) { /* lookup alternate root tests */ void lookup_alt_root (void) { json_t *root; - json_t *dir1ref; - json_t *dir2ref; + json_t *dirref1; + json_t *dirref2; json_t *test; struct cache *cache; lookup_t *lh; - + href_t dirref1_ref; + href_t dirref2_ref; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); /* This cache is * - * root-ref - * { "dir1" : { "DIRREF" : "dir1-ref" } - * "dir2" : { "DIRREF" : "dir2-ref" } } + * dirref1_ref + * "val" to "foo" * - * dir1-ref - * { "fileval" : { "FILEVAL" : 42 } } + * dirref2_ref + * "val" to "bar" * - * dir2-ref - * { "fileval" : { "FILEVAL" : 43 } } + * root_ref + * "dirref1" : dirref to dirref1_ref + * "dirref2" : dirref to dirref2_ref */ - root = json_object (); - json_object_set_new (root, "dir1", j_dirent_create ("DIRREF", "dir1-ref")); - json_object_set_new (root, "dir2", j_dirent_create ("DIRREF", "dir2-ref")); - cache_insert (cache, "root-ref", cache_entry_create (root)); + dirref1 = treeobj_create_dir (); + treeobj_insert_entry (dirref1, "val", treeobj_create_val ("foo", 3)); + kvs_util_json_hash ("sha1", dirref1, dirref1_ref); + cache_insert (cache, dirref1_ref, cache_entry_create (dirref1)); - dir1ref = json_object (); - json_object_set_new (dir1ref, "fileval", j_dirent_create ("FILEVAL", json_integer (42))); - cache_insert (cache, "dir1-ref", cache_entry_create (dir1ref)); + dirref2 = treeobj_create_dir (); + treeobj_insert_entry (dirref2, "val", treeobj_create_val ("bar", 3)); + kvs_util_json_hash ("sha1", dirref2, dirref2_ref); + cache_insert (cache, dirref2_ref, cache_entry_create (dirref2)); - dir2ref = json_object (); - json_object_set_new (dir2ref, "fileval", j_dirent_create ("FILEVAL", json_integer (43))); - cache_insert (cache, "dir2-ref", cache_entry_create (dir2ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dirref1", treeobj_create_dirref (dirref1_ref)); + treeobj_insert_entry (root, "dirref2", treeobj_create_dirref (dirref2_ref)); + kvs_util_json_hash ("sha1", root, root_ref); + cache_insert (cache, root_ref, cache_entry_create (root)); - /* lookup fileval, alt root-ref dir1-ref */ + /* lookup val, alt root-ref dirref1_ref */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "dir1-ref", - "fileval", + root_ref, + dirref1_ref, + "val", 0)) != NULL, - "lookup_create fileval w/ dir1ref root_ref"); - test = json_integer (42); - check (lh, true, 0, test, NULL, "alt root fileval"); + "lookup_create val w/ dirref1 root_ref"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "alt root val"); json_decref (test); - /* lookup fileval, alt root-ref dir2-ref */ + /* lookup val, alt root-ref dirref2_ref */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "dir2-ref", - "fileval", + root_ref, + dirref2_ref, + "val", 0)) != NULL, - "lookup_create fileval w/ dir2ref root_ref"); - test = json_integer (43); - check (lh, true, 0, test, NULL, "alt root fileval"); + "lookup_create val w/ dirref2 root_ref"); + test = treeobj_create_val ("bar", 3); + check (lh, true, 0, test, NULL, "alt root val"); json_decref (test); cache_destroy (cache); @@ -942,6 +1000,7 @@ void lookup_stall_root (void) { json_t *root; struct cache *cache; lookup_t *lh; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); @@ -952,22 +1011,23 @@ void lookup_stall_root (void) { * { "dir" : { "DIRREF" : "dir-ref" } } */ - root = json_object (); - json_object_set_new (root, "dir", j_dirent_create ("DIRREF", "dir-ref")); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "val", treeobj_create_val ("foo", 3)); + kvs_util_json_hash ("sha1", root, root_ref); /* do not insert entries into cache until later for these stall tests */ /* lookup root ".", should stall on root */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, ".", FLUX_KVS_READDIR)) != NULL, "lookup_create stalltest \".\""); - check_stall (lh, false, EAGAIN, NULL, "root-ref", "root \".\" stall"); + check_stall (lh, false, EAGAIN, NULL, root_ref, "root \".\" stall"); - cache_insert (cache, "root-ref", cache_entry_create (root)); + cache_insert (cache, root_ref, cache_entry_create (root)); /* lookup root ".", should succeed */ check (lh, true, 0, root, NULL, "root \".\" #1"); @@ -975,8 +1035,8 @@ void lookup_stall_root (void) { /* lookup root ".", now fully cached, should succeed */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", + root_ref, + root_ref, ".", FLUX_KVS_READDIR)) != NULL, "lookup_create stalltest \".\""); @@ -988,141 +1048,149 @@ void lookup_stall_root (void) { /* lookup stall tests */ void lookup_stall (void) { json_t *root; - json_t *dir1ref; - json_t *dir2ref; - json_t *fileref; + json_t *dirref1; + json_t *dirref2; + json_t *opaque_data; json_t *test; struct cache *cache; lookup_t *lh; + href_t valref_ref; + href_t dirref1_ref; + href_t dirref2_ref; + href_t root_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); /* This cache is * - * root-ref - * { "dir1" : { "DIRREF" : "dir1-ref" } - * "dir2" : { "DIRREF" : "dir2-ref" } - * "linkval" : { "LINKVAL" : "dir2" } } + * valref_ref + * "abcd" * - * dir1-ref - * { "fileval" : { "FILEVAL" : 42 } - * "file" : { "FILEREF" : "file-ref" } } + * dirref1_ref + * "val" : val to "foo" + * "valref" : valref to valref_ref * - * dir2-ref - * { "fileval" : { "FILEVAL" : 43 } } + * dirref2_ref + * "val" : val to "bar" * - * file-ref - * { 44 } + * root_ref + * "symlink" : symlink to "dirref2" + * "dirref1" : dirref to dirref1_ref + * "dirref2" : dirref to dirref2_ref * */ - root = json_object (); - json_object_set_new (root, "dir1", j_dirent_create ("DIRREF", "dir1-ref")); - json_object_set_new (root, "dir2", j_dirent_create ("DIRREF", "dir2-ref")); - json_object_set_new (root, "linkval", j_dirent_create ("LINKVAL", json_string ("dir2"))); + opaque_data = get_json_base64_string ("abcd"); + kvs_util_json_hash ("sha1", opaque_data, valref_ref); - dir1ref = json_object (); - json_object_set_new (dir1ref, "fileval", j_dirent_create ("FILEVAL", json_integer (42))); - json_object_set_new (dir1ref, "file", j_dirent_create ("FILEREF", "file-ref")); + dirref1 = treeobj_create_dir (); + treeobj_insert_entry (dirref1, "val", treeobj_create_val ("foo", 3)); + treeobj_insert_entry (dirref1, "valref", treeobj_create_valref (valref_ref)); + kvs_util_json_hash ("sha1", dirref1, dirref1_ref); - dir2ref = json_object (); - json_object_set_new (dir2ref, "fileval", j_dirent_create ("FILEVAL", json_integer (43))); + dirref2 = treeobj_create_dir (); + treeobj_insert_entry (dirref2, "val", treeobj_create_val ("bar", 3)); + kvs_util_json_hash ("sha1", dirref2, dirref2_ref); - fileref = json_integer (44); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dirref1", treeobj_create_dirref (dirref1_ref)); + treeobj_insert_entry (root, "dirref2", treeobj_create_dirref (dirref2_ref)); + treeobj_insert_entry (root, "symlink", treeobj_create_symlink ("dirref2")); + kvs_util_json_hash ("sha1", root, root_ref); /* do not insert entries into cache until later for these stall tests */ - /* lookup dir1.fileval, should stall on root */ + /* lookup dirref1.val, should stall on root */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.fileval", + root_ref, + root_ref, + "dirref1.val", 0)) != NULL, - "lookup_create stalltest dir1.fileval"); - check_stall (lh, false, EAGAIN, NULL, "root-ref", "dir1.fileval stall #1"); + "lookup_create stalltest dirref1.val"); + check_stall (lh, false, EAGAIN, NULL, root_ref, "dirref1.val stall #1"); - cache_insert (cache, "root-ref", cache_entry_create (root)); + cache_insert (cache, root_ref, cache_entry_create (root)); /* next call to lookup, should stall */ - check_stall (lh, false, EAGAIN, NULL, "dir1-ref", "dir1.fileval stall #2"); + check_stall (lh, false, EAGAIN, NULL, dirref1_ref, "dirref1.val stall #2"); - cache_insert (cache, "dir1-ref", cache_entry_create (dir1ref)); + cache_insert (cache, dirref1_ref, cache_entry_create (dirref1)); /* final call to lookup, should succeed */ - test = json_integer (42); - check (lh, true, 0, test, NULL, "dir1.fileval #1"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "dirref1.val #1"); json_decref (test); - /* lookup dir1.fileval, now fully cached, should succeed */ + /* lookup dirref1.val, now fully cached, should succeed */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.fileval", + root_ref, + root_ref, + "dirref1.val", 0)) != NULL, - "lookup_create dir1.fileval"); - test = json_integer (42); - check (lh, true, 0, test, NULL, "dir1.fileval #2"); + "lookup_create dirref1.val"); + test = treeobj_create_val ("foo", 3); + check (lh, true, 0, test, NULL, "dirref1.val #2"); json_decref (test); - /* lookup linkval.fileval, should stall */ + /* lookup symlink.val, should stall */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "linkval.fileval", + root_ref, + root_ref, + "symlink.val", 0)) != NULL, - "lookup_create stalltest linkval.fileval"); - check_stall (lh, false, EAGAIN, NULL, "dir2-ref", "linkval.fileval stall"); + "lookup_create stalltest symlink.val"); + check_stall (lh, false, EAGAIN, NULL, dirref2_ref, "symlink.val stall"); - cache_insert (cache, "dir2-ref", cache_entry_create (dir2ref)); + cache_insert (cache, dirref2_ref, cache_entry_create (dirref2)); - /* lookup linkval.fileval, should succeed */ - test = json_integer (43); - check (lh, true, 0, test, NULL, "linkval.fileval #1"); + /* lookup symlink.val, should succeed */ + test = treeobj_create_val ("bar", 3); + check (lh, true, 0, test, NULL, "symlink.val #1"); json_decref (test); - /* lookup linkval.fileval, now fully cached, should succeed */ + /* lookup symlink.val, now fully cached, should succeed */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "linkval.fileval", + root_ref, + root_ref, + "symlink.val", 0)) != NULL, - "lookup_create linkval.fileval"); - test = json_integer (43); - check (lh, true, 0, test, NULL, "linkval.fileval #2"); + "lookup_create symlink.val"); + test = treeobj_create_val ("bar", 3); + check (lh, true, 0, test, NULL, "symlink.val #2"); json_decref (test); - /* lookup dir1.file, should stall */ + /* lookup dirref1.valref, should stall */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.file", + root_ref, + root_ref, + "dirref1.valref", 0)) != NULL, - "lookup_create stalltest dir1.file"); - check_stall (lh, false, EAGAIN, NULL, "file-ref", "dir1.file stall"); + "lookup_create stalltest dirref1.valref"); + check_stall (lh, false, EAGAIN, NULL, valref_ref, "dirref1.valref stall"); - cache_insert (cache, "file-ref", cache_entry_create (fileref)); + cache_insert (cache, valref_ref, cache_entry_create (opaque_data)); - /* lookup dir1.file, should succeed */ - test = json_integer (44); - check (lh, true, 0, test, NULL, "dir1.file #1"); + /* lookup dirref1.valref, should succeed */ + test = treeobj_create_val ("abcd", 4); + check (lh, true, 0, test, NULL, "dirref1.valref #1"); json_decref (test); - /* lookup dir1.file, now fully cached, should succeed */ + /* lookup dirref1.valref, now fully cached, should succeed */ ok ((lh = lookup_create (cache, 1, - "root-ref", - "root-ref", - "dir1.file", + root_ref, + root_ref, + "dirref1.valref", 0)) != NULL, - "lookup_create stalltest dir1.file"); - test = json_integer (44); - check (lh, true, 0, test, NULL, "dir1.file #2"); + "lookup_create stalltest dirref1.valref"); + test = treeobj_create_val ("abcd", 4); + check (lh, true, 0, test, NULL, "dirref1.valref #2"); json_decref (test); cache_destroy (cache); From 5369e7eb9c60903c67b3c363d8560b07754fea32 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 4 Aug 2017 15:28:51 -0700 Subject: [PATCH 15/27] modules/kvs/test: Add new lookup tests Add new unit tests, detecting if a valref points to an invalid type, dirref points to an invalid type, caller passes in illegal root reference, or if dirref/valref have multiple references. --- src/modules/kvs/test/lookup.c | 93 ++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 2 deletions(-) diff --git a/src/modules/kvs/test/lookup.c b/src/modules/kvs/test/lookup.c index 9dc5f8f30bb3..700f6cb856ad 100644 --- a/src/modules/kvs/test/lookup.c +++ b/src/modules/kvs/test/lookup.c @@ -212,7 +212,7 @@ void check_common (lookup_t *lh, ok (lookup (lh) == lookup_result, "%s: lookup matched result", msg); ok (lookup_get_errnum (lh) == get_errnum_result, - "%s: lookup_get_errnum returns expected errnum", msg); + "%s: lookup_get_errnum returns expected errnum %d", msg, lookup_get_errnum (lh)); if (get_value_result) { ok ((val = lookup_get_value (lh)) != NULL, "%s: lookup_get_value returns non-NULL as expected", msg); @@ -517,6 +517,8 @@ void lookup_errors (void) { json_t *dirref; json_t *dir; json_t *opaque_data; + json_t *valref_multi; + json_t *dirref_multi; struct cache *cache; lookup_t *lh; href_t dirref_ref; @@ -542,7 +544,10 @@ void lookup_errors (void) { * "valref" : valref to valref_ref * "dirref" : dirref to dirref_ref * "dir" : dir w/ "val" : val to "baz" - * + * "dirref_bad" : dirref to valref_ref + * "valref_bad" : valref to dirref_ref + * "dirref_multi" : dirref to [ dirref_ref, dirref_ref ] + * "valref_multi" : valref to [ valref_ref, valref_ref ] */ opaque_data = get_json_base64_string ("abcd"); @@ -565,6 +570,17 @@ void lookup_errors (void) { treeobj_insert_entry (root, "valref", treeobj_create_valref (valref_ref)); treeobj_insert_entry (root, "dirref", treeobj_create_dirref (dirref_ref)); treeobj_insert_entry (root, "dir", dir); + treeobj_insert_entry (root, "dirref_bad", treeobj_create_dirref (valref_ref)); + treeobj_insert_entry (root, "valref_bad", treeobj_create_valref (dirref_ref)); + + valref_multi = treeobj_create_valref (valref_ref); + treeobj_append_blobref (valref_multi, valref_ref); + + dirref_multi = treeobj_create_dirref (dirref_ref); + treeobj_append_blobref (dirref_multi, dirref_ref); + + treeobj_insert_entry (root, "dirref_multi", dirref_multi); + treeobj_insert_entry (root, "valref_multi", valref_multi); kvs_util_json_hash ("sha1", root, root_ref); cache_insert (cache, root_ref, cache_entry_create (root)); @@ -712,6 +728,79 @@ void lookup_errors (void) { "lookup_create on symlink"); check (lh, true, ENOTDIR, NULL, NULL, "lookup symlink, expecting dir"); + /* Lookup a dirref that doesn't point to a dir, should get EPERM. */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref_bad", + FLUX_KVS_READDIR)) != NULL, + "lookup_create on dirref_bad"); + check (lh, true, EPERM, NULL, NULL, "lookup dirref_bad"); + + /* Lookup a valref that doesn't point to a base64 string, should get EPERM */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "valref_bad", + 0)) != NULL, + "lookup_create on valref_bad"); + check (lh, true, EPERM, NULL, NULL, "lookup valref_bad"); + + /* Lookup with an invalid root_ref, should get EINVAL */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + valref_ref, + "val", + 0)) != NULL, + "lookup_create on bad root_ref"); + check (lh, true, EINVAL, NULL, NULL, "lookup bad root_ref"); + + /* Lookup dirref with multiple blobrefs, should get EPERM */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref_multi", + FLUX_KVS_READDIR)) != NULL, + "lookup_create on dirref_multi"); + check (lh, true, EPERM, NULL, NULL, "lookup dirref_multi"); + + /* Lookup path w/ dirref w/ multiple blobrefs in middle, should + * get EPERM */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "dirref_multi.foo", + 0)) != NULL, + "lookup_create on dirref_multi, part of path"); + check (lh, true, EPERM, NULL, NULL, "lookup dirref_multi, part of path"); + + /* Lookup valref with multiple blobrefs, should get EPERM */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "valref_multi", + 0)) != NULL, + "lookup_create on valref_multi"); + check (lh, true, EPERM, NULL, NULL, "lookup valref_multi"); + + /* Lookup path w/ valref w/ multiple blobrefs in middle, Not + * ENOENT - caller of lookup decides what to do with entry not + * found */ + ok ((lh = lookup_create (cache, + 1, + root_ref, + root_ref, + "valref_multi.foo", + 0)) != NULL, + "lookup_create on valref_multi, part of path"); + check (lh, true, 0, NULL, NULL, "lookup valref_multi, part of path"); + cache_destroy (cache); } From 6664161e98c2a1ef58cecd05035a72c6da565257 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 7 Aug 2017 13:39:25 -0700 Subject: [PATCH 16/27] common/libkvs: Add treeobj_copy_dir() Add treeobj_copy_dir() to perform a shallow copy of a treeobj dir. json_copy() is not safe for copying. Add unit tests appropriately. --- src/common/libkvs/test/treeobj.c | 88 ++++++++++++++++++++++++++++++++ src/common/libkvs/treeobj.c | 31 +++++++++++ src/common/libkvs/treeobj.h | 7 +++ 3 files changed, 126 insertions(+) diff --git a/src/common/libkvs/test/treeobj.c b/src/common/libkvs/test/treeobj.c index 47bdc4878a9e..8df135d03f07 100644 --- a/src/common/libkvs/test/treeobj.c +++ b/src/common/libkvs/test/treeobj.c @@ -363,6 +363,93 @@ void test_dir (void) json_decref (dir); } +void test_copy_dir (void) +{ + json_t *dir, *dircpy; + json_t *val1, *val2, *val3; + json_t *test; + + /* create couple test values */ + val1 = treeobj_create_val ("a", 1); + val2 = treeobj_create_val ("b", 1); + val3 = treeobj_create_val ("c", 1); + if (!val1 || !val2 || !val3) + BAIL_OUT ("can't continue without test values"); + + /* First, some corner case tests */ + + ok (treeobj_copy_dir (val1) == NULL, + "tree_copy_dir fails on non-dir"); + + /* Next, test to show json_copy() is not safe. + * + * json_copy(), which makes a shallow copy of a JSON object, + * cannot be used directly on a RFC 11 dir object, since the + * directory itself is one level down under the "data" key. + */ + + ok ((dir = treeobj_create_dir ()) != NULL, + "treeobj_create_dir works"); + ok (treeobj_insert_entry (dir, "a", val1) == 0, + "treeobj_insert_entry works"); + ok (treeobj_insert_entry (dir, "b", val2) == 0, + "treeobj_insert_entry works"); + ok (treeobj_get_count (dir) == 2, + "treeobj_get_count gets correct count"); + + ok ((dircpy = json_copy (dir)) != NULL, + "json_copy works"); + + /* change "b" to "c" in original */ + ok (treeobj_insert_entry (dir, "b", val3) == 0, + "treeobj_insert_entry works"); + + ok (json_equal (dir, dircpy) == 1, + "dir and dircpy are equal, json_copy() not safe"); + + json_decref (dir); + json_decref (dircpy); + + /* Now test to show treeobj_copy_dir() is safe */ + + ok ((dir = treeobj_create_dir ()) != NULL, + "treeobj_create_dir works"); + ok (treeobj_insert_entry (dir, "a", val1) == 0, + "treeobj_insert_entry works"); + ok (treeobj_insert_entry (dir, "b", val2) == 0, + "treeobj_insert_entry works"); + ok (treeobj_get_count (dir) == 2, + "treeobj_get_count gets correct count"); + + ok ((dircpy = treeobj_copy_dir (dir)) != NULL, + "treeobj_copy_dir works"); + + /* change "b" to "c" in original */ + ok (treeobj_insert_entry (dir, "b", val3) == 0, + "treeobj_insert_entry works"); + + ok (json_equal (dir, dircpy) == 0, + "dir and dircpy are not equal, treeobj_copy_dir() safe"); + + /* Make sure values are in the right copies */ + ok ((test = treeobj_get_entry (dir, "b")) != NULL, + "treeobj_get_entry works"); + ok (json_equal (test, val3) == 1, + "correct value is in original"); + + ok ((test = treeobj_get_entry (dircpy, "b")) != NULL, + "treeobj_get_entry works"); + ok (json_equal (test, val2) == 1, + "correct value is in copy"); + + json_decref (dir); + json_decref (dircpy); + + json_decref (val1); + json_decref (val2); + json_decref (val3); +} + void test_symlink (void) { json_t *o, *data; @@ -479,6 +566,7 @@ int main(int argc, char** argv) test_val_base64 (); test_dirref (); test_dir (); + test_copy_dir (); test_symlink (); test_corner_cases (); diff --git a/src/common/libkvs/treeobj.c b/src/common/libkvs/treeobj.c index 7966d82a6fda..ba7cbae57de2 100644 --- a/src/common/libkvs/treeobj.c +++ b/src/common/libkvs/treeobj.c @@ -263,6 +263,37 @@ int treeobj_insert_entry (json_t *obj, const char *name, json_t *obj2) return 0; } +json_t *treeobj_copy_dir (json_t *obj) +{ + const char *type; + json_t *data; + json_t *cpy; + json_t *datacpy; + int save_errno; + + if (treeobj_unpack (obj, &type, &data) < 0 + || strcmp (type, "dir") != 0) { + errno = EINVAL; + return NULL; + } + if (!(cpy = treeobj_create_dir ())) + return NULL; + if (!(datacpy = json_copy (data))) { + save_errno = errno; + json_decref (cpy); + errno = save_errno; + return NULL; + } + if (json_object_set_new (cpy, "data", datacpy) < 0) { + save_errno = errno; + json_decref (datacpy); + json_decref (cpy); + errno = save_errno; + return NULL; + } + return cpy; +} + int treeobj_append_blobref (json_t *obj, const char *blobref) { const char *type; diff --git a/src/common/libkvs/treeobj.h b/src/common/libkvs/treeobj.h index da99054a0da4..a3acd284d11e 100644 --- a/src/common/libkvs/treeobj.h +++ b/src/common/libkvs/treeobj.h @@ -69,6 +69,13 @@ json_t *treeobj_get_entry (json_t *obj, const char *name); int treeobj_insert_entry (json_t *obj, const char *name, json_t *obj2); int treeobj_delete_entry (json_t *obj, const char *name); +/* Copy a treeobj dir. + * This function performs a shallow copy on a dir object, copying the + * first level of directory entries. Use this function over json_copy() + * on dir objects, as the latter is not safe on treeobj objects. + */ +json_t *treeobj_copy_dir (json_t *obj); + /* add blobref to dirref,valref object. * Return 0 on success, -1 on failure with errno set. */ From 5661a86265e3efb5660ebd2fc3824613074bd13a Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 7 Aug 2017 14:21:36 -0700 Subject: [PATCH 17/27] modules/kvs: Update internal commit API to RFC 11 --- src/modules/kvs/commit.c | 177 +++++++++++++++++++++++---------------- 1 file changed, 105 insertions(+), 72 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index f1e3ba97e19f..324c5c2c421e 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -37,7 +37,7 @@ #include #include "src/common/libutil/log.h" -#include "src/common/libkvs/jansson_dirent.h" +#include "src/common/libkvs/treeobj.h" #include "commit.h" #include "kvs_util.h" @@ -242,24 +242,31 @@ static int store_cache (commit_t *c, int current_epoch, json_t *o, */ static int commit_unroll (commit_t *c, int current_epoch, json_t *dir) { - json_t *value; - json_t *subdir, *key_value; - json_t *tmpdirent; + json_t *dir_entry; + json_t *dir_data; + json_t *tmp; href_t ref; int ret; struct cache_entry *hp; - void *iter = json_object_iter (dir); + void *iter; + + assert (treeobj_is_dir (dir)); + + if (!(dir_data = treeobj_get_data (dir))) + return -1; + + iter = json_object_iter (dir_data); /* Do not use json_object_foreach(), unsafe to modify via * json_object_set() while iterating. */ while (iter) { - value = json_object_iter_value (iter); - if ((subdir = json_object_get (value, "DIRVAL"))) { - if (commit_unroll (c, current_epoch, subdir) < 0) /* depth first */ + dir_entry = json_object_iter_value (iter); + if (treeobj_is_dir (dir_entry)) { + if (commit_unroll (c, current_epoch, dir_entry) < 0) /* depth first */ return -1; - json_incref (subdir); - if ((ret = store_cache (c, current_epoch, subdir, ref, &hp)) < 0) + json_incref (dir_entry); + if ((ret = store_cache (c, current_epoch, dir_entry, ref, &hp)) < 0) return -1; if (ret) { if (zlist_push (c->item_callback_list, hp) < 0) { @@ -268,21 +275,25 @@ static int commit_unroll (commit_t *c, int current_epoch, json_t *dir) return -1; } } - if (!(tmpdirent = j_dirent_create ("DIRREF", ref))) + if (!(tmp = treeobj_create_dirref (ref))) return -1; - if (json_object_iter_set_new (dir, iter, tmpdirent) < 0) { - json_decref (tmpdirent); + if (json_object_iter_set_new (dir, iter, tmp) < 0) { + json_decref (tmp); errno = ENOMEM; return -1; } } - else if ((key_value = json_object_get (value, "FILEVAL"))) { + else if (treeobj_is_val (dir_entry)) { + json_t *val_data; size_t size; - if (kvs_util_json_encoded_size (key_value, &size) < 0) + + if (!(val_data = treeobj_get_data (dir_entry))) + return -1; + if (kvs_util_json_encoded_size (val_data, &size) < 0) return -1; if (size > BLOBREF_MAX_STRING_SIZE) { - json_incref (key_value); - if ((ret = store_cache (c, current_epoch, key_value, + json_incref (val_data); + if ((ret = store_cache (c, current_epoch, val_data, ref, &hp)) < 0) return -1; if (ret) { @@ -292,16 +303,16 @@ static int commit_unroll (commit_t *c, int current_epoch, json_t *dir) return -1; } } - if (!(tmpdirent = j_dirent_create ("FILEREF", ref))) + if (!(tmp = treeobj_create_valref (ref))) return -1; - if (json_object_iter_set_new (dir, iter, tmpdirent) < 0) { - json_decref (tmpdirent); + if (json_object_iter_set_new (dir, iter, tmp) < 0) { + json_decref (tmp); errno = ENOMEM; return -1; } } } - iter = json_object_iter_next (dir, iter); + iter = json_object_iter_next (dir_data, iter); } return 0; @@ -316,8 +327,7 @@ static int commit_link_dirent (commit_t *c, int current_epoch, char *cpy = strdup (key); char *next, *name = cpy; json_t *dir = rootdir; - json_t *o, *subdir = NULL, *subdirent; - json_t *tmpdirent; + json_t *subdir = NULL, *dir_entry; int saved_errno, rc = -1; if (!cpy) { @@ -333,62 +343,85 @@ static int commit_link_dirent (commit_t *c, int current_epoch, } /* This is the first part of a key with multiple path components. - * Make sure that it is a directory in DIRVAL form, then recurse - * on the remaining path components. + * Make sure that it is a treeobj dir, then recurse on the + * remaining path components. */ while ((next = strchr (name, '.'))) { *next++ = '\0'; - if (!(subdirent = json_object_get (dir, name))) { + + if (!treeobj_is_dir (dir)) { + saved_errno = EPERM; + goto done; + } + + if (!(dir_entry = treeobj_get_entry (dir, name))) { if (json_is_null (dirent)) /* key deletion - it doesn't exist so return */ goto success; - if (!(subdir = json_object ())) { - saved_errno = ENOMEM; + if (!(subdir = treeobj_create_dir ())) { + saved_errno = errno; goto done; } - if (!(tmpdirent = j_dirent_create ("DIRVAL", subdir))) { + if (treeobj_insert_entry (dir, name, subdir) < 0) { saved_errno = errno; json_decref (subdir); goto done; } - if (json_object_set_new (dir, name, tmpdirent) < 0) { - json_decref (tmpdirent); - json_decref (subdir); - saved_errno = ENOMEM; + json_decref (subdir); + } else if (treeobj_is_dir (dir_entry)) { + subdir = dir_entry; + } else if (treeobj_is_dirref (dir_entry)) { + const char *ref; + int refcount; + + if ((refcount = treeobj_get_count (dir_entry)) < 0) { + saved_errno = errno; goto done; } - json_decref (subdir); - } else if ((o = json_object_get (subdirent, "DIRVAL"))) { - subdir = o; - } else if ((o = json_object_get (subdirent, "DIRREF"))) { - assert (json_is_string (o)); - const char *ref = json_string_value (o); + + if (refcount != 1) { + log_msg ("invalid dirref count: %d", refcount); + saved_errno = EPERM; + goto done; + } + + if (!(ref = treeobj_get_blobref (dir_entry, 0))) { + saved_errno = errno; + goto done; + } + if (!(subdir = cache_lookup_and_get_json (c->cm->cache, ref, current_epoch))) { *missing_ref = ref; goto success; /* stall */ } - /* do not corrupt store by modify orig. */ - if (!(subdir = json_copy (subdir))) { - saved_errno = ENOMEM; - goto done; - } - if (!(tmpdirent = j_dirent_create ("DIRVAL", subdir))) { + + /* do not corrupt store by modifying orig. */ + if (!(subdir = treeobj_copy_dir (subdir))) { saved_errno = errno; - json_decref (subdir); goto done; } - if (json_object_set_new (dir, name, tmpdirent) < 0) { - json_decref (tmpdirent); + + if (treeobj_insert_entry (dir, name, subdir) < 0) { + saved_errno = errno; json_decref (subdir); - saved_errno = ENOMEM; goto done; } json_decref (subdir); - } else if ((o = json_object_get (subdirent, "LINKVAL"))) { - assert (json_is_string (o)); + } else if (treeobj_is_symlink (dir_entry)) { + json_t *symlink = treeobj_get_data (dir_entry); + const char *symlinkstr; char *nkey = NULL; - if (asprintf (&nkey, "%s.%s", json_string_value (o), next) < 0) { + + if (!symlink) { + saved_errno = errno; + goto done; + } + + assert (json_is_string (symlink)); + + symlinkstr = json_string_value (symlink); + if (asprintf (&nkey, "%s.%s", symlinkstr, next) < 0) { saved_errno = ENOMEM; goto done; } @@ -407,19 +440,13 @@ static int commit_link_dirent (commit_t *c, int current_epoch, } else { if (json_is_null (dirent)) /* key deletion - it doesn't exist so return */ goto success; - if (!(subdir = json_object ())) { - saved_errno = ENOMEM; - goto done; - } - if (!(tmpdirent = j_dirent_create ("DIRVAL", subdir))) { + if (!(subdir = treeobj_create_dir ())) { saved_errno = errno; - json_decref (subdir); goto done; } - if (json_object_set_new (dir, name, tmpdirent) < 0) { - json_decref (tmpdirent); + if (treeobj_insert_entry (dir, name, subdir) < 0) { + saved_errno = errno; json_decref (subdir); - saved_errno = ENOMEM; goto done; } json_decref (subdir); @@ -430,14 +457,20 @@ static int commit_link_dirent (commit_t *c, int current_epoch, /* This is the final path component of the key. Add it to the directory. */ if (!json_is_null (dirent)) { - if (json_object_set_new (dir, name, json_incref (dirent)) < 0) { + if (treeobj_insert_entry (dir, name, dirent) < 0) { saved_errno = errno; - json_decref (dirent); goto done; } } - else - json_object_del (dir, name); + else { + if (treeobj_delete_entry (dir, name) < 0) { + /* if ENOENT, it's ok since we're deleting */ + if (errno != ENOENT) { + saved_errno = errno; + goto done; + } + } + } success: rc = 0; done: @@ -480,8 +513,8 @@ commit_process_t commit_process (commit_t *c, goto stall_load; } - if (!(c->rootcpy = json_copy (rootdir))) { - c->errnum = ENOMEM; + if (!(c->rootcpy = treeobj_copy_dir (rootdir))) { + c->errnum = errno; return COMMIT_PROCESS_ERROR; } @@ -492,9 +525,9 @@ commit_process_t commit_process (commit_t *c, { /* Apply each op (e.g. key = val) in sequence to the root * copy. A side effect of walking key paths is to convert - * DIRREFs to DIRVALs in the copy. This allows the commit - * to be self-contained in the rootcpy until it is - * unrolled later on. + * dirref objects to dir objects in the copy. This allows + * the commit to be self-contained in the rootcpy until it + * is unrolled later on. */ if (fence_get_json_ops (c->f)) { json_t *op, *key, *dirent; @@ -546,8 +579,8 @@ commit_process_t commit_process (commit_t *c, case COMMIT_STATE_STORE: { /* Unroll the root copy. - * When a DIRVAL is found, store an object and replace it - * with a DIRREF. Finally, store the unrolled root copy + * When a dir is found, store an object and replace it + * with a dirref. Finally, store the unrolled root copy * as an object and keep its reference in c->newroot. * Flushes to content cache are asynchronous but we don't * proceed until they are completed. From 37d6a0633af545c3b88222a146b846f0e6863973 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 24 Aug 2017 11:42:03 -0700 Subject: [PATCH 18/27] modules/kvs: Refactor store_cache() logic Refactor the logic of store_cache(), so that it no longer steals references, but lets the caller keep ownership. Simplifies logic in several places and makes code easier to understand. --- src/modules/kvs/commit.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index 324c5c2c421e..70cc68a2bb9a 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -181,8 +181,7 @@ static void cleanup_dirty_cache_list (commit_t *c) } /* Store object 'o' under key 'ref' in local cache. - * Object reference is given to this function, it will either give it - * to the cache or decref it. + * Object reference is still owned by the caller. * Returns -1 on error, 0 on success entry already there, 1 on success * entry needs to be flushed to content store */ @@ -195,29 +194,31 @@ static int store_cache (commit_t *c, int current_epoch, json_t *o, if (kvs_util_json_hash (c->cm->hash_name, o, ref) < 0) { saved_errno = errno; log_err ("kvs_util_json_hash"); - goto decref_done; + goto done; } if (!(hp = cache_lookup (c->cm->cache, ref, current_epoch))) { if (!(hp = cache_entry_create (NULL))) { saved_errno = ENOMEM; - goto decref_done; + goto done; } cache_insert (c->cm->cache, ref, hp); } if (cache_entry_get_valid (hp)) { c->cm->noop_stores++; - json_decref (o); rc = 0; } else { + json_incref (o); if (cache_entry_set_json (hp, o) < 0) { int ret; saved_errno = errno; + json_decref (o); ret = cache_remove_entry (c->cm->cache, ref); assert (ret == 1); - goto decref_done; + goto done; } if (cache_entry_set_dirty (hp, true) < 0) { - /* cache_remove_entry will decref object */ + /* cache entry now owns a reference, cache_remove_entry + * will decref object */ int ret; saved_errno = errno; ret = cache_remove_entry (c->cm->cache, ref); @@ -229,8 +230,6 @@ static int store_cache (commit_t *c, int current_epoch, json_t *o, *hpp = hp; return rc; - decref_done: - json_decref (o); done: errno = saved_errno; return rc; @@ -265,7 +264,6 @@ static int commit_unroll (commit_t *c, int current_epoch, json_t *dir) if (treeobj_is_dir (dir_entry)) { if (commit_unroll (c, current_epoch, dir_entry) < 0) /* depth first */ return -1; - json_incref (dir_entry); if ((ret = store_cache (c, current_epoch, dir_entry, ref, &hp)) < 0) return -1; if (ret) { @@ -292,7 +290,6 @@ static int commit_unroll (commit_t *c, int current_epoch, json_t *dir) if (kvs_util_json_encoded_size (val_data, &size) < 0) return -1; if (size > BLOBREF_MAX_STRING_SIZE) { - json_incref (val_data); if ((ret = store_cache (c, current_epoch, val_data, ref, &hp)) < 0) return -1; @@ -607,10 +604,11 @@ commit_process_t commit_process (commit_t *c, return COMMIT_PROCESS_ERROR; } - /* cache took ownership of rootcpy, we're done, but - * may still need to stall user. + /* cache now has ownership of rootcpy, we don't need our + * rootcpy anymore. But we may still need to stall user. */ c->state = COMMIT_STATE_PRE_FINISHED; + json_decref (c->rootcpy); c->rootcpy = NULL; /* fallthrough */ From 18677baf07984104dee966e6b5afd608c0b56d31 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 7 Aug 2017 19:39:48 -0700 Subject: [PATCH 19/27] modules/kvs/test: Update commit tests to RFC 11 --- src/modules/kvs/test/commit.c | 363 ++++++++++++++++------------------ 1 file changed, 167 insertions(+), 196 deletions(-) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index 05ba0d334c98..e250dac354bd 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -6,7 +6,7 @@ #include "src/common/libtap/tap.h" #include "src/common/libkvs/kvs.h" -#include "src/common/libkvs/jansson_dirent.h" +#include "src/common/libkvs/treeobj.h" #include "src/modules/kvs/cache.h" #include "src/modules/kvs/commit.h" #include "src/modules/kvs/lookup.h" @@ -16,8 +16,9 @@ static int test_global = 5; -/* Append a JSON object containing - * { "key" : key, "dirent" : dirent } +/* Append a json object containing + * { "key" : key, "dirent" : } } + * or * { "key" : key, "dirent" : null } * to a json array. */ @@ -31,7 +32,7 @@ void ops_append (json_t *array, const char *key, const char *value) json_object_set_new (op, "key", o); if (value) { - json_t *dirent = j_dirent_create ("FILEVAL", json_string (value)); + json_t *dirent = treeobj_create_val ((void *)value, strlen (value)); json_object_set_new (op, "dirent", dirent); } else { @@ -46,7 +47,9 @@ struct cache *create_cache_with_empty_rootdir (href_t ref) { struct cache *cache; struct cache_entry *hp; - json_t *rootdir = json_object (); + json_t *rootdir; + + rootdir = treeobj_create_dir (); ok ((cache = cache_create ()) != NULL, "cache_create works"); @@ -398,7 +401,7 @@ void verify_value (struct cache *cache, "lookup found result"); if (val) { - test = json_string (val); + test = treeobj_create_val (val, strlen (val)); ok ((o = lookup_get_value (lh)) != NULL, "lookup_get_value returns non-NULL as expected"); ok (json_equal (test, o) == true, @@ -593,12 +596,15 @@ struct rootref_data { int rootref_cb (commit_t *c, const char *ref, void *data) { struct rootref_data *rd = data; - json_t *rootdir = json_object (); + json_t *rootdir; struct cache_entry *hp; ok (strcmp (ref, rd->rootref) == 0, "missing root reference is what we expect it to be"); + ok ((rootdir = treeobj_create_dir ()) != NULL, + "treeobj_create_dir works"); + ok ((hp = cache_entry_create (rootdir)) != NULL, "cache_entry_create works"); @@ -614,12 +620,15 @@ void commit_process_root_missing (void) commit_t *c; href_t rootref; struct rootref_data rd; - json_t *rootdir = json_object (); + json_t *rootdir; const char *newroot; ok ((cache = cache_create ()) != NULL, "cache_create works"); + ok ((rootdir = treeobj_create_dir ()) != NULL, + "treeobj_create_dir works"); + ok (kvs_util_json_hash ("sha1", rootdir, rootref) == 0, "kvs_util_json_hash worked"); @@ -706,26 +715,24 @@ void commit_process_missing_ref (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); /* don't add dir entry, we want it to miss */ - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -735,7 +742,7 @@ void commit_process_missing_ref (void) { ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, "commit_mgr_create works"); - create_ready_commit (cm, "fence1", "dir.fileval", "52", 0); + create_ready_commit (cm, "fence1", "dir.val", "52", 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -770,7 +777,7 @@ void commit_process_missing_ref (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dir.fileval", "52"); + verify_value (cache, newroot, "dir.val", "52"); commit_mgr_destroy (cm); cache_destroy (cache); @@ -801,26 +808,24 @@ void commit_process_error_callbacks (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); /* don't add dir entry, we want it to miss */ - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -830,7 +835,7 @@ void commit_process_error_callbacks (void) { ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, "commit_mgr_create works"); - create_ready_commit (cm, "fence1", "dir.file", "52", 0); + create_ready_commit (cm, "fence1", "dir.val", "52", 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -885,26 +890,24 @@ void commit_process_error_callbacks_partway (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -953,26 +956,24 @@ void commit_process_invalid_operation (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1015,26 +1016,24 @@ void commit_process_invalid_hash (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1078,40 +1077,37 @@ void commit_process_follow_link (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } - * "linkval" : { "LINKVAL" : "dir" } } + * root_ref + * "dir" : dirref to dir_ref + * "symlink" : symlink to "dir" * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); - json_object_set (root, - "linkval", - j_dirent_create ("LINKVAL", json_string ("dir"))); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); + treeobj_insert_entry (root, "symlink", treeobj_create_symlink ("dir")); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, root_ref, cache_entry_create (root)); + ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, "commit_mgr_create works"); - create_ready_commit (cm, "fence1", "linkval.fileval", "52", 0); + create_ready_commit (cm, "fence1", "symlink.val", "52", 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1128,7 +1124,7 @@ void commit_process_follow_link (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "linkval.fileval", "52"); + verify_value (cache, newroot, "symlink.val", "52"); commit_mgr_destroy (cm); cache_destroy (cache); @@ -1148,18 +1144,16 @@ void commit_process_dirval_test (void) { /* This root is * - * root - * { "dirval" : { "DIRVAL" : { "fileval" : { "FILEVAL" : "42" } } } } + * root_ref + * "dir" : dir with { "val" : val to 42 } * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - root = json_object (); - json_object_set (root, "dirval", j_dirent_create ("DIRVAL", dir)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", dir); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1169,7 +1163,7 @@ void commit_process_dirval_test (void) { ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, "commit_mgr_create works"); - create_ready_commit (cm, "fence1", "dirval.fileval", "52", 0); + create_ready_commit (cm, "fence1", "dir.val", "52", 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1186,7 +1180,7 @@ void commit_process_dirval_test (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dirval.fileval", "52"); + verify_value (cache, newroot, "dir.val", "52"); commit_mgr_destroy (cm); cache_destroy (cache); @@ -1207,26 +1201,24 @@ void commit_process_delete_test (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1237,7 +1229,7 @@ void commit_process_delete_test (void) { "commit_mgr_create works"); /* NULL value --> delete */ - create_ready_commit (cm, "fence1", "dir.fileval", NULL, 0); + create_ready_commit (cm, "fence1", "dir.val", NULL, 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1254,7 +1246,7 @@ void commit_process_delete_test (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dir.fileval", NULL); + verify_value (cache, newroot, "dir.val", NULL); commit_mgr_destroy (cm); cache_destroy (cache); @@ -1275,26 +1267,24 @@ void commit_process_delete_nosubdir_test (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1306,7 +1296,7 @@ void commit_process_delete_nosubdir_test (void) { /* subdir doesn't exist for this key */ /* NULL value --> delete */ - create_ready_commit (cm, "fence1", "noexistdir.fileval", NULL, 0); + create_ready_commit (cm, "fence1", "noexistdir.val", NULL, 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1317,7 +1307,7 @@ void commit_process_delete_nosubdir_test (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "noexistdir.fileval", NULL); + verify_value (cache, newroot, "noexistdir.val", NULL); commit_mgr_destroy (cm); cache_destroy (cache); @@ -1338,26 +1328,24 @@ void commit_process_delete_filevalinpath_test (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1367,9 +1355,9 @@ void commit_process_delete_filevalinpath_test (void) { ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, "commit_mgr_create works"); - /* fileval is in path */ + /* val is in path */ /* NULL value --> delete */ - create_ready_commit (cm, "fence1", "dir.fileval.filebaz", NULL, 0); + create_ready_commit (cm, "fence1", "dir.val.valbaz", NULL, 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1380,7 +1368,7 @@ void commit_process_delete_filevalinpath_test (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dir.fileval.filebaz", NULL); + verify_value (cache, newroot, "dir.val.valbaz", NULL); commit_mgr_destroy (cm); cache_destroy (cache); @@ -1404,26 +1392,24 @@ void commit_process_big_fileval (void) { /* This root is * - * root - * { "dir" : { "DIRREF" : } } + * root_ref + * "dir" : dirref to dir_ref * - * dir - * { "fileval" : { "FILEVAL" : "42" } } + * dir_ref + * "val" : val w/ "42" * */ - dir = json_object(); - json_object_set (dir, - "fileval", - j_dirent_create ("FILEVAL", json_string ("42"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1437,7 +1423,7 @@ void commit_process_big_fileval (void) { for (i = 0; i < bigstrsize - 1; i++) bigstr[i] = 'a'; - create_ready_commit (cm, "fence1", "dir.fileval", bigstr, 0); + create_ready_commit (cm, "fence1", "dir.val", bigstr, 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1454,7 +1440,7 @@ void commit_process_big_fileval (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dir.fileval", bigstr); + verify_value (cache, newroot, "dir.val", bigstr); commit_mgr_destroy (cm); cache_destroy (cache); @@ -1479,72 +1465,57 @@ void commit_process_giant_dir (void) { /* This root is. * * root - * { "dir" : { "DIRREF" : } } + * "dir" : dirref to dir_ref * * Mix up keys and upper/lower case to get different hash ordering * other than the "obvious" one. * - * dir - * { "fileval0000" : { "FILEVAL" : "0" }, - * "fileval0010" : { "FILEVAL" : "1" }, - * "fileval0200" : { "FILEVAL" : "2" }, - * "fileval3000" : { "FILEVAL" : "3" }, - * "fileval0004" : { "FILEVAL" : "4" }, - * "fileval0050" : { "FILEVAL" : "5" }, - * "fileval0600" : { "FILEVAL" : "6" }, - * "fileval7000" : { "FILEVAL" : "7" }, - * "fileval0008" : { "FILEVAL" : "8" }, - * "fileval0090" : { "FILEVAL" : "9" }, - * "fileval0a00" : { "FILEVAL" : "A" }, - * "filevalB000" : { "FILEVAL" : "b" }, - * "fileval000c" : { "FILEVAL" : "C" }, - * "fileval00D0" : { "FILEVAL" : "d" }, - * "fileval0e00" : { "FILEVAL" : "E" }, - * "filevalF000" : { "FILEVAL" : "f" } } + * dir_ref + * "val0000" : val to "0" + * "val0010" : val to "1" + * "val0200" : val to "2" + * "val3000" : val to "3" + * "val0004" : val to "4" + * "val0050" : val to "5" + * "val0600" : val to "6" + * "val7000" : val to "7" + * "val0008" : val to "8" + * "val0090" : val to "9" + * "val0a00" : val to "A" + * "valB000" : val to "b" + * "val000c" : val to "C" + * "val00D0" : val to "d" + * "val0e00" : val to "E" + * "valF000" : val to "f" * */ dir = json_object(); - json_object_set (dir, "fileval0000", - j_dirent_create ("FILEVAL", json_string ("0"))); - json_object_set (dir, "fileval0010", - j_dirent_create ("FILEVAL", json_string ("1"))); - json_object_set (dir, "fileval0200", - j_dirent_create ("FILEVAL", json_string ("2"))); - json_object_set (dir, "fileval3000", - j_dirent_create ("FILEVAL", json_string ("3"))); - json_object_set (dir, "fileval0004", - j_dirent_create ("FILEVAL", json_string ("4"))); - json_object_set (dir, "fileval0050", - j_dirent_create ("FILEVAL", json_string ("5"))); - json_object_set (dir, "fileval0600", - j_dirent_create ("FILEVAL", json_string ("6"))); - json_object_set (dir, "fileval7000", - j_dirent_create ("FILEVAL", json_string ("7"))); - json_object_set (dir, "fileval0008", - j_dirent_create ("FILEVAL", json_string ("8"))); - json_object_set (dir, "fileval0090", - j_dirent_create ("FILEVAL", json_string ("9"))); - json_object_set (dir, "fileval0a00", - j_dirent_create ("FILEVAL", json_string ("A"))); - json_object_set (dir, "filevalB000", - j_dirent_create ("FILEVAL", json_string ("b"))); - json_object_set (dir, "fileval000c", - j_dirent_create ("FILEVAL", json_string ("C"))); - json_object_set (dir, "fileval00D0", - j_dirent_create ("FILEVAL", json_string ("d"))); - json_object_set (dir, "fileval0e00", - j_dirent_create ("FILEVAL", json_string ("E"))); - json_object_set (dir, "filevalF000", - j_dirent_create ("FILEVAL", json_string ("f"))); + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val0000", treeobj_create_val ("0", 1)); + treeobj_insert_entry (dir, "val0010", treeobj_create_val ("1", 1)); + treeobj_insert_entry (dir, "val0200", treeobj_create_val ("2", 1)); + treeobj_insert_entry (dir, "val3000", treeobj_create_val ("3", 1)); + treeobj_insert_entry (dir, "val0004", treeobj_create_val ("4", 1)); + treeobj_insert_entry (dir, "val0050", treeobj_create_val ("5", 1)); + treeobj_insert_entry (dir, "val0600", treeobj_create_val ("6", 1)); + treeobj_insert_entry (dir, "val7000", treeobj_create_val ("7", 1)); + treeobj_insert_entry (dir, "val0008", treeobj_create_val ("8", 1)); + treeobj_insert_entry (dir, "val0090", treeobj_create_val ("9", 1)); + treeobj_insert_entry (dir, "val0a00", treeobj_create_val ("A", 1)); + treeobj_insert_entry (dir, "valB000", treeobj_create_val ("b", 1)); + treeobj_insert_entry (dir, "val000c", treeobj_create_val ("C", 1)); + treeobj_insert_entry (dir, "val00D0", treeobj_create_val ("d", 1)); + treeobj_insert_entry (dir, "val0e00", treeobj_create_val ("E", 1)); + treeobj_insert_entry (dir, "valF000", treeobj_create_val ("f", 1)); ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, "kvs_util_json_hash worked"); cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = json_object (); - json_object_set (root, "dir", j_dirent_create ("DIRREF", dir_ref)); + root = treeobj_create_dir (); + treeobj_insert_entry (dir, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1555,10 +1526,10 @@ void commit_process_giant_dir (void) { "commit_mgr_create works"); /* make three ready commits */ - create_ready_commit (cm, "fence1", "dir.fileval0200", "foo", 0); - create_ready_commit (cm, "fence2", "dir.fileval0090", "bar", 0); + create_ready_commit (cm, "fence1", "dir.val0200", "foo", 0); + create_ready_commit (cm, "fence2", "dir.val0090", "bar", 0); /* NULL value --> delete */ - create_ready_commit (cm, "fence3", "dir.fileval00D0", NULL, 0); + create_ready_commit (cm, "fence3", "dir.val00D0", NULL, 0); /* merge these three commits */ ok (commit_mgr_merge_ready_commits (cm) == 0, @@ -1579,9 +1550,9 @@ void commit_process_giant_dir (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dir.fileval0200", "foo"); - verify_value (cache, newroot, "dir.fileval0090", "bar"); - verify_value (cache, newroot, "dir.fileval00D0", NULL); + verify_value (cache, newroot, "dir.val0200", "foo"); + verify_value (cache, newroot, "dir.val0090", "bar"); + verify_value (cache, newroot, "dir.val00D0", NULL); commit_mgr_remove_commit (cm, c); @@ -1600,10 +1571,10 @@ int main (int argc, char *argv[]) commit_mgr_merge_tests (); commit_basic_tests (); commit_basic_commit_process_test (); - commit_process_root_missing (); - commit_process_missing_ref (); commit_basic_commit_process_test_multiple_fences (); commit_basic_commit_process_test_multiple_fences_merge (); + commit_process_root_missing (); + commit_process_missing_ref (); /* no need for dirty_cache_entries() test, as it is the most * "normal" situation and is tested throughout */ From b9ff6c909cad8b4023031def9265585827681d1c Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 8 Aug 2017 16:46:09 -0700 Subject: [PATCH 20/27] modules/kvs/test: Add new commit tests Add test for invalid treeobj object as root and dirrefs that have multiple references. --- src/modules/kvs/test/commit.c | 107 ++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index e250dac354bd..56b051f59480 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -588,6 +588,47 @@ void commit_basic_commit_process_test_multiple_fences_merge (void) cache_destroy (cache); } +void commit_basic_root_not_dir (void) +{ + struct cache *cache; + commit_mgr_t *cm; + commit_t *c; + json_t *root; + href_t root_ref; + + ok ((cache = cache_create ()) != NULL, + "cache_create works"); + + /* make a non-dir root */ + root = treeobj_create_val ("abcd", 4); + + ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, + "kvs_util_json_hash worked"); + + cache_insert (cache, root_ref, cache_entry_create (root)); + + ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, + "commit_mgr_create works"); + + create_ready_commit (cm, "fence1", "val", "42", 0); + + ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, + "commit_mgr_get_ready_commit returns ready commit"); + + ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_ERROR, + "commit_process returns COMMIT_PROCESS_ERROR"); + + /* error is caught continuously */ + ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_ERROR, + "commit_process returns COMMIT_PROCESS_ERROR again"); + + ok (commit_get_errnum (c) == EINVAL, + "commit_get_errnum return EINVAL"); + + commit_mgr_destroy (cm); + cache_destroy (cache); +} + struct rootref_data { struct cache *cache; const char *rootref; @@ -1374,6 +1415,70 @@ void commit_process_delete_filevalinpath_test (void) { cache_destroy (cache); } +void commit_process_bad_dirrefs (void) { + struct cache *cache; + commit_mgr_t *cm; + commit_t *c; + json_t *root; + json_t *dirref; + json_t *dir; + href_t root_ref; + href_t dir_ref; + + ok ((cache = cache_create ()) != NULL, + "cache_create works"); + + /* This root is + * + * root_ref + * "dir" : dirref to [ dir_ref, dir_ref ] + * + * dir_ref + * "val" : val w/ "42" + * + */ + + dir = treeobj_create_dir (); + treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); + + ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, + "kvs_util_json_hash worked"); + + cache_insert (cache, dir_ref, cache_entry_create (dir)); + + dirref = treeobj_create_dirref (dir_ref); + treeobj_append_blobref (dirref, dir_ref); + + root = treeobj_create_dir (); + treeobj_insert_entry (root, "dir", dirref); + + ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, + "kvs_util_json_hash worked"); + + cache_insert (cache, root_ref, cache_entry_create (root)); + + ok ((cm = commit_mgr_create (cache, "sha1", &test_global)) != NULL, + "commit_mgr_create works"); + + create_ready_commit (cm, "fence1", "dir.val", "52", 0); + + ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, + "commit_mgr_get_ready_commit returns ready commit"); + + ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_ERROR, + "commit_process returns COMMIT_PROCESS_ERROR"); + + /* error is caught continuously */ + ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_ERROR, + "commit_process returns COMMIT_PROCESS_ERROR again"); + + ok (commit_get_errnum (c) == EPERM, + "commit_get_errnum return EPERM"); + + commit_mgr_destroy (cm); + cache_destroy (cache); +} + void commit_process_big_fileval (void) { struct cache *cache; commit_mgr_t *cm; @@ -1573,6 +1678,7 @@ int main (int argc, char *argv[]) commit_basic_commit_process_test (); commit_basic_commit_process_test_multiple_fences (); commit_basic_commit_process_test_multiple_fences_merge (); + commit_basic_root_not_dir (); commit_process_root_missing (); commit_process_missing_ref (); /* no need for dirty_cache_entries() test, as it is the most @@ -1587,6 +1693,7 @@ int main (int argc, char *argv[]) commit_process_delete_test (); commit_process_delete_nosubdir_test (); commit_process_delete_filevalinpath_test (); + commit_process_bad_dirrefs (); commit_process_big_fileval (); commit_process_giant_dir (); From dfda684941c4f86665dbda3583d595814194d335 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 8 Aug 2017 18:03:58 -0700 Subject: [PATCH 21/27] modules/kvs/test: Cleanup commit tests Remove unnecessary code, in particular with error type tests. --- src/modules/kvs/test/commit.c | 86 +++-------------------------------- 1 file changed, 6 insertions(+), 80 deletions(-) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index 56b051f59480..f3596ee19d87 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -988,33 +988,13 @@ void commit_process_invalid_operation (void) { commit_mgr_t *cm; commit_t *c; json_t *root; - json_t *dir; href_t root_ref; - href_t dir_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); - /* This root is - * - * root_ref - * "dir" : dirref to dir_ref - * - * dir_ref - * "val" : val w/ "42" - * - */ - - dir = treeobj_create_dir (); - treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); - - cache_insert (cache, dir_ref, cache_entry_create (dir)); - + /* This root is an empty root */ root = treeobj_create_dir (); - treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1048,33 +1028,13 @@ void commit_process_invalid_hash (void) { commit_mgr_t *cm; commit_t *c; json_t *root; - json_t *dir; href_t root_ref; - href_t dir_ref; ok ((cache = cache_create ()) != NULL, "cache_create works"); - /* This root is - * - * root_ref - * "dir" : dirref to dir_ref - * - * dir_ref - * "val" : val w/ "42" - * - */ - - dir = treeobj_create_dir (); - treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); - - cache_insert (cache, dir_ref, cache_entry_create (dir)); - + /* This root is an empty root */ root = treeobj_create_dir (); - treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1298,34 +1258,14 @@ void commit_process_delete_nosubdir_test (void) { commit_mgr_t *cm; commit_t *c; json_t *root; - json_t *dir; href_t root_ref; - href_t dir_ref; const char *newroot; ok ((cache = cache_create ()) != NULL, "cache_create works"); - /* This root is - * - * root_ref - * "dir" : dirref to dir_ref - * - * dir_ref - * "val" : val w/ "42" - * - */ - - dir = treeobj_create_dir (); - treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); - - cache_insert (cache, dir_ref, cache_entry_create (dir)); - + /* This root is an empty root */ root = treeobj_create_dir (); - treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1484,9 +1424,7 @@ void commit_process_big_fileval (void) { commit_mgr_t *cm; commit_t *c; json_t *root; - json_t *dir; href_t root_ref; - href_t dir_ref; const char *newroot; int bigstrsize = BLOBREF_MAX_STRING_SIZE * 2; char bigstr[bigstrsize]; @@ -1498,23 +1436,11 @@ void commit_process_big_fileval (void) { /* This root is * * root_ref - * "dir" : dirref to dir_ref - * - * dir_ref * "val" : val w/ "42" - * */ - dir = treeobj_create_dir (); - treeobj_insert_entry (dir, "val", treeobj_create_val ("42", 2)); - - ok (kvs_util_json_hash ("sha1", dir, dir_ref) == 0, - "kvs_util_json_hash worked"); - - cache_insert (cache, dir_ref, cache_entry_create (dir)); - root = treeobj_create_dir (); - treeobj_insert_entry (root, "dir", treeobj_create_dirref (dir_ref)); + treeobj_insert_entry (root, "val", treeobj_create_val ("42", 2)); ok (kvs_util_json_hash ("sha1", root, root_ref) == 0, "kvs_util_json_hash worked"); @@ -1528,7 +1454,7 @@ void commit_process_big_fileval (void) { for (i = 0; i < bigstrsize - 1; i++) bigstr[i] = 'a'; - create_ready_commit (cm, "fence1", "dir.val", bigstr, 0); + create_ready_commit (cm, "fence1", "val", bigstr, 0); ok ((c = commit_mgr_get_ready_commit (cm)) != NULL, "commit_mgr_get_ready_commit returns ready commit"); @@ -1545,7 +1471,7 @@ void commit_process_big_fileval (void) { ok ((newroot = commit_get_newroot_ref (c)) != NULL, "commit_get_newroot_ref returns != NULL when processing complete"); - verify_value (cache, newroot, "dir.val", bigstr); + verify_value (cache, newroot, "val", bigstr); commit_mgr_destroy (cm); cache_destroy (cache); From 3c633c702125a41a9025db19c93b6183c926c8e9 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 9 Aug 2017 07:20:30 -0700 Subject: [PATCH 22/27] modules/kvs: Update main source module to RFC 11 --- src/modules/kvs/kvs.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 621b4a1a7852..4b06b6aae78d 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -42,7 +42,7 @@ #include "src/common/libutil/monotime.h" #include "src/common/libutil/tstat.h" #include "src/common/libutil/log.h" -#include "src/common/libkvs/jansson_dirent.h" +#include "src/common/libkvs/treeobj.h" #include "waitqueue.h" #include "cache.h" @@ -705,7 +705,7 @@ static void get_request_cb (flux_t *h, flux_msg_handler_t *w, json_t *root_dirent = NULL; json_t *tmp_dirent = NULL; lookup_t *lh = NULL; - json_t *root_ref = NULL; + const char *root_ref = NULL; wait_t *wait = NULL; int rc = -1; int ret; @@ -729,8 +729,9 @@ static void get_request_cb (flux_t *h, flux_msg_handler_t *w, * Otherwise, use the current root. */ if (root_dirent) { - if (j_dirent_validate (root_dirent) < 0 - || !(root_ref = json_object_get (root_dirent, "DIRREF"))) { + if (treeobj_validate (root_dirent) < 0 + || !treeobj_is_dirref (root_dirent) + || !(root_ref = treeobj_get_blobref (root_dirent, 0))) { errno = EINVAL; goto done; } @@ -739,7 +740,7 @@ static void get_request_cb (flux_t *h, flux_msg_handler_t *w, if (!(lh = lookup_create (ctx->cache, ctx->epoch, ctx->rootdir, - json_string_value (root_ref), + root_ref, key, flags))) goto done; @@ -785,8 +786,8 @@ static void get_request_cb (flux_t *h, flux_msg_handler_t *w, if (!root_dirent) { char *tmprootref = (char *)lookup_get_root_ref (lh); - if (!(tmp_dirent = j_dirent_create ("DIRREF", tmprootref))) { - flux_log_error (h, "%s: j_dirent_create", __FUNCTION__); + if (!(tmp_dirent = treeobj_create_dirref (tmprootref))) { + flux_log_error (h, "%s: treeobj_create_dirref", __FUNCTION__); goto done; } root_dirent = tmp_dirent; @@ -1668,8 +1669,8 @@ int mod_main (flux_t *h, int argc, char **argv) json_t *rootdir; href_t href; - if (!(rootdir = json_object ())) { - flux_log_error (h, "json_object"); + if (!(rootdir = treeobj_create_dir ())) { + flux_log_error (h, "treeobj_create_dir"); goto done; } From 9ad8e05cd12bfece4bc525df6c16b964febe9519 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 14 Aug 2017 16:10:45 -0700 Subject: [PATCH 23/27] t/kvs: Fix error in null tests In original test of setting the string "null" within the KVS there was an error that worked out by chance but was incorrect. When doing a kvs put with the string "null", such as with: flux kvs put key=null this should create a json null object. On the other hand: flux kvs put key=\"null\" should create a string "null". Correct for this and create tests for both scenarios. --- t/t1000-kvs.t | 20 ++++++++++++++++---- t/t1002-kvs-extra.t | 8 +++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/t/t1000-kvs.t b/t/t1000-kvs.t index 8f8be6924f5c..c4b3f7e4a6a1 100755 --- a/t/t1000-kvs.t +++ b/t/t1000-kvs.t @@ -46,9 +46,12 @@ test_expect_success 'kvs: string put' ' test_expect_success 'kvs: empty string put' ' flux kvs put $KEY.emptystring= ' -test_expect_success 'kvs: JSON null is converted to string put' ' +test_expect_success 'kvs: null is converted to json null' ' flux kvs put $KEY.jsonnull=null ' +test_expect_success 'kvs: quoted null is converted to string' ' + flux kvs put $KEY.strnull=\"null\" +' test_expect_success 'kvs: boolean true put' ' flux kvs put $KEY.booleantrue=true ' @@ -76,9 +79,12 @@ test_expect_success 'kvs: string get' ' test_expect_success 'kvs: empty string get' ' test_kvs_key $KEY.emptystring "" ' -test_expect_success 'kvs: JSON null is converted to string get' ' - test_kvs_key $KEY.jsonnull "null" +test_expect_success 'kvs: null is converted to json null' ' + test_kvs_key $KEY.jsonnull nil ' +test_expect_success 'kvs: quoted null is converted to string' ' + test_kvs_key $KEY.strnull null +# ' test_expect_success 'kvs: boolean true get' ' test_kvs_key $KEY.booleantrue true ' @@ -108,9 +114,10 @@ $KEY.booleantrue = true $KEY.double = 3.140000 $KEY.emptystring = $KEY.integer = 42 -$KEY.jsonnull = null +$KEY.jsonnull = nil $KEY.object = {"a": 42} $KEY.string = foo +$KEY.strnull = null EOF test_cmp expected output ' @@ -126,6 +133,7 @@ $KEY.integer $KEY.jsonnull $KEY.object $KEY.string +$KEY.strnull EOF test_cmp expected output ' @@ -149,6 +157,10 @@ test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.jsonnull && test_must_fail flux kvs get $KEY.jsonnull ' +test_expect_success 'kvs: unlink works' ' + flux kvs unlink $KEY.strnull && + test_must_fail flux kvs get $KEY.strnull +' test_expect_success 'kvs: unlink works' ' flux kvs unlink $KEY.booleantrue && test_must_fail flux kvs get $KEY.booleantrue diff --git a/t/t1002-kvs-extra.t b/t/t1002-kvs-extra.t index a1862f00f4c4..117030e499e9 100755 --- a/t/t1002-kvs-extra.t +++ b/t/t1002-kvs-extra.t @@ -55,8 +55,14 @@ test_expect_success 'kvs: value can be empty' ' test_kvs_key $KEY "" && test_kvs_type $KEY string ' -test_expect_success 'kvs: put JSON null is converted to string' ' +test_expect_success 'kvs: null is converted to json null' ' flux kvs put $KEY=null && + test_kvs_key $KEY nil && + test_kvs_type $KEY null +' + +test_expect_success 'kvs: quoted null is converted to string' ' + flux kvs put $KEY=\"null\" && test_kvs_key $KEY null && test_kvs_type $KEY string ' From 922167a0750e755f775eacc112195144dc9a4549 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 14 Aug 2017 19:36:23 -0700 Subject: [PATCH 24/27] cmd/kvs,t/kvs,bindings/lua: Fix readlink w/ treeobj With change to treeobj implementation, readlink code should be adjusted, calling flux_kvs_lookup_get() instead of flux_kvs_lookup_get_unpack(). --- src/bindings/lua/flux-lua.c | 2 +- src/cmd/flux-kvs.c | 4 ++-- t/kvs/basic.c | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bindings/lua/flux-lua.c b/src/bindings/lua/flux-lua.c index d5d4c6cd219d..1f00f6bb59e4 100644 --- a/src/bindings/lua/flux-lua.c +++ b/src/bindings/lua/flux-lua.c @@ -315,7 +315,7 @@ static int l_flux_kvs_type (lua_State *L) return lua_pusherror (L, "key expected in arg #2"); if ((future = flux_kvs_lookup (f, FLUX_KVS_READLINK, key)) - && flux_kvs_lookup_get_unpack (future, "s", &target) == 0) { + && flux_kvs_lookup_get (future, &target) == 0) { lua_pushstring (L, "symlink"); lua_pushstring (L, target); flux_future_destroy (future); diff --git a/src/cmd/flux-kvs.c b/src/cmd/flux-kvs.c index 201c46ca1f23..b984e93a56f6 100644 --- a/src/cmd/flux-kvs.c +++ b/src/cmd/flux-kvs.c @@ -480,7 +480,7 @@ int cmd_readlink (optparse_t *p, int argc, char **argv) for (i = optindex; i < argc; i++) { if (!(f = flux_kvs_lookup (h, FLUX_KVS_READLINK, argv[i])) - || flux_kvs_lookup_get_unpack (f, "s", &target) < 0) + || flux_kvs_lookup_get (f, &target) < 0) log_err_exit ("%s", argv[i]); else printf ("%s\n", target); @@ -755,7 +755,7 @@ static void dump_kvs_dir (kvsdir_t *dir, bool Ropt, bool dopt) if (kvsdir_issymlink (dir, name)) { const char *link; if (!(f = flux_kvs_lookupat (h, FLUX_KVS_READLINK, key, rootref)) - || flux_kvs_lookup_get_unpack (f, "s", &link) < 0) + || flux_kvs_lookup_get (f, &link) < 0) log_err_exit ("%s", key); printf ("%s -> %s\n", key, link); flux_future_destroy (f); diff --git a/t/kvs/basic.c b/t/kvs/basic.c index 4cd9164e6281..8954b1dae9a8 100644 --- a/t/kvs/basic.c +++ b/t/kvs/basic.c @@ -356,7 +356,7 @@ static void dump_kvs_dir (kvsdir_t *dir, bool ropt) if (kvsdir_issymlink (dir, name)) { const char *link; if (!(f = flux_kvs_lookupat (h, FLUX_KVS_READLINK, key, rootref)) - || flux_kvs_lookup_get_unpack (f, "s", &link) < 0) + || flux_kvs_lookup_get (f, &link) < 0) log_err_exit ("%s", key); printf ("%s -> %s\n", key, link); flux_future_destroy (f); @@ -490,7 +490,7 @@ void cmd_readlinkat (flux_t *h, int argc, char **argv) if (argc != 2) log_msg_exit ("readlink: specify treeobj and key"); if (!(f = flux_kvs_lookupat (h, FLUX_KVS_READLINK, argv[1], argv[0])) - || flux_kvs_lookup_get_unpack (f, "s", &target) < 0) + || flux_kvs_lookup_get (f, &target) < 0) log_err_exit ("%s", argv[1]); else printf ("%s\n", target); From 581390cd3f3e5ddba2481f50fb85511f72ea1b7d Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Mon, 14 Aug 2017 22:45:29 -0700 Subject: [PATCH 25/27] t/kvs: Update t1002-kvs-extra tests for RFC 11 Update t1002-kvs-extra.t tests for RFC 11, notably adjusting former dirent types (i.e. FILEVAL) for treeobj types (i.e. "val"). --- t/t1002-kvs-extra.t | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/t/t1002-kvs-extra.t b/t/t1002-kvs-extra.t index 117030e499e9..d690ba90a1e5 100755 --- a/t/t1002-kvs-extra.t +++ b/t/t1002-kvs-extra.t @@ -158,34 +158,34 @@ test_expect_success 'kvs: get_symlinkat works after symlink unlinked' ' test "$LINKVAL" = "$TEST.a.b.X" ' -test_expect_success 'kvs: get-treeobj: returns directory reference for root' ' +test_expect_success 'kvs: get-treeobj: returns dirref object for root' ' flux kvs unlink -Rf $TEST && - ${KVSBASIC} get-treeobj . | grep -q "DIRREF" + ${KVSBASIC} get-treeobj . | grep -q \"dirref\" ' -test_expect_success 'kvs: get-treeobj: returns directory reference for directory' ' +test_expect_success 'kvs: get-treeobj: returns dirref object for directory' ' flux kvs unlink -Rf $TEST && flux kvs mkdir $TEST.a && - ${KVSBASIC} get-treeobj $TEST.a | grep -q "DIRREF" + ${KVSBASIC} get-treeobj $TEST.a | grep -q \"dirref\" ' -test_expect_success 'kvs: get-treeobj: returns value for small value' ' +test_expect_success 'kvs: get-treeobj: returns val object for small value' ' flux kvs unlink -Rf $TEST && flux kvs put $TEST.a=b && - ${KVSBASIC} get-treeobj $TEST.a | grep -q "FILEVAL" + ${KVSBASIC} get-treeobj $TEST.a | grep -q \"val\" ' test_expect_success 'kvs: get-treeobj: returns value ref for large value' ' flux kvs unlink -Rf $TEST && dd if=/dev/zero bs=4096 count=1 | ${KVSBASIC} copy-tokvs $TEST.a - && - ${KVSBASIC} get-treeobj $TEST.a | grep -q "FILEREF" + ${KVSBASIC} get-treeobj $TEST.a | grep -q \"valref\" ' test_expect_success 'kvs: get-treeobj: returns link value for symlink' ' flux kvs unlink -Rf $TEST && flux kvs put $TEST.a.b.X=42 && flux kvs link $TEST.a.b.X $TEST.a.b.link && - ${KVSBASIC} get-treeobj $TEST.a.b.link | grep -q LINKVAL + ${KVSBASIC} get-treeobj $TEST.a.b.link | grep -q \"symlink\" ' test_expect_success 'kvs: put-treeobj: can make root snapshot' ' @@ -212,31 +212,31 @@ test_expect_success 'kvs: put-treeobj: fails bad dirent: not JSON' ' test_expect_success 'kvs: put-treeobj: fails bad dirent: unknown type' ' flux kvs unlink -Rf $TEST && - test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"ERSTWHILE\":\"fubar\"}" + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":\"MQA=\",\"type\":\"FOO\",\"ver\":1}" ' -test_expect_success 'kvs: put-treeobj: fails bad dirent: bad link type' ' +test_expect_success 'kvs: put-treeobj: fails bad dirent: bad link data' ' flux kvs unlink -Rf $TEST && - test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"LINKVAL\":42}" + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":42,\"type\":\"symlink\",\"ver\":1}" ' -test_expect_success 'kvs: put-treeobj: fails bad dirent: bad ref type' ' +test_expect_success 'kvs: put-treeobj: fails bad dirent: bad ref data' ' flux kvs unlink -Rf $TEST && - test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"DIRREF\":{}}" + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":42,\"type\":\"dirref\",\"ver\":1}" ' test_expect_success 'kvs: put-treeobj: fails bad dirent: bad blobref' ' flux kvs unlink -Rf $TEST && - test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"DIRREF\":\"sha2-aaa\"}" && - test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"DIRREF\":\"sha1-bbb\"}" + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":[\"sha1-aaa\"],\"type\":\"dirref\",\"ver\":1}" && + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":[\"sha1-bbb\"],\"type\":\"dirref\",\"ver\":1}" ' test_expect_success 'kvs: getat: fails bad on dirent' ' flux kvs unlink -Rf $TEST && test_must_fail ${KVSBASIC} getat 42 $TEST.a && - test_must_fail ${KVSBASIC} getat "{\"DIRREF\":\"sha2-aaa\"}" $TEST.a && - test_must_fail ${KVSBASIC} getat "{\"DIRREF\":\"sha1-bbb\"}" $TEST.a && - test_must_fail ${KVSBASIC} getat "{\"DIRVAL\":{}}" $TEST.a + test_must_fail ${KVSBASIC} getat "{\"data\":[\"sha1-aaa\"],\"type\":\"dirref\",\"ver\":1}" $TEST.a && + test_must_fail ${KVSBASIC} getat "{\"data\":[\"sha1-bbb\"],\"type\":\"dirref\",\"ver\":1}" $TEST.a && + test_must_fail ${KVSBASIC} getat "{\"data\":42,\"type\":\"dirref\",\"ver\":1}" $TEST.a ' test_expect_success 'kvs: getat: works on root from get-treeobj' ' From 8657feeaa8fe23f7d975a0e48166541fd2d72df7 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 15 Aug 2017 08:56:16 -0700 Subject: [PATCH 26/27] t/kvs: Add more tests to t1002-kvs-extra Add additional bad treeobj test cases. --- t/t1002-kvs-extra.t | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t1002-kvs-extra.t b/t/t1002-kvs-extra.t index d690ba90a1e5..f3e92ba4a508 100755 --- a/t/t1002-kvs-extra.t +++ b/t/t1002-kvs-extra.t @@ -222,7 +222,8 @@ test_expect_success 'kvs: put-treeobj: fails bad dirent: bad link data' ' test_expect_success 'kvs: put-treeobj: fails bad dirent: bad ref data' ' flux kvs unlink -Rf $TEST && - test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":42,\"type\":\"dirref\",\"ver\":1}" + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":42,\"type\":\"dirref\",\"ver\":1}" && + test_must_fail ${KVSBASIC} put-treeobj $TEST.a="{\"data\":"sha1-4087718d190b373fb490b27873f61552d7f29dbe",\"type\":\"dirref\",\"ver\":1}" ' test_expect_success 'kvs: put-treeobj: fails bad dirent: bad blobref' ' @@ -236,7 +237,8 @@ test_expect_success 'kvs: getat: fails bad on dirent' ' test_must_fail ${KVSBASIC} getat 42 $TEST.a && test_must_fail ${KVSBASIC} getat "{\"data\":[\"sha1-aaa\"],\"type\":\"dirref\",\"ver\":1}" $TEST.a && test_must_fail ${KVSBASIC} getat "{\"data\":[\"sha1-bbb\"],\"type\":\"dirref\",\"ver\":1}" $TEST.a && - test_must_fail ${KVSBASIC} getat "{\"data\":42,\"type\":\"dirref\",\"ver\":1}" $TEST.a + test_must_fail ${KVSBASIC} getat "{\"data\":42,\"type\":\"dirref\",\"ver\":1}" $TEST.a && + test_must_fail ${KVSBASIC} getat "{\"data\":"sha1-4087718d190b373fb490b27873f61552d7f29dbe",\"type\":\"dirref\",\"ver\":1}" $TEST.a ' test_expect_success 'kvs: getat: works on root from get-treeobj' ' From de398d8645f8a16e1a4f585359280a7a1e217ead Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Thu, 17 Aug 2017 17:36:16 -0700 Subject: [PATCH 27/27] common/libkvs: Remove jansson_dirent Remove jansson_dirent internal library, as it is no longer used. Remove lingering includes of jansson_dirent.h. --- src/common/libkvs/Makefile.am | 7 -- src/common/libkvs/jansson_dirent.c | 131 ------------------------ src/common/libkvs/jansson_dirent.h | 48 --------- src/common/libkvs/kvs_commit.c | 2 - src/common/libkvs/test/jansson_dirent.c | 85 --------------- 5 files changed, 273 deletions(-) delete mode 100644 src/common/libkvs/jansson_dirent.c delete mode 100644 src/common/libkvs/jansson_dirent.h delete mode 100644 src/common/libkvs/test/jansson_dirent.c diff --git a/src/common/libkvs/Makefile.am b/src/common/libkvs/Makefile.am index 7150a5ef6b70..cf276729da58 100644 --- a/src/common/libkvs/Makefile.am +++ b/src/common/libkvs/Makefile.am @@ -17,8 +17,6 @@ libkvs_la_SOURCES = \ kvs_dir.c \ kvs_classic.c \ kvs_watch.c \ - jansson_dirent.c \ - jansson_dirent.h \ kvs_commit.c \ kvs_txn.c \ kvs_txn_private.h \ @@ -35,7 +33,6 @@ fluxcoreinclude_HEADERS = \ kvs_commit.h TESTS = \ - test_jansson_dirent.t \ test_kvs_txn.t \ test_kvs_lookup.t \ test_kvs_dir.t \ @@ -59,10 +56,6 @@ test_cppflags = \ $(AM_CPPFLAGS) \ -I$(top_srcdir)/src/common/libtap -test_jansson_dirent_t_SOURCES = test/jansson_dirent.c -test_jansson_dirent_t_CPPFLAGS = $(test_cppflags) -test_jansson_dirent_t_LDADD = $(test_ldadd) - test_kvs_txn_t_SOURCES = test/kvs_txn.c test_kvs_txn_t_CPPFLAGS = $(test_cppflags) test_kvs_txn_t_LDADD = $(test_ldadd) $(LIBDL) diff --git a/src/common/libkvs/jansson_dirent.c b/src/common/libkvs/jansson_dirent.c deleted file mode 100644 index 20aa2917a360..000000000000 --- a/src/common/libkvs/jansson_dirent.c +++ /dev/null @@ -1,131 +0,0 @@ -/*****************************************************************************\ - * Copyright (c) 2014 Lawrence Livermore National Security, LLC. Produced at - * the Lawrence Livermore National Laboratory (cf, AUTHORS, DISCLAIMER.LLNS). - * LLNL-CODE-658032 All rights reserved. - * - * This file is part of the Flux resource manager framework. - * For details, see https://github.com/flux-framework. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the Free - * Software Foundation; either version 2 of the license, or (at your option) - * any later version. - * - * Flux is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the IMPLIED WARRANTY OF MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the terms and conditions of the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA. - * See also: http://www.gnu.org/licenses/ -\*****************************************************************************/ - -#if HAVE_CONFIG_H -#include "config.h" -#endif -#include -#include -#include -#include - -#include "src/common/libutil/blobref.h" - -#include "jansson_dirent.h" - -json_t *j_dirent_create (const char *type, void *arg) -{ - json_t *dirent = NULL; - bool valid_type = false; - - if (!(dirent = json_object ())) { - errno = ENOMEM; - goto error; - } - - if (!strcmp (type, "FILEREF") || !strcmp (type, "DIRREF")) { - char *ref = arg; - json_t *o; - - if (!(o = json_string (ref))) { - errno = ENOMEM; - goto error; - } - if (json_object_set_new (dirent, type, o) < 0) { - json_decref (o); - errno = ENOMEM; - goto error; - } - - valid_type = true; - } else if (!strcmp (type, "FILEVAL") || !strcmp (type, "DIRVAL") - || !strcmp (type, "LINKVAL")) { - json_t *val = arg; - - if (val) - json_incref (val); - else { - if (!(val = json_object ())) { - errno = ENOMEM; - goto error; - } - } - if (json_object_set_new (dirent, type, val) < 0) { - json_decref (val); - errno = ENOMEM; - goto error; - } - valid_type = true; - } - assert (valid_type == true); - - return dirent; - -error: - json_decref (dirent); - return NULL; -} - -int j_dirent_validate (json_t *dirent) -{ - json_t *o; - - if (!dirent) - goto error; - if ((o = json_object_get (dirent, "DIRVAL"))) { - const char *key; - json_t *val; - json_object_foreach (o, key, val) { - if (j_dirent_validate (val) < 0) - goto error; - } - } - else if ((o = json_object_get (dirent, "FILEVAL"))) { - if (json_typeof (o) == JSON_NULL) - goto error; - } - else if ((o = json_object_get (dirent, "LINKVAL"))) { - if (json_typeof (o) != JSON_STRING) - goto error; - } - else if ((o = json_object_get (dirent, "DIRREF")) - || (o = json_object_get (dirent, "FILEREF"))) { - if (json_typeof (o) != JSON_STRING) - goto error; - const char *s = json_string_value (o); - uint8_t hash[BLOBREF_MAX_DIGEST_SIZE]; - if (blobref_strtohash (s, hash, sizeof (hash)) < 0) - goto error; - } - else - goto error; - return 0; -error: - errno = EINVAL; - return -1; -} - -/* - * vi:tabstop=4 shiftwidth=4 expandtab - */ diff --git a/src/common/libkvs/jansson_dirent.h b/src/common/libkvs/jansson_dirent.h deleted file mode 100644 index c2be15a68b77..000000000000 --- a/src/common/libkvs/jansson_dirent.h +++ /dev/null @@ -1,48 +0,0 @@ -#include -#include - -/* JSON directory object: - * list of key-value pairs where key is a name, value is a dirent - * - * JSON dirent objects: - * object containing one key-value pair where key is one of - * "FILEREF", "DIRREF", "FILEVAL", "DIRVAL", "LINKVAL", and value is a - * blobref key into ctx->store (FILEREF, DIRREF), an actual directory, file - * (value), or link target JSON object (FILEVAL, DIRVAL, LINKVAL). - * - * For example, consider KVS containing: - * a="foo" - * b="bar" - * c.d="baz" - * X -> c.d - * - * Root directory: - * {"a":{"FILEREF":"f1d2d2f924e986ac86fdf7b36c94bcdf32beec15"}, - * "b":{"FILEREF","8714e0ef31edb00e33683f575274379955b3526c"}, - * "c":{"DIRREF","6eadd3a778e410597c85d74c287a57ad66071a45"}, - * "X":{"LINKVAL","c.d"}} - * - * Deep copy of root directory: - * {"a":{"FILEVAL":"foo"}, - * "b":{"FILEVAL","bar"}, - * "c":{"DIRVAL",{"d":{"FILEVAL":"baz"}}}, - * "X":{"LINKVAL","c.d"}} - * - * On LINKVAL's: - * - target is always fully qualified key name - * - links are always followed in path traversal of intermediate directories - * - for kvs_get, terminal links are only followed if 'readlink' flag is set - * - for kvs_put, terminal links are never followed - */ - -/* Create a KVS dirent. - * 'type' is one of { "FILEREF", "DIRREF", "FILEVAL", "DIRVAL", "LINKVAL" }. - * 'arg' is dependent on the type. This function asserts on failure. - */ -json_t *j_dirent_create (const char *type, void *arg); - -int j_dirent_validate (json_t *dirent); - -/* - * vi:tabstop=4 shiftwidth=4 expandtab - */ diff --git a/src/common/libkvs/kvs_commit.c b/src/common/libkvs/kvs_commit.c index 1981e2298ef9..818d9b846fa8 100644 --- a/src/common/libkvs/kvs_commit.c +++ b/src/common/libkvs/kvs_commit.c @@ -29,8 +29,6 @@ #include #include -#include "jansson_dirent.h" - #include "kvs_txn_private.h" #include "src/common/libutil/blobref.h" diff --git a/src/common/libkvs/test/jansson_dirent.c b/src/common/libkvs/test/jansson_dirent.c deleted file mode 100644 index 0b0a6676ce73..000000000000 --- a/src/common/libkvs/test/jansson_dirent.c +++ /dev/null @@ -1,85 +0,0 @@ -#include -#include - -#include "src/common/libkvs/jansson_dirent.h" -#include "src/common/libtap/tap.h" - -void jdiag (json_t *o) -{ - if (o) { - char *tmp = json_dumps (o, JSON_COMPACT); - diag ("%s", tmp); - free (tmp); - } else - diag ("nil"); -} - -int main (int argc, char *argv[]) -{ - json_t *d1; - json_t *d2; - json_t *dir; - char *s; - - plan (NO_PLAN); - - d1 = j_dirent_create ("FILEREF", "sha1-fbedb4eb241948f6f802bf47d95ec932e9d4deaf"); - d2 = j_dirent_create ("FILEREF", "sha1-fbedb4eb241948f6f802bf47d95ec932e9d4deaf"); - ok (d1 && d2, - "j_dirent_create FILEREF works"); - jdiag (d1); - jdiag (d2); - ok (json_equal (d1, d2), - "json_equal says identical dirents match"); - ok (j_dirent_validate (d1) == 0 && j_dirent_validate (d2) == 0, - "j_dirent_validate says they are valid"); - json_decref (d1); - json_decref (d2); - - /* ownership of new objects transferred to dirents */ - d1 = j_dirent_create ("FILEVAL", json_integer (42)); - d2 = j_dirent_create ("FILEVAL", json_string ("hello world")); - ok (d1 && d2, - "j_dirent_create FILEVAL works"); - jdiag (d1); - jdiag (d2); - ok (json_equal (d1, d2) == false, - "json_equal says different dirents are different"); - ok (j_dirent_validate (d1) == 0 && j_dirent_validate (d2) == 0, - "j_dirent_validate says they are valid"); - json_decref (d1); - json_decref (d2); - - dir = json_object (); - json_object_set_new (dir, "foo", j_dirent_create ("FILEVAL", json_integer (33))); - json_object_set_new (dir, "bar", j_dirent_create ("FILEVAL", json_string ("Mrrrrnn?"))); - d1 = j_dirent_create ("DIRVAL", dir); - ok (d1 != NULL, - "j_dirent_create DIRVAL works"); - ok (j_dirent_validate (d1) == 0, - "j_dirent_validate says it is valid"); - jdiag (d1); - json_decref (d1); - - /* jansson: How is "null" decoded? How is NULL encoded? */ - json_t *o; - char *str; - o = json_loads ("null", JSON_DECODE_ANY, NULL); - ok (o != NULL, - "json_loads (\"null\") decodes as valid json_t"); - str = json_dumps (o, JSON_ENCODE_ANY); - ok (str != NULL && !strcmp (str, "null"), - "json_dumps encodes returned object as \"null\""); - free (str); - json_decref (o); - s = json_dumps (NULL, JSON_ENCODE_ANY); - ok (s == NULL, - "json_dumps (NULL) returns NULL, which is a failure"); - - done_testing(); - return (0); -} - -/* - * vi:tabstop=4 shiftwidth=4 expandtab - */