You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by st...@apache.org on 2011/11/10 07:30:34 UTC

svn commit: r1200185 - in /myfaces/extensions/cdi/trunk: core/api/src/main/java/org/apache/myfaces/extensions/cdi/core/api/scope/conversation/ jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/ jee-...

Author: struberg
Date: Thu Nov 10 06:30:33 2011
New Revision: 1200185

URL: http://svn.apache.org/viewvc?rev=1200185&view=rev
Log:
EXTCDI-239 reset bean also when we hit a different view via GET

please note that this might have consequences on PRG pages.
But actually this has been the case before already, but now
it's much more consistent as the conversation always gets cleared
whenever we hit a new view via GET.

Thanks to Marcus Büttner for the patch ideas.

Added:
    myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/
    myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/
    myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/windowId.js   (with props)
Modified:
    myfaces/extensions/cdi/trunk/core/api/src/main/java/org/apache/myfaces/extensions/cdi/core/api/scope/conversation/RestScoped.java
    myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestConversationExpirationEvaluator.java
    myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestParameters.java
    myfaces/extensions/cdi/trunk/jee-modules/jsf20-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf2/impl/scope/conversation/JsfRestParameters.java
    myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/webapp/pages/restscoped/restScoped1.xhtml

Modified: myfaces/extensions/cdi/trunk/core/api/src/main/java/org/apache/myfaces/extensions/cdi/core/api/scope/conversation/RestScoped.java
URL: http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/core/api/src/main/java/org/apache/myfaces/extensions/cdi/core/api/scope/conversation/RestScoped.java?rev=1200185&r1=1200184&r2=1200185&view=diff
==============================================================================
--- myfaces/extensions/cdi/trunk/core/api/src/main/java/org/apache/myfaces/extensions/cdi/core/api/scope/conversation/RestScoped.java (original)
+++ myfaces/extensions/cdi/trunk/core/api/src/main/java/org/apache/myfaces/extensions/cdi/core/api/scope/conversation/RestScoped.java Thu Nov 10 06:30:33 2011
@@ -32,7 +32,9 @@ import static java.lang.annotation.Reten
 /**
  * The scope is similar to @ConversationScoped but it will
  * automatically end once the view get's invoked with a GET and the
- * viewParams are different than on the previous invoke
+ * viewParams are different than on the previous invoke.
+ * The conversation will also get resetted when you hit a different
+ * View via a GET request.
  */
 @Target({METHOD,TYPE,FIELD})
 @Retention(RUNTIME)

Modified: myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestConversationExpirationEvaluator.java
URL: http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestConversationExpirationEvaluator.java?rev=1200185&r1=1200184&r2=1200185&view=diff
==============================================================================
--- myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestConversationExpirationEvaluator.java (original)
+++ myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestConversationExpirationEvaluator.java Thu Nov 10 06:30:33 2011
@@ -24,9 +24,14 @@ import org.apache.myfaces.extensions.cdi
 
 import javax.enterprise.inject.Typed;
 import javax.enterprise.inject.spi.BeanManager;
+import javax.faces.context.FacesContext;
 
 /**
  * Evaluates when a @RestScoped Conversation context needs to get restarted.
+ * Each Conversation gets it's own ExpirationEvaluator. We just use the RestParameters
+ * to get the actual viewParams depending on the underlying technology.
+ * This can be done evaluating JSF-2 <f:viewParam> or an
+ * own implementation (e.g. for PrettyFaces).
  */
 @Typed() // this is not a CDI bean
 class RestConversationExpirationEvaluator implements ConversationExpirationEvaluator
@@ -37,6 +42,8 @@ class RestConversationExpirationEvaluato
 
     private RestParameters restParameters;
 
+    private String oldRestId;
+
     RestConversationExpirationEvaluator(BeanManager beanManager, AccessDecisionVoterContext accessDecisionVoterContext)
     {
         this.accessDecisionVoterContext = accessDecisionVoterContext;
@@ -56,7 +63,28 @@ class RestConversationExpirationEvaluato
         }
 
         // if the view params changed, then our Conversation must expire.
