From 8127cb1647a2cc89ab0d705070fc6a7e5669c45b Mon Sep 17 00:00:00 2001 From: atusa17 Date: Sun, 7 Apr 2019 20:25:33 -0600 Subject: [PATCH] PAN-46 Added validation to the AccountController --- .../controller/AccountController.java | 31 ++--- .../ExceptionHandlingController.java | 74 ++++++++++++ .../exception/BadRequestException.java | 20 ++++ .../UnprocessableEntityException.java | 21 ++++ .../controller/AccountControllerTest.java | 107 ++++++------------ 5 files changed, 170 insertions(+), 83 deletions(-) create mode 100644 persistence/src/main/java/edu/msudenver/tsp/persistence/controller/ExceptionHandlingController.java create mode 100644 persistence/src/main/java/edu/msudenver/tsp/persistence/exception/BadRequestException.java create mode 100644 persistence/src/main/java/edu/msudenver/tsp/persistence/exception/UnprocessableEntityException.java diff --git a/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/AccountController.java b/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/AccountController.java index 3301f65..f94eb99 100644 --- a/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/AccountController.java +++ b/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/AccountController.java @@ -1,6 +1,8 @@ package edu.msudenver.tsp.persistence.controller; import edu.msudenver.tsp.persistence.dto.Account; +import edu.msudenver.tsp.persistence.exception.BadRequestException; +import edu.msudenver.tsp.persistence.exception.UnprocessableEntityException; import edu.msudenver.tsp.persistence.repository.AccountsRepository; import edu.msudenver.tsp.utilities.PersistenceUtilities; import lombok.AllArgsConstructor; @@ -23,6 +25,7 @@ import java.util.Optional; @RestController @AllArgsConstructor @RequestMapping("/accounts") +@Validated public class AccountController { private final AccountsRepository accountsRepository; @@ -48,11 +51,11 @@ public class AccountController { @GetMapping("/id") public @ResponseBody - ResponseEntity getAccountById(@RequestParam("id") final Integer id) { + ResponseEntity getAccountById(@RequestParam("id") final Integer id) throws BadRequestException { LOG.info("Received request to query for account with id {}", id); if (id == null) { LOG.error("ERROR: ID was null"); - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + throw new BadRequestException("ERROR: ID cannot be null"); } LOG.debug("Querying for account with id {}", id); @@ -77,11 +80,11 @@ public class AccountController { @GetMapping("/username") public @ResponseBody - ResponseEntity getAccountByUsername(@RequestParam("username") final String username) { + ResponseEntity getAccountByUsername(@RequestParam("username") final String username) throws BadRequestException { LOG.info("Received request to query for account with username {}", username); if (username == null) { LOG.error("ERROR: username was null"); - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + throw new BadRequestException("ERROR: Username cannot be null"); } LOG.debug("Querying for account with username {}", username); @@ -107,17 +110,18 @@ public class AccountController { @PostMapping({"","/"}) @Validated({Account.Insert.class, Default.class}) public @ResponseBody ResponseEntity insertAccount( - @Valid @RequestBody final Account account, final BindingResult bindingResult) { + @Valid @RequestBody final Account account, final BindingResult bindingResult) + throws UnprocessableEntityException, BadRequestException { LOG.info("Received request to insert a new account"); if (bindingResult.hasErrors()) { LOG.error("Binding result is unprocessable"); - return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY); + throw new UnprocessableEntityException(bindingResult.getAllErrors().toString()); } if (account == null) { LOG.error("Passed account is unprocessable"); - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + throw new BadRequestException("Passed account is unprocessable"); } LOG.info("Checking for any existing users with username {}", account.getUsername()); @@ -152,22 +156,23 @@ public class AccountController { @PatchMapping("/{id}") public @ResponseBody ResponseEntity updateAccount( @PathVariable("id") final Integer id, - @RequestBody final Account account, final BindingResult bindingResult) { + @RequestBody final Account account, final BindingResult bindingResult) + throws UnprocessableEntityException, BadRequestException { LOG.info("Received request to update an account"); if (bindingResult.hasErrors()) { LOG.error("Binding result is unprocessable"); - return new ResponseEntity<>(HttpStatus.UNPROCESSABLE_ENTITY); + throw new UnprocessableEntityException(bindingResult.getAllErrors().toString()); } if (account == null) { LOG.error("Passed entity is null"); - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + throw new BadRequestException("Passed account is null"); } if (id == null) { LOG.error("Account ID must be specified"); - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + throw new BadRequestException("Account ID must be specified"); } LOG.debug("Checking for existence of account with id {}", id); @@ -204,11 +209,11 @@ public class AccountController { } @DeleteMapping("/{id}") - public @ResponseBody ResponseEntity deleteAccountById(@PathVariable("id") final Integer id) { + public @ResponseBody ResponseEntity deleteAccountById(@PathVariable("id") final Integer id) throws BadRequestException { LOG.info("Received request to delete account with id {}", id); if (id == null) { LOG.error("Specified Id is null"); - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + throw new BadRequestException("Specified ID is null"); } LOG.debug("Deleting account with id {}", id); diff --git a/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/ExceptionHandlingController.java b/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/ExceptionHandlingController.java new file mode 100644 index 0000000..737298f --- /dev/null +++ b/persistence/src/main/java/edu/msudenver/tsp/persistence/controller/ExceptionHandlingController.java @@ -0,0 +1,74 @@ +package edu.msudenver.tsp.persistence.controller; + +import edu.msudenver.tsp.persistence.exception.BadRequestException; +import edu.msudenver.tsp.persistence.exception.UnprocessableEntityException; +import lombok.extern.slf4j.Slf4j; +import org.springframework.http.HttpStatus; +import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.web.HttpMediaTypeNotSupportedException; +import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestControllerAdvice; + +import javax.servlet.http.HttpServletRequest; +import javax.validation.ValidationException; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +@Slf4j +@RestControllerAdvice +public class ExceptionHandlingController { + + @ExceptionHandler(HttpMessageNotReadableException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + public void handleMessageNotReadableException(final HttpServletRequest request, final HttpMessageNotReadableException e) { + LOG.error("Unable to bind post data sent to: {} \nCaught Exception: {}", request.getRequestURI(), e.getMessage()); + } + + @ExceptionHandler(ValidationException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + public String handleMessageInvalidRequest(final HttpServletRequest request, final ValidationException e) { + LOG.error("Request did not validate. Experienced the following error: {}", e.getMessage()); + return "Request did not evaluate. Experienced the following error: " + e.getMessage(); + } + + @ExceptionHandler(HttpRequestMethodNotSupportedException.class) + @ResponseStatus(HttpStatus.METHOD_NOT_ALLOWED) + public void handleHttpRequestMethodNotSupported(final HttpServletRequest request) { + LOG.error("Unsupported request method ({}) for URL: {}", request.getMethod(), request.getRequestURI()); + } + + @ExceptionHandler(HttpMediaTypeNotSupportedException.class) + @ResponseStatus(HttpStatus.UNSUPPORTED_MEDIA_TYPE) + public void handleHttpMediaTypeNotSupportedException(final HttpServletRequest request, final HttpMediaTypeNotSupportedException e) { + LOG.error("Unsupported media type ({}) for URL: {}", e.getContentType(), request.getRequestURI()); + } + + @ExceptionHandler(Exception.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + public void handleException(final HttpServletRequest request, final Exception e) { + LOG.error("Exception caught for URL: {}", request.getRequestURI(), e); + } + + @ExceptionHandler(MethodArgumentNotValidException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + public List handleMethodArgumentNotValidException(final MethodArgumentNotValidException e) { + LOG.error("Request did not evaluate. Experienced the following error: {}", e.getMessage()); + return e.getBindingResult().getFieldErrors().stream().map(x -> x.getField() + " " + x.getDefaultMessage()).collect(Collectors.toList()); + } + + @ExceptionHandler(BadRequestException.class) + @ResponseStatus(HttpStatus.BAD_REQUEST) + public List handleBadRequestException(final BadRequestException e) { + return e.getErrors(); + } + + @ExceptionHandler(UnprocessableEntityException.class) + @ResponseStatus(HttpStatus.UNPROCESSABLE_ENTITY) + public List handleUnprocessableEntityException(final UnprocessableEntityException e) { + return Collections.singletonList("Request did not evaluate. Experienced the following error: " + e.getMessage()); + } +} diff --git a/persistence/src/main/java/edu/msudenver/tsp/persistence/exception/BadRequestException.java b/persistence/src/main/java/edu/msudenver/tsp/persistence/exception/BadRequestException.java new file mode 100644 index 0000000..ec37a6d --- /dev/null +++ b/persistence/src/main/java/edu/msudenver/tsp/persistence/exception/BadRequestException.java @@ -0,0 +1,20 @@ +package edu.msudenver.tsp.persistence.exception; + +import lombok.Getter; + +import java.util.Collections; +import java.util.List; + +public class BadRequestException extends Exception { + @Getter private final List errors; + + public BadRequestException(final String message) { + super(message); + errors = Collections.singletonList(message); + } + + public BadRequestException(final List errorMessages) { + super(String.join(" ", errorMessages)); + errors = errorMessages; + } +} diff --git a/persistence/src/main/java/edu/msudenver/tsp/persistence/exception/UnprocessableEntityException.java b/persistence/src/main/java/edu/msudenver/tsp/persistence/exception/UnprocessableEntityException.java new file mode 100644 index 0000000..758e33e --- /dev/null +++ b/persistence/src/main/java/edu/msudenver/tsp/persistence/exception/UnprocessableEntityException.java @@ -0,0 +1,21 @@ +package edu.msudenver.tsp.persistence.exception; + +import lombok.Getter; + +import java.util.Collections; +import java.util.List; + +public class UnprocessableEntityException extends Exception { + @Getter + private final List errors; + + public UnprocessableEntityException(final String message) { + super(message); + errors = Collections.singletonList(message); + } + + public UnprocessableEntityException(final List errorMessages) { + super(String.join(" ", errorMessages)); + errors = errorMessages; + } +} diff --git a/persistence/src/test/java/edu/msudenver/tsp/persistence/controller/AccountControllerTest.java b/persistence/src/test/java/edu/msudenver/tsp/persistence/controller/AccountControllerTest.java index 503e2f6..11a099c 100644 --- a/persistence/src/test/java/edu/msudenver/tsp/persistence/controller/AccountControllerTest.java +++ b/persistence/src/test/java/edu/msudenver/tsp/persistence/controller/AccountControllerTest.java @@ -1,6 +1,8 @@ package edu.msudenver.tsp.persistence.controller; import edu.msudenver.tsp.persistence.dto.Account; +import edu.msudenver.tsp.persistence.exception.BadRequestException; +import edu.msudenver.tsp.persistence.exception.UnprocessableEntityException; import edu.msudenver.tsp.persistence.repository.AccountsRepository; import org.junit.Test; import org.junit.runner.RunWith; @@ -51,7 +53,7 @@ public class AccountControllerTest { } @Test - public void testGetAccountById() { + public void testGetAccountById() throws BadRequestException { final Account account = createAccount(); when(accountsRepository.findById(anyInt())).thenReturn(Optional.ofNullable(account)); @@ -65,18 +67,13 @@ public class AccountControllerTest { verify(accountsRepository).findById(anyInt()); } - @Test - public void testGetAccountById_nullId() { - final ResponseEntity responseEntity = accountController.getAccountById(null); - - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); + @Test(expected = BadRequestException.class) + public void testGetAccountById_nullId() throws BadRequestException { + accountController.getAccountById(null); } @Test - public void testGetAccountById_noAccountFound() { + public void testGetAccountById_noAccountFound() throws BadRequestException { when(accountsRepository.findById(anyInt())).thenReturn(Optional.empty()); final ResponseEntity responseEntity = accountController.getAccountById(1); @@ -88,7 +85,7 @@ public class AccountControllerTest { } @Test - public void testGetAccountByUsername() { + public void testGetAccountByUsername() throws BadRequestException { final Account account = createAccount(); when(accountsRepository.findByUsername(anyString())).thenReturn(Optional.ofNullable(account)); @@ -102,18 +99,13 @@ public class AccountControllerTest { verify(accountsRepository).findByUsername(anyString()); } - @Test - public void testGetAccountById_nullUsername() { - final ResponseEntity responseEntity = accountController.getAccountByUsername(null); - - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); + @Test(expected = BadRequestException.class) + public void testGetAccountById_nullUsername() throws BadRequestException { + accountController.getAccountByUsername(null); } @Test - public void testGetAccountByUsername_noAccountFound() { + public void testGetAccountByUsername_noAccountFound() throws BadRequestException { when(accountsRepository.findByUsername(anyString())).thenReturn(Optional.empty()); final ResponseEntity responseEntity = accountController.getAccountByUsername("Test username"); @@ -125,7 +117,7 @@ public class AccountControllerTest { } @Test - public void testInsertAccount() { + public void testInsertAccount() throws UnprocessableEntityException, BadRequestException { final Account account = createAccount(); when(accountsRepository.save(any(Account.class))).thenReturn(account); when(accountsRepository.findByUsername(anyString())).thenReturn(Optional.empty()); @@ -141,7 +133,7 @@ public class AccountControllerTest { } @Test - public void testInsertAccount_usernameAlreadyExists() { + public void testInsertAccount_usernameAlreadyExists() throws UnprocessableEntityException, BadRequestException { final Account account = createAccount(); when(accountsRepository.findByUsername(anyString())).thenReturn(Optional.of(account)); @@ -154,8 +146,8 @@ public class AccountControllerTest { verify(accountsRepository, times(0)).save(any(Account.class)); } - @Test - public void testInsertAccount_accountsDtoIsNull() { + @Test(expected = BadRequestException.class) + public void testInsertAccount_accountsDtoIsNull() throws UnprocessableEntityException, BadRequestException { final ResponseEntity responseEntity = accountController.insertAccount(null, bindingResult); assertNotNull(responseEntity); @@ -164,21 +156,16 @@ public class AccountControllerTest { verifyZeroInteractions(accountsRepository); } - @Test - public void testInsertAccount_bindingResultHasErrors() { + @Test(expected = UnprocessableEntityException.class) + public void testInsertAccount_bindingResultHasErrors() throws UnprocessableEntityException, BadRequestException { final Account account = createAccount(); when(bindingResult.hasErrors()).thenReturn(true); - final ResponseEntity responseEntity = accountController.insertAccount(account, bindingResult); - - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); + accountController.insertAccount(account, bindingResult); } @Test - public void testUpdateAccount() { + public void testUpdateAccount() throws UnprocessableEntityException, BadRequestException { final Account existingAccount = createAccount(); existingAccount.setId(1); existingAccount.setVersion(1); @@ -199,40 +186,25 @@ public class AccountControllerTest { verify(accountsRepository).save(any(Account.class)); } - @Test - public void testUpdateAccount_bindingResultHasErrors() { + @Test(expected = UnprocessableEntityException.class) + public void testUpdateAccount_bindingResultHasErrors() throws UnprocessableEntityException, BadRequestException { when(bindingResult.hasErrors()).thenReturn(true); - final ResponseEntity responseEntity = accountController.updateAccount(1, createAccount(), bindingResult); + accountController.updateAccount(1, createAccount(), bindingResult); + } - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.UNPROCESSABLE_ENTITY, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); + @Test(expected = BadRequestException.class) + public void testUpdateAccount_accountsDtoIsNull() throws UnprocessableEntityException, BadRequestException { + accountController.updateAccount(1, null, bindingResult); + } + + @Test(expected = BadRequestException.class) + public void testUpdateAccount_idIsNull() throws UnprocessableEntityException, BadRequestException { + accountController.updateAccount(null, createAccount(), bindingResult); } @Test - public void testUpdateAccount_accountsDtoIsNull() { - final ResponseEntity responseEntity = accountController.updateAccount(1, null, bindingResult); - - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); - } - - @Test - public void testUpdateAccount_idIsNull() { - final ResponseEntity responseEntity = accountController.updateAccount(null, createAccount(), bindingResult); - - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); - } - - @Test - public void testUpdateAccount_accountDoesNotExist() { + public void testUpdateAccount_accountDoesNotExist() throws UnprocessableEntityException, BadRequestException { when(accountsRepository.findById(anyInt())).thenReturn(Optional.empty()); final ResponseEntity responseEntity = accountController.updateAccount(1, createAccount(), bindingResult); @@ -244,7 +216,7 @@ public class AccountControllerTest { } @Test - public void testDeleteAccountById() { + public void testDeleteAccountById() throws BadRequestException { doNothing().when(accountsRepository).deleteById(anyInt()); final ResponseEntity responseEntity = accountController.deleteAccountById(1); @@ -255,14 +227,9 @@ public class AccountControllerTest { verify(accountsRepository).deleteById(anyInt()); } - @Test - public void testDeleteAccountById_idIsNull() { - final ResponseEntity responseEntity = accountController.deleteAccountById(null); - - assertNotNull(responseEntity); - assertFalse(responseEntity.hasBody()); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - verifyZeroInteractions(accountsRepository); + @Test(expected = BadRequestException.class) + public void testDeleteAccountById_idIsNull() throws BadRequestException { + accountController.deleteAccountById(null); } private Account createAccount() {