You are viewing a plain text version of this content. The canonical link for it is here.
Posted to adffaces-commits@incubator.apache.org by aw...@apache.org on 2007/02/14 00:08:23 UTC

svn commit: r507320 - in /incubator/adffaces/branches/faces-1_2-070201/trinidad: trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/ trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/ trinidad-impl/src/main/java/org/apac...

Author: awiner
Date: Tue Feb 13 16:08:22 2007
New Revision: 507320

URL: http://svn.apache.org/viewvc?view=rev&rev=507320
Log:
ADFFACES-379: Configurator.endRequest fails to execute under certain conditions.
- and simplification of logic for creating and cleaning up the RequestContext, mostly
  getting rid of it from the TrinidadPhaseListener, where it's not needed

While testing this patch, discovered problems with the file upload code, which
seem to have already been present, but fixed them:
- Fix potential double-decoding of parameters if ExternalContext.getRequestCharacterEncdoing()
  returns something other ISO-8859-1 prior to file upload being handled
- Fix incorrect warnings of "can't call setCharacterEncoding(), it's too late"
  caused by the request wrapper calling super.setCharacterEncoding(), instead
  of just handling it internally
- Fix unnecessary extra merge of the already-present request parameters - the
  map was getting pushed in three times, not just once
- Add warning of "can't call setCharacterEncoding()" back where appropriate;  client
  calls to setCharacterEncoding() after that performed by ViewHandler.initView()
  will be appropriately trapped and ignored.


Modified:
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/ResourceServlet.java
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/ActionUploadRequestWrapper.java
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/FileUploadConfiguratorImpl.java
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/UploadRequestWrapper.java
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/FacesContextFactoryImpl.java
    incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/TrinidadPhaseListener.java

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/ResourceServlet.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/ResourceServlet.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/ResourceServlet.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-api/src/main/java/org/apache/myfaces/trinidad/webapp/ResourceServlet.java Tue Feb 13 16:08:22 2007
@@ -163,11 +163,6 @@
     }
     finally
     {
-      //=-= Scott O'Bryan =-=
-      // HACK: This never actually goes through the lifecycle like it should.  
-      // So we'll need to set response complete so configurator does its 
-      // cleanup.
-      context.responseComplete();
       context.release();
     }
   }

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/GlobalConfiguratorImpl.java Tue Feb 13 16:08:22 2007
@@ -208,12 +208,12 @@
       // the contract.
       if (!_isDisabled(externalContext))
       {
+        // If this hasn't been initialized then please initialize
         if (!_initialized)
         {
           init(externalContext);
         }
 
-        // If this hasn't been initialized then please initialize
         _attachRequestContext(externalContext);
 
         if (externalContext.getRequestMap().get(_IN_REQUEST) == null)
@@ -292,13 +292,8 @@
         {
           _endConfiguratorServiceRequest(externalContext);
         }
-
-        final RequestContext context = RequestContext.getCurrentInstance();
-        if (context != null)
-        {
-          context.release();
-          assert RequestContext.getCurrentInstance() == null;
-        }
+        
+        _releaseRequestContext(externalContext);
       }
       RequestType.clearType(externalContext);
     }
@@ -344,7 +339,7 @@
         //when we have to so it should be no biggy under normal circumstances.
         externalContext = new ClearRequestExternalContext(externalContext);
       }
-      
+
       // Wrap ExternalContexts
       for (final Configurator config : _services)
       {
@@ -434,7 +429,7 @@
     // See if we've got a cached RequestContext instance; if so,
     // reattach it
     final Object cachedRequestContext = externalContext.getRequestMap().get(
-        TrinidadPhaseListener.CACHED_REQUEST_CONTEXT);
+        _REQUEST_CONTEXT);
 
     // Catch both the null scenario and the
     // RequestContext-from-a-different-classloader scenario
@@ -448,11 +443,28 @@
       final RequestContextFactory factory = RequestContextFactory.getFactory();
       assert factory != null;
       context = factory.createContext(externalContext);
-      externalContext.getRequestMap().put(TrinidadPhaseListener.CACHED_REQUEST_CONTEXT, context);
+      externalContext.getRequestMap().put(_REQUEST_CONTEXT, context);
     }
 
     assert RequestContext.getCurrentInstance() == context;
   }
