From b8ab348095e73ae595b5756aadc07e1298463abd Mon Sep 17 00:00:00 2001
From: Daniel <dhaavi@users.noreply.github.com>
Date: Tue, 4 Mar 2025 15:25:44 +0100
Subject: [PATCH] Fix tests and linters
---
.github/workflows/go.yml | 2 +-
.golangci.yml | 8 ++---
Earthfile | 2 +-
base/database/storage/sqlite/schema.go | 10 +-----
base/database/storage/sqlite/sqlite.go | 38 ++++++++++++++++-----
base/database/storage/sqlite/sqlite_test.go | 17 +++------
go.mod | 2 +-
service/intel/geoip/init_test.go | 2 ++
8 files changed, 41 insertions(+), 40 deletions(-)
diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index 16447305..7d94e54a 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -40,7 +40,7 @@ jobs:
- name: Run golangci-lint
uses: golangci/golangci-lint-action@v6
with:
- version: v1.64.5
+ version: v1.64.6
only-new-issues: true
args: -c ./.golangci.yml --timeout 15m
diff --git a/.golangci.yml b/.golangci.yml
index f01395b8..3811a25e 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -8,7 +8,6 @@ linters:
- contextcheck
- cyclop
- depguard
- - exhaustivestruct
- exhaustruct
- forbidigo
- funlen
@@ -16,12 +15,8 @@ linters:
- gochecknoinits
- gocognit
- gocyclo
- - goerr113
- - gomnd
- gomoddirectives
- - ifshort
- interfacebloat
- - interfacer
- ireturn
- lll
- mnd
@@ -32,7 +27,6 @@ linters:
- noctx
- nolintlint
- nonamedreturns
- - nosnakecase
- perfsprint # TODO(ppacher): we should re-enanble this one to avoid costly fmt.* calls in the hot-path
- revive
- tagliatelle
@@ -42,6 +36,8 @@ linters:
- whitespace
- wrapcheck
- wsl
+ - gci
+ - tenv # Deprecated
linters-settings:
revive:
diff --git a/Earthfile b/Earthfile
index 6ac59502..67a7759b 100644
--- a/Earthfile
+++ b/Earthfile
@@ -3,7 +3,7 @@ VERSION --arg-scope-and-set --global-cache 0.8
ARG --global go_version = 1.24
ARG --global node_version = 18
ARG --global rust_version = 1.79
-ARG --global golangci_lint_version = 1.57.1
+ARG --global golangci_lint_version = 1.64.6
ARG --global go_builder_image = "golang:${go_version}-alpine"
ARG --global node_builder_image = "node:${node_version}"
diff --git a/base/database/storage/sqlite/schema.go b/base/database/storage/sqlite/schema.go
index c87402f3..fcf87bcd 100644
--- a/base/database/storage/sqlite/schema.go
+++ b/base/database/storage/sqlite/schema.go
@@ -19,6 +19,7 @@ import (
"embed"
migrate "github.com/rubenv/sql-migrate"
+
"github.com/safing/portmaster/base/database/record"
"github.com/safing/portmaster/base/database/storage/sqlite/models"
)
@@ -48,12 +49,3 @@ func getMeta(r *models.Record) *record.Meta {
}
return meta
}
-
-func setMeta(r *models.Record, m *record.Meta) {
- r.Created = m.Created
- r.Modified = m.Modified
- r.Expires = m.Expires
- r.Deleted = m.Deleted
- r.Secret = m.IsSecret()
- r.Crownjewel = m.IsCrownJewel()
-}
diff --git a/base/database/storage/sqlite/sqlite.go b/base/database/storage/sqlite/sqlite.go
index 325da138..454870ad 100644
--- a/base/database/storage/sqlite/sqlite.go
+++ b/base/database/storage/sqlite/sqlite.go
@@ -29,6 +29,11 @@ import (
"github.com/safing/structures/dsd"
)
+// Errors.
+var (
+ ErrQueryTimeout = errors.New("query timeout")
+)
+
// SQLite storage.
type SQLite struct {
name string
@@ -108,7 +113,7 @@ func (db *SQLite) Get(key string) (record.Record, error) {
// Get record from database.
r, err := models.FindRecord(db.ctx, db.bob, key)
if err != nil {
- return nil, fmt.Errorf("%w: %s", storage.ErrNotFound, err)
+ return nil, fmt.Errorf("%w: %w", storage.ErrNotFound, err)
}
// Return data in wrapper.
@@ -116,7 +121,7 @@ func (db *SQLite) Get(key string) (record.Record, error) {
db.name,
key,
getMeta(r),
- uint8(r.Format.GetOrZero()),
+ uint8(r.Format.GetOrZero()), //nolint:gosec // Values are within uint8.
r.Value.GetOrZero(),
)
}
@@ -285,7 +290,9 @@ func (db *SQLite) queryExecutor(queryIter *iterator.Iterator, q *query.Query, lo
queryIter.Finish(err)
return
}
- defer cursor.Close()
+ defer func() {
+ _ = cursor.Close()
+ }()
recordsLoop:
for cursor.Next() {
@@ -326,7 +333,7 @@ recordsLoop:
db.name,
r.Key,
m,
- uint8(r.Format.GetOrZero()),
+ uint8(r.Format.GetOrZero()), //nolint:gosec // Values are within uint8.
r.Value.GetOrZero(),
)
@@ -340,7 +347,7 @@ recordsLoop:
break recordsLoop
case queryIter.Next <- matched:
case <-time.After(1 * time.Second):
- err = errors.New("query timeout")
+ err = ErrQueryTimeout
break recordsLoop
}
}
@@ -435,7 +442,7 @@ func (db *SQLite) MaintainRecordStates(ctx context.Context, purgeDeletedBefore t
// Option 1: Using shadow delete.
if shadowDelete {
// Mark expired records as deleted.
- models.Records.Update(
+ _, err := models.Records.Update(
um.SetCol("format").ToArg(nil),
um.SetCol("value").ToArg(nil),
um.SetCol("deleted").ToArg(now),
@@ -443,26 +450,39 @@ func (db *SQLite) MaintainRecordStates(ctx context.Context, purgeDeletedBefore t
models.UpdateWhere.Records.Expires.GT(0),
models.UpdateWhere.Records.Expires.LT(now),
).Exec(db.ctx, db.bob)
+ if err != nil {
+ return fmt.Errorf("failed to shadow delete expired records: %w", err)
+ }
// Purge deleted records before threshold.
- models.Records.Delete(
+ _, err = models.Records.Delete(
models.DeleteWhere.Records.Deleted.GT(0),
models.DeleteWhere.Records.Deleted.LT(purgeThreshold),
).Exec(db.ctx, db.bob)
+ if err != nil {
+ return fmt.Errorf("failed to purge deleted records (before threshold): %w", err)
+ }
return nil
}
// Option 2: Immediate delete.
// Delete expired record.
- models.Records.Delete(
+ _, err := models.Records.Delete(
models.DeleteWhere.Records.Expires.GT(0),
models.DeleteWhere.Records.Expires.LT(now),
).Exec(db.ctx, db.bob)
+ if err != nil {
+ return fmt.Errorf("failed to delete expired records: %w", err)
+ }
+
// Delete shadow deleted records.
- models.Records.Delete(
+ _, err = models.Records.Delete(
models.DeleteWhere.Records.Deleted.GT(0),
).Exec(db.ctx, db.bob)
+ if err != nil {
+ return fmt.Errorf("failed to purge deleted records: %w", err)
+ }
return nil
}
diff --git a/base/database/storage/sqlite/sqlite_test.go b/base/database/storage/sqlite/sqlite_test.go
index 19bb36a0..799a230b 100644
--- a/base/database/storage/sqlite/sqlite_test.go
+++ b/base/database/storage/sqlite/sqlite_test.go
@@ -1,8 +1,6 @@
package sqlite
import (
- "context"
- "os"
"sync"
"testing"
"time"
@@ -43,15 +41,8 @@ type TestRecord struct { //nolint:maligned
func TestSQLite(t *testing.T) {
t.Parallel()
- testDir, err := os.MkdirTemp("", "testing-")
- if err != nil {
- t.Fatal(err)
- }
- defer func() {
- _ = os.RemoveAll(testDir) // clean up
- }()
-
// start
+ testDir := t.TempDir()
db, err := openSQLite("test", testDir, true)
if err != nil {
t.Fatal(err)
@@ -160,19 +151,19 @@ func TestSQLite(t *testing.T) {
}
// maintenance
- err = db.MaintainRecordStates(context.TODO(), time.Now().Add(-time.Minute), true)
+ err = db.MaintainRecordStates(t.Context(), time.Now().Add(-time.Minute), true)
if err != nil {
t.Fatal(err)
}
// maintenance
- err = db.MaintainRecordStates(context.TODO(), time.Now(), false)
+ err = db.MaintainRecordStates(t.Context(), time.Now(), false)
if err != nil {
t.Fatal(err)
}
// purging
- n, err := db.Purge(context.TODO(), query.New("test:path/to/").MustBeValid(), true, true, true)
+ n, err := db.Purge(t.Context(), query.New("test:path/to/").MustBeValid(), true, true, true)
if err != nil {
t.Fatal(err)
}
diff --git a/go.mod b/go.mod
index 2ce4d421..cd1831f9 100644
--- a/go.mod
+++ b/go.mod
@@ -54,6 +54,7 @@ require (
github.com/safing/structures v1.2.0
github.com/seehuhn/fortuna v1.0.1
github.com/shirou/gopsutil v3.21.11+incompatible
+ github.com/simukti/sqldb-logger v0.0.0-20230108155151-646c1a075551
github.com/spf13/cobra v1.8.1
github.com/spkg/zipfs v0.7.1
github.com/stephenafamo/bob v0.30.0
@@ -145,7 +146,6 @@ require (
github.com/satori/go.uuid v1.2.0 // indirect
github.com/seehuhn/sha256d v1.0.0 // indirect
github.com/shopspring/decimal v1.3.1 // indirect
- github.com/simukti/sqldb-logger v0.0.0-20230108155151-646c1a075551 // indirect
github.com/spf13/cast v1.5.0 // indirect
github.com/spf13/pflag v1.0.5 // indirect
github.com/stephenafamo/scan v0.6.1 // indirect
diff --git a/service/intel/geoip/init_test.go b/service/intel/geoip/init_test.go
index b6d722dc..c4d87b33 100644
--- a/service/intel/geoip/init_test.go
+++ b/service/intel/geoip/init_test.go
@@ -8,6 +8,7 @@ import (
"github.com/safing/portmaster/base/api"
"github.com/safing/portmaster/base/config"
"github.com/safing/portmaster/base/database/dbmodule"
+ "github.com/safing/portmaster/base/dataroot"
"github.com/safing/portmaster/base/notifications"
"github.com/safing/portmaster/service/updates"
)
@@ -56,6 +57,7 @@ func runTest(m *testing.M) error {
defer func() { _ = os.RemoveAll(ds) }()
stub := &testInstance{}
+ dbmodule.SetDatabaseLocation(dataroot.Root())
stub.db, err = dbmodule.New(stub)
if err != nil {
return fmt.Errorf("failed to create database: %w", err)