From d5d5e72f9c00715da15dd57c0daa9e34d6c287c3 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 09:32:37 -0500 Subject: [PATCH 1/8] Verify behavior of re-abstracted Object method --- .../TestAbstractClassesWithOverrides.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) 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 82deaac4..0eb71784 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 @@ -42,6 +42,11 @@ public abstract static class CoffeeBean extends Bean { public abstract static class PeruvianCoffeeBean extends CoffeeBean {} + public abstract static class StringlessCoffeeBean extends CoffeeBean + { + @Override public abstract String toString(); + } + /* /********************************************************** @@ -51,8 +56,7 @@ public abstract static class PeruvianCoffeeBean extends CoffeeBean {} public void testOverrides() throws Exception { - ObjectMapper mapper = new ObjectMapper(); - mapper.registerModule(new MrBeanModule()); + ObjectMapper mapper = newMrBeanMapper(); Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBean.class); verifyBean(bean); @@ -61,6 +65,19 @@ public void testOverrides() throws Exception verifyBean(bean2); } + public void testReAbstractedMethods() throws Exception + { + ObjectMapper mapper = newMrBeanMapper(); + + Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", StringlessCoffeeBean.class); + verifyBean(bean); + try { + assertNotNull(bean.toString()); + } catch (UnsupportedOperationException e) { + verifyException(e, "Unimplemented method 'toString'"); + } + } + private void verifyBean(Bean bean) { assertNotNull(bean); assertEquals("abc", bean.getX()); From c852089390222f01ae3f8515a0c76ca63a83da83 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 10:38:05 -0500 Subject: [PATCH 2/8] Verify behavior of re-abstracted property --- .../mrbean/TestAbstractClassesWithOverrides.java | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 0eb71784..f74ba42d 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 @@ -47,6 +47,11 @@ public abstract static class StringlessCoffeeBean extends CoffeeBean @Override public abstract String toString(); } + public abstract static class CoffeeBeanWithVariableFoo extends CoffeeBean + { + @Override public abstract String getFoo(); + } + /* /********************************************************** @@ -76,6 +81,12 @@ public void testReAbstractedMethods() throws Exception } catch (UnsupportedOperationException e) { verifyException(e, "Unimplemented method 'toString'"); } + + // Ensure that the re-abstracted method will read "foo" from the JSON + Bean beanWithNoFoo = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBeanWithVariableFoo.class); + assertNull(beanWithNoFoo.getFoo()); + Bean beanWithOtherFoo = mapper.readValue("{ \"foo\": \"Another Foo!\", \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBeanWithVariableFoo.class); + assertEquals("Another Foo!", beanWithOtherFoo.getFoo()); } private void verifyBean(Bean bean) { From 7203d7adad4a6f856638d8670742fabb6d2ed73a Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 10:41:00 -0500 Subject: [PATCH 3/8] Be explicit about deferred failure --- .../module/mrbean/TestAbstractClassesWithOverrides.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 f74ba42d..21165613 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 @@ -72,7 +72,11 @@ public void testOverrides() throws Exception public void testReAbstractedMethods() throws Exception { - ObjectMapper mapper = newMrBeanMapper(); + AbstractTypeMaterializer mat = new AbstractTypeMaterializer(); + // ensure that we will only get deferred error methods + mat.disable(AbstractTypeMaterializer.Feature.FAIL_ON_UNMATERIALIZED_METHOD); + ObjectMapper mapper = new ObjectMapper() + .registerModule(new MrBeanModule(mat)); Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", StringlessCoffeeBean.class); verifyBean(bean); From 4929d6632c882ce3099fc713664667f29587f3e3 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 10:53:03 -0500 Subject: [PATCH 4/8] Check behavior for re-abstract method (currently throws java.lang.AbstractMethodError) --- .../mrbean/TestAbstractClassesWithOverrides.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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 21165613..c91fb37b 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 @@ -1,5 +1,6 @@ package com.fasterxml.jackson.module.mrbean; +import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; public class TestAbstractClassesWithOverrides @@ -47,6 +48,11 @@ public abstract static class StringlessCoffeeBean extends CoffeeBean @Override public abstract String toString(); } + public abstract static class UnroastableCoffeeBean extends CoffeeBean + { + @Override public abstract String roast(int temperature); + } + public abstract static class CoffeeBeanWithVariableFoo extends CoffeeBean { @Override public abstract String getFoo(); @@ -82,10 +88,19 @@ public void testReAbstractedMethods() throws Exception verifyBean(bean); try { assertNotNull(bean.toString()); + fail("Should not pass"); } catch (UnsupportedOperationException e) { verifyException(e, "Unimplemented method 'toString'"); } + Bean unroastableBean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", UnroastableCoffeeBean.class); + try { + unroastableBean.roast(123); + fail("Should not pass"); + } catch (UnsupportedOperationException e) { + verifyException(e, "Unimplemented method 'roast'"); + } + // Ensure that the re-abstracted method will read "foo" from the JSON Bean beanWithNoFoo = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBeanWithVariableFoo.class); assertNull(beanWithNoFoo.getFoo()); From d4a99dde650e11539eef7a9ce2b1e23648423035 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 10:59:02 -0500 Subject: [PATCH 5/8] Check behavior for re-abstract method with eager failure (currently does not throw on custom method) --- .../TestAbstractClassesWithOverrides.java | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) 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 c91fb37b..87308d1d 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 @@ -84,6 +84,8 @@ public void testReAbstractedMethods() throws Exception ObjectMapper mapper = new ObjectMapper() .registerModule(new MrBeanModule(mat)); + verifyReAbstractedProperty(mapper); + Bean bean = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", StringlessCoffeeBean.class); verifyBean(bean); try { @@ -100,14 +102,41 @@ public void testReAbstractedMethods() throws Exception } catch (UnsupportedOperationException e) { verifyException(e, "Unimplemented method 'roast'"); } + } - // Ensure that the re-abstracted method will read "foo" from the JSON + // Ensures that the re-abstracted method will read "foo" from the JSON, regardless of the FAIL_ON_UNMATERIALIZED_METHOD setting + private void verifyReAbstractedProperty(ObjectMapper mapper) throws com.fasterxml.jackson.core.JsonProcessingException { Bean beanWithNoFoo = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBeanWithVariableFoo.class); assertNull(beanWithNoFoo.getFoo()); Bean beanWithOtherFoo = mapper.readValue("{ \"foo\": \"Another Foo!\", \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBeanWithVariableFoo.class); assertEquals("Another Foo!", beanWithOtherFoo.getFoo()); } + public void testEagerFailureOnReAbstractedMethods() throws Exception + { + AbstractTypeMaterializer mat = new AbstractTypeMaterializer(); + // ensure that we will get eager failure on abstract methods + mat.enable(AbstractTypeMaterializer.Feature.FAIL_ON_UNMATERIALIZED_METHOD); + ObjectMapper mapper = new ObjectMapper() + .registerModule(new MrBeanModule(mat)); + + verifyReAbstractedProperty(mapper); + + try { + mapper.readValue("{}", StringlessCoffeeBean.class); + fail("Should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized abstract method 'toString'"); + } + + try { + mapper.readValue("{}", UnroastableCoffeeBean.class); + fail("Should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized abstract method 'roast'"); + } + } + private void verifyBean(Bean bean) { assertNotNull(bean); assertEquals("abc", bean.getX()); From a3ff1a7acf9e69fbf871aef77c2d847ecfdbf3b3 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 11:00:47 -0500 Subject: [PATCH 6/8] Always check concreteness of method found via getMethod --- .../com/fasterxml/jackson/module/mrbean/BeanBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 36c963da..2af0e332 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 @@ -189,9 +189,9 @@ protected boolean hasConcreteOverride(Method m0, JavaType implementedType) // 22-Sep-2020: [modules-base#109]: 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; - } + + // we've found the method, so we can simply check whether it is concrete + return BeanUtil.isConcrete(effectiveMethod); } catch (NoSuchMethodException e) { // method must be non-public, fallback to using getDeclaredMethod } From 6536463bc01cf2dcc5b065e65b45e6611cf5cc35 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 11:08:19 -0500 Subject: [PATCH 7/8] Check behavior for re-abstracted non-public method (currently broken) --- .../TestAbstractClassesWithOverrides.java | 28 +++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) 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 87308d1d..df157411 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 @@ -43,6 +43,15 @@ public abstract static class CoffeeBean extends Bean { public abstract static class PeruvianCoffeeBean extends CoffeeBean {} + /* + * Test classes where some concrete method has been re-"abstract"-ed + */ + + public abstract static class CoffeeBeanWithVariableFoo extends CoffeeBean + { + @Override public abstract String getFoo(); + } + public abstract static class StringlessCoffeeBean extends CoffeeBean { @Override public abstract String toString(); @@ -53,9 +62,9 @@ public abstract static class UnroastableCoffeeBean extends CoffeeBean @Override public abstract String roast(int temperature); } - public abstract static class CoffeeBeanWithVariableFoo extends CoffeeBean + public abstract static class CoffeeBeanLackingPublicMethod extends CoffeeBean { - @Override public abstract String getFoo(); + @Override protected abstract Object protectedAbstractMethod(); } @@ -102,6 +111,14 @@ public void testReAbstractedMethods() throws Exception } catch (UnsupportedOperationException e) { verifyException(e, "Unimplemented method 'roast'"); } + + Bean beanLackingNonPublicMethod = mapper.readValue("{ \"x\" : \"abc\", \"y\" : 13, \"z\" : \"def\" }", CoffeeBeanLackingPublicMethod.class); + try { + beanLackingNonPublicMethod.customMethod(); + fail("Should not pass"); + } catch (UnsupportedOperationException e) { + verifyException(e, "Unimplemented method 'protectedAbstractMethod'"); + } } // Ensures that the re-abstracted method will read "foo" from the JSON, regardless of the FAIL_ON_UNMATERIALIZED_METHOD setting @@ -135,6 +152,13 @@ public void testEagerFailureOnReAbstractedMethods() throws Exception } catch (JsonMappingException e) { verifyException(e, "Unrecognized abstract method 'roast'"); } + + try { + mapper.readValue("{}", CoffeeBeanLackingPublicMethod.class); + fail("Should not pass"); + } catch (JsonMappingException e) { + verifyException(e, "Unrecognized abstract method 'protectedAbstractMethod'"); + } } private void verifyBean(Bean bean) { From 90d902a5396ed9b2476c65e12dbb41df4ed2ac71 Mon Sep 17 00:00:00 2001 From: robbytx Date: Wed, 23 Sep 2020 11:08:46 -0500 Subject: [PATCH 8/8] Always check concreteness of method when found --- .../com/fasterxml/jackson/module/mrbean/BeanBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 2af0e332..9cd5e343 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 @@ -202,9 +202,9 @@ protected boolean hasConcreteOverride(Method m0, JavaType implementedType) // be better here? try { Method effectiveMethod = curr.getRawClass().getDeclaredMethod(name, argTypes); - if (BeanUtil.isConcrete(effectiveMethod)) { - return true; - } + + // we've found the method, so we can simply check whether it is concrete + return BeanUtil.isConcrete(effectiveMethod); } catch (NoSuchMethodException e) { // method must exist on a superclass, continue searching... }