stores: address review comments

SiaFoundation · Oct 17, 2023 · fac9ee7 · fac9ee7
1 parent 49bcb39
commit fac9ee7
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 48 deletions.
diff --git a/api/bus.go b/api/bus.go
@@ -573,8 +573,8 @@ type (
 	}
 
 	ContractSetMetricsQueryOpts struct {
-		Name   *string
-		After  *time.Time
-		Before *time.Time
+		Name   string
+		After  time.Time
+		Before time.Time
 	}
 )
diff --git a/stores/metrics.go b/stores/metrics.go
@@ -81,20 +81,20 @@ func (dbContractSetMetric) TableName() string      { return "contract_sets" }
 func (dbContractSetChurnMetric) TableName() string { return "contract_sets_churn" }
 func (dbPerformanceMetric) TableName() string      { return "performance" }
 
-func scopeTimeRange(tx *gorm.DB, after, before *time.Time) *gorm.DB {
-	if after != nil {
-		tx = tx.Where("time > ?", unixTimeMS(*after))
+func scopeTimeRange(tx *gorm.DB, after, before time.Time) *gorm.DB {
+	if after != (time.Time{}) {
+		tx = tx.Where("time > ?", unixTimeMS(after))
 	}
-	if before != nil {
-		tx = tx.Where("time <= ?", unixTimeMS(*before))
+	if before != (time.Time{}) {
+		tx = tx.Where("time <= ?", unixTimeMS(before))
 	}
 	return tx
 }
 
 func (s *SQLStore) contractSetMetrics(ctx context.Context, opts api.ContractSetMetricsQueryOpts) ([]dbContractSetMetric, error) {
 	tx := s.dbMetrics
-	if opts.Name != nil {
-		tx = tx.Where("name = ?", *opts.Name)
+	if opts.Name != "" {
+		tx = tx.Where("name = ?", opts.Name)
 	}
 	var metrics []dbContractSetMetric
 	err := tx.Scopes(func(tx *gorm.DB) *gorm.DB {

diff --git a/stores/metrics_test.go b/stores/metrics_test.go
@@ -2,7 +2,6 @@ package stores
 
 import (
 	"context"
-	"fmt"
 	"sort"
 	"testing"
 	"time"
@@ -39,49 +38,34 @@ func TestContractSetMetrics(t *testing.T) {
 		}
 	}
 
-	// Check that we have 4 metrics now.
-	metrics, err = ss.ContractSetMetrics(context.Background(), api.ContractSetMetricsQueryOpts{})
-	if err != nil {
-		t.Fatal(err)
-	} else if len(metrics) != 4 {
-		t.Fatalf("expected 4 metrics, got %v", len(metrics))
-	} else if !sort.SliceIsSorted(metrics, func(i, j int) bool {
-		return time.Time(metrics[i].Time).Before(time.Time(metrics[j].Time))
-	}) {
-		for _, m := range metrics {
-			fmt.Println(time.Time(m.Time).UnixMilli())
+	assertMetrics := func(opts api.ContractSetMetricsQueryOpts, expected int, contracts []int) {
+		t.Helper()
+		metrics, err := ss.ContractSetMetrics(context.Background(), opts)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(metrics) != expected {
+			t.Fatalf("expected %v metrics, got %v", expected, len(metrics))
+		} else if !sort.SliceIsSorted(metrics, func(i, j int) bool {
+			return time.Time(metrics[i].Time).Before(time.Time(metrics[j].Time))
+		}) {
+			t.Fatal("expected metrics to be sorted by time")
+		}
+		for i, m := range metrics {
+			if m.Contracts != contracts[i] {
+				t.Fatalf("expected %v contracts, got %v", contracts[i], m.Contracts)
+			}
 		}
-		t.Fatal("expected metrics to be sorted by time")
 	}
 
+	// Check that we have 4 metrics now.
+	assertMetrics(api.ContractSetMetricsQueryOpts{}, 4, []int{2, 3, 1, 0})
+
 	// Query all metrics by contract set.
-	metrics, err = ss.ContractSetMetrics(context.Background(), api.ContractSetMetricsQueryOpts{
-		Name: &cs,
-	})
-	if err != nil {
-		t.Fatal(err)
-	} else if len(metrics) != 3 {
-		t.Fatalf("expected 3 metrics, got %v", len(metrics))
-	} else if metrics[0].Contracts != 2 {
-		t.Fatalf("expected 2 contracts, got %v", metrics[0].Contracts)
-	} else if metrics[1].Contracts != 3 {
-		t.Fatalf("expected 3 contracts, got %v", metrics[1].Contracts)
-	} else if metrics[2].Contracts != 1 {
-		t.Fatalf("expected 1 contracts, got %v", metrics[2].Contracts)
-	}
+	assertMetrics(api.ContractSetMetricsQueryOpts{Name: cs}, 3, []int{2, 3, 1})
 
 	// Query the metric in the middle of the 3 we added.
 	after := time.UnixMilli(1)  // 'after' is exclusive
 	before := time.UnixMilli(2) // 'before' is inclusive
-	metrics, err = ss.ContractSetMetrics(context.Background(), api.ContractSetMetricsQueryOpts{
-		After:  &after,
-		Before: &before,
-	})
-	if err != nil {
-		t.Fatal(err)
-	} else if len(metrics) != 1 {
-		t.Fatalf("expected 1 metrics, got %v", len(metrics))
-	} else if metrics[0].Contracts != 3 {
-		t.Fatalf("expected 3 contracts, got %v", metrics[1].Contracts)
-	}
+	assertMetrics(api.ContractSetMetricsQueryOpts{After: after, Before: before}, 1, []int{3})
 }