Skip to content

[4주차] RateLimit 적용 및 공통 응답 작업#90

Merged
BAEKJungHo merged 3 commits intohanghae-skillup:kimbro97from
kimbro97:kimbro97
Feb 6, 2025
Merged

[4주차] RateLimit 적용 및 공통 응답 작업#90
BAEKJungHo merged 3 commits intohanghae-skillup:kimbro97from
kimbro97:kimbro97

Conversation

@kimbro97
Copy link

@kimbro97 kimbro97 commented Feb 3, 2025

[4주차] RateLimit 적용 및 공통 응답 작업


작업 내용

  • 공통 응답 객체를 만들어 응답할 수 있도록 작업했습니다.
  • Guava RateLimit을 활용하여 조회 및 예약 API에 적용했습니다.
    • RateLimit 인터페이스를 설계하여 차후에 Redis 도입하더라고 클라이언트 코드는 변경없이 작업할 수 있도록 개발했습니다.

발생했던 문제와 해결 과정을 남겨 주세요.

이번 주차에서 고민되었던 지점이나, 어려웠던 점을 알려 주세요.

  • 현재 프로젝트는 레이어드 아키텍처 기반의 멀티 모듈 구조로, api, application, domain 모듈로 구성되어 있습니다. ApiResponse는 api 모듈에서 주로 사용되고, exception 클래스는 application 모듈에서 주로 사용되지만, 이 둘이 공통적으로 사용될 가능성이 있어 따로 지원 모듈을 만들어야 할지 고민 중인데 어떻게하는게 좋을까요?

리뷰 포인트

  • 영화 조회 API RateLimit을 적절하게 구현했는지 봐주시면 감사하겠습니다.
  • RateLimit 기능을 개발할때 인터페이스를 만들어서 객체간의 결합도를 낮췄습니다. 적절하게 인터페이스를 사용했는지 궁금합니다!!

기타 질문

@BAEKJungHo
Copy link
Contributor

@kimbro97 안녕하세요 형재님 ! 4주간 고생 많으셨습니다 ! 리뷰 진행하겠습니다 ~

}

