Match web_sessions cache pattern only when synchronized When high-availability has been instructed not to handle web_sessions (by setting websession.synchronize=false), it should not trigger web_sessions cache evictions. In Gerrit, evicting an entry from the 'web_sessions' is equivalent to logging out the user associated with that session. Forwarding web_sessions eviction is the correct behavior when their handling and storing is responsibility of the high-availability plugin. It is a bug when this happens unconditionally (even when websession.synchronize=false), because it triggers a log out of that user from the peer node. Remove 'web_sessions' cache from the list of default caches that are always forwarded and, instead, conditionally add it only when websession synchronization is enabled. Bug: Issue 13347 Change-Id: I271dbf8bff31da8c850fc98ff6369dca9a156acc
diff --git a/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java b/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java index cab8f58..9a8adce 100644 --- a/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java +++ b/src/main/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcher.java
@@ -26,14 +26,18 @@ @Singleton class CachePatternMatcher { private static final List<String> DEFAULT_PATTERNS = - ImmutableList.of( - "^accounts.*", "^groups.*", "ldap_usernames", "projects", "sshkeys", "web_sessions"); + ImmutableList.of("^accounts.*", "^groups.*", "ldap_usernames", "projects", "sshkeys"); private final Pattern pattern; @Inject CachePatternMatcher(Configuration cfg) { List<String> patterns = new ArrayList<>(DEFAULT_PATTERNS); + + if (cfg.websession().synchronize()) { + patterns.add("web_sessions"); + } + patterns.addAll(cfg.cache().patterns()); this.pattern = Pattern.compile(Joiner.on("|").join(patterns)); }
diff --git a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcherTest.java b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcherTest.java index 9093238..0aac3e4 100644 --- a/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcherTest.java +++ b/src/test/java/com/ericsson/gerrit/plugins/highavailability/cache/CachePatternMatcherTest.java
@@ -20,6 +20,7 @@ import com.ericsson.gerrit.plugins.highavailability.Configuration; import com.google.common.collect.ImmutableList; +import java.util.Collections; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -78,4 +79,28 @@ assertWithMessage(cache + " should not match").that(matcher.matches(cache)).isFalse(); } } + + @Test + public void testShouldNotMatchWebSessionsWhenNotSynchronized() { + String cache = "web_sessions"; + when(configurationMock.cache().patterns()).thenReturn(Collections.emptyList()); + when(configurationMock.websession().synchronize()).thenReturn(false); + CachePatternMatcher matcher = new CachePatternMatcher(configurationMock); + + assertWithMessage(cache + " should NOT match when websession.synchronize is false") + .that(matcher.matches(cache)) + .isFalse(); + } + + @Test + public void testShouldMatchWebSessionsWhenSynchronized() { + String cache = "web_sessions"; + when(configurationMock.cache().patterns()).thenReturn(Collections.emptyList()); + when(configurationMock.websession().synchronize()).thenReturn(true); + CachePatternMatcher matcher = new CachePatternMatcher(configurationMock); + + assertWithMessage(cache + " should match when websession.synchronize is true") + .that(matcher.matches(cache)) + .isTrue(); + } }