From 4fef8ad7d49bdb22676879193d832aaba7c2df6f Mon Sep 17 00:00:00 2001 From: Mac Browning Date: Thu, 17 Apr 2014 18:57:13 -0400 Subject: [PATCH] Correct url building for branch query. Fixes drone/drone#265 --- pkg/plugin/notify/slack.go | 9 ++++++-- pkg/plugin/notify/slack_test.go | 41 +++++++++++++++++++++++++++++++++ pkg/queue/worker.go | 12 +++++++--- pkg/queue/worker_test.go | 31 +++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 pkg/plugin/notify/slack_test.go create mode 100644 pkg/queue/worker_test.go diff --git a/pkg/plugin/notify/slack.go b/pkg/plugin/notify/slack.go index 1ca6bfcd..009c4538 100644 --- a/pkg/plugin/notify/slack.go +++ b/pkg/plugin/notify/slack.go @@ -3,7 +3,7 @@ package notify import ( "encoding/json" "fmt" - "path" + "net/url" ) const ( @@ -37,7 +37,12 @@ func (s *Slack) Send(context *Context) error { } func getBuildUrl(context *Context) string { - return context.Host + path.Join("/", context.Repo.Slug, "commit", context.Commit.Hash) + branchQuery := url.Values{} + if context.Commit.Branch != "" { + branchQuery.Set("branch", context.Commit.Branch) + } + + return fmt.Sprintf("%s/%s/commit/%s?%s", context.Host, context.Repo.Slug, context.Commit.Hash, branchQuery.Encode()) } func getMessage(context *Context, message string) string { diff --git a/pkg/plugin/notify/slack_test.go b/pkg/plugin/notify/slack_test.go new file mode 100644 index 00000000..887d355c --- /dev/null +++ b/pkg/plugin/notify/slack_test.go @@ -0,0 +1,41 @@ +package notify + +import ( + "github.com/drone/drone/pkg/model" + "testing" +) + +func Test_getBuildUrl(t *testing.T) { + c := &Context{ + Host: "http://examplehost.com", + Repo: &model.Repo{ + Slug: "examplegit.com/owner/repo", + }, + Commit: &model.Commit{ + Hash: "abc", + Branch: "example", + }, + } + expected := "http://examplehost.com/examplegit.com/owner/repo/commit/abc?branch=example" + output := getBuildUrl(c) + + if output != expected { + t.Errorf("Failed to build url. Expected: %s, got %s", expected, output) + } + + c.Commit.Branch = "url/unsafe/branch" + expected = "http://examplehost.com/examplegit.com/owner/repo/commit/abc?branch=url%2Funsafe%2Fbranch" + output = getBuildUrl(c) + + if output != expected { + t.Errorf("Failed to build url. Expected: %s, got %s", expected, output) + } + + c.Commit.Branch = "" + expected = "http://examplehost.com/examplegit.com/owner/repo/commit/abc?" + output = getBuildUrl(c) + + if output != expected { + t.Errorf("Failed to build url. Expected: %s, got %s", expected, output) + } +} diff --git a/pkg/queue/worker.go b/pkg/queue/worker.go index 8c9b4a51..bf9aeab3 100644 --- a/pkg/queue/worker.go +++ b/pkg/queue/worker.go @@ -12,6 +12,7 @@ import ( "github.com/drone/go-github/github" "io" "log" + "net/url" "path/filepath" "time" ) @@ -222,11 +223,16 @@ func updateGitHubStatus(repo *Repo, commit *Commit) error { client := github.New(user.GithubToken) client.ApiUrl = settings.GitHubApiUrl + buildUrl := getBuildUrl(settings.URL().String(), repo, commit) - var url string - url = settings.URL().String() + "/" + repo.Slug + "/commit/" + commit.Branch + "/" + commit.Hash + return client.Repos.CreateStatus(repo.Owner, repo.Name, status, buildUrl, message, commit.Hash) +} - return client.Repos.CreateStatus(repo.Owner, repo.Name, status, url, message, commit.Hash) +func getBuildUrl(host string, repo *Repo, commit *Commit) string { + branchQuery := url.Values{} + branchQuery.Set("branch", commit.Branch) + buildUrl := fmt.Sprintf("%s/%s/commit/%s?%s", host, repo.Slug, commit.Hash, branchQuery.Encode()) + return buildUrl } type bufferWrapper struct { diff --git a/pkg/queue/worker_test.go b/pkg/queue/worker_test.go new file mode 100644 index 00000000..ef6610be --- /dev/null +++ b/pkg/queue/worker_test.go @@ -0,0 +1,31 @@ +package queue + +import ( + . "github.com/drone/drone/pkg/model" + "testing" +) + +func Test_getBuildUrl(t *testing.T) { + repo := &Repo{ + Slug: "examplegit.com/owner/repo", + } + commit := &Commit{ + Hash: "abc", + Branch: "example", + } + + expected := "http://examplehost.com/examplegit.com/owner/repo/commit/abc?branch=example" + output := getBuildUrl("http://examplehost.com", repo, commit) + + if output != expected { + t.Errorf("Failed to build url. Expected: %s, got %s", expected, output) + } + + commit.Branch = "url/unsafe/branch" + expected = "http://examplehost.com/examplegit.com/owner/repo/commit/abc?branch=url%2Funsafe%2Fbranch" + output = getBuildUrl("http://examplehost.com", repo, commit) + + if output != expected { + t.Errorf("Failed to build url. Expected: %s, got %s", expected, output) + } +}