✨ Feature: dataFile 삭제 기능 추가 #8
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
2 tasks
WalkthroughDELETE 엔드포인트(/api/v1/data-files/{dataFilesId})가 추가되어 서비스 계층에서 트랜잭션으로 파일과 연관 태그를 순차적으로 하드 삭제합니다. 리포지토리에 ID 조회(findDataFileById)와 태그 일괄 삭제(deleteAllByDataFile) 메서드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as DataFileController
participant Service as DataFileService
participant TagRepo as DataFileTagRepository
participant FileRepo as DataFileRepository
Client->>Controller: DELETE /api/v1/data-files/{id}
Controller->>Service: deleteFile(id)
Service->>FileRepo: findDataFileById(id)
alt 존재하지 않음
Service-->>Controller: throw FileException(NOT_FOUND)
Controller-->>Client: 404 (ApiResponse)
else 존재함
Service->>TagRepo: deleteAllByDataFile(dataFile)
Service->>FileRepo: delete(dataFile)
Service-->>Controller: 완료
Controller-->>Client: 200 ApiResponse(null, "데이터 삭제 완료")
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(해당 없음) Poem
✨ 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: 10
📜 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 (7)
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt(2 hunks)src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.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/service/DataFileService.kt(3 hunks)src/main/resources/application.yml(1 hunks)src/test/kotlin/simplerag/ragback/domain/document/service/DataFileServiceTest.kt(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileTagRepository.kt (2)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (1)
name(7-32)
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (3)
src/main/kotlin/simplerag/ragback/domain/index/entity/DataFileIndex.kt (1)
name(7-22)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (1)
name(7-32)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt (2)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (1)
name(7-32)src/main/kotlin/simplerag/ragback/domain/index/entity/DataFileIndex.kt (1)
name(7-22)
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (1)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt (3)
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileRequestDTO.kt (2)
message(13-20)min(7-11)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (1)
name(7-32)
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 (6)
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt (1)
3-4: Slice/Pageable 사용은 적절합니다.카운트 쿼리를 피하는 Slice 반환형 선택 좋습니다. 키셋 전환 시에도 Slice를 유지하면 hasNext 계산이 용이합니다.
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (2)
13-13: 필요 DTO import 추가 확인신규 GET 응답 타입(
DataFileDetailResponseList) 사용을 위한 import 추가 적절합니다.
54-60: 경로 변수 바인딩 및 연관 엔티티 삭제 로직 검증검증 결과,
DataFileService.deleteFile내부에서 태그 연관 삭제(dataFileTagRepository.deleteAllByDataFile(dataFile)— DataFileService.kt:108)는 구현되어 있습니다.
다만 인덱스 연관 삭제(수동 로직 또는 JPA cascade) 설정 여부를 반드시 확인해주세요.• Controller:
@PathVariable("dataFilesId") dataFileId로 명시적 바인딩 및 변수명 일관성 유지
• Service: 태그 연관 삭제 로직 확인(DataFileService.kt:108)
• 추가 검증: DataFileIndex 엔티티에 대한 연관 삭제(또는 cascade) 구현 여부 확인- @DeleteMapping("/{dataFilesId}") - fun deleteFile( - @PathVariable dataFilesId: Long, - ): ApiResponse<Unit> { - dataFileService.deleteFile(dataFilesId) - return ApiResponse.ok(null, "데이터 삭제 완료") - } + @DeleteMapping("/{dataFilesId}") + fun deleteFile( + @PathVariable("dataFilesId") dataFileId: Long, + ): ApiResponse<Unit> { + dataFileService.deleteFile(dataFileId) + return ApiResponse.ok(null, "데이터 삭제 완료") + }REST 관점에서는 204 No Content가 일반적이지만, 프로젝트의
ApiResponse컨벤션을 유지한다면 200 OK도 수용 가능합니다.src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt (3)
3-5: 필요 import 추가 확인
DataFile,LocalDateTime사용을 위한 import 추가 적절합니다.
13-13: 사소: 후행 쉼표 유지 OKKotlin 포매터와 diff 최소화를 위해 후행 쉼표 유지하는 접근 문제 없습니다.
45-48: TagDTO 설계 적절필수 정보만 노출하며 간결합니다. 추후 태그 색상/설명 등 확장 필요 시 별도 필드 추가로 대응 가능해 보입니다.
Comment on lines
45
to
52
| @GetMapping | ||
| fun getDataFiles( | ||
| @RequestParam(name = "cursor") cursor: Long, | ||
| @RequestParam(name = "take") take: Int, | ||
| ): ApiResponse<DataFileDetailResponseList> { | ||
| val data = dataFileService.getDataFiles(cursor, take) | ||
| return ApiResponse.ok(data) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
페이지네이션 파라미터에 기본값/검증 추가 제안
- 초기 페이지 호출 시
cursor가 필수가 되면 400이 발생합니다.cursor는 선택적으로 받고 기본값(예: 0)을 둬주세요. take는 과도한 값 방지를 위해 상한선을 두는 것이 안전합니다(예: 1~100).
다음과 같이 수정하면 초기 페이지 호출과 파라미터 검증이 개선됩니다:
- fun getDataFiles(
- @RequestParam(name = "cursor") cursor: Long,
- @RequestParam(name = "take") take: Int,
- ): ApiResponse<DataFileDetailResponseList> {
- val data = dataFileService.getDataFiles(cursor, take)
- return ApiResponse.ok(data)
- }
+ fun getDataFiles(
+ @RequestParam(name = "cursor", required = false, defaultValue = "0") cursor: Long?,
+ @RequestParam(name = "take", defaultValue = "20") @Min(1) @Max(100) take: Int,
+ ): ApiResponse<DataFileDetailResponseList> {
+ val data = dataFileService.getDataFiles(cursor ?: 0L, take)
+ return ApiResponse.ok(data)
+ }위 변경에 필요한 import(파일 상단에 추가):
import jakarta.validation.constraints.Min
import jakarta.validation.constraints.Max다음 스크립트로 기본값 적용과 검증 추가 후, 컨트롤러 파라미터 사용 위치와 서비스 시그니처를 간단히 점검할 수 있습니다.
🏁 Script executed:
#!/bin/bash
# getDataFiles 시그니처와 호출부 점검
rg -n -C2 --type=kotlin 'fun\s+getDataFiles\(' src/main | sed -n '1,200p'
rg -n -C2 --type=kotlin 'dataFileService\.getDataFiles\(' src/mainLength of output: 1957
🏁 Script executed:
#!/bin/bash
# Check if @Validated is present and display class annotations
rg -n "@Validated" src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt || echo "No @Validated annotation found"
rg -n -C3 "class\s+DataFileController" src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.ktLength of output: 405
페이지네이션 기본값 및 요청 파라미터 검증 추가
컨트롤러에 이미 @Validated가 선언되어 있어, 파라미터 수준의 Bean Validation이 적용됩니다. 아래와 같이 수정해 초기 호출 시 cursor 기본값을 제공하고, take 값에 상한/하한 검증을 추가해주세요.
cursor를 선택(Optional) 파라미터로 받고 기본값"0"을 지정한 뒤,null병합 처리take에@Min(1)·@Max(100)검증과 기본값"20"설정
- fun getDataFiles(
- @RequestParam(name = "cursor") cursor: Long,
- @RequestParam(name = "take") take: Int,
- ): ApiResponse<DataFileDetailResponseList> {
- val data = dataFileService.getDataFiles(cursor, take)
- return ApiResponse.ok(data)
- }
+ fun getDataFiles(
+ @RequestParam(name = "cursor", required = false, defaultValue = "0") cursor: Long?,
+ @RequestParam(name = "take", defaultValue = "20") @Min(1) @Max(100) take: Int,
+ ): ApiResponse<DataFileDetailResponseList> {
+ val data = dataFileService.getDataFiles(cursor ?: 0L, take)
+ return ApiResponse.ok(data)
+ }필요한 import (파일 상단에 추가):
import jakarta.validation.constraints.Min
import jakarta.validation.constraints.Max🤖 Prompt for AI Agents
In
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt
around lines 45 to 52, make pagination params safe: change cursor to an optional
request param with defaultValue = "0" and merge null to 0 before calling
service, and add validation and a default for take by annotating it with @Min(1)
and @Max(100) and setting defaultValue = "20"; also add the imports
jakarta.validation.constraints.Min and jakarta.validation.constraints.Max at the
file top.
Comment on lines
15
to
20
| data class DataFileDetailResponseList( | ||
| val dataFilePreviewResponseList: List<DataFileDetailResponse>, | ||
| val cursor: Long?, | ||
| val hasNext: Boolean, | ||
| ) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
필드명 혼동: Detail 리스트인데 Preview 라벨 사용됨
DataFileDetailResponseList 내 컬렉션 필드명이 dataFilePreviewResponseList로 되어 있어 API 소비자에게 혼란을 줄 수 있습니다. Detail 응답에는 중립적인 이름(items) 또는 dataFileDetailResponseList가 더 명확합니다.
권장 수정:
data class DataFileDetailResponseList(
- val dataFilePreviewResponseList: List<DataFileDetailResponse>,
+ val items: List<DataFileDetailResponse>,
val cursor: Long?,
val hasNext: Boolean,
)이 변경은 직렬화 JSON 필드명이 바뀌므로(계약 변경), 서비스/테스트/클라이언트 사용처 점검이 필요합니다.
다음 스크립트로 Kotlin 코드에서 명명 인자를 사용하는 생성 호출을 우선적으로 찾아 일괄 수정 범위를 파악하세요.
🏁 Script executed:
#!/bin/bash
# Detail 리스트에 대한 명명 인자 사용 여부 확인
rg -n -C2 --type=kotlin '\bDataFileDetailResponseList\s*\('
rg -n -C2 --type=kotlin '\bdataFilePreviewResponseList\s*='Length of output: 1166
DTO 필드명 변경: dataFilePreviewResponseList → items
DataFileDetailResponseList 생성 호출에서 명명 인자를 사용하지 않으므로 position 기반 호출에 영향이 없습니다. 다만 JSON 직렬화 시 필드명이 변경되므로 서비스, 테스트, 클라이언트 사용처를 반드시 점검해야 합니다.
수정 대상:
src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt(DTO 정의)src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt(생성자 호출 – positional argument로 영향 없음)- 그 외 API 문서·테스트·클라이언트 코드 전반
제안된 diff:
data class DataFileDetailResponseList(
- val dataFilePreviewResponseList: List<DataFileDetailResponse>,
+ val items: List<DataFileDetailResponse>,
val cursor: Long?,
val hasNext: Boolean,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| data class DataFileDetailResponseList( | |
| val dataFilePreviewResponseList: List<DataFileDetailResponse>, | |
| val cursor: Long?, | |
| val hasNext: Boolean, | |
| ) | |
| data class DataFileDetailResponseList( | |
| val items: List<DataFileDetailResponse>, | |
| val cursor: Long?, | |
| val hasNext: Boolean, | |
| ) |
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt
around lines 15–20, rename the DTO field dataFilePreviewResponseList to items
and update all usages; to avoid breaking existing JSON clients either (A) add a
JSON mapping annotation so the serialized name remains
"dataFilePreviewResponseList" (e.g., annotate the new property with
@get:JsonProperty("dataFilePreviewResponseList") / @JsonProperty on getter) or
(B) update all consumers, tests, API docs and client code to use the new "items"
field name; also review
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt for
constructor calls (positional calls not broken) and update imports/usages and
tests accordingly so compilation and serialization behavior remain correct.
Comment on lines
21
to
43
| data class DataFileDetailResponse( | ||
| var id: Long?, | ||
| val title: String, | ||
| val type: String, | ||
| val lastModified: LocalDateTime, | ||
| val sizeMB: Double, | ||
| val tags: List<TagDTO>, | ||
| val sha256: String, | ||
| ) { | ||
| companion object { | ||
| fun of(dataFile: DataFile, tags: List<TagDTO>): DataFileDetailResponse { | ||
| return DataFileDetailResponse( | ||
| dataFile.id, | ||
| dataFile.title, | ||
| dataFile.type, | ||
| dataFile.updatedAt, | ||
| dataFile.sizeBytes / (1024.0 * 1024.0), | ||
| tags, | ||
| dataFile.sha256, | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
DTO 불변성 강화 및 sizeMB 표현 개선 제안
- id를
var로 둘 이유가 없어 보입니다. DTO는 불변(val)로 유지하는 것이 안전합니다. sizeMB는 소수점이 과도하게 길어질 수 있으니 표시 목적에 맞춰 반올림(예: 소수점 둘째 자리) 적용을 고려해 보세요.
다음과 같이 개선할 수 있습니다:
data class DataFileDetailResponse(
- var id: Long?,
+ val id: Long?,
val title: String,
val type: String,
val lastModified: LocalDateTime,
val sizeMB: Double,
val tags: List<TagDTO>,
val sha256: String,
) {
companion object {
fun of(dataFile: DataFile, tags: List<TagDTO>): DataFileDetailResponse {
return DataFileDetailResponse(
dataFile.id,
dataFile.title,
dataFile.type,
dataFile.updatedAt,
- dataFile.sizeBytes / (1024.0 * 1024.0),
+ ((dataFile.sizeBytes / (1024.0 * 1024.0)).let { kotlin.math.round(it * 100) / 100.0 }),
tags,
dataFile.sha256,
)
}
}
}반올림 사용 시 다음 import가 필요합니다(파일 상단):
import kotlin.math.roundid를 non-null로 강제(Long)하려면 생성 시점에 항상 영속 상태임이 보장되어야 하므로, 그 전제 확인 후 적용하세요.
🤖 Prompt for AI Agents
In src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt
around lines 21 to 43, make the DTO fields immutable by changing id from var to
val (or to non-null Long only if you can guarantee persistent id at creation),
round sizeMB to two decimal places for display (e.g. compute sizeMB =
round(dataFile.sizeBytes / (1024.0 * 1024.0) * 100) / 100), and add the required
import kotlin.math.round at the top of the file; keep id nullable Long? only if
you cannot guarantee persistence at construction time.
Comment on lines
11
to
13
| fun findByOrderById(pageable: Pageable): Slice<DataFile> | ||
|
|
||
| fun findDataFileById(dataFileId: Long): DataFile? |
There was a problem hiding this comment.
🛠️ Refactor suggestion
"cursor" 의미와 구현이 어긋납니다: 페이지 번호가 아닌 키셋(cursor) 페이지네이션으로 전환 필요
현재 서비스는 응답에 마지막 아이템의 id를 cursor로 반환하지만, 조회는 PageRequest.of(cursor.toInt(), take)로 "cursor=페이지 번호"로 해석합니다. 두 번째 페이지부터 오동작합니다.
키셋 페이지네이션으로 전환 제안:
- 리포지토리: 마지막 id를 기준으로 이후 레코드를 가져오도록 메서드 교체
- 서비스: Page index는 항상 0, 조건은 "id > lastId"
아래처럼 수정 제안합니다.
- fun findByOrderById(pageable: Pageable): Slice<DataFile>
-
- fun findDataFileById(dataFileId: Long): DataFile?
+ fun findByIdGreaterThanOrderById(lastIdExclusive: Long, pageable: Pageable): Slice<DataFile>
+ // NOTE: 단건 조회는 기본 제공되는 findById를 사용하는 편이 일관성/가독성에 좋습니다.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun findByOrderById(pageable: Pageable): Slice<DataFile> | |
| fun findDataFileById(dataFileId: Long): DataFile? | |
| fun findByIdGreaterThanOrderById(lastIdExclusive: Long, pageable: Pageable): Slice<DataFile> | |
| // NOTE: 단건 조회는 기본 제공되는 findById를 사용하는 편이 일관성/가독성에 좋습니다. |
🤖 Prompt for AI Agents
In
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt
around lines 11-13, the repository currently assumes pageable page-index
semantics while the API returns a cursor (last item id); change to keyset
pagination by replacing findByOrderById(pageable: Pageable) with a method that
accepts a lastId and pageable/limit and returns records where id > lastId
ordered by id (ascending) so the DB selects records after the cursor; then
update callers/service to always use PageRequest.of(0, take) (or equivalent
limit-only pageable) and pass the lastId as the cursor condition (id > lastId)
so pagination uses the last item id instead of page number.
Comment on lines
13
to
21
| @Query(""" | ||
| SELECT DISTINCT t | ||
| FROM DataFileTag dft | ||
| JOIN dft.tag t | ||
| WHERE dft.dataFile = :dataFile | ||
| """) | ||
| fun findTagsByDataFile(@Param("dataFile") dataFile: DataFile): List<Tag> | ||
|
|
||
| fun deleteAllByDataFile(dataFile: DataFile) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
N+1 쿼리 유발: 다건 DataFile에 대한 태그를 한 번에 가져오는 메서드를 추가하세요.
현재 서비스는 파일마다 findTagsByDataFile를 호출하여 N+1이 발생합니다. 다음과 같은 일괄 조회용 쿼리(Projections 또는 엔티티 반환)를 도입해, 한 번의 쿼리로 모든 태그를 가져온 뒤 메모리에서 그룹핑하는 것을 권장합니다.
예시(프로젝션 사용):
interface DataFileTagProjection {
fun getDataFileId(): Long
fun getTagId(): Long
fun getTagName(): String
}
@Query("""
SELECT dft.dataFile.id as dataFileId, t.id as tagId, t.name as tagName
FROM DataFileTag dft
JOIN dft.tag t
WHERE dft.dataFile in :dataFiles
""")
fun findTagsByDataFiles(@Param("dataFiles") dataFiles: Collection<DataFile>): List<DataFileTagProjection>서비스에서는 groupBy { it.dataFileId }로 매핑하면 N+1을 제거할 수 있습니다.
🤖 Prompt for AI Agents
In
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileTagRepository.kt
around lines 13 to 21, the current findTagsByDataFile method causes N+1 queries
when called per DataFile; add a batch query method that accepts a collection of
DataFile and returns a lightweight projection (containing dataFileId, tagId,
tagName) or entities so the service can fetch all tags in one query and then
group them in memory (e.g., groupBy dataFileId) to map tags to each DataFile;
implement the projection interface, add a @Query method that selects
dft.dataFile.id, t.id, t.name for dft.dataFile IN :dataFiles, and use this new
repository method from the service to replace per-file calls.
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
Show resolved
Hide resolved
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.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
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
📜 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 (4)
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.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/service/DataFileService.kt(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (1)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)
src/main/kotlin/simplerag/ragback/domain/document/controller/DataFileController.kt (3)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (1)
name(7-32)src/main/kotlin/simplerag/ragback/domain/index/entity/DataFileIndex.kt (1)
name(7-22)src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileTagRepository.kt (1)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFileTag.kt (1)
name(6-24)
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt (2)
src/main/kotlin/simplerag/ragback/domain/document/entity/DataFile.kt (1)
name(7-32)src/main/kotlin/simplerag/ragback/domain/document/dto/DataFileResponseDTO.kt (1)
id(7-10)
Comment on lines
+56
to
+62
| @DeleteMapping("/{dataFilesId}") | ||
| fun deleteFile( | ||
| @PathVariable dataFilesId: Long, | ||
| ): ApiResponse<Unit> { | ||
| dataFileService.deleteFile(dataFilesId) | ||
| return ApiResponse.ok(null, "데이터 삭제 완료") | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
DELETE 응답은 204 No Content가 더 적합합니다(본문 없이 성공만 알림)
현재 200 OK + 본문 메시지를 반환합니다. REST 관점에서는 204 No Content가 일반적입니다. 전역 응답 규약(ApiResponse 래퍼 강제)과의 정합성을 확인한 뒤 적용을 고려해 주세요.
- @DeleteMapping("/{dataFilesId}")
- fun deleteFile(
- @PathVariable dataFilesId: Long,
- ): ApiResponse<Unit> {
- dataFileService.deleteFile(dataFilesId)
- return ApiResponse.ok(null, "데이터 삭제 완료")
- }
+ @DeleteMapping("/{dataFilesId}")
+ @ResponseStatus(HttpStatus.NO_CONTENT)
+ fun deleteFile(
+ @PathVariable dataFilesId: Long,
+ ) {
+ dataFileService.deleteFile(dataFilesId)
+ }검토 포인트:
- 기존 API가 일관적으로 ApiResponse 래퍼를 강제한다면, 200 OK 유지도 허용될 수 있습니다. 팀 규약 확인 바랍니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @DeleteMapping("/{dataFilesId}") | |
| fun deleteFile( | |
| @PathVariable dataFilesId: Long, | |
| ): ApiResponse<Unit> { | |
| dataFileService.deleteFile(dataFilesId) | |
| return ApiResponse.ok(null, "데이터 삭제 완료") | |
| } | |
| @DeleteMapping("/{dataFilesId}") | |
| @ResponseStatus(HttpStatus.NO_CONTENT) | |
| fun deleteFile( | |
| @PathVariable dataFilesId: Long, | |
| ) { | |
| dataFileService.deleteFile(dataFilesId) | |
| } |
|
|
||
| fun findByIdGreaterThanOrderById(cursorId: Long, pageable: Pageable): Slice<DataFile> | ||
|
|
||
| fun findDataFileById(dataFileId: Long): DataFile? |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
불필요한 커스텀 조회 메서드: JpaRepository#findById 사용으로 단순화하세요
Line 13: findDataFileById는 JpaRepository#findById와 역할이 중복됩니다. 표준 메서드를 사용하면 일관성과 유지보수성이 좋아집니다.
[suggest_optional_refactor]
다음 변경을 제안합니다.
- fun findDataFileById(dataFileId: Long): DataFile?Service 사용부 교체(참고용):
val dataFile = dataFileRepository.findById(dataFilesId).orElseThrow {
FileException(ErrorCode.NOT_FOUND, dataFilesId.toString())
}사용처 점검 스크립트:
🏁 Script executed:
#!/bin/bash
rg -n --type=kt '\bfindDataFileById\('Length of output: 67
rg가 kt 파일 타입을 인식하지 못해 검색이 실행되지 않았습니다. 호출 위치를 정확히 찾기 위해 아래 스크립트로 다시 한번 확인해주세요.
🏁 Script executed:
#!/bin/bash
# Kotlin 파일(.kt) 전체에서 findDataFileById 호출 위치 검색
rg -n '\bfindDataFileById\(' -g '*.kt'Length of output: 357
불필요한 커스텀 메서드 제거 및 표준 findById 활용 제안
DataFileRepository의 findDataFileById는 JpaRepository#findById와 중복되므로 제거하고, 서비스 호출부에서 표준 메서드를 사용해 일관성을 높이길 권장합니다.
-
수정 대상:
- src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt (라인 13)
- src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt (라인 104)
-
제안 변경:
--- a/src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt
+++ b/src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt
@@ -13
- fun findDataFileById(dataFileId: Long): DataFile?--- a/src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
+++ b/src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
@@ -104
- val dataFile = dataFileRepository.findDataFileById(dataFilesId)
- ?: throw FileException(ErrorCode.NOT_FOUND, dataFilesId.toString())
+ val dataFile = dataFileRepository.findById(dataFilesId)
+ .orElseThrow { FileException(ErrorCode.NOT_FOUND, dataFilesId.toString()) }전체 사용처가 findDataFileById 호출로 작성된 부분이 더 있는지 확인 후 안전하게 제거하세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun findDataFileById(dataFileId: Long): DataFile? |
Comment on lines
+20
to
21
| fun deleteAllByDataFile(dataFile: DataFile) | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
삭제 행 수 반환으로 관측성 강화
Line 20: 파생 삭제 쿼리는 삭제 건수를 반환할 수 있습니다. 모니터링/검증에 유용하니 반환형을 Long으로 변경을 권장합니다. @Modifying은 파생 delete 메서드에선 불필요합니다.
- fun deleteAllByDataFile(dataFile: DataFile)
+ fun deleteAllByDataFile(dataFile: DataFile): Long📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun deleteAllByDataFile(dataFile: DataFile) | |
| } | |
| fun deleteAllByDataFile(dataFile: DataFile): Long | |
| } |
🤖 Prompt for AI Agents
In
src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileTagRepository.kt
around lines 20-21, the derived delete method should return the number of
deleted rows for observability; change the signature from fun
deleteAllByDataFile(dataFile: DataFile) to return Long (e.g., fun
deleteAllByDataFile(dataFile: DataFile): Long) and remove any unnecessary
@Modifying annotation from this derived delete method so that the repository
returns the deleted count for monitoring/verification.
Comment on lines
+102
to
+112
| @Transactional | ||
| fun deleteFile(dataFilesId: Long) { | ||
| val dataFile = dataFileRepository.findDataFileById(dataFilesId) ?: throw FileException( | ||
| ErrorCode.NOT_FOUND, | ||
| dataFilesId.toString() | ||
| ) | ||
|
|
||
| dataFileTagRepository.deleteAllByDataFile(dataFile) | ||
|
|
||
| dataFileRepository.delete(dataFile) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
하드 삭제 누락: S3 원본 및 연관 인덱스(DataFileIndex) 미삭제로 인한 FK 위반/유기 객체 위험
- Line 104: 커스텀
findDataFileById사용 → 표준findById(...).orElseThrow { ... }로 단순화 권장. - Lines 109-111: 태그만 삭제 후 본문 삭제. DataFileIndex(다대다 조인 테이블) 정리는 누락되어 FK 제약 위반 가능.
- S3 원본 파일 삭제가 수행되지 않아 “hard delete” 요구사항에 미달. 트랜잭션 커밋 후(afterCommit) 삭제로 정합성 보장 필요.
아래와 같이 수정해 주세요.
[offer_assistance]
@Transactional
fun deleteFile(dataFilesId: Long) {
- val dataFile = dataFileRepository.findDataFileById(dataFilesId) ?: throw FileException(
- ErrorCode.NOT_FOUND,
- dataFilesId.toString()
- )
-
- dataFileTagRepository.deleteAllByDataFile(dataFile)
-
- dataFileRepository.delete(dataFile)
+ val dataFile = dataFileRepository.findById(dataFilesId).orElseThrow {
+ FileException(ErrorCode.NOT_FOUND, dataFilesId.toString())
+ }
+ val fileUrl = dataFile.fileUrl
+
+ // 1) 태그 삭제
+ dataFileTagRepository.deleteAllByDataFile(dataFile)
+ // 2) 인덱스 삭제 (FK 제약 방지)
+ dataFileIndexRepository.deleteAllByDataFile(dataFile)
+ // 3) 파일 레코드 삭제
+ dataFileRepository.delete(dataFile)
+ // 4) 트랜잭션 커밋 후 S3 삭제 (DB 롤백 시 외부 상태 불일치 방지)
+ TransactionSynchronizationManager.registerSynchronization(object : TransactionSynchronization {
+ override fun afterCommit() {
+ runCatching { s3Util.deleteByUrl(fileUrl) }
+ }
+ })
}서비스 주입부(클래스 생성자)에 리포지토리 의존성 추가 필요:
class DataFileService(
private val dataFileRepository: DataFileRepository,
private val tagRepository: TagRepository,
private val dataFileTagRepository: DataFileTagRepository,
private val s3Util: S3Util,
private val dataFileIndexRepository: DataFileIndexRepository, // 추가
)DataFileIndexRepository가 없다면 추가:
package simplerag.ragback.domain.index.repository
import org.springframework.data.jpa.repository.JpaRepository
import simplerag.ragback.domain.document.entity.DataFile
import simplerag.ragback.domain.index.entity.DataFileIndex
interface DataFileIndexRepository : JpaRepository<DataFileIndex, Long> {
fun deleteAllByDataFile(dataFile: DataFile): Long
}검증 스크립트(연관 삭제/캐스케이드 유무 확인):
원하시면 @TransactionalEventListener 기반의 이벤트-드리븐 삭제로도 리팩터링해 드리겠습니다.
🏁 Script executed:
#!/bin/bash
echo "== DataFileIndexRepository / deleteAllByDataFile 존재 확인 =="
rg -n -C2 -P 'interface\\s+DataFileIndexRepository|delete(All)?ByDataFile' --type=kt
echo "== 관계 캐스케이드/OnDelete 애너테이션 존재 확인 =="
rg -n -C2 -P '@OnDelete|cascade\\s*=' --type=kt
echo "== 서비스에서 dataFileIndexRepository 사용 여부 =="
rg -n -C2 -P '\\bdataFileIndexRepository\\b' --type=ktLength of output: 573
아직 DataFileIndex 관련 정보가 파악되지 않아 추가 확인이 필요합니다. 아래 스크립트로 .kt 파일 전반을 검색해 보겠습니다.
🏁 Script executed:
#!/bin/bash
echo "== DataFileIndexRepository 및 deleteAllByDataFile 존재 여부 =="
rg -n -C2 -P 'interface\s+DataFileIndexRepository|delete(All)?ByDataFile' -g '*.kt'
echo "== DataFileIndex 엔티티 정의 검색 =="
rg -n -C2 -P 'class\s+DataFileIndex' -g '*.kt'
echo "== @OnDelete 또는 cascade 애너테이션 검색 =="
rg -n -C2 -P '@OnDelete|cascade' -g '*.kt'
echo "== 서비스에서 dataFileIndexRepository 사용 여부 =="
rg -n -C2 '\bdataFileIndexRepository\b' -g '*.kt'Length of output: 2087
아래 추가 스크립트로 관련 리포지토리 메소드, 서비스 생성자, 엔티티 매핑을 한 번 더 확인해 주세요.
🏁 Script executed:
#!/bin/bash
echo "== DataFileRepository 정의 및 findDataFileById 확인 =="
sed -n '1,50p' src/main/kotlin/simplerag/ragback/domain/document/repository/DataFileRepository.kt
echo "== DataFileService 생성자 및 필드 주입 확인 =="
sed -n '1,30p' src/main/kotlin/simplerag/ragback/domain/document/service/DataFileService.kt
echo "== DataFileIndex 엔티티 매핑 확인 =="
sed -n '1,50p' src/main/kotlin/simplerag/ragback/domain/index/entity/DataFileIndex.ktLength of output: 3137
DataFileIndex 삭제 & S3 하드 삭제 미구현
현재 deleteFile 메서드는 태그만 삭제하고, 인덱스 테이블(data_files_indexes) 레코드와 S3 원본 파일을 제거하지 않아 아래 리스크가 있습니다.
- FK 제약 위반 또는 인덱스 데이터 유기
- S3 버킷에 남은 “휴지통” 없는 잔존 객체
■ 수정 제안
- 표준
findById(...).orElseThrow{}사용으로 예외 처리 간결화 DataFileIndexRepository를 주입 및deleteAllByDataFile호출로 인덱스 레코드 삭제- 트랜잭션 커밋 후(
afterCommit)s3Util.deleteByUrl(fileUrl)로 S3 하드 삭제
■ 주요 변경점
@Transactional
fun deleteFile(dataFileId: Long) {
- val dataFile = dataFileRepository.findDataFileById(dataFilesId) ?: throw FileException(
- ErrorCode.NOT_FOUND, dataFilesId.toString()
- )
+ val dataFile = dataFileRepository.findById(dataFileId)
+ .orElseThrow { FileException(ErrorCode.NOT_FOUND, dataFileId.toString()) }
+ val fileUrl = dataFile.fileUrl
// 1) 태그 삭제
dataFileTagRepository.deleteAllByDataFile(dataFile)
+ // 2) 인덱스 삭제
+ dataFileIndexRepository.deleteAllByDataFile(dataFile)
// 3) DB 레코드 삭제
dataFileRepository.delete(dataFile)
// 4) 트랜잭션 커밋 후 S3 삭제
TransactionSynchronizationManager.registerSynchronization(object : TransactionSynchronization {
override fun afterCommit() {
runCatching { s3Util.deleteByUrl(fileUrl) }
}
})
}■ 서비스 클래스 생성자
@Service
class DataFileService(
private val dataFileRepository: DataFileRepository,
private val dataFileTagRepository: DataFileTagRepository,
private val tagRepository: TagRepository,
private val s3Util: S3Util,
+ private val dataFileIndexRepository: DataFileIndexRepository
) { … }■ DataFileIndexRepository 추가
package simplerag.ragback.domain.index.repository
import org.springframework.data.jpa.repository.JpaRepository
import simplerag.ragback.domain.document.entity.DataFile
import simplerag.ragback.domain.index.entity.DataFileIndex
interface DataFileIndexRepository : JpaRepository<DataFileIndex, Long> {
fun deleteAllByDataFile(dataFile: DataFile)
}위 변경사항을 반영해 주시기 바랍니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 PR 개요
dataFile 삭제 기능 추가
🔍 관련 이슈
✨ 변경 사항
✨ feature: 데이터 삭제 기능 추
📸 스크린샷 / 동작 확인 결과 (선택)
필요하다면 이미지나 동영상 첨부.
✅ 체크리스트
🗒️ 추가 메모
기타 참고 사항을 적어주세요.
Summary by CodeRabbit