feat(issues): the honest processing-issues ledger — row-less import failures, self-healing resolution, family lens (faceted plan Phase 11)
processing_issues only recorded cross-library move rejections; files that failed to import vanished into an error counter and stdout. The ledger now records every failure class and heals itself: Schema (in place, pre-release boundary): - media_item_id NULLABLE + file_path TEXT (deliberately unbounded — an over-long path is itself a failure mode; a VARCHAR(500) ledger column died on the exact 22001 it existed to report — caught by the new test). Row-less failures key on (library, file_path); item-backed on (item, type). Widened dedup = COALESCE-expression unique index; the old table UNIQUE stays (NULLs pairwise-distinct for row-less). - Conflict-target upsert: repeat failures UPDATE the same row and REOPEN resolved ones — never duplicates. Wirings: - CreateMediaItem failure (single-file + multifile createAudioBookItem) → row-less import_failed (error). - Extraction hard-fail with fallback import → item-backed metadata_extraction_failed (warning). - Change-path re-extraction failure (the ev/sidecar guard) → item-backed metadata_extraction_failed; LogProcessingIssue keeps its signature, a row-less helper joins it. Auto-resolution: - Successful import → ResolveOpenProcessingIssuesForPath (library, file_path). - Successful (re)extraction → ResolveOpenProcessingIssuesForItem (single-file change path, multifile editorial apply). - Unchanged-skip scans do NOT resolve: extraction never re-ran, so the degraded metadata is still degraded — the page heals exactly when the book does. Family lens on ALL issues surfaces (list, stats, resolve, delete, SSR page): explicit out-of-set library id → 403, never silent. This tightens one pre-lens behavior — a nonexistent library id returned 200-empty, now 403 (pinned, changelog entry in docs/contract_pins.md). List is LEFT-JOINed so row-less rows appear (media_item_id "", title falls back to the path). Tests: full lifecycle state machine through real scans (fail → appears, repeat → no dups, success-at-path → resolved, LEFT-JOIN list shape); family lens 403s with a fresh non-shared admin; all 7 pre-existing issue suites green (two expectations widened to the lens, changelogged); adjacent suites (CrossLibraryMove, Audiobook, Calibre, ExtractorVersion, SchemaIdempotency) green; unit suites green.
This commit is contained in:
John O'Keefe committed
1 parent
19b9c7c9b8
commit
9b5defcb31
50 files changed
+1244
-681
No files matched your search
@@ -1,12 +1,22 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"bookhoard/internal/database"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -119,8 +129,8 @@ func TestProcessingIssuesListInputValidation(t *testing.T) {
|
||||
{
|
||||
name: "Nonexistent library UUID",
|
||||
libraryID: "00000000-0000-0000-0000-000000000000",
|
||||
expectedStatus: http.StatusOK,
|
||||
description: "Should return empty array for valid but nonexistent UUID",
|
||||
expectedStatus: http.StatusForbidden,
|
||||
description: "Family lens: an id outside the visible set is a 403, never a silent pass (nonexistent is trivially out-of-set)",
|
||||
},
|
||||
{
|
||||
name: "Special characters in UUID",
|
||||
@@ -293,8 +303,8 @@ func TestProcessingIssueStatsInputValidation(t *testing.T) {
|
||||
{
|
||||
name: "Nonexistent library UUID",
|
||||
libraryID: "00000000-0000-0000-0000-000000000000",
|
||||
expectedStatus: http.StatusOK,
|
||||
description: "Should return zero counts for valid but nonexistent UUID",
|
||||
expectedStatus: http.StatusForbidden,
|
||||
description: "Family lens: an id outside the visible set is a 403, never a silent pass (nonexistent is trivially out-of-set)",
|
||||
},
|
||||
{
|
||||
name: "Special characters in UUID",
|
||||
@@ -513,3 +523,166 @@ func TestProcessingIssuesInvalidTokens(t *testing.T) {
|
||||
assert.Equal(t, http.StatusUnauthorized, rec.Code)
|
||||
})
|
||||
}
|
||||
|
||||
// ============================================================================
|
||||
// Phase 11 (faceted plan): the honest ledger — row-less import failures,
|
||||
// truthful upserts, self-healing resolution, family lens.
|
||||
// ============================================================================
|
||||
|
||||
// createTestAdminOnce creates a fresh admin user (never the shared
|
||||
// testuser) with unique credentials and registers its cleanup.
|
||||
func createTestAdminOnce(t *testing.T, db *database.Queries, ts *httptest.Server) (UserTestData, string) {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
uniqueID := uuid.New().String()[:8]
|
||||
email := fmt.Sprintf("issuesadmin-%s@tests.bookhoard.internal", uniqueID)
|
||||
username := fmt.Sprintf("issuesadmin-%s", uniqueID)
|
||||
|
||||
passwordHash := "$2a$10$JjAtK7PPa1WexQC3AUGe8OXLeuseZ/haN1Mz7emMo6CfOvMiTVXWq"
|
||||
newUser, err := db.CreateUser(ctx, database.CreateUserParams{
|
||||
Email: email,
|
||||
Username: username,
|
||||
PasswordHash: passwordHash,
|
||||
FirstName: pgtype.Text{String: "Issues", Valid: true},
|
||||
LastName: pgtype.Text{String: "Admin", Valid: true},
|
||||
Role: "admin",
|
||||
Timezone: pgtype.Text{String: "UTC", Valid: true},
|
||||
})
|
||||
require.NoError(t, err, "Should create admin test user")
|
||||
|
||||
userUUID, err := uuid.FromBytes(newUser.ID.Bytes[0:16])
|
||||
require.NoError(t, err, "Should parse admin user UUID")
|
||||
|
||||
t.Cleanup(func() {
|
||||
db.DeleteUser(context.Background(), pgtype.UUID{Bytes: [16]byte(userUUID), Valid: true})
|
||||
})
|
||||
|
||||
user := UserTestData{ID: userUUID, Email: email, Username: username, Password: "Test@Pass123!"}
|
||||
token := loginWithCredentials(t, ts, email, "Test@Pass123!")
|
||||
return user, token
|
||||
}
|
||||
|
||||
func TestProcessingIssuesLedgerImportFailures(t *testing.T) {
|
||||
setup := setupTestServer(t)
|
||||
t.Cleanup(func() { _ = setup.Close() })
|
||||
|
||||
ctx := context.Background()
|
||||
admin, _ := moveTestAdminID(t, setup)
|
||||
libID, folder := createTypedTestLibrary(t, setup, fmt.Sprintf("Test Issues Ledger %d", time.Now().UnixNano()), "ebooks")
|
||||
t.Cleanup(func() { deleteLibraries(t, setup, libID) })
|
||||
|
||||
// A book whose library-relative path exceeds media_items.file_path's
|
||||
// VARCHAR(500): CreateMediaItem fails deterministically, and the
|
||||
// relative path itself cannot ever import — the classic row-less
|
||||
// ledger case. Filename and dir components stay within filesystem
|
||||
// limits; only the total path overflows.
|
||||
longRel := ""
|
||||
for i := 0; i < 9; i++ {
|
||||
longRel = filepath.Join(longRel, strings.Repeat("d", 60))
|
||||
}
|
||||
longRel = filepath.Join(longRel, "deepbook.epub")
|
||||
require.Greater(t, len(longRel), 500, "relative path must exceed the column limit")
|
||||
deepDir := filepath.Join(folder, filepath.Dir(longRel))
|
||||
require.NoError(t, os.MkdirAll(deepDir, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(folder, longRel), []byte("dummy epub content"), 0o644))
|
||||
|
||||
scanTestFolder(t, setup, admin, folder)
|
||||
|
||||
// Fail → the row-less issue appears, keyed by (library, path).
|
||||
open := func() []database.ListProcessingIssuesByLibraryRow {
|
||||
rows, err := setup.DB.ListProcessingIssuesByLibrary(ctx, libID)
|
||||
require.NoError(t, err)
|
||||
return rows
|
||||
}
|
||||
issues := open()
|
||||
require.Len(t, issues, 1, "the failed import must appear in the ledger")
|
||||
issue := issues[0]
|
||||
require.Equal(t, "import_failed", issue.IssueType)
|
||||
require.Equal(t, "error", issue.Severity)
|
||||
require.False(t, issue.MediaItemID.Valid, "row-less: no media_items row exists")
|
||||
require.Equal(t, filepath.ToSlash(longRel), issue.FilePath, "row-less key is the library-relative path")
|
||||
require.Empty(t, issue.Title, "no item row → no title")
|
||||
|
||||
// The JSON list surfaces the row-less shape: empty item id, title
|
||||
// fallen back to the path (pins the LEFT JOIN — an inner join would
|
||||
// silently drop the ledger's newest kind).
|
||||
req := httptest.NewRequest("GET", "/api/libraries/"+libID.String()+"/issues/list", nil)
|
||||
req.Header.Set("Authorization", "Bearer "+setup.Token)
|
||||
rec := httptest.NewRecorder()
|
||||
setup.Server.Config.Handler.ServeHTTP(rec, req)
|
||||
require.Equal(t, http.StatusOK, rec.Code)
|
||||
var listed []map[string]interface{}
|
||||
require.NoError(t, json.NewDecoder(rec.Body).Decode(&listed))
|
||||
require.Len(t, listed, 1)
|
||||
assert.Equal(t, "", listed[0]["media_item_id"], "row-less issues carry no item id")
|
||||
assert.Equal(t, filepath.ToSlash(longRel), listed[0]["title"], "title falls back to the file path")
|
||||
assert.Equal(t, "import_failed", listed[0]["issue_type"])
|
||||
|
||||
// Repeat failure → the SAME row updates (no dups).
|
||||
scanTestFolder(t, setup, admin, folder)
|
||||
issues = open()
|
||||
require.Len(t, issues, 1, "repeat failures upsert, never duplicate")
|
||||
require.Equal(t, issue.ID, issues[0].ID)
|
||||
|
||||
// Success resolves by (library, path): a normal book imports at a
|
||||
// path that has an open row-less issue — seeded here to model the
|
||||
// transient failure the long path can't have (its path IS the
|
||||
// failure).
|
||||
goodRel := "Author/ledgerbook.epub"
|
||||
_, err := setup.DBPool.Exec(ctx,
|
||||
`INSERT INTO processing_issues (media_item_id, library_id, issue_type, issue_description, severity, file_path)
|
||||
VALUES (NULL, $1, 'import_failed', 'seeded transient failure', 'error', $2)`,
|
||||
libID, goodRel)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(folder, "Author"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(folder, "Author", "ledgerbook.epub"), []byte("dummy epub content"), 0o644))
|
||||
|
||||
scanTestFolder(t, setup, admin, folder)
|
||||
|
||||
var resolved int
|
||||
require.NoError(t, setup.DBPool.QueryRow(ctx,
|
||||
`SELECT count(*) FROM processing_issues WHERE library_id = $1 AND file_path = $2 AND resolved AND resolved_at IS NOT NULL`,
|
||||
libID, goodRel).Scan(&resolved))
|
||||
require.Equal(t, 1, resolved, "the file's successful import must resolve its open issue")
|
||||
require.Len(t, open(), 1, "only the long-path issue stays open")
|
||||
}
|
||||
|
||||
func TestProcessingIssuesFamilyLens(t *testing.T) {
|
||||
setup := setupTestServer(t)
|
||||
t.Cleanup(func() { _ = setup.Close() })
|
||||
|
||||
ctx := context.Background()
|
||||
libID, _ := createTypedTestLibrary(t, setup, fmt.Sprintf("Test Issues Lens %d", time.Now().UnixNano()), "ebooks")
|
||||
t.Cleanup(func() { deleteLibraries(t, setup, libID) })
|
||||
|
||||
// A fresh admin (never the shared testuser) + a hidden-library row:
|
||||
// default-ALLOW visibility with one explicit hide.
|
||||
lensAdmin, lensToken := createTestAdminOnce(t, setup.DB, setup.Server)
|
||||
_, err := setup.DBPool.Exec(ctx,
|
||||
`INSERT INTO library_visibility (library_id, user_id, is_visible) VALUES ($1, $2, false)`,
|
||||
libID, pgtype.UUID{Bytes: [16]byte(lensAdmin.ID), Valid: true})
|
||||
require.NoError(t, err)
|
||||
|
||||
// An open issue in that library (row-less, no item to cascade).
|
||||
_, err = setup.DBPool.Exec(ctx,
|
||||
`INSERT INTO processing_issues (media_item_id, library_id, issue_type, issue_description, severity, file_path)
|
||||
VALUES (NULL, $1, 'import_failed', 'lens probe', 'error', 'hidden/book.epub')`,
|
||||
libID)
|
||||
require.NoError(t, err)
|
||||
|
||||
get := func(token, suffix string) *httptest.ResponseRecorder {
|
||||
req := httptest.NewRequest("GET", "/api/libraries/"+libID.String()+"/issues/"+suffix, nil)
|
||||
req.Header.Set("Authorization", "Bearer "+token)
|
||||
rec := httptest.NewRecorder()
|
||||
setup.Server.Config.Handler.ServeHTTP(rec, req)
|
||||
return rec
|
||||
}
|
||||
|
||||
// Hidden from the fresh admin → 403 on both endpoints (never silent).
|
||||
assert.Equal(t, http.StatusForbidden, get(lensToken, "list").Code, "hidden library list is a 403")
|
||||
assert.Equal(t, http.StatusForbidden, get(lensToken, "stats").Code, "hidden library stats are a 403")
|
||||
|
||||
// Visible to the shared admin (no hide row) → the rows show.
|
||||
assert.Equal(t, http.StatusOK, get(setup.Token, "list").Code)
|
||||
assert.Equal(t, http.StatusOK, get(setup.Token, "stats").Code)
|
||||
}
|
||||
Reference in new issue
Block a user