Conversation
…re since its removed when table is altered.
…ady exist if not data is saved. Also checks that book with the samt title not already exists.
…on of handling of sql query
…and added withReuse to reuse database on all tests
WalkthroughAdds a Spring Boot 3 web application for a book store: entities, repositories, DTOs, services, REST and MVC controllers, custom security provider and filter chain, Flyway migrations, Thymeleaf + HTMX views, startup data seeding, Docker Compose MySQL, and Testcontainers-based integration tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Browser as LoginForm
participant Security as SecurityFilterChain
participant Provider as CustomAuthenticationProvider
participant UserSvc as CustomUserDetailService
participant UserRepo as UserRepository
participant Encoder as PasswordEncoder
Client->>Browser: POST /login (username,password)
Browser->>Security: Submit credentials
Security->>Provider: authenticate(token)
Provider->>UserSvc: loadUserByUsername(username)
UserSvc->>UserRepo: findByUserName(username)
UserRepo-->>UserSvc: CustomizedUser
UserSvc-->>Provider: UserDetails (username, encodedPwd, authorities)
Provider->>Encoder: matches(raw, encoded)
Encoder-->>Provider: true/false
alt password matches
Provider-->>Security: authenticated token
Security-->>Client: authentication success (session)
else mismatch
Provider-->>Client: authentication failure (401)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas to pay special attention:
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
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: 18
🧹 Nitpick comments (32)
src/main/resources/static/styles/index.css (1)
1-4: Consider a more descriptive class name than.test.The styling is fine, but a semantic name (e.g.,
.hero-textor.intro-message) will age better and be clearer when used inindex.html.src/main/resources/templates/index.html (1)
1-27: Index template wiring with layout fragments andwelcomemodel attribute looks correct.Header/footer fragments, CSS includes, and
${welcome}binding fromHomePageControllerall line up; the page should render as expected. If you keep this beyond experimentation, you might also wrap the main content in a<main>element and use a more semantic class thantestfor the paragraph.src/main/java/org/example/Genre.java (1)
1-5: LGTM with minor naming consideration.The enum is well-defined. Note that NOVEL and FICTION are conceptually overlapping (a novel is a type of fiction), which might cause classification ambiguity. For an exercise project, this is acceptable.
src/main/resources/db/migration/V4__store.sql (1)
1-6: Consider making store_name NOT NULL.The store_name column is nullable, which allows stores without names. While this might be intentional for flexibility, stores typically require identifying names for business logic.
If store names should be required, apply this diff:
CREATE TABLE store ( store_id INT AUTO_INCREMENT NOT NULL, - store_name VARCHAR(255) NULL, + store_name VARCHAR(255) NOT NULL, CONSTRAINT pk_store PRIMARY KEY (store_id) );src/main/resources/db/migration/V2__language.sql (1)
1-6: Consider making text_language NOT NULL.The text_language column is nullable, which allows language records without actual language values. This seems counterintuitive as a language record should always have a language identifier.
If language values should be required, apply this diff:
CREATE TABLE language ( id INT AUTO_INCREMENT NOT NULL, - text_language VARCHAR(255) NULL, + text_language VARCHAR(255) NOT NULL, CONSTRAINT pk_language PRIMARY KEY (id) );docker-compose.yml (1)
1-11: Consider adding a volume mount for data persistence (optional for dev/test).The MySQL service has no volume mounts, meaning all database data will be lost when the container stops. While this is acceptable for testing and development environments, consider documenting this behavior or adding an optional volume mount if persistence is desired.
Example addition:
mysql: image: 'mysql:9.5.0' environment: - 'MYSQL_DATABASE=books' - 'MYSQL_PASSWORD=secret' - 'MYSQL_ROOT_PASSWORD=verysecret' - 'MYSQL_USER=user' ports: - '3306:3306' + # Uncomment to persist data across container restarts + # volumes: + # - mysql-data:/var/lib/mysql +# volumes: +# mysql-data:src/test/java/org/example/controller/UserControllerTest.java (1)
20-31: Consider verifying user persistence in the test.The test currently only validates the HTTP status code (201 CREATED) but doesn't verify that the user was actually persisted to the database or that the response contains the expected user data. While this provides basic coverage, consider adding assertions to verify the user exists in the database.
Example enhancement:
@Autowired UserRepository userRepository; // Add this field @Test void newUserIsSuccessfullyCreated() { CustomizedUser newUser = new CustomizedUser("Test", "Testar"); HttpHeaders headers = new HttpHeaders(); headers.setContentType(MediaType.APPLICATION_JSON); HttpEntity<CustomizedUser> request = new HttpEntity<>(newUser, headers); ResponseEntity<String> response = loggedInUser().exchange("/api/users", HttpMethod.POST, request, String.class); assertThat(response.getStatusCode()).isEqualTo(HttpStatus.CREATED); // Verify persistence Optional<CustomizedUser> savedUser = userRepository.findByUsername("Test"); assertThat(savedUser).isPresent(); assertThat(savedUser.get().getUsername()).isEqualTo("Test"); }src/main/resources/templates/listOfBooks.html (1)
1-7: Consider simplifying template structure (optional).The fragment includes a full HTML structure (
<html>,<head>,<body>) but only the fragment itself is used. While harmless, you could simplify by keeping only the fragment content since it's loaded dynamically via HTMX.Example simplified version:
-<!DOCTYPE html> -<html lang="en"> -<head> - <meta charset="UTF-8"> - <title>Title</title> -</head> -<body> - <div th:fragment="listOfBooks"> +<div th:fragment="listOfBooks"> <h2>Books in store</h2> <ol> <li th:each="bookDto : ${books}" ><span th:text="${bookDto.title}"></span> by <span th:text="${bookDto.author.firstName}"></span> <span th:text="${bookDto.author.lastName}"></span></li> </ol> - </div> -</body> -</html> +</div>src/main/resources/db/migration/V3__book.sql (3)
7-8: Consider adding indexes on foreign key columns.Foreign key columns
language_idandauthor_idshould have indexes to improve join performance. While MySQL InnoDB automatically creates indexes for foreign key constraints, it's good practice to explicitly define them for clarity.Add these indexes after the table creation:
CREATE INDEX idx_book_language ON book(language_id); CREATE INDEX idx_book_author ON book(author_id);
6-6: Consider adding a check constraint for rating validity (optional).The
ratingcolumn is defined asINT NOT NULLbut has no validation on valid ranges. Consider adding a check constraint if ratings should be within a specific range (e.g., 1-5 or 0-10).Example for 1-5 rating scale:
rating INT NOT NULL, + CONSTRAINT chk_rating CHECK (rating BETWEEN 1 AND 5),
4-4: Consider making title NOT NULL (optional).The
titlecolumn is nullable, but books should typically always have titles. Consider whetherNULLvalues are intentional or if this should beNOT NULL.- title VARCHAR(255) NULL, + title VARCHAR(255) NOT NULL,src/main/java/org/example/repository/RoleRepository.java (1)
10-10: Consider returning Optional for consistency and null safety.Other repositories in this codebase (e.g.,
LanguageRepository.findByTextLanguage,AuthorRepository.findByFirstNameAndLastName) returnOptionalto handle cases where the entity might not be found. ReturningRoledirectly requires callers to perform null checks and creates potential NPE risks.Apply this diff to improve null safety:
- Role findByAuthority(Authority authority); + Optional<Role> findByAuthority(Authority authority);src/main/java/org/example/dto/BookDto.java (1)
15-15: Consider adding validation for the rating field.The rating field lacks validation constraints. Consider adding range validation to ensure ratings are within expected bounds (e.g., 0-5 or 1-10).
For example:
@Min(value = 0, message = "Rating must be at least 0") @Max(value = 5, message = "Rating must not exceed 5") int rating,src/main/java/org/example/repository/StoreRepository.java (1)
14-21: Remove hard-coded schema name for better portability.The query hard-codes the schema name
booksin table references (books.inventory,books.book,books.author). This reduces portability across environments where the schema name might differ.Apply this diff:
@Query(value = """ SELECT store_name FROM store - JOIN books.inventory i on store.store_id = i.store_id - JOIN books.book b on b.book_id = i.book_id - JOIN books.author a on a.id = b.author_id + JOIN inventory i on store.store_id = i.store_id + JOIN book b on b.book_id = i.book_id + JOIN author a on a.id = b.author_id WHERE a.first_name = :author GROUP BY store_name; """, nativeQuery = true)src/test/java/org/example/controller/StoreControllerTest.java (1)
28-31: Consider using a more flexible assertion.The test expects exactly 2 stores, which could become brittle if seed data changes. Consider asserting
stores.length > 0or documenting why exactly 2 stores are expected from the test setup.src/main/java/org/example/dto/UserDto.java (1)
5-9: Consider tightening validation constraints for username/passwordThe DTO works as-is, but
@NotEmptyalone is very permissive, especially for passwords. You might want to use@NotBlankand add a minimum length (and possibly other complexity rules) forpassword, and maybe a length limit foruserName. This remains optional given the exercise nature of the repo, but will make the API more realistic.Example:
-import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Size; -public record UserDto( - @NotEmpty(message = "Username must be inserted") - String userName, - @NotEmpty(message = "Password must be inserted") - String password) { +public record UserDto( + @NotBlank(message = "Username must be inserted") + @Size(max = 64, message = "Username must be at most 64 characters") + String userName, + @NotBlank(message = "Password must be inserted") + @Size(min = 8, message = "Password must be at least 8 characters") + String password) { }Based on learnings
src/main/java/org/example/service/BookService.java (1)
25-55: MakecreateBooktransactional and consider more specific error handlingFunctionally this works, but there are a couple of robustness improvements worth considering:
- Without
@Transactional, author and language may be created in separate transactions before the book; ifbookRepository.savefails, you can end up with orphaned rows.- Throwing a bare
RuntimeException("Book already exists")makes it harder to map to an HTTP status; a custom exception orResponseStatusExceptionwould integrate better with the REST layer.Potential refactor:
-import org.springframework.stereotype.Service; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; @Service public class BookService { @@ - //The controller sends a bookDto that is mapped to a book entity to be stored in repository - public Book createBook(BookDto bookDto){ + //The controller sends a bookDto that is mapped to a book entity to be stored in repository + @Transactional + public Book createBook(BookDto bookDto){ @@ - if(bookRepository.findBookByTitle(bookDto.title()).isPresent()){ - throw new RuntimeException("Book already exists"); - } + if (bookRepository.findBookByTitle(bookDto.title()).isPresent()) { + // Consider a custom exception type mapped to 409 CONFLICT at controller level + throw new IllegalStateException("Book already exists"); + }Please verify that adding
@Transactionalhere aligns with the rest of your transaction configuration and tests.src/main/java/org/example/entities/Book.java (1)
18-29: Align field access modifiers and normalizeratingnamingThe entity will work, but there are a couple of consistency issues:
title,genre, andRatingare package‑private whilelanguageandauthorare private. Making all fields private improves encapsulation and avoids accidental package-level access.- The
Ratingfield and its accessors (getRating/setRating) use a capitalized field name, which is unusual in Java and may be confusing, especially since the DB column is likelyrating.Possible cleanup:
- String title; + private String title; @@ - @Enumerated(EnumType.STRING) - Genre genre; + @Enumerated(EnumType.STRING) + private Genre genre; @@ - int Rating; + private int rating; @@ - public Book(String title, Genre genre, int Rating, Author author, Language language) { - this.title = title; - this.genre = genre; - this.Rating = Rating; + public Book(String title, Genre genre, int rating, Author author, Language language) { + this.title = title; + this.genre = genre; + this.rating = rating; @@ - public int getRating() { - return Rating; + public int getRating() { + return rating; @@ - public void setRating(int rating) { - Rating = rating; + public void setRating(int rating) { + this.rating = rating; @@ - ", Rating=" + Rating + + ", rating=" + rating +(You’d also need to update any references in DTOs/repositories/tests accordingly.)
Also applies to: 65-71
src/main/java/org/example/repository/BookRepository.java (1)
13-20: Simplify base repository inheritance (optional)The custom query methods look good, but extending both
ListCrudRepositoryandPagingAndSortingRepositoryis a bit redundant. In Spring Data JPA,JpaRepositoryalready combines CRUD operations with pagination/sorting (and, in newer versions, list-returning findAll).If you’re using Spring Data JPA, you could simplify to:
-import org.springframework.data.repository.ListCrudRepository; -import org.springframework.data.repository.PagingAndSortingRepository; +import org.springframework.data.jpa.repository.JpaRepository; @@ -@Repository -public interface BookRepository extends ListCrudRepository<Book,Integer>, PagingAndSortingRepository<Book,Integer> { +@Repository +public interface BookRepository extends JpaRepository<Book, Integer> {Please verify against the Spring Data JPA version in this project to ensure
JpaRepositoryprovides all the methods you currently rely on.src/main/java/org/example/controller/StoreController.java (1)
25-40: Align method names and DTO mapping with the “store” domain
getBooksByAuthorandgetBooksByNameare returningStoreDtoon/stores/...paths, whilegetStoresalso returnsStoreDtobut uses a slightly different mapping style. For clarity, consider renaming the methods togetStoresByAuthor/getStoresByName(or similar) and consistently using the same mapping style (constructor reference vs lambda) across all three endpoints.src/main/java/org/example/entities/Inventory.java (1)
69-83: Equals/hashCode can wrongly consider transient Inventory instances equalBecause
inventoryIdis eagerly initialized and never null, two newInventoryinstances (before persistence, withbookId/storeIdstill null insideInventoryId) will compare equal viaObjects.equals(getInventoryId(), inventory.getInventoryId()). This can break Sets/Maps or other logic that relies on equality for new entities. Consider guarding equality on non‑null key parts (e.g., require non‑null bookId and storeId insideInventoryIdbefore returning true) or adopting a pattern similar to other entities (e.g., basing equality on a surrogate key or only when IDs are assigned).src/main/java/org/example/entities/Language.java (1)
17-35: Tighten encapsulation fortextLanguage
textLanguageis package‑private while other entity fields tend to be private with getters/setters. Making itprivatewould align with the rest of the model and avoid accidental direct access from other classes in the package.src/test/java/org/example/controller/BookControllerTest.java (1)
75-94: Use a 4xx status instead of 500 for “book already exists”
shouldReturnErrorMessageIfBookWithTitleAlreadyExistscurrently expectsHttpStatus.INTERNAL_SERVER_ERROR, which semantically signals an unexpected server failure. For a known domain conflict like “Book already exists”, a 4xx status (typically409 CONFLICTor400 BAD_REQUEST) would be more appropriate; if you decide to adjust the controller accordingly, please also update this test to assert on the new status.src/main/java/org/example/entities/Author.java (1)
11-44: Author mapping is straightforward; consider making fields privateEntity structure is fine for JPA, but you may want to make
firstNameandlastNameprivate for consistency with other entities and encapsulation, keeping the existing getters/setters.src/main/java/org/example/entities/Role.java (1)
11-52: Role–user mapping looks consistent; consider pluralizing collection nameThe many-to-many mapping to
CustomizedUsermatches therolesside onCustomizedUser; functionally it’s fine, though renamingcustomizedUsertocustomizedUserswould better reflect that it’s a collection.src/main/java/org/example/exception/CustomException.java (2)
16-20: ResponseStatusException handler is fine; minor typo in variable nameBehavior is correct; if you care about readability, consider renaming
formatedMessagetoformattedMessage.
36-39: Consider logging RuntimeException server-side and returning a generic message to clientsThe current implementation exposes
e.getMessage()directly, which can leak internal details (database errors, framework stack-trace hints, etc.). While no logging is currently configured, best practice would be to:
- Log the full exception server-side for debugging
- Return a generic message like
"An error occurred"to the clientOther handlers in this class similarly expose exception details (e.g.,
handleNotFundappendse.getMessage()), so applying this pattern consistently across all handlers would be beneficial.src/main/java/org/example/entities/InventoryId.java (1)
28-30: RenamesetBookIdparameter for clarityThe parameter name
titleinsetBookId(Integer title)is misleading; it should match the field (e.g.,bookId) to avoid confusion when reading or refactoring.- public void setBookId(Integer title) { - this.bookId = title; + public void setBookId(Integer bookId) { + this.bookId = bookId; }src/main/java/org/example/entities/CustomizedUser.java (1)
19-33:getAuthoritieslogic is fine; consider a slightly tighter implementationThe current
getAuthoritiesimplementation is correct for Spring Security. If you want to trim it a bit and avoid the mutable intermediate list, you could use a stream and return an unmodifiable list:- public List<GrantedAuthority> getAuthorities() { - List<GrantedAuthority> authorities = new ArrayList<>(); - for (Role role : roles) { - authorities.add(new SimpleGrantedAuthority("ROLE_"+role.getAuthority())); - } - return authorities; - } + public List<GrantedAuthority> getAuthorities() { + return roles.stream() + .map(role -> new SimpleGrantedAuthority("ROLE_" + role.getAuthority())) + .toList(); + }Purely stylistic; existing code is acceptable.
src/main/java/org/example/controller/BookController.java (1)
64-67: Handle deletion of non‑existent books more explicitly
bookRepository.deleteById(id)will throw if the id doesn’t exist, which will surface as a 500 rather than a clear 404 or a safe 204.If you want a clean API behavior, consider checking existence first and returning an appropriate status:
@DeleteMapping("/books/delete/{id}") - public void removeBook(@PathVariable Integer id) { - bookRepository.deleteById(id); - } + @ResponseStatus(HttpStatus.NO_CONTENT) + public void removeBook(@PathVariable Integer id) { + if (!bookRepository.existsById(id)) { + throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Book not found"); + } + bookRepository.deleteById(id); + }src/main/java/org/example/config/DevDataInitialize.java (1)
80-99: Dev/test seeding looks good; consider reusing loaded rolesSeeding roles and users under
developer/testprofiles with hard‑coded credentials is fine for this exercise project (and you’re encoding the passwords, which is nice). Based on learnings.A small robustness/readability tweak: avoid querying
roleRepository.findByAuthority(...)twice right after you’ve just seeded the roles. For example:- if(roleRepository.count()==0){ - Role admin = new Role(Authority.ADMIN); - Role user = new Role(Authority.USER); - roleRepository.saveAll(List.of(admin, user)); - roleRepository.flush(); - } - - if(userRepository.count()==0){ - CustomizedUser admin = new CustomizedUser(); + Role adminRole = roleRepository.findByAuthority(Authority.ADMIN); + Role userRole = roleRepository.findByAuthority(Authority.USER); + + if(userRepository.count()==0){ + CustomizedUser admin = new CustomizedUser(); admin.setUserName("admin"); admin.setPassword(passwordEncoder.encode("admin")); - admin.getRoles().add(roleRepository.findByAuthority(Authority.ADMIN)); + admin.getRoles().add(adminRole); userRepository.save(admin); CustomizedUser visitor = new CustomizedUser(); visitor.setUserName("visitor"); visitor.setPassword(passwordEncoder.encode("visitor")); - visitor.getRoles().add(roleRepository.findByAuthority(Authority.USER)); + visitor.getRoles().add(userRole); userRepository.save(visitor);You can still keep the
roleRepository.count()==0guard, but caching the role lookups makes the flow a bit clearer.src/main/java/org/example/entities/Store.java (1)
15-35: Tighten visibility ofstoreName
storeNameis currently package‑private, whilestoreIdisprivate. It’s probably intended to be encapsulated the same way:- String storeName; + private String storeName;The rest of the class (getters/setters, equals/hashCode, toString) already assumes encapsulation, so this change should be safe.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (67)
books.http(1 hunks)docker-compose.yml(1 hunks)pom.xml(1 hunks)src/main/java/org/example/App.java(0 hunks)src/main/java/org/example/Authority.java(1 hunks)src/main/java/org/example/Exercise2025Application.java(1 hunks)src/main/java/org/example/Genre.java(1 hunks)src/main/java/org/example/config/DevDataInitialize.java(1 hunks)src/main/java/org/example/config/PasswordEncodeConfig.java(1 hunks)src/main/java/org/example/config/SecurityConfig.java(1 hunks)src/main/java/org/example/controller/BookController.java(1 hunks)src/main/java/org/example/controller/BookPageController.java(1 hunks)src/main/java/org/example/controller/HomePageController.java(1 hunks)src/main/java/org/example/controller/SecretPageController.java(1 hunks)src/main/java/org/example/controller/StoreController.java(1 hunks)src/main/java/org/example/controller/UserController.java(1 hunks)src/main/java/org/example/dto/Author.java(1 hunks)src/main/java/org/example/dto/BookDto.java(1 hunks)src/main/java/org/example/dto/Inventory.java(1 hunks)src/main/java/org/example/dto/Language.java(1 hunks)src/main/java/org/example/dto/StoreDto.java(1 hunks)src/main/java/org/example/dto/UserDto.java(1 hunks)src/main/java/org/example/entities/Author.java(1 hunks)src/main/java/org/example/entities/Book.java(1 hunks)src/main/java/org/example/entities/CustomizedUser.java(1 hunks)src/main/java/org/example/entities/Inventory.java(1 hunks)src/main/java/org/example/entities/InventoryId.java(1 hunks)src/main/java/org/example/entities/Language.java(1 hunks)src/main/java/org/example/entities/Role.java(1 hunks)src/main/java/org/example/entities/Store.java(1 hunks)src/main/java/org/example/exception/CustomException.java(1 hunks)src/main/java/org/example/repository/AuthorRepository.java(1 hunks)src/main/java/org/example/repository/BookRepository.java(1 hunks)src/main/java/org/example/repository/InventoryRepository.java(1 hunks)src/main/java/org/example/repository/LanguageRepository.java(1 hunks)src/main/java/org/example/repository/RoleRepository.java(1 hunks)src/main/java/org/example/repository/StoreRepository.java(1 hunks)src/main/java/org/example/repository/UserRepository.java(1 hunks)src/main/java/org/example/service/BookService.java(1 hunks)src/main/java/org/example/service/CustomAuthenticationProvider.java(1 hunks)src/main/java/org/example/service/CustomUserDetailService.java(1 hunks)src/main/java/org/example/sql/store.sql(1 hunks)src/main/resources/application-developer.properties(1 hunks)src/main/resources/application.properties(1 hunks)src/main/resources/db/migration/V1__author.sql(1 hunks)src/main/resources/db/migration/V2__language.sql(1 hunks)src/main/resources/db/migration/V3__book.sql(1 hunks)src/main/resources/db/migration/V4__store.sql(1 hunks)src/main/resources/db/migration/V5__inventory.sql(1 hunks)src/main/resources/db/migration/V6__.sql(1 hunks)src/main/resources/static/styles/index.css(1 hunks)src/main/resources/static/styles/layout.css(1 hunks)src/main/resources/templates/books.html(1 hunks)src/main/resources/templates/index.html(1 hunks)src/main/resources/templates/layout.html(1 hunks)src/main/resources/templates/listOfBooks.html(1 hunks)src/main/resources/templates/secret.html(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/BookControllerTest.java(1 hunks)src/test/java/org/example/controller/StoreControllerTest.java(1 hunks)src/test/java/org/example/controller/TestSetup.java(1 hunks)src/test/java/org/example/controller/UserControllerTest.java(1 hunks)src/test/java/org/example/repository/StoreRepositoryTest.java(1 hunks)src/test/resources/application-test.properties(1 hunks)store.http(1 hunks)
💤 Files with no reviewable changes (3)
- src/main/java/org/example/App.java
- src/test/java/org/example/AppTest.java
- src/test/java/org/example/AppIT.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-09-19T11:36:59.329Z
Learnt from: LolloGro
Repo: fungover/exercise2025 PR: 106
File: src/main/java/exercise4/repository/InMemoryProductRepository.java:0-0
Timestamp: 2025-09-19T11:36:59.329Z
Learning: In the InMemoryProductRepository class in src/main/java/exercise4/repository/InMemoryProductRepository.java, the updateProduct method implements a versioning system where multiple products with the same ID can exist, but identical product objects are rejected using contains() check. This allows maintaining product history while preventing duplicate identical entries.
Applied to files:
src/main/java/org/example/repository/InventoryRepository.javasrc/test/java/org/example/repository/StoreRepositoryTest.java
📚 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.javasrc/main/resources/application.propertiessrc/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 (13)
src/main/java/org/example/controller/SecretPageController.java (2)
src/main/java/org/example/controller/BookPageController.java (1)
Controller(9-30)src/main/java/org/example/controller/HomePageController.java (1)
Controller(7-16)
src/main/java/org/example/config/SecurityConfig.java (1)
src/main/java/org/example/config/PasswordEncodeConfig.java (1)
Configuration(8-16)
src/main/java/org/example/entities/Inventory.java (2)
src/main/java/org/example/entities/Book.java (1)
Entity(9-116)src/main/java/org/example/entities/Store.java (1)
Entity(8-58)
src/main/java/org/example/entities/Language.java (2)
src/main/java/org/example/entities/Author.java (1)
Entity(11-62)src/main/java/org/example/entities/Book.java (1)
Entity(9-116)
src/main/java/org/example/controller/HomePageController.java (2)
src/main/java/org/example/controller/BookPageController.java (1)
Controller(9-30)src/main/java/org/example/controller/SecretPageController.java (1)
Controller(8-18)
src/main/java/org/example/controller/BookPageController.java (2)
src/main/java/org/example/controller/HomePageController.java (1)
Controller(7-16)src/main/java/org/example/controller/SecretPageController.java (1)
Controller(8-18)
src/main/java/org/example/entities/Role.java (1)
src/main/java/org/example/entities/CustomizedUser.java (1)
Entity(10-90)
src/main/java/org/example/config/PasswordEncodeConfig.java (1)
src/main/java/org/example/config/SecurityConfig.java (1)
Configuration(12-39)
src/main/java/org/example/entities/Store.java (1)
src/main/java/org/example/entities/Inventory.java (1)
Entity(9-85)
src/main/java/org/example/entities/CustomizedUser.java (1)
src/main/java/org/example/entities/Role.java (1)
Entity(11-69)
src/test/java/org/example/controller/TestSetup.java (1)
src/test/java/org/example/repository/StoreRepositoryTest.java (1)
Testcontainers(19-90)
src/main/java/org/example/entities/Book.java (2)
src/main/java/org/example/entities/Author.java (1)
Entity(11-62)src/main/java/org/example/entities/Language.java (1)
Entity(11-53)
src/main/java/org/example/entities/Author.java (1)
src/main/java/org/example/entities/Book.java (1)
Entity(9-116)
🔇 Additional comments (38)
src/main/java/org/example/Exercise2025Application.java (1)
1-13: LGTM!This is a standard Spring Boot application entry point that correctly uses the
@SpringBootApplicationannotation and delegates toSpringApplication.run()in the main method. The implementation follows Spring Boot conventions and best practices.src/main/resources/static/styles/layout.css (1)
1-8: Base layout styling looks good.The centered, max-width body and
.headerstyling align well withlayout.htmland provide a simple, consistent frame for the pages.src/main/resources/application-developer.properties (1)
1-3: Dev-only SQL logging and compose settings are appropriate.Scoping
show-sqland formatted SQL to thedeveloperprofile plus usingstart_onlyfor Docker Compose is a reasonable developer experience choice for this exercise.src/main/resources/templates/layout.html (1)
1-23: Header/footer fragments and navigation look consistent with the rest of the app.The layout template cleanly defines reusable
header/footerfragments and the nav targets (/,/books,/secret) match the controllers, so composition viath:replaceshould work smoothly.src/main/java/org/example/controller/HomePageController.java (1)
1-16: Home page controller correctly backs theindexview.The root GET mapping, model attribute name (
welcome), and returned view name (index) are all aligned with the template, so this should render the welcome page without issues.books.http (1)
1-19: HTTP collection matches the book API and is useful for manual testing.The login flow and the three book endpoints mirror the controller design and give a nice quick way to exercise the API during development. Using
admin/adminhere is acceptable for this learning repository; just something to revisit if this ever moves beyond an exercise context. Based on learningsstore.http (1)
1-19: Store HTTP examples align with the store API endpoints.The sample requests for all stores, by author, and by name match the REST contract and provide a handy way to exercise the store API alongside login.
pom.xml (3)
32-118: Well-structured dependency configuration.The dependency set is appropriate for a Spring Boot book store application with JPA, security, and Thymeleaf views. Good practices observed:
- Testcontainers for integration testing
- Docker Compose support for reproducible database setup
- Proper scoping (runtime, test, provided)
30-30: Invalid Java version specified.Java 25 does not exist yet. The current latest Java version is 23 (released September 2024), with Java 21 being the latest LTS release. This will cause build failures.
Apply this diff to use a valid Java version:
- <java.version>25</java.version> + <java.version>21</java.version>Likely an incorrect or invalid review comment.
5-10: Spring Boot 3.5.7 is the latest Spring Boot 3.x release (released October 23, 2025). The pom.xml declaration is valid and uses a current stable version.src/main/java/org/example/Authority.java (1)
1-5: LGTM!Clean enum definition for authority levels. The two-level authority model (USER and ADMIN) is appropriate for the application's access control requirements.
src/test/resources/application-test.properties (1)
1-2: LGTM!Enabling SQL logging and formatting in the test profile is helpful for debugging and verifying database interactions during integration tests.
src/main/resources/application.properties (2)
3-4: LGTM! Development-friendly caching configuration.Disabling caching for web resources and Thymeleaf templates is appropriate for development environments, enabling immediate feedback during development.
2-2: Virtual threads enabled—requirements verified in build configuration.Virtual threads require Java 21+, and this project specifies Java 25 in
pom.xml, meeting the requirement. Spring Boot 3.5.7 (configured viaspring-boot-starter-parent) supports virtual threads and exceeds the 3.2+ requirement. The Java and Spring Boot versions are documented in the build configuration, satisfying the documentation requirement.src/test/java/org/example/controller/UserControllerTest.java (1)
16-18: LGTM! Clean authentication helper.The
loggedInUser()helper method provides a clean way to add Basic Auth credentials to test requests. The hard-coded credentials are acceptable for this exercise project.Based on learnings.
src/main/java/org/example/dto/StoreDto.java (1)
5-5: LGTM! Clean and idiomatic DTO record.The record uses modern Java features effectively with a clear validation message. The
@NotEmptyconstraint ensures data integrity at the API boundary.src/main/java/org/example/dto/Language.java (1)
5-5: LGTM! Consistent DTO pattern.The Language record follows the same clean pattern as other DTOs in the project with clear validation constraints.
src/test/java/org/example/Exercise2025ApplicationTest.java (1)
6-9: LGTM! Standard Spring Boot smoke test.This context load test is a common pattern in Spring Boot applications. It provides valuable coverage by verifying that the application context loads successfully with all beans wired correctly. The empty test body is intentional—the test passes if the context loads without errors.
docker-compose.yml (1)
3-3: No issues found. MySQL 9.5.0 is a valid, available image on Docker Hub.The verification confirms that
mysql:9.5.0exists and is actively available on Docker Hub, including variants like9.5.0-oracleand9.5.0-oraclelinux9. The image will pull without issues.src/main/java/org/example/repository/InventoryRepository.java (1)
7-8: LGTM!Standard JPA repository interface with appropriate generic types for the entity and composite key.
src/main/java/org/example/config/PasswordEncodeConfig.java (1)
8-16: LGTM!Standard Spring Security configuration using BCryptPasswordEncoder, which is appropriate for password hashing in production and learning scenarios.
src/main/resources/db/migration/V1__author.sql (1)
1-7: LGTM!The migration correctly creates the author table with appropriate data types and constraints. The nullable name fields provide flexibility for data entry.
src/main/java/org/example/repository/UserRepository.java (1)
9-12: LGTM!Standard Spring Data JPA repository with a properly named finder method that follows naming conventions. The use of
Optionalas the return type is a good practice for handling potentially missing users.src/main/java/org/example/repository/LanguageRepository.java (1)
9-12: LGTM!The repository interface is well-structured with proper Spring Data conventions and null-safe return type.
src/main/java/org/example/repository/AuthorRepository.java (1)
9-13: LGTM!Clean repository interface following Spring Data best practices with null-safe return type.
src/main/java/org/example/config/SecurityConfig.java (1)
22-38: LGTM!The security configuration is well-structured with appropriate authorization rules. The disabled CSRF and simplified authentication setup are acceptable for this learning project.
Based on learnings
src/test/java/org/example/controller/StoreControllerTest.java (1)
17-25: LGTM!The test setup and HTTP status validation are well-structured. The hard-coded credentials are acceptable for this exercise project.
Based on learnings
src/main/resources/templates/books.html (1)
6-6: No action required — unpkg.dev is valid.Both
unpkg.comandunpkg.devare valid endpoints for unpkg, and the htmx docs list unpkg as a CDN option. The code is correct as written.src/main/java/org/example/service/CustomUserDetailService.java (1)
23-35: UserDetails mapping looks solidThe
loadUserByUsernameimplementation is straightforward: it fails fast when the user is missing and correctly maps roles to"ROLE_"-prefixedSimpleGrantedAuthorityinstances collected into aSet. I don’t see any functional issues here.src/main/java/org/example/controller/BookPageController.java (1)
9-28: Controller wiring and view mapping look correctThe
/booksand/books/listmappings are simple and consistent with the described templates (page + fragment), and usingbookRepository.findAll()directly here is fine for this read‑only listing use case.src/test/java/org/example/controller/BookControllerTest.java (1)
23-53: Solid end‑to‑end coverage for read pathsThe tests for
/api/books,/api/books/id/{id}, and/api/books/genre/{genre}nicely cover status codes, payload shape, and specific field values, which should catch many regressions in the read‑only API behavior. Based on learnings.src/main/java/org/example/service/CustomAuthenticationProvider.java (2)
15-21: Constructor-based injection and dependencies look goodUsing final fields with constructor injection for
CustomUserDetailServiceandPasswordEncoderis clean and testable; no issues here.
37-40: supports() implementation is appropriateRestricting support to
UsernamePasswordAuthenticationTokenis standard and matches the expected authentication type.src/main/java/org/example/entities/Author.java (1)
46-60: equals/hashCode pattern is consistent with Hibernate best practicesUsing the proxy-aware, id-based equals/hashCode pattern matches the approach in
Bookand should behave correctly with Hibernate.src/main/java/org/example/controller/UserController.java (1)
18-26: Controller wiring is cleanConstructor injection of
UserRepository,RoleRepository, andPasswordEncoderis idiomatic and keeps the controller easy to test.src/main/java/org/example/entities/Role.java (1)
54-68: Equality implementation aligns with id-based entity identityProxy-aware equals/hashCode using
roleIdis consistent with the rest of the model and should behave correctly with Hibernate.src/main/java/org/example/exception/CustomException.java (1)
27-34: Validation handler produces clear messagesCollecting field errors into
"field: message"pairs joined by commas is reasonable and user-friendly for an exercise project.src/main/java/org/example/controller/BookController.java (1)
28-62: Disregard theRating()accessor concern—it is correct and will compileThe review comment incorrectly flagged
bookDto.Rating()as non-idiomatic or non-compiling. Verification confirms thatBookDtois a Java record withint Ratingas a component, and records auto-generate accessors matching the component name exactly. Therefore,bookDto.Rating()is the correct accessor and will compile successfully. The suggested diff that changes it tobookDto.rating()would actually cause a compilation error.The only valid point is the API response type consistency between endpoints returning
BookDtovs.Bookentities, which is a design consideration rather than a bug.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/main/java/org/example/controller/SecretPageController.java (1)
13-15: Typo fixed - LGTM!The typo identified in the previous review ("know" → "now") has been corrected. The controller follows standard Spring MVC patterns and is consistent with other controllers in the codebase.
src/main/java/org/example/service/BookService.java (1)
35-40: NewAuthorinstances misslastName, breaking lookups and causing duplicatesIn the
orElseGetbranch, you only setfirstNameon the newAuthor, leavinglastNamenull. SincefindByFirstNameAndLastNameuses both names, subsequent lookups for the same person will never match this row, and you will repeatedly insert duplicate authors.Suggested fix:
- Author author = authorRepository.findByFirstNameAndLastName(bookDto.author().getFirstName(), bookDto.author().getLastName()) - .orElseGet(() ->{ - Author newAuthor = new Author(); - newAuthor.setFirstName(bookDto.author().getFirstName()); - return authorRepository.save(newAuthor); - }); + Author author = authorRepository + .findByFirstNameAndLastName( + bookDto.author().getFirstName(), + bookDto.author().getLastName()) + .orElseGet(() -> { + Author newAuthor = new Author(); + newAuthor.setFirstName(bookDto.author().getFirstName()); + newAuthor.setLastName(bookDto.author().getLastName()); + return authorRepository.save(newAuthor); + });src/test/java/org/example/controller/BookControllerTest.java (1)
31-35: Tests depend on shared DB state and fixed IDs/counts; ensure isolation between testsSeveral tests assume a specific initial database state and stable IDs:
- Line 33–35:
shouldReturnAllBooksexpects exactly 5 books.- Line 39–42:
shouldFindBookByIdassumes book with id1and a specific title.- Line 53–55:
shouldReturnListOfBooksOfGivenGenreexpects exactly 4 FICTION books.- Line 123–124 & 130–131:
shouldUpdateBookandshouldDeleteBookoperate on book with id3.If the underlying DB is not reset between tests (and the Testcontainers MySQL instance is reused), create/update/delete tests will change the state and make these assertions order-dependent and flaky across runs or when adding new tests.
Please confirm that your
TestSetup(or this test class) now enforces isolation, e.g.:
- Marking the test class with
@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD), or- Using
@Transactionaltests with rollback per method, or- Manually cleaning relevant tables in
@BeforeEach/@AfterEach.If such a mechanism is already in place after the previous review, this comment can be ignored.
To cross-check in this repo, you can run:
#!/bin/bash rg -n "@DirtiesContext|@Transactional|@BeforeEach|@AfterEach" src/test/javaAlso applies to: 37-55, 114-132
🧹 Nitpick comments (3)
src/main/java/org/example/service/UserService.java (3)
15-17: Consider reordering fields to match constructor parameter order.The field declaration order (userRepository, roleRepository, passwordEncoder) differs from the constructor parameter order (userRepository, passwordEncoder, roleRepository). While not a functional issue, aligning these improves readability.
Apply this diff to align field order with constructor:
private final UserRepository userRepository; -private final RoleRepository roleRepository; private final PasswordEncoder passwordEncoder; +private final RoleRepository roleRepository;
29-31: Consider consistent exception types across similar services.This method throws
IllegalArgumentExceptionfor "already exists" scenarios, whileBookService.createBook()throwsRuntimeExceptionfor the same pattern. WhileIllegalArgumentExceptionis more semantically appropriate for invalid user input, consistency across similar service methods improves maintainability.You may want to update
BookService.createBook()to also useIllegalArgumentException:// In BookService.java line 18 if(bookRepository.findBookByTitle(bookDto.title()).isPresent()){ throw new IllegalArgumentException("Book already exists"); }
36-40: Consider usingIllegalStateExceptionfor system configuration errors.The missing USER role indicates a system configuration problem (e.g., missing Flyway migration or data seeding), not invalid user input.
IllegalStateExceptionwould better communicate that the system is not in an expected state, whileIllegalArgumentExceptiontypically signals invalid parameters.Apply this diff to use a more appropriate exception type:
if(userRole == null){ - throw new IllegalArgumentException("Role of user was not found"); + throw new IllegalStateException("USER role not found - system configuration error"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/org/example/controller/SecretPageController.java(1 hunks)src/main/java/org/example/controller/UserController.java(1 hunks)src/main/java/org/example/dto/Author.java(1 hunks)src/main/java/org/example/dto/BookDto.java(1 hunks)src/main/java/org/example/service/BookService.java(1 hunks)src/main/java/org/example/service/UserService.java(1 hunks)src/main/resources/templates/secret.html(1 hunks)src/test/java/org/example/controller/BookControllerTest.java(1 hunks)src/test/java/org/example/controller/TestSetup.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/templates/secret.html
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/org/example/dto/Author.java
- src/test/java/org/example/controller/TestSetup.java
- src/main/java/org/example/dto/BookDto.java
- src/main/java/org/example/controller/UserController.java
🧰 Additional context used
🧠 Learnings (1)
📓 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.
🧬 Code graph analysis (3)
src/main/java/org/example/service/BookService.java (1)
src/main/java/org/example/service/UserService.java (1)
Service(12-46)
src/main/java/org/example/service/UserService.java (1)
src/main/java/org/example/service/BookService.java (1)
Service(13-59)
src/main/java/org/example/controller/SecretPageController.java (2)
src/main/java/org/example/controller/HomePageController.java (1)
Controller(7-16)src/main/java/org/example/controller/BookPageController.java (1)
Controller(9-30)
🔇 Additional comments (3)
src/main/java/org/example/controller/SecretPageController.java (1)
13-16: Security configuration is properly configured.The Spring Security configuration in
src/main/java/org/example/config/SecurityConfig.javaat line 30 correctly restricts the/secret/**path to users with the ADMIN role via.requestMatchers("/secret/**").hasRole("ADMIN"). The controller message accurately reflects this protection, as the endpoint will only be reached by authenticated admin users.src/main/java/org/example/service/UserService.java (1)
41-41: No issue identified – roles collection is properly initialized.The
rolesfield inCustomizedUseris initialized at the field declaration level (private Set<Role> roles = new HashSet<>();), sogetRoles()will never return null. The call togetRoles().add(userRole)on line 41 is safe and will not throw aNullPointerException.Likely an incorrect or invalid review comment.
src/main/java/org/example/service/BookService.java (1)
50-52: Double-check theBookDtoaccessor name for ratingYou call
bookDto.Rating()with a capital “R”. IfBookDtois a record or a typical DTO with aratingfield, the accessor is usuallyrating()(lowercase) orgetRating(). As written, this may not compile or may be calling an unintended method.Please verify the
BookDtoAPI and, if needed, adjust this line accordingly.
…d handling of exception added transactional if user isn't created properly
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/org/example/exception/CustomException.java (1)
22-25: RenamehandleNotFundtohandleNotFoundfor clarityMinor typo: the method name
handleNotFundis misleading. Renaming it tohandleNotFoundwill better reflect its purpose and improve readability.
🧹 Nitpick comments (7)
src/main/java/org/example/entities/Book.java (2)
25-29: Consider makingManyToOneassociations explicitly LAZYRight now both
languageandauthoruse default@ManyToOnefetch (EAGER). For larger datasets this can easily cause N+1 issues when listing books. Consider:@ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.PERSIST)for both fields to align with how
Inventoryuses lazy associations and to keep loading under control.
33-39: Normalize constructor parameter namingThe constructor argument
int Ratingwith a capital “R” is easy to misread as a type. Renaming it toratingkeeps it consistent with the field and the rest of the code:public Book(String title, Genre genre, int rating, Author author, Language language) { this.title = title; this.genre = genre; this.rating = rating; this.author = author; this.language = language; }Purely cosmetic, but improves readability.
src/main/java/org/example/exception/DuplicateBooks.java (1)
3-7: Optional: align exception naming with common conventionsThe class name
DuplicateBooksworks but is a bit unclear (plural noun, noExceptionsuffix). Consider renaming to something likeDuplicateBookExceptionto make its purpose obvious at call sites and in logs.src/test/java/org/example/controller/BookControllerTest.java (1)
56-122: Tests postBookentities to endpoints that acceptBookDtoAll the create/update tests build
Bookentities and send them to controller methods that acceptBookDto. This currently works because the JSON shape matches, but it tightly couples tests to the internal entity structure.For clearer layering and less fragile tests, consider constructing and sending
BookDto(or raw JSON payloads) instead. That way changes in the entity (e.g., extra fields, annotations) don’t accidentally break the API tests.src/main/java/org/example/exception/CustomException.java (1)
41-44: Tighten handler signature toDuplicateBooksinstead ofExceptionSince this method is specifically annotated with
@ExceptionHandler(DuplicateBooks.class), usingDuplicateBooksas the parameter type is clearer thanExceptionand avoids any temptation to reuse it for unrelated exceptions:@ExceptionHandler(DuplicateBooks.class) public ResponseEntity<String> handleDuplicateBooks(DuplicateBooks e) { return ResponseEntity.status(HttpStatus.CONFLICT).body(e.getMessage()); }This keeps the mapping explicit alongside the generic
RuntimeExceptionhandler.src/main/java/org/example/controller/BookController.java (2)
28-62: Unify API contract: avoid mixing DTOs and entities in responsesRight now
/booksand/books/genre/{genre}returnList<BookDto>, while/books/id/{id}, POST/books/add, and PUT/books/update/{id}returnBookentities. Functionally this works, but it makes the REST API harder to reason about and evolve.Consider standardizing on a single representation (e.g., always
BookDtofor responses) and routing all mutations throughBookServiceinstead of usingBookRepositorydirectly in the controller. That keeps the boundary clean and reduces accidental exposure of JPA internals.
64-67: Consider handling deletes of non‑existent IDs as 404
deleteById(id)will throw if the book doesn’t exist, which will currently be turned into a 500 via the genericRuntimeExceptionhandler. For a REST API it’s usually more expressive to return 404 when deleting a non‑existent resource. You could pre-check withexistsById, catch the specific exception, or map it via a dedicated@ExceptionHandler.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/org/example/controller/BookController.java(1 hunks)src/main/java/org/example/dto/BookDto.java(1 hunks)src/main/java/org/example/entities/Book.java(1 hunks)src/main/java/org/example/exception/CustomException.java(1 hunks)src/main/java/org/example/exception/DuplicateBooks.java(1 hunks)src/main/java/org/example/service/BookService.java(1 hunks)src/main/java/org/example/service/UserService.java(1 hunks)src/test/java/org/example/controller/BookControllerTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/org/example/service/UserService.java
- src/main/java/org/example/dto/BookDto.java
🧰 Additional context used
🧠 Learnings (2)
📓 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-09-19T11:48:06.449Z
Learnt from: LolloGro
Repo: fungover/exercise2025 PR: 106
File: src/test/java/exercise4/service/ManagingProductTest.java:41-44
Timestamp: 2025-09-19T11:48:06.449Z
Learning: In the ManagingProductTest class in src/test/java/exercise4/service/ManagingProductTest.java, the throwsIllegalArgumentExceptionIfProductWithUniqIdAlreadyExist test is specifically designed to verify that two identical products cannot be created, not just products with the same ID but different properties.
Applied to files:
src/test/java/org/example/controller/BookControllerTest.java
🧬 Code graph analysis (2)
src/main/java/org/example/service/BookService.java (1)
src/main/java/org/example/exception/DuplicateBooks.java (1)
DuplicateBooks(3-7)
src/main/java/org/example/entities/Book.java (3)
src/main/java/org/example/entities/Author.java (1)
Entity(11-62)src/main/java/org/example/entities/Inventory.java (1)
Entity(9-85)src/main/java/org/example/entities/Language.java (1)
Entity(11-53)
🔇 Additional comments (1)
src/main/java/org/example/service/BookService.java (1)
28-59: createBook flow looks coherent and consistentThe transactional flow here is solid: you guard on title uniqueness, reuse or create
AuthorandLanguageviaorElseGet, then persist a properly populatedBook. Also good to see bothfirstNameandlastNameset when creating a newAuthor.
|
I think your implementation looks very structured and relatively straight forward to wrap ones head around. On that note, I'd recommend adding some instructions on how to setup your application to run locally. Maybe a single line command for Linux and WSL or Powershell-equivalent for straight forward hands on testing?
|
3 participants
Exercise 8 - Spring Boot application
• Spring Data JPA for MySQL access
• Docker Compose for reproducible database setup
• Spring Security with customized access control
• REST API for books and stores
• Controller with endpoints for API and website
• E2E test of API endpoints
• Integration test for native SQL query in store repository
• Thymeleaf templates for simple view of books in store and page for admin to add new user
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.