7 Common Spring Backend Code Review Mistakes and Their Fixes

1. Use a custom thread pool for @Async

When @Async is used without specifying a thread pool, Spring falls back to SimpleAsyncTaskExecutor, which creates a new thread for each invocation and has no upper bound. Under high concurrency this quickly exhausts memory and leads to OOM.

@Service
public class MsgService {
    // Default async configuration – creates a new thread per call
    @Async
    public void sendAsyncMsg(String msg) {
        System.out.println("发送消息：" + msg + "，当前线程：" + Thread.currentThread().getName());
        try { Thread.sleep(1000); } catch (InterruptedException e) {}
    }
}

Define a bounded ThreadPoolTaskExecutor (e.g., asyncTianLuoExecutor) and reference it in @Async so threads are reused.

@Service
public class MsgService {
    @Async("asyncTianLuoExecutor")
    public void sendAsyncMsg(String msg) {
        System.out.println("发送消息：" + msg + "，当前线程：" + Thread.currentThread().getName());
        try { Thread.sleep(1000); } catch (InterruptedException e) {}
    }
}

2. Do not expose raw exception information to the frontend

Returning exception messages or stack traces reveals internal details (SQL, field names, API paths) and creates security risks.

@RestController
@RequestMapping("/user")
public class UserController {
    @GetMapping("/info/{id}")
    public Result getUserInfo(@PathVariable Long id) {
        try {
            User user = userService.getById(id);
            return Result.success(user);
        } catch (Exception e) {
            // Bad practice – leaks exception details
            return Result.fail(e.getMessage());
        }
    }
}

Use a global exception handler that logs the full stack trace internally and returns a generic error code/message.

@RestControllerAdvice
public class GlobalExceptionHandler {
    @ExceptionHandler(BusinessException.class)
    public Result handleBusinessException(BusinessException e) {
        log.error("业务异常：{}", e.getMessage());
        return Result.fail(e.getCode(), e.getMessage());
    }

    @ExceptionHandler(Exception.class)
    public Result handleException(Exception e) {
        log.error("系统异常", e);
        return Result.fail(500, "服务器繁忙，请稍后再试");
    }
}

// Simplified controller
@GetMapping("/info/{id}")
public Result getUserInfo(@PathVariable Long id) {
    User user = userService.getById(id);
    return Result.success(user);
}

3. Distributed‑lock value must be globally unique

Using a lock value that is only unique per JVM (e.g., thread name + timestamp) can cause different servers to generate identical values, allowing one thread to release another's lock.

// Bad example – not globally unique
private String getLockValue() {
    return Thread.currentThread().getName() + ":" + System.currentTimeMillis();
}

Generate a globally unique identifier such as a Snowflake ID or UUID.

// Good example
private String getLockValue() {
    return Thread.currentThread().getName() + ":" + SnowflakeIdWorker.generateId();
    // or UUID.randomUUID().toString()
}

4. Avoid deep pagination that triggers full‑table scans

When offset becomes large, MySQL must scan many rows before returning the requested page, dramatically degrading performance.

@GetMapping("/order/list")
public Result getOrderList(@RequestParam Integer pageNum, @RequestParam Integer pageSize) {
    // pageNum=10000, pageSize=10 → offset=99990 → full scan of ~100k rows
    PageHelper.startPage(pageNum, pageSize);
    List<Order> orderList = orderService.list();
    return Result.success(new PageInfo<>(orderList));
}

Use cursor‑based pagination (the last record ID) so the query can leverage the primary‑key index.

@GetMapping("/order/list")
public Result getOrderList(@RequestParam Long lastId, @RequestParam Integer pageSize) {
    List<Order> orderList = orderService.lambdaQuery()
        .gt(Order::getId, lastId)
        .orderByAsc(Order::getId)
        .last("limit " + pageSize)
        .list();
    return Result.success(orderList);
}

5. Acquire distributed lock before starting a transaction

Opening a transaction first and then acquiring the lock can lead to the lock being released before the transaction commits, allowing other threads to read stale data.

@Service
public class PayService {
    @Transactional // transaction starts first – wrong order
    public void pay(Long orderId) {
        String lockKey = "pay:lock:" + orderId;
        Boolean lock = redisTemplate.opsForValue().setIfAbsent(lockKey, "1", 5, TimeUnit.SECONDS);
        if (!lock) throw new BusinessException("请勿重复支付");
        try {
            // business logic
        } finally {
            redisTemplate.delete(lockKey); // lock released early
        }
        // transaction not yet committed – other threads may see dirty data
    }
}

