-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat/#127 ManiaDB API 를 사용하여 등록되지 않은 노래 검색 기능 구현 #153
base: main
Are you sure you want to change the base?
Changes from 1 commit
d7f5560
ee3e38b
53443bd
6a7bf4e
89fd828
2383468
0577695
237d01e
4d2ab0c
e8eb415
2c5c53b
7bd679a
ef58544
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 | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,70 @@ | ||||||
package shook.shook.song.application; | ||||||
|
||||||
import java.nio.charset.StandardCharsets; | ||||||
import java.util.Collections; | ||||||
import java.util.List; | ||||||
import java.util.Objects; | ||||||
import lombok.RequiredArgsConstructor; | ||||||
import org.springframework.http.HttpStatusCode; | ||||||
import org.springframework.http.MediaType; | ||||||
import org.springframework.stereotype.Service; | ||||||
import org.springframework.web.reactive.function.client.WebClient; | ||||||
import shook.shook.song.application.dto.UnregisteredSongSearchResponse; | ||||||
import shook.shook.song.application.dto.maniadb.ManiaDBAPISearchResponse; | ||||||
import shook.shook.song.application.dto.maniadb.UnregisteredSongResponses; | ||||||
import shook.shook.song.exception.UnregisteredSongException; | ||||||
import shook.shook.song.exception.UnregisteredSongException.EmptyResultException; | ||||||
|
||||||
@RequiredArgsConstructor | ||||||
@Service | ||||||
public class ManiaDBSearchService { | ||||||
|
||||||
private static final String MANIA_DB_API_URI = "/%s/?sr=song&display=%d&key=example&v=0.5"; | ||||||
private static final int SEARCH_SIZE = 100; | ||||||
private static final String SPECIAL_MARK_REGEX = "[^ㄱ-ㅎㅏ-ㅣ가-힣a-zA-Z0-9,. ]"; | ||||||
private final WebClient webClient; | ||||||
|
||||||
public List<UnregisteredSongSearchResponse> searchSongs(final String searchWord) { | ||||||
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. 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. 헉 꼼꼼한 예외 처리 👍 |
||||||
final String parsedSearchWord = replaceSpecialMark(searchWord); | ||||||
final UnregisteredSongResponses searchResult = getSearchResult(parsedSearchWord); | ||||||
|
||||||
if (Objects.isNull(searchResult.getSongs())) { | ||||||
return Collections.emptyList(); | ||||||
} | ||||||
|
||||||
return searchResult.getSongs().stream() | ||||||
.map(UnregisteredSongSearchResponse::from) | ||||||
.toList(); | ||||||
} | ||||||
|
||||||
private String replaceSpecialMark(final String rawSearchWord) { | ||||||
return rawSearchWord.replaceAll(SPECIAL_MARK_REGEX, ""); | ||||||
} | ||||||
|
||||||
private UnregisteredSongResponses getSearchResult(final String searchWord) { | ||||||
final String searchUrl = String.format(MANIA_DB_API_URI, searchWord, SEARCH_SIZE); | ||||||
final ManiaDBAPISearchResponse result = getResultFromManiaDB(searchUrl); | ||||||
|
||||||
if (Objects.isNull(result)) { | ||||||
throw new EmptyResultException(); | ||||||
} | ||||||
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. 제가 생각한 해당 분기에 들어오는 경우입니다.
언제 해당 상황이 발생하고 이 때 던지는 예외가 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. 💬 저도 스플릿이랑 비슷한 질문입니다 ㅎ.ㅎ 처음에 서비스 코드만 봤을 때는 Response 객체가 null이 되는 상황이 maniaDB에서 그리구 빈 문자열만 해당한다면 언제 이런 상황이 생기는지도 궁금합니다 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. 앗 그리구 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. 스플릿이 말한 플로우가 맞습니다. 아직 저 예외를 만나본 적은 없어요.
예외를 경험한 것은 아니지만, 충분히 예측 가능한 예외라고 생각하여 null 체크를 해서 예외를 던지도록 했습니다. 좀 더 생각해보니 이 경우는 ++ 상위 예외가 자동으로 import 됐나 보네요 ㅠㅠ 한 3번쯤 확인한 것 같은데 어째서! |
||||||
|
||||||
return result.getSongs(); | ||||||
} | ||||||
|
||||||
private ManiaDBAPISearchResponse getResultFromManiaDB(final String searchUrl) { | ||||||
return webClient.get() | ||||||
.uri(searchUrl) | ||||||
.accept(MediaType.TEXT_XML) | ||||||
.acceptCharset(StandardCharsets.UTF_8) | ||||||
.retrieve() | ||||||
.onStatus(HttpStatusCode::is4xxClientError, (clientResponse) -> { | ||||||
throw new UnregisteredSongException.ManiaDBClientException(); | ||||||
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
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. 좋은 피드백 감사합니다! 사실 이름 짓기가 너무 어려워서 ㅋㅋㅋ 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. 예외처리 기가막히고 신기하네요 👍 |
||||||
}) | ||||||
.onStatus(HttpStatusCode::is5xxServerError, (clientResponse) -> { | ||||||
throw new UnregisteredSongException.ManiaDBServerException(); | ||||||
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
|
||||||
}) | ||||||
.bodyToMono(ManiaDBAPISearchResponse.class) | ||||||
.block(); | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
package shook.shook.song.application.dto; | ||
|
||
import java.util.stream.Collectors; | ||
import lombok.AllArgsConstructor; | ||
import lombok.Getter; | ||
import shook.shook.song.application.dto.maniadb.SongArtistResponse; | ||
|
||
@AllArgsConstructor | ||
@Getter | ||
public class UnregisteredSongSearchResponse { | ||
|
||
private static final String EMPTY_SINGER = ""; | ||
private static final String SINGER_DELIMITER = ", "; | ||
private String title; | ||
private String singer; | ||
private String albumImageUrl; | ||
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 static UnregisteredSongSearchResponse from( | ||
final shook.shook.song.application.dto.maniadb.UnregisteredSongResponse unregisteredSongResponse) { | ||
if (unregisteredSongResponse.getTrackArtists() == null | ||
|| unregisteredSongResponse.getTrackArtists().getArtists() == null) { | ||
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.
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.
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. 💬 요 if문 내의 조건이 어떤걸 의미하는지 잘 나타낼 수 있게 따로 메서드로 분리하는건 어떻게 생각하시나요 ?.? 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 new UnregisteredSongSearchResponse( | ||
unregisteredSongResponse.getTitle().trim(), | ||
EMPTY_SINGER, | ||
unregisteredSongResponse.getAlbum().getImage().trim() | ||
); | ||
} | ||
|
||
final String singers = collectToString(unregisteredSongResponse); | ||
|
||
return new UnregisteredSongSearchResponse( | ||
unregisteredSongResponse.getTitle().trim(), | ||
singers, | ||
unregisteredSongResponse.getAlbum().getImage().trim() | ||
); | ||
} | ||
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 static String collectToString( | ||
final shook.shook.song.application.dto.maniadb.UnregisteredSongResponse unregisteredSongResponse) { | ||
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. 여기도 import문으로 경로를 나타낼 수 있을 것 같습니다. |
||
return unregisteredSongResponse.getTrackArtists().getArtists().stream() | ||
.map(SongArtistResponse::getName) | ||
.map(String::trim) | ||
.collect(Collectors.joining(SINGER_DELIMITER)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package shook.shook.song.application.dto.maniadb; | ||
|
||
import jakarta.xml.bind.annotation.XmlElement; | ||
import jakarta.xml.bind.annotation.XmlRootElement; | ||
import lombok.Getter; | ||
|
||
@Getter | ||
@XmlRootElement(name = "rss") | ||
public class ManiaDBAPISearchResponse { | ||
|
||
@XmlElement(name = "channel") | ||
private UnregisteredSongResponses songs; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package shook.shook.song.application.dto.maniadb; | ||
|
||
import jakarta.xml.bind.annotation.XmlElement; | ||
import jakarta.xml.bind.annotation.XmlRootElement; | ||
import lombok.Getter; | ||
|
||
@Getter | ||
@XmlRootElement(name = "album", namespace = "http://www.maniadb.com/api") | ||
public class SongAlbumResponse { | ||
|
||
@XmlElement(name = "image") | ||
private String image; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package shook.shook.song.application.dto.maniadb; | ||
|
||
import jakarta.xml.bind.annotation.XmlElement; | ||
import jakarta.xml.bind.annotation.XmlRootElement; | ||
import lombok.Getter; | ||
|
||
@Getter | ||
@XmlRootElement(name = "artist", namespace = "http://www.maniadb.com/api") | ||
public class SongArtistResponse { | ||
|
||
@XmlElement(name = "name") | ||
private String name; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package shook.shook.song.application.dto.maniadb; | ||
|
||
import jakarta.xml.bind.annotation.XmlElement; | ||
import jakarta.xml.bind.annotation.XmlRootElement; | ||
import java.util.List; | ||
import lombok.Getter; | ||
|
||
@Getter | ||
@XmlRootElement(name = "trackartists", namespace = "http://www.maniadb.com/api") | ||
public class SongTrackArtistsResponse { | ||
|
||
@XmlElement(name = "artist", namespace = "http://www.maniadb.com/api") | ||
private List<SongArtistResponse> artists; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
package shook.shook.song.application.dto.maniadb; | ||
|
||
import jakarta.xml.bind.annotation.XmlElement; | ||
import jakarta.xml.bind.annotation.XmlRootElement; | ||
import lombok.Getter; | ||
|
||
@Getter | ||
@XmlRootElement(name = "item") | ||
public class UnregisteredSongResponse { | ||
|
||
@XmlElement(name = "title") | ||
private String title; | ||
|
||
@XmlElement(name = "trackartists", namespace = "http://www.maniadb.com/api") | ||
private SongTrackArtistsResponse trackArtists; | ||
|
||
@XmlElement(name = "album", namespace = "http://www.maniadb.com/api") | ||
private SongAlbumResponse album; | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,14 @@ | ||||||
package shook.shook.song.application.dto.maniadb; | ||||||
|
||||||
import jakarta.xml.bind.annotation.XmlElement; | ||||||
import jakarta.xml.bind.annotation.XmlRootElement; | ||||||
import java.util.List; | ||||||
import lombok.Getter; | ||||||
|
||||||
@Getter | ||||||
@XmlRootElement(name = "channel") | ||||||
public class UnregisteredSongResponses { | ||||||
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. 등록되지 않은 노래 목록 응답이라는 뜻 UnregisteredSongResponses 네이밍을 사용했네요~ 개인적으로 아직 ManiaDB가 DB에 없는 노래만 검색하기 위해서 사용한다는 정책이 모두와 같이 결정된 사항이 아니기도 하고
어차피 ManiaDB api에 종속적인 코드들에서 사용되는 DTO기에 개인적으로는 아래 네이밍이 어떨까 싶어요~
Suggested change
혹시 추후에 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. 넹 이해하신 내용이 맞습니다 그렇다면 ManiaDB에서 가져오는 결과라는 뜻을 좀 더 확실하게 담기 위해 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. i like it👍 |
||||||
|
||||||
@XmlElement(name = "item") | ||||||
private List<UnregisteredSongResponse> songs; | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package shook.shook.song.config; | ||
|
||
import org.springframework.context.annotation.Bean; | ||
import org.springframework.context.annotation.Configuration; | ||
import org.springframework.http.codec.xml.Jaxb2XmlDecoder; | ||
import org.springframework.web.reactive.function.client.ExchangeStrategies; | ||
import org.springframework.web.reactive.function.client.WebClient; | ||
|
||
@Configuration | ||
public class ManiaDBConfiguration { | ||
|
||
private static final String MANIA_DB_BASE_URL = "http://www.maniadb.com/api/search"; | ||
|
||
@Bean | ||
public WebClient getWebClient() { | ||
return WebClient.builder() | ||
.baseUrl(MANIA_DB_BASE_URL) | ||
.exchangeStrategies( | ||
ExchangeStrategies.builder() | ||
.codecs(configurer -> | ||
configurer.defaultCodecs().jaxb2Decoder(new Jaxb2XmlDecoder()) | ||
) | ||
.codecs(configurer -> | ||
configurer.defaultCodecs().maxInMemorySize(4 * 1024 * 1024) | ||
) | ||
.build() | ||
) | ||
.build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package shook.shook.song.exception; | ||
|
||
public class UnregisteredSongException extends RuntimeException { | ||
|
||
public static class EmptyResultException extends UnregisteredSongException { | ||
|
||
public EmptyResultException() { | ||
super(); | ||
} | ||
} | ||
|
||
public static class ManiaDBServerException extends UnregisteredSongException { | ||
|
||
public ManiaDBServerException() { | ||
super(); | ||
} | ||
} | ||
|
||
public static class ManiaDBClientException extends UnregisteredSongException { | ||
|
||
public ManiaDBClientException() { | ||
super(); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package shook.shook.song.ui; | ||
|
||
import java.util.List; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.RestController; | ||
import shook.shook.song.application.ManiaDBSearchService; | ||
import shook.shook.song.application.dto.UnregisteredSongSearchResponse; | ||
|
||
@RequiredArgsConstructor | ||
@RequestMapping("/songs/unregistered/search") | ||
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. 등록되지 않은 상황에서만으로 가정한 기능이였군요! 위에 코멘트에 대한 의문이 풀렸습니다~ 👍 |
||
@RestController | ||
public class ManiaDBApiController { | ||
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. 개인적으로 UnregisteredSongSearchController 인터페이스, UnregisteredSongSearchService 인터페이스 분리를 통하여 ManiaDB를 사용하는 구현체를 둘 수 있는 방법에 대해서 베로도 알고 있을 거에요. 아마 베로도 인터페이스로 모두 분리하기에는 굳이? 와 오버엔지니어링일 꺼라고 생각했을 것 같고 저도 그렇게 생각합니다! Controller 이름만을 컨트롤러 이름이 ManiaDBApiController 지만 사실상 하는 일은 구체적인 비즈니스 로직을 수행하는 Service 레이어의 타입에만 ManiaDB이라는 단어를 포함해도 충분하지 않을 까라는 생각을 해봅니다~ 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 final ManiaDBSearchService maniaDBSearchService; | ||
|
||
@GetMapping | ||
public ResponseEntity<List<UnregisteredSongSearchResponse>> searchUnregisteredSong( | ||
final @RequestParam("keyword") String searchWord | ||
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. 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. 이 부분 빠졌었군요 😅 역시 코드 리뷰의 중요성... |
||
) { | ||
final List<UnregisteredSongSearchResponse> songs = maniaDBSearchService.searchSongs( | ||
searchWord); | ||
|
||
return ResponseEntity.ok(songs); | ||
} | ||
} |
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.
상수와 필드 사이에 줄 간격 두면 가독성이 더 좋아질 것 같아요!