opendev/gerrit
Code Issues Proposed changes

Extend PatchSetWebLink to include subject and branch name

Browse Source 
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
This commit is contained in:
Gal Paikin
2020-11-27 14:35:22 +01:00
parent fcefd46b52
commit 57c9d58f1d
10 changed files with 79 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
Documentation/dev-plugins.txt
View File

@@ -2241,7 +2241,8 @@ public class MyWeblinkPlugin implements PatchSetWebLink {
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),

9
java/com/google/gerrit/extensions/webui/ParentWebLink.java
View File

@@ -30,10 +30,13 @@ public interface ParentWebLink extends WebLink {
*
* <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);
}

9
java/com/google/gerrit/extensions/webui/PatchSetWebLink.java
View File

@@ -30,10 +30,13 @@ public interface PatchSetWebLink extends WebLink {
*
* <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);
}

18
java/com/google/gerrit/server/WebLinks.java
View File

@@ -86,19 +86,29 @@ public class WebLinks {
/**
* @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));
}
/**

18
java/com/google/gerrit/server/change/RevisionJson.java
View File

@@ -164,7 +164,12 @@ public class RevisionJson {
* 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 @@ public class RevisionJson {
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 @@ public class RevisionJson {
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 @@ public class RevisionJson {
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());

9
java/com/google/gerrit/server/config/GitwebConfig.java
View File

@@ -327,8 +327,10 @@ public class GitwebConfig {
}
@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 @@ public class GitwebConfig {
}
@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

8
java/com/google/gerrit/server/restapi/change/GetCommit.java
View File

@@ -58,7 +58,13 @@ public class GetCommit implements RestReadView<RevisionResource> {
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));

9
java/com/google/gerrit/server/restapi/change/GetMergeList.java
View File

@@ -81,7 +81,14 @@ public class GetMergeList implements RestReadView<RevisionResource> {
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);
}

23
javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
View File

@@ -91,6 +91,7 @@ import com.google.gerrit.common.data.GlobalCapability;
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.ChangeMessagesUtil;
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 class ChangeIT extends AbstractDaemonTest {
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 =

3
javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
View File

@@ -1573,7 +1573,8 @@ public class RevisionIT extends AbstractDaemonTest {
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;
}
};