Skip to content

Conversation

@CJ-1998
Copy link

@CJ-1998 CJ-1998 commented Apr 6, 2025

제목(title)

PR 제목은 변경 사항의 요약을 담아야 합니다.

[4주차] Rate Limit 구현 및 테스트 코드 작성

📝작업 내용

이번 PR에서 작업한 내용을 설명해주세요(이미지 첨부 가능)
주요 변경 사항을 기술해 주세요. 코드 구조, 핵심 로직 등에 대해 설명하면 좋습니다.

  • 단일 서버 환경에서의 RateLimit 구현
  • 분산 환경에서의 RateLimit 구현
  • RateLimit에 대한 테스트 코드 작성

🔒해결하려는 문제 혹은 고민이 되었던 부분을 남겨주세요.(문제 수 만큼 복사해서 사용할 것)

  • application 모듈에서 @SpringBootTest를 사용할 때 문제
    • presentation 모듈의 MovieController에서 IP 주소를 받기 위해 HttpRequestServlet을 사용하였습니다.
    • 그 후 application 모듈의 MovieQueryService를 테스트하기 위해 테스트 코드에 @SpringBootTest를 사용하였습니다.
    • 이때 application 모듈의 dependency에는 spring-boot-starter-web이 없었습니다.
    • 그래서 테스트를 실행하려고 하니 MovieQueryController bean을 생성할 수 없다고 오류가 발생하였습니다.
    • 해결 방법 및 고민은 아래 작성하겠습니다.

  • Rate Limiter annotation과 Cache annotation 동시에 붙은 메서드에서 문제
    • MovieQueryService의 getNowPlayingMovies 메서드에 Cache annotation이 존재하였습니다.
    • Rate Limiter를 AOP로 구현 후 동일한 메서드에 Rate Limiter annotation을 달았습니다.
    • 그 후 테스트를 진행하였는데 Rate Limiter가 동작하지 않았습니다.
    • 해결 방법 및 고민은 아래 작성하겠습니다.

🔑해당 문제를 어떻게 해결했나요? 그리고 왜 그렇게 생각했는지 이유를 남겨주세요.(자세하게 남겨주실 수록 좋습니다.)

  • application 모듈에서 @SpringBootTest를 사용할 때 문제 해결 및 고민
    • 그래서 우선 application 모듈의 dependency에 spring-boot-starter-web를 추가하였습니다.
    • 하지만 이렇게 되면 멀티 모듈로 나눈 의미가 사라지는 것 같다고 생각하였습니다.
    • 따라서 이렇게 테스트 코드를 작성할 때 dependency를 추가하는 것이 맞는지 궁금합니다.
    • 아니라면 다른 방법은 어떤 방법이 있는지 궁금합니다.

  • Rate Limiter annotation과 Cache annotation 동시에 붙은 메서드에서 문제 고민
    • 그래서 우선 Cache annotation을 주석 처리해 두었습니다.
    • cache annotation이 있으면 메서드로 진입하지 않아서 Rate Limiter가 처음에만 작동하고 그 이후에는 작동하지 않는 것인지 궁금합니다.
    • 그렇다면 해결 방법이 어떤 것인지 궁금합니다.
    • 제가 생각한 방법으로는 Cache annotation을 다른 곳으로 이동해 다른 값들을 caching 해야 하는지 궁금합니다.

💬리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요


기타 사항 📌

추가로 언급할 사항이 있다면 여기에 적어주세요.

  • Guava RateLimiter로 요구 사항을 어떻게 구현해야 할지 궁금합니다.
    • 현재는 Map을 사용하여 단일 서버에서의 RateLimiter를 구현하였습니다. (MapFetchRateLimiter, MapReserveRateLimiter)
    • 제가 이해한 바로는 Guava RateLimiter는 1초에 몇 번의 요청을 허용하는지 정하고 지정된 횟수의 요청만 실행한다고 이해했습니다.
    • 그렇다면 요구 사항인 IP별, 유저별 이런 요구 사항엔 각 IP, 유저에 따라 RateLimiter를 각각 생성해두고 관리해야 하는지 궁금합니다.

