PAN-46 Added validation to the AccountController

2019-04-07 20:25:33 -06:00
parent 1bddb2d646
commit 8127cb1647
5 changed files with 170 additions and 83 deletions
@@ -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<Account> getAccountById(@RequestParam("id") final Integer id) {
+    ResponseEntity<Account> 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<Account> getAccountByUsername(@RequestParam("username") final String username) {
+    ResponseEntity<Account> 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<Account> 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<Account> 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<Void> deleteAccountById(@PathVariable("id") final Integer id) {
+    public @ResponseBody ResponseEntity<Void> 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);
@@ -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<String> 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<String> handleBadRequestException(final BadRequestException e) {
+        return e.getErrors();
+    }
+
+    @ExceptionHandler(UnprocessableEntityException.class)
+    @ResponseStatus(HttpStatus.UNPROCESSABLE_ENTITY)
+    public List<String> handleUnprocessableEntityException(final UnprocessableEntityException e) {
+        return Collections.singletonList("Request did not evaluate. Experienced the following error: " + e.getMessage());
+    }
+}
@@ -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<String> errors;
+
+    public BadRequestException(final String message) {
+        super(message);
+        errors = Collections.singletonList(message);
+    }
+
+    public BadRequestException(final List<String> errorMessages) {
+        super(String.join(" ", errorMessages));
+        errors = errorMessages;
+    }
+}
@@ -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<String> errors;
+
+    public UnprocessableEntityException(final String message) {
+        super(message);
+        errors = Collections.singletonList(message);
+    }
+
+    public UnprocessableEntityException(final List<String> errorMessages) {
+        super(String.join(" ", errorMessages));
+        errors = errorMessages;
+    }
+}
@@ -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<Account> 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<Account> 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<Account> 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<Account> 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() {