Merge remote-tracking branch 'origin/master' into servlet-3.1-api

jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java

@@ -162,55 +162,50 @@ public class HttpOutput extends ServletOutputStream
         if (isClosed())
             throw new EOFException("Closed");
 
-        // Do we have an aggregate buffer ?
+        _written+=len;
-        if (_aggregate == null)
+        boolean complete=_channel.getResponse().isAllContentWritten(_written);
-        {
+        int capacity = getBufferSize();
-            // NO - should we have an aggregate buffer? yes if this write will easily fit in it
+
-            int size = getBufferSize();
+        // Should we aggregate?
-            if (len<=size/2)
+        if (!complete && len<=capacity/4)
-            {
-                _aggregate = _channel.getByteBufferPool().acquire(size, false);
-                BufferUtil.append(_aggregate, b, off, len);
-                _written += len;
-                closeIfAllContentWritten();
-                return;
-            }
-        }
-        else
         {
+            if (_aggregate == null)
+                _aggregate = _channel.getByteBufferPool().acquire(capacity, false);
+
             // YES - fill the aggregate with content from the buffer
             int filled = BufferUtil.fill(_aggregate, b, off, len);
-            _written += filled;
 
-            // if closed or there is no content left over and we are not full, then we are done
+            // return if we are not complete, not full and filled all the content
-            if (closeIfAllContentWritten() || filled==len && !BufferUtil.isFull(_aggregate))
+            if (!complete && filled==len && !BufferUtil.isFull(_aggregate))
                 return;
 
+            // adjust offset/length
             off+=filled;
             len-=filled;
         }
 
-        // flush the aggregate
+        // flush any content from the aggregate
         if (BufferUtil.hasContent(_aggregate))
         {
-            _channel.write(_aggregate, false);
+            _channel.write(_aggregate, complete && len==0);
 
             // should we fill aggregate again from the buffer?
-            if (len<_aggregate.capacity()/2)
+            if (len>0 && !complete && len<=_aggregate.capacity()/4)
             {
                 BufferUtil.append(_aggregate, b, off, len);
-                _written += len;
-                closeIfAllContentWritten();
                 return;
             }
         }
 
         // write any remaining content in the buffer directly
-        _written += len;
+        if (len>0)
-        boolean complete=_channel.getResponse().isAllContentWritten(_written);
+            _channel.write(ByteBuffer.wrap(b, off, len), complete);
-        _channel.write(ByteBuffer.wrap(b, off, len), complete);
+
         if (complete)
+        {
+            closed();
             _channel.getResponse().closeOutput();
+        }
     }
 
 

jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSession.java

@@ -479,6 +479,12 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
     /* ------------------------------------------------------------ */
     @Override
     public void setAttribute(String name, Object value)
+    {
+        updateAttribute(name,value);
+    }
+    
+    /* ------------------------------------------------------------ */
+    protected boolean updateAttribute (String name, Object value)
     {
         Object old=null;
         synchronized (this)
@@ -495,8 +501,9 @@ public abstract class AbstractSession implements AbstractSessionManager.SessionI
                 bindValue(name,value);
 
             _manager.doSessionAttributeListeners(this,name,old,value);
-
+            return true;
         }
+        return false;
     }
 
     /* ------------------------------------------------------------ */

jetty-server/src/main/java/org/eclipse/jetty/server/session/JDBCSessionManager.java

@@ -244,8 +244,7 @@ public class JDBCSessionManager extends AbstractSessionManager
         @Override
         public void setAttribute (String name, Object value)
         {
-            super.setAttribute(name, value);
+            _dirty=updateAttribute(name, value);
-            _dirty=true;
         }
 
         @Override

jetty-server/src/test/java/org/eclipse/jetty/server/HttpManyWaysToCommitTest.java

@@ -327,7 +327,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
         SimpleHttpResponse response = executeRequest();
 
         assertThat("response code is 200", response.getCode(), is("200"));
-        assertResponseBody(response, "foobar");
+        assertResponseBody(response, "foobarfoobar");
         if (!"HTTP/1.0".equals(httpVersion))
             assertHeader(response, "transfer-encoding", "chunked");
     }
@@ -341,7 +341,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
         SimpleHttpResponse response = executeRequest();
 
         assertThat("response code is 200", response.getCode(), is("200"));
-        assertResponseBody(response, "foobar");
+        assertResponseBody(response, "foobarfoobar");
         if (!"HTTP/1.0".equals(httpVersion))
             assertHeader(response, "transfer-encoding", "chunked");
     }
@@ -372,7 +372,7 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
         {
             baseRequest.setHandled(true);
-            response.setBufferSize(3);
+            response.setBufferSize(4);
             response.getWriter().write("foobar");
             super.handle(target, baseRequest, request, response);
         }
@@ -389,9 +389,9 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
         {
             baseRequest.setHandled(true);
-            response.setBufferSize(3);
+            response.setBufferSize(8);
-            response.getWriter().write("f");
+            response.getWriter().write("fo");
-            response.getWriter().write("oobar");
+            response.getWriter().write("obarfoobar");
             super.handle(target, baseRequest, request, response);
         }
     }
