From d790c2b6f2c0c6bca04d866b76cbdc124c70d7a6 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Fri, 14 Nov 2008 04:45:20 +0000 Subject: [PATCH] polish of portlet fix --- .../webflow/mvc/portlet/PortletMvcView.java | 12 --- .../mvc/portlet/PortletMvcViewFactory.java | 20 +---- .../webflow/mvc/view/AbstractMvcView.java | 76 ++++++++++--------- .../mvc/view/AbstractMvcViewFactory.java | 5 ++ .../ViewActionStateHolder.java} | 32 +++----- .../webflow/validation/ValidationHelper.java | 39 +++++++++- .../mvc/portlet/PortletMvcViewTests.java | 48 ++---------- 7 files changed, 98 insertions(+), 134 deletions(-) rename spring-webflow/src/main/java/org/springframework/webflow/mvc/{portlet/MappingResultsHolder.java => view/ViewActionStateHolder.java} (56%) diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcView.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcView.java index 9f98ff41..c8dcfa6c 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcView.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcView.java @@ -43,18 +43,6 @@ public class PortletMvcView extends AbstractMvcView { */ public PortletMvcView(org.springframework.web.servlet.View view, RequestContext context) { super(view, context); - - } - - /* - * Cache {@link MappingResultsHolder} attributes into flow scope so they can be accessed during the RenderRequest - * - * @see AbstractMvcView#processUserEvent() - */ - public void processUserEvent() { - super.processUserEvent(); - MappingResultsHolder holder = new MappingResultsHolder(getEventId(), getMappingResults(), getViewErrors()); - this.getRequestContext().getFlashScope().put(MappingResultsHolder.MAPPING_RESULTS_HOLDER_KEY, holder); } protected void doRender(Map model) throws Exception { diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcViewFactory.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcViewFactory.java index b263bbf9..a1baf554 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcViewFactory.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/PortletMvcViewFactory.java @@ -50,22 +50,4 @@ public class PortletMvcViewFactory extends AbstractMvcViewFactory { return new PortletMvcView(view, context); } - /* - * Populates attributes from {@link MappingResultsHolder}, if available, into the view. - * - * @see AbstractMvcViewFactory#getView(RequestContext) - */ - public org.springframework.webflow.execution.View getView(RequestContext context) { - org.springframework.webflow.execution.View view = super.getView(context); - if (view instanceof AbstractMvcView - && context.getFlashScope().contains(MappingResultsHolder.MAPPING_RESULTS_HOLDER_KEY)) { - AbstractMvcView mvcView = (AbstractMvcView) view; - MappingResultsHolder holder = (MappingResultsHolder) context.getFlashScope().get( - MappingResultsHolder.MAPPING_RESULTS_HOLDER_KEY); - mvcView.setEventId(holder.getEventId()); - mvcView.setMappingResults(holder.getMappingResults()); - mvcView.setViewErrors(holder.getViewErrors()); - } - return view; - } -} +} \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java index f2d690d4..56c5a2eb 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcView.java @@ -40,6 +40,7 @@ import org.springframework.binding.mapping.impl.DefaultMapper; import org.springframework.binding.mapping.impl.DefaultMapping; import org.springframework.binding.message.MessageBuilder; import org.springframework.binding.message.MessageResolver; +import org.springframework.core.style.ToStringCreator; import org.springframework.validation.BindingResult; import org.springframework.web.util.WebUtils; import org.springframework.webflow.core.collection.ParameterMap; @@ -81,8 +82,6 @@ public abstract class AbstractMvcView implements View { private MappingResults mappingResults; - private boolean viewErrors; - private BinderConfiguration binderConfiguration; /** @@ -159,6 +158,9 @@ public abstract class AbstractMvcView implements View { } model.put("currentUser", requestContext.getExternalContext().getCurrentUser()); try { + if (logger.isDebugEnabled()) { + logger.debug("Rendering MVC [" + view + "] with model map [" + model + "]"); + } doRender(model); } catch (IOException e) { throw e; @@ -185,29 +187,27 @@ public abstract class AbstractMvcView implements View { return; } if (logger.isDebugEnabled()) { - logger.debug("Obtained model " + model); + logger.debug("Resolved model " + model); } TransitionDefinition transition = requestContext.getMatchingTransition(eventId); if (shouldBind(model, transition)) { mappingResults = bind(model); if (hasErrors(mappingResults)) { - viewErrors = true; if (logger.isDebugEnabled()) { - logger.debug("Binding resulted in errors; adding error messages to context"); + logger.debug("Model binding resulted in errors; adding error messages to context"); } addErrorMessages(mappingResults); } } if (shouldValidate(model, transition)) { validate(model); - if (!viewErrors & requestContext.getMessageContext().hasErrorMessages()) { - viewErrors = true; - } } + requestContext.getFlashScope().put(ViewActionStateHolder.KEY, + new ViewActionStateHolder(eventId, mappingResults)); } public boolean hasFlowEvent() { - return eventId != null && !viewErrors; + return eventId != null && !requestContext.getMessageContext().hasErrorMessages(); } public Event getFlowEvent() { @@ -217,6 +217,10 @@ public abstract class AbstractMvcView implements View { return new Event(this, eventId, requestContext.getRequestParameters().asAttributeMap()); } + public String toString() { + return new ToStringCreator(this).append("view", view).toString(); + } + // subclassing hooks /** @@ -252,6 +256,14 @@ public abstract class AbstractMvcView implements View { return WebUtils.findParameterValue(context.getRequestParameters().asMap(), eventIdParameterName); } + /** + * Returns the id of the user event being processed. + * @return the user event + */ + protected String getEventId() { + return eventId; + } + /** * Determines if model data binding should be invoked given the Transition that matched the current user event being * processed. Returns true unless the bind attribute of the Transition has been set to false. @@ -267,6 +279,21 @@ public abstract class AbstractMvcView implements View { return transition.getAttributes().getBoolean("bind", Boolean.TRUE).booleanValue(); } + /** + * Returns the results of binding to the view's model, if model binding has occurred. + * @return the binding (mapping) results + */ + protected MappingResults getMappingResults() { + return mappingResults; + } + + // package private + + void restoreState(ViewActionStateHolder stateHolder) { + eventId = stateHolder.getEventId(); + mappingResults = stateHolder.getMappingResults(); + } + /** * Determines if model validation should execute given the Transition that matched the current user event being * processed. Returns true unless the validate attribute of the Transition has been set to false. @@ -321,7 +348,7 @@ public abstract class AbstractMvcView implements View { private MappingResults bind(Object model) { if (logger.isDebugEnabled()) { - logger.debug("Binding to model" + model); + logger.debug("Binding to model"); } DefaultMapper mapper = new DefaultMapper(); ParameterMap requestParameters = requestContext.getRequestParameters(); @@ -446,38 +473,12 @@ public abstract class AbstractMvcView implements View { private void validate(Object model) { if (logger.isDebugEnabled()) { - logger.debug("Validating model " + model); + logger.debug("Validating model"); } new ValidationHelper(model, requestContext, eventId, getModelExpression().getExpressionString(), expressionParser, mappingResults).validate(); } - // accessors for mapping results - - public String getEventId() { - return eventId; - } - - public void setEventId(String eventId) { - this.eventId = eventId; - } - - public MappingResults getMappingResults() { - return mappingResults; - } - - public void setMappingResults(MappingResults mappingResults) { - this.mappingResults = mappingResults; - } - - public boolean getViewErrors() { - return viewErrors; - } - - public void setViewErrors(boolean viewErrors) { - this.viewErrors = viewErrors; - } - private static class PropertyNotFoundError implements MappingResultsCriteria { public boolean test(MappingResult result) { return result.isError() && "propertyNotFound".equals(result.getCode()); @@ -519,4 +520,5 @@ public abstract class AbstractMvcView implements View { return "parameter:'" + parameterName + "'"; } } + } \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcViewFactory.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcViewFactory.java index 07cdc6f8..fb0c6f29 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcViewFactory.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/AbstractMvcViewFactory.java @@ -83,6 +83,11 @@ public abstract class AbstractMvcViewFactory implements ViewFactory { if (StringUtils.hasText(fieldMarkerPrefix)) { mvcView.setFieldMarkerPrefix(fieldMarkerPrefix); } + ViewActionStateHolder stateHolder = (ViewActionStateHolder) context.getFlashScope().get( + ViewActionStateHolder.KEY); + if (stateHolder != null) { + mvcView.restoreState(stateHolder); + } return mvcView; } diff --git a/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/MappingResultsHolder.java b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/ViewActionStateHolder.java similarity index 56% rename from spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/MappingResultsHolder.java rename to spring-webflow/src/main/java/org/springframework/webflow/mvc/view/ViewActionStateHolder.java index d11a049c..2f972735 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/mvc/portlet/MappingResultsHolder.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/mvc/view/ViewActionStateHolder.java @@ -13,53 +13,39 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.springframework.webflow.mvc.portlet; +package org.springframework.webflow.mvc.view; import org.springframework.binding.mapping.MappingResults; +import org.springframework.core.style.ToStringCreator; /** - * Holder class for mapping results to pass them from an ActionRequest to a RenderRequest + * Holder class for passing view state through a redirect. * * @author Scott Andrews */ -class MappingResultsHolder { +public class ViewActionStateHolder { - static final String MAPPING_RESULTS_HOLDER_KEY = "org.springframework.webflow.mvc.portlet.MAPPING_RESULTS_HOLDER"; + public static final String KEY = "webflowViewActionStateHolder"; private String eventId; private MappingResults mappingResults; - private boolean viewErrors; - - public MappingResultsHolder(String eventId, MappingResults mappingResults, boolean viewErrors) { + public ViewActionStateHolder(String eventId, MappingResults mappingResults) { this.eventId = eventId; this.mappingResults = mappingResults; - this.viewErrors = viewErrors; } public String getEventId() { return eventId; } - public void setEventId(String eventId) { - this.eventId = eventId; - } - public MappingResults getMappingResults() { return mappingResults; } - public void setMappingResults(MappingResults mappingResults) { - this.mappingResults = mappingResults; + public String toString() { + return new ToStringCreator(this).append("eventId", eventId).append("mappingResults", mappingResults).toString(); } - public boolean getViewErrors() { - return viewErrors; - } - - public void setViewErrors(boolean viewErrors) { - this.viewErrors = viewErrors; - } - -} +} \ No newline at end of file diff --git a/spring-webflow/src/main/java/org/springframework/webflow/validation/ValidationHelper.java b/spring-webflow/src/main/java/org/springframework/webflow/validation/ValidationHelper.java index 07cf89ef..10042781 100644 --- a/spring-webflow/src/main/java/org/springframework/webflow/validation/ValidationHelper.java +++ b/spring-webflow/src/main/java/org/springframework/webflow/validation/ValidationHelper.java @@ -18,6 +18,8 @@ package org.springframework.webflow.validation; import java.lang.reflect.Method; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; import org.springframework.binding.expression.ExpressionParser; import org.springframework.binding.mapping.MappingResults; @@ -25,6 +27,7 @@ import org.springframework.binding.message.MessageContext; import org.springframework.binding.message.MessageContextErrors; import org.springframework.binding.validation.ValidationContext; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; import org.springframework.validation.Errors; @@ -38,6 +41,8 @@ import org.springframework.webflow.execution.RequestContext; */ public class ValidationHelper { + private static final Log logger = LogFactory.getLog(ValidationHelper.class); + private final Object model; private final RequestContext requestContext; @@ -101,6 +106,9 @@ public class ValidationHelper { Method validateMethod = ReflectionUtils.findMethod(model.getClass(), methodName, new Class[] { ValidationContext.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking current state model validation method '" + methodName + "(ValidationContext)'"); + } ReflectionUtils.invokeMethod(validateMethod, model, new Object[] { new DefaultValidationContext( requestContext, eventId, mappingResults) }); return true; @@ -116,6 +124,9 @@ public class ValidationHelper { if (validateMethod != null) { MessageContextErrors errors = new MessageContextErrors(requestContext.getMessageContext(), modelName, model, expressionParser, mappingResults); + if (logger.isDebugEnabled()) { + logger.debug("Invoking current state model validation method '" + methodName + "(Errors)'"); + } ReflectionUtils.invokeMethod(validateMethod, model, new Object[] { errors }); return true; } @@ -127,6 +138,9 @@ public class ValidationHelper { Method validateMethod = ReflectionUtils.findMethod(model.getClass(), "validate", new Class[] { ValidationContext.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking default model validation method 'validate(ValidationContext)'"); + } ReflectionUtils.invokeMethod(validateMethod, model, new Object[] { new DefaultValidationContext( requestContext, eventId, mappingResults) }); return true; @@ -134,6 +148,9 @@ public class ValidationHelper { // mvc 2 compatibility only validateMethod = ReflectionUtils.findMethod(model.getClass(), "validate", new Class[] { Errors.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking default model validation method 'validate(Errors)'"); + } MessageContextErrors errors = new MessageContextErrors(requestContext.getMessageContext(), modelName, model, expressionParser, mappingResults); ReflectionUtils.invokeMethod(validateMethod, model, new Object[] { errors }); @@ -164,6 +181,11 @@ public class ValidationHelper { Method validateMethod = ReflectionUtils.findMethod(validator.getClass(), methodName, new Class[] { model.getClass(), ValidationContext.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking current state validator method '" + + ClassUtils.getShortName(validator.getClass()) + "." + methodName + "(" + + ClassUtils.getShortName(model.getClass()) + ", ValidationContext)'"); + } ReflectionUtils.invokeMethod(validateMethod, validator, new Object[] { model, new DefaultValidationContext(requestContext, eventId, mappingResults) }); return true; @@ -172,6 +194,11 @@ public class ValidationHelper { validateMethod = ReflectionUtils.findMethod(validator.getClass(), methodName, new Class[] { model.getClass(), Errors.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking current state validator method '" + + ClassUtils.getShortName(validator.getClass()) + "." + methodName + "(" + + ClassUtils.getShortName(model.getClass()) + ", Errors)'"); + } MessageContextErrors errors = new MessageContextErrors(requestContext.getMessageContext(), modelName, model, expressionParser, mappingResults); ReflectionUtils.invokeMethod(validateMethod, validator, new Object[] { model, errors }); @@ -191,6 +218,9 @@ public class ValidationHelper { private boolean invokeValidatorDefaultValidateMethod(Object model, Object validator) { if (validator instanceof Validator) { // supports existing validators + if (logger.isDebugEnabled()) { + logger.debug("Invoking Spring Validator '" + ClassUtils.getShortName(validator.getClass()) + "'"); + } MessageContextErrors errors = new MessageContextErrors(requestContext.getMessageContext(), modelName, model, expressionParser, mappingResults); ((Validator) validator).validate(model, errors); @@ -200,6 +230,10 @@ public class ValidationHelper { Method validateMethod = ReflectionUtils.findMethod(validator.getClass(), "validate", new Class[] { model.getClass(), ValidationContext.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking default validator method '" + ClassUtils.getShortName(validator.getClass()) + + ".validate(" + ClassUtils.getShortName(model.getClass()) + ", ValidationContext)'"); + } ReflectionUtils.invokeMethod(validateMethod, validator, new Object[] { model, new DefaultValidationContext(requestContext, eventId, mappingResults) }); return true; @@ -208,6 +242,10 @@ public class ValidationHelper { validateMethod = ReflectionUtils.findMethod(validator.getClass(), "validate", new Class[] { model.getClass(), Errors.class }); if (validateMethod != null) { + if (logger.isDebugEnabled()) { + logger.debug("Invoking default validator method '" + ClassUtils.getShortName(validator.getClass()) + + ".validate(" + ClassUtils.getShortName(model.getClass()) + ", Errors)'"); + } MessageContextErrors errors = new MessageContextErrors(requestContext.getMessageContext(), modelName, model, expressionParser, mappingResults); ReflectionUtils.invokeMethod(validateMethod, validator, new Object[] { model, errors }); @@ -215,5 +253,4 @@ public class ValidationHelper { } return false; } - } \ No newline at end of file diff --git a/spring-webflow/src/test/java/org/springframework/webflow/mvc/portlet/PortletMvcViewTests.java b/spring-webflow/src/test/java/org/springframework/webflow/mvc/portlet/PortletMvcViewTests.java index b9481db1..93728a91 100644 --- a/spring-webflow/src/test/java/org/springframework/webflow/mvc/portlet/PortletMvcViewTests.java +++ b/spring-webflow/src/test/java/org/springframework/webflow/mvc/portlet/PortletMvcViewTests.java @@ -1,11 +1,7 @@ package org.springframework.webflow.mvc.portlet; -import java.util.Map; - import javax.portlet.RenderRequest; import javax.portlet.RenderResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import junit.framework.TestCase; @@ -17,21 +13,16 @@ import org.springframework.mock.web.MockServletContext; import org.springframework.mock.web.portlet.MockPortletContext; import org.springframework.mock.web.portlet.MockRenderRequest; import org.springframework.mock.web.portlet.MockRenderResponse; -import org.springframework.web.servlet.View; import org.springframework.web.servlet.ViewRendererServlet; -import org.springframework.webflow.execution.RequestContext; import org.springframework.webflow.expression.DefaultExpressionParserFactory; import org.springframework.webflow.mvc.view.AbstractMvcView; +import org.springframework.webflow.mvc.view.ViewActionStateHolder; import org.springframework.webflow.mvc.view.MvcViewTests.BindBean; import org.springframework.webflow.test.MockFlowExecutionKey; import org.springframework.webflow.test.MockRequestContext; public class PortletMvcViewTests extends TestCase { - private boolean renderCalled; - - private Map model; - public void testRender() throws Exception { RenderRequest request = new MockRenderRequest(); RenderResponse response = new MockRenderResponse(); @@ -48,7 +39,7 @@ public class PortletMvcViewTests extends TestCase { assertNotNull(request.getAttribute(ViewRendererServlet.MODEL_ATTRIBUTE)); } - public void testResumeEventModelBindingFieldMarkerFieldPresent() throws Exception { + public void testResumeEvent() throws Exception { MockRequestContext context = new MockRequestContext(); context.putRequestParameter("_eventId", "submit"); context.putRequestParameter("booleanProperty", "true"); @@ -62,42 +53,15 @@ public class PortletMvcViewTests extends TestCase { context.getMockExternalContext().setNativeRequest(new MockHttpServletRequest()); context.getMockExternalContext().setNativeResponse(new MockHttpServletResponse()); context.getMockFlowExecutionContext().setKey(new MockFlowExecutionKey("c1v1")); - org.springframework.web.servlet.View mvcView = new MockView(); - AbstractMvcView view = new MockPortletMvcView(mvcView, context); + org.springframework.web.servlet.View mvcView = (org.springframework.web.servlet.View) EasyMock + .createMock(org.springframework.web.servlet.View.class); + AbstractMvcView view = new PortletMvcView(mvcView, context); view.setExpressionParser(DefaultExpressionParserFactory.getExpressionParser()); view.processUserEvent(); assertEquals(true, bindBean.getBooleanProperty()); - MappingResultsHolder holder = (MappingResultsHolder) context.getFlashScope().get( - MappingResultsHolder.MAPPING_RESULTS_HOLDER_KEY); + ViewActionStateHolder holder = (ViewActionStateHolder) context.getFlashScope().get(ViewActionStateHolder.KEY); assertEquals("submit", holder.getEventId()); assertNotNull(holder.getMappingResults()); - assertFalse(holder.getViewErrors()); - } - - private class MockPortletMvcView extends PortletMvcView { - - public MockPortletMvcView(View view, RequestContext context) { - super(view, context); - } - - protected void doRender(Map model) throws Exception { - getView().render(model, (HttpServletRequest) getRequestContext().getExternalContext().getNativeRequest(), - (HttpServletResponse) getRequestContext().getExternalContext().getNativeResponse()); - } - - } - - private class MockView implements View { - - public String getContentType() { - return "text/html"; - } - - public void render(Map model, HttpServletRequest request, HttpServletResponse response) throws Exception { - renderCalled = true; - model = model; - } - } }