Support to delete a comment with 'NoteDbRewriter' in BatchUpdate

This change adds a new interface 'NoteDbRewriter', which can be used to
rewrite the NoteDb commit history in BatchUpdate.

By implementing the new interface, this change allows users with
administrate server capability to delete a comment by replacing the
comment's message.

To delete a whole comment, the rewritter is required to update the
commit message, which contains the number of the comments put in by the
original commit. This can be done in later changes when necessary.

Bug: Issue 4445
Change-Id: Icaeb3c24f5dc88f6b44b1297366a692a32676910
This commit is contained in:
Changcheng Xiao
2017-02-10 09:39:48 +01:00
parent 368c922dfc
commit e5b14cebd5
15 changed files with 857 additions and 6 deletions

View File

@@ -4394,6 +4394,62 @@ describes the published comment.
}
----
[[delete-comment]]
=== Delete Comment
--
'DELETE /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/comments/link:#comment-id[\{comment-id\}]' +
'POST /changes/link:#change-id[\{change-id\}]/revisions/link:#revision-id[\{revision-id\}]/comments/link:#comment-id[\{comment-id\}]/delete'
--
Deletes a published comment of a revision. Instead of deleting the
whole comment, this endpoint just replaces the comment's message
with a new message, which contains the name of the user who deletes
the comment and the reason why it's deleted. The reason can be
provided in the request body as a
link:#delete-comment-input[DeleteCommentInput] entity.
Note that only users with the
link:access-control.html#capability_administrateServer[Administrate Server]
global capability are permitted to delete a comment.
Please note that some proxies prohibit request bodies for DELETE
requests. In this case, if you want to specify options, use a
POST request:
.Request
----
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/comments/TvcXrmjM/delete HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
"reason": "contains confidential information"
}
----
As response a link:#comment-info[CommentInfo] entity is returned that
describes the updated comment.
.Response
----
HTTP/1.1 200 OK
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
)]}'
{
"id": "TvcXrmjM",
"path": "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java",
"line": 23,
"message": "Comment removed by: Administrator; Reason: contains confidential information",
"updated": "2013-02-26 15:40:43.986000000",
"author": {
"_account_id": 1000096,
"name": "John Doe",
"email": "john.doe@example.com"
}
}
----
[[list-robot-comments]]
=== List Robot Comments
--
@@ -5739,6 +5795,20 @@ Links to the commit in external sites as a list of
link:#web-link-info[WebLinkInfo] entities.
|===========================
[[delete-comment-input]]
=== DeleteCommentInput
The `DeleteCommentInput` entity contains the option for deleting a comment.
[options="header",cols="1,^1,5"]
|=============================
|Field Name ||Description
|`reason` |optional|
The reason why the comment should be deleted. +
If set, the comment's message will be replaced with
"Comment removed by: `name`; Reason: `reason`",
or just "Comment removed by: `name`." if not set.
|=============================
[[delete-reviewer-input]]
=== DeleteReviewerInput
The `DeleteReviewerInput` entity contains options for the deletion of a

View File

