[4주차] RateLimit 적용 및 테스트 코드 작성#93
Closed
sliverzero wants to merge 10 commits intohanghae-skillup:sliverzerofrom
Closed
[4주차] RateLimit 적용 및 테스트 코드 작성#93sliverzero wants to merge 10 commits intohanghae-skillup:sliverzerofrom
sliverzero wants to merge 10 commits intohanghae-skillup:sliverzerofrom
Conversation
youngxpepp
approved these changes
Feb 4, 2025
There was a problem hiding this comment.
안녕하세요 은영님! 이건홍 코치입니다.
좋았던 점
- 영화 조회와 티켓 예매에 대한 rate limit을 분리하여 인터셉터를 만들어 주셔서, 코드 읽기 수월했습니다.
- Rate limiter 를 직접 구현해주셨네요? 코드 리뷰 하면서 직접 구현한 분은 은영님이 혼자셔서 놀랬습니다.
아쉬운 점
- rate limit 구현 시 동시성을 조금만 더 고려하셨으면 어땠을까 싶습니다!
질의응답
- Q. 테스트 코드 작성에 도움이 되는 책이나 자료, 강의가 있다면 추천해 주실 수 있을까요?
A. 저는 아래 블로그에서 테스트에 대한 인사이트를 많이 얻었습니다. https://jojoldu.tistory.com/category/%ED%85%8C%EC%8A%A4%ED%8A%B8%EC%BD%94%EB%93%9C%20%26%20%EC%A0%95%EC%A0%81%EB%B6%84%EC%84%9D
| } | ||
|
|
||
| private void sendRateLimitResponse(HttpServletResponse response) throws IOException { | ||
| ObjectMapper objectMapper = new ObjectMapper(); |
There was a problem hiding this comment.
spring 에서 기본적으로 등록하는 ObjectMapper bean이 있을거에요.
https://docs.spring.io/spring-boot/how-to/spring-mvc.html#howto.spring-mvc.customize-jackson-objectmapper
Comment on lines
+80
to
+87
| private final List<LocalDateTime> requests = new ArrayList<>(); | ||
|
|
||
| // 요청 시간 추가 | ||
| public void addRequest(LocalDateTime requestTime) { | ||
| // 1분 이상 지난 요청들을 삭제 | ||
| requests.removeIf(time -> time.isBefore(requestTime.minusSeconds(TIME_MINUTE))); | ||
| requests.add(requestTime); | ||
| } |
There was a problem hiding this comment.
해당 로직도 동시에 수행될 가능성이 있어요.
동시성을 고려해보셨으면 좋겠습니다!
Comment on lines
+89
to
+93
| // 1분 내 요청 횟수 계산 | ||
| public long getRequestCountInLastMinute(LocalDateTime now) { | ||
| // 1분 전 요청들을 제외하고 남은 요청 수를 반환 | ||
| return requests.size(); | ||
| } |
There was a problem hiding this comment.
다른 시간으로 바뀔 수도 있지 않을까요?
변경 가능성을 염두해두고 파라미터를 추가하는 것도 괜찮을거 같네요!
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class ReservationRateLimitInterceptor implements HandlerInterceptor { |
There was a problem hiding this comment.
영화 조회와 티켓 예매를 잘 분리해주셨네요!
책임 분리를 해주셔서 코드를 집중해서 볼 수 있었어요👍
Comment on lines
+28
to
+30
| if (!(handler instanceof HandlerMethod handlerMethod)) { | ||
| return true; // 컨트롤러의 요청이 아닐 경우 통과 | ||
| } |
There was a problem hiding this comment.
컨트롤러 요청이 아닌 경우가 있을 수 있나요?
더군다나 해당 인터셉터는 url 기반으로 등록이 되어서 티켓 예매할 때만 수행되도록 되어 있는 듯 한데요!
| "redis.call('SET', key, 1, 'EX', ttl) " + | ||
| "return 1"; | ||
|
|
||
| public boolean isAllowed(Long userId, Long screeningId) { |
There was a problem hiding this comment.
Suggested change
| public boolean isAllowed(Long userId, Long screeningId) { | |
| public boolean isAllowed(long userId, long screeningId) { |
null일 가능성도 주지 맙시다!
Comment on lines
+21
to
+25
| registry.addInterceptor(rateLimitInterceptor) | ||
| .addPathPatterns("/screening/movies"); | ||
|
|
||
| registry.addInterceptor(reservationRateLimitInterceptor) | ||
| .addPathPatterns("/reservation/movie"); |
| RScript script = redissonClient.getScript(); | ||
|
|
||
| List<Object> keys = Collections.singletonList(key); | ||
| List<Object> args = Collections.singletonList(300); // TTL 5분 (300초) |
There was a problem hiding this comment.
ttl을 파라미터로 전달할 필요가 있나요?
lua script에 상수로 박아두거나 문자열 만들 때 넣어줘도 될거 같아요!
Author
|
꼼꼼하게 리뷰 해주셔서 정말 감사합니다! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
제목(title)
[4주차] RateLimit 적용 및 테스트 코드 작성
작업 내용
발생했던 문제와 해결 과정을 남겨 주세요.
이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.
리뷰 포인트
다른 분들처럼 계층별 멀티 모듈로 구조를 바꾸는 게 좋을까요?
기타 질문
아직 예약 API에 대한 테스트는 오류가 많이 발생해서 작성하지 못했습니다. 추가 리뷰 주차까지 마무리하도록 하겠습니다!
아직 많이 부족하겠지만 열심히 배우겠습니다!