You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ar...@apache.org on 2012/02/01 19:03:19 UTC

svn commit: r1239240 - in /myfaces/trinidad/trunk/trinidad-api/src/main: java/org/apache/myfaces/trinidad/component/UIXComponent.java xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts

Author: arobinson74
Date: Wed Feb  1 18:03:19 2012
New Revision: 1239240

URL: http://svn.apache.org/viewvc?rev=1239240&view=rev
Log:
TRINIDAD-2203 - Add checks with exceptions to UIXComponent that is only performed while the project stage is in unit testing mode to validate that components enter visiting and encoding context only once. Includes code to check the stack trace of the initial call to make it easier to debug when calls are incorrectly made

Modified:
    myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java
    myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts

Modified: myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java?rev=1239240&r1=1239239&r2=1239240&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/java/org/apache/myfaces/trinidad/component/UIXComponent.java Wed Feb  1 18:03:19 2012
@@ -23,6 +23,11 @@ import java.util.Collection;
 import java.util.Iterator;
 
 import javax.el.MethodExpression;
+
+import javax.faces.FactoryFinder;
+import javax.faces.application.Application;
+import javax.faces.application.ApplicationFactory;
+import javax.faces.application.ProjectStage;
 import javax.faces.component.NamingContainer;
 import javax.faces.component.StateHelper;
 import javax.faces.component.UIComponent;
@@ -38,8 +43,11 @@ import javax.faces.render.Renderer;
 
 import org.apache.myfaces.buildtools.maven2.plugin.builder.annotation.JSFComponent;
 import org.apache.myfaces.trinidad.bean.FacesBean;
+import org.apache.myfaces.trinidad.context.ComponentContextChange;
+import org.apache.myfaces.trinidad.context.ComponentContextManager;
 import org.apache.myfaces.trinidad.context.PartialPageContext;
 import org.apache.myfaces.trinidad.context.RenderingContext;
+import org.apache.myfaces.trinidad.context.RequestContext;
 import org.apache.myfaces.trinidad.event.AttributeChangeListener;
 import org.apache.myfaces.trinidad.logging.TrinidadLogger;
 import org.apache.myfaces.trinidad.render.CoreRenderer;
@@ -926,20 +934,28 @@ abstract public class UIXComponent exten
    * @see #setupEncodingContext
    * @see #tearDownEncodingContext
    * @see #setupChildrenVistingContext
-   *
    */
   protected void setupVisitingContext(@SuppressWarnings("unused") FacesContext context)
   {
-    if (_inVisitingContext)
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_ALREADY_IN_VISITING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // First, check to ensure that we are not already in context
+      if (_inVisitingContext)
       {
-        Thread.currentThread().dumpStack();
-      }
+        String errorMessage = _LOG.getMessage("COMPONENT_ALREADY_IN_VISITING_CONTEXT",
+                                new Object[] { getClientId(context), _setupVisitingCaller });
+        throw new IllegalStateException(errorMessage);
+      }
+
+      // Next, add a context change so that the flags are reset during an invokeOnComponent,
+      // or a visitTree call:
+      ComponentContextManager componentContextManager =
+        RequestContext.getCurrentInstance().getComponentContextManager();
+      componentContextManager.pushChange(new DebugContextChange(this));
+      _inVisitingContext = true;
+      _setupVisitingCaller = _getStackTraceElementForCaller();
     }
-
-    _inVisitingContext = true;
   }
 
   /**
@@ -957,16 +973,20 @@ abstract public class UIXComponent exten
    */
   protected void setupChildrenVisitingContext(@SuppressWarnings("unused") FacesContext context)
   {
-    if (_inChildrenVisitingContext)
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_ALREADY_IN_CHILDREN_VISITING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // Check to ensure that we are not already in context
+      if (_inChildrenVisitingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_ALREADY_IN_CHILDREN_VISITING_CONTEXT",
+          new Object[] { getClientId(context), _setupChildrenVisitingCaller });
+        throw new IllegalStateException(errorMessage);
       }
