From e03800472eaad3d9bfa3292e518264ee0210d991 Mon Sep 17 00:00:00 2001 From: Martin Grigorov Date: Tue, 20 Feb 2024 11:17:56 +0200 Subject: [PATCH] AVRO-3939: [Rust] Make it possible to use custom schema equality comparators (#2739) * AVRO-3939: [Rust] Make it possible to use custom schema comparators Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Temporarily use StructFieldComparator as default Implement compare_fields() Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Fix formatting and clippy issues Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Rename the trait and its impls from Comparator to Eq Add unit tests for the primitives Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Add support for comparing the custom attributes to StructFieldEq Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Add more unit tests Fix clippy error with Rust nightly, e.g.: ``` error: the item `TryFrom` is imported redundantly --> avro/src/types.rs:34:5 | 34 | convert::TryFrom, | ^^^^^^^^^^^^^^^^ --> /rustc/2bf78d12d33ae02d10010309a0d85dd04e7cff72/library/std/src/prelude/mod.rs:129:13 | = note: the item `TryFrom` is already defined here error: could not compile `apache-avro` (lib) due to 9 previous errors ``` Signed-off-by: Martin Tzvetanov Grigorov * AVRo-3939: Add more unit tests fix more Rust nightly clippy errors in tests Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: More unit tests and better logging Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Fix rust nightly clippy error ``` error: the item `RecordField` is imported redundantly --> avro/src/types.rs:1153:18 | 1150 | use super::*; | -------- the item `RecordField` is already imported here ... 1153 | schema::{RecordField, RecordFieldOrder}, | ^^^^^^^^^^^ | = note: `-D unused-imports` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(unused_imports)]` ``` Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Improve TestLogger's assert_logged to assert against all logged messages Not just against the last logged message Signed-off-by: Martin Tzvetanov Grigorov * Fix rust nightly clippy error ``` error: the item `FromIterator` is imported redundantly --> avro/benches/serde_json.rs:20:33 | 20 | use std::{collections::HashMap, iter::FromIterator}; | ^^^^^^^^^^^^^^^^^^ --> /rustc/2bf78d12d33ae02d10010309a0d85dd04e7cff72/library/std/src/prelude/mod.rs:129:13 | = note: the item `FromIterator` is already defined here | = note: `-D unused-imports` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(unused_imports)]` error: could not compile `apache-avro` (bench "serde_json") due to 1 previous error warning: build failed, waiting for other jobs to finish... Error: Process completed with exit code 101. ``` Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: More test assertions and better logging Signed-off-by: Martin Tzvetanov Grigorov * AVRO-3939: Use StructFieldEq impl by default It is much faster than SpecificationEq which serializes the schemas to JSON and then compares them as strings Signed-off-by: Martin Tzvetanov Grigorov * AVro-3939: Format the code Signed-off-by: Martin Tzvetanov Grigorov --------- Signed-off-by: Martin Tzvetanov Grigorov --- lang/rust/Cargo.lock | 7 + lang/rust/avro/Cargo.toml | 2 +- lang/rust/avro/benches/serde_json.rs | 2 +- lang/rust/avro/src/bigdecimal.rs | 5 +- lang/rust/avro/src/decimal.rs | 1 - lang/rust/avro/src/decode.rs | 1 - lang/rust/avro/src/encode.rs | 7 +- lang/rust/avro/src/lib.rs | 1 + lang/rust/avro/src/reader.rs | 3 +- lang/rust/avro/src/schema.rs | 8 +- lang/rust/avro/src/schema_equality.rs | 588 +++++++++++++++++++++++ lang/rust/avro/src/types.rs | 8 +- lang/rust/avro/src/util.rs | 1 - lang/rust/avro/src/writer.rs | 2 +- lang/rust/avro_test_helper/src/logger.rs | 17 +- 15 files changed, 623 insertions(+), 30 deletions(-) create mode 100644 lang/rust/avro/src/schema_equality.rs diff --git a/lang/rust/Cargo.lock b/lang/rust/Cargo.lock index 52470c36045..4cfaa1a64b1 100644 --- a/lang/rust/Cargo.lock +++ b/lang/rust/Cargo.lock @@ -79,6 +79,7 @@ dependencies = [ "log", "md-5", "num-bigint", + "paste", "pretty_assertions", "quad-rand", "rand", @@ -886,6 +887,12 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "paste" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" + [[package]] name = "pin-project-lite" version = "0.2.13" diff --git a/lang/rust/avro/Cargo.toml b/lang/rust/avro/Cargo.toml index b175d25de66..9efca8f4db1 100644 --- a/lang/rust/avro/Cargo.toml +++ b/lang/rust/avro/Cargo.toml @@ -90,7 +90,7 @@ md-5 = { default-features = false, version = "0.10.6" } pretty_assertions = { default-features = false, version = "1.4.0", features = ["std"] } serial_test = "3.0.0" sha2 = { default-features = false, version = "0.10.8" } - +paste = { default-features = false, version = "1.0.14" } [package.metadata.docs.rs] all-features = true diff --git a/lang/rust/avro/benches/serde_json.rs b/lang/rust/avro/benches/serde_json.rs index 9a3a5dbfe81..780de2b2a08 100644 --- a/lang/rust/avro/benches/serde_json.rs +++ b/lang/rust/avro/benches/serde_json.rs @@ -17,7 +17,7 @@ use criterion::{criterion_group, criterion_main, Criterion}; use serde_json::Value; -use std::{collections::HashMap, iter::FromIterator}; +use std::collections::HashMap; fn make_big_json_record() -> Value { let address = HashMap::<_, _>::from_iter(vec![ diff --git a/lang/rust/avro/src/bigdecimal.rs b/lang/rust/avro/src/bigdecimal.rs index 93ddae48de4..65ddf97339b 100644 --- a/lang/rust/avro/src/bigdecimal.rs +++ b/lang/rust/avro/src/bigdecimal.rs @@ -63,10 +63,7 @@ pub(crate) fn deserialize_big_decimal(bytes: &Vec) -> Result TestResult { diff --git a/lang/rust/avro/src/decode.rs b/lang/rust/avro/src/decode.rs index 340fdb6e3fa..86584d3e0c1 100644 --- a/lang/rust/avro/src/decode.rs +++ b/lang/rust/avro/src/decode.rs @@ -31,7 +31,6 @@ use crate::{ use std::{ borrow::Borrow, collections::HashMap, - convert::TryFrom, io::{ErrorKind, Read}, str::FromStr, }; diff --git a/lang/rust/avro/src/encode.rs b/lang/rust/avro/src/encode.rs index 8b9b02a2fab..a7d34a4f504 100644 --- a/lang/rust/avro/src/encode.rs +++ b/lang/rust/avro/src/encode.rs @@ -25,11 +25,7 @@ use crate::{ util::{zig_i32, zig_i64}, AvroResult, Error, }; -use std::{ - borrow::Borrow, - collections::HashMap, - convert::{TryFrom, TryInto}, -}; +use std::{borrow::Borrow, collections::HashMap}; /// Encode a `Value` into avro format. /// @@ -311,7 +307,6 @@ pub(crate) mod tests { use super::*; use apache_avro_test_helper::TestResult; use pretty_assertions::assert_eq; - use std::collections::HashMap; use uuid::Uuid; pub(crate) fn success(value: &Value, schema: &Schema) -> String { diff --git a/lang/rust/avro/src/lib.rs b/lang/rust/avro/src/lib.rs index 1cfdd6a93cd..9d5dcb83c25 100644 --- a/lang/rust/avro/src/lib.rs +++ b/lang/rust/avro/src/lib.rs @@ -816,6 +816,7 @@ mod writer; pub mod rabin; pub mod schema; pub mod schema_compatibility; +pub mod schema_equality; pub mod types; pub mod validator; diff --git a/lang/rust/avro/src/reader.rs b/lang/rust/avro/src/reader.rs index 9b598315c8a..9a3be4c872c 100644 --- a/lang/rust/avro/src/reader.rs +++ b/lang/rust/avro/src/reader.rs @@ -28,7 +28,6 @@ use serde::de::DeserializeOwned; use serde_json::from_slice; use std::{ collections::HashMap, - convert::TryFrom, io::{ErrorKind, Read}, marker::PhantomData, str::FromStr, @@ -528,7 +527,7 @@ pub fn read_marker(bytes: &[u8]) -> [u8; 16] { #[cfg(test)] mod tests { use super::*; - use crate::{encode::encode, from_value, types::Record, Reader}; + use crate::{encode::encode, types::Record}; use apache_avro_test_helper::TestResult; use pretty_assertions::assert_eq; use serde::Deserialize; diff --git a/lang/rust/avro/src/schema.rs b/lang/rust/avro/src/schema.rs index 9712bc41d68..58bdc6ab1e6 100644 --- a/lang/rust/avro/src/schema.rs +++ b/lang/rust/avro/src/schema.rs @@ -18,7 +18,7 @@ //! Logic for parsing and interacting with schemas in Avro format. use crate::{ error::Error, - types, + schema_equality, types, util::MapHelper, validator::{ validate_enum_symbol_name, validate_namespace, validate_record_field_name, @@ -35,7 +35,6 @@ use serde_json::{Map, Value}; use std::{ borrow::{Borrow, Cow}, collections::{BTreeMap, HashMap, HashSet}, - convert::{TryFrom, TryInto}, fmt, fmt::Debug, hash::Hash, @@ -155,9 +154,9 @@ impl PartialEq for Schema { /// Assess equality of two `Schema` based on [Parsing Canonical Form]. /// /// [Parsing Canonical Form]: - /// https://avro.apache.org/docs/1.8.2/spec.html#Parsing+Canonical+Form+for+Schemas + /// https://avro.apache.org/docs/1.11.1/specification/#parsing-canonical-form-for-schemas fn eq(&self, other: &Self) -> bool { - self.canonical_form() == other.canonical_form() + schema_equality::compare_schemata(self, other) } } @@ -6356,6 +6355,7 @@ mod tests { "logicalType": "uuid" }); let parse_result = Schema::parse(&schema)?; + assert_eq!( parse_result, Schema::Fixed(FixedSchema { diff --git a/lang/rust/avro/src/schema_equality.rs b/lang/rust/avro/src/schema_equality.rs new file mode 100644 index 00000000000..ae90c3f3fef --- /dev/null +++ b/lang/rust/avro/src/schema_equality.rs @@ -0,0 +1,588 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +use crate::{ + schema::{ + ArraySchema, DecimalSchema, EnumSchema, FixedSchema, MapSchema, RecordField, RecordSchema, + UnionSchema, + }, + Schema, +}; +use std::{fmt::Debug, sync::OnceLock}; + +/// A trait that compares two schemata for equality. +/// To register a custom one use [set_schemata_equality_comparator]. +pub trait SchemataEq: Debug + Send + Sync { + /// Compares two schemata for equality. + fn compare(&self, schema_one: &Schema, schema_two: &Schema) -> bool; +} + +/// Compares two schemas according to the Avro specification by using +/// their canonical forms. +/// See +#[derive(Debug)] +pub struct SpecificationEq; +impl SchemataEq for SpecificationEq { + fn compare(&self, schema_one: &Schema, schema_two: &Schema) -> bool { + schema_one.canonical_form() == schema_two.canonical_form() + } +} + +/// Compares two schemas for equality field by field, using only the fields that +/// are used to construct their canonical forms. +/// See +#[derive(Debug)] +pub struct StructFieldEq { + /// Whether to include custom attributes in the comparison. + /// The custom attributes are not used to construct the canonical form of the schema! + pub include_attributes: bool, +} + +impl SchemataEq for StructFieldEq { + fn compare(&self, schema_one: &Schema, schema_two: &Schema) -> bool { + macro_rules! compare_primitive { + ($primitive:ident) => { + if let Schema::$primitive = schema_one { + if let Schema::$primitive = schema_two { + return true; + } + return false; + } + }; + } + + if schema_one.name() != schema_two.name() { + return false; + } + + compare_primitive!(Null); + compare_primitive!(Boolean); + compare_primitive!(Int); + compare_primitive!(Int); + compare_primitive!(Long); + compare_primitive!(Float); + compare_primitive!(Double); + compare_primitive!(Bytes); + compare_primitive!(String); + compare_primitive!(Uuid); + compare_primitive!(BigDecimal); + compare_primitive!(Date); + compare_primitive!(Duration); + compare_primitive!(TimeMicros); + compare_primitive!(TimeMillis); + compare_primitive!(TimestampMicros); + compare_primitive!(TimestampMillis); + compare_primitive!(TimestampNanos); + compare_primitive!(LocalTimestampMicros); + compare_primitive!(LocalTimestampMillis); + compare_primitive!(LocalTimestampNanos); + + if self.include_attributes + && schema_one.custom_attributes() != schema_two.custom_attributes() + { + return false; + } + + if let Schema::Record(RecordSchema { + fields: fields_one, .. + }) = schema_one + { + if let Schema::Record(RecordSchema { + fields: fields_two, .. + }) = schema_two + { + return self.compare_fields(fields_one, fields_two); + } + return false; + } + + if let Schema::Enum(EnumSchema { + symbols: symbols_one, + .. + }) = schema_one + { + if let Schema::Enum(EnumSchema { + symbols: symbols_two, + .. + }) = schema_two + { + return symbols_one == symbols_two; + } + return false; + } + + if let Schema::Fixed(FixedSchema { size: size_one, .. }) = schema_one { + if let Schema::Fixed(FixedSchema { size: size_two, .. }) = schema_two { + return size_one == size_two; + } + return false; + } + + if let Schema::Union(UnionSchema { + schemas: schemas_one, + .. + }) = schema_one + { + if let Schema::Union(UnionSchema { + schemas: schemas_two, + .. + }) = schema_two + { + return schemas_one.len() == schemas_two.len() + && schemas_one + .iter() + .zip(schemas_two.iter()) + .all(|(s1, s2)| self.compare(s1, s2)); + } + return false; + } + + if let Schema::Decimal(DecimalSchema { + precision: precision_one, + scale: scale_one, + .. + }) = schema_one + { + if let Schema::Decimal(DecimalSchema { + precision: precision_two, + scale: scale_two, + .. + }) = schema_two + { + return precision_one == precision_two && scale_one == scale_two; + } + return false; + } + + if let Schema::Array(ArraySchema { + items: items_one, .. + }) = schema_one + { + if let Schema::Array(ArraySchema { + items: items_two, .. + }) = schema_two + { + return items_one == items_two; + } + return false; + } + + if let Schema::Map(MapSchema { + types: types_one, .. + }) = schema_one + { + if let Schema::Map(MapSchema { + types: types_two, .. + }) = schema_two + { + return self.compare(types_one, types_two); + } + return false; + } + + if let Schema::Ref { name: name_one } = schema_one { + if let Schema::Ref { name: name_two } = schema_two { + return name_one == name_two; + } + return false; + } + + error!( + "This is a bug in schema_equality.rs! The following schemata types are not checked! \ + Please report it to the Avro library maintainers! \ + \n{:?}\n\n{:?}", + schema_one, schema_two + ); + false + } +} + +impl StructFieldEq { + fn compare_fields(&self, fields_one: &[RecordField], fields_two: &[RecordField]) -> bool { + fields_one.len() == fields_two.len() + && fields_one + .iter() + .zip(fields_two.iter()) + .all(|(f1, f2)| self.compare(&f1.schema, &f2.schema)) + } +} + +static SCHEMATA_COMPARATOR_ONCE: OnceLock> = OnceLock::new(); + +/// Sets a custom schemata equality comparator. +/// +/// Returns a unit if the registration was successful or the already +/// registered comparator if the registration failed. +/// +/// **Note**: This function must be called before parsing any schema because this will +/// register the default comparator and the registration is one time only! +pub fn set_schemata_equality_comparator( + comparator: Box, +) -> Result<(), Box> { + debug!( + "Setting a custom schemata equality comparator: {:?}.", + comparator + ); + SCHEMATA_COMPARATOR_ONCE.set(comparator) +} + +pub(crate) fn compare_schemata(schema_one: &Schema, schema_two: &Schema) -> bool { + SCHEMATA_COMPARATOR_ONCE + .get_or_init(|| { + debug!("Going to use the default schemata equality comparator: SpecificationEq.",); + Box::new(StructFieldEq { + include_attributes: false, + }) + }) + .compare(schema_one, schema_two) +} + +#[cfg(test)] +#[allow(non_snake_case)] +mod tests { + use super::*; + use crate::schema::{Name, RecordFieldOrder}; + use apache_avro_test_helper::TestResult; + use serde_json::Value; + use std::collections::BTreeMap; + + const SPECIFICATION_EQ: SpecificationEq = SpecificationEq; + const STRUCT_FIELD_EQ: StructFieldEq = StructFieldEq { + include_attributes: false, + }; + + macro_rules! test_primitives { + ($primitive:ident) => { + paste::item! { + #[test] + fn []() { + let specification_eq_res = SPECIFICATION_EQ.compare(&Schema::$primitive, &Schema::$primitive); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&Schema::$primitive, &Schema::$primitive); + assert_eq!(specification_eq_res, struct_field_eq_res) + } + } + }; + } + + test_primitives!(Null); + test_primitives!(Boolean); + test_primitives!(Int); + test_primitives!(Long); + test_primitives!(Float); + test_primitives!(Double); + test_primitives!(Bytes); + test_primitives!(String); + test_primitives!(Uuid); + test_primitives!(BigDecimal); + test_primitives!(Date); + test_primitives!(Duration); + test_primitives!(TimeMicros); + test_primitives!(TimeMillis); + test_primitives!(TimestampMicros); + test_primitives!(TimestampMillis); + test_primitives!(TimestampNanos); + test_primitives!(LocalTimestampMicros); + test_primitives!(LocalTimestampMillis); + test_primitives!(LocalTimestampNanos); + + #[test] + fn test_avro_3939_compare_named_schemata_with_different_names() { + let schema_one = Schema::Ref { + name: Name::from("name1"), + }; + + let schema_two = Schema::Ref { + name: Name::from("name2"), + }; + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + assert!(!specification_eq_res); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!(!struct_field_eq_res); + + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_schemata_not_including_attributes() { + let schema_one = Schema::map_with_attributes( + Schema::Boolean, + BTreeMap::from_iter([("key1".to_string(), Value::Bool(true))]), + ); + let schema_two = Schema::map_with_attributes( + Schema::Boolean, + BTreeMap::from_iter([("key2".to_string(), Value::Bool(true))]), + ); + // STRUCT_FIELD_EQ does not include attributes ! + assert!(STRUCT_FIELD_EQ.compare(&schema_one, &schema_two)); + } + + #[test] + fn test_avro_3939_compare_schemata_including_attributes() { + let struct_field_eq = StructFieldEq { + include_attributes: true, + }; + let schema_one = Schema::map_with_attributes( + Schema::Boolean, + BTreeMap::from_iter([("key1".to_string(), Value::Bool(true))]), + ); + let schema_two = Schema::map_with_attributes( + Schema::Boolean, + BTreeMap::from_iter([("key2".to_string(), Value::Bool(true))]), + ); + assert!(!struct_field_eq.compare(&schema_one, &schema_two)); + } + + #[test] + fn test_avro_3939_compare_map_schemata() { + let schema_one = Schema::map(Schema::Boolean); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::map(Schema::Boolean); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Map failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Map failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_array_schemata() { + let schema_one = Schema::array(Schema::Boolean); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::array(Schema::Boolean); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Array failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Array failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_decimal_schemata() { + let schema_one = Schema::Decimal(DecimalSchema { + precision: 10, + scale: 2, + inner: Box::new(Schema::Bytes), + }); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::Decimal(DecimalSchema { + precision: 10, + scale: 2, + inner: Box::new(Schema::Bytes), + }); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Decimal failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Decimal failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_fixed_schemata() { + let schema_one = Schema::Fixed(FixedSchema { + name: Name::from("fixed"), + doc: None, + size: 10, + aliases: None, + attributes: BTreeMap::new(), + }); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::Fixed(FixedSchema { + name: Name::from("fixed"), + doc: None, + size: 10, + aliases: None, + attributes: BTreeMap::new(), + }); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Fixed failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Fixed failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_enum_schemata() { + let schema_one = Schema::Enum(EnumSchema { + name: Name::from("enum"), + doc: None, + symbols: vec!["A".to_string(), "B".to_string()], + default: None, + aliases: None, + attributes: BTreeMap::new(), + }); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::Enum(EnumSchema { + name: Name::from("enum"), + doc: None, + symbols: vec!["A".to_string(), "B".to_string()], + default: None, + aliases: None, + attributes: BTreeMap::new(), + }); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Enum failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Enum failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_ref_schemata() { + let schema_one = Schema::Ref { + name: Name::from("ref"), + }; + 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"), + }; + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Ref failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Ref failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_record_schemata() { + let schema_one = Schema::Record(RecordSchema { + name: Name::from("record"), + doc: None, + fields: vec![RecordField { + name: "field".to_string(), + doc: None, + default: None, + schema: Schema::Boolean, + order: RecordFieldOrder::Ignore, + aliases: None, + custom_attributes: BTreeMap::new(), + position: 0, + }], + aliases: None, + attributes: BTreeMap::new(), + lookup: Default::default(), + }); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::Record(RecordSchema { + name: Name::from("record"), + doc: None, + fields: vec![RecordField { + name: "field".to_string(), + doc: None, + default: None, + schema: Schema::Boolean, + order: RecordFieldOrder::Ignore, + aliases: None, + custom_attributes: BTreeMap::new(), + position: 0, + }], + aliases: None, + attributes: BTreeMap::new(), + lookup: Default::default(), + }); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Record failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Record failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + } + + #[test] + fn test_avro_3939_compare_union_schemata() -> TestResult { + let schema_one = Schema::Union(UnionSchema::new(vec![Schema::Boolean, Schema::Int])?); + assert!(!SPECIFICATION_EQ.compare(&schema_one, &Schema::Boolean)); + assert!(!STRUCT_FIELD_EQ.compare(&schema_one, &Schema::Boolean)); + + let schema_two = Schema::Union(UnionSchema::new(vec![Schema::Boolean, Schema::Int])?); + + let specification_eq_res = SPECIFICATION_EQ.compare(&schema_one, &schema_two); + let struct_field_eq_res = STRUCT_FIELD_EQ.compare(&schema_one, &schema_two); + assert!( + specification_eq_res, + "SpecificationEq: Equality of two Schema::Union failed!" + ); + assert!( + struct_field_eq_res, + "StructFieldEq: Equality of two Schema::Union failed!" + ); + assert_eq!(specification_eq_res, struct_field_eq_res); + Ok(()) + } +} diff --git a/lang/rust/avro/src/types.rs b/lang/rust/avro/src/types.rs index 028c66188c1..8bac843bb9c 100644 --- a/lang/rust/avro/src/types.rs +++ b/lang/rust/avro/src/types.rs @@ -31,7 +31,6 @@ use serde_json::{Number, Value as JsonValue}; use std::{ borrow::Borrow, collections::{BTreeMap, HashMap}, - convert::TryFrom, fmt::Debug, hash::BuildHasher, str::FromStr, @@ -1150,10 +1149,8 @@ impl Value { mod tests { use super::*; use crate::{ - decimal::Decimal, - duration::{Days, Duration, Millis, Months}, - schema::{Name, RecordField, RecordFieldOrder, Schema, UnionSchema}, - types::Value, + duration::{Days, Millis, Months}, + schema::RecordFieldOrder, }; use apache_avro_test_helper::{ logger::{assert_logged, assert_not_logged}, @@ -1162,7 +1159,6 @@ mod tests { use num_bigint::BigInt; use pretty_assertions::assert_eq; use serde_json::json; - use uuid::Uuid; #[test] fn avro_3809_validate_nested_records_with_implicit_namespace() -> TestResult { diff --git a/lang/rust/avro/src/util.rs b/lang/rust/avro/src/util.rs index 2ea134c77a6..7afcc926b64 100644 --- a/lang/rust/avro/src/util.rs +++ b/lang/rust/avro/src/util.rs @@ -18,7 +18,6 @@ use crate::{schema::Documentation, AvroResult, Error}; use serde_json::{Map, Value}; use std::{ - convert::TryFrom, i64, io::Read, sync::{ diff --git a/lang/rust/avro/src/writer.rs b/lang/rust/avro/src/writer.rs index 446a4c0ef39..dc6fd55e83d 100644 --- a/lang/rust/avro/src/writer.rs +++ b/lang/rust/avro/src/writer.rs @@ -25,7 +25,7 @@ use crate::{ AvroResult, Codec, Error, }; use serde::Serialize; -use std::{collections::HashMap, convert::TryFrom, io::Write, marker::PhantomData}; +use std::{collections::HashMap, io::Write, marker::PhantomData}; const DEFAULT_BLOCK_SIZE: usize = 16000; const AVRO_OBJECT_HEADER: &[u8] = b"Obj\x01"; diff --git a/lang/rust/avro_test_helper/src/logger.rs b/lang/rust/avro_test_helper/src/logger.rs index f1bb5f84d0f..8617358959a 100644 --- a/lang/rust/avro_test_helper/src/logger.rs +++ b/lang/rust/avro_test_helper/src/logger.rs @@ -68,8 +68,21 @@ pub fn assert_not_logged(unexpected_message: &str) { } pub fn assert_logged(expected_message: &str) { - let last_message = LOG_MESSAGES.with(|msgs| msgs.borrow_mut().pop().unwrap()); - assert_eq!(last_message, expected_message); + let mut deleted = false; + LOG_MESSAGES.with(|msgs| { + msgs.borrow_mut().retain(|msg| { + if msg == expected_message { + deleted = true; + false + } else { + true + } + }) + }); + + if !deleted { + panic!("Expected log message has not been logged: '{expected_message}'"); + } } #[cfg(not(target_arch = "wasm32"))]