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

sean carnahan: 0013 - [backend] add payment method retrieve api #41

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
Binary file added passing-payment-controller-test.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added passing-payment-service-test.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added payment-query-1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added payment-query-2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added payment-sql.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
42 changes: 42 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,42 @@
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.utility.PageUtil;
import com.bravo.user.validator.UserValidator;

import java.util.List;
import javax.servlet.http.HttpServletResponse;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.data.domain.PageRequest;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;

@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(
final @PathVariable String userId,
final @RequestParam(required = false) Integer page,
final @RequestParam(required = false) Integer size,
final HttpServletResponse httpResponse
) {
userValidator.validateId(userId);
final PageRequest pageRequest = PageUtil.createPageRequest(page, size);
return paymentService.retrieveByUserId(userId, pageRequest, httpResponse);
}
}
10 changes: 10 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,10 @@
package com.bravo.user.dao.repository;

import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.JpaSpecificationExecutor;
import org.springframework.stereotype.Repository;

import com.bravo.user.dao.model.Payment;

@Repository
public interface PaymentRepository extends JpaRepository<Payment, String>, JpaSpecificationExecutor<Payment>{}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package com.bravo.user.dao.specification;

import com.bravo.user.dao.model.Payment;

import java.util.Collections;

import javax.persistence.criteria.*;

public class PaymentSpecification extends AbstractSpecification<Payment> {

private final String userId;

public PaymentSpecification(final String userId){
this.userId = userId;
}

@Override
public void doFilter(
Root<Payment> root,
CriteriaQuery<?> criteriaQuery,
CriteriaBuilder criteriaBuilder
){
applyStringFilter(root.get("userId"), Collections.singletonList(userId));
}
}
42 changes: 42 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,42 @@
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.dao.specification.PaymentSpecification;
import com.bravo.user.model.dto.PaymentDto;
import com.bravo.user.utility.PageUtil;

import java.util.List;

import javax.servlet.http.HttpServletResponse;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.data.domain.PageRequest;
import org.springframework.stereotype.Service;
import org.springframework.data.domain.Page;

@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 PageRequest pageRequest, HttpServletResponse httpResponse) {
final PaymentSpecification specification = new PaymentSpecification(userId);
final Page<Payment> paymentPage = paymentRepository.findAll(specification, pageRequest);
final List<PaymentDto> payments = resourceMapper.convertPayments(paymentPage.getContent());

LOGGER.info("found {} payment(s)", payments.size());
PageUtil.updatePageHeaders(httpResponse, paymentPage, pageRequest);
return payments;
}
}
2 changes: 1 addition & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ spring:
password: ${DB_PASSWORD:password}
username: ${DB_USERNAME:sa}
url: ${DB_URL:jdbc:h2:mem:testdb}
h2.console.enabled: false
h2.console.enabled: true

Choose a reason for hiding this comment

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

Like this for the testing version of the appliation.yml. In the root, is this a security issue?

Choose a reason for hiding this comment

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

Agreed. I was thinking this was done to do queries and/or look at the schema. But it should only be done locally when working with the code.

Copy link
Author

Choose a reason for hiding this comment

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

Very true, nice catch. Should not be checked into main branch. Removed

seancarnahan marked this conversation as resolved.
Show resolved Hide resolved
jpa:
database-platform: ${DB_DIALECT:org.hibernate.dialect.H2Dialect}
properties.hibernate:
Expand Down
5 changes: 5 additions & 0 deletions src/main/resources/data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -648,3 +648,8 @@ 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
('payment-1', '008a4215-0b1d-445e-b655-a964039cbb5a', 'card-number-1', 12, 2027),

Choose a reason for hiding this comment

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

Is there potential for a card number that is not only digits? I've not seen that.

Choose a reason for hiding this comment

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

LOL. Yeah I noticed these "non-numeric" card #'s as well. Maybe he was just trying to make it obvious that's it's a card #, without needing to look at which column the value is mapped to on above line. My best guess here.

Copy link
Author

Choose a reason for hiding this comment

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

yeah was just trying to make it obvious haha. But it is also a varchar within the schema:

create table payment (
    id varchar(60) primary key,
    user_id varchar(60) not null,
    card_number varchar(16) not null unique,
    expiry_month integer not null,
    expiry_year integer not null,
    updated timestamp not null default current_timestamp()
);

In practice, typically we could get test credit card numbers from creditcard providers.

Choose a reason for hiding this comment

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

