Refactor: Constructor injection, proper exceptions, @Transactional, input validation, SecurityConfig cleanup

src/main/java/com/example/bankapp/config/SecurityConfig.java

@@ -1,10 +1,7 @@
 package com.example.bankapp.config;
 
-import com.example.bankapp.service.AccountService;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
@@ -16,11 +13,8 @@
 @EnableWebSecurity
 public class SecurityConfig {
 
-    @Autowired
-    AccountService accountService;
-
     @Bean
-    public static PasswordEncoder passwordEncoder() {
+    public PasswordEncoder passwordEncoder() {
         return new BCryptPasswordEncoder();
     }
 
@@ -51,10 +45,4 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
 
         return http.build();
     }
-
-    @Autowired
-    public void configureGlobal(AuthenticationManagerBuilder auth) throws Exception {
-        auth.userDetailsService(accountService).passwordEncoder(passwordEncoder());
-
-    }
-}
+}

src/main/java/com/example/bankapp/controller/BankController.java

@@ -2,8 +2,8 @@
 
 import com.example.bankapp.model.Account;
 import com.example.bankapp.service.AccountService;
-import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.web.server.ResponseStatusException;
 import org.springframework.stereotype.Controller;
 import org.springframework.ui.Model;
 import org.springframework.web.bind.annotation.GetMapping;
