[WTH-145] board 도메인 마이그레이션#11
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughJava 기반 board/post/notice 도메인(엔티티·DTO·매퍼·레포지토리·서비스·컨트롤러·예외·테스트)을 대량 삭제하고, 대응되는 Kotlin 기반 도메인(엔티티·VO·컨버터·리포지토리·유스케이스·매퍼·컨트롤러·예외·DTO)과 테스트를 새로 추가했으며 인증·리졸버·파일·댓글 경로 및 CI 테스트 설정을 함께 변경했습니다. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant Client as Client
participant Controller as PostController
participant UseCase as ManagePostUseCase
participant BoardRepo as BoardRepository
participant PostRepo as PostRepository
participant FileRepo as FileRepository
participant Mapper as PostMapper
end
Client->>Controller: POST /api/v4/boards/{boardId}/posts (CreatePostRequest)
Controller->>UseCase: save(boardId, request, userId)
UseCase->>BoardRepo: findByIdAndIsDeletedFalse(boardId)
BoardRepo-->>UseCase: Board
UseCase->>UseCase: validate permissions/ownership
UseCase->>PostRepo: save(post)
PostRepo-->>UseCase: Post
UseCase->>FileRepo: saveAll(files)
FileRepo-->>UseCase: Files
UseCase->>Mapper: toSaveResponse(post)
Mapper-->>UseCase: PostSaveResponse
UseCase-->>Controller: PostSaveResponse
Controller-->>Client: CommonResponse<PostSaveResponse>
sequenceDiagram
rect rgba(200,255,200,0.5)
participant Client as Client
participant Controller as BoardController
participant QueryService as GetBoardQueryService
participant BoardRepo as BoardRepository
participant Mapper as BoardMapper
end
Client->>Controller: GET /api/v4/boards
Controller->>QueryService: findBoards(role)
QueryService->>BoardRepo: findAllByIsDeletedFalseOrderByIdAsc()
BoardRepo-->>QueryService: List<Board>
loop per board
QueryService->>QueryService: board.isAccessibleBy(role)
QueryService->>Mapper: toListResponse(board)
Mapper-->>QueryService: BoardListResponse
end
QueryService-->>Controller: List<BoardListResponse>
Controller-->>Client: CommonResponse<List<BoardListResponse>>
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
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 |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (8)
src/main/java/com/weeth/domain/user/application/dto/response/UserResponseDto.java (1)
90-90: TODO는 이슈로 전환하거나 즉시 구현해 주세요.
User 전역 dto 구현이 미완 상태로 남아 있으니, 릴리즈 전 요구사항이면 구현하고, 아니라면 이슈/티켓으로 추적 관리하는 편이 좋습니다.필요하면 전역 DTO 설계안을 제안해 드릴까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/weeth/domain/user/application/dto/response/UserResponseDto.java` at line 90, The TODO note in UserResponseDto indicating "User 전역 dto 구현 (id, 이름, role)" must be resolved: implement the DTO by adding fields (Long id, String name, String role), generate getters, setters (or use lombok `@Getter/`@Builder/@AllArgsConstructor/@NoArgsConstructor), and add a static factory method like of(User user) to map from the domain User entity; if you cannot implement now, replace the inline TODO with a clear comment referencing a created issue/ticket ID and create that issue for tracking. Ensure changes are made inside the UserResponseDto class so the type is ready for serialization and mapping.src/main/kotlin/com/weeth/domain/board/domain/entity/enums/Part.kt (1)
3-9: 약어에 대한 문서화 추가를 고려하세요.enum 상수들(D, BE, FE, PM)이 약어로 정의되어 있어 새로운 개발자에게 명확하지 않을 수 있습니다. KDoc 주석을 추가하여 각 상수가 무엇을 의미하는지 명시하면 가독성과 유지보수성이 향상됩니다.
📝 문서화 추가 제안
+/** + * 팀 파트를 나타내는 enum + */ enum class Part { + /** 디자인 */ D, + /** 백엔드 */ BE, + /** 프론트엔드 */ FE, + /** 기획 */ PM, + /** 전체 */ ALL, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/domain/entity/enums/Part.kt` around lines 3 - 9, The enum Part contains ambiguous abbreviations (D, BE, FE, PM, ALL); add KDoc documentation to the enum and to each enum constant (D, BE, FE, PM, ALL) explaining their full meanings (e.g., D = Designer, BE = Back-end, FE = Front-end, PM = Product/Project Manager) and any domain-specific nuances so new developers understand intent; update the Part enum declaration to include these KDoc comments above the enum and above each constant (use `@property` or per-constant KDoc) without changing the enum names or behavior.src/test/kotlin/com/weeth/domain/board/domain/converter/BoardConfigConverterTest.kt (1)
27-29:convertToDatabaseColumn(null)케이스도 테스트 추가를 권장합니다.null DB 값에 대한 테스트는 있지만, null attribute를 DB 컬럼으로 변환하는 케이스도 검증하면 양방향 null 처리가 완전해집니다.
🧪 추가 테스트 케이스 제안
"null DB 값은 null로 변환한다" { converter.convertToEntityAttribute(null).shouldBeNull() } + + "null attribute는 null JSON으로 변환한다" { + converter.convertToDatabaseColumn(null).shouldBeNull() + }🤖 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/domain/converter/BoardConfigConverterTest.kt` around lines 27 - 29, Add a test in BoardConfigConverterTest that verifies converter.convertToDatabaseColumn(null) returns null to ensure symmetric null handling; locate the existing converter tests (which already call convertToEntityAttribute(null)) and add a new test case asserting convertToDatabaseColumn(null).shouldBeNull() so both directions (convertToEntityAttribute and convertToDatabaseColumn) are covered.src/main/kotlin/com/weeth/global/common/converter/JsonConverter.kt (1)
11-16: 향후 확장성을 위해JavaTimeModule등록을 고려해보세요.현재
BoardConfig에는 날짜 타입이 없지만, 다른 VO에서 이 converter를 확장할 경우LocalDateTime등의 직렬화 문제가 발생할 수 있습니다.♻️ JavaTimeModule 추가 제안
import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule import com.fasterxml.jackson.module.kotlin.KotlinModule import jakarta.persistence.AttributeConverter abstract class JsonConverter<T>( private val typeRef: TypeReference<T>, ) : AttributeConverter<T, String> { companion object { private val objectMapper = ObjectMapper().apply { registerModule(KotlinModule.Builder().build()) + registerModule(JavaTimeModule()) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/global/common/converter/JsonConverter.kt` around lines 11 - 16, Add JavaTimeModule registration to the ObjectMapper used by JsonConverter to prevent future LocalDate/LocalDateTime serialization issues: in the companion object where objectMapper is built (ObjectMapper().apply { registerModule(KotlinModule.Builder().build()) }), also register com.fasterxml.jackson.datatype.jsr310.JavaTimeModule and configure the mapper to disable SerializationFeature.WRITE_DATES_AS_TIMESTAMPS so Java 8 date/time types (e.g., LocalDateTime) serialize/deserialize correctly when this converter is reused.src/main/kotlin/com/weeth/domain/board/application/dto/request/UpdatePostRequest.kt (1)
15-17: Post의 content 필드에 명시적 크기 제한을 추가하거나 설계 의도를 문서화할 것을 권장합니다.
CreatePostRequest와UpdatePostRequest모두content필드에@Size제약이 없는 반면,title에는@Size(max = 200)이 있습니다. Post 엔티티의content가columnDefinition = "TEXT"로 정의되어 무제한 길이를 지원하는 것으로 보아 의도된 설계로 보이지만, 명시적 제한이 필요한 경우@Size제약을 추가하거나, 의도적으로 무제한으로 두는 이유를 코드 주석으로 문서화하면 유지보수에 도움이 됩니다. (CommentUpdateRequest와 달리 Post는 더 큰 콘텐츠를 지원하는 것으로 보입니다.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/application/dto/request/UpdatePostRequest.kt` around lines 15 - 17, The content field in UpdatePostRequest (val content: String) lacks an explicit size constraint or documentation; either add an `@field`:Size(max = N) annotation to enforce a maximum length consistent with business rules (mirror CreatePostRequest/title policy if applicable) or add a clear code comment on UpdatePostRequest (and/or the Post entity) stating that content is intentionally unlimited because the Post entity uses columnDefinition = "TEXT"; update the validation annotations on UpdatePostRequest.content accordingly and ensure any related DTOs (e.g., CreatePostRequest) are aligned with the chosen approach.src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.kt (1)
51-74: 테스트에서도 Fixture 패턴으로 Board 생성 방식 통일이 필요합니다.현재
Board(...)를 직접 생성하고 있어 다른 테스트와 패턴이 달라집니다.fixture/하위의 BoardTestFixture(또는 신규 생성)로 통일하면 재사용성과 유지보수성이 좋아집니다.Based on learnings: "Follow Fixture pattern: create {Entity}TestFixture objects with factory methods in 'fixture/' test directories".
🤖 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/command/ManageBoardUseCaseTest.kt` around lines 51 - 74, Replace direct Board constructions in ManageBoardUseCaseTest with calls to a Board test fixture factory: create or reuse a BoardTestFixture in the fixture/ directory and use its factory methods instead of new Board(...) in tests around update and delete; update the tests that call boardRepository.findByIdAndIsDeletedFalse(1L) and findByIdAndIsDeletedFalse(999L) to return fixture instances (or null) so all Board instances in this test file come from BoardTestFixture, keeping behavior identical but unifying creation across tests.src/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.kt (1)
78-80: Board/Post 생성도 Fixture 패턴으로 통일해 주세요.현재
Board(...),Post(...)를 직접 생성하고 있어 테스트 간 패턴이 섞입니다.fixture/하위의 테스트 픽스처로 통일하면 가독성과 재사용성이 좋아집니다.Based on learnings: "Follow Fixture pattern: create {Entity}TestFixture objects with factory methods in 'fixture/' test directories".
Also applies to: 191-193
🤖 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/query/GetPostQueryServiceTest.kt` around lines 78 - 80, Replace direct instantiation of Board and Post in GetPostQueryServiceTest (currently using Board(...) and Post(...)) with the fixture factories under fixture/ (e.g., createBoardX(...) and createPostX(...) patterns) to match UserTestFixture usage; add or reuse BoardTestFixture.createActiveBoard(1L, BoardType.GENERAL) and PostTestFixture.createPost(id=1L, title="제목", content="내용", user=UserTestFixture.createActiveUser1(1L), board=BoardTestFixture.createActiveBoard(...), commentCount=1) and update the other occurrences around the file (notably the spots referenced at lines 191-193) to use the same fixture methods for consistency and reuse.src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt (1)
107-123: 부모-자식 삭제 판단 로직을 엔티티로 이동하면 책임이 더 명확해집니다.
현재deleteComment에서 부모 삭제 조건/처리를 직접 다루고 있어 도메인 규칙이 usecase에 퍼집니다.Comment에 삭제 규칙을 캡슐화한 메서드(예:deleteAndCollectOrphans)를 두고, usecase는 저장/삭제 호출만 담당하도록 분리해보세요. 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 107 - 123, Move the parent/child deletion decision logic out of ManageCommentUseCase.deleteComment into the Comment entity: add a method on Comment (e.g., deleteAndCollectOrphans()) that throws CommentAlreadyDeletedException if already deleted, marks the comment deleted, and returns the list of Comment instances that should be removed from persistence (the comment itself and any parent orphan when parent.isDeleted && parent.children.size == 1). Then update ManageCommentUseCase.deleteComment to call comment.deleteAndCollectOrphans(), iterate the returned comments to call deleteCommentFiles(...) and commentRepository.delete(...) for each, keeping the use case as an orchestrator and the deletion rule inside the Comment entity.
🤖 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/global/auth/annotation/CurrentUserRole.java`:
- Around line 1-11: Move the new Java annotation CurrentUserRole into Kotlin:
delete the Java file containing public `@interface` CurrentUserRole and add a
Kotlin file named CurrentUserRole.kt in the same package
(com.weeth.global.auth.annotation) under src/main/kotlin; implement it as a
Kotlin annotation with `@Target`(AnnotationTarget.VALUE_PARAMETER) and
`@Retention`(AnnotationRetention.RUNTIME) so it preserves the original behavior.
Ensure package declaration matches com.weeth.global.auth.annotation and remove
the old Java file to avoid duplicate symbols.
In `@src/main/java/com/weeth/global/auth/model/AuthenticatedUser.java`:
- Around line 1-9: Replace the Java record AuthenticatedUser with a Kotlin data
class: remove the Java record declaration (AuthenticatedUser record with fields
Long id, String email, Role role) and create a Kotlin file defining data class
AuthenticatedUser(val id: Long, val email: String, val role: Role) using the
existing Role type; ensure the new Kotlin file is placed under the Kotlin source
set (src/main/kotlin) and update any imports/usages to reference the Kotlin
class instead of the Java record.
In
`@src/main/java/com/weeth/global/auth/resolver/CurrentUserRoleArgumentResolver.java`:
- Around line 39-43: In CurrentUserRoleArgumentResolver, wrap the
Role.valueOf(role.substring("ROLE_".length())) call inside a try-catch for
IllegalArgumentException (when iterating authentication.getAuthorities() /
GrantedAuthority) and handle invalid role strings by logging a warning/debug and
continuing to the next authority or returning a safe default instead of letting
the exception propagate; ensure you reference Role.valueOf and the loop over
authentication.getAuthorities() so the invalid enum name is caught and does not
cause a 500.
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt`:
- Around line 104-110: checkWritePermission currently only checks
board.isAdminOnly and user.role, missing private-board access checks; update the
function to first call board.isAccessibleBy(user.role) and throw
CategoryAccessDeniedException if it returns false, then retain the existing
admin-only check (board.isAdminOnly && user.role != Role.ADMIN) to enforce admin
restrictions; reference the Board.isAccessibleBy(role: Role) method and Role
enum when making the change so read/write access rules stay consistent with
GetBoardQueryService/GetPostQueryService.
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryService.kt`:
- Around line 40-47: findPost currently only ensures the post itself isn't
deleted but doesn't block access when its parent Board is soft-deleted; update
the findPost flow (inside the findPost function, after retrieving post via
postRepository.findByIdAndIsDeletedFalse) to also check the board's deleted flag
(e.g. post.board.isDeleted or post.board.deleted == true) and treat it as
inaccessible by throwing PostNotFoundException, or call the same visibility
helper used elsewhere (validateBoardVisibility) before using
post.board.isAccessibleBy(role).
In `@src/main/kotlin/com/weeth/domain/board/domain/entity/Post.kt`:
- Around line 58-65: Post.updateContent allows empty content and breaks the
invariant enforced at creation; add a validation check in the updateContent
method to require newContent.isNotBlank() (similar to the constructor/creation
validation) with an appropriate error message before assigning content, so
updateContent enforces the same non-blank constraint on content as creation.
In `@src/test/kotlin/com/weeth/domain/board/domain/entity/PostEntityTest.kt`:
- Around line 26-37: The test for Post.update is missing an assertion for the
content field: after creating the post via PostTestFixture.create() and calling
post.update(newTitle = "변경", newContent = "변경 내용", newCardinalNumber = 7), add
an assertion that post.content equals "변경 내용" alongside the existing checks for
post.title and post.cardinalNumber so the update behavior for the content field
is verified.
In
`@src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt`:
- Around line 34-45: The CommentConcurrencyTest currently injects real ports
(PostCommentUsecase, BoardRepository, PostRepository, UserRepository,
CommentRepository, EntityManager, AtomicCommentCountCommand) causing an
integration test instead of a UseCase unit test; either replace the
repository/entity manager dependencies with MockK mocks and inject them into the
tested UseCase (mock BoardRepository, PostRepository, UserRepository,
CommentRepository, and EntityManager interactions used by
PostCommentUsecase/AtomicCommentCountCommand) so the test follows the
application-layer guideline, or move CommentConcurrencyTest into a clearly named
integration/performance test package and keep the live repositories (and adjust
`@Tag/`@Import if needed) so it is explicitly an integration/performance test.
- Around line 300-307: The Thread.sleep call in CommentConcurrencyTest (around
variables lastError, attempt, maxRetries) does not restore the thread interrupt
flag; wrap the sleep in a try/catch for InterruptedException, and in the catch
call Thread.currentThread().interrupt() to restore the flag and rethrow the
InterruptedException (or propagate it) so the retry loop aborts correctly
instead of swallowing the interrupt.
- Around line 100-132: The test's runConcurrentSave uses latch.await() with no
timeout and calls executor.shutdown() which can hang tests and leak threads;
update runConcurrentSave to call latch.await with a reasonable timeout (e.g.,
seconds) and check its boolean result, and then properly shut down the executor
by calling executor.shutdownNow()/awaitTermination with a timeout and fail the
test (or throw) if the latch timed out or the executor didn't terminate;
reference latch.await, executor.shutdown, and the runConcurrentSave function to
locate and implement the timeout-and-shutdown flow.
---
Nitpick comments:
In
`@src/main/java/com/weeth/domain/user/application/dto/response/UserResponseDto.java`:
- Line 90: The TODO note in UserResponseDto indicating "User 전역 dto 구현 (id, 이름,
role)" must be resolved: implement the DTO by adding fields (Long id, String
name, String role), generate getters, setters (or use lombok
`@Getter/`@Builder/@AllArgsConstructor/@NoArgsConstructor), and add a static
factory method like of(User user) to map from the domain User entity; if you
cannot implement now, replace the inline TODO with a clear comment referencing a
created issue/ticket ID and create that issue for tracking. Ensure changes are
made inside the UserResponseDto class so the type is ready for serialization and
mapping.
In
`@src/main/kotlin/com/weeth/domain/board/application/dto/request/UpdatePostRequest.kt`:
- Around line 15-17: The content field in UpdatePostRequest (val content:
String) lacks an explicit size constraint or documentation; either add an
`@field`:Size(max = N) annotation to enforce a maximum length consistent with
business rules (mirror CreatePostRequest/title policy if applicable) or add a
clear code comment on UpdatePostRequest (and/or the Post entity) stating that
content is intentionally unlimited because the Post entity uses columnDefinition
= "TEXT"; update the validation annotations on UpdatePostRequest.content
accordingly and ensure any related DTOs (e.g., CreatePostRequest) are aligned
with the chosen approach.
In `@src/main/kotlin/com/weeth/domain/board/domain/entity/enums/Part.kt`:
- Around line 3-9: The enum Part contains ambiguous abbreviations (D, BE, FE,
PM, ALL); add KDoc documentation to the enum and to each enum constant (D, BE,
FE, PM, ALL) explaining their full meanings (e.g., D = Designer, BE = Back-end,
FE = Front-end, PM = Product/Project Manager) and any domain-specific nuances so
new developers understand intent; update the Part enum declaration to include
these KDoc comments above the enum and above each constant (use `@property` or
per-constant KDoc) without changing the enum names or behavior.
In
`@src/main/kotlin/com/weeth/domain/comment/application/usecase/command/ManageCommentUseCase.kt`:
- Around line 107-123: Move the parent/child deletion decision logic out of
ManageCommentUseCase.deleteComment into the Comment entity: add a method on
Comment (e.g., deleteAndCollectOrphans()) that throws
CommentAlreadyDeletedException if already deleted, marks the comment deleted,
and returns the list of Comment instances that should be removed from
persistence (the comment itself and any parent orphan when parent.isDeleted &&
parent.children.size == 1). Then update ManageCommentUseCase.deleteComment to
call comment.deleteAndCollectOrphans(), iterate the returned comments to call
deleteCommentFiles(...) and commentRepository.delete(...) for each, keeping the
use case as an orchestrator and the deletion rule inside the Comment entity.
In `@src/main/kotlin/com/weeth/global/common/converter/JsonConverter.kt`:
- Around line 11-16: Add JavaTimeModule registration to the ObjectMapper used by
JsonConverter to prevent future LocalDate/LocalDateTime serialization issues: in
the companion object where objectMapper is built (ObjectMapper().apply {
registerModule(KotlinModule.Builder().build()) }), also register
com.fasterxml.jackson.datatype.jsr310.JavaTimeModule and configure the mapper to
disable SerializationFeature.WRITE_DATES_AS_TIMESTAMPS so Java 8 date/time types
(e.g., LocalDateTime) serialize/deserialize correctly when this converter is
reused.
In
`@src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCaseTest.kt`:
- Around line 51-74: Replace direct Board constructions in
ManageBoardUseCaseTest with calls to a Board test fixture factory: create or
reuse a BoardTestFixture in the fixture/ directory and use its factory methods
instead of new Board(...) in tests around update and delete; update the tests
that call boardRepository.findByIdAndIsDeletedFalse(1L) and
findByIdAndIsDeletedFalse(999L) to return fixture instances (or null) so all
Board instances in this test file come from BoardTestFixture, keeping behavior
identical but unifying creation across tests.
In
`@src/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.kt`:
- Around line 78-80: Replace direct instantiation of Board and Post in
GetPostQueryServiceTest (currently using Board(...) and Post(...)) with the
fixture factories under fixture/ (e.g., createBoardX(...) and createPostX(...)
patterns) to match UserTestFixture usage; add or reuse
BoardTestFixture.createActiveBoard(1L, BoardType.GENERAL) and
PostTestFixture.createPost(id=1L, title="제목", content="내용",
user=UserTestFixture.createActiveUser1(1L),
board=BoardTestFixture.createActiveBoard(...), commentCount=1) and update the
other occurrences around the file (notably the spots referenced at lines
191-193) to use the same fixture methods for consistency and reuse.
In
`@src/test/kotlin/com/weeth/domain/board/domain/converter/BoardConfigConverterTest.kt`:
- Around line 27-29: Add a test in BoardConfigConverterTest that verifies
converter.convertToDatabaseColumn(null) returns null to ensure symmetric null
handling; locate the existing converter tests (which already call
convertToEntityAttribute(null)) and add a new test case asserting
convertToDatabaseColumn(null).shouldBeNull() so both directions
(convertToEntityAttribute and convertToDatabaseColumn) are covered.
| for (GrantedAuthority authority : authentication.getAuthorities()) { | ||
| String role = authority.getAuthority(); | ||
| if (role != null && role.startsWith("ROLE_")) { | ||
| return Role.valueOf(role.substring("ROLE_".length())); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Role enum 정의 찾기
echo "=== Role enum 정의 ==="
rg -n "enum Role" -A 10 -g'*.java' -g'*.kt'Repository: Team-Weeth/weeth-server
Length of output: 402
🏁 Script executed:
#!/bin/bash
# ROLE_ 권한 문자열 사용처 확인
echo "=== ROLE_ 권한 문자열 사용처 ==="
rg -n "ROLE_" -g'*.java' -g'*.kt' | head -30Repository: Team-Weeth/weeth-server
Length of output: 1782
🏁 Script executed:
#!/bin/bash
# GrantedAuthority 부여 경로 및 소스 확인
echo "=== GrantedAuthority 부여 경로 ==="
rg -n "new SimpleGrantedAuthority|grantedAuthorities|authorities\.add|setAuthorities" -B2 -A2 -g'*.java' -g'*.kt' | head -50Repository: Team-Weeth/weeth-server
Length of output: 1226
Role.valueOf() 호출 시 IllegalArgumentException 예외 처리 필요
JWT 클레임이나 예상치 못한 권한 문자열이 Role enum에 없을 경우, Role.valueOf()는 IllegalArgumentException을 던져 500 에러가 발생합니다. 토큰 변조, 버전 불일치, 또는 권한 확장 시나리오에서 안전한 처리를 권장합니다.
제안 예시
for (GrantedAuthority authority : authentication.getAuthorities()) {
String role = authority.getAuthority();
if (role != null && role.startsWith("ROLE_")) {
- return Role.valueOf(role.substring("ROLE_".length()));
+ try {
+ return Role.valueOf(role.substring("ROLE_".length()));
+ } catch (IllegalArgumentException ignored) {
+ // 기본값으로 계속 진행
+ }
}
}📝 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.
| for (GrantedAuthority authority : authentication.getAuthorities()) { | |
| String role = authority.getAuthority(); | |
| if (role != null && role.startsWith("ROLE_")) { | |
| return Role.valueOf(role.substring("ROLE_".length())); | |
| } | |
| for (GrantedAuthority authority : authentication.getAuthorities()) { | |
| String role = authority.getAuthority(); | |
| if (role != null && role.startsWith("ROLE_")) { | |
| try { | |
| return Role.valueOf(role.substring("ROLE_".length())); | |
| } catch (IllegalArgumentException ignored) { | |
| // 기본값으로 계속 진행 | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/weeth/global/auth/resolver/CurrentUserRoleArgumentResolver.java`
around lines 39 - 43, In CurrentUserRoleArgumentResolver, wrap the
Role.valueOf(role.substring("ROLE_".length())) call inside a try-catch for
IllegalArgumentException (when iterating authentication.getAuthorities() /
GrantedAuthority) and handle invalid role strings by logging a warning/debug and
continuing to the next authority or returning a safe default instead of letting
the exception propagate; ensure you reference Role.valueOf and the loop over
authentication.getAuthorities() so the invalid enum name is caught and does not
cause a 500.
src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt
Show resolved
Hide resolved
src/main/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryService.kt
Show resolved
Hide resolved
src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt
Show resolved
Hide resolved
src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt
Show resolved
Hide resolved
src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt
Show resolved
Hide resolved
soo0711
left a comment
There was a problem hiding this comment.
수고하셨습니다! 기획에 맞게 잘 수정된 것 같습니다!!
아래에 피드백 남겨뒀습니다! 확인 부탁 드립니다 👍
| private val entityManager: EntityManager, | ||
| private val atomicCommentCountCommand: AtomicCommentCountCommand, | ||
| ) : DescribeSpec({ | ||
| val runPerformanceTests = System.getProperty("runPerformanceTests")?.toBoolean() ?: false |
There was a problem hiding this comment.
성능 테스트라면 @Tag("performance")로 분리해도 될 것 같은데 Gradle 실행 옵션을 사용하신 특별한 이유가 있을까욤??
There was a problem hiding this comment.
특정 케이스만 분리하기 위함이라고 하네욤.. 제거하겠습니당ㅋㅋ
| package com.weeth.domain.board.domain.entity.enums | ||
|
|
There was a problem hiding this comment.
아키텍처를 설계 보면 domain내에 enums 패키지 경로를 두도록 설정이 되어있어서 해당 경로로 이동하는 것이 구조상 더 적절해 보입니당
│ ├── domain/
│ │ ├── entity/ # Rich Domain Model
│ │ ├── vo/ # Value Objects
│ │ ├── enums/
│ │ ├── port/ # External system abstraction (Port interface)
│ │ ├── service/ # Multi-entity business logic only
│ │ └── repository/
There was a problem hiding this comment.
아 이렇게 잡아둿었네요.. 하나도 이렇게 안 했는데 우짜지
| it("files가 있으면 기존 파일을 soft delete 후 교체한다") { | ||
| val user = UserTestFixture.createActiveUser1(1L) |
There was a problem hiding this comment.
oldFile.status.name shouldBe "DELETED"
verify(exactly = 1) { fileRepository.saveAll(newFiles) }
삭제와 저장은 검증되었지만 제목과 내용이 실제로 교체되었는지에 대한 검증도 추가되면 좋을 것 같습니다!
| File.createUploaded( | ||
| fileName = "new.png", | ||
| storageKey = "POST/2026-02/550e8400-e29b-41d4-a716-446655440001_new.png", | ||
| fileSize = 10, | ||
| contentType = "image/png", | ||
| ownerType = FileOwnerType.POST, | ||
| ownerId = 1L, | ||
| ), |
There was a problem hiding this comment.
여러 테스트 케이스에서 File 생성 코드가 반복되고 있어서 테스트 클래스 내부에 Helper 함수를 두어 중복을 줄이면 가독성이 개선될 것 같습니당
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/test/kotlin/com/weeth/domain/board/domain/entity/PostEntityTest.kt (1)
18-24: 선택적 개선: 정상적인 감소 케이스 테스트 추가 고려현재
decreaseCommentCount와decreaseLikeCount는 0일 때 예외 케이스만 테스트합니다. 증가 후 감소하는 정상 케이스도 추가하면 더 완전한 커버리지를 확보할 수 있습니다.♻️ 정상 감소 테스트 추가 예시
"decreaseCommentCount는 댓글 수를 1 감소시킨다" { val post = PostTestFixture.create() post.increaseCommentCount() post.decreaseCommentCount() post.commentCount shouldBe 0 } "decreaseLikeCount는 좋아요 수를 1 감소시킨다" { val post = PostTestFixture.create() post.increaseLikeCount() post.decreaseLikeCount() post.likeCount shouldBe 0 }🤖 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/domain/entity/PostEntityTest.kt` around lines 18 - 24, Add tests that verify normal decrement behavior by creating a Post via PostTestFixture.create(), calling increaseCommentCount() (or increaseLikeCount()) first, then calling decreaseCommentCount() (or decreaseLikeCount()), and asserting the post.commentCount (or post.likeCount) equals 0; update PostEntityTest.kt to include two new cases referencing increaseCommentCount, decreaseCommentCount, commentCount and increaseLikeCount, decreaseLikeCount, likeCount so both the exceptional (already present) and normal decrement flows are covered.src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.kt (1)
36-55: PATCH 규칙 TODO는 실제 구현으로 옮기는 게 좋겠습니다.요청값이 현재 값과 동일해도 업데이트가 발생할 수 있어 불필요한 dirty checking/이벤트가 생길 수 있습니다. TODO를 구현해 실제 변경 시에만 rename/updateConfig가 호출되도록 정리하는 편이 안전합니다.
✅ 변경 비교 기반 PATCH 예시
- request.name?.let { board.rename(it) } + request.name?.takeIf { it != board.name }?.let { board.rename(it) } - if (request.commentEnabled != null || request.writePermission != null || request.isPrivate != null) { - board.updateConfig( - board.config.copy( - commentEnabled = request.commentEnabled ?: board.config.commentEnabled, - writePermission = request.writePermission ?: board.config.writePermission, - isPrivate = request.isPrivate ?: board.config.isPrivate, - ), - ) - } + if (request.commentEnabled != null || request.writePermission != null || request.isPrivate != null) { + val newConfig = + board.config.copy( + commentEnabled = request.commentEnabled ?: board.config.commentEnabled, + writePermission = request.writePermission ?: board.config.writePermission, + isPrivate = request.isPrivate ?: board.config.isPrivate, + ) + if (newConfig != board.config) { + board.updateConfig(newConfig) + } + }원하시면 패치 규칙을 기준으로 테스트까지 묶어서 제안드릴게요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.kt` around lines 36 - 55, The update() method currently calls board.rename(...) and board.updateConfig(...) regardless of whether incoming fields differ, causing unnecessary dirty checks; change it to compare each nullable field in UpdateBoardRequest against the current values from findBoard() and only call board.rename(newName) if request.name is non-null and != board.name, and for config build a newConfig only when any of request.commentEnabled, request.writePermission, request.isPrivate are non-null and different from board.config.*; then call board.updateConfig(newConfig) only if newConfig differs from board.config (avoid calling board.config.copy(...) unconditionally). Use request equality checks and null-coalescing logic to detect real changes before invoking board.rename or board.updateConfig.
🤖 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/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt`:
- Around line 38-45: The test is wrongly mocking FileMapper and PostMapper;
remove those mocks in ManagePostUseCaseTest and inject the real mapper
implementations instead, because the UseCase calls fileMapper.toFileList and
postMapper.toSaveResponse; keep mocks only for PostRepository, BoardRepository,
UserGetService, FileRepository and FileReader. Replace mockk<FileMapper>() and
mockk<PostMapper>() with instances of the concrete mapper classes (e.g.,
FileMapperImpl, PostMapperImpl or the actual implementations in
application/mapper) and pass those instances into the UseCase setup so the real
mapping logic runs during the test.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManageBoardUseCase.kt`:
- Around line 36-55: The update() method currently calls board.rename(...) and
board.updateConfig(...) regardless of whether incoming fields differ, causing
unnecessary dirty checks; change it to compare each nullable field in
UpdateBoardRequest against the current values from findBoard() and only call
board.rename(newName) if request.name is non-null and != board.name, and for
config build a newConfig only when any of request.commentEnabled,
request.writePermission, request.isPrivate are non-null and different from
board.config.*; then call board.updateConfig(newConfig) only if newConfig
differs from board.config (avoid calling board.config.copy(...)
unconditionally). Use request equality checks and null-coalescing logic to
detect real changes before invoking board.rename or board.updateConfig.
In `@src/test/kotlin/com/weeth/domain/board/domain/entity/PostEntityTest.kt`:
- Around line 18-24: Add tests that verify normal decrement behavior by creating
a Post via PostTestFixture.create(), calling increaseCommentCount() (or
increaseLikeCount()) first, then calling decreaseCommentCount() (or
decreaseLikeCount()), and asserting the post.commentCount (or post.likeCount)
equals 0; update PostEntityTest.kt to include two new cases referencing
increaseCommentCount, decreaseCommentCount, commentCount and increaseLikeCount,
decreaseLikeCount, likeCount so both the exceptional (already present) and
normal decrement flows are covered.
| val postRepository = mockk<PostRepository>() | ||
| val boardRepository = mockk<BoardRepository>() | ||
| val userGetService = mockk<UserGetService>() | ||
| val fileRepository = mockk<FileRepository>() | ||
| val fileReader = mockk<FileReader>() | ||
| val fileMapper = mockk<FileMapper>() | ||
| val postMapper = mockk<PostMapper>() | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find FileMapper and PostMapper definitions and their locations
echo "=== FileMapper definitions ==="
rg -n "interface FileMapper|class FileMapper|object FileMapper" src/main/kotlin --type kotlin
echo "=== PostMapper definitions ==="
rg -n "interface PostMapper|class PostMapper|object PostMapper" src/main/kotlin --type kotlin
echo "=== Checking application mapper structure ==="
fd "FileMapper|PostMapper" src/main/kotlin --type fRepository: Team-Weeth/weeth-server
Length of output: 503
🏁 Script executed:
#!/bin/bash
# Check ManagePostUseCase constructor and mapper usage
echo "=== ManagePostUseCase class definition ==="
rg -A 15 "class ManagePostUseCase" src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt
echo "=== FileMapper usage in UseCase ==="
rg -n "fileMapper|FileMapper" src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt
echo "=== PostMapper usage in UseCase ==="
rg -n "postMapper|PostMapper" src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.ktRepository: Team-Weeth/weeth-server
Length of output: 1151
UseCase 테스트에서 FileMapper와 PostMapper 모킹 제거 필요
가이드라인에 따르면 UseCase 테스트는 Repository/Reader/Port만 목킹해야 합니다. FileMapper와 PostMapper는 application/mapper 계층의 Component Mapper(어댑터)이며 포트가 아닙니다. 두 매퍼는 UseCase에서 실제로 호출되므로(fileMapper.toFileList 라인 129, postMapper.toSaveResponse 라인 57, 77) 실제 구현을 주입하여 사용하도록 수정하세요. 목킹 대상은 PostRepository, BoardRepository, UserGetService, FileRepository, FileReader만 유지하세요.
🤖 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/command/ManagePostUseCaseTest.kt`
around lines 38 - 45, The test is wrongly mocking FileMapper and PostMapper;
remove those mocks in ManagePostUseCaseTest and inject the real mapper
implementations instead, because the UseCase calls fileMapper.toFileList and
postMapper.toSaveResponse; keep mocks only for PostRepository, BoardRepository,
UserGetService, FileRepository and FileReader. Replace mockk<FileMapper>() and
mockk<PostMapper>() with instances of the concrete mapper classes (e.g.,
FileMapperImpl, PostMapperImpl or the actual implementations in
application/mapper) and pass those instances into the UseCase setup so the real
mapping logic runs during the test.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt (2)
254-256: 벤치마크 결과에 대한 검증 부재현재 성능 비교 결과를 출력만 하고 assertion이 없습니다. 성능 테스트 특성상 환경에 따라 결과가 달라질 수 있어 의도적일 수 있으나, PR 설명에서 "비관락 선택" 결정을 내렸다면 해당 전략이 일관되게 우위인지 최소한의 검증을 추가하는 것을 고려해 보세요.
// 선택적: 일관된 성능 검증이 필요하다면 // pessimisticSummary.medianElapsedMs shouldBeLessThan atomicSummary.medianElapsedMs * 2🤖 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/command/CommentConcurrencyTest.kt` around lines 254 - 256, The test currently only prints a winner but lacks assertions; add a deterministic performance assertion in CommentConcurrencyTest to validate the chosen strategy by comparing atomicSummary.medianElapsedMs and pessimisticSummary.medianElapsedMs (or using the winner variable) so the PR's claim is enforced—e.g., assert that pessimisticSummary.medianElapsedMs is consistently less than atomicSummary.medianElapsedMs or within an acceptable factor (use your test framework's assertion API to compare medianElapsedMs values and fail the test if the expectation is not met).
300-301: 데드락 감지 로직의 취약성 고려에러 메시지 문자열 매칭(
"Deadlock found","Lock wait timeout exceeded")은 MySQL 드라이버 버전이나 DB 설정에 따라 메시지 포맷이 달라질 수 있어 취약합니다. 테스트 코드이므로 큰 문제는 아니지만,SQLException의sqlState나errorCode를 활용하면 더 안정적입니다.// 예시: MySQL 데드락 에러 코드 활용 val cause = generateSequence(e as Throwable?) { it.cause } .filterIsInstance<java.sql.SQLException>() .firstOrNull() val isDeadlock = cause?.errorCode == 1213 // ER_LOCK_DEADLOCK val isLockTimeout = cause?.errorCode == 1205 // ER_LOCK_WAIT_TIMEOUT🤖 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/command/CommentConcurrencyTest.kt` around lines 300 - 301, Replace fragile string matching on e.message in CommentConcurrencyTest (where variables deadlock and lockWaitTimeout are set) with robust detection that walks the exception cause chain to locate a java.sql.SQLException and checks its errorCode (1213 for deadlock, 1205 for lock wait timeout) or sqlState instead; update the logic that currently checks e.message?.contains("Deadlock found", ...) and e.message?.contains("Lock wait timeout exceeded", ...) to use a generateSequence over e to find the first java.sql.SQLException and then compare cause.errorCode or cause.sqlState to determine isDeadlock/isLockTimeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt`:
- Around line 129-131: The test's CountDownLatch.await() call in
CommentConcurrencyTest can block indefinitely; change it to use
latch.await(timeout, TimeUnit.SECONDS) with a reasonable timeout and check the
boolean result, and after calling executor.shutdown() call
executor.awaitTermination(timeout, TimeUnit.SECONDS) (and optionally
executor.shutdownNow() if it times out) to ensure threads are cleaned up; update
logic to fail the test or throw if the latch wait or termination returns false
so the test doesn't hang silently.
- Around line 298-307: The catch block using Thread.sleep(backoffMs) in
CommentConcurrencyTest.kt does not handle InterruptedException and thus can
clear the thread's interrupt flag; update the sleep call in the retry loop
(where variables attempt, maxRetries, lastError, backoffMs are used) to catch
InterruptedException, restore the interrupt status with
Thread.currentThread().interrupt(), and then either rethrow the exception (or
wrap/throw a RuntimeException) so the test stops promptly; ensure the
InterruptedException path does not swallow the original exception information in
lastError.
---
Nitpick comments:
In
`@src/test/kotlin/com/weeth/domain/comment/application/usecase/command/CommentConcurrencyTest.kt`:
- Around line 254-256: The test currently only prints a winner but lacks
assertions; add a deterministic performance assertion in CommentConcurrencyTest
to validate the chosen strategy by comparing atomicSummary.medianElapsedMs and
pessimisticSummary.medianElapsedMs (or using the winner variable) so the PR's
claim is enforced—e.g., assert that pessimisticSummary.medianElapsedMs is
consistently less than atomicSummary.medianElapsedMs or within an acceptable
factor (use your test framework's assertion API to compare medianElapsedMs
values and fail the test if the expectation is not met).
- Around line 300-301: Replace fragile string matching on e.message in
CommentConcurrencyTest (where variables deadlock and lockWaitTimeout are set)
with robust detection that walks the exception cause chain to locate a
java.sql.SQLException and checks its errorCode (1213 for deadlock, 1205 for lock
wait timeout) or sqlState instead; update the logic that currently checks
e.message?.contains("Deadlock found", ...) and e.message?.contains("Lock wait
timeout exceeded", ...) to use a generateSequence over e to find the first
java.sql.SQLException and then compare cause.errorCode or cause.sqlState to
determine isDeadlock/isLockTimeout.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/board/application/usecase/command/ManagePostUseCase.kt`:
- Around line 59-77: The update method in ManagePostUseCase currently
unconditionally overwrites fields via post.update(newTitle = request.title,
newContent = request.content, newCardinalNumber = request.cardinalNumber) and
cannot distinguish "field omitted" vs "explicit null" for cardinalNumber; change
UpdatePostRequest to use optional semantics (e.g., Optional-wrapped fields,
nullable boxed types plus presence flags, or a separate Patch DTO) so callers
can indicate which fields to patch, then modify ManagePostUseCase.update to
apply updates conditionally (only call post.update or pass values when
title/content/cardinalNumber are explicitly provided) and preserve existing
values otherwise, ensuring validateOwner, findPost and replacePostFiles behavior
remains unchanged.
- Around line 25-36: 현재 ManagePostUseCase 클래스는 메서드별로 `@Transactional이` 적용되어 있는데,
가이드라인에 따라 클래스 레벨에 단일 `@Transactional을` 선언하고 각 메서드의 `@Transactional` 애노테이션을 제거해야 합니다;
ManagePostUseCase 클래스 선언부에 `@Transactional을` 추가하고 save(), update(), delete() 등
UseCase 내부 메서드에서 개별 `@Transactional` 어노테이션을 제거하여 트랜잭션 설정을 중앙화하고 중복을 없애세요 (참고:
update()의 TODO는 현 상태에서 데이터 손실 위험이 없으므로 그대로 두되, 향후 PATCH 의미론을 구현할 때 선택적 필드 업데이트
로직을 적용하세요).
| @Service | ||
| class ManagePostUseCase( | ||
| private val postRepository: PostRepository, | ||
| private val boardRepository: BoardRepository, // 동일 도메인 | ||
| private val userGetService: UserGetService, | ||
| private val fileRepository: FileRepository, | ||
| private val fileReader: FileReader, | ||
| private val fileMapper: FileMapper, | ||
| private val postMapper: PostMapper, | ||
| ) { | ||
| @Transactional | ||
| fun save( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# 1. ManagePostUseCase.kt dosyasını okuyarak `@Transactional` yerleşimini doğrula
cat -n src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.ktRepository: Team-Weeth/weeth-server
Length of output: 5678
🏁 Script executed:
# 2. UpdatePostRequest DTO'nun null alanlarını kontrol et
find src -name "UpdatePostRequest*" -type fRepository: Team-Weeth/weeth-server
Length of output: 150
🏁 Script executed:
# 3. Diğer UseCase sınıflarındaki `@Transactional` kullanımı kontrol et
rg -n '@Transactional' src/main/kotlin --type=kt -B2 -A2 | head -100Repository: Team-Weeth/weeth-server
Length of output: 93
🏁 Script executed:
# Read UpdatePostRequest.kt to check field nullability
cat -n src/main/kotlin/com/weeth/domain/board/application/dto/request/UpdatePostRequest.ktRepository: Team-Weeth/weeth-server
Length of output: 1071
🏁 Script executed:
# Search for other UseCase classes and their `@Transactional` patterns
find src/main/kotlin -name "*UseCase.kt" -type f | head -10Repository: Team-Weeth/weeth-server
Length of output: 336
🏁 Script executed:
# Check `@Transactional` in UseCase files more carefully
rg '@Transactional' src/main/kotlin -A2 -B2 --glob="*UseCase.kt" 2>/dev/null | head -80Repository: Team-Weeth/weeth-server
Length of output: 5014
UseCase 클래스에는 클래스 레벨 @Transactional이 필요합니다.
메서드별로 @Transactional이 적용되어 있으나, 가이드라인에 따라 UseCase 클래스 수준에 단일 @Transactional을 적용하고 메서드 어노테이션은 제거해야 합니다.
수정 예시
`@Service`
+@Transactional
class ManagePostUseCase(
private val postRepository: PostRepository,
private val boardRepository: BoardRepository, // 동일 도메인
private val userGetService: UserGetService,
private val fileRepository: FileRepository,
private val fileReader: FileReader,
private val fileMapper: FileMapper,
private val postMapper: PostMapper,
) {
- `@Transactional`
fun save(
boardId: Long,
request: CreatePostRequest,
userId: Long,
): PostSaveResponse {
@@
- `@Transactional`
fun update(
postId: Long,
request: UpdatePostRequest,
userId: Long,
): PostSaveResponse {
@@
- `@Transactional`
fun delete(
postId: Long,
userId: Long,
) {참고: update() 메서드의 TODO(PATCH 규칙)는 현재 데이터 손실 위험은 없습니다(title, content는 @NotBlank로 필수). 향후 진정한 PATCH 의미론 구현 시 선택적 필드 업데이트 로직이 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt`
around lines 25 - 36, 현재 ManagePostUseCase 클래스는 메서드별로 `@Transactional이` 적용되어 있는데,
가이드라인에 따라 클래스 레벨에 단일 `@Transactional을` 선언하고 각 메서드의 `@Transactional` 애노테이션을 제거해야 합니다;
ManagePostUseCase 클래스 선언부에 `@Transactional을` 추가하고 save(), update(), delete() 등
UseCase 내부 메서드에서 개별 `@Transactional` 어노테이션을 제거하여 트랜잭션 설정을 중앙화하고 중복을 없애세요 (참고:
update()의 TODO는 현 상태에서 데이터 손실 위험이 없으므로 그대로 두되, 향후 PATCH 의미론을 구현할 때 선택적 필드 업데이트
로직을 적용하세요).
| @Transactional | ||
| fun update( | ||
| postId: Long, | ||
| request: UpdatePostRequest, | ||
| userId: Long, | ||
| ): PostSaveResponse { | ||
| val post = findPost(postId) | ||
| validateOwner(post, userId) | ||
|
|
||
| // TODO: PATCH 규칙 - title/content/cardinalNumber는 실제 변경된 경우에만 반영하도록 수정 필요 | ||
| post.update( | ||
| newTitle = request.title, | ||
| newContent = request.content, | ||
| newCardinalNumber = request.cardinalNumber, | ||
| ) | ||
|
|
||
| replacePostFiles(post, request.files) | ||
| return postMapper.toSaveResponse(post) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine UpdatePostRequest definition
echo "=== UpdatePostRequest definition ==="
fd -a 'UpdatePostRequest.kt' src/main | head -5 | xargs -I{} sh -c 'echo "File: {}"; cat -n {}'
# Also search for UpdatePostRequest as a data class or class in any Kotlin file
echo ""
echo "=== Searching for UpdatePostRequest class definition ==="
rg -n 'class UpdatePostRequest|data class UpdatePostRequest' src/main -A 10Repository: Team-Weeth/weeth-server
Length of output: 2658
🏁 Script executed:
#!/bin/bash
# Find and examine Post.update method
echo "=== Searching for Post.update() method ==="
fd -a 'Post.kt' src/main | xargs -I{} sh -c 'echo "File: {}"; rg -n "fun update" {} -B 2 -A 15'Repository: Team-Weeth/weeth-server
Length of output: 955
title/content는 non-nullable이므로 null 덮어쓰기 위험이 없습니다.
UpdatePostRequest에서 title과 content는 @NotBlank로 제약된 String 타입(non-nullable)이므로 null로 덮어써질 수 없습니다. cardinalNumber는 Int?로 nullable이지만, 단순 Elvis 연산자(?: post.cardinalNumber)로는 "미제공" vs "명시적 null" 구분이 불가하여 진정한 PATCH 규칙 구현이 불완전합니다. 이미 TODO에서 언급된 대로 Optional 래핑, 별도 DTO 또는 조건부 업데이트 방식으로 개선 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCase.kt`
around lines 59 - 77, The update method in ManagePostUseCase currently
unconditionally overwrites fields via post.update(newTitle = request.title,
newContent = request.content, newCardinalNumber = request.cardinalNumber) and
cannot distinguish "field omitted" vs "explicit null" for cardinalNumber; change
UpdatePostRequest to use optional semantics (e.g., Optional-wrapped fields,
nullable boxed types plus presence flags, or a separate Patch DTO) so callers
can indicate which fields to patch, then modify ManagePostUseCase.update to
apply updates conditionally (only call post.update or pass values when
title/content/cardinalNumber are explicitly provided) and preserve existing
values otherwise, ensuring validateOwner, findPost and replacePostFiles behavior
remains unchanged.
📌 Summary
Board 도메인을 코틀린으로 마이그레이션 하면서 V4 기획에 따른 기반 작업을 진행했습니다.
📝 Changes
What
Why
How
📸 Screenshots / Logs
💡 Reviewer 참고사항
도메인이 크다 보니 파일 변경이 엄청 많아 졋네요.. 어떻게 보면 아직 미완성 도메인이라서 핵심 로직과 테스트만 봐주셔도 될 것 같습니당..
✅ Checklist
Summary by CodeRabbit
새로운 기능
기능 개선
기타 변경