[WP10] Mapper test-coverage gaps: concurrency, nested-AND typo, inherited fields #182

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

Work package WP10 · findings: TEST-06, TEST-05, TEST-03 · 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.

TEST-06 — fireOnlyOnce concurrency test runs with a single thread, leaving the AtomicBoolean+CountDownLatch machinery untested

• Category / severity: testing / medium · effort S · confidence high
• Location: mapper/src/test/java/tw/com/softleader/data/jpa/spec/ReflectionDatabindTest.java:52
• Evidence: The test creates var numberOfThreads = 1; and a thread pool of that size, so all getFieldValue calls happen sequentially on one worker thread; the latch.await() loser branch and the CAS race in ReflectionDatabind.getFieldValue are never exercised, and the times(1) verification would pass with no synchronization at all.
• Failure scenario: A regression in the bespoke lazy-load code (e.g. removing the latch, or the existing missing-countDown-on-exception defect) keeps the suite green because the only test targeting this code path never runs it concurrently; a real double-load or hang only surfaces in production usage.
• Suggested fix: Raise numberOfThreads (e.g. 32) with a start-barrier so threads hit getFieldValue simultaneously, and assert the underlying reflective read happened exactly once; add a test for a throwing field read not poisoning subsequent calls.
• Verification: [CONFIRMED] Traced ReflectionDatabindTest.java:40-70 and ReflectionDatabind.getFieldValue(). The test sets numberOfThreads=1 (line 52), builds a 1-thread pool, and submits a single task calling databind.forEach(Databind::getFieldValue) exactly once. Therefore every bind's no-arg getFieldValue() is invoked once, sequentially, on one worker. In getFieldValue(), loaded.compareAndSet(false,true) always wins on that single first call, so the else/latch.await() loser branch is never entered and the CAS race is never contended. The verify(times(1)).getFieldValue(object, field) assertion is trivially satisfied even if the AtomicBoolean/latch dedup were removed, since the inner reflective read only f…

TEST-05 — NestedInNestedAnd.name is annotated @NestedSpec on a String (likely a @SPEC typo), so the three-level AND scenario is never actually tested

• Category / severity: testing / medium · effort S · confidence high
• Location: mapper/src/test/java/tw/com/softleader/data/jpa/spec/NestedSpecificationResolverTest.java:392
• Evidence: NestedInNestedAnd declares @NestedSpec String name; while its OR twin NestedInNestedOr declares @Spec String name; (line 420). Recursing @NestedSpec into java.lang.String finds no annotated fields and yields null, so allAnd's assertion tree only expects the second-level Conjunction to have size 1 (lines 115-120) — i.e. the deepest level contributes nothing — whereas allOr asserts a full three-level Disjunction tree.
• Failure scenario: The 'nested in nested' AND combination path in NestedSpecificationResolver/Conjunction is not covered: a regression that breaks spec composition at depth 3 for conjunctions passes this suite because the deepest AND level silently maps to null; the test also reflects over java.lang.String internals at runtime, which can trip JPMS warnings in future JDKs.
• Suggested fix: Change NestedInNestedAnd.name to @SPEC (mirroring NestedInNestedOr) and update allAnd's assertions to expect the third-level Equals inside the nested Conjunction.
• Verification: [CONFIRMED] Traced NestedSpecificationResolverTest.java:392 — NestedInNestedAnd declares @NestedSpec String name; vs NestedInNestedOr's @Spec String name; (line 420). In NestedSpecificationResolver.java:50-72, a @NestedSpec field recurses via codec.toSpec into the field value; for a String value the recursion finds no annotated fields and returns null, so NestedInNestedAnd yields null and NestedAnd.nin collapses to null. This matches allAnd's assertions (lines 114-120) where the second-level Conjunction is .hasSize(1) (only the @SPEC name Equals) — the deepest AND level is absent — whereas allOr (lines 154-163) asserts a real third-level Disjunction with an Equals. The three-level AND …

TEST-03 — Inherited fields of criteria POJOs are silently ignored (doWithLocalFields) and no test pins this contract

• Category / severity: testing / medium · effort S · confidence high
• Location: mapper/src/main/java/tw/com/softleader/data/jpa/spec/ReflectionDatabind.java:80
• Evidence: ReflectionDatabind.of uses ReflectionUtils.doWithLocalFields (line 80), so only the concrete class's declared fields are mapped — @Spec/@Join fields on a superclass are dropped without warning. No test class in either module uses a criteria POJO with a superclass (grep for criteria classes extending anything finds none), so the behavior is neither asserted as intended nor guarded against regression.
• Failure scenario: A team factors common filters (e.g., tenantId with @SPEC) into a BaseCriteria and extends it; the inherited filter silently never becomes a predicate, and every query returns rows across all tenants — a silent data-exposure bug the test suite cannot catch because inheritance is entirely uncovered.
• Suggested fix: Add a test with a criteria subclass inheriting @SPEC fields pinning the chosen behavior; if local-only is intentional, also consider a debug/warn log when superclass declares annotated fields.
• Verification: [CONFIRMED] Traced the code. ReflectionDatabind.of at mapper/.../ReflectionDatabind.java:80 uses Spring's doWithLocalFields, which iterates only the concrete class's getDeclaredFields() and does not walk superclasses (contrast doWithFields). This is the live criteria-mapping path: SpecMapper.java:108 calls ReflectionDatabind.of(rootObject, ...) to build the databind list that feeds the @Spec/@Join/@NestedSpec resolvers. Therefore annotated fields declared on a superclass of a criteria POJO are never enumerated and silently produce no predicate — the behavioral claim is accurate. The test-coverage claim also holds: grepping test sources, every extends combined with mapper annotations is a S…

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