From d64425d0015df6f403b616d543bf0672be559bbe Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Mon, 17 Sep 2018 23:12:06 +0200 Subject: [PATCH 01/14] Revert "AsyncReceiveCommits: Move ReceiveCommits into Worker" This reverts commit 095467aa8fa68a19749062517893601ef98072f9. The reason for revert: pre-receive hook chain in ReceivePack must be established before PreReceiveHook#onPreReceive() actually gets called (in ctor of ARC) and not during the PreReceiveHook#onPreReceive() call. rc.init() initializes the hooks and establishes the hook chain in receive pack instance (eventually): void init() { for (ReceivePackInitializer i : initializers) { i.init(projectState.getNameKey(), rp); } } SignedPushModule is one from the initializers: List hooks = new ArrayList<>(3); if (ps.isRequireSignedPush()) { hooks.add(SignedPushPreReceiveHook.Required.INSTANCE); } hooks.add(hook); hooks.add(rp.getPreReceiveHook()); rp.setPreReceiveHook(PreReceiveHookChain.newChain(hooks)); However, after the move of rc.init() from ARC ctor to Worker ctor that is called from onPreReceive(), the rc.init() gets actually called from ReceivePack#service line 280: preReceive.onPreReceive(this, filterCommands(Result.NOT_ATTEMPTED)); The whole stack trace: AsyncReceiveCommits.onPreReceive(ReceivePack, Collection) line: 278 ReceivePack.service() line: 280 ReceivePack.receive(InputStream, OutputStream, OutputStream) line: 221 Receive.runImpl() line: 104 Receive(AbstractGitCommand).service() line: 97 AbstractGitCommand.access$0(AbstractGitCommand) line: 86 AbstractGitCommand$1.run() line: 63 BaseCommand$TaskThunk.run() line: 456 LoggingContextAwareRunnable.run() line: 83 So that the pre-receive chain is mutated as a side effect of ReceiveCommits#init() in the preReceive instance variable of ReceivePack instance but only during the invocation of the very first hook in this class and thus this mutation became a no-op. As the consequence other pre-receive hooks don't get called. New acceptance test is added in follow-up change to avoid such regression in future. Test Plan: 1. Set up signed push for gerrit site 2. Enable signed push for project foo 3. Enable require signed push for project foo 4. Push to project foo without passing in --signed option should fail: $ git push gerrita HEAD:refs/for/master Counting objects: 3, done. Writing objects: 100% (3/3), 317 bytes | 317.00 KiB/s, done. Total 3 (delta 0), reused 0 (delta 0) remote: ERROR: Signed push is required Bug: Issue 7750 Change-Id: Ia63ddb0554542a77d6c28b5d9e6093a0bbd56a0e --- .../server/git/receive/AsyncReceiveCommits.java | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java index 654341dea8..48c0713bde 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java @@ -102,13 +102,9 @@ public class AsyncReceiveCommits implements PreReceiveHook { final MultiProgressMonitor progress; private final Collection commands; - private final ReceiveCommits rc; private Worker(Collection commands) { this.commands = commands; - rc = factory.create(projectControl, rp, allRefsWatcher, extraReviewers); - rc.init(); - rc.setMessageSender(messageSender); progress = new MultiProgressMonitor(new MessageSenderOutputStream(), "Processing changes"); } @@ -164,7 +160,7 @@ public class AsyncReceiveCommits implements PreReceiveHook { } } - private final ReceiveCommits.Factory factory; + private final ReceiveCommits rc; private final ReceivePack rp; private final ExecutorService executor; private final RequestScopePropagator scopePropagator; @@ -173,8 +169,6 @@ public class AsyncReceiveCommits implements PreReceiveHook { private final long timeoutMillis; private final ProjectControl projectControl; private final Repository repo; - private final MessageSender messageSender; - private final SetMultimap extraReviewers; private final AllRefsWatcher allRefsWatcher; @Inject @@ -195,7 +189,6 @@ public class AsyncReceiveCommits implements PreReceiveHook { @Assisted @Nullable MessageSender messageSender, @Assisted SetMultimap extraReviewers) throws PermissionBackendException { - this.factory = factory; this.executor = executor; this.scopePropagator = scopePropagator; this.receiveConfig = receiveConfig; @@ -203,8 +196,6 @@ public class AsyncReceiveCommits implements PreReceiveHook { this.timeoutMillis = timeoutMillis; this.projectControl = projectControl; this.repo = repo; - this.messageSender = messageSender; - this.extraReviewers = extraReviewers; IdentifiedUser user = projectControl.getUser().asIdentifiedUser(); ProjectState state = projectControl.getProjectState(); @@ -237,6 +228,10 @@ public class AsyncReceiveCommits implements PreReceiveHook { advHooks.add(new ReceiveCommitsAdvertiseRefsHook(queryProvider, projectName)); advHooks.add(new HackPushNegotiateHook()); rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks)); + + rc = factory.create(projectControl, rp, allRefsWatcher, extraReviewers); + rc.init(); + rc.setMessageSender(messageSender); } /** Determine if the user can upload commits. */ From 1c6ff2e7245034dfad6754ee5313a699a2e555ac Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 18 Sep 2018 07:52:51 +0200 Subject: [PATCH 02/14] CreateProject: Provide signed push option on project creation Also add acceptance test that non-signed push to project that required signed push is rejected. Bug: Issue 7750 Change-Id: I28fa94ad08fb7f21e1af37c251e64421d2e443c2 --- Documentation/rest-api-projects.txt | 6 ++++++ .../com/google/gerrit/acceptance/AbstractDaemonTest.java | 2 ++ .../com/google/gerrit/acceptance/TestProjectInput.java | 4 ++++ .../gerrit/acceptance/git/AbstractPushForReview.java | 9 +++++++++ .../gerrit/extensions/api/projects/ProjectInput.java | 2 ++ .../com/google/gerrit/server/project/CreateProject.java | 6 ++++++ .../google/gerrit/server/project/CreateProjectArgs.java | 4 ++++ 7 files changed, 33 insertions(+) diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 1c54a7f78d..5a5d42b9d7 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -3187,6 +3187,12 @@ Whether content merge should be enabled for the project (`TRUE`, |`require_change_id` |`INHERIT` if not set| Whether the usage of Change-Ids is required for the project (`TRUE`, `FALSE`, `INHERIT`). +|`enable_signed_push` |`INHERIT` if not set| +Whether signed push validation is enabled on the project (`TRUE`, +`FALSE`, `INHERIT`). +|`require_signed_push` |`INHERIT` if not set| +Whether signed push validation is required on the project (`TRUE`, +`FALSE`, `INHERIT`). |`max_object_size_limit` |optional| Max allowed Git object size for this project. Common unit suffixes of 'k', 'm', or 'g' are supported. diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java index 1c99d1fc90..9e4595351e 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java @@ -402,6 +402,8 @@ public abstract class AbstractDaemonTest { in.useContentMerge = ann.useContributorAgreements(); in.useSignedOffBy = ann.useSignedOffBy(); in.useContentMerge = ann.useContentMerge(); + in.enableSignedPush = ann.enableSignedPush(); + in.requireSignedPush = ann.requireSignedPush(); } else { // Defaults should match TestProjectConfig, omitting nullable values. in.createEmptyCommit = true; diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestProjectInput.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestProjectInput.java index 739d4f5f93..86f3c03f09 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestProjectInput.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/TestProjectInput.java @@ -45,6 +45,10 @@ public @interface TestProjectInput { InheritableBoolean requireChangeId() default InheritableBoolean.INHERIT; + InheritableBoolean enableSignedPush() default InheritableBoolean.INHERIT; + + InheritableBoolean requireSignedPush() default InheritableBoolean.INHERIT; + // Fields specific to acceptance test behavior. /** Username to use for initial clone, passed to {@link AccountCreator}. */ diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index cf80cd59ce..39f09a85c4 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -196,6 +196,15 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { } } + @Test + @GerritConfig(name = "receive.enableSignedPush", value = "true") + @TestProjectInput( + enableSignedPush = InheritableBoolean.TRUE, + requireSignedPush = InheritableBoolean.TRUE) + public void nonSignedPushRejectedWhenSignPushRequired() throws Exception { + pushTo("refs/for/master").assertErrorStatus("push cert error"); + } + @Test public void pushInitialCommitForRefsMetaConfigBranch() throws Exception { // delete refs/meta/config diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectInput.java index 612c49ca22..2adb2ddea1 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectInput.java +++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/projects/ProjectInput.java @@ -33,6 +33,8 @@ public class ProjectInput { public InheritableBoolean useContentMerge; public InheritableBoolean requireChangeId; public InheritableBoolean createNewChangeForAllNotInTarget; + public InheritableBoolean enableSignedPush; + public InheritableBoolean requireSignedPush; public String maxObjectSizeLimit; public Map> pluginConfigValues; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java index 9b355f1921..bd8f11e3f9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateProject.java @@ -186,6 +186,10 @@ public class CreateProject implements RestModifyView Date: Tue, 25 Sep 2018 09:47:44 +0900 Subject: [PATCH 03/14] Set version to 2.14.14-SNAPSHOT Change-Id: I78c09f7f0c716ef215904b070c74aa400f9a4c96 --- gerrit-acceptance-framework/pom.xml | 2 +- gerrit-extension-api/pom.xml | 2 +- gerrit-plugin-api/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-war/pom.xml | 2 +- version.bzl | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gerrit-acceptance-framework/pom.xml b/gerrit-acceptance-framework/pom.xml index 95c9ce23b3..cfe17beadb 100644 --- a/gerrit-acceptance-framework/pom.xml +++ b/gerrit-acceptance-framework/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 2.14.13 + 2.14.14-SNAPSHOT jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/gerrit-extension-api/pom.xml b/gerrit-extension-api/pom.xml index 0b12bf8457..79c039838a 100644 --- a/gerrit-extension-api/pom.xml +++ b/gerrit-extension-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 2.14.13 + 2.14.14-SNAPSHOT jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/gerrit-plugin-api/pom.xml b/gerrit-plugin-api/pom.xml index de653383e5..57b5d20722 100644 --- a/gerrit-plugin-api/pom.xml +++ b/gerrit-plugin-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 2.14.13 + 2.14.14-SNAPSHOT jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index c0ee6a3191..4f6bf7fe36 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-gwtui - 2.14.13 + 2.14.14-SNAPSHOT jar Gerrit Code Review - Plugin GWT UI Common Classes for Gerrit GWT UI Plugins diff --git a/gerrit-war/pom.xml b/gerrit-war/pom.xml index 7cfa05a729..995a6cbaad 100644 --- a/gerrit-war/pom.xml +++ b/gerrit-war/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 2.14.13 + 2.14.14-SNAPSHOT war Gerrit Code Review - WAR Gerrit WAR diff --git a/version.bzl b/version.bzl index a7f5173e53..570113f115 100644 --- a/version.bzl +++ b/version.bzl @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = "2.14.13" +GERRIT_VERSION = "2.14.14-SNAPSHOT" From 9e7e9a129517fddfa81d063d327b4b98dd3a8192 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Tue, 25 Sep 2018 07:43:26 +0200 Subject: [PATCH 04/14] Bazel: Specify name for downloaded file to http_file starlark rule In I44ca2ecfea6 native http_file was replaced with Starlark rule. During this migration the original file name was lost and hard coded to file named "downloaded". The closure_js_library expects files with .js suffix as source files, so that we had to add an intermediate rename step to make it work gain. In context of this feature request: [1] downloaded_file_path was added to the http_file rule: [2] so that we can use it now and can remove the intermediate renaming step. The aformentioned fix was included in 0.17.1 and we already have that version as the minimum required Bazel version, so that we can clean that up now. [1] https://github.com/bazelbuild/bazel/issues/5633 [2] https://github.com/bazelbuild/bazel/pull/5647 Change-Id: Ia00e5d7b4eb9c18be808b290ac299e658ab33b9a --- WORKSPACE | 1 + lib/polymer_externs/BUILD | 9 +-------- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 622f03b5c7..3d477f2e31 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -24,6 +24,7 @@ http_archive( # https://github.com/google/closure-compiler/blob/master/contrib/externs/polymer-1.0.js http_file( name = "polymer_closure", + downloaded_file_path = "polymer_closure.js", sha256 = "5a589bdba674e1fec7188e9251c8624ebf2d4d969beb6635f9148f420d1e08b1", urls = ["https://raw.githubusercontent.com/google/closure-compiler/775609aad61e14aef289ebec4bfc09ad88877f9e/contrib/externs/polymer-1.0.js"], ) diff --git a/lib/polymer_externs/BUILD b/lib/polymer_externs/BUILD index ae8f9c0ffe..2f1bdbded2 100644 --- a/lib/polymer_externs/BUILD +++ b/lib/polymer_externs/BUILD @@ -18,16 +18,9 @@ package( load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_library") -genrule( - name = "polymer_closure_renamed", - srcs = ["@polymer_closure//file"], - outs = ["polymer_closure_renamed.js"], - cmd = "cp $< $@", -) - closure_js_library( name = "polymer_closure", - srcs = [":polymer_closure_renamed"], + srcs = ["@polymer_closure//file"], data = ["//lib:LICENSE-Apache2.0"], no_closure_library = True, ) From e8b59e7dde6a056673959e3bba56498021427663 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Sep 2018 09:00:50 +0900 Subject: [PATCH 05/14] config-gerrit: Fix "invalid reference: database.h2.cachesize" warning During the documentation build, a warning is emitted because the reference "database.h2.cachesize" does not exist. The actual name is "database.h2.cacheSize" with upper-case S. Change-Id: I6481e77a88a2b340197a1369f1aa25973df83417 --- Documentation/config-gerrit.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index af1ed13097..fcaf73167b 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -648,7 +648,7 @@ H2 uses memory to cache its database content. The parameter `h2CacheSize` allows to limit the memory used by H2 and thus prevent out-of-memory caused by the H2 database using too much memory. + -See <> for a detailed discussion. +See <> for a detailed discussion. + Default is unset, using up to half of the available memory. + From 8bddc312f1cc56a3191254314927a0db667347e2 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Sep 2018 09:33:55 +0900 Subject: [PATCH 06/14] rest-api-projects: Fix "unterminated listing block" warning Change-Id: Ifddb6d4b6aa615205e60a6dc13388ecf8de9b46e --- Documentation/rest-api-projects.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 573337e5f2..3d53130153 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -3041,8 +3041,6 @@ The path to the `GerritSiteHeader.html` file. The path to the `GerritSiteFooter.html` file. |============================= ----- - GERRIT ------ Part of link:index.html[Gerrit Code Review] From d185b59c497140fe14e3671cf5a4c87e702f6feb Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 25 Sep 2018 08:54:51 +0900 Subject: [PATCH 07/14] AbstractElasticIndex: Factor out more variants of {post,perform}Request Factor out more variants of these methods that eventually end up in the one that actually makes the call to the RestClient instance. This will allow to make any necessary adjustments to the URI or request parameters in a single place. At the same time, reorder the arguments to make the order consistent across methods. Change-Id: I3ced32e8df6cbd5a5f05f4d3241f84a7e7bebb10 --- gerrit-elasticsearch/BUILD | 1 + .../elasticsearch/AbstractElasticIndex.java | 35 ++++++++++++++----- .../elasticsearch/ElasticAccountIndex.java | 4 +-- .../elasticsearch/ElasticChangeIndex.java | 4 +-- .../elasticsearch/ElasticGroupIndex.java | 4 +-- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/gerrit-elasticsearch/BUILD b/gerrit-elasticsearch/BUILD index 3a501df225..d55f0125da 100644 --- a/gerrit-elasticsearch/BUILD +++ b/gerrit-elasticsearch/BUILD @@ -7,6 +7,7 @@ java_library( visibility = ["//visibility:public"], deps = [ "//gerrit-antlr:query_exception", + "//gerrit-common:annotations", "//gerrit-extension-api:api", "//gerrit-reviewdb:server", "//gerrit-server:server", diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index e810898902..c783dd854f 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -21,6 +21,7 @@ import static org.apache.commons.codec.binary.Base64.decodeBase64; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableMap; import com.google.common.io.CharStreams; +import com.google.gerrit.common.Nullable; import com.google.gerrit.elasticsearch.ElasticMapping.MappingProperties; import com.google.gerrit.elasticsearch.builders.SearchSourceBuilder; import com.google.gerrit.elasticsearch.bulk.DeleteRequest; @@ -135,7 +136,7 @@ abstract class AbstractElasticIndex implements Index { @Override public void delete(K id) throws IOException { String uri = getURI(type, BULK); - Response response = postRequest(getDeleteActions(id), uri, getRefreshParam()); + Response response = postRequest(uri, getDeleteActions(id), getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { throw new IOException( @@ -147,10 +148,10 @@ abstract class AbstractElasticIndex implements Index { public void deleteAll() throws IOException { // Delete the index, if it exists. String endpoint = indexName + client.adapter().indicesExistParam(); - Response response = client.get().performRequest(new Request("HEAD", endpoint)); + Response response = performRequest("HEAD", endpoint); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode == HttpStatus.SC_OK) { - response = client.get().performRequest(new Request("DELETE", indexName)); + response = performRequest("DELETE", indexName); statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { throw new IOException( @@ -160,7 +161,7 @@ abstract class AbstractElasticIndex implements Index { // Recreate the index. String indexCreationFields = concatJsonString(getSettings(), getMappings()); - response = performRequest("PUT", indexCreationFields, indexName, Collections.emptyMap()); + response = performRequest("PUT", indexName, indexCreationFields); statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { String error = String.format("Failed to create index %s: %s", indexName, statusCode); @@ -228,20 +229,36 @@ abstract class AbstractElasticIndex implements Index { return encodedIndexName + "/" + encodedType + "/" + request; } - protected Response postRequest(Object payload, String uri, Map params) + protected Response postRequest(String uri, Object payload) throws IOException { + return performRequest("POST", uri, payload); + } + + protected Response postRequest(String uri, Object payload, Map params) throws IOException { - return performRequest("POST", payload, uri, params); + return performRequest("POST", uri, payload, params); } private String concatJsonString(String target, String addition) { return target.substring(0, target.length() - 1) + "," + addition.substring(1); } + private Response performRequest(String method, String uri) throws IOException { + return performRequest(method, uri, null); + } + + private Response performRequest(String method, String uri, @Nullable Object payload) + throws IOException { + return performRequest(method, uri, payload, Collections.emptyMap()); + } + private Response performRequest( - String method, Object payload, String uri, Map params) throws IOException { + String method, String uri, @Nullable Object payload, Map params) + throws IOException { Request request = new Request(method, uri); - String payloadStr = payload instanceof String ? (String) payload : payload.toString(); - request.setEntity(new NStringEntity(payloadStr, ContentType.APPLICATION_JSON)); + if (payload != null) { + String payloadStr = payload instanceof String ? (String) payload : payload.toString(); + request.setEntity(new NStringEntity(payloadStr, ContentType.APPLICATION_JSON)); + } for (Map.Entry entry : params.entrySet()) { request.addParameter(entry.getKey(), entry.getValue()); } diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java index df1a053ac9..aee4177948 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticAccountIndex.java @@ -93,7 +93,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex(schema, as)); String uri = getURI(type, BULK); - Response response = postRequest(bulk, uri, getRefreshParam()); + Response response = postRequest(uri, bulk, getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { throw new IOException( @@ -152,7 +152,7 @@ public class ElasticAccountIndex extends AbstractElasticIndex results = Collections.emptyList(); String uri = getURI(type, SEARCH); - Response response = postRequest(search, uri, Collections.emptyMap()); + Response response = postRequest(uri, search); StatusLine statusLine = response.getStatusLine(); if (statusLine.getStatusCode() == HttpStatus.SC_OK) { String content = getContent(response); diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java index d7df1a7218..9b845e09a4 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java @@ -148,7 +148,7 @@ public class ElasticChangeIndex extends AbstractElasticIndex results = Collections.emptyList(); String uri = getURI(types); - Response response = postRequest(search, uri, Collections.emptyMap()); + Response response = postRequest(uri, search); StatusLine statusLine = response.getStatusLine(); if (statusLine.getStatusCode() == HttpStatus.SC_OK) { String content = getContent(response); diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java index 89336d44c6..c01f4b40a8 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java @@ -90,7 +90,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex(schema, group)); String uri = getURI(type, BULK); - Response response = postRequest(bulk, uri, getRefreshParam()); + Response response = postRequest(uri, bulk, getRefreshParam()); int statusCode = response.getStatusLine().getStatusCode(); if (statusCode != HttpStatus.SC_OK) { throw new IOException( @@ -149,7 +149,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex results = Collections.emptyList(); String uri = getURI(type, SEARCH); - Response response = postRequest(search, uri, Collections.emptyMap()); + Response response = postRequest(uri, search); StatusLine statusLine = response.getStatusLine(); if (statusLine.getStatusCode() == HttpStatus.SC_OK) { String content = getContent(response); From 0fce318c30b58d43bd2d53a156d0dc092da861dd Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 25 Sep 2018 15:57:26 +0900 Subject: [PATCH 08/14] Elasticsearch: Ensure request URI is always prefixed with "/" When using Amazon's Elasticsearch service, requests fail with "400 Bad Request" if they are not prefixed with "/". Ensure that requests are always prefixed with "/". Bug: Issue 9761 Change-Id: If2a178e85143ba75d7f4f6b982739a9b3f2dc21c --- .../com/google/gerrit/elasticsearch/AbstractElasticIndex.java | 2 +- .../com/google/gerrit/elasticsearch/ElasticQueryAdapter.java | 2 +- .../google/gerrit/elasticsearch/ElasticRestClientProvider.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java index c783dd854f..72cc3d0bc2 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java @@ -254,7 +254,7 @@ abstract class AbstractElasticIndex implements Index { private Response performRequest( String method, String uri, @Nullable Object payload, Map params) throws IOException { - Request request = new Request(method, uri); + Request request = new Request(method, uri.startsWith("/") ? uri : "/" + uri); if (payload != null) { String payloadStr = payload instanceof String ? (String) payload : payload.toString(); request.setEntity(new NStringEntity(payloadStr, ContentType.APPLICATION_JSON)); diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java index 64f6252fb8..b52499be57 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticQueryAdapter.java @@ -32,7 +32,7 @@ public class ElasticQueryAdapter { ElasticQueryAdapter(ElasticVersion version) { this.ignoreUnmapped = version == ElasticVersion.V2_4; this.usePostV5Type = version.isV6(); - this.versionDiscoveryUrl = version.isV6() ? "%s*" : "%s*/_aliases"; + this.versionDiscoveryUrl = version.isV6() ? "/%s*" : "/%s*/_aliases"; switch (version) { case V5_6: diff --git a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java index 0a32886b51..9c1cf02e3c 100644 --- a/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java +++ b/gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticRestClientProvider.java @@ -107,7 +107,7 @@ class ElasticRestClientProvider implements Provider, LifecycleListen private ElasticVersion getVersion() throws ElasticException { try { - Response response = client.performRequest(new Request("GET", "")); + Response response = client.performRequest(new Request("GET", "/")); StatusLine statusLine = response.getStatusLine(); if (statusLine.getStatusCode() != HttpStatus.SC_OK) { throw new FailedToGetVersion(statusLine); From 62417b11f9a7b6c140cdab5e310f3708ba4d3fed Mon Sep 17 00:00:00 2001 From: Paladox none Date: Sun, 23 Sep 2018 21:00:36 +0000 Subject: [PATCH 09/14] Update jruby to 9.1.17 and asciidoctorj to v1.5.7 Since Bazel 0.16, the build is done with an embedded JRE using Java version 9. This causes "illegal reflective access operation" warnings during the documentation build, because the version of jruby (9.1.13) doesn't support Java 9. Support for Java 9 was added in jruby 9.1.14 [1]. The subsequent releases also include bug fixes (see [2], [3], [4]). Note that we don't upgrade to the latest (9.2.0). Also upgrade asciidoctorj to the latest release, 1.5.7. [1] http://jruby.org/2017/11/08/jruby-9-1-14-0.html [2] http://jruby.org/2017/12/07/jruby-9-1-15-0.html [3] http://jruby.org/2018/02/21/jruby-9-1-16-0.html [4] http://jruby.org/2018/04/23/jruby-9-1-17-0.html Bug: Issue 9766 Change-Id: I2f84612905761c4bdfddca807e92f54247ef6f6e --- WORKSPACE | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 3d477f2e31..e4bc3b472d 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -863,14 +863,14 @@ maven_jar( maven_jar( name = "asciidoctor", - artifact = "org.asciidoctor:asciidoctorj:1.5.6", - sha1 = "bb757d4b8b0f8438ce2ed781f6688cc6c01d9237", + artifact = "org.asciidoctor:asciidoctorj:1.5.7", + sha1 = "8e8c1d8fc6144405700dd8df3b177f2801ac5987", ) maven_jar( name = "jruby", - artifact = "org.jruby:jruby-complete:9.1.13.0", - sha1 = "8903bf42272062e87a7cbc1d98919e0729a9939f", + artifact = "org.jruby:jruby-complete:9.1.17.0", + sha1 = "76716d529710fc03d1d429b43e3cedd4419f78d4", ) maven_jar( From aee3bdc2d647ca2134b5406b43edc0d778280dac Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Wed, 26 Sep 2018 08:45:30 +0900 Subject: [PATCH 10/14] Set version to 2.14.14 Change-Id: I5e6ce63b23c72f3fc293940e94a00affed294e77 --- gerrit-acceptance-framework/pom.xml | 2 +- gerrit-extension-api/pom.xml | 2 +- gerrit-plugin-api/pom.xml | 2 +- gerrit-plugin-gwtui/pom.xml | 2 +- gerrit-war/pom.xml | 2 +- version.bzl | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gerrit-acceptance-framework/pom.xml b/gerrit-acceptance-framework/pom.xml index cfe17beadb..e1deabec4f 100644 --- a/gerrit-acceptance-framework/pom.xml +++ b/gerrit-acceptance-framework/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-acceptance-framework - 2.14.14-SNAPSHOT + 2.14.14 jar Gerrit Code Review - Acceptance Test Framework Framework for Gerrit's acceptance tests diff --git a/gerrit-extension-api/pom.xml b/gerrit-extension-api/pom.xml index 79c039838a..212759ed59 100644 --- a/gerrit-extension-api/pom.xml +++ b/gerrit-extension-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-extension-api - 2.14.14-SNAPSHOT + 2.14.14 jar Gerrit Code Review - Extension API API for Gerrit Extensions diff --git a/gerrit-plugin-api/pom.xml b/gerrit-plugin-api/pom.xml index 57b5d20722..92dc19e4f2 100644 --- a/gerrit-plugin-api/pom.xml +++ b/gerrit-plugin-api/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-api - 2.14.14-SNAPSHOT + 2.14.14 jar Gerrit Code Review - Plugin API API for Gerrit Plugins diff --git a/gerrit-plugin-gwtui/pom.xml b/gerrit-plugin-gwtui/pom.xml index 4f6bf7fe36..bb7e2ffd1a 100644 --- a/gerrit-plugin-gwtui/pom.xml +++ b/gerrit-plugin-gwtui/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-plugin-gwtui - 2.14.14-SNAPSHOT + 2.14.14 jar Gerrit Code Review - Plugin GWT UI Common Classes for Gerrit GWT UI Plugins diff --git a/gerrit-war/pom.xml b/gerrit-war/pom.xml index 995a6cbaad..0908db5eb5 100644 --- a/gerrit-war/pom.xml +++ b/gerrit-war/pom.xml @@ -2,7 +2,7 @@ 4.0.0 com.google.gerrit gerrit-war - 2.14.14-SNAPSHOT + 2.14.14 war Gerrit Code Review - WAR Gerrit WAR diff --git a/version.bzl b/version.bzl index 570113f115..3307e7bac1 100644 --- a/version.bzl +++ b/version.bzl @@ -2,4 +2,4 @@ # Used by :api_install and :api_deploy targets # when talking to the destination repository. # -GERRIT_VERSION = "2.14.14-SNAPSHOT" +GERRIT_VERSION = "2.14.14" From 3727dba665b67bec4ce864fa8e7434374b10a4f3 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 27 Sep 2018 12:57:37 +0900 Subject: [PATCH 11/14] concept-changes: Fix 'invalid reference: submit-strategy' warning Change-Id: I7b81c44e828a2017936fd63b593a60c3059cdb1e --- Documentation/concept-changes.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/concept-changes.txt b/Documentation/concept-changes.txt index 6bd484b939..6c8d912f0d 100644 --- a/Documentation/concept-changes.txt +++ b/Documentation/concept-changes.txt @@ -55,7 +55,7 @@ are not required to review it. |An optional topic. |Strategy -|The <> for the change. +|The <> for the change. |Code Review |Displays the Code Review status for the change. From 2919b01659779e0c7c15b4b43da73f6a4fdaadcc Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 27 Sep 2018 12:59:12 +0900 Subject: [PATCH 12/14] concept-changes: Fix 'invalid reference: topic' warning Change-Id: Ibe05d71675b4d9f05a0288b6796e6afae6352091 --- Documentation/concept-changes.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/concept-changes.txt b/Documentation/concept-changes.txt index 6c8d912f0d..f24d5154be 100644 --- a/Documentation/concept-changes.txt +++ b/Documentation/concept-changes.txt @@ -84,10 +84,10 @@ listed next to the change message. These related changes are grouped together in several categories, including: * Relation Chain. These changes are related by parent-child relationships, - regardless of <>. + regardless of <>. * Merge Conflicts. These are changes in which there is a merge conflict with the current change. -* Submitted Together. These are changes that share the same <>. +* Submitted Together. These are changes that share the same <>. An arrow indicates the change you are currently viewing. From ab91ba1723c34b63666b02d457e2a64aa0c68192 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 27 Sep 2018 05:46:04 +0000 Subject: [PATCH 13/14] Replace download links for releases index page Since the migration to Jekyll, the homepage URL: https://www.gerritcodereview.com/download/ does not redirect to: https://gerrit-releases.storage.googleapis.com/ Change the index page to directly use the destination URL instead of the homepage URL. Bug: Issue 9630 Change-Id: Idc7a1d07f60b4c2685b7eb9f59983b714af7d36a --- website/releases/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/releases/index.html b/website/releases/index.html index 582b4959fa..ab3fcd68c7 100644 --- a/website/releases/index.html +++ b/website/releases/index.html @@ -44,7 +44,7 @@ function(data) { var doc = document; var frg = doc.createDocumentFragment(); var rx = /^gerrit(?:-full)?-([0-9.]+(?:-rc[0-9]+)?)[.]war/; - var dl = 'https://www.gerritcodereview.com/download/'; + var dl = 'https://gerrit-releases.storage.googleapis.com/'; var docs = 'https://gerrit-documentation.storage.googleapis.com/'; var src = 'https://gerrit.googlesource.com/gerrit/+/' From ba8186d5dc205dff0d2236eda85055bf696a3dd8 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 27 Sep 2018 14:28:58 +0900 Subject: [PATCH 14/14] AsyncReceiveCommits#onPreReceive: Early exit when commands already processed Change-Id: Ife5fecba72713ad7c5ab7847ff9a613d1602f474 Helped-by: David Ostrovsky --- .../gerrit/server/git/receive/AsyncReceiveCommits.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java index 48c0713bde..bed9bd4e1b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java @@ -256,6 +256,11 @@ public class AsyncReceiveCommits implements PreReceiveHook { @Override public void onPreReceive(ReceivePack rp, Collection commands) { + if (commands.stream().anyMatch(c -> c.getResult() != Result.NOT_ATTEMPTED)) { + // Stop processing when command was already processed by previously invoked + // pre-receive hooks + return; + } Worker w = new Worker(commands); try { w.progress.waitFor(