Spring Boot code review standards

# Spring Boot code review standards

Follow these standards when writing or changing code in this repository. Code review flags changes that break them.

## Catch @Transactional Pitfalls in Spring Boot

Applies to `**/*.java`.

Review Spring service code for `@Transactional` boundaries that never start, do not roll back, or hold database connections longer than needed.

Flag changes that:
- Call a `@Transactional` method from another method of the same class (`this.save(...)` or an unqualified call). The call skips the Spring proxy, so attributes such as `REQUIRES_NEW`, `readOnly`, or `rollbackFor` never apply.
- Put `@Transactional` on private or final methods, or call `@Transactional` methods on objects created with `new`, where no Spring proxy intercepts the call.
- Throw checked exceptions from a `@Transactional` method that should roll back on failure, without `rollbackFor` (or `rollbackOn` for `jakarta.transaction.Transactional`) covering them. By default only `RuntimeException` and `Error` roll back.
- Catch and continue after a runtime exception from an inner `@Transactional` call on another bean that joined the same transaction. The transaction is already marked rollback-only, and the outer commit throws `UnexpectedRollbackException`.
- Make HTTP calls, call other services, or send email inside a transaction, holding a pooled connection and row locks for the length of the remote call.
- Send messages or publish events that other systems act on before the transaction commits, instead of using `@TransactionalEventListener` or an outbox table.

Do not flag:
- Self-calls from a method that already runs in a transaction when the callee declares no different attributes.
- Checked exceptions on flows that are meant to commit, or projects that configure rollback for all exceptions globally.
- Code that uses AspectJ weaving (`AdviceMode.ASPECTJ`), where self-calls are advised.

## Prevent JPA N+1 Queries and Lazy Loading Traps in Spring Boot

Applies to `**/*.java`, `**/application*.yml`, `**/application*.yaml`, `**/application*.properties`.

Review Spring Data JPA repositories, services, and response mapping for queries that run once per row and lazy loads that happen outside the query that needs them.

Flag changes that:
- Read a lazy association, such as `order.getLines()` or a `LAZY` `@ManyToOne`, inside a loop, stream, or mapping over a query result that did not fetch it with `JOIN FETCH`, `@EntityGraph`, or a DTO projection.
- Serialize JPA entities that have lazy associations to JSON, so Jackson triggers the loads while rendering the response.
- Rely on `spring.jpa.open-in-view` (enabled by default) to make lazy loading work in controllers or views, or set `hibernate.enable_lazy_load_no_trans` to silence `LazyInitializationException`.
- Map a collection with `fetch = FetchType.EAGER`, or leave `@ManyToOne` and `@OneToOne` at their EAGER default on entities that are loaded in lists.
- Combine a collection `JOIN FETCH`, or a collection attribute in `@EntityGraph`, with a `Pageable`, which makes Hibernate load every matching row and paginate in memory.
- Call `findById` or a derived finder inside a loop over IDs instead of one `findAllById` or `IN` query.

Do not flag:
- Lazy access on a single loaded entity, such as a detail endpoint that reads one association once.
- Loops over results already fetched with `JOIN FETCH` or `@EntityGraph`, or covered by `@BatchSize` or `hibernate.default_batch_fetch_size`.
- Small bounded result sets, such as a top-10 query or a fixed lookup table.

## Validate Request DTOs and Stop Exposing Entities in Spring Boot

Applies to `**/*.java`.

Review Spring MVC and WebFlux controllers for request bodies that skip validation and for JPA entities used as request or response types.

Flag changes that:
- Take a `@RequestBody` or `@ModelAttribute` parameter whose type declares `jakarta.validation` constraints without `@Valid` or `@Validated` on the parameter, so the constraints never run.
- Add nested objects or collections to a request DTO without `@Valid` on the field, so constraints on the nested type are skipped.
- Bind a `@RequestBody` or `@ModelAttribute` directly onto a JPA entity, letting clients set fields such as `id`, `role`, or `ownerId`. A client-supplied `id` can make repository `save()` merge over an existing row.
- Copy request data onto an entity with `BeanUtils.copyProperties` or a mapper that copies every matching field, instead of naming the fields the client may change.
- Return JPA entities, or `List`, `Page`, or `ResponseEntity` of entities, from controller methods, exposing internal fields such as password hashes and triggering lazy loads during serialization.

