[GitHub] [avro] jklamer commented on a diff in pull request #1636: AVRO-3483: [Rust] Log error messages with a reason when the validation fails

[GitBox <gi...@apache.org>]

jklamer commented on code in PR #1636:
URL: https://github.com/apache/avro/pull/1636#discussion_r846702372


##########
lang/rust/avro/src/types.rs:
##########
@@ -365,50 +378,129 @@ impl Value {
             (&Value::Bytes(_), &Schema::Decimal { .. }) => true,
             (&Value::String(_), &Schema::String) => true,
             (&Value::String(_), &Schema::Uuid) => true,
-            (&Value::Fixed(n, _), &Schema::Fixed { size, .. }) => n == size,
-            (&Value::Bytes(ref b), &Schema::Fixed { size, .. }) => b.len() == size,
-            (&Value::Fixed(n, _), &Schema::Duration) => n == 12,
+            (&Value::Fixed(n, _), &Schema::Fixed { size, .. }) => {
+                let res = n == size;
+                if !res {
+                    reason = format!(
+                        "The value's size ({}) is different than the schema's size ({})",
+                        n, size
+                    );
+                }
+                res
+            }
+            (&Value::Bytes(ref b), &Schema::Fixed { size, .. }) => {
+                let res = b.len() == size;
+                if !res {
+                    reason = format!(
+                        "The bytes' length ({}) is different than the schema's size ({})",
+                        b.len(),
+                        size
+                    );
+                }
+                res
+            }
+            (&Value::Fixed(n, _), &Schema::Duration) => {
+                let res = n == 12;
+                if !res {
+                    reason = format!(
+                        "The value's size ('{}') must be exactly 12 to be a Duration",
+                        n
+                    );
+                }
+                res
+            }
             // TODO: check precision against n
             (&Value::Fixed(_n, _), &Schema::Decimal { .. }) => true,
-            (&Value::String(ref s), &Schema::Enum { ref symbols, .. }) => symbols.contains(s),
+            (&Value::String(ref s), &Schema::Enum { ref symbols, .. }) => {
+                let res = symbols.contains(s);
+                if !res {
+                    reason = format!("'{}' is not a member of the possible symbols", s);
+                }
+                res
+            }
             (&Value::Enum(i, ref s), &Schema::Enum { ref symbols, .. }) => symbols
                 .get(i as usize)
-                .map(|ref symbol| symbol == &s)
-                .unwrap_or(false),
+                .map(|ref symbol| {
+                    let res = symbol == &s;
+                    if !res {
+                        reason = format!("Symbol '{}' is not at position '{}'", s, i);
+                    }
+                    res
+                })
+                .unwrap_or_else(|| {
+                    reason = format!("No symbol at position '{}'", i);
+                    false
+                }),
             // (&Value::Union(None), &Schema::Union(_)) => true,
             (&Value::Union(i, ref value), &Schema::Union(ref inner)) => inner
                 .variants()
                 .get(i as usize)
-                .map(|schema| value.validate_internal(schema, names))
+                .map(|schema| {
+                    parent_or_leaf_check = false;
+                    value.validate_internal(schema, names)
+                })
                 .unwrap_or(false),

Review Comment:
   I believe you will need to have a reason here similar to the above if the index out of range
   ```
   .unwrap_or_else(|| {
                       reason = format!("No symbol at position '{}'", i);
                       false
                   }),
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org