refactor: 저자 정보 추출 로직 개선 및 데이터 생성 안정성 강화 #104
Conversation
Walkthrough알라딘 API 응답 불규칙성에 대응해 기본값 적용과 정규식 기반 파싱으로 리팩터링함. BookCreateRequest에 기본값 제공 로직을 추가했고, AuthorExtractor는 RegexUtils의 패턴을 사용하도록 변경했으며 RegexUtils에 정규식 상수 3개를 추가하고 관련 Validator들이 이를 참조하도록 수정함. Changes
Sequence Diagram(s)sequenceDiagram
participant Aladin as Aladin API
participant DTO as BookCreateRequest
participant AE as AuthorExtractor
participant Utils as RegexUtils
Aladin-->>DTO: BookDetailResponse
DTO->>DTO: provideDefaultIfBlank(title/author/publisher/cover)
DTO->>AE: extractAuthors(authorString)
AE->>Utils: AUTHOR_MARKER_PATTERN
AE->>AE: getPartBeforeMarker() / cleanUpContributors()
AE-->>DTO: normalized authors
DTO->>DTO: parsePublicationYear(pubDate)
DTO-->>Aladin: BookCreateRequest (prepared)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
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: 6
🔭 Outside diff range comments (1)
global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt (1)
8-14: 괄호 제거 후 공백 정규화 누락괄호 제거 시 양옆 공백이 남거나 이중 공백이 생길 수 있습니다. 후처리로 trim 및 다중 공백 축소를 권장합니다.
- fun removeParenthesesFromAuthor(author: String): String { - return author.replace(PARENTHESIS_REGEX, "") - } + fun removeParenthesesFromAuthor(author: String): String { + return author + .replace(PARENTHESIS_REGEX, " ") + .trim() + .replace(Regex("\\s{2,}"), " ") + } @@ - fun removeParenthesesFromPublisher(publisher: String): String { - return publisher.replace(PARENTHESIS_REGEX, "") - } + fun removeParenthesesFromPublisher(publisher: String): String { + return publisher + .replace(PARENTHESIS_REGEX, " ") + .trim() + .replace(Regex("\\s{2,}"), " ") + }
📜 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 (5)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt(1 hunks)apis/src/main/kotlin/org/yapp/apis/book/util/AuthorExtractor.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt(1 hunks)global-utils/src/main/kotlin/org/yapp/globalutils/validator/EmailValidator.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 (3)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (1)
103-107: 기본값 적용으로 @notblank 실패 방지 — LGTM외부 응답 값이 null/blank인 경우 합리적 기본값을 주입하는 방향이 유효합니다. 나머지 필드와의 일관성도 좋습니다.
global-utils/src/main/kotlin/org/yapp/globalutils/validator/EmailValidator.kt (1)
3-7: 정규식 중앙화로 중복 제거 — LGTMRegexUtils로의 위임으로 패턴 일관성과 유지보수성이 개선되었습니다.
global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt (1)
5-7: 정규식 중앙화 — LGTM패턴 소스를 단일화한 방향성 적절합니다.
| private const val UNKNOWN_AUTHOR = "저자 정보 없음" | ||
| private const val UNKNOWN_PUBLISHER = "출판사 정보 없음" | ||
| private const val DEFAULT_COVER_IMAGE = "https://github.com/user-attachments/assets/7ba556a4-3a76-4f27-aecb-e58924e66843" | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
상수의 위치/국제화 및 기본 표지 URL의 안정성 점검 필요
- "저자 정보 없음", "출판사 정보 없음"은 도메인/언어 의존적 텍스트입니다. 도메인 상수 또는 i18n 리소스로 외부화(예: 설정/메시지 번들)해 재사용성과 다국어 대응성을 확보하는 것이 바람직합니다.
- DEFAULT_COVER_IMAGE가 GitHub user-attachments 정적 URL인 점은 가용성/속도/라이선스 측면에서 리스크가 있습니다. 사내 CDN/S3 등 관리되는 스토리지로 이전하거나 환경설정으로 주입 가능하게 해 주세요.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt
around lines 93-96, the hard-coded Korean text constants and the GitHub static
image URL should be externalized: move "저자 정보 없음" and "출판사 정보 없음" out of this
DTO into a domain constants holder or, preferably, the i18n message bundle
(message keys used here and resolved by the localization layer) so they can be
reused and translated; replace DEFAULT_COVER_IMAGE with a configurable property
(inject from application config/environment or a CDN/S3 URL stored in
configuration) and read it via the app configuration class or a service, then
remove the hard-coded literal from this file.
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt
Show resolved
Hide resolved
| val partBeforeMarker = getPartBeforeMarker(authorString) ?: return "" | ||
| val finalAuthor = cleanUpContributors(partBeforeMarker) | ||
|
|
||
| return finalAuthor.trim() | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
마커 미존재 시 빈 문자열 반환은 과도함 — 폴백 로직 추가 권장
마커가 없는 응답(예: "홍길동, 김철수")에서도 최대한 저자명을 보전해야 합니다. 현재는 빈 문자열("")을 반환해 정보 유실이 발생합니다. 폴백: '(' 또는 ',' 이전까지, 없으면 전체를 사용.
- val partBeforeMarker = getPartBeforeMarker(authorString) ?: return ""
- val finalAuthor = cleanUpContributors(partBeforeMarker)
-
- return finalAuthor.trim()
+ val partBeforeMarker = getPartBeforeMarker(authorString)
+ val finalAuthor = cleanUpContributors(partBeforeMarker)
+ return finalAuthor.trim()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/util/AuthorExtractor.kt around lines
17 to 21, the current code returns an empty string when
getPartBeforeMarker(authorString) is null, causing loss of author info; change
the logic so that when getPartBeforeMarker(...) returns null you fallback to
extracting up to the first '(' or ',' (whichever comes first) from authorString,
and if neither exists use the whole authorString, then pass that fallback into
cleanUpContributors(...) and trim the result before returning.
| private fun getPartBeforeMarker(text: String): String? { | ||
| val parts = text.split(AUTHOR_MARKER_REGEX, limit = EXPECTED_PARTS_ON_SPLIT) | ||
|
|
||
| if (authorsPart.contains(CLOSING_PAREN)) { | ||
| authorsPart = authorsPart.substringAfterLast(DELIMITER) | ||
| return if (parts.size == EXPECTED_PARTS_ON_SPLIT) { | ||
| parts[AUTHOR_PART_INDEX] | ||
| } else { | ||
| null | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
getPartBeforeMarker 시그니처/폴백 개선
함수에서 null을 반환하기보다 의미 있는 기본값을 계산해 반환하도록 변경하면 호출부 단순화 및 데이터 보존성이 높아집니다.
- private fun getPartBeforeMarker(text: String): String? {
- val parts = text.split(AUTHOR_MARKER_REGEX, limit = EXPECTED_PARTS_ON_SPLIT)
-
- return if (parts.size == EXPECTED_PARTS_ON_SPLIT) {
- parts[AUTHOR_PART_INDEX]
- } else {
- null
- }
- }
+ private fun getPartBeforeMarker(text: String): String {
+ val parts = text.split(AUTHOR_MARKER_REGEX, limit = EXPECTED_PARTS_ON_SPLIT)
+ return if (parts.size == EXPECTED_PARTS_ON_SPLIT) {
+ parts[AUTHOR_PART_INDEX]
+ } else {
+ // 폴백: 첫 '(' 또는 ',' 이전까지, 모두 없으면 원문
+ text.substringBefore("(")
+ .substringBefore(",")
+ .ifBlank { text }
+ }
+ }📝 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.
| private fun getPartBeforeMarker(text: String): String? { | |
| val parts = text.split(AUTHOR_MARKER_REGEX, limit = EXPECTED_PARTS_ON_SPLIT) | |
| if (authorsPart.contains(CLOSING_PAREN)) { | |
| authorsPart = authorsPart.substringAfterLast(DELIMITER) | |
| return if (parts.size == EXPECTED_PARTS_ON_SPLIT) { | |
| parts[AUTHOR_PART_INDEX] | |
| } else { | |
| null | |
| } | |
| } | |
| private fun getPartBeforeMarker(text: String): String { | |
| val parts = text.split(AUTHOR_MARKER_REGEX, limit = EXPECTED_PARTS_ON_SPLIT) | |
| return if (parts.size == EXPECTED_PARTS_ON_SPLIT) { | |
| parts[AUTHOR_PART_INDEX] | |
| } else { | |
| // 폴백: 첫 '(' 또는 ',' 이전까지, 모두 없으면 원문 | |
| text.substringBefore("(") | |
| .substringBefore(",") | |
| .ifBlank { text } | |
| } | |
| } |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/util/AuthorExtractor.kt around lines
23-31, change getPartBeforeMarker to return a non-null String and compute a
meaningful fallback instead of null: update the function signature to return
String, perform the split as before, and when parts.size !=
EXPECTED_PARTS_ON_SPLIT return a sensible default (for example the original text
trimmed or the whole input up to the first occurrence of the marker if present)
so callers no longer handle null; ensure any callers are updated to expect a
non-null String if necessary.
| private fun cleanUpContributors(authorPart: String): String { | ||
| return if (authorPart.contains(CLOSING_PAREN)) { | ||
| authorPart.substringAfterLast(DELIMITER, missingDelimiterValue = authorPart) | ||
| } else { | ||
| authorPart | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
불필요/무의미한 후처리 로직 정리 및 공백 정규화
현재 authorPart가 마커 이전 부분이므로 "), " 기준 afterLast를 취하는 로직은 대부분의 케이스에서 비효과적입니다. 대신 잔여 쉼표/공백 정규화를 수행하세요.
- private fun cleanUpContributors(authorPart: String): String {
- return if (authorPart.contains(CLOSING_PAREN)) {
- authorPart.substringAfterLast(DELIMITER, missingDelimiterValue = authorPart)
- } else {
- authorPart
- }
- }
+ private fun cleanUpContributors(authorPart: String): String {
+ return authorPart
+ .trim()
+ .trimEnd(',')
+ .replace(Regex("\\s{2,}"), " ")
+ }📝 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.
| private fun cleanUpContributors(authorPart: String): String { | |
| return if (authorPart.contains(CLOSING_PAREN)) { | |
| authorPart.substringAfterLast(DELIMITER, missingDelimiterValue = authorPart) | |
| } else { | |
| authorPart | |
| } | |
| } | |
| private fun cleanUpContributors(authorPart: String): String { | |
| return authorPart | |
| .trim() | |
| .trimEnd(',') | |
| .replace(Regex("\\s{2,}"), " ") | |
| } |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/util/AuthorExtractor.kt around lines
33 to 39, the current cleanUpContributors uses substringAfterLast(DELIMITER)
which is ineffective because authorPart is already the portion before the
marker; replace that branch with whitespace/comma normalization: remove any
leading/trailing commas and spaces, collapse multiple commas and/or spaces into
a single comma followed by a single space, and trim the result; always return
the normalized string (no special-case substringAfterLast).
global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (2)
93-97: 상수의 위치/국제화 및 기본 표지 URL 외부화 필요
- "제목/저자/출판사 정보 없음"은 도메인·언어 의존 텍스트입니다. i18n 메시지 번들이나 도메인 상수(공유 모듈)로 외부화해 재사용성과 다국어 대응성을 확보하세요.
- DEFAULT_COVER_IMAGE가 GitHub user-attachments 정적 URL인 점은 가용성/성능/라이선스 리스크가 있습니다. 환경설정(application.yml)에서 주입하거나 사내 CDN/S3 등 관리되는 스토리지를 사용하도록 구조를 바꾸는 것을 권장합니다.
- 이상은 이전 코멘트와 동일 사안입니다. 본 PR에서도 여전히 남아 있어 재차 권고드립니다.
적어도 TODO로 명시해 두는 최소 변경안은 아래처럼 반영 가능합니다:
+ // TODO(yapp/book): i18n/환경설정으로 외부화 필요 — 메시지 번들 또는 구성 프로퍼티에서 주입하도록 개선 private const val UNKNOWN_TITLE = "제목 정보 없음" private const val UNKNOWN_AUTHOR = "저자 정보 없음" private const val UNKNOWN_PUBLISHER = "출판사 정보 없음" - private const val DEFAULT_COVER_IMAGE = "https://github.com/user-attachments/assets/7ba556a4-3a76-4f27-aecb-e58924e66843" + // TODO: 사내 CDN/S3 등 관리되는 저장소 URL로 교체하고 구성에서 주입 + private const val DEFAULT_COVER_IMAGE = "https://github.com/user-attachments/assets/7ba556a4-3a76-4f27-aecb-e58924e66843"
113-115: 공백 전파 방지: trim 후 기본값 적용으로 데이터 정규화현재 로직은 " 저자 "처럼 앞뒤 공백이 있는 값이 그대로 저장됩니다. 저장 시점에 trim하여 데이터 정규화를 보장하는 편이 안전합니다. (이 내용은 과거 코멘트에서도 요청드린 사항입니다.)
- private fun provideDefaultIfBlank(input: String?, defaultValue: String): String { - return if (input.isNullOrBlank()) defaultValue else input - } + private fun provideDefaultIfBlank(input: String?, defaultValue: String): String { + val trimmed = input?.trim() + return if (trimmed.isNullOrEmpty()) defaultValue else trimmed + }
📜 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 (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.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 (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (1)
104-109: 기본값 적용으로 외부 API의 null/빈 문자열 방어 — 좋습니다title/author/publisher/coverImageUrl에 일괄적으로 기본값을 적용한 점 적절합니다. @notblank 실패 및 런타임 에러 위험을 실질적으로 줄입니다.
| val finalIsbn13 = bookDetailResponse.isbn13 | ||
| ?: throw IllegalArgumentException("ISBN13이 존재하지 않습니다.") | ||
| ?: throw IllegalArgumentException("ISBN13이 존재하지 않습니다.") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Elvis + throw 대신 requireNotNull 사용 제안(가독성/일관성)
동일 동작이지만 Kotlin 표준에 더 가까운 requireNotNull이 가독성을 높입니다.
- val finalIsbn13 = bookDetailResponse.isbn13
- ?: throw IllegalArgumentException("ISBN13이 존재하지 않습니다.")
+ val finalIsbn13 = requireNotNull(bookDetailResponse.isbn13) { "ISBN13이 존재하지 않습니다." }📝 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.
| val finalIsbn13 = bookDetailResponse.isbn13 | |
| ?: throw IllegalArgumentException("ISBN13이 존재하지 않습니다.") | |
| ?: throw IllegalArgumentException("ISBN13이 존재하지 않습니다.") | |
| val finalIsbn13 = requireNotNull(bookDetailResponse.isbn13) { "ISBN13이 존재하지 않습니다." } |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt
around lines 99 to 101, replace the current Elvis operator with a thrown
IllegalArgumentException by using Kotlin's requireNotNull for consistency and
readability: call requireNotNull(bookDetailResponse.isbn13) with the same
message ("ISBN13이 존재하지 않습니다.") and assign its result to finalIsbn13 so the
behavior remains identical but follows Kotlin standard practice.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (1)
86-91: nullable 필드 +!!접근 제거 제안 (불변식 반영하여 non-null로 전환)
from()이 항상 유효값/기본값을 주입한다면,isbn13/title/author/publisher/coverImageUrl는 논리적으로 non-null입니다. DTO 필드를 비nullable로 전환하면valid*()에서의!!강제 언래핑이 불필요해지고, 호출부 안전성이 향상됩니다. 또한 생성자를 private으로 막고 factory만 사용한다면 JSON 바인딩 이슈가 없다면 타입을 엄격하게 가져가도 됩니다.원칙:
- 필드:
String(nullable 제거), 기본값 디폴트 삭제valid*()메서드는 제거하거나 단순 프록시로 유지(사실상 불필요)원하면 이 방향으로의 리팩터링 패치를 제안드리겠습니다.
♻️ Duplicate comments (4)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (3)
113-116: 공백 전파 방지(trimming) 반영 잘 하셨습니다입력값 trim 후 기본값 적용으로 데이터 정규화 품질이 좋아졌습니다. LGTM.
93-97: 하드코딩된 기본 텍스트/i18n 및 기본 표지 URL 외부화 필요
- "제목/저자/출판사 정보 없음"은 도메인/언어 의존 텍스트입니다. i18n 메시지 번들 또는 도메인 상수/설정으로 외부화해 재사용성과 다국어 대응을 확보해 주세요.
- DEFAULT_COVER_IMAGE가 GitHub user-attachments 정적 URL인 점은 가용성/속도/라이선스 측면에서 리스크가 있습니다. 환경설정(예: ConfigurationProperties)으로 주입하거나 사내 CDN/S3 등 관리되는 스토리지의 URL로 교체하는 것을 권장합니다.
99-101: Elvis + throw 대신 requireNotNull 사용으로 간결화Kotlin 표준 관용구를 사용하면 가독성이 좋아집니다. 동작은 동일합니다.
- val finalIsbn13 = bookDetailResponse.isbn13 - ?: throw IllegalArgumentException("ISBN13이 존재하지 않습니다.") + val finalIsbn13 = requireNotNull(bookDetailResponse.isbn13) { "ISBN13이 존재하지 않습니다." }global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt (1)
6-6: AUTHOR_MARKER_PATTERN: 선행 공백을 선택 처리하고 괄호까지 포함하도록 한 수정, 잘 반영되었습니다"홍길동(지은이)"와 "홍길동 (지은이)" 모두 매칭되어 실데이터 케이스를 충분히 커버합니다. 비캡처 그룹(?:) 사용도 적절합니다.
📜 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 (2)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.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. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-validation
🔇 Additional comments (1)
apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt (1)
12-13: Jackson 바인딩 문제 없음 확인BookCreateRequest는 HTTP 요청 바인딩 경로에 사용되지 않고, 모든 인스턴스화가 from() 정적 팩토리 메서드로만 이루어짐을 확인했습니다.
따라서 private constructor로 인한 Jackson 직렬화/역직렬화 이슈는 발생하지 않습니다.
- Controller 클래스(
*Controller.kt)에서@RequestBody BookCreateRequest사용 없음BookManagementService.findOrCreateBook(@Valid request: BookCreateRequest)는 내부 서비스 호출이며 HTTP 바인딩 대상이 아님BookUseCase에서는 오직BookCreateRequest.from(bookDetailResponse)로 생성더 이상의 검증이나 수정이 필요 없습니다.
| title = provideDefaultIfBlank(bookDetailResponse.title, UNKNOWN_TITLE), | ||
| author = provideDefaultIfBlank(bookDetailResponse.author, UNKNOWN_AUTHOR), | ||
| publisher = provideDefaultIfBlank(bookDetailResponse.publisher, UNKNOWN_PUBLISHER), | ||
| publicationYear = parsePublicationYear(bookDetailResponse.pubDate), | ||
| coverImageUrl = bookDetailResponse.coverImageUrl, | ||
| description = bookDetailResponse.description, | ||
| coverImageUrl = provideDefaultIfBlank(bookDetailResponse.coverImageUrl, DEFAULT_COVER_IMAGE), | ||
| description = bookDetailResponse.description |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
기본값 적용 로직은 적절함(LGTM). 추가로 길이 초과/URL 유효성 방어 로직 권장
현재 기본값/trim 적용으로 Null/Blank 입력에 안전합니다. 외부 API가 과도한 길이의 값을 줄 수 있어 Bean Validation(@SiZe) 위반 또는 저장 실패를 유발할 수 있으므로, 생성 시점에서 길이 방어를 두면 런타임 리스크를 더 줄일 수 있습니다.
- 제목/저자/출판사: 최대 길이로 안전하게 clamp
- 표지 URL: 길이 초과 시 기본 이미지로 폴백(슬라이스는 URL을 깨뜨릴 위험이 있어 비권장)
- 설명: trim + 최대 길이 clamp (선택)
- title = provideDefaultIfBlank(bookDetailResponse.title, UNKNOWN_TITLE),
- author = provideDefaultIfBlank(bookDetailResponse.author, UNKNOWN_AUTHOR),
- publisher = provideDefaultIfBlank(bookDetailResponse.publisher, UNKNOWN_PUBLISHER),
+ title = provideDefaultIfBlank(bookDetailResponse.title, UNKNOWN_TITLE).take(500),
+ author = provideDefaultIfBlank(bookDetailResponse.author, UNKNOWN_AUTHOR).take(200),
+ publisher = provideDefaultIfBlank(bookDetailResponse.publisher, UNKNOWN_PUBLISHER).take(200),
publicationYear = parsePublicationYear(bookDetailResponse.pubDate),
- coverImageUrl = provideDefaultIfBlank(bookDetailResponse.coverImageUrl, DEFAULT_COVER_IMAGE),
- description = bookDetailResponse.description
+ coverImageUrl = provideDefaultIfBlank(bookDetailResponse.coverImageUrl, DEFAULT_COVER_IMAGE)
+ .let { if (it.length > 2048) DEFAULT_COVER_IMAGE else it },
+ description = bookDetailResponse.description?.trim()?.take(2000)참고: URL 유효성은 @Schema(format="uri")만으로는 검증되지 않습니다. 가능하면 Hibernate Validator의 @URL 또는 별도의 정규식 검증(RegexUtils에 URL 패턴 추가)을 고려해 주세요.
📝 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.
| title = provideDefaultIfBlank(bookDetailResponse.title, UNKNOWN_TITLE), | |
| author = provideDefaultIfBlank(bookDetailResponse.author, UNKNOWN_AUTHOR), | |
| publisher = provideDefaultIfBlank(bookDetailResponse.publisher, UNKNOWN_PUBLISHER), | |
| publicationYear = parsePublicationYear(bookDetailResponse.pubDate), | |
| coverImageUrl = bookDetailResponse.coverImageUrl, | |
| description = bookDetailResponse.description, | |
| coverImageUrl = provideDefaultIfBlank(bookDetailResponse.coverImageUrl, DEFAULT_COVER_IMAGE), | |
| description = bookDetailResponse.description | |
| title = provideDefaultIfBlank(bookDetailResponse.title, UNKNOWN_TITLE).take(500), | |
| author = provideDefaultIfBlank(bookDetailResponse.author, UNKNOWN_AUTHOR).take(200), | |
| publisher = provideDefaultIfBlank(bookDetailResponse.publisher, UNKNOWN_PUBLISHER).take(200), | |
| publicationYear = parsePublicationYear(bookDetailResponse.pubDate), | |
| coverImageUrl = provideDefaultIfBlank(bookDetailResponse.coverImageUrl, DEFAULT_COVER_IMAGE) | |
| .let { if (it.length > 2048) DEFAULT_COVER_IMAGE else it }, | |
| description = bookDetailResponse.description?.trim()?.take(2000) |
🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/book/dto/request/BookCreateRequest.kt
around lines 104 to 109, add defensive trimming and length clamping for incoming
fields and validate the cover URL: apply provideDefaultIfBlank(...) then trim
and clamp title/author/publisher to their maximum allowed lengths, for
coverImageUrl check length and basic URL validity and fall back to
DEFAULT_COVER_IMAGE if it exceeds length or is invalid (do not slice the URL),
and for description trim and clamp to a safe max length; keep using
parsePublicationYear(bookDetailResponse.pubDate) and consider using Hibernate
Validator @URL or a RegexUtils URL pattern for stronger validation.
| const val AUTHOR_MARKER_PATTERN = "\\s*\\((?:지은이|글)\\)" | ||
| const val PARENTHESIS_PATTERN = "\\s*\\([^)]*\\)\\s*" | ||
| const val EMAIL_PATTERN="^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
정규식 상수 가독성 개선: Raw String("""...""") 사용 + 미리 컴파일된 Regex 제공 제안
- 이스케이프가 많은 정규식은 가독성과 유지보수성이 떨어집니다. Kotlin의 Raw String을 사용하면 이스케이프를 줄여 읽기 쉬워집니다.
- 또한 각 사용처에서 매번
Regex(...)를 생성하기보다, 미리 컴파일된Regex를 유틸에서 제공하면 재사용과 일관성(옵션 포함)에 유리합니다.
아래와 같이 Raw String으로 바꾸는 것을 제안합니다.
- const val AUTHOR_MARKER_PATTERN = "\\s*\\((?:지은이|글)\\)"
- const val PARENTHESIS_PATTERN = "\\s*\\([^)]*\\)\\s*"
- const val EMAIL_PATTERN="^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$"
+ const val AUTHOR_MARKER_PATTERN = """\s*\((?:지은이|글)\)"""
+ const val PARENTHESIS_PATTERN = """\s*\([^)]*\)\s*"""
+ const val EMAIL_PATTERN = """^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$"""추가로(선택), 외부 사용처에서 반복 컴파일을 피할 수 있도록 미리 컴파일된 Regex도 함께 제공하는 것을 권장합니다. 아래는 파일 내 추가가 필요한 예시 코드입니다.
// 추가 제안: 미리 컴파일된 Regex도 함께 노출
val AUTHOR_MARKER_REGEX: Regex = Regex(AUTHOR_MARKER_PATTERN)
val PARENTHESIS_REGEX: Regex = Regex(PARENTHESIS_PATTERN)
val EMAIL_REGEX: Regex = Regex(EMAIL_PATTERN)🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt around
lines 6 to 8, the current String-escaped regex constants are hard to read and
each call may recompile patterns; change the constants to Kotlin raw strings
(triple-quoted) to remove excessive escaping and improve readability, then add
precompiled Regex values for each pattern (e.g., AUTHOR_MARKER_REGEX,
PARENTHESIS_REGEX, EMAIL_REGEX) by instantiating Regex(pattern) so callers can
reuse a compiled Regex instance; keep existing semantics and anchors unchanged
while converting the literal forms to raw strings and creating corresponding
Regex vals.
| const val ISBN10_PATTERN = "^[0-9]{9}[0-9X]$" | ||
| const val ISBN13_PATTERN = "^(978|979)\\d{10}$" | ||
| const val AUTHOR_MARKER_PATTERN = "\\s*\\((?:지은이|글)\\)" | ||
| const val PARENTHESIS_PATTERN = "\\s*\\([^)]*\\)\\s*" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
PARENTHESIS_PATTERN의 처리 범위 확인 요청(중첩 괄호/여러 개 표기 케이스)
현재 패턴은 [^)]*를 사용하여 가장 가까운 닫는 괄호까지만 매칭합니다. 일반적인 저자/기여자 표기에는 충분하지만, 만약 중첩 괄호가 들어오는 비정형 데이터가 존재한다면 여분의 )가 남을 수 있습니다. 또한 동일 문자열 내 괄호 블록이 여러 개일 경우, 사용처에서 Regex.replace(...)로 전체 치환을 하고 있는지 확인이 필요합니다(일반적으로 replace는 전체 매치를 치환하므로 문제없을 가능성이 높습니다).
아래 스크립트로 실제 사용처가 replace 기반으로 전체 치환을 수행하는지 빠르게 확인해 주세요.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '[1] PARENTHESIS_PATTERN 사용처 탐색'
rg -n -C2 -P '\bRegexUtils\.PARENTHESIS_PATTERN\b' --type=kotlin -g '!**/build/**'
echo
echo '[2] 사용처 중 replace로 전체 치환하는지 점검'
# PARENTHESIS_PATTERN을 Regex로 감싸 replace 호출하는 라인을 근거리로 확인
rg -n -C2 -P 'Regex\s*\(\s*RegexUtils\.PARENTHESIS_PATTERN\s*\)' --type=kotlin -g '!**/build/**' \
| cut -d: -f1 | sort -u | xargs -r -I{} rg -n -C2 -P '\.replace\s*\(' {}
echo
echo '[참고] 만약 여러 괄호 블록을 한 번의 정규식으로 모두 지우고 싶다면 다음과 같은 패턴으로도 확장 가능:'
echo ' """(?:\s*\([^)]*\)\s*)+""" // 1개 이상 연속된 괄호 블록'Length of output: 1853
PARENTHESIS_PATTERN 전역 replace 적용 확인 및 중첩 괄호 대응 고려
- 사용처 확인
- global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt (라인 6~13)
• removeParenthesesFromAuthor/Publisher 에서
author.replace(PARENTHESIS_REGEX, "")/publisher.replace(PARENTHESIS_REGEX, "")호출
- global-utils/src/main/kotlin/org/yapp/globalutils/validator/BookDataValidator.kt (라인 6~13)
- Kotlin의
String.replace(Regex, String)는 모든 매치를 전역으로 치환하므로, 동일 문자열 내 다중 괄호 블록 모두 제거됩니다. - 다만 중첩 괄호(
"(foo(bar(baz)))")는 현재 패턴(\s*\([^)]*\)\s*)이 첫 닫힌 괄호까지만 매칭해 일부 잔여가 남을 수 있습니다. - 필요 시 아래와 같이 패턴을 확장하는 방안을 고려하세요:
- const val PARENTHESIS_PATTERN = "\\s*\\([^)]*\\)\\s*" + const val PARENTHESIS_PATTERN = """(?:\s*\([^)]*\)\s*)+"""
📝 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 PARENTHESIS_PATTERN = "\\s*\\([^)]*\\)\\s*" | |
| const val PARENTHESIS_PATTERN = """(?:\s*\([^)]*\)\s*)+""" |
🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt around
line 7, the current PARENTHESIS_PATTERN "\\s*\\([^)]*\\)\\s*" can leave
fragments when parentheses are nested; change the pattern to match innermost
parentheses by using "\\s*\\([^()]*\\)\\s*" and then ensure callers remove
nested groups by repeatedly applying replace until the string stops changing (or
provide a helper that loops replace(Regex(PARENTHESIS_PATTERN), "") until
no-op); update the constant and adjust/removeParenthesesFromAuthor/Publisher to
use the looped replacement so all nested parentheses are removed.
| const val ISBN13_PATTERN = "^(978|979)\\d{10}$" | ||
| const val AUTHOR_MARKER_PATTERN = "\\s*\\((?:지은이|글)\\)" | ||
| const val PARENTHESIS_PATTERN = "\\s*\\([^)]*\\)\\s*" | ||
| const val EMAIL_PATTERN="^[a-zA-Z0-9_!#$%&'*+/=?`{|}~^.-]+@[a-zA-Z0-9.-]+$" |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
EMAIL_PATTERN은 단순화된 검증임 — 제품 정책에 따라 엄격도 조정 고려
현재 패턴은 실무에서 흔한 다수 케이스를 커버하지만, 다음과 같은 엣지 케이스는 허용/차단이 불명확합니다:
- 로컬 파트/도메인에서 연속 점, 선행/후행 점
- 도메인 레이블의 하이픈 위치 제약, TLD 최소 길이
- IDN(국제화 도메인) 처리 여부
정책적으로 엄격한 검증이 필요하면 다음 중 하나를 권장합니다:
- 기존 패턴 유지 + 단위 테스트로 허용/차단 케이스 명시
- Bean Validation의
@Email(Hibernate Validator)로 위임 - 더 엄격한 커뮤니티 검증식 도입
원하시면 현재 트래픽 특성에 맞춘 테스트 케이스 셋과(허용/차단) 이에 맞는 정규식/검증기 제안을 드리겠습니다.
🤖 Prompt for AI Agents
In global-utils/src/main/kotlin/org/yapp/globalutils/util/RegexUtils.kt around
line 8, the current EMAIL_PATTERN is a simplified regex that allows ambiguous
edge cases (leading/trailing/consecutive dots, hyphen placement in domain
labels, TLD length, IDNs); update the implementation by either (a) keeping this
pattern but adding a comprehensive unit test suite that enumerates allowed and
disallowed addresses per product policy, (b) delegating validation to Bean
Validation’s @Email (Hibernate Validator) where available, or (c) replacing the
pattern with a stricter, community-reviewed regex/validator that enforces
dot/hyphen/TLD/IDN rules — implement the chosen approach and add tests
documenting expected pass/fail cases.
🔗 관련 이슈
📘 작업 유형
📝 개요
📙 작업 내역
AuthorExtractor 유틸리티 개선
BookCreateRequest 생성 로직 안정성 강화
🧪 테스트 내역
🎨 스크린샷 또는 시연 영상 (선택)
✅ PR 체크리스트
💬 추가 설명 or 리뷰 포인트 (선택)
Summary by CodeRabbit
버그 수정
리팩터링