-
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
웹소켓 채팅 리팩터링 #160
웹소켓 채팅 리팩터링 #160
Conversation
@Override | ||
public void configureClientInboundChannel(ChannelRegistration registry) { | ||
registry.interceptors(stompChannelInterceptor); | ||
registry.interceptors( |
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.
코드 작성하면서 생각하다보니깐 제일 바꾸고 싶었던 영역은 이부분인거같아
private static final String SUBSCRIBE_FORMAT = "%s/%s";
private static final String PUBLISHING_FORMAT = "%s/%s/%s";
기존에는 인터셉터를 통해서 모든 요청에 대해서 위와 같은 URL 패턴이아니면 에러가 발생해서 저런 형태가 아닌 방식으로 pub/sub을 할때 문제가 생겼는데 PathMatcher 를 통해서 필요한 부분만 등록해주는게 필요하다고 생각했어
이전에 확장성 떨어진다고 말한부분이 이부분 이였어
현재 구조에서는 모든요청이 해당 인터셉터를 통과하는 문제
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.
기존에는 인터셉터를 통해서 모든 요청에 대해서 위와 같은 URL 패턴이아니면 에러가 발생해서 저런 형태가 아닌 방식으로 pub/sub을 할때 문제가 생겼는데 PathMatcher 를 통해서 필요한 부분만 등록해주는게 필요하다고 생각했어
이 말이 구독을 위해서는 "sub/{code}/"이 형식이어야하고 발행은 "pub/{code}/{name}"이 형식이 어색하다는거고 이걸 PatchMatcher의 채인 메서드로 필요할때마다 추가해서 확장성을 높였다는 의미지?
난 이런 의미면 좋은거 같아
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.
형식이 어색한것도 있지만 큰문제는
- 현재 구독,발행을 위해서 "sub/?/{auth}/" , "pub/?/{auth}" 형식이 강제 되는상황인데 이유는 인터셉터가 전역으로 설정되어있어서 그럼 ("sub/{auth}/hello" , "pub/bye" 이렇게 불가능)
- 그래서 인터셉터를 구독할때 현재처럼 전역으로 설정하는게아니라 PatchMatcher를 통해서 특정 url, 메서드에만 적용범위를 좁혀서 확장성을 높임 (sub/{auth}/hello, "pub/bye" 이런 형식으로 다양한 방식으로 여러 인터셉터 적용가능)
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.
여기 기가막힌데??
구우욷!
약간 이 패턴과 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 에 할당할지도 결정하면 좋을 것 같아!
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.
고생했어 파워!
다 좋은데 이건 어떤가하는 부분이 있어서 코멘트 남겼어
src/main/java/mafia/mafiatogether/common/config/WebSocketConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/common/interceptor/PathMatcherInterceptor.java
Outdated
Show resolved
Hide resolved
@Override | ||
public void configureClientInboundChannel(ChannelRegistration registry) { | ||
registry.interceptors(stompChannelInterceptor); | ||
registry.interceptors( |
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.
기존에는 인터셉터를 통해서 모든 요청에 대해서 위와 같은 URL 패턴이아니면 에러가 발생해서 저런 형태가 아닌 방식으로 pub/sub을 할때 문제가 생겼는데 PathMatcher 를 통해서 필요한 부분만 등록해주는게 필요하다고 생각했어
이 말이 구독을 위해서는 "sub/{code}/"이 형식이어야하고 발행은 "pub/{code}/{name}"이 형식이 어색하다는거고 이걸 PatchMatcher의 채인 메서드로 필요할때마다 추가해서 확장성을 높였다는 의미지?
난 이런 의미면 좋은거 같아
src/main/java/mafia/mafiatogether/common/config/WebSocketConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/common/interceptor/ChatInterceptor.java
Outdated
Show resolved
Hide resolved
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.
고생했어
간단하게 코멘트 남겼어잉
src/main/java/mafia/mafiatogether/common/interceptor/ChatInterceptor.java
Outdated
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/common/interceptor/ChatInterceptor.java
Outdated
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/chat/ui/WebsocketPlayerArgumentResolver.java
Outdated
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/common/config/WebSocketConfig.java
Outdated
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/common/interceptor/PathMatcherInterceptor.java
Show resolved
Hide resolved
src/main/java/mafia/mafiatogether/common/interceptor/PathMatcherInterceptor.java
Show resolved
Hide resolved
@Override | ||
public void configureClientInboundChannel(ChannelRegistration registry) { | ||
registry.interceptors(stompChannelInterceptor); | ||
registry.interceptors( |
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.
여기 기가막힌데??
구우욷!
약간 이 패턴과 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 에 할당할지도 결정하면 좋을 것 같아!
* feat: 웹소켓 채팅 리팩터링 * feat: 채팅 롤백 Path 롤백 및 인터셉터 수정 * refactor: job skill 수정 * refactor: auth 인증부분 분리 * fix: WebsocketPlayerArgumentResolver 패키지 위치 수 --------- Co-authored-by: waterricecake <[email protected]>
코드에 코멘트 작성