Issue #12339 change SessionCache default flushOnResponseCommit=true

documentation/jetty/modules/operations-guide/pages/session/index.adoc

@@ -206,10 +206,11 @@ Boolean, default `false`.
 Controls whether the session cache should ask a `SessionDataStore` to delete a session that cannot be restored - for example because it is corrupted.
 
 jetty.session.flushOnResponseCommit::
-Boolean, default `false`.
-If true, if a session is "dirty" - ie its attributes have changed - it will be written to the `SessionDataStore` as the response is about to commit.
-This ensures that all subsequent requests whether to the same or different node will see the updated session data.
+Boolean, default `true`.
+If true, if a session is "dirty" - ie it has never been saved or its attributes have changed - it will be written to the `SessionDataStore` as the response is about to commit.
+This ensures that all subsequent requests - whether to the same or different node - will see the updated session data.
 If false, a dirty session will only be written to the backing store when the last simultaneous request for it leaves the session.
+Be aware that in this case, the write can happen _after_ the response is returned to the client.
 
 jetty.session.invalidateOnShutdown::
 Boolean, default `false`.

jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml

@@ -13,7 +13,7 @@
         <Set name="saveOnInactiveEviction"><Property name="jetty.session.saveOnInactiveEviction" default="false" /></Set>
         <Set name="saveOnCreate"><Property name="jetty.session.saveOnCreate" default="false" /></Set>
         <Set name="removeUnloadableSessions"><Property name="jetty.session.removeUnloadableSessions" default="false"/></Set>
-        <Set name="flushOnResponseCommit"><Property name="jetty.session.flushOnResponseCommit" default="false"/></Set>
+        <Set name="flushOnResponseCommit"><Property name="jetty.session.flushOnResponseCommit" default="true"/></Set>
         <Set name="invalidateOnShutdown"><Property name="jetty.session.invalidateOnShutdown" default="false"/></Set>
       </New>
     </Arg>

jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-null.xml

@@ -11,7 +11,7 @@
       <New class="org.eclipse.jetty.session.NullSessionCacheFactory">
         <Set name="saveOnCreate"><Property name="jetty.session.saveOnCreate" default="false" /></Set>
         <Set name="removeUnloadableSessions"><Property name="jetty.session.removeUnloadableSessions" default="false" /></Set>
-        <Set name="flushOnResponseCommit"><Property name="jetty.session.flushOnResponseCommit" default="false" /></Set>
+        <Set name="flushOnResponseCommit"><Property name="jetty.session.flushOnResponseCommit" default="true" /></Set>
       </New>
     </Arg>
   </Call>

jetty-core/jetty-server/src/main/config/modules/session-cache-hash.mod

@@ -23,5 +23,5 @@ etc/sessions/session-cache-hash.xml
 #jetty.session.saveOnInactiveEviction=false
 #jetty.session.saveOnCreate=false
 #jetty.session.removeUnloadableSessions=false
-#jetty.session.flushOnResponseCommit=false
+#jetty.session.flushOnResponseCommit=true
 #jetty.session.invalidateOnShutdown=false

jetty-core/jetty-server/src/main/config/modules/session-cache-null.mod

@@ -18,4 +18,4 @@ etc/sessions/session-cache-null.xml
 [ini-template]
 #jetty.session.saveOnCreate=false
 #jetty.session.removeUnloadableSessions=false
-#jetty.session.flushOnResponseCommit=false
+#jetty.session.flushOnResponseCommit=true

jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCache.java

@@ -93,7 +93,7 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
      * If true, when a response is about to be committed back to the client,
      * a dirty session will be flushed to the session store.
      */
-    protected boolean _flushOnResponseCommit;
+    protected boolean _flushOnResponseCommit = true;
 
     /**
      * If true, when the server shuts down, all sessions in the

jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCacheFactory.java

@@ -25,7 +25,7 @@ public abstract class AbstractSessionCacheFactory implements SessionCacheFactory
     boolean _saveOnInactiveEvict;
     boolean _saveOnCreate;
     boolean _removeUnloadableSessions;
-    boolean _flushOnResponseCommit;
+    boolean _flushOnResponseCommit = true;
     boolean _invalidateOnShutdown;
 
     public abstract SessionCache newSessionCache(SessionManager manager);

jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/DefaultSessionCacheTest.java

@@ -616,7 +616,7 @@ public void testSaveOnEvictionFail() throws Exception
         DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(sessionManager);
 
         //test values: allow first save, fail evict save, allow save
-        FailableSessionDataStore sessionDataStore = new FailableSessionDataStore(new boolean[]{true, false, true});
+        FailableSessionDataStore sessionDataStore = new FailableSessionDataStore(new boolean[]{true, true, false, true});
         cache.setSessionDataStore(sessionDataStore);
         sessionManager.setSessionCache(cache);
         server.addBean(sessionManager);
@@ -661,4 +661,21 @@ public void testSaveOnEvictionFail() throws Exception
             assertTrue(data.getLastSaved() > lastSaved);
         }
     }
+
+    @Test
+    public void testFlushOnResponseCommitDefault() throws Exception
+    {
+        //test factory defaults to flushOnResponseCommit==true
+        DefaultSessionCacheFactory sessionCacheFactory = new DefaultSessionCacheFactory();
+        assertTrue(sessionCacheFactory.isFlushOnResponseCommit());
+
+        //test cache produced by factory defaults to flushOnResponseCommit==true
+        DefaultSessionCache cacheFromFactory = (DefaultSessionCache)sessionCacheFactory.newSessionCache(new TestableSessionManager());
+        assertTrue(cacheFromFactory.isFlushOnResponseCommit());
+
+        //test cache defaults to flushOnResponseCommit==true
+        DefaultSessionCache sessionCache = new DefaultSessionCache(new TestableSessionManager());
+        assertTrue(sessionCache.isFlushOnResponseCommit());
+    }
+
 }

jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/NullSessionCacheTest.java

@@ -204,4 +204,21 @@ public void testDelete()
         assertFalse(store.exists("1234"));
         assertFalse(cache.contains("1234"));
     }
+
+    @Test
+    public void testFlushOnResponseCommitDefault() throws Exception
+    {
+        //test factory defaults to flushOnResponseCommit==true
+        NullSessionCacheFactory sessionCacheFactory = new NullSessionCacheFactory();
+        assertTrue(sessionCacheFactory.isFlushOnResponseCommit());
+
+        //test cache produced by factory defaults to flushOnResponseCommit==true
+        NullSessionCache cacheFromFactory = (NullSessionCache)sessionCacheFactory.newSessionCache(new TestableSessionManager());
+        assertTrue(cacheFromFactory.isFlushOnResponseCommit());
+
+        //test cache defaults to flushOnResponseCommit==true
+        NullSessionCache sessionCache = new NullSessionCache(new TestableSessionManager());
+        assertTrue(sessionCache.isFlushOnResponseCommit());
+    }
+
 }