Skip to content

Conversation

@minwoo1999
Copy link
Member

@minwoo1999 minwoo1999 commented Aug 6, 2025

🔗 관련 이슈

📘 작업 유형

  • ✨ Feature (기능 추가)
  • 🐞 Bugfix (버그 수정)
  • 🔧 Refactor (코드 리팩토링)
  • ⚙️ Chore (환경 설정)
  • 📝 Docs (문서 작성 및 수정)
  • ✅ Test (기능 테스트)
  • 🎨 style (코드 스타일 수정)

📙 작업 내역

  • ISBN10 → ISBN13 변환 로직 개선 (toIsbn13)
  • BookDetailResponse, BookSearchResponse 내 ISBN 처리 로직 정리 및 예외 방어 처리 추가
  • BookSearchResponse 변환 시 유효한 ISBN만 필터링
  • IsbnValidator 유틸 클래스 도입하여 유효성 검사 적용
  • 알라딘 검색 API 결과에 ISBN 필터링 적용

🧪 테스트 내역

  • 브라우저/기기에서 동작 확인
  • 엣지 케이스 테스트 완료 (예: ISBN 누락, 잘못된 포맷)
  • 기존 기능 영향 없음

🎨 스크린샷 또는 시연 영상 (선택)

기능 설명 미리보기
(예: 검색 필터링 전/후)

✅ PR 체크리스트

  • 커밋 메시지가 명확합니다
  • PR 제목이 컨벤션에 맞습니다
  • 관련 이슈 번호를 작성했습니다
  • 기능이 정상적으로 작동합니다
  • 불필요한 코드를 제거했습니다

💬 추가 설명 or 리뷰 포인트

  • isbn13이 존재하지 않으면 isbn10을 변환하여 사용합니다.
  • 유효하지 않은 ISBN은 리스트에서 제외되며, BookSummarynull인 경우도 필터링됩니다.
  • 리뷰 시 ISBN 변환 및 유효성 검증 로직이 안정적인지 확인 부탁드립니다.

Summary by CodeRabbit

  • Refactor
    • ISBN-10 및 ISBN-13 유효성 검사 로직이 간결하고 명확하게 개선되었습니다.
    • ISBN 관련 변환 및 검증 과정이 코드 내에서 일관성 있게 처리되도록 구조가 개선되었습니다.
  • New Features
    • 도서 검색 결과에서 유효한 ISBN-13만 필터링하여 반환하도록 동작이 개선되었습니다.
  • Chores
    • 일부 데이터 클래스의 생성자 접근 제한자가 public으로 변경되어 외부에서 인스턴스 생성이 가능해졌습니다.

@coderabbitai
Copy link

coderabbitai bot commented Aug 6, 2025

Walkthrough

ISBN-10 및 ISBN-13 유효성 검사와 변환 로직이 리팩토링되었습니다. Aladin API 검색 결과에서 ISBN-13이 'K'로 시작하는 도서를 제외하는 필터링 로직이 추가되었습니다. 일부 데이터 클래스의 생성자 공개 범위가 internal에서 public으로 변경되었습니다.

Changes

Cohort / File(s) Change Summary
ISBN 유효성 검사 및 변환 리팩토링
apis/src/main/kotlin/org/yapp/apis/auth/util/IsbnConverter.kt, global-utils/src/main/kotlin/org/yapp/globalutils/validator/IsbnValidator.kt
ISBN-10/13 유효성 검사 로직을 간결하게 리팩토링하고, ISBN-10/13 정규식 패턴 분리 적용.
Aladin 검색 결과 필터링 및 DTO 생성 로직 개선
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt, apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt
검색 결과에서 ISBN-13이 'K'로 시작하는 도서를 제외하고, ISBN-13 생성 및 검증 로직을 명확하게 분리.
데이터 클래스 생성자 공개 범위 변경
infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt
AladinSearchResponseAladinSearchItem의 생성자 공개 범위를 internal에서 public으로 변경.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant AladinBookQueryService
    participant AladinAPI
    participant IsbnConverter
    participant IsbnValidator

    Client->>AladinBookQueryService: searchBooks(query)
    AladinBookQueryService->>AladinAPI: fetch book list
    AladinAPI-->>AladinBookQueryService: AladinSearchResponse
    loop For each item in response
        AladinBookQueryService->>IsbnConverter: Convert ISBN if needed
        IsbnConverter-->>AladinBookQueryService: isbn13
        AladinBookQueryService->>IsbnValidator: isValidIsbn(isbn13)
        IsbnValidator-->>AladinBookQueryService: true/false
        alt isbn13 startsWith "K"
            AladinBookQueryService-->>AladinBookQueryService: exclude item
        else
            AladinBookQueryService-->>AladinBookQueryService: include item
        end
    end
    AladinBookQueryService-->>Client: BookSearchResponse (filtered)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Assessment against linked issues

