opendev/gerrit
Code Issues Proposed changes

SubmittedTogether: Use WalkSorter in the open case as well

Browse Source 
MergeSuperSet doesn't return the most useful ordering, so use
something more stable, and be consistent with the merged case.

Change-Id: Ia9dd6efe307dcf2c5535f49f7e6d38fc9bd06545
This commit is contained in:
Dave Borowitz
2015-10-12 13:12:56 -04:00
parent 8fb3de1b45
commit b729d3e82b
2 changed files with 17 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SubmitByFastForwardIT.java
View File

@@ -61,8 +61,8 @@ public class SubmitByFastForwardIT extends AbstractSubmit {
assertSubmitter(change2.getChangeId(), 1);
assertPersonEquals(admin.getIdent(), head.getAuthorIdent());
assertPersonEquals(admin.getIdent(), head.getCommitterIdent());
assertSubmittedTogether(id1, id1, id2);
assertSubmittedTogether(id2, id1, id2);
assertSubmittedTogether(id1, id2, id1);
assertSubmittedTogether(id2, id2, id1);
}
@Test

24
gerrit-server/src/main/java/com/google/gerrit/server/change/SubmittedTogether.java
View File

@@ -80,9 +80,15 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
} else {
cds = getForAbandonedChange();
}
if (cds.size() <= 1) {
cds = Collections.emptyList();
} else {
// Skip sorting for singleton lists, to avoid WalkSorter opening the
// repo just to fill out the commit field in PatchSetData.
cds = sort(cds);
}
return json.create(EnumSet.of(
ListChangesOption.CURRENT_REVISION,
ListChangesOption.CURRENT_COMMIT))
@@ -99,24 +105,20 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
return cs.changes().asList();
}
private List<ChangeData> getForMergedChange(Change c)
throws OrmException, IOException {
String subId = c.getSubmissionId();
List<ChangeData> cds = queryProvider.get().bySubmissionId(subId);
if (cds.size() <= 1) {
// Bypass WalkSorter to avoid opening the repo just to populate the commit
// field in PatchSetData that we would throw out in apply() above anyway.
private List<ChangeData> getForMergedChange(Change c) throws OrmException {
return queryProvider.get().bySubmissionId(c.getSubmissionId());
}
private List<ChangeData> getForAbandonedChange() {
return Collections.emptyList();
}
private List<ChangeData> sort(List<ChangeData> cds)
throws OrmException, IOException {
List<ChangeData> sorted = new ArrayList<>(cds.size());
for (PatchSetData psd : sorter.get().sort(cds)) {
sorted.add(psd.data());
}
return sorted;
}
private List<ChangeData> getForAbandonedChange() {
return Collections.emptyList();
}
}