+  
+  private void _releaseRequestContext(final ExternalContext ec)
+  {
+    //If it's not a portal action, we should remove the cached request because
+    //well want to create a new one next request
+    if(RequestType.getType(ec) != RequestType.PORTAL_ACTION)
+    {
+      ec.getRequestMap().remove(_REQUEST_CONTEXT);
+    }
+    
+    final RequestContext context = RequestContext.getCurrentInstance();
+    if (context != null)
+    {
+      context.release();
+      assert RequestContext.getCurrentInstance() == null;
+    }    
+  }
 
   private void _endConfiguratorServiceRequest(final ExternalContext ec)
   {
@@ -485,29 +497,29 @@
   {
     // This first check is here in order to skip synchronization until 
     // absolutely necessary.
-    if(!_setRequestBugTested)
+    if(!_sSetRequestBugTested)
     {
       synchronized(GlobalConfiguratorImpl.class)
       {
         //This second check is here in case a couple of things enter before the
         //boolean is set.  This is only an exception case and will make it so
         //the initialization code runs only once.
-        if(!_setRequestBugTested)
+        if(!_sSetRequestBugTested)
         {
           ServletRequest orig = (ServletRequest)ec.getRequest();
           Map m = ec.getInitParameterMap();
           
           ec.setRequest(new TestRequest(orig));
           
-          _hasSetRequestBug = !TestRequest.isTestParamPresent(ec);
-          _setRequestBugTested = true;
+          _sHasSetRequestBug = !TestRequest.isTestParamPresent(ec);
+          _sSetRequestBugTested = true;
           
           ec.setRequest(orig);
         }
       }
     }
     
-    return _hasSetRequestBug;
+    return _sHasSetRequestBug;
   }
   
   // This handles an issue with the ExternalContext object prior to
@@ -724,18 +736,20 @@
     }
   }
 
-  private static boolean _setRequestBugTested = false;
-  private static boolean _hasSetRequestBug = false;
+  private static boolean _sSetRequestBugTested = false;
+  private static boolean _sHasSetRequestBug = false;
 
-  private boolean                                               _initialized;
-  private List<Configurator>                                    _services;
-  static private final Map<ClassLoader, GlobalConfiguratorImpl> _CONFIGURATORS = new HashMap<ClassLoader, GlobalConfiguratorImpl>();
-  static private final String                                   _IN_REQUEST    = GlobalConfiguratorImpl.class
-                                                                                   .getName()
-                                                                                   + ".IN_REQUEST";
+  private boolean             _initialized;
+  private List<Configurator>  _services;
+  static private final Map<ClassLoader, GlobalConfiguratorImpl> _CONFIGURATORS =
+     new HashMap<ClassLoader, GlobalConfiguratorImpl>();
+  static private final String _IN_REQUEST    =
+     GlobalConfiguratorImpl.class.getName() 
+     + ".IN_REQUEST";
+  static private final String _REQUEST_CONTEXT =
+     GlobalConfiguratorImpl.class.getName()
+     + ".REQUEST_CONTEXT";
 
-  static private final TrinidadLogger                           _LOG           = TrinidadLogger
-                                                                                   .createTrinidadLogger(GlobalConfiguratorImpl.class);
     
   private enum RequestType
   {
@@ -810,6 +824,11 @@
       return ec.getRequestParameterMap().get(_TEST_PARAM) != null;
     }
     
-    static private String _TEST_PARAM = TestRequest.class.getName()+".TEST_PARAM";
+    static private String _TEST_PARAM = TestRequest.class.getName()+
+      ".TEST_PARAM";
   }
+
+
+  static private final TrinidadLogger _LOG =
+    TrinidadLogger.createTrinidadLogger(GlobalConfiguratorImpl.class);
 }

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/ActionUploadRequestWrapper.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/ActionUploadRequestWrapper.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/ActionUploadRequestWrapper.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/ActionUploadRequestWrapper.java Tue Feb 13 16:08:22 2007
@@ -50,6 +50,8 @@
     _extractedParams = new HashMap<String, String[]>(origionalMap);
     _extractedParams.putAll(params);
 
