Improve Avro schema compatibility check error message

Author: cowtowncoder

avro/src/main/java/com/fasterxml/jackson/dataformat/avro/AvroSchema.java

@@ -1,8 +1,10 @@
 package com.fasterxml.jackson.dataformat.avro;
 
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.apache.avro.Schema;
+import org.apache.avro.Schema.Type;
 import org.apache.avro.SchemaCompatibility;
 import org.apache.avro.SchemaCompatibility.SchemaCompatibilityType;
 import org.apache.avro.SchemaCompatibility.SchemaPairCompatibility;
@@ -75,21 +77,50 @@ public AvroSchema withReaderSchema(AvroSchema readerSchema)
         w = Schema.applyAliases(w, r);
 
         // and then use Avro std lib to validate compatibility
+
+        // 16-Jun-2017, tatu: First, a very common case is for Record names not
+        //   to match; so let's check that first
+        if (r.getType() == w.getType()) {
+            if (!_schemaNamesEqual(w, r)) {
+                throw new JsonMappingException(null, String.format(
+"Incompatible writer/reader schemas: root %ss have different names (\"%s\" vs \"%s\"), no match via aliases",
+r.getType().getName(), w.getFullName(), r.getFullName()));
+            }
+        }
+        
         SchemaPairCompatibility comp;
         try {
             comp = SchemaCompatibility.checkReaderWriterCompatibility(r, w);
         } catch (Exception e) {
             throw new JsonMappingException(null, String.format(
-                    "Failed to resolve given reader/writer schemas, problem: (%s) %s",
+                    "Failed to resolve given writer/reader schemas, problem: (%s) %s",
                     e.getClass().getName(), e.getMessage()));
         }
         if (comp.getType() != SchemaCompatibilityType.COMPATIBLE) {
-            throw new JsonMappingException(null, String.format("Incompatible reader/writer schema: %s",
+            throw new JsonMappingException(null, String.format("Incompatible writer/reader schemas: %s",
                     comp.getDescription()));
         }
         return Resolving.create(w, r);
     }
 
+    private boolean _schemaNamesEqual(Schema w, Schema r)
+    {
+        final String wname = w.getFullName();
+        final String rname = r.getFullName();
+
+        if ((wname == rname) ||
+                ((wname != null) && wname.equals(rname))) {
+            return true;
+        }
+
+        // but may also have alias. NOTE! Avro lib itself does this, and we rely
+        // on it, but basically only `NamedSchema` do NOT throw exception. But
+        // we have no way of checking -- need to trust other cases bail out before
+        // this (which they do). Unclean but... that's avrolib for you.
+        Set<String> aliases = r.getAliases();
+        return aliases.contains(wname);
+    }
+
     /**
      * Similar to {@link #withReaderSchema} but will NOT verify compatibility of schemas:
      * this means that certain problems (such as missing default value for a newly added

avro/src/test/java/com/fasterxml/jackson/dataformat/avro/schemaev/SimpleEvolutionTest.java

@@ -20,7 +20,7 @@ public class SimpleEvolutionTest extends AvroTestBase
             "    { 'name':'x', 'type':'int' }\n"+
             " ]\n"+
             "}\n");
-    
+
     static String SCHEMA_XY_JSON = aposToQuotes("{\n"+
             " 'type':'record',\n"+
             " 'name':'EvolvingPoint',\n"+
@@ -40,6 +40,16 @@ public class SimpleEvolutionTest extends AvroTestBase
             " ]\n"+
             "}\n");
 
+    static String SCHEMA_XYZ_RENAMED_JSON = aposToQuotes("{\n"+
+            " 'type':'record',\n"+
+            " 'name':'EvolvingPoint2',\n"+
+            " 'fields':[\n"+
+            "    { 'name':'z', 'type':'int', 'default': 99 },\n"+
+            "    { 'name':'y', 'type':'int' },\n"+
+            "    { 'name':'x', 'type':'int' }\n"+
+            " ]\n"+
+            "}\n");
+    
     static class PointXY {
         public int x, y;
 
@@ -174,8 +184,9 @@ public void testFailNewFieldNoDefault() throws Exception
             srcSchema.withReaderSchema(dstSchema);
             fail("Should not pass");
         } catch (JsonMappingException e) {
-            // 06-Feb-2016, tatu: Extremely lame error message by avro lib. Should consider
+            // 06-Feb-2017, tatu: Extremely lame error message by avro lib. Should consider
             //    rewriting to give some indication of issues...
+            verifyException(e, "Incompatible writer/reader schemas");
             verifyException(e, "Data encoded using writer schema");
             verifyException(e, "will or may fail to decode using reader schema");
         }
@@ -184,4 +195,23 @@ public void testFailNewFieldNoDefault() throws Exception
         AvroSchema risky = srcSchema.withUnsafeReaderSchema(dstSchema);
         assertNotNull(risky);
     }
+
+    public void testFailNameChangeNoAlias() throws Exception
+    {
+        final AvroSchema srcSchema = MAPPER.schemaFrom(SCHEMA_XYZ_JSON);
+        final AvroSchema dstSchema = MAPPER.schemaFrom(SCHEMA_XYZ_RENAMED_JSON);
+        try {
+            srcSchema.withReaderSchema(dstSchema);
+            fail("Should not pass");
+        } catch (JsonMappingException e) {
+            // 16-Jun-2017, tatu: As is, names must match, so...
+            verifyException(e, "Incompatible writer/reader schemas");
+            verifyException(e, "root records have different names");
+            verifyException(e, "\"EvolvingPoint\"");
+            verifyException(e, "\"EvolvingPoint2\"");
+        }
+        // However... should be possible with unsafe alternative
+        AvroSchema risky = srcSchema.withUnsafeReaderSchema(dstSchema);
+        assertNotNull(risky);
+    }
 }