fix(schema): refresh library_type_name on library change; make trigger block re-runnable

schema.sql is re-executed in full on every application startup, so every
statement in it must be idempotent. Two related problems introduced with the
cross-library move feature, plus their fix:

Problem 1: media_items.library_type_name was only populated by a BEFORE
INSERT trigger. When the scanner repoints an existing row to a different
library (cross-library move detection), the denormalized library_type_name
went stale, mislabeling the item's type for validation and display.

Fix: add a second trigger, set_library_type_name_on_library_change, that
fires BEFORE UPDATE OF library_id and re-runs the same population function.

Problem 2 (outage): the new trigger EXECUTE FUNCTIONs set_library_type_name(),
but the pre-existing idempotency dance drops that function on every startup
before recreating it. On the second and later boots, DROP FUNCTION failed
with SQLSTATE 2BP01 (function still depended on by the trigger created by
the previous boot), aborting the whole schema transaction and crash-looping
the container.

Fix: drop BOTH triggers before dropping the function, and create both after
it. The ordering now survives any number of restarts on any database state.

Adds TestSchemaInitializationIsIdempotent, which replays the production
database.Initialize twice against the same database so this class of
"second startup" regression fails in CI instead of in production. Note the
test uses a dedicated pgxpool: Initialize holds the advisory lock on one
connection while executing the schema on another, which deadlocks against
the shared test pool's MaxConns=1.
This commit is contained in:
John O'Keefe
2026-09-21 18:07:51 -04:00
parent 65f6bdf075
commit 1002cbca93
2 changed files with 65 additions and 1 deletions
@@ -0,0 +1,53 @@
package main
import (
"context"
"testing"
"bookhoard/internal/config"
"bookhoard/internal/database"
"github.com/jackc/pgx/v5/pgxpool"
"github.com/stretchr/testify/require"
)
// TestSchemaInitializationIsIdempotent replays the application's production
// schema initializer twice against the same database. schema.sql runs on
// EVERY app startup, so it must apply cleanly to a database where a previous
// startup already applied it. A regression here crash-loops the production
// container on its second boot (e.g. SQLSTATE 2BP01: DROP FUNCTION refused
// while a trigger created by the previous run still references it).
//
// Uses a dedicated pool: production Initialize holds the advisory lock on one
// connection while executing the schema on another, but the shared test pool
// is capped at MaxConns=1 and would deadlock.
func TestSchemaInitializationIsIdempotent(t *testing.T) {
if testing.Short() {
t.Skip("skipping schema re-initialization in -short mode")
}
// Bring up the standard test environment (seeds users, starts server).
setup := setupTestServer(t)
defer func() { _ = setup.Close() }()
// Dedicated pool with default connection limits for the initializer.
poolCfg, err := pgxpool.ParseConfig(config.LoadConfig().DatabaseURL())
require.NoError(t, err, "failed to parse database URL")
pool, err := pgxpool.NewWithConfig(context.Background(), poolCfg)
require.NoError(t, err, "failed to connect for schema initialization test")
defer func() { pool.Close() }()
ctx := context.Background()
// First boot: applies the schema to the (possibly fresh) database.
if err := database.Initialize(ctx, pool); err != nil {
t.Fatalf("first schema initialization failed: %v", err)
}
// Second boot against the now-initialized database: this is exactly
// where non-idempotent statements (bare CREATE TRIGGER, DROP FUNCTION
// with dependent objects, etc.) blow up.
if err := database.Initialize(ctx, pool); err != nil {
t.Fatalf("second schema initialization failed: %v — schema.sql must stay idempotent across restarts", err)
}
}