Merge branch 'stable-2.15'
* stable-2.15:
Disable draft magic branch option per default
GetUserFilter: Allow to include username in servlet response header
WorkQueue: Don't lazy-initialize the default queue
Change-Id: I36726fc943df542494925c5f2d1e67ddeb95a21a
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index a79b7a5..9a37f25 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -1156,6 +1156,17 @@
+
Default is `ALL`.
+[[change.allowDrafts]]change.allowDrafts::
++
+Legacy support for drafts workflow. If set to true, pushing a new change
+with draft option will create a private change. Pushing with draft option
+to an existing change will create change edit.
++
+Enabling this option allows to push to the `refs/drafts/branch`. When
+disabled any push to `refs/drafts/branch` will be rejected.
++
+Default is false.
+
[[change.cacheAutomerge]]change.cacheAutomerge::
+
When reviewing diff commits, the left-hand side shows the output of the
@@ -2424,6 +2435,14 @@
+
Default value is true.
+[[http.addUserAsResponseHeader]]http.addUserAsResponseHeader::
++
+If true, the header 'User' will be added to the list of response headers so it
+can be accessed from a reverse proxy for logging purposes.
+
++
+Default value is false.
+
[[httpd]]
=== Section httpd
diff --git a/java/com/google/gerrit/httpd/GetUserFilter.java b/java/com/google/gerrit/httpd/GetUserFilter.java
index 4b3d41d5..2199411 100644
--- a/java/com/google/gerrit/httpd/GetUserFilter.java
+++ b/java/com/google/gerrit/httpd/GetUserFilter.java
@@ -14,6 +14,7 @@
package com.google.gerrit.httpd;
+import com.google.common.collect.ImmutableMap;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
@@ -27,33 +28,49 @@
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
+import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.lib.Config;
-/** Stores user as a request attribute, so servlets can access it outside of the request scope. */
+/**
+ * Stores user as a request attribute and/or response header, so servlets and reverse proxies can
+ * access it outside of the request/response scope.
+ */
@Singleton
public class GetUserFilter implements Filter {
- public static final String REQ_ATTR_KEY = "User";
+ public static final String USER_ATTR_KEY = "User";
public static class Module extends ServletModule {
- private final boolean enabled;
+ private final boolean reqEnabled;
+ private final boolean resEnabled;
@Inject
Module(@GerritServerConfig Config cfg) {
- enabled = cfg.getBoolean("http", "addUserAsRequestAttribute", true);
+ reqEnabled = cfg.getBoolean("http", "addUserAsRequestAttribute", true);
+ resEnabled = cfg.getBoolean("http", "addUserAsResponseHeader", false);
}
@Override
protected void configureServlets() {
- if (enabled) {
- filter("/*").through(GetUserFilter.class);
+ if (resEnabled || reqEnabled) {
+ ImmutableMap.Builder<String, String> initParams = ImmutableMap.builder();
+ if (reqEnabled) {
+ initParams.put("reqEnabled", "");
+ }
+ if (resEnabled) {
+ initParams.put("resEnabled", "");
+ }
+ filter("/*").through(GetUserFilter.class, initParams.build());
}
}
}
private final Provider<CurrentUser> userProvider;
+ private boolean reqEnabled;
+ private boolean resEnabled;
+
@Inject
GetUserFilter(Provider<CurrentUser> userProvider) {
this.userProvider = userProvider;
@@ -64,7 +81,13 @@
throws IOException, ServletException {
CurrentUser user = userProvider.get();
if (user != null && user.isIdentifiedUser()) {
- req.setAttribute(REQ_ATTR_KEY, user.asIdentifiedUser().getLoggableName());
+ String loggableName = user.asIdentifiedUser().getLoggableName();
+ if (reqEnabled) {
+ req.setAttribute(USER_ATTR_KEY, loggableName);
+ }
+ if (resEnabled && resp instanceof HttpServletResponse) {
+ ((HttpServletResponse) resp).addHeader(USER_ATTR_KEY, loggableName);
+ }
}
chain.doFilter(req, resp);
}
@@ -73,5 +96,8 @@
public void destroy() {}
@Override
- public void init(FilterConfig arg0) {}
+ public void init(FilterConfig arg0) {
+ reqEnabled = arg0.getInitParameter("reqEnabled") != null ? true : false;
+ resEnabled = arg0.getInitParameter("resEnabled") != null ? true : false;
+ }
}
diff --git a/java/com/google/gerrit/pgm/http/jetty/HttpLog.java b/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
index 011ebf0..d7bc720 100644
--- a/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
+++ b/java/com/google/gerrit/pgm/http/jetty/HttpLog.java
@@ -89,7 +89,7 @@
String uri = req.getRequestURI();
uri = redactQueryString(uri, req.getQueryString());
- String user = (String) req.getAttribute(GetUserFilter.REQ_ATTR_KEY);
+ String user = (String) req.getAttribute(GetUserFilter.USER_ATTR_KEY);
if (user != null) {
event.setProperty(P_USER, user);
}
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 3bc44c5..74f986e 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -84,8 +84,7 @@
}
};
- private ScheduledExecutorService defaultQueue;
- private final int defaultQueueSize;
+ private final ScheduledExecutorService defaultQueue;
private final IdGenerator idGenerator;
private final CopyOnWriteArrayList<Executor> queues;
@@ -98,14 +97,11 @@
public WorkQueue(IdGenerator idGenerator, int defaultThreadPoolSize) {
this.idGenerator = idGenerator;
this.queues = new CopyOnWriteArrayList<>();
- this.defaultQueueSize = defaultThreadPoolSize;
+ this.defaultQueue = createQueue(defaultThreadPoolSize, "WorkQueue");
}
/** Get the default work queue, for miscellaneous tasks. */
- public synchronized ScheduledExecutorService getDefaultQueue() {
- if (defaultQueue == null) {
- defaultQueue = createQueue(defaultQueueSize, "WorkQueue");
- }
+ public ScheduledExecutorService getDefaultQueue() {
return defaultQueue;
}
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index dbb6bcf..c93c1d7 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1584,6 +1584,14 @@
return;
}
+ // TODO(davido): Remove legacy support for drafts magic branch option
+ // after repo-tool supports private and work-in-progress changes.
+ if (magicBranch.draft && !receiveConfig.allowDrafts) {
+ errors.put(ReceiveError.CODE_REVIEW, ref);
+ reject(cmd, "draft workflow is disabled");
+ return;
+ }
+
if (magicBranch.isPrivate && magicBranch.removePrivate) {
reject(cmd, "the options 'private' and 'remove-private' are mutually exclusive");
return;
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveConfig.java b/java/com/google/gerrit/server/git/receive/ReceiveConfig.java
index cdbf310..b1c9f13 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveConfig.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveConfig.java
@@ -27,6 +27,7 @@
class ReceiveConfig {
final boolean checkMagicRefs;
final boolean checkReferencedObjectsAreReachable;
+ final boolean allowDrafts;
final int maxBatchCommits;
final boolean disablePrivateChanges;
private final int systemMaxBatchChanges;
@@ -37,6 +38,7 @@
checkMagicRefs = config.getBoolean("receive", null, "checkMagicRefs", true);
checkReferencedObjectsAreReachable =
config.getBoolean("receive", null, "checkReferencedObjectsAreReachable", true);
+ allowDrafts = config.getBoolean("change", null, "allowDrafts", false);
maxBatchCommits = config.getInt("receive", null, "maxBatchCommits", 10000);
systemMaxBatchChanges = config.getInt("receive", "maxBatchChanges", 0);
disablePrivateChanges = config.getBoolean("change", null, "disablePrivateChanges", false);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java b/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java
index 287434d..ffb8b34 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/DisablePrivateChangesIT.java
@@ -59,6 +59,7 @@
}
@Test
+ @GerritConfig(name = "change.allowDrafts", value = "true")
@GerritConfig(name = "change.disablePrivateChanges", value = "true")
public void pushDraftsWithDisablePrivateChangesTrue() throws Exception {
RevCommit initialHead = getRemoteHead();
@@ -81,6 +82,7 @@
}
@Test
+ @GerritConfig(name = "change.allowDrafts", value = "true")
public void pushPrivatesWithDisablePrivateChangesFalse() throws Exception {
PushOneCommit.Result result =
pushFactory.create(db, admin.getIdent(), testRepo).to("refs/for/master%private");
@@ -88,6 +90,7 @@
}
@Test
+ @GerritConfig(name = "change.allowDrafts", value = "true")
public void pushDraftsWithDisablePrivateChangesFalse() throws Exception {
RevCommit initialHead = getRemoteHead();
PushOneCommit.Result result =
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index df146c7..f03b19b 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -1967,6 +1967,16 @@
}
@Test
+ public void pushWithDraftOptionIsDisabledPerDefault() throws Exception {
+ for (String ref : ImmutableSet.of("refs/drafts/master", "refs/for/master%draft")) {
+ PushOneCommit.Result r = pushTo(ref);
+ r.assertErrorStatus();
+ r.assertMessage("draft workflow is disabled");
+ }
+ }
+
+ @GerritConfig(name = "change.allowDrafts", value = "true")
+ @Test
public void pushDraftGetsPrivateChange() throws Exception {
String changeId1 = createChange("refs/drafts/master").getChangeId();
String changeId2 = createChange("refs/for/master%draft").getChangeId();
@@ -1982,6 +1992,7 @@
assertThat(info2.revisions).hasSize(1);
}
+ @GerritConfig(name = "change.allowDrafts", value = "true")
@Sandboxed
@Test
public void pushWithDraftOptionToExistingNewChangeGetsChangeEdit() throws Exception {
