From 824d7bdf43d7012d5bf34dabb8029c959a3cc8d9 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Wed, 29 Jul 2026 13:27:24 +0000 Subject: [PATCH] fix(catalog): preserve normalized TMDB ownership --- internal/catalog/provider_id_repo.go | 28 +++++++-- .../catalog/provider_id_tmdb_match_test.go | 60 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/internal/catalog/provider_id_repo.go b/internal/catalog/provider_id_repo.go index 3e32dae4..8f191064 100644 --- a/internal/catalog/provider_id_repo.go +++ b/internal/catalog/provider_id_repo.go @@ -118,14 +118,34 @@ func (r *ProviderIDRepository) AttachTMDBID(ctx context.Context, contentID, item } var existingOwnerContentID string - err = tx.QueryRow(ctx, ` + ownerQuery := ` SELECT content_id FROM media_items - WHERE type = $1 - AND tmdb_id = $2 + WHERE tmdb_id >= $2 + AND type = $1 AND content_id <> $3 + AND (tmdb_id = $2 OR tmdb_id LIKE $2 || '-%') LIMIT 1 - `, itemType, tmdbText, contentID).Scan(&existingOwnerContentID) + ` + ownerArgs := []any{itemType, tmdbText, contentID} + // Bound ordinary slug candidates with the existing tmdb_id index. A run of + // nines has no same-width numeric successor, so that rare boundary uses the + // lower-bounded predicate above rather than an empty lexical range. + tmdbUpper := strconv.FormatUint(uint64(tmdbID)+1, 10) + if len(tmdbUpper) == len(tmdbText) { + ownerQuery = ` + SELECT content_id + FROM media_items + WHERE tmdb_id >= $1 + AND tmdb_id < $2 + AND type = $3 + AND content_id <> $4 + AND (tmdb_id = $1 OR tmdb_id LIKE $1 || '-%') + LIMIT 1 + ` + ownerArgs = []any{tmdbText, tmdbUpper, itemType, contentID} + } + err = tx.QueryRow(ctx, ownerQuery, ownerArgs...).Scan(&existingOwnerContentID) if err != nil && !errors.Is(err, pgx.ErrNoRows) { return fmt.Errorf("checking tmdb id owner: %w", err) } diff --git a/internal/catalog/provider_id_tmdb_match_test.go b/internal/catalog/provider_id_tmdb_match_test.go index f661a1b9..3b599c6f 100644 --- a/internal/catalog/provider_id_tmdb_match_test.go +++ b/internal/catalog/provider_id_tmdb_match_test.go @@ -1,6 +1,15 @@ package catalog -import "testing" +import ( + "context" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" +) // Real values seen rejected in production: media_items.tmdb_id held TMDB's // "id-slug" URL form while the presence lookup attached the bare numeric id, so @@ -39,3 +48,52 @@ func TestNormalizeTMDBIDLeavesNonIdentifiersAlone(t *testing.T) { t.Errorf("normalizeTMDBID(%q) = %q", "", got) } } + +func TestAttachTMDBIDRejectsEquivalentSlugOwner(t *testing.T) { + dsn := strings.TrimSpace(os.Getenv("SILO_TEST_DATABASE_URL")) + if dsn == "" { + t.Skip("SILO_TEST_DATABASE_URL is not set") + } + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + pool, err := pgxpool.New(ctx, dsn) + if err != nil { + t.Fatalf("connect to test database: %v", err) + } + t.Cleanup(pool.Close) + + suffix := time.Now().UnixNano() + // A run of nines guards the lexical range bound used by the owner lookup: + // incrementing the numeric id would produce a shorter, invalid upper bound. + tmdbID := 999_999_999 + ownerContentID := fmt.Sprintf("test-tmdb-slug-owner-%d", suffix) + targetContentID := fmt.Sprintf("test-tmdb-slug-target-%d", suffix) + if _, err := pool.Exec(ctx, ` + INSERT INTO media_items (content_id, type, title, tmdb_id) + VALUES ($1, 'series', 'TMDB slug owner test', $3), + ($2, 'series', 'TMDB slug target test', '') + `, ownerContentID, targetContentID, fmt.Sprintf("%d-some-title", tmdbID)); err != nil { + t.Fatalf("insert test media items: %v", err) + } + t.Cleanup(func() { + cleanupCtx, cleanupCancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cleanupCancel() + if _, err := pool.Exec(cleanupCtx, `DELETE FROM media_items WHERE content_id = ANY($1)`, []string{ownerContentID, targetContentID}); err != nil { + t.Errorf("clean up test media items: %v", err) + } + }) + + err = NewProviderIDRepository(pool).AttachTMDBID(ctx, targetContentID, "series", tmdbID) + if err == nil || !strings.Contains(err.Error(), "already belongs to content_id") { + t.Fatalf("AttachTMDBID() error = %v, want existing-owner conflict", err) + } + + var targetTMDBID string + if err := pool.QueryRow(ctx, `SELECT tmdb_id FROM media_items WHERE content_id = $1`, targetContentID).Scan(&targetTMDBID); err != nil { + t.Fatalf("load target tmdb id: %v", err) + } + if targetTMDBID != "" { + t.Fatalf("target tmdb_id = %q, want unchanged", targetTMDBID) + } +}