Bookhoard/bookhoard
0
0 Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases 49 Wiki Activity

fix(media-items): normalize NOT NULL narrators on every write path

Browse Source 
A2 made narrators/narrators_search NOT NULL DEFAULT '{}' (86f0af6), but an
explicit NULL in an INSERT/UPDATE column list bypasses the column default
— the API media-create path (POST /api/media-items without narrators)
500'd on every call, taking the whole integration suite down with it
(ISBN tests, capture seeds, all direct-DB seeds).

- set_library_type_name() trigger now coalesces NULL narrators/
  narrators_search to '{}' at the row level, and its update trigger
  widens from BEFORE UPDATE OF library_id to BEFORE UPDATE so every
  write path is covered
- defense in depth at the callers: handler create/update normalize nil
  to empty slices (normalizeNarrators), scanner seeds the same
  (nilToEmpty) for files that carry no narrator credits
- analytics device-usage tests move to the dedicated per-test user
  (the shared admin's device list accrues live rows mid-suite — the
  documented testuser flapping class)
This commit is contained in:
John O'Keefe committed 2026-10-16 10:14:12 -04:00
1 parent 73ae1789c5
commit a222ef5c27
5 files changed
+56 -13
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

cmd/server/tests/analytics_test.go
+10 -5
View File
@@ -135,8 +135,12 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
})
t.Run("GetDeviceUsage_WithAuth_NoDevices", func(t *testing.T) {
// The REGULAR per-test user, not the shared admin: testuser is a
// LIVE reader whose device list accrues rows mid-suite (the app
// genuinely registers against the dev DB while tests run), so the
// "no devices" baseline is only deterministic on a fresh account.
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/device-usage", nil)
req.Header.Set("Authorization", "Bearer "+setup.Token)
req.Header.Set("Authorization", "Bearer "+setup.RegularToken)
resp, err := client.Do(req)
require.NoError(t, err)
@@ -152,7 +156,8 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
})
t.Run("GetDeviceUsage_WithAuth_WithDevices", func(t *testing.T) {
// First, create a device
// Same dedicated-user doctrine: register + assert on the fresh
// regular account instead of mutating the shared admin.
deviceReq := map[string]interface{}{
"device_name": "Test Kobo",
"device_type": "kobo",
@@ -161,7 +166,7 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
deviceReqHTTP, _ := http.NewRequest("POST", setup.Server.URL+"/api/devices/register", bytes.NewBuffer(deviceBody))
deviceReqHTTP.Header.Set("Content-Type", "application/json")
deviceReqHTTP.Header.Set("Authorization", "Bearer "+setup.Token)
deviceReqHTTP.Header.Set("Authorization", "Bearer "+setup.RegularToken)
resp, err := client.Do(deviceReqHTTP)
require.NoError(t, err)
@@ -171,7 +176,7 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
// Now get device usage
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/device-usage", nil)
req.Header.Set("Authorization", "Bearer "+setup.Token)
req.Header.Set("Authorization", "Bearer "+setup.RegularToken)
resp, err = client.Do(req)
require.NoError(t, err)
@@ -193,7 +198,7 @@ func TestAnalyticsDeviceUsage(t *testing.T) {
t.Run("GetDeviceUsage_ResponseStructure", func(t *testing.T) {
req, _ := http.NewRequest("GET", setup.Server.URL+"/api/analytics/device-usage", nil)
req.Header.Set("Authorization", "Bearer "+setup.Token)
req.Header.Set("Authorization", "Bearer "+setup.RegularToken)
resp, err := client.Do(req)
require.NoError(t, err)
database/schema/schema.sql
+16 -2
View File
@@ -289,12 +289,25 @@ DROP TRIGGER IF EXISTS set_library_type_name_on_library_change ON media_items;
DROP FUNCTION IF EXISTS set_library_type_name();
-- Add trigger to populate library_type_name on insert
-- Recompute the denormalized library type on insert, and NORMALIZE the
-- A2 NOT NULL narrators columns: narrators/narrators_search are NOT NULL
-- DEFAULT '{}' but an explicit NULL in an INSERT/UPDATE column list
-- bypasses the column default and 23502s (the API create path without
-- narrators 500'd exactly this way). "No narrators known" is an empty
-- array, never NULL — coalesce at the row level so every write path
-- (API, scanner, direct seeds) is safe by construction.
CREATE OR REPLACE FUNCTION set_library_type_name() RETURNS TRIGGER AS $$
BEGIN
SELECT lt.name INTO NEW.library_type_name
FROM libraries l
JOIN library_types lt ON l.library_type_id = lt.id
WHERE l.id = NEW.library_id;
IF NEW.narrators IS NULL THEN
NEW.narrators := '{}';
END IF;
IF NEW.narrators_search IS NULL THEN
NEW.narrators_search := '{}';
END IF;
RETURN NEW;
END;
$$ LANGUAGE plpgsql;
@@ -305,9 +318,10 @@ CREATE TRIGGER set_library_type_name_on_insert
-- Keep library_type_name truthful when an item is repointed to another
-- library (cross-library move detection reuses the same function; the row
-- keeps its ID while its library changes).
-- keeps its ID while its library changes). Fires on ALL updates so the
-- narrators normalization guards every write path, not just library moves.
CREATE TRIGGER set_library_type_name_on_library_change
BEFORE UPDATE OF library_id ON media_items
BEFORE UPDATE ON media_items
FOR EACH ROW
EXECUTE FUNCTION set_library_type_name();
internal/handlers/commonhandlers.go
+11
View File
@@ -65,6 +65,17 @@ func parseDate(dateStr string) time.Time {
return time.Time{}
}
// normalizeNarrators guards the NOT NULL narrators columns (A2): a Go nil
// slice becomes an explicit NULL in INSERT/UPDATE params and 23502s — the
// column DEFAULT never fires for explicit NULLs. Omitted narrators mean
// "none known", which is an empty array, never NULL.
func normalizeNarrators(narrators []string) []string {
if narrators == nil {
return []string{}
}
return narrators
}
func SetupRoutes(g *echo.Group, db *database.Queries, connManager *wsync.ConnectionManager, queueProcessor *wsync.SyncQueueProcessor, cfg *config.Config) *Handler {
return NewHandler(db, connManager, queueProcessor, cfg)
}
internal/handlers/media.go
+6 -4
View File
@@ -1414,7 +1414,8 @@ func (mh *MediaHandler) CreateMediaItem(c *echo.Context) error {
tagsSearch := utils.NormalizeTagsSearch(req.Tags)
contributorsSearch := utils.NormalizeContributorsSearch(req.Contributors)
narratorsSearch := utils.NormalizeNarratorsSearch(req.Narrators)
narrators := normalizeNarrators(req.Narrators)
narratorsSearch := utils.NormalizeNarratorsSearch(narrators)
// Validate and normalize ISBN
normalizedISBN, err := utils.NormalizeISBN(req.ISBN)
@@ -1464,7 +1465,7 @@ func (mh *MediaHandler) CreateMediaItem(c *echo.Context) error {
SeriesNumber: pgtype.Int4{Int32: req.SeriesNumber, Valid: req.SeriesNumber > 0},
Tags: req.Tags,
TagsSearch: tagsSearch,
Narrators: req.Narrators,
Narrators: narrators,
NarratorsSearch: narratorsSearch,
Abridged: req.Abridged,
Asin: pgtype.Text{String: req.ASIN, Valid: req.ASIN != ""},
@@ -1532,7 +1533,8 @@ func (mh *MediaHandler) UpdateMediaItem(c *echo.Context) error {
tagsSearch := utils.NormalizeTagsSearch(req.Tags)
contributorsSearch := utils.NormalizeContributorsSearch(req.Contributors)
narratorsSearch := utils.NormalizeNarratorsSearch(req.Narrators)
narrators := normalizeNarrators(req.Narrators)
narratorsSearch := utils.NormalizeNarratorsSearch(narrators)
normalizedISBN, err := utils.NormalizeISBN(req.ISBN)
if err != nil && req.ISBN != "" {
@@ -1586,7 +1588,7 @@ func (mh *MediaHandler) UpdateMediaItem(c *echo.Context) error {
SeriesNumber: pgtype.Int4{Int32: req.SeriesNumber, Valid: req.SeriesNumber > 0},
Tags: req.Tags,
TagsSearch: tagsSearch,
Narrators: req.Narrators,
Narrators: narrators,
NarratorsSearch: narratorsSearch,
Abridged: req.Abridged,
Asin: pgtype.Text{String: req.ASIN, Valid: req.ASIN != ""},
internal/services/media_scanner.go
+13 -2
View File
@@ -169,6 +169,17 @@ type CalibreOPFMetadata struct {
// watcher (and thus can never panic on EMFILE/ENOSPC). This fixes the
// fd/inotify-watch leak where every scan job created a watcher that was
// never closed.
// nilToEmpty guards the NOT NULL narrators columns (A2): a Go nil slice
// becomes an explicit NULL in INSERT/UPDATE params and 23502s — the
// column DEFAULT never fires for explicit NULLs. "No narrators known"
// is an empty array, never NULL.
func nilToEmpty(slice []string) []string {
if slice == nil {
return []string{}
}
return slice
}
func NewMediaScanner(db *database.Queries) *MediaScanner {
return &MediaScanner{
db: db,
@@ -947,7 +958,7 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool,
DatePublished: pgtype.Date{Time: metadata.PublishDate, Valid: !metadata.PublishDate.IsZero()},
Contributors: metadata.Contributors,
ContributorsSearch: contributorsSearch,
Narrators: metadata.Narrators,
Narrators: nilToEmpty(metadata.Narrators),
NarratorsSearch: utils.NormalizeNarratorsSearch(metadata.Narrators),
Abridged: metadata.Abridged,
Tags: metadata.Tags,
@@ -3210,7 +3221,7 @@ func (s *MediaScanner) updateMediaItem(ctx context.Context, existing database.Me
CoverImagePath: pgtype.Text{String: s.getRelativePath(metadata.CoverPath), Valid: metadata.CoverPath != ""},
Series: pgtype.Text{String: metadata.Series, Valid: metadata.Series != ""},
SeriesNumber: pgtype.Int4{Int32: metadata.SeriesNumber, Valid: metadata.SeriesNumber > 0},
Narrators: metadata.Narrators,
Narrators: nilToEmpty(metadata.Narrators),
NarratorsSearch: utils.NormalizeNarratorsSearch(metadata.Narrators),
Abridged: metadata.Abridged,
Tags: metadata.Tags,