Commit b4422825 authored by Matija Obreza's avatar Matija Obreza
Browse files

Merge branch '151-improve-mvc-logging' into 'master'

Improve MVC logging

Closes #151

See merge request !67
parents 6a81d5d7 207d0406
...@@ -20,8 +20,8 @@ import java.util.Locale; ...@@ -20,8 +20,8 @@ import java.util.Locale;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import org.apache.commons.logging.Log; import org.slf4j.Logger;
import org.apache.commons.logging.LogFactory; import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.MessageSource; import org.springframework.context.MessageSource;
import org.springframework.context.i18n.LocaleContextHolder; import org.springframework.context.i18n.LocaleContextHolder;
...@@ -33,7 +33,7 @@ import org.springframework.web.context.request.ServletRequestAttributes; ...@@ -33,7 +33,7 @@ import org.springframework.web.context.request.ServletRequestAttributes;
public abstract class BaseController { public abstract class BaseController {
protected Log _logger = LogFactory.getLog(getClass()); protected Logger _logger = LoggerFactory.getLogger(getClass());
protected static final String ANONYMOUS_USER = "anonymousUser"; protected static final String ANONYMOUS_USER = "anonymousUser";
......
/** /*
* Copyright 2014 Global Crop Diversity Trust * Copyright 2017 Global Crop Diversity Trust
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -16,10 +16,13 @@ ...@@ -16,10 +16,13 @@
package org.genesys2.server.servlet.controller; package org.genesys2.server.servlet.controller;
import java.text.MessageFormat;
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import javax.servlet.http.HttpServletRequest;
import org.genesys2.spring.ResourceNotFoundException; import org.genesys2.spring.ResourceNotFoundException;
import org.springframework.http.HttpStatus; import org.springframework.http.HttpStatus;
import org.springframework.security.access.AccessDeniedException; import org.springframework.security.access.AccessDeniedException;
...@@ -27,6 +30,7 @@ import org.springframework.security.core.AuthenticationException; ...@@ -27,6 +30,7 @@ import org.springframework.security.core.AuthenticationException;
import org.springframework.validation.Errors; import org.springframework.validation.Errors;
import org.springframework.validation.FieldError; import org.springframework.validation.FieldError;
import org.springframework.validation.ObjectError; import org.springframework.validation.ObjectError;
import org.springframework.web.HttpRequestMethodNotSupportedException;
import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ExceptionHandler;
...@@ -40,7 +44,7 @@ public class UserControllerAdvice extends BaseController { ...@@ -40,7 +44,7 @@ public class UserControllerAdvice extends BaseController {
@ResponseStatus(code = HttpStatus.FORBIDDEN) @ResponseStatus(code = HttpStatus.FORBIDDEN)
@ExceptionHandler(value = { AccessDeniedException.class }) @ExceptionHandler(value = { AccessDeniedException.class })
public ModelAndView handleAccessDeniedException(AccessDeniedException e) { public ModelAndView handleAccessDeniedException(final AccessDeniedException e) {
final ModelAndView mav = new ModelAndView("/errors/error"); final ModelAndView mav = new ModelAndView("/errors/error");
mav.addObject("exception", e); mav.addObject("exception", e);
return mav; return mav;
...@@ -48,7 +52,7 @@ public class UserControllerAdvice extends BaseController { ...@@ -48,7 +52,7 @@ public class UserControllerAdvice extends BaseController {
@ResponseStatus(HttpStatus.NOT_FOUND) @ResponseStatus(HttpStatus.NOT_FOUND)
@ExceptionHandler(value = { ResourceNotFoundException.class, NoHandlerFoundException.class }) @ExceptionHandler(value = { ResourceNotFoundException.class, NoHandlerFoundException.class })
public ModelAndView handleResourceNotFoundException(ResourceNotFoundException e) { public ModelAndView handleResourceNotFoundException(final ResourceNotFoundException e) {
final ModelAndView mav = new ModelAndView("/errors/error"); final ModelAndView mav = new ModelAndView("/errors/error");
mav.addObject("exception", e); mav.addObject("exception", e);
return mav; return mav;
...@@ -57,14 +61,23 @@ public class UserControllerAdvice extends BaseController { ...@@ -57,14 +61,23 @@ public class UserControllerAdvice extends BaseController {
@ResponseStatus(HttpStatus.UNAUTHORIZED) @ResponseStatus(HttpStatus.UNAUTHORIZED)
@ExceptionHandler(value = { AuthenticationException.class }) @ExceptionHandler(value = { AuthenticationException.class })
@ResponseBody @ResponseBody
public String handleAuthenticationException(AuthenticationException e) { public String handleAuthenticationException(final AuthenticationException e) {
return simpleExceptionHandler(e); return simpleExceptionHandler(e);
} }
@ResponseStatus(HttpStatus.METHOD_NOT_ALLOWED)
@ExceptionHandler(value = { HttpRequestMethodNotSupportedException.class })
public ModelAndView handleRequestMethodFail(final HttpServletRequest req, final HttpRequestMethodNotSupportedException e) {
_logger.error("Request method {} not supported for URL {}", e.getMethod(), req.getRequestURL());
final ModelAndView mav = new ModelAndView("/errors/error");
mav.addObject("exception", e);
return mav;
}
@ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR)
@ExceptionHandler(value = { Throwable.class }) @ExceptionHandler(value = { Throwable.class })
public ModelAndView handleAll(Throwable e) { public ModelAndView handleAll(final HttpServletRequest req, final Throwable e) {
_logger.error(e.getMessage(), e); _logger.error(MessageFormat.format("{} on {} {}", e.getMessage(), req.getMethod(), req.getRequestURL()), e);
final ModelAndView mav = new ModelAndView("/errors/error"); final ModelAndView mav = new ModelAndView("/errors/error");
mav.addObject("exception", e); mav.addObject("exception", e);
return mav; return mav;
...@@ -74,12 +87,12 @@ public class UserControllerAdvice extends BaseController { ...@@ -74,12 +87,12 @@ public class UserControllerAdvice extends BaseController {
@ResponseStatus(HttpStatus.BAD_REQUEST) @ResponseStatus(HttpStatus.BAD_REQUEST)
@ExceptionHandler(value = { MethodArgumentNotValidException.class }) @ExceptionHandler(value = { MethodArgumentNotValidException.class })
@ResponseBody @ResponseBody
public Object handleMethodArgumentNotValidException(MethodArgumentNotValidException e) { public Object handleMethodArgumentNotValidException(final MethodArgumentNotValidException e) {
return transformErrors(e.getBindingResult()); return transformErrors(e.getBindingResult());
} }
private Map<String, String> transformErrors(Errors errors) { private Map<String, String> transformErrors(final Errors errors) {
final Map<String, String> errorsMap = new HashMap<String, String>(); final Map<String, String> errorsMap = new HashMap<>();
final List<ObjectError> allErrors = errors.getAllErrors(); final List<ObjectError> allErrors = errors.getAllErrors();
......
/** /*
* Copyright 2014 Global Crop Diversity Trust * Copyright 2017 Global Crop Diversity Trust
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
...@@ -31,10 +31,11 @@ import javax.servlet.http.HttpServletRequest; ...@@ -31,10 +31,11 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.StringUtils;
import org.apache.log4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class LocaleURLFilter implements Filter { public class LocaleURLFilter implements Filter {
private static final Logger LOG = Logger.getLogger(LocaleURLFilter.class); private static final Logger LOG = LoggerFactory.getLogger(LocaleURLFilter.class);
private static final LocaleURLMatcher localeUrlMatcher = new LocaleURLMatcher(); private static final LocaleURLMatcher localeUrlMatcher = new LocaleURLMatcher();
public static final String REQUEST_LOCALE_ATTR = LocaleURLFilter.class.getName() + ".LOCALE"; public static final String REQUEST_LOCALE_ATTR = LocaleURLFilter.class.getName() + ".LOCALE";
...@@ -44,29 +45,29 @@ public class LocaleURLFilter implements Filter { ...@@ -44,29 +45,29 @@ public class LocaleURLFilter implements Filter {
private Locale defaultLocale; private Locale defaultLocale;
@Override @Override
public void init(FilterConfig filterConfig) throws ServletException { public void init(final FilterConfig filterConfig) throws ServletException {
String excludePaths = filterConfig.getInitParameter("exclude-paths"); final String excludePaths = filterConfig.getInitParameter("exclude-paths");
if (StringUtils.isNotBlank(excludePaths)) { if (StringUtils.isNotBlank(excludePaths)) {
String[] ex = excludePaths.split("\\s+"); final String[] ex = excludePaths.split("\\s+");
for (String e : ex) { for (final String e : ex) {
LOG.info("Excluding path: " + e); LOG.info("Excluding path: {}", e);
} }
localeUrlMatcher.setExcludedPaths(ex); localeUrlMatcher.setExcludedPaths(ex);
} }
String defaultLocale = filterConfig.getInitParameter("default-locale"); final String defaultLocale = filterConfig.getInitParameter("default-locale");
if (defaultLocale != null) { if (defaultLocale != null) {
this.defaultLocale = Locale.forLanguageTag(defaultLocale); this.defaultLocale = Locale.forLanguageTag(defaultLocale);
} else { } else {
this.defaultLocale = Locale.getDefault(); this.defaultLocale = Locale.getDefault();
} }
LOG.info("Using default locale: " + this.defaultLocale); LOG.info("Using default locale: {}", this.defaultLocale);
String allowedLocales = filterConfig.getInitParameter("allowed-locales"); final String allowedLocales = filterConfig.getInitParameter("allowed-locales");
if (StringUtils.isNotBlank(allowedLocales)) { if (StringUtils.isNotBlank(allowedLocales)) {
String[] ex = allowedLocales.split("\\s+"); final String[] ex = allowedLocales.split("\\s+");
for (String l : ex) { for (final String l : ex) {
LOG.info("Allowed locale: " + l); LOG.info("Allowed locale: {}", l);
} }
this.allowedLocales = ex; this.allowedLocales = ex;
} }
...@@ -80,9 +81,10 @@ public class LocaleURLFilter implements Filter { ...@@ -80,9 +81,10 @@ public class LocaleURLFilter implements Filter {
} }
@Override @Override
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException { public void doFilter(final ServletRequest servletRequest, final ServletResponse servletResponse, final FilterChain filterChain)
throws IOException, ServletException {
final HttpServletRequest httpRequest = (HttpServletRequest) servletRequest; final HttpServletRequest httpRequest = (HttpServletRequest) servletRequest;
HttpServletResponse httpResponse = (HttpServletResponse) servletResponse; final HttpServletResponse httpResponse = (HttpServletResponse) servletResponse;
final String url = httpRequest.getRequestURI().substring(httpRequest.getContextPath().length()); final String url = httpRequest.getRequestURI().substring(httpRequest.getContextPath().length());
if (localeUrlMatcher.isExcludedPath(url)) { if (localeUrlMatcher.isExcludedPath(url)) {
...@@ -91,17 +93,17 @@ public class LocaleURLFilter implements Filter { ...@@ -91,17 +93,17 @@ public class LocaleURLFilter implements Filter {
} }
if (LOG.isDebugEnabled()) { if (LOG.isDebugEnabled()) {
LOG.debug("Incoming URL: " + url); LOG.debug("Incoming URL: {}", url);
} }
final Matcher matcher = localeUrlMatcher.matcher(url); final Matcher matcher = localeUrlMatcher.matcher(url);
if (matcher.matches()) { if (matcher.matches()) {
String urlLanguage = matcher.group(1); final String urlLanguage = matcher.group(1);
String remainingUrl = matcher.group(2); final String remainingUrl = matcher.group(2);
if (this.allowedLocales != null) { if (this.allowedLocales != null) {
boolean localeAllowed = false; boolean localeAllowed = false;
for (String allowedLocale : this.allowedLocales) { for (final String allowedLocale : this.allowedLocales) {
if (allowedLocale.equalsIgnoreCase(urlLanguage)) { if (allowedLocale.equalsIgnoreCase(urlLanguage)) {
localeAllowed = true; localeAllowed = true;
break; break;
...@@ -115,11 +117,11 @@ public class LocaleURLFilter implements Filter { ...@@ -115,11 +117,11 @@ public class LocaleURLFilter implements Filter {
} }
} }
Locale urlLocale = Locale.forLanguageTag(urlLanguage); final Locale urlLocale = Locale.forLanguageTag(urlLanguage);
if (urlLocale.equals(this.defaultLocale)) { if (urlLocale.equals(this.defaultLocale)) {
String defaultLocaleUrl = getInternalUrl(remainingUrl, httpRequest.getQueryString()); final String defaultLocaleUrl = getInternalUrl(remainingUrl, httpRequest.getQueryString());
LOG.warn("Default locale requested, permanent-redirect to " + defaultLocaleUrl); LOG.info("Default locale requested, permanent-redirect to {}", defaultLocaleUrl);
httpResponse.reset(); httpResponse.reset();
httpResponse.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY); httpResponse.setStatus(HttpServletResponse.SC_MOVED_PERMANENTLY);
...@@ -131,37 +133,37 @@ public class LocaleURLFilter implements Filter { ...@@ -131,37 +133,37 @@ public class LocaleURLFilter implements Filter {
httpRequest.setAttribute(REQUEST_INTERNAL_URL, getInternalUrl(remainingUrl, httpRequest.getQueryString())); httpRequest.setAttribute(REQUEST_INTERNAL_URL, getInternalUrl(remainingUrl, httpRequest.getQueryString()));
if (LOG.isDebugEnabled()) { if (LOG.isDebugEnabled()) {
LOG.debug("URL matches! lang=" + urlLanguage + " remaining=" + remainingUrl); LOG.debug("URL matches! lang={} remaining={}", urlLanguage, remainingUrl);
LOG.debug("Country: " + urlLocale.getCountry() + " Lang: " + urlLocale.getLanguage() + " locale=" + urlLocale); LOG.debug("Country: {} Lang: {} locale={}", urlLocale.getCountry(), urlLocale.getLanguage(), urlLocale);
Enumeration<String> attrNames = httpRequest.getAttributeNames(); final Enumeration<String> attrNames = httpRequest.getAttributeNames();
while (attrNames.hasMoreElements()) { while (attrNames.hasMoreElements()) {
String attrName = attrNames.nextElement(); final String attrName = attrNames.nextElement();
LOG.debug("Request attr " + attrName + " = " + httpRequest.getAttribute(attrName)); LOG.debug("Request attr {} = {}", attrName, httpRequest.getAttribute(attrName));
} }
LOG.debug("Proxying request to remaining URL " + remainingUrl); LOG.debug("Proxying request to remaining URL {}", remainingUrl);
} }
LocaleWrappedServletResponse localeResponse = new LocaleWrappedServletResponse(httpResponse, localeUrlMatcher, urlLanguage, final LocaleWrappedServletResponse localeResponse = new LocaleWrappedServletResponse(httpResponse, localeUrlMatcher, urlLanguage,
defaultLocale.toLanguageTag()); defaultLocale.toLanguageTag());
LocaleWrappedServletRequest localeRequest = new LocaleWrappedServletRequest(httpRequest, url, remainingUrl); final LocaleWrappedServletRequest localeRequest = new LocaleWrappedServletRequest(httpRequest, url, remainingUrl);
// request.getRequestDispatcher(remainingUrl == null ? "/" : // request.getRequestDispatcher(remainingUrl == null ? "/" :
// remainingUrl).forward(servletRequest, localeResponse); // remainingUrl).forward(servletRequest, localeResponse);
filterChain.doFilter(localeRequest, localeResponse); filterChain.doFilter(localeRequest, localeResponse);
} else { } else {
if (LOG.isDebugEnabled()) { if (LOG.isDebugEnabled()) {
LOG.debug("No match on url " + url); LOG.debug("No match on url {}", url);
} }
httpRequest.setAttribute(REQUEST_INTERNAL_URL, getInternalUrl(url, httpRequest.getQueryString())); httpRequest.setAttribute(REQUEST_INTERNAL_URL, getInternalUrl(url, httpRequest.getQueryString()));
LocaleWrappedServletResponse localeResponse = new LocaleWrappedServletResponse(httpResponse, localeUrlMatcher, null, final LocaleWrappedServletResponse localeResponse = new LocaleWrappedServletResponse(httpResponse, localeUrlMatcher, null,
defaultLocale.toLanguageTag()); defaultLocale.toLanguageTag());
filterChain.doFilter(servletRequest, localeResponse); filterChain.doFilter(servletRequest, localeResponse);
} }
} }
private String getInternalUrl(String url, String queryString) { private String getInternalUrl(final String url, final String queryString) {
if (StringUtils.isBlank(queryString)) if (StringUtils.isBlank(queryString))
return url; return url;
else else
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment