From 498cac38caa91394de5d27afb5af6d5058c5b021 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 2 Sep 2020 20:21:24 -0500 Subject: [PATCH 1/5] Verify default methods in interface are supported --- .../TestSimpleMaterializedInterfaces.java | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java index 7072c6d6..b8be0287 100644 --- a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java +++ b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java @@ -28,13 +28,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(); } @@ -55,7 +55,13 @@ public interface ArrayBean { interface NonPublicBean { public abstract int getX(); } - + + public interface ExtendedBean extends Bean { + public default boolean anyValuePresent() { + return getX() > 0 || getA() != null; + } + } + /* /********************************************************** /* Unit tests, low level @@ -149,8 +155,8 @@ public void testBeanHolder() throws Exception assertNotNull(bean); assertEquals("b", bean.getA()); assertEquals(-4, bean.getX()); - } - + } + public void testArrayInterface() throws Exception { ObjectMapper mapper = newMrBeanMapper(); @@ -170,7 +176,17 @@ public void testSubInterface() throws Exception assertEquals(1, bean.getX()); assertEquals(2, bean.getY()); } - + + public void testDefaultMethods() throws Exception + { + ObjectMapper mapper = newMrBeanMapper(); + ExtendedBean bean = mapper.readValue("{\"a\":\"value\",\"x\":123 }", ExtendedBean.class); + assertNotNull(bean); + assertEquals("value", bean.getA()); + assertEquals(123, bean.getX()); + assertTrue(bean.anyValuePresent()); + } + /* /********************************************************** /* Unit tests, higher level, error handling @@ -210,6 +226,6 @@ public void testNonPublic() throws Exception } catch (JsonMappingException e) { verifyException(e, "is not public"); } - } - + } + } From 22b8550a9917d4129c8e03cb7048d377bc0382c0 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 2 Sep 2020 20:28:20 -0500 Subject: [PATCH 2/5] Add failing test case --- .../TestSimpleMaterializedInterfaces.java | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java index b8be0287..39335123 100644 --- a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java +++ b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestSimpleMaterializedInterfaces.java @@ -56,12 +56,20 @@ interface NonPublicBean { public abstract int getX(); } - public interface ExtendedBean extends Bean { + 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 @@ -177,10 +185,20 @@ public void testSubInterface() throws Exception assertEquals(2, bean.getY()); } - public void testDefaultMethods() throws Exception + 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(); - ExtendedBean bean = mapper.readValue("{\"a\":\"value\",\"x\":123 }", ExtendedBean.class); + BeanWithInheritedDefault bean = mapper.readValue("{\"a\":\"value\",\"x\":123 }", BeanWithInheritedDefault.class); assertNotNull(bean); assertEquals("value", bean.getA()); assertEquals(123, bean.getX()); From 66df757ba9eb53c9fe5ed59fd1b83cdc5d48886b Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 2 Sep 2020 20:44:36 -0500 Subject: [PATCH 3/5] Verify non-public class method behavior --- .../jackson/module/mrbean/TestAbstractClasses.java | 10 ++++++++-- .../mrbean/TestAbstractClassesWithOverrides.java | 12 +++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java index 9d4ffda5..68169699 100644 --- a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java +++ b/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 Object customMethod() { return new Object(); } } /* @@ -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()); + assertNotNull(bean.customMethod()); } } diff --git a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java index 50f5b500..aa2bb4f3 100644 --- a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java +++ b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java @@ -16,13 +16,17 @@ 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 new Object(); } } public abstract static class CoffeeBean extends Bean { @@ -43,9 +47,9 @@ public abstract static class PeruvianCoffeeBean extends CoffeeBean {} public void testOverrides() throws Exception { ObjectMapper mapper = newMrBeanMapper(); - 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); } @@ -54,6 +58,8 @@ private void verifyBean(Bean bean) { assertEquals("abc", bean.getX()); assertEquals(13, bean.y); assertEquals("Foo!", bean.getFoo()); + assertEquals("def", bean.getZ()); + assertNotNull(bean.customMethod()); assertEquals("The coffee beans are roasting at 123 degrees now, yummy", bean.roast(123)); } } From f8a9a0d74068518c6fe2d18882cda2537dc1d1ee Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 2 Sep 2020 20:56:41 -0500 Subject: [PATCH 4/5] Verify current behavior of non-public methods --- .../jackson/module/mrbean/TestAbstractClasses.java | 4 ++-- .../module/mrbean/TestAbstractClassesWithOverrides.java | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java index 68169699..1307a412 100644 --- a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java +++ b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClasses.java @@ -24,7 +24,7 @@ protected Bean() { } // also verify non-public methods protected abstract String getZ(); - private Object customMethod() { return new Object(); } + private String customMethod() { return "Private methods rock!"; } } /* @@ -42,6 +42,6 @@ public void testSimpleInteface() throws Exception assertEquals(13, bean.y); assertEquals("Foo!", bean.getFoo()); assertEquals("def", bean.getZ()); - assertNotNull(bean.customMethod()); + assertEquals("Private methods rock!", bean.customMethod()); } } diff --git a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java index aa2bb4f3..411f840a 100644 --- a/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java +++ b/mrbean/src/test/java/com/fasterxml/jackson/module/mrbean/TestAbstractClassesWithOverrides.java @@ -26,13 +26,18 @@ protected Bean() { } // also verify non-public methods protected abstract String getZ(); - private Object customMethod() { return new Object(); } + 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 {} @@ -59,7 +64,7 @@ private void verifyBean(Bean bean) { assertEquals(13, bean.y); assertEquals("Foo!", bean.getFoo()); assertEquals("def", bean.getZ()); - assertNotNull(bean.customMethod()); 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()); } } From 73b8f0ca5b0ad2454eabbd7b6669603a70782a6b Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 2 Sep 2020 21:01:06 -0500 Subject: [PATCH 5/5] Use Class.getMethod to locate the precise implementation, including default methods in Java 8+ interfaces --- .../jackson/module/mrbean/BeanBuilder.java | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/mrbean/src/main/java/com/fasterxml/jackson/module/mrbean/BeanBuilder.java b/mrbean/src/main/java/com/fasterxml/jackson/module/mrbean/BeanBuilder.java index 0fc20126..551f8485 100644 --- a/mrbean/src/main/java/com/fasterxml/jackson/module/mrbean/BeanBuilder.java +++ b/mrbean/src/main/java/com/fasterxml/jackson/module/mrbean/BeanBuilder.java @@ -176,16 +176,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; } @@ -240,7 +252,7 @@ protected final static boolean returnsBoolean(Method m) Class rt = m.getReturnType(); return (rt == Boolean.class || rt == Boolean.TYPE); } - + /* /********************************************************** /* Internal methods, bytecode generation @@ -288,7 +300,7 @@ private DynamicType.Builder createSetter(DynamicType.Builder builder, /* Internal methods, other /********************************************************** */ - + protected String decap(String name) { char c = name.charAt(0); if (name.length() > 1