Correct approach: acquire the lock first, then execute the transactional logic inside the locked section, and release the lock only after the transaction commits.

@Service
public class PayService {
    public void pay(Long orderId) {
        String lockKey = "pay:lock:" + orderId;
        String requestId = UUID.randomUUID().toString();
        Boolean lock = redisTemplate.opsForValue().setIfAbsent(lockKey, requestId, 10, TimeUnit.SECONDS);
        if (!lock) throw new BusinessException("请勿重复支付");
        try {
            this.doPay(orderId); // transactional business logic
        } finally {
            if (requestId.equals(redisTemplate.opsForValue().get(lockKey))) {
                redisTemplate.delete(lockKey);
            }
        }
    }

    @Transactional // runs inside the lock
    public void doPay(Long orderId) {
        // core payment logic
    }
}

6. Use batch DB operations instead of per‑row queries

Looping over a collection and inserting each record individually generates a separate SQL statement per iteration, causing unnecessary round‑trips.

// Bad – one INSERT per loop
public void batchSaveUser(List<UserDTO> dtoList) {
    for (UserDTO dto : dtoList) {
        User user = new User();
        BeanUtils.copyProperties(dto, user);
        userMapper.insert(user);
    }
}

Map DTOs to entities and call a single batch‑insert method (e.g., MyBatis‑Plus insertBatch) or use a custom foreach batch statement. Batch selects should also be used.

public void batchSaveUser(List<UserDTO> dtoList) {
    if (CollUtil.isEmpty(dtoList)) return;
    List<User> userList = dtoList.stream().map(dto -> {
        User user = new User();
        BeanUtils.copyProperties(dto, user);
        return user;
    }).collect(Collectors.toList());
    userMapper.insertBatch(userList); // single DB request
}

// Batch select example
List<Long> userIds = Arrays.asList(1L, 2L, 3L);
List<User> userList = userMapper.selectBatchIds(userIds); // one query

7. Protect all endpoints with authentication and authorization

Endpoints that lack login or permission checks allow anyone to read or modify data.

// Bad – no authentication
@GetMapping("/user/info")
public Result getUserInfo(Long userId) {
    User user = userService.getById(userId);
    return Result.success(user);
}

Apply security annotations (e.g., Spring Security or Sa‑Token) and enforce data‑level permissions.

@SaCheckLogin // must be logged in
@SaCheckPermission("user:info") // must have permission
@GetMapping("/user/info")
public Result getUserInfo(Long userId) {
    Long loginUserId = StpUtil.getLoginIdAsLong();
    if (!userId.equals(loginUserId)) {
        throw new BusinessException("无权限查看他人信息");
    }
    User user = userService.getById(userId);
    return Result.success(user);
}

8. Validate file uploads thoroughly

Accepting files without checking extensions, size, or content type enables attackers to upload malicious scripts.

// Bad – no validation
@PostMapping("/upload")
public Result upload(MultipartFile file) throws IOException {
    String fileName = file.getOriginalFilename();
    File uploadFile = new File("/upload/" + fileName);
    file.transferTo(uploadFile);
    return Result.success("上传成功");
}

Secure implementation includes a whitelist of allowed suffixes, size limits, content‑type verification, and random file naming to avoid path traversal.

private static final List<String> ALLOW_SUFFIX = Arrays.asList("jpg","png","jpeg","pdf","xlsx");

@PostMapping("/upload")
public Result upload(MultipartFile file) throws IOException {
    if (file.isEmpty()) throw new BusinessException("文件不能为空");
    String originalFilename = file.getOriginalFilename();
    String suffix = originalFilename.substring(originalFilename.lastIndexOf('.') + 1).toLowerCase();
    if (!ALLOW_SUFFIX.contains(suffix)) {
        throw new BusinessException("不支持的文件格式");
    }
    if (file.getSize() > 10 * 1024 * 1024) {
        throw new BusinessException("文件大小不能超过10MB");
    }
    String newFileName = UUID.randomUUID().toString() + "." + suffix;
    File uploadFile = new File("/upload/" + newFileName);
    file.transferTo(uploadFile);
    return Result.success("上传成功");
}