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

서버가 클라이언트에게 Ping/Pong 요청 #776

Merged
merged 8 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -7,11 +7,12 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.USER_ID;

@Getter
public class WebSocketSessions {

protected static final String CHAT_ROOM_ID_KEY = "chatRoomId";
private static final String USER_ID_KEY = "userId";

private final Set<WebSocketSession> sessions = Collections.newSetFromMap(new ConcurrentHashMap<>());

Expand All @@ -24,7 +25,7 @@ public void putIfAbsent(final WebSocketSession session, final Long chatRoomId) {

public boolean contains(final Long userId) {
return sessions.stream()
.anyMatch(session -> session.getAttributes().get(USER_ID_KEY) == userId);
.anyMatch(session -> session.getAttributes().get(USER_ID.getName()) == userId);
}

public void remove(final WebSocketSession session) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,25 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;
import org.springframework.web.socket.PingMessage;
import org.springframework.web.socket.TextMessage;
import org.springframework.web.socket.WebSocketSession;

import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.USER_ID;

@Slf4j
@Component
@RequiredArgsConstructor
public class ChatWebSocketHandleTextMessageProvider implements WebSocketHandleTextMessageProvider {
Expand Down Expand Up @@ -115,7 +124,7 @@ private TextMessage createTextMessage(
}

private boolean isMyMessage(final WebSocketSession session, final Long writerId) {
final long userId = Long.parseLong(String.valueOf(session.getAttributes().get("userId")));
final long userId = Long.parseLong(String.valueOf(session.getAttributes().get(USER_ID.getName())));

return writerId.equals(userId);
}
Expand All @@ -136,6 +145,39 @@ private void updateReadMessageLog(

@Override
public void remove(final WebSocketSession session) {
log.info("{} 연결 종료", session);
sessions.remove(session);
}

@Scheduled(fixedDelay = 60000)
public void sendPingSessions() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

칭찬

스케줄러를 잘 사용해주셨네요 👍👍🏻

final Set<WebSocketSession> webSocketSessions = getWebSocketSessions();

webSocketSessions.parallelStream()
.forEach(this::sendPingMessage);
}

private Set<WebSocketSession> getWebSocketSessions() {
return sessions.getChatRoomSessions()
.values()
.stream()
.flatMap(webSocketSessions -> webSocketSessions.getSessions().stream())
Copy link
Collaborator

Choose a reason for hiding this comment

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

칭찬

오! flatMap을 사용해주셨네요 👍🏻

.collect(Collectors.toSet());
}

private void sendPingMessage(final WebSocketSession session) {
final Map<String, Object> attributes = session.getAttributes();
final boolean connected = (boolean) attributes.get(CONNECTED.getName());
if (!connected) {
sessions.remove(session);
return;
}

attributes.put(CONNECTED.getName(), false);
try {
session.sendMessage(new PingMessage());
} catch (IOException e) {
log.error("ping 보내기 실패 : {} ", session);
Copy link
Collaborator

Choose a reason for hiding this comment

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

질문

ping 전송에 실패하면 로그만 찍고 다시 ping을 전송하지 않아도 괜찮을까요?
저는 ping 전송에 실패한 경우 정해둔 시간 동안 다시 ping 전송 요청을 해보는 것을 생각했습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

저는 ping이 실패한 경우 다시 ping 요청을 수행하더라도 실패할 것이라 생각했습니다.
네트워크, 세션 종료, 메시지 등에 대한 문제로 일어날 것으로 예상됩니다.
그래서 다시 시도해도 동일한 상황 및 메시지로는 계속 실패할 것이라는 생각이 드는 데 어떻게 생각하실까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

실패한 로직을 재전송하는 부분이 필요할 것 같다는 생각이 들었어요!
여기 뿐만 아니라 알림 전송도 비슷한 기능이 있어야 한다고 생각합니다.
이건 나중에 논의해보고 이슈 새로 파서 진행하면 좋을 것 같아요~!

Copy link
Member Author

Choose a reason for hiding this comment

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

다음 회의에서 이에 대해 한번 논의해 보기로 해요!
그 전까지 sendMessage()가 실패하는 원인에 대해 좀 더 알아보도록 하겠습니다!
그래서 일단 이번 pr에서는 해당 부분은 적용하지 않고 넘어가도록 하겠습니다.

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@

import java.util.Map;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.BASE_URL;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.USER_ID;

@Component
@RequiredArgsConstructor
public class WebSocketInterceptor extends HttpSessionHandshakeInterceptor {
Expand All @@ -27,8 +31,9 @@ public boolean beforeHandshake(
final WebSocketHandler wsHandler,
final Map<String, Object> attributes
) throws Exception {
attributes.put("userId", findUserId(request));
attributes.put("baseUrl", ImageRelativeUrl.USER.calculateAbsoluteUrl());
attributes.put(USER_ID.getName(), findUserId(request));
attributes.put(BASE_URL.getName(), ImageRelativeUrl.USER.calculateAbsoluteUrl());
attributes.put(CONNECTED.getName(), true);

return super.beforeHandshake(request, response, wsHandler, attributes);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,29 @@
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import org.springframework.web.socket.CloseStatus;
import org.springframework.web.socket.PongMessage;
import org.springframework.web.socket.TextMessage;
import org.springframework.web.socket.WebSocketSession;
import org.springframework.web.socket.handler.TextWebSocketHandler;

import java.util.List;
import java.util.Map;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.TYPE;

@Component
@RequiredArgsConstructor
public class WebSocketHandler extends TextWebSocketHandler {

private static final String TYPE_KEY = "type";

private final WebSocketHandleTextMessageProviderComposite providerComposite;
private final ObjectMapper objectMapper;

@Override
protected void handleTextMessage(final WebSocketSession session, final TextMessage message) throws Exception {
final String payload = message.getPayload();
final TextMessageDto textMessageDto = objectMapper.readValue(payload, TextMessageDto.class);
session.getAttributes().put(TYPE_KEY, textMessageDto.type());
session.getAttributes().put(TYPE.getName(), textMessageDto.type());

final WebSocketHandleTextMessageProvider provider = providerComposite.findProvider(textMessageDto.type());
final List<SendMessageDto> sendMessageDtos = provider.handleCreateSendMessage(session, textMessageDto.data());
Expand All @@ -38,9 +41,15 @@ protected void handleTextMessage(final WebSocketSession session, final TextMessa

@Override
public void afterConnectionClosed(final WebSocketSession session, final CloseStatus status) {
final String type = String.valueOf(session.getAttributes().get(TYPE_KEY));
final String type = String.valueOf(session.getAttributes().get(TYPE.getName()));
final TextMessageType textMessageType = TextMessageType.valueOf(type);
final WebSocketHandleTextMessageProvider provider = providerComposite.findProvider(textMessageType);
provider.remove(session);
}

@Override
public void handlePongMessage(WebSocketSession session, PongMessage message) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

필수

제 눈에는 왜 Final이 이렇게 잘 보이는 걸까요..?

final Map<String, Object> attributes = session.getAttributes();
attributes.put(CONNECTED.getName(), true);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.ddang.ddang.websocket.handler.dto;

import lombok.Getter;

@Getter
public enum WebSocketAttributeKey {

USER_ID("userId"),
BASE_URL("baseUrl"),
CONNECTED("connected"),
TYPE("type");

private final String name;

WebSocketAttributeKey(final String name) {
this.name = name;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,16 @@
import java.util.HashMap;
import java.util.Map;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.BASE_URL;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.USER_ID;

@SuppressWarnings("NonAsciiCharacters")
public class WebSocketSessionsTestFixture {

protected Long 사용자_아이디 = 1L;
protected Map<String, Object> 세션_attribute_정보 = new HashMap<>(Map.of("userId", 사용자_아이디, "baseUrl", "/images"));
protected Map<String, Object> 세션_attribute_정보 = new HashMap<>(
Map.of(USER_ID.getName(), 사용자_아이디, BASE_URL.getName(), "/images", CONNECTED.getName(), true)
);
protected Long 채팅방_아이디 = 1L;
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.ddang.ddang.chat.application.event.MessageNotificationEvent;
import com.ddang.ddang.chat.application.event.UpdateReadMessageLogEvent;
import com.ddang.ddang.chat.domain.WebSocketChatSessions;
import com.ddang.ddang.chat.domain.WebSocketSessions;
import com.ddang.ddang.chat.domain.repository.ReadMessageLogRepository;
import com.ddang.ddang.chat.handler.fixture.ChatWebSocketHandleTextMessageProviderTestFixture;
import com.ddang.ddang.configuration.IsolateDatabase;
Expand All @@ -22,16 +23,22 @@
import org.springframework.boot.test.mock.mockito.SpyBean;
import org.springframework.test.context.event.ApplicationEvents;
import org.springframework.test.context.event.RecordApplicationEvents;
import org.springframework.web.socket.PingMessage;
import org.springframework.web.socket.WebSocketSession;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.mockito.BDDMockito.willDoNothing;
import static org.mockito.BDDMockito.willReturn;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;

@IsolateDatabase
@RecordApplicationEvents
Expand All @@ -48,6 +55,9 @@ class ChatWebSocketHandleTextMessageProviderTest extends ChatWebSocketHandleText
@SpyBean
WebSocketChatSessions sessions;

@SpyBean
WebSocketSessions webSocketSessions;

@Mock
WebSocketSession writerSession;

Expand Down Expand Up @@ -186,4 +196,35 @@ class ChatWebSocketHandleTextMessageProviderTest extends ChatWebSocketHandleText
final boolean actual = sessions.containsByUserId(채팅방.getId(), 발신자.getId());
assertThat(actual).isFalse();
}

@Test
void 저장되어_있는_세션에_ping_메시지를_보낸다() throws IOException {
// given
given(writerSession.getAttributes()).willReturn(발신자_세션_attribute_정보);
given(webSocketSessions.getSessions()).willReturn(Set.of(writerSession));
given(sessions.getChatRoomSessions()).willReturn(Map.of(채팅방.getId(), webSocketSessions));

// when
provider.sendPingSessions();

// then
verify(sessions, never()).remove(writerSession);
verify(writerSession, times(1)).sendMessage(new PingMessage());
}

@Test
void 연결이_끊긴_세션은_삭제한다() throws IOException {
// given
given(writerSession.getAttributes()).willReturn(연결이_끊긴_세션_attribute_정보);
given(webSocketSessions.getSessions()).willReturn(Set.of(writerSession));
given(sessions.getChatRoomSessions()).willReturn(Map.of(채팅방.getId(), webSocketSessions));
willDoNothing().given(sessions).remove(writerSession);

// when
provider.sendPingSessions();

// then
verify(sessions, times(1)).remove(writerSession);
verify(writerSession, never()).sendMessage(new PingMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.ddang.ddang.chat.application.event.CreateReadMessageLogEvent;
import com.ddang.ddang.chat.domain.ChatRoom;
import com.ddang.ddang.chat.domain.repository.ChatRoomRepository;
import com.ddang.ddang.chat.domain.repository.ReadMessageLogRepository;
import com.ddang.ddang.image.domain.ProfileImage;
import com.ddang.ddang.user.domain.Reliability;
import com.ddang.ddang.user.domain.User;
Expand All @@ -22,6 +21,10 @@
import java.util.HashMap;
import java.util.Map;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.BASE_URL;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.USER_ID;

@SuppressWarnings("NonAsciiCharacters")
public class ChatWebSocketHandleTextMessageProviderTestFixture {

Expand All @@ -46,6 +49,7 @@ public class ChatWebSocketHandleTextMessageProviderTestFixture {

protected Map<String, Object> 발신자_세션_attribute_정보;
protected Map<String, Object> 수신자_세션_attribute_정보;
protected Map<String, Object> 연결이_끊긴_세션_attribute_정보;
protected Map<String, String> 메시지_전송_데이터;

protected CreateReadMessageLogEvent 메시지_로그_생성_이벤트;
Expand Down Expand Up @@ -86,8 +90,21 @@ void setUpFixture() {

chatRoomRepository.save(채팅방);

발신자_세션_attribute_정보 = new HashMap<>(Map.of("userId", 발신자.getId(), "baseUrl", "/images"));
수신자_세션_attribute_정보 = new HashMap<>(Map.of("userId", 수신자.getId(), "baseUrl", "/images"));
발신자_세션_attribute_정보 = new HashMap<>(Map.of(
USER_ID.getName(), 발신자.getId(),
BASE_URL.getName(), "/images",
CONNECTED.getName(), true
));
수신자_세션_attribute_정보 = new HashMap<>(Map.of(
USER_ID.getName(), 수신자.getId(),
BASE_URL.getName(), "/images",
CONNECTED.getName(), true
));
연결이_끊긴_세션_attribute_정보 = new HashMap<>(Map.of(
USER_ID.getName(), 수신자.getId(),
BASE_URL.getName(), "/images",
CONNECTED.getName(), false
));
메시지_전송_데이터 = Map.of(
"chatRoomId", String.valueOf(채팅방.getId()),
"receiverId", String.valueOf(수신자.getId()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
import java.util.List;
import java.util.Map;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.BASE_URL;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.USER_ID;

@SuppressWarnings("NonAsciiCharacters")
public class NotificationEventListenerFixture {

Expand Down Expand Up @@ -117,8 +121,9 @@ void setUpFixture() {
bidRepository.save(bid);

세션_attribute_정보 = new HashMap<>(Map.of(
"userId", 발신자_겸_판매자.getId(),
"baseUrl", 이미지_절대_경로
USER_ID.getName(), 발신자_겸_판매자.getId(),
BASE_URL.getName(), 이미지_절대_경로,
CONNECTED.getName(), true
));
메시지_전송_데이터 = Map.of(
"chatRoomId", String.valueOf(채팅방.getId()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,14 @@
import org.mockito.Mock;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.web.socket.PongMessage;
import org.springframework.web.socket.TextMessage;
import org.springframework.web.socket.WebSocketSession;

import java.util.List;

import static com.ddang.ddang.websocket.handler.dto.WebSocketAttributeKey.CONNECTED;
import static org.assertj.core.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.BDDMockito.given;
Expand Down Expand Up @@ -69,4 +72,16 @@ class WebSocketHandlerTest extends WebSocketHandlerTestFixture {
// then
verify(provider, times(1)).remove(any(WebSocketSession.class));
}

@Test
void pong_메시지_수신시_연결_상태를_참으로_변환한다() {
// given
given(session.getAttributes()).willReturn(세션_attribute_정보);

// when
webSocketHandler.handlePongMessage(session, new PongMessage());

// then
assertThat((boolean) 세션_attribute_정보.get(CONNECTED.getName())).isTrue();
}
}
Loading
Loading