Rework in_memory_textures_storage. Handle empty properties correctly

Gopkg.lock

@@ -277,14 +277,6 @@
   revision = "3ebf1ddaeb260c4b1ae502a01c7844fa8c1fa0e9"
   version = "v1.5.1"
 
-[[projects]]
-  branch = "master"
-  digest = "1:86e6712cfd4070a2120c03fcec41cfcbbc51813504a74e28d74479edfaf669ee"
-  name = "github.com/tevino/abool"
-  packages = ["."]
-  pruneopts = ""
-  revision = "9b9efcf221b50905aab9bbabd3daed56dc10f339"
-
 [[projects]]
   digest = "1:061754b9de261d8e1cf804970dff7b3e105d1cb4883ef446dbe911489ba8e9eb"
   name = "github.com/thedevsaddam/govalidator"
@@ -352,7 +344,6 @@
     "github.com/stretchr/testify/mock",
     "github.com/stretchr/testify/require",
     "github.com/stretchr/testify/suite",
-    "github.com/tevino/abool",
     "github.com/thedevsaddam/govalidator",
   ]
   solver-name = "gps-cdcl"

Gopkg.toml

@@ -32,10 +32,6 @@ ignored = ["github.com/elyby/chrly"]
   name = "github.com/thedevsaddam/govalidator"
   version = "^1.9.6"
 
-[[constraint]]
-  name = "github.com/tevino/abool"
-  branch = "master"
-
 [[constraint]]
   name = "github.com/asaskevich/EventBus"
   source = "https://github.com/erickskrauch/EventBus.git"

di/db.go

@@ -66,8 +66,5 @@ func newFSFactory(config *viper.Viper) (*fs.Filesystem, error) {
 }
 
 func newMojangSignedTexturesStorage() mojangtextures.TexturesStorage {
-	texturesStorage := mojangtextures.NewInMemoryTexturesStorage()
+	return mojangtextures.NewInMemoryTexturesStorage()
-	texturesStorage.Start()
-
-	return texturesStorage
 }

mojangtextures/in_memory_textures_storage.go

@@ -5,12 +5,8 @@ import (
 	"time"
 
 	"github.com/elyby/chrly/api/mojang"
-
-	"github.com/tevino/abool"
 )
 
