Report blob counts from snapshot verify --deep --json (closes #314 )

Deep verify set verified only on success and never set missing, so a failed --deep --json run reported zero for both. It now counts each blob whose contents pass as it goes, and the existence check runs through every blob, counting the missing and wrong-size ones as shallow verify does, instead of stopping at the first; it fails if it found any. Deep verify still stops at the first damaged blob and reads no blob contents while any blob is missing or the wrong size, so verified is 0 there where shallow mode counts the present blobs. Each missing or wrong-size blob is logged as a warning with its hash; the error now gives counts instead of naming one blob. Model: opus-5-5
2026-10-13 12:31:17 +00:00
7 changed files with 170 additions and 133 deletions
@@ -90,7 +90,7 @@ Stores information about packed, compressed, and encrypted blob files.
 - `created_ts` (INTEGER NOT NULL) - Creation timestamp
 - `finished_ts` (INTEGER) - Finalization timestamp (NULL if in progress)
 - `uncompressed_size` (INTEGER NOT NULL DEFAULT 0) - Total size of chunks before compression
- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption. Once uploaded, the size of the object in storage: when the blob was already stored, under another compression level or other recipients, this can differ from the size of the local copy
+- `compressed_size` (INTEGER NOT NULL DEFAULT 0) - Size after compression and encryption
 - `uploaded_ts` (INTEGER) - Upload completion timestamp (NULL if not uploaded)

 **Purpose:** Tracks blob lifecycle from creation through upload. The UUID primary key allows immediate association of chunks with blobs.
@@ -183,7 +183,7 @@ Tracks blob upload metrics.
   - `UPDATE blobs SET blob_hash = ?, finished_ts = ?` - Finalize blob after packing

 4. **Upload**
-   - `UPDATE blobs SET uploaded_ts = ?, compressed_size = ?` - Mark blob as uploaded, with the size of the object in storage (the upload is skipped when the blob is already stored)
+   - `UPDATE blobs SET uploaded_ts = ?` - Mark blob as uploaded
   - `INSERT INTO uploads` - Record upload metrics with snapshot_id
   - `INSERT INTO snapshot_blobs` - Associate blob with snapshot

@@ -159,16 +159,13 @@ func (r *BlobRepository) UpdateFinished(
 	return nil
 }

-// UpdateUploaded marks a blob as uploaded and records the size of the
-// stored object as its compressed size. That size can differ from the
-// local copy's when an identical blob was already stored with another
-// compression level or other recipients.
+// UpdateUploaded marks a blob as uploaded
 func (r *BlobRepository) UpdateUploaded(
-	ctx context.Context, tx *sql.Tx, id string, storedSize int64,
+	ctx context.Context, tx *sql.Tx, id string,
 ) error {
 	query := `
-		UPDATE blobs
-		SET uploaded_ts = ?, compressed_size = ?
+		UPDATE blobs 
+		SET uploaded_ts = ?
 		WHERE id = ?
 	`

@@ -176,9 +173,9 @@ func (r *BlobRepository) UpdateUploaded(

 	var err error
 	if tx != nil {
-		_, err = tx.ExecContext(ctx, query, now, storedSize, id)
+		_, err = tx.ExecContext(ctx, query, now, id)
 	} else {
-		_, err = r.db.ExecWithLog(ctx, query, now, storedSize, id)
+		_, err = r.db.ExecWithLog(ctx, query, now, id)
 	}

 	if err != nil {
@@ -122,8 +122,8 @@ func TestBlobRepositoryUpdates(t *testing.T) {
 		t.Errorf("expected compressed size 500, got %d", updated.CompressedSize)
 	}

-	// Test UpdateUploaded, with a stored object larger than the local copy
-	err = repo.UpdateUploaded(ctx, nil, blob.ID.String(), 600)
+	// Test UpdateUploaded
+	err = repo.UpdateUploaded(ctx, nil, blob.ID.String())
 	if err != nil {
 		t.Fatalf("failed to update blob as uploaded: %v", err)
 	}
@@ -141,10 +141,6 @@ func TestBlobRepositoryUpdates(t *testing.T) {
 	if uploaded.UploadedTS.Before(now.Add(-1 * time.Second)) {
 		t.Error("uploaded timestamp should be around test time")
 	}
-
-	if uploaded.CompressedSize != 600 {
-		t.Errorf("expected the stored size 600, got %d", uploaded.CompressedSize)
-	}
 }

 func TestBlobRepositoryDuplicate(t *testing.T) {
@@ -1514,8 +1514,7 @@ func (s *Scanner) finalizeProcessPhase(ctx context.Context, result *ScanResult)
 			// snapshot references has uploaded_ts set, so deduplication and
 			// interrupted-run repair treat these blobs as trustworthy.
 			err = s.repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error {
-				err := s.repos.Blobs.UpdateUploaded(
-					ctx, tx, b.ID, b.Compressed)
+				err := s.repos.Blobs.UpdateUploaded(ctx, tx, b.ID)
 				if err != nil {
 					return fmt.Errorf("marking blob uploaded: %w", err)
 				}
@@ -1551,7 +1550,7 @@ func (s *Scanner) handleBlobReady(
 	blobPath := fmt.Sprintf("blobs/%s/%s/%s",
 		finishedBlob.Hash[:2], finishedBlob.Hash[2:4], finishedBlob.Hash)

-	storedSize, blobExists, err := s.uploadBlobIfNeeded(
+	blobExists, err := s.uploadBlobIfNeeded(
 		ctx, blobPath, blobWithReader, startTime, result)
 	if err != nil {
 		s.cleanupBlobTempFile(blobWithReader)
@@ -1559,8 +1558,7 @@ func (s *Scanner) handleBlobReady(
 		return fmt.Errorf("uploading blob %s: %w", finishedBlob.Hash, err)
 	}

-	err = s.recordBlobMetadata(
-		ctx, finishedBlob, storedSize, blobExists, startTime)
+	err = s.recordBlobMetadata(ctx, finishedBlob, blobExists, startTime)
 	if err != nil {
 		s.cleanupBlobTempFile(blobWithReader)

@@ -1582,22 +1580,20 @@ func (s *Scanner) handleBlobReady(
 }

 // uploadBlobIfNeeded uploads the blob to storage if it doesn't already
-// exist. It returns the size of the object in storage, which for an
-// existing object can differ from the local copy's, and whether it
-// already existed.
+// exist, returns whether it existed
 func (s *Scanner) uploadBlobIfNeeded(
 	ctx context.Context,
 	blobPath string,
 	blobWithReader *blob.WithReader,
 	startTime time.Time,
 	result *ScanResult,
-) (int64, bool, error) {
+) (bool, error) {
 	finishedBlob := blobWithReader.FinishedBlob

 	// Check if blob already exists (deduplication after restart)
 	destination := s.storage.Info().Location

-	stored, err := s.storage.Stat(ctx, blobPath)
+	_, err := s.storage.Stat(ctx, blobPath)
 	if err == nil {
 		log.Info("Blob already exists in storage, skipping upload",
 			"hash", finishedBlob.Hash,
@@ -1606,7 +1602,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 			s.ui.Hex(finishedBlob.Hash), s.ui.Size(finishedBlob.Compressed),
 			s.ui.Path(destination))

-		return stored.Size, true, nil
+		return true, nil
 	}

 	s.ui.Beginf("Uploading blob %s (%s) to %s.",
@@ -1620,7 +1616,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 	if err != nil {
 		log.Error("Failed to upload blob", "hash", finishedBlob.Hash, "error", err)

-		return 0, false, fmt.Errorf("uploading blob to storage: %w", err)
+		return false, fmt.Errorf("uploading blob to storage: %w", err)
 	}

 	uploadDuration := time.Since(startTime)
@@ -1651,7 +1647,7 @@ func (s *Scanner) uploadBlobIfNeeded(
 		stats.BytesUploaded.Add(finishedBlob.Compressed)
 	}

-	return finishedBlob.Compressed, false, nil
+	return false, nil
 }

 // makeUploadProgressCallback creates a progress callback for blob uploads.
@@ -1721,7 +1717,6 @@ func (s *Scanner) makeUploadProgressCallback(
 func (s *Scanner) recordBlobMetadata(
 	ctx context.Context,
 	finishedBlob *blob.FinishedBlob,
-	storedSize int64,
 	blobExists bool,
 	startTime time.Time,
 ) error {
@@ -1733,8 +1728,7 @@ func (s *Scanner) recordBlobMetadata(
 	uploadDuration := time.Since(startTime)

 	return s.repos.WithTx(ctx, func(txCtx context.Context, tx *sql.Tx) error {
-		err := s.repos.Blobs.UpdateUploaded(
-			txCtx, tx, finishedBlob.ID, storedSize)
+		err := s.repos.Blobs.UpdateUploaded(txCtx, tx, finishedBlob.ID)
 		if err != nil {
 			return fmt.Errorf("updating blob upload timestamp: %w", err)
 		}
@@ -1,15 +1,20 @@
 package vaultik_test

 import (
+	"bytes"
 	"context"
+	"crypto/rand"
 	"io"
 	"os"
 	"path/filepath"
+	"slices"
 	"testing"

 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/vaultik/internal/config"
 	"sneak.berlin/go/vaultik/internal/log"
+	"sneak.berlin/go/vaultik/internal/storage"
 	"sneak.berlin/go/vaultik/internal/ui"
 	"sneak.berlin/go/vaultik/internal/vaultik"
 )
@@ -77,6 +82,116 @@ func TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob(t *testing.T) {
 		"deep verify should fail on a corrupted blob")
 }

+// TestDeepVerifyJSONCountsBlobs checks the verified and missing counts
+// that deep verify reports under --json: on a healthy snapshot, with a
+// damaged blob, and with missing blobs. Deep verify stops at the first
+// damaged blob, so the blobs read before it are the ones verified; when
+// blobs are missing it reads no blob contents, but counts every missing
+// blob.
+func TestDeepVerifyJSONCountsBlobs(t *testing.T) {
+	log.Initialize(log.Config{})
+	t.Parallel()
+
+	fs := afero.NewOsFs()
+	tempDir := t.TempDir()
+
+	dataDir := filepath.Join(tempDir, "source")
+	storeDir := filepath.Join(tempDir, "remote")
+	dbPath := filepath.Join(tempDir, "index.sqlite")
+
+	chunkSize := int64(32 * 1024)
+	maxBlobSize := int64(128 * 1024)
+
+	// Random data does not compress or deduplicate, so it fills several
+	// blobs.
+	data := make([]byte, maxBlobSize*4)
+	_, err := rand.Read(data)
+	require.NoError(t, err)
+	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
+	require.NoError(t, afero.WriteFile(fs,
+		filepath.Join(dataDir, "data.bin"), data, 0o644))
+
+	ctx := context.Background()
+
+	cfg, storer, snapshotID := runFileStorageBackup(
+		ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize)
+
+	healthy, err := deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.NoError(t, err)
+	require.Equal(t, "ok", healthy.Status)
+	require.GreaterOrEqual(t, healthy.BlobCount, 3)
+	require.Equal(t, healthy.BlobCount, healthy.Verified)
+	require.Zero(t, healthy.Missing)
+
+	// Deep verify reads blobs in hash order, which is the sorted order of
+	// their storage keys.
+	keys, err := storer.List(ctx, "blobs/")
+	require.NoError(t, err)
+	slices.Sort(keys)
+	require.Len(t, keys, healthy.BlobCount)
+
+	// Damage the last blob read, keeping its length, so every blob before
+	// it is read and passes first.
+	lastPath := filepath.Join(storeDir, keys[len(keys)-1])
+	blob, err := afero.ReadFile(fs, lastPath)
+	require.NoError(t, err)
+
+	blob[len(blob)/2] ^= 0xff
+	require.NoError(t, afero.WriteFile(fs, lastPath, blob, 0o600))
+
+	got, err := deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.Error(t, err)
+	require.Equal(t, "failed", got.Status)
+	require.Equal(t, healthy.BlobCount-1, got.Verified)
+	require.Zero(t, got.Missing)
+
+	// Delete the first blob and the damaged last one: both must be
+	// counted as missing, not only the first one found.
+	var deletedSize int64
+
+	for _, key := range []string{keys[0], keys[len(keys)-1]} {
+		info, statErr := storer.Stat(ctx, key)
+		require.NoError(t, statErr)
+
+		deletedSize += info.Size
+
+		require.NoError(t, storer.Delete(ctx, key))
+	}
+
+	got, err = deepVerifyJSON(ctx, t, cfg, storer, snapshotID)
+	require.Error(t, err)
+	require.Equal(t, "failed", got.Status)
+	require.Equal(t, 2, got.Missing)
+	require.Equal(t, deletedSize, got.MissingSize)
+	require.Zero(t, got.Verified)
+}
+
+// deepVerifyJSON runs deep verify with --json on snapshotID and returns
+// the decoded result and the error RunDeepVerify returned.
+func deepVerifyJSON(
+	ctx context.Context, t *testing.T,
+	cfg *config.Config, storer storage.Storer, snapshotID string,
+) (vaultik.VerifyResult, error) {
+	t.Helper()
+
+	var out bytes.Buffer
+
+	v := &vaultik.Vaultik{
+		Config:  cfg,
+		Storage: storer,
+		Fs:      afero.NewOsFs(),
+		Stdout:  &out,
+		Stderr:  io.Discard,
+		UI:      ui.NewWithColor(io.Discard, false),
+	}
+	v.SetContext(ctx)
+
+	err := v.RunDeepVerify(snapshotID,
+		&vaultik.VerifyOptions{Deep: true, JSON: true})
+
+	return decodeVerifyResult(t, out.Bytes()), err
+}
+
 // corruptOneBlob flips a middle byte of the first blob file found under
 // blobsDir, leaving the file length unchanged.
 func corruptOneBlob(t *testing.T, fs afero.Fs, blobsDir string) {
@@ -1,83 +0,0 @@
-package vaultik_test
-
-import (
-	"context"
-	"io"
-	"path/filepath"
-	"testing"
-
-	"filippo.io/age"
-	"github.com/spf13/afero"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/vaultik/internal/log"
-	"sneak.berlin/go/vaultik/internal/ui"
-	"sneak.berlin/go/vaultik/internal/vaultik"
-)
-
-// TestVerifyPassesWhenBackupReusesStoredBlob backs up a file, then backs it
-// up again from an empty local index, as after `database delete`, with an
-// extra age recipient. The second backup builds the same blob, finds it
-// already stored and skips the upload, but its own copy is larger, because
-// age writes a header entry for each recipient. The second snapshot must
-// record the size of the stored object, so shallow and deep verify pass.
-func TestVerifyPassesWhenBackupReusesStoredBlob(t *testing.T) {
-	log.Initialize(log.Config{})
-	t.Parallel()
-
-	fs := afero.NewOsFs()
-	tempDir := t.TempDir()
-
-	dataDir := filepath.Join(tempDir, "source")
-	storeDir := filepath.Join(tempDir, "remote")
-	secondDBPath := filepath.Join(tempDir, "second.sqlite")
-
-	chunkSize := int64(64 * 1024)
-	maxBlobSize := int64(512 * 1024)
-
-	require.NoError(t, fs.MkdirAll(dataDir, 0o755))
-	require.NoError(t, afero.WriteFile(fs,
-		filepath.Join(dataDir, "data.bin"),
-		bytesPattern("reuse-", int(chunkSize*3)), 0o644))
-
-	ctx := context.Background()
-
-	runFileStorageBackup(ctx, t, fs, dataDir, storeDir,
-		filepath.Join(tempDir, "first.sqlite"), chunkSize, maxBlobSize)
-
-	env := setupDedupBackupEnv(
-		ctx, t, fs, storeDir, secondDBPath, chunkSize, maxBlobSize)
-
-	defer func() { _ = env.db.Close() }()
-
-	extra, err := age.GenerateX25519Identity()
-	require.NoError(t, err)
-
-	env.cfg.AgeRecipients = append(env.cfg.AgeRecipients,
-		extra.Recipient().String())
-
-	snapshotID, result := runDedupSnapshot(ctx, t, env.sm, env.makeScanner(),
-		env.cfg.Hostname, "second", dataDir, secondDBPath)
-	require.Positive(t, result.BlobsCreated)
-	require.Zero(t, result.BlobsUploaded,
-		"the second backup should find every blob already stored")
-
-	require.NoError(t, env.db.Close())
-
-	verifier := &vaultik.Vaultik{
-		Config:  env.cfg,
-		Storage: env.storer,
-		Fs:      fs,
-		Stdout:  io.Discard,
-		Stderr:  io.Discard,
-		UI:      ui.NewWithColor(io.Discard, false),
-	}
-	verifier.SetContext(ctx)
-
-	require.NoError(t,
-		verifier.VerifySnapshotWithOptions(snapshotID, &vaultik.VerifyOptions{}),
-		"shallow verify should pass")
-	require.NoError(t,
-		verifier.VerifySnapshotWithOptions(snapshotID,
-			&vaultik.VerifyOptions{Deep: true}),
-		"deep verify should pass")
-}
@@ -32,7 +32,9 @@ var (
 	errManifestExtraBlob   = errors.New("manifest contains blob not in database")
 	errManifestMissingBlob = errors.New(
 		"manifest omits blob present in database")
-	errBlobSizeMismatch = errors.New("blob size mismatch")
+	errBlobSizeMismatch  = errors.New("blob size mismatch")
+	errBlobsNotInStorage = errors.New(
+		"blobs missing from storage or stored with the wrong size")
 )

 // verifyStatusFailed is the JSON status value for a failed verification.
@@ -135,7 +137,6 @@ func (v *Vaultik) RunDeepVerify(snapshotID string, opts *VerifyOptions) error {
 	}

 	result.Status = "ok"
-	result.Verified = len(dbBlobs)

 	if opts.JSON {
 		return v.outputVerifyJSON(result)
@@ -290,7 +291,7 @@ func (v *Vaultik) runVerificationSteps(
 		v.stdoutf("Checking blob existence in remote storage...\n")
 	}

-	err = v.verifyBlobExistenceFromDB(dbBlobs)
+	err = v.verifyBlobExistenceFromDB(dbBlobs, result)
 	if err != nil {
 		return v.deepVerifyFailure(result, opts, err.Error(), err)
 	}
@@ -301,7 +302,8 @@ func (v *Vaultik) runVerificationSteps(
 			len(dbBlobs), ubytes(totalSize))
 	}

-	err = v.performDeepVerificationFromDB(dbBlobs, tdb.db.Conn(), opts, identities)
+	err = v.performDeepVerificationFromDB(
+		dbBlobs, tdb.db.Conn(), opts, result, identities)
 	if err != nil {
 		return v.deepVerifyFailure(result, opts, err.Error(), err)
 	}
@@ -653,8 +655,13 @@ func (v *Vaultik) verifyManifestAgainstDatabase(
 	return nil
 }

-// verifyBlobExistenceFromDB checks that all blobs from database exist in S3
-func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error {
+// verifyBlobExistenceFromDB checks that every blob from the database is
+// in storage with the size the database records. It checks them all,
+// counting the missing and wrong-size blobs in result, and fails if it
+// found any.
+func (v *Vaultik) verifyBlobExistenceFromDB(
+	blobs []snapshot.BlobInfo, result *VerifyResult,
+) error {
 	log.Info("Verifying blob existence in S3", "blob_count", len(blobs))

 	for i, blob := range blobs {
@@ -667,17 +674,21 @@ func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error {
 		// Construct blob path
 		blobPath := fmt.Sprintf("blobs/%s/%s/%s", blob.Hash[:2], blob.Hash[2:4], blob.Hash)

-		// Check blob exists
 		stat, err := v.Storage.Stat(v.ctx, blobPath)
-		if err != nil {
-			return fmt.Errorf("blob %s missing from storage: %w", blob.Hash, err)
-		}
+		switch {
+		case err != nil:
+			log.Warn("Blob missing from storage",
+				"hash", blob.Hash, "error", err)

-		// Verify size matches
-		if stat.Size != blob.CompressedSize {
-			return fmt.Errorf(
-				"%w: blob %s: S3 has %d bytes, database has %d bytes",
-				errBlobSizeMismatch, blob.Hash, stat.Size, blob.CompressedSize)
+			result.Missing++
+			result.MissingSize += blob.CompressedSize
+		case stat.Size != blob.CompressedSize:
+			log.Warn("Blob has the wrong size in storage",
+				"hash", blob.Hash,
+				"storage_size", stat.Size,
+				"database_size", blob.CompressedSize)
+
+			result.Mismatched++
 		}

 		// Progress update every 100 blobs
@@ -691,16 +702,22 @@ func (v *Vaultik) verifyBlobExistenceFromDB(blobs []snapshot.BlobInfo) error {
 		}
 	}

+	if result.Missing > 0 || result.Mismatched > 0 {
+		return fmt.Errorf("%w: %d missing, %d with the wrong size",
+			errBlobsNotInStorage, result.Missing, result.Mismatched)
+	}
+
 	log.Info("✓ All blobs exist in storage")

 	return nil
 }

 // performDeepVerificationFromDB downloads and verifies the content of
-// each blob using the database as source.
+// each blob using the database as source, stopping at the first blob that
+// fails. Each blob that passes is counted in result.Verified.
 func (v *Vaultik) performDeepVerificationFromDB(
 	blobs []snapshot.BlobInfo, db *sql.DB, opts *VerifyOptions,
-	identities []age.Identity,
+	result *VerifyResult, identities []age.Identity,
 ) error {
 	// Calculate total bytes for ETA
 	var totalBytesExpected int64
@@ -723,6 +740,7 @@ func (v *Vaultik) performDeepVerificationFromDB(
 			return fmt.Errorf("blob %s verification failed: %w", blobInfo.Hash, err)
 		}

+		result.Verified++
 		bytesProcessed += blobInfo.CompressedSize
 		elapsed := time.Since(startTime)
 		remaining := len(blobs) - (i + 1)