opendev/gerrit
Code Issues Proposed changes

ChangeJson: Ensure for merged changes that labels for approvals are included

Browse Source 
There may be approvals on labels that are not contained in the submit
records. E.g. this is the case if there is an approval on a label that
is ignored by a Prolog submit rule. In this case ChangeJson was
failing with:

  com.google.gwtorm.server.OrmException: java.lang.NullPointerException
    at com.google.gerrit.server.change.ChangeJson.format(ChangeJson.java:303)
    at com.google.gerrit.server.change.ChangeJson.format(ChangeJson.java:285)
    at com.google.gerrit.server.change.ChangeJson.format(ChangeJson.java:263)
    at com.google.gerrit.server.change.GetChange.apply(GetChange.java:50)
    at com.google.gerrit.server.change.GetDetail.apply(GetDetail.java:51)
    at com.google.gerrit.server.change.GetDetail.apply(GetDetail.java:26)
    at com.google.gerrit.httpd.restapi.RestApiServlet.service(RestApiServlet.java:367)
    ...
  Caused by: java.lang.NullPointerException
    at com.google.gerrit.server.change.ChangeJson.setLabelScores(ChangeJson.java:670)
    at com.google.gerrit.server.change.ChangeJson.labelsForClosedChange(ChangeJson.java:845)
    at com.google.gerrit.server.change.ChangeJson.labelsFor(ChangeJson.java:598)
    at com.google.gerrit.server.change.ChangeJson.toChangeInfo(ChangeJson.java:499)
    at com.google.gerrit.server.change.ChangeJson.format(ChangeJson.java:294)
    ...

Change-Id: I46073f4077d83963d4a45a50189957cd8c9a1bd0
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-10-28 16:25:53 +02:00
parent 990b304742
commit 1898caeeab
2 changed files with 51 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -2229,15 +2229,42 @@ public class ChangeIT extends AbstractDaemonTest {
assertThat(change.permittedLabels.keySet())
.containsExactly("Code-Review", "Verified");
// add an approval on the new label
// ignore the new label by Prolog submit rule and assert that the label is
// no longer returned
GitUtil.fetch(testRepo, RefNames.REFS_CONFIG + ":config");
testRepo.reset("config");
PushOneCommit push2 = pushFactory.create(db, admin.getIdent(), testRepo,
"Ignore Verified",
"rules.pl",
"submit_rule(submit(CR)) :-\n"
+ " gerrit:max_with_block(-2, 2, 'Code-Review', CR).");
push2.to(RefNames.REFS_CONFIG);
change = gApi.changes()
.id(r.getChangeId())
.get();
assertThat(change.labels.keySet()).containsExactly("Code-Review");
assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
// add an approval on the new label and assert that the label is now
// returned although it is ignored by the Prolog submit rule and hence not
// included in the submit records
gApi.changes()
.id(r.getChangeId())
.revision(r.getCommit().name())
.review(new ReviewInput().label(
verified.getName(), verified.getMax().getValue()));
change = gApi.changes()
.id(r.getChangeId())
.get();
assertThat(change.labels.keySet())
.containsExactly("Code-Review", "Verified");
assertThat(change.permittedLabels.keySet()).containsExactly("Code-Review");
// remove label and assert that it's no longer returned for existing
// changes, even if there is an approval for it
cfg = projectCache.checkedGet(project).getConfig();
cfg.getLabelSections().remove(verified.getName());
Util.remove(cfg, Permission.forLabel(verified.getName()), registeredUsers,
heads);