✨ Feature: 파일 업로드 기능 추가 #2
Conversation
|
Warning Rate limit exceeded@catturtle123 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 4 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Walkthrough멀티파트 문서 업로드 기능이 추가되었습니다: 컨트롤러·서비스·DTO, JPA 엔티티·레포지토리, S3 유틸(실제·테스트), 파일 처리 유틸, 예외·오류코드 확장, OpenAPI 및 환경설정과 관련 테스트가 포함됩니다. 업로드는 SHA-256 중복 검사와 태그 연동, 트랜잭션 롤백 시 S3 정리 로직을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant Ctrl as DataFileController
participant Svc as DataFileService
participant S3 as S3Util (Impl/Fake)
participant DF as DataFileRepository
participant TG as TagRepository
participant DFT as DataFileTagRepository
C->>Ctrl: POST /api/v1/data-files (files + request JSON)
Ctrl->>Svc: upload(files, request)
Svc->>Svc: validate inputs (count, non-empty)
loop per file
Svc->>Svc: compute sha256, size, content-type (streaming)
Svc->>DF: existsBySha256(sha256)?
alt exists
Svc-->>Ctrl: throw FileException(ALREADY_FILE)
else
Svc->>S3: upload(file, ORIGINAL_FILE)
S3-->>Svc: url
Svc->>DF: save(DataFile)
Svc->>TG: getOrCreateTags(...)
Svc->>DFT: attachTagsIfMissing(...)
end
end
Svc-->>Ctrl: DataFileResponseList
Ctrl-->>C: 201 Created (ApiResponse.ok)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
✨ Finishing Touches
🧪 Generate unit tests
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 43
♻️ Duplicate comments (1)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
10-22: Kotlin JPA 플러그인 적용 여부 재확인(중요)@entity 클래스는 기본 final/no-arg 부재로 프록시 생성 문제가 발생할 수 있습니다. 위 Tag.kt 코멘트의 플러그인 점검 스크립트를 참고해 kotlin-jpa(all-open/no-arg) 적용 여부를 확인해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
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 (25)
build.gradle(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/entity/Tag.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileTagRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/config/S3Config.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/config/SwaggerConfig.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/CustomException.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt(2 hunks)src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/MultipartJackson2HttpMessageConverter.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3Type.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt(1 hunks)src/main/resources/application.yml(2 hunks)src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt(1 hunks)src/test/resources/application-test.yml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (2)
src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (2)
urlFromKey(7-7)keyFromUrl(10-10)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
urlFromKey(49-52)keyFromUrl(85-92)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
urlFromKey(30-30)keyFromUrl(36-37)delete(34-34)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (3)
urlFromKey(7-7)keyFromUrl(10-10)delete(9-9)
🪛 YAMLlint (1.37.1)
src/main/resources/application.yml
[error] 41-41: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (11)
build.gradle (2)
44-44: 테스트 의존성 추가 적절함Mockito-Kotlin 추가가 테스트 가독성과 생산성에 도움됩니다. 현재 버전 조합(Starter Test + Mockito 5.x 계열)과도 충돌 이슈 없어 보입니다.
47-47: H2 testRuntimeOnly 추가 적절함테스트 프로파일에서 H2 사용을 위한 설정이 적절합니다. application-test.yml과도 일관됩니다.
src/test/resources/application-test.yml (1)
1-11: 테스트 DB 설정 적절함
- In-memory H2 + PostgreSQL 호환 모드(MODE=PostgreSQL) 설정은 JPA 테스트에 적합합니다.
- create-drop 전략으로 테스트 간 간섭을 줄입니다.
src/main/resources/application.yml (1)
7-10: 업로드 한도(50MB/200MB) 재확인 및 검증 로직 부재 안내현재
application.yml의 스프링 multipart 설정(단일 파일 50MB, 전체 요청 200MB)에만 의존하고 있으며, 컨트롤러(DataFileController#upload)나 서비스(DataFileService#upload) 단계에서 별도의 파일 크기 검증 로직은 존재하지 않습니다.
- PDF 스캔본, PPT/동영상 포함 자료 등 실제 기대 파일 크기에 비해 설정값이 충분한지 검토해주세요.
- 멀티 파일 업로드 시 전체 제한(200MB)과 충돌 가능성도 함께 확인 후, 설정값 상향 또는 서비스 레벨 검증 로직 추가를 고려해 주세요.
src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt (1)
9-13: MB 변환 로직은 적절함소수점 셋째자리 반올림으로 MB 표기 일관성이 확보됩니다. 요구사항과 부합합니다.
src/main/kotlin/simplerag/ragback/global/util/MultipartJackson2HttpMessageConverter.kt (1)
14-24: 쓰기 비활성화 결정은 적절합니다.
multipart 시나리오에서 읽기 전용 컨버터로 제한하는 것은 안전합니다.src/main/kotlin/simplerag/ragback/global/config/S3Config.kt (1)
23-24: S3Config.bucket 프로퍼티는 실제로 사용되고 있습니다.S3UtilImpl.kt에서
s3Config.bucket을 통해 업로드(.bucket(bucket)), URL 생성(.getUrl{ it.bucket(bucket) }), 삭제(.bucket(bucket)) 등 여러 곳에서 참조하고 있으므로 제거하지 말아주세요.Likely an incorrect or invalid review comment.
src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt (1)
15-22: 에러 코드 추가 방향성 LGTM
S3 관련 코드와 중복 파일 시나리오를 명확히 구분하여 실무 운영에 유용합니다.src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt (1)
3-10: 컬렉션 필드명 간결화 제안:dataFilePreviewResponseList→itemsAPI 응답 JSON 가독성을 위해 DTO의 리스트 필드를
items로 변경하는 것을 권장합니다. 변경 시 다음 위치만 수정하면 됩니다:
- src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt
- src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
예시 diff:
// src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt data class DataFileResponseList( - val dataFilePreviewResponseList: List<DataFilePreviewResponse>, + val items: List<DataFilePreviewResponse>, )// src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt - assertEquals(1, res.dataFilePreviewResponseList.size) - val r0 = res.dataFilePreviewResponseList.first() + assertEquals(1, res.items.size) + val r0 = res.items.first() … - val saved = dataFileRepository.findById(res.dataFilePreviewResponseList.first().id).orElseThrow() + val saved = dataFileRepository.findById(res.items.first().id).orElseThrow()[optional_refactors_recommended]
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileTagRepository.kt (1)
6-8: DB 레벨 복합 유니크 제약(인덱스) 확인 완료
DataFileTag엔티티에@Table(uniqueConstraints = [UniqueConstraint(columnNames = ["data_file_id", "tag_id"])])로 복합 유니크 제약이 설정되어 있어,
- 중복 삽입 방지가 DB에서 강제되고
existsByDataFileIdAndTagId쿼리도 해당 복합 인덱스를 활용해 효율적으로 동작합니다.추가로, 대량 태깅 시에는 N+1 방지를 위해
findAllByDataFileIdAndTagIdIn등 배치 조회 방식을 고려하시면 더욱 좋습니다.src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (1)
49-53: LGTM: URL 생성 방식 적절S3 utilities().getUrl로 버킷/키 기반 URL을 안전하게 생성하고 있습니다.
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt
Show resolved
Hide resolved
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
Outdated
Show resolved
Hide resolved
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
Show resolved
Hide resolved
| } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { | ||
| // NoSuchKey 등 | ||
| throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) | ||
| } catch (e: Exception) { | ||
| throw S3Exception(ErrorCode.S3_DELETE_FAIL) | ||
| } |
There was a problem hiding this comment.
S3 삭제 에러 매핑 정교화: ‘모든 예외=OBJECT_NOT_FOUND’는 과도합니다
AccessDenied, 네트워크 오류 등도 S3Exception으로 포장됩니다. NoSuchKey(404)만 OBJECT_NOT_FOUND로, 그 외는 DELETE_FAIL로 구분하세요.
- } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) {
- // NoSuchKey 등
- throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND)
+ } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) {
+ val code = e.awsErrorDetails()?.errorCode()
+ val status = e.statusCode()
+ if ((code != null && code.equals("NoSuchKey", ignoreCase = true)) || status == 404) {
+ throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND)
+ } else {
+ throw S3Exception(ErrorCode.S3_DELETE_FAIL)
+ }
} catch (e: Exception) {
throw S3Exception(ErrorCode.S3_DELETE_FAIL)
}📝 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.
| } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { | |
| // NoSuchKey 등 | |
| throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) | |
| } catch (e: Exception) { | |
| throw S3Exception(ErrorCode.S3_DELETE_FAIL) | |
| } | |
| } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { | |
| val code = e.awsErrorDetails()?.errorCode() | |
| val status = e.statusCode() | |
| if ((code != null && code.equals("NoSuchKey", ignoreCase = true)) || status == 404) { | |
| throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) | |
| } else { | |
| throw S3Exception(ErrorCode.S3_DELETE_FAIL) | |
| } | |
| } catch (e: Exception) { | |
| throw S3Exception(ErrorCode.S3_DELETE_FAIL) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 18
♻️ Duplicate comments (15)
src/main/kotlin/simplerag/ragback/global/util/S3Type.kt (1)
7-7: 이전 지적사항 반영: enum 마지막 세미콜론 제거 확인 (LGTM)과거 커밋에서 지적된 불필요한 세미콜론이 제거되었습니다. 현재 구문은 Kotlin 1.9.x에서 문제 없습니다. 트레일링 콤마도 허용 범위입니다.
build.gradle (1)
52-54: AWS SDK BOM 도입 훌륭합니다 (버전 정합성 확보, LGTM)BOM을 통해 모듈 간 버전 스큐를 방지하는 구성이 적절합니다. s3 모듈 쪽에 개별 버전 누락된 것도 좋습니다.
src/main/kotlin/simplerag/ragback/global/config/SwaggerConfig.kt (1)
15-15: Bean 네이밍 컨벤션 반영 확인 (LGTM)openAPI로 lowerCamelCase 정렬되어 가독성과 관례에 부합합니다.
src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt (2)
13-17: 스트리밍 버전 보완: 범용 sha256Hex(InputStream) 및 MultipartFile 확장 추가 제안현재
computeMetricsStreaming()로 스트리밍 해시·크기 동시 계산이 가능해진 점 좋습니다. 재사용성을 위해 단일 책임 함수도 함께 제공하면 호출부 선택지가 넓어집니다. 이전 제안과 동일 맥락입니다.아래 보조 함수를 유틸에 추가하는 것을 권장합니다(선택):
// 파일 내 적절한 위치에 추가 import java.security.DigestInputStream import java.io.InputStream fun sha256Hex(input: InputStream): String { val md = MessageDigest.getInstance("SHA-256") DigestInputStream(input, md).use { dis -> val buffer = ByteArray(DEFAULT_BUFFER_SIZE) while (dis.read(buffer) != -1) { /* no-op */ } } return md.digest().joinToString("") { "%02x".format(it) } } fun MultipartFile.sha256Hex(): String = this.inputStream.use { sha256Hex(it) }
18-38: MIME 매핑 확장: 이미지 포맷(gif/bmp/webp/svg) 추가빈 contentType 처리와 문서 포맷 확장 반영 좋습니다. 일반 이미지 포맷 몇 가지가 누락되어 있어 추가를 권장합니다. 이전 피드백과 동일 맥락입니다.
적용 diff:
return when (ext) { "png" -> "image/png" "jpg", "jpeg" -> "image/jpeg" + "gif" -> "image/gif" + "bmp" -> "image/bmp" + "webp" -> "image/webp" + "svg" -> "image/svg+xml" "pdf" -> "application/pdf" "txt" -> "text/plain" "csv" -> "text/csv" "md" -> "text/markdown" "json" -> "application/json" "zip" -> "application/zip" "doc" -> "application/msword" "docx" -> "application/vnd.openxmlformats-officedocument.wordprocessingml.document" "xls" -> "application/vnd.ms-excel" "xlsx" -> "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet" "ppt" -> "application/vnd.ms-powerpoint" "pptx" -> "application/vnd.openxmlformats-officedocument.presentationml.presentation" else -> "application/octet-stream" }src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (1)
14-15: printStackTrace 제거 및 구조적 로깅 도입 👍운영 로깅 관점에서 바람직한 변경입니다.
src/main/kotlin/simplerag/ragback/domain/document/entity/Tag.kt (2)
10-17: 엔티티 equals/hashCode 구현 누락 — 식별자(id) 기반으로 최소화하세요JPA 엔티티는 동등성/해시 기반 컬렉션 사용 시 안정성을 위해 id 기반 equals/hashCode 구현을 권장합니다. 현재는 기본 참조 동일성만 사용되어 프록시/영속성 컨텍스트에서 예기치 않은 동작 가능성이 있습니다. 이전 리뷰에서도 동일 제안을 드렸습니다.
아래처럼 클래스 본문을 추가해 id 기반으로 구현해 주세요.
class Tag( @@ - val id: Long? = null, -) + val id: Long? = null, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is Tag) return false + return id != null && id == other.id + } + override fun hashCode(): Int = id?.hashCode() ?: System.identityHashCode(this) +}
5-17: Kotlin JPA 플러그인 설정 누락 — all-open/no-arg 플러그인 적용 필요검사 결과,
build.gradle에org.jetbrains.kotlin.plugin.jpa또는plugin-allopen/plugin-noarg관련 설정이 없습니다. JPA 엔티티 클래스는 기본적으로final이고 인자가 없는 생성자가 없어 Hibernate 프록시 생성 시no default constructor오류가 발생할 수 있습니다. 아래 중 하나를 적용해주세요:• Gradle 플러그인 추가
–build.gradle(Groovy DSL 예시)plugins { id 'org.jetbrains.kotlin.jvm' version '1.9.25' id 'org.jetbrains.kotlin.plugin.spring' version '1.9.25' + id 'org.jetbrains.kotlin.plugin.jpa' version '1.9.25' + id 'org.jetbrains.kotlin.plugin.allopen' version '1.9.25' + id 'org.jetbrains.kotlin.plugin.noarg' version '1.9.25' } + + allOpen { + annotation("javax.persistence.Entity") + } + + noArg { + annotation("javax.persistence.Entity") + }• 엔티티 클래스 수정
–src/main/kotlin/simplerag/ragback/domain/document/entity/Tag.kt
•class Tag→open class Tag
• 인자 없는 생성자 추가 (예:protected constructor() : this(""))위 두 방법 중 하나를 적용해 JPA 엔티티가 정상 동작하도록 보장해 주세요.
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt (2)
7-11: 중첩 검증 미적용 — @Valid를 @field:Valid로 교체 필요Kotlin에서는 Bean Validation을 필드 타깃에 적용하려면 @field:Valid가 필요합니다. 현재 @Valid로는 내부 항목의 제약이 동작하지 않을 수 있습니다. 이전 리뷰에서도 동일 사항을 지적했습니다.
data class DataFileBulkCreateRequest( @field:Size(min = 1, message = "최소 하나 이상 업로드해야 합니다") - @Valid + @field:Valid val items: List<DataFileCreateItem> )
18-19: 태그 요소 검증 누락 — 컨테이너 요소 제약 추가 권장각 태그 문자열의 공백/길이 제약이 없습니다. Bean Validation 2.0 컨테이너 요소 제약으로 요소 단위 검증을 추가하세요. 이전 리뷰와 동일 권고입니다.
- @field:Size(max = 10, message = "태그는 최대 10개까지 가능합니다") - val tags: List<String> = emptyList() + @field:Size(max = 10, message = "태그는 최대 10개까지 가능합니다") + val tags: List<@NotBlank(message = "태그는 비어있을 수 없습니다") @Size(max = 30, message = "태그는 최대 30자까지 가능합니다") String> = emptyList()src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (2)
11-36: 엔티티 equals/hashCode 미구현 — id 기반 구현 권장Tag와 동일하게 DataFile도 id 기반 equals/hashCode를 구현해 영속성 컨텍스트/프록시 환경에서 안전성을 확보하세요. 이전 리뷰 제안과 동일합니다.
class DataFile( @@ - val id: Long? = null, -) + val id: Long? = null, +) { + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is DataFile) return false + return id != null && id == other.id + } + override fun hashCode(): Int = id?.hashCode() ?: System.identityHashCode(this) +}
3-5: 감사 필드 자동화(선택) — Auditing으로 createdAt/updatedAt 관리서비스/도메인에서 수동 설정 대신 Spring Data JPA Auditing을 사용하면 안전하고 일관적입니다. 별도 설정(@EnableJpaAuditing) 필요합니다.
import jakarta.persistence.* import java.time.LocalDateTime +import org.springframework.data.annotation.CreatedDate +import org.springframework.data.annotation.LastModifiedDate +import org.springframework.data.jpa.domain.support.AuditingEntityListener @Entity +@EntityListeners(AuditingEntityListener::class) @Table( @@ - @Column(nullable = false) - val updatedAt: LocalDateTime, + @LastModifiedDate + @Column(nullable = false) + val updatedAt: LocalDateTime, @@ - @Column(nullable = false) - val createdAt: LocalDateTime, + @CreatedDate + @Column(nullable = false, updatable = false) + val createdAt: LocalDateTime,Also applies to: 6-7, 28-33
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt (1)
194-199: SHA-256 계산 로직 중복 — 테스트에서도 유틸 재사용 권장테스트 내에서 MessageDigest로 직접 계산하지 말고, 이미 import된
sha256Hex유틸을 재사용하면 중복을 줄이고 가독성이 좋아집니다.다음 변경을 제안합니다:
- val hash12 = MessageDigest.getInstance("SHA-256") - .digest(bytes).joinToString("") { "%02x".format(it) } - .take(12) + val hash12 = sha256Hex(bytes).take(12)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
75-80: S3 삭제 예외 매핑 정교화 필요 — NoSuchKey만 OBJECT_NOT_FOUND로 매핑모든 S3Exception을 OBJECT_NOT_FOUND로 매핑하는 것은 과도합니다. NoSuchKey(404)만 OBJECT_NOT_FOUND로, 그 외는 DELETE_FAIL로 구분하세요.
- } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { - // NoSuchKey 등 - throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) + } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { + val code = e.awsErrorDetails()?.errorCode() + val status = e.statusCode() + if ((code != null && code.equals("NoSuchKey", ignoreCase = true)) || status == 404) { + throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) + } else { + throw S3Exception(ErrorCode.S3_DELETE_FAIL) + } } catch (e: Exception) { throw S3Exception(ErrorCode.S3_DELETE_FAIL) }
35-41: Content-Type 결정 로직 통일: resolveContentType 사용업로드 시
file.contentType ?: "application/octet-stream"대신, 기존 유틸의resolveContentType()을 사용해 서비스와 S3 메타데이터의 정합성을 맞추세요.- .contentType(file.contentType ?: "application/octet-stream") + .contentType(file.resolveContentType())
📜 Review details
Configuration used: CodeRabbit UI
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 (19)
build.gradle(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/entity/Tag.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/config/SwaggerConfig.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/CustomException.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt(3 hunks)src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/MultipartJackson2HttpMessageConverter.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3Type.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt(1 hunks)src/main/resources/application-local.yml(1 hunks)src/main/resources/application.yml(1 hunks)src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-16T11:30:00.127Z
Learnt from: catturtle123
PR: Simple-RAG/BACK#2
File: src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt:29-35
Timestamp: 2025-08-16T11:30:00.127Z
Learning: FileException 오류 메시지에서는 파일명 대신 SHA256을 노출하는 것이 보안상 더 안전하며, 파일 내용을 고유하게 식별할 수 있어 디버깅에도 효과적이다.
Applied to files:
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
🧬 Code Graph Analysis (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt (1)
sha256Hex(13-16)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
urlFromKey(58-61)keyFromUrl(94-101)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (2)
urlFromKey(7-7)keyFromUrl(10-10)
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt (1)
src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt (1)
sha256Hex(13-16)
🪛 YAMLlint (1.37.1)
src/main/resources/application-local.yml
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
🪛 detekt (1.23.8)
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
[warning] 63-63: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt
[warning] 52-52: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 78-78: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 75-75: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 78-78: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (12)
src/main/kotlin/simplerag/ragback/global/util/S3Type.kt (1)
4-8: S3 객체 키에서 선행 슬래시 제거
S3Type.label에서 선행 슬래시를 제거해 주세요.buildKey메서드에서dir.trim('/')로 선행·후행 슬래시를 정규화하므로,ORIGINAL/으로 변경해도 이중 슬래시가 발생하지 않습니다. 변경 후 객체 키는"ORIGINAL/<UUID>_파일명"형태로 생성됩니다.제안 변경:
enum class S3Type( val label: String, ) { - ORIGINAL_FILE("/ORIGINAL/"), + ORIGINAL_FILE("ORIGINAL/"), }src/main/resources/application-local.yml (1)
18-26: DefaultCredentialsProvider 사용 검토현재
S3Config.kt에서access-key와secret-key를 직접 프로퍼티로 주입하고 있어, SDK의 기본 자격 증명 프로바이더 체인(DefaultCredentialsProvider)을 활용하지 않고 있습니다. 환경 이식성과 보안 강화를 위해 자격 증명 주입 부분을 제거하고, region과 bucket 설정만 남긴 후 DefaultCredentialsProvider에 위임하는 방안을 권장드립니다.
- 대상 파일:
- src/main/kotlin/simplerag/ragback/global/config/S3Config.kt (Lines 20–31)
- 제거 또는 대체할 부분:
•@Value("\${cloud.aws.credentials.access-key:}") private val accessKey: String
•@Value("\${cloud.aws.credentials.secret-key:}") private val secretKey: String제거 후 예시 diff:
- @Value("\${cloud.aws.credentials.access-key:}") - private val accessKey: String, - @Value("\${cloud.aws.credentials.secret-key:}") - private val secretKey: String, + // 자격 증명은 SDK 기본 체인에 위임 @Value("\${cloud.aws.region.static}") private val region: String, @Value("\${cloud.aws.s3.bucket}") val bucket: String,DefaultCredentialsProvider 적용 예시:
val s3Client = S3Client.builder() .region(Region.of(region)) .credentialsProvider(DefaultCredentialsProvider.create()) .build()[optional_refactors_recommended]
src/main/kotlin/simplerag/ragback/global/util/MultipartJackson2HttpMessageConverter.kt (1)
9-12: 기본 OCTET_STREAM 컨버터 충돌 방지: 우선순위 하향(@order) 적용 권장application/octet-stream을 처리하는 기본
ByteArrayHttpMessageConverter와 충돌을 방지하기 위해, 커스텀 컨버터의 실행 우선순위를 Lowest로 낮추세요.
- 대상 파일
src/main/kotlin/simplerag/ragback/global/util/MultipartJackson2HttpMessageConverter.kt- 변경 위치
- 필요한 import 추가
- 클래스 선언부에 @order 어노테이션 추가
import com.fasterxml.jackson.databind.ObjectMapper import org.springframework.http.MediaType import org.springframework.http.converter.json.AbstractJackson2HttpMessageConverter +import org.springframework.core.Ordered +import org.springframework.core.annotation.Order import org.springframework.stereotype.Component @Component +@Order(Ordered.LOWEST_PRECEDENCE) class MultipartJackson2HttpMessageConverter( objectMapper: ObjectMapper ) : AbstractJackson2HttpMessageConverter(objectMapper, MediaType.APPLICATION_OCTET_STREAM) {추가 검증: 멀티파트 업로드 시 octet-stream 바이너리 파트가 JSON으로 잘못 파싱되지 않는지 통합 테스트를 실행해 보세요. 우선순위 조정 후에도 이슈가 남으면 MediaType 범위를 좁히거나
canRead오버라이드를 통해 처리를 세분화하는 방안을 고려하시기 바랍니다.src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (2)
48-55: Unhandled 예외 로깅 적절합니다스택트레이스 로그 후 일반화된 500 응답 반환이 일관적입니다. 보편적으로 OK입니다.
32-38: 검증 완료: FileException의 메시지에 SHA256 세팅 확인 + 빈 메시지 가드 추가 권장DataFileService(
DataFileService.kt)에서throw FileException(ErrorCode.ALREADY_FILE, sha256)호출로
ex.message에 SHA256이 전달되는 것을 확인했습니다. 메시지가null이나 공백이 될 가능성은 낮지만, 안전을 위해 빈 메시지 가드를 추가할 것을 권장드립니다.
- 위치:
GlobalExceptionHandler.kthandleCustomException메서드@ExceptionHandler(FileException::class) fun handleCustomException(ex: FileException): ResponseEntity<ApiResponse<Nothing>> { val errorCode = ex.errorCode - return ResponseEntity - .status(errorCode.status) - .body(ApiResponse.fail(errorCode.code, "${errorCode.message} sha256: ${ex.message}")) + val sha = ex.message?.trim() + val msg = if (sha.isNullOrEmpty()) errorCode.message + else "${errorCode.message} sha256: $sha" + return ResponseEntity + .status(errorCode.status) + .body(ApiResponse.fail(errorCode.code, msg)) }[optional_refactors_recommended]
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (1)
27-29: 빈 파일 목록 방지: @notempty 추가 검토 요청
- DataFileController.kt
@RequestPart("files") files: List<MultipartFile>에@jakarta.validation.constraints.NotEmpty를 추가하는 것을 권장합니다.- 서비스 레이어
files.size와req.items.size가 일치하지 않을 경우 명확한 에러 처리(예: BadRequest 응답)가 구현되어 있는지 검증이 필요합니다.fun upload( - @RequestPart("files") files: List<MultipartFile>, + @jakarta.validation.constraints.NotEmpty + @RequestPart("files") files: List<MultipartFile>, @Valid @RequestPart("request") req: DataFileBulkCreateRequest ): ApiResponse<DataFileResponseList> {위 변경 후에도 서비스 내부에 사이즈 검증 및 불일치 시 에러 응답 로직이 제대로 동작하는지 확인 부탁드립니다.
src/main/kotlin/simplerag/ragback/global/error/CustomException.kt (1)
3-7: 원인 예외(cause) 체인 보존 추가 LGTMRuntimeException(message, cause)로 연결해 원인 추적 가능해졌습니다. 하위 예외에서도 전달되고 있어 적절합니다.
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt (1)
14-16: title 제약 추가 반영 LGTMNotBlank + 최대 길이 100으로 설정된 점 적절합니다. 저장소/DB 스키마와 일치하는지만 최종 확인해 주세요.
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (2)
35-40: deleteByUrl의 정책 일치 LGTM잘못된 URL에 대해 예외를 던지도록 맞춘 점이 운영 구현과 일치합니다.
44-46: keyFromUrl 정규화 LGTM선행 '/' 제거로 URL ↔ key 변환이 일관됩니다.
src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt (1)
12-22: S3/업로드 관련 에러 코드 확장 적절 — LGTM업로드/삭제/프리사인 이슈를 구분하는 세분화된 코드들이 DataFileService 및 S3 유틸과 자연스럽게 연결됩니다. 메시지/HTTP 상태 매핑도 타당합니다.
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (1)
49-53: 이름 일관성:sizeByte→sizeBytes통일 제안바이트 수를 나타내는 변수·필드를 복수형(
sizeBytes)으로 통일하면 의도를 명확히 전달할 수 있습니다. 아래 위치를 검토하시고 일괄 리팩터링을 고려해주세요:
- src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt
•val sizeByte: Long→val sizeBytes: Long- src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
•metrics.sizeByte및val sizeByte→sizeBytes- src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt
• 이미val sizeBytes: Long로 정의된 부분 확인- src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
• DTO 매핑 테스트에서sizeBytes사용 확인후방 호환성 영향 범위가 크므로, 변경 전 커뮤니케이션 및 종속성(엔티티, DTO, 유틸 클래스, 테스트 등) 업데이트 계획을 함께 수립해주세요.
[optional_refactors_recommended]
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
Outdated
Show resolved
Hide resolved
| val saved = service.upload(files, req) | ||
| return ApiResponse.ok(saved, "업로드 완료") | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
201 Created 반환은 적절합니다. Location 헤더(선택) 고려
다건 업로드인 경우 컬렉션 조회 엔드포인트를 Location으로 제공하면 REST 완결도가 높아집니다(선택).
🤖 Prompt for AI Agents
In
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
around lines 30 to 32, the controller currently returns ApiResponse.ok with a
200 status after uploading multiple files; change it to return a 201 Created
response and set an appropriate Location header—for multi-upload prefer the
collection retrieval endpoint (e.g. the GET endpoint that lists the uploaded
files) or, if you can compute a collective resource URI, use that; adjust the
ApiResponse or HTTP response builder to include status 201 and the Location
header while keeping the response body with the saved result.
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt
Outdated
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt
Show resolved
Hide resolved
| bucket: ${BUCKET_LOCAL} | ||
| credentials: | ||
| access-key: ${AWS_ACCESS_KEY_ID_LOCAL} | ||
| secret-key: ${AWS_SECRET_ACCESS_KEY_LOCAL} No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
파일 끝 개행 누락
YAMLlint가 파일 끝 개행 누락을 지적합니다. 일부 도구/플러그인에서 경고가 지속될 수 있으니 개행 추가를 권장합니다.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 26-26: no new line character at the end of file
(new-line-at-end-of-file)
🤖 Prompt for AI Agents
In src/main/resources/application-local.yml around line 26, the file is missing
a trailing newline at EOF which triggers YAMLlint warnings; fix it by adding a
single newline (line break) at the end of the file so the file ends with a
newline character and then save.
| servlet: | ||
| multipart: | ||
| max-file-size: 50MB | ||
| max-request-size: 200MB | ||
| jackson: | ||
| time-zone: Asia/Seoul | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
대용량 업로드 대비: 메모리 임계치/임시 디렉터리 설정 권장
요청/파일 크기 상한 설정은 적절합니다. 대용량 업로드 시 메모리 사용을 제어하기 위해 파일 임계치와 임시 저장 위치도 함께 지정하는 것을 권장합니다.
spring:
application:
name: rag-backend
servlet:
multipart:
max-file-size: 50MB
max-request-size: 200MB
+ # 메모리 임계치 초과 시 디스크로 스풀링
+ file-size-threshold: 1MB
+ # (선택) 임시 디렉터리 지정 — 배포 환경의 쓰기 가능한 경로로 설정
+ # location: /tmp/uploads
jackson:
time-zone: Asia/Seoul주의: 컨테이너/오케스트레이션 환경에서 location 경로의 퍼미션과 디스크 용량을 사전에 점검하세요.
📝 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.
| servlet: | |
| multipart: | |
| max-file-size: 50MB | |
| max-request-size: 200MB | |
| jackson: | |
| time-zone: Asia/Seoul | |
| spring: | |
| application: | |
| name: rag-backend | |
| servlet: | |
| multipart: | |
| max-file-size: 50MB | |
| max-request-size: 200MB | |
| # 메모리 임계치 초과 시 디스크로 스풀링 | |
| file-size-threshold: 1MB | |
| # (선택) 임시 디렉터리 지정 — 배포 환경의 쓰기 가능한 경로로 설정 | |
| # location: /tmp/uploads | |
| jackson: | |
| time-zone: Asia/Seoul |
🤖 Prompt for AI Agents
In src/main/resources/application.yml around lines 7 to 13, add multipart file
threshold and temporary storage settings so large uploads don't stay in memory:
under servlet.multipart add file-size-threshold (e.g. a sensible byte value or
"2MB") and location (path for temp files), ensuring the path is writable and has
enough disk space in container/orchestration environments; update keys
consistently (spring.servlet.multipart.* if you use the spring prefix) and
document permission/space requirements for the chosen location.
| import simplerag.ragback.global.util.S3Type | ||
| import simplerag.ragback.global.util.S3Util | ||
| import simplerag.ragback.global.util.sha256Hex | ||
| import java.security.MessageDigest |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
불필요한 import 제거
위 변경을 적용하면 MessageDigest 직접 사용이 사라지므로 import를 제거해 주세요.
-import java.security.MessageDigest📝 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.security.MessageDigest |
🤖 Prompt for AI Agents
In
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
around line 26, the import "java.security.MessageDigest" is now unused after the
recent changes; remove that unused import line to clean up the file and avoid
unused import warnings.
| @Test | ||
| @DisplayName("files와 items 개수 불일치 시 예외를 던진다.") | ||
| fun invalidInputCountMismatch() { | ||
| // given | ||
| val req = DataFileBulkCreateRequest(items = emptyList()) | ||
| val f = file("a.txt", "a".toByteArray()) | ||
|
|
||
| // when | ||
| val ex = assertThrows(CustomException::class.java) { service.upload(listOf(f), req) } | ||
|
|
||
| // then | ||
| assertEquals(ErrorCode.INVALID_INPUT, ex.errorCode) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
입력 유효성(파일/아이템 개수 불일치) 테스트 케이스 보강 제안
현재는 items가 빈 경우만 검증합니다. 개수가 서로 다른 비어있지 않은 경우(예: files=2, items=1)도 INVALID_INPUT을 보장하는 테스트를 추가하면 리그레션 방지에 도움이 됩니다.
원하시면 해당 케이스를 추가한 테스트 메서드 스니펫을 제공하겠습니다.
🤖 Prompt for AI Agents
In
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
around lines 77 to 89, add an additional test that verifies a non-empty count
mismatch (e.g., files size = 2 and items size = 1) also throws CustomException
with ErrorCode.INVALID_INPUT; create two test File-like objects (or use existing
file(...) helper) and a DataFileBulkCreateRequest with one item, call
service.upload(files, req) inside assertThrows, and assert the thrown
exception's errorCode equals ErrorCode.INVALID_INPUT.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (4)
26-26: 로거 카테고리 오타: GlobalExceptionHandler → S3UtilImpl현재 로거가 GlobalExceptionHandler로 초기화되어 있습니다. 이 클래스 자체(S3UtilImpl)로 카테고리를 맞추는 것이 정석입니다. 불필요한 import도 제거하세요.
적용 diff:
-import simpleragback.global.error.GlobalExceptionHandler @@ - private val log = LoggerFactory.getLogger(GlobalExceptionHandler::class.java) + private val log = LoggerFactory.getLogger(S3UtilImpl::class.java)Also applies to: 9-9
31-33: contentType 결정 로직 일원화 권장: FileConvertUtil 사용업로드 시 contentType을 MultipartFile에서 직접 읽지 말고, 운영에서 사용하는 MIME 판별 유틸과 일관되게 처리하는 편이 안전합니다. 저장 메타데이터/런타임 판별 간 정합성을 맞추기 위함입니다.
적용 diff(유틸 존재 가정):
- val contentType = file.contentType ?: "application/octet-stream" + val contentType = FileConvertUtil.resolveContentType(file.originalFilename, file.contentType)유틸 존재/시그니처 확인용 스크립트:
#!/bin/bash # FileConvertUtil 및 resolveContentType 존재 여부/시그니처 탐색 set -e rg -nP -C3 --type=kt '(\bclass|\bobject)\s+FileConvertUtil\b' rg -nP -C2 --type=kt '\bresolveContentType\s*\('
47-56: 예외 원인(cause) 보존 + generic Exception 축소 및 로그 컨텍스트 보강
- detekt 경고(TooGenericExceptionCaught, SwallowedException) 해소 필요.
- 업로드 실패 시 원인 예외를 보존해 추적 가능하게 하고, generic Exception 대신 SdkClientException 등 구체 예외를 우선 포착하세요.
- 로그에 bucket/key를 포함해 디버깅 컨텍스트를 강화하세요.
적용 diff:
} catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { log.error("S3 putObject fail bucket={}, key={}, status={}, awsCode={}, reqId={}, msg={}", bucket, key, e.statusCode(), e.awsErrorDetails()?.errorCode(), e.requestId(), e.awsErrorDetails()?.errorMessage(), e ) - throw S3Exception(ErrorCode.S3_UPLOAD_FAIL) - } catch (e: Exception) { - log.error(e.message, e) - throw S3Exception(ErrorCode.S3_UPLOAD_FAIL) + throw S3Exception(ErrorCode.S3_UPLOAD_FAIL, e) + } catch (e: software.amazon.awssdk.core.exception.SdkClientException) { + log.error("S3 putObject fail bucket={}, key={}, msg={}", bucket, key, e.message, e) + throw S3Exception(ErrorCode.S3_UPLOAD_FAIL, e)추가 import(파일 상단):
import software.amazon.awssdk.core.exception.SdkClientException
76-81: S3 삭제 에러 매핑 정교화 + cause 보존모든 S3Exception을 OBJECT_NOT_FOUND로 매핑하는 것은 과도합니다. NoSuchKey(또는 404)만 OBJECT_NOT_FOUND로, 그 외는 DELETE_FAIL로 구분하고 원인 예외를 보존하세요. 또한 클라이언트 계층 예외(SdkClientException)도 분기 처리하세요.
적용 diff:
- } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { - // NoSuchKey 등 - throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) - } catch (e: Exception) { - throw S3Exception(ErrorCode.S3_DELETE_FAIL) + } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { + val code = e.awsErrorDetails()?.errorCode() + val status = e.statusCode() + if ((code != null && code.equals("NoSuchKey", ignoreCase = true)) || status == 404) { + throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND, e) + } else { + log.error("S3 deleteObject fail bucket={}, key={}, status={}, awsCode={}, reqId={}, msg={}", + bucket, key, status, code, e.requestId(), e.awsErrorDetails()?.errorMessage(), e) + throw S3Exception(ErrorCode.S3_DELETE_FAIL, e) + } + } catch (e: software.amazon.awssdk.core.exception.SdkClientException) { + log.error("S3 deleteObject client fail bucket={}, key={}, msg={}", bucket, key, e.message, e) + throw S3Exception(ErrorCode.S3_DELETE_FAIL, e)추가 import(파일 상단, 아직 없다면):
import software.amazon.awssdk.core.exception.SdkClientExceptionsrc/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (3)
16-26: 이전 리뷰 피드백이 잘 반영되었습니다 — 컬럼 맵핑/제약 정합성 OK
- type → file_type 명시적 매핑, sha256 길이 64, fileUrl NotNull+길이 2048, size_bytes 네이밍 모두 적절합니다.
- sha256 유니크 제약(@table uniqueConstraints)과도 일관됩니다.
Also applies to: 19-21, 22-23
11-36: Kotlin JPA 플러그인(또는 no-arg/all-open) 적용 여부 확인 필요엔티티가 기본(final) 클래스이고 no-arg 생성자가 없으므로, Kotlin JPA 플러그인(또는 all-open/no-arg)이 미적용이면 런타임 프록시 생성 시 문제가 발생합니다. 플러그인 적용 여부를 확인해 주세요. 미적용 시 플러그인 추가 또는 클래스 open 처리/보호 생성자 제공이 필요합니다.
다음 스크립트로 Gradle 설정과 Auditing 활성화 여부를 점검할 수 있습니다:
#!/bin/bash set -e echo "🔎 Kotlin JPA/all-open/no-arg 플러그인 탐색" rg -n -C2 -i 'plugins\s*\{|id\s*\(\s*"org\.jetbrains\.kotlin\.plugin\.jpa"|kotlin\("plugin\.jpa"\)|org\.jetbrains\.kotlin\.plugin\.allopen|kotlin\("plugin\.allopen"\)|org\.jetbrains\.kotlin\.plugin\.noarg|kotlin\("plugin\.noarg"\)' build.gradle* **/build.gradle* settings.gradle* **/settings.gradle* || true echo -e "\n🔎 @EnableJpaAuditing 활성화 여부 탐색" rg -n -C2 '@EnableJpaAuditing' -g '!**/build/**' || true
34-36: 엔티티 equals/hashCode 구현 권장 — 식별자(id) 기반JPA 엔티티는 식별자 기반 동등성 비교가 안전합니다. 기본 레퍼런스 비교만 사용하면 컬렉션 동작/변경감지에 혼선을 줄 수 있습니다. 아래와 같이 class body를 열어 id 기반 equals/hashCode를 추가하세요.
-) +){ + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is DataFile) return false + return id != null && id == other.id + } + override fun hashCode(): Int = id?.hashCode() ?: System.identityHashCode(this) +}src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (1)
29-36: 201 Created 반환 시 Location 헤더(선택) 고려다건 생성의 경우 컬렉션 조회 엔드포인트를 Location으로 제공하면 REST 사용성이 좋아집니다. 적용 시 ResponseEntity로 전환해야 합니다.
예시 패치(선택):
- @ResponseStatus(HttpStatus.CREATED) - fun upload( + @ResponseStatus(HttpStatus.CREATED) + fun upload( @RequestPart("files") files: List<MultipartFile>, @Valid @RequestPart("request") req: DataFileBulkCreateRequest - ): ApiResponse<DataFileResponseList> { - val saved = service.upload(files, req) - return ApiResponse.ok(saved, "업로드 완료") - } + ): ApiResponse<DataFileResponseList> { + val saved = service.upload(files, req) + // Location을 컬렉션 리소스(예: 목록 조회 URI)로 지정 + // ApiResponse 래퍼를 유지하면서 헤더를 세팅하려면 ControllerAdvice or 필터에서 공통 처리하거나 + // ResponseEntity로 전환하는 방식을 검토하세요. + return ApiResponse.ok(saved, "업로드 완료") + }참고: ApiResponse를 그대로 유지한다면 Location 헤더 설정을 위한 공통 응답 빌더/인터셉터 도입도 한 방법입니다.
📜 Review details
Configuration used: CodeRabbit UI
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 (3)
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
urlFromKey(33-33)keyFromUrl(44-46)delete(42-42)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (3)
urlFromKey(7-7)keyFromUrl(10-10)delete(9-9)
🪛 detekt (1.23.8)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt
[warning] 53-53: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 79-79: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 76-76: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 79-79: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (1)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (1)
90-93: 검증 완료: S3Type.label은 빈 문자열이 아닙니다
현재 enum 클래스S3Type에 정의된 모든label값은"/…/"형태로 비어 있지 않으므로,dir.trim('/')이후 빈 문자열이 될 가능성이 없습니다.
따라서 제안하신 분기 처리는 실제로 실행되지 않으며, 해당 변경은 완전한 선택 사항입니다.
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
build.gradle (1)
34-39: jackson-module-kotlin 중복 선언 제거동일 의존성이 두 번 선언되어 있습니다. 중복은 불필요하며(해결은 되지만 노이즈를 유발) 유지보수에 불리합니다. 하나만 남겨주세요.
적용 diff 예시(상단 선언 제거):
- implementation("com.fasterxml.jackson.module:jackson-module-kotlin")
♻️ Duplicate comments (6)
build.gradle (1)
53-56: AWS SDK BOM 도입 잘 적용됨BOM으로 버전 정합성 확보 후 모듈에서 개별 버전 제거한 구성은 모범 사례입니다. 향후 다른 AWS 모듈 추가 시에도 일관됩니다.
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (1)
24-29: produces=application/json 명시로 콘텐츠 협상/문서 명확화멀티파트만 소비하도록 제한한 점은 좋습니다. 응답이 JSON임을 명시하면 OpenAPI 문서와 클라이언트 생성기가 더 정확해집니다.
- @PostMapping( - consumes = [ - MediaType.MULTIPART_FORM_DATA_VALUE, - ] - ) + @PostMapping( + consumes = [MediaType.MULTIPART_FORM_DATA_VALUE], + produces = [MediaType.APPLICATION_JSON_VALUE] + )src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt (2)
193-198: SHA-256 유틸 재사용으로 중복 제거 및 단순화직접 MessageDigest를 사용하여 12자 해시 프리픽스를 만들기보다, 프로덕션 유틸 sha256Hex를 재사용하면 중복이 줄고 가독성이 좋아집니다.
- val hash12 = MessageDigest.getInstance("SHA-256") - .digest(bytes).joinToString("") { "%02x".format(it) } - .take(12) + val hash12 = sha256Hex(bytes).take(12)
25-25: 불필요한 import 제거위 변경을 적용하면 MessageDigest import가 불필요해집니다. 제거해주세요.
- import java.security.MessageDigestsrc/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt (1)
12-24: 에러 코드 확장/정리: LGTM파일/S3 관련 시나리오를 포괄하는 코드들이 명확히 추가되었습니다. 과거 스타일 이슈(상수와 괄호 사이 공백)도 해소된 것으로 보입니다.
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (1)
66-70: FileException에서 SHA256 노출 정책 반영: LGTM사용자 응답에 SHA256만 노출하도록 맞춘 구현은 보안/추적성 측면에서 적절합니다. 서비스에서 FileException 메시지에 sha256을 전달하는지(파일명/경로가 아닌지)만 유지해 주세요.
📜 Review details
Configuration used: CodeRabbit UI
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)
build.gradle(2 hunks)src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/ErrorCode.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt(1 hunks)src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-16T11:30:00.127Z
Learnt from: catturtle123
PR: Simple-RAG/BACK#2
File: src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt:29-35
Timestamp: 2025-08-16T11:30:00.127Z
Learning: FileException 오류 메시지에서는 파일명 대신 SHA256을 노출하는 것이 보안상 더 안전하며, 파일 내용을 고유하게 식별할 수 있어 디버깅에도 효과적이다.
Applied to files:
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
🧬 Code Graph Analysis (1)
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt (1)
src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt (1)
sha256Hex(13-16)
🔇 Additional comments (2)
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (1)
17-22: @validated 추가와 의존성 주입 구성: LGTM메서드 파라미터 제약(@SiZe 등)을 활성화하기 위한 @validated 적용이 적절합니다.
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (1)
39-64: JSON 파싱/바인딩 오류 메시지 세분화: LGTMInvalidNull, InvalidFormat, MismatchedInput, JsonParse에 따라 사용자 메시지를 구체화한 점이 좋습니다. INVALID_JSON 코드와도 일관됩니다.
| val saved = dataFileService.upload(files, req) | ||
| return ApiResponse.ok(saved, "업로드 완료") |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
(선택) Location 헤더 설정 고려
다건 생성이지만 컬렉션 조회 엔드포인트를 Location으로 제공하면 REST 완결성이 올라갑니다. 현재는 @ResponseStatus로 201만 반환하므로, 원하시면 ResponseEntity.created(...)로 Location 헤더를 추가하는 형태로 바꾸는 스니펫을 드릴 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
around lines 40-41, the upload handler currently returns ApiResponse with
@ResponseStatus 201 but does not set a Location header; change the method to
return ResponseEntity<ApiResponse> and construct a Location URI pointing to the
collection retrieval endpoint (e.g., /api/data-files or the controller's list
path) using ServletUriComponentsBuilder or UriComponentsBuilder, then return
ResponseEntity.created(locationUri).body(ApiResponse.ok(saved, "업로드 완료")) so the
response includes the 201 status, Location header, and the body.
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
Show resolved
Hide resolved
| @Test | ||
| @DisplayName("files와 items 개수 불일치 시 예외를 던진다.") | ||
| fun invalidInputCountMismatch() { | ||
| // given | ||
| val req = DataFileBulkCreateRequest(items = emptyList()) | ||
| val f = file("a.txt", "a".toByteArray()) | ||
|
|
||
| // when | ||
| val ex = assertThrows(CustomException::class.java) { dataFileService.upload(listOf(f), req) } | ||
|
|
||
| // then | ||
| assertEquals(ErrorCode.INVALID_INPUT, ex.errorCode) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
비어있지 않은 개수 불일치 케이스 추가 제안
files=2, items=1처럼 둘 다 비어있지 않지만 개수가 다른 경우도 INVALID_INPUT을 보장하는 테스트를 추가하면 리그레션을 방지할 수 있습니다.
예시 스니펫(추가):
@Test
@DisplayName("files=2, items=1인 개수 불일치 시 INVALID_INPUT을 던진다.")
fun nonEmptyCountMismatchThrowsInvalidInput() {
// given
val f1 = file("a.txt", "a".toByteArray())
val f2 = file("b.txt", "b".toByteArray())
val req = DataFileBulkCreateRequest(items = listOf(DataFileCreateItem("only-one", emptyList())))
// when
val ex = assertThrows(CustomException::class.java) {
dataFileService.upload(listOf(f1, f2), req)
}
// then
assertEquals(ErrorCode.INVALID_INPUT, ex.errorCode)
}원하시면 PR에 맞춰 코드/메시지 규약까지 포함해 드리겠습니다.
🤖 Prompt for AI Agents
In
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
around lines 76 to 88, add a new test that covers the non-empty count mismatch
scenario (e.g., files.size = 2, items.size = 1) to assert the service throws
CustomException with ErrorCode.INVALID_INPUT; create two sample files and a
request with a single item, call dataFileService.upload(files, req) inside
assertThrows, and assert the thrown exception's errorCode equals
ErrorCode.INVALID_INPUT so this regression is prevented.
🧹 Nitpick (assertive)
테스트 이름/설명과 실제 케이스 불일치
현재 테스트는 items가 빈 경우만 검증하지만 DisplayName은 “개수 불일치”를 언급합니다. 의미가 어긋나므로 명칭을 정리하거나 실제 불일치 케이스를 추가하는 편이 좋습니다.
DisplayName만 정리하는 경우:
- @DisplayName("files와 items 개수 불일치 시 예외를 던진다.")
+ @DisplayName("items가 비어있으면 INVALID_INPUT을 던진다.")📝 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.
| @Test | |
| @DisplayName("files와 items 개수 불일치 시 예외를 던진다.") | |
| fun invalidInputCountMismatch() { | |
| // given | |
| val req = DataFileBulkCreateRequest(items = emptyList()) | |
| val f = file("a.txt", "a".toByteArray()) | |
| // when | |
| val ex = assertThrows(CustomException::class.java) { dataFileService.upload(listOf(f), req) } | |
| // then | |
| assertEquals(ErrorCode.INVALID_INPUT, ex.errorCode) | |
| } | |
| @Test | |
| @DisplayName("items가 비어있으면 INVALID_INPUT을 던진다.") | |
| fun invalidInputCountMismatch() { | |
| // given | |
| val req = DataFileBulkCreateRequest(items = emptyList()) | |
| val f = file("a.txt", "a".toByteArray()) | |
| // when | |
| val ex = assertThrows(CustomException::class.java) { dataFileService.upload(listOf(f), req) } | |
| // then | |
| assertEquals(ErrorCode.INVALID_INPUT, ex.errorCode) | |
| } |
🤖 Prompt for AI Agents
In
src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt
around lines 76 to 88, the test DisplayName says "files와 items 개수 불일치 시 예외를
던진다." but the test only covers the case where items is empty; fix by either
updating the DisplayName to reflect the actual case (e.g., "items가 비어있을 때 예외를
던진다.") or change the test to create a true count-mismatch scenario (e.g., pass a
non-empty items list whose size does not match the files list) and assert the
same exception and error code; ensure the chosen change keeps the test intent
clear and the assertions unchanged.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (17)
src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt (1)
6-9: Kotlin 관용적 시그니처 적용 적절함 (Optional → nullable)
Optional<Tag>대신Tag?반환은 Kotlin/Spring Data 관용에 부합하고 호출부 단순화에 유리합니다. 필요 빈도에 따라existsByName(name: String): Boolean추가도 고려해볼 만합니다.src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
35-40: deleteByUrl의 예외 정책이 운영 구현과 일치 — 좋습니다URL 파싱 실패 시
S3_INVALID_URL을 던지도록 맞춘 점이 운영 구현과 일관되어 테스트 신뢰성이 높아집니다.
46-49: keyFromUrl 정규화 동작 일치 — 선행 ‘/’ 제거 반영 확인운영 구현과 동일하게 선행 슬래시 제거를 적용해 URL↔key 변환 일관성을 확보했습니다. 적절합니다.
25-31: 빈 prefix 시 선행 슬래시('/')가 포함되어 키-URL 왕복 불일치 발생 가능
prefix가 빈 문자열이면key가"/{hash}_..."형태가 됩니다.urlFromKey는"fake://$key"를,keyFromUrl은 선행/을 제거하여 서로 다른 키가 되어deleteByUrl이 동작하지 않을 수 있습니다. 선행 슬래시를 피하도록 보완하세요.- val hash = sha256Hex(file.bytes).take(12) - val prefix = dir.label.trim('/') - val key = "$prefix/${hash}_$clean" + val hash = sha256Hex(file.bytes).take(12) + val prefix = dir.label.trim('/') + val key = if (prefix.isBlank()) { + "${hash}_$clean" + } else { + "$prefix/${hash}_$clean" + }src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (4)
9-11: Logger 카테고리 수정: 현재 GlobalExceptionHandler로 잘못 지정됨로거 카테고리는 현재 클래스인
S3UtilImpl을 사용해야 합니다. 불필요한 import도 제거하세요.-import simplerag.ragback.global.error.GlobalExceptionHandler @@ - private val log = LoggerFactory.getLogger(GlobalExceptionHandler::class.java) + private val log = LoggerFactory.getLogger(S3UtilImpl::class.java)Also applies to: 26-26
31-33: Content-Type 해석 일관화: resolveContentType 사용 권장파일의
contentType이 비어있거나 부정확한 경우가 있어, 유틸의 해석 로직(resolveContentType)과 일치시키는 편이 안전합니다. 운영/테스트 메타데이터 정합성도 좋아집니다.- val contentType = file.contentType ?: "application/octet-stream" + val contentType = file.resolveContentType()
47-57: 예외의 원인(cause) 보존 및 로그 컨텍스트 확장현재
S3Exception생성 시 cause를 잃어 디버깅이 어렵습니다. 원인 예외를 연결하고, generic 예외 로그에 bucket/key 컨텍스트를 포함하세요.} catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { log.error( "S3 putObject fail bucket={}, key={}, status={}, awsCode={}, reqId={}, msg={}", bucket, key, e.statusCode(), e.awsErrorDetails()?.errorCode(), e.requestId(), e.awsErrorDetails()?.errorMessage(), e ) - throw S3Exception(ErrorCode.S3_UPLOAD_FAIL) + throw S3Exception(ErrorCode.S3_UPLOAD_FAIL, e) } catch (e: Exception) { - log.error(e.message, e) - throw S3Exception(ErrorCode.S3_UPLOAD_FAIL) + log.error("S3 putObject fail bucket={}, key={}, msg={}", bucket, key, e.message, e) + throw S3Exception(ErrorCode.S3_UPLOAD_FAIL, e) }
70-83: S3 삭제 에러 매핑 정교화: NoSuchKey만 OBJECT_NOT_FOUND로모든 SDK 예외를
S3_OBJECT_NOT_FOUND로 매핑하는 것은 과도합니다. 404/NoSuchKey만 객체 없음으로, 그 외는S3_DELETE_FAIL로 구분하세요. 필요 시 로그도 추가하세요.} catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { - // NoSuchKey 등 - throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) + val code = e.awsErrorDetails()?.errorCode() + val status = e.statusCode() + if ((code != null && code.equals("NoSuchKey", ignoreCase = true)) || status == 404) { + throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND, e) + } else { + throw S3Exception(ErrorCode.S3_DELETE_FAIL, e) + } } catch (e: Exception) { - throw S3Exception(ErrorCode.S3_DELETE_FAIL) + throw S3Exception(ErrorCode.S3_DELETE_FAIL, e) }src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (2)
64-66: 엔티티 생성 시 명명 인자 사용으로 안전성/가독성 향상위치 인자는 파라미터 순서 변경 시 오류 유발 여지가 큽니다. 명명 인자를 사용하세요. 엔티티 실제 필드명(
sizeBytes,createdAt,updatedAt)에 맞춰 조정 바랍니다.- val dataFile = try { - dataFileRepository.save(DataFile(meta.title, type, sizeByte, sha256, fileUrl, now, now)) - } catch (ex: DataIntegrityViolationException) { + val dataFile = try { + dataFileRepository.save( + DataFile( + title = meta.title, + type = type, + sizeBytes = sizeByte, + sha256 = sha256, + fileUrl = fileUrl, + createdAt = now, + updatedAt = now + ) + ) + } catch (ex: DataIntegrityViolationException) { throw FileException(ErrorCode.ALREADY_FILE, sha256) }
79-89: 롤백 보상 삭제 시 예외를 삼키지 말고 경고 로그 남기기보상 삭제 실패가 추적되지 않습니다. 최소한 경고 로그를 남겨 관찰 가능하게 하세요.
- uploadedUrls.forEach { runCatching { s3Util.deleteByUrl(it) } } + uploadedUrls.forEach { url -> + runCatching { s3Util.deleteByUrl(url) } + .onFailure { e -> + // TODO: 클래스 상단에 logger 선언 후 사용 + // private val log = org.slf4j.LoggerFactory.getLogger(DataFileService::class.java) + // log.warn("Rollback cleanup failed for S3 object: {}", url, e) + } + }src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (7)
3-14: 에러 응답 HTTP 상태코드가 누락됩니다 — ResponseEntity import 추가 필요현재 모든 핸들러가 ApiResponse 객체만 반환하고 있어 상태코드가 200 OK로 고정될 위험이 큽니다. ResponseEntity로 감싸 상태코드를 명시적으로 설정해야 합니다. 우선 ResponseEntity import를 추가해 주세요.
적용 diff:
import com.fasterxml.jackson.core.JsonParseException import com.fasterxml.jackson.databind.exc.InvalidFormatException import com.fasterxml.jackson.databind.exc.InvalidNullException import com.fasterxml.jackson.databind.exc.MismatchedInputException import jakarta.validation.ConstraintViolationException import org.slf4j.LoggerFactory import org.springframework.http.converter.HttpMessageNotReadableException +import org.springframework.http.ResponseEntity import org.springframework.web.bind.MethodArgumentNotValidException import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.RestControllerAdvice import org.springframework.web.multipart.support.MissingServletRequestPartException import simplerag.ragback.global.response.ApiResponse
21-25: 예외 응답이 200 OK로 반환될 수 있습니다 — ResponseEntity로 상태코드 반영스프링에서 바디만 반환하면 기본 200이 내려갑니다. INVALID_INPUT의 HTTP 상태를 반영하도록 반환 타입과 반환 값을 수정하세요.
적용 diff:
@ExceptionHandler(MethodArgumentNotValidException::class) -fun handleValidationException(ex: MethodArgumentNotValidException): ApiResponse<Nothing> { +fun handleValidationException(ex: MethodArgumentNotValidException): ResponseEntity<ApiResponse<Nothing>> { val message = ex.bindingResult.allErrors.first().defaultMessage ?: "잘못된 요청" - return ApiResponse.fail(ErrorCode.INVALID_INPUT.code, message) + return ResponseEntity + .status(ErrorCode.INVALID_INPUT.status) + .body(ApiResponse.fail(ErrorCode.INVALID_INPUT.code, message)) }테스트에서도 해당 케이스에 대해 status 단언을 추가하는 것을 권장합니다.
27-31: ConstraintViolationException도 상태코드 반영 필요동일 이슈입니다. INVALID_INPUT 상태코드를 내려주세요.
적용 diff:
@ExceptionHandler(ConstraintViolationException::class) -fun handleConstraintViolation(ex: ConstraintViolationException): ApiResponse<Nothing> { +fun handleConstraintViolation(ex: ConstraintViolationException): ResponseEntity<ApiResponse<Nothing>> { val message = ex.constraintViolations.firstOrNull()?.message ?: "잘못된 요청" - return ApiResponse.fail(ErrorCode.INVALID_INPUT.code, message) + return ResponseEntity + .status(ErrorCode.INVALID_INPUT.status) + .body(ApiResponse.fail(ErrorCode.INVALID_INPUT.code, message)) }
33-37: 필수 파트 누락 응답에 상태코드 반영 필요FILE_PART_MISSING에 대응하는 HTTP 상태를 포함해야 합니다.
적용 diff:
@ExceptionHandler(MissingServletRequestPartException::class) -fun handleMissingPart(e: MissingServletRequestPartException): ApiResponse<Nothing> { +fun handleMissingPart(e: MissingServletRequestPartException): ResponseEntity<ApiResponse<Nothing>> { val msg = "필수 '${e.requestPartName}' 가 없습니다." - return ApiResponse.fail(ErrorCode.FILE_PART_MISSING.code, message = msg) + return ResponseEntity + .status(ErrorCode.FILE_PART_MISSING.status) + .body(ApiResponse.fail(ErrorCode.FILE_PART_MISSING.code, message = msg)) }
76-80: CustomException 응답도 상태코드 반영 필요각 에러코드의 status를 실제 HTTP 응답으로 내려주세요.
적용 diff:
@ExceptionHandler(CustomException::class) -fun handleCustomException(ex: CustomException): ApiResponse<Nothing> { +fun handleCustomException(ex: CustomException): ResponseEntity<ApiResponse<Nothing>> { val errorCode = ex.errorCode - return ApiResponse.fail(errorCode.code, errorCode.message) + return ResponseEntity + .status(errorCode.status) + .body(ApiResponse.fail(errorCode.code, errorCode.message)) }
82-87: 일반 예외 처리도 500 등 적절한 상태코드를 내려야 합니다로깅은 적절합니다. 다만 현재는 200 OK가 내려갈 가능성이 있어 사용자/클라이언트를 오도합니다. INTERNAL_ERROR의 status를 설정하세요.
적용 diff:
@ExceptionHandler(Exception::class) -fun handleGeneralException(ex: Exception): ApiResponse<Nothing> { +fun handleGeneralException(ex: Exception): ResponseEntity<ApiResponse<Nothing>> { log.error("Unhandled exception", ex) - return ApiResponse.fail(ErrorCode.INTERNAL_ERROR.code, ErrorCode.INTERNAL_ERROR.message) + return ResponseEntity + .status(ErrorCode.INTERNAL_ERROR.status) + .body(ApiResponse.fail(ErrorCode.INTERNAL_ERROR.code, ErrorCode.INTERNAL_ERROR.message)) }
70-74: FileException 핸들러 개선: HTTP 상태코드 반영 필수
- 확인 완료:
FileException(ErrorCode.ALREADY_FILE, sha256)로 던질 때ex.message에 SHA256 값이 들어오므로, 메시지 유효성이 보장됩니다.- 개선 필요: 현재 200 OK로 응답될 수 있으므로,
errorCode.status를 HTTP 상태코드로 반영하도록 핸들러 반환 타입과 응답 구조를 변경해주세요.대상 위치
- src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
적용 예시(diff):
@ExceptionHandler(FileException::class) -fun handleCustomException(ex: FileException): ApiResponse<Nothing> { +fun handleCustomException(ex: FileException): ResponseEntity<ApiResponse<Nothing>> { val errorCode = ex.errorCode - return ApiResponse.fail(errorCode.code, "${errorCode.message} sha256: ${ex.message}") + return ResponseEntity + .status(errorCode.status) + .body( + ApiResponse.fail( + errorCode.code, + "${errorCode.message} sha256: ${ex.message}" + ) + ) }
📜 Review details
Configuration used: CodeRabbit UI
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)
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt(1 hunks)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-16T11:30:00.127Z
Learnt from: catturtle123
PR: Simple-RAG/BACK#2
File: src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt:29-35
Timestamp: 2025-08-16T11:30:00.127Z
Learning: FileException 오류 메시지에서는 파일명 대신 SHA256을 노출하는 것이 보안상 더 안전하며, 파일 내용을 고유하게 식별할 수 있어 디버깅에도 효과적이다.
Applied to files:
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
🧬 Code Graph Analysis (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
src/main/kotlin/simplerag/ragback/global/util/FileConvertUtil.kt (1)
sha256Hex(13-16)src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
urlFromKey(60-63)keyFromUrl(96-103)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (2)
urlFromKey(7-7)keyFromUrl(10-10)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
urlFromKey(33-33)keyFromUrl(46-48)delete(42-44)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (3)
urlFromKey(7-7)keyFromUrl(10-10)delete(9-9)
🪛 detekt (1.23.8)
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
[warning] 54-54: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 80-80: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 77-77: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 80-80: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (3)
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt (1)
7-10: 응답 DTO 구성 타당 — 최소 노출 원칙 준수업로드 직후 클라이언트가 필요한 최소 정보(id, sha256)만 노출하는 설계는 적절합니다. 이후 확장(예: title, url)이 필요할 때를 대비해 DTO를 별도로 둔 것도 좋습니다.
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (1)
57-60: 중복 검사 선행 + 제약 위반 매핑 적절함
existsBySha256로 사전 차단 후,DataIntegrityViolationException을ALREADY_FILE로 매핑하여 경쟁 조건을 보완한 점이 좋습니다.src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (1)
19-20: 로거 도입으로 printStackTrace 제거 — 잘 반영되었습니다SLF4J 로거로 전환하여 구조적 로깅을 사용하신 점 좋습니다.
| data class DataFileResponseList( | ||
| val dataFilePreviewResponseList: List<DataFilePreviewResponse>, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
응답 목록 필드명 간결화 제안: items로 축약
현재 dataFilePreviewResponseList는 중복된 의미가 반복되어 가독성이 떨어집니다. API 응답 스키마에서도 items 같은 관용적 이름이 더 읽기 좋습니다. 하위 영향 범위가 허용된다면 다음처럼 변경을 권장합니다.
-data class DataFileResponseList(
- val dataFilePreviewResponseList: List<DataFilePreviewResponse>,
-)
+data class DataFileResponseList(
+ val items: List<DataFilePreviewResponse>,
+)호출부 및 테스트의 필드 접근도 함께 정리해야 합니다.
📝 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.
| data class DataFileResponseList( | |
| val dataFilePreviewResponseList: List<DataFilePreviewResponse>, | |
| ) | |
| data class DataFileResponseList( | |
| val items: List<DataFilePreviewResponse>, | |
| ) |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt
around lines 3 to 5, rename the DTO property dataFilePreviewResponseList to a
concise items to improve readability and API ergonomics; update the data class
definition accordingly, then refactor all call sites and unit/integration tests
to use the new items property name; if your JSON mapper requires explicit names
(e.g., Jackson/Moshi/kotlinx.serialization), add the appropriate serialization
annotation to preserve the external field name or map to "items" to avoid
breaking clients.
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
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)
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-16T11:30:00.127Z
Learnt from: catturtle123
PR: Simple-RAG/BACK#2
File: src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt:29-35
Timestamp: 2025-08-16T11:30:00.127Z
Learning: FileException 오류 메시지에서는 파일명 대신 SHA256을 노출하는 것이 보안상 더 안전하며, 파일 내용을 고유하게 식별할 수 있어 디버깅에도 효과적이다.
Applied to files:
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt
🔇 Additional comments (7)
src/main/kotlin/simplerag/ragback/global/error/GlobalExceptionHandler.kt (7)
3-13: Jackson/HTTP 관련 예외 import 추가 적절필요한 예외 타입들만 깔끔하게 가져왔고, 사용처와 일치합니다.
19-19: 로거 도입으로 표준화된 예외 로깅 — 좋습니다SLF4J 로거 추가는 운영 관측성 측면에서 바람직합니다.
21-27: 검증 예외에 HTTP 상태 반영 — OKResponseEntity로 ErrorCode.INVALID_INPUT.status를 내려 일관된 오류 응답을 보장합니다.
29-35: 제약 위반 예외 처리 — 상태/본문 매핑 일치ConstraintViolationException도 동일한 패턴으로 처리되어 일관성이 좋습니다.
90-90: 구조적 에러 로깅으로 전환 — OK예외 스택을 표준 로거로 기록하여 운영에서 추적이 용이합니다.
37-43: 메시지 개선 및 업로드 용량 초과 예외 핸들러 추가 제안GlobalExceptionHandler.kt(37–43)에서 다음을 적용해 보세요:
- 메시지 문구 소폭 개선
- MaxUploadSizeExceededException 핸들러 추가 고려
메시지 문구 수정(diff):
- val msg = "필수 '${e.requestPartName}' 가 없습니다." + val msg = "필수 파트 '${e.requestPartName}'가 없습니다."업로드 한도 초과 예외 핸들러 추가 예시:
@ExceptionHandler(MaxUploadSizeExceededException::class) fun handleMaxSize(e: MaxUploadSizeExceededException): ResponseEntity<ApiResponse<Nothing>> { return ResponseEntity .status(HttpStatus.PAYLOAD_TOO_LARGE) .body(ApiResponse.fail("FILE_TOO_LARGE", "파일 크기가 허용 한도를 초과했습니다.")) }– 위에서 사용한
"FILE_TOO_LARGE"코드가 ErrorCode enum에 없으므로,
필요 시ErrorCode에 새 항목을 추가하거나 기존 코드를 재활용하세요.[optional_refactors_recommended]
72-78: 메시지 포맷 및 FileException 설계 개선 제안
- 현재
FileException의message는 non-null이므로?.호출 대신 빈 문자열 여부만 체크하면 충분합니다. 예를 들어:.body( ApiResponse.fail( errorCode.code, ex.message .takeIf { it.isNotBlank() } ?.let { "${errorCode.message} (SHA256: $it)" } ?: errorCode.message ) )- 추가로, SHA256 값을
message대신 별도 프로퍼티(val sha256: String)로 두면 표현이 더 명확해집니다.
[optional_refactors_recommended]
| @ExceptionHandler(HttpMessageNotReadableException::class) | ||
| fun handleUnreadable(e: HttpMessageNotReadableException): ResponseEntity<ApiResponse<Nothing>> { | ||
| val cause = e.cause | ||
| val msg = when (cause) { | ||
| is InvalidNullException -> { | ||
| val field = cause.path.lastOrNull()?.fieldName ?: "unknown" | ||
| "'$field' 값이 비어있습니다." | ||
| } | ||
|
|
||
| is InvalidFormatException -> { | ||
| val field = cause.path.lastOrNull()?.fieldName ?: "unknown" | ||
| "'$field' 값 형식이 올바르지 않습니다." | ||
| } | ||
|
|
||
| is MismatchedInputException -> { | ||
| val field = cause.path.lastOrNull()?.fieldName ?: "unknown" | ||
| "'$field' 값 타입이 올바르지 않습니다." | ||
| } | ||
|
|
||
| else -> "유효하지 않은 요청입니다." | ||
| } | ||
| return ResponseEntity | ||
| .status(ErrorCode.INVALID_JSON.status) | ||
| .body(ApiResponse.fail(ErrorCode.INVALID_JSON.code, message = msg)) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
JSON 파싱 예외 원인 추출 개선 — mostSpecificCause 사용, 경로 포맷팅 상세화, 디버그 로그 추가 제안
현재 e.cause만 확인하고 마지막 필드만 노출합니다. Jackson 예외는 중첩되는 경우가 많으므로 mostSpecificCause를 사용하고, 경로는 필드/인덱스를 모두 포함해 조합하면 디버깅에 유리합니다. 또한 원인 타입을 debug 로그로 남기면 운영 가시성이 좋아집니다.
권장 diff:
- fun handleUnreadable(e: HttpMessageNotReadableException): ResponseEntity<ApiResponse<Nothing>> {
- val cause = e.cause
+ fun handleUnreadable(e: HttpMessageNotReadableException): ResponseEntity<ApiResponse<Nothing>> {
+ val cause = e.mostSpecificCause ?: e.cause
+ log.debug("HttpMessageNotReadableException cause: {}", cause?.javaClass?.name)
val msg = when (cause) {
is InvalidNullException -> {
- val field = cause.path.lastOrNull()?.fieldName ?: "unknown"
+ val field = cause.path.joinToString(".") { it.fieldName ?: "[${it.index}]" }
+ .ifBlank { "unknown" }
"'$field' 값이 비어있습니다."
}
is InvalidFormatException -> {
- val field = cause.path.lastOrNull()?.fieldName ?: "unknown"
+ val field = cause.path.joinToString(".") { it.fieldName ?: "[${it.index}]" }
+ .ifBlank { "unknown" }
"'$field' 값 형식이 올바르지 않습니다."
}
is MismatchedInputException -> {
- val field = cause.path.lastOrNull()?.fieldName ?: "unknown"
+ val field = cause.path.joinToString(".") { it.fieldName ?: "[${it.index}]" }
+ .ifBlank { "unknown" }
"'$field' 값 타입이 올바르지 않습니다."
}
+ is com.fasterxml.jackson.core.JsonParseException -> {
+ "JSON 문법 오류입니다."
+ }
+
else -> "유효하지 않은 요청입니다."
}📝 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.
| @ExceptionHandler(HttpMessageNotReadableException::class) | |
| fun handleUnreadable(e: HttpMessageNotReadableException): ResponseEntity<ApiResponse<Nothing>> { | |
| val cause = e.cause | |
| val msg = when (cause) { | |
| is InvalidNullException -> { | |
| val field = cause.path.lastOrNull()?.fieldName ?: "unknown" | |
| "'$field' 값이 비어있습니다." | |
| } | |
| is InvalidFormatException -> { | |
| val field = cause.path.lastOrNull()?.fieldName ?: "unknown" | |
| "'$field' 값 형식이 올바르지 않습니다." | |
| } | |
| is MismatchedInputException -> { | |
| val field = cause.path.lastOrNull()?.fieldName ?: "unknown" | |
| "'$field' 값 타입이 올바르지 않습니다." | |
| } | |
| else -> "유효하지 않은 요청입니다." | |
| } | |
| return ResponseEntity | |
| .status(ErrorCode.INVALID_JSON.status) | |
| .body(ApiResponse.fail(ErrorCode.INVALID_JSON.code, message = msg)) | |
| } | |
| @ExceptionHandler(HttpMessageNotReadableException::class) | |
| fun handleUnreadable(e: HttpMessageNotReadableException): ResponseEntity<ApiResponse<Nothing>> { | |
| val cause = e.mostSpecificCause ?: e.cause | |
| log.debug("HttpMessageNotReadableException cause: {}", cause?.javaClass?.name) | |
| val msg = when (cause) { | |
| is InvalidNullException -> { | |
| val field = cause.path.joinToString(".") { it.fieldName ?: "[${it.index}]" } | |
| .ifBlank { "unknown" } | |
| "'$field' 값이 비어있습니다." | |
| } | |
| is InvalidFormatException -> { | |
| val field = cause.path.joinToString(".") { it.fieldName ?: "[${it.index}]" } | |
| .ifBlank { "unknown" } | |
| "'$field' 값 형식이 올바르지 않습니다." | |
| } | |
| is MismatchedInputException -> { | |
| val field = cause.path.joinToString(".") { it.fieldName ?: "[${it.index}]" } | |
| .ifBlank { "unknown" } | |
| "'$field' 값 타입이 올바르지 않습니다." | |
| } | |
| is com.fasterxml.jackson.core.JsonParseException -> { | |
| "JSON 문법 오류입니다." | |
| } | |
| else -> "유효하지 않은 요청입니다." | |
| } | |
| return ResponseEntity | |
| .status(ErrorCode.INVALID_JSON.status) | |
| .body(ApiResponse.fail(ErrorCode.INVALID_JSON.code, message = msg)) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (5)
9-9: 로거 카테고리 import 잘못됨 — GlobalExceptionHandler import 제거 필요이 파일의 로거는 S3UtilImpl을 카테고리로 사용해야 합니다. GlobalExceptionHandler import는 불필요합니다.
적용 diff:
-import simplerag.ragback.global.error.GlobalExceptionHandler
26-26: Logger 카테고리 잘못 지정 — S3UtilImpl로 교체로거 카테고리를 현재 클래스(S3UtilImpl)로 지정해 주세요.
- private val log = LoggerFactory.getLogger(GlobalExceptionHandler::class.java) + private val log = LoggerFactory.getLogger(S3UtilImpl::class.java)
31-33: Content-Type 결정 로직 정합성 개선 제안업로드 시 저장 메타의 contentType을 런타임 해석 로직과 일치시키는 것이 좋습니다. 파일명 기준으로 보정하는 유틸(FileConvertUtil 등)이 있다면 그것을 사용해 주세요.
제안 diff:
- val contentType = file.contentType ?: "application/octet-stream" + val contentType = FileConvertUtil.resolveContentType(file.originalFilename, file.contentType) + ?: "application/octet-stream"필요 시 import 추가:
// 프로젝트 내 실제 패키지 경로에 맞게 조정 import simplerag.ragback.global.util.FileConvertUtil검증 스크립트(유틸 존재/시그니처 확인):
#!/bin/bash # FileConvertUtil 및 resolveContentType 시그니처 검색 rg -n -C2 -g '!**/build/**' -P '\bobject\s+FileConvertUtil\b|\bclass\s+FileConvertUtil\b' rg -n -C2 -g '!**/build/**' -P '\bresolveContentType\s*\('
47-57: 업로드 예외의 원인 보존 및 로그 컨텍스트 보강 필요현재 업로드 실패 시 원인(cause)을 보존하지 않고 재래핑합니다. detekt의 swallowed/too generic 경고를 해소하고 디버깅 가능성을 높이기 위해 cause를 포함해 재던지고, generic 블록 로그에 bucket/key 컨텍스트를 포함해 주세요.
} catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { log.error( "S3 putObject fail bucket={}, key={}, status={}, awsCode={}, reqId={}, msg={}", bucket, key, e.statusCode(), e.awsErrorDetails()?.errorCode(), e.requestId(), e.awsErrorDetails()?.errorMessage(), e ) - throw S3Exception(ErrorCode.S3_UPLOAD_FAIL) + throw S3Exception(ErrorCode.S3_UPLOAD_FAIL, e) } catch (e: Exception) { - log.error(e.message, e) - throw S3Exception(ErrorCode.S3_UPLOAD_FAIL) + log.error("S3 putObject fail bucket={}, key={}, msg={}", bucket, key, e.message, e) + throw S3Exception(ErrorCode.S3_UPLOAD_FAIL, e) }추가로, 가능하면 Exception 대신 IOException/SdkClientException 등 구체 예외로 한정해 주세요.
70-82: S3 삭제 에러 매핑 정교화 및 cause 보존 필요현재 모든 S3Exception을 OBJECT_NOT_FOUND로 매핑합니다. NoSuchKey(404)만 OBJECT_NOT_FOUND로, 그 외는 DELETE_FAIL로 구분하고, 원인(cause)을 보존하세요. detekt의 swallowed/too generic 경고도 함께 해소됩니다.
try { val req = DeleteObjectRequest.builder() .bucket(bucket) .key(key) .build() s3.deleteObject(req) } catch (e: software.amazon.awssdk.services.s3.model.S3Exception) { - // NoSuchKey 등 - throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND) + val code = e.awsErrorDetails()?.errorCode() + val status = e.statusCode() + if ((code != null && code.equals("NoSuchKey", ignoreCase = true)) || status == 404) { + log.warn("S3 deleteObject not found bucket={}, key={}, status={}, awsCode={}, reqId={}", + bucket, key, status, code, e.requestId(), e) + throw S3Exception(ErrorCode.S3_OBJECT_NOT_FOUND, e) + } else { + log.error("S3 deleteObject fail bucket={}, key={}, status={}, awsCode={}, reqId={}, msg={}", + bucket, key, status, code, e.requestId(), e.awsErrorDetails()?.errorMessage(), e) + throw S3Exception(ErrorCode.S3_DELETE_FAIL, e) + } } catch (e: Exception) { - throw S3Exception(ErrorCode.S3_DELETE_FAIL) + log.error("S3 deleteObject fail bucket={}, key={}, msg={}", bucket, key, e.message, e) + throw S3Exception(ErrorCode.S3_DELETE_FAIL, e) }참고: 필요 시 아래 import를 추가해 구체 예외로 한정하는 것도 고려해 주세요.
import software.amazon.awssdk.core.exception.SdkClientException
📜 Review details
Configuration used: CodeRabbit UI
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)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
src/main/kotlin/simplerag/ragback/global/storage/FakeS3Util.kt (3)
urlFromKey(33-33)keyFromUrl(46-48)delete(42-44)src/main/kotlin/simplerag/ragback/global/util/S3Util.kt (3)
urlFromKey(7-7)keyFromUrl(10-10)delete(9-9)
🪛 detekt (1.23.8)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt
[warning] 54-54: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 80-80: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled.
(detekt.exceptions.TooGenericExceptionCaught)
[warning] 77-77: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 80-80: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (2)
src/main/kotlin/simplerag/ragback/global/util/S3UtilImpl.kt (2)
60-64: LGTM: urlFromKey 구현 적절SDK의 utilities().getUrl 사용으로 퍼블릭 URL 생성이 일관적입니다.
85-100: LGTM: 키(prefix) 빈 문자열 처리 반영빈 prefix에서 선행 '/'가 생기지 않도록 안전하게 처리되어 FakeS3Util 동작과도 정합적입니다.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt (1)
6-11: 태그 존재여부 빈번 체크 시 existsByName 추가 고려태그 존재 여부만 확인하는 호출이 잦다면 불필요한 로딩을 줄이기 위해
existsByName를 추가해 두는 것이 유용합니다.아래처럼 보강 가능합니다:
interface TagRepository : JpaRepository<Tag, Long> { fun findByName(name: String): Tag? fun findByNameIn(names: Collection<String>): List<Tag> + + fun existsByName(name: String): Boolean }src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (3)
52-56: 파일 메트릭 필드명 일관화(sizeByte → sizeBytes) 필요로컬 변수는
sizeBytes인데, 메트릭 필드는sizeByte로 표기되어 있어 혼동 소지가 있습니다. 유틸의 모델/프로퍼티명을sizeBytes로 통일하고 이곳에서도 동일 명칭을 사용하세요.적용 예시(유틸에
sizeBytes로 변경된 경우):- val sizeBytes = metrics.sizeByte + val sizeBytes = metrics.sizeBytes유틸(FileConvertUtil)의
FileMetrics정의도val sizeByte: Long→val sizeBytes: Long으로 함께 정리되어야 합니다. 관련 호출부 전체 영향 범위 점검을 권장합니다.
64-68: 예외 원인 보존 + DataFile 생성자 명명 인자로 전환
- detekt 경고대로 원인 예외가 삼켜집니다.
cause를 보존해 추후 트러블슈팅 가능성을 높이세요.- 엔티티 생성자 호출에 명명 인자를 사용하면 필드 순서 변경에 안전하고 가독성이 좋아집니다.
- val dataFile = try { - dataFileRepository.save(DataFile(meta.title, type, sizeBytes, sha256, fileUrl, now, now)) - } catch (ex: DataIntegrityViolationException) { - throw FileException(ErrorCode.ALREADY_FILE, sha256) - } + val dataFile = try { + dataFileRepository.save( + DataFile( + title = meta.title, + type = type, + sizeBytes = sizeBytes, + sha256 = sha256, + fileUrl = fileUrl, + createdAt = now, + updatedAt = now, + ) + ) + } catch (ex: DataIntegrityViolationException) { + // FileException에 cause 인자를 받는 생성자가 없다면, + // throw FileException(ErrorCode.ALREADY_FILE, sha256).also { it.initCause(ex) } 형태로 대체하세요. + throw FileException(ErrorCode.ALREADY_FILE, sha256, ex) + }
79-89: 롤백 보상 삭제 시 예외 삼킴 — 경고 로그 추가 권장S3 정리 실패를 조용히 무시하면 고아 오브젝트가 생겨도 추적이 어렵습니다. 최소한 warn 로그를 남기세요.
- uploadedUrls.forEach { runCatching { s3Util.deleteByUrl(it) } } + uploadedUrls.forEach { url -> + runCatching { s3Util.deleteByUrl(url) } + .onFailure { e -> log.warn("Rollback cleanup failed for S3 object: {}", url, e) } + }클래스 상단에 로거가 없다면 추가하세요(선택적 지원 코드):
private val log = org.slf4j.LoggerFactory.getLogger(DataFileService::class.java)
📜 Review details
Configuration used: CodeRabbit UI
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)
src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt(1 hunks)src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt(1 hunks)
🧰 Additional context used
🪛 detekt (1.23.8)
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
[warning] 66-66: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🔇 Additional comments (4)
src/main/kotlin/simplerag/ragback/domain/document/repository/TagRepository.kt (1)
8-11: Kotlin 관용적(nullable) 반환 채택 훌륭합니다
Optional<Tag>대신Tag?반환과findByNameIn제공은 Kotlin/Spring Data 관용에 부합합니다. 호출부 단순화와 사용성 측면에서 적절한 선택입니다.src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (3)
40-42: 입력 검증 로직 적절빈 목록/사이즈 불일치 방지 검증으로 잘 막아두셨습니다.
57-60: DataFile.sha256 유니크 제약 확인 필요현재 엔티티 정의와 마이그레이션 파일 내에서
sha256컬럼에 유니크 제약이 적용된 부분을 찾을 수 없습니다. 아래 위치들을 수동으로 점검해 주시고, 실제 DB에도 제약이 설정되어 있는지 확인 부탁드립니다.
src/main/kotlin/.../entity/DataFile.kt
@Column(unique = true)또는@Table(uniqueConstraints = [...])어노테이션resources/db/migration/*.sql(또는 Flyway/Liquibase 스크립트)
CREATE UNIQUE INDEX또는UNIQUE(...)구문- 운영 DB 스키마 직접 조회
확인 후 결과를 알려주세요.
108-116: 트랜잭션 범위 확인 필요
DataFileTag엔티티에 이미(data_file_id, tag_id)복합 유니크 제약이 설정되어 있어, 제안된 try–catch 방식으로 중복 삽입을 무시해도 데이터 무결성은 보장됩니다.- 다만
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt파일의attachTagsIfMissing메서드가 @transactional 범위 내에서 실행되어야, 개별DataIntegrityViolationException만 무시하고 나머지 처리가 정상 커밋됩니다.프로젝트 설정에서 해당 메서드 또는 클래스에 @transactional이 적용되어 있는지 확인해 주세요.
📌 PR 개요
🔍 관련 이슈
✨ 변경 사항
📸 스크린샷 / 동작 확인 결과 (선택)
필요하다면 이미지나 동영상 첨부.
✅ 체크리스트
🗒️ 추가 메모
기타 참고 사항을 적어주세요.
Summary by CodeRabbit
New Features
Improvements
Chores
Tests