fix(server): enforce Spring correctness rules - #1517
Merged
Conversation
Contributor
|
Warning Review limit reachedNext included review available in 16 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (77)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
application-server
github-actions
Bot
added
size:XXL
FelixTJDietrich
force-pushed
the
1493-spring-correctness-rules
branch
from
August 24, 2026 12:09
4a9305c to
804f05b
Compare
FelixTJDietrich
changed the title
FelixTJDietrich
force-pushed
the
1493-spring-correctness-rules
branch
from
August 24, 2026 13:20
804f05b to
6855b8b
Compare
github-actions
Bot
added
the
webapp
FelixTJDietrich
force-pushed
the
1493-spring-correctness-rules
branch
from
August 24, 2026 13:44
bbdba03 to
591a093
Compare
Contributor
📚 Documentation Preview
|
FelixTJDietrich
added a commit
that referenced
this pull request
Aug 24, 2026
#1517 added a rule that a @transactional method may not be reached by self-invocation, because the call never passes the proxy and the inner annotation is decorative. Both classes this branch adds broke it seven times: the convenience overloads of the coverage service and the delivery policy delegated to each other, so only the outermost annotation ever took effect. Each public entry point now opens its own read-only transaction and goes straight to a private implementation that assumes one. Behaviour is unchanged — the reads were already running inside the outer transaction — but the annotations now say what actually happens. The same merge made the two policy entry points public, which is what makes their @transactional apply at all.
FelixTJDietrich
added a commit
that referenced
this pull request
Aug 24, 2026
#1517 added a rule that a @transactional method may not be reached by self-invocation, because the call never passes the proxy and the inner annotation is decorative. Both classes this branch adds broke it seven times: the convenience overloads of the coverage service and the delivery policy delegated to each other, so only the outermost annotation ever took effect. Each public entry point now opens its own read-only transaction and goes straight to a private implementation that assumes one. Behaviour is unchanged — the reads were already running inside the outer transaction — but the annotations now say what actually happens. The same merge made the two policy entry points public, which is what makes their @transactional apply at all.
FelixTJDietrich
added a commit
that referenced
this pull request
Aug 25, 2026
#1517 added a rule that a @transactional method may not be reached by self-invocation, because the call never passes the proxy and the inner annotation is decorative. Both classes this branch adds broke it seven times: the convenience overloads of the coverage service and the delivery policy delegated to each other, so only the outermost annotation ever took effect. Each public entry point now opens its own read-only transaction and goes straight to a private implementation that assumes one. Behaviour is unchanged — the reads were already running inside the outer transaction — but the annotations now say what actually happens. The same merge made the two policy entry points public, which is what makes their @transactional apply at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a single ArchUnit suite that makes silent Spring correctness failures fail in CI. It enforces proxy-safe transaction boundaries, constructor injection, request validation, safe lifecycle usage, and Spring Data collection return contracts.
The change also fixes every existing violation uncovered by the new rules—there are no baselines, allowlists, or suppressed findings. Transaction boundaries were preserved explicitly: same-transaction delegation now uses private non-annotated methods, independent work uses focused collaborators or
TransactionTemplate, and remote pagination/retry work remains outside database transactions.The enforced contracts are:
@Transactionalmethods are public and non-final;@Autowired;@RequestBodyparameters use@Valid;@PostConstructmethods are not transactional;Optional; and@Transactional, not the JTA annotation.Request validation now uses real
@NotNullconstraints for required values rather than inert annotations. The resulting client-facing behavior is documented in the patch changeset, and the OpenAPI specification and generated client are updated.Each rule was verified against a deliberate violation before the production code was corrected. Those temporary violations were removed rather than retained as test-only theatre.
These checks implement the documented behavior of Spring transaction proxies, Spring AOP proxying, Spring MVC validation, and Spring Data repository return types.
Fixes #1493
How to test
Run the same local gates used for this branch:
All four commands pass locally. For a manual smoke test, temporarily add a private or final
@Transactionalmethod, a same-class call to a transactional method, or field@Autowired; the focused architecture test should report the violating member. Revert the deliberate violation afterward.Checklist
.changeset/README.md**Operators:** …) andMIGRATION.mdis updated — no operator action is required