Merge "Modify draft comment tooltip text"
diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt
index 59ed6ff..998b143 100644
--- a/Documentation/cmd-review.txt
+++ b/Documentation/cmd-review.txt
@@ -129,9 +129,9 @@
-t::
Apply a 'TAG' to the change message, votes, and inline comments. The 'TAG'
can represent an external system like CI that does automated verification
- of the change. Comments with specific 'TAG' values can be filtered out in
- the web UI.
- Note that to apply different tags on on different votes/comments, multiple
+ of the change. Comments that contain TAG values with 'autogenerated:' prefix
+ can be filtered out in the web UI.
+ Note that to apply different tags on different votes/comments, multiple
invocations of the SSH command are required.
== ACCESS
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index cb3b88b..c144a09 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2475,7 +2475,66 @@
"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
}
----
@@ -4796,8 +4855,8 @@
GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/files/ HTTP/1.0
----
-As result a map is returned that maps the link:#file-id[file path] to a list of
-link:#file-info[FileInfo] entries. The entries in the map are
+As result a map is returned that maps the link:#file-id[file path] to a
+link:#file-info[FileInfo] entry. The entries in the map are
sorted by file path.
.Response
@@ -5589,8 +5648,9 @@
The time and date describing when the approval was made.
|`tag` |optional|
Value of the `tag` field from link:#review-input[ReviewInput] set
-while posting the review.
-NOTE: To apply different tags on on different votes/comments multiple
+while posting the review. Votes/comments that contain `tag` with
+'autogenerated:' prefix can be filtered out in the web UI.
+NOTE: To apply different tags on different votes/comments multiple
invocations of the REST call are required.
|`post_submit` |not set if `false`|
If true, this vote was made after the change was submitted.
@@ -5853,8 +5913,9 @@
|`message` ||The text left by the user.
|`tag` |optional|
Value of the `tag` field from link:#review-input[ReviewInput] set
-while posting the review.
-NOTE: To apply different tags on on different votes/comments multiple
+while posting the review. Votes/comments that contain `tag` with
+'autogenerated:' prefix can be filtered out in the web UI.
+NOTE: To apply different tags on different votes/comments multiple
invocations of the REST call are required.
|`_revision_number` |optional|
Which patchset (if any) generated this message.
@@ -5974,7 +6035,8 @@
|`tag` |optional, drafts only|
Value of the `tag` field. Only allowed on link:#create-draft[draft comment] +
inputs; for published comments, use the `tag` field in +
-link#review-input[ReviewInput]
+link#review-input[ReviewInput]. Votes/comments that contain `tag` with
+'autogenerated:' prefix can be filtered out in the web UI.
|`unresolved` |optional|
Whether or not the comment must be addressed by the user. This value will
default to false if the comment is an orphan, or the value of the `in_reply_to`
@@ -6049,6 +6111,20 @@
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.
@@ -6792,8 +6868,8 @@
|`tag` |optional|
Apply this tag to the review comment message, votes, and inline
comments. Tags may be used by CI or other automated systems to
-distinguish them from human reviews. Comments with specific tag
-values can be filtered out in the web UI.
+distinguish them from human reviews. Votes/comments that contain `tag` with
+'autogenerated:' prefix can be filtered out in the web UI.
|`labels` |optional|
The votes that should be added to the revision as a map that maps the
label names to the voting values.
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index f32480b..5fd8be4 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -2525,6 +2525,51 @@
}
----
+[[list-files]]
+=== List Files
+--
+'GET /projects/link:#project-name[\{project-name\}]/commits/link:#commit-id[\{commit-id\}]/files/'
+--
+
+Lists the files that were modified, added or deleted in a commit.
+
+.Request
+----
+ GET /projects/work%2Fmy-project/commits/a8a477efffbbf3b44169bb9a1d3a334cbbd9aa96/files/ HTTP/1.0
+----
+
+As result a map is returned that maps the link:rest-api-changes.html#file-id[file path] to a
+link:rest-api-changes.html#file-info[FileInfo] entry. The entries in the map are
+sorted by file path.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "/COMMIT_MSG": {
+ "status": "A",
+ "lines_inserted": 7,
+ "size_delta": 551,
+ "size": 551
+ },
+ "gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java": {
+ "lines_inserted": 5,
+ "lines_deleted": 3,
+ "size_delta": 98,
+ "size": 23348
+ }
+ }
+----
+
+The integer-valued request parameter `parent` changes the response to return a
+list of the files which are different in this commit compared to the given
+parent commit. This is useful for supporting review of merge commits. The value
+is the 1-based index of the parent's position in the commit object.
+
[[dashboard-endpoints]]
== Dashboard Endpoints
diff --git a/WORKSPACE b/WORKSPACE
index f255ac7..4491b12 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -680,30 +680,36 @@
sha1 = "4785a3c21320980282f9f33d0d1264a69040538f",
)
-TRUTH_VERS = "0.40"
+TRUTH_VERS = "0.42"
maven_jar(
name = "truth",
artifact = "com.google.truth:truth:" + TRUTH_VERS,
- sha1 = "0d74e716afec045cc4a178dbbfde2a8314ae5574",
+ sha1 = "b5768f644b114e6cf5c3962c2ebcb072f788dcbb",
)
maven_jar(
name = "truth-java8-extension",
artifact = "com.google.truth.extensions:truth-java8-extension:" + TRUTH_VERS,
- sha1 = "636e49d675bc28e0b3ae0edd077d6acbbb159166",
+ sha1 = "4d01dfa5b3780632a3d109e14e101f01d10cce2c",
)
maven_jar(
name = "truth-liteproto-extension",
artifact = "com.google.truth.extensions:truth-liteproto-extension:" + TRUTH_VERS,
- sha1 = "21210ac07e5cfbe83f04733f806224a6c0ae4d2d",
+ sha1 = "c231e6735aa6c133c7e411ae1c1c90b124900a8b",
)
maven_jar(
name = "truth-proto-extension",
artifact = "com.google.truth.extensions:truth-proto-extension:" + TRUTH_VERS,
- sha1 = "5a2b504143a5fec2b6be8bce292b3b7577a81789",
+ sha1 = "c41d22e8b4a61b4171e57c44a2959ebee0091a14",
+)
+
+maven_jar(
+ name = "diffutils",
+ artifact = "com.googlecode.java-diff-utils:diffutils:1.3.0",
+ sha1 = "7e060dd5b19431e6d198e91ff670644372f60fbd",
)
# When bumping the easymock version number, make sure to also move powermock to a compatible version
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 960b558..1d87880 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -33,6 +33,7 @@
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.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.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.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.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 @@
}
}
}
+
+ 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;
+ }
}
diff --git a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
index f2d0f70..df57c27 100644
--- a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance;
+import static com.google.common.truth.Fact.fact;
import static com.google.common.truth.Truth.assertAbout;
import static com.google.gerrit.extensions.api.changes.RecipientType.BCC;
import static com.google.gerrit.extensions.api.changes.RecipientType.CC;
@@ -100,7 +101,7 @@
public FakeEmailSenderSubject notSent() {
if (actual().peekMessage() != null) {
- fail("a message wasn't sent");
+ failWithoutActual(fact("expected message", "sent"));
}
return this;
}
@@ -108,7 +109,7 @@
public FakeEmailSenderSubject sent(String messageType, StagedUsers users) {
message = actual().nextMessage();
if (message == null) {
- fail("a message was sent");
+ failWithoutActual(fact("expected message", "not sent"));
}
recipients = new HashMap<>();
recipients.put(TO, parseAddresses(message, "To"));
@@ -123,11 +124,12 @@
.collect(toList()));
this.users = users;
if (!message.headers().containsKey("X-Gerrit-MessageType")) {
- fail("a message was sent with X-Gerrit-MessageType header");
+ failWithoutActual(
+ fact("expected to have message sent with", "X-Gerrit-MessageType header"));
}
EmailHeader header = message.headers().get("X-Gerrit-MessageType");
if (!header.equals(new EmailHeader.String(messageType))) {
- fail("message of type " + messageType + " was sent; X-Gerrit-MessageType is " + header);
+ failWithoutActual(fact("expected message of type", messageType));
}
// Return a named subject that displays a human-readable table of
@@ -191,9 +193,10 @@
private void rcpt(@Nullable RecipientType type, String email, boolean expected) {
if (recipients.get(type).contains(email) != expected) {
- fail(
- expected ? "notifies" : "doesn't notify",
- "]\n" + type + ": " + users.emailToName(email) + "\n]");
+ failWithoutActual(
+ fact(
+ expected ? "notifies" : "doesn't notify",
+ "[\n" + type + ": " + users.emailToName(email) + "\n]"));
}
if (expected) {
accountedFor.add(email);
@@ -219,9 +222,10 @@
}
}
if (!ok) {
- fail(
- "was fully tested, missing assertions for: "
- + recipientMapToString(unaccountedFor, e -> users.emailToName(e)));
+ failWithoutActual(
+ fact(
+ "expected assertions for",
+ recipientMapToString(unaccountedFor, e -> users.emailToName(e))));
}
return this;
}
@@ -282,7 +286,7 @@
private void rcpt(@Nullable RecipientType type, NotifyType watch) {
if (!users.watchers.containsKey(watch)) {
- fail("configured to watch", watch);
+ failWithoutActual(fact("expected to be configured to watch", watch));
}
rcpt(type, users.watchers.get(watch));
}
diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeMessageApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeMessageApi.java
index eb2d2b9..66356f1 100644
--- a/java/com/google/gerrit/extensions/api/changes/ChangeMessageApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/ChangeMessageApi.java
@@ -24,6 +24,14 @@
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 ChangeMessageInfo get() throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public ChangeMessageInfo delete(DeleteChangeMessageInput input) throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/java/com/google/gerrit/extensions/api/changes/DeleteChangeMessageInput.java b/java/com/google/gerrit/extensions/api/changes/DeleteChangeMessageInput.java
new file mode 100644
index 0000000..ece5a61
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/changes/DeleteChangeMessageInput.java
@@ -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;
+ }
+}
diff --git a/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java b/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java
index a2e61b8..07ad71b 100644
--- a/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java
+++ b/java/com/google/gerrit/extensions/common/ChangeMessageInfo.java
@@ -45,4 +45,24 @@
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
+ + "]}";
+ }
}
diff --git a/java/com/google/gerrit/extensions/common/FileInfo.java b/java/com/google/gerrit/extensions/common/FileInfo.java
index a812908..747f9e2 100644
--- a/java/com/google/gerrit/extensions/common/FileInfo.java
+++ b/java/com/google/gerrit/extensions/common/FileInfo.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.common;
+import java.util.Objects;
+
public class FileInfo {
public Character status;
public Boolean binary;
@@ -22,4 +24,18 @@
public Integer linesDeleted;
public long sizeDelta;
public long size;
+
+ public boolean equals(Object o) {
+ if (o instanceof FileInfo) {
+ FileInfo fileInfo = (FileInfo) o;
+ return Objects.equals(status, fileInfo.status)
+ && Objects.equals(binary, fileInfo.binary)
+ && Objects.equals(oldPath, fileInfo.oldPath)
+ && Objects.equals(linesInserted, fileInfo.linesInserted)
+ && Objects.equals(linesDeleted, fileInfo.linesDeleted)
+ && Objects.equals(sizeDelta, fileInfo.sizeDelta)
+ && Objects.equals(size, fileInfo.size);
+ }
+ return false;
+ }
}
diff --git a/java/com/google/gerrit/extensions/common/testing/RangeSubject.java b/java/com/google/gerrit/extensions/common/testing/RangeSubject.java
index b478a7e..db7f0d1 100644
--- a/java/com/google/gerrit/extensions/common/testing/RangeSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/RangeSubject.java
@@ -14,6 +14,7 @@
package com.google.gerrit.extensions.common.testing;
+import static com.google.common.truth.Fact.fact;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureMetadata;
@@ -51,14 +52,14 @@
public void isValid() {
isNotNull();
if (!actual().isValid()) {
- fail("is valid");
+ failWithoutActual(fact("expected", "valid"));
}
}
public void isInvalid() {
isNotNull();
if (actual().isValid()) {
- fail("is invalid");
+ failWithoutActual(fact("expected", "not valid"));
}
}
}
diff --git a/java/com/google/gerrit/server/ChangeMessagesUtil.java b/java/com/google/gerrit/server/ChangeMessagesUtil.java
index e635072..a6dbb53 100644
--- a/java/com/google/gerrit/server/ChangeMessagesUtil.java
+++ b/java/com/google/gerrit/server/ChangeMessagesUtil.java
@@ -16,6 +16,7 @@
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.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;
@@ -129,6 +132,71 @@
}
/**
+ * 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.
*/
diff --git a/java/com/google/gerrit/server/api/changes/ChangeMessageApiImpl.java b/java/com/google/gerrit/server/api/changes/ChangeMessageApiImpl.java
index 988ac91..14310e8 100644
--- a/java/com/google/gerrit/server/api/changes/ChangeMessageApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/ChangeMessageApiImpl.java
@@ -17,9 +17,11 @@
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 @@
}
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 @@
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);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/change/FileInfoJson.java b/java/com/google/gerrit/server/change/FileInfoJson.java
index f4b7457..56cc8df 100644
--- a/java/com/google/gerrit/server/change/FileInfoJson.java
+++ b/java/com/google/gerrit/server/change/FileInfoJson.java
@@ -20,6 +20,7 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
@@ -68,7 +69,12 @@
private Map<String, FileInfo> toFileInfoMap(Change change, PatchListKey key)
throws PatchListNotAvailableException {
- PatchList list = patchListCache.get(key, change.getProject());
+ return toFileInfoMap(change.getProject(), key);
+ }
+
+ public Map<String, FileInfo> toFileInfoMap(Project.NameKey project, PatchListKey key)
+ throws PatchListNotAvailableException {
+ PatchList list = patchListCache.get(key, project);
Map<String, FileInfo> files = new TreeMap<>();
for (PatchListEntry e : list.getPatches()) {
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index 516dcd4..d4c013d 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -45,7 +45,6 @@
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedListMultimap;
@@ -1486,19 +1485,6 @@
}
}
- /**
- * Gets an unmodifiable view of the pushOptions.
- *
- * <p>The collection is empty if the client does not support push options, or if the client did
- * not send any options.
- *
- * @return an unmodifiable view of pushOptions.
- */
- @Nullable
- ListMultimap<String, String> getPushOptions() {
- return ImmutableListMultimap.copyOf(pushOptions);
- }
-
private void parseMagicBranch(ReceiveCommand cmd) throws PermissionBackendException {
// Permit exactly one new change request per push.
if (magicBranch != null) {
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 6cc6eb5..1cf71c0 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -238,25 +238,17 @@
public static class ChangeIdValidator implements CommitValidationListener {
private static final String CHANGE_ID_PREFIX = FooterConstants.CHANGE_ID.getName() + ":";
private static final String MISSING_CHANGE_ID_MSG =
- "[%s] missing " + FooterConstants.CHANGE_ID.getName() + " in commit message footer";
+ "[%s] missing Change-Id in commit message footer";
private static final String MISSING_SUBJECT_MSG =
- "[%s] missing subject; "
- + FooterConstants.CHANGE_ID.getName()
- + " must be in commit message footer";
+ "[%s] missing subject; Change-Id must be in commit message footer";
private static final String MULTIPLE_CHANGE_ID_MSG =
- "[%s] multiple " + FooterConstants.CHANGE_ID.getName() + " lines in commit message footer";
+ "[%s] multiple Change-Id lines in commit message footer";
private static final String INVALID_CHANGE_ID_MSG =
- "[%s] invalid "
- + FooterConstants.CHANGE_ID.getName()
- + " line format in commit message footer";
+ "[%s] invalid Change-Id line format in commit message footer";
@VisibleForTesting
public static final String CHANGE_ID_MISMATCH_MSG =
- "[%s] "
- + FooterConstants.CHANGE_ID.getName()
- + " in commit message footer does not match"
- + FooterConstants.CHANGE_ID.getName()
- + " of target change";
+ "[%s] Change-Id in commit message footer does not match Change-Id of target change";
private static final Pattern CHANGE_ID = Pattern.compile(CHANGE_ID_PATTERN);
@@ -367,10 +359,9 @@
// If there are no SSH keys, the commit-msg hook must be installed via
// HTTP(S)
if (hostKeys.isEmpty()) {
- String p = "${gitdir}/hooks/commit-msg";
return String.format(
- " gitdir=$(git rev-parse --git-dir); curl -o %s %stools/hooks/commit-msg ; chmod +x %s",
- p, canonicalWebUrl, p);
+ " f=\"$(git rev-parse --git-dir)/hooks/commit-msg\"; curl -o \"$f\" %stools/hooks/commit-msg ; chmod +x \"$f\"",
+ canonicalWebUrl);
}
// SSH keys exist, so the hook can be installed with scp.
diff --git a/java/com/google/gerrit/server/mail/receive/MailProcessor.java b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
index 75053c8..707056c 100644
--- a/java/com/google/gerrit/server/mail/receive/MailProcessor.java
+++ b/java/com/google/gerrit/server/mail/receive/MailProcessor.java
@@ -237,7 +237,7 @@
.sorted(CommentsUtil.COMMENT_ORDER)
.collect(toList());
Project.NameKey project = cd.project();
- String changeUrl = canonicalUrl.get() + "#/c/" + cd.getId().get();
+ String changeUrl = canonicalUrl.get() + "c/" + cd.project().get() + "/+/" + cd.getId().get();
List<MailComment> parsedComments;
if (useHtmlParser(message)) {
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 00a496a2..4ba3b67 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -221,10 +221,7 @@
/** Get a link to the change; null if the server doesn't know its own address. */
public String getChangeUrl() {
if (getGerritUrl() != null) {
- final StringBuilder r = new StringBuilder();
- r.append(getGerritUrl());
- r.append(change.getChangeId());
- return r.toString();
+ return getGerritUrl() + "c/" + change.getProject().get() + "/+/" + change.getChangeId();
}
return null;
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 903a982..bd7f8fc 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -160,6 +160,7 @@
private ChangeDraftUpdate draftUpdate;
private RobotCommentUpdate robotCommentUpdate;
private DeleteCommentRewriter deleteCommentRewriter;
+ private DeleteChangeMessageRewriter deleteChangeMessageRewriter;
@AssistedInject
private ChangeUpdate(
@@ -395,6 +396,11 @@
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 @@
@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 @@
return deleteCommentRewriter;
}
+ public DeleteChangeMessageRewriter getDeleteChangeMessageRewriter() {
+ return deleteChangeMessageRewriter;
+ }
+
public void setAllowWriteToNewRef(boolean allow) {
isAllowWriteToNewtRef = allow;
}
diff --git a/java/com/google/gerrit/server/notedb/DeleteChangeMessageRewriter.java b/java/com/google/gerrit/server/notedb/DeleteChangeMessageRewriter.java
new file mode 100644
index 0000000..212aa37
--- /dev/null
+++ b/java/com/google/gerrit/server/notedb/DeleteChangeMessageRewriter.java
@@ -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);
+ }
+}
diff --git a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index b20bc73..fd25059 100644
--- a/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -467,14 +467,33 @@
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);
diff --git a/java/com/google/gerrit/server/project/ProjectCacheImpl.java b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
index 1f51fda..df80e35 100644
--- a/java/com/google/gerrit/server/project/ProjectCacheImpl.java
+++ b/java/com/google/gerrit/server/project/ProjectCacheImpl.java
@@ -146,7 +146,9 @@
} catch (Exception e) {
if (!(e.getCause() instanceof RepositoryNotFoundException)) {
logger.atWarning().withCause(e).log("Cannot read project %s", projectName.get());
- Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
+ if (e.getCause() != null) {
+ Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
+ }
throw new IOException(e);
}
logger.atFine().withCause(e).log("Cannot find project %s", projectName.get());
diff --git a/java/com/google/gerrit/server/restapi/account/Capabilities.java b/java/com/google/gerrit/server/restapi/account/Capabilities.java
index 85f9da0..07b1214 100644
--- a/java/com/google/gerrit/server/restapi/account/Capabilities.java
+++ b/java/com/google/gerrit/server/restapi/account/Capabilities.java
@@ -72,7 +72,7 @@
GlobalOrPluginPermission perm = parse(id);
try {
- permissionBackend.user(target).check(perm);
+ permissionBackend.absentUser(target.getAccountId()).check(perm);
return new AccountResource.Capability(target, globalOrPluginPermissionName(perm));
} catch (AuthException e) {
throw new ResourceNotFoundException(id);
diff --git a/java/com/google/gerrit/server/restapi/account/GetCapabilities.java b/java/com/google/gerrit/server/restapi/account/GetCapabilities.java
index d2236fd..7889f6e 100644
--- a/java/com/google/gerrit/server/restapi/account/GetCapabilities.java
+++ b/java/com/google/gerrit/server/restapi/account/GetCapabilities.java
@@ -85,7 +85,7 @@
PermissionBackend.WithUser perm = permissionBackend.currentUser();
if (!self.get().hasSameAccountId(resource.getUser())) {
perm.check(GlobalPermission.ADMINISTRATE_SERVER);
- perm = permissionBackend.user(resource.getUser());
+ perm = permissionBackend.absentUser(resource.getUser().getAccountId());
}
Map<String, Object> have = new LinkedHashMap<>();
diff --git a/java/com/google/gerrit/server/restapi/change/DeleteChangeMessage.java b/java/com/google/gerrit/server/restapi/change/DeleteChangeMessage.java
new file mode 100644
index 0000000..67c54c2
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/change/DeleteChangeMessage.java
@@ -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());
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index 5b28971..5a5d2bb 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -182,6 +182,8 @@
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);
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 20f3d8a..9bbaf69 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -33,6 +33,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.common.collect.Ordering;
+import com.google.common.flogger.FluentLogger;
import com.google.common.hash.HashCode;
import com.google.common.hash.Hashing;
import com.google.gerrit.common.Nullable;
@@ -145,6 +146,8 @@
@Singleton
public class PostReview
extends RetryingRestModifyView<RevisionResource, ReviewInput, Response<ReviewResult>> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
public static final String ERROR_ADDING_REVIEWER = "error adding reviewer";
public static final String ERROR_ONLY_OWNER_CAN_MODIFY_WORK_IN_PROGRESS =
"only change owner can specify work_in_progress or ready";
@@ -1216,8 +1219,15 @@
}
forceCallerAsReviewer(projectState, ctx, current, ups, del);
- ctx.getDb().patchSetApprovals().delete(del);
- ctx.getDb().patchSetApprovals().upsert(ups);
+
+ if (!del.isEmpty()) {
+ ctx.getDb().patchSetApprovals().delete(del);
+ }
+
+ if (!ups.isEmpty()) {
+ ctx.getDb().patchSetApprovals().upsert(ups);
+ }
+
return !del.isEmpty() || !ups.isEmpty();
}
@@ -1303,8 +1313,15 @@
if (del.isEmpty()) {
// If no existing label is being set to 0, hack in the caller
// as a reviewer by picking the first server-wide LabelType.
- LabelId labelId =
- projectState.getLabelTypes(ctx.getNotes()).getLabelTypes().get(0).getLabelId();
+ List<LabelType> labelTypes = projectState.getLabelTypes(ctx.getNotes()).getLabelTypes();
+ if (labelTypes.isEmpty()) {
+ logger.atWarning().log(
+ "no label type found for project %s, change %s",
+ projectState.getName(), ctx.getChange().getChangeId());
+ return;
+ }
+
+ LabelId labelId = labelTypes.get(0).getLabelId();
PatchSetApproval c = ApprovalsUtil.newApproval(psId, user, labelId, 0, ctx.getWhen());
c.setTag(in.tag);
c.setGranted(ctx.getWhen());
diff --git a/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java b/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java
index 53411b8..09f973b 100644
--- a/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java
+++ b/java/com/google/gerrit/server/restapi/project/FilesInCommitCollection.java
@@ -14,34 +14,46 @@
package com.google.gerrit.server.restapi.project;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Patch;
+import com.google.gerrit.server.change.FileInfoJson;
import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.patch.PatchListKey;
+import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.CommitResource;
import com.google.gerrit.server.project.FileResource;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.kohsuke.args4j.Option;
@Singleton
public class FilesInCommitCollection implements ChildCollection<CommitResource, FileResource> {
private final DynamicMap<RestView<FileResource>> views;
+ private final Provider<ListFiles> list;
private final GitRepositoryManager repoManager;
@Inject
FilesInCommitCollection(
- DynamicMap<RestView<FileResource>> views, GitRepositoryManager repoManager) {
+ DynamicMap<RestView<FileResource>> views,
+ Provider<ListFiles> list,
+ GitRepositoryManager repoManager) {
this.views = views;
+ this.list = list;
this.repoManager = repoManager;
}
@Override
public RestView<CommitResource> list() throws ResourceNotFoundException {
- throw new ResourceNotFoundException();
+ return list.get();
}
@Override
@@ -57,4 +69,32 @@
public DynamicMap<RestView<FileResource>> views() {
return views;
}
+
+ public static final class ListFiles implements RestReadView<CommitResource> {
+ @Option(name = "--parent", metaVar = "parent-number")
+ int parentNum;
+
+ private final FileInfoJson fileInfoJson;
+
+ @Inject
+ public ListFiles(FileInfoJson fileInfoJson) {
+ this.fileInfoJson = fileInfoJson;
+ }
+
+ @Override
+ public Object apply(CommitResource resource) throws PatchListNotAvailableException {
+ RevCommit commit = resource.getCommit();
+ PatchListKey key;
+
+ if (parentNum > 0) {
+ key =
+ PatchListKey.againstParentNum(
+ parentNum, commit, DiffPreferencesInfo.Whitespace.IGNORE_NONE);
+ } else {
+ key = PatchListKey.againstCommit(null, commit, DiffPreferencesInfo.Whitespace.IGNORE_NONE);
+ }
+
+ return fileInfoJson.toFileInfoMap(resource.getProjectState().getNameKey(), key);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index cb2e306..9cfa272 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -35,6 +35,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.SubmitRecord;
+import com.google.gerrit.common.data.SubmitRequirement;
import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.api.changes.RecipientType;
import com.google.gerrit.extensions.api.changes.SubmitInput;
@@ -315,7 +316,7 @@
throw new ResourceConflictException("submit rule error: " + record.errorMessage);
case NOT_READY:
- throw new ResourceConflictException(describeLabels(cd, record.labels));
+ throw new ResourceConflictException(describeNotReady(cd, record));
case FORCED:
default:
@@ -336,6 +337,21 @@
return cd.submitRecords(submitRuleOptions(allowClosed));
}
+ private static String describeNotReady(ChangeData cd, SubmitRecord record) throws OrmException {
+ List<String> blockingConditions = new ArrayList<>();
+ if (record.labels != null) {
+ blockingConditions.add(describeLabels(cd, record.labels));
+ }
+ if (record.requirements != null) {
+ record
+ .requirements
+ .stream()
+ .map(SubmitRequirement::fallbackText)
+ .forEach(blockingConditions::add);
+ }
+ return Joiner.on("; ").join(blockingConditions);
+ }
+
private static String describeLabels(ChangeData cd, List<SubmitRecord.Label> labels)
throws OrmException {
List<String> labelResults = new ArrayList<>();
diff --git a/java/com/google/gerrit/truth/ListSubject.java b/java/com/google/gerrit/truth/ListSubject.java
index cccf51b..bd9df30 100644
--- a/java/com/google/gerrit/truth/ListSubject.java
+++ b/java/com/google/gerrit/truth/ListSubject.java
@@ -15,6 +15,7 @@
package com.google.gerrit.truth;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.truth.Fact.fact;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.FailureMetadata;
@@ -46,7 +47,7 @@
isNotNull();
List<E> list = getActualList();
if (index >= list.size()) {
- fail("has an element at index " + index);
+ failWithoutActual(fact("expected to have element at index", index));
}
return elementAssertThatFunction.apply(list.get(index));
}
diff --git a/java/com/google/gerrit/truth/OptionalSubject.java b/java/com/google/gerrit/truth/OptionalSubject.java
index f24b5da..d91f07b 100644
--- a/java/com/google/gerrit/truth/OptionalSubject.java
+++ b/java/com/google/gerrit/truth/OptionalSubject.java
@@ -14,6 +14,7 @@
package com.google.gerrit.truth;
+import static com.google.common.truth.Fact.fact;
import static com.google.common.truth.Truth.assertAbout;
import com.google.common.truth.DefaultSubject;
@@ -57,7 +58,7 @@
isNotNull();
Optional<T> optional = actual();
if (!optional.isPresent()) {
- fail("has a value");
+ failWithoutActual(fact("expected to have", "value"));
}
}
@@ -65,7 +66,7 @@
isNotNull();
Optional<T> optional = actual();
if (optional.isPresent()) {
- fail("does not have a value");
+ failWithoutActual(fact("expected not to have", "value"));
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
index a86a739..6563de3 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
@@ -144,10 +144,7 @@
ImmutableList.of(
RestCall.get("/projects/%s/commits/%s"),
RestCall.get("/projects/%s/commits/%s/in"),
- RestCall.builder(GET, "/projects/%s/commits/%s/files")
- // GET /projects/<project>/branches/<branch>/files is not implemented
- .expectedResponseCode(SC_NOT_FOUND)
- .build(),
+ RestCall.get("/projects/%s/commits/%s/files"),
RestCall.post("/projects/%s/commits/%s/cherrypick"));
/**
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
index d0b01b7..790b884 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
@@ -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 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 @@
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);
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java b/javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java
new file mode 100644
index 0000000..3ac2d10
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/rest/project/ListCommitFilesIT.java
@@ -0,0 +1,90 @@
+// 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.acceptance.rest.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.GitUtil.getChangeId;
+import static com.google.gerrit.acceptance.GitUtil.pushHead;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.extensions.common.FileInfo;
+import com.google.gson.reflect.TypeToken;
+import java.lang.reflect.Type;
+import java.util.Map;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Test;
+
+public class ListCommitFilesIT extends AbstractDaemonTest {
+ private static String SUBJECT_1 = "subject 1";
+ private static String SUBJECT_2 = "subject 2";
+ private static String FILE_A = "a.txt";
+ private static String FILE_B = "b.txt";
+
+ @Test
+ public void listCommitFiles() throws Exception {
+ commitBuilder().add(FILE_B, "2").message(SUBJECT_1).create();
+ pushHead(testRepo, "refs/heads/master", false);
+
+ RevCommit a = commitBuilder().add(FILE_A, "1").rm(FILE_B).message(SUBJECT_2).create();
+ String id = getChangeId(testRepo, a).get();
+ pushHead(testRepo, "refs/for/master", false);
+
+ RestResponse r =
+ userRestSession.get("/projects/" + project.get() + "/commits/" + a.name() + "/files/");
+ r.assertOK();
+ Type type = new TypeToken<Map<String, FileInfo>>() {}.getType();
+ Map<String, FileInfo> files1 = newGson().fromJson(r.getReader(), type);
+ r.consume();
+
+ r = userRestSession.get("/changes/" + id + "/revisions/" + a.name() + "/files");
+ r.assertOK();
+ Map<String, FileInfo> files2 = newGson().fromJson(r.getReader(), type);
+ r.consume();
+
+ assertThat(files1).isEqualTo(files2);
+ }
+
+ @Test
+ public void listMergeCommitFiles() throws Exception {
+ PushOneCommit.Result result = createMergeCommitChange("refs/for/master");
+
+ RestResponse r =
+ userRestSession.get(
+ "/projects/"
+ + project.get()
+ + "/commits/"
+ + result.getCommit().name()
+ + "/files/?parent=2");
+ r.assertOK();
+ Type type = new TypeToken<Map<String, FileInfo>>() {}.getType();
+ Map<String, FileInfo> files1 = newGson().fromJson(r.getReader(), type);
+ r.consume();
+
+ r =
+ userRestSession.get(
+ "/changes/"
+ + result.getChangeId()
+ + "/revisions/"
+ + result.getCommit().name()
+ + "/files/?parent=2");
+ r.assertOK();
+ Map<String, FileInfo> files2 = newGson().fromJson(r.getReader(), type);
+ r.consume();
+
+ assertThat(files1).isEqualTo(files2);
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 211af59..8b1f4bc 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -47,7 +47,6 @@
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.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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
List<RevCommit> commitsBeforeDelete = new ArrayList<>();
if (notesMigration.commitChangeWrites()) {
- commitsBeforeDelete = getCommits(id);
+ commitsBeforeDelete = getChangeMetaCommitsInReverseOrder(id);
}
setApiUser(admin);
@@ -944,7 +938,7 @@
if (notesMigration.commitChangeWrites()) {
assertMetaBranchCommitsAfterRewriting(commitsBeforeDelete, id, uuid, expectedMsg);
}
- assertThat(getChangeSortedComments(changeId)).hasSize(3);
+ assertThat(getChangeSortedComments(id.get())).hasSize(3);
}
@Test
@@ -962,19 +956,6 @@
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 @@
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 @@
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 @@
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();
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
index 988b051..a0170d2 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailMetadataIT.java
@@ -24,6 +24,7 @@
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.mail.EmailHeader;
import com.google.gerrit.mail.MailProcessingUtil;
+import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.FakeEmailSender;
import com.google.gerrit.testing.TestTimeUtil;
import java.sql.Timestamp;
@@ -63,7 +64,7 @@
assertThat(emails).hasSize(1);
FakeEmailSender.Message message = emails.get(0);
- String changeURL = "<" + canonicalWebUrl.get() + newChange.getChange().getId().get() + ">";
+ String changeURL = "<" + getChangeUrl(newChange.getChange()) + ">";
Map<String, Object> expectedHeaders = new HashMap<>();
expectedHeaders.put("Gerrit-PatchSet", "1");
@@ -100,7 +101,7 @@
assertThat(emails).hasSize(1);
FakeEmailSender.Message message = emails.get(0);
- String changeURL = "<" + canonicalWebUrl.get() + newChange.getChange().getId().get() + ">";
+ String changeURL = "<" + getChangeUrl(newChange.getChange()) + ">";
Map<String, Object> expectedHeaders = new HashMap<>();
expectedHeaders.put("Gerrit-PatchSet", "1");
expectedHeaders.put(
@@ -159,4 +160,12 @@
}
}
}
+
+ private String getChangeUrl(ChangeData changeData) {
+ return canonicalWebUrl.get()
+ + "c/"
+ + changeData.project().get()
+ + "/+/"
+ + changeData.getId().get();
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index b9f30d6..9ff2c05 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -46,12 +46,7 @@
// Build Message
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt =
- newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- "Test Message",
- null,
- null,
- null);
+ newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -74,12 +69,7 @@
// Build Message
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt =
- newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- null,
- "Some Inline Comment",
- null,
- null);
+ newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, "Some Inline Comment", null, null);
b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -111,11 +101,7 @@
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt =
newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- null,
- null,
- "Some Comment on File 1",
- null);
+ getChangeUrl(changeInfo) + "/1", null, null, "Some Comment on File 1", null);
b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -147,12 +133,7 @@
// Build Message
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt =
- newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- null,
- "Some Inline Comment",
- null,
- null);
+ newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, "Some Inline Comment", null, null);
b.textContent(txt + textFooterForChange(changeInfo._number, ts));
mailProcessor.process(b.build());
@@ -178,12 +159,7 @@
// Build Message
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt =
- newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- null,
- "Some Inline Comment",
- null,
- null);
+ newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, "Some Inline Comment", null, null);
b.textContent(txt + textFooterForChange(changeInfo._number, ts));
// Set account state to inactive
@@ -211,12 +187,7 @@
// Build Message
String txt =
- newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- "Test Message",
- null,
- null,
- null);
+ newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
.from(user.emailAddress)
@@ -238,12 +209,7 @@
// Build Message
String txt =
- newPlaintextBody(
- canonicalWebUrl.get() + "#/c/" + changeInfo._number + "/1",
- "Test Message",
- null,
- null,
- null);
+ newPlaintextBody(getChangeUrl(changeInfo) + "/1", "Test Message", null, null, null);
MailMessage.Builder b =
messageBuilderWithDefaultFields()
.from(user.emailAddress)
@@ -257,4 +223,8 @@
assertThat(message.body()).contains("was unable to parse your email");
assertThat(message.headers()).containsKey("Subject");
}
+
+ private String getChangeUrl(ChangeInfo changeInfo) {
+ return canonicalWebUrl.get() + "c/" + changeInfo.project + "/+/" + changeInfo._number;
+ }
}
diff --git a/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java b/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java
index b5b942d..2fc06a6 100644
--- a/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java
+++ b/javatests/com/google/gerrit/gpg/PublicKeyStoreTest.java
@@ -26,6 +26,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import com.google.common.collect.Iterators;
import com.google.gerrit.gpg.testing.TestKey;
import java.util.ArrayList;
import java.util.Arrays;
@@ -163,6 +164,8 @@
TestKey key5 = validKeyWithSecondUserId();
PGPPublicKeyRing keyRing = key5.getPublicKeyRing();
PGPPublicKey key = keyRing.getPublicKey();
+ PGPPublicKey subKey =
+ keyRing.getPublicKey(Iterators.get(keyRing.getPublicKeys(), 1).getKeyID());
store.add(keyRing);
assertEquals(RefUpdate.Result.NEW, store.save(newCommitBuilder()));
@@ -171,9 +174,11 @@
"Testuser Five <test5@example.com>",
"foo:myId");
+ keyRing = PGPPublicKeyRing.removePublicKey(keyRing, subKey);
keyRing = PGPPublicKeyRing.removePublicKey(keyRing, key);
key = PGPPublicKey.removeCertification(key, "foo:myId");
keyRing = PGPPublicKeyRing.insertPublicKey(keyRing, key);
+ keyRing = PGPPublicKeyRing.insertPublicKey(keyRing, subKey);
store.add(keyRing);
assertEquals(RefUpdate.Result.FAST_FORWARD, store.save(newCommitBuilder()));
diff --git a/javatests/com/google/gerrit/mail/AbstractParserTest.java b/javatests/com/google/gerrit/mail/AbstractParserTest.java
index 5219ce8..6ab4ca2 100644
--- a/javatests/com/google/gerrit/mail/AbstractParserTest.java
+++ b/javatests/com/google/gerrit/mail/AbstractParserTest.java
@@ -26,7 +26,8 @@
@Ignore
public class AbstractParserTest {
- protected static final String CHANGE_URL = "https://gerrit-review.googlesource.com/#/changes/123";
+ protected static final String CHANGE_URL =
+ "https://gerrit-review.googlesource.com/c/project/+/123";
protected static void assertChangeMessage(String message, MailComment comment) {
assertThat(comment.fileName).isNull();
diff --git a/javatests/com/google/gerrit/mail/TextParserTest.java b/javatests/com/google/gerrit/mail/TextParserTest.java
index e11321a..a5c2152 100644
--- a/javatests/com/google/gerrit/mail/TextParserTest.java
+++ b/javatests/com/google/gerrit/mail/TextParserTest.java
@@ -23,7 +23,7 @@
public class TextParserTest extends AbstractParserTest {
private static final String quotedFooter =
""
- + "> To view, visit https://gerrit-review.googlesource.com/123\n"
+ + "> To view, visit https://gerrit-review.googlesource.com/c/project/+/123\n"
+ "> To unsubscribe, visit https://gerrit-review.googlesource.com\n"
+ "> \n"
+ "> Gerrit-MessageType: comment\n"
diff --git a/lib/truth/BUILD b/lib/truth/BUILD
index 82cd98a..db5bc48 100644
--- a/lib/truth/BUILD
+++ b/lib/truth/BUILD
@@ -4,6 +4,7 @@
visibility = ["//visibility:public"],
exports = ["@truth//jar"],
runtime_deps = [
+ ":diffutils",
"//lib:guava",
"//lib:junit",
],
@@ -33,6 +34,13 @@
)
java_library(
+ name = "diffutils",
+ data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
+ visibility = ["//visibility:private"],
+ exports = ["@diffutils//jar"],
+)
+
+java_library(
name = "truth-proto-extension",
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:public"],
diff --git a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html
index d18e442..ff9b54a 100644
--- a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior.html
@@ -30,7 +30,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
'Size',
diff --git a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html
index dc98b59..4d6913b 100644
--- a/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html
+++ b/polygerrit-ui/app/behaviors/gr-change-table-behavior/gr-change-table-behavior_test.html
@@ -63,7 +63,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
'Size',
@@ -74,7 +74,7 @@
'Subject',
'Status',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Size',
];
@@ -83,13 +83,13 @@
});
test('isColumnHidden', () => {
- const columnToCheck = 'Project';
+ const columnToCheck = 'Repo';
let columnsToDisplay = [
'Subject',
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
'Size',
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index eab1b01..6890d53 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -169,21 +169,21 @@
<span class="placeholder">--</span>
</template>
</td>
- <td class="cell project"
- hidden$="[[isColumnHidden('Project', visibleChangeTableColumns)]]">
- <a class="fullProject" href$="[[_computeProjectURL(change)]]">
- [[_computeProjectDisplay(change)]]
+ <td class="cell repo"
+ hidden$="[[isColumnHidden('Repo', visibleChangeTableColumns)]]">
+ <a class="fullRepo" href$="[[_computeRepoUrl(change)]]">
+ [[_computeRepoDisplay(change)]]
</a>
<a
- class="truncatedProject"
- href$="[[_computeProjectURL(change)]]"
- title$="[[_computeProjectDisplay(change)]]">
- [[_computeProjectDisplay(change, 'true')]]
+ class="truncatedRepo"
+ href$="[[_computeRepoUrl(change)]]"
+ title$="[[_computeRepoDisplay(change)]]">
+ [[_computeRepoDisplay(change, 'true')]]
</a>
</td>
<td class="cell branch"
hidden$="[[isColumnHidden('Branch', visibleChangeTableColumns)]]">
- <a href$="[[_computeProjectBranchURL(change)]]">
+ <a href$="[[_computeRepoBranchURL(change)]]">
[[change.branch]]
</a>
<template is="dom-if" if="[[change.topic]]">
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
index 3f5a402..65fe9ea 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -131,12 +131,12 @@
return '';
},
- _computeProjectURL(change) {
+ _computeRepoUrl(change) {
return Gerrit.Nav.getUrlForProjectChanges(change.project, true,
change.internalHost);
},
- _computeProjectBranchURL(change) {
+ _computeRepoBranchURL(change) {
return Gerrit.Nav.getUrlForBranch(change.branch, change.project, null,
change.internalHost);
},
@@ -155,7 +155,7 @@
* truncated. If this value is truthy, the name will be truncated.
* @return {string}
*/
- _computeProjectDisplay(change, truncate) {
+ _computeRepoDisplay(change, truncate) {
if (!change || !change.project) { return ''; }
let str = '';
if (change.internalHost) { str += change.internalHost + '/'; }
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
index f4c66c7..aaad362 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
@@ -125,7 +125,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
'Size',
@@ -135,31 +135,13 @@
for (const column of element.columnNames) {
const elementClass = '.' + column.toLowerCase();
+ assert.isOk(element.$$(elementClass),
+ `Expect ${elementClass} element to be found`);
assert.isFalse(element.$$(elementClass).hidden);
}
});
- test('no hidden columns', () => {
- element.visibleChangeTableColumns = [
- 'Subject',
- 'Status',
- 'Owner',
- 'Assignee',
- 'Project',
- 'Branch',
- 'Updated',
- 'Size',
- ];
-
- flushAsynchronousOperations();
-
- for (const column of element.columnNames) {
- const elementClass = '.' + column.toLowerCase();
- assert.isFalse(element.$$(elementClass).hidden);
- }
- });
-
- test('project column hidden', () => {
+ test('repo column hidden', () => {
element.visibleChangeTableColumns = [
'Subject',
'Status',
@@ -174,7 +156,7 @@
for (const column of element.columnNames) {
const elementClass = '.' + column.toLowerCase();
- if (column === 'Project') {
+ if (column === 'Repo') {
assert.isTrue(element.$$(elementClass).hidden);
} else {
assert.isFalse(element.$$(elementClass).hidden);
@@ -274,17 +256,17 @@
[change.topic, change.internalHost]);
});
- test('_computeProjectDisplay', () => {
+ test('_computeRepoDisplay', () => {
const change = {
project: 'a/test/repo',
internalHost: 'host',
};
- assert.equal(element._computeProjectDisplay(change), 'host/a/test/repo');
- assert.equal(element._computeProjectDisplay(change, true),
+ assert.equal(element._computeRepoDisplay(change), 'host/a/test/repo');
+ assert.equal(element._computeRepoDisplay(change, true),
'host/…/test/repo');
delete change.internalHost;
- assert.equal(element._computeProjectDisplay(change), 'a/test/repo');
- assert.equal(element._computeProjectDisplay(change, true),
+ assert.equal(element._computeRepoDisplay(change), 'a/test/repo');
+ assert.equal(element._computeRepoDisplay(change, true),
'…/test/repo');
});
});
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
index ec581b6..3cf5f9a 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -310,7 +310,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
'Size',
@@ -349,10 +349,10 @@
flushAsynchronousOperations();
});
- test('all columns except project visible', () => {
+ test('all columns except repo visible', () => {
for (const column of element.changeTableColumns) {
const elementClass = '.' + column.toLowerCase();
- if (column === 'Project') {
+ if (column === 'Repo') {
assert.isTrue(element.$$(elementClass).hidden);
} else {
assert.isFalse(element.$$(elementClass).hidden);
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 2801a7c..8413388 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -192,7 +192,7 @@
</section>
</template>
<section>
- <span class="title">Project</span>
+ <span class="title">Repo</span>
<span class="value">
<a href$="[[_computeProjectURL(change.project)]]">
<gr-limited-text limit="40" text="[[change.project]]"></gr-limited-text>
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
index e635d23..8ec00dd 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements.js
@@ -65,7 +65,8 @@
if (change.requirements) {
for (const requirement of change.requirements) {
requirement.satisfied = requirement.status === 'OK';
- requirement.style = this._computeRequirementClass(requirement);
+ requirement.style =
+ this._computeRequirementClass(requirement.satisfied);
_requirements.push(requirement);
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
index 93b2c36..3f35158 100644
--- a/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-requirements/gr-change-requirements_test.html
@@ -170,5 +170,53 @@
assert.equal(requirement.querySelector('.name').text,
'Resolve all comments');
});
+
+ test('satisfied class is applied with OK', () => {
+ element.change = {
+ status: 'NEW',
+ labels: {},
+ requirements: [{
+ fallback_text: 'Resolve all comments',
+ status: 'OK',
+ }],
+ };
+ flushAsynchronousOperations();
+
+ const requirement = element.$$('.requirement');
+ assert.ok(requirement);
+ assert.ok(requirement.querySelector('.approved'));
+ });
+
+ test('satisfied class is not applied with NOT_READY', () => {
+ element.change = {
+ status: 'NEW',
+ labels: {},
+ requirements: [{
+ fallback_text: 'Resolve all comments',
+ status: 'NOT_READY',
+ }],
+ };
+ flushAsynchronousOperations();
+
+ const requirement = element.$$('.requirement');
+ assert.ok(requirement);
+ assert.strictEqual(requirement.querySelector('.approved'), null);
+ });
+
+ test('satisfied class is not applied with RULE_ERROR', () => {
+ element.change = {
+ status: 'NEW',
+ labels: {},
+ requirements: [{
+ fallback_text: 'Resolve all comments',
+ status: 'RULE_ERROR',
+ }],
+ };
+ flushAsynchronousOperations();
+
+ const requirement = element.$$('.requirement');
+ assert.ok(requirement);
+ assert.strictEqual(requirement.querySelector('.approved'), null);
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
index a3fc06f..7d8bcf5 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.js
@@ -39,8 +39,8 @@
const defaultTheme = config.default_theme;
const pluginsPending = []
.concat(jsPlugins, htmlPlugins, defaultTheme || [])
- .filter(p => !Gerrit._isPluginPreloaded(p))
- .map(p => this._urlFor(p));
+ .map(p => this._urlFor(p))
+ .filter(p => !Gerrit._isPluginPreloaded(p));
Gerrit._setPluginsPending(pluginsPending);
if (defaultTheme) {
// Make theme first to be first to load.
@@ -96,8 +96,9 @@
},
_urlFor(pathOrUrl) {
- if (pathOrUrl.startsWith('http')) {
- // Plugins are loaded from another domain.
+ if (pathOrUrl.startsWith('preloaded:') ||
+ pathOrUrl.startsWith('http')) {
+ // Plugins are loaded from another domain or preloaded.
return pathOrUrl;
}
if (!pathOrUrl.startsWith('/')) {
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
index d9def78..a5b3c3e 100644
--- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
+++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.html
@@ -190,8 +190,8 @@
test('skips preloaded plugins', () => {
sandbox.stub(Gerrit, '_isPluginPreloaded')
- .withArgs('plugins/foo/bar').returns(true)
- .withArgs('plugins/42').returns(true);
+ .withArgs(url + '/plugins/foo/bar').returns(true)
+ .withArgs(url + '/plugins/42').returns(true);
sandbox.stub(Gerrit, '_setPluginsCount');
sandbox.stub(Gerrit, '_setPluginsPending');
element.config = {
diff --git a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
index daf9437..587cc3b 100644
--- a/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-change-table-editor/gr-change-table-editor_test.html
@@ -47,7 +47,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
];
@@ -90,7 +90,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
]);
@@ -151,7 +151,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
]);
@@ -163,7 +163,7 @@
'Status',
'Owner',
'Assignee',
- 'Project',
+ 'Repo',
'Branch',
'Updated',
'Size',
diff --git a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
index 52649db..7846b7a 100644
--- a/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
+++ b/polygerrit-ui/app/elements/settings/gr-watched-projects-editor/gr-watched-projects-editor.html
@@ -49,7 +49,7 @@
<table id="watchedProjects">
<thead>
<tr>
- <th class="projectHeader">Project</th>
+ <th>Repo</th>
<template is="dom-repeat" items="[[_getTypes()]]">
<th class="notifType">[[item.name]]</th>
</template>
@@ -98,7 +98,7 @@
id="newProject"
query="[[_query]]"
threshold="1"
- placeholder="Project"></gr-autocomplete>
+ placeholder="Repo"></gr-autocomplete>
</th>
<th colspan$="[[_getTypeCount()]]">
<input
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index bc41b3e..c1d0936 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -412,6 +412,14 @@
element.EventType.ADMIN_MENU_LINKS);
});
+ test('Gerrit._isPluginPreloaded', () => {
+ Gerrit._preloadedPlugins = {baz: ()=>{}};
+ assert.isFalse(Gerrit._isPluginPreloaded('plugins/foo/bar'));
+ assert.isFalse(Gerrit._isPluginPreloaded('http://a.com/42'));
+ assert.isTrue(Gerrit._isPluginPreloaded('preloaded:baz'));
+ Gerrit._preloadedPlugins = null;
+ });
+
test('preloaded plugins are installed', () => {
const installStub = sandbox.stub();
Gerrit._preloadedPlugins = {foo: installStub};
diff --git a/polygerrit-ui/app/styles/gr-change-list-styles.html b/polygerrit-ui/app/styles/gr-change-list-styles.html
index 6d7469b..37e8bdc 100644
--- a/polygerrit-ui/app/styles/gr-change-list-styles.html
+++ b/polygerrit-ui/app/styles/gr-change-list-styles.html
@@ -116,7 +116,7 @@
.updated,
.size,
.status,
- .project {
+ .repo {
white-space: nowrap;
}
.star {
@@ -136,7 +136,7 @@
.topHeader .label {
border: none;
}
- .truncatedProject {
+ .truncatedRepo {
display: none;
}
@media only screen and (max-width: 150em) {
@@ -147,10 +147,10 @@
max-width: 18rem;
text-overflow: ellipsis;
}
- .truncatedProject {
+ .truncatedRepo {
display: inline-block;
}
- .fullProject {
+ .fullRepo {
display: none;
}
}
@@ -186,7 +186,7 @@
.topHeader,
.leftPadding,
.status,
- .project,
+ .repo,
.branch,
.updated,
.label,
