Skip to content
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
3 changes: 3 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ dependencies {
runtimeOnly 'com.mysql:mysql-connector-j'
annotationProcessor 'org.projectlombok:lombok'

// Websocket
implementation 'org.springframework.boot:spring-boot-starter-websocket'

// JWT
implementation 'io.jsonwebtoken:jjwt-api:0.11.5'
implementation 'io.jsonwebtoken:jjwt-impl:0.11.5'
Expand Down
27 changes: 6 additions & 21 deletions src/main/java/com/debatetimer/config/CorsConfig.java
Original file line number Diff line number Diff line change
@@ -1,39 +1,24 @@
package com.debatetimer.config;

import com.debatetimer.exception.custom.DTInitializationException;
import com.debatetimer.exception.errorcode.InitializationErrorCode;
import org.springframework.beans.factory.annotation.Value;
import lombok.RequiredArgsConstructor;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.web.servlet.config.annotation.CorsRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

@Configuration
@RequiredArgsConstructor
@EnableConfigurationProperties(CorsProperties.class)
public class CorsConfig implements WebMvcConfigurer {

private final String[] corsOrigin;

public CorsConfig(@Value("${cors.origin}") String[] corsOrigin) {
validate(corsOrigin);
this.corsOrigin = corsOrigin;
}

private void validate(String[] corsOrigin) {
if (corsOrigin == null || corsOrigin.length == 0) {
throw new DTInitializationException(InitializationErrorCode.CORS_ORIGIN_EMPTY);
}
for (String origin : corsOrigin) {
if (origin == null || origin.isBlank()) {
throw new DTInitializationException(InitializationErrorCode.CORS_ORIGIN_STRING_BLANK);
}
}
}
private final CorsProperties corsProperties;

@Override
public void addCorsMappings(CorsRegistry registry) {
registry.addMapping("/**")
.allowedOriginPatterns(corsOrigin)
.allowedOriginPatterns(corsProperties.getCorsOrigin())
.allowedMethods(
HttpMethod.GET.name(),
HttpMethod.POST.name(),
Expand Down
31 changes: 31 additions & 0 deletions src/main/java/com/debatetimer/config/CorsProperties.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package com.debatetimer.config;

import com.debatetimer.exception.custom.DTInitializationException;
import com.debatetimer.exception.errorcode.InitializationErrorCode;
import lombok.Getter;
import org.springframework.boot.context.properties.ConfigurationProperties;


@Getter
@ConfigurationProperties(prefix = "cors.origin")
public class CorsProperties {

private final String[] corsOrigin;

//TODO 머지될 때 dev, prod secret 갱신 필요
public CorsProperties(String[] corsOrigin) {
validate(corsOrigin);
this.corsOrigin = corsOrigin;
}

private void validate(String[] corsOrigin) {
if (corsOrigin == null || corsOrigin.length == 0) {
throw new DTInitializationException(InitializationErrorCode.CORS_ORIGIN_EMPTY);
}
for (String origin : corsOrigin) {
if (origin == null || origin.isBlank()) {
throw new DTInitializationException(InitializationErrorCode.CORS_ORIGIN_STRING_BLANK);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.debatetimer.config.sharing;

import com.debatetimer.controller.auth.AuthMember;
import com.debatetimer.controller.tool.jwt.AuthManager;
import com.debatetimer.exception.custom.DTClientErrorException;
import com.debatetimer.exception.errorcode.ClientErrorCode;
import com.debatetimer.service.auth.AuthService;
import lombok.RequiredArgsConstructor;
import org.springframework.core.MethodParameter;
import org.springframework.http.HttpHeaders;
import org.springframework.messaging.Message;
import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
import org.springframework.messaging.simp.stomp.StompHeaderAccessor;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class WebSocketAuthMemberResolver implements HandlerMethodArgumentResolver {

private final AuthManager authManager;
private final AuthService authService;

@Override
public boolean supportsParameter(MethodParameter parameter) {
return parameter.hasParameterAnnotation(AuthMember.class);
}
Comment on lines +16 to +26
Copy link
Member

Choose a reason for hiding this comment

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

이렇게 하면 웹소켓을 사용하지 않는 @AuthMember가 붙은 곳에도 해당 ArgumentResolver가 적용되는 건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

웹소켓 로직에만 적용됩니다.

웹소켓 MessageMapping 을 통해 애플리케이션에 위임된 메시지 핸들링 로직에서 @AuthMember가 붙은 엔드포인트에서 해당 로직을 인터셉트하여 실행해주어요.

package org.springframework.messaging.handler.invocation;

여기서 messaging이 Spring STOMP 소켓 관련 핸들링 로직이 있는 패키지입니다.


@Override
public Object resolveArgument(MethodParameter parameter, Message<?> message) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(message);
String token = accessor.getFirstNativeHeader(HttpHeaders.AUTHORIZATION);

if (token == null) {
throw new DTClientErrorException(ClientErrorCode.UNAUTHORIZED_MEMBER);
}

String email = authManager.resolveAccessToken(token);
return authService.getMember(email);
Comment on lines +37 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

단순 궁금증입니다.

  1. AccessToken을 확인해보니 유효기간이 1시간으로 되어있던데 만약 중간에 토큰이 만료되면 다시 로그인해서 접속해야 되나요?
  2. getMember에 DB 조회 로직이 있어서 매 요청마다 등록된 모든 사람에 대한 DB 조회가 발생할 것 같은데 이 부분에 대한 의견이 궁금합니다.

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초 이상이 지연이 생기면 잘 연동되는 느낌이 아니라서요.

뭔가 프론트랑 규약할 때 웹소켓용으로 토큰을 재발급해주거나, 웹소켓 시에는 인증을 약하게 유지하는 방식도 있을 것 같아요. 확실한 건 기존 어플리케이션 인증용 토큰을 그대로 사용하면 안될 것 같습니다.

WebScoket Security 도 있어서 Spring 에서 제공해주는 웹소켓 보안 관련 스펙 한번 확인해보고 적절한 대안을 생각해볼게요. 이건 추후 따로 이슈파서 해결하겠습니다.

}
}

38 changes: 38 additions & 0 deletions src/main/java/com/debatetimer/config/sharing/WebSocketConfig.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package com.debatetimer.config.sharing;

import com.debatetimer.config.CorsProperties;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Configuration;
import org.springframework.messaging.handler.invocation.HandlerMethodArgumentResolver;
import org.springframework.messaging.simp.config.MessageBrokerRegistry;
import org.springframework.web.socket.config.annotation.EnableWebSocketMessageBroker;
import org.springframework.web.socket.config.annotation.StompEndpointRegistry;
import org.springframework.web.socket.config.annotation.WebSocketMessageBrokerConfigurer;

@Configuration
@RequiredArgsConstructor
@EnableWebSocketMessageBroker
public class WebSocketConfig implements WebSocketMessageBrokerConfigurer {

private final CorsProperties corsProperties;
private final WebSocketAuthMemberResolver webSocketAuthMemberResolver;

@Override
public void addArgumentResolvers(List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(webSocketAuthMemberResolver);
}

@Override
public void configureMessageBroker(MessageBrokerRegistry registry) {
registry.enableSimpleBroker("/room", "/chairman");
registry.setApplicationDestinationPrefixes("/app");
}
Comment on lines +26 to +30
Copy link
Member

Choose a reason for hiding this comment

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

나중에 웹소켓 관련 로직이 추가되면, 해당 부분도 추가되야하는거죠?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 맞습니다.

-> 메모리에 내장된 Spring이 제공해주는 SimpleBroker에 위임할 prefix는 enableSimpleBroker에 추가
-> MessageMapping을 통해 애플리케이션에서 우리가 처리할 로직은 setApplicationDestinations에 추가


@Override
public void registerStompEndpoints(StompEndpointRegistry registry) {
registry.addEndpoint("/ws")
.setAllowedOriginPatterns(corsProperties.getCorsOrigin())
.withSockJS();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.debatetimer.controller.sharing;

import com.debatetimer.controller.auth.AuthMember;
import com.debatetimer.domain.member.Member;
import com.debatetimer.dto.sharing.request.SharingRequest;
import com.debatetimer.dto.sharing.response.SharingResponse;
import org.springframework.messaging.handler.annotation.DestinationVariable;
import org.springframework.messaging.handler.annotation.MessageMapping;
import org.springframework.messaging.handler.annotation.Payload;
import org.springframework.messaging.handler.annotation.SendTo;
import org.springframework.stereotype.Controller;

@Controller
public class SharingController {

@MessageMapping("/event/{roomId}")
@SendTo("/room/{roomId}")
public SharingResponse share(
@AuthMember Member member,
@DestinationVariable(value = "roomId") long roomId,
@Payload SharingRequest request
) {
return new SharingResponse(request.time());
}
Comment on lines +16 to +24
Copy link
Member

Choose a reason for hiding this comment

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

혹시, 발행자-구독자가 모두 연결되어 있는 상황에서, 발행자가 갑자기 연결이 끊길 경우에 대한 처리가 있나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

없습니다

그런데 이걸 백에서 해야할지도 조금 더 알아봐야할 것 같아요. 최근에 테코톡에서 웹소켓 끊김 대응전략을 보아서 오히려 프론트 단의 처리가 더 좋을 수도 있을 것 같아서요.

어떻게 클라이언트가 비정상종료되었는지를 서버가 감지하고 대응할지는 저도 공부가 더 필요할 것 같습니다.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.debatetimer.dto.sharing.request;

public record ChairmanSharingRequest(
Copy link
Contributor

Choose a reason for hiding this comment

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

Chairman이 뭔지 궁금해서 검색해보니 기업의 회장, 이사회의 의장 이런 개념이던데 맞을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사회자, 의장 으로 의도했습니다.

Copy link
Member

@leegwichan leegwichan Nov 24, 2025

Choose a reason for hiding this comment

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

(선택) 체어맨하면 한 눈에 이해하기 어려우니까, Announcer 나 구독자/발행자로 해보는 것도... (근데 애매하네요;;)
개인적으로는 체어맨하면 차 밖에 안떠올라서

long roomId
) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.debatetimer.dto.sharing.request;

import java.time.LocalDateTime;

public record SharingRequest(
LocalDateTime time
) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.debatetimer.dto.sharing.response;

import java.time.LocalDateTime;

public record SharingResponse(
LocalDateTime time
) {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package com.debatetimer.event.sharing;

import com.debatetimer.dto.sharing.request.ChairmanSharingRequest;
import com.debatetimer.exception.custom.DTClientErrorException;
import com.debatetimer.exception.errorcode.ClientErrorCode;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.context.event.EventListener;
import org.springframework.messaging.simp.SimpMessagingTemplate;
import org.springframework.messaging.simp.stomp.StompHeaderAccessor;
import org.springframework.stereotype.Component;
import org.springframework.web.socket.messaging.SessionSubscribeEvent;

@Slf4j
@Component
@RequiredArgsConstructor
public class RoomSubscribeListener {

private static final String AUDIENCE_SUBSCRIBE_PREFIX = "/room/";
private static final String CHAIRMAN_CHANNEL_PREFIX = "/chairman/";

private final SimpMessagingTemplate messagingTemplate;

@EventListener
public void handleSubscribeEvent(SessionSubscribeEvent event) {
StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
String destination = accessor.getDestination();
if (destination == null) {
return;
}

if (destination.startsWith(AUDIENCE_SUBSCRIBE_PREFIX)) {
long roomId = parseRoomId(destination);
messagingTemplate.convertAndSend(CHAIRMAN_CHANNEL_PREFIX + roomId, new ChairmanSharingRequest(roomId));
}
Comment on lines 24 to 35
Copy link

@coderabbitai coderabbitai bot Nov 17, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

구독 destination 파싱 시 잘못된 roomId에 대한 방어 코드 제안

현재 destination"/room/"로만 오거나, "/room/123/extra" 같이 예상하지 못한 값이 올 경우 Long.parseLong(...)에서 NumberFormatException이 발생해 리스너 전체가 실패할 수 있습니다.

예외를 로그로 남기고 무시하도록 방어 코드를 두면, 잘못된 구독 요청이 와도 다른 세션 처리에는 영향을 주지 않을 것 같습니다.

    @EventListener
    public void handleSubscribeEvent(SessionSubscribeEvent event) {
        StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
        String destination = accessor.getDestination();
        if (destination == null) {
            return;
        }

        if (destination.startsWith(AUDIENCE_SUBSCRIBE_PREFIX)) {
-            long roomId = Long.parseLong(destination.replace(AUDIENCE_SUBSCRIBE_PREFIX, ""));
-            messagingTemplate.convertAndSend(CHAIRMAN_CHANNEL_PREFIX + roomId, new ChairmanSharingRequest(roomId));
+            String roomIdPart = destination.substring(AUDIENCE_SUBSCRIBE_PREFIX.length());
+            try {
+                long roomId = Long.parseLong(roomIdPart);
+                messagingTemplate.convertAndSend(
+                        CHAIRMAN_CHANNEL_PREFIX + roomId,
+                        new ChairmanSharingRequest(roomId)
+                );
+            } catch (NumberFormatException e) {
+                log.warn("Invalid room subscription destination: {}", destination, e);
+            }
        }
    }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/com/debatetimer/event/sharing/RoomSubscribeListener.java around
lines 22 to 33, the code calls Long.parseLong on the destination substring
without validation which can throw NumberFormatException for bad values like
"/room/" or "/room/123/extra"; add defensive validation: after confirming
destination.startsWith(AUDIENCE_SUBSCRIBE_PREFIX) extract the suffix, validate
it contains only digits (or split on '/' and take the first segment) and parse
that; wrap parsing in a try/catch(NumberFormatException) and on error log a
warning including the invalid destination and exception, then return/ignore so
the listener continues handling other events.

Copy link
Contributor

Choose a reason for hiding this comment

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

@coli-geonwoo 이 내용 한번 확인해주세요

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

반영해주었습니다... 값 객체로 감싸고 싶긴 했는데 너무 API 규약에 의존하는 느낌이라 순수한 VO 생성이 안될 것 같아 일단 private method로 처리해주었어요.

}

private long parseRoomId(String destination) {
try {
String parsedRoomId = destination.substring(AUDIENCE_SUBSCRIBE_PREFIX.length());
return Long.parseLong(parsedRoomId);
} catch (NumberFormatException exception) {
throw new DTClientErrorException(ClientErrorCode.INVALID_ROOM_ID);
}
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ public enum ClientErrorCode implements ResponseErrorCode {
ALREADY_DONE_POLL(HttpStatus.BAD_REQUEST, "이미 완료된 투표 입니다"),
ALREADY_VOTED_PARTICIPANT(HttpStatus.BAD_REQUEST, "이미 참여한 투표자 입니다"),

INVALID_ROOM_ID(HttpStatus.BAD_REQUEST, "잘못된 roomId 값입니다"),

TABLE_NOT_FOUND(HttpStatus.NOT_FOUND, "토론 테이블을 찾을 수 없습니다."),
NOT_TABLE_OWNER(HttpStatus.UNAUTHORIZED, "테이블을 소유한 회원이 아닙니다."),
POLL_NOT_FOUND(HttpStatus.NOT_FOUND, "투표를 찾을 수 없습니다."),
Expand Down
4 changes: 3 additions & 1 deletion src/main/resources/application-dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ spring:
baseline-version: 1

cors:
origin: ${secret.cors.origin}
origin:
cors-origin:
- ${secret.cors.origin}

oauth:
client_id: ${secret.oauth.client_id}
Expand Down
4 changes: 3 additions & 1 deletion src/main/resources/application-prod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ spring:
baseline-version: 1

cors:
origin: ${secret.cors.origin}
origin:
cors-origin:
- ${secret.cors.origin}

oauth:
client_id: ${secret.oauth.client_id}
Expand Down
76 changes: 76 additions & 0 deletions src/test/java/com/debatetimer/BaseStompTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package com.debatetimer;

import com.debatetimer.fixture.HeaderGenerator;
import com.debatetimer.fixture.entity.MemberGenerator;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.web.server.LocalServerPort;
import org.springframework.messaging.converter.MappingJackson2MessageConverter;
import org.springframework.messaging.converter.MessageConverter;
import org.springframework.messaging.simp.stomp.StompSession;
import org.springframework.messaging.simp.stomp.StompSessionHandlerAdapter;
import org.springframework.web.socket.client.standard.StandardWebSocketClient;
import org.springframework.web.socket.messaging.WebSocketStompClient;
import org.springframework.web.socket.sockjs.client.SockJsClient;
import org.springframework.web.socket.sockjs.client.Transport;
import org.springframework.web.socket.sockjs.client.WebSocketTransport;

@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT)
public abstract class BaseStompTest {

private static final String SOCKET_ENDPOINT = "/ws";

protected StompSession stompSession;

@LocalServerPort
private int port;

private final String url;

private final WebSocketStompClient websocketClient;

@Autowired
protected MemberGenerator memberGenerator;

@Autowired
protected HeaderGenerator headerGenerator;

public BaseStompTest() {
List<Transport> transports = List.of(new WebSocketTransport(new StandardWebSocketClient()));
this.websocketClient = new WebSocketStompClient(new SockJsClient(transports));
this.websocketClient.setMessageConverter(buildMessageConverter());
this.url = "ws://localhost:";
}

private MessageConverter buildMessageConverter() {
MappingJackson2MessageConverter converter = new MappingJackson2MessageConverter();
ObjectMapper objectMapper = new ObjectMapper();
objectMapper.registerModule(new JavaTimeModule());
objectMapper.findAndRegisterModules();
converter.setObjectMapper(objectMapper);
return converter;
}

@BeforeEach
public void connect() throws ExecutionException, InterruptedException, TimeoutException {
this.stompSession = this.websocketClient
.connectAsync(url + port + SOCKET_ENDPOINT, new StompSessionHandlerAdapter() {
})
.get(3, TimeUnit.SECONDS);
}

@AfterEach
public void disconnect() {
if (this.stompSession.isConnected()) {
this.stompSession.disconnect();
}
}
Comment on lines +62 to +75
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

connect 실패 시 disconnect에서 NPE 발생 가능성

connect()에서 예외가 발생하면 stompSession이 초기화되지 않을 수 있는데, 이 경우 disconnect()

if (this.stompSession.isConnected()) {

에서 NullPointerException이 발생할 수 있습니다. 테스트 실패 원인을 흐리기도 해서, 널 체크를 추가하는 편이 안전해 보입니다.

    @AfterEach
    public void disconnect() {
-        if (this.stompSession.isConnected()) {
-            this.stompSession.disconnect();
-        }
+        if (this.stompSession != null && this.stompSession.isConnected()) {
+            this.stompSession.disconnect();
+        }
    }
🤖 Prompt for AI Agents
In src/test/java/com/debatetimer/BaseStompTest.java around lines 62 to 75, the
AfterEach disconnect() can throw a NullPointerException if connect() failed and
stompSession was never initialized; update disconnect() to first check that
this.stompSession is not null before calling isConnected() (e.g., guard with
this.stompSession != null && this.stompSession.isConnected()) and then call
disconnect() only when both conditions hold.

}
Loading