diff --git a/db/redis/redis.go b/db/redis/redis.go index 7c38d52..54146bf 100644 --- a/db/redis/redis.go +++ b/db/redis/redis.go @@ -14,7 +14,6 @@ import ( "github.com/mediocregopher/radix.v2/util" "github.com/elyby/chrly/model" - "github.com/elyby/chrly/mojangtextures" ) var now = time.Now @@ -186,20 +185,21 @@ func removeByUsername(username string, conn util.Cmder) error { return nil } -func (db *Redis) GetUuid(username string) (string, error) { +func (db *Redis) GetUuid(username string) (string, bool, error) { conn, err := db.pool.Get() if err != nil { - return "", err + return "", false, err } defer db.pool.Put(conn) return findMojangUuidByUsername(username, conn) } -func findMojangUuidByUsername(username string, conn util.Cmder) (string, error) { - response := conn.Cmd("HGET", mojangUsernameToUuidKey, strings.ToLower(username)) +func findMojangUuidByUsername(username string, conn util.Cmder) (string, bool, error) { + key := strings.ToLower(username) + response := conn.Cmd("HGET", mojangUsernameToUuidKey, key) if response.IsType(redis.Nil) { - return "", &mojangtextures.ValueNotFound{} + return "", false, nil } data, _ := response.Str() @@ -207,10 +207,11 @@ func findMojangUuidByUsername(username string, conn util.Cmder) (string, error) timestamp, _ := strconv.ParseInt(parts[1], 10, 64) storedAt := time.Unix(timestamp, 0) if storedAt.Add(time.Hour * 24 * 30).Before(now()) { - return "", &mojangtextures.ValueNotFound{} + conn.Cmd("HDEL", mojangUsernameToUuidKey, key) + return "", false, nil } - return parts[0], nil + return parts[0], true, nil } func (db *Redis) StoreUuid(username string, uuid string) error { diff --git a/db/redis/redis_integration_test.go b/db/redis/redis_integration_test.go index 5bc566f..6228e29 100644 --- a/db/redis/redis_integration_test.go +++ b/db/redis/redis_integration_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/elyby/chrly/model" - "github.com/elyby/chrly/mojangtextures" ) const redisAddr = "localhost:6379" @@ -317,15 +316,30 @@ func (suite *redisTestSuite) TestGetUuid() { fmt.Sprintf("%s:%d", "d3ca513eb3e14946b58047f2bd3530fd", time.Now().Unix()), ) - uuid, err := suite.Redis.GetUuid("Mock") + uuid, found, err := suite.Redis.GetUuid("Mock") suite.Require().Nil(err) + suite.Require().True(found) suite.Require().Equal("d3ca513eb3e14946b58047f2bd3530fd", uuid) }) + suite.RunSubTest("exists record with empty uuid value", func() { + suite.cmd("HSET", + "hash:mojang-username-to-uuid", + "mock", + fmt.Sprintf(":%d", time.Now().Unix()), + ) + + uuid, found, err := suite.Redis.GetUuid("Mock") + suite.Require().Nil(err) + suite.Require().True(found) + suite.Require().Empty("", uuid) + }) + suite.RunSubTest("not exists record", func() { - uuid, err := suite.Redis.GetUuid("Mock") + uuid, found, err := suite.Redis.GetUuid("Mock") + suite.Require().Nil(err) + suite.Require().False(found) suite.Require().Empty(uuid) - suite.Require().IsType(new(mojangtextures.ValueNotFound), err) }) suite.RunSubTest("exists, but expired record", func() { @@ -335,9 +349,10 @@ func (suite *redisTestSuite) TestGetUuid() { fmt.Sprintf("%s:%d", "d3ca513eb3e14946b58047f2bd3530fd", time.Now().Add(-1*time.Hour*24*31).Unix()), ) - uuid, err := suite.Redis.GetUuid("Mock") + uuid, found, err := suite.Redis.GetUuid("Mock") suite.Require().Empty(uuid) - suite.Require().IsType(new(mojangtextures.ValueNotFound), err) + suite.Require().False(found) + suite.Require().Nil(err) }) } diff --git a/di/db.go b/di/db.go index 8f0738c..32664d7 100644 --- a/di/db.go +++ b/di/db.go @@ -22,7 +22,7 @@ import ( var db = di.Options( di.Provide(newRedis, di.As(new(http.SkinsRepository)), - di.As(new(mojangtextures.UuidsStorage)), + di.As(new(mojangtextures.UUIDsStorage)), ), di.Provide(newFSFactory, di.As(new(http.CapesRepository)), diff --git a/di/mojang_textures.go b/di/mojang_textures.go index 10cfdea..5620a6b 100644 --- a/di/mojang_textures.go +++ b/di/mojang_textures.go @@ -210,11 +210,11 @@ func newMojangSignedTexturesProvider(emitter mojangtextures.Emitter) mojangtextu } func newMojangTexturesStorageFactory( - uuidsStorage mojangtextures.UuidsStorage, + uuidsStorage mojangtextures.UUIDsStorage, texturesStorage mojangtextures.TexturesStorage, ) mojangtextures.Storage { return &mojangtextures.SeparatedStorage{ - UuidsStorage: uuidsStorage, + UUIDsStorage: uuidsStorage, TexturesStorage: texturesStorage, } } diff --git a/eventsubscribers/stats_reporter.go b/eventsubscribers/stats_reporter.go index 3c535d2..5216676 100644 --- a/eventsubscribers/stats_reporter.go +++ b/eventsubscribers/stats_reporter.go @@ -34,8 +34,8 @@ func (s *StatsReporter) ConfigureWithDispatcher(d Subscriber) { // Mojang signed textures source events d.Subscribe("mojang_textures:call", s.incCounterHandler("mojang_textures.request")) - d.Subscribe("mojang_textures:usernames:after_cache", func(username string, uuid string, err error) { - if err != nil { + d.Subscribe("mojang_textures:usernames:after_cache", func(username string, uuid string, found bool, err error) { + if err != nil || !found { return } diff --git a/eventsubscribers/stats_reporter_test.go b/eventsubscribers/stats_reporter_test.go index 33eb4ed..83eedf7 100644 --- a/eventsubscribers/stats_reporter_test.go +++ b/eventsubscribers/stats_reporter_test.go @@ -213,13 +213,19 @@ var statsReporterTestCases = []*StatsReporterTestCase{ }, { Events: [][]interface{}{ - {"mojang_textures:usernames:after_cache", "username", "", errors.New("error")}, + {"mojang_textures:usernames:after_cache", "username", "", false, errors.New("error")}, }, ExpectedCalls: [][]interface{}{}, }, { Events: [][]interface{}{ - {"mojang_textures:usernames:after_cache", "username", "", nil}, + {"mojang_textures:usernames:after_cache", "username", "", false, nil}, + }, + ExpectedCalls: [][]interface{}{}, + }, + { + Events: [][]interface{}{ + {"mojang_textures:usernames:after_cache", "username", "", true, nil}, }, ExpectedCalls: [][]interface{}{ {"IncCounter", "mojang_textures:usernames:cache_hit_nil", int64(1)}, @@ -227,7 +233,7 @@ var statsReporterTestCases = []*StatsReporterTestCase{ }, { Events: [][]interface{}{ - {"mojang_textures:usernames:after_cache", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", nil}, + {"mojang_textures:usernames:after_cache", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true, nil}, }, ExpectedCalls: [][]interface{}{ {"IncCounter", "mojang_textures:usernames:cache_hit", int64(1)}, diff --git a/mojangtextures/in_memory_textures_storage.go b/mojangtextures/in_memory_textures_storage.go index e5b72b2..14a33a0 100644 --- a/mojangtextures/in_memory_textures_storage.go +++ b/mojangtextures/in_memory_textures_storage.go @@ -66,7 +66,7 @@ func (s *InMemoryTexturesStorage) GetTextures(uuid string) (*mojang.SignedTextur item, exists := s.data[uuid] validRange := s.getMinimalNotExpiredTimestamp() if !exists || validRange > item.timestamp { - return nil, &ValueNotFound{} + return nil, nil } return item.textures, nil diff --git a/mojangtextures/in_memory_textures_storage_test.go b/mojangtextures/in_memory_textures_storage_test.go index 3ad6789..8a178c8 100644 --- a/mojangtextures/in_memory_textures_storage_test.go +++ b/mojangtextures/in_memory_textures_storage_test.go @@ -1,12 +1,12 @@ package mojangtextures import ( + "testing" "time" - "github.com/elyby/chrly/api/mojang" + assert "github.com/stretchr/testify/require" - testify "github.com/stretchr/testify/assert" - "testing" + "github.com/elyby/chrly/api/mojang" ) var texturesWithSkin = &mojang.SignedTexturesResponse{ @@ -45,30 +45,24 @@ var texturesWithoutSkin = &mojang.SignedTexturesResponse{ } func TestInMemoryTexturesStorage_GetTextures(t *testing.T) { - t.Run("get error when uuid is not exists", func(t *testing.T) { - assert := testify.New(t) - + t.Run("should return nil, nil when textures are unavailable", func(t *testing.T) { storage := NewInMemoryTexturesStorage() result, err := storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579") - assert.Nil(result) - assert.Error(err, "value not found in the storage") + assert.Nil(t, result) + assert.Nil(t, err) }) t.Run("get textures object, when uuid is stored in the storage", func(t *testing.T) { - assert := testify.New(t) - storage := NewInMemoryTexturesStorage() storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithSkin) result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - assert.Equal(texturesWithSkin, result) - assert.Nil(err) + assert.Equal(t, texturesWithSkin, result) + assert.Nil(t, err) }) - t.Run("get error when uuid is exists, but textures are expired", func(t *testing.T) { - assert := testify.New(t) - + t.Run("should return nil, nil when textures are exists, but cache duration is expired", func(t *testing.T) { storage := NewInMemoryTexturesStorage() storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithSkin) @@ -78,8 +72,8 @@ func TestInMemoryTexturesStorage_GetTextures(t *testing.T) { result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - assert.Nil(result) - assert.Error(err, "value not found in the storage") + assert.Nil(t, result) + assert.Nil(t, err) now = time.Now }) @@ -87,50 +81,42 @@ func TestInMemoryTexturesStorage_GetTextures(t *testing.T) { func TestInMemoryTexturesStorage_StoreTextures(t *testing.T) { t.Run("store textures for previously not existed uuid", func(t *testing.T) { - assert := testify.New(t) - storage := NewInMemoryTexturesStorage() storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithSkin) result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - assert.Equal(texturesWithSkin, result) - assert.Nil(err) + assert.Equal(t, texturesWithSkin, result) + assert.Nil(t, err) }) t.Run("override already existed textures for uuid", func(t *testing.T) { - assert := testify.New(t) - storage := NewInMemoryTexturesStorage() storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithoutSkin) storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", texturesWithSkin) result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - assert.NotEqual(texturesWithoutSkin, result) - assert.Equal(texturesWithSkin, result) - assert.Nil(err) + assert.NotEqual(t, texturesWithoutSkin, result) + assert.Equal(t, texturesWithSkin, result) + assert.Nil(t, err) }) t.Run("store nil textures", func(t *testing.T) { - assert := testify.New(t) - storage := NewInMemoryTexturesStorage() storage.StoreTextures("dead24f9a4fa4877b7b04c8c6c72bb46", nil) result, err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - assert.Nil(result) - assert.Nil(err) + assert.Nil(t, result) + assert.Nil(t, err) }) t.Run("should panic if textures prop is not decoded", func(t *testing.T) { - assert := testify.New(t) - toStore := &mojang.SignedTexturesResponse{ Id: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", Name: "mock", Props: []*mojang.Property{}, } - assert.PanicsWithValue("unable to decode textures", func() { + assert.PanicsWithValue(t, "unable to decode textures", func() { storage := NewInMemoryTexturesStorage() storage.StoreTextures("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", toStore) }) @@ -138,8 +124,6 @@ func TestInMemoryTexturesStorage_StoreTextures(t *testing.T) { } func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) { - assert := testify.New(t) - storage := NewInMemoryTexturesStorage() storage.GCPeriod = 10 * time.Millisecond storage.Duration = 10 * time.Millisecond @@ -179,22 +163,25 @@ func TestInMemoryTexturesStorage_GarbageCollection(t *testing.T) { storage.StoreTextures("b5d58475007d4f9e9ddd1403e2497579", textures2) storage.Start() + defer storage.Stop() time.Sleep(storage.GCPeriod + time.Millisecond) // Let it start first iteration - _, textures1Err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - _, textures2Err := storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579") + texturesFromStorage1, textures1Err := storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") + texturesFromStorage2, textures2Err := storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579") - assert.Nil(textures1Err) - assert.Error(textures2Err) + assert.NotNil(t, texturesFromStorage1) + assert.Nil(t, textures1Err) + assert.Nil(t, texturesFromStorage2) + assert.Nil(t, textures2Err) time.Sleep(storage.GCPeriod + time.Millisecond) // Let another iteration happen - _, textures1Err = storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") - _, textures2Err = storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579") + texturesFromStorage1, textures1Err = storage.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") + texturesFromStorage2, textures2Err = storage.GetTextures("b5d58475007d4f9e9ddd1403e2497579") - assert.Error(textures1Err) - assert.Error(textures2Err) - - storage.Stop() + assert.Nil(t, texturesFromStorage1) + assert.Nil(t, textures1Err) + assert.Nil(t, texturesFromStorage2) + assert.Nil(t, textures2Err) } diff --git a/mojangtextures/mojang_textures.go b/mojangtextures/mojang_textures.go index 205431c..c5a3978 100644 --- a/mojangtextures/mojang_textures.go +++ b/mojangtextures/mojang_textures.go @@ -98,14 +98,18 @@ func (ctx *Provider) GetForUsername(username string) (*mojang.SignedTexturesResp username = strings.ToLower(username) ctx.Emit("mojang_textures:call", username) - uuid, err := ctx.getUuidFromCache(username) - if err == nil && uuid == "" { + uuid, found, err := ctx.getUuidFromCache(username) + if err != nil { + return nil, err + } + + if found && uuid == "" { return nil, nil } if uuid != "" { textures, err := ctx.getTexturesFromCache(uuid) - if err == nil { + if err == nil && textures != nil { return textures, nil } } @@ -162,12 +166,12 @@ func (ctx *Provider) getResult(username string, uuid string) *broadcastResult { return &broadcastResult{textures, nil} } -func (ctx *Provider) getUuidFromCache(username string) (string, error) { +func (ctx *Provider) getUuidFromCache(username string) (string, bool, error) { ctx.Emit("mojang_textures:usernames:before_cache", username) - uuid, err := ctx.Storage.GetUuid(username) - ctx.Emit("mojang_textures:usernames:after_cache", username, uuid, err) + uuid, found, err := ctx.Storage.GetUuid(username) + ctx.Emit("mojang_textures:usernames:after_cache", username, uuid, found, err) - return uuid, err + return uuid, found, err } func (ctx *Provider) getTexturesFromCache(uuid string) (*mojang.SignedTexturesResponse, error) { diff --git a/mojangtextures/mojang_textures_test.go b/mojangtextures/mojang_textures_test.go index b5dca95..cbbb07e 100644 --- a/mojangtextures/mojang_textures_test.go +++ b/mojangtextures/mojang_textures_test.go @@ -122,9 +122,9 @@ type mockStorage struct { mock.Mock } -func (m *mockStorage) GetUuid(username string) (string, error) { +func (m *mockStorage) GetUuid(username string) (string, bool, error) { args := m.Called(username) - return args.String(0), args.Error(1) + return args.String(0), args.Bool(1), args.Error(2) } func (m *mockStorage) StoreUuid(username string, uuid string) error { @@ -186,7 +186,7 @@ func (suite *providerTestSuite) TestGetForUsernameWithoutAnyCache() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", &ValueNotFound{}).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, nil).Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:after_call", "username", expectedProfile, nil).Once() @@ -194,7 +194,7 @@ func (suite *providerTestSuite) TestGetForUsernameWithoutAnyCache() { suite.Emitter.On("Emit", "mojang_textures:textures:after_call", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult, nil).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, nil).Once() - suite.Storage.On("GetUuid", "username").Once().Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Once().Return("", false, nil) suite.Storage.On("StoreUuid", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil) suite.Storage.On("StoreTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult).Once() @@ -213,16 +213,16 @@ func (suite *providerTestSuite) TestGetForUsernameWithCachedUuid() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", nil).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true, nil).Once() suite.Emitter.On("Emit", "mojang_textures:textures:before_cache", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once() - suite.Emitter.On("Emit", "mojang_textures:textures:after_cache", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedCachedTextures, &ValueNotFound{}).Once() + suite.Emitter.On("Emit", "mojang_textures:textures:after_cache", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedCachedTextures, nil).Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once() suite.Emitter.On("Emit", "mojang_textures:textures:before_call", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once() suite.Emitter.On("Emit", "mojang_textures:textures:after_call", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult, nil).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, nil).Once() - suite.Storage.On("GetUuid", "username").Once().Return("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", nil) - suite.Storage.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Once().Return("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true, nil) + suite.Storage.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, nil) suite.Storage.On("StoreTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult).Once() suite.TexturesProvider.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Return(expectedResult, nil) @@ -238,11 +238,11 @@ func (suite *providerTestSuite) TestGetForUsernameWithFullyCachedResult() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", nil).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true, nil).Once() suite.Emitter.On("Emit", "mojang_textures:textures:before_cache", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once() suite.Emitter.On("Emit", "mojang_textures:textures:after_cache", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult, nil).Once() - suite.Storage.On("GetUuid", "username").Once().Return("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", nil) + suite.Storage.On("GetUuid", "username").Once().Return("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true, nil) suite.Storage.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(expectedResult, nil) result, err := suite.Provider.GetForUsername("username") @@ -254,9 +254,9 @@ func (suite *providerTestSuite) TestGetForUsernameWithFullyCachedResult() { func (suite *providerTestSuite) TestGetForUsernameWithCachedUnknownUuid() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", nil).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", true, nil).Once() - suite.Storage.On("GetUuid", "username").Once().Return("", nil) + suite.Storage.On("GetUuid", "username").Once().Return("", true, nil) result, err := suite.Provider.GetForUsername("username") @@ -270,13 +270,13 @@ func (suite *providerTestSuite) TestGetForUsernameWhichHasNoMojangAccount() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", &ValueNotFound{}).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, nil).Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:after_call", "username", expectedProfile, nil).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, nil).Once() - suite.Storage.On("GetUuid", "username").Once().Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Once().Return("", false, nil) suite.Storage.On("StoreUuid", "username", "").Once().Return(nil) suite.UuidsProvider.On("GetUuid", "username").Once().Return(nil, nil) @@ -293,7 +293,7 @@ func (suite *providerTestSuite) TestGetForUsernameWhichHasMojangAccountButHasNoM suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", &ValueNotFound{}).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, nil).Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:after_call", "username", expectedProfile, nil).Once() @@ -301,7 +301,7 @@ func (suite *providerTestSuite) TestGetForUsernameWhichHasMojangAccountButHasNoM suite.Emitter.On("Emit", "mojang_textures:textures:after_call", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult, nil).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, nil).Once() - suite.Storage.On("GetUuid", "username").Once().Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Once().Return("", false, nil) suite.Storage.On("StoreUuid", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil) suite.Storage.On("StoreTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult).Once() @@ -320,7 +320,7 @@ func (suite *providerTestSuite) TestGetForTheSameUsernames() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Twice() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Twice() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", &ValueNotFound{}).Twice() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, nil).Twice() suite.Emitter.On("Emit", "mojang_textures:already_processing", "username").Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_call", "username").Once() @@ -329,7 +329,7 @@ func (suite *providerTestSuite) TestGetForTheSameUsernames() { suite.Emitter.On("Emit", "mojang_textures:textures:after_call", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult, nil).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, nil).Once() - suite.Storage.On("GetUuid", "username").Twice().Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Twice().Return("", false, nil) suite.Storage.On("StoreUuid", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil) suite.Storage.On("StoreTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult).Once() @@ -359,6 +359,21 @@ func (suite *providerTestSuite) TestGetForNotAllowedMojangUsername() { suite.Assert().Nil(result) } +func (suite *providerTestSuite) TestGetErrorFromUUIDsStorage() { + expectedErr := errors.New("mock error") + + suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, expectedErr).Once() + + suite.Storage.On("GetUuid", "username").Once().Return("", false, expectedErr) + + result, err := suite.Provider.GetForUsername("username") + + suite.Assert().Nil(result) + suite.Assert().Equal(expectedErr, err) +} + func (suite *providerTestSuite) TestGetErrorFromUuidsProvider() { var expectedProfile *mojang.ProfileInfo var expectedResult *mojang.SignedTexturesResponse @@ -366,13 +381,13 @@ func (suite *providerTestSuite) TestGetErrorFromUuidsProvider() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", &ValueNotFound{}).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, nil).Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:after_call", "username", expectedProfile, err).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, err).Once() - suite.Storage.On("GetUuid", "username").Once().Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Once().Return("", false, nil) suite.UuidsProvider.On("GetUuid", "username").Once().Return(nil, err) result, resErr := suite.Provider.GetForUsername("username") @@ -387,7 +402,7 @@ func (suite *providerTestSuite) TestGetErrorFromTexturesProvider() { suite.Emitter.On("Emit", "mojang_textures:call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_cache", "username").Once() - suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", &ValueNotFound{}).Once() + suite.Emitter.On("Emit", "mojang_textures:usernames:after_cache", "username", "", false, nil).Once() suite.Emitter.On("Emit", "mojang_textures:before_result", "username", "").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:before_call", "username").Once() suite.Emitter.On("Emit", "mojang_textures:usernames:after_call", "username", expectedProfile, nil).Once() @@ -395,7 +410,7 @@ func (suite *providerTestSuite) TestGetErrorFromTexturesProvider() { suite.Emitter.On("Emit", "mojang_textures:textures:after_call", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", expectedResult, err).Once() suite.Emitter.On("Emit", "mojang_textures:after_result", "username", expectedResult, err).Once() - suite.Storage.On("GetUuid", "username").Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "username").Return("", false, nil) suite.Storage.On("StoreUuid", "username", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Return(nil) suite.UuidsProvider.On("GetUuid", "username").Once().Return(expectedProfile, nil) suite.TexturesProvider.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, err) diff --git a/mojangtextures/storage.go b/mojangtextures/storage.go index d73249a..7553b4b 100644 --- a/mojangtextures/storage.go +++ b/mojangtextures/storage.go @@ -4,12 +4,13 @@ import ( "github.com/elyby/chrly/api/mojang" ) -// UuidsStorage is a key-value storage of Mojang usernames pairs to its UUIDs, +// UUIDsStorage is a key-value storage of Mojang usernames pairs to its UUIDs, // used to reduce the load on the account information queue -type UuidsStorage interface { - // Since only primitive types are used in this method, you should return a special error ValueNotFound - // to return the information that no error has occurred and username does not have uuid - GetUuid(username string) (string, error) +type UUIDsStorage interface { + // The second argument indicates whether a record was found in the storage, + // since depending on it, the empty value must be interpreted as "no cached record" + // or "value cached and has an empty value" + GetUuid(username string) (uuid string, found bool, err error) // An empty uuid value can be passed if the corresponding account has not been found StoreUuid(username string, uuid string) error } @@ -24,23 +25,23 @@ type TexturesStorage interface { } type Storage interface { - UuidsStorage + UUIDsStorage TexturesStorage } // SeparatedStorage allows you to use separate storage engines to satisfy // the Storage interface type SeparatedStorage struct { - UuidsStorage + UUIDsStorage TexturesStorage } -func (s *SeparatedStorage) GetUuid(username string) (string, error) { - return s.UuidsStorage.GetUuid(username) +func (s *SeparatedStorage) GetUuid(username string) (string, bool, error) { + return s.UUIDsStorage.GetUuid(username) } func (s *SeparatedStorage) StoreUuid(username string, uuid string) error { - return s.UuidsStorage.StoreUuid(username, uuid) + return s.UUIDsStorage.StoreUuid(username, uuid) } func (s *SeparatedStorage) GetTextures(uuid string) (*mojang.SignedTexturesResponse, error) { @@ -50,12 +51,3 @@ func (s *SeparatedStorage) GetTextures(uuid string) (*mojang.SignedTexturesRespo func (s *SeparatedStorage) StoreTextures(uuid string, textures *mojang.SignedTexturesResponse) { s.TexturesStorage.StoreTextures(uuid, textures) } - -// This error can be used to indicate, that requested -// value doesn't exists in the storage -type ValueNotFound struct { -} - -func (*ValueNotFound) Error() string { - return "value not found in the storage" -} diff --git a/mojangtextures/storage_test.go b/mojangtextures/storage_test.go index 1b82349..cdb9777 100644 --- a/mojangtextures/storage_test.go +++ b/mojangtextures/storage_test.go @@ -12,9 +12,9 @@ type uuidsStorageMock struct { mock.Mock } -func (m *uuidsStorageMock) GetUuid(username string) (string, error) { +func (m *uuidsStorageMock) GetUuid(username string) (string, bool, error) { args := m.Called(username) - return args.String(0), args.Error(1) + return args.String(0), args.Bool(1), args.Error(2) } func (m *uuidsStorageMock) StoreUuid(username string, uuid string) error { @@ -50,9 +50,10 @@ func TestSplittedStorage(t *testing.T) { t.Run("GetUuid", func(t *testing.T) { storage, uuidsMock, _ := createMockedStorage() - uuidsMock.On("GetUuid", "username").Once().Return("find me", nil) - result, err := storage.GetUuid("username") + uuidsMock.On("GetUuid", "username").Once().Return("find me", true, nil) + result, found, err := storage.GetUuid("username") assert.Nil(t, err) + assert.True(t, found) assert.Equal(t, "find me", result) uuidsMock.AssertExpectations(t) }) @@ -82,8 +83,3 @@ func TestSplittedStorage(t *testing.T) { texturesMock.AssertExpectations(t) }) } - -func TestValueNotFound_Error(t *testing.T) { - err := &ValueNotFound{} - assert.Equal(t, "value not found in the storage", err.Error()) -}