Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions internal/datasource/biblioreads/adapter.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -151,6 +152,17 @@ func (p *Enricher) enrich(ctx context.Context, idx *index.BleveIndexer, document
if title := strings.TrimSpace(document.Title); title != q {
queries = append(queries, title)
}
if title := strings.TrimSpace(withoutParenthetical(document.Title)); title != "" && title != strings.TrimSpace(document.Title) {
if len(document.Authors) > 0 {
query := strings.TrimSpace(title + " " + document.Authors[0])
if !slices.Contains(queries, query) {
queries = append(queries, query)
}
}
if !slices.Contains(queries, title) {
queries = append(queries, title)
}
}
for _, query := range queries {
response, err := p.source.SearchBooks(ctx, query)
if err != nil {
Expand Down
159 changes: 158 additions & 1 deletion internal/datasource/biblioreads/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"fmt"
"image"
"reflect"
"slices"
Expand Down Expand Up @@ -514,20 +515,58 @@ func TestTitleMatchScore(t *testing.T) {
{name: "subtitle", title: "La promesa", candidate: "La promesa: Una historia", want: 1},
{name: "document subtitle", title: "La promesa: Una historia", candidate: "La promesa", want: 1},
{name: "parenthetical", title: "La promesa", candidate: "La promesa (The Promise)", want: 1},
{name: "document parenthetical", title: "La promesa (The Promise)", candidate: "La promesa", want: 1},
{name: "document volume parenthetical retained", title: "La promesa (Volume 2)", candidate: "La promesa"},
{name: "candidate volume parenthetical retained", title: "La promesa", candidate: "La promesa (Volume 2)"},
{name: "document edition", title: "Crímenes Célebres (2ª ed.)", candidate: "Crímenes célebres", want: 1},
{name: "candidate edition", title: "Crímenes Célebres", candidate: "Crímenes célebres (2ª ed.)", want: 1},
{name: "different editions", title: "La promesa (2ª ed.)", candidate: "La promesa (3ª ed.)"},
{name: "different parentheticals", title: "La promesa (First)", candidate: "La promesa (Second)"},
{name: "different subtitles", title: "La promesa: Una historia", candidate: "La promesa: Otra historia"},
{name: "standalone volume", title: "La promesa", candidate: "La promesa (#1)"},
{name: "document standalone volume", title: "La promesa (#1)", candidate: "La promesa"},
{name: "unrelated", title: "La promesa", candidate: "Otro libro"},
{name: "empty candidate", title: "La promesa"},
} {
t.Run(tt.name, func(t *testing.T) {
got := titleMatchScore(tt.candidate, normalizedTitle(tt.title), mainTitle(tt.title))
withoutSuffix := normalizedTitle(withoutParenthetical(tt.title))
got := titleMatchScore(tt.candidate, normalizedTitle(tt.title), mainTitle(tt.title), withoutSuffix)
if got != tt.want {
t.Fatalf("title match score = %d, want %d", got, tt.want)
}
})
}
}

func TestWithoutParenthetical(t *testing.T) {
for _, tt := range []struct {
title, want string
}{
{"Crímenes Célebres (2ª ed.)", "Crímenes Célebres"},
{"Book (2nd edition)", "Book"},
{"Libro (2.ª edición)", "Libro"},
{"Book (Revised edition)", "Book"},
{"Book (Illustrated)", "Book"},
{"Book (2024 revised)", "Book"},
{"Book (#2)", "Book (#2)"},
{"Book (2)", "Book (2)"},
{"Book (II)", "Book (II)"},
{"Book (Volume 2)", "Book (Volume 2)"},
{"Book (Vol. II)", "Book (Vol. II)"},
{"Book (Tomo 2)", "Book (Tomo 2)"},
{"Book (2nd part)", "Book (2nd part)"},
{"Book (Series, #2)", "Book (Series, #2)"},
{"Book (nested (annotation))", "Book (nested (annotation))"},
{"Book ()", "Book ()"},
} {
t.Run(tt.title, func(t *testing.T) {
if got := withoutParenthetical(tt.title); got != tt.want {
t.Fatalf("got %q, want %q", got, tt.want)
}
})
}
}

func TestPreferBook(t *testing.T) {
for _, tt := range []struct {
name string
Expand Down Expand Up @@ -875,6 +914,124 @@ func TestTitleFallback(t *testing.T) {
}
}

func TestParentheticalFallback(t *testing.T) {
const title = "Crímenes Célebres"
const author = "Alexandre Dumas"
const bookURL = "/book/show/20371277-cr-menes-c-lebres"
for _, suffix := range []string{"(2ª ed.)", "(Revised edition)", "(Illustrated)"} {
for _, withAuthor := range []bool{true, false} {
t.Run(fmt.Sprint(suffix, " with author ", withAuthor), func(t *testing.T) {
editionTitle := title + " " + suffix
result := SearchResult{Title: title, Author: author, BookURL: bookURL, Rating: "4.00"}
query := title
if withAuthor {
query += " " + author
}
source := &fakeSource{byQuery: map[string][]SearchResult{
editionTitle + " " + author: {{Title: "Otro libro", Author: author, BookURL: "/book/show/10"}},
query: {result},
}}
idx := newTestIndexer(t, source)
doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: editionTitle, Authors: []string{author}}}
if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil {
t.Fatal(err)
}
got, err := idx.Book(context.Background(), doc)
if err != nil || got.BookURL != bookURL {
t.Fatalf("got %+v, %v; want %q", got, err, bookURL)
}
stored, err := idx.DocumentByID(doc.ID)
if err != nil || !mustData(t, stored).Enriched || mustData(t, stored).URL != bookURL {
t.Fatalf("expected persisted match, got %+v, %v", stored, err)
}
want := []string{editionTitle + " " + author, editionTitle, title + " " + author}
if !withAuthor {
want = append(want, title)
}
if !slices.Equal(source.Queries(), want) {
t.Fatalf("queries = %q, want %q", source.Queries(), want)
}
})
}
}
}

func TestParentheticalFallbackRejectsWrongAuthor(t *testing.T) {
const title = "Crímenes Célebres"
const author = "Alexandre Dumas"
source := &fakeSource{byQuery: map[string][]SearchResult{
title + " " + author: {{Title: title, Author: "Another Writer", BookURL: "/book/show/1"}},
title: {{Title: title, Author: "Another Writer", BookURL: "/book/show/1"}},
}}
idx := newTestIndexer(t, source)
doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: title + " (Revised edition)", Authors: []string{author}}}
if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil {
t.Fatal(err)
}
got, err := idx.Book(context.Background(), doc)
if err != nil || got.BookURL != "" {
t.Fatalf("wrong author must not match, got %+v, %v", got, err)
}
}