@@ -407,7 +407,10 @@ public class HttpManyWaysToCommitTest extends AbstractHttpTest
         public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
         {
             baseRequest.setHandled(true);
-            response.setBufferSize(5);
+            response.setBufferSize(8);
+            response.getWriter().write("fo");
+            response.getWriter().write("ob");
+            response.getWriter().write("ar");
             response.getWriter().write("fo");
             response.getWriter().write("ob");
             response.getWriter().write("ar");

tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/DirtyAttributeTest.java

@@ -0,0 +1,225 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2013 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server.session;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.io.Serializable;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+import javax.servlet.http.HttpSessionActivationListener;
+import javax.servlet.http.HttpSessionBindingEvent;
+import javax.servlet.http.HttpSessionBindingListener;
+import javax.servlet.http.HttpSessionEvent;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.junit.Test;
+
+
+/**
+ * DirtyAttributeTest
+ *
+ * Check that repeated calls to setAttribute with the same value do not cause writes to the
+ * database - this is inferred via calls to passivate and activate  listeners.
+ *
+ *
+ */
+public class DirtyAttributeTest
+{
+    public static TestValue A_VALUE = new TestValue();
+    public static TestValue B_VALUE = new TestValue();
+    public static String THE_NAME = "__theName";
+    public static int INACTIVE = 4;
+    public static int SCAVENGE = 1;
+
+    @Test
+    public void testDiryWrite() throws Exception
+    {
+        AbstractTestServer server = new JdbcTestServer(0,INACTIVE,SCAVENGE);
+        
+        ServletContextHandler ctxA = server.addContext("/mod");
+        ctxA.addServlet(TestDirtyServlet.class, "/test");
+        
+        server.start();
+        int port=server.getPort();
+        try
+        {
+            HttpClient client = new HttpClient();
+            client.start();
+            try
+            {
+                // Perform a request to create a session              
+                ContentResponse response = client.GET("http://localhost:" + port + "/mod/test?action=create");
+                
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                String sessionCookie = response.getHeaders().getStringField("Set-Cookie");
+                assertTrue(sessionCookie != null);
+                // Mangle the cookie, replacing Path with $Path, etc.
+                sessionCookie = sessionCookie.replaceFirst("(\\W)(P|p)ath=", "$1\\$Path=");
+                
+                //do another request to change the session attribute
+                Request request = client.newRequest("http://localhost:" + port + "/mod/test?action=setA");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                A_VALUE.assertPassivatesEquals(1);
+                A_VALUE.assertActivatesEquals(1);
+                A_VALUE.assertBindsEquals(1);
+                A_VALUE.assertUnbindsEquals(0);
+                
+                //do another request using the cookie to try changing the session attribute to the same value again              
+                request= client.newRequest("http://localhost:" + port + "/mod/test?action=setA");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                A_VALUE.assertPassivatesEquals(1);
+                A_VALUE.assertActivatesEquals(1);
+                A_VALUE.assertBindsEquals(1);
+                A_VALUE.assertUnbindsEquals(0);
+                
+                //do another request using the cookie and change to a different value             
+                request= client.newRequest("http://localhost:" + port + "/mod/test?action=setB");
+                request.header("Cookie", sessionCookie);
+                response = request.send();
+                assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+                B_VALUE.assertPassivatesEquals(1);
+                B_VALUE.assertActivatesEquals(1);
+                B_VALUE.assertBindsEquals(1);
+                B_VALUE.assertUnbindsEquals(0);
+                A_VALUE.assertBindsEquals(1);
+                A_VALUE.assertUnbindsEquals(1);
+                
+            }
+            finally
+            {
+                client.stop();
+            }
+        }
+        finally
+        {
+            server.stop();
+        }
+    }
+    
+    public static class TestValue implements HttpSessionActivationListener, HttpSessionBindingListener, Serializable
+    {
+        int passivates = 0;
+        int activates = 0;
+        int binds = 0;
+        int unbinds = 0;
+        
+        /** 
+         * @see javax.servlet.http.HttpSessionActivationListener#sessionWillPassivate(javax.servlet.http.HttpSessionEvent)
+         */
+        public void sessionWillPassivate(HttpSessionEvent se)
+        {
+            ++passivates;
+        }
+
+        /** 
+         * @see javax.servlet.http.HttpSessionActivationListener#sessionDidActivate(javax.servlet.http.HttpSessionEvent)
+         */
+        public void sessionDidActivate(HttpSessionEvent se)
+        {
+           ++activates;
+        }
+        
+        public void assertPassivatesEquals (int expected)
+        {
+            assertEquals(expected, passivates);
+        }
+        
+        public void assertActivatesEquals (int expected)
+        {
+            assertEquals(expected, activates);
+        }
+        
+        public void assertBindsEquals (int expected)
+        {
+            assertEquals(expected, binds);
+        }
+        
+        public void assertUnbindsEquals (int expected)
+        {
+            assertEquals(expected, unbinds);
+        }
+        
+
+        /** 
+         * @see javax.servlet.http.HttpSessionBindingListener#valueBound(javax.servlet.http.HttpSessionBindingEvent)
+         */
+        public void valueBound(HttpSessionBindingEvent event)
+        {
+            ++binds;
+        }
+
+        /** 
+         * @see javax.servlet.http.HttpSessionBindingListener#valueUnbound(javax.servlet.http.HttpSessionBindingEvent)
+         */
+        public void valueUnbound(HttpSessionBindingEvent event)
+        {
+            ++unbinds;
+        }      
+    }
+    
+    public static class TestDirtyServlet extends HttpServlet
+    {
+        @Override
+        protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
+        {
+            String action = request.getParameter("action");
+            
+            if ("create".equals(action))
+            {
+                HttpSession session = request.getSession(true);
+                return;
+            }
+            
+            if ("setA".equals(action))
+            {
+                HttpSession session = request.getSession(false);
+                if (session == null)
+                    throw new ServletException("Session is null for action=change");
+
+                session.setAttribute(THE_NAME, A_VALUE);
+                return;
+            }
+            
+            if ("setB".equals(action))
+            {
+                HttpSession session = request.getSession(false);
+                if (session == null)
+                    throw new ServletException("Session does not exist");
+                session.setAttribute(THE_NAME, B_VALUE);
+                return;
+            }
+        }
+    }
+    
+}