Skip to content

Nilsishome/exercise8#222

Open
nilsishome wants to merge 22 commits intomainfrom
nilsishome/exercise8
Open

Nilsishome/exercise8#222
nilsishome wants to merge 22 commits intomainfrom
nilsishome/exercise8

Conversation

@nilsishome
Copy link
Copy Markdown

@nilsishome nilsishome commented Nov 27, 2025

REST API made in Spring Boot

Summary by CodeRabbit

  • New Features

    • Added REST API for browsing and managing skateboard decks and truck sizes.
    • Introduced JWT-based authentication with token issuance and request filtering.
  • Database

    • Added automated migrations for deck, truck brand, and truck size schemas.
    • Added development data initializer and application datasource configuration.
  • Infrastructure

    • Added Docker Compose configuration to run a MySQL service.
    • Migrated project to a Spring Boot baseline.
  • Tests

    • Added integration tests verifying API access control and endpoints with authentication.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 27, 2025

Walkthrough

Transforms the project into a Spring Boot web application: adds Spring Boot parent/POM, REST controllers, JWT authentication and filter, security config, JPA entities/repositories, Flyway migrations, MySQL docker-compose and properties, dev data initializer, and Testcontainers-backed integration tests.

Changes

Cohort / File(s) Summary
Build & Infrastructure
docker-compose.yml, pom.xml
Added docker-compose.yml for MySQL 9.5.0 and replaced POM with Spring Boot parent, spring-boot-maven-plugin, and many Spring/Flyway/JWT/Testcontainers/security dependencies and properties.
Application Entry Point
src/main/java/org/example/Exercise2025Application.java
New Spring Boot application class with @SpringBootApplication and main method.
REST Controllers
src/main/java/org/example/SkateboardController.java, src/main/java/org/example/UserController.java
Added SkateboardController exposing deck/truck endpoints and UserController endpoint to authenticate and return JWT.
Security & JWT
src/main/java/org/example/config/SecurityConfig.java, src/main/java/org/example/filters/JwtAuthFilter.java, src/main/java/org/example/services/JwtService.java
Added SecurityConfig (in-memory user, SecurityFilterChain), JwtAuthFilter (request JWT extraction/validation), and JwtService (token creation, parsing, validation, 30min expiry, HS256).
Entities & DTOs
src/main/java/org/example/entities/Deck.java, .../TruckBrand.java, .../TruckSize.java, .../DTOs/DeckDTO.java, .../DTOs/TruckDTO.java
New JPA entities (Deck, TruckBrand, TruckSize) with timestamps and proxy-aware equals/hashCode; record DTOs DeckDTO and TruckDTO.
DAOs / Repositories
src/main/java/org/example/entities/DAOs/AuthRequest.java, src/main/java/org/example/repositories/SkateboardRepository.java, .../TruckBrandRepository.java, .../TruckSizeRepository.java
Added AuthRequest record and Spring Data repositories (ListCrudRepository) with custom JPQL queries returning DTOs and entities.
Data Initialization
src/main/java/org/example/config/DevDataInitializer.java
Dev-profile ApplicationRunner that conditionally seeds decks, truck brands, and truck sizes (supports force-init to clear and reinsert).
Configuration & Migrations
src/main/resources/application.properties, src/main/resources/application-dev.properties, src/main/resources/db/migration/V1__Initial.sql, .../V2__Truck.sql
Added application properties (dev profile, datasource, flyway), dev JVM/JPA/logging props, and Flyway migrations creating deck, truck_brand, and truck_size tables with FK.
Tests & Test Data
src/test/resources/org/example/test-data.sql, src/test/java/org/example/SkateboardControllerTest.java
Added test SQL inserts and an integration test using Testcontainers + MockMvc to exercise JWT-protected endpoints (asserts 403 without token, 200 with valid JWT).
Removed / Replaced
src/main/java/org/example/App.java, src/test/java/org/example/AppIT.java, src/test/java/org/example/AppTest.java
Removed legacy standalone App and placeholder unit/integration tests.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant UserCtrl as UserController
    participant AuthMgr as AuthenticationManager
    participant JwtSvc as JwtService
    participant JwtFilter as JwtAuthFilter
    participant SKCtrl as SkateboardController
    participant Repo as UserRepo/DB

    Client->>UserCtrl: POST /auth/generateToken {username,password}
    UserCtrl->>AuthMgr: authenticate(UsernamePasswordAuthenticationToken)
    AuthMgr->>Repo: load user details
    alt auth succeeds
        AuthMgr-->>UserCtrl: Authentication
        UserCtrl->>JwtSvc: generateToken(username)
        JwtSvc-->>UserCtrl: JWT
        UserCtrl-->>Client: 200 {token}
    else auth fails
        AuthMgr-->>UserCtrl: throws
        UserCtrl-->>Client: 401/exception
    end

    Client->>SKCtrl: GET /api/decks (Authorization: Bearer token)
    SKCtrl->>JwtFilter: request passes through filter
    JwtFilter->>JwtSvc: extractUsername(token)
    JwtSvc-->>JwtFilter: username
    JwtFilter->>Repo: load UserDetails
    JwtFilter->>JwtSvc: validateToken(token, userDetails)
    alt token valid
        JwtFilter->>SKCtrl: continue with SecurityContext set
        SKCtrl->>Repo: findDeckBy()/findDeckById(...)
        Repo-->>SKCtrl: deck(s)/DTOs
        SKCtrl-->>Client: 200 OK with payload
    else invalid/expired
        JwtFilter-->>Client: 403 Forbidden
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

  • JwtService: hard-coded SECRET and key handling.
  • SecurityConfig: in-memory user, password encoder, and endpoint authorization rules.
  • SkateboardController: in-place entity mutation to attach DTOs and exception handling (404 behavior).
  • Repositories/JPQL: DTO constructor usage and uppercasing correctness.
  • Flyway migrations vs. entity column mappings and FK constraints.
  • DevDataInitializer: destructive force-init behavior and transactional semantics.
  • Testcontainers test: container lifecycle, dynamic property wiring, and test stability.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nilsishome/exercise8

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0955992 and 3e55270.

📒 Files selected for processing (1)
  • src/test/java/org/example/SkateboardControllerTest.java (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/test/java/org/example/SkateboardControllerTest.java

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🧹 Nitpick comments (12)
src/main/java/org/example/entities/DAOs/AuthRequest.java (2)

1-1: Consider renaming the package from DAOs to a more appropriate name.

The package is named "DAOs" (Data Access Objects), but AuthRequest is a request DTO, not a DAO. Consider renaming to org.example.entities.dtos or org.example.entities.requests for better semantic clarity.


3-3: Consider adding validation annotations to the record components.

For production-grade applications, adding Bean Validation annotations like @NotBlank would ensure proper validation of authentication credentials. However, this may be deferred in an exercise project.

Example:

import jakarta.validation.constraints.NotBlank;

public record AuthRequest(@NotBlank String username, @NotBlank String password) {
}
src/main/java/org/example/repositories/SkateboardRepository.java (2)

12-16: Replace List<Object> with a typed DTO projection.

Returning List<Object> (actually List<Object[]> at runtime) sacrifices type safety and requires manual casting. Additionally, findDeckBy() is a misleading name since it doesn't filter by any criteria.

Create a projection DTO and rename the method:

// Create a new DTO record in org.example.entities.DTOs
public record DeckBrandWidthDTO(String brand, Double boardWidth) {}

Then update the query:

     @Query("""
-            select upper(deck.brand), deck.boardWidth
+            select new org.example.entities.DTOs.DeckBrandWidthDTO(upper(deck.brand), deck.boardWidth)
             from Deck deck
             """)
-    List<Object> findDeckBy();
+    List<DeckBrandWidthDTO> findAllBrandAndWidth();

18-23: Rename method for clarity.

The method name findDeckBy without any suffix is ambiguous since it filters by brand. Consider renaming to findByBrand or findDecksByBrand to follow Spring Data naming conventions.

     @Query("""
             select deck
             from Deck deck
             where deck.brand = :brand
             """)
-    List<Deck> findDeckBy(@Param("brand") String brand);
+    List<Deck> findByBrand(@Param("brand") String brand);

Note: With this naming, Spring Data JPA can derive the query automatically, eliminating the need for the explicit @Query.

src/test/java/org/example/SkateboardControllerTest.java (1)

8-8: Remove unused import.

ServiceConnection is imported but not used. Either remove this import or use @ServiceConnection on the container to simplify the datasource configuration (which would eliminate the need for @DynamicPropertySource).

-import org.springframework.boot.testcontainers.service.connection.ServiceConnection;
src/main/java/org/example/UserController.java (2)

18-24: Consider making fields final for immutability.

Declaring jwtService and authManager as final better conveys intent and is idiomatic for constructor-injected dependencies.

-    private JwtService jwtService;
-    private AuthenticationManager authManager;
+    private final JwtService jwtService;
+    private final AuthenticationManager authManager;

32-36: The isAuthenticated() check is unreachable on failure.

AuthenticationManager.authenticate() throws an AuthenticationException (e.g., BadCredentialsException) when credentials are invalid—it never returns an unauthenticated Authentication object. The else branch is dead code. Additionally, UsernameNotFoundException is semantically incorrect for invalid credentials.

     @PostMapping("/generateToken")
     public String authenticateAndGetToken(@RequestBody AuthRequest authRequest) {
-        Authentication authentication = authManager.authenticate(
+        authManager.authenticate(
                 new UsernamePasswordAuthenticationToken(authRequest.username(),
                         authRequest.password())
         );
-        if (authentication.isAuthenticated()) {
-            return jwtService.generateToken(authRequest.username());
-        } else {
-            throw new UsernameNotFoundException("Invalid credentials!");
-        }
+        return jwtService.generateToken(authRequest.username());
     }
src/main/java/org/example/entities/TruckBrand.java (1)

17-17: Consider adding column constraints for brand.

Adding @Column constraints improves data integrity and documents expectations:

+    @Column(nullable = false, length = 100)
     private String brand;
src/main/java/org/example/entities/TruckSize.java (1)

21-21: Consider using a proper JPA relationship for brandId.

The brandId field is stored as a plain Integer instead of using @ManyToOne TruckBrand brand. This works, but you lose JPA-level referential integrity and the ability to navigate the relationship directly. The database FK (from the migration) still enforces integrity at the DB level.

For an exercise project this is acceptable, but in production code a proper relationship would be preferred.

src/main/java/org/example/SkateboardController.java (1)

29-34: Consider throwing a more descriptive exception for 404 responses.

Using orElseThrow() without arguments throws a generic NoSuchElementException, resulting in a 500 error. For REST APIs, consider ResponseStatusException or a custom exception to return a proper 404.

+import org.springframework.web.server.ResponseStatusException;
+import org.springframework.http.HttpStatus;
 ...
     @GetMapping("/decks/id/{id}")
     public Deck findDeckById(@PathVariable Integer id) {
-        Deck deck = repository.findById(id).orElseThrow();
+        Deck deck = repository.findById(id)
+                .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Deck not found: " + id));
         deck.setFittingTrucks(truckSizeRepository.findTruckByWidth(deck.getBoardWidth()));
         return deck;
     }
pom.xml (2)

16-28: Empty metadata elements should be filled in or removed.

These placeholder elements (<url/>, <license/>, <developer/>, <scm> with empty children) add noise. Consider removing them or filling in actual values.


82-86: Fix indentation for consistency.

The jjwt-api dependency block has inconsistent indentation compared to other dependencies.

         <dependency>
-        <groupId>io.jsonwebtoken</groupId>
-        <artifactId>jjwt-api</artifactId>
-        <version>${jjwt.version}</version>
+            <groupId>io.jsonwebtoken</groupId>
+            <artifactId>jjwt-api</artifactId>
+            <version>${jjwt.version}</version>
         </dependency>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8618163 and 192dee4.

📒 Files selected for processing (26)
  • docker-compose.yml (1 hunks)
  • pom.xml (1 hunks)
  • src/main/java/org/example/App.java (0 hunks)
  • src/main/java/org/example/Exercise2025Application.java (1 hunks)
  • src/main/java/org/example/SkateboardController.java (1 hunks)
  • src/main/java/org/example/UserController.java (1 hunks)
  • src/main/java/org/example/config/ApiSecurityConfig.java (1 hunks)
  • src/main/java/org/example/config/DevDataInitializer.java (1 hunks)
  • src/main/java/org/example/config/SecurityConfig.java (1 hunks)
  • src/main/java/org/example/entities/DAOs/AuthRequest.java (1 hunks)
  • src/main/java/org/example/entities/DTOs/TruckDTO.java (1 hunks)
  • src/main/java/org/example/entities/Deck.java (1 hunks)
  • src/main/java/org/example/entities/TruckBrand.java (1 hunks)
  • src/main/java/org/example/entities/TruckSize.java (1 hunks)
  • src/main/java/org/example/filters/JwtAuthFilter.java (1 hunks)
  • src/main/java/org/example/repositories/SkateboardRepository.java (1 hunks)
  • src/main/java/org/example/repositories/TruckBrandRepository.java (1 hunks)
  • src/main/java/org/example/repositories/TruckSizeRepository.java (1 hunks)
  • src/main/java/org/example/services/JwtService.java (1 hunks)
  • src/main/resources/application-dev.properties (1 hunks)
  • src/main/resources/application.properties (1 hunks)
  • src/main/resources/db/migration/V1__Initial.sql (1 hunks)
  • src/main/resources/db/migration/V2__Truck.sql (1 hunks)
  • src/test/java/org/example/AppIT.java (0 hunks)
  • src/test/java/org/example/AppTest.java (0 hunks)
  • src/test/java/org/example/SkateboardControllerTest.java (1 hunks)
💤 Files with no reviewable changes (3)
  • src/main/java/org/example/App.java
  • src/test/java/org/example/AppIT.java
  • src/test/java/org/example/AppTest.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jennymakki
Repo: fungover/exercise2025 PR: 204
File: src/main/java/org/example/config/SecurityConfig.java:19-24
Timestamp: 2025-11-16T13:56:11.263Z
Learning: The repository fungover/exercise2025 is an exercise/learning project where hard-coded credentials and other shortcuts for ease of testing are acceptable and intentional.
📚 Learning: 2025-11-16T13:56:11.263Z
Learnt from: jennymakki
Repo: fungover/exercise2025 PR: 204
File: src/main/java/org/example/config/SecurityConfig.java:19-24
Timestamp: 2025-11-16T13:56:11.263Z
Learning: The repository fungover/exercise2025 is an exercise/learning project where hard-coded credentials and other shortcuts for ease of testing are acceptable and intentional.

Applied to files:

  • src/main/resources/application.properties
  • pom.xml
  • src/main/java/org/example/Exercise2025Application.java
📚 Learning: 2025-11-21T16:16:54.793Z
Learnt from: LolloGro
Repo: fungover/exercise2025 PR: 211
File: src/main/resources/templates/listOfBooks.html:11-12
Timestamp: 2025-11-21T16:16:54.793Z
Learning: In the repository fungover/exercise2025, the Author DTO uses NotEmpty validation on firstName and lastName fields, and BookDto uses NotNull validation on the author field. This validation architecture ensures that books always have complete author information, making null-safety checks in Thymeleaf templates unnecessary.

Applied to files:

  • pom.xml
🧬 Code graph analysis (5)
src/main/java/org/example/services/JwtService.java (1)
src/main/java/org/example/filters/JwtAuthFilter.java (1)
  • Component (19-61)
src/main/java/org/example/filters/JwtAuthFilter.java (1)
src/main/java/org/example/services/JwtService.java (1)
  • Component (20-87)
src/main/java/org/example/entities/TruckSize.java (2)
src/main/java/org/example/entities/Deck.java (1)
  • Entity (12-102)
src/main/java/org/example/entities/TruckBrand.java (1)
  • Entity (10-67)
src/main/java/org/example/entities/Deck.java (2)
src/main/java/org/example/entities/TruckBrand.java (1)
  • Entity (10-67)
src/main/java/org/example/entities/TruckSize.java (1)
  • Entity (10-89)
src/main/java/org/example/entities/TruckBrand.java (2)
src/main/java/org/example/entities/Deck.java (1)
  • Entity (12-102)
src/main/java/org/example/entities/TruckSize.java (1)
  • Entity (10-89)
🔇 Additional comments (18)
src/main/resources/application-dev.properties (1)

1-4: LGTM! Well-configured development profile.

The development profile settings are appropriate:

  • Docker Compose lifecycle management allows manual service control
  • SQL logging aids in debugging database interactions
  • Security debug logging helps troubleshoot JWT authentication flows
src/main/resources/db/migration/V1__Initial.sql (1)

1-9: LGTM! Clean initial schema migration.

The deck table schema is well-defined with appropriate constraints and data types. The primary key, nullable brand, and required board_width align with typical skateboard domain modeling.

src/main/java/org/example/entities/DTOs/TruckDTO.java (1)

1-4: LGTM! Clean and appropriate DTO.

The TruckDTO record is well-structured for its purpose of carrying truck brand and size information from the repository layer to the controller.

src/main/java/org/example/repositories/TruckBrandRepository.java (1)

1-7: LGTM! Standard Spring Data repository.

The repository interface correctly extends ListCrudRepository and follows Spring Data conventions. No custom methods are needed for the current use case.

src/main/resources/application.properties (1)

1-7: Configuration verified—Java 25 supports virtual threads.

Java version 25 is configured in pom.xml, which exceeds the minimum requirement of Java 21 for virtual threads. The application configuration is correct and properly aligned.

docker-compose.yml (1)

3-3: MySQL 9.5.0 is a valid Docker image tag and is available on Docker Hub.

The specified image version exists and is properly available for use. No changes are required.

src/main/java/org/example/filters/JwtAuthFilter.java (1)

19-29: LGTM on the filter structure and dependency injection.

The constructor injection pattern with @Autowired is clean, and extending OncePerRequestFilter ensures the filter runs exactly once per request. The authentication flow correctly sets the SecurityContext when a valid token is present.

src/main/java/org/example/config/DevDataInitializer.java (1)

36-79: LGTM on the overall structure.

The dev data initializer is well-structured with proper profile scoping, transactional boundaries, and logging. The seeded data provides a good baseline for development testing.

src/main/java/org/example/entities/TruckBrand.java (1)

52-66: Proxy-aware equals/hashCode implementation is correct.

The implementation correctly handles Hibernate proxies and uses ID-based equality, consistent with the TruckSize and Deck entities.

src/main/java/org/example/entities/TruckSize.java (1)

74-88: LGTM!

The equals/hashCode implementation follows the recommended proxy-aware pattern for JPA entities, consistent with other entities in the codebase (TruckBrand, Deck).

src/main/java/org/example/entities/Deck.java (2)

12-37: LGTM!

The entity structure is well-designed with proper JPA annotations. The @Transient annotation for fittingTrucks correctly excludes it from persistence while allowing runtime enrichment in the controller.


87-101: LGTM!

The equals/hashCode implementation is proxy-aware and consistent with the other entities in this PR.

src/main/java/org/example/services/JwtService.java (3)

23-24: Hard-coded JWT secret is acceptable for this exercise project.

Based on learnings, this repository is an exercise/learning project where hard-coded credentials are intentional for ease of testing. In production, this should be externalized to configuration/secrets management.


71-77: LGTM!

The token parsing implementation is correct for jjwt 0.11.x. Exceptions like ExpiredJwtException or MalformedJwtException will propagate and be handled by Spring Security's filter chain.


43-49: Verified: Review comment is accurate and appropriately scoped.

The project uses jjwt 0.11.5 as confirmed in pom.xml. The code correctly uses the builder methods (setClaims(), setSubject(), setIssuedAt(), setExpiration(), and signWith(key, SignatureAlgorithm.HS256)) which are valid for this version. The web search confirms these methods are indeed deprecated in jjwt 0.12+, making this an appropriate optional refactoring suggestion for future upgrades. No action is required on the current version, but the guidance is sound.

pom.xml (3)

30-30: Java 25 may not be released yet.

Java 25 is scheduled for September 2025. If you're using early access builds, this is fine. Otherwise, consider using Java 21 (LTS) or Java 23 for stability.


33-111: LGTM!

Dependencies are well-structured for a Spring Boot application with JWT authentication, JPA persistence, Flyway migrations, and Testcontainers for integration testing.


5-10: Spring Boot 3.5.7 is valid and current.

Spring Boot 3.5.7 was released on October 23, 2025, and is available from Maven Central. The version includes 69 bug fixes, documentation improvements, and dependency upgrades, making it a suitable choice for your project.

Comment thread src/main/java/org/example/config/ApiSecurityConfig.java Outdated
Comment thread src/main/java/org/example/config/DevDataInitializer.java
Comment thread src/main/java/org/example/config/SecurityConfig.java
Comment thread src/main/java/org/example/Exercise2025Application.java Outdated
Comment thread src/main/java/org/example/filters/JwtAuthFilter.java
Comment thread src/main/java/org/example/repositories/TruckSizeRepository.java
Comment thread src/main/java/org/example/SkateboardController.java
Comment thread src/main/java/org/example/SkateboardController.java
Comment thread src/main/resources/db/migration/V2__Truck.sql
Comment thread src/test/java/org/example/SkateboardControllerTest.java
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
src/test/java/org/example/SkateboardControllerTest.java (1)

44-84: Consider adding response body assertions for more robust tests.

The tests currently only verify HTTP status codes. For more comprehensive coverage, consider asserting on response body content (e.g., using jsonPath() or content() matchers) to verify the expected data structure and values are returned.

Example enhancement:

mockMvc.perform(get("/api/decks")
                .header("Authorization", token))
        .andExpect(status().isOk())
        .andExpect(jsonPath("$").isArray())
        .andExpect(jsonPath("$[0].brand").exists());

Note: The test at line 63 for /api/decks/id/1 assumes a deck with ID 1 exists in test data - this was flagged in a previous review.

🧹 Nitpick comments (4)
src/main/java/org/example/entities/DTOs/DeckDTO.java (1)

1-4: Consider lowercase package name for consistency with Java conventions.

The package uses DTOs with uppercase, but Java conventions typically prefer lowercase package names (e.g., org.example.entities.dto or org.example.entities.dtos). This is a minor style consideration.

The record itself is well-defined and correctly used for JPQL constructor expression projections.

src/main/java/org/example/config/DevDataInitializer.java (1)

56-79: Verify that brand IDs are populated before creating TruckSize entities.

The code accesses ace.getId(), independent.getId(), and thunder.getId() after saveAll(). This works correctly because saveAll() on a JPA repository returns managed entities with populated IDs, and you're reusing the same object references.

However, to make this more explicit and safer:

-            truckBrandRepository.saveAll(List.of(
-                    ace,
-                    independent,
-                    thunder
-            ));
+            List<TruckBrand> savedBrands = truckBrandRepository.saveAll(List.of(
+                    ace,
+                    independent,
+                    thunder
+            ));
+            var savedAce = savedBrands.get(0);
+            var savedIndependent = savedBrands.get(1);
+            var savedThunder = savedBrands.get(2);

Then use the saved references for TruckSize creation.

src/main/java/org/example/repositories/SkateboardRepository.java (1)

11-25: Consider more descriptive method names to avoid confusion.

Both methods are named findDeckBy() with different signatures and return types (List<DeckDTO> vs List<Deck>). While valid, this can be confusing for callers. Consider more descriptive names:

-    List<DeckDTO> findDeckBy();
+    List<DeckDTO> findAllDecksAsDto();

-    List<Deck> findDeckBy(@Param("brand") String brand);
+    List<Deck> findByBrand(@Param("brand") String brand);

The second method could also use Spring Data's derived query naming (findByBrand) to eliminate the custom @Query if the simple equality check is sufficient.

src/main/java/org/example/SkateboardController.java (1)

51-54: Previous save/return concern has been addressed.

The endpoint now correctly returns the result of repository.save(newDeck), which includes generated ID and any other persisted values.

Consider adding @Valid annotation for request body validation if the Deck entity has validation constraints:

public Deck addDeck(@Valid @RequestBody Deck newDeck)

Also, the RESTful convention would prefer POST /decks over POST /decks/create.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 192dee4 and 755123c.

📒 Files selected for processing (10)
  • src/main/java/org/example/Exercise2025Application.java (1 hunks)
  • src/main/java/org/example/SkateboardController.java (1 hunks)
  • src/main/java/org/example/config/DevDataInitializer.java (1 hunks)
  • src/main/java/org/example/entities/DTOs/DeckDTO.java (1 hunks)
  • src/main/java/org/example/filters/JwtAuthFilter.java (1 hunks)
  • src/main/java/org/example/repositories/SkateboardRepository.java (1 hunks)
  • src/main/java/org/example/repositories/TruckSizeRepository.java (1 hunks)
  • src/main/resources/db/migration/V2__Truck.sql (1 hunks)
  • src/test/java/org/example/SkateboardControllerTest.java (1 hunks)
  • src/test/resources/org/example/test-data.sql (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/test/resources/org/example/test-data.sql
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/main/resources/db/migration/V2__Truck.sql
  • src/main/java/org/example/filters/JwtAuthFilter.java
  • src/main/java/org/example/repositories/TruckSizeRepository.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-16T13:56:11.263Z
Learnt from: jennymakki
Repo: fungover/exercise2025 PR: 204
File: src/main/java/org/example/config/SecurityConfig.java:19-24
Timestamp: 2025-11-16T13:56:11.263Z
Learning: The repository fungover/exercise2025 is an exercise/learning project where hard-coded credentials and other shortcuts for ease of testing are acceptable and intentional.

Applied to files:

  • src/main/java/org/example/Exercise2025Application.java
🔇 Additional comments (4)
src/main/java/org/example/Exercise2025Application.java (1)

9-11: LGTM! Previous issue resolved.

The main method signature is now correct with public static modifiers. The Spring Boot application entry point follows the standard pattern and will work properly with conventional Java application launchers.

src/test/java/org/example/SkateboardControllerTest.java (1)

19-23: Good test setup with Testcontainers and class-level SQL initialization.

The test configuration with @Testcontainers, @SpringBootTest, @AutoConfigureMockMvc, and @Sql at the class level is well-structured for integration testing with a real MySQL database.

src/main/java/org/example/config/DevDataInitializer.java (1)

44-48: Good fix for the duplicate data issue.

The previous review's concern about force-init creating duplicate data has been addressed. The deletion order (truckSize → truckBrand → skateboard) correctly respects foreign key constraints.

src/main/java/org/example/SkateboardController.java (1)

25-28: Previous DTO concern has been addressed.

The endpoint now correctly returns List<DeckDTO> instead of List<Object>, providing a clear API contract.

Comment thread src/main/java/org/example/SkateboardController.java
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
src/main/java/org/example/SkateboardController.java (3)

32-39: Proper error handling, but consider a response DTO for truck enrichment.

The 404 handling is now correctly implemented. However, mutating the retrieved entity in-place (line 37) couples the controller to entity internals. Consider creating a dedicated response DTO that combines deck information with fitting trucks, rather than setting a transient field on the entity.

Example refactor:

// Create DeckWithTrucksDTO
public record DeckWithTrucksDTO(DeckDTO deck, List<TruckDTO> fittingTrucks) {}

// In controller:
@GetMapping("/decks/id/{id}")
public DeckWithTrucksDTO findDeckById(@PathVariable Integer id) {
    Deck deck = repository.findById(id)
            .orElseThrow(() -> new ResponseStatusException(
                    HttpStatus.NOT_FOUND, "Deck not found with id: " + id));
    List<TruckDTO> trucks = truckSizeRepository.findTruckByWidth(deck.getBoardWidth());
    return new DeckWithTrucksDTO(deck.toDTO(), trucks);
}

41-53: Consider optimizing truck lookups to avoid N+1 queries.

The loop at lines 49-51 calls truckSizeRepository.findTruckByWidth() for each deck, resulting in N+1 database queries. For larger result sets, this could impact performance. Consider batch-fetching trucks for all unique board widths in a single query, then mapping them to the corresponding decks.

Example approach:

@GetMapping("/decks/brand/{brand}")
public List<Deck> findDeckBy(@PathVariable String brand) {
    List<Deck> decks = repository.findDeckBy(brand);
    if (decks.isEmpty()) {
        throw new ResponseStatusException(
                HttpStatus.NOT_FOUND, "No " + brand + " deck found."
        );
    }
    // Collect unique widths and fetch trucks in batch
    Set<Double> widths = decks.stream()
            .map(Deck::getBoardWidth)
            .collect(Collectors.toSet());
    Map<Double, List<TruckDTO>> trucksByWidth = widths.stream()
            .collect(Collectors.toMap(
                    w -> w,
                    w -> truckSizeRepository.findTruckByWidth(w)
            ));
    // Enrich decks
    decks.forEach(deck -> 
        deck.setFittingTrucks(trucksByWidth.get(deck.getBoardWidth()))
    );
    return decks;
}

60-63: Add input validation for better error handling.

The endpoint now correctly returns the saved entity. However, adding @Valid to the @RequestBody parameter (along with validation annotations on the Deck entity) would provide clearer error messages for invalid input and catch issues before reaching the database.

Example:

+import jakarta.validation.Valid;

 @PostMapping("/decks/create")
-public Deck addDeck(@RequestBody Deck newDeck) {
+public Deck addDeck(@Valid @RequestBody Deck newDeck) {
     return repository.save(newDeck);
 }

Then add validation annotations to the Deck entity:

// In Deck.java
@NotBlank
private String brand;

@Positive
private Double boardWidth;
// etc.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 755123c and 0955992.

📒 Files selected for processing (1)
  • src/main/java/org/example/SkateboardController.java (1 hunks)
🔇 Additional comments (3)
src/main/java/org/example/SkateboardController.java (3)

14-25: LGTM! Clean controller structure with proper dependency injection.

The controller uses constructor injection with final fields, which is a Spring Boot best practice. The base path /api is appropriate for REST endpoints.


27-30: LGTM! Clean delegation to repository.

The endpoint properly returns typed DTOs, addressing previous feedback.


55-58: LGTM! Clean endpoint for truck lookups.

Simple delegation with appropriate return type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants