Skip to content

Klumpish/exercise8#207

Open
Klumpish wants to merge 13 commits intomainfrom
Klumpish/exercise8
Open

Klumpish/exercise8#207
Klumpish wants to merge 13 commits intomainfrom
Klumpish/exercise8

Conversation

@Klumpish
Copy link
Copy Markdown

@Klumpish Klumpish commented Nov 16, 2025

issue #193

Description:

This PR implements a simple zoo management web application using

  • Spring Boot
  • Spring data JPA
  • MySQL (via docker compose)
  • Spring Security

REST API for Animal entity (GET,POST,PUT,DELETE)

Role-based security:

GET /api/animals is public

POST/PUT/DELETE restricted to ADMIN

Thymeleaf pages for:

  • Listing all animals
  • Adding an animal (admin only)
  • Login/logout with role-based view

Testing for REST endpoints and security access.

How to test:

  • Run docker-compose to start MySQL
  • Start the Spring boot application
  • Open /login and test logging in as:
    • Admin: admin/adminpass -> "Add Animal" button visible
    • User: user/userpass -> "Add Animal" button hidden
  • Go to /animals and verify:
    • Animals from database are displayed
    • Adding a new animal via the form works and persists to db
  • REST API tests:
    • GET /api/animals works without authentication
    • POST/PUT/DELETE endpoints require admin credentials

Quick start

1. Setup MySQL with Docker compose

  1. create a .env file in root and fill it out ex
MYSQL_DATABASE=exercise8_zoo
MYSQL_ROOT_PASSWORD=verysecret
MYSQL_USER=user
MYSQL_PASSWORD=secret
  1. run docker-compose.yml
  2. Fill in application.properties to match your .env
  • you can find an example in application.properties.example
  1. Start the application
  2. the app will start at http://localhost:8080

Summary by CodeRabbit

  • New Features

    • REST API for managing animals with full CRUD operations
    • Web interface for viewing and adding animals
    • User authentication with role-based access control
    • Admin-restricted features for adding new animals
    • Database persistence with MySQL integration
  • Chores

    • Migrated project to Spring Boot framework
    • Added database configuration and initialization

@Klumpish Klumpish self-assigned this Nov 16, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 16, 2025

Walkthrough

This pull request converts a basic Maven template project into a Spring Boot web application for zoo animal management. It replaces the build system with Spring Boot, restructures to use Spring Data JPA, Spring Security, REST controllers, and Thymeleaf templates. A MySQL database is introduced via docker-compose. The legacy entry point is replaced with a Spring Boot application, and security configurations enable both API (HTTP Basic) and web form authentication.

Changes

