-
Notifications
You must be signed in to change notification settings - Fork 7
Dagim H. #4
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?
Dagim H. #4
Changes from all commits
653144b
f45732a
5a1f3af
20fd0bd
ea8c4f5
0e2d640
3abef7a
0852d49
9508ba8
2c3677a
c290957
0f9d340
9c13433
ee8347f
21c4992
2eacbe0
8c1cf4e
5de1082
ca9d674
dfa31bd
634853e
3fa14a6
a9a18d5
5a898f3
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,16 @@ | ||
| package net.hackyourfuture.backend.week6.postify.config; | ||
|
|
||
| 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() { | ||
| return RestClient.builder() | ||
| .baseUrl("https://api.lyrics.ovh/v1") | ||
| .build(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
|
|
||
| package net.hackyourfuture.backend.week6.postify.controller; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyricsResponse; | ||
| import net.hackyourfuture.backend.week6.postify.service.TrackLyricsService; | ||
| import org.springframework.web.bind.annotation.*; | ||
|
|
||
| @RestController | ||
| @RequestMapping("/tracks") | ||
| public class TrackLyricsController { | ||
|
|
||
| private final TrackLyricsService trackLyricsService; | ||
|
|
||
| public TrackLyricsController( | ||
| TrackLyricsService trackLyricsService) { | ||
|
|
||
| this.trackLyricsService = trackLyricsService; | ||
| } | ||
|
|
||
| @GetMapping("/{id}/lyrics") | ||
| public TrackLyricsResponse getLyrics( | ||
| @PathVariable Long id) { | ||
|
|
||
| return trackLyricsService.getLyrics(id); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package net.hackyourfuture.backend.week6.postify.controller; | ||
|
|
||
|
|
||
| import net.hackyourfuture.backend.week6.postify.model.UserStatisticsResponse; | ||
| import net.hackyourfuture.backend.week6.postify.service.UserStatisticsService; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.RequestMapping; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
|
|
||
|
|
||
| @RestController | ||
| @RequestMapping("/users") | ||
| public class UserStatisticsController { | ||
|
|
||
| @Autowired | ||
| private UserStatisticsService service; | ||
|
|
||
| @GetMapping("/{id}/statistics") | ||
| public ResponseEntity<?> getStats(@PathVariable int id) { | ||
| UserStatisticsResponse stats = service.getStats(id); | ||
|
|
||
| if (stats == null) { | ||
| return ResponseEntity.status(404).body("User not found"); | ||
| } | ||
|
|
||
| return ResponseEntity.ok(stats); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
|
|
||
|
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. This is a solid approach for object deserialization on DTOs for API calls. However, the use of @Setter suggests that the DTO is mutable. Using the record feature of Java instead could be a good idea here to make the immutability more clear. Jackson works very well with Java records. |
||
| @Getter | ||
| @Setter | ||
| public class LyricsApiResponse { | ||
|
|
||
| private String lyrics; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import lombok.Setter; | ||
|
|
||
|
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. This model mixes constructor style injection with setter injection. It is better to make a choice here: either a @Setter or an @AllArgsConstructor. In this case, the dto is immutable so constructor injection would be the best choice. Then the @Setter and @NoArgsContructor can be removed. |
||
| @Getter | ||
| @Setter | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| public class TrackLyricsResponse { | ||
| private int trackId; | ||
| private String trackTitle; | ||
| private String artistName; | ||
| private String lyrics; | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package net.hackyourfuture.backend.week6.postify.exception; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.ExceptionHandler; | ||
| import org.springframework.web.bind.annotation.ResponseStatus; | ||
| import org.springframework.web.bind.annotation.RestControllerAdvice; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
|
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. Nice GlobalExceptionHandler. Also the 404 for the UserStatistics service could be added here. |
||
| @RestControllerAdvice | ||
| public class GlobalExceptionHandler { | ||
|
|
||
| @ExceptionHandler(TrackNotFoundException.class) | ||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
| public Map<String, String> handleTrackNotFound( | ||
| TrackNotFoundException exception) { | ||
|
|
||
| return Map.of( | ||
| "message", | ||
| exception.getMessage() | ||
| ); | ||
| } | ||
|
|
||
| @ExceptionHandler(LyricsNotFoundException.class) | ||
| @ResponseStatus(HttpStatus.NOT_FOUND) | ||
| public Map<String, String> handleLyricsNotFound( | ||
| LyricsNotFoundException exception) { | ||
|
|
||
| return Map.of( | ||
| "message", | ||
| exception.getMessage() | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
|
|
||
| package net.hackyourfuture.backend.week6.postify.exception; | ||
|
|
||
| public class LyricsNotFoundException extends RuntimeException { | ||
|
|
||
| public LyricsNotFoundException(String artistName, String trackTitle) { | ||
| super("Lyrics not found for '" + trackTitle + "' by " + artistName); | ||
| } | ||
|
|
||
| } | ||
|
|
||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package net.hackyourfuture.backend.week6.postify.exception; | ||
|
|
||
| public class TrackNotFoundException extends RuntimeException { | ||
| public TrackNotFoundException(int trackId) { | ||
| super("Track with id " + trackId + " not found"); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package net.hackyourfuture.backend.week6.postify.model; | ||
|
|
||
| import com.fasterxml.jackson.annotation.JsonPropertyOrder; | ||
| import lombok.AllArgsConstructor; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
| import lombok.Setter; | ||
|
|
||
| @JsonPropertyOrder({ | ||
| "userId", | ||
| "userName", | ||
| "userCountry", | ||
| "totalStreams", | ||
| "uniqueTracksStreamed", | ||
| "uniqueArtistsStreamed", | ||
| "favoriteGenre", | ||
| "totalListeningTimeSeconds" | ||
| }) | ||
|
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. Here again, choose either constructor- or setter dependency injection style. |
||
| @Getter | ||
| @Setter | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| public class UserStatisticsResponse { | ||
|
|
||
|
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. Given the annotations already present on this class, these fields should be private. Public fields are generally best avoided in favor of methods, because methods preserve encapsulation and give us more flexibility to change the implementation later, such as computing a value from other fields instead of storing it directly. |
||
| public int userId; | ||
| public String userName; | ||
| public String userCountry; | ||
| public int totalStreams; | ||
| public int uniqueTracksStreamed; | ||
| public int uniqueArtistsStreamed; | ||
| public String favoriteGenre; | ||
| public int totalListeningTimeSeconds; | ||
|
|
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| package net.hackyourfuture.backend.week6.postify.repository; | ||
|
|
||
|
|
||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
|
|
||
| @Repository | ||
| public class TrackRepository { | ||
|
|
||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| public TrackRepository(JdbcTemplate jdbcTemplate) { | ||
| this.jdbcTemplate = jdbcTemplate; | ||
| } | ||
|
|
||
| public Optional<TrackWithArtist> findTrackWithArtistsById(int trackId) { | ||
| String sql = """ | ||
| SELECT | ||
| t.track_id, | ||
| t.track_title, | ||
| a.artist_name | ||
| FROM tracks t | ||
| JOIN albums al ON t.album_id = al.album_id | ||
| JOIN artists a ON al.artist_id = a.artist_id | ||
| WHERE t.track_id = ? | ||
| """; | ||
|
|
||
| return jdbcTemplate.query(sql, rs -> { | ||
| if (!rs.next()) { | ||
| return Optional.empty(); | ||
| } | ||
| return Optional.of(new TrackWithArtist( | ||
| rs.getInt("track_id"), | ||
| rs.getString("track_title"), | ||
| rs.getString("artist_name") | ||
| )); | ||
|
|
||
| }, trackId); | ||
| } | ||
|
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. Nice choice choosing a record here. For now, since there is no update logic you could make the records immutable using the final keyword on its fields. |
||
| public record TrackWithArtist(int trackId, String trackTitle, String 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. Since this is also part of the model, it would be more clear to put this record definition not as an inner class, but give a a separate file in de model package. Additionally, the dto package could even be a subpackage of model to maintain more overview over all used models in the application. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package net.hackyourfuture.backend.week6.postify.repository; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.model.UserStatisticsResponse; | ||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import java.util.List; | ||
|
|
||
| @Repository | ||
| public class UserStatisticsRepository { | ||
|
|
||
| @Autowired | ||
| private JdbcTemplate jdbc; | ||
|
|
||
| public UserStatisticsResponse getUserStats(int userId) { | ||
| String sql = """ | ||
| SELECT | ||
| u.user_id AS user_id, | ||
| u.user_name AS user_name, | ||
| u.user_country AS user_country, | ||
| COUNT(s.stream_id) AS total_streams, | ||
| COUNT(DISTINCT t.track_id) AS unique_tracks, | ||
| COUNT(DISTINCT ar.artist_id) AS unique_artists, | ||
| ( | ||
| SELECT t2.genre | ||
| FROM streams s2 | ||
| JOIN tracks t2 ON s2.track_id = t2.track_id | ||
| JOIN albums al2 ON t2.album_id = al2.album_id | ||
| JOIN artists ar2 ON al2.artist_id = ar2.artist_id | ||
| WHERE s2.user_id = u.user_id | ||
| GROUP BY t2.genre | ||
| ORDER BY COUNT(*) DESC | ||
| LIMIT 1 | ||
| ) AS favorite_genre, | ||
| SUM(t.track_duration_s) AS total_listening_seconds | ||
| FROM users u | ||
| LEFT JOIN streams s ON u.user_id = s.user_id | ||
| LEFT JOIN tracks t ON s.track_id = t.track_id | ||
| LEFT JOIN albums al ON t.album_id = al.album_id | ||
| LEFT JOIN artists ar ON al.artist_id = ar.artist_id | ||
| WHERE u.user_id = ? | ||
| GROUP BY u.user_id, u.user_name, u.user_country | ||
| """; | ||
|
|
||
| List<UserStatisticsResponse> result = jdbc.query(sql, (rs, rowNum) -> { | ||
| UserStatisticsResponse r = new UserStatisticsResponse(); | ||
| r.userId = rs.getInt("user_id"); | ||
| r.userName = rs.getString("user_name"); | ||
| r.userCountry = rs.getString("user_country"); | ||
| r.totalStreams = rs.getInt("total_streams"); | ||
| r.uniqueTracksStreamed = rs.getInt("unique_tracks"); | ||
| r.uniqueArtistsStreamed = rs.getInt("unique_artists"); | ||
| r.favoriteGenre = rs.getString("favorite_genre"); | ||
| r.totalListeningTimeSeconds = rs.getInt("total_listening_seconds"); | ||
| return r; | ||
| }, userId); | ||
|
|
||
| return result.isEmpty() ? null : result.get(0); | ||
| } | ||
| } |
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.
This null check can be avoided nicely by using the GlobalExceptionHandler to produce a 404.