김종범 튜터님

[ 👍 잘한 점 ] 어떤 기술을 왜 사용했는지에 대한 설명 분업이 잘 이루어 졌고, 협조적인 팀 분위기.

[ 🔎 개선 제안 ] 기술적 피드백에 함께 작성 [ 💡 기술적 피드백 ]

1. 예외 처리 일관성이 아쉽습니다. """ Product product = productRepository.findById(productId).orElseThrow( () -> new IllegalStateException("없는 상품입니다.") ); """ 이런 부분이 있는데, AdminService, CustomerService에서는 CommonException을 사용하지만, ProductService와 ReviewService에서는 IllegalStateException을 그대로 사용하고 있습니다. GlobalExceptionHandler가 Exception.class를 잡아서 모두 500 INTERNAL_SERVER_ERROR로 반환합니다. "없는 상품입니다."라는 메시지가 500으로 클라이언트에 반환되는 문제가 생깁니다.

""" Admin admin = adminRepository.findById(adminId).orElseThrow( () -> new CommonException(CommonError.ADMIN_NOT_FOUND)

); """ 이렇게 수정 해 주는 게 더 좋을 것 같습니다 .

1. AdminService.login() 에서 Rejected 상태에 관하여, """ switch (admin.getStatus()) { case PENDING -> throw new CommonException(CommonError.PENDING_ACCOUNT); case SUSPENDED -> throw new CommonException(CommonError.SUSPENDED_ACCOUNT); case INACTIVATE -> throw new CommonException(CommonError.INACTIVE_ACCOUNT); default -> throw new CommonException(CommonError.LOGIN_FAILED);

   } """ 이렇게 처리해 주셨는데, Rejected 면 단순히 로그인 실패보다는 "가입이 거부 되었습니다" 와 같은 처리를 명확하게 해주었으면 더 좋았을 것 같습니다.

2. 리뷰 상세 조회 API 에서 파라미터가 좀 아쉬운데요, """ @GetMapping("/reviews/{id}") public ResponseEntity<...> getDetailReview( @PathVariable Long id, // reviewId @RequestParam(required = true) Long productId, // -> 이거랑 @RequestParam(required = true) Long customerId // -> 요거 ) """ Review Entity가 이미 Product, Customer와 연관 관계를 가지고 있기 때문에, reviewId 하나 만으로 product와 customer 정보를 꺼낼 수 있는 상태입니다. 굳이 마지막의 두 파라미터가 필요한지, 필요하다 하더라도 기본 값인 required=true 를 중복으로 써줬어야 하는지 의문이 남습니다.

3. admin 에 있는 method 중에, adminStatus(), 이 친구는 changeStatus 라는 이름이 좀 더 명확했을 것 같습니다.

4. DashboardService의 static 날짜 필드, """ @Service public class DashboardService { final static LocalDate today = LocalDate.now();

   final static LocalDateTime start = today.atStartOfDay(); final static LocalDateTime end = today.atTime(LocalTime.MAX);

""" 이러한 부분이 있는데, static final로 선언하면 서버 최초 실행 시점의 날짜로 고정됩니다. 서버를 재시작하지 않는 한 "오늘 주문 수", "오늘 매출"이 항상 배포 당일로 고정됩니다. 매 요청 시점에 LocalDate.now()를 할당 할 수 있게 수정해야 합니다.

1. 주문 생성 시 product 의 상태 값을 확인 하는 부분이 없습니다.

""" if (product.getStock() < request.quantity()) { throw new CommonException(CommonError.PRODUCT_OUT_OF_STOCK); } product.decreaseStock(request.quantity()); """ 이렇게 코드를 작성해주셨는데, 해당 부분으로 재고가 없는 경우에는 주문이 불가능 하지만 단종(Discontinued) 일 때는 재고만 있으면 주문이 가능하도록 설계가 되어 있습니다. 재고 확인 전에 상품 상태 확인 -> 재고 확인 순서로 이어져야 할 것 같습니다.

7 Security Config 에서 조금 의아한 부분이 있는데, """ .requestMatchers("/api/signup", "/api/login", "/api/admins/me/", "/error").permitAll() .requestMatchers("/api/admins/", "/api/admins/{adminId}/**").hasAuthority("SUPER_ADMIN") """ 이 부분 이였습니다. /api/admins/me/**를 permitAll로 먼저 선언했는데, /api/admins/**에 SUPER_ADMIN 권한이 붙어 있습니다. Spring Security는 선언 순서대로 매칭하므로 me 경로는 SUPER_ADMIN 체크 없이 통과됩니다. 의도한 부분 이였을지, 실수 일지 판단 이 안 서서 체크했습니다.

1. Jwt Filter 에서의 Runtime Exception 처리를 보면,

""" try { userDetails = userDetailsService.loadUserByUsername(info.getSubject()); } catch (UsernameNotFoundException e) { throw new RuntimeException(e);

} """

이런 부분이 있는데, 사용자를 못 찾은 상황인데 500 에러를 반환 합니다. Common Exception 으로 변경해서 유저 없음을 반환해 주는 게 더 좋을 것 같습니다.

1. 오타가 하나도 없어서 감동할 뻔 하다가 아쉽게 하나 발견했는데, """ ProductWidgetDto getProdectWidget(); """ Product 를 Prodect 라고 쓰셨습니다..........