fix(ebooks): batch-load enrichment identifiers
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user