-
Notifications
You must be signed in to change notification settings - Fork 7
R. Yusup #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
base: main
Are you sure you want to change the base?
R. Yusup #2
Changes from all commits
192907a
dd2d235
2bc6fc6
cddde45
425570e
1df502d
fce3e67
ca4bc4d
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,21 @@ | ||
| package net.hackyourfuture.backend.week6.postify; | ||
|
|
||
| import org.springframework.boot.context.event.ApplicationReadyEvent; | ||
| import org.springframework.context.event.EventListener; | ||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Component; | ||
|
|
||
| @Component | ||
| public class DatabaseConnectionCheck { | ||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| public DatabaseConnectionCheck(JdbcTemplate jdbcTemplate) { | ||
| this.jdbcTemplate = jdbcTemplate; | ||
| } | ||
|
|
||
| @EventListener(ApplicationReadyEvent.class) | ||
| public void checkConnection() { | ||
| String result = this.jdbcTemplate.queryForObject("SELECT 'connected'", String.class); | ||
| System.out.println("Database status: " + result); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| package net.hackyourfuture.backend.week6.postify.client; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.client.HttpClientErrorException; | ||
| import org.springframework.web.client.RestClient; | ||
|
|
||
| /** | ||
| * Wraps the external lyrics.ovh API call. | ||
| * Returns Optional.empty() when the API has no lyrics for the song. | ||
| */ | ||
| @Component | ||
| public class LyricsApiClient { | ||
| private final RestClient lyricsRestClient; | ||
|
|
||
| public LyricsApiClient(RestClient lyricsRestClient) { | ||
| this.lyricsRestClient = lyricsRestClient; | ||
| } | ||
|
|
||
| public Optional<String> fetchLyrics(String artistName, String trackTitle) { | ||
| try { | ||
| LyricsResponse response = lyricsRestClient.get() | ||
| .uri("/{artist}/{title}", artistName, trackTitle) | ||
| .retrieve() | ||
| .body(LyricsResponse.class); | ||
|
|
||
| if (response == null || response.lyrics() == null || response.lyrics().isBlank()) { | ||
| return Optional.empty(); | ||
| } | ||
| return Optional.of(response.lyrics()); | ||
| } catch (HttpClientErrorException.NotFound e) { | ||
| return Optional.empty(); | ||
| } | ||
| } | ||
|
|
||
| // Shape of the lyrics.ovh response: {"lyrics": "..."} | ||
|
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. You could add the shape in Javadoc comments that start with Then in the Javadoc segment you can either use All Java versions: An advantage of this is that if you hover over |
||
| private record LyricsResponse(String lyrics) { | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package net.hackyourfuture.backend.week6.postify.config; | ||
|
|
||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.web.client.RestClient; | ||
|
|
||
| @Configuration | ||
| public class RestClientConfig { | ||
|
|
||
| @Bean | ||
| public RestClient lyricsRestClient(RestClient.Builder builder, | ||
| @Value("${lyrics.api.base-url}") String baseUrl) { | ||
|
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. Very nice usage of |
||
| return builder | ||
| .baseUrl(baseUrl) | ||
| .build(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package net.hackyourfuture.backend.week6.postify.controller; | ||
|
|
||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyrics; | ||
| import net.hackyourfuture.backend.week6.postify.service.TrackLyricsService; | ||
|
|
||
| @RestController | ||
| public class TrackLyricsController { | ||
| private final TrackLyricsService service; | ||
|
|
||
| public TrackLyricsController(TrackLyricsService service) { | ||
| this.service = service; | ||
| } | ||
|
|
||
| @GetMapping("/tracks/{id}/lyrics") | ||
| public TrackLyrics getLyrics(@PathVariable Long id) { | ||
|
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. Incredibly thin controller, well-done! |
||
| return service.getLyrics(id); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package net.hackyourfuture.backend.week6.postify.controller; | ||
|
|
||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.UserStatistics; | ||
| import net.hackyourfuture.backend.week6.postify.service.UserStatisticsService; | ||
|
|
||
| @RestController | ||
| public class UserStatisticsController { | ||
| private final UserStatisticsService service; | ||
|
|
||
| public UserStatisticsController(UserStatisticsService service){ | ||
| this.service = service; | ||
| } | ||
| @GetMapping("/users/{id}/stats") | ||
| public UserStatistics getStats(@PathVariable Long id) { | ||
| return service.getStats(id); | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
| import lombok.Builder; | ||
| import lombok.Data; | ||
|
|
||
| @Data | ||
| @Builder | ||
| public class TrackLyrics { | ||
| private long trackId; | ||
| private String trackTitle; | ||
| private String artistName; | ||
| private String lyrics; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
| import lombok.Builder; | ||
| import lombok.Data; | ||
|
|
||
| @Data | ||
| @Builder | ||
| public class UserStatistics { | ||
| private long userId; | ||
| private String userName; | ||
| private String userCountry; | ||
| private long totalStreams; | ||
| private long uniqueTracksStreamed; | ||
| private long uniqueArtistsStreamed; | ||
| private String favoriteGenre; | ||
| private long totalListeningTimeSeconds; | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package net.hackyourfuture.backend.week6.postify.exception; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
|
||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
|
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 would recommend avoiding Annotating an exception with There are two main reasons for that:
Spring’s documentation also warns about this annotation: |
||
| public class LyricsNotFoundException extends RuntimeException { | ||
| public LyricsNotFoundException(String artistName, String trackTitle) { | ||
| super("No lyrics found for \"" + trackTitle + "\" by " + artistName + "."); | ||
|
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. Nicely done on building the message with artistName and trackTitle rather than taking the whole message! If you'd like to do this without concatenation: super("No lyrics found for \"%s\" by %s.".formatted(trackTitle, artistName));Another possible improvement is to take the original exception too and passing it as a cause: public LyricsNotFoundException(String trackTitle, String artistName, Throwable cause) {
super("No lyrics found for \"%s\" by %s.".formatted(trackTitle, artistName), cause);
}This way you won't lose the stacktrace. However, it's good to have both constructors as sometimes a stacktrace wouldn't help much when you're sure an exception had to do with bad user input: public LyricsNotFoundException(String trackTitle, String artistName, Throwable cause) {
super("No lyrics found for \"%s\" by %s.".formatted(trackTitle, artistName), cause);
}
public LyricsNotFoundException(String trackTitle, String artistName) {
this(trackTitle, artistName, null);
} |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package net.hackyourfuture.backend.week6.postify.exception; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
|
||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
| public class TrackNotFoundException extends RuntimeException { | ||
| public TrackNotFoundException(Long id) { | ||
| super("Track not found: " + id); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package net.hackyourfuture.backend.week6.postify.exception; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
|
|
||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
| public class UserNotFoundException extends RuntimeException { | ||
| public UserNotFoundException(Long id) { | ||
| super("User not found: " + id); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package net.hackyourfuture.backend.week6.postify.repository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.UserStatistics; | ||
|
|
||
| @Repository | ||
| public class StreamRepository { | ||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| public StreamRepository(JdbcTemplate jdbcTemplate) { | ||
| this.jdbcTemplate = jdbcTemplate; | ||
| } | ||
|
|
||
| public Optional<UserStatistics> findStats(Long userId) { | ||
| String sql = """ | ||
| SELECT users.user_id, users.user_name, users.user_country, | ||
| COUNT(streams.stream_id) AS total_streams, | ||
| COUNT(DISTINCT streams.track_id) AS unique_tracks, | ||
| COUNT(DISTINCT albums.artist_id) AS unique_artists, | ||
| COALESCE(SUM(tracks.track_duration_s), 0) AS total_listening_seconds | ||
| FROM users | ||
| LEFT JOIN streams ON streams.user_id = users.user_id | ||
| LEFT JOIN tracks ON tracks.track_id = streams.track_id | ||
| LEFT JOIN albums ON albums.album_id = tracks.album_id | ||
| WHERE users.user_id = ? | ||
| GROUP BY users.user_id, users.user_name, users.user_country | ||
| """; | ||
|
|
||
| return jdbcTemplate.query(sql, (rs, rowNum) -> UserStatistics.builder() | ||
| .userId(rs.getLong("user_id")) | ||
| .userName(rs.getString("user_name")) | ||
| .userCountry(rs.getString("user_country")) | ||
| .totalStreams(rs.getLong("total_streams")) | ||
| .uniqueTracksStreamed(rs.getLong("unique_tracks")) | ||
| .uniqueArtistsStreamed(rs.getLong("unique_artists")) | ||
| .totalListeningTimeSeconds(rs.getLong("total_listening_seconds")) | ||
| .build(), | ||
| userId) | ||
| .stream() | ||
| .findFirst(); | ||
|
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. Same comment here as in |
||
| } | ||
|
|
||
|
|
||
| public Optional<String> findFavoriteGenre(Long userId) { | ||
| String sql = """ | ||
| SELECT tracks.genre | ||
| FROM streams | ||
| JOIN tracks ON tracks.track_id = streams.track_id | ||
| WHERE streams.user_id = ? AND tracks.genre IS NOT NULL | ||
| GROUP BY tracks.genre | ||
| ORDER BY COUNT(*) DESC | ||
| LIMIT 1 | ||
| """; | ||
|
|
||
| return jdbcTemplate.query(sql, (rs, rowNum) -> rs.getString("genre"), userId) | ||
| .stream() | ||
| .findFirst(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| package net.hackyourfuture.backend.week6.postify.repository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyrics; | ||
|
|
||
| @Repository | ||
| public class TrackRepository { | ||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| public TrackRepository(JdbcTemplate jdbcTemplate) { | ||
| this.jdbcTemplate = jdbcTemplate; | ||
| } | ||
|
|
||
|
|
||
| public Optional<TrackLyrics> findTrackWithArtist(Long trackId) { | ||
| String sql = """ | ||
| SELECT tracks.track_id, tracks.track_title, artists.artist_name | ||
| FROM tracks | ||
| JOIN albums ON albums.album_id = tracks.album_id | ||
| JOIN artists ON artists.artist_id = albums.artist_id | ||
| WHERE tracks.track_id = ? | ||
| """; | ||
|
|
||
| return jdbcTemplate.query(sql, (rs, rowNum) -> TrackLyrics.builder() | ||
| .trackId(rs.getLong("track_id")) | ||
| .trackTitle(rs.getString("track_title")) | ||
| .artistName(rs.getString("artist_name")) | ||
| .build(), | ||
| trackId) | ||
| .stream() | ||
| .findFirst(); | ||
|
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. Consider using This method is expected to return at most one track because Example: public Optional<TrackLyrics> findTrackWithArtist(Long trackId) {
try {
String sql = """
SELECT tracks.track_id, tracks.track_title, artists.artist_name
FROM tracks
JOIN albums ON albums.album_id = tracks.album_id
JOIN artists ON artists.artist_id = albums.artist_id
WHERE tracks.track_id = ?
""";
TrackLyrics trackLyrics = jdbcTemplate.queryForObject(sql, (rs, rowNum) -> TrackLyrics.builder()
.trackId(rs.getLong("track_id"))
.trackTitle(rs.getString("track_title"))
.artistName(rs.getString("artist_name"))
.build(),
trackId);
return Optional.of(trackLyrics);
} catch (EmptyResultDataAccessException e) {
return Optional.empty();
}
}I would avoid adding |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package net.hackyourfuture.backend.week6.postify.service; | ||
|
|
||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.client.LyricsApiClient; | ||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyrics; | ||
| import net.hackyourfuture.backend.week6.postify.exception.LyricsNotFoundException; | ||
| import net.hackyourfuture.backend.week6.postify.exception.TrackNotFoundException; | ||
| import net.hackyourfuture.backend.week6.postify.repository.TrackRepository; | ||
|
|
||
| @Service | ||
| public class TrackLyricsService { | ||
| private final TrackRepository trackRepository; | ||
| private final LyricsApiClient lyricsApiClient; | ||
|
|
||
| public TrackLyricsService(TrackRepository trackRepository, LyricsApiClient lyricsApiClient) { | ||
|
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. Since you're already using Lombok (In the DTOs, import lombok.RequiredArgsConstructor;
import net.hackyourfuture.backend.week6.postify.client.LyricsApiClient;
import net.hackyourfuture.backend.week6.postify.repository.TrackRepository;
import org.springframework.stereotype.Service;
@Service
@RequiredArgsConstructor
public class TrackLyricsService {
private final TrackRepository trackRepository;
private final LyricsApiClient lyricsApiClient;
// ... getLyrics() method and rest of the class
} |
||
| this.trackRepository = trackRepository; | ||
| this.lyricsApiClient = lyricsApiClient; | ||
| } | ||
|
|
||
| public TrackLyrics getLyrics(Long trackId) { | ||
| TrackLyrics track = trackRepository.findTrackWithArtist(trackId) | ||
| .orElseThrow(() -> new TrackNotFoundException(trackId)); | ||
|
|
||
| String lyrics = lyricsApiClient.fetchLyrics(track.getArtistName(), track.getTrackTitle()) | ||
| .orElseThrow(() -> new LyricsNotFoundException(track.getArtistName(), track.getTrackTitle())); | ||
|
|
||
| track.setLyrics(lyrics); | ||
| return track; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package net.hackyourfuture.backend.week6.postify.service; | ||
|
|
||
| import org.springframework.stereotype.Service; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.UserStatistics; | ||
| import net.hackyourfuture.backend.week6.postify.exception.UserNotFoundException; | ||
| import net.hackyourfuture.backend.week6.postify.repository.StreamRepository; | ||
|
|
||
| @Service | ||
| public class UserStatisticsService { | ||
| private final StreamRepository streamRepository; | ||
|
|
||
| public UserStatisticsService(StreamRepository streamRepository) { | ||
| this.streamRepository = streamRepository; | ||
| } | ||
|
|
||
| public UserStatistics getStats(Long id) { | ||
| // Fetch the aggregate row. Empty means the user id does not exist -> 404. | ||
| UserStatistics stats = streamRepository.findStats(id) | ||
| .orElseThrow(() -> new UserNotFoundException(id)); | ||
|
|
||
| // Favorite genre is a separate query; absent when the user has no streams. | ||
| streamRepository.findFavoriteGenre(id).ifPresent(stats::setFavoriteGenre); | ||
|
|
||
| return stats; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,7 @@ | ||
| spring.application.name=postify | ||
| spring.datasource.url=jdbc:postgresql://localhost:5432/POSTIFY | ||
| spring.datasource.username=hyfuser | ||
| spring.datasource.password=hyfpassword | ||
| spring.datasource.driver-class-name=org.postgresql.Driver | ||
|
|
||
| # External lyrics API (https://lyricsovh.docs.apiary.io) | ||
| lyrics.api.base-url=https://api.lyrics.ovh/v1 |
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.
I really like the short-circuit conditions here, they were immediately readable. Also well done on not having an unnecessary else block.
However, since you are returning an
Optional, have you considered using theOptionalAPI?For Spring projects you can also use
hasText()method inorg.springframework.util.StringUtilsto make it still simpler:What you currently have though is perfectly fine and some prefer it for readability.