Spring Boot Reviewer

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

// 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

// 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

// 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

// 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

// 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

// 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

// 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

// 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

// 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

1. Constructor Injection: Always prefer over field injection
2. Service Layer: Keep controllers thin
3. Transactions: At service layer, readOnly for queries
4. Validation: @Valid on all request bodies
5. Security: Method-level with @PreAuthorize
6. 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