diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java index 172397f8..a171f545 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java @@ -2,8 +2,6 @@ import java.io.IOException; import java.lang.reflect.InvocationTargetException; -import java.util.logging.Level; -import java.util.logging.Logger; import com.fasterxml.jackson.core.*; @@ -26,13 +24,6 @@ abstract class OptimizedSettableBeanProperty _receiverType; /** - * Whether this is a property of a builder, whose setter failures are reported - * by {@link #_reportProblemAndReturn} from every path, {@link #set} included. + * Whether this is a property of a builder; if so, {@link #set} passes instances + * the generated setter can't accept (see {@link #_receiverType}) to the delegate. */ protected final boolean _forBuilder; @@ -116,71 +107,24 @@ public abstract Object deserializeSetAndReturn(JsonParser jp, */ /** - * Helper method called when an exception is throw from mutator, to figure - * out what to do. The parser is only used for a builder's error location, and - * is null when called from {@link #set}. - * - * @since 2.9 - */ - protected void _reportProblem(JsonParser p, Object bean, Object value, Throwable e) - throws IOException - { - // databind calls `set()` on builders too (for properties buffered before a - // property-based creator, or external type ids), which must behave like - // `setAndReturn`, regardless of the order of properties in the JSON - if (_forBuilder) { - _reportProblemAndReturn(p, e); - return; - } - if ((e instanceof IllegalAccessError) - || (e instanceof SecurityException)) { - _logAccessProblem(bean, e); - delegate.set(bean, value); - return; - } - if (e instanceof IOException) { - throw (IOException) e; - } - if (e instanceof Error) { - throw (Error) e; - } - if (e instanceof RuntimeException) { - throw (RuntimeException) e; - } - // how could this ever happen? - throw new RuntimeException(e); - } - - /** - * Like {@link #_reportProblem}, for builders: anything the generated - * setter throws is reported the way {@code MethodProperty} reports it, and never - * retried. Telling a failure of the generated accessor itself from one thrown by - * the setter would mean guessing from the stack trace, and a retry could run the - * setter twice; setup instead leaves the setters it can't call to reflection. + * Helper method called when an exception is thrown from mutator, to be thrown + * by the caller (which keeps the compiler aware it never returns): anything the + * generated setter throws is reported the way {@code MethodProperty} reports it, + * and never retried. Telling a failure of the generated accessor itself from one + * thrown by the setter would mean guessing from the stack trace, and a retry could + * run the setter twice; setup instead leaves the setters it can't call to reflection. * The parser is only used for the error location, and is null outside of * {@link #deserializeSetAndReturn} and {@link #deserializeAndSet}, as it is * for {@code MethodProperty}. + * + * @since 2.9 */ - protected Object _reportProblemAndReturn(JsonParser p, Throwable e) + protected IOException _reportProblem(JsonParser p, Throwable e) throws IOException { // `Method.invoke` wraps whatever the setter throws, and `MethodProperty` // translates that wrapper; doing the same keeps type, message, cause and path - throw _throwAsIOE(p, new InvocationTargetException(e)); - } - - private void _logAccessProblem(Object bean, Throwable e) { - synchronized (this) { - // yes, double-locking, so not guaranteed; but all we want here is to reduce likelihood - // of multiple logging of same underlying problem. Not guaranteed, just improved. - if (!loggedAccessProblem) { - loggedAccessProblem = true; - String msg = String.format("Blackbird deserialization for %s (field %s; mutator %s) falling back to reflection, due to access error (type %s, message=%s); further such errors are not logged%n", - bean.getClass(), _propName, getClass().getName(), - e.getClass().getName(), e.getMessage()); - Logger.getLogger(getClass().getName()).log(Level.WARNING, msg, e); - } - } + return _throwAsIOE(p, new InvocationTargetException(e)); } /* diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableBooleanProperty.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableBooleanProperty.java index d5e3aca1..a3a553b0 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableBooleanProperty.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableBooleanProperty.java @@ -48,7 +48,7 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, Object try { _optimizedSetter.accept(bean, b); } catch (Throwable e) { - _reportProblem(p, bean, b, e); + throw _reportProblem(p, e); } } @@ -82,7 +82,7 @@ public void set(Object bean, Object value) throws IOException { try { _optimizedSetter.accept(bean, b); } catch (Throwable e) { - _reportProblem(null, bean, b, e); + throw _reportProblem(null, e); } } @@ -104,7 +104,7 @@ private Object _setAndReturn(JsonParser p, Object instance, boolean b) throws IO Object result = _optimizedSetterAndReturn.apply(instance, b); return (result == null) ? instance : result; } catch (Throwable e) { - return _reportProblemAndReturn(p, e); + throw _reportProblem(p, e); } } } diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableIntProperty.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableIntProperty.java index 6f203a8d..b2f49c70 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableIntProperty.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableIntProperty.java @@ -46,7 +46,7 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, try { _optimizedSetter.accept(bean, v); } catch (Throwable e) { - _reportProblem(p, bean, v, e); + throw _reportProblem(p, e); } } @@ -76,7 +76,7 @@ public void set(Object bean, Object value) throws IOException { try { _optimizedSetter.accept(bean, v); } catch (Throwable e) { - _reportProblem(null, bean, v, e); + throw _reportProblem(null, e); } } @@ -98,7 +98,7 @@ private Object _setAndReturn(JsonParser p, Object instance, int v) throws IOExce Object result = _optimizedSetterAndReturn.apply(instance, v); return (result == null) ? instance : result; } catch (Throwable e) { - return _reportProblemAndReturn(p, e); + throw _reportProblem(p, e); } } } diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableLongProperty.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableLongProperty.java index 667551ec..dec00904 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableLongProperty.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableLongProperty.java @@ -46,7 +46,7 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, try { _optimizedSetter.accept(bean, v); } catch (Throwable e) { - _reportProblem(p, bean, v, e); + throw _reportProblem(p, e); } } @@ -75,7 +75,7 @@ public void set(Object bean, Object value) throws IOException { try { _optimizedSetter.accept(bean, v); } catch (Throwable e) { - _reportProblem(null, bean, v, e); + throw _reportProblem(null, e); } } @@ -97,7 +97,7 @@ private Object _setAndReturn(JsonParser p, Object instance, long v) throws IOExc Object result = _optimizedSetterAndReturn.apply(instance, v); return (result == null) ? instance : result; } catch (Throwable e) { - return _reportProblemAndReturn(p, e); + throw _reportProblem(p, e); } } } diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableObjectProperty.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableObjectProperty.java index 34da3984..a01b9746 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableObjectProperty.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableObjectProperty.java @@ -61,6 +61,8 @@ public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, } else { value = _valueDeserializer.deserializeWithType(p, ctxt, _valueTypeDeserializer); } + // Do not call set(): MethodProperty passes nulls returned by polymorphic + // deserialization to the setter, even with Nulls.SKIP. _set(p, bean, value); } @@ -118,7 +120,7 @@ private void _set(JsonParser p, Object bean, Object v) throws IOException { try { _optimizedSetter.accept(bean, v); } catch (Throwable e) { - _reportProblem(p, bean, v, e); + throw _reportProblem(p, e); } } @@ -143,7 +145,7 @@ private Object _setAndReturn(JsonParser p, Object instance, Object value) throws Object result = _optimizedSetterAndReturn.apply(instance, value); return (result == null) ? instance : result; } catch (Throwable e) { - return _reportProblemAndReturn(p, e); + throw _reportProblem(p, e); } } diff --git a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableStringProperty.java b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableStringProperty.java index cf56feee..b1b57b14 100644 --- a/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableStringProperty.java +++ b/blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableStringProperty.java @@ -38,11 +38,14 @@ protected SettableBeanProperty withDelegate(SettableBeanProperty del) { @Override public void deserializeAndSet(JsonParser p, DeserializationContext ctxt, Object bean) throws IOException { - if (!p.hasToken(JsonToken.VALUE_STRING)) { + String text = p.hasToken(JsonToken.VALUE_STRING) ? p.getText() : null; + // some parsers give null text (e.g. for a `TextNode(null)`), which needs the + // delegate's null handling + if (text == null) { delegate.deserializeAndSet(p, ctxt, bean); return; } - _set(p, bean, p.getText()); + _set(p, bean, text); } @Override @@ -80,7 +83,7 @@ private void _set(JsonParser p, Object bean, Object value) throws IOException { try { _optimizedSetter.accept(bean, (String) value); } catch (Throwable e) { - _reportProblem(p, bean, value, e); + throw _reportProblem(p, e); } } @@ -105,7 +108,7 @@ private Object _setAndReturn(JsonParser p, Object instance, Object value) throws Object result = _optimizedSetterAndReturn.apply(instance, value); return (result == null) ? instance : result; } catch (Throwable e) { - return _reportProblemAndReturn(p, e); + throw _reportProblem(p, e); } } } diff --git a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/TestAccessFallback.java b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/TestAccessFallback.java index b2f93352..c103e318 100644 --- a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/TestAccessFallback.java +++ b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/TestAccessFallback.java @@ -17,36 +17,36 @@ public BogusTestError(String msg) { } } + // Getter fails when called through the generated accessor, and works when + // called through reflection (the fallback writer) static class MyBean { private String e; + int optimizedCalls, reflectiveCalls; + public MyBean() { } MyBean(String e) { - setE(e); + this.e = e; } public void setE(String e) { - /* 07-Mar-2015, tatu: This is bit tricky, as the exact stack trace varies - * depending on how code is generated. So right now we must get the call - * immediately from constructed class. - */ - - StackTraceElement[] elems = new Throwable().getStackTrace(); - StackTraceElement prev = elems[1]; - if (prev.getClassName().contains("Access4JacksonDeserializer")) { - throw new BogusTestError("boom!"); - } this.e = e; } public String getE() { + // nearest of `Method.invoke` or a Blackbird frame tells which path called for (StackTraceElement elem : new Throwable().getStackTrace()) { - if (elem.getClassName().contains("Access4JacksonSerializer")) { + if (elem.getClassName().equals("java.lang.reflect.Method")) { + ++reflectiveCalls; + return e; + } + if (elem.getClassName().startsWith("com.fasterxml.jackson.module.blackbird.ser.")) { + ++optimizedCalls; throw new BogusTestError("boom!"); } } @@ -60,22 +60,17 @@ public String getE() public void testSerializeAccess() throws Exception { ObjectMapper abMapper = newObjectMapper(); - assertEquals(BEAN_JSON, abMapper.writeValueAsString(new MyBean("a"))); - - // actually try again, to ensure handling works reliably - assertEquals(BEAN_JSON, abMapper.writeValueAsString(new MyBean("a"))); - } - - @Test - public void testDeserializeAccess() throws Exception - { - ObjectMapper abMapper = newObjectMapper(); - MyBean bean = abMapper.readValue(BEAN_JSON, MyBean.class); - assertEquals("a", bean.getE()); - - // actually try again, to ensure handling works reliably - MyBean bean2 = abMapper.readValue(BEAN_JSON, MyBean.class); - assertEquals("a", bean2.getE()); + MyBean bean = new MyBean("a"); + assertEquals(BEAN_JSON, abMapper.writeValueAsString(bean)); + // the generated getter failed, and the value was then written through reflection + assertEquals(1, bean.optimizedCalls); + assertEquals(1, bean.reflectiveCalls); + + // after that, the writer stays on reflection + MyBean bean2 = new MyBean("a"); + assertEquals(BEAN_JSON, abMapper.writeValueAsString(bean2)); + assertEquals(0, bean2.optimizedCalls); + assertEquals(1, bean2.reflectiveCalls); } @Test diff --git a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/TestSetterFailures.java b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/TestSetterFailures.java new file mode 100644 index 00000000..4c9bc392 --- /dev/null +++ b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/TestSetterFailures.java @@ -0,0 +1,458 @@ +package com.fasterxml.jackson.module.blackbird.deser; + +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.lang.invoke.LambdaMetafactory; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; +import java.lang.reflect.Array; +import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.List; +import java.util.function.BiConsumer; +import java.util.function.BiFunction; +import java.util.function.Supplier; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; +import org.junit.jupiter.api.parallel.Execution; +import org.junit.jupiter.api.parallel.ExecutionMode; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import com.fasterxml.jackson.annotation.JsonTypeInfo; +import com.fasterxml.jackson.annotation.Nulls; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.deser.BeanDeserializerBase; +import com.fasterxml.jackson.databind.deser.DefaultDeserializationContext; +import com.fasterxml.jackson.databind.deser.SettableBeanProperty; +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; +import com.fasterxml.jackson.module.blackbird.BlackbirdTestBase; +import com.fasterxml.jackson.module.blackbird.deser.access.InaccessibleSetterValue; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +// The beans record calls and failures in static fields, so tests must not run concurrently +@Execution(ExecutionMode.SAME_THREAD) +public class TestSetterFailures extends BlackbirdTestBase +{ + private static Supplier failure; + private static Throwable lastFailure; + private static int calls; + + static class VoidBean { + public void setStr(String value) { failOnce(); } + public void setObj(List value) { failOnce(); } + public void setI(int value) { failOnce(); } + public void setL(long value) { failOnce(); } + public void setB(boolean value) { failOnce(); } + } + + static class FluentBean { + public FluentBean setStr(String value) { failOnce(); return this; } + public FluentBean setObj(List value) { failOnce(); return this; } + public FluentBean setI(int value) { failOnce(); return this; } + public FluentBean setL(long value) { failOnce(); return this; } + public FluentBean setB(boolean value) { failOnce(); return this; } + } + + // Always fail, so reporting can be compared with plain Jackson + private static Throwable alwaysThrow; + + static class AlwaysFailingBean { + public void setStr(String value) { alwaysFail(); } + public void setI(int value) { alwaysFail(); } + } + + private static void alwaysFail() { + calls++; + if (alwaysThrow instanceof Error) { + throw (Error) alwaysThrow; + } + throw (RuntimeException) alwaysThrow; + } + + // `set()` skips nulls as `MethodProperty.set` does; properties before the creator's + // are buffered and assigned through it + static class SkipNullsBean { + final int id; + String str = "default"; + List list = Arrays.asList("keep"); + int i = 7; + long l = 8L; + boolean b = true; + + @JsonCreator + SkipNullsBean(@JsonProperty("id") int id) { this.id = id; } + + @JsonSetter(nulls=Nulls.SKIP) + public void setI(int value) { calls++; i = value; } + + @JsonSetter(nulls=Nulls.SKIP) + public void setL(long value) { calls++; l = value; } + + @JsonSetter(nulls=Nulls.SKIP) + public void setB(boolean value) { calls++; b = value; } + + @JsonSetter(nulls=Nulls.SKIP) + public void setStr(String value) { calls++; str = value; } + + @JsonSetter(nulls=Nulls.SKIP) + public void setList(List value) { calls++; list = value; } + } + + @JsonTypeInfo(use=JsonTypeInfo.Id.NAME, defaultImpl=Void.class) + static class Item { + public String name; + } + + static class PolymorphicNullBean { + int calls; + Item value = new Item(); + + @JsonSetter(nulls=Nulls.SKIP) + public void setVoid(Item v) { calls++; value = v; } + + @JsonSetter(nulls=Nulls.SKIP) + public PolymorphicNullBean setFluent(Item v) { calls++; value = v; return this; } + } + + static class NullTextBean { + String skipped = "default"; + String plain = "default"; + + @JsonSetter(nulls=Nulls.SKIP) + public void setSkipped(String value) { skipped = value; } + + public void setPlain(String value) { plain = value; } + } + + // The bean is a hidden subclass, whose setters are left out of stack traces as + // the generated accessors are + private static Class hiddenVoid, hiddenFluent; + + public static class HideableVoidBean { + @JsonCreator + public static HideableVoidBean create() throws Exception { + return (HideableVoidBean) hiddenVoid.getConstructor().newInstance(); + } + public void setStr(String value) { } + } + + public static class HiddenVoidBean extends HideableVoidBean { + public HiddenVoidBean() { } + @Override + public void setStr(String value) { calls++; throw new SecurityException("thrown by hidden setter"); } + } + + public static class HideableFluentBean { + @JsonCreator + public static HideableFluentBean create() throws Exception { + return (HideableFluentBean) hiddenFluent.getConstructor().newInstance(); + } + public HideableFluentBean setStr(String value) { return this; } + } + + public static class HiddenFluentBean extends HideableFluentBean { + public HiddenFluentBean() { } + @Override + public HideableFluentBean setStr(String value) { calls++; throw new SecurityException("thrown by hidden setter"); } + } + + // A retry would succeed and hide the original failure. + private static void failOnce() { + if (calls++ == 0) { + lastFailure = failure.get(); + if (lastFailure instanceof Error) { + throw (Error) lastFailure; + } + throw (RuntimeException) lastFailure; + } + } + + private List> failures() { + SecurityException savedSecurity = new SecurityException("saved"); + IllegalAccessError savedAccess = new IllegalAccessError("saved"); + SecurityException emptySecurity = new SecurityException("no trace"); + emptySecurity.setStackTrace(new StackTraceElement[0]); + IllegalAccessError emptyAccess = new IllegalAccessError("no trace"); + emptyAccess.setStackTrace(new StackTraceElement[0]); + return Arrays.asList( + () -> new SecurityException("thrown by setter"), + () -> new IllegalAccessError("thrown by setter"), + () -> savedSecurity, + () -> savedAccess, + () -> emptySecurity, + () -> emptyAccess); + } + + @Test + public void testDeserializeDoesNotRetrySetterFailures() throws Exception { + for (boolean wrap : new boolean[] { true, false }) { + ObjectMapper mapper = mapperBuilder() + .configure(DeserializationFeature.WRAP_EXCEPTIONS, wrap).build(); + for (Class beanType : new Class[] { VoidBean.class, FluentBean.class }) { + for (String json : new String[] { a2q("{'str':'foo'}"), a2q("{'obj':['foo']}"), + a2q("{'i':42}"), a2q("{'l':12345678901}"), a2q("{'b':true}") }) { + for (Supplier nextFailure : failures()) { + failure = nextFailure; + assertRunsOnce(() -> mapper.readValue(json, beanType), + beanType.getSimpleName() + ", wrap=" + wrap + ", " + json); + } + } + } + } + } + + @Test + public void testSetDoesNotRetrySetterFailures() throws Exception { + ObjectMapper mapper = newObjectMapper(); + String[] names = { "str", "obj", "i", "l", "b" }; + Object[] values = { "foo", Arrays.asList("foo"), 42, 12345678901L, true }; + List> beans = Arrays.asList(VoidBean::new, FluentBean::new); + for (Supplier newBean : beans) { + Class beanType = newBean.get().getClass(); + for (int i = 0; i < names.length; i++) { + SettableBeanProperty property = property(mapper, beanType, names[i]); + assertTrue(property instanceof OptimizedSettableBeanProperty, names[i]); + Object value = values[i]; + for (Supplier nextFailure : failures()) { + failure = nextFailure; + assertRunsOnce(() -> property.set(newBean.get(), value), + beanType.getSimpleName() + ", " + names[i]); + } + } + } + } + + // Not retried, and reported as `MethodProperty` reports them, not as the setter's + // raw `Error` or exception + @Test + public void testSetterFailuresAreReportedAsVanilla() throws Exception { + for (boolean wrap : new boolean[] { true, false }) { + ObjectMapper vanilla = JsonMapper.builder().configure(DeserializationFeature.WRAP_EXCEPTIONS, wrap).build(); + ObjectMapper blackbird = mapperBuilder().configure(DeserializationFeature.WRAP_EXCEPTIONS, wrap).build(); + for (Throwable t : new Throwable[] { new SecurityException("se"), new IllegalAccessError("iae"), + new IllegalStateException("ise"), new NoClassDefFoundError("ncdfe"), new AssertionError("ae") }) { + for (String json : new String[] { a2q("{'str':'foo'}"), a2q("{'i':42}") }) { + String desc = "wrap=" + wrap + ", " + t + ", " + json; + alwaysThrow = t; + calls = 0; + Throwable expected = assertThrows(Throwable.class, () -> vanilla.readValue(json, AlwaysFailingBean.class)); + Throwable actual = assertThrows(Throwable.class, () -> blackbird.readValue(json, AlwaysFailingBean.class)); + assertEquals(2, calls, desc); + assertEquals(expected.getClass(), actual.getClass(), desc); + JsonMappingException e = (JsonMappingException) expected; + JsonMappingException a = (JsonMappingException) actual; + assertEquals(e.getOriginalMessage(), a.getOriginalMessage(), desc); + assertEquals(e.getPathReference(), a.getPathReference(), desc); + assertEquals(e.getLocation(), a.getLocation(), desc); + assertSame(t, a.getCause(), desc); + } + } + } + } + + @Test + public void testSetSkipsNulls() throws Exception { + for (String name : new String[] { "str", "list", "i", "l", "b" }) { + SettableBeanProperty property = property(newObjectMapper(), SkipNullsBean.class, name); + assertTrue(property instanceof OptimizedSettableBeanProperty, name); + SkipNullsBean bean = new SkipNullsBean(1); + calls = 0; + property.set(bean, null); + assertEquals(0, calls, name); + } + // with the nulls before the creator property, so they're buffered + String json = a2q("{'str':null,'list':null,'i':null,'l':null,'b':null,'id':1}"); + for (ObjectMapper mapper : new ObjectMapper[] { newVanillaJSONMapper(), newObjectMapper() }) { + calls = 0; + SkipNullsBean bean = mapper.readValue(json, SkipNullsBean.class); + assertEquals("default", bean.str); + assertEquals(Arrays.asList("keep"), bean.list); + assertEquals(7, bean.i); + assertEquals(8L, bean.l); + assertTrue(bean.b); + assertEquals(0, calls); + } + } + + // Without skipping, a null for a primitive fails as it does through reflection + @Test + public void testSetNullPrimitiveWithoutSkipping() throws Exception { + ObjectMapper vanilla = newVanillaJSONMapper(); + ObjectMapper blackbird = newObjectMapper(); + for (String name : new String[] { "i", "l", "b" }) { + SettableBeanProperty expectedProperty = property(vanilla, VoidBean.class, name); + SettableBeanProperty actualProperty = property(blackbird, VoidBean.class, name); + assertTrue(actualProperty instanceof OptimizedSettableBeanProperty, name); + calls = 0; + JsonMappingException expected = assertThrows(JsonMappingException.class, + () -> expectedProperty.set(new VoidBean(), null), name); + JsonMappingException actual = assertThrows(JsonMappingException.class, + () -> actualProperty.set(new VoidBean(), null), name); + assertEquals(expected.getOriginalMessage(), actual.getOriginalMessage(), name); + assertEquals(expected.getCause().getClass(), actual.getCause().getClass(), name); + assertEquals(0, calls, name); + } + } + + @Test + public void testPolymorphicNullMatchesVanilla() throws Exception { + ObjectMapper blackbird = newObjectMapper(); + for (String name : new String[] { "void", "fluent" }) { + SettableBeanProperty prop = property(blackbird, PolymorphicNullBean.class, name); + assertTrue(prop instanceof OptimizedSettableBeanProperty, name); + + PolymorphicNullBean direct = new PolymorphicNullBean(); + prop.set(direct, null); + assertEquals(0, direct.calls, name); + assertNotNull(direct.value, name); + + for (ObjectMapper mapper : new ObjectMapper[] { newVanillaJSONMapper(), blackbird }) { + PolymorphicNullBean explicit = mapper.readValue(a2q("{'" + name + "':null}"), PolymorphicNullBean.class); + assertEquals(0, explicit.calls, name); + assertNotNull(explicit.value, name); + + // MethodProperty passes a null from polymorphic deserialization to + // the setter, even with Nulls.SKIP. + PolymorphicNullBean polymorphic = mapper.readValue( + a2q("{'" + name + "':{'@type':'Unknown'}}"), PolymorphicNullBean.class); + assertEquals(1, polymorphic.calls, name); + assertNull(polymorphic.value, name); + } + } + } + + // `TextNode(null)` reads as a VALUE_STRING whose text is null + @Test + public void testNullTextMatchesVanilla() throws Exception { + assertTrue(property(newObjectMapper(), NullTextBean.class, "skipped") instanceof OptimizedSettableBeanProperty); + ObjectNode tree = JsonNodeFactory.instance.objectNode(); + tree.set("skipped", new TextNode(null)); + tree.set("plain", new TextNode(null)); + for (ObjectMapper mapper : new ObjectMapper[] { newVanillaJSONMapper(), newObjectMapper() }) { + NullTextBean bean = mapper.treeToValue(tree, NullTextBean.class); + assertEquals("default", bean.skipped); + assertNull(bean.plain); + } + } + + @Test + public void testHiddenBeanSetterFailureIsNotRetried() throws Exception { + Method define; + Object noOptions; + try { + Class option = Class.forName("java.lang.invoke.MethodHandles$Lookup$ClassOption"); + noOptions = Array.newInstance(option, 0); + define = MethodHandles.Lookup.class.getMethod("defineHiddenClass", + byte[].class, boolean.class, noOptions.getClass()); + } catch (ReflectiveOperationException e) { + assumeTrue(false, "hidden classes need Java 15+"); + return; + } + hiddenVoid = defineHidden(define, noOptions, HiddenVoidBean.class); + hiddenFluent = defineHidden(define, noOptions, HiddenFluentBean.class); + for (Class type : new Class[] { HideableVoidBean.class, HideableFluentBean.class }) { + assertTrue(property(newObjectMapper(), type, "str") instanceof OptimizedSettableBeanProperty); + for (ObjectMapper mapper : new ObjectMapper[] { newVanillaJSONMapper(), newObjectMapper() }) { + calls = 0; + assertThrows(JsonMappingException.class, () -> mapper.readValue(a2q("{'str':'foo'}"), type)); + assertEquals(1, calls, type.getSimpleName()); + } + } + } + + private static Class defineHidden(Method define, Object noOptions, Class type) throws Exception { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + try (InputStream in = type.getResourceAsStream("/" + type.getName().replace('.', '/') + ".class")) { + byte[] buffer = new byte[4096]; + int n; + while ((n = in.read(buffer)) != -1) { + out.write(buffer, 0, n); + } + } + return ((MethodHandles.Lookup) define.invoke(MethodHandles.lookup(), out.toByteArray(), true, noOptions)) + .lookupClass(); + } + + private void assertRunsOnce(Executable action, String description) { + calls = 0; + Throwable thrown = assertThrows(Throwable.class, action, description); + assertEquals(1, calls, description); + while (thrown.getCause() != null) { + thrown = thrown.getCause(); + } + assertSame(lastFailure, thrown, description); + } + + @Test + public void testVoidAccessorAccessFailureIsNotRetried() throws Throwable { + InaccessibleSetterValue bean = new InaccessibleSetterValue(); + Object value = InaccessibleSetterValue.newValue(); + Method setter = InaccessibleSetterValue.class.getMethod("setVoidValue", value.getClass()); + BiConsumer accessor = voidAccessor(setter); + SettableObjectProperty property = new SettableObjectProperty( + property(newVanillaJSONMapper(), bean.getClass(), "voidValue"), accessor, null, + InaccessibleSetterValue.class, setter.getParameterTypes()[0], false); + + // The generated accessor cannot cast to the package-private parameter type; + // that is reported like the setter's failure, and the setter never runs + assertThrows(IllegalAccessError.class, () -> accessor.accept(bean, value)); + JsonMappingException e = assertThrows(JsonMappingException.class, () -> property.set(bean, value)); + assertEquals(IllegalAccessError.class, e.getCause().getClass()); + assertEquals(0, bean.calls); + } + + @Test + public void testFluentAccessorAccessFailureIsNotRetried() throws Throwable { + InaccessibleSetterValue bean = new InaccessibleSetterValue(); + Object value = InaccessibleSetterValue.newValue(); + Method setter = InaccessibleSetterValue.class.getMethod("setFluentValue", value.getClass()); + BiFunction thunk = returningAccessor(setter); + BiConsumer accessor = (instance, v) -> thunk.apply(instance, v); + SettableObjectProperty property = new SettableObjectProperty( + property(newVanillaJSONMapper(), bean.getClass(), "fluentValue"), accessor, thunk, + InaccessibleSetterValue.class, setter.getParameterTypes()[0], false); + + // the same through the wrapper that fluent setters on regular beans use + assertThrows(IllegalAccessError.class, () -> accessor.accept(bean, value)); + JsonMappingException e = assertThrows(JsonMappingException.class, () -> property.set(bean, value)); + assertEquals(IllegalAccessError.class, e.getCause().getClass()); + assertEquals(0, bean.calls); + } + + private SettableBeanProperty property(ObjectMapper mapper, Class beanType, String name) throws Exception { + DefaultDeserializationContext context = ((DefaultDeserializationContext) mapper.getDeserializationContext()) + .createInstance(mapper.getDeserializationConfig(), null, null); + BeanDeserializerBase deserializer = (BeanDeserializerBase) context.findRootValueDeserializer( + mapper.constructType(beanType)); + return deserializer.findProperty(name); + } + + @SuppressWarnings("unchecked") + private BiConsumer voidAccessor(Method setter) throws Throwable { + MethodHandle handle = MethodHandles.lookup().unreflect(setter); + return (BiConsumer) LambdaMetafactory.metafactory( + MethodHandles.lookup(), "accept", MethodType.methodType(BiConsumer.class), + MethodType.methodType(void.class, Object.class, Object.class), handle, handle.type()) + .getTarget().invoke(); + } + + @SuppressWarnings("unchecked") + private BiFunction returningAccessor(Method setter) throws Throwable { + MethodHandle handle = MethodHandles.lookup().unreflect(setter); + return (BiFunction) LambdaMetafactory.metafactory( + MethodHandles.lookup(), "apply", MethodType.methodType(BiFunction.class), + MethodType.methodType(Object.class, Object.class, Object.class), handle, handle.type()) + .getTarget().invoke(); + } +} diff --git a/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/access/InaccessibleSetterValue.java b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/access/InaccessibleSetterValue.java new file mode 100644 index 00000000..e5603596 --- /dev/null +++ b/blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/access/InaccessibleSetterValue.java @@ -0,0 +1,23 @@ +package com.fasterxml.jackson.module.blackbird.deser.access; + +public class InaccessibleSetterValue { + public int calls; + public Object value; + + public void setVoidValue(SetterValue value) { + calls++; + this.value = value; + } + + public InaccessibleSetterValue setFluentValue(SetterValue value) { + calls++; + this.value = value; + return this; + } + + public static Object newValue() { + return new SetterValue(); + } +} + +class SetterValue { } diff --git a/release-notes/CREDITS-2.x b/release-notes/CREDITS-2.x index 073353a3..a5d22c9d 100644 --- a/release-notes/CREDITS-2.x +++ b/release-notes/CREDITS-2.x @@ -169,7 +169,12 @@ Apoorva Manjunath (@apoorvam) Parker Winchester (@parker89) +* Contributed fix for #363: (blackbird) `@JsonSetter(nulls = Nulls.SKIP)` ignored + for properties of beans with a properties-based creator + (2.23.0) * Contributed #364: (blackbird) Use generated setters for Builder-based deserialization (2.23.0) * Contributed #366: (blackbird) Skip generated setters and getters the accessor can't call (2.23.0) +* Contributed #367: (blackbird) Don't retry failed bean setters through reflection + (2.23.0) diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x index 97d72110..536ae5db 100644 --- a/release-notes/VERSION-2.x +++ b/release-notes/VERSION-2.x @@ -25,10 +25,16 @@ Active maintainers: 2.23.0 (not yet released) +#363: (blackbird) `@JsonSetter(nulls = Nulls.SKIP)` ignored for properties of + beans with a properties-based creator + (reported by @jkuipers, contributed by Parker W) #364: (blackbird) Use generated setters for Builder-based deserialization (contributed by Parker W) #366: (blackbird) Skip generated setters and getters the accessor can't call (contributed by Parker W) +#367: (blackbird) Don't retry failed bean setters through reflection; report + anything a setter throws as `JsonMappingException`, as plain databind does + (contributed by Parker W) 2.22.4 (17-Oct-2026) 2.22.3 (21-Sep-2026)