From c3ceee255154e11008527710f0ddd68280cd9084 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Wed, 20 Apr 2016 14:38:19 -0400 Subject: [PATCH 01/16] First part of auto merge and versioning --- .gitignore | 2 ++ api/repos.go | 11 ++++++ model/status_hook.go | 6 ++++ remote/github/github.go | 61 +++++++++++++++++++++++++++++--- remote/github/types.go | 38 ++++++++++++++------ remote/github/utils.go | 15 +++++++- remote/remote.go | 16 +++++++++ router/router.go | 1 + web/status_hook.go | 77 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 210 insertions(+), 17 deletions(-) create mode 100644 model/status_hook.go create mode 100644 web/status_hook.go diff --git a/.gitignore b/.gitignore index fea4936..4d670ec 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,5 @@ bindata.go web/react/node_modules web/static/files/script.js #web/static/files/*.css + +.idea \ No newline at end of file diff --git a/api/repos.go b/api/repos.go index 0a549fc..9f4c088 100644 --- a/api/repos.go +++ b/api/repos.go @@ -106,6 +106,17 @@ func PostRepo(c *gin.Context) { return } + statusLink := fmt.Sprintf( + "%s/status_hook?access_token=%s", + httputil.GetURL(c.Request), + sig, + ) + err = remote.SetStatusHook(c, user, repo, statusLink) + if err != nil { + c.String(500, "Error creating status hook. %s", err) + return + } + err = store.CreateRepo(c, repo) if err != nil { c.String(500, "Error activating the repository. %s", err) diff --git a/model/status_hook.go b/model/status_hook.go new file mode 100644 index 0000000..e508732 --- /dev/null +++ b/model/status_hook.go @@ -0,0 +1,6 @@ +package model + +type StatusHook struct { + Repo *Repo + +} diff --git a/remote/github/github.go b/remote/github/github.go index 65e3abe..a58c3bb 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -180,11 +180,6 @@ func (g *Github) GetRepos(u *model.User) ([]*model.Repo, error) { func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error { client := setupClient(g.API, user.Token) - repo_, _, err := client.Repositories.Get(repo.Owner, repo.Name) - if err != nil { - return err - } - old, err := GetHook(client, repo.Owner, repo.Name, link) if err == nil && old != nil { client.Repositories.DeleteHook(repo.Owner, repo.Name, *old.ID) @@ -196,6 +191,13 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error return err } + /* + Does not work with enterprise github version installed as of 4/20/2016 + repo_, _, err := client.Repositories.Get(repo.Owner, repo.Name) + if err != nil { + return err + } + in := new(Branch) in.Protection.Enabled = true in.Protection.Checks.Enforcement = "non_admins" @@ -209,6 +211,24 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error } log.Warnf("Error configuring protected branch for %s/%s@%s. %s", repo.Owner, repo.Name, *repo_.DefaultBranch, err) } + */ + return nil +} + +func (g *Github) SetStatusHook(user *model.User, repo *model.Repo, link string) error { + client := setupClient(g.API, user.Token) + + old, err := GetHook(client, repo.Owner, repo.Name, link) + if err == nil && old != nil { + client.Repositories.DeleteHook(repo.Owner, repo.Name, *old.ID) + } + + _, err = CreateStatusHook(client, repo.Owner, repo.Name, link) + if err != nil { + log.Debugf("Error creating the webhook at %s. %s", link, err) + return err + } + return nil } @@ -330,3 +350,34 @@ func (g *Github) GetHook(r *http.Request) (*model.Hook, error) { return hook, nil } + +func (g *Github) GetStatusHook(r *http.Request) (*model.StatusHook, error) { + + // only process comment hooks + if r.Header.Get("X-Github-Event") != "status" { + return nil, nil + } + + data := statusHook{} + err := json.NewDecoder(r.Body).Decode(&data) + if err != nil { + return nil, err + } + + log.Debug(data) + + if data.State != "success" { + return nil, nil + } + + + hook := new(model.StatusHook) + + hook.Repo = new(model.Repo) + hook.Repo.Owner = data.Repository.Owner.Login + hook.Repo.Name = data.Repository.Name + hook.Repo.Slug = data.Repository.FullName + + log.Debug(hook) + return hook, nil +} diff --git a/remote/github/types.go b/remote/github/types.go index 4f990e1..2835131 100644 --- a/remote/github/types.go +++ b/remote/github/types.go @@ -38,15 +38,31 @@ type commentHook struct { } `json:"user"` } `json:"comment"` - Repository struct { - Name string `json:"name"` - FullName string `json:"full_name"` - Desc string `json:"description"` - Private bool `json:"private"` - Owner struct { - Login string `json:"login"` - Type string `json:"type"` - Avatar string `json:"avatar_url"` - } `json:"owner"` - } `json:"repository"` + Repository Repository `json:"repository"` +} + +type statusHook struct { + State string `json:"state"` + + Branches []struct { + Name string `json:"name"` + Commit struct { + SHA string `json:"sha"` + URL string `json:"url"` + } `json:"commit"` + } `json:"branches"` + + Repository Repository `json:"repository"` +} + +type Repository struct { + Name string `json:"name"` + FullName string `json:"full_name"` + Desc string `json:"description"` + Private bool `json:"private"` + Owner struct { + Login string `json:"login"` + Type string `json:"type"` + Avatar string `json:"avatar_url"` + } `json:"owner"` } diff --git a/remote/github/utils.go b/remote/github/utils.go index d81823c..d1aae60 100644 --- a/remote/github/utils.go +++ b/remote/github/utils.go @@ -59,7 +59,7 @@ func DeleteHook(client *github.Client, owner, name, url string) error { return err } -// CreateHook is a heper function that creates a post-commit hook +// CreateHook is a helper function that creates a post-commit hook // for the specified repository. func CreateHook(client *github.Client, owner, name, url string) (*github.Hook, error) { var hook = new(github.Hook) @@ -72,6 +72,19 @@ func CreateHook(client *github.Client, owner, name, url string) (*github.Hook, e return created, err } +// CreateStatusHook is a helper function that creates a post-commit hook +// for the specified repository. +func CreateStatusHook(client *github.Client, owner, name, url string) (*github.Hook, error) { + var hook = new(github.Hook) + hook.Name = github.String("web") + hook.Events = []string{"status"} + hook.Config = map[string]interface{}{} + hook.Config["url"] = url + hook.Config["content_type"] = "json" + created, _, err := client.Repositories.CreateHook(owner, name, hook) + return created, err +} + // GetFile is a heper function that retrieves a file from // GitHub and returns its contents in byte array format. func GetFile(client *github.Client, owner, name, path, ref string) ([]byte, error) { diff --git a/remote/remote.go b/remote/remote.go index 77a0d8b..e91b96a 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -35,6 +35,9 @@ type Remote interface { // SetHook adds a webhook to the remote repository. SetHook(*model.User, *model.Repo, string) error + // SetStatusHook adds a webhook to the remote repository. + SetStatusHook(*model.User, *model.Repo, string) error + // DelHook deletes a webhook from the remote repository. DelHook(*model.User, *model.Repo, string) error @@ -49,6 +52,9 @@ type Remote interface { // GetHook gets the hook from the http Request. GetHook(r *http.Request) (*model.Hook, error) + + // GetHook gets the status hook from the http Request. + GetStatusHook(r *http.Request) (*model.StatusHook, error) } // GetUser authenticates a user with the remote system. @@ -102,6 +108,11 @@ func SetHook(c context.Context, u *model.User, r *model.Repo, hook string) error return FromContext(c).SetHook(u, r, hook) } +// SetHook adds a webhook to the remote repository. +func SetStatusHook(c context.Context, u *model.User, r *model.Repo, hook string) error { + return FromContext(c).SetStatusHook(u, r, hook) +} + // DelHook deletes a webhook from the remote repository. func DelHook(c context.Context, u *model.User, r *model.Repo, hook string) error { return FromContext(c).DelHook(u, r, hook) @@ -116,3 +127,8 @@ func SetStatus(c context.Context, u *model.User, r *model.Repo, num int, ok bool func GetHook(c context.Context, r *http.Request) (*model.Hook, error) { return FromContext(c).GetHook(r) } + +// GetStatusHook gets the status hook from the http Request. +func GetStatusHook(c context.Context, r *http.Request) (*model.StatusHook, error) { + return FromContext(c).GetStatusHook(r) +} diff --git a/router/router.go b/router/router.go index 9813ee7..da19681 100644 --- a/router/router.go +++ b/router/router.go @@ -36,6 +36,7 @@ func Load(middleware ...gin.HandlerFunc) http.Handler { e.GET("/api/repos/:owner/:repo/maintainers/:org", session.UserMust, access.RepoPull, api.GetMaintainerOrg) e.POST("/hook", web.Hook) + e.POST("/status_hook", web.StatusHook) e.GET("/login", web.Login) e.POST("/login", web.LoginToken) e.GET("/logout", web.Logout) diff --git a/web/status_hook.go b/web/status_hook.go new file mode 100644 index 0000000..8cd6f7b --- /dev/null +++ b/web/status_hook.go @@ -0,0 +1,77 @@ +package web + +import ( + "regexp" + + "github.com/lgtmco/lgtm/cache" + "github.com/lgtmco/lgtm/model" + "github.com/lgtmco/lgtm/remote" + "github.com/lgtmco/lgtm/store" + + log "github.com/Sirupsen/logrus" + "github.com/gin-gonic/gin" +) + +func StatusHook(c *gin.Context) { + hook, err := remote.GetStatusHook(c, c.Request) + if err != nil { + log.Errorf("Error parsing hook. %s", err) + c.String(500, "Error parsing hook. %s", err) + return + } + if hook == nil { + c.String(200, "pong") + return + } + + repo, err := store.GetRepoSlug(c, hook.Repo.Slug) + if err != nil { + log.Errorf("Error getting repository %s. %s", hook.Repo.Slug, err) + c.String(404, "Repository not found.") + return + } + user, err := store.GetUser(c, repo.UserID) + if err != nil { + log.Errorf("Error getting repository owner %s. %s", repo.Slug, err) + c.String(404, "Repository owner not found.") + return + } + + rcfile, _ := remote.GetContents(c, user, repo, ".lgtm") + config, err := model.ParseConfig(rcfile) + if err != nil { + log.Errorf("Error parsing .lgtm file for %s. %s", repo.Slug, err) + c.String(500, "Error parsing .lgtm file. %s.", err) + return + } + + //todo check the statuses of all of the checks on the branches for this commit + //todo if all of the statuses are success, then merge and create a tag for the version + //todo to create the version, need to scan the comments on the pull request to see if anyone specified a version # + //todo if so, use the largest specified version #. if not, increment the last version version # for the release + /* + comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) + if err != nil { + log.Errorf("Error retrieving comments for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) + c.String(500, "Error retrieving comments. %s.", err) + return + } + approvers := getApprovers(config, maintainer, hook.Issue, comments) + approved := len(approvers) >= config.Approvals + err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved) + if err != nil { + log.Errorf("Error setting status for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) + c.String(500, "Error setting status. %s.", err) + return + } + + */ + log.Debugf("processed status for %s. received %v ", repo.Slug, hook) + + c.IndentedJSON(200, gin.H{ + //"approvers": maintainer.People, + "settings": config, + //"approved": approved, + //"approved_by": approvers, + }) +} From f2b46bcbe0fc0fee89c024c05018b894deeee596 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Wed, 20 Apr 2016 14:51:34 -0400 Subject: [PATCH 02/16] Fix imports --- web/status_hook.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/web/status_hook.go b/web/status_hook.go index 8cd6f7b..3487499 100644 --- a/web/status_hook.go +++ b/web/status_hook.go @@ -1,9 +1,6 @@ package web import ( - "regexp" - - "github.com/lgtmco/lgtm/cache" "github.com/lgtmco/lgtm/model" "github.com/lgtmco/lgtm/remote" "github.com/lgtmco/lgtm/store" From a49a2aff6ee57c9314d901f3c50e6099f826fc42 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Wed, 20 Apr 2016 15:04:20 -0400 Subject: [PATCH 03/16] delete the status hook when removing the integration --- api/repos.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/api/repos.go b/api/repos.go index 81e03d0..d38ab24 100644 --- a/api/repos.go +++ b/api/repos.go @@ -152,5 +152,13 @@ func DeleteRepo(c *gin.Context) { if err != nil { logrus.Errorf("Error deleting repository hook for %s. %s", name, err) } + statusLink := fmt.Sprintf( + "%s/status_hook", + httputil.GetURL(c.Request), + ) + err = remote.DelHook(c, user, repo, statusLink) + if err != nil { + logrus.Errorf("Error deleting repository status hook for %s. %s", name, err) + } c.String(200, "") } From 31bb3ca92f17778d959520d5ff2953b9de9789fd Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Wed, 20 Apr 2016 15:21:17 -0400 Subject: [PATCH 04/16] go back to a single webhook, because that's all we get. --- api/repos.go | 19 ----------- remote/github/github.go | 17 ---------- remote/github/utils.go | 15 +-------- remote/remote.go | 8 ----- router/router.go | 1 - web/hook.go | 72 ++++++++++++++++++++++++++++++++++++++- web/status_hook.go | 74 ----------------------------------------- 7 files changed, 72 insertions(+), 134 deletions(-) delete mode 100644 web/status_hook.go diff --git a/api/repos.go b/api/repos.go index d38ab24..724a29d 100644 --- a/api/repos.go +++ b/api/repos.go @@ -106,17 +106,6 @@ func PostRepo(c *gin.Context) { return } - statusLink := fmt.Sprintf( - "%s/status_hook?access_token=%s", - httputil.GetURL(c.Request), - sig, - ) - err = remote.SetStatusHook(c, user, repo, statusLink) - if err != nil { - c.String(500, "Error creating status hook. %s", err) - return - } - err = store.CreateRepo(c, repo) if err != nil { c.String(500, "Error activating the repository. %s", err) @@ -152,13 +141,5 @@ func DeleteRepo(c *gin.Context) { if err != nil { logrus.Errorf("Error deleting repository hook for %s. %s", name, err) } - statusLink := fmt.Sprintf( - "%s/status_hook", - httputil.GetURL(c.Request), - ) - err = remote.DelHook(c, user, repo, statusLink) - if err != nil { - logrus.Errorf("Error deleting repository status hook for %s. %s", name, err) - } c.String(200, "") } diff --git a/remote/github/github.go b/remote/github/github.go index a58c3bb..2350dc1 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -215,23 +215,6 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error return nil } -func (g *Github) SetStatusHook(user *model.User, repo *model.Repo, link string) error { - client := setupClient(g.API, user.Token) - - old, err := GetHook(client, repo.Owner, repo.Name, link) - if err == nil && old != nil { - client.Repositories.DeleteHook(repo.Owner, repo.Name, *old.ID) - } - - _, err = CreateStatusHook(client, repo.Owner, repo.Name, link) - if err != nil { - log.Debugf("Error creating the webhook at %s. %s", link, err) - return err - } - - return nil -} - func (g *Github) DelHook(user *model.User, repo *model.Repo, link string) error { client := setupClient(g.API, user.Token) diff --git a/remote/github/utils.go b/remote/github/utils.go index d1aae60..279096e 100644 --- a/remote/github/utils.go +++ b/remote/github/utils.go @@ -64,20 +64,7 @@ func DeleteHook(client *github.Client, owner, name, url string) error { func CreateHook(client *github.Client, owner, name, url string) (*github.Hook, error) { var hook = new(github.Hook) hook.Name = github.String("web") - hook.Events = []string{"issue_comment"} - hook.Config = map[string]interface{}{} - hook.Config["url"] = url - hook.Config["content_type"] = "json" - created, _, err := client.Repositories.CreateHook(owner, name, hook) - return created, err -} - -// CreateStatusHook is a helper function that creates a post-commit hook -// for the specified repository. -func CreateStatusHook(client *github.Client, owner, name, url string) (*github.Hook, error) { - var hook = new(github.Hook) - hook.Name = github.String("web") - hook.Events = []string{"status"} + hook.Events = []string{"issue_comment", "status"} hook.Config = map[string]interface{}{} hook.Config["url"] = url hook.Config["content_type"] = "json" diff --git a/remote/remote.go b/remote/remote.go index e91b96a..8a5429a 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -35,9 +35,6 @@ type Remote interface { // SetHook adds a webhook to the remote repository. SetHook(*model.User, *model.Repo, string) error - // SetStatusHook adds a webhook to the remote repository. - SetStatusHook(*model.User, *model.Repo, string) error - // DelHook deletes a webhook from the remote repository. DelHook(*model.User, *model.Repo, string) error @@ -108,11 +105,6 @@ func SetHook(c context.Context, u *model.User, r *model.Repo, hook string) error return FromContext(c).SetHook(u, r, hook) } -// SetHook adds a webhook to the remote repository. -func SetStatusHook(c context.Context, u *model.User, r *model.Repo, hook string) error { - return FromContext(c).SetStatusHook(u, r, hook) -} - // DelHook deletes a webhook from the remote repository. func DelHook(c context.Context, u *model.User, r *model.Repo, hook string) error { return FromContext(c).DelHook(u, r, hook) diff --git a/router/router.go b/router/router.go index da19681..9813ee7 100644 --- a/router/router.go +++ b/router/router.go @@ -36,7 +36,6 @@ func Load(middleware ...gin.HandlerFunc) http.Handler { e.GET("/api/repos/:owner/:repo/maintainers/:org", session.UserMust, access.RepoPull, api.GetMaintainerOrg) e.POST("/hook", web.Hook) - e.POST("/status_hook", web.StatusHook) e.GET("/login", web.Login) e.POST("/login", web.LoginToken) e.GET("/logout", web.Logout) diff --git a/web/hook.go b/web/hook.go index 1378415..54cce12 100644 --- a/web/hook.go +++ b/web/hook.go @@ -19,11 +19,81 @@ func Hook(c *gin.Context) { c.String(500, "Error parsing hook. %s", err) return } - if hook == nil { + if hook != nil { + processCommentHook(c, hook) + } + + statusHook, err := remote.GetStatusHook(c, c.Request) + if err != nil { + log.Errorf("Error parsing status hook. %s", err) + c.String(500, "Error parsing status hook. %s", err) + return + } + + if statusHook == nil { c.String(200, "pong") return } + processStatusHook(c, statusHook) +} + +func processStatusHook(c *gin.Context, hook *model.StatusHook) { + repo, err := store.GetRepoSlug(c, hook.Repo.Slug) + if err != nil { + log.Errorf("Error getting repository %s. %s", hook.Repo.Slug, err) + c.String(404, "Repository not found.") + return + } + user, err := store.GetUser(c, repo.UserID) + if err != nil { + log.Errorf("Error getting repository owner %s. %s", repo.Slug, err) + c.String(404, "Repository owner not found.") + return + } + + rcfile, _ := remote.GetContents(c, user, repo, ".lgtm") + config, err := model.ParseConfig(rcfile) + if err != nil { + log.Errorf("Error parsing .lgtm file for %s. %s", repo.Slug, err) + c.String(500, "Error parsing .lgtm file. %s.", err) + return + } + + //todo check the statuses of all of the checks on the branches for this commit + //todo if all of the statuses are success, then merge and create a tag for the version + //todo to create the version, need to scan the comments on the pull request to see if anyone specified a version # + //todo if so, use the largest specified version #. if not, increment the last version version # for the release + /* + comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) + if err != nil { + log.Errorf("Error retrieving comments for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) + c.String(500, "Error retrieving comments. %s.", err) + return + } + approvers := getApprovers(config, maintainer, hook.Issue, comments) + approved := len(approvers) >= config.Approvals + err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved) + if err != nil { + log.Errorf("Error setting status for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) + c.String(500, "Error setting status. %s.", err) + return + } + + */ + log.Debugf("processed status for %s. received %v ", repo.Slug, hook) + + c.IndentedJSON(200, gin.H{ + //"approvers": maintainer.People, + "settings": config, + //"approved": approved, + //"approved_by": approvers, + }) +} + + +func processCommentHook(c *gin.Context, hook *model.Hook) { + repo, err := store.GetRepoSlug(c, hook.Repo.Slug) if err != nil { log.Errorf("Error getting repository %s. %s", hook.Repo.Slug, err) diff --git a/web/status_hook.go b/web/status_hook.go deleted file mode 100644 index 3487499..0000000 --- a/web/status_hook.go +++ /dev/null @@ -1,74 +0,0 @@ -package web - -import ( - "github.com/lgtmco/lgtm/model" - "github.com/lgtmco/lgtm/remote" - "github.com/lgtmco/lgtm/store" - - log "github.com/Sirupsen/logrus" - "github.com/gin-gonic/gin" -) - -func StatusHook(c *gin.Context) { - hook, err := remote.GetStatusHook(c, c.Request) - if err != nil { - log.Errorf("Error parsing hook. %s", err) - c.String(500, "Error parsing hook. %s", err) - return - } - if hook == nil { - c.String(200, "pong") - return - } - - repo, err := store.GetRepoSlug(c, hook.Repo.Slug) - if err != nil { - log.Errorf("Error getting repository %s. %s", hook.Repo.Slug, err) - c.String(404, "Repository not found.") - return - } - user, err := store.GetUser(c, repo.UserID) - if err != nil { - log.Errorf("Error getting repository owner %s. %s", repo.Slug, err) - c.String(404, "Repository owner not found.") - return - } - - rcfile, _ := remote.GetContents(c, user, repo, ".lgtm") - config, err := model.ParseConfig(rcfile) - if err != nil { - log.Errorf("Error parsing .lgtm file for %s. %s", repo.Slug, err) - c.String(500, "Error parsing .lgtm file. %s.", err) - return - } - - //todo check the statuses of all of the checks on the branches for this commit - //todo if all of the statuses are success, then merge and create a tag for the version - //todo to create the version, need to scan the comments on the pull request to see if anyone specified a version # - //todo if so, use the largest specified version #. if not, increment the last version version # for the release - /* - comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) - if err != nil { - log.Errorf("Error retrieving comments for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) - c.String(500, "Error retrieving comments. %s.", err) - return - } - approvers := getApprovers(config, maintainer, hook.Issue, comments) - approved := len(approvers) >= config.Approvals - err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved) - if err != nil { - log.Errorf("Error setting status for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) - c.String(500, "Error setting status. %s.", err) - return - } - - */ - log.Debugf("processed status for %s. received %v ", repo.Slug, hook) - - c.IndentedJSON(200, gin.H{ - //"approvers": maintainer.People, - "settings": config, - //"approved": approved, - //"approved_by": approvers, - }) -} From 51190daeb3f6c0a3b150ebbef80bfde4f5a91a74 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Wed, 20 Apr 2016 16:37:28 -0400 Subject: [PATCH 05/16] Add merging of branches with success status --- model/branch_status.go | 3 +++ model/status_hook.go | 2 +- remote/github/github.go | 34 +++++++++++++++++++++++++++++++++- remote/remote.go | 15 +++++++++++++++ web/hook.go | 24 ++++++++++++++++++++---- 5 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 model/branch_status.go diff --git a/model/branch_status.go b/model/branch_status.go new file mode 100644 index 0000000..51f3824 --- /dev/null +++ b/model/branch_status.go @@ -0,0 +1,3 @@ +package model + +type BranchStatus string \ No newline at end of file diff --git a/model/status_hook.go b/model/status_hook.go index e508732..270c6b7 100644 --- a/model/status_hook.go +++ b/model/status_hook.go @@ -2,5 +2,5 @@ package model type StatusHook struct { Repo *Repo - + Branches []string } diff --git a/remote/github/github.go b/remote/github/github.go index 2350dc1..9d8a158 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -361,6 +361,38 @@ func (g *Github) GetStatusHook(r *http.Request) (*model.StatusHook, error) { hook.Repo.Name = data.Repository.Name hook.Repo.Slug = data.Repository.FullName - log.Debug(hook) + hook.Branches = []string{} + for _, v := range data.Branches { + hook.Branches = append(hook.Branches, v.Name) + } + + log.Debug(*hook) return hook, nil } + +func (g *Github) GetBranchStatus(u *model.User, r *model.Repo, branch string) (*model.BranchStatus, error) { + client := setupClient(g.API, u.Token) + statuses, _, err := client.Repositories.GetCombinedStatus(r.Owner, r.Name, branch, nil) + if err != nil { + return nil, err + } + + return (*model.BranchStatus)(statuses.State), nil +} + +func (g *Github) MergeBranch(u *model.User, r *model.Repo, branch string) error { + client := setupClient(g.API, u.Token) + + repo_, _, err := client.Repositories.Get(r.Owner, r.Name) + if err != nil { + return err + } + + _, _, err = client.Repositories.Merge(r.Owner, r.Name, &github.RepositoryMergeRequest{ + Base: repo_.DefaultBranch, + Head: github.String(branch), + CommitMessage: github.String("Merged by LGTM"), + }) + return err +} + diff --git a/remote/remote.go b/remote/remote.go index 8a5429a..e0da2db 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -52,6 +52,12 @@ type Remote interface { // GetHook gets the status hook from the http Request. GetStatusHook(r *http.Request) (*model.StatusHook, error) + + // GetBranchStatus returns overall status for the named branch from the remote system + GetBranchStatus(*model.User, *model.Repo, string) (*model.BranchStatus, error) + + // MergeBranch merges the named branch from the remote system + MergeBranch(u *model.User, r *model.Repo, branch string) error } // GetUser authenticates a user with the remote system. @@ -124,3 +130,12 @@ func GetHook(c context.Context, r *http.Request) (*model.Hook, error) { func GetStatusHook(c context.Context, r *http.Request) (*model.StatusHook, error) { return FromContext(c).GetStatusHook(r) } + +// GetBranchStatus gets the overal status for a branch from the remote repository. +func GetBranchStatus(c context.Context, u *model.User, r *model.Repo, branch string) (*model.BranchStatus, error) { + return FromContext(c).GetBranchStatus(u, r, branch) +} + +func MergeBranch(c context.Context, u *model.User, r *model.Repo, branch string) error { + return FromContext(c).MergeBranch(u, r, branch) +} \ No newline at end of file diff --git a/web/hook.go b/web/hook.go index 54cce12..98d78ba 100644 --- a/web/hook.go +++ b/web/hook.go @@ -60,10 +60,26 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { return } - //todo check the statuses of all of the checks on the branches for this commit - //todo if all of the statuses are success, then merge and create a tag for the version - //todo to create the version, need to scan the comments on the pull request to see if anyone specified a version # - //todo if so, use the largest specified version #. if not, increment the last version version # for the release + //check the statuses of all of the checks on the branches for this commit + for _, v := range hook.Branches { + b, err := remote.GetBranchStatus(c, user, hook.Repo, v) + if err != nil { + log.Warnf("Unable to process branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + continue + } + //if all of the statuses are success, then merge and create a tag for the version + if *b == "success" { + err = remote.MergeBranch(c, user, hook.Repo, v) + if err != nil { + log.Warnf("Unable to merge branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + } else { + log.Debugf("Merged branch %s/%s/%s",hook.Repo.Owner, hook.Repo.Name, v) + } + } + //todo to create the version, need to scan the comments on the pull request to see if anyone specified a version # + //todo if so, use the largest specified version #. if not, increment the last version version # for the release + + } /* comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) if err != nil { From 5e2214c5c8a1b2274aaeb706f7508f9e41805eed Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 09:31:18 -0400 Subject: [PATCH 06/16] Add property support for managing versioning and merging. Add initial versioning support. --- model/config.go | 6 ++-- model/tag.go | 3 ++ remote/github/github.go | 69 +++++++++++++++++++++++++++++++++++++++-- remote/remote.go | 19 ++++++++++-- web/hook.go | 46 ++++++++++++++++++++++----- 5 files changed, 128 insertions(+), 15 deletions(-) create mode 100644 model/tag.go diff --git a/model/config.go b/model/config.go index a4c2599..41fc740 100644 --- a/model/config.go +++ b/model/config.go @@ -11,8 +11,10 @@ type Config struct { Pattern string `json:"pattern" toml:"pattern"` Team string `json:"team" toml:"team"` SelfApprovalOff bool `json:"self_approval_off" toml:"self_approval_off"` + DoMerge bool `json:"do_merge" toml:"do_merge"` + DoVersion bool `json:"do_version" toml:"do_version"` - re *regexp.Regexp + re *regexp.Regexp } // ParseConfig parses a projects .lgtm file @@ -31,7 +33,7 @@ func ParseConfigStr(data string) (*Config, error) { c.Approvals = 2 } if len(c.Pattern) == 0 { - c.Pattern = "(?i)LGTM" + c.Pattern = `(?i)LGTM\s*(\S*)` } if len(c.Team) == 0 { c.Team = "MAINTAINERS" diff --git a/model/tag.go b/model/tag.go new file mode 100644 index 0000000..cf4b1a0 --- /dev/null +++ b/model/tag.go @@ -0,0 +1,3 @@ +package model + +type Tag string diff --git a/remote/github/github.go b/remote/github/github.go index 9d8a158..4fcfd6f 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -12,6 +12,7 @@ import ( "github.com/lgtmco/lgtm/model" "github.com/lgtmco/lgtm/shared/httputil" "golang.org/x/oauth2" + "github.com/hashicorp/go-version" ) // name of the status message posted to GitHub @@ -380,7 +381,7 @@ func (g *Github) GetBranchStatus(u *model.User, r *model.Repo, branch string) (* return (*model.BranchStatus)(statuses.State), nil } -func (g *Github) MergeBranch(u *model.User, r *model.Repo, branch string) error { +func (g *Github) MergeBranch(u *model.User, r *model.Repo, branch string) (*string, error) { client := setupClient(g.API, u.Token) repo_, _, err := client.Repositories.Get(r.Owner, r.Name) @@ -388,11 +389,73 @@ func (g *Github) MergeBranch(u *model.User, r *model.Repo, branch string) error return err } - _, _, err = client.Repositories.Merge(r.Owner, r.Name, &github.RepositoryMergeRequest{ + result, _, err := client.Repositories.Merge(r.Owner, r.Name, &github.RepositoryMergeRequest{ Base: repo_.DefaultBranch, Head: github.String(branch), CommitMessage: github.String("Merged by LGTM"), }) - return err + + if err != nil { + return nil, err + } + return result.SHA, err } +func (g *Github) GetMaxExistingTag(u *model.User, r *model.Repo) (*version.Version, error) { + client := setupClient(g.API, u.Token) + + //find the previous largest semver value + var maxVer *version.Version + + tags, _, err := client.Repositories.ListTags(r.Owner, r.Name, nil) + if err != nil { + return nil, err + } + for _, v := range tags { + curVer, err := version.NewVersion(*v.Name) + if err != nil { + continue + } + if maxVer == nil || curVer.GreaterThan(maxVer) { + maxVer = curVer + } + } + + if maxVer == nil { + maxVer, _ = version.NewVersion("v0.0.0") + } + log.Debugf("maxVer found is %s", maxVer.String()) + return maxVer, nil +} + +func (g *Github) Tag(u *model.User, r *model.Repo, version *version.Version, sha *string) error { + client := setupClient(g.API, u.Token) + + t := time.Now() + tag, _, err := client.Git.CreateTag(r.Owner, r.Name, &github.Tag{ + Tag: github.String(version.String()), + SHA: sha, + Message: github.String("Tagged by Shazbot"), + Tagger: &github.CommitAuthor{ + Date: &t, + Name: github.String("Shazbot"), + Email: github.String("shazbot@capitalone.com"), + }, + Object: &github.GitObject{ + SHA: sha, + Type: github.String("commit"), + }, + }) + + if err != nil { + return err + } + _, _, err = client.Git.CreateRef(r.Owner, r.Name, &github.Reference{ + Ref: github.String("refs/tags/" + version.String()), + Object: &github.GitObject{ + SHA: tag.SHA, + }, + }) + + return err +} diff --git a/remote/remote.go b/remote/remote.go index e0da2db..2a6bc2c 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -7,6 +7,7 @@ import ( "github.com/lgtmco/lgtm/model" "golang.org/x/net/context" + "github.com/hashicorp/go-version" ) type Remote interface { @@ -57,7 +58,13 @@ type Remote interface { GetBranchStatus(*model.User, *model.Repo, string) (*model.BranchStatus, error) // MergeBranch merges the named branch from the remote system - MergeBranch(u *model.User, r *model.Repo, branch string) error + MergeBranch(u *model.User, r *model.Repo, branch string) (*string, error) + + // GetMaxExistingTag finds the highest version across all tags + GetMaxExistingTag(u *model.User, r *model.Repo) (*version.Version, error) + + // Tag applies a tag with the specified version to the specified sha + Tag(u *model.User, r *model.Repo, version *version.Version, sha *string) error } // GetUser authenticates a user with the remote system. @@ -136,6 +143,14 @@ func GetBranchStatus(c context.Context, u *model.User, r *model.Repo, branch str return FromContext(c).GetBranchStatus(u, r, branch) } -func MergeBranch(c context.Context, u *model.User, r *model.Repo, branch string) error { +func MergeBranch(c context.Context, u *model.User, r *model.Repo, branch string) (*string, error) { return FromContext(c).MergeBranch(u, r, branch) +} + +func GetMaxExistingTag(c context.Context, u *model.User, r *model.Repo) (*version.Version, error) { + return FromContext(c).GetMaxExistingTag(u, r) +} + +func Tag(c context.Context, u *model.User, r *model.Repo, version *version.Version, sha *string) error { + return FromContext(c).Tag(u, r, version, sha) } \ No newline at end of file diff --git a/web/hook.go b/web/hook.go index 98d78ba..d90e12f 100644 --- a/web/hook.go +++ b/web/hook.go @@ -10,6 +10,8 @@ import ( log "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" + "github.com/hashicorp/go-version" + "fmt" ) func Hook(c *gin.Context) { @@ -60,6 +62,15 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { return } + if !config.DoMerge { + c.IndentedJSON(200, gin.H{ + }) + return + } + + merged := map[string]string{} + vers := map[string]string{} + //check the statuses of all of the checks on the branches for this commit for _, v := range hook.Branches { b, err := remote.GetBranchStatus(c, user, hook.Repo, v) @@ -69,16 +80,37 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { } //if all of the statuses are success, then merge and create a tag for the version if *b == "success" { - err = remote.MergeBranch(c, user, hook.Repo, v) + sha, err := remote.MergeBranch(c, user, hook.Repo, v) if err != nil { log.Warnf("Unable to merge branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + continue } else { log.Debugf("Merged branch %s/%s/%s",hook.Repo.Owner, hook.Repo.Name, v) } - } - //todo to create the version, need to scan the comments on the pull request to see if anyone specified a version # - //todo if so, use the largest specified version #. if not, increment the last version version # for the release + merged[v] = sha + + if !config.DoVersion { + continue + } + + // to create the version, need to scan the comments on the pull request to see if anyone specified a version # + // if so, use the largest specified version #. if not, increment the last version version # for the release + maxVer, err := remote.GetMaxExistingTag(c, user, hook.Repo) + if err != nil { + log.Warnf("Unable to find the max version tag for branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + continue + } + maxParts := maxVer.Segments() + //todo scan comments for version info + curVersion, _ := version.NewVersion(fmt.Sprintf("%d.%d.%d",maxParts[0],maxParts[1],maxParts[2]+1)) + err = remote.Tag(c, user, repo, curVersion, sha) + if err != nil { + log.Warnf("Unable to tag branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + continue + } + vers[v] = curVersion.String() + } } /* comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) @@ -100,10 +132,8 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { log.Debugf("processed status for %s. received %v ", repo.Slug, hook) c.IndentedJSON(200, gin.H{ - //"approvers": maintainer.People, - "settings": config, - //"approved": approved, - //"approved_by": approvers, + "merged": merged, + "versions": vers, }) } From 249b0d3da22f8162a2e7277ae224fcfa1e721d99 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 10:18:47 -0400 Subject: [PATCH 07/16] Fix bug in LGTM that wipes out existing contexts when LGTM context is added to protected branch. --- remote/github/github.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/remote/github/github.go b/remote/github/github.go index 65e3abe..496cb34 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -199,9 +199,16 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error in := new(Branch) in.Protection.Enabled = true in.Protection.Checks.Enforcement = "non_admins" - in.Protection.Checks.Contexts = []string{context} + /* + JCB 04/21/16 confirmed with Github support -- must specify all existing contexts when + adding a new one, otherwise the other contexts will be removed. + */ client_ := NewClientToken(g.API, user.Token) + branch, _ := client_.Branch(repo.Owner, repo.Name, *repo_.DefaultBranch) + + in.Protection.Checks.Contexts = append(buildOtherContextSlice(branch), context) + err = client_.BranchProtect(repo.Owner, repo.Name, *repo_.DefaultBranch, in) if err != nil { if g.URL == "https://github.com" { @@ -236,14 +243,21 @@ func (g *Github) DelHook(user *model.User, repo *model.Repo, link string) error if len(branch.Protection.Checks.Contexts) == 0 { return nil } + + branch.Protection.Checks.Contexts = buildOtherContextSlice(branch) + + return client_.BranchProtect(repo.Owner, repo.Name, *repo_.DefaultBranch, branch) +} + +// buildOtherContextSlice returns all contexts besides the one for LGTM +func buildOtherContextSlice(branch *Branch) []string { checks := []string{} for _, check := range branch.Protection.Checks.Contexts { if check != context { checks = append(checks, check) } } - branch.Protection.Checks.Contexts = checks - return client_.BranchProtect(repo.Owner, repo.Name, *repo_.DefaultBranch, branch) + return checks } func (g *Github) GetComments(u *model.User, r *model.Repo, num int) ([]*model.Comment, error) { From e6dd9254d0140a42ae981f2ed9faf07ba98460bc Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 11:17:10 -0400 Subject: [PATCH 08/16] Merge status check fix and fix compilation issues. --- remote/github/github.go | 5 +---- web/hook.go | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/remote/github/github.go b/remote/github/github.go index 65c0039..5914743 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -192,8 +192,6 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error return err } - /* - Does not work with enterprise github version installed as of 4/20/2016 repo_, _, err := client.Repositories.Get(repo.Owner, repo.Name) if err != nil { return err @@ -219,7 +217,6 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error } log.Warnf("Error configuring protected branch for %s/%s@%s. %s", repo.Owner, repo.Name, *repo_.DefaultBranch, err) } - */ return nil } @@ -400,7 +397,7 @@ func (g *Github) MergeBranch(u *model.User, r *model.Repo, branch string) (*stri repo_, _, err := client.Repositories.Get(r.Owner, r.Name) if err != nil { - return err + return nil, err } result, _, err := client.Repositories.Merge(r.Owner, r.Name, &github.RepositoryMergeRequest{ diff --git a/web/hook.go b/web/hook.go index d90e12f..599aa6a 100644 --- a/web/hook.go +++ b/web/hook.go @@ -88,7 +88,7 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { log.Debugf("Merged branch %s/%s/%s",hook.Repo.Owner, hook.Repo.Name, v) } - merged[v] = sha + merged[v] = *sha if !config.DoVersion { continue From 6b31038398a9d7d5d31124dc9f3a61e2b2bc3b12 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 14:48:12 -0400 Subject: [PATCH 09/16] Version support in comments added. --- model/status_hook.go | 13 ++- remote/github/github.go | 55 ++++++++---- remote/github/types.go | 1 + remote/remote.go | 18 ++-- web/hook.go | 186 ++++++++++++++++++++++++++-------------- 5 files changed, 187 insertions(+), 86 deletions(-) diff --git a/model/status_hook.go b/model/status_hook.go index 270c6b7..4e707d5 100644 --- a/model/status_hook.go +++ b/model/status_hook.go @@ -1,6 +1,17 @@ package model type StatusHook struct { + SHA string Repo *Repo - Branches []string +} + +type PullRequest struct { + Issue + Branch Branch +} + +type Branch struct { + Name string + BranchStatus string + Mergeable bool } diff --git a/remote/github/github.go b/remote/github/github.go index 5914743..49b8011 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -13,6 +13,7 @@ import ( "github.com/lgtmco/lgtm/shared/httputil" "golang.org/x/oauth2" "github.com/hashicorp/go-version" + "errors" ) // name of the status message posted to GitHub @@ -365,23 +366,49 @@ func (g *Github) GetStatusHook(r *http.Request) (*model.StatusHook, error) { return nil, nil } - hook := new(model.StatusHook) + hook.SHA = data.SHA + hook.Repo = new(model.Repo) hook.Repo.Owner = data.Repository.Owner.Login hook.Repo.Name = data.Repository.Name hook.Repo.Slug = data.Repository.FullName - hook.Branches = []string{} - for _, v := range data.Branches { - hook.Branches = append(hook.Branches, v.Name) - } - log.Debug(*hook) + return hook, nil } +func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { + client := setupClient(g.API, u.Token) + issues, _, err := client.Search.Issues(fmt.Sprintf("%s&type=pr", *sha), nil) + if err != nil { + return nil, err + } + out := make([]model.PullRequest, len(issues.Issues)) + for k, v := range issues.Issues { + pr, _, err := client.PullRequests.Get(r.Owner, r.Name, *v.Number) + if err != nil { + return nil, err + } + out[k] = model.PullRequest{ + Issue: model.Issue{ + Number: *v.Number, + Title: *v.Title, + Author: *v.User.Login, + }, + Branch: model.Branch { + Name: *pr.Head.Ref, + BranchStatus: *pr.State, + Mergeable: *pr.Mergeable, + + }, + } + } + return out, nil +} + func (g *Github) GetBranchStatus(u *model.User, r *model.Repo, branch string) (*model.BranchStatus, error) { client := setupClient(g.API, u.Token) statuses, _, err := client.Repositories.GetCombinedStatus(r.Owner, r.Name, branch, nil) @@ -392,24 +419,18 @@ func (g *Github) GetBranchStatus(u *model.User, r *model.Repo, branch string) (* return (*model.BranchStatus)(statuses.State), nil } -func (g *Github) MergeBranch(u *model.User, r *model.Repo, branch string) (*string, error) { +func (g *Github) MergePR(u *model.User, r *model.Repo, pullRequest model.PullRequest) (*string, error) { client := setupClient(g.API, u.Token) - repo_, _, err := client.Repositories.Get(r.Owner, r.Name) + result, _, err := client.PullRequests.Merge(r.Owner, r.Name, pullRequest.Number, "Merged by LGTM") if err != nil { return nil, err } - result, _, err := client.Repositories.Merge(r.Owner, r.Name, &github.RepositoryMergeRequest{ - Base: repo_.DefaultBranch, - Head: github.String(branch), - CommitMessage: github.String("Merged by LGTM"), - }) - - if err != nil { - return nil, err + if !(*result.Merged) { + return nil, errors.New(*result.Message) } - return result.SHA, err + return result.SHA, nil } func (g *Github) GetMaxExistingTag(u *model.User, r *model.Repo) (*version.Version, error) { diff --git a/remote/github/types.go b/remote/github/types.go index 2835131..d116298 100644 --- a/remote/github/types.go +++ b/remote/github/types.go @@ -42,6 +42,7 @@ type commentHook struct { } type statusHook struct { + SHA string `json:"sha"` State string `json:"state"` Branches []struct { diff --git a/remote/remote.go b/remote/remote.go index 2a6bc2c..50c1d8e 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -51,20 +51,23 @@ type Remote interface { // GetHook gets the hook from the http Request. GetHook(r *http.Request) (*model.Hook, error) - // GetHook gets the status hook from the http Request. + // GetStatusHook gets the status hook from the http Request. GetStatusHook(r *http.Request) (*model.StatusHook, error) // GetBranchStatus returns overall status for the named branch from the remote system GetBranchStatus(*model.User, *model.Repo, string) (*model.BranchStatus, error) - // MergeBranch merges the named branch from the remote system - MergeBranch(u *model.User, r *model.Repo, branch string) (*string, error) + // MergePR merges the named pull request from the remote system + MergePR(u *model.User, r *model.Repo, pullRequest model.PullRequest) (*string, error) // GetMaxExistingTag finds the highest version across all tags GetMaxExistingTag(u *model.User, r *model.Repo) (*version.Version, error) // Tag applies a tag with the specified version to the specified sha Tag(u *model.User, r *model.Repo, version *version.Version, sha *string) error + + // GetPullRequestsForCommit returns all pull requests associated with a commit SHA + GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) } // GetUser authenticates a user with the remote system. @@ -143,8 +146,8 @@ func GetBranchStatus(c context.Context, u *model.User, r *model.Repo, branch str return FromContext(c).GetBranchStatus(u, r, branch) } -func MergeBranch(c context.Context, u *model.User, r *model.Repo, branch string) (*string, error) { - return FromContext(c).MergeBranch(u, r, branch) +func MergePR(c context.Context, u *model.User, r *model.Repo, pullRequest model.PullRequest) (*string, error) { + return FromContext(c).MergePR(u, r, pullRequest) } func GetMaxExistingTag(c context.Context, u *model.User, r *model.Repo) (*version.Version, error) { @@ -153,4 +156,9 @@ func GetMaxExistingTag(c context.Context, u *model.User, r *model.Repo) (*versio func Tag(c context.Context, u *model.User, r *model.Repo, version *version.Version, sha *string) error { return FromContext(c).Tag(u, r, version, sha) +} + +func GetPullRequestsForCommit(c context.Context, u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { + return FromContext(c).GetPullRequestsForCommit(u, r, sha) + } \ No newline at end of file diff --git a/web/hook.go b/web/hook.go index 599aa6a..6800477 100644 --- a/web/hook.go +++ b/web/hook.go @@ -54,11 +54,8 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { return } - rcfile, _ := remote.GetContents(c, user, repo, ".lgtm") - config, err := model.ParseConfig(rcfile) + config, maintainer, err := getConfigAndMaintainers(c, user, repo) if err != nil { - log.Errorf("Error parsing .lgtm file for %s. %s", repo.Slug, err) - c.String(500, "Error parsing .lgtm file. %s.", err) return } @@ -71,24 +68,25 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { merged := map[string]string{} vers := map[string]string{} + pullRequests, err := remote.GetPullRequestsForCommit(c, user, hook.Repo, &hook.SHA) + if err != nil { + log.Errorf("Error while getting pull requests for commit %s %s", hook.SHA, err) + c.String(500, "Error while getting pull requests for commit %s %s", hook.SHA, err) + return + } //check the statuses of all of the checks on the branches for this commit - for _, v := range hook.Branches { - b, err := remote.GetBranchStatus(c, user, hook.Repo, v) - if err != nil { - log.Warnf("Unable to process branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) - continue - } + for _, v := range pullRequests { //if all of the statuses are success, then merge and create a tag for the version - if *b == "success" { - sha, err := remote.MergeBranch(c, user, hook.Repo, v) + if v.Branch.BranchStatus == "success" && v.Branch.Mergeable { + sha, err := remote.MergePR(c, user, hook.Repo, v) if err != nil { - log.Warnf("Unable to merge branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + log.Warnf("Unable to merge pull request %s: %s", v.Title, err) continue } else { - log.Debugf("Merged branch %s/%s/%s",hook.Repo.Owner, hook.Repo.Name, v) + log.Debugf("Merged pull request %s", v.Title) } - merged[v] = *sha + merged[v.Title] = *sha if !config.DoVersion { continue @@ -98,37 +96,33 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { // if so, use the largest specified version #. if not, increment the last version version # for the release maxVer, err := remote.GetMaxExistingTag(c, user, hook.Repo) if err != nil { - log.Warnf("Unable to find the max version tag for branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + log.Warnf("Unable to find the max version tag for %s/%s: %s", hook.Repo.Owner, hook.Repo.Name, err) continue } - maxParts := maxVer.Segments() - //todo scan comments for version info - curVersion, _ := version.NewVersion(fmt.Sprintf("%d.%d.%d",maxParts[0],maxParts[1],maxParts[2]+1)) - err = remote.Tag(c, user, repo, curVersion, sha) + + comments, err := getComments(c, user, repo, v.Number) if err != nil { - log.Warnf("Unable to tag branch %s/%s/%s: %s",hook.Repo.Owner, hook.Repo.Name, v, err) + log.Warnf("Unable to find the comments for pull request %s: %s", v.Title, err) continue } - vers[v] = curVersion.String() + + foundVersion := getMaxVersionComment(config, maintainer, v.Issue,comments) + + if foundVersion != nil && foundVersion.GreaterThan(maxVer) { + maxVer = foundVersion + } else { + maxParts := maxVer.Segments() + maxVer, _ = version.NewVersion(fmt.Sprintf("%d.%d.%d", maxParts[0], maxParts[1], maxParts[2] + 1)) + } + + err = remote.Tag(c, user, repo, maxVer, sha) + if err != nil { + log.Warnf("Unable to tag branch %s: %s", v.Title, err) + continue + } + vers[v.Title] = maxVer.String() } } - /* - comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) - if err != nil { - log.Errorf("Error retrieving comments for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) - c.String(500, "Error retrieving comments. %s.", err) - return - } - approvers := getApprovers(config, maintainer, hook.Issue, comments) - approved := len(approvers) >= config.Approvals - err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved) - if err != nil { - log.Errorf("Error setting status for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) - c.String(500, "Error setting status. %s.", err) - return - } - - */ log.Debugf("processed status for %s. received %v ", repo.Slug, hook) c.IndentedJSON(200, gin.H{ @@ -137,7 +131,6 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { }) } - func processCommentHook(c *gin.Context, hook *model.Hook) { repo, err := store.GetRepoSlug(c, hook.Repo.Slug) @@ -153,12 +146,42 @@ func processCommentHook(c *gin.Context, hook *model.Hook) { return } + config, maintainer, err := getConfigAndMaintainers(c, user, repo) + if err != nil { + return + } + + comments, err := getComments(c, user, repo, hook.Issue.Number) + if err != nil { + return + } + + approvers := getApprovers(config, maintainer, hook.Issue, comments) + approved := len(approvers) >= config.Approvals + err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved) + if err != nil { + log.Errorf("Error setting status for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) + c.String(500, "Error setting status. %s.", err) + return + } + + log.Debugf("processed comment for %s. received %d of %d approvals", repo.Slug, len(approvers), config.Approvals) + + c.IndentedJSON(200, gin.H{ + "approvers": maintainer.People, + "settings": config, + "approved": approved, + "approved_by": approvers, + }) +} + +func getConfigAndMaintainers(c *gin.Context, user *model.User, repo *model.Repo) (*model.Config, *model.Maintainer, error) { rcfile, _ := remote.GetContents(c, user, repo, ".lgtm") config, err := model.ParseConfig(rcfile) if err != nil { log.Errorf("Error parsing .lgtm file for %s. %s", repo.Slug, err) c.String(500, "Error parsing .lgtm file. %s.", err) - return + return nil, nil, err } // THIS IS COMPLETELY DUPLICATED IN THE API SECTION. NOT IDEAL @@ -170,7 +193,7 @@ func processCommentHook(c *gin.Context, hook *model.Hook) { log.Errorf("Error getting repository %s. %s", repo.Slug, err) log.Errorf("Error getting org members %s. %s", repo.Owner, merr) c.String(404, "MAINTAINERS file not found. %s", err) - return + return nil, nil, err } else { for _, member := range members { file = append(file, member.Login...) @@ -183,32 +206,19 @@ func processCommentHook(c *gin.Context, hook *model.Hook) { if err != nil { log.Errorf("Error parsing MAINTAINERS file for %s. %s", repo.Slug, err) c.String(500, "Error parsing MAINTAINERS file. %s.", err) - return + return nil, nil, err } + return config, maintainer, nil +} - comments, err := remote.GetComments(c, user, repo, hook.Issue.Number) +func getComments(c *gin.Context, user *model.User, repo *model.Repo, num int) ([]*model.Comment, error) { + comments, err := remote.GetComments(c, user, repo, num) if err != nil { - log.Errorf("Error retrieving comments for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) + log.Errorf("Error retrieving comments for %s pr %d. %s", repo.Slug, num, err) c.String(500, "Error retrieving comments. %s.", err) - return - } - approvers := getApprovers(config, maintainer, hook.Issue, comments) - approved := len(approvers) >= config.Approvals - err = remote.SetStatus(c, user, repo, hook.Issue.Number, approved) - if err != nil { - log.Errorf("Error setting status for %s pr %d. %s", repo.Slug, hook.Issue.Number, err) - c.String(500, "Error setting status. %s.", err) - return + return nil, err } - - log.Debugf("processed comment for %s. received %d of %d approvals", repo.Slug, len(approvers), config.Approvals) - - c.IndentedJSON(200, gin.H{ - "approvers": maintainer.People, - "settings": config, - "approved": approved, - "approved_by": approvers, - }) + return comments, nil } // getApprovers is a helper function that analyzes the list of comments @@ -246,3 +256,53 @@ func getApprovers(config *model.Config, maintainer *model.Maintainer, issue *mod return approvers } + +// getMaxVersionComment is a helper function that analyzes the list of comments +// and returns the maximum version found in a comment. +func getMaxVersionComment(config *model.Config, maintainer *model.Maintainer, issue model.Issue, comments []*model.Comment) *version.Version { + approverm := map[string]bool{} + approvers := []*model.Person{} + + matcher, err := regexp.Compile(config.Pattern) + if err != nil { + // this should never happen + return nil + } + + var maxVersion *version.Version + + for _, comment := range comments { + // cannot lgtm your own pull request + if config.SelfApprovalOff && comment.Author == issue.Author { + continue + } + // the user must be a valid maintainer of the project + person, ok := maintainer.People[comment.Author] + if !ok { + continue + } + // the same author can't approve something twice + if _, ok := approverm[comment.Author]; ok { + continue + } + // verify the comment matches the approval pattern + m := matcher.FindStringSubmatch(comment.Body) + if len(m) > 0 { + approverm[comment.Author] = true + approvers = append(approvers, person) + + if len(m) > 1 { + //has a version + curVersion, err := version.NewVersion(m[1]) + if err != nil { + continue + } + if maxVersion == nil || curVersion.GreaterThan(maxVersion) { + maxVersion = curVersion + } + } + } + } + + return maxVersion +} From 18cc9977b53d17265117c4d6741217d8394b6508 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 20:05:18 +0000 Subject: [PATCH 10/16] Put in some fixes for getting pull requests via search --- remote/github/github.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/remote/github/github.go b/remote/github/github.go index 49b8011..f626679 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -382,7 +382,10 @@ func (g *Github) GetStatusHook(r *http.Request) (*model.StatusHook, error) { func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { client := setupClient(g.API, u.Token) - issues, _, err := client.Search.Issues(fmt.Sprintf("%s&type=pr", *sha), nil) + fmt.Println("sha == ", sha, *sha) + issues, _, err := client.Search.Issues(fmt.Sprintf("%s&type=pr", *sha), &github.SearchOptions { + TextMatch: false, + }) if err != nil { return nil, err } @@ -392,6 +395,11 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str if err != nil { return nil, err } + + mergeable:= true + if pr.Mergeable != nil { + mergeable = *pr.Mergeable + } out[k] = model.PullRequest{ Issue: model.Issue{ Number: *v.Number, @@ -401,7 +409,7 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str Branch: model.Branch { Name: *pr.Head.Ref, BranchStatus: *pr.State, - Mergeable: *pr.Mergeable, + Mergeable: mergeable, }, } From 0da11a3f2be3eaf1082dd148ade14c95032e4158 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 16:11:56 -0400 Subject: [PATCH 11/16] Get the combined status for a branch. Also be sure that the pong messageis only returned when there is no hook at all --- remote/github/github.go | 8 +++++++- web/hook.go | 4 +++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/remote/github/github.go b/remote/github/github.go index f626679..e4c4abb 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -400,6 +400,12 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str if pr.Mergeable != nil { mergeable = *pr.Mergeable } + + status, _, err := client.Repositories.GetCombinedStatus(r.Owner, r.Name, *sha, nil) + if err != nil { + return nil, err + } + out[k] = model.PullRequest{ Issue: model.Issue{ Number: *v.Number, @@ -408,7 +414,7 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str }, Branch: model.Branch { Name: *pr.Head.Ref, - BranchStatus: *pr.State, + BranchStatus: *status.State, Mergeable: mergeable, }, diff --git a/web/hook.go b/web/hook.go index 6800477..2357bf1 100644 --- a/web/hook.go +++ b/web/hook.go @@ -32,7 +32,7 @@ func Hook(c *gin.Context) { return } - if statusHook == nil { + if hook == nil && statusHook == nil { c.String(200, "pong") return } @@ -69,6 +69,8 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { vers := map[string]string{} pullRequests, err := remote.GetPullRequestsForCommit(c, user, hook.Repo, &hook.SHA) + log.Debugf("sha for commit is %s, pull requests are: %s", hook.SHA, pullRequests) + if err != nil { log.Errorf("Error while getting pull requests for commit %s %s", hook.SHA, err) c.String(500, "Error while getting pull requests for commit %s %s", hook.SHA, err) From f2f12e25bcd5a1d7fe8619987019dfb5447e601e Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 16:40:40 -0400 Subject: [PATCH 12/16] Stop trying to run the status hook when a comment hook was received --- web/hook.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/web/hook.go b/web/hook.go index 2357bf1..eae5b19 100644 --- a/web/hook.go +++ b/web/hook.go @@ -31,13 +31,14 @@ func Hook(c *gin.Context) { c.String(500, "Error parsing status hook. %s", err) return } + if statusHook != nil { + processStatusHook(c, statusHook) + } if hook == nil && statusHook == nil { c.String(200, "pong") return } - - processStatusHook(c, statusHook) } func processStatusHook(c *gin.Context, hook *model.StatusHook) { From 1d407bd89936ae18c73dec4c3bac50efb4e53942 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Thu, 21 Apr 2016 16:46:47 -0400 Subject: [PATCH 13/16] Remove shazbot references. --- remote/github/github.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/remote/github/github.go b/remote/github/github.go index e4c4abb..bb9094b 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -481,11 +481,11 @@ func (g *Github) Tag(u *model.User, r *model.Repo, version *version.Version, sha tag, _, err := client.Git.CreateTag(r.Owner, r.Name, &github.Tag{ Tag: github.String(version.String()), SHA: sha, - Message: github.String("Tagged by Shazbot"), + Message: github.String("Tagged by LGTM"), Tagger: &github.CommitAuthor{ Date: &t, - Name: github.String("Shazbot"), - Email: github.String("shazbot@capitalone.com"), + Name: github.String("LGTM"), + Email: github.String("LGTM@lgtm.co"), }, Object: &github.GitObject{ SHA: sha, From 88f076bea65db04a78cd46f715e4a098bde45a82 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Sat, 9 Jul 2016 22:14:53 -0400 Subject: [PATCH 14/16] go fmt code --- model/branch_status.go | 2 +- model/config.go | 6 +++--- model/status_hook.go | 4 ++-- remote/github/github.go | 19 +++++++++---------- remote/github/types.go | 16 ++++++++-------- remote/remote.go | 4 ++-- web/hook.go | 13 ++++++------- 7 files changed, 31 insertions(+), 33 deletions(-) diff --git a/model/branch_status.go b/model/branch_status.go index 51f3824..452f937 100644 --- a/model/branch_status.go +++ b/model/branch_status.go @@ -1,3 +1,3 @@ package model -type BranchStatus string \ No newline at end of file +type BranchStatus string diff --git a/model/config.go b/model/config.go index 41fc740..756c106 100644 --- a/model/config.go +++ b/model/config.go @@ -11,10 +11,10 @@ type Config struct { Pattern string `json:"pattern" toml:"pattern"` Team string `json:"team" toml:"team"` SelfApprovalOff bool `json:"self_approval_off" toml:"self_approval_off"` - DoMerge bool `json:"do_merge" toml:"do_merge"` - DoVersion bool `json:"do_version" toml:"do_version"` + DoMerge bool `json:"do_merge" toml:"do_merge"` + DoVersion bool `json:"do_version" toml:"do_version"` - re *regexp.Regexp + re *regexp.Regexp } // ParseConfig parses a projects .lgtm file diff --git a/model/status_hook.go b/model/status_hook.go index 4e707d5..31ca850 100644 --- a/model/status_hook.go +++ b/model/status_hook.go @@ -1,8 +1,8 @@ package model type StatusHook struct { - SHA string - Repo *Repo + SHA string + Repo *Repo } type PullRequest struct { diff --git a/remote/github/github.go b/remote/github/github.go index bb9094b..024d277 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -7,13 +7,13 @@ import ( "strings" "time" + "errors" log "github.com/Sirupsen/logrus" "github.com/google/go-github/github" + "github.com/hashicorp/go-version" "github.com/lgtmco/lgtm/model" "github.com/lgtmco/lgtm/shared/httputil" "golang.org/x/oauth2" - "github.com/hashicorp/go-version" - "errors" ) // name of the status message posted to GitHub @@ -205,7 +205,7 @@ func (g *Github) SetHook(user *model.User, repo *model.Repo, link string) error /* JCB 04/21/16 confirmed with Github support -- must specify all existing contexts when adding a new one, otherwise the other contexts will be removed. - */ + */ client_ := NewClientToken(g.API, user.Token) branch, _ := client_.Branch(repo.Owner, repo.Name, *repo_.DefaultBranch) @@ -383,7 +383,7 @@ func (g *Github) GetStatusHook(r *http.Request) (*model.StatusHook, error) { func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { client := setupClient(g.API, u.Token) fmt.Println("sha == ", sha, *sha) - issues, _, err := client.Search.Issues(fmt.Sprintf("%s&type=pr", *sha), &github.SearchOptions { + issues, _, err := client.Search.Issues(fmt.Sprintf("%s&type=pr", *sha), &github.SearchOptions{ TextMatch: false, }) if err != nil { @@ -396,7 +396,7 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str return nil, err } - mergeable:= true + mergeable := true if pr.Mergeable != nil { mergeable = *pr.Mergeable } @@ -409,14 +409,13 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str out[k] = model.PullRequest{ Issue: model.Issue{ Number: *v.Number, - Title: *v.Title, + Title: *v.Title, Author: *v.User.Login, }, - Branch: model.Branch { - Name: *pr.Head.Ref, + Branch: model.Branch{ + Name: *pr.Head.Ref, BranchStatus: *status.State, - Mergeable: mergeable, - + Mergeable: mergeable, }, } } diff --git a/remote/github/types.go b/remote/github/types.go index d116298..608fc5e 100644 --- a/remote/github/types.go +++ b/remote/github/types.go @@ -42,28 +42,28 @@ type commentHook struct { } type statusHook struct { - SHA string `json:"sha"` + SHA string `json:"sha"` State string `json:"state"` Branches []struct { - Name string `json:"name"` + Name string `json:"name"` Commit struct { SHA string `json:"sha"` URL string `json:"url"` - } `json:"commit"` + } `json:"commit"` } `json:"branches"` Repository Repository `json:"repository"` } -type Repository struct { +type Repository struct { Name string `json:"name"` FullName string `json:"full_name"` Desc string `json:"description"` Private bool `json:"private"` Owner struct { - Login string `json:"login"` - Type string `json:"type"` - Avatar string `json:"avatar_url"` - } `json:"owner"` + Login string `json:"login"` + Type string `json:"type"` + Avatar string `json:"avatar_url"` + } `json:"owner"` } diff --git a/remote/remote.go b/remote/remote.go index 50c1d8e..0f85f04 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -5,9 +5,9 @@ package remote import ( "net/http" + "github.com/hashicorp/go-version" "github.com/lgtmco/lgtm/model" "golang.org/x/net/context" - "github.com/hashicorp/go-version" ) type Remote interface { @@ -161,4 +161,4 @@ func Tag(c context.Context, u *model.User, r *model.Repo, version *version.Versi func GetPullRequestsForCommit(c context.Context, u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { return FromContext(c).GetPullRequestsForCommit(u, r, sha) -} \ No newline at end of file +} diff --git a/web/hook.go b/web/hook.go index eae5b19..7aacc9c 100644 --- a/web/hook.go +++ b/web/hook.go @@ -8,10 +8,10 @@ import ( "github.com/lgtmco/lgtm/remote" "github.com/lgtmco/lgtm/store" + "fmt" log "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" "github.com/hashicorp/go-version" - "fmt" ) func Hook(c *gin.Context) { @@ -31,7 +31,7 @@ func Hook(c *gin.Context) { c.String(500, "Error parsing status hook. %s", err) return } - if statusHook != nil { + if statusHook != nil { processStatusHook(c, statusHook) } @@ -61,8 +61,7 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { } if !config.DoMerge { - c.IndentedJSON(200, gin.H{ - }) + c.IndentedJSON(200, gin.H{}) return } @@ -109,13 +108,13 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { continue } - foundVersion := getMaxVersionComment(config, maintainer, v.Issue,comments) + foundVersion := getMaxVersionComment(config, maintainer, v.Issue, comments) if foundVersion != nil && foundVersion.GreaterThan(maxVer) { maxVer = foundVersion } else { maxParts := maxVer.Segments() - maxVer, _ = version.NewVersion(fmt.Sprintf("%d.%d.%d", maxParts[0], maxParts[1], maxParts[2] + 1)) + maxVer, _ = version.NewVersion(fmt.Sprintf("%d.%d.%d", maxParts[0], maxParts[1], maxParts[2]+1)) } err = remote.Tag(c, user, repo, maxVer, sha) @@ -129,7 +128,7 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { log.Debugf("processed status for %s. received %v ", repo.Slug, hook) c.IndentedJSON(200, gin.H{ - "merged": merged, + "merged": merged, "versions": vers, }) } From ccd9950029b0c7198f201cddd4373220d9d3fc33 Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Sat, 9 Jul 2016 22:18:05 -0400 Subject: [PATCH 15/16] rename model.Branch.BranchStatus to model.Branch.Status --- model/status_hook.go | 6 +++--- remote/github/github.go | 6 +++--- web/hook.go | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/model/status_hook.go b/model/status_hook.go index 31ca850..f7ea9c9 100644 --- a/model/status_hook.go +++ b/model/status_hook.go @@ -11,7 +11,7 @@ type PullRequest struct { } type Branch struct { - Name string - BranchStatus string - Mergeable bool + Name string + Status string + Mergeable bool } diff --git a/remote/github/github.go b/remote/github/github.go index 024d277..7a19628 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -413,9 +413,9 @@ func (g *Github) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *str Author: *v.User.Login, }, Branch: model.Branch{ - Name: *pr.Head.Ref, - BranchStatus: *status.State, - Mergeable: mergeable, + Name: *pr.Head.Ref, + Status: *status.State, + Mergeable: mergeable, }, } } diff --git a/web/hook.go b/web/hook.go index 7aacc9c..4782c3e 100644 --- a/web/hook.go +++ b/web/hook.go @@ -12,6 +12,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" "github.com/hashicorp/go-version" + "github.com/lgtmco/lgtm/version" ) func Hook(c *gin.Context) { @@ -79,7 +80,7 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { //check the statuses of all of the checks on the branches for this commit for _, v := range pullRequests { //if all of the statuses are success, then merge and create a tag for the version - if v.Branch.BranchStatus == "success" && v.Branch.Mergeable { + if v.Branch.Status == "success" && v.Branch.Mergeable { sha, err := remote.MergePR(c, user, hook.Repo, v) if err != nil { log.Warnf("Unable to merge pull request %s: %s", v.Title, err) From 34ccb739fd412796cf4a7d1fbb927f1035454e8c Mon Sep 17 00:00:00 2001 From: Jon Bodner Date: Sat, 9 Jul 2016 23:10:07 -0400 Subject: [PATCH 16/16] Removed GetStatusHook from remote API and merged it with GetHook. Removed GetMaxExistingTag from remote API and put in GetTags instead. Added GetTags to API. Added GetMaxTag method to new TagList type. --- api/tags.go | 37 +++++ cache/helper.go | 19 +++ model/hook.go | 6 + model/tag.go | 30 ++++ remote/github/github.go | 56 ++++--- remote/mock/remote.go | 318 ++++++++++++++++++++++++++++++---------- remote/remote.go | 17 +-- router/router.go | 1 + store/mock/store.go | 151 +++++++++++-------- web/hook.go | 31 ++-- 10 files changed, 460 insertions(+), 206 deletions(-) create mode 100644 api/tags.go diff --git a/api/tags.go b/api/tags.go new file mode 100644 index 0000000..b8fcf40 --- /dev/null +++ b/api/tags.go @@ -0,0 +1,37 @@ +package api + +import ( + log "github.com/Sirupsen/logrus" + "github.com/gin-gonic/gin" + "github.com/lgtmco/lgtm/cache" + "github.com/lgtmco/lgtm/model" + "github.com/lgtmco/lgtm/router/middleware/session" + "github.com/lgtmco/lgtm/store" +) + +func GetTags(c *gin.Context) { + var ( + owner = c.Param("owner") + name = c.Param("repo") + user = session.User(c) + ) + repo, err := store.GetRepoOwnerName(c, owner, name) + if err != nil { + log.Errorf("Error getting repository %s. %s", name, err) + c.AbortWithStatus(404) + return + } + tags, err := cache.GetTags(c, user, repo) + if err != nil { + log.Errorf("Error getting remote tag list. %s", err) + c.String(500, "Error getting remote tag list") + return + } + + // copy the slice since we don't + // want any nasty data races if the slice came from the cache. + tagsc := make(model.TagList, len(tags)) + copy(tagsc, tags) + + c.JSON(200, tagsc) +} diff --git a/cache/helper.go b/cache/helper.go index 7cee5d1..95d6d0d 100644 --- a/cache/helper.go +++ b/cache/helper.go @@ -93,3 +93,22 @@ func GetMembers(c context.Context, user *model.User, team string) ([]*model.Memb FromContext(c).Set(key, members) return members, nil } + +func GetTags(c context.Context, user *model.User, repo *model.Repo) (model.TagList, error) { + key := fmt.Sprintf("tags:%s", + user.Login, + ) + // if we fetch from the cache we can return immediately + val, err := FromContext(c).Get(key) + if err == nil { + return val.(model.TagList), nil + } + // else we try to grab from the remote system and + // populate our cache. + tags, err := remote.GetTagList(c, user, repo) + if err != nil { + return nil, err + } + FromContext(c).Set(key, tags) + return tags, nil +} diff --git a/model/hook.go b/model/hook.go index 2a171f9..01a0ad1 100644 --- a/model/hook.go +++ b/model/hook.go @@ -1,6 +1,12 @@ package model type Hook struct { + Kind string + IssueComment *IssueCommentHook + Status *StatusHook +} + +type IssueCommentHook struct { Repo *Repo Issue *Issue Comment *Comment diff --git a/model/tag.go b/model/tag.go index cf4b1a0..89b7ee2 100644 --- a/model/tag.go +++ b/model/tag.go @@ -1,3 +1,33 @@ package model +import ( + log "github.com/Sirupsen/logrus" + "github.com/hashicorp/go-version" +) + type Tag string + +type TagList []Tag + +func (tl TagList) GetMaxTag() (Tag, *version.Version) { + //find the previous largest semver value + var maxVer *version.Version + var maxTag Tag + + for _, tag := range tl { + curVer, err := version.NewVersion(string(tag)) + if err != nil { + continue + } + if maxVer == nil || curVer.GreaterThan(maxVer) { + maxVer = curVer + maxTag = tag + } + } + + if maxVer == nil { + maxVer, _ = version.NewVersion("v0.0.0") + } + log.Debugf("maxVer found is %s", maxVer.String()) + return maxTag, maxVer +} diff --git a/remote/github/github.go b/remote/github/github.go index 7a19628..ed67b0e 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -316,12 +316,27 @@ func (g *Github) SetStatus(u *model.User, r *model.Repo, num int, ok bool) error } func (g *Github) GetHook(r *http.Request) (*model.Hook, error) { - - // only process comment hooks - if r.Header.Get("X-Github-Event") != "issue_comment" { - return nil, nil + hook := &model.Hook{} + kind := r.Header.Get("X-Github-Event") + hook.Kind = kind + switch kind { + case "issue_comment": + issueComment, err := processIssueCommentHook(r) + if err != nil { + return nil, err + } + hook.IssueComment = issueComment + case "status": + status, err := processStatusHook(r) + if err != nil { + return nil, err + } + hook.Status = status } + return hook, nil +} +func processIssueCommentHook(r *http.Request) (*model.IssueCommentHook, error) { data := commentHook{} err := json.NewDecoder(r.Body).Decode(&data) if err != nil { @@ -332,7 +347,7 @@ func (g *Github) GetHook(r *http.Request) (*model.Hook, error) { return nil, nil } - hook := new(model.Hook) + hook := new(model.IssueCommentHook) hook.Issue = new(model.Issue) hook.Issue.Number = data.Issue.Number hook.Issue.Author = data.Issue.User.Login @@ -347,13 +362,7 @@ func (g *Github) GetHook(r *http.Request) (*model.Hook, error) { return hook, nil } -func (g *Github) GetStatusHook(r *http.Request) (*model.StatusHook, error) { - - // only process comment hooks - if r.Header.Get("X-Github-Event") != "status" { - return nil, nil - } - +func processStatusHook(r *http.Request) (*model.StatusHook, error) { data := statusHook{} err := json.NewDecoder(r.Body).Decode(&data) if err != nil { @@ -446,31 +455,18 @@ func (g *Github) MergePR(u *model.User, r *model.Repo, pullRequest model.PullReq return result.SHA, nil } -func (g *Github) GetMaxExistingTag(u *model.User, r *model.Repo) (*version.Version, error) { +func (g *Github) GetTagList(u *model.User, r *model.Repo) (model.TagList, error) { client := setupClient(g.API, u.Token) - //find the previous largest semver value - var maxVer *version.Version - tags, _, err := client.Repositories.ListTags(r.Owner, r.Name, nil) if err != nil { return nil, err } - for _, v := range tags { - curVer, err := version.NewVersion(*v.Name) - if err != nil { - continue - } - if maxVer == nil || curVer.GreaterThan(maxVer) { - maxVer = curVer - } + out := make(model.TagList, len(tags)) + for k, v := range tags { + out[k] = model.Tag(*v.Name) } - - if maxVer == nil { - maxVer, _ = version.NewVersion("v0.0.0") - } - log.Debugf("maxVer found is %s", maxVer.String()) - return maxVer, nil + return out, nil } func (g *Github) Tag(u *model.User, r *model.Repo, version *version.Version, sha *string) error { diff --git a/remote/mock/remote.go b/remote/mock/remote.go index 98d361f..ef999c7 100644 --- a/remote/mock/remote.go +++ b/remote/mock/remote.go @@ -1,75 +1,125 @@ package mock -import "github.com/stretchr/testify/mock" +import ( + "net/http" -import "net/http" -import "github.com/lgtmco/lgtm/model" + "github.com/hashicorp/go-version" + "github.com/lgtmco/lgtm/model" + "github.com/stretchr/testify/mock" +) +// Remote is an autogenerated mock type for the Remote type type Remote struct { mock.Mock } -func (_m *Remote) GetUser(_a0 http.ResponseWriter, _a1 *http.Request) (*model.User, error) { - ret := _m.Called(_a0, _a1) +// DelHook provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) DelHook(_a0 *model.User, _a1 *model.Repo, _a2 string) error { + ret := _m.Called(_a0, _a1, _a2) - var r0 *model.User - if rf, ok := ret.Get(0).(func(http.ResponseWriter, *http.Request) *model.User); ok { - r0 = rf(_a0, _a1) + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) error); ok { + r0 = rf(_a0, _a1, _a2) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// GetBranchStatus provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) GetBranchStatus(_a0 *model.User, _a1 *model.Repo, _a2 string) (*model.BranchStatus, error) { + ret := _m.Called(_a0, _a1, _a2) + + var r0 *model.BranchStatus + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) *model.BranchStatus); ok { + r0 = rf(_a0, _a1, _a2) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.User) + r0 = ret.Get(0).(*model.BranchStatus) } } var r1 error - if rf, ok := ret.Get(1).(func(http.ResponseWriter, *http.Request) error); ok { - r1 = rf(_a0, _a1) + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, string) error); ok { + r1 = rf(_a0, _a1, _a2) } else { r1 = ret.Error(1) } return r0, r1 } -func (_m *Remote) GetUserToken(_a0 string) (string, error) { - ret := _m.Called(_a0) - var r0 string - if rf, ok := ret.Get(0).(func(string) string); ok { - r0 = rf(_a0) +// GetComments provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) GetComments(_a0 *model.User, _a1 *model.Repo, _a2 int) ([]*model.Comment, error) { + ret := _m.Called(_a0, _a1, _a2) + + var r0 []*model.Comment + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, int) []*model.Comment); ok { + r0 = rf(_a0, _a1, _a2) } else { - r0 = ret.Get(0).(string) + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.Comment) + } } var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(_a0) + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, int) error); ok { + r1 = rf(_a0, _a1, _a2) } else { r1 = ret.Error(1) } return r0, r1 } -func (_m *Remote) GetTeams(_a0 *model.User) ([]*model.Team, error) { - ret := _m.Called(_a0) - var r0 []*model.Team - if rf, ok := ret.Get(0).(func(*model.User) []*model.Team); ok { - r0 = rf(_a0) +// GetContents provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) GetContents(_a0 *model.User, _a1 *model.Repo, _a2 string) ([]byte, error) { + ret := _m.Called(_a0, _a1, _a2) + + var r0 []byte + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) []byte); ok { + r0 = rf(_a0, _a1, _a2) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Team) + r0 = ret.Get(0).([]byte) } } var r1 error - if rf, ok := ret.Get(1).(func(*model.User) error); ok { - r1 = rf(_a0) + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, string) error); ok { + r1 = rf(_a0, _a1, _a2) } else { r1 = ret.Error(1) } return r0, r1 } + +// GetHook provides a mock function with given fields: r +func (_m *Remote) GetHook(r *http.Request) (*model.Hook, error) { + ret := _m.Called(r) + + var r0 *model.Hook + if rf, ok := ret.Get(0).(func(*http.Request) *model.Hook); ok { + r0 = rf(r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Hook) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*http.Request) error); ok { + r1 = rf(r) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetMembers provides a mock function with given fields: _a0, _a1 func (_m *Remote) GetMembers(_a0 *model.User, _a1 string) ([]*model.Member, error) { ret := _m.Called(_a0, _a1) @@ -91,15 +141,17 @@ func (_m *Remote) GetMembers(_a0 *model.User, _a1 string) ([]*model.Member, erro return r0, r1 } -func (_m *Remote) GetRepo(_a0 *model.User, _a1 string, _a2 string) (*model.Repo, error) { + +// GetPerm provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) GetPerm(_a0 *model.User, _a1 string, _a2 string) (*model.Perm, error) { ret := _m.Called(_a0, _a1, _a2) - var r0 *model.Repo - if rf, ok := ret.Get(0).(func(*model.User, string, string) *model.Repo); ok { + var r0 *model.Perm + if rf, ok := ret.Get(0).(func(*model.User, string, string) *model.Perm); ok { r0 = rf(_a0, _a1, _a2) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Repo) + r0 = ret.Get(0).(*model.Perm) } } @@ -112,15 +164,40 @@ func (_m *Remote) GetRepo(_a0 *model.User, _a1 string, _a2 string) (*model.Repo, return r0, r1 } -func (_m *Remote) GetPerm(_a0 *model.User, _a1 string, _a2 string) (*model.Perm, error) { + +// GetPullRequestsForCommit provides a mock function with given fields: u, r, sha +func (_m *Remote) GetPullRequestsForCommit(u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { + ret := _m.Called(u, r, sha) + + var r0 []model.PullRequest + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *string) []model.PullRequest); ok { + r0 = rf(u, r, sha) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]model.PullRequest) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, *string) error); ok { + r1 = rf(u, r, sha) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetRepo provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) GetRepo(_a0 *model.User, _a1 string, _a2 string) (*model.Repo, error) { ret := _m.Called(_a0, _a1, _a2) - var r0 *model.Perm - if rf, ok := ret.Get(0).(func(*model.User, string, string) *model.Perm); ok { + var r0 *model.Repo + if rf, ok := ret.Get(0).(func(*model.User, string, string) *model.Repo); ok { r0 = rf(_a0, _a1, _a2) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Perm) + r0 = ret.Get(0).(*model.Repo) } } @@ -133,6 +210,8 @@ func (_m *Remote) GetPerm(_a0 *model.User, _a1 string, _a2 string) (*model.Perm, return r0, r1 } + +// GetRepos provides a mock function with given fields: _a0 func (_m *Remote) GetRepos(_a0 *model.User) ([]*model.Repo, error) { ret := _m.Called(_a0) @@ -154,102 +233,181 @@ func (_m *Remote) GetRepos(_a0 *model.User) ([]*model.Repo, error) { return r0, r1 } -func (_m *Remote) SetHook(_a0 *model.User, _a1 *model.Repo, _a2 string) error { - ret := _m.Called(_a0, _a1, _a2) - var r0 error - if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) error); ok { - r0 = rf(_a0, _a1, _a2) +// GetStatusHook provides a mock function with given fields: r +func (_m *Remote) GetStatusHook(r *http.Request) (*model.StatusHook, error) { + ret := _m.Called(r) + + var r0 *model.StatusHook + if rf, ok := ret.Get(0).(func(*http.Request) *model.StatusHook); ok { + r0 = rf(r) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.StatusHook) + } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(*http.Request) error); ok { + r1 = rf(r) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } -func (_m *Remote) DelHook(_a0 *model.User, _a1 *model.Repo, _a2 string) error { - ret := _m.Called(_a0, _a1, _a2) - var r0 error - if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) error); ok { - r0 = rf(_a0, _a1, _a2) +// GetTagList provides a mock function with given fields: u, r +func (_m *Remote) GetTagList(u *model.User, r *model.Repo) (model.TagList, error) { + ret := _m.Called(u, r) + + var r0 model.TagList + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo) model.TagList); ok { + r0 = rf(u, r) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(model.TagList) + } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo) error); ok { + r1 = rf(u, r) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } -func (_m *Remote) GetComments(_a0 *model.User, _a1 *model.Repo, _a2 int) ([]*model.Comment, error) { - ret := _m.Called(_a0, _a1, _a2) - var r0 []*model.Comment - if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, int) []*model.Comment); ok { - r0 = rf(_a0, _a1, _a2) +// GetTeams provides a mock function with given fields: _a0 +func (_m *Remote) GetTeams(_a0 *model.User) ([]*model.Team, error) { + ret := _m.Called(_a0) + + var r0 []*model.Team + if rf, ok := ret.Get(0).(func(*model.User) []*model.Team); ok { + r0 = rf(_a0) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Comment) + r0 = ret.Get(0).([]*model.Team) } } var r1 error - if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, int) error); ok { - r1 = rf(_a0, _a1, _a2) + if rf, ok := ret.Get(1).(func(*model.User) error); ok { + r1 = rf(_a0) } else { r1 = ret.Error(1) } return r0, r1 } -func (_m *Remote) GetContents(_a0 *model.User, _a1 *model.Repo, _a2 string) ([]byte, error) { - ret := _m.Called(_a0, _a1, _a2) - var r0 []byte - if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) []byte); ok { - r0 = rf(_a0, _a1, _a2) +// GetUser provides a mock function with given fields: _a0, _a1 +func (_m *Remote) GetUser(_a0 http.ResponseWriter, _a1 *http.Request) (*model.User, error) { + ret := _m.Called(_a0, _a1) + + var r0 *model.User + if rf, ok := ret.Get(0).(func(http.ResponseWriter, *http.Request) *model.User); ok { + r0 = rf(_a0, _a1) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]byte) + r0 = ret.Get(0).(*model.User) } } var r1 error - if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, string) error); ok { - r1 = rf(_a0, _a1, _a2) + if rf, ok := ret.Get(1).(func(http.ResponseWriter, *http.Request) error); ok { + r1 = rf(_a0, _a1) } else { r1 = ret.Error(1) } return r0, r1 } -func (_m *Remote) SetStatus(_a0 *model.User, _a1 *model.Repo, _a2 int, _a3 bool) error { - ret := _m.Called(_a0, _a1, _a2, _a3) - var r0 error - if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, int, bool) error); ok { - r0 = rf(_a0, _a1, _a2, _a3) +// GetUserToken provides a mock function with given fields: _a0 +func (_m *Remote) GetUserToken(_a0 string) (string, error) { + ret := _m.Called(_a0) + + var r0 string + if rf, ok := ret.Get(0).(func(string) string); ok { + r0 = rf(_a0) } else { - r0 = ret.Error(0) + r0 = ret.Get(0).(string) } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(_a0) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } -func (_m *Remote) GetHook(r *http.Request) (*model.Hook, error) { - ret := _m.Called(r) - var r0 *model.Hook - if rf, ok := ret.Get(0).(func(*http.Request) *model.Hook); ok { - r0 = rf(r) +// MergePR provides a mock function with given fields: u, r, pullRequest +func (_m *Remote) MergePR(u *model.User, r *model.Repo, pullRequest model.PullRequest) (*string, error) { + ret := _m.Called(u, r, pullRequest) + + var r0 *string + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, model.PullRequest) *string); ok { + r0 = rf(u, r, pullRequest) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Hook) + r0 = ret.Get(0).(*string) } } var r1 error - if rf, ok := ret.Get(1).(func(*http.Request) error); ok { - r1 = rf(r) + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, model.PullRequest) error); ok { + r1 = rf(u, r, pullRequest) } else { r1 = ret.Error(1) } return r0, r1 } + +// SetHook provides a mock function with given fields: _a0, _a1, _a2 +func (_m *Remote) SetHook(_a0 *model.User, _a1 *model.Repo, _a2 string) error { + ret := _m.Called(_a0, _a1, _a2) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) error); ok { + r0 = rf(_a0, _a1, _a2) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// SetStatus provides a mock function with given fields: _a0, _a1, _a2, _a3 +func (_m *Remote) SetStatus(_a0 *model.User, _a1 *model.Repo, _a2 int, _a3 bool) error { + ret := _m.Called(_a0, _a1, _a2, _a3) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, int, bool) error); ok { + r0 = rf(_a0, _a1, _a2, _a3) + } else { + r0 = ret.Error(0) + } + + return r0 +} + +// Tag provides a mock function with given fields: u, r, _a2, sha +func (_m *Remote) Tag(u *model.User, r *model.Repo, _a2 *version.Version, sha *string) error { + ret := _m.Called(u, r, _a2, sha) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *version.Version, *string) error); ok { + r0 = rf(u, r, _a2, sha) + } else { + r0 = ret.Error(0) + } + + return r0 +} diff --git a/remote/remote.go b/remote/remote.go index 0f85f04..1ccfa86 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -51,17 +51,14 @@ type Remote interface { // GetHook gets the hook from the http Request. GetHook(r *http.Request) (*model.Hook, error) - // GetStatusHook gets the status hook from the http Request. - GetStatusHook(r *http.Request) (*model.StatusHook, error) - // GetBranchStatus returns overall status for the named branch from the remote system GetBranchStatus(*model.User, *model.Repo, string) (*model.BranchStatus, error) // MergePR merges the named pull request from the remote system MergePR(u *model.User, r *model.Repo, pullRequest model.PullRequest) (*string, error) - // GetMaxExistingTag finds the highest version across all tags - GetMaxExistingTag(u *model.User, r *model.Repo) (*version.Version, error) + // GetTagList gets all tags in the repo + GetTagList(u *model.User, r *model.Repo) (model.TagList, error) // Tag applies a tag with the specified version to the specified sha Tag(u *model.User, r *model.Repo, version *version.Version, sha *string) error @@ -136,11 +133,6 @@ func GetHook(c context.Context, r *http.Request) (*model.Hook, error) { return FromContext(c).GetHook(r) } -// GetStatusHook gets the status hook from the http Request. -func GetStatusHook(c context.Context, r *http.Request) (*model.StatusHook, error) { - return FromContext(c).GetStatusHook(r) -} - // GetBranchStatus gets the overal status for a branch from the remote repository. func GetBranchStatus(c context.Context, u *model.User, r *model.Repo, branch string) (*model.BranchStatus, error) { return FromContext(c).GetBranchStatus(u, r, branch) @@ -150,8 +142,8 @@ func MergePR(c context.Context, u *model.User, r *model.Repo, pullRequest model. return FromContext(c).MergePR(u, r, pullRequest) } -func GetMaxExistingTag(c context.Context, u *model.User, r *model.Repo) (*version.Version, error) { - return FromContext(c).GetMaxExistingTag(u, r) +func GetTagList(c context.Context, u *model.User, r *model.Repo) (model.TagList, error) { + return FromContext(c).GetTagList(u, r) } func Tag(c context.Context, u *model.User, r *model.Repo, version *version.Version, sha *string) error { @@ -160,5 +152,4 @@ func Tag(c context.Context, u *model.User, r *model.Repo, version *version.Versi func GetPullRequestsForCommit(c context.Context, u *model.User, r *model.Repo, sha *string) ([]model.PullRequest, error) { return FromContext(c).GetPullRequestsForCommit(u, r, sha) - } diff --git a/router/router.go b/router/router.go index 9813ee7..3d2c3b6 100644 --- a/router/router.go +++ b/router/router.go @@ -34,6 +34,7 @@ func Load(middleware ...gin.HandlerFunc) http.Handler { e.DELETE("/api/repos/:owner/:repo", session.UserMust, access.RepoAdmin, api.DeleteRepo) e.GET("/api/repos/:owner/:repo/maintainers", session.UserMust, access.RepoPull, api.GetMaintainer) e.GET("/api/repos/:owner/:repo/maintainers/:org", session.UserMust, access.RepoPull, api.GetMaintainerOrg) + e.GET("/api/repos/:owner/:repo/tags", session.UserMust, access.RepoPull, api.GetTags) e.POST("/hook", web.Hook) e.GET("/login", web.Login) diff --git a/store/mock/store.go b/store/mock/store.go index 506f50e..6a3d49e 100644 --- a/store/mock/store.go +++ b/store/mock/store.go @@ -1,55 +1,30 @@ -package mock +package mocks -import "github.com/stretchr/testify/mock" - -import "github.com/lgtmco/lgtm/model" +import ( + "github.com/lgtmco/lgtm/model" + "github.com/stretchr/testify/mock" +) +// Store is an autogenerated mock type for the Store type type Store struct { mock.Mock } -func (_m *Store) GetUser(_a0 int64) (*model.User, error) { +// CreateRepo provides a mock function with given fields: _a0 +func (_m *Store) CreateRepo(_a0 *model.Repo) error { ret := _m.Called(_a0) - var r0 *model.User - if rf, ok := ret.Get(0).(func(int64) *model.User); ok { + var r0 error + if rf, ok := ret.Get(0).(func(*model.Repo) error); ok { r0 = rf(_a0) } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.User) - } - } - - var r1 error - if rf, ok := ret.Get(1).(func(int64) error); ok { - r1 = rf(_a0) - } else { - r1 = ret.Error(1) + r0 = ret.Error(0) } - return r0, r1 + return r0 } -func (_m *Store) GetUserLogin(_a0 string) (*model.User, error) { - ret := _m.Called(_a0) - - var r0 *model.User - if rf, ok := ret.Get(0).(func(string) *model.User); ok { - r0 = rf(_a0) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.User) - } - } - var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(_a0) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} +// CreateUser provides a mock function with given fields: _a0 func (_m *Store) CreateUser(_a0 *model.User) error { ret := _m.Called(_a0) @@ -62,11 +37,13 @@ func (_m *Store) CreateUser(_a0 *model.User) error { return r0 } -func (_m *Store) UpdateUser(_a0 *model.User) error { + +// DeleteRepo provides a mock function with given fields: _a0 +func (_m *Store) DeleteRepo(_a0 *model.Repo) error { ret := _m.Called(_a0) var r0 error - if rf, ok := ret.Get(0).(func(*model.User) error); ok { + if rf, ok := ret.Get(0).(func(*model.Repo) error); ok { r0 = rf(_a0) } else { r0 = ret.Error(0) @@ -74,6 +51,8 @@ func (_m *Store) UpdateUser(_a0 *model.User) error { return r0 } + +// DeleteUser provides a mock function with given fields: _a0 func (_m *Store) DeleteUser(_a0 *model.User) error { ret := _m.Called(_a0) @@ -86,6 +65,8 @@ func (_m *Store) DeleteUser(_a0 *model.User) error { return r0 } + +// GetRepo provides a mock function with given fields: _a0 func (_m *Store) GetRepo(_a0 int64) (*model.Repo, error) { ret := _m.Called(_a0) @@ -107,33 +88,37 @@ func (_m *Store) GetRepo(_a0 int64) (*model.Repo, error) { return r0, r1 } -func (_m *Store) GetRepoSlug(_a0 string) (*model.Repo, error) { + +// GetRepoMulti provides a mock function with given fields: _a0 +func (_m *Store) GetRepoMulti(_a0 ...string) ([]*model.Repo, error) { ret := _m.Called(_a0) - var r0 *model.Repo - if rf, ok := ret.Get(0).(func(string) *model.Repo); ok { - r0 = rf(_a0) + var r0 []*model.Repo + if rf, ok := ret.Get(0).(func(...string) []*model.Repo); ok { + r0 = rf(_a0...) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).(*model.Repo) + r0 = ret.Get(0).([]*model.Repo) } } var r1 error - if rf, ok := ret.Get(1).(func(string) error); ok { - r1 = rf(_a0) + if rf, ok := ret.Get(1).(func(...string) error); ok { + r1 = rf(_a0...) } else { r1 = ret.Error(1) } return r0, r1 } -func (_m *Store) GetRepoMulti(_a0 ...string) ([]*model.Repo, error) { + +// GetRepoOwner provides a mock function with given fields: _a0 +func (_m *Store) GetRepoOwner(_a0 string) ([]*model.Repo, error) { ret := _m.Called(_a0) var r0 []*model.Repo - if rf, ok := ret.Get(0).(func(...string) []*model.Repo); ok { - r0 = rf(_a0...) + if rf, ok := ret.Get(0).(func(string) []*model.Repo); ok { + r0 = rf(_a0) } else { if ret.Get(0) != nil { r0 = ret.Get(0).([]*model.Repo) @@ -141,23 +126,25 @@ func (_m *Store) GetRepoMulti(_a0 ...string) ([]*model.Repo, error) { } var r1 error - if rf, ok := ret.Get(1).(func(...string) error); ok { - r1 = rf(_a0...) + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(_a0) } else { r1 = ret.Error(1) } return r0, r1 } -func (_m *Store) GetRepoOwner(_a0 string) ([]*model.Repo, error) { + +// GetRepoSlug provides a mock function with given fields: _a0 +func (_m *Store) GetRepoSlug(_a0 string) (*model.Repo, error) { ret := _m.Called(_a0) - var r0 []*model.Repo - if rf, ok := ret.Get(0).(func(string) []*model.Repo); ok { + var r0 *model.Repo + if rf, ok := ret.Get(0).(func(string) *model.Repo); ok { r0 = rf(_a0) } else { if ret.Get(0) != nil { - r0 = ret.Get(0).([]*model.Repo) + r0 = ret.Get(0).(*model.Repo) } } @@ -170,18 +157,54 @@ func (_m *Store) GetRepoOwner(_a0 string) ([]*model.Repo, error) { return r0, r1 } -func (_m *Store) CreateRepo(_a0 *model.Repo) error { + +// GetUser provides a mock function with given fields: _a0 +func (_m *Store) GetUser(_a0 int64) (*model.User, error) { ret := _m.Called(_a0) - var r0 error - if rf, ok := ret.Get(0).(func(*model.Repo) error); ok { + var r0 *model.User + if rf, ok := ret.Get(0).(func(int64) *model.User); ok { r0 = rf(_a0) } else { - r0 = ret.Error(0) + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.User) + } } - return r0 + var r1 error + if rf, ok := ret.Get(1).(func(int64) error); ok { + r1 = rf(_a0) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetUserLogin provides a mock function with given fields: _a0 +func (_m *Store) GetUserLogin(_a0 string) (*model.User, error) { + ret := _m.Called(_a0) + + var r0 *model.User + if rf, ok := ret.Get(0).(func(string) *model.User); ok { + r0 = rf(_a0) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.User) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(_a0) + } else { + r1 = ret.Error(1) + } + + return r0, r1 } + +// UpdateRepo provides a mock function with given fields: _a0 func (_m *Store) UpdateRepo(_a0 *model.Repo) error { ret := _m.Called(_a0) @@ -194,11 +217,13 @@ func (_m *Store) UpdateRepo(_a0 *model.Repo) error { return r0 } -func (_m *Store) DeleteRepo(_a0 *model.Repo) error { + +// UpdateUser provides a mock function with given fields: _a0 +func (_m *Store) UpdateUser(_a0 *model.User) error { ret := _m.Called(_a0) var r0 error - if rf, ok := ret.Get(0).(func(*model.Repo) error); ok { + if rf, ok := ret.Get(0).(func(*model.User) error); ok { r0 = rf(_a0) } else { r0 = ret.Error(0) diff --git a/web/hook.go b/web/hook.go index 4782c3e..15b85df 100644 --- a/web/hook.go +++ b/web/hook.go @@ -12,7 +12,6 @@ import ( log "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" "github.com/hashicorp/go-version" - "github.com/lgtmco/lgtm/version" ) func Hook(c *gin.Context) { @@ -22,23 +21,13 @@ func Hook(c *gin.Context) { c.String(500, "Error parsing hook. %s", err) return } - if hook != nil { - processCommentHook(c, hook) - } - - statusHook, err := remote.GetStatusHook(c, c.Request) - if err != nil { - log.Errorf("Error parsing status hook. %s", err) - c.String(500, "Error parsing status hook. %s", err) - return - } - if statusHook != nil { - processStatusHook(c, statusHook) - } - - if hook == nil && statusHook == nil { + switch hook.Kind { + case "issue_comment": + processCommentHook(c, hook.IssueComment) + case "status": + processStatusHook(c, hook.Status) + default: c.String(200, "pong") - return } } @@ -97,12 +86,14 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { // to create the version, need to scan the comments on the pull request to see if anyone specified a version # // if so, use the largest specified version #. if not, increment the last version version # for the release - maxVer, err := remote.GetMaxExistingTag(c, user, hook.Repo) + tags, err := remote.GetTagList(c, user, hook.Repo) if err != nil { - log.Warnf("Unable to find the max version tag for %s/%s: %s", hook.Repo.Owner, hook.Repo.Name, err) + log.Warnf("Unable to get the tags for %s/%s: %s", hook.Repo.Owner, hook.Repo.Name, err) continue } + _, maxVer := tags.GetMaxTag() + comments, err := getComments(c, user, repo, v.Number) if err != nil { log.Warnf("Unable to find the comments for pull request %s: %s", v.Title, err) @@ -134,7 +125,7 @@ func processStatusHook(c *gin.Context, hook *model.StatusHook) { }) } -func processCommentHook(c *gin.Context, hook *model.Hook) { +func processCommentHook(c *gin.Context, hook *model.IssueCommentHook) { repo, err := store.GetRepoSlug(c, hook.Repo.Slug) if err != nil {