iter9 review fixes

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")

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}
 }

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 {

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
 	}

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,15 +68,15 @@ 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)
 			return
 		}
 
 		mex.Delta = &delta
-	case "gauge":
+	case metrics.KindGauge:
 		value, err := metrics.ToGauge(rawValue)
 		if err != nil {
 			writeErrorResponse(ctx, rw, errToStatus(err), err)

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,15 +283,15 @@ 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"},
 		},
 		{
 			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,

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")
 	}