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

웹소켓 채팅 리팩터링 #160

Merged
merged 6 commits into from
Dec 26, 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
@@ -1,5 +1,6 @@
package mafia.mafiatogether.chat.ui;

import java.util.List;
import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.chat.annotation.SendToChatWithRedis;
import mafia.mafiatogether.chat.application.ChatService;
Expand All @@ -15,8 +16,6 @@
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.GetMapping;

import java.util.List;

@Controller
@RequiredArgsConstructor
public class ChatController {
Expand Down Expand Up @@ -56,5 +55,4 @@ public Message createChat(
return chatService.chat(name, code, request.content());
}


}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package mafia.mafiatogether.common.config;

import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.common.resolver.PlayerArgumentResolver;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpHeaders;
Expand All @@ -10,11 +11,14 @@
import java.util.List;

@Configuration
@RequiredArgsConstructor
public class WebMvcConfig implements WebMvcConfigurer {

private final PlayerArgumentResolver playerArgumentResolver;

@Override
public void addArgumentResolvers(final List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(new PlayerArgumentResolver());
resolvers.add(playerArgumentResolver);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package mafia.mafiatogether.common.config;


import java.util.List;
import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.common.interceptor.StompChannelInterceptor;
import mafia.mafiatogether.common.interceptor.ChatInterceptor;
import mafia.mafiatogether.common.interceptor.PathMatcherInterceptor;
import mafia.mafiatogether.common.resolver.WebsocketPlayerArgumentResolver;
import org.springframework.context.annotation.Configuration;
import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
import org.springframework.messaging.simp.config.ChannelRegistration;
import org.springframework.messaging.simp.config.MessageBrokerRegistry;
import org.springframework.messaging.simp.stomp.StompCommand;
import org.springframework.web.socket.config.annotation.EnableWebSocketMessageBroker;
import org.springframework.web.socket.config.annotation.StompEndpointRegistry;
import org.springframework.web.socket.config.annotation.WebSocketMessageBrokerConfigurer;
Expand All @@ -15,7 +19,8 @@
@RequiredArgsConstructor
public class WebSocketConfig implements WebSocketMessageBrokerConfigurer {

private final StompChannelInterceptor stompChannelInterceptor;
private final ChatInterceptor chatInterceptor;
private final WebsocketPlayerArgumentResolver websocketPlayerArgumentResolver;

@Override
public void registerStompEndpoints(StompEndpointRegistry registry) {
Expand All @@ -28,6 +33,11 @@ public void registerStompEndpoints(StompEndpointRegistry registry) {
);
}

@Override
public void addArgumentResolvers(final List<HandlerMethodArgumentResolver> argumentResolvers) {
argumentResolvers.add(websocketPlayerArgumentResolver);
}

@Override
public void configureMessageBroker(MessageBrokerRegistry registry) {
registry.setApplicationDestinationPrefixes("/pub");
Expand All @@ -36,7 +46,12 @@ public void configureMessageBroker(MessageBrokerRegistry registry) {

@Override
public void configureClientInboundChannel(ChannelRegistration registry) {
registry.interceptors(stompChannelInterceptor);
registry.interceptors(
Copy link
Contributor Author

@thdwoqor thdwoqor Dec 4, 2024

Choose a reason for hiding this comment

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

코드 작성하면서 생각하다보니깐 제일 바꾸고 싶었던 영역은 이부분인거같아

private static final String SUBSCRIBE_FORMAT = "%s/%s";
private static final String PUBLISHING_FORMAT = "%s/%s/%s";

기존에는 인터셉터를 통해서 모든 요청에 대해서 위와 같은 URL 패턴이아니면 에러가 발생해서 저런 형태가 아닌 방식으로 pub/sub을 할때 문제가 생겼는데 PathMatcher 를 통해서 필요한 부분만 등록해주는게 필요하다고 생각했어

이전에 확장성 떨어진다고 말한부분이 이부분 이였어
현재 구조에서는 모든요청이 해당 인터셉터를 통과하는 문제

Copy link
Contributor

Choose a reason for hiding this comment

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

기존에는 인터셉터를 통해서 모든 요청에 대해서 위와 같은 URL 패턴이아니면 에러가 발생해서 저런 형태가 아닌 방식으로 pub/sub을 할때 문제가 생겼는데 PathMatcher 를 통해서 필요한 부분만 등록해주는게 필요하다고 생각했어

이 말이 구독을 위해서는 "sub/{code}/"이 형식이어야하고 발행은 "pub/{code}/{name}"이 형식이 어색하다는거고 이걸 PatchMatcher의 채인 메서드로 필요할때마다 추가해서 확장성을 높였다는 의미지?

난 이런 의미면 좋은거 같아

Copy link
Contributor Author

Choose a reason for hiding this comment

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

형식이 어색한것도 있지만 큰문제는

  1. 현재 구독,발행을 위해서 "sub/?/{auth}/" , "pub/?/{auth}" 형식이 강제 되는상황인데 이유는 인터셉터가 전역으로 설정되어있어서 그럼 ("sub/{auth}/hello" , "pub/bye" 이렇게 불가능)
  2. 그래서 인터셉터를 구독할때 현재처럼 전역으로 설정하는게아니라 PatchMatcher를 통해서 특정 url, 메서드에만 적용범위를 좁혀서 확장성을 높임 (sub/{auth}/hello, "pub/bye" 이런 형식으로 다양한 방식으로 여러 인터셉터 적용가능)

Copy link
Collaborator

Choose a reason for hiding this comment

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

여기 기가막힌데??
구우욷!

약간 이 패턴과 COMMAND에 걸리면 우리가 만든 Interceptor 로 보내주는거지?

    private final Map<String, ChannelInterceptor> channelInterceptors;
    private final PathMatcher pathMatcher;
    private final List<StompMapping> includePathPattern;
    private final List<StompMapping> excludePathPattern;

    public record Record(
            String key,
            ChannelInterceptor channelInterceptor
    ) {
    }

    public PathMatcherInterceptor(
            final Record... records
    ) {
        channelInterceptors = Arrays.stream(records).collect(
                Collectors.toMap(
                        Record::key,
                        Record::channelInterceptor
                )
        );
        this.pathMatcher = new AntPathMatcher();
        this.includePathPattern = new ArrayList<>();
        this.excludePathPattern = new ArrayList<>();
    }
...

그렇다면, 뭔가 이런 느낌으로 만들어주고, includePath 할 때 특정 키워드 넣어가지고 어떤 키워드에 매칭 되었을 때, channel Interceptor 에 할당할지도 결정하면 좋을 것 같아!

new PathMatcherInterceptor(chatInterceptor)
.includePathPattern("/sub/chat/**", StompCommand.SUBSCRIBE)
.includePathPattern("/pub/chat/**", StompCommand.MESSAGE)
.includePathPattern("/pub/chat/**", StompCommand.SEND)
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;
import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.common.util.AuthExtractor;
import org.springframework.context.annotation.Configuration;
import org.springframework.messaging.Message;
Expand All @@ -16,8 +15,7 @@

@Component
@Configuration
@RequiredArgsConstructor
public class StompChannelInterceptor implements ChannelInterceptor {
public class ChatInterceptor implements ChannelInterceptor {

private static final String SUBSCRIBE_FORMAT = "%s/%s";
private static final String PUBLISHING_FORMAT = "%s/%s/%s";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package mafia.mafiatogether.common.interceptor;

import java.util.ArrayList;
import java.util.List;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;
import org.springframework.messaging.simp.stomp.StompCommand;
import org.springframework.messaging.simp.stomp.StompHeaderAccessor;
import org.springframework.messaging.support.ChannelInterceptor;
import org.springframework.messaging.support.MessageHeaderAccessor;
import org.springframework.util.AntPathMatcher;
import org.springframework.util.PathMatcher;

public class PathMatcherInterceptor implements ChannelInterceptor {

private final ChannelInterceptor channelInterceptor;
private final PathMatcher pathMatcher;
private final List<StompMapping> includePathPatterns;
private final List<StompMapping> excludePathPatterns;

public PathMatcherInterceptor(final ChannelInterceptor channelInterceptor) {
this.channelInterceptor = channelInterceptor;
this.pathMatcher = new AntPathMatcher();
this.includePathPatterns = new ArrayList<>();
this.excludePathPatterns = new ArrayList<>();
}

@Override
public Message<?> preSend(final Message<?> message, final MessageChannel channel) {
StompHeaderAccessor headerAccessor = MessageHeaderAccessor.getAccessor(message, StompHeaderAccessor.class);

if (headerAccessor != null &&
headerAccessor.getDestination() != null &&
thdwoqor marked this conversation as resolved.
Show resolved Hide resolved
shouldIntercept(headerAccessor.getDestination(), headerAccessor.getCommand())) {
return channelInterceptor.preSend(message, channel);
}

return ChannelInterceptor.super.preSend(message, channel);
thdwoqor marked this conversation as resolved.
Show resolved Hide resolved
}

private boolean shouldIntercept(String destination, StompCommand command) {
boolean isExcluded = excludePathPatterns.stream()
.anyMatch(stompMapping -> matchesPathAndCommand(destination, command, stompMapping));

boolean isIncluded = includePathPatterns.stream()
.anyMatch(stompMapping -> matchesPathAndCommand(destination, command, stompMapping));

return isIncluded && !isExcluded;
}

private boolean matchesPathAndCommand(String destination, StompCommand command, StompMapping stompMapping) {
return pathMatcher.match(stompMapping.destination(), destination) &&
stompMapping.command() == command;
}

public PathMatcherInterceptor includePathPattern(String targetPath, StompCommand command) {
this.includePathPatterns.add(new StompMapping(targetPath, command));
return this;
}

public PathMatcherInterceptor excludePathPattern(String targetPath, StompCommand command) {
this.excludePathPatterns.add(new StompMapping(targetPath, command));
return this;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package mafia.mafiatogether.common.interceptor;

import org.springframework.messaging.simp.stomp.StompCommand;

public record StompMapping(
String destination,
StompCommand command
) {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package mafia.mafiatogether.common.resolver;

import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.common.exception.AuthException;
import mafia.mafiatogether.common.exception.ExceptionCode;
import mafia.mafiatogether.common.util.AuthExtractor;
import org.springframework.stereotype.Component;

@Component
public class BasicAuthResolver {

public String[] resolve(final String authorization) {
if (authorization == null) {
throw new AuthException(ExceptionCode.MISSING_AUTHENTICATION_HEADER);
}
if (!authorization.startsWith("Basic")) {
throw new AuthException(ExceptionCode.MISSING_AUTHENTICATION_HEADER);
}

return AuthExtractor.extractByAuthorization(authorization);
}
}
Original file line number Diff line number Diff line change
@@ -1,19 +1,23 @@
package mafia.mafiatogether.common.resolver;

import jakarta.servlet.http.HttpServletRequest;

import mafia.mafiatogether.common.util.AuthExtractor;
import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.common.annotation.PlayerInfo;
import mafia.mafiatogether.common.exception.AuthException;
import mafia.mafiatogether.common.exception.ExceptionCode;
import org.springframework.core.MethodParameter;
import org.springframework.stereotype.Component;
import org.springframework.web.bind.support.WebDataBinderFactory;
import org.springframework.web.context.request.NativeWebRequest;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.method.support.ModelAndViewContainer;

@Component
@RequiredArgsConstructor
public class PlayerArgumentResolver implements HandlerMethodArgumentResolver {

private final BasicAuthResolver basicAuthResolver;

@Override
public boolean supportsParameter(final MethodParameter parameter) {
return parameter.hasParameterAnnotation(PlayerInfo.class);
Expand All @@ -23,17 +27,12 @@ public boolean supportsParameter(final MethodParameter parameter) {
public PlayerInfoDto resolveArgument(final MethodParameter parameter, final ModelAndViewContainer mavContainer,
final NativeWebRequest webRequest, final WebDataBinderFactory binderFactory) {
HttpServletRequest httpServletRequest = webRequest.getNativeRequest(HttpServletRequest.class);

if (httpServletRequest == null) {
throw new AuthException(ExceptionCode.NOT_FOUND_REQUEST);
}

String authorization = httpServletRequest.getHeader("Authorization");
if (authorization == null) {
throw new AuthException(ExceptionCode.MISSING_AUTHENTICATION_HEADER);
}

String[] information = AuthExtractor.extractByAuthorization(authorization);
String[] information = basicAuthResolver.resolve(authorization);
return new PlayerInfoDto(information[0], information[1]);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package mafia.mafiatogether.common.resolver;

import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.common.annotation.PlayerInfo;
import org.springframework.core.MethodParameter;
import org.springframework.messaging.Message;
import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class WebsocketPlayerArgumentResolver implements HandlerMethodArgumentResolver {

private final BasicAuthResolver basicAuthResolver;

@Override
public boolean supportsParameter(final MethodParameter parameter) {
return parameter.hasParameterAnnotation(PlayerInfo.class);
}

@Override
public Object resolveArgument(final MethodParameter parameter, final Message<?> message) throws Exception {
SimpMessageHeaderAccessor headerAccessor = SimpMessageHeaderAccessor.wrap(message);
String authorization = headerAccessor.getFirstNativeHeader("Authorization");
String[] information = basicAuthResolver.resolve(authorization);

return new PlayerInfoDto(information[0], information[1]);
}

}
31 changes: 20 additions & 11 deletions src/main/java/mafia/mafiatogether/job/ui/JobController.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
package mafia.mafiatogether.job.ui;


import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.util.Base64;
import lombok.RequiredArgsConstructor;
import mafia.mafiatogether.chat.annotation.SendToChatWithRedis;
import mafia.mafiatogether.chat.domain.Message;
import mafia.mafiatogether.common.annotation.PlayerInfo;
import mafia.mafiatogether.common.resolver.PlayerInfoDto;
import mafia.mafiatogether.job.application.JobService;
import mafia.mafiatogether.job.application.dto.request.JobExecuteAbilityRequest;
import mafia.mafiatogether.job.application.dto.response.JobExecuteAbilityResponse;
import mafia.mafiatogether.job.application.dto.response.JobResponse;
import mafia.mafiatogether.job.application.dto.response.JobResultResponse;
import org.springframework.data.redis.core.StringRedisTemplate;
import org.springframework.http.ResponseEntity;
import org.springframework.messaging.handler.annotation.DestinationVariable;
import org.springframework.messaging.handler.annotation.MessageMapping;
import org.springframework.messaging.handler.annotation.Payload;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -25,22 +27,29 @@
@RequestMapping("/jobs")
public class JobController {

private final StringRedisTemplate stringRedisTemplate;
private final ObjectMapper objectMapper;
private final JobService jobService;

@GetMapping("/my")
public ResponseEntity<JobResponse> getJob(@PlayerInfo PlayerInfoDto playerInfoDto) {
return ResponseEntity.ok(jobService.getPlayerJob(playerInfoDto.code(), playerInfoDto.name()));
}

@MessageMapping("/skill/{code}/{name}")
@SendToChatWithRedis("/sub/mafia/{code}")
public Message executeSkill(
@DestinationVariable("code") String code,
@DestinationVariable("name") String name,
@MessageMapping("/jobs/skill")
public void executeWebSocketSkill(
@PlayerInfo PlayerInfoDto playerInfoDto,
@Payload JobExecuteAbilityRequest request
) {
JobExecuteAbilityResponse response = jobService.executeSkill(code, name, request);
return Message.ofChat(response.job(), response.result());
) throws JsonProcessingException {
JobExecuteAbilityResponse response = jobService.executeSkill(playerInfoDto.code(), playerInfoDto.name(),
request);
String auth = Base64.getEncoder()
.encodeToString((playerInfoDto.code() + ":" + playerInfoDto.name()).getBytes());

stringRedisTemplate.convertAndSend(
String.format("/sub/job/skill/%s/%s", response.job().toLowerCase(), auth),
objectMapper.writeValueAsString(response)
);
}

@PostMapping("/skill")
Expand Down
Loading