@@ -20,12 +20,14 @@ import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
import com.google.common.base.Function;
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.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
@@ -34,27 +36,41 @@ import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
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.ChangesCollection;
import com.google.gerrit.server.change.PostReview;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.notedb.ChangeNoteUtil;
import com.google.gerrit.server.notedb.DeleteCommentRewriter;
import com.google.gerrit.testutil.FakeEmailSender;
import com.google.gerrit.testutil.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.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.eclipse.jgit.errors.ConfigInvalidException;
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;
@@ -67,6 +83,8 @@ public class CommentsIT extends AbstractDaemonTest {
@Inject private FakeEmailSender email;
@Inject private ChangeNoteUtil noteUtil;
private final Integer[] lines = {0, 1};
@Before
@@ -739,6 +757,168 @@ public class CommentsIT extends AbstractDaemonTest {
}
}
@Test
public void deleteCommentCannotBeAppliedByUser() throws Exception {
PushOneCommit.Result result = createChange();
CommentInput targetComment = addComment(result.getChangeId(), "My password: abc123");
Map<String, List<CommentInfo>> commentsMap =
getPublishedComments(result.getChangeId(), result.getCommit().name());
assertThat(commentsMap.size()).isEqualTo(1);
assertThat(commentsMap.get(FILE_NAME)).hasSize(1);
String uuid = commentsMap.get(targetComment.path).get(0).id;
DeleteCommentInput input = new DeleteCommentInput("contains confidential information");
setApiUser(user);
exception.expect(AuthException.class);
gApi.changes().id(result.getChangeId()).current().comment(uuid).delete(input);
}
@Test
public void deleteCommentByRewritingCommitHistory() throws Exception {
// Create change (the 1st commit on the change's meta branch).
PushOneCommit.Result result = createChange();
String changeId = result.getChangeId();
Change.Id id = result.getChange().getId();
// Add two comments in patch set 1 (the 2nd commit on the change's meta branch).
ReviewInput reviewInput = new ReviewInput();
CommentInput comment1 = newComment(FILE_NAME, Side.REVISION, 0, "My password: abc123", false);
CommentInput comment2 = newComment(FILE_NAME, Side.REVISION, 1, "nit: long line", false);
reviewInput.comments = ImmutableMap.of(FILE_NAME, Lists.newArrayList(comment1, comment2));
reviewInput.label("Code-Review", 1);
gApi.changes().id(changeId).current().review(reviewInput);
// Create patch set 2 (the 3rd commit on the change's meta branch).
amendChange(changeId);
// Add 'comment3' in patch set 2 (the 4th commit on the change's meta branch).
CommentInput comment3 = addComment(changeId, "typo");
Map<String, List<CommentInfo>> commentsMap = getPublishedComments(changeId);
assertThat(commentsMap).hasSize(1);
assertThat(commentsMap.get(FILE_NAME)).hasSize(3);
Optional<CommentInfo> targetCommentInfo =
commentsMap
.get(FILE_NAME)
.stream()
.filter(c -> c.message.equals("My password: abc123"))
.findFirst();
assertThat(targetCommentInfo.isPresent()).isTrue();
List<RevCommit> commitsBeforeDelete = new ArrayList<>();
if (notesMigration.commitChangeWrites()) {
commitsBeforeDelete = getCommits(id);
}
String uuid = targetCommentInfo.get().id;
// Get the target comment.
CommentInfo oldComment =
gApi.changes().id(changeId).revision(result.getCommit().getName()).comment(uuid).get();
// Delete the target comment.
DeleteCommentInput input = new DeleteCommentInput("contains confidential information");
setApiUser(admin);
CommentInfo updatedComment =
gApi.changes()
.id(changeId)
.revision(result.getCommit().getName())
.comment(uuid)
.delete(input);
String expectedMsg =
String.format("Comment removed by: %s; Reason: %s", admin.fullName, input.reason);
assertThat(updatedComment.message).isEqualTo(expectedMsg);
updatedComment.message = oldComment.message;
assertThat(updatedComment).isEqualTo(oldComment);
// Check the comment's message has been replaced in NoteDb.
if (notesMigration.commitChangeWrites()) {
assertMetaBranchCommitsAfterRewriting(commitsBeforeDelete, id, uuid, expectedMsg);
}
// Make sure that comments can still be added correctly.
CommentInput comment4 = addComment(changeId, "too much space");
commentsMap = getPublishedComments(changeId);
assertThat(commentsMap).hasSize(1);
List<CommentInput> comments =
Lists.transform(commentsMap.get(FILE_NAME), infoToInput(FILE_NAME));
// Change comment1's message to the expected message.
comment1.message = expectedMsg;
assertThat(comments).containsExactly(comment1, comment2, comment3, comment4);
}
private CommentInput addComment(String changeId, String message) throws Exception {
ReviewInput input = new ReviewInput();
CommentInput comment = newComment(FILE_NAME, Side.REVISION, 0, message, false);
input.comments = ImmutableMap.of(comment.path, Lists.newArrayList(comment));
gApi.changes().id(changeId).current().review(input);
return comment;
}
private List<RevCommit> getCommits(Change.Id changeId)
throws IOException, ConfigInvalidException {
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.
*/
private void assertMetaBranchCommitsAfterRewriting(
List<RevCommit> beforeDelete,
Change.Id changeId,
String targetCommentUuid,
String expectedMessage)
throws Exception {
List<RevCommit> afterDelete = getCommits(changeId);
assertThat(afterDelete).hasSize(beforeDelete.size());
try (Repository repo = repoManager.openRepository(project);
ObjectReader reader = repo.newObjectReader()) {
for (int i = 0; i < beforeDelete.size(); i++) {
RevCommit commitBefore = beforeDelete.get(i);
RevCommit commitAfter = afterDelete.get(i);
Map<String, com.google.gerrit.reviewdb.client.Comment> commentMapBefore =
DeleteCommentRewriter.getPublishedComments(
noteUtil, changeId, reader, NoteMap.read(reader, commitBefore));
Map<String, com.google.gerrit.reviewdb.client.Comment> commentMapAfter =
DeleteCommentRewriter.getPublishedComments(
noteUtil, changeId, reader, NoteMap.read(reader, commitAfter));
if (commentMapBefore.containsKey(targetCommentUuid)) {
assertThat(commentMapAfter).containsKey(targetCommentUuid);
com.google.gerrit.reviewdb.client.Comment comment =
commentMapAfter.get(targetCommentUuid);
assertThat(comment.message).isEqualTo(expectedMessage);
comment.message = commentMapBefore.get(targetCommentUuid).message;
commentMapAfter.put(targetCommentUuid, comment);
assertThat(commentMapAfter).isEqualTo(commentMapBefore);
} else {
assertThat(commentMapAfter).doesNotContainKey(targetCommentUuid);
}
// Other metas should be exactly the same.
assertThat(commitAfter.getFullMessage()).isEqualTo(commitBefore.getFullMessage());
assertThat(commitAfter.getCommitterIdent()).isEqualTo(commitBefore.getCommitterIdent());
assertThat(commitAfter.getAuthorIdent()).isEqualTo(commitBefore.getAuthorIdent());
assertThat(commitAfter.getEncoding()).isEqualTo(commitBefore.getEncoding());
assertThat(commitAfter.getEncodingName()).isEqualTo(commitBefore.getEncodingName());
}
}
}
private static String extractComments(String msg) {
// Extract lines between start "....." and end "-- ".
Pattern p = Pattern.compile(".*[.]{5}\n+(.*)\\n+-- \n.*", Pattern.DOTALL);
@@ -803,6 +983,10 @@ 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();
}

View File

@@ -21,6 +21,17 @@ import com.google.gerrit.extensions.restapi.RestApiException;
public interface CommentApi {
CommentInfo get() throws RestApiException;
/**
* Deletes a published comment of a revision. For NoteDb, it deletes the comment by rewriting the
* commit history.
*
* <p>Note instead of deleting the whole comment, this endpoint just replaces the comment's
* message.
*
* @return the comment with its message updated.
*/
CommentInfo delete(DeleteCommentInput input) throws RestApiException;
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -30,5 +41,10 @@ public interface CommentApi {
public CommentInfo get() {
throw new NotImplementedException();
}
@Override
public CommentInfo delete(DeleteCommentInput input) {
throw new NotImplementedException();
}
}
}

View File

@@ -0,0 +1,30 @@
// Copyright (C) 2017 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.common.base.Strings;
import com.google.gerrit.extensions.restapi.DefaultInput;
public class DeleteCommentInput {
@DefaultInput public String reason;
public DeleteCommentInput() {
reason = "";
}
public DeleteCommentInput(String reason) {
this.reason = Strings.nullToEmpty(reason);
}
}

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.gerrit.reviewdb.client.PatchLineComment.Status.PUBLISHED;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
@@ -44,6 +45,7 @@ import com.google.gerrit.server.config.GerritServerId;
import com.google.gerrit.server.git.GitRepositoryManager;
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.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -223,8 +225,7 @@ public class CommentsUtil {
public List<Comment> publishedByChange(ReviewDb db, ChangeNotes notes) throws OrmException {
if (!migration.readChanges()) {
return sort(
byCommentStatus(db.patchComments().byChange(notes.getChangeId()), Status.PUBLISHED));
return sort(byCommentStatus(db.patchComments().byChange(notes.getChangeId()), PUBLISHED));
}
notes.load();
@@ -403,6 +404,25 @@ public class CommentsUtil {
.delete(toPatchLineComments(update.getId(), PatchLineComment.Status.DRAFT, comments));
}
public void deleteCommentByRewritingHistory(
ReviewDb db, ChangeUpdate update, Comment.Key commentKey, PatchSet.Id psId, String newMessage)
throws OrmException {
if (PrimaryStorage.of(update.getChange()).equals(PrimaryStorage.REVIEW_DB)) {
PatchLineComment.Key key =
new PatchLineComment.Key(new Patch.Key(psId, commentKey.filename), commentKey.uuid);
PatchLineComment patchLineComment = db.patchComments().get(key);
if (!patchLineComment.getStatus().equals(PUBLISHED)) {
throw new OrmException(String.format("comment %s is not published", key));
}
patchLineComment.setMessage(newMessage);
db.patchComments().upsert(Collections.singleton(patchLineComment));
}
update.deleteCommentByRewritingHistory(commentKey.uuid, newMessage);
}
public void deleteAllDraftsFromAllUsers(Change.Id changeId) throws IOException {
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
@@ -533,7 +553,7 @@ public class CommentsUtil {
ctx.getUser().updateRealAccountId(d::setRealAuthor);
setCommentRevId(d, patchListCache, notes.getChange(), ps);
}
putComments(ctx.getDb(), ctx.getUpdate(psId), PatchLineComment.Status.PUBLISHED, drafts);
putComments(ctx.getDb(), ctx.getUpdate(psId), PUBLISHED, drafts);
}
private static PatchSet.Id psId(ChangeNotes notes, Comment c) {

View File

@@ -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.CommentApi;
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.change.CommentResource;
import com.google.gerrit.server.change.DeleteComment;
import com.google.gerrit.server.change.GetComment;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -30,11 +32,14 @@ class CommentApiImpl implements CommentApi {
}
private final GetComment getComment;
private final DeleteComment deleteComment;
private final CommentResource comment;
@Inject
CommentApiImpl(GetComment getComment, @Assisted CommentResource comment) {
CommentApiImpl(
GetComment getComment, DeleteComment deleteComment, @Assisted CommentResource comment) {
this.getComment = getComment;
this.deleteComment = deleteComment;
this.comment = comment;
}
@@ -46,4 +51,13 @@ class CommentApiImpl implements CommentApi {
throw asRestApiException("Cannot retrieve comment", e);
}
}
@Override
public CommentInfo delete(DeleteCommentInput input) throws RestApiException {
try {
return deleteComment.apply(comment, input);
} catch (Exception e) {
throw asRestApiException("Cannot delete comment", e);
}
}
}

View File

@@ -16,9 +16,11 @@ package com.google.gerrit.server.api.changes;
import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
import com.google.gerrit.extensions.api.changes.DraftApi;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.change.DeleteDraftComment;
import com.google.gerrit.server.change.DraftCommentResource;
@@ -75,4 +77,9 @@ class DraftApiImpl implements DraftApi {
throw asRestApiException("Cannot delete draft", e);
}
}
@Override
public CommentInfo delete(DeleteCommentInput input) {
throw new NotImplementedException();
}
}

View File

@@ -48,4 +48,8 @@ public class CommentResource implements RestResource {
Account.Id getAuthorId() {
return comment.author.getId();
}
RevisionResource getRevisionResource() {
return rev;
}
}

View File

@@ -0,0 +1,145 @@
// Copyright (C) 2017 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.change;
import com.google.common.base.Strings;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.DeleteCommentInput;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
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.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;
import java.util.Optional;
import org.eclipse.jgit.errors.ConfigInvalidException;
@Singleton
public class DeleteComment implements RestModifyView<CommentResource, DeleteCommentInput> {
private final Provider<CurrentUser> userProvider;
private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
private final BatchUpdate.Factory batchUpdateFactory;
private final CommentsUtil commentsUtil;
private final PatchSetUtil psUtil;
private final BatchUpdate.Factory updateFactory;
private final PatchListCache patchListCache;
private final Provider<CommentJson> commentJson;
private final ChangeNotes.Factory notesFactory;
@Inject
public DeleteComment(
Provider<CurrentUser> userProvider,
Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
BatchUpdate.Factory batchUpdateFactory,
CommentsUtil commentsUtil,
PatchSetUtil psUtil,
BatchUpdate.Factory updateFactory,
PatchListCache patchListCache,
Provider<CommentJson> commentJson,
ChangeNotes.Factory notesFactory) {
this.userProvider = userProvider;
this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
this.batchUpdateFactory = batchUpdateFactory;
this.commentsUtil = commentsUtil;
this.psUtil = psUtil;
this.updateFactory = updateFactory;
this.patchListCache = patchListCache;
this.commentJson = commentJson;
this.notesFactory = notesFactory;
}
@Override
public CommentInfo apply(CommentResource rsrc, DeleteCommentInput input)
throws RestApiException, IOException, ConfigInvalidException, OrmException,
PermissionBackendException, UpdateException {
CurrentUser user = userProvider.get();
permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
String newMessage = getCommentNewMessage(user.asIdentifiedUser().getName(), input.reason);
DeleteCommentOp deleteCommentOp = new DeleteCommentOp(rsrc, newMessage);
try (BatchUpdate batchUpdate =
batchUpdateFactory.create(
dbProvider.get(), rsrc.getRevisionResource().getProject(), user, TimeUtil.nowTs())) {
batchUpdate.addOp(rsrc.getRevisionResource().getChange().getId(), deleteCommentOp).execute();
}
ChangeNotes updatedNotes =
notesFactory.createChecked(rsrc.getRevisionResource().getChange().getId());
List<Comment> changeComments = commentsUtil.publishedByChange(dbProvider.get(), updatedNotes);
Optional<Comment> updatedComment =
changeComments.stream().filter(c -> c.key.equals(rsrc.getComment().key)).findFirst();
if (!updatedComment.isPresent()) {
// This should not happen as this endpoint should not remove the whole comment.
throw new ResourceNotFoundException("comment not found: " + rsrc.getComment().key);
}
return commentJson.get().newCommentFormatter().format(updatedComment.get());
}
private static String getCommentNewMessage(String name, String reason) {
StringBuilder stringBuilder = new StringBuilder("Comment removed by: ").append(name);
if (!Strings.isNullOrEmpty(reason)) {
stringBuilder.append("; Reason: ").append(reason);
}
return stringBuilder.toString();
}
private class DeleteCommentOp implements BatchUpdateOp {
private final CommentResource rsrc;
private final String newMessage;
DeleteCommentOp(CommentResource rsrc, String newMessage) {
this.rsrc = rsrc;
this.newMessage = newMessage;
}
@Override
public boolean updateChange(ChangeContext ctx)
throws ResourceConflictException, OrmException, ResourceNotFoundException {
PatchSet.Id psId = ctx.getChange().currentPatchSetId();
commentsUtil.deleteCommentByRewritingHistory(
ctx.getDb(),
ctx.getUpdate(psId),
rsrc.getComment().key,
rsrc.getPatchSet().getId(),
newMessage);
return true;
}
}
}

View File

@@ -136,6 +136,8 @@ public class Module extends RestApiModule {
child(REVISION_KIND, "comments").to(Comments.class);
get(COMMENT_KIND).to(GetComment.class);
delete(COMMENT_KIND).to(DeleteComment.class);
post(COMMENT_KIND, "delete").to(DeleteComment.class);
child(REVISION_KIND, "robotcomments").to(RobotComments.class);
get(ROBOT_COMMENT_KIND).to(GetRobotComment.class);

View File

@@ -124,9 +124,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
}
private final AccountCache accountCache;
private final NoteDbUpdateManager.Factory updateManagerFactory;
private final ChangeDraftUpdate.Factory draftUpdateFactory;
private final RobotCommentUpdate.Factory robotCommentUpdateFactory;
private final NoteDbUpdateManager.Factory updateManagerFactory;
private final DeleteCommentRewriter.Factory deleteCommentRewriterFactory;
private final Table<String, Account.Id, Optional<Short>> approvals;
private final Map<Account.Id, ReviewerStateInternal> reviewers = new LinkedHashMap<>();
@@ -158,6 +159,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
private ChangeDraftUpdate draftUpdate;
private RobotCommentUpdate robotCommentUpdate;
private DeleteCommentRewriter deleteCommentRewriter;
@AssistedInject
private ChangeUpdate(
@@ -169,6 +171,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
NoteDbUpdateManager.Factory updateManagerFactory,
ChangeDraftUpdate.Factory draftUpdateFactory,
RobotCommentUpdate.Factory robotCommentUpdateFactory,
DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
ProjectCache projectCache,
@Assisted ChangeControl ctl,
ChangeNoteUtil noteUtil) {
@@ -181,6 +184,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
updateManagerFactory,
draftUpdateFactory,
robotCommentUpdateFactory,
deleteCommentRewriterFactory,
projectCache,
ctl,
serverIdent.getWhen(),
@@ -197,6 +201,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
NoteDbUpdateManager.Factory updateManagerFactory,
ChangeDraftUpdate.Factory draftUpdateFactory,
RobotCommentUpdate.Factory robotCommentUpdateFactory,
DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
ProjectCache projectCache,
@Assisted ChangeControl ctl,
@Assisted Date when,
@@ -210,6 +215,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
updateManagerFactory,
draftUpdateFactory,
robotCommentUpdateFactory,
deleteCommentRewriterFactory,
ctl,
when,
projectCache.get(getProjectName(ctl)).getLabelTypes().nameComparator(),
@@ -235,15 +241,17 @@ public class ChangeUpdate extends AbstractChangeUpdate {
NoteDbUpdateManager.Factory updateManagerFactory,
ChangeDraftUpdate.Factory draftUpdateFactory,
RobotCommentUpdate.Factory robotCommentUpdateFactory,
DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
@Assisted ChangeControl ctl,
@Assisted Date when,
@Assisted Comparator<String> labelNameComparator,
ChangeNoteUtil noteUtil) {
super(cfg, migration, ctl, serverIdent, anonymousCowardName, noteUtil, when);
this.accountCache = accountCache;
this.updateManagerFactory = updateManagerFactory;
this.draftUpdateFactory = draftUpdateFactory;
this.robotCommentUpdateFactory = robotCommentUpdateFactory;
this.updateManagerFactory = updateManagerFactory;
this.deleteCommentRewriterFactory = deleteCommentRewriterFactory;
this.approvals = approvals(labelNameComparator);
}
@@ -257,6 +265,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
NoteDbUpdateManager.Factory updateManagerFactory,
ChangeDraftUpdate.Factory draftUpdateFactory,
RobotCommentUpdate.Factory robotCommentUpdateFactory,
DeleteCommentRewriter.Factory deleteCommentRewriterFactory,
ChangeNoteUtil noteUtil,
@Assisted Change change,
@Assisted("effective") @Nullable Account.Id accountId,
@@ -280,6 +289,7 @@ public class ChangeUpdate extends AbstractChangeUpdate {
this.draftUpdateFactory = draftUpdateFactory;
this.robotCommentUpdateFactory = robotCommentUpdateFactory;
this.updateManagerFactory = updateManagerFactory;
this.deleteCommentRewriterFactory = deleteCommentRewriterFactory;
this.approvals = approvals(labelNameComparator);
}
@@ -394,6 +404,11 @@ public class ChangeUpdate extends AbstractChangeUpdate {
createDraftUpdateIfNull().deleteComment(c);
}
public void deleteCommentByRewritingHistory(String uuid, String newMessage) {
deleteCommentRewriter =
deleteCommentRewriterFactory.create(getChange().getId(), uuid, newMessage);
}
@VisibleForTesting
ChangeDraftUpdate createDraftUpdateIfNull() {
if (draftUpdate == null) {
@@ -596,6 +611,8 @@ 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");
CommitBuilder cb = new CommitBuilder();
int ps = psId != null ? psId.get() : getChange().currentPatchSetId().get();
@@ -798,6 +815,10 @@ public class ChangeUpdate extends AbstractChangeUpdate {
return robotCommentUpdate;
}
public DeleteCommentRewriter getDeleteCommentRewriter() {
return deleteCommentRewriter;
}
public void setAllowWriteToNewRef(boolean allow) {
isAllowWriteToNewtRef = allow;
}

View File

@@ -0,0 +1,246 @@
// Copyright (C) 2017 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.gerrit.reviewdb.client.PatchLineComment.Status.PUBLISHED;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Comment;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevSort;
import org.eclipse.jgit.revwalk.RevWalk;
/**
* Deletes a published comment from NoteDb by rewriting the commit history. Instead of deleting the
* whole comment, it just replaces the comment's message with a new message.
*/
public class DeleteCommentRewriter implements NoteDbRewriter {
public interface Factory {
/**
* Creates a DeleteCommentRewriter instance.
*
* @param id the id of the change which contains the target comment.
* @param uuid the uuid of the target comment.
* @param newMessage the message used to replace the old message of the target comment.
* @return
*/
DeleteCommentRewriter create(
Change.Id id, @Assisted("uuid") String uuid, @Assisted("newMessage") String newMessage);
}
private final ChangeNoteUtil noteUtil;
private final Change.Id changeId;
private final String uuid;
private final String newMessage;
@Inject
DeleteCommentRewriter(
ChangeNoteUtil noteUtil,
@Assisted Change.Id changeId,
@Assisted("uuid") String uuid,
@Assisted("newMessage") String newMessage) {
this.noteUtil = noteUtil;
this.changeId = changeId;
this.uuid = uuid;
this.newMessage = newMessage;
}
@Override
public String getRefName() {
return RefNames.changeMetaRef(changeId);
}
@Override
public ObjectId rewriteCommitHistory(RevWalk revWalk, ObjectInserter inserter, ObjectId currTip)
throws IOException, ConfigInvalidException, OrmException {
checkArgument(!currTip.equals(ObjectId.zeroId()));
// Walk from the first commit of the branch.
revWalk.reset();
revWalk.markStart(revWalk.parseCommit(currTip));
revWalk.sort(RevSort.REVERSE);
ObjectReader reader = revWalk.getObjectReader();
ObjectId newTip = revWalk.next(); // The first commit will not be rewritten.
NoteMap newTipNoteMap = NoteMap.read(reader, revWalk.parseCommit(newTip));
Map<String, Comment> parentComments =
getPublishedComments(noteUtil, changeId, reader, newTipNoteMap);
boolean rewrite = false;
RevCommit originalCommit;
while ((originalCommit = revWalk.next()) != null) {
NoteMap noteMap = NoteMap.read(reader, originalCommit);
Map<String, Comment> currComments = getPublishedComments(noteUtil, changeId, reader, noteMap);
if (!rewrite && currComments.containsKey(uuid)) {
rewrite = true;
}
if (!rewrite) {
parentComments = currComments;
newTip = originalCommit;
continue;
}
List<Comment> putInComments = getPutInComments(parentComments, currComments);
List<Comment> deletedComments = getDeletedComments(parentComments, currComments);
newTip =
rewriteCommit(
originalCommit,
newTipNoteMap,
newTip,
inserter,
reader,
putInComments,
deletedComments);
newTipNoteMap = NoteMap.read(reader, revWalk.parseCommit(newTip));
parentComments = currComments;
}
return newTip;
}
/**
* Gets all the comments which are presented at a commit. Note they include the comments put in by
* the previous commits.
*/
@VisibleForTesting
public static Map<String, Comment> getPublishedComments(
ChangeNoteUtil noteUtil, Change.Id changeId, ObjectReader reader, NoteMap noteMap)
throws IOException, ConfigInvalidException {
return RevisionNoteMap.parse(noteUtil, changeId, reader, noteMap, PUBLISHED)
.revisionNotes
.values()
.stream()
.flatMap(n -> n.getComments().stream())
.collect(Collectors.toMap(c -> c.key.uuid, c -> c));
}
/**
* Gets the comments put in by the current commit. The message of the target comment will be
* replaced by the new message.
*
* @param parMap the comment map of the parent commit.
* @param curMap the comment map of the current commit.
* @return The comments put in by the current commit.
*/
private List<Comment> getPutInComments(Map<String, Comment> parMap, Map<String, Comment> curMap) {
List<Comment> comments = new ArrayList<>();
for (String key : curMap.keySet()) {
if (!parMap.containsKey(key)) {
Comment comment = curMap.get(key);
if (key.equals(uuid)) {
comment.message = newMessage;
}
comments.add(comment);
}
}
return comments;
}
/**
* Gets the comments deleted by the current commit.
*
* @param parMap the comment map of the parent commit.
* @param curMap the comment map of the current commit.
* @return The comments deleted by the current commit.
*/
private List<Comment> getDeletedComments(
Map<String, Comment> parMap, Map<String, Comment> curMap) {
return parMap
.entrySet()
.stream()
.filter(c -> !curMap.containsKey(c.getKey()))
.map(c -> c.getValue())
.collect(toList());
}
/**
* Rewrites one commit.
*
* @param originalCommit the original commit to be rewritten.
* @param parentNoteMap the {@code NoteMap} of the new commit's parent.
* @param parentId the {@code ObjectId} of the new commit's parent.
* @param inserter the {@code ObjectInserter} for the rewrite process.
* @param reader the {@code ObjectReader} for the rewrite process.
* @param putInComments the comments put in by this commit.
* @param deletedComments the comments deleted by this commit.
* @return the {@code objectId} of the new commit.
* @throws IOException
* @throws ConfigInvalidException
*/
private ObjectId rewriteCommit(
RevCommit originalCommit,
NoteMap parentNoteMap,
ObjectId parentId,
ObjectInserter inserter,
ObjectReader reader,
List<Comment> putInComments,
List<Comment> deletedComments)
throws IOException, ConfigInvalidException {
RevisionNoteMap<ChangeRevisionNote> revNotesMap =
RevisionNoteMap.parse(noteUtil, changeId, reader, parentNoteMap, PUBLISHED);
RevisionNoteBuilder.Cache cache = new RevisionNoteBuilder.Cache(revNotesMap);
for (Comment c : putInComments) {
cache.get(new RevId(c.revId)).putComment(c);
}
for (Comment c : deletedComments) {
cache.get(new RevId(c.revId)).deleteComment(c.key);
}
Map<RevId, RevisionNoteBuilder> builders = cache.getBuilders();
for (Map.Entry<RevId, RevisionNoteBuilder> entry : builders.entrySet()) {
ObjectId objectId = ObjectId.fromString(entry.getKey().get());
byte[] data = entry.getValue().build(noteUtil, noteUtil.getWriteJson());
if (data.length == 0) {
revNotesMap.noteMap.remove(objectId);
}
revNotesMap.noteMap.set(objectId, inserter.insert(OBJ_BLOB, data));
}
CommitBuilder cb = new CommitBuilder();
cb.setParentId(parentId);
cb.setTreeId(revNotesMap.noteMap.writeTree(inserter));
cb.setMessage(originalCommit.getFullMessage());
cb.setCommitter(originalCommit.getCommitterIdent());
cb.setAuthor(originalCommit.getAuthorIdent());
cb.setEncoding(originalCommit.getEncoding());
return inserter.insert(cb);
}
}

View File

@@ -49,6 +49,7 @@ public class NoteDbModule extends FactoryModule {
public void configure() {
factory(ChangeUpdate.Factory.class);
factory(ChangeDraftUpdate.Factory.class);
factory(DeleteCommentRewriter.Factory.class);
factory(DraftCommentNotes.Factory.class);
factory(RobotCommentUpdate.Factory.class);
factory(RobotCommentNotes.Factory.class);

View File

@@ -0,0 +1,39 @@
// Copyright (C) 2017 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 com.google.gwtorm.server.OrmException;
import java.io.IOException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.revwalk.RevWalk;
public interface NoteDbRewriter {
/** Gets the name of the target ref which will be rewritten. */
String getRefName();
/**
* Rewrites the commit history.
*
* @param revWalk a {@code RevWalk} instance.
* @param inserter a {@code ObjectInserter} instance.
* @param currTip the {@code ObjectId} of the ref's tip commit.
* @return the {@code ObjectId} of the ref's new tip commit.
*/
ObjectId rewriteCommitHistory(RevWalk revWalk, ObjectInserter inserter, ObjectId currTip)
throws IOException, ConfigInvalidException, OrmException;
}

View File

@@ -54,6 +54,7 @@ import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.JGitText;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.NullProgressMonitor;
@@ -205,6 +206,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
private final ListMultimap<String, ChangeUpdate> changeUpdates;
private final ListMultimap<String, ChangeDraftUpdate> draftUpdates;
private final ListMultimap<String, RobotCommentUpdate> robotCommentUpdates;
private final ListMultimap<String, NoteDbRewriter> rewriters;
private final Set<Change.Id> toDelete;
private OpenRepo changeRepo;
@@ -232,6 +234,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
changeUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
draftUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
robotCommentUpdates = MultimapBuilder.hashKeys().arrayListValues().build();
rewriters = MultimapBuilder.hashKeys().arrayListValues().build();
toDelete = new HashSet<>();
}
@@ -344,6 +347,7 @@ public class NoteDbUpdateManager implements AutoCloseable {
return changeUpdates.isEmpty()
&& draftUpdates.isEmpty()
&& robotCommentUpdates.isEmpty()
&& rewriters.isEmpty()
&& toDelete.isEmpty()
&& !hasCommands(changeRepo)
&& !hasCommands(allUsersRepo);
@@ -377,6 +381,10 @@ public class NoteDbUpdateManager implements AutoCloseable {
if (rcu != null) {
robotCommentUpdates.put(rcu.getRefName(), rcu);
}
DeleteCommentRewriter deleteCommentRewriter = update.getDeleteCommentRewriter();
if (deleteCommentRewriter != null) {
rewriters.put(deleteCommentRewriter.getRefName(), deleteCommentRewriter);
}
}
public void add(ChangeDraftUpdate draftUpdate) {
@@ -603,6 +611,21 @@ public class NoteDbUpdateManager implements AutoCloseable {
if (!robotCommentUpdates.isEmpty()) {
addUpdates(robotCommentUpdates, changeRepo);
}
if (!rewriters.isEmpty()) {
Optional<String> conflictKey =
rewriters
.keySet()
.stream()
.filter(k -> (draftUpdates.containsKey(k) || robotCommentUpdates.containsKey(k)))
.findAny();
if (conflictKey.isPresent()) {
throw new IllegalArgumentException(
String.format(
"cannot update and rewrite ref %s in one BatchUpdate", conflictKey.get()));
}
addRewrites(rewriters, changeRepo);
}
for (Change.Id id : toDelete) {
doDelete(id);
}
@@ -723,6 +746,35 @@ public class NoteDbUpdateManager implements AutoCloseable {
}
}
private static void addRewrites(ListMultimap<String, NoteDbRewriter> rewriters, OpenRepo openRepo)
throws OrmException, IOException {
for (Map.Entry<String, Collection<NoteDbRewriter>> entry : rewriters.asMap().entrySet()) {
String refName = entry.getKey();
ObjectId oldTip = openRepo.cmds.get(refName).orElse(ObjectId.zeroId());
if (oldTip.equals(ObjectId.zeroId())) {
throw new OrmException(String.format("Ref %s is empty", refName));
}
ObjectId currTip = oldTip;
try {
for (NoteDbRewriter noteDbRewriter : entry.getValue()) {
ObjectId nextTip =
noteDbRewriter.rewriteCommitHistory(openRepo.rw, openRepo.tempIns, currTip);
if (nextTip != null) {
currTip = nextTip;
}
}
} catch (ConfigInvalidException e) {
throw new OrmException("Cannot rewrite commit history", e);
}
if (!oldTip.equals(currTip)) {
openRepo.cmds.add(new ReceiveCommand(oldTip, currTip, refName));
}
}
}
private static <U extends AbstractChangeUpdate> boolean allowWrite(
Collection<U> updates, ObjectId old) {
if (!old.equals(ObjectId.zeroId())) {