Code Review Asked by forhadmethun on February 1, 2021
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.
.
├── 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
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());
}
}
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.
Answered by gervais.b on February 1, 2021
Get help from others!
Recent Answers
Recent Questions
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP