Refactor: Constructor injection, proper exceptions, @Transactional, input validation, SecurityConfig cleanup#174
Open
devin-ai-integration[bot] wants to merge 4 commits intoDevOpsfrom
Conversation
- Replace field @Autowired with constructor injection in SecurityConfig, BankController, and AccountService - Fix loadUserByUsername(): remove dead null check, query repository directly and throw UsernameNotFoundException - Replace generic RuntimeException with ResponseStatusException (NOT_FOUND, CONFLICT, BAD_REQUEST) throughout AccountService - Add @transactional to deposit(), withdraw(), transferAmount(), and registerAccount() - Add input validation: positive amount checks for deposit/withdraw/transfer, non-blank checks for username/password in registration - Simplify SecurityConfig: remove legacy configureGlobal() method, let Spring Security auto-discover UserDetailsService bean, remove unnecessary AccountService dependency Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
Devin Review
|
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…usException ResponseStatusException.getMessage() in Spring 6.x returns a formatted string like '400 BAD_REQUEST "Insufficient funds"' including the status code prefix. Extract the clean reason phrase via getReason() so users see only the human- readable message. Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
The deposit endpoint was missing error handling for the newly added amount validation in AccountService.deposit(). Without it, invalid amounts would show a Whitelabel Error Page instead of a user-friendly error on the dashboard. Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
The register template had a hardcoded 'User already present.' message that was shown for any registration error. With new validation errors (blank username/password), the hardcoded text was misleading. Now uses th:text to display the actual error reason from the controller. Co-Authored-By: Arjun Mishra <arjunsaxmishra@gmail.com>
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
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
Refactors the application architecture and fixes code quality issues across
SecurityConfig,BankController,AccountService, and one minimal template fix, without changing endpoints or build tooling.Changes
Constructor injection — Replaced all field
@Autowiredwith constructor injection inSecurityConfig,BankController, andAccountService. Fields are nowprivate final.loadUserByUsername()cleanup — Removed the deadif (account == null)check (the precedingfindAccountByUsernamealready throws on not-found). Now queriesaccountRepositorydirectly and throwsUsernameNotFoundException.Proper exceptions — Replaced generic
RuntimeExceptionwithResponseStatusException:NOT_FOUNDfor "Account not found" and "Recipient account not found"CONFLICTfor "Username already exists"BAD_REQUESTfor "Insufficient funds" and validation failuresgetReason()(notgetMessage()) to extract clean error text for UI display@Transactional— Added todeposit(),withdraw(),transferAmount(), andregisterAccount()to ensure atomicity (especiallytransferAmountwhich does multiple DB writes).Input validation — Deposit/withdraw/transfer amounts validated as positive (
> 0). Username and password validated as non-blank during registration.SecurityConfig simplification — Removed the legacy
configureGlobal(AuthenticationManagerBuilder)method. SinceAccountServiceimplementsUserDetailsServiceand is a@Service, Spring Security auto-discovers it. Removed theAccountServicedependency fromSecurityConfigentirely. ChangedpasswordEncoder()fromstaticto instance method.Consistent error handling in controller — Added try-catch to the deposit endpoint (was missing, unlike withdraw/transfer). Updated register template to use
th:text="${error}"instead of hardcoded "User already present." so new validation errors display correctly.Review & Testing Checklist for Human
UserDetailsService; Spring auto-discovers it viaAccountService @ServiceNotes
pom.xmlor testsregister.htmlupdated to show dynamic error messages (required to match new validation behavior)./mvnw clean compile -DskipTestsLink to Devin session: https://app.devin.ai/sessions/cb1fa0ebeb6f4536a023113b2ec924c6
Requested by: @Colhodm