opendev/gerrit
Code Issues Proposed changes

Merge changes I66527dd1,Iafcfe3f2,I1f7d77fc,I8a75c32a

Browse Source 
* changes:
  GetMergeList: Also allow caching of responses for non-merges
  Include a magic /MERGE_LIST file for merge commits
  Patch: Add static method to check if file is magic
  Don't compare commit message against auto-merge commit
This commit is contained in:
ekempin 2016-09-21 09:06:58 +00:00 committed by Gerrit Code Review
commit b7f970bf12
36 changed files with 808 additions and 239 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -17,6 +17,8 @@ package com.google.gerrit.acceptance;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.GitUtil.initSsh;
import static com.google.gerrit.extensions.api.changes.SubmittedTogetherOption.NON_VISIBLE_CHANGES;
import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static org.eclipse.jgit.lib.Constants.HEAD;
@ -48,6 +50,8 @@ import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeType;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.IdString;
@ -67,6 +71,7 @@ import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.change.Abandon;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.FileContentUtil;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Revisions;
import com.google.gerrit.server.config.AllProjectsName;
@ -546,21 +551,26 @@ public abstract class AbstractDaemonTest {
protected PushOneCommit.Result createMergeCommitChange(String ref)
throws Exception {
return createMergeCommitChange(ref, "foo");
}
protected PushOneCommit.Result createMergeCommitChange(String ref, String file)
throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
PushOneCommit.Result p1 = pushFactory.create(db, admin.getIdent(),
testRepo, "parent 1", ImmutableMap.of("foo", "foo-1", "bar", "bar-1"))
testRepo, "parent 1", ImmutableMap.of(file, "foo-1", "bar", "bar-1"))
.to(ref);
// reset HEAD in order to create a sibling of the first change
testRepo.reset(initial);
PushOneCommit.Result p2 = pushFactory.create(db, admin.getIdent(),
testRepo, "parent 2", ImmutableMap.of("foo", "foo-2", "bar", "bar-2"))
testRepo, "parent 2", ImmutableMap.of(file, "foo-2", "bar", "bar-2"))
.to(ref);
PushOneCommit m = pushFactory.create(db, admin.getIdent(), testRepo, "merge",
ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
ImmutableMap.of(file, "foo-1", "bar", "bar-2"));
m.setParents(ImmutableList.of(p1.getCommit(), p2.getCommit()));
PushOneCommit.Result result = m.to(ref);
result.assertOkStatus();
@ -1086,4 +1096,45 @@ public abstract class AbstractDaemonTest {
}
assertThat(refValues.keySet()).containsAnyIn(trees.keySet());
}
protected void assertDiffForNewFile(DiffInfo diff, RevCommit commit,
String path, String expectedContentSideB) throws Exception {
List<String> expectedLines = new ArrayList<>();
for (String line : expectedContentSideB.split("\n")) {
expectedLines.add(line);
}
assertThat(diff.binary).isNull();
assertThat(diff.changeType).isEqualTo(ChangeType.ADDED);
assertThat(diff.diffHeader).isNotNull();
assertThat(diff.intralineStatus).isNull();
assertThat(diff.webLinks).isNull();
assertThat(diff.metaA).isNull();
assertThat(diff.metaB).isNotNull();
assertThat(diff.metaB.commitId).isEqualTo(commit.name());
String expectedContentType = "text/plain";
if (COMMIT_MSG.equals(path)) {
expectedContentType = FileContentUtil.TEXT_X_GERRIT_COMMIT_MESSAGE;
} else if (MERGE_LIST.equals(path)) {
expectedContentType = FileContentUtil.TEXT_X_GERRIT_MERGE_LIST;
}
assertThat(diff.metaB.contentType).isEqualTo(expectedContentType);
assertThat(diff.metaB.lines).isEqualTo(expectedLines.size());
assertThat(diff.metaB.name).isEqualTo(path);
assertThat(diff.metaB.webLinks).isNull();
assertThat(diff.content).hasSize(1);
DiffInfo.ContentEntry contentEntry = diff.content.get(0);
assertThat(contentEntry.b).containsExactlyElementsIn(expectedLines)
.inOrder();
assertThat(contentEntry.a).isNull();
assertThat(contentEntry.ab).isNull();
assertThat(contentEntry.common).isNull();
assertThat(contentEntry.editA).isNull();
assertThat(contentEntry.editB).isNull();
assertThat(contentEntry.skip).isNull();
}
}

80
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/GetMergeListIT.java
View File

@ -1,80 +0,0 @@
// Copyright (C) 2016 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.api.change;
import static com.google.common.truth.Truth.assertThat;
import static org.eclipse.jgit.lib.Constants.HEAD;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.common.CommitInfo;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
import java.util.List;
@NoHttpd
public class GetMergeListIT extends AbstractDaemonTest {
@Test
public void getMergeList() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
PushOneCommit.Result gp1 = pushFactory
.create(db, admin.getIdent(), testRepo, "grand parent 1",
ImmutableMap.of("foo", "foo-1.1", "bar", "bar-1.1"))
.to("refs/for/master");
PushOneCommit.Result p1 = pushFactory
.create(db, admin.getIdent(), testRepo, "parent 1",
ImmutableMap.of("foo", "foo-1.2", "bar", "bar-1.2"))
.to("refs/for/master");
// reset HEAD in order to create a sibling of the first change
testRepo.reset(initial);
PushOneCommit.Result gp2 = pushFactory
.create(db, admin.getIdent(), testRepo, "grand parent 1",
ImmutableMap.of("foo", "foo-2.1", "bar", "bar-2.1"))
.to("refs/for/master");
PushOneCommit.Result p2 = pushFactory
.create(db, admin.getIdent(), testRepo, "parent 2",
ImmutableMap.of("foo", "foo-2.2", "bar", "bar-2.2"))
.to("refs/for/master");
PushOneCommit m = pushFactory.create(db, admin.getIdent(), testRepo,
"merge", ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
m.setParents(ImmutableList.of(p1.getCommit(), p2.getCommit()));
PushOneCommit.Result result = m.to("refs/for/master");
result.assertOkStatus();
List<CommitInfo> mergeList =
gApi.changes().id(result.getChangeId()).current().getMergeList().get();
assertThat(mergeList).hasSize(2);
assertThat(mergeList.get(0).commit).isEqualTo(p2.getCommit().name());
assertThat(mergeList.get(1).commit).isEqualTo(gp2.getCommit().name());
mergeList = gApi.changes().id(result.getChangeId()).current().getMergeList()
.withUninterestingParent(2).get();
assertThat(mergeList).hasSize(2);
assertThat(mergeList.get(0).commit).isEqualTo(p1.getCommit().name());
assertThat(mergeList.get(1).commit).isEqualTo(gp1.getCommit().name());
}
}

209
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/MergeListIT.java Normal file
View File

