Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions postify/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-webmvc</artifactId>
</dependency>
<dependency>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-jdbc</artifactId>
</dependency>

<dependency>
<groupId>org.postgresql</groupId>
Expand Down
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) {

Copy link
Copy Markdown

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.

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()) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LyricsApiException for non-404 here are not caught by ApiExceptionHandler (only ResponseStatusException is handled). This ends up as a generic 500 INTERNAL_SERVER_ERROR.

You can also handle LyricsApiException in the ApiExceptionHandler and convert it to consistent response for the client.

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();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicit type parameter <String> is not needed here.

With Optional<T>, T gets inferred by the compiler on return. If your functions' return is String then it gets inferred as Optional<String>.

}
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is up to preference but you could use Lombok here to automatically generate the constructor. That'd be using RequiredArgsConstructor

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(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good use of ResponseStatusException but one important note because this is already a Spring web exception, Spring MVC translates it directly into an HTTP error response. That means your custom ApiExceptionHandler may not always be the code path responsible for the response, even though the endpoint still returns the correct 404. Please see my comment on ApiExceptionHandler.

Another thing to keep in mind: For larger applications: creating specific exception types, such as TrackNotFoundException or LyricsNotFoundException also helps logs and analytics easier to navigate.
That is because you can filter errors by exception type when you investigate production issues. You don't need that for this assignment yet; it is more relevant later when you get into observability and monitoring in Week 13.

HttpStatus.NOT_FOUND, "Track " + id + " does not exist"));

String lyrics = lyricsClient.fetchLyrics(track.artistName(), track.trackTitle())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 TrackLyrics response.

Then ideally your service would throw a domain-specific exception that would become an HTTP error response in your ExceptionHandler class with @RestControllerAdvice

Keeping controllers thin has two main benefits.

  • First, the same use case can be reused from another controller, a scheduled job, a message consumer, or a CLI command without copying controller logic.
  • Second, service methods are usually easier to test directly because they do not require setting up the web layer.

.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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResponseStatusException is already a Spring web exception, Spring MVC can translates it directly into an HTTP error response. That means it won't reach this handler.

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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice null handling here. If a null is passed here error response becomes a generic 500 with no immediately clear reason why.

));
}
}
9 changes: 9 additions & 0 deletions postify/src/main/resources/application.properties
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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.
Since the changes for Testcontainers setup won't be fitting in a review comment, I added a branch on my fork for a Testcontainer setup.
Currently there's nothing to test but it does boots up a docker container with mocked testdata which allow data to be retrieved and remove the need for mocking Repository. Please take a look and adapt if needed: https://github.com/HackYourAssignment/c55-backend-week-6/compare/main...cometbroom:example/testcontainers-pgdb-mock?expand=1


@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"));
}
}
Loading