Merge pull request #725 from renanigt/validationIgnoredOnRedirects

Fixing problem: validation errors ignored on redirects. Closes #476

vraptor-core/src/main/java/br/com/caelum/vraptor/core/DefaultResult.java

@@ -34,6 +34,7 @@
 import br.com.caelum.vraptor.View;
 import br.com.caelum.vraptor.interceptor.TypeNameExtractor;
 import br.com.caelum.vraptor.ioc.Container;
+import br.com.caelum.vraptor.validator.Messages;
 
 /**
  * A basic implementation of a Result
@@ -48,28 +49,34 @@ public class DefaultResult extends AbstractResult {
 	private final Container container;
 	private final ExceptionMapper exceptions;
 	private final TypeNameExtractor extractor;
+	private final Messages messages;
 
 	private Map<String, Object> includedAttributes;
 	private boolean responseCommitted = false;
+
 
 	/** 
 	 * @deprecated CDI eyes only
 	 */
 	protected DefaultResult() {
-		this(null, null, null, null);
+		this(null, null, null, null, null);
 	}
 
 	@Inject
-	public DefaultResult(HttpServletRequest request, Container container, ExceptionMapper exceptions, TypeNameExtractor extractor) {
+	public DefaultResult(HttpServletRequest request, Container container, ExceptionMapper exceptions, TypeNameExtractor extractor,
+			Messages messages) {
 		this.request = request;
 		this.container = container;
 		this.extractor = extractor;
 		this.includedAttributes = new HashMap<>();
 		this.exceptions = exceptions;
+		this.messages = messages;
 	}
 
 	@Override
 	public <T extends View> T use(Class<T> view) {
+		messages.assertAbsenceOfErrors();
+
 		responseCommitted = true;
 		return container.instanceFor(view);
 	}
@@ -107,4 +114,5 @@ public Result include(Object value) {
 		String key = extractor.nameFor(value.getClass());
 		return include(key, value);
 	}
+
 }

vraptor-core/src/main/java/br/com/caelum/vraptor/observer/ExecuteMethod.java

@@ -17,27 +17,29 @@
 
 package br.com.caelum.vraptor.observer;
 
+import static org.slf4j.LoggerFactory.getLogger;
+
+import java.lang.reflect.Method;
+
+import javax.enterprise.context.Dependent;
+import javax.enterprise.event.Event;
+import javax.enterprise.event.Observes;
+import javax.inject.Inject;
+
+import net.vidageek.mirror.dsl.Mirror;
+import net.vidageek.mirror.exception.ReflectionProviderException;
+
+import org.slf4j.Logger;
+
 import br.com.caelum.vraptor.InterceptionException;
 import br.com.caelum.vraptor.controller.ControllerMethod;
 import br.com.caelum.vraptor.core.MethodInfo;
 import br.com.caelum.vraptor.events.InterceptorsExecuted;
 import br.com.caelum.vraptor.events.MethodExecuted;
 import br.com.caelum.vraptor.events.MethodReady;
 import br.com.caelum.vraptor.interceptor.ApplicationLogicException;
+import br.com.caelum.vraptor.validator.Messages;
 import br.com.caelum.vraptor.validator.ValidationException;
