Код-ревью Middle Spring MVC и REST обновлено
Код-ревью: контроллер пользователей
Вопрос
Найдите проблемы в REST-контроллере.
@RestController
@RequestMapping("/api/users")
public class UserController {
@Autowired
private UserRepository repo;
@GetMapping("/{id}")
public User get(@PathVariable Long id) {
return repo.findById(id).get();
}
@PostMapping
public User create(@RequestBody User user) {
return repo.save(user);
}
@GetMapping
public List<User> all() {
return repo.findAll();
}
@GetMapping("/{id}/avatar")
public byte[] avatar(@PathVariable Long id) {
try {
return repo.findById(id).get().getAvatar();
} catch (Exception e) {
return null;
}
}
}
Коротко
Контроллер должен принимать и отдавать DTO, валидировать вход и превращать «не найдено» в 404 через orElseThrow и ControllerAdvice. Списки только постранично.
Что должен найти ревьюер
- Безопасностьстрока 8Наружу отдаётся сущность User целиком: хеш пароля, служебные поля, ленивые связи (LazyInitializationException или рекурсия). Нужен DTO.
- Ошибкастрока 9
Optional.get()на отсутствующем id бросит NoSuchElementException и даст 500 вместо 404. - Безопасностьстрока 13Сущность из тела запроса: клиент может передать id (перезапись чужого пользователя), role или passwordHash (mass assignment). Нет @Valid.
- Производительностьстрока 19
findAll()без пагинации: на большой таблице ответ огромный и тяжёлый для БД и памяти. - Ошибкастрока 26Исключение проглочено, клиент получает пустой 200 вместо 404 или 500, ошибка не видна в логах.
- Стильстрока 5Контроллер ходит в репозиторий напрямую через field injection: нет слоя сервиса для бизнес-правил и транзакций.
Разбор
Контроллер должен принимать и отдавать DTO, валидировать вход и превращать «не найдено» в 404 через orElseThrow и ControllerAdvice. Списки только постранично. Глушить исключения нельзя: ошибка должна дойти до глобального обработчика и в лог.
Ответьте сами, а не прочитайте
В тренажёре вопрос попадёт в повторения и вернётся, когда начнёт забываться. Плюс билеты с зачётом: 20 вопросов, не больше 2 ошибок.