Extend PatchSetWebLink to include subject and branch name
This is useful for more complicated external links.
While at it, also extend ParentWebLink since our docs claim they should
have similar implementation in most cases.
Some tests were fixed.
Change-Id: I420161ca4cf618e45f4cec966eb4e7d45dcacab9
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 1db27d5..2f4c46c 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2241,7 +2241,8 @@
private String imageUrl = "http://placehold.it/16x16.gif";
@Override
- public WebLinkInfo getPatchSetWebLink(String projectName, String commit) {
+ public WebLinkInfo getPatchSetWebLink(String projectName, String commit,
+ String subject, String branchName) {
return new WebLinkInfo(name,
imageUrl,
String.format(placeHolderUrlProjectCommit, project, commit),
diff --git a/java/com/google/gerrit/extensions/webui/ParentWebLink.java b/java/com/google/gerrit/extensions/webui/ParentWebLink.java
index dfc970d..2176e78 100644
--- a/java/com/google/gerrit/extensions/webui/ParentWebLink.java
+++ b/java/com/google/gerrit/extensions/webui/ParentWebLink.java
@@ -30,10 +30,13 @@
*
* <p>
*
- * @param projectName Name of the project
- * @param commit Commit sha1 of the parent revision
+ * @param projectName name of the project
+ * @param commit commit sha1 of the parent revision
+ * @param subject first line of the commit message
+ * @param branchName target branch of the change
* @return WebLinkInfo that links to parent commit in external service, null if there should be no
* link.
*/
- WebLinkInfo getParentWebLink(String projectName, String commit);
+ WebLinkInfo getParentWebLink(
+ String projectName, String commit, String subject, String branchName);
}
diff --git a/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java b/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
index 93fe8e1..b8ba5c4 100644
--- a/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
+++ b/java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
@@ -30,10 +30,13 @@
*
* <p>
*
- * @param projectName Name of the project
- * @param commit Commit of the patch set
+ * @param projectName name of the project
+ * @param commit commit of the patch set
+ * @param subject first line of the commit message
+ * @param branchName target branch of the change
* @return WebLinkInfo that links to patch set in external service, null if there should be no
* link.
*/
- WebLinkInfo getPatchSetWebLink(String projectName, String commit);
+ WebLinkInfo getPatchSetWebLink(
+ String projectName, String commit, String subject, String branchName);
}
diff --git a/java/com/google/gerrit/server/WebLinks.java b/java/com/google/gerrit/server/WebLinks.java
index 88b0b21..47ba325 100644
--- a/java/com/google/gerrit/server/WebLinks.java
+++ b/java/com/google/gerrit/server/WebLinks.java
@@ -86,19 +86,29 @@
/**
* @param project Project name.
* @param commit SHA1 of commit.
+ * @param subject subject of the commit.
+ * @param branchName branch of the commit.
* @return Links for patch sets.
*/
- public ImmutableList<WebLinkInfo> getPatchSetLinks(Project.NameKey project, String commit) {
- return filterLinks(patchSetLinks, webLink -> webLink.getPatchSetWebLink(project.get(), commit));
+ public ImmutableList<WebLinkInfo> getPatchSetLinks(
+ Project.NameKey project, String commit, String subject, String branchName) {
+ return filterLinks(
+ patchSetLinks,
+ webLink -> webLink.getPatchSetWebLink(project.get(), commit, subject, branchName));
}
/**
* @param project Project name.
* @param revision SHA1 of the parent revision.
+ * @param subject subject of the parent revision.
+ * @param branchName branch of the revision (and parent revision).
* @return Links for patch sets.
*/
- public ImmutableList<WebLinkInfo> getParentLinks(Project.NameKey project, String revision) {
- return filterLinks(parentLinks, webLink -> webLink.getParentWebLink(project.get(), revision));
+ public ImmutableList<WebLinkInfo> getParentLinks(
+ Project.NameKey project, String revision, String subject, String branchName) {
+ return filterLinks(
+ parentLinks,
+ webLink -> webLink.getParentWebLink(project.get(), revision, subject, branchName));
}
/**
diff --git a/java/com/google/gerrit/server/change/RevisionJson.java b/java/com/google/gerrit/server/change/RevisionJson.java
index 414107f..90caea0 100644
--- a/java/com/google/gerrit/server/change/RevisionJson.java
+++ b/java/com/google/gerrit/server/change/RevisionJson.java
@@ -164,7 +164,12 @@
* RevWalk and assumes it is backed by an open repository.
*/
public CommitInfo getCommitInfo(
- Project.NameKey project, RevWalk rw, RevCommit commit, boolean addLinks, boolean fillCommit)
+ Project.NameKey project,
+ RevWalk rw,
+ RevCommit commit,
+ boolean addLinks,
+ boolean fillCommit,
+ String branchName)
throws IOException {
CommitInfo info = new CommitInfo();
if (fillCommit) {
@@ -177,7 +182,8 @@
info.message = commit.getFullMessage();
if (addLinks) {
- ImmutableList<WebLinkInfo> links = webLinks.getPatchSetLinks(project, commit.name());
+ ImmutableList<WebLinkInfo> links =
+ webLinks.getPatchSetLinks(project, commit.name(), commit.getShortMessage(), branchName);
info.webLinks = links.isEmpty() ? null : links;
}
@@ -187,7 +193,8 @@
i.commit = parent.name();
i.subject = parent.getShortMessage();
if (addLinks) {
- ImmutableList<WebLinkInfo> parentLinks = webLinks.getParentLinks(project, parent.name());
+ ImmutableList<WebLinkInfo> parentLinks =
+ webLinks.getParentLinks(project, parent.name(), parent.getShortMessage(), branchName);
i.webLinks = parentLinks.isEmpty() ? null : parentLinks;
}
info.parents.add(i);
@@ -288,11 +295,12 @@
String rev = in.commitId().name();
RevCommit commit = rw.parseCommit(ObjectId.fromString(rev));
rw.parseBody(commit);
+ String branchName = cd.change().getDest().branch();
if (setCommit) {
- out.commit = getCommitInfo(project, rw, commit, has(WEB_LINKS), fillCommit);
+ out.commit = getCommitInfo(project, rw, commit, has(WEB_LINKS), fillCommit, branchName);
}
if (addFooters) {
- Ref ref = repo.exactRef(cd.change().getDest().branch());
+ Ref ref = repo.exactRef(branchName);
RevCommit mergeTip = null;
if (ref != null) {
mergeTip = rw.parseCommit(ref.getObjectId());
diff --git a/java/com/google/gerrit/server/config/GitwebConfig.java b/java/com/google/gerrit/server/config/GitwebConfig.java
index b5f09fd..3340a52 100644
--- a/java/com/google/gerrit/server/config/GitwebConfig.java
+++ b/java/com/google/gerrit/server/config/GitwebConfig.java
@@ -327,8 +327,10 @@
}
@Override
- public WebLinkInfo getPatchSetWebLink(String projectName, String commit) {
+ public WebLinkInfo getPatchSetWebLink(
+ String projectName, String commit, String subject, String branchName) {
if (revision != null) {
+ // subject and branchName are not needed, hence not used.
return link(
revision
.replace("project", encode(projectName))
@@ -339,9 +341,10 @@
}
@Override
- public WebLinkInfo getParentWebLink(String projectName, String commit) {
+ public WebLinkInfo getParentWebLink(
+ String projectName, String commit, String subject, String branchName) {
// For Gitweb treat parent revision links the same as patch set links
- return getPatchSetWebLink(projectName, commit);
+ return getPatchSetWebLink(projectName, commit, subject, branchName);
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/change/GetCommit.java b/java/com/google/gerrit/server/restapi/change/GetCommit.java
index 21a08dc..d76ce04 100644
--- a/java/com/google/gerrit/server/restapi/change/GetCommit.java
+++ b/java/com/google/gerrit/server/restapi/change/GetCommit.java
@@ -58,7 +58,13 @@
rw.parseBody(commit);
CommitInfo info =
json.create(ImmutableSet.of())
- .getCommitInfo(rsrc.getProject(), rw, commit, addLinks, true);
+ .getCommitInfo(
+ rsrc.getProject(),
+ rw,
+ commit,
+ addLinks,
+ /* fillCommit= */ true,
+ rsrc.getChange().getDest().branch());
Response<CommitInfo> r = Response.ok(info);
if (rsrc.isCacheable()) {
r.caching(CacheControl.PRIVATE(7, TimeUnit.DAYS));
diff --git a/java/com/google/gerrit/server/restapi/change/GetMergeList.java b/java/com/google/gerrit/server/restapi/change/GetMergeList.java
index 0c67fd6..f0639b5 100644
--- a/java/com/google/gerrit/server/restapi/change/GetMergeList.java
+++ b/java/com/google/gerrit/server/restapi/change/GetMergeList.java
@@ -81,7 +81,14 @@
List<CommitInfo> result = new ArrayList<>(commits.size());
RevisionJson changeJson = json.create(ImmutableSet.of());
for (RevCommit c : commits) {
- result.add(changeJson.getCommitInfo(rsrc.getProject(), rw, c, addLinks, true));
+ result.add(
+ changeJson.getCommitInfo(
+ rsrc.getProject(),
+ rw,
+ c,
+ addLinks,
+ /* fillCommit= */ true,
+ rsrc.getChange().getDest().branch()));
}
return createResponse(rsrc, result);
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index f59fba0..d108fea 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -91,6 +91,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
import com.google.gerrit.entities.Address;
+import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.LabelType;
@@ -154,6 +155,7 @@
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.testing.TestChangeETagComputation;
+import com.google.gerrit.server.git.ChangeMessageModifier;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@@ -2922,14 +2924,19 @@
public void customCommitFooters() throws Exception {
PushOneCommit.Result change = createChange();
ChangeInfo actual;
- try (Registration registration =
- extensionRegistry
- .newRegistration()
- .add(
- (newCommitMessage, original, mergeTip, destination) -> {
- assertThat(original.getName()).isNotEqualTo(mergeTip.getName());
- return newCommitMessage + "Custom: " + destination.branch();
- })) {
+ ChangeMessageModifier link =
+ new ChangeMessageModifier() {
+ @Override
+ public String onSubmit(
+ String newCommitMessage,
+ RevCommit original,
+ RevCommit mergeTip,
+ BranchNameKey destination) {
+ assertThat(original.getName()).isNotEqualTo(mergeTip.getName());
+ return newCommitMessage + "Custom: " + destination.branch();
+ }
+ };
+ try (Registration registration = extensionRegistry.newRegistration().add(link)) {
actual = gApi.changes().id(change.getChangeId()).get(ALL_REVISIONS, COMMIT_FOOTERS);
}
List<String> footers =
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index 839b051..7662a54 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -1573,7 +1573,8 @@
PatchSetWebLink link =
new PatchSetWebLink() {
@Override
- public WebLinkInfo getPatchSetWebLink(String projectName, String commit) {
+ public WebLinkInfo getPatchSetWebLink(
+ String projectName, String commit, String subject, String branchName) {
return expectedWebLinkInfo;
}
};