Cohort / File(s) Summary
Build & Infrastructure Configuration
pom.xml, docker-compose.yml, .gitignore
Replaces Maven parent with Spring Boot starter parent (3.5.6); changes artifact ID to exercise2025; adds Spring Web, Security, Data JPA, Thymeleaf, Validation, Flyway, DevTools, and MySQL connector; removes explicit plugin configs in favor of spring-boot-maven-plugin. Introduces MySQL 9.5.0 service with database initialization. Adds .env and application.properties to gitignore.
Spring Boot Application Entry Point
src/main/java/org/example/Exercise2025Application.java
Adds new @SpringBootApplication class with main method; replaces deleted App.java.
Data Access Layer
src/main/java/org/example/entity/Animal.java, src/main/java/org/example/repository/AnimalRepository.java, src/main/java/org/example/data/DataInitializer.java
Introduces JPA entity Animal with id, name, species, age, enclosure, endangered fields. Adds repository with findBySpeciesContainingIgnoreCase. Implements database seeding via CommandLineRunner creating six animals on startup.
Service Layer
src/main/java/org/example/service/AnimalService.java
Provides thin service wrapper over AnimalRepository with findAll, save, findById, delete operations.
REST & MVC Controllers
src/main/java/org/example/controller/AnimalController.java, src/main/java/org/example/controller/ViewController.java
AnimalController exposes REST endpoints for CRUD operations on /api/animals with appropriate status codes. ViewController renders Thymeleaf templates for /animals, /animals/add, and /login routes.
Security Configuration
src/main/java/org/example/security/SecurityConfig.java
Configures dual security filter chains: API chain (Order 1) enforces HTTP Basic auth for /api/** with public read access; form login chain (Order 2) handles web UI with /login and ADMIN-only access to /animals/add. Adds BCrypt PasswordEncoder and in-memory users (admin, user).
Configuration & Templates
src/main/resources/application.properties, src/main/resources/application.properties.example, src/main/resources/templates/animals.html, src/main/resources/templates/add-animal.html, src/main/resources/templates/login.html
Adds application.properties with MySQL datasource, JPA, and Security logging configs. Provides example configuration template. Introduces three Thymeleaf HTML templates with glass-panel styling for animal listing, creation form, and login page.
Test Suite Refactoring
src/test/java/org/example/Exercise2025ApplicationTest.java, src/test/java/org/example/controller/AnimalControllerTest.java
Adds Spring Boot integration test with contextLoads method. Introduces controller test suite validating REST security: unauthenticated POST returns 401, admin user POST returns 201, public GET returns 200. Removes obsolete AppIT.java and AppTest.java.
Deleted Files
src/main/java/org/example/App.java, src/test/java/org/example/AppIT.java, src/test/java/org/example/AppTest.java
Removes legacy main entry point and placeholder tests.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Browser
    participant ViewController
    participant AnimalService
    participant AnimalRepository
    participant MySQL

    rect rgb(200, 220, 255)
    Note over User,MySQL: Animal Listing Flow (GET /animals)
    User->>Browser: Click "View Animals"
    Browser->>ViewController: GET /animals
    ViewController->>AnimalService: findAll()
    AnimalService->>AnimalRepository: findAll()
    AnimalRepository->>MySQL: SELECT * FROM animal
    MySQL-->>AnimalRepository: List of animals
    AnimalRepository-->>AnimalService: List<Animal>
    AnimalService-->>ViewController: List<Animal>
    ViewController->>Browser: Render animals.html template
    Browser-->>User: Display animal table
    end

    rect rgb(220, 255, 220)
    Note over User,MySQL: REST API Auth Flow (POST /api/animals)
    User->>Browser: POST /api/animals (no auth)
    Browser->>AnimalController: Request (Unauthorized)
    AnimalController-->>Browser: 401 Unauthorized
    rect rgb(255, 240, 200)
    Note over User: Admin user with Basic Auth
    end
    User->>Browser: POST /api/animals (Basic Auth: admin)
    Browser->>AnimalController: Request (Authorization header)
    AnimalController->>AnimalService: save(Animal)
    AnimalService->>AnimalRepository: save(Animal)
    AnimalRepository->>MySQL: INSERT INTO animal
    MySQL-->>AnimalRepository: Persisted animal
    AnimalRepository-->>AnimalService: Animal
    AnimalService-->>AnimalController: Animal
    AnimalController-->>Browser: 201 Created + Animal JSON
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • pom.xml: Major structural transformation with dependency overhaul and plugin consolidation requires verification of Spring Boot version compatibility and plugin behavior changes
  • SecurityConfig: Dual filter chain configuration with order-dependent logic and in-memory authentication needs careful security audit
  • AnimalController & ViewController: Endpoint coverage and routing logic across REST and MVC paradigms requires consistency validation
  • Data persistence layer: JPA entity, repository, and DataInitializer interaction requires verification of database schema and initialization logic
  • Template files: Three Thymeleaf templates with security directives and model binding require UX and role-based access validation
  • Test coverage: New test suite using WebMvcTest and mocking patterns requires alignment with actual controller behavior
✨ 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 Klumpish/exercise8

Tip

📝 Customizable high-level summaries are now available!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide custom instructions to shape the summary (bullet lists, tables, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example:

"Create a concise high-level summary as a bullet-point list. Then include a Markdown table showing lines added and removed by each contributing author."


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: 9

🧹 Nitpick comments (16)
docker-compose.yml (1)

1-10: Consider adding a volume for data persistence.

Without a volume mapping, all database data will be lost when the container is removed or recreated. For a development/learning environment, adding a named volume ensures data persists across container restarts.

Apply this diff to add a volume:

 services:
   mysql_zoo:
     image: 'mysql:9.5.0'
     environment:
       - MYSQL_DATABASE=${MYSQL_DATABASE}
       - MYSQL_ROOT_PASSWORD=${MYSQL_ROOT_PASSWORD}
       - MYSQL_USER=${MYSQL_USER}
       - MYSQL_PASSWORD=${MYSQL_PASSWORD}
     ports:
       - '3306:3306'
+    volumes:
+      - mysql_zoo_data:/var/lib/mysql
+
+volumes:
+  mysql_zoo_data:
src/main/resources/application.properties.example (2)

4-4: Review JDBC URL security parameters.

The JDBC URL contains useSSL=false and allowPublicKeyRetrieval=true, which weaken security. While acceptable for local development and learning, consider documenting these parameters or using more secure defaults (e.g., useSSL=true with proper certificate configuration) if this template might be used in other environments.

Based on learnings


8-8: Document the ddl-auto setting implications.

The spring.jpa.hibernate.ddl-auto=update setting automatically modifies the database schema, which is convenient for development but can lead to data loss or schema inconsistencies in production. Since this is an example file for a learning project, consider adding a comment to remind users about the implications.

Based on learnings

src/main/resources/templates/add-animal.html (1)

6-77: Consider externalizing CSS to a separate stylesheet.

The inline CSS makes the template harder to maintain and prevents CSS reuse across templates. Consider moving styles to a separate CSS file in src/main/resources/static/css/.

src/main/java/org/example/data/DataInitializer.java (2)

57-58: Use a proper logger instead of System.out.println.

Replace System.out.println with a logger for consistent logging practices and better control over log levels and output formatting.

Apply this diff:

+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 @Configuration
 public class DataInitializer {
+    private static final Logger log = LoggerFactory.getLogger(DataInitializer.class);

     @Bean
     CommandLineRunner initDatabase(AnimalRepository repository) {
         return args -> {
             if (repository.count() == 0) {
                 // ... animal creation code ...
-                System.out.println(
-                  "Seeded initial zoo animals into database!");
+                log.info("Seeded initial zoo animals into database!");
             }
         };
     }

17-55: Use saveAll for better efficiency.

Instead of calling repository.save() for each animal individually, collect all animals in a list and use repository.saveAll() to reduce database round-trips.

Apply this diff:

+import java.util.List;
+
 @Configuration
 public class DataInitializer {
     @Bean
     CommandLineRunner initDatabase(AnimalRepository repository) {
         return args -> {
             if (repository.count() == 0) {
                 Animal leo = new Animal();
                 leo.setName("Leo");
                 leo.setSpecies("Lion");
                 leo.setAge(5);
                 leo.setEnclosure("Savannah");
                 leo.setEndangered(true);
-                repository.save(leo);

                 Animal molly = new Animal();
                 molly.setName("Molly");
                 molly.setSpecies("Giraffe");
                 molly.setAge(7);
                 molly.setEnclosure("Grassland");
                 molly.setEndangered(false);
-                repository.save(molly);

                 // ... similar for other animals ...
                 
+                repository.saveAll(List.of(leo, molly, zara, ellie, polly));

                 System.out.println(
                   "Seeded initial zoo animals into database!");
             }
         };
     }
src/main/java/org/example/Exercise2025Application.java (1)

8-8: Consider removing the obvious comment.

The comment "has to be public static" states a basic Java requirement that would be evident to any Java developer. Consider removing it to reduce noise in the code.

src/main/resources/templates/animals.html (1)

94-113: Consider adding a table caption for accessibility.

The table lacks a <caption> element, which helps screen reader users understand the table's purpose before navigating its contents. While the page heading provides context, an explicit caption improves accessibility.

     <table>
+        <caption style="caption-side: top; text-align: left; font-weight: bold; margin-bottom: 1rem;">
+            List of Zoo Animals
+        </caption>
         <thead>
src/test/java/org/example/controller/AnimalControllerTest.java (2)

9-17: Remove unused imports.

Lines 9 and 17 contain unused imports: SpringBootTest and assertEquals. Line 10's MockBean is also unused since the test uses @MockitoBean from the newer org.springframework.test.context.bean.override.mockito package at line 14.

-import org.springframework.boot.test.context.SpringBootTest;
 import org.springframework.boot.test.mock.mockito.MockBean;
 import org.springframework.context.annotation.Import;
 
-import static org.junit.jupiter.api.Assertions.*;
 import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf;

34-57: Consider expanding test coverage.

The current tests verify POST security and public GET access. However, the PR summary mentions PUT and DELETE endpoints that also require ADMIN role. Consider adding tests for:

  • PUT endpoint with/without admin credentials
  • DELETE endpoint with/without admin credentials
  • Regular USER role attempting admin-only operations (should get 403 Forbidden)

This would provide more comprehensive security verification.

src/main/java/org/example/entity/Animal.java (1)

9-68: Consider adding equals, hashCode, and toString methods.

JPA entities typically benefit from implementing equals(), hashCode(), and toString() methods. While not strictly required for basic CRUD operations, these methods improve:

  • Entity comparison and set operations
  • JPA collection handling and caching behavior
  • Debugging and logging

For a natural key approach using business identifiers:

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (!(o instanceof Animal)) return false;
    Animal animal = (Animal) o;
    return Objects.equals(name, animal.name) && 
           Objects.equals(species, animal.species);
}

@Override
public int hashCode() {
    return Objects.hash(name, species);
}

@Override
public String toString() {
    return "Animal{id=" + id + ", name='" + name + "', species='" + species + 
           "', age=" + age + ", enclosure='" + enclosure + 
           "', endangered=" + endangered + '}';
}
src/main/java/org/example/security/SecurityConfig.java (1)

35-44: Review anyRequest() configuration in API filter chain.

Lines 43-44 specify .anyRequest().authenticated() after already requiring ADMIN role for all /api/** endpoints (line 42). Since this filter chain is scoped to /api/** via securityMatcher at line 33, and line 39 already matches all /api/** paths with .hasRole("ADMIN"), the anyRequest() clause appears unreachable and redundant.

This doesn't cause functional issues but may confuse future maintainers about the intended security model.

Consider clarifying the intent or removing the redundant clause:

             .authorizeHttpRequests(
               auth -> auth.requestMatchers(HttpMethod.GET,
                                            "/api/animals/**")
                           .permitAll() // allow anyone to GET /api/animals
                           .requestMatchers("/api/**")
                           // requires Admin role for all post,put,
                           // delete for /api/**
-                          .hasRole("ADMIN")
-                          .anyRequest()
-                          .authenticated())
+                          .hasRole("ADMIN"))
src/main/java/org/example/service/AnimalService.java (1)

30-32: Consider adding explicit entity existence handling in the delete method.

Spring Data JPA 3.x's deleteById silently ignores calls when the entity doesn't exist. This can be confusing if callers expect feedback about deletion success.

Consider one of these approaches:

Option 1: Check existence explicitly

 public void delete(Long id) {
+    if (!animalRepository.existsById(id)) {
+        throw new EntityNotFoundException("Animal not found with id: " + id);
+    }
     animalRepository.deleteById(id);
 }

Option 2: Return a boolean indicating success

-public void delete(Long id) {
+public boolean delete(Long id) {
+    if (!animalRepository.existsById(id)) {
+        return false;
+    }
     animalRepository.deleteById(id);
+    return true;
 }
src/main/java/org/example/controller/AnimalController.java (2)

6-6: Remove unused import.

The import org.springframework.context.annotation.Role is not used anywhere in this controller.

Apply this diff:

-import org.springframework.context.annotation.Role;

54-58: Consider returning 404 when deleting a non-existent entity.

The delete operation silently succeeds even if the entity doesn't exist. While this is idempotent and acceptable for some REST designs, returning 404 for non-existent resources improves API clarity and helps clients distinguish between successful deletions and no-ops.

If you'd like to return 404 for non-existent entities, apply this diff:

 @DeleteMapping("/{id}")
-@ResponseStatus(HttpStatus.NO_CONTENT)
-public void delete(@PathVariable Long id) {
-    animalService.delete(id);
+public ResponseEntity<Void> delete(@PathVariable Long id) {
+    return animalService.findById(id)
+            .map(existing -> {
+                animalService.delete(id);
+                return ResponseEntity.noContent().<Void>build();
+            })
+            .orElse(ResponseEntity.notFound().build());
 }
pom.xml (1)

60-64: Consider improving Lombok configuration for better IDE support.

The Lombok dependency uses only annotationProcessor scope. While this works for compilation, adding the dependency at compile scope as well improves IDE support and code completion.

Apply this diff:

 <dependency>
     <groupId>org.projectlombok</groupId>
     <artifactId>lombok</artifactId>
-    <scope>annotationProcessor</scope>
+    <optional>true</optional>
 </dependency>

Note: Spring Boot's dependency management already handles Lombok correctly when you remove the scope restriction. The <optional>true</optional> prevents Lombok from being transitively inherited by projects that depend on this one.

📜 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 8ea17be.

📒 Files selected for processing (21)
  • .gitignore (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/Exercise2025Application.java (1 hunks)
  • src/main/java/org/example/controller/AnimalController.java (1 hunks)
  • src/main/java/org/example/controller/ViewController.java (1 hunks)
  • src/main/java/org/example/data/DataInitializer.java (1 hunks)
  • src/main/java/org/example/entity/Animal.java (1 hunks)
  • src/main/java/org/example/repository/AnimalRepository.java (1 hunks)
  • src/main/java/org/example/security/SecurityConfig.java (1 hunks)
  • src/main/java/org/example/service/AnimalService.java (1 hunks)
  • src/main/resources/application.properties (1 hunks)
  • src/main/resources/application.properties.example (1 hunks)
  • src/main/resources/templates/add-animal.html (1 hunks)
  • src/main/resources/templates/animals.html (1 hunks)
  • src/main/resources/templates/login.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/AnimalControllerTest.java (1 hunks)
💤 Files with no reviewable changes (3)
  • src/test/java/org/example/AppIT.java
  • src/main/java/org/example/App.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.243Z
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.243Z
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.243Z
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
  • src/main/resources/application.properties.example
  • src/main/resources/application.properties
  • src/test/java/org/example/Exercise2025ApplicationTest.java
  • pom.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
🔇 Additional comments (5)
src/test/java/org/example/Exercise2025ApplicationTest.java (1)

6-12: LGTM!

Standard Spring Boot context loading test. This ensures the application context initializes successfully, which is a valuable smoke test for configuration issues.

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

6-11: LGTM!

Standard Spring Boot application entry point. The implementation is correct and follows Spring Boot conventions.

docker-compose.yml (1)

3-3: Remove critical issue flag — MySQL 9.5.0 is a valid, current version.

As of November 16, 2025, MySQL 9.5.0 is the latest MySQL 9.x Docker image, published October 22, 2025. The container will start successfully with this version.

Likely an incorrect or invalid review comment.

src/main/java/org/example/security/SecurityConfig.java (1)

27-90: LGTM: Dual security filter chain approach is well-structured.

The two-filter-chain configuration effectively separates API authentication (HTTP Basic, CSRF disabled) from web form authentication. The Order annotations ensure correct precedence, with the API chain handling /api/** first and the form chain handling remaining paths. Role-based access control is properly configured for both chains.

Based on learnings: The in-memory user store with hardcoded credentials is acceptable for this exercise project.

pom.xml (1)

31-31: Java version 25 is available and stable; no action required.

Java SE JDK 25 was released as General Availability on September 16, 2025, with update 25.0.1 released October 21, 2025, and is the latest stable release. The specified version in pom.xml is appropriate for current development environments and CI/CD pipelines.

Comment thread .gitignore
target/
/.idea/
.env
/src/main/resources/application.properties No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify that application.properties is not committed.

The file src/main/resources/application.properties is being added to .gitignore, yet it appears to be part of this PR's committed files. If the file contains environment-specific configuration or credentials, it should be removed from the commit. Only application.properties.example should be tracked in version control.

Run the following script to confirm whether application.properties is being added in this PR:


🏁 Script executed:

#!/bin/bash
# Check if application.properties is being added in the commit
git diff --name-status origin/main...HEAD | grep "application.properties"

Length of output: 40


🏁 Script executed:

#!/bin/bash
# Try multiple approaches to check if application.properties is in the commit
echo "=== Approach 1: Git status ===" 
git status --short | grep -i "application.properties" || echo "Not in git status"

echo ""
echo "=== Approach 2: Git ls-files (tracked files) ===" 
git ls-files | grep -i "application.properties" || echo "Not in tracked files"

echo ""
echo "=== Approach 3: Check if file exists in working directory ===" 
find . -name "application.properties" -type f 2>/dev/null || echo "File not found"

echo ""
echo "=== Approach 4: Git log recent commits ===" 
git log --oneline --name-status -10 | grep -i "application.properties" || echo "Not in recent commits"

Length of output: 481


Remove application.properties from git history before adding .gitignore entry.

The file src/main/resources/application.properties is already tracked in the repository. Adding it to .gitignore now will not prevent it from being tracked—it only affects future commits. The file must first be removed from git history using:

git rm --cached src/main/resources/application.properties

Then commit the .gitignore entry. Without this step, environment-specific configuration remains exposed in the repository history.

🤖 Prompt for AI Agents
In .gitignore around line 4 (entry /src/main/resources/application.properties)
the file is already tracked so adding it to .gitignore has no effect; remove the
tracked file from the index with git rm --cached
src/main/resources/application.properties, commit that removal, then commit the
updated .gitignore so future commits ignore it; if you need the file purged from
repository history as well, run a history-rewriting tool (e.g., git filter-repo
or BFG) and follow your repo's guidelines for force-pushing and notifying
collaborators.

Comment thread pom.xml
Comment on lines +96 to +99
<dependency>
<groupId>org.thymeleaf.extras</groupId>
<artifactId>thymeleaf-extras-springsecurity6</artifactId>
</dependency>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove duplicate dependency.

The thymeleaf-extras-springsecurity6 dependency is declared twice (lines 71-73 and lines 96-99).

Apply this diff:

-        <dependency>
-            <groupId>org.thymeleaf.extras</groupId>
-            <artifactId>thymeleaf-extras-springsecurity6</artifactId>
-        </dependency>
-
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<dependency>
<groupId>org.thymeleaf.extras</groupId>
<artifactId>thymeleaf-extras-springsecurity6</artifactId>
</dependency>
🤖 Prompt for AI Agents
In pom.xml around lines 96 to 99, there's a duplicate declaration of the
thymeleaf-extras-springsecurity6 dependency (already declared at lines 71-73);
remove the redundant dependency block at lines 96-99 so the dependency is
declared only once in the POM.

Comment on lines +42 to +52
@PutMapping("/{id}")
public ResponseEntity<Animal> update(@PathVariable Long id,
@RequestBody Animal animal) {
return animalService.findById(id)
.map(existing -> {
animal.setId(id);
return ResponseEntity.ok(animalService.save(animal));
})
.orElse(ResponseEntity.notFound()
.build());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add validation to the update endpoint.

The create endpoint validates the Animal with @Valid (line 36), but the update endpoint does not apply the same validation. This inconsistency could allow invalid data to be persisted during updates.

Apply this diff:

 @PutMapping("/{id}")
 public ResponseEntity<Animal> update(@PathVariable Long id,
-                                         @RequestBody Animal animal) {
+                                         @RequestBody @Valid Animal animal) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@PutMapping("/{id}")
public ResponseEntity<Animal> update(@PathVariable Long id,
@RequestBody Animal animal) {
return animalService.findById(id)
.map(existing -> {
animal.setId(id);
return ResponseEntity.ok(animalService.save(animal));
})
.orElse(ResponseEntity.notFound()
.build());
}
@PutMapping("/{id}")
public ResponseEntity<Animal> update(@PathVariable Long id,
@RequestBody @Valid Animal animal) {
return animalService.findById(id)
.map(existing -> {
animal.setId(id);
return ResponseEntity.ok(animalService.save(animal));
})
.orElse(ResponseEntity.notFound()
.build());
}
🤖 Prompt for AI Agents
In src/main/java/org/example/controller/AnimalController.java around lines 42 to
52, the update endpoint is missing request-body validation: add javax.validation
validation to the @RequestBody parameter by annotating it with @Valid (matching
the create endpoint), ensure the controller class has the necessary imports
(javax.validation.Valid) and, if not already present, that any class-level
@Validated is applied where needed so that the same bean-validation constraints
used on create are enforced on update; after adding the annotation, run tests or
validation to confirm invalid payloads return 400.

Comment on lines +34 to +38
@PostMapping("/animals/add")
public String addAnimal(@ModelAttribute Animal animal) {
animalService.save(animal);
return "redirect:/animals";
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add input validation and error handling.

The addAnimal endpoint accepts user input without validation. This could allow invalid data (e.g., negative ages, null names, excessively long strings) to be persisted to the database. Additionally, there's no error handling if the save operation fails.

Consider:

  1. Add validation annotations to the Animal entity (e.g., @NotBlank, @Min, @Size)
  2. Add @Valid annotation to the @ModelAttribute Animal parameter
  3. Add a BindingResult parameter to capture validation errors
  4. Return to the form with error messages instead of always redirecting
+import jakarta.validation.Valid;
+import org.springframework.validation.BindingResult;
+
 @PostMapping("/animals/add")
-public String addAnimal(@ModelAttribute Animal animal) {
+public String addAnimal(@Valid @ModelAttribute Animal animal, BindingResult bindingResult, Model model) {
+    if (bindingResult.hasErrors()) {
+        return "add-animal";
+    }
+    try {
         animalService.save(animal);
+    } catch (Exception e) {
+        model.addAttribute("error", "Failed to save animal");
+        return "add-animal";
+    }
     return "redirect:/animals";
 }
🤖 Prompt for AI Agents
In src/main/java/org/example/controller/ViewController.java around lines 34 to
38, the addAnimal POST handler currently accepts unvalidated input and always
redirects, with no handling for validation failures or save errors; update the
method to accept a validated model and capture validation results by adding
@Valid to the Animal parameter and a BindingResult parameter, check
bindingResult.hasErrors() and if true return the original form view (populate
model as needed so errors/values are shown), and wrap the
animalService.save(animal) call in try/catch to handle persistence exceptions
(log the error, add a user-facing error message to the model, and return the
form view) otherwise redirect on success; also ensure the Animal entity has
appropriate validation annotations such as @NotBlank for name, @Min(0) for age
and @Size(max=...) where applicable so the @Valid checks enforce constraints.

Comment on lines +14 to +18
private String name;
private String species;
private Integer age;
private String enclosure;
private boolean endangered;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add validation constraints to entity fields.

The Animal entity lacks validation annotations, which could allow invalid data to be persisted (e.g., null names, negative ages, excessively long strings). This is particularly important given that the ViewController accepts user input without validation.

+import jakarta.validation.constraints.*;
+
 @Entity
 public class Animal {
     @Id
     @GeneratedValue(strategy = GenerationType.IDENTITY)
     private Long id;
+    
+    @NotBlank(message = "Name is required")
+    @Size(max = 100, message = "Name must not exceed 100 characters")
     private String name;
+    
+    @NotBlank(message = "Species is required")
+    @Size(max = 100, message = "Species must not exceed 100 characters")
     private String species;
+    
+    @Min(value = 0, message = "Age must be non-negative")
     private Integer age;
+    
+    @Size(max = 100, message = "Enclosure must not exceed 100 characters")
     private String enclosure;
+    
     private boolean endangered;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private String name;
private String species;
private Integer age;
private String enclosure;
private boolean endangered;
import jakarta.validation.constraints.*;
@Entity
public class Animal {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@NotBlank(message = "Name is required")
@Size(max = 100, message = "Name must not exceed 100 characters")
private String name;
@NotBlank(message = "Species is required")
@Size(max = 100, message = "Species must not exceed 100 characters")
private String species;
@Min(value = 0, message = "Age must be non-negative")
private Integer age;
@Size(max = 100, message = "Enclosure must not exceed 100 characters")
private String enclosure;
private boolean endangered;
🤖 Prompt for AI Agents
In src/main/java/org/example/entity/Animal.java around lines 14 to 18, the
entity fields lack validation; annotate fields to prevent invalid data: add
@NotBlank and @Size(max=100) to name and species, add @Min(0) (and @NotNull if
you change age to Integer) for age to forbid negatives, add @Size(max=200) to
enclosure to limit length, and add @NotNull or keep primitive for endangered (if
you need nullable support change to Boolean and annotate @NotNull). Also add
corresponding javax.validation imports and consider adding @Column(nullable =
false, length = ...) for DB-level constraints where appropriate.

import java.util.List;

public interface AnimalRepository extends JpaRepository<Animal, Long> {
List<Animal> findBySpeciesContainingIgnoreCase(String species);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Verify usage of custom query method.

The findBySpeciesContainingIgnoreCase method is defined but doesn't appear to be used anywhere in the service layer, controllers, or tests. If this method is not needed, consider removing it to keep the codebase lean. If it's for future use, consider adding a TODO comment or test coverage.


🏁 Script executed:

#!/bin/bash
# Verify if findBySpeciesContainingIgnoreCase is actually used
rg -n 'findBySpeciesContainingIgnoreCase' --type=java

Length of output: 128


[/run_scripts]


🏁 Script executed:

#!/bin/bash
# Comprehensive search for any calls to findBySpeciesContainingIgnoreCase
# Check in all Java files including tests
echo "=== Direct method calls ==="
rg -nC3 'findBySpeciesContainingIgnoreCase' --type=java

echo ""
echo "=== Checking if AnimalRepository is used anywhere ==="
rg -n 'AnimalRepository' --type=java -C2

Length of output: 2922


🏁 Script executed:

#!/bin/bash
# Check if the method is referenced as a string (reflection, configuration, etc)
rg -n '"findBySpeciesContainingIgnoreCase"|'"'"'findBySpeciesContainingIgnoreCase'"'"'' --type=java

Length of output: 0


[/run_scripts]


🏁 Script executed:

#!/bin/bash
# Check what methods are actually called on animalRepository in AnimalService
echo "=== Methods called on animalRepository in AnimalService ==="
rg -n 'animalRepository\.' src/main/java/org/example/service/AnimalService.java -A1

echo ""
echo "=== Methods called on repository in DataInitializer ==="
rg -n 'repository\.' src/main/java/org/example/data/DataInitializer.java -A1

Length of output: 752


Remove unused custom query method findBySpeciesContainingIgnoreCase.

The method is defined at line 9 but is never invoked in the service layer (AnimalService uses only findAll(), save(), findById(), deleteById()), data initialization, or elsewhere in the codebase. This is confirmed dead code and should be removed to keep the repository interface clean.

🤖 Prompt for AI Agents
In src/main/java/org/example/repository/AnimalRepository.java around line 9,
remove the unused custom query method declaration
`findBySpeciesContainingIgnoreCase` because it is never called anywhere in the
service layer or codebase; simply delete that method line from the repository
interface and run project tests/compile to ensure no references remain.

Comment on lines +1 to +7
spring.application.name=exercise2025
spring.datasource.url=jdbc:mysql://localhost:3306/exercise8_zoo?useSSL=false&allowPublicKeyRetrieval=true
spring.datasource.username=user
spring.datasource.password=secret
spring.jpa.hibernate.ddl-auto=update
spring.jpa.show-sql=true
logging.level.org.springframework.security=debug No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This file should not be committed to version control.

As noted in the .gitignore changes, application.properties is being added to the ignore list, indicating it contains environment-specific configuration. This file should be removed from the commit, and developers should create their own application.properties from application.properties.example.

Based on learnings

🤖 Prompt for AI Agents
In src/main/resources/application.properties lines 1-7: this
environment-specific properties file containing DB credentials and app settings
must not be committed; remove it from the commit (delete the file from the repo
or revert the addition), ensure it remains listed in .gitignore, add a template
file application.properties.example (sans secrets) if missing, and update README
or contributing notes with instructions for developers to copy
application.properties.example to application.properties and populate their own
local credentials before running.

<div>Species: <input type="text" name="species" required></div>
<div>Age: <input type="number" name="age"></div>
<div>Enclosure: <input type="text" name="enclosure"></div>
<div>Endangered: <input type="text" name="endangered" required></div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use a checkbox for the boolean endangered field.

The endangered field is defined as a text input, but based on the data model (Animal entity with boolean endangered flag), it should be a checkbox to properly represent boolean values and improve user experience.

Apply this diff:

-        <div>Endangered: <input type="text" name="endangered" required></div>
+        <div>
+            <label>
+                <input type="checkbox" name="endangered"> Endangered
+            </label>
+        </div>

Note: If using a checkbox, you may need to adjust the controller to handle the checkbox value (present when checked, absent when unchecked) or add a hidden field for proper binding.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<div>Endangered: <input type="text" name="endangered" required></div>
<div>
<label>
<input type="checkbox" name="endangered"> Endangered
</label>
</div>
🤖 Prompt for AI Agents
In src/main/resources/templates/add-animal.html around line 87, the endangered
field is currently a text input but should be a checkbox to represent the
boolean Animal.endangered; replace the text input with a checkbox input named
"endangered" (checkbox value "true") and add a hidden input with the same name
and value "false" immediately before it so an unchecked box still binds as
false, ensuring the controller can bind to a Boolean/boolean field without
additional changes.

Comment on lines +105 to +115
<form th:action="@{/login}" method="post">
<div><input type="text" name="username" placeholder="Username" required>
</div>
<div><input type="password" name="password" placeholder="Password" required>
</div>
<div>
<button type="submit">Login</button>
</div>
<p th:if="${param.error}" style="color:red;">Invalid username or password</p>
<p th:if="${param.logout}" style="color:green;">You have been logged out</p>
</form>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add accessible labels for form inputs.

The username and password inputs rely solely on placeholder attributes without associated <label> elements. Screen readers may not properly announce the purpose of these fields, making the form inaccessible to users with visual impairments.

Apply this diff to add proper labels:

 <form th:action="@{/login}" method="post">
-    <div><input type="text" name="username" placeholder="Username" required>
+    <div>
+        <label for="username">Username</label>
+        <input type="text" id="username" name="username" placeholder="Username" required>
     </div>
-    <div><input type="password" name="password" placeholder="Password" required>
+    <div>
+        <label for="password">Password</label>
+        <input type="password" id="password" name="password" placeholder="Password" required>
     </div>

You may also want to visually hide the labels with CSS if you prefer the current aesthetic:

label {
    position: absolute;
    left: -10000px;
    width: 1px;
    height: 1px;
    overflow: hidden;
}
🤖 Prompt for AI Agents
In src/main/resources/templates/login.html around lines 105 to 115, the username
and password inputs lack associated labels; add <label> elements for each input
and give each input a matching id (e.g., id="username" and id="password") and a
for attribute on the labels to improve accessibility; keep the existing
placeholders and optionally apply a visually-hidden CSS class to the labels if
you want them hidden visually while remaining accessible to screen readers.

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