-    }
 
-    _inChildrenVisitingContext = true;
+      _inChildrenVisitingContext = true;
+      _setupChildrenVisitingCaller = _getStackTraceElementForCaller();
+    }
   }
 
   /**
@@ -984,15 +1004,34 @@ abstract public class UIXComponent exten
    */
   protected void tearDownVisitingContext(@SuppressWarnings("unused") FacesContext context)
   {
-    if (!_inVisitingContext)
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_NOT_IN_VISITING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // First, check to ensure that we are not already in context
+      if (!_inVisitingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_NOT_IN_VISITING_CONTEXT",
+                                new Object[] { getClientId(context), _tearDownVisitingCaller });
+        throw new IllegalStateException(errorMessage);
       }
+
+      // Next, remove the context change that was added in setupVisitingContext:
+      ComponentContextManager componentContextManager =
+        RequestContext.getCurrentInstance().getComponentContextManager();
+
+      ComponentContextChange contextChange = componentContextManager.popChange();
+
+      // Validate the state of the context change stack:
+      if (!(contextChange instanceof DebugContextChange) ||
+          ((DebugContextChange)contextChange)._component != this)
+      {
+        String errorMessage = _LOG.getMessage("INVALID_CONTEXT_CHANGE_FOUND");
+        throw new IllegalStateException(errorMessage);
+      }
+
+      _inVisitingContext = false;
+      _tearDownVisitingCaller = _getStackTraceElementForCaller();
     }
-    _inVisitingContext = false;
   }
 
   /**
@@ -1009,15 +1048,20 @@ abstract public class UIXComponent exten
    */
   protected void tearDownChildrenVisitingContext(@SuppressWarnings("unused") FacesContext context)
   {
-    if (!_inChildrenVisitingContext)
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_NOT_IN_CHILDREN_VISITING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // Check to ensure that we are not already in context
+      if (!_inChildrenVisitingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_NOT_IN_CHILDREN_VISITING_CONTEXT",
+          new Object[] { getClientId(context), _tearDownChildrenVisitingCaller });
+        throw new IllegalStateException(errorMessage);
       }
+
+      _inChildrenVisitingContext = false;
+      _tearDownChildrenVisitingCaller = _getStackTraceElementForCaller();
     }
-    _inChildrenVisitingContext = false;
   }
 
   @Deprecated
@@ -1054,16 +1098,22 @@ abstract public class UIXComponent exten
    */
   protected void setupEncodingContext(FacesContext context, RenderingContext rc)
   {
-    if (_inEncodingContext)
+    setupVisitingContext(context);
+
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_ALREADY_IN_ENCODING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // Check to ensure that we are not already in context
+      if (_inEncodingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_ALREADY_IN_ENCODING_CONTEXT",
+                                new Object[] { getClientId(context), _setupEncodingCaller});
+        throw new IllegalStateException(errorMessage);
       }
-    }
 
-    setupVisitingContext(context);
+      _inEncodingContext = true;
+      _setupEncodingCaller = _getStackTraceElementForCaller();
+    }
 
     Renderer renderer = getRenderer(context);
 
@@ -1088,16 +1138,22 @@ abstract public class UIXComponent exten
    */
   public void setupChildrenEncodingContext(FacesContext context, RenderingContext rc)
   {
-    if (_inChildrenEncodingContext)
+    setupChildrenVisitingContext(context);
+
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_ALREADY_IN_CHILDREN_ENCODING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // Check to ensure that we are not already in context
+      if (_inChildrenEncodingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_ALREADY_IN_CHILDREN_ENCODING_CONTEXT",
+          new Object[] { getClientId(context), _setupChildrenEncodingCaller });
+        throw new IllegalStateException(errorMessage);
       }
-    }
 
