Skip to content

fix: 도서 검색 DTO 변경사항에 따른 LastPage 로직 수정 및 빈 ISBN 대응 로직 추가 #135

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.

Sign up for GitHub

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

Merged
seoyoon513 merged 4 commits into from
Aug 10, 2025
Merged

fix: 도서 검색 DTO 변경사항에 따른 LastPage 로직 수정 및 빈 ISBN 대응 로직 추가 #135

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8090ee7
[BOOK-250] fix: DTO 변경사항 적용 및 lastPage 판별 로직 수정
seoyoon513 Aug 10, 2025
4344267
[BOOK-250] refactor: 빈 isbn이 대응 로직 추가
seoyoon513 Aug 10, 2025
f2d3b71
[BOOK-250] chore: 버전 코드 변경
seoyoon513 Aug 10, 2025
a80f2f5
[BOOK-250] chore: 버전 코드 2 -> 3으로 변경
seoyoon513 Aug 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build-logic/src/main/kotlin/com/ninecraft/booket/convention/ApplicationConstants.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ internal object ApplicationConstants {
const val MIN_SDK = 28
const val TARGET_SDK = 35
const val COMPILE_SDK = 35
const val VERSION_CODE = 1
const val VERSION_CODE = 2
const val VERSION_NAME = "1.0.0"
const val JAVA_VERSION_INT = 17
val javaVersion = JavaVersion.VERSION_17
Expand Down
4 changes: 3 additions & 1 deletion core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,14 @@ internal fun BookSearchResponse.toModel(): BookSearchModel {
return BookSearchModel(
version = version,
title = title,
link = link,
pubDate = pubDate,
totalResults = totalResults,
startIndex = startIndex,
itemsPerPage = itemsPerPage,
query = query,
searchCategoryId = searchCategoryId,
searchCategoryName = searchCategoryName,
lastPage = lastPage,
books = books.map { it.toModel() },
)
}
Expand All @@ -73,7 +73,9 @@ internal fun BookSummary.toModel(): BookSummaryModel {
author = author,
publisher = publisher,
coverImageUrl = coverImageUrl,
link = link,
userBookStatus = userBookStatus,
key = "$title-$isbn13",
)
}

Expand Down
4 changes: 3 additions & 1 deletion core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import androidx.compose.runtime.Stable
data class BookSearchModel(
val version: String = "",
val title: String = "",
val link: String = "",
val pubDate: String = "",
val totalResults: Int = 0,
val startIndex: Int = 0,
val itemsPerPage: Int = 0,
val query: String = "",
val searchCategoryId: Int = 0,
val searchCategoryName: String = "",
val lastPage: Boolean = false,
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 10, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

마지막 페이지 플래그 추가 LGTM — 네이밍(nit)과 하위호환 확인 요청

  • 서버 DTO 변경 반영 OK.
  • Kotlin 관례상 Boolean은 isLastPage 네이밍도 고려 가능하나, API와의 정합성이 더 중요하다면 현 상태 유지도 무방합니다. (nit)
  • 구버전/필드 미존재 응답 대비로, 매퍼에서 lastPage 누락 시 fallback 계산(예: books.size < itemsPerPage)을 두었는지 확인 부탁드립니다.
  • Presenter/UI가 개수 기반 판별 로직에서 lastPage 기반으로 전환되었는지 잔존 로직 점검 권장.

다음 스크립트로 매핑 및 사용처를 빠르게 확인할 수 있습니다:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== lastPage 매핑 위치 탐색 =="
fd -p "ResponseToModel.kt" | xargs -I{} rg -n "BookSearchModel\\(" "{}" -n -A 10 || true

echo
echo "== lastPage 사용처 탐색 =="
rg -n "lastPage" -A 3 || true

echo
echo "== (레거시) 개수 기반 마지막 페이지 판별 잔존 여부 =="
rg -n "books\\.size\\s*[<!=]" -A 2 || true

Length of output: 2808

마지막 페이지 플래그 누락 시 안전 대체 로직 추가 필요

  • core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt 에서
    lastPage를 단순 매핑만 하고 있어, JSON 응답에 해당 필드가 없으면 파싱 오류가 발생할 수 있습니다.
    → lastPage를 nullable로 변경하거나, 매퍼에서 누락 시 books.size < itemsPerPage 기반으로 fallback 처리 로직을 추가해야 합니다.
  • feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt (82행) 에서
    isLastPage = result.lastPage 로 전환되어 있으며, 개수(size) 기반 판별 로직은 더 이상 남아 있지 않습니다.

예시 수정안 (ResponseToModel.kt):

 fun BookSearchResponse.toModel(): BookSearchModel = BookSearchModel(
     version = version,
     title = title,
     pubDate = pubDate,
     totalResults = totalResults,
     startIndex = startIndex,
     itemsPerPage = itemsPerPage,
     query = query,
     searchCategoryId = searchCategoryId,
     searchCategoryName = searchCategoryName,
-    lastPage = lastPage,
+    lastPage = lastPage ?: (books.size < itemsPerPage),
     books = books.map { it.toModel() },
 )

위와 같이 매퍼 레벨에서 하위 호환과 필드 미존재 시에도 정상 동작을 보장하도록 수정해주세요.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt
at line 16, the lastPage property is currently non-nullable and defaults to
false, which can cause parsing errors if the JSON response lacks this field. To
fix this, make lastPage nullable or update the mapper in
core/data/impl/src/main/kotlin/com/ninecraft/booket/core/data/impl/mapper/ResponseToModel.kt
to provide a fallback by checking if books.size is less than itemsPerPage when
lastPage is missing. This ensures backward compatibility and prevents errors
when the field is absent.

val books: List<BookSummaryModel> = emptyList(),
)

