-
Notifications
You must be signed in to change notification settings - Fork 7
Shadi A. #6
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?
Shadi A. #6
Changes from all commits
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,44 @@ | ||
| package net.hackyourfuture.backend.week6.postify.tracks; | ||
|
|
||
| import org.springframework.beans.factory.annotation.Value; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.web.client.RestClient; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @Component | ||
| public class LyricsClient { | ||
|
|
||
| private final RestClient restClient; | ||
|
|
||
| public LyricsClient(@Value("${postify.lyrics-api.base-url}") String baseUrl) { | ||
| this.restClient = RestClient.builder().baseUrl(baseUrl).build(); | ||
| } | ||
|
|
||
| public Optional<String> fetchLyrics(String artistName, String trackTitle) { | ||
| return restClient.get() | ||
| .uri("/v1/{artist}/{title}", artistName, trackTitle) | ||
| .exchange((request, response) -> { | ||
| if (response.getStatusCode().value() == 404) { | ||
| return Optional.<String>empty(); | ||
| } | ||
| if (response.getStatusCode().isError()) { | ||
|
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 can also handle |
||
| throw new LyricsApiException( | ||
| "Lyrics API returned " + response.getStatusCode()); | ||
| } | ||
| LyricsResponse body = response.bodyTo(LyricsResponse.class); | ||
| if (body == null || body.lyrics() == null || body.lyrics().isBlank()) { | ||
| return Optional.<String>empty(); | ||
|
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. Explicit type parameter With |
||
| } | ||
| return Optional.of(body.lyrics()); | ||
| }); | ||
| } | ||
|
|
||
| private record LyricsResponse(String lyrics) {} | ||
|
|
||
| public static class LyricsApiException extends RuntimeException { | ||
| public LyricsApiException(String message) { | ||
| super(message); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| package net.hackyourfuture.backend.week6.postify.tracks; | ||
|
|
||
| public record TrackInfo(int trackId, String trackTitle, String artistName) {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package net.hackyourfuture.backend.week6.postify.tracks; | ||
|
|
||
| public record TrackLyrics( | ||
| int trackId, | ||
| String trackTitle, | ||
| String artistName, | ||
| String lyrics | ||
| ) {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package net.hackyourfuture.backend.week6.postify.tracks; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
| import org.springframework.web.server.ResponseStatusException; | ||
|
|
||
| @RestController | ||
| public class TrackLyricsController { | ||
|
|
||
| private final TrackRepository trackRepository; | ||
| private final LyricsClient lyricsClient; | ||
|
|
||
| public TrackLyricsController(TrackRepository trackRepository, LyricsClient lyricsClient) { | ||
| this.trackRepository = trackRepository; | ||
| this.lyricsClient = lyricsClient; | ||
|
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 up to preference but you could use import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.RestController;
@RestController
@RequiredArgsConstructor
public class TrackLyricsService {
private final TrackRepository trackRepository;
private final LyricsApiClient lyricsApiClient;
}Having the constructor written down is also completely fine. Some people prefer that as the injection mechanism might be clearer. |
||
| } | ||
|
|
||
| @GetMapping("/tracks/{id}/lyrics") | ||
| public TrackLyrics getTrackLyrics(@PathVariable int id) { | ||
| TrackInfo track = trackRepository.findById(id) | ||
| .orElseThrow(() -> new ResponseStatusException( | ||
|
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. Good use of Another thing to keep in mind: For larger applications: creating specific exception types, such as |
||
| HttpStatus.NOT_FOUND, "Track " + id + " does not exist")); | ||
|
|
||
| String lyrics = lyricsClient.fetchLyrics(track.artistName(), track.trackTitle()) | ||
|
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 clear flow in this controller. As the application grows, though, this orchestration logic is better placed in a service layer: find the track, fetch the lyrics, decide what happens when either one is missing, and return the final Then ideally your service would throw a domain-specific exception that would become an HTTP error response in your ExceptionHandler class with Keeping controllers thin has two main benefits.
|
||
| .orElseThrow(() -> new ResponseStatusException( | ||
| HttpStatus.NOT_FOUND, | ||
| "No lyrics available for '" + track.trackTitle() | ||
| + "' by " + track.artistName())); | ||
|
|
||
| return new TrackLyrics(track.trackId(), track.trackTitle(), track.artistName(), lyrics); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package net.hackyourfuture.backend.week6.postify.tracks; | ||
|
|
||
| import org.springframework.jdbc.core.simple.JdbcClient; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @Repository | ||
| public class TrackRepository { | ||
|
|
||
| private final JdbcClient jdbc; | ||
|
|
||
| public TrackRepository(JdbcClient jdbc) { | ||
| this.jdbc = jdbc; | ||
| } | ||
|
|
||
| public Optional<TrackInfo> findById(int trackId) { | ||
| return jdbc.sql(""" | ||
| SELECT t.track_id, t.track_title, ar.artist_name | ||
| FROM tracks t | ||
| JOIN albums al ON t.album_id = al.album_id | ||
| JOIN artists ar ON al.artist_id = ar.artist_id | ||
| WHERE t.track_id = :id | ||
| """) | ||
| .param("id", trackId) | ||
| .query((rs, rowNum) -> new TrackInfo( | ||
| rs.getInt("track_id"), | ||
| rs.getString("track_title"), | ||
| rs.getString("artist_name"))) | ||
| .optional(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package net.hackyourfuture.backend.week6.postify.users; | ||
|
|
||
| public record UserStats( | ||
| int userId, | ||
| String userName, | ||
| String userCountry, | ||
| long totalStreams, | ||
| long uniqueTracksStreamed, | ||
| long uniqueArtistsStreamed, | ||
| String favoriteGenre, | ||
| long totalListeningTimeSeconds | ||
| ) {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package net.hackyourfuture.backend.week6.postify.users; | ||
|
|
||
| import org.springframework.http.HttpStatus; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.RestController; | ||
| import org.springframework.web.server.ResponseStatusException; | ||
|
|
||
| @RestController | ||
| public class UserStatsController { | ||
|
|
||
| private final UserStatsRepository repository; | ||
|
|
||
| public UserStatsController(UserStatsRepository repository) { | ||
| this.repository = repository; | ||
| } | ||
|
|
||
| @GetMapping("/users/{id}/stats") | ||
| public UserStats getUserStats(@PathVariable int id) { | ||
| return repository.findStatsByUserId(id) | ||
| .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package net.hackyourfuture.backend.week6.postify.users; | ||
|
|
||
| import org.springframework.jdbc.core.simple.JdbcClient; | ||
| import org.springframework.stereotype.Repository; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| @Repository | ||
| public class UserStatsRepository { | ||
|
|
||
| private final JdbcClient jdbc; | ||
|
|
||
| public UserStatsRepository(JdbcClient jdbc) { | ||
| this.jdbc = jdbc; | ||
| } | ||
|
|
||
| public Optional<UserStats> findStatsByUserId(int userId) { | ||
| // Query 1: confirm the user exists and grab profile fields. | ||
| var profile = jdbc.sql(""" | ||
| SELECT user_id, user_name, user_country | ||
| FROM users | ||
| WHERE user_id = :id | ||
| """) | ||
| .param("id", userId) | ||
| .query((rs, rowNum) -> new UserProfile( | ||
| rs.getInt("user_id"), | ||
| rs.getString("user_name"), | ||
| rs.getString("user_country"))) | ||
| .optional(); | ||
|
|
||
| if (profile.isEmpty()) { | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| // Query 2: aggregate listening stats across streams -> tracks -> albums -> artists. | ||
| // favoriteGenre is computed via a correlated scalar subquery with GROUP BY + LIMIT 1. | ||
| UserProfile p = profile.get(); | ||
| UserStats stats = jdbc.sql(""" | ||
| SELECT | ||
| COUNT(*) AS total_streams, | ||
| COUNT(DISTINCT t.track_id) AS unique_tracks, | ||
| COUNT(DISTINCT al.artist_id) AS unique_artists, | ||
| COALESCE(SUM(t.track_duration_s), 0) AS total_seconds, | ||
| ( | ||
| SELECT t2.genre | ||
| FROM streams s2 | ||
| JOIN tracks t2 ON s2.track_id = t2.track_id | ||
| WHERE s2.user_id = :id AND t2.genre IS NOT NULL | ||
| GROUP BY t2.genre | ||
| ORDER BY COUNT(*) DESC | ||
| LIMIT 1 | ||
| ) AS favorite_genre | ||
| FROM streams s | ||
| JOIN tracks t ON s.track_id = t.track_id | ||
| JOIN albums al ON t.album_id = al.album_id | ||
| WHERE s.user_id = :id | ||
| """) | ||
| .param("id", userId) | ||
| .query((rs, rowNum) -> new UserStats( | ||
| p.userId(), | ||
| p.userName(), | ||
| p.userCountry(), | ||
| rs.getLong("total_streams"), | ||
| rs.getLong("unique_tracks"), | ||
| rs.getLong("unique_artists"), | ||
| rs.getString("favorite_genre"), | ||
| rs.getLong("total_seconds"))) | ||
| .single(); | ||
|
|
||
| return Optional.of(stats); | ||
| } | ||
|
|
||
| private record UserProfile(int userId, String userName, String userCountry) {} | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package net.hackyourfuture.backend.week6.postify.web; | ||
|
|
||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.web.bind.annotation.ExceptionHandler; | ||
| import org.springframework.web.bind.annotation.RestControllerAdvice; | ||
| import org.springframework.web.server.ResponseStatusException; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| @RestControllerAdvice | ||
| public class ApiExceptionHandler { | ||
|
|
||
| @ExceptionHandler(ResponseStatusException.class) | ||
|
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 you want full control over the error response body, often you'd have to throw your own exception. |
||
| public ResponseEntity<Map<String, Object>> handleResponseStatus(ResponseStatusException ex) { | ||
| return ResponseEntity | ||
| .status(ex.getStatusCode()) | ||
| .body(Map.of( | ||
| "status", ex.getStatusCode().value(), | ||
| "message", ex.getReason() == null ? "" : ex.getReason() | ||
|
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 null handling here. If a |
||
| )); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,10 @@ | ||
| 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 | ||
|
|
||
| postify.lyrics-api.base-url=https://api.lyrics.ovh | ||
|
|
||
| spring.web.error.include-message=always |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package net.hackyourfuture.backend.week6.postify.tracks; | ||
|
|
||
| import org.junit.jupiter.api.Test; | ||
| import org.springframework.beans.factory.annotation.Autowired; | ||
| import org.springframework.test.context.bean.override.mockito.MockitoBean; | ||
| import org.springframework.test.web.servlet.MockMvc; | ||
| import org.springframework.boot.webmvc.test.autoconfigure.WebMvcTest; | ||
|
|
||
| import java.util.Optional; | ||
|
|
||
| import static org.mockito.Mockito.when; | ||
| import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; | ||
| import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; | ||
|
|
||
| @WebMvcTest(TrackLyricsController.class) | ||
| class TrackLyricsControllerTest { | ||
|
|
||
| @Autowired | ||
| private MockMvc mockMvc; | ||
|
|
||
| @MockitoBean | ||
| private TrackRepository trackRepository; | ||
|
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 was not covered in the assignment requirement but for a true integration tests, repository layer shouldn't be mocked either. Only the database itself would then have to be mocked with an h2 database or Testcontainers. |
||
|
|
||
| @MockitoBean | ||
| private LyricsClient lyricsClient; | ||
|
|
||
| @Test | ||
| void returnsLyricsForExistingTrack() throws Exception { | ||
| TrackInfo track = new TrackInfo(41, "LUNCH", "Billie Eilish"); | ||
| when(trackRepository.findById(41)).thenReturn(Optional.of(track)); | ||
| when(lyricsClient.fetchLyrics("Billie Eilish", "LUNCH")) | ||
| .thenReturn(Optional.of("I could eat that girl for lunch...")); | ||
|
|
||
| mockMvc.perform(get("/tracks/{id}/lyrics", 41)) | ||
| .andExpect(status().isOk()) | ||
| .andExpect(jsonPath("$.trackId").value(41)) | ||
| .andExpect(jsonPath("$.trackTitle").value("LUNCH")) | ||
| .andExpect(jsonPath("$.artistName").value("Billie Eilish")) | ||
| .andExpect(jsonPath("$.lyrics").value("I could eat that girl for lunch...")); | ||
| } | ||
|
|
||
| @Test | ||
| void returns404WhenTrackDoesNotExist() throws Exception { | ||
| when(trackRepository.findById(9999)).thenReturn(Optional.empty()); | ||
|
|
||
| mockMvc.perform(get("/tracks/{id}/lyrics", 9999)) | ||
| .andExpect(status().isNotFound()) | ||
| .andExpect(jsonPath("$.message").value("Track 9999 does not exist")); | ||
| } | ||
|
|
||
| @Test | ||
| void returns404WhenLyricsApiHasNoLyrics() throws Exception { | ||
| TrackInfo track = new TrackInfo(7, "Some Dutch Song", "Een Artiest"); | ||
| when(trackRepository.findById(7)).thenReturn(Optional.of(track)); | ||
| when(lyricsClient.fetchLyrics("Een Artiest", "Some Dutch Song")) | ||
| .thenReturn(Optional.empty()); | ||
|
|
||
| mockMvc.perform(get("/tracks/{id}/lyrics", 7)) | ||
| .andExpect(status().isNotFound()) | ||
| .andExpect(jsonPath("$.message") | ||
| .value("No lyrics available for 'Some Dutch Song' by Een Artiest")); | ||
| } | ||
| } | ||
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.
Awesome job returning an
Optional<String>here! Then each service which relies on this client can decide if they need it so much that they throw an error or just continue.