chore: update golangci-lint to v2.12.2 with canonical config
All checks were successful
check / check (push) Successful in 2m3s
All checks were successful
check / check (push) Successful in 2m3s
Replace .golangci.yml with the canonical v2-schema config (default: all minus six disabled linters, lll 88, tests included) and bump every golangci-lint pin to v2.12.2: - Dockerfile: golangci/golangci-lint:v2.12.2-alpine (hash-pinned) - script/bootstrap: GOLANGCI_LINT_VERSION 2.12.2 with new linux-amd64/arm64 release-archive sha256 pins Fix all 747 findings the stricter config surfaces, with no behavior changes: t.Parallel() throughout the test suite, static sentinel errors and errors.Is comparisons, checked error returns, context propagation (contextcheck/noctx), 88-column wrapping, extracted constants and helpers for goconst/dupl/funlen/cyclop, exhaustive switch cases replicating existing defaults, and white-box test files renamed to *_internal_test.go for testpackage. Three nolint:tagliatelle directives preserve the existing snake_case JSON wire and on-disk metadata formats.
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"embed"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"path/filepath"
|
||||
@@ -29,10 +30,15 @@ const bootstrapVersion = 0
|
||||
// Params defines dependencies for Database.
|
||||
type Params struct {
|
||||
fx.In
|
||||
|
||||
Logger *logger.Logger
|
||||
Config *config.Config
|
||||
}
|
||||
|
||||
// errInvalidMigrationFilename is returned when a migration filename does
|
||||
// not match the "<version>[_<description>].sql" pattern.
|
||||
var errInvalidMigrationFilename = errors.New("invalid migration filename")
|
||||
|
||||
// Database wraps the SQL database connection.
|
||||
type Database struct {
|
||||
db *sql.DB
|
||||
@@ -48,33 +54,31 @@ type Database struct {
|
||||
func ParseMigrationVersion(filename string) (int, error) {
|
||||
name := strings.TrimSuffix(filename, filepath.Ext(filename))
|
||||
if name == "" {
|
||||
return 0, fmt.Errorf("invalid migration filename %q: empty name", filename)
|
||||
return 0, fmt.Errorf("%w %q: empty name", errInvalidMigrationFilename, filename)
|
||||
}
|
||||
|
||||
// Split on underscore to separate version from description.
|
||||
// If there's no underscore, the entire stem is the version.
|
||||
versionStr := name
|
||||
if idx := strings.IndexByte(name, '_'); idx >= 0 {
|
||||
versionStr = name[:idx]
|
||||
}
|
||||
|
||||
versionStr, _, _ := strings.Cut(name, "_")
|
||||
if versionStr == "" {
|
||||
return 0, fmt.Errorf("invalid migration filename %q: empty version prefix", filename)
|
||||
return 0, fmt.Errorf(
|
||||
"%w %q: empty version prefix", errInvalidMigrationFilename, filename,
|
||||
)
|
||||
}
|
||||
|
||||
// Validate the version is purely numeric.
|
||||
for _, ch := range versionStr {
|
||||
if ch < '0' || ch > '9' {
|
||||
return 0, fmt.Errorf(
|
||||
"invalid migration filename %q: version %q contains non-numeric character %q",
|
||||
filename, versionStr, string(ch),
|
||||
"%w %q: version %q contains non-numeric character %q",
|
||||
errInvalidMigrationFilename, filename, versionStr, string(ch),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
version, err := strconv.Atoi(versionStr)
|
||||
if err != nil {
|
||||
return 0, fmt.Errorf("invalid migration filename %q: %w", filename, err)
|
||||
return 0, fmt.Errorf("%w %q: %w", errInvalidMigrationFilename, filename, err)
|
||||
}
|
||||
|
||||
return version, nil
|
||||
@@ -97,6 +101,7 @@ func New(lc fx.Lifecycle, params Params) (*Database, error) {
|
||||
},
|
||||
OnStop: func(_ context.Context) error {
|
||||
s.log.Info("Database OnStop Hook")
|
||||
|
||||
if s.db != nil {
|
||||
return s.db.Close()
|
||||
}
|
||||
@@ -108,30 +113,6 @@ func New(lc fx.Lifecycle, params Params) (*Database, error) {
|
||||
return s, nil
|
||||
}
|
||||
|
||||
func (s *Database) connect(ctx context.Context) error {
|
||||
dbURL := s.config.DBURL
|
||||
|
||||
s.log.Info("connecting to database", "url", dbURL)
|
||||
|
||||
db, err := sql.Open("sqlite", dbURL)
|
||||
if err != nil {
|
||||
s.log.Error("failed to open database", "error", err)
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
if err := db.PingContext(ctx); err != nil {
|
||||
s.log.Error("failed to ping database", "error", err)
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
s.db = db
|
||||
s.log.Info("database connected")
|
||||
|
||||
return ApplyMigrations(ctx, s.db, s.log)
|
||||
}
|
||||
|
||||
// collectMigrations reads the embedded schema directory and returns
|
||||
// migration filenames sorted lexicographically.
|
||||
func collectMigrations() ([]string, error) {
|
||||
@@ -191,7 +172,8 @@ func bootstrapMigrationsTable(ctx context.Context, db *sql.DB, log *slog.Logger)
|
||||
// This is exported so tests can apply the real schema without the full fx
|
||||
// lifecycle.
|
||||
func ApplyMigrations(ctx context.Context, db *sql.DB, log *slog.Logger) error {
|
||||
if err := bootstrapMigrationsTable(ctx, db, log); err != nil {
|
||||
err := bootstrapMigrationsTable(ctx, db, log)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -261,3 +243,28 @@ func ApplyMigrations(ctx context.Context, db *sql.DB, log *slog.Logger) error {
|
||||
func (s *Database) DB() *sql.DB {
|
||||
return s.db
|
||||
}
|
||||
|
||||
func (s *Database) connect(ctx context.Context) error {
|
||||
dbURL := s.config.DBURL
|
||||
|
||||
s.log.Info("connecting to database", "url", dbURL)
|
||||
|
||||
db, err := sql.Open("sqlite", dbURL)
|
||||
if err != nil {
|
||||
s.log.Error("failed to open database", "error", err)
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
err = db.PingContext(ctx)
|
||||
if err != nil {
|
||||
s.log.Error("failed to ping database", "error", err)
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
s.db = db
|
||||
s.log.Info("database connected")
|
||||
|
||||
return ApplyMigrations(ctx, s.db, s.log)
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
package database
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"testing"
|
||||
|
||||
@@ -17,12 +16,14 @@ func openTestDB(t *testing.T) *sql.DB {
|
||||
t.Fatalf("failed to open test db: %v", err)
|
||||
}
|
||||
|
||||
t.Cleanup(func() { db.Close() })
|
||||
t.Cleanup(func() { _ = db.Close() })
|
||||
|
||||
return db
|
||||
}
|
||||
|
||||
func TestParseMigrationVersion(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
filename string
|
||||
@@ -78,6 +79,8 @@ func TestParseMigrationVersion(t *testing.T) {
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
got, err := ParseMigrationVersion(tt.filename)
|
||||
if tt.wantErr {
|
||||
if err == nil {
|
||||
@@ -101,37 +104,50 @@ func TestParseMigrationVersion(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestApplyMigrations_CreatesSchemaAndTables(t *testing.T) {
|
||||
db := openTestDB(t)
|
||||
ctx := context.Background()
|
||||
t.Parallel()
|
||||
|
||||
if err := ApplyMigrations(ctx, db, nil); err != nil {
|
||||
db := openTestDB(t)
|
||||
ctx := t.Context()
|
||||
|
||||
err := ApplyMigrations(ctx, db, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyMigrations failed: %v", err)
|
||||
}
|
||||
|
||||
// The schema_migrations table must exist and contain at least
|
||||
// version 0 (the bootstrap) and 1 (the initial schema).
|
||||
rows, err := db.Query("SELECT version FROM schema_migrations ORDER BY version")
|
||||
rows, err := db.QueryContext(
|
||||
ctx, "SELECT version FROM schema_migrations ORDER BY version",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to query schema_migrations: %v", err)
|
||||
}
|
||||
defer rows.Close()
|
||||
|
||||
defer func() { _ = rows.Close() }()
|
||||
|
||||
var versions []int
|
||||
|
||||
for rows.Next() {
|
||||
var v int
|
||||
if err := rows.Scan(&v); err != nil {
|
||||
t.Fatalf("failed to scan version: %v", err)
|
||||
|
||||
scanErr := rows.Scan(&v)
|
||||
if scanErr != nil {
|
||||
t.Fatalf("failed to scan version: %v", scanErr)
|
||||
}
|
||||
|
||||
versions = append(versions, v)
|
||||
}
|
||||
|
||||
if err := rows.Err(); err != nil {
|
||||
err = rows.Err()
|
||||
if err != nil {
|
||||
t.Fatalf("row iteration error: %v", err)
|
||||
}
|
||||
|
||||
if len(versions) < 2 {
|
||||
t.Fatalf("expected at least 2 migrations recorded, got %d: %v", len(versions), versions)
|
||||
t.Fatalf(
|
||||
"expected at least 2 migrations recorded, got %d: %v",
|
||||
len(versions), versions,
|
||||
)
|
||||
}
|
||||
|
||||
if versions[0] != 0 {
|
||||
@@ -143,10 +159,15 @@ func TestApplyMigrations_CreatesSchemaAndTables(t *testing.T) {
|
||||
}
|
||||
|
||||
// Verify that the application tables created by 001.sql exist.
|
||||
for _, table := range []string{"source_content", "source_metadata", "output_content", "request_cache", "negative_cache", "cache_stats"} {
|
||||
tables := []string{
|
||||
"source_content", "source_metadata", "output_content",
|
||||
"request_cache", "negative_cache", "cache_stats",
|
||||
}
|
||||
for _, table := range tables {
|
||||
var count int
|
||||
|
||||
err := db.QueryRow(
|
||||
err := db.QueryRowContext(
|
||||
ctx,
|
||||
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name=?",
|
||||
table,
|
||||
).Scan(&count)
|
||||
@@ -161,22 +182,28 @@ func TestApplyMigrations_CreatesSchemaAndTables(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestApplyMigrations_Idempotent(t *testing.T) {
|
||||
db := openTestDB(t)
|
||||
ctx := context.Background()
|
||||
t.Parallel()
|
||||
|
||||
if err := ApplyMigrations(ctx, db, nil); err != nil {
|
||||
db := openTestDB(t)
|
||||
ctx := t.Context()
|
||||
|
||||
err := ApplyMigrations(ctx, db, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("first ApplyMigrations failed: %v", err)
|
||||
}
|
||||
|
||||
// Running a second time must succeed without errors.
|
||||
if err := ApplyMigrations(ctx, db, nil); err != nil {
|
||||
err = ApplyMigrations(ctx, db, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("second ApplyMigrations failed: %v", err)
|
||||
}
|
||||
|
||||
// Verify no duplicate rows in schema_migrations.
|
||||
var count int
|
||||
|
||||
err := db.QueryRow("SELECT COUNT(*) FROM schema_migrations WHERE version = 0").Scan(&count)
|
||||
err = db.QueryRowContext(
|
||||
ctx, "SELECT COUNT(*) FROM schema_migrations WHERE version = 0",
|
||||
).Scan(&count)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to count version 0 rows: %v", err)
|
||||
}
|
||||
@@ -187,17 +214,21 @@ func TestApplyMigrations_Idempotent(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) {
|
||||
db := openTestDB(t)
|
||||
ctx := context.Background()
|
||||
t.Parallel()
|
||||
|
||||
if err := bootstrapMigrationsTable(ctx, db, nil); err != nil {
|
||||
db := openTestDB(t)
|
||||
ctx := t.Context()
|
||||
|
||||
err := bootstrapMigrationsTable(ctx, db, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("bootstrapMigrationsTable failed: %v", err)
|
||||
}
|
||||
|
||||
// schema_migrations table must exist.
|
||||
var tableCount int
|
||||
|
||||
err := db.QueryRow(
|
||||
err = db.QueryRowContext(
|
||||
ctx,
|
||||
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='schema_migrations'",
|
||||
).Scan(&tableCount)
|
||||
if err != nil {
|
||||
@@ -211,8 +242,8 @@ func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) {
|
||||
// Version 0 must be recorded.
|
||||
var recorded int
|
||||
|
||||
err = db.QueryRow(
|
||||
"SELECT COUNT(*) FROM schema_migrations WHERE version = 0",
|
||||
err = db.QueryRowContext(
|
||||
ctx, "SELECT COUNT(*) FROM schema_migrations WHERE version = 0",
|
||||
).Scan(&recorded)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to check version: %v", err)
|
||||
Reference in New Issue
Block a user