From b2ee10f72fb86c5b86af54deda92aa4266b1c778 Mon Sep 17 00:00:00 2001 From: ErickSkrauch Date: Wed, 29 Jan 2020 01:34:15 +0300 Subject: [PATCH] Completely rework the HTTP app layer structure. Replace a logger with an event dispatcher. Adjust tests to the new app architecture. --- cmd/serve.go | 10 ++-- cmd/worker.go | 11 ++-- http/http.go | 23 ++++++++ http/http_test.go | 9 +++ http/skinsystem.go | 112 ++++++++++++-------------------------- http/skinsystem_test.go | 107 ++++++++++++++++++++---------------- http/uuids_worker.go | 31 +---------- http/uuids_worker_test.go | 27 +++++---- 8 files changed, 156 insertions(+), 174 deletions(-) diff --git a/cmd/serve.go b/cmd/serve.go index 3732e33..5510627 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -74,20 +74,20 @@ var serveCmd = &cobra.Command{ } logger.Info("Mojang's textures queue is successfully initialized") - cfg := &http.Skinsystem{ - ListenSpec: fmt.Sprintf("%s:%d", viper.GetString("server.host"), viper.GetInt("server.port")), + address := fmt.Sprintf("%s:%d", viper.GetString("server.host"), viper.GetInt("server.port")) + handler := (&http.Skinsystem{ SkinsRepo: skinsRepo, CapesRepo: capesRepo, MojangTexturesProvider: mojangTexturesProvider, - Logger: logger, Auth: &auth.JwtAuth{Key: []byte(viper.GetString("chrly.secret"))}, TexturesExtraParamName: viper.GetString("textures.extra_param_name"), TexturesExtraParamValue: viper.GetString("textures.extra_param_value"), - } + }).CreateHandler() finishChan := make(chan bool) go func() { - if err := cfg.Run(); err != nil { + logger.Info(fmt.Sprintf("Starting the app, HTTP on: %s", address)) + if err := http.Serve(address, handler); err != nil { logger.Error(fmt.Sprintf("Error in main(): %v", err)) finishChan <- true } diff --git a/cmd/worker.go b/cmd/worker.go index 79ef7ed..e2ac655 100644 --- a/cmd/worker.go +++ b/cmd/worker.go @@ -28,15 +28,16 @@ var workerCmd = &cobra.Command{ return } - cfg := &http.UUIDsWorker{ - ListenSpec: fmt.Sprintf("%s:%d", viper.GetString("server.host"), viper.GetInt("server.port")), + address := fmt.Sprintf("%s:%d", viper.GetString("server.host"), viper.GetInt("server.port")) + handler := (&http.UUIDsWorker{ UUIDsProvider: uuidsProvider, - Logger: logger, - } + // TODO: create an emitter, restore logger + }).CreateHandler() finishChan := make(chan bool) go func() { - if err := cfg.Run(); err != nil { + logger.Info(fmt.Sprintf("Starting the worker, HTTP on: %s", address)) + if err := http.Serve(address, handler); err != nil { logger.Error(fmt.Sprintf("Error in main(): %v", err)) finishChan <- true } diff --git a/http/http.go b/http/http.go index aec1182..6a76daf 100644 --- a/http/http.go +++ b/http/http.go @@ -2,9 +2,32 @@ package http import ( "encoding/json" + "net" "net/http" + "time" ) +type Emitter interface { + Emit(name string, args ...interface{}) +} + +func Serve(address string, handler http.Handler) error { + listener, err := net.Listen("tcp", address) + if err != nil { + return err + } + + server := &http.Server{ + ReadTimeout: 5 * time.Second, + WriteTimeout: 5 * time.Second, + IdleTimeout: 60 * time.Second, + MaxHeaderBytes: 1 << 16, + Handler: handler, + } + + return server.Serve(listener) +} + func NotFound(response http.ResponseWriter, _ *http.Request) { data, _ := json.Marshal(map[string]string{ "status": "404", diff --git a/http/http_test.go b/http/http_test.go index 2b6d93d..376f1f6 100644 --- a/http/http_test.go +++ b/http/http_test.go @@ -6,8 +6,17 @@ import ( "testing" testify "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) +type emitterMock struct { + mock.Mock +} + +func (e *emitterMock) Emit(name string, args ...interface{}) { + e.Called((append([]interface{}{name}, args...))...) +} + func TestConfig_NotFound(t *testing.T) { assert := testify.New(t) diff --git a/http/skinsystem.go b/http/skinsystem.go index 86d4b04..0022941 100644 --- a/http/skinsystem.go +++ b/http/skinsystem.go @@ -5,15 +5,12 @@ import ( "errors" "fmt" "io" - "net" "net/http" "regexp" "strconv" "strings" - "time" "github.com/gorilla/mux" - "github.com/mono83/slf/wd" "github.com/thedevsaddam/govalidator" "github.com/elyby/chrly/api/mojang" @@ -21,7 +18,6 @@ import ( "github.com/elyby/chrly/model" ) -//noinspection GoSnakeCaseUsage const UUID_ANY = "^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$" var regexUuidAny = regexp.MustCompile(UUID_ANY) @@ -67,7 +63,7 @@ type SkinNotFoundError struct { } func (e SkinNotFoundError) Error() string { - return "Skin data not found." + return "skin data not found" } type CapeNotFoundError struct { @@ -75,7 +71,7 @@ type CapeNotFoundError struct { } func (e CapeNotFoundError) Error() string { - return "Cape file not found." + return "cape file not found" } type MojangTexturesProvider interface { @@ -87,33 +83,13 @@ type AuthChecker interface { } type Skinsystem struct { - ListenSpec string + Emitter TexturesExtraParamName string TexturesExtraParamValue string - - SkinsRepo SkinsRepository - CapesRepo CapesRepository - MojangTexturesProvider MojangTexturesProvider - Auth AuthChecker - Logger wd.Watchdog -} - -func (ctx *Skinsystem) Run() error { - ctx.Logger.Info(fmt.Sprintf("Starting the app, HTTP on: %s\n", ctx.ListenSpec)) - - listener, err := net.Listen("tcp", ctx.ListenSpec) - if err != nil { - return err - } - - server := &http.Server{ - ReadTimeout: 60 * time.Second, - WriteTimeout: 60 * time.Second, - MaxHeaderBytes: 1 << 16, - Handler: ctx.CreateHandler(), - } - - return server.Serve(listener) + SkinsRepo SkinsRepository + CapesRepo CapesRepository + MojangTexturesProvider MojangTexturesProvider + Auth AuthChecker } func (ctx *Skinsystem) CreateHandler() *mux.Router { @@ -129,9 +105,9 @@ func (ctx *Skinsystem) CreateHandler() *mux.Router { // API apiRouter := router.PathPrefix("/api").Subrouter() apiRouter.Use(ctx.AuthenticationMiddleware) - apiRouter.Handle("/skins", http.HandlerFunc(ctx.PostSkin)).Methods("POST") - apiRouter.Handle("/skins/id:{id:[0-9]+}", http.HandlerFunc(ctx.DeleteSkinByUserId)).Methods("DELETE") - apiRouter.Handle("/skins/{username}", http.HandlerFunc(ctx.DeleteSkinByUsername)).Methods("DELETE") + apiRouter.HandleFunc("/skins", ctx.PostSkin).Methods("POST") + apiRouter.HandleFunc("/skins/id:{id:[0-9]+}", ctx.DeleteSkinByUserId).Methods("DELETE") + apiRouter.HandleFunc("/skins/{username}", ctx.DeleteSkinByUsername).Methods("DELETE") // 404 router.NotFoundHandler = http.HandlerFunc(NotFound) @@ -139,10 +115,6 @@ func (ctx *Skinsystem) CreateHandler() *mux.Router { } func (ctx *Skinsystem) Skin(response http.ResponseWriter, request *http.Request) { - if mux.Vars(request)["converted"] == "" { - ctx.Logger.IncCounter("skins.request", 1) - } - username := parseUsername(mux.Vars(request)["username"]) rec, err := ctx.SkinsRepo.FindByUsername(username) if err == nil && rec.SkinId != 0 { @@ -173,7 +145,6 @@ func (ctx *Skinsystem) SkinGET(response http.ResponseWriter, request *http.Reque return } - ctx.Logger.IncCounter("skins.get_request", 1) mux.Vars(request)["username"] = username mux.Vars(request)["converted"] = "1" @@ -181,10 +152,6 @@ func (ctx *Skinsystem) SkinGET(response http.ResponseWriter, request *http.Reque } func (ctx *Skinsystem) Cape(response http.ResponseWriter, request *http.Request) { - if mux.Vars(request)["converted"] == "" { - ctx.Logger.IncCounter("capes.request", 1) - } - username := parseUsername(mux.Vars(request)["username"]) rec, err := ctx.CapesRepo.FindByUsername(username) if err == nil { @@ -216,7 +183,6 @@ func (ctx *Skinsystem) CapeGET(response http.ResponseWriter, request *http.Reque return } - ctx.Logger.IncCounter("capes.get_request", 1) mux.Vars(request)["username"] = username mux.Vars(request)["converted"] = "1" @@ -224,7 +190,6 @@ func (ctx *Skinsystem) CapeGET(response http.ResponseWriter, request *http.Reque } func (ctx *Skinsystem) Textures(response http.ResponseWriter, request *http.Request) { - ctx.Logger.IncCounter("textures.request", 1) username := parseUsername(mux.Vars(request)["username"]) var textures *mojang.TexturesResponse @@ -261,8 +226,8 @@ func (ctx *Skinsystem) Textures(response http.ResponseWriter, request *http.Requ texturesProp := mojangTextures.DecodeTextures() if texturesProp == nil { - response.WriteHeader(http.StatusInternalServerError) - ctx.Logger.Error("Unable to find textures property") + ctx.Emitter.Emit("skinsystem.error", errors.New("unable to find textures property")) + apiServerError(response) return } @@ -275,7 +240,6 @@ func (ctx *Skinsystem) Textures(response http.ResponseWriter, request *http.Requ } func (ctx *Skinsystem) SignedTextures(response http.ResponseWriter, request *http.Request) { - ctx.Logger.IncCounter("signed_textures.request", 1) username := parseUsername(mux.Vars(request)["username"]) var responseData *mojang.SignedTexturesResponse @@ -316,10 +280,8 @@ func (ctx *Skinsystem) SignedTextures(response http.ResponseWriter, request *htt } func (ctx *Skinsystem) PostSkin(resp http.ResponseWriter, req *http.Request) { - ctx.Logger.IncCounter("api.skins.post.request", 1) validationErrors := validatePostSkinRequest(req) if validationErrors != nil { - ctx.Logger.IncCounter("api.skins.post.validation_failed", 1) apiBadRequest(resp, validationErrors) return } @@ -329,7 +291,7 @@ func (ctx *Skinsystem) PostSkin(resp http.ResponseWriter, req *http.Request) { record, err := findIdentity(ctx.SkinsRepo, identityId, username) if err != nil { - ctx.Logger.Error("Error on requesting a skin from the repository: :err", wd.ErrParam(err)) + ctx.Emitter.Emit("skinsystem:error", fmt.Errorf("error on requesting a skin from the repository: %w", err)) apiServerError(resp) return } @@ -348,71 +310,67 @@ func (ctx *Skinsystem) PostSkin(resp http.ResponseWriter, req *http.Request) { err = ctx.SkinsRepo.Save(record) if err != nil { - ctx.Logger.Error("Unable to save record to the repository: :err", wd.ErrParam(err)) + ctx.Emitter.Emit("skinsystem:error", fmt.Errorf("unable to save record to the repository: %w", err)) apiServerError(resp) return } - ctx.Logger.IncCounter("api.skins.post.success", 1) resp.WriteHeader(http.StatusCreated) } func (ctx *Skinsystem) DeleteSkinByUserId(resp http.ResponseWriter, req *http.Request) { - ctx.Logger.IncCounter("api.skins.delete.request", 1) id, _ := strconv.Atoi(mux.Vars(req)["id"]) skin, err := ctx.SkinsRepo.FindByUserId(id) - if err != nil { - ctx.Logger.IncCounter("api.skins.delete.not_found", 1) - apiNotFound(resp, "Cannot find record for requested user id") - return - } - - ctx.deleteSkin(skin, resp) + ctx.deleteSkin(skin, err, resp) } func (ctx *Skinsystem) DeleteSkinByUsername(resp http.ResponseWriter, req *http.Request) { - ctx.Logger.IncCounter("api.skins.delete.request", 1) username := mux.Vars(req)["username"] skin, err := ctx.SkinsRepo.FindByUsername(username) - if err != nil { - ctx.Logger.IncCounter("api.skins.delete.not_found", 1) - apiNotFound(resp, "Cannot find record for requested username") - return - } - - ctx.deleteSkin(skin, resp) + ctx.deleteSkin(skin, err, resp) } func (ctx *Skinsystem) AuthenticationMiddleware(handler http.Handler) http.Handler { return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { - ctx.Logger.IncCounter("authentication.challenge", 1) + // TODO: decide on how I would cover this with logging + // ctx.Logger.IncCounter("authentication.challenge", 1) err := ctx.Auth.Check(req) if err != nil { if _, ok := err.(*auth.Unauthorized); ok { - ctx.Logger.IncCounter("authentication.failed", 1) + // ctx.Logger.IncCounter("authentication.failed", 1) apiForbidden(resp, err.Error()) } else { - ctx.Logger.Error("Unknown error on validating api request: :err", wd.ErrParam(err)) + // ctx.Logger.Error("Unknown error on validating api request: :err", wd.ErrParam(err)) apiServerError(resp) } return } - ctx.Logger.IncCounter("authentication.success", 1) + // ctx.Logger.IncCounter("authentication.success", 1) handler.ServeHTTP(resp, req) }) } -func (ctx *Skinsystem) deleteSkin(skin *model.Skin, resp http.ResponseWriter) { - err := ctx.SkinsRepo.RemoveByUserId(skin.UserId) +func (ctx *Skinsystem) deleteSkin(skin *model.Skin, err error, resp http.ResponseWriter) { if err != nil { - ctx.Logger.Error("Cannot delete skin by error: :err", wd.ErrParam(err)) + if _, ok := err.(*SkinNotFoundError); ok { + apiNotFound(resp, "Cannot find record for the requested identifier") + } else { + ctx.Emitter.Emit("skinsystem:error", fmt.Errorf("unable to find skin info from the repository: %w", err)) + apiServerError(resp) + } + + return + } + + err = ctx.SkinsRepo.RemoveByUserId(skin.UserId) + if err != nil { + ctx.Emitter.Emit("skinsystem:error", fmt.Errorf("cannot delete skin by error: %w", err)) apiServerError(resp) return } - ctx.Logger.IncCounter("api.skins.delete.success", 1) resp.WriteHeader(http.StatusNoContent) } diff --git a/http/skinsystem_test.go b/http/skinsystem_test.go index 463eeb2..78bc29c 100644 --- a/http/skinsystem_test.go +++ b/http/skinsystem_test.go @@ -3,8 +3,7 @@ package http import ( "bytes" "encoding/base64" - "github.com/elyby/chrly/auth" - testify "github.com/stretchr/testify/assert" + "errors" "image" "image/png" "io" @@ -16,12 +15,13 @@ import ( "testing" "time" + testify "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/suite" "github.com/elyby/chrly/api/mojang" + "github.com/elyby/chrly/auth" "github.com/elyby/chrly/model" - "github.com/elyby/chrly/tests" ) /*************** @@ -113,7 +113,7 @@ type skinsystemTestSuite struct { CapesRepository *capesRepositoryMock MojangTexturesProvider *mojangTexturesProviderMock Auth *authCheckerMock - Logger *tests.WdMock + Emitter *emitterMock } /******************** @@ -125,14 +125,14 @@ func (suite *skinsystemTestSuite) SetupTest() { suite.CapesRepository = &capesRepositoryMock{} suite.MojangTexturesProvider = &mojangTexturesProviderMock{} suite.Auth = &authCheckerMock{} - suite.Logger = &tests.WdMock{} + suite.Emitter = &emitterMock{} suite.App = &Skinsystem{ SkinsRepo: suite.SkinsRepository, CapesRepo: suite.CapesRepository, MojangTexturesProvider: suite.MojangTexturesProvider, Auth: suite.Auth, - Logger: suite.Logger, + Emitter: suite.Emitter, } } @@ -141,7 +141,7 @@ func (suite *skinsystemTestSuite) TearDownTest() { suite.CapesRepository.AssertExpectations(suite.T()) suite.MojangTexturesProvider.AssertExpectations(suite.T()) suite.Auth.AssertExpectations(suite.T()) - suite.Logger.AssertExpectations(suite.T()) + suite.Emitter.AssertExpectations(suite.T()) } func (suite *skinsystemTestSuite) RunSubTest(name string, subTest func()) { @@ -215,7 +215,6 @@ var skinsTestsCases = []*skinsystemTestCase{ func (suite *skinsystemTestSuite) TestSkin() { for _, testCase := range skinsTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "skins.request", int64(1)).Once() testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/skins/mock_username", nil) @@ -228,7 +227,6 @@ func (suite *skinsystemTestSuite) TestSkin() { } suite.RunSubTest("Pass username with png extension", func() { - suite.Logger.On("IncCounter", "skins.request", int64(1)).Once() suite.SkinsRepository.On("FindByUsername", "mock_username").Return(createSkinModel("mock_username", false), nil) req := httptest.NewRequest("GET", "http://chrly/skins/mock_username.png", nil) @@ -245,7 +243,6 @@ func (suite *skinsystemTestSuite) TestSkin() { func (suite *skinsystemTestSuite) TestSkinGET() { for _, testCase := range skinsTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "skins.get_request", int64(1)).Once() testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/skins?name=mock_username", nil) @@ -321,7 +318,6 @@ var capesTestsCases = []*skinsystemTestCase{ func (suite *skinsystemTestSuite) TestCape() { for _, testCase := range capesTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "capes.request", int64(1)).Once() testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/cloaks/mock_username", nil) @@ -334,7 +330,6 @@ func (suite *skinsystemTestSuite) TestCape() { } suite.RunSubTest("Pass username with png extension", func() { - suite.Logger.On("IncCounter", "capes.request", int64(1)).Once() suite.CapesRepository.On("FindByUsername", "mock_username").Return(createCapeModel(), nil) req := httptest.NewRequest("GET", "http://chrly/cloaks/mock_username.png", nil) @@ -353,7 +348,6 @@ func (suite *skinsystemTestSuite) TestCape() { func (suite *skinsystemTestSuite) TestCapeGET() { for _, testCase := range capesTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "capes.get_request", int64(1)).Once() testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/cloaks?name=mock_username", nil) @@ -492,7 +486,6 @@ var texturesTestsCases = []*skinsystemTestCase{ func (suite *skinsystemTestSuite) TestTextures() { for _, testCase := range texturesTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "textures.request", int64(1)).Once() testCase.BeforeTest(suite) req := httptest.NewRequest("GET", "http://chrly/textures/mock_username", nil) @@ -616,7 +609,6 @@ var signedTexturesTestsCases = []*signedTexturesTestCase{ func (suite *skinsystemTestSuite) TestSignedTextures() { for _, testCase := range signedTexturesTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "signed_textures.request", int64(1)).Once() testCase.BeforeTest(suite) var target string @@ -769,15 +761,62 @@ var postSkinTestsCases = []*postSkinTestCase{ suite.Empty(body) }, }, + { + Name: "Handle an error when loading the data from the repository", + Form: bytes.NewBufferString(url.Values{ + "identityId": {"1"}, + "username": {"mock_username"}, + "uuid": {"0f657aa8-bfbe-415d-b700-5750090d3af3"}, + "skinId": {"5"}, + "is1_8": {"1"}, + "isSlim": {"1"}, + "url": {"http://textures-server.com/skin.png"}, + }.Encode()), + BeforeTest: func(suite *skinsystemTestSuite) { + suite.SkinsRepository.On("FindByUserId", 1).Return(createSkinModel("mock_username", false), nil) + err := errors.New("mock error") + suite.SkinsRepository.On("Save", mock.Anything).Return(err) + suite.Emitter.On("Emit", "skinsystem:error", mock.MatchedBy(func(cErr error) bool { + return cErr.Error() == "unable to save record to the repository: mock error" && + errors.Is(cErr, err) + })).Once() + }, + AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { + suite.Equal(500, response.StatusCode) + body, _ := ioutil.ReadAll(response.Body) + suite.Empty(body) + }, + }, + { + Name: "Handle an error when saving the data into the repository", + Form: bytes.NewBufferString(url.Values{ + "identityId": {"1"}, + "username": {"changed_username"}, + "uuid": {"0f657aa8-bfbe-415d-b700-5750090d3af3"}, + "skinId": {"5"}, + "is1_8": {"0"}, + "isSlim": {"0"}, + "url": {"http://example.com/skin.png"}, + }.Encode()), + BeforeTest: func(suite *skinsystemTestSuite) { + err := errors.New("mock error") + suite.SkinsRepository.On("FindByUserId", 1).Return(nil, err) + suite.Emitter.On("Emit", "skinsystem:error", mock.MatchedBy(func(cErr error) bool { + return cErr.Error() == "error on requesting a skin from the repository: mock error" && + errors.Is(cErr, err) + })).Once() + }, + AfterTest: func(suite *skinsystemTestSuite, response *http.Response) { + suite.Equal(500, response.StatusCode) + body, _ := ioutil.ReadAll(response.Body) + suite.Empty(body) + }, + }, } func (suite *skinsystemTestSuite) TestPostSkin() { for _, testCase := range postSkinTestsCases { suite.RunSubTest(testCase.Name, func() { - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.post.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.post.success", int64(1)).Once() suite.Auth.On("Check", mock.Anything).Return(nil) testCase.BeforeTest(suite) @@ -792,10 +831,6 @@ func (suite *skinsystemTestSuite) TestPostSkin() { } suite.RunSubTest("Get errors about required fields", func() { - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.post.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.post.validation_failed", int64(1)).Once() suite.Auth.On("Check", mock.Anything).Return(nil) req := httptest.NewRequest("POST", "http://chrly/api/skins", bytes.NewBufferString(url.Values{ @@ -848,8 +883,6 @@ func (suite *skinsystemTestSuite) TestPostSkin() { w := httptest.NewRecorder() suite.Auth.On("Check", mock.Anything).Return(&auth.Unauthorized{Reason: "Cannot parse passed JWT token"}) - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.failed", int64(1)).Once() suite.App.CreateHandler().ServeHTTP(w, req) @@ -863,10 +896,6 @@ func (suite *skinsystemTestSuite) TestPostSkin() { }) suite.RunSubTest("Upload textures with skin as file", func() { - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.post.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.post.validation_failed", int64(1)).Once() suite.Auth.On("Check", mock.Anything).Return(nil) inputBody := &bytes.Buffer{} @@ -914,10 +943,6 @@ func (suite *skinsystemTestSuite) TestDeleteByUserId() { suite.Auth.On("Check", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUserId", 1).Return(createSkinModel("mock_username", false), nil) suite.SkinsRepository.On("RemoveByUserId", 1).Once().Return(nil) - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.success", int64(1)).Once() req := httptest.NewRequest("DELETE", "http://chrly/api/skins/id:1", nil) w := httptest.NewRecorder() @@ -934,10 +959,6 @@ func (suite *skinsystemTestSuite) TestDeleteByUserId() { suite.RunSubTest("Try to remove not exists identity id", func() { suite.Auth.On("Check", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUserId", 1).Return(nil, &SkinNotFoundError{Who: "unknown"}) - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.not_found", int64(1)).Once() req := httptest.NewRequest("DELETE", "http://chrly/api/skins/id:1", nil) w := httptest.NewRecorder() @@ -949,7 +970,7 @@ func (suite *skinsystemTestSuite) TestDeleteByUserId() { suite.Equal(404, resp.StatusCode) body, _ := ioutil.ReadAll(resp.Body) suite.JSONEq(`[ - "Cannot find record for requested user id" + "Cannot find record for the requested identifier" ]`, string(body)) }) } @@ -963,10 +984,6 @@ func (suite *skinsystemTestSuite) TestDeleteByUsername() { suite.Auth.On("Check", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUsername", "mock_username").Return(createSkinModel("mock_username", false), nil) suite.SkinsRepository.On("RemoveByUserId", 1).Once().Return(nil) - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.success", int64(1)).Once() req := httptest.NewRequest("DELETE", "http://chrly/api/skins/mock_username", nil) w := httptest.NewRecorder() @@ -983,10 +1000,6 @@ func (suite *skinsystemTestSuite) TestDeleteByUsername() { suite.RunSubTest("Try to remove not exists identity username", func() { suite.Auth.On("Check", mock.Anything).Return(nil) suite.SkinsRepository.On("FindByUsername", "mock_username").Return(nil, &SkinNotFoundError{Who: "mock_username"}) - suite.Logger.On("IncCounter", "authentication.challenge", int64(1)).Once() - suite.Logger.On("IncCounter", "authentication.success", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.request", int64(1)).Once() - suite.Logger.On("IncCounter", "api.skins.delete.not_found", int64(1)).Once() req := httptest.NewRequest("DELETE", "http://chrly/api/skins/mock_username", nil) w := httptest.NewRecorder() @@ -998,7 +1011,7 @@ func (suite *skinsystemTestSuite) TestDeleteByUsername() { suite.Equal(404, resp.StatusCode) body, _ := ioutil.ReadAll(resp.Body) suite.JSONEq(`[ - "Cannot find record for requested username" + "Cannot find record for the requested identifier" ]`, string(body)) }) } diff --git a/http/uuids_worker.go b/http/uuids_worker.go index f983a5e..fd4aa16 100644 --- a/http/uuids_worker.go +++ b/http/uuids_worker.go @@ -2,13 +2,9 @@ package http import ( "encoding/json" - "fmt" - "net" "net/http" - "time" "github.com/gorilla/mux" - "github.com/mono83/slf/wd" "github.com/elyby/chrly/api/mojang" "github.com/elyby/chrly/mojangtextures" @@ -19,31 +15,11 @@ type UuidsProvider interface { } type UUIDsWorker struct { - ListenSpec string - + Emitter UUIDsProvider mojangtextures.UUIDsProvider - Logger wd.Watchdog } -func (ctx *UUIDsWorker) Run() error { - ctx.Logger.Info(fmt.Sprintf("Starting the worker, HTTP on: %s\n", ctx.ListenSpec)) - - listener, err := net.Listen("tcp", ctx.ListenSpec) - if err != nil { - return err - } - - server := &http.Server{ - ReadTimeout: 60 * time.Second, - WriteTimeout: 60 * time.Second, // TODO: should I adjust this values? - MaxHeaderBytes: 1 << 16, - Handler: ctx.CreateHandler(), - } - - return server.Serve(listener) -} - -func (ctx *UUIDsWorker) CreateHandler() http.Handler { +func (ctx *UUIDsWorker) CreateHandler() *mux.Router { router := mux.NewRouter().StrictSlash(true) router.NotFoundHandler = http.HandlerFunc(NotFound) @@ -56,13 +32,12 @@ func (ctx *UUIDsWorker) GetUUID(response http.ResponseWriter, request *http.Requ username := parseUsername(mux.Vars(request)["username"]) profile, err := ctx.UUIDsProvider.GetUuid(username) if err != nil { + ctx.Emitter.Emit("uuids_provider:error", err) // TODO: do I need emitter here? if _, ok := err.(*mojang.TooManyRequestsError); ok { - ctx.Logger.Warning("Got 429 Too Many Requests") response.WriteHeader(http.StatusTooManyRequests) return } - ctx.Logger.Warning("Got non success response: :err", wd.ErrParam(err)) response.Header().Set("Content-Type", "application/json") response.WriteHeader(http.StatusInternalServerError) result, _ := json.Marshal(map[string]interface{}{ diff --git a/http/uuids_worker_test.go b/http/uuids_worker_test.go index ea0e64e..0271dd8 100644 --- a/http/uuids_worker_test.go +++ b/http/uuids_worker_test.go @@ -2,14 +2,15 @@ package http import ( "errors" - "github.com/elyby/chrly/api/mojang" - "github.com/elyby/chrly/tests" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/suite" "io/ioutil" "net/http" "net/http/httptest" "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/suite" + + "github.com/elyby/chrly/api/mojang" ) /*************** @@ -36,7 +37,7 @@ type uuidsWorkerTestSuite struct { App *UUIDsWorker UuidsProvider *uuidsProviderMock - Logger *tests.WdMock + Emitter *emitterMock } /******************** @@ -45,17 +46,17 @@ type uuidsWorkerTestSuite struct { func (suite *uuidsWorkerTestSuite) SetupTest() { suite.UuidsProvider = &uuidsProviderMock{} - suite.Logger = &tests.WdMock{} + suite.Emitter = &emitterMock{} suite.App = &UUIDsWorker{ UUIDsProvider: suite.UuidsProvider, - Logger: suite.Logger, + Emitter: suite.Emitter, } } func (suite *uuidsWorkerTestSuite) TearDownTest() { suite.UuidsProvider.AssertExpectations(suite.T()) - suite.Logger.AssertExpectations(suite.T()) + suite.Emitter.AssertExpectations(suite.T()) } func (suite *uuidsWorkerTestSuite) RunSubTest(name string, subTest func()) { @@ -115,8 +116,9 @@ var getUuidTestsCases = []*uuidsWorkerTestCase{ { Name: "Receive error from UUIDs provider", BeforeTest: func(suite *uuidsWorkerTestSuite) { - suite.UuidsProvider.On("GetUuid", "mock_username").Return(nil, errors.New("this is an error")) - suite.Logger.On("Warning", "Got non success response: :err", mock.Anything).Times(1) + err := errors.New("this is an error") + suite.UuidsProvider.On("GetUuid", "mock_username").Return(nil, err) + suite.Emitter.On("Emit", "uuids_provider:error", err).Times(1) }, AfterTest: func(suite *uuidsWorkerTestSuite, response *http.Response) { suite.Equal(500, response.StatusCode) @@ -130,8 +132,9 @@ var getUuidTestsCases = []*uuidsWorkerTestCase{ { Name: "Receive Too Many Requests from UUIDs provider", BeforeTest: func(suite *uuidsWorkerTestSuite) { - suite.UuidsProvider.On("GetUuid", "mock_username").Return(nil, &mojang.TooManyRequestsError{}) - suite.Logger.On("Warning", "Got 429 Too Many Requests").Times(1) + err := &mojang.TooManyRequestsError{} + suite.UuidsProvider.On("GetUuid", "mock_username").Return(nil, err) + suite.Emitter.On("Emit", "uuids_provider:error", err).Times(1) }, AfterTest: func(suite *uuidsWorkerTestSuite, response *http.Response) { suite.Equal(429, response.StatusCode)