Add support for deleting change message
This commit adds a new REST endpoint to support deletion of a change message, which is similar to Icaeb3c24f. The deletion for NoteDb is implemented by rewriting the change meta branch, which will replace the whole change message with a sentence like "Change message removed by:... ". Messages like "(N comments)" and quotes will also be replaced as they are part of a change message. To be able to delete a change message, a user must hold the "Administrate Server" global capability. Change-Id: I439b00fcf03a629d9a33c401cf4d8cdd49705ace
This commit is contained in:
Changcheng Xiao
parent
ab6a072e84
commit
6d4ee64106
@ -2475,7 +2475,66 @@ As response a link:#change-message-info[ChangeMessageInfo] entity is returned.
|
||||
"username": "jdoe"
|
||||
},
|
||||
"date": "2013-03-23 21:34:02.419000000",
|
||||
"message": "Change message removed by: Administrator; Reason: spam",
|
||||
"message": "a change message",
|
||||
"_revision_number": 1
|
||||
}
|
||||
----
|
||||
|
||||
[[delete-change-message]]
|
||||
=== Delete Change Message
|
||||
--
|
||||
'DELETE /changes/link:#change-id[\{change-id\}]/message/link:#change-message-id[\{change-message-id\}' +
|
||||
'POST /changes/link:#change-id[\{change-id\}]//message/link:#change-message-id[\{change-message-id\}/delete'
|
||||
--
|
||||
|
||||
Deletes a change message by replacing the change message with a new message,
|
||||
which contains the name of the user who deleted the change message and the
|
||||
reason why it was deleted. The reason can be provided in the request body as a
|
||||
link:#delete-change-message-input[DeleteChangeMessageInput] entity.
|
||||
|
||||
Note that only users with the
|
||||
link:access-control.html#capability_administrateServer[Administrate Server]
|
||||
global capability are permitted to delete a change message.
|
||||
|
||||
To delete a change message, send a DELETE request:
|
||||
|
||||
.Request
|
||||
----
|
||||
DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/message/aaee04dcb46bafc8be24d8aa70b3b1beb7df5780 HTTP/1.0
|
||||
----
|
||||
|
||||
To provide a reason for the deletion, use a POST request:
|
||||
|
||||
.Request
|
||||
----
|
||||
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/message/aaee04dcb46bafc8be24d8aa70b3b1beb7df5780/delete HTTP/1.0
|
||||
Content-Type: application/json; charset=UTF-8
|
||||
|
||||
{
|
||||
"reason": "spam"
|
||||
}
|
||||
----
|
||||
|
||||
As response a link:#change-message-info[ChangeMessageInfo] entity is returned that
|
||||
describes the updated change message.
|
||||
|
||||
.Response
|
||||
----
|
||||
HTTP/1.1 200 OK
|
||||
Content-Disposition: attachment
|
||||
Content-Type: application/json; charset=UTF-8
|
||||
|
||||
)]}'
|
||||
{
|
||||
"id": "aaee04dcb46bafc8be24d8aa70b3b1beb7df5780",
|
||||
"author": {
|
||||
"_account_id": 1000096,
|
||||
"name": "John Doe",
|
||||
"email": "john.doe@example.com",
|
||||
"username": "jdoe"
|
||||
},
|
||||
"date": "2013-03-23 21:34:02.419000000",
|
||||
"message": "Change message removed by: Administrator\nReason: spam",
|
||||
"_revision_number": 1
|
||||
}
|
||||
----
|
||||
@ -6049,6 +6108,20 @@ Additional information about whom to notify about the update as a map
|
||||
of recipient type to link:#notify-info[NotifyInfo] entity.
|
||||
|=============================
|
||||
|
||||
[[delete-change-message-input]]
|
||||
=== DeleteChangeMessageInput
|
||||
The `DeleteChangeMessageInput` entity contains the options for deleting a change message.
|
||||
|
||||
[options="header",cols="1,^1,5"]
|
||||
|=============================
|
||||
|Field Name ||Description
|
||||
|`reason` |optional|
|
||||
The reason why the change message should be deleted. +
|
||||
If set, the change message will be replaced with
|
||||
"Change message removed by: `name`\nReason: `reason`",
|
||||
or just "Change message removed by: `name`." if not set.
|
||||
|=============================
|
||||
|
||||
[[delete-comment-input]]
|
||||
=== DeleteCommentInput
|
||||
The `DeleteCommentInput` entity contains the option for deleting a comment.
|
||||
|
||||
@ -33,6 +33,7 @@ import com.google.common.base.Strings;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.collect.Lists;
|
||||
import com.google.common.jimfs.Jimfs;
|
||||
import com.google.common.primitives.Chars;
|
||||
import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
|
||||
@ -63,6 +64,7 @@ import com.google.gerrit.extensions.client.ProjectWatchInfo;
|
||||
import com.google.gerrit.extensions.client.SubmitType;
|
||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||
import com.google.gerrit.extensions.common.ChangeType;
|
||||
import com.google.gerrit.extensions.common.CommentInfo;
|
||||
import com.google.gerrit.extensions.common.DiffInfo;
|
||||
import com.google.gerrit.extensions.common.EditInfo;
|
||||
import com.google.gerrit.extensions.restapi.BinaryResult;
|
||||
@ -76,6 +78,7 @@ import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.reviewdb.client.AccountGroup;
|
||||
import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
|
||||
import com.google.gerrit.reviewdb.client.Branch;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.reviewdb.client.Project;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
@ -148,6 +151,7 @@ import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.Comparator;
|
||||
import java.util.EnumSet;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
@ -167,6 +171,7 @@ import org.eclipse.jgit.lib.PersonIdent;
|
||||
import org.eclipse.jgit.lib.Ref;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevSort;
|
||||
import org.eclipse.jgit.revwalk.RevTree;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.eclipse.jgit.transport.FetchResult;
|
||||
@ -1695,4 +1700,29 @@ public abstract class AbstractDaemonTest {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
protected List<RevCommit> getChangeMetaCommitsInReverseOrder(Change.Id changeId)
|
||||
throws IOException {
|
||||
try (Repository repo = repoManager.openRepository(project);
|
||||
RevWalk revWalk = new RevWalk(repo)) {
|
||||
revWalk.sort(RevSort.TOPO);
|
||||
revWalk.sort(RevSort.REVERSE);
|
||||
Ref metaRef = repo.exactRef(RefNames.changeMetaRef(changeId));
|
||||
revWalk.markStart(revWalk.parseCommit(metaRef.getObjectId()));
|
||||
return Lists.newArrayList(revWalk);
|
||||
}
|
||||
}
|
||||
|
||||
protected List<CommentInfo> getChangeSortedComments(int changeNum) throws Exception {
|
||||
List<CommentInfo> comments = new ArrayList<>();
|
||||
Map<String, List<CommentInfo>> commentsMap = gApi.changes().id(changeNum).comments();
|
||||
for (Map.Entry<String, List<CommentInfo>> e : commentsMap.entrySet()) {
|
||||
for (CommentInfo c : e.getValue()) {
|
||||
c.path = e.getKey(); // Set the comment's path field.
|
||||
comments.add(c);
|
||||
}
|
||||
}
|
||||
comments.sort(Comparator.comparing(c -> c.id));
|
||||
return comments;
|
||||
}
|
||||
}
|
||||
|
||||
@ -23,6 +23,14 @@ public interface ChangeMessageApi {
|
||||
/** Gets one change message. */
|
||||
ChangeMessageInfo get() throws RestApiException;
|
||||
|
||||
/**
|
||||
* Deletes a change message by replacing its message. For NoteDb, it's implemented by rewriting
|
||||
* the commit history of change meta branch.
|
||||
*
|
||||
* @return the change message with its message updated.
|
||||
*/
|
||||
ChangeMessageInfo delete(DeleteChangeMessageInput input) throws RestApiException;
|
||||
|
||||
/**
|
||||
* A default implementation which allows source compatibility when adding new methods to the
|
||||
* interface.
|
||||
@ -32,5 +40,10 @@ public interface ChangeMessageApi {
|
||||
public ChangeMessageInfo get() throws RestApiException {
|
||||
throw new NotImplementedException();
|
||||
}
|
||||
|
||||
@Override
|
||||
public ChangeMessageInfo delete(DeleteChangeMessageInput input) throws RestApiException {
|
||||
throw new NotImplementedException();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,31 @@
|
||||
// Copyright (C) 2018 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;
|
||||
|
||||
/** Input for deleting a change message. */
|
||||
public class DeleteChangeMessageInput {
|
||||
/** An optional reason for deleting the change message. */
|
||||
@DefaultInput public String reason;
|
||||
|
||||
public DeleteChangeMessageInput() {
|
||||
this.reason = "";
|
||||
}
|
||||
|
||||
public DeleteChangeMessageInput(String reason) {
|
||||
this.reason = reason;
|
||||
}
|
||||
}
|
||||
@ -45,4 +45,24 @@ public class ChangeMessageInfo {
|
||||
public int hashCode() {
|
||||
return Objects.hash(id, tag, author, realAuthor, date, message, _revisionNumber);
|
||||
}
|
||||
|
||||
@Override
|
||||
public String toString() {
|
||||
return "ChangeMessageInfo{"
|
||||
+ "id="
|
||||
+ id
|
||||
+ ", tag="
|
||||
+ tag
|
||||
+ ", author="
|
||||
+ author
|
||||
+ ", realAuthor="
|
||||
+ realAuthor
|
||||
+ ", date="
|
||||
+ date
|
||||
+ ", _revisionNumber"
|
||||
+ _revisionNumber
|
||||
+ ", message=["
|
||||
+ message
|
||||
+ "]}";
|
||||
}
|
||||
}
|
||||
|
||||
@ -16,6 +16,7 @@ package com.google.gerrit.server;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static com.google.common.base.Preconditions.checkState;
|
||||
import static com.google.gerrit.reviewdb.server.ReviewDbUtil.unwrapDb;
|
||||
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
@ -27,7 +28,9 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.account.AccountLoader;
|
||||
import com.google.gerrit.server.notedb.ChangeNotes;
|
||||
import com.google.gerrit.server.notedb.ChangeUpdate;
|
||||
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
|
||||
import com.google.gerrit.server.notedb.NotesMigration;
|
||||
import com.google.gerrit.server.update.BatchUpdateReviewDb;
|
||||
import com.google.gerrit.server.update.ChangeContext;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
@ -128,6 +131,71 @@ public class ChangeMessagesUtil {
|
||||
db.changeMessages().insert(Collections.singleton(changeMessage));
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace an existing change message with the provided new message.
|
||||
*
|
||||
* <p>The ID of a change message is different between NoteDb and ReviewDb. In NoteDb, it's the
|
||||
* commit SHA-1, but in ReviewDb it's generated randomly. To make sure the change message can be
|
||||
* deleted from both NoteDb and ReviewDb, the index of the change message must be used rather than
|
||||
* its ID.
|
||||
*
|
||||
* @param db the {@code ReviewDb} instance to update.
|
||||
* @param update change update.
|
||||
* @param targetMessageIdx the index of the target change message.
|
||||
* @param newMessage the new message which is going to replace the old.
|
||||
* @throws OrmException
|
||||
*/
|
||||
public void replaceChangeMessage(
|
||||
ReviewDb db, ChangeUpdate update, int targetMessageIdx, String newMessage)
|
||||
throws OrmException {
|
||||
if (PrimaryStorage.of(update.getChange()).equals(PrimaryStorage.REVIEW_DB)) {
|
||||
if (db instanceof BatchUpdateReviewDb) {
|
||||
db = ((BatchUpdateReviewDb) db).unsafeGetDelegate();
|
||||
}
|
||||
db = unwrapDb(db);
|
||||
|
||||
List<ChangeMessage> messagesInReviewDb =
|
||||
sortChangeMessages(db.changeMessages().byChange(update.getId()));
|
||||
if (migration.readChanges()) {
|
||||
sanityCheckForChangeMessages(messagesInReviewDb, update.getNotes().getChangeMessages());
|
||||
}
|
||||
ChangeMessage targetMessage = messagesInReviewDb.get(targetMessageIdx);
|
||||
targetMessage.setMessage(newMessage);
|
||||
db.changeMessages().upsert(Collections.singleton(targetMessage));
|
||||
}
|
||||
|
||||
update.deleteChangeMessageByRewritingHistory(targetMessageIdx, newMessage);
|
||||
}
|
||||
|
||||
private static void sanityCheckForChangeMessages(
|
||||
List<ChangeMessage> messagesInReviewDb, List<ChangeMessage> messagesInNoteDb) {
|
||||
String message =
|
||||
String.format(
|
||||
"Change messages in ReivewDb and NoteDb don't match: NoteDb %s; ReviewDb %s",
|
||||
messagesInNoteDb, messagesInReviewDb);
|
||||
if (messagesInReviewDb.size() != messagesInNoteDb.size()) {
|
||||
throw new IllegalStateException(message);
|
||||
}
|
||||
|
||||
for (int i = 0; i < messagesInReviewDb.size(); i++) {
|
||||
ChangeMessage messageInReviewDb = messagesInReviewDb.get(i);
|
||||
ChangeMessage messageInNoteDb = messagesInNoteDb.get(i);
|
||||
|
||||
// Don't compare the keys because they are different for the same change message in NoteDb and
|
||||
// ReviewDb.
|
||||
boolean isEqual =
|
||||
Objects.equals(messageInReviewDb.getAuthor(), messageInNoteDb.getAuthor())
|
||||
&& Objects.equals(messageInReviewDb.getWrittenOn(), messageInNoteDb.getWrittenOn())
|
||||
&& Objects.equals(messageInReviewDb.getMessage(), messageInNoteDb.getMessage())
|
||||
&& Objects.equals(messageInReviewDb.getPatchSetId(), messageInNoteDb.getPatchSetId())
|
||||
&& Objects.equals(messageInReviewDb.getTag(), messageInNoteDb.getTag())
|
||||
&& Objects.equals(messageInReviewDb.getRealAuthor(), messageInNoteDb.getRealAuthor());
|
||||
if (!isEqual) {
|
||||
throw new IllegalStateException(message);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param tag value of a tag, or null.
|
||||
* @return whether the tag starts with the autogenerated prefix.
|
||||
|
||||
@ -17,9 +17,11 @@ package com.google.gerrit.server.api.changes;
|
||||
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
|
||||
|
||||
import com.google.gerrit.extensions.api.changes.ChangeMessageApi;
|
||||
import com.google.gerrit.extensions.api.changes.DeleteChangeMessageInput;
|
||||
import com.google.gerrit.extensions.common.ChangeMessageInfo;
|
||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.server.change.ChangeMessageResource;
|
||||
import com.google.gerrit.server.restapi.change.DeleteChangeMessage;
|
||||
import com.google.gerrit.server.restapi.change.GetChangeMessage;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.assistedinject.Assisted;
|
||||
@ -30,12 +32,16 @@ class ChangeMessageApiImpl implements ChangeMessageApi {
|
||||
}
|
||||
|
||||
private final GetChangeMessage getChangeMessage;
|
||||
private final DeleteChangeMessage deleteChangeMessage;
|
||||
private final ChangeMessageResource changeMessageResource;
|
||||
|
||||
@Inject
|
||||
ChangeMessageApiImpl(
|
||||
GetChangeMessage getChangeMessage, @Assisted ChangeMessageResource changeMessageResource) {
|
||||
GetChangeMessage getChangeMessage,
|
||||
DeleteChangeMessage deleteChangeMessage,
|
||||
@Assisted ChangeMessageResource changeMessageResource) {
|
||||
this.getChangeMessage = getChangeMessage;
|
||||
this.deleteChangeMessage = deleteChangeMessage;
|
||||
this.changeMessageResource = changeMessageResource;
|
||||
}
|
||||
|
||||
@ -47,4 +53,13 @@ class ChangeMessageApiImpl implements ChangeMessageApi {
|
||||
throw asRestApiException("Cannot retrieve change message", e);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public ChangeMessageInfo delete(DeleteChangeMessageInput input) throws RestApiException {
|
||||
try {
|
||||
return deleteChangeMessage.apply(changeMessageResource, input).value();
|
||||
} catch (Exception e) {
|
||||
throw asRestApiException("Cannot delete change message", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -160,6 +160,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
private ChangeDraftUpdate draftUpdate;
|
||||
private RobotCommentUpdate robotCommentUpdate;
|
||||
private DeleteCommentRewriter deleteCommentRewriter;
|
||||
private DeleteChangeMessageRewriter deleteChangeMessageRewriter;
|
||||
|
||||
@AssistedInject
|
||||
private ChangeUpdate(
|
||||
@ -395,6 +396,11 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
deleteCommentRewriterFactory.create(getChange().getId(), uuid, newMessage);
|
||||
}
|
||||
|
||||
public void deleteChangeMessageByRewritingHistory(int targetMessageIdx, String newMessage) {
|
||||
deleteChangeMessageRewriter =
|
||||
new DeleteChangeMessageRewriter(getChange().getId(), targetMessageIdx, newMessage);
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
ChangeDraftUpdate createDraftUpdateIfNull() {
|
||||
if (draftUpdate == null) {
|
||||
@ -609,7 +615,9 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
@Override
|
||||
protected CommitBuilder applyImpl(RevWalk rw, ObjectInserter ins, ObjectId curr)
|
||||
throws OrmException, IOException {
|
||||
checkState(deleteCommentRewriter == null, "cannot update and rewrite ref in one BatchUpdate");
|
||||
checkState(
|
||||
deleteCommentRewriter == null && deleteChangeMessageRewriter == null,
|
||||
"cannot update and rewrite ref in one BatchUpdate");
|
||||
|
||||
CommitBuilder cb = new CommitBuilder();
|
||||
|
||||
@ -823,6 +831,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
|
||||
return deleteCommentRewriter;
|
||||
}
|
||||
|
||||
public DeleteChangeMessageRewriter getDeleteChangeMessageRewriter() {
|
||||
return deleteChangeMessageRewriter;
|
||||
}
|
||||
|
||||
public void setAllowWriteToNewRef(boolean allow) {
|
||||
isAllowWriteToNewtRef = allow;
|
||||
}
|
||||
|
||||
@ -0,0 +1,133 @@
|
||||
// Copyright (C) 2018 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.server.notedb;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkArgument;
|
||||
import static com.google.common.base.Preconditions.checkState;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.parseCommitMessageRange;
|
||||
import static org.eclipse.jgit.util.RawParseUtils.decode;
|
||||
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import java.io.IOException;
|
||||
import java.nio.charset.Charset;
|
||||
import java.util.Optional;
|
||||
import org.eclipse.jgit.lib.CommitBuilder;
|
||||
import org.eclipse.jgit.lib.ObjectId;
|
||||
import org.eclipse.jgit.lib.ObjectInserter;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevSort;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.eclipse.jgit.util.RawParseUtils;
|
||||
|
||||
/**
|
||||
* Deletes a change message from NoteDb by rewriting the commit history. After deletion, the whole
|
||||
* change message will be replaced by a new message indicating the original change message has been
|
||||
* deleted for the given reason.
|
||||
*/
|
||||
public class DeleteChangeMessageRewriter implements NoteDbRewriter {
|
||||
|
||||
private final Change.Id changeId;
|
||||
private final int targetMessageIdx;
|
||||
private final String newChangeMessage;
|
||||
|
||||
DeleteChangeMessageRewriter(Change.Id changeId, int targetMessageIdx, String newChangeMessage) {
|
||||
this.changeId = changeId;
|
||||
this.targetMessageIdx = targetMessageIdx;
|
||||
this.newChangeMessage = newChangeMessage;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getRefName() {
|
||||
return RefNames.changeMetaRef(changeId);
|
||||
}
|
||||
|
||||
@Override
|
||||
public ObjectId rewriteCommitHistory(RevWalk revWalk, ObjectInserter inserter, ObjectId currTip)
|
||||
throws IOException {
|
||||
checkArgument(!currTip.equals(ObjectId.zeroId()));
|
||||
|
||||
// Walk from the first commit of the branch.
|
||||
revWalk.reset();
|
||||
revWalk.markStart(revWalk.parseCommit(currTip));
|
||||
revWalk.sort(RevSort.TOPO);
|
||||
revWalk.sort(RevSort.REVERSE);
|
||||
|
||||
ObjectId newTipId = null;
|
||||
RevCommit originalCommit;
|
||||
int idx = 0;
|
||||
while ((originalCommit = revWalk.next()) != null) {
|
||||
if (idx < targetMessageIdx) {
|
||||
newTipId = originalCommit;
|
||||
idx++;
|
||||
continue;
|
||||
}
|
||||
|
||||
String newCommitMessage =
|
||||
(idx == targetMessageIdx)
|
||||
? createNewCommitMessage(originalCommit)
|
||||
: originalCommit.getFullMessage();
|
||||
newTipId = rewriteOneCommit(originalCommit, newTipId, newCommitMessage, inserter);
|
||||
|
||||
idx++;
|
||||
}
|
||||
return newTipId;
|
||||
}
|
||||
|
||||
private String createNewCommitMessage(RevCommit commit) {
|
||||
byte[] raw = commit.getRawBuffer();
|
||||
|
||||
Optional<ChangeNoteUtil.CommitMessageRange> range = parseCommitMessageRange(commit);
|
||||
checkState(range.isPresent(), "failed to parse commit message");
|
||||
|
||||
// Only replace the commit message body, which is the user-provided message. The subject and
|
||||
// footers are NoteDb metadata.
|
||||
Charset encoding = RawParseUtils.parseEncoding(raw);
|
||||
String prefix =
|
||||
decode(encoding, raw, range.get().subjectStart(), range.get().changeMessageStart());
|
||||
String postfix = decode(encoding, raw, range.get().changeMessageEnd() + 1, raw.length);
|
||||
return prefix + newChangeMessage + postfix;
|
||||
}
|
||||
|
||||
/**
|
||||
* Rewrites one commit.
|
||||
*
|
||||
* @param originalCommit the original commit to be rewritten.
|
||||
* @param parentCommitId the parent of the new commit. For the first rewritten commit, it's the
|
||||
* parent of 'originalCommit'. For the latter rewritten commits, it's the commit rewritten
|
||||
* just before it.
|
||||
* @param commitMessage the full commit message of the new commit.
|
||||
* @param inserter the {@code ObjectInserter} for the rewrite process.
|
||||
* @return the {@code objectId} of the new commit.
|
||||
* @throws IOException
|
||||
*/
|
||||
private ObjectId rewriteOneCommit(
|
||||
RevCommit originalCommit,
|
||||
ObjectId parentCommitId,
|
||||
String commitMessage,
|
||||
ObjectInserter inserter)
|
||||
throws IOException {
|
||||
CommitBuilder cb = new CommitBuilder();
|
||||
if (parentCommitId != null) {
|
||||
cb.setParentId(parentCommitId);
|
||||
}
|
||||
cb.setTreeId(originalCommit.getTree());
|
||||
cb.setMessage(commitMessage);
|
||||
cb.setCommitter(originalCommit.getCommitterIdent());
|
||||
cb.setAuthor(originalCommit.getAuthorIdent());
|
||||
cb.setEncoding(originalCommit.getEncoding());
|
||||
return inserter.insert(cb);
|
||||
}
|
||||
}
|
||||
@ -467,14 +467,33 @@ public class NoteDbUpdateManager implements AutoCloseable {
|
||||
if (rcu != null) {
|
||||
robotCommentUpdates.put(rcu.getRefName(), rcu);
|
||||
}
|
||||
DeleteCommentRewriter rwt = update.getDeleteCommentRewriter();
|
||||
if (rwt != null) {
|
||||
// Checks whether there is any ChangeUpdate added earlier trying to update the same ref.
|
||||
DeleteCommentRewriter deleteCommentRewriter = update.getDeleteCommentRewriter();
|
||||
if (deleteCommentRewriter != null) {
|
||||
// Checks whether there is any ChangeUpdate or rewriter added earlier for the same ref.
|
||||
checkArgument(
|
||||
!changeUpdates.containsKey(rwt.getRefName()),
|
||||
!changeUpdates.containsKey(deleteCommentRewriter.getRefName()),
|
||||
"cannot update & rewrite ref %s in one BatchUpdate",
|
||||
rwt.getRefName());
|
||||
rewriters.put(rwt.getRefName(), rwt);
|
||||
deleteCommentRewriter.getRefName());
|
||||
checkArgument(
|
||||
!rewriters.containsKey(deleteCommentRewriter.getRefName()),
|
||||
"cannot rewrite the same ref %s in one BatchUpdate",
|
||||
deleteCommentRewriter.getRefName());
|
||||
rewriters.put(deleteCommentRewriter.getRefName(), deleteCommentRewriter);
|
||||
}
|
||||
|
||||
DeleteChangeMessageRewriter deleteChangeMessageRewriter =
|
||||
update.getDeleteChangeMessageRewriter();
|
||||
if (deleteChangeMessageRewriter != null) {
|
||||
// Checks whether there is any ChangeUpdate or rewriter added earlier for the same ref.
|
||||
checkArgument(
|
||||
!changeUpdates.containsKey(deleteChangeMessageRewriter.getRefName()),
|
||||
"cannot update & rewrite ref %s in one BatchUpdate",
|
||||
deleteChangeMessageRewriter.getRefName());
|
||||
checkArgument(
|
||||
!rewriters.containsKey(deleteChangeMessageRewriter.getRefName()),
|
||||
"cannot rewrite the same ref %s in one BatchUpdate",
|
||||
deleteChangeMessageRewriter.getRefName());
|
||||
rewriters.put(deleteChangeMessageRewriter.getRefName(), deleteChangeMessageRewriter);
|
||||
}
|
||||
|
||||
changeUpdates.put(update.getRefName(), update);
|
||||
|
||||
@ -0,0 +1,175 @@
|
||||
// Copyright (C) 2018 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.server.restapi.change;
|
||||
|
||||
import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static com.google.gerrit.server.ChangeMessagesUtil.createChangeMessageInfo;
|
||||
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.common.TimeUtil;
|
||||
import com.google.gerrit.extensions.api.changes.DeleteChangeMessageInput;
|
||||
import com.google.gerrit.extensions.common.ChangeMessageInfo;
|
||||
import com.google.gerrit.extensions.common.Input;
|
||||
import com.google.gerrit.extensions.restapi.Response;
|
||||
import com.google.gerrit.extensions.restapi.RestApiException;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.ChangeMessage;
|
||||
import com.google.gerrit.reviewdb.client.PatchSet;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.ChangeMessagesUtil;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
import com.google.gerrit.server.account.AccountLoader;
|
||||
import com.google.gerrit.server.change.ChangeMessageResource;
|
||||
import com.google.gerrit.server.notedb.ChangeNotes;
|
||||
import com.google.gerrit.server.permissions.GlobalPermission;
|
||||
import com.google.gerrit.server.permissions.PermissionBackend;
|
||||
import com.google.gerrit.server.permissions.PermissionBackendException;
|
||||
import com.google.gerrit.server.update.BatchUpdate;
|
||||
import com.google.gerrit.server.update.BatchUpdateOp;
|
||||
import com.google.gerrit.server.update.ChangeContext;
|
||||
import com.google.gerrit.server.update.RetryHelper;
|
||||
import com.google.gerrit.server.update.RetryingRestModifyView;
|
||||
import com.google.gerrit.server.update.UpdateException;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import com.google.inject.Singleton;
|
||||
import java.io.IOException;
|
||||
import java.util.List;
|
||||
|
||||
/** Deletes a change message by rewriting history. */
|
||||
@Singleton
|
||||
public class DeleteChangeMessage
|
||||
extends RetryingRestModifyView<
|
||||
ChangeMessageResource, DeleteChangeMessageInput, Response<ChangeMessageInfo>> {
|
||||
|
||||
private final Provider<CurrentUser> userProvider;
|
||||
private final Provider<ReviewDb> dbProvider;
|
||||
private final PermissionBackend permissionBackend;
|
||||
private final ChangeMessagesUtil changeMessagesUtil;
|
||||
private final AccountLoader.Factory accountLoaderFactory;
|
||||
private final ChangeNotes.Factory notesFactory;
|
||||
|
||||
@Inject
|
||||
public DeleteChangeMessage(
|
||||
Provider<CurrentUser> userProvider,
|
||||
Provider<ReviewDb> dbProvider,
|
||||
PermissionBackend permissionBackend,
|
||||
ChangeMessagesUtil changeMessagesUtil,
|
||||
AccountLoader.Factory accountLoaderFactory,
|
||||
ChangeNotes.Factory notesFactory,
|
||||
RetryHelper retryHelper) {
|
||||
super(retryHelper);
|
||||
this.userProvider = userProvider;
|
||||
this.dbProvider = dbProvider;
|
||||
this.permissionBackend = permissionBackend;
|
||||
this.changeMessagesUtil = changeMessagesUtil;
|
||||
this.accountLoaderFactory = accountLoaderFactory;
|
||||
this.notesFactory = notesFactory;
|
||||
}
|
||||
|
||||
@Override
|
||||
public Response<ChangeMessageInfo> applyImpl(
|
||||
BatchUpdate.Factory updateFactory,
|
||||
ChangeMessageResource resource,
|
||||
DeleteChangeMessageInput input)
|
||||
throws RestApiException, PermissionBackendException, OrmException, UpdateException,
|
||||
IOException {
|
||||
CurrentUser user = userProvider.get();
|
||||
permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
|
||||
|
||||
String newChangeMessage =
|
||||
createNewChangeMessage(user.asIdentifiedUser().getName(), input.reason);
|
||||
DeleteChangeMessageOp deleteChangeMessageOp =
|
||||
new DeleteChangeMessageOp(resource.getChangeMessageIndex(), newChangeMessage);
|
||||
try (BatchUpdate batchUpdate =
|
||||
updateFactory.create(
|
||||
dbProvider.get(), resource.getChangeResource().getProject(), user, TimeUtil.nowTs())) {
|
||||
batchUpdate.addOp(resource.getChangeId(), deleteChangeMessageOp).execute();
|
||||
}
|
||||
|
||||
ChangeMessageInfo updatedMessageInfo =
|
||||
createUpdatedChangeMessageInfo(resource.getChangeId(), resource.getChangeMessageIndex());
|
||||
return Response.created(updatedMessageInfo);
|
||||
}
|
||||
|
||||
private ChangeMessageInfo createUpdatedChangeMessageInfo(Change.Id id, int targetIdx)
|
||||
throws OrmException, PermissionBackendException {
|
||||
List<ChangeMessage> messages =
|
||||
changeMessagesUtil.byChange(dbProvider.get(), notesFactory.createChecked(id));
|
||||
ChangeMessage updatedChangeMessage = messages.get(targetIdx);
|
||||
AccountLoader accountLoader = accountLoaderFactory.create(true);
|
||||
ChangeMessageInfo info = createChangeMessageInfo(updatedChangeMessage, accountLoader);
|
||||
accountLoader.fill();
|
||||
return info;
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
public static String createNewChangeMessage(String deletedBy, @Nullable String deletedReason) {
|
||||
checkNotNull(deletedBy, "user name must not be null");
|
||||
|
||||
if (Strings.isNullOrEmpty(deletedReason)) {
|
||||
return createNewChangeMessage(deletedBy);
|
||||
}
|
||||
return String.format("Change message removed by: %s\nReason: %s", deletedBy, deletedReason);
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
public static String createNewChangeMessage(String deletedBy) {
|
||||
checkNotNull(deletedBy, "user name must not be null");
|
||||
|
||||
return "Change message removed by: " + deletedBy;
|
||||
}
|
||||
|
||||
private class DeleteChangeMessageOp implements BatchUpdateOp {
|
||||
private final int targetMessageIdx;
|
||||
private final String newMessage;
|
||||
|
||||
DeleteChangeMessageOp(int targetMessageIdx, String newMessage) {
|
||||
this.targetMessageIdx = targetMessageIdx;
|
||||
this.newMessage = newMessage;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean updateChange(ChangeContext ctx) throws OrmException {
|
||||
PatchSet.Id psId = ctx.getChange().currentPatchSetId();
|
||||
changeMessagesUtil.replaceChangeMessage(
|
||||
ctx.getDb(), ctx.getUpdate(psId), targetMessageIdx, newMessage);
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
@Singleton
|
||||
public static class DefaultDeleteChangeMessage
|
||||
extends RetryingRestModifyView<ChangeMessageResource, Input, Response<ChangeMessageInfo>> {
|
||||
private final DeleteChangeMessage deleteChangeMessage;
|
||||
|
||||
@Inject
|
||||
public DefaultDeleteChangeMessage(
|
||||
DeleteChangeMessage deleteChangeMessage, RetryHelper retryHelper) {
|
||||
super(retryHelper);
|
||||
this.deleteChangeMessage = deleteChangeMessage;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Response<ChangeMessageInfo> applyImpl(
|
||||
BatchUpdate.Factory updateFactory, ChangeMessageResource resource, Input input)
|
||||
throws Exception {
|
||||
return deleteChangeMessage.applyImpl(updateFactory, resource, new DeleteChangeMessageInput());
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -182,6 +182,8 @@ public class Module extends RestApiModule {
|
||||
|
||||
child(CHANGE_KIND, "messages").to(ChangeMessages.class);
|
||||
get(CHANGE_MESSAGE_KIND).to(GetChangeMessage.class);
|
||||
delete(CHANGE_MESSAGE_KIND).to(DeleteChangeMessage.DefaultDeleteChangeMessage.class);
|
||||
post(CHANGE_MESSAGE_KIND, "delete").to(DeleteChangeMessage.class);
|
||||
|
||||
factory(AccountLoader.Factory.class);
|
||||
factory(ChangeInserter.Factory.class);
|
||||
|
||||
@ -11,26 +11,42 @@
|
||||
// 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.acceptance.rest.change;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
|
||||
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
|
||||
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
|
||||
import static com.google.gerrit.server.notedb.ChangeNoteUtil.parseCommitMessageRange;
|
||||
import static com.google.gerrit.server.restapi.change.DeleteChangeMessage.createNewChangeMessage;
|
||||
import static java.util.concurrent.TimeUnit.SECONDS;
|
||||
import static java.util.stream.Collectors.toSet;
|
||||
import static org.eclipse.jgit.util.RawParseUtils.decode;
|
||||
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.Lists;
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||
import com.google.gerrit.acceptance.PushOneCommit;
|
||||
import com.google.gerrit.acceptance.TestAccount;
|
||||
import com.google.gerrit.common.data.GlobalCapability;
|
||||
import com.google.gerrit.extensions.api.changes.DeleteChangeMessageInput;
|
||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
||||
import com.google.gerrit.extensions.common.ChangeInfo;
|
||||
import com.google.gerrit.extensions.common.ChangeMessageInfo;
|
||||
import com.google.gerrit.extensions.common.CommentInfo;
|
||||
import com.google.gerrit.extensions.restapi.AuthException;
|
||||
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.server.notedb.ChangeNoteUtil;
|
||||
import com.google.gerrit.testing.ConfigSuite;
|
||||
import com.google.gerrit.testing.TestTimeUtil;
|
||||
import java.nio.charset.Charset;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.util.RawParseUtils;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
@ -114,14 +130,85 @@ public class ChangeMessagesIT extends AbstractDaemonTest {
|
||||
public void getOneChangeMessage() throws Exception {
|
||||
int changeNum = createOneChangeWithMultipleChangeMessagesInHistory();
|
||||
List<ChangeMessageInfo> messages = new ArrayList<>(gApi.changes().id(changeNum).get().messages);
|
||||
|
||||
for (ChangeMessageInfo messageInfo : messages) {
|
||||
String id = messageInfo.id;
|
||||
assertThat(gApi.changes().id(changeNum).message(id).get()).isEqualTo(messageInfo);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteCannotBeAppliedWithoutAdministrateServerCapability() throws Exception {
|
||||
int changeNum = createOneChangeWithMultipleChangeMessagesInHistory();
|
||||
setApiUser(user);
|
||||
|
||||
try {
|
||||
deleteOneChangeMessage(changeNum, 0, user, "spam");
|
||||
fail("expected AuthException");
|
||||
} catch (AuthException e) {
|
||||
assertThat(e.getMessage()).isEqualTo("administrate server not permitted");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteCanBeAppliedWithAdministrateServerCapability() throws Exception {
|
||||
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ADMINISTRATE_SERVER);
|
||||
int changeNum = createOneChangeWithMultipleChangeMessagesInHistory();
|
||||
setApiUser(user);
|
||||
deleteOneChangeMessage(changeNum, 0, user, "spam");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteCannotBeAppliedWithEmptyChangeMessageUuid() throws Exception {
|
||||
String changeId = createChange().getChangeId();
|
||||
|
||||
try {
|
||||
gApi.changes().id(changeId).message("").delete(new DeleteChangeMessageInput("spam"));
|
||||
fail("expected ResourceNotFoundException");
|
||||
} catch (ResourceNotFoundException e) {
|
||||
assertThat(e.getMessage()).isEqualTo("change message not found");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteCannotBeAppliedWithNonExistingChangeMessageUuid() throws Exception {
|
||||
String changeId = createChange().getChangeId();
|
||||
DeleteChangeMessageInput input = new DeleteChangeMessageInput();
|
||||
String id = "8473b95934b5732ac55d26311a706c9c2bde9941";
|
||||
input.reason = "spam";
|
||||
|
||||
try {
|
||||
gApi.changes().id(changeId).message(id).delete(input);
|
||||
fail("expected ResourceNotFoundException");
|
||||
} catch (ResourceNotFoundException e) {
|
||||
assertThat(e.getMessage()).isEqualTo(String.format("change message %s not found", id));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteCanBeAppliedWithoutProvidingReason() throws Exception {
|
||||
int changeNum = createOneChangeWithMultipleChangeMessagesInHistory();
|
||||
deleteOneChangeMessage(changeNum, 2, admin, "");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteOneChangeMessageTwice() throws Exception {
|
||||
int changeNum = createOneChangeWithMultipleChangeMessagesInHistory();
|
||||
// Deletes the second change message twice.
|
||||
deleteOneChangeMessage(changeNum, 1, admin, "reason 1");
|
||||
deleteOneChangeMessage(changeNum, 1, admin, "reason 2");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void deleteMultipleChangeMessages() throws Exception {
|
||||
int changeNum = createOneChangeWithMultipleChangeMessagesInHistory();
|
||||
for (int i = 0; i < 7; ++i) {
|
||||
deleteOneChangeMessage(changeNum, i, admin, "reason " + i);
|
||||
}
|
||||
}
|
||||
|
||||
private int createOneChangeWithMultipleChangeMessagesInHistory() throws Exception {
|
||||
// Creates the following commit history on the meta branch of the test change.
|
||||
|
||||
setApiUser(user);
|
||||
// Commit 1: create a change.
|
||||
PushOneCommit.Result result = createChange();
|
||||
@ -158,6 +245,150 @@ public class ChangeMessagesIT extends AbstractDaemonTest {
|
||||
gApi.changes().id(changeId).current().review(reviewInput);
|
||||
}
|
||||
|
||||
private void deleteOneChangeMessage(
|
||||
int changeNum, int deletedMessageIndex, TestAccount deletedBy, String reason)
|
||||
throws Exception {
|
||||
List<ChangeMessageInfo> messagesBeforeDeletion = gApi.changes().id(changeNum).messages();
|
||||
|
||||
List<CommentInfo> commentsBefore = getChangeSortedComments(changeNum);
|
||||
List<RevCommit> commitsBefore = new ArrayList<>();
|
||||
if (notesMigration.readChanges()) {
|
||||
commitsBefore = getChangeMetaCommitsInReverseOrder(new Change.Id(changeNum));
|
||||
}
|
||||
|
||||
String id = messagesBeforeDeletion.get(deletedMessageIndex).id;
|
||||
DeleteChangeMessageInput input = new DeleteChangeMessageInput(reason);
|
||||
ChangeMessageInfo info = gApi.changes().id(changeNum).message(id).delete(input);
|
||||
|
||||
// Verify the return change message info is as expect.
|
||||
assertThat(info.message).isEqualTo(createNewChangeMessage(deletedBy.fullName, reason));
|
||||
List<ChangeMessageInfo> messagesAfterDeletion = gApi.changes().id(changeNum).messages();
|
||||
assertMessagesAfterDeletion(
|
||||
messagesBeforeDeletion, messagesAfterDeletion, deletedMessageIndex, deletedBy, reason);
|
||||
assertCommentsAfterDeletion(changeNum, commentsBefore);
|
||||
|
||||
// Verify change index is updated after deletion.
|
||||
List<ChangeInfo> changes = gApi.changes().query("message removed").get();
|
||||
assertThat(changes.stream().map(c -> c._number).collect(toSet())).contains(changeNum);
|
||||
|
||||
// Verifies states of commits if NoteDb is on.
|
||||
if (notesMigration.readChanges()) {
|
||||
assertMetaCommitsAfterDeletion(
|
||||
commitsBefore, changeNum, deletedMessageIndex, deletedBy, reason);
|
||||
}
|
||||
}
|
||||
|
||||
private void assertMessagesAfterDeletion(
|
||||
List<ChangeMessageInfo> messagesBeforeDeletion,
|
||||
List<ChangeMessageInfo> messagesAfterDeletion,
|
||||
int deletedMessageIndex,
|
||||
TestAccount deletedBy,
|
||||
String deleteReason) {
|
||||
assertThat(messagesAfterDeletion)
|
||||
.named("after: %s; before: %s", messagesAfterDeletion, messagesBeforeDeletion)
|
||||
.hasSize(messagesBeforeDeletion.size());
|
||||
|
||||
for (int i = 0; i < messagesAfterDeletion.size(); ++i) {
|
||||
ChangeMessageInfo before = messagesBeforeDeletion.get(i);
|
||||
ChangeMessageInfo after = messagesAfterDeletion.get(i);
|
||||
|
||||
if (i < deletedMessageIndex) {
|
||||
// The uuid of a commit message will be updated after rewriting.
|
||||
assertThat(after.id).isEqualTo(before.id);
|
||||
}
|
||||
|
||||
assertThat(after.tag).isEqualTo(before.tag);
|
||||
assertThat(after.author).isEqualTo(before.author);
|
||||
assertThat(after.realAuthor).isEqualTo(before.realAuthor);
|
||||
assertThat(after._revisionNumber).isEqualTo(before._revisionNumber);
|
||||
|
||||
if (i == deletedMessageIndex) {
|
||||
assertThat(after.message)
|
||||
.isEqualTo(createNewChangeMessage(deletedBy.fullName, deleteReason));
|
||||
} else {
|
||||
assertThat(after.message).isEqualTo(before.message);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private void assertMetaCommitsAfterDeletion(
|
||||
List<RevCommit> commitsBeforeDeletion,
|
||||
int changeNum,
|
||||
int deletedMessageIndex,
|
||||
TestAccount deletedBy,
|
||||
String deleteReason)
|
||||
throws Exception {
|
||||
List<RevCommit> commitsAfterDeletion =
|
||||
getChangeMetaCommitsInReverseOrder(new Change.Id(changeNum));
|
||||
assertThat(commitsAfterDeletion).hasSize(commitsBeforeDeletion.size());
|
||||
|
||||
for (int i = 0; i < commitsBeforeDeletion.size(); i++) {
|
||||
RevCommit commitBefore = commitsBeforeDeletion.get(i);
|
||||
RevCommit commitAfter = commitsAfterDeletion.get(i);
|
||||
if (i == deletedMessageIndex) {
|
||||
byte[] rawBefore = commitBefore.getRawBuffer();
|
||||
byte[] rawAfter = commitAfter.getRawBuffer();
|
||||
Charset encodingBefore = RawParseUtils.parseEncoding(rawBefore);
|
||||
Charset encodingAfter = RawParseUtils.parseEncoding(rawAfter);
|
||||
Optional<ChangeNoteUtil.CommitMessageRange> rangeBefore =
|
||||
parseCommitMessageRange(commitBefore);
|
||||
Optional<ChangeNoteUtil.CommitMessageRange> rangeAfter =
|
||||
parseCommitMessageRange(commitAfter);
|
||||
assertThat(rangeBefore.isPresent()).isTrue();
|
||||
assertThat(rangeAfter.isPresent()).isTrue();
|
||||
|
||||
String subjectBefore =
|
||||
decode(
|
||||
encodingBefore,
|
||||
rawBefore,
|
||||
rangeBefore.get().subjectStart(),
|
||||
rangeBefore.get().subjectEnd());
|
||||
String subjectAfter =
|
||||
decode(
|
||||
encodingAfter,
|
||||
rawAfter,
|
||||
rangeAfter.get().subjectStart(),
|
||||
rangeAfter.get().subjectEnd());
|
||||
assertThat(subjectBefore).isEqualTo(subjectAfter);
|
||||
|
||||
String footersBefore =
|
||||
decode(
|
||||
encodingBefore,
|
||||
rawBefore,
|
||||
rangeBefore.get().changeMessageEnd() + 1,
|
||||
rawBefore.length);
|
||||
String footersAfter =
|
||||
decode(
|
||||
encodingAfter, rawAfter, rangeAfter.get().changeMessageEnd() + 1, rawAfter.length);
|
||||
assertThat(footersBefore).isEqualTo(footersAfter);
|
||||
|
||||
String message =
|
||||
decode(
|
||||
encodingAfter,
|
||||
rawAfter,
|
||||
rangeAfter.get().changeMessageStart(),
|
||||
rangeAfter.get().changeMessageEnd() + 1);
|
||||
assertThat(message).isEqualTo(createNewChangeMessage(deletedBy.fullName, deleteReason));
|
||||
} else {
|
||||
assertThat(commitAfter.getFullMessage()).isEqualTo(commitBefore.getFullMessage());
|
||||
}
|
||||
|
||||
assertThat(commitAfter.getCommitterIdent().getName())
|
||||
.isEqualTo(commitBefore.getCommitterIdent().getName());
|
||||
assertThat(commitAfter.getAuthorIdent().getName())
|
||||
.isEqualTo(commitBefore.getAuthorIdent().getName());
|
||||
assertThat(commitAfter.getEncoding()).isEqualTo(commitBefore.getEncoding());
|
||||
assertThat(commitAfter.getEncodingName()).isEqualTo(commitBefore.getEncodingName());
|
||||
}
|
||||
}
|
||||
|
||||
/** Verifies comments are not changed after deleting change message(s). */
|
||||
private void assertCommentsAfterDeletion(int changeNum, List<CommentInfo> commentsBeforeDeletion)
|
||||
throws Exception {
|
||||
List<CommentInfo> commentsAfterDeletion = getChangeSortedComments(changeNum);
|
||||
assertThat(commentsAfterDeletion).containsExactlyElementsIn(commentsBeforeDeletion).inOrder();
|
||||
}
|
||||
|
||||
private static void assertMessage(String expected, String actual) {
|
||||
assertThat(actual).isEqualTo("Patch Set 1:\n\n" + expected);
|
||||
}
|
||||
|
||||
@ -47,7 +47,6 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
||||
import com.google.gerrit.extensions.restapi.TopLevelResource;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.client.Patch;
|
||||
import com.google.gerrit.reviewdb.client.RefNames;
|
||||
import com.google.gerrit.server.change.ChangeResource;
|
||||
import com.google.gerrit.server.change.RevisionResource;
|
||||
import com.google.gerrit.server.notedb.ChangeNoteUtil;
|
||||
@ -58,26 +57,21 @@ import com.google.gerrit.testing.FakeEmailSender;
|
||||
import com.google.gerrit.testing.FakeEmailSender.Message;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
import java.io.IOException;
|
||||
import java.sql.Timestamp;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.Collection;
|
||||
import java.util.Comparator;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Map.Entry;
|
||||
import java.util.Optional;
|
||||
import java.util.function.Supplier;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import org.eclipse.jgit.lib.ObjectReader;
|
||||
import org.eclipse.jgit.lib.Ref;
|
||||
import org.eclipse.jgit.lib.Repository;
|
||||
import org.eclipse.jgit.notes.NoteMap;
|
||||
import org.eclipse.jgit.revwalk.RevCommit;
|
||||
import org.eclipse.jgit.revwalk.RevWalk;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
@ -838,7 +832,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
CommentInput c9 = newComment("b.txt", "comment 9");
|
||||
addComments(changeId, ps2, c9);
|
||||
|
||||
List<CommentInfo> commentsBeforeDelete = getChangeSortedComments(changeId);
|
||||
List<CommentInfo> commentsBeforeDelete = getChangeSortedComments(id.get());
|
||||
assertThat(commentsBeforeDelete).hasSize(9);
|
||||
// PS1 has comments [c1, c2, c3, c4, c5].
|
||||
assertThat(getRevisionComments(changeId, ps1)).hasSize(5);
|
||||
@ -853,7 +847,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
for (int i = 0; i < commentsBeforeDelete.size(); i++) {
|
||||
List<RevCommit> commitsBeforeDelete = new ArrayList<>();
|
||||
if (notesMigration.commitChangeWrites()) {
|
||||
commitsBeforeDelete = getCommits(id);
|
||||
commitsBeforeDelete = getChangeMetaCommitsInReverseOrder(id);
|
||||
}
|
||||
|
||||
CommentInfo comment = commentsBeforeDelete.get(i);
|
||||
@ -879,7 +873,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
|
||||
comment.message = expectedMsg;
|
||||
commentsBeforeDelete.set(i, comment);
|
||||
List<CommentInfo> commentsAfterDelete = getChangeSortedComments(changeId);
|
||||
List<CommentInfo> commentsAfterDelete = getChangeSortedComments(id.get());
|
||||
assertThat(commentsAfterDelete).isEqualTo(commentsBeforeDelete);
|
||||
}
|
||||
|
||||
@ -893,7 +887,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
addComments(changeId, ps3, c12);
|
||||
addComments(changeId, ps4, c13);
|
||||
|
||||
assertThat(getChangeSortedComments(changeId)).hasSize(13);
|
||||
assertThat(getChangeSortedComments(id.get())).hasSize(13);
|
||||
assertThat(getRevisionComments(changeId, ps1)).hasSize(6);
|
||||
assertThat(getRevisionComments(changeId, ps2)).hasSize(3);
|
||||
assertThat(getRevisionComments(changeId, ps3)).hasSize(1);
|
||||
@ -914,7 +908,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
addComments(changeId, ps1, c2);
|
||||
addComments(changeId, ps1, c3);
|
||||
|
||||
List<CommentInfo> commentsBeforeDelete = getChangeSortedComments(changeId);
|
||||
List<CommentInfo> commentsBeforeDelete = getChangeSortedComments(id.get());
|
||||
assertThat(commentsBeforeDelete).hasSize(3);
|
||||
Optional<CommentInfo> targetComment =
|
||||
commentsBeforeDelete.stream().filter(c -> c.message.equals("comment 2")).findFirst();
|
||||
@ -924,7 +918,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
|
||||
List<RevCommit> commitsBeforeDelete = new ArrayList<>();
|
||||
if (notesMigration.commitChangeWrites()) {
|
||||
commitsBeforeDelete = getCommits(id);
|
||||
commitsBeforeDelete = getChangeMetaCommitsInReverseOrder(id);
|
||||
}
|
||||
|
||||
setApiUser(admin);
|
||||
@ -944,7 +938,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
if (notesMigration.commitChangeWrites()) {
|
||||
assertMetaBranchCommitsAfterRewriting(commitsBeforeDelete, id, uuid, expectedMsg);
|
||||
}
|
||||
assertThat(getChangeSortedComments(changeId)).hasSize(3);
|
||||
assertThat(getChangeSortedComments(id.get())).hasSize(3);
|
||||
}
|
||||
|
||||
@Test
|
||||
@ -962,19 +956,6 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
assertThat(comment.legacyFormat).isFalse();
|
||||
}
|
||||
|
||||
private List<CommentInfo> getChangeSortedComments(String changeId) throws Exception {
|
||||
List<CommentInfo> comments = new ArrayList<>();
|
||||
Map<String, List<CommentInfo>> commentsMap = getPublishedComments(changeId);
|
||||
for (Entry<String, List<CommentInfo>> e : commentsMap.entrySet()) {
|
||||
for (CommentInfo c : e.getValue()) {
|
||||
c.path = e.getKey(); // Set the comment's path field.
|
||||
comments.add(c);
|
||||
}
|
||||
}
|
||||
comments.sort(Comparator.comparing(c -> c.id));
|
||||
return comments;
|
||||
}
|
||||
|
||||
private List<CommentInfo> getRevisionComments(String changeId, String revId) throws Exception {
|
||||
return getPublishedComments(changeId, revId)
|
||||
.values()
|
||||
@ -998,15 +979,6 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
gApi.changes().id(changeId).revision(revision).review(input);
|
||||
}
|
||||
|
||||
private List<RevCommit> getCommits(Change.Id changeId) throws IOException {
|
||||
try (Repository repo = repoManager.openRepository(project);
|
||||
RevWalk revWalk = new RevWalk(repo)) {
|
||||
Ref metaRef = repo.exactRef(RefNames.changeMetaRef(changeId));
|
||||
revWalk.markStart(revWalk.parseCommit(metaRef.getObjectId()));
|
||||
return Lists.newArrayList(revWalk);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* All the commits, which contain the target comment before, should still contain the comment with
|
||||
* the updated message. All the other metas of the commits should be exactly the same.
|
||||
@ -1017,7 +989,7 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
String targetCommentUuid,
|
||||
String expectedMessage)
|
||||
throws Exception {
|
||||
List<RevCommit> afterDelete = getCommits(changeId);
|
||||
List<RevCommit> afterDelete = getChangeMetaCommitsInReverseOrder(changeId);
|
||||
assertThat(afterDelete).hasSize(beforeDelete.size());
|
||||
|
||||
try (Repository repo = repoManager.openRepository(project);
|
||||
@ -1119,10 +1091,6 @@ public class CommentsIT extends AbstractDaemonTest {
|
||||
return gApi.changes().id(changeId).revision(revId).drafts();
|
||||
}
|
||||
|
||||
private Map<String, List<CommentInfo>> getPublishedComments(String changeId) throws Exception {
|
||||
return gApi.changes().id(changeId).comments();
|
||||
}
|
||||
|
||||
private CommentInfo getDraftComment(String changeId, String revId, String uuid) throws Exception {
|
||||
return gApi.changes().id(changeId).revision(revId).draft(uuid).get();
|
||||
}
|
||||
|
||||
Loading…
Reference in New Issue
Block a user