Repository navigation
Blackbird: avoid retrying failed bean setters - #367
Merged
cowtowncoder merged 49 commits intoOct 30, 2026
Merged
Conversation
… pw/blackbird-setter-retries # Conflicts: # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java
…/blackbird-builder-setters # Conflicts: # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/BBDeserializerModifier.java # blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/TestInaccessibleSetterTypes.java # blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/deser/otherpackage/ModeBeanBase.java # blackbird/src/test/java/com/fasterxml/jackson/module/blackbird/util/TestReflectionHack.java
… pw/blackbird-setter-retries
This was referenced Oct 29, 2026
…/blackbird-builder-setters
… pw/blackbird-setter-retries
… pw/blackbird-setter-retries # Conflicts: # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java
…/blackbird-builder-setters # Conflicts: # release-notes/CREDITS-2.x # release-notes/VERSION-2.x
… pw/blackbird-setter-retries
…/blackbird-builder-setters
… pw/blackbird-setter-retries
… pw/blackbird-setter-retries
2.x has FasterXML#366 squashed; this branch already had it up to 2806ae1, so resolve against that rather than the older merge base. Keep both release notes entries and both sets of tests; builder tests now use `_mapperWithLookupIn`, and expect inherited protected builder setters to be optimized on JDK 14+ as for beans. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…retries Picks up the builder branch's merge of 2.x, which has FasterXML#366 squashed, so this branch no longer conflicts with 2.x. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
databind also calls set() on builders, for properties buffered before a property-based creator and for external type ids, so whether a setter failure was retried, how it was reported, and whether access modifiers were honored depended on the order of properties in the JSON. - Builder properties now report failures from set() like setAndReturn(): no retry through reflection, wrapped as MethodProperty does - A builder setter Jackson wouldn't make accessible is left to reflection entirely, not just for setAndReturn() - Primitive setAndReturn() checks for a generated setter before unboxing Regular beans keep their existing behavior (see TestAccessFallback). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… pw/blackbird-setter-retries # Conflicts: # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/OptimizedSettableBeanProperty.java # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableBooleanProperty.java # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableIntProperty.java # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableLongProperty.java # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableObjectProperty.java # blackbird/src/main/java/com/fasterxml/jackson/module/blackbird/deser/SettableStringProperty.java
Keeps 2.x's squashed builder-setter changes (FasterXML#364) and re-applies this branch's no-retry and null-handling changes on top. TestSetterFailures is updated for 2.x's SettableObjectProperty constructor and removal of BBDeserializerModifier.trampoline. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Drop unused `value` parameter of `_reportProblem`, and report access failures through `_reportProblemAndReturn` (so the throw is explicit) - Remove `TestAccessFallback.testDeserializeAccess`, whose trigger only matched Afterburner-generated classes and never fired for Blackbird - Run `TestSetterFailures` in a single thread, as it uses static state Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Anything a generated setter throws is now wrapped and translated the way `MethodProperty` does it (into a `JsonMappingException` with location and path), not just `IllegalAccessError` and `SecurityException`; other exceptions and errors used to escape unwrapped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cowtowncoder
changed the title
- `_reportProblem` now returns the `IOException` for callers to throw, replacing `_reportProblemAndReturn` - Clarify `_forBuilder` javadoc - Note in FasterXML#367 release note that setter failures are reported as `JsonMappingException` Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… fallback The getter only failed for Afterburner-generated callers, so the test never reached the fallback writer under Blackbird. It now fails when called through the generated accessor, and verifies that the value is written through reflection, and that the writer stays on reflection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cowtowncoder
added a commit
that referenced
this pull request
Oct 30, 2026
Ports #367 (Blackbird: don't retry failed bean setters through reflection) to 3.x, following 3.x `MethodProperty`: - Setter failures (access errors included) are reported with `_throwAsJacksonE(p, e, value)` from every path, so Errors and runtime exceptions are rethrown as they are, as without Blackbird; never retried through the delegate - `_reportProblem` returns the exception for callers to throw, replacing `_reportProblemAndReturn` Tests adapted accordingly: vanilla failures are compared as thrown, the `TextNode(null)` test is dropped (as `StringNode` rejects null in 3.x), hidden classes are defined directly (Java 17), and the serialization fallback test recognizes databind's `BeanPropertyWriter` (which calls getters through a method handle) as the fallback path. Also adds #363 and #367 to 3.1.9 release notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cowtowncoder
added a commit
that referenced
this pull request
Oct 30, 2026
cowtowncoder
added a commit
to stevenschlansker/jackson-modules-base
that referenced
this pull request
Oct 30, 2026
Brings in FasterXML#367 (don't retry failed bean setters through reflection) for the old LambdaMetafactory engine. Its Settable*Property classes stay deleted here. The new TestSetterFailures checks for a generated reader instead of optimized properties, and drops the two tests that build the removed property classes by hand (and their helper class). TestAccessFallback is rewritten for the generated writers, which have no stock fallback on getter failure: it checks that a failing getter runs once and fails as plain databind does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Released versions of Blackbird retry regular bean setters that throw
SecurityExceptionorIllegalAccessError, even if they have already changed state. A setter that succeeds on the second call can hide the original failure.Stop retrying them, as #364 does for builders: once the generated setter is called, a failure is never retried, for void and fluent setters alike.
SecurityExceptionandIllegalAccessErrorare still reported the wayMethodPropertyreports them, as they were after the retry; other exceptions are unchanged. The trade-off is that a failure of the generated accessor itself that #366's setup checks miss now fails deserialization instead of falling back to reflection.This also fixes two null-handling differences that predate this stack:
Nulls.SKIPinset(). With a property-based creator, nulls buffered before the creator parameters no longer overwrite defaults (String and Object) or fail deserialization (the primitives unboxed the null). Fixes Blackbird ignores@JsonSetter(nulls = Nulls.SKIP)for properties of beans with a properties-based creator #363 on 2.x; the fix also needs to be merged forward to 3.x.TextNode(null), receive the usual null handling.Third in the stack: #366 → #364 → #367. #366 is merged; the diff includes #364 until it is merged. This addresses the regular-setter review feedback.
Tests cover all five optimized property types, void and fluent setters, hidden classes, real accessor failures (reported without running the setter), and comparison with vanilla error/null handling, including explicit and polymorphic nulls. Blackbird's 414-test suite passes on Java 8, 11, 17 and 25, with the expected hidden-class skips on 8 and 11.