diff --git a/internal/database/blob_chunks.go b/internal/database/blob_chunks.go index 13e6b27..6897e21 100644 --- a/internal/database/blob_chunks.go +++ b/internal/database/blob_chunks.go @@ -3,6 +3,7 @@ package database import ( "context" "database/sql" + "errors" "fmt" ) @@ -49,12 +50,15 @@ func (r *BlobChunkRepository) GetByBlobID(ctx context.Context, blobID string) ([ defer CloseRows(rows) var blobChunks []*BlobChunk + for rows.Next() { var bc BlobChunk + err := rows.Scan(&bc.BlobID, &bc.ChunkHash, &bc.Offset, &bc.Length) if err != nil { return nil, fmt.Errorf("scanning blob chunk: %w", err) } + blobChunks = append(blobChunks, &bc) } @@ -70,7 +74,9 @@ func (r *BlobChunkRepository) GetByChunkHash(ctx context.Context, chunkHash stri ` LogSQL("GetByChunkHash", query, chunkHash) + var bc BlobChunk + err := r.db.conn.QueryRowContext(ctx, query, chunkHash).Scan( &bc.BlobID, &bc.ChunkHash, @@ -78,16 +84,20 @@ func (r *BlobChunkRepository) GetByChunkHash(ctx context.Context, chunkHash stri &bc.Length, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { LogSQL("GetByChunkHash", "No rows found", chunkHash) + return nil, nil } + if err != nil { LogSQL("GetByChunkHash", "Error", chunkHash, err) + return nil, fmt.Errorf("querying blob chunk: %w", err) } LogSQL("GetByChunkHash", "Found blob", chunkHash, "blob", bc.BlobID) + return &bc, nil } @@ -101,7 +111,9 @@ func (r *BlobChunkRepository) GetByChunkHashTx(ctx context.Context, tx *sql.Tx, ` LogSQL("GetByChunkHashTx", query, chunkHash) + var bc BlobChunk + err := tx.QueryRowContext(ctx, query, chunkHash).Scan( &bc.BlobID, &bc.ChunkHash, @@ -109,16 +121,20 @@ func (r *BlobChunkRepository) GetByChunkHashTx(ctx context.Context, tx *sql.Tx, &bc.Length, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { LogSQL("GetByChunkHashTx", "No rows found", chunkHash) + return nil, nil } + if err != nil { LogSQL("GetByChunkHashTx", "Error", chunkHash, err) + return nil, fmt.Errorf("querying blob chunk: %w", err) } LogSQL("GetByChunkHashTx", "Found blob", chunkHash, "blob", bc.BlobID) + return &bc, nil } diff --git a/internal/database/blob_chunks_test.go b/internal/database/blob_chunks_test.go index 9dae601..97d3e68 100644 --- a/internal/database/blob_chunks_test.go +++ b/internal/database/blob_chunks_test.go @@ -22,6 +22,7 @@ func TestBlobChunkRepository(t *testing.T) { Hash: types.BlobHash("blob1-hash"), CreatedTS: time.Now(), } + err := repos.Blobs.Create(ctx, nil, blob) if err != nil { t.Fatalf("failed to create blob: %v", err) @@ -34,6 +35,7 @@ func TestBlobChunkRepository(t *testing.T) { ChunkHash: chunkHash, Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk %s: %v", chunkHash, err) @@ -60,6 +62,7 @@ func TestBlobChunkRepository(t *testing.T) { Offset: 1024, Length: 2048, } + err = repos.BlobChunks.Create(ctx, nil, bc2) if err != nil { t.Fatalf("failed to create second blob chunk: %v", err) @@ -71,6 +74,7 @@ func TestBlobChunkRepository(t *testing.T) { Offset: 3072, Length: 512, } + err = repos.BlobChunks.Create(ctx, nil, bc3) if err != nil { t.Fatalf("failed to create third blob chunk: %v", err) @@ -81,6 +85,7 @@ func TestBlobChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get blob chunks: %v", err) } + if len(blobChunks) != 3 { t.Errorf("expected 3 chunks, got %d", len(blobChunks)) } @@ -98,12 +103,15 @@ func TestBlobChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get blob chunk by chunk hash: %v", err) } + if bc == nil { t.Fatal("expected blob chunk, got nil") } + if bc.BlobID != blob.ID { t.Errorf("wrong blob ID: expected %s, got %s", blob.ID, bc.BlobID) } + if bc.Offset != 1024 { t.Errorf("wrong offset: expected 1024, got %d", bc.Offset) } @@ -113,6 +121,7 @@ func TestBlobChunkRepository(t *testing.T) { if err == nil { t.Fatal("duplicate blob_chunk insert should fail due to primary key constraint") } + if !strings.Contains(err.Error(), "UNIQUE") && !strings.Contains(err.Error(), "constraint") { t.Fatalf("expected constraint error, got: %v", err) } @@ -122,6 +131,7 @@ func TestBlobChunkRepository(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + if bc != nil { t.Error("expected nil for non-existent chunk") } @@ -150,6 +160,7 @@ func TestBlobChunkRepositoryMultipleBlobs(t *testing.T) { if err != nil { t.Fatalf("failed to create blob1: %v", err) } + err = repos.Blobs.Create(ctx, nil, blob2) if err != nil { t.Fatalf("failed to create blob2: %v", err) @@ -162,6 +173,7 @@ func TestBlobChunkRepositoryMultipleBlobs(t *testing.T) { ChunkHash: chunkHash, Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk %s: %v", chunkHash, err) @@ -189,6 +201,7 @@ func TestBlobChunkRepositoryMultipleBlobs(t *testing.T) { if err != nil { t.Fatalf("failed to get blob1 chunks: %v", err) } + if len(chunks) != 2 { t.Errorf("expected 2 chunks for blob1, got %d", len(chunks)) } @@ -198,6 +211,7 @@ func TestBlobChunkRepositoryMultipleBlobs(t *testing.T) { if err != nil { t.Fatalf("failed to get blob2 chunks: %v", err) } + if len(chunks) != 2 { t.Errorf("expected 2 chunks for blob2, got %d", len(chunks)) } @@ -207,6 +221,7 @@ func TestBlobChunkRepositoryMultipleBlobs(t *testing.T) { if err != nil { t.Fatalf("failed to get shared chunk: %v", err) } + if bc == nil { t.Fatal("expected shared chunk, got nil") } diff --git a/internal/database/blobs.go b/internal/database/blobs.go index 58aed0f..5bab2a7 100644 --- a/internal/database/blobs.go +++ b/internal/database/blobs.go @@ -3,6 +3,7 @@ package database import ( "context" "database/sql" + "errors" "fmt" "time" @@ -24,10 +25,12 @@ func (r *BlobRepository) Create(ctx context.Context, tx *sql.Tx, blob *Blob) err ` var finishedTS, uploadedTS *int64 + if blob.FinishedTS != nil { ts := blob.FinishedTS.Unix() finishedTS = &ts } + if blob.UploadedTS != nil { ts := blob.UploadedTS.Unix() uploadedTS = &ts @@ -56,9 +59,11 @@ func (r *BlobRepository) GetByHash(ctx context.Context, hash string) (*Blob, err WHERE blob_hash = ? ` - var blob Blob - var createdTSUnix int64 - var finishedTSUnix, uploadedTSUnix sql.NullInt64 + var ( + blob Blob + createdTSUnix int64 + finishedTSUnix, uploadedTSUnix sql.NullInt64 + ) err := r.db.conn.QueryRowContext(ctx, query, hash).Scan( &blob.ID, @@ -70,9 +75,10 @@ func (r *BlobRepository) GetByHash(ctx context.Context, hash string) (*Blob, err &uploadedTSUnix, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying blob: %w", err) } @@ -82,10 +88,12 @@ func (r *BlobRepository) GetByHash(ctx context.Context, hash string) (*Blob, err ts := time.Unix(finishedTSUnix.Int64, 0).UTC() blob.FinishedTS = &ts } + if uploadedTSUnix.Valid { ts := time.Unix(uploadedTSUnix.Int64, 0).UTC() blob.UploadedTS = &ts } + return &blob, nil } @@ -97,9 +105,11 @@ func (r *BlobRepository) GetByID(ctx context.Context, id string) (*Blob, error) WHERE id = ? ` - var blob Blob - var createdTSUnix int64 - var finishedTSUnix, uploadedTSUnix sql.NullInt64 + var ( + blob Blob + createdTSUnix int64 + finishedTSUnix, uploadedTSUnix sql.NullInt64 + ) err := r.db.conn.QueryRowContext(ctx, query, id).Scan( &blob.ID, @@ -111,9 +121,10 @@ func (r *BlobRepository) GetByID(ctx context.Context, id string) (*Blob, error) &uploadedTSUnix, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying blob: %w", err) } @@ -123,10 +134,12 @@ func (r *BlobRepository) GetByID(ctx context.Context, id string) (*Blob, error) ts := time.Unix(finishedTSUnix.Int64, 0).UTC() blob.FinishedTS = &ts } + if uploadedTSUnix.Valid { ts := time.Unix(uploadedTSUnix.Int64, 0).UTC() blob.UploadedTS = &ts } + return &blob, nil } @@ -146,11 +159,15 @@ func (r *BlobRepository) GetAll(ctx context.Context) (map[string]*Blob, error) { defer CloseRows(rows) out := make(map[string]*Blob) + for rows.Next() { - var blob Blob - var createdTSUnix int64 - var finishedTSUnix, uploadedTSUnix sql.NullInt64 - if err := rows.Scan( + var ( + blob Blob + createdTSUnix int64 + finishedTSUnix, uploadedTSUnix sql.NullInt64 + ) + + err := rows.Scan( &blob.ID, &blob.Hash, &createdTSUnix, @@ -158,20 +175,25 @@ func (r *BlobRepository) GetAll(ctx context.Context) (map[string]*Blob, error) { &blob.UncompressedSize, &blob.CompressedSize, &uploadedTSUnix, - ); err != nil { + ) + if err != nil { return nil, fmt.Errorf("scanning blob: %w", err) } + blob.CreatedTS = time.Unix(createdTSUnix, 0).UTC() if finishedTSUnix.Valid { ts := time.Unix(finishedTSUnix.Int64, 0).UTC() blob.FinishedTS = &ts } + if uploadedTSUnix.Valid { ts := time.Unix(uploadedTSUnix.Int64, 0).UTC() blob.UploadedTS = &ts } + out[blob.ID.String()] = &blob } + return out, rows.Err() } @@ -184,6 +206,7 @@ func (r *BlobRepository) UpdateFinished(ctx context.Context, tx *sql.Tx, id stri ` now := time.Now().UTC().Unix() + var err error if tx != nil { _, err = tx.ExecContext(ctx, query, hash, now, uncompressedSize, compressedSize, id) @@ -207,6 +230,7 @@ func (r *BlobRepository) UpdateUploaded(ctx context.Context, tx *sql.Tx, id stri ` now := time.Now().UTC().Unix() + var err error if tx != nil { _, err = tx.ExecContext(ctx, query, now, id) diff --git a/internal/database/blobs_test.go b/internal/database/blobs_test.go index 3d06c5d..2b8e88b 100644 --- a/internal/database/blobs_test.go +++ b/internal/database/blobs_test.go @@ -32,12 +32,15 @@ func TestBlobRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get blob: %v", err) } + if retrieved == nil { t.Fatal("expected blob, got nil") } + if retrieved.Hash != blob.Hash { t.Errorf("blob hash mismatch: got %s, want %s", retrieved.Hash, blob.Hash) } + if !retrieved.CreatedTS.Equal(blob.CreatedTS) { t.Errorf("created timestamp mismatch: got %v, want %v", retrieved.CreatedTS, blob.CreatedTS) } @@ -47,9 +50,11 @@ func TestBlobRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get blob by ID: %v", err) } + if retrievedByID == nil { t.Fatal("expected blob, got nil") } + if retrievedByID.ID != blob.ID { t.Errorf("blob ID mismatch: got %s, want %s", retrievedByID.ID, blob.ID) } @@ -60,6 +65,7 @@ func TestBlobRepository(t *testing.T) { Hash: types.BlobHash("blobhash456"), CreatedTS: time.Now().Truncate(time.Second), } + err = repo.Create(ctx, nil, blob2) if err != nil { t.Fatalf("failed to create second blob: %v", err) @@ -67,6 +73,7 @@ func TestBlobRepository(t *testing.T) { // Test UpdateFinished now := time.Now() + err = repo.UpdateFinished(ctx, nil, blob.ID.String(), blob.Hash.String(), 1000, 500) if err != nil { t.Fatalf("failed to update blob as finished: %v", err) @@ -77,12 +84,15 @@ func TestBlobRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get updated blob: %v", err) } + if updated.FinishedTS == nil { t.Fatal("expected finished timestamp to be set") } + if updated.UncompressedSize != 1000 { t.Errorf("expected uncompressed size 1000, got %d", updated.UncompressedSize) } + if updated.CompressedSize != 500 { t.Errorf("expected compressed size 500, got %d", updated.CompressedSize) } @@ -98,6 +108,7 @@ func TestBlobRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get uploaded blob: %v", err) } + if uploaded.UploadedTS == nil { t.Fatal("expected uploaded timestamp to be set") } diff --git a/internal/database/cascade_debug_test.go b/internal/database/cascade_debug_test.go index 6ef933f..fc71f9f 100644 --- a/internal/database/cascade_debug_test.go +++ b/internal/database/cascade_debug_test.go @@ -19,10 +19,12 @@ func TestCascadeDeleteDebug(t *testing.T) { // Check if foreign keys are enabled var fkEnabled int + err := db.conn.QueryRow("PRAGMA foreign_keys").Scan(&fkEnabled) if err != nil { t.Fatal(err) } + t.Logf("Foreign keys enabled: %d", fkEnabled) // Create a file @@ -34,18 +36,21 @@ func TestCascadeDeleteDebug(t *testing.T) { UID: 1000, GID: 1000, } + err = repos.Files.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file: %v", err) } + t.Logf("Created file with ID: %s", file.ID) // Create chunks and file-chunk mappings - for i := 0; i < 3; i++ { + for i := range 3 { chunk := &Chunk{ ChunkHash: types.ChunkHash(fmt.Sprintf("cascade-chunk-%d", i)), Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk: %v", err) @@ -56,10 +61,12 @@ func TestCascadeDeleteDebug(t *testing.T) { Idx: i, ChunkHash: chunk.ChunkHash, } + err = repos.FileChunks.Create(ctx, nil, fc) if err != nil { t.Fatalf("failed to create file chunk: %v", err) } + t.Logf("Created file chunk mapping: file_id=%s, idx=%d, chunk=%s", fc.FileID, fc.Idx, fc.ChunkHash) } @@ -68,10 +75,12 @@ func TestCascadeDeleteDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("File chunks before delete: %d", len(fileChunks)) // Check the foreign key constraint var fkInfo string + err = db.conn.QueryRow(` SELECT sql FROM sqlite_master WHERE type='table' AND name='file_chunks' @@ -79,10 +88,12 @@ func TestCascadeDeleteDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("file_chunks table definition:\n%s", fkInfo) // Delete the file t.Log("Deleting file...") + err = repos.Files.DeleteByID(ctx, nil, file.ID) if err != nil { t.Fatalf("failed to delete file: %v", err) @@ -93,6 +104,7 @@ func TestCascadeDeleteDebug(t *testing.T) { if err != nil { t.Fatal(err) } + if deletedFile != nil { t.Error("file should have been deleted") } else { @@ -104,14 +116,17 @@ func TestCascadeDeleteDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("File chunks after delete: %d", len(fileChunks)) // Manually check the database var count int + err = db.conn.QueryRow("SELECT COUNT(*) FROM file_chunks WHERE file_id = ?", file.ID).Scan(&count) if err != nil { t.Fatal(err) } + t.Logf("Manual count of file_chunks for deleted file: %d", count) if len(fileChunks) != 0 { diff --git a/internal/database/chunk_files.go b/internal/database/chunk_files.go index 0e5f0df..01e9175 100644 --- a/internal/database/chunk_files.go +++ b/internal/database/chunk_files.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "strings" "sneak.berlin/go/vaultik/internal/types" ) @@ -90,18 +91,25 @@ func (r *ChunkFileRepository) GetByFileID(ctx context.Context, fileID types.File // scanChunkFiles is a helper that scans chunk file rows func (r *ChunkFileRepository) scanChunkFiles(rows *sql.Rows) ([]*ChunkFile, error) { var chunkFiles []*ChunkFile + for rows.Next() { - var cf ChunkFile - var chunkHashStr, fileIDStr string + var ( + cf ChunkFile + chunkHashStr, fileIDStr string + ) + err := rows.Scan(&chunkHashStr, &fileIDStr, &cf.FileOffset, &cf.Length) if err != nil { return nil, fmt.Errorf("scanning chunk file: %w", err) } + cf.ChunkHash = types.ChunkHash(chunkHashStr) + cf.FileID, err = types.ParseFileID(fileIDStr) if err != nil { return nil, fmt.Errorf("parsing file ID: %w", err) } + chunkFiles = append(chunkFiles, &cf) } @@ -136,14 +144,13 @@ func (r *ChunkFileRepository) DeleteByFileIDs(ctx context.Context, tx *sql.Tx, f const batchSize = 500 for i := 0; i < len(fileIDs); i += batchSize { - end := i + batchSize - if end > len(fileIDs) { - end = len(fileIDs) - } + end := min(i+batchSize, len(fileIDs)) + batch := fileIDs[i:end] query := "DELETE FROM chunk_files WHERE file_id IN (?" + repeatPlaceholder(len(batch)-1) + ")" - args := make([]interface{}, len(batch)) + + args := make([]any, len(batch)) for j, id := range batch { args[j] = id.String() } @@ -154,6 +161,7 @@ func (r *ChunkFileRepository) DeleteByFileIDs(ctx context.Context, tx *sql.Tx, f } else { _, err = r.db.ExecWithLog(ctx, query, args...) } + if err != nil { return fmt.Errorf("batch deleting chunk_files: %w", err) } @@ -172,21 +180,28 @@ func (r *ChunkFileRepository) CreateBatch(ctx context.Context, tx *sql.Tx, cfs [ const batchSize = 200 for i := 0; i < len(cfs); i += batchSize { - end := i + batchSize - if end > len(cfs) { - end = len(cfs) - } + end := min(i+batchSize, len(cfs)) + batch := cfs[i:end] query := "INSERT INTO chunk_files (chunk_hash, file_id, file_offset, length) VALUES " - args := make([]interface{}, 0, len(batch)*4) + + args := make([]any, 0, len(batch)*4) + + var querySb183 strings.Builder + for j, cf := range batch { if j > 0 { - query += ", " + querySb183.WriteString(", ") } - query += "(?, ?, ?, ?)" + + querySb183.WriteString("(?, ?, ?, ?)") + args = append(args, cf.ChunkHash.String(), cf.FileID.String(), cf.FileOffset, cf.Length) } + + query += querySb183.String() + query += " ON CONFLICT(chunk_hash, file_id) DO NOTHING" var err error @@ -195,6 +210,7 @@ func (r *ChunkFileRepository) CreateBatch(ctx context.Context, tx *sql.Tx, cfs [ } else { _, err = r.db.ExecWithLog(ctx, query, args...) } + if err != nil { return fmt.Errorf("batch inserting chunk_files: %w", err) } diff --git a/internal/database/chunk_files_test.go b/internal/database/chunk_files_test.go index a4e8bbc..ca3fd98 100644 --- a/internal/database/chunk_files_test.go +++ b/internal/database/chunk_files_test.go @@ -28,6 +28,7 @@ func TestChunkFileRepository(t *testing.T) { GID: 1000, LinkTarget: "", } + err := fileRepo.Create(ctx, nil, file1) if err != nil { t.Fatalf("failed to create file1: %v", err) @@ -42,6 +43,7 @@ func TestChunkFileRepository(t *testing.T) { GID: 1000, LinkTarget: "", } + err = fileRepo.Create(ctx, nil, file2) if err != nil { t.Fatalf("failed to create file2: %v", err) @@ -52,6 +54,7 @@ func TestChunkFileRepository(t *testing.T) { ChunkHash: types.ChunkHash("chunk1"), Size: 1024, } + err = chunksRepo.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk: %v", err) @@ -77,6 +80,7 @@ func TestChunkFileRepository(t *testing.T) { FileOffset: 2048, Length: 1024, } + err = repo.Create(ctx, nil, cf2) if err != nil { t.Fatalf("failed to create second chunk file: %v", err) @@ -87,6 +91,7 @@ func TestChunkFileRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get chunk files: %v", err) } + if len(chunkFiles) != 2 { t.Errorf("expected 2 files for chunk, got %d", len(chunkFiles)) } @@ -94,14 +99,17 @@ func TestChunkFileRepository(t *testing.T) { // Verify both files are returned foundFile1 := false foundFile2 := false + for _, cf := range chunkFiles { if cf.FileID == file1.ID && cf.FileOffset == 0 { foundFile1 = true } + if cf.FileID == file2.ID && cf.FileOffset == 2048 { foundFile2 = true } } + if !foundFile1 || !foundFile2 { t.Error("not all expected files found") } @@ -111,9 +119,11 @@ func TestChunkFileRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get chunks by file ID: %v", err) } + if len(chunkFiles) != 1 { t.Errorf("expected 1 chunk for file, got %d", len(chunkFiles)) } + if chunkFiles[0].ChunkHash != types.ChunkHash("chunk1") { t.Errorf("wrong chunk hash: expected chunk1, got %s", chunkFiles[0].ChunkHash) } @@ -143,9 +153,11 @@ func TestChunkFileRepositoryComplexDeduplication(t *testing.T) { if err := fileRepo.Create(ctx, nil, file1); err != nil { t.Fatalf("failed to create file1: %v", err) } + if err := fileRepo.Create(ctx, nil, file2); err != nil { t.Fatalf("failed to create file2: %v", err) } + if err := fileRepo.Create(ctx, nil, file3); err != nil { t.Fatalf("failed to create file3: %v", err) } @@ -157,6 +169,7 @@ func TestChunkFileRepositoryComplexDeduplication(t *testing.T) { ChunkHash: chunkHash, Size: 1024, } + err := chunksRepo.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk %s: %v", chunkHash, err) @@ -194,6 +207,7 @@ func TestChunkFileRepositoryComplexDeduplication(t *testing.T) { if err != nil { t.Fatalf("failed to get files for chunk1: %v", err) } + if len(files) != 2 { t.Errorf("expected 2 files for chunk1, got %d", len(files)) } @@ -203,6 +217,7 @@ func TestChunkFileRepositoryComplexDeduplication(t *testing.T) { if err != nil { t.Fatalf("failed to get files for chunk2: %v", err) } + if len(files) != 2 { t.Errorf("expected 2 files for chunk2, got %d", len(files)) } @@ -212,6 +227,7 @@ func TestChunkFileRepositoryComplexDeduplication(t *testing.T) { if err != nil { t.Fatalf("failed to get chunks for file2: %v", err) } + if len(file2Chunks) != 3 { t.Errorf("expected 3 chunks for file2, got %d", len(file2Chunks)) } diff --git a/internal/database/chunks.go b/internal/database/chunks.go index 04fc4ef..91b438b 100644 --- a/internal/database/chunks.go +++ b/internal/database/chunks.go @@ -3,7 +3,9 @@ package database import ( "context" "database/sql" + "errors" "fmt" + "strings" "sneak.berlin/go/vaultik/internal/log" ) @@ -51,9 +53,10 @@ func (r *ChunkRepository) GetByHash(ctx context.Context, hash string) (*Chunk, e &chunk.Size, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying chunk: %w", err) } @@ -71,14 +74,22 @@ func (r *ChunkRepository) GetByHashes(ctx context.Context, hashes []string) ([]* FROM chunks WHERE chunk_hash IN (` - args := make([]interface{}, len(hashes)) + args := make([]any, len(hashes)) + + var querySb75 strings.Builder + for i, hash := range hashes { if i > 0 { - query += ", " + querySb75.WriteString(", ") } - query += "?" + + querySb75.WriteString("?") + args[i] = hash } + + query += querySb75.String() + query += ") ORDER BY chunk_hash" rows, err := r.db.conn.QueryContext(ctx, query, args...) @@ -88,6 +99,7 @@ func (r *ChunkRepository) GetByHashes(ctx context.Context, hashes []string) ([]* defer CloseRows(rows) var chunks []*Chunk + for rows.Next() { var chunk Chunk @@ -122,6 +134,7 @@ func (r *ChunkRepository) ListUnpacked(ctx context.Context, limit int) ([]*Chunk defer CloseRows(rows) var chunks []*Chunk + for rows.Next() { var chunk Chunk diff --git a/internal/database/chunks_ext.go b/internal/database/chunks_ext.go index b38c170..a322a9a 100644 --- a/internal/database/chunks_ext.go +++ b/internal/database/chunks_ext.go @@ -19,6 +19,7 @@ func (r *ChunkRepository) List(ctx context.Context) ([]*Chunk, error) { defer CloseRows(rows) var chunks []*Chunk + for rows.Next() { var chunk Chunk diff --git a/internal/database/chunks_test.go b/internal/database/chunks_test.go index 5122bb1..8f96bd1 100644 --- a/internal/database/chunks_test.go +++ b/internal/database/chunks_test.go @@ -30,12 +30,15 @@ func TestChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get chunk: %v", err) } + if retrieved == nil { t.Fatal("expected chunk, got nil") } + if retrieved.ChunkHash != chunk.ChunkHash { t.Errorf("chunk hash mismatch: got %s, want %s", retrieved.ChunkHash, chunk.ChunkHash) } + if retrieved.Size != chunk.Size { t.Errorf("size mismatch: got %d, want %d", retrieved.Size, chunk.Size) } @@ -51,6 +54,7 @@ func TestChunkRepository(t *testing.T) { ChunkHash: types.ChunkHash("chunkhash456"), Size: 8192, } + err = repo.Create(ctx, nil, chunk2) if err != nil { t.Fatalf("failed to create second chunk: %v", err) @@ -60,6 +64,7 @@ func TestChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get chunks by hashes: %v", err) } + if len(chunks) != 2 { t.Errorf("expected 2 chunks, got %d", len(chunks)) } @@ -69,6 +74,7 @@ func TestChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to list unpacked chunks: %v", err) } + if len(unpacked) != 2 { t.Errorf("expected 2 unpacked chunks, got %d", len(unpacked)) } @@ -86,6 +92,7 @@ func TestChunkRepositoryNotFound(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + if chunk != nil { t.Error("expected nil for non-existent chunk") } @@ -95,6 +102,7 @@ func TestChunkRepositoryNotFound(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + if chunks != nil { t.Error("expected nil for empty hash list") } diff --git a/internal/database/database.go b/internal/database/database.go index 5f0f14c..7fc717f 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -57,8 +57,8 @@ func ParseMigrationVersion(filename string) (int, error) { // 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] + if before, _, ok := strings.Cut(name, "_"); ok { + versionStr = before } if versionStr == "" { @@ -98,6 +98,7 @@ func New(ctx context.Context, path string) (*DB, error) { // First attempt with standard WAL mode log.Debug("Attempting to open database with WAL mode", "path", path) + conn, err := sql.Open( "sqlite", path+"?_journal_mode=WAL&_synchronous=NORMAL&_busy_timeout=10000&_locking_mode=NORMAL&_foreign_keys=ON", @@ -110,7 +111,8 @@ func New(ctx context.Context, path string) (*DB, error) { conn.SetMaxOpenConns(1) conn.SetMaxIdleConns(1) - if err := conn.PingContext(ctx); err == nil { + err := conn.PingContext(ctx) + if err == nil { // Success on first try log.Debug("Database opened successfully with WAL mode", "path", path) @@ -120,13 +122,19 @@ func New(ctx context.Context, path string) (*DB, error) { } db := &DB{conn: conn, path: path} - if err := applyMigrations(ctx, conn); err != nil { + + err := applyMigrations(ctx, conn) + if err != nil { _ = conn.Close() + return nil, fmt.Errorf("applying migrations: %w", err) } + return db, nil } + log.Debug("Failed to ping database, closing connection", "path", path, "error", err) + _ = conn.Close() } @@ -135,6 +143,7 @@ func New(ctx context.Context, path string) (*DB, error) { "Database appears locked, attempting recovery with TRUNCATE mode", "path", path, ) + conn, err = sql.Open( "sqlite", path+"?_journal_mode=TRUNCATE&_synchronous=NORMAL&_busy_timeout=10000&_foreign_keys=ON", @@ -152,7 +161,9 @@ func New(ctx context.Context, path string) (*DB, error) { if err := conn.PingContext(ctx); err != nil { log.Debug("Failed to ping database in recovery mode, closing", "path", path, "error", err) + _ = conn.Close() + return nil, fmt.Errorf( "database still locked after recovery attempt: %w", err, @@ -163,6 +174,7 @@ func New(ctx context.Context, path string) (*DB, error) { // Switch back to WAL mode log.Debug("Switching database back to WAL mode", "path", path) + if _, err := conn.ExecContext(ctx, "PRAGMA journal_mode=WAL"); err != nil { log.Warn("Failed to switch back to WAL mode", "path", path, "error", err) } @@ -175,10 +187,12 @@ func New(ctx context.Context, path string) (*DB, error) { db := &DB{conn: conn, path: path} if err := applyMigrations(ctx, conn); err != nil { _ = conn.Close() + return nil, fmt.Errorf("applying migrations: %w", err) } log.Debug("Database connection established successfully", "path", path) + return db, nil } @@ -187,11 +201,16 @@ func New(ctx context.Context, path string) (*DB, error) { // Returns an error if the database connection cannot be closed properly. func (db *DB) Close() error { log.Debug("Closing database connection", "path", db.path) - if err := db.conn.Close(); err != nil { + + err := db.conn.Close() + if err != nil { log.Error("Failed to close database", "path", db.path, "error", err) + return fmt.Errorf("failed to close database: %w", err) } + log.Debug("Database connection closed successfully", "path", db.path) + return nil } @@ -227,9 +246,10 @@ func (db *DB) BeginTx( func (db *DB) ExecWithLog( ctx context.Context, query string, - args ...interface{}, + args ...any, ) (sql.Result, error) { LogSQL("Execute", query, args...) + return db.conn.ExecContext(ctx, query, args...) } @@ -240,9 +260,10 @@ func (db *DB) ExecWithLog( func (db *DB) QueryRowWithLog( ctx context.Context, query string, - args ...interface{}, + args ...any, ) *sql.Row { LogSQL("QueryRow", query, args...) + return db.conn.QueryRowContext(ctx, query, args...) } @@ -375,6 +396,7 @@ func repeatPlaceholder(n int) string { if n <= 0 { return "" } + return strings.Repeat(", ?", n) } @@ -385,7 +407,7 @@ func repeatPlaceholder(n int) string { // The operation parameter describes the type of SQL operation (e.g., "Execute", "Query"). // The query parameter is the SQL statement being executed. // The args parameter contains the query arguments that will be interpolated. -func LogSQL(operation, query string, args ...interface{}) { +func LogSQL(operation, query string, args ...any) { if strings.Contains(os.Getenv("GODEBUG"), "vaultik") { log.Debug( "SQL "+operation, diff --git a/internal/database/database_test.go b/internal/database/database_test.go index 6d763a3..7700ac1 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -17,7 +17,8 @@ func TestDatabase(t *testing.T) { t.Fatalf("failed to create database: %v", err) } defer func() { - if err := db.Close(); err != nil { + err := db.Close() + if err != nil { t.Errorf("failed to close database: %v", err) } }() @@ -37,6 +38,7 @@ func TestDatabase(t *testing.T) { for _, table := range tables { var name string + err := db.conn.QueryRow("SELECT name FROM sqlite_master WHERE type='table' AND name=?", table).Scan(&name) if err != nil { t.Errorf("table %s does not exist: %v", table, err) @@ -63,7 +65,8 @@ func TestDatabaseConcurrentAccess(t *testing.T) { t.Fatalf("failed to create database: %v", err) } defer func() { - if err := db.Close(); err != nil { + err := db.Close() + if err != nil { t.Errorf("failed to close database: %v", err) } }() @@ -73,9 +76,10 @@ func TestDatabaseConcurrentAccess(t *testing.T) { index int err error } + results := make(chan result, 10) - for i := 0; i < 10; i++ { + for i := range 10 { go func(i int) { _, err := db.ExecWithLog(ctx, "INSERT INTO chunks (chunk_hash, size) VALUES (?, ?)", fmt.Sprintf("hash%d", i), i*1024) @@ -84,7 +88,7 @@ func TestDatabaseConcurrentAccess(t *testing.T) { } // Wait for all goroutines and check results - for i := 0; i < 10; i++ { + for range 10 { r := <-results if r.err != nil { t.Fatalf("concurrent insert %d failed: %v", r.index, r.err) @@ -93,10 +97,12 @@ func TestDatabaseConcurrentAccess(t *testing.T) { // Verify all inserts succeeded var count int + err = db.conn.QueryRowContext(ctx, "SELECT COUNT(*) FROM chunks").Scan(&count) if err != nil { t.Fatalf("failed to count chunks: %v", err) } + if count != 10 { t.Errorf("expected 10 chunks, got %d", count) } @@ -127,12 +133,16 @@ func TestParseMigrationVersion(t *testing.T) { if err == nil { t.Errorf("ParseMigrationVersion(%q) = %d, nil; want error", tc.filename, got) } + return } + if err != nil { t.Errorf("ParseMigrationVersion(%q) unexpected error: %v", tc.filename, err) + return } + if got != tc.wantVer { t.Errorf("ParseMigrationVersion(%q) = %d; want %d", tc.filename, got, tc.wantVer) } @@ -148,7 +158,8 @@ func TestApplyMigrations_Idempotent(t *testing.T) { t.Fatalf("failed to open database: %v", err) } defer func() { - if err := conn.Close(); err != nil { + err := conn.Close() + if err != nil { t.Errorf("failed to close database: %v", err) } }() @@ -191,7 +202,8 @@ func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) { t.Fatalf("failed to open database: %v", err) } defer func() { - if err := conn.Close(); err != nil { + err := conn.Close() + if err != nil { t.Errorf("failed to close database: %v", err) } }() @@ -206,6 +218,7 @@ func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) { ).Scan(&tableBefore); err != nil { t.Fatalf("failed to check for table before bootstrap: %v", err) } + if tableBefore != 0 { t.Fatal("schema_migrations table should not exist before bootstrap") } @@ -222,6 +235,7 @@ func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) { ).Scan(&tableAfter); err != nil { t.Fatalf("failed to check for table after bootstrap: %v", err) } + if tableAfter != 1 { t.Fatalf("schema_migrations table should exist after bootstrap, got count=%d", tableAfter) } @@ -233,6 +247,7 @@ func TestBootstrapMigrationsTable_FreshDatabase(t *testing.T) { ).Scan(&version); err != nil { t.Fatalf("version 0 row not found in schema_migrations: %v", err) } + if version != 0 { t.Errorf("expected version 0, got %d", version) } diff --git a/internal/database/errors.go b/internal/database/errors.go index b2b4497..59f1279 100644 --- a/internal/database/errors.go +++ b/internal/database/errors.go @@ -7,14 +7,15 @@ import ( ) // Fatal prints an error message to stderr and exits with status 1 -func Fatal(format string, args ...interface{}) { +func Fatal(format string, args ...any) { fmt.Fprintf(os.Stderr, "FATAL: "+format+"\n", args...) os.Exit(1) } // CloseRows closes rows and exits on error func CloseRows(rows *sql.Rows) { - if err := rows.Close(); err != nil { + err := rows.Close() + if err != nil { Fatal("failed to close rows: %v", err) } } diff --git a/internal/database/file_chunks.go b/internal/database/file_chunks.go index 31fa142..c6b32f4 100644 --- a/internal/database/file_chunks.go +++ b/internal/database/file_chunks.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "strings" "sneak.berlin/go/vaultik/internal/types" ) @@ -84,6 +85,7 @@ func (r *FileChunkRepository) GetByPathTx(ctx context.Context, tx *sql.Tx, path ` LogSQL("GetByPathTx", query, path) + rows, err := tx.QueryContext(ctx, query, path) if err != nil { return nil, fmt.Errorf("querying file chunks: %w", err) @@ -92,23 +94,30 @@ func (r *FileChunkRepository) GetByPathTx(ctx context.Context, tx *sql.Tx, path fileChunks, err := r.scanFileChunks(rows) LogSQL("GetByPathTx", "Complete", path, "count", len(fileChunks)) + return fileChunks, err } // scanFileChunks is a helper that scans file chunk rows func (r *FileChunkRepository) scanFileChunks(rows *sql.Rows) ([]*FileChunk, error) { var fileChunks []*FileChunk + for rows.Next() { - var fc FileChunk - var fileIDStr, chunkHashStr string + var ( + fc FileChunk + fileIDStr, chunkHashStr string + ) + err := rows.Scan(&fileIDStr, &fc.Idx, &chunkHashStr) if err != nil { return nil, fmt.Errorf("scanning file chunk: %w", err) } + fc.FileID, err = types.ParseFileID(fileIDStr) if err != nil { return nil, fmt.Errorf("parsing file ID: %w", err) } + fc.ChunkHash = types.ChunkHash(chunkHashStr) fileChunks = append(fileChunks, &fc) } @@ -161,14 +170,13 @@ func (r *FileChunkRepository) DeleteByFileIDs(ctx context.Context, tx *sql.Tx, f const batchSize = 500 for i := 0; i < len(fileIDs); i += batchSize { - end := i + batchSize - if end > len(fileIDs) { - end = len(fileIDs) - } + end := min(i+batchSize, len(fileIDs)) + batch := fileIDs[i:end] query := "DELETE FROM file_chunks WHERE file_id IN (?" + repeatPlaceholder(len(batch)-1) + ")" - args := make([]interface{}, len(batch)) + + args := make([]any, len(batch)) for j, id := range batch { args[j] = id.String() } @@ -179,6 +187,7 @@ func (r *FileChunkRepository) DeleteByFileIDs(ctx context.Context, tx *sql.Tx, f } else { _, err = r.db.ExecWithLog(ctx, query, args...) } + if err != nil { return fmt.Errorf("batch deleting file_chunks: %w", err) } @@ -199,22 +208,29 @@ func (r *FileChunkRepository) CreateBatch(ctx context.Context, tx *sql.Tx, fcs [ const batchSize = 300 for i := 0; i < len(fcs); i += batchSize { - end := i + batchSize - if end > len(fcs) { - end = len(fcs) - } + end := min(i+batchSize, len(fcs)) + batch := fcs[i:end] // Build the query with multiple value sets query := "INSERT INTO file_chunks (file_id, idx, chunk_hash) VALUES " - args := make([]interface{}, 0, len(batch)*3) + + args := make([]any, 0, len(batch)*3) + + var querySb211 strings.Builder + for j, fc := range batch { if j > 0 { - query += ", " + querySb211.WriteString(", ") } - query += "(?, ?, ?)" + + querySb211.WriteString("(?, ?, ?)") + args = append(args, fc.FileID.String(), fc.Idx, fc.ChunkHash.String()) } + + query += querySb211.String() + query += " ON CONFLICT(file_id, idx) DO NOTHING" var err error @@ -223,6 +239,7 @@ func (r *FileChunkRepository) CreateBatch(ctx context.Context, tx *sql.Tx, fcs [ } else { _, err = r.db.ExecWithLog(ctx, query, args...) } + if err != nil { return fmt.Errorf("batch inserting file_chunks: %w", err) } @@ -236,6 +253,7 @@ func (r *FileChunkRepository) GetByFile(ctx context.Context, path string) ([]*Fi LogSQL("GetByFile", "Starting", path) result, err := r.GetByPath(ctx, path) LogSQL("GetByFile", "Complete", path, "count", len(result)) + return result, err } @@ -244,5 +262,6 @@ func (r *FileChunkRepository) GetByFileTx(ctx context.Context, tx *sql.Tx, path LogSQL("GetByFileTx", "Starting", path) result, err := r.GetByPathTx(ctx, tx, path) LogSQL("GetByFileTx", "Complete", path, "count", len(result)) + return result, err } diff --git a/internal/database/file_chunks_test.go b/internal/database/file_chunks_test.go index ebd2e01..e1eb2b9 100644 --- a/internal/database/file_chunks_test.go +++ b/internal/database/file_chunks_test.go @@ -28,6 +28,7 @@ func TestFileChunkRepository(t *testing.T) { GID: 1000, LinkTarget: "", } + err := fileRepo.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file: %v", err) @@ -36,11 +37,13 @@ func TestFileChunkRepository(t *testing.T) { // Create chunks first chunks := []types.ChunkHash{"chunk1", "chunk2", "chunk3"} chunkRepo := NewChunkRepository(db) + for _, chunkHash := range chunks { chunk := &Chunk{ ChunkHash: chunkHash, Size: 1024, } + err = chunkRepo.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk %s: %v", chunkHash, err) @@ -65,6 +68,7 @@ func TestFileChunkRepository(t *testing.T) { Idx: 1, ChunkHash: types.ChunkHash("chunk2"), } + err = repo.Create(ctx, nil, fc2) if err != nil { t.Fatalf("failed to create second file chunk: %v", err) @@ -75,6 +79,7 @@ func TestFileChunkRepository(t *testing.T) { Idx: 2, ChunkHash: types.ChunkHash("chunk3"), } + err = repo.Create(ctx, nil, fc3) if err != nil { t.Fatalf("failed to create third file chunk: %v", err) @@ -85,6 +90,7 @@ func TestFileChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get file chunks: %v", err) } + if len(fileChunks) != 3 { t.Errorf("expected 3 chunks, got %d", len(fileChunks)) } @@ -112,6 +118,7 @@ func TestFileChunkRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get deleted file chunks: %v", err) } + if len(fileChunks) != 0 { t.Errorf("expected 0 chunks after delete, got %d", len(fileChunks)) } @@ -140,22 +147,26 @@ func TestFileChunkRepositoryMultipleFiles(t *testing.T) { GID: 1000, LinkTarget: "", } + err := fileRepo.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file %s: %v", path, err) } + files[i] = file } // Create all chunks first chunkRepo := NewChunkRepository(db) + for i := range files { - for j := 0; j < 2; j++ { + for j := range 2 { chunkHash := types.ChunkHash(fmt.Sprintf("file%d_chunk%d", i, j)) chunk := &Chunk{ ChunkHash: chunkHash, Size: 1024, } + err := chunkRepo.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk %s: %v", chunkHash, err) @@ -165,12 +176,13 @@ func TestFileChunkRepositoryMultipleFiles(t *testing.T) { // Create chunks for multiple files for i, file := range files { - for j := 0; j < 2; j++ { + for j := range 2 { fc := &FileChunk{ FileID: file.ID, Idx: j, ChunkHash: types.ChunkHash(fmt.Sprintf("file%d_chunk%d", i, j)), } + err := repo.Create(ctx, nil, fc) if err != nil { t.Fatalf("failed to create file chunk: %v", err) @@ -184,6 +196,7 @@ func TestFileChunkRepositoryMultipleFiles(t *testing.T) { if err != nil { t.Fatalf("failed to get chunks for file %d: %v", i, err) } + if len(chunks) != 2 { t.Errorf("expected 2 chunks for file %d, got %d", i, len(chunks)) } diff --git a/internal/database/files.go b/internal/database/files.go index bf81e18..77ec1ec 100644 --- a/internal/database/files.go +++ b/internal/database/files.go @@ -3,7 +3,9 @@ package database import ( "context" "database/sql" + "errors" "fmt" + "strings" "time" "sneak.berlin/go/vaultik/internal/log" @@ -38,8 +40,11 @@ func (r *FileRepository) Create(ctx context.Context, tx *sql.Tx, file *File) err RETURNING id ` - var idStr string - var err error + var ( + idStr string + err error + ) + if tx != nil { LogSQL("Execute", query, file.ID.String(), file.Path.String(), file.SourcePath.String(), file.MTime.Unix(), file.Size, file.Mode, file.UID, file.GID, file.LinkTarget.String()) err = tx.QueryRowContext(ctx, query, file.ID.String(), file.Path.String(), file.SourcePath.String(), file.MTime.Unix(), file.Size, file.Mode, file.UID, file.GID, file.LinkTarget.String()).Scan(&idStr) @@ -68,9 +73,10 @@ func (r *FileRepository) GetByPath(ctx context.Context, path string) (*File, err ` file, err := r.scanFile(r.db.conn.QueryRowContext(ctx, query, path)) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying file: %w", err) } @@ -87,9 +93,10 @@ func (r *FileRepository) GetByID(ctx context.Context, id types.FileID) (*File, e ` file, err := r.scanFile(r.db.conn.QueryRowContext(ctx, query, id.String())) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying file: %w", err) } @@ -108,9 +115,10 @@ func (r *FileRepository) GetByPathTx(ctx context.Context, tx *sql.Tx, path strin file, err := r.scanFile(tx.QueryRowContext(ctx, query, path)) LogSQL("GetByPathTx Scan complete", query, path) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying file: %w", err) } @@ -120,10 +128,12 @@ func (r *FileRepository) GetByPathTx(ctx context.Context, tx *sql.Tx, path strin // scanFile is a helper that scans a single file row func (r *FileRepository) scanFile(row *sql.Row) (*File, error) { - var file File - var idStr, pathStr, sourcePathStr string - var mtimeUnix int64 - var linkTarget sql.NullString + var ( + file File + idStr, pathStr, sourcePathStr string + mtimeUnix int64 + linkTarget sql.NullString + ) err := row.Scan( &idStr, @@ -144,8 +154,10 @@ func (r *FileRepository) scanFile(row *sql.Row) (*File, error) { if err != nil { return nil, fmt.Errorf("parsing file ID: %w", err) } + file.Path = types.FilePath(pathStr) file.SourcePath = types.SourcePath(sourcePathStr) + file.MTime = time.Unix(mtimeUnix, 0).UTC() if linkTarget.Valid { file.LinkTarget = types.FilePath(linkTarget.String) @@ -156,10 +168,12 @@ func (r *FileRepository) scanFile(row *sql.Row) (*File, error) { // scanFileRows is a helper that scans a file row from rows iterator func (r *FileRepository) scanFileRows(rows *sql.Rows) (*File, error) { - var file File - var idStr, pathStr, sourcePathStr string - var mtimeUnix int64 - var linkTarget sql.NullString + var ( + file File + idStr, pathStr, sourcePathStr string + mtimeUnix int64 + linkTarget sql.NullString + ) err := rows.Scan( &idStr, @@ -180,8 +194,10 @@ func (r *FileRepository) scanFileRows(rows *sql.Rows) (*File, error) { if err != nil { return nil, fmt.Errorf("parsing file ID: %w", err) } + file.Path = types.FilePath(pathStr) file.SourcePath = types.SourcePath(sourcePathStr) + file.MTime = time.Unix(mtimeUnix, 0).UTC() if linkTarget.Valid { file.LinkTarget = types.FilePath(linkTarget.String) @@ -205,11 +221,13 @@ func (r *FileRepository) ListModifiedSince(ctx context.Context, since time.Time) defer CloseRows(rows) var files []*File + for rows.Next() { file, err := r.scanFileRows(rows) if err != nil { return nil, fmt.Errorf("scanning file: %w", err) } + files = append(files, file) } @@ -266,11 +284,13 @@ func (r *FileRepository) ListByPrefix(ctx context.Context, prefix string) ([]*Fi defer CloseRows(rows) var files []*File + for rows.Next() { file, err := r.scanFileRows(rows) if err != nil { return nil, fmt.Errorf("scanning file: %w", err) } + files = append(files, file) } @@ -292,11 +312,13 @@ func (r *FileRepository) ListAll(ctx context.Context) ([]*File, error) { defer CloseRows(rows) var files []*File + for rows.Next() { file, err := r.scanFileRows(rows) if err != nil { return nil, fmt.Errorf("scanning file: %w", err) } + files = append(files, file) } @@ -314,21 +336,28 @@ func (r *FileRepository) CreateBatch(ctx context.Context, tx *sql.Tx, files []*F const batchSize = 100 for i := 0; i < len(files); i += batchSize { - end := i + batchSize - if end > len(files) { - end = len(files) - } + end := min(i+batchSize, len(files)) + batch := files[i:end] query := `INSERT INTO files (id, path, source_path, mtime, size, mode, uid, gid, link_target) VALUES ` - args := make([]interface{}, 0, len(batch)*9) + + args := make([]any, 0, len(batch)*9) + + var querySb325 strings.Builder + for j, f := range batch { if j > 0 { - query += ", " + querySb325.WriteString(", ") } - query += "(?, ?, ?, ?, ?, ?, ?, ?, ?)" + + querySb325.WriteString("(?, ?, ?, ?, ?, ?, ?, ?, ?)") + args = append(args, f.ID.String(), f.Path.String(), f.SourcePath.String(), f.MTime.Unix(), f.Size, f.Mode, f.UID, f.GID, f.LinkTarget.String()) } + + query += querySb325.String() + query += ` ON CONFLICT(path) DO UPDATE SET source_path = excluded.source_path, mtime = excluded.mtime, @@ -344,6 +373,7 @@ func (r *FileRepository) CreateBatch(ctx context.Context, tx *sql.Tx, files []*F } else { _, err = r.db.ExecWithLog(ctx, query, args...) } + if err != nil { return fmt.Errorf("batch inserting files: %w", err) } diff --git a/internal/database/files_test.go b/internal/database/files_test.go index 8f16421..7f4b472 100644 --- a/internal/database/files_test.go +++ b/internal/database/files_test.go @@ -3,7 +3,7 @@ package database import ( "context" "database/sql" - "fmt" + "errors" "os" "path/filepath" "testing" @@ -20,7 +20,8 @@ func setupTestDB(t *testing.T) (*DB, func()) { } cleanup := func() { - if err := db.Close(); err != nil { + err := db.Close() + if err != nil { t.Errorf("failed to close database: %v", err) } } @@ -56,18 +57,23 @@ func TestFileRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get file: %v", err) } + if retrieved == nil { t.Fatal("expected file, got nil") } + if retrieved.Path != file.Path { t.Errorf("path mismatch: got %s, want %s", retrieved.Path, file.Path) } + if !retrieved.MTime.Equal(file.MTime) { t.Errorf("mtime mismatch: got %v, want %v", retrieved.MTime, file.MTime) } + if retrieved.Size != file.Size { t.Errorf("size mismatch: got %d, want %d", retrieved.Size, file.Size) } + if retrieved.Mode != file.Mode { t.Errorf("mode mismatch: got %o, want %o", retrieved.Mode, file.Mode) } @@ -75,6 +81,7 @@ func TestFileRepository(t *testing.T) { // Test Update (upsert) file.Size = 2048 file.MTime = time.Now().Truncate(time.Second) + err = repo.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to update file: %v", err) @@ -84,6 +91,7 @@ func TestFileRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get updated file: %v", err) } + if retrieved.Size != 2048 { t.Errorf("size not updated: got %d, want %d", retrieved.Size, 2048) } @@ -93,6 +101,7 @@ func TestFileRepository(t *testing.T) { if err != nil { t.Fatalf("failed to list files: %v", err) } + if len(files) != 1 { t.Errorf("expected 1 file, got %d", len(files)) } @@ -107,6 +116,7 @@ func TestFileRepository(t *testing.T) { if err != nil { t.Fatalf("error getting deleted file: %v", err) } + if retrieved != nil { t.Error("expected nil for deleted file") } @@ -139,9 +149,11 @@ func TestFileRepositorySymlink(t *testing.T) { if err != nil { t.Fatalf("failed to get symlink: %v", err) } + if !retrieved.IsSymlink() { t.Error("expected IsSymlink() to be true") } + if retrieved.LinkTarget != symlink.LinkTarget { t.Errorf("link target mismatch: got %s, want %s", retrieved.LinkTarget, symlink.LinkTarget) } @@ -165,12 +177,13 @@ func TestFileRepositoryTransaction(t *testing.T) { GID: 1000, } - if err := repos.Files.Create(ctx, tx, file); err != nil { + err := repos.Files.Create(ctx, tx, file) + if err != nil { return err } // Return error to trigger rollback - return fmt.Errorf("test rollback") + return errors.New("test rollback") }) if err == nil || err.Error() != "test rollback" { @@ -182,6 +195,7 @@ func TestFileRepositoryTransaction(t *testing.T) { if err != nil { t.Fatalf("error checking for file: %v", err) } + if retrieved != nil { t.Error("file should not exist after rollback") } diff --git a/internal/database/local_meta.go b/internal/database/local_meta.go index cf92ff5..83c2048 100644 --- a/internal/database/local_meta.go +++ b/internal/database/local_meta.go @@ -27,15 +27,18 @@ func NewLocalMetaRepository(db *DB) *LocalMetaRepository { // "unset" (bind on first use) from "set to something" (compare). func (r *LocalMetaRepository) Get(ctx context.Context, key string) (string, error) { var value string + err := r.db.conn.QueryRowContext(ctx, "SELECT value FROM local_meta WHERE key = ?", key, ).Scan(&value) if errors.Is(err, sql.ErrNoRows) { return "", nil } + if err != nil { return "", fmt.Errorf("reading local_meta %q: %w", key, err) } + return value, nil } @@ -49,5 +52,6 @@ func (r *LocalMetaRepository) Set(ctx context.Context, key, value string) error if err != nil { return fmt.Errorf("writing local_meta %q: %w", key, err) } + return nil } diff --git a/internal/database/local_meta_test.go b/internal/database/local_meta_test.go index b2be08d..d9b1d64 100644 --- a/internal/database/local_meta_test.go +++ b/internal/database/local_meta_test.go @@ -11,18 +11,20 @@ import ( func TestLocalMetaEmptyOnFresh(t *testing.T) { db, err := database.NewTestDB() require.NoError(t, err) + defer func() { _ = db.Close() }() repos := database.NewRepositories(db) got, err := repos.LocalMeta.Get(context.Background(), database.LocalMetaKeyStorageURL) require.NoError(t, err) - require.Equal(t, "", got, "fresh DB must return empty for unset keys, not error") + require.Empty(t, got, "fresh DB must return empty for unset keys, not error") } func TestLocalMetaSetGetRoundTrip(t *testing.T) { db, err := database.NewTestDB() require.NoError(t, err) + defer func() { _ = db.Close() }() repos := database.NewRepositories(db) @@ -38,6 +40,7 @@ func TestLocalMetaSetGetRoundTrip(t *testing.T) { func TestLocalMetaSetOverwrites(t *testing.T) { db, err := database.NewTestDB() require.NoError(t, err) + defer func() { _ = db.Close() }() repos := database.NewRepositories(db) diff --git a/internal/database/module.go b/internal/database/module.go index 98a5b74..e4af208 100644 --- a/internal/database/module.go +++ b/internal/database/module.go @@ -34,11 +34,16 @@ func provideDatabase(lc fx.Lifecycle, cfg *config.Config) (*DB, error) { lc.Append(fx.Hook{ OnStop: func(ctx context.Context) error { log.Debug("Database module OnStop hook called") - if err := db.Close(); err != nil { + + err := db.Close() + if err != nil { log.Error("Failed to close database in OnStop hook", "error", err) + return err } + log.Debug("Database closed successfully in OnStop hook") + return nil }, }) diff --git a/internal/database/repositories.go b/internal/database/repositories.go index 8bcf2e7..07a1bb0 100644 --- a/internal/database/repositories.go +++ b/internal/database/repositories.go @@ -50,20 +50,25 @@ type TxFunc func(ctx context.Context, tx *sql.Tx) error // This method should be used for all write operations to ensure atomicity. func (r *Repositories) WithTx(ctx context.Context, fn TxFunc) error { LogSQL("WithTx", "Beginning transaction", "") + tx, err := r.db.BeginTx(ctx, nil) if err != nil { return fmt.Errorf("beginning transaction: %w", err) } + LogSQL("WithTx", "Transaction started", "") defer func() { if p := recover(); p != nil { - if rollbackErr := tx.Rollback(); rollbackErr != nil { + rollbackErr := tx.Rollback() + if rollbackErr != nil { Fatal("failed to rollback transaction: %v", rollbackErr) } + panic(p) } else if err != nil { - if rollbackErr := tx.Rollback(); rollbackErr != nil { + rollbackErr := tx.Rollback() + if rollbackErr != nil { Fatal("failed to rollback transaction: %v", rollbackErr) } } @@ -90,6 +95,7 @@ func (r *Repositories) WithReadTx(ctx context.Context, fn TxFunc) error { opts := &sql.TxOptions{ ReadOnly: true, } + tx, err := r.db.BeginTx(ctx, opts) if err != nil { return fmt.Errorf("beginning read transaction: %w", err) @@ -97,12 +103,15 @@ func (r *Repositories) WithReadTx(ctx context.Context, fn TxFunc) error { defer func() { if p := recover(); p != nil { - if rollbackErr := tx.Rollback(); rollbackErr != nil { + rollbackErr := tx.Rollback() + if rollbackErr != nil { Fatal("failed to rollback transaction: %v", rollbackErr) } + panic(p) } else if err != nil { - if rollbackErr := tx.Rollback(); rollbackErr != nil { + rollbackErr := tx.Rollback() + if rollbackErr != nil { Fatal("failed to rollback transaction: %v", rollbackErr) } } diff --git a/internal/database/repositories_test.go b/internal/database/repositories_test.go index 6324824..485ba61 100644 --- a/internal/database/repositories_test.go +++ b/internal/database/repositories_test.go @@ -3,7 +3,7 @@ package database import ( "context" "database/sql" - "fmt" + "errors" "testing" "time" @@ -28,7 +28,9 @@ func TestRepositoriesTransaction(t *testing.T) { UID: 1000, GID: 1000, } - if err := repos.Files.Create(ctx, tx, file); err != nil { + + err := repos.Files.Create(ctx, tx, file) + if err != nil { return err } @@ -37,7 +39,9 @@ func TestRepositoriesTransaction(t *testing.T) { ChunkHash: types.ChunkHash("tx_chunk1"), Size: 512, } - if err := repos.Chunks.Create(ctx, tx, chunk1); err != nil { + + err = repos.Chunks.Create(ctx, tx, chunk1) + if err != nil { return err } @@ -45,7 +49,9 @@ func TestRepositoriesTransaction(t *testing.T) { ChunkHash: types.ChunkHash("tx_chunk2"), Size: 512, } - if err := repos.Chunks.Create(ctx, tx, chunk2); err != nil { + + err = repos.Chunks.Create(ctx, tx, chunk2) + if err != nil { return err } @@ -55,7 +61,9 @@ func TestRepositoriesTransaction(t *testing.T) { Idx: 0, ChunkHash: chunk1.ChunkHash, } - if err := repos.FileChunks.Create(ctx, tx, fc1); err != nil { + + err = repos.FileChunks.Create(ctx, tx, fc1) + if err != nil { return err } @@ -64,7 +72,9 @@ func TestRepositoriesTransaction(t *testing.T) { Idx: 1, ChunkHash: chunk2.ChunkHash, } - if err := repos.FileChunks.Create(ctx, tx, fc2); err != nil { + + err = repos.FileChunks.Create(ctx, tx, fc2) + if err != nil { return err } @@ -74,7 +84,9 @@ func TestRepositoriesTransaction(t *testing.T) { Hash: types.BlobHash("tx_blob1"), CreatedTS: time.Now().Truncate(time.Second), } - if err := repos.Blobs.Create(ctx, tx, blob); err != nil { + + err = repos.Blobs.Create(ctx, tx, blob) + if err != nil { return err } @@ -85,7 +97,9 @@ func TestRepositoriesTransaction(t *testing.T) { Offset: 0, Length: 512, } - if err := repos.BlobChunks.Create(ctx, tx, bc1); err != nil { + + err = repos.BlobChunks.Create(ctx, tx, bc1) + if err != nil { return err } @@ -95,13 +109,14 @@ func TestRepositoriesTransaction(t *testing.T) { Offset: 512, Length: 512, } - if err := repos.BlobChunks.Create(ctx, tx, bc2); err != nil { + + err = repos.BlobChunks.Create(ctx, tx, bc2) + if err != nil { return err } return nil }) - if err != nil { t.Fatalf("transaction failed: %v", err) } @@ -111,6 +126,7 @@ func TestRepositoriesTransaction(t *testing.T) { if err != nil { t.Fatalf("failed to get file: %v", err) } + if file == nil { t.Error("expected file after transaction") } @@ -119,6 +135,7 @@ func TestRepositoriesTransaction(t *testing.T) { if err != nil { t.Fatalf("failed to get file chunks: %v", err) } + if len(chunks) != 2 { t.Errorf("expected 2 file chunks, got %d", len(chunks)) } @@ -127,6 +144,7 @@ func TestRepositoriesTransaction(t *testing.T) { if err != nil { t.Fatalf("failed to get blob: %v", err) } + if blob == nil { t.Error("expected blob after transaction") } @@ -150,7 +168,9 @@ func TestRepositoriesTransactionRollback(t *testing.T) { UID: 1000, GID: 1000, } - if err := repos.Files.Create(ctx, tx, file); err != nil { + + err := repos.Files.Create(ctx, tx, file) + if err != nil { return err } @@ -159,12 +179,14 @@ func TestRepositoriesTransactionRollback(t *testing.T) { ChunkHash: types.ChunkHash("rollback_chunk"), Size: 1024, } - if err := repos.Chunks.Create(ctx, tx, chunk); err != nil { + + err = repos.Chunks.Create(ctx, tx, chunk) + if err != nil { return err } // Return error to trigger rollback - return fmt.Errorf("intentional rollback") + return errors.New("intentional rollback") }) if err == nil || err.Error() != "intentional rollback" { @@ -176,6 +198,7 @@ func TestRepositoriesTransactionRollback(t *testing.T) { if err != nil { t.Fatalf("error checking for file: %v", err) } + if file != nil { t.Error("file should not exist after rollback") } @@ -184,6 +207,7 @@ func TestRepositoriesTransactionRollback(t *testing.T) { if err != nil { t.Fatalf("error checking for chunk: %v", err) } + if chunk != nil { t.Error("chunk should not exist after rollback") } @@ -205,6 +229,7 @@ func TestRepositoriesReadTransaction(t *testing.T) { UID: 1000, GID: 1000, } + err := repos.Files.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file: %v", err) @@ -212,8 +237,10 @@ func TestRepositoriesReadTransaction(t *testing.T) { // Test read-only transaction var retrievedFile *File + err = repos.WithReadTx(ctx, func(ctx context.Context, tx *sql.Tx) error { var err error + retrievedFile, err = repos.Files.GetByPathTx(ctx, tx, "/test/read_file.txt") if err != nil { return err @@ -232,7 +259,6 @@ func TestRepositoriesReadTransaction(t *testing.T) { return nil }) - if err != nil { t.Fatalf("read transaction failed: %v", err) } diff --git a/internal/database/repository_comprehensive_test.go b/internal/database/repository_comprehensive_test.go index 384f80a..f3fce80 100644 --- a/internal/database/repository_comprehensive_test.go +++ b/internal/database/repository_comprehensive_test.go @@ -3,6 +3,7 @@ package database import ( "context" "database/sql" + "errors" "fmt" "testing" "time" @@ -39,6 +40,7 @@ func TestFileRepositoryUUIDGeneration(t *testing.T) { } uuids := make(map[string]bool) + for _, file := range files { err := repo.Create(ctx, nil, file) if err != nil { @@ -54,6 +56,7 @@ func TestFileRepositoryUUIDGeneration(t *testing.T) { if uuids[file.ID.String()] { t.Errorf("duplicate UUID generated: %s", file.ID) } + uuids[file.ID.String()] = true } } @@ -90,16 +93,19 @@ func TestFileRepositoryGetByID(t *testing.T) { if retrieved.ID != file.ID { t.Errorf("ID mismatch: expected %s, got %s", file.ID, retrieved.ID) } + if retrieved.Path != file.Path { t.Errorf("Path mismatch: expected %s, got %s", file.Path, retrieved.Path) } // Test non-existent ID nonExistentID := types.NewFileID() // Generate a new UUID that won't exist in the database + nonExistent, err := repo.GetByID(ctx, nonExistentID) if err != nil { t.Fatalf("GetByID should not return error for non-existent ID: %v", err) } + if nonExistent != nil { t.Error("expected nil for non-existent ID") } @@ -135,6 +141,7 @@ func TestOrphanedFileCleanup(t *testing.T) { if err != nil { t.Fatalf("failed to create file1: %v", err) } + err = repos.Files.Create(ctx, nil, file2) if err != nil { t.Fatalf("failed to create file2: %v", err) @@ -146,6 +153,7 @@ func TestOrphanedFileCleanup(t *testing.T) { Hostname: "test-host", StartedAt: time.Now(), } + err = repos.Snapshots.Create(ctx, nil, snapshot) if err != nil { t.Fatalf("failed to create snapshot: %v", err) @@ -168,6 +176,7 @@ func TestOrphanedFileCleanup(t *testing.T) { if err != nil { t.Fatalf("error getting file: %v", err) } + if orphanedFile != nil { t.Error("orphaned file should have been deleted") } @@ -177,6 +186,7 @@ func TestOrphanedFileCleanup(t *testing.T) { if err != nil { t.Fatalf("error getting file: %v", err) } + if referencedFile == nil { t.Error("referenced file should not have been deleted") } @@ -204,6 +214,7 @@ func TestOrphanedChunkCleanup(t *testing.T) { if err != nil { t.Fatalf("failed to create chunk1: %v", err) } + err = repos.Chunks.Create(ctx, nil, chunk2) if err != nil { t.Fatalf("failed to create chunk2: %v", err) @@ -218,6 +229,7 @@ func TestOrphanedChunkCleanup(t *testing.T) { UID: 1000, GID: 1000, } + err = repos.Files.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file: %v", err) @@ -229,6 +241,7 @@ func TestOrphanedChunkCleanup(t *testing.T) { Idx: 0, ChunkHash: chunk2.ChunkHash, } + err = repos.FileChunks.Create(ctx, nil, fc) if err != nil { t.Fatalf("failed to create file chunk: %v", err) @@ -245,6 +258,7 @@ func TestOrphanedChunkCleanup(t *testing.T) { if err != nil { t.Fatalf("error getting chunk: %v", err) } + if orphanedChunk != nil { t.Error("orphaned chunk should have been deleted") } @@ -254,6 +268,7 @@ func TestOrphanedChunkCleanup(t *testing.T) { if err != nil { t.Fatalf("error getting chunk: %v", err) } + if referencedChunk == nil { t.Error("referenced chunk should not have been deleted") } @@ -283,6 +298,7 @@ func TestOrphanedBlobCleanup(t *testing.T) { if err != nil { t.Fatalf("failed to create blob1: %v", err) } + err = repos.Blobs.Create(ctx, nil, blob2) if err != nil { t.Fatalf("failed to create blob2: %v", err) @@ -294,6 +310,7 @@ func TestOrphanedBlobCleanup(t *testing.T) { Hostname: "test-host", StartedAt: time.Now(), } + err = repos.Snapshots.Create(ctx, nil, snapshot) if err != nil { t.Fatalf("failed to create snapshot: %v", err) @@ -316,6 +333,7 @@ func TestOrphanedBlobCleanup(t *testing.T) { if err != nil { t.Fatalf("error getting blob: %v", err) } + if orphanedBlob != nil { t.Error("orphaned blob should have been deleted") } @@ -325,6 +343,7 @@ func TestOrphanedBlobCleanup(t *testing.T) { if err != nil { t.Fatalf("error getting blob: %v", err) } + if referencedBlob == nil { t.Error("referenced blob should not have been deleted") } @@ -347,6 +366,7 @@ func TestFileChunkRepositoryWithUUIDs(t *testing.T) { UID: 1000, GID: 1000, } + err := repos.Files.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file: %v", err) @@ -359,6 +379,7 @@ func TestFileChunkRepositoryWithUUIDs(t *testing.T) { ChunkHash: chunkHash, Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk: %v", err) @@ -370,6 +391,7 @@ func TestFileChunkRepositoryWithUUIDs(t *testing.T) { Idx: i, ChunkHash: chunkHash, } + err = repos.FileChunks.Create(ctx, nil, fc) if err != nil { t.Fatalf("failed to create file chunk: %v", err) @@ -381,6 +403,7 @@ func TestFileChunkRepositoryWithUUIDs(t *testing.T) { if err != nil { t.Fatalf("failed to get file chunks: %v", err) } + if len(fileChunks) != 3 { t.Errorf("expected 3 chunks, got %d", len(fileChunks)) } @@ -395,6 +418,7 @@ func TestFileChunkRepositoryWithUUIDs(t *testing.T) { if err != nil { t.Fatalf("failed to get file chunks after delete: %v", err) } + if len(fileChunks) != 0 { t.Errorf("expected 0 chunks after delete, got %d", len(fileChunks)) } @@ -430,6 +454,7 @@ func TestChunkFileRepositoryWithUUIDs(t *testing.T) { if err != nil { t.Fatalf("failed to create file1: %v", err) } + err = repos.Files.Create(ctx, nil, file2) if err != nil { t.Fatalf("failed to create file2: %v", err) @@ -440,6 +465,7 @@ func TestChunkFileRepositoryWithUUIDs(t *testing.T) { ChunkHash: types.ChunkHash("shared-chunk"), Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk: %v", err) @@ -463,6 +489,7 @@ func TestChunkFileRepositoryWithUUIDs(t *testing.T) { if err != nil { t.Fatalf("failed to create chunk file 1: %v", err) } + err = repos.ChunkFiles.Create(ctx, nil, cf2) if err != nil { t.Fatalf("failed to create chunk file 2: %v", err) @@ -473,6 +500,7 @@ func TestChunkFileRepositoryWithUUIDs(t *testing.T) { if err != nil { t.Fatalf("failed to get chunk files: %v", err) } + if len(chunkFiles) != 2 { t.Errorf("expected 2 files for chunk, got %d", len(chunkFiles)) } @@ -482,6 +510,7 @@ func TestChunkFileRepositoryWithUUIDs(t *testing.T) { if err != nil { t.Fatalf("failed to get chunks by file ID: %v", err) } + if len(chunkFiles) != 1 { t.Errorf("expected 1 chunk for file, got %d", len(chunkFiles)) } @@ -528,15 +557,19 @@ func TestSnapshotRepositoryExtendedFields(t *testing.T) { if retrieved.VaultikVersion != snapshot.VaultikVersion { t.Errorf("version mismatch: expected %s, got %s", snapshot.VaultikVersion, retrieved.VaultikVersion) } + if retrieved.VaultikGitRevision != snapshot.VaultikGitRevision { t.Errorf("git revision mismatch: expected %s, got %s", snapshot.VaultikGitRevision, retrieved.VaultikGitRevision) } + if retrieved.CompressionLevel != snapshot.CompressionLevel { t.Errorf("compression level mismatch: expected %d, got %d", snapshot.CompressionLevel, retrieved.CompressionLevel) } + if retrieved.BlobUncompressedSize != snapshot.BlobUncompressedSize { t.Errorf("uncompressed size mismatch: expected %d, got %d", snapshot.BlobUncompressedSize, retrieved.BlobUncompressedSize) } + if retrieved.UploadDurationMs != snapshot.UploadDurationMs { t.Errorf("upload duration mismatch: expected %d, got %d", snapshot.UploadDurationMs, retrieved.UploadDurationMs) } @@ -566,6 +599,7 @@ func TestComplexOrphanedDataScenario(t *testing.T) { if err != nil { t.Fatalf("failed to create snapshot1: %v", err) } + err = repos.Snapshots.Create(ctx, nil, snapshot2) if err != nil { t.Fatalf("failed to create snapshot2: %v", err) @@ -582,6 +616,7 @@ func TestComplexOrphanedDataScenario(t *testing.T) { UID: 1000, GID: 1000, } + err = repos.Files.Create(ctx, nil, files[i]) if err != nil { t.Fatalf("failed to create file%d: %v", i, err) @@ -598,14 +633,17 @@ func TestComplexOrphanedDataScenario(t *testing.T) { if err != nil { t.Fatal(err) } + err = repos.Snapshots.AddFileByID(ctx, nil, snapshot1.ID.String(), files[1].ID) if err != nil { t.Fatal(err) } + err = repos.Snapshots.AddFileByID(ctx, nil, snapshot2.ID.String(), files[1].ID) if err != nil { t.Fatal(err) } + err = repos.Snapshots.AddFileByID(ctx, nil, snapshot2.ID.String(), files[2].ID) if err != nil { t.Fatal(err) @@ -616,6 +654,7 @@ func TestComplexOrphanedDataScenario(t *testing.T) { if err != nil { t.Fatal(err) } + err = repos.Snapshots.Delete(ctx, snapshot1.ID.String()) if err != nil { t.Fatal(err) @@ -633,6 +672,7 @@ func TestComplexOrphanedDataScenario(t *testing.T) { if err != nil { t.Fatalf("error getting file0: %v", err) } + if file0 != nil { t.Error("file0 should have been deleted") } @@ -642,6 +682,7 @@ func TestComplexOrphanedDataScenario(t *testing.T) { if err != nil { t.Fatalf("error getting file1: %v", err) } + if file1 == nil { t.Error("file1 should still exist") } @@ -651,6 +692,7 @@ func TestComplexOrphanedDataScenario(t *testing.T) { if err != nil { t.Fatalf("error getting file2: %v", err) } + if file2 == nil { t.Error("file2 should still exist") } @@ -673,17 +715,19 @@ func TestCascadeDelete(t *testing.T) { UID: 1000, GID: 1000, } + err := repos.Files.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file: %v", err) } // Create chunks and file-chunk mappings - for i := 0; i < 3; i++ { + for i := range 3 { chunk := &Chunk{ ChunkHash: types.ChunkHash(fmt.Sprintf("cascade-chunk-%d", i)), Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatalf("failed to create chunk: %v", err) @@ -694,6 +738,7 @@ func TestCascadeDelete(t *testing.T) { Idx: i, ChunkHash: chunk.ChunkHash, } + err = repos.FileChunks.Create(ctx, nil, fc) if err != nil { t.Fatalf("failed to create file chunk: %v", err) @@ -705,6 +750,7 @@ func TestCascadeDelete(t *testing.T) { if err != nil { t.Fatal(err) } + if len(fileChunks) != 3 { t.Errorf("expected 3 file chunks, got %d", len(fileChunks)) } @@ -720,6 +766,7 @@ func TestCascadeDelete(t *testing.T) { if err != nil { t.Fatal(err) } + if len(fileChunks) != 0 { t.Errorf("expected 0 file chunks after cascade delete, got %d", len(fileChunks)) } @@ -744,6 +791,7 @@ func TestTransactionIsolation(t *testing.T) { UID: 1000, GID: 1000, } + err := repos.Files.Create(ctx, tx, file) if err != nil { return err @@ -754,9 +802,8 @@ func TestTransactionIsolation(t *testing.T) { // For now, we'll just test that rollback works // Return an error to trigger rollback - return fmt.Errorf("intentional rollback") + return errors.New("intentional rollback") }) - if err == nil { t.Fatal("expected error from transaction") } @@ -766,6 +813,7 @@ func TestTransactionIsolation(t *testing.T) { if err != nil { t.Fatal(err) } + if len(files) != 0 { t.Error("file should not exist after rollback") } @@ -790,13 +838,14 @@ func TestConcurrentOrphanedCleanup(t *testing.T) { Hostname: "test-host", StartedAt: time.Now(), } + err := repos.Snapshots.Create(ctx, nil, snapshot) if err != nil { t.Fatal(err) } // Create many files, some orphaned - for i := 0; i < 20; i++ { + for i := range 20 { file := &File{ Path: types.FilePath(fmt.Sprintf("/concurrent-%d.txt", i)), MTime: time.Now().Truncate(time.Second), @@ -805,6 +854,7 @@ func TestConcurrentOrphanedCleanup(t *testing.T) { UID: 1000, GID: 1000, } + err = repos.Files.Create(ctx, nil, file) if err != nil { t.Fatal(err) @@ -822,14 +872,15 @@ func TestConcurrentOrphanedCleanup(t *testing.T) { // Run multiple cleanup operations concurrently // Note: SQLite has limited support for concurrent writes, so we expect some to fail done := make(chan error, 3) - for i := 0; i < 3; i++ { + + for range 3 { go func() { done <- repos.Files.DeleteOrphaned(ctx) }() } // Wait for all to complete - for i := 0; i < 3; i++ { + for i := range 3 { err := <-done if err != nil { t.Errorf("cleanup %d failed: %v", i, err) @@ -850,10 +901,12 @@ func TestConcurrentOrphanedCleanup(t *testing.T) { // Verify all remaining files are even-numbered for _, file := range files { var num int + _, err := fmt.Sscanf(file.Path.String(), "/concurrent-%d.txt", &num) if err != nil { t.Logf("failed to parse file number from %s: %v", file.Path, err) } + if num%2 != 0 { t.Errorf("odd-numbered file %s should have been deleted", file.Path) } diff --git a/internal/database/repository_debug_test.go b/internal/database/repository_debug_test.go index 2bd9493..9deedaa 100644 --- a/internal/database/repository_debug_test.go +++ b/internal/database/repository_debug_test.go @@ -36,12 +36,14 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatalf("failed to create file1: %v", err) } + t.Logf("Created file1 with ID: %s", file1.ID) err = repos.Files.Create(ctx, nil, file2) if err != nil { t.Fatalf("failed to create file2: %v", err) } + t.Logf("Created file2 with ID: %s", file2.ID) // Create a snapshot and reference only file2 @@ -50,18 +52,22 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { Hostname: "test-host", StartedAt: time.Now(), } + err = repos.Snapshots.Create(ctx, nil, snapshot) if err != nil { t.Fatalf("failed to create snapshot: %v", err) } + t.Logf("Created snapshot: %s", snapshot.ID) // Check snapshot_files before adding var count int + err = db.conn.QueryRow("SELECT COUNT(*) FROM snapshot_files").Scan(&count) if err != nil { t.Fatal(err) } + t.Logf("snapshot_files count before add: %d", count) // Add file2 to snapshot @@ -69,6 +75,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatalf("failed to add file to snapshot: %v", err) } + t.Logf("Added file2 to snapshot") // Check snapshot_files after adding @@ -76,6 +83,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("snapshot_files count after add: %d", count) // Check which files are referenced @@ -84,16 +92,22 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { t.Fatal(err) } defer func() { - if err := rows.Close(); err != nil { + err := rows.Close() + if err != nil { t.Logf("failed to close rows: %v", err) } }() + t.Log("Files in snapshot_files:") + for rows.Next() { var fileID string - if err := rows.Scan(&fileID); err != nil { + + err := rows.Scan(&fileID) + if err != nil { t.Fatal(err) } + t.Logf(" - %s", fileID) } @@ -102,6 +116,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("Files count before cleanup: %d", count) // Run orphaned cleanup @@ -109,6 +124,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatalf("failed to delete orphaned files: %v", err) } + t.Log("Ran orphaned cleanup") // Check files after cleanup @@ -116,6 +132,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("Files count after cleanup: %d", count) // List remaining files @@ -123,7 +140,9 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Log("Remaining files:") + for _, f := range files { t.Logf(" - ID: %s, Path: %s", f.ID, f.Path) } @@ -133,10 +152,12 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatalf("error getting file: %v", err) } + if orphanedFile != nil { t.Error("orphaned file should have been deleted") // Let's check why it wasn't deleted var exists bool + err = db.conn.QueryRow(` SELECT EXISTS( SELECT 1 FROM snapshot_files @@ -145,6 +166,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatal(err) } + t.Logf("File1 exists in snapshot_files: %v", exists) } else { t.Log("Orphaned file was correctly deleted") @@ -155,6 +177,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { if err != nil { t.Fatalf("error getting file: %v", err) } + if referencedFile == nil { t.Error("referenced file should not have been deleted") } else { diff --git a/internal/database/repository_edge_cases_test.go b/internal/database/repository_edge_cases_test.go index b4ba6ba..666f33f 100644 --- a/internal/database/repository_edge_cases_test.go +++ b/internal/database/repository_edge_cases_test.go @@ -98,6 +98,7 @@ func TestFileRepositoryEdgeCases(t *testing.T) { if (err != nil) != tt.wantErr { t.Errorf("Create() error = %v, wantErr %v", err, tt.wantErr) } + if err != nil && tt.errMsg != "" && !strings.Contains(err.Error(), tt.errMsg) { t.Errorf("Create() error = %v, want error containing %q", err, tt.errMsg) } @@ -136,6 +137,7 @@ func TestDuplicateHandling(t *testing.T) { if err != nil { t.Fatalf("failed to create file1: %v", err) } + originalID := file1.ID // Create with same path should update the existing record (UPSERT behavior) @@ -190,6 +192,7 @@ func TestDuplicateHandling(t *testing.T) { UID: 1000, GID: 1000, } + err := repos.Files.Create(ctx, nil, file) if err != nil { t.Fatal(err) @@ -199,6 +202,7 @@ func TestDuplicateHandling(t *testing.T) { ChunkHash: types.ChunkHash("test-chunk-dup"), Size: 1024, } + err = repos.Chunks.Create(ctx, nil, chunk) if err != nil { t.Fatal(err) @@ -325,6 +329,7 @@ func TestLargeDatasets(t *testing.T) { Hostname: "test-host", StartedAt: time.Now(), } + err := repos.Snapshots.Create(ctx, nil, snapshot) if err != nil { t.Fatal(err) @@ -332,11 +337,12 @@ func TestLargeDatasets(t *testing.T) { // Create many files const fileCount = 1000 + fileIDs := make([]types.FileID, fileCount) t.Run("create many files", func(t *testing.T) { start := time.Now() - for i := 0; i < fileCount; i++ { + for i := range fileCount { file := &File{ Path: types.FilePath(fmt.Sprintf("/large/file%05d.txt", i)), MTime: time.Now(), @@ -345,10 +351,12 @@ func TestLargeDatasets(t *testing.T) { UID: uint32(1000 + (i % 10)), GID: uint32(1000 + (i % 10)), } + err := repos.Files.Create(ctx, nil, file) if err != nil { t.Fatalf("failed to create file %d: %v", i, err) } + fileIDs[i] = file.ID // Add half to snapshot @@ -359,29 +367,35 @@ func TestLargeDatasets(t *testing.T) { } } } + t.Logf("Created %d files in %v", fileCount, time.Since(start)) }) // Test ListByPrefix performance t.Run("list by prefix performance", func(t *testing.T) { start := time.Now() + files, err := repos.Files.ListByPrefix(ctx, "/large/") if err != nil { t.Fatal(err) } + if len(files) != fileCount { t.Errorf("expected %d files, got %d", fileCount, len(files)) } + t.Logf("Listed %d files in %v", len(files), time.Since(start)) }) // Test orphaned cleanup performance t.Run("orphaned cleanup performance", func(t *testing.T) { start := time.Now() + err := repos.Files.DeleteOrphaned(ctx) if err != nil { t.Fatal(err) } + t.Logf("Cleaned up orphaned files in %v", time.Since(start)) // Verify correct number remain @@ -389,6 +403,7 @@ func TestLargeDatasets(t *testing.T) { if err != nil { t.Fatal(err) } + if len(files) != fileCount/2 { t.Errorf("expected %d files after cleanup, got %d", fileCount/2, len(files)) } @@ -409,6 +424,7 @@ func TestErrorPropagation(t *testing.T) { if err != nil { t.Errorf("GetByID should not return error for non-existent ID, got: %v", err) } + if file != nil { t.Error("expected nil file for non-existent ID") } @@ -420,6 +436,7 @@ func TestErrorPropagation(t *testing.T) { if err != nil { t.Errorf("GetByPath should not return error for non-existent path, got: %v", err) } + if file != nil { t.Error("expected nil file for non-existent path") } @@ -432,10 +449,12 @@ func TestErrorPropagation(t *testing.T) { Idx: 0, ChunkHash: types.ChunkHash("some-chunk"), } + err := repos.FileChunks.Create(ctx, nil, fc) if err == nil { t.Error("expected error for invalid foreign key") } + if !strings.Contains(err.Error(), "FOREIGN KEY") { t.Errorf("expected foreign key error, got: %v", err) } @@ -475,6 +494,7 @@ func TestQueryInjection(t *testing.T) { // Verify tables still exist var count int + err := db.conn.QueryRow("SELECT COUNT(*) FROM files").Scan(&count) if err != nil { t.Fatal("files table was damaged by injection") diff --git a/internal/database/snapshots.go b/internal/database/snapshots.go index 93cc517..3677edf 100644 --- a/internal/database/snapshots.go +++ b/internal/database/snapshots.go @@ -3,7 +3,9 @@ package database import ( "context" "database/sql" + "errors" "fmt" + "strings" "time" "sneak.berlin/go/vaultik/internal/types" @@ -26,6 +28,7 @@ func (r *SnapshotRepository) Create(ctx context.Context, tx *sql.Tx, snapshot *S ` var completedAt *int64 + if snapshot.CompletedAt != nil { ts := snapshot.CompletedAt.Unix() completedAt = &ts @@ -84,9 +87,11 @@ func (r *SnapshotRepository) UpdateCounts(ctx context.Context, tx *sql.Tx, snaps func (r *SnapshotRepository) UpdateExtendedStats(ctx context.Context, tx *sql.Tx, snapshotID string, blobUncompressedSize int64, compressionLevel int, uploadDurationMs int64) error { // Calculate compression ratio based on uncompressed vs compressed sizes var compressionRatio float64 + if blobUncompressedSize > 0 { // Get current blob_size from DB to calculate ratio var blobSize int64 + queryGet := `SELECT blob_size FROM snapshots WHERE id = ?` if tx != nil { err := tx.QueryRowContext(ctx, queryGet, snapshotID).Scan(&blobSize) @@ -99,6 +104,7 @@ func (r *SnapshotRepository) UpdateExtendedStats(ctx context.Context, tx *sql.Tx return fmt.Errorf("getting blob size: %w", err) } } + compressionRatio = float64(blobSize) / float64(blobUncompressedSize) } else { compressionRatio = 1.0 @@ -124,6 +130,7 @@ func (r *SnapshotRepository) UpdateExtendedStats(ctx context.Context, tx *sql.Tx if err != nil { return fmt.Errorf("updating extended stats: %w", err) } + return nil } @@ -136,9 +143,11 @@ func (r *SnapshotRepository) GetByID(ctx context.Context, snapshotID string) (*S WHERE id = ? ` - var snapshot Snapshot - var startedAtUnix int64 - var completedAtUnix *int64 + var ( + snapshot Snapshot + startedAtUnix int64 + completedAtUnix *int64 + ) err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan( &snapshot.ID, @@ -159,9 +168,10 @@ func (r *SnapshotRepository) GetByID(ctx context.Context, snapshotID string) (*S &snapshot.UploadDurationMs, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, fmt.Errorf("querying snapshot: %w", err) } @@ -190,10 +200,13 @@ func (r *SnapshotRepository) ListRecent(ctx context.Context, limit int) ([]*Snap defer CloseRows(rows) var snapshots []*Snapshot + for rows.Next() { - var snapshot Snapshot - var startedAtUnix int64 - var completedAtUnix *int64 + var ( + snapshot Snapshot + startedAtUnix int64 + completedAtUnix *int64 + ) err := rows.Scan( &snapshot.ID, @@ -301,28 +314,35 @@ func (r *SnapshotRepository) AddFilesByIDBatch(ctx context.Context, tx *sql.Tx, const batchSize = 400 for i := 0; i < len(fileIDs); i += batchSize { - end := i + batchSize - if end > len(fileIDs) { - end = len(fileIDs) - } + end := min(i+batchSize, len(fileIDs)) + batch := fileIDs[i:end] query := "INSERT OR IGNORE INTO snapshot_files (snapshot_id, file_id) VALUES " - args := make([]interface{}, 0, len(batch)*2) + + args := make([]any, 0, len(batch)*2) + + var querySb312 strings.Builder + for j, fileID := range batch { if j > 0 { - query += ", " + querySb312.WriteString(", ") } - query += "(?, ?)" + + querySb312.WriteString("(?, ?)") + args = append(args, snapshotID, fileID.String()) } + query += querySb312.String() + var err error if tx != nil { _, err = tx.ExecContext(ctx, query, args...) } else { _, err = r.db.ExecWithLog(ctx, query, args...) } + if err != nil { return fmt.Errorf("batch adding files to snapshot: %w", err) } @@ -353,18 +373,22 @@ func (r *SnapshotRepository) PopulateReferencedBlobs(ctx context.Context, tx *sq AND blobs.blob_hash IS NOT NULL ` - var result sql.Result - var err error + var ( + result sql.Result + err error + ) if tx != nil { result, err = tx.ExecContext(ctx, query, snapshotID, snapshotID) } else { result, err = r.db.ExecWithLog(ctx, query, snapshotID, snapshotID) } + if err != nil { return 0, fmt.Errorf("populating referenced blobs: %w", err) } n, _ := result.RowsAffected() + return n, nil } @@ -405,11 +429,15 @@ func (r *SnapshotRepository) GetBlobHashes(ctx context.Context, snapshotID strin defer CloseRows(rows) var blobs []string + for rows.Next() { var blobHash string - if err := rows.Scan(&blobHash); err != nil { + + err := rows.Scan(&blobHash) + if err != nil { return nil, fmt.Errorf("scanning blob hash: %w", err) } + blobs = append(blobs, blobHash) } @@ -426,6 +454,7 @@ func (r *SnapshotRepository) GetSnapshotTotalCompressedSize(ctx context.Context, ` var totalSize int64 + err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&totalSize) if err != nil { return 0, fmt.Errorf("querying total compressed size: %w", err) @@ -449,6 +478,7 @@ func (r *SnapshotRepository) GetSnapshotUncompressedChunkSize(ctx context.Contex ` var totalSize int64 + err := r.db.conn.QueryRowContext(ctx, query, snapshotID).Scan(&totalSize) if err != nil { return 0, fmt.Errorf("querying uncompressed chunk size: %w", err) @@ -485,6 +515,7 @@ func (r *SnapshotRepository) GetSnapshotNewChunkSize(ctx context.Context, snapsh ` var totalSize int64 + err := r.db.conn.QueryRowContext(ctx, query, snapshotID, snapshotID, snapshotID).Scan(&totalSize) if err != nil { return 0, fmt.Errorf("querying new chunk size: %w", err) @@ -509,10 +540,13 @@ func (r *SnapshotRepository) GetIncompleteSnapshots(ctx context.Context) ([]*Sna defer CloseRows(rows) var snapshots []*Snapshot + for rows.Next() { - var snapshot Snapshot - var startedAtUnix int64 - var completedAtUnix *int64 + var ( + snapshot Snapshot + startedAtUnix int64 + completedAtUnix *int64 + ) err := rows.Scan( &snapshot.ID, @@ -560,10 +594,13 @@ func (r *SnapshotRepository) GetIncompleteByHostname(ctx context.Context, hostna defer CloseRows(rows) var snapshots []*Snapshot + for rows.Next() { - var snapshot Snapshot - var startedAtUnix int64 - var completedAtUnix *int64 + var ( + snapshot Snapshot + startedAtUnix int64 + completedAtUnix *int64 + ) err := rows.Scan( &snapshot.ID, diff --git a/internal/database/snapshots_test.go b/internal/database/snapshots_test.go index c856163..a1efaa1 100644 --- a/internal/database/snapshots_test.go +++ b/internal/database/snapshots_test.go @@ -52,15 +52,19 @@ func TestSnapshotRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get snapshot: %v", err) } + if retrieved == nil { t.Fatal("expected snapshot, got nil") } + if retrieved.ID != snapshot.ID { t.Errorf("ID mismatch: got %s, want %s", retrieved.ID, snapshot.ID) } + if retrieved.Hostname != snapshot.Hostname { t.Errorf("hostname mismatch: got %s, want %s", retrieved.Hostname, snapshot.Hostname) } + if retrieved.FileCount != snapshot.FileCount { t.Errorf("file count mismatch: got %d, want %d", retrieved.FileCount, snapshot.FileCount) } @@ -75,21 +79,27 @@ func TestSnapshotRepository(t *testing.T) { if err != nil { t.Fatalf("failed to get updated snapshot: %v", err) } + if retrieved.FileCount != 200 { t.Errorf("file count not updated: got %d, want %d", retrieved.FileCount, 200) } + if retrieved.ChunkCount != 1000 { t.Errorf("chunk count not updated: got %d, want %d", retrieved.ChunkCount, 1000) } + if retrieved.BlobCount != 20 { t.Errorf("blob count not updated: got %d, want %d", retrieved.BlobCount, 20) } + if retrieved.TotalSize != twoHundredMebibytes { t.Errorf("total size not updated: got %d, want %d", retrieved.TotalSize, twoHundredMebibytes) } + if retrieved.BlobSize != sixtyMebibytes { t.Errorf("blob size not updated: got %d, want %d", retrieved.BlobSize, sixtyMebibytes) } + expectedRatio := compressionRatioPoint3 // 0.3 if math.Abs(retrieved.CompressionRatio-expectedRatio) > 0.001 { t.Errorf("compression ratio not updated: got %f, want %f", retrieved.CompressionRatio, expectedRatio) @@ -108,6 +118,7 @@ func TestSnapshotRepository(t *testing.T) { ChunkCount: int64(500 * i), BlobCount: int64(10 * i), } + err := repo.Create(ctx, nil, s) if err != nil { t.Fatalf("failed to create snapshot %d: %v", i, err) @@ -119,12 +130,13 @@ func TestSnapshotRepository(t *testing.T) { if err != nil { t.Fatalf("failed to list recent snapshots: %v", err) } + if len(recent) != 3 { t.Errorf("expected 3 recent snapshots, got %d", len(recent)) } // Verify order (most recent first) - for i := 0; i < len(recent)-1; i++ { + for i := range len(recent) - 1 { if recent[i].StartedAt.Before(recent[i+1].StartedAt) { t.Error("snapshots not in descending order") } @@ -143,6 +155,7 @@ func TestSnapshotRepositoryNotFound(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } + if snapshot != nil { t.Error("expected nil for non-existent snapshot") } diff --git a/internal/database/uploads.go b/internal/database/uploads.go index c1a3c09..4d64243 100644 --- a/internal/database/uploads.go +++ b/internal/database/uploads.go @@ -3,6 +3,7 @@ package database import ( "context" "database/sql" + "errors" "time" "sneak.berlin/go/vaultik/internal/log" @@ -53,6 +54,7 @@ func (r *UploadRepository) GetByBlobHash(ctx context.Context, blobHash string) ( ` var upload Upload + err := r.conn.QueryRowContext(ctx, query, blobHash).Scan( &upload.BlobHash, &upload.UploadedAt, @@ -60,9 +62,10 @@ func (r *UploadRepository) GetByBlobHash(ctx context.Context, blobHash string) ( &upload.DurationMs, ) - if err == sql.ErrNoRows { + if errors.Is(err, sql.ErrNoRows) { return nil, nil } + if err != nil { return nil, err } @@ -84,17 +87,22 @@ func (r *UploadRepository) GetRecentUploads(ctx context.Context, limit int) ([]* return nil, err } defer func() { - if err := rows.Close(); err != nil { + err := rows.Close() + if err != nil { log.Error("failed to close rows", "error", err) } }() var uploads []*Upload + for rows.Next() { var upload Upload - if err := rows.Scan(&upload.BlobHash, &upload.UploadedAt, &upload.Size, &upload.DurationMs); err != nil { + + err := rows.Scan(&upload.BlobHash, &upload.UploadedAt, &upload.Size, &upload.DurationMs) + if err != nil { return nil, err } + uploads = append(uploads, &upload) } @@ -115,6 +123,7 @@ func (r *UploadRepository) GetUploadStats(ctx context.Context, since time.Time) ` var stats UploadStats + err := r.conn.QueryRowContext(ctx, query, since).Scan( &stats.Count, &stats.TotalSize, @@ -138,10 +147,13 @@ type UploadStats struct { // GetCountBySnapshot returns the count of uploads for a specific snapshot func (r *UploadRepository) GetCountBySnapshot(ctx context.Context, snapshotID string) (int64, error) { query := `SELECT COUNT(*) FROM uploads WHERE snapshot_id = ?` + var count int64 + err := r.conn.QueryRowContext(ctx, query, snapshotID).Scan(&count) if err != nil { return 0, err } + return count, nil }