From 49d85d60f443bebb12ef78d0729a34430c3590ed Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Mon, 15 Jul 2024 11:45:02 +0300 Subject: [PATCH 01/11] added basic iter6 fixes --- cmd/agent/main.go | 14 ++++---- cmd/server/main.go | 16 +++++---- go.mod | 2 ++ go.sum | 4 +++ internal/agent/agent.go | 9 +++-- internal/agent/api.go | 13 +++++--- internal/agent/stats.go | 5 ++- internal/logging/logging.go | 29 ++++++++++------ internal/logging/logging_test.go | 5 ++- internal/logging/middleware.go | 51 ++++++++++++++++++++++++++++ internal/server/handlers.go | 57 +++++++++++++++++++------------- internal/server/router.go | 8 +++-- 12 files changed, 155 insertions(+), 58 deletions(-) create mode 100644 internal/logging/middleware.go diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 8ffb0ec..d1b5d8c 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -1,28 +1,30 @@ package main import ( + "context" + "github.com/ex0rcist/metflix/internal/agent" "github.com/ex0rcist/metflix/internal/logging" ) func main() { - logging.Setup() + ctx := logging.Setup(context.Background()) - logging.LogInfo("starting agent...") + logging.LogInfo(ctx, "starting agent...") agnt, err := agent.New() if err != nil { - logging.LogFatal(err) + logging.LogFatal(ctx, err) } err = agnt.ParseFlags() if err != nil { - logging.LogFatal(err) + logging.LogFatal(ctx, err) } - logging.LogInfo(agnt.Config.String()) + logging.LogInfo(ctx, agnt.Config.String()) agnt.Run() - logging.LogInfo("agent ready") + logging.LogInfo(ctx, "agent ready") } diff --git a/cmd/server/main.go b/cmd/server/main.go index 0a1e3a4..aa8bd71 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -1,30 +1,32 @@ package main import ( + "context" + "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/server" ) func main() { - logging.Setup() + ctx := logging.Setup(context.Background()) - logging.LogInfo("starting server...") + logging.LogInfo(ctx, "starting server...") srv, err := server.New() if err != nil { - logging.LogFatal(err) + logging.LogFatal(ctx, err) } err = srv.ParseFlags() if err != nil { - logging.LogFatal(err) + logging.LogFatal(ctx, err) } - logging.LogInfo(srv.Config.String()) - logging.LogInfo("server ready") // TODO: must be after run? + logging.LogInfo(ctx, srv.Config.String()) + logging.LogInfo(ctx, "server ready") // TODO: must be after run? err = srv.Run() if err != nil { - logging.LogFatal(err) + logging.LogFatal(ctx, err) } } diff --git a/go.mod b/go.mod index b665d6e..c564eb1 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,11 @@ go 1.22.4 require ( github.com/caarlos0/env/v11 v11.1.0 + github.com/go-chi/chi v1.5.5 github.com/go-chi/chi/v5 v5.1.0 github.com/pkg/errors v0.9.1 github.com/rs/zerolog v1.33.0 + github.com/satori/go.uuid v1.2.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 ) diff --git a/go.sum b/go.sum index 25ff8a3..b123195 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,8 @@ github.com/caarlos0/env/v11 v11.1.0/go.mod h1:LwgkYk1kDvfGpHthrWWLof3Ny7PezzFwS4 github.com/coreos/go-systemd/v22 v22.5.0/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-chi/chi v1.5.5 h1:vOB/HbEMt9QqBqErz07QehcOKHaWFtuj87tTDVz2qXE= +github.com/go-chi/chi v1.5.5/go.mod h1:C9JqLr3tIYjDOZpzn+BCuxY8z8vmca43EeMgyZt7irw= github.com/go-chi/chi/v5 v5.1.0 h1:acVI1TYaD+hhedDJ3r54HyA6sExp3HfXq7QWEEY/xMw= github.com/go-chi/chi/v5 v5.1.0/go.mod h1:DslCQbL2OYiznFReuXYUmQ2hGd1aDpCnlMNITLSKoi8= github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= @@ -18,6 +20,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/rs/xid v1.5.0/go.mod h1:trrq9SKmegXys3aeAKXMUTdJsYXVwGY3RLcfgqegfbg= github.com/rs/zerolog v1.33.0 h1:1cU2KZkvPxNyfgEmhHAz/1A9Bz+llsdYzklWFzgp0r8= github.com/rs/zerolog v1.33.0/go.mod h1:/7mN4D5sKwJLZQ2b/znpjC3/GQWY/xaDXUM0kKWRHss= +github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww= +github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= diff --git a/internal/agent/agent.go b/internal/agent/agent.go index c964619..c0a93e0 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1,6 +1,7 @@ package agent import ( + "context" "fmt" "sync" "time" @@ -79,10 +80,12 @@ func (a *Agent) Run() { func (a *Agent) startPolling() { defer a.wg.Done() + ctx := context.Background() + for { err := a.Stats.Poll() if err != nil { - logging.LogError(err) + logging.LogError(ctx, err) } time.Sleep(intToDuration(a.Config.PollInterval)) @@ -100,7 +103,9 @@ func (a *Agent) startReporting() { } func (a *Agent) reportStats() { - logging.LogInfo("reporting stats ... ") + ctx := context.Background() + + logging.LogInfo(ctx, "reporting stats ... ") // agent continues polling while report is in progress, take snapshot? snapshot := *a.Stats diff --git a/internal/agent/api.go b/internal/agent/api.go index d7550cb..173e920 100644 --- a/internal/agent/api.go +++ b/internal/agent/api.go @@ -1,6 +1,7 @@ package agent import ( + "context" "fmt" "io" "net/http" @@ -33,33 +34,35 @@ func NewAPI(address *entities.Address, httpTransport http.RoundTripper) *API { } func (c *API) Report(name string, metric metrics.Metric) *API { + ctx := context.Background() + // todo: another transport? url := "http://" + c.address.String() + fmt.Sprintf("/update/%s/%s/%s", metric.Kind(), name, metric) req, err := http.NewRequest(http.MethodPost, url, http.NoBody) if err != nil { - logging.LogError(err, "httpRequest error") + logging.LogError(ctx, err, "httpRequest error") } req.Header.Set("Content-Type", "text/plain") - logging.LogInfo(fmt.Sprintf("sending POST to %v", url)) + logging.LogInfo(ctx, fmt.Sprintf("sending POST to %v", url)) resp, err := c.httpClient.Do(req) if err != nil { - logging.LogError(err, "httpClient error") + logging.LogError(ctx, err, "httpClient error") return c } defer resp.Body.Close() respBody, err := io.ReadAll(resp.Body) // нужно прочитать ответ для keepalive? if err != nil { - logging.LogError(entities.ErrMetricReport, "error reading response body") + logging.LogError(ctx, entities.ErrMetricReport, "error reading response body") } if resp.StatusCode != http.StatusOK { - logging.LogError(entities.ErrMetricReport, string(respBody)) + logging.LogError(ctx, entities.ErrMetricReport, string(respBody)) } return c diff --git a/internal/agent/stats.go b/internal/agent/stats.go index 743bd2d..57154d3 100644 --- a/internal/agent/stats.go +++ b/internal/agent/stats.go @@ -1,6 +1,7 @@ package agent import ( + "context" "math/rand" "time" @@ -22,7 +23,9 @@ func NewStats() *Stats { } func (m *Stats) Poll() error { - logging.LogInfo("polling stats ... ") + ctx := context.Background() + + logging.LogInfo(ctx, "polling stats ... ") m.PollCount++ m.RandomValue = metrics.Gauge(m.generator.Float64()) diff --git a/internal/logging/logging.go b/internal/logging/logging.go index d7a3c2f..be59379 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -1,6 +1,7 @@ package logging import ( + "context" "os" "strings" "time" @@ -11,32 +12,38 @@ import ( "github.com/rs/zerolog/pkgerrors" ) -func Setup() { - zerolog.ErrorStackMarshaler = pkgerrors.MarshalStack +func Setup(ctx context.Context) context.Context { + output := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339Nano} + // output := os.Stdout + logger := zerolog.New(output).With().Timestamp().Logger() + + // set global logger + log.Logger = logger - output := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339} + zerolog.ErrorStackMarshaler = pkgerrors.MarshalStack + zerolog.DefaultContextLogger = &logger // HELP: is it bad? -> logging.LogInfo(context.Background(), "some message") - l := zerolog.New(output).With().Timestamp() - log.Logger = l.Logger() + // added logger to ctx + return logger.WithContext(ctx) } func NewError(err error) error { // wrap err to pkg/errors return errors.New(err.Error()) // TODO: can we remove this func from stack? } -func LogError(err error, messages ...string) { +func LogError(ctx context.Context, err error, messages ...string) { msg := optMessagesToString(messages) - log.Error().Stack().Err(err).Msg(msg) + log.Ctx(ctx).Error().Stack().Err(err).Msg(msg) } -func LogFatal(err error, messages ...string) { +func LogFatal(ctx context.Context, err error, messages ...string) { msg := optMessagesToString(messages) - log.Fatal().Stack().Err(err).Msg(msg) + log.Ctx(ctx).Fatal().Stack().Err(err).Msg(msg) } -func LogInfo(messages ...string) { +func LogInfo(ctx context.Context, messages ...string) { msg := optMessagesToString(messages) - log.Info().Msg(msg) + log.Ctx(ctx).Info().Msg(msg) } func optMessagesToString(messages []string) string { diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index ea0bf4e..4cd9634 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -1,6 +1,7 @@ package logging_test import ( + "context" "testing" "github.com/ex0rcist/metflix/internal/logging" @@ -9,5 +10,7 @@ import ( func TestSetup(t *testing.T) { require := require.New(t) - require.NotPanics(logging.Setup) + ctx := context.Background() + + require.NotPanics(func() { logging.Setup(ctx) }) } diff --git a/internal/logging/middleware.go b/internal/logging/middleware.go new file mode 100644 index 0000000..fd1e332 --- /dev/null +++ b/internal/logging/middleware.go @@ -0,0 +1,51 @@ +package logging + +import ( + "net/http" + "time" + + "github.com/go-chi/chi/middleware" + "github.com/rs/zerolog/log" + uuid "github.com/satori/go.uuid" +) + +func RequestsLogger(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requestID := findOrCreateRequestID(r) + start := time.Now() + + // setup child logger for middleware + logger := log.Logger.With(). + Str("rid", requestID). + Logger() + + // log started + logger.Info(). + Str("method", r.Method). + Str("url", r.URL.String()). + Str("remote-addr", r.RemoteAddr). // middleware.RealIP + Msg("Started") + + // execute + ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) + ctx := logger.WithContext(r.Context()) + next.ServeHTTP(ww, r.WithContext(ctx)) + + // log completed + logger.Info(). + Float64("elapsed", time.Since(start).Seconds()). + Int("status", ww.Status()). + Int("size", ww.BytesWritten()). + Msg("Completed") + }) +} + +func findOrCreateRequestID(r *http.Request) string { + requestID := r.Header.Get("X-Request-Id") + + if requestID == "" { + requestID = uuid.NewV4().String() + } + + return requestID +} diff --git a/internal/server/handlers.go b/internal/server/handlers.go index bcfbf05..564601b 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -1,6 +1,7 @@ package server import ( + "context" "errors" "fmt" "net/http" @@ -23,8 +24,8 @@ func NewResource(s storage.Storage) Resource { } } -func writeErrorResponse(w http.ResponseWriter, code int, err error) { - logging.LogError(err) +func writeErrorResponse(ctx context.Context, w http.ResponseWriter, code int, err error) { + logging.LogError(ctx, err) w.WriteHeader(code) // only header for now @@ -32,7 +33,9 @@ func writeErrorResponse(w http.ResponseWriter, code int, err error) { // http.Error(w, resp, code) } -func (r Resource) Homepage(res http.ResponseWriter, _ *http.Request) { +func (r Resource) Homepage(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + body := fmt.Sprintln("mainpage here.") // todo: errors @@ -45,29 +48,35 @@ func (r Resource) Homepage(res http.ResponseWriter, _ *http.Request) { } } - _, err := res.Write([]byte(body)) + _, err := rw.Write([]byte(body)) if err != nil { - logging.LogError(err) + logging.LogError(ctx, err) } } -func (r Resource) UpdateMetric(res http.ResponseWriter, req *http.Request) { +func (r Resource) UpdateMetric(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + // logger := zerolog.Ctx(req.Context()) + metricName := req.PathValue("metricName") metricKind := req.PathValue("metricKind") metricValue := req.PathValue("metricValue") + //logger.Info().Msg(fmt.Sprintf("updating1 metric... %v", metricName)) + logging.LogInfo(ctx, fmt.Sprintf("updating metric... %v", metricName)) + if err := validators.EnsureNamePresent(metricName); err != nil { - writeErrorResponse(res, http.StatusNotFound, err) + writeErrorResponse(ctx, rw, http.StatusNotFound, err) return } if err := validators.ValidateName(metricName); err != nil { - writeErrorResponse(res, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } if err := validators.ValidateKind(metricKind); err != nil { - writeErrorResponse(res, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } @@ -83,7 +92,7 @@ func (r Resource) UpdateMetric(res http.ResponseWriter, req *http.Request) { if err != nil && errors.Is(err, entities.ErrMetricNotFound) { currentValue = 0 } else if err != nil { - writeErrorResponse(res, http.StatusInternalServerError, err) + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) return } else { currentValue = int64(current.Value.(metrics.Counter)) @@ -91,7 +100,7 @@ func (r Resource) UpdateMetric(res http.ResponseWriter, req *http.Request) { incr, err := strconv.ParseInt(metricValue, 10, 64) if err != nil { - writeErrorResponse(res, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } currentValue += incr @@ -100,41 +109,43 @@ func (r Resource) UpdateMetric(res http.ResponseWriter, req *http.Request) { case "gauge": current, err := strconv.ParseFloat(metricValue, 64) if err != nil { - writeErrorResponse(res, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } rec = storage.Record{Name: metricName, Value: metrics.Gauge(current)} default: - writeErrorResponse(res, http.StatusBadRequest, entities.ErrMetricUnknown) + writeErrorResponse(ctx, rw, http.StatusBadRequest, entities.ErrMetricUnknown) return } err := r.storage.Push(rec) if err != nil { - writeErrorResponse(res, http.StatusInternalServerError, err) + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) return } - res.WriteHeader(http.StatusOK) + rw.WriteHeader(http.StatusOK) } -func (r Resource) ShowMetric(res http.ResponseWriter, req *http.Request) { +func (r Resource) ShowMetric(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + metricName := req.PathValue("metricName") metricKind := req.PathValue("metricKind") if err := validators.EnsureNamePresent(metricName); err != nil { - writeErrorResponse(res, http.StatusNotFound, err) + writeErrorResponse(ctx, rw, http.StatusNotFound, err) return } if err := validators.ValidateName(metricName); err != nil { - writeErrorResponse(res, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } if err := validators.ValidateKind(metricKind); err != nil { - writeErrorResponse(res, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } @@ -143,17 +154,17 @@ func (r Resource) ShowMetric(res http.ResponseWriter, req *http.Request) { recordID := storage.CalculateRecordID(metricName, metricKind) record, err := r.storage.Get(recordID) if err != nil { - writeErrorResponse(res, http.StatusNotFound, err) + writeErrorResponse(ctx, rw, http.StatusNotFound, err) return } body := record.Value.String() - res.WriteHeader(http.StatusOK) + rw.WriteHeader(http.StatusOK) - _, err = res.Write([]byte(body)) + _, err = rw.Write([]byte(body)) if err != nil { - writeErrorResponse(res, http.StatusInternalServerError, err) + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) return } } diff --git a/internal/server/router.go b/internal/server/router.go index fb3cb02..8e125cf 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -3,8 +3,10 @@ package server import ( "net/http" + "github.com/go-chi/chi/middleware" "github.com/go-chi/chi/v5" + "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/storage" ) @@ -12,9 +14,11 @@ func NewRouter(storage storage.Storage) http.Handler { router := chi.NewRouter() resource := Resource{storage: storage} + router.Use(middleware.RealIP) + router.Use(logging.RequestsLogger) + router.NotFound(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // no default body - w.WriteHeader(http.StatusNotFound) + w.WriteHeader(http.StatusNotFound) // no default body })) router.Get("/", resource.Homepage) // TODO: resource? From 6f881370bf81276868d95ba45637af238aecf06f Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Mon, 15 Jul 2024 14:13:17 +0300 Subject: [PATCH 02/11] iter6 fixes --- internal/logging/logging.go | 44 ++++++++++++++++++++++++++++++++----- internal/server/handlers.go | 2 -- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/internal/logging/logging.go b/internal/logging/logging.go index be59379..aa3462f 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -2,26 +2,41 @@ package logging import ( "context" + "fmt" + "io" "os" "strings" "time" + "github.com/caarlos0/env/v11" "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/rs/zerolog/pkgerrors" ) +type config struct { + Env string `env:"APP_ENV" envDefault:"development"` +} + func Setup(ctx context.Context) context.Context { - output := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339Nano} - // output := os.Stdout + cfg := parseConfig() + + var output io.Writer + switch cfg.Env { + case "development": + output = zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339Nano} + case "production": + output = os.Stdout + } + logger := zerolog.New(output).With().Timestamp().Logger() // set global logger log.Logger = logger zerolog.ErrorStackMarshaler = pkgerrors.MarshalStack - zerolog.DefaultContextLogger = &logger // HELP: is it bad? -> logging.LogInfo(context.Background(), "some message") + zerolog.DefaultContextLogger = &logger // added logger to ctx return logger.WithContext(ctx) @@ -33,17 +48,17 @@ func NewError(err error) error { // wrap err to pkg/errors func LogError(ctx context.Context, err error, messages ...string) { msg := optMessagesToString(messages) - log.Ctx(ctx).Error().Stack().Err(err).Msg(msg) + zerolog.Ctx(ctx).Error().Stack().Err(err).Msg(msg) } func LogFatal(ctx context.Context, err error, messages ...string) { msg := optMessagesToString(messages) - log.Ctx(ctx).Fatal().Stack().Err(err).Msg(msg) + zerolog.Ctx(ctx).Fatal().Stack().Err(err).Msg(msg) } func LogInfo(ctx context.Context, messages ...string) { msg := optMessagesToString(messages) - log.Ctx(ctx).Info().Msg(msg) + zerolog.Ctx(ctx).Info().Msg(msg) } func optMessagesToString(messages []string) string { @@ -53,3 +68,20 @@ func optMessagesToString(messages []string) string { return strings.Join(messages, "; ") } + +func parseConfig() config { + cfg := config{} + if err := env.Parse(&cfg); err != nil { + fmt.Printf("%+v\n", err) + } + + return cfg +} + +func (c config) isDevelopment() bool { + return c.Env == "development" +} + +func (c config) isProduction() bool { + return c.Env == "production" +} diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 564601b..5235bb7 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -56,13 +56,11 @@ func (r Resource) Homepage(rw http.ResponseWriter, req *http.Request) { func (r Resource) UpdateMetric(rw http.ResponseWriter, req *http.Request) { ctx := req.Context() - // logger := zerolog.Ctx(req.Context()) metricName := req.PathValue("metricName") metricKind := req.PathValue("metricKind") metricValue := req.PathValue("metricValue") - //logger.Info().Msg(fmt.Sprintf("updating1 metric... %v", metricName)) logging.LogInfo(ctx, fmt.Sprintf("updating metric... %v", metricName)) if err := validators.EnsureNamePresent(metricName); err != nil { From c6e21416cc4a752f26a932f0a06a91dbca8a34c2 Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Wed, 17 Jul 2024 16:36:06 +0300 Subject: [PATCH 03/11] iter6 fixes --- internal/logging/logging.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/internal/logging/logging.go b/internal/logging/logging.go index aa3462f..70ddb8b 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -77,11 +77,3 @@ func parseConfig() config { return cfg } - -func (c config) isDevelopment() bool { - return c.Env == "development" -} - -func (c config) isProduction() bool { - return c.Env == "production" -} From f5d497c45ffbbea67bc3e65a20d8d0d923248bad Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Wed, 17 Jul 2024 16:32:45 +0300 Subject: [PATCH 04/11] added basic iter7 features --- go.mod | 1 + go.sum | 2 + internal/entities/errors.go | 19 +- internal/metrics/types.go | 52 ++++- internal/metrics/types_test.go | 18 +- internal/server/converters.go | 52 +++++ internal/server/handlers.go | 194 ++++++++++------ internal/server/handlers_test.go | 310 ++++++++++++++++++++----- internal/server/router.go | 11 +- internal/server/server.go | 10 +- internal/storage/memory.go | 35 ++- internal/storage/memory_test.go | 25 +- internal/storage/record.go | 9 +- internal/storage/record_test.go | 8 +- internal/storage/service.go | 85 +++++++ internal/storage/service_mock.go | 32 +++ internal/storage/storage.go | 9 +- internal/validators/validators.go | 18 +- internal/validators/validators_test.go | 88 +------ 19 files changed, 705 insertions(+), 273 deletions(-) create mode 100644 internal/server/converters.go create mode 100644 internal/storage/service.go create mode 100644 internal/storage/service_mock.go diff --git a/go.mod b/go.mod index c564eb1..5508c4c 100644 --- a/go.mod +++ b/go.mod @@ -18,6 +18,7 @@ require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.19 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect golang.org/x/sys v0.21.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index b123195..2d08347 100644 --- a/go.sum +++ b/go.sum @@ -24,6 +24,8 @@ github.com/satori/go.uuid v1.2.0 h1:0uYX9dsZ2yD7q2RtLRtPSdGDWzjeM3TbMJP9utgA0ww= github.com/satori/go.uuid v1.2.0/go.mod h1:dA0hQrYB0VpLJoorglMZABFdXlWrHn1NEOzdhQKdks0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/internal/entities/errors.go b/internal/entities/errors.go index fcfb3b4..ee6cdfc 100644 --- a/internal/entities/errors.go +++ b/internal/entities/errors.go @@ -5,10 +5,17 @@ import "errors" var ( ErrBadAddressFormat = errors.New("bad net address format") - ErrMetricNotFound = errors.New("metric not found") - ErrMetricUnknown = errors.New("unknown metric type") - ErrMetricReport = errors.New("metric report error") - ErrMetricMissingName = errors.New("metric name is missing") - ErrMetricInvalidName = errors.New("metric name contains invalid characters") - ErrMetricLongName = errors.New("metric name is too long") + ErrMetricNotFound = errors.New("metric not found") + ErrMetricUnknown = errors.New("unknown metric type") + ErrMetricReport = errors.New("metric report error") + ErrMetricMissingName = errors.New("metric name is missing") + ErrMetricInvalidName = errors.New("metric name contains invalid characters") + ErrMetricLongName = errors.New("metric name is too long") + ErrMetricMissingValue = errors.New("metric value is missing") + ErrMetricInvalidValue = errors.New("metric value is invalid") + + ErrStoragePush = errors.New("failed to push record") + ErrStorageFetch = errors.New("failed to get record") + + ErrUnexpected = errors.New("unexpected error") ) diff --git a/internal/metrics/types.go b/internal/metrics/types.go index a25eb24..9e852d8 100644 --- a/internal/metrics/types.go +++ b/internal/metrics/types.go @@ -1,12 +1,18 @@ package metrics -import "strconv" +import ( + "strconv" + + "github.com/ex0rcist/metflix/internal/entities" +) type Metric interface { Kind() string String() string } +// Counter + type Counter int64 func (c Counter) Kind() string { @@ -17,6 +23,8 @@ func (c Counter) String() string { return strconv.FormatInt(int64(c), 10) } +// Gauge + type Gauge float64 func (g Gauge) Kind() string { @@ -26,3 +34,45 @@ func (g Gauge) Kind() string { func (g Gauge) String() string { return strconv.FormatFloat(float64(g), 'f', -1, 64) } + +func ToCounter(value string) (Counter, error) { + rawValue, err := strconv.ParseInt(value, 10, 64) + if err != nil { + return 0, entities.ErrMetricInvalidValue + } + + return Counter(rawValue), nil +} + +func ToGauge(value string) (Gauge, error) { + rawValue, err := strconv.ParseFloat(value, 64) + if err != nil { + return 0, entities.ErrMetricInvalidValue + } + + return Gauge(rawValue), nil +} + +// Agent/Server exchange schema according to spec +type MetricExchange struct { + ID string `json:"id"` + MType string `json:"type"` + Delta *Counter `json:"delta,omitempty"` + Value *Gauge `json:"value,omitempty"` +} + +func NewUpdateCounterMex(name string, value Counter) MetricExchange { + return MetricExchange{ID: name, MType: value.Kind(), Delta: &value} +} + +func NewUpdateGaugeMex(name string, value Gauge) MetricExchange { + return MetricExchange{ID: name, MType: value.Kind(), Value: &value} +} + +func NewGetCounterMex(name string) MetricExchange { + return MetricExchange{ID: name, MType: "counter"} +} + +func NewGetGaugeMex(name string) MetricExchange { + return MetricExchange{ID: name, MType: "gauge"} +} diff --git a/internal/metrics/types_test.go b/internal/metrics/types_test.go index f1845b8..15632d0 100644 --- a/internal/metrics/types_test.go +++ b/internal/metrics/types_test.go @@ -1,25 +1,23 @@ -package metrics_test +package metrics import ( "testing" - - "github.com/ex0rcist/metflix/internal/metrics" ) func TestKind(t *testing.T) { tests := []struct { name string - metric metrics.Metric + metric Metric want string }{ { name: "kind = counter", - metric: metrics.Counter(1), + metric: Counter(1), want: "counter", }, { name: "kind = gauge", - metric: metrics.Gauge(1.01), + metric: Gauge(1.01), want: "gauge", }, } @@ -35,22 +33,22 @@ func TestKind(t *testing.T) { func TestString(t *testing.T) { tests := []struct { name string - metric metrics.Metric + metric Metric want string }{ { name: "String() for counter", - metric: metrics.Counter(42), + metric: Counter(42), want: "42", }, { name: "String() for gauge", - metric: metrics.Gauge(42.01), + metric: Gauge(42.01), want: "42.01", }, { name: "String() for small gauge", - metric: metrics.Gauge(0.42), + metric: Gauge(0.42), want: "0.42", }, } diff --git a/internal/server/converters.go b/internal/server/converters.go new file mode 100644 index 0000000..72786c5 --- /dev/null +++ b/internal/server/converters.go @@ -0,0 +1,52 @@ +package server + +import ( + "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/metrics" + "github.com/ex0rcist/metflix/internal/storage" + "github.com/ex0rcist/metflix/internal/validators" +) + +// from JSON-struct to storage.Record +func toRecord(mex *metrics.MetricExchange) (storage.Record, error) { + var record storage.Record + + if err := validators.ValidateMetric(mex.ID, mex.MType); err != nil { + return record, err + } + + switch mex.MType { + case "counter": + if mex.Delta == nil { + return record, entities.ErrMetricMissingValue + } + + record = storage.Record{Name: mex.ID, Value: *mex.Delta} + case "gauge": + if mex.Value == nil { + return record, entities.ErrMetricMissingValue + } + + record = storage.Record{Name: mex.ID, Value: *mex.Value} + default: + return record, entities.ErrMetricUnknown + } + + return record, nil +} + +func toMetricExchange(record storage.Record) (*metrics.MetricExchange, error) { + req := &metrics.MetricExchange{ID: record.Name, MType: record.Value.Kind()} + + switch record.Value.Kind() { + case "counter": + delta, _ := record.Value.(metrics.Counter) + req.Delta = &delta + + case "gauge": + value, _ := record.Value.(metrics.Gauge) + req.Value = &value + } + + return req, nil +} diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 5235bb7..77ecf66 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -2,10 +2,10 @@ package server import ( "context" - "errors" + "encoding/json" "fmt" + "io" "net/http" - "strconv" "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/logging" @@ -14,13 +14,13 @@ import ( "github.com/ex0rcist/metflix/internal/validators" ) -type Resource struct { - storage storage.Storage +type MetricResource struct { + storageService storage.StorageService } -func NewResource(s storage.Storage) Resource { - return Resource{ - storage: s, +func NewMetricResource(storageService storage.StorageService) MetricResource { + return MetricResource{ + storageService: storageService, } } @@ -33,13 +33,16 @@ func writeErrorResponse(ctx context.Context, w http.ResponseWriter, code int, er // http.Error(w, resp, code) } -func (r Resource) Homepage(rw http.ResponseWriter, req *http.Request) { +func (r MetricResource) Homepage(rw http.ResponseWriter, req *http.Request) { ctx := req.Context() body := fmt.Sprintln("mainpage here.") - // todo: errors - records, _ := r.storage.GetAll() + records, err := r.storageService.List() + if err != nil { + writeErrorResponse(ctx, rw, errToStatus(err), err) + return + } if len(records) > 0 { body += fmt.Sprintln("metrics list:") @@ -48,111 +51,128 @@ func (r Resource) Homepage(rw http.ResponseWriter, req *http.Request) { } } - _, err := rw.Write([]byte(body)) + _, err = rw.Write([]byte(body)) if err != nil { logging.LogError(ctx, err) } } -func (r Resource) UpdateMetric(rw http.ResponseWriter, req *http.Request) { +func (r MetricResource) UpdateMetric(rw http.ResponseWriter, req *http.Request) { ctx := req.Context() - metricName := req.PathValue("metricName") - metricKind := req.PathValue("metricKind") - metricValue := req.PathValue("metricValue") - - logging.LogInfo(ctx, fmt.Sprintf("updating metric... %v", metricName)) - - if err := validators.EnsureNamePresent(metricName); err != nil { - writeErrorResponse(ctx, rw, http.StatusNotFound, err) - return - } - - if err := validators.ValidateName(metricName); err != nil { - writeErrorResponse(ctx, rw, http.StatusBadRequest, err) - return - } - - if err := validators.ValidateKind(metricKind); err != nil { - writeErrorResponse(ctx, rw, http.StatusBadRequest, err) - return + mex := metrics.MetricExchange{ + ID: req.PathValue("metricName"), + MType: req.PathValue("metricKind"), } - var rec storage.Record + rawValue := req.PathValue("metricValue") - switch metricKind { + switch mex.MType { case "counter": - var currentValue int64 - - recordID := storage.CalculateRecordID(metricName, metricKind) - current, err := r.storage.Get(recordID) - - if err != nil && errors.Is(err, entities.ErrMetricNotFound) { - currentValue = 0 - } else if err != nil { - writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) - return - } else { - currentValue = int64(current.Value.(metrics.Counter)) - } - - incr, err := strconv.ParseInt(metricValue, 10, 64) + delta, err := metrics.ToCounter(rawValue) if err != nil { - writeErrorResponse(ctx, rw, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, errToStatus(err), err) return } - currentValue += incr - rec = storage.Record{Name: metricName, Value: metrics.Counter(currentValue)} + mex.Delta = &delta case "gauge": - current, err := strconv.ParseFloat(metricValue, 64) + value, err := metrics.ToGauge(rawValue) if err != nil { - writeErrorResponse(ctx, rw, http.StatusBadRequest, err) + writeErrorResponse(ctx, rw, errToStatus(err), err) return } - rec = storage.Record{Name: metricName, Value: metrics.Gauge(current)} - default: - writeErrorResponse(ctx, rw, http.StatusBadRequest, entities.ErrMetricUnknown) + mex.Value = &value + } + + record, err := toRecord(&mex) + if err != nil { + writeErrorResponse(ctx, rw, errToStatus(err), err) return } - err := r.storage.Push(rec) + newRecord, err := r.storageService.Push(record) if err != nil { writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) return } rw.WriteHeader(http.StatusOK) + + if _, err = io.WriteString(rw, newRecord.Value.String()); err != nil { + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) + return + } } -func (r Resource) ShowMetric(rw http.ResponseWriter, req *http.Request) { +func (r MetricResource) UpdateMetricJSON(rw http.ResponseWriter, req *http.Request) { ctx := req.Context() - metricName := req.PathValue("metricName") - metricKind := req.PathValue("metricKind") + mex := new(metrics.MetricExchange) + if err := json.NewDecoder(req.Body).Decode(mex); err != nil { + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) + return + } - if err := validators.EnsureNamePresent(metricName); err != nil { - writeErrorResponse(ctx, rw, http.StatusNotFound, err) + record, err := toRecord(mex) + if err != nil { + writeErrorResponse(ctx, rw, errToStatus(err), err) return } - if err := validators.ValidateName(metricName); err != nil { - writeErrorResponse(ctx, rw, http.StatusBadRequest, err) + newRecord, err := r.storageService.Push(record) + if err != nil { + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) return } - if err := validators.ValidateKind(metricKind); err != nil { - writeErrorResponse(ctx, rw, http.StatusBadRequest, err) + mex, err = toMetricExchange(newRecord) + if err != nil { + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) return } - var record storage.Record + rw.Header().Set("Content-Type", "application/json") - recordID := storage.CalculateRecordID(metricName, metricKind) - record, err := r.storage.Get(recordID) + if err := json.NewEncoder(rw).Encode(mex); err != nil { + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) + return + } +} + +func errToStatus(err error) int { + switch err { + case entities.ErrMetricNotFound, entities.ErrMetricMissingName: + return http.StatusNotFound + case + entities.ErrMetricUnknown, entities.ErrMetricInvalidValue, + entities.ErrMetricInvalidName, entities.ErrMetricLongName, + entities.ErrMetricMissingValue: + + return http.StatusBadRequest + case entities.ErrUnexpected: + return http.StatusInternalServerError + default: + return http.StatusInternalServerError + } +} + +func (r MetricResource) GetMetric(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + + metricName := req.PathValue("metricName") + metricKind := req.PathValue("metricKind") + + if err := validators.ValidateMetric(metricName, metricKind); err != nil { + writeErrorResponse(ctx, rw, errToStatus(err), err) + return + } + + var record storage.Record + record, err := r.storageService.Get(metricName, metricKind) if err != nil { - writeErrorResponse(ctx, rw, http.StatusNotFound, err) + writeErrorResponse(ctx, rw, errToStatus(err), err) return } @@ -166,3 +186,37 @@ func (r Resource) ShowMetric(rw http.ResponseWriter, req *http.Request) { return } } + +func (r MetricResource) GetMetricJSON(rw http.ResponseWriter, req *http.Request) { + ctx := req.Context() + + mex := new(metrics.MetricExchange) + if err := json.NewDecoder(req.Body).Decode(mex); err != nil { + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) + return + } + + if err := validators.ValidateMetric(mex.ID, mex.MType); err != nil { + writeErrorResponse(ctx, rw, errToStatus(err), err) + return + } + + record, err := r.storageService.Get(mex.ID, mex.MType) + if err != nil { + writeErrorResponse(ctx, rw, errToStatus(err), err) + return + } + + mex, err = toMetricExchange(record) + if err != nil { + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) + return + } + + rw.Header().Set("Content-Type", "application/json") + + if err := json.NewEncoder(rw).Encode(mex); err != nil { + writeErrorResponse(ctx, rw, http.StatusInternalServerError, err) + return + } +} diff --git a/internal/server/handlers_test.go b/internal/server/handlers_test.go index 7424cda..d0ab2cb 100644 --- a/internal/server/handlers_test.go +++ b/internal/server/handlers_test.go @@ -1,21 +1,27 @@ -package server_test +package server import ( - "fmt" + "bytes" + "encoding/json" "io" "net/http" "net/http/httptest" "testing" + "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/metrics" - "github.com/ex0rcist/metflix/internal/server" "github.com/ex0rcist/metflix/internal/storage" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -// https://github.com/go-chi/chi/blob/cca4135d8dddff765463feaf1118047a9e506b4a/middleware/get_head_test.go -func testRequest(t *testing.T, ts *httptest.Server, method, path string, body io.Reader) (int, string, []byte) { +func testRequest(t *testing.T, router http.Handler, method, path string, payload []byte) (int, string, []byte) { + ts := httptest.NewServer(router) + defer ts.Close() + + body := bytes.NewReader(payload) + req, err := http.NewRequest(method, ts.URL+path, body) require.NoError(t, err) @@ -62,21 +68,14 @@ func TestHomepage(t *testing.T) { }, } - storage := storage.NewMemStorage() - router := server.NewRouter(storage) - testServer := httptest.NewServer(router) - defer testServer.Close() - for _, tt := range tests { - if len(tt.metrics) > 0 { - for _, r := range tt.metrics { - err := storage.Push(r) - require.NoError(err) - } - } + m := storage.ServiceMock{} + m.On("List").Return(tt.metrics, nil) + + router := NewRouter(&m) t.Run(tt.name, func(t *testing.T) { - code, _, body := testRequest(t, testServer, http.MethodGet, tt.path, nil) + code, _, body := testRequest(t, router, http.MethodGet, tt.path, nil) require.Equal(tt.want.code, code) @@ -101,28 +100,36 @@ func TestUpdateMetric(t *testing.T) { require := require.New(t) tests := []struct { - name string - path string - metrics []storage.Record - want result + name string + path string + mock func(m *storage.ServiceMock) + want result }{ { name: "push counter", path: "/update/counter/test/42", - want: result{code: http.StatusOK, body: ""}, + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "counter").Return(storage.Record{}, nil) + m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) + }, + want: result{code: http.StatusOK, body: "42"}, }, { name: "push counter with existing value", path: "/update/counter/test/42", - metrics: []storage.Record{ - {Name: "test", Value: metrics.Counter(42)}, + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "counter").Return(storage.Record{Name: "test", Value: metrics.Counter(21)}, nil) + m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) }, - want: result{code: http.StatusOK, body: ""}, + want: result{code: http.StatusOK, body: "42"}, }, { name: "push gauge", path: "/update/gauge/test/42.42", - want: result{code: http.StatusOK, body: ""}, + mock: func(m *storage.ServiceMock) { + m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}, nil) + }, + want: result{code: http.StatusOK, body: "42.42"}, }, { name: "fail on invalid kind", @@ -156,21 +163,17 @@ func TestUpdateMetric(t *testing.T) { }, } - strg := storage.NewMemStorage() - router := server.NewRouter(strg) - testServer := httptest.NewServer(router) - defer testServer.Close() - for _, tt := range tests { - if len(tt.metrics) > 0 { - for _, r := range tt.metrics { - err := strg.Push(r) - require.NoError(err) - } + m := new(storage.ServiceMock) + + if tt.mock != nil { + tt.mock(m) } + router := NewRouter(m) + t.Run(tt.name, func(t *testing.T) { - code, _, body := testRequest(t, testServer, http.MethodPost, tt.path, nil) + code, _, body := testRequest(t, router, http.MethodPost, tt.path, nil) require.Equal(tt.want.code, code) require.Equal(tt.want.body, string(body)) @@ -178,26 +181,118 @@ func TestUpdateMetric(t *testing.T) { } } -func TestShowMetric(t *testing.T) { +func TestUpdateJSONMetric(t *testing.T) { + type result struct { + code int + } + + tests := []struct { + name string + mex metrics.MetricExchange + mock func(m *storage.ServiceMock) + want result + }{ + { + name: "Should push counter", + mex: metrics.NewUpdateCounterMex("test", 42), + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "counter").Return(storage.Record{}, nil) + m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) + }, + want: result{code: http.StatusOK}, + }, + { + name: "Should push gauge", + mex: metrics.NewUpdateGaugeMex("test", 42.42), + mock: func(m *storage.ServiceMock) { + m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}, nil) + }, + want: result{code: http.StatusOK}, + }, + { + name: "Should fail on unknown metric kind", + mex: metrics.MetricExchange{ID: "42", MType: "test"}, + want: result{code: http.StatusBadRequest}, + }, + { + name: "Should fail on counter with invalid name", + mex: metrics.NewUpdateCounterMex("X)", 10), + want: result{code: http.StatusBadRequest}, + }, + { + name: "Should fail on gauge with invalid name", + mex: metrics.NewUpdateGaugeMex("X;", 13.123), + want: result{code: http.StatusBadRequest}, + }, + { + name: "Should fail on broken storage", + mex: metrics.NewUpdateCounterMex("fail", 13), + mock: func(m *storage.ServiceMock) { + m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{}, entities.ErrUnexpected) + }, + want: result{code: http.StatusInternalServerError}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert := assert.New(t) + require := require.New(t) + + m := storage.ServiceMock{} + + if tt.mock != nil { + tt.mock(&m) + } + + router := NewRouter(&m) + + payload, err := json.Marshal(tt.mex) + require.NoError(err) + + code, contentType, body := testRequest(t, router, http.MethodPost, "/update", payload) + + assert.Equal(tt.want.code, code) + + if tt.want.code == http.StatusOK { + assert.Equal("application/json", contentType) + + var resp metrics.MetricExchange + err = json.Unmarshal(body, &resp) + require.NoError(err) + + assert.Equal(tt.mex, resp) + } + }) + } +} + +func TestGetMetric(t *testing.T) { type result struct { code int body string } - //require := require.New(t) tests := []struct { name string path string + mock func(m *storage.ServiceMock) want result }{ { name: "get counter", path: "/value/counter/test", + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "counter").Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) + }, want: result{code: http.StatusOK, body: "42"}, }, { name: "get gauge", path: "/value/gauge/test", + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "gauge").Return(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}, nil) + }, want: result{code: http.StatusOK, body: "42.42"}, }, { @@ -222,27 +317,138 @@ func TestShowMetric(t *testing.T) { }, } - strg := storage.NewMemStorage() - router := server.NewRouter(strg) - testServer := httptest.NewServer(router) - defer testServer.Close() + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := storage.ServiceMock{} + router := NewRouter(&m) + + if tt.mock != nil { + tt.mock(&m) + } + + code, _, body := testRequest(t, router, http.MethodGet, tt.path, nil) + + assert.Equal(t, tt.want.code, code) + assert.Equal(t, tt.want.body, string(body)) + }) + } +} - err := strg.Push(storage.Record{Name: "test", Value: metrics.Counter(42)}) - if err != nil { - fmt.Println(err) +func TestGetMetricJSON(t *testing.T) { + type result struct { + code int + body metrics.MetricExchange } - err = strg.Push(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}) - if err != nil { - fmt.Println(err) + tests := []struct { + name string + mex metrics.MetricExchange + mock func(m *storage.ServiceMock) + expected result + }{ + { + name: "Should get counter", + mex: metrics.NewGetCounterMex("test"), + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "counter").Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) + }, + expected: result{ + code: http.StatusOK, + body: metrics.NewUpdateCounterMex("test", 42), + }, + }, + { + name: "Should get gauge", + mex: metrics.NewGetGaugeMex("test"), + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "gauge").Return(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}, nil) + }, + expected: result{ + code: http.StatusOK, + body: metrics.NewUpdateGaugeMex("test", 42.42), + }, + }, + { + name: "Should fail on unknown metric kind", + mex: metrics.MetricExchange{ID: "test", MType: "unknown"}, + expected: result{ + code: http.StatusBadRequest, + }, + }, + { + name: "Should fail on unknown counter", + mex: metrics.NewGetCounterMex("test"), + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "counter").Return(storage.Record{}, entities.ErrMetricNotFound) + }, + expected: result{ + code: http.StatusNotFound, + }, + }, + { + name: "Should fail on unknown gauge", + mex: metrics.NewGetGaugeMex("test"), + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "gauge").Return(storage.Record{}, entities.ErrMetricNotFound) + }, + expected: result{ + code: http.StatusNotFound, + }, + }, + { + name: "Should fail on counter with invalid name", + mex: metrics.NewGetCounterMex("X)"), + expected: result{ + code: http.StatusBadRequest, + }, + }, + { + name: "Should fail on gauge with invalid name", + mex: metrics.NewGetGaugeMex("X;"), + expected: result{ + code: http.StatusBadRequest, + }, + }, + { + name: "Should fail on broken service", + mex: metrics.NewGetGaugeMex("test"), + mock: func(m *storage.ServiceMock) { + m.On("Get", "test", "gauge").Return(storage.Record{}, entities.ErrUnexpected) + }, + expected: result{ + code: http.StatusInternalServerError, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - code, _, body := testRequest(t, testServer, http.MethodGet, tt.path, nil) + assert := assert.New(t) + require := require.New(t) - assert.Equal(t, tt.want.code, code) - assert.Equal(t, tt.want.body, string(body)) + m := storage.ServiceMock{} + + if tt.mock != nil { + tt.mock(&m) + } + + router := NewRouter(&m) + + payload, err := json.Marshal(tt.mex) + require.NoError(err) + + code, contentType, body := testRequest(t, router, http.MethodPost, "/value", payload) + assert.Equal(tt.expected.code, code) + + if tt.expected.code == http.StatusOK { + assert.Equal("application/json", contentType) + + var resp metrics.MetricExchange + err = json.Unmarshal(body, &resp) + require.NoError(err) + + assert.Equal(tt.expected.body, resp) + } }) } } diff --git a/internal/server/router.go b/internal/server/router.go index 8e125cf..5dd085d 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -10,9 +10,8 @@ import ( "github.com/ex0rcist/metflix/internal/storage" ) -func NewRouter(storage storage.Storage) http.Handler { +func NewRouter(storageService storage.StorageService) http.Handler { router := chi.NewRouter() - resource := Resource{storage: storage} router.Use(middleware.RealIP) router.Use(logging.RequestsLogger) @@ -21,9 +20,15 @@ func NewRouter(storage storage.Storage) http.Handler { w.WriteHeader(http.StatusNotFound) // no default body })) + resource := NewMetricResource(storageService) + router.Get("/", resource.Homepage) // TODO: resource? + router.Post("/update/{metricKind}/{metricName}/{metricValue}", resource.UpdateMetric) - router.Get("/value/{metricKind}/{metricName}", resource.ShowMetric) + router.Post("/update", resource.UpdateMetricJSON) + + router.Get("/value/{metricKind}/{metricName}", resource.GetMetric) + router.Post("/value", resource.GetMetricJSON) return router } diff --git a/internal/server/server.go b/internal/server/server.go index 79011db..ad6a949 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -13,7 +13,7 @@ import ( type Server struct { Config *Config - Storage storage.Storage + Storage storage.MetricsStorage Router http.Handler } @@ -26,12 +26,14 @@ func New() (*Server, error) { Address: "0.0.0.0:8080", } - storage := storage.NewMemStorage() - router := NewRouter(storage) + memStorage := storage.NewMemStorage() + storageService := storage.NewService(memStorage) + + router := NewRouter(storageService) return &Server{ Config: config, - Storage: storage, + Storage: memStorage, Router: router, }, nil } diff --git a/internal/storage/memory.go b/internal/storage/memory.go index c6c1b84..47d5fcf 100644 --- a/internal/storage/memory.go +++ b/internal/storage/memory.go @@ -1,30 +1,30 @@ package storage import ( - "sort" - "github.com/ex0rcist/metflix/internal/entities" ) +// check that MemStorage implements MetricsStorage +var _ MetricsStorage = (*MemStorage)(nil) + type MemStorage struct { - Data map[RecordID]Record + Data map[string]Record } func NewMemStorage() *MemStorage { return &MemStorage{ - Data: make(map[RecordID]Record), + Data: make(map[string]Record), } } -func (strg *MemStorage) Push(record Record) error { - recordID := CalculateRecordID(record.Name, record.Value.Kind()) - strg.Data[recordID] = record +func (s *MemStorage) Push(id string, record Record) error { + s.Data[id] = record return nil } -func (strg MemStorage) Get(recordID RecordID) (Record, error) { - record, ok := strg.Data[recordID] +func (s *MemStorage) Get(id string) (Record, error) { + record, ok := s.Data[id] if !ok { return Record{}, entities.ErrMetricNotFound } @@ -32,18 +32,13 @@ func (strg MemStorage) Get(recordID RecordID) (Record, error) { return record, nil } -func (strg MemStorage) GetAll() ([]Record, error) { - names := make([]string, 0, len(strg.Data)) - - for k := range strg.Data { - names = append(names, string(k)) - } - - arr := make([]Record, len(strg.Data)) - sort.Strings(names) +func (s *MemStorage) List() ([]Record, error) { + arr := make([]Record, len(s.Data)) - for i, v := range names { - arr[i] = strg.Data[RecordID(v)] + i := 0 + for _, record := range s.Data { + arr[i] = record + i++ } return arr, nil diff --git a/internal/storage/memory_test.go b/internal/storage/memory_test.go index 8f016f8..5abae1f 100644 --- a/internal/storage/memory_test.go +++ b/internal/storage/memory_test.go @@ -13,11 +13,13 @@ func TestPushCounter(t *testing.T) { require := require.New(t) strg := storage.NewMemStorage() + name := "test" + id := storage.CalculateRecordID(name, "counter") value := metrics.Counter(42) record := storage.Record{Name: name, Value: value} - err := strg.Push(record) + err := strg.Push(id, record) require.NoError(err) require.Equal(value, strg.Data[record.CalculateRecordID()].Value) @@ -27,11 +29,10 @@ func TestPushGauge(t *testing.T) { require := require.New(t) strg := storage.NewMemStorage() - name := "test" value := metrics.Gauge(42.42) - record := storage.Record{Name: name, Value: value} - err := strg.Push(record) + record := storage.Record{Name: "test", Value: value} + err := strg.Push(record.CalculateRecordID(), record) require.NoError(err) require.Equal(value, strg.Data[record.CalculateRecordID()].Value) @@ -46,11 +47,13 @@ func TestPushWithSameName(t *testing.T) { gaugeValue := metrics.Gauge(42.42) record1 := storage.Record{Name: "test", Value: counterValue} - err1 := strg.Push(record1) + id1 := record1.CalculateRecordID() + err1 := strg.Push(id1, record1) require.NoError(err1) record2 := storage.Record{Name: "test", Value: gaugeValue} - err2 := strg.Push(record2) + id2 := record2.CalculateRecordID() + err2 := strg.Push(id2, record2) require.NoError(err2) require.Equal(counterValue, strg.Data[record1.CalculateRecordID()].Value) @@ -64,7 +67,8 @@ func TestGet(t *testing.T) { value := metrics.Counter(6) record := storage.Record{Name: "test", Value: value} - err := strg.Push(record) + id := record.CalculateRecordID() + err := strg.Push(id, record) require.NoError(err) gotRecord, err := strg.Get(record.CalculateRecordID()) @@ -81,7 +85,7 @@ func TestGetNonExistantKey(t *testing.T) { require.Error(err) } -func TestGetAll(t *testing.T) { +func TestList(t *testing.T) { require := require.New(t) strg := storage.NewMemStorage() @@ -93,11 +97,12 @@ func TestGetAll(t *testing.T) { } for _, r := range records { - err := strg.Push(r) + err := strg.Push(r.CalculateRecordID(), r) require.NoError(err) } - allRecords, err := strg.GetAll() + allRecords, err := strg.List() + require.NoError(err) require.ElementsMatch(records, allRecords) } diff --git a/internal/storage/record.go b/internal/storage/record.go index 243137a..56c5378 100644 --- a/internal/storage/record.go +++ b/internal/storage/record.go @@ -2,20 +2,19 @@ package storage import "github.com/ex0rcist/metflix/internal/metrics" -type RecordID string type Record struct { Name string Value metrics.Metric } -func CalculateRecordID(name, kind string) RecordID { +func CalculateRecordID(name, kind string) string { if len(name) == 0 || len(kind) == 0 { - return RecordID("") + return "" } - return RecordID(name + "_" + kind) + return name + "_" + kind } -func (r Record) CalculateRecordID() RecordID { +func (r Record) CalculateRecordID() string { return CalculateRecordID(r.Name, r.Value.Kind()) } diff --git a/internal/storage/record_test.go b/internal/storage/record_test.go index 5177ad8..1a4e834 100644 --- a/internal/storage/record_test.go +++ b/internal/storage/record_test.go @@ -14,16 +14,16 @@ func TestRecordId(t *testing.T) { record1 := storage.Record{Name: "test", Value: metrics.Counter(42)} record2 := storage.Record{Name: "test", Value: metrics.Gauge(42.42)} - require.Equal(storage.RecordID("test_counter"), record1.CalculateRecordID()) - require.Equal(storage.RecordID("test_gauge"), record2.CalculateRecordID()) + require.Equal("test_counter", record1.CalculateRecordID()) + require.Equal("test_gauge", record2.CalculateRecordID()) } func TestRecordIdWithEmptyName(t *testing.T) { require := require.New(t) - require.Equal(storage.RecordID(""), storage.CalculateRecordID("", "counter")) + require.Equal("", storage.CalculateRecordID("", "counter")) } func TestRecordIdWithEmptyKind(t *testing.T) { require := require.New(t) - require.Equal(storage.RecordID(""), storage.CalculateRecordID("test", "")) + require.Equal("", storage.CalculateRecordID("test", "")) } diff --git a/internal/storage/service.go b/internal/storage/service.go new file mode 100644 index 0000000..0593fb8 --- /dev/null +++ b/internal/storage/service.go @@ -0,0 +1,85 @@ +package storage + +import ( + "errors" + "sort" + + "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/metrics" +) + +// Common interface for service layer +// Storage service prepares data before calling storage +type StorageService interface { + List() ([]Record, error) + Push(record Record) (Record, error) + Get(name, kind string) (Record, error) +} + +// ensure Service implements StorageService +var _ StorageService = Service{} + +type Service struct { + storage MetricsStorage +} + +func NewService(storage MetricsStorage) Service { + return Service{storage: storage} +} + +func (s Service) Get(name, kind string) (Record, error) { + id := CalculateRecordID(name, kind) + + record, err := s.storage.Get(id) + if err != nil { + return Record{}, err + } + + return record, nil +} + +func (s Service) Push(record Record) (Record, error) { + id := CalculateRecordID(record.Name, record.Value.Kind()) + + newValue, err := s.calculateNewValue(id, record) + if err != nil { + return Record{}, err + } + + record.Value = newValue + err = s.storage.Push(id, record) + + if err != nil { + return Record{}, err + } + + return record, nil +} + +func (s Service) List() ([]Record, error) { + records, err := s.storage.List() + if err != nil { + return nil, err + } + + sort.Slice(records, func(i, j int) bool { + return records[i].Name < records[j].Name + }) + + return records, nil +} + +func (s Service) calculateNewValue(id string, newRecord Record) (metrics.Metric, error) { + if newRecord.Value.Kind() != "counter" { + return newRecord.Value, nil + } + + storedRecord, err := s.storage.Get(id) + if errors.Is(err, entities.ErrMetricNotFound) { + return newRecord.Value, nil + } else if err != nil { + return nil, err + } + + return storedRecord.Value.(metrics.Counter) + newRecord.Value.(metrics.Counter), nil +} diff --git a/internal/storage/service_mock.go b/internal/storage/service_mock.go new file mode 100644 index 0000000..e693bcd --- /dev/null +++ b/internal/storage/service_mock.go @@ -0,0 +1,32 @@ +package storage + +import ( + "github.com/stretchr/testify/mock" +) + +// Ensure ServiceMock implements StorageService +var _ StorageService = (*ServiceMock)(nil) + +type ServiceMock struct { + mock.Mock +} + +func (m *ServiceMock) Get(name, kind string) (Record, error) { + args := m.Called(name, kind) + return args.Get(0).(Record), args.Error(1) +} + +func (m *ServiceMock) Push(record Record) (Record, error) { + args := m.Called(record) + return args.Get(0).(Record), args.Error(1) +} + +func (m *ServiceMock) List() ([]Record, error) { + args := m.Called() + + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).([]Record), args.Error(1) +} diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 6656e5c..65629f4 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -1,7 +1,8 @@ package storage -type Storage interface { - Push(record Record) error - Get(recordID RecordID) (Record, error) - GetAll() ([]Record, error) +// common interface for storages: mem, file, etc +type MetricsStorage interface { + Push(id string, record Record) error + Get(id string) (Record, error) + List() ([]Record, error) } diff --git a/internal/validators/validators.go b/internal/validators/validators.go index 2bdaa62..15da45d 100644 --- a/internal/validators/validators.go +++ b/internal/validators/validators.go @@ -8,15 +8,23 @@ import ( var nameRegexp = regexp.MustCompile(`^[A-Za-z\d]+$`) -func EnsureNamePresent(name string) error { - if len(name) == 0 { - return entities.ErrMetricMissingName +func ValidateMetric(name, kind string) error { + if err := validateMetricName(name); err != nil { + return err + } + + if err := validateMetricKind(kind); err != nil { + return err } return nil } -func ValidateName(name string) error { +func validateMetricName(name string) error { + if len(name) == 0 { + return entities.ErrMetricMissingName + } + if !nameRegexp.MatchString(name) { return entities.ErrMetricInvalidName } @@ -24,7 +32,7 @@ func ValidateName(name string) error { return nil } -func ValidateKind(kind string) error { +func validateMetricKind(kind string) error { switch kind { case "counter", "gauge": return nil diff --git a/internal/validators/validators_test.go b/internal/validators/validators_test.go index bffd861..207c2a6 100644 --- a/internal/validators/validators_test.go +++ b/internal/validators/validators_test.go @@ -6,99 +6,29 @@ import ( "github.com/ex0rcist/metflix/internal/validators" ) -func TestEnsureNamePresent(t *testing.T) { +func TestValidateMetric(t *testing.T) { type args struct { name string + kind string } tests := []struct { name string args args wantErr bool }{ - { - name: "no error when name present", - args: args{name: "testname"}, - wantErr: false, - }, - { - name: "has error when name is not present", - args: args{name: ""}, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := validators.EnsureNamePresent(tt.args.name); (err != nil) != tt.wantErr { - t.Errorf("EnsureNamePresent() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} + {name: "correct counter", args: args{name: "testname", kind: "counter"}, wantErr: false}, + {name: "correct gauge", args: args{name: "testname", kind: "gauge"}, wantErr: false}, -func TestValidateName(t *testing.T) { - type args struct { - name string - } - tests := []struct { - name string - args args - wantErr bool - }{ - { - name: "correct name", - args: args{name: "correctname1"}, - wantErr: false, - }, - { - name: "incorrect name", - args: args{name: "некорректноеимя"}, - wantErr: true, - }, - { - name: "incorrect name", - args: args{name: "incorrect name"}, - wantErr: true, - }, + {name: "name not present", args: args{name: "", kind: "counter"}, wantErr: true}, + {name: "incorrect name", args: args{name: "некорректноеимя", kind: "counter"}, wantErr: true}, + {name: "incorrect name", args: args{name: "incorrect name", kind: "gauge"}, wantErr: true}, + {name: "incorrect kind", args: args{kind: "gauger"}, wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := validators.ValidateName(tt.args.name); (err != nil) != tt.wantErr { + if err := validators.ValidateMetric(tt.args.name, tt.args.kind); (err != nil) != tt.wantErr { t.Errorf("ValidateName() error = %v, wantErr %v", err, tt.wantErr) } }) } } - -func TestValidateKind(t *testing.T) { - type args struct { - kind string - } - tests := []struct { - name string - args args - wantErr bool - }{ - { - name: "correct kind = counter", - args: args{kind: "counter"}, - wantErr: false, - }, - { - name: "correct kind = gauge", - args: args{kind: "gauge"}, - wantErr: false, - }, - { - name: "incorrect kind = gauger", - args: args{kind: "gauger"}, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if err := validators.ValidateKind(tt.args.kind); (err != nil) != tt.wantErr { - t.Errorf("ValidateKind() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -} From 1ba09a72bee5b1b83760e8b6010ff4cc9571df6a Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Fri, 19 Jul 2024 16:54:19 +0300 Subject: [PATCH 05/11] iter7 fixes --- go.mod | 3 +++ go.sum | 6 ++++++ internal/agent/api.go | 35 +++++++++++++++++++++++++-------- internal/agent/api_test.go | 14 +++++++++++-- internal/server/handlers.go | 39 +++++++++++++++++++++---------------- 5 files changed, 70 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index 5508c4c..189fe2c 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/satori/go.uuid v1.2.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 + github.com/tidwall/gjson v1.17.1 ) require ( @@ -19,6 +20,8 @@ require ( github.com/mattn/go-isatty v0.0.19 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.2 // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.0 // indirect golang.org/x/sys v0.21.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 2d08347..4ed1999 100644 --- a/go.sum +++ b/go.sum @@ -28,6 +28,12 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/tidwall/gjson v1.17.1 h1:wlYEnwqAHgzmhNUFfw7Xalt2JzQvsMx2Se4PcoFCT/U= +github.com/tidwall/gjson v1.17.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/internal/agent/api.go b/internal/agent/api.go index 173e920..0e86d0c 100644 --- a/internal/agent/api.go +++ b/internal/agent/api.go @@ -1,8 +1,9 @@ package agent import ( + "bytes" "context" - "fmt" + "encoding/json" "io" "net/http" "time" @@ -10,6 +11,7 @@ import ( "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/metrics" + "github.com/rs/zerolog/log" ) type API struct { @@ -37,21 +39,38 @@ func (c *API) Report(name string, metric metrics.Metric) *API { ctx := context.Background() // todo: another transport? - url := "http://" + c.address.String() + fmt.Sprintf("/update/%s/%s/%s", metric.Kind(), name, metric) + url := "http://" + c.address.String() + "/update" + + var mex metrics.MetricExchange + + // HELP: можно ли тут вместо приведения типов + // использовать рефлексию через metric.(type) ? + switch metric.Kind() { + case "counter": + mex = metrics.NewUpdateCounterMex(name, metric.(metrics.Counter)) + case "gauge": + mex = metrics.NewUpdateGaugeMex(name, metric.(metrics.Gauge)) + default: + log.Warn().Msg("unknown metric") // todo + } - req, err := http.NewRequest(http.MethodPost, url, http.NoBody) + body, err := json.Marshal(mex) if err != nil { - logging.LogError(ctx, err, "httpRequest error") + log.Warn().Msg("unknown metric") // todo } - req.Header.Set("Content-Type", "text/plain") + log.Info().Str("target", url).Str("payload", string(body)).Msg("sending report") - logging.LogInfo(ctx, fmt.Sprintf("sending POST to %v", url)) + req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body)) + if err != nil { + logging.LogError(ctx, err, "httpRequest error") + } - resp, err := c.httpClient.Do(req) + req.Header.Set("Content-Type", "application/json") + resp, err := c.httpClient.Do(req) if err != nil { - logging.LogError(ctx, err, "httpClient error") + log.Warn().Msg("httpClient error") // todo return c } diff --git a/internal/agent/api_test.go b/internal/agent/api_test.go index 45da750..b8656da 100644 --- a/internal/agent/api_test.go +++ b/internal/agent/api_test.go @@ -1,6 +1,8 @@ package agent import ( + "encoding/json" + "io" "net/http" "testing" @@ -29,9 +31,17 @@ func TestNewApi(t *testing.T) { func TestApiClientReport(t *testing.T) { rtf := func(req *http.Request) *http.Response { - assert.Equal(t, "http://localhost:8080/update/counter/Test/0", req.URL.String()) + assert.Equal(t, "http://localhost:8080/update", req.URL.String()) assert.Equal(t, http.MethodPost, req.Method) + expectedJSON, err := json.Marshal(metrics.NewUpdateCounterMex("test", 42)) + require.NoError(t, err) + + actualJSON, err := io.ReadAll(req.Body) + require.NoError(t, err) + + assert.Equal(t, expectedJSON, actualJSON) + return &http.Response{ StatusCode: 200, Body: http.NoBody, @@ -42,5 +52,5 @@ func TestApiClientReport(t *testing.T) { address := entities.Address("localhost:8080") api := NewAPI(&address, RoundTripFunc(rtf)) - api.Report("Test", metrics.Counter(0)) + api.Report("test", metrics.Counter(42)) } diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 77ecf66..cc1104b 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -111,6 +112,10 @@ func (r MetricResource) UpdateMetricJSON(rw http.ResponseWriter, req *http.Reque mex := new(metrics.MetricExchange) if err := json.NewDecoder(req.Body).Decode(mex); err != nil { + if err == io.EOF { + err = errors.New("no json provided") + } + writeErrorResponse(ctx, rw, http.StatusBadRequest, err) return } @@ -141,23 +146,6 @@ func (r MetricResource) UpdateMetricJSON(rw http.ResponseWriter, req *http.Reque } } -func errToStatus(err error) int { - switch err { - case entities.ErrMetricNotFound, entities.ErrMetricMissingName: - return http.StatusNotFound - case - entities.ErrMetricUnknown, entities.ErrMetricInvalidValue, - entities.ErrMetricInvalidName, entities.ErrMetricLongName, - entities.ErrMetricMissingValue: - - return http.StatusBadRequest - case entities.ErrUnexpected: - return http.StatusInternalServerError - default: - return http.StatusInternalServerError - } -} - func (r MetricResource) GetMetric(rw http.ResponseWriter, req *http.Request) { ctx := req.Context() @@ -220,3 +208,20 @@ func (r MetricResource) GetMetricJSON(rw http.ResponseWriter, req *http.Request) return } } + +func errToStatus(err error) int { + switch err { + case entities.ErrMetricNotFound, entities.ErrMetricMissingName: + return http.StatusNotFound + case + entities.ErrMetricUnknown, entities.ErrMetricInvalidValue, + entities.ErrMetricInvalidName, entities.ErrMetricLongName, + entities.ErrMetricMissingValue: + + return http.StatusBadRequest + case entities.ErrUnexpected: + return http.StatusInternalServerError + default: + return http.StatusInternalServerError + } +} From b66b6ad408963608ee812524e9622b59da6988f9 Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Fri, 19 Jul 2024 17:15:35 +0300 Subject: [PATCH 06/11] iter7 fixes --- internal/agent/api.go | 2 +- internal/server/router.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/agent/api.go b/internal/agent/api.go index 0e86d0c..8bf5b6b 100644 --- a/internal/agent/api.go +++ b/internal/agent/api.go @@ -39,7 +39,7 @@ func (c *API) Report(name string, metric metrics.Metric) *API { ctx := context.Background() // todo: another transport? - url := "http://" + c.address.String() + "/update" + url := "http://" + c.address.String() + "/update/" var mex metrics.MetricExchange diff --git a/internal/server/router.go b/internal/server/router.go index 5dd085d..77f6b30 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -15,6 +15,7 @@ func NewRouter(storageService storage.StorageService) http.Handler { router.Use(middleware.RealIP) router.Use(logging.RequestsLogger) + router.Use(middleware.StripSlashes) router.NotFound(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) // no default body From e91455a48ea238416c7691d1ce777ecf7c4fb2dc Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Fri, 19 Jul 2024 17:25:51 +0300 Subject: [PATCH 07/11] iter7 fixes --- internal/agent/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/agent/api.go b/internal/agent/api.go index 8bf5b6b..0e86d0c 100644 --- a/internal/agent/api.go +++ b/internal/agent/api.go @@ -39,7 +39,7 @@ func (c *API) Report(name string, metric metrics.Metric) *API { ctx := context.Background() // todo: another transport? - url := "http://" + c.address.String() + "/update/" + url := "http://" + c.address.String() + "/update" var mex metrics.MetricExchange From 5d41387ccd5e11e04e50b9ea6c343c86dc099434 Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Tue, 23 Jul 2024 19:47:19 +0300 Subject: [PATCH 08/11] added basic iter8 features --- cmd/agent/main.go | 14 +--- cmd/server/main.go | 15 ++-- go.mod | 3 - go.sum | 6 -- internal/agent/agent.go | 24 +++--- internal/agent/api.go | 61 +++++++++++---- internal/agent/api_test.go | 10 ++- internal/agent/stats.go | 5 +- internal/compression/compression.go | 26 +++++++ internal/compression/compressor.go | 61 +++++++++++++++ internal/compression/decompressor.go | 73 ++++++++++++++++++ internal/entities/errors.go | 7 ++ internal/logging/debug.go | 22 ++++++ internal/logging/error.go | 27 +++++++ internal/logging/fatal.go | 27 +++++++ internal/logging/info.go | 22 ++++++ internal/logging/logging.go | 79 ++++++++++--------- internal/logging/logging_test.go | 4 +- internal/logging/middleware.go | 51 ------------- internal/middleware/middleware.go | 110 +++++++++++++++++++++++++++ internal/server/handlers.go | 4 +- internal/server/router.go | 13 ++-- internal/server/server.go | 26 +++---- internal/utils/http.go | 19 +++++ internal/utils/request_id.go | 9 +++ 25 files changed, 546 insertions(+), 172 deletions(-) create mode 100644 internal/compression/compression.go create mode 100644 internal/compression/compressor.go create mode 100644 internal/compression/decompressor.go create mode 100644 internal/logging/debug.go create mode 100644 internal/logging/error.go create mode 100644 internal/logging/fatal.go create mode 100644 internal/logging/info.go delete mode 100644 internal/logging/middleware.go create mode 100644 internal/middleware/middleware.go create mode 100644 internal/utils/http.go create mode 100644 internal/utils/request_id.go diff --git a/cmd/agent/main.go b/cmd/agent/main.go index d1b5d8c..87e3c3c 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -1,30 +1,24 @@ package main import ( - "context" - "github.com/ex0rcist/metflix/internal/agent" "github.com/ex0rcist/metflix/internal/logging" ) func main() { - ctx := logging.Setup(context.Background()) + logging.Setup() - logging.LogInfo(ctx, "starting agent...") + logging.LogInfo("starting agent...") agnt, err := agent.New() if err != nil { - logging.LogFatal(ctx, err) + logging.LogFatal(err) } err = agnt.ParseFlags() if err != nil { - logging.LogFatal(ctx, err) + logging.LogFatal(err) } - logging.LogInfo(ctx, agnt.Config.String()) - agnt.Run() - - logging.LogInfo(ctx, "agent ready") } diff --git a/cmd/server/main.go b/cmd/server/main.go index aa8bd71..ba8c1e2 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -1,32 +1,27 @@ package main import ( - "context" - "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/server" ) func main() { - ctx := logging.Setup(context.Background()) + logging.Setup() - logging.LogInfo(ctx, "starting server...") + logging.LogInfo("starting server...") srv, err := server.New() if err != nil { - logging.LogFatal(ctx, err) + logging.LogFatal(err) } err = srv.ParseFlags() if err != nil { - logging.LogFatal(ctx, err) + logging.LogFatal(err) } - logging.LogInfo(ctx, srv.Config.String()) - logging.LogInfo(ctx, "server ready") // TODO: must be after run? - err = srv.Run() if err != nil { - logging.LogFatal(ctx, err) + logging.LogFatal(err) } } diff --git a/go.mod b/go.mod index 189fe2c..5508c4c 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,6 @@ require ( github.com/satori/go.uuid v1.2.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.9.0 - github.com/tidwall/gjson v1.17.1 ) require ( @@ -20,8 +19,6 @@ require ( github.com/mattn/go-isatty v0.0.19 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.2 // indirect - github.com/tidwall/match v1.1.1 // indirect - github.com/tidwall/pretty v1.2.0 // indirect golang.org/x/sys v0.21.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 4ed1999..2d08347 100644 --- a/go.sum +++ b/go.sum @@ -28,12 +28,6 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -github.com/tidwall/gjson v1.17.1 h1:wlYEnwqAHgzmhNUFfw7Xalt2JzQvsMx2Se4PcoFCT/U= -github.com/tidwall/gjson v1.17.1/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= -github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= -github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= -github.com/tidwall/pretty v1.2.0 h1:RWIZEg2iJ8/g6fDDYzMpobmaoGh5OLl4AXtGUGPcqCs= -github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= diff --git a/internal/agent/agent.go b/internal/agent/agent.go index c0a93e0..62bb1cf 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1,7 +1,6 @@ package agent import ( - "context" "fmt" "sync" "time" @@ -46,7 +45,7 @@ func New() (*Agent, error) { func (a *Agent) ParseFlags() error { address := a.Config.Address - pflag.VarP(&address, "address", "a", "address:port for HTTP API requests") // HELP: "&"" because Set() has pointer receiver? + pflag.VarP(&address, "address", "a", "address:port for HTTP API requests") pflag.IntVarP(&a.Config.PollInterval, "poll-interval", "p", a.Config.PollInterval, "interval (s) for polling stats") pflag.IntVarP(&a.Config.ReportInterval, "report-interval", "r", a.Config.ReportInterval, "interval (s) for polling stats") @@ -69,6 +68,9 @@ func (a *Agent) ParseFlags() error { } func (a *Agent) Run() { + logging.LogInfo(a.Config.String()) + logging.LogInfo("agent ready") + a.wg.Add(2) go a.startPolling() @@ -80,12 +82,10 @@ func (a *Agent) Run() { func (a *Agent) startPolling() { defer a.wg.Done() - ctx := context.Background() - for { err := a.Stats.Poll() if err != nil { - logging.LogError(ctx, err) + logging.LogError(err) } time.Sleep(intToDuration(a.Config.PollInterval)) @@ -103,9 +103,7 @@ func (a *Agent) startReporting() { } func (a *Agent) reportStats() { - ctx := context.Background() - - logging.LogInfo(ctx, "reporting stats ... ") + logging.LogInfo("reporting stats ... ") // agent continues polling while report is in progress, take snapshot? snapshot := *a.Stats @@ -154,10 +152,8 @@ func intToDuration(s int) time.Duration { } func (c Config) String() string { - out := "agent config: " - - out += fmt.Sprintf("address=%v \t", c.Address) - out += fmt.Sprintf("poll-interval=%v \t", c.PollInterval) - out += fmt.Sprintf("report-interval=%v \t", c.ReportInterval) - return out + return fmt.Sprintf( + "agent config: address=%v; poll-interval=%v; report-interval=%v", + c.Address, c.PollInterval, c.ReportInterval, + ) } diff --git a/internal/agent/api.go b/internal/agent/api.go index 0e86d0c..e3d3bdd 100644 --- a/internal/agent/api.go +++ b/internal/agent/api.go @@ -1,16 +1,18 @@ package agent import ( - "bytes" "context" "encoding/json" + "fmt" "io" "net/http" "time" + "github.com/ex0rcist/metflix/internal/compression" "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/metrics" + "github.com/ex0rcist/metflix/internal/utils" "github.com/rs/zerolog/log" ) @@ -36,11 +38,10 @@ func NewAPI(address *entities.Address, httpTransport http.RoundTripper) *API { } func (c *API) Report(name string, metric metrics.Metric) *API { - ctx := context.Background() - - // todo: another transport? url := "http://" + c.address.String() + "/update" + var requestID = utils.GenerateRequestID() + var ctx = setupLoggerCtx(requestID) var mex metrics.MetricExchange // HELP: можно ли тут вместо приведения типов @@ -51,38 +52,72 @@ func (c *API) Report(name string, metric metrics.Metric) *API { case "gauge": mex = metrics.NewUpdateGaugeMex(name, metric.(metrics.Gauge)) default: - log.Warn().Msg("unknown metric") // todo + logging.LogError(entities.ErrMetricReport, "unknown metric") + return c } body, err := json.Marshal(mex) if err != nil { - log.Warn().Msg("unknown metric") // todo + logging.LogErrorCtx(ctx, entities.ErrMetricReport, "error during marshaling", err.Error()) + return c } - log.Info().Str("target", url).Str("payload", string(body)).Msg("sending report") + payload, err := compression.Pack(body) + if err != nil { + logging.LogErrorCtx(ctx, entities.ErrMetricReport, "error during compression", err.Error()) + return c + } - req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(body)) + req, err := http.NewRequest(http.MethodPost, url, payload) if err != nil { - logging.LogError(ctx, err, "httpRequest error") + logging.LogErrorCtx(ctx, entities.ErrMetricReport, "httpRequest error", err.Error()) + return c } req.Header.Set("Content-Type", "application/json") + req.Header.Set("Content-Encoding", "gzip") + req.Header.Set("X-Request-Id", requestID) + + logRequest(ctx, url, req.Header, body) resp, err := c.httpClient.Do(req) if err != nil { - log.Warn().Msg("httpClient error") // todo + logging.LogErrorCtx(ctx, entities.ErrMetricReport, "error making http request", err.Error()) return c } defer resp.Body.Close() - respBody, err := io.ReadAll(resp.Body) // нужно прочитать ответ для keepalive? + respBody, err := io.ReadAll(resp.Body) if err != nil { - logging.LogError(ctx, entities.ErrMetricReport, "error reading response body") + logging.LogErrorCtx(ctx, entities.ErrMetricReport, "error reading response body", err.Error()) + return c } + logResponse(ctx, resp, respBody) + if resp.StatusCode != http.StatusOK { - logging.LogError(ctx, entities.ErrMetricReport, string(respBody)) + logging.LogErrorCtx(ctx, entities.ErrMetricReport, "error reporting stat", resp.Status, string(respBody)) } return c } + +func setupLoggerCtx(requestID string) context.Context { + // empty context for now + ctx := context.Background() + + // setup logger with rid attached + logger := log.Logger.With().Ctx(ctx).Str("rid", requestID).Logger() + + // return context for logging + return logger.WithContext(ctx) +} + +func logRequest(ctx context.Context, url string, headers http.Header, body []byte) { + logging.LogInfoCtx(ctx, "sending request to: "+url) + logging.LogDebugCtx(ctx, fmt.Sprintf("request: headers=%s; body=%s", utils.HeadersToStr(headers), string(body))) +} + +func logResponse(ctx context.Context, resp *http.Response, respBody []byte) { + logging.LogDebugCtx(ctx, fmt.Sprintf("response: %v; headers=%s; body=%s", resp.Status, utils.HeadersToStr(resp.Header), respBody)) +} diff --git a/internal/agent/api_test.go b/internal/agent/api_test.go index b8656da..def3eff 100644 --- a/internal/agent/api_test.go +++ b/internal/agent/api_test.go @@ -6,6 +6,7 @@ import ( "net/http" "testing" + "github.com/ex0rcist/metflix/internal/compression" "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/metrics" "github.com/stretchr/testify/assert" @@ -34,13 +35,16 @@ func TestApiClientReport(t *testing.T) { assert.Equal(t, "http://localhost:8080/update", req.URL.String()) assert.Equal(t, http.MethodPost, req.Method) - expectedJSON, err := json.Marshal(metrics.NewUpdateCounterMex("test", 42)) + payload, err := json.Marshal(metrics.NewUpdateCounterMex("test", 42)) require.NoError(t, err) - actualJSON, err := io.ReadAll(req.Body) + expectedPayload, err := compression.Pack(payload) require.NoError(t, err) - assert.Equal(t, expectedJSON, actualJSON) + actualPayload, err := io.ReadAll(req.Body) + require.NoError(t, err) + + assert.Equal(t, expectedPayload.Bytes(), actualPayload) return &http.Response{ StatusCode: 200, diff --git a/internal/agent/stats.go b/internal/agent/stats.go index 57154d3..06cb271 100644 --- a/internal/agent/stats.go +++ b/internal/agent/stats.go @@ -1,7 +1,6 @@ package agent import ( - "context" "math/rand" "time" @@ -23,9 +22,7 @@ func NewStats() *Stats { } func (m *Stats) Poll() error { - ctx := context.Background() - - logging.LogInfo(ctx, "polling stats ... ") + logging.LogDebug("polling stats ... ") m.PollCount++ m.RandomValue = metrics.Gauge(m.generator.Float64()) diff --git a/internal/compression/compression.go b/internal/compression/compression.go new file mode 100644 index 0000000..84ac1ac --- /dev/null +++ b/internal/compression/compression.go @@ -0,0 +1,26 @@ +package compression + +import ( + "bytes" + "compress/gzip" + "fmt" +) + +func Pack(data []byte) (*bytes.Buffer, error) { + bb := new(bytes.Buffer) + + encoder, err := gzip.NewWriterLevel(bb, gzip.BestSpeed) + if err != nil { + return nil, fmt.Errorf("failed init compress writer: %v", err) + } + + if _, err = encoder.Write(data); err != nil { + return nil, fmt.Errorf("failed write data to compress temporary buffer: %v", err) + } + + if err = encoder.Close(); err != nil { + return nil, fmt.Errorf("failed compress data: %v", err) + } + + return bb, nil +} diff --git a/internal/compression/compressor.go b/internal/compression/compressor.go new file mode 100644 index 0000000..46ff7a2 --- /dev/null +++ b/internal/compression/compressor.go @@ -0,0 +1,61 @@ +package compression + +import ( + "compress/gzip" + "context" + "net/http" + + "github.com/ex0rcist/metflix/internal/logging" +) + +type Compressor struct { + http.ResponseWriter + + context context.Context + encoder *gzip.Writer + supportedContent map[string]struct{} +} + +func NewCompressor(w http.ResponseWriter, ctx context.Context) *Compressor { + supportedContent := map[string]struct{}{ + "application/json": {}, // {} uses no memory + "text/html": {}, + } + + return &Compressor{ + ResponseWriter: w, + context: ctx, + supportedContent: supportedContent, + } +} + +func (c *Compressor) Write(resp []byte) (int, error) { + contentType := c.Header().Get("Content-Type") + if _, ok := c.supportedContent[contentType]; !ok { + logging.LogDebugCtx(c.context, "compression not supported for "+contentType) + return c.ResponseWriter.Write(resp) + } + + if c.encoder == nil { + encoder, err := gzip.NewWriterLevel(c.ResponseWriter, gzip.BestSpeed) + if err != nil { + logging.LogErrorCtx(c.context, err) + return c.ResponseWriter.Write(resp) + } + c.encoder = encoder + } + + c.Header().Set("Content-Encoding", "gzip") + + return c.encoder.Write(resp) +} + +func (c *Compressor) Close() { + if c.encoder == nil { + return + } + + if err := c.encoder.Close(); err != nil { + logging.LogErrorCtx(c.context, err, "error closing compressor encoder", err.Error()) + } +} diff --git a/internal/compression/decompressor.go b/internal/compression/decompressor.go new file mode 100644 index 0000000..4b86c10 --- /dev/null +++ b/internal/compression/decompressor.go @@ -0,0 +1,73 @@ +package compression + +import ( + "compress/gzip" + "context" + "net/http" + + "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/logging" +) + +type Decompressor struct { + request *http.Request + reader *gzip.Reader + context context.Context + supportedEncodings map[string]struct{} +} + +func NewDecompressor(req *http.Request, ctx context.Context) *Decompressor { + supportedEncodings := map[string]struct{}{ + "gzip": {}, // {} uses no memory + } + + return &Decompressor{ + request: req, + context: ctx, + supportedEncodings: supportedEncodings, + } +} + +func (d *Decompressor) Decompress() error { + encoding := d.request.Header.Get("Content-Encoding") + + if len(encoding) == 0 { + logging.LogDebugCtx(d.context, "no encoding provided") + return nil + } + + logging.LogDebugCtx(d.context, "got request compressed with "+encoding) + + if _, ok := d.supportedEncodings[encoding]; !ok { + err := entities.ErrEncodingUnsupported + logging.LogErrorCtx(d.context, err, "decoding not supported for "+encoding) + + return err + } + + if d.reader == nil { + reader, err := gzip.NewReader(d.request.Body) + if err != nil { + err := entities.ErrEncodingInternal + logging.LogErrorCtx(d.context, err, "failed to create gzip reader: "+err.Error()) + + return err + } + + d.reader = reader + } + + d.request.Body = d.reader + + return nil +} + +func (d *Decompressor) Close() { + if d.reader == nil { + return + } + + if err := d.reader.Close(); err != nil { + logging.LogErrorCtx(d.context, err, "error closing decompressor reader", err.Error()) + } +} diff --git a/internal/entities/errors.go b/internal/entities/errors.go index ee6cdfc..3c5c7fd 100644 --- a/internal/entities/errors.go +++ b/internal/entities/errors.go @@ -17,5 +17,12 @@ var ( ErrStoragePush = errors.New("failed to push record") ErrStorageFetch = errors.New("failed to get record") + ErrEncodingInternal = errors.New("internal encoding error") + ErrEncodingUnsupported = errors.New("requsted encoding is not supported") + ErrUnexpected = errors.New("unexpected error") ) + +func NewStackError(err error) error { + return errors.New(err.Error()) // TODO: can we remove this func from stack? +} diff --git a/internal/logging/debug.go b/internal/logging/debug.go new file mode 100644 index 0000000..0733c34 --- /dev/null +++ b/internal/logging/debug.go @@ -0,0 +1,22 @@ +package logging + +import ( + "context" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +func LogDebug(messages ...string) { + logDebug(&log.Logger, messages...) +} + +func LogDebugCtx(ctx context.Context, messages ...string) { + logger := loggerFromContext(ctx) + logDebug(logger, messages...) +} + +func logDebug(logger *zerolog.Logger, messages ...string) { + msg := optMessagesToString(messages) + logger.Debug().Msg(msg) +} diff --git a/internal/logging/error.go b/internal/logging/error.go new file mode 100644 index 0000000..2e38a25 --- /dev/null +++ b/internal/logging/error.go @@ -0,0 +1,27 @@ +package logging + +import ( + "context" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +func LogError(err error, messages ...string) { + logError(&log.Logger, err, messages...) +} + +func LogErrorCtx(ctx context.Context, err error, messages ...string) { + logger := loggerFromContext(ctx) + logError(logger, err, messages...) +} + +func logError(logger *zerolog.Logger, err error, messages ...string) { + msg := optMessagesToString(messages) + + if isDebugLevel() { + logger.Error().Stack().Err(err).Msg(msg) // Stack() must be called before Err() + } else { + logger.Error().Err(err).Msg(msg) + } +} diff --git a/internal/logging/fatal.go b/internal/logging/fatal.go new file mode 100644 index 0000000..626cd14 --- /dev/null +++ b/internal/logging/fatal.go @@ -0,0 +1,27 @@ +package logging + +import ( + "context" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +func LogFatal(err error, messages ...string) { + logFatal(&log.Logger, err, messages...) +} + +func LogFatalCtx(ctx context.Context, err error, messages ...string) { + logger := loggerFromContext(ctx) + logFatal(logger, err, messages...) +} + +func logFatal(logger *zerolog.Logger, err error, messages ...string) { + msg := optMessagesToString(messages) + + if isDebugLevel() { + logger.Fatal().Stack().Err(err).Msg(msg) // Stack() must be called before Err() + } else { + logger.Fatal().Err(err).Msg(msg) + } +} diff --git a/internal/logging/info.go b/internal/logging/info.go new file mode 100644 index 0000000..e42c1a4 --- /dev/null +++ b/internal/logging/info.go @@ -0,0 +1,22 @@ +package logging + +import ( + "context" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +func LogInfo(messages ...string) { + logInfo(&log.Logger, messages...) +} + +func LogInfoCtx(ctx context.Context, messages ...string) { + logger := loggerFromContext(ctx) + logInfo(logger, messages...) +} + +func logInfo(logger *zerolog.Logger, messages ...string) { + msg := optMessagesToString(messages) + logger.Info().Msg(msg) +} diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 70ddb8b..a2a612b 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -9,56 +9,56 @@ import ( "time" "github.com/caarlos0/env/v11" - "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/rs/zerolog/log" "github.com/rs/zerolog/pkgerrors" ) type config struct { - Env string `env:"APP_ENV" envDefault:"development"` + ENV string `env:"APP_ENV" envDefault:"development"` } -func Setup(ctx context.Context) context.Context { +func Setup() { cfg := parseConfig() var output io.Writer - switch cfg.Env { + var logger zerolog.Logger + + switch cfg.ENV { + case "tracing": + zerolog.SetGlobalLevel(zerolog.TraceLevel) + zerolog.ErrorStackMarshaler = pkgerrors.MarshalStack + + output = zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339Nano} case "development": + zerolog.SetGlobalLevel(zerolog.DebugLevel) + output = zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339Nano} case "production": + zerolog.SetGlobalLevel(zerolog.InfoLevel) + output = os.Stdout } - logger := zerolog.New(output).With().Timestamp().Logger() + loggerCtx := zerolog.New(output).With().Timestamp() + switch { + case isTraceLevel(): + logger = loggerCtx.Caller().Logger() + default: + logger = loggerCtx.Logger() + } - // set global logger log.Logger = logger - - zerolog.ErrorStackMarshaler = pkgerrors.MarshalStack zerolog.DefaultContextLogger = &logger - - // added logger to ctx - return logger.WithContext(ctx) } -func NewError(err error) error { // wrap err to pkg/errors - return errors.New(err.Error()) // TODO: can we remove this func from stack? -} - -func LogError(ctx context.Context, err error, messages ...string) { - msg := optMessagesToString(messages) - zerolog.Ctx(ctx).Error().Stack().Err(err).Msg(msg) -} - -func LogFatal(ctx context.Context, err error, messages ...string) { - msg := optMessagesToString(messages) - zerolog.Ctx(ctx).Fatal().Stack().Err(err).Msg(msg) -} +func parseConfig() config { + cfg := config{} + if err := env.Parse(&cfg); err != nil { + fmt.Printf("%+v\n", err) + } -func LogInfo(ctx context.Context, messages ...string) { - msg := optMessagesToString(messages) - zerolog.Ctx(ctx).Info().Msg(msg) + return cfg } func optMessagesToString(messages []string) string { @@ -66,14 +66,25 @@ func optMessagesToString(messages []string) string { return "" } - return strings.Join(messages, "; ") + // remove empty + var result []string + for _, str := range messages { + if str != "" { + result = append(result, str) + } + } + + return strings.Join(result, "; ") } -func parseConfig() config { - cfg := config{} - if err := env.Parse(&cfg); err != nil { - fmt.Printf("%+v\n", err) - } +func loggerFromContext(ctx context.Context) *zerolog.Logger { + return zerolog.Ctx(ctx) +} - return cfg +func isDebugLevel() bool { + return zerolog.GlobalLevel() == zerolog.DebugLevel +} + +func isTraceLevel() bool { + return zerolog.GlobalLevel() == zerolog.TraceLevel } diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index 4cd9634..2678c65 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -1,7 +1,6 @@ package logging_test import ( - "context" "testing" "github.com/ex0rcist/metflix/internal/logging" @@ -10,7 +9,6 @@ import ( func TestSetup(t *testing.T) { require := require.New(t) - ctx := context.Background() - require.NotPanics(func() { logging.Setup(ctx) }) + require.NotPanics(func() { logging.Setup() }) } diff --git a/internal/logging/middleware.go b/internal/logging/middleware.go deleted file mode 100644 index fd1e332..0000000 --- a/internal/logging/middleware.go +++ /dev/null @@ -1,51 +0,0 @@ -package logging - -import ( - "net/http" - "time" - - "github.com/go-chi/chi/middleware" - "github.com/rs/zerolog/log" - uuid "github.com/satori/go.uuid" -) - -func RequestsLogger(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - requestID := findOrCreateRequestID(r) - start := time.Now() - - // setup child logger for middleware - logger := log.Logger.With(). - Str("rid", requestID). - Logger() - - // log started - logger.Info(). - Str("method", r.Method). - Str("url", r.URL.String()). - Str("remote-addr", r.RemoteAddr). // middleware.RealIP - Msg("Started") - - // execute - ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) - ctx := logger.WithContext(r.Context()) - next.ServeHTTP(ww, r.WithContext(ctx)) - - // log completed - logger.Info(). - Float64("elapsed", time.Since(start).Seconds()). - Int("status", ww.Status()). - Int("size", ww.BytesWritten()). - Msg("Completed") - }) -} - -func findOrCreateRequestID(r *http.Request) string { - requestID := r.Header.Get("X-Request-Id") - - if requestID == "" { - requestID = uuid.NewV4().String() - } - - return requestID -} diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go new file mode 100644 index 0000000..634bec3 --- /dev/null +++ b/internal/middleware/middleware.go @@ -0,0 +1,110 @@ +package middleware + +import ( + "errors" + "net/http" + "time" + + "github.com/ex0rcist/metflix/internal/compression" + "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/logging" + "github.com/ex0rcist/metflix/internal/utils" + "github.com/go-chi/chi/middleware" + "github.com/rs/zerolog/log" +) + +func RequestsLogger(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + start := time.Now() + requestID := findOrCreateRequestID(r) + + // setup child logger for middleware + logger := log.Logger.With(). + Str("rid", requestID). + Logger() + + // log started + logger.Info(). + Str("method", r.Method). + Str("url", r.URL.String()). + Str("remote-addr", r.RemoteAddr). // middleware.RealIP + Msg("Started") + + logger.Debug(). + Msgf("request: %s", utils.HeadersToStr(r.Header)) + + // TODO: context? + + // execute + ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) + ctx := logger.WithContext(r.Context()) + next.ServeHTTP(ww, r.WithContext(ctx)) + + logger.Debug(). + Msgf("response: %s", utils.HeadersToStr(ww.Header())) + + // log completed + logger.Info(). + Float64("elapsed", time.Since(start).Seconds()). + Int("status", ww.Status()). + Int("size", ww.BytesWritten()). + Msg("Completed") + }) +} + +func DecompressRequest(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + encoding := r.Header.Get("Content-Encoding") + if len(encoding) == 0 { + next.ServeHTTP(w, r) + return + } + + decompressor := compression.NewDecompressor(r, ctx) + defer decompressor.Close() + + err := decompressor.Decompress() + if err != nil { + switch { + case errors.Is(err, entities.ErrEncodingUnsupported): + http.Error(w, err.Error(), http.StatusBadRequest) + return + case errors.Is(err, entities.ErrEncodingInternal): + http.Error(w, "", http.StatusInternalServerError) + return + } + } + + next.ServeHTTP(w, r) + }) +} + +func CompressResponse(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + if len(r.Header.Get("Accept-Encoding")) == 0 { + logging.LogDebugCtx(ctx, "compression not requested by client") + + next.ServeHTTP(w, r) + return + } + + compressor := compression.NewCompressor(w, ctx) + defer compressor.Close() + + next.ServeHTTP(compressor, r) + }) +} + +func findOrCreateRequestID(r *http.Request) string { + requestID := r.Header.Get("X-Request-Id") + + if requestID == "" { + requestID = utils.GenerateRequestID() + } + + return requestID +} diff --git a/internal/server/handlers.go b/internal/server/handlers.go index cc1104b..3374018 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -26,7 +26,7 @@ func NewMetricResource(storageService storage.StorageService) MetricResource { } func writeErrorResponse(ctx context.Context, w http.ResponseWriter, code int, err error) { - logging.LogError(ctx, err) + logging.LogErrorCtx(ctx, err) w.WriteHeader(code) // only header for now @@ -54,7 +54,7 @@ func (r MetricResource) Homepage(rw http.ResponseWriter, req *http.Request) { _, err = rw.Write([]byte(body)) if err != nil { - logging.LogError(ctx, err) + logging.LogErrorCtx(ctx, err) } } diff --git a/internal/server/router.go b/internal/server/router.go index 77f6b30..c300b87 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -3,19 +3,22 @@ package server import ( "net/http" - "github.com/go-chi/chi/middleware" + chimdlw "github.com/go-chi/chi/middleware" "github.com/go-chi/chi/v5" - "github.com/ex0rcist/metflix/internal/logging" + "github.com/ex0rcist/metflix/internal/middleware" "github.com/ex0rcist/metflix/internal/storage" ) func NewRouter(storageService storage.StorageService) http.Handler { router := chi.NewRouter() - router.Use(middleware.RealIP) - router.Use(logging.RequestsLogger) - router.Use(middleware.StripSlashes) + router.Use(chimdlw.RealIP) + router.Use(chimdlw.StripSlashes) + + router.Use(middleware.RequestsLogger) + router.Use(middleware.DecompressRequest) + router.Use(middleware.CompressResponse) router.NotFound(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusNotFound) // no default body diff --git a/internal/server/server.go b/internal/server/server.go index ad6a949..3514f1d 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -12,7 +12,7 @@ import ( ) type Server struct { - Config *Config + config *Config Storage storage.MetricsStorage Router http.Handler } @@ -32,42 +32,40 @@ func New() (*Server, error) { router := NewRouter(storageService) return &Server{ - Config: config, + config: config, Storage: memStorage, Router: router, }, nil } func (s *Server) ParseFlags() error { - address := s.Config.Address + address := s.config.Address - pflag.VarP(&address, "address", "a", "address:port for HTTP API requests") // HELP: "&"" because Set() has pointer receiver? + pflag.VarP(&address, "address", "a", "address:port for HTTP API requests") pflag.Parse() // because VarP gets non-pointer value, set it manually pflag.Visit(func(f *pflag.Flag) { switch f.Name { case "address": - s.Config.Address = address + s.config.Address = address } }) - if err := env.Parse(s.Config); err != nil { - return logging.NewError(err) + if err := env.Parse(s.config); err != nil { + return entities.NewStackError(err) } return nil } func (s *Server) Run() error { - // HELP: почему тип, реализующий String() не приводится к строке автоматически? - err := http.ListenAndServe(s.Config.Address.String(), s.Router) - return err + logging.LogInfo(s.config.String()) + logging.LogInfo("server ready") + + return http.ListenAndServe(s.config.Address.String(), s.Router) } func (c Config) String() string { - out := "server config: " - - out += fmt.Sprintf("address=%s\t", c.Address) - return out + return "server config: " + fmt.Sprintf("address=%s\t", c.Address) } diff --git a/internal/utils/http.go b/internal/utils/http.go new file mode 100644 index 0000000..c2003cd --- /dev/null +++ b/internal/utils/http.go @@ -0,0 +1,19 @@ +package utils + +import ( + "fmt" + "net/http" + "strings" +) + +func HeadersToStr(headers http.Header) string { + stringsSlice := []string{} + + for name, values := range headers { + for _, value := range values { + stringsSlice = append(stringsSlice, fmt.Sprintf("%s:%s", name, value)) + } + } + + return strings.Join(stringsSlice, ", ") +} diff --git a/internal/utils/request_id.go b/internal/utils/request_id.go new file mode 100644 index 0000000..60f1eb5 --- /dev/null +++ b/internal/utils/request_id.go @@ -0,0 +1,9 @@ +package utils + +import ( + uuid "github.com/satori/go.uuid" +) + +func GenerateRequestID() string { + return uuid.NewV4().String() +} From 1d1d81354296f12217d45fe0aab2b7d0aefed531 Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Tue, 23 Jul 2024 20:28:31 +0300 Subject: [PATCH 09/11] iter8 fixes --- internal/middleware/middleware.go | 18 ++++++++++++++++-- internal/server/handlers.go | 2 ++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 634bec3..962a091 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -85,8 +85,8 @@ func CompressResponse(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - if len(r.Header.Get("Accept-Encoding")) == 0 { - logging.LogDebugCtx(ctx, "compression not requested by client") + if !needGzipEncoding(r) { + logging.LogDebugCtx(ctx, "compression not requested or not supported by client") next.ServeHTTP(w, r) return @@ -108,3 +108,17 @@ func findOrCreateRequestID(r *http.Request) string { return requestID } + +func needGzipEncoding(r *http.Request) bool { + if len(r.Header.Get("Accept-Encoding")) == 0 { + return false + } + + for _, encoding := range r.Header.Values("Accept-Encoding") { + if encoding == "gzip" { + return true + } + } + + return false +} diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 3374018..5ea6a39 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -52,6 +52,8 @@ func (r MetricResource) Homepage(rw http.ResponseWriter, req *http.Request) { } } + rw.Header().Set("Content-Type", "text/html") + _, err = rw.Write([]byte(body)) if err != nil { logging.LogErrorCtx(ctx, err) From 8b7cf0cdca0507b2da163a44ef58aaf639c12c1b Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Wed, 24 Jul 2024 17:28:43 +0300 Subject: [PATCH 10/11] iter8 features --- .DS_Store | Bin 6148 -> 6148 bytes go.mod | 4 +- go.sum | 4 +- internal/entities/errors.go | 2 +- internal/server/handlers.go | 2 +- internal/server/handlers_test.go | 4 +- internal/storage/memory.go | 14 +- internal/storage/memory_test.go | 161 +++++++------ internal/storage/record_test.go | 56 +++-- internal/storage/service.go | 23 +- internal/storage/service_test.go | 307 +++++++++++++++++++++++++ internal/storage/storage_mock.go | 32 +++ internal/utils/http.go | 5 + internal/utils/utils_test.go | 61 +++++ internal/validators/validators_test.go | 3 +- 15 files changed, 565 insertions(+), 113 deletions(-) create mode 100644 internal/storage/service_test.go create mode 100644 internal/storage/storage_mock.go create mode 100644 internal/utils/utils_test.go diff --git a/.DS_Store b/.DS_Store index 9dd95b1cbe9427140f471242c09dec42604f7a83..846ce6a254621adcbeb33eed42adde336fa12a1b 100644 GIT binary patch delta 105 zcmZoMXfc=|#>B)qu~2NHo}w^20|Nsi1A_oVesWSyeiD!;u(5C@BP09d0G8;<_N*L} zrC9?eFJ(2D%+01VIhD<5GC%u|%?cccm^ZU?@N)nS+APTNoq009h$9D3GsrBK%@HDN Fm;np}7vul{ delta 443 zcmZvZF-rqM5QU#RLXI<%ph?XEG+C4ZSIak0;{X$0@~*X7`C?;0t)wA z?EM#72>Nz|=&4&QGrRA7GrKbxOa^y7wHQvXdedlqwGChaXv1WTm`PF{%7g69FVcN& z@=SkUQWSmo#mHH+x&kpz@4pnbsZP2fa*Q!T34?QNIP{E^M8_-1{zH?5-TwTamH#|gqu;RN?fk{=FYJN5RgIt(a5NmitJqd2 O4mcS_BC4a#Og;fYYG@|_ diff --git a/go.mod b/go.mod index 5508c4c..a842faa 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,6 @@ require ( github.com/caarlos0/env/v11 v11.1.0 github.com/go-chi/chi v1.5.5 github.com/go-chi/chi/v5 v5.1.0 - github.com/pkg/errors v0.9.1 github.com/rs/zerolog v1.33.0 github.com/satori/go.uuid v1.2.0 github.com/spf13/pflag v1.0.5 @@ -17,8 +16,9 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.19 // indirect + github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.2 // indirect - golang.org/x/sys v0.21.0 // indirect + golang.org/x/sys v0.22.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 2d08347..2ee3bb2 100644 --- a/go.sum +++ b/go.sum @@ -31,8 +31,8 @@ github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8 golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws= -golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI= +golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/internal/entities/errors.go b/internal/entities/errors.go index 3c5c7fd..ffa4393 100644 --- a/internal/entities/errors.go +++ b/internal/entities/errors.go @@ -5,7 +5,7 @@ import "errors" var ( ErrBadAddressFormat = errors.New("bad net address format") - ErrMetricNotFound = errors.New("metric not found") + ErrRecordNotFound = errors.New("metric not found") ErrMetricUnknown = errors.New("unknown metric type") ErrMetricReport = errors.New("metric report error") ErrMetricMissingName = errors.New("metric name is missing") diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 5ea6a39..466b0c5 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -213,7 +213,7 @@ func (r MetricResource) GetMetricJSON(rw http.ResponseWriter, req *http.Request) func errToStatus(err error) int { switch err { - case entities.ErrMetricNotFound, entities.ErrMetricMissingName: + case entities.ErrRecordNotFound, entities.ErrMetricMissingName: return http.StatusNotFound case entities.ErrMetricUnknown, entities.ErrMetricInvalidValue, diff --git a/internal/server/handlers_test.go b/internal/server/handlers_test.go index d0ab2cb..ed6c329 100644 --- a/internal/server/handlers_test.go +++ b/internal/server/handlers_test.go @@ -379,7 +379,7 @@ func TestGetMetricJSON(t *testing.T) { name: "Should fail on unknown counter", mex: metrics.NewGetCounterMex("test"), mock: func(m *storage.ServiceMock) { - m.On("Get", "test", "counter").Return(storage.Record{}, entities.ErrMetricNotFound) + m.On("Get", "test", "counter").Return(storage.Record{}, entities.ErrRecordNotFound) }, expected: result{ code: http.StatusNotFound, @@ -389,7 +389,7 @@ func TestGetMetricJSON(t *testing.T) { name: "Should fail on unknown gauge", mex: metrics.NewGetGaugeMex("test"), mock: func(m *storage.ServiceMock) { - m.On("Get", "test", "gauge").Return(storage.Record{}, entities.ErrMetricNotFound) + m.On("Get", "test", "gauge").Return(storage.Record{}, entities.ErrRecordNotFound) }, expected: result{ code: http.StatusNotFound, diff --git a/internal/storage/memory.go b/internal/storage/memory.go index 47d5fcf..933efba 100644 --- a/internal/storage/memory.go +++ b/internal/storage/memory.go @@ -8,35 +8,35 @@ import ( var _ MetricsStorage = (*MemStorage)(nil) type MemStorage struct { - Data map[string]Record + data map[string]Record } func NewMemStorage() *MemStorage { return &MemStorage{ - Data: make(map[string]Record), + data: make(map[string]Record), } } func (s *MemStorage) Push(id string, record Record) error { - s.Data[id] = record + s.data[id] = record return nil } func (s *MemStorage) Get(id string) (Record, error) { - record, ok := s.Data[id] + record, ok := s.data[id] if !ok { - return Record{}, entities.ErrMetricNotFound + return Record{}, entities.ErrRecordNotFound } return record, nil } func (s *MemStorage) List() ([]Record, error) { - arr := make([]Record, len(s.Data)) + arr := make([]Record, len(s.data)) i := 0 - for _, record := range s.Data { + for _, record := range s.data { arr[i] = record i++ } diff --git a/internal/storage/memory_test.go b/internal/storage/memory_test.go index 5abae1f..caf8b27 100644 --- a/internal/storage/memory_test.go +++ b/internal/storage/memory_test.go @@ -1,108 +1,129 @@ -package storage_test +package storage import ( "testing" "github.com/ex0rcist/metflix/internal/metrics" - "github.com/ex0rcist/metflix/internal/storage" - "github.com/stretchr/testify/require" ) -func TestPushCounter(t *testing.T) { - require := require.New(t) - - strg := storage.NewMemStorage() - - name := "test" - id := storage.CalculateRecordID(name, "counter") - - value := metrics.Counter(42) - record := storage.Record{Name: name, Value: value} - err := strg.Push(id, record) +func TestMemStorage_Push(t *testing.T) { + strg := NewMemStorage() - require.NoError(err) - require.Equal(value, strg.Data[record.CalculateRecordID()].Value) -} - -func TestPushGauge(t *testing.T) { - require := require.New(t) + records := []Record{ + {Name: "counter", Value: metrics.Counter(42)}, + {Name: "gauge", Value: metrics.Gauge(42.42)}, + } - strg := storage.NewMemStorage() + for _, r := range records { + id := r.CalculateRecordID() - value := metrics.Gauge(42.42) - record := storage.Record{Name: "test", Value: value} - err := strg.Push(record.CalculateRecordID(), record) + err := strg.Push(id, r) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } - require.NoError(err) - require.Equal(value, strg.Data[record.CalculateRecordID()].Value) + if s, _ := strg.Get(id); r != s { + t.Fatalf("expected record %v, got %v", r, s) + } + } } -func TestPushWithSameName(t *testing.T) { - require := require.New(t) - - strg := storage.NewMemStorage() +func TestMemStorage_Push_WithSameName(t *testing.T) { + strg := NewMemStorage() counterValue := metrics.Counter(42) gaugeValue := metrics.Gauge(42.42) - record1 := storage.Record{Name: "test", Value: counterValue} - id1 := record1.CalculateRecordID() - err1 := strg.Push(id1, record1) - require.NoError(err1) + records := []Record{ + {Name: "test", Value: counterValue}, + {Name: "test", Value: gaugeValue}, + } - record2 := storage.Record{Name: "test", Value: gaugeValue} - id2 := record2.CalculateRecordID() - err2 := strg.Push(id2, record2) - require.NoError(err2) + for _, r := range records { + id := r.CalculateRecordID() - require.Equal(counterValue, strg.Data[record1.CalculateRecordID()].Value) - require.Equal(gaugeValue, strg.Data[record2.CalculateRecordID()].Value) -} + if err := strg.Push(id, r); err != nil { + t.Fatalf("expected no error, got %v", err) + } + } -func TestGet(t *testing.T) { - require := require.New(t) + storedCounter, err := strg.Get(records[0].CalculateRecordID()) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } - strg := storage.NewMemStorage() + storedGauge, err := strg.Get(records[1].CalculateRecordID()) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } - value := metrics.Counter(6) - record := storage.Record{Name: "test", Value: value} - id := record.CalculateRecordID() - err := strg.Push(id, record) - require.NoError(err) + if storedCounter != records[0] { + t.Fatalf("expected stored %v, got %v", records[0], storedCounter) + } - gotRecord, err := strg.Get(record.CalculateRecordID()) - require.NoError(err) - require.Equal(value, gotRecord.Value) + if storedGauge != records[1] { + t.Fatalf("expected stored %v, got %v", records[1], storedGauge) + } } -func TestGetNonExistantKey(t *testing.T) { - require := require.New(t) +func TestMemStorage_Get(t *testing.T) { + strg := NewMemStorage() + record := Record{Name: "1", Value: metrics.Counter(42)} + err := strg.Push(record.CalculateRecordID(), record) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } - strg := storage.NewMemStorage() + tests := []struct { + name string + id string + want Record + wantError bool + }{ + {name: "existing record", id: record.CalculateRecordID(), want: record, wantError: false}, + {name: "non-existing record", id: "test", want: Record{}, wantError: true}, + } - _, err := strg.Get("none") - require.Error(err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := strg.Get(tt.id) + if (err != nil) != tt.wantError { + t.Fatalf("expected error: %v, got %v", tt.wantError, err) + } + if got != tt.want { + t.Fatalf("expected record %v, got %v", tt.want, got) + } + }) + } } -func TestList(t *testing.T) { - require := require.New(t) +func TestMemStorage_List(t *testing.T) { + storage := NewMemStorage() - strg := storage.NewMemStorage() + records := []Record{ + {Name: "gauge", Value: metrics.Gauge(42.42)}, + {Name: "counter", Value: metrics.Counter(42)}, + } - records := []storage.Record{ - {Name: "test1", Value: metrics.Counter(1)}, - {Name: "test2", Value: metrics.Counter(2)}, - {Name: "test3", Value: metrics.Gauge(3.4)}, + err := storage.Push(records[0].CalculateRecordID(), records[0]) + if err != nil { + t.Fatalf("expected no error, got %v", err) } - for _, r := range records { - err := strg.Push(r.CalculateRecordID(), r) - require.NoError(err) + err = storage.Push(records[1].CalculateRecordID(), records[1]) + if err != nil { + t.Fatalf("expected no error, got %v", err) } - allRecords, err := strg.List() + got, err := storage.List() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if len(got) != len(records) { + t.Fatalf("expected %d records, got %d", len(records), len(got)) + } - require.NoError(err) - require.ElementsMatch(records, allRecords) + require.ElementsMatch(t, records, got) } diff --git a/internal/storage/record_test.go b/internal/storage/record_test.go index 1a4e834..bd1341f 100644 --- a/internal/storage/record_test.go +++ b/internal/storage/record_test.go @@ -1,29 +1,51 @@ -package storage_test +package storage import ( "testing" "github.com/ex0rcist/metflix/internal/metrics" - "github.com/ex0rcist/metflix/internal/storage" - "github.com/stretchr/testify/require" ) -func TestRecordId(t *testing.T) { - require := require.New(t) +func TestCalculateRecordID(t *testing.T) { + tests := []struct { + test string + name string + kind string + expected string + }{ + {test: "valid inputs", name: "metricName", kind: "metricKind", expected: "metricName_metricKind"}, + {test: "empty name", name: "", kind: "metricKind", expected: ""}, + {test: "empty kind", name: "metricName", kind: "", expected: ""}, + {test: "both empty", name: "", kind: "", expected: ""}, + } - record1 := storage.Record{Name: "test", Value: metrics.Counter(42)} - record2 := storage.Record{Name: "test", Value: metrics.Gauge(42.42)} - - require.Equal("test_counter", record1.CalculateRecordID()) - require.Equal("test_gauge", record2.CalculateRecordID()) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CalculateRecordID(tt.name, tt.kind) + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + }) + } } -func TestRecordIdWithEmptyName(t *testing.T) { - require := require.New(t) - require.Equal("", storage.CalculateRecordID("", "counter")) -} +func TestRecord_CalculateRecordID(t *testing.T) { + tests := []struct { + name string + record Record + expected string + }{ + {name: "valid record with counter", record: Record{Name: "metricName", Value: metrics.Counter(100)}, expected: "metricName_counter"}, + {name: "valid record with gauge", record: Record{Name: "metricName", Value: metrics.Gauge(100.0)}, expected: "metricName_gauge"}, + {name: "empty name", record: Record{Name: "", Value: metrics.Counter(100)}, expected: ""}, + } -func TestRecordIdWithEmptyKind(t *testing.T) { - require := require.New(t) - require.Equal("", storage.CalculateRecordID("test", "")) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.record.CalculateRecordID() + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + }) + } } diff --git a/internal/storage/service.go b/internal/storage/service.go index 0593fb8..ad1a63f 100644 --- a/internal/storage/service.go +++ b/internal/storage/service.go @@ -39,15 +39,13 @@ func (s Service) Get(name, kind string) (Record, error) { } func (s Service) Push(record Record) (Record, error) { - id := CalculateRecordID(record.Name, record.Value.Kind()) - - newValue, err := s.calculateNewValue(id, record) + newValue, err := s.calculateNewValue(record) if err != nil { return Record{}, err } record.Value = newValue - err = s.storage.Push(id, record) + err = s.storage.Push(record.CalculateRecordID(), record) if err != nil { return Record{}, err @@ -69,17 +67,22 @@ func (s Service) List() ([]Record, error) { return records, nil } -func (s Service) calculateNewValue(id string, newRecord Record) (metrics.Metric, error) { - if newRecord.Value.Kind() != "counter" { - return newRecord.Value, nil +func (s Service) calculateNewValue(record Record) (metrics.Metric, error) { + if record.Value.Kind() != "counter" { + return record.Value, nil + } + + id := record.CalculateRecordID() + if id == "" { + return record.Value, entities.ErrMetricMissingName } storedRecord, err := s.storage.Get(id) - if errors.Is(err, entities.ErrMetricNotFound) { - return newRecord.Value, nil + if errors.Is(err, entities.ErrRecordNotFound) { + return record.Value, nil } else if err != nil { return nil, err } - return storedRecord.Value.(metrics.Counter) + newRecord.Value.(metrics.Counter), nil + return storedRecord.Value.(metrics.Counter) + record.Value.(metrics.Counter), nil } diff --git a/internal/storage/service_test.go b/internal/storage/service_test.go new file mode 100644 index 0000000..bbeb5a7 --- /dev/null +++ b/internal/storage/service_test.go @@ -0,0 +1,307 @@ +package storage + +import ( + "testing" + + "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/metrics" + "github.com/stretchr/testify/mock" +) + +func TestService_Get(t *testing.T) { + type args struct { + name string + kind string + } + + tests := []struct { + name string + mock func(m *StorageMock) + args args + expected Record + wantErr bool + }{ + { + name: "existing record", + mock: func(m *StorageMock) { + m.On("Get", "test_counter").Return(Record{Name: "test", Value: metrics.Counter(42)}, nil) + + }, + args: args{name: "test", kind: "counter"}, + expected: Record{Name: "test", Value: metrics.Counter(42)}, + wantErr: false, + }, + + { + name: "non-existing record", + mock: func(m *StorageMock) { + m.On("Get", "test_counter").Return(Record{}, entities.ErrRecordNotFound) + }, + args: args{name: "test", kind: "counter"}, + expected: Record{}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := new(StorageMock) + service := NewService(m) + + if tt.mock != nil { + tt.mock(m) + } + + result, err := service.Get(tt.args.name, tt.args.kind) + if (err != nil) != tt.wantErr { + t.Fatalf("expected error: %v, got %v", tt.wantErr, err) + } + if result != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) + } +} + +// m.On("Push", mock.AnythingOfType("Record")).Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) + +func TestService_Push(t *testing.T) { + tests := []struct { + name string + mock func(m *StorageMock) + record Record + expected Record + wantErr bool + }{ + { + name: "new counter record", + mock: func(m *StorageMock) { + r := Record{Name: "test", Value: metrics.Counter(42)} + + m.On("Get", "test_counter").Return(Record{}, entities.ErrRecordNotFound) + m.On("Push", "test_counter", r).Return(nil) // no error, successful push + }, + record: Record{Name: "test", Value: metrics.Counter(42)}, + expected: Record{Name: "test", Value: metrics.Counter(42)}, + wantErr: false, + }, + { + name: "update counter record", + mock: func(m *StorageMock) { + oldr := Record{Name: "test", Value: metrics.Counter(42)} + newr := Record{Name: "test", Value: metrics.Counter(84)} + + m.On("Get", "test_counter").Return(oldr, nil) + m.On("Push", "test_counter", newr).Return(nil) // no error, successful push + }, + record: Record{Name: "test", Value: metrics.Counter(42)}, + expected: Record{Name: "test", Value: metrics.Counter(84)}, + wantErr: false, + }, + { + name: "new gauge record", + mock: func(m *StorageMock) { + r := Record{Name: "test", Value: metrics.Gauge(42.42)} + + m.On("Get", "test_gauge").Return(Record{}, entities.ErrRecordNotFound) + m.On("Push", "test_gauge", r).Return(nil) // no error, successful push + }, + record: Record{Name: "test", Value: metrics.Gauge(42.42)}, + expected: Record{Name: "test", Value: metrics.Gauge(42.42)}, + wantErr: false, + }, + { + name: "update gauge record", + mock: func(m *StorageMock) { + oldr := Record{Name: "test", Value: metrics.Gauge(42.42)} + newr := Record{Name: "test", Value: metrics.Gauge(43.43)} + + m.On("Get", "test_gauge").Return(oldr, nil) + m.On("Push", "test_gauge", newr).Return(nil) // no error, successful push + }, + record: Record{Name: "test", Value: metrics.Gauge(43.43)}, + expected: Record{Name: "test", Value: metrics.Gauge(43.43)}, + wantErr: false, + }, + { + name: "underlying error", + mock: func(m *StorageMock) { + m.On("Push", "test_gauge", mock.AnythingOfType("Record")).Return(entities.ErrUnexpected) + }, + record: Record{Name: "test", Value: metrics.Gauge(43.43)}, + expected: Record{}, + wantErr: true, + }, + { + name: "missing record name", + record: Record{Name: "", Value: metrics.Counter(43)}, + expected: Record{}, + wantErr: true, + }, + { + name: "storage get error", + mock: func(m *StorageMock) { + m.On("Get", "test_counter").Return(Record{}, entities.ErrUnexpected) + }, + record: Record{Name: "test", Value: metrics.Counter(43)}, + expected: Record{}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := new(StorageMock) + service := NewService(m) + + if tt.mock != nil { + tt.mock(m) + } + + result, err := service.Push(tt.record) + if (err != nil) != tt.wantErr { + t.Fatalf("expected error: %v, got %v", tt.wantErr, err) + } + if result != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestService_List(t *testing.T) { + tests := []struct { + name string + mock func(m *StorageMock) + expected []Record + wantErr bool + }{ + { + name: "normal list", + mock: func(m *StorageMock) { + m.On("List").Return([]Record{ + {Name: "metricX", Value: metrics.Counter(42)}, + {Name: "metricA", Value: metrics.Gauge(42.42)}, + }, nil) + }, + expected: []Record{ // sorted + {Name: "metricA", Value: metrics.Gauge(42.42)}, + {Name: "metricX", Value: metrics.Counter(42)}, + }, + wantErr: false, + }, + + { + name: "had error", + mock: func(m *StorageMock) { + m.On("List").Return([]Record{}, entities.ErrUnexpected) + }, + expected: []Record{}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + m := new(StorageMock) + service := NewService(m) + + if tt.mock != nil { + tt.mock(m) + } + + result, err := service.List() + if (err != nil) != tt.wantErr { + t.Fatalf("expected error: %v, got %v", tt.wantErr, err) + } + + if len(result) != len(tt.expected) { + t.Fatalf("expected %d records, got %d", len(tt.expected), len(result)) + } + + for i, record := range result { + if record != tt.expected[i] { + t.Errorf("expected %v, got %v", tt.expected[i], record) + } + } + + }) + } + +} + +func TestService_calculateNewValue(t *testing.T) { + tests := []struct { + name string + mock func(m *StorageMock) + record Record + expected metrics.Metric + wantErr bool + }{ + { + name: "new counter record", + mock: func(m *StorageMock) { + m.On("Get", "test_counter").Return(Record{}, entities.ErrRecordNotFound) + }, + record: Record{Name: "test", Value: metrics.Counter(42)}, + expected: metrics.Counter(42), + wantErr: false, + }, + { + name: "existing counter record", + mock: func(m *StorageMock) { + m.On("Get", "test_counter").Return(Record{Name: "test", Value: metrics.Counter(42)}, nil) + }, + record: Record{Name: "test", Value: metrics.Counter(42)}, + expected: metrics.Counter(84), + wantErr: false, + }, + { + name: "new gauge record", + mock: func(m *StorageMock) { + m.On("Get", "test_gauge").Return(Record{}, entities.ErrRecordNotFound) + }, + record: Record{Name: "test", Value: metrics.Gauge(42.42)}, + expected: metrics.Gauge(42.42), + wantErr: false, + }, + { + name: "existing gauge record", + mock: func(m *StorageMock) { + m.On("Get", "test_gauge").Return(Record{Name: "test", Value: metrics.Gauge(42.42)}) + }, + record: Record{Name: "test", Value: metrics.Gauge(43.43)}, + expected: metrics.Gauge(43.43), + wantErr: false, + }, + { + name: "underlying error", + mock: func(m *StorageMock) { + m.On("Get", "test_counter").Return(Record{}, entities.ErrUnexpected) + }, + record: Record{Name: "test", Value: metrics.Counter(42)}, + expected: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + + m := new(StorageMock) + service := NewService(m) + + if tt.mock != nil { + tt.mock(m) + } + + result, err := service.calculateNewValue(tt.record) + if (err != nil) != tt.wantErr { + t.Fatalf("expected error: %v, got %v", tt.wantErr, err) + } + if result != tt.expected { + t.Errorf("expected %v, got %v", tt.expected, result) + } + }) + } +} diff --git a/internal/storage/storage_mock.go b/internal/storage/storage_mock.go new file mode 100644 index 0000000..9639d89 --- /dev/null +++ b/internal/storage/storage_mock.go @@ -0,0 +1,32 @@ +package storage + +import ( + "github.com/stretchr/testify/mock" +) + +// Ensure StorageMock implements MetricsStorage +var _ MetricsStorage = (*StorageMock)(nil) + +type StorageMock struct { + mock.Mock +} + +func (m *StorageMock) Get(id string) (Record, error) { + args := m.Called(id) + return args.Get(0).(Record), args.Error(1) +} + +func (m *StorageMock) Push(id string, record Record) error { + args := m.Called(id, record) + return args.Error(0) +} + +func (m *StorageMock) List() ([]Record, error) { + args := m.Called() + + if args.Get(0) == nil { + return nil, args.Error(1) + } + + return args.Get(0).([]Record), args.Error(1) +} diff --git a/internal/utils/http.go b/internal/utils/http.go index c2003cd..9cd4ba7 100644 --- a/internal/utils/http.go +++ b/internal/utils/http.go @@ -3,6 +3,7 @@ package utils import ( "fmt" "net/http" + "sort" "strings" ) @@ -15,5 +16,9 @@ func HeadersToStr(headers http.Header) string { } } + sort.Slice(stringsSlice, func(i, j int) bool { + return stringsSlice[i] < stringsSlice[j] + }) + return strings.Join(stringsSlice, ", ") } diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go new file mode 100644 index 0000000..a8f1dc3 --- /dev/null +++ b/internal/utils/utils_test.go @@ -0,0 +1,61 @@ +package utils + +import ( + "net/http" + "regexp" + "testing" +) + +func TestHeadersToStr(t *testing.T) { + tests := []struct { + name string + headers http.Header + expected string + }{ + { + name: "Single header with single value", + headers: http.Header{ + "Content-Type": {"application/json"}, + }, + expected: "Content-Type:application/json", + }, + { + name: "Single header with multiple values", + headers: http.Header{ + "Accept": {"text/plain", "text/html"}, + }, + expected: "Accept:text/html, Accept:text/plain", + }, + { + name: "Multiple headers with single values", + headers: http.Header{ + "Content-Type": {"application/json"}, + "User-Agent": {"Go-http-client/1.1"}, + }, + expected: "Content-Type:application/json, User-Agent:Go-http-client/1.1", + }, + { + name: "Empty headers", + headers: http.Header{}, + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := HeadersToStr(tt.headers) + if result != tt.expected { + t.Errorf("expected %q, got %q", tt.expected, result) + } + }) + } +} + +func TestGenerateRequestID(t *testing.T) { + regex := regexp.MustCompile(`^[a-f0-9]{8}-[a-f0-9]{4}-4[a-f0-9]{3}-[89ab][a-f0-9]{3}-[a-f0-9]{12}$`) + + requestID := GenerateRequestID() + if !regex.MatchString(requestID) { + t.Errorf("GenerateRequestID() returned invalid UUIDv4: %s", requestID) + } +} diff --git a/internal/validators/validators_test.go b/internal/validators/validators_test.go index 207c2a6..880ff10 100644 --- a/internal/validators/validators_test.go +++ b/internal/validators/validators_test.go @@ -22,12 +22,13 @@ func TestValidateMetric(t *testing.T) { {name: "name not present", args: args{name: "", kind: "counter"}, wantErr: true}, {name: "incorrect name", args: args{name: "некорректноеимя", kind: "counter"}, wantErr: true}, {name: "incorrect name", args: args{name: "incorrect name", kind: "gauge"}, wantErr: true}, + {name: "incorrect name", args: args{name: "correctname", kind: "incorrectgauge"}, wantErr: true}, {name: "incorrect kind", args: args{kind: "gauger"}, wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if err := validators.ValidateMetric(tt.args.name, tt.args.kind); (err != nil) != tt.wantErr { - t.Errorf("ValidateName() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("ValidateName() error = %v, wantErr %v", (err != nil), tt.wantErr) } }) } From f045716c93404cbc41029099a1d249162b47eb8f Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Thu, 25 Jul 2024 11:13:29 +0300 Subject: [PATCH 11/11] iter8 features --- cmd/server/main.go | 5 --- internal/server/router.go | 2 +- internal/server/server.go | 62 +++++++++++++++++++++--------- internal/server/server_test.go | 70 ++++++++++++++++++++++++++++++++++ 4 files changed, 116 insertions(+), 23 deletions(-) create mode 100644 internal/server/server_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index ba8c1e2..2ddce1b 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -15,11 +15,6 @@ func main() { logging.LogFatal(err) } - err = srv.ParseFlags() - if err != nil { - logging.LogFatal(err) - } - err = srv.Run() if err != nil { logging.LogFatal(err) diff --git a/internal/server/router.go b/internal/server/router.go index c300b87..e1a86cf 100644 --- a/internal/server/router.go +++ b/internal/server/router.go @@ -26,7 +26,7 @@ func NewRouter(storageService storage.StorageService) http.Handler { resource := NewMetricResource(storageService) - router.Get("/", resource.Homepage) // TODO: resource? + router.Get("/", resource.Homepage) router.Post("/update/{metricKind}/{metricName}/{metricValue}", resource.UpdateMetric) router.Post("/update", resource.UpdateMetricJSON) diff --git a/internal/server/server.go b/internal/server/server.go index 3514f1d..df86067 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,6 +3,7 @@ package server import ( "fmt" "net/http" + "os" "github.com/caarlos0/env/v11" "github.com/ex0rcist/metflix/internal/entities" @@ -12,9 +13,10 @@ import ( ) type Server struct { - config *Config - Storage storage.MetricsStorage - Router http.Handler + config *Config + httpServer *http.Server + Storage storage.MetricsStorage + Router http.Handler } type Config struct { @@ -26,34 +28,60 @@ func New() (*Server, error) { Address: "0.0.0.0:8080", } + err := parseFlags(config, os.Args[0], os.Args[1:]) + if err != nil { + return nil, err + } + + err = parseEnv(config) + if err != nil { + return nil, err + } + memStorage := storage.NewMemStorage() storageService := storage.NewService(memStorage) - router := NewRouter(storageService) + httpServer := &http.Server{ + Addr: config.Address.String(), + Handler: router, + } + return &Server{ - config: config, - Storage: memStorage, - Router: router, + config: config, + httpServer: httpServer, + Storage: memStorage, + Router: router, }, nil } -func (s *Server) ParseFlags() error { - address := s.config.Address +func parseFlags(config *Config, progname string, args []string) error { + flags := pflag.NewFlagSet(progname, pflag.ContinueOnError) - pflag.VarP(&address, "address", "a", "address:port for HTTP API requests") - pflag.Parse() + address := config.Address + + flags.VarP(&address, "address", "a", "address:port for HTTP API requests") + err := flags.Parse(args) + + if err != nil { + return err + } // because VarP gets non-pointer value, set it manually - pflag.Visit(func(f *pflag.Flag) { + flags.Visit(func(f *pflag.Flag) { switch f.Name { case "address": - s.config.Address = address + config.Address = address } }) - if err := env.Parse(s.config); err != nil { - return entities.NewStackError(err) + return nil +} + +func parseEnv(config *Config) error { + fmt.Println("====== " + os.Getenv("ADDRESS") + " ======") + if err := env.Parse(config); err != nil { + return err } return nil @@ -63,9 +91,9 @@ func (s *Server) Run() error { logging.LogInfo(s.config.String()) logging.LogInfo("server ready") - return http.ListenAndServe(s.config.Address.String(), s.Router) + return s.httpServer.ListenAndServe() } func (c Config) String() string { - return "server config: " + fmt.Sprintf("address=%s\t", c.Address) + return "server config: " + fmt.Sprintf("address=%s", c.Address) } diff --git a/internal/server/server_test.go b/internal/server/server_test.go new file mode 100644 index 0000000..e9da1ff --- /dev/null +++ b/internal/server/server_test.go @@ -0,0 +1,70 @@ +package server + +import ( + "testing" +) + +func TestNew(t *testing.T) { + server, err := New() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + if server.Storage == nil { + t.Fatal("expected storage to not be nil") + } + + if server.Router == nil { + t.Fatal("expected router to not be nil") + } +} + +func TestParseFlags(t *testing.T) { + + tests := []struct { + name string + args []string + want Config + wantErr bool + }{ + { + name: "shortcut", + args: []string{"-a0.0.0.0:8080"}, + want: Config{Address: "0.0.0.0:8080"}, + wantErr: false, + }, + { + name: "shortcut", + args: []string{"--address=127.0.0.1:81"}, + want: Config{Address: "127.0.0.1:81"}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + config := &Config{Address: "default"} + + err := parseFlags(config, "progname", tt.args) + if (err != nil) != tt.wantErr { + t.Fatalf("Expected no error, got %v", err) + } + if tt.want != *config { + t.Errorf("Expected %v, got %v", tt.want, config) + } + }) + } +} + +func TestRun(t *testing.T) { + // pending: how to test lsitenAndServe? goroutine? +} + +// TestConfigString tests the String method of Config. +func TestConfigString(t *testing.T) { + config := Config{Address: "0.0.0.0:8080"} + expected := "server config: address=0.0.0.0:8080" + if config.String() != expected { + t.Errorf("Expected %v, got %v", expected, config.String()) + } +}