martin-g commented on code in PR #3317:
URL: https://github.com/apache/avro/pull/3317#discussion_r2454697352


##########
lang/ruby/lib/avro/schema_validator.rb:
##########
@@ -198,23 +198,38 @@ def validate_union(expected_schema, datum, path, result, 
options)
         compatible_type = first_compatible_type(datum, expected_schema, path, 
failures, options)
         return unless compatible_type.nil?
 
-        complex_type_failed = failures.detect { |r| 
COMPLEX_TYPES.include?(r[:type]) }
-        if complex_type_failed
-          complex_type_failed[:result].errors.each { |error| result << error }
-        else
-          types = expected_schema.schemas.map { |s| "'#{s.type_sym}'" 
}.join(', ')
-          result.add_error(path, "expected union of [#{types}], got 
#{actual_value_message(datum)}")
+

Review Comment:
   ```suggestion
   ```



##########
lang/ruby/lib/avro/schema_validator.rb:
##########
@@ -198,23 +198,38 @@ def validate_union(expected_schema, datum, path, result, 
options)
         compatible_type = first_compatible_type(datum, expected_schema, path, 
failures, options)
         return unless compatible_type.nil?
 
-        complex_type_failed = failures.detect { |r| 
COMPLEX_TYPES.include?(r[:type]) }
-        if complex_type_failed
-          complex_type_failed[:result].errors.each { |error| result << error }
-        else
-          types = expected_schema.schemas.map { |s| "'#{s.type_sym}'" 
}.join(', ')
-          result.add_error(path, "expected union of [#{types}], got 
#{actual_value_message(datum)}")
+
+        failed_complex_types = failures.select { |r| 
COMPLEX_TYPES.include?(r[:type]) }
+        complex_type_errors =  []

Review Comment:
   ```suggestion
           complex_type_errors = []
   ```



##########
lang/ruby/lib/avro/schema_validator.rb:
##########
@@ -198,23 +198,38 @@ def validate_union(expected_schema, datum, path, result, 
options)
         compatible_type = first_compatible_type(datum, expected_schema, path, 
failures, options)
         return unless compatible_type.nil?
 
-        complex_type_failed = failures.detect { |r| 
COMPLEX_TYPES.include?(r[:type]) }
-        if complex_type_failed
-          complex_type_failed[:result].errors.each { |error| result << error }
-        else
-          types = expected_schema.schemas.map { |s| "'#{s.type_sym}'" 
}.join(', ')
-          result.add_error(path, "expected union of [#{types}], got 
#{actual_value_message(datum)}")
+
+        failed_complex_types = failures.select { |r| 
COMPLEX_TYPES.include?(r[:type]) }
+        complex_type_errors =  []
+        failed_complex_types.each do |failed_complex_type|
+          error_msg = failed_complex_type[:result].errors.map do |error|
+            error
+          end.join("; ")
+          schema_name_prefix = "#{failed_complex_type[:schema_name]}: " if 
failed_complex_type[:schema_name] 

Review Comment:
   ```suggestion
             schema_name_prefix = "#{failed_complex_type[:schema_name]}: " if 
failed_complex_type[:schema_name]
   ```



##########
lang/ruby/test/test_schema_validator.rb:
##########
@@ -556,9 +561,51 @@ def test_validate_union_extra_fields
       validate!(schema, { 'name' => 'apple', 'color' => 'green' }, 
fail_on_extra_fields: true)
     end
     assert_equal(1, exception.result.errors.size)
-    assert_equal("at . extra field 'color' - not in schema", exception.to_s)
+    expected_error = <<~EXPECTED_ERROR.chomp
+      at . expected union of ['null', fruit ('record')], got record with value 
{"name"=>"apple", "color"=>"green"}
+      Union type specific errors:
+      fruit: at . extra field 'color' - not in schema
+    EXPECTED_ERROR
+    assert_equal(expected_error, exception.to_s)
   end
 
+  def test_validate_union_complex_and_simple_types
+    schema = hash_to_schema([
+                              'null',
+                              {
+                                type: 'record',
+                                name: 'fruit',
+                                fields: [{ name: 'name', type: 'string' }]
+                              },
+                              {
+                                type: 'record',
+                                name: 'animal',
+                                fields: [
+                                  { name: 'name', type: 'string' },
+                                  { name: 'species', type: 'string' }
+                                ]
+                              },
+                              {
+                                type: 'enum',

Review Comment:
   Are enums supported by the Ruby SDK ?
   
https://github.com/davidgm0/avro/blob/63796919c00bd2ac9fc3b82d7139a2432c09b3d6/lang/ruby/lib/avro/schema_validator.rb#L24
 does not list `:enum` ...



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to