-
Notifications
You must be signed in to change notification settings - Fork 41
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
trello ticket 13 - candidate-sample-payment-addition #32
base: main
Are you sure you want to change the base?
Changes from all commits
5a94198
43db612
1269862
212c5f1
e1e8463
e55716f
535cd72
1e12677
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package com.bravo.user.controller; | ||
|
||
import com.bravo.user.annotation.SwaggerController; | ||
import com.bravo.user.exception.BadRequestException; | ||
import com.bravo.user.model.dto.PaymentDto; | ||
import com.bravo.user.model.filter.PaymentFilter; | ||
import com.bravo.user.service.PaymentService; | ||
import com.bravo.user.utility.PageUtil; | ||
import com.bravo.user.validator.UserValidator; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.data.domain.PageRequest; | ||
import org.springframework.web.bind.annotation.*; | ||
|
||
import javax.servlet.http.HttpServletResponse; | ||
import java.util.List; | ||
|
||
@RequestMapping(value = "/payment") | ||
@SwaggerController | ||
public class PaymentController { | ||
|
||
private final UserValidator userValidator; | ||
private final PaymentService paymentService; | ||
|
||
public PaymentController( | ||
UserValidator userValidator, | ||
PaymentService paymentService | ||
){ | ||
this.userValidator = userValidator; | ||
this.paymentService = paymentService; | ||
} | ||
|
||
@GetMapping(value = "/retrieve") | ||
@ResponseBody | ||
public List<PaymentDto> retrieve( | ||
final @RequestParam 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.retrievePaymentByUserId(userId, pageRequest, httpResponse); | ||
} | ||
|
||
@PostMapping(value = "/retrieve") | ||
@ResponseBody | ||
public List<PaymentDto> retrieve( | ||
final @RequestBody PaymentFilter filter, | ||
final @RequestParam(required = false) Integer page, | ||
final @RequestParam(required = false) Integer size, | ||
final HttpServletResponse httpResponse | ||
){ | ||
final PageRequest pageRequest = PageUtil.createPageRequest(page, size); | ||
return paymentService.retrieve(filter, pageRequest, httpResponse); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package com.bravo.user.dao.repository; | ||
|
||
import com.bravo.user.dao.model.Payment; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
import org.springframework.data.jpa.repository.JpaSpecificationExecutor; | ||
import org.springframework.stereotype.Repository; | ||
|
||
@Repository | ||
public interface PaymentRepository extends JpaRepository<Payment, String>, JpaSpecificationExecutor<Payment>{} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package com.bravo.user.dao.specification; | ||
|
||
import com.bravo.user.dao.model.Payment; | ||
import com.bravo.user.model.filter.PaymentFilter; | ||
import lombok.EqualsAndHashCode; | ||
|
||
import javax.persistence.criteria.CriteriaBuilder; | ||
import javax.persistence.criteria.CriteriaQuery; | ||
import javax.persistence.criteria.Root; | ||
import java.util.Set; | ||
|
||
@EqualsAndHashCode | ||
public class PaymentSpecification extends AbstractSpecification<Payment>{ | ||
|
||
private final PaymentFilter filter; | ||
|
||
public PaymentSpecification(final PaymentFilter filter) { | ||
this.filter = filter; | ||
} | ||
|
||
@Override | ||
void doFilter( | ||
Root<Payment> root, | ||
CriteriaQuery<?> criteriaQuery, | ||
CriteriaBuilder criteriaBuilder | ||
){ | ||
applyStringFilterToFields(Set.of( | ||
root.get("userId") | ||
), filter.getUserId()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package com.bravo.user.model.filter; | ||
|
||
import lombok.Builder; | ||
import lombok.Data; | ||
import lombok.Value; | ||
import lombok.extern.jackson.Jacksonized; | ||
|
||
@Value | ||
@Builder | ||
@Jacksonized | ||
public class PaymentFilter { | ||
|
||
private String userId; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
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.model.filter.PaymentFilter; | ||
import com.bravo.user.utility.PageUtil; | ||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
import org.springframework.data.domain.Page; | ||
import org.springframework.data.domain.PageRequest; | ||
import org.springframework.stereotype.Service; | ||
|
||
import javax.servlet.http.HttpServletResponse; | ||
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> retrievePaymentByUserId( | ||
final String userId, | ||
final PageRequest pageRequest, | ||
final HttpServletResponse httpResponse | ||
){ | ||
return retrieve(PaymentFilter.builder().userId(userId).build(), pageRequest, httpResponse); | ||
} | ||
|
||
public List<PaymentDto> retrieve( | ||
final PaymentFilter filter, | ||
final PageRequest pageRequest, | ||
final HttpServletResponse httpResponse | ||
){ | ||
LOGGER.debug("Request to retrieve payment information being conducted... paymentFilter: {}", filter); | ||
final PaymentSpecification specification = new PaymentSpecification(filter); | ||
final Page<Payment> paymentPage = paymentRepository.findAll(specification,pageRequest); | ||
final List<PaymentDto> payments = resourceMapper.convertPayments(paymentPage.getContent()); | ||
LOGGER.debug("Found {} payment(s)", payments.size()); | ||
|
||
PageUtil.updatePageHeaders(httpResponse, paymentPage, pageRequest); | ||
return payments; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
package com.bravo.user.controller; | ||
|
||
import com.bravo.user.App; | ||
import com.bravo.user.model.dto.PaymentDto; | ||
import com.bravo.user.model.filter.PaymentFilter; | ||
import com.bravo.user.service.PaymentService; | ||
import com.bravo.user.utility.PageUtil; | ||
import com.fasterxml.jackson.databind.ObjectMapper; | ||
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.http.MediaType; | ||
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 javax.servlet.http.HttpServletResponse; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import java.util.stream.IntStream; | ||
|
||
|
||
import static org.mockito.ArgumentMatchers.*; | ||
import static org.mockito.Mockito.verify; | ||
import static org.mockito.Mockito.when; | ||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; | ||
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 | ||
public class PaymentControllerTest { | ||
|
||
private static final ObjectMapper MAPPER = new ObjectMapper(); | ||
|
||
@Autowired | ||
private MockMvc mockMvc; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Happy to see mockmvc. That is often missed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, it's been a little while since I have done JUnits and Mockito, so I am trying to do more learning and example coding in my free-time. |
||
@MockBean | ||
private PaymentService paymentService; | ||
|
||
private List<PaymentDto> payments; | ||
private PaymentFilter paymentFilter; | ||
|
||
@BeforeEach | ||
public void beforeEach(){ | ||
this.payments = IntStream | ||
.range(1,6) | ||
.mapToObj(id -> PaymentDto.builder().id(Integer.toString(id)).build()) | ||
.collect(Collectors.toList()); | ||
|
||
this.paymentFilter = PaymentFilter.builder().userId("1").build(); | ||
} | ||
|
||
@Test | ||
public void getRetrieveWithUserId() throws Exception { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the throws needed here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it is to cover the .perform, and result.andExpect calls as both throw exceptions. I could wrap these into a try/catch block, but this makes the code cleaner. |
||
when(paymentService | ||
.retrievePaymentByUserId(anyString(), any(PageRequest.class), any(HttpServletResponse.class))) | ||
.thenReturn(payments); | ||
|
||
final ResultActions result = this.mockMvc | ||
.perform(get( "/payment/retrieve?userId=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(); | ||
verify(paymentService).retrievePaymentByUserId( | ||
eq("1"), eq(pageRequest), any(HttpServletResponse.class) | ||
); | ||
} | ||
|
||
@Test | ||
public void postRetrieveWithFilter() throws Exception { | ||
when(paymentService.retrieve(any(PaymentFilter.class), any(PageRequest.class), any(HttpServletResponse.class))) | ||
.thenReturn(payments); | ||
Comment on lines
+84
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Which layers are being tested with this test? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only the flow of the Controller, as the mock is designed to merely mock the results of calling the Service class. |
||
|
||
final String jsonResult = MAPPER.writeValueAsString(paymentFilter); | ||
|
||
final ResultActions result = this.mockMvc | ||
.perform(post("/payment/retrieve") | ||
.contentType(MediaType.APPLICATION_JSON) | ||
.content(jsonResult) | ||
.accept(MediaType.APPLICATION_JSON)) | ||
.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(); | ||
verify(paymentService).retrieve(eq(paymentFilter), eq(pageRequest), any(HttpServletResponse.class)); | ||
} | ||
|
||
@Test | ||
public void retrieveWithUserIdMissing() throws Exception { | ||
this.mockMvc.perform(get("/payment/retrieve")).andExpect(status().isBadRequest()); | ||
} | ||
|
||
@Test | ||
public void retrieveWithEmptyUserId() throws Exception { | ||
this.mockMvc.perform(get("/payment/retrieve?userId=")).andExpect(status().isBadRequest()); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it is only used for test code. In general, having nulls can create additional checks that need to happen in code. Since this is only in test, could the other initializer be used with hardcoded values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was more of a code cleanup effort from having null values in tests to acquire default values from the PageUtil (page 1, size 20). Just a personal choice, but the original call could still be used, and this removed.