From a563cdca7646713edb8323344efab02a5f4b1a6d Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 11 Sep 2019 09:10:35 +1000 Subject: [PATCH] Issue #4025 Add flushOnResponseCommit mode to sessions. (#4049) * Issue #4025 Add flushOnCommit mode to sessions. Added flushOnCommit mode to write a session to the backing store as the response commits, before any bytes are returned to the client. Signed-off-by: Jan Bartel --- .../session-configuration-sessioncache.adoc | 26 +- .../etc/sessions/session-cache-hash.xml | 3 +- .../etc/sessions/session-cache-null.xml | 6 +- .../config/modules/session-cache-hash.mod | 8 +- .../config/modules/session-cache-null.mod | 2 +- .../org/eclipse/jetty/server/HttpChannel.java | 6 +- .../org/eclipse/jetty/server/Request.java | 44 +- .../server/session/AbstractSessionCache.java | 57 +- .../session/AbstractSessionCacheFactory.java | 114 ++++ .../session/AbstractSessionDataStore.java | 12 +- .../session/DefaultSessionCacheFactory.java | 75 +-- .../server/session/NullSessionCache.java | 150 ----- .../session/NullSessionCacheFactory.java | 66 +-- .../eclipse/jetty/server/session/Session.java | 31 +- .../jetty/server/session/SessionCache.java | 26 +- .../jetty/server/session/SessionData.java | 28 + .../jetty/server/session/SessionHandler.java | 25 +- .../session/AbstractSessionCacheTest.java | 513 ++++++++++++++++++ .../session/DefaultSessionCacheTest.java | 309 +---------- .../server/session/NullSessionCacheTest.java | 383 +++++-------- .../session/SessionEvictionFailureTest.java | 2 + 21 files changed, 1045 insertions(+), 841 deletions(-) create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCacheFactory.java create mode 100644 tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AbstractSessionCacheTest.java diff --git a/jetty-documentation/src/main/asciidoc/administration/sessions/session-configuration-sessioncache.adoc b/jetty-documentation/src/main/asciidoc/administration/sessions/session-configuration-sessioncache.adoc index d06c0f74f28..cd7ebfa5a0e 100644 --- a/jetty-documentation/src/main/asciidoc/administration/sessions/session-configuration-sessioncache.adoc +++ b/jetty-documentation/src/main/asciidoc/administration/sessions/session-configuration-sessioncache.adoc @@ -34,6 +34,7 @@ Once the `session-cache-hash` module has been enabled, you can view a list of al #jetty.session.saveOnInactiveEvict=false #jetty.session.saveOnCreate=false #jetty.session.removeUnloadableSessions=false +#jetty.session.flushOnResponseCommit=false ---- jetty.session.evictionPolicy:: @@ -63,6 +64,12 @@ jetty.session.removeUnloadableSessions:: Boolean, default `false`. Controls whether a session that cannot be restored - for example because it is corrupted - from the `SessionDataStore` is deleted by the `SessionDataStore`. +jetty.session.flushOnResponseCommit:: +Boolean, default `false`. +If true, if a session is "dirty" - ie its attributes have changed - it will be written to the backing store 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. + For more general information on the uses of these configuration properties, see link:#sessions-details[Session Components]. @@ -72,4 +79,21 @@ The `NullSessionCache` is a trivial implementation of the `SessionCache` that do You may need to use it if your clustering setup does not have a sticky load balancer, or if you want absolutely minimal support for sessions. If you use this in conjunction with the `NullSessionDataStore`, then sessions will neither be retained in memory nor persisted. -To enable the `NullSessionCache`, enable the `sesssion-cache-null` link:#startup-modules[module]. +To enable the `NullSessionCache`, enable the `sesssion-cache-null` link:#startup-modules[module]. +Configuration options are: + +jetty.session.saveOnCreate:: +Boolean, default `false`. +Controls whether a session that is newly created will be immediately saved to the `SessionDataStore` or lazily saved as the last request for the session exits. + +jetty.session.removeUnloadableSessions:: +Boolean, default `false`. +Controls whether a session that cannot be restored - for example because it is corrupted - from the `SessionDataStore` is deleted by the `SessionDataStore`. + +jetty.session.flushOnResponseCommit:: +Boolean, default `false`. +If true, if a session is "dirty" - ie its attributes have changed - it will be written to the backing store 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. + +For more general information on the uses of these configuration properties, see link:#sessions-details[Session Components]. diff --git a/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml b/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml index 5280fb42f75..43e4b09a280 100644 --- a/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml +++ b/jetty-server/src/main/config/etc/sessions/session-cache-hash.xml @@ -4,7 +4,7 @@ - + @@ -13,6 +13,7 @@ + diff --git a/jetty-server/src/main/config/etc/sessions/session-cache-null.xml b/jetty-server/src/main/config/etc/sessions/session-cache-null.xml index 3f030bf05b4..a9dc52b83e6 100644 --- a/jetty-server/src/main/config/etc/sessions/session-cache-null.xml +++ b/jetty-server/src/main/config/etc/sessions/session-cache-null.xml @@ -11,11 +11,7 @@ - - - - - + diff --git a/jetty-server/src/main/config/modules/session-cache-hash.mod b/jetty-server/src/main/config/modules/session-cache-hash.mod index 32ab705c7a2..2d336bc1d99 100644 --- a/jetty-server/src/main/config/modules/session-cache-hash.mod +++ b/jetty-server/src/main/config/modules/session-cache-hash.mod @@ -1,10 +1,9 @@ DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html [description] -Enable first level session cache in ConcurrentHashMap. -If not enabled, sessions will use a HashSessionCache by default, so enabling -via this module is only needed if the configuration properties need to be -changed. +Enable first level session cache. If this module is not enabled, sessions will +use the DefaultSessionCache by default, so enabling via this module is only needed +if the configuration properties need to be changed from their defaults. [tags] session @@ -23,3 +22,4 @@ etc/sessions/session-cache-hash.xml #jetty.session.saveOnInactiveEvict=false #jetty.session.saveOnCreate=false #jetty.session.removeUnloadableSessions=false +#jetty.session.flushOnResponseCommit=false diff --git a/jetty-server/src/main/config/modules/session-cache-null.mod b/jetty-server/src/main/config/modules/session-cache-null.mod index 6069c8f8168..2a94f59cb82 100644 --- a/jetty-server/src/main/config/modules/session-cache-null.mod +++ b/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.writeThroughMode=ON_EXIT +#jetty.session.flushOnResponseCommit=false diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index e48d6f22156..11b0f74a2fb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -824,15 +824,15 @@ public class HttpChannel implements Runnable, HttpOutput.Interceptor if (info == null) info = _response.newResponseMetaData(); commit(info); - + _combinedListener.onResponseBegin(_request); + _request.onResponseCommit(); + // wrap callback to process 100 responses final int status = info.getStatus(); final Callback committed = (status < HttpStatus.OK_200 && status >= HttpStatus.CONTINUE_100) ? new Send100Callback(callback) : new SendCallback(callback, content, true, complete); - _combinedListener.onResponseBegin(_request); - // committing write _transport.send(info, _request.isHead(), content, complete, committed); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index ef94f6cbf80..cf417816ac5 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -222,7 +222,7 @@ public class Request implements HttpServletRequest private long _timeStamp; private MultiParts _multiParts; //if the request is a multi-part mime private AsyncContextState _async; - private List _sessions; //list of sessions used during lifetime of request + private List _sessions; //list of sessions used during lifetime of request public Request(HttpChannel channel, HttpInput input) { @@ -377,32 +377,41 @@ public class Request implements HttpServletRequest */ public void enterSession(HttpSession s) { - if (s == null) + if (!(s instanceof Session)) return; if (_sessions == null) _sessions = new ArrayList<>(); if (LOG.isDebugEnabled()) LOG.debug("Request {} entering session={}", this, s); - _sessions.add(s); + _sessions.add((Session)s); } /** * Complete this request's access to a session. * - * @param s the session + * @param session the session */ - private void leaveSession(HttpSession s) + private void leaveSession(Session session) { - if (s == null) - return; - - Session session = (Session)s; if (LOG.isDebugEnabled()) LOG.debug("Request {} leaving session {}", this, session); session.getSessionHandler().complete(session); } + /** + * A response is being committed for a session, + * potentially write the session out before the + * client receives the response. + * @param session the session + */ + private void commitSession(Session session) + { + if (LOG.isDebugEnabled()) + LOG.debug("Response {} committing for session {}", this, session); + session.getSessionHandler().commit(session); + } + private MultiMap getParameters() { if (!_contentParamsExtracted) @@ -1516,13 +1525,26 @@ public class Request implements HttpServletRequest */ public void onCompleted() { - if (_sessions != null && _sessions.size() > 0) + if (_sessions != null) { - for (HttpSession s:_sessions) + for (Session s:_sessions) leaveSession(s); } } + /** + * Called when a response is about to be committed, ie sent + * back to the client + */ + public void onResponseCommit() + { + if (_sessions != null) + { + for (Session s:_sessions) + commitSession(s); + } + } + /** * Find a session that this request has already entered for the * given SessionHandler diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java index 096bc9e6395..48ccac95481 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java @@ -91,6 +91,12 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements * deleted from the SessionDataStore. */ protected boolean _removeUnloadableSessions; + + /** + * 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; /** * Create a new Session object from pre-existing session data @@ -296,6 +302,18 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements _removeUnloadableSessions = removeUnloadableSessions; } + @Override + public void setFlushOnResponseCommit(boolean flushOnResponseCommit) + { + _flushOnResponseCommit = flushOnResponseCommit; + } + + @Override + public boolean isFlushOnResponseCommit() + { + return _flushOnResponseCommit; + } + /** * Get a session object. * @@ -501,6 +519,42 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements } } + /** + * A response that has accessed this session is about to + * be returned to the client. Pass the session to the store + * to persist, so that any changes will be visible to + * subsequent requests on the same node (if using NullSessionCache), + * or on other nodes. + */ + @Override + public void commit(Session session) throws Exception + { + if (session == null) + return; + + try (Lock lock = session.lock()) + { + //only write the session out at this point if the attributes changed. If only + //the lastAccess/expiry time changed defer the write until the last request exits + if (session.getSessionData().isDirty() && _flushOnResponseCommit) + { + if (LOG.isDebugEnabled()) + LOG.debug("Flush session {} on response commit", session); + //save the session + if (!_sessionDataStore.isPassivating()) + { + _sessionDataStore.store(session.getId(), session.getSessionData()); + } + else + { + session.willPassivate(); + _sessionDataStore.store(session.getId(), session.getSessionData()); + session.didActivate(); + } + } + } + } + /** * @deprecated */ @@ -739,6 +793,8 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements if (_sessionDataStore.isPassivating()) session.willPassivate(); + //Fake being dirty to force the write + session.getSessionData().setDirty(true); _sessionDataStore.store(session.getId(), session.getSessionData()); } @@ -748,7 +804,6 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements catch (Exception e) { LOG.warn("Passivation of idle session {} failed", session.getId(), e); - //session.updateInactivityTimer(); } } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCacheFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCacheFactory.java new file mode 100644 index 00000000000..b29e5585a19 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCacheFactory.java @@ -0,0 +1,114 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 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; + +/** + * AbstractSessionCacheFactory + * + * Base class for SessionCacheFactories. + * + */ +public abstract class AbstractSessionCacheFactory implements SessionCacheFactory +{ + int _evictionPolicy; + boolean _saveOnInactiveEvict; + boolean _saveOnCreate; + boolean _removeUnloadableSessions; + boolean _flushOnResponseCommit; + + /** + * @return the flushOnResponseCommit + */ + public boolean isFlushOnResponseCommit() + { + return _flushOnResponseCommit; + } + + /** + * @param flushOnResponseCommit the flushOnResponseCommit to set + */ + public void setFlushOnResponseCommit(boolean flushOnResponseCommit) + { + _flushOnResponseCommit = flushOnResponseCommit; + } + + /** + * @return the saveOnCreate + */ + public boolean isSaveOnCreate() + { + return _saveOnCreate; + } + + /** + * @param saveOnCreate the saveOnCreate to set + */ + public void setSaveOnCreate(boolean saveOnCreate) + { + _saveOnCreate = saveOnCreate; + } + + /** + * @return the removeUnloadableSessions + */ + public boolean isRemoveUnloadableSessions() + { + return _removeUnloadableSessions; + } + + /** + * @param removeUnloadableSessions the removeUnloadableSessions to set + */ + public void setRemoveUnloadableSessions(boolean removeUnloadableSessions) + { + _removeUnloadableSessions = removeUnloadableSessions; + } + + /** + * @return the evictionPolicy + */ + public int getEvictionPolicy() + { + return _evictionPolicy; + } + + /** + * @param evictionPolicy the evictionPolicy to set + */ + public void setEvictionPolicy(int evictionPolicy) + { + _evictionPolicy = evictionPolicy; + } + + /** + * @return the saveOnInactiveEvict + */ + public boolean isSaveOnInactiveEvict() + { + return _saveOnInactiveEvict; + } + + /** + * @param saveOnInactiveEvict the saveOnInactiveEvict to set + */ + public void setSaveOnInactiveEvict(boolean saveOnInactiveEvict) + { + _saveOnInactiveEvict = saveOnInactiveEvict; + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java index b876c2e81ef..01dc84e87d6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStore.java @@ -129,10 +129,14 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem long savePeriodMs = (_savePeriodSec <= 0 ? 0 : TimeUnit.SECONDS.toMillis(_savePeriodSec)); if (LOG.isDebugEnabled()) - LOG.debug("Store: id={}, dirty={}, lsave={}, period={}, elapsed={}", id, data.isDirty(), data.getLastSaved(), savePeriodMs, (System.currentTimeMillis() - lastSave)); + { + LOG.debug("Store: id={}, mdirty={}, dirty={}, lsave={}, period={}, elapsed={}", id, data.isMetaDataDirty(), + data.isDirty(), data.getLastSaved(), savePeriodMs, (System.currentTimeMillis() - lastSave)); + } - //save session if attribute changed or never been saved or time between saves exceeds threshold - if (data.isDirty() || (lastSave <= 0) || ((System.currentTimeMillis() - lastSave) >= savePeriodMs)) + //save session if attribute changed, never been saved or metadata changed (eg expiry time) and save interval exceeded + if (data.isDirty() || (lastSave <= 0) || + (data.isMetaDataDirty() && ((System.currentTimeMillis() - lastSave) >= savePeriodMs))) { //set the last saved time to now data.setLastSaved(System.currentTimeMillis()); @@ -140,7 +144,7 @@ public abstract class AbstractSessionDataStore extends ContainerLifeCycle implem { //call the specific store method, passing in previous save time doStore(id, data, lastSave); - data.setDirty(false); //only undo the dirty setting if we saved it + data.clean(); //unset all dirty flags } catch (Exception e) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCacheFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCacheFactory.java index b1261647414..87b945e5a0b 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCacheFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCacheFactory.java @@ -23,80 +23,8 @@ package org.eclipse.jetty.server.session; * * Factory for creating new DefaultSessionCaches. */ -public class DefaultSessionCacheFactory implements SessionCacheFactory +public class DefaultSessionCacheFactory extends AbstractSessionCacheFactory { - int _evictionPolicy; - boolean _saveOnInactiveEvict; - boolean _saveOnCreate; - boolean _removeUnloadableSessions; - - /** - * @return the saveOnCreate - */ - public boolean isSaveOnCreate() - { - return _saveOnCreate; - } - - /** - * @param saveOnCreate the saveOnCreate to set - */ - public void setSaveOnCreate(boolean saveOnCreate) - { - _saveOnCreate = saveOnCreate; - } - - /** - * @return the removeUnloadableSessions - */ - public boolean isRemoveUnloadableSessions() - { - return _removeUnloadableSessions; - } - - /** - * @param removeUnloadableSessions the removeUnloadableSessions to set - */ - public void setRemoveUnloadableSessions(boolean removeUnloadableSessions) - { - _removeUnloadableSessions = removeUnloadableSessions; - } - - /** - * @return the evictionPolicy - */ - public int getEvictionPolicy() - { - return _evictionPolicy; - } - - /** - * @param evictionPolicy the evictionPolicy to set - */ - public void setEvictionPolicy(int evictionPolicy) - { - _evictionPolicy = evictionPolicy; - } - - /** - * @return the saveOnInactiveEvict - */ - public boolean isSaveOnInactiveEvict() - { - return _saveOnInactiveEvict; - } - - /** - * @param saveOnInactiveEvict the saveOnInactiveEvict to set - */ - public void setSaveOnInactiveEvict(boolean saveOnInactiveEvict) - { - _saveOnInactiveEvict = saveOnInactiveEvict; - } - - /** - * @see org.eclipse.jetty.server.session.SessionCacheFactory#getSessionCache(org.eclipse.jetty.server.session.SessionHandler) - */ @Override public SessionCache getSessionCache(SessionHandler handler) { @@ -105,6 +33,7 @@ public class DefaultSessionCacheFactory implements SessionCacheFactory cache.setSaveOnInactiveEviction(isSaveOnInactiveEvict()); cache.setSaveOnCreate(isSaveOnCreate()); cache.setRemoveUnloadableSessions(isRemoveUnloadableSessions()); + cache.setFlushOnResponseCommit(isFlushOnResponseCommit()); return cache; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java index e22919eed7b..c589aab4bdb 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCache.java @@ -18,12 +18,7 @@ package org.eclipse.jetty.server.session; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; - import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSessionAttributeListener; -import javax.servlet.http.HttpSessionBindingEvent; /** * NullSessionCache @@ -35,151 +30,6 @@ import javax.servlet.http.HttpSessionBindingEvent; */ public class NullSessionCache extends AbstractSessionCache { - /** - * If the writethrough mode is ALWAYS or NEW, then use an - * attribute listener to ascertain when the attribute has changed. - * - */ - public class WriteThroughAttributeListener implements HttpSessionAttributeListener - { - Set _sessionsBeingWritten = ConcurrentHashMap.newKeySet(); - - @Override - public void attributeAdded(HttpSessionBindingEvent event) - { - doAttributeChanged(event); - } - - @Override - public void attributeRemoved(HttpSessionBindingEvent event) - { - doAttributeChanged(event); - } - - @Override - public void attributeReplaced(HttpSessionBindingEvent event) - { - doAttributeChanged(event); - } - - private void doAttributeChanged(HttpSessionBindingEvent event) - { - if (_writeThroughMode == WriteThroughMode.ON_EXIT) - return; - - Session session = (Session)event.getSession(); - - SessionDataStore store = getSessionDataStore(); - - if (store == null) - return; - - if (_writeThroughMode == WriteThroughMode.ALWAYS || (_writeThroughMode == WriteThroughMode.NEW && session.isNew())) - { - //ensure that a call to willPassivate doesn't result in a passivation - //listener removing an attribute, which would cause this listener to - //be called again - if (_sessionsBeingWritten.add(session)) - { - try - { - //should hold the lock on the session, but as sessions are never shared - //with the NullSessionCache, there can be no other thread modifying the - //same session at the same time (although of course there can be another - //request modifying its copy of the session data, so it is impossible - //to guarantee the order of writes). - if (store.isPassivating()) - session.willPassivate(); - store.store(session.getId(), session.getSessionData()); - if (store.isPassivating()) - session.didActivate(); - } - catch (Exception e) - { - LOG.warn("Write through of {} failed", e); - } - finally - { - _sessionsBeingWritten.remove(session); - } - } - } - } - } - - /** - * Defines the circumstances a session will be written to the backing store. - */ - public enum WriteThroughMode - { - /** - * ALWAYS means write through every attribute change. - */ - ALWAYS, - /** - * NEW means to write through every attribute change only - * while the session is freshly created, ie its id has not yet been returned to the client - */ - NEW, - /** - * ON_EXIT means write the session only when the request exits - * (which is the default behaviour of AbstractSessionCache) - */ - ON_EXIT - } - - private WriteThroughMode _writeThroughMode = WriteThroughMode.ON_EXIT; - protected WriteThroughAttributeListener _listener = null; - - /** - * @return the writeThroughMode - */ - public WriteThroughMode getWriteThroughMode() - { - return _writeThroughMode; - } - - /** - * @param writeThroughMode the writeThroughMode to set - */ - public void setWriteThroughMode(WriteThroughMode writeThroughMode) - { - if (getSessionHandler() == null) - throw new IllegalStateException("No SessionHandler"); - - //assume setting null is the same as ON_EXIT - if (writeThroughMode == null) - { - if (_listener != null) - getSessionHandler().removeEventListener(_listener); - _listener = null; - _writeThroughMode = WriteThroughMode.ON_EXIT; - return; - } - - switch (writeThroughMode) - { - case ON_EXIT: - { - if (_listener != null) - getSessionHandler().removeEventListener(_listener); - _listener = null; - break; - } - case NEW: - case ALWAYS: - { - if (_listener == null) - { - _listener = new WriteThroughAttributeListener(); - getSessionHandler().addEventListener(_listener); - } - break; - } - } - _writeThroughMode = writeThroughMode; - } - /** * @param handler The SessionHandler related to this SessionCache */ diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCacheFactory.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCacheFactory.java index dd4a4cd0986..40bf5f45f9d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCacheFactory.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCacheFactory.java @@ -18,75 +18,51 @@ package org.eclipse.jetty.server.session; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + /** * NullSessionCacheFactory * * Factory for NullSessionCaches. */ -public class NullSessionCacheFactory implements SessionCacheFactory +public class NullSessionCacheFactory extends AbstractSessionCacheFactory { - boolean _saveOnCreate; - boolean _removeUnloadableSessions; - NullSessionCache.WriteThroughMode _writeThroughMode; - - /** - * @return the writeThroughMode - */ - public NullSessionCache.WriteThroughMode getWriteThroughMode() + private static final Logger LOG = Log.getLogger("org.eclipse.jetty.server.session"); + + @Override + public int getEvictionPolicy() { - return _writeThroughMode; + return SessionCache.EVICT_ON_SESSION_EXIT; //never actually stored } - /** - * @param writeThroughMode the writeThroughMode to set - */ - public void setWriteThroughMode(NullSessionCache.WriteThroughMode writeThroughMode) + @Override + public void setEvictionPolicy(int evictionPolicy) { - _writeThroughMode = writeThroughMode; + if (LOG.isDebugEnabled()) + LOG.debug("Ignoring eviction policy setting for NullSessionCaches"); } - /** - * @return the saveOnCreate - */ - public boolean isSaveOnCreate() + @Override + public boolean isSaveOnInactiveEvict() { - return _saveOnCreate; + return false; //never kept in cache } - /** - * @param saveOnCreate the saveOnCreate to set - */ - public void setSaveOnCreate(boolean saveOnCreate) + @Override + public void setSaveOnInactiveEvict(boolean saveOnInactiveEvict) { - _saveOnCreate = saveOnCreate; + if (LOG.isDebugEnabled()) + LOG.debug("Ignoring eviction policy setting for NullSessionCaches"); } - /** - * @return the removeUnloadableSessions - */ - public boolean isRemoveUnloadableSessions() - { - return _removeUnloadableSessions; - } - - /** - * @param removeUnloadableSessions the removeUnloadableSessions to set - */ - public void setRemoveUnloadableSessions(boolean removeUnloadableSessions) - { - _removeUnloadableSessions = removeUnloadableSessions; - } - - /** - * @see org.eclipse.jetty.server.session.SessionCacheFactory#getSessionCache(org.eclipse.jetty.server.session.SessionHandler) - */ @Override public SessionCache getSessionCache(SessionHandler handler) { NullSessionCache cache = new NullSessionCache(handler); cache.setSaveOnCreate(isSaveOnCreate()); cache.setRemoveUnloadableSessions(isRemoveUnloadableSessions()); - cache.setWriteThroughMode(_writeThroughMode); + cache.setFlushOnResponseCommit(isFlushOnResponseCommit()); return cache; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java index 76d146b2ea3..9d89cefa49c 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/Session.java @@ -379,16 +379,31 @@ public class Session implements SessionHandler.SessionIf */ public void didActivate() { - HttpSessionEvent event = new HttpSessionEvent(this); - for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) + //A passivate listener might remove a non-serializable attribute that + //the activate listener might put back in again, which would spuriously + //set the dirty bit to true, causing another round of passivate/activate + //when the request exits. The store clears the dirty bit if it does a + //save, so ensure dirty flag is set to the value determined by the store, + //not a passivation listener. + boolean dirty = getSessionData().isDirty(); + + try { - Object value = _sessionData.getAttribute(iter.next()); - if (value instanceof HttpSessionActivationListener) + HttpSessionEvent event = new HttpSessionEvent(this); + for (Iterator iter = _sessionData.getKeys().iterator(); iter.hasNext();) { - HttpSessionActivationListener listener = (HttpSessionActivationListener)value; - listener.sessionDidActivate(event); + Object value = _sessionData.getAttribute(iter.next()); + if (value instanceof HttpSessionActivationListener) + { + HttpSessionActivationListener listener = (HttpSessionActivationListener)value; + listener.sessionDidActivate(event); + } } } + finally + { + getSessionData().setDirty(dirty); + } } /** @@ -512,6 +527,10 @@ public class Session implements SessionHandler.SessionIf { _sessionData.setMaxInactiveMs((long)secs * 1000L); _sessionData.calcAndSetExpiry(); + //dirty metadata writes can be skipped, but changing the + //maxinactiveinterval should write the session out because + //it may affect the session on other nodes, or on the same + //node in the case of the nullsessioncache _sessionData.setDirty(true); if (LOG.isDebugEnabled()) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java index eb2fabc35a6..1b16173d73e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java @@ -164,7 +164,17 @@ public interface SessionCache extends LifeCycle * @throws Exception if any error occurred */ void release(String id, Session session) throws Exception; - + + /** + * Called when a response is about to be committed. The + * cache can write the session to ensure that the + * SessionDataStore contains changes to the session + * that occurred during the lifetime of the request. This + * can help ensure that if a subsequent request goes to a + * different server, it will be able to see the session + * changes via the shared store. + */ + void commit(Session session) throws Exception; /** * Check to see if a Session is in the cache. Does NOT consult @@ -288,4 +298,18 @@ public interface SessionCache extends LifeCycle * @return if true unloadable session will be deleted */ boolean isRemoveUnloadableSessions(); + + /** + * If true, a dirty session will be written to the SessionDataStore + * just before a response is returned to the client. This ensures + * that subsequent requests to either the same node or a different + * node see the changed session data. + */ + void setFlushOnResponseCommit(boolean flushOnResponse); + + /** + * @return true if dirty sessions should be written + * before the response is committed. + */ + boolean isFlushOnResponseCommit(); } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java index cf6617f78f9..bd2cb95d4dd 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionData.java @@ -58,6 +58,7 @@ public class SessionData implements Serializable protected Map _attributes; protected boolean _dirty; protected long _lastSaved; //time in msec since last save + protected boolean _metaDataDirty; //non-attribute data has changed /** * Serialize the attribute map of the session. @@ -235,6 +236,22 @@ public class SessionData implements Serializable _dirty = dirty; } + /** + * @return the metaDataDirty + */ + public boolean isMetaDataDirty() + { + return _metaDataDirty; + } + + /** + * @param metaDataDirty true means non-attribute data has changed + */ + public void setMetaDataDirty(boolean metaDataDirty) + { + _metaDataDirty = metaDataDirty; + } + /** * @param name the name of the attribute * @return the value of the attribute named @@ -267,6 +284,15 @@ public class SessionData implements Serializable setDirty(true); } + /** + * Clear all dirty flags. + */ + public void clean() + { + setDirty(false); + setMetaDataDirty(false); + } + public void putAllAttributes(Map attributes) { _attributes.putAll(attributes); @@ -366,11 +392,13 @@ public class SessionData implements Serializable public void calcAndSetExpiry(long time) { setExpiry(calcExpiry(time)); + setMetaDataDirty(true); } public void calcAndSetExpiry() { setExpiry(calcExpiry()); + setMetaDataDirty(true); } public long getCreated() diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java index 7c958c465eb..9d79e8507bd 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionHandler.java @@ -350,10 +350,9 @@ public class SessionHandler extends ScopedHandler } /** - * Called by the {@link SessionHandler} when a session is last accessed by a request. + * Called when a request is finally leaving a session. * * @param session the session object - * @see #access(HttpSession, boolean) */ public void complete(HttpSession session) { @@ -373,6 +372,28 @@ public class SessionHandler extends ScopedHandler LOG.warn(e); } } + + /** + * Called when a response is about to be committed. + * We might take this opportunity to persist the session + * so that any subsequent requests to other servers + * will see the modifications. + */ + public void commit(HttpSession session) + { + if (session == null) + return; + + Session s = ((SessionIf)session).getSession(); + try + { + _sessionCache.commit(s); + } + catch (Exception e) + { + LOG.warn(e); + } + } @Deprecated public void complete(Session session, Request baseRequest) diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AbstractSessionCacheTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AbstractSessionCacheTest.java new file mode 100644 index 00000000000..c889b27bb85 --- /dev/null +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/AbstractSessionCacheTest.java @@ -0,0 +1,513 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 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 java.util.Collections; +import java.util.Set; +import java.util.concurrent.TimeUnit; + +import javax.servlet.http.HttpSessionActivationListener; +import javax.servlet.http.HttpSessionEvent; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Base class for all tests on all flavours of SessionCache + * + */ +public abstract class AbstractSessionCacheTest +{ + + public static class TestSessionActivationListener implements HttpSessionActivationListener + { + public int passivateCalls = 0; + public int activateCalls = 0; + + @Override + public void sessionWillPassivate(HttpSessionEvent se) + { + ++passivateCalls; + } + + @Override + public void sessionDidActivate(HttpSessionEvent se) + { + ++activateCalls; + } + } + + public abstract AbstractSessionCacheFactory newSessionCacheFactory(int evictionPolicy, boolean saveOnCreate, + boolean saveOnInactiveEvict, boolean removeUnloadableSessions, + boolean flushOnResponseCommit); + + /** + * Test that a new Session object can be created from + * previously persisted data (SessionData). + */ + @Test + public void testNewSessionFromPersistedData() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); + cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(true);//fake passivation + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + + context.start(); + + long now = System.currentTimeMillis(); + //fake persisted data + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + Session session = cache.newSession(data); + assertNotNull(session); + assertEquals("1234", session.getId()); + } + + + /** + * Test that the cache can load from the SessionDataStore + */ + @Test + public void testGetSessionNotInCache() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + AbstractSessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //put session data into the store + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + store.store("1234", data); + + assertFalse(cache.contains("1234")); + + Session session = cache.get("1234"); + assertEquals(1, session.getRequests()); + assertNotNull(session); + assertEquals("1234", session.getId()); + assertEquals(now - 20, session.getCreationTime()); + } + + @Test + public void testCommit() throws Exception + { + //Test state of session with call to commit + + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + //flushOnResponseCommit is true + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, true); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //Mimic various states of a session when a response is about + //to be committed: + + //call commit: session has not changed, should not be written + store._numSaves.set(0); //clear save counter + Session session = createUnExpiredSession(cache, store, "1234"); + cache.add("1234", session); + session.getSessionData().setLastSaved(100);//simulate previously saved + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + + //call commit: session has changed, should be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "456"); + cache.add("456", session); + session.getSessionData().setLastSaved(100);//simulate previously saved + session.setAttribute("foo", "bar"); + commitAndCheckSaveState(cache, store, session, true, true, false, false, 0, 1); + + //call commit: only the metadata has changed will not be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "678"); + cache.add("678", session); + session.getSessionData().setLastSaved(100);//simulate previously saved + session.getSessionData().calcAndSetExpiry(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1)); + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + + //Test again with a savePeriod set - as savePeriod only + //affects saving when the session is not dirty, the savePeriod + //should not affect whether or not the session is saved on call + //to commit + store.setSavePeriodSec(60); + + //call commit: session has not changed, should not be written anyway + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "890"); + cache.add("890", session); + session.getSessionData().setLastSaved(100);//simulate previously saved + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + + //call commit: session has changed so session must be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "012"); + cache.add("012", session); + session.getSessionData().setLastSaved(100);//simulate previously saved + session.setAttribute("foo", "bar"); + commitAndCheckSaveState(cache, store, session, true, true, false, false, 0, 1); + + //call commit: only the metadata has changed will not be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "234"); + session.getSessionData().setMetaDataDirty(true); + cache.add("234", session); + session.getSessionData().setLastSaved(100);//simulate previously saved + session.getSessionData().calcAndSetExpiry(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1)); + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + } + + @Test + public void testCommitAndRelease() throws Exception + { + //test what happens with various states of a session when commit + //is called before release + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + //flushOnResponseCommit is true + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, true); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //Mimic various states of a session when a response is about + //to be committed: + + //call commit: session has not changed, should not be written + Session session = createUnExpiredSession(cache, store, "1234"); + cache.add("1234", session); + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + //call release: session has not changed, but metadata has, should be written + cache.release("1234", session); + assertEquals(1, store._numSaves.get()); + assertFalse(session.getSessionData().isDirty()); + assertFalse(session.getSessionData().isMetaDataDirty()); + + //call commit: session has changed, should be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "456"); + cache.add("456", session); + session.setAttribute("foo", "bar"); + session.getSessionData().setLastSaved(100);//simulate not "new" session, ie has been previously saved + commitAndCheckSaveState(cache, store, session, true, true, false, false, 0, 1); + //call release: session not dirty but release changes metadata, so it will be saved + cache.release("456", session); + assertEquals(2, store._numSaves.get()); + assertFalse(session.getSessionData().isDirty()); + assertFalse(session.getSessionData().isMetaDataDirty()); + + //call commit: only the metadata has changed will not be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "678"); + session.getSessionData().calcAndSetExpiry(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1)); + session.getSessionData().setLastSaved(100); //simulate session not being "new", ie never previously saved + cache.add("678", session); + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + //call release: the metadata is dirty session should be written + cache.release("678", session); + assertEquals(1, store._numSaves.get()); + assertFalse(session.getSessionData().isDirty()); + assertFalse(session.getSessionData().isMetaDataDirty()); + + //Test again with a savePeriod set - only save if time last saved exceeds 60sec + store.setSavePeriodSec(60); + + //call commit: session has not changed, should not be written anyway + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "890"); + cache.add("890", session); + session.getSessionData().setLastSaved(100); //simulate last save long time ago + session.getSessionData().setMetaDataDirty(false); + commitAndCheckSaveState(cache, store, session, false, false, false, false, 0, 0); + //call release: not dirty but release sets metadata true, plus save period exceeded so write + cache.release("1234", session); + assertEquals(1, store._numSaves.get()); + assertFalse(session.getSessionData().isDirty()); + assertFalse(session.getSessionData().isMetaDataDirty()); + + //call commit: session has changed so session must be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "012"); + cache.add("012", session); + session.getSessionData().setLastSaved(100);//simulate previously saved session + session.setAttribute("foo", "bar"); + session.getSessionData().setMetaDataDirty(false); + commitAndCheckSaveState(cache, store, session, true, false, false, false, 0, 1); + //call release: not dirty, release sets metadirty true (recalc expiry) but previous save too recent to exceed save period --> no write + cache.release("012", session); + assertEquals(1, store._numSaves.get()); + assertFalse(session.getSessionData().isDirty()); + assertTrue(session.getSessionData().isMetaDataDirty()); + + //call commit: only the metadata has changed will not be written + store._numSaves.set(0); //clear save counter + session = createUnExpiredSession(cache, store, "234"); + session.getSessionData().calcAndSetExpiry(System.currentTimeMillis() + TimeUnit.DAYS.toMillis(1)); + session.getSessionData().setLastSaved(System.currentTimeMillis());//simulate session last saved recently + commitAndCheckSaveState(cache, store, session, false, true, false, true, 0, 0); + //call release: not dirty, release sets metadirty true (recalc expiry) but not within saveperiod so skip write + cache.release("1234", session); + assertEquals(0, store._numSaves.get()); + assertFalse(session.getSessionData().isDirty()); + assertTrue(session.getSessionData().isMetaDataDirty()); + } + + /** + * Test the exist method. + */ + @Test + public void testExists() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = (SessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //test one that doesn't exist at all + assertFalse(cache.exists("1234")); + + //test one that only exists in the store + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + store.store("1234", data); + assertTrue(cache.exists("1234")); + + //test one that exists in the cache also + Session session = cache.newSession(data); + cache.add("1234", session); + assertTrue(cache.exists("1234")); + } + + /** + * Test the delete method. + */ + @Test + public void testDelete() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, true, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //test remove non-existent session + Session session = cache.delete("1234"); + assertNull(session); + + //test remove of existing session in store only + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + store.store("1234", data); + session = cache.delete("1234"); + assertNotNull(session); + assertFalse(store.exists("1234")); + assertFalse(cache.contains("1234")); + + //test remove of session in both store and cache + session = cache.newSession(null, "1234",now - 20, TimeUnit.MINUTES.toMillis(10));//saveOnCreate ensures write to store + cache.add("1234", session); + assertTrue(store.exists("1234")); + assertTrue(cache.contains("1234")); + session = cache.delete("1234"); + assertNotNull(session); + assertFalse(store.exists("1234")); + assertFalse(cache.contains("1234")); + } + + @Test + public void testExpiration() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //test no candidates, no data in store + Set result = cache.checkExpiration(Collections.emptySet()); + assertTrue(result.isEmpty()); + + //test candidates that are in the cache and NOT expired + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); + Session session = cache.newSession(data); + cache.add("1234", session); + cache.release("1234", session); + assertTrue(cache.exists("1234")); + result = cache.checkExpiration(Collections.singleton("1234")); + assertTrue(result.isEmpty()); + + //test candidates that are in the cache AND expired + data.setExpiry(1); + result = cache.checkExpiration(Collections.singleton("1234")); + assertEquals(1, result.size()); + assertEquals("1234", result.iterator().next()); + + //test candidates that are not in the cache + SessionData data2 = store.newSessionData("567", now - 50, now - 40, now - 30, TimeUnit.MINUTES.toMillis(10)); + data2.setExpiry(1); + store.store("567", data2); + + result = cache.checkExpiration(Collections.emptySet()); + assertThat(result, containsInAnyOrder("1234", "567")); + } + + @Test + public void testSaveOnCreateTrue() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, true, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + long now = System.currentTimeMillis(); + cache.newSession(null, "1234", now, TimeUnit.MINUTES.toMillis(10)); + assertTrue(store.exists("1234")); + } + + @Test + public void testSaveOnCreateFalse() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + long now = System.currentTimeMillis(); + cache.newSession(null, "1234", now, TimeUnit.MINUTES.toMillis(10)); + assertFalse(store.exists("1234")); + } + + public void commitAndCheckSaveState(SessionCache cache, TestSessionDataStore store, Session session, + boolean expectedBeforeDirty, boolean expectedBeforeMetaDirty, + boolean expectedAfterDirty, boolean expectedAfterMetaDirty, + int expectedBeforeNumSaves, int expectedAfterNumSaves) + throws Exception + { + assertEquals(expectedBeforeDirty, session.getSessionData().isDirty()); + assertEquals(expectedBeforeMetaDirty, session.getSessionData().isMetaDataDirty()); + assertEquals(expectedBeforeNumSaves, store._numSaves.get()); + cache.commit(session); + assertEquals(expectedAfterDirty, session.getSessionData().isDirty()); + assertEquals(expectedAfterMetaDirty, session.getSessionData().isMetaDataDirty()); + assertEquals(expectedAfterNumSaves, store._numSaves.get()); + } + + public Session createUnExpiredSession(SessionCache cache, SessionDataStore store, String id) + { + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData(id, now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); + return cache.newSession(data); + } +} diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DefaultSessionCacheTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DefaultSessionCacheTest.java index 21b9bc44b83..46a812a5976 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DefaultSessionCacheTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/DefaultSessionCacheTest.java @@ -18,25 +18,19 @@ package org.eclipse.jetty.server.session; -import java.util.Collections; import java.util.Random; -import java.util.Set; import java.util.concurrent.TimeUnit; + import javax.servlet.http.HttpSession; -import javax.servlet.http.HttpSessionActivationListener; -import javax.servlet.http.HttpSessionEvent; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletContextHandler; import org.junit.jupiter.api.Test; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -44,25 +38,21 @@ import static org.junit.jupiter.api.Assertions.fail; /** * DefaultSessionCacheTest */ -public class DefaultSessionCacheTest +public class DefaultSessionCacheTest extends AbstractSessionCacheTest { - public static class TestSessionActivationListener implements HttpSessionActivationListener + @Override + public AbstractSessionCacheFactory newSessionCacheFactory(int evictionPolicy, boolean saveOnCreate, + boolean saveOnInactiveEvict, boolean removeUnloadableSessions, + boolean flushOnResponseCommit) { - public int passivateCalls = 0; - public int activateCalls = 0; - - @Override - public void sessionWillPassivate(HttpSessionEvent se) - { - ++passivateCalls; - } - - @Override - public void sessionDidActivate(HttpSessionEvent se) - { - ++activateCalls; - } + DefaultSessionCacheFactory factory = new DefaultSessionCacheFactory(); + factory.setEvictionPolicy(evictionPolicy); + factory.setSaveOnCreate(saveOnCreate); + factory.setSaveOnInactiveEvict(saveOnInactiveEvict); + factory.setRemoveUnloadableSessions(removeUnloadableSessions); + factory.setFlushOnResponseCommit(flushOnResponseCommit); + return factory; } @Test @@ -72,9 +62,8 @@ public class DefaultSessionCacheTest int inactivePeriod = 20; int scavengePeriod = 3; - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setSaveOnCreate(true); //ensures that a session is persisted as soon as it is created - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + AbstractSessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory(); TestServer server = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory); ServletContextHandler contextHandler = server.addContext("/test"); @@ -203,8 +192,7 @@ public class DefaultSessionCacheTest context.setContextPath("/test"); context.setServer(server); - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + AbstractSessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); TestSessionDataStore store = new TestSessionDataStore(true);//fake passivation @@ -234,38 +222,6 @@ public class DefaultSessionCacheTest assertEquals(1, listener.activateCalls); } - /** - * Test that a new Session object can be created from - * previously persisted data (SessionData). - */ - @Test - public void testNewSessionFromPersistedData() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(true);//fake passivation - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - - context.start(); - - long now = System.currentTimeMillis(); - //fake persisted data - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - Session session = cache.newSession(data); - assertNotNull(session); - assertEquals("1234", session.getId()); - } - /** * Test that a session id can be renewed. */ @@ -318,8 +274,7 @@ public class DefaultSessionCacheTest context.setContextPath("/test"); context.setServer(server); - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + AbstractSessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); TestSessionDataStore store = new TestSessionDataStore(); @@ -338,42 +293,6 @@ public class DefaultSessionCacheTest assertTrue(((AbstractSessionCache)cache).contains("1234")); } - /** - * Test that the cache can load from the SessionDataStore - */ - @Test - public void testGetSessionNotInCache() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - //put session data into the store - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - store.store("1234", data); - - assertFalse(cache.contains("1234")); - - Session session = cache.get("1234"); - assertEquals(1, session.getRequests()); - assertNotNull(session); - assertEquals("1234", session.getId()); - assertEquals(now - 20, session.getCreationTime()); - } - @Test public void testAdd() throws Exception @@ -384,8 +303,7 @@ public class DefaultSessionCacheTest context.setContextPath("/test"); context.setServer(server); - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + AbstractSessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); TestSessionDataStore store = new TestSessionDataStore(); @@ -418,8 +336,7 @@ public class DefaultSessionCacheTest context.setContextPath("/test"); context.setServer(server); - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); TestSessionDataStore store = new TestSessionDataStore(); @@ -458,9 +375,8 @@ public class DefaultSessionCacheTest context.setContextPath("/test"); context.setServer(server); - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = (SessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); TestSessionDataStore store = new TestSessionDataStore(); cache.setSessionDataStore(store); @@ -478,139 +394,6 @@ public class DefaultSessionCacheTest assertTrue(cache.contains("1234")); } - /** - * Test the exist method. - */ - @Test - public void testExists() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - //test one that doesn't exist at all - assertFalse(cache.exists("1234")); - - //test one that only exists in the store - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - store.store("1234", data); - assertTrue(cache.exists("1234")); - - //test one that exists in the cache also - Session session = cache.newSession(data); - cache.add("1234", session); - assertTrue(cache.exists("1234")); - } - - /** - * Test the delete method. - */ - @Test - public void testDelete() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - cacheFactory.setSaveOnCreate(true); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - //test remove non-existent session - Session session = cache.delete("1234"); - assertNull(session); - - //test remove of existing session in store only - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - store.store("1234", data); - session = cache.delete("1234"); - assertNotNull(session); - assertFalse(store.exists("1234")); - assertFalse(cache.contains("1234")); - - //test remove of session in both store and cache - session = cache.newSession(null, "1234",now - 20, TimeUnit.MINUTES.toMillis(10));//saveOnCreate ensures write to store - cache.add("1234", session); - assertTrue(store.exists("1234")); - assertTrue(cache.contains("1234")); - session = cache.delete("1234"); - assertNotNull(session); - assertFalse(store.exists("1234")); - assertFalse(cache.contains("1234")); - } - - @Test - public void testExpiration() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - //test no candidates, no data in store - Set result = cache.checkExpiration(Collections.emptySet()); - assertTrue(result.isEmpty()); - - //test candidates that are in the cache and NOT expired - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); - Session session = cache.newSession(data); - cache.add("1234", session); - cache.release("1234", session); - assertTrue(cache.exists("1234")); - result = cache.checkExpiration(Collections.singleton("1234")); - assertTrue(result.isEmpty()); - - //test candidates that are in the cache AND expired - data.setExpiry(1); - result = cache.checkExpiration(Collections.singleton("1234")); - assertEquals(1, result.size()); - assertEquals("1234", result.iterator().next()); - - //test candidates that are not in the cache - SessionData data2 = store.newSessionData("567", now - 50, now - 40, now - 30, TimeUnit.MINUTES.toMillis(10)); - data2.setExpiry(1); - store.store("567", data2); - - result = cache.checkExpiration(Collections.emptySet()); - assertThat(result, containsInAnyOrder("1234", "567")); - } - @Test public void testCheckInactiveSession() throws Exception @@ -725,54 +508,4 @@ public class DefaultSessionCacheTest SessionData retrieved = store.load("1234"); assertEquals(accessed, retrieved.getAccessed()); //check that we persisted the session before we evicted } - - @Test - public void testSaveOnCreateTrue() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - cacheFactory.setSaveOnCreate(true); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - long now = System.currentTimeMillis(); - cache.newSession(null, "1234", now, TimeUnit.MINUTES.toMillis(10)); - assertTrue(store.exists("1234")); - } - - @Test - public void testSaveOnCreateFalse() - throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory(); - cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT); - cacheFactory.setSaveOnCreate(false); - DefaultSessionCache cache = (DefaultSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - long now = System.currentTimeMillis(); - cache.newSession(null, "1234", now, TimeUnit.MINUTES.toMillis(10)); - assertFalse(store.exists("1234")); - } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/NullSessionCacheTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/NullSessionCacheTest.java index 5e63b62101b..6dce6c5e0ef 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/NullSessionCacheTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/NullSessionCacheTest.java @@ -18,129 +18,37 @@ package org.eclipse.jetty.server.session; -import java.io.Serializable; import java.util.concurrent.TimeUnit; -import javax.servlet.http.HttpSessionActivationListener; -import javax.servlet.http.HttpSessionEvent; - import org.eclipse.jetty.server.Server; import org.eclipse.jetty.servlet.ServletContextHandler; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; /** * NullSessionCacheTest */ -public class NullSessionCacheTest -{ - public static class SerializableTestObject implements Serializable, HttpSessionActivationListener +public class NullSessionCacheTest extends AbstractSessionCacheTest +{ + @Override + public AbstractSessionCacheFactory newSessionCacheFactory(int evictionPolicy, boolean saveOnCreate, + boolean saveOnInactiveEvict, boolean removeUnloadableSessions, + boolean flushOnResponseCommit) { - int count; - static int passivates = 0; - static int activates = 0; - - public SerializableTestObject(int i) - { - count = i; - } - - @Override - public void sessionWillPassivate(HttpSessionEvent se) - { - //should never be called, as we are replaced with the - //non-serializable object and thus passivate will be called on that - ++passivates; - } - - @Override - public void sessionDidActivate(HttpSessionEvent se) - { - ++activates; - //remove myself, replace with something serializable - se.getSession().setAttribute("pv", new TestObject(count)); - } - } - - public static class TestObject implements HttpSessionActivationListener - { - int i; - static int passivates = 0; - static int activates = 0; - - public TestObject(int j) - { - i = j; - } - - @Override - public void sessionWillPassivate(HttpSessionEvent se) - { - ++passivates; - //remove myself, replace with something serializable - se.getSession().setAttribute("pv", new SerializableTestObject(i)); - } - - @Override - public void sessionDidActivate(HttpSessionEvent se) - { - //this should never be called because we replace ourselves during passivation, - //so it is the SerializableTestObject that is activated instead - ++activates; - } - } - - @Test - public void testWritesWithPassivation() throws Exception - { - //Test that a session that is in the process of being saved cannot cause - //another save via a passivation listener - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory(); - cacheFactory.setWriteThroughMode(NullSessionCache.WriteThroughMode.ALWAYS); - - NullSessionCache cache = (NullSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(true); //pretend to passivate - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - - context.start(); - - //make a session - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); - Session session = cache.newSession(null, data); //mimic a request making a session - cache.add("1234", session); - //at this point the session should not be saved to the store - assertEquals(0, store._numSaves.get()); - - //set an attribute that is not serializable, should cause a save - TestObject obj = new TestObject(1); - session.setAttribute("pv", obj); - assertTrue(cache._listener._sessionsBeingWritten.isEmpty()); - assertTrue(store.exists("1234")); - assertEquals(1, store._numSaves.get()); - assertEquals(1, TestObject.passivates); - assertEquals(0, TestObject.activates); - assertEquals(1, SerializableTestObject.activates); - assertEquals(0, SerializableTestObject.passivates); + NullSessionCacheFactory factory = new NullSessionCacheFactory(); + factory.setSaveOnCreate(saveOnCreate); + factory.setRemoveUnloadableSessions(removeUnloadableSessions); + factory.setFlushOnResponseCommit(flushOnResponseCommit); + return factory; } @Test - public void testChangeWriteThroughMode() throws Exception + public void testShutdownWithSessionStore() + throws Exception { Server server = new Server(); @@ -148,157 +56,45 @@ public class NullSessionCacheTest context.setContextPath("/test"); context.setServer(server); - NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory(); + AbstractSessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); - NullSessionCache cache = (NullSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); + TestSessionDataStore store = new TestSessionDataStore(true);//fake passivation cache.setSessionDataStore(store); context.getSessionHandler().setSessionCache(cache); - - assertEquals(NullSessionCache.WriteThroughMode.ON_EXIT, cache.getWriteThroughMode()); - assertNull(cache._listener); - - //change mode to NEW - cache.setWriteThroughMode(NullSessionCache.WriteThroughMode.NEW); - assertEquals(NullSessionCache.WriteThroughMode.NEW, cache.getWriteThroughMode()); - assertNotNull(cache._listener); - assertEquals(1, context.getSessionHandler()._sessionAttributeListeners.size()); - assertTrue(context.getSessionHandler()._sessionAttributeListeners.contains(cache._listener)); - - //change mode to ALWAYS from NEW, listener should remain - NullSessionCache.WriteThroughAttributeListener old = cache._listener; - cache.setWriteThroughMode(NullSessionCache.WriteThroughMode.ALWAYS); - assertEquals(NullSessionCache.WriteThroughMode.ALWAYS, cache.getWriteThroughMode()); - assertNotNull(cache._listener); - assertSame(old,cache._listener); - assertEquals(1, context.getSessionHandler()._sessionAttributeListeners.size()); - - //check null is same as ON_EXIT - cache.setWriteThroughMode(null); - assertEquals(NullSessionCache.WriteThroughMode.ON_EXIT, cache.getWriteThroughMode()); - assertNull(cache._listener); - assertEquals(0, context.getSessionHandler()._sessionAttributeListeners.size()); - - //change to ON_EXIT - cache.setWriteThroughMode(NullSessionCache.WriteThroughMode.ON_EXIT); - assertEquals(NullSessionCache.WriteThroughMode.ON_EXIT, cache.getWriteThroughMode()); - assertNull(cache._listener); - assertEquals(0, context.getSessionHandler()._sessionAttributeListeners.size()); - } - - @Test - public void testWriteThroughAlways() throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory(); - cacheFactory.setWriteThroughMode(NullSessionCache.WriteThroughMode.ALWAYS); - - NullSessionCache cache = (NullSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); context.start(); - //make a session + //put a session in the cache and store long now = System.currentTimeMillis(); SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); - Session session = cache.newSession(null, data); //mimic a request making a session + Session session = cache.newSession(data); + TestSessionActivationListener listener = new TestSessionActivationListener(); cache.add("1234", session); - //at this point the session should not be saved to the store - assertEquals(0, store._numSaves.get()); - - //check each call to set attribute results in a store - session.setAttribute("colour", "blue"); - assertTrue(store.exists("1234")); - assertEquals(1, store._numSaves.get()); - - //mimic releasing the session after the request is finished - cache.release("1234", session); - assertTrue(store.exists("1234")); + //cache never contains the session assertFalse(cache.contains("1234")); - assertEquals(2, store._numSaves.get()); - - //simulate a new request using the previously created session - //the session should not now be new - session = cache.get("1234"); //get the session again - session.access(now); //simulate a request - session.setAttribute("spin", "left"); - assertTrue(store.exists("1234")); - assertEquals(3, store._numSaves.get()); - cache.release("1234", session); //finish with the session + session.setAttribute("aaa", listener); + //write session out on release + cache.release("1234", session); + assertEquals(1, store._numSaves.get()); + assertEquals(1, listener.passivateCalls); + assertEquals(0, listener.activateCalls); //NullSessionCache always evicts on release, so never reactivates - assertFalse(session.isResident()); + assertTrue(store.exists("1234")); + //cache never contains session + assertFalse(cache.contains("1234")); + + context.stop(); //calls shutdown + + //session should still exist in store + assertTrue(store.exists("1234")); + //cache never contains the session + assertFalse(cache.contains("1234")); + //shutdown does not save session + assertEquals(1, listener.passivateCalls); + assertEquals(0, listener.activateCalls); } - @Test - public void testWriteThroughNew() throws Exception - { - Server server = new Server(); - - ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); - context.setContextPath("/test"); - context.setServer(server); - - NullSessionCacheFactory cacheFactory = new NullSessionCacheFactory(); - cacheFactory.setWriteThroughMode(NullSessionCache.WriteThroughMode.NEW); - - NullSessionCache cache = (NullSessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); - - TestSessionDataStore store = new TestSessionDataStore(); - cache.setSessionDataStore(store); - context.getSessionHandler().setSessionCache(cache); - context.start(); - - //make a session - long now = System.currentTimeMillis(); - SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); - data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); - Session session = cache.newSession(null, data); //mimic a request making a session - cache.add("1234", session); - //at this point the session should not be saved to the store - assertEquals(0, store._numSaves.get()); - assertTrue(session.isNew()); - - //check each call to set attribute results in a store while the session is new - session.setAttribute("colour", "blue"); - assertTrue(store.exists("1234")); - assertEquals(1, store._numSaves.get()); - session.setAttribute("charge", "positive"); - assertEquals(2, store._numSaves.get()); - - //mimic releasing the session after the request is finished - cache.release("1234", session); - assertTrue(store.exists("1234")); - assertFalse(cache.contains("1234")); - assertEquals(3, store._numSaves.get()); //even if the session isn't dirty, we will save the access time - - - //simulate a new request using the previously created session - //the session should not now be new, so setAttribute should - //not result in a save - session = cache.get("1234"); //get the session again - session.access(now); //simulate a request - assertFalse(session.isNew()); - assertEquals(3, store._numSaves.get()); - session.setAttribute("spin", "left"); - assertTrue(store.exists("1234")); - assertEquals(3, store._numSaves.get()); - session.setAttribute("flavor", "charm"); - assertEquals(3, store._numSaves.get()); - cache.release("1234", session); //finish with the session - assertEquals(4, store._numSaves.get());//release session should write it out - assertFalse(session.isResident()); - } - @Test public void testNotCached() throws Exception { @@ -324,7 +120,7 @@ public class NullSessionCacheTest data.setExpiry(now + TimeUnit.DAYS.toMillis(1)); Session session = cache.newSession(null, data); //mimic a request making a session cache.add("1234", session); - assertFalse(cache.contains("1234"));//null cache doesn't actually store the session + assertFalse(cache.contains("1234"));//null cache doesn't actually retain the session //mimic releasing the session after the request is finished cache.release("1234", session); @@ -336,7 +132,104 @@ public class NullSessionCacheTest session.access(now); //simulate a request cache.release("1234", session); //finish with the session assertFalse(cache.contains("1234")); - assertFalse(session.isResident()); } + + /** + * Test contains method. + */ + @Test + public void testContains() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = (SessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //test one that doesn't exist + assertFalse(cache.contains("1234")); + + //test one that exists + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + Session session = cache.newSession(data); + cache.add("1234", session); + assertFalse(cache.contains("1234")); + } + + /** + * Test the exist method. + */ + @Test + public void testExists() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, false, false, false, false); + SessionCache cache = (SessionCache)cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //test one that doesn't exist anywhere at all + assertFalse(cache.exists("1234")); + + //test one that only exists in the store + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + store.store("1234", data); + assertTrue(cache.exists("1234")); + } + + /** + * Test the delete method. + */ + @Test + public void testDelete() + throws Exception + { + Server server = new Server(); + + ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS); + context.setContextPath("/test"); + context.setServer(server); + + SessionCacheFactory cacheFactory = newSessionCacheFactory(SessionCache.NEVER_EVICT, true, false, false, false); + SessionCache cache = cacheFactory.getSessionCache(context.getSessionHandler()); + + TestSessionDataStore store = new TestSessionDataStore(); + cache.setSessionDataStore(store); + context.getSessionHandler().setSessionCache(cache); + context.start(); + + //test remove non-existent session + Session session = cache.delete("1234"); + assertNull(session); + + //test remove of existing session in store only + long now = System.currentTimeMillis(); + SessionData data = store.newSessionData("1234", now - 20, now - 10, now - 20, TimeUnit.MINUTES.toMillis(10)); + store.store("1234", data); + session = cache.delete("1234"); + assertNull(session); //NullSessionCache never returns the session that was removed from the cache because it was never in the cache! + assertFalse(store.exists("1234")); + assertFalse(cache.contains("1234")); + } } diff --git a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java index 9a7a0d90801..245cb63c94c 100644 --- a/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java +++ b/tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionEvictionFailureTest.java @@ -37,6 +37,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; /** * SessionEvictionFailureTest @@ -192,6 +193,7 @@ public class SessionEvictionFailureTest // Make another request to see if the session is still in the cache and can be used, //allow it to be saved this time + assertTrue(context.getSessionHandler().getSessionCache().contains(TestServer.extractSessionId(sessionCookie))); Request request = client.newRequest(url + "?action=test"); response = request.send(); assertEquals(HttpServletResponse.SC_OK, response.getStatus());