[WTH-144] comment 도메인 코틀린 마이그레이션#8
Merged
Conversation
📝 Walkthrough개요Java 기반 댓글 시스템을 Kotlin으로 재작성하고, 새로운 쿼리 서비스 기반의 계층적 댓글 트리 구성 방식으로 리팩토링했습니다. 이전 DTO와 예외 처리를 새로운 request/response DTO와 전문화된 예외 타입으로 교체하고, 파일 조회 최적화를 추가했습니다. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant Client as REST Client
participant Controller as PostCommentController
participant UseCase as ManageCommentUseCase
participant Repo as CommentRepository
participant QuerySvc as GetCommentQueryService
participant FileReader as FileReader
Client->>Controller: POST /api/v1/board/{boardId}/comments<br/>(CommentSaveRequest)
Controller->>UseCase: savePostComment(dto, boardId, userId)
UseCase->>Repo: save(comment)
UseCase->>Repo: findById(commentId)
Repo-->>UseCase: Comment
UseCase->>UseCase: saveCommentFiles(fileUrls)
UseCase->>Repo: findAllByPostId(postId)
Repo-->>UseCase: List<Comment>
UseCase->>QuerySvc: toCommentTreeResponses(comments)
QuerySvc->>FileReader: findAll(ownerType, ownerIds)
FileReader-->>QuerySvc: List<File>
QuerySvc->>QuerySvc: mapToCommentResponse (재귀)
QuerySvc-->>UseCase: List<CommentResponse>
UseCase-->>Controller: success
Controller-->>Client: CommonResponse<String>
예상 코드 리뷰 시간🎯 5 (Critical) | ⏱️ ~120분 관련 가능성이 있는 PR
제안된 리뷰어
시
🚥 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: 3
🧹 Nitpick comments (9)
src/main/java/com/weeth/domain/board/domain/entity/Board.java (1)
59-61: 새 로직 추가 시 Kotlin 전환을 우선 고려해 주세요.현재 파일에 신규 메서드를 추가하고 있어, 가능한 범위에서 Kotlin으로의 점진적 마이그레이션을 검토 부탁드립니다.
As per coding guidelines: New code should be written in Kotlin; ~271 existing Java files remain and should be migrated incrementally
🤖 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/domain/entity/Board.java` around lines 59 - 61, The new mutator increaseCommentCount added to Board.java should be implemented in Kotlin per the guideline: create a Kotlin counterpart (e.g., Board.kt) or a Kotlin extension that defines fun Board.increaseCommentCount() { commentCount++ } and wire it to the existing Java usage via JVM interop, then remove or deprecate the Java method increaseCommentCount in the Board class to avoid duplicate logic; ensure field visibility and constructors remain compatible so callers still compile.src/main/java/com/weeth/domain/board/domain/repository/NoticeRepository.java (1)
17-20: 새 메서드 추가는 Kotlin으로 진행하는 방향을 고려해 주세요.점진적 마이그레이션 원칙에 맞춰, 가능하다면 Kotlin으로 옮기는 방향을 검토 부탁드립니다.
As per coding guidelines: New code should be written in Kotlin; ~271 existing Java files remain and should be migrated incrementally
🤖 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/domain/repository/NoticeRepository.java` around lines 17 - 20, The new repository method findByIdWithLock should be added in Kotlin rather than Java to follow the incremental migration rule; move or recreate the method signature and annotations (`@Lock`, `@QueryHints/`@QueryHint, `@Query`, `@Param`) from the Java NoticeRepository into a Kotlin interface in the same package (keeping the interface name NoticeRepository or a Kotlin equivalent), ensure the method signature returns Notice and accepts id: Long, and remove or avoid adding the Java method so new code is Kotlin-native while preserving the exact JPA annotations and behavior.src/main/java/com/weeth/domain/board/domain/entity/Notice.java (1)
21-21: TODO는 이슈로 추적해 주세요.주석만 남기면 누락될 수 있으니 이슈 번호 또는 완료 시점을 명시해 주세요. 필요하면 이슈 등록 도와드리겠습니다.
🤖 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/domain/entity/Notice.java` at line 21, Replace the lone TODO comment "// Todo: OneToMany 매핑 제거" in the Notice class with a tracked issue reference: create a ticket describing "Remove OneToMany mapping for Notice" and update the comment to include the issue ID or target completion date (e.g., "TODO (issue `#123`): remove OneToMany mapping") or remove the comment once resolved; ensure the change appears in the Notice class so future reviewers can find the corresponding issue easily.src/main/java/com/weeth/domain/board/domain/repository/PostRepository.java (1)
21-24: 새 메서드 추가는 Kotlin으로 진행하는 방향을 고려해 주세요.점진적 마이그레이션 원칙에 맞춰, 가능하다면 Kotlin으로 옮기는 방향을 검토 부탁드립니다.
As per coding guidelines: New code should be written in Kotlin; ~271 existing Java files remain and should be migrated incrementally
🤖 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/domain/repository/PostRepository.java` around lines 21 - 24, New Java method addition violates the guideline that new code should be authored in Kotlin; move the new repository method into Kotlin by adding an equivalent declaration in the Kotlin repository interface/class rather than in PostRepository.java. Specifically, remove or revert the Java method findByIdWithLock from PostRepository and declare the same method signature (with `@Lock`(LockModeType.PESSIMISTIC_WRITE), `@QueryHints`(...), and `@Query`("select p from Post p where p.id = :id")) in the Kotlin repository counterpart (or create a Kotlin interface extending the JPA repository and add findByIdWithLock there) so the new code is implemented in Kotlin while preserving the same annotations and behavior.src/test/kotlin/com/weeth/domain/board/application/usecase/PostUseCaseImplTest.kt (1)
271-278: 사용되지 않는 변수가 있습니다.
file변수가 생성되었지만 테스트 assertion에서 사용되지 않습니다. 테스트 의도를 명확히 하거나 불필요한 코드를 제거하는 것을 고려해 주세요.♻️ 사용하지 않는 변수 제거
describe("checkFileExistsByPost") { it("파일이 존재하는 경우 true를 반환한다") { val postId = 1L - val file = - FileTestFixture.createFile( - postId, - "파일1", - storageKey = StorageKey("POST/2026-02/00000000-0000-0000-0000-000000000000_url1"), - ownerType = FileOwnerType.POST, - ownerId = postId, - ) every { fileReader.exists(FileOwnerType.POST, postId, null) } returns true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/board/application/usecase/PostUseCaseImplTest.kt` around lines 271 - 278, The local variable `file` created via FileTestFixture.createFile(postId, "파일1", storageKey = StorageKey(...), ownerType = FileOwnerType.POST, ownerId = postId) is unused; either remove this creation or assert against it to reflect the test intent. Locate the creation in PostUseCaseImplTest (variable name `file`) and either delete the FileTestFixture.createFile call if the test doesn't need a file, or add assertions that reference `file` (e.g., verify its storageKey/ownerType/ownerId or that it appears in returned results) so the fixture is actually validated.src/test/kotlin/com/weeth/domain/comment/application/usecase/query/CommentQueryPerformanceTest.kt (1)
205-209: 불필요한 null 체크 제거
map함수는 항상List를 반환하므로?: emptyList()가 불필요합니다.♻️ 코드 정리 제안
val files = fileRepository .findAll(FileOwnerType.COMMENT, comment.id) .map(fileMapper::toFileResponse) - ?: emptyList()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/comment/application/usecase/query/CommentQueryPerformanceTest.kt` around lines 205 - 209, The null-coalescing fallback is unnecessary: remove the `?: emptyList()` after mapping and let `files` be assigned directly from `fileRepository.findAll(FileOwnerType.COMMENT, comment.id).map(fileMapper::toFileResponse)` because `map` always returns a non-null List; update the `files` declaration in CommentQueryPerformanceTest (the `files` variable and the call sites using `fileRepository.findAll` and `fileMapper::toFileResponse`) to drop the redundant null check.src/main/java/com/weeth/domain/board/application/usecase/PostUseCaseImpl.java (1)
255-257: 메서드 이름이 실제 동작과 불일치합니다
filterParentComments는 필터링이 아닌 댓글 트리 응답 변환을 수행합니다.toCommentTreeResponses또는buildCommentTree와 같이 실제 동작을 반영하는 이름으로 변경하면 가독성이 향상됩니다.♻️ 메서드 이름 개선 제안
- private List<CommentResponse> filterParentComments(List<Comment> comments) { + private List<CommentResponse> toCommentTreeResponses(List<Comment> comments) { return getCommentQueryService.toCommentTreeResponses(comments); }🤖 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/PostUseCaseImpl.java` around lines 255 - 257, The method name filterParentComments in PostUseCaseImpl is misleading because it delegates to getCommentQueryService.toCommentTreeResponses and actually converts comments into a tree response; rename filterParentComments to a name that reflects conversion/building (e.g., toCommentTreeResponses or buildCommentTree) and update all usages/call sites and relevant tests accordingly so the signature and behavior remain unchanged while the name clearly conveys it delegates to getCommentQueryService.toCommentTreeResponses.src/test/kotlin/com/weeth/config/QueryCountUtil.kt (1)
31-55: 병렬 테스트 실행 시 통계 초기화 경쟁 조건 가능성
SessionFactory.statistics는 전역 상태이므로, 여러 테스트가 병렬로 실행될 경우stats.clear()호출이 다른 테스트의 측정에 영향을 줄 수 있습니다. 현재 성능 테스트가@Tag("performance")로 분리되어 있고 기본적으로 비활성화되어 있으므로 당장 문제는 없으나, 향후 병렬 실행이 필요하다면 테스트별 격리를 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/config/QueryCountUtil.kt` around lines 31 - 55, The global SessionFactory.statistics usage in count(...) risks race conditions from stats.clear(); instead capture the pre-run snapshot and compute deltas so you don't mutate global state: read current stats values (e.g., prepareStatementCount, entityLoadCount, collectionFetchCount) and the current isStatisticsEnabled flag, enable statistics if needed, run block(), read post-run values and compute per-test counts as post - pre, then restore isStatisticsEnabled to its original value and return the delta-based Result; reference the count function, sessionFactory.statistics, stats.clear(), and stats.isStatisticsEnabled when making these changes.src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt (1)
25-36: UseCase에 도메인 규칙이 집중되어 있습니다.
deleteComment/파일 교체 등 도메인 규칙은 엔티티나 도메인 서비스로 옮기고, 이 클래스는 오케스트레이션만 담당하도록 분리해 주세요.As per coding guidelines, 'UseCase classes orchestrate only; business logic lives in Entities following Rich Domain Model pattern'.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt` around lines 25 - 36, The ManageCommentUseCase currently contains domain rules (e.g., deleteComment logic and file replacement) that should be moved into the domain layer; create methods on the Comment entity such as deleteBy(User user) and replaceFile(File newFile) or introduce a CommentDomainService with clear domain methods (deleteComment, replaceCommentFile) to encapsulate business rules, and update ManageCommentUseCase to only orchestrate by calling commentRepository.findById(...), invoking the new entity/domain-service methods, and then saving via commentRepository (or fileRepository) as needed; ensure permission checks and side-effecting operations remain in the domain methods and the use case handles transaction boundaries and error handling only.
🤖 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/domain/entity/Board.java`:
- Around line 59-61: The increaseCommentCount() and decreaseCommentCount()
methods can NPE when the Integer commentCount is null (existing DB rows); either
change the commentCount field to primitive int to enforce non-null, or add
null-safe guards in both methods: if commentCount is null set it to 0, then
increment or decrement (ensuring decrement does not go below 0). Update any
`@PrePersist` usage remains for new entities but add these runtime null defenses
in increaseCommentCount()/decreaseCommentCount() and coordinate a DB migration
or default value for commentCount to ensure consistency.
In
`@src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt`:
- Around line 172-194: In deleteComment, the check whether the parent should
also be deleted is performed after removing the child, causing
parent.children.size to drop and skipping cleanup; change the logic in
ManageCommentUseCase.deleteComment to evaluate the parent's child-count
condition before deleting the current comment (or compute an adjusted count such
as parent.children.count { it !== comment && !it.isDeleted } or a
pre-deleteChildrenCount variable), then call deleteCommentFiles and
commentRepository.delete for the parent when that precomputed condition
indicates the parent had only this child, and keep using deleteCommentFiles,
commentRepository.delete(comment) and comment.markAsDeleted as currently named.
In `@src/main/kotlin/com/weeth/domain/comment/domain/entity/Comment.kt`:
- Around line 43-44: The Comment entity currently declares children:
MutableList<Comment> with CascadeType.REMOVE which conflicts with the
soft-delete flow (markAsDeleted()) and ManageCommentUseCase.deleteComment()
calling commentRepository.delete(); remove CascadeType.REMOVE from the children
mapping and instead implement soft-delete propagation: update
Comment.markAsDeleted() to recursively mark child comments as deleted (e.g.,
iterate children and call child.markAsDeleted()), ensure
ManageCommentUseCase.deleteComment() calls comment.markAsDeleted() and saves the
entity via repository (instead of hard delete) for normal deletes, and reserve
commentRepository.delete() only for explicit hard-delete paths after verifying
no children remain or after a separate cleanup operation.
---
Duplicate comments:
In `@src/main/java/com/weeth/domain/board/domain/repository/PostRepository.java`:
- Around line 21-24: PostRepository.findByIdWithLock currently uses `@Lock` and
QueryHints but lacks the same transactional/null-handling safeguards applied in
NoticeRepository; update the repository and call sites so the pessimistic lock
is always acquired inside a transaction and the absence of an entity is handled:
change the repository method to return Optional<Post> (keep
`@Lock`(LockModeType.PESSIMISTIC_WRITE) and the `@Query/`@QueryHints), annotate the
repository method or, preferably, ensure callers invoke it from a
`@Transactional`(propagation = Propagation.REQUIRED) service method, and update
callers to handle Optional.empty() (or map to a clear exception) consistent with
the validation logic used in NoticeRepository.
---
Nitpick comments:
In
`@src/main/java/com/weeth/domain/board/application/usecase/PostUseCaseImpl.java`:
- Around line 255-257: The method name filterParentComments in PostUseCaseImpl
is misleading because it delegates to
getCommentQueryService.toCommentTreeResponses and actually converts comments
into a tree response; rename filterParentComments to a name that reflects
conversion/building (e.g., toCommentTreeResponses or buildCommentTree) and
update all usages/call sites and relevant tests accordingly so the signature and
behavior remain unchanged while the name clearly conveys it delegates to
getCommentQueryService.toCommentTreeResponses.
In `@src/main/java/com/weeth/domain/board/domain/entity/Board.java`:
- Around line 59-61: The new mutator increaseCommentCount added to Board.java
should be implemented in Kotlin per the guideline: create a Kotlin counterpart
(e.g., Board.kt) or a Kotlin extension that defines fun
Board.increaseCommentCount() { commentCount++ } and wire it to the existing Java
usage via JVM interop, then remove or deprecate the Java method
increaseCommentCount in the Board class to avoid duplicate logic; ensure field
visibility and constructors remain compatible so callers still compile.
In `@src/main/java/com/weeth/domain/board/domain/entity/Notice.java`:
- Line 21: Replace the lone TODO comment "// Todo: OneToMany 매핑 제거" in the
Notice class with a tracked issue reference: create a ticket describing "Remove
OneToMany mapping for Notice" and update the comment to include the issue ID or
target completion date (e.g., "TODO (issue `#123`): remove OneToMany mapping") or
remove the comment once resolved; ensure the change appears in the Notice class
so future reviewers can find the corresponding issue easily.
In
`@src/main/java/com/weeth/domain/board/domain/repository/NoticeRepository.java`:
- Around line 17-20: The new repository method findByIdWithLock should be added
in Kotlin rather than Java to follow the incremental migration rule; move or
recreate the method signature and annotations (`@Lock`, `@QueryHints/`@QueryHint,
`@Query`, `@Param`) from the Java NoticeRepository into a Kotlin interface in the
same package (keeping the interface name NoticeRepository or a Kotlin
equivalent), ensure the method signature returns Notice and accepts id: Long,
and remove or avoid adding the Java method so new code is Kotlin-native while
preserving the exact JPA annotations and behavior.
In `@src/main/java/com/weeth/domain/board/domain/repository/PostRepository.java`:
- Around line 21-24: New Java method addition violates the guideline that new
code should be authored in Kotlin; move the new repository method into Kotlin by
adding an equivalent declaration in the Kotlin repository interface/class rather
than in PostRepository.java. Specifically, remove or revert the Java method
findByIdWithLock from PostRepository and declare the same method signature (with
`@Lock`(LockModeType.PESSIMISTIC_WRITE), `@QueryHints`(...), and `@Query`("select p
from Post p where p.id = :id")) in the Kotlin repository counterpart (or create
a Kotlin interface extending the JPA repository and add findByIdWithLock there)
so the new code is implemented in Kotlin while preserving the same annotations
and behavior.
In
`@src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt`:
- Around line 25-36: The ManageCommentUseCase currently contains domain rules
(e.g., deleteComment logic and file replacement) that should be moved into the
domain layer; create methods on the Comment entity such as deleteBy(User user)
and replaceFile(File newFile) or introduce a CommentDomainService with clear
domain methods (deleteComment, replaceCommentFile) to encapsulate business
rules, and update ManageCommentUseCase to only orchestrate by calling
commentRepository.findById(...), invoking the new entity/domain-service methods,
and then saving via commentRepository (or fileRepository) as needed; ensure
permission checks and side-effecting operations remain in the domain methods and
the use case handles transaction boundaries and error handling only.
In `@src/test/kotlin/com/weeth/config/QueryCountUtil.kt`:
- Around line 31-55: The global SessionFactory.statistics usage in count(...)
risks race conditions from stats.clear(); instead capture the pre-run snapshot
and compute deltas so you don't mutate global state: read current stats values
(e.g., prepareStatementCount, entityLoadCount, collectionFetchCount) and the
current isStatisticsEnabled flag, enable statistics if needed, run block(), read
post-run values and compute per-test counts as post - pre, then restore
isStatisticsEnabled to its original value and return the delta-based Result;
reference the count function, sessionFactory.statistics, stats.clear(), and
stats.isStatisticsEnabled when making these changes.
In
`@src/test/kotlin/com/weeth/domain/board/application/usecase/PostUseCaseImplTest.kt`:
- Around line 271-278: The local variable `file` created via
FileTestFixture.createFile(postId, "파일1", storageKey = StorageKey(...),
ownerType = FileOwnerType.POST, ownerId = postId) is unused; either remove this
creation or assert against it to reflect the test intent. Locate the creation in
PostUseCaseImplTest (variable name `file`) and either delete the
FileTestFixture.createFile call if the test doesn't need a file, or add
assertions that reference `file` (e.g., verify its storageKey/ownerType/ownerId
or that it appears in returned results) so the fixture is actually validated.
In
`@src/test/kotlin/com/weeth/domain/comment/application/usecase/query/CommentQueryPerformanceTest.kt`:
- Around line 205-209: The null-coalescing fallback is unnecessary: remove the
`?: emptyList()` after mapping and let `files` be assigned directly from
`fileRepository.findAll(FileOwnerType.COMMENT,
comment.id).map(fileMapper::toFileResponse)` because `map` always returns a
non-null List; update the `files` declaration in CommentQueryPerformanceTest
(the `files` variable and the call sites using `fileRepository.findAll` and
`fileMapper::toFileResponse`) to drop the redundant null check.
Comment on lines
+59
to
+61
| public void increaseCommentCount() { | ||
| commentCount++; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/com/weeth/domain/board/domain/entity/Board.java | head -100Repository: Team-Weeth/weeth-server
Length of output: 3041
🏁 Script executed:
rg "increaseCommentCount" -A 2 -B 2Repository: Team-Weeth/weeth-server
Length of output: 1696
🏁 Script executed:
rg "decreaseCommentCount" -A 2 -B 2Repository: Team-Weeth/weeth-server
Length of output: 1572
commentCount가 null이면 NPE가 발생합니다. decreaseCommentCount()도 동일한 문제가 있습니다.
Integer 타입의 commentCount는 기존 DB에서 null일 수 있으며, @PrePersist는 신규 엔티티에만 초기화되므로 기존 데이터는 보호되지 않습니다. 현재 decreaseCommentCount()의 조건문도 null 체크 없이 commentCount > 0 비교 시 NPE가 발생합니다.
두 메서드 모두에서 안전한 null 처리가 필요하며, 근본적으로는 Integer를 int로 변경하거나 모든 접근점에서 일관된 null 방어 로직을 추가해야 합니다. 또한 DB 기본값/마이그레이션 전략도 확인이 필요합니다.
🛠️ 수정 제안
public void increaseCommentCount() {
+ if (commentCount == null) {
+ commentCount = 0;
+ }
commentCount++;
}
public void decreaseCommentCount() {
- if (commentCount > 0) {
+ if (commentCount != null && commentCount > 0) {
commentCount--;
}
}🤖 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/domain/entity/Board.java` around lines
59 - 61, The increaseCommentCount() and decreaseCommentCount() methods can NPE
when the Integer commentCount is null (existing DB rows); either change the
commentCount field to primitive int to enforce non-null, or add null-safe guards
in both methods: if commentCount is null set it to 0, then increment or
decrement (ensuring decrement does not go below 0). Update any `@PrePersist` usage
remains for new entities but add these runtime null defenses in
increaseCommentCount()/decreaseCommentCount() and coordinate a DB migration or
default value for commentCount to ensure consistency.
Comment on lines
+172
to
+194
| private fun deleteComment(comment: Comment) { | ||
| if (comment.isDeleted) { | ||
| throw CommentAlreadyDeletedException() | ||
| } | ||
|
|
||
| // 자식 댓글이 없는 경우 -> 삭제 | ||
| if (comment.children.isEmpty()) { | ||
| deleteCommentFiles(comment) | ||
| val parent = comment.parent | ||
| commentRepository.delete(comment) | ||
|
|
||
| // 부모 댓글이 삭제된 상태이고 자식 댓글이 1개인 경우 -> 부모 댓글도 삭제 | ||
| if (parent != null && parent.isDeleted && parent.children.size == 1) { | ||
| deleteCommentFiles(parent) | ||
| commentRepository.delete(parent) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| // 자식 댓글이 있는 경우 -> 댓글을 Soft Delete해 서비스에서 "삭제된 댓글"으로 표시 | ||
| deleteCommentFiles(comment) | ||
| comment.markAsDeleted() | ||
| } |
There was a problem hiding this comment.
부모 삭제 조건이 삭제 이후 평가되어 누락될 수 있습니다.
자식 삭제 후 parent.children.size == 1을 검사하면, 삭제 반영 시 0이 되어 부모 정리가 건너뛸 수 있습니다. 삭제 전에 조건을 계산하거나 별도 카운트로 판단하세요.
🛠️ 제안 수정
if (comment.children.isEmpty()) {
deleteCommentFiles(comment)
val parent = comment.parent
+ val shouldDeleteParent = parent?.let { it.isDeleted && it.children.size == 1 } == true
commentRepository.delete(comment)
// 부모 댓글이 삭제된 상태이고 자식 댓글이 1개인 경우 -> 부모 댓글도 삭제
- if (parent != null && parent.isDeleted && parent.children.size == 1) {
- deleteCommentFiles(parent)
- commentRepository.delete(parent)
- }
+ if (shouldDeleteParent) {
+ parent?.let {
+ deleteCommentFiles(it)
+ commentRepository.delete(it)
+ }
+ }
return
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt`
around lines 172 - 194, In deleteComment, the check whether the parent should
also be deleted is performed after removing the child, causing
parent.children.size to drop and skipping cleanup; change the logic in
ManageCommentUseCase.deleteComment to evaluate the parent's child-count
condition before deleting the current comment (or compute an adjusted count such
as parent.children.count { it !== comment && !it.isDeleted } or a
pre-deleteChildrenCount variable), then call deleteCommentFiles and
commentRepository.delete for the parent when that precomputed condition
indicates the parent had only this child, and keep using deleteCommentFiles,
commentRepository.delete(comment) and comment.markAsDeleted as currently named.
There was a problem hiding this comment.
@soo0711 네, 확인했습니다! 해당 이슈는 실제로 발생할 수 있는 버그입니다.
문제 상황
현재 코드의 실행 흐름:
commentRepository.delete(comment)실행 → 자식 댓글 삭제- JPA가 부모의
children컬렉션을 자동으로 업데이트 parent.children.size를 확인 → 이미 0이 됨- 조건
parent.children.size == 1이 거짓이 되어 부모 정리 로직이 실행되지 않음
결과: 삭제된 상태(isDeleted = true)이면서 자식이 없는 부모 댓글이 DB에 남게 됩니다.
해결 방법
삭제 전에 조건을 미리 평가해야 합니다:
if (comment.children.isEmpty()) {
deleteCommentFiles(comment)
val parent = comment.parent
val shouldDeleteParent = parent?.let { it.isDeleted && it.children.size == 1 } == true
commentRepository.delete(comment)
if (shouldDeleteParent) {
parent?.let {
deleteCommentFiles(it)
commentRepository.delete(it)
}
}
return
}검증 추천
다음과 같은 테스트 케이스를 추가하면 이 동작을 확인할 수 있습니다:
"부모가 삭제 상태이고 마지막 남은 자식을 삭제하면 부모도 함께 삭제된다" {
// given: 삭제된 부모 + 자식 1개
val parent = createComment(isDeleted = true)
val child = createComment(parent = parent)
// when: 자식 삭제
deleteComment(child)
// then: 부모도 삭제됨
commentRepository.findById(parent.id) shouldBe null
}🧠 Learnings used
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/usecase/**/*.{java,kt} : Add Transactional on UseCase classes only — Domain Services must not have transaction annotations
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/usecase/**/*.{java,kt} : UseCase classes orchestrate only; business logic lives in Entities following Rich Domain Model pattern
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/**/*.{java,kt} : Application layer contains UseCase (command/query), DTOs, Mappers, Exceptions, and Validators
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/*.{java,kt} : New code should be written in Kotlin; ~271 existing Java files remain and should be migrated incrementally
Contributor
Author
There was a problem hiding this comment.
트랜잭션이 완료되기 전이라 메모리 상에서는 개수가 올바르게 계산되긴 할 것 같아요! 하지만 리뷰대로 안정성이 좀 떨어지긴 하니 미리 계산하고 삭제 로직을 돌리는 게 좋을 것 같아 수정하겠습니다!
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Comment on lines
+43
to
+44
| @OneToMany(mappedBy = "parent", cascade = [CascadeType.REMOVE], fetch = FetchType.LAZY) | ||
| val children: MutableList<Comment> = mutableListOf(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1. Comment 엔티티 파일 먼저 확인
cat src/main/kotlin/com/weeth/domain/comment/domain/entity/Comment.kt
# 2. Comment 관련 delete/remove 작업 검색
echo "=== Searching for delete/remove operations ==="
rg -n "delete|remove" --type=kt --type=java -i src/main/kotlin/com/weeth/domain/comment/ -A3
# 3. Comment repository 확인
echo "=== Comment Repository ==="
find src/main/kotlin/com/weeth/domain/comment -name "*Repository*" -o -name "*Dao*" | xargs cat 2>/dev/null
# 4. 전체 soft delete 패턴 확인
echo "=== Soft delete pattern usage ==="
rg "markAsDeleted|isDeleted" --type=kt -B2 -A2 src/main/kotlin/com/weeth/domain/comment/Repository: Team-Weeth/weeth-server
Length of output: 3671
🏁 Script executed:
# rg file type 문제 해결하고 service/application 레이어에서 delete 사용 검색
echo "=== Searching for delete operations in service/application layer ==="
rg -n "delete|remove" -C3 -i src/main --type=java | grep -i "comment" -A3 -B3
echo "=== Search for CommentService or UseCase ==="
find src/main -name "*CommentService*" -o -name "*CommentUseCase*" -o -name "*CommentCommand*" | head -20
echo "=== Check for any delete calls on Comment ==="
rg "\.delete|\.deleteById|repository\.delete|commentRepository\.delete" -C3 src/main
echo "=== Check if soft delete is enforced (e.g., through queries) ==="
rg "isDeleted" -C2 src/main/kotlin/com/weeth/domain/comment/ --type=kotlin 2>/dev/null || rg "isDeleted" -C2 src/main/kotlin/com/weeth/domain/comment/Repository: Team-Weeth/weeth-server
Length of output: 28783
CascadeType.REMOVE와 soft delete 패턴의 설계 불일치 해결 필요
markAsDeleted() 메서드는 soft delete(논리 삭제)를 수행하지만, ManageCommentUseCase의 deleteComment() 메서드에서 commentRepository.delete()를 호출하여 hard delete(물리 삭제)를 실행합니다. children에 설정된 CascadeType.REMOVE는 부모 댓글이 hard delete될 때 자식 댓글들도 물리적으로 삭제되도록 합니다. 이는 soft delete 패턴과 불일치합니다.
현재 로직은 자식이 없는 댓글만 hard delete되므로 cascade가 즉시 발동되지 않지만, 설계적으로는 두 가지 삭제 전략(soft/hard)이 혼재되어 있습니다. soft delete를 기본 원칙으로 유지하려면 CascadeType.REMOVE 대신 자식 댓글도 soft delete 처리하거나, 명시적으로 자식 댓글을 수동으로 관리해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/comment/domain/entity/Comment.kt` around
lines 43 - 44, The Comment entity currently declares children:
MutableList<Comment> with CascadeType.REMOVE which conflicts with the
soft-delete flow (markAsDeleted()) and ManageCommentUseCase.deleteComment()
calling commentRepository.delete(); remove CascadeType.REMOVE from the children
mapping and instead implement soft-delete propagation: update
Comment.markAsDeleted() to recursively mark child comments as deleted (e.g.,
iterate children and call child.markAsDeleted()), ensure
ManageCommentUseCase.deleteComment() calls comment.markAsDeleted() and saves the
entity via repository (instead of hard delete) for normal deletes, and reserve
commentRepository.delete() only for explicit hard-delete paths after verifying
no children remain or after a separate cleanup operation.
soo0711
approved these changes
Feb 18, 2026
Collaborator
soo0711
left a comment
soo0711
left a comment
There was a problem hiding this comment.
수고하셨습니다!! 작업속도가 짱 빠루시네요 .. 클로드말고 코덱스로 작업하신건가요?!!
아래에 몇가지 피드백 적어뒀습니당 확인 부탁드립니다! 👍
Comment on lines
+83
to
+88
| val post = findPostWithLock(postId) | ||
| val comment = commentRepository.findByIdAndPostId(commentId, postId) ?: throw CommentNotFoundException() | ||
| ensureOwner(comment, userId) | ||
|
|
||
| deleteComment(comment) | ||
| post.decreaseCommentCount() |
Collaborator
There was a problem hiding this comment.
바로 lock을 거는 것 보다 락 점유 시간 최소화하기 위해 삭제할 자격을 확인 한 후 삭제 시점에 lock을 거는 게 어떨까요?
Contributor
Author
There was a problem hiding this comment.
현재 lock을 거는 이유는 comment count의 정합성을 보장하고자하는 의미가 큰데, 그 의도라면 post를 가져오는 부분에서 lock을 거는 것이 맞을 것 같아요!
그러나 리뷰해주신대로 현재 코드에서 굳이 락을 쓰지 않고도 문제 해소 가능할 것 같아서 한 번 성능 테스트 하고 수정하겠습니다!
Contributor
Author
There was a problem hiding this comment.
테스트 결과 구조적 결함이 발견되어 Board 도메인 마이그레이션 + 개선 진행하면서 락 변경도 진행해볼게요!
| import com.weeth.domain.user.domain.entity.enums.Role | ||
| import java.time.LocalDateTime | ||
|
|
||
| data class CommentResponse( |
Collaborator
There was a problem hiding this comment.
response에도 스웨거 어노테이션을 붙이면 가독성이 올라갈 것 같슴니당
4 tasks
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
Comment 도메인 마이그레이션을 진행했습니다. 구조 개선도 함께 진행 했습니다.
📝 Changes
What
Why
How
📸 Screenshots / Logs
💡 Reviewer 참고사항
메서드를 Java 호환용으로 명시적 선언했습니다. Board
마이그레이션 시 제거 예정입니다.
리팩토링(통합 Board 엔티티) 시 변경될 예정이며, // Todo
주석으로 표시해두었습니다.
✅ Checklist
Summary by CodeRabbit
릴리스 노트
새 기능
버그 수정
개선 사항