+    _encoding = super.getCharacterEncoding();
+
     //add these params to the render request
     _response.setRenderParameters(_extractedParams);
   }
@@ -64,6 +66,13 @@
     return _WWW_FORM_URLENCODED_TYPE;
   }
 
+  @Override
+  public String getCharacterEncoding()
+  {
+    return _encoding;
+  }
+
+
   /**
    * Trap calls to setCharacterEncoding() to decode parameters correctly
    */
@@ -71,7 +80,13 @@
   public void setCharacterEncoding(String encoding)
     throws UnsupportedEncodingException
   {
-    super.setCharacterEncoding(encoding);
+    // If the encoding is already right, we can bail
+    if (encoding.equals(_encoding))
+      return;
+    
+    // Don't call super.setCharacterEncoding() - it's too late
+    // and we'll get a warning
+    _encoding = encoding;
     if (_LOG.isFine())
       _LOG.fine("Switching encoding of wrapper to " + encoding);
 
@@ -158,6 +173,7 @@
   private Map<String, String[]> _extractedAndDecodedParams;
   private Map<String, String[]> _extractedParams;
   private ActionResponse _response;
+  private String         _encoding;
   private static final String _WWW_FORM_URLENCODED_TYPE =
     "application/x-www-form-urlencoded";
   private static final TrinidadLogger _LOG =

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/FileUploadConfiguratorImpl.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/FileUploadConfiguratorImpl.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/FileUploadConfiguratorImpl.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/FileUploadConfiguratorImpl.java Tue Feb 13 16:08:22 2007
@@ -169,13 +169,6 @@
 
     if(addedParams != null)
     {
-      @SuppressWarnings("unchecked")
-      Map<String, String[]> parameters = new HashMap<String, String[]>(externalContext.getRequestParameterValuesMap());
-      parameters.putAll(addedParams);
-
-      // TODO sobryan (dependency = JSF 1.2)
-      // For JSF 1.2, we can probably simply wrap the request objects and set them on
-      // the existing ExternalContext.
       return _getExternalContextWrapper(externalContext, addedParams);
     }
 
@@ -220,14 +213,11 @@
   {
     if(!isApplied(externalContext))
     {
-      Map<String, String[]> params = new HashMap<String, String[]>(externalContext.getRequestParameterValuesMap());
-      params.putAll(addedParams);
-
       if(!ExternalContextUtils.isPortlet(externalContext))
       {  
         externalContext.setRequest(new UploadRequestWrapper(
             (HttpServletRequest)externalContext.getRequest(),
-            params));        
+            addedParams));        
       }
       else if(ExternalContextUtils.isAction(externalContext))
       {
@@ -238,7 +228,7 @@
          * render requests will retain these parameters for us.
          */
         externalContext.setRequest(new ActionUploadRequestWrapper(externalContext,
-           params));
+           addedParams));
       }
       apply(externalContext);        
     }

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/UploadRequestWrapper.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/UploadRequestWrapper.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/UploadRequestWrapper.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/config/upload/UploadRequestWrapper.java Tue Feb 13 16:08:22 2007
@@ -16,10 +16,10 @@
  * Request wrapper class that hooks in parameters identified in
  * the servlet request.
  *
- * @todo Stop going String -> bytes -> String;  change MultipartFormHandler
- *  to simply extract byte arrays, and do all the type conversion here.
  * @author The Oracle ADF Faces Team
  */
+// TODO Stop going String -> bytes -> String;  change MultipartFormHandler
+//    to simply extract byte arrays, and do all the type conversion here.
 @SuppressWarnings("deprecation")
 public class UploadRequestWrapper extends HttpServletRequestWrapper
 {
@@ -30,10 +30,12 @@
     super(request);
     
     @SuppressWarnings("unchecked")
-    Map<String, String[]> origionalMap = super.getParameterMap();
+    // Merge in all the original parameters
+    Map<String, String[]> originalMap = super.getParameterMap();
     
-    _extractedParams = new HashMap<String, String[]>(origionalMap);
+    _extractedParams = new HashMap<String, String[]>(originalMap);
     _extractedParams.putAll(params);
+    _encoding = super.getCharacterEncoding();
   }
   
   /**
@@ -46,6 +48,12 @@
     return _WWW_FORM_URLENCODED_TYPE;
   }
 
+  @Override
+  public String getCharacterEncoding()
+  {
+    return _encoding;
+  }
+
   /**
    * Trap calls to setCharacterEncoding() to decode parameters correctly
    */