@ -0,0 +1,209 @@
// Copyright (C) 2016 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.api.change;
import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.lib.Constants.HEAD;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.common.CommitInfo;
import com.google.gerrit.extensions.common.DiffInfo;
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.inject.Inject;
import org.eclipse.jgit.dircache.InvalidPathException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
import org.junit.Before;
import org.junit.Test;
import java.io.ByteArrayOutputStream;
import java.util.List;
import java.util.Set;
@NoHttpd
public class MergeListIT extends AbstractDaemonTest {
private String changeId;
private RevCommit merge;
private RevCommit parent1;
private RevCommit grandParent1;
private RevCommit parent2;
private RevCommit grandParent2;
@Inject
private ChangeEditModifier modifier;
@Inject
private ChangeEditUtil editUtil;
@Before
public void setup() throws Exception {
ObjectId initial = repo().exactRef(HEAD).getLeaf().getObjectId();
PushOneCommit.Result gp1 = pushFactory
.create(db, admin.getIdent(), testRepo, "grand parent 1",
ImmutableMap.of("foo", "foo-1.1", "bar", "bar-1.1"))
.to("refs/for/master");
grandParent1 = gp1.getCommit();
PushOneCommit.Result p1 = pushFactory
.create(db, admin.getIdent(), testRepo, "parent 1",
ImmutableMap.of("foo", "foo-1.2", "bar", "bar-1.2"))
.to("refs/for/master");
parent1 = p1.getCommit();
// reset HEAD in order to create a sibling of the first change
testRepo.reset(initial);
PushOneCommit.Result gp2 = pushFactory
.create(db, admin.getIdent(), testRepo, "grand parent 2",
ImmutableMap.of("foo", "foo-2.1", "bar", "bar-2.1"))
.to("refs/for/master");
grandParent2 = gp2.getCommit();
PushOneCommit.Result p2 = pushFactory
.create(db, admin.getIdent(), testRepo, "parent 2",
ImmutableMap.of("foo", "foo-2.2", "bar", "bar-2.2"))
.to("refs/for/master");
parent2 = p2.getCommit();
PushOneCommit m = pushFactory.create(db, admin.getIdent(), testRepo,
"merge", ImmutableMap.of("foo", "foo-1", "bar", "bar-2"));
m.setParents(ImmutableList.of(p1.getCommit(), p2.getCommit()));
PushOneCommit.Result result = m.to("refs/for/master");
result.assertOkStatus();
merge = result.getCommit();
changeId = result.getChangeId();
}
@Test
public void getMergeList() throws Exception {
List<CommitInfo> mergeList = current(changeId).getMergeList().get();
assertThat(mergeList).hasSize(2);
assertThat(mergeList.get(0).commit).isEqualTo(parent2.name());
assertThat(mergeList.get(1).commit).isEqualTo(grandParent2.name());
mergeList = current(changeId).getMergeList()
.withUninterestingParent(2).get();
assertThat(mergeList).hasSize(2);
assertThat(mergeList.get(0).commit).isEqualTo(parent1.name());
assertThat(mergeList.get(1).commit).isEqualTo(grandParent1.name());
}
@Test
public void getMergeListContent() throws Exception {
BinaryResult bin = current(changeId).file(MERGE_LIST).content();
ByteArrayOutputStream os = new ByteArrayOutputStream();
bin.writeTo(os);
String content = new String(os.toByteArray(), UTF_8);
assertThat(content).isEqualTo(
getMergeListContent(parent2, grandParent2));
}
@Test
public void getFileList() throws Exception {
assertThat(getFiles(changeId)).contains(MERGE_LIST);
assertThat(getFiles(changeId, 1)).contains(MERGE_LIST);
assertThat(getFiles(changeId, 2)).contains(MERGE_LIST);
assertThat(getFiles(createChange().getChangeId()))
.doesNotContain(MERGE_LIST);
}
@Test
public void getDiffForMergeList() throws Exception {
DiffInfo diff = getMergeListDiff(changeId);
assertDiffForNewFile(diff, merge, MERGE_LIST,
getMergeListContent(parent2, grandParent2));
diff = getMergeListDiff(changeId, 1);
assertDiffForNewFile(diff, merge, MERGE_LIST,
getMergeListContent(parent2, grandParent2));
diff = getMergeListDiff(changeId, 2);
assertDiffForNewFile(diff, merge, MERGE_LIST,
getMergeListContent(parent1, grandParent1));
}
@Test
public void editMergeList() throws Exception {
ChangeData cd = getOnlyElement(queryProvider.get().byKeyPrefix(changeId));
modifier.createEdit(cd.change(), cd.currentPatchSet());
exception.expect(InvalidPathException.class);
exception.expectMessage("Invalid path: " + MERGE_LIST);
modifier.modifyFile(editUtil.byChange(cd.change()).get(), MERGE_LIST,
RawInputUtil.create("new content"));
}
@Test
public void deleteMergeList() throws Exception {
ChangeData cd = getOnlyElement(queryProvider.get().byKeyPrefix(changeId));
modifier.createEdit(cd.change(), cd.currentPatchSet());
exception.expect(InvalidChangeOperationException.class);
exception.expectMessage("no changes were made");
modifier.deleteFile(editUtil.byChange(cd.change()).get(), MERGE_LIST);
}
private String getMergeListContent(RevCommit... commits) {
StringBuilder mergeList = new StringBuilder("Merge List:\n\n");
for (RevCommit c : commits) {
mergeList.append("* ")
.append(c.abbreviate(8).name())
.append(" ")
.append(c.getShortMessage())
.append("\n");
}
return mergeList.toString();
}
private Set<String> getFiles(String changeId) throws Exception {
return current(changeId).files().keySet();
}
private Set<String> getFiles(String changeId, int parent) throws Exception {
return current(changeId).files(parent).keySet();
}
private DiffInfo getMergeListDiff(String changeId) throws Exception {
return current(changeId).file(MERGE_LIST).diff();
}
private DiffInfo getMergeListDiff(String changeId, int parent)
throws Exception {
return current(changeId).file(MERGE_LIST).diff(parent);
}
private RevisionApi current(String changeId) throws Exception {
return gApi.changes()
.id(changeId)
.current();
}
}

59
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@ -20,11 +20,13 @@ import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.PATCH;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
import static com.google.gerrit.reviewdb.client.Patch.COMMIT_MSG;
import static com.google.gerrit.reviewdb.client.Patch.MERGE_LIST;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.eclipse.jgit.lib.Constants.HEAD;
import static org.junit.Assert.fail;
import com.google.common.base.Joiner;
import com.google.common.base.Predicate;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@ -45,7 +47,6 @@ import com.google.gerrit.extensions.client.ChangeStatus;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
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.FileInfo;
@ -539,7 +540,7 @@ public class RevisionIT extends AbstractDaemonTest {
.revision(r.getCommit().name())
.files()
.keySet()
).containsExactly(COMMIT_MSG, "foo", "bar");
).containsExactly(COMMIT_MSG, MERGE_LIST, "foo", "bar");
// list files against parent 1
assertThat(gApi.changes()
@ -547,7 +548,7 @@ public class RevisionIT extends AbstractDaemonTest {
.revision(r.getCommit().name())
.files(1)
.keySet()
).containsExactly(COMMIT_MSG, "bar");
).containsExactly(COMMIT_MSG, MERGE_LIST, "bar");
// list files against parent 2
assertThat(gApi.changes()
@ -555,7 +556,7 @@ public class RevisionIT extends AbstractDaemonTest {
.revision(r.getCommit().name())
.files(2)
.keySet()
).containsExactly(COMMIT_MSG, "foo");
).containsExactly(COMMIT_MSG, MERGE_LIST, "foo");
}
@Test
@ -853,66 +854,40 @@ public class RevisionIT extends AbstractDaemonTest {
.file(path)
.diff();
List<String> expectedLines = new ArrayList<>();
List<String> headers = new ArrayList<>();
if (path.equals(COMMIT_MSG)) {
RevCommit c = pushResult.getCommit();
RevCommit parentCommit = c.getParents()[0];
String parentCommitId = testRepo.getRevWalk().getObjectReader()
.abbreviate(parentCommit.getId(), 8).name();
expectedLines.add("Parent: " + parentCommitId + " ("
headers.add("Parent: " + parentCommitId + " ("
+ parentCommit.getShortMessage() + ")");
SimpleDateFormat dtfmt =
new SimpleDateFormat("yyyy-MM-dd HH:mm:ss Z", Locale.US);
PersonIdent author = c.getAuthorIdent();
dtfmt.setTimeZone(author.getTimeZone());
expectedLines.add("Author: " + author.getName() + " <"
headers.add("Author: " + author.getName() + " <"
+ author.getEmailAddress() + ">");
expectedLines.add("AuthorDate: "
headers.add("AuthorDate: "
+ dtfmt.format(Long.valueOf(author.getWhen().getTime())));
PersonIdent committer = c.getCommitterIdent();
dtfmt.setTimeZone(committer.getTimeZone());
expectedLines.add("Commit: " + committer.getName() + " <"
headers.add("Commit: " + committer.getName() + " <"
+ committer.getEmailAddress() + ">");
expectedLines.add("CommitDate: "
headers.add("CommitDate: "
+ dtfmt.format(Long.valueOf(committer.getWhen().getTime())));
expectedLines.add("");
headers.add("");
}
for (String line : expectedContentSideB.split("\n")) {
expectedLines.add(line);
if (!headers.isEmpty()) {
String header = Joiner.on("\n").join(headers);
expectedContentSideB = header + "\n" + expectedContentSideB;
}
assertThat(diff.binary).isNull();
assertThat(diff.changeType).isEqualTo(ChangeType.ADDED);
assertThat(diff.diffHeader).isNotNull();
assertThat(diff.intralineStatus).isNull();
assertThat(diff.webLinks).isNull();
assertThat(diff.metaA).isNull();
assertThat(diff.metaB).isNotNull();
assertThat(diff.metaB.commitId).isEqualTo(pushResult.getCommit().name());
assertThat(diff.metaB.contentType).isEqualTo(
path.equals(COMMIT_MSG)
? "text/x-gerrit-commit-message"
: "text/plain");
assertThat(diff.metaB.lines).isEqualTo(expectedLines.size());
assertThat(diff.metaB.name).isEqualTo(path);
assertThat(diff.metaB.webLinks).isNull();
assertThat(diff.content).hasSize(1);
DiffInfo.ContentEntry contentEntry = diff.content.get(0);
assertThat(contentEntry.b).hasSize(expectedLines.size());
for (int i = 0; i < contentEntry.b.size(); i++) {
assertThat(contentEntry.b.get(i)).isEqualTo(expectedLines.get(i));
}
assertThat(contentEntry.a).isNull();
assertThat(contentEntry.ab).isNull();
assertThat(contentEntry.common).isNull();
assertThat(contentEntry.editA).isNull();
assertThat(contentEntry.editB).isNull();
assertThat(contentEntry.skip).isNull();
assertDiffForNewFile(diff, pushResult.getCommit(), path,
expectedContentSideB);
}
}

39
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
View File

@ -32,9 +32,11 @@ import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.PostReview;
@ -148,8 +150,8 @@ public class CommentsIT extends AbstractDaemonTest {
@Test
public void postCommentOnMergeCommitChange() throws Exception {
for (Integer line : lines) {
final String file = "/COMMIT_MSG";
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");
String file = "foo";
PushOneCommit.Result r = createMergeCommitChange("refs/for/master", file);
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput input = new ReviewInput();
@ -165,6 +167,39 @@ public class CommentsIT extends AbstractDaemonTest {
assertThat(Lists.transform(result.get(file), infoToInput(file)))
.containsExactly(c1, c2, c3, c4);
}
// for the commit message comments on the auto-merge are not possible
for (Integer line : lines) {
String file = Patch.COMMIT_MSG;
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");
String changeId = r.getChangeId();
String revId = r.getCommit().getName();
ReviewInput input = new ReviewInput();
CommentInput c1 = newComment(file, Side.REVISION, line, "ps-1");
CommentInput c2 = newCommentOnParent(file, 1, line, "parent-1 of ps-1");
CommentInput c3 = newCommentOnParent(file, 2, line, "parent-2 of ps-1");
input.comments = new HashMap<>();
input.comments.put(file, ImmutableList.of(c1, c2, c3));
revision(r).review(input);
Map<String, List<CommentInfo>> result = getPublishedComments(changeId, revId);
assertThat(result).isNotEmpty();
assertThat(Lists.transform(result.get(file), infoToInput(file)))
.containsExactly(c1, c2, c3);
}
}
@Test
public void postCommentOnCommitMessageOnAutoMerge() throws Exception {
PushOneCommit.Result r = createMergeCommitChange("refs/for/master");
ReviewInput input = new ReviewInput();
CommentInput c =
newComment(Patch.COMMIT_MSG, Side.PARENT, 0, "comment on auto-merge");
input.comments = new HashMap<>();
input.comments.put(Patch.COMMIT_MSG, ImmutableList.of(c));
exception.expect(BadRequestException.class);
exception.expectMessage(
"cannot comment on " + Patch.COMMIT_MSG + " on auto-merge");
revision(r).review(input);
}
@Test

18
gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/FileInfo.java
View File

@ -56,6 +56,12 @@ public class FileInfo extends JavaScriptObject {
} else if (Patch.COMMIT_MSG.equals(b.path())) {
return 1;
}
if (Patch.MERGE_LIST.equals(a.path())) {
return -1;
} else if (Patch.MERGE_LIST.equals(b.path())) {
return 1;
}
// Look at file suffixes to check if it makes sense to use a different order
int s1 = a.path().lastIndexOf('.');
int s2 = b.path().lastIndexOf('.');
@ -76,9 +82,15 @@ public class FileInfo extends JavaScriptObject {
}
public static String getFileName(String path) {
String fileName = Patch.COMMIT_MSG.equals(path)
? "Commit Message"
: path;
String fileName;
if (Patch.COMMIT_MSG.equals(path)) {
fileName = "Commit Message";
} else if (Patch.MERGE_LIST.equals(path)) {
fileName = "Merge List";
} else {
fileName = path;
}
int s = fileName.lastIndexOf('/');
return s >= 0 ? fileName.substring(s + 1) : fileName;
}

6
gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
View File

@ -486,7 +486,11 @@ public class Dispatcher {
} else if ("unified".equals(panel)) {
unified(token, baseId, id, side, line);
} else if ("edit".equals(panel)) {
codemirrorForEdit(token, id, line);
if (!Patch.isMagic(id.get()) || Patch.COMMIT_MSG.equals(id.get())) {
codemirrorForEdit(token, id, line);
} else {
Gerrit.display(token, new NotFoundScreen());
}
} else {
Gerrit.display(token, new NotFoundScreen());
}

69
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/FileTable.java
View File

@ -267,11 +267,18 @@ public class FileTable extends FlowPanel {
if (table != null) {
String self = Gerrit.selfRedirect(null);
for (FileInfo info : Natives.asList(table.list)) {
Window.open(self + "#" + url(info), "_blank", null);
if (canOpen(info.path())) {
Window.open(self + "#" + url(info), "_blank", null);
}
}
}
}
private boolean canOpen(String path) {
return mode != Mode.EDIT || !Patch.isMagic(path)
|| Patch.COMMIT_MSG.equals(path);
}
private void setTable(MyTable table) {
clear();
add(table);
@ -429,7 +436,10 @@ public class FileTable extends FlowPanel {
@Override
protected void onOpenRow(int row) {
if (1 <= row && row <= list.length()) {
Gerrit.display(url(list.get(row - 1)));
FileInfo info = list.get(row - 1);
if (canOpen(info.path())) {
Gerrit.display(url(info));
}
}
}
@ -452,7 +462,10 @@ public class FileTable extends FlowPanel {
@Override
public void onKeyPress(KeyPressEvent event) {
Gerrit.display(url(list.get(index)));
FileInfo info = list.get(index);
if (canOpen(info.path())) {
Gerrit.display(url(info));
}
}
}
}
@ -538,7 +551,7 @@ public class FileTable extends FlowPanel {
bytesDeleted = 0;
for (int i = 0; i < list.length(); i++) {
FileInfo info = list.get(i);
if (!Patch.COMMIT_MSG.equals(info.path())) {
if (!Patch.isMagic(info.path())) {
if (!info.binary()) {
hasNonBinaryFile = true;
inserted += info.linesInserted();
@ -628,7 +641,7 @@ public class FileTable extends FlowPanel {
private void columnDeleteRestore(SafeHtmlBuilder sb, FileInfo info) {
sb.openTd().setStyleName(R.css().restoreDelete());
if (hasUser) {
if (!Patch.COMMIT_MSG.equals(info.path())) {
if (!Patch.isMagic(info.path())) {
boolean editable = isEditable(info);
sb.openDiv()
.openElement("button")
@ -659,7 +672,7 @@ public class FileTable extends FlowPanel {
private void columnStatus(SafeHtmlBuilder sb, FileInfo info) {
sb.openTd().setStyleName(R.css().status());
if (!Patch.COMMIT_MSG.equals(info.path())
if (!Patch.isMagic(info.path())
&& info.status() != null
&& !ChangeType.MODIFIED.matches(info.status())) {
sb.append(info.status());
@ -668,20 +681,43 @@ public class FileTable extends FlowPanel {
}
private void columnPath(SafeHtmlBuilder sb, FileInfo info) {
sb.openTd()
.setStyleName(R.css().pathColumn())
.openAnchor();
String path = info.path();
sb.openTd()
.setStyleName(R.css().pathColumn());
if (!canOpen(path)) {
sb.openDiv();
appendPath(path);
sb.closeDiv();
sb.closeTd();
return;
}
sb.openAnchor();
if (mode == Mode.EDIT && !isEditable(info)) {
sb.setAttribute("onclick", RESTORE + "(event," + info._row() + ")");
} else {
sb.setAttribute("href", "#" + url(info))
.setAttribute("onclick", OPEN + "(event," + info._row() + ")");
}
appendPath(path);
sb.closeAnchor();
if (info.oldPath() != null) {
sb.br();
sb.openSpan().setStyleName(R.css().renameCopySource())
.append(info.oldPath())
.closeSpan();
}
sb.closeTd();
}
private void appendPath(String path) {
if (Patch.COMMIT_MSG.equals(path)) {
sb.append(Util.C.commitMessage());
} else if (Patch.MERGE_LIST.equals(path)) {
sb.append(Util.C.mergeList());
} else if (Gerrit.getUserPreferences().muteCommonPathPrefixes()) {
int commonPrefixLen = commonPrefix(path);
if (commonPrefixLen > 0) {
@ -694,15 +730,6 @@ public class FileTable extends FlowPanel {
} else {
sb.append(path);
}
sb.closeAnchor();
if (info.oldPath() != null) {
sb.br();
sb.openSpan().setStyleName(R.css().renameCopySource())
.append(info.oldPath())
.closeSpan();
}
sb.closeTd();
}
private int commonPrefix(String path) {
@ -784,7 +811,7 @@ public class FileTable extends FlowPanel {
private void columnDelta1(SafeHtmlBuilder sb, FileInfo info) {
sb.openTd().setStyleName(R.css().deltaColumn1());
if (!Patch.COMMIT_MSG.equals(info.path()) && !info.binary()) {
if (!Patch.isMagic(info.path()) && !info.binary()) {
if (showChangeSizeBars) {
sb.append(info.linesInserted() + info.linesDeleted());
} else if (!ChangeType.DELETED.matches(info.status())) {
@ -813,7 +840,7 @@ public class FileTable extends FlowPanel {
private void columnDelta2(SafeHtmlBuilder sb, FileInfo info) {
sb.openTd().setStyleName(R.css().deltaColumn2());
if (showChangeSizeBars
&& !Patch.COMMIT_MSG.equals(info.path()) && !info.binary()
&& !Patch.isMagic(info.path()) && !info.binary()
&& (info.linesInserted() != 0 || info.linesDeleted() != 0)) {
int w = 80;
int t = inserted + deleted;

4
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/Message.java
View File

@ -176,6 +176,10 @@ class Message extends Composite {
if (l != null) {
comments.add(new FileComments(clp, ps, Util.C.commitMessage(), l));
}
l = m.remove(Patch.MERGE_LIST);
if (l != null) {
comments.add(new FileComments(clp, ps, Util.C.mergeList(), l));
}
for (Map.Entry<String, List<CommentInfo>> e : m.entrySet()) {
comments.add(new FileComments(clp, ps, e.getKey(), e.getValue()));
}

7
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ReplyBox.java
View File

@ -422,12 +422,17 @@ public class ReplyBox extends Composite {
comments.add(new FileComments(clp, psId,
Util.C.commitMessage(), copyPath(Patch.COMMIT_MSG, l)));
}
l = m.get(Patch.MERGE_LIST);
if (l != null) {
comments.add(new FileComments(clp, psId, Util.C.commitMessage(),
copyPath(Patch.MERGE_LIST, l)));
}
List<String> paths = new ArrayList<>(m.keySet());
Collections.sort(paths);
for (String path : paths) {
if (!path.equals(Patch.COMMIT_MSG)) {
if (!Patch.isMagic(path)) {
comments.add(new FileComments(clp, psId,
path, copyPath(path, m.get(path))));
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.java
View File

@ -63,6 +63,7 @@ public interface ChangeConstants extends Constants {
String patchTableColumnComments();
String patchTableColumnSize();
String commitMessage();
String mergeList();
String patchTablePrev();
String patchTableNext();

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeConstants.properties
View File

@ -45,6 +45,7 @@ patchTableColumnName = File Path
patchTableColumnComments = Comments
patchTableColumnSize = Size
commitMessage = Commit Message
mergeList = Merge List
patchTablePrev = Previous file
patchTableNext = Next file

2
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java
View File

@ -122,6 +122,8 @@ public class Header extends Composite {
SafeHtmlBuilder b = new SafeHtmlBuilder();
if (Patch.COMMIT_MSG.equals(path)) {
return b.append(Util.C.commitMessage());
} else if (Patch.MERGE_LIST.equals(path)) {
return b.append(Util.C.mergeList());
}
int s = path.lastIndexOf('/') + 1;

4
gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PatchSetSelectBox.java
View File

@ -128,7 +128,7 @@ class PatchSetSelectBox extends Composite {
if (meta == null) {
return;
}
if (!Patch.COMMIT_MSG.equals(path)) {
if (!Patch.isMagic(path)) {
linkPanel.add(createDownloadLink());
}
if (!binary && open && idActive != null && Gerrit.isSignedIn()) {
@ -147,7 +147,7 @@ class PatchSetSelectBox extends Composite {
void setUpBlame(final CodeMirror cm, final boolean isBase,
final PatchSet.Id rev, final String path) {
if (!Patch.COMMIT_MSG.equals(path) && Gerrit.isSignedIn()
if (!Patch.isMagic(path) && Gerrit.isSignedIn()
&& Gerrit.info().change().allowBlame()) {
Anchor blameIcon = createBlameIcon();
blameIcon.addClickHandler(new ClickHandler() {

16
gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Patch.java
View File

@ -22,6 +22,22 @@ public final class Patch {
/** Magical file name which represents the commit message. */
public static final String COMMIT_MSG = "/COMMIT_MSG";
/** Magical file name which represents the merge list of a merge commit. */
public static final String MERGE_LIST = "/MERGE_LIST";
/**
* Checks if the given path represents a magic file. A magic file is a
* generated file that is automatically included into changes. It does not
* exist in the commit of the patch set.
*
* @param path the file path
* @return {@code true} if the path represents a magic file, otherwise
* {@code false}.
*/
public static boolean isMagic(String path) {
return COMMIT_MSG.equals(path) || MERGE_LIST.equals(path);
}
public static class Key extends StringKey<PatchSet.Id> {
private static final long serialVersionUID = 1L;

26
gerrit-server/src/main/java/com/google/gerrit/server/PatchLineCommentsUtil.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server;
import static java.util.stream.Collectors.toList;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.stream.Collectors.toList;
@ -27,6 +29,7 @@ import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.CommentInfo;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
@ -210,10 +213,27 @@ public class PatchLineCommentsUtil {
public List<PatchLineComment> publishedByPatchSet(ReviewDb db,
ChangeNotes notes, PatchSet.Id psId) throws OrmException {
if (!migration.readChanges()) {
return sort(
db.patchComments().publishedByPatchSet(psId).toList());
return removeCommentsOnAncestorOfCommitMessage(sort(
db.patchComments().publishedByPatchSet(psId).toList()));
}
return commentsOnPatchSet(notes.load().getComments().values(), psId);
return removeCommentsOnAncestorOfCommitMessage(
commentsOnPatchSet(notes.load().getComments().values(), psId));
}
/**
* For the commit message the A side in a diff view is always empty when a
* comparison against an ancestor is done, so there can't be any comments on
* this ancestor. However earlier we showed the auto-merge commit message on
* side A when for a merge commit a comparison against the auto-merge was
* done. From that time there may still be comments on the auto-merge commit
* message and those we want to filter out.
*/
private List<PatchLineComment> removeCommentsOnAncestorOfCommitMessage(
List<PatchLineComment> list) {
return list.stream()
.filter(c -> c.getSide() != 0
|| !Patch.COMMIT_MSG.equals(c.getKey().getParentKey().get()))
.collect(toList());
}
public List<PatchLineComment> draftByPatchSetAuthor(ReviewDb db,

1
gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
View File

@ -1056,6 +1056,7 @@ public class ChangeJson {
if (has(ALL_FILES) || (out.isCurrent && has(CURRENT_FILES))) {
out.files = fileInfoJson.toFileInfoMap(c, in);
out.files.remove(Patch.COMMIT_MSG);
out.files.remove(Patch.MERGE_LIST);
}
if ((out.isCurrent || (out.draft != null && out.draft))

4
gerrit-server/src/main/java/com/google/gerrit/server/change/FileContentUtil.java
View File

@ -54,6 +54,7 @@ import java.util.zip.ZipOutputStream;
@Singleton
public class FileContentUtil {
public static final String TEXT_X_GERRIT_COMMIT_MESSAGE = "text/x-gerrit-commit-message";
public static final String TEXT_X_GERRIT_MERGE_LIST = "text/x-gerrit-merge-list";
private static final String X_GIT_SYMLINK = "x-git/symlink";
private static final String X_GIT_GITLINK = "x-git/gitlink";
private static final int MAX_SIZE = 5 << 20;
@ -264,6 +265,9 @@ public class FileContentUtil {
if (Patch.COMMIT_MSG.equals(path)) {
return TEXT_X_GERRIT_COMMIT_MESSAGE;
}
if (Patch.MERGE_LIST.equals(path)) {
return TEXT_X_GERRIT_MERGE_LIST;
}
if (project != null) {
for (ProjectState p : project.tree()) {
String t = p.getConfig().getMimeTypes().getMimeType(path);

26
gerrit-server/src/main/java/com/google/gerrit/server/change/GetContent.java
View File

@ -24,6 +24,8 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.ComparisonType;
import com.google.gerrit.server.patch.Text;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@ -68,6 +70,12 @@ public class GetContent implements RestReadView<FileResource> {
return BinaryResult.create(msg)
.setContentType(FileContentUtil.TEXT_X_GERRIT_COMMIT_MESSAGE)
.base64();
} else if (Patch.MERGE_LIST.equals(path)) {
byte[] mergeList = getMergeList(
rsrc.getRevision().getChangeResource().getNotes());
return BinaryResult.create(mergeList)
.setContentType(FileContentUtil.TEXT_X_GERRIT_MERGE_LIST)
.base64();
}
return fileContentUtil.getContent(
rsrc.getRevision().getControl().getProjectControl().getProjectState(),
@ -92,4 +100,22 @@ public class GetContent implements RestReadView<FileResource> {
throw new NoSuchChangeException(changeId, e);
}
}
private byte[] getMergeList(ChangeNotes notes)
throws NoSuchChangeException, OrmException, IOException {
Change.Id changeId = notes.getChangeId();
PatchSet ps = psUtil.current(db.get(), notes);
if (ps == null) {
throw new NoSuchChangeException(changeId);
}
try (Repository git = gitManager.openRepository(notes.getProjectName());
RevWalk revWalk = new RevWalk(git)) {
return Text.forMergeList(ComparisonType.againstAutoMerge(),
revWalk.getObjectReader(),
ObjectId.fromString(ps.getRevision().get())).getContent();
} catch (RepositoryNotFoundException e) {
throw new NoSuchChangeException(changeId, e);
}
}
}

29
gerrit-server/src/main/java/com/google/gerrit/server/change/GetMergeList.java
View File

@ -22,6 +22,7 @@ import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.MergeListBuilder;
import com.google.inject.Inject;
import org.eclipse.jgit.lib.ObjectId;
@ -46,7 +47,8 @@ public class GetMergeList implements RestReadView<RevisionResource> {
private boolean addLinks;
@Inject
GetMergeList(GitRepositoryManager repoManager, ChangeJson.Factory json) {
GetMergeList(GitRepositoryManager repoManager,
ChangeJson.Factory json) {
this.repoManager = repoManager;
this.json = json;
}
@ -62,7 +64,6 @@ public class GetMergeList implements RestReadView<RevisionResource> {
@Override
public Response<List<CommitInfo>> apply(RevisionResource rsrc)
throws BadRequestException, IOException {
List<CommitInfo> result = new ArrayList<>();
Project.NameKey p = rsrc.getChange().getProject();
try (Repository repo = repoManager.openRepository(p);
RevWalk rw = new RevWalk(repo)) {
@ -76,26 +77,22 @@ public class GetMergeList implements RestReadView<RevisionResource> {
}
if (commit.getParentCount() < 2) {
return Response.<List<CommitInfo>> ok(ImmutableList.<CommitInfo> of());
}
for (int parent = 0; parent < commit.getParentCount(); parent++) {
if (parent == uninterestingParent - 1) {
rw.markUninteresting(commit.getParent(parent));
} else {
rw.markStart(commit.getParent(parent));
}
return createResponse(rsrc, ImmutableList.<CommitInfo> of());
}
List<RevCommit> commits =
MergeListBuilder.build(rw, commit, uninterestingParent);
List<CommitInfo> result = new ArrayList<>(commits.size());
ChangeJson changeJson = json.create(ChangeJson.NO_OPTIONS);
RevCommit c;
while ((c = rw.next()) != null) {
CommitInfo info =
changeJson.toCommit(rsrc.getControl(), rw, c, addLinks, true);
result.add(info);
for (RevCommit c : commits) {
result.add(changeJson.toCommit(rsrc.getControl(), rw, c, addLinks, true));
}
return createResponse(rsrc, result);
}
}
private static Response<List<CommitInfo>> createResponse(
RevisionResource rsrc, List<CommitInfo> result) {
Response<List<CommitInfo>> r = Response.ok(result);
if (rsrc.isCacheable()) {
r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));

10
gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
View File

@ -324,11 +324,19 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
while (mapItr.hasNext()) {
Map.Entry<String, List<CommentInput>> ent = mapItr.next();
String path = ent.getKey();
if (!filePaths.contains(path) && !Patch.COMMIT_MSG.equals(path)) {
if (!filePaths.contains(path) && !Patch.isMagic(path)) {
throw new BadRequestException(String.format(
"file %s not found in revision %s",
path, revision.getChange().currentPatchSetId()));
}
if (Patch.isMagic(path)) {
for (CommentInput comment : ent.getValue()) {
if (comment.side == Side.PARENT && comment.parent == null) {
throw new BadRequestException(
String.format("cannot comment on %s on auto-merge", path));
}
}
}
List<CommentInput> list = ent.getValue();
if (list == null) {

2
gerrit-server/src/main/java/com/google/gerrit/server/events/EventFactory.java
View File

@ -496,7 +496,7 @@ public class EventFactory {
List<Patch> list =
patchListCache.get(change, patchSet).toPatchList(pId);
for (Patch pe : list) {
if (!Patch.COMMIT_MSG.equals(pe.getFileName())) {
if (!Patch.isMagic(pe.getFileName())) {
p.sizeDeletions -= pe.getDeletions();
p.sizeInsertions += pe.getInsertions();
}

2
gerrit-server/src/main/java/com/google/gerrit/server/mail/ChangeEmail.java
View File

@ -259,7 +259,7 @@ public abstract class ChangeEmail extends NotificationEmail {
detail.append("---\n");
PatchList patchList = getPatchList();
for (PatchListEntry p : patchList.getPatches()) {
if (Patch.COMMIT_MSG.equals(p.getNewName())) {
if (Patch.isMagic(p.getNewName())) {
continue;
}
detail.append(p.getChangeType().getCode())

7
gerrit-server/src/main/java/com/google/gerrit/server/mail/CommentSender.java
View File

@ -75,7 +75,7 @@ public class CommentSender extends ReplyToChangeSender {
Set<String> paths = new HashSet<>();
for (PatchLineComment c : plc) {
Patch.Key p = c.getKey().getParentKey();
if (!Patch.COMMIT_MSG.equals(p.getFileName())) {
if (!Patch.isMagic(p.getFileName())) {
paths.add(p.getFileName());
}
}
@ -137,6 +137,8 @@ public class CommentSender extends ReplyToChangeSender {
}
if (Patch.COMMIT_MSG.equals(pk.get())) {
cmts.append("Commit Message:\n\n");
} else if (Patch.MERGE_LIST.equals(pk.get())) {
cmts.append("Merge List:\n\n");
} else {
cmts.append("File ").append(pk.get()).append(":\n\n");
}
@ -144,8 +146,7 @@ public class CommentSender extends ReplyToChangeSender {
if (patchList != null) {
try {
currentFileData =
new PatchFile(repo, patchList, pk.get());
currentFileData = new PatchFile(repo, patchList, pk.get());
} catch (IOException e) {
log.warn(String.format(
"Cannot load %s from %s in %s",

77
gerrit-server/src/main/java/com/google/gerrit/server/patch/ComparisonType.java Normal file
View File

@ -0,0 +1,77 @@
// Copyright (C) 2016 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.patch;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.gerrit.server.ioutil.BasicSerialization.readVarInt32;
import static com.google.gerrit.server.ioutil.BasicSerialization.writeVarInt32;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
public class ComparisonType {
/** 1-based parent */
private final Integer parentNum;
private final boolean autoMerge;
public static ComparisonType againstOtherPatchSet() {
return new ComparisonType(null, false);
}
public static ComparisonType againstParent(int parentNum) {
return new ComparisonType(parentNum, false);
}
public static ComparisonType againstAutoMerge() {
return new ComparisonType(null, true);
}
private ComparisonType(Integer parentNum, boolean autoMerge) {
this.parentNum = parentNum;
this.autoMerge = autoMerge;
}
public boolean isAgainstParentOrAutoMerge() {
return isAgainstParent() || isAgainstAutoMerge();
}
public boolean isAgainstParent() {
return parentNum != null;
}
public boolean isAgainstAutoMerge() {
return autoMerge;
}
public int getParentNum() {
checkNotNull(parentNum);
return parentNum;
}
void writeTo(OutputStream out) throws IOException {
writeVarInt32(out, parentNum != null ? parentNum : 0);
writeVarInt32(out, autoMerge ? 1 : 0);
}
static ComparisonType readFrom(InputStream in) throws IOException {
int p = readVarInt32(in);
Integer parentNum = p > 0 ? p : null;
boolean autoMerge = readVarInt32(in) != 0;
return new ComparisonType(parentNum, autoMerge);
}
}

52
gerrit-server/src/main/java/com/google/gerrit/server/patch/MergeListBuilder.java Normal file
View File

@ -0,0 +1,52 @@
// Copyright (C) 2016 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.patch;
import com.google.common.collect.ImmutableList;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
public class MergeListBuilder {
public static List<RevCommit> build(RevWalk rw, RevCommit merge,
int uninterestingParent) throws IOException {
rw.reset();
rw.parseBody(merge);
if (merge.getParentCount() < 2) {
return ImmutableList.of();
}
for (int parent = 0; parent < merge.getParentCount(); parent++) {
RevCommit parentCommit = merge.getParent(parent);
rw.parseBody(parentCommit);
if (parent == uninterestingParent - 1) {
rw.markUninteresting(parentCommit);
} else {
rw.markStart(parentCommit);
}
}
List<RevCommit> result = new ArrayList<>();
RevCommit c;
while ((c = rw.next()) != null) {
result.add(c);
}
return result;
}
}

16
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchFile.java
View File

@ -44,9 +44,8 @@ public class PatchFile {
private Text a;
private Text b;
public PatchFile(final Repository repo, final PatchList patchList,
final String fileName) throws MissingObjectException,
IncorrectObjectTypeException, IOException {
public PatchFile(Repository repo, PatchList patchList, String fileName)
throws MissingObjectException, IncorrectObjectTypeException, IOException {
this.repo = repo;
this.entry = patchList.get(fileName);
@ -55,7 +54,7 @@ public class PatchFile {
final RevCommit bCommit = rw.parseCommit(patchList.getNewId());
if (Patch.COMMIT_MSG.equals(fileName)) {
if (patchList.isAgainstParent()) {
if (patchList.getComparisonType().isAgainstParentOrAutoMerge()) {
a = Text.EMPTY;
} else {
// For the initial commit, we have an empty tree on Side A
@ -68,7 +67,16 @@ public class PatchFile {
aTree = null;
bTree = null;
} else if (Patch.MERGE_LIST.equals(fileName)) {
// For the initial commit, we have an empty tree on Side A
RevObject object = rw.parseAny(patchList.getOldId());
a = object instanceof RevCommit
? Text.forMergeList(patchList.getComparisonType(), reader, object)
: Text.EMPTY;
b = Text.forMergeList(patchList.getComparisonType(), reader, bCommit);
aTree = null;
bTree = null;
} else {
if (patchList.getOldId() != null) {
aTree = rw.parseTree(patchList.getOldId());

28
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchList.java
View File

@ -58,16 +58,19 @@ public class PatchList implements Serializable {
@Nullable
private transient ObjectId oldId;
private transient ObjectId newId;
private transient boolean againstParent;
private transient boolean isMerge;
private transient ComparisonType comparisonType;
private transient int insertions;
private transient int deletions;
private transient PatchListEntry[] patches;
public PatchList(@Nullable final AnyObjectId oldId, final AnyObjectId newId,
final boolean againstParent, final PatchListEntry[] patches) {
public PatchList(@Nullable AnyObjectId oldId, AnyObjectId newId,
boolean isMerge, ComparisonType comparisonType,
PatchListEntry[] patches) {
this.oldId = oldId != null ? oldId.copy() : null;
this.newId = newId.copy();
this.againstParent = againstParent;
this.isMerge = isMerge;
this.comparisonType = comparisonType;
// We assume index 0 contains the magic commit message entry.
if (patches.length > 1) {
@ -97,9 +100,9 @@ public class PatchList implements Serializable {
return Collections.unmodifiableList(Arrays.asList(patches));
}
/** @return true if {@link #getOldId} is {@link #getNewId}'s ancestor. */
public boolean isAgainstParent() {
return againstParent;
/** @return the comparison type */
public ComparisonType getComparisonType() {
return comparisonType;
}
/** @return total number of new lines added. */
@ -144,9 +147,12 @@ public class PatchList implements Serializable {
if (Patch.COMMIT_MSG.equals(fileName)) {
return 0;
}
if (isMerge && Patch.MERGE_LIST.equals(fileName)) {
return 1;
}
int high = patches.length;
int low = 1;
int low = isMerge ? 2 : 1;
while (low < high) {
final int mid = (low + high) >>> 1;
final int cmp = patches[mid].getNewName().compareTo(fileName);
@ -166,7 +172,8 @@ public class PatchList implements Serializable {
try (DeflaterOutputStream out = new DeflaterOutputStream(buf)) {
writeCanBeNull(out, oldId);
writeNotNull(out, newId);
writeVarInt32(out, againstParent ? 1 : 0);
writeVarInt32(out, isMerge ? 1 : 0);
comparisonType.writeTo(out);
writeVarInt32(out, insertions);
writeVarInt32(out, deletions);
writeVarInt32(out, patches.length);
@ -182,7 +189,8 @@ public class PatchList implements Serializable {
try (InflaterInputStream in = new InflaterInputStream(buf)) {
oldId = readCanBeNull(in);
newId = readNotNull(in);
againstParent = readVarInt32(in) != 0;
isMerge = readVarInt32(in) != 0;
comparisonType = ComparisonType.readFrom(in);
insertions = readVarInt32(in);
deletions = readVarInt32(in);
final int cnt = readVarInt32(in);

6
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
View File

@ -35,7 +35,7 @@ import java.io.Serializable;
import java.util.Objects;
public class PatchListKey implements Serializable {
public static final long serialVersionUID = 22L;
public static final long serialVersionUID = 24L;
public static final BiMap<Whitespace, Character> WHITESPACE_TYPES = ImmutableBiMap.of(
Whitespace.IGNORE_NONE, 'N',
@ -138,6 +138,10 @@ public class PatchListKey implements Serializable {
n.append("..");
n.append(newId.name());
n.append(" ");
if (parentNum != null) {
n.append(parentNum);
n.append(" ");
}
n.append(whitespace.name());
n.append("]");
return n.toString();

100
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListLoader.java
View File

@ -16,7 +16,6 @@
package com.google.gerrit.server.patch;
import static com.google.common.base.Preconditions.checkArgument;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toSet;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
@ -156,14 +155,19 @@ public class PatchListLoader implements Callable<PatchList> {
if (a == null) {
// TODO(sop) Remove this case.
// This is a merge commit, compared to its ancestor.
// This is an octopus merge commit which should be compared against the
// auto-merge. However since we don't support computing the auto-merge
// for octopus merge commits, we fall back to diffing against the first
// parent, even though this wasn't what was requested.
//
PatchListEntry[] entries = new PatchListEntry[1];
ComparisonType comparisonType = ComparisonType.againstParent(1);
PatchListEntry[] entries = new PatchListEntry[2];
entries[0] = newCommitMessage(cmp, reader, null, b);
return new PatchList(a, b, true, entries);
entries[1] = newMergeList(cmp, reader, null, b, comparisonType);
return new PatchList(a, b, true, comparisonType, entries);
}
boolean againstParent = isAgainstParent(a, b);
ComparisonType comparisonType = getComparisonType(a, b);
RevCommit aCommit = a instanceof RevCommit ? (RevCommit) a : null;
RevTree aTree = rw.parseTree(a);
@ -190,7 +194,13 @@ public class PatchListLoader implements Callable<PatchList> {
int cnt = diffEntries.size();
List<PatchListEntry> entries = new ArrayList<>();
entries.add(newCommitMessage(cmp, reader,
againstParent ? null : aCommit, b));
comparisonType.isAgainstParentOrAutoMerge() ? null : aCommit, b));
boolean isMerge = b.getParentCount() > 1;
if (isMerge) {
entries.add(newMergeList(cmp, reader,
comparisonType.isAgainstParentOrAutoMerge() ? null : aCommit, b,
comparisonType));
}
for (int i = 0; i < cnt; i++) {
DiffEntry e = diffEntries.get(i);
if (paths == null || paths.contains(e.getNewPath())
@ -204,19 +214,23 @@ public class PatchListLoader implements Callable<PatchList> {
entries.add(newEntry(aTree, fh, newSize, newSize - oldSize));
}
}
return new PatchList(a, b, againstParent,
return new PatchList(a, b, isMerge, comparisonType,
entries.toArray(new PatchListEntry[entries.size()]));
}
}
private boolean isAgainstParent(RevObject a, RevCommit b) {
private ComparisonType getComparisonType(RevObject a, RevCommit b) {
for (int i = 0; i < b.getParentCount(); i++) {
if (b.getParent(i).equals(a)) {
return true;
return ComparisonType.againstParent(i + 1);
}
}
return false;
if (key.getOldId() == null && b.getParentCount() > 0) {
return ComparisonType.againstAutoMerge();
}
return ComparisonType.againstOtherPatchSet();
}
private static long getFileSize(ObjectReader reader,
@ -278,32 +292,30 @@ public class PatchListLoader implements Callable<PatchList> {
return diffFormatter.toFileHeader(diffEntry);
}
private PatchListEntry newCommitMessage(final RawTextComparator cmp,
final ObjectReader reader,
final RevCommit aCommit, final RevCommit bCommit) throws IOException {
StringBuilder hdr = new StringBuilder();
hdr.append("diff --git");
if (aCommit != null) {
hdr.append(" a/").append(Patch.COMMIT_MSG);
} else {
hdr.append(" ").append(FileHeader.DEV_NULL);
}
hdr.append(" b/").append(Patch.COMMIT_MSG);
hdr.append("\n");
if (aCommit != null) {
hdr.append("--- a/").append(Patch.COMMIT_MSG).append("\n");
} else {
hdr.append("--- ").append(FileHeader.DEV_NULL).append("\n");
}
hdr.append("+++ b/").append(Patch.COMMIT_MSG).append("\n");
Text aText =
aCommit != null ? Text.forCommit(reader, aCommit) : Text.EMPTY;
private PatchListEntry newCommitMessage(RawTextComparator cmp,
ObjectReader reader, RevCommit aCommit, RevCommit bCommit)
throws IOException {
Text aText = aCommit != null
? Text.forCommit(reader, aCommit)
: Text.EMPTY;
Text bText = Text.forCommit(reader, bCommit);
return createPatchListEntry(cmp, aCommit, aText, bText, Patch.COMMIT_MSG);
}
byte[] rawHdr = hdr.toString().getBytes(UTF_8);
private PatchListEntry newMergeList(RawTextComparator cmp,
ObjectReader reader, RevCommit aCommit, RevCommit bCommit,
ComparisonType comparisonType) throws IOException {
Text aText = aCommit != null
? Text.forMergeList(comparisonType, reader, aCommit)
: Text.EMPTY;
Text bText =
Text.forMergeList(comparisonType, reader, bCommit);
return createPatchListEntry(cmp, aCommit, aText, bText, Patch.MERGE_LIST);
}
private static PatchListEntry createPatchListEntry(RawTextComparator cmp,
RevCommit aCommit, Text aText, Text bText, String fileName) {
byte[] rawHdr = getRawHeader(aCommit != null, fileName);
byte[] aContent = aText.getContent();
byte[] bContent = bText.getContent();
long size = bContent.length;
@ -315,6 +327,26 @@ public class PatchListLoader implements Callable<PatchList> {
return new PatchListEntry(fh, edits, size, sizeDelta);
}
private static byte[] getRawHeader(boolean hasA, String fileName) {
StringBuilder hdr = new StringBuilder();
hdr.append("diff --git");
if (hasA) {
hdr.append(" a/").append(fileName);
} else {
hdr.append(" ").append(FileHeader.DEV_NULL);
}
hdr.append(" b/").append(fileName);
hdr.append("\n");
if (hasA) {
hdr.append("--- a/").append(fileName).append("\n");
} else {
hdr.append("--- ").append(FileHeader.DEV_NULL).append("\n");
}
hdr.append("+++ b/").append(fileName).append("\n");
return hdr.toString().getBytes(UTF_8);
}
private PatchListEntry newEntry(RevTree aTree, FileHeader fileHeader,
long size, long sizeDelta) {
if (aTree == null // want combined diff

33
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
View File

@ -66,7 +66,7 @@ class PatchScriptBuilder {
private ObjectReader reader;
private Change change;
private DiffPreferencesInfo diffPrefs;
private boolean againstParent;
private ComparisonType comparisonType;
private ObjectId aId;
private ObjectId bId;
@ -79,7 +79,8 @@ class PatchScriptBuilder {
private int context;
@Inject
PatchScriptBuilder(final FileTypeRegistry ftr, final PatchListCache plc) {
PatchScriptBuilder(FileTypeRegistry ftr,
PatchListCache plc) {
a = new Side();
b = new Side();
registry = ftr;
@ -106,8 +107,8 @@ class PatchScriptBuilder {
}
}
void setTrees(final boolean ap, final ObjectId a, final ObjectId b) {
againstParent = ap;
void setTrees(final ComparisonType ct, final ObjectId a, final ObjectId b) {
comparisonType = ct;
aId = a;
bId = b;
}
@ -435,7 +436,8 @@ class PatchScriptBuilder {
try {
final boolean reuse;
if (Patch.COMMIT_MSG.equals(path)) {
if (againstParent && (aId == within || within.equals(aId))) {
if (comparisonType.isAgainstParentOrAutoMerge()
&& (aId == within || within.equals(aId))) {
id = ObjectId.zeroId();
src = Text.EMPTY;
srcContent = Text.NO_BYTES;
@ -453,7 +455,26 @@ class PatchScriptBuilder {
}
}
reuse = false;
} else if (Patch.MERGE_LIST.equals(path)) {
if (comparisonType.isAgainstParentOrAutoMerge()
&& (aId == within || within.equals(aId))) {
id = ObjectId.zeroId();
src = Text.EMPTY;
srcContent = Text.NO_BYTES;
mode = FileMode.MISSING;
displayMethod = DisplayMethod.NONE;
} else {
id = within;
src = Text.forMergeList(comparisonType, reader, within);
srcContent = src.getContent();
if (src == Text.EMPTY) {
mode = FileMode.MISSING;
displayMethod = DisplayMethod.NONE;
} else {
mode = FileMode.REGULAR_FILE;
}
}
reuse = false;
} else {
final TreeWalk tw = find(within);

2
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchScriptFactory.java
View File

@ -260,7 +260,7 @@ public class PatchScriptFactory implements Callable<PatchScript> {
b.setRepository(git, project);
b.setChange(change);
b.setDiffPrefs(diffPrefs);
b.setTrees(list.isAgainstParent(), list.getOldId(), list.getNewId());
b.setTrees(list.getComparisonType(), list.getOldId(), list.getNewId());
return b;
}

30
gerrit-server/src/main/java/com/google/gerrit/server/patch/Text.java
View File

@ -87,6 +87,36 @@ public class Text extends RawText {
}
}
public static Text forMergeList(ComparisonType comparisonType,
ObjectReader reader, AnyObjectId commitId) throws IOException {
try (RevWalk rw = new RevWalk(reader)) {
RevCommit c = rw.parseCommit(commitId);
StringBuilder b = new StringBuilder();
switch (c.getParentCount()) {
case 0:
break;
case 1: {
break;
}
default:
int uniterestingParent = comparisonType.isAgainstParent()
? comparisonType.getParentNum()
: 1;
b.append("Merge List:\n\n");
for (RevCommit commit : MergeListBuilder.build(rw, c,
uniterestingParent)) {
b.append("* ");
b.append(reader.abbreviate(commit, 8).name());
b.append(" ");
b.append(commit.getShortMessage());
b.append("\n");
}
}
return new Text(b.toString().getBytes(UTF_8));
}
}
private static void appendPersonIdent(StringBuilder b, String field,
PersonIdent person) {
if (person != null) {

2
gerrit-server/src/main/java/com/google/gerrit/server/project/FilesInCommitCollection.java
View File

@ -47,7 +47,7 @@ public class FilesInCommitCollection implements
@Override
public FileResource parse(CommitResource parent, IdString id)
throws ResourceNotFoundException, IOException {
if (Patch.COMMIT_MSG.equals(id.get())) {
if (Patch.isMagic(id.get())) {
return new FileResource(parent.getProject(), parent.getCommit(),
id.get());
}

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
View File

@ -599,7 +599,7 @@ public class ChangeData {
r = new ArrayList<>(p.get().getPatches().size());
for (PatchListEntry e : p.get().getPatches()) {
if (Patch.COMMIT_MSG.equals(e.getNewName())) {
if (Patch.isMagic(e.getNewName())) {
continue;
}
switch (e.getChangeType()) {

20
gerrit-server/src/main/java/gerrit/PRED_commit_stats_3.java
View File

@ -14,8 +14,10 @@
package gerrit;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.rules.StoredValues;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
import com.googlecode.prolog_cafe.exceptions.PrologException;
import com.googlecode.prolog_cafe.lang.IntegerTerm;
@ -24,6 +26,8 @@ import com.googlecode.prolog_cafe.lang.Predicate;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlecode.prolog_cafe.lang.Term;
import java.util.List;
/**
* Exports basic commit statistics.
*
@ -48,7 +52,11 @@ public class PRED_commit_stats_3 extends Predicate.P3 {
Term a3 = arg3.dereference();
PatchList pl = StoredValues.PATCH_LIST.get(engine);
if (!a1.unify(new IntegerTerm(pl.getPatches().size() - 1),engine.trail)) { //Account for /COMMIT_MSG.
// Account for magic files
if (!a1.unify(
new IntegerTerm(
pl.getPatches().size() - countMagicFiles(pl.getPatches())),
engine.trail)) {
return engine.fail();
}
if (!a2.unify(new IntegerTerm(pl.getInsertions()),engine.trail)) {
@ -59,4 +67,14 @@ public class PRED_commit_stats_3 extends Predicate.P3 {
}
return cont;
}
private int countMagicFiles(List<PatchListEntry> entries) {
int count = 0;
for (PatchListEntry e : entries) {
if (Patch.isMagic(e.getNewName())) {
count++;
}
}
return count;
}
}