From 62d968559db66c9b0ed24c5d18e7509b57cd5842 Mon Sep 17 00:00:00 2001 From: Nikita Koksharov Date: Fri, 18 Sep 2020 08:21:49 +0300 Subject: [PATCH] Fixed - Incorrect session attributes being returned in UpdateMode=AFTER_REQUEST and ReadMode=REDIS. #3040 --- .../org/redisson/tomcat/RedissonSession.java | 37 ++++-- .../tomcat/RedissonSessionManager.java | 75 ++++++----- .../java/org/redisson/tomcat/UpdateValve.java | 13 +- .../java/org/redisson/tomcat/UsageValve.java | 24 +++- .../tomcat/RedissonSessionManagerTest.java | 122 ++++++++++++----- .../org/redisson/tomcat/RedissonSession.java | 37 ++++-- .../tomcat/RedissonSessionManager.java | 68 +++++----- .../java/org/redisson/tomcat/UpdateValve.java | 13 +- .../java/org/redisson/tomcat/UsageValve.java | 24 +++- .../tomcat/RedissonSessionManagerTest.java | 122 ++++++++++++----- .../org/redisson/tomcat/RedissonSession.java | 37 ++++-- .../tomcat/RedissonSessionManager.java | 68 +++++----- .../java/org/redisson/tomcat/UpdateValve.java | 15 ++- .../java/org/redisson/tomcat/UsageValve.java | 24 +++- .../tomcat/RedissonSessionManagerTest.java | 125 ++++++++++++------ 15 files changed, 539 insertions(+), 265 deletions(-) diff --git a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSession.java b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSession.java index 35c1c4e89..8b9a31e2a 100644 --- a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSession.java +++ b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSession.java @@ -67,6 +67,7 @@ public class RedissonSession extends StandardSession { private final AtomicInteger usages = new AtomicInteger(); private Map loadedAttributes = Collections.emptyMap(); + private Map updatedAttributes = Collections.emptyMap(); private Set removedAttributes = Collections.emptySet(); private final boolean broadcastSessionEvents; @@ -84,6 +85,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes = new ConcurrentHashMap<>(); + updatedAttributes = new ConcurrentHashMap<>(); } try { @@ -182,6 +184,7 @@ public class RedissonSession extends StandardSession { } map = null; loadedAttributes.clear(); + updatedAttributes.clear(); } @Override @@ -324,6 +327,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes.put(name, value); + updatedAttributes.put(name, value); } if (updateMode == UpdateMode.AFTER_REQUEST) { removedAttributes.remove(name); @@ -346,6 +350,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes.remove(name); + updatedAttributes.remove(name); } if (updateMode == UpdateMode.AFTER_REQUEST) { removedAttributes.add(name); @@ -373,15 +378,20 @@ public class RedissonSession extends StandardSession { if (broadcastSessionEvents) { newMap.put(IS_EXPIRATION_LOCKED, isExpirationLocked); } - - if (attrs != null) { - for (Entry entry : attrs.entrySet()) { - newMap.put(entry.getKey(), entry.getValue()); + + if (readMode == ReadMode.MEMORY) { + if (attrs != null) { + for (Entry entry : attrs.entrySet()) { + newMap.put(entry.getKey(), entry.getValue()); + } } + } else { + newMap.putAll(updatedAttributes); + updatedAttributes.clear(); } - + map.putAll(newMap); - map.fastRemove(removedAttributes.toArray(new String[removedAttributes.size()])); + map.fastRemove(removedAttributes.toArray(new String[0])); if (readMode == ReadMode.MEMORY) { topic.publish(createPutAllMessage(newMap)); @@ -394,8 +404,7 @@ public class RedissonSession extends StandardSession { } removedAttributes.clear(); - loadedAttributes.clear(); - + expireSession(); } @@ -437,8 +446,10 @@ public class RedissonSession extends StandardSession { this.authType = authType; } - for (Entry entry : attrs.entrySet()) { - super.setAttribute(entry.getKey(), entry.getValue(), false); + if (readMode == ReadMode.MEMORY) { + for (Entry entry : attrs.entrySet()) { + super.setAttribute(entry.getKey(), entry.getValue(), false); + } } } @@ -447,6 +458,8 @@ public class RedissonSession extends StandardSession { super.recycle(); map = null; loadedAttributes.clear(); + updatedAttributes.clear(); + removedAttributes.clear(); } public void startUsage() { @@ -454,7 +467,9 @@ public class RedissonSession extends StandardSession { } public void endUsage() { - if (usages.decrementAndGet() == 0) { + // don't decrement usages if startUsage wasn't called +// if (usages.decrementAndGet() == 0) { + if (usages.get() == 0 || usages.decrementAndGet() == 0) { loadedAttributes.clear(); } } diff --git a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSessionManager.java b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSessionManager.java index 3411bf660..4902f6f7a 100644 --- a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSessionManager.java +++ b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/RedissonSessionManager.java @@ -15,21 +15,13 @@ */ package org.redisson.tomcat; -import java.io.File; -import java.io.IOException; -import java.util.*; -import java.util.concurrent.ConcurrentHashMap; - -import javax.servlet.http.HttpSession; - import org.apache.catalina.*; import org.apache.catalina.session.ManagerBase; -import org.apache.catalina.valves.ValveBase; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.redisson.Redisson; -import org.redisson.api.RSet; import org.redisson.api.RMap; +import org.redisson.api.RSet; import org.redisson.api.RTopic; import org.redisson.api.RedissonClient; import org.redisson.api.listener.MessageListener; @@ -38,6 +30,11 @@ import org.redisson.client.codec.StringCodec; import org.redisson.codec.CompositeCodec; import org.redisson.config.Config; +import javax.servlet.http.HttpSession; +import java.io.File; +import java.io.IOException; +import java.util.*; + /** * Redisson Session Manager for Apache Tomcat * @@ -62,10 +59,6 @@ public class RedissonSessionManager extends ManagerBase { private final String nodeId = UUID.randomUUID().toString(); - private static ValveBase updateValve; - - private static Set contextInUse = Collections.newSetFromMap(new ConcurrentHashMap()); - private MessageListener messageListener; private Codec codecToUse; @@ -204,12 +197,6 @@ public class RedissonSessionManager extends ManagerBase { return new RedissonSession(this, readMode, updateMode, broadcastSessionEvents); } - @Override - public void add(Session session) { - super.add(session); - ((RedissonSession)session).save(); - } - @Override public void remove(Session session, boolean update) { super.remove(session, update); @@ -218,7 +205,13 @@ public class RedissonSessionManager extends ManagerBase { ((RedissonSession)session).delete(); } } - + + @Override + public void add(Session session) { + super.add(session); + ((RedissonSession)session).save(); + } + public RedissonClient getRedisson() { return redisson; } @@ -248,16 +241,20 @@ public class RedissonSessionManager extends ManagerBase { Pipeline pipeline = getEngine().getPipeline(); synchronized (pipeline) { - contextInUse.add(((Context) getContainer()).getName()); - if (updateMode == UpdateMode.AFTER_REQUEST) { - if (updateValve == null) { - updateValve = new UpdateValve(); - pipeline.addValve(updateValve); + if (readMode == ReadMode.REDIS) { + Optional res = Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UsageValve.class).findAny(); + if (res.isPresent()) { + ((UsageValve)res.get()).incUsage(); + } else { + pipeline.addValve(new UsageValve()); } - } else if (readMode == ReadMode.REDIS) { - if (updateValve == null) { - updateValve = new UsageValve(); - pipeline.addValve(updateValve); + } + if (updateMode == UpdateMode.AFTER_REQUEST) { + Optional res = Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UpdateValve.class).findAny(); + if (res.isPresent()) { + ((UpdateValve)res.get()).incUsage(); + } else { + pipeline.addValve(new UpdateValve()); } } } @@ -360,13 +357,19 @@ public class RedissonSessionManager extends ManagerBase { Pipeline pipeline = getEngine().getPipeline(); synchronized (pipeline) { - contextInUse.remove(((Context) getContainer()).getName()); - //remove valves when all of the RedissonSessionManagers (web apps) are not in use anymore - if (contextInUse.isEmpty()) { - if (updateValve != null) { - pipeline.removeValve(updateValve); - updateValve = null; - } + if (readMode == ReadMode.REDIS) { + Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UsageValve.class).forEach(v -> { + if (((UsageValve)v).decUsage() == 0){ + pipeline.removeValve(v); + } + }); + } + if (updateMode == UpdateMode.AFTER_REQUEST) { + Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UpdateValve.class).forEach(v -> { + if (((UpdateValve)v).decUsage() == 0){ + pipeline.removeValve(v); + } + }); } } diff --git a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UpdateValve.java b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UpdateValve.java index 13ef51b94..d80cbe217 100644 --- a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UpdateValve.java +++ b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UpdateValve.java @@ -16,6 +16,7 @@ package org.redisson.tomcat; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletException; @@ -33,11 +34,21 @@ import org.apache.catalina.valves.ValveBase; public class UpdateValve extends ValveBase { private static final String ALREADY_FILTERED_NOTE = UpdateValve.class.getName() + ".ALREADY_FILTERED_NOTE"; - + + private final AtomicInteger usage = new AtomicInteger(1); + public UpdateValve() { super(true); } + public void incUsage() { + usage.incrementAndGet(); + } + + public int decUsage() { + return usage.decrementAndGet(); + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { if (getNext() == null) { diff --git a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UsageValve.java b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UsageValve.java index 40f71f191..3be08d209 100644 --- a/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UsageValve.java +++ b/redisson-tomcat/redisson-tomcat-7/src/main/java/org/redisson/tomcat/UsageValve.java @@ -22,6 +22,7 @@ import org.apache.catalina.valves.ValveBase; import javax.servlet.ServletException; import javax.servlet.http.HttpSession; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; /** * Redisson Valve object for Apache Tomcat @@ -33,10 +34,20 @@ public class UsageValve extends ValveBase { private static final String ALREADY_FILTERED_NOTE = UsageValve.class.getName() + ".ALREADY_FILTERED_NOTE"; + private final AtomicInteger usage = new AtomicInteger(1); + public UsageValve() { super(true); } + public void incUsage() { + usage.incrementAndGet(); + } + + public int decUsage() { + return usage.decrementAndGet(); + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { if (getNext() == null) { @@ -46,12 +57,11 @@ public class UsageValve extends ValveBase { //check if we already filtered/processed this request if (request.getNote(ALREADY_FILTERED_NOTE) == null) { request.setNote(ALREADY_FILTERED_NOTE, Boolean.TRUE); - RedissonSession s = null; try { if (request.getContext() != null) { HttpSession session = request.getSession(false); if (session != null) { - s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); + RedissonSession s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); if (s != null) { s.startUsage(); } @@ -61,8 +71,14 @@ public class UsageValve extends ValveBase { getNext().invoke(request, response); } finally { request.removeNote(ALREADY_FILTERED_NOTE); - if (s != null) { - s.endUsage(); + if (request.getContext() != null) { + HttpSession session = request.getSession(false); + if (session != null) { + RedissonSession s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); + if (s != null) { + s.endUsage(); + } + } } } } else { diff --git a/redisson-tomcat/redisson-tomcat-7/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java b/redisson-tomcat/redisson-tomcat-7/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java index a40348cb0..1add753a9 100644 --- a/redisson-tomcat/redisson-tomcat-7/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java +++ b/redisson-tomcat/redisson-tomcat-7/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java @@ -1,6 +1,5 @@ package org.redisson.tomcat; -import org.apache.http.client.ClientProtocolException; import org.apache.http.client.fluent.Executor; import org.apache.http.client.fluent.Request; import org.apache.http.cookie.Cookie; @@ -19,6 +18,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Arrays; +import java.util.concurrent.TimeUnit; @RunWith(Parameterized.class) public class RedissonSessionManagerTest { @@ -43,6 +43,31 @@ public class RedissonSessionManagerTest { Files.copy(Paths.get(basePath + contextName), Paths.get(basePath + "context.xml")); } + @Test + public void testUpdateTwoServers_readValue() throws Exception { + TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + try { + server1.start(); + server2.start(); + + Executor executor = Executor.newInstance(); + BasicCookieStore cookieStore = new BasicCookieStore(); + executor.use(cookieStore); + + write(8080, executor, "test", "from_server1"); + write(8081, executor, "test", "from_server2"); + + read(8080, executor, "test", "from_server2"); + read(8080, executor, "test", "from_server2"); + + } finally { + Executor.closeIdleConnections(); + server1.stop(); + server2.stop(); + } + } + @Test public void testHttpSessionListener() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); @@ -58,7 +83,7 @@ public class RedissonSessionManagerTest { TestHttpSessionListener.CREATED_INVOCATION_COUNTER = 0; TestHttpSessionListener.DESTROYED_INVOCATION_COUNTER = 0; - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); TomcatServer server3 = new TomcatServer("myapp", 8082, "src/test/"); server3.start(); @@ -76,23 +101,50 @@ public class RedissonSessionManagerTest { server3.stop(); } + @Test + public void testUpdateTwoServers_twoValues() throws Exception { + TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + try { + server1.start(); + server2.start(); + + Executor executor = Executor.newInstance(); + BasicCookieStore cookieStore = new BasicCookieStore(); + executor.use(cookieStore); + + write(8080, executor, "key1", "value1"); + write(8080, executor, "key2", "value1"); + + write(8081, executor, "key1", "value2"); + write(8081, executor, "key2", "value2"); + + read(8080, executor, "key1", "value2"); + read(8080, executor, "key2", "value2"); + + } finally { + Executor.closeIdleConnections(); + server1.stop(); + server2.stop(); + } + } + @Test public void testUpdateTwoServers() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); server1.start(); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + server2.start(); Executor executor = Executor.newInstance(); BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - - write(executor, "test", "1234"); - TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); - server2.start(); + write(8080, executor, "test", "1234"); read(8081, executor, "test", "1234"); - read(executor, "test", "1234"); - write(executor, "test", "324"); + read(8080, executor, "test", "1234"); + write(8080, executor, "test", "324"); read(8081, executor, "test", "324"); Executor.closeIdleConnections(); @@ -110,7 +162,7 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); server2.start(); @@ -122,7 +174,7 @@ public class RedissonSessionManagerTest { Thread.sleep(40000); executor.use(cookieStore); - read(executor, "test", "1234"); + read(8080, executor, "test", "1234"); Executor.closeIdleConnections(); server1.stop(); @@ -139,7 +191,8 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); + System.out.println("done1"); Cookie cookie = cookieStore.getCookies().get(0); Executor.closeIdleConnections(); @@ -152,7 +205,7 @@ public class RedissonSessionManagerTest { cookieStore = new BasicCookieStore(); cookieStore.addCookie(cookie); executor.use(cookieStore); - read(executor, "test", "1234"); + read(8080, executor, "test", "1234"); remove(executor, "test", "null"); Executor.closeIdleConnections(); @@ -168,8 +221,8 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1234"); - read(executor, "test", "1234"); + write(8080, executor, "test", "1234"); + read(8080, executor, "test", "1234"); remove(executor, "test", "null"); Executor.closeIdleConnections(); @@ -184,9 +237,9 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1"); + write(8080, executor, "test", "1"); recreate(executor, "test", "2"); - read(executor, "test", "2"); + read(8080, executor, "test", "2"); Executor.closeIdleConnections(); server.stop(); @@ -200,10 +253,10 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1"); - read(executor, "test", "1"); - write(executor, "test", "2"); - read(executor, "test", "2"); + write(8080, executor, "test", "1"); + read(8080, executor, "test", "1"); + write(8080, executor, "test", "2"); + read(8080, executor, "test", "2"); Executor.closeIdleConnections(); server.stop(); @@ -225,7 +278,7 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); Cookie cookie = cookieStore.getCookies().get(0); invalidate(executor); @@ -235,49 +288,46 @@ public class RedissonSessionManagerTest { cookieStore = new BasicCookieStore(); cookieStore.addCookie(cookie); executor.use(cookieStore); - read(executor, "test", "null"); + read(8080, executor, "test", "null"); invalidate(executor); Executor.closeIdleConnections(); server.stop(); - + + TimeUnit.SECONDS.sleep(60); Assert.assertEquals(0, r.getKeys().count()); } - private void write(Executor executor, String key, String value) throws IOException, ClientProtocolException { - String url = "http://localhost:8080/myapp/write?key=" + key + "&value=" + value; + private void write(int port, Executor executor, String key, String value) throws IOException { + String url = "http://localhost:" + port + "/myapp/write?key=" + key + "&value=" + value; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } - - private void read(int port, Executor executor, String key, String value) throws IOException, ClientProtocolException { + + private void read(int port, Executor executor, String key, String value) throws IOException { String url = "http://localhost:" + port + "/myapp/read?key=" + key; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals(value, response); } - private void read(Executor executor, String key, String value) throws IOException, ClientProtocolException { - String url = "http://localhost:8080/myapp/read?key=" + key; - String response = executor.execute(Request.Get(url)).returnContent().asString(); - Assert.assertEquals(value, response); - } - - private void remove(Executor executor, String key, String value) throws IOException, ClientProtocolException { + private void remove(Executor executor, String key, String value) throws IOException { String url = "http://localhost:8080/myapp/remove?key=" + key; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals(value, response); } - private void invalidate(Executor executor) throws IOException, ClientProtocolException { + private void invalidate(Executor executor) throws IOException { String url = "http://localhost:8080/myapp/invalidate"; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } - private void recreate(Executor executor, String key, String value) throws IOException, ClientProtocolException { + private void recreate(Executor executor, String key, String value) throws IOException { String url = "http://localhost:8080/myapp/recreate?key=" + key + "&value=" + value; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } + + } diff --git a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSession.java b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSession.java index 35c1c4e89..8b9a31e2a 100644 --- a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSession.java +++ b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSession.java @@ -67,6 +67,7 @@ public class RedissonSession extends StandardSession { private final AtomicInteger usages = new AtomicInteger(); private Map loadedAttributes = Collections.emptyMap(); + private Map updatedAttributes = Collections.emptyMap(); private Set removedAttributes = Collections.emptySet(); private final boolean broadcastSessionEvents; @@ -84,6 +85,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes = new ConcurrentHashMap<>(); + updatedAttributes = new ConcurrentHashMap<>(); } try { @@ -182,6 +184,7 @@ public class RedissonSession extends StandardSession { } map = null; loadedAttributes.clear(); + updatedAttributes.clear(); } @Override @@ -324,6 +327,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes.put(name, value); + updatedAttributes.put(name, value); } if (updateMode == UpdateMode.AFTER_REQUEST) { removedAttributes.remove(name); @@ -346,6 +350,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes.remove(name); + updatedAttributes.remove(name); } if (updateMode == UpdateMode.AFTER_REQUEST) { removedAttributes.add(name); @@ -373,15 +378,20 @@ public class RedissonSession extends StandardSession { if (broadcastSessionEvents) { newMap.put(IS_EXPIRATION_LOCKED, isExpirationLocked); } - - if (attrs != null) { - for (Entry entry : attrs.entrySet()) { - newMap.put(entry.getKey(), entry.getValue()); + + if (readMode == ReadMode.MEMORY) { + if (attrs != null) { + for (Entry entry : attrs.entrySet()) { + newMap.put(entry.getKey(), entry.getValue()); + } } + } else { + newMap.putAll(updatedAttributes); + updatedAttributes.clear(); } - + map.putAll(newMap); - map.fastRemove(removedAttributes.toArray(new String[removedAttributes.size()])); + map.fastRemove(removedAttributes.toArray(new String[0])); if (readMode == ReadMode.MEMORY) { topic.publish(createPutAllMessage(newMap)); @@ -394,8 +404,7 @@ public class RedissonSession extends StandardSession { } removedAttributes.clear(); - loadedAttributes.clear(); - + expireSession(); } @@ -437,8 +446,10 @@ public class RedissonSession extends StandardSession { this.authType = authType; } - for (Entry entry : attrs.entrySet()) { - super.setAttribute(entry.getKey(), entry.getValue(), false); + if (readMode == ReadMode.MEMORY) { + for (Entry entry : attrs.entrySet()) { + super.setAttribute(entry.getKey(), entry.getValue(), false); + } } } @@ -447,6 +458,8 @@ public class RedissonSession extends StandardSession { super.recycle(); map = null; loadedAttributes.clear(); + updatedAttributes.clear(); + removedAttributes.clear(); } public void startUsage() { @@ -454,7 +467,9 @@ public class RedissonSession extends StandardSession { } public void endUsage() { - if (usages.decrementAndGet() == 0) { + // don't decrement usages if startUsage wasn't called +// if (usages.decrementAndGet() == 0) { + if (usages.get() == 0 || usages.decrementAndGet() == 0) { loadedAttributes.clear(); } } diff --git a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSessionManager.java b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSessionManager.java index 489da2016..0b12845da 100644 --- a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSessionManager.java +++ b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/RedissonSessionManager.java @@ -15,26 +15,13 @@ */ package org.redisson.tomcat; -import java.io.File; -import java.io.IOException; -import java.util.*; -import java.util.concurrent.ConcurrentHashMap; - -import javax.servlet.http.HttpSession; - -import org.apache.catalina.LifecycleException; -import org.apache.catalina.LifecycleState; -import org.apache.catalina.Pipeline; -import org.apache.catalina.Session; -import org.apache.catalina.SessionEvent; -import org.apache.catalina.SessionListener; +import org.apache.catalina.*; import org.apache.catalina.session.ManagerBase; -import org.apache.catalina.valves.ValveBase; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.redisson.Redisson; -import org.redisson.api.RSet; import org.redisson.api.RMap; +import org.redisson.api.RSet; import org.redisson.api.RTopic; import org.redisson.api.RedissonClient; import org.redisson.api.listener.MessageListener; @@ -43,6 +30,11 @@ import org.redisson.client.codec.StringCodec; import org.redisson.codec.CompositeCodec; import org.redisson.config.Config; +import javax.servlet.http.HttpSession; +import java.io.File; +import java.io.IOException; +import java.util.*; + /** * Redisson Session Manager for Apache Tomcat * @@ -67,10 +59,6 @@ public class RedissonSessionManager extends ManagerBase { private final String nodeId = UUID.randomUUID().toString(); - private static ValveBase updateValve; - - private static Set contextInUse = Collections.newSetFromMap(new ConcurrentHashMap()); - private MessageListener messageListener; private Codec codecToUse; @@ -253,16 +241,20 @@ public class RedissonSessionManager extends ManagerBase { Pipeline pipeline = getEngine().getPipeline(); synchronized (pipeline) { - contextInUse.add(getContext().getName()); - if (updateMode == UpdateMode.AFTER_REQUEST) { - if (updateValve == null) { - updateValve = new UpdateValve(); - pipeline.addValve(updateValve); + if (readMode == ReadMode.REDIS) { + Optional res = Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UsageValve.class).findAny(); + if (res.isPresent()) { + ((UsageValve)res.get()).incUsage(); + } else { + pipeline.addValve(new UsageValve()); } - } else if (readMode == ReadMode.REDIS) { - if (updateValve == null) { - updateValve = new UsageValve(); - pipeline.addValve(updateValve); + } + if (updateMode == UpdateMode.AFTER_REQUEST) { + Optional res = Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UpdateValve.class).findAny(); + if (res.isPresent()) { + ((UpdateValve)res.get()).incUsage(); + } else { + pipeline.addValve(new UpdateValve()); } } } @@ -365,13 +357,19 @@ public class RedissonSessionManager extends ManagerBase { Pipeline pipeline = getEngine().getPipeline(); synchronized (pipeline) { - contextInUse.remove(getContext().getName()); - //remove valves when all of the RedissonSessionManagers (web apps) are not in use anymore - if (contextInUse.isEmpty()) { - if (updateValve != null) { - pipeline.removeValve(updateValve); - updateValve = null; - } + if (readMode == ReadMode.REDIS) { + Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UsageValve.class).forEach(v -> { + if (((UsageValve)v).decUsage() == 0){ + pipeline.removeValve(v); + } + }); + } + if (updateMode == UpdateMode.AFTER_REQUEST) { + Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UpdateValve.class).forEach(v -> { + if (((UpdateValve)v).decUsage() == 0){ + pipeline.removeValve(v); + } + }); } } diff --git a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UpdateValve.java b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UpdateValve.java index 13ef51b94..d80cbe217 100644 --- a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UpdateValve.java +++ b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UpdateValve.java @@ -16,6 +16,7 @@ package org.redisson.tomcat; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletException; @@ -33,11 +34,21 @@ import org.apache.catalina.valves.ValveBase; public class UpdateValve extends ValveBase { private static final String ALREADY_FILTERED_NOTE = UpdateValve.class.getName() + ".ALREADY_FILTERED_NOTE"; - + + private final AtomicInteger usage = new AtomicInteger(1); + public UpdateValve() { super(true); } + public void incUsage() { + usage.incrementAndGet(); + } + + public int decUsage() { + return usage.decrementAndGet(); + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { if (getNext() == null) { diff --git a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UsageValve.java b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UsageValve.java index 40f71f191..3be08d209 100644 --- a/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UsageValve.java +++ b/redisson-tomcat/redisson-tomcat-8/src/main/java/org/redisson/tomcat/UsageValve.java @@ -22,6 +22,7 @@ import org.apache.catalina.valves.ValveBase; import javax.servlet.ServletException; import javax.servlet.http.HttpSession; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; /** * Redisson Valve object for Apache Tomcat @@ -33,10 +34,20 @@ public class UsageValve extends ValveBase { private static final String ALREADY_FILTERED_NOTE = UsageValve.class.getName() + ".ALREADY_FILTERED_NOTE"; + private final AtomicInteger usage = new AtomicInteger(1); + public UsageValve() { super(true); } + public void incUsage() { + usage.incrementAndGet(); + } + + public int decUsage() { + return usage.decrementAndGet(); + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { if (getNext() == null) { @@ -46,12 +57,11 @@ public class UsageValve extends ValveBase { //check if we already filtered/processed this request if (request.getNote(ALREADY_FILTERED_NOTE) == null) { request.setNote(ALREADY_FILTERED_NOTE, Boolean.TRUE); - RedissonSession s = null; try { if (request.getContext() != null) { HttpSession session = request.getSession(false); if (session != null) { - s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); + RedissonSession s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); if (s != null) { s.startUsage(); } @@ -61,8 +71,14 @@ public class UsageValve extends ValveBase { getNext().invoke(request, response); } finally { request.removeNote(ALREADY_FILTERED_NOTE); - if (s != null) { - s.endUsage(); + if (request.getContext() != null) { + HttpSession session = request.getSession(false); + if (session != null) { + RedissonSession s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); + if (s != null) { + s.endUsage(); + } + } } } } else { diff --git a/redisson-tomcat/redisson-tomcat-8/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java b/redisson-tomcat/redisson-tomcat-8/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java index a40348cb0..1add753a9 100644 --- a/redisson-tomcat/redisson-tomcat-8/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java +++ b/redisson-tomcat/redisson-tomcat-8/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java @@ -1,6 +1,5 @@ package org.redisson.tomcat; -import org.apache.http.client.ClientProtocolException; import org.apache.http.client.fluent.Executor; import org.apache.http.client.fluent.Request; import org.apache.http.cookie.Cookie; @@ -19,6 +18,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Arrays; +import java.util.concurrent.TimeUnit; @RunWith(Parameterized.class) public class RedissonSessionManagerTest { @@ -43,6 +43,31 @@ public class RedissonSessionManagerTest { Files.copy(Paths.get(basePath + contextName), Paths.get(basePath + "context.xml")); } + @Test + public void testUpdateTwoServers_readValue() throws Exception { + TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + try { + server1.start(); + server2.start(); + + Executor executor = Executor.newInstance(); + BasicCookieStore cookieStore = new BasicCookieStore(); + executor.use(cookieStore); + + write(8080, executor, "test", "from_server1"); + write(8081, executor, "test", "from_server2"); + + read(8080, executor, "test", "from_server2"); + read(8080, executor, "test", "from_server2"); + + } finally { + Executor.closeIdleConnections(); + server1.stop(); + server2.stop(); + } + } + @Test public void testHttpSessionListener() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); @@ -58,7 +83,7 @@ public class RedissonSessionManagerTest { TestHttpSessionListener.CREATED_INVOCATION_COUNTER = 0; TestHttpSessionListener.DESTROYED_INVOCATION_COUNTER = 0; - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); TomcatServer server3 = new TomcatServer("myapp", 8082, "src/test/"); server3.start(); @@ -76,23 +101,50 @@ public class RedissonSessionManagerTest { server3.stop(); } + @Test + public void testUpdateTwoServers_twoValues() throws Exception { + TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + try { + server1.start(); + server2.start(); + + Executor executor = Executor.newInstance(); + BasicCookieStore cookieStore = new BasicCookieStore(); + executor.use(cookieStore); + + write(8080, executor, "key1", "value1"); + write(8080, executor, "key2", "value1"); + + write(8081, executor, "key1", "value2"); + write(8081, executor, "key2", "value2"); + + read(8080, executor, "key1", "value2"); + read(8080, executor, "key2", "value2"); + + } finally { + Executor.closeIdleConnections(); + server1.stop(); + server2.stop(); + } + } + @Test public void testUpdateTwoServers() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); server1.start(); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + server2.start(); Executor executor = Executor.newInstance(); BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - - write(executor, "test", "1234"); - TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); - server2.start(); + write(8080, executor, "test", "1234"); read(8081, executor, "test", "1234"); - read(executor, "test", "1234"); - write(executor, "test", "324"); + read(8080, executor, "test", "1234"); + write(8080, executor, "test", "324"); read(8081, executor, "test", "324"); Executor.closeIdleConnections(); @@ -110,7 +162,7 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); server2.start(); @@ -122,7 +174,7 @@ public class RedissonSessionManagerTest { Thread.sleep(40000); executor.use(cookieStore); - read(executor, "test", "1234"); + read(8080, executor, "test", "1234"); Executor.closeIdleConnections(); server1.stop(); @@ -139,7 +191,8 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); + System.out.println("done1"); Cookie cookie = cookieStore.getCookies().get(0); Executor.closeIdleConnections(); @@ -152,7 +205,7 @@ public class RedissonSessionManagerTest { cookieStore = new BasicCookieStore(); cookieStore.addCookie(cookie); executor.use(cookieStore); - read(executor, "test", "1234"); + read(8080, executor, "test", "1234"); remove(executor, "test", "null"); Executor.closeIdleConnections(); @@ -168,8 +221,8 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1234"); - read(executor, "test", "1234"); + write(8080, executor, "test", "1234"); + read(8080, executor, "test", "1234"); remove(executor, "test", "null"); Executor.closeIdleConnections(); @@ -184,9 +237,9 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1"); + write(8080, executor, "test", "1"); recreate(executor, "test", "2"); - read(executor, "test", "2"); + read(8080, executor, "test", "2"); Executor.closeIdleConnections(); server.stop(); @@ -200,10 +253,10 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1"); - read(executor, "test", "1"); - write(executor, "test", "2"); - read(executor, "test", "2"); + write(8080, executor, "test", "1"); + read(8080, executor, "test", "1"); + write(8080, executor, "test", "2"); + read(8080, executor, "test", "2"); Executor.closeIdleConnections(); server.stop(); @@ -225,7 +278,7 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); Cookie cookie = cookieStore.getCookies().get(0); invalidate(executor); @@ -235,49 +288,46 @@ public class RedissonSessionManagerTest { cookieStore = new BasicCookieStore(); cookieStore.addCookie(cookie); executor.use(cookieStore); - read(executor, "test", "null"); + read(8080, executor, "test", "null"); invalidate(executor); Executor.closeIdleConnections(); server.stop(); - + + TimeUnit.SECONDS.sleep(60); Assert.assertEquals(0, r.getKeys().count()); } - private void write(Executor executor, String key, String value) throws IOException, ClientProtocolException { - String url = "http://localhost:8080/myapp/write?key=" + key + "&value=" + value; + private void write(int port, Executor executor, String key, String value) throws IOException { + String url = "http://localhost:" + port + "/myapp/write?key=" + key + "&value=" + value; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } - - private void read(int port, Executor executor, String key, String value) throws IOException, ClientProtocolException { + + private void read(int port, Executor executor, String key, String value) throws IOException { String url = "http://localhost:" + port + "/myapp/read?key=" + key; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals(value, response); } - private void read(Executor executor, String key, String value) throws IOException, ClientProtocolException { - String url = "http://localhost:8080/myapp/read?key=" + key; - String response = executor.execute(Request.Get(url)).returnContent().asString(); - Assert.assertEquals(value, response); - } - - private void remove(Executor executor, String key, String value) throws IOException, ClientProtocolException { + private void remove(Executor executor, String key, String value) throws IOException { String url = "http://localhost:8080/myapp/remove?key=" + key; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals(value, response); } - private void invalidate(Executor executor) throws IOException, ClientProtocolException { + private void invalidate(Executor executor) throws IOException { String url = "http://localhost:8080/myapp/invalidate"; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } - private void recreate(Executor executor, String key, String value) throws IOException, ClientProtocolException { + private void recreate(Executor executor, String key, String value) throws IOException { String url = "http://localhost:8080/myapp/recreate?key=" + key + "&value=" + value; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } + + } diff --git a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSession.java b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSession.java index 35c1c4e89..8b9a31e2a 100644 --- a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSession.java +++ b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSession.java @@ -67,6 +67,7 @@ public class RedissonSession extends StandardSession { private final AtomicInteger usages = new AtomicInteger(); private Map loadedAttributes = Collections.emptyMap(); + private Map updatedAttributes = Collections.emptyMap(); private Set removedAttributes = Collections.emptySet(); private final boolean broadcastSessionEvents; @@ -84,6 +85,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes = new ConcurrentHashMap<>(); + updatedAttributes = new ConcurrentHashMap<>(); } try { @@ -182,6 +184,7 @@ public class RedissonSession extends StandardSession { } map = null; loadedAttributes.clear(); + updatedAttributes.clear(); } @Override @@ -324,6 +327,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes.put(name, value); + updatedAttributes.put(name, value); } if (updateMode == UpdateMode.AFTER_REQUEST) { removedAttributes.remove(name); @@ -346,6 +350,7 @@ public class RedissonSession extends StandardSession { } if (readMode == ReadMode.REDIS) { loadedAttributes.remove(name); + updatedAttributes.remove(name); } if (updateMode == UpdateMode.AFTER_REQUEST) { removedAttributes.add(name); @@ -373,15 +378,20 @@ public class RedissonSession extends StandardSession { if (broadcastSessionEvents) { newMap.put(IS_EXPIRATION_LOCKED, isExpirationLocked); } - - if (attrs != null) { - for (Entry entry : attrs.entrySet()) { - newMap.put(entry.getKey(), entry.getValue()); + + if (readMode == ReadMode.MEMORY) { + if (attrs != null) { + for (Entry entry : attrs.entrySet()) { + newMap.put(entry.getKey(), entry.getValue()); + } } + } else { + newMap.putAll(updatedAttributes); + updatedAttributes.clear(); } - + map.putAll(newMap); - map.fastRemove(removedAttributes.toArray(new String[removedAttributes.size()])); + map.fastRemove(removedAttributes.toArray(new String[0])); if (readMode == ReadMode.MEMORY) { topic.publish(createPutAllMessage(newMap)); @@ -394,8 +404,7 @@ public class RedissonSession extends StandardSession { } removedAttributes.clear(); - loadedAttributes.clear(); - + expireSession(); } @@ -437,8 +446,10 @@ public class RedissonSession extends StandardSession { this.authType = authType; } - for (Entry entry : attrs.entrySet()) { - super.setAttribute(entry.getKey(), entry.getValue(), false); + if (readMode == ReadMode.MEMORY) { + for (Entry entry : attrs.entrySet()) { + super.setAttribute(entry.getKey(), entry.getValue(), false); + } } } @@ -447,6 +458,8 @@ public class RedissonSession extends StandardSession { super.recycle(); map = null; loadedAttributes.clear(); + updatedAttributes.clear(); + removedAttributes.clear(); } public void startUsage() { @@ -454,7 +467,9 @@ public class RedissonSession extends StandardSession { } public void endUsage() { - if (usages.decrementAndGet() == 0) { + // don't decrement usages if startUsage wasn't called +// if (usages.decrementAndGet() == 0) { + if (usages.get() == 0 || usages.decrementAndGet() == 0) { loadedAttributes.clear(); } } diff --git a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSessionManager.java b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSessionManager.java index 489da2016..0b12845da 100644 --- a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSessionManager.java +++ b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/RedissonSessionManager.java @@ -15,26 +15,13 @@ */ package org.redisson.tomcat; -import java.io.File; -import java.io.IOException; -import java.util.*; -import java.util.concurrent.ConcurrentHashMap; - -import javax.servlet.http.HttpSession; - -import org.apache.catalina.LifecycleException; -import org.apache.catalina.LifecycleState; -import org.apache.catalina.Pipeline; -import org.apache.catalina.Session; -import org.apache.catalina.SessionEvent; -import org.apache.catalina.SessionListener; +import org.apache.catalina.*; import org.apache.catalina.session.ManagerBase; -import org.apache.catalina.valves.ValveBase; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.redisson.Redisson; -import org.redisson.api.RSet; import org.redisson.api.RMap; +import org.redisson.api.RSet; import org.redisson.api.RTopic; import org.redisson.api.RedissonClient; import org.redisson.api.listener.MessageListener; @@ -43,6 +30,11 @@ import org.redisson.client.codec.StringCodec; import org.redisson.codec.CompositeCodec; import org.redisson.config.Config; +import javax.servlet.http.HttpSession; +import java.io.File; +import java.io.IOException; +import java.util.*; + /** * Redisson Session Manager for Apache Tomcat * @@ -67,10 +59,6 @@ public class RedissonSessionManager extends ManagerBase { private final String nodeId = UUID.randomUUID().toString(); - private static ValveBase updateValve; - - private static Set contextInUse = Collections.newSetFromMap(new ConcurrentHashMap()); - private MessageListener messageListener; private Codec codecToUse; @@ -253,16 +241,20 @@ public class RedissonSessionManager extends ManagerBase { Pipeline pipeline = getEngine().getPipeline(); synchronized (pipeline) { - contextInUse.add(getContext().getName()); - if (updateMode == UpdateMode.AFTER_REQUEST) { - if (updateValve == null) { - updateValve = new UpdateValve(); - pipeline.addValve(updateValve); + if (readMode == ReadMode.REDIS) { + Optional res = Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UsageValve.class).findAny(); + if (res.isPresent()) { + ((UsageValve)res.get()).incUsage(); + } else { + pipeline.addValve(new UsageValve()); } - } else if (readMode == ReadMode.REDIS) { - if (updateValve == null) { - updateValve = new UsageValve(); - pipeline.addValve(updateValve); + } + if (updateMode == UpdateMode.AFTER_REQUEST) { + Optional res = Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UpdateValve.class).findAny(); + if (res.isPresent()) { + ((UpdateValve)res.get()).incUsage(); + } else { + pipeline.addValve(new UpdateValve()); } } } @@ -365,13 +357,19 @@ public class RedissonSessionManager extends ManagerBase { Pipeline pipeline = getEngine().getPipeline(); synchronized (pipeline) { - contextInUse.remove(getContext().getName()); - //remove valves when all of the RedissonSessionManagers (web apps) are not in use anymore - if (contextInUse.isEmpty()) { - if (updateValve != null) { - pipeline.removeValve(updateValve); - updateValve = null; - } + if (readMode == ReadMode.REDIS) { + Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UsageValve.class).forEach(v -> { + if (((UsageValve)v).decUsage() == 0){ + pipeline.removeValve(v); + } + }); + } + if (updateMode == UpdateMode.AFTER_REQUEST) { + Arrays.stream(pipeline.getValves()).filter(v -> v.getClass() == UpdateValve.class).forEach(v -> { + if (((UpdateValve)v).decUsage() == 0){ + pipeline.removeValve(v); + } + }); } } diff --git a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UpdateValve.java b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UpdateValve.java index 71beef0a5..d80cbe217 100644 --- a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UpdateValve.java +++ b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UpdateValve.java @@ -16,6 +16,7 @@ package org.redisson.tomcat; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletException; @@ -33,18 +34,28 @@ import org.apache.catalina.valves.ValveBase; public class UpdateValve extends ValveBase { private static final String ALREADY_FILTERED_NOTE = UpdateValve.class.getName() + ".ALREADY_FILTERED_NOTE"; - + + private final AtomicInteger usage = new AtomicInteger(1); + public UpdateValve() { super(true); } + public void incUsage() { + usage.incrementAndGet(); + } + + public int decUsage() { + return usage.decrementAndGet(); + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { if (getNext() == null) { return; } - //check if we already filtered/processed this request + //check if we already filtered/processed this request if (request.getNote(ALREADY_FILTERED_NOTE) == null) { request.setNote(ALREADY_FILTERED_NOTE, Boolean.TRUE); try { diff --git a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UsageValve.java b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UsageValve.java index 40f71f191..3be08d209 100644 --- a/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UsageValve.java +++ b/redisson-tomcat/redisson-tomcat-9/src/main/java/org/redisson/tomcat/UsageValve.java @@ -22,6 +22,7 @@ import org.apache.catalina.valves.ValveBase; import javax.servlet.ServletException; import javax.servlet.http.HttpSession; import java.io.IOException; +import java.util.concurrent.atomic.AtomicInteger; /** * Redisson Valve object for Apache Tomcat @@ -33,10 +34,20 @@ public class UsageValve extends ValveBase { private static final String ALREADY_FILTERED_NOTE = UsageValve.class.getName() + ".ALREADY_FILTERED_NOTE"; + private final AtomicInteger usage = new AtomicInteger(1); + public UsageValve() { super(true); } + public void incUsage() { + usage.incrementAndGet(); + } + + public int decUsage() { + return usage.decrementAndGet(); + } + @Override public void invoke(Request request, Response response) throws IOException, ServletException { if (getNext() == null) { @@ -46,12 +57,11 @@ public class UsageValve extends ValveBase { //check if we already filtered/processed this request if (request.getNote(ALREADY_FILTERED_NOTE) == null) { request.setNote(ALREADY_FILTERED_NOTE, Boolean.TRUE); - RedissonSession s = null; try { if (request.getContext() != null) { HttpSession session = request.getSession(false); if (session != null) { - s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); + RedissonSession s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); if (s != null) { s.startUsage(); } @@ -61,8 +71,14 @@ public class UsageValve extends ValveBase { getNext().invoke(request, response); } finally { request.removeNote(ALREADY_FILTERED_NOTE); - if (s != null) { - s.endUsage(); + if (request.getContext() != null) { + HttpSession session = request.getSession(false); + if (session != null) { + RedissonSession s = (RedissonSession) request.getContext().getManager().findSession(session.getId()); + if (s != null) { + s.endUsage(); + } + } } } } else { diff --git a/redisson-tomcat/redisson-tomcat-9/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java b/redisson-tomcat/redisson-tomcat-9/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java index a5caa922d..1add753a9 100644 --- a/redisson-tomcat/redisson-tomcat-9/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java +++ b/redisson-tomcat/redisson-tomcat-9/src/test/java/org/redisson/tomcat/RedissonSessionManagerTest.java @@ -1,12 +1,10 @@ package org.redisson.tomcat; -import org.apache.http.client.ClientProtocolException; import org.apache.http.client.fluent.Executor; import org.apache.http.client.fluent.Request; import org.apache.http.cookie.Cookie; import org.apache.http.impl.client.BasicCookieStore; import org.junit.Assert; -import org.junit.Assume; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -20,6 +18,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; import java.util.Arrays; +import java.util.concurrent.TimeUnit; @RunWith(Parameterized.class) public class RedissonSessionManagerTest { @@ -44,6 +43,31 @@ public class RedissonSessionManagerTest { Files.copy(Paths.get(basePath + contextName), Paths.get(basePath + "context.xml")); } + @Test + public void testUpdateTwoServers_readValue() throws Exception { + TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + try { + server1.start(); + server2.start(); + + Executor executor = Executor.newInstance(); + BasicCookieStore cookieStore = new BasicCookieStore(); + executor.use(cookieStore); + + write(8080, executor, "test", "from_server1"); + write(8081, executor, "test", "from_server2"); + + read(8080, executor, "test", "from_server2"); + read(8080, executor, "test", "from_server2"); + + } finally { + Executor.closeIdleConnections(); + server1.stop(); + server2.stop(); + } + } + @Test public void testHttpSessionListener() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); @@ -59,7 +83,7 @@ public class RedissonSessionManagerTest { TestHttpSessionListener.CREATED_INVOCATION_COUNTER = 0; TestHttpSessionListener.DESTROYED_INVOCATION_COUNTER = 0; - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); TomcatServer server3 = new TomcatServer("myapp", 8082, "src/test/"); server3.start(); @@ -77,23 +101,50 @@ public class RedissonSessionManagerTest { server3.stop(); } + @Test + public void testUpdateTwoServers_twoValues() throws Exception { + TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + try { + server1.start(); + server2.start(); + + Executor executor = Executor.newInstance(); + BasicCookieStore cookieStore = new BasicCookieStore(); + executor.use(cookieStore); + + write(8080, executor, "key1", "value1"); + write(8080, executor, "key2", "value1"); + + write(8081, executor, "key1", "value2"); + write(8081, executor, "key2", "value2"); + + read(8080, executor, "key1", "value2"); + read(8080, executor, "key2", "value2"); + + } finally { + Executor.closeIdleConnections(); + server1.stop(); + server2.stop(); + } + } + @Test public void testUpdateTwoServers() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); server1.start(); + TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); + server2.start(); Executor executor = Executor.newInstance(); BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - - write(executor, "test", "1234"); - TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); - server2.start(); + write(8080, executor, "test", "1234"); read(8081, executor, "test", "1234"); - read(executor, "test", "1234"); - write(executor, "test", "324"); + read(8080, executor, "test", "1234"); + write(8080, executor, "test", "324"); read(8081, executor, "test", "324"); Executor.closeIdleConnections(); @@ -102,7 +153,7 @@ public class RedissonSessionManagerTest { } -// @Test + @Test public void testExpiration() throws Exception { TomcatServer server1 = new TomcatServer("myapp", 8080, "src/test/"); server1.start(); @@ -111,7 +162,7 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); TomcatServer server2 = new TomcatServer("myapp", 8081, "src/test/"); server2.start(); @@ -123,7 +174,7 @@ public class RedissonSessionManagerTest { Thread.sleep(40000); executor.use(cookieStore); - read(executor, "test", "1234"); + read(8080, executor, "test", "1234"); Executor.closeIdleConnections(); server1.stop(); @@ -140,7 +191,8 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); + System.out.println("done1"); Cookie cookie = cookieStore.getCookies().get(0); Executor.closeIdleConnections(); @@ -153,7 +205,7 @@ public class RedissonSessionManagerTest { cookieStore = new BasicCookieStore(); cookieStore.addCookie(cookie); executor.use(cookieStore); - read(executor, "test", "1234"); + read(8080, executor, "test", "1234"); remove(executor, "test", "null"); Executor.closeIdleConnections(); @@ -169,8 +221,8 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1234"); - read(executor, "test", "1234"); + write(8080, executor, "test", "1234"); + read(8080, executor, "test", "1234"); remove(executor, "test", "null"); Executor.closeIdleConnections(); @@ -185,9 +237,9 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1"); + write(8080, executor, "test", "1"); recreate(executor, "test", "2"); - read(executor, "test", "2"); + read(8080, executor, "test", "2"); Executor.closeIdleConnections(); server.stop(); @@ -201,10 +253,10 @@ public class RedissonSessionManagerTest { Executor executor = Executor.newInstance(); - write(executor, "test", "1"); - read(executor, "test", "1"); - write(executor, "test", "2"); - read(executor, "test", "2"); + write(8080, executor, "test", "1"); + read(8080, executor, "test", "1"); + write(8080, executor, "test", "2"); + read(8080, executor, "test", "2"); Executor.closeIdleConnections(); server.stop(); @@ -226,7 +278,7 @@ public class RedissonSessionManagerTest { BasicCookieStore cookieStore = new BasicCookieStore(); executor.use(cookieStore); - write(executor, "test", "1234"); + write(8080, executor, "test", "1234"); Cookie cookie = cookieStore.getCookies().get(0); invalidate(executor); @@ -236,49 +288,46 @@ public class RedissonSessionManagerTest { cookieStore = new BasicCookieStore(); cookieStore.addCookie(cookie); executor.use(cookieStore); - read(executor, "test", "null"); + read(8080, executor, "test", "null"); invalidate(executor); Executor.closeIdleConnections(); server.stop(); - + + TimeUnit.SECONDS.sleep(60); Assert.assertEquals(0, r.getKeys().count()); } - private void write(Executor executor, String key, String value) throws IOException, ClientProtocolException { - String url = "http://localhost:8080/myapp/write?key=" + key + "&value=" + value; + private void write(int port, Executor executor, String key, String value) throws IOException { + String url = "http://localhost:" + port + "/myapp/write?key=" + key + "&value=" + value; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } - - private void read(int port, Executor executor, String key, String value) throws IOException, ClientProtocolException { + + private void read(int port, Executor executor, String key, String value) throws IOException { String url = "http://localhost:" + port + "/myapp/read?key=" + key; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals(value, response); } - private void read(Executor executor, String key, String value) throws IOException, ClientProtocolException { - String url = "http://localhost:8080/myapp/read?key=" + key; - String response = executor.execute(Request.Get(url)).returnContent().asString(); - Assert.assertEquals(value, response); - } - - private void remove(Executor executor, String key, String value) throws IOException, ClientProtocolException { + private void remove(Executor executor, String key, String value) throws IOException { String url = "http://localhost:8080/myapp/remove?key=" + key; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals(value, response); } - private void invalidate(Executor executor) throws IOException, ClientProtocolException { + private void invalidate(Executor executor) throws IOException { String url = "http://localhost:8080/myapp/invalidate"; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } - private void recreate(Executor executor, String key, String value) throws IOException, ClientProtocolException { + private void recreate(Executor executor, String key, String value) throws IOException { String url = "http://localhost:8080/myapp/recreate?key=" + key + "&value=" + value; String response = executor.execute(Request.Get(url)).returnContent().asString(); Assert.assertEquals("OK", response); } + + }