You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by lu...@apache.org on 2010/12/01 19:59:33 UTC

svn commit: r1041132 - in /myfaces/core/trunk/api/src: main/java/javax/faces/component/UIViewRoot.java test/java/javax/faces/component/UIViewRootTest.java

Author: lu4242
Date: Wed Dec  1 18:59:32 2010
New Revision: 1041132

URL: http://svn.apache.org/viewvc?rev=1041132&view=rev
Log:
MYFACES-2882 3.4.2.6 Event Broadcasting: processing events during the current lifecycle phase doesn't process additional events (thanks to Martin Koci for provide this patch)

Modified:
    myfaces/core/trunk/api/src/main/java/javax/faces/component/UIViewRoot.java
    myfaces/core/trunk/api/src/test/java/javax/faces/component/UIViewRootTest.java

Modified: myfaces/core/trunk/api/src/main/java/javax/faces/component/UIViewRoot.java
URL: http://svn.apache.org/viewvc/myfaces/core/trunk/api/src/main/java/javax/faces/component/UIViewRoot.java?rev=1041132&r1=1041131&r2=1041132&view=diff
==============================================================================
--- myfaces/core/trunk/api/src/main/java/javax/faces/component/UIViewRoot.java (original)
+++ myfaces/core/trunk/api/src/main/java/javax/faces/component/UIViewRoot.java Wed Dec  1 18:59:32 2010
@@ -34,10 +34,12 @@ import java.util.logging.Logger;
 import javax.el.MethodExpression;
 import javax.el.ValueExpression;
 import javax.faces.FactoryFinder;
+import javax.faces.application.ProjectStage;
 import javax.faces.context.ExternalContext;
 import javax.faces.context.FacesContext;
 import javax.faces.context.PartialViewContext;
 import javax.faces.event.AbortProcessingException;
+import javax.faces.event.ActionEvent;
 import javax.faces.event.ExceptionQueuedEvent;
 import javax.faces.event.ExceptionQueuedEventContext;
 import javax.faces.event.FacesEvent;
@@ -246,32 +248,62 @@ public class UIViewRoot extends UICompon
         {
             return;
         }
-
-        // Gather the events and purge the event list to prevent concurrent modification during broadcasting
-        List<FacesEvent> anyPhase = new ArrayList<FacesEvent>(_events.size());
-        List<FacesEvent> onPhase = new ArrayList<FacesEvent>(_events.size());
-        for (Iterator<FacesEvent> iterator = _events.iterator(); iterator.hasNext();)
+        
+        Events events = _getEvents(phaseId);
+        
+        // Spec. 3.4.2.6 Event Broadcasting:
+        // Queue one or more additional events, from the same source component or a different one, for processing during the
+        // current lifecycle phase.
+        
+        // Unfortunately with that requirement it is easy to create infinite loop in processing. One example can be:
+        //
+        // public processAction(ActionEvent actionEvent)
+        // {
+        // actionEvent  = new ActionEvent(actionEvent.getComponent());
+        // actionEvent.queue();
+        // }
+        // 
+        // Thus we iterate here only 15x. If iteration overreachs 15 we output a warning  
+        
+        int loops = 0;
+        int maxLoops = 15;
+        do
         {
-            FacesEvent event = iterator.next();
-            if (event.getPhaseId().equals(PhaseId.ANY_PHASE))
-            {
-                anyPhase.add(event);
-                iterator.remove();
-            }
-            else if (event.getPhaseId().equals(phaseId))
+            // First broadcast events that have been queued for PhaseId.ANY_PHASE.
+            if (_broadcastAll(context, events.getAnyPhase()))
             {
-                onPhase.add(event);
-                iterator.remove();
+                _broadcastAll(context, events.getOnPhase());
             }
-        }
 
-        // First broadcast events that have been queued for PhaseId.ANY_PHASE.
-        if (_broadcastAll(context, anyPhase))
-        {
-            _broadcastAll(context, onPhase);
+            events = _getEvents(phaseId);
+            loops++;
+            
+        } while (events.hasMoreEvents() && loops < maxLoops);
+        
+        if (loops == maxLoops && events.hasMoreEvents()) {
+            // broadcast reach maxLoops - probably a infinitive recursion:
+            boolean production = getFacesContext().isProjectStage(ProjectStage.Production);
+            Level level = production ? Level.FINE : Level.WARNING;
+            if (logger.isLoggable(level)) {
+                List<String> name = new ArrayList<String>(events.getAnyPhase().size() + events.getOnPhase().size());
+                for (FacesEvent facesEvent : events.getAnyPhase())
+                {
+                    String clientId = facesEvent.getComponent().getClientId(getFacesContext());
+                    name.add(clientId);
+                }
+                for (FacesEvent facesEvent : events.getOnPhase())
+                {
+                    String clientId = facesEvent.getComponent().getClientId(getFacesContext());
+                    name.add(clientId);
+                }
+                logger.log(level, "Event broadcating for PhaseId {0} at UIViewRoot {1} reaches maximal limit, please check " +
+                        "listeners for infinite recursion. Component id: {2}",
+                        new Object [] {phaseId, getViewId(), name});
+            }
         }
     }
 
