@@ -, +, @@ smaller object. It helps to serialize this sessions with libraries like kryo. --- .../valves/CrawlerSessionManagerValve.java | 23 +++++++--- .../TestCrawlerSessionManagerValve.java | 46 ++++++++++++++++++- 2 files changed, 61 insertions(+), 8 deletions(-) --- a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java +++ a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java @@ -42,7 +42,7 @@ * users - regardless of whether or not they provide a session token with their * requests. */ -public class CrawlerSessionManagerValve extends ValveBase implements HttpSessionBindingListener { +public class CrawlerSessionManagerValve extends ValveBase { private static final Log log = LogFactory.getLog(CrawlerSessionManagerValve.class); @@ -241,7 +241,7 @@ public void invoke(Request request, Response response) throws IOException, Servl clientIdSessionId.put(clientIdentifier, s.getId()); sessionIdClientId.put(s.getId(), clientIdentifier); // #valueUnbound() will be called on session expiration - s.setAttribute(this.getClass().getName(), this); + s.setAttribute(this.getClass().getName(), new CrawlerHttpSessionBindingListener(clientIdSessionId, sessionIdClientId)); s.setMaxInactiveInterval(sessionInactiveInterval); if (log.isDebugEnabled()) { @@ -269,12 +269,21 @@ private String getClientIdentifier(Host host, Context context, String clientIp) return result.toString(); } + private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener { + private final Map clientIdSessionId; + private final Map sessionIdClientId; - @Override - public void valueUnbound(HttpSessionBindingEvent event) { - String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); - if (clientIdentifier != null) { - clientIdSessionId.remove(clientIdentifier); + public CrawlerHttpSessionBindingListener(Map clientIdSessionId, Map sessionIdClientId) { + this.clientIdSessionId = clientIdSessionId; + this.sessionIdClientId = sessionIdClientId; + } + + @Override + public void valueUnbound(HttpSessionBindingEvent event) { + String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); + if (clientIdentifier != null) { + clientIdSessionId.remove(clientIdentifier); + } } } } --- a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java +++ a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java @@ -22,7 +22,13 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpSession; +import javax.servlet.http.HttpSessionBindingListener; +import org.apache.catalina.Manager; +import org.apache.catalina.core.StandardContext; +import org.apache.catalina.session.StandardManager; +import org.apache.catalina.session.StandardSession; +import org.hamcrest.CoreMatchers; import org.junit.Test; import org.apache.catalina.Context; @@ -33,8 +39,20 @@ import org.easymock.EasyMock; import org.easymock.IExpectationSetters; +import static org.hamcrest.CoreMatchers.*; +import static org.hamcrest.MatcherAssert.assertThat; + public class TestCrawlerSessionManagerValve { + private static final Manager TEST_MANAGER; + + static { + TEST_MANAGER = new StandardManager(); + TEST_MANAGER.setContext(new StandardContext()); + } + + + @Test public void testCrawlerIpsPositive() throws Exception { CrawlerSessionManagerValve valve = new CrawlerSessionManagerValve(); @@ -79,6 +97,32 @@ public void testCrawlerMultipleHostsHostAware() throws Exception { verifyCrawlingLocalhost(valve, "example.invalid"); } + @Test + public void testCrawlersSessionIdIsRemovedAfterSessionExpiry() throws IOException, ServletException { + CrawlerSessionManagerValve valve = new CrawlerSessionManagerValve(); + valve.setCrawlerIps("216\\.58\\.206\\.174"); + valve.setCrawlerUserAgents(valve.getCrawlerUserAgents()); + valve.setNext(EasyMock.createMock(Valve.class)); + valve.setSessionInactiveInterval(0); + StandardSession session = new StandardSession(TEST_MANAGER); + session.setId("id"); + session.setValid(true); + + Request request = createRequestExpectations("216.58.206.174", session, true); + + EasyMock.replay(request); + + valve.invoke(request, EasyMock.createMock(Response.class)); + + EasyMock.verify(request); + + assertThat(valve.getClientIpSessionId().values(), hasItem("id")); + + session.expire(); + + assertThat(valve.getClientIpSessionId().values().size(), is(0)); + } + private void verifyCrawlingLocalhost(CrawlerSessionManagerValve valve, String hostname) throws IOException, ServletException { @@ -97,7 +141,7 @@ private HttpSession createSessionExpectations(CrawlerSessionManagerValve valve, HttpSession session = EasyMock.createMock(HttpSession.class); if (isBot) { EasyMock.expect(session.getId()).andReturn("id").times(2); - session.setAttribute(valve.getClass().getName(), valve); + session.setAttribute(EasyMock.eq(valve.getClass().getName()), EasyMock.anyObject(HttpSessionBindingListener.class)); EasyMock.expectLastCall(); session.setMaxInactiveInterval(60); EasyMock.expectLastCall(); smaller object. It helps to serialize this sessions with libraries like kryo. --- .../valves/CrawlerSessionManagerValve.java | 18 +++++++++--------- .../valves/TestCrawlerSessionManagerValve.java | 1 - 2 files changed, 9 insertions(+), 10 deletions(-) --- a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java +++ a/java/org/apache/catalina/valves/CrawlerSessionManagerValve.java @@ -17,6 +17,7 @@ package org.apache.catalina.valves; import java.io.IOException; +import java.io.Serializable; import java.util.Enumeration; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -241,7 +242,7 @@ public void invoke(Request request, Response response) throws IOException, Servl clientIdSessionId.put(clientIdentifier, s.getId()); sessionIdClientId.put(s.getId(), clientIdentifier); // #valueUnbound() will be called on session expiration - s.setAttribute(this.getClass().getName(), new CrawlerHttpSessionBindingListener(clientIdSessionId, sessionIdClientId)); + s.setAttribute(this.getClass().getName(), new CrawlerHttpSessionBindingListener(clientIdSessionId, clientIdentifier)); s.setMaxInactiveInterval(sessionInactiveInterval); if (log.isDebugEnabled()) { @@ -269,20 +270,19 @@ private String getClientIdentifier(Host host, Context context, String clientIp) return result.toString(); } - private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener { - private final Map clientIdSessionId; - private final Map sessionIdClientId; + private static class CrawlerHttpSessionBindingListener implements HttpSessionBindingListener, Serializable { + private final transient Map clientIdSessionId; + private final transient String clientIdentifier; - public CrawlerHttpSessionBindingListener(Map clientIdSessionId, Map sessionIdClientId) { + private CrawlerHttpSessionBindingListener(Map clientIdSessionId, String clientIdentifier) { this.clientIdSessionId = clientIdSessionId; - this.sessionIdClientId = sessionIdClientId; + this.clientIdentifier = clientIdentifier; } @Override public void valueUnbound(HttpSessionBindingEvent event) { - String clientIdentifier = sessionIdClientId.remove(event.getSession().getId()); - if (clientIdentifier != null) { - clientIdSessionId.remove(clientIdentifier); + if (clientIdentifier != null && clientIdSessionId != null) { + clientIdSessionId.remove(clientIdentifier, event.getSession().getId()); } } } --- a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java +++ a/test/org/apache/catalina/valves/TestCrawlerSessionManagerValve.java @@ -28,7 +28,6 @@ import org.apache.catalina.core.StandardContext; import org.apache.catalina.session.StandardManager; import org.apache.catalina.session.StandardSession; -import org.hamcrest.CoreMatchers; import org.junit.Test; import org.apache.catalina.Context;