Repository navigation
[WP9] Dead Ordered/getOrder contract on custom resolvers #186
Description
此內容由 AI 產生（specification-mapper 全庫審查 2026-07-16）。
Work package WP9 · findings: MAINT-01 · 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.
MAINT-01 — Ordered/getOrder contract is documented but never honored anywhere — resolver order is purely insertion order
- Category / severity: maintainability / medium · effort M · confidence high
- Location:
mapper/src/main/java/tw/com/softleader/data/jpa/spec/SpecificationResolver.java:73 - Evidence: SpecificationResolver extends Ordered and its javadoc (lines 42-43) says implementations 'can define the order in which they should be applied by implementing the Ordered interface', and SpecificationResolverBuilder exposes order(int) (line 130) which the built resolver returns (line 160). But SpecMapper.resolveSpec streams
resolvers.stream()(SpecMapper.java line 122) without ever sorting by getOrder, and SpecificationResolverBuilder's own getOrder() hardcodes 0 (lines 166-169) ignoring the configured order field, so the starter'sbuilders.orderedStream()(SpecMapperAutoConfiguration.java line 96) also ignores it. - Failure scenario: A user registers a custom resolver via SpecificationResolver.builder().order(Ordered.HIGHEST_PRECEDENCE)... expecting it to run before the default resolvers (e.g. to seed the Context or register join aliases before SimpleSpecificationResolver reads them); it actually runs last, so its context contributions are invisible to the specs built earlier and the produced Specification composes predicates in the wrong order — with no error and no log.
- Suggested fix: Either sort resolvers by Ordered (stable sort keeps insertion order for equal values, preserving the default chain) in SpecMapperBuilder.build(), make SpecificationResolverBuilder.getOrder() return the configured order, or delete the Ordered inheritance and order(int) and document insertion-order semantics.
- Verification: [CONFIRMED] Traced all cited code. SpecMapper.java:122 (resolveSpec) uses resolvers.stream() with no sort; SpecMapper.java:243-247 (build) collects in pure insertion order with defaultResolvers() prepended. grep for getOrder/sorted/OrderComparator across mapper/ and starter/ confirms the core mapper module never consumes getOrder. The built resolver's getOrder() (SpecificationResolver.java:160) returns the configured order field but is never read anywhere. The builder's own Ordered.getOrder() (line 167) hardcodes 0, so SpecMapperAutoConfiguration.java:96 builders.orderedStream() ignores order(int). resolvers.orderedStream() (line 95) does sort standalone resolver beans by getOrder, but only …
Full report: see the codebase-review-2026-07-16 branch (linked on the parent tracking issue).
Activity
shihyuho commentedon Jul 16, 2026
Triage Notes - needs human
Category: bug
Summary: The
Ordered/getOrder()contract advertised onSpecificationResolver(and configurable viaSpecificationResolver.builder().order(int)) is never honored by the core mapper, so custom resolver ordering is silently ignored — but which way to resolve it is a design/API decision.Current behavior:
SpecificationResolverextendsorg.springframework.core.Orderedand its javadoc states implementations "can define the order in which they should be applied by implementing theOrderedinterface".SpecificationResolver.SpecificationResolverBuilderexposesorder(int), and the resolver produced bySpecificationResolverBuilder.build()returns that configured value fromgetOrder().SpecMapper.resolveSpec(Context, Databind)iteratesresolvers.stream()with no ordering step, andSpecMapper.SpecMapperBuilder.build()collects resolvers in pure insertion order (withdefaultResolvers()prepended). The core mapper module never consumesgetOrder()anywhere. SogetOrder()on a builder-built resolver is dead.SpecificationResolverBuilder.getOrder()(the builder's ownOrderedimplementation) is hardcoded to return0, ignoring the configuredorderfield. In the starter,SpecMapperAutoConfiguration.specMapper(...)callsbuilders.orderedStream(), which therefore cannot see the configured order at all — builder beans are never reordered by theirorder(int).resolvers.orderedStream()does sort standaloneSpecificationResolverbeans among themselves, but they are appended to the mapper afterdefaultResolvers(), so even aHIGHEST_PRECEDENCEcustom resolver still runs after the default chain (NestedSpecificationResolver,JoinFetchSpecificationResolver,JoinSpecificationResolver,SimpleSpecificationResolver).order(Ordered.HIGHEST_PRECEDENCE)expecting to run before the default resolvers gets no error, no log, and their resolver runs last — predicates compose in an unexpected order.Desired behavior: One coherent, documented ordering semantics — either the
Orderedcontract is actually honored end to end, or the dead API is removed and insertion-order semantics are documented. See Needs human because for the decision.Key interfaces:
tw.com.softleader.data.jpa.spec.SpecificationResolver(extendsOrdered;default int getOrder()).tw.com.softleader.data.jpa.spec.SpecificationResolver.SpecificationResolverBuilder(order(int),getOrder(),build()).tw.com.softleader.data.jpa.spec.SpecMapper.resolveSpec(Context, Databind)andSpecMapper.SpecMapperBuilder.build()/defaultResolvers().tw.com.softleader.data.jpa.spec.starter.autoconfigure.SpecMapperAutoConfiguration.specMapper(...)(resolvers.orderedStream(),builders.orderedStream(),codecBuilders.orderedStream()).tw.com.softleader.data.jpa.spec.starter.SpecificationResolverCodecBuilder(also declaresdefault int getOrder()returning0).Acceptance criteria (to be finalized once the direction below is chosen):
SpecificationResolver.builder().order(int)either measurably affects execution order or is removed; no configurable-but-ignored knob remains.SpecificationResolverBuilder.getOrder()no longer contradicts the configuredorderfield (either it returns the field, or the field andorder(int)are removed).defaultResolvers()) is preserved.mappermodule asserts the chosen ordering semantics for a custom resolver relative to the default resolvers; a starter test asserts builder/codec-builder bean ordering behaves as documented.SpecificationResolvermatches actual behavior.Out of scope:
Context/Databindor predicate-composition logic beyond ordering.Needs human because: The fix requires a design/API decision with mutually exclusive, behavior- or contract-changing options, each needing sign-off:
Ordered— sort resolvers bygetOrder()(stable, so equal-order defaults keep insertion order) inSpecMapper.SpecMapperBuilder.build(), and makeSpecificationResolverBuilder.getOrder()return the configuredorder. This is a behavioral change: it would let custom resolvers interleave with or precede the default chain, changing the documented "Join must precede Simple" guarantees for existing users, and must reconcile core-mapper ordering with the starter'sorderedStream()(custom beans currently always append after defaults). Whether custom resolvers should be allowed to run before defaults is a policy call.Orderedinheritance fromSpecificationResolver, removeorder(int)/theorderfield fromSpecificationResolverBuilder, and document insertion-order semantics. This is a breaking public-API removal requiring release sign-off, and interacts with the starter'sorderedStream()usage on resolver/builder/codec-builder beans.Picking between (1) and (2), and judging the acceptable breaking change / default policy, is a human decision. Once chosen, the remaining implementation is agent-sized.
shihyuho commentedon Jul 20, 2026
Superseded by #203 in the execution map #204, which carries the same finding (MAINT-01) with a plan and acceptance criteria. This 2026-07-16 static plan is being retired in favour of that map.