-    setupChildrenVisitingContext(context);
+      _inChildrenEncodingContext = true;
+      _setupChildrenEncodingCaller = _getStackTraceElementForCaller();
+    }
 
     Renderer renderer = getRenderer(context);
 
@@ -1131,13 +1187,19 @@ abstract public class UIXComponent exten
     FacesContext context,
     RenderingContext rc)
   {
-    if (!_inEncodingContext)
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_NOT_IN_ENCODING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // Check to ensure that we are not already in context
+      if (!_inEncodingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_NOT_IN_ENCODING_CONTEXT",
+                                new Object[] { getClientId(context), _tearDownEncodingCaller });
+        throw new IllegalStateException(errorMessage);
       }
+
+      _inEncodingContext = false;
+      _tearDownEncodingCaller = _getStackTraceElementForCaller();
     }
 
     Renderer renderer = getRenderer(context);
@@ -1154,7 +1216,6 @@ abstract public class UIXComponent exten
     finally
     {
       tearDownVisitingContext(context);
-      _inEncodingContext = false;
     }
   }
 
@@ -1171,13 +1232,19 @@ abstract public class UIXComponent exten
     FacesContext context,
     RenderingContext rc)
   {
-    if (!_inChildrenEncodingContext)
+    // If in testing phase, add code to determine if this function is being used correctly.
+    if (_inTestingPhase)
     {
-      _LOG.warning("COMPONENT_NOT_IN_CHILDREN_ENCODING_CONTEXT", getClientId(context));
-      if (_LOG.isFine())
+      // Check to ensure that we are not already in context
+      if (!_inChildrenEncodingContext)
       {
-        Thread.currentThread().dumpStack();
+        String errorMessage = _LOG.getMessage("COMPONENT_NOT_IN_CHILDREN_ENCODING_CONTEXT",
+          new Object[] { getClientId(context), _tearDownChildrenEncodingCaller });
+        throw new IllegalStateException(errorMessage);
       }
+
+      _inChildrenEncodingContext = false;
+      _tearDownChildrenEncodingCaller = _getStackTraceElementForCaller();
     }
 
     Renderer renderer = getRenderer(context);
@@ -1306,6 +1373,24 @@ abstract public class UIXComponent exten
     throw new UnsupportedOperationException();
   }
 
