From b6db0e84a45d2c02e8f0ab7d05ff25f770cd0980 Mon Sep 17 00:00:00 2001 From: Kriskras99 Date: Sun, 6 Sep 2026 16:05:56 +0200 Subject: [PATCH 1/4] fix!: Don't log potentially sensitive value in `Value::validate` This is done by removing the logging completely and returing an error instead of a boolean. The code will also no longer panic when it fails to resolve the schema. This is a breaking change, users can fix their code by adding `.is_ok()` to get the previous behaviour. Reported-by: CodeQL --- avro/src/error.rs | 6 + avro/src/schema/mod.rs | 14 +- avro/src/serde/with.rs | 2 +- avro/src/types.rs | 602 +++++++++++++++++++++------------------- avro/tests/avro-3786.rs | 30 +- avro/tests/avro-3787.rs | 10 +- avro/tests/io.rs | 5 +- avro/tests/schema.rs | 5 +- 8 files changed, 333 insertions(+), 341 deletions(-) diff --git a/avro/src/error.rs b/avro/src/error.rs index 58f9145f..022bab57 100644 --- a/avro/src/error.rs +++ b/avro/src/error.rs @@ -729,6 +729,12 @@ pub enum Details { position: usize, total_elements: usize, }, + + #[error("The value is invalid for the given schema: {0}")] + InvalidValueForSchema(String), + + #[error("The value is invalid for all the given schemas: {0:?}")] + InvalidValueForAllSchemas(Vec), } #[derive(thiserror::Error, PartialEq)] diff --git a/avro/src/schema/mod.rs b/avro/src/schema/mod.rs index b503f12c..3aaede7f 100644 --- a/avro/src/schema/mod.rs +++ b/avro/src/schema/mod.rs @@ -2975,7 +2975,7 @@ mod tests { }; let avro_value = crate::to_value(foo)?; - assert!(avro_value.validate(&schema)); + avro_value.validate(&schema)?; let mut writer = crate::Writer::new(&schema, Vec::new())?; @@ -3065,10 +3065,7 @@ mod tests { bar_use: Bar::Bar1, }; let avro_value = crate::to_value(foo)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -3651,10 +3648,7 @@ mod tests { // Serialize using the writer schema. let writer_schema = Schema::parse(writer_schema)?; let avro_value = crate::to_value(s)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -3668,7 +3662,7 @@ mod tests { .reader_schema(&reader_schema) .build()? .read_value(&mut x)?; - assert!(deser_value.validate(&reader_schema)); + deser_value.validate(&reader_schema)?; // Verify that we can read a field from the record. let d: MyRecordReader = crate::from_value(&deser_value)?; diff --git a/avro/src/serde/with.rs b/avro/src/serde/with.rs index 13f199f2..6059f667 100644 --- a/avro/src/serde/with.rs +++ b/avro/src/serde/with.rs @@ -881,7 +881,7 @@ mod tests { }"#, ) .unwrap(); - assert!(value.validate(&schema)); + value.validate(&schema).unwrap(); } #[test] diff --git a/avro/src/types.rs b/avro/src/types.rs index 70759a09..da3b4973 100644 --- a/avro/src/types.rs +++ b/avro/src/types.rs @@ -377,11 +377,12 @@ impl Value { /// /// See the [Avro specification](https://avro.apache.org/docs/++version++/specification) /// for the full set of rules of schema validation. - /// - /// # Panics - /// Will panic if the schema contain unresolved references or duplicate named types. - pub fn validate(&self, schema: &Schema) -> bool { - self.validate_schemata(&[schema]) + pub fn validate(&self, schema: &Schema) -> AvroResult<()> { + let rs = ResolvedSchema::new(schema)?; + match self.validate_internal(schema, rs.get_names(), None) { + Some(reason) => Err(Details::InvalidValueForSchema(reason).into()), + None => Ok(()), + } } /// Validate the value against the given schemata. @@ -392,28 +393,25 @@ impl Value { /// /// See the [Avro specification](https://avro.apache.org/docs/++version++/specification) /// for the full set of rules of schema validation. - /// - /// # Panics - /// Will panic if the schemata contain unresolved references or duplicate schemas. - pub fn validate_schemata(&self, schemata: &[&Schema]) -> bool { - let rs = ResolvedSchema::try_from(schemata.to_vec()) - .expect("Schemata didn't successfully resolve"); + pub fn validate_schemata(&self, schemata: &[&Schema]) -> AvroResult<()> { + let rs = ResolvedSchema::try_from(schemata.to_vec())?; let schemata_len = schemata.len(); - schemata.iter().any( - |schema| match self.validate_internal(schema, rs.get_names(), None) { + let mut errors = Vec::with_capacity(schemata_len); + let found = schemata.iter().any(|schema| { + match self.validate_internal(schema, rs.get_names(), None) { Some(reason) => { - let log_message = - format!("Invalid value: {self:?} for schema: {schema:?}. Reason: {reason}"); - if schemata_len == 1 { - error!("{log_message}"); - } else { - debug!("{log_message}"); - }; + errors.push(reason); false } None => true, - }, - ) + } + }); + + if found { + Ok(()) + } else { + Err(Details::InvalidValueForAllSchemas(errors).into()) + } } /// Validate the value against the given schema using `names` to resolve any references. @@ -424,13 +422,10 @@ impl Value { &self, schema: &Schema, names: &HashMap, - ) -> bool { + ) -> AvroResult<()> { match self.validate_internal(schema, names, None) { - Some(reason) => { - error!("Invalid value: {self:?} for schema: {schema:?}. Reason: {reason}"); - false - } - None => true, + Some(reason) => Err(Details::InvalidValueForSchema(reason).into()), + None => Ok(()), } } @@ -1369,10 +1364,7 @@ mod tests { error::Details, to_value, }; - use apache_avro_test_helper::{ - TestResult, - logger::{assert_logged, assert_not_logged}, - }; + use apache_avro_test_helper::TestResult; use num_bigint::BigInt; use pretty_assertions::assert_eq; use serde_json::json; @@ -1423,7 +1415,7 @@ mod tests { ]), )]); - assert!(value.validate(&schema)); + value.validate(&schema)?; Ok(()) } @@ -1593,26 +1585,22 @@ mod tests { attributes: Default::default(), }); - assert!(Value::Fixed(4, vec![0, 0, 0, 0]).validate(&schema)); + Value::Fixed(4, vec![0, 0, 0, 0]).validate(&schema)?; let value = Value::Fixed(5, vec![0, 0, 0, 0, 0]); - assert!(!value.validate(&schema)); - assert_logged( - format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, schema, "The value's size (5) is different than the schema's size (4)" - ) - .as_str(), + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: The value's size (5) is different than the schema's size (4)" + ); + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: The value's size (5) is different than the schema's size (4)" ); - assert!(Value::Bytes(vec![0, 0, 0, 0]).validate(&schema)); + Value::Bytes(vec![0, 0, 0, 0]).validate(&schema)?; let value = Value::Bytes(vec![0, 0, 0, 0, 0]); - assert!(!value.validate(&schema)); - assert_logged( - format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, schema, "The bytes' length (5) is different than the schema's size (4)" - ) - .as_str(), + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: The bytes' length (5) is different than the schema's size (4)" ); Ok(()) @@ -1634,37 +1622,25 @@ mod tests { attributes: Default::default(), }); - assert!(Value::Enum(0, "spades".to_string()).validate(&schema)); - assert!(Value::String("spades".to_string()).validate(&schema)); + Value::Enum(0, "spades".to_string()).validate(&schema)?; + Value::String("spades".to_string()).validate(&schema)?; let value = Value::Enum(1, "spades".to_string()); - assert!(!value.validate(&schema)); - assert_logged( - format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, schema, "Symbol 'spades' is not at position '1'" - ) - .as_str(), + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: Symbol 'spades' is not at position '1'" ); let value = Value::Enum(1000, "spades".to_string()); - assert!(!value.validate(&schema)); - assert_logged( - format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, schema, "No symbol at position '1000'" - ) - .as_str(), + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: No symbol at position '1000'" ); let value = Value::String("lorem".to_string()); - assert!(!value.validate(&schema)); - assert_logged( - format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, schema, "'lorem' is not a member of the possible symbols" - ) - .as_str(), + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: 'lorem' is not a member of the possible symbols" ); let other_schema = Schema::Enum(EnumSchema { @@ -1682,13 +1658,9 @@ mod tests { }); let value = Value::Enum(0, "spades".to_string()); - assert!(!value.validate(&other_schema)); - assert_logged( - format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, other_schema, "Symbol 'spades' is not at position '0'" - ) - .as_str(), + assert_eq!( + value.validate(&other_schema).unwrap_err().to_string(), + "The value is invalid for the given schema: Symbol 'spades' is not at position '0'" ); Ok(()) @@ -1741,48 +1713,43 @@ mod tests { attributes: Default::default(), }); - assert!( - Value::Record(vec![ - ("a".to_string(), Value::Long(42i64)), - ("b".to_string(), Value::String("foo".to_string())), - ]) - .validate(&schema) - ); + Value::Record(vec![ + ("a".to_string(), Value::Long(42i64)), + ("b".to_string(), Value::String("foo".to_string())), + ]) + .validate(&schema)?; let value = Value::Record(vec![ ("b".to_string(), Value::String("foo".to_string())), ("a".to_string(), Value::Long(42i64)), ]); - assert!(value.validate(&schema)); + value.validate(&schema)?; let value = Value::Record(vec![ ("a".to_string(), Value::Boolean(false)), ("b".to_string(), Value::String("foo".to_string())), ]); - assert!(!value.validate(&schema)); - assert_logged( - r#"Invalid value: Record([("a", Boolean(false)), ("b", String("foo"))]) for schema: Record(RecordSchema { name: Name { name: "some_record", .. }, fields: [RecordField { name: "a", schema: Long, .. }, RecordField { name: "b", schema: String, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), .. }], .. }). Reason: Unsupported value-schema combination! Value: Boolean(false), schema: Long"#, + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Boolean(false), schema: Long" ); let value = Value::Record(vec![ ("a".to_string(), Value::Long(42i64)), ("c".to_string(), Value::String("foo".to_string())), ]); - assert!(!value.validate(&schema)); - assert_logged( - r#"Invalid value: Record([("a", Long(42)), ("c", String("foo"))]) for schema: Record(RecordSchema { name: Name { name: "some_record", .. }, fields: [RecordField { name: "a", schema: Long, .. }, RecordField { name: "b", schema: String, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), .. }], .. }). Reason: Could not find matching type in union"#, - ); - assert_not_logged( - r#"Invalid value: String("foo") for schema: Int. Reason: Unsupported value-schema combination"#, + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: Could not find matching type in union", ); let value = Value::Record(vec![ ("a".to_string(), Value::Long(42i64)), ("d".to_string(), Value::String("foo".to_string())), ]); - assert!(!value.validate(&schema)); - assert_logged( - r#"Invalid value: Record([("a", Long(42)), ("d", String("foo"))]) for schema: Record(RecordSchema { name: Name { name: "some_record", .. }, fields: [RecordField { name: "a", schema: Long, .. }, RecordField { name: "b", schema: String, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), .. }], .. }). Reason: There is no schema field for field 'd'"#, + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: There is no schema field for field 'd'", ); let value = Value::Record(vec![ @@ -1791,63 +1758,57 @@ mod tests { ("c".to_string(), Value::Null), ("d".to_string(), Value::Null), ]); - assert!(!value.validate(&schema)); - assert_logged( - r#"Invalid value: Record([("a", Long(42)), ("b", String("foo")), ("c", Null), ("d", Null)]) for schema: Record(RecordSchema { name: Name { name: "some_record", .. }, fields: [RecordField { name: "a", schema: Long, .. }, RecordField { name: "b", schema: String, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), .. }], .. }). Reason: The value's records length (4) is greater than the schema's (3 fields)"#, + assert_eq!( + value.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: The value's records length (4) is greater than the schema's (3 fields)", ); - assert!( - Value::Map( - vec![ - ("a".to_string(), Value::Long(42i64)), - ("b".to_string(), Value::String("foo".to_string())), - ] - .into_iter() - .collect() - ) - .validate(&schema) - ); + Value::Map( + vec![ + ("a".to_string(), Value::Long(42i64)), + ("b".to_string(), Value::String("foo".to_string())), + ] + .into_iter() + .collect(), + ) + .validate(&schema)?; - assert!( - !Value::Map( + assert_eq!( + Value::Map( vec![("d".to_string(), Value::Long(123_i64)),] .into_iter() .collect() ) .validate(&schema) - ); - assert_logged( - r#"Invalid value: Map({"d": Long(123)}) for schema: Record(RecordSchema { name: Name { name: "some_record", .. }, fields: [RecordField { name: "a", schema: Long, .. }, RecordField { name: "b", schema: String, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), .. }], .. }). Reason: Field with name '"a"' is not a member of the map items + .unwrap_err() + .to_string(), + r#"The value is invalid for the given schema: Field with name '"a"' is not a member of the map items Field with name '"b"' is not a member of the map items"#, ); let union_schema = Schema::Union(UnionSchema::new(vec![Schema::Null, schema])?); - assert!( - Value::Union( - 1, - Box::new(Value::Record(vec![ + Value::Union( + 1, + Box::new(Value::Record(vec![ + ("a".to_string(), Value::Long(42i64)), + ("b".to_string(), Value::String("foo".to_string())), + ])), + ) + .validate(&union_schema)?; + + Value::Union( + 1, + Box::new(Value::Map( + vec![ ("a".to_string(), Value::Long(42i64)), ("b".to_string(), Value::String("foo".to_string())), - ])) - ) - .validate(&union_schema) - ); - - assert!( - Value::Union( - 1, - Box::new(Value::Map( - vec![ - ("a".to_string(), Value::Long(42i64)), - ("b".to_string(), Value::String("foo".to_string())), - ] - .into_iter() - .collect() - )) - ) - .validate(&union_schema) - ); + ] + .into_iter() + .collect(), + )), + ) + .validate(&union_schema)?; Ok(()) } @@ -1888,7 +1849,10 @@ Field with name '"b"' is not a member of the map items"#, #[test] fn resolve_bytes_failure() { let value = Value::Array(vec![Value::Int(2000), Value::Int(-42)]); - assert!(value.resolve(&Schema::Bytes).is_err()); + assert_eq!( + value.resolve(&Schema::Bytes).unwrap_err().to_string(), + "Unable to convert to u8, got Int(2000)" + ); } #[test] @@ -1899,7 +1863,10 @@ Field with name '"b"' is not a member of the map items"#, scale: 4, inner: InnerDecimalSchema::Bytes, }))?; - assert!(value.resolve(&Schema::String).is_err()); + assert_eq!( + value.resolve(&Schema::String).unwrap_err().to_string(), + "Expected Value::String, Value::Bytes or Value::Fixed, got: Decimal(Decimal { value: 4328719365, len: 5 })" + ); Ok(()) } @@ -1929,14 +1896,16 @@ Field with name '"b"' is not a member of the map items"#, ); let value = Value::String("\u{0100}".to_string()); - assert!( + assert_eq!( value .resolve(&Schema::Decimal(DecimalSchema { precision: 10, scale: 4, inner: InnerDecimalSchema::Bytes, })) - .is_err() + .unwrap_err() + .to_string(), + r#"Expected Value::Decimal, Value::Bytes, Value::Fixed or Value::String, got: String("Ā")"# ); Ok(()) @@ -1970,132 +1939,202 @@ Field with name '"b"' is not a member of the map items"#, #[test] fn resolve_decimal_invalid_scale() { let value = Value::Decimal(Decimal::from(vec![1, 2])); - assert!( + assert_eq!( value .resolve(&Schema::Decimal(DecimalSchema { precision: 2, scale: 3, inner: InnerDecimalSchema::Bytes, })) - .is_err() + .unwrap_err() + .to_string(), + "Scale 3 is greater than precision 2" ); } #[test] fn resolve_decimal_invalid_precision_for_length() { let value = Value::Decimal(Decimal::from((1u8..=8u8).rev().collect::>())); - assert!( - value - .resolve(&Schema::Decimal(DecimalSchema { - precision: 1, - scale: 0, - inner: InnerDecimalSchema::Bytes, - })) - .is_ok() - ); + value + .resolve(&Schema::Decimal(DecimalSchema { + precision: 1, + scale: 0, + inner: InnerDecimalSchema::Bytes, + })) + .unwrap(); } #[test] fn resolve_decimal_fixed() { let value = Value::Decimal(Decimal::from(vec![1, 2, 3, 4, 5])); - assert!( - value - .clone() - .resolve(&Schema::Decimal(DecimalSchema { - precision: 10, - scale: 1, - inner: InnerDecimalSchema::Fixed(FixedSchema { - name: Name::new("decimal").unwrap(), - aliases: None, - size: 20, - doc: None, - attributes: Default::default(), - }) - })) - .is_ok() + value + .clone() + .resolve(&Schema::Decimal(DecimalSchema { + precision: 10, + scale: 1, + inner: InnerDecimalSchema::Fixed(FixedSchema { + name: Name::new("decimal").unwrap(), + aliases: None, + size: 20, + doc: None, + attributes: Default::default(), + }), + })) + .unwrap(); + assert_eq!( + value.resolve(&Schema::String).unwrap_err().to_string(), + "Expected Value::String, Value::Bytes or Value::Fixed, got: Decimal(Decimal { value: 4328719365, len: 5 })" ); - assert!(value.resolve(&Schema::String).is_err()); } #[test] fn resolve_date() { let value = Value::Date(2345); - assert!(value.clone().resolve(&Schema::Date).is_ok()); - assert!(value.resolve(&Schema::String).is_err()); + value.clone().resolve(&Schema::Date).unwrap(); + assert_eq!( + value.resolve(&Schema::String).unwrap_err().to_string(), + "Expected Value::String, Value::Bytes or Value::Fixed, got: Date(2345)" + ); } #[test] fn resolve_time_millis() { let value = Value::TimeMillis(10); - assert!(value.clone().resolve(&Schema::TimeMillis).is_ok()); - assert!(value.resolve(&Schema::TimeMicros).is_err()); + value.clone().resolve(&Schema::TimeMillis).unwrap(); + assert_eq!( + value.resolve(&Schema::TimeMicros).unwrap_err().to_string(), + "Expected Value::TimeMicros, Value::Long or Value::Int, got: TimeMillis(10)" + ); } #[test] fn resolve_time_micros() { let value = Value::TimeMicros(10); - assert!(value.clone().resolve(&Schema::TimeMicros).is_ok()); - assert!(value.resolve(&Schema::TimeMillis).is_err()); + value.clone().resolve(&Schema::TimeMicros).unwrap(); + assert_eq!( + value.resolve(&Schema::TimeMillis).unwrap_err().to_string(), + "Expected Value::TimeMillis or Value::Int, got: TimeMicros(10)" + ); } #[test] fn resolve_timestamp_millis() { let value = Value::TimestampMillis(10); - assert!(value.clone().resolve(&Schema::TimestampMillis).is_ok()); - assert!(value.resolve(&Schema::Float).is_err()); + value.clone().resolve(&Schema::TimestampMillis).unwrap(); + assert_eq!( + value.resolve(&Schema::Float).unwrap_err().to_string(), + r#"Expected Value::Float, Value::Double, Value::Int, Value::Long or Value::String ("NaN", "INF", "Infinity", "-INF" or "-Infinity"), got: TimestampMillis(10)"# + ); let value = Value::Float(10.0f32); - assert!(value.resolve(&Schema::TimestampMillis).is_err()); + assert_eq!( + value + .resolve(&Schema::TimestampMillis) + .unwrap_err() + .to_string(), + "Expected Value::TimestampMillis, Value::Long or Value::Int, got: Float(10.0)" + ); } #[test] fn resolve_timestamp_micros() { let value = Value::TimestampMicros(10); - assert!(value.clone().resolve(&Schema::TimestampMicros).is_ok()); - assert!(value.resolve(&Schema::Int).is_err()); + value.clone().resolve(&Schema::TimestampMicros).unwrap(); + assert_eq!( + value.resolve(&Schema::Int).unwrap_err().to_string(), + "Expected Value::Int, got: TimestampMicros(10)" + ); let value = Value::Double(10.0); - assert!(value.resolve(&Schema::TimestampMicros).is_err()); + assert_eq!( + value + .resolve(&Schema::TimestampMicros) + .unwrap_err() + .to_string(), + "Expected Value::TimestampMicros, Value::Long or Value::Int, got: Double(10.0)" + ); } #[test] fn test_avro_3914_resolve_timestamp_nanos() { let value = Value::TimestampNanos(10); - assert!(value.clone().resolve(&Schema::TimestampNanos).is_ok()); - assert!(value.resolve(&Schema::Int).is_err()); + value.clone().resolve(&Schema::TimestampNanos).unwrap(); + assert_eq!( + value.resolve(&Schema::Int).unwrap_err().to_string(), + "Expected Value::Int, got: TimestampNanos(10)" + ); let value = Value::Double(10.0); - assert!(value.resolve(&Schema::TimestampNanos).is_err()); + assert_eq!( + value + .resolve(&Schema::TimestampNanos) + .unwrap_err() + .to_string(), + "Expected Value::TimestampNanos, Value::Long or Value::Int, got: Double(10.0)" + ); } #[test] fn test_avro_3853_resolve_timestamp_millis() { let value = Value::LocalTimestampMillis(10); - assert!(value.clone().resolve(&Schema::LocalTimestampMillis).is_ok()); - assert!(value.resolve(&Schema::Float).is_err()); + value + .clone() + .resolve(&Schema::LocalTimestampMillis) + .unwrap(); + assert_eq!( + value.resolve(&Schema::Float).unwrap_err().to_string(), + r#"Expected Value::Float, Value::Double, Value::Int, Value::Long or Value::String ("NaN", "INF", "Infinity", "-INF" or "-Infinity"), got: LocalTimestampMillis(10)"# + ); let value = Value::Float(10.0f32); - assert!(value.resolve(&Schema::LocalTimestampMillis).is_err()); + assert_eq!( + value + .resolve(&Schema::LocalTimestampMillis) + .unwrap_err() + .to_string(), + "Expected Value::LocalTimestampMillis, Value::Long or Value::Int, got: Float(10.0)" + ); } #[test] fn test_avro_3853_resolve_timestamp_micros() { let value = Value::LocalTimestampMicros(10); - assert!(value.clone().resolve(&Schema::LocalTimestampMicros).is_ok()); - assert!(value.resolve(&Schema::Int).is_err()); + value + .clone() + .resolve(&Schema::LocalTimestampMicros) + .unwrap(); + assert_eq!( + value.resolve(&Schema::Int).unwrap_err().to_string(), + "Expected Value::Int, got: LocalTimestampMicros(10)" + ); let value = Value::Double(10.0); - assert!(value.resolve(&Schema::LocalTimestampMicros).is_err()); + assert_eq!( + value + .resolve(&Schema::LocalTimestampMicros) + .unwrap_err() + .to_string(), + "Expected Value::LocalTimestampMicros, Value::Long or Value::Int, got: Double(10.0)" + ); } #[test] fn test_avro_3916_resolve_timestamp_nanos() { let value = Value::LocalTimestampNanos(10); - assert!(value.clone().resolve(&Schema::LocalTimestampNanos).is_ok()); - assert!(value.resolve(&Schema::Int).is_err()); + value.clone().resolve(&Schema::LocalTimestampNanos).unwrap(); + assert_eq!( + value.resolve(&Schema::Int).unwrap_err().to_string(), + "Expected Value::Int, got: LocalTimestampNanos(10)" + ); let value = Value::Double(10.0); - assert!(value.resolve(&Schema::LocalTimestampNanos).is_err()); + assert_eq!( + value + .resolve(&Schema::LocalTimestampNanos) + .unwrap_err() + .to_string(), + "Expected Value::LocalTimestampNanos, Value::Long or Value::Int, got: Double(10.0)" + ); } #[test] @@ -2105,20 +2144,24 @@ Field with name '"b"' is not a member of the map items"#, Days::new(5), Millis::new(3000), )); - assert!( + value + .clone() + .resolve(&Schema::Duration(FixedSchema { + name: Name::try_from("TestName").expect("Name is valid"), + aliases: None, + doc: None, + size: 12, + attributes: BTreeMap::new(), + })) + .unwrap(); + assert_eq!( value - .clone() - .resolve(&Schema::Duration(FixedSchema { - name: Name::try_from("TestName").expect("Name is valid"), - aliases: None, - doc: None, - size: 12, - attributes: BTreeMap::new() - })) - .is_ok() + .resolve(&Schema::TimestampMicros) + .unwrap_err() + .to_string(), + "Expected Value::TimestampMicros, Value::Long or Value::Int, got: Duration(Duration { months: Months(10), days: Days(5), millis: Millis(3000) })" ); - assert!(value.resolve(&Schema::TimestampMicros).is_err()); - assert!( + assert_eq!( Value::Long(1i64) .resolve(&Schema::Duration(FixedSchema { name: Name::try_from("TestName").expect("Name is valid"), @@ -2127,38 +2170,33 @@ Field with name '"b"' is not a member of the map items"#, size: 12, attributes: BTreeMap::new() })) - .is_err() + .unwrap_err() + .to_string(), + "Expected Value::Duration or Value::Fixed(12), got: Long(1)" ); } #[test] fn resolve_uuid() -> TestResult { let value = Value::Uuid(Uuid::parse_str("1481531d-ccc9-46d9-a56f-5b67459c0537")?); - assert!( - value - .clone() - .resolve(&Schema::Uuid(UuidSchema::String)) - .is_ok() - ); - assert!( - value - .clone() - .resolve(&Schema::Uuid(UuidSchema::Bytes)) - .is_ok() - ); - assert!( + value.clone().resolve(&Schema::Uuid(UuidSchema::String))?; + value.clone().resolve(&Schema::Uuid(UuidSchema::Bytes))?; + value + .clone() + .resolve(&Schema::Uuid(UuidSchema::Fixed(FixedSchema { + name: Name::new("some_name")?, + aliases: None, + doc: None, + size: 16, + attributes: Default::default(), + })))?; + assert_eq!( value - .clone() - .resolve(&Schema::Uuid(UuidSchema::Fixed(FixedSchema { - name: Name::new("some_name")?, - aliases: None, - doc: None, - size: 16, - attributes: Default::default(), - }))) - .is_ok() + .resolve(&Schema::TimestampMicros) + .unwrap_err() + .to_string(), + "Expected Value::TimestampMicros, Value::Long or Value::Int, got: Uuid(1481531d-ccc9-46d9-a56f-5b67459c0537)" ); - assert!(value.resolve(&Schema::TimestampMicros).is_err()); Ok(()) } @@ -2166,7 +2204,7 @@ Field with name '"b"' is not a member of the map items"#, #[test] fn avro_3678_resolve_float_to_double() { let value = Value::Float(2345.1); - assert!(value.resolve(&Schema::Double).is_ok()); + value.resolve(&Schema::Double).unwrap(); } #[test] @@ -2209,13 +2247,16 @@ Field with name '"b"' is not a member of the map items"#, "event".to_string(), Value::Record(vec![("amount".to_string(), Value::Int(200))]), )]); - assert!(value.resolve(&schema).is_ok()); + value.resolve(&schema)?; let value = Value::Record(vec![( "event".to_string(), Value::Record(vec![("size".to_string(), Value::Int(1))]), )]); - assert!(value.resolve(&schema).is_err()); + assert_eq!( + value.resolve(&schema).unwrap_err().to_string(), + r#"Could not find matching type in UnionSchema { schemas: [Null, Record(RecordSchema { name: Name { name: "event", .. }, fields: [RecordField { name: "amount", schema: Int, .. }, RecordField { name: "size", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), .. }], .. })] } for Record([("size", Int(1))])"# + ); Ok(()) } @@ -2974,14 +3015,14 @@ Field with name '"b"' is not a member of the map items"#, ("b".into(), inner_value_wrong2), ]); - assert!( - !outer1.validate(&schema), - "field b record is invalid against the schema" - ); // this should pass, but doesn't - assert!( - !outer2.validate(&schema), - "field b record is invalid against the schema" - ); // this should pass, but doesn't + assert_eq!( + outer1.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Null, schema: Int", + ); + assert_eq!( + outer2.validate(&schema).unwrap_err().to_string(), + "The value is invalid for the given schema: There is no schema field for field 'a'" + ); Ok(()) } @@ -3056,17 +3097,17 @@ Field with name '"b"' is not a member of the map items"#, let test_outer2: Value = to_value(test_outer2)?; let test_outer3: Value = to_value(test_outer3)?; - assert!( - !test_outer1.validate(&schema), - "field b record is invalid against the schema" + assert_eq!( + test_outer1.validate(&schema).unwrap_err().to_string(), + r#"The value is invalid for the given schema: Unsupported value-schema combination! Value: String("testing"), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })"# ); - assert!( - !test_outer2.validate(&schema), - "field b record is invalid against the schema" + assert_eq!( + test_outer2.validate(&schema).unwrap_err().to_string(), + r#"The value is invalid for the given schema: Unsupported value-schema combination! Value: Int(24), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })"# ); - assert!( - !test_outer3.validate(&schema), - "field b record is invalid against the schema" + assert_eq!( + test_outer3.validate(&schema).unwrap_err().to_string(), + r#"The value is invalid for the given schema: Unsupported value-schema combination! Value: Union(0, Null), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })"# ); Ok(()) @@ -3144,11 +3185,8 @@ Field with name '"b"' is not a member of the map items"#, }; let test_value: Value = to_value(msg)?; - assert!(test_value.validate(&schema), "test_value should validate"); - assert!( - test_value.resolve(&schema).is_ok(), - "test_value should resolve" - ); + test_value.validate(&schema)?; + test_value.resolve(&schema)?; Ok(()) } @@ -3225,11 +3263,8 @@ Field with name '"b"' is not a member of the map items"#, }; let test_value: Value = to_value(msg)?; - assert!(test_value.validate(&schema), "test_value should validate"); - assert!( - test_value.resolve(&schema).is_ok(), - "test_value should resolve" - ); + test_value.validate(&schema)?; + test_value.resolve(&schema)?; Ok(()) } @@ -3265,17 +3300,11 @@ Field with name '"b"' is not a member of the map items"#, let main_schema = schemas.first().unwrap(); let schemata: Vec<_> = schemas.iter().skip(1).collect(); - let resolve_result = avro_value.clone().resolve_schemata(main_schema, schemata); + avro_value.clone().resolve_schemata(main_schema, schemata)?; - assert!( - resolve_result.is_ok(), - "result of resolving with schemata should be ok, got: {resolve_result:?}" - ); - - let resolve_result = avro_value.resolve(main_schema); - assert!( - resolve_result.is_err(), - "result of resolving without schemata should be err, got: {resolve_result:?}" + assert_eq!( + avro_value.resolve(main_schema).unwrap_err().to_string(), + "Unresolved schema reference: enumForReference" ); Ok(()) @@ -3308,10 +3337,7 @@ Field with name '"b"' is not a member of the map items"#, let resolve_result = avro_value.resolve_schemata(main_schema, other_schemata)?; let schemata_ref = schemata.iter().collect::>(); - assert!( - resolve_result.validate_schemata(&schemata_ref), - "result of validation with schemata should be true" - ); + resolve_result.validate_schemata(&schemata_ref)?; Ok(()) } @@ -3324,11 +3350,7 @@ Field with name '"b"' is not a member of the map items"#, BigInt::from(12345678u32).to_signed_bytes_be(), )); let schema = Schema::parse_str(schema)?; - let resolve_result = avro_value.resolve(&schema); - assert!( - resolve_result.is_ok(), - "resolve result must be ok, got: {resolve_result:?}" - ); + avro_value.resolve(&schema)?; Ok(()) } @@ -3340,11 +3362,7 @@ Field with name '"b"' is not a member of the map items"#, let avro_value = Value::BigDecimal(BigDecimal::from(12345678u32)); let schema = Schema::parse_str(schema)?; - let resolve_result: AvroResult = avro_value.resolve(&schema); - assert!( - resolve_result.is_ok(), - "resolve result must be ok, got: {resolve_result:?}" - ); + avro_value.resolve(&schema)?; Ok(()) } @@ -3364,7 +3382,7 @@ Field with name '"b"' is not a member of the map items"#, ); let value = Value::Bytes(vec![97, 99]); - assert!( + assert_eq!( value .resolve(&Schema::Fixed(FixedSchema { name: "test".try_into()?, @@ -3373,11 +3391,13 @@ Field with name '"b"' is not a member of the map items"#, size: 3, attributes: Default::default() })) - .is_err(), + .unwrap_err() + .to_string(), + "Fixed size mismatch, expected: 3, got: 2" ); let value = Value::Bytes(vec![97, 98, 99, 100]); - assert!( + assert_eq!( value .resolve(&Schema::Fixed(FixedSchema { name: "test".try_into()?, @@ -3386,7 +3406,9 @@ Field with name '"b"' is not a member of the map items"#, size: 3, attributes: Default::default() })) - .is_err(), + .unwrap_err() + .to_string(), + "Fixed size mismatch, expected: 3, got: 4" ); Ok(()) diff --git a/avro/tests/avro-3786.rs b/avro/tests/avro-3786.rs index 9719fd0c..13088737 100644 --- a/avro/tests/avro-3786.rs +++ b/avro/tests/avro-3786.rs @@ -129,10 +129,7 @@ fn avro_3786_deserialize_union_with_different_enum_order() -> TestResult { bar_use_parent: Some(BarUseParent { bar_use: Bar::Bar1 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -258,10 +255,7 @@ fn avro_3786_deserialize_union_with_different_enum_order_defined_in_record() -> bar_parent: Some(BarParent { bar: Bar::Bar0 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -376,10 +370,7 @@ fn test_avro_3786_deserialize_union_with_different_enum_order_defined_in_record_ bar_parent: Some(BarParent { bar: Bar::Bar1 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -494,10 +485,7 @@ fn test_avro_3786_deserialize_union_with_different_enum_order_defined_in_record_ bar_parent: Some(BarParent { bar: Bar::Bar1 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -612,10 +600,7 @@ fn deserialize_union_with_different_enum_order_defined_in_record() -> TestResult bar_parent: Some(BarParent { bar: Bar::Bar2 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -891,10 +876,7 @@ fn deserialize_union_with_record_with_enum_defined_inline_reader_has_different_i }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; diff --git a/avro/tests/avro-3787.rs b/avro/tests/avro-3787.rs index b1558d9d..844b3c81 100644 --- a/avro/tests/avro-3787.rs +++ b/avro/tests/avro-3787.rs @@ -130,10 +130,7 @@ fn avro_3787_deserialize_union_with_unknown_symbol() -> TestResult { bar_use_parent: Some(BarUseParent { bar_use: Bar::Bar2 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; @@ -266,10 +263,7 @@ fn avro_3787_deserialize_union_with_unknown_symbol_no_ref() -> TestResult { bar_parent: Some(BarParent { bar: Bar::Bar2 }), }; let avro_value = to_value(foo2)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; diff --git a/avro/tests/io.rs b/avro/tests/io.rs index 2998a2cb..9fcf60b4 100644 --- a/avro/tests/io.rs +++ b/avro/tests/io.rs @@ -219,10 +219,7 @@ fn long_record_datum() -> &'static Value { fn test_validate() -> TestResult { for (raw_schema, value) in schemas_to_validate().iter() { let schema = Schema::parse_str(raw_schema)?; - assert!( - value.validate(&schema), - "value {value:?} does not validate schema: {raw_schema}" - ); + value.validate(&schema)?; } Ok(()) diff --git a/avro/tests/schema.rs b/avro/tests/schema.rs index 1419138c..690be0fa 100644 --- a/avro/tests/schema.rs +++ b/avro/tests/schema.rs @@ -952,10 +952,7 @@ fn test_avro_3785_deserialize_namespace_with_nullable_type_containing_reference_ bar_use_parent: Some(BarUseParent { bar_use: Bar::Bar1 }), }; let avro_value = to_value(foo1)?; - assert!( - avro_value.validate(&writer_schema), - "value is valid for schema", - ); + avro_value.validate(&writer_schema)?; let datum = GenericDatumWriter::builder(&writer_schema) .build()? .write_value_to_vec(avro_value)?; From 33b55c582556b0aa2e29e1e324708b0d9088fa5b Mon Sep 17 00:00:00 2001 From: Kriskras99 Date: Mon, 7 Sep 2026 22:06:05 +0200 Subject: [PATCH 2/4] feat: Display the full path of the Value in `validate` error messages --- avro/src/types.rs | 222 ++++++++++++++++++++++++------- avro/src/writer/datum.rs | 8 +- avro/src/writer/mod.rs | 2 + avro/src/writer/single_object.rs | 2 + 4 files changed, 184 insertions(+), 50 deletions(-) diff --git a/avro/src/types.rs b/avro/src/types.rs index da3b4973..caf63fd0 100644 --- a/avro/src/types.rs +++ b/avro/src/types.rs @@ -32,6 +32,7 @@ use crate::{ use bigdecimal::BigDecimal; use log::{debug, error}; use serde_json::{Number, Value as JsonValue}; +use std::fmt::Formatter; use std::{ borrow::Borrow, collections::{BTreeMap, HashMap}, @@ -368,6 +369,67 @@ impl TryFrom for JsonValue { } } +pub(crate) enum ValuePath<'a> { + Start, + Value(&'a Value, &'a ValuePath<'a>), + Index(usize, &'a ValuePath<'a>), + Key(&'a str, &'a ValuePath<'a>), + Field(&'a str, &'a ValuePath<'a>), +} + +impl<'a> std::fmt::Display for ValuePath<'a> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + ValuePath::Start => {} + ValuePath::Value(value, prev) => { + prev.fmt(f)?; + match value { + Value::Null => write!(f, "Null")?, + Value::Boolean(_) => write!(f, "Boolean")?, + Value::Int(_) => write!(f, "Int")?, + Value::Long(_) => write!(f, "Long")?, + Value::Float(_) => write!(f, "Float")?, + Value::Double(_) => write!(f, "Double")?, + Value::Bytes(_) => write!(f, "Bytes")?, + Value::String(_) => write!(f, "String")?, + Value::Fixed(_, _) => write!(f, "Fixed")?, + Value::Enum(_, _) => write!(f, "Enum")?, + Value::Union(_, _) => write!(f, "Union")?, + Value::Array(_) => write!(f, "Array")?, + Value::Map(_) => write!(f, "Map")?, + Value::Record(_) => write!(f, "Record")?, + Value::Date(_) => write!(f, "Date")?, + Value::Decimal(_) => write!(f, "Decimal")?, + Value::BigDecimal(_) => write!(f, "BigDecimal")?, + Value::TimeMillis(_) => write!(f, "TimeMillis")?, + Value::TimeMicros(_) => write!(f, "TimeMicros")?, + Value::TimestampMillis(_) => write!(f, "TimestampMillis")?, + Value::TimestampMicros(_) => write!(f, "TimestampMicros")?, + Value::TimestampNanos(_) => write!(f, "TimestampNanos")?, + Value::LocalTimestampMillis(_) => write!(f, "LocalTimestampMillis")?, + Value::LocalTimestampMicros(_) => write!(f, "LocalTimestampMicros")?, + Value::LocalTimestampNanos(_) => write!(f, "LocalTimestampNanos")?, + Value::Duration(_) => write!(f, "Duration")?, + Value::Uuid(_) => write!(f, "Uuid")?, + } + } + ValuePath::Index(index, prev) => { + prev.fmt(f)?; + write!(f, "[{index}].")?; + } + ValuePath::Key(key, prev) => { + prev.fmt(f)?; + write!(f, r#"["{key}"]."#)?; + } + ValuePath::Field(field, prev) => { + prev.fmt(f)?; + write!(f, ".{field}.")?; + } + } + Ok(()) + } +} + impl Value { /// Validate the value against the given [`Schema`]. /// @@ -379,7 +441,7 @@ impl Value { /// for the full set of rules of schema validation. pub fn validate(&self, schema: &Schema) -> AvroResult<()> { let rs = ResolvedSchema::new(schema)?; - match self.validate_internal(schema, rs.get_names(), None) { + match self.validate_internal(schema, rs.get_names(), None, &ValuePath::Start) { Some(reason) => Err(Details::InvalidValueForSchema(reason).into()), None => Ok(()), } @@ -398,7 +460,7 @@ impl Value { let schemata_len = schemata.len(); let mut errors = Vec::with_capacity(schemata_len); let found = schemata.iter().any(|schema| { - match self.validate_internal(schema, rs.get_names(), None) { + match self.validate_internal(schema, rs.get_names(), None, &ValuePath::Start) { Some(reason) => { errors.push(reason); false @@ -423,7 +485,7 @@ impl Value { schema: &Schema, names: &HashMap, ) -> AvroResult<()> { - match self.validate_internal(schema, names, None) { + match self.validate_internal(schema, names, None, &ValuePath::Start) { Some(reason) => Err(Details::InvalidValueForSchema(reason).into()), None => Ok(()), } @@ -444,6 +506,7 @@ impl Value { schema: &Schema, names: &HashMap, enclosing_namespace: NamespaceRef, + value_path: &ValuePath, ) -> Option { match (self, schema) { (_, Schema::Ref { name }) => { @@ -456,7 +519,7 @@ impl Value { names.keys() )) }, - |s| self.validate_internal(s.borrow(), names, name.namespace()), + |s| self.validate_internal(s.borrow(), names, name.namespace(), value_path), ) } (&Value::Null, &Schema::Null) => None, @@ -490,9 +553,10 @@ impl Value { (&Value::Bytes(_), &Schema::Bytes) => None, (&Value::Bytes(_), &Schema::Decimal { .. }) => None, (Value::Bytes(bytes), &Schema::Uuid(UuidSchema::Bytes)) => { + let value_path = ValuePath::Value(self, value_path); if bytes.len() != 16 { Some(format!( - "The value's size ({}) is not the right length for a bytes UUID (16)", + "Size of {value_path} ({}) is not the right length for a bytes UUID (16)", bytes.len() )) } else { @@ -501,10 +565,11 @@ impl Value { } (&Value::String(_), &Schema::String) => None, (Value::String(string), &Schema::Uuid(UuidSchema::String)) => { + let value_path = ValuePath::Value(self, value_path); // Non-hyphenated is 32 characters, hyphenated is longer if string.len() < 32 { Some(format!( - "The value's size ({}) is not the right length for a string UUID (>=32)", + "Size of {value_path} ({}) is not the right length for a string UUID (>=32)", string.len() )) } else { @@ -512,18 +577,20 @@ impl Value { } } (&Value::Fixed(n, _), &Schema::Fixed(FixedSchema { size, .. })) => { + let value_path = ValuePath::Value(self, value_path); if n != size { Some(format!( - "The value's size ({n}) is different than the schema's size ({size})" + "Size of {value_path} ({n}) is different than the schema's size ({size})" )) } else { None } } (Value::Bytes(b), &Schema::Fixed(FixedSchema { size, .. })) => { + let value_path = ValuePath::Value(self, value_path); if b.len() != size { Some(format!( - "The bytes' length ({}) is different than the schema's size ({})", + "Size of {value_path} ({}) is different than the schema's size ({})", b.len(), size )) @@ -532,23 +599,25 @@ impl Value { } } (&Value::Fixed(n, _), &Schema::Duration(_)) => { + let value_path = ValuePath::Value(self, value_path); if n != 12 { Some(format!( - "The value's size ('{n}') must be exactly 12 to be a Duration" + "Size of {value_path} ({n}) must be exactly 12 to be a Duration" )) } else { None } } (&Value::Fixed(n, _), Schema::Uuid(UuidSchema::Fixed(size, ..))) => { + let value_path = ValuePath::Value(self, value_path); if size.size != 16 { Some(format!( - "The schema's size ('{}') must be exactly 16 to be a Uuid", + "The schema's size ({}) must be exactly 16 to be a Uuid", size.size )) } else if n != 16 { Some(format!( - "The value's size ('{n}') must be exactly 16 to be a Uuid" + "Size of {value_path} ({n}) must be exactly 16 to be a Uuid" )) } else { None @@ -557,8 +626,12 @@ impl Value { // TODO: check precision against n (&Value::Fixed(_n, _), &Schema::Decimal { .. }) => None, (Value::String(s), Schema::Enum(EnumSchema { symbols, .. })) => { + let value_path = ValuePath::Value(self, value_path); if !symbols.contains(s) { - Some(format!("'{s}' is not a member of the possible symbols")) + // By doing s:? we get an escaped string + Some(format!( + "{value_path}({s:?}) is not a member of the possible symbols" + )) } else { None } @@ -568,42 +641,76 @@ impl Value { Schema::Enum(EnumSchema { symbols, default, .. }), - ) => symbols - .get(i as usize) - .map(|ref symbol| { - if symbol != &s { - Some(format!("Symbol '{s}' is not at position '{i}'")) - } else { - None - } - }) - .unwrap_or_else(|| match default { - Some(_) => None, - None => Some(format!("No symbol at position '{i}'")), - }), - // (&Value::Union(None), &Schema::Union(_)) => None, - (&Value::Union(i, ref value), Schema::Union(inner)) => inner - .variants() - .get(i as usize) - .map(|schema| value.validate_internal(schema, names, enclosing_namespace)) - .unwrap_or_else(|| Some(format!("No schema in the union at position '{i}'"))), + ) => { + let value_path = ValuePath::Value(self, value_path); + symbols + .get(i as usize) + .map(|ref symbol| { + if symbol != &s { + Some(format!("{value_path}({s:?}) is not at position {i} in the schema")) + } else { + None + } + }) + .unwrap_or_else(|| match default { + Some(_) => None, + None => Some(format!("{value_path}({s:?}) is at position {i} but that position is not in the schema")), + }) + } + (&Value::Union(i, ref value), Schema::Union(inner)) => { + let value_path = ValuePath::Value(self, value_path); + inner + .variants() + .get(i as usize) + .map(|schema| { + value.validate_internal( + schema, + names, + enclosing_namespace, + &ValuePath::Index(i as usize, &value_path), + ) + }) + .unwrap_or_else(|| { + Some(format!( + "{} is at position {i} but that position is not in the schema", + ValuePath::Index(i as usize, &value_path) + )) + }) + } (v, Schema::Union(inner)) => { match inner.find_schema_with_known_schemata(v, Some(names), enclosing_namespace) { Some(_) => None, - None => Some("Could not find matching type in union".to_string()), + None => Some(format!( + "Could not find matching type in union for {}", + ValuePath::Value(v, value_path) + )), } } - (Value::Array(items), Schema::Array(inner)) => items.iter().fold(None, |acc, item| { - Value::accumulate( - acc, - item.validate_internal(&inner.items, names, enclosing_namespace), - ) - }), + (Value::Array(items), Schema::Array(inner)) => { + let value_path = ValuePath::Value(self, value_path); + items.iter().enumerate().fold(None, |acc, (index, item)| { + Value::accumulate( + acc, + item.validate_internal( + &inner.items, + names, + enclosing_namespace, + &ValuePath::Index(index, &value_path), + ), + ) + }) + } (Value::Map(items), Schema::Map(inner)) => { - items.iter().fold(None, |acc, (_, value)| { + let value_path = ValuePath::Value(self, value_path); + items.iter().fold(None, |acc, (key, value)| { Value::accumulate( acc, - value.validate_internal(&inner.types, names, enclosing_namespace), + value.validate_internal( + &inner.types, + names, + enclosing_namespace, + &ValuePath::Key(key, &value_path), + ), ) }) } @@ -616,19 +723,20 @@ impl Value { .. }), ) => { + let value_path = ValuePath::Value(self, value_path); let non_nullable_fields_count = fields.iter().filter(|&rf| !rf.is_nullable()).count(); // If the record contains fewer fields as required fields by the schema, it is invalid. if record_fields.len() < non_nullable_fields_count { return Some(format!( - "The value's records length ({}) doesn't match the schema ({} non-nullable fields)", + "{value_path} has {} fields which doesn't match the schema ({} non-nullable fields)", record_fields.len(), non_nullable_fields_count )); } else if record_fields.len() > fields.len() { return Some(format!( - "The value's records length ({}) is greater than the schema's ({} fields)", + "{value_path} has {} fields which is greater than the schema's ({} fields)", record_fields.len(), fields.len(), )); @@ -647,26 +755,34 @@ impl Value { &field.schema, names, record_namespace, + &ValuePath::Field(&field.name, &value_path), ), ) } None => Value::accumulate( acc, - Some(format!("There is no schema field for field '{field_name}'")), + Some(format!("There is no schema field for field '{field_name}' in {value_path}")), ), } }) } (Value::Map(items), Schema::Record(RecordSchema { fields, .. })) => { + let value_path = ValuePath::Value(self, value_path); fields.iter().fold(None, |acc, field| { if let Some(item) = items.get(&field.name) { - let res = item.validate_internal(&field.schema, names, enclosing_namespace); + // ValuePath is a Key, because the Value is a Map not a Record + let res = item.validate_internal( + &field.schema, + names, + enclosing_namespace, + &ValuePath::Key(&field.name, &value_path), + ); Value::accumulate(acc, res) } else if !field.is_nullable() { Value::accumulate( acc, Some(format!( - "Field with name '{:?}' is not a member of the map items", + "Field with name {:?} is not a key in {value_path}", field.name )), ) @@ -675,9 +791,12 @@ impl Value { } }) } - (v, s) => Some(format!( - "Unsupported value-schema combination! Value: {v:?}, schema: {s:?}" - )), + (_, s) => { + let value_path = ValuePath::Value(self, value_path); + Some(format!( + "Unsupported value-schema combination! Value: {value_path}, schema: {s:?}" + )) + } } } @@ -1559,7 +1678,12 @@ mod tests { ]; for (value, schema, valid, expected_err_message) in value_schema_valid.into_iter() { - let err_message = value.validate_internal::(&schema, &HashMap::default(), None); + let err_message = value.validate_internal::( + &schema, + &HashMap::default(), + None, + &ValuePath::Start, + ); assert_eq!(valid, err_message.is_none()); if !valid { let full_err_message = format!( diff --git a/avro/src/writer/datum.rs b/avro/src/writer/datum.rs index 30804ff6..9c186238 100644 --- a/avro/src/writer/datum.rs +++ b/avro/src/writer/datum.rs @@ -19,6 +19,7 @@ use bon::bon; use serde::Serialize; use std::io::Write; +use crate::types::ValuePath; use crate::{ AvroResult, Schema, encode::encode_internal, @@ -125,7 +126,12 @@ impl GenericDatumWriter<'_> { pub fn write_value_ref(&self, writer: &mut W, value: &Value) -> AvroResult { if self.validate && value - .validate_internal(self.schema, self.resolved.get_names(), None) + .validate_internal( + self.schema, + self.resolved.get_names(), + None, + &ValuePath::Start, + ) .is_some() { return Err(Details::Validation.into()); diff --git a/avro/src/writer/mod.rs b/avro/src/writer/mod.rs index 528b3e7b..bdc3b996 100644 --- a/avro/src/writer/mod.rs +++ b/avro/src/writer/mod.rs @@ -16,6 +16,7 @@ // under the License. //! Logic handling writing in Avro format at user level. +use crate::types::ValuePath; use crate::{ AvroResult, Codec, Error, encode::{encode, encode_internal, encode_to_vec}, @@ -220,6 +221,7 @@ impl<'a, W: Write> Writer<'a, W> { self.schema, self.resolved_schema.get_names(), self.schema.namespace(), + &ValuePath::Start, ) { return Err(Details::ValidationWithReason { value: value.clone(), diff --git a/avro/src/writer/single_object.rs b/avro/src/writer/single_object.rs index 2fdab1be..fa146e23 100644 --- a/avro/src/writer/single_object.rs +++ b/avro/src/writer/single_object.rs @@ -23,6 +23,7 @@ use serde::Serialize; use crate::Error; use crate::encode::encode_internal; use crate::serde::ser_schema::{Config, SchemaAwareSerializer}; +use crate::types::ValuePath; use crate::util::is_human_readable; use crate::{ AvroResult, AvroSchema, Schema, @@ -237,6 +238,7 @@ fn write_value_ref_owned_resolved( root_schema, resolved_schema.get_names(), root_schema.namespace(), + &ValuePath::Start, ) { return Err(Details::ValidationWithReason { value: value.clone(), From f5fc0a50fa6c9711f8584cfe8bb69943f47e658e Mon Sep 17 00:00:00 2001 From: Kriskras99 Date: Tue, 8 Sep 2026 15:13:44 +0200 Subject: [PATCH 3/4] feat: Display the full path of the Schema in `validate` error messages --- avro/src/types.rs | 309 ++++++++++++++++++++++--------- avro/src/writer/datum.rs | 3 +- avro/src/writer/mod.rs | 7 +- avro/src/writer/single_object.rs | 3 +- 4 files changed, 225 insertions(+), 97 deletions(-) diff --git a/avro/src/types.rs b/avro/src/types.rs index caf63fd0..2622e354 100644 --- a/avro/src/types.rs +++ b/avro/src/types.rs @@ -40,6 +40,7 @@ use std::{ hash::BuildHasher, str::FromStr, }; +use strum::IntoDiscriminant; use uuid::Uuid; /// Compute the maximum decimal value precision of a byte array of length `len` could hold. @@ -430,6 +431,93 @@ impl<'a> std::fmt::Display for ValuePath<'a> { } } +pub(crate) enum SchemaPath<'a> { + Start, + Schema(&'a Schema, &'a SchemaPath<'a>), + Index(usize, &'a SchemaPath<'a>), + Field(&'a str, &'a SchemaPath<'a>), +} + +impl<'a> std::fmt::Display for SchemaPath<'a> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + self.fmt_inner(f).map(|_| ()) + } +} + +impl<'a> SchemaPath<'a> { + fn fmt_inner(&self, f: &mut Formatter<'_>) -> Result, std::fmt::Error> { + match self { + SchemaPath::Start => Ok(None), + SchemaPath::Schema(schema, prev) => { + let enclosing_namespace = prev.fmt_inner(f)?; + match schema { + Schema::Null => write!(f, "Null")?, + Schema::Boolean => write!(f, "Boolean")?, + Schema::Int => write!(f, "Int")?, + Schema::Long => write!(f, "Long")?, + Schema::Float => write!(f, "Float")?, + Schema::Double => write!(f, "Double")?, + Schema::Bytes => write!(f, "Bytes")?, + Schema::String => write!(f, "String")?, + Schema::Array(_) => write!(f, "Array")?, + Schema::Map(_) => write!(f, "Map")?, + Schema::Union(_) => write!(f, "Union")?, + Schema::Record(_) => write!(f, "Record")?, + Schema::Enum(_) => write!(f, "Enum")?, + Schema::Fixed(_) => write!(f, "Fixed")?, + Schema::Decimal(DecimalSchema { + inner: InnerDecimalSchema::Fixed(_), + .. + }) => write!(f, "Decimal(Fixed)")?, + Schema::Decimal(DecimalSchema { + inner: InnerDecimalSchema::Bytes, + .. + }) => write!(f, "Decimal(Bytes)")?, + Schema::BigDecimal => write!(f, "BigDecimal")?, + Schema::Uuid(UuidSchema::Fixed(_)) => write!(f, "Uuid(Bytes)")?, + Schema::Uuid(UuidSchema::Bytes) => write!(f, "Uuid(String)")?, + Schema::Uuid(UuidSchema::String) => write!(f, "Uuid(Fixed)")?, + Schema::Date => write!(f, "Date")?, + Schema::TimeMillis => write!(f, "TimeMillis")?, + Schema::TimeMicros => write!(f, "TimeMicros")?, + Schema::TimestampMillis => write!(f, "TimestampMillis")?, + Schema::TimestampMicros => write!(f, "TimestampMicros")?, + Schema::TimestampNanos => write!(f, "TimestampNanos")?, + Schema::LocalTimestampMillis => write!(f, "LocalTimestampMillis")?, + Schema::LocalTimestampMicros => write!(f, "LocalTimestampMicros")?, + Schema::LocalTimestampNanos => write!(f, "LocalTimestampNanos")?, + Schema::Duration(_) => write!(f, "Duration")?, + Schema::Ref { .. } => write!(f, "&")?, + } + + if schema.discriminant() != SchemaKind::Ref + && let Some(name) = schema.name() + { + if name.namespace().is_none() || name.namespace() == enclosing_namespace { + write!(f, "{{{}}}", name.name())?; + Ok(enclosing_namespace) + } else { + write!(f, "{{{name}}}")?; + Ok(name.namespace()) + } + } else { + Ok(enclosing_namespace) + } + } + SchemaPath::Index(index, prev) => { + let enclosing_namespace = prev.fmt_inner(f)?; + write!(f, "[{index}].")?; + Ok(enclosing_namespace) + } + SchemaPath::Field(field, prev) => { + let enclosing_namespace = prev.fmt_inner(f)?; + write!(f, ".{field}.")?; + Ok(enclosing_namespace) + } + } + } +} + impl Value { /// Validate the value against the given [`Schema`]. /// @@ -441,7 +529,13 @@ impl Value { /// for the full set of rules of schema validation. pub fn validate(&self, schema: &Schema) -> AvroResult<()> { let rs = ResolvedSchema::new(schema)?; - match self.validate_internal(schema, rs.get_names(), None, &ValuePath::Start) { + match self.validate_internal( + schema, + rs.get_names(), + None, + &ValuePath::Start, + &SchemaPath::Start, + ) { Some(reason) => Err(Details::InvalidValueForSchema(reason).into()), None => Ok(()), } @@ -459,8 +553,14 @@ impl Value { let rs = ResolvedSchema::try_from(schemata.to_vec())?; let schemata_len = schemata.len(); let mut errors = Vec::with_capacity(schemata_len); - let found = schemata.iter().any(|schema| { - match self.validate_internal(schema, rs.get_names(), None, &ValuePath::Start) { + let found = schemata.iter().enumerate().any(|(index, schema)| { + match self.validate_internal( + schema, + rs.get_names(), + None, + &ValuePath::Start, + &SchemaPath::Index(index, &SchemaPath::Start), + ) { Some(reason) => { errors.push(reason); false @@ -485,7 +585,7 @@ impl Value { schema: &Schema, names: &HashMap, ) -> AvroResult<()> { - match self.validate_internal(schema, names, None, &ValuePath::Start) { + match self.validate_internal(schema, names, None, &ValuePath::Start, &SchemaPath::Start) { Some(reason) => Err(Details::InvalidValueForSchema(reason).into()), None => Ok(()), } @@ -507,6 +607,7 @@ impl Value { names: &HashMap, enclosing_namespace: NamespaceRef, value_path: &ValuePath, + schema_path: &SchemaPath, ) -> Option { match (self, schema) { (_, Schema::Ref { name }) => { @@ -519,7 +620,15 @@ impl Value { names.keys() )) }, - |s| self.validate_internal(s.borrow(), names, name.namespace(), value_path), + |s| { + self.validate_internal( + s.borrow(), + names, + name.namespace(), + value_path, + &SchemaPath::Schema(schema, schema_path), + ) + }, ) } (&Value::Null, &Schema::Null) => None, @@ -556,8 +665,9 @@ impl Value { let value_path = ValuePath::Value(self, value_path); if bytes.len() != 16 { Some(format!( - "Size of {value_path} ({}) is not the right length for a bytes UUID (16)", - bytes.len() + "Size of {value_path} ({}) is not the right length for {} (16)", + bytes.len(), + SchemaPath::Schema(schema, schema_path) )) } else { None @@ -566,11 +676,14 @@ impl Value { (&Value::String(_), &Schema::String) => None, (Value::String(string), &Schema::Uuid(UuidSchema::String)) => { let value_path = ValuePath::Value(self, value_path); - // Non-hyphenated is 32 characters, hyphenated is longer - if string.len() < 32 { + if string.len() < uuid::fmt::Simple::LENGTH || string.len() > uuid::fmt::Urn::LENGTH + { Some(format!( - "Size of {value_path} ({}) is not the right length for a string UUID (>=32)", - string.len() + "Size of {value_path} ({}) is not the right length for {} ({}..={})", + string.len(), + SchemaPath::Schema(schema, schema_path), + uuid::fmt::Simple::LENGTH, + uuid::fmt::Urn::LENGTH, )) } else { None @@ -580,7 +693,8 @@ impl Value { let value_path = ValuePath::Value(self, value_path); if n != size { Some(format!( - "Size of {value_path} ({n}) is different than the schema's size ({size})" + "Size of {value_path} ({n}) is different than {} ({size})", + SchemaPath::Schema(schema, schema_path), )) } else { None @@ -590,9 +704,10 @@ impl Value { let value_path = ValuePath::Value(self, value_path); if b.len() != size { Some(format!( - "Size of {value_path} ({}) is different than the schema's size ({})", + "Size of {value_path} ({}) is different than {} ({})", b.len(), - size + size, + SchemaPath::Schema(schema, schema_path), )) } else { None @@ -602,7 +717,8 @@ impl Value { let value_path = ValuePath::Value(self, value_path); if n != 12 { Some(format!( - "Size of {value_path} ({n}) must be exactly 12 to be a Duration" + "Size of {value_path} ({n}) must be exactly 12 for {}", + SchemaPath::Schema(schema, schema_path), )) } else { None @@ -612,12 +728,14 @@ impl Value { let value_path = ValuePath::Value(self, value_path); if size.size != 16 { Some(format!( - "The schema's size ({}) must be exactly 16 to be a Uuid", - size.size + "Invalid schema: {} must be exactly 16 not {}", + SchemaPath::Schema(schema, schema_path), + size.size, )) } else if n != 16 { Some(format!( - "Size of {value_path} ({n}) must be exactly 16 to be a Uuid" + "Size of {value_path} ({n}) must be exactly 16 for {}", + SchemaPath::Schema(schema, schema_path), )) } else { None @@ -630,7 +748,8 @@ impl Value { if !symbols.contains(s) { // By doing s:? we get an escaped string Some(format!( - "{value_path}({s:?}) is not a member of the possible symbols" + "{value_path}({s:?}) is not a symbol in {}", + SchemaPath::Schema(schema, schema_path), )) } else { None @@ -647,18 +766,21 @@ impl Value { .get(i as usize) .map(|ref symbol| { if symbol != &s { - Some(format!("{value_path}({s:?}) is not at position {i} in the schema")) + Some(format!("{value_path}({s:?}) does not exist at {i} in {}", + SchemaPath::Schema(schema, schema_path),)) } else { None } }) .unwrap_or_else(|| match default { Some(_) => None, - None => Some(format!("{value_path}({s:?}) is at position {i} but that position is not in the schema")), + None => Some(format!("{value_path}({s:?}) is at position {i} but that position does not exist in {}", + SchemaPath::Schema(schema, schema_path),)), }) } (&Value::Union(i, ref value), Schema::Union(inner)) => { let value_path = ValuePath::Value(self, value_path); + let schema_path = SchemaPath::Schema(schema, schema_path); inner .variants() .get(i as usize) @@ -668,11 +790,12 @@ impl Value { names, enclosing_namespace, &ValuePath::Index(i as usize, &value_path), + &SchemaPath::Index(i as usize, &schema_path), ) }) .unwrap_or_else(|| { Some(format!( - "{} is at position {i} but that position is not in the schema", + "{} is at position {i} but that position does not exist in {schema_path}", ValuePath::Index(i as usize, &value_path) )) }) @@ -681,13 +804,15 @@ impl Value { match inner.find_schema_with_known_schemata(v, Some(names), enclosing_namespace) { Some(_) => None, None => Some(format!( - "Could not find matching type in union for {}", - ValuePath::Value(v, value_path) + "Could not find type matching {} in {}", + ValuePath::Value(v, value_path), + SchemaPath::Schema(schema, schema_path), )), } } (Value::Array(items), Schema::Array(inner)) => { let value_path = ValuePath::Value(self, value_path); + let schema_path = SchemaPath::Schema(schema, schema_path); items.iter().enumerate().fold(None, |acc, (index, item)| { Value::accumulate( acc, @@ -696,12 +821,14 @@ impl Value { names, enclosing_namespace, &ValuePath::Index(index, &value_path), + &SchemaPath::Index(index, &schema_path), ), ) }) } (Value::Map(items), Schema::Map(inner)) => { let value_path = ValuePath::Value(self, value_path); + let schema_path = SchemaPath::Schema(schema, schema_path); items.iter().fold(None, |acc, (key, value)| { Value::accumulate( acc, @@ -710,6 +837,7 @@ impl Value { names, enclosing_namespace, &ValuePath::Key(key, &value_path), + &schema_path, ), ) }) @@ -724,19 +852,20 @@ impl Value { }), ) => { let value_path = ValuePath::Value(self, value_path); + let schema_path = SchemaPath::Schema(schema, schema_path); let non_nullable_fields_count = fields.iter().filter(|&rf| !rf.is_nullable()).count(); // If the record contains fewer fields as required fields by the schema, it is invalid. if record_fields.len() < non_nullable_fields_count { return Some(format!( - "{value_path} has {} fields which doesn't match the schema ({} non-nullable fields)", + "{value_path} has {} fields which doesn't match {schema_path} ({} non-nullable fields)", record_fields.len(), non_nullable_fields_count )); } else if record_fields.len() > fields.len() { return Some(format!( - "{value_path} has {} fields which is greater than the schema's ({} fields)", + "{value_path} has {} fields which is greater than {schema_path} ({} fields)", record_fields.len(), fields.len(), )); @@ -756,18 +885,20 @@ impl Value { names, record_namespace, &ValuePath::Field(&field.name, &value_path), + &SchemaPath::Field(&field.name, &schema_path), ), ) } None => Value::accumulate( acc, - Some(format!("There is no schema field for field '{field_name}' in {value_path}")), + Some(format!("{value_path} has a field '{field_name}' but that does not exist in {schema_path}")), ), } }) } (Value::Map(items), Schema::Record(RecordSchema { fields, .. })) => { let value_path = ValuePath::Value(self, value_path); + let schema_path = SchemaPath::Schema(schema, schema_path); fields.iter().fold(None, |acc, field| { if let Some(item) = items.get(&field.name) { // ValuePath is a Key, because the Value is a Map not a Record @@ -776,14 +907,15 @@ impl Value { names, enclosing_namespace, &ValuePath::Key(&field.name, &value_path), + &SchemaPath::Field(&field.name, &schema_path), ); Value::accumulate(acc, res) } else if !field.is_nullable() { Value::accumulate( acc, Some(format!( - "Field with name {:?} is not a key in {value_path}", - field.name + "{} is not a key in {value_path}", + SchemaPath::Field(&field.name, &schema_path), )), ) } else { @@ -791,10 +923,11 @@ impl Value { } }) } - (_, s) => { + (_, _) => { let value_path = ValuePath::Value(self, value_path); + let schema_path = SchemaPath::Schema(schema, schema_path); Some(format!( - "Unsupported value-schema combination! Value: {value_path}, schema: {s:?}" + "Unsupported value-schema combination! Value: {value_path}, schema: {schema_path}" )) } } @@ -1541,33 +1674,31 @@ mod tests { #[test] fn validate() -> TestResult { let value_schema_valid = vec![ - (Value::Int(42), Schema::Int, true, ""), - (Value::Int(43), Schema::Long, true, ""), - (Value::Float(43.2), Schema::Float, true, ""), - (Value::Float(45.9), Schema::Double, true, ""), + (Value::Int(42), Schema::Int, None), + (Value::Int(43), Schema::Long, None), + (Value::Float(43.2), Schema::Float, None), + (Value::Float(45.9), Schema::Double, None), ( Value::Int(42), Schema::Boolean, - false, - "Invalid value: Int(42) for schema: Boolean. Reason: Unsupported value-schema combination! Value: Int(42), schema: Boolean", + Some("Unsupported value-schema combination! Value: Int, schema: Boolean"), ), ( Value::Union(0, Box::new(Value::Null)), Schema::Union(UnionSchema::new(vec![Schema::Null, Schema::Int])?), - true, - "", + None, ), ( Value::Union(1, Box::new(Value::Int(42))), Schema::Union(UnionSchema::new(vec![Schema::Null, Schema::Int])?), - true, - "", + None, ), ( Value::Union(0, Box::new(Value::Null)), Schema::Union(UnionSchema::new(vec![Schema::Double, Schema::Int])?), - false, - "Invalid value: Union(0, Null) for schema: Union(UnionSchema { schemas: [Double, Int] }). Reason: Unsupported value-schema combination! Value: Null, schema: Double", + Some( + "Unsupported value-schema combination! Value: Union[0].Null, schema: Union[0].Double", + ), ), ( Value::Union(3, Box::new(Value::Int(42))), @@ -1577,8 +1708,7 @@ mod tests { Schema::String, Schema::Int, ])?), - true, - "", + None, ), ( Value::Union(1, Box::new(Value::Long(42i64))), @@ -1586,32 +1716,36 @@ mod tests { Schema::Null, Schema::TimestampMillis, ])?), - true, - "", + None, ), ( Value::Union(2, Box::new(Value::Long(1_i64))), Schema::Union(UnionSchema::new(vec![Schema::Null, Schema::Int])?), - false, - "Invalid value: Union(2, Long(1)) for schema: Union(UnionSchema { schemas: [Null, Int] }). Reason: No schema in the union at position '2'", + Some("Union[2]. is at position 2 but that position does not exist in Union"), ), ( Value::Array(vec![Value::Long(42i64)]), Schema::array(Schema::Long).build(), - true, - "", + None, ), ( Value::Array(vec![Value::Boolean(true)]), Schema::array(Schema::Long).build(), - false, - "Invalid value: Array([Boolean(true)]) for schema: Array(ArraySchema { items: Long, .. }). Reason: Unsupported value-schema combination! Value: Boolean(true), schema: Long", + Some( + "Unsupported value-schema combination! Value: Array[0].Boolean, schema: Array[0].Long", + ), + ), + ( + Value::Array(vec![Value::Long(42), Value::Boolean(true)]), + Schema::array(Schema::Long).build(), + Some( + "Unsupported value-schema combination! Value: Array[1].Boolean, schema: Array[1].Long", + ), ), ( Value::Record(vec![]), Schema::Null, - false, - "Invalid value: Record([]) for schema: Null. Reason: Unsupported value-schema combination! Value: Record([]), schema: Null", + Some("Unsupported value-schema combination! Value: Record, schema: Null"), ), ( Value::Fixed(12, vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]), @@ -1622,8 +1756,7 @@ mod tests { size: 12, attributes: BTreeMap::new(), }), - true, - "", + None, ), ( Value::Fixed(11, vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]), @@ -1634,8 +1767,7 @@ mod tests { size: 12, attributes: BTreeMap::new(), }), - false, - r#"Invalid value: Fixed(11, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]) for schema: Duration(FixedSchema { name: Name { name: "TestName", .. }, size: 12, .. }). Reason: The value's size ('11') must be exactly 12 to be a Duration"#, + Some("Size of Fixed (11) must be exactly 12 for Duration{TestName}"), ), ( Value::Record(vec![("unknown_field_name".to_string(), Value::Null)]), @@ -1652,8 +1784,9 @@ mod tests { lookup: Default::default(), attributes: Default::default(), }), - false, - r#"Invalid value: Record([("unknown_field_name", Null)]) for schema: Record(RecordSchema { name: Name { name: "record_name", .. }, fields: [RecordField { name: "field_name", schema: Int, .. }], .. }). Reason: There is no schema field for field 'unknown_field_name'"#, + Some( + "Record has a field 'unknown_field_name' but that does not exist in Record{record_name}", + ), ), ( Value::Record(vec![("field_name".to_string(), Value::Null)]), @@ -1672,28 +1805,21 @@ mod tests { lookup: [("field_name".to_string(), 0)].iter().cloned().collect(), attributes: Default::default(), }), - false, - r#"Invalid value: Record([("field_name", Null)]) for schema: Record(RecordSchema { name: Name { name: "record_name", .. }, fields: [RecordField { name: "field_name", schema: Ref { name: Name { name: "missing", .. } }, .. }], .. }). Reason: Unresolved schema reference: 'Name { name: "missing", .. }'. Parsed names: []"#, + Some( + r#"Unresolved schema reference: 'Name { name: "missing", .. }'. Parsed names: []"#, + ), ), ]; - for (value, schema, valid, expected_err_message) in value_schema_valid.into_iter() { + for (value, schema, expected_err_message) in value_schema_valid.into_iter() { let err_message = value.validate_internal::( &schema, &HashMap::default(), None, &ValuePath::Start, + &SchemaPath::Start, ); - assert_eq!(valid, err_message.is_none()); - if !valid { - let full_err_message = format!( - "Invalid value: {:?} for schema: {:?}. Reason: {}", - value, - schema, - err_message.unwrap() - ); - assert_eq!(expected_err_message, full_err_message); - } + assert_eq!(err_message.as_deref(), expected_err_message); } Ok(()) @@ -1713,18 +1839,18 @@ mod tests { let value = Value::Fixed(5, vec![0, 0, 0, 0, 0]); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: The value's size (5) is different than the schema's size (4)" + "The value is invalid for the given schema: Size of Fixed (5) is different than Fixed{some_fixed} (4)" ); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: The value's size (5) is different than the schema's size (4)" + "The value is invalid for the given schema: Size of Fixed (5) is different than Fixed{some_fixed} (4)" ); Value::Bytes(vec![0, 0, 0, 0]).validate(&schema)?; let value = Value::Bytes(vec![0, 0, 0, 0, 0]); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: The bytes' length (5) is different than the schema's size (4)" + "The value is invalid for the given schema: Size of Bytes (5) is different than 4 (Fixed{some_fixed})" ); Ok(()) @@ -1752,19 +1878,19 @@ mod tests { let value = Value::Enum(1, "spades".to_string()); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: Symbol 'spades' is not at position '1'" + r#"The value is invalid for the given schema: Enum("spades") does not exist at 1 in Enum{some_enum}"# ); let value = Value::Enum(1000, "spades".to_string()); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: No symbol at position '1000'" + r#"The value is invalid for the given schema: Enum("spades") is at position 1000 but that position does not exist in Enum{some_enum}"# ); let value = Value::String("lorem".to_string()); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: 'lorem' is not a member of the possible symbols" + r#"The value is invalid for the given schema: String("lorem") is not a symbol in Enum{some_enum}"# ); let other_schema = Schema::Enum(EnumSchema { @@ -1784,7 +1910,7 @@ mod tests { let value = Value::Enum(0, "spades".to_string()); assert_eq!( value.validate(&other_schema).unwrap_err().to_string(), - "The value is invalid for the given schema: Symbol 'spades' is not at position '0'" + r#"The value is invalid for the given schema: Enum("spades") does not exist at 0 in Enum{some_other_enum}"# ); Ok(()) @@ -1855,7 +1981,7 @@ mod tests { ]); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: Unsupported value-schema combination! Value: Boolean(false), schema: Long" + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.a.Boolean, schema: Record{some_record}.a.Long" ); let value = Value::Record(vec![ @@ -1864,7 +1990,7 @@ mod tests { ]); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: Could not find matching type in union", + "The value is invalid for the given schema: Could not find type matching Record.c.String in Record{some_record}.c.Union", ); let value = Value::Record(vec![ @@ -1873,7 +1999,7 @@ mod tests { ]); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: There is no schema field for field 'd'", + "The value is invalid for the given schema: Record has a field 'd' but that does not exist in Record{some_record}", ); let value = Value::Record(vec![ @@ -1884,7 +2010,7 @@ mod tests { ]); assert_eq!( value.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: The value's records length (4) is greater than the schema's (3 fields)", + "The value is invalid for the given schema: Record has 4 fields which is greater than Record{some_record} (3 fields)", ); Value::Map( @@ -1906,8 +2032,7 @@ mod tests { .validate(&schema) .unwrap_err() .to_string(), - r#"The value is invalid for the given schema: Field with name '"a"' is not a member of the map items -Field with name '"b"' is not a member of the map items"#, + "The value is invalid for the given schema: Record{some_record}.a. is not a key in Map\nRecord{some_record}.b. is not a key in Map", ); let union_schema = Schema::Union(UnionSchema::new(vec![Schema::Null, schema])?); @@ -3141,11 +3266,11 @@ Field with name '"b"' is not a member of the map items"#, assert_eq!( outer1.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: Unsupported value-schema combination! Value: Null, schema: Int", + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.b.Record.z.Null, schema: Record{TestStruct}.b.&Record{Inner}.z.Int", ); assert_eq!( outer2.validate(&schema).unwrap_err().to_string(), - "The value is invalid for the given schema: There is no schema field for field 'a'" + "The value is invalid for the given schema: Record.b.Record has a field 'a' but that does not exist in Record{TestStruct}.b.&Record{Inner}" ); Ok(()) @@ -3223,15 +3348,15 @@ Field with name '"b"' is not a member of the map items"#, assert_eq!( test_outer1.validate(&schema).unwrap_err().to_string(), - r#"The value is invalid for the given schema: Unsupported value-schema combination! Value: String("testing"), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })"# + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.b.String, schema: Record{TestStruct}.b.&Record{Inner}" ); assert_eq!( test_outer2.validate(&schema).unwrap_err().to_string(), - r#"The value is invalid for the given schema: Unsupported value-schema combination! Value: Int(24), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })"# + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.b.Int, schema: Record{TestStruct}.b.&Record{Inner}" ); assert_eq!( test_outer3.validate(&schema).unwrap_err().to_string(), - r#"The value is invalid for the given schema: Unsupported value-schema combination! Value: Union(0, Null), schema: Record(RecordSchema { name: Name { name: "Inner", .. }, fields: [RecordField { name: "z", schema: Int, .. }], .. })"# + "The value is invalid for the given schema: Unsupported value-schema combination! Value: Record.b.Union, schema: Record{TestStruct}.b.&Record{Inner}" ); Ok(()) diff --git a/avro/src/writer/datum.rs b/avro/src/writer/datum.rs index 9c186238..54b1f1c8 100644 --- a/avro/src/writer/datum.rs +++ b/avro/src/writer/datum.rs @@ -19,7 +19,7 @@ use bon::bon; use serde::Serialize; use std::io::Write; -use crate::types::ValuePath; +use crate::types::{SchemaPath, ValuePath}; use crate::{ AvroResult, Schema, encode::encode_internal, @@ -131,6 +131,7 @@ impl GenericDatumWriter<'_> { self.resolved.get_names(), None, &ValuePath::Start, + &SchemaPath::Start, ) .is_some() { diff --git a/avro/src/writer/mod.rs b/avro/src/writer/mod.rs index bdc3b996..efe4337e 100644 --- a/avro/src/writer/mod.rs +++ b/avro/src/writer/mod.rs @@ -16,7 +16,7 @@ // under the License. //! Logic handling writing in Avro format at user level. -use crate::types::ValuePath; +use crate::types::{SchemaPath, ValuePath}; use crate::{ AvroResult, Codec, Error, encode::{encode, encode_internal, encode_to_vec}, @@ -222,6 +222,7 @@ impl<'a, W: Write> Writer<'a, W> { self.resolved_schema.get_names(), self.schema.namespace(), &ValuePath::Start, + &SchemaPath::Start, ) { return Err(Details::ValidationWithReason { value: value.clone(), @@ -1241,12 +1242,12 @@ mod tests { let err = writer.append_value_ref(&value).unwrap_err(); assert_eq!( err.to_string(), - "Value Int(1) does not match schema String: Reason: Unsupported value-schema combination! Value: Int(1), schema: String" + "Value Int(1) does not match schema String: Reason: Unsupported value-schema combination! Value: Int, schema: String" ); let err = writer.append_value(value).unwrap_err(); assert_eq!( err.to_string(), - "Value Int(1) does not match schema String: Reason: Unsupported value-schema combination! Value: Int(1), schema: String" + "Value Int(1) does not match schema String: Reason: Unsupported value-schema combination! Value: Int, schema: String" ); Ok(()) diff --git a/avro/src/writer/single_object.rs b/avro/src/writer/single_object.rs index fa146e23..8df3f7ad 100644 --- a/avro/src/writer/single_object.rs +++ b/avro/src/writer/single_object.rs @@ -23,7 +23,7 @@ use serde::Serialize; use crate::Error; use crate::encode::encode_internal; use crate::serde::ser_schema::{Config, SchemaAwareSerializer}; -use crate::types::ValuePath; +use crate::types::{SchemaPath, ValuePath}; use crate::util::is_human_readable; use crate::{ AvroResult, AvroSchema, Schema, @@ -239,6 +239,7 @@ fn write_value_ref_owned_resolved( resolved_schema.get_names(), root_schema.namespace(), &ValuePath::Start, + &SchemaPath::Start, ) { return Err(Details::ValidationWithReason { value: value.clone(), From 5db4927a6526f20b3ffd9099ab883a5ca9accbab Mon Sep 17 00:00:00 2001 From: Kriskras99 Date: Wed, 9 Sep 2026 09:38:51 +0200 Subject: [PATCH 4/4] fix: Show union variant type that does not exist in the schema --- avro/src/types.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/avro/src/types.rs b/avro/src/types.rs index 2622e354..74d07fd5 100644 --- a/avro/src/types.rs +++ b/avro/src/types.rs @@ -796,7 +796,7 @@ impl Value { .unwrap_or_else(|| { Some(format!( "{} is at position {i} but that position does not exist in {schema_path}", - ValuePath::Index(i as usize, &value_path) + ValuePath::Value(value, &ValuePath::Index(i as usize, &value_path)), )) }) } @@ -1721,7 +1721,7 @@ mod tests { ( Value::Union(2, Box::new(Value::Long(1_i64))), Schema::Union(UnionSchema::new(vec![Schema::Null, Schema::Int])?), - Some("Union[2]. is at position 2 but that position does not exist in Union"), + Some("Union[2].Long is at position 2 but that position does not exist in Union"), ), ( Value::Array(vec![Value::Long(42i64)]),