Skip to content

Refactor michelle #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yjtpj4zf99-art wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Refactor michelle #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b8bc9a
mich changes
yjtpj4zf99-art Apr 17, 2026
fad75ce
new mich changes
yjtpj4zf99-art Apr 17, 2026
51af78c
for deployment automation
yjtpj4zf99-art Apr 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@

FROM eclipse-temurin:21-jdk-alpine AS build
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

JDK version mismatch with pom.xml (java.version=17).

The build and runtime images use JDK/JRE 21 while pom.xml targets Java 17. The build will succeed (21 ≥ 17), but running the resulting JAR on a JRE that differs from the declared target can hide compatibility issues and drift between local/CI and container runtimes. Either bump <java.version> to 21 or switch the images to eclipse-temurin:17-... to keep them aligned.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` at line 2, The Dockerfile uses "FROM
eclipse-temurin:21-jdk-alpine AS build" which mismatches the project's
<java.version> (17) in pom.xml; align them by either updating the pom.xml
<java.version> to 21 or changing both Dockerfile images (build and runtime) to
eclipse-temurin:17-... so the container JDK/JRE matches the declared target Java
version; locate the image references in the Dockerfile (the FROM lines) and the
<java.version> property in pom.xml and make the corresponding one-line change to
keep build/runtime and project target consistent.

WORKDIR /app

COPY .mvn/ .mvn
COPY mvnw pom.xml ./
RUN ./mvnw dependency:go-offline
Comment on lines +5 to +7
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Maven wrapper will fail: maven-wrapper.jar missing and mvnw likely not executable.

Per the cross-file context, .mvn/wrapper/ contains only maven-wrapper.properties — maven-wrapper.jar is absent, so ./mvnw dependency:go-offline (line 7) and ./mvnw clean package (line 11) will fail inside the container. Additionally, mvnw needs the executable bit; on hosts where it isn't set in git, the RUN ./mvnw ... step fails with "permission denied".

🛠️ Suggested fix 
 COPY .mvn/ .mvn
 COPY mvnw pom.xml ./
+RUN chmod +x mvnw
 RUN ./mvnw dependency:go-offline

Also commit .mvn/wrapper/maven-wrapper.jar (run mvn -N wrapper:wrapper locally) or switch the build stage to a maven:3.9-eclipse-temurin-21 image and invoke mvn directly instead of the wrapper.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 5 - 7, The Dockerfile uses the Maven wrapper (./mvnw
and .mvn/wrapper/maven-wrapper.properties) but the maven-wrapper.jar is not
present and mvnw may lack the executable bit, so the RUN ./mvnw ... steps will
fail; fix by either committing .mvn/wrapper/maven-wrapper.jar (generate it
locally with mvn -N wrapper:wrapper) and ensure mvnw is executable (git
update-index --add --chmod=+x mvnw or set chmod +x in the Dockerfile before
calling it), or change the build stage to a Maven base image (e.g.,
maven:3.9-eclipse-temurin-21) and call mvn dependency:go-offline / mvn clean
package directly instead of ./mvnw.



COPY src ./src
RUN ./mvnw clean package -DskipTests


FROM eclipse-temurin:21-jre-alpine
WORKDIR /app
COPY --from=build /app/target/*.jar app.jar
EXPOSE 8080
ENTRYPOINT ["java", "-jar", "app.jar"]
Comment on lines +14 to +18
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Run as a non-root user and narrow the JAR copy glob.

Trivy DS-0002: no USER directive, so the container runs as root. Add an unprivileged user in the runtime stage. Also, COPY --from=build /app/target/*.jar app.jar silently misbehaves if Maven produces more than one jar (e.g., *-sources.jar, *-javadoc.jar) — pin the finalName or use a more specific pattern.

🔒 Suggested fix 
 FROM eclipse-temurin:21-jre-alpine
 WORKDIR /app
-COPY --from=build /app/target/*.jar app.jar
+RUN addgroup -S app && adduser -S app -G app
+COPY --from=build /app/target/*.jar app.jar
+RUN chown app:app app.jar
+USER app
 EXPOSE 8080
 ENTRYPOINT ["java", "-jar", "app.jar"]
📝 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
FROM eclipse-temurin:21-jre-alpine
WORKDIR /app
COPY --from=build /app/target/*.jar app.jar
EXPOSE 8080
ENTRYPOINT ["java", "-jar", "app.jar"]
FROM eclipse-temurin:21-jre-alpine
WORKDIR /app
RUN addgroup -S app && adduser -S app -G app
COPY --from=build /app/target/*.jar app.jar
RUN chown app:app app.jar
USER app
EXPOSE 8080
ENTRYPOINT ["java", "-jar", "app.jar"]
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile` around lines 14 - 18, The runtime Dockerfile currently runs as
root and copies all jars with a broad glob; update the runtime stage to create
and switch to an unprivileged user (e.g., add a non-root user/group, chown the
app.jar and add a USER directive) and narrow the COPY from the build stage by
pinning the artifact name or using a specific pattern instead of
/app/target/*.jar (replace COPY --from=build /app/target/*.jar app.jar with a
copy that targets the final artifact name or a more specific glob), ensuring the
jar ownership is set for the new user before switching to it.

37 changes: 37 additions & 0 deletions docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
services:
db:
image: mysql:8.0
container_name: libraryman-db
environment:
MYSQL_DATABASE: libraryman
MYSQL_ROOT_PASSWORD: root
ports:
- "3306:3306"
Comment on lines +5 to +9
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hardcoded root credentials and DB port exposed on host.

  • MYSQL_ROOT_PASSWORD: root and using root as the application user is insecure even for staging — app should use a dedicated, least-privileged user. Prefer MYSQL_USER/MYSQL_PASSWORD and sourcing secrets via an .env file (and .gitignore it).
  • Publishing 3306:3306 exposes MySQL on the host interface. Since only the api service needs it, drop the host mapping and rely on the internal Docker network (expose: ["3306"] or omit entirely).
  • No volume is declared, so DB data is lost on docker compose down. Add a named volume mounted at /var/lib/mysql.
🔧 Suggested adjustment 
   db:
     image: mysql:8.0
     container_name: libraryman-db
     environment:
       MYSQL_DATABASE: libraryman
-      MYSQL_ROOT_PASSWORD: root
-    ports:
-      - "3306:3306"
+      MYSQL_ROOT_PASSWORD: ${MYSQL_ROOT_PASSWORD}
+      MYSQL_USER: ${MYSQL_USER}
+      MYSQL_PASSWORD: ${MYSQL_PASSWORD}
+    expose:
+      - "3306"
+    volumes:
+      - db-data:/var/lib/mysql

and add at the bottom:

volumes:
  db-data:
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 5 - 9, The docker-compose service currently
hardcodes MYSQL_ROOT_PASSWORD and exposes port 3306 to the host and lacks
persistent storage; change to use a least-privileged DB user by replacing
MYSQL_ROOT_PASSWORD/root with MYSQL_USER and MYSQL_PASSWORD (read values from an
.env file and add that .env to .gitignore), remove the host port mapping
("3306:3306") so the DB is not published (use expose: ["3306"] or omit ports
entirely) and add a named volume mounted at /var/lib/mysql for persistence
(declare a top-level volume such as db-data). Ensure references to MYSQL_USER,
MYSQL_PASSWORD, ports/expose, and the volume name (db-data) are updated in the
service definition.

healthcheck:
test: ["CMD", "mysqladmin" ,"ping", "-h", "localhost"]
timeout: 20s
retries: 10

api:
build: .
container_name: libraryman-api-staging
ports:
- "8080:8080"
depends_on:
db:
condition: service_healthy
environment:
- SPRING_DATASOURCE_URL=jdbc:mysql://db:3306/libraryman
- SPRING_DATASOURCE_USERNAME=root
- SPRING_DATASOURCE_PASSWORD=root
Comment on lines +24 to +26
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

App should not connect as MySQL root.

SPRING_DATASOURCE_USERNAME=root/PASSWORD=root grants the application full administrative privileges on the DB server — a clear least-privilege violation. Create a dedicated user (via MYSQL_USER/MYSQL_PASSWORD on the db service) scoped to the libraryman schema and reference those here, ideally via ${...} from an .env file rather than inline literals.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 24 - 26, Replace use of MySQL root
credentials by creating and referencing a dedicated, least-privilege DB user:
set MYSQL_USER and MYSQL_PASSWORD on the db service to create a non-root account
scoped to the libraryman schema, update SPRING_DATASOURCE_USERNAME and
SPRING_DATASOURCE_PASSWORD to reference those values (use ${MYSQL_USER} and
${MYSQL_PASSWORD} from an .env file rather than literals), and ensure the DB
initialization creates/grants that user only the needed privileges for the
libraryman database so the application does not connect as root.

- SPRING_MAIL_HOST=localhost
- SPRING_MAIL_PORT=25
- SPRING_MAIL_PROPERTIES_MAIL_SMTP_AUTH=false
- SPRING_MAIL_PROPERTIES_MAIL_SMTP_STARTTLS_ENABLE=false
- SPRING_MAIL_PROPERTIES_DOMAIN_NAME=libraryman.com
# DISABLE OAUTH2 FEATURES COMPLETELY
- SPRING_SECURITY_OAUTH2_CLIENT_ENABLED=false
- SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED=false
# PROVIDE A DUMMY REGISTRATION JUST IN CASE THE BEAN SCANNER STILL LOOKS
- SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_ID=dummy
- SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_SECRET=dummy
Comment on lines +32 to +37
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 18, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Is there a Spring Boot property spring.security.oauth2.client.enabled that disables the OAuth2 client auto-configuration, or is SPRING_AUTOCONFIGURE_EXCLUDE the correct mechanism?

💡 Result:

No, there is no Spring Boot property spring.security.oauth2.client.enabled that disables the OAuth2 client auto-configuration. The correct mechanism is to use spring.autoconfigure.exclude (or the exclude attribute on @SpringBootApplication) to exclude the OAuth2ClientAutoConfiguration class. The OAuth2 client auto-configuration class is org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration, which imports the registration repository and web security configurations when spring-security-oauth2-client is on the classpath and it's a servlet web application. To disable it via properties: spring.autoconfigure.exclude=org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration Official Spring Boot documentation on disabling auto-configuration confirms this approach, and the GitHub issue #6474 in Spring Security explicitly states that Spring Boot provides this capability for OAuth2ClientAutoConfiguration rather than a dedicated disable property. No sources mention or document a spring.security.oauth2.client.enabled property; all OAuth2 client properties are under spring.security.oauth2.client.registration.* and provider.* for configuring registrations, not enabling/disabling the auto-config itself.

Citations:

🏁 Script executed:

# Search for WebConfiguration file
fd -i WebConfiguration

# Search for oauth2Login references
rg "oauth2Login" -i --type java

Repository: ajaynegi45/LibraryMan-API

Length of output: 252

🏁 Script executed:

# Check docker-compose.yml full content
cat -n docker-compose.yml | head -60

# Search for SPRING_AUTOCONFIGURE_EXCLUDE usage
rg "SPRING_AUTOCONFIGURE_EXCLUDE|spring.autoconfigure.exclude" -i

Repository: ajaynegi45/LibraryMan-API

Length of output: 1563

Remove dummy OAuth2 credentials; use SPRING_AUTOCONFIGURE_EXCLUDE to properly disable OAuth2 auto-configuration.

The SPRING_SECURITY_OAUTH2_CLIENT_ENABLED and SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED properties are not standard Spring Boot properties and will not prevent OAuth2 auto-configuration. With dummy credentials present, the application may start, but OAuth2 login attempts will fail at runtime. Instead, exclude the auto-configuration classes entirely by setting:

SPRING_AUTOCONFIGURE_EXCLUDE=org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration

Alternatively, define a Spring profile that excludes oauth2Login() from the security chain for local/staging environments.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 32 - 37, Remove the dummy OAuth2 credential
environment variables
(SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_ID and
SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_GOOGLE_CLIENT_SECRET) and stop
relying on non-standard flags SPRING_SECURITY_OAUTH2_CLIENT_ENABLED /
SPRING_SECURITY_OAUTH2_RESOURCESERVER_ENABLED; instead add
SPRING_AUTOCONFIGURE_EXCLUDE with the value
org.springframework.boot.autoconfigure.security.oauth2.client.servlet.OAuth2ClientAutoConfiguration
to the compose config to fully disable OAuth2 client auto-configuration, or
alternatively implement a Spring profile that excludes oauth2Login() from your
SecurityFilterChain for local/staging environments.

2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
<dependency>
<groupId>io.jsonwebtoken</groupId>
<artifactId>jjwt-jackson</artifactId>
<version>0.12.0</version>
<version>0.11.5</version>
<scope>runtime</scope>
</dependency>

Expand Down
10 changes: 7 additions & 3 deletions src/main/java/com/libraryman_api/book/BookController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@
* This controller provides endpoints for performing CRUD operations on books,
* including retrieving all books, getting a book by its ID, adding a new book,
* updating an existing book, and deleting a book.
* * Version: 1.1
*/
@RestController
@RequestMapping("/api")
@RequestMapping("/api/v1/books") //Versioning API path -> provide more context
Comment on lines 22 to +23
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 17, 2026

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

🏁 Script executed:

#!/bin/bash
# Find references to old BookController paths anywhere in the repo.
rg -nP '/api/(get-all-books|get-book-by-id|add-book|update-book|delete-book|book/search)' -C1
# Find MockMvc or test usages of book endpoints.
rg -nP --type=java -C2 '(perform|get|post|put|delete)\s*\(\s*"[^"]*(book|api)[^"]*"'
# Check MemberController base path for cross-controller consistency.
rg -nP --type=java -C1 '@RequestMapping\('

Repository: ajaynegi45/LibraryMan-API

Length of output: 2183

🏁 Script executed:

# Check what methods exist in BookController now
rg -A 30 '@RestController' src/main/java/com/libraryman_api/book/BookController.java | head -50

# Verify WebConfiguration is actually using those old paths
cat src/main/java/com/libraryman_api/security/config/WebConfiguration.java | grep -A 2 -B 2 '/api/get-all-books\|/api/book/search'

# Check if there are any other references to the old /api paths for books
rg -i '/api/(add-book|get-book|update-book|delete-book)' --type=java

Repository: ajaynegi45/LibraryMan-API

Length of output: 2023

🏁 Script executed:

# Check if there are test files for BookController
find . -name "*BookControllerTest*" -o -name "*BookTest*" | grep -i test

