This is an automated email from the ASF dual-hosted git repository. kriskras99 pushed a commit to branch fix/name_alias_from in repository https://gitbox.apache.org/repos/asf/avro-rs.git
commit 3bda6f33de7e8c387cf7a6e243adc57f6b69eecd Author: Kriskras99 <[email protected]> AuthorDate: Thu Jan 22 21:01:38 2026 +0100 fix!: `Name` and `Alias` incorrectly implement `From<&str>` The documentation for `From` mentions the following: > The conversion is _infallible_: if the conversion can fail, use `TryFrom` instead; don’t provide a `From` impl that panics. This removes the `From` implementation and implements `TryFrom` instead. It also implements `TryFrom<String>`. I would have prefered to implement `TryFrom<T>` where `T: AsRef<str>` but that's not possible in the current type resolver. This is a **breaking** change, any user using `Name::into` or `Alias::into` will have to update their code. --- avro/src/decode.rs | 2 +- avro/src/encode.rs | 2 +- avro/src/schema/mod.rs | 2 +- avro/src/schema/name.rs | 34 +++++++++++++++++++++++++++------- avro/src/schema/record/schema.rs | 7 +++++-- avro/src/schema/union.rs | 12 ++++++++++-- avro/src/schema_equality.rs | 36 ++++++++++++++++++------------------ avro/src/types.rs | 14 +++++++------- avro/src/writer.rs | 2 +- avro_derive/src/lib.rs | 15 +++++++++------ 10 files changed, 80 insertions(+), 46 deletions(-) diff --git a/avro/src/decode.rs b/avro/src/decode.rs index b5b828c..86d3577 100644 --- a/avro/src/decode.rs +++ b/avro/src/decode.rs @@ -891,7 +891,7 @@ mod tests { let fixed = FixedSchema { size: 16, - name: "uuid".into(), + name: "uuid".try_into().expect("Name is valid"), aliases: None, doc: None, default: None, diff --git a/avro/src/encode.rs b/avro/src/encode.rs index 467dd53..f8c611e 100644 --- a/avro/src/encode.rs +++ b/avro/src/encode.rs @@ -968,7 +968,7 @@ pub(crate) mod tests { fn avro_3926_encode_decode_uuid_to_fixed_wrong_schema_size() -> TestResult { let schema = Schema::Fixed(FixedSchema { size: 15, - name: "uuid".into(), + name: "uuid".try_into().expect("Name is valid"), aliases: None, doc: None, default: None, diff --git a/avro/src/schema/mod.rs b/avro/src/schema/mod.rs index 4d0c8a9..cd1733e 100644 --- a/avro/src/schema/mod.rs +++ b/avro/src/schema/mod.rs @@ -6269,7 +6269,7 @@ mod tests { #[test] fn avro_rs_382_serialize_duration_schema() -> TestResult { let schema = Schema::Duration(FixedSchema { - name: Name::from("Duration"), + name: Name::try_from("Duration").expect("Name is valid"), aliases: None, doc: None, size: 12, diff --git a/avro/src/schema/name.rs b/avro/src/schema/name.rs index 7873dc0..0b09876 100644 --- a/avro/src/schema/name.rs +++ b/avro/src/schema/name.rs @@ -22,7 +22,7 @@ use serde::{Deserialize, Serialize, Serializer}; use serde_json::{Map, Value}; use crate::{ - AvroResult, Schema, + AvroResult, Error, Schema, error::Details, util::MapHelper, validator::{validate_namespace, validate_schema_name}, @@ -145,9 +145,19 @@ impl Name { } } -impl From<&str> for Name { - fn from(name: &str) -> Self { - Name::new(name).unwrap() +impl TryFrom<&str> for Name { + type Error = Error; + + fn try_from(value: &str) -> Result<Self, Self::Error> { + Self::new(value) + } +} + +impl TryFrom<String> for Name { + type Error = Error; + + fn try_from(value: String) -> Result<Self, Self::Error> { + Self::new(&value) } } @@ -200,9 +210,19 @@ impl Alias { } } -impl From<&str> for Alias { - fn from(name: &str) -> Self { - Alias::new(name).unwrap() +impl TryFrom<&str> for Alias { + type Error = Error; + + fn try_from(value: &str) -> Result<Self, Self::Error> { + Self::new(value) + } +} + +impl TryFrom<String> for Alias { + type Error = Error; + + fn try_from(value: String) -> Result<Self, Self::Error> { + Self::new(&value) } } diff --git a/avro/src/schema/record/schema.rs b/avro/src/schema/record/schema.rs index e02a23e..2786328 100644 --- a/avro/src/schema/record/schema.rs +++ b/avro/src/schema/record/schema.rs @@ -90,11 +90,14 @@ mod tests { let record_schema = RecordSchema::builder() .name(name.clone()) - .aliases(Some(vec!["alias_1".into()])) + .aliases(Some(vec!["alias_1".try_into().expect("Alias is valid")])) .build(); assert_eq!(record_schema.name, name); - assert_eq!(record_schema.aliases, Some(vec!["alias_1".into()])); + assert_eq!( + record_schema.aliases, + Some(vec!["alias_1".try_into().expect("Alias is valid")]) + ); assert_eq!(record_schema.doc, None); assert_eq!(record_schema.fields.len(), 0); assert_eq!(record_schema.lookup.len(), 0); diff --git a/avro/src/schema/union.rs b/avro/src/schema/union.rs index a7a869b..b49c727 100644 --- a/avro/src/schema/union.rs +++ b/avro/src/schema/union.rs @@ -151,8 +151,16 @@ mod tests { #[test] fn avro_rs_402_new_union_schema_duplicate_names() -> TestResult { let res = UnionSchema::new(vec![ - Schema::Record(RecordSchema::builder().name("Same_name".into()).build()), - Schema::Record(RecordSchema::builder().name("Same_name".into()).build()), + Schema::Record( + RecordSchema::builder() + .name("Same_name".try_into().expect("Name is valid")) + .build(), + ), + Schema::Record( + RecordSchema::builder() + .name("Same_name".try_into().expect("Name is valid")) + .build(), + ), ]) .map_err(Error::into_details); diff --git a/avro/src/schema_equality.rs b/avro/src/schema_equality.rs index 1811ba5..cd1c2cd 100644 --- a/avro/src/schema_equality.rs +++ b/avro/src/schema_equality.rs @@ -273,7 +273,7 @@ mod tests { #[test] fn avro_rs_382_compare_schemata_duration_equal() { let schema_one = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -281,7 +281,7 @@ mod tests { attributes: BTreeMap::new(), }); let schema_two = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -296,7 +296,7 @@ mod tests { #[test] fn avro_rs_382_compare_schemata_duration_different_names() { let schema_one = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -304,7 +304,7 @@ mod tests { attributes: BTreeMap::new(), }); let schema_two = Schema::Duration(FixedSchema { - name: Name::from("name2"), + name: Name::try_from("name2").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -321,7 +321,7 @@ mod tests { #[test] fn avro_rs_382_compare_schemata_duration_different_attributes() { let schema_one = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -331,7 +331,7 @@ mod tests { .collect(), }); let schema_two = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -352,7 +352,7 @@ mod tests { #[test] fn avro_rs_382_compare_schemata_duration_different_sizes() { let schema_one = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 8, aliases: None, doc: None, @@ -360,7 +360,7 @@ mod tests { attributes: BTreeMap::new(), }); let schema_two = Schema::Duration(FixedSchema { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), size: 12, aliases: None, doc: None, @@ -377,11 +377,11 @@ mod tests { #[test] fn test_avro_3939_compare_named_schemata_with_different_names() { let schema_one = Schema::Ref { - name: Name::from("name1"), + name: Name::try_from("name1").expect("Name is valid"), }; let schema_two = Schema::Ref { - name: Name::from("name2"), + name: Name::try_from("name2").expect("Name is valid"), }; let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); @@ -496,7 +496,7 @@ mod tests { #[test] fn test_avro_3939_compare_fixed_schemata() { let schema_one = Schema::Fixed(FixedSchema { - name: Name::from("fixed"), + name: Name::try_from("fixed").expect("Name is valid"), doc: None, size: 10, default: None, @@ -507,7 +507,7 @@ mod tests { assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); let schema_two = Schema::Fixed(FixedSchema { - name: Name::from("fixed"), + name: Name::try_from("fixed").expect("Name is valid"), doc: None, size: 10, default: None, @@ -531,7 +531,7 @@ mod tests { #[test] fn test_avro_3939_compare_enum_schemata() { let schema_one = Schema::Enum(EnumSchema { - name: Name::from("enum"), + name: Name::try_from("enum").expect("Name is valid"), doc: None, symbols: vec!["A".to_string(), "B".to_string()], default: None, @@ -542,7 +542,7 @@ mod tests { assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); let schema_two = Schema::Enum(EnumSchema { - name: Name::from("enum"), + name: Name::try_from("enum").expect("Name is valid"), doc: None, symbols: vec!["A".to_string(), "B".to_string()], default: None, @@ -566,13 +566,13 @@ mod tests { #[test] fn test_avro_3939_compare_ref_schemata() { let schema_one = Schema::Ref { - name: Name::from("ref"), + name: Name::try_from("ref").expect("Name is valid"), }; assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); let schema_two = Schema::Ref { - name: Name::from("ref"), + name: Name::try_from("ref").expect("Name is valid"), }; let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); @@ -591,7 +591,7 @@ mod tests { #[test] fn test_avro_3939_compare_record_schemata() { let schema_one = Schema::Record(RecordSchema { - name: Name::from("record"), + name: Name::try_from("record").expect("Name is valid"), doc: None, fields: vec![RecordField { name: "field".to_string(), @@ -611,7 +611,7 @@ mod tests { assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); let schema_two = Schema::Record(RecordSchema { - name: Name::from("record"), + name: Name::try_from("record").expect("Name is valid"), doc: None, fields: vec![RecordField { name: "field".to_string(), diff --git a/avro/src/types.rs b/avro/src/types.rs index 4c659c5..85841bc 100644 --- a/avro/src/types.rs +++ b/avro/src/types.rs @@ -1356,7 +1356,7 @@ mod tests { ( Value::Fixed(12, vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]), Schema::Duration(FixedSchema { - name: Name::from("TestName"), + name: Name::try_from("TestName").expect("Name is valid"), aliases: None, doc: None, size: 12, @@ -1369,7 +1369,7 @@ mod tests { ( Value::Fixed(11, vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]), Schema::Duration(FixedSchema { - name: Name::from("TestName"), + name: Name::try_from("TestName").expect("Name is valid"), aliases: None, doc: None, size: 12, @@ -1924,7 +1924,7 @@ Field with name '"b"' is not a member of the map items"#, value .clone() .resolve(&Schema::Duration(FixedSchema { - name: Name::from("TestName"), + name: Name::try_from("TestName").expect("Name is valid"), aliases: None, doc: None, size: 12, @@ -1937,7 +1937,7 @@ Field with name '"b"' is not a member of the map items"#, assert!( Value::Long(1i64) .resolve(&Schema::Duration(FixedSchema { - name: Name::from("TestName"), + name: Name::try_from("TestName").expect("Name is valid"), aliases: None, doc: None, size: 12, @@ -3187,7 +3187,7 @@ Field with name '"b"' is not a member of the map items"#, let value = Value::Bytes(vec![97, 98, 99]); assert_eq!( value.resolve(&Schema::Fixed(FixedSchema { - name: "test".into(), + name: "test".try_into().expect("Name is valid"), aliases: None, doc: None, size: 3, @@ -3201,7 +3201,7 @@ Field with name '"b"' is not a member of the map items"#, assert!( value .resolve(&Schema::Fixed(FixedSchema { - name: "test".into(), + name: "test".try_into().expect("Name is valid"), aliases: None, doc: None, size: 3, @@ -3215,7 +3215,7 @@ Field with name '"b"' is not a member of the map items"#, assert!( value .resolve(&Schema::Fixed(FixedSchema { - name: "test".into(), + name: "test".try_into().expect("Name is valid"), aliases: None, doc: None, size: 3, diff --git a/avro/src/writer.rs b/avro/src/writer.rs index 1c375d9..5573e25 100644 --- a/avro/src/writer.rs +++ b/avro/src/writer.rs @@ -1048,7 +1048,7 @@ mod tests { logical_type_test( r#"{"type": {"type": "fixed", "name": "duration", "size": 12}, "logicalType": "duration"}"#, &Schema::Duration(FixedSchema { - name: Name::from("duration"), + name: Name::try_from("duration").expect("Name is valid"), aliases: None, doc: None, size: 12, diff --git a/avro_derive/src/lib.rs b/avro_derive/src/lib.rs index 12551f8..8c49d05 100644 --- a/avro_derive/src/lib.rs +++ b/avro_derive/src/lib.rs @@ -176,7 +176,7 @@ fn get_struct_schema_def( } None => quote! { None }, }; - let aliases = preserve_vec(&field_attrs.alias); + let aliases = aliases(&field_attrs.alias); let schema_expr = get_field_schema_expr(&field, field_attrs.with)?; record_field_exprs.push(quote! { schema_fields.push(::apache_avro::schema::RecordField { @@ -207,7 +207,7 @@ fn get_struct_schema_def( } let record_doc = preserve_optional(container_attrs.doc.as_ref()); - let record_aliases = preserve_vec(&container_attrs.aliases); + let record_aliases = aliases(&container_attrs.aliases); let full_schema_name = &container_attrs.name; // When flatten is involved, there will be more but we don't know how many. This optimises for @@ -309,7 +309,7 @@ fn get_data_enum_schema_def( ident_span: Span, ) -> Result<TokenStream, Vec<syn::Error>> { let doc = preserve_optional(container_attrs.doc.as_ref()); - let enum_aliases = preserve_vec(&container_attrs.aliases); + let enum_aliases = aliases(&container_attrs.aliases); if data_enum.variants.iter().all(|v| Fields::Unit == v.fields) { let default_value = default_enum_variant(&data_enum, ident_span)?; let default = preserve_optional(default_value); @@ -409,8 +409,11 @@ fn preserve_optional(op: Option<impl quote::ToTokens>) -> TokenStream { } } -fn preserve_vec(op: &[impl quote::ToTokens]) -> TokenStream { - let items: Vec<TokenStream> = op.iter().map(|tt| quote! {#tt.into()}).collect(); +fn aliases(op: &[impl quote::ToTokens]) -> TokenStream { + let items: Vec<TokenStream> = op + .iter() + .map(|tt| quote! {#tt.try_into().expect("Alias is invalid")}) + .collect(); if items.is_empty() { quote! {None} } else { @@ -687,7 +690,7 @@ mod tests { match syn::parse2::<DeriveInput>(test_struct) { Ok(input) => { let schema_res = derive_avro_schema(input); - let expected_token_stream = r#"# [automatically_derived] impl apache_avro :: AvroSchemaComponent for A { fn get_schema_in_ctxt (named_schemas : & mut apache_avro :: schema :: Names , enclosing_namespace : & Option < String >) -> apache_avro :: schema :: Schema { let name = apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qualified_name (enclosing_namespace) ; if named_schemas . contains_key (& name) { apache_avr [...] + let expected_token_stream = r#"# [automatically_derived] impl apache_avro :: AvroSchemaComponent for A { fn get_schema_in_ctxt (named_schemas : & mut apache_avro :: schema :: Names , enclosing_namespace : & Option < String >) -> apache_avro :: schema :: Schema { let name = apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qualified_name (enclosing_namespace) ; if named_schemas . contains_key (& name) { apache_avr [...] let schema_token_stream = schema_res.unwrap().to_string(); assert_eq!(schema_token_stream, expected_token_stream); }
