From aa3cf21b8323bbb7f57a132d65865512f1211c17 Mon Sep 17 00:00:00 2001 From: Martin Tzvetanov Grigorov Date: Mon, 20 Jan 2025 10:06:01 +0200 Subject: [PATCH] Minor cleanup and better naming Signed-off-by: Martin Tzvetanov Grigorov --- avro/src/schema.rs | 39 +++++++++++++++++++-------------------- avro/tests/schema.rs | 36 ++++++++++++++++++------------------ 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/avro/src/schema.rs b/avro/src/schema.rs index 707cf74..1737665 100644 --- a/avro/src/schema.rs +++ b/avro/src/schema.rs @@ -25,7 +25,6 @@ use crate::{ validate_schema_name, }, AvroResult, - Schema::Ref, }; use digest::Digest; use log::{debug, error, warn}; @@ -1052,9 +1051,9 @@ impl Schema { /// [Parsing Canonical Form]: /// https://avro.apache.org/docs/current/specification/#parsing-canonical-form-for-schemas pub fn independent_canonical_form(&self, schemata: &Vec) -> Result { - let mut d = self.clone(); - d.denormalize(schemata)?; - Ok(d.canonical_form()) + let mut this = self.clone(); + this.denormalize(schemata)?; + Ok(this.canonical_form()) } /// Generate [fingerprint] of Schema's [Parsing Canonical Form]. @@ -1262,32 +1261,32 @@ impl Schema { fn denormalize(&mut self, schemata: &Vec) -> AvroResult<()> { match self { - Ref { name } => { - let repl = schemata + Schema::Ref { name } => { + let replacement_schema = schemata .iter() .find(|s| s.name().map(|n| *n == *name).unwrap_or(false)); - if let Some(r) = repl { - let mut denorm = r.clone(); + if let Some(schema) = replacement_schema { + let mut denorm = schema.clone(); denorm.denormalize(schemata)?; *self = denorm; } else { return Err(Error::SchemaResolutionError(name.clone())); } } - Schema::Record(r) => { - for rr in &mut r.fields { - rr.schema.denormalize(schemata)?; + Schema::Record(record_schema) => { + for field in &mut record_schema.fields { + field.schema.denormalize(schemata)?; } } - Schema::Array(a) => { - a.items.denormalize(schemata)?; + Schema::Array(array_schema) => { + array_schema.items.denormalize(schemata)?; } - Schema::Map(m) => { - m.types.denormalize(schemata)?; + Schema::Map(map_schema) => { + map_schema.types.denormalize(schemata)?; } - Schema::Union(u) => { - for uu in &mut u.schemas { - uu.denormalize(schemata)?; + Schema::Union(union_schema) => { + for schema in &mut union_schema.schemas { + schema.denormalize(schemata)?; } } _ => (), @@ -2319,7 +2318,7 @@ fn pcf_map(schema: &Map, defined_names: &mut HashSet) -> //if this is already a defined type, early return if let Some(ref n) = name { - if defined_names.get(n).is_some() { + if defined_names.contains(n) { return pcf_string(n); } else { defined_names.insert(n.clone()); @@ -2349,7 +2348,7 @@ fn pcf_map(schema: &Map, defined_names: &mut HashSet) -> // Fully qualify the name, if it isn't already ([FULLNAMES] rule). if k == "name" { if let Some(ref n) = name { - fields.push((k, format!("{}:{}", pcf_string(k), pcf_string(n)))); + fields.push(("name", format!("{}:{}", pcf_string(k), pcf_string(n)))); continue; } } diff --git a/avro/tests/schema.rs b/avro/tests/schema.rs index a0e860a..0c90615 100644 --- a/avro/tests/schema.rs +++ b/avro/tests/schema.rs @@ -2019,7 +2019,7 @@ fn test_avro_3851_read_default_value_for_enum() -> TestResult { } #[test] -fn test_independent_canonical_form_primitives() -> TestResult { +fn avro_rs_66_test_independent_canonical_form_primitives() -> TestResult { init(); let record_primitive = r#"{ "name": "Rec", @@ -2100,7 +2100,7 @@ fn test_independent_canonical_form_primitives() -> TestResult { for schema_str_perm in permutations(&schema_strs) { let schema_str_perm: Vec<&str> = schema_str_perm.iter().map(|s| **s).collect(); let schemata = Schema::parse_list(&schema_str_perm)?; - assert_eq!(schemata.len(), 4); + assert_eq!(schemata.len(), schema_strs.len()); let test_schema = schemata .iter() .find(|a| a.name().unwrap().to_string() == *"RecWithDeps") @@ -2120,7 +2120,7 @@ fn test_independent_canonical_form_primitives() -> TestResult { } #[test] -fn test_independent_canonical_form_usages() -> TestResult { +fn avro_rs_66_test_independent_canonical_form_usages() -> TestResult { init(); let record_primitive = r#"{ "name": "Rec", @@ -2218,36 +2218,39 @@ fn test_independent_canonical_form_usages() -> TestResult { for schema_str_perm in permutations(&schema_strs) { let schema_str_perm: Vec<&str> = schema_str_perm.iter().map(|s| **s).collect(); let schemata = Schema::parse_list(&schema_str_perm)?; - for s in &schemata { - match s.name().unwrap().to_string().as_str() { + for schema in &schemata { + match schema.name().unwrap().to_string().as_str() { "RecUsage" => { assert_eq!( - s.independent_canonical_form(&schemata)?, + schema.independent_canonical_form(&schemata)?, Schema::parse_str(record_usage_independent)?.canonical_form() ); } "ArrayUsage" => { assert_eq!( - s.independent_canonical_form(&schemata)?, + schema.independent_canonical_form(&schemata)?, Schema::parse_str(array_usage_independent)?.canonical_form() ); } "UnionUsage" => { assert_eq!( - s.independent_canonical_form(&schemata)?, + schema.independent_canonical_form(&schemata)?, Schema::parse_str(union_usage_independent)?.canonical_form() ); } "MapUsage" => { assert_eq!( - s.independent_canonical_form(&schemata)?, + schema.independent_canonical_form(&schemata)?, Schema::parse_str(map_usage_independent)?.canonical_form() ); } "ns.Rec" => { - assert_eq!(s.independent_canonical_form(&schemata)?, s.canonical_form()); + assert_eq!( + schema.independent_canonical_form(&schemata)?, + schema.canonical_form() + ); } - _ => panic!(), + other => unreachable!("Unknown schema name: {}", other), } } } @@ -2255,7 +2258,7 @@ fn test_independent_canonical_form_usages() -> TestResult { } #[test] -fn test_independent_canonical_form_deep_recursion() -> TestResult { +fn avro_rs_66_test_independent_canonical_form_deep_recursion() -> TestResult { init(); let record_primitive = r#"{ "name": "Rec", @@ -2323,7 +2326,7 @@ fn test_independent_canonical_form_deep_recursion() -> TestResult { } #[test] -fn test_independent_canonical_form_missing_ref() -> TestResult { +fn avro_rs_66_test_independent_canonical_form_missing_ref() -> TestResult { init(); let record_primitive = r#"{ "name": "Rec", @@ -2345,11 +2348,8 @@ fn test_independent_canonical_form_missing_ref() -> TestResult { let schema_strs = [record_primitive, record_usage]; let schemata = Schema::parse_list(&schema_strs)?; assert!(matches!( - schemata[1] - .independent_canonical_form(&vec![]) - .err() - .unwrap(), //NOTE - we're passing in an empty schemata - Error::SchemaResolutionError(..) + schemata[1].independent_canonical_form(&Vec::with_capacity(0)), //NOTE - we're passing in an empty schemata + Err(Error::SchemaResolutionError(..)) )); Ok(()) }