Kudriavtseva Olga: Application pull request

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;
     }
 }

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);
+    }
 }

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<Void> transfer(long sourceId, long targetId, double amount);
+    abstract ResponseEntity<Void> transfer(Long sourceId, Long targetId, BigDecimal amount);
 }

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<Void> transfer(
+            @RequestParam("sourceId") Long sourceId,
+            @RequestParam("targetId") Long targetId,
+            @RequestParam("amount") BigDecimal amount) {
 
-    public ResponseEntity<Void> 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
+        }
     }
 }

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);
 }

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<Account> accounts = new ArrayList<>();
+    // change List to Map to improve the performance of the getAccount() method
+    private final Map<AccountKey, Account> 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");
+        }
     }
 }

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);
+    }
+}

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);
+    }
+}

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
+    );

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)
+    );

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;

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());
+    }
+}