func TestVolumeDoesNotBroadenSearch(t *testing.T) {
const title = "A book (Volume 2)"
const author = "A Writer"
source := &fakeSource{}
idx := newTestIndexer(t, source)
doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: title, Authors: []string{author}}}
if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil {
t.Fatal(err)
}
if _, err := idx.Book(context.Background(), doc); err != nil {
t.Fatal(err)
}
if want := []string{title + " " + author, title}; !slices.Equal(source.Queries(), want) {
t.Fatalf("queries = %q, want %q", source.Queries(), want)
}
}

func TestParentheticalFallbackDoesNotDuplicateQueries(t *testing.T) {
source := &fakeSource{}
idx := newTestIndexer(t, source)
doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: "Crímenes Célebres (2ª ed.)", Authors: []string{""}}}
if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil {
t.Fatal(err)
}
if _, err := idx.Book(context.Background(), doc); err != nil {
t.Fatal(err)
}
want := []string{"Crímenes Célebres (2ª ed.)", "Crímenes Célebres"}
if !slices.Equal(source.Queries(), want) {
t.Fatalf("queries = %q, want %q", source.Queries(), want)
}
}

func TestParentheticalFallbackErrorLeavesDocumentPending(t *testing.T) {
const title = "Crímenes Célebres"
const author = "Alexandre Dumas"
source := &fakeSource{
byQuery: map[string][]SearchResult{},
queryErrors: map[string]error{title + " " + author: ErrUnavailable},
}
idx := newTestIndexer(t, source)
doc := index.Document{ID: testID, Metadata: metadata.Metadata{Title: title + " (2ª ed.)", Authors: []string{author}}}
if err := index.IndexDocument(idx.documentsIdx, doc.ID, doc); err != nil {
t.Fatal(err)
}
if _, err := idx.Book(context.Background(), doc); !errors.Is(err, ErrUnavailable) {
t.Fatalf("expected unavailable error, got %v", err)
}
stored, err := idx.DocumentByID(doc.ID)
if err != nil || mustData(t, stored).Enriched {
t.Fatalf("expected document left pending, got %+v, %v", stored, err)
}
want := []string{doc.Title + " " + author, doc.Title, title + " " + author}
if !slices.Equal(source.Queries(), want) {
t.Fatalf("queries = %q, want %q", source.Queries(), want)
}
}

func TestUnavailableSearchDoesNotBroadenQuery(t *testing.T) {
const title = "El economista camuflado"
const author = "Tim Harford"
Expand Down
30 changes: 26 additions & 4 deletions internal/datasource/biblioreads/matching.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ var seriesSuffix = regexp.MustCompile(`\s*\((?:[^()]+,\s*#[0-9]+(?:\.[0-9]+)?|#[

var subtitleSeparator = regexp.MustCompile(`:\s*|\.\s+|\s+[-–—]\s+`)

var parentheticalTitle = regexp.MustCompile(`\s+\([\p{L}][^()]*\)\s*$`)
var parentheticalTitle = regexp.MustCompile(`\s+\(([^()]*)\)\s*$`)
var standaloneVolume = regexp.MustCompile(`(?i)^(?:[0-9]+|[ivxlcdm]+)$`)

var goodreadsBookPath = regexp.MustCompile(`^/book/show/([1-9][0-9]*)(?:[.-][A-Za-z0-9._~-]+)?/?$`)

Expand Down Expand Up @@ -88,6 +89,25 @@ func normalizedTitle(value string) string {
return normalizeName(seriesSuffix.ReplaceAllString(value, ""))
}

func withoutParenthetical(value string) string {
match := parentheticalTitle.FindStringSubmatchIndex(value)
if match == nil {
return value
}
annotation := value[match[2]:match[3]]
normalized := normalizeName(annotation)
if normalized == "" || strings.Contains(annotation, "#") || standaloneVolume.MatchString(normalized) {
return value
}
for _, word := range strings.Fields(normalized) {
switch word {
case "vol", "volume", "volumen", "tomo", "tome", "part", "parte", "book", "libro", "livre":
return value
}
}
return strings.TrimSpace(value[:match[0]])
}

func mainTitle(value string) string {
value = seriesSuffix.ReplaceAllString(value, "")
if separator := subtitleSeparator.FindStringIndex(value); separator != nil {
Expand All @@ -96,7 +116,7 @@ func mainTitle(value string) string {
return normalizeName(value)
}

func titleMatchScore(candidate, title, documentMainTitle string) int {
func titleMatchScore(candidate, title, documentMainTitle, documentWithoutParenthetical string) int {
candidateTitle := normalizedTitle(candidate)
if candidateTitle == "" {
return 0
Expand All @@ -105,7 +125,8 @@ func titleMatchScore(candidate, title, documentMainTitle string) int {
return 3
}
if mainTitle(candidate) == title || documentMainTitle == candidateTitle ||
normalizedTitle(parentheticalTitle.ReplaceAllString(candidate, "")) == title {
normalizedTitle(withoutParenthetical(candidate)) == title ||
(documentWithoutParenthetical != "" && documentWithoutParenthetical == candidateTitle) {
return 1
}
return 0
Expand Down Expand Up @@ -163,13 +184,14 @@ func matchingBook(document index.Document, results []SearchResult) (SearchResult
return SearchResult{}, false
}
mainTitle := mainTitle(document.Title)
documentWithoutParenthetical := normalizedTitle(withoutParenthetical(document.Title))
var best SearchResult
bestScore := 0
for _, result := range results {
if result.BookURL == "" {
continue
}
score := titleMatchScore(result.Title, title, mainTitle)
score := titleMatchScore(result.Title, title, mainTitle, documentWithoutParenthetical)
if score == 0 {
continue
}
Expand Down
63 changes: 63 additions & 0 deletions internal/webserver/background_progress_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package webserver_test

import (
"errors"
"net/http"
"sync/atomic"
"testing"
Expand All @@ -22,6 +23,16 @@ type progressCountingInfo struct {
calls atomic.Int64
}

type progressStateInfo struct {
webserver.IndexInfo
tasks []index.Progress
err error
}

func (i *progressStateInfo) BackgroundProgress() ([]index.Progress, error) {
return i.tasks, i.err
}

func (i *progressCountingInfo) BackgroundProgress() ([]index.Progress, error) {
i.calls.Add(1)
return i.IndexInfo.BackgroundProgress()
Expand Down Expand Up @@ -199,6 +210,58 @@ func TestBackgroundProgressPollingAccess(t *testing.T) {
}
}

func TestBackgroundProgressPollingIntervalTransitions(t *testing.T) {
db := infrastructure.Connect(":memory:", 250)
sqlDB, err := db.DB()
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = sqlDB.Close() })
cfg := defaultTestConfig()
sender := &infrastructure.NoEmail{}
appFS := afero.NewMemMapFs()
unusedApp, idx := bootstrapAppWithIndexer(db, sender, appFS, cfg)
t.Cleanup(func() { _ = unusedApp.Shutdown() })
t.Cleanup(func() { _ = idx.Close() })
source := wikidata.NewWikidataSource(wikidata.Gowikidata{})
provider := biblioreads.NewReviewProvider(nil, nil)
controllers := webserver.SetupControllers(cfg, db, idx, sender, appFS, source, nil, source, provider)
state := &progressStateInfo{IndexInfo: idx}
app := webserver.New(cfg, controllers, sender, state, &model.UserRepository{DB: db})
t.Cleanup(func() { _ = app.Shutdown() })

for _, step := range []struct {
name string
tasks []index.Progress
err error
trigger string
}{
{name: "initially idle", trigger: "every 60s"},
{name: "work starts", tasks: []index.Progress{{Kind: index.ProgressDocuments, InProgress: true}}, trigger: "every 5s"},
{name: "work completes", trigger: "every 60s"},
{name: "progress read fails", err: errors.New("progress unavailable"), trigger: "every 5s"},
{name: "progress read recovers", trigger: "every 60s"},
} {
t.Run(step.name, func(t *testing.T) {
state.tasks, state.err = step.tasks, step.err
for _, path := range []string{"/?l=en", "/background-progress?l=en"} {
page := getPage(t, app, path)
banner := page.Find("#background-progress")
if got := banner.AttrOr("hx-trigger", ""); got != step.trigger {
t.Errorf("%s: trigger = %q, want %q", path, got, step.trigger)
}
if banner.AttrOr("hx-get", "") != "/background-progress?l=en" ||
banner.AttrOr("hx-swap", "") != "outerHTML" {
t.Errorf("%s: polling endpoint or swap changed", path)
}
if (banner.Find(`[role="alert"]`).Length() != 0) != (step.err != nil) {
t.Errorf("%s: incorrect error alert", path)
}
}
})
}
}

func TestBackgroundProgressErrorHandlerUsesFeedbackScript(t *testing.T) {
db := infrastructure.Connect(":memory:", 250)
app := bootstrapApp(db, &infrastructure.NoEmail{}, afero.NewMemMapFs(), webserver.Config{})
Expand Down
10 changes: 10 additions & 0 deletions internal/webserver/controller/document/detail.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ func (d *Controller) Detail(c fiber.Ctx) error {

title, _ := documentPageTitle(document)

var storedScore datasourcemodel.Score
if d.reviewProvider != nil {
storedScore, err = d.StoredScore(document)
if err != nil {
log.Printf("Error reading stored score for %s: %s", document.ID, err)
storedScore = datasourcemodel.Score{}
}
}

lang, _ := c.Locals("Lang").(string)
authorSummaries, illustratorSummaries := d.authorAndIllustratorSummaries(document, lang)

Expand Down Expand Up @@ -75,6 +84,7 @@ func (d *Controller) Detail(c fiber.Ctx) error {
"IllustratorSummaries": illustratorSummaries,
"Debug": c.RequestCtx().QueryArgs().Has("debug"),
"ReviewsEnabled": d.reviewProvider != nil,
"StoredScore": storedScore,
"DocumentLink": d.dataSourceDocumentLink(document),
}, "layout")
}
Expand Down
Loading
Loading