Skip to content

[WP5] Reflective spec construction masks TypeMismatchException + hidden context invariant #177

Description

@shihyuho

此內容由 AI 產生(specification-mapper 全庫審查 2026-07-16)。

Work package WP5 · findings: COR-09, TEST-07, MAINT-04 · worst severity: medium

From the 2026-07-16 whole-codebase review (branch jakarta, modules mapper/ + starter/). Each finding below is CONFIRMED by independent adversarial verification. File paths and line numbers reflect the review snapshot and may drift — treat them as starting points, not exact coordinates.


COR-09 — TypeMismatchException from spec constructors is masked as InvocationTargetException when going through SpecMapper

  • Category / severity: correctness / medium · effort S · confidence high
  • Location: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/SimpleSpecification.java:67
  • Evidence: SimpleSpecification.newSpec (lines 59-67) builds specs via accessibleConstructor(...).newInstance(...) under @SneakyThrows. Constructor.newInstance wraps any exception thrown by the constructor — the TypeMismatchException from In/Between/BooleanSpecification/ComparableSpecification and the IllegalArgumentException from Between — in java.lang.reflect.InvocationTargetException, and nothing in mapper/ or starter/ main code unwraps getCause() (grep for InvocationTargetException/getCause over src/main returns nothing).
  • Failure scenario: A web layer maps a query POJO with @Spec(In.class) String name (or a Between field bound to a 1-element list): SpecMapper.toSpec throws a sneaky InvocationTargetException instead of TypeMismatchException, so an exception handler written to translate TypeMismatchException into a 400 never fires and the client gets an opaque 500; the meaningful message is buried in the cause.
  • Suggested fix: Catch InvocationTargetException in newSpec and rethrow its cause (rethrow RuntimeException causes directly, wrap checked ones), so the documented TypeMismatchException/IllegalArgumentException surface to SpecMapper.toSpec callers.
  • Verification: [CONFIRMED] Traced the actual code. SimpleSpecification.newSpec (lines 66-67) constructs specs via accessibleConstructor(...).newInstance(...) under @SneakyThrows. Constructor.newInstance wraps any exception thrown inside the constructor in a checked java.lang.reflect.InvocationTargetException; @SneakyThrows rethrows that InvocationTargetException as-is without unwrapping getCause().

The constructor-time throws are real: Between.java lines 52-59 throw TypeMismatchException (value not Iterable) and IllegalArgumentException (element count != 2) from within the constructor, so both get wrapped. TypeMismatchException extends RuntimeException (line 38). grep over mapper/src/main and starter/sr…

TEST-07 — Type-mismatch behavior is only tested via direct construction, never through SpecMapper.toSpec, hiding the exception-masking bug

  • Category / severity: testing / medium · effort S · confidence high
  • Location: mapper/src/test/java/tw/com/softleader/data/jpa/spec/domain/InTest.java:51
  • Evidence: Every typeMismatch test (InTest.java:51, NotInTest, TrueTest, FalseTest, IsNullTest, NotNullTest, HasTextTest, HasLengthTest, GreaterThan*/LessThan* tests) instantiates the spec constructor directly and asserts TypeMismatchException; grep shows no test in the repo asserts what SpecMapper.toSpec throws when a @SPEC field value has the wrong type — the path real applications hit.
  • Failure scenario: The reflective-construction path (SimpleSpecification.newSpec) rethrows InvocationTargetException instead of TypeMismatchException today, and the suite stays green; any future regression in mapper-path error translation is equally invisible because the trust-boundary flow (POJO -> SpecMapper -> exception) has zero coverage.
  • Suggested fix: Add tests that call SpecMapper.toSpec on POJOs with mismatched @SPEC value types (In on a String field, Between with 1 element, True on a String) and assert the exception type/message the public API is supposed to expose.
  • Verification: [CONFIRMED] Verified in code. All typeMismatch tests (InTest.java:51-58 and the listed siblings) assert TypeMismatchException by directly invoking the spec constructor (e.g. new In<Customer>(context, "name", value)), never through the mapper. The production path SpecMapper -> SimpleSpecificationResolver.buildSpecification (line 79) uses SimpleSpecification.builder().build() -> newSpec (SimpleSpecification.java:61-68), which calls accessibleConstructor(...).newInstance(...). Constructor.newInstance wraps constructor exceptions in InvocationTargetException; newSpec is @SneakyThrows with no getCause/getTargetException unwrapping, and no caller unwraps it. So the real path leaks InvocationTa…

