Get commentlinks per-project in UI code

Remove commentlinks from GerritConfig and instead provide them in JSON
form from GET /projects/X/config. On the client side, create new
CommentLinkProcessor instances when loading a change or patch.
Fortunately, even though this requires another RPC to get the project
config, in all existing screen instances, there is already an RPC we
can parallelize it with.

In the long term we may want to enable server-side HTML rendering, but
that requires enabling this option on a variety of RPC endpoints, not
all of which are converted to the new REST API. In addition, there is
the problem of defining a stable HTML fragment style. For example, the
commit message template is currently implemented using a
not-quite-trivial template in GWT's UiBinder, so some of that
formatting and styling would need to be hoisted out into the server
side; doable, but we're not there yet.

Change-Id: Iaecbeff939c8fcbc1c6f500e0b04ce03f35e1fd3
This commit is contained in:
Dave Borowitz
2013-04-08 10:42:23 -07:00
parent 39f5688fdb
commit 2002789f41
22 changed files with 266 additions and 167 deletions

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.client.account.AccountInfo;
import com.google.gerrit.client.changes.PatchTable;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.CommentPanel;
import com.google.gerrit.client.ui.NavigationTable;
import com.google.gerrit.client.ui.NeedsSignInKeyCommand;
@@ -86,6 +87,7 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
private HandlerRegistration regComment;
private final KeyCommandSet keysOpenByEnter;
private HandlerRegistration regOpenByEnter;
private CommentLinkProcessor commentLinkProcessor;
boolean isDisplayBinary;
protected AbstractPatchContentTable() {
@@ -241,6 +243,10 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
render(s, d);
}
void setCommentLinkProcessor(CommentLinkProcessor commentLinkProcessor) {
this.commentLinkProcessor = commentLinkProcessor;
}
protected boolean hasDifferences(PatchScript script) {
return hasEdits(script) || hasMeta(script);
}
@@ -553,7 +559,8 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
return null;
}
final CommentEditorPanel ed = new CommentEditorPanel(newComment);
final CommentEditorPanel ed =
new CommentEditorPanel(newComment, commentLinkProcessor);
ed.addFocusHandler(this);
ed.addBlurHandler(this);
boolean isCommentRow = false;
@@ -690,7 +697,8 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
protected void bindComment(final int row, final int col,
final PatchLineComment line, final boolean isLast, boolean expandComment) {
if (line.getStatus() == PatchLineComment.Status.DRAFT) {
final CommentEditorPanel plc = new CommentEditorPanel(line);
final CommentEditorPanel plc =
new CommentEditorPanel(line, commentLinkProcessor);
plc.addFocusHandler(this);
plc.addBlurHandler(this);
table.setWidget(row, col, plc);
@@ -864,7 +872,7 @@ public abstract class AbstractPatchContentTable extends NavigationTable<Object>
final Button replyDone;
PublishedCommentPanel(final AccountInfo author, final PatchLineComment c) {
super(author, c.getWrittenOn(), c.getMessage());
super(author, c.getWrittenOn(), c.getMessage(), commentLinkProcessor);
this.comment = c;
reply = new Button(PatchUtil.C.buttonReply());

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.client.patches;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.CommentPanel;
import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gwt.event.dom.client.ClickEvent;
@@ -25,10 +26,10 @@ import com.google.gwt.event.dom.client.DoubleClickHandler;
import com.google.gwt.event.dom.client.KeyDownEvent;
import com.google.gwt.event.dom.client.KeyDownHandler;
import com.google.gwt.user.client.Timer;
import com.google.gwtjsonrpc.common.AsyncCallback;
import com.google.gwt.user.client.ui.Button;
import com.google.gwt.user.client.ui.Widget;
import com.google.gwtexpui.globalkey.client.NpTextArea;
import com.google.gwtjsonrpc.common.AsyncCallback;
import com.google.gwtjsonrpc.common.VoidResult;
import java.sql.Timestamp;
@@ -58,7 +59,9 @@ public class CommentEditorPanel extends CommentPanel implements ClickHandler,
private final Button discard;
private final Timer expandTimer;
public CommentEditorPanel(final PatchLineComment plc) {
public CommentEditorPanel(final PatchLineComment plc,
final CommentLinkProcessor commentLinkProcessor) {
super(commentLinkProcessor);
comment = plc;
addStyleName(Gerrit.RESOURCES.css().commentEditorPanel());

View File

@@ -21,8 +21,12 @@ import com.google.gerrit.client.RpcStatus;
import com.google.gerrit.client.changes.CommitMessageBlock;
import com.google.gerrit.client.changes.PatchTable;
import com.google.gerrit.client.changes.Util;
import com.google.gerrit.client.projects.ConfigInfo;
import com.google.gerrit.client.projects.ProjectApi;
import com.google.gerrit.client.rpc.CallbackGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.ListenableAccountDiffPreference;
import com.google.gerrit.client.ui.Screen;
import com.google.gerrit.common.data.PatchScript;
@@ -38,6 +42,7 @@ import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwt.user.client.ui.FlowPanel;
import com.google.gwtexpui.globalkey.client.GlobalKey;
import com.google.gwtexpui.globalkey.client.KeyCommand;
@@ -97,6 +102,7 @@ public abstract class PatchScreen extends Screen implements
protected PatchSet.Id idSideB;
protected PatchScriptSettingsPanel settingsPanel;
protected TopView topView;
protected CommentLinkProcessor commentLinkProcessor;
private ReviewedPanels reviewedPanels;
private HistoryTable historyTable;
@@ -365,8 +371,9 @@ public abstract class PatchScreen extends Screen implements
if (isFirst && fileList != null) {
fileList.movePointerTo(patchKey);
}
PatchUtil.DETAIL_SVC.patchScript(patchKey, idSideA, idSideB, //
settingsPanel.getValue(), new ScreenLoadCallback<PatchScript>(this) {
com.google.gwtjsonrpc.common.AsyncCallback<PatchScript> pscb =
new ScreenLoadCallback<PatchScript>(this) {
@Override
protected void preDisplay(final PatchScript result) {
if (rpcSequence == rpcseq) {
@@ -381,7 +388,29 @@ public abstract class PatchScreen extends Screen implements
super.onFailure(caught);
}
}
});
};
if (commentLinkProcessor == null) {
// Fetch config in parallel if we haven't previously.
CallbackGroup cb = new CallbackGroup();
ProjectApi.config(patchSetDetail.getProject())
.get(cb.add(new AsyncCallback<ConfigInfo>() {
@Override
public void onSuccess(ConfigInfo result) {
commentLinkProcessor =
new CommentLinkProcessor(result.commentlinks());
contentTable.setCommentLinkProcessor(commentLinkProcessor);
}
@Override
public void onFailure(Throwable caught) {
// Handled by ScreenLoadCallback.onFailure.
}
}));
pscb = cb.addGwtjsonrpc(pscb);
}
PatchUtil.DETAIL_SVC.patchScript(patchKey, idSideA, idSideB, //
settingsPanel.getValue(), pscb);
}
private void onResult(final PatchScript script, final boolean isFirst) {
@@ -397,7 +426,8 @@ public abstract class PatchScreen extends Screen implements
if (idSideB.equals(patchSetDetail.getPatchSet().getId())) {
commitMessageBlock.setVisible(true);
commitMessageBlock.display(patchSetDetail.getInfo().getMessage());
commitMessageBlock.display(patchSetDetail.getInfo().getMessage(),
commentLinkProcessor);
} else {
commitMessageBlock.setVisible(false);
Util.DETAIL_SVC.patchSetDetail(idSideB,
@@ -405,7 +435,8 @@ public abstract class PatchScreen extends Screen implements
@Override
public void onSuccess(PatchSetDetail result) {
commitMessageBlock.setVisible(true);
commitMessageBlock.display(result.getInfo().getMessage());
commitMessageBlock.display(result.getInfo().getMessage(),
commentLinkProcessor);
}
});
}
@@ -432,6 +463,7 @@ public abstract class PatchScreen extends Screen implements
contentTable.removeFromParent();
contentTable = new UnifiedDiffTable();
contentTable.fileList = fileList;
contentTable.setCommentLinkProcessor(commentLinkProcessor);
contentPanel.add(contentTable);
setToken(Dispatcher.toPatchUnified(idSideA, patchKey));
}

View File

@@ -20,6 +20,7 @@ import static com.google.gerrit.client.patches.PatchLine.Type.INSERT;
import static com.google.gerrit.client.patches.PatchLine.Type.REPLACE;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.common.data.CommentDetail;
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.common.data.PatchScript.FileMode;
@@ -39,6 +40,7 @@ import com.google.gwt.user.client.ui.InlineLabel;
import com.google.gwt.user.client.ui.UIObject;
import com.google.gwtexpui.safehtml.client.SafeHtml;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
import org.eclipse.jgit.diff.Edit;
import java.util.ArrayList;

View File

@@ -19,6 +19,7 @@ import static com.google.gerrit.client.patches.PatchLine.Type.DELETE;
import static com.google.gerrit.client.patches.PatchLine.Type.INSERT;
import com.google.gerrit.client.Gerrit;
import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.common.data.CommentDetail;
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.common.data.PatchScript.DisplayMethod;
@@ -32,8 +33,8 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwt.core.client.GWT;
import com.google.gwt.user.client.DOM;
import com.google.gwt.user.client.Element;
import com.google.gwt.user.client.ui.UIObject;
import com.google.gwt.user.client.ui.HTMLTable.CellFormatter;
import com.google.gwt.user.client.ui.UIObject;
import com.google.gwtexpui.safehtml.client.SafeHtml;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
import com.google.gwtorm.client.KeyUtil;