Skip to content

[WP3] Join machinery: silent-wrong-result edge cases + missing edge/error tests #198

Description

@shihyuho

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

Work package WP3 · findings: COR-05, COR-06, COR-07, COR-10, COR-11, MAINT-04, PERF-02, TEST-04 · worst severity: medium · effort: M

From the 2026-07-16 whole-codebase review of branch jakarta (snapshot 7de4a33; modules mapper/ + starter/). Every finding below is CONFIRMED by independent adversarial verification. File:line coordinates are from the snapshot and may have drifted. (MAINT-04 was folded in from the review's WP5, whose other findings — COR-09, TEST-07 — were already fixed post-review by PR #189. The review's COR-08 moved to WP2, since it edits the same JoinFetch delegation method as PERF-01.)

Findings

COR-05 — query.distinct() is last-writer-wins across joins (correctness, medium)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Join.java:84
  • Evidence: every join spec calls query.distinct(distinct) with its own flag; the last-executed join silently overwrites all earlier ones.

COR-06 — Conflicting join definitions sharing an alias are silently dropped (correctness, medium)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Join.java:94
  • Evidence: if the alias is already registered, the spec returns without comparing the stored path/joinType — a conflicting definition (same alias, different path or join type) is silently ignored, first registration wins.

COR-07 — Dotted join path silently drops every segment after the second (correctness, medium)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/Join.java:112
  • Evidence: pathToJoinOn.split("\\.") is consumed as byDot[0]/byDot[1] only; a.b.c joins a.b while registering an alias that looks three-level.

COR-10 — A @Spec path referencing an unregistered join alias silently falls back to root.get(alias) (correctness, medium)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/SimpleSpecification.java:86
  • Evidence: declaration order and null-valued join fields determine whether the alias exists at resolution time; when absent, the spec silently queries a same-named attribute on the root instead of failing.

COR-11 — Count and content queries resolve a @JoinFetch alias with different join semantics (correctness, medium)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/SimpleSpecification.java:100
  • Failure scenario: for non-INNER fetches, totalElements can disagree with the page content.

MAINT-04 — Dotted-path specs constructed directly (outside SpecMapper) throw bare NoSuchElementException (maintainability, low)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/domain/SimpleSpecification.java:91
  • Evidence: CTX_JOIN presence is a hidden invariant of the mapper pipeline; direct construction hits an opaque failure.

PERF-02 — joined/fetched maps keyed by Root grow on every execution of a reused Specification (performance, medium)

  • File: mapper/src/main/java/tw/com/softleader/data/jpa/spec/SpecJoinContext.java:41
  • Evidence: entries are never cleaned; a cached/reused Specification accumulates per-execution Root keys unboundedly.

TEST-04 — Join machinery edge and error paths have no test coverage (testing, medium)

  • File: mapper/src/test/java/tw/com/softleader/data/jpa/spec/JoinSpecificationResolverTest.java:48

Plan

The cluster's root cause is shared: the resolvers assume well-formed, well-ordered input and silently mis-handle anything else. One pass, validate-or-fail-loud:

  1. Join.toPredicate: accumulate the distinct flag (query.distinct(query.isDistinct() || distinct)) instead of overwriting (COR-05).
  2. Alias registration: when the alias already exists, compare the stored path and joinType; throw a descriptive exception on mismatch (COR-06).
  3. Dotted-path handling: validate byDot.length == 2 and throw a named error citing the two-level limit for deeper paths (COR-07).
  4. Alias resolution in SimpleSpecification: when a dotted first segment matches no registered join alias, throw a descriptive error naming the alias and the declaration-order requirement instead of falling back to root.get (COR-10); when CTX_JOIN is absent (direct construction), throw a clear error explaining the mapper-pipeline requirement (MAINT-04).
  5. Resolve @JoinFetch aliases with identical join semantics in count and content queries so totals match content (COR-11).
  6. Scope join/fetch bookkeeping to a single query execution (or evict the per-Root entries when the execution completes) so reused Specifications do not leak (PERF-02).
  7. Tests (TEST-04): multi-join distinct accumulation; conflicting alias → exception; 3-segment path → exception; unregistered alias → exception; non-INNER fetch count-vs-content parity; reused-Specification repeated execution.

Release note: several silent-wrong-result paths become thrown exceptions — document the tightened validation.

Acceptance criteria

  • Every scenario above either returns correct results or fails loudly with a descriptive message; page totals match page content for non-INNER fetches; reused Specifications hold no unbounded state.
  • New tests from step 8 pass; regression: make test green.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

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