-        return restParameters.checkForNewViewParameters();
+        return checkRestId();
+    }
+
+    public boolean checkRestId()
+    {
+        FacesContext facesContext = FacesContext.getCurrentInstance();
+        if (facesContext == null)
+        {
+            // this might happen if we are outside the JSF-Servlet, e.g. in a ServletFilter.
+            return false;
+        }
+
+
+        if (restParameters.isPostback())
+        {
+            // we ignore POST requests
+            return false;
+        }
+
+        String currentRestId = restParameters.getRestId();
+
+        return currentRestId != null && !currentRestId.equals(oldRestId);
     }
 
     /**
@@ -64,7 +92,15 @@ class RestConversationExpirationEvaluato
      */
     public void touch()
     {
-        // do nothing
+        if (oldRestId == null)
+        {
+            FacesContext facesContext = FacesContext.getCurrentInstance();
+            if (facesContext == null)
+            {
+                return;
+            }
+            oldRestId = restParameters.getRestId();
+        }
     }
 
     /**
@@ -72,7 +108,7 @@ class RestConversationExpirationEvaluato
      */
     public void expire()
     {
-        restParameters.reset();
+        oldRestId = null;
     }
 
 }

Modified: myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestParameters.java
URL: http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestParameters.java?rev=1200185&r1=1200184&r2=1200185&view=diff
==============================================================================
--- myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestParameters.java (original)
+++ myfaces/extensions/cdi/trunk/jee-modules/jsf-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf/impl/scope/conversation/RestParameters.java Thu Nov 10 06:30:33 2011
@@ -31,10 +31,8 @@ public abstract class RestParameters
      * @return <code>true</code> if the viewParameters are now different than at the last invocation.
      *         In this default implementation it always returns false!
      */
-    public abstract boolean checkForNewViewParameters();
+    public abstract String getRestId();
+
+    public abstract boolean isPostback();
 
-    /**
-     * This method will get called to reset the stored viewParameters
-     */
-    public abstract void reset();
 }

Modified: myfaces/extensions/cdi/trunk/jee-modules/jsf20-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf2/impl/scope/conversation/JsfRestParameters.java
URL: http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/jee-modules/jsf20-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf2/impl/scope/conversation/JsfRestParameters.java?rev=1200185&r1=1200184&r2=1200185&view=diff
==============================================================================
--- myfaces/extensions/cdi/trunk/jee-modules/jsf20-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf2/impl/scope/conversation/JsfRestParameters.java (original)
+++ myfaces/extensions/cdi/trunk/jee-modules/jsf20-module/impl/src/main/java/org/apache/myfaces/extensions/cdi/jsf2/impl/scope/conversation/JsfRestParameters.java Thu Nov 10 06:30:33 2011
@@ -18,22 +18,16 @@
  */
 package org.apache.myfaces.extensions.cdi.jsf2.impl.scope.conversation;
 
-import org.apache.myfaces.extensions.cdi.core.api.scope.conversation.WindowScoped;
-import org.apache.myfaces.extensions.cdi.jsf.api.listener.phase.AfterPhase;
-import org.apache.myfaces.extensions.cdi.jsf.api.listener.phase.JsfPhaseId;
 import org.apache.myfaces.extensions.cdi.jsf.impl.scope.conversation.RestParameters;
 
-import javax.enterprise.event.Observes;
+import javax.enterprise.context.RequestScoped;
 import javax.faces.component.UIViewParameter;
 import javax.faces.component.UIViewRoot;
 import javax.faces.context.FacesContext;
-import javax.faces.event.PhaseEvent;
 import javax.faces.view.ViewMetadata;
 import java.io.Serializable;
 import java.util.Collection;
-import java.util.Map;
 import java.util.TreeSet;
-import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * This class holds information about the last used RestParameters for a given JSF view.
@@ -41,88 +35,69 @@ import java.util.concurrent.ConcurrentHa
  * It will expire the conversation when any of those Views get accessed via GET with
  * a different set of &lt;f:viewParam&gt;s.
  */
