From e751bb8678a5d7203de2376ad14a929af3b5b7a3 Mon Sep 17 00:00:00 2001
From: Umputun <umputun@gmail.com>
Date: Mon, 18 Jun 2018 16:21:39 -0500
Subject: [PATCH] fail on bad cache options

---
 app/rest/cache/memory.go      |  5 +++--
 app/rest/cache/memory_test.go | 11 +++++++++++
 app/rest/cache/options.go     | 11 +++++++++++
 app/store/service/service.go  |  3 +--
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/app/rest/cache/memory.go b/app/rest/cache/memory.go
index 22a3f57e..1426f999 100644
--- a/app/rest/cache/memory.go
+++ b/app/rest/cache/memory.go
@@ -27,7 +27,7 @@ func NewMemoryCache(options ...Option) (LoadingCache, error) {
 	}
 	for _, opt := range options {
 		if err := opt(&res); err != nil {
-			log.Printf("[WARN] failed to set cache option, %v", err)
+			return nil, errors.Wrap(err, "failed to set cache option")
 		}
 	}
 
@@ -42,7 +42,8 @@ func NewMemoryCache(options ...Option) (LoadingCache, error) {
 		return nil, errors.Wrap(err, "failed to make cache")
 	}
 
-	log.Printf("[DEBUG] create lru cache, maxKeys=%d, maxValueSize=%d", res.maxKeys, res.maxValueSize)
+	log.Printf("[DEBUG] create lru cache, maxKeys=%d, maxValueSize=%d, maxCacheSize=%d",
+		res.maxKeys, res.maxValueSize, res.maxCacheSize)
 	return &res, nil
 }
 
diff --git a/app/rest/cache/memory_test.go b/app/rest/cache/memory_test.go
index 90396b5c..eb03bad5 100644
--- a/app/rest/cache/memory_test.go
+++ b/app/rest/cache/memory_test.go
@@ -297,3 +297,14 @@ func TestMemoryCache_FlushFailed(t *testing.T) {
 	lc.Flush("invalid-composite")
 	assert.Equal(t, 1, lc.(*memoryCache).bytesCache.Len())
 }
+
+func TestMemoryCache_BadOptions(t *testing.T) {
+	_, err := NewMemoryCache(MaxCacheSize(-1))
+	assert.EqualError(t, err, "failed to set cache option: negative size or MaxCacheSize, -1")
+
+	_, err = NewMemoryCache(MaxKeys(-1))
+	assert.EqualError(t, err, "failed to set cache option: negative size for MaxKeys, -1")
+
+	_, err = NewMemoryCache(MaxValSize(-1))
+	assert.EqualError(t, err, "failed to set cache option: negative size for MaxValSize, -1")
+}
diff --git a/app/rest/cache/options.go b/app/rest/cache/options.go
index 2902b0fa..89a9c5a3 100644
--- a/app/rest/cache/options.go
+++ b/app/rest/cache/options.go
@@ -1,5 +1,7 @@
 package cache
 
+import "github.com/pkg/errors"
+
 // Option func type
 type Option func(lc *memoryCache) error
 
@@ -8,6 +10,9 @@ type Option func(lc *memoryCache) error
 func MaxValSize(max int) Option {
 	return func(lc *memoryCache) error {
 		lc.maxValueSize = max
+		if max <= 0 {
+			return errors.Errorf("negative size for MaxValSize, %d", max)
+		}
 		return nil
 	}
 }
@@ -17,6 +22,9 @@ func MaxValSize(max int) Option {
 func MaxKeys(max int) Option {
 	return func(lc *memoryCache) error {
 		lc.maxKeys = max
+		if max <= 0 {
+			return errors.Errorf("negative size for MaxKeys, %d", max)
+		}
 		return nil
 	}
 }
@@ -26,6 +34,9 @@ func MaxKeys(max int) Option {
 func MaxCacheSize(max int64) Option {
 	return func(lc *memoryCache) error {
 		lc.maxCacheSize = max
+		if max <= 0 {
+			return errors.Errorf("negative size or MaxCacheSize, %d", max)
+		}
 		return nil
 	}
 }
diff --git a/app/store/service/service.go b/app/store/service/service.go
index 44acc410..1644b729 100644
--- a/app/store/service/service.go
+++ b/app/store/service/service.go
@@ -62,8 +62,7 @@ func (s *DataStore) SetPin(locator store.Locator, commentID string, status bool)
 func (s *DataStore) Vote(locator store.Locator, commentID string, userID string, val bool) (comment store.Comment, err error) {
 
 	cLock := s.getsScopedLocks(locator.URL) // get lock for URL scope
-
-	cLock.Lock()
+	cLock.Lock()                            // prevents race on voting
 	defer cLock.Unlock()
 
 	comment, err = s.Get(locator, commentID)