diff --git a/classes/material_index.go b/classes/material_index.go index 5b50502..2eb0c9a 100644 --- a/classes/material_index.go +++ b/classes/material_index.go @@ -13,8 +13,11 @@ import ( "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/beachcomber" "git.golem15.com/golem15/summercms/modules/beachcomber/typesense" + "git.golem15.com/golem15/summercms/modules/conga" "git.golem15.com/golem15/summercms/modules/diary" "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/jakub/sm-bm-plugin/jobs" "git.golem15.com/jakub/sm-bm-plugin/models" "gorm.io/gorm" ) @@ -92,6 +95,9 @@ type MaterialIndexer struct { Service func() *beachcomber.Service // Log is where failures are logged; nil means slog.Default. Log *slog.Logger + // Enqueue inserts a background job, inside tx when tx is in a + // transaction. Nil means jobs are not queued. + Enqueue func(ctx context.Context, tx *gorm.DB, args pact.JobArgs) error } // NewMaterialIndexer returns the indexer of app. The service is resolved on @@ -109,6 +115,27 @@ func NewMaterialIndexer(app *backpack.App) *MaterialIndexer { return svc }, Log: diary.For("jz.bm"), + Enqueue: func(ctx context.Context, tx *gorm.DB, args pact.JobArgs) error { + m, err := conga.From(app) + if err != nil { + return err + } + return m.Enqueue(ctx, tx, args, conga.EnqueueOpts{}) + }, + } +} + +// QueueExtract enqueues the file text job of material id inside tx, so the +// job exists only when the attach commits. The job extracts the text of the +// material's new PDFs and then syncs its document. A failure is logged and +// not returned: an attach never fails because of the job, and +// bm:materials-reindex backfills the text. +func (ix *MaterialIndexer) QueueExtract(ctx context.Context, tx *gorm.DB, id uint) { + if ix == nil || ix.Enqueue == nil || id == 0 { + return + } + if err := ix.Enqueue(ctx, tx, jobs.ExtractMaterialTextArgs{MaterialID: id}); err != nil { + ix.log().Warn("jz.bm: could not queue the file text job", "material_id", id, "err", err) } } diff --git a/classes/material_texts.go b/classes/material_texts.go new file mode 100644 index 0000000..8ceeb67 --- /dev/null +++ b/classes/material_texts.go @@ -0,0 +1,76 @@ +package classes + +import ( + "context" + "errors" + "fmt" + "strconv" + "time" + + "git.golem15.com/golem15/summercms/modules/lagoon/attach" + "git.golem15.com/jakub/sm-bm-plugin/models" + "gocloud.dev/blob" + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +// EnsureFileTexts extracts the text of every current public PDF of material +// materialID that has no text row yet, and stores one row per file (an +// empty text for a scan or a broken PDF, so it is not retried). It returns +// how many rows it added. ErrPDFToolMissing stops the loop and is returned +// as is, so callers can match it; a blob, extraction or insert error is +// returned wrapped with the file id. It runs only in the extraction job and +// bm:materials-reindex, never in a request. +func EnsureFileTexts(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, pdf PDFText, materialID uint) (int, error) { + if db == nil { + return 0, fmt.Errorf("jz.bm: no database for the file texts of material %d", materialID) + } + if bucket == nil { + return 0, fmt.Errorf("jz.bm: no uploads bucket for the file texts of material %d", materialID) + } + db = db.WithContext(ctx) + // The conditions of the API's materialFiles, which classes cannot import. + var files []attach.File + err := db.Model(&attach.File{}). + Select("system_files.*"). + Joins("LEFT JOIN jz_bm_material_file_texts t ON t.file_id = system_files.id"). + Where("system_files.attachment_type = ? AND system_files.attachment_id = ? AND system_files.field = ?", + models.Material{}.MorphName(), strconv.FormatUint(uint64(materialID), 10), models.MaterialFilesField). + Where("system_files.is_public"). + Where("(system_files.content_type = 'application/pdf' OR lower(system_files.file_name) LIKE '%.pdf')"). + Where("t.file_id IS NULL"). + Order("system_files.sort_order NULLS LAST, system_files.id"). + Find(&files).Error + if err != nil { + return 0, err + } + added := 0 + for _, f := range files { + text, truncated, err := extractBlob(ctx, bucket, pdf, f) + if errors.Is(err, ErrPDFToolMissing) { + return added, err + } + if err != nil { + return added, fmt.Errorf("jz.bm: file %d: %w", f.ID, err) + } + row := models.MaterialFileText{FileID: f.ID, MaterialID: materialID, Text: text, Truncated: truncated, ExtractedAt: time.Now().UTC()} + res := db.Clauses(clause.OnConflict{Columns: []clause.Column{{Name: "file_id"}}, DoNothing: true}).Create(&row) + if res.Error != nil { + return added, fmt.Errorf("jz.bm: file %d: %w", f.ID, res.Error) + } + if res.RowsAffected > 0 { + added++ + } + } + return added, nil +} + +// extractBlob runs pdf on the blob of f. +func extractBlob(ctx context.Context, bucket *blob.Bucket, pdf PDFText, f attach.File) (string, bool, error) { + rd, err := bucket.NewReader(ctx, attach.BlobKey(f.DiskName), nil) + if err != nil { + return "", false, err + } + defer rd.Close() + return pdf.Extract(ctx, rd) +} diff --git a/classes/pdftext.go b/classes/pdftext.go new file mode 100644 index 0000000..4af7ff0 --- /dev/null +++ b/classes/pdftext.go @@ -0,0 +1,124 @@ +package classes + +import ( + "context" + "errors" + "io" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +// ErrPDFToolMissing is returned by Extract when the pdftotext binary is not +// installed (poppler-utils). Nothing is stored then, so a later run +// retries. +var ErrPDFToolMissing = errors.New("jz.bm: pdftotext is not installed") + +// PDFText runs pdftotext (poppler-utils) on one PDF. The PDF comes from a +// material manager, so it is untrusted input to a native tool: it runs +// without a shell, from a 0600 temp file in a 0700 directory that is +// removed afterwards, for at most Timeout and with at most MaxBytes of text +// read. It runs only in the extraction job and the reindex command, never in +// a request. +type PDFText struct { + // Bin is the pdftotext binary, looked up in PATH. + Bin string + // Timeout bounds one run. + Timeout time.Duration + // MaxBytes is the most text kept per file. + MaxBytes int + // TempDir is where the PDF is copied for the tool. + TempDir string +} + +// DefaultPDFText is pdftotext with a 60 s timeout and 1 MiB of text per +// file, with its temp files under storage/app/tmp in the app's working +// directory (beside the uploads bucket, not a tmpfs /tmp). +func DefaultPDFText() PDFText { + return PDFText{ + Bin: "pdftotext", + Timeout: 60 * time.Second, + MaxBytes: 1 << 20, + TempDir: filepath.Join("storage", "app", "tmp"), + } +} + +// Extract returns the text of the PDF read from r. A PDF with no text layer +// (a scan) or a broken one makes pdftotext fail; that is not an error: the +// text read so far, usually "", comes back with a nil error. truncated is +// set when the text went over MaxBytes or the run timed out. A missing +// binary is ErrPDFToolMissing; a cancelled ctx is its error. +func (p PDFText) Extract(ctx context.Context, r io.Reader) (string, bool, error) { + bin := p.Bin + if bin == "" { + bin = "pdftotext" + } + dir := p.TempDir + if dir == "" { + dir = os.TempDir() + } + if err := os.MkdirAll(dir, 0o700); err != nil { + return "", false, err + } + tmp, err := os.CreateTemp(dir, "jz-bm-pdftext-*.pdf") + if err != nil { + return "", false, err + } + path := tmp.Name() + defer os.Remove(path) + if _, err := io.Copy(tmp, r); err != nil { + tmp.Close() + return "", false, err + } + if err := tmp.Close(); err != nil { + return "", false, err + } + + runCtx := ctx + if p.Timeout > 0 { + var cancel context.CancelFunc + runCtx, cancel = context.WithTimeout(ctx, p.Timeout) + defer cancel() + } + cmd := exec.CommandContext(runCtx, bin, "-q", "-enc", "UTF-8", path, "-") + // No stdin (the PDF is a path) and stderr is discarded. + stdout, err := cmd.StdoutPipe() + if err != nil { + return "", false, err + } + if err := cmd.Start(); err != nil { + if errors.Is(err, exec.ErrNotFound) { + return "", false, ErrPDFToolMissing + } + return "", false, err + } + limit := p.MaxBytes + if limit <= 0 { + limit = 1 << 20 + } + out, _ := io.ReadAll(io.LimitReader(stdout, int64(limit)+1)) + truncated := len(out) > limit + if truncated { + out = out[:limit] + // The rest is not wanted; stop the tool instead of draining it. + _ = cmd.Process.Kill() + } + _ = cmd.Wait() + if ctx.Err() != nil { + return "", false, ctx.Err() + } + if runCtx.Err() != nil { + truncated = true + } + return CleanExtractedText(string(out)), truncated, nil +} + +// CleanExtractedText removes NUL bytes, which a Postgres TEXT column +// refuses, and invalid UTF-8 (such as a sequence cut at the size cap) from +// text a tool extracted. +func CleanExtractedText(s string) string { + s = strings.ReplaceAll(s, "\x00", "") + return strings.ToValidUTF8(s, "") +} diff --git a/classes/pdftext_test.go b/classes/pdftext_test.go new file mode 100644 index 0000000..007b1b0 --- /dev/null +++ b/classes/pdftext_test.go @@ -0,0 +1,89 @@ +package classes_test + +import ( + "bytes" + "context" + "crypto/rand" + "errors" + "os" + "os/exec" + "strings" + "testing" + "time" + "unicode/utf8" + + "git.golem15.com/jakub/sm-bm-plugin/classes" + "git.golem15.com/jakub/sm-bm-plugin/internal/pdftest" +) + +func TestPDFTextExtract(t *testing.T) { + if _, err := exec.LookPath("pdftotext"); err != nil { + t.Skip("pdftotext is not installed (poppler-utils)") + } + ctx := context.Background() + dir := t.TempDir() + pdf := classes.DefaultPDFText() + pdf.TempDir = dir + if pdf.Bin != "pdftotext" || pdf.Timeout != 60*time.Second || pdf.MaxBytes != 1<<20 { + t.Errorf("DefaultPDFText = %+v, want pdftotext, 60 s, 1 MiB", pdf) + } + doc := pdftest.OnePage("Globalizacja i demokracja") + + text, truncated, err := pdf.Extract(ctx, bytes.NewReader(doc)) + if err != nil { + t.Fatalf("Extract: %v", err) + } + if !strings.Contains(text, "Globalizacja i demokracja") || truncated { + t.Errorf("Extract = %q truncated %v, want the phrase, not truncated", text, truncated) + } + + small := pdf + small.MaxBytes = 5 + text, truncated, err = small.Extract(ctx, bytes.NewReader(doc)) + if err != nil || !truncated || len(text) > 5 { + t.Errorf("MaxBytes 5: %q truncated %v err %v, want at most 5 bytes and truncated", text, truncated, err) + } + + junk := make([]byte, 4096) + _, _ = rand.Read(junk) + text, _, err = pdf.Extract(ctx, bytes.NewReader(junk)) + if err != nil || text != "" { + t.Errorf("random bytes: %q err %v, want empty text and no error", text, err) + } + + missing := pdf + missing.Bin = "jz-bm-no-such-pdftotext" + if _, _, err := missing.Extract(ctx, bytes.NewReader(doc)); !errors.Is(err, classes.ErrPDFToolMissing) { + t.Errorf("missing binary: err %v, want ErrPDFToolMissing", err) + } + + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Errorf("the temp dir holds %d files after extraction, want none", len(entries)) + } + // A temp dir Extract creates itself is private to the app user. + sub := pdf + sub.TempDir = dir + "/sub" + if _, _, err := sub.Extract(ctx, bytes.NewReader(doc)); err != nil { + t.Fatal(err) + } + if info, err := os.Stat(sub.TempDir); err != nil || info.Mode().Perm() != 0o700 { + t.Errorf("created temp dir: %v %v, want mode 0700", info, err) + } + + for _, c := range []struct{ in, want string }{ + {"plain tekst", "plain tekst"}, + {"a\x00b\x00", "ab"}, + {"zażółć\xff\xfe gęślą", "zażółć gęślą"}, + {"cut \xc5", "cut "}, + {"", ""}, + } { + got := classes.CleanExtractedText(c.in) + if got != c.want || !utf8.ValidString(got) { + t.Errorf("CleanExtractedText(%q) = %q, want %q", c.in, got, c.want) + } + } +} diff --git a/controllers/api/export_test.go b/controllers/api/export_test.go index b50a62e..c4fdecd 100644 --- a/controllers/api/export_test.go +++ b/controllers/api/export_test.go @@ -9,4 +9,5 @@ var ( PickSnippet = pickSnippet FlattenSnippet = flattenSnippet RankOrder = rankOrder + HasFileSnippet = hasFileSnippet ) diff --git a/controllers/api/manage.go b/controllers/api/manage.go index dc828ab..cae8f05 100644 --- a/controllers/api/manage.go +++ b/controllers/api/manage.go @@ -251,13 +251,21 @@ func (h *Handlers) ManagedMaterials(w http.ResponseWriter, r *http.Request) { writeOpaque500(w) return } + var files map[uint][]models.IndexedFile + if cands != nil { + files, err = snippetFiles(db, materialIDs(rows), cands.Hits) + if err != nil { + writeOpaque500(w) + return + } + } mcp := viaMCP(r) out := make([]any, 0, len(rows)) for _, m := range rows { row := newManagedMaterialJSON(m, people) if cands != nil { if hit, ok := cands.Hits[m.ID]; ok { - row.Snippet = snippetValue(pickSnippet(hit, m), mcp) + row.Snippet = snippetValue(pickSnippet(hit, m, files[m.ID]), mcp) } } out = append(out, managedMaterialView(row, mcp, includeBody(r))) diff --git a/controllers/api/manage_files.go b/controllers/api/manage_files.go index 5f718a1..dcf8d8f 100644 --- a/controllers/api/manage_files.go +++ b/controllers/api/manage_files.go @@ -115,7 +115,8 @@ func (h *Handlers) AttachMaterialFiles(w http.ResponseWriter, r *http.Request) { h.writeMaterialDetail(w, r, db, m.ID, http.StatusCreated) } -// storeParts stores and binds every "file" part in one transaction. A +// storeParts stores and binds every "file" part in one transaction and +// queues the file text job in it. A // refusal (no file, too many, a type or size attach.Store turns down) is // set in *refusal and rolls back; the returned keys are the blobs written. func (h *Handlers) storeParts(ctx context.Context, db *gorm.DB, bucket *blob.Bucket, m models.Material, mr *multipart.Reader, refusal *string) ([]string, error) { @@ -153,6 +154,9 @@ func (h *Handlers) storeParts(ctx context.Context, db *gorm.DB, bucket *blob.Buc *refusal = "No file: send at least one multipart part named \"file\"." return errRefused } + // The text job joins the transaction: it exists only when the + // files commit, and it syncs the material's document. + h.MaterialIndex.QueueExtract(ctx, tx, m.ID) return nil }) if errors.Is(err, errRefused) { diff --git a/controllers/api/material_search.go b/controllers/api/material_search.go index 7e7d28c..2b73810 100644 --- a/controllers/api/material_search.go +++ b/controllers/api/material_search.go @@ -9,6 +9,7 @@ import ( "git.golem15.com/golem15/summercms/modules/beachcomber" "git.golem15.com/golem15/summercms/modules/wire" "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" ) // Material search: the engine (beachcomber, Typesense) answers candidate ids @@ -66,17 +67,20 @@ type snippetJSON struct { } // pickSnippet returns the snippet of m from hit: the first body snippet, -// mapped one to one. It is nil when the hit has none. Call it only for a -// row the SQL re-gate returned. -func pickSnippet(hit beachcomber.Hit, m models.Material) *snippetJSON { +// mapped one to one, or else the first file_texts snippet, with the name of +// the file at its array index in files (the IndexedFiles of m, in document +// order; nil when the index does not resolve). A file text is long and is +// not loaded for the page, so a file snippet is always cut on both sides. +// It is nil when the hit has neither. Call it only for a row the SQL +// re-gate returned. +func pickSnippet(hit beachcomber.Hit, m models.Material, files []models.IndexedFile) *snippetJSON { for _, sn := range hit.Highlights { if sn.Field != "body" || len(sn.Segments) == 0 { continue } - out := &snippetJSON{Source: "body", Segments: make([]segmentJSON, 0, len(sn.Segments))} + out := &snippetJSON{Source: "body", Segments: snippetSegments(sn)} var joined strings.Builder for _, seg := range sn.Segments { - out.Segments = append(out.Segments, segmentJSON{Text: seg.Text, Hit: seg.Match}) joined.WriteString(seg.Text) } body := "" @@ -88,9 +92,66 @@ func pickSnippet(hit beachcomber.Hit, m models.Material) *snippetJSON { out.Trail = !strings.HasSuffix(body, text) return out } + for _, sn := range hit.Highlights { + if sn.Field != fileTextsField || len(sn.Segments) == 0 { + continue + } + out := &snippetJSON{Source: "file", Lead: true, Trail: true, Segments: snippetSegments(sn)} + if sn.ArrayIndex >= 0 && sn.ArrayIndex < len(files) { + name := files[sn.ArrayIndex].FileName + out.FileName = &name + } + return out + } return nil } +// fileTextsField is the document field of the attached files' texts. +const fileTextsField = "file_texts" + +// snippetSegments maps the engine's segments one to one. +func snippetSegments(sn beachcomber.Snippet) []segmentJSON { + out := make([]segmentJSON, 0, len(sn.Segments)) + for _, seg := range sn.Segments { + out = append(out, segmentJSON{Text: seg.Text, Hit: seg.Match}) + } + return out +} + +// hasFileSnippet reports whether hit carries a file_texts snippet and no +// body one, so its snippet needs the material's file names. +func hasFileSnippet(hit beachcomber.Hit) bool { + file := false + for _, sn := range hit.Highlights { + if len(sn.Segments) == 0 { + continue + } + switch sn.Field { + case "body": + return false + case fileTextsField: + file = true + } + } + return file +} + +// snippetFiles loads, once per page, the indexed files (names only) of the +// rows whose snippet comes from a file. It reads nothing when no row needs +// it. ids are rows SQL returned. +func snippetFiles(db *gorm.DB, ids []uint, hits map[uint]beachcomber.Hit) (map[uint][]models.IndexedFile, error) { + var need []uint + for _, id := range ids { + if hit, ok := hits[id]; ok && hasFileSnippet(hit) { + need = append(need, id) + } + } + if len(need) == 0 { + return nil, nil + } + return models.IndexedFiles(db, need, false) +} + // flattenSnippet is a snippet as plain text, for MCP: the segment texts // joined, with an ellipsis where text was cut. func flattenSnippet(s *snippetJSON) string { diff --git a/controllers/api/material_search_test.go b/controllers/api/material_search_test.go index d8034c5..d83da9a 100644 --- a/controllers/api/material_search_test.go +++ b/controllers/api/material_search_test.go @@ -33,7 +33,7 @@ func TestMaterialSnippet(t *testing.T) { {Field: "title", ArrayIndex: -1, Segments: segments("Tytuł", true)}, {Field: "body", ArrayIndex: -1, Segments: segments("Proces ", false, "globalizacji", true, " zmienia państwa.", false)}, }} - s := api.PickSnippet(hit, m) + s := api.PickSnippet(hit, m, nil) if s == nil { t.Fatal("no snippet") } @@ -59,24 +59,72 @@ func TestMaterialSnippet(t *testing.T) { // The whole body: neither lead nor trail. short := "Krótka **notatka**" - whole := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Krótka ", false, "notatka", true)}}}, models.Material{Body: &short}) + whole := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Krótka ", false, "notatka", true)}}}, models.Material{Body: &short}, nil) if whole == nil || whole.Lead || whole.Trail { t.Errorf("whole body snippet = %+v, want no lead or trail", whole) } // Start of the body only: trail but no lead. - start := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Wstęp ", false, "Na", true)}}}, m) + start := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Wstęp ", false, "Na", true)}}}, m, nil) if start == nil || start.Lead || !start.Trail { t.Errorf("start snippet = %+v, want trail only", start) } // A snippet on another field only, or none at all, gives no snippet. - if got := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "title", ArrayIndex: -1, Segments: segments("x", true)}}}, m); got != nil { + if got := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "title", ArrayIndex: -1, Segments: segments("x", true)}}}, m, nil); got != nil { t.Errorf("title-only hit gave %+v", got) } - if got := api.PickSnippet(beachcomber.Hit{ID: "3"}, m); got != nil { + if got := api.PickSnippet(beachcomber.Hit{ID: "3"}, m, nil); got != nil { t.Errorf("hit without highlights gave %+v", got) } + // A match only in an attached file: the file snippet, named by the + // snippet's array index into the indexed files, cut on both sides. + files := []models.IndexedFile{{FileID: 7, FileName: "wstep.pdf"}, {FileID: 9, FileName: "rozdzial-2.pdf"}} + fileHit := beachcomber.Hit{Highlights: []beachcomber.Snippet{ + {Field: "title", ArrayIndex: -1, Segments: segments("Tytuł", true)}, + {Field: "file_texts", ArrayIndex: 1, Segments: segments("o ", false, "demokracji", true, " w Europie", false)}, + }} + fs := api.PickSnippet(fileHit, m, files) + if fs == nil || fs.Source != "file" || fs.FileName == nil || *fs.FileName != "rozdzial-2.pdf" || !fs.Lead || !fs.Trail { + t.Fatalf("file snippet = %+v, want source file, rozdzial-2.pdf, lead and trail", fs) + } + if len(fs.Segments) != 3 || fs.Segments[1].Text != "demokracji" || !fs.Segments[1].Hit { + t.Errorf("file segments = %+v", fs.Segments) + } + fb, err := json.Marshal(fs) + if err != nil { + t.Fatal(err) + } + if got := string(fb); !strings.Contains(got, `"source":"file","file_name":"rozdzial-2.pdf","lead":true,"trail":true`) { + t.Errorf("file JSON = %s", got) + } + // The first file by index. + first := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "file_texts", ArrayIndex: 0, Segments: segments("x", true)}}}, m, files) + if first == nil || first.FileName == nil || *first.FileName != "wstep.pdf" { + t.Errorf("index 0 snippet = %+v, want wstep.pdf", first) + } + // An index the files do not resolve (out of range, -1, no files): no name. + for _, c := range []struct { + idx int + files []models.IndexedFile + }{{5, files}, {-1, files}, {0, nil}} { + got := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "file_texts", ArrayIndex: c.idx, Segments: segments("x", true)}}}, m, c.files) + if got == nil || got.Source != "file" || got.FileName != nil { + t.Errorf("index %d with %d files: snippet = %+v, want a file snippet with no name", c.idx, len(c.files), got) + } + } + // A body snippet is preferred over a file snippet, whatever the order. + both := beachcomber.Hit{Highlights: []beachcomber.Snippet{ + {Field: "file_texts", ArrayIndex: 0, Segments: segments("plik", true)}, + {Field: "body", ArrayIndex: -1, Segments: segments("Proces ", false, "globalizacji", true)}, + }} + if got := api.PickSnippet(both, m, files); got == nil || got.Source != "body" || got.FileName != nil { + t.Errorf("body and file hit gave %+v, want the body snippet", got) + } + if !api.HasFileSnippet(fileHit) || api.HasFileSnippet(both) || api.HasFileSnippet(hit) { + t.Error("HasFileSnippet: want true for a file-only hit, false with a body snippet") + } + if got := api.FlattenSnippet(s); got != "…Proces globalizacji zmienia państwa.…" { t.Errorf("FlattenSnippet = %q", got) } @@ -221,19 +269,19 @@ func TestManagedMaterialsSearch(t *testing.T) { func TestMaterialSnippetMCP(t *testing.T) { body := "Wstęp. Proces globalizacji zmienia państwa. Koniec." m := models.Material{Body: &body} - mid := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Proces ", false, "globalizacji", true, " zmienia", false)}}}, m) + mid := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Proces ", false, "globalizacji", true, " zmienia", false)}}}, m, nil) if got := api.FlattenSnippet(mid); got != "…Proces globalizacji zmienia…" { t.Errorf("middle = %q, want ellipses on both sides", got) } - head := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Wstęp", true, ". Proces", false)}}}, m) + head := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("Wstęp", true, ". Proces", false)}}}, m, nil) if got := api.FlattenSnippet(head); got != "Wstęp. Proces…" { t.Errorf("head = %q, want a trailing ellipsis only", got) } - tail := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("państwa. ", false, "Koniec", true, ".", false)}}}, m) + tail := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments("państwa. ", false, "Koniec", true, ".", false)}}}, m, nil) if got := api.FlattenSnippet(tail); got != "…państwa. Koniec." { t.Errorf("tail = %q, want a leading ellipsis only", got) } - whole := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments(body, false)}}}, m) + whole := api.PickSnippet(beachcomber.Hit{Highlights: []beachcomber.Snippet{{Field: "body", ArrayIndex: -1, Segments: segments(body, false)}}}, m, nil) if got := api.FlattenSnippet(whole); got != body { t.Errorf("whole = %q, want no ellipsis", got) } diff --git a/controllers/api/materials.go b/controllers/api/materials.go index 8311304..bd0a6b0 100644 --- a/controllers/api/materials.go +++ b/controllers/api/materials.go @@ -157,15 +157,21 @@ func (h *Handlers) Materials(w http.ResponseWriter, r *http.Request) { return } if cands != nil { - attachSnippets(out, rows, cands.Hits, viaMCP(r)) + files, err := snippetFiles(db, materialIDs(rows), cands.Hits) + if err != nil { + writeOpaque500(w) + return + } + attachSnippets(out, rows, cands.Hits, files, viaMCP(r)) } writeJSONPage(w, lagoon.Paginate(out, page, perPage, total)) } -// attachSnippets sets the snippet of each list row from its Hit. rows are +// attachSnippets sets the snippet of each list row from its Hit and, for a +// file snippet, the row's indexed files. rows are // the rows SQL returned (they passed the re-gate), in the order of out; the // Hit of an id SQL did not return is never read. -func attachSnippets(out []materialListJSON, rows []models.Material, hits map[uint]beachcomber.Hit, mcp bool) { +func attachSnippets(out []materialListJSON, rows []models.Material, hits map[uint]beachcomber.Hit, files map[uint][]models.IndexedFile, mcp bool) { for i := range out { if i >= len(rows) || rows[i].ID != out[i].ID { continue @@ -174,10 +180,19 @@ func attachSnippets(out []materialListJSON, rows []models.Material, hits map[uin if !ok { continue } - out[i].Snippet = snippetValue(pickSnippet(hit, rows[i]), mcp) + out[i].Snippet = snippetValue(pickSnippet(hit, rows[i], files[rows[i].ID]), mcp) } } +// materialIDs are the ids of rows, in order. +func materialIDs(rows []models.Material) []uint { + ids := make([]uint, 0, len(rows)) + for _, m := range rows { + ids = append(ids, m.ID) + } + return ids +} + // Material answers GET /materials/{id}: one published material the caller // may see, with its body, tags and files. A draft, a future publication or // another user's private material is a 404. can_manage tells the author (a diff --git a/internal/pdftest/pdf.go b/internal/pdftest/pdf.go new file mode 100644 index 0000000..94e47c0 --- /dev/null +++ b/internal/pdftest/pdf.go @@ -0,0 +1,42 @@ +// Package pdftest builds small PDFs for the jz.bm tests. Only tests import +// it. +package pdftest + +import ( + "bytes" + "fmt" + "strings" +) + +// OnePage is a one-page PDF that shows text in the base-14 Helvetica font, +// so pdftotext extracts it without any embedded font. text must be ASCII +// and hold no parentheses or backslashes. The xref offsets are computed from +// the objects as written. +func OnePage(text string) []byte { + if strings.ContainsAny(text, "()\\") { + panic("pdftest: text holds a parenthesis or a backslash") + } + stream := "BT /F1 24 Tf 72 700 Td (" + text + ") Tj ET" + objects := []string{ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >>", + fmt.Sprintf("<< /Length %d >>\nstream\n%s\nendstream", len(stream), stream), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", + } + var b bytes.Buffer + b.WriteString("%PDF-1.4\n") + offsets := make([]int, len(objects)) + for i, obj := range objects { + offsets[i] = b.Len() + fmt.Fprintf(&b, "%d 0 obj\n%s\nendobj\n", i+1, obj) + } + xref := b.Len() + fmt.Fprintf(&b, "xref\n0 %d\n", len(objects)+1) + b.WriteString("0000000000 65535 f \n") + for _, off := range offsets { + fmt.Fprintf(&b, "%010d 00000 n \n", off) + } + fmt.Fprintf(&b, "trailer\n<< /Size %d /Root 1 0 R >>\nstartxref\n%d\n%%%%EOF\n", len(objects)+1, xref) + return b.Bytes() +} diff --git a/jobs.go b/jobs.go new file mode 100644 index 0000000..b470e2f --- /dev/null +++ b/jobs.go @@ -0,0 +1,47 @@ +package bm + +import ( + "context" + "errors" + "sync" + + "git.golem15.com/golem15/summercms/modules/diary" + "git.golem15.com/jakub/sm-bm-plugin/classes" + "git.golem15.com/jakub/sm-bm-plugin/jobs" + "gocloud.dev/blob" + "gorm.io/gorm" +) + +// pdfToolWarning is logged at most once per process when pdftotext is +// missing. +var pdfToolWarning sync.Once + +// extractMaterialText runs one jz.bm.extract_material_text job: the text of +// the material's PDFs that have none yet is extracted and stored, then the +// material's search document is synced. A missing database or bucket is an +// error, so River retries; a missing pdftotext is logged once and the +// document is still synced. +func (p *Plugin) extractMaterialText(ctx context.Context, args jobs.ExtractMaterialTextArgs) error { + if p.app == nil { + return errors.New("jz.bm: app is not booted") + } + db, ok := p.app.Lookup[*gorm.DB]() + if !ok || db == nil { + return errors.New("jz.bm: database is not configured") + } + bucket, ok := p.app.Lookup[*blob.Bucket]() + if !ok || bucket == nil { + return errors.New("jz.bm: the uploads bucket is not configured") + } + _, err := classes.EnsureFileTexts(ctx, db, bucket, classes.DefaultPDFText(), args.MaterialID) + switch { + case errors.Is(err, classes.ErrPDFToolMissing): + pdfToolWarning.Do(func() { + diary.For("jz.bm").Warn("pdftotext not found; attached PDFs add no search text until poppler-utils is installed") + }) + case err != nil: + return err + } + p.materialIndex.Sync(ctx, db, args.MaterialID) + return nil +} diff --git a/jobs/extract_material_text.go b/jobs/extract_material_text.go new file mode 100644 index 0000000..7c57704 --- /dev/null +++ b/jobs/extract_material_text.go @@ -0,0 +1,15 @@ +package jobs + +// ExtractMaterialTextKind is the River kind of the file text extraction job. +const ExtractMaterialTextKind = "jz.bm.extract_material_text" + +// ExtractMaterialTextArgs are the arguments of the job that extracts the +// text of a material's new PDFs and resyncs its search document. It is +// enqueued inside every attach transaction, so it exists only when the +// files committed. +type ExtractMaterialTextArgs struct { + MaterialID uint `json:"material_id"` +} + +// Kind is the job kind. +func (ExtractMaterialTextArgs) Kind() string { return ExtractMaterialTextKind } diff --git a/jobs/extract_material_text_test.go b/jobs/extract_material_text_test.go new file mode 100644 index 0000000..2ef51c9 --- /dev/null +++ b/jobs/extract_material_text_test.go @@ -0,0 +1,29 @@ +package jobs_test + +import ( + "encoding/json" + "testing" + + "git.golem15.com/jakub/sm-bm-plugin/jobs" +) + +func TestExtractMaterialTextArgs(t *testing.T) { + args := jobs.ExtractMaterialTextArgs{MaterialID: 42} + if got := args.Kind(); got != "jz.bm.extract_material_text" { + t.Errorf("Kind = %q, want jz.bm.extract_material_text", got) + } + if jobs.ExtractMaterialTextKind != "jz.bm.extract_material_text" { + t.Errorf("ExtractMaterialTextKind = %q", jobs.ExtractMaterialTextKind) + } + b, err := json.Marshal(args) + if err != nil { + t.Fatal(err) + } + if string(b) != `{"material_id":42}` { + t.Errorf("JSON = %s, want {\"material_id\":42}", b) + } + var back jobs.ExtractMaterialTextArgs + if err := json.Unmarshal(b, &back); err != nil || back.MaterialID != 42 { + t.Errorf("round trip = %+v, %v", back, err) + } +} diff --git a/models/material_file_text.go b/models/material_file_text.go new file mode 100644 index 0000000..a8c788f --- /dev/null +++ b/models/material_file_text.go @@ -0,0 +1,18 @@ +package models + +import "time" + +// MaterialFileText is the text extracted from one PDF attached to a +// material. An empty Text means the file was extracted and held none (a +// scan or a broken PDF). Truncated is set when the text was cut at the +// extraction cap. The row goes with its file (foreign-key cascade). +type MaterialFileText struct { + FileID uint `gorm:"column:file_id;primaryKey;autoIncrement:false"` + MaterialID uint `gorm:"column:material_id"` + Text string `gorm:"column:text"` + Truncated bool `gorm:"column:truncated"` + ExtractedAt time.Time `gorm:"column:extracted_at"` +} + +// TableName is the file text table. +func (MaterialFileText) TableName() string { return "jz_bm_material_file_texts" } diff --git a/models/material_search.go b/models/material_search.go index 5c145b1..d3665d5 100644 --- a/models/material_search.go +++ b/models/material_search.go @@ -58,7 +58,71 @@ func (m *Material) ToSearchableArray(ctx context.Context, db *gorm.DB) (map[stri if err != nil { return nil, err } - return MaterialDocument(*m, course, meetingNumber, tags, nil), nil + files, err := IndexedFiles(db, []uint{m.ID}, true) + if err != nil { + return nil, err + } + texts := make([]string, 0, len(files[m.ID])) + for _, f := range files[m.ID] { + texts = append(texts, f.Text) + } + return MaterialDocument(*m, course, meetingNumber, tags, texts), nil +} + +// IndexedFile is one entry of a material document's file_texts: the file, +// its name and, when asked for, its extracted text. +type IndexedFile struct { + FileID uint + FileName string + Text string +} + +// IndexedFiles returns, per material id, the files whose text the search +// document holds, in document order: the current public files of the +// material's files field that have a non-empty extracted text, by +// sort_order (nulls last) and id. It is the one query that decides what +// file_texts holds, so a snippet's array index maps back to a file through +// it. A detached file is no longer a current file, so its text drops out +// even before the cascade removes it. The text is read only when withText. +func IndexedFiles(db *gorm.DB, materialIDs []uint, withText bool) (map[uint][]IndexedFile, error) { + out := map[uint][]IndexedFile{} + if len(materialIDs) == 0 { + return out, nil + } + ids := make([]string, 0, len(materialIDs)) + for _, id := range materialIDs { + ids = append(ids, strconv.FormatUint(uint64(id), 10)) + } + cols := "system_files.id AS file_id, system_files.file_name, system_files.attachment_id" + if withText { + cols += ", t.text" + } + var rows []struct { + FileID uint + FileName string + AttachmentID string + Text string + } + err := db.Table("system_files"). + Select(cols). + Joins("JOIN jz_bm_material_file_texts t ON t.file_id = system_files.id"). + Where("system_files.attachment_type = ? AND system_files.field = ? AND system_files.is_public", Material{}.MorphName(), MaterialFilesField). + Where("system_files.attachment_id IN ?", ids). + Where("t.text <> ''"). + Order("system_files.attachment_id, system_files.sort_order NULLS LAST, system_files.id"). + Scan(&rows).Error + if err != nil { + return nil, err + } + for _, r := range rows { + n, err := strconv.ParseUint(r.AttachmentID, 10, 64) + if err != nil { + continue + } + id := uint(n) + out[id] = append(out[id], IndexedFile{FileID: r.FileID, FileName: r.FileName, Text: r.Text}) + } + return out, nil } // MaterialDocument is the search document of m with its course, meeting diff --git a/models/material_search_test.go b/models/material_search_test.go index 9b32ae6..0ea82cb 100644 --- a/models/material_search_test.go +++ b/models/material_search_test.go @@ -102,6 +102,16 @@ func TestMaterialDocument(t *testing.T) { t.Errorf("file_texts = %v", doc["file_texts"]) } + // File texts keep their order (the IndexedFiles order); an empty list + // leaves the key out. + ordered := models.MaterialDocument(m, course, 3, tags, []string{"pierwszy plik", "drugi plik", "trzeci plik"}) + if got, _ := ordered["file_texts"].([]string); !slices.Equal(got, []string{"pierwszy plik", "drugi plik", "trzeci plik"}) { + t.Errorf("file_texts = %v, want the three texts in order", ordered["file_texts"]) + } + if _, ok := models.MaterialDocument(m, course, 3, tags, []string{})["file_texts"]; ok { + t.Error("an empty file text list carries file_texts") + } + // No code, no course id, no meeting, a draft, no files. bare := models.MaterialDocument(models.Material{ID: 5, Title: "Szkic", Type: "inne"}, &models.Course{Name: "Teoria"}, 0, nil, nil) if bare["course"] != "Teoria" { diff --git a/plugin.go b/plugin.go index 2195615..3de260a 100644 --- a/plugin.go +++ b/plugin.go @@ -5,9 +5,11 @@ import ( "embed" "errors" "io/fs" + "time" "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/bonfire" + "git.golem15.com/golem15/summercms/modules/conga" "git.golem15.com/golem15/summercms/modules/lagoon" "git.golem15.com/golem15/summercms/modules/lighthouse" "git.golem15.com/golem15/summercms/modules/lighthouse/centrifugo" @@ -108,7 +110,15 @@ func (p *Plugin) MailLayouts() map[string]string { return nil } func (p *Plugin) Models() []any { return generatedModels() } func (p *Plugin) Migrations() []*gormigrate.Migration { return generatedMigrations() } -func (p *Plugin) Jobs() []pact.Job { return generatedJobs() } + +// Jobs returns the generated jobs and jz.bm.extract_material_text, which +// extracts the text of attached PDFs for search: three attempts, two +// minutes each. +func (p *Plugin) Jobs() []pact.Job { + timeout := conga.Timeout(2 * time.Minute) + return append(generatedJobs(), conga.Job(p.extractMaterialText, conga.MaxAttempts(3), timeout)) +} + func (p *Plugin) AdminControllers() []pact.AdminController { return generatedAdminControllers() } diff --git a/plugin_test.go b/plugin_test.go index 032c873..3bebadc 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -79,8 +79,8 @@ func TestPluginActivates(t *testing.T) { if n := len(p.Models()); n != 14 { t.Errorf("Models = %d, want 14", n) } - if n := len(p.Migrations()); n != 19 { - t.Errorf("Migrations = %d, want 19", n) + if n := len(p.Migrations()); n != 20 { + t.Errorf("Migrations = %d, want 20", n) } if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:seed" }) { t.Error("bm:seed is not registered") diff --git a/registry.gen.go b/registry.gen.go index 259e627..013ef19 100644 --- a/registry.gen.go +++ b/registry.gen.go @@ -51,6 +51,7 @@ func generatedMigrations() []*gormigrate.Migration { updates.CreateChecklists(), updates.CreateChecklistItems(), updates.AddPrivacyToMaterials(), + updates.CreateMaterialFileTexts(), } } diff --git a/updates/20261108130000_create_jz_bm_material_file_texts.go b/updates/20261108130000_create_jz_bm_material_file_texts.go new file mode 100644 index 0000000..568f6fe --- /dev/null +++ b/updates/20261108130000_create_jz_bm_material_file_texts.go @@ -0,0 +1,34 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// CreateMaterialFileTexts returns the +// 20261108130000_create_jz_bm_material_file_texts gormigrate entry: the text +// pdftotext extracted from each PDF attached to a material, cached per file +// so the search document can carry it without running the tool again. An +// empty text means the file was extracted and held none (a scan or a broken +// PDF), so it is not retried. The foreign key to system_files cascades, so +// detaching a file removes its text; deleting a material removes the texts +// of its files. +func CreateMaterialFileTexts() *gormigrate.Migration { + return &gormigrate.Migration{ + ID: "20261108130000_create_jz_bm_material_file_texts", + Migrate: func(tx *gorm.DB) error { + return execAll(tx, ` +CREATE TABLE jz_bm_material_file_texts ( + file_id INTEGER PRIMARY KEY REFERENCES system_files (id) ON DELETE CASCADE, + material_id BIGINT NOT NULL REFERENCES jz_bm_materials (id) ON DELETE CASCADE, + text TEXT NOT NULL DEFAULT '', + truncated BOOLEAN NOT NULL DEFAULT FALSE, + extracted_at TIMESTAMP(0) WITHOUT TIME ZONE NOT NULL +); +CREATE INDEX jz_bm_material_file_texts_material_id_index ON jz_bm_material_file_texts (material_id)`) + }, + Rollback: func(tx *gorm.DB) error { + return execAll(tx, `DROP TABLE IF EXISTS jz_bm_material_file_texts`) + }, + } +}