-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(chat): declare chat domain #43
Conversation
Warning Rate limit exceeded@waterfogSW has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
개요Walkthrough이 풀 리퀘스트는 도메인 계층에 새로운 엔티티 클래스들을 추가했습니다. Changes
Sequence DiagramsequenceDiagram
participant User
participant ChatRoom
participant ChatMember
participant ChatMessage
User->>ChatRoom: 채팅방 생성
ChatRoom->>ChatMember: 멤버 추가
User->>ChatMessage: 메시지 전송
ChatMessage-->>ChatMessage: 메시지 상태 업데이트
토끼의 시
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (3)
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatRoom.kt (1)
13-13
: 테스트 용이성 개선 필요LocalDateTime.now()의 직접 사용은 테스트를 어렵게 만듭니다. Clock을 주입받아 시간을 제어할 수 있도록 수정이 필요합니다.
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt (2)
22-27
: 상태값에 대한 문서화 필요각 상태값의 의미와 전환 조건에 대한 문서화가 필요합니다.
다음과 같이 개선하는 것을 제안드립니다:
enum class Status { + /** 메시지가 전송됨 */ SENT, + /** 수신자의 기기에 도달함 */ DELIVERED, + /** 수신자가 메시지를 읽음 */ READ, + /** 메시지 전송 실패 */ FAILED, }
29-34
: 오류 처리 방식 개선 필요
- 각 실패 사유에 대한 문서화가 필요합니다.
UNKNOWN_ERROR
사용 시 실제 오류 정보가 손실될 수 있습니다.다음과 같이 개선하는 것을 제안드립니다:
enum class FailureReason { + /** 네트워크 연결 실패 또는 타임아웃 */ NETWORK_ERROR, + /** 존재하지 않는 수신자 */ INVALID_RECIPIENT, + /** 메시지 크기 제한 초과 */ MESSAGE_TOO_LARGE, - UNKNOWN_ERROR + /** 기타 오류 (추가 컨텍스트 필요) */ + UNKNOWN_ERROR; + + var additionalContext: String? = null }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMember.kt
(1 hunks)domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt
(1 hunks)domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatRoom.kt
(1 hunks)domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt
(1 hunks)
🔇 Additional comments (1)
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt (1)
19-19
: 구현이 적절합니다
TypeId
를 상속받아 UUID를 래핑하는 구현이 도메인 설계 패턴을 잘 따르고 있습니다.
domain/src/main/kotlin/com/threedays/domain/connection/entity/Connection.kt
Show resolved
Hide resolved
data class ChatRoom( | ||
override val id: Id, | ||
val connectionId: Connection.Id, | ||
val members: MutableList<ChatMember> = mutableListOf(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가변 컬렉션 사용에 대한 경고
public MutableList는 외부에서 직접 수정이 가능하여 도메인 규칙을 우회할 수 있습니다. 이는 애그리게이트의 불변성을 해칠 수 있는 심각한 설계 문제입니다.
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatRoom.kt
Outdated
Show resolved
Hide resolved
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt
Outdated
Show resolved
Hide resolved
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt (3)
8-17
: KDoc 문서화 추가 필요클래스와 속성에 대한 문서화가 누락되어 있습니다. 다음 사항들을 고려해주세요:
- 클래스 수준의 문서화
- 클래스의 책임과 역할
- Aggregate Root로서의 불변식(invariants)
- 주요 속성들에 대한 설명
- 특히
status
,failureReason
의 null 가능성과 의미다음과 같이 문서화를 추가하는 것을 제안드립니다:
+/** + * 채팅 메시지를 나타내는 Aggregate Root 엔티티 + * + * @property id 메시지의 고유 식별자 + * @property chatRoomId 메시지가 속한 채팅방 ID + * @property senderId 발신자 ID + * @property content 메시지 내용 + * @property status 메시지의 현재 상태 + * @property createdAt 메시지 생성 시간 + * @property updatedAt 메시지 상태 변경 시간 + * @property failureReason 실패 상태일 경우의 실패 사유 + */ data class ChatMessage( override val id: Id, val chatRoomId: ChatRoom.Id,
22-27
: 상태 정의 개선 필요현재 상태 정의에 대해 다음 개선사항을 고려해주세요:
- 각 상태에 대한 문서화 필요
- 중간 상태 고려 필요:
SENDING
- 전송 중인 상태DELETING
- 삭제 중인 상태- 상태 전이 규칙 정의 필요
다음과 같은 개선을 제안드립니다:
enum class Status { + /** 전송 중 */ + SENDING, + /** 전송됨 */ SENT, + /** 전달됨 */ DELIVERED, + /** 읽음 */ READ, + /** 실패 */ FAILED, + /** 삭제됨 */ + DELETED, }
29-34
: 실패 사유 상세화 필요현재 실패 사유가 너무 일반적입니다. 다음과 같은 구체적인 사유들을 추가로 고려해주세요:
- 서버 관련:
SERVER_ERROR
DATABASE_ERROR
- 클라이언트 관련:
CLIENT_TIMEOUT
INVALID_MESSAGE_FORMAT
- 보안 관련:
UNAUTHORIZED
BLOCKED_RECIPIENT
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt
(1 hunks)domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatRoom.kt
(1 hunks)openapi
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- openapi
🚧 Files skipped from review as they are similar to previous changes (1)
- domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatRoom.kt
🔇 Additional comments (1)
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt (1)
19-19
: 구현이 적절합니다
UUID를 사용한 타입 안전한 ID 구현이 잘 되어있습니다.
domain/src/main/kotlin/com/threedays/domain/chat/entity/ChatMessage.kt
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
Summary by CodeRabbit