cleaning up the middleware and adding caching with TTL

Makefile

@@ -36,7 +36,7 @@ build:
 	go build
 
 build_static:
-	go build --ldflags '-extldflags "-static"' -o drone_static
+	go build --ldflags '-extldflags "-static" -X main.version=$(BUILD_NUMBER)' -o drone_static
 
 test:
 	go test -cover $(PACKAGES)

controller/user.go

@@ -9,19 +9,8 @@ import (
 	"github.com/drone/drone/router/middleware/context"
 	"github.com/drone/drone/router/middleware/session"
 	"github.com/drone/drone/shared/token"
-	"github.com/hashicorp/golang-lru"
 )
 
-var cache *lru.Cache
-
-func init() {
-	var err error
-	cache, err = lru.New(1028)
-	if err != nil {
-		panic(err)
-	}
-}
-
 func GetSelf(c *gin.Context) {
 	c.IndentedJSON(200, session.User(c))
 }
@@ -52,11 +41,9 @@ func GetRemoteRepos(c *gin.Context) {
 	user := session.User(c)
 	remote := context.Remote(c)
 
-	// attempt to get the repository list from the
-	// cache since the operation is expensive
-	v, ok := cache.Get(user.Login)
+	reposv, ok := c.Get("repos")
 	if ok {
-		c.IndentedJSON(http.StatusOK, v)
+		c.IndentedJSON(http.StatusOK, reposv)
 		return
 	}
 
@@ -65,7 +52,8 @@ func GetRemoteRepos(c *gin.Context) {
 		c.AbortWithStatus(http.StatusInternalServerError)
 		return
 	}
-	cache.Add(user.Login, repos)
+
+	c.Set("repos", repos)
 	c.IndentedJSON(http.StatusOK, repos)
 }
 

drone.go

@@ -7,13 +7,18 @@ import (
 	"github.com/drone/drone/remote"
 	"github.com/drone/drone/router"
 	"github.com/drone/drone/router/middleware/context"
+	"github.com/drone/drone/router/middleware/header"
 	"github.com/drone/drone/shared/database"
 	"github.com/drone/drone/shared/envconfig"
 	"github.com/drone/drone/shared/server"
 
 	"github.com/Sirupsen/logrus"
 )
 
+// build revision number populated by the continuous
+// integration server at compile time.
+var build string = "custom"
+
 var (
 	dotenv = flag.String("config", ".env", "")
 	debug  = flag.Bool("debug", false, "")
@@ -43,6 +48,7 @@ func main() {
 	server_ := server.Load(env)
 	server_.Run(
 		router.Load(
+			header.Version(build),
 			context.SetDatabase(database_),
 			context.SetRemote(remote_),
 			context.SetEngine(engine_),

router/middleware/cache/cache.go

@@ -0,0 +1,49 @@
+package cache
+
+import (
+	"time"
+
+	"github.com/hashicorp/golang-lru"
+)
+
+// single instance of a thread-safe lru cache
+var cache *lru.Cache
+
+func init() {
+	var err error
+	cache, err = lru.New(2048)
+	if err != nil {
+		panic(err)
+	}
+}
+
+// item is a simple wrapper around a cacheable object
+// that tracks the ttl for item expiration in the cache.
+type item struct {
+	value interface{}
+	ttl   time.Time
+}
+
+// set adds the key value pair to the cache with the
+// specified ttl expiration.
+func set(key string, value interface{}, ttl int64) {
+	ttlv := time.Now().Add(time.Duration(ttl) * time.Second)
+	cache.Add(key, &item{value, ttlv})
+}
+
+// get gets the value from the cache for the given key.
+// if the value does not exist, a nil value is returned.
+// if the value exists, but is expired, the value is returned
+// with a bool flag set to true.
+func get(key string) (interface{}, bool) {
+	v, ok := cache.Get(key)
+	if !ok {
+		return nil, false
+	}
+	vv := v.(*item)
+	expired := vv.ttl.Before(time.Now())
+	if expired {
+		cache.Remove(key)
+	}
+	return vv.value, expired
+}

router/middleware/cache/cache_test.go

@@ -0,0 +1,40 @@
+package cache
+
+import (
+	"testing"
+
+	"github.com/franela/goblin"
+)
+
+func TestCache(t *testing.T) {
+
+	g := goblin.Goblin(t)
+	g.Describe("Cache", func() {
+
+		g.BeforeEach(func() {
+			cache.Purge()
+		})
+
+		g.It("should set and get item", func() {
+			set("foo", "bar", 1000)
+			val, expired := get("foo")
+			g.Assert(val).Equal("bar")
+			g.Assert(expired).Equal(false)
+		})
+
+		g.It("should return nil when item not found", func() {
+			val, expired := get("foo")
+			g.Assert(val == nil).IsTrue()
+			g.Assert(expired).Equal(false)
+		})
+
+		g.It("should get expired item and purge", func() {
+			set("foo", "bar", -900)
+			val, expired := get("foo")
+			g.Assert(val).Equal("bar")
+			g.Assert(expired).Equal(true)
+			val, _ = get("foo")
+			g.Assert(val == nil).IsTrue()
+		})
+	})
+}

router/middleware/cache/perms.go

@@ -0,0 +1,52 @@
+package cache
+
+import (
+	"fmt"
+
+	"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 rempository 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
+	}
+
+	key := fmt.Sprintf("perm/%s/%s/%s",
+		user.(*model.User).Login,
+		owner,
+		name,
+	)
+
+	// if the item already exists in the cache
+	// we can continue the middleware chain and
+	// exit afterwards.
+	v, _ := get(key)
+	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 {
+		set(key, perm, 86400) // 24 hours
+	}
+}

router/middleware/cache/perms_test.go

@@ -0,0 +1,60 @@
+package cache
+
+import (
+	"testing"
+
+	"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() {
+
+		g.BeforeEach(func() {
+			cache.Purge()
+		})
+
+		g.It("should skip when no user session", func() {
+			c := &gin.Context{}
+			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 := &gin.Context{}
+			c.Params = gin.Params{
+				gin.Param{Key: "owner", Value: "octocat"},
+				gin.Param{Key: "name", Value: "hello-world"},
+			}
+			c.Set("user", fakeUser)
+			set("perm/octocat/octocat/hello-world", fakePerm, 999)
+
+			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",
+}

router/middleware/cache/repos.go

@@ -0,0 +1,44 @@
+package cache
+
+import (
+	"fmt"
+
+	"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
+	}
+
+	key := fmt.Sprintf("repos/%s",
+		user.(*model.User).Login,
+	)
+
+	// if the item already exists in the cache
+	// we can continue the middleware chain and
+	// exit afterwards.
+	v, _ := get(key)
+	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 {
+		set(key, repos, 86400) // 24 hours
+	}
+}

router/middleware/cache/repos_test.go

@@ -0,0 +1,46 @@
+package cache
+
+import (
+	"testing"
+
+	"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() {
+
+		g.BeforeEach(func() {
+			cache.Purge()
+		})
+
+		g.It("should skip when no user session", func() {
+			c := &gin.Context{}
+
+			Perms(c)
+
+			_, ok := c.Get("perm")
+			g.Assert(ok).IsFalse()
+		})
+
+		g.It("should get repos from cache", func() {
+			c := &gin.Context{}
+			c.Set("user", fakeUser)
+			set("repos/octocat", fakeRepos, 999)
+
+			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"},
+}