feat: Guest용 도서 조회 API 구현 #102
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough게스트용 도서 검색 API를 추가하고, 기존 회원용 검색 API는 인증 userId를 요구하도록 시그니처를 변경했습니다. 게스트 응답 DTO를 신설하고, 회원용 응답 DTO를 확장했습니다. 유스케이스와 컨트롤러/인터페이스를 이에 맞게 수정했으며, 게이트웨이 화이트리스트를 /guest/search 경로로 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Guest as Client(Guest)
participant Gateway
participant API as BookController
participant UC as BookUseCase
participant Query as BookQueryService
Guest->>Gateway: GET /api/v1/books/guest/search?...
Gateway->>API: Forward (public)
API->>UC: searchBooksForGuest(request)
UC->>Query: search(request)
Query-->>UC: BookSearchResponse
UC-->>API: GuestBookSearchResponse.from(BookSearchResponse)
API-->>Guest: 200 OK (GuestBookSearchResponse)
sequenceDiagram
participant User as Client(Authenticated)
participant Gateway
participant API as BookController
participant UC as BookUseCase
participant Query as BookQueryService
participant Status as UserBookStatusService
User->>Gateway: GET /api/v1/books/search?...
Gateway->>API: Forward (authenticated userId)
API->>UC: searchBooks(request, userId)
UC->>Query: search(request)
Query-->>UC: BookSearchResponse
UC->>Status: mergeWithUserBookStatus(books, userId)
Status-->>UC: books with user status
UC-->>API: BookSearchResponse
API-->>User: 200 OK (BookSearchResponse)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, 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: 11
🔭 Outside diff range comments (2)
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (2)
102-107: OpenAPI 스키마 개선: link의 URI 포맷 명시 및 예시를 https로문서 정확도를 위해 link 필드에 URI 포맷을 명시하고, 예시는 가능하면 https로 제공하는 것을 권장합니다.
@field:Schema( - description = "알라딘 도서 상세 페이지 링크", - example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" + description = "알라딘 도서 상세 페이지 링크", + example = "https://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175", + format = "uri" ) val link: String,
115-137: 외부 링크 필드 안정성 확인: null/blank 가능성 및 정규화 여부Aladin 응답의 link가 항상 비어있지 않은 문자열로 온다는 보장이 있는지 확인 부탁드립니다. null/blank가 유입될 수 있다면 현재 시그니처(비nullable String)와 require 검증 부재로 인해 불일치가 발생할 수 있습니다. 필요 시 다음을 검토하세요.
- Query 계층에서 link 기본값/정규화 적용
- or of(...)에서 빈 문자열 대체/validation 추가
아울러 도메인 정책상 http를 https로 강제 전환하거나, 허용 도메인 화이트리스트를 적용할지 여부도 검토 가치가 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt(2 hunks)apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt(3 hunks)apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt(2 hunks)gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt(1 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 (7)
gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt (1)
24-26: 게스트 검색 경로 화이트리스트 추가 적절회원용(/search)은 보호되고, 게스트용(/guest/search)은 공개로 분리되어 보안 의도가 잘 반영되었습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/BookSearchResponse.kt (1)
53-66: isLastPage 전달 로직의 경계값(오프바이원) 확인 요청외부 응답(Aladin) 기반의 페이지네이션에서 마지막 페이지 판단은 0/1 기반 인덱스 차이로 쉽게 오프바이원이 발생합니다. isLastPage 계산이 다음과 같은 케이스에서 정확한지 확인 부탁드립니다.
- totalResults가 0일 때
- 마지막 결과가 정확히 pageSize 배수로 끝날 때
- 마지막 페이지 직전/직후 경계
해당 계산은 QueryService 쪽에서 수행될 가능성이 높으니, 거기서의 로직/테스트도 함께 점검해 주세요.
apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt (1)
30-35: 게스트 전용 검색 분리 설계 적절게스트용 DTO로 변환을 UseCase에서 담당해 컨트롤러 단을 간결하게 유지했습니다. 회원 데이터(상태) 의존성을 제거한 경계 설정이 명확합니다.
apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt (1)
41-47: @AuthenticationPrincipal로 UUID 바인딩 확인컨트롤러 시그니처가 UUID를 직접 받도록 변경되었습니다. Gateway/Resource Server의 JwtAuthenticationConverter가 UUID를 principal로 매핑하도록 일관되게 설정되어 있는지(예: sub -> UUID) 확인 부탁드립니다. 일치하지 않으면 바인딩 실패가 발생할 수 있습니다.
apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt (3)
6-10: 게스트 전용 DTO 신설 방향 적절합니다응답 전용 DTO를 분리하고 생성자를 private으로 숨긴 뒤 팩토리로만 생성하도록 한 설계는 기존 API와의 충돌을 피하면서도 사용 범위를 명확히 제한해줍니다. OpenAPI 메타데이터도 충분합니다.
71-73: private 생성자 + 직렬화 호환성 확인응답 전용 직렬화에는 문제가 없으나, 혹시라도 이 DTO를 역직렬화(요청 바인딩)에 재사용하지는 않는지 확인 바랍니다. 역직렬화가 필요하다면
@JsonCreator또는 public ctor가 필요해질 수 있습니다.
45-46: BookSearchResponse import 및 nullability 검증 완료
- GuestBookSearchResponse.kt와 BookSearchResponse.kt가 동일 패키지(
org.yapp.apis.book.dto.response)에 위치하여 별도 import 없이도 참조 가능합니다.lastPage: Boolean및books: List<…>필드는 모두 non-nullable으로 선언되어 있어 추가 null 체크가 불필요합니다.
Comment on lines
+33
to
+39
| @GetMapping("/guest/search") | ||
| override fun searchBooksForGuest( | ||
| @Valid @ModelAttribute request: BookSearchRequest | ||
| ): ResponseEntity<GuestBookSearchResponse> { | ||
| val response = bookUseCase.searchBooksForGuest(request) | ||
| return ResponseEntity.ok(response) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
게스트 검색 엔드포인트 추가 적절 + 남용 방지 전략 고려
API 분리는 명확합니다. 공개 엔드포인트인 만큼 트래픽 남용(스크래핑, 대량 호출) 대비를 위해 게이트웨이/인프라 레벨에서의 레이트 리밋, 캐시 전략 등을 함께 검토해 주세요.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookController.kt around
lines 33–39, you added a public guest search endpoint but the review requests
safeguards against abuse; implement rate limiting and caching: add a rate-limit
mechanism (either configure gateway/ingress rules or add a server-side limiter
such as Bucket4j/Resilience4j filter or servlet filter tied to IP/API key) to
throttle requests, apply caching for identical search queries (use @Cacheable on
the use-case method or add appropriate Cache-Control response headers) to reduce
load, validate and enforce sensible request limits (page size, max query length)
in BookSearchRequest, and update API docs/infra config with the chosen
rate-limit and cache settings.
Comment on lines
31
to
+53
| @Operation( | ||
| summary = "도서 검색", description = "알라딘 API를 통해 키워드로 도서를 검색합니다. \n" + | ||
| summary = "비회원 도서 검색", description = "알라딘 API를 통해 키워드로 도서를 검색합니다. \n" + | ||
| " 비회원이기에 도서 상태(읽음, 읽는 중 등)은 표시되지 않습니다. " | ||
| ) | ||
| @ApiResponses( | ||
| value = [ | ||
| ApiResponse( | ||
| responseCode = "200", | ||
| description = "성공적인 검색", | ||
| content = [Content(schema = Schema(implementation = GuestBookSearchResponse::class))] | ||
| ), | ||
| ApiResponse( | ||
| responseCode = "400", | ||
| description = "잘못된 요청 파라미터", | ||
| content = [Content(schema = Schema(implementation = ErrorResponse::class))] | ||
| ) | ||
| ] | ||
| ) | ||
| @GetMapping("/guest/search") | ||
| fun searchBooksForGuest( | ||
| @Valid @Parameter(description = "도서 검색 요청 객체") request: BookSearchRequest | ||
| ): ResponseEntity<GuestBookSearchResponse> | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
게스트용 API 스펙 명료화 좋습니다
게스트 응답 DTO를 별도 분리하고 비회원 제약(도서 상태 비노출)을 명확히 기재한 점 좋습니다. 페이지네이션 필드(lastPage, itemsPerPage 등)가 응답에 포함됨을 문서에도 간단히 언급하면 소비자 입장에서 더 명확해집니다.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
around lines 31 to 53, the OpenAPI @Operation description for the guest search
endpoint should explicitly mention that the response includes pagination fields
(e.g., lastPage, itemsPerPage, currentPage, totalItems) so consumers know paging
metadata is provided; update the description string to append a short sentence
listing the pagination fields included in GuestBookSearchResponse (or adjust
wording to say "응답에 lastPage, itemsPerPage 등을 포함한 페이지네이션 메타데이터가 포함됩니다") and
ensure the schema annotation remains unchanged.
Comment on lines
72
to
76
| @GetMapping("/search") | ||
| fun searchBooks( | ||
| @AuthenticationPrincipal userId: UUID, | ||
| @Valid @Parameter(description = "도서 검색 요청 객체") request: BookSearchRequest | ||
| ): ResponseEntity<BookSearchResponse> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
회원 검색에 대한 보안 스키마 노출(스웨거) 제안
실제 보안은 잘 적용되어 있지만, 스웨거 문서에도 보안 요구 사항을 노출하면 더 명확합니다. springdoc 기준으로 SecurityRequirement를 추가하세요(스키마 이름은 프로젝트 설정에 맞춰 조정 필요).
예시:
@Operation(
summary = "...",
security = [SecurityRequirement(name = "bearerAuth")]
)🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/controller/BookControllerApi.kt
around lines 72 to 76, the controller method exposes authentication in runtime
but does not declare the security requirement in the generated OpenAPI/Swagger
docs; add a springdoc @Operation annotation to the method including security =
[SecurityRequirement(name = "bearerAuth")] (adjust the schema name to match
project settings), keep or add summary/description as needed, and ensure the
io.swagger.v3.oas.annotations.security.SecurityRequirement import is present so
the generated docs show the bearer auth requirement.
Comment on lines
+38
to
+43
| @field:Schema(description = "마지막 페이지 여부", example = "false") | ||
| val lastPage: Boolean, | ||
|
|
||
| @field:Schema(description = "검색된 책 목록 (게스트용)") | ||
| val books: List<GuestBookSummary> | ||
| ) { |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
OpenAPI 스키마에서 필수 필드 표시
게스트 응답의 lastPage, books는 논리적으로 필수입니다. 스키마에 명시하면 클라이언트/문서 일관성이 좋아집니다.
- @field:Schema(description = "마지막 페이지 여부", example = "false")
+ @field:Schema(description = "마지막 페이지 여부", example = "false", requiredMode = Schema.RequiredMode.REQUIRED)
val lastPage: Boolean,
- @field:Schema(description = "검색된 책 목록 (게스트용)")
+ @field:Schema(description = "검색된 책 목록 (게스트용)", requiredMode = Schema.RequiredMode.REQUIRED)
val books: List<GuestBookSummary>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @field:Schema(description = "마지막 페이지 여부", example = "false") | |
| val lastPage: Boolean, | |
| @field:Schema(description = "검색된 책 목록 (게스트용)") | |
| val books: List<GuestBookSummary> | |
| ) { | |
| @field:Schema(description = "마지막 페이지 여부", example = "false", requiredMode = Schema.RequiredMode.REQUIRED) | |
| val lastPage: Boolean, | |
| @field:Schema(description = "검색된 책 목록 (게스트용)", requiredMode = Schema.RequiredMode.REQUIRED) | |
| val books: List<GuestBookSummary> | |
| ) { |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt
around lines 38 to 43, the OpenAPI schema doesn't mark the logically required
properties lastPage and books as required; update the @field:Schema annotations
for both properties to include required = true (e.g., @field:Schema(description
= "...", example = "...", required = true)) so the generated OpenAPI spec and
client docs show these fields as mandatory.
Comment on lines
+44
to
+69
| companion object { | ||
| fun from(response: BookSearchResponse): GuestBookSearchResponse { | ||
| return GuestBookSearchResponse( | ||
| version = response.version, | ||
| title = response.title, | ||
| pubDate = response.pubDate, | ||
| totalResults = response.totalResults, | ||
| startIndex = response.startIndex, | ||
| itemsPerPage = response.itemsPerPage, | ||
| query = response.query, | ||
| searchCategoryId = response.searchCategoryId, | ||
| searchCategoryName = response.searchCategoryName, | ||
| lastPage = response.lastPage, | ||
| books = response.books.map { userBookSummary -> | ||
| GuestBookSummary.of( | ||
| isbn13 = userBookSummary.isbn13, | ||
| title = userBookSummary.title, | ||
| author = userBookSummary.author, | ||
| publisher = userBookSummary.publisher, | ||
| coverImageUrl = userBookSummary.coverImageUrl, | ||
| link = userBookSummary.link | ||
| ) | ||
| } | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
단위 테스트 추가 제안: 매핑/정규화 검증
게스트 DTO 팩토리의 핵심 로직이므로, 최소한 다음 케이스에 대한 단위 테스트를 권장합니다:
- books=null일 때 빈 리스트 매핑
- lastPage=null일 때 계산/기본값 적용
- publisher 괄호/공백 정리 확인
필요하시면 테스트 스캐폴딩을 생성해 드리겠습니다.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt
around lines 44 to 69, add null-safe mapping and normalization and add unit
tests: ensure response.books is handled with response.books?.map { ... } ?:
emptyList() so a null books yields an empty list; ensure lastPage is derived or
defaulted when response.lastPage is null (apply the same calculation or default
used elsewhere); normalize publisher by trimming and stripping surrounding
parentheses/extra spaces when mapping each book; then add unit tests covering
books=null, lastPage=null, and publisher containing parentheses/extra whitespace
to verify mapping/normalization behavior.
Comment on lines
+57
to
+64
| books = response.books.map { userBookSummary -> | ||
| GuestBookSummary.of( | ||
| isbn13 = userBookSummary.isbn13, | ||
| title = userBookSummary.title, | ||
| author = userBookSummary.author, | ||
| publisher = userBookSummary.publisher, | ||
| coverImageUrl = userBookSummary.coverImageUrl, | ||
| link = userBookSummary.link |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
변수명 간결화 제안: userBookSummary → book
게스트 DTO 변환 컨텍스트에서 userBookSummary는 혼동 여지가 있습니다. book처럼 일반화된 이름이 가독성에 더 유리합니다. 위의 보강 diff에 반영했습니다.
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt
around lines 57 to 64, the lambda parameter name userBookSummary is overly
verbose and should be renamed to book for clarity; update the map {
userBookSummary -> ... } to map { book -> ... } and replace all occurrences of
userBookSummary.isbn13, .title, .author, .publisher, .coverImageUrl, and .link
inside the lambda with book.isbn13, book.title, book.author, book.publisher,
book.coverImageUrl, and book.link respectively to keep naming concise and
consistent.
Comment on lines
+93
to
+95
| description = "알라딘 도서 상세 페이지 링크", | ||
| example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
예시 링크 https 권장
문서 예시 값은 가급적 https를 권장합니다.
- example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175"
+ example = "https://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175"📝 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
| description = "알라딘 도서 상세 페이지 링크", | |
| example = "http://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" | |
| ) | |
| description = "알라딘 도서 상세 페이지 링크", | |
| example = "https://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175" | |
| ) |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt
around lines 93 to 95, the example URL uses http; update the example to use an
https link (e.g. https://www.aladin.co.kr/shop/wproduct.aspx?ItemId=3680175) so
the documented example prefers secure scheme—simply replace the example string
with the https variant while keeping the description unchanged.
Comment on lines
+98
to
+116
| companion object { | ||
| fun of( | ||
| isbn13: String, | ||
| title: String, | ||
| author: String?, | ||
| publisher: String?, | ||
| coverImageUrl: String, | ||
| link: String | ||
| ): GuestBookSummary { | ||
| return GuestBookSummary( | ||
| isbn13 = isbn13, | ||
| title = title, | ||
| author = author, | ||
| publisher = publisher?.let { BookDataValidator.removeParenthesesFromPublisher(it) }, | ||
| coverImageUrl = coverImageUrl, | ||
| link = link | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
publisher 정규화 추가 보강: trim/blank 처리
출판사 값이 공백이거나 공백만 있는 경우 null로 정규화하면 응답 품질이 좋아집니다. 현재 괄호 제거만 하므로 최소한의 전처리를 더하는 것을 제안합니다.
- return GuestBookSummary(
- isbn13 = isbn13,
- title = title,
- author = author,
- publisher = publisher?.let { BookDataValidator.removeParenthesesFromPublisher(it) },
- coverImageUrl = coverImageUrl,
- link = link
- )
+ val normalizedPublisher = publisher
+ ?.trim()
+ ?.takeIf { it.isNotBlank() }
+ ?.let(BookDataValidator::removeParenthesesFromPublisher)
+ return GuestBookSummary(
+ isbn13 = isbn13,
+ title = title,
+ author = author,
+ publisher = normalizedPublisher,
+ coverImageUrl = coverImageUrl,
+ link = link
+ )📝 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
| companion object { | |
| fun of( | |
| isbn13: String, | |
| title: String, | |
| author: String?, | |
| publisher: String?, | |
| coverImageUrl: String, | |
| link: String | |
| ): GuestBookSummary { | |
| return GuestBookSummary( | |
| isbn13 = isbn13, | |
| title = title, | |
| author = author, | |
| publisher = publisher?.let { BookDataValidator.removeParenthesesFromPublisher(it) }, | |
| coverImageUrl = coverImageUrl, | |
| link = link | |
| ) | |
| } | |
| } | |
| companion object { | |
| fun of( | |
| isbn13: String, | |
| title: String, | |
| author: String?, | |
| publisher: String?, | |
| coverImageUrl: String, | |
| link: String | |
| ): GuestBookSummary { | |
| val normalizedPublisher = publisher | |
| ?.trim() | |
| ?.takeIf { it.isNotBlank() } | |
| ?.let(BookDataValidator::removeParenthesesFromPublisher) | |
| return GuestBookSummary( | |
| isbn13 = isbn13, | |
| title = title, | |
| author = author, | |
| publisher = normalizedPublisher, | |
| coverImageUrl = coverImageUrl, | |
| link = link | |
| ) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
apis/src/main/kotlin/org/yapp/apis/book/dto/response/GuestBookSearchResponse.kt
around lines 98 to 116, the publisher normalization only strips parentheses but
doesn't trim whitespace or convert blank-only strings to null; update the
publisher handling to trim the string after removing parentheses and return null
if the trimmed result is blank (e.g., publisher?.let {
BookDataValidator.removeParenthesesFromPublisher(it).trim().takeIf {
it.isNotBlank() } }) so the GuestBookSummary.publisher is normalized to null for
empty/blank values.
Comment on lines
+37
to
45
| fun searchBooks( | ||
| request: BookSearchRequest, | ||
| userId: UUID | ||
| ): BookSearchResponse { | ||
| val searchResponse = bookQueryService.searchBooks(request) | ||
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, null) | ||
| val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId) | ||
|
|
||
| return searchResponse.withUpdatedBooks(booksWithUserStatus) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
회원 검색에서 사용자 검증/널 처리 일관성 및 단순화 제안
- getBookDetail 등 다른 경로에서는 userService.validateUserExists(userId)를 호출하는데, 검색에서는 생략되어 있습니다. 의도(성능 최적화 vs 일관된 404 전략)를 문서화하거나 확인 부탁드립니다. 비존재 사용자에 대한 기대 동작을 팀 컨벤션에 맞추는 것이 좋습니다.
- searchBooks에서 userId는 non-null로 전달되므로, 아래 private 함수들의 userId: UUID? 및 null 처리 분기를 제거해 시그니처를 단순화할 수 있습니다.
아래와 같이 단순화할 수 있습니다.
- private fun mergeWithUserBookStatus(
- searchedBooks: List<BookSummary>,
- userId: UUID?
- ): List<BookSummary> {
- if (userId == null || searchedBooks.isEmpty()) {
- return searchedBooks
- }
+ private fun mergeWithUserBookStatus(
+ searchedBooks: List<BookSummary>,
+ userId: UUID
+ ): List<BookSummary> {
+ if (searchedBooks.isEmpty()) return searchedBooks
val isbn13s = searchedBooks.map { it.isbn13 }
val userBookStatusMap = getUserBookStatusMap(isbn13s, userId)
return searchedBooks.map { bookSummary ->
userBookStatusMap[bookSummary.isbn13]
?.let { bookSummary.updateStatus(it) }
?: bookSummary
}
}
- private fun getUserBookStatusMap(
- isbn13s: List<String>,
- userId: UUID?
- ): Map<String, BookStatus> {
- if (userId == null) return emptyMap()
+ private fun getUserBookStatusMap(
+ isbn13s: List<String>,
+ userId: UUID
+ ): Map<String, BookStatus> {
val userBooksResponse = userBookService.findAllByUserIdAndBookIsbn13In(
UserBooksByIsbn13sRequest.of(userId, isbn13s)
)
return userBooksResponse.associate { userBook ->
userBook.isbn13 to userBook.status
}
}사용자 존재 검증을 넣을지 여부는 아래와 같이 선택적으로 적용 가능합니다.
fun searchBooks(request: BookSearchRequest, userId: UUID): BookSearchResponse {
// 정책에 따라 활성화
// userService.validateUserExists(userId)
val searchResponse = bookQueryService.searchBooks(request)
val booksWithUserStatus = mergeWithUserBookStatus(searchResponse.books, userId)
return searchResponse.withUpdatedBooks(booksWithUserStatus)
}🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/usecase/BookUseCase.kt around lines
37 to 45, the searchBooks path omits the user existence validation used
elsewhere and passes a non-null userId into private functions typed as nullable;
update to either call userService.validateUserExists(userId) here (or document
why omitted) to match team convention, then simplify the private helper
signatures to accept userId: UUID (remove UUID? and eliminate null branches) and
update all internal usages accordingly so there is no redundant nullable
handling.
Comment on lines
24
to
26
| private val WHITELIST_URLS = arrayOf( | ||
| "/api/v1/books/search", | ||
| "/api/v1/books/guest/search", | ||
| "/api/v1/auth/refresh", |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
permitAll 경로에서 잘못된 토큰에 의한 401 가능성 점검
Spring Security에서 BearerTokenAuthenticationFilter는 Authorization 헤더가 존재하면 인증을 시도합니다. 게스트 공개 엔드포인트라도 잘못된 토큰이 실린 요청은 401이 날 수 있으니, UX 정책에 따라 다음 중 하나를 고려해 주세요.
- 공개 엔드포인트에서는 Authorization 헤더를 무시하도록 필터 추가
- 또는 문서화/모니터링으로 “잘못된 토큰은 공개 엔드포인트에서도 401” 정책을 명시
필터를 추가하는 예시는 아래와 같습니다.
// 새 필터 추가 예시 (별도 파일로 생성)
import jakarta.servlet.http.HttpServletRequest
import jakarta.servlet.http.HttpServletRequestWrapper
import org.springframework.util.AntPathMatcher
import org.springframework.web.filter.OncePerRequestFilter
import java.util.*
class SkipBearerOnWhitelistFilter(
private val whitelist: Array<String>
) : OncePerRequestFilter() {
private val matcher = AntPathMatcher()
override fun shouldNotFilter(request: HttpServletRequest): Boolean {
return whitelist.none { matcher.match(it, request.requestURI) }
}
override fun doFilterInternal(request: HttpServletRequest, response: jakarta.servlet.http.HttpServletResponse, filterChain: jakarta.servlet.FilterChain) {
val wrapped = object : HttpServletRequestWrapper(request) {
override fun getHeader(name: String?): String? {
return if (name.equals("Authorization", ignoreCase = true)) null else super.getHeader(name)
}
override fun getHeaders(name: String?): Enumeration<String> {
if (name.equals("Authorization", ignoreCase = true)) return Collections.emptyEnumeration()
return super.getHeaders(name)
}
}
filterChain.doFilter(wrapped, response)
}
}SecurityConfig 에 필터를 BearerTokenAuthenticationFilter 이전에 추가:
.authorizeHttpRequests {
it.requestMatchers(*WHITELIST_URLS).permitAll()
it.requestMatchers("/api/v1/admin/**").hasRole("ADMIN")
it.anyRequest().authenticated()
}
+.addFilterBefore(SkipBearerOnWhitelistFilter(WHITELIST_URLS), BearerTokenAuthenticationFilter::class.java)
.addFilterAfter(mdcLoggingFilter, BearerTokenAuthenticationFilter::class.java)🤖 Prompt for AI Agents
gateway/src/main/kotlin/org/yapp/gateway/security/SecurityConfig.kt lines 24-26:
permitAll endpoints may still trigger BearerTokenAuthenticationFilter when an
Authorization header is present, causing unintended 401s; add a
OncePerRequestFilter (e.g., SkipBearerOnWhitelistFilter) that matches requests
against WHITELIST_URLS and, for matches, strips/returns empty for the
Authorization header (override getHeader and getHeaders, handle case-insensitive
"Authorization"), then register this filter in SecurityConfig before
BearerTokenAuthenticationFilter so whitelist endpoints ignore bearer tokens and
avoid unwanted authentication attempts.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔗 관련 이슈
📘 작업 유형
📙 작업 내역
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
신기능
변경 사항