You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR 제목은 변경 사항의 요약을 담아야 합니다. 예시: "[1주차] 사용자 로그인 기능 구현"
📝작업 내용
이번 PR에서 작업한 내용을 설명해주세요(이미지 첨부 가능)
주요 변경 사항을 기술해 주세요. 코드 구조, 핵심 로직 등에 대해 설명하면 좋습니다.
예시:
1 ConcurrentOrderService에 동시 주문 요청 처리 기능 추가
2 RedisLock을 사용해 주문 생성 시 동시성 제어 구현
3 동시에 여러 사용자가 주문할 경우 재고 감소가 정확히 반영되도록 로직 개선
조회 API RateLimit 구현
(todo) 예약 API Rate Limit 구현
Jacoco Report 생성
🔒해결하려는 문제 혹은 고민이 되었던 부분을 남겨주세요.(문제 수 만큼 복사해서 사용할 것)
잘 짰다고 생각되는 부분이 있다면 자랑해주세요😘
예시) 다수의 사용자가 동시에 같은 리소스를 업데이트할 때 재고 수량이 음수로 내려가는 데이터 불일치 문제가 발생했습니다.
리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
ex) 메서드 ㅇㅇ의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?
요청 단위가 글로벌 하지 않고, 핵심 로직을 테스트하기 쉽고, 비즈니스 응집적이라는 장점 때문에 AOP 를 사용해서 Rate Limit을 구현했습니다. 만약 모든 요청에 RateLimit을 적용한다고 하면 필터에 Rate Limit 로직을 추가하는것도 고민해볼 것 같은데, 리뷰어님 만의 Filter vs. Interpreter vs. AOP 에 각 어떤 로직을 넣는지에 대한 기준이 있는지 궁금합니다!
저의 멀티모듈 구조는 api 모듈이 core 모듈을 의존하고, api 모듈에만 web 관련 의존성이 있습니다. 구현 시나리오 중에서 요청 횟수에 따라 특정 IP를 차단하는 기능이 있는데, AOP가 IP를 가지고 와야하는 역할이 있어서 AOP를 api 모듈에 두었습니다. 또 다른 방식으로는 api 모듈에서 IP 주소를 String 타입으로 가지고 와서 서비스 레이어의 파라미터로 전달하는 방법도 있는데, 이렇게 하게되면 Rate Limit 가 필요한 모든 메서드 마다 다 파리미터로 IP 주소를 가지고 있어야 하는데 이건 너무 비즈니스를 해치는 요소라고 생각이 들었습니다. 이러한 구조가 현재 모듈 구조에서 가장 적절한가요?
앞선 이유(AOP가 api 모듈에 있어야 하는 이유) 때문에 AOP가 controller 보다 선행되서 실행이 되는데, 이런 경우에 입력 값 validation 보다 AOP가 먼저 실행이 되어서, AOP 로직의 값에 신뢰성이 떨어집니다. 이런 경우에는 어떻게 값의 신뢰성을 보장할 수 있을까요?
예약API 에서 Rate Limit 로직은 비즈니스 시작 전에 요청 가능한지 확인하고, 비즈니스 로직이 완료(예매 성공)이 되면 요청 내역을 Redis에 저장하는 로직으로 구상을 했습니다. 이렇게 되면 read 와 write의 시점이 굉장히 떨어져 있는데, 이런 경우에는 Lua Script로 Rate Limit 로직을 구현하기에는 무리가 있다고 생각했습니다. 과제 시나리오에서 Lua Script를 사용해보라는 건 '동시성을 염두하고 구현해라'라는 뜻으로 받아들었는데, 지금 상황과 같이 Lua Script를 사용하기 적절하지 않은 로직에서는 읽기와 쓰기 로직을 분리해서 구현해도 문제 없을까요? 예약 API에서 어떤 식으로 Rate Limit 로직을 구현해야 할지 고민이 됩니다..!
Lua 파일을 string 이 아닌 파일로 관리하여, 재사용성을 더 높이신 것 같아 좋습니다.
아쉬웠던 점
domain, presentation 에 대한 테스트 코드 작성을 하신 이후, coverage 를 더 높여봐도 좋을 것 같습니다.
질문에 대한 답변
요청 단위가 글로벌 하지 않고, 핵심 로직을 테스트하기 쉽고, 비즈니스 응집적이라는 장점 때문에 AOP 를 사용해서 Rate Limit을 구현했습니다. 만약 모든 요청에 RateLimit을 적용한다고 하면 필터에 Rate Limit 로직을 추가하는것도 고민해볼 것 같은데, 리뷰어님 만의 Filter vs. Interpreter vs. AOP 에 각 어떤 로직을 넣는지에 대한 기준이 있는지 궁금합니다!
data 가공이 필요한 경우에는 AOP 가 아닌 인터셉터나 resolver 로 구현을 하구
data 가공이 없이, 권한 체크, traffic 체크, 로그 전송 등의 로직에선 AOP 를 주로 사용합니다.
저의 멀티모듈 구조는 api 모듈이 core 모듈을 의존하고, api 모듈에만 web 관련 의존성이 있습니다. 구현 시나리오 중에서 요청 횟수에 따라 특정 IP를 차단하는 기능이 있는데, AOP가 IP를 가지고 와야하는 역할이 있어서 AOP를 api 모듈에 두었습니다. 또 다른 방식으로는 api 모듈에서 IP 주소를 String 타입으로 가지고 와서 서비스 레이어의 파라미터로 전달하는 방법도 있는데, 이렇게 하게되면 Rate Limit 가 필요한 모든 메서드 마다 다 파리미터로 IP 주소를 가지고 있어야 하는데 이건 너무 비즈니스를 해치는 요소라고 생각이 들었습니다. 이러한 구조가 현재 모듈 구조에서 가장 적절한가요?
네 지금 해주신 부분이 저는 적당하다고 생각합니다. ip 를 가지고 메서드에서 넘기기에는, 비즈니스 로직을 헤칠 것 같다는 생각이 들어서 저도 해당부분을 중심으로 리뷰를 진행했었는데, 잘 구현해주셨습니다. 👍
앞선 이유(AOP가 api 모듈에 있어야 하는 이유) 때문에 AOP가 controller 보다 선행되서 실행이 되는데, 이런 경우에 입력 값 validation 보다 AOP가 먼저 실행이 되어서, AOP 로직의 값에 신뢰성이 떨어집니다. 이런 경우에는 어떻게 값의 신뢰성을 보장할 수 있을까요?
해당부분은 저도 좀 더 고민을 해봐야할 것 같아요. 찾아보니 아래와 같은 방법을 적용할 수 있겠네요.
방법1. AOP 에는 validation 되는 필드를 참조하지 않는 식으로 구현하면 될 것 같구요. (현재 url, ip)
예약 완료 시점에 저장하고, 다시 같은 시간대 스케줄을 예약하려고 시도하는 경우 limit 을 거는거기 때문에 생각하신 대로 , 읽기-쓰기 시점이 길더라도 무방하다고 판단됩니다.
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
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.
[4주차] Rate Limit 구현 및 Jacoco Report 생성
📝작업 내용
🔒해결하려는 문제 혹은 고민이 되었던 부분을 남겨주세요.(문제 수 만큼 복사해서 사용할 것)
🔑해당 문제를 어떻게 해결했나요? 그리고 왜 그렇게 생각했는지 이유를 남겨주세요.(자세하게 남겨주실 수록 좋습니다.)
💬리뷰 요구사항(선택)
기타 사항 📌
추가로 언급할 사항이 있다면 여기에 적어주세요.