[5주차] rate limit 적용#103
Conversation
|
안녕하세요 정현님 :) 리부트 코스때 뵙고 오랜만이네요! |
|
|
||
| @Target(ElementType.METHOD) | ||
| @Retention(RetentionPolicy.RUNTIME) | ||
| public @interface RateLimitWith { |
There was a problem hiding this comment.
RateLimiter를 AOP를 활용해서 구현해주신점 좋습니다 :)
- default를 활용한 부분도 좋네요 👍
| import org.springframework.web.util.ContentCachingRequestWrapper; | ||
| import org.springframework.web.util.ContentCachingResponseWrapper; | ||
|
|
||
| public interface RateLimiter { |
There was a problem hiding this comment.
preCheck, postCheck 행위를 "선언 및 추상화"한 부분 인상깊습니다. interface로 추상화할때 주의점은
- 일반 애플리케이션 개발에서는 "진짜 범용 추상화"라는 것이 거의 없다는 점입니다(변화하기 때문, 라이브러리는 예외).
- 다만, RateLimiter를 정의하는 구현체가 해당 메서드의 선언을 정확하게 동작하도록 "정의"를 잘하고 있는지?
- 만약, 어떤 구현체는 postCheck가 필요없어 로깅만 하던지, 의미없는 리턴을 한다면 잘못된 추상화라고 할 수 있습니다 :)
There was a problem hiding this comment.
진짜 범용 추상화 라는 것이 없다는 점, 잘못된 추상화에 대해 알려주셔서 너무 감사합니다. 앞으로 프로그래밍을 하며 많이 참고할 것 같습니다. 해당 부분 좀 더 고민하여 더 좋은 코드로 만들어보겠습니다
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class ShowingSearchRateLimiter implements module.config.ratelimit.limiters.RateLimiter { |
There was a problem hiding this comment.
Lua Script를 활용해 API에 맞는 RateLimiter를 구현해주신 부분 좋습니다 👍
There was a problem hiding this comment.
- 다만 해당 부분이 "하드코딩"되어 있습니다. 아시겠지만, 버그 발생시 런타임에 오류가 발생 그리고 버그 파악도 어려운 부분이 있습니다.
- 시간이되신다면 커스텀하게 해당 부분을 책임지는 컴포넌트(클래스)를 만들어 보는 것도 고려할 수 있습니다.(eg QueryDSL)
There was a problem hiding this comment.
흥미로운 시도일 것 같습니다!! 시도해보겠습니다! 감사합니다.
| String username = ticketReservationRequest.getUsername(); | ||
|
|
||
| if(isBlocked(username,showingId)){ | ||
| throw new TwoManyReservationRequestException(); |
|
|
||
| public boolean isBlocked(String user, Long showingId) { | ||
| String key = keyFormatter(user, showingId); | ||
| return redisTemplate.opsForValue().get(key) != null; |
There was a problem hiding this comment.
만약 Redis 서버(보통 현업에서는 Caching 서버를 따로 배포해서 활용)가 중간에 문제로 다운된다면? 어떻게 될까요.
- 주석이라던지, 명확한 가정이 필요해보입니다. 아니라면 Redis 장애에 대비한 동작이 있으면 좋겠습니다.
There was a problem hiding this comment.
Redis 도입에 급급했던 것 같습니다. 장애에 대한 대비 해보겠습니다.
| String username = request.getUsername(); | ||
| List<TicketDTO> ticketList = request.getTicketList(); | ||
| return ResponseEntity.ok(ticketService.reservationWithFunctional(showingId, username, ticketList)); | ||
| return ResponseEntity.ok(ticketService.reservation(showingId, username, ticketList)); |
There was a problem hiding this comment.
자바 코딩 스타일 컨벤션은 메서드는 동사를 사용함으로 reserve가 더 맞아보입니다 :)
| @ExceptionHandler(TooManyRequestException.class) | ||
| @ResponseStatus(HttpStatus.NO_CONTENT) | ||
| protected ResponseEntity<String> tooManyRequestException(TooManyRequestException e){ | ||
| return ResponseEntity.status(HttpStatus.BAD_REQUEST) |
There was a problem hiding this comment.
Too many request에 알맞게 429 상태코드를 리턴하도록 해주시면 더 적합할듯 합니다 :)
| } | ||
|
|
||
| @Test | ||
| public void 리미터_51번째_조회() throws Exception { |
| @@ -148,15 +168,7 @@ public void validateAndReserve(Long showingId, User user, List<TicketDTO> ticket | |||
| int userAge = Period.between(user.getBirth(), LocalDate.now()).getYears(); | |||
| if (userAge < movie.getRating().getAge()) | |||
There was a problem hiding this comment.
비교연산자(if문)는 의미있는 메서드로 Extract 해주면 가독성이 좀 더 향상될것 같습니다. eg) isValidAge
There was a problem hiding this comment.
세세한 리뷰 감사드립니다!!ㅠㅠ 이런 한 말씀 한 말씀,, 너무 귀중하네요
|
|
||
| public void validateAndReserve(Long showingId, User user, List<TicketDTO> ticketDtoList) { | ||
| public void validateReservation(Long showingId, String username, List<TicketDTO> ticketDtoList) { | ||
| Optional<User> optionalUser = userRepository.findByUsername(username); |
There was a problem hiding this comment.
Optional을 활용한다면 findByUsername에서 바로 throw 해줘서 user 객체를 바로 사용할 수 있도록 간결하게 바꾸는 것도 좋아보입니다 :)
|
|
||
| // 최종연산 [ 결제 완료 처리 ] | ||
| for (Ticket ticket : ticketList) { | ||
| ticket.setTicketStatus(TicketStatus.RESERVED); |
There was a problem hiding this comment.
간단한 로직이지만 객체 내부에서 상태를 바꾸도록 하고, 좀더 의미있는 메서드 이름으로 좀 더 OOP에 맞게 개선할 수 있을것같습니다 :)
좋았던 점
아쉬웠던 점
리뷰 포인트RateLimit을 이런식으로 구현해도 되는 건지, 너무 생각을 많이 해서 조금 오바스럽게 구현한 건 아닌지 여쭤보고 싶습니다
이번 코스 고생많으셨고, 새해복많이받으세요!! |
2 participants
[5주차] rate limit 적용
작업 내용
발생했던 문제와 해결 과정을 남겨 주세요.
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
기타 질문
이런 발상으로 코드를 작성했고, api별 RateLimit 클래스를 작성하고, 어노테이션만 추가하면 각 api에 rate limit을 적용할 수 있도록 설계를 해보았는데 이게 RateLimit을 이런식으로 구현해도 되는 건지, 너무 생각을 많이 해서 조금 오바스럽게 구현한 건 아닌지 여쭤보고 싶습니다.