Skip to content

Commit

Permalink
Port tests from apache/avro test_schema.py and implement schema equal…
Browse files Browse the repository at this point in the history
…ity based on parsing canonical form (#97)

I noticed that we were deriving PartialEq for Schema while my understanding
is that the spec asserts that equality between schemas has to rely on
parsing canonical form. So I manually impemented PartialEq and Eq based
on that.

I also created a bunch of issues to follow with the tests that I had to
comment because they were failing.
  • Loading branch information
poros authored Oct 20, 2019
1 parent c4971ac commit 3a93d16
Show file tree
Hide file tree
Showing 4 changed files with 783 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ mod tests {
#[test]
fn test_reader_empty_buffer() {
let empty = Cursor::new(Vec::new());
Reader::new(empty).is_err();
assert!(Reader::new(empty).is_err());
}

#[test]
Expand Down
20 changes: 16 additions & 4 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use digest::Digest;
use failure::{Error, Fail};
use serde::{
ser::{SerializeMap, SerializeSeq},
Serialize, Serializer,
Deserialize, Serialize, Serializer,
};
use serde_json::{self, Map, Value};

Expand Down Expand Up @@ -52,7 +52,7 @@ impl fmt::Display for SchemaFingerprint {
/// Represents any valid Avro schema
/// More information about Avro schemas can be found in the
/// [Avro Specification](https://avro.apache.org/docs/current/spec.html#schemas)
#[derive(Clone, Debug, PartialEq)]
#[derive(Clone, Debug)]
pub enum Schema {
/// A `null` Avro schema.
Null,
Expand Down Expand Up @@ -101,6 +101,18 @@ pub enum Schema {
Fixed { name: Name, size: usize },
}

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
fn eq(&self, other: &Self) -> bool {
self.canonical_form() == other.canonical_form()
}
}

impl Eq for Schema {}

/// This type is used to simplify enum variant comparison between `Schema` and `types::Value`.
/// It may have utility as part of the public API, but defining as `pub(crate)` for now.
///
Expand Down Expand Up @@ -182,7 +194,7 @@ impl<'a> From<&'a types::Value> for SchemaKind {
///
/// More information about schema names can be found in the
/// [Avro specification](https://avro.apache.org/docs/current/spec.html#names)
#[derive(Clone, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq, Deserialize)]
pub struct Name {
pub name: String,
pub namespace: Option<String>,
Expand Down Expand Up @@ -380,6 +392,7 @@ impl PartialEq for UnionSchema {
impl Schema {
/// Create a `Schema` from a string representing a JSON Avro schema.
pub fn parse_str(input: &str) -> Result<Self, Error> {
// TODO: (#82) this should be a ParseSchemaError wrapping the JSON error
let value = serde_json::from_str(input)?;
Self::parse(&value)
}
Expand Down Expand Up @@ -981,5 +994,4 @@ mod tests {
format!("{}", schema.fingerprint::<Md5>())
);
}

}
6 changes: 5 additions & 1 deletion tests/io.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Port of https://github.com/apache/avro/blob/master/lang/py/test/test_io.py
//! Port of https://github.com/apache/avro/blob/release-1.9.1/lang/py/test/test_io.py
use std::io::Cursor;

use avro_rs::{
Expand Down Expand Up @@ -40,15 +40,19 @@ lazy_static! {
(r#""null""#, "null", Value::Null),
(r#""boolean""#, "true", Value::Boolean(true)),
(r#""string""#, r#""foo""#, Value::String("foo".to_string())),
// TODO: (#96) investigate why this is failing
//(r#""bytes""#, r#""\u00FF\u00FF""#, Value::Bytes(vec![0xff, 0xff])),
(r#""int""#, "5", Value::Int(5)),
(r#""long""#, "5", Value::Long(5)),
(r#""float""#, "1.1", Value::Float(1.1)),
(r#""double""#, "1.1", Value::Double(1.1)),
// TODO: (#96) investigate why this is failing
//(r#"{"type": "fixed", "name": "F", "size": 2}"#, r#""\u00FF\u00FF""#, Value::Bytes(vec![0xff, 0xff])),
// TODO: (#96) investigate why this is failing
//(r#"{"type": "enum", "name": "F", "symbols": ["FOO", "BAR"]}"#, r#""FOO""#, Value::Enum(0, "FOO".to_string())),
(r#"{"type": "array", "items": "int"}"#, "[1, 2, 3]", Value::Array(vec![Value::Int(1), Value::Int(2), Value::Int(3)])),
(r#"{"type": "map", "values": "int"}"#, r#"{"a": 1, "b": 2}"#, Value::Map([("a".to_string(), Value::Int(1)), ("b".to_string(), Value::Int(2))].iter().cloned().collect())),
// TODO: (#96) investigate why this is failing
//(r#"["int", "null"]"#, "5", Value::Union(Box::new(Value::Int(5)))),
(r#"{"type": "record", "name": "F", "fields": [{"name": "A", "type": "int"}]}"#, r#"{"A": 5}"#,Value::Record(vec![("A".to_string(), Value::Int(5))])),
];
Expand Down
Loading

0 comments on commit 3a93d16

Please sign in to comment.