Monera A

[Miuroro]

Summary

This PR completes Task 1 and Task 2 for the Week 6 Assignment, introducing user statistics tracking and external lyrics integration.

Features Implemented

• User Stats Endpoint (GET /users/{id}/stats): Added a single aggregate SQL query to calculate listening metrics (streams, unique tracks/artists, favorite genre, and listening time) entirely in the database. Returns a 404 Not Found if the user is missing.
• Track Lyrics Endpoint (GET /tracks/{id}/lyrics): Added a database lookup for track metadata and connected Spring's RestClient to fetch live song lyrics from the external lyrics.ovh API. Returns a descriptive 404 Not Found message if lyrics or tracks are missing.

Automated Testing Added

• Task 1 Tests: MockMvc integration tests verifying the happy path response fields and the 404 user-not-found error code.
• Task 2 Tests: MockMvc integration tests verifying successful track lyrics retrieval, a database track miss (404), and an external API lyrics miss (404 with a custom error message).

[cometbroom]

Well done Monera! This covers both required endpoints and follows a recognizable, layered Spring Boot structure with controllers, services, repositories, DTOs.
SQL statements are also very well-written. Clean use of table aliases and proper JOIN syntax selecting only needed columns. Using COUNT(DISTINCT ...) to avoid inflated counts. COALESCE() for null-handling. Correctly uses COUNT(DISTINCT ...) to avoid inflated counts from multiple joins. Proper use of GROUP BY. And using parameterized queries with ? preventing SQLi attacks.

The main area to improve is test reliability and dependency design. The lyrics service creates its own RestClient internally, even though a configured RestClient bean exists. This makes the application harder to test because the integration test depends on the live external lyrics API instead of a controlled mock. Please see my comment on that.

[cometbroom]

Minor architectural point: Services should stay free of HTTP/web concerns.
Throwing ResponseStatusException here works for now, but it couples the service to the specific HTTP layer you're building. If you later wanted to reuse the same service from a batch job, a message consumer, or a different kind of controller, you would need to change it.

Since your repository layer already returns an Optional, an option is to return an Optional from the service too. Then you'd do the orElse logic in the controller.

Alternatively, you can throw a domain-specific exception from the service and handle the HTTP mapping in GlobalExceptionHandler or another @RestControllerAdvice. Then services express what went wrong in business terms, while the exception handler decides how that problem should be represented in an HTTP response.

[cometbroom]

You could simplify this by inverting the condition.
Current: Is apiResponse and apiResponse.lyrics() not null? Then do happy flow.
Inverted: Is apiResponse or apiResponse.lyrics() null? Then throw exception.

if (apiResponse == null || apiResponse.lyrics() == null) {
    throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Lyrics not found for this track in the external API");
}
dto.setLyrics(apiResponse.lyrics());
return dto;

|| has the same short-circuit property of && with the difference that it shorts on the first true evaluated rather than false.

[cometbroom]

The current integration tests for lyrics directly call the real external API. The pattern shown in the Integration Testing chapter is to wrap external API calls behind a Spring bean so it can be replaced with a mock during tests. This keeps the tests focused on your own code (controller → service → repository) while removing flakiness and external dependencies from the test results.

Recommended approach

1. Move the lyrics API call into its own LyricsService bean.
2. Inject LyricsService into TrackLyricsService (constructor injection).

   @Service
   public class TrackLyricsService {
   
       private final TrackRepository trackRepository;
       private final LyricsService lyricsService;
   
       public TrackLyricsService(TrackRepository trackRepository, LyricsService lyricsService) {
           this.trackRepository = trackRepository;
           this.lyricsService = lyricsService;
       }
       // ... rest of the class
   }

3. In your integration tests, add a mock of LyricsService to the context with @MockitoBean like below and stub it in tests:

   @SpringBootTest
   class TrackControllerIntegrationTest {
   
     private MockMvc mockMvc;
     @MockitoBean private LyricsService lyricsService;
   
     @Autowired
     void setMockMvc(WebApplicationContext wac) {
       this.mockMvc = MockMvcBuilders.webAppContextSetup(wac).build();
     }
   
     @Test
     void testGetLyrics_HappyPath() throws Exception {
       when(lyricsService.getLyrics("Billie Eilish", "LUNCH"))
           .thenReturn("API_LYRICS");
   
       mockMvc.perform(get("/tracks/41/lyrics")
               .contentType(MediaType.APPLICATION_JSON))
           .andExpect(status().isOk())
           .andExpect(jsonPath("$.trackId").value(41))
           .andExpect(jsonPath("$.trackTitle").value("LUNCH"))
           .andExpect(jsonPath("$.lyrics").value("API_LYRICS"));
     }

Important note on mocking annotations:
@MockBean has been deprecated since Spring Boot 3.4.0 and is removedl in 4.0.0. The replacement is @MockitoBean as you can see in the example.

[cometbroom]

Both for performance and ease of testing it is best to declare a RestClient bean that you would then inject where it's relevant. You have already declared a RestClient bean in WebClientConfig.java. To use it you only need to inject the bean:

public TrackLyricsService(TrackRepository trackRepository, @Qualifier("lyricsRestClient") RestClient restClient) {
  this.trackRepository = trackRepository;
  this.restClient = restClient;
}

If you're hitting application context startup issues, that can be because multiple beans with the RestClient type are found. In that case, you need to use a @Qualifier like above to help Spring narrow down the bean that has to be injected.

[cometbroom]

This is completely optional and just for future reference, feel free to skip it for now.

In case you'd like to write @Qualifier on an injectable field and still use Lombok's RequiredArgsConstructor to generate the constructor, you can add @Qualifier to the field like so:

@RequiredArgsConstructor
public class TrackLyricsService {

    private final TrackRepository trackRepository;
    @Qualifier("lyricsRestClient") private final RestClient restClient;

However, Lombok then needs to copy that annotation to the generated constructor and it doesn't do that by default. You can configure Lombok to do that by adding the line below to a lombok.config file in the root of your project:

# Copy annotation on the field to the constructor
lombok.copyableAnnotations += org.springframework.beans.factory.annotation.Qualifier

[cometbroom]

This is manually writing the getters/setters and the all args constructor while UserStatsDto is using lombok to generate them. Since Lombok already provides @Setter annotation, you could still use it to setup this class for consistency.

Listing options with Lombok here:

1. You could also manually write only the methods you need, so setLyrics in this case:

   @AllArgsConstructor
   @Getter
   public class TrackLyricsDto {
       private Long trackId;
       private String trackTitle;
       private String artistName;
       private String lyrics;
   
       public void setLyrics(String lyrics) {
           this.lyrics = lyrics;
       }
   }

2. Or if your setter method is just a standard one without custom logic, you could use the annotation only on the field to only generate a setter method for lyrics:

   @Getter
   @AllArgsConstructor
   public class TrackLyricsDto {
       private Long trackId;
       private String trackTitle;
       private String artistName;
       @Setter private String lyrics;
   }

3. You can also have @Setter annotation on the class and declare your own setLyrics with the same return and argument type, effectively overriding Lombok generation:

   @Getter
   @AllArgsConstructor
   @Setter
   public class TrackLyricsDto {
       private Long trackId;
       private String trackTitle;
       private String artistName;
       private String lyrics;
   
       public void setLyrics(String lyrics) {
           IO.println("Custom setter to override the generated one!");
           this.lyrics = lyrics;
       }
   }