From e22bfc3afa60ca691d6a91ec34010f01c802bfea Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 10 Mar 2020 14:07:10 +0100 Subject: [PATCH 1/5] Support /COMMIT_MSG for change edit REST endpoints The change edit REST endpoints to get and set file contents failed with "409 Conflict: Invalid path: /COMMIT_MSG" if the magic file "/COMMIT_MSG" is used to update the commit message. This is what is currently used by PolyGerrit if you have a change edit, open the diff via for the commit message and then try to edit and save the file content. Updating the commit message of change edits from the change screen works, because that is using a dedicated REST endpoint to update the commit message. This fix makes the change edit REST endpoints aware of the magic file "/COMMIT_MSG" by making them forward the request to the dedicated REST endpoints to get/set the commit message. Alternatively PolyGerrit could be changed to directly use these REST endpoints when the commit message is edited. Bug: Issue 11706 Signed-off-by: Edwin Kempin Change-Id: I5c2ef8dbabe72e04ed61606e7b923d877efa3a23 --- .../server/restapi/change/ChangeEdits.java | 34 +++++++++++++++---- .../gerrit/acceptance/edit/ChangeEditIT.java | 12 +++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java index cabb30df7b..d487da039e 100644 --- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java +++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java @@ -14,7 +14,10 @@ package com.google.gerrit.server.restapi.change; +import static java.nio.charset.StandardCharsets.UTF_8; + import com.google.common.base.Strings; +import com.google.common.io.ByteStreams; import com.google.gerrit.extensions.common.DiffWebLinkInfo; import com.google.gerrit.extensions.common.EditInfo; import com.google.gerrit.extensions.common.Input; @@ -36,6 +39,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.WebLinks; @@ -120,7 +124,7 @@ public class ChangeEdits implements ChildCollection apply(ChangeResource resource, IdString id, Put.Input input) throws AuthException, ResourceConflictException, IOException, OrmException, - PermissionBackendException { + PermissionBackendException, BadRequestException { putEdit.apply(resource, id.get(), input.content); return Response.none(); } @@ -277,23 +281,34 @@ public class ChangeEdits implements ChildCollection apply(ChangeEditResource rsrc, Input input) throws AuthException, ResourceConflictException, IOException, OrmException, - PermissionBackendException { + PermissionBackendException, BadRequestException { return apply(rsrc.getChangeResource(), rsrc.getPath(), input.content); } public Response apply(ChangeResource rsrc, String path, RawInput newContent) throws ResourceConflictException, AuthException, IOException, OrmException, - PermissionBackendException { + PermissionBackendException, BadRequestException { + if (Patch.COMMIT_MSG.equals(path)) { + EditMessage.Input editCommitMessageInput = new EditMessage.Input(); + editCommitMessageInput.message = + new String(ByteStreams.toByteArray(newContent.getInputStream()), UTF_8); + return Response.ok(editMessage.apply(rsrc, editCommitMessageInput)); + } if (Strings.isNullOrEmpty(path) || path.charAt(0) == '/') { throw new ResourceConflictException("Invalid path: " + path); } @@ -347,6 +362,7 @@ public class ChangeEdits implements ChildCollection { private final FileContentUtil fileContentUtil; private final ProjectCache projectCache; + private final GetMessage getMessage; @Option( name = "--base", @@ -355,14 +371,20 @@ public class ChangeEdits implements ChildCollection apply(ChangeEditResource rsrc) throws IOException { + public Response apply(ChangeEditResource rsrc) + throws AuthException, IOException, OrmException { try { + if (Patch.COMMIT_MSG.equals(rsrc.getPath())) { + return Response.ok(getMessage.apply(rsrc.getChangeResource())); + } + ChangeEdit edit = rsrc.getChangeEdit(); return Response.ok( fileContentUtil.getContent( diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 47f4a8f01b..7a67fba910 100644 --- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -53,6 +53,7 @@ import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.reviewdb.client.Patch; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -320,6 +321,17 @@ public class ChangeEditIT extends AbstractDaemonTest { ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_NEW); } + @Test + public void updateCommitMessageByEditingMagicCommitMsgFile() throws Exception { + createEmptyEditFor(changeId); + gApi.changes() + .id(changeId) + .edit() + .modifyFile(Patch.COMMIT_MSG, RawInputUtil.create("Foo Bar".getBytes(UTF_8))); + assertThat(getEdit(changeId)).isPresent(); + ensureSameBytes(getFileContentOfEdit(changeId, Patch.COMMIT_MSG), "Foo Bar\n".getBytes(UTF_8)); + } + @Test @TestProjectInput(createEmptyCommit = false) public void updateRootCommitMessage() throws Exception { From 154cb56d68f462205140c2414c75605490816d35 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Thu, 4 Jun 2020 13:07:58 -0400 Subject: [PATCH 2/5] ElasticContainer: Upgrade V7_7 to elasticsearch 7.7.1 Change-Id: Ibad2867e70d9ae960428bed7c127bc35f93b73b5 --- javatests/com/google/gerrit/elasticsearch/ElasticContainer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index 15094fdd1e..e436db5922 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -59,7 +59,7 @@ public class ElasticContainer extends ElasticsearchContainer { case V7_6: return "blacktop/elasticsearch:7.6.2"; case V7_7: - return "blacktop/elasticsearch:7.7.0"; + return "blacktop/elasticsearch:7.7.1"; } throw new IllegalStateException("No tests for version: " + version.name()); } From b7602dd1aa6c6cc31c600ed0bd8e24f4b23ea58f Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Thu, 4 Jun 2020 22:35:31 +0900 Subject: [PATCH 3/5] Upgrade elasticsearch-rest-client to 7.7.1 Change-Id: I16784371a8487cb05e705d7edf4c075fb6392ed0 --- tools/nongoogle.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl index d4f67366e2..b88c00aee5 100644 --- a/tools/nongoogle.bzl +++ b/tools/nongoogle.bzl @@ -94,8 +94,8 @@ def declare_nongoogle_deps(): # and httpasyncclient as necessary. maven_jar( name = "elasticsearch-rest-client", - artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.7.0", - sha1 = "5fc25eec3940bc0e9b0ffddcf50554a609e9db8e", + artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.7.1", + sha1 = "6d44a8e35c11df6883747200bcf46f476a1782b8", ) maven_jar( From 40e757d48f73f0416e22ac08112c77409818342b Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Thu, 4 Jun 2020 12:57:49 -0400 Subject: [PATCH 4/5] ElasticContainer: Upgrade V6_8 to elasticsearch 6.8.10 Change-Id: I208764b373bc46957e79a5c887b63ac28c79ff6f --- javatests/com/google/gerrit/elasticsearch/ElasticContainer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java index e436db5922..c20650f381 100644 --- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java +++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java @@ -43,7 +43,7 @@ public class ElasticContainer extends ElasticsearchContainer { case V6_7: return "blacktop/elasticsearch:6.7.2"; case V6_8: - return "blacktop/elasticsearch:6.8.9"; + return "blacktop/elasticsearch:6.8.10"; case V7_0: return "blacktop/elasticsearch:7.0.1"; case V7_1: From 8a632a56b64aee0c0d41f5e0780230bc028e859d Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Fri, 5 Jun 2020 09:57:59 +0900 Subject: [PATCH 5/5] =?UTF-8?q?Remove=20Hugo=20Ar=C3=A8s=20from=20develope?= =?UTF-8?q?rs=20section=20in=20pom.xml=20files?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a follow-up to [1] where Hugo was moved to the "Former" section of the maintainers list. All other former maintainers are already removed from the developers section. [1] https://gerrit-review.googlesource.com/c/homepage/+/266394 Change-Id: I2b1220db82f0a504cf45d2d328fd0347568f391a --- tools/maven/gerrit-acceptance-framework_pom.xml | 3 --- tools/maven/gerrit-extension-api_pom.xml | 3 --- tools/maven/gerrit-plugin-api_pom.xml | 3 --- tools/maven/gerrit-war_pom.xml | 3 --- 4 files changed, 12 deletions(-) diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml index 96dc17ba44..7278525e41 100644 --- a/tools/maven/gerrit-acceptance-framework_pom.xml +++ b/tools/maven/gerrit-acceptance-framework_pom.xml @@ -43,9 +43,6 @@ Han-Wen Nienhuys - - Hugo Arès - Luca Milanesio diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml index 38c3100fc4..7c59b09f52 100644 --- a/tools/maven/gerrit-extension-api_pom.xml +++ b/tools/maven/gerrit-extension-api_pom.xml @@ -43,9 +43,6 @@ Han-Wen Nienhuys - - Hugo Arès - Luca Milanesio diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml index 6e58782c70..5ecae7e2d0 100644 --- a/tools/maven/gerrit-plugin-api_pom.xml +++ b/tools/maven/gerrit-plugin-api_pom.xml @@ -43,9 +43,6 @@ Han-Wen Nienhuys - - Hugo Arès - Luca Milanesio diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml index 59b5685c5e..7427c08393 100644 --- a/tools/maven/gerrit-war_pom.xml +++ b/tools/maven/gerrit-war_pom.xml @@ -43,9 +43,6 @@ Han-Wen Nienhuys - - Hugo Arès - Luca Milanesio