MAINT-04 — Dotted-path specs constructed directly (outside SpecMapper) throw NoSuchElementException because CTX_JOIN is a hidden invariant

  • Category / severity: maintainability / low · effort S · confidence high
  • Location: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/SimpleSpecification.java:91
  • Evidence: getPath for any multi-segment path calls getExpr -> getJoin/getFetch, which call context.getAs(CTX_JOIN, JoinContext.class) (lines 91, 96); Context.getAs throws NoSuchElementException when the key is absent. Only SpecMapper.toSpec seeds CTX_JOIN, and the sole JoinContext implementation SpecJoinContext is package-private (SpecJoinContext.java:38) while the spec constructors, Context interface, and SpecContext are all public — so a caller composing specs directly cannot satisfy the invariant without implementing JoinContext themselves.
  • Failure scenario: A user follows the library's own documented/direct-construction pattern (as every domain test does) and writes new Equals<Customer>(new SpecContext(), "address.city", "Taipei") for a plain embedded attribute with no joins at all; repository.findAll(spec) throws NoSuchElementException("No value found for key '_JOIN'...") at toPredicate time.
  • Suggested fix: In getExpr, fall back to plain path navigation when CTX_JOIN is absent (e.g. use context.get(CTX_JOIN) and treat empty as no-join) instead of getAs, or make SpecContext seed an empty JoinContext by default.
  • Verification: [CONFIRMED] Traced SimpleSpecification.getPath (lines 71-83): single-segment paths return root.get() and never touch context; multi-segment paths call getExpr->getJoin/getFetch (lines 91,96) which invoke context.getAs(CTX_JOIN, JoinContext.class). Context.getAs (Context.java) throws NoSuchElementException on an empty Optional. SpecContext.get returns Optional.empty() for an unseeded key, and grep shows CTX_JOIN is written only by SpecMapper.java:84 — every other site (JoinSpecificationResolver, JoinFetchSpecificationResolver, Join, JoinFetch, SimpleSpecification) only reads it. So new Equals<>(new SpecContext(), "address.city", ...) does throw NoSuchElementException at toPredicate time, an…

Full report: see the codebase-review-2026-07-16 branch (linked on the parent tracking issue).

Activity

  1. added theissue type on Jul 16, 2026
  2. shihyuho commented on Jul 16, 2026

    @shihyuho
    MemberAuthor

    This was generated by AI during triage.

    Agent Brief

    Category: bug (correctness bug + its missing test coverage)

    Summary: Reflective spec construction via SimpleSpecification.newSpec lets InvocationTargetException escape to SpecMapper.toSpec callers, masking the TypeMismatchException / IllegalArgumentException that spec constructors actually throw.

    Current behavior:

    • SimpleSpecification.newSpec is annotated @SneakyThrows and builds specs via accessibleConstructor(...).newInstance(...). Constructor.newInstance wraps any exception thrown inside the constructor in a checked java.lang.reflect.InvocationTargetException, and @SneakyThrows rethrows that wrapper verbatim without unwrapping the cause.
    • Constructor-time validation throws are real and get wrapped: Between throws TypeMismatchException (value not Iterable) and IllegalArgumentException (element count != 2); In / BooleanSpecification / ComparableSpecification similarly throw TypeMismatchException. TypeMismatchException extends RuntimeException.
    • No code in mapper/ or starter/ main sources unwraps getCause() / getTargetException(). Net effect: a query POJO with a mismatched @Spec value type causes SpecMapper.toSpec to leak InvocationTargetException instead of the meaningful TypeMismatchException / IllegalArgumentException. A downstream exception handler written to translate TypeMismatchException into an HTTP 400 never fires; the client gets an opaque 500 with the real message buried in the cause.
    • Test coverage gap: every typeMismatch test (e.g. InTest, NotInTest, TrueTest, FalseTest, IsNullTest, NotNullTest, HasTextTest, HasLengthTest, GreaterThan*/LessThan*) asserts TypeMismatchException by invoking the spec constructor directly. No test asserts what SpecMapper.toSpec throws when a @Spec field value has the wrong type — the trust-boundary path real applications hit — so the masking bug stays invisible and any future regression in mapper-path error translation is equally uncaught.

    Desired behavior:

    • When a spec constructor throws during reflective construction, the original exception surfaces to SpecMapper.toSpec callers: RuntimeException causes (including TypeMismatchException and IllegalArgumentException) are rethrown as-is with their message intact; checked causes are wrapped appropriately rather than leaked as InvocationTargetException.
    • InvocationTargetException must never escape newSpec.
    • The mapper path (POJO -> SpecMapper.toSpec -> exception) has direct test coverage for the type-mismatch cases.

    Key interfaces:

    • tw.com.softleader.data.jpa.spec.domain.SimpleSpecification#newSpec (the @Builder/@SneakyThrows reflective factory) — the unwrap point.
    • tw.com.softleader.data.jpa.spec.SpecMapper#toSpec — the public entry point whose thrown exception type must become the constructor's real exception.
    • tw.com.softleader.data.jpa.spec.domain.TypeMismatchException (extends RuntimeException) and java.lang.IllegalArgumentException — the exceptions that must surface.
    • Constructors of tw.com.softleader.data.jpa.spec.domain.Between and ...In — representative throwers used to exercise the path.

    Acceptance criteria:

    • SpecMapper.toSpec on a POJO whose @Spec(In.class) (or equivalent) field holds a non-Iterable value throws TypeMismatchException (not InvocationTargetException), with the same message the direct constructor produces.
    • SpecMapper.toSpec on a POJO whose @Spec(Between.class) field is bound to a 1-element list throws IllegalArgumentException with the constructor's message (@Between expected exact 2 elements...).
    • InvocationTargetException never propagates out of SimpleSpecification.newSpec; a RuntimeException cause is rethrown unchanged, a checked cause is wrapped (not leaked raw).
    • New tests exercise the SpecMapper.toSpec path (constructing a query POJO and mapping it), not the direct spec constructor, for at least the In/non-Iterable and Between/1-element cases, asserting the surfaced exception type and message.
    • All existing direct-construction typeMismatch tests remain green.
    • mvn test passes.

    Out of scope:

    • MAINT-04 (the CTX_JOIN hidden-invariant finding: directly-constructed dotted-path specs such as new Equals<>(new SpecContext(), "address.city", "Taipei") throw NoSuchElementException at toPredicate time because only SpecMapper.toSpec seeds CTX_JOIN and the sole JoinContext implementation is package-private). This is confirmed present but deferred: it turns on a maintainer design decision — whether direct spec construction outside SpecMapper is a supported use case at all (and if so, whether to fall back to plain path navigation, seed an empty JoinContext by default, or instead surface a clearer error / document the constraint). The existing domain tests do not actually exercise this path (they use a single-segment noopContext()), so there is no established contract for an agent to preserve. Resolve COR-09 + TEST-07 here; MAINT-04 needs a human design call before implementation.
    • Changing the public exception types thrown by the spec constructors themselves, or adding new HTTP-layer exception handling in starter/.
  3. added a commit that references this issue on Jul 16, 2026
    8b356df
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    status: ready-for-agentFully specified and ready for an autonomous AFK agent to implementtype: bugSomething isn't working

    Type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions