From bb53bd06522b4a91a2d32a22a4c3bfa6f7993d4f Mon Sep 17 00:00:00 2001 From: rxwatcher Date: Sun, 19 Jul 2026 19:36:49 +0200 Subject: [PATCH] fix(ebooks): batch-load enrichment identifiers --- internal/ebooks/enrichment.go | 54 ++++++++++++++++++++----- internal/ebooks/enrichment_test.go | 64 ++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 9 deletions(-) diff --git a/internal/ebooks/enrichment.go b/internal/ebooks/enrichment.go index bc14c5da..cc6383f7 100644 --- a/internal/ebooks/enrichment.go +++ b/internal/ebooks/enrichment.go @@ -119,6 +119,17 @@ type EnrichmentRunResult struct { HasMore bool `json:"-"` } +type ebookProviderIDRepository interface { + GetByContentIDs(ctx context.Context, contentIDs []string) (map[string][]*models.MediaItemProviderID, error) + ReplaceByContentID(ctx context.Context, contentID string, providerIDs map[string]string) error + FindContentIDByProviderIDs( + ctx context.Context, + providerIDs map[string]string, + itemType string, + excludeContentID string, + ) (string, error) +} + // Enricher drives the ebook metadata enrichment sweep. type Enricher struct { pool *pgxpool.Pool @@ -126,7 +137,7 @@ type Enricher struct { resolver *metadata.PluginResolverAdapter itemRepo *catalog.ItemRepository personRepo *catalog.PersonRepository - providerIDs *catalog.ProviderIDRepository + providerIDs ebookProviderIDRepository imageCacher metadata.ImageCacher imageCacheJobs metadata.ImageCacheJobEnqueuer workLinker literaryWorkLinker @@ -151,13 +162,17 @@ func NewEnricher( personRepo *catalog.PersonRepository, providerIDs *catalog.ProviderIDRepository, ) *Enricher { + var providerIDStore ebookProviderIDRepository + if providerIDs != nil { + providerIDStore = providerIDs + } return &Enricher{ pool: pool, chainRepo: chainRepo, resolver: resolver, itemRepo: itemRepo, personRepo: personRepo, - providerIDs: providerIDs, + providerIDs: providerIDStore, batchSize: defaultEnrichBatchSize, workers: ebookEnrichWorkers(), queue: NewEnrichmentQueue(pool), @@ -649,18 +664,38 @@ func (e *Enricher) loadClaimedItems(ctx context.Context, jobs []EnrichmentJob) ( return nil, fmt.Errorf("iterating ebook enrichment rows: %w", err) } - if e.providerIDs != nil { - for i := range items { - pids, err := e.providerIDs.GetByContentID(ctx, items[i].ContentID) - if err == nil { - items[i].ProviderIDs = providerIDMapFromRows(pids) - } - } + if err := e.loadProviderIDs(ctx, contentIDs, items); err != nil { + return nil, err } return items, nil } +func (e *Enricher) loadProviderIDs( + ctx context.Context, + contentIDs []string, + items []enrichmentItemRow, +) error { + if e == nil || e.providerIDs == nil || len(contentIDs) == 0 { + return nil + } + rowsByContentID, err := e.providerIDs.GetByContentIDs(ctx, contentIDs) + if err != nil { + return fmt.Errorf("loading ebook provider IDs: %w", err) + } + for i := range items { + items[i].ProviderIDs = providerIDMapFromRows(rowsByContentID[items[i].ContentID]) + _, hasISBN := items[i].ProviderIDs["isbn"] + slog.DebugContext(ctx, "ebook enrichment: loaded provider identifiers", + "component", "ebooks", + "content_id", items[i].ContentID, + "identifier_count", len(items[i].ProviderIDs), + "has_isbn", hasISBN, + ) + } + return nil +} + func (e *Enricher) enrichItem(ctx context.Context, item enrichmentItemRow) error { outcome, err := e.enrichClaimedItem(ctx, item) if err == nil && outcome == EnrichmentOutcomeSkipped { @@ -951,6 +986,7 @@ func collectEbookMetadata(ctx context.Context, item enrichmentItemRow, providers if result == nil || !result.HasMetadata { continue } + accumulator.HasMetadata = true mergeEnrichmentProviderIDs(accumulator, result) metadata.MergeMetadata(result, accumulator, nil, metadata.MergeFillEmpty) diff --git a/internal/ebooks/enrichment_test.go b/internal/ebooks/enrichment_test.go index a382c6b1..60e080ec 100644 --- a/internal/ebooks/enrichment_test.go +++ b/internal/ebooks/enrichment_test.go @@ -25,6 +25,13 @@ func TestEbookContentType(t *testing.T) { } } +func TestNewEnricherPreservesNilProviderIDRepository(t *testing.T) { + e := NewEnricher(nil, nil, nil, nil, nil, nil) + if e.providerIDs != nil { + t.Fatal("providerIDs is a non-nil typed interface for a nil repository") + } +} + func TestFilterEbookPeopleKeepsAuthorsOnly(t *testing.T) { people := []models.ItemPerson{ {Person: models.Person{Name: "Author One"}, Kind: models.PersonKindAuthor, SortOrder: 7}, @@ -625,6 +632,9 @@ func TestCollectEbookMetadataAccumulatesProviderErrors(t *testing.T) { if accumulator.Overview != "found" { t.Fatalf("accumulator overview = %q, want metadata from the working provider", accumulator.Overview) } + if !accumulator.HasMetadata { + t.Fatal("accumulator HasMetadata = false after a provider returned metadata") + } if ids["openlibrary"] != "OL1M" { t.Fatalf("accumulated IDs = %v, want search-result openlibrary ID", ids) } @@ -938,6 +948,60 @@ func TestBuildEbookMetadataRequestCarriesAccumulatedISBN(t *testing.T) { } } +type fakeEbookProviderIDRepository struct { + rows map[string][]*models.MediaItemProviderID + err error + calls [][]string +} + +func (f *fakeEbookProviderIDRepository) GetByContentIDs( + _ context.Context, + contentIDs []string, +) (map[string][]*models.MediaItemProviderID, error) { + f.calls = append(f.calls, append([]string(nil), contentIDs...)) + return f.rows, f.err +} + +func (f *fakeEbookProviderIDRepository) ReplaceByContentID(context.Context, string, map[string]string) error { + return nil +} + +func (f *fakeEbookProviderIDRepository) FindContentIDByProviderIDs( + context.Context, + map[string]string, + string, + string, +) (string, error) { + return "", nil +} + +func TestEnricherLoadsProviderIDsInOneBatchAndSurfacesErrors(t *testing.T) { + repo := &fakeEbookProviderIDRepository{ + rows: map[string][]*models.MediaItemProviderID{ + "ebook-1": { + {ContentID: "ebook-1", ItemType: "ebook", Provider: "isbn", ProviderID: "9781982173456"}, + }, + }, + } + e := &Enricher{providerIDs: repo} + items := []enrichmentItemRow{{ContentID: "ebook-1"}, {ContentID: "ebook-2"}} + + if err := e.loadProviderIDs(context.Background(), []string{"ebook-1", "ebook-2"}, items); err != nil { + t.Fatalf("loadProviderIDs() error = %v", err) + } + if len(repo.calls) != 1 || strings.Join(repo.calls[0], ",") != "ebook-1,ebook-2" { + t.Fatalf("GetByContentIDs() calls = %#v, want one batch", repo.calls) + } + if got := items[0].ProviderIDs["isbn"]; got != "9781982173456" { + t.Fatalf("items[0].ProviderIDs[isbn] = %q", got) + } + + repo.err = errors.New("provider IDs unavailable") + if err := e.loadProviderIDs(context.Background(), []string{"ebook-1"}, items[:1]); err == nil { + t.Fatal("loadProviderIDs() error = nil, want repository error") + } +} + func TestPreserveDurableEbookLocalMetadataAcrossRefreshes(t *testing.T) { item := enrichmentItemRow{ Status: "matched",