From f8ff191872c6f1aef291165f9870551dbd9eb24a Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Tue, 8 Oct 2024 16:56:34 +1100 Subject: [PATCH] Issue #12339 change SessionCache default flushOnResponseCommit=true (#12344) * Issue #12339 change SessionCache default flushOnResponseCommit=true --- .../operations-guide/pages/session/index.adoc | 7 ++++--- .../etc/sessions/session-cache-hash.xml | 2 +- .../etc/sessions/session-cache-null.xml | 2 +- .../config/modules/session-cache-hash.mod | 2 +- .../config/modules/session-cache-null.mod | 2 +- .../jetty/session/AbstractSessionCache.java | 2 +- .../session/AbstractSessionCacheFactory.java | 2 +- .../session/DefaultSessionCacheTest.java | 19 ++++++++++++++++++- .../jetty/session/NullSessionCacheTest.java | 17 +++++++++++++++++ 9 files changed, 45 insertions(+), 10 deletions(-) diff --git a/documentation/jetty/modules/operations-guide/pages/session/index.adoc b/documentation/jetty/modules/operations-guide/pages/session/index.adoc index 64d5bbae950a..f71a7a5fb697 100644 --- a/documentation/jetty/modules/operations-guide/pages/session/index.adoc +++ b/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`. diff --git a/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml b/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml index 5608e56e90c7..92e4361d572c 100644 --- a/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml +++ b/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml @@ -13,7 +13,7 @@ - + diff --git a/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-null.xml b/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-null.xml index 721eb48ba20f..1a953ad74c75 100644 --- a/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-null.xml +++ b/jetty-core/jetty-server/src/main/config/etc/sessions/session-cache-null.xml @@ -11,7 +11,7 @@ - + diff --git a/jetty-core/jetty-server/src/main/config/modules/session-cache-hash.mod b/jetty-core/jetty-server/src/main/config/modules/session-cache-hash.mod index aaedd0585ede..d7335e3b294f 100644 --- a/jetty-core/jetty-server/src/main/config/modules/session-cache-hash.mod +++ b/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 diff --git a/jetty-core/jetty-server/src/main/config/modules/session-cache-null.mod b/jetty-core/jetty-server/src/main/config/modules/session-cache-null.mod index e7da2bbb42c9..b2092fef0026 100644 --- a/jetty-core/jetty-server/src/main/config/modules/session-cache-null.mod +++ b/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 diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCache.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCache.java index 3986a37df724..b9fbb928ba8c 100644 --- a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCache.java +++ b/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 diff --git a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCacheFactory.java b/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCacheFactory.java index 045b5a71a825..77c44f32196b 100644 --- a/jetty-core/jetty-session/src/main/java/org/eclipse/jetty/session/AbstractSessionCacheFactory.java +++ b/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); diff --git a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/DefaultSessionCacheTest.java b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/DefaultSessionCacheTest.java index 7b163d95477d..328897abe134 100644 --- a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/DefaultSessionCacheTest.java +++ b/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()); + } + } diff --git a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/NullSessionCacheTest.java b/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/NullSessionCacheTest.java index c0dbba1cd71d..3236aab1f1a2 100644 --- a/jetty-core/jetty-session/src/test/java/org/eclipse/jetty/session/NullSessionCacheTest.java +++ b/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()); + } + }