Re: [tarantool-patches] Re: [PATCH v1 3/5] lib: introduce json_token_path_snprint

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Hi! Thank you for your comments. A new version bellow:
=========================================

Implemented a helper function for getting a path to a json_token
in a json_tree. When routine is called with size=0, return value
(as always) as the number of characters that would have been
written in case the output string has been large enough.
We will use this function for reporting tuple format violations
in further patches.

Needed for #1012
---
 src/lib/json/json.c   | 71 +++++++++++++++++++++++++++++++++++++++++++
 src/lib/json/json.h   | 24 +++++++++++++++
 test/unit/json.c      | 67 +++++++++++++++++++++++++++++++++++++++-
 test/unit/json.result | 30 +++++++++++++++++-
 4 files changed, 190 insertions(+), 2 deletions(-)

diff --git a/src/lib/json/json.c b/src/lib/json/json.c
index c7909fde2..57449a3aa 100644
--- a/src/lib/json/json.c
+++ b/src/lib/json/json.c
@@ -317,6 +317,77 @@ json_path_validate(const char *path, int path_len, int index_base)
 	return rc;
 }
 
+/**
+ * Helper routine for json_tree_snprint_path to snprintf atomic
+ * token. Works the same way as json_tree_snprint_path, read it's
+ * comment for more details.
+ */
+static int
+json_token_snprint(char *buf, int size, const struct json_token *token,
+		   int index_base)
+{
+	enum json_token_type type = token->type;
+	assert(type == JSON_TOKEN_NUM || type == JSON_TOKEN_STR);
+	int len = 0;
+	if (type == JSON_TOKEN_NUM)
+		len = snprintf(buf, size, "[%d]", token->num + index_base);
+	else if (type == JSON_TOKEN_STR)
+		len = snprintf(buf, size, "[\"%.*s\"]", token->len, token->str);
+	return len;
+}
+
+int
+json_tree_snprint_path(char *buf, int size, const struct json_token *token,
+		       const struct json_token *root, int index_base)
+{
+	/*
+	 * Calculate JSON path string length passing 0-buffer in
+	 * json_token_snprint routine.
+	 */
+	int len = 0;
+	const struct json_token *ptr = token;
+	while (ptr != root && ptr->type != JSON_TOKEN_END) {
+		len += json_token_snprint(NULL, 0, ptr, index_base);
+		ptr = ptr->parent;
+	}
+	if (size == 0)
+		return len;
+
+	/*
+	 * Write to the memory buffer buf as many tokens,
+	 * starting with the root, as it can fit. If the fragment
+	 * of the token does not fit into the buffer, it would be
+	 * truncated.
+	 */
+	int pos = len;
+	char last = '\0';
+	ptr = token;
+	while (ptr != root && ptr->type != JSON_TOKEN_END) {
+		pos -= json_token_snprint(NULL, 0, ptr, index_base);
+		assert(pos >= 0);
+		if (pos + 1 < size) {
+			int rc = json_token_snprint(buf + pos, size - pos, ptr,
+						    index_base);
+			if (rc < 0)
+				return rc;
+			/*
+			 * The json_token_snprint writes a
+			 * '\0'-terminated string in memory
+			 * buffer. The first character in
+			 * token string representation would be
+			 * overwritten on next cycle iteration.
+			 * Let's keep it in 'last' variable to
+			 * restore next time.
+			 */
+			buf[pos + rc] = last;
+			last = buf[pos];
+		}
+		ptr = ptr->parent;
+	}
+	assert(buf[MIN(len, size - 1)] == '\0');
+	return len;
+}
+
 #define MH_SOURCE 1
 #define mh_name _json
 #define mh_key_t struct json_token *
diff --git a/src/lib/json/json.h b/src/lib/json/json.h
index 7ce10ce2c..bed20536d 100644
--- a/src/lib/json/json.h
+++ b/src/lib/json/json.h
@@ -253,6 +253,30 @@ json_path_cmp(const char *a, int a_len, const char *b, int b_len,
 int
 json_path_validate(const char *path, int path_len, int index_base);
 
+/**
+ * Write a JSON tree path path between root and token items to
+ * memory buffer buf, at most size bytes (including the null byte
+ * used to end output to strings).
+ * The parent argument may omitted via passing NULL value. It this
+ * case the whole JSON path would be printed.
+ * When routine is called with size=0, return value (as always)
+ * is the number of characters that would have been written in
+ * case the output string had been large enough.
+ *
+ * Upon successful return, routine returns the number of
+ * characters printed (excluding the null byte used to end output
+ * to strings). If the output was truncated due the size limit,
+ * then the return value is the number of characters (excluding
+ * the terminating null byte) which would have been written to
+ * the final string if enough space had been available.
+ * Thus, a return value of size or more means that the output
+ * was truncated.
+ * If an output error is encountered, a negative value is
+ * returned.
+ */
+int
+json_tree_snprint_path(char *buf, int size, const struct json_token *token,
+		       const struct json_token *root, int index_base);
 /**
  * Initialize a new empty JSON tree.
  *
diff --git a/test/unit/json.c b/test/unit/json.c
index e553ff23c..122e605cf 100644
--- a/test/unit/json.c
+++ b/test/unit/json.c
@@ -438,16 +438,81 @@ test_path_cmp()
 	footer();
 }
 
+void
+test_path_snprintf()
+{
+	header();
+	plan(24);
+
+	struct json_tree tree;
+	int rc = json_tree_create(&tree);
+	fail_if(rc != 0);
+	struct test_struct records[5];
+	const char *path = "[1][20][\"file\"][8]";
+	int path_len = strlen(path);
+
+	int records_idx = 0;
+	struct test_struct *node, *node_tmp;
+	node = test_add_path(&tree, path, path_len, records, &records_idx);
+	fail_if(&node->node != &records[3].node);
+
+	/* Test size estimation. */
+	char buff[64];
+	int len = json_tree_snprint_path(NULL, 0, &node->node, NULL, INDEX_BASE);
+	is(len, path_len, "estimate path length: have %d expected %d",
+	   len, path_len);
+	len = json_tree_snprint_path(NULL, 0, &node->node, &records[0].node,
+				     INDEX_BASE);
+	is(len, 15, "estimate path part length: have %d expected %d", len, 15);
+
+	len = json_tree_snprint_path(NULL, 0, &node->node, &records[1].node,
+				     INDEX_BASE);
+	is(len, 11, "estimate path part length: have %d expected %d", len, 11);
+	len = json_tree_snprint_path(NULL, 0, &node->node, &records[2].node,
+				     INDEX_BASE);
+	is(len, 3, "estimate path part length: have %d expected %d", len, 3);
+
+	/* Test truncate. */
+	for (int i = path_len + 1; i > strrchr(path, '[') - path - 2; i--) {
+		len = json_tree_snprint_path(buff, i + 1, &node->node, NULL,
+					     INDEX_BASE);
+		is(len, path_len, "correct char count: have %d expected %d",
+		   len, path_len);
+		len = MIN(len, i);
+		is(memcmp(buff, path, len), 0,
+		   "correct string fragment: have \"%s\", expected \"%.*s\"",
+		   buff, len, path);
+		is(buff[len], '\0', "terminating '\\0' at appropriate place");
+	}
+
+	/* Test path fragment snprintf. */
+	len = json_tree_snprint_path(buff, 16, &node->node, &records[0].node,
+				     INDEX_BASE);
+	is(memcmp(buff, path + 3, len), 0,
+	   "correct partial JSON path: have \"%s\" expected \"%s\"", buff,
+	   path + 3);
+	is(buff[len], '\0', "terminating '\\0' at appropriate place");
+
+	json_tree_foreach_entry_safe(node, &tree.root, struct test_struct,
+				     node, node_tmp)
+		json_tree_del(&tree, &node->node);
+	json_tree_destroy(&tree);
+
+	check_plan();
+	footer();
+}
+
 int
 main()
 {
 	header();
-	plan(4);
+	plan(5);
 
 	test_basic();
 	test_errors();
 	test_tree();
 	test_path_cmp();
+	test_path_snprintf();
 
 	int rc = check_plan();
 	footer();
diff --git a/test/unit/json.result b/test/unit/json.result
index a17451099..4e18c7ab7 100644
--- a/test/unit/json.result
+++ b/test/unit/json.result
@@ -1,5 +1,5 @@
 	*** main ***
-1..4
+1..5
 	*** test_basic ***
     1..71
     ok 1 - parse <[1]>
@@ -169,4 +169,32 @@ ok 3 - subtests
     ok 7 - path Data[[1]["FIO"].fname error pos 6 expected 6
 ok 4 - subtests
 	*** test_path_cmp: done ***
+	*** test_path_snprintf ***
+    1..24
+    ok 1 - estimate path length: have 18 expected 18
+    ok 2 - estimate path part length: have 15 expected 15
+    ok 3 - estimate path part length: have 11 expected 11
+    ok 4 - estimate path part length: have 3 expected 3
+    ok 5 - correct char count: have 18 expected 18
+    ok 6 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
+    ok 7 - terminating '\0' at appropriate place
+    ok 8 - correct char count: have 18 expected 18
+    ok 9 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
+    ok 10 - terminating '\0' at appropriate place
+    ok 11 - correct char count: have 18 expected 18
+    ok 12 - correct string fragment: have "[1][20]["file"][8", expected "[1][20]["file"][8"
+    ok 13 - terminating '\0' at appropriate place
+    ok 14 - correct char count: have 18 expected 18
+    ok 15 - correct string fragment: have "[1][20]["file"][", expected "[1][20]["file"]["
+    ok 16 - terminating '\0' at appropriate place
+    ok 17 - correct char count: have 18 expected 18
+    ok 18 - correct string fragment: have "[1][20]["file"]", expected "[1][20]["file"]"
+    ok 19 - terminating '\0' at appropriate place
+    ok 20 - correct char count: have 18 expected 18
+    ok 21 - correct string fragment: have "[1][20]["file"", expected "[1][20]["file""
+    ok 22 - terminating '\0' at appropriate place
+    ok 23 - correct partial JSON path: have "[20]["file"][8]" expected "[20]["file"][8]"
+    ok 24 - terminating '\0' at appropriate place
+ok 5 - subtests
+	*** test_path_snprintf: done ***
 	*** main: done ***
-- 
2.19.2