myfaces-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mfreed...@apache.org
Subject svn commit: r1076904 - in /myfaces/portlet-bridge/core/branches/2.0.0-branch: ./ impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java
Date Fri, 04 Mar 2011 00:37:09 GMT
Author: mfreedman
Date: Fri Mar  4 00:37:09 2011
New Revision: 1076904

URL: http://svn.apache.org/viewvc?rev=1076904&view=rev
Log:
PORTLETBRIDGE-185: NPE in restore resource request if scope isn't in Map  This time its the
true fix rather than the one described in the previous bug (184) that merely got rid of the
NPE.

Modified:
    myfaces/portlet-bridge/core/branches/2.0.0-branch/   (props changed)
    myfaces/portlet-bridge/core/branches/2.0.0-branch/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java

Propchange: myfaces/portlet-bridge/core/branches/2.0.0-branch/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Mar  4 00:37:09 2011
@@ -1 +1 @@
-/myfaces/portlet-bridge/core/trunk_2.0.x:1063461-1071817,1075931
+/myfaces/portlet-bridge/core/trunk_2.0.x:1063461-1071817,1075931,1076900

Modified: myfaces/portlet-bridge/core/branches/2.0.0-branch/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java
URL: http://svn.apache.org/viewvc/myfaces/portlet-bridge/core/branches/2.0.0-branch/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java?rev=1076904&r1=1076903&r2=1076904&view=diff
==============================================================================
--- myfaces/portlet-bridge/core/branches/2.0.0-branch/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java
(original)
+++ myfaces/portlet-bridge/core/branches/2.0.0-branch/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java
Fri Mar  4 00:37:09 2011
@@ -29,6 +29,7 @@ import java.net.URL;
 import java.rmi.server.UID;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
@@ -392,7 +393,7 @@ public class BridgeImpl
           // within the same request scope but Faces does (assumes this),
           // preserve the request scope data and the Faces view tree at
           // RequestScope.
-          saveBridgeRequestScopeData(context, scopeId, preExistingAttributes);
+          saveBridgeRequestScopeData(context, scopeId, preExistingAttributes, false);
         }
 
       }
@@ -593,18 +594,7 @@ public class BridgeImpl
     FacesContext context = null;
     Lifecycle lifecycle = null;
     String scopeId = getRequestScopeId(request);
-    if (scopeId != null)
-    {
-      // Its possible we didn't detect the mode change but its the wrong scope
-      // as the scope is encoded with the mode -- confirm its right
-      StringBuffer sb = new StringBuffer(10);
-      String modeCheck = sb.append(":").append(request.getPortletMode().toString()).append(":").toString();
-      if (scopeId.indexOf(modeCheck) < 0 )
-      {
-        // scope is for a different mode
-        scopeId = null;
-      }
-    }
+
     boolean restoredScope = false;
     boolean removeScope = false;
     
@@ -699,7 +689,7 @@ public class BridgeImpl
         // within the same request scope but Faces does (assumes this),
         // preserve the request scope data and the Faces view tree at
         // RequestScope.
-        saveBridgeRequestScopeData(context, scopeId, preExistingAttributes);
+        saveBridgeRequestScopeData(context, scopeId, preExistingAttributes, false);
       }
       else
       {
@@ -852,28 +842,30 @@ public class BridgeImpl
     }
     
     
-    // never restore a scope if relying on render redirect cache
-    if (redirectParams == null && !clientDirectedView)
-    {
-      // If available -- restore the bridge request scope before getting the
-      // FacesContext in case anything in the context construction relies
-      // on these restored values.
-      // don't restore scope if mode changed
+    // Get the scopeId -- will be null if first render
       scopeId = getRequestScopeId(request);
-    }
     
-    if (scopeId != null)
-    { 
-      restoredScope = restoreBridgeRequestScopeData(request, scopeId);
-    }
-    else
+    // First render request or a clientDirectedView
+    if (scopeId == null)
     {
       // first request 
       // create a scope and store in the session until an action occurs
       // pass null as we aren't a StateAwareResponse
       scopeId = initBridgeRequestScope(request, null);
     }
-    
+    else if (clientDirectedView)
+    {
+      // Since the client has told us the specific view to use assume we shouldn't use existing
scope
+      clearBridgeRequestScopeData(scopeId);
+    }
+    else
+    {
+      // If available -- restore the bridge request scope before getting the
+      // FacesContext in case anything in the context construction relies
+      // on these restored values.
+      // don't restore scope if mode changed
+      restoredScope = restoreBridgeRequestScopeData(request, scopeId);
+    } 
 
     FacesContext context = null;
     try