@Around("@annotation(reservationRateLimited)")
public Object around(ProceedingJoinPoint joinPoint, ReservationRateLimited reservationRateLimited) throws Throwable {
Copy link
Contributor

Choose a reason for hiding this comment

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

RateLimit 어노테이션과 AOP 를 잘 활용해주셨네요 👍🏿

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface ReservationRateLimited {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

val ttl: Int = 1, // 호출 제한 시간
val count: Int, // 분당 호출 제한 카운트
val timeUnit: TimeUnit = TimeUnit.HOURS

이런 옵션들도 설정 가능하게끔 하는 것을 고려해보는 것도 좋을 것 같아요. Controller 에서 API 의 제약조건에 대해서 더 명확하게 알 수 있을 것 같습니다.

package com.example.config.ratelimit;

public interface RateLimit {

Copy link
Contributor

Choose a reason for hiding this comment

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

RateLimit 기능을 개발할때 인터페이스를 만들어서 객체간의 결합도를 낮췄습니다. 적절하게 인터페이스를 사용했는지 궁금합니다!!

인터페이스를 사용하여 다양한 구현체를 만드는 방식은 좋은 접근 입니다. 클라이언트 측은 인터페이스에 대한 명세만 알고있으면되고, 세부 구현체가 어떤 것인지는 몰라도 되기 때문입니다.

다만, 어떤 구현체를 선택해서 사용할지에 대한 로직도 추가적으로 필요할 것 같습니다.
Guava, Redis 의 RateLimit 활용에 사용을 해주셨는데, 시나리오 특성상 2개의 구현체가 존재해야하므로 잘 써주신 것 같아요.

다만 실무적으로 본다면 분산 애플리케이션 환경에서는 Redis 만 사용할 가능성이 높기 때문에 인터페이스 없이 바로 구현체를 활용할 수 있을 것 같습니다.

아이디어랑 접근 방식은 좋습니다 💯

return movieService.getMovies(request.toServiceRequest());
@MovieSearchRateLimited
public ApiResponse<List<MovieResponse>> getMovies(MovieSearchRequest request) {
return ApiResponse.ok("영화 목록 조회",movieService.getMovies(request.toServiceRequest()));
Copy link
Contributor

Choose a reason for hiding this comment

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

하드코딩은 제거하면 좋을 것 같습니다 !

@Getter
public enum BusinessError {
// 유저관련 exception 3000
USER_LOGIN_ERROR(HttpStatus.BAD_REQUEST, 3000, "로그인이 필요합니다."),
Copy link
Contributor

Choose a reason for hiding this comment

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

실무에서 자주 사용되는 방식으로 잘 구성해주셨네요 👍🏿


@Test
@DisplayName("초당 요청 제한 테스트: 연속 요청 시 per-second RateLimiter가 제한하는지 확인")
void test1() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@DisplayName 을 사용했다고 해서 test 이름을 대충 지어도 되는건 아닙니다 !
오히려 test 이름을 함수로 사용하고 @DisplayName 를 없애는 것을 고려해보는 것도 좋을 것 같습니다.

memberRepository.save(member);
assertThatThrownBy(() -> { reservationService.reserve(new ReservationServiceRequest(2L, 1L, List.of(1L, 2L))); })
.isInstanceOf(IllegalArgumentException.class)
.isInstanceOf(BusinessException.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

given 에 해당되는 사전 준비 과정(데이터 생성 & save) 가 5군데 정도 중복이 있는 것 같은데요, 별도 함수로 빼서 관리해도 좋을 것 같습니다.

e.g createMovie("히트맨 2", "http://example.com/hitmen.jpg", 120, Genre.ACTION, Rating.ALL, LocalDate.of(2025, 1, 22));


@Getter
public class ApiResponse<T> {

Copy link
Contributor

Choose a reason for hiding this comment

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

ApiResponse 도 잘 만들어주셨습니다 👍🏿

@BAEKJungHo
Copy link
Contributor

@kimbro97

좋았던 점

  • RateLimit 시 AOP 를 활용한 부분이 좋았습니다.
  • Service 통합 테스트를 잘 작성 해주셨습니다.
  • RateLimit 활용을 잘 해주셨습니다 !

아쉬웠던 점

  • Seats 도메인 모델에, 도메인 로직을 잘 응집 시켜주셨는데, 이러한 부분을 단위 테스트로 적용한 부분이 없는게 아쉽습니다.
    • 통합 테스트는 커버리지를 많이 높이는데 훌륭하며, 복잡한 시나리오를 커버하기 위해서 작성하는 경우가 많습니다.
    • 실무에서는 단위 테스트 비중이 가장 높아야 합니다.

리뷰 포인트 및 추가 질문에 대한 답변

현재 프로젝트는 레이어드 아키텍처 기반의 멀티 모듈 구조로, api, application, domain 모듈로 구성되어 있습니다. ApiResponse는 api 모듈에서 주로 사용되고, exception 클래스는 application 모듈에서 주로 사용되지만, 이 둘이 공통적으로 사용될 가능성이 있어 따로 지원 모듈을 만들어야 할지 고민 중인데 어떻게하는게 좋을까요?

실무에서도 따로 공통(지원) 모듈을 만들어서 exception or ApiResponse 를 정의해두고 사용하곤 합니다.
각 Layer 에 맞는 예외들을 각각의 Layer 에서 담당하게 할지, 혹은 공통(지원) 모듈을 두고 전역적으로 사용할지는
애플리케이션의 규모에 맞게 trade-off 해야할 것 같습니다. 이 마저도 사람마다 관점이 다르기 때문에 어떤게 정답이다라고 말하긴 힘들 것 같습니다.

@kimbro97
Copy link
Author

kimbro97 commented Feb 6, 2025

@BAEKJungHo

안녕하세요, 코치님!
자세한 피드백 감사합니다.

머지해 주시면 이번 주에 부족한 부분을 보완하여 PR 다시 드리겠습니다!

@BAEKJungHo BAEKJungHo merged commit fb25421 into hanghae-skillup:kimbro97 Feb 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants