Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robert test branch #39

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/main/java/com/bravo/user/controller/PaymentController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package com.bravo.user.controller;

import com.bravo.user.annotation.SwaggerController;
import com.bravo.user.model.dto.PaymentDto;
import com.bravo.user.service.PaymentService;
import com.bravo.user.validator.UserValidator;
import lombok.NonNull;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.ResponseBody;

import java.util.List;

@RequestMapping(value = "/payment")
@SwaggerController
public class PaymentController {

private final PaymentService paymentService;
private final UserValidator userValidator;

public PaymentController(PaymentService paymentService, UserValidator userValidator) {
this.paymentService = paymentService;
this.userValidator = userValidator;
}

@GetMapping(value = "/retrieve/{userId}")
@ResponseBody
public List<PaymentDto> retrieve(@NonNull final @PathVariable String userId) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What benefit does @NonNull provide here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the @nonnull annotation here to make sure that a NullPointerException would be thrown if this method is ever called without userId since userId is required for this method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tested this some more by making curl calls to the API on my terminal. It seems like even if I don't provide a userId the API just treats this as a userId with the value of EMPTYSTRING so I think you are right and this check is not needed. I think the only way userId will be null is if someone specifically passes in an encoded version of null and when I tested that, it was causing the API to throw a BadRequest exception so I think this is already handled.

userValidator.validateId(userId);
return paymentService.retrieveByUserId(userId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public <T extends Collection<Payment>> List<PaymentDto> convertPayments(final T
public PaymentDto convertPayment(final Payment payment){
final String cardNumber = payment.getCardNumber();
final PaymentDto dto = mapperFacade.map(payment, PaymentDto.class);
dto.setCardNumberLast4(cardNumber.substring(cardNumber.length() - 5));
dto.setCardNumberLast4(cardNumber.substring(cardNumber.length() - 4));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job catching this bug! 👍🏻

return dto;
}

Expand Down
13 changes: 13 additions & 0 deletions src/main/java/com/bravo/user/dao/repository/PaymentRepository.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.bravo.user.dao.repository;

import com.bravo.user.dao.model.Payment;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;

import java.util.List;

@Repository
public interface PaymentRepository extends JpaRepository<Payment, String> {

List<Payment> findPaymentByUserId(final String userId);
}
36 changes: 36 additions & 0 deletions src/main/java/com/bravo/user/service/PaymentService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.bravo.user.service;

import com.bravo.user.dao.model.Payment;
import com.bravo.user.dao.model.mapper.ResourceMapper;
import com.bravo.user.dao.repository.PaymentRepository;
import com.bravo.user.exception.DataNotFoundException;
import com.bravo.user.model.dto.PaymentDto;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Service;

import java.util.List;

@Service
public class PaymentService {

private static final Logger LOGGER = LoggerFactory.getLogger(PaymentService.class);

private final PaymentRepository paymentRepository;
private final ResourceMapper resourceMapper;

public PaymentService(PaymentRepository paymentRepository, ResourceMapper resourceMapper) {
this.paymentRepository = paymentRepository;
this.resourceMapper = resourceMapper;
}

public List<PaymentDto> retrieveByUserId(final String userId) {
final List<Payment> paymentList = paymentRepository.findPaymentByUserId(userId);
if (paymentList.isEmpty()) {
throw new DataNotFoundException(String.format("No payment methods were found for user %s", userId));
}
LOGGER.info("found {} payment method(s)", paymentList.size());

return resourceMapper.convertPayments(paymentList);
}
}
7 changes: 7 additions & 0 deletions src/main/resources/data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -648,3 +648,10 @@ insert into address (id, user_id, line1, line2, city, state, zip) values
('42f33d30-f3f8-4743-a94e-4db11fdb747d', '008a4215-0b1d-445e-b655-a964039cbb5a', '412 Maple St', null, 'Dowagiac', 'Michigan', '49047'),
('579872ec-46f8-46b5-b809-d0724d965f0e', '00963d9b-f884-485e-9455-fcf30c6ac379', '237 Mountain Ter', 'Apt 10', 'Odenville', 'Alabama', '35120'),
('95a983d0-ba0e-4f30-afb6-667d4724b253', '00963d9b-f884-485e-9455-fcf30c6ac379', '107 Annettes Ct', null, 'Aydlett', 'North Carolina', '27916');

insert into payment (id, user_id, card_number, expiry_month, expiry_year) values
('96bc3112-7753-11ed-a1eb-0242ac120002', '008a4215-0b1d-445e-b655-a964039cbb5a', '1234567890123456', '01', '2023'),
('9d0b2230-7753-11ed-a1eb-0242ac120002', '00963d9b-f884-485e-9455-fcf30c6ac379', '0987654321098765', '07', '2025'),
('a1c8c7fa-7753-11ed-a1eb-0242ac120002', 'fd6d21f6-f1c2-473d-8ed7-f3f9c7550cc9', '1234246835790987', '08', '2024'),
('39d95670-7756-11ed-a1eb-0242ac120002', 'fd6d21f6-f1c2-473d-8ed7-f3f9c7550cc9', '3691232465796801', '08', '2024'),
('410e0df0-7756-11ed-a1eb-0242ac120002', 'fd6d21f6-f1c2-473d-8ed7-f3f9c7550cc9', '1234509876123450', '08', '2024');
82 changes: 82 additions & 0 deletions src/test/java/com/bravo/user/controller/PaymentControllerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package com.bravo.user.controller;

import com.bravo.user.App;
import com.bravo.user.model.dto.PaymentDto;
import com.bravo.user.service.PaymentService;
import org.junit.jupiter.api.BeforeEach;
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.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.ResultActions;

import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

@ContextConfiguration(classes = { App.class })
@ExtendWith(SpringExtension.class)
@SpringBootTest()
@AutoConfigureMockMvc
class PaymentControllerTest {

@Autowired
private MockMvc mockMvc;

@MockBean
private PaymentService paymentService;

private List<PaymentDto> payments;

@BeforeEach
public void beforeEach() {
final List<Integer> ids = IntStream.range(1, 10).boxed().collect(Collectors.toList());

this.payments = ids.stream().map(id -> createPaymentDto(Integer.toString(id)))
.collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ids variable is unnecessary since it's only used once. You can combine these statements together.

this.payments = IntStream.range(1, 10).boxed()
    .map(id -> createPaymentDto(Integer.toString(id)))
    .collect(Collectors.toList())

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call out. I have refactored the code and fixed this to removed the redundant variable.

}

@Test
void givenValidUserId_retrievePayment_shouldReturnPayment() throws Exception {
final String userId = "123a-456b";

when(paymentService.retrieveByUserId(anyString())).thenReturn(payments);

final ResultActions result = this.mockMvc.perform(get("/payment/retrieve/".concat(userId)))
.andExpect(status().isOk());

for (int i = 0; i < payments.size(); i++) {
result.andExpect(jsonPath(String.format("$[%d].id", i)).value(payments.get(i).getId()));
}

verify(paymentService).retrieveByUserId(userId);
}

@Test
void givenWhitespaceUserId_retrievePayment_shouldBadRequestException() throws Exception {
this.mockMvc.perform(get("/payment/retrieve/ /")).andExpect(status().isBadRequest());
}

@Test
void givenMissingUserId_retrievePayment_shouldThrowNotFoundException() throws Exception {
this.mockMvc.perform(get("/payment/retrieve")).andExpect(status().isNotFound());
}

private PaymentDto createPaymentDto(final String id) {
final PaymentDto payment = new PaymentDto();
payment.setId(id);
return payment;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.bravo.user.dao.model.mapper;

import com.bravo.user.dao.model.Payment;
import com.bravo.user.model.dto.PaymentDto;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
Expand Down Expand Up @@ -48,4 +50,15 @@ void convertAddressTest(
@ConvertWith(MapperArgConverter.class) AddressDto addressDto) {
Assertions.assertEquals(addressDto, resourceMapper.convertAddress(address));
}

@ParameterizedTest
@CsvFileSource(
resources = ("/ResourceMapperTest/convertPaymentTest.csv"),
delimiter = '$',
lineSeparator = ">")
void convertPaymentTest(
@ConvertWith(MapperArgConverter.class) Payment payment,
@ConvertWith(MapperArgConverter.class) PaymentDto paymentDto) {
Assertions.assertEquals(paymentDto, resourceMapper.convertPayment(payment));
}
}
77 changes: 77 additions & 0 deletions src/test/java/com/bravo/user/service/PaymentServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package com.bravo.user.service;

import com.bravo.user.App;
import com.bravo.user.dao.model.Payment;
import com.bravo.user.dao.model.mapper.ResourceMapper;
import com.bravo.user.dao.repository.PaymentRepository;
import com.bravo.user.model.dto.PaymentDto;
import org.junit.jupiter.api.BeforeEach;
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.context.SpringBootTest;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;

import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ContextConfiguration(classes = { App.class })
@ExtendWith(SpringExtension.class)
@SpringBootTest
class PaymentServiceTest {

@Autowired
private PaymentService paymentService;

@MockBean
private ResourceMapper resourceMapper;

@MockBean
private PaymentRepository paymentRepository;

private List<PaymentDto> paymentDtos;

@BeforeEach
public void beforeEach() {
final List<Integer> ids = IntStream.range(1, 10).boxed().collect(Collectors.toList());

final List<Payment> daoPayments = ids.stream()
.map(id -> createPayment(Integer.toString(id))).collect(Collectors.toList());

when(paymentRepository.findPaymentByUserId(anyString())).thenReturn(daoPayments);

this.paymentDtos = ids.stream().map(id -> createPaymentDto(Integer.toString(id)))
.collect(Collectors.toList());

when(resourceMapper.convertPayments(daoPayments)).thenReturn(paymentDtos);
}

@Test
void givenValidUserId_findPaymentByUserId_retrievesPaymentMethods() {
final String userId = "123a-456b";
final List<PaymentDto> results = paymentService.retrieveByUserId(userId);
assertEquals(paymentDtos, results);

verify(paymentRepository).findPaymentByUserId(userId);
}

private Payment createPayment(final String id) {
final Payment payment = new Payment();
payment.setId(id);
return payment;
}

private PaymentDto createPaymentDto(final String id) {
final PaymentDto paymentDto = new PaymentDto();
paymentDto.setId(id);
return paymentDto;
}
}
15 changes: 15 additions & 0 deletions src/test/resources/ResourceMapperTest/convertPaymentTest.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
>{
"id":"testId",
"userId":"testUserId",
"cardNumber":"1234567890123456",
"expiryMonth":"01",
"expiryYear":"2023",
"updated":"2021-07-12 12:00:00"
}
${
"id":"testId",
"cardNumberLast4":"3456",
"expiryMonth":"01",
"expiryYear":"2023",
"updated":"2021-07-12 12:00:00"
}