Conversation
- Added docker.compose file to start up mysql database (added start_only to profile "dev") - added basic dependencies for the application
…with GET /api/recipes - Also added http file to store requests to use in dev
- Added repo/interface for recipes
- GET api/recipes now works e2e (entity -> dto -> json) - Updated the recipe_item id column (it was Long, now changed to Interger)
- Added GET api/recipes/{id} to get recipe by id with all details
-Changed so GET api/recipes return all recipes without the items (faster and only a overview)
- Added mapper for better SoC
…ipe and RecipeItem - Implemented manual builder pattern for Recipe and RecipeItem entities - Added cascade + orphanRemoval to Recipe.items for consistent persistence - Added addItem/removeItem/clearItems helpers to maintain bidirectional relation - Formatet the files
…file - Activated only under "dev" Spring profile to avoid affecting production - Uses builder pattern to construct consistent entities
…endpoints (controller is now clean)
…ons into clean JSON error responses
- made DevDataLoader idempotent per recipe title and now inserts only missing sample recipes
- Added HTTP Basic authentication - Created in-memory users (ADMIN, USER) - Restricted POST/PUT/DELETE to ADMIN - Allowed GET for USER and ADMIN
…ata after every server restart
…n headers and added different scenarios
…ners with real MySQL and seeded DevDataLoader with profile "test"
- Implemented RecipeControllerTest using @WebMvcTest and @MockitoBean - Mocked RecipeService to isolate controller and security behavior - Added tests for 401, 403, and successful admin creation - Validates role-based access and JSON response structure
WalkthroughConverts a plain Java project into a Spring Boot REST service for recipe management: adds Spring Boot parent and dependencies, MySQL Docker Compose and Flyway migrations, secured CRUD API (role-based), JPA domain models, service/mapper layers, global error handling, and unit/integration tests using Testcontainers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (user/admin)
participant API as RecipeController
participant Sec as SecurityFilterChain
participant Service as RecipeService
participant Mapper as RecipeMapper
participant Repo as RecipeRepository
participant DB as MySQL
Client->>API: HTTP request /api/recipes...
API->>Sec: SecurityFilterChain checks auth & roles
alt GET allowed (USER or ADMIN)
Sec-->>API: permit
API->>Service: listAll() / getById(id)
Service->>Repo: query (findAll/findById with items)
Repo->>DB: SQL SELECT
DB-->>Repo: rows
Repo-->>Service: entities
Service->>Mapper: toList/toDetail
Mapper-->>Service: DTO(s)
Service-->>API: DTO(s)
API-->>Client: 200 OK + JSON
else Write blocked (USER)
Sec-->>API: deny
API-->>Client: 403 Forbidden
else Write allowed (ADMIN)
Sec-->>API: permit
API->>Service: create/update/delete(dto)
Service->>Mapper: fromUpsert / apply
Mapper-->>Service: entity
Service->>Repo: save/delete
Repo->>DB: INSERT/UPDATE/DELETE
DB-->>Repo: result
Repo-->>Service: entity/result
Service->>Mapper: toDetail
Service-->>API: ResponseEntity (201/204/200)
API-->>Client: appropriate status + Location/body
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/main/resources/application-dev.properties (1)
1-3: Keep verbose SQL and security DEBUG logging strictly in dev environmentsThese settings are useful while building the exercise but will be noisy and can expose sensitive details (e.g., queries, security internals) if ever reused beyond local dev. Make sure they stay confined to a dedicated dev profile and are not enabled in any shared or prod-like environment.
pom.xml (1)
77-90: Remove duplicatespring-security-testdependency declaration
org.springframework.security:spring-security-testis declared twice as a test dependency, which is redundant and can confuse future readers. Keeping a single entry is enough:- <dependency> - <groupId>org.springframework.security</groupId> - <artifactId>spring-security-test</artifactId> - <scope>test</scope> - </dependency> ... - <dependency> - <groupId>org.springframework.security</groupId> - <artifactId>spring-security-test</artifactId> - <scope>test</scope> - </dependency> + <dependency> + <groupId>org.springframework.security</groupId> + <artifactId>spring-security-test</artifactId> + <scope>test</scope> + </dependency>(Leave just one of the two blocks.)
src/main/resources/db/migration/V3__recipe_title_unique.sql (1)
1-2: Good to enforce unique titles; ensure duplicate insert behavior is well-definedAdding a UNIQUE constraint on
recipe.titleis a solid integrity check. Make sure your service/exception handling translates violations of this constraint into a clear client-facing response (e.g., 4xx with an explanatory error) and that you have a test covering the “create recipe with duplicate title” case so it doesn’t regress.src/main/resources/db/migration/V1__Intial.sql (1)
3-3: Consider using consistent ID types across related tables.There's a type inconsistency:
recipe.idisINT(line 3) whilerecipe_item.idisBIGINT(line 11). While functionally correct, using different integer types for primary keys in related tables can be confusing and may complicate joins or comparisons in some contexts.Consider standardizing to
INTfor both tables (sufficient for most use cases) orBIGINTfor both (if you anticipate very large datasets):CREATE TABLE recipe_item ( - id BIGINT AUTO_INCREMENT NOT NULL, + id INT AUTO_INCREMENT NOT NULL, name VARCHAR(255) NOT NULL, amount DOUBLE NOT NULL, unit VARCHAR(255) NOT NULL, recipe_id INT NOT NULL, CONSTRAINT pk_recipeitem PRIMARY KEY (id) );Also applies to: 11-11
src/main/java/org/example/config/SecurityConfig.java (1)
19-39: Consider explicitly setting stateless session management to match the commentYou disable CSRF “for stateless REST API” and use HTTP Basic, but Spring Security will still create an HTTP session by default. If you truly want stateless behavior, consider adding:
http .csrf(AbstractHttpConfigurer::disable) .sessionManagement(sm -> sm.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) .authorizeHttpRequests( /* ... */ ) .httpBasic(Customizer.withDefaults());This keeps the configuration aligned with the stateless intent without changing your role rules.
src/main/java/org/example/controller/RecipeController.java (1)
34-40: Optionally derive the Location URI from the current requestThe
Locationheader is currently built with a hard‑coded"/api/recipes/" + created.id(). That works, but you could make it more robust against future path changes by deriving it from the current request:URI location = ServletUriComponentsBuilder .fromCurrentRequest() .path("/{id}") .buildAndExpand(created.id()) .toUri(); return ResponseEntity.created(location).body(created);This keeps the Location in sync if you ever change the base mapping.
src/main/java/org/example/service/RecipeService.java (1)
31-36: Consider extracting a private helper for repeated “load or 404” logic
getById,update, anddeleteall repeat the samefindById(id).orElseThrow(...NOT_FOUND...)pattern. It’s fine as-is, but you could reduce duplication and ensure consistent behavior/messages by extracting a private helper:private Recipe loadRequired(Integer id) { return repository.findById(id) .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Recipe not found")); }Then call
loadRequired(id)fromgetById,update, anddelete.Also applies to: 45-53, 55-61
src/main/java/org/example/model/Recipe.java (1)
70-80: Consider adding null validation for consistency with Builder.The
replaceContentmethod directly assignstitleandinstructionswithout null checks, whereas theBuildervalidates these fields. For consistency and to prevent potential issues if null values are passed, consider adding validation.public void replaceContent(String title, String instructions, List<RecipeItem> newItems) { + if (title == null || instructions == null) { + throw new IllegalArgumentException("title and instructions cannot be null"); + } this.title = title; this.instructions = instructions; clearItems(); if (newItems != null) { for (RecipeItem item : newItems) { addItem(item); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
docker-compose.yml(1 hunks)pom.xml(1 hunks)recipes.http(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/config/DevDataLoader.java(1 hunks)src/main/java/org/example/config/SecurityConfig.java(1 hunks)src/main/java/org/example/controller/RecipeController.java(1 hunks)src/main/java/org/example/dto/RecipeItemResponse.java(1 hunks)src/main/java/org/example/dto/RecipeItemUpsertDto.java(1 hunks)src/main/java/org/example/dto/RecipeListResponse.java(1 hunks)src/main/java/org/example/dto/RecipeResponse.java(1 hunks)src/main/java/org/example/dto/RecipeUpsertDto.java(1 hunks)src/main/java/org/example/error/GlobalExceptionHandler.java(1 hunks)src/main/java/org/example/mapper/RecipeMapper.java(1 hunks)src/main/java/org/example/model/Recipe.java(1 hunks)src/main/java/org/example/model/RecipeItem.java(1 hunks)src/main/java/org/example/repository/RecipeRepository.java(1 hunks)src/main/java/org/example/service/RecipeService.java(1 hunks)src/main/resources/application-dev.properties(1 hunks)src/main/resources/application-test.properties(1 hunks)src/main/resources/application.properties(1 hunks)src/main/resources/db/migration/V1__Intial.sql(1 hunks)src/main/resources/db/migration/V2__recipe_item_id_to_int.sql(1 hunks)src/main/resources/db/migration/V3__recipe_title_unique.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/Exercise2025ApplicationTest.java(1 hunks)src/test/java/org/example/controller/RecipeControllerIntegrationTest.java(1 hunks)src/test/java/org/example/controller/RecipeControllerTest.java(1 hunks)
💤 Files with no reviewable changes (3)
- src/test/java/org/example/AppTest.java
- src/test/java/org/example/AppIT.java
- src/main/java/org/example/App.java
🧰 Additional context used
🧠 Learnings (4)
📓 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-21T16:16:54.741Z
Learnt from: LolloGro
Repo: fungover/exercise2025 PR: 211
File: src/main/resources/templates/listOfBooks.html:11-12
Timestamp: 2025-11-21T16:16:54.741Z
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:
src/main/java/org/example/dto/RecipeItemUpsertDto.javasrc/main/java/org/example/dto/RecipeUpsertDto.javapom.xml
📚 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.propertiessrc/main/java/org/example/Exercise2025Application.javasrc/test/java/org/example/Exercise2025ApplicationTest.javapom.xml
📚 Learning: 2025-09-10T14:02:54.365Z
Learnt from: alfredbrannare
Repo: fungover/exercise2025 PR: 66
File: src/test/java/org/example/service/WarehouseTest.java:1-21
Timestamp: 2025-09-10T14:02:54.365Z
Learning: In the fungover/exercise2025 project, static mocking of LocalDateTime.now() works successfully with the existing Mockito setup without requiring mockito-inline dependency.
Applied to files:
src/test/java/org/example/Exercise2025ApplicationTest.java
🧬 Code graph analysis (3)
src/main/java/org/example/service/RecipeService.java (1)
src/main/java/org/example/mapper/RecipeMapper.java (1)
RecipeMapper(9-58)
src/main/java/org/example/model/RecipeItem.java (1)
src/main/java/org/example/model/Recipe.java (2)
Entity(10-129)Builder(99-128)
src/main/java/org/example/model/Recipe.java (1)
src/main/java/org/example/model/RecipeItem.java (2)
Entity(8-110)Builder(80-109)
🔇 Additional comments (28)
src/main/java/org/example/dto/RecipeItemUpsertDto.java (1)
1-16: Validation annotations and record design look appropriateUsing
@NotBlankforname/unitand@Positiveforamounton a compact record is a clean fit with the validation-centric DTO approach used elsewhere in this repo.pom.xml (1)
29-32: Confirm Java 25 compatibility with your Spring Boot/Testcontainers toolchainYou’re targeting
<java.version>25</java.version>under Spring Boot parent3.5.6. That combination is newer than my knowledge cutoff, so please confirm that:
- Your local JDK and CI use Java 25, and
- Spring Boot 3.5.6 and Testcontainers
${testcontainers.version}fully support it.Adjust the
java.versionor dependency versions if you hit build/runtime issues.src/main/resources/db/migration/V2__recipe_item_id_to_int.sql (1)
1-6: Review comment is incorrect: no foreign keys depend on recipe_item.id, and MODIFY is not safer than DROP/ADD for this type change.The concern about breaking foreign keys is unfounded. The foreign key constraint FK_RECIPEITEM_ON_RECIPE points outward from recipe_item.recipe_id to recipe(id)—it does not point inward to recipe_item.id. Dropping and re-adding recipe_item.id does not affect this constraint or any incoming references.
For a BIGINT→INT type change, MODIFY also requires a table rebuild (ALGORITHM=COPY), so the suggested MODIFY approach is not demonstrably safer or more efficient than DROP/ADD. Both incur the cost of restructuring the table.
In an exercise context with a fresh database and no pre-existing data, the DROP/ADD approach is straightforward and correct. The migration is appropriate as written.
Likely an incorrect or invalid review comment.
src/main/java/org/example/dto/RecipeListResponse.java (1)
1-7: RecipeListResponse DTO is clean and idiomaticThe record definition is minimal, readable, and appropriate for a list-oriented projection of a recipe; no issues from a correctness or design standpoint here.
src/main/resources/application.properties (1)
1-2: Base application properties are reasonableSetting the application name and enabling virtual threads via configuration is a good, non-invasive default; nothing problematic stands out in these values.
src/main/java/org/example/dto/RecipeItemResponse.java (1)
1-9: RecipeItemResponse record is well-structuredThe field set and types are appropriate for representing a recipe item/ingredient, and using a record keeps the response model concise and immutable.
docker-compose.yml (1)
1-10: Verify MySQL env values match your Spring datasource configurationThe service definition itself looks fine, but
MYSQL_DATABASE=springandMYSQL_USER=usermust line up with whatever database name and username you configure in your Spring profiles (e.g.,spring.datasource.url,spring.datasource.username). If those differ, the app will fail to connect even though the container is healthy.src/main/resources/application-test.properties (1)
1-3: Test profile configuration aligns with migration-based testingEnabling Flyway and SQL logging while disabling Spring’s Docker Compose lifecycle management is consistent with running migrations against an externally managed test MySQL (e.g., Testcontainers); no issues here.
src/test/java/org/example/Exercise2025ApplicationTest.java (1)
1-13: Context smoke test is appropriateA minimal
@SpringBootTestwithcontextLoads()is a useful guardrail to catch misconfigurations as the app evolves; this looks good.src/main/java/org/example/dto/RecipeResponse.java (1)
1-10: RecipeResponse DTO cleanly models the full recipe viewThe record neatly captures the recipe plus its items, and using
List<RecipeItemResponse>keeps the aggregate response structure explicit and easy to map in the service/mapper layers.src/main/java/org/example/dto/RecipeUpsertDto.java (1)
8-18: LGTM! Well-structured DTO with appropriate validation.The validation constraints are correctly applied:
- Title and instructions use
@NotBlankto prevent empty values- Title has a sensible length constraint of 100 characters
- Items list uses
@Validfor cascading validation- Allowing null items enables creating recipes without ingredients initially, which the mapper handles gracefully
src/main/java/org/example/config/DevDataLoader.java (1)
11-45: LGTM! Appropriate dev data loader for the exercise project.The implementation correctly:
- Restricts execution to dev/test profiles only
- Clears existing data before seeding (acceptable in non-production environments)
- Uses builder patterns to construct sample recipes with items
- Leverages cascade/orphanRemoval for clean data reset
Based on learnings, hard-coded test data and shortcuts are acceptable in this exercise project.
src/main/java/org/example/repository/RecipeRepository.java (1)
10-14: LGTM! Good use of @EntityGraph to prevent N+1 queries.The
@EntityGraph(attributePaths = "items")annotation onfindByIdensures that recipe items are eagerly loaded in a single query, preventing the N+1 query problem. The@NonNullannotations provide additional null-safety guarantees.recipes.http (1)
1-80: LGTM! Comprehensive HTTP test scenarios.The test collection effectively covers:
- Authentication (401 for unauthenticated)
- Authorization (403 for insufficient privileges)
- CRUD operations with appropriate role-based access
- Conflict handling (409 for duplicate titles)
- All major success and error paths
The scenarios align well with the security configuration and provide a solid manual testing resource.
src/main/java/org/example/mapper/RecipeMapper.java (1)
9-57: LGTM! Clean mapper implementation with proper null handling.The mapper correctly:
- Uses a private constructor to prevent instantiation of the utility class
- Handles null
itemslists gracefully in bothfromUpsert(line 33) andapply(line 47)- Leverages builder patterns for entity construction
- Separates list and detail mapping concerns
- Encapsulates item-level mapping as a private helper
src/test/java/org/example/controller/RecipeControllerTest.java (1)
29-105: LGTM! Well-structured controller tests with comprehensive security coverage.The test suite effectively:
- Isolates the controller layer using
@WebMvcTestwith a mocked service- Imports
SecurityConfigto verify role-based access control- Tests authentication (401 for unauthenticated users)
- Tests authorization (403 when USER attempts admin operations)
- Verifies successful operations with proper roles
- Uses
@WithMockUserto simulate different user contexts- Validates both HTTP status codes and response payloads
src/main/java/org/example/Exercise2025Application.java (1)
9-12: Critical: main method must bepublic static.The
mainmethod is missing thepublic staticmodifiers. Java requires the entry point to bepublic static void main(String[] args). The current declarationvoid main(String[] args)will not work, and the application will fail to start.Apply this diff to fix the entry point:
- void main(String[] args) { - + public static void main(String[] args) { SpringApplication.run(Exercise2025Application.class, args); }⛔ Skipped due to 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.src/test/java/org/example/controller/RecipeControllerIntegrationTest.java (1)
33-47: Good use of a shared MySQL Testcontainer and DynamicPropertySourceStatic
MySQLContainerplus@DynamicPropertySourceis a clean way to wire the container into Spring Boot’s datasource for all tests in this class. Configuration (URL, username, password, driver) looks consistent and should work well with Flyway and your dev data seeding.src/main/java/org/example/config/SecurityConfig.java (1)
41-59: In‑memory users are appropriate for this exercise setupUsing an
InMemoryUserDetailsManagerwith bcrypt‑encodedadmin/usercredentials is a good fit for this training project and lines up with the tests and the basic‑auth requirements. No changes needed here for the current context. Based on learnings, this use of hard‑coded credentials is intentional and acceptable.src/main/java/org/example/model/RecipeItem.java (1)
8-27: Entity mapping and builder design look solidThe JPA mapping (
@Table,@ManyToOnewith non‑nullrecipe_id) plus the builder that enforcesname,amount, andunitare consistent with howRecipeandRecipeMapperuseRecipeItem. The Hibernate‑proxy‑safeequals/hashCodeimplementation is also appropriate for entity identity. No changes needed here.Also applies to: 80-108
src/main/java/org/example/model/Recipe.java (8)
1-8: LGTM!The imports are appropriate for a JPA entity with Hibernate proxy-aware equality semantics.
10-12: LGTM!Standard JPA entity annotations with explicit table naming.
14-26: LGTM!The field declarations follow JPA best practices. The
@Lobannotation for instructions is appropriate for potentially large text content, and the cascade settings correctly model a composition relationship where items are owned by the recipe.
28-29: LGTM!The protected no-arg constructor satisfies JPA requirements.
31-34: LGTM!Standard builder factory method.
54-68: LGTM!The helper methods correctly maintain bidirectional relationship consistency. The defensive copy in
clearItems()properly avoidsConcurrentModificationException.
82-97: LGTM!The
equalsandhashCodeimplementations correctly handle Hibernate proxies and use id-based equality, which is the recommended pattern for JPA entities to ensure correct behavior with lazy-loading and persistence contexts.
99-128: LGTM!The Builder implementation is well-designed with proper validation and uses the
addItem()method to ensure bidirectional relationship consistency. The fluent API design is clean and intuitive.
| CREATE TABLE recipe | ||
| ( | ||
| id INT AUTO_INCREMENT NOT NULL, | ||
| title VARCHAR(255) NOT NULL, | ||
| instructions LONGTEXT NOT NULL, | ||
| CONSTRAINT pk_recipe PRIMARY KEY (id) | ||
| ); | ||
|
|
||
| CREATE TABLE recipe_item | ||
| ( | ||
| id BIGINT AUTO_INCREMENT NOT NULL, | ||
| name VARCHAR(255) NOT NULL, | ||
| amount DOUBLE NOT NULL, | ||
| unit VARCHAR(255) NOT NULL, | ||
| recipe_id INT NOT NULL, | ||
| CONSTRAINT pk_recipeitem PRIMARY KEY (id) | ||
| ); | ||
|
|
||
| ALTER TABLE recipe_item | ||
| ADD CONSTRAINT FK_RECIPEITEM_ON_RECIPE FOREIGN KEY (recipe_id) REFERENCES recipe (id); No newline at end of file |
There was a problem hiding this comment.
Fix the filename typo before merging.
The migration filename contains a typo: V1__Intial.sql should be V1__Initial.sql. Once this migration is applied to any environment, the filename becomes part of Flyway's version history and cannot be easily renamed.
Apply this fix:
- Rename file from
V1__Intial.sqltoV1__Initial.sql
🤖 Prompt for AI Agents
In src/main/resources/db/migration/V1__Intial.sql around lines 1 to 20, the
migration filename contains a typo (V1__Intial.sql) which will be recorded by
Flyway; rename the file to V1__Initial.sql before merging to preserve correct
version history and avoid applying a migration with a misspelled name in any
environment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/main/java/org/example/error/GlobalExceptionHandler.java (1)
64-71: Log unexpected exceptions for observability.The catch-all handler returns a generic message to the client (appropriate for security), but doesn't log the exception details. This makes debugging production issues difficult, as operators have no visibility into what caused the 500 error.
Add logging to capture exception details:
+import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + @ControllerAdvice public class GlobalExceptionHandler { + private static final Logger log = LoggerFactory.getLogger(GlobalExceptionHandler.class); ... @ExceptionHandler(Exception.class) public ResponseEntity<Map<String, Object>> handleGeneric(Exception ex) { + log.error("Unexpected exception caught by global handler", ex); Map<String, Object> body = new HashMap<>(); body.put("timestamp", Instant.now()); body.put("status", HttpStatus.INTERNAL_SERVER_ERROR.value()); body.put("error", "Unexpected error"); return ResponseEntity.internalServerError().body(body); } }src/main/java/org/example/model/Recipe.java (3)
18-27: Consider making the OneToMany fetch type explicit.
@OneToManydefaults can vary across providers/versions; given you appear to control loading via entity graphs, you may want to declarefetch = FetchType.LAZYexplicitly to avoid surprises.- @OneToMany(mappedBy = "recipe", cascade = CascadeType.ALL, orphanRemoval = true) + @OneToMany( + mappedBy = "recipe", + cascade = CascadeType.ALL, + orphanRemoval = true, + fetch = FetchType.LAZY + ) private List<RecipeItem> items = new ArrayList<>();
53-68: Edge case when “re‑parenting” aRecipeItembetween recipes.If the same
RecipeIteminstance is ever moved from oneRecipeto another,addItemwill callitem.setRecipe(this)but won’t clean up the previous recipe’sitemslist, which can leave the other side inconsistent (depending onRecipeItem.setRecipeimplementation insrc/main/java/org/example/model/RecipeItem.java). A defensive pattern is to detach from the old parent before attaching to the new one.For example:
public void addItem(RecipeItem item) { if (item == null) return; - item.setRecipe(this); - this.items.add(item); + // If the item already belongs to another recipe, detach it first + if (item.getRecipe() != null && item.getRecipe() != this) { + item.getRecipe().removeItem(item); + } + item.setRecipe(this); + this.items.add(item); }
70-80: AlignreplaceContentwith non‑null DB constraints.
titleandinstructionsare@Column(nullable = false), butreplaceContentallows nulls and will only fail at flush time. Consider enforcing the invariant at the entity boundary (similar to the builder) for earlier, clearer failures.public void replaceContent(String title, String instructions, List<RecipeItem> newItems) { - this.title = title; - this.instructions = instructions; + if (title == null || instructions == null) { + throw new IllegalArgumentException("title and instructions are required"); + } + this.title = title; + this.instructions = instructions;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/org/example/error/GlobalExceptionHandler.java(1 hunks)src/main/java/org/example/model/Recipe.java(1 hunks)src/test/java/org/example/controller/RecipeControllerIntegrationTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/org/example/controller/RecipeControllerIntegrationTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/org/example/model/Recipe.java (1)
src/main/java/org/example/model/RecipeItem.java (1)
Builder(80-109)
🔇 Additional comments (5)
src/main/java/org/example/error/GlobalExceptionHandler.java (2)
19-32: LGTM! Validation error handling is well-structured.The handler correctly maps field-level validation errors into a structured response with timestamp, status, and field-specific messages.
43-61: LGTM! The null-safety guard has been properly implemented.The handler now safely checks for null messages at lines 49-50 before calling
.toLowerCase(), addressing the concern from the previous review. The heuristic logic for identifying constraint violation types is appropriate.src/main/java/org/example/model/Recipe.java (3)
49-51: Good: expose items as an unmodifiable view.Returning
List.copyOf(items)protects the bidirectional association from external mutation and forces callers to go throughaddItem/removeItem, which is exactly what you want here.
82-97: Equals/hashCode implementation looks appropriate for JPA/Hibernate.The proxy‑aware
equals/hashCodeusing the identifier is a solid pattern for entities and should behave well with Hibernate proxies and collections.
99-127: Builder correctly preserves invariants and relationship management.The builder enforces required
title/instructionsand delegates item wiring throughrecipe.addItem(item), ensuringRecipeItem.setRecipeis invoked. This is a clean, safe way to construct aggregate roots.
| @ExceptionHandler(ResponseStatusException.class) | ||
| public ResponseEntity<Map<String, Object>> handleResponseStatus(ResponseStatusException ex) { | ||
| Map<String, Object> body = new HashMap<>(); | ||
| body.put("timestamp", Instant.now()); | ||
| body.put("status", ex.getStatusCode().value()); | ||
| body.put("error", ex.getReason()); | ||
| return ResponseEntity.status(ex.getStatusCode()).body(body); | ||
| } |
There was a problem hiding this comment.
Guard against null reason in ResponseStatusException.
ex.getReason() can return null if the exception was constructed without a reason string. This would result in "error": null in the response, which is inconsistent with the other handlers that provide fallback messages.
Apply this diff to provide a fallback message:
@ExceptionHandler(ResponseStatusException.class)
public ResponseEntity<Map<String, Object>> handleResponseStatus(ResponseStatusException ex) {
Map<String, Object> body = new HashMap<>();
body.put("timestamp", Instant.now());
body.put("status", ex.getStatusCode().value());
- body.put("error", ex.getReason());
+ body.put("error", ex.getReason() != null ? ex.getReason() : "Request failed");
return ResponseEntity.status(ex.getStatusCode()).body(body);
}🤖 Prompt for AI Agents
In src/main/java/org/example/error/GlobalExceptionHandler.java around lines 34
to 41, the handler uses ex.getReason() which may be null; change the code to
guard against null by using a fallback (for example: use ex.getReason() != null
? ex.getReason() : ex.getStatusCode().getReasonPhrase() or a constant like
"Unknown error") when putting the "error" value into the body so the response
never contains a null error field.
viktornoskire
left a comment
viktornoskire
left a comment
There was a problem hiding this comment.
Really nice work here! The project structure is super clean—DTOs, mappers, service, repo, controller all make sense. I like the DevDataLoader for seeding test data, and your security setup for USER vs ADMIN is straightforward.
Exception handling is solid, and the tests do a good job covering authentication, authorization, and CRUD.
3 participants
This project implements a RESTful Recipe Management API using Spring Boot. It supports full CRUD operations, database persistence via JPA with MySQL, and Flyway database migrations. The application includes HTTP Basic authentication with role-based access control:
ROLE_USER: read-only access to recipes (GET)
ROLE_ADMIN: full access (GET, POST, PUT, DELETE)
Input validation is handled using DTOs with @Valid, while structured error responses are implemented via a central GlobalExceptionHandler.
Testing consists of:
Integration tests using Testcontainers (MySQL), covering real database interactions and security.
Web-layer tests using @WebMvcTest, with mocked service to isolate controller and verify authorization and HTTP behavior.
Summary by CodeRabbit
New Features
Infrastructure
Tests
Dev
✏️ Tip: You can customize this high-level summary in your review settings.