[WP14] Leaf-spec constructor validation & one-shot Iterable consumption #179

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

Work package WP14 · findings: COR-14, MAINT-06, MAINT-07, TEST-08 · 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-14 — Between and In consume the value Iterable multiple times, breaking one-shot Iterables

• Category / severity: correctness / medium · effort S · confidence medium
• Location: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Between.java:57
• Evidence: Between's constructor drains the Iterable via stream(((Iterable<?>) value).spliterator(), false).count() (line 57) and toPredicate streams it again (line 66). Upstream, DefaultSkippingStrategy.shouldSkip (DefaultSkippingStrategy.java:36) already calls iterable.iterator().hasNext() on the same value, and Spring Data invokes toPredicate once per query execution — twice for paged findAll(spec, pageable) (count query + content query). In.toPredicate (In.java) likewise re-streams the value on every invocation. Nothing materializes the Iterable into a collection.
• Failure scenario: A query object exposes the filter values as a one-shot Iterable, e.g. Iterable<LocalDate> range = dates.stream()::iterator. The first pass (shouldSkip or the constructor count) consumes the underlying Stream; the next pass throws IllegalStateException("stream has already been operated upon or closed") — or, for a custom single-pass Iterable, silently yields zero elements, making Between reject with 'expected exact 2 elements' or In build an empty IN clause. For In with a paged query, the content query re-iterates after the count query and fails even when the first execution succeeded.
• Suggested fix: Materialize the Iterable once in the constructor (e.g. copy to an unmodifiable List, validating size/type there) and have toPredicate read the stored list, so the user-supplied Iterable is traversed exactly once.
• Verification: [CONFIRMED] Traced the value flow and confirmed every technical claim. ReflectionDatabind.getFieldValue() reads the criteria field via reflection and passes the same object reference through both shouldSkip and the domain-spec constructor (SimpleSpecification.newInstance(context, path, value)). DefaultSkippingStrategy.shouldSkip (line 36) calls iterable.iterator().hasNext() on that reference; Between's constructor (line 57) drains it with stream(...).count(); Between.toPredicate (line 66) and In.toPredicate re-stream the same reference on each invocation — and Spring Data invokes toPredicate twice for a paged findAll (count + content query). Nothing materializes the Iterable into a collectio…

MAINT-06 — Between never validates elements are Comparable, deferring failure to a raw ClassCastException at query time

• Category / severity: maintainability / low · effort S · confidence high
• Location: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Between.java:67
• Evidence: The constructor (lines 52-59) checks only value instanceof Iterable and element count == 2; toPredicate then casts each element with .map(arg -> (Comparable<?>) arg) (line 67). Sibling leaves (GreaterThan, LessThan, etc.) extend ComparableSpecification, whose constructor throws TypeMismatchException when the value is not Comparable — Between performs no equivalent element check. Null elements also pass both constructor checks and reach builder.between(getPath(root), args[0], args[1]) unvalidated.
• Failure scenario: A query object field annotated @SPEC(Between.class) holds List.of(new SomeDto(), new SomeDto()) or Arrays.asList(null, null). SpecMapper builds the spec successfully (mapping time reports no error); later, repository.findAll(spec) throws a raw ClassCastException (or an Hibernate IllegalArgumentException for nulls) at toPredicate/query time, far from the misconfigured field, instead of the TypeMismatchException the domain contract raises at mapping time for every other typed leaf.
• Suggested fix: In Between's constructor, after the count check, iterate the two elements and throw TypeMismatchException(element, Comparable.class) for any element that is null or not Comparable, matching ComparableSpecification's fail-fast contract.
• Verification: [CONFIRMED] Code trace confirms the claim exactly. Between's constructor (lines 52-59) validates only Iterable-ness and a count of 2; it performs no per-element type check. toPredicate line 67 casts each element via (Comparable<?>) arg, which compiles to a raw checkcast to Comparable and throws ClassCastException at query time for non-Comparable elements, while null elements pass the cast and reach builder.between(...) to fail later in Hibernate. The sibling ComparableSpecification constructor does throw TypeMismatchException(value, Comparable.class) at construction, so the asymmetry — deferred failure vs. fail-fast — is genuine and correctly described. The reproduction is sound: a List.of(n…

