-
Notifications
You must be signed in to change notification settings - Fork 0
feat: SSE 서비스 운영 상태 정의 로직 구현 #205
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
2596d39
66b6bed
7dd19ce
8121323
3fffe02
b9eecbd
9fd96f4
d72fd31
43c3d1f
c5ddc3b
8cee796
bff7473
bd3b4a7
92f831b
91083e2
4bb883c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -15,12 +15,14 @@ | |||||||||||||||||||
| public class SseService { | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private final SseEmitterStorage sseEmitterStorage; | ||||||||||||||||||||
| private volatile SseStatus currentStatus = SseStatus.IDLE; | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. volatile을 사용한 이유가 무엇인지 궁금해요!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아주 유익한 말랑씨의 블로그 잘 봤습니다 :)
다음은 => 여러 스레드가 동시에 들어오면, 서로 같은 이전 상태를 보고 각각 이를 해결하기 위해서는 가시성과 원자성을 모두 보장해주는 정리하자면, 제가 volatile을 쓴 주목적은 가시성 보장입니다!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 PR에서 관리하려는 것은 개인적으로 관리하려는 상태가 SseService에서 관리하는 것이 올바른가? 하는 생각이 들어요. SseService는 Sse연결과 데이터 전송을 담당하는 클래스에요. 실시간 여석 서비스가 어떻게 되든 관심 없는 클래스에요. 대신에 Admin 클래스에서 상태 관리하는 것이 객체의 책임 관점에서 더 명확하지 않을까요?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
맥락을 잘 이해해주셨네요.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
말씀하신 내용에 동의합니다! |
||||||||||||||||||||
| public SseEmitter connect(String token) { | ||||||||||||||||||||
| SseEmitter sseEmitter = createSseEmitter(); | ||||||||||||||||||||
| sseEmitterStorage.add(token, sseEmitter); | ||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because Useful? React with 👍 / 👎. |
||||||||||||||||||||
| SseEventBuilder initialEvent = SseEventBuilderFactory.createInitialEvent(); | ||||||||||||||||||||
| sendEvent(sseEmitter, initialEvent); | ||||||||||||||||||||
| sendStatusEvent(sseEmitter); | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 혹시
그런데 항상 |
||||||||||||||||||||
| return sseEmitter; | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
|
|
@@ -42,6 +44,13 @@ public void propagate(String token, String eventName, Object data) { | |||||||||||||||||||
| }); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public void updateStatus(SseStatus newStatus) { | ||||||||||||||||||||
| if (this.currentStatus != newStatus) { | ||||||||||||||||||||
| this.currentStatus = newStatus; | ||||||||||||||||||||
| propagateStatus(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private void sendEvent(SseEmitter sseEmitter, SseEventBuilder eventBuilder) { | ||||||||||||||||||||
| try { | ||||||||||||||||||||
| sseEmitter.send(eventBuilder); | ||||||||||||||||||||
|
|
@@ -51,12 +60,25 @@ private void sendEvent(SseEmitter sseEmitter, SseEventBuilder eventBuilder) { | |||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public List<String> getConnectedTokens() { | ||||||||||||||||||||
| return sseEmitterStorage.getUserTokens().stream().toList(); | ||||||||||||||||||||
| private void sendStatusEvent(SseEmitter sseEmitter) { | ||||||||||||||||||||
| SseStatusResponse sseStatusResponse = SseStatusResponse.of(currentStatus.name().toLowerCase(), | ||||||||||||||||||||
| currentStatus.getMessage()); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| SseEventBuilder eventBuilder = SseEventBuilderFactory.create("status", sseStatusResponse); | ||||||||||||||||||||
| sendEvent(sseEmitter, eventBuilder); | ||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| private void propagateStatus() { | ||||||||||||||||||||
| SseStatusResponse statusResponse = SseStatusResponse.of(currentStatus.name().toLowerCase(), | ||||||||||||||||||||
| currentStatus.getMessage()); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| sseEmitterStorage.getEmitters().forEach(emitter -> { | ||||||||||||||||||||
| SseEventBuilder eventBuilder = SseEventBuilderFactory.create("status", statusResponse); | ||||||||||||||||||||
| sendEvent(emitter, eventBuilder); | ||||||||||||||||||||
| }); | ||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 메서드들은 SseService를 처음 구현할 때에 철학과는 다소 다른 점이 있네요! 지금 PR처럼 sendStatusEvent, propagateStatus 같이 쓰임새가 한정되어 있는 메서드가 추가될수록 확장 가능한 형태에서 벗어난다고 생각해요. 또, 코드 중복도 생겼지요. // 코드 중복
sseEmitterStorage.getEmitters().forEach(emitter -> {
SseEventBuilder eventBuilder = SseEventBuilderFactory.create("status", statusResponse);
sendEvent(emitter, eventBuilder);
});이렇게 하기 보다는 sendStatusEvent 메서드를 사용하는 connect 메서드 내부에서 코드 작성하는 것이 좋겠어요.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 아하! 그런 의도가 있었군요. |
||||||||||||||||||||
| } | ||||||||||||||||||||
|
|
||||||||||||||||||||
| public SseStatusResponse isConnected(String token) { | ||||||||||||||||||||
| boolean isConnected = sseEmitterStorage.getEmitter(token).isPresent(); | ||||||||||||||||||||
| return SseStatusResponse.of(isConnected); | ||||||||||||||||||||
| public List<String> getConnectedTokens() { | ||||||||||||||||||||
| return sseEmitterStorage.getUserTokens().stream().toList(); | ||||||||||||||||||||
| } | ||||||||||||||||||||
| } | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package kr.allcll.backend.support.sse; | ||
|
|
||
| public enum SseStatus { | ||
| LIVE("실시간 여석을 제공중이에요"), | ||
| PRESEAT("preseat 여석을 이용해주세요"), | ||
| IDLE("실시간 여석 제공 전이에요. 서비스가 시작되면 알림을 드릴게요."), | ||
| ERROR("2025-09-02 까지 서비스 점검 중이에요"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 메시지는 사용자에게 노출되는 건가요? 아니면 FE에게 제공하는 문구인가요?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
사용자에게 노출되는 정보입니다! 주환님 말씀대로 사용자에게 노출되는 정보인만큼 문구를 잘 다듬어야겠네요,,
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이 클래스도 마찬가지로, SseStatus라는 클래스명이 어색하죠. 의미는 Sse 상태인데, 내부에서 실질적으로 관리하는 상태는 실시간 여석과 관련되었으니깐요. 클래스 명을 바꾸고 다른 패키지로 이동하는 것이 좋겠습니다 |
||
|
|
||
| private final String message; | ||
|
|
||
| SseStatus(String message) { | ||
| this.message = message; | ||
| } | ||
|
|
||
| public String getMessage() { | ||
| return message; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| package kr.allcll.backend.support.sse.dto; | ||
|
|
||
| public record SseStatusResponse( | ||
| boolean isConnected | ||
| String status, | ||
| String message | ||
| ) { | ||
| public static SseStatusResponse of(boolean isConnected) { | ||
| return new SseStatusResponse(isConnected); | ||
|
|
||
| public static SseStatusResponse of(String status, String message) { | ||
| return new SseStatusResponse(status, message); | ||
| } | ||
| } |
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.
로직을 실행하기 전에 상태를 바꾸면, 이후 fetchPinSeat / fetchGeneralSeat 실행 중 오류가 발생해 실제 크롤링이 시작되지 않아도 상태가 LIVE로 바뀐다는 문제가 생길 것 같은데 상태를 바꾸는 시점을
이렇게 수정하는 것에 대해 어떻게 생각하시나요?
Uh oh!
There was an error while loading. Please reload this page.
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.
로직이 실행 되었을 때 에러가 발생하면?
다음 코드와 같이,
ERROR상태로 업데이트 되고propagate됩니다!크롤링을 시작한다 == 서비스 운영을 시작한다와 같은 의미이기 때문에 저 로직 순서가 맞다고 판단을 했는데, 다른 분들의 의견이 궁금하네요!!