name: spring-boot-reviewer
description: |
WHEN: Spring Boot code review, DI patterns, @Transactional, REST API design, security configuration
WHAT: Dependency injection + Transaction management + API design + Security config + JPA patterns
WHEN NOT: Kotlin Spring → kotlin-spring-reviewer, Pure Java → java-reviewer, Django/FastAPI → respective reviewers
Spring Boot Reviewer Skill
Purpose
Reviews Spring Boot applications for dependency injection patterns, transaction management, REST API design, security configuration, and JPA best practices.
When to Use
- Spring Boot code review requests
- "Spring", "@Transactional", "JPA", "REST controller" mentions
- API security configuration review
- Projects with
spring-boot-starter-* dependencies
@SpringBootApplication class present
Project Detection
spring-boot-starter-* in pom.xml/build.gradle
@SpringBootApplication annotation
application.yml or application.properties
src/main/resources/application*.yml
Workflow
Step 1: Analyze Project
**Spring Boot**: 3.2.x
**Java**: 17 / 21
**Dependencies**:
- spring-boot-starter-web
- spring-boot-starter-data-jpa
- spring-boot-starter-security
- spring-boot-starter-validation
Step 2: Select Review Areas
AskUserQuestion:
"Which Spring Boot areas to review?"
Options:
- Full Spring Boot audit (recommended)
- Dependency Injection patterns
- Transaction management
- REST API design
- Security configuration
- JPA/Repository patterns
multiSelect: true
Detection Rules
Critical: Field Injection
| Pattern | Issue | Severity |
|---|
@Autowired on field | Not testable | HIGH |
@Inject on field | Same issue | HIGH |
@Value on field | Consider constructor | MEDIUM |
// BAD: Field injection
@Service
public class UserService {
@Autowired
private UserRepository userRepository;
@Autowired
private EmailService emailService;
}
// GOOD: Constructor injection
@Service
public class UserService {
private final UserRepository userRepository;
private final EmailService emailService;
public UserService(UserRepository userRepository,
EmailService emailService) {
this.userRepository = userRepository;
this.emailService = emailService;
}
}
// BETTER: Lombok + constructor injection
@Service
@RequiredArgsConstructor
public class UserService {
private final UserRepository userRepository;
private final EmailService emailService;
}
Critical: JPA N+1 Query Problem
| Pattern | Issue | Severity |
|---|
| Lazy load in loop | N+1 queries | CRITICAL |
Missing @EntityGraph | Suboptimal fetching | HIGH |
No fetch join | Multiple queries | HIGH |
// BAD: N+1 problem
@Entity
public class Order {
@OneToMany(mappedBy = "order", fetch = FetchType.LAZY)
private List<OrderItem> items;
}
// In service - N+1 queries!
List<Order> orders = orderRepository.findAll();
for (Order order : orders) {
order.getItems().size(); // Triggers query per order
}
// GOOD: Fetch join in repository
@Query("SELECT o FROM Order o JOIN FETCH o.items WHERE o.status = :status")
List<Order> findByStatusWithItems(@Param("status") OrderStatus status);
// GOOD: @EntityGraph
@EntityGraph(attributePaths = {"items", "customer"})
List<Order> findByStatus(OrderStatus status);
// GOOD: Batch fetching
@Entity
public class Order {
@OneToMany(mappedBy = "order")
@BatchSize(size = 20) // Fetch 20 at a time
private List<OrderItem> items;
}
Critical: Missing Security
| Pattern | Issue | Severity |
|---|
No @PreAuthorize | Unauthorized access | CRITICAL |
| Hardcoded credentials | Security breach | CRITICAL |
| Exposed Actuator endpoints | Info/control leak | CRITICAL |
| Missing CSRF config | CSRF vulnerable | HIGH |
| No rate limiting | DoS vulnerable | HIGH |
| Entity returned from Controller | Data leak / OSIV | HIGH |
// BAD: No authorization check
@RestController
@RequestMapping("/api/admin")
public class AdminController {
@GetMapping("/users")
public List<User> getAllUsers() { // Anyone can access!
return userService.findAll();
}
}
// GOOD: Method-level security
@RestController
@RequestMapping("/api/admin")
public class AdminController {
@GetMapping("/users")
@PreAuthorize("hasRole('ADMIN')")
public List<User> getAllUsers() {
return userService.findAll();
}
}
// Security configuration
@Configuration
@EnableWebSecurity
@EnableMethodSecurity
public class SecurityConfig {
@Bean
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
return http
.authorizeHttpRequests(auth -> auth
.requestMatchers("/api/public/**").permitAll()
.requestMatchers("/api/admin/**").hasRole("ADMIN")
.anyRequest().authenticated()
)
.csrf(csrf -> csrf.csrfTokenRepository(
CookieCsrfTokenRepository.withHttpOnlyFalse()))
.build();
}
}
High: Controller with Business Logic
| Pattern | Issue | Severity |
|---|
| DB access in controller | Layer violation | HIGH |
| Complex logic in controller | Not testable | HIGH |
| Transaction in controller | Wrong layer | HIGH |
// BAD: Business logic in controller
@RestController
@RequestMapping("/api/orders")
public class OrderController {
@Autowired
private OrderRepository orderRepository;
@PostMapping
@Transactional // Wrong layer!
public Order createOrder(@RequestBody CreateOrderRequest request) {
// Business logic in controller
if (request.getItems().isEmpty()) {
throw new BadRequestException("Items required");
}
Order order = new Order();
order.setCustomerId(request.getCustomerId());
BigDecimal total = BigDecimal.ZERO;
for (ItemRequest item : request.getItems()) {
total = total.add(item.getPrice().multiply(
BigDecimal.valueOf(item.getQuantity())));
}
order.setTotal(total);
return orderRepository.save(order);
}
}
// GOOD: Thin controller, service layer
@RestController
@RequestMapping("/api/orders")
@RequiredArgsConstructor
public class OrderController {
private final OrderService orderService;
@PostMapping
public ResponseEntity<OrderResponse> createOrder(
@Valid @RequestBody CreateOrderRequest request) {
Order order = orderService.create(request);
return ResponseEntity.created(
URI.create("/api/orders/" + order.getId()))
.body(OrderResponse.from(order));
}
}
@Service
@RequiredArgsConstructor
public class OrderService {
private final OrderRepository orderRepository;
@Transactional // Correct layer
public Order create(CreateOrderRequest request) {
// Business logic here
Order order = buildOrder(request);
return orderRepository.save(order);
}
}
High: Missing @Transactional
| Pattern | Issue | Severity |
|---|
| Multiple saves without txn | Partial commit | HIGH |
| Self-invocation of @Transactional | Proxy bypassed | CRITICAL |
Read without readOnly | Missed optimization | MEDIUM |
| Wrong propagation | Unexpected behavior | HIGH |
// BAD: No transaction - partial failure
@Service
public class TransferService {
public void transfer(Long fromId, Long toId, BigDecimal amount) {
Account from = accountRepository.findById(fromId).orElseThrow();
Account to = accountRepository.findById(toId).orElseThrow();
from.setBalance(from.getBalance().subtract(amount));
accountRepository.save(from);
// Exception here = money lost!
to.setBalance(to.getBalance().add(amount));
accountRepository.save(to);
}
}
// GOOD: Transactional
@Service
public class TransferService {
@Transactional
public void transfer(Long fromId, Long toId, BigDecimal amount) {
Account from = accountRepository.findById(fromId).orElseThrow();
Account to = accountRepository.findById(toId).orElseThrow();
from.setBalance(from.getBalance().subtract(amount));
to.setBalance(to.getBalance().add(amount));
// Both or nothing
}
}
// GOOD: readOnly for queries
@Transactional(readOnly = true)
public List<Account> findAll() {
return accountRepository.findAll();
}
// GOOD: Propagation for nested transactions
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void logAudit(String action) {
// Commits even if outer transaction rolls back
}
// CRITICAL: Self-invocation bypasses proxy
@Service
public class OrderService {
@Transactional
public void processOrder(Order order) {
// ...
this.sendNotification(order); // BAD: @Transactional ignored!
}
@Transactional(propagation = Propagation.REQUIRES_NEW)
public void sendNotification(Order order) {
// This runs WITHOUT a new transaction due to self-invocation
}
}
// GOOD: Inject self or use separate service
@Service
public class OrderService {
private final NotificationService notificationService; // Separate bean
@Transactional
public void processOrder(Order order) {
notificationService.sendNotification(order); // Proxy works
}
}
High: Missing DTO Validation
| Pattern | Issue | Severity |
|---|
No @Valid on request | Unvalidated input | HIGH |
| Missing validation annotations | Bad data accepted | HIGH |
| No error handling | 500 on validation fail | MEDIUM |
// BAD: No validation
@PostMapping("/users")
public User createUser(@RequestBody CreateUserRequest request) {
// name could be null, email invalid
return userService.create(request);
}
// GOOD: Validated request
public record CreateUserRequest(
@NotBlank(message = "Name is required")
@Size(max = 100, message = "Name too long")
String name,
@NotBlank
@Email(message = "Invalid email format")
String email,
@NotNull
@Min(value = 0, message = "Age must be positive")
@Max(value = 150, message = "Invalid age")
Integer age
) {}
@PostMapping("/users")
public ResponseEntity<UserResponse> createUser(
@Valid @RequestBody CreateUserRequest request) {
return ResponseEntity.ok(userService.create(request));
}
// Global exception handler
@RestControllerAdvice
public class GlobalExceptionHandler {
@ExceptionHandler(MethodArgumentNotValidException.class)
public ResponseEntity<ErrorResponse> handleValidation(
MethodArgumentNotValidException ex) {
List<String> errors = ex.getBindingResult()
.getFieldErrors().stream()
.map(error -> error.getField() + ": " + error.getDefaultMessage())
.toList();
return ResponseEntity.badRequest()
.body(new ErrorResponse("Validation failed", errors));
}
}
High: Hardcoded Configuration
| Pattern | Issue | Severity |
|---|
| Hardcoded URL in code | Not configurable | HIGH |
| Hardcoded credentials | Security risk | CRITICAL |
| Magic numbers | Maintainability | MEDIUM |
// BAD: Hardcoded configuration
@Service
public class ExternalApiService {
private final String apiUrl = "https://api.example.com"; // Hardcoded
private final String apiKey = "sk-secret-key"; // CRITICAL!
private final int timeout = 5000;
}
// GOOD: Externalized configuration
@Configuration
@ConfigurationProperties(prefix = "external-api")
@Validated
public class ExternalApiProperties {
@NotBlank
private String url;
@NotBlank
private String apiKey;
@Min(100)
@Max(60000)
private int timeout = 5000;
// getters, setters
}
@Service
@RequiredArgsConstructor
public class ExternalApiService {
private final ExternalApiProperties properties;
public void call() {
// Use properties.getUrl(), etc.
}
}
// application.yml
external-api:
url: ${EXTERNAL_API_URL}
api-key: ${EXTERNAL_API_KEY}
timeout: 5000
Critical: WebFlux Blocking Calls
| Pattern | Issue | Severity |
|---|
| JDBC in WebFlux | Blocks event loop | CRITICAL |
| Thread.sleep() | Blocks thread | CRITICAL |
| Blocking I/O | Performance death | CRITICAL |
// BAD: Blocking call in reactive stack
@RestController
public class ReactiveController {
@Autowired
private JdbcTemplate jdbcTemplate; // Blocking!
@GetMapping("/users")
public Mono<List<User>> getUsers() {
// Blocks event loop thread!
List<User> users = jdbcTemplate.query(...);
return Mono.just(users);
}
}
// GOOD: Use R2DBC for reactive DB access
@RestController
@RequiredArgsConstructor
public class ReactiveController {
private final UserRepository userRepository; // R2DBC
@GetMapping("/users")
public Flux<User> getUsers() {
return userRepository.findAll(); // Non-blocking
}
}
// If must use blocking: subscribeOn(Schedulers.boundedElastic())
@GetMapping("/legacy")
public Mono<String> callLegacy() {
return Mono.fromCallable(() -> legacyBlockingService.call())
.subscribeOn(Schedulers.boundedElastic());
}
Medium: Missing API Versioning
| Pattern | Issue | Severity |
|---|
| No version in URL | Breaking changes | MEDIUM |
| No version header | Hard to evolve | MEDIUM |
// BAD: No versioning
@RestController
@RequestMapping("/api/users")
public class UserController { }
// GOOD: URL versioning
@RestController
@RequestMapping("/api/v1/users")
public class UserControllerV1 { }
@RestController
@RequestMapping("/api/v2/users")
public class UserControllerV2 { }
// ALTERNATIVE: Header versioning
@RestController
@RequestMapping("/api/users")
public class UserController {
@GetMapping(headers = "X-API-Version=1")
public List<UserV1Response> getUsersV1() { }
@GetMapping(headers = "X-API-Version=2")
public List<UserV2Response> getUsersV2() { }
}
Response Template
## Spring Boot Code Review Results
**Project**: [name]
**Spring Boot**: 3.2.x | **Java**: 17
**Profile**: [dev/prod]
### Dependency Injection
#### HIGH
| File | Line | Issue |
|------|------|-------|
| UserService.java | 12 | Field injection with @Autowired |
| OrderService.java | 8 | Multiple @Autowired fields |
### Transaction Management
| File | Line | Issue |
|------|------|-------|
| TransferService.java | 34 | Missing @Transactional on multi-save |
| ReportService.java | 56 | Read method without readOnly=true |
### JPA/Repository
| File | Line | Issue |
|------|------|-------|
| OrderService.java | 23 | N+1 query in loop |
| ProductService.java | 45 | Missing fetch join |
### Security
| File | Line | Issue |
|------|------|-------|
| AdminController.java | 12 | Missing @PreAuthorize |
| application.yml | 34 | Hardcoded API key |
### REST API
| File | Line | Issue |
|------|------|-------|
| UserController.java | 23 | Business logic in controller |
| OrderController.java | 45 | Missing @Valid on request body |
### Recommendations
1. [ ] Replace field injection with constructor injection
2. [ ] Add @Transactional to service methods
3. [ ] Fix N+1 with @EntityGraph or fetch join
4. [ ] Add method-level security annotations
5. [ ] Move business logic to service layer
### Positive Patterns
- Good use of @ConfigurationProperties
- Proper exception handling with @RestControllerAdvice
Best Practices
- Constructor Injection: Always prefer over field injection
- Service Layer: Keep controllers thin
- Transactions: At service layer, readOnly for queries
- Validation: @Valid on all request bodies
- Security: Method-level with @PreAuthorize
- Configuration: Externalize all config values
Integration
java-reviewer skill: Java idioms
kotlin-spring-reviewer skill: Kotlin Spring
orm-reviewer skill: JPA deep dive
security-scanner skill: Security audit
Notes
- Based on Spring Boot 3.x best practices
- Assumes Spring Security 6.x
- Works with both MVC and WebFlux
- Compatible with Java 17+ and Kotlin