opendev/gerrit
Code Issues Proposed changes

Remove CURRENT_COMMIT from default SubmittedTogether opts

Browse Source 
SubmittedTogether potentially returns hundreds of changes.
CURRENT_COMMIT requires opening the repository that contains the change
to load more information about the commit. This can make the endpoint
very slow (>1 minute for 500 changes).

SubmittedTogether already provides a way of specifying additional
options for ChangeJson. Therefore removing an option from the defaults
seems safe as users that still want to receive commit information can
just specify o=CURRENT_COMMIT on the request.

This commit specifies this option for the GWT UI so that we don't run
into (potential) issues with WebLinks. Since the GWT UI will be deleted
soon, keeping it on the slow version of the endpoint seems OK.

This commit also implements a lazyload boolean in SubmittedTogether.
This forces the default call to not do any lazyloading which helps us to
catch issues earlier. The boolean is updated if the user specified an
option that requires lazy loading.

Change-Id: I763d15447fa79846fe0656a869b15edcf8502cb5
This commit is contained in:
Patrick Hiesel 2018-04-05 15:10:13 +02:00
parent f90142c4e8
commit 6fbbdaa4f6
3 changed files with 7 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
Documentation/rest-api-changes.txt
View File

@ -1581,8 +1581,7 @@ changes to be merged.
The listed changes use the same format as in
link:#list-changes[Query Changes] with the
link:#labels[`LABELS`], link:#detailed-labels[`DETAILED_LABELS`],
link:#current-revision[`CURRENT_REVISION`],
link:#current-commit[`CURRENT_COMMIT`], and
link:#current-revision[`CURRENT_REVISION`],and
link:#submittable[`SUBMITTABLE`] options set.
Standard link:#query-options[formatting options] can be specified
@ -1692,28 +1691,6 @@ option `NON_VISIBLE_CHANGES`.
},
"ref": "refs/changes/79/1779/1",
"fetch": {},
"commit": {
"parents": [
{
"commit": "2d3176497a2747faed075f163707e57d9f961a1c",
"subject": "Merge changes from topic \u0027submodule-subscription-tests-and-fixes-3\u0027"
}
],
"author": {
"name": "Stefan Beller",
"email": "sbeller@google.com",
"date": "2015-04-29 21:36:52.000000000",
"tz": -420
},
"committer": {
"name": "Stefan Beller",
"email": "sbeller@google.com",
"date": "2015-05-01 00:11:16.000000000",
"tz": -420
},
"subject": "ChangeMergeQueue: Rewrite such that it works on set of changes",
"message": "ChangeMergeQueue: Rewrite such that it works on set of changes\n\nChangeMergeQueue used to work on branches rather than sets of changes.\nThis change is a first step to merge sets of changes (e.g. grouped by a\ntopic and `changes.submitWholeTopic` enabled) in an atomic fashion.\nThis change doesn\u0027t aim to implement these changes, but only as a step\ntowards it.\n\nMergeOp keeps its functionality and behavior as is. A new class\nMergeOpMapper is introduced which will map the set of changes to\nthe set of branches. Additionally the MergeOpMapper is also\nresponsible for the threading done right now, which was part of\nthe ChangeMergeQueue before.\n\nChange-Id: I1ffe09a505e25f15ce1521bcfb222e51e62c2a14\n"
}
}
}
},
@ -1811,28 +1788,6 @@ option `NON_VISIBLE_CHANGES`.
},
"ref": "refs/changes/80/1780/1",
"fetch": {},
"commit": {
"parents": [
{
"commit": "9adb9f4c7b40eeee0646e235de818d09164d7379",
"subject": "ChangeMergeQueue: Rewrite such that it works on set of changes"
}
],
"author": {
"name": "Stefan Beller",
"email": "sbeller@google.com",
"date": "2015-04-25 00:11:59.000000000",
"tz": -420
},
"committer": {
"name": "Stefan Beller",
"email": "sbeller@google.com",
"date": "2015-05-01 00:11:16.000000000",
"tz": -420
},
"subject": "AbstractSubmoduleSubscription: Split up createSubscription",
"message": "AbstractSubmoduleSubscription: Split up createSubscription\n\nLater we want to have subscriptions to more submodules, so we need to\nfind a way to add more submodule entries into the file. By splitting up\nthe createSubscription() method, that is very easy by using the\naddSubmoduleSubscription method multiple times.\n\nChange-Id: I7fe807e63792b3d26776fd1422e5e790a5697e22\n"
}
}
}
}

1
gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChanges.java
View File

@ -226,6 +226,7 @@ public class RelatedChanges extends TabPanel {
if (info.currentRevision() != null && info.currentRevision().equals(revision)) {
ChangeApi.change(info.project(), info.legacyId().get())
.view("submitted_together")
.addParameter("o", "CURRENT_COMMIT")
.get(new TabChangeListCallback(Tab.SUBMITTED_TOGETHER, info.project(), revision));
}

10
java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
View File

@ -54,10 +54,7 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
EnumSet.noneOf(SubmittedTogetherOption.class);
private final EnumSet<ListChangesOption> jsonOpt =
EnumSet.of(
ListChangesOption.CURRENT_REVISION,
ListChangesOption.CURRENT_COMMIT,
ListChangesOption.SUBMITTABLE);
EnumSet.of(ListChangesOption.CURRENT_REVISION, ListChangesOption.SUBMITTABLE);
private final ChangeJson.Factory json;
private final Provider<ReviewDb> dbProvider;
@ -65,11 +62,14 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
private final Provider<MergeSuperSet> mergeSuperSet;
private final Provider<WalkSorter> sorter;
private boolean lazyLoad = false;
@Option(name = "-o", usage = "Output options")
void addOption(String option) {
for (ListChangesOption o : ListChangesOption.values()) {
if (o.name().equalsIgnoreCase(option)) {
jsonOpt.add(o);
lazyLoad |= ChangeJson.REQUIRE_LAZY_LOAD.contains(o);
return;
}
}
@ -152,7 +152,7 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
}
SubmittedTogetherInfo info = new SubmittedTogetherInfo();
info.changes = json.create(jsonOpt).formatChangeDatas(cds);
info.changes = json.create(jsonOpt).lazyLoad(lazyLoad).formatChangeDatas(cds);
info.nonVisibleChanges = hidden;
return info;
} catch (OrmException | IOException e) {