+  private String _getStackTraceElementForCaller()
+  {
+    StackTraceElement[] elements = Thread.currentThread().getStackTrace();
+
+    // 0 is the call to getStackTrace, 1 is this method, 2 is the calling method in UIXComponent
+    // so 3 (4th element) is the caller to the UIXComponent method, the one that is desired.
+
+    String lineSep = System.getProperty("line.separator");
+    StringBuilder stack = new StringBuilder();
+    for (int i = 3, size = elements.length; i < size && i < 10; ++i)
+    {
+      stack.append(elements[i].toString());
+      stack.append(lineSep);
+    }
+
+    return stack.toString();
+  }
+
   /**
    * Determine if we can flatten a core JSF component.
    * @param component The component
@@ -1333,11 +1418,108 @@ abstract public class UIXComponent exten
     return UIPanel.class == componentClass;
   }
 
+  private static class DebugContextChange
+    extends ComponentContextChange
+  {
+    private DebugContextChange(
+      UIXComponent     component)
+    {
+      _component = component;
+    }
+
+    @Override
+    public void resume(FacesContext facesContext)
+    {
+      _component._inVisitingContext = _inVisitingContext;
+      _component._inChildrenVisitingContext = _inChildrenVisitingContext;
+      _component._inEncodingContext = _inEncodingContext;
+      _component._inChildrenEncodingContext = _inChildrenEncodingContext;
+      _component._setupVisitingCaller = _setupVisitingCaller;
+      _component._tearDownVisitingCaller = _tearDownVisitingCaller;
+      _component._setupEncodingCaller = _setupEncodingCaller;
+      _component._tearDownEncodingCaller = _tearDownEncodingCaller;
+      _component._setupChildrenEncodingCaller = _setupChildrenEncodingCaller;
+      _component._tearDownChildrenEncodingCaller = _tearDownChildrenEncodingCaller;
+      _component._setupChildrenVisitingCaller = _setupChildrenVisitingCaller;
+      _component._tearDownChildrenVisitingCaller = _tearDownChildrenVisitingCaller;
+    }
+
+    @Override
+    public void suspend(FacesContext facesContext)
+    {
+      _inVisitingContext = _component._inVisitingContext;
+      _inChildrenVisitingContext = _component._inChildrenVisitingContext;
+      _inEncodingContext = _component._inEncodingContext;
+      _inChildrenEncodingContext = _component._inChildrenEncodingContext;
+      _setupVisitingCaller = _component._setupVisitingCaller;
+      _tearDownVisitingCaller = _component._tearDownVisitingCaller;
+      _setupEncodingCaller = _component._setupEncodingCaller;
+      _tearDownEncodingCaller = _component._tearDownEncodingCaller;
+      _setupChildrenEncodingCaller = _component._setupChildrenEncodingCaller;
+      _tearDownChildrenEncodingCaller = _component._tearDownChildrenEncodingCaller;
+      _setupChildrenVisitingCaller = _component._setupChildrenVisitingCaller;
+      _tearDownChildrenVisitingCaller = _component._tearDownChildrenVisitingCaller;
+    }
+
+    private final UIXComponent _component;
+    private boolean _inVisitingContext;
+    private boolean _inChildrenVisitingContext;
+    private boolean _inEncodingContext;
+    private boolean _inChildrenEncodingContext;
+    private String _setupVisitingCaller;
+    private String _tearDownVisitingCaller;
+    private String _setupEncodingCaller;
+    private String _tearDownEncodingCaller;
+    private String _setupChildrenEncodingCaller;
+    private String _tearDownChildrenEncodingCaller;
+    private String _setupChildrenVisitingCaller;
+    private String _tearDownChildrenVisitingCaller;
+  }
+
+  private final static boolean _inTestingPhase;
+  private static final TrinidadLogger _LOG =
+    TrinidadLogger.createTrinidadLogger(UIXComponent.class);
+
+  static
+  {
+    // If Trinidad is to be ever shared with a shared class loader, we should change this
+    // static boolean flag to be non-class loader bound.
+    Application app = null;
+    try
+    {
+      FacesContext facesContext = FacesContext.getCurrentInstance();
+      app = facesContext == null ? null : facesContext.getApplication();
+      if (app == null)
+      {
+        ApplicationFactory appFactory = (ApplicationFactory)
+          FactoryFinder.getFactory(FactoryFinder.APPLICATION_FACTORY);
+        app = appFactory.getApplication();
+      }
+    }
+    catch (Throwable t)
+    {
+      // This occurs during unit testing without a full JSF environment, ignore
+      ;
+    }
+
+    _inTestingPhase = app == null ? false : app.getProjectStage() == ProjectStage.UnitTest;
+    if (_inTestingPhase)
+    {
+      _LOG.info("Application is running in testing phase, UIXComponent will " +
+        "perform extra validation steps to ensure proper component usage");
+    }
+  };
+
+  private String _setupVisitingCaller;
+  private String _tearDownVisitingCaller;
+  private String _setupEncodingCaller;
+  private String _tearDownEncodingCaller;
+  private String _setupChildrenEncodingCaller;
+  private String _tearDownChildrenEncodingCaller;
+  private String _setupChildrenVisitingCaller;
+  private String _tearDownChildrenVisitingCaller;
   private boolean _inVisitingContext;
   private boolean _inChildrenVisitingContext;
   private boolean _inEncodingContext;
   private boolean _inChildrenEncodingContext;
-
-  private static final TrinidadLogger _LOG =
-    TrinidadLogger.createTrinidadLogger(UIXComponent.class);
 }
\ No newline at end of file

Modified: myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts
URL: http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts?rev=1239240&r1=1239239&r2=1239240&view=diff
==============================================================================
--- myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts (original)
+++ myfaces/trinidad/trunk/trinidad-api/src/main/xrts/org/apache/myfaces/trinidad/resource/LoggerBundle.xrts Wed Feb  1 18:03:19 2012
@@ -490,27 +490,30 @@
 <resource key="COLLECTION_NOT_IN_CONTEXT">The row key or row index of a UIXCollection component is being changed outside of the component's context. Changing the key or index of a collection when the collection is not currently being visited, invoked on, broadcasting an event or processing a lifecycle method, is not valid. Data corruption and errors may result from this call. Component ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
 
 <!-- COMPONENT_ALREADY_IN_CHILDREN_ENCODING_CONTEXT -->
-<resource key="COMPONENT_ALREADY_IN_CHILDREN_ENCODING_CONTEXT">setupChildrenEncodingContext called on a component that is already in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_ALREADY_IN_CHILDREN_ENCODING_CONTEXT">setupChildrenEncodingContext called on a component that is already in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_ALREADY_IN_CHILDREN_VISITING_CONTEXT -->
-<resource key="COMPONENT_ALREADY_IN_CHILDREN_VISITING_CONTEXT">setupChildrenVisitingContext called on a component that is already in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_ALREADY_IN_CHILDREN_VISITING_CONTEXT">setupChildrenVisitingContext called on a component that is already in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_NOT_IN_CHILDREN_ENCODING_CONTEXT -->
-<resource key="COMPONENT_NOT_IN_CHILDREN_ENCODING_CONTEXT">tearDownChildrenEncodingContext called on a component that is not in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_NOT_IN_CHILDREN_ENCODING_CONTEXT">tearDownChildrenEncodingContext called on a component that is not in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_NOT_IN_CHILDREN_VISITING_CONTEXT -->
-<resource key="COMPONENT_NOT_IN_CHILDREN_VISITING_CONTEXT">tearDownChildrenVisitingContext called on a component that is not in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_NOT_IN_CHILDREN_VISITING_CONTEXT">tearDownChildrenVisitingContext called on a component that is not in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_ALREADY_IN_ENCODING_CONTEXT -->
-<resource key="COMPONENT_ALREADY_IN_ENCODING_CONTEXT">setupEncodingContext called on a component that is already in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_ALREADY_IN_ENCODING_CONTEXT">setupEncodingContext called on a component that is already in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_ALREADY_IN_VISITING_CONTEXT -->
-<resource key="COMPONENT_ALREADY_IN_VISITING_CONTEXT">setupVisitingContext called on a component that is already in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_ALREADY_IN_VISITING_CONTEXT">setupVisitingContext called on a component that is already in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_NOT_IN_ENCODING_CONTEXT -->
-<resource key="COMPONENT_NOT_IN_ENCODING_CONTEXT">tearDownEncodingContext called on a component that is not in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_NOT_IN_ENCODING_CONTEXT">tearDownEncodingContext called on a component that is not in context. Client ID: {0}. Originally called by {1}.</resource>
 
 <!-- COMPONENT_NOT_IN_VISITING_CONTEXT -->
-<resource key="COMPONENT_NOT_IN_VISITING_CONTEXT">tearDownVisitingContext called on a component that is not in context. Client ID: {0}. Turn on fine level logging for a stack trace of this call.</resource>
+<resource key="COMPONENT_NOT_IN_VISITING_CONTEXT">tearDownVisitingContext called on a component that is not in context. Client ID: {0}. Originally called by {1}.</resource>
+
+<!-- INVALID_CONTEXT_CHANGE_FOUND -->
+<resource key="INVALID_CONTEXT_CHANGE_FOUND">An unexpected component context change was found. This is due to components not correctly maintaining the context change stack.</resource>
 
 </resources>