-
Notifications
You must be signed in to change notification settings - Fork 1
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
Api: ✨ 카테고리에 등록된 소비 리스트 무한 스크롤 조회 API #120
Conversation
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.
고생하셨습니다! 아래 코멘트 하나만 확인해주시면 될 것 같습니다ㅎ
혹시 무한 스크롤 응답에 대해 잘 모르신다면, 꼭 질문해주세요! 페이지네이션과 살짝 다릅니다.
무한 스크롤을 구현하는 것은 해본적이 없어서, Slice에 대해서 조금 공부했습니다.
궁금한건, 클라이언트에서는 hasNext 필드값을 이용해 다음 Api 요청을 할지 말지를 결정하는 것인가요?
Mapper의 toMonthSlice() 메서드 시간 복잡도 대략 O(KlogK) 정도로 고려됨.
조회된 데이터를 클라이언트에게 가공해 전달하기 위한 전처리 치고는 무거운 연산량인 것 같긴한데, 저도 현안이 최선책인듯 합니다ㅎ.
Pageable의 size 상한선을 제한하지 않았는데, 필요하다고 보시나요?
다른 부분들과 다르게 pagenation에 사용되는 limit은 클라이언트단에서 값이 변경될 일이 없는 부분인 것 같아 정상적이지 않은 수준의 요청에 대응을 굳이 할 필요는 없다고 생각합니다.
...way-domain/src/main/java/kr/co/pennyway/domain/domains/spending/service/SpendingService.java
Show resolved
Hide resolved
.../src/main/java/kr/co/pennyway/api/common/security/authorization/SpendingCategoryManager.java
Show resolved
Hide resolved
...way-app-external-api/src/main/java/kr/co/pennyway/api/apis/ledger/mapper/SpendingMapper.java
Show resolved
Hide resolved
네, 맞습니다. 다음 원소 존재 여부를 알 수 있는 이유는 데이터를 가져올 때 기본 size인 30에서 1을 더한 값만큼 불러오기 때문입니다.
ㅠㅠ 더 나은 방법이 있을 지 고민해볼게요.
아, 이런 부분들을 신경쓰는 이유는 정상적인 client의 요청이 아닌 악의적인 공격에 대응할 필요가 있는 지에 대해 논의해보기 위함이었습니다! |
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.
의견 남겨뒀습니다!
승인할게요ㅎ
작업 이유
작업 사항
1️⃣ Controller 계층
GET /v2/spending-categories/{category_id}/spendings
2️⃣ UseCase 계층
SpendingSearchRes.Month
DTO를 배열로 갖는 신규 DTO를 추가해주었습니다.spending.id
를 기준으로 정렬)3️⃣ Service 계층
type
에 따라 같은 도메인 서비스의 다른 메서드를 호출합니다.4️⃣ DAO 계층
➕ 전체 개수 조회 시 DB에 전송하는 쿼리
리뷰어가 중점적으로 확인해야 하는 부분
toMonthSlice()
메서드 시간 복잡도 대략 O(KlogK) 정도로 고려됨.size
상한선을 제한하지 않았는데, 필요하다고 보시나요??size=100000
라고 써버리면, 그대로 통과합니다. 🤗중점적으로 확인할 부분이라기엔 좀 애매한데, 테스트 케이스를 작성하지 않은 이유는 생각보다 구현이 너무 오래 걸려서..
여기서 테케까지 작성해보려고 욕심 부렸으면, 시간이 배로 걸렸을 거 같아 일단 포기하고 빠르게 넘겨버렸습니다.
혹시 필요하다고 생각하시면 추가해둘게용.
발견한 이슈