Set SLF4J log format; improve logs; deprecate HttpExceptionHandler class

Signed-off-by: Pekka Helenius <fincer89@hotmail.com>

bookstore/.gitignore

@@ -1,5 +1,6 @@
 HELP.md
 target/
+logs/
 !.mvn/wrapper/maven-wrapper.jar
 !**/src/main/**/target/
 !**/src/test/**/target/

bookstore/src/main/java/com/fjordtek/bookstore/web/BookController.java

@@ -7,6 +7,7 @@ import java.util.HashMap;
 import java.util.Map;
 
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import javax.validation.Valid;
 
 import org.springframework.beans.factory.annotation.Autowired;
@@ -49,7 +50,6 @@ public class BookController {
 	}};
 
 	private HttpServerLogger     httpServerLogger     = new HttpServerLogger();
-	//private HttpExceptionHandler httpExceptionHandler = new HttpExceptionHandler();
 
 	@Autowired
 	private BookRepository       bookRepository;
@@ -70,14 +70,15 @@ public class BookController {
 			value    = bookListPageView,
 			method   = { RequestMethod.GET, RequestMethod.POST }
 			)
-	public String defaultWebFormGet(HttpServletRequest requestData, Model dataModel) {
+	public String defaultWebFormGet(
+			HttpServletRequest requestData,
+			HttpServletResponse responseData,
+			Model dataModel
+			) {
 
 		dataModel.addAttribute("books", bookRepository.findAll());
 
-		httpServerLogger.logMessageNormal(
-				requestData,
-				bookListPageView + ": " + "HTTPOK"
-				);
+		httpServerLogger.log(requestData, responseData);
 
 		return bookListPageView;
 
@@ -92,6 +93,7 @@ public class BookController {
 			)
 	public String webFormAddBook(
 			HttpServletRequest requestData,
+			HttpServletResponse responseData,
 			Model dataModel
 			) {
 
@@ -100,10 +102,7 @@ public class BookController {
 		dataModel.addAttribute("book", newBook);
 		dataModel.addAttribute("categories", categoryRepository.findAll());
 
-		httpServerLogger.logMessageNormal(
-				requestData,
-				bookAddPageView + ": " + "HTTPOK"
-				);
+		httpServerLogger.log(requestData, responseData);
 
 		return bookAddPageView;
 	}
@@ -115,7 +114,8 @@ public class BookController {
 	public String webFormSaveNewBook(
 			@Valid @ModelAttribute("book") Book book,
 			BindingResult bindingResult,
-			HttpServletRequest requestData
+			HttpServletRequest requestData,
+			HttpServletResponse responseData
 			) {
 
 		// TODO consider better solution. Add custom Hibernate annotation for Book class?
@@ -124,14 +124,12 @@ public class BookController {
 		}
 
 		if (bindingResult.hasErrors()) {
-			httpServerLogger.commonError("Book add: error " + book.toString(), requestData);
+			responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+			httpServerLogger.log(requestData, responseData);
 			return bookAddPageView;
 		}
 
-		httpServerLogger.logMessageNormal(
-				requestData,
-				bookAddPageView + ": " + "HTTPOK"
-				);
+		httpServerLogger.log(requestData, responseData);
 
 		return "redirect:" + bookListPageView;
 	}
@@ -145,15 +143,13 @@ public class BookController {
 			)
 	public String webFormDeleteBook(
 			@PathVariable("id") Long bookId,
-			HttpServletRequest requestData
+			HttpServletRequest requestData,
+			HttpServletResponse responseData
 			) {
 
 		bookRepository.deleteById(bookId);
 
-		httpServerLogger.logMessageNormal(
-				requestData,
-				bookDeletePageView + ": " + "HTTPOK"
-				);
+		httpServerLogger.log(requestData, responseData);
 
 		return "redirect:../" + bookListPageView;
 	}
@@ -168,17 +164,15 @@ public class BookController {
 	public String webFormEditBook(
 			@PathVariable("id") Long bookId,
 			Model dataModel,
-			HttpServletRequest requestData
+			HttpServletRequest requestData,
+			HttpServletResponse responseData
 			) {
 
 		Book book = bookRepository.findById(bookId).get();
 		dataModel.addAttribute("book", book);
 		dataModel.addAttribute("categories", categoryRepository.findAll());
 
-		httpServerLogger.logMessageNormal(
-				requestData,
-				bookEditPageView + ": " + "HTTPOK"
-				);
+		httpServerLogger.log(requestData, responseData);
 
 		return bookEditPageView;
 	}
@@ -198,23 +192,22 @@ public class BookController {
 			BindingResult bindingResult,
 			Model dataModel,
 			@PathVariable("id") Long bookId,
-			HttpServletRequest requestData
+			HttpServletRequest requestData,
+			HttpServletResponse responseData
 			) {
 
 		bookId = book.getId();
 		dataModel.addAttribute("categories", categoryRepository.findAll());
 
 		if (bindingResult.hasErrors()) {
-			httpServerLogger.commonError("Book edit: error " + book.toString(), requestData);
+			responseData.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+			httpServerLogger.log(requestData, responseData);
 			return bookEditPageView;
 		}
 
 		bookRepository.save(book);
 
-		httpServerLogger.logMessageNormal(
-				requestData,
-				bookEditPageView + ": " + "HTTPOK"
-				);
+		httpServerLogger.log(requestData, responseData);
 
 		return "redirect:../" + bookListPageView;
 	}
@@ -223,20 +216,15 @@ public class BookController {
 	// REDIRECTS
 
 	@RequestMapping(
-			value    = { "/", landingPageView },
-			method   = RequestMethod.GET
+			value    = { "*" }
 			)
 	@ResponseStatus(HttpStatus.FOUND)
-	public String redirectToDefaultWebForm() {
-		return "redirect:" + bookListPageView;
-	}
-
-	// Other URL requests
-	@RequestMapping(
-			value    = "*"
+	public String redirectToDefaultWebForm(
+			HttpServletRequest requestData,
+			HttpServletResponse responseData
 			)
-	public String errorWebForm(HttpServletRequest requestData) {
-		//return httpExceptionHandler.notFoundErrorHandler(requestData);
+	{
+		httpServerLogger.log(requestData, responseData);
 		return "redirect:" + bookListPageView;
 	}
 

bookstore/src/main/java/com/fjordtek/bookstore/web/HttpExceptionHandler.java

@@ -1,33 +0,0 @@
-// Pekka Helenius <fincer89@hotmail.com>, Fjordtek 2020
-
-package com.fjordtek.bookstore.web;
-
-import javax.servlet.http.HttpServletRequest;
-
-import org.springframework.http.HttpStatus;
-import org.springframework.web.bind.annotation.ControllerAdvice;
-import org.springframework.web.bind.annotation.ExceptionHandler;
-import org.springframework.web.bind.annotation.ResponseStatus;
-
-@ControllerAdvice
-public class HttpExceptionHandler {
-
-	private static final String HTTPNOTFOUND  = "Invalid request";
-	private HttpServerLogger httpServerLogger = new HttpServerLogger();
-
-	@ResponseStatus(
-			value   = HttpStatus.NOT_FOUND,
-			reason  = HTTPNOTFOUND
-			)
-	// Very simple exception handler (not any sophistication)
-	@ExceptionHandler(Exception.class)
-	public String notFoundErrorHandler(HttpServletRequest requestData) {
-
-		httpServerLogger.logMessageError(
-				requestData,
-				"HTTPNOTFOUND"
-				);
-		return "error";
-	}
-
-}

bookstore/src/main/java/com/fjordtek/bookstore/web/HttpServerLogger.java

@@ -2,58 +2,83 @@
 
 package com.fjordtek.bookstore.web;
 
-import java.time.LocalDateTime;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
 
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
-public class HttpServerLogger {
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
-	private LocalDateTime logTimeStamp;
+public class HttpServerLogger {
 
-	private void setLogTimeStamp() {
-		this.logTimeStamp = LocalDateTime.now();
-	}
+	private static final Logger serverLogger = LoggerFactory.getLogger(HttpServerLogger.class);
 
-	private LocalDateTime getLogTimeStamp() {
-		return this.logTimeStamp;
+	private static boolean httpStatusRange(int comparable, int lowLimit, int upLimit) {
+		return lowLimit <= comparable && comparable <= upLimit;
 	}
 
-	public void logMessageNormal(
+	public void log(
 			HttpServletRequest request,
-			String HttpRawStatusType
+			HttpServletResponse response
 			) {
 
-		setLogTimeStamp();
-		System.out.printf(
-				"%s: HTTP request to '%s' from client %s (%s)\n",
-				getLogTimeStamp(),
-				request.getRequestURL(),
-				request.getRemoteAddr(),
-				HttpRawStatusType
-				);
+		int status = response.getStatus();
+
+		List<String> requestParams            = new ArrayList<String>();
+		Enumeration<String> requestParamNames = request.getParameterNames();
+
+		if (requestParamNames != null) {
+			while (requestParamNames.hasMoreElements()) {
+
+				String paramName     = requestParamNames.nextElement().toString();
+				String[] paramValues = request.getParameterValues(paramName);
+
+				requestParams.add(
+						paramName + " = " +
+						String.join(", ", paramValues)
+						);
+			}
+		}
+
+		if (httpStatusRange(status, 0, 399)) {
+			serverLogger.info(
+					"HTTP request to '{}' from client '{}' [status: {}, attributes: {}]",
+					request.getRequestURL(),
+					request.getRemoteAddr(),
+					response.getStatus(),
+					requestParams
+					);
+		} else {
+			serverLogger.error(
+					"Invalid HTTP request to '{}' from client '{}' [status: {}, attributes: {}]",
+					request.getRequestURL(),
+					request.getRemoteAddr(),
+					response.getStatus(),
+					requestParams
+					);
+		}
+
 	}
 
-	public void logMessageError(
+	public void commonError(
+			String errorMsg,
 			HttpServletRequest request,
-			String HttpRawStatusType
-			) {
+			HttpServletResponse response,
+			String ...strings) {
 
-		setLogTimeStamp();
-		System.err.printf(
-				"%s: Invalid HTTP request to '%s' from client %s (%s)\n",
-				getLogTimeStamp(),
-				request.getRequestURL(),
+		String[] dataString = strings;
+
+		serverLogger.error(
+				"{} - {} [status: {}]",
 				request.getRemoteAddr(),
-				HttpRawStatusType
+				errorMsg,
+				response.getStatus()
 				);
-	}
-
-	public void commonError(String errorMsg, HttpServletRequest request, String ...strings) {
-
-		// Splitting log streams for proper server error logging
-		System.err.printf("%s: %s - %s\n", getLogTimeStamp(), request.getRemoteAddr(), errorMsg);
-		for (String dataString : strings) {
-			System.err.printf("%s\n", dataString);
+		if (dataString != null) {
+			serverLogger.error("{}", String.join(", ", dataString));
 		}
 	}
 

bookstore/src/main/resources/logback.xml

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+
+<Configuration>
+
+	<property name="LOG_FILE" value="logs/bookstore.log" />
+
+	<Appender name="console" target="SYSTEM_OUT" class="ch.qos.logback.core.ConsoleAppender">
+		<Layout class="ch.qos.logback.classic.PatternLayout">
+			<Pattern>
+				%d{yyyy-MM-dd HH:mm:ss.SSS} - [%thread] %-5level %logger{40} - %msg%n
+			</Pattern>
+		</Layout>
+	</Appender>
+
+	<Appender name="file" class="ch.qos.logback.core.FileAppender">
+		<File>${LOG_FILE}</File>
+		<Append>True</Append>
+		<Layout class="ch.qos.logback.classic.PatternLayout">
+			<Pattern>
+				%d{yyyy-MM-dd HH:mm:ss.SSS} - [%thread] %-5level %logger{40} - %msg%n
+			</Pattern>
+		</Layout>
+		<!--
+		<RollingPolicy class="ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy">
+			<fileNamePattern>
+				${LOG_FILE}.%d{yyyy-MM-dd}.%i.log
+			</fileNamePattern>
+			<MaxFileSize>2MB</MaxFileSize>
+		</RollingPolicy> -->
+	</Appender>
+
+	<Root level="info">
+		<Appender-Ref ref="console" />
+		<Appender-Ref ref="file"/>
+	</Root>
+
+</Configuration>