Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add public current depth field to JSON reader. #1802

Merged
merged 5 commits into from
Jul 6, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions sdk/inc/azure/core/az_json.h
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,11 @@ typedef struct
/// and it shouldn't be modified by the caller.
az_json_token token;

/// The depth of the current token. This read-only field tracks the recursive depth of the nested
/// objects or arrays within the JSON text processed so far, and it shouldn't be modified by the
/// caller.
int32_t current_depth;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeffreyRichter, do you think it makes more sense to expose this field from the az_json_token struct rather than from the az_json_reader? I have it from the reader because it is easier to access from the caller (the user doesn't have to fish it out from the nested token field), but the depth can be thought of as a property of a particular JSON token.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you share the customer code that uses this field. The usage patterns would probably highlight if it should be on the reader on the token.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did above in the PR description.

Copy link
Member Author

@ahsonkhan ahsonkhan Jun 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I didn't want to add it to the token is, right now, the token can be created as a standalone type, and used outside of the reader (at least when looking at the public fields). Adding depth to it turns it into something that has to be a part of a larger JSON payload (and then we might not be able to use it in the future for other cases like in the writer).

Also, in .NET, I had added the depth on the reader directly (but we didn't have the nested token struct there) :)

The reason why I posed the question is because I feel that if we emphasize single responsibility, a case can easily be made to move the field to az_json_token.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All that said, this is still in beta and we can revisit it separately, if necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw I'd prefer in the json reader rather than the token unless there's a good reason for it to be in the token.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will close on it later, and decide if a change is needed or we are happy where we are:
#1808


struct
{
/// The first buffer containing the JSON payload.
Expand Down
14 changes: 13 additions & 1 deletion sdk/src/azure/core/az_json_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ AZ_NODISCARD az_result az_json_reader_init(
.end_buffer_offset = -1,
},
},
.current_depth = 0,
._internal = {
.json_buffer = json_buffer,
.json_buffers = &AZ_SPAN_EMPTY,
Expand All @@ -57,7 +58,8 @@ AZ_NODISCARD az_result az_json_reader_chunked_init(
_az_PRECONDITION(number_of_buffers >= 1);
_az_PRECONDITION(az_span_size(json_buffers[0]) >= 1);

*out_json_reader = (az_json_reader){
*out_json_reader = (az_json_reader)
{
.token = (az_json_token){
.kind = AZ_JSON_TOKEN_NONE,
.slice = AZ_SPAN_EMPTY,
Expand All @@ -72,6 +74,7 @@ AZ_NODISCARD az_result az_json_reader_chunked_init(
.end_buffer_offset = -1,
},
},
.current_depth = 0,
._internal = {
.json_buffer = json_buffers[0],
.json_buffers = json_buffers,
Expand Down Expand Up @@ -104,6 +107,15 @@ static void _az_json_reader_update_state(
{
ref_json_reader->token.kind = token_kind;
ref_json_reader->token.size = consumed;
ref_json_reader->current_depth = ref_json_reader->_internal.bit_stack._internal.current_depth;

// The depth of the start of the container will be one less than the bit stack managing the state.
// That is because we push on the stack when we see a start of the container (above in the call
// stack), but it's actual depth and "indentation" level is one lower.
ahsonkhan marked this conversation as resolved.
Show resolved Hide resolved
if (token_kind == AZ_JSON_TOKEN_BEGIN_ARRAY || token_kind == AZ_JSON_TOKEN_BEGIN_OBJECT)
{
ref_json_reader->current_depth--;
}

ref_json_reader->_internal.bytes_consumed += current_segment_consumed;
ref_json_reader->_internal.total_bytes_consumed += consumed;
Expand Down
145 changes: 145 additions & 0 deletions sdk/tests/core/test_az_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,13 @@ static void test_json_reader_init(void** state)

az_json_reader reader = { 0 };

assert_int_equal(reader.current_depth, 0);

assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{}"), NULL), AZ_OK);
assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{}"), &options), AZ_OK);

assert_int_equal(reader.current_depth, 0);

// Verify that initialization doesn't process any JSON text, even if it is invalid or incomplete.
assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" "), NULL), AZ_OK);
assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" "), &options), AZ_OK);
Expand All @@ -59,6 +63,119 @@ static void test_json_reader_init(void** state)
assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("\""), &options), AZ_OK);

TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NONE, AZ_SPAN_EMPTY);

assert_int_equal(reader.current_depth, 0);
}

static void test_json_reader_current_depth_array(void** state)
{
(void)state;

az_json_reader reader = { 0 };

assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("[ ]"), NULL), AZ_OK);
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);

assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("[ [ 1, 2, 3] ]"), NULL), AZ_OK);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 0);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 1);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 1);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);

assert_int_equal(az_json_reader_next_token(&reader), AZ_ERROR_JSON_READER_DONE);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);
}

static void test_json_reader_current_depth_object(void** state)
{
(void)state;

az_json_reader reader = { 0 };

assert_int_equal(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{}"), NULL), AZ_OK);
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);

assert_int_equal(
az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{\"array\": [1,2,3,{}]}"), NULL), AZ_OK);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 0);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 1);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 1);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 3);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 2);
assert_int_equal(reader.current_depth, 2);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 1);

TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);

assert_int_equal(az_json_reader_next_token(&reader), AZ_ERROR_JSON_READER_DONE);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);
}

/** Json writer **/
Expand Down Expand Up @@ -1033,55 +1150,64 @@ static void test_json_reader(void** state)
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" "), NULL));
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_UNEXPECTED_END);
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NONE, AZ_SPAN_EMPTY);
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" null "), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NULL, AZ_SPAN_FROM_STR("null"));
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" nul"), NULL));
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_UNEXPECTED_END);
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NONE, AZ_SPAN_EMPTY);
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" false"), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_FALSE, AZ_SPAN_FROM_STR("false"));
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" falsx "), NULL));
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_UNEXPECTED_CHAR);
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NONE, AZ_SPAN_EMPTY);
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("true "), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_TRUE, AZ_SPAN_FROM_STR("true"));
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" truem"), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_TRUE, AZ_SPAN_FROM_STR("true"));
}
{
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" 123a"), NULL));
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_UNEXPECTED_CHAR);
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NONE, AZ_SPAN_EMPTY);
}
{
az_span const s = AZ_SPAN_FROM_STR(" \"tr\\\"ue\\t\" ");
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, s, NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_STRING, AZ_SPAN_FROM_STR("tr\\\"ue\\t"));
assert_true(az_span_ptr(reader.token.slice) == (az_span_ptr(s) + 2));
}
Expand All @@ -1090,13 +1216,15 @@ static void test_json_reader(void** state)
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, s, NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_STRING, AZ_SPAN_FROM_STR("\\uFf0F"));
assert_true(az_span_ptr(reader.token.slice) == az_span_ptr(s) + 1);
}
{
az_span const s = AZ_SPAN_FROM_STR("\"\\uFf0\"");
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, s, NULL));
assert_int_equal(reader.current_depth, 0);
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_UNEXPECTED_CHAR);
}
/* Testing reading number and converting to double */
Expand All @@ -1105,6 +1233,7 @@ static void test_json_reader(void** state)
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" 23 "), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NUMBER, AZ_SPAN_FROM_STR("23"));

uint64_t actual_u64 = 0;
Expand Down Expand Up @@ -1230,29 +1359,39 @@ static void test_json_reader(void** state)
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR(" [ true, 0.25 ]"), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_BEGIN_ARRAY, AZ_SPAN_FROM_STR("["));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 1);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_TRUE, AZ_SPAN_FROM_STR("true"));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 1);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_NUMBER, AZ_SPAN_FROM_STR("0.25"));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_END_ARRAY, AZ_SPAN_FROM_STR("]"));
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_JSON_READER_DONE);
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_END_ARRAY, AZ_SPAN_FROM_STR("]"));
}
{
az_span const json = AZ_SPAN_FROM_STR("{\"a\":\"Hello world!\"}");
az_json_reader reader = { 0 };
TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, json, NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_BEGIN_OBJECT, AZ_SPAN_FROM_STR("{"));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 1);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_PROPERTY_NAME, AZ_SPAN_FROM_STR("a"));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 1);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_STRING, AZ_SPAN_FROM_STR("Hello world!"));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_END_OBJECT, AZ_SPAN_FROM_STR("}"));
assert_true(az_json_reader_next_token(&reader) == AZ_ERROR_JSON_READER_DONE);
assert_int_equal(reader.current_depth, 0);
TEST_JSON_TOKEN_HELPER(reader.token, AZ_JSON_TOKEN_END_OBJECT, AZ_SPAN_FROM_STR("}"));
}
{
Expand Down Expand Up @@ -1670,13 +1809,15 @@ static void test_json_skip_children(void** state)
TEST_EXPECT_SUCCESS(az_json_reader_skip_children(&reader));
assert_int_equal(reader.token.kind, AZ_JSON_TOKEN_END_OBJECT);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);

TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{\"foo\":{}}"), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
assert_int_equal(reader.token.kind, AZ_JSON_TOKEN_BEGIN_OBJECT);
TEST_EXPECT_SUCCESS(az_json_reader_skip_children(&reader));
assert_int_equal(reader.token.kind, AZ_JSON_TOKEN_END_OBJECT);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 0);
assert_int_equal(reader.current_depth, 0);

TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{\"foo\":{}}"), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
Expand All @@ -1685,6 +1826,7 @@ static void test_json_skip_children(void** state)
TEST_EXPECT_SUCCESS(az_json_reader_skip_children(&reader));
assert_int_equal(reader.token.kind, AZ_JSON_TOKEN_END_OBJECT);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 1);

TEST_EXPECT_SUCCESS(az_json_reader_init(&reader, AZ_SPAN_FROM_STR("{\"foo\":{}}"), NULL));
TEST_EXPECT_SUCCESS(az_json_reader_next_token(&reader));
Expand All @@ -1694,6 +1836,7 @@ static void test_json_skip_children(void** state)
TEST_EXPECT_SUCCESS(az_json_reader_skip_children(&reader));
assert_int_equal(reader.token.kind, AZ_JSON_TOKEN_END_OBJECT);
assert_int_equal(reader._internal.bit_stack._internal.current_depth, 1);
assert_int_equal(reader.current_depth, 1);
}

/** Json Value **/
Expand Down Expand Up @@ -2826,6 +2969,8 @@ int test_az_json()
{
const struct CMUnitTest tests[]
= { cmocka_unit_test(test_json_reader_init),
cmocka_unit_test(test_json_reader_current_depth_array),
cmocka_unit_test(test_json_reader_current_depth_object),
cmocka_unit_test(test_json_writer),
cmocka_unit_test(test_json_writer_append_nested),
cmocka_unit_test(test_json_writer_append_nested_invalid),
Expand Down