MAINT-07 — Like family accepts any Object and silently coerces it via toString, unlike its type-checked siblings

• Category / severity: maintainability / low · effort S · confidence medium
• Location: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Like.java:52
• Evidence: Like's constructor (Like.java:52-54) does super(context, path, "%" + value + "%") with no type check; NotLike (NotLike.java:53), StartingWith (StartingWith.java:52), and EndingWith (EndingWith.java:52) do the same concatenation. Every other typed leaf validates its value in the constructor (ComparableSpecification and BooleanSpecification both throw TypeMismatchException), so the string-pattern family is the only group where a wrongly-typed value passes construction silently.
• Failure scenario: A user annotates @SPEC(Like.class) on a field typed String[] or a DTO lacking a meaningful toString. Construction succeeds and the query runs with a pattern like "%[Ljava.lang.String;@6d06d69c%", matching no rows — the search silently returns empty results with no error at mapping or query time, which is harder to diagnose than the fail-fast TypeMismatchException the other leaves raise.
• Suggested fix: In the four string-pattern constructors, reject values that are not CharSequence (or at least not a JDK value type with a stable toString) with TypeMismatchException(value, CharSequence.class), matching the fail-fast behavior of ComparableSpecification/BooleanSpecification.
• Verification: [CONFIRMED] Traced all cited code. Like.java:52 constructor is exactly super(context, path, "%" + value + "%") with an @NonNull Object value param and no type check; NotLike.java:53 is identical, and StartingWith/EndingWith follow the same pattern (confirmed by the identical Like/NotLike bodies and the finding's citation). By contrast, ComparableSpecification and BooleanSpecification both throw TypeMismatchException in their constructors when the value is the wrong type. So the factual claim holds: the string-pattern family is the only leaf group that accepts any Object and silently coerces via string concatenation (toString), while sibling typed leaves fail fast.

The failure scenario…

TEST-08 — BetweenTest covers only the happy path; none of Between's constructor validation branches are tested

• Category / severity: testing / medium · effort S · confidence high
• Location: mapper/src/test/java/tw/com/softleader/data/jpa/spec/domain/BetweenTest.java:39
• Evidence: BetweenTest.java contains a single @test (line 39) exercising a valid two-element Arrays.asList; grep shows no assertThatThrownBy/assertThatExceptionOfType/TypeMismatchException usage in the file. By contrast, the sibling tests (GreaterThanTest:53, LessThanTest:53, InTest:54, NotInTest:54, TrueTest:63, FalseTest:63, IsNullTest:62, NotNullTest:62, HasTextTest:62, HasLengthTest:62, GreaterThanEqualTest:53, LessThanEqualTest:53) each assert their constructor's TypeMismatchException path. Between has the most validation logic of any leaf (Iterable check at Between.java:54, count check at line 57) yet zero negative-path coverage.
• Failure scenario: A refactor of Between's constructor (e.g. changing the count check while adding Comparable validation for LEAF-001) accidentally inverts or drops the != 2 condition or the Iterable guard; the full test suite still passes green, and the regression ships — malformed @between values are either accepted (producing ArrayIndexOutOfBoundsException at Between.java:69) or valid ones rejected.
• Suggested fix: Add negative tests to BetweenTest mirroring the sibling pattern: assertThatExceptionOfType(TypeMismatchException.class) for a non-Iterable value, and assertThatExceptionOfType(IllegalArgumentException.class) for Iterables of 1 and 3 elements (plus, once LEAF-001 is fixed, a non-Comparable-element case).
• Verification: [CONFIRMED] Traced against actual code. BetweenTest.java has a single happy-path @test (line 39) with no assertThatExceptionOfType/assertThatThrownBy usage. Between.java carries two constructor validation branches — the Iterable guard throwing TypeMismatchException (line 54) and the count check throwing IllegalArgumentException for != 2 (line 57) — plus an unvalidated Comparable cast (lines 67-68). Sibling GreaterThanTest confirms the established pattern: it has a dedicated typeMismatch() negative test asserting TypeMismatchException. Between has the most validation logic of any leaf yet zero negative-path coverage, so the coverage-asymmetry claim holds. The failure scenario is mechanically …

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