Refactor/#107 스트리밍 WebFlux WebSocket 으로 전환합니다. #108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- MVC WebSocket 의존성 삭제 - 직접적인 Netty 의존성 삭제
- 디스커버리 포함된 `WebClient.Builder` 을 주입받도록 수정 - `handleTextMessage` 메소드의 큰 변경 없이 `Flux<WebSocketMessage>` 반환 형태로 변경 - 음원 서버 / 스토리지 서버 통신 로직을 `WebClient` Mono / Flux 구조로 변경 - 예외 발생 로직을 `Mono.error(Throwable)` 으로 변경 - `BaseResponse<>` 사용을 위한 `typeReference` 코드 추가 - 스토리지 서버에서 음원 다운로드 시 `DataBuffer` NIO 버퍼를 사용하도록 변경
- 스레드풀의 스레드가 계속 살아있어, 마지막 반복문을 빠져나오지 못함 - `CountDownLatch` 으로 프로세스 종료를 알 수 있도록 로직 추가 - Flux 전송이 끝나면 원본 MP3 파일도 삭제하는 코드 추가
doxxx93
reviewed
Mar 12, 2024
There was a problem hiding this comment.
이 코드를 개선하는 방법은 여러 가지가 있을 수 있습니다. 아래에 몇 가지 제안을 드려보겠습니다.
- 클래스 이름 변경: StreamingHandler라는 이름은 너무 일반적이므로, 이 클래스가 실제로 어떤 작업을 수행하는지 더 잘 설명하는 이름으로 변경하는 것이 좋을 수 있습니다. 예를 들어, MusicStreamingHandler 또는 AudioStreamingHandler 등으로 변경할 수 있습니다.
- Command 패턴 적용: handleTextMessage 메서드에서는 명령어에 따라 다른 작업을 수행하고 있습니다. 이런 경우 Command 패턴을 사용하면 코드를 더 깔끔하게 만들 수 있습니다. 각 명령어에 대한 작업을 별도의 클래스로 분리하고, 이 클래스들이 공통 인터페이스를 구현하도록 만들 수 있습니다. 그런 다음 handleTextMessage 메서드에서는 명령어에 해당하는 클래스를 찾아 해당 클래스의 메서드를 호출하면 됩니다.
- 외부 요청 순서 관리: 현재 코드에서는 getMusic과 downloadMusic 메서드를 호출하여 외부 요청을 두 번 보내고 있습니다. 이 두 요청은 순서가 정해져 있으므로, 이를 하나의 메서드로 합치는 것이 좋을 수 있습니다. 이렇게 하면 외부 요청의 순서를 더 명확하게 관리할 수 있습니다.
- 에러 처리 개선: 현재 코드에서는 에러가 발생하면 onErrorResume을 사용하여 에러를 처리하고 있습니다. 하지만 이 방법은 에러가 발생한 원인을 파악하기 어렵게 만들 수 있습니다. 따라서 각 메서드에서 발생할 수 있는 에러를 명시적으로 처리하고, 필요한 경우 사용자에게 에러 메시지를 전달하는 것이 좋을 수 있습니다.
- 코드 재사용성 개선: handleTextMessage 메서드에서는 여러 번 payload.split("/")를 호출하여 명령어와 파라미터를 파싱하고 있습니다. 이런 코드는 별도의 메서드로 분리하여 재사용하는 것이 좋을 수 있습니다. 또한, getMusic과 downloadMusic 메서드에서는 비슷한 작업을 수행하고 있으므로, 이 두 메서드를 하나로 합치거나 공통 코드를 별도의 메서드로 분리하는 것이 좋을 수 있습니다.
이 외에도 코드를 개선하는 방법은 많이 있습니다. 위의 제안들은 코드를 개선하는 몇 가지 방법일 뿐이며, 실제로는 프로젝트의 요구 사항과 개발 팀의 코딩 스타일에 따라 적절한 방법이 달라질 수 있습니다.
There was a problem hiding this comment.
Mono, Flux 구분 & 예외 통일 정도만 했음!
커맨드 패턴이나, 순서 메소드 정리는 다음 태스크로 넘기겠음~
리뷰 감사링
- I/O 작업 `boundedElastic` 변경 - 예외 처리 통일 - `Mono` & `Flux` 구분 - Retry Backoff 추가
|
bounded elastic은 어떤 거? |
|
근데 확실히 기존보다 흐름은 나아진 거 같은데? 내일 더 봐봄 |
|
@doxxx93 블록킹 I/O 처리가 긴 경우와 스레드 수를 제한하고 싶을 떄 씀! |
|
이 PR 목적은 같은 기능을 하는 게 목적이라, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issue
#107
Changes
RestClient대신 Mono, Flux 구조를 사용하는WebClient를 사용하도록 변경CountDownLatch으로 프로세스가 종료됨을 인지하도록 로직 변경nio.Paths,nio.Files)Screenshots
To Reviewer
Additional Context(optional)
How Has This Been Tested?
Checklist