Skip to content

Improve Bitbucket support to sent back build statuses for pull requests #26

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve Bitbucket support to sent back build statuses for pull requests #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions ...tatus-publisher-server/src/main/java/jetbrains/buildServer/commitPublisher/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public class Constants {

public static final String STASH_PUBLISHER_ID = "atlassianStashPublisher";
public static final String STASH_BASE_URL = "stashBaseUrl";
public static final String STASH_PROJECT_KEY = "stashProjectKey";
public static final String STASH_REPO_NAME = "stashRepoName";
public static final String STASH_USERNAME = "stashUsername";
public static final String STASH_PASSWORD = "secure:stashPassword";

Expand Down Expand Up @@ -91,9 +93,13 @@ public String getSshKey() {
}

@NotNull
public String getStashBaseUrl() {
return STASH_BASE_URL;
}
public String getStashBaseUrl() { return STASH_BASE_URL; }

@NotNull
public String getStashProjectKey() { return STASH_PROJECT_KEY; }

@NotNull
public String getStashRepoName() { return STASH_REPO_NAME; }

@NotNull
public String getStashUsername() {
Expand Down
77 changes: 73 additions & 4 deletions ...sher-server/src/main/java/jetbrains/buildServer/commitPublisher/stash/StashPublisher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,34 @@
package jetbrains.buildServer.commitPublisher.stash;

import com.google.common.collect.Iterables;
import com.google.gson.*;
import com.intellij.openapi.diagnostic.Logger;
import jetbrains.buildServer.commitPublisher.*;
import jetbrains.buildServer.commitPublisher.stash.data.PullRequestInfo;
import jetbrains.buildServer.commitPublisher.stash.data.StashCommit;
import jetbrains.buildServer.serverSide.*;
import jetbrains.buildServer.serverSide.executors.ExecutorServices;
import jetbrains.buildServer.serverSide.impl.LogUtil;
import jetbrains.buildServer.users.User;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.StatusLine;
import org.apache.http.*;
import org.apache.http.entity.ContentType;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;

class StashPublisher extends HttpBasedCommitStatusPublisher {
public static final String PUBLISH_QUEUED_BUILD_STATUS = "teamcity.stashCommitStatusPublisher.publishQueuedBuildStatus";

private static final Logger LOG = Logger.getInstance(StashPublisher.class.getName());

protected final Gson myGson = new Gson();

private final WebLinks myLinks;

StashPublisher(@NotNull CommitStatusPublisherSettings settings,
Expand Down Expand Up @@ -121,7 +127,16 @@ private void vote(@NotNull SBuild build,
@NotNull StashBuildStatus status,
@NotNull String comment) {
String msg = createMessage(status, build.getBuildPromotion().getBuildTypeExternalId(), getBuildName(build), myLinks.getViewResultsUrl(build), comment);
vote(revision.getRevision(), msg, LogUtil.describe(build));
String commitRevision = revision.getRevision();
try {
StashCommit commit = findPullRequestCommit(build);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this can be addressed easily, but at the moment we try to issue HTTP requests outside the threads where the event handlers code works, so the event handlers return as quick as possible. See postAsync(...) calls in vote(...). The findPullRequestCommit(...) call is performed in the same thread and will make buildStarted, buildFinished etc. waiting if the branch name contains "merge".

if (commit != null) {
commitRevision = commit.id;
}
} catch (PublisherException e) {
e.printStackTrace();
}
vote(commitRevision, msg, LogUtil.describe(build));
}

private void vote(@NotNull SQueuedBuild build,
Expand Down Expand Up @@ -214,4 +229,58 @@ private String getUsername() {
private String getPassword() {
return myParams.get(Constants.STASH_PASSWORD);
}

private StashCommit findPullRequestCommit(@NotNull SBuild build) throws PublisherException {
String apiUrl = myParams.get(Constants.STASH_BASE_URL);
String projectKey = myParams.get(Constants.STASH_PROJECT_KEY);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other publishers, as well as for StashSettings::testConnection method we use GitRepositoryParser.parseRepository method to extract the repo details necessary to build the URL. It may be beneficial to introduce an explicit Commit Status Publisher settings for the same, but they have to be optional in my opinion.

String repository = myParams.get(Constants.STASH_REPO_NAME);

final List<StashCommit> lastCommit = new ArrayList<StashCommit>();

// Plugin will recognize pull request only when Branch specification parameter will be configured as below:
// +:refs/pull-requests/(*/merge)
// https://blog.jetbrains.com/teamcity/2013/02/automatically-building-pull-requests-from-github-with-teamcity

if (build.getBranch().getName().contains("merge")) { // try to find pull request only for branches with merge in the name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possible NPE: build.getBranch() is nullable

String pullRequestNumber = build.getBranch().getName().replace("/merge",""); //remove merge from brache name to get only pull request number
apiUrl = apiUrl + "/rest/api/1.0/projects/" + projectKey + "/repos/" + repository + "/pull-requests/" + pullRequestNumber + "/commits/";
LOG.debug("Bitbucket Pull Request apiUrl: " + apiUrl);
}

if (null != apiUrl || apiUrl.length() != 0) {
try {
HttpResponseProcessor processor = new DefaultHttpResponseProcessor() {
@Override
public void processResponse(HttpResponse response) throws HttpPublisherException, IOException {

final HttpEntity entity = response.getEntity();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and below please refer to the most recent changes in Commit Status Publisher, as the way we make and process these HTTP requests has changed a bit.

if (null == entity) {
throw new HttpPublisherException("Bitbucket publisher has received no response");
}
ByteArrayOutputStream bos = new ByteArrayOutputStream();
entity.writeTo(bos);
final String json = bos.toString("utf-8");
PullRequestInfo commitInfo = myGson.fromJson(json, PullRequestInfo.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems if there is no PullRequestInfo in the response (e.g. the branch name contained "merge" substring for some other reason), the status publishing will fail. Is that right?

if (null == commitInfo)
throw new HttpPublisherException("Bitbucket Server publisher has received a malformed response");
if (null != commitInfo.values && !commitInfo.values.isEmpty()) {
StashCommit commit = Iterables.get(commitInfo.values, 0);
if (commit != null) {
lastCommit.add(commit);
}
}
}
};

HttpHelper.get(apiUrl, getUsername(), getPassword(),
Collections.singletonMap("Accept", "application/json"), BaseCommitStatusPublisher.DEFAULT_CONNECTION_TIMEOUT, processor);
} catch (Exception ex) {
throw new PublisherException(String.format("Bitbucket Server publisher has failed to connect to %s repository", apiUrl), ex);
}
}
if (!lastCommit.isEmpty() && lastCommit.size() > 0) {
return lastCommit.get(0);
}
return null;
}
}
14 changes: 14 additions & 0 deletions ...erver/src/main/java/jetbrains/buildServer/commitPublisher/stash/data/PullRequestInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package jetbrains.buildServer.commitPublisher.stash.data;

import java.util.Collection;

/**
* Created by rafalkasa on 2017-03-19.
*/

/**
* This class does not represent full repository information
*/
public class PullRequestInfo {
public Collection<StashCommit> values;
}
13 changes: 13 additions & 0 deletions ...er-server/src/main/java/jetbrains/buildServer/commitPublisher/stash/data/StashCommit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package jetbrains.buildServer.commitPublisher.stash.data;

/**
* Created by rafalkasa on 2017-03-19.
*/

/**
* This class does not represent full repository information
*/
public class StashCommit {
public String id;
public String displayId;
}
44 changes: 30 additions & 14 deletions ...t-status-publisher-server/src/main/resources/buildServerResources/stash/stashSettings.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,35 @@
<span class="error" id="error_${keys.stashBaseUrl}"></span>
</td>
</tr>

<tr>
<th><label for="${keys.stashUsername}">Username: <l:star/></label></th>
<td>
<props:textProperty name="${keys.stashUsername}" style="width:18em;"/>
</td>
</tr>

<tr>
<th><label for="${keys.stashPassword}">Password: <l:star/></label></th>
<td>
<props:passwordProperty name="${keys.stashPassword}" style="width:18em;"/>
</td>
</tr>
<l:settingsGroup title="Repository">
<span class="smallNote">Plugin will recognize Pull Request branches only when <strong><a href="https://confluence.jetbrains.com/display/TCD10/Working+with+Feature+Branches">Branch specification</a></strong> parameter in <strong>VCS Roots</strong> will be configured as below:</span>
<span class="smallNote"><strong><i>+:refs/pull-requests/(*/merge)</i></strong></span>
<tr>
<th><label for="${keys.stashProjectKey}">Project Key: <l:star/></label></th>
<td>
<props:textProperty name="${keys.stashProjectKey}" style="width:18em;"/>
</td>
</tr>
<tr>
<th><label for="${keys.stashRepoName}">Repository: <l:star/></label></th>
<td>
<props:textProperty name="${keys.stashRepoName}" style="width:18em;"/>
</td>
</tr>
</l:settingsGroup>
<l:settingsGroup title="Authentication">
<tr>
<th><label for="${keys.stashUsername}">Username: <l:star/></label></th>
<td>
<props:textProperty name="${keys.stashUsername}" style="width:18em;"/>
</td>
</tr>
<tr>
<th><label for="${keys.stashPassword}">Password: <l:star/></label></th>
<td>
<props:passwordProperty name="${keys.stashPassword}" style="width:18em;"/>
</td>
</tr>
</l:settingsGroup>
</table>