@@ -53,7 +61,24 @@
   public void setCharacterEncoding(String encoding)
     throws UnsupportedEncodingException
   {
-    super.setCharacterEncoding(encoding);
+    // It is illegal to set the character encoding after parameters
+    // have been retrieved.  This is an annoying restriction,
+    // but we shouldn't break it
+    if (_parametersRetrieved)
+    {
+      _LOG.warning("Unable to set request character encoding to {0}, " + 
+                   "because request parameters have already been read.",
+                   encoding);
+      return;
+    }
+
+    // If the encoding is already right, we can bail
+    if (encoding.equals(_encoding))
+      return;
+    
+    // Don't call super.setCharacterEncoding() - it's too late
+    // and we'll get a warning
+    _encoding = encoding;
     if (_LOG.isFine())
       _LOG.fine("Switching encoding of wrapper to " + encoding);
 
@@ -62,6 +87,9 @@
       
     byte[] buffer = new byte[256];
     
+    // FIXME: decodeRequestParameter() assumes the incoming
+    // character set is ISO-8859-1 - but this is not
+    // necessarily true!
     for(Map.Entry<String, String[]> entry : _extractedParams.entrySet())
     {
       String key = entry.getKey();
@@ -130,6 +158,9 @@
    */
   private Map<String, String[]> _getMap()
   {
+    // Mark that parameters have been retrieved so we 
+    // can log a proper warning
+    _parametersRetrieved = true;
     if (_extractedAndDecodedParams != null)
       return _extractedAndDecodedParams;
 
@@ -138,6 +169,8 @@
 
   private Map<String, String[]> _extractedAndDecodedParams;
   private Map<String, String[]> _extractedParams;
+  private String                _encoding;
+  private boolean               _parametersRetrieved;
 
   private static final String _WWW_FORM_URLENCODED_TYPE =
     "application/x-www-form-urlencoded";

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/FacesContextFactoryImpl.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/FacesContextFactoryImpl.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/FacesContextFactoryImpl.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/FacesContextFactoryImpl.java Tue Feb 13 16:08:22 2007
@@ -61,51 +61,27 @@
       Object response, 
       Lifecycle lifecycle)
   {
-    FacesContext fc = _factory.getFacesContext(context, request, response, lifecycle);
-    ExternalContext ec = fc.getExternalContext();
-
-    GlobalConfiguratorImpl config = GlobalConfiguratorImpl.getInstance();    
-
-    //This should be done only if the filter or other logic was not done before this
-    //we try to retrieve the FacesContext.  If this is the case then we'll need to handle
-    //cleanup on the release of the FacesContext.  Otherwise the endRequest should be
-    //called by whatever did he origional beginRequest.
-    
-    if(!GlobalConfiguratorImpl.isRequestStarted(ec))
-    {
-      config.beginRequest(ec);
-      ec.getApplicationMap().put(_CONFIG_IN_CONTEXT, Boolean.TRUE);
-    }
-    
-    return new CacheRenderKit(fc);
+    return new CacheRenderKit(_factory.getFacesContext(context, request, response, lifecycle));
   }
   
-  /**
-   * Sets the configurator up to execute an endRequest when it is destroyed
-   * 
-   * @param fc
-   */
-  @SuppressWarnings("unchecked")
-  static void endRequestIfNecessary(FacesContext fc)
-  {
-    ExternalContext ec = fc.getExternalContext();
-    if(Boolean.TRUE.equals(ec.getApplicationMap().remove(_CONFIG_IN_CONTEXT)))
-    {
-      ec.getApplicationMap().put(_READY_FOR_CLEANUP, Boolean.TRUE);      
-    }
-  }
-
   static public class CacheRenderKit extends FacesContext
   {
     public CacheRenderKit(FacesContext base)
     {
       _base = base;
-      
-      //SMO: TODO: is this still needed?
       ExternalContext baseExternal = base.getExternalContext();
-      ExternalContext external = 
-        GlobalConfiguratorImpl.getInstance().getExternalContext(baseExternal);
-      _external = new OverrideDispatch(external);
+      GlobalConfiguratorImpl config = GlobalConfiguratorImpl.getInstance();
+
+      //This should be done only if beginRequest was not called on the configurator
+      //before we retrieve the FacesContext.  If this is the case then we'll need to handle
+      //cleanup on the release of the FacesContext.  Otherwise the endRequest should be
+      //called by whatever did he origional beginRequest.
+      if(!GlobalConfiguratorImpl.isRequestStarted(baseExternal))
+      {
+        baseExternal.getRequestMap().put(_CONFIG_IN_CONTEXT, Boolean.TRUE);
+      }
+
+      _external = new OverrideDispatch(config.getExternalContext(baseExternal));
       setCurrentInstance(this);
     }
 
@@ -246,12 +222,8 @@
     @Override
     public void release()
     {
-      //=- Scott O'Bryan -=
-      // JSR-301 should allow us to call the cleanup.  So this and all logic
-      // pertaining to creation and cleanup of the configurator per request
-      // could probably go away.
       ExternalContext ec = getExternalContext();
-      if(Boolean.TRUE.equals(ec.getApplicationMap().remove(_READY_FOR_CLEANUP)))
+      if(Boolean.TRUE.equals(ec.getRequestMap().get(_CONFIG_IN_CONTEXT)))
       {
         GlobalConfiguratorImpl.getInstance().endRequest(ec);
       }
@@ -268,6 +240,8 @@
     private final ExternalContext _external;
     private String    _renderKitId;
     private RenderKit _kit;
+    
+    static private final String _CONFIG_IN_CONTEXT = FacesContextFactoryImpl.class.getName()+".CONFIG_IN_CONTEXT";
   }
 
   static public class OverrideDispatch extends ExternalContextDecorator
@@ -299,7 +273,5 @@
     private final ExternalContext _decorated;
   }
 
-  static private final String _CONFIG_IN_CONTEXT = FacesContextFactoryImpl.class.getName()+".CONFIG_IN_CONTEXT";
-  static private final String _READY_FOR_CLEANUP = FacesContextFactoryImpl.class.getName()+".CONFIG_READY_FOR_CLEANUP";
   private final FacesContextFactory _factory;
 }

Modified: incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/TrinidadPhaseListener.java
URL: http://svn.apache.org/viewvc/incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/TrinidadPhaseListener.java?view=diff&rev=507320&r1=507319&r2=507320
==============================================================================
--- incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/TrinidadPhaseListener.java (original)
+++ incubator/adffaces/branches/faces-1_2-070201/trinidad/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/context/TrinidadPhaseListener.java Tue Feb 13 16:08:22 2007
@@ -30,8 +30,7 @@
 import org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl;
 
 /**
- * PhaseListener that hacks to ensure that the RequestContext is
- * available even if the filter doesn't execute.
+ * Performs some trinidad logic and provides some hooks.
  *
  * @author The Oracle ADF Faces Team
  */
@@ -42,9 +41,6 @@
    */
   private static final long serialVersionUID = -1249678874100309402L;
 
-  static public final String CACHED_REQUEST_CONTEXT =
-    "org.apache.myfaces.trinidadinternal.context.CachedRequestContext";
-
   /**
    * Returns true if the request might be a postback request.
    */
@@ -54,8 +50,7 @@
     return !Boolean.FALSE.equals(context.getExternalContext().
                                    getRequestMap().get(_POSTBACK_KEY));
   }
