applying code review suggestions:

- simplifying solution and removing not necessary things
- fixing places not following clean code principles (methods with more
than 3 arguments)
- removing not necessary unit tests (ApplicationTest)
- checking balance of the accounts in integration (REST API) test after
committing the transaction
- handling corner case with transfering negative amount of money

README.md

@@ -1,8 +1,6 @@
 # money-transfer-api [![Build Status](https://img.shields.io/travis/pwittchen/money-transfer-api.svg?branch=master&style=flat-square)](https://travis-ci.org/pwittchen/money-transfer-api) [![codecov](https://img.shields.io/codecov/c/github/pwittchen/money-transfer-api/master.svg?style=flat-square&label=coverage)](https://codecov.io/gh/pwittchen/money-transfer-api/branch/master)
 
-HTTP server with REST API for money transfer between bank accounts
-
-Description of the task can be found in [TASK.md](https://github.com/pwittchen/money-transfer-api/blob/master/TASK.md) file.
+HTTP server with REST API for money transfer between bank accounts. Description of the task can be found in [TASK.md](https://github.com/pwittchen/money-transfer-api/blob/master/TASK.md) file.
 
 Contents
 --------

src/main/java/com/pwittchen/money/transfer/api/Application.java

@@ -15,7 +15,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static io.javalin.apibuilder.ApiBuilder.delete;
 import static io.javalin.apibuilder.ApiBuilder.get;
 import static io.javalin.apibuilder.ApiBuilder.path;
 import static io.javalin.apibuilder.ApiBuilder.post;
@@ -69,16 +68,11 @@ public static void main(String args[]) {
 
     app.routes(() -> {
       path("/account", () -> {
-        path(":id", () -> {
-          get(accountController::getOne);
-          delete(accountController::delete);
-        });
         get(accountController::getAll);
         post(accountController::create);
       });
 
       path("/transaction", () -> {
-        path(":id", () -> get(transactionController::getOne));
         get(transactionController::getAll);
         post(transactionController::commit);
       });

src/main/java/com/pwittchen/money/transfer/api/command/exception/DifferentCurrencyException.java

@@ -1,6 +1,7 @@
 package com.pwittchen.money.transfer.api.command.exception;
 
 public class DifferentCurrencyException extends RuntimeException {
+
   private final String accountNumberFrom;
   private final String accountNumberTo;
 

src/main/java/com/pwittchen/money/transfer/api/command/exception/EmptyAccountNumberException.java

@@ -1,6 +1,7 @@
 package com.pwittchen.money.transfer.api.command.exception;
 
 public class EmptyAccountNumberException extends RuntimeException {
+
   @Override public String getMessage() {
     return "Account number is empty";
   }

src/main/java/com/pwittchen/money/transfer/api/command/exception/EmptyAccountOwnerException.java

@@ -1,6 +1,7 @@
 package com.pwittchen.money.transfer.api.command.exception;
 
 public class EmptyAccountOwnerException extends RuntimeException {
+
   @Override public String getMessage() {
     return "Account owner is empty";
   }

src/main/java/com/pwittchen/money/transfer/api/command/exception/NegativeMoneyValueException.java

@@ -0,0 +1,7 @@
+package com.pwittchen.money.transfer.api.command.exception;
+
+public class NegativeMoneyValueException extends RuntimeException {
+  @Override public String getMessage() {
+    return "Money could not be negative";
+  }
+}

src/main/java/com/pwittchen/money/transfer/api/command/exception/NotEnoughMoneyException.java

@@ -1,6 +1,7 @@
 package com.pwittchen.money.transfer.api.command.exception;
 
 public class NotEnoughMoneyException extends RuntimeException {
+
   private final String accountNumber;
 
   public NotEnoughMoneyException(final String accountNumber) {

src/main/java/com/pwittchen/money/transfer/api/command/exception/TransferToTheSameAccountException.java

@@ -1,6 +1,7 @@
 package com.pwittchen.money.transfer.api.command.exception;
 
 public class TransferToTheSameAccountException extends RuntimeException {
+
   @Override public String getMessage() {
     return "Sender and receiver account numbers have to be different";
   }

src/main/java/com/pwittchen/money/transfer/api/command/implementation/DefaultCommitTransactionCommand.java

@@ -3,6 +3,7 @@
 import com.pwittchen.money.transfer.api.command.CommitTransactionCommand;
 import com.pwittchen.money.transfer.api.command.exception.AccountNotExistsException;
 import com.pwittchen.money.transfer.api.command.exception.DifferentCurrencyException;
+import com.pwittchen.money.transfer.api.command.exception.NegativeMoneyValueException;
 import com.pwittchen.money.transfer.api.command.exception.NotEnoughMoneyException;
 import com.pwittchen.money.transfer.api.command.exception.TransferToTheSameAccountException;
 import com.pwittchen.money.transfer.api.model.Account;
@@ -60,9 +61,12 @@ public class DefaultCommitTransactionCommand implements CommitTransactionCommand
   }
 
   private void validateTransaction(Transaction transaction) {
-    Account sender = getSender(transaction.from());
-    Account receiver = getReceiver(transaction.to());
+    final Account sender = getSender(transaction.from());
+    final Account receiver = getReceiver(transaction.to());
 
+    if (transaction.money().isNegative()) {
+      throw new NegativeMoneyValueException();
+    }
     if (sender.money().isLessThan(transaction.money())) {
       throw new NotEnoughMoneyException(sender.number());
     }

src/main/java/com/pwittchen/money/transfer/api/command/implementation/DefaultCreateAccountCommand.java

@@ -4,6 +4,7 @@
 import com.pwittchen.money.transfer.api.command.exception.AccountAlreadyExistsException;
 import com.pwittchen.money.transfer.api.command.exception.EmptyAccountNumberException;
 import com.pwittchen.money.transfer.api.command.exception.EmptyAccountOwnerException;
+import com.pwittchen.money.transfer.api.command.exception.NegativeMoneyValueException;
 import com.pwittchen.money.transfer.api.model.Account;
 import com.pwittchen.money.transfer.api.repository.AccountRepository;
 import javax.inject.Inject;
@@ -26,6 +27,10 @@ private void validateAccount(Account account) {
       throw new EmptyAccountNumberException();
     }
 
+    if (account.money().isNegative()) {
+      throw new NegativeMoneyValueException();
+    }
+
     if (accountRepository
         .getAll()
         .stream()

src/main/java/com/pwittchen/money/transfer/api/configuration/module/CommandModule.java

@@ -2,10 +2,8 @@
 
 import com.pwittchen.money.transfer.api.command.CommitTransactionCommand;
 import com.pwittchen.money.transfer.api.command.CreateAccountCommand;
-import com.pwittchen.money.transfer.api.command.DeleteAccountCommand;
 import com.pwittchen.money.transfer.api.command.implementation.DefaultCommitTransactionCommand;
 import com.pwittchen.money.transfer.api.command.implementation.DefaultCreateAccountCommand;
-import com.pwittchen.money.transfer.api.command.implementation.DefaultDeleteAccountCommand;
 import com.pwittchen.money.transfer.api.repository.AccountRepository;
 import com.pwittchen.money.transfer.api.repository.TransactionRepository;
 import dagger.Module;
@@ -31,11 +29,4 @@ CommitTransactionCommand provideCommitTransactionCommand(
   CreateAccountCommand provideCreateAccountCommand(AccountRepository accountRepository) {
     return new DefaultCreateAccountCommand(accountRepository);
   }
-
-  @Inject
-  @Provides
-  @Singleton
-  DeleteAccountCommand provideDeleteAccountCommand(AccountRepository accountRepository) {
-    return new DefaultDeleteAccountCommand(accountRepository);
-  }
 }

src/main/java/com/pwittchen/money/transfer/api/configuration/module/ControllerModule.java

@@ -2,15 +2,12 @@
 
 import com.pwittchen.money.transfer.api.command.CommitTransactionCommand;
 import com.pwittchen.money.transfer.api.command.CreateAccountCommand;
-import com.pwittchen.money.transfer.api.command.DeleteAccountCommand;
 import com.pwittchen.money.transfer.api.controller.AccountController;
 import com.pwittchen.money.transfer.api.controller.TransactionController;
 import com.pwittchen.money.transfer.api.controller.context.ContextWrapper;
 import com.pwittchen.money.transfer.api.controller.context.DefaultContextWrapper;
-import com.pwittchen.money.transfer.api.query.GetAccountQuery;
 import com.pwittchen.money.transfer.api.query.GetAllAccountsQuery;
 import com.pwittchen.money.transfer.api.query.GetAllTransactionsQuery;
-import com.pwittchen.money.transfer.api.query.GetTransactionQuery;
 import dagger.Module;
 import dagger.Provides;
 import javax.inject.Inject;
@@ -30,17 +27,13 @@ ContextWrapper provideContextWrapper() {
   @Singleton
   AccountController provideAccountController(
       final ContextWrapper contextWrapper,
-      final GetAccountQuery getAccountQuery,
       final GetAllAccountsQuery getAllAccountsQuery,
-      final CreateAccountCommand createAccountCommand,
-      final DeleteAccountCommand deleteAccountCommand
+      final CreateAccountCommand createAccountCommand
   ) {
     return new AccountController(
         contextWrapper,
-        getAccountQuery,
         getAllAccountsQuery,
-        createAccountCommand,
-        deleteAccountCommand
+        createAccountCommand
     );
   }
 
@@ -49,13 +42,11 @@ AccountController provideAccountController(
   @Singleton
   TransactionController provideTransactionController(
       final ContextWrapper contextWrapper,
-      final GetTransactionQuery getTransactionQuery,
       final GetAllTransactionsQuery getAllTransactionsQuery,
       final CommitTransactionCommand commitTransactionCommand
   ) {
     return new TransactionController(
         contextWrapper,
-        getTransactionQuery,
         getAllTransactionsQuery,
         commitTransactionCommand
     );

src/main/java/com/pwittchen/money/transfer/api/configuration/module/QueryModule.java

@@ -1,13 +1,9 @@
 package com.pwittchen.money.transfer.api.configuration.module;
 
-import com.pwittchen.money.transfer.api.query.GetAccountQuery;
 import com.pwittchen.money.transfer.api.query.GetAllAccountsQuery;
 import com.pwittchen.money.transfer.api.query.GetAllTransactionsQuery;
-import com.pwittchen.money.transfer.api.query.GetTransactionQuery;
-import com.pwittchen.money.transfer.api.query.implementation.DefaultGetAccountQuery;
 import com.pwittchen.money.transfer.api.query.implementation.DefaultGetAllAccountsQuery;
 import com.pwittchen.money.transfer.api.query.implementation.DefaultGetAllTransactionsQuery;
-import com.pwittchen.money.transfer.api.query.implementation.DefaultGetTransactionQuery;
 import com.pwittchen.money.transfer.api.repository.AccountRepository;
 import com.pwittchen.money.transfer.api.repository.TransactionRepository;
 import dagger.Module;
@@ -18,13 +14,6 @@
 @Module
 public class QueryModule {
 
-  @Inject
-  @Provides
-  @Singleton
-  GetAccountQuery provideGetAccountQuery(AccountRepository accountRepository) {
-    return new DefaultGetAccountQuery(accountRepository);
-  }
-
   @Inject
   @Provides
   @Singleton
@@ -35,15 +24,7 @@ GetAllAccountsQuery provideGetAllAccountsQuery(AccountRepository accountReposito
   @Inject
   @Provides
   @Singleton
-  GetTransactionQuery provideGetTransactionQuery(TransactionRepository transactionRepository) {
-    return new DefaultGetTransactionQuery(transactionRepository);
-  }
-
-  @Inject
-  @Provides
-  @Singleton
-  GetAllTransactionsQuery provideGetAllTransactionsQuery(
-      TransactionRepository transactionRepository) {
+  GetAllTransactionsQuery provideGetTransactionQuery(TransactionRepository transactionRepository) {
     return new DefaultGetAllTransactionsQuery(transactionRepository);
   }
 }

src/main/java/com/pwittchen/money/transfer/api/controller/AccountController.java

@@ -1,10 +1,8 @@
 package com.pwittchen.money.transfer.api.controller;
 
 import com.pwittchen.money.transfer.api.command.CreateAccountCommand;
-import com.pwittchen.money.transfer.api.command.DeleteAccountCommand;
 import com.pwittchen.money.transfer.api.controller.context.ContextWrapper;
 import com.pwittchen.money.transfer.api.model.Account;
-import com.pwittchen.money.transfer.api.query.GetAccountQuery;
 import com.pwittchen.money.transfer.api.query.GetAllAccountsQuery;
 import io.javalin.http.Context;
 import io.javalin.plugin.openapi.annotations.HttpMethod;
@@ -22,48 +20,17 @@
 public class AccountController {
 
   private ContextWrapper contextWrapper;
-  private GetAccountQuery getAccountQuery;
   private GetAllAccountsQuery getAllAccountsQuery;
   private CreateAccountCommand createAccountCommand;
-  private DeleteAccountCommand deleteAccountCommand;
 
   @Inject public AccountController(
       final ContextWrapper contextWrapper,
-      final GetAccountQuery getAccountQuery,
       final GetAllAccountsQuery getAllAccountsQuery,
-      final CreateAccountCommand createAccountCommand,
-      final DeleteAccountCommand deleteAccountCommand
+      final CreateAccountCommand createAccountCommand
   ) {
     this.contextWrapper = contextWrapper;
-    this.getAccountQuery = getAccountQuery;
     this.getAllAccountsQuery = getAllAccountsQuery;
     this.createAccountCommand = createAccountCommand;
-    this.deleteAccountCommand = deleteAccountCommand;
-  }
-
-  @OpenApi(
-      method = HttpMethod.GET,
-      path = "/account/:id",
-      description = "gets a single account with a given id",
-      pathParams = @OpenApiParam(name = "id", type = Integer.class),
-      responses = {
-          @OpenApiResponse(status = "200", content = @OpenApiContent(from = Account.class)),
-          @OpenApiResponse(status = "404", content = @OpenApiContent(from = String.class))
-      }
-  )
-  public void getOne(Context context) {
-    Optional<Account> account = getAccountQuery.run(contextWrapper.pathParam(context, "id"));
-
-    if (account.isPresent()) {
-      contextWrapper.json(context, account.get(), HttpStatus.OK_200);
-    } else {
-      String message = String.format(
-          "account with id %s does not exist",
-          contextWrapper.pathParam(context, "id")
-      );
-
-      contextWrapper.json(context, message, HttpStatus.NOT_FOUND_404);
-    }
   }
 
   @OpenApi(
@@ -94,8 +61,7 @@ public void getAll(final Context context) {
       }
   )
   public void create(final Context context) {
-    Optional<Account> account = createAccount(context);
-
+    final Optional<Account> account = createAccount(context);
     if (account.isEmpty()) {
       contextWrapper.json(context, "Invalid money format", HttpStatus.BAD_REQUEST_400);
       return;
@@ -130,29 +96,4 @@ private Optional<Money> parseMoney(Context context) {
       return Optional.empty();
     }
   }
-
-  @OpenApi(
-      method = HttpMethod.DELETE,
-      path = "/account/:id",
-      description = "deletes an account with a given id",
-      pathParams = @OpenApiParam(name = "id", type = Integer.class),
-      responses = {
-          @OpenApiResponse(status = "200", content = @OpenApiContent(from = String.class)),
-          @OpenApiResponse(status = "400", content = @OpenApiContent(from = String.class))
-      }
-  )
-  public void delete(Context context) {
-    try {
-      deleteAccountCommand.run(contextWrapper.pathParam(context, "id"));
-
-      String message = String.format(
-          "account with number %s deleted",
-          contextWrapper.pathParam(context, "id")
-      );
-
-      contextWrapper.json(context, message, HttpStatus.OK_200);
-    } catch (Exception exception) {
-      contextWrapper.json(context, exception.getMessage(), HttpStatus.BAD_REQUEST_400);
-    }
-  }
 }