[4주차] RateLimit 적용 및 테스트코드 추가 by rmsxo200 · Pull Request #89 · hanghae-skillup/redis_1st

rmsxo200 · 2025-02-02T16:58:53Z

작업 내용

조회 및 예매API에 RateLimit 적용
- 분당 50회 초과 조회 막기
- 동일 시간표 영화 5분에 1회 초과 예매 막기
예매 관련 테스트코드 작성
- adapter계층(Presentation) 예매 통합테스트 추가
- application계층 예매 단위테스트 추가
jacoco 테스트 (jacoco_report_aggregation 플러그인 추가)

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

문제상황 : 통합 테스트시 redis데이터 운영에 쌓이는 문제가 발생 하였습니다.
해결방법 : Testcontainers를 사용하여 테스트환경에서 독립적으로 동작하도록 하였습니다.

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

테스트 환경을 만드는것이 어려웠습니다.
테스트만을 위한 설정이나 Mock객체나 예상동작을 정의하는 등의 작업이 익숙치 않아 오래걸렸습니다.
멀티모듈이기에 jacoco report가 각각 모듈에 생성되는 문제가 있었습니다.
jacoco-report-aggregation를 사용하면 한번에 통합하여 만들수 있다고 하는데 참고 문서를 보고 작성하여도 뜻대로 되지 않아 해당부문은 제외시켰습니다 ㅠㅠ

리뷰 포인트

테스트 코드를 작성한 방식이 올바른지 알고 싶습니다!

기타 질문

성공 할 수 밖에 없게 stubbing을 하는데 이렇게 진행하는게 실제 동작을 테스트하는데 의미가 있을지 궁금합니다 ㄷㄷ

jacoco 테스트시 dto까지 테스트를 요구하는것 같습니다.
메서드가 있는 모든 클래스에 대해서 junit 테스트시 어디서든 해당 메서드가 호출만 되어야 하는지 궁금합니다!
- 그렇다고 한다면 getter, setter까지도 포함되는지 궁금합니다!

jacoco_report_aggregation 플러그인을 추가해 모든 모듈의 jacoco 보고서를 하나로 합쳤습니다.
결과를 보니 개별 테스트랑 좀 다른것 같은데 제가 뭘 잘못해서 그런건지, 아니면 두개의 방법이 다른건지 궁금합니다!

테스트 코드는 예매 관련 컨트롤러와 서비스 로직에 대해서만 작성 하였습니다.
조회 로직, 도메인 계층 등 테스트 코드가 누락된 부분이 많지만 리뷰 부탁 드립니다 !

pyuseon · 2025-02-03T11:20:51Z

안녕하세요! 이번 주차도 과제 하시느라 고생 많으셨습니다!
코드 리뷰 진행하도록 할게요!

pyuseon · 2025-02-03T11:49:17Z

cinema-infrastructure/src/main/resources/lua/searchRateLimit.lua

+local key = KEYS[1]  --[[ 요청 카운트 키 ]]
+local blockKey = KEYS[2]  --[[ 차단된 IP 키 ]]
+local maxRequests = tonumber(ARGV[1])  --[[ 최대 요청 횟수 (초) ]]
+local expireTime = tonumber(ARGV[2])  --[[ 요청 카운트 만료 시간 (초) ]]
+local blockTime = tonumber(ARGV[3])  --[[ 차단 시간 (초) ]]
+
+--[[ 차단 여부 확인 ]]
+if redis.call("exists", blockKey) == 1 then return 0 end
+
+--[[ 요청 카운트 증가 및 만료 시간 설정 ]]
+local count = redis.call("incr", key)
+if count == 1 then redis.call("expire", key, expireTime) end
+
+--[[ 요청 제한 초과 시 차단 ]]
+if count > maxRequests then
+    redis.call("setex", blockKey, blockTime, "BLOCKED")
+    return 0
+end
+
+return 1


요구사항을 정확하게 반영해서 Lua Script를 구현해 주셨군요!
'1분 내 50회 이상 요청 시 1시간 동안 해당 IP를 차단'하는 부분도 요구사항에 맞춰 잘 구현해 주셨어요!

pyuseon · 2025-02-03T12:08:28Z

cinema-application/src/main/java/com/hanghae/application/service/MovieScheduleServiceImpl.java

+        if (!redisRateLimitPort.isAllowed(ip)) {
+            return ApiResponse.of("너무 많은 요청으로 조회가 차단되었습니다. ", HttpStatusCode.TOO_MANY_REQUESTS);
+        }


조회 제한시 응답을 적절하게 구현하셨네요! isAllowed(ip)는 RateLimiter를 통해 조회를 제한하는 부분이므로 , 429 TOO_MANY_REQUESTS 응답을 반환하는 방식이 적절합니다!
다만 현재는 CustomError코드는 따로 설정하지 않으신 것으로 보이는데요, 클라이언트와 통신시 에러를 특정하기 위해서는 CustomCode를 구현하시는 것도 좋은 방법입니다.

혹시 CustomError코드를 사용하게 된다면 http헤더의 상태코드는 그대루 두고 내부 메시지를 통해서 해당 코드값을 표현하는 식으루 CustomCode를 구현 하면 될까요!?!

@rmsxo200
네 맞습니다. 헤더의 상태코드는 그대로 두고 내부 메세지 혹은 지금 사용하고 계신 ApiResponse에 필드를 하나 더 추가 해 구현하시는 것이 좋아요!

넵 수정하도록 하겠습니다!
감사합니다!

pyuseon · 2025-02-03T12:21:43Z

...plication/src/test/java/com/hanghae/application/service/MovieReservationServiceImplTest.java

Stubbing에 대해 질문해 주셨는데요. 충분히 궁금하실 수 있는 부분이라고 생각합니다.
이미 정해진 결과를 검증하는 느낌이 들 수도 있으니까요.

몇가지 장점이 있는데 제가 느끼는 것을 적어보면

컨트롤러 테스트에서 비즈니스 로직을 실행하지 않고, 요청과 응답 형식 검증을 위해 사용할 수 있어요. 특히 저희 회사 에서는 RestDocs을 활용할때 이 장점이 드러나는 것 같아요 .

데이터베이스를 사용하지 않고 서비스 레이어에서 일부분의 작동만 확인하기 위해 간편히 사용하기 좋습니다.

외부 API를 활용하는 로직에도 직접 호출하지 않고 사용하는 장점이 있죠

Stubbing이 '끼워 맞추는 것 같다'는 느낌이 들 수도 있지만, 독립적인 테스트를 도와주는 효과적인 방법중 하나라고 생각합니다!

많은 장점이 있는 것 같습니다!
앞으로 테스트코드를 열심히 작성하도록 하겠습니다!

pyuseon · 2025-02-03T12:31:41Z

...a-application/src/main/java/com/hanghae/application/service/MovieReservationServiceImpl.java

+                // 예매 성공 후 Redis 제한 설정
+                redisRateLimitPort.setReservationLimit(scheduleId, memberId);
+


5분에 한개씩 예매를 제한 하는 것도 요구사항에 맞춰 구현해 주셨네요! 성공후에 제한 설정 + 예매 전 확인 까지 잘 구현해 주셨어요!

pyuseon · 2025-02-03T12:40:07Z

cinema-adapter/src/test/java/com/hanghae/adapter/web/ReservationControllerTest.java

+@AutoConfigureMockMvc
+@Testcontainers // TestContainers 사용
+@Transactional
+@Sql(scripts = "/sql/reservationTest.sql") // SQL 파일 실행


통합테스트 구현을 깔끔하게 해주셨네요. sql로 실제 데이터를 넣어주셔서 실 환경과 비슷하게 구현해주셨네요! 훌륭합니다!

