[WTH-139] file 도메인 코틀린 마이그레이션#6
Merged
Conversation
📝 WalkthroughWalkthrough서비스 기반 파일 관리에서 저장소·포트 기반 아키텍처로 전면 전환: Java 파일 엔티티·서비스를 제거하고 Kotlin 도메인 엔티티, 값 객체, 포트, 리포지토리, 어댑터, DTO/매퍼 및 관련 예외·테스트를 새로 추가했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FileController
participant GenerateFileUrlUsecase
participant FileUploadUrlPort
participant S3Presigner as S3
participant FileMapper
Client->>FileController: GET /api/v4/files?ownerType=...&fileNames=[...]
FileController->>GenerateFileUrlUsecase: generateFileUploadUrls(ownerType, fileNames)
loop for each fileName
GenerateFileUrlUsecase->>FileUploadUrlPort: generateUploadUrl(ownerType, fileName)
FileUploadUrlPort->>S3: presign PUT (bucket, key, expiry)
S3-->>FileUploadUrlPort: presigned put URL
FileUploadUrlPort-->>GenerateFileUrlUsecase: FileUploadUrl(fileName, storageKey, putUrl)
GenerateFileUrlUsecase->>FileMapper: toUrlResponse(fileName, putUrl, storageKey)
FileMapper-->>GenerateFileUrlUsecase: UrlResponse
end
GenerateFileUrlUsecase-->>FileController: List<UrlResponse>
FileController-->>Client: CommonResponse<List<UrlResponse>>
sequenceDiagram
participant UseCase as Domain UseCase
participant FileReader
participant FileRepository
participant FileMapper
UseCase->>FileReader: findAll(ownerType, ownerId, status?)
FileReader-->>UseCase: List<File>
UseCase->>FileRepository: deleteAll(oldFiles)
FileRepository-->>UseCase: void
UseCase->>FileMapper: toFileList(requests, ownerType, ownerId)
FileMapper-->>UseCase: List<File>
UseCase->>FileRepository: saveAll(newFiles)
FileRepository-->>UseCase: savedFiles
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/weeth/domain/comment/application/usecase/PostCommentUsecaseImpl.java (1)
89-91:⚠️ Potential issue | 🟡 Minor사용되지 않는 변수
user변수가 조회되었으나 이후 로직에서 사용되지 않습니다. 의도된 것이라면 제거하거나, 권한 검증에 활용해야 한다면validateOwner에서 이미 처리되므로 중복 조회입니다.🔧 미사용 변수 제거
`@Override` `@Transactional` public void deletePostComment(Long commentId, Long userId) throws UserNotMatchException { - User user = userGetService.find(userId); Comment comment = validateOwner(commentId, userId); Post post = comment.getPost();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/weeth/domain/comment/application/usecase/PostCommentUsecaseImpl.java` around lines 89 - 91, The local variable user in deletePostComment is retrieved via userGetService.find(userId) but never used; either remove that redundant lookup or use it for additional authorization—since validateOwner(commentId, userId) already checks ownership, delete the user retrieval line (the call to userGetService.find in PostCommentUsecaseImpl.deletePostComment) to avoid duplicate DB calls.
🧹 Nitpick comments (10)
src/main/resources/application.yml (1)
42-45: 환경별 유연성을 위해presigned-url-expiration-minutes환경 변수화 검토
cdn-base-url은 환경 변수로 설정 가능한 반면,presigned-url-expiration-minutes는 5분으로 하드코딩되어 있습니다. 환경(dev/staging/prod)에 따라 다른 만료 시간이 필요할 수 있으므로 환경 변수로 설정 가능하게 하는 것을 권장합니다.♻️ 환경 변수 적용 제안
app: file: cdn-base-url: ${CDN_BASE_URL:} - presigned-url-expiration-minutes: 5 + presigned-url-expiration-minutes: ${PRESIGNED_URL_EXPIRATION_MINUTES:5}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/application.yml` around lines 42 - 45, Replace the hardcoded presigned-url-expiration-minutes value with an environment-variable-backed default so it can vary per environment; specifically change the app.file.presigned-url-expiration-minutes entry (which sits alongside app.file.cdn-base-url) to read from an env var (e.g., PRESIGNED_URL_EXPIRATION_MINUTES) with a sensible default (5) and ensure any code that reads this YAML treats it as an integer if needed.src/main/kotlin/com/weeth/domain/file/domain/vo/StorageKey.kt (1)
18-21: UUID 패턴 검증을 더 엄격하게 고려해보세요.현재
[0-9a-fA-F-]{36}패턴은 36자의 hex 문자와 하이픈 조합을 허용하지만, 실제 UUID 형식(8-4-4-4-12)을 정확히 검증하지는 않습니다. 예를 들어------------------------------------도 매칭됩니다.♻️ 더 엄격한 UUID 패턴 제안
private val STORAGE_KEY_PATTERN = Regex( - pattern = "^($OWNER_TYPE_PATTERN)/(\\d{4}-(0[1-9]|1[0-2]))/([0-9a-fA-F-]{36})_.+$", + pattern = "^($OWNER_TYPE_PATTERN)/(\\d{4}-(0[1-9]|1[0-2]))/([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})_.+$", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/file/domain/vo/StorageKey.kt` around lines 18 - 21, The STORAGE_KEY_PATTERN currently uses a loose UUID segment "[0-9a-fA-F-]{36}" which accepts invalid strings like all-hyphens; update the regex in STORAGE_KEY_PATTERN to enforce the canonical UUID layout (8-4-4-4-12 hex groups) instead of the 36-char bulk pattern, e.g. replace the UUID part with a pattern matching "[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" so StorageKey validation (using STORAGE_KEY_PATTERN and OWNER_TYPE_PATTERN) only accepts properly formatted UUIDs.src/test/kotlin/com/weeth/domain/file/domain/vo/FileTypeTest.kt (1)
6-19: 엣지 케이스 테스트 추가를 고려해보세요.현재 테스트는 기본 조회만 검증합니다.
FileType구현에서trim().lowercase()처리를 하고 있으므로, 해당 동작과 지원하지 않는 타입에 대한 테스트 추가를 권장합니다.💚 추가 테스트 케이스 제안
it("대소문자를 무시하고 조회한다") { FileType.fromContentType("IMAGE/PNG") shouldBe FileType.PNG FileType.fromExtension("JPG") shouldBe FileType.JPEG } it("앞뒤 공백을 무시하고 조회한다") { FileType.fromContentType(" image/png ") shouldBe FileType.PNG FileType.fromExtension(" jpg ") shouldBe FileType.JPEG } it("지원하지 않는 타입은 null을 반환한다") { FileType.fromContentType("video/mp4") shouldBe null FileType.fromExtension("exe") shouldBe null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/file/domain/vo/FileTypeTest.kt` around lines 6 - 19, Tests in FileTypeTest only cover basic lookups; add edge-case specs verifying that FileType.fromContentType and FileType.fromExtension handle trimming and case-insensitivity and return null for unsupported types. Expand the DescribeSpec in FileTypeTest with examples that call FileType.fromContentType("IMAGE/PNG"), FileType.fromContentType(" image/png "), FileType.fromContentType("video/mp4") and similarly for FileType.fromExtension("JPG"), FileType.fromExtension(" jpg "), FileType.fromExtension("exe") asserting expected FileType values or null, to exercise the trim().lowercase() behavior and unsupported-type path.src/test/kotlin/com/weeth/domain/file/application/usecase/query/FileQueryServiceTest.kt (1)
18-20: 변수명 일관성 개선
preSignedService라는 이름이 실제 타입FileUploadUrlPort와 맞지 않습니다. 포트 기반 아키텍처에 맞게fileUploadUrlPort또는uploadUrlPort로 변경하면 가독성이 향상됩니다.♻️ 변수명 수정
- val preSignedService = mockk<FileUploadUrlPort>() + val uploadUrlPort = mockk<FileUploadUrlPort>() val fileMapper = mockk<FileMapper>() - val useCase = FileQueryService(preSignedService, fileMapper) + val useCase = FileQueryService(uploadUrlPort, fileMapper) beforeEach { - clearMocks(preSignedService, fileMapper) + clearMocks(uploadUrlPort, fileMapper) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/file/application/usecase/query/FileQueryServiceTest.kt` around lines 18 - 20, Rename the misleading variable preSignedService to match its type and intent (e.g., fileUploadUrlPort or uploadUrlPort) wherever it's declared and used in the test; specifically update the mock declaration of FileUploadUrlPort and the argument passed to FileQueryService (and any subsequent references to preSignedService), ensuring FileQueryService(preSignedService, fileMapper) becomes FileQueryService(fileUploadUrlPort, fileMapper) and all uses of preSignedService are renamed accordingly.src/main/kotlin/com/weeth/domain/file/infrastructure/S3FileUploadUrlAdapter.kt (1)
58-65: 테스트 용이성을 위한 Clock 주입 고려
LocalDate.now()를 직접 호출하면 단위 테스트에서 날짜 관련 동작을 검증하기 어렵습니다.java.time.Clock을 주입받아 사용하면 테스트 시 고정된 날짜로 검증할 수 있습니다.♻️ Clock 주입을 통한 테스트 용이성 개선
class S3FileUploadUrlAdapter( private val s3Presigner: S3Presigner, private val awsS3Properties: AwsS3Properties, `@param`:Value("\${app.file.presigned-url-expiration-minutes:5}") private val presignedUrlExpirationMinutes: Long, + private val clock: Clock = Clock.systemDefaultZone(), ) : FileUploadUrlPort {private fun generateStorageKey( ownerType: FileOwnerType, sanitizedFileName: String, ): String { - val month = LocalDate.now().format(DateTimeFormatter.ofPattern("yyyy-MM")) + val month = LocalDate.now(clock).format(DateTimeFormatter.ofPattern("yyyy-MM")) val uuid = UUID.randomUUID().toString() return "${ownerType.name}/$month/${uuid}_$sanitizedFileName" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/file/infrastructure/S3FileUploadUrlAdapter.kt` around lines 58 - 65, The generateStorageKey method uses LocalDate.now() which hinders testability; modify S3FileUploadUrlAdapter to accept a java.time.Clock (e.g., via constructor or DI) and replace LocalDate.now() with LocalDate.now(clock) inside generateStorageKey, defaulting to Clock.systemDefaultZone() if no clock is provided so production behavior is unchanged; update any callers/constructors to pass the clock (or use the default) and adjust unit tests to inject a fixed Clock to assert deterministic month values.src/test/kotlin/com/weeth/domain/file/infrastructure/S3FileUploadUrlAdapterTest.kt (1)
22-27: 공통 설정 추출 고려
AwsS3Properties설정이FileAccessUrlAdapterTest.kt에서도 동일하게 반복됩니다. 테스트 fixture로 추출하면 중복을 줄일 수 있습니다.♻️ 테스트 fixture 추출 예시
// src/test/kotlin/com/weeth/domain/file/infrastructure/fixture/AwsS3PropertiesTestFixture.kt object AwsS3PropertiesTestFixture { fun create( bucket: String = "weeth-bucket", region: String = "ap-northeast-2", ) = AwsS3Properties( s3 = AwsS3Properties.S3Properties(bucket = bucket), credentials = AwsS3Properties.CredentialsProperties(accessKey = "test", secretKey = "test"), region = AwsS3Properties.RegionProperties(static = region), ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/file/infrastructure/S3FileUploadUrlAdapterTest.kt` around lines 22 - 27, Extract the duplicated AwsS3Properties creation into a shared test fixture and use it in both S3FileUploadUrlAdapterTest and FileAccessUrlAdapterTest: create an object like AwsS3PropertiesTestFixture with a create(...) helper that returns AwsS3Properties (matching the existing S3Properties/CredentialsProperties/RegionProperties structure) and replace the inline instantiations in S3FileUploadUrlAdapterTest (where AwsS3Properties is currently constructed) and FileAccessUrlAdapterTest to call AwsS3PropertiesTestFixture.create(...) with optional parameters for bucket/region.src/test/kotlin/com/weeth/domain/file/domain/entity/FileTest.kt (1)
3-5: 동일 패키지 내 불필요한 import 문이 있습니다.
FileOwnerType과FileStatus는 테스트 대상 클래스(File)와 동일한 패키지(com.weeth.domain.file.domain.entity)에 있으므로 명시적인 import가 필요하지 않습니다.♻️ 불필요한 import 제거
package com.weeth.domain.file.domain.entity import com.weeth.domain.file.application.exception.UnsupportedFileContentTypeException -import com.weeth.domain.file.domain.entity.FileOwnerType -import com.weeth.domain.file.domain.entity.FileStatus import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.DescribeSpec import io.kotest.matchers.shouldBe🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/file/domain/entity/FileTest.kt` around lines 3 - 5, Remove the unnecessary explicit imports of FileOwnerType and FileStatus from the test file FileTest.kt since both classes live in the same package com.weeth.domain.file.domain.entity as the test subject File; open FileTest.kt and delete the import lines for FileOwnerType and FileStatus while keeping the import for UnsupportedFileContentTypeException if still used, leaving references to FileOwnerType and FileStatus unqualified in the test code.src/main/kotlin/com/weeth/domain/file/domain/entity/File.kt (1)
28-29:fileName이var로 선언되었으나 변경하는 메서드가 없습니다.
fileName이 변경 가능(var)으로 선언되어 있지만, 이를 수정하는 메서드가 없습니다. 의도적으로 변경 가능하게 설계한 것이 아니라면val로 변경하여 불변성을 보장하는 것이 좋습니다.♻️ 불변 필드로 변경
`@Column`(nullable = false) - var fileName: String, + val fileName: String,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/file/domain/entity/File.kt` around lines 28 - 29, The File entity's property fileName is declared mutable (var) but there is no method that modifies it; either make it immutable by changing var fileName to val fileName in the File class or, if mutability is required, add a clear domain method (e.g., File.rename(newName: String) or setFileName(newName: String)) that performs validation and updates fileName; update any constructors/usages accordingly to maintain invariants.src/test/kotlin/com/weeth/domain/file/application/mapper/FileMapperTest.kt (1)
29-45:toFileResponse와toUrlResponse메서드에 대한 테스트가 누락되었습니다.현재
toFileList만 테스트하고 있으며,FileMapper의 다른 두 메서드(toFileResponse,toUrlResponse)에 대한 테스트가 없습니다. 또한toFileList테스트에서storageKey,fileSize,contentType,status필드에 대한 검증도 추가하면 좋겠습니다.
toFileResponse와toUrlResponse에 대한 테스트 코드를 생성해 드릴까요?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/file/application/mapper/FileMapperTest.kt` around lines 29 - 45, Add unit tests for FileMapper.toFileResponse and FileMapper.toUrlResponse and extend the existing toFileList test to assert storageKey, fileSize, contentType, and status; specifically, in FileMapperTest create a test that converts a File domain object (with known id, storageKey, fileSize, contentType, status, ownerType/ownerId) via toFileResponse and asserts each response field matches the File properties, and another test that calls toUrlResponse with a File (or storageKey) and expected URL and asserts the returned URL response contains the correct url and storageKey; also update the existing toFileList spec to assert result[n].storageKey, result[n].fileSize, result[n].contentType, and result[n].status in addition to fileName/ownerType/ownerId so all mapper outputs are covered.src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt (1)
29-31:fileNames파라미터에 Swagger 문서화가 누락되었습니다.
ownerType에는@Parameter어노테이션이 있지만fileNames에는 없습니다. 또한require()로 던지는IllegalArgumentException은 글로벌 예외 핸들러에서 적절히 처리되는지 확인이 필요합니다.♻️ Swagger 문서화 추가
fun getUrl( `@Parameter`(description = "파일 소유 타입", example = "POST") `@RequestParam` ownerType: FileOwnerType, + `@Parameter`(description = "업로드할 파일명 목록", example = "[\"image1.png\", \"image2.jpg\"]") `@RequestParam` fileNames: List<String>, ): CommonResponse<List<UrlResponse>> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt` around lines 29 - 31, Add Swagger documentation to the fileNames request parameter by annotating the fileNames parameter in FileController (the same place ownerType is annotated) with `@Parameter` (description and example) so it appears in generated docs; also replace or complement the require(fileNames.isNotEmpty()) check with an explicit BadRequest exception (or ensure the global exception handler maps IllegalArgumentException to a 400) so empty input returns a proper 400 response—update either the validation to throw a custom/BadRequestException or confirm global handler handles IllegalArgumentException accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/weeth/domain/account/application/usecase/AccountUseCaseImpl.java`:
- Around line 63-65: The getFiles method in AccountUseCaseImpl calls
fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null) which passes null for
status and therefore returns deleted files; change that call to pass
FileStatus.UPLOADED instead (i.e., fileReader.findAll(FileOwnerType.RECEIPT,
receiptId, FileStatus.UPLOADED)) so only uploaded files are returned. Apply the
same fix for the analogous calls in ReceiptUseCaseImpl, PostUseCaseImpl,
NoticeUsecaseImpl and all Comment use case classes where fileReader.findAll(...,
null) is used, replacing the null status argument with FileStatus.UPLOADED and
keep mapping via fileMapper::toFileResponse.
In
`@src/main/java/com/weeth/domain/account/application/usecase/ReceiptUseCaseImpl.java`:
- Around line 70-72: getFiles(Long receiptId) currently calls
fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null) which overrides the
safe default and returns deleted files; change the call to omit the third
parameter (use the method overload that applies the default) or explicitly pass
FileStatus.UPLOADED to ensure only uploaded files are returned, and make the
same change in the analogous call(s) inside AccountUseCaseImpl (search for
fileReader.findAll(...) with a null status) so deleted files are not exposed.
In
`@src/main/java/com/weeth/domain/board/application/usecase/NoticeUsecaseImpl.java`:
- Around line 111-116: The code in NoticeUsecaseImpl unconditionally deletes and
re-saves files using getFiles(noticeId), fileRepository.deleteAll(...),
fileMapper.toFileList(...), and fileRepository.saveAll(...), but dto.files() can
be null; add a guard like in PostUseCaseImpl so you only delete and save when
dto.files() != null: check dto.files() before calling getFiles and deleteAll,
and only call fileMapper.toFileList and fileRepository.saveAll when dto.files()
is non-null, leaving existing files untouched otherwise.
In
`@src/main/kotlin/com/weeth/domain/file/application/dto/request/FileSaveRequest.kt`:
- Around line 11-13: The schema example for storageKey in FileSaveRequest
(field: storageKey) uses a malformed UUID; update the example value to use a
valid UUID format matching tests (e.g., the UUID from FileTest.kt like
550e8400-e29b-41d4-a716-446655440000) so the storageKey example follows
`Type/YY-MM/UUID_originalFilename` format; ensure only the example string is
changed and the `@field`:Schema text still documents the same format.
---
Outside diff comments:
In
`@src/main/java/com/weeth/domain/comment/application/usecase/PostCommentUsecaseImpl.java`:
- Around line 89-91: The local variable user in deletePostComment is retrieved
via userGetService.find(userId) but never used; either remove that redundant
lookup or use it for additional authorization—since validateOwner(commentId,
userId) already checks ownership, delete the user retrieval line (the call to
userGetService.find in PostCommentUsecaseImpl.deletePostComment) to avoid
duplicate DB calls.
---
Nitpick comments:
In `@src/main/kotlin/com/weeth/domain/file/domain/entity/File.kt`:
- Around line 28-29: The File entity's property fileName is declared mutable
(var) but there is no method that modifies it; either make it immutable by
changing var fileName to val fileName in the File class or, if mutability is
required, add a clear domain method (e.g., File.rename(newName: String) or
setFileName(newName: String)) that performs validation and updates fileName;
update any constructors/usages accordingly to maintain invariants.
In `@src/main/kotlin/com/weeth/domain/file/domain/vo/StorageKey.kt`:
- Around line 18-21: The STORAGE_KEY_PATTERN currently uses a loose UUID segment
"[0-9a-fA-F-]{36}" which accepts invalid strings like all-hyphens; update the
regex in STORAGE_KEY_PATTERN to enforce the canonical UUID layout (8-4-4-4-12
hex groups) instead of the 36-char bulk pattern, e.g. replace the UUID part with
a pattern matching
"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" so
StorageKey validation (using STORAGE_KEY_PATTERN and OWNER_TYPE_PATTERN) only
accepts properly formatted UUIDs.
In
`@src/main/kotlin/com/weeth/domain/file/infrastructure/S3FileUploadUrlAdapter.kt`:
- Around line 58-65: The generateStorageKey method uses LocalDate.now() which
hinders testability; modify S3FileUploadUrlAdapter to accept a java.time.Clock
(e.g., via constructor or DI) and replace LocalDate.now() with
LocalDate.now(clock) inside generateStorageKey, defaulting to
Clock.systemDefaultZone() if no clock is provided so production behavior is
unchanged; update any callers/constructors to pass the clock (or use the
default) and adjust unit tests to inject a fixed Clock to assert deterministic
month values.
In `@src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt`:
- Around line 29-31: Add Swagger documentation to the fileNames request
parameter by annotating the fileNames parameter in FileController (the same
place ownerType is annotated) with `@Parameter` (description and example) so it
appears in generated docs; also replace or complement the
require(fileNames.isNotEmpty()) check with an explicit BadRequest exception (or
ensure the global exception handler maps IllegalArgumentException to a 400) so
empty input returns a proper 400 response—update either the validation to throw
a custom/BadRequestException or confirm global handler handles
IllegalArgumentException accordingly.
In `@src/main/resources/application.yml`:
- Around line 42-45: Replace the hardcoded presigned-url-expiration-minutes
value with an environment-variable-backed default so it can vary per
environment; specifically change the app.file.presigned-url-expiration-minutes
entry (which sits alongside app.file.cdn-base-url) to read from an env var
(e.g., PRESIGNED_URL_EXPIRATION_MINUTES) with a sensible default (5) and ensure
any code that reads this YAML treats it as an integer if needed.
In `@src/test/kotlin/com/weeth/domain/file/application/mapper/FileMapperTest.kt`:
- Around line 29-45: Add unit tests for FileMapper.toFileResponse and
FileMapper.toUrlResponse and extend the existing toFileList test to assert
storageKey, fileSize, contentType, and status; specifically, in FileMapperTest
create a test that converts a File domain object (with known id, storageKey,
fileSize, contentType, status, ownerType/ownerId) via toFileResponse and asserts
each response field matches the File properties, and another test that calls
toUrlResponse with a File (or storageKey) and expected URL and asserts the
returned URL response contains the correct url and storageKey; also update the
existing toFileList spec to assert result[n].storageKey, result[n].fileSize,
result[n].contentType, and result[n].status in addition to
fileName/ownerType/ownerId so all mapper outputs are covered.
In
`@src/test/kotlin/com/weeth/domain/file/application/usecase/query/FileQueryServiceTest.kt`:
- Around line 18-20: Rename the misleading variable preSignedService to match
its type and intent (e.g., fileUploadUrlPort or uploadUrlPort) wherever it's
declared and used in the test; specifically update the mock declaration of
FileUploadUrlPort and the argument passed to FileQueryService (and any
subsequent references to preSignedService), ensuring
FileQueryService(preSignedService, fileMapper) becomes
FileQueryService(fileUploadUrlPort, fileMapper) and all uses of preSignedService
are renamed accordingly.
In `@src/test/kotlin/com/weeth/domain/file/domain/entity/FileTest.kt`:
- Around line 3-5: Remove the unnecessary explicit imports of FileOwnerType and
FileStatus from the test file FileTest.kt since both classes live in the same
package com.weeth.domain.file.domain.entity as the test subject File; open
FileTest.kt and delete the import lines for FileOwnerType and FileStatus while
keeping the import for UnsupportedFileContentTypeException if still used,
leaving references to FileOwnerType and FileStatus unqualified in the test code.
In `@src/test/kotlin/com/weeth/domain/file/domain/vo/FileTypeTest.kt`:
- Around line 6-19: Tests in FileTypeTest only cover basic lookups; add
edge-case specs verifying that FileType.fromContentType and
FileType.fromExtension handle trimming and case-insensitivity and return null
for unsupported types. Expand the DescribeSpec in FileTypeTest with examples
that call FileType.fromContentType("IMAGE/PNG"), FileType.fromContentType("
image/png "), FileType.fromContentType("video/mp4") and similarly for
FileType.fromExtension("JPG"), FileType.fromExtension(" jpg "),
FileType.fromExtension("exe") asserting expected FileType values or null, to
exercise the trim().lowercase() behavior and unsupported-type path.
In
`@src/test/kotlin/com/weeth/domain/file/infrastructure/S3FileUploadUrlAdapterTest.kt`:
- Around line 22-27: Extract the duplicated AwsS3Properties creation into a
shared test fixture and use it in both S3FileUploadUrlAdapterTest and
FileAccessUrlAdapterTest: create an object like AwsS3PropertiesTestFixture with
a create(...) helper that returns AwsS3Properties (matching the existing
S3Properties/CredentialsProperties/RegionProperties structure) and replace the
inline instantiations in S3FileUploadUrlAdapterTest (where AwsS3Properties is
currently constructed) and FileAccessUrlAdapterTest to call
AwsS3PropertiesTestFixture.create(...) with optional parameters for
bucket/region.
Comment on lines
63
to
65
| private List<FileResponse> getFiles(Long receiptId) { | ||
| return fileGetService.findAllByReceipt(receiptId).stream() | ||
| return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null).stream() | ||
| .map(fileMapper::toFileResponse) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find FileReader interface definition
find . -type f -name "*.java" -o -name "*.kt" | xargs grep -l "interface FileReader" | head -5Repository: Team-Weeth/weeth-server
Length of output: 138
🏁 Script executed:
# Search for FileReader interface and findAll method signature
rg -n "interface FileReader" -A 20 --type=java --type=kotlinRepository: Team-Weeth/weeth-server
Length of output: 1320
🏁 Script executed:
# Look for FileStatus enum
fd "FileStatus" --type f -e java -e ktRepository: Team-Weeth/weeth-server
Length of output: 132
🏁 Script executed:
# Find any other usages of fileReader.findAll to see what status values are used
rg "fileReader\.findAll\(" -B 2 -A 2 --type=java --type=kotlinRepository: Team-Weeth/weeth-server
Length of output: 7680
🏁 Script executed:
# Find FileReader implementation (likely in adapter layer)
fd "FileReader" --type f -e java -e kt | grep -i implRepository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
# View FileStatus enum
cat src/main/kotlin/com/weeth/domain/file/domain/entity/FileStatus.ktRepository: Team-Weeth/weeth-server
Length of output: 164
🏁 Script executed:
# Search for FileReader implementation class
rg "class.*FileReader" --type=java --type=kotlin -A 5Repository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
# Search for FileReader implementation in repository/adapter folders
find src/main -type f \( -name "*.java" -o -name "*.kt" \) | xargs grep -l "implements FileReader\|: FileReader" | head -10Repository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
# Look for file repository/adapter implementations
fd "File" src/main --type f -e kt -e java | grep -i "repository\|adapter" | head -20Repository: Team-Weeth/weeth-server
Length of output: 448
🏁 Script executed:
# Search for how the null status parameter is handled in queries
rg "FileStatus" -B 3 -A 3 --type=kotlin | grep -A 5 -B 5 "status"Repository: Team-Weeth/weeth-server
Length of output: 13167
🏁 Script executed:
# Check if there are more instances of findAll with null in the Java codebase
rg "\.findAll\(" src/main/java --type=java | grep "null"Repository: Team-Weeth/weeth-server
Length of output: 1558
null 대신 FileStatus.UPLOADED를 명시적으로 전달하세요 — 삭제된 파일이 노출됩니다.
FileRepository 구현을 보면, status가 null일 때 상태 필터 없이 모든 파일을 반환합니다. 따라서 현재 코드의 fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null)은 DELETED 상태의 파일까지 포함합니다. Kotlin의 기본값(FileStatus.UPLOADED)을 명시적으로 전달하여 이를 방지하세요.
이 패턴은 AccountUseCaseImpl뿐 아니라 ReceiptUseCaseImpl, PostUseCaseImpl, NoticeUsecaseImpl, 그리고 Comment UseCase들에서도 나타나므로 일괄 수정이 필요합니다.
🛠️ 제안 수정
import com.weeth.domain.file.domain.entity.FileOwnerType;
+import com.weeth.domain.file.domain.entity.FileStatus;
import com.weeth.domain.file.domain.repository.FileReader;
@@
- return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null).stream()
+ return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, FileStatus.UPLOADED).stream()
.map(fileMapper::toFileResponse)
.toList();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/weeth/domain/account/application/usecase/AccountUseCaseImpl.java`
around lines 63 - 65, The getFiles method in AccountUseCaseImpl calls
fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null) which passes null for
status and therefore returns deleted files; change that call to pass
FileStatus.UPLOADED instead (i.e., fileReader.findAll(FileOwnerType.RECEIPT,
receiptId, FileStatus.UPLOADED)) so only uploaded files are returned. Apply the
same fix for the analogous calls in ReceiptUseCaseImpl, PostUseCaseImpl,
NoticeUsecaseImpl and all Comment use case classes where fileReader.findAll(...,
null) is used, replacing the null status argument with FileStatus.UPLOADED and
keep mapping via fileMapper::toFileResponse.
Contributor
Author
There was a problem hiding this comment.
우선 파일은 Hard Delete 하고 있어. 타 도메인도 리팩토링 예정이라 추후에 작업할 예정이ㅑㅇ.
There was a problem hiding this comment.
@hyxklee, 이해했습니다! 현재 Hard Delete를 사용하고 계시다면 삭제된 파일이 DB에 남아있지 않으므로 null을 전달해도 문제가 없습니다.
제 리뷰는 코드 구조상 FileStatus.DELETED와 markDeleted() 메서드가 있어서 Soft Delete를 사용하는 것으로 판단했었는데, 실제로는 Hard Delete를 사용하고 계시군요. 추후 Soft Delete로 전환하실 때 해당 부분도 함께 반영하시면 될 것 같습니다.
타 도메인 리팩토링과 함께 작업 예정이시니 그때 일괄 적용하시는 것이 효율적일 것 같습니다. 감사합니다!
Comment on lines
70
to
72
| private List<File> getFiles(Long receiptId) { | ||
| return fileGetService.findAllByReceipt(receiptId); | ||
| return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the file and check if it exists
find . -name "ReceiptUseCaseImpl.java" -type f | head -20Repository: Team-Weeth/weeth-server
Length of output: 151
🏁 Script executed:
#!/bin/bash
# Search for FileReader interface/class definition
find . -name "FileReader.java" -type fRepository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for FileStatus enum
find . -name "FileStatus.java" -type fRepository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Read the ReceiptUseCaseImpl.java file
cat -n ./src/main/java/com/weeth/domain/account/application/usecase/ReceiptUseCaseImpl.javaRepository: Team-Weeth/weeth-server
Length of output: 3753
🏁 Script executed:
#!/bin/bash
# Search for FileReader class/interface definition
rg "class FileReader|interface FileReader" --type java -A 5Repository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for FileStatus enum definition
rg "enum FileStatus|class FileStatus" --type java -A 10Repository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Search for FileReader
rg "FileReader" --type java -lRepository: Team-Weeth/weeth-server
Length of output: 566
🏁 Script executed:
#!/bin/bash
# Search for FileStatus
rg "FileStatus" --type java -lRepository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
#!/bin/bash
# List files in the file domain
find . -path "*/domain/file/domain/repository*" -type f
find . -path "*/domain/file/domain/entity*" -type fRepository: Team-Weeth/weeth-server
Length of output: 561
🏁 Script executed:
#!/bin/bash
# Read FileReader.kt
cat -n ./src/main/kotlin/com/weeth/domain/file/domain/repository/FileReader.ktRepository: Team-Weeth/weeth-server
Length of output: 716
🏁 Script executed:
#!/bin/bash
# Read FileStatus.kt
cat -n ./src/main/kotlin/com/weeth/domain/file/domain/entity/FileStatus.ktRepository: Team-Weeth/weeth-server
Length of output: 206
🏁 Script executed:
#!/bin/bash
# Check other usages of findAll with FileReader
rg "fileReader\.findAll" --type java --type kt -B 2 -A 2Repository: Team-Weeth/weeth-server
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Search for FileReader implementation
rg "class.*FileReader" --type kt -A 20Repository: Team-Weeth/weeth-server
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Also check if there are any annotations or logic around null handling
rg "status.*null" ./src/main/kotlin/com/weeth/domain/file/domain/repository/ --type kt -B 2 -A 2Repository: Team-Weeth/weeth-server
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Check all usages of findAll in the codebase (using both extensions)
rg "\.findAll\(" ./src/main/java ./src/main/kotlin --type java --type kotlin -B 1 -A 1 2>/dev/null | head -60Repository: Team-Weeth/weeth-server
Length of output: 5820
🏁 Script executed:
#!/bin/bash
# Find FileReader implementation (looking for class or object that implements it)
find ./src/main/kotlin -name "*.kt" -path "*/file/domain/repository/*" | xargs cat -nRepository: Team-Weeth/weeth-server
Length of output: 2480
🏁 Script executed:
#!/bin/bash
# Check if there's a JPA/database repository that implements FileReader
find ./src/main/kotlin -name "*.kt" -path "*FileRepository*"Repository: Team-Weeth/weeth-server
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Verify the fix would work by checking imports in ReceiptUseCaseImpl
grep -n "import.*FileStatus" ./src/main/java/com/weeth/domain/account/application/usecase/ReceiptUseCaseImpl.javaRepository: Team-Weeth/weeth-server
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check AccountUseCaseImpl for the same pattern
cat -n ./src/main/java/com/weeth/domain/account/application/usecase/AccountUseCaseImpl.java | grep -A 5 -B 5 "getFiles"Repository: Team-Weeth/weeth-server
Length of output: 1062
null 파라미터를 명시적으로 전달하여 안전한 기본값을 우회하고 있습니다.
fileReader.findAll()의 기본값은 FileStatus.UPLOADED로 설정되어 있으나, 명시적으로 null을 전달하면 이 안전한 기본값이 무효화됩니다. FileRepository 구현을 보면 null이 전달될 경우 findAllByOwnerTypeAndOwnerId()를 호출하여 상태 필터 없이 모든 파일(DELETED 포함)을 반환합니다. 이로 인해 삭제된 파일이 노출되거나 재삭제될 위험이 있습니다.
파라미터를 생략하거나 명시적으로 FileStatus.UPLOADED를 전달하여 의도를 명확히 하고 안전성을 보장하세요.
🛠️ 제안 수정
- return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null);
+ return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, FileStatus.UPLOADED);- List<File> fileList = fileReader.findAll(FileOwnerType.RECEIPT, id, null);
+ List<File> fileList = fileReader.findAll(FileOwnerType.RECEIPT, id, FileStatus.UPLOADED);또한 AccountUseCaseImpl.java의 동일한 문제도 함께 수정하세요.
📝 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
| private List<File> getFiles(Long receiptId) { | |
| return fileGetService.findAllByReceipt(receiptId); | |
| return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null); | |
| } | |
| private List<File> getFiles(Long receiptId) { | |
| return fileReader.findAll(FileOwnerType.RECEIPT, receiptId, FileStatus.UPLOADED); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/weeth/domain/account/application/usecase/ReceiptUseCaseImpl.java`
around lines 70 - 72, getFiles(Long receiptId) currently calls
fileReader.findAll(FileOwnerType.RECEIPT, receiptId, null) which overrides the
safe default and returns deleted files; change the call to omit the third
parameter (use the method overload that applies the default) or explicitly pass
FileStatus.UPLOADED to ensure only uploaded files are returned, and make the
same change in the analogous call(s) inside AccountUseCaseImpl (search for
fileReader.findAll(...) with a null status) so deleted files are not exposed.
src/main/java/com/weeth/domain/board/application/usecase/NoticeUsecaseImpl.java
Outdated
Show resolved
Hide resolved
Comment on lines
+11
to
+13
| @field:Schema(description = "저장소 키. `Type/YY-MM/UUID_원본파일명` 형식", example = "POST/2026-02/58400-e29b-44-a716-44665000_profile-image.png") | ||
| @field:NotBlank | ||
| val storageKey: String, |
There was a problem hiding this comment.
storageKey 예시의 UUID 형식이 불완전합니다.
Schema 예시의 UUID(58400-e29b-44-a716-44665000)가 실제 UUID 형식과 맞지 않습니다. 테스트 코드(FileTest.kt)에서 사용하는 형식(550e8400-e29b-41d4-a716-446655440000)과 일치시키는 것이 좋습니다.
📝 예시 형식 수정
- `@field`:Schema(description = "저장소 키. `Type/YY-MM/UUID_원본파일명` 형식", example = "POST/2026-02/58400-e29b-44-a716-44665000_profile-image.png")
+ `@field`:Schema(description = "저장소 키. `Type/YY-MM/UUID_원본파일명` 형식", example = "POST/2026-02/550e8400-e29b-41d4-a716-446655440000_profile-image.png")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @field:Schema(description = "저장소 키. `Type/YY-MM/UUID_원본파일명` 형식", example = "POST/2026-02/58400-e29b-44-a716-44665000_profile-image.png") | |
| @field:NotBlank | |
| val storageKey: String, | |
| `@field`:Schema(description = "저장소 키. `Type/YY-MM/UUID_원본파일명` 형식", example = "POST/2026-02/550e8400-e29b-41d4-a716-446655440000_profile-image.png") | |
| `@field`:NotBlank | |
| val storageKey: String, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/file/application/dto/request/FileSaveRequest.kt`
around lines 11 - 13, The schema example for storageKey in FileSaveRequest
(field: storageKey) uses a malformed UUID; update the example value to use a
valid UUID format matching tests (e.g., the UUID from FileTest.kt like
550e8400-e29b-41d4-a716-446655440000) so the storageKey example follows
`Type/YY-MM/UUID_originalFilename` format; ensure only the example string is
changed and the `@field`:Schema text still documents the same format.
There was a problem hiding this comment.
@hyxklee 네, 예시인 것은 맞습니다. 다만 API 문서의 예시는 개발자들이 실제로 참고하거나 복사해서 사용하는 경우가 많아서, 올바른 형식을 보여주는 것이 혼란을 줄일 수 있습니다. 특히 실제 응답에서는 올바른 UUID 형식(5bc0ef6b-ec2e-4780-b6a6-3a54064754c9)을 사용하고 있으니, 예시도 동일한 형식으로 맞춰두면 더 명확할 것 같습니다.
하지만 현재 형식으로 유지하셔도 괜찮습니다! 😊
Contributor
Author
There was a problem hiding this comment.
어차피 StorageKey는 유니크해서 예시를 쓰면 안돼
There was a problem hiding this comment.
@hyxklee 네, 이해했습니다! StorageKey는 서버에서 UUID로 생성되는 유니크한 값이기 때문에, 예시 값을 정확한 형식으로 보여주는 것보다 개발자가 직접 복사해서 사용하면 안 된다는 점이 더 중요하네요. 말씀하신 대로 실제로 사용 가능한 예시를 제공하면 오히려 혼란을 줄 수 있겠습니다.
현재 형식 그대로 유지하시는 게 맞는 것 같습니다! 👍
soo0711
approved these changes
Feb 17, 2026
Collaborator
soo0711
left a comment
soo0711
left a comment
There was a problem hiding this comment.
수고하셨습니다! 아래에 의견 하나를 추가해봤는데 확인 부탁드립니당
Comment on lines
+14
to
+16
| fun generateFileUploadUrls( | ||
| ownerType: FileOwnerType, | ||
| fileNames: List<String>, |
Collaborator
There was a problem hiding this comment.
이 메서드 이름이 generate라서 조회 역할인 QueryService 에 있으니까 조금 어색한 감이 있는 것 같습니당 DB에 저장은 안 하더라도 외부 리소스를 발급받는 작업은CommandService 쪽으로 옮기는 게 헷갈리지 않을 것 같은데 어떤가요??
Contributor
Author
There was a problem hiding this comment.
좋습니다! 외부 리소스 발급도 Usecase라고 판단하고 Command 쪽에 위치시키겠습니당
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/kotlin/com/weeth/domain/file/application/usecase/command/GenerateFileUrlUsecaseTest.kt (1)
18-18: Mock 변수명을 프로덕션 코드와 일관되게 맞추는 것을 권장합니다.
preSignedService대신fileUploadUrlPort로 명명하면 프로덕션 코드와의 일관성이 높아져 가독성이 향상됩니다.♻️ 제안하는 변수명 변경
- val preSignedService = mockk<FileUploadUrlPort>() + val fileUploadUrlPort = mockk<FileUploadUrlPort>() val fileMapper = mockk<FileMapper>() - val useCase = GenerateFileUrlUsecase(preSignedService, fileMapper) + val useCase = GenerateFileUrlUsecase(fileUploadUrlPort, fileMapper) beforeEach { - clearMocks(preSignedService, fileMapper) + clearMocks(fileUploadUrlPort, fileMapper) }이후 테스트 코드 내
preSignedService참조도fileUploadUrlPort로 변경이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/file/application/usecase/command/GenerateFileUrlUsecaseTest.kt` at line 18, Rename the mock variable preSignedService to fileUploadUrlPort in GenerateFileUrlUsecaseTest (the mock of FileUploadUrlPort) and update all its usages in the test to the new name so it matches the production interface naming; ensure the mockk<FileUploadUrlPort>() declaration and any references in setup, stubs, and verifications (e.g., where preSignedService was used) are all renamed to fileUploadUrlPort.src/main/kotlin/com/weeth/domain/file/application/usecase/command/GenerateFileUrlUsecase.kt (1)
17-20: 두 개의map호출을 하나로 합쳐서 간결하게 작성할 수 있습니다.중간 객체 생성 없이 단일
map블록으로 변환하면 가독성이 향상됩니다.♻️ 제안하는 리팩토링
): List<UrlResponse> = fileNames - .map { fileUploadUrlPort.generateUploadUrl(ownerType, it) } - .map { fileMapper.toUrlResponse(it.fileName, it.url, it.storageKey) } + .map { fileName -> + fileUploadUrlPort.generateUploadUrl(ownerType, fileName).let { + fileMapper.toUrlResponse(it.fileName, it.url, it.storageKey) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/file/application/usecase/command/GenerateFileUrlUsecase.kt` around lines 17 - 20, GenerateFileUrlUsecase currently chains two map calls over fileNames (first calling fileUploadUrlPort.generateUploadUrl, then fileMapper.toUrlResponse) which creates an intermediate collection; replace with a single map that for each fileName calls fileUploadUrlPort.generateUploadUrl(ownerType, fileName) and immediately maps the returned object into fileMapper.toUrlResponse(it.fileName, it.url, it.storageKey) to produce the List<UrlResponse> in one pass, keeping references to fileNames, fileUploadUrlPort.generateUploadUrl, and fileMapper.toUrlResponse.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/weeth/domain/board/application/usecase/NoticeUsecaseImpl.java`:
- Around line 111-117: The current block in NoticeUsecaseImpl deletes existing
files whenever dto.files() is non-null but doesn't guard against an empty list;
change the condition to only run the delete-and-save flow when the client
actually provided files (i.e., dto.files() != null && !dto.files().isEmpty()),
so update the if around getFiles(), fileRepository.deleteAll(...),
fileMapper.toFileList(...), and fileRepository.saveAll(...) accordingly
(mirroring ReceiptUseCaseImpl's if(!dto.files().isEmpty()) behavior).
In `@src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt`:
- Line 31: Replace the controller-side runtime check that uses
require(fileNames.isNotEmpty()) in FileController's getUrl method with Spring
validation annotations: annotate the fileNames request parameter with `@NotEmpty`
(and optionally `@Valid` on the method or parameter if needed) so
binding/validation produces a BindException and yields HTTP 400; remove the
require() call from getUrl and keep require() usage limited to domain value
objects/VOs only; ensure the getUrl signature for FileController reflects the
`@NotEmpty` annotation on the fileNames List<String> parameter and that the
existing CommonExceptionHandler already maps BindException to 400.
---
Duplicate comments:
In
`@src/main/java/com/weeth/domain/board/application/usecase/NoticeUsecaseImpl.java`:
- Around line 147-148: The call in checkFileExistsByNotice uses
fileReader.exists(FileOwnerType.NOTICE, noticeId, null) with a null third
argument; verify the contract/signature of fileReader.exists to determine
whether the third parameter may be null and replace the null with the correct
value (e.g., a specific FileOwnerSubType/default subtype, Optional.empty(), or
call an overload without the subtype). If the method does not accept null,
update checkFileExistsByNotice to supply the proper subtype or use the correct
exists(...) overload and add a null-safe handling path so no null is passed to
fileReader.exists.
- Around line 168-170: The call to fileReader.findAll(FileOwnerType.COMMENT,
comment.getId(), null) may violate the method's contract by passing a null
provider; confirm whether findAll permits a null third argument and if not,
supply the correct provider or an explicit optional/empty value. Update the call
in NoticeUsecaseImpl (where files is populated) to either pass a valid
FileProvider value instead of null, use an overloaded/findAll variant that
accepts no provider, or guard/handle a potential null return from
fileReader.findAll before mapping with fileMapper::toFileResponse to avoid NPEs.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/file/application/usecase/command/GenerateFileUrlUsecase.kt`:
- Around line 17-20: GenerateFileUrlUsecase currently chains two map calls over
fileNames (first calling fileUploadUrlPort.generateUploadUrl, then
fileMapper.toUrlResponse) which creates an intermediate collection; replace with
a single map that for each fileName calls
fileUploadUrlPort.generateUploadUrl(ownerType, fileName) and immediately maps
the returned object into fileMapper.toUrlResponse(it.fileName, it.url,
it.storageKey) to produce the List<UrlResponse> in one pass, keeping references
to fileNames, fileUploadUrlPort.generateUploadUrl, and fileMapper.toUrlResponse.
In
`@src/test/kotlin/com/weeth/domain/file/application/usecase/command/GenerateFileUrlUsecaseTest.kt`:
- Line 18: Rename the mock variable preSignedService to fileUploadUrlPort in
GenerateFileUrlUsecaseTest (the mock of FileUploadUrlPort) and update all its
usages in the test to the new name so it matches the production interface
naming; ensure the mockk<FileUploadUrlPort>() declaration and any references in
setup, stubs, and verifications (e.g., where preSignedService was used) are all
renamed to fileUploadUrlPort.
Comment on lines
+111
to
+117
| if (dto.files() != null) { | ||
| List<File> fileList = getFiles(noticeId); | ||
| fileRepository.deleteAll(fileList); | ||
|
|
||
| List<File> files = fileMapper.toFileList(dto.files(), notice); | ||
| fileSaveService.save(files); | ||
| List<File> files = fileMapper.toFileList(dto.files(), FileOwnerType.NOTICE, notice.getId()); | ||
| fileRepository.saveAll(files); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src -name "NoticeDTO*.java" -o -name "*NoticeDTO*" | head -20Repository: Team-Weeth/weeth-server
Length of output: 134
🏁 Script executed:
rg -n "class NoticeDTO|record NoticeDTO|interface NoticeDTO" -A 50 srcRepository: Team-Weeth/weeth-server
Length of output: 4898
🏁 Script executed:
rg -n "dto\.files\(\)|files\(" src/main -B 2 -A 2 | head -100Repository: Team-Weeth/weeth-server
Length of output: 10141
🏁 Script executed:
rg -n "NoticeUsecaseImpl|update\(" src/main/java/com/weeth/domain/board/application/usecase/NoticeUsecaseImpl.java -B 2 -A 15 | head -150Repository: Team-Weeth/weeth-server
Length of output: 1690
🏁 Script executed:
rg -n "NoticeUsecaseImpl|@PostMapping|@PutMapping|update\(" src/main/java/com/weeth/domain/board -B 3 -A 10 | grep -A 20 "update"Repository: Team-Weeth/weeth-server
Length of output: 21447
🏁 Script executed:
fd "NoticeController.java" src -x cat {} | head -200Repository: Team-Weeth/weeth-server
Length of output: 2052
🏁 Script executed:
rg -n "class.*Controller.*Notice" -A 50 src/main/java/com/weeth/domain/boardRepository: Team-Weeth/weeth-server
Length of output: 49
빈 배열 전달 시 기존 파일이 모두 삭제되는 문제 확인.
NoticeDTO.Update의 files 필드는 nullable이 아닌 List<@NotNull FileSaveRequest> 이며, 클라이언트가 빈 배열 []을 전달할 수 있습니다. 현재 코드는 null만 체크하므로 빈 배열이 전달되면 기존 파일을 전부 삭제하고 새 파일을 저장하지 않아 데이터 손실이 발생합니다.
같은 패턴을 ReceiptUseCaseImpl에서 올바르게 구현했습니다 (line 59: if(!dto.files().isEmpty())). 이와 같이 수정하여 "변경 없음"과 "파일 삭제"를 구분해주세요.
제안 수정
- if (dto.files() != null) {
+ if (dto.files() != null && !dto.files().isEmpty()) {
List<File> fileList = getFiles(noticeId);
fileRepository.deleteAll(fileList);
List<File> files = fileMapper.toFileList(dto.files(), FileOwnerType.NOTICE, notice.getId());
fileRepository.saveAll(files);
}📝 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
| if (dto.files() != null) { | |
| List<File> fileList = getFiles(noticeId); | |
| fileRepository.deleteAll(fileList); | |
| List<File> files = fileMapper.toFileList(dto.files(), notice); | |
| fileSaveService.save(files); | |
| List<File> files = fileMapper.toFileList(dto.files(), FileOwnerType.NOTICE, notice.getId()); | |
| fileRepository.saveAll(files); | |
| } | |
| if (dto.files() != null && !dto.files().isEmpty()) { | |
| List<File> fileList = getFiles(noticeId); | |
| fileRepository.deleteAll(fileList); | |
| List<File> files = fileMapper.toFileList(dto.files(), FileOwnerType.NOTICE, notice.getId()); | |
| fileRepository.saveAll(files); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/weeth/domain/board/application/usecase/NoticeUsecaseImpl.java`
around lines 111 - 117, The current block in NoticeUsecaseImpl deletes existing
files whenever dto.files() is non-null but doesn't guard against an empty list;
change the condition to only run the delete-and-save flow when the client
actually provided files (i.e., dto.files() != null && !dto.files().isEmpty()),
so update the if around getFiles(), fileRepository.deleteAll(...),
fileMapper.toFileList(...), and fileRepository.saveAll(...) accordingly
(mirroring ReceiptUseCaseImpl's if(!dto.files().isEmpty()) behavior).
src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt`:
- Around line 30-33: The getUrl handler in FileController (function getUrl) is
missing injection of the authenticated user; update the method signature to
accept the current user ID via the `@CurrentUser` annotation (e.g., add a
parameter like `@CurrentUser` userId: Long or UUID) and then use that userId where
owner/permission checks are performed for FileOwnerType and fileNames; ensure
the new parameter is validated as non-null and propagated into any service calls
invoked by getUrl so ownership/authorization follows the JWT/OAuth2 guidelines.
Comment on lines
+30
to
+33
| fun getUrl( | ||
| @Parameter(description = "파일 소유 타입", example = "POST") | ||
| @RequestParam ownerType: FileOwnerType, | ||
| @RequestParam @NotEmpty fileNames: List<@NotBlank String>, |
There was a problem hiding this comment.
인증 사용자 ID 주입이 누락되어 있습니다.
인증이 필요한 엔드포인트라면 @CurrentUser로 사용자 ID를 받아 권한/소유자 검증에 활용해야 합니다. 현재 시그니처는 인증 컨텍스트를 사용하지 않아 가이드와 불일치합니다.
As per coding guidelines, Authentication uses JWT with symmetric key (JJWT 0.13.0), OAuth2 via Kakao and Apple. Use @CurrentUser annotation to inject authenticated user ID into controller methods.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/file/presentation/FileController.kt` around
lines 30 - 33, The getUrl handler in FileController (function getUrl) is missing
injection of the authenticated user; update the method signature to accept the
current user ID via the `@CurrentUser` annotation (e.g., add a parameter like
`@CurrentUser` userId: Long or UUID) and then use that userId where
owner/permission checks are performed for FileOwnerType and fileNames; ensure
the new parameter is validated as non-null and propagated into any service calls
invoked by getUrl so ownership/authorization follows the JWT/OAuth2 guidelines.
This was referenced Feb 18, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
📌 Summary
File 도메인을 가장 먼저 코틀린으로 마이그레이션 했습니다. 마이그레이션 과정 중에 개선할 점들 같이 개선했습니다
📝 Changes
What
Why
How
📸 Screenshots / Logs
💡 Reviewer 참고사항
[프론트 분들께] 파일 관련된 로직이 수정되었습니다! 앞으로 파일 저장시에는 전체 url이 아니라 StorageKey를 받도록 하려고 합니다! 아래처럼 수정될 예정입니다 혹시 문제가 있다면 알려주세용
참고로 파일 확장자도 관리할 예정입니다. 현재는 png, jpg, jpeg, pdf, webp만 열어둠
✅ Checklist
Summary by CodeRabbit
릴리스 노트
New Features
Tests