Merge "GetRelated: Add some debug logs"
diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt
index c97e4a4..036dfbf 100644
--- a/Documentation/dev-bazel.txt
+++ b/Documentation/dev-bazel.txt
@@ -342,8 +342,14 @@
=== Elasticsearch
Successfully running the Elasticsearch tests requires Docker, and
-may require setting the local
-link:https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html[virtual memory,role=external,window=_blank].
+may require setting the local virtual memory on
+link:https://www.elastic.co/guide/en/elasticsearch/reference/current/vm-max-map-count.html[linux,role=external,window=_blank] and
+link:https://www.elastic.co/guide/en/elasticsearch/reference/current/docker.html#_set_vm_max_map_count_to_at_least_262144[macOS,role=external,window=_blank].
+
+On macOS, if using link:https://docs.docker.com/docker-for-mac/[Docker Desktop,role=external,window=_blank],
+the effective memory value can be set in the Preferences, under the Advanced tab.
+The default value usually does not suffice and is causing premature container exits.
+That default is currently 2 GB and should be set to at least 5 (GB).
If Docker is not available, the Elasticsearch tests will be skipped.
Note that Bazel currently does not show
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index dcd8121..d5abbe3 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -1457,6 +1457,10 @@
Reverts a change.
+The subject of the newly created change will be
+'Revert "<subject-of-reverted-change>"'. If the subject of the change reverted is
+above 63 characters, it will be cut down to 59 characters with "..." in the end.
+
The request body does not need to include a link:#revert-input[
RevertInput] entity if no review comment is added.
@@ -1515,7 +1519,9 @@
Creates open revert changes for all of the changes of a certain submission.
-The subject of each revert change will be "Revert <subject-of-reverted-change".
+The subject of each revert change will be 'Revert "<subject-of-reverted-change"'.
+If the subject is above 63 characters, the subject will be cut to 59 characters
+with "..." in the end.
Details for the revert can be specified in the request body inside a link:#revert-input[
RevertInput] The topic of all created revert changes will be
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
index 25eb7a8..6bd4b73 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeEditApi.java
@@ -150,7 +150,22 @@
* @param newContent the desired content of the file
* @throws RestApiException if the content of the file couldn't be modified
*/
- void modifyFile(String filePath, RawInput newContent) throws RestApiException;
+ default void modifyFile(String filePath, RawInput newContent) throws RestApiException {
+ FileContentInput input = new FileContentInput();
+ input.content = newContent;
+ modifyFile(filePath, input);
+ }
+
+ /**
+ * Modify the contents of the specified file of the change edit. If no content is provided, the
+ * content of the file is erased but the file isn't deleted. If the change edit doesn't exist, it
+ * will be created based on the current patch set of the change.
+ *
+ * @param filePath the path of the file which should be modified
+ * @param input the desired content of the file
+ * @throws RestApiException if the content of the file couldn't be modified
+ */
+ void modifyFile(String filePath, FileContentInput input) throws RestApiException;
/**
* Deletes the specified file from the change edit. If the change edit doesn't exist, it will be
@@ -235,7 +250,7 @@
}
@Override
- public void modifyFile(String filePath, RawInput newContent) throws RestApiException {
+ public void modifyFile(String filePath, FileContentInput input) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/changes/FileContentInput.java b/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
new file mode 100644
index 0000000..93c253d
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/FileContentInput.java
@@ -0,0 +1,23 @@
+// Copyright (C) 2019 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.api.changes;
+
+import com.google.gerrit.extensions.restapi.DefaultInput;
+import com.google.gerrit.extensions.restapi.RawInput;
+
+/** Content to be added to a file (new or existing) via change edit. */
+public class FileContentInput {
+ @DefaultInput public RawInput content;
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index fad3f91..f390ebc 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -114,10 +114,12 @@
import com.google.gerrit.server.change.ChangeFinder;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.group.GroupAuditService;
+import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.PerformanceLogContext;
import com.google.gerrit.server.logging.PerformanceLogger;
import com.google.gerrit.server.logging.RequestId;
import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.logging.TraceContext.TraceTimer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -718,7 +720,10 @@
ViewData viewData,
ETagView<RestResource> view,
RestResource rsrc) {
- try {
+ try (TraceTimer ignored =
+ TraceContext.newTimer(
+ "RestApiServlet#getEtagWithRetry:view",
+ Metadata.builder().restViewName(getViewName(viewData)).build())) {
return invokeRestEndpointWithRetry(
req,
traceContext,
@@ -733,7 +738,10 @@
private String getEtagWithRetry(
HttpServletRequest req, TraceContext traceContext, RestResource.HasETag rsrc) {
- try {
+ try (TraceTimer ignored =
+ TraceContext.newTimer(
+ "RestApiServlet#getEtagWithRetry:resource",
+ Metadata.builder().restViewName(rsrc.getClass().getSimpleName()).build())) {
return invokeRestEndpointWithRetry(
req,
traceContext,
@@ -1735,43 +1743,47 @@
private long handleException(
TraceContext traceContext, Throwable err, HttpServletRequest req, HttpServletResponse res)
throws IOException {
- logger.atSevere().withCause(err).log("Error in %s %s", req.getMethod(), uriForLogging(req));
- if (!res.isCommitted()) {
- res.reset();
- traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- ImmutableList<String> userMessages =
- globals.exceptionHooks.stream()
- .map(h -> h.getUserMessage(err))
- .filter(Optional::isPresent)
- .map(Optional::get)
- .collect(toImmutableList());
+ if (res.isCommitted()) {
+ logger.atSevere().withCause(err).log(
+ "Error in %s %s, response already committed", req.getMethod(), uriForLogging(req));
+ return 0;
+ }
- Optional<Integer> statusCode =
- globals.exceptionHooks.stream()
- .map(h -> h.getStatusCode(err))
- .filter(Optional::isPresent)
- .map(Optional::get)
- .findFirst();
- if (statusCode.isPresent() && statusCode.get() < 400) {
- StringBuilder msg = new StringBuilder();
- if (userMessages.size() == 1) {
- msg.append(userMessages.get(0));
- } else {
- userMessages.forEach(m -> msg.append("\n* ").append(m));
- }
+ res.reset();
+ traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
+ ImmutableList<String> userMessages =
+ globals.exceptionHooks.stream()
+ .map(h -> h.getUserMessage(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .collect(toImmutableList());
- res.setStatus(statusCode.get());
- logger.atFinest().withCause(err).log("REST call finished: %d", statusCode.get().intValue());
- return replyText(req, res, true, msg.toString());
- }
-
- StringBuilder msg = new StringBuilder("Internal server error");
- if (!userMessages.isEmpty()) {
+ Optional<Integer> statusCode =
+ globals.exceptionHooks.stream()
+ .map(h -> h.getStatusCode(err))
+ .filter(Optional::isPresent)
+ .map(Optional::get)
+ .findFirst();
+ if (statusCode.isPresent() && statusCode.get() < 400) {
+ StringBuilder msg = new StringBuilder();
+ if (userMessages.size() == 1) {
+ msg.append(userMessages.get(0));
+ } else {
userMessages.forEach(m -> msg.append("\n* ").append(m));
}
- return replyError(req, res, statusCode.orElse(SC_INTERNAL_SERVER_ERROR), msg.toString(), err);
+
+ res.setStatus(statusCode.get());
+ logger.atFinest().withCause(err).log("REST call finished: %d", statusCode.get().intValue());
+ return replyText(req, res, true, msg.toString());
}
- return 0;
+
+ logger.atSevere().withCause(err).log("Error in %s %s", req.getMethod(), uriForLogging(req));
+
+ StringBuilder msg = new StringBuilder("Internal server error");
+ if (!userMessages.isEmpty()) {
+ userMessages.forEach(m -> msg.append("\n* ").append(m));
+ }
+ return replyError(req, res, statusCode.orElse(SC_INTERNAL_SERVER_ERROR), msg.toString(), err);
}
private static String uriForLogging(HttpServletRequest req) {
diff --git a/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
index 7f0feba..1b0f0c5 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeEditApiImpl.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
+import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
import com.google.gerrit.extensions.client.ChangeEditDetailOption;
import com.google.gerrit.extensions.common.EditInfo;
@@ -24,7 +25,6 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.RawInput;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -200,9 +200,9 @@
}
@Override
- public void modifyFile(String filePath, RawInput newContent) throws RestApiException {
+ public void modifyFile(String filePath, FileContentInput input) throws RestApiException {
try {
- changeEditsPut.apply(changeResource, filePath, newContent);
+ changeEditsPut.apply(changeResource, filePath, input);
} catch (Exception e) {
throw asRestApiException("Cannot modify file of change edit", e);
}
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 476037b..393ccb7 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -146,12 +146,14 @@
revertCommitBuilder.setCommitter(authorIdent);
Change changeToRevert = notes.getChange();
+ String subject = changeToRevert.getSubject();
+ if (subject.length() > 63) {
+ subject = subject.substring(0, 59) + "...";
+ }
if (message == null) {
message =
MessageFormat.format(
- ChangeMessages.get().revertChangeDefaultMessage,
- changeToRevert.getSubject(),
- patch.commitId().name());
+ ChangeMessages.get().revertChangeDefaultMessage, subject, patch.commitId().name());
}
if (generatedChangeId != null) {
revertCommitBuilder.setMessage(ChangeIdUtil.insertId(message, generatedChangeId, true));
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
index 1808fa6..e220f35 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeEdits.java
@@ -19,6 +19,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.common.DiffWebLinkInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.Input;
@@ -109,7 +110,7 @@
*/
@Singleton
public static class Create
- implements RestCollectionCreateView<ChangeResource, ChangeEditResource, Put.Input> {
+ implements RestCollectionCreateView<ChangeResource, ChangeEditResource, FileContentInput> {
private final Put putEdit;
@Inject
@@ -118,9 +119,9 @@
}
@Override
- public Response<?> apply(ChangeResource resource, IdString id, Put.Input input)
+ public Response<?> apply(ChangeResource resource, IdString id, FileContentInput input)
throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
- putEdit.apply(resource, id.get(), input.content);
+ putEdit.apply(resource, id.get(), input);
return Response.none();
}
}
@@ -267,11 +268,7 @@
/** Put handler that is activated when PUT request is called on collection element. */
@Singleton
- public static class Put implements RestModifyView<ChangeEditResource, Put.Input> {
- public static class Input {
- @DefaultInput public RawInput content;
- }
-
+ public static class Put implements RestModifyView<ChangeEditResource, FileContentInput> {
private final ChangeEditModifier editModifier;
private final GitRepositoryManager repositoryManager;
@@ -282,17 +279,18 @@
}
@Override
- public Response<?> apply(ChangeEditResource rsrc, Input input)
+ public Response<?> apply(ChangeEditResource rsrc, FileContentInput input)
throws AuthException, ResourceConflictException, IOException, PermissionBackendException {
- return apply(rsrc.getChangeResource(), rsrc.getPath(), input.content);
+ return apply(rsrc.getChangeResource(), rsrc.getPath(), input);
}
- public Response<?> apply(ChangeResource rsrc, String path, RawInput newContent)
+ public Response<?> apply(ChangeResource rsrc, String path, FileContentInput input)
throws ResourceConflictException, AuthException, IOException, PermissionBackendException {
if (Strings.isNullOrEmpty(path) || path.charAt(0) == '/') {
throw new ResourceConflictException("Invalid path: " + path);
}
+ RawInput newContent = input.content;
try (Repository repository = repositoryManager.openRepository(rsrc.getProject())) {
editModifier.modifyFile(repository, rsrc.getNotes(), path, newContent);
} catch (InvalidChangeOperationException e) {
diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
index df4c83e..89b265f 100644
--- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
+++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java
@@ -27,6 +27,7 @@
import com.google.gerrit.entities.ChangeMessage;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Project.NameKey;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.CherryPickInput;
@@ -34,6 +35,7 @@
import com.google.gerrit.extensions.api.changes.RevertInput;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.RevertSubmissionInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -185,6 +187,22 @@
}
List<ChangeData> changeDatas = queryProvider.get().bySubmissionId(submissionId);
+ checkPermissionsForAllChanges(changeResource, changeDatas);
+ input.topic = createTopic(input.topic, submissionId);
+ return Response.ok(revertSubmission(changeDatas, input));
+ }
+
+ private String createTopic(String topic, String submissionId) {
+ if (topic == null) {
+ return String.format(
+ "revert-%s-%s", submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase());
+ }
+ return topic;
+ }
+
+ private void checkPermissionsForAllChanges(
+ ChangeResource changeResource, List<ChangeData> changeDatas)
+ throws IOException, AuthException, PermissionBackendException, ResourceConflictException {
for (ChangeData changeData : changeDatas) {
Change change = changeData.change();
@@ -203,23 +221,122 @@
"current patch set %s of change %s not found",
change.currentPatchSetId(), change.currentPatchSetId()));
}
-
- if (input.topic == null) {
- input.topic =
- String.format(
- "revert-%s-%s", submissionId, RandomStringUtils.randomAlphabetic(10).toUpperCase());
- }
-
- return Response.ok(revertSubmission(changeDatas, input));
}
private RevertSubmissionInfo revertSubmission(
List<ChangeData> changeData, RevertInput revertInput)
throws RestApiException, IOException, UpdateException, PermissionBackendException,
NoSuchProjectException, ConfigInvalidException, StorageException {
+
Multimap<BranchNameKey, ChangeData> changesPerProjectAndBranch = ArrayListMultimap.create();
changeData.stream().forEach(c -> changesPerProjectAndBranch.put(c.change().getDest(), c));
+ cherryPickInput = createCherryPickInput(revertInput);
+ for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
+ cherryPickInput.base = null;
+ Project.NameKey project = projectAndBranch.project();
+ cherryPickInput.destination = projectAndBranch.branch();
+ Collection<ChangeData> changesInProjectAndBranch =
+ changesPerProjectAndBranch.get(projectAndBranch);
+
+ // Sort the changes topologically.
+ Iterator<PatchSetData> sortedChangesInProjectAndBranch =
+ sorter.sort(changesInProjectAndBranch).iterator();
+
+ Set<ObjectId> commitIdsInProjectAndBranch =
+ changesInProjectAndBranch.stream()
+ .map(c -> c.currentPatchSet().commitId())
+ .collect(Collectors.toSet());
+
+ revertAllChangesInProjectAndBranch(
+ revertInput, project, sortedChangesInProjectAndBranch, commitIdsInProjectAndBranch);
+ }
+ results.sort(Comparator.comparing(c -> c.revertOf));
+ RevertSubmissionInfo revertSubmissionInfo = new RevertSubmissionInfo();
+ revertSubmissionInfo.revertChanges = results;
+ return revertSubmissionInfo;
+ }
+
+ private void revertAllChangesInProjectAndBranch(
+ RevertInput revertInput,
+ NameKey project,
+ Iterator<PatchSetData> sortedChangesInProjectAndBranch,
+ Set<ObjectId> commitIdsInProjectAndBranch)
+ throws IOException, RestApiException, UpdateException, PermissionBackendException,
+ NoSuchProjectException, ConfigInvalidException {
+
+ String groupName = null;
+ while (sortedChangesInProjectAndBranch.hasNext()) {
+ ChangeNotes changeNotes = sortedChangesInProjectAndBranch.next().data().notes();
+ if (cherryPickInput.base == null) {
+ cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
+ }
+
+ revertInput.message = getMessage(revertInput, changeNotes);
+ if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
+ // This is the code in case this is the first revert of this project + branch, and the
+ // revert would be on top of the change being reverted.
+ craeteNormalRevert(revertInput, changeNotes);
+ groupName = cherryPickInput.base;
+ } else {
+ // This is the code in case this is the second revert (or more) of this project + branch.
+ if (groupName == null) {
+ groupName = cherryPickInput.base;
+ }
+ createCherryPickedRevert(revertInput, project, groupName, changeNotes);
+ }
+ }
+ }
+
+ private void createCherryPickedRevert(
+ RevertInput revertInput, NameKey project, String groupName, ChangeNotes changeNotes)
+ throws IOException, ConfigInvalidException, UpdateException, RestApiException {
+ ObjectId revCommitId =
+ commitUtil.createRevertCommit(revertInput.message, changeNotes, user.get());
+ // TODO (paiking): As a future change, the revert should just be done directly on the
+ // target rather than just creating a commit and then cherry-picking it.
+ cherryPickInput.message = revertInput.message;
+ ObjectId generatedChangeId = Change.generateChangeId();
+ Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
+ // TODO (paiking): In the the future, the timestamp should be the same for all the revert
+ // changes.
+ try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.nowTs())) {
+ bu.setNotify(
+ notifyResolver.resolve(
+ firstNonNull(cherryPickInput.notify, NotifyHandling.ALL),
+ cherryPickInput.notifyDetails));
+ bu.addOp(
+ changeNotes.getChange().getId(),
+ new CreateCherryPickOp(
+ revCommitId,
+ revertInput.topic,
+ generatedChangeId,
+ cherryPickRevertChangeId,
+ groupName));
+ bu.addOp(changeNotes.getChange().getId(), new PostRevertedMessageOp(generatedChangeId));
+ bu.addOp(
+ cherryPickRevertChangeId,
+ new NotifyOp(changeNotes.getChange(), cherryPickRevertChangeId));
+
+ bu.execute();
+ }
+ }
+
+ private void craeteNormalRevert(RevertInput revertInput, ChangeNotes changeNotes)
+ throws IOException, RestApiException, UpdateException, PermissionBackendException,
+ NoSuchProjectException, ConfigInvalidException {
+ ChangeInfo revertChangeInfo =
+ revert.apply(changeResourceFactory.create(changeNotes, user.get()), revertInput).value();
+ results.add(revertChangeInfo);
+ cherryPickInput.base =
+ changeNotesFactory
+ .createChecked(Change.id(revertChangeInfo._number))
+ .getCurrentPatchSet()
+ .commitId()
+ .getName();
+ }
+
+ private CherryPickInput createCherryPickInput(RevertInput revertInput) {
cherryPickInput = new CherryPickInput();
// To create a revert change, we create a revert commit that is then cherry-picked. The revert
// change is created for the cherry-picked commit. Notifications are sent only for this change,
@@ -228,91 +345,14 @@
cherryPickInput.notifyDetails = revertInput.notifyDetails;
cherryPickInput.parent = 1;
cherryPickInput.keepReviewers = true;
-
- for (BranchNameKey projectAndBranch : changesPerProjectAndBranch.keySet()) {
- String groupName = null;
- Project.NameKey project = projectAndBranch.project();
- cherryPickInput.destination = projectAndBranch.branch();
- Collection<ChangeData> changesInProjectAndBranch =
- changesPerProjectAndBranch.get(projectAndBranch);
-
- // Sort the changes topologically.
- Iterator<PatchSetData> sortedChangesInProject =
- sorter.sort(changesInProjectAndBranch).iterator();
-
- Set<ObjectId> commitIdsInProjectAndBranch =
- changesInProjectAndBranch.stream()
- .map(c -> c.currentPatchSet().commitId())
- .collect(Collectors.toSet());
-
- while (sortedChangesInProject.hasNext()) {
- ChangeNotes changeNotes = sortedChangesInProject.next().data().notes();
- if (cherryPickInput.base == null) {
- cherryPickInput.base = getBase(changeNotes, commitIdsInProjectAndBranch).name();
- }
-
- revertInput.message = getMessage(revertInput, changeNotes);
- // This is the code in case this is the first revert of this project + branch, and the
- // revert would be on top of the change being reverted.
- if (cherryPickInput.base.equals(changeNotes.getCurrentPatchSet().commitId().getName())) {
- ChangeInfo revertChangeInfo =
- revert
- .apply(changeResourceFactory.create(changeNotes, user.get()), revertInput)
- .value();
- results.add(revertChangeInfo);
- cherryPickInput.base =
- changeNotesFactory
- .createChecked(Change.id(revertChangeInfo._number))
- .getCurrentPatchSet()
- .commitId()
- .getName();
- groupName = cherryPickInput.base;
- } else {
- // This is the code in case this is the second revert (or more) of this project + branch.
- ObjectId revCommitId =
- commitUtil.createRevertCommit(revertInput.message, changeNotes, user.get());
- // TODO (paiking): As a future change, the revert should just be done directly on the
- // target rather than just creating a commit and then cherry-picking it.
- cherryPickInput.message = revertInput.message;
- ObjectId generatedChangeId = Change.generateChangeId();
- Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId());
- if (groupName == null) {
- groupName = cherryPickInput.base;
- }
- // TODO (paiking): In the the future, the timestamp should be the same for all the revert
- // changes.
- try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.nowTs())) {
- bu.setNotify(
- notifyResolver.resolve(
- firstNonNull(cherryPickInput.notify, NotifyHandling.ALL),
- cherryPickInput.notifyDetails));
- bu.addOp(
- changeNotes.getChange().getId(),
- new CreateCherryPickOp(
- revCommitId,
- revertInput.topic,
- generatedChangeId,
- cherryPickRevertChangeId,
- groupName));
- bu.addOp(changeNotes.getChange().getId(), new PostRevertedMessageOp(generatedChangeId));
- bu.addOp(
- cherryPickRevertChangeId,
- new NotifyOp(changeNotes.getChange(), cherryPickRevertChangeId));
-
- bu.execute();
- }
- }
- }
- cherryPickInput.base = null;
- }
- results.sort(Comparator.comparing(c -> c.revertOf));
- RevertSubmissionInfo revertSubmissionInfo = new RevertSubmissionInfo();
- revertSubmissionInfo.revertChanges = results;
- return revertSubmissionInfo;
+ return cherryPickInput;
}
private String getMessage(RevertInput revertInput, ChangeNotes changeNotes) {
String subject = changeNotes.getChange().getSubject();
+ if (subject.length() > 63) {
+ subject = subject.substring(0, 59) + "...";
+ }
if (revertInput.message == null) {
return MessageFormat.format(
ChangeMessages.get().revertChangeDefaultMessage,
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 64c4578..79ae43b 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -16,7 +16,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkState;
import static java.util.Comparator.comparing;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toSet;
@@ -459,11 +458,17 @@
try {
ChangeSet indexBackedChangeSet =
mergeSuperSet.setMergeOpRepoManager(orm).completeChangeSet(change, caller);
- checkState(
- indexBackedChangeSet.ids().contains(change.getId()),
- "change %s missing from %s",
- change.getId(),
- indexBackedChangeSet);
+ if (!indexBackedChangeSet.ids().contains(change.getId())) {
+ // indexBackedChangeSet contains only open changes, if the change is missing in this set
+ // it might be that the change was concurrently submitted in the meantime.
+ change = changeDataFactory.create(change).reloadChange();
+ if (!change.isNew()) {
+ throw new ResourceConflictException("change is " + ChangeUtil.status(change));
+ }
+ throw new IllegalStateException(
+ String.format("change %s missing from %s", change.getId(), indexBackedChangeSet));
+ }
+
if (indexBackedChangeSet.furtherHiddenChanges()) {
throw new AuthException(
"A change to be submitted with " + change.getId() + " is not visible");
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index bb69453..e13cab1 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -481,9 +481,9 @@
if (!traceContext.isTracing()) {
String traceId = "retry-on-failure-" + new RequestId();
traceContext.addTag(RequestId.Type.TRACE_ID, traceId).forceLogging();
- opts.onAutoTrace().ifPresent(c -> c.accept(traceId));
logger.atFine().withCause(t).log(
"AutoRetry: %s failed, retry with tracing enabled", actionName);
+ opts.onAutoTrace().ifPresent(c -> c.accept(traceId));
metrics.autoRetryCount.increment(actionType, actionName, cause);
return true;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
index f1ea319..7703b1c 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java
@@ -284,6 +284,27 @@
}
@Test
+ public void revertChangeWithLongSubject() throws Exception {
+ String changeTitle =
+ "This change has a very long title and therefore it will be cut to 50 characters when the"
+ + " revert change will revert this change";
+ String result = createChange(changeTitle, "a.txt", "message").getChangeId();
+ gApi.changes().id(result).current().review(ReviewInput.approve());
+ gApi.changes().id(result).current().submit();
+ RevertInput revertInput = new RevertInput();
+ ChangeInfo revertChange = gApi.changes().id(result).revert(revertInput).get();
+ assertThat(revertChange.subject)
+ .isEqualTo(String.format("Revert \"%s...\"", changeTitle.substring(0, 59)));
+ assertThat(gApi.changes().id(revertChange.id).current().commit(false).message)
+ .isEqualTo(
+ String.format(
+ "Revert \"%s...\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n",
+ changeTitle.substring(0, 59),
+ gApi.changes().id(result).get().currentRevision,
+ revertChange.changeId));
+ }
+
+ @Test
public void revertNotifications() throws Exception {
PushOneCommit.Result r = createChange();
gApi.changes().id(r.getChangeId()).addReviewer(user.email());
@@ -705,6 +726,28 @@
}
@Test
+ public void revertSubmissionRevertsChangeWithLongSubject() throws Exception {
+ String changeTitle =
+ "This change has a very long title and therefore it will be cut to 50 characters when the"
+ + " revert change will revert this change";
+ String result = createChange(changeTitle, "a.txt", "message").getChangeId();
+ gApi.changes().id(result).current().review(ReviewInput.approve());
+ gApi.changes().id(result).current().submit();
+ RevertInput revertInput = new RevertInput();
+ ChangeInfo revertChange =
+ gApi.changes().id(result).revertSubmission(revertInput).revertChanges.get(0);
+ assertThat(revertChange.subject)
+ .isEqualTo(String.format("Revert \"%s...\"", changeTitle.substring(0, 59)));
+ assertThat(gApi.changes().id(revertChange.id).current().commit(false).message)
+ .isEqualTo(
+ String.format(
+ "Revert \"%s...\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n",
+ changeTitle.substring(0, 59),
+ gApi.changes().id(result).get().currentRevision,
+ revertChange.changeId));
+ }
+
+ @Test
@GerritConfig(name = "change.submitWholeTopic", value = "true")
public void revertSubmissionDifferentRepositoriesWithDependantChange() throws Exception {
projectOperations.newProject().name("secondProject").create();
diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
index cc8d233..bf93d58 100644
--- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
+++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java
@@ -45,6 +45,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.FileContentInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -64,7 +65,6 @@
import com.google.gerrit.server.project.testing.TestLabels;
import com.google.gerrit.server.restapi.change.ChangeEdits.EditMessage;
import com.google.gerrit.server.restapi.change.ChangeEdits.Post;
-import com.google.gerrit.server.restapi.change.ChangeEdits.Put;
import com.google.gson.reflect.TypeToken;
import com.google.gson.stream.JsonReader;
import com.google.inject.Inject;
@@ -501,7 +501,7 @@
@Test
public void createAndChangeEditInOneRequestRest() throws Exception {
- Put.Input in = new Put.Input();
+ FileContentInput in = new FileContentInput();
in.content = RawInputUtil.create(CONTENT_NEW);
adminRestSession.putRaw(urlEditFile(changeId, FILE_NAME), in.content).assertNoContent();
ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_NEW);
@@ -513,7 +513,7 @@
@Test
public void changeEditRest() throws Exception {
createEmptyEditFor(changeId);
- Put.Input in = new Put.Input();
+ FileContentInput in = new FileContentInput();
in.content = RawInputUtil.create(CONTENT_NEW);
adminRestSession.putRaw(urlEditFile(changeId, FILE_NAME), in.content).assertNoContent();
ensureSameBytes(getFileContentOfEdit(changeId, FILE_NAME), CONTENT_NEW);
@@ -534,7 +534,7 @@
@Test
public void getFileContentRest() throws Exception {
- Put.Input in = new Put.Input();
+ FileContentInput in = new FileContentInput();
in.content = RawInputUtil.create(CONTENT_NEW);
adminRestSession.putRaw(urlEditFile(changeId, FILE_NAME), in.content).assertNoContent();
gApi.changes().id(changeId).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW2));
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index c692a3b..c67a842 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -51,7 +51,7 @@
case V6_7:
return "blacktop/elasticsearch:6.7.2";
case V6_8:
- return "blacktop/elasticsearch:6.8.5";
+ return "blacktop/elasticsearch:6.8.6";
case V7_0:
return "blacktop/elasticsearch:7.0.1";
case V7_1:
@@ -63,7 +63,7 @@
case V7_4:
return "blacktop/elasticsearch:7.4.2";
case V7_5:
- return "blacktop/elasticsearch:7.5.0";
+ return "blacktop/elasticsearch:7.5.1";
}
throw new IllegalStateException("No tests for version: " + version.name());
}
diff --git a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
index 46e7d32..403d9d5 100644
--- a/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
+++ b/polygerrit-ui/app/elements/core/gr-main-header/gr-main-header_test.html
@@ -398,4 +398,4 @@
assert.equal(element._registerText, 'Sign up');
});
});
- </script>
\ No newline at end of file
+ </script>
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index e16c0a0..fa8008c 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -102,8 +102,8 @@
# and httpasyncclient as necessary.
maven_jar(
name = "elasticsearch-rest-client",
- artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.0",
- sha1 = "62535b6fc3a4e943e88e7640eac22e29f03a696d",
+ artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.5.1",
+ sha1 = "094c155906dc94146fc5adc344ea2c676d487cf2",
)
maven_jar(