감사합니당

pyuseon · 2025-02-03T12:41:59Z

cinema-adapter/src/test/java/com/hanghae/adapter/web/ReservationControllerTest.java

+        mockMvc.perform(post("/api/v1/movie-reservation")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(objectMapper.writeValueAsString(requestDto)))
+                .andExpect(status().isCreated());
+
+        // 5분 내 동일한 요청을 보내면 예외 발생 확인
+        mockMvc.perform(post("/api/v1/movie-reservation")
+                        .contentType(MediaType.APPLICATION_JSON)
+                        .content(objectMapper.writeValueAsString(requestDto)))
+                .andExpect(status().isTooManyRequests())
+                .andExpect(jsonPath("$.message").value("동일 시간대 영화 예매는 5분 후 가능합니다."));


RateLimiter가 걸렸을때 Too Many Request가 반환되는지 확인하는 코드도 테스트로 작성해 주셨네요! 👍
다만, 현재는 여러사용자가 동시에 요청했을때 RateLimiter가 작동하는 지는 검증하고 있지 않은것 같습니다.
통합테스트에 이부분도 같이 고려해주시면 더 완성도 높은 코드가 될것 같아요!

넵 해당 부분 추가하도록 하겠습니다!

pyuseon · 2025-02-03T12:48:43Z

cinema-adapter/build.gradle.kts

    id("java")
    id("org.springframework.boot") version "3.4.1"
    id("io.spring.dependency-management") version "1.1.7"
+    id("java-test-fixtures") // 테스트 픽스처 활성화


텍스트 픽스처를 활용해주셨네요! 놓치기 쉬운 부분인데 세심하게 구현해 주셨어요!

pyuseon · 2025-02-03T13:02:56Z

cinema-adapter/src/test/java/com/hanghae/adapter/web/ReservationControllerTest.java