CJ-1998 added 30 commits April 4, 2025 22:49
명사형이던 메서드명 동사로 수정
메서드명에 불필요한 중복 제거
JpaRepository에 @repository annotation 제거
락 확득 실패 시 그냥 500 에러로 응답되고 있었음
이 예외를 409 conflict로 응답하도록 ApiControllerAdvice에 추가
ReservationService에서 수행하던 valid 로직 클래스로 추출
ReservationValidator라는 클래스에서 수행하도록 이동
DB ddl에 id에 unsigned 추가
인덱스 컬럼 순서 수정
data.sql에 불필요한 데이터 제거
ApiControllerAdvice에 예외 타입 수정하지 않아 오류 발생하던 점 수정
Infrastructure 모듈에 Guava dependency 추가
rate limiter에 필요한 메타데이터 위한 annotation 생성
Rate Limiter의 로직 있는 인터페이스 생성
Guava 사용해 RateLimiter 구현할 구현체 클래스 생성
로직 작성 중
조회 RateLimit 관련 annotation 수정
조회 요청에 대한 Rate Limit 로직 GuavaRateLimiter 구현체에 구현
조회 Rate Limit 위한 Aspect 구현
조회 RateLimiter 인터페이스, 구현체 Object 반환하도록 수정
IP 주소 쉽게 얻기 위한 config 생성
yml에 설정 추가
조회 api에 annotation에서 key로 IP 사용
따라서 조회 api controller에서 IP 받아오게 함
조회 api service의 메서드 파라미터로 IP 넘기도록 수정
GuavaRateLimiter에 예외 터지면 Too Many Requests 터지도록 추가
50번째 요청에서 예외 발생하는지 테스트 코드 구현
Controller에 HttpServlerRequest 사용
그러고 Application 모듈에서 @SpringBootTest 하니까 테스트 터짐
Application 모듈에 Spring Web dependency 추가

이게 맞는 해결 방법인지는 모르겠음
캐시 annotation과 Rate Limit annotation이 함께 있으니 문제 발생
Rate Limit annotation이 작동 안하는 것 같음
테스트에서 예외 발생 안함
우선 캐시 annotation 주석 처리해 놓음

어떻게 해결할지 추후 고민
조회 Rate Limit 정상 동작 테스트 추가
Rate Limiter의 Map clear 하는 메서드 추가
조회용 Rate Limiter 관련 fetchratelimiter 패키지로 이동
조회용 Rate Limiter 모두 Fetch 추가
예약 api에 적용할 Rate Limiter의 인터페이스, 구현체 구현
예약 Api 있는 ReservationService에 annotation 적용
annotation에 userId, screeningId String으로 수정
예약 api에서 Rate Limit 잘 동작하는지 테스트
Guava 붙은 것들 Map으로 수정
생각해보니 Guava는 사용하지 않고 Map으로 해결하고 있었음
Redis 이용한 예약 API Rate Limiter 구현
CJ-1998 added 8 commits April 6, 2025 23:12
테스트 코드 application 모듈에 TestApplication 만들어 동작하게 함
presentation 모듈의 CinemaApplication 사용하려고 하니 자꾸 오류남
application 모듈 테스트를 위한 데이터 넣을 data.sql 추가
예약 동시성 테스트 및 예약 Rate Limiter 테스트 동시 수행되게 수정
테스트 이전, 이후에 Rate Limiter의 Redis 비우도록 추가
jacoco 멀티 모듈 프로젝트에서 전체에 대한 리포트 나오도록 설정 추가
@ann-mj
Copy link

ann-mj commented Apr 8, 2025

@CJ-1998 님, 안녕하세요~ 리뷰 시작하겠습니다.

@ann-mj
Copy link

ann-mj commented Apr 8, 2025

application 모듈에서 https://github.com/SpringBootTest를 사용할 때 문제 해결 및 고민
그래서 우선 application 모듈의 dependency에 spring-boot-starter-web를 추가하였습니다.
하지만 이렇게 되면 멀티 모듈로 나눈 의미가 사라지는 것 같다고 생각하였습니다.
따라서 이렇게 테스트 코드를 작성할 때 dependency를 추가하는 것이 맞는지 궁금합니다.
아니라면 다른 방법은 어떤 방법이 있는지 궁금합니다.

  • 서비스 레이어만 테스트하신다면 application 에서 해도 좋은데요.
  • 통합 테스트는 presentation 모듈에서 수행하는 구조로 책임을 나누는 걸 추천드리겠습니다.
  • application 계층에서 테스트를 하셔야한다면 dependency 추가는 필수라고 생각됩니다.

