svn commit: r634169 - in /portals/pluto/branches/pluto-1.1.x/pluto-container/src: main/java/org/apache/pluto/internal/impl/PortletRequestImpl.java test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java

[es...@apache.org]

Author: esm
Date: Wed Mar  5 21:12:47 2008
New Revision: 634169

URL: http://svn.apache.org/viewvc?rev=634169&view=rev
Log:
[PLUTO-474]: Fix for incorrectly invalidating http sessions due to an uninitialized last accessed time on the HttpSession.  Thanks Paul McMahan!  (added test case)

Added:
    portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java   (with props)
Modified:
    portals/pluto/branches/pluto-1.1.x/pluto-container/src/main/java/org/apache/pluto/internal/impl/PortletRequestImpl.java

Modified: portals/pluto/branches/pluto-1.1.x/pluto-container/src/main/java/org/apache/pluto/internal/impl/PortletRequestImpl.java
URL: http://svn.apache.org/viewvc/portals/pluto/branches/pluto-1.1.x/pluto-container/src/main/java/org/apache/pluto/internal/impl/PortletRequestImpl.java?rev=634169&r1=634168&r2=634169&view=diff
==============================================================================
--- portals/pluto/branches/pluto-1.1.x/pluto-container/src/main/java/org/apache/pluto/internal/impl/PortletRequestImpl.java (original)
+++ portals/pluto/branches/pluto-1.1.x/pluto-container/src/main/java/org/apache/pluto/internal/impl/PortletRequestImpl.java Wed Mar  5 21:12:47 2008
@@ -217,10 +217,10 @@
         if (httpSession != null) {
             // HttpSession is not null does NOT mean that it is valid.
             int maxInactiveInterval = httpSession.getMaxInactiveInterval();
-            if (maxInactiveInterval >= 0) {    // < 0 => Never expires.
+            long lastAccesstime = httpSession.getLastAccessedTime();
+            if (maxInactiveInterval >= 0 && lastAccesstime > 0) {    // < 0 => Never expires.
                 long maxInactiveTime = httpSession.getMaxInactiveInterval() * 1000L;
-                long currentInactiveTime = System.currentTimeMillis()
-                    - httpSession.getLastAccessedTime();
+                long currentInactiveTime = System.currentTimeMillis() - lastAccesstime;
                 if (currentInactiveTime > maxInactiveTime) {
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("The underlying HttpSession is expired and "
@@ -228,6 +228,9 @@
                     }
                     httpSession.invalidate();
                     httpSession = getHttpServletRequest().getSession(create);
+                    // a cached portletSession is no longer useable.
+                    // a new one will be created below.
+                    portletSession = null;
                 }
             }
         }

Added: portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java
URL: http://svn.apache.org/viewvc/portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java?rev=634169&view=auto
==============================================================================
--- portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java (added)
+++ portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java Wed Mar  5 21:12:47 2008
@@ -0,0 +1,112 @@
+/*
+ * 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.
+ */
+package org.apache.pluto.internal.impl;
+
+import org.apache.pluto.OptionalContainerServices;
+import org.apache.pluto.PortletContainer;
+import org.apache.pluto.RequiredContainerServices;
+import org.apache.pluto.core.PortletContainerImpl;
+import org.apache.pluto.internal.InternalPortletWindow;
+import org.jmock.Mock;
+import org.jmock.cglib.MockObjectTestCase;
+
+import javax.portlet.PortalContext;
+import javax.portlet.PortletContext;
+import javax.portlet.PortletSession;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpSession;
+
+/**
+ * Created by IntelliJ IDEA.
+ * User: esm
+ * Date: Mar 5, 2008
+ * Time: 9:50:43 PM
+ * To change this template use File | Settings | File Templates.
+ */
+public class PortletRequestImplTest extends MockObjectTestCase
+{
+    // Mock Objects
+    private Mock mockContainer = null;
+    private Mock mockServices = null;
+    private Mock mockPortalContext = null;
+    private Mock mockPortletContext = null;
+    private Mock mockHttpServletRequest = null;
+
+    private InternalPortletWindow window = null;
+
+    /* (non-Javadoc)
+     * @see junit.framework.TestCase#setUp()
+     */
+    protected void setUp( ) throws Exception
+    {
+        super.setUp();
+
+        // Create mocks
+        mockServices = mock( RequiredContainerServices.class );
+        mockPortalContext = mock( PortalContext.class );
+        mockPortletContext = mock( PortletContext.class );
+        mockContainer = mock( PortletContainerImpl.class,
+                new Class[] { String.class, RequiredContainerServices.class, OptionalContainerServices.class },
+                new Object[] { "Mock Pluto Container", (RequiredContainerServices) mockServices.proxy(), null } );
+        window = (InternalPortletWindow) mock( InternalPortletWindow.class ).proxy();
+        mockHttpServletRequest = mock( HttpServletRequest.class );
+
+        // Constructor expectations for RenderRequestImpl
+        mockContainer.expects( once() ).method( "getRequiredContainerServices" ).will( returnValue( mockServices.proxy() ) );
+        mockServices.expects( once() ).method( "getPortalContext" ).will( returnValue( mockPortalContext.proxy() ) );
+    }
+
+    /**
+     * Test for PLUTO-474.
+     */
+    public void testInvalidateSessionWithUnititializedLastAccessTime() throws Exception
+    {
+        // maximum inactive interval of the underlying PortletRequest's HttpSession
+        int maxInactiveInterval = 5; // in seconds
+
+        // last accessed time of the underlying PortletRequest's HttpSession
+        // A 'lastAccessedTime' of 0 emulates the behavior
+        // of a servlet container that doesn't initialize
+        // its value.
+        long lastAccessedTime = 0L;  // in milliseconds
+
+        // Create the render request that is under test, and initialize its state
+        RenderRequestImpl request = new RenderRequestImpl( (PortletContainer)mockContainer.proxy(), window, (HttpServletRequest)mockHttpServletRequest.proxy() );
+        request.init( (PortletContext)mockPortletContext.proxy(), ( HttpServletRequest)mockHttpServletRequest.proxy() );
+
+        // Mock the HttpSession, and set its expectations: it will return 0 for the last accessed time, and 5
+        // for the maximum inactive interval
+        Mock mockHttpSession = mock( HttpSession.class );
+        mockHttpSession.expects( once() ).method( "getLastAccessedTime" ).will( returnValue( lastAccessedTime ) );
+        // Prior to applying PLUTO-474, this expectation is invoked exactly twice, not once
+        mockHttpSession.expects( once() ).method( "getMaxInactiveInterval" ).will( returnValue( maxInactiveInterval ) );
+
+        // Set the expectation for the servlet request - it will return the mock http session
+        // Prior to applying PLUTO-474, this expectation is invoked exactly twice, not once
+        mockHttpServletRequest.expects( once() ).method( "getSession" ).will( returnValue( mockHttpSession.proxy() ) );
+
+        // this is the important expectation -
+        // Prior to applying PLUTO-474, the HttpSession was
+        // incorrectly determined to be invalid, and thus the
+        // HttpSession's invalidate() method was invoked.
+        //
+        // After applying PLUTO-474, invalidate() should never be called
+        mockHttpSession.expects( never() ).method( "invalidate" );
+        
+        PortletSession s = request.getPortletSession( true );
+    }
+}

Propchange: portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java
------------------------------------------------------------------------------
    svn:keywords = Id

Propchange: portals/pluto/branches/pluto-1.1.x/pluto-container/src/test/java/org/apache/pluto/internal/impl/PortletRequestImplTest.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain