(avro) 01/01: AVRO-3946: [Rust] Log warning/error for strange union types

[mg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mgrigorov pushed a commit to branch avro-3946-log-warn-error-for-strange-unions
in repository https://gitbox.apache.org/repos/asf/avro.git

commit 57de9d7f9d8a6766962a515d68e8f92030249a80
Author: Martin Tzvetanov Grigorov <mg...@apache.org>
AuthorDate: Mon Feb 26 10:44:02 2024 +0200

    AVRO-3946: [Rust] Log warning/error for strange union types
    
    A Union schema without any variants will log an ERROR from now on.
    A Union schema with a single variant will log a WARN suggesting to drop
    the union.
    
    Signed-off-by: Martin Tzvetanov Grigorov <mg...@apache.org>
---
 lang/rust/avro/src/schema.rs | 71 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/lang/rust/avro/src/schema.rs b/lang/rust/avro/src/schema.rs
index 58bdc6ab1..f16b002ae 100644
--- a/lang/rust/avro/src/schema.rs
+++ b/lang/rust/avro/src/schema.rs
@@ -1606,6 +1606,8 @@ impl Parser {
 
         self.register_resolving_schema(&fully_qualified_name, &aliases);
 
+        debug!("Going to parse record schema: {:?}", &fully_qualified_name);
+
         let fields: Vec<RecordField> = fields_opt
             .and_then(|fields| fields.as_array())
             .ok_or(Error::GetRecordFieldsJson)
@@ -1786,7 +1788,22 @@ impl Parser {
             .iter()
             .map(|v| self.parse(v, enclosing_namespace))
             .collect::<Result<Vec<_>, _>>()
-            .and_then(|schemas| Ok(Schema::Union(UnionSchema::new(schemas)?)))
+            .and_then(|schemas| {
+                if schemas.is_empty() {
+                    error!(
+                        "Union schemas should have at least two members! \
+                    Please enable debug logging to find out which Record schema \
+                    declares the union with 'RUST_LOG=apache_avro::schema=debug'."
+                    );
+                } else if schemas.len() == 1 {
+                    warn!(
+                        "Union schema with just one member! Consider dropping the union! \
+                    Please enable debug logging to find out which Record schema \
+                    declares the union with 'RUST_LOG=apache_avro::schema=debug'."
+                    );
+                }
+                Ok(Schema::Union(UnionSchema::new(schemas)?))
+            })
     }
 
     /// Parse a `serde_json::Value` representing a Avro fixed type into a
@@ -6639,4 +6656,56 @@ mod tests {
 
         Ok(())
     }
+
+    #[test]
+    fn avro_3946_union_with_single_type() -> TestResult {
+        let schema = r#"
+        {
+          "type": "record",
+          "name": "Issue",
+          "namespace": "invalid.example",
+          "fields": [
+            {
+              "name": "myField",
+              "type": ["long"]
+            }
+          ]
+        }"#;
+
+        let _ = Schema::parse_str(schema)?;
+
+        assert_logged(
+            "Union schema with just one member! Consider dropping the union! \
+                    Please enable debug logging to find out which Record schema \
+                    declares the union with 'RUST_LOG=apache_avro::schema=debug'.",
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn avro_3946_union_without_any_types() -> TestResult {
+        let schema = r#"
+        {
+          "type": "record",
+          "name": "Issue",
+          "namespace": "invalid.example",
+          "fields": [
+            {
+              "name": "myField",
+              "type": []
+            }
+          ]
+        }"#;
+
+        let _ = Schema::parse_str(schema)?;
+
+        assert_logged(
+            "Union schemas should have at least two members! \
+                    Please enable debug logging to find out which Record schema \
+                    declares the union with 'RUST_LOG=apache_avro::schema=debug'.",
+        );
+
+        Ok(())
+    }
 }