+
     /**
      * Provides a unique id for this component instance.
      */
@@ -1290,6 +1322,34 @@ public class UIViewRoot extends UICompon
         super.processValidators(context);
     }
 
+    /**
+     * Gathers all event for current and ANY phase
+     * @param phaseId current phase id
+     */
+    private Events _getEvents(PhaseId phaseId) {
+        // Gather the events and purge the event list to prevent concurrent modification during broadcasting
+        List<FacesEvent> anyPhase = new ArrayList<FacesEvent>(
+                _events.size());
+        List<FacesEvent> onPhase = new ArrayList<FacesEvent>(_events.size());
+        for (Iterator<FacesEvent> iterator = _events.iterator(); iterator
+                .hasNext();)
+        {
+            FacesEvent event = iterator.next();
+            if (event.getPhaseId().equals(PhaseId.ANY_PHASE))
+            {
+                anyPhase.add(event);
+                iterator.remove();
+            }
+            else if (event.getPhaseId().equals(phaseId))
+            {
+                onPhase.add(event);
+                iterator.remove();
+            }
+        }
+        
+        return new Events(anyPhase, onPhase);
+    }
+
     private static interface PhaseProcessor
     {
         public void process(FacesContext context, UIViewRoot root);
@@ -1410,4 +1470,36 @@ public class UIViewRoot extends UICompon
         }
         
     }
+
+    /**
+     * Agregates events for ANY_PHASE and current phase 
+     */
+    private class Events {
+        
+        private final List<FacesEvent> _anyPhase;
+        
+        private final List<FacesEvent> _onPhase;
+        
+        public Events(List<FacesEvent> anyPhase, List<FacesEvent> onPhase)
+        {
+            super();
+            this._anyPhase = anyPhase;
+            this._onPhase = onPhase;
+        }
+
+        public boolean hasMoreEvents()
+        {
+            return (_anyPhase != null && _anyPhase.size() > 0) || (_onPhase != null && _onPhase.size() > 0); 
+        }
+
+        public List<FacesEvent> getAnyPhase()
+        {
+            return _anyPhase;
+        }
+
+        public List<FacesEvent> getOnPhase()
+        {
+            return _onPhase;
+        }
+    }
 }

Modified: myfaces/core/trunk/api/src/test/java/javax/faces/component/UIViewRootTest.java
URL: http://svn.apache.org/viewvc/myfaces/core/trunk/api/src/test/java/javax/faces/component/UIViewRootTest.java?rev=1041132&r1=1041131&r2=1041132&view=diff
==============================================================================
--- myfaces/core/trunk/api/src/test/java/javax/faces/component/UIViewRootTest.java (original)
+++ myfaces/core/trunk/api/src/test/java/javax/faces/component/UIViewRootTest.java Wed Dec  1 18:59:32 2010
@@ -18,50 +18,58 @@
  */
 package javax.faces.component;
 
-import org.apache.myfaces.TestRunner;
-import org.apache.myfaces.test.mock.MockFacesContext12;
-import org.apache.myfaces.test.mock.MockFacesContext20;
-import org.easymock.IAnswer;
-import org.easymock.classextension.EasyMock;
-import org.easymock.classextension.IMocksControl;
-import org.testng.annotations.AfterMethod;
-import org.testng.annotations.BeforeMethod;
-import org.testng.annotations.Test;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.aryEq;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.getCurrentArguments;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.fail;
+
+import java.lang.reflect.Method;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Locale;
+import java.util.Map;
 
 import javax.el.ELContext;
 import javax.el.MethodExpression;
 import javax.faces.FactoryFinder;
 import javax.faces.application.Application;
+import javax.faces.application.ProjectStage;
 import javax.faces.application.ViewHandler;
 import javax.faces.context.ExternalContext;
+import javax.faces.event.AbortProcessingException;
+import javax.faces.event.ActionEvent;
+import javax.faces.event.ActionListener;
 import javax.faces.event.PhaseEvent;
 import javax.faces.event.PhaseId;
 import javax.faces.event.PhaseListener;
 import javax.faces.lifecycle.Lifecycle;
 import javax.faces.lifecycle.LifecycleFactory;
 import javax.faces.webapp.FacesServlet;
-import java.lang.reflect.Method;
-import java.util.Collection;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Locale;
-import java.util.Map;
 
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.aryEq;
-import static org.easymock.EasyMock.eq;
-import static org.easymock.EasyMock.expect;
-import static org.easymock.EasyMock.getCurrentArguments;
-import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertNull;
-import static org.testng.Assert.fail;
+import org.apache.myfaces.TestRunner;
+import org.apache.myfaces.test.base.junit4.AbstractJsfTestCase;
+import org.apache.myfaces.test.mock.MockFacesContext12;
+import org.easymock.IAnswer;
+import org.easymock.classextension.EasyMock;
+import org.easymock.classextension.IMocksControl;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+
+
 
 /**
  * @author Mathias Broekelmann (latest modification by $Author$)
  * @version $Revision$ $Date$
  */