-import br.com.caelum.vraptor.validator.Validator;
-import net.vidageek.mirror.dsl.Mirror;
-import net.vidageek.mirror.exception.ReflectionProviderException;
-import org.slf4j.Logger;
-
-import javax.enterprise.context.Dependent;
-import javax.enterprise.event.Event;
-import javax.enterprise.event.Observes;
-import javax.inject.Inject;
-import java.lang.reflect.Method;
-
-import static br.com.caelum.vraptor.view.Results.nothing;
-import static org.slf4j.LoggerFactory.getLogger;
 
 /**
  * Interceptor that executes the logic method.
@@ -52,7 +54,7 @@ public class ExecuteMethod {
 	private final static Logger log = getLogger(ExecuteMethod.class);
 
 	private final MethodInfo methodInfo;
-	private final Validator validator;
+	private final Messages messages;
 
 	private Event<MethodExecuted> methodExecutedEvent;
 	private Event<MethodReady> methodReady;
@@ -65,10 +67,10 @@ protected ExecuteMethod() {
 	}
 
 	@Inject
-	public ExecuteMethod(MethodInfo methodInfo, Validator validator, 
+	public ExecuteMethod(MethodInfo methodInfo, Messages messages, 
 			Event<MethodExecuted> methodExecutedEvent, Event<MethodReady> methodReady) {
 		this.methodInfo = methodInfo;
-		this.validator = validator;
+		this.messages = messages;
 		this.methodExecutedEvent = methodExecutedEvent;
 		this.methodReady = methodReady;
 	}
@@ -84,22 +86,8 @@ public void execute(@Observes InterceptorsExecuted event) {
 			Object instance = event.getControllerInstance();
 			Object result = new Mirror().on(instance).invoke().method(reflectionMethod).withArgs(parameters);
 
-			if (validator.hasErrors()) { // method should have thrown
-											// ValidationException
-				if (log.isDebugEnabled()) {
-					try {
-						validator.onErrorUse(nothing());
-					} catch (ValidationException e) {
-						log.debug("Some validation errors occured: {}", e.getErrors());
-					}
-				}
-				throw new InterceptionException(
-						"There are validation errors and you forgot to specify where to go. Please add in your method "
-								+ "something like:\n"
-								+ "validator.onErrorUse(page()).of(AnyController.class).anyMethod();\n"
-								+ "or any view that you like.\n"
-								+ "If you didn't add any validation error, it is possible that a conversion error had happened.");
-			}
+			messages.assertAbsenceOfErrors();
+
 			this.methodInfo.setResult(result);
 			methodExecutedEvent.fire(new MethodExecuted(method, methodInfo));
 		} catch (IllegalArgumentException e) {

vraptor-core/src/main/java/br/com/caelum/vraptor/validator/DefaultValidator.java

@@ -143,7 +143,7 @@ public <T extends View> T onErrorUse(Class<T> view) {
 		outjector.outjectRequestMap();
 
 		logger.debug("there are errors on result: {}", getErrors());
-		return viewsFactory.instanceFor(view, getErrors());
+		return viewsFactory.instanceFor(view, messages.handleErrors());
 	}
 
 	@Override

vraptor-core/src/main/java/br/com/caelum/vraptor/validator/Messages.java

@@ -15,6 +15,8 @@
  */
 package br.com.caelum.vraptor.validator;
 
+import static org.slf4j.LoggerFactory.getLogger;
+
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -23,6 +25,8 @@
 import javax.enterprise.context.RequestScoped;
 import javax.inject.Named;
 
