-
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
Fix #51 유니크 추가 및 post send 구현 #51
base: dev
Are you sure you want to change the base?
The head ref may contain hidden characters: "fix/#46/\uC720\uB2C8\uD06C-\uCD94\uAC00-postSend"
Conversation
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.
고생하셨어요!!
커멘트 확인 부탁드립니당~~
@Table( | ||
name = "chatting_participant", | ||
uniqueConstraints = { | ||
@UniqueConstraint(columnNames = {"members_id", "chatting_room_id"}) | ||
} | ||
) |
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.
멤버와 채팅방을 유니크로 묶었군요!!
데이터 무결성이 더 잘 지켜질 것 같습니당!!👍
@@ -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); |
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.
StompSubscribeStrategy
클래스 말고는 해당 메서드를 구현하는 클래스는 없어보입니다.
그렇다면 StompCommandStrategy
를 상속하는, PostSendableStompCommandStrategy
인터페이스를 따로 만들어서 해당 클래스에만 구현하도록 하는건 어떤가요?
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.
StompSubscribeStrategy
이외 stomp 전략들이 postSend 행위를 항상 override해야하므로 주영님이 말씀하신 방법이 좀 더 좋을거 같아요!
다만, 다른 stomp전략들이 postSend가 필요하다면 상관없을 거 같습니다!
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.
StompSubscribeStrategy 이외에도 postSend를 항상 override 해줘야한다는 점에서 저도 우석님의 의견처럼 추가적인 인터페이스 분리가 오히려 복잡성을 높일 수도 있지 않을까라는 생각이 있긴합니다,
모든 Stomp에서 postSend가 꼭 필요한 전략인지 검토해보고 명시적으로 postSend를 강제해주지 않는 것도 하나의 방법일거같습니당
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.
postSend를 추가해서 입장, 읽음과 구독의 실행 순서를 분리하는 코드로 이해했습니다!
문제 없어 보입니다 고생하셨어요!
@@ -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); |
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.
StompSubscribeStrategy
이외 stomp 전략들이 postSend 행위를 항상 override해야하므로 주영님이 말씀하신 방법이 좀 더 좋을거 같아요!
다만, 다른 stomp전략들이 postSend가 필요하다면 상관없을 거 같습니다!
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.
고생 많으셨습니다 !!
@@ -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); |
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.
StompSubscribeStrategy 이외에도 postSend를 항상 override 해줘야한다는 점에서 저도 우석님의 의견처럼 추가적인 인터페이스 분리가 오히려 복잡성을 높일 수도 있지 않을까라는 생각이 있긴합니다,
모든 Stomp에서 postSend가 꼭 필요한 전략인지 검토해보고 명시적으로 postSend를 강제해주지 않는 것도 하나의 방법일거같습니당
📌 관련 이슈
관련 이슈 번호 #46
Close #
🚀 작업 내용
📸 스크린샷
📢 리뷰 요구사항