Open
Conversation
WalkthroughThe changes introduce a data-access layer with a DataSource-backed architecture. New repository interfaces and JDBC implementations manage account and moon mission data. Main.java is refactored to use these repositories within an interactive CLI loop, replacing direct database connection logic. Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as Main<br/>(CLI Loop)
participant AcctRepo as AccountRepository
participant MissionRepo as MoonMissionRepository
participant DB as Database
User->>CLI: Start Application
CLI->>CLI: Initialize DataSource & Repositories
CLI->>User: Prompt for Authentication
User->>CLI: Enter name & password
CLI->>AcctRepo: authenticate(name, password)
AcctRepo->>DB: SELECT userId WHERE name=?
DB-->>AcctRepo: Result (userId or none)
AcctRepo-->>CLI: Optional<Long>
alt Authentication Success
CLI->>User: Show Menu
User->>CLI: Select Operation
alt List Operation
CLI->>MissionRepo: listSpacecraftNames()
MissionRepo->>DB: SELECT from moon_mission
DB-->>MissionRepo: ResultSet
MissionRepo-->>CLI: List<String>
else Get Operation
CLI->>MissionRepo: getMissionAsMap(missionId)
MissionRepo->>DB: SELECT * WHERE mission_id=?
DB-->>MissionRepo: Row data
MissionRepo-->>CLI: Optional<Map>
else Count Operation
CLI->>MissionRepo: countByYear(year)
MissionRepo->>DB: SELECT * from moon_mission
DB-->>MissionRepo: All rows
MissionRepo-->>CLI: int (count)
else Create/Update/Delete
CLI->>AcctRepo: createAccount/updatePassword/deleteAccount
AcctRepo->>DB: INSERT/UPDATE/DELETE
DB-->>AcctRepo: Result
end
CLI->>User: Display Results
CLI->>User: Prompt for Next Operation
else Authentication Failed
CLI->>User: Access Denied
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Key areas requiring careful attention:
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: 4
🧹 Nitpick comments (5)
src/main/java/com/example/db/SimpleDriverManagerDataSource.java (1)
10-39: PreferSQLFeatureNotSupportedException(orSQLException) overUnsupportedOperationExceptioninDataSourcestubs.
This keeps behavior aligned with JDBC expectations and avoids surprising runtime failures in libs that probe these methods.import javax.sql.DataSource; import java.io.PrintWriter; import java.sql.Connection; import java.sql.DriverManager; import java.sql.SQLException; +import java.sql.SQLFeatureNotSupportedException; import java.util.logging.Logger; ... - @Override public PrintWriter getLogWriter() { throw new UnsupportedOperationException(); } - @Override public void setLogWriter(PrintWriter out) { throw new UnsupportedOperationException(); } - @Override public void setLoginTimeout(int seconds) { throw new UnsupportedOperationException(); } + @Override public PrintWriter getLogWriter() throws SQLException { throw new SQLFeatureNotSupportedException(); } + @Override public void setLogWriter(PrintWriter out) throws SQLException { throw new SQLFeatureNotSupportedException(); } + @Override public void setLoginTimeout(int seconds) throws SQLException { throw new SQLFeatureNotSupportedException(); } @Override public int getLoginTimeout() { return 0; } - @Override public Logger getParentLogger() { throw new UnsupportedOperationException(); } - @Override public <T> T unwrap(Class<T> iface) { throw new UnsupportedOperationException(); } + @Override public Logger getParentLogger() throws SQLFeatureNotSupportedException { throw new SQLFeatureNotSupportedException(); } + @Override public <T> T unwrap(Class<T> iface) throws SQLException { throw new SQLFeatureNotSupportedException(); } @Override public boolean isWrapperFor(Class<?> iface) { return false; }src/main/java/com/example/repo/JdbcAccountRepository.java (1)
62-85: CheckexecuteUpdate()row counts for update/delete and signal “not found”.
Right nowupdatePassword/deleteAccountcan report success even whenuser_iddoesn’t exist.public void updatePassword(long userId, String newPassword) { String sql = "UPDATE account SET password = ? WHERE user_id = ?"; try (Connection c = ds.getConnection(); PreparedStatement ps = c.prepareStatement(sql)) { ps.setString(1, newPassword); ps.setLong(2, userId); - ps.executeUpdate(); + int n = ps.executeUpdate(); + if (n == 0) throw new IllegalArgumentException("No account with user_id=" + userId); } catch (SQLException e) { throw new RuntimeException(e); } } ... public void deleteAccount(long userId) { String sql = "DELETE FROM account WHERE user_id = ?"; try (Connection c = ds.getConnection(); PreparedStatement ps = c.prepareStatement(sql)) { ps.setLong(1, userId); - ps.executeUpdate(); + int n = ps.executeUpdate(); + if (n == 0) throw new IllegalArgumentException("No account with user_id=" + userId); } catch (SQLException e) { throw new RuntimeException(e); } }src/main/java/com/example/repo/JdbcMoonMissionRepository.java (1)
15-70:listSpacecraftNames()column-guessing is brittle; prefer choosing by column label.
Using a small “known values” set + “first text column” fallback can break as seed data evolves. Consider selecting the column whose label matchesspacecraft/spacecraft_name/namefirst (case-insensitive), then fallback to your current heuristic.src/main/java/com/example/Main.java (2)
12-22: Avoid wrapping and rethrowing asRuntimeExceptioninmainunless required.
Right now any checked error becomes an unchecked crash; if tests don’t require this, consider printing a message + exiting non-zero instead.
143-149:format()should usejoining(avoids repeated string concatenation).private static String format(Map<String, Object> row) { return row.entrySet().stream() .sorted(Map.Entry.comparingByKey()) .map(e -> e.getKey() + "=" + e.getValue()) - .reduce((a, b) -> a + " " + b) - .orElse(""); + .collect(java.util.stream.Collectors.joining(" ")); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(1 hunks)src/main/java/com/example/Main.java(1 hunks)src/main/java/com/example/db/SimpleDriverManagerDataSource.java(1 hunks)src/main/java/com/example/repo/AccountRepository.java(1 hunks)src/main/java/com/example/repo/JdbcAccountRepository.java(1 hunks)src/main/java/com/example/repo/JdbcMoonMissionRepository.java(1 hunks)src/main/java/com/example/repo/MoonMissionRepository.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/Main.java (2)
src/main/java/com/example/db/SimpleDriverManagerDataSource.java (1)
SimpleDriverManagerDataSource(10-39)src/main/java/com/example/DevDatabaseInitializer.java (1)
DevDatabaseInitializer(6-24)
🔇 Additional comments (3)
README.md (1)
1-1: README badge addition is fine.
No concerns; this is a harmless doc/header addition.src/main/java/com/example/repo/MoonMissionRepository.java (1)
7-11: Interface is a good fit for the CLI + repository split.
Optional<Map<String,Object>>is pragmatic here and keepsMaindecoupled from JDBC.src/main/java/com/example/Main.java (1)
40-58: Login loop matches the assignment shape; exit-on-0is handled.
This looks aligned with the stated CLI contract (prompt, validate, print “invalid”, allow exit).
Comment on lines
+78
to
+139
| case "2" -> { | ||
| out.print("mission_id:"); | ||
| out.flush(); | ||
| long id = Long.parseLong(in.readLine()); | ||
| missions.getMissionAsMap(id) | ||
| .ifPresentOrElse( | ||
| m -> out.println(format(m)), | ||
| () -> out.println("not found") | ||
| ); | ||
| } | ||
| case "3" -> { | ||
| out.print("year:"); | ||
| out.flush(); | ||
| int year = Integer.parseInt(in.readLine()); | ||
| int count = missions.countByYear(year); | ||
| out.println(year + ": " + count); | ||
| } | ||
| case "4" -> { | ||
| out.print("first name:"); | ||
| out.flush(); | ||
| String first = in.readLine(); | ||
|
|
||
| out.print("last name:"); | ||
| out.flush(); | ||
| String last = in.readLine(); | ||
|
|
||
| out.print("ssn:"); | ||
| out.flush(); | ||
| String ssn = in.readLine(); | ||
|
|
||
| out.print("password:"); | ||
| out.flush(); | ||
| String pw = in.readLine(); | ||
|
|
||
| long id = accounts.createAccount(first, last, ssn, pw); | ||
| out.println("account created " + id); | ||
| } | ||
| case "5" -> { | ||
| out.print("user_id:"); | ||
| out.flush(); | ||
| long id = Long.parseLong(in.readLine()); | ||
|
|
||
| out.print("new password:"); | ||
| out.flush(); | ||
| String pw = in.readLine(); | ||
|
|
||
| accounts.updatePassword(id, pw); | ||
| out.println("password updated"); | ||
| } | ||
| case "6" -> { | ||
| out.print("user_id:"); | ||
| out.flush(); | ||
| long id = Long.parseLong(in.readLine()); | ||
|
|
||
| accounts.deleteAccount(id); | ||
| out.println("account deleted"); | ||
| } | ||
| case "0" -> { | ||
| return; | ||
| } | ||
| default -> out.println("invalid option"); | ||
| } |
There was a problem hiding this comment.
Guard readLine() + numeric parsing to avoid hard-crashing on malformed input.
Long.parseLong(in.readLine()) and Integer.parseInt(in.readLine()) will throw on null/blank/non-numeric input; consider re-prompting or printing “invalid option” / “invalid input”.
🤖 Prompt for AI Agents
In src/main/java/com/example/Main.java around lines 78-139, the code calls
in.readLine() and passes the result straight to Long.parseLong/Integer.parseInt
(and assumes non-null strings for other fields), which will throw on
null/blank/non-numeric input; change these handlers to validate inputs: check
readLine() != null and trim() is not empty, and wrap numeric parsing in
try-catch(NumberFormatException) to handle malformed numbers; for invalid or
null inputs print a clear "invalid input" (or re-prompt if you prefer) and
skip/return to the menu instead of letting the exception propagate; also
validate required string fields (first, last, ssn, password) for null/blank and
handle them the same way.
Comment on lines
+5
to
+10
| public interface AccountRepository { | ||
| Optional<Long> authenticate(String name, String password); | ||
| long createAccount(String firstName, String lastName, String ssn, String password); | ||
| void updatePassword(long userId, String newPassword); | ||
| void deleteAccount(long userId); | ||
| } |
There was a problem hiding this comment.
Repository boundary is clean, but password handling is plaintext end-to-end.
If this isn’t strictly “assignment-only”, consider hashing (e.g., BCrypt) and changing authenticate to verify hashes rather than matching password = ?.
🤖 Prompt for AI Agents
In src/main/java/com/example/repo/AccountRepository.java around lines 5–10, the
repository currently handles plaintext passwords; update the implementation so
passwords are hashed (e.g., BCrypt) before persisting and only the hash is
stored. Keep the public signatures (accept raw password on
createAccount/updatePassword and authenticate) but change the persistence logic:
on createAccount/updatePassword, hash the provided password with BCrypt and save
the hash; on authenticate, load the stored hash for the given username and
verify the raw password with BCrypt.checkpw, returning the user id only on
successful verification. Also ensure DB column length suits the hash, avoid
logging raw passwords, and handle/propagate hashing-related exceptions
appropriately.
Comment on lines
+31
to
+60
| @Override | ||
| public long createAccount(String firstName, String lastName, String ssn, String password) { | ||
| String name = makeUsername(firstName, lastName); | ||
| String sql = "INSERT INTO account (name, first_name, last_name, ssn, password) VALUES (?, ?, ?, ?, ?)"; | ||
|
|
||
| try (Connection c = ds.getConnection(); | ||
| PreparedStatement ps = c.prepareStatement(sql, Statement.RETURN_GENERATED_KEYS)) { | ||
| ps.setString(1, name); | ||
| ps.setString(2, firstName); | ||
| ps.setString(3, lastName); | ||
| ps.setString(4, ssn); | ||
| ps.setString(5, password); | ||
| ps.executeUpdate(); | ||
|
|
||
| try (ResultSet keys = ps.getGeneratedKeys()) { | ||
| if (keys.next()) return keys.getLong(1); | ||
| } | ||
|
|
||
| try (PreparedStatement ps2 = c.prepareStatement("SELECT user_id FROM account WHERE name = ?")) { | ||
| ps2.setString(1, name); | ||
| try (ResultSet rs = ps2.executeQuery()) { | ||
| if (rs.next()) return rs.getLong(1); | ||
| } | ||
| } | ||
|
|
||
| throw new IllegalStateException("Could not get new user id"); | ||
| } catch (SQLException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
createAccount() fallback lookup by name can return the wrong user if usernames collide.
If account.name isn’t guaranteed UNIQUE, SELECT user_id ... WHERE name=? is unsafe—especially with the XXX padding strategy. Consider either (a) enforcing uniqueness at DB level, or (b) removing the fallback and failing if no generated key is returned.
- try (PreparedStatement ps2 = c.prepareStatement("SELECT user_id FROM account WHERE name = ?")) {
- ps2.setString(1, name);
- try (ResultSet rs = ps2.executeQuery()) {
- if (rs.next()) return rs.getLong(1);
- }
- }
-
- throw new IllegalStateException("Could not get new user id");
+ throw new IllegalStateException("Could not get generated user id");🤖 Prompt for AI Agents
In src/main/java/com/example/repo/JdbcAccountRepository.java around lines 31 to
60, the fallback SELECT by name can return the wrong user when usernames
collide; remove the unsafe fallback and fail when no generated key is returned
(or alternatively enforce a UNIQUE constraint on account.name at the database
level and ensure makeUsername produces unique values). Update the method to rely
on getGeneratedKeys only and throw a clear exception if it is absent, or if you
choose DB-side uniqueness, add the unique constraint/migration and handle the
SQLIntegrityConstraintViolationException to surface a meaningful
duplicate-username error.
Comment on lines
+93
to
+125
| @Override | ||
| public int countByYear(int year) { | ||
| try (Connection c = ds.getConnection(); | ||
| PreparedStatement ps = c.prepareStatement("SELECT * FROM moon_mission"); | ||
| ResultSet rs = ps.executeQuery()) { | ||
|
|
||
| ResultSetMetaData md = rs.getMetaData(); | ||
| int cols = md.getColumnCount(); | ||
| int count = 0; | ||
|
|
||
| while (rs.next()) { | ||
| for (int i = 1; i <= cols; i++) { | ||
| Object v = rs.getObject(i); | ||
| if (v instanceof java.sql.Date d && d.toLocalDate().getYear() == year) { | ||
| count++; | ||
| break; | ||
| } | ||
| if (v instanceof java.sql.Timestamp t && t.toLocalDateTime().getYear() == year) { | ||
| count++; | ||
| break; | ||
| } | ||
| if (v instanceof Integer y && y == year) { | ||
| count++; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| return count; | ||
|
|
||
| } catch (SQLException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
countByYear() can overcount because it matches the year in any column.
If any integer/date column happens to equal year (including unrelated fields), the row is counted. Prefer counting against the actual launch/date/year column in SQL (e.g., WHERE YEAR(launch_date)=?) or at least choose the date/year column by label/type once and only inspect that column.
🤖 Prompt for AI Agents
In src/main/java/com/example/repo/JdbcMoonMissionRepository.java around lines
93-125, the current implementation iterates every column and counts a row if any
column matches the year, causing false positives; change the logic to target the
actual mission date column only. Fix by updating the SQL to filter in the DB
(preferred) e.g., use a PreparedStatement with a WHERE clause that tests the
launch/date column year (use YEAR(launch_date)=? or EXTRACT(YEAR FROM
launch_date)=? depending on your DB) and set the year parameter, or if you must
filter in Java, only select the specific date column (e.g., SELECT launch_date
FROM moon_mission) and inspect that single column’s value to compute the year
before incrementing the counter.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.