Ensure memcache TTL cannot be over 30 days

[zeripath]

Memcached TTL cannot be > 30 days and if it is attempted the TTL is interpreted as
a unix timestamp.

This PR ensures that the maximum is set at 30 days.

Fix #14571

Signed-off-by: Andrew Thornton art27@cantab.net

[CirnoT]

I believe it would be better to allow it, but calculate unix timestamp from now + TTL time and feed that to memcached - that way, we don't lose anything.

[zeripath]

@CirnoT that would require something like:

diff --git a/modules/cache/cache.go b/modules/cache/cache.go
index 609f5a242..e830cb58b 100644
--- a/modules/cache/cache.go
+++ b/modules/cache/cache.go
@@ -58,7 +58,7 @@ func GetString(key string, getFunc func() (string, error)) (string, error) {
 		if value, err = getFunc(); err != nil {
 			return value, err
 		}
-		err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds()))
+		err = conn.Put(key, value, int64(setting.CacheService.TTLSeconds()))
 		if err != nil {
 			return "", err
 		}
@@ -86,7 +86,7 @@ func GetInt(key string, getFunc func() (int, error)) (int, error) {
 		if value, err = getFunc(); err != nil {
 			return value, err
 		}
-		err = conn.Put(key, value, int64(setting.CacheService.TTL.Seconds()))
+		err = conn.Put(key, value, int64(setting.CacheService.TTLSeconds()))
 		if err != nil {
 			return 0, err
 		}
diff --git a/modules/git/last_commit_cache.go b/modules/git/last_commit_cache.go
index 7cca60122..37a59e1fa 100644
--- a/modules/git/last_commit_cache.go
+++ b/modules/git/last_commit_cache.go
@@ -25,5 +25,5 @@ func (c *LastCommitCache) getCacheKey(repoPath, ref, entryPath string) string {
 // Put put the last commit id with commit and entry path
 func (c *LastCommitCache) Put(ref, entryPath, commitID string) error {
 	log("LastCommitCache save: [%s:%s:%s]", ref, entryPath, commitID)
-	return c.cache.Put(c.getCacheKey(c.repoPath, ref, entryPath), commitID, c.ttl)
+	return c.cache.Put(c.getCacheKey(c.repoPath, ref, entryPath), commitID, c.ttl())
 }
diff --git a/modules/git/last_commit_cache_nogogit.go b/modules/git/last_commit_cache_nogogit.go
index b9c50b5cf..0e52d1653 100644
--- a/modules/git/last_commit_cache_nogogit.go
+++ b/modules/git/last_commit_cache_nogogit.go
@@ -13,14 +13,14 @@ import (
 // LastCommitCache represents a cache to store last commit
 type LastCommitCache struct {
 	repoPath    string
-	ttl         int64
+	ttl         func() int64
 	repo        *Repository
 	commitCache map[string]*Commit
 	cache       Cache
 }
 
 // NewLastCommitCache creates a new last commit cache for repo
-func NewLastCommitCache(repoPath string, gitRepo *Repository, ttl int64, cache Cache) *LastCommitCache {
+func NewLastCommitCache(repoPath string, gitRepo *Repository, ttl func() int64, cache Cache) *LastCommitCache {
 	if cache == nil {
 		return nil
 	}
diff --git a/modules/repository/cache.go b/modules/repository/cache.go
index 0852771a5..5bb5fe485 100644
--- a/modules/repository/cache.go
+++ b/modules/repository/cache.go
@@ -41,7 +41,7 @@ func CacheRef(repo *models.Repository, gitRepo *git.Repository, fullRefName stri
 		return nil
 	}
 
-	commitCache := git.NewLastCommitCache(repo.FullName(), gitRepo, int64(setting.CacheService.LastCommit.TTL.Seconds()), cache.GetCache())
+	commitCache := git.NewLastCommitCache(repo.FullName(), gitRepo, setting.CacheService.LastCommit.TTLSeconds, cache.GetCache())
 
 	return commitCache.CacheCommit(commit)
 }
diff --git a/modules/setting/cache.go b/modules/setting/cache.go
index 2277c9eae..7bfea9196 100644
--- a/modules/setting/cache.go
+++ b/modules/setting/cache.go
@@ -49,7 +49,8 @@ var (
 	}
 )
 
-const memcachemax = 30 * 24 * time.Hour
+// MemcacheMaxTTL represents the maximum memcache TTL
+const MemcacheMaxTTL = 30 * 24 * time.Hour
 
 func newCacheService() {
 	sec := Cfg.Section("cache")
@@ -60,19 +61,8 @@ func newCacheService() {
 	CacheService.Adapter = sec.Key("ADAPTER").In("memory", []string{"memory", "redis", "memcache"})
 	switch CacheService.Adapter {
 	case "memory":
-	case "redis":
+	case "redis", "memcache":
 		CacheService.Conn = strings.Trim(sec.Key("HOST").String(), "\" ")
-	case "memcache":
-		CacheService.Conn = strings.Trim(sec.Key("HOST").String(), "\" ")
-		if CacheService.TTL > memcachemax {
-			log.Warn("Cache service: provided TTL %v is too long for memcache, defaulting to maximum of 30 days", CacheService.TTL)
-			CacheService.TTL = memcachemax
-		}
-		if CacheService.LastCommit.TTL > memcachemax {
-			log.Warn("Cache service: provided LastCommit cache TTL %v is too long for memcache, defaulting to maximum of 30 days", CacheService.LastCommit.TTL)
-
-			CacheService.LastCommit.TTL = memcachemax
-		}
 	case "": // disable cache
 		CacheService.Enabled = false
 	default:
@@ -98,3 +88,19 @@ func newCacheService() {
 		log.Info("Last Commit Cache Service Enabled")
 	}
 }
+
+// TTLSeconds returns the TTLSeconds or unix timestamp for memcache
+func (c Cache) TTLSeconds() int64 {
+	if c.Adapter == "memcache" && c.TTL > MemcacheMaxTTL {
+		return time.Now().Add(c.TTL).Unix()
+	}
+	return int64(c.TTL.Seconds())
+}
+
+// LastCommitCacheTTLSeconds returns the TTLSeconds or unix timestamp for memcache
+func LastCommitCacheTTLSeconds() int64 {
+	if CacheService.Adapter == "memcache" && CacheService.LastCommit.TTL > MemcacheMaxTTL {
+		return time.Now().Add(CacheService.LastCommit.TTL).Unix()
+	}
+	return int64(CacheService.LastCommit.TTL.Seconds())
+}
diff --git a/routers/repo/view.go b/routers/repo/view.go
index e50e4613b..a5e3cbe3e 100644
--- a/routers/repo/view.go
+++ b/routers/repo/view.go
@@ -140,7 +140,7 @@ func renderDirectory(ctx *context.Context, treeLink string) {
 
 	var c *git.LastCommitCache
 	if setting.CacheService.LastCommit.Enabled && ctx.Repo.CommitsCount >= setting.CacheService.LastCommit.CommitsCount {
-		c = git.NewLastCommitCache(ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, int64(setting.CacheService.LastCommit.TTL.Seconds()), cache.GetCache())
+		c = git.NewLastCommitCache(ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, setting.LastCommitCacheTTLSeconds, cache.GetCache())
 	}
 
 	var latestCommit *git.Commit

[zeripath]

Note the function passing into LastCommitCache...

[lunny]

Please also update docs on cache configuration.

[zeripath]

as discussed with @techknowlogick - have gone with changes suggested by @CirnoT

[zeripath]

Make lgtm work