Named schema resolution and docs updates (#4)

* chore: update public and internal docs

* Use order for field equality

* chore: writer docs

* bugfix: named schema resolution

Author: creativcoder

CHANGELOG.md

@@ -4,7 +4,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
-# [Unreleased]
+## [Unreleased]
+
+### Fixed
+- Named schema resolution outside of union variants.
+
+### Updated
+- Documentation.
 
 ## 0.2.0 - 2020-10-10
 

CONTRIBUTING.md

@@ -1,7 +1,29 @@
 
 # Contributing
 
-When contributing to this repository, please first discuss the change you wish to make via issue,
+Some of the features of avrow are feature gated.
+While making changes it's a good idea to build and
+test with `--all-features` flag.
+
+## Building the project
+
+```
+cargo build --all-features
+```
+
+## Running test cases
+
+```
+cargo test --all-features
+```
+
+## Generating and opening documentation locally
+
+```
+BROWSER=firefox cargo doc --no-deps --open
+```
+
+When contributing to this repository, please discuss the change you wish to make via issue,
 email, or any other method with the owners of this repository before making a change. 
 
 Please note we have a [code of conduct](./CODE_OF_CONDUCT.md), please follow it in all your interactions with the project.

README.md

@@ -64,7 +64,7 @@ The Avro specification provides two kinds of encoding:
 
 This crate implements only the binary encoding as that's the format practically used for performance and storage reasons.
 
-## Features.
+## Features
 
 * Full support for recursive self-referential schemas with Serde serialization/deserialization.
 * All compressions codecs (`deflate`, `bzip2`, `snappy`, `xz`, `zstd`) supported as per spec.
@@ -139,7 +139,7 @@ fn main() -> Result<(), Error> {
 
 ```
 
-A more involved self-referential recursive schema example:
+### Self-referential recursive schema example
 
 ```rust
 use anyhow::Error;
@@ -202,7 +202,7 @@ fn main() -> Result<(), Error> {
 
 ```
 
-An example of writing a json object with a confirming schema. The json object maps to an `avrow::Record` type.
+### An example of writing a json object with a confirming schema. The json object maps to the `avrow::Record` type.
 
 ```rust
 use anyhow::Error;

src/lib.rs

@@ -1,16 +1,16 @@
 //! Avrow is a pure Rust implementation of the [Apache Avro specification](https://avro.apache.org/docs/current/spec.html).
 //!
-//! Pleaes refer to the [README](https://github.com/creativcoder/avrow/blob/main/README.md) for an overview.
+//! Please refer to the [README](https://github.com/creativcoder/avrow/blob/main/README.md) for an overview.
 //! For more details on the spec, head over to the [FAQ](https://cwiki.apache.org/confluence/display/AVRO/FAQ).
 //!
 //! ## Using the library
 //!
-//! Add to your `Cargo.toml`:
+//! Add avrow to your `Cargo.toml`:
 //!```toml
 //! [dependencies]
 //! avrow = "0.2.0"
 //!```
-//! ### A hello world example of reading and writing avro data files
+//! ## A hello world example of reading and writing avro data files
 
 //!```rust
 //! use avrow::{Reader, Schema, Writer, from_value};
@@ -26,7 +26,7 @@
 //!     let mut writer = Writer::new(&schema, vec![])?;
 //!     // Write data using write
 //!     writer.write(())?;
-//!     // or serialize
+//!     // or serialize via serde
 //!     writer.serialize(())?;
 //!     // retrieve the underlying buffer using the into_inner method.
 //!     let buf = writer.into_inner()?;
@@ -49,7 +49,6 @@
 
 //!```
 
-// TODO update logo
 #![doc(
     html_favicon_url = "https://raw.githubusercontent.com/creativcoder/avrow/main/assets/avrow_logo.png"
 )]

src/reader.rs

@@ -485,7 +485,7 @@ pub(crate) fn decode_with_resolution<R: Read>(
 pub(crate) fn decode<R: Read>(
     schema: &Variant,
     reader: &mut R,
-    r_cxt: &Registry,
+    w_cxt: &Registry,
 ) -> Result<Value, AvrowErr> {
     let value = match schema {
         Variant::Null => Value::Null,
@@ -538,7 +538,7 @@ pub(crate) fn decode<R: Read>(
 
             let mut it = Vec::with_capacity(block_count as usize);
             for _ in 0..block_count {
-                let decoded = decode(&**items, reader, r_cxt)?;
+                let decoded = decode(&**items, reader, w_cxt)?;
                 it.push(decoded);
             }
 
@@ -550,7 +550,7 @@ pub(crate) fn decode<R: Read>(
             let mut hm = HashMap::new();
             for _ in 0..block_count {
                 let key = decode_string(reader)?;
-                let value = decode(values, reader, r_cxt)?;
+                let value = decode(values, reader, w_cxt)?;
                 hm.insert(key, value);
             }
 
@@ -560,7 +560,7 @@ pub(crate) fn decode<R: Read>(
             let mut v = IndexMap::with_capacity(fields.len());
             for (field_name, field) in fields {
                 let field_name = field_name.to_string();
-                let field_value = decode(&field.ty, reader, r_cxt)?;
+                let field_value = decode(&field.ty, reader, w_cxt)?;
                 let field_value = FieldValue::new(field_value);
                 v.insert(field_name, field_value);
             }
@@ -571,15 +571,22 @@ pub(crate) fn decode<R: Read>(
             };
             Value::Record(rec)
         }
+        Variant::Fixed { size, .. } => {
+            let mut buf = vec![0; *size];
+            reader
+                .read_exact(&mut buf)
+                .map_err(AvrowErr::DecodeFailed)?;
+            Value::Fixed(buf)
+        }
         Variant::Union { variants } => {
             let variant_idx: i64 = reader.read_varint().map_err(AvrowErr::DecodeFailed)?;
-            decode(&variants[variant_idx as usize], reader, r_cxt)?
+            decode(&variants[variant_idx as usize], reader, w_cxt)?
         }
         Variant::Named(schema_name) => {
-            let schema_variant = r_cxt
+            let schema_variant = w_cxt
                 .get(schema_name)
                 .ok_or(AvrowErr::NamedSchemaNotFound)?;
-            decode(schema_variant, reader, r_cxt)?
+            decode(schema_variant, reader, w_cxt)?
         }
         a => {
             return Err(AvrowErr::DecodeFailed(Error::new(

src/schema/canonical.rs

@@ -27,8 +27,7 @@ const RELEVANT_FIELDS: [&str; 7] = [
     "name", "type", "fields", "symbols", "items", "values", "size",
 ];
 /// Represents canonical form of an avro schema. This representation removes irrelevant fields
-/// such as docs and aliases in the schema.
-/// Fingerprinting methods are available on this instance.
+/// such as docs and aliases in the schema. Fingerprinting methods are available on this instance.
 #[derive(Debug, PartialEq)]
 pub struct CanonicalSchema(pub(crate) JsonValue);
 

src/schema/common.rs

@@ -1,5 +1,5 @@
 // This module contains definition of types that are common across a subset of
-// avro schemas.
+// avro Schema implementation.
 
 use crate::error::AvrowErr;
 use crate::schema::Variant;
@@ -33,16 +33,16 @@ pub(crate) fn validate_namespace(s: &str) -> Result<(), AvrowErr> {
     Ok(())
 }
 
-/// Represents `fullname` attribute and its constituents
-/// of a named avro type i.e, Record, Fixed and Enum
+/// Represents the `fullname` attribute
+/// of a named avro type i.e, Record, Fixed and Enum.
 #[derive(Debug, Clone, Eq, PartialOrd, Ord)]
 pub struct Name {
     pub(crate) name: String,
     pub(crate) namespace: Option<String>,
 }
 
 impl Name {
-    // Creates an validates the name. This will also extract the namespace if a dot is present in `name`
+    // Creates a new name with validation. This will extract the namespace if a dot is present in `name`
     // Any further calls to set_namespace, will be a noop if the name already contains a dot.
     pub(crate) fn new(name: &str) -> Result<Self, AvrowErr> {
         let mut namespace = None;
@@ -56,10 +56,6 @@ impl Name {
             validate_name(0, name)?;
             name
         } else {
-            // TODO perform namespace lookups from enclosing schema if any
-            // This will require us to pass context to this method.
-            // Update: this is now handled by from_json method as that's called from places
-            // where we have context on most tightly enclosing schema.
             validate_name(0, name)?;
             name
         };
@@ -70,7 +66,6 @@ impl Name {
         })
     }
 
-    // TODO also parse namespace from json value
     pub(crate) fn from_json(
         json: &serde_json::map::Map<String, JsonValue>,
         enclosing_namespace: Option<&str>,
@@ -105,7 +100,6 @@ impl Name {
     }
 
     // receives a mutable json and parses a Name and removes namespace. Used for canonicalization.
-    // TODO change as above from_json method, should take enclosing namespace.
     pub(crate) fn from_json_mut(
         json: &mut serde_json::map::Map<String, JsonValue>,
         enclosing_namespace: Option<&str>,
@@ -129,13 +123,6 @@ impl Name {
             }
         }
 
-        // if let Some(namespace) = json.get("namespace") {
-        //     if let JsonValue::String(s) = namespace {
-        //         name.set_namespace(s)?;
-        //         json.remove("namespace");
-        //     }
-        // }
-
         Ok(name)
     }
 
@@ -156,24 +143,8 @@ impl Name {
     }
 
     // TODO according to Rust convention, item path separators are :: instead of .
-    // TODO should we add a configurable separator.
-    // TODO should do namespace lookup from enclosing name schema if applicable. (pass enclosing schema as a context)
+    // should we add a configurable separator?
     pub(crate) fn fullname(&self) -> String {
-        // if self.name.contains(".") {
-        //     self.name.to_string()
-        // } else if let Some(n) = &self.namespace {
-        //     if n.is_empty() {
-        //         // According to spec, it's fine to put "" as a namespace, which becomes a null namespace
-        //         format!("{}", self.name)
-        //     } else {
-        //         format!("{}.{}", n, self.name)
-        //     }
-        // } else {
-        //     // The case when only name exists.
-        //     // TODO As of now we just return without any enclosing namespace.
-        //     // TODO pass the most tightly enclosing namespace here when only name is provided.
-        //     self.name.to_string()
-        // }
         if let Some(n) = &self.namespace {
             if n.is_empty() {
                 // According to spec, it's fine to put "" as a namespace, which becomes a null namespace
@@ -255,10 +226,9 @@ pub struct Field {
     pub(crate) aliases: Option<Vec<String>>,
 }
 
-// TODO do we also use order for equality?
 impl std::cmp::PartialEq for Field {
     fn eq(&self, other: &Self) -> bool {
-        self.name == other.name && self.ty == other.ty
+        self.name == other.name && self.ty == other.ty && self.order == other.order
     }
 }
 
@@ -270,6 +240,7 @@ impl Field {
         order: Order,
         aliases: Option<Vec<String>>,
     ) -> Result<Self, AvrowErr> {
+        // According to spec, field names also must adhere to a valid nane.
         validate_name(0, name)?;
         Ok(Field {
             name: name.to_string(),

src/schema/mod.rs

@@ -22,7 +22,6 @@ use std::fmt::Debug;
 use std::fs::OpenOptions;
 use std::path::Path;
 
-/// A schema parsed from json value
 #[derive(Debug, Clone, PartialEq)]
 pub(crate) enum Variant {
     Null,
@@ -59,7 +58,7 @@ pub(crate) enum Variant {
     Named(String),
 }
 
-/// Represents the avro schema used to write encoded avro data
+/// Represents the avro schema used to write encoded avro data.
 #[derive(Debug)]
 pub struct Schema {
     // TODO can remove this if not needed
@@ -80,7 +79,7 @@ impl PartialEq for Schema {
 
 impl std::str::FromStr for Schema {
     type Err = AvrowErr;
-    /// Parse an avro schema from a json string
+    /// Parse an avro schema from a JSON string
     /// One can use Rust's raw string syntax (r##""##) to pass schema.
     fn from_str(schema: &str) -> Result<Self, Self::Err> {
         let schema_json =
@@ -90,8 +89,9 @@ impl std::str::FromStr for Schema {
 }
 
 impl Schema {
-    /// Parses an avro schema from a json description of schema in a file.
-    /// Alternatively, one can use the `FromStr` impl to create a `Schema` from a JSON string:
+    /// Parses an avro schema from a JSON schema in a file.
+    /// Alternatively, one can use the [`FromStr`](https://doc.rust-lang.org/std/str/trait.FromStr.html)
+    /// impl to create the Schema from a JSON string:
     /// ```
     /// use std::str::FromStr;
     /// use avrow::Schema;
@@ -134,7 +134,8 @@ impl Schema {
         self.variant.validate(value, &self.cxt)
     }
 
-    /// Returns the canonical form of an Avro schema
+    /// Returns the canonical form of an Avro schema.
+    /// Example:
     /// ```rust
     /// use avrow::Schema;
     /// use std::str::FromStr;
@@ -150,6 +151,7 @@ impl Schema {
     ///         }]
     ///     }
     /// "##).unwrap();
+    ///
     /// let canonical = schema.canonical_form();
     /// ```
     pub fn canonical_form(&self) -> &CanonicalSchema {

src/schema/parser.rs

@@ -304,8 +304,8 @@ impl Registry {
 
 // Parses the `order` of a field, defaults to `ascending` order
 pub(crate) fn parse_field_order(order: &JsonValue) -> AvrowResult<Order> {
-    match *order {
-        JsonValue::String(ref s) => match &**s {
+    match order {
+        JsonValue::String(s) => match s.as_ref() {
             "ascending" => Ok(Order::Ascending),
             "descending" => Ok(Order::Descending),
             "ignore" => Ok(Order::Ignore),
@@ -439,7 +439,7 @@ mod tests {
             "value",
             Variant::Long,
             Some(Value::Long(1)),
-            Order::Ascending,
+            Order::Descending,
             None,
         )
         .unwrap();