From 01769f8ad6132b0236b37bd9190f70ce640ff2e7 Mon Sep 17 00:00:00 2001 From: Pekka Helenius Date: Wed, 30 Sep 2020 15:36:15 +0300 Subject: [PATCH] Improve controller logic Signed-off-by: Pekka Helenius --- .../web/BookBasePathAwareController.java | 8 +- .../bookstore/web/BookController.java | 118 ++++++++++-------- .../bookstore/web/BookRestController.java | 17 ++- 3 files changed, 87 insertions(+), 56 deletions(-) diff --git a/bookstore/src/main/java/com/fjordtek/bookstore/web/BookBasePathAwareController.java b/bookstore/src/main/java/com/fjordtek/bookstore/web/BookBasePathAwareController.java index 4917ee7..19c99d3 100644 --- a/bookstore/src/main/java/com/fjordtek/bookstore/web/BookBasePathAwareController.java +++ b/bookstore/src/main/java/com/fjordtek/bookstore/web/BookBasePathAwareController.java @@ -70,7 +70,7 @@ public class BookBasePathAwareController { String authorFirstName = null, authorLastName = null; String categoryName = null; - // We keep going even if some of these is still null + // We keep going even if some of these are still null try { authorFirstName = bookNode.get("author").get("firstname").textValue(); } catch (NullPointerException e) {}; try { authorLastName = bookNode.get("author").get("lastname").textValue(); } catch (NullPointerException e) {}; try { categoryName = bookNode.get("category").get("name").textValue(); } catch (NullPointerException e) {}; @@ -139,7 +139,10 @@ public class BookBasePathAwareController { } catch (Exception e) { e.printStackTrace(); + + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); httpServerLogger.log(requestData, responseData); + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } } @@ -180,7 +183,10 @@ public class BookBasePathAwareController { } catch (Exception e) { e.printStackTrace(); + + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); httpServerLogger.log(requestData, responseData); + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } diff --git a/bookstore/src/main/java/com/fjordtek/bookstore/web/BookController.java b/bookstore/src/main/java/com/fjordtek/bookstore/web/BookController.java index 010fab7..f16ad1a 100644 --- a/bookstore/src/main/java/com/fjordtek/bookstore/web/BookController.java +++ b/bookstore/src/main/java/com/fjordtek/bookstore/web/BookController.java @@ -209,18 +209,23 @@ public class BookController { HttpServletResponse responseData ) { - Long bookId = new Long(bookHashRepository.findByHashId(bookHashId).getBookId()); + try { + Long bookId = new Long(bookHashRepository.findByHashId(bookHashId).getBookId()); - /* - * Delete associated book id foreign key (+ other row data) from book hash table - * at first, after which delete the book. - */ - bookHashRepository.deleteByBookId(bookId); - bookRepository.deleteById(bookId); + /* + * Delete associated book id foreign key (+ other row data) from book hash table + * at first, after which delete the book. + */ + bookHashRepository.deleteByBookId(bookId); + bookRepository.deleteById(bookId); + + } catch (NullPointerException e) { + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); + } httpServerLogger.log(requestData, responseData); - return "redirect:../" + bookListPageView; + return "redirect:/" + bookListPageView; } ////////////////////////////// @@ -237,14 +242,21 @@ public class BookController { HttpServletResponse responseData ) { - Long bookId = new Long(bookHashRepository.findByHashId(bookHashId).getBookId()); + try { + Long bookIdFromHash = bookHashRepository.findByHashId(bookHashId).getBookId(); + Book book = bookRepository.findById(bookIdFromHash).get(); - Book book = bookRepository.findById(bookId).get(); - dataModel.addAttribute("book", book); + dataModel.addAttribute("book", book); - httpServerLogger.log(requestData, responseData); + httpServerLogger.log(requestData, responseData); + return bookEditPageView; + + } catch (NullPointerException e) { + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); + httpServerLogger.log(requestData, responseData); + return "redirect:/" + bookListPageView; + } - return bookEditPageView; } /* NOTE: We keep Id here for the sake of proper URL formatting. @@ -259,54 +271,56 @@ public class BookController { ) public String webFormUpdateBook( @Valid @ModelAttribute("book") Book book, - BindingResult bindingResult, - @PathVariable("hash_id") String bookHashId, + BindingResult bindingResultBook, + @ModelAttribute ("hash_id") String bookHashId, HttpServletRequest requestData, HttpServletResponse responseData ) { BookHash bookHash = bookHashRepository.findByHashId(bookHashId); + if (bookHash == null) { - bindingResult.rejectValue("name", "error.user", "Unknown book"); - } else { + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); + httpServerLogger.log(requestData, responseData); + return "redirect:/" + bookListPageView; + } - // One-to-one unidirectional relationship handling - /* - * Must be set. Otherwise, we get TemplateProcessingException - * when user puts invalid field values - * (Thymeleaf template engine can't handle book.bookHash.hashId). - */ - book.setBookHash(bookHash); - /* - * Must be set. Otherwise, we may get merge errors when user - * has preceding error inputs in this view. - */ - bookHash.setBook(book); + // One-to-one unidirectional relationship handling + /* + * Must be set. Otherwise, we get TemplateProcessingException + * when user puts invalid field values + * (Thymeleaf template engine can't handle book.bookHash.hashId). + */ + book.setBookHash(bookHash); + /* + * Must be set. Otherwise, we may get merge errors when user + * has preceding error inputs in this view. + */ + bookHash.setBook(book); - // TODO consider better solution. Add custom Hibernate annotation for Book class? - Book bookI = bookRepository.findByIsbn(book.getIsbn()); - Long bookId = bookHash.getBookId(); + // TODO consider better solution. Add custom Hibernate annotation for Book class? + Book bookI = bookRepository.findByIsbn(book.getIsbn()); + Long bookId = bookHash.getBookId(); - if (bookId == null) { - bindingResult.rejectValue("name", "error.user", "Unknown book"); - } else { + if (bookId == null) { + bindingResultBook.rejectValue("name", "error.user", "Unknown book"); + } else { - // If existing ISBN value is not attached to the current book... - if (bookI != null) { - if (bookI.getId() != bookId) { - bindingResult.rejectValue("isbn", "error.user", "ISBN code already exists"); - } + // If existing ISBN value is not attached to the current book... + if (bookI != null) { + if (bookI.getId() != bookId) { + bindingResultBook.rejectValue("isbn", "error.user", "ISBN code already exists"); } - - /* - * This is necessary so that Hibernate does not attempt to INSERT data - * but UPDATEs current table data. - */ - book.setId(bookId); } + + /* + * This is necessary so that Hibernate does not attempt to INSERT data + * but UPDATEs current table data. + */ + book.setId(bookId); } - if (bindingResult.hasErrors()) { + if (bindingResultBook.hasErrors()) { responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); httpServerLogger.log(requestData, responseData); return bookEditPageView; @@ -316,8 +330,7 @@ public class BookController { bookRepository.save(book); httpServerLogger.log(requestData, responseData); - - return "redirect:../" + bookListPageView; + return "redirect:/" + bookListPageView; } ////////////////////////////// @@ -338,7 +351,7 @@ public class BookController { // REDIRECTS @RequestMapping( - value = { "*" } + value = { "*", "error" } ) @ResponseStatus(HttpStatus.FOUND) public String redirectToDefaultWebForm( @@ -346,8 +359,11 @@ public class BookController { HttpServletResponse responseData ) { + if (requestData.getRequestURI().matches("error")) { + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); + } httpServerLogger.log(requestData, responseData); - return "redirect:" + bookListPageView; + return "redirect:/" + bookListPageView; } @RequestMapping( diff --git a/bookstore/src/main/java/com/fjordtek/bookstore/web/BookRestController.java b/bookstore/src/main/java/com/fjordtek/bookstore/web/BookRestController.java index 826041e..10a932a 100644 --- a/bookstore/src/main/java/com/fjordtek/bookstore/web/BookRestController.java +++ b/bookstore/src/main/java/com/fjordtek/bookstore/web/BookRestController.java @@ -72,11 +72,20 @@ public class BookRestController { HttpServletResponse responseData ) { - Long bookId = new Long(bookHashRepository.findByHashId(bookHashId).getBookId()); + try { - httpServerLogger.log(requestData, responseData); + Long bookId = new Long(bookHashRepository.findByHashId(bookHashId).getBookId()); + httpServerLogger.log(requestData, responseData); + return bookRepository.findById(bookId); + + } catch (NullPointerException e) { + + responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST); + httpServerLogger.log(requestData, responseData); - return bookRepository.findById(bookId); + this.redirectToDefaultWebForm(requestData, responseData); + return null; + } } ////////////////////////////// @@ -90,9 +99,9 @@ public class BookRestController { HttpServletRequest requestData, HttpServletResponse responseData ) { - httpServerLogger.log(requestData, responseData); responseData.setHeader("Location", "/" + bookListPageView); responseData.setStatus(302); + httpServerLogger.log(requestData, responseData); } } \ No newline at end of file