Objective Addressed Explanation
K로 시작하는 isbn에 대해서는 검색대상에서 제외 (#75)

Assessment against linked issues: Out-of-scope changes

Code Change Explanation
데이터 클래스 생성자 공개 범위 변경 (infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt) 이 변경은 이슈 #75의 목적(검색 대상 필터링)과 직접적인 관련이 없습니다. 생성자 공개 범위 변경은 외부 모듈 접근성과 관련된 변경입니다.

Suggested reviewers

  • hoonyworld

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8a83459 and 2c07d12.

📒 Files selected for processing (1)
  • apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (2 hunks)
🔇 Additional comments (3)
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (3)

13-14: 새로운 유틸리티 클래스 import가 적절히 추가되었습니다.

ISBN 변환과 검증을 위한 IsbnConverterIsbnValidator 유틸리티 클래스가 올바르게 import되어 필터링 로직에서 사용되고 있습니다.


57-70: 필터링된 결과로 새로운 응답 객체를 적절히 생성하고 있습니다.

AladinSearchResponse의 새 인스턴스를 생성할 때 다음 사항들이 올바르게 처리되었습니다:

  • 모든 원본 메타데이터 보존 (version, title, link 등)
  • totalResults를 필터링된 아이템 개수로 업데이트
  • 필터링된 아이템 리스트 사용

데이터 일관성이 보장되는 좋은 구현입니다.


71-71: 필터링된 응답을 올바르게 변환하여 반환합니다.

필터링된 AladinSearchResponseBookSearchResponse로 변환하여 반환하는 로직이 적절합니다.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BOOK-227-fix/#75

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot requested a review from move-hoon August 6, 2025 13:38
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be8137b and 8a83459.

📒 Files selected for processing (6)
  • apis/src/main/kotlin/org/yapp/apis/auth/util/IsbnConverter.kt (1 hunks)
  • apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (2 hunks)
  • apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (4 hunks)
  • apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (2 hunks)
  • global-utils/src/main/kotlin/org/yapp/globalutils/validator/IsbnValidator.kt (1 hunks)
  • infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt (2 hunks)
⏰ 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 (12)
infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinSearchResponse.kt (1)

8-8: 생성자 가시성 변경이 적절합니다.

internal 한정자 제거로 인해 다른 모듈에서 이 클래스들의 인스턴스를 생성할 수 있게 되었습니다. AladinBookQueryService에서 필터링된 응답 객체를 생성하기 위해 필요한 변경사항으로 보입니다.

Also applies to: 23-23

global-utils/src/main/kotlin/org/yapp/globalutils/validator/IsbnValidator.kt (2)

3-3: ISBN 패턴 분리로 검증 정확도가 향상되었습니다.

RegexUtils에서 가져온 별도의 ISBN-10과 ISBN-13 패턴을 사용하여 더 정확한 검증이 가능해졌습니다. 이전의 단순한 길이 기반 검증보다 개선된 접근법입니다.

Also applies to: 6-7


10-10: OR 연산을 통한 유연한 ISBN 검증 로직이 적절합니다.

ISBN-10과 ISBN-13 두 패턴 중 하나라도 매칭되면 유효한 것으로 판단하는 로직이 올바릅니다.

apis/src/main/kotlin/org/yapp/apis/auth/util/IsbnConverter.kt (1)

14-15: Kotlin 관용구를 활용한 깔끔한 검증 로직 리팩토링입니다.

takeIf를 사용하여 길이와 패턴 검증을 체인으로 연결한 것이 이전의 여러 조건문보다 간결하고 읽기 쉽습니다. 기능적 동작은 동일하게 유지되면서 코드 품질이 개선되었습니다.

apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (2)

34-36: ISBN 처리 로직의 중앙화와 방어적 프로그래밍이 잘 적용되었습니다.

ISBN-13을 우선 사용하고, 없으면 ISBN-10을 변환하는 폴백 로직이 명확합니다. 둘 다 사용할 수 없을 때 예외를 발생시키는 것도 적절합니다.


46-46: 계산된 변수 사용으로 가독성이 향상되었습니다.

인라인 로직 대신 미리 계산된 isbn13 변수를 사용하여 코드가 더 깔끔해졌습니다.

apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (3)

13-14: 필요한 유틸리티 import가 적절합니다.

ISBN 변환과 검증을 위한 유틸리티들이 올바르게 import되었습니다.


55-67: 필터링된 응답 객체 생성 로직이 적절합니다.

원본 응답의 모든 필드를 유지하면서 필터링된 아이템과 업데이트된 totalResults만 변경한 것이 올바릅니다.


69-69: 필터링된 응답 사용이 적절합니다.

변환 로직에 필터링된 응답을 전달하는 것이 맞습니다.

apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (3)

5-5: Import 추가 승인

ISBN 변환 로직을 위한 IsbnConverter import 추가가 적절합니다.


38-47: Null 안전성 개선 승인

mapNotNull 사용과 emptyList() 기본값 제공으로 null 안전성이 크게 개선되었습니다. BookSummary.of가 nullable을 반환하므로 이 처리가 적절합니다.


80-80: finalIsbn 사용 승인

계산된 finalIsbn 값을 사용하여 일관성 있는 ISBN 할당이 이루어졌습니다.

Comment on lines 72 to 78
): BookSummary? {
require(!title.isNullOrBlank()) { "Title is required" }

val finalIsbn = isbn13?.takeIf { it.isNotBlank() }
?: isbn?.takeIf { it.isNotBlank() }?.let { IsbnConverter.toIsbn13(it) }
?: return null

Copy link

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

ISBN 처리 로직 개선 승인 및 K로 시작하는 ISBN 필터링 누락

ISBN 처리 로직이 크게 개선되었습니다:

  • nullable 반환 타입으로 변경하여 유효하지 않은 경우 적절히 처리
  • ISBN-13 우선, 그 다음 ISBN-10에서 변환하는 로직이 합리적

하지만 PR 목적 중 하나인 'K로 시작하는 ISBN 제외' 로직이 누락되었습니다.

다음 스크립트로 K로 시작하는 ISBN 필터링 로직이 구현되어 있는지 확인해보겠습니다:

또한 IsbnConverter.toIsbn13() 메서드의 오류 처리 방식을 확인해보겠습니다:


🏁 Script executed:

#!/bin/bash
# K로 시작하는 ISBN 필터링 로직 확인
rg -A 5 -B 5 "startsWith.*K|K.*startsWith" --type kotlin

Length of output: 58


🏁 Script executed:

#!/bin/bash
# IsbnConverter.toIsbn13 메서드 구현 확인
ast-grep --pattern $'fun toIsbn13($_) {
  $$$
}'