-var now = time.Now
-
 type inMemoryItem struct {
 	textures  *mojang.SignedTexturesResponse
 	timestamp int64
@@ -20,9 +16,10 @@ type InMemoryTexturesStorage struct {
 	GCPeriod time.Duration
 	Duration time.Duration
 
-	lock    sync.RWMutex
+	once sync.Once
-	data    map[string]*inMemoryItem
+	lock sync.RWMutex
-	working *abool.AtomicBool
+	data map[string]*inMemoryItem
+	done chan struct{}
 }
 
 func NewInMemoryTexturesStorage() *InMemoryTexturesStorage {
@@ -35,30 +32,6 @@ func NewInMemoryTexturesStorage() *InMemoryTexturesStorage {
 	return storage
 }
 
-func (s *InMemoryTexturesStorage) Start() {
-	if s.working == nil {
-		s.working = abool.New()
-	}
-
-	if !s.working.IsSet() {
-		go func() {
-			time.Sleep(s.GCPeriod)
-			// TODO: this can be reimplemented in future with channels, but right now I have no idea how to make it right
-			for s.working.IsSet() {
-				start := time.Now()
-				s.gc()
-				time.Sleep(s.GCPeriod - time.Since(start))
-			}
-		}()
-	}
-
-	s.working.Set()
-}
-
-func (s *InMemoryTexturesStorage) Stop() {
-	s.working.UnSet()
-}
-
 func (s *InMemoryTexturesStorage) GetTextures(uuid string) (*mojang.SignedTexturesResponse, error) {
 	s.lock.RLock()
 	defer s.lock.RUnlock()
@@ -73,27 +46,36 @@ func (s *InMemoryTexturesStorage) GetTextures(uuid string) (*mojang.SignedTextur
 }
 
 func (s *InMemoryTexturesStorage) StoreTextures(uuid string, textures *mojang.SignedTexturesResponse) {
-	var timestamp int64
+	s.once.Do(s.start)
-	if textures != nil {
-		decoded, err := textures.DecodeTextures()
-		if err != nil {
-			panic(err)
-		}
-
-		timestamp = decoded.Timestamp
-	} else {
-		timestamp = unixNanoToUnixMicro(now().UnixNano())
-	}
 
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
 	s.data[uuid] = &inMemoryItem{
 		textures:  textures,
-		timestamp: timestamp,
+		timestamp: unixNanoToUnixMicro(time.Now().UnixNano()),
 	}
 }
 
+func (s *InMemoryTexturesStorage) start() {
+	s.done = make(chan struct{})
+	ticker := time.NewTicker(s.GCPeriod)
+	go func() {
+		for {
+			select {
+			case <-s.done:
+				return
+			case <-ticker.C:
+				s.gc()
+			}
+		}
+	}()
+}
+
+func (s *InMemoryTexturesStorage) Stop() {
+	close(s.done)
+}
+
 func (s *InMemoryTexturesStorage) gc() {
 	s.lock.Lock()
 	defer s.lock.Unlock()
@@ -107,7 +89,7 @@ func (s *InMemoryTexturesStorage) gc() {
 }
 
 func (s *InMemoryTexturesStorage) getMinimalNotExpiredTimestamp() int64 {
-	return unixNanoToUnixMicro(now().Add(s.Duration * time.Duration(-1)).UnixNano())
+	return unixNanoToUnixMicro(time.Now().Add(s.Duration * time.Duration(-1)).UnixNano())
 }
 
 func unixNanoToUnixMicro(unixNano int64) int64 {

mojangtextures/in_memory_textures_storage_test.go

@@ -64,18 +64,16 @@ func TestInMemoryTexturesStorage_GetTextures(t *testing.T) {
 
 	t.Run("should return nil, nil when textures are exists, but cache duration is expired", func(t *testing.T) {
 		storage := NewInMemoryTexturesStorage()
+		storage.Duration = 10 * time.Millisecond
+		storage.GCPeriod = time.Minute
 		storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithSkin)
 
-		now = func() time.Time {
+		time.Sleep(storage.Duration * 2)
-			return time.Now().Add(time.Minute * 2)
-		}
 
 		result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46")
 
 		assert.Nil(t, result)
 		assert.Nil(t, err)
-
-		now = time.Now
 	})
 }
 
@@ -100,6 +98,21 @@ func TestInMemoryTexturesStorage_StoreTextures(t *testing.T) {
 		assert.Nil(t, err)
 	})
 
+	t.Run("store textures with empty properties", func(t *testing.T) {
+		texturesWithEmptyProps := &mojang.SignedTexturesResponse{
+			Id:    "dead24f9a4fa4877b7b04c8c6c72bb46",
+			Name:  "mock",
+			Props: []*mojang.Property{},
+		}
+
+		storage := NewInMemoryTexturesStorage()
+		storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithEmptyProps)
+		result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46")
+
+		assert.Exactly(t, texturesWithEmptyProps, result)
+		assert.Nil(t, err)
+	})
+
 	t.Run("store nil textures", func(t *testing.T) {
 		storage := NewInMemoryTexturesStorage()
 		storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", nil)
@@ -108,31 +121,13 @@ func TestInMemoryTexturesStorage_StoreTextures(t *testing.T) {
 		assert.Nil(t, result)
 		assert.Nil(t, err)
 	})
-
-	t.Run("should panic if textures prop is not decoded", func(t *testing.T) {
-		toStore := &mojang.SignedTexturesResponse{
-			Id:   "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
-			Name: "mock",
-			Props: []*mojang.Property{
-				{
-					Name:      "textures",
-					Value:     "totally not base64 encoded json",
-					Signature: "totally not base64 encoded signature",
-				},
-			},
-		}
-
-		assert.Panics(t, func() {
-			storage := NewInMemoryTexturesStorage()
-			storage.StoreTextures("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", toStore)
-		})
-	})
 }
 
 func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) {
 	storage := NewInMemoryTexturesStorage()
-	storage.GCPeriod = 10 * time.Millisecond
+	defer storage.Stop()
-	storage.Duration = 10 * time.Millisecond
+	storage.GCPeriod = 40 * time.Millisecond
+	storage.Duration = 40 * time.Millisecond
 
 	textures1 := &mojang.SignedTexturesResponse{
 		Id:   "dead24f9a4fa4877b7b04c8c6c72bb46",
@@ -141,7 +136,7 @@ func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) {
 			{
 				Name: "textures",
 				Value: mojang.EncodeTextures(&mojang.TexturesProp{
-					Timestamp:   time.Now().Add(storage.GCPeriod-time.Millisecond*time.Duration(5)).UnixNano() / 10e5,
+					Timestamp:   time.Now().UnixNano() / 10e5,
 					ProfileID:   "dead24f9a4fa4877b7b04c8c6c72bb46",
 					ProfileName: "mock1",
 					Textures:    &mojang.TexturesResponse{},
@@ -149,6 +144,7 @@ func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) {
 			},
 		},
 	}
+
 	textures2 := &mojang.SignedTexturesResponse{
 		Id:   "b5d58475007d4f9e9ddd1403e2497579",
 		Name: "mock2",
@@ -156,7 +152,7 @@ func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) {
 			{
 				Name: "textures",
 				Value: mojang.EncodeTextures(&mojang.TexturesProp{
-					Timestamp:   time.Now().Add(storage.GCPeriod-time.Millisecond*time.Duration(15)).UnixNano() / 10e5,
+					Timestamp:   time.Now().UnixNano() / 10e5,
 					ProfileID:   "b5d58475007d4f9e9ddd1403e2497579",
 					ProfileName: "mock2",
 					Textures:    &mojang.TexturesResponse{},
@@ -166,22 +162,20 @@ func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) {
 	}
 
 	storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", textures1)
+	time.Sleep(storage.GCPeriod / 4) // Store second texture a bit later to avoid it removing by gc
 	storage.StoreTextures("b5d58475007d4f9e9ddd1403e2497579", textures2)
 
-	storage.Start()
+	time.Sleep(storage.GCPeriod) // Let it start first iteration
-	defer storage.Stop()
-
-	time.Sleep(storage.GCPeriod + time.Millisecond) // Let it start first iteration
 
 	texturesFromStorage1, textures1Err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46")
 	texturesFromStorage2, textures2Err := storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579")
 
-	assert.NotNil(t, texturesFromStorage1)
+	assert.Nil(t, texturesFromStorage1)
 	assert.Nil(t, textures1Err)
-	assert.Nil(t, texturesFromStorage2)
+	assert.NotNil(t, texturesFromStorage2)
 	assert.Nil(t, textures2Err)
 
-	time.Sleep(storage.GCPeriod + time.Millisecond) // Let another iteration happen
+	time.Sleep(storage.GCPeriod) // Let another iteration happen
 
 	texturesFromStorage1, textures1Err = storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46")
 	texturesFromStorage2, textures2Err = storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579")