-public class UIViewRootTest
+public class UIViewRootTest extends AbstractJsfTestCase
 {
     private Map<PhaseId, Class<? extends PhaseListener>> phaseListenerClasses;
     private IMocksControl _mocksControl;
@@ -76,9 +84,10 @@ public class UIViewRootTest
 
     private static ThreadLocal<LifecycleFactory> LIFECYCLEFACTORY = new ThreadLocal<LifecycleFactory>();
 
-    @BeforeMethod(alwaysRun=true)
-    protected void setUp() throws Exception
+    @Before
+    public void setUp() throws Exception
     {
+        super.setUp();
         phaseListenerClasses = new HashMap<PhaseId, Class<? extends PhaseListener>>();
         phaseListenerClasses.put(PhaseId.APPLY_REQUEST_VALUES, ApplyRequesValuesPhaseListener.class);
         phaseListenerClasses.put(PhaseId.PROCESS_VALIDATIONS, ProcessValidationsPhaseListener.class);
@@ -88,7 +97,7 @@ public class UIViewRootTest
 
         _mocksControl = EasyMock.createControl();
         _externalContext = _mocksControl.createMock(ExternalContext.class);
-        _facesContext = new MockFacesContext20(_externalContext);
+        _facesContext = (MockFacesContext12) facesContext;
         _application = _mocksControl.createMock(Application.class);
         _lifecycleFactory = _mocksControl.createMock(LifecycleFactory.class);
         _testimpl = new UIViewRoot();
@@ -99,11 +108,14 @@ public class UIViewRootTest
 
         LIFECYCLEFACTORY.set(_lifecycleFactory);
         FactoryFinder.setFactory(FactoryFinder.LIFECYCLE_FACTORY, MockLifeCycleFactory.class.getName());
+        servletContext.addInitParameter(ProjectStage.PROJECT_STAGE_PARAM_NAME, ProjectStage.UnitTest.name());
+        
     }
 
-    @AfterMethod(alwaysRun=true)
-    protected void tearDown() throws Exception
+    @After
+    public void tearDown() throws Exception
     {
+        super.tearDown();
         _mocksControl.reset();
     }
 
@@ -483,6 +495,31 @@ public class UIViewRootTest
         _mocksControl.verify();
     }
 
+    private final class ActionListenerImplementation implements ActionListener
+    {
+        public int invocationCount = 0;
+        
+        public ActionEvent newActionEventFromListener;
+
+        public ActionListenerImplementation(UICommand otherUiCommand)
+        {
+            // from spec: Queue one or more additional events, from the same source component
+            // or a DIFFERENT one
+            newActionEventFromListener = new ActionEvent(otherUiCommand);
+        }
+
+        public void processAction(ActionEvent actionEvent)
+                throws AbortProcessingException
+        {
+            invocationCount++;
+              
+            newActionEventFromListener.queue();
+            
+            // Simulate infinite recursion,most likely coding error:
+            actionEvent.queue();
+        }
+    }
+
     public static class MockLifeCycleFactory extends LifecycleFactory
     {
 
@@ -562,4 +599,41 @@ public class UIViewRootTest
         }
     }
 
+    @Test
+    public void testBroadcastEvents()
+    {
+        
+        UICommand uiCommand = new UICommand();
+        uiCommand.setId("idOfCommandOne");
+        facesContext.getViewRoot().getChildren().add(uiCommand);
+        
+        // Spec 3.4.2.6 Event Broadcasting: During event broadcasting, a listener processing an event may
+        // Queue one or more additional events from the same source component or a different one:
+        // and the DIFFERENT ONE is the next UICommand instance
+        UICommand differentUiCommand = new UICommand();
+        uiCommand.setId("idOfdifferentUiCommand");
+        facesContext.getViewRoot().getChildren().add(differentUiCommand);
+        
+        
+        ActionListenerImplementation actionListener = new ActionListenerImplementation(differentUiCommand);
+        uiCommand.addActionListener(actionListener);
+        
+        ActionListener differentActionListener = org.easymock.EasyMock.createNiceMock(ActionListener.class);
+        differentActionListener.processAction(actionListener.newActionEventFromListener);
+        org.easymock.EasyMock.expectLastCall().times(1);
+        org.easymock.EasyMock.replay(differentActionListener);
+        differentUiCommand.addActionListener(differentActionListener);
+        
+        // Simulates first event, in most cases click in GUI
+        ActionEvent invokeApplicationEvent = new ActionEvent(uiCommand);
+        invokeApplicationEvent.queue();
+        
+        // tested method: In this method is actionListener called and that
+        // listener itself queues new event
+        facesContext.getViewRoot().broadcastEvents(facesContext, PhaseId.INVOKE_APPLICATION);
+        
+        assertEquals(15, actionListener.invocationCount);
+        org.easymock.EasyMock.verify(differentActionListener);
+    }
+
 }