0
\$\begingroup\$

I wrote two microservices in Springboot and tried to follow the best practices. I would appreciate any suggestions about any improvement that I can make on the whole codebase. What are the parts of the code that I can improve on the whole project?

Project Link: https://github.com/forhadmethun/mbank

Technology used:

Java 11+
SpringBoot
JPA / Hibernate
MyBatis
Gradle
Postgres
RabbitMQ
JUnit

Thank you very much.

Directory Structure

.
├── main
│   ├── java
│   │   └── com
│   │       └── forhadmethun
│   │           └── accountservice
│   │               ├── config
│   │               │   └── RabbitMQConfiguration.java
│   │               ├── controller
│   │               │   ├── AccountController.java
│   │               │   ├── ExceptionController.java
│   │               │   ├── HealthController.java
│   │               │   └── TransactionController.java
│   │               ├── db
│   │               │   ├── entity
│   │               │   │   ├── Account.java
│   │               │   │   ├── Balance.java
│   │               │   │   ├── Customer.java
│   │               │   │   └── Transaction.java
│   │               │   ├── repository
│   │               │   │   ├── AccountCommandRepository.java
│   │               │   │   ├── AccountQueryRepository.java
│   │               │   │   ├── BalanceCommandRepository.java
│   │               │   │   ├── BalanceQueryRepository.java
│   │               │   │   ├── CustomerRepository.java
│   │               │   │   ├── TransactionQueryRepository.java
│   │               │   │   └── TransactionRepository.java
│   │               │   └── services
│   │               │       ├── AccountService.java
│   │               │       ├── BalanceService.java
│   │               │       ├── bean
│   │               │       │   ├── AccountServiceBean.java
│   │               │       │   ├── BalanceServiceBean.java
│   │               │       │   ├── CustomerServiceBean.java
│   │               │       │   ├── MessageQueueBean.java
│   │               │       │   └── TransactionServiceBean.java
│   │               │       ├── CustomerService.java
│   │               │       ├── MessageQueueService.java
│   │               │       └── TransactionService.java
│   │               ├── MbankAccountServiceApplication.java
│   │               └── utility
│   │                   ├── constant
│   │                   │   └── PersistenceConstant.java
│   │                   ├── CurrencyUtil.java
│   │                   ├── dto
│   │                   │   ├── mapper
│   │                   │   │   ├── AccountMapper.java
│   │                   │   │   ├── BalanceMapper.java
│   │                   │   │   ├── CustomerMapper.java
│   │                   │   │   └── TransactionMapper.java
│   │                   │   └── model
│   │                   │       ├── AccountDto.java
│   │                   │       ├── BalanceDto.java
│   │                   │       ├── CustomerDto.java
│   │                   │       ├── DirectionOfTransaction.java
│   │                   │       ├── mq
│   │                   │       │   └── BalanceMQDto.java
│   │                   │       └── TransactionDto.java
│   │                   ├── exception
│   │                   │   ├── PersistenceException.java
│   │                   │   └── RequestException.java
│   │                   ├── io
│   │                   │   ├── AccountOperationResponse.java
│   │                   │   ├── ErrorResponse.java
│   │                   │   └── mq
│   │                   │       ├── AccountCreationInfo.java
│   │                   │       └── TransactionCreationInfo.java
│   │                   ├── TransactionUtil.java
│   │                   └── validation
│   │                       ├── AccountServiceValidation.java
│   │                       ├── ValidationImpl.java
│   │                       ├── Validation.java
│   │                       └── ValidationResult.java
│   └── resources
│       ├── application.properties
│       ├── db
│       │   └── migration
│       │       └── V1__create_table.sql
│       ├── static
│       └── templates
└── test
    └── java
        └── com
            └── forhadmethun
                └── accountservice
                    ├── controller
                    │   ├── AccountControllerTest.java
                    │   └── TransactionControllerTest.java
                    └── MbankAccountServiceApplicationTests.java


Some of the Code(Please see the Github repo for all the files)

AccountController.java

package com.forhadmethun.accountservice.controller;

/**
 * @author Md Forhad Hossain
 * @since 01/10/20
 */

import com.forhadmethun.accountservice.db.services.AccountService;
import com.forhadmethun.accountservice.db.services.CustomerService;
import com.forhadmethun.accountservice.utility.CurrencyUtil;
import com.forhadmethun.accountservice.utility.dto.model.CustomerDto;
import com.forhadmethun.accountservice.utility.exception.PersistenceException;
import com.forhadmethun.accountservice.utility.exception.RequestException;
import com.forhadmethun.accountservice.utility.io.AccountOperationResponse;

import lombok.AllArgsConstructor;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.*;

@RestController
@AllArgsConstructor
public class AccountController {
    private final CustomerService customerService;
    private final AccountService accountService;

    @PostMapping("/accounts")
    public ResponseEntity<AccountOperationResponse> createAccount(
            @RequestBody CustomerDto customerDto
    ) throws RequestException {

        CurrencyUtil.checkCurrencyValidity(customerDto.getCurrencies());

        AccountOperationResponse accountCreationResponse =
                customerService.createCustomer(customerDto);

        return new ResponseEntity<>(accountCreationResponse, HttpStatus.OK);
    }

    @GetMapping("/accounts/{accountId}")
    public ResponseEntity<AccountOperationResponse> getAccount(
            @PathVariable Long accountId
    ) throws PersistenceException {

        AccountOperationResponse accountOperationResponse =
                accountService.findByAccountId(accountId);

        return new ResponseEntity<>(accountOperationResponse, HttpStatus.OK);
    }
}

AccountServiceBean.java

package com.forhadmethun.accountservice.db.services.bean;

/**
 * @author Md Forhad Hossain
 * @since 01/10/20
 */

import com.forhadmethun.accountservice.db.entity.Account;
import com.forhadmethun.accountservice.db.entity.Balance;
import com.forhadmethun.accountservice.db.repository.AccountQueryRepository;
import com.forhadmethun.accountservice.db.repository.AccountCommandRepository;
import com.forhadmethun.accountservice.db.repository.BalanceQueryRepository;
import com.forhadmethun.accountservice.db.services.AccountService;
import com.forhadmethun.accountservice.db.services.BalanceService;
import com.forhadmethun.accountservice.utility.constant.PersistenceConstant;
import com.forhadmethun.accountservice.utility.dto.mapper.BalanceMapper;
import com.forhadmethun.accountservice.utility.dto.model.AccountDto;
import com.forhadmethun.accountservice.utility.exception.PersistenceException;
import com.forhadmethun.accountservice.utility.io.AccountOperationResponse;

import lombok.AllArgsConstructor;
import org.springframework.stereotype.Service;

import java.util.List;
@AllArgsConstructor
@Service
public class AccountServiceBean implements AccountService {
    private final AccountCommandRepository accountCommandRepository;
    private final AccountQueryRepository accountQueryRepository;
    private final BalanceService balanceService;

    @Override
    public Account createAccount(Account account) {
        return accountCommandRepository.save(account);
    }

    @Override
    public AccountOperationResponse findByAccountId(Long accountId) throws PersistenceException {
        AccountDto account =  accountQueryRepository.findByAccountId(accountId);

        if (account == null)
            throw new PersistenceException(PersistenceConstant.ACCOUNT_NOT_FOUND);

        List<Balance> balances = balanceService.findByAccountId(accountId);

        AccountOperationResponse accountOperationResponse =
                AccountOperationResponse.createAccountCreationResponse(
                        account.getAccountId(),
                        account.getCustomerId(),
                        BalanceMapper.toBalanceDtoList(balances)
                );

        return accountOperationResponse;
    }
}

AccountControllerTest.java

package com.forhadmethun.accountservice.controller;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.forhadmethun.accountservice.db.services.CustomerService;
import com.forhadmethun.accountservice.utility.dto.model.CustomerDto;
import com.forhadmethun.accountservice.utility.io.AccountOperationResponse;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.http.MediaType;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;


import java.math.BigDecimal;
import java.util.Arrays;

import static org.hamcrest.Matchers.*;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.junit.jupiter.api.Assertions.*;

@ExtendWith(SpringExtension.class)
@SpringBootTest
@AutoConfigureMockMvc
class AccountControllerTest {

    @Autowired
    CustomerService customerService;

    @Autowired
    private MockMvc mockMvc;

    @Autowired
    private ObjectMapper objectMapper;

    @Test
    void createAccount() throws Exception {
        MvcResult mockMvcResult = mockMvc.perform(MockMvcRequestBuilders
                .post("/accounts", 42L)
                .contentType("application/json")
                .content(objectMapper.writeValueAsString(
                        CustomerDto.builder()
                                .customerId(1L)
                                .country("Bangladesh")
                                .currencies(Arrays.asList("EUR"))
                                .build()
                )))
                .andExpect(status().isOk())
                .andReturn();
        String contentAsString = mockMvcResult.getResponse().getContentAsString();
        AccountOperationResponse response = objectMapper.readValue(contentAsString, AccountOperationResponse.class);
        assertEquals(1L, response.getCustomerId());
        assertEquals(1, response.getBalances().size());
        assertEquals("EUR", response.getBalances().get(0).getCurrency());
        assertEquals(BigDecimal.ZERO, response.getBalances().get(0).getBalance());
    }

    @Test
    void getAccount() throws Exception {
        AccountOperationResponse createdAccountObject =
                customerService.createCustomer(
                        CustomerDto.builder()
                                .customerId(36L)
                                .country("Bangladesh")
                                .currencies(Arrays.asList("EUR"))
                                .build()
                );
        MvcResult mockMvcResult  =
                mockMvc.perform(get("/accounts/" + createdAccountObject.getAccountId()))
                .andExpect(content().contentType(MediaType.APPLICATION_JSON))
                .andExpect(status().isOk())
                .andExpect(jsonPath("$.balances[0].currency", is("EUR"))
                ).andReturn();
        String contentAsString = mockMvcResult.getResponse().getContentAsString();
        AccountOperationResponse response = objectMapper.readValue(contentAsString, AccountOperationResponse.class);
        assertEquals(response.getAccountId(), createdAccountObject.getAccountId());
        assertEquals(response.getCustomerId(), createdAccountObject.getCustomerId());
        assertEquals(response.getBalances().size(), createdAccountObject.getBalances().size());
        assertEquals("EUR", response.getBalances().get(0).getCurrency());
    }
}

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Thanks, @Emma for your valuable comment. Actually, I want the whole project review so it's tough to include all code in the question so shared the GitHub link. Actually, I gave some information about the deployment also in the README.md file of the git. Can you check if you get the idea that how to deploy? \$\endgroup\$ Oct 5, 2020 at 1:25
  • 1
    \$\begingroup\$ Thanks, @Emma, I have added a few codes. Hope that I'll get some review. \$\endgroup\$ Oct 5, 2020 at 13:33

1 Answer 1

2
\$\begingroup\$

Actually I did not reviewed your code as such (as warned by @Emma). But for an "architectural" perspective I see some improvements. However some are more personal guts than real good practices.

The first reflexion is about your the motivation between those two microservices. Why did you split reporting from anything else. Why did you group Account, Balance, Customer and Transaction in one service instead of creatiing dedicated services ?

The second one is about the packaging (! Personnal feeling). Are you aware of the package by feature principle ? The idea is to regroup classes by feature instead of layers, so you will have Account, AccountCommandRepository, AccountQuyeryRepository, AccountController, Customer, Balance, AccountDto, ... in the same package.

Another reflexion is about your choice to create a command and a query repositories. I assume taht your are trying to create a CQRS system. But there are much more behind that than two distinct classes.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.