Expand All @@ -24,5 +24,7 @@ data class BookSummaryModel(
val author: String = "",
val publisher: String = "",
val coverImageUrl: String = "",
val link: String = "",
val userBookStatus: String = "",
val key: String = "",
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 10, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

key의 고유성/안정성 보장 필요(빈 ISBN 시 충돌 가능성)

  • UI 무한 스크롤의 item key는 “유일”하고 “재조합에도 불변”해야 합니다.
  • 현재 매퍼에서 title + isbn13 조합으로 생성한다면(요약에 따르면), isbn13이 빈 경우 동명서·동저자 도서에서 충돌 가능성이 있습니다.
  • 제안: 매퍼에서 아래 우선순위로 생성
    1. isbn13이 존재하면 isbn13
    2. 없으면 link(상세 URL)
    3. 그마저 없으면 title|author|publisher|coverImageUrl 조합 문자열(또는 해시)

권장 주석 추가 예시:

-    val key: String = "",
+    // UI 리스트 key(고유/불변). isbn13 우선, 없으면 link, 그마저 없으면 대표 특성 조합으로 생성.
+    val key: String = "",

매퍼 예시(외부 파일 참고용):

// core/data/impl/.../ResponseToModel.kt
fun BookSummary.toModel(): BookSummaryModel = BookSummaryModel(
  isbn13 = isbn13.orEmpty(),
  title = title.orEmpty(),
  author = author.orEmpty(),
  publisher = publisher.orEmpty(),
  coverImageUrl = coverImageUrl.orEmpty(),
  link = link.orEmpty(),
  userBookStatus = userBookStatus.orEmpty(),
  key = when {
    !isbn13.isNullOrBlank() -> isbn13
    !link.isNullOrBlank() -> link
    else -> listOf(title, author, publisher, coverImageUrl).joinToString("|") { it.orEmpty() }
  }
)

빠른 확인 스크립트:

필요하시면 테스트 케이스(빈 ISBN/중복 title 시 key 충돌 방지)도 함께 작성해 드리겠습니다.

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== key 생성 로직 확인(ResponseToModel) =="
fd -p "ResponseToModel.kt" | xargs -I{} rg -n "key\\s*=" "{}" -n -A 3 -B 3 || true

echo
echo "== Compose 리스트에서 key 사용 확인 =="
rg -n "key\\s*=\\s*\\{\\s*it\\.key\\s*\\}" -A 1 -B 1 || true

Length of output: 471

UI 리스트 key 고유성/불변성 보장 필요: key 생성 로직 및 Compose key 지정 검토

현재 확인 결과

  • ResponseToModel.kt(75–79행)에서 
    key = "$title-$isbn13",
    로직을 사용하고 있어 isbn13이 빈 문자열일 경우 "$title-" 형태가 되어 동명·동저자 도서 간 충돌 가능성이 있습니다.
  • Compose 리스트(items) 호출부에서는 key = { it.key }를 지정하지 않아 기본 인덱스 기반 key 사용 중으로 보입니다.

◆ 제안 사항

  1. Mapper(key) 생성 로직 개선
 fun BookSummary.toModel(): BookSummaryModel = BookSummaryModel(
   // …기존 필드…
-  key = "$title-$isbn13",
+  key = when {
+    !isbn13.isNullOrBlank() -> isbn13
+    !link.isNullOrBlank()  -> link
+    else -> listOf(title, author, publisher, coverImageUrl)
+              .joinToString("|") { it.orEmpty() }
+  }
 )
  1. BookSearchModel.kt에 주석 추가
-    val key: String = "",
+    // UI 리스트 key(고유/불변): isbn13 우선, 없으면 link, 그마저 없으면 주요 특성 결합
+    val key: String = "",
  1. Compose LazyColumn 등에서 key 파라미터 지정
- items(bookList) { item -> … }
+ items(bookList, key = { it.key }) { item -> … }

테스트 케이스(빈 ISBN·동명 도서 충돌 방지) 추가 작성도 권장드립니다.

🤖 Prompt for AI Agents 
In core/model/src/main/kotlin/com/ninecraft/booket/core/model/BookSearchModel.kt
at line 29, the current key generation logic uses "$title-$isbn13" which can
cause key collisions if isbn13 is empty, resulting in non-unique keys for books
with the same title. To fix this, improve the key generation logic to ensure
uniqueness and immutability, for example by including a fallback or a unique
identifier when isbn13 is empty. Add a comment in BookSearchModel.kt explaining
the key's purpose and uniqueness requirements. Also, update the Compose
LazyColumn or similar UI components to explicitly specify the key parameter
using the improved key property to avoid default index-based keys. Finally, add
test cases to cover scenarios with empty ISBNs and duplicate titles to prevent
collisions.

)
6 changes: 4 additions & 2 deletions .../network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ data class BookSearchResponse(
val version: String,
@SerialName("title")
val title: String,
@SerialName("link")
val link: String,
@SerialName("pubDate")
val pubDate: String,
@SerialName("totalResults")
Expand All @@ -25,6 +23,8 @@ data class BookSearchResponse(
val searchCategoryId: Int,
@SerialName("searchCategoryName")
val searchCategoryName: String,
@SerialName("lastPage")
val lastPage: Boolean,
Comment on lines +26 to +27
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 10, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

❓ Verification inconclusive

lastPage 추가 시 백워드 호환성 고려(기본값 지정 권장)

kotlinx.serialization 사용 시, 구버전 서버 응답에 lastPage가 없으면 디코딩 실패할 수 있습니다. 기본값을 두면 안전합니다.

서버/클라이언트 배포 타이밍이 완전히 동기화되어 있다면 스킵 가능하지만, 그렇지 않다면 아래처럼 기본값을 권장합니다.

-    @SerialName("lastPage")
-    val lastPage: Boolean,
+    @SerialName("lastPage")
+    val lastPage: Boolean = false,

lastPage 및 link 필드에 기본값 추가(백워드 호환성 확보)

kotlinx.serialization 사용 시, 서버 응답에 해당 키가 없으면 MissingFieldException이 발생합니다. 서버·클라이언트 배포 시점이 완전히 동기화되지 않을 경우를 대비해 다음과 같이 기본값을 지정하세요:

  • 대상 파일:
    core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt
  • 적용 위치: @SerialName("lastPage") 및 @SerialName("link") 선언부
@@ core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt
-    @SerialName("lastPage")
-    val lastPage: Boolean,
+    @SerialName("lastPage")
+    val lastPage: Boolean = false,

-    @SerialName("link")
-    val link: String,
+    @SerialName("link")
+    val link: String = "",

이렇게 설정하면 구버전 서버 응답에서도 안전하게 디코딩할 수 있습니다.

🤖 Prompt for AI Agents 
In
core/network/src/main/kotlin/com/ninecraft/booket/core/network/response/BookSearchResponse.kt
around lines 26 to 27, add default values to the lastPage and link fields to
prevent MissingFieldException during deserialization when the server response
lacks these keys. Modify the declarations by assigning appropriate default
values (e.g., false for lastPage and an empty string or null for link) to ensure
backward compatibility with older server versions.

@SerialName("books")
val books: List<BookSummary>,
)
Expand All @@ -41,6 +41,8 @@ data class BookSummary(
val publisher: String,
@SerialName("coverImageUrl")
val coverImageUrl: String,
@SerialName("link")
val link: String,
@SerialName("userBookStatus")
val userBookStatus: String,
)
10 changes: 7 additions & 3 deletions ...re/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class BookSearchPresenter @AssistedInject constructor(
private val repository: BookRepository,
) : Presenter<BookSearchUiState> {
companion object {
private const val PAGE_SIZE = 20
private const val START_INDEX = 1
}

Expand Down Expand Up @@ -80,7 +79,7 @@ class BookSearchPresenter @AssistedInject constructor(
}

currentStartIndex = startIndex
isLastPage = result.books.size < PAGE_SIZE
isLastPage = result.lastPage

if (startIndex == START_INDEX) {
uiState = UiState.Success
Expand Down Expand Up @@ -185,7 +184,12 @@ class BookSearchPresenter @AssistedInject constructor(

is BookSearchUiEvent.OnBookClick -> {
selectedBookIsbn = event.isbn13
isBookRegisterBottomSheetVisible = true

if (selectedBookIsbn.isEmpty()) {
sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다")
} else {
isBookRegisterBottomSheetVisible = true
}
Comment on lines 186 to +192
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 10, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

빈 ISBN 처리 시 상태 오염 방지 및 견고성 개선(isEmpty→isBlank, 할당 위치 이동)

빈 ISBN이면 하단시트를 열지 않으므로 selectedBookIsbn을 미리 세팅하지 않는 편이 상태 일관성에 유리합니다. 공백 포함 케이스도 방지하려면 isBlank 사용을 권장합니다.

-                    selectedBookIsbn = event.isbn13
-
-                    if (selectedBookIsbn.isEmpty()) {
+                    if (event.isbn13.isBlank()) {
                         sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다")
                     } else {
+                        selectedBookIsbn = event.isbn13
                         isBookRegisterBottomSheetVisible = true
                     }
📝 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
selectedBookIsbn = event.isbn13
isBookRegisterBottomSheetVisible = true
if (selectedBookIsbn.isEmpty()) {
sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다")
} else {
isBookRegisterBottomSheetVisible = true
}
if (event.isbn13.isBlank()) {
sideEffect = BookSearchSideEffect.ShowToast("isbn이 없는 도서는 등록할 수 없습니다")
} else {
selectedBookIsbn = event.isbn13
isBookRegisterBottomSheetVisible = true
}
🤖 Prompt for AI Agents 
In
feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchPresenter.kt
around lines 186 to 192, avoid setting selectedBookIsbn before validating it
because if the ISBN is blank, the bottom sheet should not open and the state
could become inconsistent. Move the assignment of selectedBookIsbn inside the
else block after confirming the ISBN is not blank, and replace the isEmpty check
with isBlank to handle cases with whitespace-only strings.

}

is BookSearchUiEvent.OnBookRegisterBottomSheetDismiss -> {
Expand Down
2 changes: 1 addition & 1 deletion feature/search/src/main/kotlin/com/ninecraft/booket/feature/search/book/BookSearchUi.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ internal fun SearchContent(
) {
items(
count = state.books.size,
key = { index -> state.books[index].isbn13 },
key = { index -> state.books[index].key },
) { index ->
Column {
BookItem(
Expand Down
Loading