-@WindowScoped
+@RequestScoped
 public class JsfRestParameters extends RestParameters implements Serializable
 {
     private static final long serialVersionUID = 1349109309042072780L;
 
     /**
-     * This flag will be used to remember a storage request;
+     * We cache the viewParams values as long as the viewId remains the same
+     * for this very request. We do this because evaluating the
+     * viewParams with every bean invocation is very expensive.
      */
-    private boolean resetPending;
+    private String restId = null;
 
     /**
-     * key= viewId
-     * value= concatenated viewParam names + values
-     *
-     * We use @WindowScoped to automatically include the windowId in a correct way.
-     *
-     * TODO we might change this to only store a hashKey.
+     * Used to determine when we need to recalculate {@link #restId}
      */
-    private Map<String, String> viewParametersForViewId = new ConcurrentHashMap<String, String>();
+    private String oldViewId = null;
 
     /**
-     * Check and update the view parameters of the given viewId.
-     *
-     * @return <code>true</code> if the viewParameters are now different than at the last invocation
+     * This flag will be used to remember a storage request;
      */
-    public boolean checkForNewViewParameters()
+    public boolean isPostback()
     {
         FacesContext facesContext = FacesContext.getCurrentInstance();
+        return facesContext != null && facesContext.isPostback();
+    }
 
+    public String getRestId()
+    {
+        FacesContext facesContext = FacesContext.getCurrentInstance();
         if (facesContext == null)
         {
-            // this might happen if we are outside the JSF-Servlet, e.g. in a ServletFilter.
-            return false;
+            return null;
         }
-
-
-        if (facesContext.isPostback())
-        {
-            // we ignore POST requests
-            return false;
-        }
-
         String viewId = getViewId(facesContext);
-
         if (viewId == null)
         {
-            return false;
+            return null;
         }
 
-        String currentViewParams = getViewParams(facesContext, viewId);
-        String oldViewParams = viewParametersForViewId.get(viewId);
-
-        if (!currentViewParams.equals(oldViewParams))
+        if (oldViewId != null && oldViewId.equals(viewId))
         {
-            viewParametersForViewId.put(viewId, currentViewParams);
-
-            if (resetPending)
-            {
-                // if a reset is pending, then we need to expire the context
-                resetPending = false;
-                return true;
-            }
-
-            // only reset the rest context if the oldViewParamaeters were different
-            // but not if they didn't got set yet
-            return oldViewParams != null;
+            // use the already calculated restId
+            return restId;
         }
 
-        resetPending = false;
-
-        return false;
+        restId = getViewParams(facesContext);
+        oldViewId = viewId;
+        return restId;
     }
 
     /**
      * @param facesContext current faces-context
-     * @param viewId current question
      * @return the concatenated String of all viewParamName=viewParamValue of the given viewId
      */
-    private String getViewParams(FacesContext facesContext, String viewId)
+    private String getViewParams(FacesContext facesContext)
     {
         Collection<UIViewParameter> currentViewParams = ViewMetadata.getViewParameters(facesContext.getViewRoot());
-        StringBuilder sb = new StringBuilder();
+        String viewId = getViewId(facesContext);
+        if (viewId == null)
+        {
+            return null;
+        }
+        StringBuilder sb = new StringBuilder(viewId).append("?");
 
         // for sorting the view params
         TreeSet<String> viewParamNames = new TreeSet<String>();
@@ -143,46 +118,9 @@ public class JsfRestParameters extends R
 
             sb.append(viewParamName).append("=").append(viewParamValue).append("+/+");
         }
-
         return sb.toString();
     }
 
