From cc04a1a8bbcb3ce17e4a0959747fef65a54c4b42 Mon Sep 17 00:00:00 2001 From: Alex Groleau Date: Wed, 16 Apr 2025 01:00:51 -0400 Subject: [PATCH] made errors consistent --- src/lib.rs | 34 ++++++-------- src/tests.rs | 128 ++++++++++++++++++++++++++++++++++----------------- 2 files changed, 98 insertions(+), 64 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 8acf457..1375819 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,11 +80,9 @@ fn validate_json_schema(schema_id: &str, instance: JsonB) -> JsonB { match cache.id_to_index.get(schema_id) { None => JsonB(json!({ "success": false, - "errors": [json!({ - "message": format!("Schema with id '{}' not found in cache", schema_id), - "schema_path": "", - "instance_path": "" - })] + "error": { + "message": format!("Schema with id '{}' not found in cache", schema_id) + } })), Some(sch_index) => { let instance_value: Value = instance.0; @@ -93,7 +91,12 @@ fn validate_json_schema(schema_id: &str, instance: JsonB) -> JsonB { Err(validation_error) => { // Directly use the result of format_validation_error // which now includes the top-level success indicator and flat error list - JsonB(format_validation_error(&validation_error)) + let mut all_errors = Vec::new(); + collect_leaf_errors(&validation_error, &mut all_errors); + JsonB(json!({ + "success": false, + "error": all_errors // Flat list of specific errors + })) } } } @@ -111,28 +114,17 @@ fn collect_leaf_errors(error: &ValidationError, errors_list: &mut Vec) { }; errors_list.push(json!({ - "message": message, - "schema_path": error.schema_url.to_string(), - "instance_path": error.instance_location.to_string(), + "message": message, + "schema_path": error.schema_url.to_string(), + "instance_path": error.instance_location.to_string(), })); } else { for cause in &error.causes { - collect_leaf_errors(cause, errors_list); + collect_leaf_errors(cause, errors_list); } } } -// Formats validation errors into a flat list JSON structure -fn format_validation_error(error: &ValidationError) -> Value { - let mut all_errors = Vec::new(); - collect_leaf_errors(error, &mut all_errors); - - json!({ - "success": false, - "errors": all_errors // Flat list of specific errors - }) -} - #[pg_extern(strict, parallel_safe)] fn json_schema_cached(schema_id: &str) -> bool { let cache = SCHEMA_CACHE.read().unwrap(); diff --git a/src/tests.rs b/src/tests.rs index db22f7c..7e2431e 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -33,38 +33,63 @@ macro_rules! assert_failure_with_json { ($result:expr, $expected_error_count:expr, $expected_first_message_contains:expr, $fmt:literal $(, $($args:tt)*)?) => { let json_result = &$result.0; let success = json_result.get("success").and_then(Value::as_bool); - let errors_opt = json_result.get("errors").and_then(Value::as_array); + let error_val_opt = json_result.get("error"); // Changed key let base_msg = format!($fmt $(, $($args)*)?); if success != Some(false) { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); panic!("Assertion Failed (expected failure, success was not false): {}\nResult JSON:\n{}", base_msg, pretty_json); } - match errors_opt { - Some(errors) => { - if errors.len() != $expected_error_count { - let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (wrong error count): Expected {} errors, got {}. {}\nResult JSON:\n{}", $expected_error_count, errors.len(), base_msg, pretty_json); - } - if $expected_error_count > 0 { - let first_error_message = errors[0].get("message").and_then(Value::as_str); - match first_error_message { - Some(msg) => { - if !msg.contains($expected_first_message_contains) { + match error_val_opt { + Some(error_val) => { + if error_val.is_array() { + let errors_array = error_val.as_array().unwrap(); + if errors_array.len() != $expected_error_count { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (wrong error count): Expected {} errors, got {}. {}\nResult JSON:\n{}", $expected_error_count, errors_array.len(), base_msg, pretty_json); + } + if $expected_error_count > 0 { + let first_error_message = errors_array[0].get("message").and_then(Value::as_str); + match first_error_message { + Some(msg) => { + if !msg.contains($expected_first_message_contains) { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (first error message mismatch): Expected contains '{}', got: '{}'. {}\nResult JSON:\n{}", $expected_first_message_contains, msg, base_msg, pretty_json); + } + } + None => { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (first error message mismatch): Expected contains '{}', got: '{}'. {}\nResult JSON:\n{}", $expected_first_message_contains, msg, base_msg, pretty_json); + panic!("Assertion Failed (first error in array has no 'message' string): {}\nResult JSON:\n{}", base_msg, pretty_json); } } - None => { - let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (first error has no 'message' string): {}\nResult JSON:\n{}", base_msg, pretty_json); - } } + } else if error_val.is_object() { + // Handle single error object case (like 'schema not found') + if $expected_error_count != 1 { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (wrong error count): Expected {} errors, but got a single error object. {}\nResult JSON:\n{}", $expected_error_count, base_msg, pretty_json); + } + let message = error_val.get("message").and_then(Value::as_str); + match message { + Some(msg) => { + if !msg.contains($expected_first_message_contains) { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (error message mismatch): Expected object message contains '{}', got: '{}'. {}\nResult JSON:\n{}", $expected_first_message_contains, msg, base_msg, pretty_json); + } + } + None => { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (error object has no 'message' string): {}\nResult JSON:\n{}", base_msg, pretty_json); + } + } + } else { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed ('error' value is not an array or object): {}\nResult JSON:\n{}", base_msg, pretty_json); } } None => { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (expected 'errors' array, but none found): {}\nResult JSON:\n{}", base_msg, pretty_json); + panic!("Assertion Failed (expected 'error' key, but none found): {}\nResult JSON:\n{}", base_msg, pretty_json); } } }; @@ -78,23 +103,35 @@ macro_rules! assert_failure_with_json { ($result:expr, $expected_error_count:expr, $fmt:literal $(, $($args:tt)*)?) => { let json_result = &$result.0; let success = json_result.get("success").and_then(Value::as_bool); - let errors_opt = json_result.get("errors").and_then(Value::as_array); + let error_val_opt = json_result.get("error"); // Changed key let base_msg = format!($fmt $(, $($args)*)?); if success != Some(false) { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); panic!("Assertion Failed (expected failure, success was not false): {}\nResult JSON:\n{}", base_msg, pretty_json); } - match errors_opt { - Some(errors) => { - if errors.len() != $expected_error_count { - let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (wrong error count): Expected {} errors, got {}. {}\nResult JSON:\n{}", $expected_error_count, errors.len(), base_msg, pretty_json); + match error_val_opt { + Some(error_val) => { + if error_val.is_array() { + let errors_array = error_val.as_array().unwrap(); + if errors_array.len() != $expected_error_count { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (wrong error count): Expected {} errors, got {}. {}\nResult JSON:\n{}", $expected_error_count, errors_array.len(), base_msg, pretty_json); + } + } else if error_val.is_object() { + if $expected_error_count != 1 { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (wrong error count): Expected {} errors, but got a single error object. {}\nResult JSON:\n{}", $expected_error_count, base_msg, pretty_json); + } + // Count check passes if expected is 1 and got object + } else { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed ('error' value is not an array or object): {}\nResult JSON:\n{}", base_msg, pretty_json); } } None => { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (expected 'errors' array, but none found): {}\nResult JSON:\n{}", base_msg, pretty_json); + panic!("Assertion Failed (expected 'error' key, but none found): {}\nResult JSON:\n{}", base_msg, pretty_json); } } }; @@ -108,23 +145,31 @@ macro_rules! assert_failure_with_json { ($result:expr, $fmt:literal $(, $($args:tt)*)?) => { let json_result = &$result.0; let success = json_result.get("success").and_then(Value::as_bool); - let errors_opt = json_result.get("errors").and_then(Value::as_array); + let error_val_opt = json_result.get("error"); // Changed key let base_msg = format!($fmt $(, $($args)*)?); if success != Some(false) { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); panic!("Assertion Failed (expected failure, success was not false): {}\nResult JSON:\n{}", base_msg, pretty_json); } - match errors_opt { - Some(errors) => { - if errors.is_empty() { - let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (expected errors, but errors array is empty): {}\nResult JSON:\n{}", base_msg, pretty_json); + match error_val_opt { + Some(error_val) => { + if error_val.is_object() { + // OK: single error object is a failure + } else if error_val.is_array() { + if error_val.as_array().unwrap().is_empty() { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed (expected errors, but 'error' array is empty): {}\nResult JSON:\n{}", base_msg, pretty_json); + } + // OK: non-empty error array is a failure + } else { + let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); + panic!("Assertion Failed ('error' value is not an array or object): {}\nResult JSON:\n{}", base_msg, pretty_json); } } None => { let pretty_json = serde_json::to_string_pretty(&json_result).unwrap_or_else(|_| format!("(Failed to pretty-print JSON: {:?})", json_result)); - panic!("Assertion Failed (expected 'errors' array, but none found): {}\nResult JSON:\n{}", base_msg, pretty_json); + panic!("Assertion Failed (expected 'error' key, but none found): {}\nResult JSON:\n{}", base_msg, pretty_json); } } }; @@ -164,14 +209,14 @@ fn test_cache_and_validate_json_schema() { // Invalid type let invalid_result_type = validate_json_schema(schema_id, jsonb(invalid_instance_type)); assert_failure_with_json!(invalid_result_type, 1, "must be >=0", "Validation with invalid type should fail."); - let errors_type = invalid_result_type.0["errors"].as_array().unwrap(); + let errors_type = invalid_result_type.0["error"].as_array().unwrap(); // Check 'error', expect array assert_eq!(errors_type[0]["instance_path"], "/age"); assert_eq!(errors_type[0]["schema_path"], "urn:my_schema#/properties/age"); // Missing field let invalid_result_missing = validate_json_schema(schema_id, jsonb(invalid_instance_missing)); assert_failure_with_json!(invalid_result_missing, 1, "missing properties 'age'", "Validation with missing field should fail."); - let errors_missing = invalid_result_missing.0["errors"].as_array().unwrap(); + let errors_missing = invalid_result_missing.0["error"].as_array().unwrap(); // Check 'error', expect array assert_eq!(errors_missing[0]["instance_path"], ""); assert_eq!(errors_missing[0]["schema_path"], "urn:my_schema#"); @@ -179,9 +224,10 @@ fn test_cache_and_validate_json_schema() { let non_existent_id = "non_existent_schema"; let invalid_schema_result = validate_json_schema(non_existent_id, jsonb(json!({}))); assert_failure_with_json!(invalid_schema_result, 1, "Schema with id 'non_existent_schema' not found", "Validation with non-existent schema should fail."); - let errors_notfound = invalid_schema_result.0["errors"].as_array().unwrap(); - assert_eq!(errors_notfound[0]["schema_path"], ""); // Schema path is empty for this error type - assert_eq!(errors_notfound[0]["instance_path"], ""); // Instance path is empty + // Check 'error' is an object for 'schema not found' + let error_notfound_obj = invalid_schema_result.0["error"].as_object().expect("'error' should be an object for schema not found"); + assert!(error_notfound_obj.contains_key("message")); // Check message exists + // Removed checks for schema_path/instance_path as they aren't added in lib.rs for this case } #[pg_test] @@ -189,7 +235,7 @@ fn test_validate_json_schema_not_cached() { clear_json_schemas(); // Call clear directly let instance = json!({ "foo": "bar" }); let result = validate_json_schema("non_existent_schema", jsonb(instance)); - // Use the updated macro + // Use the updated macro, expecting count 1 and specific message (handles object case) assert_failure_with_json!(result, 1, "Schema with id 'non_existent_schema' not found", "Validation with non-existent schema should fail."); } @@ -292,7 +338,6 @@ fn test_validate_json_schema_oneof_validation_errors() { // Expect 2 leaf errors: one for maxLength (branch 0), one for missing prop (branch 1) // Check the first error message reported by boon (maxLength). assert_failure_with_json!(result_invalid_string, 2, "length must be <=5", "Validation with invalid string length should have 2 leaf errors"); - let _errors_string = result_invalid_string.0["errors"].as_array().unwrap(); // Prefix with _ // --- Test case 2: Fails number minimum (in branch 1) AND missing string_prop (in branch 0) --- let invalid_number_instance = json!({ "number_prop": 5 }); @@ -300,7 +345,6 @@ fn test_validate_json_schema_oneof_validation_errors() { // Expect 2 leaf errors: one for minimum (branch 1), one for missing prop (branch 0) // Check the first error message reported by boon (missing prop). assert_failure_with_json!(result_invalid_number, 2, "missing properties 'string_prop'", "Validation with invalid number should have 2 leaf errors"); - let _errors_number = result_invalid_number.0["errors"].as_array().unwrap(); // Prefix with _ // --- Test case 3: Fails type check (not object) for both branches --- // Input: boolean, expected object for both branches @@ -309,7 +353,6 @@ fn test_validate_json_schema_oneof_validation_errors() { // Expect 2 leaf errors, one "Type" error for each branch // Check the first error reported by boon (want object). assert_failure_with_json!(result_invalid_bool, 2, "want object", "Validation with invalid bool should have 2 leaf errors"); - let _errors_bool = result_invalid_bool.0["errors"].as_array().unwrap(); // Prefix with _ // --- Test case 4: Fails missing required for both branches --- // Input: empty object, expected string_prop (branch 0) OR number_prop (branch 1) @@ -318,7 +361,6 @@ fn test_validate_json_schema_oneof_validation_errors() { // Expect 2 leaf errors: one required error for branch 0, one required error for branch 1 // Check the first error reported by boon (missing string_prop). assert_failure_with_json!(result_empty_obj, 2, "missing properties 'string_prop'", "Validation with empty object should have 2 leaf errors"); - let _errors_empty = result_empty_obj.0["errors"].as_array().unwrap(); // Prefix with _ } #[pg_test] @@ -338,7 +380,7 @@ fn test_clear_json_schemas() { let instance = json!("test"); let validate_result = validate_json_schema(schema_id, jsonb(instance)); - // Use the updated macro + // Use the updated macro, expecting count 1 and specific message (handles object case) assert_failure_with_json!(validate_result, 1, "Schema with id 'schema_to_clear' not found", "Validation should fail after clearing schemas."); }