-
Notifications
You must be signed in to change notification settings - Fork 7
Salem B. #5
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?
Salem B. #5
Changes from all commits
8d529df
f634b00
cb962a3
ecb88ef
0c13eb1
6db6c68
f68f83d
ae996ed
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,14 @@ | ||
| 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 restClient() { | ||
| return RestClient.create(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package net.hackyourfuture.backend.week6.postify.controller; | ||
|
|
||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyricsResponse; | ||
| import net.hackyourfuture.backend.week6.postify.service.TrackService; | ||
| 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; | ||
|
|
||
|
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. Mooi gebruik van @AllArgsConstructor voor constructor injection. Eventueel zou je hier ook @requiredargsconstructor kunnen gebruiken, omdat die beter aansluit wanneer alleen final dependencies geïnjecteerd hoeven te worden. |
||
| @AllArgsConstructor | ||
| @RestController | ||
| @RequestMapping("/tracks") | ||
| public class TrackController { | ||
|
|
||
| private final TrackService trackService; | ||
|
|
||
| @GetMapping("/{id}/lyrics") | ||
| public TrackLyricsResponse getTrackLyrics(@PathVariable Long id) { | ||
| return trackService.getTrackLyrics(id); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package net.hackyourfuture.backend.week6.postify.controller; | ||
|
|
||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import net.hackyourfuture.backend.week6.postify.dto.UserStatsResponse; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.http.ResponseEntity; | ||
| import net.hackyourfuture.backend.week6.postify.repository.UserRepository; | ||
| 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; | ||
|
|
||
|
|
||
| @AllArgsConstructor | ||
| @RestController | ||
| @RequestMapping("/users") | ||
| public class UserController { | ||
|
|
||
| private final UserRepository userRepository; | ||
|
|
||
| @GetMapping("/{id}/stats") | ||
| public ResponseEntity<?> getUserStatsById(@PathVariable Long id) { | ||
| UserStatsResponse stats = userRepository.getUserStatsById(id); | ||
|
|
||
| return stats != null ? ResponseEntity.ok(stats) | ||
| : ResponseEntity.status(HttpStatus.NOT_FOUND).body("User not found"); | ||
|
|
||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
|
|
||
| @Getter | ||
| @Setter | ||
| public class ExternalLyricsResponse { | ||
| private String lyrics; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
| import lombok.AllArgsConstructor; | ||
| 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. Using @Setter together with @AllArgsConstructor mixes mutable and constructor-based initialization styles. Since this DTO appears to be intended as immutable, I would remove @Setter and rely on constructor-based initialization instead. |
||
| @Getter | ||
| @Setter | ||
| @AllArgsConstructor | ||
| public class TrackLyricsResponse { | ||
| 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,19 @@ | ||
| package net.hackyourfuture.backend.week6.postify.dto; | ||
|
|
||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Getter; | ||
|
|
||
|
|
||
|
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 work! I like how the chosen annotations make the intended immutability of this DTO clear. |
||
| @Getter | ||
| @AllArgsConstructor | ||
| public class UserStatsResponse { | ||
| private final Long userId; | ||
| private final String userName; | ||
| private final String userCountry; | ||
| private final Long totalStreams; | ||
| private final Long uniqueTracksStreamed; | ||
| private final Long uniqueArtistsStreamed; | ||
| private final String favoriteGenre; | ||
| private final Long totalListeningTimeSeconds; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package net.hackyourfuture.backend.week6.postify.repository; | ||
|
|
||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyricsResponse; | ||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| @Repository | ||
| public class TrackRepository { | ||
|
|
||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| public TrackRepository(JdbcTemplate jdbcTemplate) { | ||
| this.jdbcTemplate = jdbcTemplate; | ||
| } | ||
|
|
||
| public TrackLyricsResponse getTrackAndArtistById(Long trackId) { | ||
| String sql = "SELECT " + | ||
| " tracks.track_id AS trackId, " + | ||
| " tracks.track_title AS trackTitle, " + | ||
| " artists.artist_name AS artistName " + | ||
| "FROM tracks " + | ||
| "JOIN albums ON tracks.album_id = albums.album_id " + | ||
| "JOIN artists ON albums.artist_id = artists.artist_id " + | ||
| "WHERE tracks.track_id = ?;"; | ||
|
|
||
| try { | ||
| return jdbcTemplate.queryForObject(sql, (rs, rowNum) -> | ||
| new TrackLyricsResponse( | ||
| rs.getLong("trackId"), | ||
| rs.getString("trackTitle"), | ||
| rs.getString("artistName"), | ||
| null // from external API | ||
| ), | ||
| 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. This catches all exceptions and returns null, which may hide real production issues such as SQL errors or database/connectivity problems. Consider catching only EmptyResultDataAccessException for the “not found” case and letting unexpected errors propagate or be logged. |
||
| } catch (Exception e) { | ||
| return 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. Returning null is prone to NullPointerExceptions in the calling code. Consider using Java’s Optional to make the “not found” case explicit, or even better, throw a domain-specific TrackNotFoundException. |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package net.hackyourfuture.backend.week6.postify.repository; | ||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.UserStatsResponse; | ||
| import org.springframework.jdbc.core.JdbcTemplate; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| @Repository | ||
| public class UserRepository { | ||
|
|
||
| private final JdbcTemplate jdbcTemplate; | ||
|
|
||
| public UserRepository(JdbcTemplate jdbcTemplate) { | ||
| this.jdbcTemplate = jdbcTemplate; | ||
| } | ||
|
|
||
| public UserStatsResponse getUserStatsById(Long id) { | ||
|
|
||
| String mainSql = "SELECT " + | ||
| " users.user_id AS userId, " + | ||
| " users.user_name AS userName, " + | ||
| " users.user_country AS userCountry, " + | ||
| " COUNT(streams.stream_id) AS totalStreams, " + | ||
| " COUNT(DISTINCT streams.track_id) AS uniqueTracksStreamed, " + | ||
| " COUNT(DISTINCT albums.artist_id) AS uniqueArtistsStreamed, " + | ||
| " COALESCE(SUM(tracks.track_duration_s), 0) AS totalListeningTimeSeconds " + | ||
| "FROM users " + | ||
| "LEFT JOIN streams ON users.user_id = streams.user_id " + | ||
| "LEFT JOIN tracks ON streams.track_id = tracks.track_id " + | ||
| "LEFT JOIN albums ON tracks.album_id = albums.album_id " + | ||
| "WHERE users.user_id = ? " + | ||
| "GROUP BY users.user_id, users.user_name, users.user_country;"; | ||
|
|
||
| String genreSql = "SELECT tracks.genre " + | ||
| "FROM streams " + | ||
| "JOIN tracks ON streams.track_id = tracks.track_id " + | ||
| "WHERE streams.user_id = ? " + | ||
| "GROUP BY tracks.genre " + | ||
| "ORDER BY COUNT(streams.stream_id) DESC " + | ||
| "LIMIT 1;"; | ||
|
|
||
| try { | ||
| String favoriteGenre; | ||
| try { | ||
| favoriteGenre = jdbcTemplate.queryForObject(genreSql, String.class, 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. The repository currently contains presentation logic by returning "None" as a fallback value. Consider handling this default value in the service or response layer instead. |
||
| if (favoriteGenre == null) { | ||
| favoriteGenre = "None"; | ||
| } | ||
| } catch (Exception e) { | ||
|
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. Catch only the exception you expect here. |
||
| favoriteGenre = "None"; | ||
| } | ||
|
|
||
| final String finalGenre = favoriteGenre; | ||
|
|
||
| return jdbcTemplate.queryForObject(mainSql, (rs, rowNum) -> | ||
| new UserStatsResponse( | ||
| rs.getLong("userId"), | ||
| rs.getString("userName"), | ||
| rs.getString("userCountry"), | ||
| rs.getLong("totalStreams"), | ||
| rs.getLong("uniqueTracksStreamed"), | ||
| rs.getLong("uniqueArtistsStreamed"), | ||
| finalGenre, | ||
| rs.getLong("totalListeningTimeSeconds") | ||
| ), | ||
| id | ||
| ); | ||
|
|
||
| } catch (Exception e) { | ||
| return 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. Catch only the exception you expect here. |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package net.hackyourfuture.backend.week6.postify.service; | ||
|
|
||
|
|
||
| import net.hackyourfuture.backend.week6.postify.dto.ExternalLyricsResponse; | ||
| import net.hackyourfuture.backend.week6.postify.dto.TrackLyricsResponse; | ||
| import net.hackyourfuture.backend.week6.postify.repository.TrackRepository; | ||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.stereotype.Service; | ||
| import org.springframework.web.client.RestClient; | ||
| import org.springframework.web.server.ResponseStatusException; | ||
|
|
||
| @Service | ||
| public class TrackService { | ||
|
|
||
| private final TrackRepository trackRepository; | ||
| private final RestClient restClient; | ||
|
|
||
| public TrackService(TrackRepository trackRepository, RestClient restClient) { | ||
| this.trackRepository = trackRepository; | ||
| this.restClient = restClient; | ||
| } | ||
|
|
||
| public TrackLyricsResponse getTrackLyrics(Long trackId) { | ||
|
|
||
| TrackLyricsResponse trackInfo = trackRepository.getTrackAndArtistById(trackId); | ||
|
|
||
| if (trackInfo == null) { | ||
| throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Track not found in our database"); | ||
| } | ||
|
|
||
|
|
||
| String url = "https://api.lyrics.ovh/v1/" + trackInfo.getArtistName() + "/" + trackInfo.getTrackTitle(); | ||
|
|
||
| try { | ||
|
|
||
| ExternalLyricsResponse externalResponse = restClient.get() | ||
| .uri(url) | ||
| .retrieve() | ||
| .body(ExternalLyricsResponse.class); | ||
|
|
||
|
|
||
| if (externalResponse != null && externalResponse.getLyrics() != null) { | ||
| return new TrackLyricsResponse( | ||
| trackInfo.getTrackId(), | ||
| trackInfo.getTrackTitle(), | ||
| trackInfo.getArtistName(), | ||
| externalResponse.getLyrics() | ||
| ); | ||
| } else { | ||
| throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Lyrics not found for this track"); | ||
| } | ||
|
|
||
| } catch (Exception e) { | ||
|
|
||
| throw new ResponseStatusException(HttpStatus.NOT_FOUND, "The lyrics API has no lyrics for this track."); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,4 @@ | ||
| 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 |
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.
Consider placing more of the static configuration for this RestClient in one central setup location. For example, the baseUrl could be configured once when creating the client, instead of being repeated or handled separately in the individual calls.