-
Notifications
You must be signed in to change notification settings - Fork 1
WebSocket 세션 처리 #102
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
base: develop
Are you sure you want to change the base?
WebSocket 세션 처리 #102
Conversation
- WebSocket 연결 시 기존 HTTP 세션을 WebSocket 세션에 저장하는 인터셉터 구현 - WebSocket 요청 시 세션 정보를 활용할 수 있도록 설정 추가 Resolves: #100
- 기존에 senderId로 직접 사용자 정보를 보내던 방식에서 세션을 이용하여 현재 로그인된 사용자 정보를 바로 전송하는 방식으로 변경 Resolves: #100
- ChatMessageService에서 HttpServletRequest 대신 SignInReponse DTO를 사용하도록 변경 - WebSocket과 REST API에서 세션처리 방법이 달라 임시로 읽음 처리 기능만 작동하도록 구현 Resolves: #100
- 기존 채팅방을 반환하는 경우도 "방이 성공적으로 생성되었습니다." 가 응답되던 문제 수정 - 기존 채팅방을 반환하는 경우 "기존 채팅방을 불러왔습니다."로 응답하도록 변경 Resolves: #100
| private final ChatRoomService chatRoomService; | ||
|
|
||
| // 방 생성 | ||
| @PostMapping() |
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.
/api/chat/room 엔드포인트에 POST 요청을 보내면 방 생성 엔드포인트로 괜찮을거라 생각했었는데 다시 보니 /create 등으로 지정하는게 더 좋을 것 같습니다!
|
|
||
| // 구독 목록에 추가 | ||
| subscribeUserToChatRoom(buyer, newRoom); | ||
| subscribeUserToChatRoom(seller, newRoom); |
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.
새로운 Room을 생성하고, 판매자와 구매자를 추가한다.
라는 부분은 별도의 메소드나 클래스로 관리하는 편이 좋지 않을까 하는 생각도 드는데, 어떻게 생각하실까요?
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.
방을 생성하는 로직이 이 부분 말고 쓰이는 곳이 없어서 따로 분리하여 관리는 안 했는데, createNewRoom() 같은 메서드로 분리하면 해당 로직을 읽기 편해질 것 같다고 생각합니다!
JinTaekLim
left a comment
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.
매개변수로 Pair 를 사용하는 부분이 여럿 확인되는데, 사용하신 이유나 의도가 궁금합니다!
|
Pair는 방 생성 API에만 사용하고 있는데, 해당 API가 방이 존재하면 기존에 있던 방을 불러오는데 응답이 "방이 생성되어있습니다."로 고정이 되어있습니다. |
-> 코드 전체적인 통일성 및 이후 유지보수를 생각해 Pair은 사용하지 않는 것으로 결정 |
- 기존에는 Pair 타입을 이용하여 두 가지 응답값을 보냄 - DTO에 두 가지 응답값을 모두 포함하도록 변경 Resolves: #100
#️⃣ Issue Number
#100
📝 작업 설명
웹 소켓에서 사용자 정보가 필요한 부분에 세션을 받아와서 사용할 수 있도록 수정했습니다.
📸스크린샷 (선택)
💬 리뷰 요구사항
리뷰어가 중점적으로 봐줬으면 좋겠는 부분이 있으면 적어주세요.
논의해야할 부분이 있다면 적어주세요.
ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?
✅ PR Checklist
PR이 다음 요구 사항을 충족하는지 확인하세요.