Do not flag:
- Controllers that validate explicitly with an injected `Validator` before using the input.
- Response types that are records, DTOs, or Spring Data projections, even when they mirror entity fields.
- Entities added to the model of a server-rendered view (Thymeleaf, JSP), where only the fields the template reads are rendered.

## Require Authorization on Every Spring Boot Endpoint

Applies to `**/*.java`.

Review Spring Security configuration and controller endpoints for requests that reach business logic without an authorization decision.

Flag changes that:
- Add admin, internal, or data-changing endpoints protected only by `anyRequest().authenticated()`, with no role rule, `@PreAuthorize`, `@Secured`, or other authorization check.
- Add broad `permitAll()` matchers such as `"/api/**"` or `"/**"`, or place a `permitAll()` rule before a more specific restriction. `authorizeHttpRequests` applies the first rule that matches.
- End the rules with `anyRequest().permitAll()` instead of `anyRequest().authenticated()` or `denyAll()`.
- Use `web.ignoring()` in a `WebSecurityCustomizer` for application endpoints, which removes them from the Spring Security filter chain.
- Use `@PreAuthorize` or `@PostAuthorize` in an application that enables neither `@EnableMethodSecurity` nor, on Spring Security 5, `@EnableGlobalMethodSecurity(prePostEnabled = true)`, or use `@Secured` without `securedEnabled = true` on whichever of the two it uses, so the annotations are never enforced.
- Load, update, or delete a record by a client-supplied ID, such as `findById(id)` or `deleteById(id)`, without checking that it belongs to the current user, account, or tenant.

Do not flag:
- Endpoints that are public by design and listed explicitly, such as login, health checks, static assets, or webhook receivers that verify a signature.
- Endpoints already covered by a path rule that requires the needed role or authority.
- Lookups scoped to the principal, such as `findByIdAndAccountId(id, user.getAccountId())`, or guarded by a `@PreAuthorize` or `@PostAuthorize` ownership expression.

## Lock Down Spring Boot Actuator, Secrets, and CORS

Applies to `**/application*.yml`, `**/application*.yaml`, `**/application*.properties`, `**/*.java`.

Review Spring Boot configuration and web security setup for exposed Actuator endpoints, committed secrets, and CORS rules that let other sites make credentialed requests.

Flag changes that:
- Set `management.endpoints.web.exposure.include` to `"*"`, or expose `env`, `heapdump`, `threaddump`, `configprops`, `loggers`, or `shutdown` over HTTP without a security rule restricting them.
- Set `show-values` to `always` for the `env` or `configprops` endpoint, or `management.endpoint.health.show-details` to `always` on a publicly reachable health endpoint.
- Permit Actuator paths beyond health and info in a custom `SecurityFilterChain`, for example `EndpointRequest.toAnyEndpoint().permitAll()`.
- Commit passwords, API keys, JWT signing keys, or OAuth client secrets in `application.yml`, `application.properties`, or profile variants such as `application-prod.yml`.
- Give a secret placeholder a usable default, such as `${JWT_SECRET:changeme}`, so a missing environment variable starts the app with a known secret.
- Combine `allowCredentials(true)` with `allowedOriginPatterns("*")`, or echo the request `Origin` header back, in `CorsConfiguration`, `WebMvcConfigurer`, or `@CrossOrigin`.

Do not flag:
- Local-only credentials in test resources or a local profile that point at localhost, Docker Compose, or Testcontainers.
- Actuator endpoints exposed only on a separate `management.server.port` that is not publicly routed, or restricted to an operations role.
- `allowedOrigins("*")` without credentials on public, read-only APIs.

## Flag Security Vulnerabilities

Scan for security flaws: hardcoded secrets, SQL injection, XSS, weak auth, exposed data, poor crypto, unsafe deserialization, insecure HTTP calls in production, and more. Ensure all network requests use HTTPS/TLS. Identify attack vectors and security violations to maintain robust defenses.

## Require API Auth, Validation, and Tenant Scoping