-
-
+  
   /**
    * Marks that this is a postback request.
    */
@@ -76,21 +71,6 @@
       context.getExternalContext().getRequestMap().put(INITIAL_VIEW_ROOT_KEY,
                                                        context.getViewRoot());
     }
-
-    // If we've finished up Render Response, or for some other
-    // reason the response is complete, free up the RequestContext
-    // if we created.
-    // Note, however, that this code is *not* bulletproof!  There
-    // is nothing stopping an "afterPhase()" listener getting called
-    // after this one that calls responseComplete(), in which case
-    // we'd never get notified.
-    if ((event.getPhaseId() == PhaseId.RENDER_RESPONSE) ||
-        (event.getFacesContext().getResponseComplete()))
-    {
-      _releaseContextIfNecessary(event.getFacesContext());
-      FacesContextFactoryImpl.endRequestIfNecessary(context);
-    }
-        
   }
 
   @SuppressWarnings("unchecked")
@@ -100,21 +80,12 @@
     // "restore view" would be sufficient, but someone can call
     // renderResponse() before even calling Lifecycle.execute(),
     // in which case RESTORE_VIEW doesn't actually run.
-    if ((event.getPhaseId() == PhaseId.RESTORE_VIEW) ||
-        (event.getPhaseId() == PhaseId.RENDER_RESPONSE))
+    if (event.getPhaseId() == PhaseId.RESTORE_VIEW)
     {
-      if (event.getPhaseId() == PhaseId.RESTORE_VIEW)
-      {
-        FacesContext context = event.getFacesContext();
-        // Assume it's not a postback request
-        context.getExternalContext().getRequestMap().put(_POSTBACK_KEY,
-                                                         Boolean.FALSE);
-        
-        //This check doesn't make sense here
-        //TrinidadFilterImpl.verifyFilterIsInstalled(context);
-      }
-
-      _createContextIfNecessary(event.getFacesContext());
+      FacesContext context = event.getFacesContext();
+      // Assume it's not a postback request
+      context.getExternalContext().getRequestMap().put(_POSTBACK_KEY,
+                                                       Boolean.FALSE);      
     }
     // If we've reached "apply request values", this is definitely a
     // postback (the ViewHandler should have reached the same conclusion too,
