From e7c0fac34655672ef7cbe1bd3fdcef553cd6257c Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Sat, 20 Apr 2019 22:22:02 +0300 Subject: [PATCH] #1: Split textures processing to 2 separate steps --- api/mojang/queue/queue.go | 64 +++++++++++------ api/mojang/queue/queue_test.go | 125 ++++++++++++++++++++++++++------- api/mojang/queue/storage.go | 29 +++++--- 3 files changed, 160 insertions(+), 58 deletions(-) diff --git a/api/mojang/queue/queue.go b/api/mojang/queue/queue.go index fb83462..8739c9e 100644 --- a/api/mojang/queue/queue.go +++ b/api/mojang/queue/queue.go @@ -44,10 +44,10 @@ func (ctx *JobsQueue) GetTexturesForUsername(username string) chan *mojang.Signe return responseChan } - cachedResult := ctx.Storage.Get(username) - if cachedResult != nil { + uuid, err := ctx.Storage.GetUuid(username) + if err == nil && uuid == "" { go func() { - responseChan <- cachedResult + responseChan <- nil close(responseChan) }() @@ -56,9 +56,16 @@ func (ctx *JobsQueue) GetTexturesForUsername(username string) chan *mojang.Signe isFirstListener := ctx.broadcast.AddListener(username, responseChan) if isFirstListener { + // TODO: respond nil if processing takes more than 5 seconds + resultChan := make(chan *mojang.SignedTexturesResponse) - ctx.queue.Enqueue(&jobItem{username, resultChan}) - // TODO: return nil if processing takes more than 5 seconds + if uuid == "" { + ctx.queue.Enqueue(&jobItem{username, resultChan}) + } else { + go func() { + resultChan <- ctx.getTextures(uuid) + }() + } go func() { result := <-resultChan @@ -108,9 +115,8 @@ func (ctx *JobsQueue) queueRound() { for _, job := range jobs { wg.Add(1) go func(job *jobItem) { - var result *mojang.SignedTexturesResponse - shouldCache := true var uuid string + // Profiles in response not ordered, so we must search each username over full array for _, profile := range profiles { if strings.EqualFold(job.Username, profile.Name) { uuid = profile.Id @@ -118,27 +124,39 @@ func (ctx *JobsQueue) queueRound() { } } - if uuid != "" { - var err error - result, err = uuidToTextures(uuid, true) - if err != nil { - if _, ok := err.(*mojang.TooManyRequestsError); !ok { - panic(err) - } - - shouldCache = false - } + ctx.Storage.StoreUuid(job.Username, uuid) + if uuid == "" { + job.RespondTo <- nil + } else { + job.RespondTo <- ctx.getTextures(uuid) } wg.Done() - - if shouldCache && result != nil { - ctx.Storage.Set(result) - } - - job.RespondTo <- result }(job) } wg.Wait() } + +func (ctx *JobsQueue) getTextures(uuid string) *mojang.SignedTexturesResponse { + existsTextures, err := ctx.Storage.GetTextures(uuid) + if err == nil { + return existsTextures + } + + shouldCache := true + result, err := uuidToTextures(uuid, true) + if err != nil { + if _, ok := err.(*mojang.TooManyRequestsError); !ok { + panic(err) + } + + shouldCache = false + } + + if shouldCache && result != nil { + ctx.Storage.StoreTextures(result) + } + + return result +} diff --git a/api/mojang/queue/queue_test.go b/api/mojang/queue/queue_test.go index c94858b..f288edf 100644 --- a/api/mojang/queue/queue_test.go +++ b/api/mojang/queue/queue_test.go @@ -3,14 +3,12 @@ package queue import ( "crypto/rand" "encoding/base64" - "strings" - "time" - "github.com/elyby/chrly/api/mojang" - "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" + "strings" "testing" + "time" ) type MojangApiMocks struct { @@ -41,17 +39,26 @@ type MockStorage struct { mock.Mock } -func (m *MockStorage) Get(username string) *mojang.SignedTexturesResponse { +func (m *MockStorage) GetUuid(username string) (string, error) { args := m.Called(username) + return args.String(0), args.Error(1) +} + +func (m *MockStorage) StoreUuid(username string, uuid string) { + m.Called(username, uuid) +} + +func (m *MockStorage) GetTextures(uuid string) (*mojang.SignedTexturesResponse, error) { + args := m.Called(uuid) var result *mojang.SignedTexturesResponse if casted, ok := args.Get(0).(*mojang.SignedTexturesResponse); ok { result = casted } - return result + return result, args.Error(1) } -func (m *MockStorage) Set(textures *mojang.SignedTexturesResponse) { +func (m *MockStorage) StoreTextures(textures *mojang.SignedTexturesResponse) { m.Called(textures) } @@ -99,11 +106,13 @@ func (suite *QueueTestSuite) TearDownTest() { suite.Storage.AssertExpectations(suite.T()) } -func (suite *QueueTestSuite) TestReceiveTexturesForOneUsername() { +func (suite *QueueTestSuite) TestReceiveTexturesForOneUsernameWithoutAnyCache() { expectedResult := &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"} - suite.Storage.On("Get", mock.Anything).Return(nil) - suite.Storage.On("Set", expectedResult).Once() + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", "maksimkurb", "0d252b7218b648bfb86c2ae476954d32").Once() + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("StoreTextures", expectedResult).Once() suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{ {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, }, nil) @@ -117,13 +126,18 @@ func (suite *QueueTestSuite) TestReceiveTexturesForOneUsername() { suite.Assert().Equal(expectedResult, result) } -func (suite *QueueTestSuite) TestReceiveTexturesForFewUsernames() { +func (suite *QueueTestSuite) TestReceiveTexturesForFewUsernamesWithoutAnyCache() { expectedResult1 := &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"} expectedResult2 := &mojang.SignedTexturesResponse{Id: "4566e69fc90748ee8d71d7ba5aa00d20", Name: "Thinkofdeath"} - suite.Storage.On("Get", mock.Anything).Return(nil) - suite.Storage.On("Set", expectedResult1).Once() - suite.Storage.On("Set", expectedResult2).Once() + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("", &ValueNotFound{}) + suite.Storage.On("GetUuid", "Thinkofdeath").Once().Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", "maksimkurb", "0d252b7218b648bfb86c2ae476954d32").Once() + suite.Storage.On("StoreUuid", "Thinkofdeath", "4566e69fc90748ee8d71d7ba5aa00d20").Once() + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("GetTextures", "4566e69fc90748ee8d71d7ba5aa00d20").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("StoreTextures", expectedResult1).Once() + suite.Storage.On("StoreTextures", expectedResult2).Once() suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb", "Thinkofdeath"}).Once().Return([]*mojang.ProfileInfo{ {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, {Id: "4566e69fc90748ee8d71d7ba5aa00d20", Name: "Thinkofdeath"}, @@ -140,14 +154,66 @@ func (suite *QueueTestSuite) TestReceiveTexturesForFewUsernames() { suite.Assert().Equal(expectedResult2, <-resultChan2) } +func (suite *QueueTestSuite) TestReceiveTexturesForUsernameWithCachedUuid() { + expectedResult := &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"} + + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("0d252b7218b648bfb86c2ae476954d32", nil) + // Storage.StoreUuid shouldn't be called + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("StoreTextures", expectedResult).Once() + // MojangApi.UsernameToUuids shouldn't be called + suite.MojangApi.On("UuidToTextures", "0d252b7218b648bfb86c2ae476954d32", true).Once().Return(expectedResult, nil) + + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + // Note that there is no iteration + + result := <-resultChan + suite.Assert().Equal(expectedResult, result) +} + +func (suite *QueueTestSuite) TestReceiveTexturesForUsernameWithFullyCachedResult() { + expectedResult := &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"} + + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("0d252b7218b648bfb86c2ae476954d32", nil) + // Storage.StoreUuid shouldn't be called + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(expectedResult, nil) + // Storage.StoreTextures shouldn't be called + // MojangApi.UsernameToUuids shouldn't be called + // MojangApi.UuidToTextures shouldn't be called + + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + // Note that there is no iteration + + result := <-resultChan + suite.Assert().Equal(expectedResult, result) +} + +func (suite *QueueTestSuite) TestReceiveTexturesForUsernameWithCachedUnknownUuid() { + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("", nil) + // Storage.StoreUuid shouldn't be called + // Storage.GetTextures shouldn't be called + // Storage.StoreTextures shouldn't be called + // MojangApi.UsernameToUuids shouldn't be called + // MojangApi.UuidToTextures shouldn't be called + + resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") + + // Note that there is no iteration + + suite.Assert().Nil(<-resultChan) +} + func (suite *QueueTestSuite) TestReceiveTexturesForMoreThan100Usernames() { usernames := make([]string, 120, 120) for i := 0; i < 120; i++ { usernames[i] = randStr(8) } - suite.Storage.On("Get", mock.Anything).Times(120).Return(nil) - // Storage.Set shouldn't be called + suite.Storage.On("GetUuid", mock.Anything).Times(120).Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", mock.Anything, "").Times(120) // if username is not compared to uuid, then receive "" + // Storage.GetTextures and Storage.SetTextures shouldn't be called suite.MojangApi.On("UsernameToUuids", usernames[0:100]).Once().Return([]*mojang.ProfileInfo{}, nil) suite.MojangApi.On("UsernameToUuids", usernames[100:120]).Once().Return([]*mojang.ProfileInfo{}, nil) @@ -162,8 +228,10 @@ func (suite *QueueTestSuite) TestReceiveTexturesForMoreThan100Usernames() { func (suite *QueueTestSuite) TestReceiveTexturesForTheSameUsernames() { expectedResult := &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"} - suite.Storage.On("Get", mock.Anything).Twice().Return(nil) - suite.Storage.On("Set", expectedResult).Once() + suite.Storage.On("GetUuid", "maksimkurb").Twice().Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", "maksimkurb", "0d252b7218b648bfb86c2ae476954d32").Once() + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("StoreTextures", expectedResult).Once() suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{ {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, }, nil) @@ -181,8 +249,10 @@ func (suite *QueueTestSuite) TestReceiveTexturesForTheSameUsernames() { func (suite *QueueTestSuite) TestReceiveTexturesForUsernameThatAlreadyProcessing() { expectedResult := &mojang.SignedTexturesResponse{Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"} - suite.Storage.On("Get", mock.Anything).Return(nil) - suite.Storage.On("Set", expectedResult).Once() + suite.Storage.On("GetUuid", "maksimkurb").Twice().Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", "maksimkurb", "0d252b7218b648bfb86c2ae476954d32").Once() + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(nil, &ValueNotFound{}) + suite.Storage.On("StoreTextures", expectedResult).Once() suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{ {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, }, nil) @@ -206,8 +276,9 @@ func (suite *QueueTestSuite) TestReceiveTexturesForUsernameThatAlreadyProcessing } func (suite *QueueTestSuite) TestDoNothingWhenNoTasks() { - suite.Storage.On("Get", mock.Anything).Return(nil) - // Storage.Set shouldn't be called + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", "maksimkurb", "").Once() + // Storage.GetTextures and Storage.StoreTextures shouldn't be called suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{}, nil) // Perform first iteration and await it finish @@ -223,8 +294,8 @@ func (suite *QueueTestSuite) TestDoNothingWhenNoTasks() { } func (suite *QueueTestSuite) TestHandle429ResponseWhenExchangingUsernamesToUuids() { - suite.Storage.On("Get", mock.Anything).Return(nil) - // Storage.Set shouldn't be called + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("", &ValueNotFound{}) + // Storage.StoreUuid, Storage.GetTextures and Storage.StoreTextures shouldn't be called suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return(nil, &mojang.TooManyRequestsError{}) resultChan := suite.Queue.GetTexturesForUsername("maksimkurb") @@ -235,8 +306,10 @@ func (suite *QueueTestSuite) TestHandle429ResponseWhenExchangingUsernamesToUuids } func (suite *QueueTestSuite) TestHandle429ResponseWhenRequestingUsersTextures() { - suite.Storage.On("Get", mock.Anything).Return(nil) - // Storage.Set shouldn't be called + suite.Storage.On("GetUuid", "maksimkurb").Once().Return("", &ValueNotFound{}) + suite.Storage.On("StoreUuid", "maksimkurb", "0d252b7218b648bfb86c2ae476954d32").Once() + suite.Storage.On("GetTextures", "0d252b7218b648bfb86c2ae476954d32").Once().Return(nil, &ValueNotFound{}) + // Storage.StoreTextures shouldn't be called suite.MojangApi.On("UsernameToUuids", []string{"maksimkurb"}).Once().Return([]*mojang.ProfileInfo{ {Id: "0d252b7218b648bfb86c2ae476954d32", Name: "maksimkurb"}, }, nil) diff --git a/api/mojang/queue/storage.go b/api/mojang/queue/storage.go index 93c99a7..7d9c424 100644 --- a/api/mojang/queue/storage.go +++ b/api/mojang/queue/storage.go @@ -2,18 +2,29 @@ package queue import "github.com/elyby/chrly/api/mojang" +type UuidsStorage interface { + GetUuid(username string) (string, error) + StoreUuid(username string, uuid string) +} + +type TexturesStorage interface { + // nil can be returned to indicate that there is no textures for uuid + // and we know about it. Return err only in case, when storage completely + // don't know anything about uuid + GetTextures(uuid string) (*mojang.SignedTexturesResponse, error) + StoreTextures(textures *mojang.SignedTexturesResponse) +} + type Storage interface { - Get(username string) *mojang.SignedTexturesResponse - Set(textures *mojang.SignedTexturesResponse) + UuidsStorage + TexturesStorage } -// NilStorage used for testing purposes -type NilStorage struct { +// This error can be used to indicate, that requested +// value doesn't exists in the storage +type ValueNotFound struct { } -func (*NilStorage) Get(username string) *mojang.SignedTexturesResponse { - return nil -} - -func (*NilStorage) Set(textures *mojang.SignedTexturesResponse) { +func (*ValueNotFound) Error() string { + return "value not found in storage" }