Review API handlers and controller endpoints for missing authentication, authorization, tenant scoping, and request validation.

Flag new or modified endpoints that:
- Accept request bodies without runtime validation, such as Zod schemas, DTOs, or framework-native validators.
- Trust user-supplied identifiers for tenant, organization, or account access instead of deriving access from the authenticated session.
- Return sensitive internal errors or use inconsistent HTTP status codes for 400, 401, 403, 404, and 500 cases.
- Mix reads and writes in a way that makes side effects hard to reason about.
- Add public or high-volume endpoints without rate limiting or abuse controls.

## Prevent Database Query Performance Regressions

Flag database access patterns that can degrade production performance, including N+1 queries, unbounded list endpoints, missing pagination, full-table scans, missing indexes on selective filters, per-row writes in loops, and queries that load more columns or rows than needed. Prefer bounded reads, batching, selective indexes, and query shapes that scale with table size.

## Block Unsafe Database Migrations

Review database migrations for operations that can lock large tables, rewrite full tables, run long backfills, create blocking indexes, or require full table scans. Prefer phased rollouts, concurrent indexes, nullable columns followed by backfills, batched data changes, and explicit rollback or recovery plans for risky schema changes.

## Prevent Sensitive Data in Logs

Logging, error reporting, analytics events, and audit trails must not expose sensitive data. Flag code that logs PII, emails, passwords, API keys, auth tokens, session identifiers, payment data, or raw request/response bodies without redaction. Error serialization must preserve enough context to debug while sanitizing secrets and user data.

## Enforce Pragmatic Test Coverage

Be reasonable when evaluating tests. Flag changes when important behavior is untested, but do not require tests for every export, type, config, metadata file, or mechanical change.

Flag PRs that:
- Change core business logic without covering the main success and failure paths.
- Add UI behavior without testing the user-visible interaction or state change.
- Add tests that only assert rendering, existence, snapshots, or that a function returns any value.
- Skip, mute, or weaken existing tests without a clear reason.
- Mock so much of the system that the test no longer validates the behavior the PR claims to change.

## Enforce Strict Maintainability Standards

Review changes for structural maintainability regressions. Behavior-correct code can still violate this rule if it makes the codebase harder to understand, extend, or decompose.

Be ambitious about structural simplification. Look for reframings that delete whole branches, helpers, modes, conditionals, or layers instead of adding more of them. Prefer solutions that remove complexity rather than rearrange it.

Flag PRs that:
- Push a file from under 1,000 lines to over 1,000 without a strong reason or clear organization. Prefer extracting helpers, subcomponents, modules, or local abstractions first.
- Add ad-hoc conditionals, scattered special cases, one-off booleans, nullable modes, or flags into already busy shared flows instead of a dedicated abstraction, helper, state machine, policy object, or separate module.
- Introduce thin wrappers, identity abstractions, or pass-through helpers that add indirection without reducing complexity.
- Use brittle, ad-hoc, or "magic" mechanisms that hide simple data-shape assumptions.
- Rely on unnecessary casts, any, unknown, or optional parameters when a clearer type boundary or explicit model would simplify control flow.
- Use silent fallbacks to paper over unclear invariants instead of making the boundary explicit.
- Duplicate existing helpers or put logic in the wrong layer when a canonical utility or module already owns the concept.
- Leak feature-specific logic into general-purpose or shared paths.
- Copy-paste logic instead of extracting a shared helper or pure function.
- Handle narrow edge cases in the middle of an already busy function.
- Refactor in ways that move complexity around without reducing the number of concepts a reader must hold in their head.
- Add "temporary" branching that is likely to become permanent debt.
- Serialize obviously independent work or leave related updates in a non-atomic shape when a cleaner structure is available.

Prioritize findings in this order:
1. Structural code-quality regressions
2. Missed opportunities for dramatic simplification
3. Spaghetti or branching complexity increases
4. Boundary, abstraction, or type-contract problems
5. File-size and decomposition concerns
6. Modularity and legibility concerns

Do not flag cosmetic nits when a larger structural issue is present. Prefer a smaller number of high-conviction findings over a long list of minor notes. Only report violations on changed lines with clear evidence from the diff.