Merge pull request #109 from bazaarvoice/patch-default-in-interfaces

MrBean: Fix detection of inherited default method in Java 8+ interface

Author: cowtowncoder

mrbean/pom.xml

@@ -51,6 +51,16 @@ ${project.groupId}.mrbean.*;version=${project.version}
 	<artifactId>replacer</artifactId>
       </plugin>
 
+      <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-compiler-plugin</artifactId>
+          <configuration>
+              <!-- Enable testing with Java 8 features, especially default methods in interfaces -->
+              <testSource>1.8</testSource>
+              <testTarget>1.8</testTarget>
+          </configuration>
+      </plugin>
+
       <plugin>
         <!--  We will shade ASM, to simplify deployment, avoid version conflicts -->
         <groupId>org.apache.maven.plugins</groupId>

mrbean/src/main/java/com/fasterxml/jackson/module/mrbean/BeanBuilder.java

@@ -181,16 +181,28 @@ protected boolean hasConcreteOverride(Method m0, JavaType implementedType)
     {
         final String name = m0.getName();
         final Class<?>[] argTypes = m0.getParameterTypes();
+        try {
+            // getMethod returns the most-specific method implementation, for public methods only (which is any method in an interface)
+            Method effectiveMethod = implementedType.getRawClass().getMethod(name, argTypes);
+            if (BeanUtil.isConcrete(effectiveMethod)) {
+                return true;
+            }
+        } catch (NoSuchMethodException e) {
+            // method must be non-public, fallback to using getDeclaredMethod
+        }
+
         for (JavaType curr = implementedType; (curr != null) && !curr.isJavaLangObject();
                 curr = curr.getSuperClass()) {
             // 29-Nov-2015, tatu: Avoiding exceptions would be good, so would linear scan
             //    be better here?
             try {
                 Method effectiveMethod = curr.getRawClass().getDeclaredMethod(name, argTypes);
-                if (effectiveMethod != null && BeanUtil.isConcrete(effectiveMethod)) {
+                if (BeanUtil.isConcrete(effectiveMethod)) {
                     return true;
                 }
-            } catch (NoSuchMethodException e) { }
+            } catch (NoSuchMethodException e) {
+                // method must exist on a superclass, continue searching...
+            }
         }
         return false;
     }
@@ -245,7 +257,7 @@ protected final static boolean returnsBoolean(Method m)
         Class<?> rt = m.getReturnType();
         return (rt == Boolean.class || rt == Boolean.TYPE);
     }
-    
+
     /*
     /**********************************************************
     /* Internal methods, bytecode generation
@@ -354,7 +366,7 @@ protected void createUnimplementedMethod(ClassWriter cw, String internalClassNam
     /* Internal methods, other
     /**********************************************************
      */
-    
+
     protected String decap(String name) {
         char c = name.charAt(0);
         if (name.length() > 1

mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java

@@ -16,11 +16,15 @@ public abstract static class Bean
         int y;
 
         protected Bean() { }
-        
+
         public abstract String getX();
 
         public String getFoo() { return "Foo!"; }
         public void setY(int value) { y = value; }
+
+        // also verify non-public methods
+        protected abstract String getZ();
+        private String customMethod() { return "Private methods rock!"; }
     }
 
     /*
@@ -32,10 +36,12 @@ protected Bean() { }
     public void testSimpleInteface() throws Exception
     {
         ObjectMapper mapper = newMrBeanMapper();
-        Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13 }", Bean.class);
+        Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", Bean.class);
         assertNotNull(bean);
         assertEquals("abc", bean.getX());
         assertEquals(13, bean.y);
         assertEquals("Foo!", bean.getFoo());
+        assertEquals("def", bean.getZ());
+        assertEquals("Private methods rock!", bean.customMethod());
     }
 }

mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java

@@ -16,19 +16,28 @@ public abstract static class Bean
         int y;
 
         protected Bean() { }
-        
+
         public abstract String getX();
 
         public abstract String roast(int temperature);
 
         public String getFoo() { return "Foo!"; }
         public void setY(int value) { y = value; }
+
+        // also verify non-public methods
+        protected abstract String getZ();
+        private Object customMethod() { return protectedAbstractMethod(); }
+        protected abstract Object protectedAbstractMethod();
     }
 
     public abstract static class CoffeeBean extends Bean {
         @Override public String roast(int temperature) {
             return "The coffee beans are roasting at " + temperature + " degrees now, yummy";
         }
+
+        @Override protected Object protectedAbstractMethod() {
+            return "Private methods invoking protected abstract methods is the bomb!";
+        }
     }
 
     public abstract static class PeruvianCoffeeBean extends CoffeeBean {}
@@ -45,10 +54,10 @@ public void testOverrides() throws Exception
         ObjectMapper mapper = new ObjectMapper();
         mapper.registerModule(new MrBeanModule());
 
-        Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13 }", CoffeeBean.class);
+        Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBean.class);
         verifyBean(bean);
 
-        Bean bean2 = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13 }", PeruvianCoffeeBean.class);
+        Bean bean2 = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", PeruvianCoffeeBean.class);
         verifyBean(bean2);
     }
 
@@ -57,6 +66,8 @@ private void verifyBean(Bean bean) {
         assertEquals("abc", bean.getX());
         assertEquals(13, bean.y);
         assertEquals("Foo!", bean.getFoo());
+        assertEquals("def", bean.getZ());
         assertEquals("The coffee beans are roasting at 123 degrees now, yummy", bean.roast(123));
+        assertEquals("Private methods invoking protected abstract methods is the bomb!", bean.customMethod());
     }
 }

mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java

@@ -26,13 +26,13 @@ public interface BeanWithY extends Bean
     {
         public int getY();
     }
-    
+
     public interface PartialBean {
         public boolean isOk();
         // and then non-getter/setter one:
         public int foobar();
     }
-    
+
     public interface BeanHolder {
         public Bean getBean();
     }
@@ -53,7 +53,21 @@ public interface ArrayBean {
     interface NonPublicBean {
         public abstract int getX();
     }
-    
+
+    public interface OtherInterface {
+        public boolean anyValuePresent();
+    }
+
+    public interface BeanWithDefaultForOtherInterface extends Bean, OtherInterface {
+        public default boolean anyValuePresent() {
+            return getX() > 0 || getA() != null;
+        }
+    }
+
+    public interface BeanWithInheritedDefault extends BeanWithDefaultForOtherInterface {
+        // in this interface, anyValuePresent() is an inherited (rather than declared) concrete method
+    }
+
     /*
     /**********************************************************
     /* Unit tests, low level
@@ -149,8 +163,8 @@ public void testBeanHolder() throws Exception
         assertNotNull(bean);
         assertEquals("b", bean.getA());
         assertEquals(-4, bean.getX());
-    }    
-    
+    }
+
     public void testArrayInterface() throws Exception
     {
         ObjectMapper mapper = new ObjectMapper();
@@ -172,7 +186,27 @@ public void testSubInterface() throws Exception
         assertEquals(1, bean.getX());
         assertEquals(2, bean.getY());
     }
-    
+
+    public void testDefaultMethodInInterface() throws Exception
+    {
+        ObjectMapper mapper = newMrBeanMapper();
+        BeanWithDefaultForOtherInterface bean = mapper.readValue("{\"a\":\"value\",\"x\":123 }", BeanWithDefaultForOtherInterface.class);
+        assertNotNull(bean);
+        assertEquals("value", bean.getA());
+        assertEquals(123, bean.getX());
+        assertTrue(bean.anyValuePresent());
+    }
+
+    public void testInheritedDefaultMethodInInterface() throws Exception
+    {
+        ObjectMapper mapper = newMrBeanMapper();
+        BeanWithInheritedDefault bean = mapper.readValue("{\"a\":\"value\",\"x\":123 }", BeanWithInheritedDefault.class);
+        assertNotNull(bean);
+        assertEquals("value", bean.getA());
+        assertEquals(123, bean.getX());
+        assertTrue(bean.anyValuePresent());
+    }
+
     /*
     /**********************************************************
     /* Unit tests, higher level, error handling
@@ -212,6 +246,6 @@ public void testNonPublic() throws Exception
         } catch (JsonMappingException e) {
             verifyException(e, "is not public");
         }
-    }    
-    
+    }
+
 }