@@ -131,76 +102,11 @@
   {
     return PhaseId.ANY_PHASE;
   }
-
-  //
-  // Create the RequestContext if necessary;  ideally, this is unnecessary
-  // because our filter will have executed - but if not, deal.
-  //
-  @SuppressWarnings("unchecked")
-  static private void _createContextIfNecessary(FacesContext fContext)
-  {
-    
-    Map<String, Object> requestMap = fContext.getExternalContext().getRequestMap();
-    Boolean createdContext = (Boolean)
-      requestMap.get(_CREATED_CONTEXT_KEY);
-    if (createdContext == null)
-    {
-      RequestContext context = RequestContext.getCurrentInstance();
-      // Let our code know if it has to clean up.
-      requestMap.put(_CREATED_CONTEXT_KEY,
-                     context == null ? Boolean.TRUE : Boolean.FALSE);
-
-      if (context == null)
-      {
-        Object cachedRequestContext = requestMap.get(CACHED_REQUEST_CONTEXT);
-        
-        // Catch both the null scenario and the 
-        // RequestContext-from-a-different-classloader scenario
-        if (cachedRequestContext instanceof RequestContext)
-        {
-          context = (RequestContext) cachedRequestContext;
-          context.attach();
-        }
-        else
-        {
-          RequestContextFactory factory = RequestContextFactory.getFactory();
-          if (factory == null)
-          {
-            RequestContextFactory.setFactory(new RequestContextFactoryImpl());
-            factory = RequestContextFactory.getFactory();
-          }
-
-          assert(factory != null);
-          context = factory.createContext(fContext.getExternalContext());
-          requestMap.put(CACHED_REQUEST_CONTEXT, context);
-        }
-      }
-    }
-  }
-
-  //
-  // Release the RequestContext if we created it.
-  //
-  static private void _releaseContextIfNecessary(FacesContext fContext)
-  {
-    Boolean createdContext = (Boolean)
-      fContext.getExternalContext().getRequestMap().get(_CREATED_CONTEXT_KEY);
-    if (Boolean.TRUE.equals(createdContext))
-    {
-      RequestContext context = RequestContext.getCurrentInstance();
-      if (context != null)
-        context.release();
-    }
-  }
   
   static public final String INITIAL_VIEW_ROOT_KEY =
     "org.apache.myfaces.trinidadinternal.InitialViewRoot";
 
-  static private final String _CREATED_CONTEXT_KEY =
-    "org.apache.myfaces.trinidadinternal.context.AdfFacesPhaseListener.CREATED_CONTEXT";
-
   static private final String _POSTBACK_KEY =
     "org.apache.myfaces.trinidadinternal.context.AdfFacesPhaseListener.POSTBACK";
   
-    
 }