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.

Change-Id: Id542528d93af1cab49b001ca5e90addfc5d05b78
This commit is contained in:
Shawn Pearce
2013-08-13 09:59:24 -07:00
parent da5153f551
commit f646aa77fb
4 changed files with 16 additions and 13 deletions

View File

@@ -16,8 +16,8 @@ package com.google.gerrit.client.changes;
import com.google.gerrit.client.ui.CommentLinkProcessor;
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;
@@ -37,12 +37,12 @@ public class ChangeDescriptionBlock extends Composite {
initWidget(hp);
}
public void display(Change chg, Boolean starred, Boolean canEditCommitMessage,
public void display(ChangeDetail changeDetail, Boolean starred, Boolean canEditCommitMessage,
PatchSetInfo info, AccountInfoCache acc,
SubmitTypeRecord submitTypeRecord,
CommentLinkProcessor commentLinkProcessor) {
infoBlock.display(chg, acc, submitTypeRecord);
messageBlock.display(chg.currentPatchSetId(), starred,
infoBlock.display(changeDetail, acc, submitTypeRecord);
messageBlock.display(changeDetail.getChange().currentPatchSetId(), starred,
canEditCommitMessage, info.getMessage(), commentLinkProcessor);
}
}

View File

@@ -95,8 +95,9 @@ public class ChangeInfoBlock extends Composite {
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 @@ public class ChangeInfoBlock extends Composite {
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 class ChangeInfoBlock extends Composite {
}
}
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 @@ public class ChangeInfoBlock extends Composite {
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());

View File

@@ -322,7 +322,7 @@ public class ChangeScreen extends Screen
dependencies.setAccountInfoCache(detail.getAccounts());
descriptionBlock.display(detail.getChange(),
descriptionBlock.display(detail,
detail.isStarred(),
detail.canEditCommitMessage(),
detail.getCurrentPatchSetDetail().getInfo(),

View File

@@ -31,6 +31,7 @@ import com.google.gerrit.client.ui.CommentLinkProcessor;
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;
@@ -317,9 +318,12 @@ public class PublishCommentScreen extends AccountScreen implements
}
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(), commentLinkProcessor);
if (r.getChange().getStatus().isOpen()) {