Fix browser NPE when ChangeCache is incomplete
The ChangeCache is buggy and does not always populate its
contents. Unfortunately it is also designed to be unable to
load missing contents on demand, causing consumers to NPE.
ConfigInfoCache is a better design for this sort of lazy
loading and reuse of data.
ChangeCache can be missing information if:
1) opens side-by-side view in a new tab;
2) user presses 'r' to open publish comment screen
Instead of using the unreliable ChangeCache, stub out a
ChangeDetail to feed to the info block on the publish
comments screen.
Bug: issue 2039
Change-Id: Id542528d93af1cab49b001ca5e90addfc5d05b78
(cherry picked from commit f646aa77fb67b73c8724d7a632cafd441365e901)
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java
index 5cd6cdb..8395907 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java
@@ -15,8 +15,8 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.common.data.AccountInfoCache;
+import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.SubmitTypeRecord;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.HorizontalPanel;
@@ -36,11 +36,11 @@
initWidget(hp);
}
- public void display(Change chg, Boolean starred, Boolean canEditCommitMessage,
+ public void display(ChangeDetail chg, Boolean starred, Boolean canEditCommitMessage,
PatchSetInfo info,
final AccountInfoCache acc, SubmitTypeRecord submitTypeRecord) {
infoBlock.display(chg, acc, submitTypeRecord);
- messageBlock.display(chg.currentPatchSetId(), starred,
+ messageBlock.display(chg.getChange().currentPatchSetId(), starred,
canEditCommitMessage, info.getMessage());
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
index e4dbc32..afc17f7 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
@@ -95,8 +95,9 @@
table.getCellFormatter().addStyleName(row, 0, Gerrit.RESOURCES.css().header());
}
- public void display(final Change chg, final AccountInfoCache acc,
- SubmitTypeRecord submitTypeRecord) {
+ public void display(final ChangeDetail changeDetail,
+ final AccountInfoCache acc, SubmitTypeRecord submitTypeRecord) {
+ final Change chg = changeDetail.getChange();
final Branch.NameKey dst = chg.getDest();
CopyableLabel changeIdLabel =
@@ -114,7 +115,7 @@
table.setWidget(R_BRANCH, 1, new BranchLink(dst.getShortName(), chg
.getProject(), chg.getStatus(), dst.get(), null));
- table.setWidget(R_TOPIC, 1, topic(chg));
+ table.setWidget(R_TOPIC, 1, topic(changeDetail));
table.setText(R_UPLOADED, 1, mediumFormat(chg.getCreatedOn()));
table.setText(R_UPDATED, 1, mediumFormat(chg.getLastUpdatedOn()));
table.setText(R_STATUS, 1, Util.toLongString(chg.getStatus()));
@@ -146,7 +147,8 @@
}
}
- public Widget topic(final Change chg) {
+ public Widget topic(final ChangeDetail changeDetail) {
+ final Change chg = changeDetail.getChange();
final Branch.NameKey dst = chg.getDest();
FlowPanel fp = new FlowPanel();
@@ -154,9 +156,6 @@
fp.add(new BranchLink(chg.getTopic(), chg.getProject(), chg.getStatus(),
dst.get(), chg.getTopic()));
- ChangeDetailCache detailCache = ChangeCache.get(chg.getId()).getChangeDetailCache();
- ChangeDetail changeDetail = detailCache.get();
-
if (changeDetail.canEditTopicName()) {
final Image edit = new Image(Gerrit.RESOURCES.edit());
edit.addStyleName(Gerrit.RESOURCES.css().link());
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
index 7983512..91c8856 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
@@ -301,7 +301,7 @@
dependencies.setAccountInfoCache(detail.getAccounts());
- descriptionBlock.display(detail.getChange(),
+ descriptionBlock.display(detail,
detail.isStarred(),
detail.canEditCommitMessage(),
detail.getCurrentPatchSetDetail().getInfo(),
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
index 97949e3..7f61977 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -30,6 +30,7 @@
import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.PatchSetPublishDetail;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Patch;
@@ -298,9 +299,12 @@
}
private void display(final PatchSetPublishDetail r) {
+ ChangeDetail changeDetail = new ChangeDetail();
+ changeDetail.setChange(r.getChange());
+
setPageTitle(Util.M.publishComments(r.getChange().getKey().abbreviate(),
patchSetId.get()));
- descBlock.display(r.getChange(), null, false, r.getPatchSetInfo(), r.getAccounts(),
+ descBlock.display(changeDetail, null, false, r.getPatchSetInfo(), r.getAccounts(),
r.getSubmitTypeRecord());
if (r.getChange().getStatus().isOpen()) {
