feat: Pin 한 과목 SSE 전송 기능 구현 #63
Conversation
Test Results59 tests 59 ✅ 6s ⏱️ Results for commit b599121. ♻️ This comment has been updated with latest results. |
오.. 훌륭한 고민인데요? 덕분에 저도 고민해 보네요. 저는 Seat에 두는 것이 좋아보여요! 여석 관련된 기능이라고 생각해서요. seat이 pin에 의존하면 안될까요? 의존 관계는 좀더 여유가 생겼을 때, 둘이 얘기해 보면 좋을 것 같아요. 저는 순환참조만 일어나지 않으면 된다고 생각해서 seat -> pin으로 의존하는 구조가 좋아보이네요! 하지만 상황이 바뀌면 달라질 수도 있었어요.
제 두눈으로 확인한 건 아니지만, 아마도 없을 거예요. ThreadLocal이잖아요?! 토큰 정보는 요청 스레드에 존재하고, 스케줄링 스레드에는 토큰 정보가 없을거예요. 그럼, 어떤 사용자에게 보낼지 알기 어려운데요. sse 연결 요청이 들어왔을 때, 사용자 토큰 정보를 식별해서 저장하는 로직이 필요하겠어요. 그럼 스케줄링에서는 토큰을 꺼내서 쓰기만 하면 되죠! 혹시 좀 어려운 기능이라면 페어프로그래밍으로 해결해 봐도 좋을 것 같아요! |
|
추가 커밋 올렸습니다. 봐야할 것SchedulerConfig 설정 검토해야함 |
| @Bean | ||
| public ThreadPoolTaskScheduler threadPoolTaskScheduler() { | ||
| ThreadPoolTaskScheduler scheduler = new ThreadPoolTaskScheduler(); | ||
| scheduler.setThreadNamePrefix("reset-session-time-scheduler"); |
There was a problem hiding this comment.
스레드 이름을 변경해야 할 것 같아요.
핀 고정된 과목 스케줄링에서만 사용한다면 더 구체적인 이름으로 변경하고,
전역적으로 이 스케줄링을 사용한다면 더 추상적인 이름으로 바꿔야 겠어요.
|
|
||
| @RestController | ||
| @RequiredArgsConstructor | ||
| public class PinApi { | ||
|
|
||
| private final PinService pinService; | ||
| private final SseService sseService; |
There was a problem hiding this comment.
| private final SseService sseService; |
| ScheduledFuture<?> scheduledFuture = scheduler.scheduleAtFixedRate(task, Duration.ofMillis(1000)); | ||
| scheduler.schedule(() -> scheduledFuture.cancel(true), | ||
| new Date(System.currentTimeMillis() + 10000)); |
There was a problem hiding this comment.
여기 있는 숫자들은 추후 설정 값으로 분리해야 할 것 같아요.
일단은 보기 쉽게 상수로 분리해 볼까요?
| @@ -21,6 +22,19 @@ public List<Seat> getNonMajorSeats(int limit) { | |||
| .toList(); | |||
| } | |||
|
|
|||
| public List<Seat> getPinSeats(List<Subject> subjects) { | |||
There was a problem hiding this comment.
요기 pin이라는 이름을 붙여서 재사용하기 어려운 메서드가 되었어요. 지우면 어떨까요?
| public List<Seat> getPinSeats(List<Subject> subjects) { | |
| public List<Seat> getSeats(List<Subject> subjects) { |
| List<Seat> result = new ArrayList<>(); | ||
| for (Subject subject : subjects) { | ||
| for (Seat seat : seats) { | ||
| if (seat.getSubject().getId().equals(subject.getId())) { | ||
| result.add(seat); | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
성능이 꽤 느릴 거 같아요. 최적화를 위해 Map을 고려해야 봐야겠어요. 이슈로 등록해 둘게요. 여기 pr에서는 변경하지 않아도 됩니당
작업 내용
사용자가 pin 한 과목의 여석정보를 sse로 전송하는 기능을 구현하였습니다.
아직 완성은 아니구요, 틀 정도 작성해 두었습니다.
처음 사용해보는 것이라 미숙해서요 의아한지점이 있거나 다른 생각이 있으시면 많이 말해주세요
고민 지점과 리뷰 포인트
PinService에
가 들어있는데요, 이 해당 메서드가 pinRepository에 접근해야 하고, pin에 관한 서비스 라고 생각해서 우선 PinService에 두었습니다. 그런데 어떻게 보면 seat에 관련된 기능이기도 하잖아요? 그래서 SeatService에 두어야하나 고민도 했어요. 만약 SeatService에 둘 경우 SeatService는 PinRepository도 의존하게 되어요. 어떻게 하는게 좋을까요?
그리고,
ThreadLocalHolder.SHARED_TOKEN.get()로, 접근한 사용자의 토큰을 식별해서 PinRepository에 접근해야하잖아요?그리고 토큰은 저희가 요청이 있으면 바로 저장해두고요.
그런데 해당 메서드가 스케줄링으로 실행될 때에도 이 토큰이 계속 존재하는지를 모르겠어요...
이 지점이 좀 불명확해보여요.. 테스트가 필요할 것 같고, 로직을 수정도해야할 것 같아요