From bc4d714112107556b8f9926813dcab8f09a0637a Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Wed, 7 Feb 2024 01:36:18 +0100 Subject: [PATCH] Make Mojang profiles provider cancellable --- internal/http/skinsystem.go | 12 +- internal/http/skinsystem_test.go | 134 +++++++++++-------- internal/mojang/batch_uuids_provider.go | 65 ++++++--- internal/mojang/batch_uuids_provider_test.go | 39 +++++- internal/mojang/provider.go | 13 +- internal/mojang/provider_test.go | 120 +++++++++-------- internal/mojang/textures_provider.go | 9 +- internal/mojang/textures_provider_test.go | 18 +-- internal/mojang/uuids_provider.go | 6 +- internal/mojang/uuids_provider_test.go | 25 ++-- internal/profiles/provider.go | 7 +- internal/profiles/provider_test.go | 76 ++++++----- 12 files changed, 320 insertions(+), 204 deletions(-) diff --git a/internal/http/skinsystem.go b/internal/http/skinsystem.go index 725b49d..6479429 100644 --- a/internal/http/skinsystem.go +++ b/internal/http/skinsystem.go @@ -1,6 +1,7 @@ package http import ( + "context" "crypto/rsa" "crypto/x509" "encoding/base64" @@ -20,7 +21,7 @@ import ( var timeNow = time.Now type ProfilesProvider interface { - FindProfileByUsername(username string, allowProxy bool) (*db.Profile, error) + FindProfileByUsername(ctx context.Context, username string, allowProxy bool) (*db.Profile, error) } type TexturesSigner interface { @@ -55,7 +56,7 @@ func (ctx *Skinsystem) Handler() *mux.Router { } func (ctx *Skinsystem) skinHandler(response http.ResponseWriter, request *http.Request) { - profile, err := ctx.ProfilesProvider.FindProfileByUsername(parseUsername(mux.Vars(request)["username"]), true) + profile, err := ctx.ProfilesProvider.FindProfileByUsername(request.Context(), parseUsername(mux.Vars(request)["username"]), true) if err != nil { apiServerError(response, "Unable to retrieve a skin", err) return @@ -82,7 +83,7 @@ func (ctx *Skinsystem) skinGetHandler(response http.ResponseWriter, request *htt } func (ctx *Skinsystem) capeHandler(response http.ResponseWriter, request *http.Request) { - profile, err := ctx.ProfilesProvider.FindProfileByUsername(parseUsername(mux.Vars(request)["username"]), true) + profile, err := ctx.ProfilesProvider.FindProfileByUsername(request.Context(), parseUsername(mux.Vars(request)["username"]), true) if err != nil { apiServerError(response, "Unable to retrieve a cape", err) return @@ -109,7 +110,7 @@ func (ctx *Skinsystem) capeGetHandler(response http.ResponseWriter, request *htt } func (ctx *Skinsystem) texturesHandler(response http.ResponseWriter, request *http.Request) { - profile, err := ctx.ProfilesProvider.FindProfileByUsername(mux.Vars(request)["username"], true) + profile, err := ctx.ProfilesProvider.FindProfileByUsername(request.Context(), mux.Vars(request)["username"], true) if err != nil { apiServerError(response, "Unable to retrieve a profile", err) return @@ -134,6 +135,7 @@ func (ctx *Skinsystem) texturesHandler(response http.ResponseWriter, request *ht func (ctx *Skinsystem) signedTexturesHandler(response http.ResponseWriter, request *http.Request) { profile, err := ctx.ProfilesProvider.FindProfileByUsername( + request.Context(), mux.Vars(request)["username"], getToBool(request.URL.Query().Get("proxy")), ) @@ -174,7 +176,7 @@ func (ctx *Skinsystem) signedTexturesHandler(response http.ResponseWriter, reque } func (ctx *Skinsystem) profileHandler(response http.ResponseWriter, request *http.Request) { - profile, err := ctx.ProfilesProvider.FindProfileByUsername(mux.Vars(request)["username"], true) + profile, err := ctx.ProfilesProvider.FindProfileByUsername(request.Context(), mux.Vars(request)["username"], true) if err != nil { apiServerError(response, "Unable to retrieve a profile", err) return diff --git a/internal/http/skinsystem_test.go b/internal/http/skinsystem_test.go index 24b9091..86388f3 100644 --- a/internal/http/skinsystem_test.go +++ b/internal/http/skinsystem_test.go @@ -1,6 +1,7 @@ package http import ( + "context" "crypto/rsa" "crypto/x509" "encoding/pem" @@ -23,8 +24,8 @@ type ProfilesProviderMock struct { mock.Mock } -func (m *ProfilesProviderMock) FindProfileByUsername(username string, allowProxy bool) (*db.Profile, error) { - args := m.Called(username, allowProxy) +func (m *ProfilesProviderMock) FindProfileByUsername(ctx context.Context, username string, allowProxy bool) (*db.Profile, error) { + args := m.Called(ctx, username, allowProxy) var result *db.Profile if casted, ok := args.Get(0).(*db.Profile); ok { result = casted @@ -90,12 +91,14 @@ func (t *SkinsystemTestSuite) TearDownSubTest() { func (t *SkinsystemTestSuite) TestSkinHandler() { for _, url := range []string{"http://chrly/skins/mock_username", "http://chrly/skins?name=mock_username"} { t.Run("known username with a skin", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ - SkinUrl: "https://example.com/skin.png", - }, nil) req := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() + // TODO: see the TODO about context above + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ + SkinUrl: "https://example.com/skin.png", + }, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -104,10 +107,11 @@ func (t *SkinsystemTestSuite) TestSkinHandler() { }) t.Run("known username without a skin", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{}, nil) req := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{}, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -115,10 +119,11 @@ func (t *SkinsystemTestSuite) TestSkinHandler() { }) t.Run("err from profiles provider", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, errors.New("mock error")) req := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, errors.New("mock error")) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -127,12 +132,13 @@ func (t *SkinsystemTestSuite) TestSkinHandler() { } t.Run("username with png extension", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ - SkinUrl: "https://example.com/skin.png", - }, nil) req := httptest.NewRequest("GET", "http://chrly/skins/mock_username.png", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ + SkinUrl: "https://example.com/skin.png", + }, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -154,12 +160,16 @@ func (t *SkinsystemTestSuite) TestSkinHandler() { func (t *SkinsystemTestSuite) TestCapeHandler() { for _, url := range []string{"http://chrly/cloaks/mock_username", "http://chrly/cloaks?name=mock_username"} { t.Run("known username with a skin", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ - CapeUrl: "https://example.com/cape.png", - }, nil) req := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() + // TODO: I can't find a way to verify that it's the context from the request that was passed in, + // as the Mux calls WithValue() on it, which creates a new Context and I haven't been able + // to find a way to verify that the passed context matches the base + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ + CapeUrl: "https://example.com/cape.png", + }, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -168,10 +178,11 @@ func (t *SkinsystemTestSuite) TestCapeHandler() { }) t.Run("known username without a skin", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{}, nil) req := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{}, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -179,10 +190,11 @@ func (t *SkinsystemTestSuite) TestCapeHandler() { }) t.Run("err from profiles provider", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, errors.New("mock error")) req := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, errors.New("mock error")) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -191,12 +203,13 @@ func (t *SkinsystemTestSuite) TestCapeHandler() { } t.Run("username with png extension", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ - CapeUrl: "https://example.com/cape.png", - }, nil) req := httptest.NewRequest("GET", "http://chrly/cloaks/mock_username.png", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ + CapeUrl: "https://example.com/cape.png", + }, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -217,12 +230,14 @@ func (t *SkinsystemTestSuite) TestCapeHandler() { func (t *SkinsystemTestSuite) TestTexturesHandler() { t.Run("known username with both textures", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ + req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) + w := httptest.NewRecorder() + + // TODO: see the TODO about context above + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ SkinUrl: "https://example.com/skin.png", CapeUrl: "https://example.com/cape.png", }, nil) - req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) - w := httptest.NewRecorder() t.App.Handler().ServeHTTP(w, req) @@ -241,12 +256,13 @@ func (t *SkinsystemTestSuite) TestTexturesHandler() { }) t.Run("known username with only slim skin", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ + req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) + w := httptest.NewRecorder() + + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ SkinUrl: "https://example.com/skin.png", SkinModel: "slim", }, nil) - req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) - w := httptest.NewRecorder() t.App.Handler().ServeHTTP(w, req) @@ -263,12 +279,13 @@ func (t *SkinsystemTestSuite) TestTexturesHandler() { }) t.Run("known username with only cape", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ - CapeUrl: "https://example.com/cape.png", - }, nil) req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ + CapeUrl: "https://example.com/cape.png", + }, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -281,10 +298,11 @@ func (t *SkinsystemTestSuite) TestTexturesHandler() { }) t.Run("known username without any textures", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{}, nil) req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{}, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -294,10 +312,11 @@ func (t *SkinsystemTestSuite) TestTexturesHandler() { }) t.Run("unknown username", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, nil) req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -307,10 +326,11 @@ func (t *SkinsystemTestSuite) TestTexturesHandler() { }) t.Run("err from profiles provider", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, errors.New("mock error")) req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, errors.New("mock error")) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -318,24 +338,18 @@ func (t *SkinsystemTestSuite) TestTexturesHandler() { }) } -type signedTexturesTestCase struct { - Name string - AllowProxy bool - BeforeTest func(suite *SkinsystemTestSuite) - PanicErr string - AfterTest func(suite *SkinsystemTestSuite, response *http.Response) -} - func (t *SkinsystemTestSuite) TestSignedTextures() { t.Run("exists profile with mojang textures", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", false).Return(&db.Profile{ + req := httptest.NewRequest("GET", "http://chrly/textures/signed/mock_username", nil) + w := httptest.NewRecorder() + + // TODO: see the TODO about context above + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", false).Return(&db.Profile{ Uuid: "mock-uuid", Username: "mock", MojangTextures: "mock-mojang-textures", MojangSignature: "mock-mojang-signature", }, nil) - req := httptest.NewRequest("GET", "http://chrly/textures/signed/mock_username", nil) - w := httptest.NewRecorder() t.App.Handler().ServeHTTP(w, req) @@ -361,10 +375,11 @@ func (t *SkinsystemTestSuite) TestSignedTextures() { }) t.Run("exists profile without mojang textures", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", false).Return(&db.Profile{}, nil) req := httptest.NewRequest("GET", "http://chrly/textures/signed/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", false).Return(&db.Profile{}, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -374,10 +389,11 @@ func (t *SkinsystemTestSuite) TestSignedTextures() { }) t.Run("not exists profile", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", false).Return(nil, nil) req := httptest.NewRequest("GET", "http://chrly/textures/signed/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", false).Return(nil, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -387,10 +403,11 @@ func (t *SkinsystemTestSuite) TestSignedTextures() { }) t.Run("err from profiles provider", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", false).Return(nil, errors.New("mock error")) req := httptest.NewRequest("GET", "http://chrly/textures/signed/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", false).Return(nil, errors.New("mock error")) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -398,25 +415,28 @@ func (t *SkinsystemTestSuite) TestSignedTextures() { }) t.Run("should allow proxying when specified get param", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, nil) req := httptest.NewRequest("GET", "http://chrly/textures/signed/mock_username?proxy=true", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, nil) + t.App.Handler().ServeHTTP(w, req) }) } func (t *SkinsystemTestSuite) TestProfile() { t.Run("exists profile with skin and cape", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ + req := httptest.NewRequest("GET", "http://chrly/profile/mock_username", nil) + w := httptest.NewRecorder() + + // TODO: see the TODO about context above + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ Uuid: "mock-uuid", Username: "mock_username", SkinUrl: "https://example.com/skin.png", SkinModel: "slim", CapeUrl: "https://example.com/cape.png", }, nil) - req := httptest.NewRequest("GET", "http://chrly/profile/mock_username", nil) - w := httptest.NewRecorder() t.App.Handler().ServeHTTP(w, req) @@ -441,15 +461,16 @@ func (t *SkinsystemTestSuite) TestProfile() { }) t.Run("exists signed profile with skin", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{ + req := httptest.NewRequest("GET", "http://chrly/profile/mock_username?unsigned=false", nil) + w := httptest.NewRecorder() + + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{ Uuid: "mock-uuid", Username: "mock_username", SkinUrl: "https://example.com/skin.png", SkinModel: "slim", }, nil) t.TexturesSigner.On("SignTextures", "eyJ0aW1lc3RhbXAiOjE2MTQyMTQyMjMwMDAsInByb2ZpbGVJZCI6Im1vY2stdXVpZCIsInByb2ZpbGVOYW1lIjoibW9ja191c2VybmFtZSIsInRleHR1cmVzIjp7IlNLSU4iOnsidXJsIjoiaHR0cHM6Ly9leGFtcGxlLmNvbS9za2luLnBuZyIsIm1ldGFkYXRhIjp7Im1vZGVsIjoic2xpbSJ9fX19").Return("mock signature", nil) - req := httptest.NewRequest("GET", "http://chrly/profile/mock_username?unsigned=false", nil) - w := httptest.NewRecorder() t.App.Handler().ServeHTTP(w, req) @@ -475,10 +496,11 @@ func (t *SkinsystemTestSuite) TestProfile() { }) t.Run("not exists profile", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, nil) req := httptest.NewRequest("GET", "http://chrly/profile/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, nil) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -488,10 +510,11 @@ func (t *SkinsystemTestSuite) TestProfile() { }) t.Run("err from profiles provider", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(nil, errors.New("mock error")) req := httptest.NewRequest("GET", "http://chrly/profile/mock_username", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(nil, errors.New("mock error")) + t.App.Handler().ServeHTTP(w, req) result := w.Result() @@ -499,11 +522,12 @@ func (t *SkinsystemTestSuite) TestProfile() { }) t.Run("err from textures signer", func() { - t.ProfilesProvider.On("FindProfileByUsername", "mock_username", true).Return(&db.Profile{}, nil) - t.TexturesSigner.On("SignTextures", mock.Anything).Return("", errors.New("mock error")) req := httptest.NewRequest("GET", "http://chrly/profile/mock_username?unsigned=false", nil) w := httptest.NewRecorder() + t.ProfilesProvider.On("FindProfileByUsername", mock.Anything, "mock_username", true).Return(&db.Profile{}, nil) + t.TexturesSigner.On("SignTextures", mock.Anything).Return("", errors.New("mock error")) + t.App.Handler().ServeHTTP(w, req) result := w.Result() diff --git a/internal/mojang/batch_uuids_provider.go b/internal/mojang/batch_uuids_provider.go index 59b9b00..de0854f 100644 --- a/internal/mojang/batch_uuids_provider.go +++ b/internal/mojang/batch_uuids_provider.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "strings" "sync" "time" @@ -39,6 +40,7 @@ func NewBatchUuidsProvider( type job struct { Username string + Ctx context.Context ResultChan chan<- *jobResult } @@ -47,43 +49,68 @@ type jobResult struct { Error error } -func (ctx *BatchUuidsProvider) GetUuid(username string) (*ProfileInfo, error) { +func (p *BatchUuidsProvider) GetUuid(ctx context.Context, username string) (*ProfileInfo, error) { resultChan := make(chan *jobResult) - n := ctx.queue.Enqueue(&job{username, resultChan}) - if ctx.fireOnFull && n%ctx.batch == 0 { - ctx.fireChan <- struct{}{} + n := p.queue.Enqueue(&job{username, ctx, resultChan}) + if p.fireOnFull && n%p.batch == 0 { + p.fireChan <- struct{}{} } - ctx.onFirstCall.Do(ctx.startQueue) + p.onFirstCall.Do(p.startQueue) - result := <-resultChan - - return result.Profile, result.Error + select { + case <-ctx.Done(): + return nil, ctx.Err() + case result := <-resultChan: + return result.Profile, result.Error + } } -func (ctx *BatchUuidsProvider) StopQueue() { - close(ctx.stopChan) +func (p *BatchUuidsProvider) StopQueue() { + close(p.stopChan) } -func (ctx *BatchUuidsProvider) startQueue() { +func (p *BatchUuidsProvider) startQueue() { go func() { for { - t := time.NewTimer(ctx.delay) + t := time.NewTimer(p.delay) select { - case <-ctx.stopChan: + case <-p.stopChan: return case <-t.C: - go ctx.fireRequest() - case <-ctx.fireChan: + go p.fireRequest() + case <-p.fireChan: t.Stop() - go ctx.fireRequest() + go p.fireRequest() } } }() } -func (ctx *BatchUuidsProvider) fireRequest() { - jobs, _ := ctx.queue.Dequeue(ctx.batch) +func (p *BatchUuidsProvider) fireRequest() { + jobs := make([]*job, 0, p.batch) + n := p.batch + for { + foundJobs, left := p.queue.Dequeue(n) + for i := range foundJobs { + if foundJobs[i].Ctx.Err() != nil { + // If the job context has already ended, its result will be returned in the GetUuid method + close(foundJobs[i].ResultChan) + + foundJobs[i] = foundJobs[len(foundJobs)-1] + foundJobs = foundJobs[:len(foundJobs)-1] + } + } + + jobs = append(jobs, foundJobs...) + if len(jobs) != p.batch && left != 0 { + n = p.batch - len(jobs) + continue + } + + break + } + if len(jobs) == 0 { return } @@ -93,7 +120,7 @@ func (ctx *BatchUuidsProvider) fireRequest() { usernames[i] = job.Username } - profiles, err := ctx.UsernamesToUuidsEndpoint(usernames) + profiles, err := p.UsernamesToUuidsEndpoint(usernames) for _, job := range jobs { response := &jobResult{} if err == nil { diff --git a/internal/mojang/batch_uuids_provider_test.go b/internal/mojang/batch_uuids_provider_test.go index 0bb4bd5..f8d4ffa 100644 --- a/internal/mojang/batch_uuids_provider_test.go +++ b/internal/mojang/batch_uuids_provider_test.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "errors" "testing" "time" @@ -54,11 +55,15 @@ func (s *batchUuidsProviderTestSuite) TearDownTest() { } func (s *batchUuidsProviderTestSuite) GetUuidAsync(username string) <-chan *batchUuidsProviderGetUuidResult { + return s.GetUuidAsyncWithCtx(context.Background(), username) +} + +func (s *batchUuidsProviderTestSuite) GetUuidAsyncWithCtx(ctx context.Context, username string) <-chan *batchUuidsProviderGetUuidResult { startedChan := make(chan any) c := make(chan *batchUuidsProviderGetUuidResult, 1) go func() { close(startedChan) - profile, err := s.Provider.GetUuid(username) + profile, err := s.Provider.GetUuid(ctx, username) c <- &batchUuidsProviderGetUuidResult{ Result: profile, Error: err, @@ -125,6 +130,38 @@ func (s *batchUuidsProviderTestSuite) TestGetUuidForManyUsernamesSplitByMultiple s.Require().NotEmpty(resultChan4) } +func (s *batchUuidsProviderTestSuite) TestGetUuidForManyUsernamesWhenOneOfContextIsDeadlined() { + var emptyResponse []string + + s.MojangApi.On("UsernamesToUuids", []string{"username1", "username2", "username4"}).Once().Return(emptyResponse, nil) + + ctx, cancelCtx := context.WithCancel(context.Background()) + + resultChan1 := s.GetUuidAsync("username1") + resultChan2 := s.GetUuidAsync("username2") + resultChan3 := s.GetUuidAsyncWithCtx(ctx, "username3") + resultChan4 := s.GetUuidAsync("username4") + + cancelCtx() + + time.Sleep(time.Duration(float64(awaitDelay) * 0.5)) + + s.Empty(resultChan1) + s.Empty(resultChan2) + s.NotEmpty(resultChan3, "canceled context must immediately release the job") + s.Empty(resultChan4) + + result3 := <-resultChan3 + s.Nil(result3.Result) + s.ErrorIs(result3.Error, context.Canceled) + + time.Sleep(awaitDelay) + + s.Require().NotEmpty(resultChan1) + s.Require().NotEmpty(resultChan2) + s.Require().NotEmpty(resultChan4) +} + func (s *batchUuidsProviderTestSuite) TestGetUuidForManyUsernamesFireOnFull() { s.Provider.fireOnFull = true diff --git a/internal/mojang/provider.go b/internal/mojang/provider.go index 76669df..c9b84d4 100644 --- a/internal/mojang/provider.go +++ b/internal/mojang/provider.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "errors" "regexp" "strings" @@ -14,11 +15,11 @@ var InvalidUsername = errors.New("the username passed doesn't meet Mojang's requ var allowedUsernamesRegex = regexp.MustCompile(`(?i)^[0-9a-z_]{3,16}$`) type UuidsProvider interface { - GetUuid(username string) (*ProfileInfo, error) + GetUuid(ctx context.Context, username string) (*ProfileInfo, error) } type TexturesProvider interface { - GetTextures(uuid string) (*ProfileResponse, error) + GetTextures(ctx context.Context, uuid string) (*ProfileResponse, error) } type MojangTexturesProvider struct { @@ -28,7 +29,7 @@ type MojangTexturesProvider struct { group singleflight.Group[string, *ProfileResponse] } -func (p *MojangTexturesProvider) GetForUsername(username string) (*ProfileResponse, error) { +func (p *MojangTexturesProvider) GetForUsername(ctx context.Context, username string) (*ProfileResponse, error) { if !allowedUsernamesRegex.MatchString(username) { return nil, InvalidUsername } @@ -36,7 +37,7 @@ func (p *MojangTexturesProvider) GetForUsername(username string) (*ProfileRespon username = strings.ToLower(username) result, err, _ := p.group.Do(username, func() (*ProfileResponse, error) { - profile, err := p.UuidsProvider.GetUuid(username) + profile, err := p.UuidsProvider.GetUuid(ctx, username) if err != nil { return nil, err } @@ -45,7 +46,7 @@ func (p *MojangTexturesProvider) GetForUsername(username string) (*ProfileRespon return nil, nil } - return p.TexturesProvider.GetTextures(profile.Id) + return p.TexturesProvider.GetTextures(ctx, profile.Id) }) return result, err @@ -54,6 +55,6 @@ func (p *MojangTexturesProvider) GetForUsername(username string) (*ProfileRespon type NilProvider struct { } -func (*NilProvider) GetForUsername(username string) (*ProfileResponse, error) { +func (*NilProvider) GetForUsername(ctx context.Context, username string) (*ProfileResponse, error) { return nil, nil } diff --git a/internal/mojang/provider_test.go b/internal/mojang/provider_test.go index 90763b4..3f78626 100644 --- a/internal/mojang/provider_test.go +++ b/internal/mojang/provider_test.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "errors" "sync" "testing" @@ -15,8 +16,8 @@ type mockUuidsProvider struct { mock.Mock } -func (m *mockUuidsProvider) GetUuid(username string) (*ProfileInfo, error) { - args := m.Called(username) +func (m *mockUuidsProvider) GetUuid(ctx context.Context, username string) (*ProfileInfo, error) { + args := m.Called(ctx, username) var result *ProfileInfo if casted, ok := args.Get(0).(*ProfileInfo); ok { result = casted @@ -29,8 +30,8 @@ type TexturesProviderMock struct { mock.Mock } -func (m *TexturesProviderMock) GetTextures(uuid string) (*ProfileResponse, error) { - args := m.Called(uuid) +func (m *TexturesProviderMock) GetTextures(ctx context.Context, uuid string) (*ProfileResponse, error) { + args := m.Called(ctx, uuid) var result *ProfileResponse if casted, ok := args.Get(0).(*ProfileResponse); ok { result = casted @@ -46,64 +47,64 @@ type providerTestSuite struct { TexturesProvider *TexturesProviderMock } -func (suite *providerTestSuite) SetupTest() { - suite.UuidsProvider = &mockUuidsProvider{} - suite.TexturesProvider = &TexturesProviderMock{} +func (s *providerTestSuite) SetupTest() { + s.UuidsProvider = &mockUuidsProvider{} + s.TexturesProvider = &TexturesProviderMock{} - suite.Provider = &MojangTexturesProvider{ - UuidsProvider: suite.UuidsProvider, - TexturesProvider: suite.TexturesProvider, + s.Provider = &MojangTexturesProvider{ + UuidsProvider: s.UuidsProvider, + TexturesProvider: s.TexturesProvider, } } -func (suite *providerTestSuite) TearDownTest() { - suite.UuidsProvider.AssertExpectations(suite.T()) - suite.TexturesProvider.AssertExpectations(suite.T()) +func (s *providerTestSuite) TearDownTest() { + s.UuidsProvider.AssertExpectations(s.T()) + s.TexturesProvider.AssertExpectations(s.T()) } -func (suite *providerTestSuite) TestGetForValidUsernameSuccessfully() { +func (s *providerTestSuite) TestGetForValidUsernameSuccessfully() { expectedProfile := &ProfileInfo{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} expectedResult := &ProfileResponse{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} + ctx := context.Background() - suite.UuidsProvider.On("GetUuid", "username").Once().Return(expectedProfile, nil) - suite.TexturesProvider.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(expectedResult, nil) + s.UuidsProvider.On("GetUuid", ctx, "username").Once().Return(expectedProfile, nil) + s.TexturesProvider.On("GetTextures", ctx, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(expectedResult, nil) - result, err := suite.Provider.GetForUsername("username") + result, err := s.Provider.GetForUsername(ctx, "username") - suite.Assert().NoError(err) - suite.Assert().Equal(expectedResult, result) + s.NoError(err) + s.Same(expectedResult, result) } -func (suite *providerTestSuite) TestGetForUsernameWhichHasNoMojangAccount() { - suite.UuidsProvider.On("GetUuid", "username").Once().Return(nil, nil) +func (s *providerTestSuite) TestGetForUsernameWhichHasNoMojangAccount() { + s.UuidsProvider.On("GetUuid", mock.Anything, "username").Once().Return(nil, nil) - result, err := suite.Provider.GetForUsername("username") + result, err := s.Provider.GetForUsername(context.Background(), "username") - suite.Assert().NoError(err) - suite.Assert().Nil(result) + s.NoError(err) + s.Nil(result) } -func (suite *providerTestSuite) TestGetForUsernameWhichHasMojangAccountButHasNoMojangSkin() { +func (s *providerTestSuite) TestGetForUsernameWhichHasMojangAccountButHasNoMojangSkin() { expectedProfile := &ProfileInfo{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} - suite.UuidsProvider.On("GetUuid", "username").Once().Return(expectedProfile, nil) - suite.TexturesProvider.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, nil) + s.UuidsProvider.On("GetUuid", mock.Anything, "username").Once().Return(expectedProfile, nil) + s.TexturesProvider.On("GetTextures", mock.Anything, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, nil) - result, err := suite.Provider.GetForUsername("username") + result, err := s.Provider.GetForUsername(context.Background(), "username") - suite.Assert().NoError(err) - suite.Assert().Nil(result) + s.NoError(err) + s.Nil(result) } -func (suite *providerTestSuite) TestGetForTheSameUsername() { +func (s *providerTestSuite) TestGetForTheSameUsernameInRow() { expectedProfile := &ProfileInfo{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} expectedResult := &ProfileResponse{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} awaitChan := make(chan time.Time) - // If possible, then remove this .After call - suite.UuidsProvider.On("GetUuid", "username").Once().WaitUntil(awaitChan).Return(expectedProfile, nil) - suite.TexturesProvider.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(expectedResult, nil) + s.UuidsProvider.On("GetUuid", mock.Anything, "username").Once().WaitUntil(awaitChan).Return(expectedProfile, nil) + s.TexturesProvider.On("GetTextures", mock.Anything, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(expectedResult, nil) results := make([]*ProfileResponse, 2) var wgStarted sync.WaitGroup @@ -113,7 +114,7 @@ func (suite *providerTestSuite) TestGetForTheSameUsername() { wgDone.Add(1) go func(i int) { wgStarted.Done() - textures, _ := suite.Provider.GetForUsername("username") + textures, _ := s.Provider.GetForUsername(context.Background(), "username") results[i] = textures wgDone.Done() }(i) @@ -123,35 +124,48 @@ func (suite *providerTestSuite) TestGetForTheSameUsername() { close(awaitChan) wgDone.Wait() - suite.Assert().Equal(expectedResult, results[0]) - suite.Assert().Equal(expectedResult, results[1]) + s.Same(expectedResult, results[0]) + s.Same(expectedResult, results[1]) } -func (suite *providerTestSuite) TestGetForNotAllowedMojangUsername() { - result, err := suite.Provider.GetForUsername("Not allowed") - suite.Assert().ErrorIs(err, InvalidUsername) - suite.Assert().Nil(result) +func (s *providerTestSuite) TestGetForTheSameUsernameOneAfterAnother() { + expectedProfile := &ProfileInfo{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} + expectedResult := &ProfileResponse{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} + + s.UuidsProvider.On("GetUuid", mock.Anything, "username").Times(2).Return(expectedProfile, nil) + s.TexturesProvider.On("GetTextures", mock.Anything, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Times(2).Return(expectedResult, nil) + + // Just ensure that providers will be called twice + _, _ = s.Provider.GetForUsername(context.Background(), "username") + time.Sleep(time.Millisecond * 20) + _, _ = s.Provider.GetForUsername(context.Background(), "username") +} + +func (s *providerTestSuite) TestGetForNotAllowedMojangUsername() { + result, err := s.Provider.GetForUsername(context.Background(), "Not allowed") + s.ErrorIs(err, InvalidUsername) + s.Nil(result) } -func (suite *providerTestSuite) TestGetErrorFromUuidsProvider() { +func (s *providerTestSuite) TestGetErrorFromUuidsProvider() { err := errors.New("mock error") - suite.UuidsProvider.On("GetUuid", "username").Once().Return(nil, err) + s.UuidsProvider.On("GetUuid", mock.Anything, "username").Once().Return(nil, err) - result, resErr := suite.Provider.GetForUsername("username") - suite.Assert().Nil(result) - suite.Assert().Equal(err, resErr) + result, resErr := s.Provider.GetForUsername(context.Background(), "username") + s.Nil(result) + s.Equal(err, resErr) } -func (suite *providerTestSuite) TestGetErrorFromTexturesProvider() { +func (s *providerTestSuite) TestGetErrorFromTexturesProvider() { expectedProfile := &ProfileInfo{Id: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", Name: "username"} err := errors.New("mock error") - suite.UuidsProvider.On("GetUuid", "username").Once().Return(expectedProfile, nil) - suite.TexturesProvider.On("GetTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, err) + s.UuidsProvider.On("GetUuid", mock.Anything, "username").Once().Return(expectedProfile, nil) + s.TexturesProvider.On("GetTextures", mock.Anything, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil, err) - result, resErr := suite.Provider.GetForUsername("username") - suite.Assert().Nil(result) - suite.Assert().Equal(err, resErr) + result, resErr := s.Provider.GetForUsername(context.Background(), "username") + s.Nil(result) + s.Same(err, resErr) } func TestProvider(t *testing.T) { @@ -160,7 +174,7 @@ func TestProvider(t *testing.T) { func TestNilProvider_GetForUsername(t *testing.T) { provider := &NilProvider{} - result, err := provider.GetForUsername("username") + result, err := provider.GetForUsername(context.Background(), "username") require.Nil(t, result) require.NoError(t, err) } diff --git a/internal/mojang/textures_provider.go b/internal/mojang/textures_provider.go index 7102ce3..4247041 100644 --- a/internal/mojang/textures_provider.go +++ b/internal/mojang/textures_provider.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "sync" "time" @@ -11,8 +12,8 @@ type MojangApiTexturesProvider struct { MojangApiTexturesEndpoint func(uuid string, signed bool) (*ProfileResponse, error) } -func (ctx *MojangApiTexturesProvider) GetTextures(uuid string) (*ProfileResponse, error) { - return ctx.MojangApiTexturesEndpoint(uuid, true) +func (p *MojangApiTexturesProvider) GetTextures(ctx context.Context, uuid string) (*ProfileResponse, error) { + return p.MojangApiTexturesEndpoint(uuid, true) } // Perfectly there should be an object with provider and cache implementation, @@ -35,14 +36,14 @@ func NewTexturesProviderWithInMemoryCache(provider TexturesProvider) *TexturesPr return storage } -func (s *TexturesProviderWithInMemoryCache) GetTextures(uuid string) (*ProfileResponse, error) { +func (s *TexturesProviderWithInMemoryCache) GetTextures(ctx context.Context, uuid string) (*ProfileResponse, error) { item := s.cache.Get(uuid) // Don't check item.IsExpired() since Get function is already did this check if item != nil { return item.Value(), nil } - result, err := s.provider.GetTextures(uuid) + result, err := s.provider.GetTextures(ctx, uuid) if err != nil { return nil, err } diff --git a/internal/mojang/textures_provider_test.go b/internal/mojang/textures_provider_test.go index 8bb9bd7..6665a63 100644 --- a/internal/mojang/textures_provider_test.go +++ b/internal/mojang/textures_provider_test.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "errors" "testing" "time" @@ -64,7 +65,7 @@ func (s *MojangApiTexturesProviderSuite) TearDownTest() { func (s *MojangApiTexturesProviderSuite) TestGetTextures() { s.MojangApi.On("UuidToTextures", "dead24f9a4fa4877b7b04c8c6c72bb46", true).Once().Return(signedTexturesResponse, nil) - result, err := s.Provider.GetTextures("dead24f9a4fa4877b7b04c8c6c72bb46") + result, err := s.Provider.GetTextures(context.Background(), "dead24f9a4fa4877b7b04c8c6c72bb46") s.Require().NoError(err) s.Require().Equal(signedTexturesResponse, result) @@ -74,7 +75,7 @@ func (s *MojangApiTexturesProviderSuite) TestGetTexturesWithError() { expectedError := errors.New("mock error") s.MojangApi.On("UuidToTextures", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", true).Once().Return(nil, expectedError) - result, err := s.Provider.GetTextures("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + result, err := s.Provider.GetTextures(context.Background(), "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") s.Require().Nil(result) s.Require().Equal(expectedError, err) @@ -101,10 +102,11 @@ func (s *TexturesProviderWithInMemoryCacheSuite) TearDownTest() { } func (s *TexturesProviderWithInMemoryCacheSuite) TestGetTexturesWithSuccessfulOriginalProviderResponse() { - s.Original.On("GetTextures", "uuid").Once().Return(signedTexturesResponse, nil) + ctx := context.Background() + s.Original.On("GetTextures", ctx, "uuid").Once().Return(signedTexturesResponse, nil) // Do the call multiple times to ensure, that there will be only one call to the Original provider for i := 0; i < 5; i++ { - result, err := s.Provider.GetTextures("uuid") + result, err := s.Provider.GetTextures(ctx, "uuid") s.Require().NoError(err) s.Require().Same(signedTexturesResponse, result) @@ -112,10 +114,10 @@ func (s *TexturesProviderWithInMemoryCacheSuite) TestGetTexturesWithSuccessfulOr } func (s *TexturesProviderWithInMemoryCacheSuite) TestGetTexturesWithEmptyOriginalProviderResponse() { - s.Original.On("GetTextures", "uuid").Once().Return(nil, nil) + s.Original.On("GetTextures", mock.Anything, "uuid").Once().Return(nil, nil) // Do the call multiple times to ensure, that there will be only one call to the original provider for i := 0; i < 5; i++ { - result, err := s.Provider.GetTextures("uuid") + result, err := s.Provider.GetTextures(context.Background(), "uuid") s.Require().NoError(err) s.Require().Nil(result) @@ -124,10 +126,10 @@ func (s *TexturesProviderWithInMemoryCacheSuite) TestGetTexturesWithEmptyOrigina func (s *TexturesProviderWithInMemoryCacheSuite) TestGetTexturesWithErrorFromOriginalProvider() { expectedErr := errors.New("mock error") - s.Original.On("GetTextures", "uuid").Times(5).Return(nil, expectedErr) + s.Original.On("GetTextures", mock.Anything, "uuid").Times(5).Return(nil, expectedErr) // Do the call multiple times to ensure, that the error will not be cached and there will be a request on each call for i := 0; i < 5; i++ { - result, err := s.Provider.GetTextures("uuid") + result, err := s.Provider.GetTextures(context.Background(), "uuid") s.Require().Same(expectedErr, err) s.Require().Nil(result) diff --git a/internal/mojang/uuids_provider.go b/internal/mojang/uuids_provider.go index 438ef87..36621ca 100644 --- a/internal/mojang/uuids_provider.go +++ b/internal/mojang/uuids_provider.go @@ -1,5 +1,7 @@ package mojang +import "context" + type MojangUuidsStorage interface { // The second argument must be returned as a incoming username in case, // when cached result indicates that there is no Mojang user with provided username @@ -13,7 +15,7 @@ type UuidsProviderWithCache struct { Storage MojangUuidsStorage } -func (p *UuidsProviderWithCache) GetUuid(username string) (*ProfileInfo, error) { +func (p *UuidsProviderWithCache) GetUuid(ctx context.Context, username string) (*ProfileInfo, error) { uuid, foundUsername, err := p.Storage.GetUuidForMojangUsername(username) if err != nil { return nil, err @@ -27,7 +29,7 @@ func (p *UuidsProviderWithCache) GetUuid(username string) (*ProfileInfo, error) return nil, nil } - profile, err := p.Provider.GetUuid(username) + profile, err := p.Provider.GetUuid(ctx, username) if err != nil { return nil, err } diff --git a/internal/mojang/uuids_provider_test.go b/internal/mojang/uuids_provider_test.go index a25e4ab..eacb4ac 100644 --- a/internal/mojang/uuids_provider_test.go +++ b/internal/mojang/uuids_provider_test.go @@ -1,6 +1,7 @@ package mojang import ( + "context" "errors" "testing" @@ -14,8 +15,8 @@ type UuidsProviderMock struct { mock.Mock } -func (m *UuidsProviderMock) GetUuid(username string) (*ProfileInfo, error) { - args := m.Called(username) +func (m *UuidsProviderMock) GetUuid(ctx context.Context, username string) (*ProfileInfo, error) { + args := m.Called(ctx, username) var result *ProfileInfo if casted, ok := args.Get(0).(*ProfileInfo); ok { result = casted @@ -61,12 +62,14 @@ func (s *UuidsProviderWithCacheSuite) TearDownTest() { } func (s *UuidsProviderWithCacheSuite) TestUncachedSuccessfully() { + ctx := context.Background() + s.Storage.On("GetUuidForMojangUsername", "username").Return("", "", nil) s.Storage.On("StoreMojangUuid", "UserName", "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa").Once().Return(nil) - s.Original.On("GetUuid", "username").Once().Return(mockProfile, nil) + s.Original.On("GetUuid", ctx, "username").Once().Return(mockProfile, nil) - result, err := s.Provider.GetUuid("username") + result, err := s.Provider.GetUuid(ctx, "username") s.Require().NoError(err) s.Require().Equal(mockProfile, result) @@ -76,9 +79,9 @@ func (s *UuidsProviderWithCacheSuite) TestUncachedNotExistsMojangUsername() { s.Storage.On("GetUuidForMojangUsername", "username").Return("", "", nil) s.Storage.On("StoreMojangUuid", "username", "").Once().Return(nil) - s.Original.On("GetUuid", "username").Once().Return(nil, nil) + s.Original.On("GetUuid", mock.Anything, "username").Once().Return(nil, nil) - result, err := s.Provider.GetUuid("username") + result, err := s.Provider.GetUuid(context.Background(), "username") s.Require().NoError(err) s.Require().Nil(result) @@ -87,7 +90,7 @@ func (s *UuidsProviderWithCacheSuite) TestUncachedNotExistsMojangUsername() { func (s *UuidsProviderWithCacheSuite) TestKnownCachedUsername() { s.Storage.On("GetUuidForMojangUsername", "username").Return("mock-uuid", "UserName", nil) - result, err := s.Provider.GetUuid("username") + result, err := s.Provider.GetUuid(context.Background(), "username") s.Require().NoError(err) s.Require().NotNil(result) @@ -98,7 +101,7 @@ func (s *UuidsProviderWithCacheSuite) TestKnownCachedUsername() { func (s *UuidsProviderWithCacheSuite) TestUnknownCachedUsername() { s.Storage.On("GetUuidForMojangUsername", "username").Return("", "UserName", nil) - result, err := s.Provider.GetUuid("username") + result, err := s.Provider.GetUuid(context.Background(), "username") s.Require().NoError(err) s.Require().Nil(result) @@ -108,7 +111,7 @@ func (s *UuidsProviderWithCacheSuite) TestErrorDuringCacheQuery() { expectedError := errors.New("mock error") s.Storage.On("GetUuidForMojangUsername", "username").Return("", "", expectedError) - result, err := s.Provider.GetUuid("username") + result, err := s.Provider.GetUuid(context.Background(), "username") s.Require().Same(expectedError, err) s.Require().Nil(result) @@ -118,9 +121,9 @@ func (s *UuidsProviderWithCacheSuite) TestErrorFromOriginalProvider() { expectedError := errors.New("mock error") s.Storage.On("GetUuidForMojangUsername", "username").Return("", "", nil) - s.Original.On("GetUuid", "username").Once().Return(nil, expectedError) + s.Original.On("GetUuid", mock.Anything, "username").Once().Return(nil, expectedError) - result, err := s.Provider.GetUuid("username") + result, err := s.Provider.GetUuid(context.Background(), "username") s.Require().Same(expectedError, err) s.Require().Nil(result) diff --git a/internal/profiles/provider.go b/internal/profiles/provider.go index 63e33b5..e5f1900 100644 --- a/internal/profiles/provider.go +++ b/internal/profiles/provider.go @@ -1,6 +1,7 @@ package profiles import ( + "context" "errors" "ely.by/chrly/internal/db" @@ -12,7 +13,7 @@ type ProfilesFinder interface { } type MojangProfilesProvider interface { - GetForUsername(username string) (*mojang.ProfileResponse, error) + GetForUsername(ctx context.Context, username string) (*mojang.ProfileResponse, error) } type Provider struct { @@ -20,7 +21,7 @@ type Provider struct { MojangProfilesProvider } -func (p *Provider) FindProfileByUsername(username string, allowProxy bool) (*db.Profile, error) { +func (p *Provider) FindProfileByUsername(ctx context.Context, username string, allowProxy bool) (*db.Profile, error) { profile, err := p.ProfilesFinder.FindProfileByUsername(username) if err != nil { return nil, err @@ -31,7 +32,7 @@ func (p *Provider) FindProfileByUsername(username string, allowProxy bool) (*db. } if allowProxy { - mojangProfile, err := p.MojangProfilesProvider.GetForUsername(username) + mojangProfile, err := p.MojangProfilesProvider.GetForUsername(ctx, username) // If we at least know something about the user, // then we can ignore an error and return profile without textures if err != nil && profile != nil { diff --git a/internal/profiles/provider_test.go b/internal/profiles/provider_test.go index 4bc1839..7a7f215 100644 --- a/internal/profiles/provider_test.go +++ b/internal/profiles/provider_test.go @@ -1,6 +1,7 @@ package profiles import ( + "context" "errors" "testing" "time" @@ -31,8 +32,8 @@ type MojangProfilesProviderMock struct { mock.Mock } -func (m *MojangProfilesProviderMock) GetForUsername(username string) (*mojang.ProfileResponse, error) { - args := m.Called(username) +func (m *MojangProfilesProviderMock) GetForUsername(ctx context.Context, username string) (*mojang.ProfileResponse, error) { + args := m.Called(ctx, username) var result *mojang.ProfileResponse if casted, ok := args.Get(0).(*mojang.ProfileResponse); ok { result = casted @@ -46,21 +47,21 @@ type CombinedProfilesProviderSuite struct { Provider *Provider - ProfilesRepository *ProfilesFinderMock + ProfilesFinder *ProfilesFinderMock MojangProfilesProvider *MojangProfilesProviderMock } func (t *CombinedProfilesProviderSuite) SetupSubTest() { - t.ProfilesRepository = &ProfilesFinderMock{} + t.ProfilesFinder = &ProfilesFinderMock{} t.MojangProfilesProvider = &MojangProfilesProviderMock{} t.Provider = &Provider{ - ProfilesFinder: t.ProfilesRepository, + ProfilesFinder: t.ProfilesFinder, MojangProfilesProvider: t.MojangProfilesProvider, } } func (t *CombinedProfilesProviderSuite) TearDownSubTest() { - t.ProfilesRepository.AssertExpectations(t.T()) + t.ProfilesFinder.AssertExpectations(t.T()) t.MojangProfilesProvider.AssertExpectations(t.T()) } @@ -71,9 +72,9 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { Username: "Mock", SkinUrl: "https://example.com/skin.png", } - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(profile, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(profile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.NoError(err) t.Same(profile, foundProfile) }) @@ -84,9 +85,9 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { Username: "Mock", CapeUrl: "https://example.com/cape.png", } - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(profile, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(profile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.NoError(err) t.Same(profile, foundProfile) }) @@ -96,26 +97,26 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { Uuid: "mock-uuid", Username: "Mock", } - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(profile, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(profile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", false) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", false) t.NoError(err) t.Same(profile, foundProfile) }) t.Run("not exists profile (no proxy)", func() { - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", false) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", false) t.NoError(err) t.Nil(foundProfile) }) t.Run("handle error from profiles repository", func() { expectedError := errors.New("mock error") - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, expectedError) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, expectedError) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", false) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", false) t.Same(expectedError, err) t.Nil(foundProfile) }) @@ -126,10 +127,11 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { Username: "Mock", } mojangProfile := createMojangProfile(true, true) - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(profile, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(mojangProfile, nil) + ctx := context.Background() + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(profile, nil) + t.MojangProfilesProvider.On("GetForUsername", ctx, "Mock").Return(mojangProfile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(ctx, "Mock", true) t.NoError(err) t.Equal(&db.Profile{ Uuid: "mock-mojang-uuid", @@ -144,10 +146,10 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { t.Run("not exists profile (with proxy)", func() { mojangProfile := createMojangProfile(true, true) - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(mojangProfile, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, nil) + t.MojangProfilesProvider.On("GetForUsername", mock.Anything, "Mock").Return(mojangProfile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.NoError(err) t.Equal(&db.Profile{ Uuid: "mock-mojang-uuid", @@ -165,29 +167,29 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { Uuid: "mock-uuid", Username: "Mock", } - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(profile, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(nil, errors.New("mock error")) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(profile, nil) + t.MojangProfilesProvider.On("GetForUsername", mock.Anything, "Mock").Return(nil, errors.New("mock error")) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.NoError(err) t.Same(profile, foundProfile) }) t.Run("should not return an error when passed the invalid username", func() { - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(nil, mojang.InvalidUsername) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, nil) + t.MojangProfilesProvider.On("GetForUsername", mock.Anything, "Mock").Return(nil, mojang.InvalidUsername) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.NoError(err) t.Nil(foundProfile) }) t.Run("should return an error from mojang provider", func() { expectedError := errors.New("mock error") - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(nil, expectedError) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, nil) + t.MojangProfilesProvider.On("GetForUsername", mock.Anything, "Mock").Return(nil, expectedError) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.Same(expectedError, err) t.Nil(foundProfile) }) @@ -202,10 +204,10 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { }, }, } - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(mojangProfile, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, nil) + t.MojangProfilesProvider.On("GetForUsername", mock.Anything, "Mock").Return(mojangProfile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.ErrorContains(err, "illegal base64 data") t.Nil(foundProfile) }) @@ -216,10 +218,10 @@ func (t *CombinedProfilesProviderSuite) TestFindByUsername() { Name: "mOcK", Props: []*mojang.Property{}, } - t.ProfilesRepository.On("FindProfileByUsername", "Mock").Return(nil, nil) - t.MojangProfilesProvider.On("GetForUsername", "Mock").Return(mojangProfile, nil) + t.ProfilesFinder.On("FindProfileByUsername", "Mock").Return(nil, nil) + t.MojangProfilesProvider.On("GetForUsername", mock.Anything, "Mock").Return(mojangProfile, nil) - foundProfile, err := t.Provider.FindProfileByUsername("Mock", true) + foundProfile, err := t.Provider.FindProfileByUsername(context.Background(), "Mock", true) t.NoError(err) t.Equal(&db.Profile{ Uuid: "mock-mojang-uuid",