feat(connection): Connection 관련 도메인 및 jpa쪽 기본 구현#48
Conversation
Walkthrough이 PR에서는 도메인, 퍼시스턴스, 테스트 계층 전반에 걸쳐 변경 사항이 추가되었습니다.
Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant C as Connection
participant CC as ConnectionCancellation
U->>C: cancel(userId, reason, detail) 호출
C->>CC: ConnectionCancellation.create(userId, reason, detail)
CC-->>C: 생성된 ConnectionCancellation 반환
C-->>U: cancellation 속성이 반영된 새 Connection 반환
sequenceDiagram
participant App as 어플리케이션
participant CA as ConnectionAttemptPersistenceAdapter
participant J as ConnectionAttemptJpaRepository
App->>CA: save(connectionAttempt) 호출
CA->>CA: connectionAttempt.toJpaEntity() 변환
CA->>J: jpaEntity 저장 요청
J-->>CA: 저장된 jpaEntity 반환
CA->>CA: toDomainEntity() 변환
CA-->>App: 저장 완료된 ConnectionAttempt 반환
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
dojinyou
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (12)
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt (1)
46-52: 취소 정보에 대한 관계 설정이 적절합니다.
@OneToOne관계와FetchType.EAGER설정은 취소 정보를 즉시 로딩하도록 합니다. 그러나 대부분의 연결이 취소되지 않을 것이라면, 성능 최적화를 위해FetchType.LAZY를 고려해 볼 수 있습니다.@OneToOne( - fetch = FetchType.EAGER, + fetch = FetchType.LAZY, cascade = [CascadeType.ALL], ) var cancellation: ConnectionCancellationJpaEntity? = cancellation private setdomain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (2)
20-38: Status 열거형의 처리 방식이 적절합니다.Status 열거형과 from() 메서드를 통해 문자열을 열거형으로 변환하는 패턴이 잘 구현되어 있습니다. 알 수 없는 상태값에 대한 예외 처리와 로깅을 포함하여 안정성을 확보했습니다.
다만, UNKNOWN 상태가 비즈니스 로직에서 어떤 의미를 가지는지 주석으로 설명하면 더 좋을 것 같습니다.
40-54: MatchingAttempt 생성 로직이 잘 구현되어 있습니다.create() 메서드를 통해 간결하게 새 MatchingAttempt 인스턴스를 생성할 수 있도록 구현되어 있습니다. 기본값 설정과 시간 처리가 적절합니다.
상태 변경을 위한 도우미 메서드 추가를 고려해볼만 합니다. 예를 들어 MATCHING에서 MATCHED나 FAILED로 상태를 변경하는 메서드가 있으면 도메인 로직이 더 명확해질 수 있습니다.
+ fun match(connection: Connection): MatchingAttempt { + return copy( + connection = connection, + status = Status.MATCHED + ) + } + + fun fail(): MatchingAttempt { + return copy( + status = Status.FAILED + ) + }infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt (1)
12-40: JPA 엔티티 구조가 잘 정의되어 있습니다.ConnectionCancellationJpaEntity 클래스가 필요한 JPA 어노테이션을 잘 활용하고 있습니다. 모든 속성에 private setter를 사용하여 불변성을 유지하는 패턴이 좋습니다.
불필요한 빈 줄(42라인)을 제거하면 코드 일관성이 더 향상될 것 같습니다.
@Column(name = "created_at", nullable = false) var createdAt: LocalDateTime = createdAt private set - function toDomain(): ConnectionCancellation {infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (3)
35-40: FetchType을 LAZY로 변경하는 것이 좋을 것 같습니다.Connection 관계를 EAGER 로딩으로 설정하면 MatchingAttempt를 조회할 때마다 항상 Connection 데이터를 함께 로드하게 됩니다. 이는 필요하지 않은 경우에도 데이터를 로드하여 성능 저하의 원인이 될 수 있습니다.
@OneToOne( - fetch = FetchType.EAGER, + fetch = FetchType.LAZY, cascade = [CascadeType.ALL], )
35-40: CascadeType을 더 구체적으로 지정하는 것이 좋습니다.현재 CascadeType.ALL은 모든 작업(PERSIST, MERGE, REMOVE, REFRESH, DETACH)을 연결된 엔티티에 전파합니다. 특히 REMOVE 작업이 전파되면 의도치 않게 Connection 엔티티가 삭제될 수 있습니다. 실제 요구사항에 맞게 더 구체적인 cascade 타입을 지정하는 것이 좋습니다.
@OneToOne( fetch = FetchType.EAGER, - cascade = [CascadeType.ALL], + cascade = [CascadeType.PERSIST, CascadeType.MERGE], )
50-52: 감사 필드(Audit fields) 추가를 고려해보세요.현재
createdAt은 있지만, 일반적으로 유용한 감사 필드인updatedAt이나createdBy가 없습니다. 이러한 필드들은 데이터 변경 추적에 매우 유용합니다.@Column(name = "created_at", nullable = false) var createdAt: LocalDateTime = createdAt private set +@Column(name = "updated_at", nullable = false) +var updatedAt: LocalDateTime = createdAt + private setinfrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt (5)
16-18: 저장된 엔티티를 반환하도록 수정하면 좋을 것 같습니다.현재 save 메서드는 저장된 엔티티를 반환하지 않습니다. 저장 후 엔티티를 반환하면 생성된 ID나 업데이트된 값들을 클라이언트에서 바로 사용할 수 있어 편리합니다.
-override fun save(root: MatchingAttempt) { - matchingAttemptJpaRepository.save(root.toJpaEntity()) +override fun save(root: MatchingAttempt): MatchingAttempt { + val savedEntity = matchingAttemptJpaRepository.save(root.toJpaEntity()) + return savedEntity.toDomainEntity() }
24-26: 구현되지 않은 delete 메서드에 대한 설명 추가가 필요합니다.현재 delete 메서드는 NotImplementedError를 발생시킵니다. 미구현 상태라면 TODO 주석을 추가하여 왜 구현되지 않았는지, 언제 구현될 예정인지 명시하는 것이 좋습니다.
override fun delete(id: MatchingAttempt.Id) { - throw NotImplementedError() + // TODO: 데이터 무결성 보장을 위한 삭제 정책 확립 후 구현 예정 (JIRA-123) + throw NotImplementedError("MatchingAttempt 삭제 기능은 아직 구현되지 않았습니다.") }
28-30: 이 메서드도 마찬가지로 구현 계획에 대한 주석이 필요합니다.두 번째 delete 메서드도 동일하게 NotImplementedError를 발생시킵니다. 여기에도 구현 계획에 대한 설명이 있으면 더 명확할 것 같습니다.
override fun delete(root: MatchingAttempt) { - throw NotImplementedError() + // TODO: 데이터 무결성 보장을 위한 삭제 정책 확립 후 구현 예정 (JIRA-123) + throw NotImplementedError("MatchingAttempt 삭제 기능은 아직 구현되지 않았습니다.") }
20-22: find 메서드에 @transactional(readOnly = true) 추가를 고려해보세요.조회 메서드에는 readOnly = true 속성을 추가하면 데이터베이스 최적화에 도움이 됩니다. 클래스 레벨에 이미 @transactional이 있지만, 메서드 레벨에서 구체적인 속성을 설정할 수 있습니다.
+@Transactional(readOnly = true) override fun find(id: MatchingAttempt.Id): MatchingAttempt? { return matchingAttemptJpaRepository.findById(id.value).getOrNull()?.toDomainEntity() }
32-33: 불필요한 빈 줄이 있습니다.32-33번 라인에 불필요한 빈 줄이 있습니다. 코드 일관성을 위해 제거하는 것이 좋습니다.
override fun delete(root: MatchingAttempt) { throw NotImplementedError() } - - } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt(4 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt(1 hunks)infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attemp.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run unit test and static analysis
🔇 Additional comments (16)
domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt (1)
6-7: 레포지토리 인터페이스 정의가 잘 되어 있습니다.이 인터페이스는 도메인 레이어에서
MatchingAttempt엔티티와 관련된 데이터 접근 계약을 정의하고 있습니다. 기본 Repository 인터페이스를 확장하여 모든 기본 CRUD 작업을 상속받아 사용할 수 있게 설계되어 있습니다.infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt (1)
8-9: JPA 리포지토리 인터페이스가 적절하게 정의되었습니다.Spring Data JPA의 관례에 맞게
@Repository어노테이션을 사용하고 있으며,JpaRepository를 상속받아 기본 CRUD 기능을 제공합니다.MatchingAttemptJpaEntity와 UUID를 타입 파라미터로 올바르게 지정하고 있습니다.infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attemp.sql (1)
15-27: 연결 취소 테이블 정의가 적절히 되어 있습니다.테이블 구조와 인덱스, 제약조건이 잘 정의되어 있습니다. 각 필드에 대한 한글 설명도 명확하게 작성되어 있어 다른 개발자들이 이해하기 쉽습니다.
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt (4)
12-12: import 문이 구체적으로 수정되었습니다.
java.util.*대신java.util.UUID로 구체적인 import 문을 사용하도록 변경했습니다. 이는 코드의 명확성을 높이는 좋은 관행입니다.
21-21: 새로운 취소 관련 속성이 추가되었습니다.Connection 엔티티에 취소 정보를 저장하기 위한
cancellation속성이 추가되었습니다. 이 변경은 도메인 모델의 변화를 적절하게 반영하고 있습니다.
59-60: 도메인 변환 메서드가 올바르게 업데이트되었습니다.
toDomain()메서드에서 새로 추가된cancellation속성을 도메인 객체로 변환하는 로직이 잘 반영되었습니다. null 값을 처리하기 위한 안전 호출 연산자(?.)를 적절히 사용하고 있습니다.
71-72: 정적 팩토리 메서드가 올바르게 업데이트되었습니다.
from메서드에서 도메인 객체의cancellation속성을 JPA 엔티티로 변환하는 로직이 적절하게 구현되었습니다. null 처리를 위한let함수와 안전 호출 연산자를 올바르게 사용하고 있습니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (1)
11-18: 잘 설계된 도메인 엔티티 클래스입니다.MatchingAttempt 클래스가 AggregateRoot를 확장하여 도메인 주도 설계 원칙을 잘 따르고 있습니다. 속성들이 불변(immutable)으로 선언되어 있어 안전한 상태 관리가 가능합니다.
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt (2)
43-51: 도메인 변환 메서드가 잘 구현되어 있습니다.toDomain() 메서드가 JPA 엔티티를 도메인 객체로 변환하는 로직을 명확하게 구현하고 있습니다. 이는 영속성 계층과 도메인 계층의 분리를 잘 지원합니다.
53-64: 정적 팩토리 메서드가 잘 구현되어 있습니다.companion object의 from() 메서드가 도메인 객체를 JPA 엔티티로 변환하는 로직을 깔끔하게 구현하고 있습니다. 이는 도메인 주도 설계에서 영속성 어댑터 패턴을 잘 따르고 있습니다.
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt (3)
19-32: 취소 기능이 잘 구현되어 있습니다.cancel() 메서드가 함수형 프로그래밍 원칙을 잘 따르고 있습니다. 새로운 ConnectionCancellation을 생성하고 불변 객체 패턴을 사용하여 새 Connection을 반환하는 방식이 좋습니다.
36-46: match 메서드 업데이트가 적절하게 이루어졌습니다.match() 메서드가 새로운 cancellation 속성을 고려하여 업데이트되었습니다. null 초기화를 통해 새로 생성된 Connection이 취소되지 않은 상태임을 명확히 합니다.
4-15:Details
❓ Verification inconclusive
상속 구조 변경에 대한 고려가 필요합니다.
Connection 클래스가 AggregateRoot에서 DomainEntity로 상속 구조가 변경되었습니다. 이는 도메인 모델링에서 중요한 변화로, 이 변경의 이유와 영향을 고려해볼 필요가 있습니다.
cancellation 속성 추가는 취소 기능을 지원하기 위한 좋은 접근법입니다.
🏁 Script executed:
#!/bin/bash # AggregateRoot와 DomainEntity 클래스의 차이점 및 상속 관계 확인 fd -e kt -e java "AggregateRoot|DomainEntity" | xargs cat | grep -n "class"Length of output: 244
상속 구조 변경 재검토 요청 및 취소 기능 개선 확인
- 스크립트 결과,
DomainEntity와AggregateRoot가 각각 정의되어 있으며,Connection클래스가 의도적으로AggregateRoot대신DomainEntity를 상속하는 것으로 보입니다.cancellation속성 추가를 통해 취소 기능 지원이 강화된 점은 긍정적입니다.- 다만, 상속 구조 변경이 도메인 모델링 전반에 미치는 영향에 대해 추가적으로 검토하고, 관련 문서화가 필요해 보입니다.
해당 변경사항이 도메인 로직 전반에 미치는 영향을 재검토해주시기 바랍니다.
domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (3)
10-18: 도메인 엔티티 설계가 잘 되어 있습니다.ConnectionCancellation 클래스가 DomainEntity를 확장하여 도메인 모델의 일관성을 유지하고 있습니다. 모든 속성이 불변(val)으로 선언되어 있어 안전한 상태 관리가 가능합니다.
19-41: 취소 사유 열거형이 잘 구현되어 있습니다.Reason 열거형이 다양한 취소 사유를 포괄적으로 정의하고 있습니다. from() 메서드를 통한 문자열 변환 처리와 예외 상황에 대한 로깅이 적절합니다.
43-57: 팩토리 메서드가 적절하게 구현되어 있습니다.create() 메서드를 통해 ConnectionCancellation 생성을 단순화했습니다. 고유 ID 생성과 시간 처리가 적절하게 구현되어 있습니다.
dojinyou
force-pushed
the
feature/connections
branch
from
69d9a3d to
e741be3
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (2)
32-34: 경고 로그 메시지가 영어로 작성되어 있습니다.로그 메시지의 일관성을 위해 한글로 변경하는 것이 좋을 수 있습니다. 다른 로그 메시지들과의 일관성을 확인해보세요.
- logger.warn { "MatchingAttempt status $status not found" } + logger.warn { "MatchingAttempt 상태 $status를 찾을 수 없습니다" }
11-18: MatchingAttempt의 불변 상태 처리 방식 개선 제안현재 MatchingAttempt는 connection과 status가 변경 가능한 속성인데, 이러한 변경을 처리하는 메서드가 없습니다. 상태 변경을 위한 메서드를 추가하는 것이 도메인 모델의 일관성을 유지하는 데 도움이 될 것입니다.
fun updateStatus(newStatus: Status): MatchingAttempt { return copy(status = newStatus) } fun assignConnection(connection: Connection): MatchingAttempt { return copy(connection = connection, status = Status.MATCHED) } fun markAsFailed(): MatchingAttempt { return copy(status = Status.FAILED) }infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attemp.sql (1)
7-8: 열 이름의 명명 규칙이 일관되지 않습니다.7-8번 라인의
attempt_date와created_at는 스네이크 케이스(snake_case)로 작성되어 있는데, 다른 열 이름들과 명명 규칙이 일치하는지 확인해주세요. 일반적으로 SQL에서는 스네이크 케이스를 사용하므로, 다른 열 이름들도 이와 같이 일관되게 사용하는 것이 좋습니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (2)
35-36: 경고 로그 메시지가 영어로 작성되어 있습니다.로그 메시지의 일관성을 위해 한글로 변경하는 것이 좋을 수 있습니다. 다른 로그 메시지들과의 일관성을 확인해보세요.
- logger.warn { "ConnectionCancellation reason $value not found" } + logger.warn { "ConnectionCancellation 사유 $value를 찾을 수 없습니다" }
19-21: 취소 사유(Reason) 열거형에 대한 설명 추가 제안여러 취소 사유가 정의되어 있지만, 각 사유에 대한 설명이 없어 코드를 읽는 사람이 각 사유의 정확한 의미를 이해하기 어려울 수 있습니다. 각 사유에 주석이나 설명을 추가하는 것을 고려해 보세요.
enum class Reason { - NO_RESPONSE, FIRST_MESSAGE_TIMEOUT, DUPLICATED, REPORTED, ADMIN_CANCELLATION, ETC, UNKNOWN, + /** 응답 없음 */ + NO_RESPONSE, + /** 첫 메시지 시간 초과 */ + FIRST_MESSAGE_TIMEOUT, + /** 중복 연결 */ + DUPLICATED, + /** 신고됨 */ + REPORTED, + /** 관리자에 의한 취소 */ + ADMIN_CANCELLATION, + /** 기타 사유 */ + ETC, + /** 알 수 없는 사유 */ + UNKNOWN, ;infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (3)
27-30: 엔티티 키 생성 전략을 고려해 보세요.
현재UUID를 직접 주입받는 구조로, JPA에서 ID를 자동 생성하지 않습니다. 필요하다면@GeneratedValue를 사용하여 ID 생성 전략을 명시하고,@Column(updatable=false)옵션 설정 등을 고려해 보세요.
35-38: EAGER + CascadeType.ALL에 의해 발생할 수 있는 불필요한 로딩 및 영속성 전이 문제를 검토해 주세요.
OneToOne 관계에서EAGER는 세심한 고려가 필요합니다. 필요하지 않은 연관 엔티티까지 즉시 로딩할 위험이 있고, CascadeType.ALL은 자칫 의도치 않은 업데이트/삭제 전이를 유발할 수 있습니다.
42-45: status 필드를 enum으로 직접 매핑하는 것을 고려해 보세요.
현재 String으로 저장하고 있으나,@Enumerated(EnumType.STRING)등을 사용하면 도메인 계층과 매핑을 더 안전하게 관리할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt(4 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt(1 hunks)infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attemp.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt
- domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run unit test and static analysis
🔇 Additional comments (11)
domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (1)
1-55: 도메인 모델 설계가 잘 되어 있습니다.MatchingAttempt 클래스의 구조와 기능이 명확하게 정의되어 있습니다. AggregateRoot 상속, 식별자 관리, 상태(Status) 열거형 및 팩토리 메서드가 잘 구현되어 있습니다.
infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attemp.sql (1)
1-1: 파일명에 오타가 있습니다.현재 파일명이
V4__init_matching_attemp.sql로 되어 있는데, "attemp"는 "attempt"의 오타로 보입니다. 파일명을V4__init_matching_attempt.sql로 수정하는 것이 좋겠습니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt (5)
4-7: AggregateRoot에서 DomainEntity로 상속 변경상속 구조가 AggregateRoot에서 DomainEntity로 변경되었습니다. 이는 도메인 모델의 역할 변경을 의미할 수 있습니다. 이 변경이 의도적인 것인지 확인하고, 관련 코드베이스 전체에 일관된 방식으로 적용되었는지 확인해주세요.
14-15: Connection에 취소 기능 추가Connection 클래스에 취소 정보(cancellation) 속성이 추가되어 확장성이 높아졌습니다. 취소 기능이 추가됨에 따라 도메인 모델이 더 완전해졌습니다.
19-32: 취소 기능 구현이 잘 되어 있습니다.불변 객체 패턴을 잘 따르고 있으며, 취소 로직이 명확하게 구현되어 있습니다. 원본 객체를 변경하지 않고 새 객체를 반환하는 방식이 Kotlin의 불변성 원칙에 잘 맞습니다.
38-39: 코드 스타일 개선match 메서드의 매개변수 목록에서 user2 뒤에 콤마를 추가하여 코드 스타일이 개선되었습니다. 이는 Kotlin의 권장 코딩 스타일을 따르는 좋은 변경입니다.
44-45: 생성자에 cancellation 파라미터 추가Connection 생성 시 cancellation 파라미터가 추가되었고 기본값으로 null이 할당됩니다. 이는 취소 기능 추가에 맞춰 적절하게 변경되었습니다.
domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (1)
1-58: ConnectionCancellation 도메인 모델이 잘 설계되어 있습니다.새로 추가된 ConnectionCancellation 클래스는 도메인 설계 원칙을 잘 따르고 있으며, 취소 사유와 세부 정보를 적절히 모델링하고 있습니다. 팩토리 메서드 패턴을 사용하여 객체 생성이 간결해진 점이 좋습니다.
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (3)
16-17: 테이블 이름에 대한 확인이 필요합니다.
"matching_attempt"라는 단수형 테이블명이 적절한지, 기존 DDL이나 다른 테이블들과의 네이밍 일관성이 맞는지 검토해 주세요.
54-63: companion object 및 확장 함수 구현은 적절해 보입니다.
도메인 ↔ JPA 간 변환 로직이 명확하며, 각 필드를 일관성 있게 매핑하고 있습니다.
65-73: toDomainEntity 함수의 매핑이 도메인 규칙과 일치하는지 최종 확인 필요
현재 매핑 로직 자체에는 문제 없어 보이지만, 도메인 쪽에서 ID, userId, connection 필드가null일 수 있는지 여부, status의 올바른 변환 등을 꼼꼼히 점검해 주세요.
dojinyou
force-pushed
the
feature/connections
branch
from
e741be3 to
d19dd88
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (1)
23-40:Reason열거형의from메서드 구현 개선이 필요합니다.
from메서드는 문자열을Reason열거형으로 변환하는 기능을 합니다. 현재 구현은 작동하지만, 새로운Reason값이 추가될 때마다 이 메서드를 수동으로 업데이트해야 합니다. 이는 유지보수 측면에서 위험할 수 있습니다.다음과 같이 리플렉션을 사용하여 자동으로 열거형 값을 처리하도록 개선할 수 있습니다:
fun from(value: String): Reason { - return when (value) { - "NO_RESPONSE" -> NO_RESPONSE - "FIRST_MESSAGE_TIMEOUT" -> FIRST_MESSAGE_TIMEOUT - "DUPLICATED" -> DUPLICATED - "REPORTED" -> REPORTED - "ADMIN_CANCELLATION" -> ADMIN_CANCELLATION - "ETC" -> ETC - else -> { - logger.warn { "ConnectionCancellation reason $value not found" } - UNKNOWN - } - } + return try { + valueOf(value) + } catch (e: IllegalArgumentException) { + logger.warn { "ConnectionCancellation reason $value not found" } + UNKNOWN + } }domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (2)
43-53:create메서드의 파라미터 순서가 생성자와 불일치합니다.
create메서드에서MatchingAttempt객체를 생성할 때, 파라미터의 순서가 클래스 선언의 순서와 다릅니다. 이는 코드의 가독성과 일관성을 해칠 수 있습니다.파라미터 순서를 클래스 선언과 일치시키는 것이 좋습니다:
fun create(userId: User.Id): MatchingAttempt { val now = LocalDateTime.now() return MatchingAttempt( id = UUIDTypeId.random(), - userId = userId, - status = Status.MATCHING, - attemptDate = now.toLocalDate(), - createdAt = now, - connection = null, + connection = null, + userId = userId, + status = Status.MATCHING, + attemptDate = now.toLocalDate(), + createdAt = now, ) }
1-55:enum class Status의from메서드 구현 개선이 필요합니다.
Status열거형의from메서드는 문자열을Status열거형으로 변환하지만, 현재 구현은 새로운 상태가 추가될 때마다 메서드를 업데이트해야 합니다. 이는 유지보수 측면에서 좋지 않습니다.다음과 같이 리플렉션을 사용하여 자동으로 열거형 값을 처리하도록 개선할 수 있습니다:
fun from(status: String): Status { - return when (status) { - "MATCHING" -> MATCHING - "MATCHED" -> MATCHED - "FAILED" -> FAILED - else -> { - logger.warn { "MatchingAttempt status $status not found" } - UNKNOWN - } - } + return try { + valueOf(status) + } catch (e: IllegalArgumentException) { + logger.warn { "MatchingAttempt status $status not found" } + UNKNOWN + } }infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (1)
54-63:toJpaEntity확장 함수의 위치가 적절한지 검토해주세요.도메인 객체를 JPA 엔티티로 변환하는
toJpaEntity함수가 JPA 엔티티 클래스의 companion object 내에 정의되어 있습니다. 이 방식은 작동하지만, 도메인 계층에 인프라 계층에 대한 의존성을 만들지 않기 위해 확장 함수를 사용하는 것입니다. 그러나 이 패턴은 코드 구조를 복잡하게 만들 수 있습니다.도메인 엔티티 변환을 위한 별도의 매퍼 클래스를 고려해 보세요. 이는 관심사 분리를 더 명확히 하고 유지보수성을 향상시킬 수 있습니다:
class MatchingAttemptMapper { companion object { fun fromDomain(domain: MatchingAttempt): MatchingAttemptJpaEntity = MatchingAttemptJpaEntity( id = domain.id.value, userId = domain.userId.value, connection = domain.connection?.let { ConnectionJpaEntity.from(it) }, status = domain.status.name, attemptDate = domain.attemptDate, createdAt = domain.createdAt ) fun toDomain(entity: MatchingAttemptJpaEntity): MatchingAttempt = MatchingAttempt( id = MatchingAttempt.Id(entity.id), userId = User.Id(entity.userId), connection = entity.connection?.toDomain(), status = MatchingAttempt.Status.from(entity.status), attemptDate = entity.attemptDate, createdAt = entity.createdAt, ) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt(4 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt(1 hunks)infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attempt.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt
- domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run unit test and static analysis
🔇 Additional comments (4)
domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (2)
44-57:create메서드가 잘 구현되었습니다.도메인 객체 생성을 위한 팩토리 메서드가 적절하게 구현되었습니다. 이 메서드는 UUID 생성, 현재 시간 설정 등 도메인 객체 생성에 필요한 모든 책임을 캡슐화하고 있습니다.
1-58: DDD 패턴을 잘 적용한 도메인 엔티티 설계입니다.이 클래스는 도메인 주도 설계(DDD) 원칙을 잘 따르고 있습니다:
- 불변(Immutable) 데이터 클래스로 정의되어 있어 객체의 상태 변경이 예측 가능합니다.
- 식별자(ID)가 타입화되어 있어 타입 안전성을 제공합니다.
- 팩토리 메서드를 통해 객체 생성 로직을 캡슐화했습니다.
- 도메인 특화 언어(DSL)를 사용하여 가독성 높은 코드를 작성했습니다.
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (2)
35-40:CascadeType.ALL사용이 적절한지 검토가 필요합니다.현재
connection필드에CascadeType.ALL이 설정되어 있습니다. 이는MatchingAttemptJpaEntity에 대한 모든 영속성 작업(저장, 삭제 등)이 연관된ConnectionJpaEntity에도 적용됨을 의미합니다. 특히 삭제 시 연결 엔티티도 함께 삭제될 수 있어 의도치 않은 데이터 손실이 발생할 수 있습니다.실제로
MatchingAttempt가 삭제될 때 연관된Connection도 함께 삭제되어야 하는지 검토해주세요. 그렇지 않다면 다음과 같이 수정하는 것이 좋습니다:@OneToOne( fetch = FetchType.EAGER, - cascade = [CascadeType.ALL], + cascade = [CascadeType.PERSIST, CascadeType.MERGE], )
16-18: JPA 엔티티 명명이 적절합니다.테이블 명을 단수형
matching_attempt로 지정한 것은 JPA 컨벤션에 맞게 적절히 구현되었습니다. 이전 리뷰 코멘트에서 언급된 이슈가 잘 해결되었습니다.
dojinyou
force-pushed
the
feature/connections
branch
from
d19dd88 to
27ddb60
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (1)
19-41: 취소 사유 열거형 구현 개선 가능
Reason열거형이 다양한 취소 사유를 포함하고 있으며, 문자열을 열거형으로 변환하는 기능을 제공하는 것은 좋습니다. 다만 몇 가지 개선 사항을 제안합니다:
- 각 열거 값에 대한 설명 주석이 없어 각 취소 사유의 정확한 의미나 사용 시나리오를 이해하기 어렵습니다.
from메서드에서when문 대신valueOf를 사용하여 코드를 간소화하는 방법도 고려해볼 수 있습니다. 다만 이 경우 예외 처리가 필요합니다.enum class Reason { // 응답 없음 NO_RESPONSE, // 첫 메시지 타임아웃 FIRST_MESSAGE_TIMEOUT, // 중복 매칭 DUPLICATED, // 신고됨 REPORTED, // 관리자에 의한 취소 ADMIN_CANCELLATION, // 기타 사유 ETC, // 알 수 없는 사유 UNKNOWN, ; companion object { private val logger = KotlinLogging.logger { } fun from(value: String): Reason { return try { valueOf(value) } catch (e: IllegalArgumentException) { logger.warn { "ConnectionCancellation reason $value not found" } UNKNOWN } } } }domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (1)
42-54: 클래스 레벨에서의 로깅 추가 고려
Statusenum의 companion object에만 로거가 정의되어 있고, 클래스 레벨에서는 로깅이 없습니다. 클래스 레벨에서도 로깅을 추가하여create메소드 호출이나 다른 중요한 이벤트를 기록하는 것을 고려해보세요.companion object { + private val logger = KotlinLogging.logger { } fun create(userId: User.Id): MatchingAttempt { val now = LocalDateTime.now() + logger.debug { "Creating new MatchingAttempt for user: ${userId.value}" } return MatchingAttempt( id = UUIDTypeId.random(), userId = userId, status = Status.MATCHING, attemptDate = now.toLocalDate(), createdAt = now, connection = null, ) } }로깅은 디버깅과 모니터링에 중요한 역할을 하므로, 적절히 추가하는 것이 좋습니다.
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (2)
54-63: 변환 메소드의 일관성 개선
toJpaEntity확장 함수와toDomainEntity멤버 함수 간에 비대칭성이 있습니다. 또한,ConnectionJpaEntity.from(it)와connection?.toDomain()의 사용이 일관되지 않습니다.다음과 같은 개선을 고려해보세요:
- 두 변환 메소드의 패턴을 일관되게 유지하세요. 예를 들어, 둘 다 확장 함수로 만들거나 둘 다 companion object의 정적 메소드로 만드는 것이 좋습니다.
Connection객체의 변환에 사용되는 메소드 이름을 일관되게 유지하세요.companion object { fun MatchingAttempt.toJpaEntity() = MatchingAttemptJpaEntity( id = id.value, userId = userId.value, - connection = connection?.let { ConnectionJpaEntity.from(it) }, + connection = connection?.toJpaEntity(), status = status.name, attemptDate = attemptDate, createdAt = createdAt ) + + fun from(domainEntity: MatchingAttempt) = domainEntity.toJpaEntity() }이러한 변경은 코드의 일관성을 높이고 다른 개발자들이 코드를 이해하기 쉽게 만듭니다.
65-72: 도메인 변환 메소드에 대한 단위 테스트 추가 권장
toDomainEntity메소드와toJpaEntity확장 함수는 도메인 모델과 영속성 모델 간의 변환을 담당하는 중요한 부분입니다. 이러한 변환이 정확하게 이루어지지 않으면 애플리케이션에 심각한 버그가 발생할 수 있습니다.다음과 같은 단위 테스트를 작성하여 변환의 정확성을 검증하는 것을 권장합니다:
- 도메인 엔티티 -> JPA 엔티티 -> 도메인 엔티티 변환 후 원본과 동일한지 확인
- null 값, 경계값 등 다양한 케이스에 대한 테스트
- 변환 과정에서 발생할 수 있는 예외 상황에 대한 테스트
이러한 테스트는 변환 로직의 정확성을 보장하고, 향후 변경 시 회귀 테스트로 활용할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt(4 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt(1 hunks)infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attempt.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- domain/src/main/kotlin/com/threedays/domain/connection/repository/MatchingAttemptRepository.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/MatchingAttemptJpaRepository.kt
- infrastructure/persistence/src/main/resources/db/migration/V4__init_matching_attempt.sql
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/MatchingAttemptPersistenceAdapter.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt
🔇 Additional comments (7)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt (3)
14-14: 새로운 필드 추가 적절Connection 클래스에 취소 정보를 담는
cancellation필드를 nullable 타입으로 추가한 것은 적절합니다. 이를 통해 연결 취소 여부와 취소 관련 정보를 효과적으로 관리할 수 있습니다.
19-32: 취소 메서드 구현이 불변성을 잘 유지함
cancel메서드가 원본 객체를 변경하지 않고copy를 통해 새로운 객체를 반환하는 방식으로 구현되어 있어 불변성(immutability)을 잘 유지하고 있습니다. 이는 함수형 프로그래밍 스타일에 부합하며 사이드 이펙트를 방지하는 좋은 접근법입니다.
45-46: match 메서드에 cancellation 필드 추가 적절
match메서드에서 생성되는 새 Connection 객체에cancellation = null을 명시적으로 설정한 것은 적절합니다. 새로 만들어진 연결은 취소되지 않았다는 상태를 명확히 표현합니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (2)
10-16: 도메인 엔티티 구조가 적절함
ConnectionCancellation클래스가DomainEntity를 상속받아 ID 값을 기반으로 식별 가능한 엔티티로 정의된 것은 적절합니다. 필요한 속성들(id, userId, reason, detail, createdAt)이 포함되어 있어 취소 정보를 충분히 표현할 수 있습니다.
43-57: 팩토리 메서드 패턴 적절히 활용
create메서드를 통해 팩토리 메서드 패턴을 적절히 활용하여 객체 생성을 캡슐화했습니다. 이는 객체 생성 로직을 한 곳에 집중시키고, 필요한 경우 생성 로직을 쉽게 수정할 수 있게 합니다. 또한 파라미터에 기본값을 지정하여 사용성을 높인 것도 좋은 방식입니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/MatchingAttempt.kt (1)
46-46:UUIDTypeId.random()이MatchingAttempt.Id타입을 반환하는지 확인 필요
UUIDTypeId.random()이MatchingAttempt.Id타입의 인스턴스를 반환하는지 확인해주세요. 현재 코드에서는id매개변수가MatchingAttempt.Id타입이므로,UUIDTypeId.random()이 해당 타입을 반환하지 않는다면 타입 불일치 오류가 발생할 수 있습니다.다음과 같이 수정하는 것이 좋을 수 있습니다:
- id = UUIDTypeId.random(), + id = Id(UUID.randomUUID()),infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/MatchingAttemptJpaEntity.kt (1)
16-25: 데이터베이스 컬럼 매핑에 관한 추가 고려사항현재
MatchingAttemptJpaEntity의 테이블 이름은 "matching_attempt"로 설정되어 있으며, 이는 일반적인 명명 규칙을 따릅니다. 과거 리뷰 댓글에서 "DDL은 matching_attempt로 단수형 같던데, 로컬에서 동작했나요?"라는 질문이 있었습니다.이는 데이터베이스 스키마와 JPA 엔티티 간의 매핑에 문제가 있었을 가능성을 시사합니다. 다음 사항을 확인하세요:
- 데이터베이스 마이그레이션 스크립트에서 테이블 이름이 "matching_attempt"인지 확인
- 컬럼 이름과 타입이 데이터베이스 스키마와 일치하는지 확인
- 제약 조건이 적절히 설정되어 있는지 확인 (예: 기본키, 외래키, NOT NULL 등)
이러한 확인은 런타임에 JPA 매핑 오류를 방지하는 데 도움이 됩니다.
dojinyou
force-pushed
the
feature/connections
branch
from
27ddb60 to
752efdf
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt (1)
4-4: AggregateRoot에서 DomainEntity로 변경된 근거 설명이 필요합니다.
도메인 설계 변경 사유를 PR 설명이나 도메인 문서에 남겨 주시면 이해 관계자들이 배경을 파악하기 좋습니다.
🧹 Nitpick comments (6)
infrastructure/persistence/src/main/resources/db/migration/V4__init_connection_attempt.sql (1)
25-25: connection 테이블 변경이 적절합니다.기존 connection 테이블에 cancellation_id 컬럼을 추가하여 취소 정보와의 연결을 구현했습니다. 다만, cancellation_id에 대한 외래 키 제약 조건을 추가하는 것이 데이터 일관성을 보장하는 데 도움이 될 수 있습니다.
-ALTER TABLE connection ADD COLUMN cancellation_id BINARY(16) NULL COMMENT '연결 취소 ID'; +ALTER TABLE connection ADD COLUMN cancellation_id BINARY(16) NULL COMMENT '연결 취소 ID', +ADD CONSTRAINT fk_connection_cancellation FOREIGN KEY (cancellation_id) REFERENCES connection_cancellation(id);domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionTest.kt (1)
34-35: 시간 비교 시 테스트 실패 가능성을 고려해 주세요.
LocalDateTime.now()와 테스트 실행 시간이 다를 수 있어, 초 단위나 특정 범위 이내로 비교하도록 수정하는 방안을 고려하면 더 안정적인 테스트가 가능합니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionAttempt.kt (1)
56-67: 테스트 용이성 개선 필요
create메서드에서LocalDateTime.now()를 직접 사용하면 단위 테스트가 어려워질 수 있습니다. 시간을 파라미터로 받거나 시간 공급자를 주입받는 방식을 고려해보세요.companion object { - fun create(userId: User.Id): ConnectionAttempt { - val now = LocalDateTime.now() + fun create(userId: User.Id, now: LocalDateTime = LocalDateTime.now()): ConnectionAttempt { return ConnectionAttempt( id = UUIDTypeId.random(), userId = userId, status = Status.CONNECTING, attemptDate = now.toLocalDate(), createdAt = now, connection = null, ) } }infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/ConnectionAttemptPersistenceAdapter.kt (3)
16-18: 예외 처리 추가 필요
save메서드에서 저장 작업 중 발생할 수 있는 예외 처리가 없습니다. 트랜잭션 롤백이 자동으로 처리되더라도, 로깅이나 구체적인 예외 변환을 고려해보세요.override fun save(root: ConnectionAttempt) { + try { connectionAttemptJpaRepository.save(root.toJpaEntity()) + } catch (e: Exception) { + // 로깅 및 적절한 예외 변환 + throw RuntimeException("Failed to save connection attempt: ${e.message}", e) + } }
24-30: 미구현 메서드에 대한 문서화 필요
delete메서드들이NotImplementedError를 발생시키는 것은 의도적인 것으로 보입니다. 이러한 메서드들에 대해 주석을 통해 미구현 이유나 구현 계획을 문서화하는 것이 좋습니다.override fun delete(id: ConnectionAttempt.Id) { + // 현재 구현 단계에서는 Connection Attempt 삭제 기능이 필요하지 않습니다. + // TODO: 향후 필요시 구현 예정 throw NotImplementedError() } override fun delete(root: ConnectionAttempt) { + // 현재 구현 단계에서는 Connection Attempt 삭제 기능이 필요하지 않습니다. + // TODO: 향후 필요시 구현 예정 throw NotImplementedError() }
32-33: 불필요한 빈 줄 제거파일 끝에 불필요한 빈 줄이 있습니다. 코드 일관성을 위해 제거하는 것이 좋습니다.
} -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionAttempt.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/repository/ConnectionAttemptRepository.kt(1 hunks)domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionAttemptTest.kt(1 hunks)domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionTest.kt(1 hunks)domain/src/testFixtures/kotlin/com/threedays/domain/connection/repository/ConnectionAttemptRepositorySpy.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/ConnectionAttemptPersistenceAdapter.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionAttemptJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt(1 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt(4 hunks)infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/ConnectionAttemptJpaRepository.kt(1 hunks)infrastructure/persistence/src/main/resources/db/migration/V4__init_connection_attempt.sql(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/repository/ConnectionAttemptJpaRepository.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionJpaEntity.kt
- infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionCancellationJpaEntity.kt
🧰 Additional context used
🪛 detekt (1.23.7)
domain/src/testFixtures/kotlin/com/threedays/domain/connection/repository/ConnectionAttemptRepositorySpy.kt
[warning] 7-8: The class or object ConnectionAttemptRepositorySpy is empty.
(detekt.empty-blocks.EmptyClassBlock)
🔇 Additional comments (16)
domain/src/main/kotlin/com/threedays/domain/connection/repository/ConnectionAttemptRepository.kt (1)
1-7: Repository 인터페이스 구현이 잘 되었습니다.Repository 패턴을 적절하게 활용하여 ConnectionAttempt 엔티티에 대한 리포지토리 인터페이스를 깔끔하게 정의하셨습니다. 상위 인터페이스에서 기본 메소드들을 상속받기 때문에 추가 메소드가 없어도 적절합니다.
infrastructure/persistence/src/main/resources/db/migration/V4__init_connection_attempt.sql (2)
1-12: connection_attempt 테이블 생성 및 인덱스 설정이 잘 되었습니다.테이블 구조와 인덱스 설정이 적절합니다. 다만, 몇 가지 고려사항이 있습니다:
- connection_id가 NULL 허용이므로, 연결 시도가 실제 연결과 연관되지 않을 수 있는 상황을 처리하기 위한 로직이 도메인 계층에 있는지 확인해주세요.
- connection_id에 대한 외래 키 제약 조건이 없습니다. 일관성을 위해 필요할 수 있습니다.
- attempt_date는 DATE 타입인데, 시도 시간까지 기록할 필요가 없는지 검토해 보세요.
14-23: connection_cancellation 테이블 구조가 적절합니다.취소 관련 정보를 저장하는 테이블 구조와 인덱스가 잘 설계되었습니다. user_id 인덱스도 적절히 추가되었습니다.
infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/entity/ConnectionAttemptJpaEntity.kt (3)
16-53: JPA 엔티티 클래스 구현이 잘 되었습니다.ConnectionAttemptJpaEntity 클래스가 DB 테이블 스키마에 맞게 잘 정의되었습니다. private set을 사용하여 변경 불가능성을 보장한 점도 좋습니다.
다만, 다음 부분을 검토해 보시길 권장합니다:
connection속성에 대해FetchType.EAGER와CascadeType.ALL을 사용하고 있는데, 이는 성능에 영향을 줄 수 있고 의도치 않은 동작을 유발할 수 있습니다. 특히CascadeType.ALL은 ConnectionAttemptJpaEntity를 삭제할 때 연결된 ConnectionJpaEntity도 함께 삭제하게 됩니다. 이것이 의도된 동작인지 확인해 주세요.@OneToOne( - fetch = FetchType.EAGER, - cascade = [CascadeType.ALL], + fetch = FetchType.LAZY, + cascade = [CascadeType.PERSIST, CascadeType.MERGE], )
54-63: 도메인 엔티티 변환 로직이 깔끔합니다.도메인 엔티티에서 JPA 엔티티로 변환하는 companion object extension 함수가 잘 작성되었습니다. 옵셔널 값 처리도 적절합니다.
65-72: JPA에서 도메인 엔티티로의 변환 로직이 잘 작성되었습니다.toDomainEntity 메서드를 통해 JPA 엔티티를 도메인 엔티티로 변환하는 로직이 명확하게 구현되었습니다. 옵셔널 값의 처리도 적절합니다.
domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionAttemptTest.kt (3)
24-38: 테스트 시나리오가 명확하고 필드 검증이 꼼꼼합니다.
새로운 연결 시도 생성 로직에 대한 테스트가 충분히 이뤄져 있습니다.
40-55: 연결 성공 시나리오가 명확히 검증됩니다.
fixtureMonkey로 상태와 연결 객체를 세팅하고,connect메서드 수행 후 상태와 객체가 올바르게 적용되는지 검사하여 테스트 범위가 적절합니다.
73-83: 알 수 없는 문자열 처리 로직이 적절합니다.
Status.from에서 유효하지 않은 문자열을UNKNOWN으로 처리하는 로직이 정상적으로 검증되어, 안정성이 확보됩니다.domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionTest.kt (1)
39-71: 취소 로직이 충분히 테스트되고 있습니다.
cancel메서드로 취소 정보를 세팅한 뒤 검증하는 과정이 명확합니다. 추후 권한 체크나 참가자 확인 로직 등이 추가되면 관련 테스트도 보강해 주세요.domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt (2)
19-31: 취소 로직이 잘 구조화되었습니다.
cancel메서드로ConnectionCancellation을 쉽게 생성하도록 설계되어 있습니다. 다만 실제 업무 요구사항에 따라 취소 가능 조건이나 권한 체크가 필요한지 검토해 주세요.
34-36: 참여자 식별 로직이 간단하고 명확합니다.
두 참가자 중 일치하는 사용자 ID가 있는지 간단히 판별하는 로직으로, 가독성이 좋고 유지보수에 용이합니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionCancellation.kt (2)
19-20: UNKNOWN 상태 처리가 유연합니다.
알 수 없는 문자열을UNKNOWN으로 설정하고 경고 로그를 남기는 방식이 예상치 못한 입력에 대비하여 안전성을 높여줍니다.
43-57: 유연한 객체 생성 메서드가 제공됩니다.
create메서드로ConnectionCancellation객체를 쉽게 생성할 수 있어, 사용 측에서 중복 코드 없이 취소 객체를 생성하도록 도와줍니다.domain/src/main/kotlin/com/threedays/domain/connection/entity/ConnectionAttempt.kt (1)
11-18: 도메인 모델 설계에 대한 검토
ConnectionAttempt클래스가AggregateRoot를 상속받고 있습니다. 이 클래스가 DDD 관점에서 진정한 애그리게이트 루트의 역할을 하는지 검토해보세요. 애그리게이트 루트는 일관성을 유지해야 하는 도메인 객체들의 클러스터에 대한 진입점 역할을 해야 합니다. 이 클래스가 다른 엔티티들의 일관성을 책임지는 역할인지 확인해보세요.infrastructure/persistence/src/main/kotlin/com/threedays/persistence/connection/adapter/ConnectionAttemptPersistenceAdapter.kt (1)
20-22: 도메인 엔티티 변환 검증
toDomainEntity()메서드가 파일에서 명시적으로 임포트되지 않았습니다. 이 메서드가 어디에 정의되어 있는지 확인하고, 필요하다면 명시적으로 임포트하세요.
dojinyou
force-pushed
the
feature/connections
branch
from
752efdf to
cab8283
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionTest.kt (1)
37-37: 코드 포맷팅 이슈가 있습니다.
shouldBeGreaterThan과beforeConnectAt사이에 불필요한 공백이 있습니다.-connection.connectedAt shouldBeGreaterThan beforeConnectAt +connection.connectedAt shouldBeGreaterThan beforeConnectAt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionAttemptTest.kt(1 hunks)domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionTest.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run unit test and static analysis
🔇 Additional comments (13)
domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionAttemptTest.kt (7)
1-16: 패키지 및 import 구성이 적절합니다.테스트에 필요한 라이브러리들(Kotest, FixtureMonkey)과 도메인 클래스들이 잘 가져와져 있습니다. BDD 스타일의 테스트를 위한 DescribeSpec과 assertion을 위한 라이브러리들이 잘 구성되어 있습니다.
17-23: 테스트 클래스 선언 및 FixtureMonkey 설정이 적절합니다.DisplayName 애노테이션을 통해 테스트 목적을 명확히 표현하고 있으며, FixtureMonkey 설정이 잘 구성되어 있습니다. PrimaryConstructorArbitraryIntrospector를 사용하여 Kotlin 기본 생성자를 활용한 객체 생성이 가능하도록 설정했습니다.
24-38: 연결 시도 생성 테스트가 잘 구현되어 있습니다.새로운 ConnectionAttempt 생성 테스트가 잘 작성되었습니다. Arrange-Act-Assert 패턴을 명확히 따르고 있으며, 생성된 객체의 속성이 기대값과 일치하는지 검증하고 있습니다.
40-55: 연결 성공 테스트가 적절하게 구현되어 있습니다.연결 시도가 성공했을 때의 상태 변경을 테스트하는 코드가 잘 작성되었습니다. FixtureMonkey를 활용하여 테스트 객체를 생성하고, connect 메서드 호출 후 상태와 연결 객체가 적절히 변경되었는지 검증하고 있습니다.
57-71: 연결 실패 테스트가 잘 구현되어 있습니다.연결 시도 실패 시 상태 변경을 검증하는 테스트가 적절히 작성되었습니다. fail 메서드 호출 후 status가 FAILED로 변경되는지와 connection 객체가 유지되는지 검증하고 있습니다.
73-83: Status.from 메서드 테스트가 포괄적으로 작성되어 있습니다.Status 열거형의 from 메서드에 대한 테스트가 두 가지 케이스(유효한 문자열, 유효하지 않은 문자열)에 대해 잘 작성되어 있습니다. 특히 모든 Status 항목을 반복하여 검증하는 방식이 효율적입니다.
85-91: Id 클래스 상속 테스트가 적절합니다.ConnectionAttempt.Id 클래스가 UUIDTypeId를 상속받는지 검증하는 테스트가 잘 작성되어 있습니다. 이를 통해 ID 타입의 일관성을 확인할 수 있습니다.
domain/src/test/kotlin/com/threedays/domain/connection/entity/ConnectionTest.kt (6)
1-17: 패키지 및 import 구성이 적절합니다.테스트에 필요한 라이브러리(Kotest, FixtureMonkey)와 도메인 클래스들이 잘 가져와져 있습니다. DescribeSpec을 활용한 BDD 스타일의 테스트와 assertion을 위한 다양한 matcher들이 잘 준비되어 있습니다.
18-24: 테스트 클래스 선언 및 FixtureMonkey 설정이 적절합니다.DisplayName 애노테이션을 통해 테스트 목적을 명확히 표현하고 있으며, FixtureMonkey 설정도 KotlinPlugin을 사용하여 Kotlin 객체 생성에 적합하게 구성되어 있습니다.
25-42: Connection 생성 테스트가 잘 구현되어 있습니다.두 사용자 간의 Connection 생성 테스트가 잘 작성되었습니다. match 메서드를 통해 생성된 Connection 객체의 속성들이 기대값과 일치하는지 검증하고 있습니다.
44-77: Connection 취소 테스트가 충분히 구현되어 있습니다.Connection 취소 기능에 대한 테스트가 두 가지 시나리오(상세 정보 있음/없음)에 대해 잘 작성되어 있습니다. cancel 메서드 호출 후 cancellation 객체의 속성들이 기대값과 일치하는지 검증하고 있습니다.
79-104: isParticipant 메서드 테스트가 포괄적으로 작성되어 있습니다.사용자가 Connection의 참여자인지 확인하는 isParticipant 메서드에 대한 테스트가 세 가지 시나리오(participant1인 경우, participant2인 경우, 참여자가 아닌 경우)에 대해 잘 작성되어 있습니다.
106-112: Id 클래스 상속 테스트가 적절합니다.Connection.Id 클래스가 UUIDTypeId를 상속받는지 검증하는 테스트가 잘 작성되어 있습니다. 이를 통해 ID 타입의 일관성을 확인할 수 있습니다.
|
2 participants
Summary by CodeRabbit
신규 기능
테스트
유지 관리