@@ -15,8 +15,11 @@
 @Controller
 public class BankController {
 
-    @Autowired
-    private AccountService accountService;
+    private final AccountService accountService;
+
+    public BankController(AccountService accountService) {
+        this.accountService = accountService;
+    }
 
     @GetMapping("/dashboard")
     public String dashboard(Model model) {
@@ -37,7 +40,7 @@ public String registerAccount(@RequestParam String username, @RequestParam Strin
             accountService.registerAccount(username, password);
             return "redirect:/login";
         } catch (RuntimeException e) {
-            model.addAttribute("error", e.getMessage());
+            model.addAttribute("error", extractErrorMessage(e));
             return "register";
         }
     }
@@ -48,10 +51,18 @@ public String login() {
     }
 
     @PostMapping("/deposit")
-    public String deposit(@RequestParam BigDecimal amount) {
+    public String deposit(@RequestParam BigDecimal amount, Model model) {
         String username = SecurityContextHolder.getContext().getAuthentication().getName();
         Account account = accountService.findAccountByUsername(username);
-        accountService.deposit(account, amount);
+
+        try {
+            accountService.deposit(account, amount);
+        } catch (RuntimeException e) {
+            model.addAttribute("error", extractErrorMessage(e));
+            model.addAttribute("account", account);
+            return "dashboard";
+        }
+
         return "redirect:/dashboard";
     }
 
@@ -63,7 +74,7 @@ public String withdraw(@RequestParam BigDecimal amount, Model model) {
         try {
             accountService.withdraw(account, amount);
         } catch (RuntimeException e) {
-            model.addAttribute("error", e.getMessage());
+            model.addAttribute("error", extractErrorMessage(e));
             model.addAttribute("account", account);
             return "dashboard";
         }
@@ -79,6 +90,13 @@ public String transactionHistory(Model model) {
         return "transactions";
     }
 
+    private String extractErrorMessage(RuntimeException e) {
+        if (e instanceof ResponseStatusException rse) {
+            return rse.getReason();
+        }
+        return e.getMessage();
+    }
+
     @PostMapping("/transfer")
     public String transferAmount(@RequestParam String toUsername, @RequestParam BigDecimal amount, Model model) {
         String username = SecurityContextHolder.getContext().getAuthentication().getName();
@@ -87,12 +105,11 @@ public String transferAmount(@RequestParam String toUsername, @RequestParam BigD
         try {
             accountService.transferAmount(fromAccount, toUsername, amount);
         } catch (RuntimeException e) {
-            model.addAttribute("error", e.getMessage());
+            model.addAttribute("error", extractErrorMessage(e));
             model.addAttribute("account", fromAccount);
             return "dashboard";
         }
 
         return "redirect:/dashboard";
     }
-
 }

src/main/java/com/example/bankapp/service/AccountService.java

@@ -4,14 +4,16 @@
 import com.example.bankapp.model.Transaction;
 import com.example.bankapp.repository.AccountRepository;
 import com.example.bankapp.repository.TransactionRepository;
-import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpStatus;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.authority.SimpleGrantedAuthority;
 import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.security.core.userdetails.UserDetailsService;
 import org.springframework.security.core.userdetails.UsernameNotFoundException;
 import org.springframework.security.crypto.password.PasswordEncoder;
 import org.springframework.stereotype.Service;
+import org.springframework.transaction.annotation.Transactional;
+import org.springframework.web.server.ResponseStatusException;
 
 import java.math.BigDecimal;
 import java.time.LocalDateTime;
@@ -22,33 +24,47 @@
 @Service
 public class AccountService implements UserDetailsService {
 
-    @Autowired
-    PasswordEncoder passwordEncoder;
+    private final PasswordEncoder passwordEncoder;
+    private final AccountRepository accountRepository;
+    private final TransactionRepository transactionRepository;
 
-    @Autowired
-    private AccountRepository accountRepository;
-
-    @Autowired
-    private TransactionRepository transactionRepository;
+    public AccountService(PasswordEncoder passwordEncoder,
+                          AccountRepository accountRepository,
+                          TransactionRepository transactionRepository) {
+        this.passwordEncoder = passwordEncoder;
+        this.accountRepository = accountRepository;
+        this.transactionRepository = transactionRepository;
+    }
 
     public Account findAccountByUsername(String username) {
-        return accountRepository.findByUsername(username).orElseThrow(() -> new RuntimeException("Account not found"));
+        return accountRepository.findByUsername(username)
+                .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Account not found"));
     }
 
+    @Transactional
     public Account registerAccount(String username, String password) {
+        if (username == null || username.isBlank()) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Username must not be blank");
+        }
+        if (password == null || password.isBlank()) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Password must not be blank");
+        }
         if (accountRepository.findByUsername(username).isPresent()) {
-            throw new RuntimeException("Username already exists");
+            throw new ResponseStatusException(HttpStatus.CONFLICT, "Username already exists");
         }
 
         Account account = new Account();
         account.setUsername(username);
-        account.setPassword(passwordEncoder.encode(password)); // Encrypt password
-        account.setBalance(BigDecimal.ZERO); // Initial balance set to 0
+        account.setPassword(passwordEncoder.encode(password));
+        account.setBalance(BigDecimal.ZERO);
         return accountRepository.save(account);
     }
 
-
+    @Transactional
     public void deposit(Account account, BigDecimal amount) {
+        if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Deposit amount must be positive");
+        }
         account.setBalance(account.getBalance().add(amount));
         accountRepository.save(account);
 
@@ -61,9 +77,13 @@ public void deposit(Account account, BigDecimal amount) {
         transactionRepository.save(transaction);
     }
 
+    @Transactional
     public void withdraw(Account account, BigDecimal amount) {
+        if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Withdrawal amount must be positive");
+        }
         if (account.getBalance().compareTo(amount) < 0) {
-            throw new RuntimeException("Insufficient funds");
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Insufficient funds");
         }
         account.setBalance(account.getBalance().subtract(amount));
         accountRepository.save(account);
@@ -83,11 +103,8 @@ public List<Transaction> getTransactionHistory(Account account) {
 
     @Override
     public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
-
-        Account account = findAccountByUsername(username);
-        if (account == null) {
-            throw new UsernameNotFoundException("Username or Password not found");
-        }
+        Account account = accountRepository.findByUsername(username)
+                .orElseThrow(() -> new UsernameNotFoundException("Username or Password not found"));
         return new Account(
                 account.getUsername(),
                 account.getPassword(),
@@ -100,23 +117,24 @@ public Collection<? extends GrantedAuthority> authorities() {
         return Arrays.asList(new SimpleGrantedAuthority("USER"));
     }
 
+    @Transactional
     public void transferAmount(Account fromAccount, String toUsername, BigDecimal amount) {
+        if (amount == null || amount.compareTo(BigDecimal.ZERO) <= 0) {
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Transfer amount must be positive");
+        }
         if (fromAccount.getBalance().compareTo(amount) < 0) {
-            throw new RuntimeException("Insufficient funds");
+            throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "Insufficient funds");
         }
 
         Account toAccount = accountRepository.findByUsername(toUsername)
-                .orElseThrow(() -> new RuntimeException("Recipient account not found"));
+                .orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "Recipient account not found"));
 
-        // Deduct from sender's account
         fromAccount.setBalance(fromAccount.getBalance().subtract(amount));
         accountRepository.save(fromAccount);
 
-        // Add to recipient's account
         toAccount.setBalance(toAccount.getBalance().add(amount));
         accountRepository.save(toAccount);
 
-        // Create transaction records for both accounts
         Transaction debitTransaction = new Transaction(
                 amount,
                 "Transfer Out to " + toAccount.getUsername(),
@@ -133,5 +151,4 @@ public void transferAmount(Account fromAccount, String toUsername, BigDecimal am
         );
         transactionRepository.save(creditTransaction);
     }
-
 }

src/main/resources/templates/register.html

@@ -118,9 +118,7 @@ <h2 class="text-left">Register a New Account</h2>
     </form>
     <p class="mt-3">Already have an account? <a href="/login" class="custom-link">Login here</a></p>
 
-    <div th:if="${error}" class="alert alert-danger mt-3 text-center">
-        User already present.
-    </div>
+    <div th:if="${error}" th:text="${error}" class="alert alert-danger mt-3 text-center"></div>
 </div>
 
 <footer class="footer">