This is an automated email from the ASF dual-hosted git repository.
mgrigorov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/avro-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 08deef9 fix!: `Name` and `Alias` incorrectly implement `From<&str>`
(#423)
08deef9 is described below
commit 08deef9f70942277bcb6fad8cb4dee00217cc31e
Author: Kriskras99 <[email protected]>
AuthorDate: Fri Jan 23 12:12:12 2026 +0100
fix!: `Name` and `Alias` incorrectly implement `From<&str>` (#423)
* 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.
* feat: Implement `FromStr` for `Name` and `Alias`
---
avro/src/decode.rs | 2 +-
avro/src/encode.rs | 2 +-
avro/src/schema/mod.rs | 2 +-
avro/src/schema/name.rs | 56 +++++++++++++++++++++++++++++++++-------
avro/src/schema/record/schema.rs | 4 +--
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, 96 insertions(+), 49 deletions(-)
diff --git a/avro/src/decode.rs b/avro/src/decode.rs
index b5b828c..1adc095 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()?,
aliases: None,
doc: None,
default: None,
diff --git a/avro/src/encode.rs b/avro/src/encode.rs
index 467dd53..85c4461 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()?,
aliases: None,
doc: None,
default: None,
diff --git a/avro/src/schema/mod.rs b/avro/src/schema/mod.rs
index 4d0c8a9..cff2a2f 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")?,
aliases: None,
doc: None,
size: 12,
diff --git a/avro/src/schema/name.rs b/avro/src/schema/name.rs
index 7873dc0..624443f 100644
--- a/avro/src/schema/name.rs
+++ b/avro/src/schema/name.rs
@@ -15,14 +15,14 @@
// specific language governing permissions and limitations
// under the License.
-use std::collections::HashMap;
-use std::fmt;
-
use serde::{Deserialize, Serialize, Serializer};
use serde_json::{Map, Value};
+use std::collections::HashMap;
+use std::fmt;
+use std::str::FromStr;
use crate::{
- AvroResult, Schema,
+ AvroResult, Error, Schema,
error::Details,
util::MapHelper,
validator::{validate_namespace, validate_schema_name},
@@ -145,9 +145,27 @@ 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)
+ }
+}
+
+impl FromStr for Name {
+ type Err = Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ Self::new(s)
}
}
@@ -200,9 +218,27 @@ 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)
+ }
+}
+
+impl FromStr for Alias {
+ type Err = Error;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ Self::new(s)
}
}
diff --git a/avro/src/schema/record/schema.rs b/avro/src/schema/record/schema.rs
index e02a23e..c1c7a73 100644
--- a/avro/src/schema/record/schema.rs
+++ b/avro/src/schema/record/schema.rs
@@ -90,11 +90,11 @@ mod tests {
let record_schema = RecordSchema::builder()
.name(name.clone())
- .aliases(Some(vec!["alias_1".into()]))
+ .aliases(Some(vec!["alias_1".try_into()?]))
.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()?]));
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..3b26379 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()?)
+ .build(),
+ ),
+ Schema::Record(
+ RecordSchema::builder()
+ .name("Same_name".try_into()?)
+ .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..4e61ce7 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")?,
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")?,
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()?,
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()?,
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()?,
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);
}