Nice. Yeah, I was on a project where we did the actual checking for the number transposing and modulo checks. That's a bit over the top for this :) . Using all the rules here https://www.validcreditcardnumber.com/ or here https://developer.visa.com/capabilities/pav/reference#tag/Payment-Account-Validation-API/operation/Card%20Validation_v1%20-%20Latest. I would've non-varchared it because of potential savings in disk usage. That's likely negligible up to a few million records though. So no biggie here.

('payment-2', '00963d9b-f884-485e-9455-fcf30c6ac379', 'card-number-2', 11, 2026),
('payment-3', '00963d9b-f884-485e-9455-fcf30c6ac379', 'card-number-3', 10, 2025);
97 changes: 97 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,97 @@
package com.bravo.user.controller;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
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;

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

import javax.servlet.http.HttpServletResponse;

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.data.domain.PageRequest;
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 com.bravo.user.App;
import com.bravo.user.model.dto.PaymentDto;
import com.bravo.user.service.PaymentService;
import com.bravo.user.utility.PageUtil;

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

@Test
void getRetrieveByUserId() throws Exception {
when(paymentService
.retrieveByUserId(anyString(), any(PageRequest.class), any(HttpServletResponse.class)))
.thenReturn(payments);

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

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

final PageRequest pageRequest = PageUtil.createPageRequest(null, null);
verify(paymentService).retrieveByUserId(
eq("1"), eq(pageRequest), any(HttpServletResponse.class)
);
}

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

@Test
void getRetrieveByUserId_Missing() 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;
}
}
107 changes: 107 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,107 @@
package com.bravo.user.service;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.when;

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

import javax.servlet.http.HttpServletResponse;

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.data.domain.Page;
import org.springframework.data.domain.PageRequest;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit.jupiter.SpringExtension;

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.dao.specification.PaymentSpecification;
import com.bravo.user.model.dto.PaymentDto;
import com.bravo.user.utility.PageUtil;

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

@Autowired
private HttpServletResponse httpResponse;

@Autowired
private PaymentService paymentService;

@MockBean
private ResourceMapper resourceMapper;

@MockBean
private PaymentRepository paymentRepository;

private List<PaymentDto> dtoPayments;

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

final Page<Payment> mockPage = mock(Page.class);
when(paymentRepository.findAll(any(PaymentSpecification.class), any(PageRequest.class)))
.thenReturn(mockPage);


final List<Payment> daoPayments = ids.stream()
.map(id -> createPayment(Integer.toString(id)))
.collect(Collectors.toList());
when(mockPage.getContent()).thenReturn(daoPayments);
when(mockPage.getTotalPages()).thenReturn(9);

this.dtoPayments = ids.stream()
.map(id -> createPaymentDto(Integer.toString(id)))
.collect(Collectors.toList());
when(resourceMapper.convertPayments(daoPayments)).thenReturn(dtoPayments);
}

@Test
void retrieveByUserId() {
final String input = "input";
final PageRequest pageRequest = PageUtil.createPageRequest(null, null);
final List<PaymentDto> results = paymentService.retrieveByUserId(input, pageRequest, httpResponse);
assertEquals(dtoPayments, results);
assertEquals("9", httpResponse.getHeader("page-count"));
assertEquals("1", httpResponse.getHeader("page-number"));
assertEquals("20", httpResponse.getHeader("page-size"));
}

@Test
void retrieveByUserIdPaged() {
final String input = "input2";
final PageRequest pageRequest = PageUtil.createPageRequest(2, 5);
final List<PaymentDto> results = paymentService.retrieveByUserId(input, pageRequest, httpResponse);
assertEquals(dtoPayments, results);
assertEquals("9", httpResponse.getHeader("page-count"));
assertEquals("2", httpResponse.getHeader("page-number"));
assertEquals("5", httpResponse.getHeader("page-size"));
}

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

private PaymentDto createPaymentDto(final String id) {
final PaymentDto payment = new PaymentDto();
payment.setId(id);
return payment;
}
Comment on lines +101 to +105

Choose a reason for hiding this comment

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

This sets the payment id. Are the other fields checked in these tests?

Copy link
Author

Choose a reason for hiding this comment

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

Was mainly just following the structure of the UserServiceTest, but we could and should test other fields as well.

Choose a reason for hiding this comment

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

The code in the repo isn't necessarily correct :) . I have been following Uncle Bob pretty closely and was a bit shocked by his calling untested code "immoral" https://www.amazon.com/Clean-Coder-Conduct-Professional-Programmers/dp/0137081073. While I think that is a bit overly harsh, from our business client's perspective it's of huge importance. Thanks for the replies. I put in a good word about you to Issac. :)

Copy link
Author

Choose a reason for hiding this comment

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

Great, thank you so much! And will have to check out that book and couldn't agree more that what matters for the client is what matters most.


}
Binary file added user_query.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.