[github&storage] GitHub Actions to test compile things & safer DB que…

…ries. (#10)

.github/workflows/pull_request.yaml

@@ -0,0 +1,33 @@
+name: Pull Request Test
+
+on:
+  pull_request:
+    branches:
+      - main
+
+permissions:
+  contents: read
+
+jobs:
+  test:
+    name: Test compile golang and run go tests
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: "1.23"
+          cache: false
+
+      - name: Install Dependencies
+        run: go mod tidy
+
+      - name: Build source
+        run: go build main.go
+
+      - name: Unit tests
+        run: go test -v ./...

pkg/stats/top.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"strconv"
-	"strings"
 	"time"
 
 	"github.com/jylitalo/mystats/pkg/telemetry"
@@ -15,26 +14,33 @@ func Top(ctx context.Context, db Storage, measure, period string, types, workout
 	_, span := telemetry.NewSpan(ctx, "stats.Top")
 	defer span.End()
 
-	modifier := float64(1)
-	unit := "%4.0fm"
-	switch {
-	case strings.Contains(measure, "distance") && !strings.Contains(measure, "count"):
-		modifier = 1000
-		unit = "%4.1fkm"
-	case strings.Contains(measure, "time") && !strings.Contains(measure, "count"):
-		modifier = 3600
+	var m, unit, p string
+	switch measure {
+	case "time":
+		m = "sum(elapsedtime)/3600"
 		unit = "%4.1fh"
+	case "distance":
+		m = "sum(distance)/1000"
+		unit = "%4.1fkm"
+	case "elevation":
+		m = "sum(elevation)"
+		unit = "%4.0fm"
 	}
-	if measure == "time" {
-		measure = "elapsedtime"
+	switch period {
+	case "month":
+		p = "month"
+	case "week":
+		p = "week"
+	case "day":
+		p = "day"
 	}
 	results := [][]string{}
 	rows, err := db.QuerySummary(
-		[]string{"sum(" + measure + ") as total", "year", period},
+		[]string{m + " as total", "year", p},
 		storage.SummaryConditions{Types: types, WorkoutTypes: workoutTypes, Years: years},
 		&storage.Order{
-			GroupBy: []string{"year", period},
-			OrderBy: []string{"total desc", "year desc", period + " desc"},
+			GroupBy: []string{"year", p},
+			OrderBy: []string{"total desc", "year desc", p + " desc"},
 			Limit:   limit},
 	)
 	if err != nil {
@@ -47,14 +53,14 @@ func Top(ctx context.Context, db Storage, measure, period string, types, workout
 		if err = rows.Scan(&measureValue, &year, &periodValue); err != nil {
 			return nil, nil, telemetry.Error(span, err)
 		}
-		value := fmt.Sprintf(unit, measureValue/modifier)
+		value := fmt.Sprintf(unit, measureValue)
 		periodStr := strconv.FormatInt(int64(periodValue), 10)
-		if period == "month" {
+		if p == "month" {
 			periodStr = time.Month(periodValue).String()
 		}
 		results = append(
 			results, []string{value, strconv.FormatInt(int64(year), 10), periodStr},
 		)
 	}
-	return []string{measure, "year", period}, results, nil
+	return []string{measure, "year", p}, results, nil
 }

pkg/telemetry/telemetry.go

@@ -2,6 +2,7 @@ package telemetry
 
 import (
 	"context"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -86,6 +87,10 @@ func Setup(ctx context.Context, name string) (context.Context, *sdktrace.TracerP
 }
 
 func NewSpan(ctx context.Context, name string) (context.Context, trace.Span) {
+	value := ctx.Value(otelCtxKey)
+	if value == nil {
+		log.Fatal("Telemetry has not been setup")
+	}
 	return ctx.Value(otelCtxKey).(trace.Tracer).Start(ctx, name)
 }
 

server/plot.go

@@ -123,6 +123,10 @@ func (p *PlotPage) render(
 			foundYears = append(foundYears, year)
 		}
 	}
+	if len(foundYears) == 0 {
+		slog.Error("No years found in plot.render()")
+		return nil
+	}
 	refTime, err := time.Parse(time.DateOnly, fmt.Sprintf("%d-01-01", slices.Max(foundYears)))
 	if err != nil {
 		return err
@@ -193,6 +197,9 @@ func scan(rows *sql.Rows, years []int) (numbers, error) {
 		ys[year] = []float64{}
 	}
 	xmax := 0
+	if rows == nil {
+		return ys, nil
+	}
 	for rows.Next() {
 		var year, month, day int
 		var value float64
@@ -239,18 +246,24 @@ func getNumbers(
 	if err != nil {
 		return nil, err
 	}
-	m := "sum(" + measure + ")"
+	var m string
 	switch measure {
 	case "time":
 		m = "sum(elapsedtime)/3600"
 	case "distance":
 		m = "sum(distance)/1000"
+	case "elevation":
+		m = "sum(elevation)"
 	}
 	o := []string{"year", "month", "day"}
 	rows, err := db.QuerySummary(append(o, m), cond, &storage.Order{GroupBy: o, OrderBy: o})
 	if err != nil {
 		return nil, fmt.Errorf("select caused: %w", err)
 	}
-	defer rows.Close()
+	defer func() {
+		if rows != nil {
+			rows.Close()
+		}
+	}()
 	return scan(rows, years)
 }

server/server_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/jylitalo/mystats/pkg/stats"
+	"github.com/jylitalo/mystats/pkg/telemetry"
 	"github.com/jylitalo/mystats/storage"
 )
 
@@ -49,7 +50,8 @@ func TestTemplateRender(t *testing.T) {
 	) {
 		return nil, nil, nil, nil
 	}
-	err := p.Plot.render(context.TODO(), &testDB{}, map[string]bool{"Run": true}, nil, 6, 12, map[int]bool{2024: true}, "month")
+	ctx, _, _ := telemetry.Setup(context.TODO(), "test")
+	err := p.Plot.render(ctx, &testDB{}, map[string]bool{"Run": true}, nil, 6, 12, map[int]bool{2024: true}, "month")
 	if err != nil {
 		t.Error(err)
 	}

storage/storage.go

@@ -217,39 +217,46 @@ func (sq *Sqlite3) InsertSplit(ctx context.Context, records []SplitRecord) error
 	return telemetry.Error(span, tx.Commit())
 }
 
-func sqlQuery(tables []string, fields []string, cond conditions, order *Order) string {
+func sqlQuery(tables []string, fields []string, cond conditions, order *Order) (string, []interface{}) {
 	where := []string{}
+	args := []string{}
 	if len(tables) > 0 {
 		for _, table := range tables[1:] {
 			where = append(where, fmt.Sprintf("%s.StravaID=%s.StravaID", tables[0], table))
 		}
 	}
 	if len(cond.WorkoutTypes) > 0 {
-		where = append(where, "(workouttype='"+strings.Join(cond.WorkoutTypes, "' or workouttype='")+"')")
+		where = append(where, "(workouttype="+strings.Repeat("? or workouttype=", len(cond.WorkoutTypes)-1)+"?)")
+		args = append(args, cond.WorkoutTypes...)
 	}
 	if len(cond.Types) > 0 {
-		where = append(where, "(type='"+strings.Join(cond.Types, "' or type='")+"')")
+		where = append(where, "(type="+strings.Repeat("? or type=", len(cond.Types)-1)+"?)")
+		args = append(args, cond.Types...)
 	}
 	if cond.Month > 0 && cond.Day > 0 {
-		where = append(where, fmt.Sprintf("(month < %d or (month=%d and day<=%d))", cond.Month, cond.Month, cond.Day))
+		where = append(where, "(month < ? or (month=? and day<=?))")
+		month := strconv.Itoa(cond.Month)
+		args = append(args, month, month, strconv.Itoa(cond.Day))
 	}
 	if len(cond.Years) > 0 {
-		yearStr := []string{}
+		where = append(where, "(year="+strings.Repeat("? or year=", len(cond.Years)-1)+"?)")
 		for _, y := range cond.Years {
-			yearStr = append(yearStr, strconv.Itoa(y))
+			args = append(args, strconv.Itoa(y))
 		}
-		where = append(where, "(year="+strings.Join(yearStr, " or year=")+")")
 	}
 	if cond.BEName != "" {
-		where = append(where, "besteffort.name='"+cond.BEName+"'")
+		where = append(where, "besteffort.name=?")
+		args = append(args, cond.BEName)
 	}
 	if cond.StravaID > 0 {
 		for _, t := range tables {
-			where = append(where, fmt.Sprintf("%s.stravaid=%d", t, cond.StravaID))
+			where = append(where, t+".stravaid=")
+			args = append(args, strconv.FormatInt(cond.StravaID, 10))
 		}
 	}
 	if cond.Name != "" {
-		where = append(where, fmt.Sprintf("summary.name LIKE '%s'", cond.Name))
+		where = append(where, "summary.name LIKE ?")
+		args = append(args, cond.Name)
 	}
 	condition := ""
 	if len(where) > 0 {
@@ -267,19 +274,23 @@ func sqlQuery(tables []string, fields []string, cond conditions, order *Order) s
 			sorting += " limit " + strconv.FormatInt(int64(order.Limit), 10)
 		}
 	}
+	ifArgs := make([]interface{}, len(args))
+	for i, v := range args {
+		ifArgs[i] = v
+	}
 	return fmt.Sprintf(
 		"select %s from %s%s%s", strings.Join(fields, ","), strings.Join(tables, ","),
 		condition, sorting,
-	)
+	), ifArgs
 }
 
 func (sq *Sqlite3) QueryBestEffort(fields []string, name string, order *Order) (*sql.Rows, error) {
 	if sq.db == nil {
 		return nil, errors.New("database is nil")
 	}
-	query := sqlQuery([]string{"besteffort", "summary"}, fields, conditions{BEName: name}, order)
+	query, values := sqlQuery([]string{"besteffort", "summary"}, fields, conditions{BEName: name}, order)
 	// slog.Info("storage.Query", "query", query)
-	rows, err := sq.db.Query(query)
+	rows, err := sq.db.Query(query, values...)
 	if err != nil {
 		return nil, fmt.Errorf("%s failed: %w", query, err)
 	}
@@ -290,12 +301,12 @@ func (sq *Sqlite3) QueryBestEffortDistances() ([]string, error) {
 	if sq.db == nil {
 		return nil, errors.New("database is nil")
 	}
-	query := sqlQuery(
+	query, values := sqlQuery(
 		[]string{"besteffort"}, []string{"distinct(name)"}, conditions{},
 		&Order{OrderBy: []string{"distance desc"}},
 	)
 	// slog.Info("storage.Query", "query", query)
-	rows, err := sq.db.Query(query)
+	rows, err := sq.db.Query(query, values...)
 	if err != nil {
 		return nil, fmt.Errorf("%s failed: %w", query, err)
 	}
@@ -316,9 +327,11 @@ func (sq *Sqlite3) QuerySplit(fields []string, id int64) (*sql.Rows, error) {
 	if sq.db == nil {
 		return nil, errors.New("database is nil")
 	}
-	query := sqlQuery([]string{"split"}, fields, conditions{StravaID: id}, &Order{OrderBy: []string{"split"}})
+	query, values := sqlQuery(
+		[]string{"split"}, fields, conditions{StravaID: id}, &Order{OrderBy: []string{"split"}},
+	)
 	// slog.Info("storage.Query", "query", query)
-	rows, err := sq.db.Query(query)
+	rows, err := sq.db.Query(query, values...)
 	if err != nil {
 		return nil, fmt.Errorf("%s failed: %w", query, err)
 	}
@@ -329,7 +342,7 @@ func (sq *Sqlite3) QuerySummary(fields []string, cond SummaryConditions, order *
 	if sq.db == nil {
 		return nil, errors.New("database is nil")
 	}
-	query := sqlQuery(
+	query, values := sqlQuery(
 		[]string{"summary"}, fields,
 		conditions{
 			Types: cond.Types, WorkoutTypes: cond.WorkoutTypes,
@@ -339,7 +352,7 @@ func (sq *Sqlite3) QuerySummary(fields []string, cond SummaryConditions, order *
 		order,
 	)
 	// slog.Info("storage.Query", "query", query)
-	rows, err := sq.db.Query(query)
+	rows, err := sq.db.Query(query, values...)
 	if err != nil {
 		return nil, fmt.Errorf("%s failed: %w", query, err)
 	}