-
Notifications
You must be signed in to change notification settings - Fork 0
๐ :: [#731] - ์ด๊ธฐํ ์คํฌ๋ฆฝํธ ์ํฐํฐ ์ถ๊ฐ #733
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
Walkthrough์์ฉํ๋ก๊ทธ๋จ์ ์ด๊ธฐ ์คํฌ๋ฆฝํธ ๋๋ฉ์ธ ๋ชจ๋ธ๊ณผ ํด๋น CRUD ํฌํธ, JPA ์ํฐํฐยท์ ์ฅ์, ์์์ฑ ์ด๋ํฐ ๋ฐ ๋๋ฉ์ธโ์ํฐํฐ ๋งคํผ์ ํ ์คํธ์ฉ DB ์คํค๋ง๋ฅผ ์ถ๊ฐํฉ๋๋ค. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter as PersistenceAdapter
participant Repo as ApplicationInitialScriptRepository
participant DB as Database
Client->>Adapter: save(ApplicationInitialScript)
Adapter->>Adapter: ApplicationInitialScript.toEntity()
Adapter->>Repo: save(ApplicationInitialScriptJpaEntity)
Repo->>DB: INSERT
DB-->>Repo: OK
Repo-->>Adapter: saved entity
Adapter->>Client: saved domain
Client->>Adapter: findAllByApplication(Application)
Adapter->>Repo: findAllByApplication(ApplicationJpaEntity)
Repo->>DB: SELECT
DB-->>Repo: entities[]
Adapter->>Adapter: toDomain() for each
Adapter-->>Client: ApplicationInitialScript[]
Client->>Adapter: deleteByApplication(Application)
Adapter->>Repo: deleteAllByApplication(ApplicationJpaEntity)
Repo->>DB: DELETE
DB-->>Repo: OK
Repo-->>Adapter: result
Adapter-->>Client: ack
Estimated code review effort๐ฏ 4 (Complex) | โฑ๏ธ ~40 ๋ถ ์ฃผ์ ๊ฒํ ๋์:
Poem
Pre-merge checks and finishing touchesโ Passed checks (2 passed)
โจ Finishing touches๐งช Generate unit tests (beta)
๐ Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ๐ Files selected for processing (1)
๐งฐ Additional context used๐ง Learnings (1)๐ Common learningsโฐ 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)
๐ Additional comments (1)
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: 1
๐ Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ Files selected for processing (9)
src/main/kotlin/com/dcd/server/core/domain/application/model/ApplicationInitialScript.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/application/spi/ApplicationInitialScriptPort.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/application/spi/CommandApplicationInitialScriptPort.kt(1 hunks)src/main/kotlin/com/dcd/server/core/domain/application/spi/QueryApplicationInitialScriptPort.kt(1 hunks)src/main/kotlin/com/dcd/server/persistence/application/ApplicationInitialScriptPersistenceAdapter.kt(1 hunks)src/main/kotlin/com/dcd/server/persistence/application/adapter/ApplicationAdapter.kt(2 hunks)src/main/kotlin/com/dcd/server/persistence/application/entity/ApplicationInitialScriptJpaEntity.kt(1 hunks)src/main/kotlin/com/dcd/server/persistence/application/repository/ApplicationInitialScriptRepository.kt(1 hunks)src/test/resources/data.sql(3 hunks)
๐งฐ Additional context used
๐ง Learnings (1)
๐ Common learnings
Learnt from: dolong2
Repo: dolong2/dcd PR: 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.
Learnt from: dolong2
Repo: dolong2/dcd PR: 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.
Learnt from: dolong2
Repo: dolong2/dcd PR: 655
File: src/main/kotlin/com/dcd/server/persistence/workspace/entity/WorkspaceJpaEntity.kt:12-14
Timestamp: 2025-07-12T14:05:23.333Z
Learning: dolong2๋ JPA์ GeneratedValue(strategy = GenerationType.UUID) ๋์ ์์ฑ์์์ UUID.randomUUID()๋ก ์ง์ ์ด๊ธฐํํ๋ ๋ฐฉ์์ ์ ํธํ๋ค. ๋ ๊ฐ๋จํ๊ณ ์์ธก ๊ฐ๋ฅํ ๋์์ ์ํด์๋ค.
Learnt from: dolong2
Repo: dolong2/dcd PR: 659
File: src/main/kotlin/com/dcd/server/core/domain/domain/usecase/ConnectDomainUseCase.kt:40-41
Timestamp: 2025-07-14T15:01:47.531Z
Learning: dolong2๋ ๊ธฐ๋ฅ์ ์ผ๋ก ๋ฌธ์ ๊ฐ ์๋ ์ฝ๋ ๊ฐ์ ์ฌํญ์ ๊ฒฝ์ฐ, ์ค์ ๋ก ํ์ํ ๋๊น์ง ์์ ์ ๋ฏธ๋ฃจ๋ ์ค์ฉ์ ์ธ ์ ๊ทผ๋ฒ์ ์ ํธํ๋ค. ์๋ฅผ ๋ค์ด ์๋ฏธ๋ก ์ ์ ํ์ฑ์ ์ํ ๋ณ๊ฒฝ์ ํด๋น ๊ธฐ๋ฅ์ด ์ค์ ๋ก ํ์ํ ๋ ์์ ํ๋ ๋ฐฉ์์ ํํ๋ค.
๐ช detekt (1.23.8)
src/main/kotlin/com/dcd/server/core/domain/application/spi/ApplicationInitialScriptPort.kt
[warning] 3-4: The class or object ApplicationInitialScriptPort is empty.
(detekt.empty-blocks.EmptyClassBlock)
src/main/kotlin/com/dcd/server/core/domain/application/model/ApplicationInitialScript.kt
[warning] 9-10: The class or object ApplicationInitialScript is empty.
(detekt.empty-blocks.EmptyClassBlock)
src/main/kotlin/com/dcd/server/persistence/application/entity/ApplicationInitialScriptJpaEntity.kt
[warning] 22-23: The class or object ApplicationInitialScriptJpaEntity is empty.
(detekt.empty-blocks.EmptyClassBlock)
โฐ 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 (4)
src/main/kotlin/com/dcd/server/core/domain/application/spi/QueryApplicationInitialScriptPort.kt (1)
7-10: LGTM!์ฟผ๋ฆฌ ํฌํธ ์ธํฐํ์ด์ค๊ฐ ๊น๋ํ๊ฒ ์ ์๋์ด ์์ต๋๋ค. nullable ๋ฐํ ํ์ ๊ณผ ๋ฆฌ์คํธ ๋ฐํ์ด ์ ์ ํ๊ฒ ์ฌ์ฉ๋์์ต๋๋ค.
src/main/kotlin/com/dcd/server/persistence/application/repository/ApplicationInitialScriptRepository.kt (1)
10-10: ํธ๋์ญ์ ์ปจํ ์คํธ ํ์ธ ๊ถ์ฅ
deleteAllByApplication๋ Spring Data JPA์ derived delete ์ฟผ๋ฆฌ์ ๋๋ค. ์ด ๋ฉ์๋๋ฅผ ํธ์ถํ๋ ๊ณณ์์@Transactional์ปจํ ์คํธ ๋ด์์ ์คํ๋๋์ง ํ์ธํ์ธ์. ๋ํ, ํ์ต๋ ์ ํธ์ฌํญ์ ๋ฐ๋ฅด๋ฉด ์ญ์ ์ ๋น์ฆ๋์ค ๋ก์ง์์ ๋ช ์์ ์ธ ์ ํจ์ฑ ๊ฒ์ฆ์ ์ํํ๋ ๊ฒ์ด ์ข์ต๋๋ค.Based on learnings
src/main/kotlin/com/dcd/server/core/domain/application/spi/CommandApplicationInitialScriptPort.kt (1)
6-12: LGTM!์ปค๋งจ๋ ํฌํธ ์ธํฐํ์ด์ค๊ฐ ๋ช ํํ๊ฒ ์ ์๋์ด ์์ต๋๋ค. CRUD ์์ ๊ณผ ์ ํ๋ฆฌ์ผ์ด์ ๊ธฐ๋ฐ ์ญ์ ์์ ์ด ์ ์ ํ๊ฒ ํฌํจ๋์ด ์์ต๋๋ค.
src/main/kotlin/com/dcd/server/persistence/application/adapter/ApplicationAdapter.kt (1)
43-55: ํ์ฌ ์ฝ๋๋ ์ํ ์ฐธ์กฐ ์ํ์ด ์์ต๋๋ค๊ฒ์ฆ ๊ฒฐ๊ณผ, Application ๋๋ฉ์ธ ๋ชจ๋ธ๊ณผ ApplicationJpaEntity ๋ชจ๋ ApplicationInitialScript ์ปฌ๋ ์ ํ๋๋ฅผ ํฌํจํ์ง ์์ต๋๋ค. ApplicationInitialScript๋ Application์ ์ฐธ์กฐํ์ง๋ง Application์ ์ญ์ฐธ์กฐ๋ฅผ ํ์ง ์์ผ๋ฏ๋ก ๋ฌดํ ์ฌ๊ท ์ํ์ด ์์ต๋๋ค. ApplicationInitialScript ๋ชฉ๋ก์ ๋ณ๋์ ํฌํธ(QueryApplicationInitialScriptPort)๋ฅผ ํตํด ๊ด๋ฆฌ๋ฉ๋๋ค.
๊ฐ์
์์ ๋ด์ฉ
์ฒดํฌ๋ฆฌ์คํธ
Summary by CodeRabbit