From 49d85d60f443bebb12ef78d0729a34430c3590ed Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Mon, 15 Jul 2024 11:45:02 +0300 Subject: [PATCH 01/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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/15] 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()) + } +} From fd9be8c9a92f87aa859047446e494c86baa6b04b Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Thu, 25 Jul 2024 15:03:35 +0300 Subject: [PATCH 12/15] added basic iter9 features --- db | 1 + db2 | 1 + internal/agent/agent.go | 9 +- internal/logging/warn.go | 22 +++++ internal/middleware/middleware.go | 5 +- internal/server/handlers_test.go | 18 ++-- internal/server/server.go | 143 ++++++++++++++++++++++++++---- internal/server/server_test.go | 13 +-- internal/storage/file.go | 96 ++++++++++++++++++++ internal/storage/memory.go | 26 ++++-- internal/storage/record.go | 57 +++++++++++- internal/storage/storage.go | 7 ++ internal/storage/storage_mock.go | 4 + internal/utils/time.go | 7 ++ 14 files changed, 361 insertions(+), 48 deletions(-) create mode 100644 db create mode 100644 db2 create mode 100644 internal/logging/warn.go create mode 100644 internal/storage/file.go create mode 100644 internal/utils/time.go diff --git a/db b/db new file mode 100644 index 0000000..e61643c --- /dev/null +++ b/db @@ -0,0 +1 @@ +{"records":{"Alloc_gauge":{"kind":"gauge","name":"Alloc","value":"1548608"},"BuckHashSys_gauge":{"kind":"gauge","name":"BuckHashSys","value":"7332"},"Frees_gauge":{"kind":"gauge","name":"Frees","value":"8253"},"GCCPUFraction_gauge":{"kind":"gauge","name":"GCCPUFraction","value":"0.0002756141105024748"},"GCSys_gauge":{"kind":"gauge","name":"GCSys","value":"2361592"},"HeapAlloc_gauge":{"kind":"gauge","name":"HeapAlloc","value":"1548608"},"HeapIdle_gauge":{"kind":"gauge","name":"HeapIdle","value":"5398528"},"HeapInuse_gauge":{"kind":"gauge","name":"HeapInuse","value":"2375680"},"HeapObjects_gauge":{"kind":"gauge","name":"HeapObjects","value":"895"},"HeapReleased_gauge":{"kind":"gauge","name":"HeapReleased","value":"2842624"},"HeapSys_gauge":{"kind":"gauge","name":"HeapSys","value":"7774208"},"LastGC_gauge":{"kind":"gauge","name":"LastGC","value":"1721906462828472000"},"Lookups_gauge":{"kind":"gauge","name":"Lookups","value":"0"},"MCacheInuse_gauge":{"kind":"gauge","name":"MCacheInuse","value":"9600"},"MCacheSys_gauge":{"kind":"gauge","name":"MCacheSys","value":"15600"},"MSpanInuse_gauge":{"kind":"gauge","name":"MSpanInuse","value":"82720"},"MSpanSys_gauge":{"kind":"gauge","name":"MSpanSys","value":"97920"},"Mallocs_gauge":{"kind":"gauge","name":"Mallocs","value":"9148"},"NextGC_gauge":{"kind":"gauge","name":"NextGC","value":"4194304"},"NumForcedGC_gauge":{"kind":"gauge","name":"NumForcedGC","value":"0"},"NumGC_gauge":{"kind":"gauge","name":"NumGC","value":"14"},"OtherSys_gauge":{"kind":"gauge","name":"OtherSys","value":"969476"},"PauseTotalNs_gauge":{"kind":"gauge","name":"PauseTotalNs","value":"399125"},"PollCount_counter":{"kind":"counter","name":"PollCount","value":"10"},"RandomValue_gauge":{"kind":"gauge","name":"RandomValue","value":"0.5895582908291291"},"StackInuse_gauge":{"kind":"gauge","name":"StackInuse","value":"589824"},"StackSys_gauge":{"kind":"gauge","name":"StackSys","value":"589824"},"Sys_gauge":{"kind":"gauge","name":"Sys","value":"11815952"},"TotalAlloc_gauge":{"kind":"gauge","name":"TotalAlloc","value":"37623104"}}} diff --git a/db2 b/db2 new file mode 100644 index 0000000..e2f2276 --- /dev/null +++ b/db2 @@ -0,0 +1 @@ +{"records":{}} diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 62bb1cf..9946d18 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -8,6 +8,7 @@ import ( "github.com/caarlos0/env/v11" "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/logging" + "github.com/ex0rcist/metflix/internal/utils" "github.com/spf13/pflag" ) @@ -88,7 +89,7 @@ func (a *Agent) startPolling() { logging.LogError(err) } - time.Sleep(intToDuration(a.Config.PollInterval)) + time.Sleep(utils.IntToDuration(a.Config.PollInterval)) } } @@ -96,7 +97,7 @@ func (a *Agent) startReporting() { defer a.wg.Done() for { - time.Sleep(intToDuration(a.Config.ReportInterval)) + time.Sleep(utils.IntToDuration(a.Config.ReportInterval)) a.reportStats() } @@ -147,10 +148,6 @@ func (a *Agent) reportStats() { a.Stats.PollCount -= snapshot.PollCount } -func intToDuration(s int) time.Duration { - return time.Duration(s) * time.Second -} - func (c Config) String() string { return fmt.Sprintf( "agent config: address=%v; poll-interval=%v; report-interval=%v", diff --git a/internal/logging/warn.go b/internal/logging/warn.go new file mode 100644 index 0000000..83daecf --- /dev/null +++ b/internal/logging/warn.go @@ -0,0 +1,22 @@ +package logging + +import ( + "context" + + "github.com/rs/zerolog" + "github.com/rs/zerolog/log" +) + +func LogWarn(messages ...string) { + logWarn(&log.Logger, messages...) +} + +func LogWarnCtx(ctx context.Context, messages ...string) { + logger := loggerFromContext(ctx) + logWarn(logger, messages...) +} + +func logWarn(logger *zerolog.Logger, messages ...string) { + msg := optMessagesToString(messages) + logger.Warn().Msg(msg) +} diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 962a091..3ee22f5 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -3,6 +3,7 @@ package middleware import ( "errors" "net/http" + "strings" "time" "github.com/ex0rcist/metflix/internal/compression" @@ -33,8 +34,6 @@ func RequestsLogger(next http.Handler) http.Handler { logger.Debug(). Msgf("request: %s", utils.HeadersToStr(r.Header)) - // TODO: context? - // execute ww := middleware.NewWrapResponseWriter(w, r.ProtoMajor) ctx := logger.WithContext(r.Context()) @@ -115,7 +114,7 @@ func needGzipEncoding(r *http.Request) bool { } for _, encoding := range r.Header.Values("Accept-Encoding") { - if encoding == "gzip" { + if strings.Contains(encoding, "gzip") { return true } } diff --git a/internal/server/handlers_test.go b/internal/server/handlers_test.go index ed6c329..3e8af73 100644 --- a/internal/server/handlers_test.go +++ b/internal/server/handlers_test.go @@ -106,7 +106,7 @@ func TestUpdateMetric(t *testing.T) { want result }{ { - name: "push counter", + name: "Should push counter", path: "/update/counter/test/42", mock: func(m *storage.ServiceMock) { m.On("Get", "test", "counter").Return(storage.Record{}, nil) @@ -115,7 +115,7 @@ func TestUpdateMetric(t *testing.T) { want: result{code: http.StatusOK, body: "42"}, }, { - name: "push counter with existing value", + name: "Should push counter with existing value", path: "/update/counter/test/42", mock: func(m *storage.ServiceMock) { m.On("Get", "test", "counter").Return(storage.Record{Name: "test", Value: metrics.Counter(21)}, nil) @@ -124,7 +124,7 @@ func TestUpdateMetric(t *testing.T) { want: result{code: http.StatusOK, body: "42"}, }, { - name: "push gauge", + name: "Should push gauge", path: "/update/gauge/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) @@ -132,32 +132,32 @@ func TestUpdateMetric(t *testing.T) { want: result{code: http.StatusOK, body: "42.42"}, }, { - name: "fail on invalid kind", + name: "Should fail on invalid kind", path: "/update/xxx/test/1", want: result{code: http.StatusBadRequest}, }, { - name: "fail on empty metric name", + name: "Should fail on empty metric name", path: "/update/counter//1", want: result{code: http.StatusNotFound}, }, { - name: "fail on counter with invalid name", + name: "Should fail on counter with invalid name", path: "/update/counter/inva!id/10", want: result{code: http.StatusBadRequest}, }, { - name: "fail on counter with invalid value", + name: "Should fail on counter with invalid value", path: "/update/counter/test/10.0", want: result{code: http.StatusBadRequest}, }, { - name: "fail on gauge with invalid name", + name: "Should fail on gauge with invalid name", path: "/update/gauge/inval!d/42.42", want: result{code: http.StatusBadRequest}, }, { - name: "fail on gauge with invalid value", + name: "Should fail on gauge with invalid value", path: "/update/gauge/test/42.42!", want: result{code: http.StatusBadRequest}, }, diff --git a/internal/server/server.go b/internal/server/server.go index df86067..9d5bf24 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -4,11 +4,14 @@ import ( "fmt" "net/http" "os" + "strings" + "time" "github.com/caarlos0/env/v11" "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/storage" + "github.com/ex0rcist/metflix/internal/utils" "github.com/spf13/pflag" ) @@ -20,26 +23,31 @@ type Server struct { } type Config struct { - Address entities.Address `env:"ADDRESS"` + Address entities.Address `env:"ADDRESS"` + StoreInterval int `env:"STORE_INTERVAL"` + StorePath string `env:"FILE_STORAGE_PATH"` + RestoreOnStart bool `env:"RESTORE"` } func New() (*Server, error) { config := &Config{ - Address: "0.0.0.0:8080", + Address: "0.0.0.0:8080", + StoreInterval: 300, + RestoreOnStart: true, } - err := parseFlags(config, os.Args[0], os.Args[1:]) + err := parseConfig(config) if err != nil { return nil, err } - err = parseEnv(config) + storageKind := detectStorageKind(config) + dataStorage, err := newDataStorage(storageKind, config) if err != nil { return nil, err } - memStorage := storage.NewMemStorage() - storageService := storage.NewService(memStorage) + storageService := storage.NewService(dataStorage) router := NewRouter(storageService) httpServer := &http.Server{ @@ -50,28 +58,117 @@ func New() (*Server, error) { return &Server{ config: config, httpServer: httpServer, - Storage: memStorage, + Storage: dataStorage, Router: router, }, nil } +func (s *Server) Run() error { + logging.LogInfo(s.String()) + + // restore storage if possible + if s.storageNeedsRestore() { + if err := s.restoreStorage(); err != nil { + return err + } + } + + logging.LogInfo("server ready") + + // start dumping if neede + if s.storageNeedsDumping() { + go s.startStorageDumping() + } + + return s.httpServer.ListenAndServe() +} + +func (s *Server) String() string { + str := []string{ + fmt.Sprintf("address=%s", s.config.Address), + fmt.Sprintf("storage=%s", s.Storage.Kind()), + } + + if s.Storage.Kind() == storage.KindFile { + str = append(str, fmt.Sprintf("store-interval=%d", s.config.StoreInterval)) + str = append(str, fmt.Sprintf("store-path=%s", s.config.StorePath)) + str = append(str, fmt.Sprintf("restore=%t", s.config.RestoreOnStart)) + } + + return "server config: " + strings.Join(str, "; ") +} + +func (s *Server) storageNeedsRestore() bool { + return s.Storage.Kind() == storage.KindFile && s.config.RestoreOnStart +} + +func (s *Server) restoreStorage() error { + // HELP: не уверен что тут корректное решение... Но иначе нужно добавлять Restore() в интерфейс, а его логически нет у MemStorage + return s.Storage.(*storage.FileStorage).Restore() +} + +func (s *Server) storageNeedsDumping() bool { + return s.Storage.Kind() == storage.KindFile && s.config.StoreInterval > 0 +} + +func (s *Server) startStorageDumping() { + ticker := time.NewTicker(utils.IntToDuration(s.config.StoreInterval)) + defer ticker.Stop() + + for { + _, ok := <-ticker.C + if !ok { + break + } + + // HELP: не уверен что тут корректное решение... Но иначе нужно добавлять Dump() в интерфейс, а его логически нет у MemStorage + if err := s.Storage.(*storage.FileStorage).Dump(); err != nil { + logging.LogError(fmt.Errorf("error during FileStorage Dump(): %s", err.Error())) + } + } +} + +func parseConfig(config *Config) error { + err := parseFlags(config, os.Args[0], os.Args[1:]) + if err != nil { + return err + } + + err = parseEnv(config) + if err != nil { + return err + } + + return nil +} + func parseFlags(config *Config, progname string, args []string) error { flags := pflag.NewFlagSet(progname, pflag.ContinueOnError) address := config.Address - flags.VarP(&address, "address", "a", "address:port for HTTP API requests") - err := flags.Parse(args) + // define flags + storeInterval := flags.IntP("store-interval", "i", config.StoreInterval, "interval (s) for dumping metrics to the disk, zero value means saving after each request") + storePath := flags.StringP("store-file", "f", config.StorePath, "path to file to store metrics") + restoreOnStart := flags.BoolP("restore", "r", config.RestoreOnStart, "whether to restore state on startup") + + err := flags.Parse(args) if err != nil { return err } - // because VarP gets non-pointer value, set it manually + // fill values flags.Visit(func(f *pflag.Flag) { switch f.Name { case "address": config.Address = address + case "store-interval": + config.StoreInterval = *storeInterval + case "store-file": + config.StorePath = *storePath + case "restore": + config.RestoreOnStart = *restoreOnStart } }) @@ -79,7 +176,6 @@ func parseFlags(config *Config, progname string, args []string) error { } func parseEnv(config *Config) error { - fmt.Println("====== " + os.Getenv("ADDRESS") + " ======") if err := env.Parse(config); err != nil { return err } @@ -87,13 +183,26 @@ func parseEnv(config *Config) error { return nil } -func (s *Server) Run() error { - logging.LogInfo(s.config.String()) - logging.LogInfo("server ready") +func detectStorageKind(c *Config) string { + var sk string - return s.httpServer.ListenAndServe() + switch { + case c.StorePath != "": + sk = storage.KindFile + default: + sk = storage.KindMemory + } + + return sk } -func (c Config) String() string { - return "server config: " + fmt.Sprintf("address=%s", c.Address) +func newDataStorage(kind string, config *Config) (storage.MetricsStorage, error) { + switch kind { + case storage.KindMemory: + return storage.NewMemStorage(), nil + case storage.KindFile: + return storage.NewFileStorage(config.StorePath, config.StoreInterval), nil + default: + return nil, fmt.Errorf("unknown storage type") + } } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index e9da1ff..6f11dbc 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -60,11 +60,12 @@ 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()) +func TestString(t *testing.T) { + // config := Config{Address: "0.0.0.0:8080"} + srv, _ := New() // TODO + + expected := "server config: address=0.0.0.0:8080; storage=memory" + if srv.String() != expected { + t.Errorf("Expected %v, got %v", expected, srv.String()) } } diff --git a/internal/storage/file.go b/internal/storage/file.go new file mode 100644 index 0000000..12dfeca --- /dev/null +++ b/internal/storage/file.go @@ -0,0 +1,96 @@ +package storage + +import ( + "encoding/json" + "fmt" + "os" + + "github.com/ex0rcist/metflix/internal/logging" +) + +// check that FileStorage implements MetricsStorage +var _ MetricsStorage = (*FileStorage)(nil) + +type FileStorage struct { + *MemStorage + + storePath string + syncMode bool +} + +func NewFileStorage(storePath string, storeInterval int) *FileStorage { + return &FileStorage{ + MemStorage: NewMemStorage(), + storePath: storePath, + syncMode: storeInterval == 0, + } +} + +func (s *FileStorage) Push(id string, record Record) error { + if err := s.MemStorage.Push(id, record); err != nil { + return err + } + + if s.syncMode { + return s.Dump() + } + + return nil +} + +func (s *FileStorage) Dump() (err error) { + logging.LogInfo("dumping storage to file " + s.storePath) + + file, err := os.OpenFile(s.storePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + if err != nil { + return fmt.Errorf("error during FileStorage.Dump()/os.OpenFile(): %w", err) + } + + defer func() { + if closeErr := file.Close(); err == nil && closeErr != nil { + err = fmt.Errorf("error during FileStorage.Dump()/file.Close(): %w", closeErr) + } + }() + + encoder := json.NewEncoder(file) + snapshot := s.Snapshot() + + if err := encoder.Encode(snapshot); err != nil { + return fmt.Errorf("error during FileStorage.Dump()/encoder.Encode(): %w", err) + } + + return nil +} + +func (s *FileStorage) Restore() (err error) { + logging.LogInfo("restoring storage from file " + s.storePath) + + file, err := os.Open(s.storePath) + if err != nil { + if os.IsNotExist(err) { + logging.LogWarn("no storage dump found to restore") + return nil + } + + return fmt.Errorf("error during FileStorage.Restore()/os.Open(): %w", err) + } + + defer func() { + if closeErr := file.Close(); err == nil && closeErr != nil { + err = fmt.Errorf("error during FileStorage.Restore()/file.Close(): %w", closeErr) + } + }() + + decoder := json.NewDecoder(file) + if err := decoder.Decode(s.MemStorage); err != nil { + return fmt.Errorf("error during FileStorage.Restore()/decoder.Decode(): %w", err) + } + + logging.LogInfo("storage data was restored") + + return nil +} + +func (s *FileStorage) Kind() string { + return KindFile +} diff --git a/internal/storage/memory.go b/internal/storage/memory.go index 933efba..f06e9c3 100644 --- a/internal/storage/memory.go +++ b/internal/storage/memory.go @@ -8,23 +8,23 @@ import ( var _ MetricsStorage = (*MemStorage)(nil) type MemStorage struct { - data map[string]Record + Data map[string]Record `json:"records"` } 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.ErrRecordNotFound } @@ -33,13 +33,27 @@ func (s *MemStorage) Get(id string) (Record, error) { } 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++ } return arr, nil } + +func (s *MemStorage) Snapshot() *MemStorage { + snapshot := make(map[string]Record, len(s.Data)) + + for k, v := range s.Data { + snapshot[k] = v + } + + return &MemStorage{Data: snapshot} +} + +func (s *MemStorage) Kind() string { + return KindMemory +} diff --git a/internal/storage/record.go b/internal/storage/record.go index 56c5378..9fce565 100644 --- a/internal/storage/record.go +++ b/internal/storage/record.go @@ -1,6 +1,12 @@ package storage -import "github.com/ex0rcist/metflix/internal/metrics" +import ( + "encoding/json" + "fmt" + + "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/metrics" +) type Record struct { Name string @@ -18,3 +24,52 @@ func CalculateRecordID(name, kind string) string { func (r Record) CalculateRecordID() string { return CalculateRecordID(r.Name, r.Value.Kind()) } + +func (r Record) MarshalJSON() ([]byte, error) { + jv, err := json.Marshal(map[string]string{ + "name": r.Name, + "kind": r.Value.Kind(), + "value": r.Value.String(), + }) + + if err != nil { + return nil, fmt.Errorf("record marshaling fail: %w", err) + } + + return jv, nil +} + +func (r *Record) UnmarshalJSON(src []byte) error { + var data map[string]string + + if err := json.Unmarshal(src, &data); err != nil { + return unmarshalFailed(err) + } + + r.Name = data["name"] + + switch data["kind"] { + case "counter": + value, err := metrics.ToCounter(data["value"]) + if err != nil { + return unmarshalFailed(err) + } + + r.Value = value + case "gauge": + value, err := metrics.ToGauge(data["value"]) + if err != nil { + return unmarshalFailed(err) + } + + r.Value = value + default: + return unmarshalFailed(entities.ErrMetricUnknown) + } + + return nil +} + +func unmarshalFailed(reason error) error { + return fmt.Errorf("record unmarshaling failed: %w", reason) +} diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 65629f4..7474bad 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -1,8 +1,15 @@ package storage +const ( + KindMemory = "memory" + KindFile = "file" + KindMock = "mock" +) + // common interface for storages: mem, file, etc type MetricsStorage interface { Push(id string, record Record) error Get(id string) (Record, error) List() ([]Record, error) + Kind() string } diff --git a/internal/storage/storage_mock.go b/internal/storage/storage_mock.go index 9639d89..9c40e70 100644 --- a/internal/storage/storage_mock.go +++ b/internal/storage/storage_mock.go @@ -30,3 +30,7 @@ func (m *StorageMock) List() ([]Record, error) { return args.Get(0).([]Record), args.Error(1) } + +func (m *StorageMock) Kind() string { + return KindMock +} diff --git a/internal/utils/time.go b/internal/utils/time.go new file mode 100644 index 0000000..0deff25 --- /dev/null +++ b/internal/utils/time.go @@ -0,0 +1,7 @@ +package utils + +import "time" + +func IntToDuration(s int) time.Duration { + return time.Duration(s) * time.Second +} From 42a295da31674147c0c10eb19ec8b95e85e8a52a Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Thu, 25 Jul 2024 16:24:53 +0300 Subject: [PATCH 13/15] iter9 features --- db | 1 - db2 | 1 - internal/compression/compression_test.go | 31 ++++ internal/compression/compressor.go | 2 + internal/compression/compressor_test.go | 97 ++++++++++ internal/compression/decompressor.go | 2 + internal/compression/decompressor_test.go | 130 ++++++++++++++ internal/middleware/middleware_test.go | 204 ++++++++++++++++++++++ internal/storage/file_test.go | 85 +++++++++ internal/storage/record.go | 12 +- internal/storage/record_test.go | 55 ++++++ internal/utils/utils_test.go | 21 +++ 12 files changed, 631 insertions(+), 10 deletions(-) delete mode 100644 db delete mode 100644 db2 create mode 100644 internal/compression/compression_test.go create mode 100644 internal/compression/compressor_test.go create mode 100644 internal/compression/decompressor_test.go create mode 100644 internal/middleware/middleware_test.go create mode 100644 internal/storage/file_test.go diff --git a/db b/db deleted file mode 100644 index e61643c..0000000 --- a/db +++ /dev/null @@ -1 +0,0 @@ -{"records":{"Alloc_gauge":{"kind":"gauge","name":"Alloc","value":"1548608"},"BuckHashSys_gauge":{"kind":"gauge","name":"BuckHashSys","value":"7332"},"Frees_gauge":{"kind":"gauge","name":"Frees","value":"8253"},"GCCPUFraction_gauge":{"kind":"gauge","name":"GCCPUFraction","value":"0.0002756141105024748"},"GCSys_gauge":{"kind":"gauge","name":"GCSys","value":"2361592"},"HeapAlloc_gauge":{"kind":"gauge","name":"HeapAlloc","value":"1548608"},"HeapIdle_gauge":{"kind":"gauge","name":"HeapIdle","value":"5398528"},"HeapInuse_gauge":{"kind":"gauge","name":"HeapInuse","value":"2375680"},"HeapObjects_gauge":{"kind":"gauge","name":"HeapObjects","value":"895"},"HeapReleased_gauge":{"kind":"gauge","name":"HeapReleased","value":"2842624"},"HeapSys_gauge":{"kind":"gauge","name":"HeapSys","value":"7774208"},"LastGC_gauge":{"kind":"gauge","name":"LastGC","value":"1721906462828472000"},"Lookups_gauge":{"kind":"gauge","name":"Lookups","value":"0"},"MCacheInuse_gauge":{"kind":"gauge","name":"MCacheInuse","value":"9600"},"MCacheSys_gauge":{"kind":"gauge","name":"MCacheSys","value":"15600"},"MSpanInuse_gauge":{"kind":"gauge","name":"MSpanInuse","value":"82720"},"MSpanSys_gauge":{"kind":"gauge","name":"MSpanSys","value":"97920"},"Mallocs_gauge":{"kind":"gauge","name":"Mallocs","value":"9148"},"NextGC_gauge":{"kind":"gauge","name":"NextGC","value":"4194304"},"NumForcedGC_gauge":{"kind":"gauge","name":"NumForcedGC","value":"0"},"NumGC_gauge":{"kind":"gauge","name":"NumGC","value":"14"},"OtherSys_gauge":{"kind":"gauge","name":"OtherSys","value":"969476"},"PauseTotalNs_gauge":{"kind":"gauge","name":"PauseTotalNs","value":"399125"},"PollCount_counter":{"kind":"counter","name":"PollCount","value":"10"},"RandomValue_gauge":{"kind":"gauge","name":"RandomValue","value":"0.5895582908291291"},"StackInuse_gauge":{"kind":"gauge","name":"StackInuse","value":"589824"},"StackSys_gauge":{"kind":"gauge","name":"StackSys","value":"589824"},"Sys_gauge":{"kind":"gauge","name":"Sys","value":"11815952"},"TotalAlloc_gauge":{"kind":"gauge","name":"TotalAlloc","value":"37623104"}}} diff --git a/db2 b/db2 deleted file mode 100644 index e2f2276..0000000 --- a/db2 +++ /dev/null @@ -1 +0,0 @@ -{"records":{}} diff --git a/internal/compression/compression_test.go b/internal/compression/compression_test.go new file mode 100644 index 0000000..ec23374 --- /dev/null +++ b/internal/compression/compression_test.go @@ -0,0 +1,31 @@ +package compression + +import ( + "bytes" + "compress/gzip" + "io" + "testing" +) + +func TestPack_Success(t *testing.T) { + data := []byte("test data") + buffer, err := Pack(data) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + reader, err := gzip.NewReader(buffer) + if err != nil { + t.Fatalf("expected no error creating gzip reader, got %v", err) + } + defer reader.Close() + + unpackedData, err := io.ReadAll(reader) + if err != nil { + t.Fatalf("expected no error reading from gzip reader, got %v", err) + } + + if !bytes.Equal(data, unpackedData) { + t.Fatalf("expected %s, got %s", data, unpackedData) + } +} diff --git a/internal/compression/compressor.go b/internal/compression/compressor.go index 46ff7a2..6eaec34 100644 --- a/internal/compression/compressor.go +++ b/internal/compression/compressor.go @@ -58,4 +58,6 @@ func (c *Compressor) Close() { if err := c.encoder.Close(); err != nil { logging.LogErrorCtx(c.context, err, "error closing compressor encoder", err.Error()) } + + c.encoder = nil } diff --git a/internal/compression/compressor_test.go b/internal/compression/compressor_test.go new file mode 100644 index 0000000..99b1d0f --- /dev/null +++ b/internal/compression/compressor_test.go @@ -0,0 +1,97 @@ +package compression + +import ( + "bytes" + "compress/gzip" + "context" + "net/http/httptest" + "testing" +) + +func TestCompressor_Write_SupportedContent(t *testing.T) { + ctx := context.Background() + recorder := httptest.NewRecorder() + + compressor := NewCompressor(recorder, ctx) + compressor.Header().Set("Content-Type", "application/json") + + data := []byte(`{"message": "test"}`) + n, err := compressor.Write(data) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if n != len(data) { + t.Fatalf("expected %d bytes written, got %d", len(data), n) + } + + compressor.Close() + + resp := recorder.Result() + defer resp.Body.Close() + + if resp.Header.Get("Content-Encoding") != "gzip" { + t.Fatalf("expected Content-Encoding to be gzip, got %s", resp.Header.Get("Content-Encoding")) + } + + gr, err := gzip.NewReader(resp.Body) + if err != nil { + t.Fatalf("expected no error creating gzip reader, got %v", err) + } + defer gr.Close() + + uncompressedData := new(bytes.Buffer) + _, err = uncompressedData.ReadFrom(gr) + if err != nil { + t.Fatalf("expected no error decompressing dara, got %v", err) + } + if !bytes.Equal(data, uncompressedData.Bytes()) { + t.Fatalf("expected %s, got %s", data, uncompressedData.Bytes()) + } +} + +func TestCompressor_Write_UnsupportedContent(t *testing.T) { + recorder := httptest.NewRecorder() + ctx := context.Background() + compressor := NewCompressor(recorder, ctx) + compressor.Header().Set("Content-Type", "text/plain") + + data := []byte("test data") + n, err := compressor.Write(data) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if n != len(data) { + t.Fatalf("expected %d bytes written, got %d", len(data), n) + } + + resp := recorder.Result() + defer resp.Body.Close() + + if resp.Header.Get("Content-Encoding") == "gzip" { + t.Fatalf("expected Content-Encoding to not be gzip") + } + + body := recorder.Body.Bytes() + if !bytes.Equal(data, body) { + t.Fatalf("expected %s, got %s", data, body) + } +} + +func TestCompressor_Close(t *testing.T) { + recorder := httptest.NewRecorder() + ctx := context.Background() + compressor := NewCompressor(recorder, ctx) + compressor.Header().Set("Content-Type", "application/json") + + data := []byte(`{"message": "test"}`) + _, err := compressor.Write(data) + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + compressor.Close() + + if compressor.encoder != nil { + t.Fatalf("expected encoder to be nil after close") + } +} diff --git a/internal/compression/decompressor.go b/internal/compression/decompressor.go index 4b86c10..d9eee9d 100644 --- a/internal/compression/decompressor.go +++ b/internal/compression/decompressor.go @@ -70,4 +70,6 @@ func (d *Decompressor) Close() { if err := d.reader.Close(); err != nil { logging.LogErrorCtx(d.context, err, "error closing decompressor reader", err.Error()) } + + d.reader = nil } diff --git a/internal/compression/decompressor_test.go b/internal/compression/decompressor_test.go new file mode 100644 index 0000000..934cd80 --- /dev/null +++ b/internal/compression/decompressor_test.go @@ -0,0 +1,130 @@ +package compression + +import ( + "bytes" + "compress/gzip" + "context" + "errors" + "io" + + "github.com/ex0rcist/metflix/internal/entities" + + "net/http" + "net/http/httptest" + "testing" +) + +func TestDecompressor_Decompress_SupportedEncoding(t *testing.T) { + data := []byte("test data") + var buf bytes.Buffer + writer := gzip.NewWriter(&buf) + _, err := writer.Write(data) + if err != nil { + t.Fatalf("expected no error on writer.Write(), got %v", err) + } + + writer.Close() + + req := httptest.NewRequest(http.MethodPost, "/", &buf) + req.Header.Set("Content-Encoding", "gzip") + + ctx := context.Background() + decompressor := NewDecompressor(req, ctx) + + err = decompressor.Decompress() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + decompressedData, err := io.ReadAll(decompressor.request.Body) + if err != nil { + t.Fatalf("expected no error reading decompressed data, got %v", err) + } + + if !bytes.Equal(data, decompressedData) { + t.Fatalf("expected %s, got %s", data, decompressedData) + } + + decompressor.Close() +} + +func TestDecompressor_Decompress_NoEncoding(t *testing.T) { + data := []byte("test data") + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(data)) + + ctx := context.Background() + decompressor := NewDecompressor(req, ctx) + + err := decompressor.Decompress() + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + + decompressedData, err := io.ReadAll(decompressor.request.Body) + if err != nil { + t.Fatalf("expected no error reading data, got %v", err) + } + + if !bytes.Equal(data, decompressedData) { + t.Fatalf("expected %s, got %s", data, decompressedData) + } + + decompressor.Close() +} + +func TestDecompressor_Decompress_UnsupportedEncoding(t *testing.T) { + data := []byte("test data") + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(data)) + req.Header.Set("Content-Encoding", "deflate") + + ctx := context.Background() + decompressor := NewDecompressor(req, ctx) + + err := decompressor.Decompress() + if !errors.Is(err, entities.ErrEncodingUnsupported) { + t.Fatalf("expected %v, got %v", entities.ErrEncodingUnsupported, err) + } +} + +func TestDecompressor_Close(t *testing.T) { + data := []byte("test data") + var buf bytes.Buffer + writer := gzip.NewWriter(&buf) + _, err := writer.Write(data) + if err != nil { + t.Fatalf("expected no error on writer.Write, got %v", err) + } + writer.Close() + + req := httptest.NewRequest(http.MethodPost, "/", &buf) + req.Header.Set("Content-Encoding", "gzip") + + ctx := context.Background() + decompressor := NewDecompressor(req, ctx) + + err = decompressor.Decompress() + if err != nil { + t.Fatalf("expected no error on Decompress(), got %v", err) + } + + decompressor.Close() + + if decompressor.reader != nil { + t.Fatalf("expected reader to be nil after close") + } +} + +func TestDecompressor_Decompress_ErrorOnInit(t *testing.T) { + // providing invalid gzip data to simulate error on NewReader + data := []byte("invalid gzip data") + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(data)) + req.Header.Set("Content-Encoding", "gzip") + + ctx := context.Background() + decompressor := NewDecompressor(req, ctx) + + err := decompressor.Decompress() + if !errors.Is(err, entities.ErrEncodingInternal) { + t.Fatalf("expected %v, got %v", entities.ErrEncodingInternal, err) + } +} diff --git a/internal/middleware/middleware_test.go b/internal/middleware/middleware_test.go new file mode 100644 index 0000000..c61105f --- /dev/null +++ b/internal/middleware/middleware_test.go @@ -0,0 +1,204 @@ +package middleware + +import ( + "bytes" + "compress/gzip" + "io" + "net/http" + "net/http/httptest" + + "testing" +) + +func TestDecompressRequest_Success(t *testing.T) { + data := []byte("test data") + var buf bytes.Buffer + writer := gzip.NewWriter(&buf) + _, err := writer.Write(data) + if err != nil { + t.Fatalf("expected no error writing writer.Write(), got %v", err) + } + + writer.Close() + + req := httptest.NewRequest(http.MethodPost, "/", &buf) + req.Header.Set("Content-Encoding", "gzip") + + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + decompressedData, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("expected no error reading decompressed data, got %v", err) + } + if !bytes.Equal(data, decompressedData) { + t.Fatalf("expected %s, got %s", data, decompressedData) + } + }) + + handler := DecompressRequest(nextHandler) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) +} + +func TestDecompressRequest_NoEncoding(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader([]byte("test data"))) + + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + if string(body) != "test data" { + t.Fatalf("expected 'test data', got %s", string(body)) + } + }) + + handler := DecompressRequest(nextHandler) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) +} + +func TestDecompressRequest_UnsupportedEncoding(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader([]byte("test data"))) + req.Header.Set("Content-Encoding", "deflate") + + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("expected handler not to be called") + }) + + handler := DecompressRequest(nextHandler) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusBadRequest { + t.Fatalf("expected status %d, got %d", http.StatusBadRequest, rr.Code) + } +} + +func TestDecompressRequest_InternalError(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader([]byte("invalid gzip data"))) + req.Header.Set("Content-Encoding", "gzip") + + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("expected handler not to be called") + }) + + handler := DecompressRequest(nextHandler) + rr := httptest.NewRecorder() + + handler.ServeHTTP(rr, req) + + if rr.Code != http.StatusInternalServerError { + t.Fatalf("expected status %d, got %d", http.StatusInternalServerError, rr.Code) + } +} + +func TestCompressResponse_Success(t *testing.T) { + data := []byte("test data") + + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, err := w.Write(data) + + if err != nil { + t.Fatalf("expected no error writing writer.Write(), got %v", err) + } + }) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("Accept-Encoding", "gzip") + + rr := httptest.NewRecorder() + handler := CompressResponse(nextHandler) + + handler.ServeHTTP(rr, req) + + resp := rr.Result() + defer resp.Body.Close() + + if resp.Header.Get("Content-Encoding") != "gzip" { + t.Fatalf("expected Content-Encoding to be gzip, got %s", resp.Header.Get("Content-Encoding")) + } + + gr, err := gzip.NewReader(resp.Body) + if err != nil { + t.Fatalf("expected no error creating gzip reader, got %v", err) + } + defer gr.Close() + + decompressedData, err := io.ReadAll(gr) + if err != nil { + t.Fatalf("expected no error reading decompressed data, got %v", err) + } + + if !bytes.Equal(data, decompressedData) { + t.Fatalf("expected %s, got %s", data, decompressedData) + } +} + +func TestCompressResponse_NoCompressionRequested(t *testing.T) { + data := []byte("test data") + + nextHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := w.Write(data) + if err != nil { + t.Fatalf("expected no error writing writer.Write(), got %v", err) + } + }) + + req := httptest.NewRequest(http.MethodGet, "/", nil) + + rr := httptest.NewRecorder() + handler := CompressResponse(nextHandler) + + handler.ServeHTTP(rr, req) + + resp := rr.Result() + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("expected no error reading body, got %v", err) + } + + if !bytes.Equal(data, body) { + t.Fatalf("expected %s, got %s", data, body) + } +} + +// findOrCreateRequestID tests +func TestFindOrCreateRequestID_ExistingID(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("X-Request-Id", "existing-id") + + requestID := findOrCreateRequestID(req) + if requestID != "existing-id" { + t.Fatalf("expected 'existing-id', got %s", requestID) + } +} + +func TestFindOrCreateRequestID_NewID(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + + requestID := findOrCreateRequestID(req) + if requestID == "" { + t.Fatalf("expected non-empty request ID") + } +} + +// needGzipEncoding tests +func TestNeedGzipEncoding_Supported(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("Accept-Encoding", "gzip") + + if !needGzipEncoding(req) { + t.Fatalf("expected needGzipEncoding to return true") + } +} + +func TestNeedGzipEncoding_Unsupported(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/", nil) + + if needGzipEncoding(req) { + t.Fatalf("expected needGzipEncoding to return false") + } +} diff --git a/internal/storage/file_test.go b/internal/storage/file_test.go new file mode 100644 index 0000000..3be8edc --- /dev/null +++ b/internal/storage/file_test.go @@ -0,0 +1,85 @@ +package storage + +import ( + "os" + "testing" + + "github.com/ex0rcist/metflix/internal/metrics" + "github.com/stretchr/testify/require" +) + +func createStoreWithData(t *testing.T, storePath string, storeInterval int) *FileStorage { + store := NewFileStorage(storePath, storeInterval) + + err := store.Push("test_gauge", Record{Name: "test", Value: metrics.Gauge(42.42)}) + if err != nil { + t.Fatalf("expected no error on push, got %v", err) + } + + err = store.Push("test2_counter", Record{Name: "test2", Value: metrics.Counter(1)}) + if err != nil { + t.Fatalf("expected no error on push, got %v", err) + } + + return store +} + +func TestSyncDumpRestoreStorage(t *testing.T) { + storePath := "/tmp/db.json" + + t.Cleanup(func() { + err := os.Remove(storePath) + if err != nil { + t.Fatalf("expected no error on cleanup, got %v", err) + } + }) + + store := createStoreWithData(t, storePath, 0) + storedData := store.Snapshot() + + store = NewFileStorage(storePath, 0) + err := store.Restore() + if err != nil { + t.Fatalf("expected no error on restore, got %v", err) + } + + restoredData := store.Snapshot() + require.Equal(t, storedData, restoredData) +} + +func TestAsyncDumpRestoreStorage(t *testing.T) { + storePath := "/tmp/db.json" + + t.Cleanup(func() { + err := os.Remove(storePath) + if err != nil { + t.Fatalf("expected no error on cleanup, got %v", err) + } + }) + + store := createStoreWithData(t, storePath, 300) + storedData := store.Snapshot() + + err := store.Dump() + if err != nil { + t.Fatalf("expected no error on dump, got %v", err) + } + + store = NewFileStorage(storePath, 300) + err = store.Restore() + if err != nil { + t.Fatalf("expected no error on restore, got %v", err) + } + + restoredData := store.Snapshot() + require.Equal(t, storedData, restoredData) +} + +func TestRestoreDoesntFailIfNoSourceFile(t *testing.T) { + store := NewFileStorage("xxx", 0) + + err := store.Restore() + if err != nil { + t.Fatalf("expected no error on empty file, got %v", err) + } +} diff --git a/internal/storage/record.go b/internal/storage/record.go index 9fce565..2cbf6b0 100644 --- a/internal/storage/record.go +++ b/internal/storage/record.go @@ -43,7 +43,7 @@ func (r *Record) UnmarshalJSON(src []byte) error { var data map[string]string if err := json.Unmarshal(src, &data); err != nil { - return unmarshalFailed(err) + return fmt.Errorf("record unmarshaling failed: %w", err) } r.Name = data["name"] @@ -52,24 +52,20 @@ func (r *Record) UnmarshalJSON(src []byte) error { case "counter": value, err := metrics.ToCounter(data["value"]) if err != nil { - return unmarshalFailed(err) + return fmt.Errorf("record unmarshaling failed: %w", err) } r.Value = value case "gauge": value, err := metrics.ToGauge(data["value"]) if err != nil { - return unmarshalFailed(err) + return fmt.Errorf("record unmarshaling failed: %w", err) } r.Value = value default: - return unmarshalFailed(entities.ErrMetricUnknown) + return fmt.Errorf("record unmarshaling failed: %w", entities.ErrMetricUnknown) } return nil } - -func unmarshalFailed(reason error) error { - return fmt.Errorf("record unmarshaling failed: %w", reason) -} diff --git a/internal/storage/record_test.go b/internal/storage/record_test.go index bd1341f..3995f42 100644 --- a/internal/storage/record_test.go +++ b/internal/storage/record_test.go @@ -1,9 +1,13 @@ package storage import ( + "encoding/json" + "strconv" "testing" + "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/metrics" + "github.com/stretchr/testify/require" ) func TestCalculateRecordID(t *testing.T) { @@ -49,3 +53,54 @@ func TestRecord_CalculateRecordID(t *testing.T) { }) } } + +func TestRecordMarshalingNormalData(t *testing.T) { + tests := []struct { + name string + source Record + }{ + {name: "Should convert counter", source: Record{Name: "PollCount", Value: metrics.Counter(10)}}, + {name: "Should convert gauge", source: Record{Name: "Alloc", Value: metrics.Gauge(42.0)}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + json, err := tt.source.MarshalJSON() + if err != nil { + t.Fatalf("expected no error marshaling json, got: %v", err) + } + + target := new(Record) + err = target.UnmarshalJSON(json) + if err != nil { + t.Fatalf("expected no error unmarshaling json, got: %v", err) + } + + if tt.source != *target { + t.Fatal("expected records to be equal:", tt.source, target) + } + }) + } +} + +func TestRecordUnmarshalingCorruptedData(t *testing.T) { + tests := []struct { + name string + data string + expected error + }{ + {name: "Should fail on broken json", data: `{"name": "xxx",`, expected: &json.SyntaxError{}}, + {name: "Should fail on invalid counter", data: `{"name": "xxx", "kind": "counter", "value": "12.345"}`, expected: strconv.ErrSyntax}, + {name: "Should fail on invalid gauge", data: `{"name": "xxx", "kind": "gauge", "value": "12.)"}`, expected: strconv.ErrSyntax}, + {name: "Should fail on unknown kind", data: `{"name": "xxx", "kind": "unknown", "value": "12"}`, expected: entities.ErrMetricUnknown}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := new(Record) + + err := r.UnmarshalJSON([]byte(tt.data)) + require.ErrorAs(t, err, &tt.expected) + }) + } +} diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index a8f1dc3..3eacc73 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -4,8 +4,29 @@ import ( "net/http" "regexp" "testing" + "time" ) +func TestIntToDuration(t *testing.T) { + tests := []struct { + input int + expected time.Duration + }{ + {input: 0, expected: 0 * time.Second}, + {input: 1, expected: 1 * time.Second}, + {input: 60, expected: 60 * time.Second}, + {input: -1, expected: -1 * time.Second}, + {input: 3600, expected: 3600 * time.Second}, + } + + for _, tt := range tests { + result := IntToDuration(tt.input) + if result != tt.expected { + t.Errorf("IntToDuration(%d) = %v; expected %v", tt.input, result, tt.expected) + } + } +} + func TestHeadersToStr(t *testing.T) { tests := []struct { name string From 97b8cc934d4d57afeb9cf08c17ef3e380cdbf58b Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Fri, 26 Jul 2024 17:51:20 +0300 Subject: [PATCH 14/15] iter9 review fixes --- internal/agent/api.go | 10 +- internal/metrics/types.go | 17 +-- internal/metrics/types_test.go | 4 +- internal/server/converters.go | 8 +- internal/server/handlers.go | 7 +- internal/server/handlers_test.go | 20 ++-- internal/server/server.go | 53 +--------- internal/storage/file.go | 66 +++++++++--- internal/storage/file_test.go | 139 ++++++++++++++++--------- internal/storage/memory.go | 4 +- internal/storage/memory_test.go | 8 +- internal/storage/record.go | 4 +- internal/storage/service.go | 2 +- internal/storage/service_test.go | 4 +- internal/storage/storage.go | 2 +- internal/storage/storage_mock.go | 6 +- internal/validators/validators.go | 3 +- internal/validators/validators_test.go | 11 +- 18 files changed, 200 insertions(+), 168 deletions(-) diff --git a/internal/agent/api.go b/internal/agent/api.go index e3d3bdd..29fdc0d 100644 --- a/internal/agent/api.go +++ b/internal/agent/api.go @@ -40,16 +40,14 @@ func NewAPI(address *entities.Address, httpTransport http.RoundTripper) *API { func (c *API) Report(name string, metric metrics.Metric) *API { url := "http://" + c.address.String() + "/update" - var requestID = utils.GenerateRequestID() - var ctx = setupLoggerCtx(requestID) + requestID := utils.GenerateRequestID() + ctx := setupLoggerCtx(requestID) var mex metrics.MetricExchange - // HELP: можно ли тут вместо приведения типов - // использовать рефлексию через metric.(type) ? switch metric.Kind() { - case "counter": + case metrics.KindCounter: mex = metrics.NewUpdateCounterMex(name, metric.(metrics.Counter)) - case "gauge": + case metrics.KindGauge: mex = metrics.NewUpdateGaugeMex(name, metric.(metrics.Gauge)) default: logging.LogError(entities.ErrMetricReport, "unknown metric") diff --git a/internal/metrics/types.go b/internal/metrics/types.go index 9e852d8..28153e2 100644 --- a/internal/metrics/types.go +++ b/internal/metrics/types.go @@ -6,29 +6,30 @@ import ( "github.com/ex0rcist/metflix/internal/entities" ) +const ( + KindCounter = "counter" + KindGauge = "gauge" +) + type Metric interface { Kind() string String() string } -// Counter - type Counter int64 func (c Counter) Kind() string { - return "counter" + return KindCounter } func (c Counter) String() string { return strconv.FormatInt(int64(c), 10) } -// Gauge - type Gauge float64 func (g Gauge) Kind() string { - return "gauge" + return KindGauge } func (g Gauge) String() string { @@ -70,9 +71,9 @@ func NewUpdateGaugeMex(name string, value Gauge) MetricExchange { } func NewGetCounterMex(name string) MetricExchange { - return MetricExchange{ID: name, MType: "counter"} + return MetricExchange{ID: name, MType: KindCounter} } func NewGetGaugeMex(name string) MetricExchange { - return MetricExchange{ID: name, MType: "gauge"} + return MetricExchange{ID: name, MType: KindGauge} } diff --git a/internal/metrics/types_test.go b/internal/metrics/types_test.go index 15632d0..e6ce4e9 100644 --- a/internal/metrics/types_test.go +++ b/internal/metrics/types_test.go @@ -13,12 +13,12 @@ func TestKind(t *testing.T) { { name: "kind = counter", metric: Counter(1), - want: "counter", + want: KindCounter, }, { name: "kind = gauge", metric: Gauge(1.01), - want: "gauge", + want: KindGauge, }, } for _, tt := range tests { diff --git a/internal/server/converters.go b/internal/server/converters.go index 72786c5..bc505de 100644 --- a/internal/server/converters.go +++ b/internal/server/converters.go @@ -16,13 +16,13 @@ func toRecord(mex *metrics.MetricExchange) (storage.Record, error) { } switch mex.MType { - case "counter": + case metrics.KindCounter: if mex.Delta == nil { return record, entities.ErrMetricMissingValue } record = storage.Record{Name: mex.ID, Value: *mex.Delta} - case "gauge": + case metrics.KindGauge: if mex.Value == nil { return record, entities.ErrMetricMissingValue } @@ -39,11 +39,11 @@ func toMetricExchange(record storage.Record) (*metrics.MetricExchange, error) { req := &metrics.MetricExchange{ID: record.Name, MType: record.Value.Kind()} switch record.Value.Kind() { - case "counter": + case metrics.KindCounter: delta, _ := record.Value.(metrics.Counter) req.Delta = &delta - case "gauge": + case metrics.KindGauge: value, _ := record.Value.(metrics.Gauge) req.Value = &value } diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 466b0c5..1504a19 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -29,9 +29,6 @@ func writeErrorResponse(ctx context.Context, w http.ResponseWriter, code int, er logging.LogErrorCtx(ctx, err) w.WriteHeader(code) // only header for now - - // resp := fmt.Sprintf("%d %v", code, err) - // http.Error(w, resp, code) } func (r MetricResource) Homepage(rw http.ResponseWriter, req *http.Request) { @@ -71,7 +68,7 @@ func (r MetricResource) UpdateMetric(rw http.ResponseWriter, req *http.Request) rawValue := req.PathValue("metricValue") switch mex.MType { - case "counter": + case metrics.KindCounter: delta, err := metrics.ToCounter(rawValue) if err != nil { writeErrorResponse(ctx, rw, errToStatus(err), err) @@ -79,7 +76,7 @@ func (r MetricResource) UpdateMetric(rw http.ResponseWriter, req *http.Request) } mex.Delta = &delta - case "gauge": + case metrics.KindGauge: value, err := metrics.ToGauge(rawValue) if err != nil { writeErrorResponse(ctx, rw, errToStatus(err), err) diff --git a/internal/server/handlers_test.go b/internal/server/handlers_test.go index 3e8af73..48d9546 100644 --- a/internal/server/handlers_test.go +++ b/internal/server/handlers_test.go @@ -109,7 +109,7 @@ func TestUpdateMetric(t *testing.T) { name: "Should push counter", path: "/update/counter/test/42", mock: func(m *storage.ServiceMock) { - m.On("Get", "test", "counter").Return(storage.Record{}, nil) + m.On("Get", "test", metrics.KindCounter).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"}, @@ -118,7 +118,7 @@ func TestUpdateMetric(t *testing.T) { name: "Should push counter with existing value", path: "/update/counter/test/42", mock: func(m *storage.ServiceMock) { - m.On("Get", "test", "counter").Return(storage.Record{Name: "test", Value: metrics.Counter(21)}, nil) + m.On("Get", "test", metrics.KindCounter).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: "42"}, @@ -196,7 +196,7 @@ func TestUpdateJSONMetric(t *testing.T) { 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("Get", "test", metrics.KindCounter).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}, @@ -283,7 +283,7 @@ func TestGetMetric(t *testing.T) { 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) + m.On("Get", "test", metrics.KindCounter).Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) }, want: result{code: http.StatusOK, body: "42"}, }, @@ -291,7 +291,7 @@ func TestGetMetric(t *testing.T) { 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) + m.On("Get", "test", metrics.KindGauge).Return(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}, nil) }, want: result{code: http.StatusOK, body: "42.42"}, }, @@ -350,7 +350,7 @@ func TestGetMetricJSON(t *testing.T) { 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) + m.On("Get", "test", metrics.KindCounter).Return(storage.Record{Name: "test", Value: metrics.Counter(42)}, nil) }, expected: result{ code: http.StatusOK, @@ -361,7 +361,7 @@ func TestGetMetricJSON(t *testing.T) { 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) + m.On("Get", "test", metrics.KindGauge).Return(storage.Record{Name: "test", Value: metrics.Gauge(42.42)}, nil) }, expected: result{ code: http.StatusOK, @@ -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.ErrRecordNotFound) + m.On("Get", "test", metrics.KindCounter).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.ErrRecordNotFound) + m.On("Get", "test", metrics.KindGauge).Return(storage.Record{}, entities.ErrRecordNotFound) }, expected: result{ code: http.StatusNotFound, @@ -413,7 +413,7 @@ func TestGetMetricJSON(t *testing.T) { 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) + m.On("Get", "test", metrics.KindGauge).Return(storage.Record{}, entities.ErrUnexpected) }, expected: result{ code: http.StatusInternalServerError, diff --git a/internal/server/server.go b/internal/server/server.go index 9d5bf24..7c1fba1 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -5,13 +5,11 @@ import ( "net/http" "os" "strings" - "time" "github.com/caarlos0/env/v11" "github.com/ex0rcist/metflix/internal/entities" "github.com/ex0rcist/metflix/internal/logging" "github.com/ex0rcist/metflix/internal/storage" - "github.com/ex0rcist/metflix/internal/utils" "github.com/spf13/pflag" ) @@ -65,31 +63,20 @@ func New() (*Server, error) { func (s *Server) Run() error { logging.LogInfo(s.String()) - - // restore storage if possible - if s.storageNeedsRestore() { - if err := s.restoreStorage(); err != nil { - return err - } - } - logging.LogInfo("server ready") - // start dumping if neede - if s.storageNeedsDumping() { - go s.startStorageDumping() - } - return s.httpServer.ListenAndServe() } func (s *Server) String() string { + kind := detectStorageKind(s.config) + str := []string{ fmt.Sprintf("address=%s", s.config.Address), - fmt.Sprintf("storage=%s", s.Storage.Kind()), + fmt.Sprintf("storage=%s", kind), } - if s.Storage.Kind() == storage.KindFile { + if kind == storage.KindFile { str = append(str, fmt.Sprintf("store-interval=%d", s.config.StoreInterval)) str = append(str, fmt.Sprintf("store-path=%s", s.config.StorePath)) str = append(str, fmt.Sprintf("restore=%t", s.config.RestoreOnStart)) @@ -98,36 +85,6 @@ func (s *Server) String() string { return "server config: " + strings.Join(str, "; ") } -func (s *Server) storageNeedsRestore() bool { - return s.Storage.Kind() == storage.KindFile && s.config.RestoreOnStart -} - -func (s *Server) restoreStorage() error { - // HELP: не уверен что тут корректное решение... Но иначе нужно добавлять Restore() в интерфейс, а его логически нет у MemStorage - return s.Storage.(*storage.FileStorage).Restore() -} - -func (s *Server) storageNeedsDumping() bool { - return s.Storage.Kind() == storage.KindFile && s.config.StoreInterval > 0 -} - -func (s *Server) startStorageDumping() { - ticker := time.NewTicker(utils.IntToDuration(s.config.StoreInterval)) - defer ticker.Stop() - - for { - _, ok := <-ticker.C - if !ok { - break - } - - // HELP: не уверен что тут корректное решение... Но иначе нужно добавлять Dump() в интерфейс, а его логически нет у MemStorage - if err := s.Storage.(*storage.FileStorage).Dump(); err != nil { - logging.LogError(fmt.Errorf("error during FileStorage Dump(): %s", err.Error())) - } - } -} - func parseConfig(config *Config) error { err := parseFlags(config, os.Args[0], os.Args[1:]) if err != nil { @@ -201,7 +158,7 @@ func newDataStorage(kind string, config *Config) (storage.MetricsStorage, error) case storage.KindMemory: return storage.NewMemStorage(), nil case storage.KindFile: - return storage.NewFileStorage(config.StorePath, config.StoreInterval), nil + return storage.NewFileStorage(config.StorePath, config.StoreInterval, config.RestoreOnStart) default: return nil, fmt.Errorf("unknown storage type") } diff --git a/internal/storage/file.go b/internal/storage/file.go index 12dfeca..6ae2e15 100644 --- a/internal/storage/file.go +++ b/internal/storage/file.go @@ -4,8 +4,11 @@ import ( "encoding/json" "fmt" "os" + "sync" + "time" "github.com/ex0rcist/metflix/internal/logging" + "github.com/ex0rcist/metflix/internal/utils" ) // check that FileStorage implements MetricsStorage @@ -13,17 +16,34 @@ var _ MetricsStorage = (*FileStorage)(nil) type FileStorage struct { *MemStorage + sync.Mutex - storePath string - syncMode bool + storePath string + storeInterval int + restoreOnStart bool + dumpTicker *time.Ticker } -func NewFileStorage(storePath string, storeInterval int) *FileStorage { - return &FileStorage{ - MemStorage: NewMemStorage(), - storePath: storePath, - syncMode: storeInterval == 0, +func NewFileStorage(storePath string, storeInterval int, restoreOnStart bool) (*FileStorage, error) { + fs := &FileStorage{ + MemStorage: NewMemStorage(), + storePath: storePath, + storeInterval: storeInterval, + restoreOnStart: restoreOnStart, } + + if fs.restoreOnStart { + if err := fs.restore(); err != nil { + return nil, err + } + } + + if fs.storeInterval > 0 { + fs.dumpTicker = time.NewTicker(utils.IntToDuration(fs.storeInterval)) + go fs.startStorageDumping(fs.dumpTicker) + } + + return fs, nil } func (s *FileStorage) Push(id string, record Record) error { @@ -31,14 +51,23 @@ func (s *FileStorage) Push(id string, record Record) error { return err } - if s.syncMode { - return s.Dump() + if s.storeInterval == 0 { + return s.dump() } return nil } -func (s *FileStorage) Dump() (err error) { +func (s *FileStorage) Close() error { + if s.dumpTicker != nil { + fmt.Println("stopping ticker") + s.dumpTicker.Stop() + } + + return s.dump() +} + +func (s *FileStorage) dump() (err error) { logging.LogInfo("dumping storage to file " + s.storePath) file, err := os.OpenFile(s.storePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) @@ -62,7 +91,7 @@ func (s *FileStorage) Dump() (err error) { return nil } -func (s *FileStorage) Restore() (err error) { +func (s *FileStorage) restore() (err error) { logging.LogInfo("restoring storage from file " + s.storePath) file, err := os.Open(s.storePath) @@ -91,6 +120,17 @@ func (s *FileStorage) Restore() (err error) { return nil } -func (s *FileStorage) Kind() string { - return KindFile +func (s *FileStorage) startStorageDumping(ticker *time.Ticker) { + defer ticker.Stop() + + for { + _, ok := <-ticker.C + if !ok { + break + } + + if err := s.dump(); err != nil { + logging.LogError(fmt.Errorf("error during FileStorage Dump(): %s", err.Error())) + } + } } diff --git a/internal/storage/file_test.go b/internal/storage/file_test.go index 3be8edc..948b139 100644 --- a/internal/storage/file_test.go +++ b/internal/storage/file_test.go @@ -1,85 +1,120 @@ package storage import ( + "encoding/json" + "fmt" "os" "testing" + "time" "github.com/ex0rcist/metflix/internal/metrics" - "github.com/stretchr/testify/require" ) -func createStoreWithData(t *testing.T, storePath string, storeInterval int) *FileStorage { - store := NewFileStorage(storePath, storeInterval) - - err := store.Push("test_gauge", Record{Name: "test", Value: metrics.Gauge(42.42)}) +func checkNoError(t *testing.T, err error, msg string) { if err != nil { - t.Fatalf("expected no error on push, got %v", err) + t.Fatalf("%s: %v", msg, err) } +} - err = store.Push("test2_counter", Record{Name: "test2", Value: metrics.Counter(1)}) - if err != nil { - t.Fatalf("expected no error on push, got %v", err) +func removeFile(t *testing.T, path string) { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + t.Fatalf("failed to remove file: %v", err) } +} - return store +func TestNewFileStorage(t *testing.T) { + storePath := "test_store.json" + defer removeFile(t, storePath) + + fs, err := NewFileStorage(storePath, 0, false) + checkNoError(t, err, "failed to create new FileStorage") + + if fs == nil { + t.Fatalf("expected FileStorage instance, got nil") + } + if fs.storePath != storePath { + t.Errorf("expected storePath=%s, got %s", storePath, fs.storePath) + } + if fs.storeInterval != 0 { + t.Errorf("expected storeInterval=0, got %d", fs.storeInterval) + } + if fs.restoreOnStart { + t.Errorf("expected restoreOnStart=false, got true") + } } -func TestSyncDumpRestoreStorage(t *testing.T) { - storePath := "/tmp/db.json" +func TestSyncPushAndDump(t *testing.T) { + storePath := "test_store.json" + defer removeFile(t, storePath) - t.Cleanup(func() { - err := os.Remove(storePath) - if err != nil { - t.Fatalf("expected no error on cleanup, got %v", err) - } - }) + fs, err := NewFileStorage(storePath, 0, false) + checkNoError(t, err, "failed to create new FileStorage") - store := createStoreWithData(t, storePath, 0) - storedData := store.Snapshot() + record := Record{Name: "test", Value: metrics.Counter(42)} + err = fs.Push(record.CalculateRecordID(), record) + checkNoError(t, err, "failed to push record") - store = NewFileStorage(storePath, 0) - err := store.Restore() - if err != nil { - t.Fatalf("expected no error on restore, got %v", err) - } + data, err := os.ReadFile(storePath) + checkNoError(t, err, "failed to read storage file") + fmt.Println(string(data)) + + err = json.Unmarshal(data, &fs.MemStorage) + checkNoError(t, err, "failed to unmarshal storage file") - restoredData := store.Snapshot() - require.Equal(t, storedData, restoredData) + if got, err := fs.Get(record.CalculateRecordID()); err != nil || got != record { + t.Errorf("expected record %v, got %v", record, got) + } } -func TestAsyncDumpRestoreStorage(t *testing.T) { - storePath := "/tmp/db.json" +func TestRestore(t *testing.T) { + storePath := "test_store.json" + defer removeFile(t, storePath) - t.Cleanup(func() { - err := os.Remove(storePath) - if err != nil { - t.Fatalf("expected no error on cleanup, got %v", err) - } - }) + // create dump + fs1, err := NewFileStorage(storePath, 0, false) + checkNoError(t, err, "failed to create new FileStorage") - store := createStoreWithData(t, storePath, 300) - storedData := store.Snapshot() + record := Record{Name: "test", Value: metrics.Counter(42)} + err = fs1.Push(record.CalculateRecordID(), record) // dumped + checkNoError(t, err, "failed to push to FileStorage") - err := store.Dump() - if err != nil { - t.Fatalf("expected no error on dump, got %v", err) - } + // new storage from dump + fs2, err := NewFileStorage(storePath, 0, true) + checkNoError(t, err, "failed to create new FileStorage") - store = NewFileStorage(storePath, 300) - err = store.Restore() + restoredRecord, err := fs2.Get(record.CalculateRecordID()) if err != nil { - t.Fatalf("expected no error on restore, got %v", err) + checkNoError(t, err, "expected to find restored record, but did not") + } + if restoredRecord != record { + t.Errorf("expected restored record %v, got %v", record, restoredRecord) } - - restoredData := store.Snapshot() - require.Equal(t, storedData, restoredData) } -func TestRestoreDoesntFailIfNoSourceFile(t *testing.T) { - store := NewFileStorage("xxx", 0) +func TestAsyncDumping(t *testing.T) { + storePath := "test_store.json" + defer removeFile(t, storePath) - err := store.Restore() - if err != nil { - t.Fatalf("expected no error on empty file, got %v", err) + fs, err := NewFileStorage(storePath, 1, false) + checkNoError(t, err, "failed to create new FileStorage") + + record := Record{Name: "test", Value: metrics.Counter(42)} + err = fs.Push(record.CalculateRecordID(), record) + checkNoError(t, err, "failed to push record") + + time.Sleep(1000 * time.Millisecond) + + err = fs.Close() + checkNoError(t, err, "failed to close fs") + + data, err := os.ReadFile(storePath) + checkNoError(t, err, "failed to read storage file") + + ms := NewMemStorage() + err = json.Unmarshal(data, &ms) + checkNoError(t, err, "failed to unmarshal storage file") + + if restored, err := ms.Get(record.CalculateRecordID()); err != nil || restored != record { + t.Errorf("expected record %v, got %v", record, restored) } } diff --git a/internal/storage/memory.go b/internal/storage/memory.go index f06e9c3..a38b1be 100644 --- a/internal/storage/memory.go +++ b/internal/storage/memory.go @@ -54,6 +54,6 @@ func (s *MemStorage) Snapshot() *MemStorage { return &MemStorage{Data: snapshot} } -func (s *MemStorage) Kind() string { - return KindMemory +func (s *MemStorage) Close() error { + return nil // do nothing } diff --git a/internal/storage/memory_test.go b/internal/storage/memory_test.go index caf8b27..5b60253 100644 --- a/internal/storage/memory_test.go +++ b/internal/storage/memory_test.go @@ -11,8 +11,8 @@ func TestMemStorage_Push(t *testing.T) { strg := NewMemStorage() records := []Record{ - {Name: "counter", Value: metrics.Counter(42)}, - {Name: "gauge", Value: metrics.Gauge(42.42)}, + {Name: metrics.KindCounter, Value: metrics.Counter(42)}, + {Name: metrics.KindGauge, Value: metrics.Gauge(42.42)}, } for _, r := range records { @@ -102,8 +102,8 @@ func TestMemStorage_List(t *testing.T) { storage := NewMemStorage() records := []Record{ - {Name: "gauge", Value: metrics.Gauge(42.42)}, - {Name: "counter", Value: metrics.Counter(42)}, + {Name: metrics.KindGauge, Value: metrics.Gauge(42.42)}, + {Name: metrics.KindCounter, Value: metrics.Counter(42)}, } err := storage.Push(records[0].CalculateRecordID(), records[0]) diff --git a/internal/storage/record.go b/internal/storage/record.go index 2cbf6b0..f3102eb 100644 --- a/internal/storage/record.go +++ b/internal/storage/record.go @@ -49,14 +49,14 @@ func (r *Record) UnmarshalJSON(src []byte) error { r.Name = data["name"] switch data["kind"] { - case "counter": + case metrics.KindCounter: value, err := metrics.ToCounter(data["value"]) if err != nil { return fmt.Errorf("record unmarshaling failed: %w", err) } r.Value = value - case "gauge": + case metrics.KindGauge: value, err := metrics.ToGauge(data["value"]) if err != nil { return fmt.Errorf("record unmarshaling failed: %w", err) diff --git a/internal/storage/service.go b/internal/storage/service.go index ad1a63f..12eeee7 100644 --- a/internal/storage/service.go +++ b/internal/storage/service.go @@ -68,7 +68,7 @@ func (s Service) List() ([]Record, error) { } func (s Service) calculateNewValue(record Record) (metrics.Metric, error) { - if record.Value.Kind() != "counter" { + if record.Value.Kind() != metrics.KindCounter { return record.Value, nil } diff --git a/internal/storage/service_test.go b/internal/storage/service_test.go index bbeb5a7..2d72ff8 100644 --- a/internal/storage/service_test.go +++ b/internal/storage/service_test.go @@ -27,7 +27,7 @@ func TestService_Get(t *testing.T) { m.On("Get", "test_counter").Return(Record{Name: "test", Value: metrics.Counter(42)}, nil) }, - args: args{name: "test", kind: "counter"}, + args: args{name: "test", kind: metrics.KindCounter}, expected: Record{Name: "test", Value: metrics.Counter(42)}, wantErr: false, }, @@ -37,7 +37,7 @@ func TestService_Get(t *testing.T) { mock: func(m *StorageMock) { m.On("Get", "test_counter").Return(Record{}, entities.ErrRecordNotFound) }, - args: args{name: "test", kind: "counter"}, + args: args{name: "test", kind: metrics.KindCounter}, expected: Record{}, wantErr: true, }, diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 7474bad..e6f7ee4 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -11,5 +11,5 @@ type MetricsStorage interface { Push(id string, record Record) error Get(id string) (Record, error) List() ([]Record, error) - Kind() string + Close() error } diff --git a/internal/storage/storage_mock.go b/internal/storage/storage_mock.go index 9c40e70..2853eee 100644 --- a/internal/storage/storage_mock.go +++ b/internal/storage/storage_mock.go @@ -31,6 +31,8 @@ func (m *StorageMock) List() ([]Record, error) { return args.Get(0).([]Record), args.Error(1) } -func (m *StorageMock) Kind() string { - return KindMock +func (m *StorageMock) Close() error { + args := m.Called() + + return args.Error(0) } diff --git a/internal/validators/validators.go b/internal/validators/validators.go index 15da45d..ba6916f 100644 --- a/internal/validators/validators.go +++ b/internal/validators/validators.go @@ -4,6 +4,7 @@ import ( "regexp" "github.com/ex0rcist/metflix/internal/entities" + "github.com/ex0rcist/metflix/internal/metrics" ) var nameRegexp = regexp.MustCompile(`^[A-Za-z\d]+$`) @@ -34,7 +35,7 @@ func validateMetricName(name string) error { func validateMetricKind(kind string) error { switch kind { - case "counter", "gauge": + case metrics.KindCounter, metrics.KindGauge: return nil default: diff --git a/internal/validators/validators_test.go b/internal/validators/validators_test.go index 880ff10..6b68ac0 100644 --- a/internal/validators/validators_test.go +++ b/internal/validators/validators_test.go @@ -3,6 +3,7 @@ package validators_test import ( "testing" + "github.com/ex0rcist/metflix/internal/metrics" "github.com/ex0rcist/metflix/internal/validators" ) @@ -16,12 +17,12 @@ func TestValidateMetric(t *testing.T) { args args wantErr bool }{ - {name: "correct counter", args: args{name: "testname", kind: "counter"}, wantErr: false}, - {name: "correct gauge", args: args{name: "testname", kind: "gauge"}, wantErr: false}, + {name: "correct counter", args: args{name: "testname", kind: metrics.KindCounter}, wantErr: false}, + {name: "correct gauge", args: args{name: "testname", kind: metrics.KindGauge}, wantErr: false}, - {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: "name not present", args: args{name: "", kind: metrics.KindCounter}, wantErr: true}, + {name: "incorrect name", args: args{name: "некорректноеимя", kind: metrics.KindCounter}, wantErr: true}, + {name: "incorrect name", args: args{name: "incorrect name", kind: metrics.KindGauge}, wantErr: true}, {name: "incorrect name", args: args{name: "correctname", kind: "incorrectgauge"}, wantErr: true}, {name: "incorrect kind", args: args{kind: "gauger"}, wantErr: true}, } From 09ac0242205d2445e1ebd74ae408ef809e6d9bfb Mon Sep 17 00:00:00 2001 From: Evgenii Shuvalov Date: Fri, 26 Jul 2024 17:57:55 +0300 Subject: [PATCH 15/15] iter9 review fixes --- internal/storage/file.go | 3 --- internal/storage/file_test.go | 2 -- 2 files changed, 5 deletions(-) diff --git a/internal/storage/file.go b/internal/storage/file.go index 6ae2e15..78583f0 100644 --- a/internal/storage/file.go +++ b/internal/storage/file.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "sync" "time" "github.com/ex0rcist/metflix/internal/logging" @@ -16,7 +15,6 @@ var _ MetricsStorage = (*FileStorage)(nil) type FileStorage struct { *MemStorage - sync.Mutex storePath string storeInterval int @@ -60,7 +58,6 @@ func (s *FileStorage) Push(id string, record Record) error { func (s *FileStorage) Close() error { if s.dumpTicker != nil { - fmt.Println("stopping ticker") s.dumpTicker.Stop() } diff --git a/internal/storage/file_test.go b/internal/storage/file_test.go index 948b139..4da3299 100644 --- a/internal/storage/file_test.go +++ b/internal/storage/file_test.go @@ -2,7 +2,6 @@ package storage import ( "encoding/json" - "fmt" "os" "testing" "time" @@ -56,7 +55,6 @@ func TestSyncPushAndDump(t *testing.T) { data, err := os.ReadFile(storePath) checkNoError(t, err, "failed to read storage file") - fmt.Println(string(data)) err = json.Unmarshal(data, &fs.MemStorage) checkNoError(t, err, "failed to unmarshal storage file")