# Check the README or API documentation for endpoint references
rg -i 'api/.*book' README.md OpenAPI* docs* 2>/dev/null || echo "No docs found matching pattern"

# See the full WebConfiguration to understand the impact
cat src/main/java/com/libraryman_api/security/config/WebConfiguration.java

Repository: ajaynegi45/LibraryMan-API

Length of output: 4313

🏁 Script executed:

# Check if WebConfiguration has been updated to permit the new /api/v1/books paths
rg '/api/v1' src/main/java/com/libraryman_api/security/config/WebConfiguration.java

# List all endpoints in BookController to understand the full scope of the breaking change
rg -B 1 '@(GetMapping|PostMapping|PutMapping|DeleteMapping|PatchMapping)' src/main/java/com/libraryman_api/book/BookController.java | head -40

Repository: ajaynegi45/LibraryMan-API

Length of output: 340

🏁 Script executed:

# Confirm there are no permission rules for /api/v1/books in WebConfiguration
rg '/api/v1/books' src/main/java/com/libraryman_api/security/config/WebConfiguration.java

# Check if there are any test files and their current endpoints
find . -path "*/test/*" -name "*BookController*" -type f
find . -path "*/test/*" -name "*Test.java" | xargs grep -l "book" 2>/dev/null | head -5

Repository: ajaynegi45/LibraryMan-API

Length of output: 348

🏁 Script executed:

# Check if BookServiceTest or any test actually tests the endpoints
cat src/test/java/com/libraryman_api/book/BookServiceTest.java | head -30

# Search for any MockMvc tests that test book endpoints
rg -i 'mockmvc|perform.*book' --type=java src/test/

# Check if there's a general integration test or e2e test
find . -path "*/test/*" -name "*IntegrationTest*" -o -path "*/test/*" -name "*E2E*"

Repository: ajaynegi45/LibraryMan-API

Length of output: 1082

🏁 Script executed:

# Check the other endpoints and see what methods they reference
rg -A 1 '@(GetMapping|PostMapping|PutMapping|DeleteMapping)' src/main/java/com/libraryman_api/book/BookController.java | grep -E '@|value|path'

# Verify what the actual endpoint paths will be when fully qualified (base path + method mapping)
echo "BookController base: /api/v1/books"
rg '@(GetMapping|PostMapping|PutMapping|DeleteMapping)\(' src/main/java/com/libraryman_api/book/BookController.java

Repository: ajaynegi45/LibraryMan-API

Length of output: 1040

Security configuration out of sync with new BookController paths — endpoints will be inaccessible.

The base path change from /api to /api/v1/books breaks existing clients, but there's a critical accompanying issue: WebConfiguration.java (lines 43-44) still permits the old /api/get-all-books/** and /api/book/search** paths, while the new /api/v1/books/* paths have no matching security rules. This means:

  1. Old endpoints (/api/get-all-books, etc.) return 404 (no longer exist).
  2. New endpoints (/api/v1/books/get-all-books, etc.) likely fail with 403 Forbidden (no security rule permits them), unless they accidentally fall under .anyRequest().authenticated() in a way that breaks the intended behavior.

