This is an automated email from the ASF dual-hosted git repository. kriskras99 pushed a commit to branch feat/better_record_builder in repository https://gitbox.apache.org/repos/asf/avro-rs.git
commit ecf6bbc98fe273e05f4542d11a39ed4b61a5070f Author: default <[email protected]> AuthorDate: Fri Feb 27 15:07:10 2026 +0000 temp --- avro/src/encode.rs | 9 +- avro/src/schema/mod.rs | 331 +++++++++++++-------------------------- avro/src/schema/parser.rs | 15 +- avro/src/schema/record/field.rs | 84 ++++------ avro/src/schema/record/mod.rs | 2 +- avro/src/schema_compatibility.rs | 2 +- avro/src/schema_equality.rs | 10 +- avro/src/serde/derive.rs | 100 +++--------- avro/src/serde/ser_schema/mod.rs | 104 ++++++------ avro/src/types.rs | 52 ++---- avro/src/writer.rs | 2 +- avro/tests/get_record_fields.rs | 54 +------ avro/tests/schema.rs | 2 - avro_derive/src/lib.rs | 52 +++--- avro_derive/tests/derive.rs | 34 +--- 15 files changed, 279 insertions(+), 574 deletions(-) diff --git a/avro/src/encode.rs b/avro/src/encode.rs index 610e1b8..656509b 100644 --- a/avro/src/encode.rs +++ b/avro/src/encode.rs @@ -301,11 +301,10 @@ pub(crate) fn encode_internal<W: Write, S: Borrow<Schema>>( for schema_field in schema_fields.iter() { let name = &schema_field.name; let value_opt = lookup.get(name).or_else(|| { - if let Some(aliases) = &schema_field.aliases { - aliases.iter().find_map(|alias| lookup.get(alias)) - } else { - None - } + schema_field + .aliases + .iter() + .find_map(|alias| lookup.get(alias)) }); if let Some(value) = value_opt { diff --git a/avro/src/schema/mod.rs b/avro/src/schema/mod.rs index 70d3066..a0f3077 100644 --- a/avro/src/schema/mod.rs +++ b/avro/src/schema/mod.rs @@ -29,9 +29,7 @@ pub(crate) use crate::schema::resolve::{ }; pub use crate::schema::{ name::{Alias, Aliases, Name, Names, NamesRef, Namespace}, - record::{ - RecordField, RecordFieldBuilder, RecordFieldOrder, RecordSchema, RecordSchemaBuilder, - }, + record::{RecordField, RecordFieldBuilder, RecordSchema, RecordSchemaBuilder}, resolve::ResolvedSchema, union::{UnionSchema, UnionSchemaBuilder}, }; @@ -1340,16 +1338,12 @@ mod tests { name: Name::new("A")?, aliases: None, doc: None, - fields: vec![RecordField { - name: "field_one".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Float, - order: RecordFieldOrder::Ignore, - position: 0, - custom_attributes: Default::default(), - }], + fields: vec![ + RecordField::builder() + .name("field_one".to_string()) + .schema(Schema::Float) + .build(), + ], lookup: BTreeMap::from_iter(vec![("field_one".to_string(), 0)]), attributes: Default::default(), }); @@ -1358,16 +1352,12 @@ mod tests { name: Name::new("B")?, aliases: None, doc: None, - fields: vec![RecordField { - name: "field_one".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Float, - order: RecordFieldOrder::Ignore, - position: 0, - custom_attributes: Default::default(), - }], + fields: vec![ + RecordField::builder() + .name("field_one".to_string()) + .schema(Schema::Float) + .build(), + ], lookup: BTreeMap::from_iter(vec![("field_one".to_string(), 0)]), attributes: Default::default(), }); @@ -1512,21 +1502,18 @@ mod tests { name: Name::new("OptionA")?, aliases: None, doc: None, - fields: vec![RecordField { - name: "field_one".to_string(), - doc: None, - default: Some(JsonValue::Null), - aliases: None, - schema: Schema::Union(UnionSchema::new(vec![ - Schema::Null, - Schema::Ref { - name: Name::new("A")?, - }, - ])?), - order: RecordFieldOrder::Ignore, - position: 0, - custom_attributes: Default::default(), - }], + fields: vec![ + RecordField::builder() + .name("field_one".to_string()) + .default(JsonValue::Null) + .schema(Schema::Union(UnionSchema::new(vec![ + Schema::Null, + Schema::Ref { + name: Name::new("A")?, + }, + ])?)) + .build(), + ], lookup: BTreeMap::from_iter(vec![("field_one".to_string(), 0)]), attributes: Default::default(), }); @@ -1560,26 +1547,15 @@ mod tests { aliases: None, doc: None, fields: vec![ - RecordField { - name: "a".to_string(), - doc: None, - default: Some(JsonValue::Number(42i64.into())), - aliases: None, - schema: Schema::Long, - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }, - RecordField { - name: "b".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::String, - order: RecordFieldOrder::Ascending, - position: 1, - custom_attributes: Default::default(), - }, + RecordField::builder() + .name("a".to_string()) + .default(JsonValue::Number(42i64.into())) + .schema(Schema::Long) + .build(), + RecordField::builder() + .name("b".to_string()) + .schema(Schema::String) + .build(), ], lookup, attributes: Default::default(), @@ -1623,47 +1599,33 @@ mod tests { name: Name::new("test")?, aliases: None, doc: None, - fields: vec![RecordField { - name: "recordField".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Record(RecordSchema { - name: Name::new("Node")?, - aliases: None, - doc: None, - fields: vec![ - RecordField { - name: "label".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::String, - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }, - RecordField { - name: "children".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::array(Schema::Ref { - name: Name::new("Node")?, - }) - .build(), - order: RecordFieldOrder::Ascending, - position: 1, - custom_attributes: Default::default(), - }, - ], - lookup: node_lookup, - attributes: Default::default(), - }), - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }], + fields: vec![ + RecordField::builder() + .name("recordField".to_string()) + .schema(Schema::Record(RecordSchema { + name: Name::new("Node")?, + aliases: None, + doc: None, + fields: vec![ + RecordField::builder() + .name("label".to_string()) + .schema(Schema::String) + .build(), + RecordField::builder() + .name("children".to_string()) + .schema( + Schema::array(Schema::Ref { + name: Name::new("Node")?, + }) + .build(), + ) + .build(), + ], + lookup: node_lookup, + attributes: Default::default(), + })) + .build(), + ], lookup, attributes: Default::default(), }); @@ -1808,22 +1770,13 @@ mod tests { aliases: Some(vec![Alias::new("LinkedLongs").unwrap()]), doc: None, fields: vec![ - RecordField { - name: "value".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Long, - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }, - RecordField { - name: "next".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Union(UnionSchema::new(vec![ + RecordField::builder() + .name("value".to_string()) + .schema(Schema::Long) + .build(), + RecordField::builder() + .name("next".to_string()) + .schema(Schema::Union(UnionSchema::new(vec![ Schema::Null, Schema::Ref { name: Name { @@ -1831,11 +1784,8 @@ mod tests { namespace: None, }, }, - ])?), - order: RecordFieldOrder::Ascending, - position: 1, - custom_attributes: Default::default(), - }, + ])?)) + .build(), ], lookup, attributes: Default::default(), @@ -1876,31 +1826,19 @@ mod tests { aliases: None, doc: None, fields: vec![ - RecordField { - name: "value".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Long, - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }, - RecordField { - name: "next".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Ref { + RecordField::builder() + .name("value".to_string()) + .schema(Schema::Long) + .build(), + RecordField::builder() + .name("next".to_string()) + .schema(Schema::Ref { name: Name { name: "record".to_owned(), namespace: None, }, - }, - order: RecordFieldOrder::Ascending, - position: 1, - custom_attributes: Default::default(), - }, + }) + .build(), ], lookup, attributes: Default::default(), @@ -1948,12 +1886,9 @@ mod tests { aliases: None, doc: None, fields: vec![ - RecordField { - name: "enum".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Enum( + RecordField::builder() + .name("enum".to_string()) + .schema(Schema::Enum( EnumSchema::builder() .name(Name::new("enum")?) .symbols(vec![ @@ -1962,23 +1897,14 @@ mod tests { "three".to_string(), ]) .build(), - ), - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }, - RecordField { - name: "next".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Ref { + )) + .build(), + RecordField::builder() + .name("next".to_string()) + .schema(Schema::Ref { name: Name::new("enum")?, - }, - order: RecordFieldOrder::Ascending, - position: 1, - custom_attributes: Default::default(), - }, + }) + .build(), ], lookup, attributes: Default::default(), @@ -2026,12 +1952,9 @@ mod tests { aliases: None, doc: None, fields: vec![ - RecordField { - name: "fixed".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Fixed(FixedSchema { + RecordField::builder() + .name("fixed".to_string()) + .schema(Schema::Fixed(FixedSchema { name: Name { name: "fixed".to_owned(), namespace: None, @@ -2040,23 +1963,14 @@ mod tests { doc: None, size: 456, attributes: Default::default(), - }), - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: Default::default(), - }, - RecordField { - name: "next".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Ref { + })) + .build(), + RecordField::builder() + .name("next".to_string()) + .schema(Schema::Ref { name: Name::new("fixed")?, - }, - order: RecordFieldOrder::Ascending, - position: 1, - custom_attributes: Default::default(), - }, + }) + .build(), ], lookup, attributes: Default::default(), @@ -2267,27 +2181,6 @@ mod tests { Ok(()) } - #[test] - fn record_field_order_from_str() -> TestResult { - use std::str::FromStr; - - assert_eq!( - RecordFieldOrder::from_str("ascending").unwrap(), - RecordFieldOrder::Ascending - ); - assert_eq!( - RecordFieldOrder::from_str("descending").unwrap(), - RecordFieldOrder::Descending - ); - assert_eq!( - RecordFieldOrder::from_str("ignore").unwrap(), - RecordFieldOrder::Ignore - ); - assert!(RecordFieldOrder::from_str("not an ordering").is_err()); - - Ok(()) - } - #[test] fn test_avro_3374_preserve_namespace_for_primitive() -> TestResult { let schema = Schema::parse_str( @@ -2514,7 +2407,7 @@ mod tests { { "name": "time", "type": "long", - "doc": "The documentation is not serialized", + "doc": "The documentation is serialized", "default": 123, "aliases": ["time1", "ns.time2"] } @@ -2526,7 +2419,7 @@ mod tests { let value = serde_json::to_value(&schema)?; let serialized = serde_json::to_string(&value)?; assert_eq!( - r#"{"aliases":["space.b","x.y","c"],"fields":[{"aliases":["time1","ns.time2"],"default":123,"name":"time","type":"long"}],"name":"a","namespace":"space","type":"record"}"#, + r#"{"aliases":["space.b","x.y","c"],"fields":[{"aliases":["time1","ns.time2"],"default":123,"doc":"The documentation is serialized","name":"time","type":"long"}],"name":"a","namespace":"space","type":"record"}"#, &serialized ); assert_eq!(schema, Schema::parse_str(&serialized)?); @@ -2962,7 +2855,10 @@ mod tests { if let Schema::Record(RecordSchema { fields, .. }) = schema { let num_field = &fields[0]; assert_eq!(num_field.name, "num"); - assert_eq!(num_field.aliases, Some(vec!("num1".into(), "num2".into()))); + assert_eq!( + num_field.aliases, + vec!["num1".to_string(), "num2".to_string()] + ); } else { panic!("Expected a record schema!"); } @@ -4597,16 +4493,13 @@ mod tests { }, aliases: Some(vec![Alias::new("LinkedLongs").unwrap()]), doc: None, - fields: vec![RecordField { - name: "value".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Long, - order: RecordFieldOrder::Ascending, - position: 0, - custom_attributes: BTreeMap::from([("field-id".to_string(), 1.into())]), - }], + fields: vec![ + RecordField::builder() + .name("value".to_string()) + .schema(Schema::Long) + .custom_attributes(BTreeMap::from([("field-id".to_string(), 1.into())])) + .build(), + ], lookup, attributes: BTreeMap::from([("custom-attribute".to_string(), "value".into())]), }) diff --git a/avro/src/schema/parser.rs b/avro/src/schema/parser.rs index f896e17..6011d82 100644 --- a/avro/src/schema/parser.rs +++ b/avro/src/schema/parser.rs @@ -560,22 +560,17 @@ impl Parser { fields .iter() .filter_map(|field| field.as_object()) - .enumerate() - .map(|(position, field)| { - RecordField::parse(field, position, self, &fully_qualified_name) - }) + .map(|field| RecordField::parse(field, self, &fully_qualified_name)) .collect::<Result<_, _>>() })?; - for field in &fields { - if let Some(_old) = lookup.insert(field.name.clone(), field.position) { + for (position, field) in fields.iter().enumerate() { + if let Some(_old) = lookup.insert(field.name.clone(), position) { return Err(Details::FieldNameDuplicate(field.name.clone()).into()); } - if let Some(ref field_aliases) = field.aliases { - for alias in field_aliases { - lookup.insert(alias.clone(), field.position); - } + for alias in &field.aliases { + lookup.insert(alias.clone(), position); } } diff --git a/avro/src/schema/record/field.rs b/avro/src/schema/record/field.rs index cb1b6dd..043a51c 100644 --- a/avro/src/schema/record/field.rs +++ b/avro/src/schema/record/field.rs @@ -27,8 +27,6 @@ use serde::{Serialize, Serializer}; use serde_json::{Map, Value}; use std::collections::BTreeMap; use std::fmt::{Debug, Formatter}; -use std::str::FromStr; -use strum::EnumString; /// Represents a `field` in a `record` Avro schema. #[derive(bon::Builder, Clone, PartialEq)] @@ -40,21 +38,14 @@ pub struct RecordField { #[builder(default)] pub doc: Documentation, /// Aliases of the field's name. They have no namespace. - pub aliases: Option<Vec<String>>, + #[builder(default)] + pub aliases: Vec<String>, /// Default value of the field. /// This value will be used when reading Avro datum if schema resolution /// is enabled. pub default: Option<Value>, /// Schema of the field. pub schema: Schema, - /// Order of the field. - /// - /// **NOTE** This currently has no effect. - #[builder(default = RecordFieldOrder::Ignore)] - pub order: RecordFieldOrder, - /// Position of the field in the list of `field` of its parent `Schema` - #[builder(default)] - pub position: usize, /// A collection of all unknown fields in the record field. #[builder(default = BTreeMap::new())] pub custom_attributes: BTreeMap<String, Value>, @@ -67,38 +58,32 @@ impl Debug for RecordField { if let Some(doc) = &self.doc { debug.field("doc", &doc); } - if let Some(aliases) = &self.aliases { - debug.field("aliases", &aliases); + if !self.aliases.is_empty() { + debug.field("aliases", &self.aliases); } if let Some(default) = &self.default { debug.field("default", &default); } debug.field("schema", &self.schema); - // This field is ignored as it currently has no effect - // debug.field("order", &self.order); - debug.field("position", &self.position); if !self.custom_attributes.is_empty() { debug.field("custom_attributes", &self.custom_attributes); } - // As we are always skipping self.order, always show the .. - debug.finish_non_exhaustive() + if self.doc.is_none() + || !self.aliases.is_empty() + || self.default.is_none() + || self.custom_attributes.is_empty() + { + debug.finish_non_exhaustive() + } else { + debug.finish() + } } } -/// Represents any valid order for a `field` in a `record` Avro schema. -#[derive(Clone, Debug, Eq, PartialEq, EnumString)] -#[strum(serialize_all = "kebab_case")] -pub enum RecordFieldOrder { - Ascending, - Descending, - Ignore, -} - impl RecordField { /// Parse a `serde_json::Value` into a `RecordField`. pub(crate) fn parse( field: &Map<String, Value>, - position: usize, parser: &mut Parser, enclosing_record: &Name, ) -> AvroResult<Self> { @@ -124,29 +109,24 @@ impl RecordField { &default, )?; - let aliases = field.get("aliases").and_then(|aliases| { - aliases.as_array().map(|aliases| { - aliases - .iter() - .flat_map(|alias| alias.as_str()) - .map(|alias| alias.to_string()) - .collect::<Vec<String>>() + let aliases = field + .get("aliases") + .and_then(|aliases| { + aliases.as_array().map(|aliases| { + aliases + .iter() + .flat_map(|alias| alias.as_str()) + .map(|alias| alias.to_string()) + .collect::<Vec<String>>() + }) }) - }); - - let order = field - .get("order") - .and_then(|order| order.as_str()) - .and_then(|order| RecordFieldOrder::from_str(order).ok()) - .unwrap_or(RecordFieldOrder::Ascending); + .unwrap_or_default(); Ok(RecordField { name, doc: field.doc(), default, aliases, - order, - position, custom_attributes: RecordField::get_field_custom_attributes(field), schema, }) @@ -211,7 +191,7 @@ impl RecordField { let mut custom_attributes: BTreeMap<String, Value> = BTreeMap::new(); for (key, value) in field { match key.as_str() { - "type" | "name" | "doc" | "default" | "order" | "aliases" => continue, + "type" | "name" | "doc" | "default" | "aliases" => continue, _ => custom_attributes.insert(key.clone(), value.clone()), }; } @@ -236,12 +216,16 @@ impl Serialize for RecordField { map.serialize_entry("name", &self.name)?; map.serialize_entry("type", &self.schema)?; - if let Some(ref default) = self.default { + if let Some(default) = &self.default { map.serialize_entry("default", default)?; } - if let Some(ref aliases) = self.aliases { - map.serialize_entry("aliases", aliases)?; + if let Some(doc) = &self.doc { + map.serialize_entry("doc", doc)?; + } + + if !self.aliases.is_empty() { + map.serialize_entry("aliases", &self.aliases)?; } for attr in &self.custom_attributes { @@ -272,8 +256,6 @@ mod tests { }, }, ])?)) - .order(RecordFieldOrder::Ascending) - .position(1) .build(); assert!(nullable_record_field.is_nullable()); @@ -282,8 +264,6 @@ mod tests { .name("next".to_string()) .default(json!(2)) .schema(Schema::Long) - .order(RecordFieldOrder::Ascending) - .position(1) .build(); assert!(!non_nullable_record_field.is_nullable()); diff --git a/avro/src/schema/record/mod.rs b/avro/src/schema/record/mod.rs index 080d9fe..61da1e7 100644 --- a/avro/src/schema/record/mod.rs +++ b/avro/src/schema/record/mod.rs @@ -16,7 +16,7 @@ // under the License. mod field; -pub use field::{RecordField, RecordFieldBuilder, RecordFieldOrder}; +pub use field::{RecordField, RecordFieldBuilder}; mod schema; pub use schema::{RecordSchema, RecordSchemaBuilder}; diff --git a/avro/src/schema_compatibility.rs b/avro/src/schema_compatibility.rs index 4cc8c2f..b172d94 100644 --- a/avro/src/schema_compatibility.rs +++ b/avro/src/schema_compatibility.rs @@ -404,7 +404,7 @@ impl Checker { // are not allowed to match on writer aliases. // Search using field name and *after* that aliases. if let Some(w_field) = once(&r_field.name) - .chain(r_field.aliases.as_deref().unwrap_or(&[]).iter()) + .chain(r_field.aliases.iter()) .find_map(|ra| w_fields.iter().find(|wf| &wf.name == ra)) { // Check that the schemas are compatible diff --git a/avro/src/schema_equality.rs b/avro/src/schema_equality.rs index f0b8ce5..08a99be 100644 --- a/avro/src/schema_equality.rs +++ b/avro/src/schema_equality.rs @@ -263,7 +263,7 @@ pub(crate) fn compare_schemata(schema_one: &Schema, schema_two: &Schema) -> bool #[allow(non_snake_case)] mod tests { use super::*; - use crate::schema::{InnerDecimalSchema, Name, RecordFieldOrder}; + use crate::schema::{InnerDecimalSchema, Name}; use apache_avro_test_helper::TestResult; use serde_json::Value; use std::collections::BTreeMap; @@ -634,10 +634,8 @@ mod tests { doc: None, default: None, schema: Schema::Boolean, - order: RecordFieldOrder::Ignore, - aliases: None, + aliases: Vec::new(), custom_attributes: BTreeMap::new(), - position: 0, }], aliases: None, attributes: BTreeMap::new(), @@ -654,10 +652,8 @@ mod tests { doc: None, default: None, schema: Schema::Boolean, - order: RecordFieldOrder::Ignore, - aliases: None, + aliases: Vec::new(), custom_attributes: BTreeMap::new(), - position: 0, }], aliases: None, attributes: BTreeMap::new(), diff --git a/avro/src/serde/derive.rs b/avro/src/serde/derive.rs index e9b782a..e01f112 100644 --- a/avro/src/serde/derive.rs +++ b/avro/src/serde/derive.rs @@ -232,7 +232,7 @@ pub trait AvroSchema { /// Schema::Int /// } /// -/// fn get_record_fields_in_ctxt(_: usize, _: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { +/// fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { /// None // A Schema::Int is not a Schema::Record so there are no fields to return /// } /// @@ -259,8 +259,8 @@ pub trait AvroSchema { /// T::get_schema_in_ctxt(named_schemas, enclosing_namespace) /// } /// -/// fn get_record_fields_in_ctxt(first_field_position: usize, named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Option<Vec<RecordField>> { -/// T::get_record_fields_in_ctxt(first_field_position, named_schemas, enclosing_namespace) +/// fn get_record_fields_in_ctxt(named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Option<Vec<RecordField>> { +/// T::get_record_fields_in_ctxt(named_schemas, enclosing_namespace) /// } /// /// fn field_default() -> Option<serde_json::Value> { @@ -304,29 +304,26 @@ pub trait AvroSchema { /// named_schemas.insert(name.clone()); /// let schema = Schema::Record(RecordSchema::builder() /// .name(name.clone()) -/// .fields(Self::get_record_fields_in_ctxt(0, named_schemas, enclosing_namespace).expect("Impossible!")) +/// .fields(Self::get_record_fields_in_ctxt(named_schemas, enclosing_namespace).expect("Impossible!")) /// .build() /// ); /// schema /// } /// } /// -/// fn get_record_fields_in_ctxt(first_field_position: usize, named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Option<Vec<RecordField>> { +/// fn get_record_fields_in_ctxt(named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Option<Vec<RecordField>> { /// Some(vec![ /// RecordField::builder() /// .name("one") /// .schema(String::get_schema_in_ctxt(named_schemas, enclosing_namespace)) -/// .position(first_field_position) /// .build(), /// RecordField::builder() /// .name("two") /// .schema(i32::get_schema_in_ctxt(named_schemas, enclosing_namespace)) -/// .position(first_field_position+1) /// .build(), /// RecordField::builder() /// .name("three") /// .schema(<Option<Duration>>::get_schema_in_ctxt(named_schemas, enclosing_namespace)) -/// .position(first_field_position+2) /// .build(), /// ]) /// } @@ -351,16 +348,10 @@ pub trait AvroSchemaComponent { /// The default implementation has to do a lot of extra work, so it is strongly recommended to /// implement this function when manually implementing this trait. fn get_record_fields_in_ctxt( - first_field_position: usize, named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace, ) -> Option<Vec<RecordField>> { - get_record_fields_in_ctxt( - first_field_position, - named_schemas, - enclosing_namespace, - Self::get_schema_in_ctxt, - ) + get_record_fields_in_ctxt(named_schemas, enclosing_namespace, Self::get_schema_in_ctxt) } /// The default value of this type when used for a record field. @@ -378,7 +369,6 @@ pub trait AvroSchemaComponent { /// /// This is public so the derive macro can use it for `#[avro(with = ||)]` and `#[avro(with = path)]` pub fn get_record_fields_in_ctxt( - first_field_position: usize, named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace, schema_fn: fn(named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Schema, @@ -401,15 +391,7 @@ pub fn get_record_fields_in_ctxt( let Schema::Record(record) = schema else { return None; }; - let fields = record - .fields - .into_iter() - .map(|mut f| { - f.position += first_field_position; - f - }) - .collect(); - return Some(fields); + return Some(record.fields); } _ => return None, }; @@ -462,8 +444,6 @@ pub fn get_record_fields_in_ctxt( } else { field.schema.clone() }, - order: field.order.clone(), - position: field.position, custom_attributes: field.custom_attributes.clone(), }) .collect(); @@ -497,15 +477,7 @@ pub fn get_record_fields_in_ctxt( } } - let fields = record - .fields - .into_iter() - .map(|mut f| { - f.position += first_field_position; - f - }) - .collect(); - Some(fields) + Some(record.fields) } impl<T> AvroSchema for T @@ -524,7 +496,7 @@ macro_rules! impl_schema ( $variant_constructor } - fn get_record_fields_in_ctxt(_: usize, _: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -553,8 +525,8 @@ macro_rules! impl_passthrough_schema ( T::get_schema_in_ctxt(named_schemas, enclosing_namespace) } - fn get_record_fields_in_ctxt(first_field_position: usize, named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Option<Vec<RecordField>> { - T::get_record_fields_in_ctxt(first_field_position, named_schemas, enclosing_namespace) + fn get_record_fields_in_ctxt(named_schemas: &mut HashSet<Name>, enclosing_namespace: &Namespace) -> Option<Vec<RecordField>> { + T::get_record_fields_in_ctxt(named_schemas, enclosing_namespace) } fn field_default() -> Option<serde_json::Value> { @@ -577,7 +549,7 @@ macro_rules! impl_array_schema ( Schema::array(T::get_schema_in_ctxt(named_schemas, enclosing_namespace)).build() } - fn get_record_fields_in_ctxt(_: usize, _: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -600,11 +572,7 @@ where Schema::array(T::get_schema_in_ctxt(named_schemas, enclosing_namespace)).build() } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -620,11 +588,7 @@ where Schema::map(T::get_schema_in_ctxt(named_schemas, enclosing_namespace)).build() } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -647,11 +611,7 @@ where ) } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } @@ -686,11 +646,7 @@ impl AvroSchemaComponent for core::time::Duration { } } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -721,11 +677,7 @@ impl AvroSchemaComponent for uuid::Uuid { } } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -754,11 +706,7 @@ impl AvroSchemaComponent for u64 { } } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -787,11 +735,7 @@ impl AvroSchemaComponent for u128 { } } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } @@ -820,11 +764,7 @@ impl AvroSchemaComponent for i128 { } } - fn get_record_fields_in_ctxt( - _: usize, - _: &mut HashSet<Name>, - _: &Namespace, - ) -> Option<Vec<RecordField>> { + fn get_record_fields_in_ctxt(_: &mut HashSet<Name>, _: &Namespace) -> Option<Vec<RecordField>> { None } } diff --git a/avro/src/serde/ser_schema/mod.rs b/avro/src/serde/ser_schema/mod.rs index 3945263..b49f059 100644 --- a/avro/src/serde/ser_schema/mod.rs +++ b/avro/src/serde/ser_schema/mod.rs @@ -273,11 +273,16 @@ impl<'a, 's, W: Write> SchemaAwareWriteSerializeStruct<'a, 's, W> { } } - fn serialize_next_field<T>(&mut self, field: &RecordField, value: &T) -> Result<(), Error> + fn serialize_next_field<T>( + &mut self, + field: &RecordField, + position: usize, + value: &T, + ) -> Result<(), Error> where T: ?Sized + ser::Serialize, { - match self.field_position.cmp(&field.position) { + match self.field_position.cmp(&position) { Ordering::Equal => { // If we receive fields in order, write them directly to the main writer let mut value_ser = SchemaAwareWriteSerializer::new( @@ -310,7 +315,7 @@ impl<'a, 's, W: Write> SchemaAwareWriteSerializeStruct<'a, 's, W> { self.ser.enclosing_namespace.clone(), ); value.serialize(&mut value_ser)?; - if self.field_cache.insert(field.position, bytes).is_some() { + if self.field_cache.insert(position, bytes).is_some() { Err(Details::FieldNameDuplicate(field.name.clone()).into()) } else { Ok(()) @@ -336,7 +341,7 @@ impl<'a, 's, W: Write> SchemaAwareWriteSerializeStruct<'a, 's, W> { self.bytes_written += bytes.len(); self.field_position += 1; } else if let Some(default) = &field_info.default { - self.serialize_next_field(field_info, default) + self.serialize_next_field(field_info, self.field_position, default) .map_err(|e| Details::SerializeRecordFieldWithSchema { field_name: field_info.name.clone(), record_schema: Schema::Record(self.record_schema.clone()), @@ -373,52 +378,45 @@ impl<W: Write> ser::SerializeStruct for SchemaAwareWriteSerializeStruct<'_, '_, where T: ?Sized + ser::Serialize, { - let record_field = self - .record_schema - .lookup - .get(key) - .and_then(|idx| self.record_schema.fields.get(*idx)); - - match record_field { - Some(field) => self.serialize_next_field(field, value).map_err(|e| { - Details::SerializeRecordFieldWithSchema { - field_name: key.to_string(), - record_schema: Schema::Record(self.record_schema.clone()), - error: Box::new(e), - } - .into() - }), - None => Err(Details::FieldName(String::from(key)).into()), + if let Some(position) = self.record_schema.lookup.get(key).copied() { + let field = &self.record_schema.fields[position]; + self.serialize_next_field(field, position, value) + .map_err(|e| { + Details::SerializeRecordFieldWithSchema { + field_name: key.to_string(), + record_schema: Schema::Record(self.record_schema.clone()), + error: Box::new(e), + } + .into() + }) + } else { + Err(Details::FieldName(String::from(key)).into()) } } fn skip_field(&mut self, key: &'static str) -> Result<(), Self::Error> { - let skipped_field = self - .record_schema - .lookup - .get(key) - .and_then(|idx| self.record_schema.fields.get(*idx)); - - if let Some(skipped_field) = skipped_field { - if let Some(default) = &skipped_field.default { - self.serialize_next_field(skipped_field, default) - .map_err(|e| Details::SerializeRecordFieldWithSchema { - field_name: key.to_string(), - record_schema: Schema::Record(self.record_schema.clone()), - error: Box::new(e), - })?; + if let Some(position) = self.record_schema.lookup.get(key).copied() { + let field = &self.record_schema.fields[position]; + if let Some(default) = &field.default { + self.serialize_next_field(field, position, default) + .map_err(|e| { + Details::SerializeRecordFieldWithSchema { + field_name: key.to_string(), + record_schema: Schema::Record(self.record_schema.clone()), + error: Box::new(e), + } + .into() + }) } else { - return Err(Details::MissingDefaultForSkippedField { + Err(Details::MissingDefaultForSkippedField { field_name: key.to_string(), schema: Schema::Record(self.record_schema.clone()), } - .into()); + .into()) } } else { - return Err(Details::GetField(key.to_string()).into()); + Err(Details::GetField(key.to_string()).into()) } - - Ok(()) } fn end(self) -> Result<Self::Ok, Self::Error> { @@ -453,21 +451,19 @@ impl<W: Write> ser::SerializeMap for SchemaAwareWriteSerializeStruct<'_, '_, W> T: ?Sized + Serialize, { let key = self.map_field_name.take().ok_or(Details::MapNoKey)?; - let record_field = self - .record_schema - .lookup - .get(&key) - .and_then(|idx| self.record_schema.fields.get(*idx)); - match record_field { - Some(field) => self.serialize_next_field(field, value).map_err(|e| { - Details::SerializeRecordFieldWithSchema { - field_name: key.to_string(), - record_schema: Schema::Record(self.record_schema.clone()), - error: Box::new(e), - } - .into() - }), - None => Err(Details::FieldName(key).into()), + if let Some(position) = self.record_schema.lookup.get(&key).copied() { + let field = &self.record_schema.fields[position]; + self.serialize_next_field(field, position, value) + .map_err(|e| { + Details::SerializeRecordFieldWithSchema { + field_name: key.to_string(), + record_schema: Schema::Record(self.record_schema.clone()), + error: Box::new(e), + } + .into() + }) + } else { + Err(Details::FieldName(key).into()) } } diff --git a/avro/src/types.rs b/avro/src/types.rs index 8a9bdf6..9795f05 100644 --- a/avro/src/types.rs +++ b/avro/src/types.rs @@ -1238,7 +1238,6 @@ mod tests { use crate::{ duration::{Days, Millis, Months}, error::Details, - schema::RecordFieldOrder, to_value, }; use apache_avro_test_helper::{ @@ -1404,21 +1403,12 @@ mod tests { name: Name::new("record_name")?, aliases: None, doc: None, - fields: vec![RecordField { - name: "field_name".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Int, - order: RecordFieldOrder::Ignore, - position: 0, - custom_attributes: Default::default(), - }], + fields: vec![RecordField::builder().name("field_name".to_string()).schema(Schema::Int).build()], lookup: Default::default(), attributes: Default::default(), }), false, - r#"Invalid value: Record([("unknown_field_name", Null)]) for schema: Record(RecordSchema { name: Name { name: "record_name", namespace: None }, fields: [RecordField { name: "field_name", schema: Int, position: 0, .. }], .. }). Reason: There is no schema field for field 'unknown_field_name'"#, + r#"Invalid value: Record([("unknown_field_name", Null)]) for schema: Record(RecordSchema { name: Name { name: "record_name", namespace: None }, fields: [RecordField { name: "field_name", schema: Int, .. }], .. }). Reason: There is no schema field for field 'unknown_field_name'"#, ), ( Value::Record(vec![("field_name".to_string(), Value::Null)]), @@ -1426,23 +1416,15 @@ mod tests { name: Name::new("record_name")?, aliases: None, doc: None, - fields: vec![RecordField { - name: "field_name".to_string(), - doc: None, - default: None, - aliases: None, - schema: Schema::Ref { + fields: vec![ + RecordField::builder().name("field_name".to_string()).schema(Schema::Ref { name: Name::new("missing")?, - }, - order: RecordFieldOrder::Ignore, - position: 0, - custom_attributes: Default::default(), - }], + }).build()], 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", namespace: None }, fields: [RecordField { name: "field_name", schema: Ref { name: Name { name: "missing", namespace: None } }, position: 0, .. }], .. }). Reason: Unresolved schema reference: 'Name { name: "missing", namespace: None }'. Parsed names: []"#, + r#"Invalid value: Record([("field_name", Null)]) for schema: Record(RecordSchema { name: Name { name: "record_name", namespace: None }, fields: [RecordField { name: "field_name", schema: Ref { name: Name { name: "missing", namespace: None } }, .. }], .. }). Reason: Unresolved schema reference: 'Name { name: "missing", namespace: None }'. Parsed names: []"#, ), ]; @@ -1598,30 +1580,24 @@ mod tests { name: "a".to_string(), doc: None, default: None, - aliases: None, + aliases: Vec::new(), schema: Schema::Long, - order: RecordFieldOrder::Ascending, - position: 0, custom_attributes: Default::default(), }, RecordField { name: "b".to_string(), doc: None, default: None, - aliases: None, + aliases: Vec::new(), schema: Schema::String, - order: RecordFieldOrder::Ascending, - position: 1, custom_attributes: Default::default(), }, RecordField { name: "c".to_string(), doc: None, default: Some(JsonValue::Null), - aliases: None, + aliases: Vec::new(), schema: Schema::Union(UnionSchema::new(vec![Schema::Null, Schema::Int])?), - order: RecordFieldOrder::Ascending, - position: 2, custom_attributes: Default::default(), }, ], @@ -1656,7 +1632,7 @@ mod tests { ]); 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", namespace: None }, fields: [RecordField { name: "a", schema: Long, position: 0, .. }, RecordField { name: "b", schema: String, position: 1, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), position: 2, .. }], .. }). Reason: Unsupported value-schema combination! Value: Boolean(false), schem [...] + r#"Invalid value: Record([("a", Boolean(false)), ("b", String("foo"))]) for schema: Record(RecordSchema { name: Name { name: "some_record", namespace: None }, 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"#, ); let value = Value::Record(vec![ @@ -1665,7 +1641,7 @@ mod tests { ]); 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", namespace: None }, fields: [RecordField { name: "a", schema: Long, position: 0, .. }, RecordField { name: "b", schema: String, position: 1, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), position: 2, .. }], .. }). Reason: Could not find matching type in union"#, + r#"Invalid value: Record([("a", Long(42)), ("c", String("foo"))]) for schema: Record(RecordSchema { name: Name { name: "some_record", namespace: None }, 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"#, @@ -1677,7 +1653,7 @@ mod tests { ]); 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", namespace: None }, fields: [RecordField { name: "a", schema: Long, position: 0, .. }, RecordField { name: "b", schema: String, position: 1, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), position: 2, .. }], .. }). Reason: There is no schema field for field 'd'"#, + r#"Invalid value: Record([("a", Long(42)), ("d", String("foo"))]) for schema: Record(RecordSchema { name: Name { name: "some_record", namespace: None }, 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'"#, ); let value = Value::Record(vec![ @@ -1688,7 +1664,7 @@ mod tests { ]); 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", namespace: None }, fields: [RecordField { name: "a", schema: Long, position: 0, .. }, RecordField { name: "b", schema: String, position: 1, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), position: 2, .. }], .. }). Reason: The value's records length (4) is greater than [...] + r#"Invalid value: Record([("a", Long(42)), ("b", String("foo")), ("c", Null), ("d", Null)]) for schema: Record(RecordSchema { name: Name { name: "some_record", namespace: None }, 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!( @@ -1712,7 +1688,7 @@ mod tests { .validate(&schema) ); assert_logged( - r#"Invalid value: Map({"d": Long(123)}) for schema: Record(RecordSchema { name: Name { name: "some_record", namespace: None }, fields: [RecordField { name: "a", schema: Long, position: 0, .. }, RecordField { name: "b", schema: String, position: 1, .. }, RecordField { name: "c", default: Null, schema: Union(UnionSchema { schemas: [Null, Int] }), position: 2, .. }], .. }). Reason: Field with name '"a"' is not a member of the map items + r#"Invalid value: Map({"d": Long(123)}) for schema: Record(RecordSchema { name: Name { name: "some_record", namespace: None }, 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 Field with name '"b"' is not a member of the map items"#, ); diff --git a/avro/src/writer.rs b/avro/src/writer.rs index 3031236..3c9bf86 100644 --- a/avro/src/writer.rs +++ b/avro/src/writer.rs @@ -1785,7 +1785,7 @@ mod tests { Err(e) => { assert_eq!( e.to_string(), - r#"Failed to serialize field 'time' for record Record(RecordSchema { name: Name { name: "Conference", namespace: None }, fields: [RecordField { name: "name", schema: String, position: 0, .. }, RecordField { name: "date", aliases: ["time2", "time"], schema: Union(UnionSchema { schemas: [Null, Long] }), position: 1, .. }], .. }): Failed to serialize value of type f64 using schema Union(UnionSchema { schemas: [Null, Long] }): 12345678.9. Cause: Cannot find a Double schem [...] + r#"Failed to serialize field 'time' for record Record(RecordSchema { name: Name { name: "Conference", namespace: None }, fields: [RecordField { name: "name", schema: String, .. }, RecordField { name: "date", aliases: ["time2", "time"], schema: Union(UnionSchema { schemas: [Null, Long] }), .. }], .. }): Failed to serialize value of type f64 using schema Union(UnionSchema { schemas: [Null, Long] }): 12345678.9. Cause: Cannot find a Double schema in [Null, Long]"# ); } } diff --git a/avro/tests/get_record_fields.rs b/avro/tests/get_record_fields.rs index 3417f26..765fda6 100644 --- a/avro/tests/get_record_fields.rs +++ b/avro/tests/get_record_fields.rs @@ -33,7 +33,7 @@ fn avro_rs_448_default_get_record_fields_no_recursion() -> TestResult { let mut named_schemas = HashSet::new(); let fields = - get_record_fields_in_ctxt(0, &mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); + get_record_fields_in_ctxt(&mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); assert_eq!(fields.len(), 2); assert!( @@ -53,7 +53,7 @@ fn avro_rs_448_default_get_record_fields_no_recursion() -> TestResult { ); let fields = - get_record_fields_in_ctxt(0, &mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); + get_record_fields_in_ctxt(&mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); assert_eq!(fields.len(), 2); assert_eq!( named_schemas.len(), @@ -74,7 +74,7 @@ fn avro_rs_448_default_get_record_fields_recursion() -> TestResult { let mut named_schemas = HashSet::new(); let fields = - get_record_fields_in_ctxt(0, &mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); + get_record_fields_in_ctxt(&mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); assert_eq!(fields.len(), 2); assert_eq!( @@ -91,7 +91,7 @@ fn avro_rs_448_default_get_record_fields_recursion() -> TestResult { assert_eq!(named_schemas.len(), 1); let fields = - get_record_fields_in_ctxt(0, &mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); + get_record_fields_in_ctxt(&mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); assert_eq!(fields.len(), 2); assert_eq!( named_schemas.len(), @@ -101,49 +101,3 @@ fn avro_rs_448_default_get_record_fields_recursion() -> TestResult { Ok(()) } - -#[test] -fn avro_rs_448_default_get_record_fields_position() -> TestResult { - #[derive(apache_avro_derive::AvroSchema)] - struct Foo { - _a: i32, - _b: String, - } - - let mut named_schemas = HashSet::new(); - let fields = - get_record_fields_in_ctxt(10, &mut named_schemas, &None, Foo::get_schema_in_ctxt).unwrap(); - - assert_eq!(fields.len(), 2); - assert!( - named_schemas.is_empty(), - "Name shouldn't have been added: {named_schemas:?}" - ); - let positions = fields.into_iter().map(|f| f.position).collect::<Vec<_>>(); - assert_eq!(positions.as_slice(), &[10, 11][..]); - - // Insert Foo into named_schemas - match Foo::get_schema_in_ctxt(&mut named_schemas, &None) { - Schema::Record(_) => {} - schema => panic!("Expected a record got {schema:?}"), - } - assert_eq!( - named_schemas.len(), - 1, - "Name should have been added: {named_schemas:?}" - ); - - let fields = - get_record_fields_in_ctxt(5043, &mut named_schemas, &None, Foo::get_schema_in_ctxt) - .unwrap(); - assert_eq!(fields.len(), 2); - assert_eq!( - named_schemas.len(), - 1, - "Name shouldn't have been removed: {named_schemas:?}" - ); - let positions = fields.into_iter().map(|f| f.position).collect::<Vec<_>>(); - assert_eq!(positions.as_slice(), &[5043, 5044][..]); - - Ok(()) -} diff --git a/avro/tests/schema.rs b/avro/tests/schema.rs index 89bfd68..e7fb61f 100644 --- a/avro/tests/schema.rs +++ b/avro/tests/schema.rs @@ -375,8 +375,6 @@ fn test_parse_reused_record_schema_by_fullname() -> TestResult { default: _, aliases: _, schema, - order: _, - position: _, custom_attributes: _, } = fields.get(2).unwrap(); diff --git a/avro_derive/src/lib.rs b/avro_derive/src/lib.rs index 3b9b724..51d0117 100644 --- a/avro_derive/src/lib.rs +++ b/avro_derive/src/lib.rs @@ -122,7 +122,7 @@ fn create_trait_definition( #get_schema_impl } - fn get_record_fields_in_ctxt(mut field_position: usize, named_schemas: &mut ::std::collections::HashSet<::apache_avro::schema::Name>, enclosing_namespace: &::std::option::Option<::std::string::String>) -> ::std::option::Option<::std::vec::Vec<::apache_avro::schema::RecordField>> { + fn get_record_fields_in_ctxt(named_schemas: &mut ::std::collections::HashSet<::apache_avro::schema::Name>, enclosing_namespace: &::std::option::Option<::std::string::String>) -> ::std::option::Option<::std::vec::Vec<::apache_avro::schema::RecordField>> { #get_record_fields_impl } @@ -185,7 +185,6 @@ fn get_struct_schema_def( get_field_get_record_fields_expr(&field, field_attrs.with)?; record_field_exprs.push(quote! { if let Some(flattened_fields) = #get_record_fields { - field_position += flattened_fields.len(); schema_fields.extend(flattened_fields); } else { panic!("{} does not have any fields to flatten to", stringify!(#field)); @@ -211,7 +210,7 @@ fn get_struct_schema_def( } } }; - let aliases = aliases(&field_attrs.alias); + let aliases = field_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 { @@ -220,11 +219,8 @@ fn get_struct_schema_def( default: #default_value, aliases: #aliases, schema: #schema_expr, - order: ::apache_avro::schema::RecordFieldOrder::Ascending, - position: field_position, custom_attributes: Default::default(), }); - field_position += 1; }); } } @@ -253,14 +249,14 @@ fn get_struct_schema_def( let schema_def = quote! { { let mut schema_fields = Vec::with_capacity(#minimum_fields); - let mut field_position = 0; #(#record_field_exprs)* let schema_field_set: ::std::collections::HashSet<_> = schema_fields.iter().map(|rf| &rf.name).collect(); assert_eq!(schema_fields.len(), schema_field_set.len(), "Duplicate field names found: {schema_fields:?}"); let name = ::apache_avro::schema::Name::new(#full_schema_name).expect(&format!("Unable to parse struct name for schema {}", #full_schema_name)[..]); let lookup: std::collections::BTreeMap<String, usize> = schema_fields .iter() - .map(|field| (field.name.to_owned(), field.position)) + .enumerate() + .map(|(position, field)| (field.name.to_owned(), position)) .collect(); ::apache_avro::schema::Schema::Record(apache_avro::schema::RecordSchema { name, @@ -355,14 +351,13 @@ fn get_field_get_record_fields_expr( ) -> Result<TokenStream, Vec<syn::Error>> { match with { With::Trait => Ok(type_to_get_record_fields_expr(&field.ty)?), - With::Serde(path) => Ok( - quote! { #path::get_record_fields_in_ctxt(field_position, named_schemas, enclosing_namespace) }, - ), + With::Serde(path) => { + Ok(quote! { #path::get_record_fields_in_ctxt(named_schemas, enclosing_namespace) }) + } With::Expr(Expr::Closure(closure)) => { if closure.inputs.is_empty() { Ok(quote! { ::apache_avro::serde::get_record_fields_in_ctxt( - field_position, named_schemas, enclosing_namespace, |_, _| (#closure)(), @@ -376,7 +371,7 @@ fn get_field_get_record_fields_expr( } } With::Expr(Expr::Path(path)) => Ok(quote! { - ::apache_avro::serde::get_record_fields_in_ctxt(field_position, named_schemas, enclosing_namespace, #path) + ::apache_avro::serde::get_record_fields_in_ctxt(named_schemas, enclosing_namespace, #path) }), With::Expr(_expr) => Err(vec![syn::Error::new( field.span(), @@ -411,7 +406,7 @@ fn type_to_schema_expr(ty: &Type) -> Result<TokenStream, Vec<syn::Error>> { fn type_to_get_record_fields_expr(ty: &Type) -> Result<TokenStream, Vec<syn::Error>> { match ty { Type::Array(_) | Type::Slice(_) | Type::Path(_) | Type::Reference(_) => Ok( - quote! {<#ty as :: apache_avro::AvroSchemaComponent>::get_record_fields_in_ctxt(field_position, named_schemas, enclosing_namespace)}, + quote! {<#ty as :: apache_avro::AvroSchemaComponent>::get_record_fields_in_ctxt(named_schemas, enclosing_namespace)}, ), Type::Ptr(_) => Err(vec![syn::Error::new_spanned( ty, @@ -471,9 +466,21 @@ fn aliases(op: &[impl quote::ToTokens]) -> TokenStream { .map(|tt| quote! {#tt.try_into().expect("Alias is invalid")}) .collect(); if items.is_empty() { - quote! {None} + quote! {::std::option::Option::None} + } else { + quote! {::std::option::Option::Some(vec![#(#items),*])} + } +} + +fn field_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! {::std::vec::Vec::new()} } else { - quote! {Some(vec![#(#items),*])} + quote! {vec![#(#items),*]} } } @@ -604,7 +611,7 @@ mod tests { name: ::apache_avro::schema::Name::new("Basic").expect( &format!("Unable to parse enum name for schema {}", "Basic")[..] ), - aliases: None, + aliases: ::std::option::Option::None, doc: None, symbols: vec![ "A".to_owned(), @@ -619,7 +626,6 @@ mod tests { } fn get_record_fields_in_ctxt( - mut field_position: usize, named_schemas: &mut ::std::collections::HashSet<::apache_avro::schema::Name>, enclosing_namespace: &::std::option::Option<::std::string::String> ) -> ::std::option::Option <::std::vec::Vec<::apache_avro::schema::RecordField>> { @@ -755,7 +761,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 :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] + let expected_token_stream = r#"# [automatically_derived] impl :: apache_avro :: AvroSchemaComponent for A { fn get_schema_in_ctxt (named_schemas : & mut :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] let schema_token_stream = schema_res.unwrap().to_string(); assert_eq!(schema_token_stream, expected_token_stream); } @@ -774,7 +780,7 @@ mod tests { match syn::parse2::<DeriveInput>(test_enum) { 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 :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] + let expected_token_stream = r#"# [automatically_derived] impl :: apache_avro :: AvroSchemaComponent for A { fn get_schema_in_ctxt (named_schemas : & mut :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] let schema_token_stream = schema_res.unwrap().to_string(); assert_eq!(schema_token_stream, expected_token_stream); } @@ -797,7 +803,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 :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] + let expected_token_stream = r#"# [automatically_derived] impl :: apache_avro :: AvroSchemaComponent for A { fn get_schema_in_ctxt (named_schemas : & mut :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] let schema_token_stream = schema_res.unwrap().to_string(); assert_eq!(schema_token_stream, expected_token_stream); } @@ -817,7 +823,7 @@ mod tests { match syn::parse2::<DeriveInput>(test_enum) { Ok(input) => { let schema_res = derive_avro_schema(input); - let expected_token_stream = r#"# [automatically_derived] impl :: apache_avro :: AvroSchemaComponent for B { fn get_schema_in_ctxt (named_schemas : & mut :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("B") . expect (concat ! ("Unable to parse schema name " , "B")) . fully_qual [...] + let expected_token_stream = r#"# [automatically_derived] impl :: apache_avro :: AvroSchemaComponent for B { fn get_schema_in_ctxt (named_schemas : & mut :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("B") . expect (concat ! ("Unable to parse schema name " , "B")) . fully_qual [...] let schema_token_stream = schema_res.unwrap().to_string(); assert_eq!(schema_token_stream, expected_token_stream); } @@ -841,7 +847,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 :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] + let expected_token_stream = r#"# [automatically_derived] impl :: apache_avro :: AvroSchemaComponent for A { fn get_schema_in_ctxt (named_schemas : & mut :: std :: collections :: HashSet < :: apache_avro :: schema :: Name > , enclosing_namespace : & :: std :: option :: Option < :: std :: string :: String >) -> :: apache_avro :: schema :: Schema { let name = :: apache_avro :: schema :: Name :: new ("A") . expect (concat ! ("Unable to parse schema name " , "A")) . fully_qual [...] let schema_token_stream = schema_res.unwrap().to_string(); assert_eq!(schema_token_stream, expected_token_stream); } diff --git a/avro_derive/tests/derive.rs b/avro_derive/tests/derive.rs index 4b3c5cf..b6855b8 100644 --- a/avro_derive/tests/derive.rs +++ b/avro_derive/tests/derive.rs @@ -2310,7 +2310,7 @@ fn avro_rs_448_transparent_with() { let mut named_schemas = HashSet::new(); assert_eq!( - TestStruct::get_record_fields_in_ctxt(0, &mut named_schemas, &None), + TestStruct::get_record_fields_in_ctxt(&mut named_schemas, &None), None ); assert!( @@ -2335,7 +2335,7 @@ fn avro_rs_448_transparent_with_2() { } let mut named_schemas = HashSet::new(); - let fields = TestStruct::get_record_fields_in_ctxt(0, &mut named_schemas, &None).unwrap(); + let fields = TestStruct::get_record_fields_in_ctxt(&mut named_schemas, &None).unwrap(); assert!( named_schemas.is_empty(), "No name should've been added: {named_schemas:?}" @@ -2349,7 +2349,7 @@ fn avro_rs_448_transparent_with_2() { "One name should've been added: {named_schemas:?}" ); - let fields = TestStruct::get_record_fields_in_ctxt(0, &mut named_schemas, &None).unwrap(); + let fields = TestStruct::get_record_fields_in_ctxt(&mut named_schemas, &None).unwrap(); assert_eq!( named_schemas.len(), 1, @@ -2358,34 +2358,6 @@ fn avro_rs_448_transparent_with_2() { assert_eq!(fields.len(), 2); } -#[test] -fn avro_rs_448_flatten_field_positions() { - #[derive(AvroSchema)] - struct Foo { - _a: i32, - _b: String, - } - - #[derive(AvroSchema)] - struct Bar { - _c: Vec<i64>, - #[serde(flatten)] - _d: Foo, - _e: bool, - } - - let Schema::Record(schema) = Bar::get_schema() else { - panic!("Structs should generate records") - }; - - let positions = schema - .fields - .into_iter() - .map(|f| f.position) - .collect::<Vec<_>>(); - assert_eq!(positions.as_slice(), &[0, 1, 2, 3][..]); -} - #[test] fn avro_rs_476_field_default() { #[derive(AvroSchema)]
