refactor: 알라딘 외부 검색 API 호출 시 totalResult를 200개로 제한, lastPage 컬럼 값 추가 #89
Conversation
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
Walkthrough알라딘 도서 검색 API의 최대 결과 제한(200개)을 반영하여, 관련 서비스 로직, 에러 코드, DTO를 수정하였고, 페이징 응답 구조를 개선하였습니다. 또한 독서 기록 조회 API의 반환 타입을 커스텀 페이지 응답 객체로 변경하고, 이에 따라 관련 인터페이스 및 유스케이스 로직도 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant UseCase
participant Service
participant AladinAPI
Client->>Controller: 도서 검색 요청
Controller->>Service: searchBooks 호출
Service->>AladinAPI: 외부 API 호출
AladinAPI-->>Service: 검색 결과 반환 (totalResults 등)
Service->>Service: cappedTotalResults, isLastPage 계산
alt currentFetchedCount > 200
Service->>Controller: BookException(ALADIN_API_RESULT_LIMIT) throw
Controller-->>Client: 403 Forbidden 에러 반환
else
Service->>Controller: BookSearchResponse.of(..., isLastPage)
Controller-->>Client: 검색 결과 반환 (lastPage, totalResults 등)
end
sequenceDiagram
participant Client
participant Controller
participant UseCase
participant Service
Client->>Controller: 독서기록 조회 요청
Controller->>UseCase: getReadingRecordsByUserBookId 호출
UseCase->>Service: getReadingRecordsByDynamicCondition 호출
Service-->>UseCase: Page<ReadingRecordResponse> 반환
UseCase->>UseCase: ReadingRecordPageResponse.from(page)
UseCase->>Controller: ReadingRecordPageResponse 반환
Controller-->>Client: ReadingRecordPageResponse 반환
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 사항 없음) Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
minwoo1999
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt (1)
103-110: OpenAPI 스키마 업데이트 누락: Page -> ReadingRecordPageResponse로 변경 필요메서드 반환 타입은
ReadingRecordPageResponse로 변경되었지만,@ApiResponses의 스키마는 여전히Page::class로 지정되어 있습니다. 문서/클라이언트 코드 생성 시 혼란을 야기할 수 있으므로 아래와 같이 수정해 주세요.@ApiResponses( value = [ ApiResponse( responseCode = "200", description = "독서 기록 목록 조회 성공", - content = [Content(schema = Schema(implementation = Page::class))] + content = [Content(schema = Schema(implementation = ReadingRecordPageResponse::class))] ),Also applies to: 90-95
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/exception/BookErrorCode.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/response/ReadingRecordPageResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt(2 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt(1 hunks)
🔇 Additional comments (5)
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordControllerApi.kt (1)
19-19: 필요한 DTO import 추가 확인ReadingRecordPageResponse 사용을 위한 import 추가는 적절합니다.
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt (1)
58-66: 반환 타입 변경 일관성 확인 완료컨트롤러 반환 타입을
ReadingRecordPageResponse로 통일한 점 좋습니다. UseCase 및 인터페이스와도 일치합니다.apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt (1)
54-69: Page -> DTO 변환 로직 적절서비스에서 받은
Page<ReadingRecordResponse>를 DTO로 변환하는 흐름이 명확합니다. 반환 타입 변경과도 일관됩니다. DTO 내부 필드 설계(예: totalResults, startIndex)는 DTO 파일 코멘트를 참고해 주세요.apis/src/main/kotlin/org/yapp/apis/book/exception/BookErrorCode.kt (1)
14-15: 알라딘 API 검색 결과 제한 예외 메시지 및 상태 코드 검토 필요
- BookErrorCode.kt (apis/src/main/kotlin/org/yapp/apis/book/exception/BookErrorCode.kt)
ALADIN_API_RESULT_LIMIT의 메시지와 HTTP 상태 재검토- 메시지 수정 제안: 실제 예외 상황(200개 초과로 차단) 안내에 맞춰 표현을 명확히
- 상태 코드 변경(4xx) 및 코드 체계(BOOK_4xx_xx) 재정렬 시 전역 예외 처리기와 연관된 영역(ControllerAdvice, 테스트 등) 영향도 확인 필요
제안 드래프트:
- ALADIN_API_RESULT_LIMIT(HttpStatus.INTERNAL_SERVER_ERROR, "BOOK_500_03", "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다."); + ALADIN_API_RESULT_LIMIT(HttpStatus.INTERNAL_SERVER_ERROR, "BOOK_500_03", "알라딘 API 제한으로 인해 최대 200개까지만 조회할 수 있습니다.");(선택) 4xx로 변경 시:
- ALADIN_API_RESULT_LIMIT(HttpStatus.INTERNAL_SERVER_ERROR, "BOOK_500_03", "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다."); + ALADIN_API_RESULT_LIMIT(HttpStatus.BAD_REQUEST, "BOOK_400_03", "알라딘 API 제한으로 인해 검색 결과는 최대 200개까지만 조회할 수 있습니다.");변경 후 전역 예외 처리기 및 통합/단위 테스트에서 의도한 HTTP 응답이 나오는지 꼭 확인 바랍니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (1)
49-51: lastPage 필드 추가 적절클라이언트 페이지네이션 제어에 필요한 정보가 명확히 반영되었습니다. 스키마 설명도 적절합니다.
| query = response.query, | ||
| searchCategoryId = response.searchCategoryId, | ||
| searchCategoryName = response.searchCategoryName, | ||
| lastPage = response.lastPage, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Infra DTO에 의존한 lastPage 전달 최소화 제안
lastPage = response.lastPage는 서비스 계층에서 계산된 값을 Infra DTO(외부 응답 모델)에 세팅한 뒤 재사용하는 패턴입니다. 결합도를 낮추려면 BookSearchResponse.from에 lastPage: Boolean 인자를 추가해 직접 전달하는 방식을 고려해 주세요. 이렇게 하면 Infra 모델을 내부 상태 전달 용도로 변형할 필요가 없습니다.
from 시그니처 변경 영향도 확인:
🏁 Script executed:
#!/bin/bash
ast-grep --pattern $'companion object { $$$ fun from($_): $_ { $$$ } $$$ }' | rg -n "BookSearchResponse"
rg -n "BookSearchResponse\\.from\\(" -A 2Length of output: 440
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.ktLength of output: 4943
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.ktLength of output: 4947
BookSearchResponse.from에 lastPage 매개변수 추가로 Infra DTO 변형 최소화
서비스 계층에서 Infra DTO(AladinSearchResponse)의 lastPage 값을 변경한 뒤 재사용하는 대신, BookSearchResponse.from에 lastPage: Boolean 인자를 추가해 직접 전달하도록 변경해주세요. 이렇게 하면 Infra 모델을 내부 상태 전달 용도로 변형(apply)할 필요가 없어집니다.
영향 받는 위치:
- apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
- apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
제안하는 수정 예시:
// BookSearchResponse.kt
companion object {
- fun from(response: AladinSearchResponse): BookSearchResponse {
+ fun from(response: AladinSearchResponse, lastPage: Boolean): BookSearchResponse {
return BookSearchResponse(
version = response.version,
title = response.title,
link = response.link,
pubDate = response.pubDate,
totalResults = response.totalResults,
startIndex = response.startIndex,
itemsPerPage = response.itemsPerPage,
query = response.query,
searchCategoryId = response.searchCategoryId,
searchCategoryName = response.searchCategoryName,
- lastPage = response.lastPage,
+ lastPage = lastPage,
books = response.item.map { … }
)
}
}// AladinBookQueryService.kt
- // 기존: DTO에 lastPage를 설정(apply) 후 넘김
- val filteredResponse = AladinSearchResponse( … ).apply { lastPage = isLastPage }
- return BookSearchResponse.from(filteredResponse)
+ // 수정: apply로 DTO 상태를 바꾸지 않고 lastPage를 직접 전달
+ val filteredResponse = AladinSearchResponse( … )
+ return BookSearchResponse.from(filteredResponse, isLastPage)이렇게 변경하면 Infra 모델의 내부 프로퍼티를 변형하지 않고도 lastPage 정보를 전달할 수 있어 결합도를 낮출 수 있습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| lastPage = response.lastPage, | |
| // apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt | |
| companion object { | |
| - fun from(response: AladinSearchResponse): BookSearchResponse { | |
| + fun from(response: AladinSearchResponse, lastPage: Boolean): BookSearchResponse { | |
| return BookSearchResponse( | |
| version = response.version, | |
| title = response.title, | |
| link = response.link, | |
| pubDate = response.pubDate, | |
| totalResults = response.totalResults, | |
| startIndex = response.startIndex, | |
| itemsPerPage = response.itemsPerPage, | |
| query = response.query, | |
| searchCategoryId = response.searchCategoryId, | |
| searchCategoryName = response.searchCategoryName, | |
| - lastPage = response.lastPage, | |
| + lastPage = lastPage, | |
| books = response.item.map { … } | |
| ) | |
| } | |
| } |
Suggested change
| lastPage = response.lastPage, | |
| // apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt | |
| - // 기존: DTO에 lastPage를 설정(apply) 후 넘김 | |
| - val filteredResponse = AladinSearchResponse( … ).apply { lastPage = isLastPage } | |
| - return BookSearchResponse.from(filteredResponse) | |
| + // 수정: apply로 DTO 상태를 바꾸지 않고 lastPage를 직접 전달 | |
| + val filteredResponse = AladinSearchResponse( … ) | |
| + return BookSearchResponse.from(filteredResponse, isLastPage) |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt at
line 72 and also in
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt,
modify the BookSearchResponse.from method to add a lastPage Boolean parameter.
Pass the lastPage value directly to this method instead of modifying the Infra
DTO (AladinSearchResponse). Update all calls to BookSearchResponse.from
accordingly to supply the lastPage argument, thereby avoiding internal state
changes to the Infra DTO and reducing coupling.
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
Outdated
Show resolved
Hide resolved
Comment on lines
69
to
79
| totalResults = cappedTotalResults, | ||
| startIndex = response.startIndex, | ||
| itemsPerPage = response.itemsPerPage, | ||
| query = response.query, | ||
| searchCategoryId = response.searchCategoryId, | ||
| searchCategoryName = response.searchCategoryName, | ||
| item = transformedItems | ||
| ) | ||
| ).apply { | ||
| lastPage = isLastPage | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
lastPage 설정은 생성자 주입으로 간소화 + 로그 민감도 고려
apply { lastPage = isLastPage }대신 생성자 인자로 전달하면 가독성과 불변성이 좋아집니다.- 대용량/민감 데이터 로그 노출 방지를 위해 전체 응답을 INFO로 출력하는 로그는 축소/마스킹/DEBUG 전환을 권장합니다.
생성자 전달로 정리:
val filteredResponse = AladinSearchResponse(
version = response.version,
title = response.title,
link = response.link,
pubDate = response.pubDate,
totalResults = cappedTotalResults,
startIndex = response.startIndex,
itemsPerPage = response.itemsPerPage,
query = response.query,
searchCategoryId = response.searchCategoryId,
searchCategoryName = response.searchCategoryName,
- item = transformedItems
- ).apply {
- lastPage = isLastPage
- }
+ item = transformedItems,
+ lastPage = isLastPage
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| totalResults = cappedTotalResults, | |
| startIndex = response.startIndex, | |
| itemsPerPage = response.itemsPerPage, | |
| query = response.query, | |
| searchCategoryId = response.searchCategoryId, | |
| searchCategoryName = response.searchCategoryName, | |
| item = transformedItems | |
| ) | |
| ).apply { | |
| lastPage = isLastPage | |
| } | |
| val filteredResponse = AladinSearchResponse( | |
| version = response.version, | |
| title = response.title, | |
| link = response.link, | |
| pubDate = response.pubDate, | |
| totalResults = cappedTotalResults, | |
| startIndex = response.startIndex, | |
| itemsPerPage = response.itemsPerPage, | |
| query = response.query, | |
| searchCategoryId = response.searchCategoryId, | |
| searchCategoryName = response.searchCategoryName, | |
| item = transformedItems, | |
| lastPage = isLastPage | |
| ) |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
between lines 69 and 79, the lastPage property is set using an apply block after
object creation, which reduces readability and immutability. Refactor the code
to pass lastPage as a constructor argument instead of setting it afterward.
Additionally, review any INFO-level logging of the full response data and reduce
its verbosity by masking sensitive information or changing the log level to
DEBUG to prevent exposing large or sensitive data.
| import org.springframework.web.bind.annotation.RequestParam | ||
| import org.springframework.web.bind.annotation.RestController | ||
| import org.yapp.apis.readingrecord.dto.request.CreateReadingRecordRequest | ||
| import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse // Added import |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
리뷰 잔재 주석 제거
프로덕션 코드에서 // Added import 주석은 불필요합니다. 정리 부탁드립니다.
-import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse // Added import
+import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse // Added import | |
| import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/controller/ReadingRecordController.kt
at line 18, remove the unnecessary comment "// Added import" that was added
alongside the import statement for ReadingRecordPageResponse, as such comments
are not needed in production code.
Comment on lines
+10
to
+16
| data class ReadingRecordPageResponse private constructor( | ||
| @field:Schema(description = "마지막 페이지 여부", example = "false") | ||
| val lastPage: Boolean, | ||
|
|
||
| @field:Schema(description = "총 결과 개수", example = "42") | ||
| val totalResults: Int, | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
totalResults 다운캐스팅(Int)으로 인한 정보 손실 가능성 — Long 사용 권장
Page.totalElements는 Long입니다. 현재 toInt()로 다운캐스팅하고 있어 대용량 시 오버플로/정보 손실 위험이 있습니다. totalResults를 Long으로 변경해 주세요.
data class ReadingRecordPageResponse private constructor(
@field:Schema(description = "마지막 페이지 여부", example = "false")
val lastPage: Boolean,
- @field:Schema(description = "총 결과 개수", example = "42")
- val totalResults: Int,
+ @field:Schema(description = "총 결과 개수", example = "42")
+ val totalResults: Long,
@@
return ReadingRecordPageResponse(
lastPage = page.isLast,
- totalResults = page.totalElements.toInt(),
+ totalResults = page.totalElements,Also applies to: 27-31
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/response/ReadingRecordPageResponse.kt
around lines 10 to 16 and also lines 27 to 31, the totalResults property is
currently an Int but should be changed to Long to avoid potential overflow or
data loss when converting from Page.totalElements which is a Long. Update the
totalResults type from Int to Long and ensure any related code uses Long
consistently without downcasting to Int.
Comment on lines
+17
to
+22
| @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0") | ||
| val startIndex: Int, | ||
|
|
||
| @field:Schema(description = "한 페이지당 아이템 개수", example = "10") | ||
| val itemsPerPage: Int, | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
startIndex 의미 명확화 필요: 필드명 vs 값 불일치 가능성
설명은 "현재 페이지 번호(0부터 시작)"인데, 필드명은 보통 "첫 항목의 시작 오프셋"을 연상시키는 startIndex입니다. 혼란 방지를 위해 아래 둘 중 하나를 권장합니다.
- 대안 A(권장, 오프셋 의미 유지):
startIndex = page.number * page.size로 변경하고 설명도 오프셋으로 수정 - 대안 B(페이지 번호 의미 유지): 필드명을
pageIndex로 변경하고 설명은 현행 유지
대안 A(diff):
- @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0")
- val startIndex: Int,
+ @field:Schema(description = "현재 페이지의 첫 결과 오프셋(0부터 시작)", example = "0")
+ val startIndex: Int,
@@
- startIndex = page.number,
+ startIndex = page.number * page.size,대안 B(diff):
- @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0")
- val startIndex: Int,
+ @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0")
+ val pageIndex: Int,
@@
- startIndex = page.number,
+ pageIndex = page.number,확정하신 의도를 알려주시면 그에 맞춰 나머지 레이어(문서/클라이언트 영향 범위) 반영 패치도 도와드리겠습니다.
Also applies to: 27-35
아래 스크립트를 실행하여 ReadingRecordPageResponse.kt 파일 내에서 startIndex 필드 정의와 매핑 로직을 확인해 주세요.
🏁 Script executed:
#!/bin/bash
set -e
# 파일 위치 찾기
file=$(fd -e kt ReadingRecordPageResponse.kt)
echo "Found file: $file"
echo
echo "=== Fields (1~50 lines) ==="
sed -n '1,50p' "$file"
echo
echo "=== Mapping logic for startIndex ==="
rg -n 'startIndex' -C 3 "$file"Length of output: 2465
startIndex 필드명과 실제 매핑 불일치: 페이지 번호 vs 오프셋 의미 검토 및 수정 필요
현재 startIndex는 필드명으로는 “첫 결과 오프셋”을 연상시키지만, 실제 매핑 로직에서는 page.number(페이지 인덱스)를 사용하고 있어 혼동을 초래합니다. 아래 둘 중 하나를 선택해 반영해 주세요.
• 대상 파일
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/response/ReadingRecordPageResponse.kt
• 문제 위치
- 필드 정의: 17행
- 매핑 로직: 31행
• 대안 A (오프셋 의미 유지, 매핑 수정)
- 설명 변경:
"현재 페이지의 첫 결과 오프셋(0부터 시작)" - 매핑 변경:
startIndex = page.number * page.size
@@ 17,18c17,18
- @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0")
+ @field:Schema(description = "현재 페이지의 첫 결과 오프셋(0부터 시작)", example = "0")
@@ 31c31
- startIndex = page.number,
+ startIndex = page.number * page.size,• 대안 B (페이지 번호 의미 유지, 필드명 변경)
- 필드명:
startIndex→pageIndex - 설명 유지 및 매핑 유지
@@ 17,18c17,18
- @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0")
- val startIndex: Int,
+ @field:Schema(description = "현재 페이지 번호 (0부터 시작)", example = "0")
+ val pageIndex: Int,
@@ 31,32c31,32
- startIndex = page.number,
+ pageIndex = page.number,위 중 적절한 의도를 선택해 알려주시면, 나머지 문서·클라이언트 레이어 반영도 도와드리겠습니다.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/response/ReadingRecordPageResponse.kt
around lines 17 to 22 and 31, the field name 'startIndex' suggests an offset but
is mapped to the page number, causing confusion. To fix this, either update the
description to "현재 페이지의 첫 결과 오프셋(0부터 시작)" and change the mapping to calculate
offset as page.number * page.size, or rename the field to 'pageIndex' to reflect
the current page number and keep the existing mapping and description. Choose
one approach and apply consistent changes to the field definition and mapping
accordingly.
Comment on lines
+23
to
+25
| @field:Schema(description = "독서 기록 목록") | ||
| val readingRecords: List<ReadingRecordResponse> | ||
| ) { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
배열 스키마 명시로 문서 명확성 향상 권장
스웨거에서 제네릭 배열을 잘 추론하긴 하지만, 명시적으로 item 타입을 지정하면 문서 가독성이 좋아집니다.
+import io.swagger.v3.oas.annotations.media.ArraySchema
@@
- @field:Schema(description = "독서 기록 목록")
- val readingRecords: List<ReadingRecordResponse>
+ @field:ArraySchema(arraySchema = Schema(description = "독서 기록 목록"), schema = Schema(implementation = ReadingRecordResponse::class))
+ val readingRecords: List<ReadingRecordResponse>🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/dto/response/ReadingRecordPageResponse.kt
around lines 23 to 25, the Swagger schema annotation for the readingRecords list
should explicitly specify the item type to improve documentation clarity. Update
the @field:Schema annotation to include the 'implementation' or 'type' attribute
that clearly defines the type of items in the list, ensuring Swagger generates
more readable and precise API docs.
| import org.springframework.transaction.annotation.Transactional | ||
| import org.yapp.apis.book.service.UserBookService | ||
| import org.yapp.apis.readingrecord.dto.request.CreateReadingRecordRequest | ||
| import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse // Added import |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
리뷰 잔재 주석 제거
import 라인의 // Added import 주석은 제거하는 것이 깔끔합니다.
-import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse // Added import
+import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse // Added import | |
| import org.yapp.apis.readingrecord.dto.response.ReadingRecordPageResponse |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/readingrecord/usecase/ReadingRecordUseCase.kt
at line 8, remove the trailing comment "// Added import" from the import
statement to keep the code clean and professional.
infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt
Outdated
Show resolved
Hide resolved
move-hoon
reviewed
Aug 10, 2025
Member
There was a problem hiding this comment.
고생하셨습니다 민우님!
제가 드리고 싶은 의견을 빠르게 요약하면 다음과 같습니다.
- AladinSearchResponse 내부에 var lastPage 칼럼 삭제(오직 알라딘 API의 응답을 담는 역할)
- Service 계층(AladinBookQueryService)에서 lastPage 계산
- BookSearchResponse.of(filteredResponse, isLastPage) 형식으로 isLastPage 프로퍼티 주입
- 200이라는 상수에 대해서 매직넘버 처리(의미를 담기 위함)
- 500에러 대신 403 Forbidden 에러를 던지는게 의미에 부합할 것 같습니다.
- 상위 "383개의 결과 중 상위 200개 표시"라는 기획안에 맞추기 위해서는 기존 totalResult를 보여줘야 하는데, 지금은 200개 초과시 totalResult=200을 보여주고 있어 수정이 필요할 것 같습니다.
- 알라딘 검색 응답의 link를 아이템별 link로 변경
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
Outdated
Show resolved
Hide resolved
Comment on lines
19
to
20
| @JsonProperty("item") val item: List<AladinSearchItem> = emptyList(), | ||
| var lastPage: Boolean = false |
Member
There was a problem hiding this comment.
순수 알라딘 API의 응답이기 때문에 삭제하는게 좋을 것 같습니다!!
Member
|
문제상황이 한 가지 있을 것 같습니다. 문제 상황 정리(totalResults가 200개 이하인 경우는 상관없음)
해결책
1단계: 빠른 첫 페이지 응답 (사용자 요청 시)
2단계: 비동기 전체 개수 계산 (서버 백그라운드)
3단계: 클라이언트의 후속 처리
|
|
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (2)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (2)
47-48: 대용량 응답 전체를 INFO로 로깅하지 마세요전체 응답을 INFO로 남기면 로그가 비대해지고 운영비/가독성에 악영향을 줍니다. 필요한 요약 정보만 DEBUG로 낮추세요.
- log.info { "Before filtering - Full Response: $response" } + log.debug { "Before filtering - size=${response.item.size}, total=${response.totalResults}, start=${response.startIndex}, perPage=${response.itemsPerPage}" } ... - log.info { "After filtering - Full Response: $filteredResponse" } + log.debug { "After filtering - size=${filteredResponse.item.size}, total=${filteredResponse.totalResults}, start=${filteredResponse.startIndex}, perPage=${filteredResponse.itemsPerPage}" }Also applies to: 78-78
27-37: 사전 호출 전 200건 제한 검증 추가 제안Aladin API의
start파라미터가 ‘아이템 인덱스’가 아닌 1부터 시작하는 페이지 번호라는 점을 반영해, 외부 호출 직전에(page × pageSize) > 200여부를 체크해 불필요한 요청을 차단하는 로직을 추가하는 것을 권장합니다.• 적용 위치
•AladinBookQueryService.searchBooks메서드 내부,aladinSearchRequest생성 직전
• 검증 로직
• 총 요청 건수 =request.start * request.maxResults
• 페이지 번호(start)가 1부터 시작하므로 그대로 곱셈 사용
• 총 요청 건수가 200초과 시, 동일한 에러 메시지/코드로 예외 발생
• 추가 고려사항
•request.maxResults기본값을AladinBookSearchRequest의 실제 기본값과 단일 소스로 맞추기
• 사전 검증 에러에 사용할 메시지/코드 정의부(상수) 재사용예시(diff):
override fun searchBooks(request: BookSearchRequest): BookSearchResponse { // 1) 사전 검증: 페이지*페이지당항목 > 200 차단 val totalItems = request.start * request.maxResults if (totalItems > 200) { throw IllegalArgumentException(AladinError.OVER_LIMIT.message) } log.info { "Service - Converting BookSearchRequest to AladinBookSearchRequest and calling Aladin API for book search." } val aladinSearchRequest = AladinBookSearchRequest.of( request.validQuery(), request.queryType, request.searchTarget, request.maxResults, request.start, request.sort, request.categoryId ) … }
♻️ Duplicate comments (1)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (1)
54-64: 페이지 상한/lastPage 계산 로직 교정: 실제 반환 개수 기반 endIndex 사용
- 현재
currentFetchedCount = (start × itemsPerPage)는 페이지 끝 추정치이며, null 시 0으로 계산되어 한도 검증이 무력화될 수 있습니다.- 팀 합의(learned): start/startIndex는 1-based "페이지 번호"입니다. 따라서 endIndex는 "현재 페이지의 시작 인덱스 + 실제 반환 개수 - 1"로 계산해야 정확합니다.
- 필터링으로
transformedItems.size가itemsPerPage보다 작을 수 있어 lastPage 판정에 이를 반영해야 합니다.- val cappedTotalResults = minOf(response.totalResults ?: 0, MAX_ALADIN_RESULTS) - val currentFetchedCount = (request.start ?: 1) * (response.itemsPerPage ?: 0) - - if (currentFetchedCount > MAX_ALADIN_RESULTS) { - throw BookException( - BookErrorCode.ALADIN_API_RESULT_LIMIT - ) - } - - val isLastPage = currentFetchedCount >= cappedTotalResults + val cappedTotalResults = minOf(response.totalResults ?: 0, MAX_ALADIN_RESULTS) + val currentPage = request.start ?: 1 // 1-based page number (team convention) + val itemsPerPage = response.itemsPerPage ?: transformedItems.size + val endIndex = (currentPage - 1) * itemsPerPage + transformedItems.size + + if (endIndex > MAX_ALADIN_RESULTS) { + throw BookException(BookErrorCode.ALADIN_API_RESULT_LIMIT) + } + + val isLastPage = endIndex >= cappedTotalResults || transformedItems.isEmpty()보완 권고:
- 네트워크 비용 절감을 위해 외부 호출 전
request.start와request.maxResults로 사전 차단(preflight) 검증을 추가하세요(요청이 200을 넘어서는 범위를 요구하면 즉시 예외).request.maxResults의 기본값은AladinBookSearchRequest의 기본과 일치시켜 단일 소스로 관리하는 것이 안전합니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(5 hunks)apis/src/main/kotlin/org/yapp/apis/book/exception/BookErrorCode.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-10T09:30:58.123Z
Learnt from: hoonyworld
PR: YAPP-Github/Reed-Server#89
File: apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt:53-63
Timestamp: 2025-08-10T09:30:58.123Z
Learning: 알라딘 API의 `start` 또는 `startIndex` 파라미터는 페이지 번호를 나타낸다 (예: start=2는 2페이지를 의미). 이는 1부터 시작하는 페이지 번호이며, 아이템 인덱스가 아니다. AladinBookQueryService.kt 파일에서 페이지 기반 계산 시 이를 고려해야 한다.
Applied to files:
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (3)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (1)
80-81: BookSearchResponse.of로 lastPage 전달하는 방향 LGTMInfra DTO를 변형하지 않고 계산된 lastPage를 전달하는 구조로 정리된 점 좋습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (2)
54-79: of(response, isLastPage) 팩토리로 lastPage 전달하는 구조 LGTMInfra DTO를 내부 상태 전달 용도로 변형하지 않고, 서비스 계층 계산값을 명시적으로 주입하는 방식이 깔끔합니다.
66-76: BookSummary.of의 cover/link null-안전성 검증 결과
AladinSearchItem의cover·link가 모두 non-nullableString으로 정의되어 있어, JSON에 null이 들어올 경우 Jackson 바인딩 단계에서 예외가 발생하므로 이후 NPE 우려는 없습니다. 다만 빈 문자열("")이 API 응답으로 올 가능성은 검토가 필요합니다.검토 포인트:
- infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt
•@JsonProperty("cover") val cover: String
•@JsonProperty("link") val link: String- apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
•BookSummary.of(..., coverImageUrl = it.cover, link = it.link)매핑 지점추가 제안:
- API에서 빈 문자열 반환 여부 확인
- 빈값 대응을 위해 기본 이미지 또는 기본 링크 제공 로직 도입
- 필요 시 Jackson
defaultValue설정 또는 커스텀 Deserializer 활용
Comment on lines
+43
to
+45
| @field:Schema(description = "마지막 페이지 여부", example = "false") | ||
| val lastPage: Boolean, | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
lastPage 필드 추가 LGTM — 스키마 설명에 200개 제한 정책을 명시 제안
필터링/상한(200개) 정책 하에서 lastPage는 총합과 불일치할 수 있음을 스키마 설명에 한 줄 보강하면 소비자 혼선을 줄일 수 있습니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
around lines 43 to 45, the lastPage field's schema description should be
enhanced to mention the 200-item limit policy. Update the @field:Schema
description to include a note that due to filtering or the 200-item maximum
limit, lastPage may not always align with the total count, to reduce confusion
for API consumers.
Comment on lines
+102
to
+107
| @field:Schema( | ||
| description = "알라딘 도서 상세 페이지 링크", | ||
| example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" | ||
| ) | ||
| val link: String, // Added link field | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
BookSummary.link의 null 허용 또는 기본값 제공 검토
외부 링크가 누락될 수 있다면 nullable 또는 기본값이 안전합니다. 오픈API 문서에도 선택값임을 반영하세요.
- val link: String, // Added link field
+ val link: String?, // 외부 응답 누락 대비📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @field:Schema( | |
| description = "알라딘 도서 상세 페이지 링크", | |
| example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" | |
| ) | |
| val link: String, // Added link field | |
| @field:Schema( | |
| description = "알라딘 도서 상세 페이지 링크", | |
| example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" | |
| ) | |
| val link: String?, // 외부 응답 누락 대비 |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
around lines 102 to 107, the 'link' field should be made nullable or given a
default value to handle cases where the external link might be missing. Update
the 'link' property to be nullable (e.g., String?) or provide a default value,
and modify the @Schema annotation to indicate that this field is optional in the
OpenAPI documentation.
Comment on lines
+122
to
124
| coverImageUrl: String, | ||
| link: String // Added link | ||
| ): BookSummary { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
팩토리 시그니처/구현을 null 허용에 맞추어 조정
위 제안과 함께 일관되게 처리하세요.
- coverImageUrl: String,
- link: String // Added link
+ coverImageUrl: String?,
+ link: String?
): BookSummary {
require(!title.isNullOrBlank()) { "Title is required" }
return BookSummary(
isbn13 = isbn13 ?: IsbnConverter.toIsbn13(isbn)
?: throw IllegalArgumentException("Either isbn13 or isbn must be provided"),
title = title,
author = author,
publisher = publisher?.let { BookDataValidator.removeParenthesesFromPublisher(it) },
- coverImageUrl = coverImageUrl,
- link = link,
+ coverImageUrl = coverImageUrl ?: "",
+ link = link,
userBookStatus = BookStatus.BEFORE_REGISTRATION
)추가로, 커버 기본값(예: placeholder 이미지 URL)을 환경설정으로 관리하면 좋습니다.
Also applies to: 134-135
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
around lines 122-124 and 134-135, adjust the factory method signatures and
implementations to consistently allow null values for parameters like
coverImageUrl and link. Additionally, manage the default cover image URL (e.g.,
a placeholder image) through environment configuration rather than hardcoding
it. Update the code to fetch this default from configuration when the
coverImageUrl is null.
Comment on lines
+12
to
+15
| /* 403 FORBIDDEN */ | ||
| ALADIN_API_SEARCH_FAILED(HttpStatus.FORBIDDEN, "BOOK_403_01", "알라딘 도서 검색 API 호출에 실패했습니다."), | ||
| ALADIN_API_LOOKUP_FAILED(HttpStatus.FORBIDDEN, "BOOK_403_02", "알라딘 도서 상세 조회 API 호출에 실패했습니다."), | ||
| ALADIN_API_RESULT_LIMIT(HttpStatus.FORBIDDEN, "BOOK_403_03", "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다."); |
There was a problem hiding this comment.
💡 Verification agent
❓ Verification inconclusive
ALADIN_API_RESULT_LIMIT 메시지 수정 및 403 적용 타당성 검토 제안
- 현재 메시지 "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다."는 예외를 던지는 동작과 상충됩니다. "요청 범위가 제한을 초과"했다는 식으로 명확화가 필요합니다.
- 200개 한도 위반은 권한 문제(403)라기보다 요청 유효성(400/422) 혹은 범위 문제(416)에 가깝습니다. 기존 클라이언트와 에러 정책에 따라 403 유지도 가능하나, 사전 합의/문서화가 필요해 보입니다.
적용 예시:
- ALADIN_API_RESULT_LIMIT(HttpStatus.FORBIDDEN, "BOOK_403_03", "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다.");
+ ALADIN_API_RESULT_LIMIT(
+ HttpStatus.FORBIDDEN,
+ "BOOK_403_03",
+ "검색 결과 상한(200개)을 초과하는 범위를 요청했습니다. 상위 200개까지만 조회 가능합니다."
+ );추가 권고:
- 이 파일의 403 전환이 외부/내부 클라이언트 계약(에러 처리 분기)에 영향을 주지 않는지 통합테스트/계약테스트 확인을 권장합니다. 필요 시 API 문서(에러 코드표) 업데이트 바랍니다.
ALADIN_API_RESULT_LIMIT 메시지 및 HTTP 상태 코드 검토 필요
ALADIN_API_RESULT_LIMIT 예외 메시지가 실제 동작(요청 범위 초과)과 불일치하며, 403 FORBIDDEN 적용 타당성도 재검토가 필요합니다.
• 위치
- 파일: apis/src/main/kotlin/org/yapp/apis/book/exception/BookErrorCode.kt
- 라인: 12–15
• 제안된 수정 예시
- ALADIN_API_RESULT_LIMIT(HttpStatus.FORBIDDEN, "BOOK_403_03", "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다.");
+ ALADIN_API_RESULT_LIMIT(
+ HttpStatus.FORBIDDEN,
+ "BOOK_403_03",
+ "검색 결과 상한(200개)을 초과하는 범위를 요청했습니다. 상위 200개까지만 조회 가능합니다."
+ );• 검토 사항
- “요청 범위 제한 초과”를 명확히 반영한 메시지로 변경
- 403 FORBIDDEN 대신 400/422(요청 유효성) 또는 416(범위 초과)이 더 적합한지 사내 정책·클라이언트 계약에 따라 결정
- 통합 테스트 및 계약 테스트에서 에러 분기 영향 확인
- API 문서(에러 코드표) 및 클라이언트 가이드 업데이트
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /* 403 FORBIDDEN */ | |
| ALADIN_API_SEARCH_FAILED(HttpStatus.FORBIDDEN, "BOOK_403_01", "알라딘 도서 검색 API 호출에 실패했습니다."), | |
| ALADIN_API_LOOKUP_FAILED(HttpStatus.FORBIDDEN, "BOOK_403_02", "알라딘 도서 상세 조회 API 호출에 실패했습니다."), | |
| ALADIN_API_RESULT_LIMIT(HttpStatus.FORBIDDEN, "BOOK_403_03", "검색 결과가 200개를 초과하여 모든 결과를 조회했습니다."); | |
| /* 403 FORBIDDEN */ | |
| ALADIN_API_SEARCH_FAILED(HttpStatus.FORBIDDEN, "BOOK_403_01", "알라딘 도서 검색 API 호출에 실패했습니다."), | |
| ALADIN_API_LOOKUP_FAILED(HttpStatus.FORBIDDEN, "BOOK_403_02", "알라딘 도서 상세 조회 API 호출에 실패했습니다."), | |
| ALADIN_API_RESULT_LIMIT( | |
| HttpStatus.FORBIDDEN, | |
| "BOOK_403_03", | |
| "검색 결과 상한(200개)을 초과하는 범위를 요청했습니다. 상위 200개까지만 조회 가능합니다." | |
| ); |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/exception/BookErrorCode.kt around
lines 12 to 15, the ALADIN_API_RESULT_LIMIT error message and HTTP status code
do not accurately reflect the actual error condition of exceeding the request
range limit. Update the error message to clearly indicate the request range
limit exceeded scenario. Replace the HTTP status code 403 FORBIDDEN with a more
appropriate code such as 400 BAD REQUEST, 422 UNPROCESSABLE ENTITY, or 416 RANGE
NOT SATISFIABLE based on internal policy and client contract. After making these
changes, verify the impact on integration and contract tests, and update the API
documentation and client guides accordingly.
| private val aladinApi: AladinApi | ||
| ) : BookQueryService { | ||
| private val log = KotlinLogging.logger {} | ||
| private val MAX_ALADIN_RESULTS = 200 // Added constant |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
상수는 companion object의 const로 승격
인스턴스별로 생성할 이유가 없고 컴파일 타임 상수로 적합합니다.
- private val MAX_ALADIN_RESULTS = 200 // Added constant
+ companion object {
+ private const val MAX_ALADIN_RESULTS: Int = 200
+ }🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt at
line 25, the MAX_ALADIN_RESULTS constant is currently a private val instance
property. Move this constant into a companion object and declare it as const val
to make it a compile-time constant and avoid creating it per instance.
Comment on lines
+54
to
+64
| val cappedTotalResults = minOf(response.totalResults ?: 0, MAX_ALADIN_RESULTS) | ||
| val currentFetchedCount = (request.start ?: 1) * (response.itemsPerPage ?: 0) | ||
|
|
||
| if (currentFetchedCount > MAX_ALADIN_RESULTS) { | ||
| throw BookException( | ||
| BookErrorCode.ALADIN_API_RESULT_LIMIT | ||
| ) | ||
| } | ||
|
|
||
| val isLastPage = currentFetchedCount >= cappedTotalResults | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
필터링 불일치로 인한 빈 페이지 문제: 점진적 개선안
ISBN 필터링으로 유효 아이템이 줄어 lastPage 계산과 실제 페이지네이션이 엇갈릴 수 있습니다. 현재 PR 범위에서는 정확한 총량을 즉시 알 수 없으므로, 다음 중 하나를 고려해 주세요.
- 간단안:
isLastPage = transformedItems.size < (response.itemsPerPage ?: transformedItems.size)조건을 추가해 마지막 페이지 근사 - 근본해결: 제안하신 비동기 집계(검색 세션 searchId 발급 → 백그라운드 수집/필터링 → 클라이언트 폴링) 모델을 별도 이슈로 도입
비동기 모델을 추진할 경우, 필요 API/캐시 키 설계 초안을 함께 드릴 수 있습니다.
다음 스크립트로 transformedItems 사용 위치를 확인해 보겠습니다:
🏁 Script executed:
#!/bin/bash
rg "transformedItems" -n -C3 apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.ktLength of output: 687
알라딘 API 필터링 후 빈 페이지 문제: 점진적 개선 제안
File: apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (54–64)
ISBN 필터링으로 유효 아이템 수가 줄어들면, isLastPage 계산이 실제 페이지네이션과 어긋나 빈 페이지가 반환될 수 있습니다. PR 범위 내에서 즉시 정확한 총건수를 알기 어렵기 때문에, 아래 두 가지 중 하나를 고려해주세요.
- 간단안: 마지막 페이지 근사치를 위해
isLastPage계산식에 필터 후 아이템 개수 비교 추가// 기존 val isLastPage = currentFetchedCount >= cappedTotalResults // 개선 val isLastPage = transformedItems.size < (response.itemsPerPage ?: transformedItems.size) || currentFetchedCount >= cappedTotalResults
- 근본해결: 검색 세션(searchId) 발급 → 백그라운드 수집·필터링 → 클라이언트 폴링 방식의 비동기 집계 모델을 별도 이슈로 도입
필요 시 API 설계 및 캐시 키 흐름 초안을 함께 검토할 수 있습니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
around lines 54 to 64, the calculation of isLastPage does not account for the
reduced number of valid items after ISBN filtering, which can cause empty pages
to be returned. To fix this, update the isLastPage logic to also check if the
size of the filtered items is less than the expected items per page, indicating
the last page, by adding a condition comparing transformedItems.size with
response.itemsPerPage or its fallback. This adjustment will provide a better
approximation of the last page without requiring immediate changes to total
count handling.
| link = response.link, | ||
| pubDate = response.pubDate, | ||
| totalResults = transformedItems.size, | ||
| totalResults = response.totalResults, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
totalResults는 capped 값으로 노출(UX/정합성 일치)
UI는 totalResults를 기반으로 페이지 수를 계산하는 경우가 많습니다. UX 정책이 상위 200개까지만 제공이라면, 응답에도 이를 반영하는 편이 혼선을 줄입니다.
- totalResults = response.totalResults,
+ totalResults = cappedTotalResults,참고: 실제 전체 건수(200 초과)를 별도로 노출해야 한다면 originalTotalResults 같은 필드를 추가하는 것도 선택지입니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| totalResults = response.totalResults, | |
| val filteredResponse = AladinSearchResponse( | |
| version = response.version, | |
| title = response.title, | |
| link = response.link, | |
| pubDate = response.pubDate, | |
| - totalResults = response.totalResults, | |
| + totalResults = cappedTotalResults, | |
| startIndex = response.startIndex, | |
| itemsPerPage = response.itemsPerPage, | |
| query = response.query, | |
| searchCategoryId = response.searchCategoryId, | |
| searchCategoryName = response.searchCategoryName, | |
| item = transformedItems, | |
| lastPage = isLastPage | |
| ) |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt at
line 70, the totalResults field should be capped to a maximum value (e.g., 200)
to align with the UX policy limiting results to the top 200 items. Modify the
assignment to totalResults to return the minimum between response.totalResults
and the cap value. Optionally, if the actual total count above the cap is
needed, add a separate field like originalTotalResults to expose the true total
without confusing the UI pagination.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
1. 알라딘 API 200개 제한 처리 로직 추가
(request.start ?: 1) * (response.itemsPerPage ?: 0)계산 결과가 200을 초과하는 경우BookException발생BookErrorCode에ALADIN_API_RESULT_LIMIT코드 추가lastPage여부를 계산하여AladinSearchResponse에 포함2.
ReadingRecordController페이징 응답 개선Page<ReadingRecordResponse>반환 구조를ReadingRecordPageResponseDTO로 변경lastPage(마지막 페이지 여부)totalResults(총 결과 개수)startIndex(현재 페이지 번호)itemsPerPage(페이지당 아이템 수)readingRecords(독서 기록 목록)3.
ReadingRecordUseCase수정Page객체를 직접 반환하지 않고,ReadingRecordPageResponse.from()메서드를 통해 변환 후 반환4.
AladinSearchResponse수정lastPage필드 추가 (마지막 페이지 여부 응답에 포함)🧪 테스트 내역
ReadingRecordController응답 구조 변경 후 API 정상 응답 여부 확인lastPage) 값 검증 완료🎨 스크린샷
(해당 사항 없음)
✅ PR 체크리스트
Summary by CodeRabbit
신규 기능
버그 수정
변경 사항