Required fixes:

  • Update WebConfiguration.java to permit /api/v1/books/** instead of /api/get-all-books/** and /api/book/search**.
  • Update MockMvc tests (if BookControllerTest exists) to use the new /api/v1/books paths.
  • Update frontend, API docs (README/OpenAPI), and Postman collections to reflect /api/v1/books.
  • Align MemberController (still on /api, src/main/java/com/libraryman_api/member/MemberController.java:22) for API consistency, or document why BookController alone uses versioning.
  • Consider a deprecation window if external clients are consuming this API.
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/main/java/com/libraryman_api/book/BookController.java` around lines 22 -
23, Your BookController class changed its base path to "/api/v1/books" but
security rules in WebConfiguration still permit the old "/api/..." paths,
causing new endpoints to be blocked; update WebConfiguration to replace any
permitted patterns like "/api/get-all-books/**" and "/api/book/search**" with
"/api/v1/books/**" (or add explicit permits for the new pattern) so requests to
BookController (class BookController, `@RequestMapping`("/api/v1/books")) are
allowed, update any MockMvc tests in BookControllerTest to use the new
"/api/v1/books" URLs, and audit MemberController and API docs/tests/Postman
collections to either migrate to the v1 path or document/version the difference
for consistency.

public class BookController {

@Autowired
Expand Down Expand Up @@ -80,7 +81,8 @@ public BookDto addBook(@Valid @RequestBody BookDto bookDto) {

/**
* Updates an existing book in the library.
*
* This operation is restricted to LIBRARIAN and ADMIN roles.
*
* @param id the ID of the book to update.
* @param bookDtoDetails the {@link Book} object containing the updated book details.
* @return the updated {@link Book} object.
Expand All @@ -93,13 +95,15 @@ public BookDto updateBook(@PathVariable int id, @Valid @RequestBody BookDto book

/**
* Deletes a book from the library by its ID.
* Provides confirmation message upon successful deletion.
*
* @param id the ID of the book to delete.
*/
@DeleteMapping("delete-book/{id}")
@PreAuthorize("hasRole('LIBRARIAN') or hasRole('ADMIN')")
public void deleteBook(@PathVariable int id) {
public ResponseEntity<String> deleteBook(@PathVariable int id) {
bookService.deleteBook(id);
return ResponseEntity.ok("Book with ID " + id + " has been successfully deleted from the system.");
}

/**
Expand Down
Loading