+import org.slf4j.Logger;
+
 /**
  * Managed class that stores all application messages like errors, warnings and info. This
  * class is useful to display messages categorized by severity in your view. To choose a severity
@@ -40,10 +44,16 @@
 @RequestScoped
 public class Messages {
 
+	private final static Logger log = getLogger(Messages.class);
+
 	private Map<Severity, List<Message>> messages = new HashMap<>();
+	private boolean unhandledErrors = false;
 
 	public Messages add(Message message) {
 		get(message.getSeverity()).add(message);
+		if(Severity.ERROR.equals(message.getSeverity())) {
+			unhandledErrors = true;
+		}
 		return this;
 	}
 
@@ -83,4 +93,27 @@ public List<Message> getAll() {
 	private MessageList createMessageList() {
 		return new MessageList(new ArrayList<Message>());
 	}
+
+	public List<Message> handleErrors() {
+		unhandledErrors = false;
+		return getErrors();
+	}
+
+	public boolean hasUnhandledErrors() {
+		return unhandledErrors;
+	}
+
+	public void assertAbsenceOfErrors() {
+		if (hasUnhandledErrors()) {
+			log.debug("Some validation errors occured: {}", getErrors());
+
+			throw new IllegalStateException(
+				"There are validation errors and you forgot to specify where to go. Please add in your method "
+				+ "something like:\n"
+				+ "validator.onErrorUse(page()).of(AnyController.class).anyMethod();\n"
+				+ "or any view that you like.\n"
+				+ "If you didn't add any validation error, it is possible that a conversion error had happened.");
+		}
+	}
+
 }

vraptor-core/src/test/java/br/com/caelum/vraptor/core/DefaultResultTest.java

@@ -40,31 +40,34 @@
 import br.com.caelum.vraptor.interceptor.TypeNameExtractor;
 import br.com.caelum.vraptor.ioc.Container;
 import br.com.caelum.vraptor.proxy.WeldProxifier;
+import br.com.caelum.vraptor.validator.Messages;
 import br.com.caelum.vraptor.view.DefaultHttpResultTest.RandomController;
 import br.com.caelum.vraptor.view.LogicResult;
 import br.com.caelum.vraptor.view.PageResult;
+import br.com.caelum.vraptor.view.Results;
 import br.com.caelum.vraptor.view.Status;
 
 public class DefaultResultTest {
 
 	@Mock private HttpServletRequest request;
 	@Mock private Container container;
 	@Mock private TypeNameExtractor extractor;
+	@Mock private Messages messages;
 
 	private Result result;
 	private WeldProxifier weldProxifier;
 
 	@Before
 	public void setup() {
 		MockitoAnnotations.initMocks(this);
-		result = new DefaultResult(request, container, null, extractor);
+		result = new DefaultResult(request, container, null, extractor, messages);
 		weldProxifier = new WeldProxifier();
 	}
 
 	public static class MyView implements View {
 
 	}
-
+	
 	@Test
 	public void shouldUseContainerForNewView() {
 		final MyView expectedView = new MyView();
@@ -126,8 +129,8 @@ public void shouldDelegateToLogicResultOnForwardToLogic() throws Exception {
 		result.forwardTo(RandomController.class);
 
 		verify(logicResult).forwardTo(RandomController.class);
-
 	}
+
 	@Test
 	public void shouldDelegateToLogicResultOnRedirectToLogic() throws Exception {
 
@@ -136,8 +139,8 @@ public void shouldDelegateToLogicResultOnRedirectToLogic() throws Exception {
 		result.redirectTo(RandomController.class);
 
 		verify(logicResult).redirectTo(RandomController.class);
-
 	}
+
 	@Test
 	public void shouldDelegateToLogicResultOnRedirectToLogicWithInstance() throws Exception {
 
@@ -146,7 +149,6 @@ public void shouldDelegateToLogicResultOnRedirectToLogicWithInstance() throws Ex
 		result.redirectTo(new RandomController());
 
 		verify(logicResult).redirectTo(RandomController.class);
-
 	}
 
 	@Test
@@ -157,8 +159,8 @@ public void shouldDelegateToLogicResultOnForwardToLogicWithInstance() throws Exc
 		result.forwardTo(new RandomController());
 
 		verify(logicResult).forwardTo(RandomController.class);
-
 	}
+
 	@Test
 	public void shouldDelegateToPageResultOnPageOfWithInstance() throws Exception {
 
@@ -167,7 +169,6 @@ public void shouldDelegateToPageResultOnPageOfWithInstance() throws Exception {
 		result.of(new RandomController());
 
 		verify(pageResult).of(RandomController.class);
-
 	}
 
 	@Test
@@ -205,7 +206,6 @@ public void shouldDelegateToStatusOnNotFound() throws Exception {
 		result.notFound();
 
 		verify(status).notFound();
-
 	}
 
 	@Test
@@ -216,7 +216,6 @@ public void shouldDelegateToStatusOnPermanentlyRedirectToUri() throws Exception
 		result.permanentlyRedirectTo("url");
 
 		verify(status).movedPermanentlyTo("url");
-
 	}
 
 	@Test
@@ -227,7 +226,6 @@ public void shouldDelegateToStatusOnPermanentlyRedirectToControllerClass() throw
 		result.permanentlyRedirectTo(RandomController.class);
 
 		verify(status).movedPermanentlyTo(RandomController.class);
-
 	}
 
 	@Test
@@ -238,7 +236,12 @@ public void shouldDelegateToStatusOnPermanentlyRedirectToControllerInstance() th
 		result.permanentlyRedirectTo(new RandomController());
 
 		verify(status).movedPermanentlyTo(RandomController.class);
-
+	}
+
+	@Test
+	public void shouldCallAssertAbsenceOfErrorsMethodFromMessages() throws Exception {
+		result.use(Results.json());
+		verify(messages).assertAbsenceOfErrors();
 	}