-
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
Refactor/#165: 채팅 메시지 도메인 리팩토링 #171
base: develop
Are you sure you want to change the base?
The head ref may contain hidden characters: "refactor/#165/\uCC44\uD305-\uBA54\uC2DC\uC9C0-\uB3C4\uBA54\uC778-\uB9AC\uD329\uD1A0\uB9C1"
Changes from all commits
c6f25fd
88e15b7
81d9097
443a2af
f2295bd
f4e05e4
9be06d2
8d57a15
1f239be
800e0ad
c1edef3
2d64406
06a4167
757d0db
234ab5f
de4ee39
ebf1898
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package space.space_spring.domain.chat.chatroom.model; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Objects; | ||
import java.util.function.BiFunction; | ||
import java.util.function.Function; | ||
import space.space_spring.domain.chat.chatroom.model.dto.LastMessageInfoDto; | ||
import space.space_spring.domain.chat.chatroom.model.response.ChatRoomResponse; | ||
|
||
public class ChatRooms { | ||
|
||
private List<ChatRoom> chatRooms; | ||
|
||
private ChatRooms(List<ChatRoom> chatRooms) { | ||
this.chatRooms = chatRooms; | ||
} | ||
|
||
public static ChatRooms of(List<ChatRoom> chatRooms) { | ||
return new ChatRooms(chatRooms); | ||
} | ||
|
||
public List<ChatRoomResponse> toChatRoomResponses( | ||
Long userId, | ||
Function<ChatRoom, LastMessageInfoDto> lastMessageFinder, | ||
BiFunction<Long, ChatRoom, Integer> unreadMessageCounter) { | ||
return chatRooms.stream() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오오 함수형 인터페이스, 람다표현식, 스트림 까지 고급 자바 기술들의 집합체네요. 코드를 보고 개인적으로 자바8에 대한 공부가 많이 필요하다고 느꼈습니다 하하 음 제안드리고 싶은 사안이 있는데, 혹시 return type을 List 에서 DTOs 일급 컬렉션으로 변경하는건 어떤가요?? 이러면 toChatRoomResponses 메서드의 책임을 ChatRooms 가 가지고 있는 CharRoom 엔티티들 중 필요한 정보를 뽑아낸다 라는 것으로 국한시키고, 뽑아낸 정보들을 반환하는 것에 대한 책임을 일급컬렉션으로 위임할 수 있지않을까 싶습니다. |
||
.map(chatRoom -> { | ||
LastMessageInfoDto lastMessageInfo = lastMessageFinder.apply(chatRoom); | ||
LocalDateTime lastUpdateTime = lastMessageInfo.getLastUpdateTime(); | ||
HashMap<String, String> lastContent = lastMessageInfo.getLastContent(); | ||
|
||
int unreadMsgCount = unreadMessageCounter.apply(userId, chatRoom); | ||
|
||
return ChatRoomResponse.create(chatRoom, lastContent, String.valueOf(lastUpdateTime), | ||
unreadMsgCount); | ||
}) | ||
.filter(Objects::nonNull) | ||
.toList(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,23 +1,23 @@ | ||
package space.space_spring.entity; | ||
package space.space_spring.domain.chat.chatroom.model; | ||
|
||
import jakarta.annotation.Nullable; | ||
import jakarta.persistence.*; | ||
import lombok.AllArgsConstructor; | ||
import lombok.AccessLevel; | ||
import lombok.Builder; | ||
import lombok.Getter; | ||
import lombok.NoArgsConstructor; | ||
import org.hibernate.annotations.Comment; | ||
|
||
import java.time.LocalDateTime; | ||
import space.space_spring.entity.BaseEntity; | ||
import space.space_spring.entity.User; | ||
|
||
@Entity | ||
@Getter | ||
@NoArgsConstructor | ||
@AllArgsConstructor | ||
@Builder | ||
@NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
@Comment("유저별 채팅방") | ||
@Table(name = "User_Chat_Room") | ||
public class UserChatRoom extends BaseEntity{ | ||
public class UserChatRoom extends BaseEntity { | ||
|
||
@Id | ||
@GeneratedValue | ||
|
@@ -39,6 +39,14 @@ public class UserChatRoom extends BaseEntity{ | |
@Column(name = "last_read_time") | ||
private LocalDateTime lastReadTime; | ||
|
||
@Builder(access = AccessLevel.PRIVATE) | ||
private UserChatRoom(Long id, ChatRoom chatRoom, User user, @Nullable LocalDateTime lastReadTime) { | ||
this.id = id; | ||
this.chatRoom = chatRoom; | ||
this.user = user; | ||
this.lastReadTime = lastReadTime; | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 부분도 ChatRoom 과 동일하게 엔티티에 유효성 검증하는 코드를 작성하신 이유가 궁금합니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 위와 같은 이슈입니다. 이 부분도 수정하겠습니다! |
||
public static UserChatRoom of(ChatRoom chatRoom, User user, LocalDateTime lastReadTime) { | ||
return UserChatRoom.builder() | ||
.chatRoom(chatRoom) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package space.space_spring.domain.chat.chatroom.model; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import space.space_spring.entity.User; | ||
|
||
public class UserChatRooms { | ||
|
||
private final List<UserChatRoom> userChatRooms; | ||
|
||
private UserChatRooms(List<UserChatRoom> userChatRooms) { | ||
this.userChatRooms = Collections.unmodifiableList(userChatRooms); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 UserChatRooms 일급컬렉션 생성 시에 관련 UserChatRoom 엔티티의 list를 모두 인자로 주입해주고, list 컬렉션을 읽기 전용으로 제한하셨군요! |
||
|
||
public static UserChatRooms of(List<UserChatRoom> userChatRooms) { | ||
return new UserChatRooms(userChatRooms); | ||
} | ||
|
||
public boolean isUserJoined(Long userId) { | ||
return userChatRooms.stream().anyMatch(userChatRoom -> userChatRoom.getUser().getUserId().equals(userId)); | ||
} | ||
|
||
public List<User> getUsers() { | ||
return userChatRooms.stream() | ||
.map(UserChatRoom::getUser) | ||
.collect(Collectors.toUnmodifiableList()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LGTM |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package space.space_spring.domain.chat.chatroom.model.dto; | ||
|
||
import lombok.AccessLevel; | ||
import lombok.Builder; | ||
import lombok.Getter; | ||
|
||
import java.time.LocalDateTime; | ||
import java.util.HashMap; | ||
|
||
@Getter | ||
public class LastMessageInfoDto { | ||
|
||
private LocalDateTime lastUpdateTime; | ||
|
||
private HashMap<String, String> lastContent; | ||
|
||
@Builder(access = AccessLevel.PRIVATE) | ||
private LastMessageInfoDto(LocalDateTime lastUpdateTime, HashMap<String, String> lastContent) { | ||
this.lastUpdateTime = lastUpdateTime; | ||
this.lastContent = lastContent; | ||
} | ||
|
||
public static LastMessageInfoDto of(LocalDateTime lastUpdateTime, HashMap<String, String> lastContent) { | ||
return LastMessageInfoDto.builder() | ||
.lastUpdateTime(lastUpdateTime) | ||
.lastContent(lastContent) | ||
.build(); | ||
} | ||
} | ||
|
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.
여기서 @nullable 어노테이션을 이용한 인자 유효성 검증을 위한 코드를 왜 추가하셨는지 궁금합니다!
ChatRoomController에서 HTTP request를 받는 입력 모델에서의 유효성 검증과정이 있고, 이 유효성 검증을 통과한 입력 모델이 서비스단으로 전달되고, 이 모델의 정보를 바탕으로 엔티티를 생성하는 로직인거 같은데
제 생각으로는 엔티티에서도 검증을 위한 코드가 있어야할까? 싶긴합니다
그리고 추가적으로 컨트롤러의 입력모델에서는 @NotNull 인데 엔티티에서는 @nullable 인 것 같습니다. 확인부탁드려요~~
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.
아 채팅방 생성 시 이미지의 필수 여부가 개발 과정에 변경되었던 것 같아요.
초기에는 이미지가 필수가 아닌 것으로 설계해서 엔티티에
@Nullable
로 설정했는데,이미지도 필수인 것으로 바뀌면서 반영이 안되었네요.
엔티티의
img
필드가@Nullable
임에 따라 생성자에도 해당 어노테이션을 추가한 거라,엔티티의
img
필드를@NotNull
로 변경하면 해결될 것 같습니다~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.
그럼 혹시 해당 엔티티의 생성시 bean validation 을 어겼을 경우에 대한 후처리 로직도 있을까요??
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.
흐름상
RequestDto
에서validation
진행 후controller
에서 이에 대한bindingResult
가 올바른지 확인함으로써API 요청을 통한 엔티티 생성 시
validatioin
처리가 되어있습니다!말씀하시는 내용이 엔티티 자체적으로
validation
에 대한 처리가 있는지에 대한 것일까요?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.
넵 엔티티 자체적으로 validation이 있어야하지 않을까? 라는 생각으로 답글을 단 것이었는데
다시 생각해보니 앞에서 이미 걸러진 친구들이기도하고, 혹시 여기서 문제가 발생할 경우에는 서버 내부 코드에서의 문제가 맞으니, 따로 validation 처리를 하는 것보다는 500 error 를 발생시키는것이 맞는건가? 싶네여
(뭔가 이 부분도 다같이 얘기해보면 좋을 거 같긴 합니다)
그러면 @NotNull 어노테이션을 controller에서 검증을 거친 인자에 대해서 엔티티 내부에서 한 번 더 안전장치를 거신 거라고 생각하면 될까요??
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.
음 의도했던 바는 안전장치라기보다는 ChatRoom 엔티티 자체의 특성을 반영한 설정을 한 것입니당
결론적으로 안전장치의 역할도 가능하구요!
이 부분은
repository
를 통해 DB에ChatRoom
엔티티를 저장할 때null
인 경우 500 error를 발생시키는 걸 말씀하시는건가요??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.
넵 맞습니다
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.
아하 이중으로 에러 처리를 해줘도 괜찮을 것 같아요 다음에 다른 분들이랑 다같이 얘기해봅시다!