From 5cba1e3a6aeb76eb0d91ace7c8616600c2727e63 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Tue, 22 Apr 2008 03:49:34 +0000 Subject: [PATCH] made renderResponse/responseComplete flags request scope to solve postback lifecycle problem commented out 3rdparty view-root corner case support - lets review this again if it needs to be in there... updated tests --- .../faces/webflow/FlowActionListener.java | 2 - .../faces/webflow/FlowFacesContext.java | 8 +-- .../faces/webflow/FlowViewHandler.java | 9 ++- .../faces/webflow/JsfView.java | 1 - .../faces/webflow/JsfViewFactory.java | 63 ++++++------------- .../webflow/FlowActionListenerTests.java | 5 +- .../webflow/JsfFinalResponseActionTests.java | 4 +- .../faces/webflow/JsfViewFactoryTests.java | 43 +++++++------ .../faces/webflow/JsfViewTests.java | 40 ++++++------ 9 files changed, 74 insertions(+), 101 deletions(-) diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/FlowActionListener.java b/spring-faces/src/main/java/org/springframework/faces/webflow/FlowActionListener.java index 95c4bb8b..6683c252 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/FlowActionListener.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/FlowActionListener.java @@ -49,7 +49,6 @@ public class FlowActionListener implements ActionListener { delegate.processAction(actionEvent); return; } - FacesContext context = FacesContext.getCurrentInstance(); ActionSource source = (ActionSource) actionEvent.getSource(); String result = null; @@ -68,6 +67,5 @@ public class FlowActionListener implements ActionListener { logger.debug("No action event detected"); context.getExternalContext().getRequestMap().remove(JsfView.EVENT_KEY); } - context.renderResponse(); } } diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/FlowFacesContext.java b/spring-faces/src/main/java/org/springframework/faces/webflow/FlowFacesContext.java index f2ba165f..de62f612 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/FlowFacesContext.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/FlowFacesContext.java @@ -151,7 +151,7 @@ public class FlowFacesContext extends FacesContext { } public boolean getRenderResponse() { - Boolean renderResponse = context.getFlashScope().getBoolean(RENDER_RESPONSE_KEY); + Boolean renderResponse = context.getRequestScope().getBoolean(RENDER_RESPONSE_KEY); if (renderResponse == null) { return false; } @@ -159,7 +159,7 @@ public class FlowFacesContext extends FacesContext { } public boolean getResponseComplete() { - Boolean responseComplete = context.getFlashScope().getBoolean(RESPONSE_COMPLETE_KEY); + Boolean responseComplete = context.getRequestScope().getBoolean(RESPONSE_COMPLETE_KEY); if (responseComplete == null) { return false; } @@ -167,11 +167,11 @@ public class FlowFacesContext extends FacesContext { } public void renderResponse() { - context.getFlashScope().put(RENDER_RESPONSE_KEY, Boolean.TRUE); + context.getRequestScope().put(RENDER_RESPONSE_KEY, Boolean.TRUE); } public void responseComplete() { - context.getFlashScope().put(RESPONSE_COMPLETE_KEY, Boolean.TRUE); + context.getRequestScope().put(RESPONSE_COMPLETE_KEY, Boolean.TRUE); } // ------------------ Pass-through delegate methods ----------------------// diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/FlowViewHandler.java b/spring-faces/src/main/java/org/springframework/faces/webflow/FlowViewHandler.java index 44a0b6d6..b5879869 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/FlowViewHandler.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/FlowViewHandler.java @@ -47,11 +47,16 @@ public class FlowViewHandler extends ViewHandler { } } - // ------------------- Pass-through delegate methods ------------------// public Locale calculateLocale(FacesContext context) { - return delegate.calculateLocale(context); + if (JsfUtils.isFlowRequest()) { + return RequestContextHolder.getRequestContext().getExternalContext().getLocale(); + } else { + return delegate.calculateLocale(context); + } } + // ------------------- Pass-through delegate methods ------------------// + public String calculateRenderKitId(FacesContext context) { return delegate.calculateRenderKitId(context); } diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java index 04c102d5..df6372fb 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfView.java @@ -104,7 +104,6 @@ public class JsfView implements View { if (restored && !facesContext.getResponseComplete() && !facesContext.getRenderResponse()) { facesLifecycle.execute(facesContext); validateModel(facesContext); - facesContext.renderResponse(); } } finally { facesContext.release(); diff --git a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java index c8ae6fdf..a20e2ff5 100644 --- a/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java +++ b/spring-faces/src/main/java/org/springframework/faces/webflow/JsfViewFactory.java @@ -65,51 +65,33 @@ public class JsfViewFactory implements ViewFactory { } public View getView(RequestContext context) { - FacesContext facesContext = FlowFacesContext.newInstance(context, lifecycle); try { - if (!facesContext.getRenderResponse()) { - JsfUtils.notifyBeforeListeners(PhaseId.RESTORE_VIEW, lifecycle, facesContext); - } - - JsfView view; - String viewName = resolveViewName(context); + JsfUtils.notifyBeforeListeners(PhaseId.RESTORE_VIEW, lifecycle, facesContext); ViewHandler viewHandler = facesContext.getApplication().getViewHandler(); - if (JsfUtils.isAtLeastJsf12() && !JsfUtils.isPortlet(facesContext)) { viewHandler.initView(facesContext); } - - if (viewExists(facesContext, viewName)) { + JsfView view; + String viewName = resolveViewName(context); + UIViewRoot viewRoot = viewHandler.restoreView(facesContext, viewName); + if (viewRoot != null) { if (logger.isDebugEnabled()) { - logger.debug("Existing view root found for '" + viewName + "'"); + logger.debug("View root restored for '" + viewName + "'"); } - view = createJsfView(facesContext.getViewRoot(), lifecycle, context); + view = createJsfView(viewRoot, lifecycle, context); + facesContext.setViewRoot(view.getViewRoot()); + processComponentBinding(facesContext, view.getViewRoot()); view.setRestored(true); } else { - UIViewRoot viewRoot = viewHandler.restoreView(facesContext, viewName); - if (viewRoot != null) { - if (logger.isDebugEnabled()) { - logger.debug("View root restored for '" + viewName + "'"); - } - view = createJsfView(viewRoot, lifecycle, context); - facesContext.setViewRoot(view.getViewRoot()); - processComponentBinding(facesContext, view.getViewRoot()); - view.setRestored(true); - } else { - if (logger.isDebugEnabled()) { - logger.debug("Creating view root for '" + viewName + "'"); - } - view = createJsfView(viewHandler.createView(facesContext, viewName), lifecycle, context); - facesContext.setViewRoot(view.getViewRoot()); - view.setRestored(true); + if (logger.isDebugEnabled()) { + logger.debug("Creating view root for '" + viewName + "'"); } + view = createJsfView(viewHandler.createView(facesContext, viewName), lifecycle, context); + facesContext.setViewRoot(view.getViewRoot()); + view.setRestored(true); } - - if (!facesContext.getRenderResponse()) { - JsfUtils.notifyAfterListeners(PhaseId.RESTORE_VIEW, lifecycle, facesContext); - } - + JsfUtils.notifyAfterListeners(PhaseId.RESTORE_VIEW, lifecycle, facesContext); return view; } finally { facesContext.release(); @@ -125,6 +107,7 @@ public class JsfViewFactory implements ViewFactory { } private boolean isSpringJavascriptAjaxRequest(ExternalContext context) { + // this is not that clean if (context.getNativeContext() instanceof ServletContext) { AjaxHandler handler = new SpringJavascriptAjaxHandler(); return handler.isAjaxRequest((ServletContext) context.getNativeContext(), (HttpServletRequest) context @@ -152,22 +135,14 @@ public class JsfViewFactory implements ViewFactory { } } - private boolean viewExists(FacesContext facesContext, String viewId) { - if (facesContext.getViewRoot() != null && facesContext.getViewRoot().getViewId().equals(viewId)) { - return true; - } - return false; - } - private void processComponentBinding(FacesContext context, UIComponent component) { ValueBinding binding = component.getValueBinding("binding"); if (binding != null) { binding.setValue(context, component); } - - Iterator i = component.getChildren().iterator(); - while (i.hasNext()) { - UIComponent child = (UIComponent) i.next(); + Iterator it = component.getChildren().iterator(); + while (it.hasNext()) { + UIComponent child = (UIComponent) it.next(); processComponentBinding(context, child); } } diff --git a/spring-faces/src/test/java/org/springframework/faces/webflow/FlowActionListenerTests.java b/spring-faces/src/test/java/org/springframework/faces/webflow/FlowActionListenerTests.java index 198dc060..6be2d885 100644 --- a/spring-faces/src/test/java/org/springframework/faces/webflow/FlowActionListenerTests.java +++ b/spring-faces/src/test/java/org/springframework/faces/webflow/FlowActionListenerTests.java @@ -1,7 +1,5 @@ package org.springframework.faces.webflow; -import org.easymock.EasyMock; - import javax.faces.component.UICommand; import javax.faces.context.FacesContext; import javax.faces.el.EvaluationException; @@ -11,6 +9,7 @@ import javax.faces.event.ActionEvent; import junit.framework.TestCase; +import org.easymock.EasyMock; import org.springframework.webflow.core.collection.AttributeMap; import org.springframework.webflow.core.collection.LocalAttributeMap; import org.springframework.webflow.execution.RequestContext; @@ -52,7 +51,6 @@ public class FlowActionListenerTests extends TestCase { JsfView.EVENT_KEY)); assertEquals("The event should be " + outcome, outcome, jsfMock.externalContext().getRequestMap().get( JsfView.EVENT_KEY)); - assertTrue("Render response flag should be set", jsfMock.facesContext().getRenderResponse()); } public final void testProcessAction_NullOutcome() { @@ -67,7 +65,6 @@ public class FlowActionListenerTests extends TestCase { assertFalse("An unexpected event was signaled", jsfMock.externalContext().getRequestMap().containsKey( JsfView.EVENT_KEY)); - assertTrue("Render response flag should be set", jsfMock.facesContext().getRenderResponse()); } private class MethodBindingStub extends MethodBinding { diff --git a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfFinalResponseActionTests.java b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfFinalResponseActionTests.java index adf32a69..53848550 100644 --- a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfFinalResponseActionTests.java +++ b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfFinalResponseActionTests.java @@ -78,8 +78,8 @@ public class JsfFinalResponseActionTests extends TestCase { ext.setNativeRequest(new MockHttpServletRequest()); ext.setNativeResponse(new MockHttpServletResponse()); EasyMock.expect(context.getExternalContext()).andStubReturn(ext); - AttributeMap flash = new LocalAttributeMap(); - EasyMock.expect(context.getFlashScope()).andStubReturn(flash); + AttributeMap requestMap = new LocalAttributeMap(); + EasyMock.expect(context.getRequestScope()).andStubReturn(requestMap); EasyMock.expect(context.getRequestParameters()).andStubReturn(new LocalParameterMap(new HashMap())); } diff --git a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewFactoryTests.java b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewFactoryTests.java index d5989bfc..0c4e1973 100644 --- a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewFactoryTests.java +++ b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewFactoryTests.java @@ -165,28 +165,27 @@ public class JsfViewFactoryTests extends TestCase { /** * Third party sets the view root before RESTORE_VIEW */ - public final void testGetView_ExternalViewRoot() { - - lifecycle = new NoExecutionLifecycle(jsfMock.lifecycle()); - factory = new JsfViewFactory(parser.parseExpression(VIEW_ID, new FluentParserContext().template().evaluate( - RequestContext.class).expectResult(String.class)), lifecycle); - - UIViewRoot newRoot = new UIViewRoot(); - newRoot.setViewId(VIEW_ID); - jsfMock.facesContext().setViewRoot(newRoot); - jsfMock.facesContext().renderResponse(); - - EasyMock.replay(new Object[] { context }); - - View newView = factory.getView(context); - - assertNotNull("A View was not created", newView); - assertTrue("A JsfView was expected", newView instanceof JsfView); - assertEquals("View name did not match", VIEW_ID, ((JsfView) newView).getViewRoot().getViewId()); - assertSame("View root was not the third party instance", newRoot, ((JsfView) newView).getViewRoot()); - assertFalse("An unexpected event was signaled,", newView.hasFlowEvent()); - } - + // public final void testGetView_ExternalViewRoot() { + // + // lifecycle = new NoExecutionLifecycle(jsfMock.lifecycle()); + // factory = new JsfViewFactory(parser.parseExpression(VIEW_ID, new FluentParserContext().template().evaluate( + // RequestContext.class).expectResult(String.class)), lifecycle); + // + // UIViewRoot newRoot = new UIViewRoot(); + // newRoot.setViewId(VIEW_ID); + // jsfMock.facesContext().setViewRoot(newRoot); + // jsfMock.facesContext().renderResponse(); + // + // EasyMock.replay(new Object[] { context }); + // + // View newView = factory.getView(context); + // + // assertNotNull("A View was not created", newView); + // assertTrue("A JsfView was expected", newView instanceof JsfView); + // assertEquals("View name did not match", VIEW_ID, ((JsfView) newView).getViewRoot().getViewId()); + // assertSame("View root was not the third party instance", newRoot, ((JsfView) newView).getViewRoot()); + // assertFalse("An unexpected event was signaled,", newView.hasFlowEvent()); + // } private class NoExecutionLifecycle extends FlowLifecycle { public NoExecutionLifecycle(Lifecycle delegate) { diff --git a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java index 486bca63..8901ee94 100644 --- a/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java +++ b/spring-faces/src/test/java/org/springframework/faces/webflow/JsfViewTests.java @@ -49,7 +49,7 @@ public class JsfViewTests extends TestCase { private RequestContext context = (RequestContext) EasyMock.createMock(RequestContext.class); private FlowExecutionContext flowExecutionContext = (FlowExecutionContext) EasyMock .createMock(FlowExecutionContext.class); - private MutableAttributeMap flashMap = (MutableAttributeMap) EasyMock.createMock(MutableAttributeMap.class); + private MutableAttributeMap requestMap = (MutableAttributeMap) EasyMock.createMock(MutableAttributeMap.class); private MutableAttributeMap flowMap = (MutableAttributeMap) EasyMock.createMock(MutableAttributeMap.class); private FlowExecutionKey key = new FlowExecutionKey() { @@ -90,7 +90,7 @@ public class JsfViewTests extends TestCase { RequestContextHolder.setRequestContext(context); EasyMock.expect(context.getExternalContext()).andStubReturn(extContext); - EasyMock.expect(context.getFlashScope()).andStubReturn(flashMap); + EasyMock.expect(context.getRequestScope()).andStubReturn(requestMap); EasyMock.expect(context.getFlowScope()).andStubReturn(flowMap); EasyMock.expect(context.getFlowExecutionContext()).andStubReturn(flowExecutionContext); EasyMock.expect(flowExecutionContext.getKey()).andStubReturn(key); @@ -104,12 +104,12 @@ public class JsfViewTests extends TestCase { public final void testRender() throws IOException { - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) .andStubReturn(null); - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY), EasyMock.anyObject())) .andStubReturn(null); - EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashMap }); + EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, requestMap }); view.render(); @@ -118,12 +118,12 @@ public class JsfViewTests extends TestCase { public final void testRenderException() throws IOException { - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) .andStubReturn(null); - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY), EasyMock.anyObject())) .andStubReturn(null); - EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashMap }); + EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, requestMap }); jsfMock.application().setViewHandler(new ExceptionalViewHandler()); @@ -139,11 +139,11 @@ public class JsfViewTests extends TestCase { */ public final void testResume_Restored_NoEvent() { - EasyMock.expect(flashMap.getBoolean(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY))).andStubReturn( + EasyMock.expect(requestMap.getBoolean(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY))).andStubReturn( Boolean.FALSE); - EasyMock.expect(flashMap.getBoolean(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY))).andStubReturn( + EasyMock.expect(requestMap.getBoolean(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY))).andStubReturn( Boolean.FALSE); - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) .andStubReturn(null); EasyMock.expect(context.getCurrentState()).andStubReturn(new MockViewState()); @@ -152,7 +152,7 @@ public class JsfViewTests extends TestCase { UIViewRoot existingRoot = new UIViewRoot(); existingRoot.setViewId(VIEW_ID); - EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashMap }); + EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, requestMap }); JsfView restoredView = new JsfView(existingRoot, lifecycle, context); restoredView.setRestored(true); @@ -169,11 +169,11 @@ public class JsfViewTests extends TestCase { */ public final void testGetView_Restore_Ajax_NoEvent() { - EasyMock.expect(flashMap.getBoolean(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY))).andStubReturn( + EasyMock.expect(requestMap.getBoolean(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY))).andStubReturn( Boolean.FALSE); - EasyMock.expect(flashMap.getBoolean(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY))).andStubReturn( + EasyMock.expect(requestMap.getBoolean(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY))).andStubReturn( Boolean.FALSE); - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) .andStubReturn(null); EasyMock.expect(context.getCurrentState()).andStubReturn(new MockViewState()); @@ -183,7 +183,7 @@ public class JsfViewTests extends TestCase { existingRoot.setViewId(VIEW_ID); AjaxViewRoot ajaxRoot = new AjaxViewRoot(existingRoot); - EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashMap }); + EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, requestMap }); JsfView restoredView = new JsfView(ajaxRoot, lifecycle, context); restoredView.setRestored(true); @@ -199,11 +199,11 @@ public class JsfViewTests extends TestCase { */ public final void testGetView_Restore_EventSignaled() { - EasyMock.expect(flashMap.getBoolean(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY))).andStubReturn( + EasyMock.expect(requestMap.getBoolean(EasyMock.matches(FlowFacesContext.RESPONSE_COMPLETE_KEY))).andStubReturn( Boolean.FALSE); - EasyMock.expect(flashMap.getBoolean(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY))).andStubReturn( + EasyMock.expect(requestMap.getBoolean(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY))).andStubReturn( Boolean.FALSE); - EasyMock.expect(flashMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) + EasyMock.expect(requestMap.put(EasyMock.matches(FlowFacesContext.RENDER_RESPONSE_KEY), EasyMock.anyObject())) .andStubReturn(null); EasyMock.expect(context.getCurrentState()).andStubReturn(new MockViewState()); @@ -212,7 +212,7 @@ public class JsfViewTests extends TestCase { UIViewRoot existingRoot = new UIViewRoot(); existingRoot.setViewId(VIEW_ID); - EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, flashMap }); + EasyMock.replay(new Object[] { context, flowExecutionContext, flowMap, requestMap }); JsfView restoredView = new JsfView(existingRoot, lifecycle, context); restoredView.setRestored(true);