Merge changes from topic 'signed-push-cleanup'
* changes:
Move REFS_GPG_KEYS to PublicKeyStore
config-gerrit.txt: Organize "receive" section
PushCertificateChecker: Don't throw exceptions
PushCertificateChecker: Tweak some error messages
diff --git a/gerrit-extension-api/BUCK b/gerrit-extension-api/BUCK
index 3882443..307aefa 100644
--- a/gerrit-extension-api/BUCK
+++ b/gerrit-extension-api/BUCK
@@ -51,6 +51,17 @@
visibility = ['PUBLIC'],
)
+java_test(
+ name = 'api_tests',
+ srcs = glob(['src/test/java/**/*.java']),
+ deps = [
+ ':api',
+ '//lib:truth',
+ '//lib/guice:guice',
+ ],
+ source_under_test = [':api'],
+)
+
java_doc(
name = 'extension-api-javadoc',
title = 'Gerrit Review Extension API Documentation',
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicSet.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicSet.java
index c99f233..28052ef 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicSet.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/registration/DynamicSet.java
@@ -182,6 +182,23 @@
}
/**
+ * Returns {@code true} if this set contains the given item.
+ *
+ * @param item item to check whether or not it is contained.
+ * @return {@code true} if this set contains the given item.
+ */
+ public boolean contains(final T item) {
+ Iterator<T> iterator = iterator();
+ while (iterator.hasNext()) {
+ T candidate = iterator.next();
+ if (candidate == item) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
* Add one new element to the set.
*
* @param item the item to add to the collection. Must not be null.
diff --git a/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/registration/DynamicSetTest.java b/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/registration/DynamicSetTest.java
new file mode 100644
index 0000000..dc71b12
--- /dev/null
+++ b/gerrit-extension-api/src/test/java/com/google/gerrit/extensions/registration/DynamicSetTest.java
@@ -0,0 +1,95 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.registration;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.inject.Key;
+import com.google.inject.util.Providers;
+
+import org.junit.Test;
+
+public class DynamicSetTest {
+ // In tests for {@link DynamicSet#contains(Object)}, be sure to avoid
+ // {@code assertThat(ds).contains(...) @} and
+ // {@code assertThat(ds).DoesNotContains(...) @} as (since
+ // {@link DynamicSet@} is not a {@link Collection@}) those boil down to
+ // iterating over the {@link DynamicSet@} and checking equality instead
+ // of calling {@link DynamicSet#contains(Object)}.
+ // To test for {@link DynamicSet#contains(Object)}, use
+ // {@code assertThat(ds.contains(...)).isTrue() @} and
+ // {@code assertThat(ds.contains(...)).isFalse() @} instead.
+
+ @Test
+ public void testContainsWithEmpty() throws Exception {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ assertThat(ds.contains(2)).isFalse(); //See above comment about ds.contains
+ }
+
+ @Test
+ public void testContainsTrueWithSingleElement() throws Exception {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add(2);
+
+ assertThat(ds.contains(2)).isTrue(); //See above comment about ds.contains
+ }
+
+ @Test
+ public void testContainsFalseWithSingleElement() throws Exception {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add(2);
+
+ assertThat(ds.contains(3)).isFalse(); //See above comment about ds.contains
+ }
+
+ @Test
+ public void testContainsTrueWithTwoElements() throws Exception {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add(2);
+ ds.add(4);
+
+ assertThat(ds.contains(4)).isTrue(); //See above comment about ds.contains
+ }
+
+ @Test
+ public void testContainsFalseWithTwoElements() throws Exception {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add(2);
+ ds.add(4);
+
+ assertThat(ds.contains(3)).isFalse(); //See above comment about ds.contains
+ }
+
+ @Test
+ public void testContainsDynamic() throws Exception {
+ DynamicSet<Integer> ds = new DynamicSet<>();
+ ds.add(2);
+
+ Key<Integer> key = Key.get(Integer.class);
+ ReloadableRegistrationHandle<Integer> handle = ds.add(key, Providers.of(4));
+
+ ds.add(6);
+
+ // At first, 4 is contained.
+ assertThat(ds.contains(4)).isTrue(); //See above comment about ds.contains
+
+ // Then we remove 4.
+ handle.remove();
+
+ // And now 4 should no longer be contained.
+ assertThat(ds.contains(4)).isFalse(); //See above comment about ds.contains
+ }
+}
diff --git a/gerrit-httpd/BUCK b/gerrit-httpd/BUCK
index 3345018..e215533 100644
--- a/gerrit-httpd/BUCK
+++ b/gerrit-httpd/BUCK
@@ -64,6 +64,7 @@
'//lib:truth',
'//lib/easymock:easymock',
'//lib/guice:guice',
+ '//lib/guice:guice-servlet',
'//lib/jgit:jgit',
'//lib/jgit:junit',
],
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/AllRequestFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/AllRequestFilter.java
index 3b48b65..bcc5842 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/AllRequestFilter.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/AllRequestFilter.java
@@ -15,8 +15,11 @@
package com.google.gerrit.httpd;
import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.plugins.Plugin;
+import com.google.gerrit.server.plugins.StopPluginListener;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.google.inject.internal.UniqueAnnotations;
import com.google.inject.servlet.ServletModule;
import java.io.IOException;
@@ -37,17 +40,62 @@
protected void configureServlets() {
DynamicSet.setOf(binder(), AllRequestFilter.class);
filter("/*").through(FilterProxy.class);
+
+ bind(StopPluginListener.class)
+ .annotatedWith(UniqueAnnotations.create())
+ .to(FilterProxy.class);
}
};
}
@Singleton
- static class FilterProxy implements Filter {
+ static class FilterProxy implements Filter, StopPluginListener {
private final DynamicSet<AllRequestFilter> filters;
+ private DynamicSet<AllRequestFilter> initializedFilters;
+ private FilterConfig filterConfig;
+
@Inject
FilterProxy(DynamicSet<AllRequestFilter> filters) {
this.filters = filters;
+ this.initializedFilters = new DynamicSet<>();
+ this.filterConfig = null;
+ }
+
+ /**
+ * Initializes a filter if needed
+ *
+ * @param filter The filter that should get initialized
+ * @return {@code true} iff filter is now initialized
+ * @throws ServletException if filter itself fails to init
+ */
+ private synchronized boolean initFilterIfNeeded(AllRequestFilter filter)
+ throws ServletException {
+ boolean ret = true;
+ if (filters.contains(filter)) {
+ // Regardless of whether or not the caller checked filter's
+ // containment in initializedFilters, we better re-check as we're now
+ // synchronized.
+ if (!initializedFilters.contains(filter)) {
+ filter.init(filterConfig);
+ initializedFilters.add(filter);
+ }
+ } else {
+ ret = false;
+ }
+ return ret;
+ }
+
+ private synchronized void cleanUpInitializedFilters() {
+ Iterable<AllRequestFilter> filtersToCleanUp = initializedFilters;
+ initializedFilters = new DynamicSet<>();
+ for (AllRequestFilter filter : filtersToCleanUp) {
+ if (filters.contains(filter)) {
+ initializedFilters.add(filter);
+ } else {
+ filter.destroy();
+ }
+ }
}
@Override
@@ -58,28 +106,67 @@
@Override
public void doFilter(ServletRequest req, ServletResponse res)
throws IOException, ServletException {
- if (itr.hasNext()) {
- itr.next().doFilter(req, res, this);
- } else {
- last.doFilter(req, res);
+ while (itr.hasNext()) {
+ AllRequestFilter filter = itr.next();
+ // To avoid {@code synchronized} on the the whole filtering (and
+ // thereby killing concurrency), we start the below disjunction
+ // with an unsynchronized check for containment. This
+ // unsynchronized check is always correct if no filters got
+ // initialized/cleaned concurrently behind our back.
+ // The case of concurrently initialized filters is saved by the
+ // call to initFilterIfNeeded. So that's fine too.
+ // The case of concurrently cleaned filters between the {@code if}
+ // condition and the call to {@code doFilter} is not saved by
+ // anything. If a filter is getting removed concurrently while
+ // another thread is in those two lines, doFilter might (but need
+ // not) fail.
+ //
+ // Since this failure only occurs if a filter is deleted
+ // (e.g.: a plugin reloaded) exactly when a thread is in those
+ // two lines, and it only breaks a single request, we're ok with
+ // it, given that this is really both really improbable and also
+ // the "proper" fix for it would basically kill concurrency of
+ // webrequests.
+ if (initializedFilters.contains(filter)
+ || initFilterIfNeeded(filter)) {
+ filter.doFilter(req, res, this);
+ return;
+ }
}
+ last.doFilter(req, res);
}
}.doFilter(req, res);
}
@Override
public void init(FilterConfig config) throws ServletException {
+ // Plugins that provide AllRequestFilters might get loaded later at
+ // runtime, long after this init method had been called. To allow to
+ // correctly init such plugins' AllRequestFilters, we keep the
+ // FilterConfig around, and reuse it to lazy init the AllRequestFilters.
+ filterConfig = config;
+
for (AllRequestFilter f: filters) {
- f.init(config);
+ initFilterIfNeeded(f);
}
}
@Override
- public void destroy() {
- for (AllRequestFilter f: filters) {
- f.destroy();
+ public synchronized void destroy() {
+ Iterable<AllRequestFilter> filtersToDestroy = initializedFilters;
+ initializedFilters = new DynamicSet<>();
+ for (AllRequestFilter filter: filtersToDestroy) {
+ filter.destroy();
}
}
+
+ @Override
+ public void onStopPlugin(Plugin plugin) {
+ // In order to allow properly garbage collection, we need to scrub
+ // initializedFilters clean of filters stemming from plugins as they
+ // get unloaded.
+ cleanUpInitializedFilters();
+ }
}
@Override
diff --git a/gerrit-httpd/src/test/java/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
new file mode 100644
index 0000000..10a35a8
--- /dev/null
+++ b/gerrit-httpd/src/test/java/com/google/gerrit/httpd/AllRequestFilterFilterProxyTest.java
@@ -0,0 +1,366 @@
+// Copyright (C) 2015 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.httpd;
+
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.capture;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.newCapture;
+
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.registration.ReloadableRegistrationHandle;
+import com.google.gerrit.server.plugins.Plugin;
+import com.google.inject.Key;
+import com.google.inject.util.Providers;
+
+import org.easymock.Capture;
+import org.easymock.EasyMockSupport;
+import org.easymock.IMocksControl;
+import org.junit.Before;
+import org.junit.Test;
+
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+public class AllRequestFilterFilterProxyTest {
+ /**
+ * Set of filters for FilterProxy
+ * <p/>
+ * This set is used to as set of filters when fetching an
+ * {@link AllRequestFilter.FilterProxy} instance through
+ * {@link #getFilterProxy()}.
+ */
+ private DynamicSet<AllRequestFilter> filters;
+
+ @Before
+ public void setUp() throws Exception {
+ // Force starting each test with an initially empty set of filters.
+ // Filters get added by the tests themselves.
+ filters = new DynamicSet<>();
+ }
+
+ // The wrapping of {@link #getFilterProxy()} and
+ // {@link #addFilter(AllRequestFilter)} into separate methods may seem
+ // overengineered at this point. However, if in the future we decide to not
+ // test the inner class directly, but rather test from the outside using
+ // Guice Injectors, it is now sufficient to change only those two methods,
+ // and we need not mess with the individual tests.
+
+ /**
+ * Obtain a FilterProxy with a known DynamicSet of filters
+ * <p/>
+ * The returned {@link AllRequestFilter.FilterProxy} can have new filters
+ * added dynamically by calling {@link #addFilter(AllRequestFilter)}.
+ */
+ private AllRequestFilter.FilterProxy getFilterProxy() {
+ return new AllRequestFilter.FilterProxy(filters);
+ }
+
+ /**
+ * Add a filter to created FilterProxy instances
+ * <p/>
+ * This method adds the given filter to all
+ * {@link AllRequestFilter.FilterProxy} instances created by
+ * {@link #getFilterProxy()}.
+ */
+ private ReloadableRegistrationHandle<AllRequestFilter> addFilter(
+ final AllRequestFilter filter) {
+ Key<AllRequestFilter> key = Key.get(AllRequestFilter.class);
+ return filters.add(key, Providers.of(filter));
+ }
+
+ @Test
+ public void testNoFilters() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock(FilterConfig.class);
+ HttpServletRequest req = ems.createMock(HttpServletRequest.class);
+ HttpServletResponse res = ems.createMock(HttpServletResponse.class);
+
+ FilterChain chain = ems.createMock(FilterChain.class);
+ chain.doFilter(req, res);
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+
+ filterProxy.init(config);
+ filterProxy.doFilter(req, res, chain);
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void testSingleFilterNoBubbling() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock("config", FilterConfig.class);
+ HttpServletRequest req = ems.createMock("req", HttpServletRequest.class);
+ HttpServletResponse res = ems.createMock("res", HttpServletResponse.class);
+
+ FilterChain chain = ems.createMock("chain", FilterChain.class);
+
+ AllRequestFilter filter = ems.createStrictMock("filter", AllRequestFilter.class);
+ filter.init(config);
+ filter.doFilter(eq(req), eq(res), anyObject(FilterChain.class));
+ filter.destroy();
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+ addFilter(filter);
+
+ filterProxy.init(config);
+ filterProxy.doFilter(req, res, chain);
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void testSingleFilterBubbling() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock(FilterConfig.class);
+ HttpServletRequest req = ems.createMock(HttpServletRequest.class);
+ HttpServletResponse res = ems.createMock(HttpServletResponse.class);
+
+ IMocksControl mockControl = ems.createStrictControl();
+ FilterChain chain = mockControl.createMock(FilterChain.class);
+
+ Capture<FilterChain> capturedChain = newCapture();
+
+ AllRequestFilter filter = mockControl.createMock(AllRequestFilter.class);
+ filter.init(config);
+ filter.doFilter(eq(req), eq(res), capture(capturedChain));
+ chain.doFilter(req, res);
+ filter.destroy();
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+ addFilter(filter);
+
+ filterProxy.init(config);
+ filterProxy.doFilter(req, res, chain);
+ capturedChain.getValue().doFilter(req, res);
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void testTwoFiltersNoBubbling() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock(FilterConfig.class);
+ HttpServletRequest req = ems.createMock(HttpServletRequest.class);
+ HttpServletResponse res = ems.createMock(HttpServletResponse.class);
+
+ IMocksControl mockControl = ems.createStrictControl();
+ FilterChain chain = mockControl.createMock(FilterChain.class);
+
+ AllRequestFilter filterA = mockControl.createMock(AllRequestFilter.class);
+
+ AllRequestFilter filterB = mockControl.createMock(AllRequestFilter.class);
+ filterA.init(config);
+ filterB.init(config);
+ filterA.doFilter(eq(req), eq(res), anyObject(FilterChain.class));
+ filterA.destroy();
+ filterB.destroy();
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+ addFilter(filterA);
+ addFilter(filterB);
+
+ filterProxy.init(config);
+ filterProxy.doFilter(req, res, chain);
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void testTwoFiltersBubbling() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock(FilterConfig.class);
+ HttpServletRequest req = ems.createMock(HttpServletRequest.class);
+ HttpServletResponse res = ems.createMock(HttpServletResponse.class);
+
+ IMocksControl mockControl = ems.createStrictControl();
+ FilterChain chain = mockControl.createMock(FilterChain.class);
+
+ Capture<FilterChain> capturedChainA = newCapture();
+ Capture<FilterChain> capturedChainB = newCapture();
+
+ AllRequestFilter filterA = mockControl.createMock(AllRequestFilter.class);
+ AllRequestFilter filterB = mockControl.createMock(AllRequestFilter.class);
+
+ filterA.init(config);
+ filterB.init(config);
+ filterA.doFilter(eq(req), eq(res), capture(capturedChainA));
+ filterB.doFilter(eq(req), eq(res), capture(capturedChainB));
+ chain.doFilter(req, res);
+ filterA.destroy();
+ filterB.destroy();
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+ addFilter(filterA);
+ addFilter(filterB);
+
+ filterProxy.init(config);
+ filterProxy.doFilter(req, res, chain);
+ capturedChainA.getValue().doFilter(req, res);
+ capturedChainB.getValue().doFilter(req, res);
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void testPostponedLoading() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock(FilterConfig.class);
+ HttpServletRequest req1 = ems.createMock("req1", HttpServletRequest.class);
+ HttpServletRequest req2 = ems.createMock("req2", HttpServletRequest.class);
+ HttpServletResponse res1 = ems.createMock("res1", HttpServletResponse.class);
+ HttpServletResponse res2 = ems.createMock("res2", HttpServletResponse.class);
+
+ IMocksControl mockControl = ems.createStrictControl();
+ FilterChain chain = mockControl.createMock("chain", FilterChain.class);
+
+ Capture<FilterChain> capturedChainA1 = newCapture();
+ Capture<FilterChain> capturedChainA2 = newCapture();
+ Capture<FilterChain> capturedChainB = newCapture();
+
+ AllRequestFilter filterA = mockControl.createMock("filterA", AllRequestFilter.class);
+ AllRequestFilter filterB = mockControl.createMock("filterB", AllRequestFilter.class);
+
+ filterA.init(config);
+ filterA.doFilter(eq(req1), eq(res1), capture(capturedChainA1));
+ chain.doFilter(req1, res1);
+
+ filterA.doFilter(eq(req2), eq(res2), capture(capturedChainA2));
+ filterB.init(config); // <-- This is crucial part. filterB got loaded
+ // after filterProxy's init finished. Nonetheless filterB gets initialized.
+ filterB.doFilter(eq(req2), eq(res2), capture(capturedChainB));
+ chain.doFilter(req2, res2);
+
+ filterA.destroy();
+ filterB.destroy();
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+ addFilter(filterA);
+
+ filterProxy.init(config);
+ filterProxy.doFilter(req1, res1, chain);
+ capturedChainA1.getValue().doFilter(req1, res1);
+
+ addFilter(filterB); // <-- Adds filter after filterProxy's init got called.
+ filterProxy.doFilter(req2, res2, chain);
+ capturedChainA2.getValue().doFilter(req2, res2);
+ capturedChainB.getValue().doFilter(req2, res2);
+
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+
+ @Test
+ public void testDynamicUnloading() throws Exception {
+ EasyMockSupport ems = new EasyMockSupport();
+
+ FilterConfig config = ems.createMock(FilterConfig.class);
+ HttpServletRequest req1 = ems.createMock("req1", HttpServletRequest.class);
+ HttpServletRequest req2 = ems.createMock("req2", HttpServletRequest.class);
+ HttpServletRequest req3 = ems.createMock("req3", HttpServletRequest.class);
+ HttpServletResponse res1 = ems.createMock("res1", HttpServletResponse.class);
+ HttpServletResponse res2 = ems.createMock("res2", HttpServletResponse.class);
+ HttpServletResponse res3 = ems.createMock("res3", HttpServletResponse.class);
+
+ Plugin plugin = ems.createMock(Plugin.class);
+
+ IMocksControl mockControl = ems.createStrictControl();
+ FilterChain chain = mockControl.createMock("chain", FilterChain.class);
+
+ Capture<FilterChain> capturedChainA1 = newCapture();
+ Capture<FilterChain> capturedChainB1 = newCapture();
+ Capture<FilterChain> capturedChainB2 = newCapture();
+
+ AllRequestFilter filterA = mockControl.createMock("filterA", AllRequestFilter.class);
+ AllRequestFilter filterB = mockControl.createMock("filterB", AllRequestFilter.class);
+
+ filterA.init(config);
+ filterB.init(config);
+
+ filterA.doFilter(eq(req1), eq(res1), capture(capturedChainA1));
+ filterB.doFilter(eq(req1), eq(res1), capture(capturedChainB1));
+ chain.doFilter(req1, res1);
+
+ filterA.destroy(); // Cleaning up of filterA after it got unloaded
+
+ filterB.doFilter(eq(req2), eq(res2), capture(capturedChainB2));
+ chain.doFilter(req2, res2);
+
+ filterB.destroy(); // Cleaning up of filterA after it got unloaded
+
+ chain.doFilter(req3, res3);
+
+ ems.replayAll();
+
+ AllRequestFilter.FilterProxy filterProxy = getFilterProxy();
+ ReloadableRegistrationHandle<AllRequestFilter> handleFilterA =
+ addFilter(filterA);
+ ReloadableRegistrationHandle<AllRequestFilter> handleFilterB =
+ addFilter(filterB);
+
+ filterProxy.init(config);
+
+ // Request #1 with filterA and filterB
+ filterProxy.doFilter(req1, res1, chain);
+ capturedChainA1.getValue().doFilter(req1, res1);
+ capturedChainB1.getValue().doFilter(req1, res1);
+
+ // Unloading filterA
+ handleFilterA.remove();
+ filterProxy.onStopPlugin(plugin);
+
+ // Request #1 only with filterB
+ filterProxy.doFilter(req2, res2, chain);
+ capturedChainA1.getValue().doFilter(req2, res2);
+
+ // Unloading filterB
+ handleFilterB.remove();
+ filterProxy.onStopPlugin(plugin);
+
+ // Request #1 with no additional filters
+ filterProxy.doFilter(req3, res3, chain);
+
+ filterProxy.destroy();
+
+ ems.verifyAll();
+ }
+}
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java
index 06d907a..aea438c 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java
@@ -18,7 +18,6 @@
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.GroupList;
import com.google.gerrit.server.git.ProjectConfig;
-import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.VersionedMetaData;
import com.google.inject.Inject;
@@ -102,15 +101,8 @@
}
private GroupList readGroupList() throws IOException {
- ValidationError.Sink errors = new ValidationError.Sink() {
- @Override
- public void error(ValidationError error) {
- log.error("Error parsing file " + GroupList.FILE_NAME + ": " + error.getMessage());
- }
- };
- String text = readUTF8(GroupList.FILE_NAME);
-
- return GroupList.parse(text, errors);
+ return GroupList.parse(readUTF8(GroupList.FILE_NAME),
+ GroupList.createLoggerSink(GroupList.FILE_NAME, log));
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountQueries.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountQueries.java
index b12e7ce..2ea6c53 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountQueries.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/VersionedAccountQueries.java
@@ -17,7 +17,6 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.git.QueryList;
-import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.VersionedMetaData;
import org.eclipse.jgit.errors.ConfigInvalidException;
@@ -53,14 +52,8 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
- ValidationError.Sink errors = new ValidationError.Sink() {
- @Override
- public void error(ValidationError error) {
- log.error("Error parsing file " + QueryList.FILE_NAME + ": " +
- error.getMessage());
- }
- };
- queryList = QueryList.parse(readUTF8(QueryList.FILE_NAME), errors);
+ queryList = QueryList.parse(readUTF8(QueryList.FILE_NAME),
+ QueryList.createLoggerSink(QueryList.FILE_NAME, log));
}
@Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/TabFile.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/TabFile.java
index 74d8f2d..13d2b1e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/TabFile.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/TabFile.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.git;
+import org.slf4j.Logger;
+
import java.io.BufferedReader;
import java.io.IOException;
import java.io.StringReader;
@@ -126,4 +128,9 @@
}
return r.toString();
}
+
+ public static ValidationError.Sink createLoggerSink(String file, Logger log) {
+ return ValidationError.createLoggerSink("Error parsing file " + file + ": ",
+ log);
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ValidationError.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ValidationError.java
index ad84046..e6a8ae4 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ValidationError.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ValidationError.java
@@ -14,6 +14,8 @@
package com.google.gerrit.server.git;
+import org.slf4j.Logger;
+
/** Indicates a problem with Git based data. */
public class ValidationError {
private final String message;
@@ -42,4 +44,13 @@
public interface Sink {
void error(ValidationError error);
}
+
+ public static Sink createLoggerSink(final String message, final Logger log) {
+ return new ValidationError.Sink() {
+ @Override
+ public void error(ValidationError error) {
+ log.error(message + error.getMessage());
+ }
+ };
+ }
}
