Conversation
WalkthroughRefactored Main to delegate startup to a new instance method Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as Console User
participant Main as Main.run()
participant DB as JDBC Database
participant Dev as DevDatabaseInitializer
Note over Main,Dev: App startup
User->>Main: start (args)
Main->>Main: isDevMode(args)?
alt dev mode
Main->>Dev: DevDatabaseInitializer.start()
Dev-->>Main: dev DB ready
end
Main->>User: prompt login/menu
loop user session
User->>Main: select action (list/get/count/create/update/delete/logout)
alt query/read action
Main->>DB: execute SELECT
DB-->>Main: result set
Main-->>User: display results
else write action
Main->>DB: execute INSERT/UPDATE/DELETE (prepared)
DB-->>Main: success/failure
Main-->>User: success/failure message
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/example/Main.java (1)
21-37: Remove or use the no-op connection inrun()(it currently connects and immediately closes).
At Line 33-36 you open aConnectionbut don’t use it; either delete this block or keep a single connection and pass it into the handlers (or move to a small repository/DataSource setup).- try (Connection connection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass)) { - } catch (SQLException e) { - throw new RuntimeException(e); - } + // Option A: remove this block entirely (handlers will fail with clear messages if DB is unreachable) + // Option B (preferred longer-term): create a DataSource / reuse a single Connection across the session
🧹 Nitpick comments (3)
src/main/java/com/example/Main.java (3)
118-133: Connection-per-operation is acceptable for G, but consider consolidating (perf/cleanliness).
Right now every menu action opens a new JDBC connection; for anything beyond the assignment, prefer a sharedDataSource(or at least a singleConnectionper session).
135-161: CloseResultSetexplicitly (try-with-resources) to avoid leaks if code grows.
Even though many drivers closeResultSetwhen the statement closes, it’s safer/clearer to include it in the try-with-resources.- try ( - Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); - PreparedStatement missionQuery = databaseConnection.prepareStatement(sql) - ) { + try ( + Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); + PreparedStatement missionQuery = databaseConnection.prepareStatement(sql); + ResultSet missionResult = (missionQuery.setLong(1, missionId), missionQuery.executeQuery()) + ) { missionQuery.setLong(1, missionId); - ResultSet missionResult = missionQuery.executeQuery();(If you don’t want the “comma expression” style, just add
ResultSetas a named resource after executing—same idea.)
163-188:YEAR(launch_date)is fine for this assignment; note it can block index usage in real DBs.
Not a blocker here; just something to keep in mind if performance ever matters.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)src/main/java/com/example/Main.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/Main.java (1)
src/main/java/com/example/DevDatabaseInitializer.java (1)
DevDatabaseInitializer(6-24)
🔇 Additional comments (5)
src/main/java/com/example/Main.java (5)
60-98: Menu loop structure is OK; be careful changing prompts/output since tests likely assert exact text.
Given this is a test-driven CLI, any future refactors should preserve observable STDOUT prompts.
100-116:printMenu()is clear; good use of text block.
228-277: CRUD statements are parameterized; looks good.
279-299: Login query is safely parameterized; OK for this assignment (but plaintext passwords).
Storing plaintext passwords is presumably per lab constraints; otherwise you’d hash+salt.
11-19: Dev-mode bootstrap looks clean; version tags are valid.
The versions used are confirmed:mysql:9.5.0is available on Docker Hub (MySQL 9.5.0 Innovation release, Oct 21, 2025), andmysql-connector-jversion9.5.0is a GA release (Oct 22, 2025). No version validity concerns.
| @@ -1,3 +1,4 @@ | |||
| [](https://classroom.github.com/a/339Lr3BJ) | |||
There was a problem hiding this comment.
Badge link is fine; just confirm it’s appropriate for the repo’s visibility (public vs classroom-only).
If this repo is public, consider whether you want to expose the Classroom assignment link.
🤖 Prompt for AI Agents
In README.md around lines 1 to 1, the Classroom assignment badge may expose a
classroom-only link if the repo is public; confirm the repository visibility and
either remove the badge from the public README or replace it with a
non-sensitive/public-facing badge. If the repo must remain public and the badge
should not be exposed, delete the badge line and optionally add a note
indicating the assignment is in a private classroom; if the repo is private or
intended for classroom use, leave the badge but add a short comment in the
README clarifying that it links to a classroom assignment and is safe for this
repository.
| //Inloggning | ||
| boolean inloggad = false; | ||
| try (BufferedReader reader = new BufferedReader(new InputStreamReader(System.in))) { | ||
| while (!inloggad) { | ||
| try { | ||
| System.out.print("Username: "); | ||
| String username = reader.readLine(); | ||
| System.out.print("Password: "); | ||
| String password = reader.readLine(); | ||
| inloggad = validateLogin(jdbcUrl, dbUser, dbPass, username, password); | ||
| if (!inloggad) { | ||
| System.out.println("Invalid username or password. Press 0 to exit."); | ||
| if (reader.readLine().equals("0")) { | ||
| return; | ||
| } | ||
| } | ||
| } catch (IOException e) { | ||
| System.err.println("Error: " + e.getMessage()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Avoid potential NPE on exit prompt when stdin closes (EOF).
At Line 51 reader.readLine().equals("0") can throw if readLine() returns null.
- if (reader.readLine().equals("0")) {
+ String exitChoice = reader.readLine();
+ if ("0".equals(exitChoice)) {
return;
}📝 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.
| //Inloggning | |
| boolean inloggad = false; | |
| try (BufferedReader reader = new BufferedReader(new InputStreamReader(System.in))) { | |
| while (!inloggad) { | |
| try { | |
| System.out.print("Username: "); | |
| String username = reader.readLine(); | |
| System.out.print("Password: "); | |
| String password = reader.readLine(); | |
| inloggad = validateLogin(jdbcUrl, dbUser, dbPass, username, password); | |
| if (!inloggad) { | |
| System.out.println("Invalid username or password. Press 0 to exit."); | |
| if (reader.readLine().equals("0")) { | |
| return; | |
| } | |
| } | |
| } catch (IOException e) { | |
| System.err.println("Error: " + e.getMessage()); | |
| } | |
| } | |
| //Inloggning | |
| boolean inloggad = false; | |
| try (BufferedReader reader = new BufferedReader(new InputStreamReader(System.in))) { | |
| while (!inloggad) { | |
| try { | |
| System.out.print("Username: "); | |
| String username = reader.readLine(); | |
| System.out.print("Password: "); | |
| String password = reader.readLine(); | |
| inloggad = validateLogin(jdbcUrl, dbUser, dbPass, username, password); | |
| if (!inloggad) { | |
| System.out.println("Invalid username or password. Press 0 to exit."); | |
| String exitChoice = reader.readLine(); | |
| if ("0".equals(exitChoice)) { | |
| return; | |
| } | |
| } | |
| } catch (IOException e) { | |
| System.err.println("Error: " + e.getMessage()); | |
| } | |
| } | |
🤖 Prompt for AI Agents
In src/main/java/com/example/Main.java around lines 39 to 59, the call
reader.readLine().equals("0") can throw a NullPointerException if stdin is
closed (readLine() returns null); to fix, read the line into a local variable,
check it for null before calling equals (or use "0".equals(line)), and treat
null the same as an exit condition (return or break) so the program handles EOF
safely.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/example/Main.java (1)
39-59: NPE risk still present on exit prompt.Line 51 still calls
reader.readLine().equals("0")which can throwNullPointerExceptionifreadLine()returns null (e.g., when stdin closes or EOF is reached).Apply this diff to safely handle null:
- if (reader.readLine().equals("0")) { + String exitChoice = reader.readLine(); + if ("0".equals(exitChoice)) { return; }Alternatively, treat null as an exit condition:
- if (reader.readLine().equals("0")) { + String exitChoice = reader.readLine(); + if (exitChoice == null || "0".equals(exitChoice)) { return; }
🧹 Nitpick comments (3)
src/main/java/com/example/Main.java (3)
33-36: Consider removing or enhancing the connection test.The try-with-resources block opens and immediately closes a connection without using it. This provides minimal value as a smoke test and any connection errors will surface during actual operations below.
You can either remove these lines entirely or enhance the test by executing a simple query like
SELECT 1to verify the connection is functional.
135-161: Add ResultSet to try-with-resources for explicit cleanup.The ResultSet at line 146 is not explicitly managed in the try-with-resources block. While it will be closed when the PreparedStatement closes, explicit management is clearer and more defensive.
Apply this diff:
String sql = "SELECT mission_id, spacecraft, launch_date FROM moon_mission WHERE mission_id = ?"; try ( Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); - PreparedStatement missionQuery = databaseConnection.prepareStatement(sql) + PreparedStatement missionQuery = databaseConnection.prepareStatement(sql); + ResultSet missionResult = missionQuery.executeQuery(); ) { missionQuery.setLong(1, missionId); - ResultSet missionResult = missionQuery.executeQuery(); if (missionResult.next()) {Wait, that won't work because you need to set parameters before executing. Better approach:
String sql = "SELECT mission_id, spacecraft, launch_date FROM moon_mission WHERE mission_id = ?"; try ( Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); PreparedStatement missionQuery = databaseConnection.prepareStatement(sql) ) { missionQuery.setLong(1, missionId); - ResultSet missionResult = missionQuery.executeQuery(); + try (ResultSet missionResult = missionQuery.executeQuery()) { - if (missionResult.next()) { + if (missionResult.next()) { System.out.println("\nMission Details:"); System.out.println("Mission ID: " + missionResult.getLong("mission_id")); System.out.println("Spacecraft: " + missionResult.getString("spacecraft")); System.out.println("Launch Date: " + missionResult.getDate("launch_date")); - } else { + } else { System.out.println("No mission found with ID: " + missionId); + } } }
163-188: Same ResultSet management pattern as getMoonMissionById.Line 174's ResultSet has the same resource management pattern as the previous method. Consider applying a similar nested try-with-resources for explicit cleanup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/example/Main.java(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/Main.java (1)
src/main/java/com/example/DevDatabaseInitializer.java (1)
DevDatabaseInitializer(6-24)
🔇 Additional comments (6)
src/main/java/com/example/Main.java (6)
11-19: LGTM! Clean startup delegation.The main method correctly handles dev-mode detection and delegates to the instance method
run(). The structure is clean and testable.
60-98: LGTM! Well-structured menu loop.The menu loop is clean, properly handles all cases including invalid input, and delegates to focused helper methods. Error handling is appropriate.
118-133: LGTM! Proper resource management.The method correctly uses try-with-resources for all JDBC resources and appropriately uses
Statementfor a parameterless query.
202-216: Good fix for the substring issue!The previous review's concern about substring crash on short names has been properly addressed. The code now validates that names are not empty and uses safe substring logic with length checks.
268-290: LGTM! Proper DELETE implementation.The method correctly uses a prepared statement to prevent SQL injection and provides appropriate user feedback based on whether the account was found and deleted.
321-339: LGTM! Well-designed configuration utilities.Both
isDevModeandresolveConfigare properly implemented with clear precedence rules and good documentation. The multiple detection methods for dev mode provide flexibility for different environments.
| String sql = "INSERT INTO account (first_name, last_name, ssn, password, name) VALUES (?, ?, ?, ?, ?)"; | ||
| try ( | ||
| Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); | ||
| PreparedStatement insertAccount = databaseConnection.prepareStatement(sql) | ||
| ) { | ||
| insertAccount.setString(1, firstName); | ||
| insertAccount.setString(2, lastName); | ||
| insertAccount.setString(3, ssn); | ||
| insertAccount.setString(4, password); | ||
| insertAccount.setString(5, name); | ||
|
|
||
| int rowsAffected = insertAccount.executeUpdate(); | ||
| if (rowsAffected > 0) { | ||
| System.out.println("Account created!"); // Bekräftelse som testet letar efter | ||
| } else { | ||
| System.out.println("Failed to create account."); | ||
| } | ||
| } | ||
| } catch (SQLException | IOException e) { | ||
| System.err.println("Error: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Plaintext password storage.
Line 227 stores passwords in plaintext in the database. This is a severe security vulnerability. Passwords should always be hashed using a strong algorithm (BCrypt, Argon2, PBKDF2) before storage.
Consider using a library like BCrypt:
import org.mindrot.jbcrypt.BCrypt;
// When creating account:
String hashedPassword = BCrypt.hashpw(password, BCrypt.gensalt(12));
insertAccount.setString(4, hashedPassword);Then update validateLogin to use BCrypt.checkpw(password, hashedPassword) for comparison.
Note: This also impacts the updateAccountPassword and validateLogin methods.
| private void updateAccountPassword(BufferedReader reader, String jdbcUrl, String dbUser, String dbPass) { | ||
| try { | ||
| System.out.print("Enter user ID: "); | ||
| long userId = Long.parseLong(reader.readLine()); | ||
| System.out.print("New password: "); | ||
| String newPassword = reader.readLine(); | ||
|
|
||
| String sql = "UPDATE account SET password = ? WHERE user_id = ?"; | ||
| try ( | ||
| Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); | ||
| PreparedStatement updatePassword = databaseConnection.prepareStatement(sql) | ||
| ) { | ||
| updatePassword.setString(1, newPassword); | ||
| updatePassword.setLong(2, userId); | ||
|
|
||
| int rowsAffected = updatePassword.executeUpdate(); | ||
| if (rowsAffected > 0) { | ||
| System.out.println("Password updated!"); // Bekräftelse som testet letar efter | ||
| } else { | ||
| System.out.println("User not found."); | ||
| } | ||
| } | ||
| } catch (SQLException | IOException | NumberFormatException e) { | ||
| System.err.println("Error: " + e.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Plaintext password issue also applies here.
Line 253 updates passwords in plaintext. This method should hash the new password before storing it, consistent with the security concern raised in createAccount.
🤖 Prompt for AI Agents
In src/main/java/com/example/Main.java around lines 241 to 266, the method
currently stores the new password in plaintext (line ~253); instead hash the new
password before saving using the same hashing utility/mechanism used in
createAccount (e.g., bcrypt with a generated salt or your existing
PasswordHasher.hash(password) method). Replace the direct newPassword usage with
a call that produces a hashedPassword, perform any same validation
(length/requirements) as createAccount, then set the hashedPassword on the
PreparedStatement parameter before executeUpdate, preserving the existing
try-with-resources and exception handling.
| private boolean validateLogin(String jdbcUrl, String dbUser, String dbPass, String username, String password){ | ||
| String sqlQuery = "SELECT COUNT(*) FROM account WHERE name = ? AND password = ?"; | ||
|
|
||
| try ( | ||
| Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass); | ||
| PreparedStatement preparedStatement = databaseConnection.prepareStatement(sqlQuery) | ||
| ) { | ||
| preparedStatement.setString(1, username); | ||
| preparedStatement.setString(2, password); | ||
|
|
||
| ResultSet resultSet = preparedStatement.executeQuery(); | ||
|
|
||
| if (resultSet.next()) { | ||
| return resultSet.getInt(1) > 0; // Returnera true om användaren finns | ||
| } | ||
| } catch (SQLException e) { | ||
| System.err.println("Fel vid validering av inloggning: " + e.getMessage()); | ||
| } | ||
|
|
||
| return false; // Returnera false om något gick fel eller om användaren inte finns | ||
| } |
There was a problem hiding this comment.
Multiple security concerns in login validation.
- Plaintext password comparison: Line 300 compares passwords in plaintext. Should verify against hashed passwords.
- Timing attack risk: Using
COUNT(*)with both username and password in the WHERE clause can leak information through timing differences between "user doesn't exist" and "wrong password" scenarios.
Better approach after implementing password hashing:
private boolean validateLogin(String jdbcUrl, String dbUser, String dbPass, String username, String password) {
String sqlQuery = "SELECT password FROM account WHERE name = ?";
try (
Connection databaseConnection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass);
PreparedStatement preparedStatement = databaseConnection.prepareStatement(sqlQuery)
) {
preparedStatement.setString(1, username);
ResultSet resultSet = preparedStatement.executeQuery();
if (resultSet.next()) {
String hashedPassword = resultSet.getString("password");
return BCrypt.checkpw(password, hashedPassword);
}
} catch (SQLException e) {
System.err.println("Fel vid validering av inloggning: " + e.getMessage());
}
return false;
}This fetches the hash first, then verifies, providing more consistent timing.
🤖 Prompt for AI Agents
In src/main/java/com/example/Main.java around lines 292 to 312, the current
login validation uses COUNT(*) with plaintext password comparison which leaks
information and is insecure; change it to SELECT the stored password hash by
username, then verify the supplied password using a secure password-hashing
library (e.g., BCrypt.checkpw), avoid using COUNT(*) for auth checks, and to
mitigate timing attacks always perform a hash verification even when the user is
not found (use a fixed/dummy hash) so timing is consistent; keep using
PreparedStatement to avoid SQL injection and ensure exceptions are logged
appropriately.
1 participant
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.