-
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 #54 이미지 추가해서 반환 #54
base: dev
Are you sure you want to change the base?
The head ref may contain hidden characters: "feat/#47/\uC774\uBBF8\uC9C0-\uCD94\uAC00\uD574\uC11C-\uBC18\uD658"
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.
고생하셨어요!!
로직에 문제는 없어보입니당~~
String key = getKey(roomId); | ||
|
||
chatRoomRedisTemplate.opsForSet().add(key, senderId); | ||
chatRoomRedisTemplate.opsForHash().put(key, String.valueOf(senderId), profilePicture); |
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.
사진도 함께 저장하는군요!!
좋은 방식인 것 같습니다!!
} | ||
|
||
return size; | ||
return profileImageUrl.toString(); |
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.
return profileImageUrl == null ? null : profileImageUrl.toString()
위의 코드로 바꾸는건 어떻게 생각하시나요??
개인적으로 로직이 조금 더 한 눈에 보이는 것 같습니당!!
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.
Optional을 이용해서 아래와같이 수정해봤는데 이건 어떠세용??
return Optional.ofNullable(chatRoomRedisTemplate.opsForHash().get(key, String.valueOf(senderId)))
.map(Object::toString)
.orElse(null);
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.
고생하셨습니다 !
구독 처리를 preSend에서 postSend로 옮겨주면서,
입장 및 읽음 이벤트가 구독 처리 이후에 발생하도록 한 부분이 적절하게 수정된거같습니당
String key = getKey(roomId); | ||
|
||
chatRoomRedisTemplate.opsForSet().add(key, senderId); | ||
chatRoomRedisTemplate.opsForHash().put(key, String.valueOf(senderId), profilePicture); |
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.
기존 opsForSet을 활용하여 사용자 정보를 저장하던 방식을 opsForHash를 사용해서 사용자의 프로필 사진을 함께 저장하는 로직으로 변경된걸 확인했습니다 !
동시에 opsForHash를 사용할 경우에 관련 데이터가 삭제될때((매칭 종료 이후나 사용자에 의해 채팅방 퇴장), 올바르게 삭제 로직이 이루어지는지 테스트가 필요해보입니다
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.
이벤트 리스너를 구현을 해서, 웹소켓 구독 해제시 데이터가 제거 되도록 구현했고, 테스트도 완료했습니다!
📌 관련 이슈
관련 이슈 번호 #47
Close #
🚀 작업 내용
📸 스크린샷
📢 리뷰 요구사항