Skip to content

Commit

Permalink
Minor cleanup and better naming
Browse files Browse the repository at this point in the history
Signed-off-by: Martin Tzvetanov Grigorov <[email protected]>
  • Loading branch information
martin-g committed Jan 20, 2025
1 parent 5850912 commit aa3cf21
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 38 deletions.
39 changes: 19 additions & 20 deletions avro/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use crate::{
validate_schema_name,
},
AvroResult,
Schema::Ref,
};
use digest::Digest;
use log::{debug, error, warn};
Expand Down Expand Up @@ -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<Schema>) -> Result<String, Error> {
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].
Expand Down Expand Up @@ -1262,32 +1261,32 @@ impl Schema {

fn denormalize(&mut self, schemata: &Vec<Schema>) -> 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)?;
}
}
_ => (),
Expand Down Expand Up @@ -2319,7 +2318,7 @@ fn pcf_map(schema: &Map<String, Value>, defined_names: &mut HashSet<String>) ->

//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());
Expand Down Expand Up @@ -2349,7 +2348,7 @@ fn pcf_map(schema: &Map<String, Value>, defined_names: &mut HashSet<String>) ->
// 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;
}
}
Expand Down
36 changes: 18 additions & 18 deletions avro/tests/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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")
Expand All @@ -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",
Expand Down Expand Up @@ -2218,44 +2218,47 @@ 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),
}
}
}
Ok(())
}

#[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",
Expand Down Expand Up @@ -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",
Expand All @@ -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(())
}

0 comments on commit aa3cf21

Please sign in to comment.