You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by mf...@apache.org on 2011/03/04 01:30:16 UTC

svn commit: r1076900 - /myfaces/portlet-bridge/core/trunk_2.0.x/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java

Author: mfreedman
Date: Fri Mar  4 00:30:15 2011
New Revision: 1076900

URL: http://svn.apache.org/viewvc?rev=1076900&view=rev
Log:
PORTLETBRIDGE-185: NPE in restore resource request if scope isn't in Map

Modified:
    myfaces/portlet-bridge/core/trunk_2.0.x/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java

Modified: myfaces/portlet-bridge/core/trunk_2.0.x/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java
URL: http://svn.apache.org/viewvc/myfaces/portlet-bridge/core/trunk_2.0.x/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java?rev=1076900&r1=1076899&r2=1076900&view=diff
==============================================================================
--- myfaces/portlet-bridge/core/trunk_2.0.x/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java (original)
+++ myfaces/portlet-bridge/core/trunk_2.0.x/impl/src/main/java/org/apache/myfaces/portlet/faces/bridge/BridgeImpl.java Fri Mar  4 00:30:15 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
-      scopeId = getRequestScopeId(request);
-    }
+    // 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,34 +1041,26 @@ public class BridgeImpl
     if ((redirectParams != null))
     {
       redirectRender(context,
-        lifecycle, request, response,
+        lifecycle, request, response, scopeId,
         redirectParams, preExistingAttributes);
       context = FacesContext.getCurrentInstance();
-      redirectedDuringRender = true;
     }
 
-    // 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) 
     {
-      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);
-      updateViewInfo(context, scopeId);
+      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
+    context.getExternalContext().getRequestMap().put(SCOPE_VIEW_KEY, getScopeViewKey(context.getExternalContext()));
+    saveFacesMessageState(context);
+    saveBridgeRequestScopeData(context, scopeId, preExistingAttributes, BridgeUtil.getPortletRequestPhase() == Bridge.PortletPhase.RESOURCE_PHASE);
+    updateViewInfo(context, scopeId);
   }
 
   public void doFacesRequest(ResourceRequest request, ResourceResponse response)
@@ -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();
@@ -1854,16 +1843,9 @@ public class BridgeImpl
   {
 
     // get the render scope this resource request is contained in
-    String scopeId = getRequestScopeId(request);    
-    Map<String, Object> m = null;
+    String scopeId = getRequestScopeId(request);   
     
-    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();
 
@@ -2020,8 +2022,8 @@ public class BridgeImpl
         requestScopeMap = createRequestScopeMap(portletContext);
         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);