feat: 알라딘 외부 API 도서검색 및 상세 조회 기능 #26
Conversation
📝 Walkthrough""" WalkthroughBook 엔티티를 도메인 및 인프라 계층에 설계하고, 알라딘 외부 API와 연동하는 전체적인 기능이 추가되었습니다. REST 기반으로 알라딘 API를 호출하는 클라이언트, 서비스, DTO, 예외 처리, 보안 설정, 그리고 관련 의존성 및 설정 파일이 일괄적으로 반영되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BookController
participant BookUseCase
participant AladinBookQueryService
participant AladinApiHelper
participant AladinApi
participant AladinRestClient
participant AladinAPI (External)
Client->>BookController: GET /api/v1/books/search (BookSearchRequest)
BookController->>BookUseCase: searchBooks(request)
BookUseCase->>AladinBookQueryService: searchBooks(request)
AladinBookQueryService->>AladinApiHelper: searchBooks(AladinBookSearchRequest)
AladinApiHelper->>AladinApi: searchBooks(request)
AladinApi->>AladinRestClient: itemSearch(ttbKey, params)
AladinRestClient->>AladinAPI (External): HTTP GET /ItemSearch.aspx
AladinAPI (External)-->>AladinRestClient: SearchResponse
AladinRestClient-->>AladinApi: AladinSearchResponse
AladinApi-->>AladinApiHelper: Result<AladinSearchResponse>
AladinApiHelper-->>AladinBookQueryService: AladinSearchResponse
AladinBookQueryService-->>BookUseCase: BookSearchResponse
BookUseCase-->>BookController: BookSearchResponse
BookController-->>Client: ResponseEntity<BookSearchResponse>
sequenceDiagram
participant Client
participant BookController
participant BookUseCase
participant AladinBookQueryService
participant AladinApiHelper
participant AladinApi
participant AladinRestClient
participant AladinAPI (External)
Client->>BookController: GET /api/v1/books/detail (BookDetailRequest)
BookController->>BookUseCase: getBookDetail(request)
BookUseCase->>AladinBookQueryService: lookupBook(request)
AladinBookQueryService->>AladinApiHelper: lookupBook(AladinBookLookupRequest)
AladinApiHelper->>AladinApi: lookupBook(request)
AladinApi->>AladinRestClient: itemLookUp(ttbKey, params)
AladinRestClient->>AladinAPI (External): HTTP GET /ItemLookUp.aspx
AladinAPI (External)-->>AladinRestClient: DetailResponse
AladinRestClient-->>AladinApi: AladinBookDetailResponse
AladinApi-->>AladinApiHelper: Result<AladinBookDetailResponse>
AladinApiHelper-->>AladinBookQueryService: AladinBookDetailResponse
AladinBookQueryService-->>BookUseCase: BookDetailResponse
BookUseCase-->>BookController: BookDetailResponse
BookController-->>Client: ResponseEntity<BookDetailResponse>
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes해당 변경사항 중 요구사항(#18)과 무관한 기능적 코드 변경은 발견되지 않았습니다. ✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (25)
apis/build.gradle.kts(0 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/helper/AladinApiHelper.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/BookService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(1 hunks)buildSrc/src/main/kotlin/Dependencies.kt(1 hunks)domain/src/main/kotlin/org/yapp/domain/book/Book.kt(1 hunks)gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt(2 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt(1 hunks)infra/build.gradle.kts(1 hunks)infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/external/oauth/FeignConfig.kt(0 hunks)infra/src/main/kotlin/org/yapp/infra/config/external/oauth/RestClientConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoApi.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoFeignApi.kt(0 hunks)infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoRestClient.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/redis/repository/impl/RefreshTokenRepositoryImpl.kt(1 hunks)
💤 Files with no reviewable changes (3)
- apis/build.gradle.kts
- infra/src/main/kotlin/org/yapp/infra/config/external/oauth/FeignConfig.kt
- infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoFeignApi.kt
🧰 Additional context used
🧬 Code Graph Analysis (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (1)
from(33-57)
🪛 detekt (1.23.8)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
[warning] 1-71: The file does not contain a package declaration.
(detekt.potential-bugs.MissingPackageDeclaration)
🔇 Additional comments (28)
infra/src/main/kotlin/org/yapp/infra/external/redis/repository/impl/RefreshTokenRepositoryImpl.kt (1)
7-7:JpaRefreshTokenRepository도입은 좋으나, 동일 타입의 Bean 중복 여부를 확인해 주세요.다른 모듈/패키지에 같은 시그니처의
JpaRefreshTokenRepositoryBean 이 이미 등록되어 있다면, Spring 컨텍스트에서 충돌이 발생할 수 있습니다.@Qualifier지정 혹은 Bean 스캔 범위 확인을 권장드립니다.gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt (1)
36-36: 도서 API 엔드포인트 공개 접근 허용 설정이 적절합니다.도서 검색 및 상세 조회 기능이 공개 API로 제공되는 것은 일반적인 패턴이며, PR 목적에 부합합니다. 인증이 필요한 도서 관련 기능이 향후 추가될 경우 더 세분화된 경로 패턴을 고려해보시기 바랍니다.
infra/build.gradle.kts (1)
9-10: Feign에서 RestClient로의 마이그레이션이 일관성 있게 진행되었습니다.Apache HttpComponents 기반의 HTTP 클라이언트 라이브러리로 변경하는 것은 적절한 선택입니다. 더 낮은 수준의 HTTP 클라이언트 제어가 가능하고, Spring의 RestClient와 잘 통합됩니다.
infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt (1)
3-3: 설정 클래스 참조 업데이트가 마이그레이션과 일치합니다.
FeignConfig에서RestClientConfig로의 변경이 일관성 있게 적용되었습니다. 설정 그룹 관리 방식이 그대로 유지되어 아키텍처 일관성이 보장됩니다.Also applies to: 14-14
infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoApi.kt (2)
7-9: KakaoApi 클래스의 RestClient 마이그레이션이 올바르게 구현되었습니다.생성자 가시성 변경과 의존성 주입 업데이트가 적절합니다.
KakaoRestClient로의 전환이 기존 기능을 유지하면서 깔끔하게 처리되었습니다.
16-16: 메서드 호출 업데이트가 적절합니다.
kakaoRestClient.getUserInfo호출로 변경되었으며, 인증 헤더 형식과 응답 래핑 방식이 기존과 동일하게 유지되어 호환성이 보장됩니다.global-utils/src/main/kotlin/org/yapp/globalutils/exception/GlobalExceptionHandler.kt (1)
62-78: IllegalArgumentException 예외 처리기가 일관성 있게 구현되었습니다.기존 예외 처리 패턴과 일치하는 구현으로, 로깅, 에러 응답 생성, HTTP 상태 코드 설정이 적절합니다. 새로운 도서 API의 매개변수 검증 오류를 효과적으로 처리할 수 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (1)
33-57: 팩토리 메서드 구현이 우수합니다외부 API 응답을 안전하게 변환하는 팩토리 메서드 패턴이 잘 구현되었습니다. null 안전성과 기본값 처리도 적절합니다.
apis/src/main/kotlin/org/yapp/apis/book/helper/AladinApiHelper.kt (2)
16-29: Result 타입을 이용한 에러 핸들링이 우수합니다Kotlin의 Result 타입을 사용한 에러 핸들링 패턴이 잘 구현되었습니다. 로깅과 예외 변환도 적절히 처리되어 있습니다.
31-44: 일관된 에러 핸들링 패턴 적용
lookupBook메서드도searchBooks와 동일한 에러 핸들링 패턴을 사용하여 일관성이 유지되었습니다.apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt (2)
24-34: toAladinParams() 메서드가 잘 구현되어 있습니다.null 체크를 통해 실제 값이 있는 파라미터만 맵에 추가하는 로직이 깔끔합니다. 알라딘 API의 파라미터 이름과 매핑도 적절히 처리되어 있습니다.
9-19: private constructor 사용 시 companion object factory method가 필요합니다.private constructor를 사용했지만 인스턴스 생성을 위한 companion object factory method가 없어 외부에서 이 클래스를 인스턴스화할 수 없습니다.
다음과 같이 수정하여 factory method를 추가하거나 constructor를 public으로 변경하세요:
-data class BookSearchRequest private constructor( +data class BookSearchRequest( @field:NotBlank(message = "검색어는 필수입니다.") val query: String, val queryType: String? = null, val searchTarget: String? = null, val maxResults: Int? = null, val start: Int? = null, val sort: String? = null, val cover: String? = null, val categoryId: Int? = null ) {또는 companion object를 추가하여 factory method를 제공하세요:
) { + companion object { + fun create( + query: String, + queryType: String? = null, + searchTarget: String? = null, + maxResults: Int? = null, + start: Int? = null, + sort: String? = null, + cover: String? = null, + categoryId: Int? = null + ): BookSearchRequest { + return BookSearchRequest( + query = query, + queryType = queryType, + searchTarget = searchTarget, + maxResults = maxResults, + start = start, + sort = sort, + cover = cover, + categoryId = categoryId + ) + } + } + /** * 알라딘 API 호출을 위한 쿼리 파라미터 맵으로 변환합니다.⛔ Skipped due to learnings
Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#12 File: apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt:12-18 Timestamp: 2025-06-24T08:46:22.341Z Learning: Request DTO에서 private constructor를 사용하는 것은 직접 인스턴스 생성을 방지하고 HTTP request 바인딩을 통해서만 객체가 생성되도록 하는 의도적인 설계 패턴이다. Spring의 Jackson은 리플렉션을 통해 private constructor도 호출할 수 있어서 HTTP request 바인딩에는 문제가 없다.apis/src/main/kotlin/org/yapp/apis/book/service/BookService.kt (2)
18-22: 서비스 레이어 구조가 잘 설계되어 있습니다.로깅과 함께 helper에 위임하는 구조가 적절하며, 외부 API 응답을 내부 DTO로 변환하는 책임이 명확합니다.
24-28: 상세 조회 메서드가 잘 구현되어 있습니다.optResult 파라미터를 포함한 로깅과 API 호출 로직이 적절합니다.
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (2)
11-15: UseCase 레이어 구조가 적절합니다.
@UseCase애노테이션과@Transactional(readOnly = true)사용이 적절하며, 읽기 전용 트랜잭션 설정이 외부 API 호출 용도에 맞습니다.
16-18: searchBooks 메서드가 잘 구현되어 있습니다.request 객체에서 쿼리와 파라미터를 추출하여 service layer에 위임하는 구조가 깔끔합니다.
domain/src/main/kotlin/org/yapp/domain/book/Book.kt (4)
8-19: 도메인 모델 설계가 훌륭합니다.private constructor와 factory method 패턴, 그리고 audit 필드들이 적절히 구현되어 있습니다. 소프트 삭제 필드도 올바르게 추가되었습니다.
20-28: restore() 메서드가 잘 구현되어 있습니다.삭제된 상태 검증과 함께 적절한 상태 변경 로직이 포함되어 있으며,
require()사용으로 사전 조건 검증이 명확합니다.
30-53: create() factory method가 적절합니다.현재 시간으로 audit 필드를 초기화하는 로직이 올바르게 구현되어 있습니다.
55-80: reconstruct() factory method가 적절합니다.영속성 계층에서 도메인 객체를 재구성할 때 사용할 수 있는 메서드가 잘 구현되어 있습니다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt (3)
20-30: addCommonQueryParams() 확장 함수가 잘 구현되어 있습니다.OptResult 리스트를 콤마로 구분된 문자열로 변환하는 로직이 적절하며, 공통 파라미터 추가 로직이 깔끔합니다.
32-48: itemSearch() 메서드 구현이 적절합니다.URI 빌더를 사용한 파라미터 구성과 예외 처리가 잘 되어 있습니다.
50-68: itemLookUp() 메서드 구현이 적절합니다.itemSearch와 일관된 패턴으로 구현되어 있으며, 적절한 예외 처리가 포함되어 있습니다.
infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt (1)
19-20: ISBN 길이 제약 재검토 필요ISBN은 10자리 또는 13자리일 수 있는데, 컬럼 길이가 13으로 고정되어 있습니다. ISBN-10도 지원해야 한다면 길이를 조정해야 합니다.
ISBN-10과 ISBN-13을 모두 지원할 계획인지 확인이 필요합니다. ISBN-10만 저장하는 경우라면 길이를 10으로, 둘 다 지원한다면 13으로 유지하되 주석을 명확히 하는 것이 좋습니다.
- @Column(length = 13, updatable = false, nullable = false) // ISBN/ISBN13 length + @Column(length = 13, updatable = false, nullable = false) // Supports both ISBN-10 (stored as ISBN-13) and ISBN-13apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt (1)
29-70: 상세한 API 문서화 작성 👍Swagger 문서화가 매우 상세하고 실용적인 예제들을 포함하고 있어 API 사용자에게 큰 도움이 될 것입니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (3)
5-17: 메인 데이터 클래스 구조가 잘 설계되었습니다.private 생성자를 사용하여 팩토리 메서드를 통한 객체 생성을 강제하는 것이 좋은 설계입니다. 필드명도 명확하고 적절한 타입을 사용하고 있습니다.
18-35: 팩토리 메서드 구현이 적절합니다.외부 API 응답을 내부 DTO로 변환하는 로직이 명확하고, null 처리도 적절히 되어 있습니다.
mapNotNull을 사용하여 유효하지 않은 책 항목을 필터링하는 것도 좋은 접근입니다.
59-67: 도메인 객체를 DTO로 변환하는 로직이 명확합니다.도메인 객체에서 필요한 필드만 선택적으로 추출하여 DTO를 생성하는 것이 적절합니다. 필드 매핑도 정확합니다.
| @@ -1,9 +1,10 @@ | |||
| package org.yapp.infra.external.redis.repository | |||
| package org.yapp.infra.external.redis.repository.impl | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
패키지 네이밍이 구현 기술과 맞지 않아 혼동을 유발합니다.
redis.repository.impl 하위에 JPA 기반 구현체가 위치해 있습니다. 실제 저장소가 RDB(JPA)라면 rdb 혹은 jpa 같은 네이밍으로 분리하거나, 최소한 redis 대신 범용적인 persistence 하위로 이동하는 편이 가독성과 유지보수에 유리합니다.
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/external/redis/repository/impl/RefreshTokenRepositoryImpl.kt
at line 1, the package naming is misleading because it suggests a Redis
implementation while the actual repository uses JPA with an RDB. To fix this,
rename or move the implementation to a package that reflects the actual
persistence technology, such as changing 'redis' to 'rdb' or 'jpa', or placing
it under a more generic 'persistence' package for better clarity and
maintainability.
There was a problem hiding this comment.
@minwoo1999 이 부분은 user랑 통일해보면 좋을 것 같아요!
user는 레포 구현체도 같은 패키지로 두어서 말씀드립니다!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| import org.yapp.domain.token.RefreshTokenRepository | ||
| import org.yapp.infra.external.redis.entity.RefreshTokenEntity | ||
| import org.yapp.infra.external.redis.repository.JpaRefreshTokenRepository | ||
| import java.util.* |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
와일드카드 임포트는 필요하지 않습니다.
java.util.* 대신 실제로 사용하는 UUID만 임포트하면 컴파일 타임 의존성과 코드 가독성이 향상됩니다.
-import java.util.*
+import java.util.UUID📝 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.
| import java.util.* | |
| -import java.util.* | |
| +import java.util.UUID |
🤖 Prompt for AI Agents
In
infra/src/main/kotlin/org/yapp/infra/external/redis/repository/impl/RefreshTokenRepositoryImpl.kt
at line 8, replace the wildcard import 'import java.util.*' with an explicit
import of only 'java.util.UUID' to improve compile-time dependency clarity and
code readability.
There was a problem hiding this comment.
🧩 Analysis chain
Based on the blog post you shared, I can see it discusses IntelliJ IDEA's import optimization settings!
@hoonyworld 좋은 정보 감사합니다! 해당 블로그 포스트에서 설명하는 IntelliJ IDEA의 import 최적화 설정들이 정말 유용하네요. 특히 "Use single class import" 옵션을 활성화하면 IDE가 자동으로 와일드카드 임포트를 명시적 임포트로 변환해주는 기능이 좋습니다.
IDE 설정을 통해 이런 코드 스타일을 자동으로 관리할 수 있다면 팀 전체적으로 일관성을 유지하기 더 쉬워질 것 같습니다. 제안해주신 설정을 프로젝트에 적용해보시는 것도 좋을 것 같아요!
🌐 Web query:
site:dev-kani.tistory.com/39 kotlin import wildcard explicit import optimization
💡 Result:
In Kotlin, wildcard imports (e.g., import foo.bar.*) allow you to import all accessible declarations within a package or class at once, rather than listing each one explicitly[6]. While Kotlin permits both wildcard and explicit imports, the topic of import optimization—especially around wildcard vs. explicit imports—has practical and tooling considerations:
Wildcard Import Behavior & IDE Support
- By default, IntelliJ IDEA (and Android Studio) applies certain rules for converting multiple explicit imports from the same package into a single wildcard import (such as when more than a configured number of classes are imported from the same package)[3].
- You can customize these settings via
Settings > Editor > Code Style > Kotlin > Imports:- Set “Name count to use star import” and “Name count to use star import for members” to very high values (e.g.,
2147483647) to always use explicit imports and prevent wildcard imports from being generated automatically[1]. - These
.editorconfigentries ensure wildcard imports are never used, and explicit imports are always preserved in Kotlin files[1][3].
- Set “Name count to use star import” and “Name count to use star import for members” to very high values (e.g.,
- The “Optimize Imports” feature in IntelliJ (shortcut:
Ctrl+Alt+O) will automatically apply these preferences—removing unused imports, sorting them, and, unless prevented as shown above, potentially converting multiple explicit imports into wildcards if you haven’t changed the settings[3][5]. - Other tools like
ktfmtdo not optimize wildcard imports (i.e., do not convert them to explicit imports or vice versa), since that operation would require scanning the classpath or other files, which is out of scope for a fast code formatter[1].
"Ktfmt does not optimize wildcards, and will likely never be able to do so. Resolving wildcards requires reading other files which we can't and don't want to as part of a formatter that has to run fast."[1]
Why Avoid Wildcard Imports?
- Explicit imports make it clear which classes, functions, or properties are being used, improving code readability and reducing the risk of name clashes.
- Wildcard imports can lead to ambiguity if two imported packages contain classes or functions with the same name, requiring disambiguation and explicit import anyway[2][6].
- Many style guides, including the Android Kotlin Style Guide, recommend against wildcard imports for these reasons, although some codebases and tools may make exceptions (notably for synthetic Android extensions)[7][8].
Summary Table
| Aspect | Wildcard Import | Explicit Import |
|---|---|---|
| Readability | Hides specific usages | Clearly shows dependencies |
| Clarity | May cause name clashes | No ambiguity |
| IDE Behavior | Used by default above threshold | Always shown if configured |
| Optimize Imports | Converts explicits to wildcard if allowed | Keeps explicit if configured |
| Configurability | Yes, via .editorconfig or IDE |
Yes, via settings |
| Formatter Support | Not optimized by tools like ktfmt | Not converted to wildcards |
How to Enforce/Disable Wildcard Imports in Kotlin
- Update your
.editorconfigwith:[*.kt,*.kts] ij_kotlin_name_count_to_use_star_import = 2147483647 ij_kotlin_name_count_to_use_star_import_for_members = 2147483647 ij_kotlin_packages_to_use_import_on_demand = - In IntelliJ, make sure these settings are also applied in the Kotlin import code style tab.
- Use the Optimize Imports command—your imports will remain explicit, and wildcards will not be introduced[1][3].
If your code style requires explicit imports only, this workflow and these settings are the authoritative way to ensure consistency across your Kotlin files.
Citations:
- 1: Optimize imports does not work for wildcard imports facebook/ktfmt#366
- 2: https://www.baeldung.com/java-wildcard-imports
- 3: https://www.baeldung.com/intellij-disable-wildcard-import
- 4: https://contributors.scala-lang.org/t/wildcard-imports-considered-harmful/3580
- 5: https://www.jetbrains.com/help/idea/creating-and-optimizing-imports.html
- 6: https://kotlinlang.org/docs/packages.html
- 7: Wildcard imports Kotlin/kotlin-style-guide#37
- 8: Allow wildcard imports android/kotlin-guides#65
- 9: https://youtrack.jetbrains.com/issue/KTIJ-12967/Wildcard-import-is-added-to-a-Kotlin-file-instead-of-unambiguous-import-when-using-class-from-java.util-package
- 10: https://discuss.kotlinlang.org/t/star-import-vs-importing-each-element/14460
@hoonyworld 정말 좋은 추천 감사합니다! 🙏
개별 파일에서 수동으로 임포트를 변경하는 것보다 IntelliJ IDEA 설정으로 프로젝트 전체에 일관성 있게 적용하는 것이 훨씬 더 효율적이고 체계적인 접근 방식이네요.
특히 .editorconfig에서 다음과 같이 설정하면:
[*.kt,*.kts]
ij_kotlin_name_count_to_use_star_import = 2147483647
ij_kotlin_name_count_to_use_star_import_for_members = 2147483647
팀 전체가 동일한 임포트 스타일을 자동으로 유지할 수 있고, "Optimize Imports" 기능을 사용해도 와일드카드 임포트가 생성되지 않겠군요.
이런 도구 기반 접근 방식이 코드 리뷰에서 스타일 관련 지적을 줄이고 개발자들이 더 중요한 로직에 집중할 수 있게 해주는 것 같습니다. 실용적인 솔루션 공유해주셔서 감사합니다!
✏️ Learnings added
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#26
File: infra/src/main/kotlin/org/yapp/infra/external/redis/repository/impl/RefreshTokenRepositoryImpl.kt:8-8
Timestamp: 2025-07-04T10:24:31.227Z
Learning: hoonyworld 사용자는 코드 품질 개선을 위해 IntelliJ IDEA의 자동 import 최적화 설정 사용을 선호한다. 와일드카드 임포트를 명시적 임포트로 변환하는 "Use single class import" 설정과 같은 IDE 기능을 활용하여 코드 스타일을 자동으로 관리하는 것을 추천한다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#26
File: infra/src/main/kotlin/org/yapp/infra/external/redis/repository/impl/RefreshTokenRepositoryImpl.kt:8-8
Timestamp: 2025-07-04T10:24:31.227Z
Learning: hoonyworld 사용자는 개별 코드 수정보다는 IntelliJ IDEA 설정과 .editorconfig를 통한 체계적인 코드 스타일 관리를 선호한다. 와일드카드 임포트 방지를 위해 IDE 설정을 통해 name_count_to_use_star_import 값을 높게 설정하여 팀 전체의 일관성을 자동으로 유지하는 접근 방식을 추천한다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: domain/src/main/kotlin/org/yapp/domain/user/User.kt:32-38
Timestamp: 2025-06-26T18:12:48.178Z
Learning: 사용자 hoonyworld는 DDD에서 도메인 모델의 순수성과 단순성을 중시한다. TimeProvider와 같은 인프라 의존성을 도메인 모델에 주입하는 것보다는 LocalDateTime.now() 직접 사용이나 Domain Service로 로직 이동을 선호한다. 도메인 모델이 복잡해지는 것을 지양하고 순수함을 유지하려고 한다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#26
File: infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt:14-15
Timestamp: 2025-07-04T10:12:50.934Z
Learning: hoonyworld 사용자는 금액/통화 관련 값에 Double 대신 BigDecimal 사용을 선호함. Double의 부동소수점 정밀도 문제(예: 0.1 + 0.2 = 0.30000000000000004)로 인해 금액 데이터의 정확성이 중요한 경우 BigDecimal을 사용하는 것이 좋음.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#17
File: .github/workflows/ci-pr.yml:8-9
Timestamp: 2025-07-01T09:50:45.498Z
Learning: hoonyworld 사용자는 Naver 코드 포맷터를 사용하며, 이 포맷터는 YAML 배열의 대괄호 내부에 자동으로 여분의 공백을 추가하는 특성이 있음.
| object RestClient { | ||
| private const val HTTP_CLIENT5_VERSION = "5.2.1" | ||
|
|
||
| const val HTTP_CLIENT5 = "org.apache.httpcomponents.client5:httpclient5:$HTTP_CLIENT5_VERSION" | ||
| const val HTTP_CORE5 = "org.apache.httpcomponents.core5:httpcore5:$HTTP_CLIENT5_VERSION" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
의존성 버전 검증 필요
Apache HttpComponents HTTP 클라이언트 버전 5.2.1이 최신 버전인지, 알려진 보안 취약점이 있는지 확인해야 합니다.
🏁 Script executed:
#!/bin/bash
# Apache HttpComponents 최신 버전 및 보안 취약점 확인
echo "Apache HttpComponents 최신 버전 확인 중..."
curl -s "https://search.maven.org/solrsearch/select?q=g:org.apache.httpcomponents.client5+AND+a:httpclient5&core=gav&rows=1&wt=json" | jq -r '.response.docs[0].v'
echo "보안 취약점 확인 중..."
curl -s "https://api.osv.dev/v1/query" \
-H "Content-Type: application/json" \
-d '{"package": {"name": "httpclient5", "ecosystem": "Maven"}, "version": "5.2.1"}' | jq .Length of output: 457
HTTPClient5 버전 5.5로 업데이트 및 버전 관리 개선 제안
현재 5.2.1을 사용 중인 Apache HttpComponents HTTPClient5 는 Maven Central 기준 최신 릴리스가 5.5이며, OSV 조회 결과 5.2.1에 알려진 취약점은 없으나 최신 버전 사용 시 보안·버그 패치 혜택을 즉시 받을 수 있습니다. 또한, 하드코딩된 버전 정보를 중앙 집중형 관리(Gradle version catalog 또는 gradle.properties)로 분리하면 유지보수성이 크게 향상됩니다.
- 파일:
buildSrc/src/main/kotlin/Dependencies.kt(43–48행) - 변경 제안:
object RestClient { -
private const val HTTP_CLIENT5_VERSION = "5.2.1"
-
}
private const val HTTP_CLIENT5_VERSION = "5.5" const val HTTP_CLIENT5 = "org.apache.httpcomponents.client5:httpclient5:$HTTP_CLIENT5_VERSION" const val HTTP_CORE5 = "org.apache.httpcomponents.core5:httpcore5:$HTTP_CLIENT5_VERSION"
- 추가 권장 사항:
•libs.versions.toml또는gradle.properties로 버전 값을 관리하도록 리팩터링
• 정기적으로 최신 버전 및 보안 공지를 검토할 수 있는 CI 검증 스크립트 도입
🤖 Prompt for AI Agents
In buildSrc/src/main/kotlin/Dependencies.kt lines 43 to 48, update the
HTTP_CLIENT5_VERSION from "5.2.1" to "5.5" to use the latest Apache
HttpComponents HTTPClient5 version. Refactor the code to remove the hardcoded
version by moving the version string to a centralized version management file
such as libs.versions.toml or gradle.properties, and modify the code to
reference the version from there. This will improve maintainability and allow
easier updates in the future.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| .header("Authorization", bearerToken) | ||
| .retrieve() | ||
| .body(KakaoResponse::class.java) | ||
| ?: throw IllegalStateException("Kakao API 응답이 null 입니다.") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
에러 메시지 국제화 고려
에러 메시지가 한국어로 하드코딩되어 있습니다. 향후 국제화를 고려한다면 메시지 키를 사용하는 것을 검토해보세요.
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoRestClient.kt
at line 21, the error message is hardcoded in Korean, which hinders
internationalization. Replace the hardcoded string with a message key that can
be resolved via a localization framework or resource bundle, ensuring the error
message can be translated based on the user's locale.
| builder: RestClient.Builder | ||
| ) { | ||
| private val client = builder | ||
| .baseUrl("https://kapi.kakao.com") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
하드코딩된 URL을 설정으로 외부화 검토
카카오 API의 base URL이 하드코딩되어 있습니다. 환경별 설정이나 프로퍼티 파일로 관리하는 것을 고려해보세요.
+@Value("\${kakao.api.base-url:https://kapi.kakao.com}")
+private val baseUrl: String
- .baseUrl("https://kapi.kakao.com")
+ .baseUrl(baseUrl)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/external/oauth/kakao/KakaoRestClient.kt
at line 12, the Kakao API base URL is hardcoded. Refactor the code to
externalize this URL by moving it to a configuration file or environment
variable, then inject or load this value into the client setup to allow
environment-specific configuration.
|
|
||
| @Entity | ||
| @Table(name = "books") | ||
| @SQLDelete(sql = "UPDATE books SET deleted_at = NOW() WHERE isbn = ?") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
데이터베이스 독립적인 SQL 사용 권장
NOW()는 MySQL 전용 함수입니다. 데이터베이스 이식성을 위해 표준 SQL을 사용하는 것이 좋습니다.
-@SQLDelete(sql = "UPDATE books SET deleted_at = NOW() WHERE isbn = ?")
+@SQLDelete(sql = "UPDATE books SET deleted_at = CURRENT_TIMESTAMP WHERE isbn = ?")📝 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.
| @SQLDelete(sql = "UPDATE books SET deleted_at = NOW() WHERE isbn = ?") | |
| @SQLDelete(sql = "UPDATE books SET deleted_at = CURRENT_TIMESTAMP WHERE isbn = ?") |
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/book/entity/BookEntity.kt at line 15,
the SQLDelete annotation uses the MySQL-specific function NOW(), which reduces
database portability. Replace NOW() with a standard SQL function or use a
parameter to set the current timestamp in a database-independent way, such as
using a bind parameter or a JPA-supported function for the current timestamp.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
Outdated
Show resolved
Hide resolved
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
Outdated
Show resolved
Hide resolved
| title = item.title ?: "제목 없음", | ||
| author = item.author, | ||
| publisher = item.publisher, | ||
| publicationYear = item.pubDate?.substringBefore("-")?.toIntOrNull(), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
날짜 파싱 로직의 안정성을 검증하세요.
substringBefore("-")?.toIntOrNull() 로직은 날짜 형식이 예상과 다를 경우 실패할 수 있습니다. 외부 API에서 제공하는 날짜 형식의 일관성을 확인해야 합니다.
다음 스크립트로 Aladin API의 실제 날짜 형식을 확인해보세요:
🏁 Script executed:
#!/bin/bash
# Aladin API 응답에서 pubDate 형식 확인
rg -A 3 -B 3 "pubDate" --type kotlinLength of output: 6900
🌐 Web query:
Aladin API pubDate 필드의 형식 예시
💡 Result:
알라딘 API의 pubDate 필드는 출판일자를 나타내는 필드로, 일반적으로 다음과 같은 형식으로 제공됩니다:
- 예시 형식: "yyyy-MM-dd" 또는 "yyyyMMdd"
예를 들어:
- "2023-04-15"
- "20230415"
알라딘 API 문서에서 제공하는 구체적인 예시나 형식을 확인하시는 것이 가장 정확합니다. 혹시 필요한 경우 알라딘 API의 최신 문서나 응답 샘플을 확인해 드릴까요?
날짜 파싱 로직 개선 제안
Aladin API의 pubDate는 "yyyy-MM-dd" 또는 "yyyyMMdd" 형식으로 제공되므로, substringBefore("-") 방식은 하이픈이 없는 경우 전체 문자열(예: "20230415")을 반환해 잘못된 연도 값을 파싱할 수 있습니다.
첫 4자리만 잘라내 연도로 변환하도록 수정해 보세요.
수정 위치
- 파일: apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
- 라인: 약 51
추천 변경사항
- publicationYear = item.pubDate?.substringBefore("-")?.toIntOrNull(),
+ publicationYear = item.pubDate?.take(4)?.toIntOrNull(),🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
around line 51, the current parsing of publicationYear using
substringBefore("-") can fail for dates without hyphens like "yyyyMMdd". To fix
this, extract only the first 4 characters of pubDate and convert that substring
to an integer, ensuring correct year parsing regardless of the date format.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| fun fromAladinItem(item: BookItem): BookSummaryDto? { | ||
| val isbn = item.isbn ?: item.isbn13 ?: return null | ||
| val book = Book.create( | ||
| title = item.title ?: "제목 없음", | ||
| author = item.author, | ||
| publisher = item.publisher, | ||
| publicationYear = item.pubDate?.substringBefore("-")?.toIntOrNull(), | ||
| coverImageUrl = item.cover, | ||
| description = item.description, | ||
| isbn = isbn | ||
| ) | ||
| return from(book) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ISBN 누락 처리와 기본 제목 설정을 개선하세요.
다음 개선 사항을 고려해보세요:
- 하드코딩된 한국어 기본 제목 "제목 없음"은 다국어 지원을 고려할 때 개선이 필요합니다.
- 도메인 객체를 생성한 후 바로 DTO로 변환하는 것은 비효율적입니다.
다음과 같이 개선할 수 있습니다:
fun fromAladinItem(item: BookItem): BookSummaryDto? {
val isbn = item.isbn ?: item.isbn13 ?: return null
- val book = Book.create(
- title = item.title ?: "제목 없음",
- author = item.author,
- publisher = item.publisher,
- publicationYear = item.pubDate?.substringBefore("-")?.toIntOrNull(),
- coverImageUrl = item.cover,
- description = item.description,
- isbn = isbn
- )
- return from(book)
+ return BookSummaryDto(
+ isbn = isbn,
+ title = item.title ?: "Unknown Title", // 또는 상수화
+ author = item.author,
+ publisher = item.publisher,
+ coverImageUrl = item.cover
+ )
}🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
between lines 45 and 57, improve the handling of missing ISBN by explicitly
checking and returning null if both ISBN and ISBN13 are absent instead of using
the Elvis operator. Replace the hardcoded Korean default title "제목 없음" with a
more neutral or localized default value or handle it at the presentation layer.
Avoid creating a domain Book object just to convert it immediately to a DTO;
instead, construct the BookSummaryDto directly from the BookItem fields to
improve efficiency.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
move-hoon
left a comment
move-hoon
left a comment
There was a problem hiding this comment.
민우님 너무 고생많으셨습니다!!
제가 너무 하다보니 리뷰를 많이 남겼네요...
코멘트 한거 한 번만 확인 부탁드리겠습니다~
아 추가적으로 BookRepository는 단순히 구현만 해놔도 좋을 것 같아요!
추후 필요할 수 있을 것 같아서 그렇습니다!!
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt
Outdated
Show resolved
Hide resolved
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
Outdated
Show resolved
Hide resolved
| @field:NotBlank(message = "검색어는 필수입니다.") | ||
| val query: String, | ||
| val queryType: String? = null, | ||
| val searchTarget: String? = null, | ||
| val maxResults: Int? = null, | ||
| val start: Int? = null, | ||
| val sort: String? = null, | ||
| val cover: String? = null, | ||
| val categoryId: Int? = null |
There was a problem hiding this comment.
해당 링크에서 선택한 최종 해결책: Nullable + Valid 메서드 패턴 부분부터 한번 봐주시면 감사할 것 같습니다!
nullable로 열어두지 않으면, validation 어노테이션 메시지가 나오지 않더라고요!
| author = bookItem.author ?: "", | ||
| pubDate = bookItem.pubDate, | ||
| description = bookItem.description ?: "", | ||
| isbn = bookItem.isbn ?: bookItem.isbn13 ?: "", |
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
Outdated
Show resolved
Hide resolved
| object RestClient { | ||
| private const val HTTP_CLIENT5_VERSION = "5.2.1" | ||
|
|
||
| const val HTTP_CLIENT5 = "org.apache.httpcomponents.client5:httpclient5:$HTTP_CLIENT5_VERSION" | ||
| const val HTTP_CORE5 = "org.apache.httpcomponents.core5:httpcore5:$HTTP_CLIENT5_VERSION" | ||
| } |
| fun searchBooks(query: String, params: Map<String, Any>): AladinSearchResponse { | ||
| return aladinApi.searchBooks(query, params) |
There was a problem hiding this comment.
개인적으로는 ApiHelper까지 DTO를 그대로 전달하고, 해당 DTO를 내부 메서드를 통해 Map으로 변환한 뒤 Aladin API에 전달하는 방식은 어떨까요?
해당 방식으로 했을 때 좀 더 네이밍에서 의미도 전달되고, 타입 안정성도 보장되어
코드의 가독성과 유지보수성이 더 좋아질 것 같아서 그렇습니다!!
infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt
Outdated
Show resolved
Hide resolved
| @@ -1,9 +1,10 @@ | |||
| package org.yapp.infra.external.redis.repository | |||
| package org.yapp.infra.external.redis.repository.impl | |||
There was a problem hiding this comment.
@minwoo1999 이 부분은 user랑 통일해보면 좋을 것 같아요!
user는 레포 구현체도 같은 패키지로 두어서 말씀드립니다!
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
infra/build.gradle.kts (1)
19-19: 삭제된 Feign 의존성 참조로 CI 빌드 실패
Dependencies.Feign.STARTER_OPENFEIGN객체가 소스에서 제거되었는데 여전히 참조되고 있어 CI가Unresolved reference: Feign오류로 실패합니다. 그대로 두면 infra 모듈은 컴파일되지 않습니다.- implementation(Dependencies.Feign.STARTER_OPENFEIGN)필요 없다면 해당 줄을 삭제하고, 대체 라이브러리가 있다면 올바른 의존성으로 교체해 주세요.
♻️ Duplicate comments (1)
buildSrc/src/main/kotlin/Dependencies.kt (1)
43-47: HTTPClient5 버전 5.2.1 → 5.5 로 업데이트 및 버전 중앙관리 필요이전 리뷰에서도 동일 내용이 지적되었으나 아직 반영되지 않았습니다. 최신 보안-버그 픽스를 위해
5.5로 올리고, 하드코딩 대신libs.versions.toml또는gradle.properties로 이동해 주세요.- private const val HTTP_CLIENT5_VERSION = "5.2.1" + private const val HTTP_CLIENT5_VERSION = "5.5"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apis/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/Dependencies.kt(1 hunks)gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt(2 hunks)infra/build.gradle.kts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI - Pull Request
infra/build.gradle.kts
[error] 19-19: Script compilation error: Unresolved reference: Feign at line 19. The build failed due to missing or incorrect reference to 'Dependencies.Feign.STARTER_OPENFEIGN'.
🔇 Additional comments (2)
gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt (2)
37-37: 헬스 체크 엔드포인트 공개 접근 허용이 적절합니다.헬스 체크 엔드포인트를 공개적으로 접근 가능하게 하는 것은 모니터링 시스템과 로드 밸런서가 인증 없이 애플리케이션의 상태를 확인할 수 있도록 하는 일반적이고 적절한 방식입니다.
36-36:/api/v1/books/**경로는 오직 조회용 GET 엔드포인트만 등록되어 있어 permitAll 적용이 적절합니다.확인된 컨트롤러 및 매핑
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt
- @GetMapping("/search")
- @GetMapping("/{itemId}")
- apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
- @GetMapping("/search")
- @GetMapping("/{itemId}")
현재 이 경로 하위에는 쓰기·수정·삭제 엔드포인트가 없으므로, 인증 없이 공개 접근을 허용해도 문제없습니다.
| testImplementation(Dependencies.Spring.BOOT_STARTER_TEST) | ||
| implementation(Dependencies.Spring.BOOT_STARTER_DATA_JPA) | ||
| implementation(Dependencies.Spring.BOOT_STARTER_DATA_REDIS) | ||
| implementation(Dependencies.RestClient.HTTP_CLIENT5) | ||
| implementation(Dependencies.RestClient.HTTP_CORE5) | ||
| testImplementation(Dependencies.Spring.BOOT_STARTER_TEST) | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
spring-boot-starter-test 중복 선언 제거
testImplementation(Dependencies.Spring.BOOT_STARTER_TEST) 이 8행과 13행에 두 번 선언되어 있습니다. 중복 의존성은 Gradle 그래프 난독화의 주범이니 하나만 남기고 제거해 주세요.
- implementation(Dependencies.Spring.BOOT_STARTER_WEB)
- testImplementation(Dependencies.Spring.BOOT_STARTER_TEST)
...
- testImplementation(Dependencies.Spring.BOOT_STARTER_TEST)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In infra/build.gradle.kts between lines 8 and 14, the dependency
'testImplementation(Dependencies.Spring.BOOT_STARTER_TEST)' is declared twice.
Remove the duplicate declaration on line 13 and keep only one instance to avoid
redundancy in the Gradle dependency graph.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| implementation(Dependencies.Logging.KOTLIN_LOGGING) | ||
| implementation(Dependencies.Feign.STARTER_OPENFEIGN) | ||
|
|
||
| testImplementation(Dependencies.Spring.BOOT_STARTER_TEST) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
spring-boot-starter-test 의존성 중복
10행과 26행에 동일 의존성이 중복되어 있습니다. 하나만 남기고 제거해 주세요.
- testImplementation(Dependencies.Spring.BOOT_STARTER_TEST)📝 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.
| testImplementation(Dependencies.Spring.BOOT_STARTER_TEST) |
🤖 Prompt for AI Agents
In apis/build.gradle.kts at line 26, the dependency spring-boot-starter-test is
duplicated as it also appears at line 10. Remove the duplicate declaration at
line 26 and keep only the one at line 10 to avoid redundancy.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| private val jwtTokenProvider: JwtTokenProvider | ||
| ) { | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
불필요한 빈 줄 제거를 고려해보세요.
클래스 선언 직후의 빈 줄이 추가되었는데, 기존 코드 스타일과 일관성을 위해 제거하는 것이 좋겠습니다.
🤖 Prompt for AI Agents
In gateway/src/main/kotlin/org/yapp/gateway/config/SecurityConfig.kt at line 21,
remove the unnecessary blank line immediately following the class declaration to
maintain consistency with the existing code style.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (10)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (1)
45-45: ISBN 필드 우선순위 수정 누락과거 리뷰에서 합의된 ISBN13 우선순위 변경이 아직 적용되지 않았습니다. ISBN13이 더 표준적이므로 우선순위를 변경해주세요.
- isbn = bookItem.isbn ?: bookItem.isbn13 ?: "", + isbn = bookItem.isbn13 ?: bookItem.isbn ?: "",apis/src/main/kotlin/org/yapp/apis/book/helper/AladinApiHelper.kt (1)
25-29: 비즈니스 예외 맵핑 TODO 해결 필요과거 리뷰에서 언급된 TODO 주석이 여전히 남아있습니다. 적절한 비즈니스 예외 처리를 구현해주세요.
알라딘 API 관련 커스텀 예외 클래스를 생성하고 예외 매핑 로직을 구현하는 코드를 작성해드릴까요?
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt (1)
7-16: 검증 패턴이 올바르게 구현되었습니다.과거 리뷰 댓글에서 언급된 "Nullable + Valid 메서드 패턴"을 정확히 따르고 있습니다. 이 패턴은 validation 어노테이션 메시지가 제대로 표시되도록 하는 적절한 접근 방식입니다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt (2)
15-16: BigDecimal 사용이 적절합니다.과거 리뷰에서 언급된 대로 금액 필드에 BigDecimal을 사용하여 부동소수점 정밀도 문제를 해결했습니다.
26-52: 클래스 중복 문제가 여전히 존재합니다.
AladinSearchResponse와AladinBookDetailResponse가 완전히 동일한 구조를 가지고 있어 DRY 원칙에 위배됩니다. 과거 리뷰에서 이미 지적된 사항입니다.infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt (2)
13-15: 하드코딩된 URL을 설정으로 분리해야 합니다.과거 리뷰에서 지적된 대로 베이스 URL이 하드코딩되어 있어 환경별 설정이 불가능하며, HTTP 프로토콜 사용으로 인한 보안 우려가 있습니다.
44-45: 더 구체적인 예외 처리가 필요합니다.과거 리뷰에서 지적된 대로 HTTP 에러 상태나 네트워크 오류에 대한 처리가 부족합니다. 현재는 null 응답에 대해서만 예외를 던지고 있습니다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt (1)
16-28: 한글 주석을 영문으로 변경하세요코드베이스의 일관성을 위해 주석은 영문으로 작성하는 것이 좋습니다.
- // AladinBookSearchRequest DTO를 직접 받도록 변경 + // Convert AladinBookSearchRequest DTO to parameters fun searchBooks(request: AladinBookSearchRequest): Result<AladinSearchResponse> { return runCatching { - // DTO의 toMap() 메서드를 호출하여 Map으로 변환 + // Convert DTO to Map using toMap() method val aladinApiParams = request.toMap() - aladinRestClient.itemSearch(ttbKey, aladinApiParams) // Map으로 전달 + aladinRestClient.itemSearch(ttbKey, aladinApiParams) // Pass as Map } } fun lookupBook(request: AladinBookLookupRequest): Result<AladinBookDetailResponse> { return runCatching { val aladinApiParams = request.toMap() - aladinRestClient.itemLookUp(ttbKey, aladinApiParams) // Map으로 전달 + aladinRestClient.itemLookUp(ttbKey, aladinApiParams) // Pass as Map } }apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (2)
50-50: 하드코딩된 한글 기본 제목을 개선하세요다국어 지원을 고려할 때 하드코딩된 한국어 기본 제목은 개선이 필요합니다.
- private val unknownTitle = "제목없음" + private val unknownTitle = "Unknown Title"또는 상수 클래스로 분리하거나 메시지 프로퍼티로 관리하는 것을 고려해보세요.
40-40: 클래스 네이밍 규칙 통일을 고려해보세요과거 리뷰 코멘트에서 언급된 대로,
BookSummaryDto에서Dto접미사를 제거하고dto패키지 위치로 역할을 드러내는 방식을 고려해보세요.- data class BookSummaryDto private constructor( + data class BookSummary private constructor(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
admin/src/main/resources/application.yml(1 hunks)apis/build.gradle.kts(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookDetailRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/helper/AladinApiHelper.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(1 hunks)apis/src/main/resources/application.yml(1 hunks)infra/build.gradle.kts(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/request/AladinBookLookupRequest.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/request/AladinBookSearchRequest.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt(2 hunks)infra/src/main/resources/application-external.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt (1)
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:57:49.486Z
Learning: 사용자 hoonyworld가 DDD에서 User 도메인 복원 로직을 구현할 때 Domain Model에서 require() 검증과 copy() 패턴을 사용하고, Domain Service에서 save(deletedUser.restore()) 패턴을 사용하는 방식을 선호한다. 이는 불변성 유지, 비즈니스 규칙 검증, 명확한 책임 분리를 모두 만족하는 훌륭한 DDD 구현 방식이다.
apis/src/main/kotlin/org/yapp/apis/book/helper/AladinApiHelper.kt (1)
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt:12-28
Timestamp: 2025-06-26T16:07:29.888Z
Learning: DTO에서는 정적 팩토리 메서드에서 require 검증을 하는 것이 적합하다. DTO는 데이터 전송이 목적이므로 프레임워크 호환성(Jackson 등), 에러 처리의 유연성, Spring Validation과의 조화, 검증 시점 제어 등을 고려해야 한다. 반면 VO는 도메인 개념을 표현하므로 init 블록에서 require 검증을 통해 항상 유효한 상태를 보장해야 한다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt (1)
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#26
File: infra/src/main/kotlin/org/yapp/infra/external/aladin/response/AladinResponseBase.kt:14-15
Timestamp: 2025-07-04T10:12:50.934Z
Learning: hoonyworld 사용자는 금액/통화 관련 값에 Double 대신 BigDecimal 사용을 선호함. Double의 부동소수점 정밀도 문제(예: 0.1 + 0.2 = 0.30000000000000004)로 인해 금액 데이터의 정확성이 중요한 경우 BigDecimal을 사용하는 것이 좋음.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (6)
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt:12-28
Timestamp: 2025-06-26T16:07:29.888Z
Learning: DTO에서는 정적 팩토리 메서드에서 require 검증을 하는 것이 적합하다. DTO는 데이터 전송이 목적이므로 프레임워크 호환성(Jackson 등), 에러 처리의 유연성, Spring Validation과의 조화, 검증 시점 제어 등을 고려해야 한다. 반면 VO는 도메인 개념을 표현하므로 init 블록에서 require 검증을 통해 항상 유효한 상태를 보장해야 한다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#12
File: apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt:12-18
Timestamp: 2025-06-24T08:46:22.341Z
Learning: Request DTO에서 private constructor를 사용하는 것은 직접 인스턴스 생성을 방지하고 HTTP request 바인딩을 통해서만 객체가 생성되도록 하는 의도적인 설계 패턴이다. Spring의 Jackson은 리플렉션을 통해 private constructor도 호출할 수 있어서 HTTP request 바인딩에는 문제가 없다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:57:49.486Z
Learning: 사용자 hoonyworld가 DDD에서 User 도메인 복원 로직을 구현할 때 Domain Model에서 require() 검증과 copy() 패턴을 사용하고, Domain Service에서 save(deletedUser.restore()) 패턴을 사용하는 방식을 선호한다. 이는 불변성 유지, 비즈니스 규칙 검증, 명확한 책임 분리를 모두 만족하는 훌륭한 DDD 구현 방식이다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: domain/src/main/kotlin/org/yapp/domain/user/User.kt:32-38
Timestamp: 2025-06-26T18:12:48.178Z
Learning: 사용자 hoonyworld는 DDD에서 도메인 모델의 순수성과 단순성을 중시한다. TimeProvider와 같은 인프라 의존성을 도메인 모델에 주입하는 것보다는 LocalDateTime.now() 직접 사용이나 Domain Service로 로직 이동을 선호한다. 도메인 모델이 복잡해지는 것을 지양하고 순수함을 유지하려고 한다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:36:15.056Z
Learning: DDD에서 Repository는 순수한 CRUD 작업(findById, save, delete 등)만 담당해야 하고, restore 같은 비즈니스 로직이 포함된 메서드는 Domain Service에서 처리하는 것이 올바른 설계이다. JPA 더티체킹을 활용하면 Domain Service에서 도메인 모델을 변경하고 save를 호출할 때 자동으로 UPDATE가 실행된다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:36:24.582Z
Learning: DDD 아키텍처에서 restore 같은 비즈니스 로직은 Repository가 아닌 Domain Service에서 처리해야 하며, JPA dirty checking을 활용하여 자동 업데이트하는 것이 더 적절한 설계이다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-validation
🔇 Additional comments (26)
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt (2)
8-8: 명시적JpaUserRepository주입으로 가독성 향상빈 주입 대상을 명시적으로 import 하여 의도를 분명히 했다는 점 👍
추가 변경 사항 없이 컴파일·런타임 모두 영향 없으므로 그대로 유지해도 좋습니다.
1-1: 레거시 패키지 경로 참조 없음 확인됨
rg --line-number 'org\.yapp\.infra\.user\.repository\.UserRepositoryImpl'실행 결과 남아있는 참조가 없습니다.admin/src/main/resources/application.yml (1)
12-12: 외부 API 프로필 추가가 적절합니다.dev와 prod 환경에 external 프로필이 올바르게 추가되었습니다. 테스트 환경에서는 외부 API 연동이 필요하지 않으므로 제외된 것이 합리적입니다.
Also applies to: 17-17
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookDetailRequest.kt (1)
12-19: 중복된 검증 로직 제거 필요
toAladinRequest()메서드의 13번째 줄에서 수동으로 검증하는 로직이 Bean Validation과 중복됩니다. 일관된 검증 전략을 사용하는 것이 좋습니다.Bean Validation을 사용한다면 수동 검증을 제거하고:
fun toAladinRequest(): AladinBookLookupRequest { - require(!itemId.isNullOrBlank()) { "아이템 ID는 비어있을 수 없습니다." } return AladinBookLookupRequest.create( - itemId = this.itemId, + itemId = this.itemId!!, itemIdType = this.itemIdType ?: "ISBN", optResult = this.optResult ) }⛔ Skipped due to learnings
Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#15 File: apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt:12-28 Timestamp: 2025-06-26T16:07:29.888Z Learning: DTO에서는 정적 팩토리 메서드에서 require 검증을 하는 것이 적합하다. DTO는 데이터 전송이 목적이므로 프레임워크 호환성(Jackson 등), 에러 처리의 유연성, Spring Validation과의 조화, 검증 시점 제어 등을 고려해야 한다. 반면 VO는 도메인 개념을 표현하므로 init 블록에서 require 검증을 통해 항상 유효한 상태를 보장해야 한다.infra/src/main/resources/application-external.yml (1)
1-4: 외부 API 설정이 적절합니다.알라딘 API 키를 환경변수로 주입받는 설정이 올바르게 구성되었습니다. 보안 관점에서도 적절한 방식입니다.
apis/src/main/resources/application.yml (1)
12-12: 외부 API 프로필 추가가 적절합니다.admin 모듈과 동일하게 dev와 prod 환경에 external 프로필이 올바르게 추가되었습니다. 모듈 간 일관된 설정입니다.
Also applies to: 17-17
infra/build.gradle.kts (1)
11-12: RestClient 의존성 추가가 적절합니다.Feign에서 RestClient로 마이그레이션하는 PR 목표에 맞게 Apache HttpComponents 의존성이 올바르게 추가되었습니다.
apis/build.gradle.kts (1)
25-25: 빌드 파일 변경사항 확인 완료과거 리뷰 코멘트에서 언급된 중복 의존성 문제는 현재 코드에서 확인되지 않습니다.
spring-boot-starter-test는 테스트 의존성이고, 10번 줄의spring-boot-starter-data-jpa는 다른 의존성입니다.apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookDetailResponse.kt (2)
10-29: DTO 구조 및 필드 타입 검토BookDetailResponse의 필드 구조가 잘 설계되어 있습니다. 특히 BigDecimal을 사용한 가격 필드와 적절한 nullable 타입 처리가 인상적입니다.
34-58: 팩토리 메서드 구현 확인외부 API 응답을 내부 DTO로 변환하는 팩토리 메서드가 잘 구현되어 있습니다. 빈 값에 대한 기본값 처리와 예외 처리도 적절합니다.
apis/src/main/kotlin/org/yapp/apis/book/service/AladinBookQueryService.kt (3)
13-17: 서비스 클래스 구조 확인
@Service어노테이션과 생성자 주입 방식이 올바르게 적용되어 있습니다. 로깅 설정도 적절합니다.
19-24: 도서 검색 메서드 구현내부 DTO를 외부 API DTO로 변환하고, 응답을 다시 내부 DTO로 변환하는 로직이 명확하게 구현되어 있습니다. 로깅을 통한 추적성도 좋습니다.
26-31: 도서 상세 조회 메서드 구현검색 메서드와 동일한 패턴으로 일관성 있게 구현되어 있습니다. 메서드명도 직관적입니다.
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (2)
3-9: Import 문 및 어노테이션 확인과거 리뷰에서 언급된 import 문제는 해결되었습니다. 모든 import가 올바르게 정의되어 있고,
@UseCase와@Transactional(readOnly = true)어노테이션도 적절하게 적용되었습니다.
11-23: UseCase 클래스 구현서비스 계층에 대한 단순한 위임 패턴으로 구현되어 있어 레이어 간 책임 분리가 명확합니다. 메서드명도 직관적입니다.
apis/src/main/kotlin/org/yapp/apis/book/helper/AladinApiHelper.kt (2)
18-31: Result 타입 활용한 에러 처리Kotlin의 Result 타입을 활용한 에러 처리가 잘 구현되어 있습니다. 성공/실패 케이스 모두 적절한 로깅이 포함되어 있습니다.
33-45: 도서 상세 조회 메서드 구현검색 메서드와 동일한 패턴으로 일관성 있게 구현되어 있습니다. 응답 데이터에서 첫 번째 아이템의 제목을 로깅하는 부분이 유용합니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt (1)
19-32: 타입 변환 로직이 깔끔하게 구현되었습니다.
toAladinRequest()메서드에서 런타임 검증과 DTO 변환을 명확하게 처리하고 있습니다.require()구문을 통한 추가 검증도 적절합니다.infra/src/main/kotlin/org/yapp/infra/external/aladin/request/AladinBookLookupRequest.kt (2)
7-11: 일관된 설계 패턴을 따르고 있습니다.private constructor와 companion object factory method 패턴이 다른 DTO들과 일관되게 적용되어 있습니다.
12-22: API 파라미터 변환 로직이 적절합니다.
toMap()메서드에서 알라딘 API 파라미터 형식에 맞게 변환하고,optResult의 null 안전성을 고려한 처리가 좋습니다.infra/src/main/kotlin/org/yapp/infra/external/aladin/request/AladinBookSearchRequest.kt (2)
3-12: 필수/선택 파라미터 구분이 명확합니다.
query필드는 non-nullable로, 나머지는 nullable로 설정하여 API 요구사항을 명확히 반영하고 있습니다.
13-24: 파라미터 매핑 로직이 효율적입니다.null 안전성을 고려한 파라미터 매핑과 알라딘 API 규격에 맞는 키 변환이 적절하게 구현되어 있습니다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt (1)
20-30: 쿼리 파라미터 처리 로직이 깔끔합니다.
OptResult리스트를 콤마로 구분하여 처리하는 로직과 공통 파라미터 추가 방식이 효율적입니다.apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
15-34: 구현이 잘 되어 있습니다Controller의 책임 분리가 적절하고, 유효성 검증과 UseCase로의 위임이 올바르게 구현되었습니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt (1)
20-113: API 문서화가 잘 되어 있습니다Swagger 어노테이션을 통한 API 문서화가 상세하고 적절하게 구현되었습니다. 예시 URL들도 도움이 됩니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (1)
52-61: 도메인 객체 생성 위치를 재고해보세요과거 리뷰 코멘트에서 언급된 대로, DTO에서 직접 도메인 객체를 생성하는 것보다
BookDomainService에서 처리하는 것이 역할 분리 측면에서 더 적절할 수 있습니다.현재 구현은 DTO에서 직접 변환하므로 적절하지만, 향후 비즈니스 로직이 복잡해질 경우 도메인 서비스로 이동을 고려해보세요.
⛔ Skipped due to learnings
Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#15 File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50 Timestamp: 2025-06-26T17:36:24.582Z Learning: DDD 아키텍처에서 restore 같은 비즈니스 로직은 Repository가 아닌 Domain Service에서 처리해야 하며, JPA dirty checking을 활용하여 자동 업데이트하는 것이 더 적절한 설계이다.Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#15 File: domain/src/main/kotlin/org/yapp/domain/user/User.kt:32-38 Timestamp: 2025-06-26T18:12:48.178Z Learning: 사용자 hoonyworld는 DDD에서 도메인 모델의 순수성과 단순성을 중시한다. TimeProvider와 같은 인프라 의존성을 도메인 모델에 주입하는 것보다는 LocalDateTime.now() 직접 사용이나 Domain Service로 로직 이동을 선호한다. 도메인 모델이 복잡해지는 것을 지양하고 순수함을 유지하려고 한다.Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#15 File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50 Timestamp: 2025-06-26T17:36:15.056Z Learning: DDD에서 Repository는 순수한 CRUD 작업(findById, save, delete 등)만 담당해야 하고, restore 같은 비즈니스 로직이 포함된 메서드는 Domain Service에서 처리하는 것이 올바른 설계이다. JPA 더티체킹을 활용하면 Domain Service에서 도메인 모델을 변경하고 save를 호출할 때 자동으로 UPDATE가 실행된다.Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#15 File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50 Timestamp: 2025-06-26T17:57:49.486Z Learning: 사용자 hoonyworld가 DDD에서 User 도메인 복원 로직을 구현할 때 Domain Model에서 require() 검증과 copy() 패턴을 사용하고, Domain Service에서 save(deletedUser.restore()) 패턴을 사용하는 방식을 선호한다. 이는 불변성 유지, 비즈니스 규칙 검증, 명확한 책임 분리를 모두 만족하는 훌륭한 DDD 구현 방식이다.Learnt from: hoonyworld PR: YAPP-Github/26th-App-Team-1-BE#15 File: apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt:12-28 Timestamp: 2025-06-26T16:07:29.888Z Learning: DTO에서는 정적 팩토리 메서드에서 require 검증을 하는 것이 적합하다. DTO는 데이터 전송이 목적이므로 프레임워크 호환성(Jackson 등), 에러 처리의 유연성, Spring Validation과의 조화, 검증 시점 제어 등을 고려해야 한다. 반면 VO는 도메인 개념을 표현하므로 init 블록에서 require 검증을 통해 항상 유효한 상태를 보장해야 한다.
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt
Show resolved
Hide resolved
| @field:NotBlank(message = "아이템 ID는 필수입니다.") | ||
| val itemId: String? = null, |
There was a problem hiding this comment.
nullable 필드에 대한 검증 어노테이션 문제
itemId가 nullable(String?)이면서 @NotBlank 어노테이션이 적용되어 있습니다. nullable 필드에 대해서는 Bean Validation이 제대로 동작하지 않을 수 있습니다.
다음 중 하나의 방법으로 수정하는 것을 권장합니다:
- @field:NotBlank(message = "아이템 ID는 필수입니다.")
- val itemId: String? = null,
+ @field:NotBlank(message = "아이템 ID는 필수입니다.")
+ val itemId: String,또는 nullable을 유지하면서 커스텀 검증 로직을 사용:
- @field:NotBlank(message = "아이템 ID는 필수입니다.")
- val itemId: String? = null,
+ val itemId: String? = null,🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookDetailRequest.kt at
lines 7-8, the itemId field is declared as nullable String? but annotated with
@NotBlank, which may cause validation to not work properly. To fix this, either
change the type to non-nullable String and keep @NotBlank, or keep it nullable
and implement a custom validator to handle blank or null values correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (1)
46-46: 다국어 지원을 위한 기본 제목 처리 개선하드코딩된 한국어 기본 제목은 다국어 지원 시 문제가 될 수 있습니다. 이전 리뷰에서도 언급된 부분이지만, 현재 상수로 분리한 것은 좋은 개선입니다.
다음 중 하나의 방식을 고려해보세요:
- 메시지 소스를 활용한 다국어 지원
- 영어 기본값 사용 (예: "Unknown Title")
- 클라이언트에서 처리하도록 null 허용
- private val unknownTitle = "제목없음" + private val unknownTitle = "Unknown Title"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (6)
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: apis/src/main/kotlin/org/yapp/apis/auth/dto/UserCreateInfo.kt:12-28
Timestamp: 2025-06-26T16:07:29.888Z
Learning: DTO에서는 정적 팩토리 메서드에서 require 검증을 하는 것이 적합하다. DTO는 데이터 전송이 목적이므로 프레임워크 호환성(Jackson 등), 에러 처리의 유연성, Spring Validation과의 조화, 검증 시점 제어 등을 고려해야 한다. 반면 VO는 도메인 개념을 표현하므로 init 블록에서 require 검증을 통해 항상 유효한 상태를 보장해야 한다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#12
File: apis/src/main/kotlin/org/yapp/apis/auth/dto/request/SocialLoginRequest.kt:12-18
Timestamp: 2025-06-24T08:46:22.341Z
Learning: Request DTO에서 private constructor를 사용하는 것은 직접 인스턴스 생성을 방지하고 HTTP request 바인딩을 통해서만 객체가 생성되도록 하는 의도적인 설계 패턴이다. Spring의 Jackson은 리플렉션을 통해 private constructor도 호출할 수 있어서 HTTP request 바인딩에는 문제가 없다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:57:49.486Z
Learning: 사용자 hoonyworld가 DDD에서 User 도메인 복원 로직을 구현할 때 Domain Model에서 require() 검증과 copy() 패턴을 사용하고, Domain Service에서 save(deletedUser.restore()) 패턴을 사용하는 방식을 선호한다. 이는 불변성 유지, 비즈니스 규칙 검증, 명확한 책임 분리를 모두 만족하는 훌륭한 DDD 구현 방식이다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: domain/src/main/kotlin/org/yapp/domain/user/User.kt:32-38
Timestamp: 2025-06-26T18:12:48.178Z
Learning: 사용자 hoonyworld는 DDD에서 도메인 모델의 순수성과 단순성을 중시한다. TimeProvider와 같은 인프라 의존성을 도메인 모델에 주입하는 것보다는 LocalDateTime.now() 직접 사용이나 Domain Service로 로직 이동을 선호한다. 도메인 모델이 복잡해지는 것을 지양하고 순수함을 유지하려고 한다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:36:15.056Z
Learning: DDD에서 Repository는 순수한 CRUD 작업(findById, save, delete 등)만 담당해야 하고, restore 같은 비즈니스 로직이 포함된 메서드는 Domain Service에서 처리하는 것이 올바른 설계이다. JPA 더티체킹을 활용하면 Domain Service에서 도메인 모델을 변경하고 save를 호출할 때 자동으로 UPDATE가 실행된다.
Learnt from: hoonyworld
PR: YAPP-Github/26th-App-Team-1-BE#15
File: infra/src/main/kotlin/org/yapp/infra/user/repository/UserRepositoryImpl.kt:42-50
Timestamp: 2025-06-26T17:36:24.582Z
Learning: DDD 아키텍처에서 restore 같은 비즈니스 로직은 Repository가 아닌 Domain Service에서 처리해야 하며, JPA dirty checking을 활용하여 자동 업데이트하는 것이 더 적절한 설계이다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-validation
🔇 Additional comments (2)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (2)
48-57: 팩토리 메서드 구현이 깔끔합니다ISBN 처리와 null 안전성 처리가 잘 구현되어 있습니다. 이전 리뷰에서 제안된 대로 도메인 객체를 생성하지 않고 직접 DTO를 구성하는 방식으로 효율적으로 구현되었습니다.
38-44: nested DTO 구조가 적절합니다BookSummary가 nested class로 구현된 것은 응답 구조의 계층을 명확히 보여주는 좋은 설계입니다. 이전 리뷰에서 언급된 대로 클래스명 통일 (Dto suffix 제거)은 팀 컨벤션에 따라 추후 조정하면 될 것 같습니다.
| data class BookSearchResponse private constructor( | ||
| val version: String?, | ||
| val title: String?, | ||
| val link: String?, | ||
| val pubDate: String?, | ||
| val totalResults: Int?, | ||
| val startIndex: Int?, | ||
| val itemsPerPage: Int?, | ||
| val query: String?, | ||
| val searchCategoryId: Int?, | ||
| val searchCategoryName: String?, | ||
| val books: List<BookSummary> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
nullable 필드가 많은 메타데이터 구조 검토
BookSearchResponse의 대부분 필드가 nullable로 정의되어 있습니다. 외부 API 응답의 불확실성을 고려한 설계로 보이지만, 일부 필드(totalResults, startIndex, itemsPerPage 등)는 검색 결과의 핵심 정보로서 non-null이 더 적절할 수 있습니다.
외부 API 응답에서 이러한 필드들이 항상 제공되는지 확인하고, 필요시 기본값을 제공하는 것을 고려해보세요.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt
between lines 6 and 17, several fields like totalResults, startIndex, and
itemsPerPage are currently nullable but represent core search result metadata.
Review the external API to confirm if these fields are always present; if so,
change their types to non-nullable and provide default values if necessary to
ensure these fields always have valid data in the response object.
move-hoon
left a comment
move-hoon
left a comment
There was a problem hiding this comment.
코드 Aprv 하겠습니다~~ (몇가지 코멘트 남긴 부분만 확인해주세요!)
|
|
||
| override fun save(user: User): User { | ||
| val userEntity = UserEntity.fromDomain(user) | ||
| val userEntity = UserEntity.Companion.fromDomain(user) |
There was a problem hiding this comment.
해당 부분 Companion을 넣은 이유가 궁금합니다!
There was a problem hiding this comment.
자동완성 된 것 같네요 저거 뺄게요! ㅎㅎ
해당 부분은 timeout, baseurl 등 설정이 다를 수 있기에 별도로 빈을 분리해서 가져가면 좋을 것 같아요!! 그래서 제 생각에는 RestClientConfig를 범용 클래스로 가져가고 그 내부에서 알라딘 빈을 등록하는 방식이 좋을 것 같습니다. @Configuration
class RestClientConfig : InfraBaseConfig {
@Bean
@Primary
fun restClient(): RestClient {
val requestConfig = RequestConfig.custom()
.setConnectionRequestTimeout(Timeout.ofSeconds(30))
.setResponseTimeout(Timeout.ofSeconds(30))
.build()
val httpClient: CloseableHttpClient = HttpClients.custom()
.setDefaultRequestConfig(requestConfig)
.build()
val factory = HttpComponentsClientHttpRequestFactory(httpClient)
return RestClient.builder()
.requestFactory(factory)
.build()
}
@Bean
fun aladinRestClient(): RestClient {
val requestConfig = RequestConfig.custom()
.setConnectionRequestTimeout(Timeout.ofSeconds(30))
.setResponseTimeout(Timeout.ofSeconds(30))
.build()
val httpClient: CloseableHttpClient = HttpClients.custom()
.setDefaultRequestConfig(requestConfig)
.build()
val factory = HttpComponentsClientHttpRequestFactory(httpClient)
return RestClient.builder()
.baseUrl("https://www.aladin.co.kr/ttb/api/")
.requestFactory(factory)
.build()
}
}class AladinService(
@Qualifier("aladinRestClient") private val restClient: RestClient
) {enum class InfraBaseConfigGroup(
val configClass: Class<out InfraBaseConfig>
) {
ASYNC(AsyncConfig::class.java),
JPA(JpaConfig::class.java),
REDIS(RedisConfig::class.java),
REST_CLIENT(RestClientConfig::class.java)
}
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt (2)
41-45: HTTP 에러 상태 처리 추가 필요현재 null 응답에 대해서만 예외를 던지고 있지만, HTTP 에러 상태나 네트워크 오류에 대한 처리도 필요합니다.
RestClient의 에러 핸들링을 추가하세요:
return client.get() .uri(uriBuilder.build().toUriString()) .retrieve() + .onStatus(HttpStatusCode::is4xxClientError) { _, response -> + throw IllegalArgumentException("Aladin API 클라이언트 오류: ${response.statusCode}") + } + .onStatus(HttpStatusCode::is5xxServerError) { _, response -> + throw IllegalStateException("Aladin API 서버 오류: ${response.statusCode}") + } .body(AladinSearchResponse::class.java) ?: throw IllegalStateException("Aladin ItemSearch API 응답이 null 입니다.")
57-61: HTTP 에러 상태 처리 추가 필요itemLookUp 메서드에서도 HTTP 에러 상태에 대한 처리가 필요합니다.
동일한 에러 핸들링을 추가하세요:
return client.get() .uri(uriBuilder.build().toUriString()) .retrieve() + .onStatus(HttpStatusCode::is4xxClientError) { _, response -> + throw IllegalArgumentException("Aladin API 클라이언트 오류: ${response.statusCode}") + } + .onStatus(HttpStatusCode::is5xxServerError) { _, response -> + throw IllegalStateException("Aladin API 서버 오류: ${response.statusCode}") + } .body(AladinBookDetailResponse::class.java) ?: throw IllegalStateException("Aladin ItemLookUp API 응답이 null 입니다.")infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt (2)
19-19: 한글 주석을 영문으로 변경이전 리뷰에서 지적된 동일한 문제입니다. 코드베이스의 일관성을 위해 주석은 영문으로 작성해주세요.
- aladinRestClient.itemSearch(ttbKey, aladinApiParams) // Map으로 전달 + aladinRestClient.itemSearch(ttbKey, aladinApiParams) // Pass as Map
26-26: 한글 주석을 영문으로 변경이전 리뷰에서 지적된 동일한 문제입니다. 코드베이스의 일관성을 위해 주석은 영문으로 작성해주세요.
- aladinRestClient.itemLookUp(ttbKey, aladinApiParams) // Map으로 전달 + aladinRestClient.itemLookUp(ttbKey, aladinApiParams) // Pass as Map
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/config/RestTemplateConfig.kt(0 hunks)infra/build.gradle.kts(1 hunks)infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/config/external/api/RestClientConfig.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt(1 hunks)infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt(1 hunks)
💤 Files with no reviewable changes (1)
- apis/src/main/kotlin/org/yapp/apis/config/RestTemplateConfig.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-validation
🔇 Additional comments (10)
infra/src/main/kotlin/org/yapp/infra/user/repository/impl/UserRepositoryImpl.kt (3)
1-1: 패키지 구조 개선이 적절합니다.Repository 구현체를
.impl하위 패키지로 이동시키는 것은 관심사 분리와 코드 구조화에 도움이 됩니다.
8-8: 필요한 임포트 추가가 적절합니다.
JpaUserRepository임포트 추가로 의존성 주입이 올바르게 구성됩니다.
24-28: 도메인-엔티티 변환 로직이 깔끔합니다.
UserEntity.fromDomain(user)호출 방식이 간결하고 읽기 쉽습니다. 과거 리뷰에서 논의된 Companion 접두어 문제가 잘 해결되었습니다.infra/build.gradle.kts (1)
9-11: RestClient 의존성 추가 승인Feign에서 RestClient로 마이그레이션하는 과정에서 Apache HttpComponents 의존성들이 올바르게 추가되었습니다. 이는 전체 프로젝트의 외부 API 통신 방식 변경과 일치합니다.
apis/src/main/kotlin/org/yapp/apis/config/InfraConfig.kt (1)
13-13: REST_CLIENT 설정 그룹 변경 승인OAUTH에서 REST_CLIENT로의 변경이 전체 프로젝트의 Feign에서 RestClient로의 마이그레이션과 일치하며 올바르게 적용되었습니다.
infra/src/main/kotlin/org/yapp/infra/InfraBaseConfigGroup.kt (2)
3-4: import 문 업데이트 승인FeignConfig에서 RestClientConfig로의 import 변경이 올바르게 적용되었습니다.
14-14: enum 상수 변경 승인OAUTH에서 REST_CLIENT로의 enum 상수 변경과 RestClientConfig 클래스 참조가 일관성 있게 적용되었습니다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinRestClient.kt (1)
20-30: OptResult 처리 로직 승인OptResult 파라미터가 List일 때 쉼표로 구분하여 처리하는 로직이 올바르게 구현되었습니다. 알라딘 API의 요구사항에 맞게 잘 처리되었습니다.
infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt (2)
16-21: 메서드 구현이 깔끔하고 적절합니다
runCatching을 사용한 예외 처리와 요청 객체의toMap()메서드 활용이 좋습니다.
23-28: lookupBook 메서드 구현 양호searchBooks와 동일한 패턴으로 일관성 있게 구현되었습니다.
| .setConnectionRequestTimeout(Timeout.ofDays(5000)) | ||
| .setResponseTimeout(Timeout.ofDays(5000)) |
There was a problem hiding this comment.
비현실적인 타임아웃 설정 수정 필요
5000일로 설정된 타임아웃이 비현실적입니다. 외부 API 호출에 적절한 타임아웃 값으로 수정하세요.
- .setConnectionRequestTimeout(Timeout.ofDays(5000))
- .setResponseTimeout(Timeout.ofDays(5000))
+ .setConnectionRequestTimeout(Timeout.ofSeconds(30))
+ .setResponseTimeout(Timeout.ofSeconds(60))📝 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.
| .setConnectionRequestTimeout(Timeout.ofDays(5000)) | |
| .setResponseTimeout(Timeout.ofDays(5000)) | |
| .setConnectionRequestTimeout(Timeout.ofSeconds(30)) | |
| .setResponseTimeout(Timeout.ofSeconds(60)) |
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/config/external/api/RestClientConfig.kt
at lines 33 to 34, the connection request and response timeouts are set to an
unrealistic 5000 days. Adjust these timeout values to a reasonable duration
suitable for external API calls, such as a few seconds or minutes, to prevent
excessively long waits and potential resource blocking.
| @Bean("aladinApiRestClient") | ||
| fun aladinRestClient(): RestClient { | ||
| return createConfiguredRestClientBuilder() | ||
| .baseUrl("http://www.aladin.co.kr/ttb/api") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 URL을 설정으로 분리하고 HTTPS 사용 권장
base URL이 하드코딩되어 있어 환경별 설정이 불가능하며, HTTP 프로토콜 사용으로 인한 보안 우려가 있습니다.
설정 파일로 분리하고 HTTPS를 사용하세요:
+@Value("\${aladin.api.base-url:https://www.aladin.co.kr/ttb/api}")
+private val baseUrl: String
+
- .baseUrl("http://www.aladin.co.kr/ttb/api")
+ .baseUrl(baseUrl)🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/config/external/api/RestClientConfig.kt
at line 27, the base URL is hardcoded with an HTTP address, which prevents
environment-specific configuration and poses security risks. Refactor the code
to load the base URL from a configuration file or environment variable, and
update the URL to use HTTPS instead of HTTP.
| class AladinApi( | ||
| private val aladinRestClient: AladinRestClient, | ||
| @Value("\${aladin.api.ttbkey:#{null}}") | ||
| private var ttbKey: String? = null | ||
| ) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 키 null 체크 보완 필요
ttbKey가 null일 수 있는 상황에서 외부 API 호출이 실패할 수 있습니다. 초기화 시점에서 API 키 유효성을 검증하거나, 각 메서드에서 null 체크를 추가하는 것을 고려해보세요.
@Component
class AladinApi(
private val aladinRestClient: AladinRestClient,
@Value("\${aladin.api.ttbkey:#{null}}")
private var ttbKey: String? = null
) {
+ init {
+ requireNotNull(ttbKey) { "Aladin API key (ttbkey) is required" }
+ }📝 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.
| class AladinApi( | |
| private val aladinRestClient: AladinRestClient, | |
| @Value("\${aladin.api.ttbkey:#{null}}") | |
| private var ttbKey: String? = null | |
| ) { | |
| class AladinApi( | |
| private val aladinRestClient: AladinRestClient, | |
| @Value("\${aladin.api.ttbkey:#{null}}") | |
| private var ttbKey: String? = null | |
| ) { | |
| init { | |
| requireNotNull(ttbKey) { "Aladin API key (ttbkey) is required" } | |
| } | |
| // … rest of the class … |
🤖 Prompt for AI Agents
In infra/src/main/kotlin/org/yapp/infra/external/aladin/AladinApi.kt around
lines 11 to 15, the ttbKey variable can be null, which may cause failures when
calling external APIs. To fix this, add a validation check for ttbKey during
initialization to ensure it is not null or empty, throwing an exception if
invalid. Alternatively, add null checks in each method that uses ttbKey to
handle the null case gracefully before making API calls.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookDetailRequest.kt (1)
9-14: nullable 필드에 대한 검증 어노테이션 문제
itemId가 nullable(String?)이면서@NotBlank어노테이션이 적용되어 있습니다. 이는 과거 리뷰에서 지적된 것과 동일한 문제입니다.nullable 필드에 대해서는 Bean Validation이 제대로 동작하지 않을 수 있습니다. 현재 nullable + Valid method pattern을 사용하고 있다면 일관성을 유지해야 합니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookDetailRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build-validation
🔇 Additional comments (3)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt (1)
18-30: 변환 로직이 잘 구현되어 있습니다
toAladinRequest()메소드가 내부 DTO에서 외부 API DTO로의 변환을 명확하게 처리하고 있습니다. 계층 간 책임 분리가 잘 되어 있습니다.apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (2)
22-26: 검색 엔드포인트가 잘 구현되었습니다
@Valid @ModelAttribute를 사용한 요청 검증과 UseCase 패턴을 통한 비즈니스 로직 분리가 잘 되어 있습니다. 계층 간 책임이 명확하게 분리되어 있습니다.
28-34: 상세 조회 엔드포인트가 잘 구현되었습니다책 상세 조회 엔드포인트도 검색 엔드포인트와 일관된 패턴을 따르고 있어 좋습니다. 코드의 일관성이 잘 유지되고 있습니다.
| val categoryId: Int? = null | ||
| ) { | ||
|
|
||
| fun validQuery(): String = query!! |
There was a problem hiding this comment.
🛠️ Refactor suggestion
안전한 검증 방법으로 개선 필요
validQuery() 메소드에서 query!!와 같은 non-null assertion을 사용하는 것은 위험할 수 있습니다. validation이 실패하거나 호출 순서가 잘못되면 KotlinNullPointerException이 발생할 수 있습니다.
-fun validQuery(): String = query!!
+fun validQuery(): String = query ?: throw IllegalArgumentException("Query parameter is required")또는 validation 통과를 보장하는 안전한 방법을 사용하는 것을 권장합니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookSearchRequest.kt at
line 17, the validQuery() method uses a non-null assertion (!!) on query, which
can cause a KotlinNullPointerException if query is null. To fix this, replace
the non-null assertion with a safe validation check that ensures query is not
null before returning it, or throw a controlled exception with a clear message
if query is null, thereby avoiding unsafe calls.
|
|
||
| val PROFILE_IMAGE_URL_PATTERN = Regex("^https?://[a-zA-Z0-9.-]+(/.*)?$") | ||
|
|
||
| const val NOT_BLANK_AND_NOT_NULL_STRING_PATTERN = "^(?!null$|NULL$|\\s*$).+" // Removed the old ISBN pattern |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
상수명과 패턴의 의도가 명확하지 않음
NOT_BLANK_AND_NOT_NULL_STRING_PATTERN 상수명이 실제 패턴이 수행하는 역할을 정확히 나타내지 못하고 있습니다. 이 패턴은 "null", "NULL", 또는 공백만 있는 문자열을 제외하는 것으로 보이는데, 상수명에서는 이런 의도가 명확하지 않습니다.
-const val NOT_BLANK_AND_NOT_NULL_STRING_PATTERN = "^(?!null$|NULL$|\\s*$).+" // Removed the old ISBN pattern
+const val NON_NULL_NON_BLANK_STRING_PATTERN = "^(?!null$|NULL$|\\s*$).+" // Excludes null strings and blank strings또는 더 구체적인 용도를 나타내는 이름을 사용하는 것을 권장합니다.
📝 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.
| const val NOT_BLANK_AND_NOT_NULL_STRING_PATTERN = "^(?!null$|NULL$|\\s*$).+" // Removed the old ISBN pattern | |
| const val NON_NULL_NON_BLANK_STRING_PATTERN = "^(?!null$|NULL$|\\s*$).+" // Excludes null strings and blank strings |
🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt at line
9, the constant name NOT_BLANK_AND_NOT_NULL_STRING_PATTERN does not clearly
convey that the pattern excludes strings that are "null", "NULL", or only
whitespace. Rename the constant to a more descriptive name that explicitly
reflects this behavior, such as EXCLUDE_NULL_OR_BLANK_STRING_PATTERN, to improve
code readability and intent clarity.
| ) { | ||
| fun toAladinRequest(): AladinBookLookupRequest { | ||
| return AladinBookLookupRequest.create( | ||
| itemId = this.itemId!!, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
안전하지 않은 non-null assertion 사용
toAladinRequest() 메소드에서 this.itemId!!와 같은 non-null assertion을 사용하는 것은 위험합니다. validation이 실패하거나 예상치 못한 상황에서 런타임 예외가 발생할 수 있습니다.
-itemId = this.itemId!!,
+itemId = this.itemId ?: throw IllegalArgumentException("Item ID is required"),또는 validation 통과를 보장하는 더 안전한 방법을 사용하는 것을 권장합니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookDetailRequest.kt at
line 20, the use of non-null assertion (!!) on this.itemId is unsafe and can
cause runtime exceptions if itemId is null. To fix this, ensure itemId is
validated before this method is called or refactor the method to safely handle
null values, such as using a safe call with an appropriate fallback or throwing
a controlled exception with a clear message instead of using !!.
| @RestController | ||
| @RequestMapping("/api/v1/books") | ||
| class BookController( | ||
| private val bookUseCase: BookUseCase | ||
| ) : BookControllerApi { | ||
|
|
||
| @GetMapping("/search") | ||
| override fun searchBooks(@Valid @ModelAttribute request: BookSearchRequest): ResponseEntity<BookSearchResponse> { | ||
| val response = bookUseCase.searchBooks(request) | ||
| return ResponseEntity.ok(response) | ||
| } | ||
|
|
||
| @GetMapping("/detail") | ||
| override fun getBookDetail( | ||
| @Valid @ModelAttribute request: BookDetailRequest | ||
| ): ResponseEntity<BookDetailResponse> { | ||
| val response = bookUseCase.getBookDetail(request) | ||
| return ResponseEntity.ok(response) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
예외 처리 고려 제안
현재 controller에서는 성공 케이스만 처리하고 있습니다. UseCase나 하위 계층에서 발생할 수 있는 예외에 대한 처리를 고려해보세요.
GlobalExceptionHandler에서 공통 예외 처리를 하고 있다면 현재 구조도 충분하지만, 비즈니스 로직 특화된 예외 처리가 필요한 경우 추가 고려가 필요합니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt around
lines 16 to 35, the controller currently only handles successful responses
without managing exceptions from the use case layer. To fix this, add try-catch
blocks around the calls to bookUseCase methods to catch and handle
business-specific exceptions, returning appropriate error responses.
Alternatively, if a GlobalExceptionHandler is used, ensure it covers all
relevant exceptions; otherwise, implement specific exception handling in the
controller to manage business logic errors properly.
move-hoon
changed the title
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
도서 상세 조회 기능 강화 (/api/v1/books/{itemId}):
optResult 쿼리 파라미터를 추가하여 책 소개, 목차, 미리보기 이미지 등 도서의 상세 부가 정보를 선택적으로 조회할 수 있도록 기능을 확장했어요.
Controller, UseCase, Service, AladinApiHelper, AladinApi 전 계층에 걸쳐 optResult 파라미터를 투명하게 전달하도록 로직을 수정했어요.
AladinApi에서 optResult: List? 타입을 Map<String, Any> 형태로 변환하여 AladinRestClient에 전달하도록 구현했어요. 이는 외부 API 파라미터 처리 로직의 효율성과 재사용성을 높여요.
도서 관련 Swagger API 명세 개선:
BookControllerApi를 업데이트하여 도서 검색 및 상세 조회 API에 대한 Swagger 문서를 더욱 명확하고 간결하게 작성했어요.
각 파라미터에 대한 상세 설명과 함께 실제 요청 예시를 추가하여 API 사용 편의성을 높였어요.
도서 도메인 및 엔티티 모델 일관성 확보:
Book 도메인 모델에 createdAt, updatedAt, deletedAt 필드를 추가하여 User 도메인 모델과 동일한 감사(audit) 필드 구조를 가지도록 통일했어요.
Book 도메인의 create 및 reconstruct 팩토리 메서드를 업데이트하여 타임스탬프 필드를 일관되게 처리하도록 했어요.
UserEntity와 동일한 패턴(private constructor, toDomain(), fromDomain(), equals()/hashCode())을 따르는 BookEntity를 신규 생성했어요.
BookEntity의 영속성 계층 구현 시 isbn을 Primary Key로 활용하고, BaseTimeEntity를 상속받아 감사 필드가 자동으로 관리되도록 설정했어요.
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신규 기능
버그 수정
환경 및 설정
기타