-    @Override
-    public void reset()
-    {
-        viewParametersForViewId.clear();
-        resetPending = true;
-    }
-
-    /**
-     * We need to store the view params after render response because
-     * we do not get them in the first initial view invocation when
-     * a new windowid got detected.
-     */
-    public void afterRenderResponse(@Observes @AfterPhase(JsfPhaseId.RENDER_RESPONSE) PhaseEvent phaseEvent)
-    {
-        if (resetPending)
-        {
-            resetPending = false;
-            return;
-        }
-
-        FacesContext facesContext = phaseEvent.getFacesContext();
-
-        // we ignore postbacks
-        if (facesContext.isPostback())
-        {
-            return;
-        }
-
-        String viewId = getViewId(facesContext);
-        if (viewId != null)
-        {
-            viewParametersForViewId.put(viewId, getViewParams(facesContext, viewId));
-        }
-    }
-
-
     private String getViewId(FacesContext facesContext)
     {
         String viewId = null;

Added: myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/windowId.js
URL: http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/windowId.js?rev=1200185&view=auto
==============================================================================
--- myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/windowId.js (added)
+++ myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/windowId.js Thu Nov 10 06:30:33 2011
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+function urlWithoutWindowId(base) {
+    var query = base;
+    var vars = query.split(/&|\?/g);
+    var newQuery = "";
+    var iParam = 0;
+    for (var i=0; vars != null && i < vars.length; i++) {
+        var pair = vars[i].split("=");
+        if (pair.length == 1) {
+            newQuery = pair[0];
+        }
+        else {
+            if (pair[0] != "windowId") {
+                var amp = iParam++ > 0 ? "&" : "?";
+                newQuery =  newQuery + amp + pair[0] + "=" + pair[1];
+            }
+        }
+    }
+    return newQuery;
+}
+
+function assertWindowId() {
+    var freshWindow = window.name.length < 1;
+    if (freshWindow) {
+        url = urlWithoutWindowId(window.location.href);
+        window.name = "window";
+        window.location = url;
+    }
+}

Propchange: myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/resources/META-INF/resources/js/windowId.js
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/webapp/pages/restscoped/restScoped1.xhtml
URL: http://svn.apache.org/viewvc/myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/webapp/pages/restscoped/restScoped1.xhtml?rev=1200185&r1=1200184&r2=1200185&view=diff
==============================================================================
--- myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/webapp/pages/restscoped/restScoped1.xhtml (original)
+++ myfaces/extensions/cdi/trunk/test-modules/cargo-test-module/src/main/webapp/pages/restscoped/restScoped1.xhtml Thu Nov 10 06:30:33 2011
@@ -24,30 +24,36 @@
       xmlns:f="http://java.sun.com/jsf/core"
       xmlns:h="http://java.sun.com/jsf/html"
       xmlns:ui="http://java.sun.com/jsf/facelets">
-<f:metadata>
-    <f:viewParam name="param" value="#{restscoped.param}"/>
-    <f:event type="javax.faces.event.PreRenderViewEvent" listener="#{restscoped.preRender}"/>
-</f:metadata>
+<f:view>
+    <h:outputScript library="js" name="windowId.js"/>
+    <script type="text/javascript">
+        assertWindowId()
+    </script>
+    <f:metadata>
+        <f:viewParam name="param" value="#{restscoped.param}"/>
+        <f:event type="javax.faces.event.PreRenderViewEvent" listener="#{restscoped.preRender}"/>
+    </f:metadata>
 
-<h:head>
-    <title>Apache MyFaces CODI - RestScoped1</title>
-</h:head>
-<h:body>
+    <h:head>
+        <title>Apache MyFaces CODI - RestScoped1</title>
+    </h:head>
+    <h:body>
 
-    <h2>RestScoped1</h2>
+        <h2>RestScoped1</h2>
 
-    <h:form id="restscoped1">
-        <p>
-            Name from REST: <h:outputText value="#{restscoped.param}" />
-        </p>
-        <p>
-            Invocation Counter: <h:outputText value="#{restscoped.invocationCount}" />
-        </p>
+        <h:form id="restscoped1">
+            <p>
+                Name from REST: <h:outputText value="#{restscoped.param}" />
+            </p>
+            <p>
+                Invocation Counter: <h:outputText value="#{restscoped.invocationCount}" />
+            </p>
 
-        <h:link id="nextPage" value="Same Page" outcome="restScoped1.xhtml">
-            <f:param name="param" value="#{restscoped.param}"/>
-        </h:link>
-    </h:form>
+            <h:link id="nextPage" value="Same Page" outcome="restScoped1.xhtml">
+                <f:param name="param" value="#{restscoped.param}"/>
+            </h:link>
+        </h:form>
 
-</h:body>
+    </h:body>
+</f:view>
 </html>