-
Notifications
You must be signed in to change notification settings - Fork 0
๐ :: [#719] - ๋ง์ดํธ ํด์ API ์ถ๊ฐ #720
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@dolong2 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 6 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 (2)
Walkthrough๋ณผ๋ฅจ ๋ง์ดํธ ๊ด๋ จ: ๋จ๊ฑด ๋ง์ดํธ ์กฐํ/์ญ์ ํฌํธยทํผ์์คํด์คยท๋ฆฌํฌ์งํ ๋ฆฌ ๋ฐ ์น ์๋ํฌ์ธํธ(DELETE) ์ถ๊ฐ, ์ค๋ณต ๋ง์ดํธ ๊ฒ์ฌยท์์ธ ์ถ๊ฐ, VolumeMount ์๋ณ์ ๋ชจ๋ธ์ UUID ๋จ์ผํค์์ ์ ํ๋ฆฌ์ผ์ด์ +๋ณผ๋ฅจ ๋ณตํฉํค๋ก ๋ณ๊ฒฝ, ๊ด๋ จ ์๋ฌ ์ฝ๋ยท์์ธยทํ ์คํธ ์ถ๊ฐ. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Web as VolumeWebAdapter
participant MUC as MountVolumeUseCase
participant QV as QueryVolumePort
participant QA as QueryApplicationPort
participant EV as ApplicationEventPublisher
Client->>Web: POST /{ws}/volume/{vid}/mount?applicationId=...
Web->>MUC: execute(volumeId, applicationId)
MUC->>QV: findById(volumeId)
MUC->>QA: findById(applicationId)
MUC->>QV: findMountByApplicationAndVolume(app, vol)
alt Mount exists
MUC-->>Web: throw AlreadyExistsVolumeMountException
Web-->>Client: 4xx
else Not exists
MUC->>QV: save mount
MUC->>EV: publish DeployApplicationEvent
Web-->>Client: 200 OK
end
sequenceDiagram
autonumber
actor Client
participant Web as VolumeWebAdapter
participant UUC as UnMountVolumeUseCase
participant QV as QueryVolumePort
participant QA as QueryApplicationPort
participant CV as CommandVolumePort
participant EV as ApplicationEventPublisher
Client->>Web: DELETE /{ws}/volume/{vid}/mount?applicationId=...
Web->>UUC: execute(volumeId, applicationId)
UUC->>QV: findById(volumeId)
UUC->>QA: findById(applicationId)
UUC->>QV: findMountByApplicationAndVolume(app, vol)
alt Mount not found
UUC-->>Web: throw VolumeMountNotFoundException
Web-->>Client: 404
else Found
UUC->>CV: deleteMount(volumeMount)
UUC->>EV: publish DeployApplicationEvent
Web-->>Client: 200 OK
end
Estimated code review effort๐ฏ 4 (Complex) | โฑ๏ธ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touchesโ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
๐งน Nitpick comments (10)
src/main/kotlin/com/dcd/server/core/domain/volume/spi/CommandVolumePort.kt (1)
13-13: ์ญ์ ์ฐ์ฐ ๊ณ์ฝ ๋ช ํํ ์ ์
deleteMount์ ๊ฒฐ๊ณผ ๊ณ์ฝ(์กด์ฌํ์ง ์์๋ ์ฑ๊ณต์ผ๋ก ๋ณผ์ง, ์์ธ๋ฅผ ๋์ง์ง)์ KDoc์ ๋ช ์ํ๋ฉด ํธ์ถ๋ถ ์ฒ๋ฆฌ ์ผ๊ด์ฑ์ด ์ข์์ง๋๋ค.src/main/kotlin/com/dcd/server/core/domain/volume/spi/QueryVolumePort.kt (1)
20-20: ๋จ๊ฑด ์กฐํ ๋ฉ์๋ ์ถ๊ฐ ์ ์ โ ๋ค์ด๋ฐ/๊ณ์ฝ ๋ช ์ ์ ์๋จ๊ฑด ๋ฐํ์ โ์ ์ผ์ฑ ๋ณด์ฅโ ์ ์ ๊ฐ ํ์ํฉ๋๋ค. ๋ฉ์๋ KDoc์ ์ ์ผ์ฑ ์ ์ ์ NotFound/์ค๋ณต ์ ๋์์ ๋ช ์ํ๊ฑฐ๋, ์ด๋ฆ์
findMountByVolumeAndApplication๋ก ๋ง์ถ๋ฉด ์ ์ฅ์ ๋ฉ์๋์ ์ผ๊ด์ฑ์ด ์ข์์ง๋๋ค.src/main/kotlin/com/dcd/server/persistence/volume/VolumePersistenceAdapter.kt (1)
61-63: ์กฐ์ธ ํค ๊ธฐ๋ฐ ์กฐํ ๊ตฌํ OK โ ์ฝ๊ธฐ ์ ์ฉ ํธ๋์ญ์ ๊ถ์ฅ์ฝ๊ธฐ ๋ฉ์๋๋
@Transactional(readOnly = true)์ ์ฉ ์ ์ฑ๋ฅ ์ด์ ์ด ์์ต๋๋ค(2์ฐจ ์บ์/ํ๋ฌ์ ๋ฐฉ์ง).src/test/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapterTest.kt (1)
155-170: ์ธ๋ง์ดํธ API ํธ์ถ ๊ฒ์ฆ ๊ฐํ ์ ์์ ํํ ์ํธ์์ฉ์ ๋ณด์ฅํ๊ธฐ ์ํด ํธ์ถ ํ์ ๋ฐ ๋ถํ์ ์ํธ์์ฉ ๋ถ์ฌ๋ฅผ ๊ฒ์ฆํ๋ฉด ์ข์ต๋๋ค.
- then("๋ณผ๋ฅจ ๋ง์ดํธ ํด์ ์ ์ค์ผ์ด์ค๊ฐ ์คํ๋์ด์ผํจ") { - verify { unMountVolumeUseCase.execute(testVolumeId, testApplicationId) } - } + then("๋ณผ๋ฅจ ๋ง์ดํธ ํด์ ์ ์ค์ผ์ด์ค๊ฐ 1ํ ์คํ๋์ด์ผํจ") { + verify(exactly = 1) { unMountVolumeUseCase.execute(testVolumeId, testApplicationId) } + confirmVerified(unMountVolumeUseCase) + }src/test/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCaseTest.kt (1)
117-136: ์ค๋ณต ๋ง์ดํธ ์๋๋ฆฌ์ค: ํ ์คํธ ๊ฐ ์ํ ๊ฒฉ๋ฆฌ ๋ณด๊ฐ๊ฐ์ ์ปจํ ์ด๋ ๋ด ์ ํ ํ ์คํธ์์ ์ด๋ฏธ ๋ง์ดํธ๊ฐ ์์ฑ๋์์ ์ ์์ด, ์ด ๋ธ๋ก์
beforeContainer์์ ๋ ์ ์ฅํ๋ฉด ํ ์คํธ ์์์ ๋ฐ๋ผ ์ค๋ณต ๋ฐ์ดํฐ๋ก ์ธํ ๋ถ์์ ์ด ์๊ธธ ์ ์์ต๋๋ค. ๋ธ๋ก ์ง์ ์ ์ ๋ฆฌํฌ์งํ ๋ฆฌ๋ฅผ ์ ๋ฆฌํ๊ฑฐ๋beforeTest๋ก ์ ํํด ์ผ์ด์ค ๊ฐ ์ํ๋ฅผ ๊ฒฉ๋ฆฌํด ์ฃผ์ธ์.- `when`("์ด๋ฏธ ๋ณผ๋ฅจ์ ๋ง์ดํธ ๋์๋") { - beforeContainer { + `when`("์ด๋ฏธ ๋ณผ๋ฅจ์ ๋ง์ดํธ ๋์๋") { + beforeTest { + volumeMountRepository.deleteAll() val application = queryApplicationPort.findById(targetApplicationId)!! val volume = volumeRepository.findByIdOrNull(targetVolumeId)!!.toDomain() val volumeMount = VolumeMount( id = UUID.randomUUID(), application = application, volume = volume, mountPath = "/test/volume", readOnly = false ) volumeMountRepository.save(volumeMount.toEntity()) }src/main/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapter.kt (2)
37-39: ์์ฑ์ ์์กด์ฑ ํ์ฅ OK โ REST ์๋งจํฑ์ค์ ์๋ต ์ฝ๋ ์ฌ๊ฒํ ๊ถ์ฅ์ธ๋ง์ดํธ๋ ๋ฆฌ์์ค ์ ๊ฑฐ ์ฑ๊ฒฉ์ด๋ฏ๋ก 200๋ณด๋ค 204(No Content)๊ฐ ์์ฐ์ค๋ฝ์ต๋๋ค. ์๋ ์๋ํฌ์ธํธ์์ ๋ฐ์ ์ ์๋๋ฆฝ๋๋ค.
94-103: DELETE ์๋ต ์ฝ๋๋ฅผ 204(No Content)๋ก ๋ณ๊ฒฝ ์ ์REST ๊ด๋ก์ ์ญ์ ์ฑ๊ณต ์ 204๊ฐ ์ ์ ํฉ๋๋ค. ํ ์คํธ๋ ํจ๊ป ์ ๋ฐ์ดํธํด ์ฃผ์ธ์.
- @DeleteMapping("/{volumeId}/mount") + @DeleteMapping("/{volumeId}/mount") @WorkspaceOwnerVerification("#workspaceId") fun unMountVolume( @PathVariable workspaceId: String, @PathVariable volumeId: UUID, @RequestParam applicationId: String, ): ResponseEntity<Void> = - unMountVolumeUseCase.execute(volumeId, applicationId) - .run { ResponseEntity.ok().build() } + unMountVolumeUseCase.execute(volumeId, applicationId) + .run { ResponseEntity.noContent().build() }src/test/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCaseTest.kt (1)
45-64: ์๋ ๋ฐ์ดํฐ ์์ฑ ์์น ์ ๊ฒ
beforeSpec์์ ์์ฑ๋ ์ํฐํฐ๋ ์ผ์ด์ค ๊ฐ ๊ณต์ ๋ฉ๋๋ค. ์ดํ ์ผ์ด์ค์์ ์ญ์ /์์ ์ ๋ค๋ฅธ ์ผ์ด์ค์ ์ํฅ ์ค ์ ์์ผ๋, ๊ฐ๋ฅํ๋ฉด ์ผ์ด์ค๋ณbeforeTest๋ก ๊ฒฉ๋ฆฌํ๊ฑฐ๋ ๊ฐ ์ผ์ด์ค ์์ ์ ๋ฆฌํฌ์งํ ๋ฆฌ ์ ๋ฆฌ๋ฅผ ๊ถ์ฅํฉ๋๋ค.src/main/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCase.kt (2)
24-43: ํธ๋์ญ์ ๊ฒฝ๊ณ์ ์ด๋ฒคํธ ๋ฐํ ์์ ์ฌ๊ฒํ์ญ์ ํ ์ฆ์
ApplicationEventPublisher๋ก ๋๊ธฐ ์ด๋ฒคํธ๋ฅผ ๋ฐํํ๋ฉด ๋กค๋ฐฑ ์ ์ ํฉ์ฑ ๋ฌธ์ ๊ฐ ์๊ธธ ์ ์์ต๋๋ค. ํธ๋์ญ์ ๊ฒฝ๊ณ ๋ด์์ ์ญ์ ๋ฅผ ๋ณด์ฅํ๊ณ , ์ปค๋ฐ ์ดํ์ ์ด๋ฒคํธ๊ฐ ์ฒ๋ฆฌ๋๋๋ก ์ค๊ณํ๋ ๊ฒ์ ๊ถ์ฅํฉ๋๋ค.๊ถ์ฅ์ฌํญ:
@Transactional(์๋น์ค ๊ณ์ธต) ์ถ๊ฐ๋ก ์ญ์ ์์์ฑ ๋ณด์ฅ.TransactionalEventPublisher๋๋@TransactionalEventListener(phase = AFTER_COMMIT)/์์๋ฐ์ค ํจํด ๋์ .์์:
+import org.springframework.transaction.annotation.Transactional @@ -@UseCase -class UnMountVolumeUseCase( +@UseCase +@Transactional +class UnMountVolumeUseCase(
38-41: ์ธ๋ง์ดํธ์ ๋ฉฑ๋ฑ์ฑ(idempotency) ๊ณ ๋ ค(์ ํ)์ค๋ฌด ์ด์์์๋ ์ด๋ฏธ ํด์ ๋ ์ํ์ ๋ํ ๋ฐ๋ณต ์์ฒญ์ ์ฑ๊ณต(204) ์ฒ๋ฆฌํ๋ ๊ฒ๋ UX/์ด์์ ์ด์ ์ด ์์ต๋๋ค. ํ์ฌ๋
VolumeMountNotFoundException์ ๋์ง๋ฏ๋ก, API ๊ณ์ฝ์ ๋ฐ๋ผ ์ ์งํ๋ ํ์ ์ ์ต์ ํ(์ฟผ๋ฆฌ ํ๋ผ๋ฏธํฐ ๋๋ ๋ณ๋ ์๋ํฌ์ธํธ ์ ์ฑ ) ๊ฒํ ๋ฅผ ์ ์ํฉ๋๋ค.๊ธฐ์กด API ๊ฐ์ด๋์ DELETE ๋ฉฑ๋ฑ์ฑ ์๊ตฌ๊ฐ ์๋์ง ํ์ธ ๋ถํ๋๋ฆฝ๋๋ค.
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (13)
src/main/kotlin/com/dcd/server/core/common/error/ErrorCode.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/volume/exception/VolumeMountNotFoundException.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/volume/spi/CommandVolumePort.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/volume/spi/QueryVolumePort.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt(2 hunks)src/main/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCase.kt(1 hunks)src/main/kotlin/com/dcd/server/infrastructure/global/config/SecurityConfig.kt(1 hunks)src/main/kotlin/com/dcd/server/persistence/volume/VolumePersistenceAdapter.kt(2 hunks)src/main/kotlin/com/dcd/server/persistence/volume/repository/VolumeMountRepository.kt(1 hunks)src/main/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapter.kt(3 hunks)src/test/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCaseTest.kt(4 hunks)src/test/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCaseTest.kt(1 hunks)src/test/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapterTest.kt(3 hunks)
๐งฐ Additional context used
๐ง Learnings (3)
๐ Common learnings
Learnt from: dolong2
PR: dolong2/dcd#703
File: src/main/kotlin/com/dcd/server/core/domain/volume/model/Volume.kt:1-12
Timestamp: 2025-09-01T14:52:42.625Z
Learning: dolong2 prefers to implement validation logic (such as path validation for Volume.physicalPath) in the business logic layer rather than in domain model init blocks, keeping domain models as simple data containers.
๐ Learning: 2025-09-04T14:59:15.319Z
Learnt from: dolong2
PR: dolong2/dcd#710
File: src/main/kotlin/com/dcd/server/core/domain/volume/service/impl/DeleteDockerVolumeServiceImpl.kt:13-18
Timestamp: 2025-09-04T14:59:15.319Z
Learning: dolong2 addressed command injection vulnerability in volume deletion by adding validation in the volume creation service (CreateDockerVolumeServiceImpl) using NAME_SAFE_REGEX pattern, preferring to validate at the source rather than in the deletion service for better security and fail-fast behavior.
Applied to files:
src/main/kotlin/com/dcd/server/infrastructure/global/config/SecurityConfig.kt
๐ Learning: 2024-12-17T14:26:50.307Z
Learnt from: dolong2
PR: dolong2/dcd#411
File: src/test/kotlin/com/dcd/server/core/domain/auth/usecase/SignOutUseCaseTest.kt:32-40
Timestamp: 2024-12-17T14:26:50.307Z
Learning: `SignOutUseCaseTest.kt`์ `afterContainer`์์ ํ
์คํธ ๋ฐ์ดํฐ ์ ๋ฆฌ๋ฅผ ํ ๋ ์์ธ ์ฒ๋ฆฌ๋ฅผ ํ์ง ์์๋ ๊ด์ฐฎ๋ค.
Applied to files:
src/test/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCaseTest.kt
โฐ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Test
๐ Additional comments (13)
src/main/kotlin/com/dcd/server/core/common/error/ErrorCode.kt (1)
50-50: ์ ์๋ฌ ์ฝ๋ ์ถ๊ฐ ์ ์ โ Web/์์ธ ๋งคํ ๋๊ธฐํ๋ง ํ์ธํด์ฃผ์ธ์๋๋ฉ์ธยท์น ๋ ์ด์ด์์
VOLUME_MOUNT_NOT_FOUND๋ก ์ง๋ ฌํ/๋งคํ๋๋์ง, ์๋ฌ ์คํ ๋ฌธ์๋ ํจ๊ป ๊ฐฑ์ ๋๋์ง ํ์ธ ๋ถํ๋๋ฆฝ๋๋ค.src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt (1)
9-9: ์์ธ import ์ถ๊ฐ ์ ์ ์ค๋ณต ๋ง์ดํธ ๋ฐฉ์ง ํ๋ก์ฐ์ ํ์ํ ์์กด์ฑ ์ ํฉํฉ๋๋ค.
src/main/kotlin/com/dcd/server/core/domain/volume/exception/VolumeMountNotFoundException.kt (1)
1-6: ๋๋ฉ์ธ ์์ธ ์ ์ ์ ์ 404 ์ฝ๋์ ์๋ฏธ๊ฐ ์ผ์นํฉ๋๋ค. ๋ฉ์์ง/i18n ๋ฆฌ์์ค ๋๊ธฐํ๋ง ํ์ธํ๋ฉด ๋ฉ๋๋ค.
src/main/kotlin/com/dcd/server/persistence/volume/VolumePersistenceAdapter.kt (1)
35-37: ์ญ์ ์์ ๊ตฌํ OK์์ ๋ ์ด์ด์์ ์กด์ฌ ๊ฒ์ฆ์ ์ํํ๋ฏ๋ก
deleteById๋ก ์ถฉ๋ถํฉ๋๋ค. ํธ๋์ญ์ ๊ฒฝ๊ณ์์ ๋์ํ๋์ง๋ง ํ์ธํด์ฃผ์ธ์.src/main/kotlin/com/dcd/server/infrastructure/global/config/SecurityConfig.kt (1)
108-108: ๊ฒ์ฆ ์๋ฃ โ DELETE ๋งค์ฒ ๋ณ๊ฒฝ ๋ถํ์VolumeWebAdapter๋ @WebAdapter("/{workspaceId}/volume") + @DeleteMapping("/{volumeId}/mount")์ด๊ณ applicationId๋ @RequestParam์ผ๋ก ์ ๋ฌ๋ฉ๋๋ค. ๋ฐ๋ผ์ SecurityConfig์ it.requestMatchers(HttpMethod.DELETE, "/{workspaceId}/volume/{volumeId}/mount").authenticated() ๋งค์ฒ๋ ์ ํํฉ๋๋ค.
Likely an incorrect or invalid review comment.
src/test/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapterTest.kt (3)
14-14: ์ ๊ท ์ ์ค์ผ์ด์ค ์์กด์ฑ ์ํฌํธ ์ถ๊ฐ OK์น ์ด๋ํฐ/ํ ์คํธ ๊ฐ ์๊ทธ๋์ฒ ์ ํฉ์ฑ์ ๋ง๊ฒ
UnMountVolumeUseCase์ํฌํธ ์ถ๊ฐ๋ ์ ํ์ธํ์ต๋๋ค.
35-35: ์ธ๋ง์ดํธ ์ ์ค์ผ์ด์ค ๋ชฉ ์ถ๊ฐ OK
relaxUnitFun = true์ค์ ์ผ๋ก ๋ถํ์ํ ์คํ ์์ด ํธ์ถ ๊ฒ์ฆ์ด ๊ฐ๋ฅํด ๋ณด์ ๋๋ค.
43-45: ํ์ธ ์๋ฃ โ ์์ฑ์ ์ธ์ ์์/์ฃผ์ ์ผ์น
src/main/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapter.kt(31โ38)์ ์์ฑ์์ src/test/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapterTest.kt(37โ45)์ ์ธ์คํด์คํ ์ธ์ ์์๊ฐ ์ผ์นํ๋ฉฐ, ์ ์ฅ์ ๋ด ๋ค๋ฅธ ์์ฑ์ ํธ์ถ/Bean ํฉํ ๋ฆฌ ์ฐธ์กฐ๋ ๊ฒ์๋์ง ์์์ต๋๋ค.src/main/kotlin/com/dcd/server/presentation/domain/volume/VolumeWebAdapter.kt (2)
9-9: ์ธ๋ง์ดํธ ์ ์ค์ผ์ด์ค ์์กด์ฑ ์ถ๊ฐ OK์ปจํธ๋กค๋ฌ ์ฑ ์ ๋ถ๋ฆฌ๊ฐ ๋ช ํํฉ๋๋ค.
94-103: ํ์ธ: DELETE /{workspaceId}/volume/{volumeId}/mount ๋ณด์ ๊ท์น์ด ๋ฑ๋ก๋์ด ์์ต๋๋คsrc/main/kotlin/com/dcd/server/infrastructure/global/config/SecurityConfig.kt:108 โ it.requestMatchers(HttpMethod.DELETE, "/{workspaceId}/volume/{volumeId}/mount").authenticated() ํ์ธ.
src/test/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCaseTest.kt (2)
72-78: ํต์ฌ ์ฑ๊ณต ๊ฒฝ๋ก ๊ฒ์ฆ LGTM์ธ๋ง์ดํธ ํ ํผ์์คํด์ค๊ฐ ๋น์ด์๋์ง ํ์ธํ๋ ๋จ์ ์ด ๋ช ํํฉ๋๋ค.
100-110: ์ธ๋ง์ดํธ ๋น์กด์ฌ ์ผ์ด์ค ํ ์คํธ LGTM์ํ ์ ๋ฆฌ ํ
VolumeMountNotFoundException๊ฒ์ฆ์ด ์ ์ ํฉ๋๋ค.src/main/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCase.kt (1)
30-37: ํ์ธ: Workspace.equals()๊ฐ id ๊ธฐ๋ฐ์ผ๋ก ์ค๋ฒ๋ผ์ด๋๋์ด ๊ฐ์ฒด ๋๋ฑ์ฑ ๋น๊ต๋ ์์ ํฉ๋๋ค.
ํ์ธ ์์น: src/main/kotlin/com/dcd/server/core/domain/workspace/model/Workspace.kt (equals โ this.id == other.id)
src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt
Show resolved
Hide resolved
src/main/kotlin/com/dcd/server/persistence/volume/repository/VolumeMountRepository.kt
Show resolved
Hide resolved
src/test/kotlin/com/dcd/server/core/domain/volume/usecase/UnMountVolumeUseCaseTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
โป๏ธ Duplicate comments (2)
src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt (1)
41-43: ์ค๋ณต ๋ง์ดํธ ๊ฒฝ์ ์กฐ๊ฑด: DB ์์ธ ๋งคํ๊น์ง ๋ณด์ ํ์๋ณตํฉ PK๋ก ์ ๋ํฌ๋ ๋ณด์ฅ๋์ง๋ง, ๋์ ์์ฒญ ์ ์ ์ฅ ๋จ๊ณ์์ DB ์์ธ๊ฐ ๋ ์ ์์ต๋๋ค.
saveMount๊ฒฝ๋ก์์DataIntegrityViolationException์AlreadyExistsVolumeMountException์ผ๋ก ๋งคํํด ์ผ๊ด ์๋ต์ ๋ณด์ฅํ์ธ์.์์(ํผ์์คํด์ค ์ด๋ํฐ ์ชฝ ๊ถ์ฅ):
try { commandVolumePort.saveMount(volumeMount) -} +} catch (e: DataIntegrityViolationException) { + throw AlreadyExistsVolumeMountException() +}src/main/kotlin/com/dcd/server/persistence/volume/repository/VolumeMountRepository.kt (1)
9-13: ์กด์ฌ ์ฌ๋ถ ์ ์ฉ ๋ฉ์๋ ์ถ๊ฐ๋ก ๋ถํ์ํ ๋ก๋ฉ ๋ฐฉ์ง์ค๋ณต ์ฒดํฌ ์ฉ๋์๋
existsByVolumeAndApplication(...)๊ฐ ๋ ๊ฒฝ๋์ ๋๋ค. ๋์ ํ UseCase์์ ํด๋น ๊ฒฝ๋ก ์ฌ์ฉ์ ๊ถ์ฅํฉ๋๋ค.interface VolumeMountRepository : JpaRepository<VolumeMountJpaEntity, VolumeMountJpaEntity.VolumeMountId> { fun findAllByVolume(volume: VolumeJpaEntity): List<VolumeMountJpaEntity> fun findAllByApplication(application: ApplicationJpaEntity): List<VolumeMountJpaEntity> fun findByVolumeAndApplication(volume: VolumeJpaEntity, application: ApplicationJpaEntity): VolumeMountJpaEntity? + fun existsByVolumeAndApplication(volume: VolumeJpaEntity, application: ApplicationJpaEntity): Boolean }
๐งน Nitpick comments (2)
src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt (1)
50-54: ์ด๋ฒคํธ ๋ฐํ์ ํธ๋์ญ์ ๊ฒฝ๊ณ ํ์ธ์ ์ฅ ์คํจ/๋กค๋ฐฑ ์ ์ฌ๋ฐฐํฌ ์ด๋ฒคํธ๊ฐ ๋๊ฐ์ง ์๋๋ก afterโcommit ๊ธฐ๋ฐ(
TransactionSynchronizationManager/@TransactionalEventListener)์ธ์ง ํ์ธ ๋ฐ๋๋๋ค.src/test/resources/data.sql (1)
29-30: ์ญ์ ์ ์ฑ ์ฌํ์ธ: CASCADE๊ฐ ๋น์ฆ๋์ค ๊ฒ์ฆ์ ์ฐํํ ์ ์์๋ฌ๋์(๊ฒ์ฆ์ ๋น์ฆ๋์ค ๋ ์ด์ด) CASCADE๋ ์๋์ ์์ถฉํ ์ ์์ต๋๋ค. ์ต์ํ
volume_id์ชฝ CASCADE ์ ๊ฑฐ๋ฅผ ๊ณ ๋ คํ๊ฑฐ๋, ๊ด๋ จ ํ ์คํธ๋ก ์ฑ ๋ ์ด์ด ๊ฒ์ฆ์ด ๋ฐ๋์ ์คํ๋จ์ ๋ณด์ฅํ์ธ์.-alter table if exists volume_mount_entity add constraint FKr5bb2ev813ioxtylyobgdphel foreign key (application_id) references application_entity (id) on delete cascade; -alter table if exists volume_mount_entity add constraint FKq6dppr5p20mvgjditmpypicey foreign key (volume_id) references volume_entity (id) on delete cascade; +alter table if exists volume_mount_entity add constraint FKr5bb2ev813ioxtylyobgdphel foreign key (application_id) references application_entity (id); +alter table if exists volume_mount_entity add constraint FKq6dppr5p20mvgjditmpypicey foreign key (volume_id) references volume_entity (id);
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (7)
src/main/kotlin/com/dcd/server/core/domain/volume/model/VolumeMount.kt(0 hunks)src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt(2 hunks)src/main/kotlin/com/dcd/server/persistence/volume/VolumePersistenceAdapter.kt(2 hunks)src/main/kotlin/com/dcd/server/persistence/volume/adapter/VolumeAdapter.kt(0 hunks)src/main/kotlin/com/dcd/server/persistence/volume/entity/VolumeMountJpaEntity.kt(1 hunks)src/main/kotlin/com/dcd/server/persistence/volume/repository/VolumeMountRepository.kt(1 hunks)src/test/resources/data.sql(1 hunks)
๐ค Files with no reviewable changes (2)
- src/main/kotlin/com/dcd/server/core/domain/volume/model/VolumeMount.kt
- src/main/kotlin/com/dcd/server/persistence/volume/adapter/VolumeAdapter.kt
๐ง Files skipped from review as they are similar to previous changes (1)
- src/main/kotlin/com/dcd/server/persistence/volume/VolumePersistenceAdapter.kt
๐งฐ Additional context used
๐ง Learnings (3)
๐ Common learnings
Learnt from: dolong2
PR: dolong2/dcd#703
File: src/main/kotlin/com/dcd/server/core/domain/volume/model/Volume.kt:1-12
Timestamp: 2025-09-01T14:52:42.625Z
Learning: dolong2 prefers to implement validation logic (such as path validation for Volume.physicalPath) in the business logic layer rather than in domain model init blocks, keeping domain models as simple data containers.
๐ Learning: 2025-09-01T14:52:42.625Z
Learnt from: dolong2
PR: dolong2/dcd#703
File: src/main/kotlin/com/dcd/server/core/domain/volume/model/Volume.kt:1-12
Timestamp: 2025-09-01T14:52:42.625Z
Learning: dolong2 prefers to implement validation logic (such as path validation for Volume.physicalPath) in the business logic layer rather than in domain model init blocks, keeping domain models as simple data containers.
Applied to files:
src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt
๐ Learning: 2025-09-01T14:57:07.656Z
Learnt from: dolong2
PR: dolong2/dcd#703
File: src/test/resources/data.sql:28-30
Timestamp: 2025-09-01T14:57:07.656Z
Learning: dolong2 prefers to implement deletion validation in business logic rather than using database CASCADE constraints. For volume deletion, they want to validate if any mounts exist before allowing deletion, preventing accidental data loss while maintaining explicit control over the deletion process.
Applied to files:
src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt
๐ Additional comments (2)
src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt (1)
44-49: mountPath ์ ๋ ฅ ๊ฒ์ฆ ์ถ๊ฐ ๊ถ์ฅ๋น์ฆ๋์ค ๊ณ์ธต์์ mountPath์ ๋ํด ๋น๊ฐ/๊ณต๋ฐฑ, ์ ๋๊ฒฝ๋ก(/๋ก ์์), ์์ ๋๋ ํ ๋ฆฌ ์ฐธ์กฐ("..") ๋ฑ ๊ฒ์ฆ์ ์ถ๊ฐํ์ธ์.
ํ์ผ: src/main/kotlin/com/dcd/server/core/domain/volume/usecase/MountVolumeUseCase.kt (๋ผ์ธ 44-49)val volumeMount = VolumeMount( application = application, volume = volume, - mountPath = mountVolumeReqDto.mountPath, + mountPath = mountVolumeReqDto.mountPath.trim().also { + require(it.isNotEmpty()) { "mountPath must not be blank" } + require(it.startsWith("/")) { "mountPath must be absolute" } + require(!it.contains("..")) { "mountPath must not contain '..'" } + }, readOnly = mountVolumeReqDto.readOnly )src/main/kotlin/com/dcd/server/persistence/volume/entity/VolumeMountJpaEntity.kt (1)
1-37: kotlin-jpa ํ๋ฌ๊ทธ์ธยทJakarta ์ด๋ ธํ ์ด์ ์ ์ฉ ํ์ธ (build.gradle.kts)
- build.gradle.kts์ kotlin("plugin.jpa") version "1.8.22" ์ ์ฉ๋์ด ์์.
- ์ํฐํฐ๊ฐ jakarta.persistence๋ฅผ ์ฌ์ฉํ๋ฏ๋ก kotlin-jpa๊ฐ jakarta ์ด๋ ธํ ์ด์ ์ ๋์์ผ๋ก no-arg/all-open์ ์ ์ฉํ๋์ง ํ์ธํ๊ฑฐ๋, ๋ช ์์ ์ผ๋ก noArg/allOpen์ jakarta.persistence ์ด๋ ธํ ์ด์ ์ ์ถ๊ฐํ๊ฑฐ๋ Hibernate bytecode enhancer๋ฅผ ์ฌ์ฉํด์ผ ํจ. (kotlinlang.org)
- ์ ์ฅ์ ๊ฒ์์์ Hibernate bytecode enhancer ์ค์ ์ ๋ฐ๊ฒฌ๋์ง ์์ โ ๋น๋/ํ ์คํธ๋ก no-arg ์์ฑ(๊ธฐ๋ณธ ์์ฑ์)๊ณผ ํ๋ก์ ๋์์ ๋ฐ๋์ ํ์ธํ ๊ฒ.
๊ฐ์
์์ ๋ด์ฉ
์ฒดํฌ๋ฆฌ์คํธ
Summary by CodeRabbit
New Features
Tests