From 3dd0260b69e9708eb9914a1fab5d63f83e4943bd Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Fri, 4 Mar 2016 21:15:50 -0800 Subject: [PATCH] improve and simplify repository caching --- cache/cache.go | 10 +- cache/helper.go | 79 ++++----- cache/helper_test.go | 74 ++++++-- controller/pages.go | 30 +--- controller/repo.go | 5 +- controller/user.go | 47 ++--- remote/github/github.go | 15 +- remote/mock/remote.go | 238 ++++++++++++++++++++++++++ remote/remote.go | 83 ++++++++- router/middleware/cache/perms.go | 54 ------ router/middleware/cache/perms_test.go | 61 ------- router/middleware/cache/repos.go | 42 ----- router/middleware/cache/repos_test.go | 46 ----- router/middleware/session/repo.go | 16 +- router/router.go | 8 +- 15 files changed, 444 insertions(+), 364 deletions(-) create mode 100644 remote/mock/remote.go delete mode 100644 router/middleware/cache/perms.go delete mode 100644 router/middleware/cache/perms_test.go delete mode 100644 router/middleware/cache/repos.go delete mode 100644 router/middleware/cache/repos_test.go diff --git a/cache/cache.go b/cache/cache.go index 80e631a8f..3c4e8eda2 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -1,5 +1,7 @@ package cache +//go:generate mockery -name Cache -output mock -case=underscore + import ( "time" @@ -23,7 +25,7 @@ func Set(c context.Context, key string, value interface{}) error { // Default creates an in-memory cache with the default // 30 minute expiration period. func Default() Cache { - return cache.NewMemoryWithTTL(time.Minute * 30) + return NewTTL(time.Minute * 30) } // NewTTL returns an in-memory cache with the specified @@ -31,9 +33,3 @@ func Default() Cache { func NewTTL(t time.Duration) Cache { return cache.NewMemoryWithTTL(t) } - -// NewTTL returns an in-memory cache with the specified -// ttl expiration period. -func NewLRU(size int) Cache { - return cache.NewLRU(size) -} diff --git a/cache/helper.go b/cache/helper.go index 74a55b606..14efc67ff 100644 --- a/cache/helper.go +++ b/cache/helper.go @@ -4,72 +4,51 @@ import ( "fmt" "github.com/drone/drone/model" + "github.com/drone/drone/remote" "golang.org/x/net/context" ) -// GetRepos returns the user permissions to the named repository -// from the cache associated with the current context. -func GetPerms(c context.Context, user *model.User, owner, name string) *model.Perm { +// GetPerm returns the user permissions repositories from the cache +// associated with the current repository. +func GetPerms(c context.Context, user *model.User, owner, name string) (*model.Perm, error) { key := fmt.Sprintf("perms:%s:%s/%s", user.Login, owner, name, ) - val, err := FromContext(c).Get(key) - if err != nil { - return nil + // if we fetch from the cache we can return immediately + val, err := Get(c, key) + if err == nil { + return val.(*model.Perm), nil } - return val.(*model.Perm) -} - -// SetRepos adds the listof user permissions to the named repsotiory -// to the cache assocaited with the current context. -func SetPerms(c context.Context, user *model.User, perm *model.Perm, owner, name string) { - key := fmt.Sprintf("perms:%s:%s/%s", - user.Login, - owner, - name, - ) - FromContext(c).Set(key, perm) + // else we try to grab from the remote system and + // populate our cache. + perm, err := remote.Perm(c, user, owner, name) + if err != nil { + return nil, err + } + Set(c, key, perm) + return perm, nil } // GetRepos returns the list of user repositories from the cache // associated with the current context. -func GetRepos(c context.Context, user *model.User) []*model.RepoLite { +func GetRepos(c context.Context, user *model.User) ([]*model.RepoLite, error) { key := fmt.Sprintf("repos:%s", user.Login, ) - val, err := FromContext(c).Get(key) - if err != nil { - return nil + // if we fetch from the cache we can return immediately + val, err := Get(c, key) + if err == nil { + return val.([]*model.RepoLite), nil + } + // else we try to grab from the remote system and + // populate our cache. + repos, err := remote.Repos(c, user) + if err != nil { + return nil, err } - return val.([]*model.RepoLite) -} -// SetRepos adds the listof user repositories to the cache assocaited -// with the current context. -func SetRepos(c context.Context, user *model.User, repos []*model.RepoLite) { - key := fmt.Sprintf("repos:%s", - user.Login, - ) - FromContext(c).Set(key, repos) + Set(c, key, repos) + return repos, nil } - -// GetSetRepos is a helper function that attempts to get the -// repository list from the cache first. If no data is in the -// cache or it is expired, it will remotely fetch the list of -// repositories and populate the cache. -// func GetSetRepos(c context.Context, user *model.User) ([]*model.RepoLite, error) { -// cache := FromContext(c).Repos() -// repos := FromContext(c).Repos().Get(user) -// if repos != nil { -// return repos, nil -// } -// var err error -// repos, err = remote.FromContext(c).Repos(user) -// if err != nil { -// return nil, err -// } -// cache.Set(user, repos) -// return repos, nil -// } diff --git a/cache/helper_test.go b/cache/helper_test.go index 3dacd9611..aeed89e48 100644 --- a/cache/helper_test.go +++ b/cache/helper_test.go @@ -1,9 +1,13 @@ package cache import ( + "errors" + "fmt" "testing" "github.com/drone/drone/model" + "github.com/drone/drone/remote" + "github.com/drone/drone/remote/mock" "github.com/franela/goblin" "github.com/gin-gonic/gin" ) @@ -11,43 +15,83 @@ import ( func TestHelper(t *testing.T) { g := goblin.Goblin(t) + g.Describe("Cache helpers", func() { var c *gin.Context + var r *mock.Remote + g.BeforeEach(func() { c = new(gin.Context) ToContext(c, Default()) + + r = new(mock.Remote) + remote.ToContext(c, r) }) - g.It("Should set and get permissions", func() { - SetPerms(c, fakeUser, fakePerm, "octocat", "Spoon-Knife") + g.It("Should get permissions from remote", func() { + r.On("Perm", fakeUser, fakeRepo.Owner, fakeRepo.Name).Return(fakePerm, nil).Once() + p, err := GetPerms(c, fakeUser, fakeRepo.Owner, fakeRepo.Name) + g.Assert(p).Equal(fakePerm) + g.Assert(err).Equal(nil) - v := GetPerms(c, fakeUser, "octocat", "Spoon-Knife") - g.Assert(v).Equal(fakePerm) }) - g.It("Should return nil if permissions if not found", func() { - v := GetPerms(c, fakeUser, "octocat", "Spoon-Knife") - g.Assert(v == nil).IsTrue() + g.It("Should get permissions from cache", func() { + key := fmt.Sprintf("perms:%s:%s/%s", + fakeUser.Login, + fakeRepo.Owner, + fakeRepo.Name, + ) + + Set(c, key, fakePerm) + r.On("Perm", fakeUser, fakeRepo.Owner, fakeRepo.Name).Return(nil, fakeErr).Once() + p, err := GetPerms(c, fakeUser, fakeRepo.Owner, fakeRepo.Name) + g.Assert(p).Equal(fakePerm) + g.Assert(err).Equal(nil) }) - g.It("Should set and get repositories", func() { - SetRepos(c, fakeUser, fakeRepos) - - v := GetRepos(c, fakeUser) - g.Assert(v).Equal(fakeRepos) + g.It("Should get permissions error", func() { + r.On("Perm", fakeUser, fakeRepo.Owner, fakeRepo.Name).Return(nil, fakeErr).Once() + p, err := GetPerms(c, fakeUser, fakeRepo.Owner, fakeRepo.Name) + g.Assert(p == nil).IsTrue() + g.Assert(err).Equal(fakeErr) }) - g.It("Should return nil if repositories not found", func() { - v := GetRepos(c, fakeUser) - g.Assert(v == nil).IsTrue() + g.It("Should set and get repos", func() { + + r.On("Repos", fakeUser).Return(fakeRepos, nil).Once() + p, err := GetRepos(c, fakeUser) + g.Assert(p).Equal(fakeRepos) + g.Assert(err).Equal(nil) + }) + + g.It("Should get repos", func() { + key := fmt.Sprintf("repos:%s", + fakeUser.Login, + ) + + Set(c, key, fakeRepos) + r.On("Repos", fakeUser).Return(nil, fakeErr).Once() + p, err := GetRepos(c, fakeUser) + g.Assert(p).Equal(fakeRepos) + g.Assert(err).Equal(nil) + }) + + g.It("Should get repos error", func() { + r.On("Repos", fakeUser).Return(nil, fakeErr).Once() + p, err := GetRepos(c, fakeUser) + g.Assert(p == nil).IsTrue() + g.Assert(err).Equal(fakeErr) }) }) } var ( + fakeErr = errors.New("Not Found") fakeUser = &model.User{Login: "octocat"} fakePerm = &model.Perm{true, true, true} + fakeRepo = &model.RepoLite{Owner: "octocat", Name: "Hello-World"} fakeRepos = []*model.RepoLite{ {Owner: "octocat", Name: "Hello-World"}, {Owner: "octocat", Name: "hello-world"}, diff --git a/controller/pages.go b/controller/pages.go index 31a014203..251e4418a 100644 --- a/controller/pages.go +++ b/controller/pages.go @@ -5,11 +5,10 @@ import ( "strconv" "time" - log "github.com/Sirupsen/logrus" "github.com/gin-gonic/gin" + "github.com/drone/drone/cache" "github.com/drone/drone/model" - "github.com/drone/drone/remote" "github.com/drone/drone/router/middleware/session" "github.com/drone/drone/shared/httputil" "github.com/drone/drone/shared/token" @@ -17,38 +16,19 @@ import ( ) func ShowIndex(c *gin.Context) { - remote := remote.FromContext(c) user := session.User(c) if user == nil { c.Redirect(http.StatusSeeOther, "/login") return } - var err error - var repos []*model.RepoLite - // get the repository list from the cache - reposv, ok := c.Get("repos") - if ok { - repos = reposv.([]*model.RepoLite) - } else { - repos, err = remote.Repos(user) - if err != nil { - log.Errorf("Failure to get remote repositories for %s. %s.", - user.Login, err) - } else { - c.Set("repos", repos) - } + repos, err := cache.GetRepos(c, user) + if err != nil { + c.String(400, err.Error()) + return } - // for each repository in the remote system we get - // the intersection of those repostiories in Drone - // repos_, err := store.GetRepoListOf(c, repos) - // if err != nil { - // log.Errorf("Failure to get repository list for %s. %s.", - // user.Login, err) - // } - c.HTML(200, "repos.html", gin.H{ "User": user, "Repos": repos, diff --git a/controller/repo.go b/controller/repo.go index ba566a91d..53c937551 100644 --- a/controller/repo.go +++ b/controller/repo.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "gopkg.in/yaml.v2" + "github.com/drone/drone/cache" "github.com/drone/drone/model" "github.com/drone/drone/remote" "github.com/drone/drone/router/middleware/session" @@ -34,7 +35,7 @@ func PostRepo(c *gin.Context) { c.String(404, err.Error()) return } - m, err := remote.Perm(user, owner, name) + m, err := cache.GetPerms(c, user, owner, name) if err != nil { c.String(404, err.Error()) return @@ -176,7 +177,7 @@ func PostRepoKey(c *gin.Context) { body, err := ioutil.ReadAll(c.Request.Body) if err != nil { c.String(500, "Error reading private key from body. %s", err) - return + return } pkey := crypto.UnmarshalPrivateKey(body) if pkey == nil { diff --git a/controller/user.go b/controller/user.go index 69759f725..92540b94e 100644 --- a/controller/user.go +++ b/controller/user.go @@ -5,8 +5,7 @@ import ( "github.com/gin-gonic/gin" - "github.com/drone/drone/model" - "github.com/drone/drone/remote" + "github.com/drone/drone/cache" "github.com/drone/drone/router/middleware/session" "github.com/drone/drone/shared/token" "github.com/drone/drone/store" @@ -18,20 +17,12 @@ func GetSelf(c *gin.Context) { func GetFeed(c *gin.Context) { user := session.User(c) - remote := remote.FromContext(c) - var repos []*model.RepoLite // get the repository list from the cache - reposv, ok := c.Get("repos") - if ok { - repos = reposv.([]*model.RepoLite) - } else { - var err error - repos, err = remote.Repos(user) - if err != nil { - c.String(400, err.Error()) - return - } + repos, err := cache.GetRepos(c, user) + if err != nil { + c.String(400, err.Error()) + return } feed, err := store.GetUserFeed(c, repos) @@ -44,20 +35,11 @@ func GetFeed(c *gin.Context) { func GetRepos(c *gin.Context) { user := session.User(c) - remote := remote.FromContext(c) - var repos []*model.RepoLite - // get the repository list from the cache - reposv, ok := c.Get("repos") - if ok { - repos = reposv.([]*model.RepoLite) - } else { - var err error - repos, err = remote.Repos(user) - if err != nil { - c.AbortWithStatus(http.StatusInternalServerError) - return - } + repos, err := cache.GetRepos(c, user) + if err != nil { + c.AbortWithStatus(http.StatusInternalServerError) + return } // for each repository in the remote system we get @@ -68,27 +50,18 @@ func GetRepos(c *gin.Context) { return } - c.Set("repos", repos) c.IndentedJSON(http.StatusOK, repos_) } func GetRemoteRepos(c *gin.Context) { user := session.User(c) - remote := remote.FromContext(c) - reposv, ok := c.Get("repos") - if ok { - c.IndentedJSON(http.StatusOK, reposv) - return - } - - repos, err := remote.Repos(user) + repos, err := cache.GetRepos(c, user) if err != nil { c.AbortWithStatus(http.StatusInternalServerError) return } - c.Set("repos", repos) c.IndentedJSON(http.StatusOK, repos) } diff --git a/remote/github/github.go b/remote/github/github.go index d7c793b7f..80883cf1c 100644 --- a/remote/github/github.go +++ b/remote/github/github.go @@ -19,9 +19,10 @@ import ( ) const ( - DefaultURL = "https://github.com" - DefaultAPI = "https://api.github.com" - DefaultScope = "repo,repo:status,user:email" + DefaultURL = "https://github.com" + DefaultAPI = "https://api.github.com" + DefaultScope = "repo,repo:status,user:email" + DefaultMergeRef = "merge" ) type Github struct { @@ -29,6 +30,7 @@ type Github struct { API string Client string Secret string + MergeRef string Orgs []string Open bool PrivateMode bool @@ -59,6 +61,7 @@ func Load(env envconfig.Env) *Github { github.SkipVerify, _ = strconv.ParseBool(params.Get("skip_verify")) github.Open, _ = strconv.ParseBool(params.Get("open")) github.GitSSH, _ = strconv.ParseBool(params.Get("ssh")) + github.MergeRef = params.Get("merge_ref") if github.URL == DefaultURL { github.API = DefaultAPI @@ -66,6 +69,10 @@ func Load(env envconfig.Env) *Github { github.API = github.URL + "/api/v3/" } + if github.MergeRef == "" { + github.MergeRef = DefaultMergeRef + } + return &github } @@ -402,7 +409,7 @@ func (g *Github) pullRequest(r *http.Request) (*model.Repo, *model.Build, error) build := &model.Build{} build.Event = model.EventPull build.Commit = *hook.PullRequest.Head.SHA - build.Ref = fmt.Sprintf("refs/pull/%d/merge", *hook.PullRequest.Number) + build.Ref = fmt.Sprintf("refs/pull/%d/%s", *hook.PullRequest.Number, g.MergeRef) build.Link = *hook.PullRequest.HTMLURL build.Branch = *hook.PullRequest.Head.Ref build.Message = *hook.PullRequest.Title diff --git a/remote/mock/remote.go b/remote/mock/remote.go new file mode 100644 index 000000000..60aed3f33 --- /dev/null +++ b/remote/mock/remote.go @@ -0,0 +1,238 @@ +package mock + +import "github.com/stretchr/testify/mock" + +import "net/http" +import "github.com/drone/drone/model" + +type Remote struct { + mock.Mock +} + +func (_m *Remote) Login(w http.ResponseWriter, r *http.Request) (*model.User, bool, error) { + ret := _m.Called(w, r) + + var r0 *model.User + if rf, ok := ret.Get(0).(func(http.ResponseWriter, *http.Request) *model.User); ok { + r0 = rf(w, r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.User) + } + } + + var r1 bool + if rf, ok := ret.Get(1).(func(http.ResponseWriter, *http.Request) bool); ok { + r1 = rf(w, r) + } else { + r1 = ret.Get(1).(bool) + } + + var r2 error + if rf, ok := ret.Get(2).(func(http.ResponseWriter, *http.Request) error); ok { + r2 = rf(w, r) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} +func (_m *Remote) Auth(token string, secret string) (string, error) { + ret := _m.Called(token, secret) + + var r0 string + if rf, ok := ret.Get(0).(func(string, string) string); ok { + r0 = rf(token, secret) + } else { + r0 = ret.Get(0).(string) + } + + var r1 error + if rf, ok := ret.Get(1).(func(string, string) error); ok { + r1 = rf(token, secret) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} +func (_m *Remote) Repo(u *model.User, owner string, repo string) (*model.Repo, error) { + ret := _m.Called(u, owner, repo) + + var r0 *model.Repo + if rf, ok := ret.Get(0).(func(*model.User, string, string) *model.Repo); ok { + r0 = rf(u, owner, repo) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Repo) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.User, string, string) error); ok { + r1 = rf(u, owner, repo) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} +func (_m *Remote) Repos(u *model.User) ([]*model.RepoLite, error) { + ret := _m.Called(u) + + var r0 []*model.RepoLite + if rf, ok := ret.Get(0).(func(*model.User) []*model.RepoLite); ok { + r0 = rf(u) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.RepoLite) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.User) error); ok { + r1 = rf(u) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} +func (_m *Remote) Perm(u *model.User, owner string, repo string) (*model.Perm, error) { + ret := _m.Called(u, owner, repo) + + var r0 *model.Perm + if rf, ok := ret.Get(0).(func(*model.User, string, string) *model.Perm); ok { + r0 = rf(u, owner, repo) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Perm) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*model.User, string, string) error); ok { + r1 = rf(u, owner, repo) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} +func (_m *Remote) Script(u *model.User, r *model.Repo, b *model.Build) ([]byte, []byte, error) { + ret := _m.Called(u, r, b) + + var r0 []byte + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *model.Build) []byte); ok { + r0 = rf(u, r, b) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]byte) + } + } + + var r1 []byte + if rf, ok := ret.Get(1).(func(*model.User, *model.Repo, *model.Build) []byte); ok { + r1 = rf(u, r, b) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).([]byte) + } + } + + var r2 error + if rf, ok := ret.Get(2).(func(*model.User, *model.Repo, *model.Build) error); ok { + r2 = rf(u, r, b) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} +func (_m *Remote) Status(u *model.User, r *model.Repo, b *model.Build, link string) error { + ret := _m.Called(u, r, b, link) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *model.Build, string) error); ok { + r0 = rf(u, r, b, link) + } else { + r0 = ret.Error(0) + } + + return r0 +} +func (_m *Remote) Netrc(u *model.User, r *model.Repo) (*model.Netrc, error) { + ret := _m.Called(u, r) + + var r0 *model.Netrc + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo) *model.Netrc); ok { + r0 = rf(u, r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Netrc) + } + } + + 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) Activate(u *model.User, r *model.Repo, k *model.Key, link string) error { + ret := _m.Called(u, r, k, link) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, *model.Key, string) error); ok { + r0 = rf(u, r, k, link) + } else { + r0 = ret.Error(0) + } + + return r0 +} +func (_m *Remote) Deactivate(u *model.User, r *model.Repo, link string) error { + ret := _m.Called(u, r, link) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.User, *model.Repo, string) error); ok { + r0 = rf(u, r, link) + } else { + r0 = ret.Error(0) + } + + return r0 +} +func (_m *Remote) Hook(r *http.Request) (*model.Repo, *model.Build, error) { + ret := _m.Called(r) + + var r0 *model.Repo + if rf, ok := ret.Get(0).(func(*http.Request) *model.Repo); ok { + r0 = rf(r) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.Repo) + } + } + + var r1 *model.Build + if rf, ok := ret.Get(1).(func(*http.Request) *model.Build); ok { + r1 = rf(r) + } else { + if ret.Get(1) != nil { + r1 = ret.Get(1).(*model.Build) + } + } + + var r2 error + if rf, ok := ret.Get(2).(func(*http.Request) error); ok { + r2 = rf(r) + } else { + r2 = ret.Error(2) + } + + return r0, r1, r2 +} diff --git a/remote/remote.go b/remote/remote.go index c143a0a34..8020bfb08 100644 --- a/remote/remote.go +++ b/remote/remote.go @@ -1,5 +1,7 @@ package remote +//go:generate mockery -name Remote -output mock -case=underscore + import ( "net/http" @@ -10,7 +12,8 @@ import ( "github.com/drone/drone/remote/gogs" "github.com/drone/drone/shared/envconfig" - log "github.com/Sirupsen/logrus" + "github.com/Sirupsen/logrus" + "golang.org/x/net/context" ) func Load(env envconfig.Env) Remote { @@ -27,7 +30,7 @@ func Load(env envconfig.Env) Remote { return gogs.Load(env) default: - log.Fatalf("unknown remote driver %s", driver) + logrus.Fatalf("unknown remote driver %s", driver) } return nil @@ -83,3 +86,79 @@ type Refresher interface { // token was not refreshed, and error if it failed to refersh. Refresh(*model.User) (bool, error) } + +// Login authenticates the session and returns the +// remote user details. +func Login(c context.Context, w http.ResponseWriter, r *http.Request) (*model.User, bool, error) { + return FromContext(c).Login(w, r) +} + +// Auth authenticates the session and returns the remote user +// login for the given token and secret +func Auth(c context.Context, token, secret string) (string, error) { + return FromContext(c).Auth(token, secret) +} + +// Repo fetches the named repository from the remote system. +func Repo(c context.Context, u *model.User, owner, repo string) (*model.Repo, error) { + return FromContext(c).Repo(u, owner, repo) +} + +// Repos fetches a list of repos from the remote system. +func Repos(c context.Context, u *model.User) ([]*model.RepoLite, error) { + return FromContext(c).Repos(u) +} + +// Perm fetches the named repository permissions from +// the remote system for the specified user. +func Perm(c context.Context, u *model.User, owner, repo string) (*model.Perm, error) { + return FromContext(c).Perm(u, owner, repo) +} + +// Script fetches the build script (.drone.yml) from the remote +// repository and returns in string format. +func Script(c context.Context, u *model.User, r *model.Repo, b *model.Build) ([]byte, []byte, error) { + return FromContext(c).Script(u, r, b) +} + +// Status sends the commit status to the remote system. +// An example would be the GitHub pull request status. +func Status(c context.Context, u *model.User, r *model.Repo, b *model.Build, link string) error { + return FromContext(c).Status(u, r, b, link) +} + +// Netrc returns a .netrc file that can be used to clone +// private repositories from a remote system. +func Netrc(c context.Context, u *model.User, r *model.Repo) (*model.Netrc, error) { + return FromContext(c).Netrc(u, r) +} + +// Activate activates a repository by creating the post-commit hook and +// adding the SSH deploy key, if applicable. +func Activate(c context.Context, u *model.User, r *model.Repo, k *model.Key, link string) error { + return FromContext(c).Activate(u, r, k, link) +} + +// Deactivate removes a repository by removing all the post-commit hooks +// which are equal to link and removing the SSH deploy key. +func Deactivate(c context.Context, u *model.User, r *model.Repo, link string) error { + return FromContext(c).Deactivate(u, r, link) +} + +// Hook parses the post-commit hook from the Request body +// and returns the required data in a standard format. +func Hook(c context.Context, r *http.Request) (*model.Repo, *model.Build, error) { + return FromContext(c).Hook(r) +} + +// Refresh refreshes an oauth token and expiration for the given +// user. It returns true if the token was refreshed, false if the +// token was not refreshed, and error if it failed to refersh. +func Refresh(c context.Context, u *model.User) (bool, error) { + remote := FromContext(c) + refresher, ok := remote.(Refresher) + if !ok { + return false, nil + } + return refresher.Refresh(u) +} diff --git a/router/middleware/cache/perms.go b/router/middleware/cache/perms.go deleted file mode 100644 index 39a90861a..000000000 --- a/router/middleware/cache/perms.go +++ /dev/null @@ -1,54 +0,0 @@ -package cache - -import ( - "github.com/drone/drone/cache" - "github.com/drone/drone/model" - "github.com/gin-gonic/gin" -) - -const permKey = "perm" - -// Perms is a middleware function that attempts to cache the -// user's remote repository permissions (ie in GitHub) to minimize -// remote calls that might be expensive, slow or rate-limited. -func Perms(c *gin.Context) { - var ( - owner = c.Param("owner") - name = c.Param("name") - user, _ = c.Get("user") - ) - - if user == nil { - c.Next() - return - } - - // if the item already exists in the cache - // we can continue the middleware chain and - // exit afterwards. - v := cache.GetPerms(c, - user.(*model.User), - owner, - name, - ) - if v != nil { - c.Set("perm", v) - c.Next() - return - } - - // otherwise, if the item isn't cached we execute - // the middleware chain and then cache the permissions - // after the request is processed. - c.Next() - - perm, ok := c.Get("perm") - if ok { - cache.SetPerms(c, - user.(*model.User), - perm.(*model.Perm), - owner, - name, - ) - } -} diff --git a/router/middleware/cache/perms_test.go b/router/middleware/cache/perms_test.go deleted file mode 100644 index 1345500fd..000000000 --- a/router/middleware/cache/perms_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package cache - -import ( - "testing" - - "github.com/drone/drone/cache" - "github.com/drone/drone/model" - "github.com/franela/goblin" - "github.com/gin-gonic/gin" -) - -func TestPermCache(t *testing.T) { - - g := goblin.Goblin(t) - g.Describe("Perm Cache", func() { - - var c *gin.Context - g.BeforeEach(func() { - c = new(gin.Context) - cache.ToContext(c, cache.Default()) - }) - - g.It("should skip when no user session", func() { - c.Params = gin.Params{ - gin.Param{Key: "owner", Value: "octocat"}, - gin.Param{Key: "name", Value: "hello-world"}, - } - - Perms(c) - - _, ok := c.Get("perm") - g.Assert(ok).IsFalse() - }) - - g.It("should get perms from cache", func() { - c.Params = gin.Params{ - gin.Param{Key: "owner", Value: "octocat"}, - gin.Param{Key: "name", Value: "hello-world"}, - } - c.Set("user", fakeUser) - cache.SetPerms(c, fakeUser, fakePerm, "octocat", "hello-world") - - Perms(c) - - perm, ok := c.Get("perm") - g.Assert(ok).IsTrue() - g.Assert(perm).Equal(fakePerm) - }) - - }) -} - -var fakePerm = &model.Perm{ - Pull: true, - Push: true, - Admin: true, -} - -var fakeUser = &model.User{ - Login: "octocat", -} diff --git a/router/middleware/cache/repos.go b/router/middleware/cache/repos.go deleted file mode 100644 index 69ade5b1a..000000000 --- a/router/middleware/cache/repos.go +++ /dev/null @@ -1,42 +0,0 @@ -package cache - -import ( - "github.com/drone/drone/cache" - "github.com/drone/drone/model" - "github.com/gin-gonic/gin" -) - -// Repos is a middleware function that attempts to cache the -// user's list of remote repositories (ie in GitHub) to minimize -// remote calls that might be expensive, slow or rate-limited. -func Repos(c *gin.Context) { - var user, _ = c.Get("user") - - if user == nil { - c.Next() - return - } - - // if the item already exists in the cache - // we can continue the middleware chain and - // exit afterwards. - v := cache.GetRepos(c, user.(*model.User)) - if v != nil { - c.Set("repos", v) - c.Next() - return - } - - // otherwise, if the item isn't cached we execute - // the middleware chain and then cache the permissions - // after the request is processed. - c.Next() - - repos, ok := c.Get("repos") - if ok { - cache.SetRepos(c, - user.(*model.User), - repos.([]*model.RepoLite), - ) - } -} diff --git a/router/middleware/cache/repos_test.go b/router/middleware/cache/repos_test.go deleted file mode 100644 index 4971362a4..000000000 --- a/router/middleware/cache/repos_test.go +++ /dev/null @@ -1,46 +0,0 @@ -package cache - -import ( - "testing" - - "github.com/drone/drone/cache" - "github.com/drone/drone/model" - "github.com/franela/goblin" - "github.com/gin-gonic/gin" -) - -func TestReposCache(t *testing.T) { - - g := goblin.Goblin(t) - g.Describe("Repo List Cache", func() { - - var c *gin.Context - g.BeforeEach(func() { - c = new(gin.Context) - cache.ToContext(c, cache.Default()) - }) - - g.It("should skip when no user session", func() { - Perms(c) - - _, ok := c.Get("perm") - g.Assert(ok).IsFalse() - }) - - g.It("should get repos from cache", func() { - c.Set("user", fakeUser) - cache.SetRepos(c, fakeUser, fakeRepos) - - Repos(c) - - repos, ok := c.Get("repos") - g.Assert(ok).IsTrue() - g.Assert(repos).Equal(fakeRepos) - }) - - }) -} - -var fakeRepos = []*model.RepoLite{ - {Owner: "octocat", Name: "hello-world"}, -} diff --git a/router/middleware/session/repo.go b/router/middleware/session/repo.go index 6962bdb4b..8a9cd37c1 100644 --- a/router/middleware/session/repo.go +++ b/router/middleware/session/repo.go @@ -4,6 +4,7 @@ import ( "net/http" "os" + "github.com/drone/drone/cache" "github.com/drone/drone/model" "github.com/drone/drone/remote" "github.com/drone/drone/shared/token" @@ -112,19 +113,6 @@ func SetPerm() gin.HandlerFunc { repo := Repo(c) perm := &model.Perm{} - if user != nil { - // attempt to get the permissions from a local cache - // just to avoid excess API calls to GitHub - val, ok := c.Get("perm") - if ok { - c.Next() - - log.Debugf("%s using cached %+v permission to %s", - user.Login, val, repo.FullName) - return - } - } - switch { // if the user is not authenticated, and the // repository is private, the user has NO permission @@ -150,7 +138,7 @@ func SetPerm() gin.HandlerFunc { // check the remote system to get the users permissiosn. default: var err error - perm, err = remote.FromContext(c).Perm(user, repo.Owner, repo.Name) + perm, err = cache.GetPerms(c, user, repo.Owner, repo.Name) if err != nil { perm.Pull = false perm.Push = false diff --git a/router/router.go b/router/router.go index 464a40694..5f64f6646 100644 --- a/router/router.go +++ b/router/router.go @@ -7,7 +7,6 @@ import ( "github.com/gin-gonic/gin" "github.com/drone/drone/controller" - "github.com/drone/drone/router/middleware/cache" "github.com/drone/drone/router/middleware/header" "github.com/drone/drone/router/middleware/location" "github.com/drone/drone/router/middleware/session" @@ -27,10 +26,9 @@ func Load(middleware ...gin.HandlerFunc) http.Handler { e.Use(header.Secure) e.Use(middleware...) e.Use(session.SetUser()) - e.Use(cache.Perms) e.Use(token.Refresh) - e.GET("/", cache.Repos, controller.ShowIndex) + e.GET("/", controller.ShowIndex) e.GET("/login", controller.ShowLogin) e.GET("/login/form", controller.ShowLoginForm) e.GET("/logout", controller.GetLogout) @@ -64,8 +62,8 @@ func Load(middleware ...gin.HandlerFunc) http.Handler { user.Use(session.MustUser()) user.GET("", controller.GetSelf) user.GET("/feed", controller.GetFeed) - user.GET("/repos", cache.Repos, controller.GetRepos) - user.GET("/repos/remote", cache.Repos, controller.GetRemoteRepos) + user.GET("/repos", controller.GetRepos) + user.GET("/repos/remote", controller.GetRemoteRepos) user.POST("/token", controller.PostToken) }