-
Notifications
You must be signed in to change notification settings - Fork 28
[4주차] 단계별 RateLimit 구현 & 테스트 작성 #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: scars97
Are you sure you want to change the base?
Conversation
- 영화, 상영관, 상영일정 조회
- Mapper 추가 : Entity -> Domain Model
- 아키텍처, 멀티모듈 설계, ERD, 시퀀스 다이어그램 추가
- 기존 api 모듈 -> infra 모듈로 통합 - infra 모듈 : 외부 시스템 연동 역할 수행
- AvailableMovieResult: 상영 중인 영화 정보 Dto - MovieResult: 전체 영화 정보 Dto - Theater, Schedule Dto 생성
- 계층별 Dto 분리
- 제거한 이유에 대한 문서 작성
- QueryDsl 설정 - FullText Index 사용을 위한 MysqlDialect 커스텀
- k6 스크립트 추가
- Caffeine 제거(사용X)
- post 요청 시 엔티티 생성되지 않는 문제로 인한 수정
- 회원, 상영일정 검증 로직 추가 - 테스트 작성
- 상태 검증, 변경 로직 작성 - 테스트 작성 - 의존성 정리
- 좌석 개수, 연속 좌석 검증 - 예약 생성 - 테스트 작성
- app push 기능 이벤트 처리
- TestContainer 설정
- infra 모듈에서 제어하도록 수정
- 동시성 테스트 작성
- 동시성 테스트 리팩토링 (feat.CompletableFuture)
- TestContainer : Redis 추가
- AOP 분산락 적용
- 문장 정리
- as-is : 인프라 모듈에서 좌석 조회 후 상태 변경 - to-be : 도메인 모델에서 상태 변경 후 전달
|
안녕하세요 성현님! 마지막 주차까지 열심히 해주셔서 감사합니다 :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
적절한 단위 테스트 or Small Test 좋습니다 👍
| class MovieServiceTest { | ||
|
|
||
| @Mock | ||
| private lateinit var movieRepository: MovieRepository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 Repository와 JpaRepository의 의존성을 분리한 아키텍처를 사용하고 계신 점이 인상 깊었습니다.
- 해당 아키텍처에서는 실제 JpaRepository가 아닌 도메인 레벨의 Repository를 대상으로 테스트를 작성할 수 있기 때문에, 굳이
@Mock을 사용하지 않아도 순수한 단위 테스트(Small Test) 작성이 가능합니다. - 현재는 Mocking 기반으로 테스트가 구성되어 있어 Medium 테스트에 가깝고, 상대적으로 실행 속도가 느립니다.
- 복잡도를 감수하고 의존성을 분리한 구조이니만큼, 이점을 충분히 활용하여 빠르고 신뢰성있는 테스트를 작성해보셔도 좋을 것 같습니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mock 이 없어도 순수한 단위 테스트가 가능한 이유가
테스트 전용 구현체를 생성해서 주입해주면 되기 때문일까요??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 맞습니다 :) Repository를 구현한 FakeRepository 구현체를 활용하는 방법이 있습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
예외 핸들링 시 @ResponseStatus(HttpStatus.TOO_MANY_REQUESTS)를 활용해서 명확하게 상태 코드를 매핑해주신 부분 좋습니다 👍
- 특히 API 클라이언트 입장에서 429 Too Many Requests를 리턴받으면, 요청 제한 상황임을 명확히 알 수 있어 UX 측면에서도 효과적입니다.
- 클라이언트에서 재시도 가능 시점을 명시하기 위해 Retry-After 헤더 또는 추가적인 정보제공도 고려할 수 있겠습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RateLimit 예외를 잘 정의해주셨네요 👍
- RateLimitExceededException 예외 메시지를 상수화 시키면 더 좋을것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bucket4j를 활용해서 RateLimiter를 잘 정의해주셨습니다 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ratelimit 애노테이션을 적절하게 잘 구현해주셨습니다 👍
- 애노테이션만 봐도 직관적으로 몇 회 제한인지, 제한 시간 단위, 호출 제한 카운트 등을 파악할 수 있어 좋습니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AOP 기반으로 RateLimiter 구현해주신 부분 좋네요 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lua Script 기반으로 원자적 RateLimit 처리를 잘 해주셨습니다 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bucket4j + Ratelimit 대한 검증, 단위 테스트 좋습니다 👍
| private val movieUseCase: MovieUseCase | ||
| ) { | ||
|
|
||
| @LimitRequestPerTime( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ratelimit 애노테이션을 적절하게 잘 적용해주셨습니다 👍
- 변수로 ttl, TimeUnit, limitCount 를 수정할 수 있도록 설계하여 유연성도 좋네요!
좋았던 점
아쉬웠던 점
리뷰 포인트
4주간 고생 많으셨습니다. 요구사항 분석부터 설계, 테스트에 이르기까지 전체적으로 완성도 높은 결과물이었습니다. 이번 프로젝트가 앞으로도 의미 있는 인사이트로 남길 바랍니다. 감사합니다 👏 |
[4주차] 단계별 RateLimit 구현 & 테스트 작성
📝작업 내용
🔒해결하려는 문제 혹은 고민이 되었던 부분을 남겨주세요.(문제 수 만큼 복사해서 사용할 것)
1. RateLimit 구현, 적용 위치 고민
구현
RateLimit을 구현하는 방법이 많아, interface를 두고
운영 상황에 따라 구현체만 갈아끼울 수 있도록 했습니다.
적용 위치
서비스 가장 앞단에서 요청을 제한해야 한다고 생각해 처음에는 interceptor 안에 적용했습니다.
interceptor 적용 시 문제
API마다 요청 제한 정책이 다를 수 있고, 검증되지 않은 값을 key로 설정한다면 안정성이 떨어질 것으로 예상됩니다. 이러한 내용을 interceptor 에서 모두 처리하기에는 한계가 있다고 생각됩니다.
해결
RateLimit 기능에 Aop를 적용하여 전역적으로 사용할 수 있도록 개선하였습니다.
관련 커밋 : b50dd01
2. RateLimit 기능 테스트 실패하는 문제
RateLimit 기능에 대해 여러 개의 테스트 코드를 작성하고 실행했을 때,
테스트가 실패하는 문제가 있었습니다.
원인
Redis 기반의 구현체를 사용하는 경우 테스트 데이터가 초기화 되지 않고 남아있어,
다른 테스트에 영향을 주고 있었습니다.
해결
각 테스트가 실행되기 전에 모든 key를 삭제하도록 RedisCleanUp 객체를 추가하여
독립된 테스트가 가능하도록 했습니다.
관련 커밋 : 38e18be
💬리뷰 요구사항(선택)
RateLimit을 적용할 때 @LimitRequestPerTime 이라는 어노테이션을 사용합니다.
현재 각 기능의 Controller에 적용했습니다만,
위에서 언급한 것과는 다르게 검증되지 않은 값을 key 로 사용하고 있습니다. (특히 예약 API)
-> 예약 API 의 RateLimit key 값 : '사용자Id-상영일정Id'
->여기서 말하는 검증이란 사용자Id, 상영일정Id 에 해당하는 데이터 존재 여부입니다.
검증 로직이 있는 UseCase에 선언해주려고 보니, Aop로 사용하고 있어서
검증 로직이 실행되기 전에 RateLimit 기능이 동작합니다.
그렇다면 검증 로직 이후에 실행되는 로직에 적용해줘야 하나 생각했습니다만,
RateLimit 과 관련이 없는 로직에 적용되는 것 같아 고민하고 있습니다.
key 값을 정하는 기준과
RateLimit 기능을 Aop로 사용하고 있다면 보통 어느 위치에 적용하는지 궁금합니다.
기타 사항 📌
[feat] Bucket4j 를 사용한 RateLimit 구현부터 입니다. -> b50dd01