Conversation
GitHub Classroom: Sync Assignment
…tes; update Main class methods accordingly
…nection handling with `SimpleDriverManagerDataSource` implementation
…nection handling with `SimpleDriverManagerDataSource` implementation
WalkthroughAdds JDBC-backed data access interfaces and implementations (DataSource, AccountRepository, MoonMissionRepository, SimpleDriverManagerDataSource, JdbcAccountRepository, JdbcMoonMissionRepository), a MoonMission record, and refactors Main to use repositories with an interactive console menu and account CRUD/login flows. README gets a badge line. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Main
participant DataSource as DataSource\n(SimpleDriverManager)
participant AcctRepo as AccountRepository\n(Jdbc)
participant DB as Database
User->>Main: Start app / enter credentials
Main->>AcctRepo: isValidLogin(username, password)
AcctRepo->>DataSource: getConnection()
DataSource->>DB: DriverManager.getConnection()
DB->>AcctRepo: Connection
AcctRepo->>DB: SELECT COUNT(*) FROM accounts WHERE name=? AND password=?
DB->>AcctRepo: count result
AcctRepo->>Main: boolean (valid/invalid)
Main->>User: Login result / show menu
sequenceDiagram
participant User
participant Main
participant MissionRepo as MoonMissionRepository\n(Jdbc)
participant DB as Database
opt List Missions
User->>Main: Choose "List Missions"
Main->>MissionRepo: findAllSpacecraftNames()
MissionRepo->>DB: SELECT spacecraft FROM missions ORDER BY launch_date
DB->>MissionRepo: names list
MissionRepo->>Main: List<String>
Main->>User: Print names
end
opt Get Mission by ID
User->>Main: Choose "Get Mission by ID"
Main->>MissionRepo: findById(id)
MissionRepo->>DB: SELECT * FROM missions WHERE mission_id=?
DB->>MissionRepo: row / empty
MissionRepo->>Main: Optional<MoonMission>
Main->>User: Print details / not found
end
opt Count by Year
User->>Main: Choose "Count by Year"
Main->>MissionRepo: countByYear(year)
MissionRepo->>DB: SELECT COUNT(*) WHERE YEAR(launch_date)=?
DB->>MissionRepo: count
MissionRepo->>Main: int
Main->>User: Print count
end
opt Account CRUD
User->>Main: Choose "Create/Update/Delete Account"
Main->>AcctRepo: createAccount/updatePassword/deleteAccount(...)
AcctRepo->>DB: INSERT/UPDATE/DELETE
DB->>AcctRepo: rows affected
AcctRepo->>Main: success/failure
Main->>User: Print result
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)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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
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)
37-40: Remove dead code.This try-with-resources block creates a connection but doesn't use it. It appears to be leftover test code.
Apply this diff:
- try (Connection connection = DriverManager.getConnection(jdbcUrl, dbUser, dbPass)) { - } catch (SQLException e) { - throw new RuntimeException(e); - } - //Todo: Starting point for your code
🧹 Nitpick comments (6)
src/main/java/com/example/MoonMission.java (1)
8-8: Fix inconsistent spacing.There's inconsistent indentation before
LocalDatecompared to other fields.Apply this diff:
- LocalDate launch_date, + LocalDate launch_date,src/main/java/com/example/SimpleDriverManagerDataSource.java (1)
13-17: Consider adding null checks.While not critical, validating that
url,user, andpasswordare non-null would provide clearer error messages if misconfigured.Example:
public SimpleDriverManagerDataSource(String url, String user, String password) { + if (url == null || user == null || password == null) { + throw new IllegalArgumentException("URL, user, and password must not be null"); + } this.url = url; this.user = user; this.password = password; }src/main/java/com/example/Main.java (1)
60-80: Improve login flow user experience.After printing "Invalid username or password" on Line 72, the code immediately reads input on Line 73 without prompting the user. The user won't understand they need to press
0to exit or any other key to retry.Consider adding a prompt:
} else{ System.out.println(" Invalid username or password. "); + System.out.print("Press 0 to exit or Enter to try again: "); String choice= scanner.nextLine().trim();src/main/java/com/example/JdbcAccountRepository.java (3)
1-3: Consider avoidingjava.sql.*wildcard importIf your course or team style prefers explicit imports, replace
java.sql.*with the specific types you use (Connection,PreparedStatement,ResultSet,SQLException) for clarity and to avoid namespace collisions.
5-14: Nice constructor injection; optionally add a defensive null-checkUsing constructor injection with a
finalDataSourceis solid. If there’s any chancenullcould be passed (e.g., in tests or future wiring), consider a simple guard likeObjects.requireNonNull(dataSource, "dataSource")to fail fast instead of NPEing later.
63-79: KeepupdatePasswordconsistent with your password storage strategyOnce you switch to hashed passwords in
createAccount, make sureupdatePasswordalso hashesnewPasswordbefore persisting so the table stays consistent (no mixed plaintext/hashed rows).You may also want to log without the leading space in the message (
"Could not update password: ...") for cleaner output, but that’s purely cosmetic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)src/main/java/com/example/AccountRepository.java(1 hunks)src/main/java/com/example/DataSource.java(1 hunks)src/main/java/com/example/JdbcAccountRepository.java(1 hunks)src/main/java/com/example/JdbcMoonMissionRepository.java(1 hunks)src/main/java/com/example/Main.java(2 hunks)src/main/java/com/example/MoonMission.java(1 hunks)src/main/java/com/example/MoonMissionRepository.java(1 hunks)src/main/java/com/example/SimpleDriverManagerDataSource.java(1 hunks)
🔇 Additional comments (9)
README.md (1)
1-1: LGTM!The badge addition is appropriate for tracking the assignment deadline.
src/main/java/com/example/DataSource.java (1)
1-11: LGTM!Clean abstraction for database connections that enables dependency injection and makes it easy to swap implementations (e.g., upgrading to a connection pool later).
src/main/java/com/example/AccountRepository.java (1)
1-12: LGTM!Clean repository interface following the Repository pattern. The boolean return values appropriately indicate operation success/failure.
src/main/java/com/example/MoonMissionRepository.java (1)
1-18: LGTM!Well-designed repository interface with appropriate use of
Optionalfor thefindByIdmethod to express the possibility of a missing result.src/main/java/com/example/SimpleDriverManagerDataSource.java (1)
20-26: LGTM!The implementation correctly delegates to
DriverManager.getConnection(). While this creates a new connection on each call without pooling, it's appropriate for this assignment scope.src/main/java/com/example/Main.java (1)
43-56: LGTM!Excellent dependency injection setup. The DataSource and repositories are properly instantiated and injected, following the VG requirement for the Repository pattern.
src/main/java/com/example/JdbcMoonMissionRepository.java (2)
20-40: LGTM!The
findAllSpacecraftNames()method correctly queries and collects spacecraft names with proper resource management using try-with-resources.
73-93: LGTM!The
countByYear()method correctly uses a parameterized query with theYEAR()function and handles the case where no results are found by returning 0.src/main/java/com/example/JdbcAccountRepository.java (1)
82-99: Delete implementation looks straightforward and correct
DELETEbyuser_idwithrowsAffected > 0as the success condition and error logging onSQLExceptionis a reasonable and clear implementation for this repository method.
| public boolean createAccount(String firstName, String lastName, String ssn, String password) { | ||
| String sql= "INSERT INTO account (first_name, last_name, ssn, password) VALUES (?, ?, ?, ?)"; | ||
|
|
||
| try(Connection connection= dataSource.getConnection(); | ||
| PreparedStatement ps= connection.prepareStatement(sql)){ | ||
|
|
||
| ps.setString(1, firstName); | ||
| ps.setString(2, lastName); | ||
| ps.setString(3, ssn); | ||
| ps.setString(4, password); | ||
|
|
||
| int rowsAffected= ps.executeUpdate(); | ||
| return rowsAffected>0; | ||
| } catch (SQLException e){ | ||
| System.err.println("Could not create account: " + e.getMessage()); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Avoid storing plaintext passwords (and be careful with SSNs)
createAccount currently writes the raw password (and ssn) straight into the database. In any real-world scenario this is unsafe:
- Passwords should be hashed with a strong, slow algorithm (e.g., bcrypt/argon2/PBKDF2) and never stored in plaintext.
- SSN is highly sensitive; it should only be stored if absolutely required, ideally encrypted or otherwise protected and tightly access-controlled.
For the purposes of the lab this may be acceptable, but if you evolve this code toward anything real you should introduce password hashing and revisit how/if SSNs are stored.
🤖 Prompt for AI Agents
In src/main/java/com/example/JdbcAccountRepository.java around lines 43 to 60,
the method stores plaintext passwords (and raw SSNs) into the DB; replace this
by hashing the password with a strong, slow algorithm (e.g.,
BCrypt/Argon2/PBKDF2) before calling the INSERT and never log or persist the raw
password, and either avoid storing SSNs or encrypt them using a vetted
library/DB encryption (or at minimum store only a hashed/peppered
representation); specifically, integrate a chosen crypto library to generate a
salted hash for the password and persist that hash instead of the plain text,
update the SQL/DB schema if needed to hold the hash, and if SSNs must be kept,
apply application-level encryption or tokenization and restrict access to the
encrypted value.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/example/JdbcAccountRepository.java (1)
44-67: Plaintext password and SSN storage remains a security concern.As noted in previous reviews, this method stores passwords and SSNs in plaintext. While acceptable for a lab exercise, this would be a critical security flaw in production.
For production, you would need to:
- Hash passwords using bcrypt, argon2, or PBKDF2
- Encrypt or avoid storing SSNs
- Update the login query to compare hashed passwords
Based on the past review comment, this is understood to be a lab limitation.
🧹 Nitpick comments (7)
src/main/java/com/example/SimpleDriverManagerDataSource.java (1)
16-20: Consider adding null-validation for constructor parameters.The constructor accepts
url,user, andpasswordwithout validation. If any are null or empty, the error will only surface later whengetConnection()is called, making debugging harder.Apply this diff to add validation:
public SimpleDriverManagerDataSource(String url, String user, String password) { + if (url == null || url.trim().isEmpty()) { + throw new IllegalArgumentException("JDBC URL cannot be null or empty"); + } + if (user == null || password == null) { + throw new IllegalArgumentException("Database credentials cannot be null"); + } this.url = url; this.user = user; this.password = password; }src/main/java/com/example/Main.java (2)
60-80: Consider adding retry limits and note password visibility.The login loop has no maximum attempt limit, which could theoretically allow unlimited brute-force attempts. Also,
Scanner.nextLine()for password input echoes characters to the console.For a lab this is acceptable, but in production you'd want:
- A maximum retry count before lockout/delay
- Use
Console.readPassword()to mask password input (requires running outside an IDE)
164-181: Consider adding input validation.The
createAccountmethod doesn't validate that inputs are non-empty. Users could create accounts with blank names or passwords. Adding simple validation would improve data quality:Example validation:
if (firstName.trim().isEmpty() || lastName.trim().isEmpty() || ssn.trim().isEmpty() || password.trim().isEmpty()) { System.out.println("All fields are required and cannot be empty."); return; }src/main/java/com/example/JdbcAccountRepository.java (4)
12-14: Consider validating the DataSource parameter.The constructor doesn't check if
dataSourceis null. Adding a null check would fail fast if misconfigured.Apply this diff:
public JdbcAccountRepository(DataSource dataSource){ + if (dataSource == null) { + throw new IllegalArgumentException("DataSource cannot be null"); + } this.dataSource=dataSource; }
47-48: Username generation may produce empty or duplicate usernames.The username is generated from the first 3 characters of each name. Consider these edge cases:
- If firstName or lastName is shorter than 3 chars, substring will use what's available (fine with
Math.min)- If firstName or lastName is empty, the username could be very short or empty
- Multiple users with similar names could get the same username (e.g., "AnnJoh")
For a lab this is probably acceptable, but consider either:
- Adding validation to reject empty names before username generation
- Using a unique identifier (like SSN or auto-increment ID) as username
- Checking for username uniqueness before insert
70-86: No validation for new password.The method doesn't check if
newPasswordis null or empty. A user could accidentally set a blank password. Consider adding validation:if (newPassword == null || newPassword.trim().isEmpty()) { System.err.println("Password cannot be empty"); return false; }Note: This method also perpetuates the plaintext password storage issue mentioned in
createAccount.
89-106: Minor: Extra space in SQL string.Line 90 has a leading space in the SQL string
" DELETE FROM...". While this doesn't affect functionality, it's inconsistent with other SQL strings in the class.Apply this diff:
-String sql= " DELETE FROM account WHERE user_id = ?"; +String sql= "DELETE FROM account WHERE user_id = ?";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/JdbcAccountRepository.java(1 hunks)src/main/java/com/example/JdbcMoonMissionRepository.java(1 hunks)src/main/java/com/example/Main.java(2 hunks)src/main/java/com/example/SimpleDriverManagerDataSource.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/example/JdbcMoonMissionRepository.java
🔇 Additional comments (7)
src/main/java/com/example/SimpleDriverManagerDataSource.java (1)
23-29: LGTM - Simple and correct DataSource implementation.The method correctly delegates to
DriverManager.getConnection()and returns a new connection each time. For a lab exercise, this non-pooled approach is acceptable.src/main/java/com/example/Main.java (5)
43-56: LGTM - Clean dependency injection and resource management.The DataSource and repositories are properly initialized with constructor injection, and the Scanner is correctly closed after use.
97-113: Good - previous unnecessary scanner.nextLine() has been removed.The catch block now correctly logs errors without attempting to consume extra input. The generic
Exceptioncatch is acceptable here since each menu method handles its own specific errors.
119-123: LGTM - Clean implementation.The method correctly fetches and displays all spacecraft names using a concise forEach pattern.
125-149: LGTM - Proper use of Optional and exception handling.The method correctly handles invalid input with
NumberFormatExceptionand usesOptional.isPresent()to check for mission existence.
151-162: LGTM - Straightforward year-based query.The method correctly parses the year input and handles format exceptions appropriately.
src/main/java/com/example/JdbcAccountRepository.java (1)
18-41: Login query now consistent with account creation.The previous issue where the login query used the
namecolumn whilecreateAccountdidn't populate it has been resolved. ThecreateAccountmethod (lines 47-59) now generates and stores a username in thenamecolumn.However, the plaintext password comparison remains a security concern (see separate comment on
createAccount).
…n class mission details output
1 participant
Labb 3: Fullständig implementering av CLI. Jag har implementerat VG-kravet med DataSource och Repository Pattern.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.