From c4566a337b3009f7a216d4f9a8190404c46d2391 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Thu, 30 Apr 2020 00:24:41 +0300 Subject: [PATCH] Rework in_memory_textures_storage. Handle empty properties correctly --- Gopkg.lock | 9 --- Gopkg.toml | 4 -- di/db.go | 5 +- mojangtextures/in_memory_textures_storage.go | 70 +++++++------------ .../in_memory_textures_storage_test.go | 64 ++++++++--------- 5 files changed, 56 insertions(+), 96 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 418c689..477f6fb 100644 --- a/Gopkg.lock +++ b/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" diff --git a/Gopkg.toml b/Gopkg.toml index 78fccf1..6632bcf 100644 --- a/Gopkg.toml +++ b/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" diff --git a/di/db.go b/di/db.go index 32664d7..33bae47 100644 --- a/di/db.go +++ b/di/db.go @@ -66,8 +66,5 @@ func newFSFactory(config *viper.Viper) (*fs.Filesystem, error) { } func newMojangSignedTexturesStorage() mojangtextures.TexturesStorage { - texturesStorage := mojangtextures.NewInMemoryTexturesStorage() - texturesStorage.Start() - - return texturesStorage + return mojangtextures.NewInMemoryTexturesStorage() } diff --git a/mojangtextures/in_memory_textures_storage.go b/mojangtextures/in_memory_textures_storage.go index 7a95a4d..dc51735 100644 --- a/mojangtextures/in_memory_textures_storage.go +++ b/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 - data map[string]*inMemoryItem - working *abool.AtomicBool + once sync.Once + lock sync.RWMutex + 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 - if textures != nil { - decoded, err := textures.DecodeTextures() - if err != nil { - panic(err) - } - - timestamp = decoded.Timestamp - } else { - timestamp = unixNanoToUnixMicro(now().UnixNano()) - } + s.once.Do(s.start) 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 { diff --git a/mojangtextures/in_memory_textures_storage_test.go b/mojangtextures/in_memory_textures_storage_test.go index ac244ec..0f0733e 100644 --- a/mojangtextures/in_memory_textures_storage_test.go +++ b/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 { - return time.Now().Add(time.Minute * 2) - } + time.Sleep(storage.Duration * 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 - storage.Duration = 10 * time.Millisecond + defer storage.Stop() + 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() - defer storage.Stop() - - time.Sleep(storage.GCPeriod + time.Millisecond) // Let it start first iteration + time.Sleep(storage.GCPeriod) // 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")