+    @DisplayName("[통합] 회원당 예매 좌석 개수 초과시 예외 발생 테스트")
+    void saveMovieReservationSeatLimitExceeded() throws Exception {
+        //테스트 sql에서 미리 넣어둔 예매데이터 + 추가로 5좌석 예매
+        MovieReservationRequestDto invalidRequest = TestDataFactory.createMovieReservationRequestDto(5);


직접 객체를 생성하는 대신 TestDataFactory를 활용하셨네요. 코드에서 유지보수성을 고민하신게 느껴집니다! 👍

pyuseon · 2025-02-03T13:37:56Z

좋았던 점

RateLimiter 설정을 깔끔하게 적용하셨고, LuaScript를 활용하여 요구사항에 맞춰 적절한 제한을 설정하신 점이 인상적이었습니다.
Presentation, Domain, Infra 세 레이어에 대한 테스트를 모두 작성해 주셔서, 서비스의 다양한 부분이 꼼꼼하게 검증되고 있었습니다.
테스트 코드에서 Test Fixture를 적극적으로 활용하여 중복을 줄인 점도 매우 좋았습니다. 덕분에 코드의 유지보수성이 향상되었고, 테스트 코드가 더욱 깔끔해졌어요!

아쉬운 점

통합 테스트에서 동시 요청을 다루는 부분이 추가되면 더욱 견고한 테스트 코드가 될 것 같습니다. 현재 RateLimiter가 정상적으로 동작하는 것은 확인되지만, 여러 사용자가 동시에 요청을 보낼 때도 일관되게 제한이 적용되는지 검증하면 더욱 강력한 테스트가 될 것 같아요.

리뷰포인트

전반적으로 테스트 코드를 꼼꼼하게 작성해 주셨는데요. 영화 예약 관련 통합 테스트 코드는 확인했지만, 영화 조회와 관련된 통합 테스트 코드는 확인할 수 없었습니다. 조회 관련된 테스트 코드도 작성하시면 좋을 것 같습니다. 이 두 API(예약 & 조회)는 핵심 기능이므로, 통합 테스트를 통해 전반적인 흐름을 검증하는 것이 중요합니다.

추가 질문

jacoco 테스트시 dto까지 테스트를 요구하는것 같습니다.
메서드가 있는 모든 클래스에 대해서 junit 테스트시 어디서든 해당 메서드가 호출만 되어야 하는지 궁금합니다!
그렇다고 한다면 getter, setter까지도 포함되는지 궁금합니다!

DTO의 모든 메서드를 테스트할 필요는 없습니다. DTO를 제외하려면 JaCoCo exclude 옵션을 활용할 수 있습니다. Getter, Setter가 Lombok 등의 어노테이션을 통해 자동 생성되었다면 별도로 테스트할 필요는 없습니다.JaCoCo의 테스트 커버리지와 관련해서는 아래 공식 문서를 참고하시면 도움이 될거에요!
JaCoCo Counters 공식 문서

jacoco_report_aggregation 플러그인을 추가하여 모든 모듈의 Jacoco 보고서를 하나로 합쳤습니다.
결과를 보니 개별 테스트와 통합 리포트가 다르게 나옵니다. 제가 뭘 잘못 설정한 건가요? 아니면 두 방법이 다른 건가요?

jacoco_report_aggregation을 활용하면 개별 테스트와 통합 리포트의 커버리지가 달라질 수 있습니다! 여러 모듈에서 동일한 코드가 호출된 경우 통합 리포트에서 커버리지가 높아질 수도 있고, 반대로 개별 모듈에서 Mock을 사용한 경우 테스트되지 않은 코드로 간주되어 커버리지가 낮아질 수도 있습니다.

4주 동안 정말 고생 많으셨습니다! 🙌
마지막까지 시나리오의 요구사항을 최대한 구현하려는 노력이 보였고, 그 과정에서 많은 고민을 하신것이 느껴졌습니다.
이제 남은 과정에서 피드백에 따라 테스트 코드를 발전시키시면, 더욱 완성도 높은 프로젝트가 될 것 같습니다!
끝까지 좋은 결과 얻으시길 응원합니다!

rmsxo200 · 2025-02-04T05:54:22Z

고생하셨습니다!
꼼꼼한 코드리뷰 감사합니다!

kim keun tae added 8 commits January 31, 2025 23:02

API 응답 메시지 수정

418e3ec

예매API 컨트롤러 분리 및 패키지 구조 변경

38c706c

영화 조회 및 예매로직 RateLimit 적용

0535785

예매 시도 및 완료 후 RateLimit 로직 분리

e71d8df

dto 패키지 위치 변경

43b5a39

영화 예매 비즈니스 로직 단위 테스트 코드 추가 및 통합 테스트 코드 추가

4770f4a

통합테스트시 redis Testcontainers 로 실행되게 수정

9a632cb

jacoco 테스트 설정 추가 및 테스트 보고서 추가

56fc2e3

github-actions bot added the 4주차 label Feb 2, 2025

jacoco_report_aggregation 플러그인 추가, jacoco 보고서 통합

f8dc766

pyuseon reviewed Feb 3, 2025

View reviewed changes

pyuseon merged commit 946b602 into hanghae-skillup:rmsxo200 Feb 3, 2025

		// 예매 성공 후 Redis 제한 설정
		redisRateLimitPort.setReservationLimit(scheduleId, memberId);

Conversation

rmsxo200 commented Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

작업 내용

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

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

리뷰 포인트

기타 질문

Uh oh!

pyuseon commented Feb 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pyuseon Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pyuseon commented Feb 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

좋았던 점

아쉬운 점

리뷰포인트

추가 질문

Uh oh!

rmsxo200 commented Feb 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rmsxo200 commented Feb 2, 2025 •

edited

Loading

pyuseon Feb 3, 2025 •

edited

Loading

pyuseon commented Feb 3, 2025 •

edited

Loading