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

chore: refine error message if schema is invalid, add the invalid schema path #195

Merged
merged 8 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
11 changes: 7 additions & 4 deletions core/include_internal/ten_utils/schema/keywords/keyword.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,33 @@
#include "ten_utils/ten_config.h"

#include "ten_utils/container/hash_handle.h"
#include "ten_utils/lib/error.h"
#include "ten_utils/lib/signature.h"
#include "ten_utils/value/value.h"

#define TEN_SCHEMA_KEYWORD_SIGNATURE 0x94F75C7B1835B931U

typedef struct ten_schema_t ten_schema_t;
typedef struct ten_schema_keyword_t ten_schema_keyword_t;
typedef struct ten_schema_error_t ten_schema_error_t;

typedef void (*ten_schema_keyword_destroy_func_t)(ten_schema_keyword_t *self);

typedef bool (*ten_schema_keyword_validate_value_func_t)(
ten_schema_keyword_t *self, ten_value_t *value, ten_error_t *err);
ten_schema_keyword_t *self, ten_value_t *value,
ten_schema_error_t *schema_err);

typedef bool (*ten_schema_keyword_adjust_value_func_t)(
ten_schema_keyword_t *self, ten_value_t *value, ten_error_t *err);
ten_schema_keyword_t *self, ten_value_t *value,
ten_schema_error_t *schema_err);

/**
* @brief Check if the keyword is compatible with the target keyword. Note that
* the `self` or `target` might be NULL (i.e., the keyword is not defined in the
* schema), all implementations should handle this case properly.
*/
typedef bool (*ten_schema_keyword_is_compatible_func_t)(
ten_schema_keyword_t *self, ten_schema_keyword_t *target, ten_error_t *err);
ten_schema_keyword_t *self, ten_schema_keyword_t *target,
ten_schema_error_t *schema_err);

typedef enum TEN_SCHEMA_KEYWORD {
TEN_SCHEMA_KEYWORD_INVALID = 0,
Expand Down
58 changes: 58 additions & 0 deletions core/include_internal/ten_utils/schema/schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "ten_utils/value/value.h"

#define TEN_SCHEMA_SIGNATURE 0x4D9FEA8F6273C974U
#define TEN_SCHEMA_ERROR_SIGNATURE 0x32B696D4FC8FFD09U

typedef struct ten_schema_keyword_type_t ten_schema_keyword_type_t;

Expand Down Expand Up @@ -81,6 +82,54 @@ typedef struct ten_schema_t {
ten_schema_keyword_type_t *keyword_type;
} ten_schema_t;

// Internal use.
//
// The error context to be used during the schema validation process. An example
// of schema is as follows:
//
// {
// "type": "object",
// "properties": {
// "a": {
// "type": "array",
// "items": {
// "type": "int32"
// }
// }
// }
// }
//
// And the value to be validated is as follows:
//
// {
// "a": [1, "2", 3]
// }
//
// Verify each value according to its corresponding schema in DFS order, until
// an error is encountered. After the validation, the error message should
// display which value is invalid, ex: it will be `a[1]` in this case. We need
// to record the path of the schema during the process, besides the error
// message. We can not use ten_error_t to record the path, because there is no
// space to achieve this. Neither can we use the `ten_schema_t` to record the
// path, because we need to know the index, if the value is an array; but there
// is no index information in the ten_schema_array_t because each item in the
// array shares the same schema.
typedef struct ten_schema_error_t {
ten_signature_t signature;
ten_error_t *err;
ten_string_t path;
} ten_schema_error_t;

TEN_UTILS_PRIVATE_API bool ten_schema_error_check_integrity(
ten_schema_error_t *self);

TEN_UTILS_PRIVATE_API void ten_schema_error_init(ten_schema_error_t *self,
ten_error_t *err);

TEN_UTILS_PRIVATE_API void ten_schema_error_deinit(ten_schema_error_t *self);

TEN_UTILS_PRIVATE_API void ten_schema_error_reset(ten_schema_error_t *self);

TEN_UTILS_PRIVATE_API bool ten_schema_check_integrity(ten_schema_t *self);

TEN_UTILS_PRIVATE_API void ten_schema_init(ten_schema_t *self);
Expand All @@ -99,14 +148,23 @@ TEN_UTILS_API ten_schema_t *ten_schema_create_from_value(ten_value_t *value);

TEN_UTILS_API void ten_schema_destroy(ten_schema_t *self);

TEN_UTILS_PRIVATE_API bool ten_schema_validate_value_with_schema_error(
ten_schema_t *self, ten_value_t *value, ten_schema_error_t *schema_err);

TEN_UTILS_API bool ten_schema_validate_value(ten_schema_t *self,
ten_value_t *value,
ten_error_t *err);

TEN_UTILS_PRIVATE_API bool ten_schema_adjust_value_type_with_schema_error(
ten_schema_t *self, ten_value_t *value, ten_schema_error_t *schema_err);

TEN_UTILS_API bool ten_schema_adjust_value_type(ten_schema_t *self,
ten_value_t *value,
ten_error_t *err);

TEN_UTILS_PRIVATE_API bool ten_schema_is_compatible_with_schema_error(
ten_schema_t *self, ten_schema_t *target, ten_schema_error_t *schema_err);

TEN_UTILS_API bool ten_schema_is_compatible(ten_schema_t *self,
ten_schema_t *target,
ten_error_t *err);
2 changes: 1 addition & 1 deletion core/src/ten_manager/tests/cmd_dev_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ async fn test_cmd_dev_server_graphs_some_property_invalid() {

let root_cause = json["error"]["message"].as_str().unwrap();
assert!(root_cause
.contains("the app uri should be some string other than 'localhost'"));
.contains("Either all nodes should have 'app' declared, or none should, but not a mix of both"));
}

#[actix_rt::test]
Expand Down
54 changes: 28 additions & 26 deletions core/src/ten_utils/schema/keywords/keyword_items.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//
#include "include_internal/ten_utils/schema/keywords/keyword_items.h"

#include "ten_utils/macro/check.h"
#include "include_internal/ten_utils/schema/keywords/keyword.h"
#include "include_internal/ten_utils/schema/schema.h"
#include "include_internal/ten_utils/schema/types/schema_array.h"
Expand Down Expand Up @@ -34,20 +33,19 @@ bool ten_schema_keyword_items_check_integrity(
return true;
}

static bool ten_schema_keyword_items_validate_value(ten_schema_keyword_t *self_,
ten_value_t *value,
ten_error_t *err) {
static bool ten_schema_keyword_items_validate_value(
ten_schema_keyword_t *self_, ten_value_t *value,
ten_schema_error_t *schema_err) {
TEN_ASSERT(self_ && ten_schema_keyword_check_integrity(self_),
"Invalid argument.");
TEN_ASSERT(value && ten_value_check_integrity(value), "Invalid argument.");
TEN_ASSERT(err && ten_error_check_integrity(err), "Invalid argument.");
TEN_ASSERT(schema_err && ten_schema_error_check_integrity(schema_err),
"Invalid argument.");

if (!ten_value_is_array(value)) {
if (err) {
ten_error_set(err, TEN_ERRNO_GENERIC,
"The value should be an array, but is: %s.",
ten_type_to_string(ten_value_get_type(value)));
}
ten_error_set(schema_err->err, TEN_ERRNO_GENERIC,
"the value should be an array, but is: %s",
ten_type_to_string(ten_value_get_type(value)));
return false;
}

Expand All @@ -65,7 +63,9 @@ static bool ten_schema_keyword_items_validate_value(ten_schema_keyword_t *self_,
TEN_ASSERT(value_field && ten_value_check_integrity(value_field),
"Invalid argument.");

if (!ten_schema_validate_value(self->item_schema, value_field, err)) {
if (!ten_schema_validate_value_with_schema_error(self->item_schema,
value_field, schema_err)) {
ten_string_prepend_formatted(&schema_err->path, "[%d]", value_iter.index);
return false;
}
}
Expand All @@ -86,24 +86,23 @@ static void ten_schema_keyword_items_destroy(ten_schema_keyword_t *self_) {
TEN_FREE(self);
}

static bool ten_schema_keyword_items_adjust_value(ten_schema_keyword_t *self_,
ten_value_t *value,
ten_error_t *err) {
static bool ten_schema_keyword_items_adjust_value(
ten_schema_keyword_t *self_, ten_value_t *value,
ten_schema_error_t *schema_err) {
TEN_ASSERT(self_ && ten_schema_keyword_check_integrity(self_),
"Invalid argument.");
TEN_ASSERT(value && ten_value_check_integrity(value), "Invalid argument.");
TEN_ASSERT(err && ten_error_check_integrity(err), "Invalid argument.");
TEN_ASSERT(schema_err && ten_schema_error_check_integrity(schema_err),
"Invalid argument.");

ten_schema_keyword_items_t *self = (ten_schema_keyword_items_t *)self_;
TEN_ASSERT(ten_schema_keyword_items_check_integrity(self),
"Invalid argument.");

if (!ten_value_is_array(value)) {
if (err) {
ten_error_set(err, TEN_ERRNO_GENERIC,
"The value should be an array, but is: %s.",
ten_type_to_string(ten_value_get_type(value)));
}
ten_error_set(schema_err->err, TEN_ERRNO_GENERIC,
"the value should be an array, but is: %s",
ten_type_to_string(ten_value_get_type(value)));
return false;
}

Expand All @@ -117,7 +116,9 @@ static bool ten_schema_keyword_items_adjust_value(ten_schema_keyword_t *self_,
TEN_ASSERT(value_field && ten_value_check_integrity(value_field),
"Invalid argument.");

if (!ten_schema_adjust_value_type(self->item_schema, value_field, err)) {
if (!ten_schema_adjust_value_type_with_schema_error(
self->item_schema, value_field, schema_err)) {
ten_string_prepend_formatted(&schema_err->path, "[%d]", value_iter.index);
return false;
}
}
Expand All @@ -135,9 +136,10 @@ static bool ten_schema_keyword_items_adjust_value(ten_schema_keyword_t *self_,
// their schemas are invalid.
static bool ten_schema_keyword_items_is_compatible(
ten_schema_keyword_t *self_, ten_schema_keyword_t *target_,
ten_error_t *err) {
ten_schema_error_t *schema_err) {
TEN_ASSERT(self_ && target_, "Invalid argument.");
TEN_ASSERT(err && ten_error_check_integrity(err), "Invalid argument.");
TEN_ASSERT(schema_err && ten_schema_error_check_integrity(schema_err),
"Invalid argument.");

ten_schema_keyword_items_t *self = (ten_schema_keyword_items_t *)self_;
TEN_ASSERT(ten_schema_keyword_items_check_integrity(self),
Expand All @@ -147,10 +149,10 @@ static bool ten_schema_keyword_items_is_compatible(
TEN_ASSERT(ten_schema_keyword_items_check_integrity(target),
"Invalid argument.");

bool success =
ten_schema_is_compatible(self->item_schema, target->item_schema, err);
bool success = ten_schema_is_compatible_with_schema_error(
self->item_schema, target->item_schema, schema_err);
if (!success) {
ten_error_prepend_errmsg(err, "items are incompatible: \n\t");
ten_string_prepend_formatted(&schema_err->path, "[]");
}

return success;
Expand Down
65 changes: 38 additions & 27 deletions core/src/ten_utils/schema/keywords/keyword_properties.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
//
#include "include_internal/ten_utils/schema/keywords/keyword_properties.h"

#include "ten_utils/macro/check.h"
#include "include_internal/ten_utils/schema/keywords/keyword.h"
#include "include_internal/ten_utils/schema/schema.h"
#include "include_internal/ten_utils/schema/types/schema_object.h"
Expand All @@ -15,6 +14,7 @@
#include "ten_utils/lib/error.h"
#include "ten_utils/lib/signature.h"
#include "ten_utils/lib/string.h"
#include "ten_utils/macro/check.h"
#include "ten_utils/macro/memory.h"
#include "ten_utils/value/type_operation.h"
#include "ten_utils/value/value.h"
Expand Down Expand Up @@ -70,17 +70,17 @@ ten_schema_t *ten_schema_keyword_properties_peek_property_schema(
}

static bool ten_schema_keyword_properties_validate_value(
ten_schema_keyword_t *self_, ten_value_t *value, ten_error_t *err) {
TEN_ASSERT(self_ && value && err, "Invalid argument.");
ten_schema_keyword_t *self_, ten_value_t *value,
ten_schema_error_t *schema_err) {
TEN_ASSERT(self_ && value && schema_err, "Invalid argument.");
TEN_ASSERT(ten_schema_keyword_check_integrity(self_), "Invalid argument.");
TEN_ASSERT(ten_value_check_integrity(value), "Invalid argument.");
TEN_ASSERT(ten_schema_error_check_integrity(schema_err), "Invalid argument.");

if (!ten_value_is_object(value)) {
if (err) {
ten_error_set(err, TEN_ERRNO_GENERIC,
"The value must be an object, but is: %s.",
ten_type_to_string(ten_value_get_type(value)));
}
ten_error_set(schema_err->err, TEN_ERRNO_GENERIC,
"the value should be an object, but is: %s",
ten_type_to_string(ten_value_get_type(value)));
return false;
}

Expand All @@ -107,7 +107,10 @@ static bool ten_schema_keyword_properties_validate_value(
continue;
}

if (!ten_schema_validate_value(prop_schema, field_value, err)) {
if (!ten_schema_validate_value_with_schema_error(prop_schema, field_value,
schema_err)) {
ten_string_prepend_formatted(&schema_err->path, ".%s",
ten_string_get_raw_str(field_key));
return false;
}
}
Expand All @@ -116,17 +119,17 @@ static bool ten_schema_keyword_properties_validate_value(
}

static bool ten_schema_keyword_properties_adjust_value(
ten_schema_keyword_t *self_, ten_value_t *value, ten_error_t *err) {
TEN_ASSERT(self_ && value && err, "Invalid argument.");
ten_schema_keyword_t *self_, ten_value_t *value,
ten_schema_error_t *schema_err) {
TEN_ASSERT(self_ && value && schema_err, "Invalid argument.");
TEN_ASSERT(ten_schema_keyword_check_integrity(self_), "Invalid argument.");
TEN_ASSERT(ten_value_check_integrity(value), "Invalid argument.");
TEN_ASSERT(ten_schema_error_check_integrity(schema_err), "Invalid argument.");

if (!ten_value_is_object(value)) {
if (err) {
ten_error_set(err, TEN_ERRNO_GENERIC,
"The value should be an object, but is: %s.",
ten_type_to_string(ten_value_get_type(value)));
}
ten_error_set(schema_err->err, TEN_ERRNO_GENERIC,
"the value should be an object, but is: %s",
ten_type_to_string(ten_value_get_type(value)));
return false;
}

Expand All @@ -148,7 +151,10 @@ static bool ten_schema_keyword_properties_adjust_value(
continue;
}

if (!ten_schema_adjust_value_type(prop_schema, field_value, err)) {
if (!ten_schema_adjust_value_type_with_schema_error(
prop_schema, field_value, schema_err)) {
ten_string_prepend_formatted(&schema_err->path, ".%s",
ten_string_get_raw_str(field_key));
return false;
}
}
Expand All @@ -164,9 +170,10 @@ static bool ten_schema_keyword_properties_adjust_value(
// otherwise their schemas are invalid.
static bool ten_schema_keyword_properties_is_compatible(
ten_schema_keyword_t *self_, ten_schema_keyword_t *target_,
ten_error_t *err) {
ten_schema_error_t *schema_err) {
TEN_ASSERT(self_ && target_, "Invalid argument.");
TEN_ASSERT(err && ten_error_check_integrity(err), "Invalid argument.");
TEN_ASSERT(schema_err && ten_schema_error_check_integrity(schema_err),
"Invalid argument.");

ten_schema_keyword_properties_t *self =
(ten_schema_keyword_properties_t *)self_;
Expand All @@ -193,22 +200,26 @@ static bool ten_schema_keyword_properties_is_compatible(
continue;
}

ten_error_t schema_err;
ten_error_init(&schema_err);
if (!ten_schema_is_compatible(property->schema, target_prop_schema,
&schema_err)) {
ten_string_append_formatted(&incompatible_fields, "\n\tproperty [%s], %s",
if (!ten_schema_is_compatible_with_schema_error(
property->schema, target_prop_schema, schema_err)) {
// Ex:
// .a[0]: type is incompatible, ...; .b: ...
const char *separator =
ten_string_is_empty(&incompatible_fields) ? "" : "; ";
ten_string_append_formatted(&incompatible_fields, "%s.%s%s: %s",
separator,
ten_string_get_raw_str(&property->name),
ten_error_errmsg(&schema_err));
ten_string_get_raw_str(&schema_err->path),
ten_error_errmsg(schema_err->err));
}

ten_error_deinit(&schema_err);
ten_schema_error_reset(schema_err);
}

bool success = ten_string_is_empty(&incompatible_fields);

if (!success) {
ten_error_set(err, TEN_ERRNO_GENERIC, "properties are incompatible: %s",
ten_error_set(schema_err->err, TEN_ERRNO_GENERIC, "{ %s }",
ten_string_get_raw_str(&incompatible_fields));
}

Expand Down
Loading
Loading