Length of output: 49


다음 스크립트들로 IsbnConverter 구현 위치와 toIsbn13 메서드 시그니처 및 본문을 확인해주세요:


🏁 Script executed:

#!/bin/bash
# 1. IsbnConverter 클래스/객체 파일 경로 찾기
rg -l "IsbnConverter" --type kotlin

Length of output: 321


🏁 Script executed:

#!/bin/bash
# 2. toIsbn13 메서드 정의 위치 및 시그니처 확인
rg -n "fun toIsbn13" --type kotlin

Length of output: 146


🏁 Script executed:

#!/bin/bash
# 3. IsbnConverter 파일 내용(최초 200줄) 출력
file=$(rg -l "IsbnConverter" --type kotlin | head -n1)
if [ -n "$file" ]; then
  sed -n '1,200p' "$file"
fi

Length of output: 4192


다음 스크립트로 IsbnConverter.kt 파일의 toIsbn13 구현을 확인해주세요:


🏁 Script executed:

#!/bin/bash
# IsbnConverter.kt 전체 내용(최초 200줄) 출력
sed -n '1,200p' apis/src/main/kotlin/org/yapp/apis/auth/util/IsbnConverter.kt

Length of output: 1001


K로 시작하는 ISBN 필터링 로직 추가 필요

BookSearchResponse.ktBookSummary.of(Lines 72–78)에서 ISBN 변환 및 null 처리 로직은 적절히 구현되었으나, “K로 시작하는 ISBN 제외” 요구사항이 반영되어 있지 않습니다.
또한 IsbnConverter.toIsbn13()는 유효하지 않은 ISBN10 입력 시 null을 반환하므로 예외 발생 우려는 없습니다.

수정 제안:

  • finalIsbn 결정 직후에 K로 시작하는지 확인하고, 해당 경우 null 반환
  • 또는 takeIf 체인에 !it.startsWith("K") 조건을 추가

예시 diff:

             val finalIsbn = isbn13?.takeIf { it.isNotBlank() }
                 ?: isbn?.takeIf { it.isNotBlank() }?.let { IsbnConverter.toIsbn13(it) }
                 ?: return null
+            // K로 시작하는 ISBN 제외
+            if (finalIsbn.startsWith("K")) return null
  • 파일: apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
  • 위치: BookSummary.of 내부, finalIsbn 할당 직후
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
around lines 72 to 78, the current logic for determining finalIsbn does not
exclude ISBNs starting with "K" as required. To fix this, after computing
finalIsbn, add a check to return null if finalIsbn starts with "K".
Alternatively, include a condition in the takeIf chain to ensure the ISBN does
not start with "K". This will properly filter out unwanted ISBNs while
preserving existing null handling.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 6, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@minwoo1999 minwoo1999 merged commit 6a15018 into develop Aug 6, 2025
3 of 4 checks passed
@move-hoon move-hoon deleted the BOOK-227-fix/#75 branch August 12, 2025 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BOOK-227/fix] K로 시작하는 isbn에 대해서는 검색대상에서 제외시킵니다.

2 participants