diff --git a/src/main/java/com/devexperts/account/Account.java b/src/main/java/com/devexperts/account/Account.java index fb2a3af..988cfaa 100644 --- a/src/main/java/com/devexperts/account/Account.java +++ b/src/main/java/com/devexperts/account/Account.java @@ -1,12 +1,14 @@ package com.devexperts.account; +import java.math.BigDecimal; + public class Account { private final AccountKey accountKey; private final String firstName; private final String lastName; - private Double balance; + private BigDecimal balance; - public Account(AccountKey accountKey, String firstName, String lastName, Double balance) { + public Account(AccountKey accountKey, String firstName, String lastName, BigDecimal balance) { this.accountKey = accountKey; this.firstName = firstName; this.lastName = lastName; @@ -25,11 +27,11 @@ public String getLastName() { return lastName; } - public Double getBalance() { + public BigDecimal getBalance() { return balance; } - public void setBalance(Double balance) { + public void setBalance(BigDecimal balance) { this.balance = balance; } } diff --git a/src/main/java/com/devexperts/account/AccountKey.java b/src/main/java/com/devexperts/account/AccountKey.java index 1b0a233..afdde3b 100644 --- a/src/main/java/com/devexperts/account/AccountKey.java +++ b/src/main/java/com/devexperts/account/AccountKey.java @@ -1,5 +1,7 @@ package com.devexperts.account; +import java.util.Objects; + /** * Unique Account identifier * @@ -17,4 +19,25 @@ private AccountKey(long accountId) { public static AccountKey valueOf(long accountId) { return new AccountKey(accountId); } + + public long getAccountId() { + return accountId; + } + + public void setAccountId(long accountId) { + accountId = this.accountId; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + AccountKey that = (AccountKey) o; + return accountId == that.accountId; + } + + @Override + public int hashCode() { + return Objects.hash(accountId); + } } diff --git a/src/main/java/com/devexperts/rest/AbstractAccountController.java b/src/main/java/com/devexperts/rest/AbstractAccountController.java index dea5a3c..5737c45 100644 --- a/src/main/java/com/devexperts/rest/AbstractAccountController.java +++ b/src/main/java/com/devexperts/rest/AbstractAccountController.java @@ -2,6 +2,8 @@ import org.springframework.http.ResponseEntity; +import java.math.BigDecimal; + public abstract class AbstractAccountController { - abstract ResponseEntity transfer(long sourceId, long targetId, double amount); + abstract ResponseEntity transfer(Long sourceId, Long targetId, BigDecimal amount); } diff --git a/src/main/java/com/devexperts/rest/AccountController.java b/src/main/java/com/devexperts/rest/AccountController.java index b300282..b5f6218 100644 --- a/src/main/java/com/devexperts/rest/AccountController.java +++ b/src/main/java/com/devexperts/rest/AccountController.java @@ -1,14 +1,45 @@ package com.devexperts.rest; +import com.devexperts.account.Account; +import com.devexperts.service.AccountService; +import com.devexperts.service.exceptions.AccountNotFoundException; +import com.devexperts.service.exceptions.InsufficientBalanceException; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RestController; +import org.springframework.ui.Model; +import org.springframework.web.bind.annotation.*; + +import java.math.BigDecimal; @RestController @RequestMapping("/api") public class AccountController extends AbstractAccountController { + private final AccountService accountService; + + @Autowired + public AccountController(AccountService accountService) { + this.accountService = accountService; + } + + @GetMapping("/operations/transfer") + public ResponseEntity transfer( + @RequestParam("sourceId") Long sourceId, + @RequestParam("targetId") Long targetId, + @RequestParam("amount") BigDecimal amount) { - public ResponseEntity transfer(long sourceId, long targetId, double amount) { - return null; + if (sourceId == null || targetId == null || amount == null || amount.compareTo(BigDecimal.ZERO) < 0) { + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); // wrong parameters + } + try { + Account source = accountService.getAccount(sourceId); + Account target = accountService.getAccount(targetId); + accountService.transfer(source,target,amount); + return new ResponseEntity<>(HttpStatus.OK); // successful transfer + } catch (AccountNotFoundException e) { + return new ResponseEntity<>(HttpStatus.NOT_FOUND); // account is not found + } catch (InsufficientBalanceException e) { + return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); // insufficient account balance + } } } diff --git a/src/main/java/com/devexperts/service/AccountService.java b/src/main/java/com/devexperts/service/AccountService.java index f287597..8760ecc 100644 --- a/src/main/java/com/devexperts/service/AccountService.java +++ b/src/main/java/com/devexperts/service/AccountService.java @@ -2,6 +2,8 @@ import com.devexperts.account.Account; +import java.math.BigDecimal; + public interface AccountService { /** @@ -33,5 +35,7 @@ public interface AccountService { * @param target account to transfer money to * @param amount dollar amount to transfer * */ - void transfer(Account source, Account target, double amount); + void transfer(Account source, Account target, BigDecimal amount); + + void validateTransferParams(Account source, Account target, BigDecimal amount); } diff --git a/src/main/java/com/devexperts/service/AccountServiceImpl.java b/src/main/java/com/devexperts/service/AccountServiceImpl.java index 91261ba..9e662ec 100644 --- a/src/main/java/com/devexperts/service/AccountServiceImpl.java +++ b/src/main/java/com/devexperts/service/AccountServiceImpl.java @@ -2,15 +2,20 @@ import com.devexperts.account.Account; import com.devexperts.account.AccountKey; +import com.devexperts.service.exceptions.AccountNotFoundException; +import com.devexperts.service.exceptions.InsufficientBalanceException; import org.springframework.stereotype.Service; -import java.util.ArrayList; -import java.util.List; +import java.math.BigDecimal; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +// Double is not recommended for financial operations. +// Change to BigDecimal @Service public class AccountServiceImpl implements AccountService { - - private final List accounts = new ArrayList<>(); + // change List to Map to improve the performance of the getAccount() method + private final Map accounts = new ConcurrentHashMap<>(); @Override public void clear() { @@ -19,19 +24,62 @@ public void clear() { @Override public void createAccount(Account account) { - accounts.add(account); + if (account == null) { + throw new NullPointerException("ACCOUNT EQUALS TO NULL."); + } + if (accounts.containsKey(account.getAccountKey())) { + throw new IllegalArgumentException("ACCOUNT WITH SUCH A KEY ALREADY EXISTS."); + } + accounts.put(account.getAccountKey(),account); } @Override public Account getAccount(long id) { - return accounts.stream() - .filter(account -> account.getAccountKey() == AccountKey.valueOf(id)) - .findAny() - .orElse(null); + if (accounts.containsKey(AccountKey.valueOf(id))) { + return accounts.get(AccountKey.valueOf(id)); + } + return null; + } + + // use the nested synchronized blocks to make possible simultaneous + // transactions between different accounts (ex. A -> B, C -> D) + @Override + public void transfer(Account source, Account target, BigDecimal amount) { + validateTransferParams(source,target,amount); + + // sort the accounts by id to avoid deadlock (ex. A -> B, B -> A) + Object firstLock, secondLock; + if (source.getAccountKey().getAccountId() > target.getAccountKey().getAccountId()) { + firstLock = source; + secondLock = target; + } else { + firstLock = target; + secondLock = source; + } + synchronized (firstLock) { + synchronized (secondLock) { + source.setBalance(source.getBalance().subtract(amount)); + target.setBalance(target.getBalance().add(amount)); + } + } } @Override - public void transfer(Account source, Account target, double amount) { - //do nothing for now + public void validateTransferParams(Account source, Account target, BigDecimal amount) { + if (source == null || target == null) { + throw new AccountNotFoundException("SOURCE OR TARGET ACCOUNT DOES NOT EXIST."); + } + if (!accounts.containsKey(source.getAccountKey()) || !accounts.containsKey(target.getAccountKey())) { + throw new AccountNotFoundException("SOURCE OR TARGET ACCOUNT DOES NOT EXIST."); + } + if (source == target) { + throw new IllegalArgumentException("SOURCE AND TARGET ACCOUNTS CANNOT BE EQUAL."); + } + if (source.getBalance().compareTo(BigDecimal.ZERO) == 0 || source.getBalance().compareTo(amount) < 0) { + throw new InsufficientBalanceException("INSUFFICIENT SOURCE ACCOUNT BALANCE"); + } + if (amount.compareTo(BigDecimal.ZERO) < 0) { + throw new IllegalArgumentException("AMOUNT CANNOT BE LESS THAN 0"); + } } } diff --git a/src/main/java/com/devexperts/service/exceptions/AccountNotFoundException.java b/src/main/java/com/devexperts/service/exceptions/AccountNotFoundException.java new file mode 100644 index 0000000..e0a217b --- /dev/null +++ b/src/main/java/com/devexperts/service/exceptions/AccountNotFoundException.java @@ -0,0 +1,7 @@ +package com.devexperts.service.exceptions; + +public class AccountNotFoundException extends RuntimeException { + public AccountNotFoundException(String message) { + super(message); + } +} diff --git a/src/main/java/com/devexperts/service/exceptions/InsufficientBalanceException.java b/src/main/java/com/devexperts/service/exceptions/InsufficientBalanceException.java new file mode 100644 index 0000000..ad252ed --- /dev/null +++ b/src/main/java/com/devexperts/service/exceptions/InsufficientBalanceException.java @@ -0,0 +1,7 @@ +package com.devexperts.service.exceptions; + +public class InsufficientBalanceException extends RuntimeException { + public InsufficientBalanceException(String message) { + super(message); + } +} diff --git a/src/main/resources/data/accounts.sql b/src/main/resources/data/accounts.sql new file mode 100644 index 0000000..aca5c5e --- /dev/null +++ b/src/main/resources/data/accounts.sql @@ -0,0 +1,7 @@ +CREATE TABLE IF NOT EXISTS Accounts +( + ID BIGINT AUTO_INCREMENT PRIMARY, + FIRST_NAME VARCHAR(255) NOT NULL, + LAST_NAME VARCHAR(255) NOT NULL, + BALANCE BIGINT NOT NULL + ); diff --git a/src/main/resources/data/transfers.sql b/src/main/resources/data/transfers.sql new file mode 100644 index 0000000..11499a9 --- /dev/null +++ b/src/main/resources/data/transfers.sql @@ -0,0 +1,10 @@ +CREATE TABLE IF NOT EXISTS Transfers +( + ID BIGINT AUTO_INCREMENT PRIMARY, + SOURCE_ID BIGINT NOT NULL, + TARGET_ID BIGINT NOT NULL, + AMOUNT BIGINT NOT NULL, + TRANSFER_TIME TIMESTAMP NOT NULL, + FOREIGN KEY (SOURCE_ID) REFERENCES accounts (ID), + FOREIGN KEY (TARGET_ID) REFERENCES accounts (ID) + ); diff --git a/src/main/resources/select.sql b/src/main/resources/select.sql new file mode 100644 index 0000000..5404163 --- /dev/null +++ b/src/main/resources/select.sql @@ -0,0 +1,4 @@ +SELECT t.source_id FROM Transfers AS t +WHERE t.transfer_time >= TIMESTAMP '2019-01-01' AND source_id != target_id AND amount > 0 +GROUP BY source_id +HAVING SUM(amount) > 1000; diff --git a/src/test/java/com/devexperts/rest/AccountControllerTest.java b/src/test/java/com/devexperts/rest/AccountControllerTest.java new file mode 100644 index 0000000..cbdd4b8 --- /dev/null +++ b/src/test/java/com/devexperts/rest/AccountControllerTest.java @@ -0,0 +1,96 @@ +package com.devexperts.rest; + +import com.devexperts.ApplicationRunner; +import com.devexperts.account.Account; +import com.devexperts.account.AccountKey; +import com.devexperts.service.AccountService; +import com.devexperts.service.AccountServiceImpl; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; +import org.springframework.test.web.servlet.result.MockMvcResultMatchers; + +import java.math.BigDecimal; + + +@RunWith(SpringRunner.class) +@ContextConfiguration(classes = ApplicationRunner.class) +@WebMvcTest(AccountController.class) +public class AccountControllerTest { + + @Autowired + private MockMvc mockMvc; + + private AccountService accountService; + private Account source; + private Account target; + + @Before + public void setUp() { + accountService = new AccountServiceImpl(); + + source = new Account(AccountKey.valueOf(1), "IVAN", "IVANOV", new BigDecimal("100.0")); + target = new Account(AccountKey.valueOf(2), "PETER", "PETROV", new BigDecimal("200.0")); + accountService.createAccount(source); + accountService.createAccount(target); + } + + @Test + public void shouldReturn200_ifTransferSuccessful() throws Exception { + MockHttpServletRequestBuilder builder = MockMvcRequestBuilders + .get("/api/operations/transfer") + .param("sourceId","1") + .param("targetId","2") + .param("amount","50.0"); + + mockMvc.perform(builder) + .andExpect(MockMvcResultMatchers.status().isOk()); + + Assert.assertEquals(source.getBalance(), new BigDecimal("50.0")); + Assert.assertEquals(target.getBalance(), new BigDecimal("250.0")); + } + + @Test + public void shouldReturn404_ifAccountNotFound() throws Exception { + MockHttpServletRequestBuilder builder = MockMvcRequestBuilders + .get("/api/operations/transfer") + .param("sourceId", "8") + .param("targetId", "7") + .param("amount", "50.0"); + + mockMvc.perform(builder) + .andExpect(MockMvcResultMatchers.status().isNotFound()); + } + + @Test + public void shouldReturn400_ifParamsInvalid() throws Exception { + MockHttpServletRequestBuilder builder = MockMvcRequestBuilders + .get("/api/operations/transfer") + .param("sourceId", "1") + .param("targetId", "2") + .param("amount", "-50.0"); + + mockMvc.perform(builder) + .andExpect(MockMvcResultMatchers.status().isBadRequest()); + } + + @Test + public void shouldReturn500_ifInsufficientBalance() throws Exception { + MockHttpServletRequestBuilder builder = MockMvcRequestBuilders + .get("/api/operations/transfer") + .param("sourceId", "1") + .param("targetId", "2") + .param("amount", "1000.0"); + + mockMvc.perform(builder) + .andExpect(MockMvcResultMatchers.status().isInternalServerError()); + } +} diff --git a/src/test/java/com/devexperts/service/AccountServiceImplTest.java b/src/test/java/com/devexperts/service/AccountServiceImplTest.java new file mode 100644 index 0000000..51955e2 --- /dev/null +++ b/src/test/java/com/devexperts/service/AccountServiceImplTest.java @@ -0,0 +1,63 @@ +package com.devexperts.service; + +import com.devexperts.account.Account; +import com.devexperts.account.AccountKey; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.math.BigDecimal; + +public class AccountServiceImplTest { + private AccountService accountService; + private Account account; + + @Before + public void setUp() { + accountService = new AccountServiceImpl(); + + account = new Account(AccountKey.valueOf(1), "IVAN", "IVANOV", new BigDecimal("100.0")); + accountService.createAccount(account); + } + + /* AccountServiceImp.clear() */ + @Test + public void shouldReturnNull_ifCollectionIsEmpty() { + Account expected = accountService.getAccount(account.getAccountKey().getAccountId()); + Assert.assertNotNull(expected); + + accountService.clear(); + expected = accountService.getAccount(account.getAccountKey().getAccountId()); + Assert.assertNull(expected); + } + + /* AccountServiceImp.createAccount() */ + @Test + public void shouldReturnTrue_ifNewAccountCreated() { + Account expected = accountService.getAccount(account.getAccountKey().getAccountId()); + Assert.assertEquals(account,expected); + } + + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerException_ifAccountIsNull() { + accountService.createAccount(null); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldThrowIllegalArgumentException_ifAccountAlreadyExists() { + accountService.createAccount(account); + } + + /* AccountServiceImp.getAccount() */ + @Test + public void shouldReturnTrue_ifAccountExists() { + Account expected = accountService.getAccount(account.getAccountKey().getAccountId()); + Assert.assertNotNull(expected); + } + + @Test + public void shouldReturnNull_ifAccountNotFound() { + Account expected = accountService.getAccount(2); + Assert.assertNull(expected); + } +} diff --git a/src/test/java/com/devexperts/service/TransferTest.java b/src/test/java/com/devexperts/service/TransferTest.java new file mode 100644 index 0000000..ea21203 --- /dev/null +++ b/src/test/java/com/devexperts/service/TransferTest.java @@ -0,0 +1,73 @@ +package com.devexperts.service; + +import com.devexperts.account.Account; +import com.devexperts.account.AccountKey; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.math.BigDecimal; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +public class TransferTest { + private AccountService accountService; + private Account source; + private Account target; + + @Before + public void setUp() { + accountService = new AccountServiceImpl(); + + source = new Account(AccountKey.valueOf(1), "IVAN", "IVANOV", new BigDecimal("100.0")); + target = new Account(AccountKey.valueOf(2), "PETER", "PETROV", new BigDecimal("200.0")); + accountService.createAccount(source); + accountService.createAccount(target); + } + + // single-thread environment + @Test + public void shouldReturnTrue_ifSingleThreadTransferIsPossible() { + accountService.transfer(source, target, new BigDecimal("50.0")); + Assert.assertEquals(source.getBalance(), new BigDecimal("50.0")); + Assert.assertEquals(target.getBalance(), new BigDecimal("250.0")); + } + + // multi-threads environment + @Test + public void shouldReturnTrue_ifPossibleToCreateAccountsFromDifferentThreads() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(10); + // start from id=3 since accounts with id=1 and id=2 have been already created in the setUp() method + for (int i = 3; i < 10; i++) { + int id = i; + new Thread(() -> { + Account account = new Account(AccountKey.valueOf(id), "IVAN", "IVANOV", BigDecimal.ZERO); + accountService.createAccount(account); + countDownLatch.countDown(); + }).start(); + } + countDownLatch.await(1, TimeUnit.SECONDS); + + for (int i = 3; i < 10; i++) { + Account account = accountService.getAccount(i); + Assert.assertNotNull(account); + Assert.assertEquals(i, account.getAccountKey().getAccountId()); + } + } + + @Test + public void shouldReturnTrue_ifMultiThreadsTransferIsPossible() throws InterruptedException { + CountDownLatch countDownLatch = new CountDownLatch(10); + Runnable transaction = () -> { + accountService.transfer(source,target,new BigDecimal("10.0")); + countDownLatch.countDown(); + }; + for (int i = 0; i < 10; i++) { + new Thread(transaction).start(); + } + countDownLatch.await(1, TimeUnit.SECONDS); + + Assert.assertEquals(source.getBalance(),BigDecimal.ZERO); + Assert.assertEquals(target.getBalance(),new BigDecimal("300.0")); + } +} diff --git a/src/test/java/com/devexperts/service/ValidateTransferParamsTest.java b/src/test/java/com/devexperts/service/ValidateTransferParamsTest.java new file mode 100644 index 0000000..2169d6f --- /dev/null +++ b/src/test/java/com/devexperts/service/ValidateTransferParamsTest.java @@ -0,0 +1,69 @@ +package com.devexperts.service; + +import com.devexperts.account.Account; +import com.devexperts.account.AccountKey; +import com.devexperts.service.exceptions.InsufficientBalanceException; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.math.BigDecimal; + +public class ValidateTransferParamsTest { + private AccountService accountService; + private Account source; + private Account target; + + @Before + public void setUp() { + accountService = new AccountServiceImpl(); + + source = new Account(AccountKey.valueOf(1), "IVAN", "IVANOV", new BigDecimal("100.0")); + target = new Account(AccountKey.valueOf(2), "PETER", "PETROV", new BigDecimal("200.0")); + accountService.createAccount(source); + accountService.createAccount(target); + } + + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerException_ifSourceIsNull() { + accountService.validateTransferParams(null,target,new BigDecimal("50.0")); + } + + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerException_ifTargetIsNull() { + accountService.validateTransferParams(source,null,new BigDecimal("50.0")); + } + + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerException_ifSourceNotExists() { + source = new Account(AccountKey.valueOf(3), "MARK", "MARKOV", new BigDecimal("10.0")); + accountService.validateTransferParams(source,target,new BigDecimal("50.0")); + } + + @Test(expected = NullPointerException.class) + public void shouldThrowNullPointerException_ifTargetNotExists() { + source = new Account(AccountKey.valueOf(3), "MARK", "MARKOV", new BigDecimal("10.0")); + accountService.validateTransferParams(source,target,new BigDecimal("50.0")); + } + + @Test + public void shouldReturnTrue_ifAccountsEqual() { + Assert.assertNotEquals(source,target); + } + + @Test(expected = InsufficientBalanceException.class) + public void shouldThrowIllegalArgumentException_ifSourceBalanceIsInsufficient_1() { + source.setBalance(BigDecimal.ZERO); + accountService.validateTransferParams(source,target,new BigDecimal("50.0")); + } + + @Test(expected = InsufficientBalanceException.class) + public void shouldThrowIllegalArgumentException_ifSourceBalanceIsInsufficient_2() { + accountService.validateTransferParams(source,target,new BigDecimal("150.0")); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldThrowIllegalArgumentException_ifAmountLessThanZero () { + accountService.validateTransferParams(source,target,new BigDecimal("-50.0")); + } +}