@@ -986,8 +978,6 @@ public class BridgeImpl
       )
     throws BridgeException, BridgeUninitializedException, NullPointerException
   {
-    boolean redirectedDuringRender = false;
-
     // Note: if the scope wasn't restored then the Faces
     // FACES_VIEW_STATE
     // parameter will not have been carried into this render and hence
@@ -1035,11 +1025,10 @@ public class BridgeImpl
     else
     {
       redirectRender(context,
-        lifecycle, request, response,
+        lifecycle, request, response, scopeId,
         redirectParams, preExistingAttributes);
       
       context = FacesContext.getCurrentInstance();
-      redirectedDuringRender = true;
     }
 
 
@@ -1052,35 +1041,27 @@ public class BridgeImpl
     if ((redirectParams != null))
     {
       redirectRender(context,
-        lifecycle, request, response,
+        lifecycle, request, response, scopeId,
         redirectParams, preExistingAttributes);
       context = FacesContext.getCurrentInstance();
-      redirectedDuringRender = true;
     }
 
+    if (scopeId == null) 
+    {
+      if (!mDebugLoggingEnabled.booleanValue())
+        context.getExternalContext().log("WARNING: Unexpected situation -- reached end of
internal doFacesRender without a scopeId");
+      
+      // Don't expect to get here -- but just create a new scope
+      scopeId = createBridgeRequestScope(request);
+    }
+    
     // At a minimum we need to update the VIEW_STATE_PARAM being maintained
     // However, if this is a resource request then we also update the full scope
-
-    // Don't update if we have redirected during this render instead remove the scope
-    if (scopeId != null && !redirectedDuringRender)
-    {
       context.getExternalContext().getRequestMap().put(SCOPE_VIEW_KEY, getScopeViewKey(context.getExternalContext()));
       saveFacesMessageState(context);
-      saveBridgeRequestScopeData(context, scopeId, preExistingAttributes);
-      updateViewInfo(context, scopeId);
-    }
-    else
-    {
-      if (scopeId != null) removeRequestScopes(scopeId); 
-      // start a new/empty in session scope -- merely add the key used to discriminate whether

-      // a follow-on resource request targets the same view as the render (PPR) or a different
view (iFrame)
-      scopeId = initBridgeRequestScope(request, null);
-      HashMap<String, Object> m = (HashMap<String, Object>) new HashMap();
-      m.put(SCOPE_VIEW_KEY, getScopeViewKey(context.getExternalContext()));
-      putBridgeRequestScopeData(scopeId, m);
+    saveBridgeRequestScopeData(context, scopeId, preExistingAttributes, BridgeUtil.getPortletRequestPhase()
== Bridge.PortletPhase.RESOURCE_PHASE);
       updateViewInfo(context, scopeId);
     }
-  }
 
   public void doFacesRequest(ResourceRequest request, ResourceResponse response)
     throws BridgeException,
@@ -1516,6 +1497,7 @@ public class BridgeImpl
                               Lifecycle lifecycle,
                               PortletRequest request,
                               MimeResponse response,
+                              String scopeId,
                               QueryString redirectParams,
                               List<String> preExistingAttrs)
   {
@@ -1536,9 +1518,16 @@ public class BridgeImpl
     // into the new wrapped request so the redirect will use include
     Boolean hasRenderRedirectedAfterForward = (Boolean) request.getAttribute(HAS_RENDER_REDIRECTED_AFTER_FORWARD);
     
+    // Reset the scope
+    if (scopeId != null)
+    {
+      clearBridgeRequestScopeData(scopeId);
+    }
+    
     // close the FacesContext
     context.release();
     
+    
     // Now put the HAS_RENDER_REDIRECTED_AFTER_FORWARDflag back
     if (hasRenderRedirectedAfterForward != null)
     {
@@ -1558,7 +1547,7 @@ public class BridgeImpl
     context.getExternalContext().getSessionMap().remove(BridgeImpl.RENDER_REDIRECT_VIEWPARAMS);
     
     // Run lifecycle.execute again ... then render
-    doFacesRender(request, response, context, lifecycle, null, preExistingAttrs);
+    doFacesRender(request, response, context, lifecycle, scopeId, preExistingAttrs);
     
     // Reacquire the context as it may have changed if a recursive redirect render occurred
     context = FacesContext.getCurrentInstance();
@@ -1855,15 +1844,8 @@ public class BridgeImpl
 
     // get the render scope this resource request is contained in
     String scopeId = getRequestScopeId(request);    
-    Map<String, Object> m = null;
        
-    if (scopeId != null)
-    {
-      m = getScopeMap(scopeId);
-    }
-    
-    // Its possible to be passed a scope but for that scope to have been dropped from the
cache in the meantime
-    if (scopeId == null || m == null)
+    if (scopeId == null)
     {
       // first request is a resource request
       // create a scope and store in the session until an action occurs
@@ -1871,6 +1853,16 @@ public class BridgeImpl
       return initBridgeRequestScope(request, null);
     }
     
+    Map<String, Object> m = getScopeMap(scopeId);
+    
+    // If the scope is actually empty then this is first request -- hence 
+    // the resource must be targeting the same view
+    if (m == null || m.equals(Collections.EMPTY_MAP))
+    {
+      putBridgeRequestScopeData(scopeId, Collections.EMPTY_MAP, false);
+      return scopeId;
+    }
+    
     // Check to see if this resource request is targeting the same view or a different one
     Map<String, String> childResourceScopeMap = (Map<String, String>) m.get(CHILD_RESOURCE_REQUEST_SCOPE_MAP);
     String scopeIdKey = (String) m.get(SCOPE_VIEW_KEY);
@@ -1894,6 +1886,8 @@ public class BridgeImpl
       if (childScopeId == null)
       {
         childScopeId = createBridgeRequestScope(request);
+        // place scope in the scopeMap
+        clearBridgeRequestScopeData(childScopeId);
         
         if (childResourceScopeMap == null)
         {
@@ -1924,7 +1918,6 @@ public class BridgeImpl
 
   private String getRequestScopeId(PortletRequest request)
   {
-    boolean fromSession = false;
     String scopeId = request.getParameter(REQUEST_SCOPE_ID_RENDER_PARAM);
 
     if (scopeId == null)
@@ -1933,7 +1926,6 @@ public class BridgeImpl
       if (session != null)
       {
         scopeId = (String) session.getAttribute(BRIDGE_PACKAGE_PREFIX + REQUEST_SCOPE_ID_RENDER_PARAM);
-        fromSession = true;
       }
     }
     
@@ -1969,10 +1961,13 @@ public class BridgeImpl
 
   private String initBridgeRequestScope(PortletRequest request, StateAwareResponse response)
   {
-
-
+    // This call just gets us a new scopeId
     String requestScopeId = createBridgeRequestScope(request);
+    
+    // This call ensures the new scopeId is in the ScopeMap 
+    clearBridgeRequestScopeData(requestScopeId);
 
+    // Now ensure the scopeId can be recovered in the next request
     if (response != null)
     {
       // set in response render parameter so will receive in future calls
@@ -1990,21 +1985,28 @@ public class BridgeImpl
 
     return requestScopeId;
   }
+  
+  private void clearBridgeRequestScopeData(String scopeId)
+  {
+    notifyPreDestroy(getScopeMap(scopeId));
+    putBridgeRequestScopeData(scopeId, Collections.EMPTY_MAP, false);
+  }
 
   private void saveBridgeRequestScopeData(FacesContext context, String scopeId, 
-                                          List<String> preExistingList)
+                                          List<String> preExistingList, boolean mergeExisting)
   {
     // Store the RequestMap @ the bridge's request scope
     putBridgeRequestScopeData(scopeId, 
-                              copyRequestMap(context.getExternalContext().getRequestMap(),
preExistingList));
+                              copyRequestMap(context.getExternalContext().getRequestMap(),
preExistingList), mergeExisting);
 
     // flag the data so can remove it if the session terminates
     // as its unlikely useful if the session disappears
     watchScope(context, scopeId);
   }
+  
 
   @SuppressWarnings("unchecked")
-  private void putBridgeRequestScopeData(String scopeId, Map<String, Object> o)
+  private void putBridgeRequestScopeData(String scopeId, Map<String, Object> o, boolean
mergeExisting)
   {
     PortletContext portletContext = mPortletConfig.getPortletContext();
 
@@ -2021,7 +2023,7 @@ public class BridgeImpl
         portletContext.setAttribute(REQUEST_SCOPE_MAP, requestScopeMap);
       }
       
-      if (BridgeUtil.getPortletRequestPhase() == Bridge.PortletPhase.RESOURCE_PHASE)
+      if (mergeExisting)
       {
         // Because we didn't restore them at the beginning of the request
         // Merge any existing scope attributes that weren't written during this request
@@ -2164,12 +2166,12 @@ public class BridgeImpl
       return requestScopeMap.get(scopeId);
     }
   }
+  
 
   @SuppressWarnings("unchecked")
   private boolean restoreBridgeRequestScopeData(PortletRequest request, String scopeId)
     throws BridgeException
   {
-
     Map<String, Object> m;
     
     //TODO: Since this is a private method, is it easier to ensure scope id is not null here
thus replacing this with
@@ -2461,6 +2463,11 @@ public class BridgeImpl
   // notify this scope's attributes that they are being removed
   private void notifyPreDestroy(Map<String, Object> scope)
   {
+    if (scope == null || scope.equals(Collections.EMPTY_MAP))
+    {
+      return;
+    }
+    
     // Scopes can now contain a Map to child scopes -- first check to see if this scope 
     // contains such a Map -- if so them remove these scopes too.
     Map<String, String> childMap = (Map<String, String>) scope.get(CHILD_RESOURCE_REQUEST_SCOPE_MAP);



Mime
View raw message