Skip to content
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

Fix #51 유니크 추가 및 post send 구현 #51

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@

import com.gachtaxi.domain.members.entity.Members;
import com.gachtaxi.global.common.entity.BaseEntity;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.*;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand All @@ -18,6 +15,12 @@
@Entity
@SuperBuilder
@NoArgsConstructor(access = AccessLevel.PROTECTED)
@Table(
name = "chatting_participant",
uniqueConstraints = {
@UniqueConstraint(columnNames = {"members_id", "chatting_room_id"})
}
)
Comment on lines +18 to +23
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

멤버와 채팅방을 유니크로 묶었군요!!
데이터 무결성이 더 잘 지켜질 것 같습니당!!👍

public class ChattingParticipant extends BaseEntity {

@ManyToOne
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,13 @@ public void delete(long chattingRoomId) {
}

@Transactional
public void subscribeChatRoom(long roomId, SimpMessageHeaderAccessor accessor) {
public void postSubscribeChatroom(SimpMessageHeaderAccessor accessor) {
Long roomId = (Long) accessor.getSessionAttributes().get(CHAT_ROOM_ID);
Long senderId = (Long) accessor.getSessionAttributes().get(CHAT_USER_ID);

ChattingRoom chattingRoom = find(roomId);
Members members = memberService.findById(senderId);

accessor.getSessionAttributes().put(CHAT_ROOM_ID, roomId);
accessor.getSessionAttributes().put(CHAT_USER_NAME, members.getNickname());

if (chattingParticipantService.checkSubscription(chattingRoom, members)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,11 @@ public Message<?> preSend(Message<?> message, MessageChannel channel) {

return chatStrategyHandler.handle(message, accessor, channel);
}

@Override
public void postSend(Message<?> message, MessageChannel channel, boolean sent) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(message);

chatStrategyHandler.handle(message, accessor, channel, sent);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,14 @@ public Message<?> handle(Message<?> message, StompHeaderAccessor accessor, Messa
.orElse(defaultCommandStrategy)
.preSend(message, accessor, channel);
}

public void handle(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel, boolean sent) {
StompCommand command = accessor.getCommand();

stompCommandStrategies.stream()
.filter(strategy -> strategy.supports(command))
.findFirst()
.orElse(defaultCommandStrategy)
.postSend(message, accessor, channel, sent);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,9 @@ public boolean supports(StompCommand command) {
public Message<?> preSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel) {
return message;
}

@Override
public void postSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel, boolean sent) {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ public interface StompCommandStrategy {
boolean supports(StompCommand command);

Message<?> preSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel);

void postSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel, boolean sent);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StompSubscribeStrategy 클래스 말고는 해당 메서드를 구현하는 클래스는 없어보입니다.
그렇다면 StompCommandStrategy를 상속하는, PostSendableStompCommandStrategy 인터페이스를 따로 만들어서 해당 클래스에만 구현하도록 하는건 어떤가요?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StompSubscribeStrategy 이외 stomp 전략들이 postSend 행위를 항상 override해야하므로 주영님이 말씀하신 방법이 좀 더 좋을거 같아요!

다만, 다른 stomp전략들이 postSend가 필요하다면 상관없을 거 같습니다!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StompSubscribeStrategy 이외에도 postSend를 항상 override 해줘야한다는 점에서 저도 우석님의 의견처럼 추가적인 인터페이스 분리가 오히려 복잡성을 높일 수도 있지 않을까라는 생각이 있긴합니다,
모든 Stomp에서 postSend가 꼭 필요한 전략인지 검토해보고 명시적으로 postSend를 강제해주지 않는 것도 하나의 방법일거같습니당

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

차후에 다른 전략에서도 postSend를 사용해야하는 경우가 생긴다면 StompCommandStrategy 내부에서 preSend, postSend 2가지 메서드를 공통으로 관리하여 하위 전략들이 하나의 인터페이스를 구현하는게 관리가 용이할 것 같다는 생각이 들었습니다!
인터페이스 분리도 좋은 방법이지만, 해당 메서드들의 성격이나 역할이 거의 유사하기 때문에 StompCommandStrategy 내부에 있으면 좋을 것 같다는 생각이 들었어요!

그래서 현재는 StompCommandStrategy 내에 defalut 메서드로 구현을 해서 StompSubscribeStrategy 만 재정의를 해서 구현을 하고, 차후 필요에 의해 다른 전략에서도 postSend를 구현한다면 메서드만 재정의하면 되니까 추가 코드 없이 확장이 될 것 같은데 여러 분들 의견은 어떠신가요?? 일단 수정한 버전으로 푸시 해두겠습니다!

}
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,9 @@ public Message<?> preSend(Message<?> message, StompHeaderAccessor accessor, Mess

return message;
}

@Override
public void postSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel, boolean sent) {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public Message<?> preSend(Message<?> message, StompHeaderAccessor accessor, Mess

return message;
}

@Override
public void postSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel, boolean sent) {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public Message<?> preSend(Message<?> message, StompHeaderAccessor accessor, Mess

if (destination.startsWith(SUB_END_POINT)) {
Long roomId = Long.valueOf(destination.replace(SUB_END_POINT, ""));
chattingRoomService.subscribeChatRoom(roomId, accessor);
accessor.getSessionAttributes().put(CHAT_ROOM_ID, roomId);

return message;
}
Expand All @@ -45,5 +45,12 @@ public Message<?> preSend(Message<?> message, StompHeaderAccessor accessor, Mess

throw new ChatSubscribeException();
}

@Override
public void postSend(Message<?> message, StompHeaderAccessor accessor, MessageChannel channel, boolean sent) {
if (sent) {
chattingRoomService.postSubscribeChatroom(accessor);
}
}
}

Loading