@ann-mj
Copy link

ann-mj commented Apr 8, 2025

Rate Limiter annotation과 Cache annotation 동시에 붙은 메서드에서 문제 고민
그래서 우선 Cache annotation을 주석 처리해 두었습니다.
cache annotation이 있으면 메서드로 진입하지 않아서 Rate Limiter가 처음에만 작동하고 그 이후에는 작동하지 않는 것인지 궁금합니다.

  • rate limiter 를 service 계층까지 가는게 아니라, presentation 모듈에서 aop 또는 인터셉터 형식으로 하면 어떨까요?

@ann-mj
Copy link

ann-mj commented Apr 8, 2025

Guava RateLimiter로 요구 사항을 어떻게 구현해야 할지 궁금합니다.
아래와 같은 형태입니다.

import com.google.common.cache.CacheBuilder
import com.google.common.cache.CacheLoader
import com.google.common.cache.LoadingCache

guava에 포함된 해당 라이브러리들을 활용해서 데이터들을 관리할 수 있습니다.

아래 예시를 참고해주세요.

private val apiCallCounts: LoadingCache<String, AtomicInteger> = CacheBuilder.newBuilder()
    .expireAfterWrite(1, TimeUnit.MINUTES)
    .build(CacheLoader.from { _ -> AtomicInteger(0) })

조회의 경우 clientIp 를 받고
예약의 경우 clientIp, movieTimeId 를 받아서 구현하시면 되겠습니다.

메서드 1: checkApiCallLimit(clientIp: String)
메서드 2: checkBookingLimit(clientIp: String, movieTimeId: String)

int blockTime = limitRequestPerTime.blockTime();

if (diffMinutes <= blockTime) {
throw new ResponseStatusException(HttpStatus.TOO_MANY_REQUESTS, "Too many requests");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResponseStatusException 를 사용하여 TOO_MANY_REQUESTS 로 처리해주셨네요.
guava로 구현된 건 아니지만, 요구사항을 잘 구현해주셨습니다.


@Component("RedisRateLimiter")
@RequiredArgsConstructor
public class RedisFetchRateLimiter implements FetchRateLimiter {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 기능은 presentation 레이어에서 구현하시지 않고, infra 계층에서 구현하신 이유가 있을까요?

@Override
public Object tryApiCall(LimitRequestPerTime limitRequestPerTime, ProceedingJoinPoint joinPoint) throws Throwable {

String blockKey = "block_ip:" + limitRequestPerTime.key();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blockKey 로직의 경우는 재사용이 많이 되기에, 별도 메서드나 유틸로 빼도 좋을 것 같습니다.


@ResponseStatus(HttpStatus.CONFLICT)
@ExceptionHandler(IllegalStateException.class)
public ApiResponse<Object> IllegalStateException(IllegalStateException e) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요고 추가하신 이유가 궁금합니다.
특별히 CONFLICT 되는 부분이 존재하나요?

HttpServletRequest request) {
String clientIp = request.getRemoteAddr();
List<NowPlayMovieDto> nowPlayMovieDtos
= movieQueryService.getNowPlayingMovies(movieTitle, movieGenre, clientIp);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ip 가 service 계층에 영향을 줄 필요는 없을 것 같습니다.
따로 intercepter 라던지 Aop 로 관심사를 분리해주시면 더 좋을 것 같아요 👍

@ann-mj
Copy link

ann-mj commented Apr 8, 2025

@CJ-1998 님, 고생하셨습니다. 👍

좋았던 점

  • 커밋을 세세하게 잘 분리해주셨네요ㅎㅎ
  • guava, redisson 을 이용하시지는 않았지만, 요청이 많이 몰린 경우 적절한 예외처리를 추가해주셨습니다.

아쉬웠던 점

  • Lua script 를 활용한 구현이 안되어있었습니다. 나중에 작업하실 때 참고해보시면 좋을 것 같아요.
  • domain 계층에 대한 테스트 코드, jacoco 리포트가 빠져있네요 ㅠ 시간되실 때 구현해보시면 좋을 것 같습니다.
  • Rate Limit 을 적용하기 위해 IP 단위로 적용되어야 하는데, IP 라는 값이 현재 비즈니스 계층의 로직에 영향을 준 부분이 아쉬웠던 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants