Don't load ChangeNotes when lazyload is false
When we serve a change query and format the response in ChangeJson, lazyload is disabled. This means that ChangeData#notes() throws an OrmException. When specifying DETAILED_LABELS we were calling #notes() irrespective of lazyload resulting in an OrmException for the change and omitting it from the results. This was only occuring for merged changes. This commit fixes the bug by constructing ChangeNotes from the index data instead if lazyload is false and adds a test for all change options. Change-Id: Id08ef0e6fa71a875acdfdb7d4a669af2ba38a0cc
This commit is contained in:
@@ -26,6 +26,7 @@ import com.google.gerrit.acceptance.SshSession;
|
||||
import com.google.gerrit.acceptance.UseSsh;
|
||||
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
|
||||
import com.google.gerrit.extensions.api.changes.ReviewInput;
|
||||
import com.google.gerrit.extensions.client.ListChangesOption;
|
||||
import com.google.gerrit.extensions.client.Side;
|
||||
import com.google.gerrit.server.data.ChangeAttribute;
|
||||
import com.google.gson.Gson;
|
||||
@@ -310,6 +311,19 @@ public class QueryIT extends AbstractDaemonTest {
|
||||
userSession.close();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void allChangeOptionsAreServedWithoutExceptions() throws Exception {
|
||||
PushOneCommit.Result r = createChange();
|
||||
// Merge the change so that the result has more data and potentially went through more
|
||||
// computation while formatting the output, such as labels, reviewers etc.
|
||||
merge(r);
|
||||
for (ListChangesOption option : ListChangesOption.values()) {
|
||||
assertThat(gApi.changes().query(r.getChangeId()).withOption(option).get())
|
||||
.named("Option: " + option)
|
||||
.hasSize(1);
|
||||
}
|
||||
}
|
||||
|
||||
private List<ChangeAttribute> executeSuccessfulQuery(String params, SshSession session)
|
||||
throws Exception {
|
||||
String rawResponse = session.exec("gerrit query --format=JSON " + params);
|
||||
|
@@ -1074,7 +1074,7 @@ public class ChangeJson {
|
||||
for (PatchSetApproval psa :
|
||||
approvalsUtil.byPatchSetUser(
|
||||
db.get(),
|
||||
cd.notes(),
|
||||
lazyLoad ? cd.notes() : notesFactory.createFromIndexedChange(cd.change()),
|
||||
user,
|
||||
cd.change().currentPatchSetId(),
|
||||
user.getAccountId(),
|
||||
|
Reference in New Issue
Block a user