From c4a90512d540041be5c256ce6aba1dae44b7aecf Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 8 Jun 2016 21:29:53 -0700 Subject: [PATCH] Replace gitweb glue with WebLink extensions Keep the gitweb configuration section, but bind the URLs through the WebLink extension points instead of specific support in the web UI. This simplifies the server and UI implementations a bit by using only code paths that other plugins like the gitiles plugin use. Change-Id: I8508dcaf3a4405b3ce3a47f2f717472a46e5a8ca --- Documentation/config-gerrit.txt | 10 - Documentation/rest-api-config.txt | 50 ---- .../acceptance/rest/config/ServerInfoIT.java | 6 - .../google/gerrit/common/data/GitwebType.java | 15 -- .../google/gerrit/client/info/GitwebInfo.java | 168 -------------- .../gerrit/client/info/GitwebTypeInfo.java | 48 ---- .../google/gerrit/client/info/ServerInfo.java | 1 - .../client/admin/ProjectAccessEditor.java | 34 +-- .../client/admin/ProjectBranchesScreen.java | 9 - .../client/admin/ProjectListScreen.java | 18 +- .../gerrit/client/change/CommitBox.java | 42 +--- .../client/change/RelatedChangesTab.java | 13 +- .../com/google/gerrit/client/diff/Header.java | 37 +-- .../gerrit/client/editor/EditScreen.java | 2 +- .../server/config/GerritGlobalModule.java | 2 +- .../gerrit/server/config/GetServerInfo.java | 16 -- .../gerrit/server/config/GitwebConfig.java | 213 +++++++++++++----- 17 files changed, 180 insertions(+), 504 deletions(-) delete mode 100644 gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebInfo.java delete mode 100644 gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebTypeInfo.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 34cff057dc..0c856ef5b9 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -1989,16 +1989,6 @@ based viewer) + Valid values are "true" and "false," default is "true." -[[gitweb.linkDrafts]]gitweb.linkDrafts:: -+ -Whether or not Gerrit should provide links to gitweb on draft patch sets. -+ -By default, Gerrit will show links to gitweb on all patch sets. If gitweb -only allows publicly viewable references, set this to false to remove -the links to draft patch sets from the change review screen. -+ -Valid values are "true" and "false," default is "true". - [[groups]] === Section groups diff --git a/Documentation/rest-api-config.txt b/Documentation/rest-api-config.txt index 345f759e4a..0bab71a957 100644 --- a/Documentation/rest-api-config.txt +++ b/Documentation/rest-api-config.txt @@ -1305,54 +1305,6 @@ link:config-gerrit.html#gerrit.reportBugText[Display text for report bugs link]. |================================= -[[git-web-info]] -=== GitwebInfo -The `GitwebInfo` entity contains information about the -link:config-gerrit.html#gitweb[gitweb] configuration. - -[options="header",cols="1,6"] -|======================= -|Field Name |Description -|`url` | -The link:config-gerrit.html#gitweb.url[gitweb base URL]. -|`type` | -The link:config-gerrit.html#gitweb.type[gitweb type] as -link:#git-web-type-info[GitwebTypeInfo] entity. -|======================= - -[[git-web-type-info]] -=== GitwebTypeInfo -The `GitwebTypeInfo` entity contains information about the -link:config-gerrit.html#gitweb[gitweb] configuration. - -[options="header",cols="1,^1,5"] -|============================= -|Field Name ||Description -|`name` || -The link:config-gerrit.html#gitweb.linkname[gitweb link name]. -|`revision` |optional| -The link:config-gerrit.html#gitweb.revision[gitweb revision pattern]. -|`project` |optional| -The link:config-gerrit.html#gitweb.project[gitweb project pattern]. -|`branch` |optional| -The link:config-gerrit.html#gitweb.branch[gitweb branch pattern]. -|`root_tree` |optional| -The link:config-gerrit.html#gitweb.roottree[gitweb root tree pattern]. -|`file` |optional| -The link:config-gerrit.html#gitweb.file[gitweb file pattern]. -|`file_history` |optional| -The link:config-gerrit.html#gitweb.filehistory[gitweb file history -pattern]. -|`path_separator`|| -The link:config-gerrit.html#gitweb.pathSeparator[gitweb path separator]. -|`link_drafts` |optional| -link:config-gerrit.html#gitweb.linkDrafts[Whether Gerrit should provide -links to gitweb on draft patch set.] -|`url_encode` |optional| -link:config-gerrit.html#gitweb.urlEncode[Whether Gerrit should encode -the generated viewer URL.] -|============================= - [[hit-ration-info]] === HitRatioInfo The `HitRatioInfo` entity contains information about the hit ratio of a @@ -1464,8 +1416,6 @@ information about Gerrit Information about the configuration from the link:config-gerrit.html#gerrit[gerrit] section as link:#gerrit-info[ GerritInfo] entity. -|`gitweb ` |optional| -Information about the link:config-gerrit.html#gitweb[gitweb] |`plugin ` || Information about Gerrit extensions by plugins as link:#plugin-config-info[PluginConfigInfo] entity. diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java index a6ea4d2385..035817ad7d 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/config/ServerInfoIT.java @@ -108,9 +108,6 @@ public class ServerInfoIT extends AbstractDaemonTest { assertThat(i.gerrit.reportBugUrl).isEqualTo("https://example.com/report"); assertThat(i.gerrit.reportBugText).isEqualTo("REPORT BUG"); - // gitweb - assertThat(i.gitweb).isNull(); - // plugin assertThat(i.plugin.jsResourcePaths).isEmpty(); @@ -176,9 +173,6 @@ public class ServerInfoIT extends AbstractDaemonTest { assertThat(i.gerrit.reportBugUrl).isNull(); assertThat(i.gerrit.reportBugText).isNull(); - // gitweb - assertThat(i.gitweb).isNull(); - // plugin assertThat(i.plugin.jsResourcePaths).isEmpty(); diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GitwebType.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GitwebType.java index 7cdec2fb11..0ec7701ec9 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GitwebType.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GitwebType.java @@ -26,7 +26,6 @@ public class GitwebType { private String rootTree; private char pathSeparator = '/'; - private boolean linkDrafts = true; private boolean urlEncode = true; /** @return name displayed in links. */ @@ -141,20 +140,6 @@ public class GitwebType { this.pathSeparator = separator; } - /** @return whether to generate links to draft patch sets. */ - public boolean getLinkDrafts() { - return linkDrafts; - } - - /** - * Set whether to generate links to draft patch sets. - * - * @param linkDrafts new value. - */ - public void setLinkDrafts(boolean linkDrafts) { - this.linkDrafts = linkDrafts; - } - /** @return whether to URL encode path segments. */ public boolean getUrlEncode() { return urlEncode; diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebInfo.java deleted file mode 100644 index 7f26e635dd..0000000000 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebInfo.java +++ /dev/null @@ -1,168 +0,0 @@ -// Copyright (C) 2008 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.client.info; - -import com.google.gerrit.client.info.ChangeInfo.RevisionInfo; -import com.google.gerrit.common.data.ParameterizedString; -import com.google.gerrit.reviewdb.client.Branch; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.Project; -import com.google.gwt.core.client.JavaScriptObject; -import com.google.gwt.http.client.URL; - -import java.util.HashMap; -import java.util.Map; - -public class GitwebInfo extends JavaScriptObject { - public final native String url() /*-{ return this.url; }-*/; - public final native GitwebTypeInfo type() /*-{ return this.type; }-*/; - - /** - * Checks whether the given patch set can be linked. - * - * Draft patch sets can only be linked if linking of drafts was enabled by - * configuration. - * - * @param ps patch set to check whether it can be linked - * @return true if the patch set can be linked, otherwise false - */ - public final boolean canLink(PatchSet ps) { - return !ps.isDraft() || type().linkDrafts(); - } - - /** - * Checks whether the given revision can be linked. - * - * Draft revisions can only be linked if linking of drafts was enabled by - * configuration. - * - * @param revision revision to check whether it can be linked - * @return true if the revision can be linked, otherwise false - */ - public final boolean canLink(RevisionInfo revision) { - return revision.draft() || type().linkDrafts(); - } - - /** - * Returns the name for gitweb links. - * - * @return the name for gitweb links - */ - public final String getLinkName() { - return "(" + type().name() + ")"; - } - - /** - * Returns the gitweb link to a revision. - * - * @param project the name of the project - * @param commit the commit ID - * @return gitweb link to a revision - */ - public final String toRevision(String project, String commit) { - ParameterizedString pattern = new ParameterizedString(type().revision()); - Map p = new HashMap<>(); - p.put("project", encode(project)); - p.put("commit", encode(commit)); - return url() + pattern.replace(p); - } - - /** - * Returns the gitweb link to a revision. - * - * @param project the name of the project - * @param ps the patch set - * @return gitweb link to a revision - */ - public final String toRevision(Project.NameKey project, PatchSet ps) { - return toRevision(project.get(), ps.getRevision().get()); - } - - /** - * Returns the gitweb link to a project. - * - * @param project the project name key - * @return gitweb link to a project - */ - public final String toProject(Project.NameKey project) { - ParameterizedString pattern = new ParameterizedString(type().project()); - - Map p = new HashMap<>(); - p.put("project", encode(project.get())); - return url() + pattern.replace(p); - } - - /** - * Returns the gitweb link to a branch. - * - * @param branch the branch name key - * @return gitweb link to a branch - */ - public final String toBranch(Branch.NameKey branch) { - ParameterizedString pattern = new ParameterizedString(type().branch()); - - Map p = new HashMap<>(); - p.put("project", encode(branch.getParentKey().get())); - p.put("branch", encode(branch.get())); - return url() + pattern.replace(p); - } - - /** - * Returns the gitweb link to a file. - * - * @param project the branch name key - * @param commit the commit ID - * @param file the path of the file - * @return gitweb link to a file - */ - public final String toFile(String project, String commit, String file) { - Map p = new HashMap<>(); - p.put("project", encode(project)); - p.put("commit", encode(commit)); - p.put("file", encode(file)); - - ParameterizedString pattern = (file == null || file.isEmpty()) - ? new ParameterizedString(type().rootTree()) - : new ParameterizedString(type().file()); - return url() + pattern.replace(p); - } - - /** - * Returns the gitweb link to a file history. - * - * @param branch the branch name key - * @param file the path of the file - * @return gitweb link to a file history - */ - public final String toFileHistory(Branch.NameKey branch, String file) { - ParameterizedString pattern = new ParameterizedString(type().fileHistory()); - - Map p = new HashMap<>(); - p.put("project", encode(branch.getParentKey().get())); - p.put("branch", encode(branch.get())); - p.put("file", encode(file)); - return url() + pattern.replace(p); - } - - private String encode(String segment) { - if (type().urlEncode()) { - return URL.encodeQueryString(type().replacePathSeparator(segment)); - } - return segment; - } - - protected GitwebInfo() { - } -} diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebTypeInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebTypeInfo.java deleted file mode 100644 index 6726719dd0..0000000000 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GitwebTypeInfo.java +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright (C) 2015 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.client.info; - -import com.google.gwt.core.client.JavaScriptObject; - -public class GitwebTypeInfo extends JavaScriptObject { - /** - * Replace the standard path separator ('/') in a branch name or project - * name with a custom path separator configured by the property - * gitweb.pathSeparator. - * @param urlSegment The branch or project to replace the path separator in - * @return the urlSegment with the standard path separator replaced by the - * custom path separator - */ - public final String replacePathSeparator(String urlSegment) { - if (!"/".equals(pathSeparator())) { - return urlSegment.replace("/", pathSeparator()); - } - return urlSegment; - } - - public final native String name() /*-{ return this.name; }-*/; - public final native String revision() /*-{ return this.revision; }-*/; - public final native String project() /*-{ return this.project; }-*/; - public final native String branch() /*-{ return this.branch; }-*/; - public final native String rootTree() /*-{ return this.root_tree; }-*/; - public final native String file() /*-{ return this.file; }-*/; - public final native String fileHistory() /*-{ return this.file_history; }-*/; - public final native String pathSeparator() /*-{ return this.path_separator; }-*/; - public final native boolean linkDrafts() /*-{ return this.link_drafts || false; }-*/; - public final native boolean urlEncode() /*-{ return this.url_encode || false; }-*/; - - protected GitwebTypeInfo() { - } -} diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ServerInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ServerInfo.java index c91d08d567..112c4db1af 100644 --- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ServerInfo.java +++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/ServerInfo.java @@ -28,7 +28,6 @@ public class ServerInfo extends JavaScriptObject { public final native ChangeConfigInfo change() /*-{ return this.change; }-*/; public final native DownloadInfo download() /*-{ return this.download; }-*/; public final native GerritInfo gerrit() /*-{ return this.gerrit; }-*/; - public final native GitwebInfo gitweb() /*-{ return this.gitweb; }-*/; public final native PluginConfigInfo plugin() /*-{ return this.plugin; }-*/; public final native SshdInfo sshd() /*-{ return this.sshd; }-*/; public final native SuggestInfo suggest() /*-{ return this.suggest; }-*/; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java index 177faff0de..f29619ad5d 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessEditor.java @@ -15,16 +15,12 @@ package com.google.gerrit.client.admin; import com.google.gerrit.client.Dispatcher; -import com.google.gerrit.client.Gerrit; -import com.google.gerrit.client.info.GitwebInfo; import com.google.gerrit.client.ui.Hyperlink; import com.google.gerrit.client.ui.ParentProjectBox; import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.ProjectAccess; import com.google.gerrit.common.data.WebLinkInfoCommon; -import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; -import com.google.gerrit.reviewdb.client.RefNames; import com.google.gwt.core.client.GWT; import com.google.gwt.dom.client.DivElement; import com.google.gwt.dom.client.Style.Display; @@ -156,32 +152,14 @@ public class ProjectAccessEditor extends Composite implements } private void setUpWebLinks() { - if (!value.isConfigVisible()) { + List links = value.getFileHistoryLinks(); + if (!value.isConfigVisible() || links == null || links.isEmpty()) { history.getStyle().setDisplay(Display.NONE); - } else { - GitwebInfo c = Gerrit.info().gitweb(); - List links = value.getFileHistoryLinks(); - if (c == null && links == null) { - history.getStyle().setDisplay(Display.NONE); - } - if (c != null) { - webLinkPanel.add(toAnchor(c.toFileHistory(new Branch.NameKey(value.getProjectName(), - RefNames.REFS_CONFIG), "project.config"), c.getLinkName())); - } - - if (links != null) { - for (WebLinkInfoCommon link : links) { - webLinkPanel.add(toAnchor(link)); - } - } + return; + } + for (WebLinkInfoCommon link : links) { + webLinkPanel.add(toAnchor(link)); } - } - - private Anchor toAnchor(String href, String name) { - Anchor a = new Anchor(); - a.setHref(href); - a.setText(name); - return a; } private static Anchor toAnchor(WebLinkInfoCommon info) { diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectBranchesScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectBranchesScreen.java index 606a2eab98..31edefcbeb 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectBranchesScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectBranchesScreen.java @@ -25,7 +25,6 @@ import com.google.gerrit.client.access.AccessMap; import com.google.gerrit.client.access.ProjectAccessInfo; import com.google.gerrit.client.actions.ActionButton; import com.google.gerrit.client.info.ActionInfo; -import com.google.gerrit.client.info.GitwebInfo; import com.google.gerrit.client.info.WebLinkInfo; import com.google.gerrit.client.projects.BranchInfo; import com.google.gerrit.client.projects.ProjectApi; @@ -39,7 +38,6 @@ import com.google.gerrit.client.ui.NavigationTable; import com.google.gerrit.client.ui.OnEditEnabler; import com.google.gerrit.client.ui.PagingHyperlink; import com.google.gerrit.common.PageLinks; -import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gwt.core.client.JsArray; @@ -54,7 +52,6 @@ import com.google.gwt.event.dom.client.KeyUpEvent; import com.google.gwt.event.dom.client.KeyUpHandler; import com.google.gwt.event.logical.shared.ValueChangeEvent; import com.google.gwt.event.logical.shared.ValueChangeHandler; -import com.google.gwt.user.client.ui.Anchor; import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.CheckBox; import com.google.gwt.user.client.ui.FlexTable.FlexCellFormatter; @@ -408,8 +405,6 @@ public class ProjectBranchesScreen extends PaginatedProjectScreen { } void populate(int row, BranchInfo k) { - GitwebInfo c = Gerrit.info().gitweb(); - if (k.canDelete()) { CheckBox sel = new CheckBox(); sel.addValueChangeHandler(updateDeleteHandler); @@ -432,10 +427,6 @@ public class ProjectBranchesScreen extends PaginatedProjectScreen { } FlowPanel actionsPanel = new FlowPanel(); - if (c != null) { - actionsPanel.add(new Anchor(c.getLinkName(), false, - c.toBranch(new Branch.NameKey(getProjectKey(), k.ref())))); - } if (k.webLinks() != null) { for (WebLinkInfo webLink : Natives.asList(k.webLinks())) { actionsPanel.add(webLink.toAnchor()); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectListScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectListScreen.java index f31b116815..98eeb02ddf 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectListScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectListScreen.java @@ -18,7 +18,6 @@ import static com.google.gerrit.common.PageLinks.ADMIN_PROJECTS; import com.google.gerrit.client.Dispatcher; import com.google.gerrit.client.Gerrit; -import com.google.gerrit.client.info.GitwebInfo; import com.google.gerrit.client.info.WebLinkInfo; import com.google.gerrit.client.projects.ProjectInfo; import com.google.gerrit.client.projects.ProjectMap; @@ -34,7 +33,6 @@ import com.google.gwt.event.dom.client.KeyCodes; import com.google.gwt.event.dom.client.KeyUpEvent; import com.google.gwt.event.dom.client.KeyUpHandler; import com.google.gwt.user.client.History; -import com.google.gwt.user.client.ui.Anchor; import com.google.gwt.user.client.ui.FlowPanel; import com.google.gwt.user.client.ui.HorizontalPanel; import com.google.gwt.user.client.ui.Image; @@ -141,22 +139,12 @@ public class ProjectListScreen extends PaginatedProjectScreen { } private void addWebLinks(int row, ProjectInfo k) { - GitwebInfo gitwebLink = Gerrit.info().gitweb(); List webLinks = Natives.asList(k.webLinks()); - if (gitwebLink != null || (webLinks != null && !webLinks.isEmpty())) { + if (webLinks != null && !webLinks.isEmpty()) { FlowPanel p = new FlowPanel(); table.setWidget(row, ProjectsTable.C_REPO_BROWSER, p); - - if (gitwebLink != null) { - Anchor a = new Anchor(); - a.setText(gitwebLink.getLinkName()); - a.setHref(gitwebLink.toProject(k.name_key())); - p.add(a); - } - if (webLinks != null) { - for (WebLinkInfo weblink : webLinks) { - p.add(weblink.toAnchor()); - } + for (WebLinkInfo weblink : webLinks) { + p.add(weblink.toAnchor()); } } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java index b7ea168959..42963f7814 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/CommitBox.java @@ -16,13 +16,11 @@ package com.google.gerrit.client.change; import com.google.gerrit.client.AvatarImage; import com.google.gerrit.client.FormatUtil; -import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.info.AccountInfo; import com.google.gerrit.client.info.ChangeInfo; import com.google.gerrit.client.info.ChangeInfo.CommitInfo; import com.google.gerrit.client.info.ChangeInfo.GitPerson; import com.google.gerrit.client.info.ChangeInfo.RevisionInfo; -import com.google.gerrit.client.info.GitwebInfo; import com.google.gerrit.client.info.WebLinkInfo; import com.google.gerrit.client.rpc.Natives; import com.google.gerrit.client.ui.CommentLinkProcessor; @@ -38,7 +36,6 @@ import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.client.UiField; import com.google.gwt.uibinder.client.UiHandler; import com.google.gwt.user.client.DOM; -import com.google.gwt.user.client.ui.Anchor; import com.google.gwt.user.client.ui.Button; import com.google.gwt.user.client.ui.Composite; import com.google.gwt.user.client.ui.FlowPanel; @@ -118,13 +115,13 @@ class CommitBox extends Composite { committerDate, change); text.setHTML(commentLinkProcessor.apply( new SafeHtmlBuilder().append(commit.message()).linkify())); - setWebLinks(change, revision, revInfo); + setWebLinks(revInfo); if (revInfo.commit().parents().length() > 1) { mergeCommit.setVisible(true); } - setParents(change.project(), revInfo.commit().parents()); + setParents(revInfo.commit().parents()); } void setParentNotCurrent(boolean parentNotCurrent) { @@ -133,14 +130,7 @@ class CommitBox extends Composite { parentNotCurrentText.setInnerText(parentNotCurrent ? "\u25CF" : ""); } - private void setWebLinks(ChangeInfo change, String revision, - RevisionInfo revInfo) { - GitwebInfo gw = Gerrit.info().gitweb(); - if (gw != null && gw.canLink(revInfo)) { - toAnchor(gw.toRevision(change.project(), revision), - gw.getLinkName()); - } - + private void setWebLinks(RevisionInfo revInfo) { JsArray links = revInfo.commit().webLinks(); if (links != null) { for (WebLinkInfo link : Natives.asList(links)) { @@ -149,14 +139,7 @@ class CommitBox extends Composite { } } - private void toAnchor(String href, String name) { - Anchor a = new Anchor(); - a.setHref(href); - a.setText(name); - webLinkPanel.add(a); - } - - private void setParents(String project, JsArray commits) { + private void setParents(JsArray commits) { setVisible(firstParent, true); TableRowElement next = firstParent; TableRowElement previous = null; @@ -164,18 +147,11 @@ class CommitBox extends Composite { if (next == firstParent) { CopyableLabel copyLabel = getCommitLabel(c); parentCommits.add(copyLabel); - addLinks(project, c, parentWebLinks); } else { next.appendChild(DOM.createTD()); Element td1 = DOM.createTD(); td1.appendChild(getCommitLabel(c).getElement()); next.appendChild(td1); - FlowPanel linksPanel = new FlowPanel(); - linksPanel.addStyleName(style.parentWebLink()); - addLinks(project, c, linksPanel); - Element td2 = DOM.createTD(); - td2.appendChild(linksPanel.getElement()); - next.appendChild(td2); previous.getParentElement().insertAfter(next, previous); } previous = next; @@ -183,16 +159,6 @@ class CommitBox extends Composite { } } - private void addLinks(String project, CommitInfo c, FlowPanel panel) { - GitwebInfo gw = Gerrit.info().gitweb(); - if (gw != null) { - Anchor a = - new Anchor(gw.getLinkName(), gw.toRevision(project, c.commit())); - a.setStyleName(style.parentWebLink()); - panel.add(a); - } - } - private CopyableLabel getCommitLabel(CommitInfo c) { CopyableLabel copyLabel; copyLabel = new CopyableLabel(c.commit()); diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java index 788b4b09d1..9b3668c1de 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/RelatedChangesTab.java @@ -18,7 +18,6 @@ import com.google.gerrit.client.Gerrit; import com.google.gerrit.client.change.RelatedChanges.ChangeAndCommit; import com.google.gerrit.client.changes.Util; import com.google.gerrit.client.info.ChangeInfo.CommitInfo; -import com.google.gerrit.client.info.GitwebInfo; import com.google.gerrit.common.PageLinks; import com.google.gerrit.reviewdb.client.Change; import com.google.gwt.core.client.GWT; @@ -306,12 +305,7 @@ class RelatedChangesTab implements IsWidget { sb.closeSpan(); sb.openSpan(); - GitwebInfo gw = Gerrit.info().gitweb(); - if (gw != null && (!info.hasChangeNumber() || !info.hasRevisionNumber())) { - sb.setStyleName(RelatedChanges.R.css().gitweb()); - sb.setAttribute("title", gw.getLinkName()); - sb.append('\u25CF'); // Unicode 'BLACK CIRCLE' - } else if (info.status() != null && !info.status().isOpen()) { + if (info.status() != null && !info.status().isOpen()) { sb.setStyleName(RelatedChanges.R.css().gitweb()); sb.setAttribute("title", Util.toLongString(info.status())); sb.append('\u25CF'); // Unicode 'BLACK CIRCLE' @@ -340,11 +334,6 @@ class RelatedChangesTab implements IsWidget { if (info.hasChangeNumber() && info.hasRevisionNumber()) { return "#" + PageLinks.toChange(info.patchSetId()); } - - GitwebInfo gw = Gerrit.info().gitweb(); - if (gw != null && project != null) { - return gw.toRevision(project, info.commit().commit()); - } return null; } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java index 0d509b7261..f3770382fa 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java @@ -22,9 +22,7 @@ import com.google.gerrit.client.changes.ReviewInfo; import com.google.gerrit.client.changes.Util; import com.google.gerrit.client.diff.DiffInfo.Region; import com.google.gerrit.client.info.ChangeInfo; -import com.google.gerrit.client.info.ChangeInfo.RevisionInfo; import com.google.gerrit.client.info.FileInfo; -import com.google.gerrit.client.info.GitwebInfo; import com.google.gerrit.client.info.WebLinkInfo; import com.google.gerrit.client.patches.PatchUtil; import com.google.gerrit.client.rpc.CallbackGroup; @@ -114,32 +112,20 @@ public class Header extends Composite { if (!Gerrit.isSignedIn()) { reviewed.getElement().getStyle().setVisibility(Visibility.HIDDEN); } - SafeHtml.setInnerHTML(filePath, formatPath(path, null, null)); + SafeHtml.setInnerHTML(filePath, formatPath(path)); up.setTargetHistoryToken(PageLinks.toChange( patchSetId.getParentKey(), base != null ? base.getId() : null, patchSetId.getId())); } - public static SafeHtml formatPath(String path, String project, String commit) { + public static SafeHtml formatPath(String path) { SafeHtmlBuilder b = new SafeHtmlBuilder(); if (Patch.COMMIT_MSG.equals(path)) { return b.append(Util.C.commitMessage()); } - GitwebInfo gw = (project != null && commit != null) - ? Gerrit.info().gitweb() : null; int s = path.lastIndexOf('/') + 1; - if (gw != null && s > 0) { - String base = path.substring(0, s - 1); - b.openAnchor() - .setAttribute("href", gw.toFile(project, commit, base)) - .setAttribute("title", gw.getLinkName()) - .append(base) - .closeAnchor() - .append('/'); - } else { - b.append(path.substring(0, s)); - } + b.append(path.substring(0, s)); b.openElement("b"); b.append(path.substring(s)); b.closeElement("b"); @@ -200,23 +186,6 @@ public class Header extends Composite { } void setChangeInfo(ChangeInfo info) { - GitwebInfo gw = Gerrit.info().gitweb(); - if (gw != null) { - for (RevisionInfo rev : Natives.asList(info.revisions().values())) { - if (patchSetId.getId().equals(rev.id())) { - String c = rev.name(); - SafeHtml.setInnerHTML(filePath, formatPath(path, info.project(), c)); - SafeHtml.setInnerHTML(project, new SafeHtmlBuilder() - .openAnchor() - .setAttribute("href", gw.toFile(info.project(), c, "")) - .setAttribute("title", gw.getLinkName()) - .append(info.project()) - .closeAnchor()); - return; - } - } - } - project.setInnerText(info.project()); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/editor/EditScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/editor/EditScreen.java index b7adf3bb6c..da7ca4478f 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/editor/EditScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/editor/EditScreen.java @@ -191,7 +191,7 @@ public class EditScreen extends Screen { @Override public void onSuccess(ChangeInfo c) { project.setInnerText(c.project()); - SafeHtml.setInnerHTML(filePath, Header.formatPath(path, null, null)); + SafeHtml.setInnerHTML(filePath, Header.formatPath(path)); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 00fd97bd72..bb509f4c19 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -248,7 +248,6 @@ public class GerritGlobalModule extends FactoryModule { bind(ToolsCatalog.class); bind(EventFactory.class); bind(TransferConfig.class); - bind(GitwebConfig.class); bind(GcConfig.class); bind(ChangeCleanupConfig.class); @@ -329,6 +328,7 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.setOf(binder(), UploadValidationListener.class); DynamicMap.mapOf(binder(), ChangeQueryBuilder.ChangeOperatorFactory.class); + install(new GitwebConfig.LegacyModule(cfg)); bind(AnonymousUser.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java index 6df7a2ea2a..6c774cf7a4 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GetServerInfo.java @@ -65,7 +65,6 @@ public class GetServerInfo implements RestReadView { private final AllProjectsName allProjectsName; private final AllUsersName allUsersName; private final String anonymousCowardName; - private final GitwebConfig gitwebConfig; private final DynamicItem avatar; private final boolean enableSignedPush; private final QueryDocumentationExecutor docSearcher; @@ -83,7 +82,6 @@ public class GetServerInfo implements RestReadView { AllProjectsName allProjectsName, AllUsersName allUsersName, @AnonymousCowardName String anonymousCowardName, - GitwebConfig gitwebConfig, DynamicItem avatar, @EnableSignedPush boolean enableSignedPush, QueryDocumentationExecutor docSearcher) { @@ -98,7 +96,6 @@ public class GetServerInfo implements RestReadView { this.allProjectsName = allProjectsName; this.allUsersName = allUsersName; this.anonymousCowardName = anonymousCowardName; - this.gitwebConfig = gitwebConfig; this.avatar = avatar; this.enableSignedPush = enableSignedPush; this.docSearcher = docSearcher; @@ -113,7 +110,6 @@ public class GetServerInfo implements RestReadView { getDownloadInfo(downloadSchemes, downloadCommands, cloneCommands, archiveFormats); info.gerrit = getGerritInfo(config, allProjectsName, allUsersName); - info.gitweb = getGitwebInfo(gitwebConfig); info.plugin = getPluginInfo(); info.sshd = getSshdInfo(config); info.suggest = getSuggestInfo(config); @@ -262,17 +258,6 @@ public class GetServerInfo implements RestReadView { return CharMatcher.is('/').trimTrailingFrom(docUrl) + '/'; } - private GitwebInfo getGitwebInfo(GitwebConfig cfg) { - if (cfg.getUrl() == null || cfg.getGitwebType() == null) { - return null; - } - - GitwebInfo info = new GitwebInfo(); - info.url = cfg.getUrl(); - info.type = cfg.getGitwebType(); - return info; - } - private PluginConfigInfo getPluginInfo() { PluginConfigInfo info = new PluginConfigInfo(); info.hasAvatars = toBoolean(avatar.get() != null); @@ -335,7 +320,6 @@ public class GetServerInfo implements RestReadView { public ChangeConfigInfo change; public DownloadInfo download; public GerritInfo gerrit; - public GitwebInfo gitweb; public PluginConfigInfo plugin; public SshdInfo sshd; public SuggestInfo suggest; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GitwebConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GitwebConfig.java index 10b9fb03bd..836390d1d6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GitwebConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GitwebConfig.java @@ -15,12 +15,23 @@ package com.google.gerrit.server.config; import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.base.Strings.emptyToNull; import static com.google.common.base.Strings.isNullOrEmpty; import static com.google.common.base.Strings.nullToEmpty; -import com.google.common.base.Strings; import com.google.gerrit.common.data.GitwebType; +import com.google.gerrit.common.data.ParameterizedString; +import com.google.gerrit.extensions.common.WebLinkInfo; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.extensions.restapi.Url; +import com.google.gerrit.extensions.webui.BranchWebLink; +import com.google.gerrit.extensions.webui.FileHistoryWebLink; +import com.google.gerrit.extensions.webui.FileWebLink; +import com.google.gerrit.extensions.webui.PatchSetWebLink; +import com.google.gerrit.extensions.webui.ProjectWebLink; +import com.google.inject.AbstractModule; import com.google.inject.Inject; +import com.google.inject.Singleton; import org.eclipse.jgit.lib.Config; import org.slf4j.Logger; @@ -34,21 +45,52 @@ public class GitwebConfig { || isEmptyString(cfg, "gitweb", null, "cgi"); } + public static class LegacyModule extends AbstractModule { + private final Config cfg; + + public LegacyModule(Config cfg) { + this.cfg = cfg; + } + + @Override + protected void configure() { + GitwebType type = typeFromConfig(cfg); + if (type != null) { + bind(GitwebType.class).toInstance(type); + + if (!isNullOrEmpty(type.getBranch())) { + DynamicSet.bind(binder(), BranchWebLink.class).to(GitwebLinks.class); + } + + if (!isNullOrEmpty(type.getFile()) + || !isNullOrEmpty(type.getRootTree())) { + DynamicSet.bind(binder(), FileWebLink.class).to(GitwebLinks.class); + } + + if (!isNullOrEmpty(type.getFileHistory())) { + DynamicSet.bind(binder(), FileHistoryWebLink.class).to(GitwebLinks.class); + } + + if (!isNullOrEmpty(type.getRevision())) { + DynamicSet.bind(binder(), PatchSetWebLink.class).to(GitwebLinks.class); + } + + if (!isNullOrEmpty(type.getProject())) { + DynamicSet.bind(binder(), ProjectWebLink.class).to(GitwebLinks.class); + } + } + } + } + private static boolean isEmptyString(Config cfg, String section, String subsection, String name) { // This is currently the only way to check for the empty string in a JGit // config. Fun! String[] values = cfg.getStringList(section, subsection, name); - return values.length > 0 && Strings.isNullOrEmpty(values[0]); + return values.length > 0 && isNullOrEmpty(values[0]); } - /** - * Get a GitwebType based on the given config. - * - * @param cfg Gerrit config. - * @return GitwebType from the given name, else null if not found. - */ - public static GitwebType typeFromConfig(Config cfg) { + private static GitwebType typeFromConfig(Config cfg) { GitwebType defaultType = defaultType(cfg.getString("gitweb", null, "type")); if (defaultType == null) { return null; @@ -76,9 +118,6 @@ public class GitwebConfig { type.setFileHistory(firstNonNull( cfg.getString("gitweb", null, "filehistory"), defaultType.getFileHistory())); - type.setLinkDrafts( - cfg.getBoolean("gitweb", null, "linkdrafts", - defaultType.getLinkDrafts())); type.setUrlEncode( cfg.getBoolean("gitweb", null, "urlencode", defaultType.getUrlEncode())); @@ -133,6 +172,7 @@ public class GitwebConfig { type.setFile(""); type.setFileHistory(""); break; + case "disabled": default: return null; } @@ -147,48 +187,18 @@ public class GitwebConfig { if (isDisabled(cfg)) { type = null; url = null; - return; - } - - String cfgUrl = cfg.getString("gitweb", null, "url"); - GitwebType type = typeFromConfig(cfg); - if (type == null) { - this.type = null; - url = null; - return; - } else if (cgiConfig.getGitwebCgi() == null) { - // Use an externally managed gitweb instance, and not an internal one. - url = cfgUrl; } else { - url = firstNonNull(cfgUrl, "gitweb"); + String cfgUrl = cfg.getString("gitweb", null, "url"); + type = typeFromConfig(cfg); + if (type == null) { + url = null; + } else if (cgiConfig.getGitwebCgi() == null) { + // Use an externally managed gitweb instance, and not an internal one. + url = cfgUrl; + } else { + url = firstNonNull(cfgUrl, "gitweb"); + } } - - if (isNullOrEmpty(type.getBranch())) { - log.warn("No Pattern specified for gitweb.branch, disabling."); - this.type = null; - } else if (isNullOrEmpty(type.getProject())) { - log.warn("No Pattern specified for gitweb.project, disabling."); - this.type = null; - } else if (isNullOrEmpty(type.getRevision())) { - log.warn("No Pattern specified for gitweb.revision, disabling."); - this.type = null; - } else if (isNullOrEmpty(type.getRootTree())) { - log.warn("No Pattern specified for gitweb.roottree, disabling."); - this.type = null; - } else if (isNullOrEmpty(type.getFile())) { - log.warn("No Pattern specified for gitweb.file, disabling."); - this.type = null; - } else if (isNullOrEmpty(type.getFileHistory())) { - log.warn("No Pattern specified for gitweb.filehistory, disabling."); - this.type = null; - } else { - this.type = type; - } - } - - /** @return GitwebType for gitweb viewer. */ - public GitwebType getGitwebType() { - return type; } /** @@ -226,4 +236,103 @@ public class GitwebConfig { return false; } } + + @Singleton + static class GitwebLinks implements BranchWebLink, FileHistoryWebLink, + FileWebLink, PatchSetWebLink, ProjectWebLink { + private final String url; + private final GitwebType type; + private final ParameterizedString branch; + private final ParameterizedString file; + private final ParameterizedString fileHistory; + private final ParameterizedString project; + private final ParameterizedString revision; + + @Inject + GitwebLinks(GitwebConfig config, GitwebType type) { + this.url = config.getUrl(); + this.type = type; + this.branch = parse(type.getBranch()); + this.file = parse(firstNonNull( + emptyToNull(type.getFile()), + nullToEmpty(type.getRootTree()))); + this.fileHistory = parse(type.getFileHistory()); + this.project = parse(type.getProject()); + this.revision = parse(type.getRevision()); + } + + @Override + public WebLinkInfo getBranchWebLink(String projectName, String branchName) { + if (branch != null) { + return link(branch + .replace("project", encode(projectName)) + .replace("branch", encode(branchName)) + .toString()); + } + return null; + } + + @Override + public WebLinkInfo getFileHistoryWebLink(String projectName, + String revision, String fileName) { + if (fileHistory != null) { + return link(revision + .replace("project", encode(projectName)) + .replace("branch", encode(revision)) + .replace("file", encode(fileName)) + .toString()); + } + return null; + } + + @Override + public WebLinkInfo getFileWebLink(String projectName, String revision, + String fileName) { + if (file != null) { + return link(file + .replace("project", encode(projectName)) + .replace("commit", encode(revision)) + .replace("file", encode(fileName)) + .toString()); + } + return null; + } + + @Override + public WebLinkInfo getPatchSetWebLink(String projectName, String commit) { + if (revision != null) { + return link(revision + .replace("project", encode(projectName)) + .replace("commit", encode(commit)) + .toString()); + } + return null; + } + + @Override + public WebLinkInfo getProjectWeblink(String projectName) { + if (project != null) { + return link(project.replace("project", encode(projectName)).toString()); + } + return null; + } + + private String encode(String val) { + if (type.getUrlEncode()) { + return Url.encode(type.replacePathSeparator(val)); + } + return val; + } + + private WebLinkInfo link(String rest) { + return new WebLinkInfo(type.getLinkName(), null, url + rest, null); + } + + private static ParameterizedString parse(String pattern) { + if (!isNullOrEmpty(pattern)) { + return new ParameterizedString(pattern); + } + return null; + } + } }