Report a prune count that could not be read as unknown, not 0 (closes #96)
check / check (pull_request) Successful in 3m36s
check / check (pull_request) Successful in 3m36s
PruneDatabase read seven table counts with the error discarded, so a query that could not run silently became 0 and the before/after delta computed from it looked like real work. Each read now goes through a helper that logs at warn on failure and returns nil; nil renders as "unknown", never "0", so an empty table is distinguishable from one that could not be queried. Deltas built from an unknown count are themselves unknown. The failure is surfaced, not propagated, so the command's failure conditions are unchanged. These counts have no --json output — under --json the summary is suppressed entirely — so nothing there can show a false 0. Model: opus-4-8
This commit is contained in:
@@ -0,0 +1,79 @@
|
||||
package vaultik //nolint:testpackage // exercises unexported count helpers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/vaultik/internal/database"
|
||||
"sneak.berlin/go/vaultik/internal/log"
|
||||
)
|
||||
|
||||
// TestTableCountForReportSurfacesReadFailure is the regression guard for
|
||||
// the discarded-error bug: getTableCount for a table its query cannot
|
||||
// resolve must not silently become 0. A count that could not be read is
|
||||
// reported as unknown, which a reader can tell apart from an empty table.
|
||||
//
|
||||
//nolint:paralleltest // installs the global logger via log.Initialize
|
||||
func TestTableCountForReportSurfacesReadFailure(t *testing.T) {
|
||||
log.Initialize(log.Config{})
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
db, err := database.New(ctx, ":memory:")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() { _ = db.Close() })
|
||||
|
||||
v := &Vaultik{DB: db}
|
||||
v.SetContext(ctx)
|
||||
|
||||
// A table present in the schema reads as a real count.
|
||||
blobs := v.tableCountForReport("blobs")
|
||||
require.NotNil(t, blobs, "an existing table must read as a real count")
|
||||
assert.Equal(t, int64(0), *blobs)
|
||||
|
||||
// A syntactically valid name the sanitizer accepts but whose table
|
||||
// the query cannot resolve is the exact shape #96 describes: a
|
||||
// would-be loud failure that used to be discarded into a 0.
|
||||
_, err = v.getTableCount("snapshots_missing")
|
||||
require.Error(t, err, "a query against a nonexistent table must fail")
|
||||
|
||||
missing := v.tableCountForReport("snapshots_missing")
|
||||
assert.Nil(t, missing, "a failed read is unknown, not a count")
|
||||
|
||||
// The rendered count for a failed read must say unknown, never 0.
|
||||
assert.Equal(t, countUnknown, countText(missing))
|
||||
assert.NotEqual(t, "0", countText(missing))
|
||||
}
|
||||
|
||||
// TestCountTextDistinguishesEmptyFromUnknown pins the distinction the
|
||||
// output has to preserve: 0 means the table was empty, "unknown" means
|
||||
// the count could not be read.
|
||||
func TestCountTextDistinguishesEmptyFromUnknown(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
zero := int64(0)
|
||||
seven := int64(7)
|
||||
|
||||
assert.Equal(t, "0", countText(&zero))
|
||||
assert.Equal(t, "7", countText(&seven))
|
||||
assert.Equal(t, countUnknown, countText(nil))
|
||||
}
|
||||
|
||||
// TestCountDiffUnknownWhenEitherSideUnknown checks that a delta computed
|
||||
// from an unreadable count is itself unknown rather than a plausible
|
||||
// number.
|
||||
func TestCountDiffUnknownWhenEitherSideUnknown(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
before := int64(10)
|
||||
after := int64(3)
|
||||
|
||||
require.NotNil(t, countDiff(&before, &after))
|
||||
assert.Equal(t, int64(7), *countDiff(&before, &after))
|
||||
|
||||
assert.Nil(t, countDiff(nil, &after), "unknown before yields unknown delta")
|
||||
assert.Nil(t, countDiff(&before, nil), "unknown after yields unknown delta")
|
||||
assert.Nil(t, countDiff(nil, nil))
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -1540,12 +1541,17 @@ func (v *Vaultik) outputRemoveJSON(result *RemoveResult) error {
|
||||
return encoder.Encode(result)
|
||||
}
|
||||
|
||||
// PruneResult contains statistics about the prune operation
|
||||
// PruneResult contains statistics about the prune operation.
|
||||
// SnapshotsDeleted counts snapshots actually deleted. FilesDeleted,
|
||||
// ChunksDeleted, and BlobsDeleted are derived from before/after row
|
||||
// counts of the local index; each is nil when a count could not be read,
|
||||
// so an unreadable count is reported as unknown rather than silently
|
||||
// as 0.
|
||||
type PruneResult struct {
|
||||
SnapshotsDeleted int64
|
||||
FilesDeleted int64
|
||||
ChunksDeleted int64
|
||||
BlobsDeleted int64
|
||||
FilesDeleted *int64
|
||||
ChunksDeleted *int64
|
||||
BlobsDeleted *int64
|
||||
}
|
||||
|
||||
// PruneDatabase removes incomplete snapshots and orphaned files, chunks,
|
||||
@@ -1560,7 +1566,7 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
|
||||
result := &PruneResult{}
|
||||
|
||||
// Snapshot counts before deletion of incompletes.
|
||||
snapshotCountBefore, _ := v.getTableCount("snapshots")
|
||||
snapshotCountBefore := v.tableCountForReport("snapshots")
|
||||
|
||||
// First, delete any incomplete snapshots
|
||||
incompleteSnapshots, err := v.Repositories.Snapshots.GetIncompleteSnapshots(v.ctx)
|
||||
@@ -1575,9 +1581,9 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
|
||||
}
|
||||
|
||||
// Get counts before cleanup for reporting
|
||||
fileCountBefore, _ := v.getTableCount("files")
|
||||
chunkCountBefore, _ := v.getTableCount("chunks")
|
||||
blobCountBefore, _ := v.getTableCount("blobs")
|
||||
fileCountBefore := v.tableCountForReport("files")
|
||||
chunkCountBefore := v.tableCountForReport("chunks")
|
||||
blobCountBefore := v.tableCountForReport("blobs")
|
||||
|
||||
// Run the cleanup
|
||||
err = v.SnapshotManager.CleanupOrphanedData(v.ctx)
|
||||
@@ -1586,36 +1592,83 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) {
|
||||
}
|
||||
|
||||
// Get counts after cleanup
|
||||
fileCountAfter, _ := v.getTableCount("files")
|
||||
chunkCountAfter, _ := v.getTableCount("chunks")
|
||||
blobCountAfter, _ := v.getTableCount("blobs")
|
||||
fileCountAfter := v.tableCountForReport("files")
|
||||
chunkCountAfter := v.tableCountForReport("chunks")
|
||||
blobCountAfter := v.tableCountForReport("blobs")
|
||||
|
||||
result.FilesDeleted = fileCountBefore - fileCountAfter
|
||||
result.ChunksDeleted = chunkCountBefore - chunkCountAfter
|
||||
result.BlobsDeleted = blobCountBefore - blobCountAfter
|
||||
result.FilesDeleted = countDiff(fileCountBefore, fileCountAfter)
|
||||
result.ChunksDeleted = countDiff(chunkCountBefore, chunkCountAfter)
|
||||
result.BlobsDeleted = countDiff(blobCountBefore, blobCountAfter)
|
||||
|
||||
log.Info("Local database prune complete",
|
||||
"incomplete_snapshots", result.SnapshotsDeleted,
|
||||
"orphaned_files", result.FilesDeleted,
|
||||
"orphaned_chunks", result.ChunksDeleted,
|
||||
"orphaned_blobs", result.BlobsDeleted,
|
||||
"orphaned_files", countText(result.FilesDeleted),
|
||||
"orphaned_chunks", countText(result.ChunksDeleted),
|
||||
"orphaned_blobs", countText(result.BlobsDeleted),
|
||||
)
|
||||
|
||||
snapshotCountAfter := snapshotCountBefore - result.SnapshotsDeleted
|
||||
// Snapshots remaining after removing the incomplete ones; unknown if
|
||||
// the pre-prune snapshot count could not be read.
|
||||
snapshotsRemain := countDiff(snapshotCountBefore, &result.SnapshotsDeleted)
|
||||
|
||||
v.UI.Completef("Pruned local index database.")
|
||||
v.UI.Detailf("Incomplete snapshots: %d removed (%d remain).",
|
||||
result.SnapshotsDeleted, snapshotCountAfter)
|
||||
v.UI.Detailf("Orphaned files: %d removed (%d remain).",
|
||||
result.FilesDeleted, fileCountAfter)
|
||||
v.UI.Detailf("Orphaned chunks: %d removed (%d remain).",
|
||||
result.ChunksDeleted, chunkCountAfter)
|
||||
v.UI.Detailf("Orphaned blobs: %d removed (%d remain).",
|
||||
result.BlobsDeleted, blobCountAfter)
|
||||
v.UI.Detailf("Incomplete snapshots: %s removed (%s remain).",
|
||||
countText(&result.SnapshotsDeleted), countText(snapshotsRemain))
|
||||
v.UI.Detailf("Orphaned files: %s removed (%s remain).",
|
||||
countText(result.FilesDeleted), countText(fileCountAfter))
|
||||
v.UI.Detailf("Orphaned chunks: %s removed (%s remain).",
|
||||
countText(result.ChunksDeleted), countText(chunkCountAfter))
|
||||
v.UI.Detailf("Orphaned blobs: %s removed (%s remain).",
|
||||
countText(result.BlobsDeleted), countText(blobCountAfter))
|
||||
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// countUnknown is what a count reads as when its query could not be run,
|
||||
// distinct from "0", which means the table really was empty.
|
||||
const countUnknown = "unknown"
|
||||
|
||||
// tableCountForReport returns the row count of a table for the prune
|
||||
// summary, or nil if the count could not be read. A read failure is
|
||||
// logged at warn — visible even under --json, which routes warnings to
|
||||
// stderr — and then rendered as unknown rather than silently becoming 0,
|
||||
// so a broken query is a visible failure instead of a plausible wrong
|
||||
// number.
|
||||
func (v *Vaultik) tableCountForReport(tableName string) *int64 {
|
||||
count, err := v.getTableCount(tableName)
|
||||
if err != nil {
|
||||
log.Warn("could not read table row count for prune summary",
|
||||
"table", tableName, "error", err)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
return &count
|
||||
}
|
||||
|
||||
// countDiff returns before-after, or nil if either count is unknown so
|
||||
// that an unreadable count does not collapse into a plausible delta.
|
||||
func countDiff(before, after *int64) *int64 {
|
||||
if before == nil || after == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
diff := *before - *after
|
||||
|
||||
return &diff
|
||||
}
|
||||
|
||||
// countText renders a count that may be unknown: nil (the read failed)
|
||||
// becomes "unknown", never "0", so a reader can tell an empty table from
|
||||
// one that could not be queried.
|
||||
func countText(count *int64) string {
|
||||
if count == nil {
|
||||
return countUnknown
|
||||
}
|
||||
|
||||
return strconv.FormatInt(*count, 10)
|
||||
}
|
||||
|
||||
// validTableNameRe matches table names containing only lowercase
|
||||
// alphanumeric characters and underscores.
|
||||
var validTableNameRe = regexp.MustCompile(`^[a-z0-9_]+$`)
|
||||
|
||||
Reference in New Issue
Block a user