diff --git a/console/materials_reindex.go b/console/materials_reindex.go new file mode 100644 index 0000000..e573f8b --- /dev/null +++ b/console/materials_reindex.go @@ -0,0 +1,139 @@ +package console + +import ( + "context" + "errors" + "fmt" + + "git.golem15.com/golem15/summercms/modules/beachcomber" + "git.golem15.com/golem15/summercms/modules/bonfire" + "git.golem15.com/jakub/sm-bm-plugin/classes" + "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" +) + +// MaterialsReindexName is the name of the materials reindex command. +const MaterialsReindexName = "bm:materials-reindex" + +// materialsReindexBatch is how many material ids one database read loads. +const materialsReindexBatch = 200 + +// errSearchNotConfigured is the refusal of a reindex without an engine. +var errSearchNotConfigured = errors.New("jz.bm: the search engine is not configured (set search.driver and search.typesense.api_key)") + +// MaterialsReindexDeps are what the reindex command needs. DB returns the +// application database, published on the app (Sync skips silently when no +// database is published), and done, which closes a pool DB opened. Index +// reaches the beachcomber service. +type MaterialsReindexDeps struct { + DB func(ctx context.Context) (*gorm.DB, func(), error) + Index *classes.MaterialIndexer +} + +// MaterialsReindexCommand returns bm:materials-reindex: it ensures the +// materials index (dropping it first with --fresh) and syncs every +// material. It refuses with an error when the engine is not configured, +// unless --if-configured, which the nightly schedule passes, makes that a +// notice. +func MaterialsReindexCommand(deps MaterialsReindexDeps) bonfire.Command { + return bonfire.Command{ + Name: MaterialsReindexName, + Description: "Sync every material into the search index", + Long: `Upserts the search document of every material, drafts and private ones +included (visibility is enforced in SQL, never by the index), through the same +path as the sync after a save. + +--fresh first drops the collection and recreates it from the material schema +(models.Material.SearchIndexSchema). Use it on first deploy and after the +schema changed; search finds nothing while the collection is being refilled. + +The command refuses, and sends nothing, when the search engine is not +configured (search.driver null, or no search.typesense.api_key). +--if-configured makes that a notice and a zero exit; the nightly schedule +runs the command with it. It ends with "Reindexed: N synced, M failed" and +exits non-zero when any material failed.`, + Flags: []bonfire.Flag{ + {Name: "fresh", Description: "Drop and recreate the collection from the schema before syncing", Bare: true}, + {Name: "if-configured", Description: "Exit 0 with a notice when the search engine is not configured", Bare: true}, + }, + Examples: []bonfire.Example{ + {Command: MaterialsReindexName, Description: "Sync every material into the existing collection"}, + {Command: MaterialsReindexName + " --fresh", Description: "Recreate the collection from the schema, then sync every material"}, + }, + Run: func(ctx context.Context, in bonfire.Input, out bonfire.Output) error { + return reindexMaterials(ctx, deps, flagOn(in, "fresh"), flagOn(in, "if-configured"), out) + }, + } +} + +func reindexMaterials(ctx context.Context, deps MaterialsReindexDeps, fresh, ifConfigured bool, out bonfire.Output) error { + var svc *beachcomber.Service + if deps.Index != nil && deps.Index.Service != nil { + svc = deps.Index.Service() + } + engine := svc.Engine() + if engine == nil || !engine.Configured() { + if ifConfigured { + out.Info(MaterialsReindexName + ": search is not configured; nothing to do") + return nil + } + return errSearchNotConfigured + } + if deps.DB == nil { + return errors.New("jz.bm: no database") + } + gdb, done, err := deps.DB(ctx) + if err != nil { + return err + } + defer done() + + index := svc.IndexName(&models.Material{}) + if fresh { + existed, err := beachcomber.DropIndex(ctx, engine, index) + if err != nil { + return fmt.Errorf("jz.bm: drop %s: %w", index, err) + } + if existed { + out.Printf("Dropped %s\n", index) + } + } + if err := beachcomber.EnsureIndex(ctx, engine, index, models.Material{}.SearchIndexSchema()); err != nil { + return fmt.Errorf("jz.bm: create %s: %w", index, err) + } + + marked := classes.MarkMaterialSearch(ctx) + synced, failed := 0, 0 + var batch []models.Material + err = gdb.WithContext(ctx).Model(&models.Material{}). + Select("id"). + Order("id"). + FindInBatches(&batch, materialsReindexBatch, func(_ *gorm.DB, _ int) error { + for i := range batch { + id := batch[i].ID + if err := svc.Sync(marked, gdb, &models.Material{ID: id}); err != nil { + failed++ + out.Warning(fmt.Sprintf("material %d: %v", id, err)) + continue + } + synced++ + } + return ctx.Err() + }).Error + if err != nil { + return err + } + msg := fmt.Sprintf("Reindexed: %d synced, %d failed", synced, failed) + if failed > 0 { + out.Error(msg) + return fmt.Errorf("jz.bm: %d materials failed to sync", failed) + } + out.Success(msg) + return nil +} + +// flagOn reports whether a bare flag was given. +func flagOn(in bonfire.Input, name string) bool { + v, _ := in.Flag(name) + return v == "true" +} diff --git a/console/materials_reindex_test.go b/console/materials_reindex_test.go new file mode 100644 index 0000000..f622982 --- /dev/null +++ b/console/materials_reindex_test.go @@ -0,0 +1,89 @@ +package console_test + +import ( + "bytes" + "context" + "net/http" + "slices" + "strings" + "testing" + + "git.golem15.com/golem15/summercms/modules/bonfire" + "git.golem15.com/golem15/summercms/modules/lagoon" + "git.golem15.com/jakub/sm-bm-plugin/classes" + "git.golem15.com/jakub/sm-bm-plugin/console" + "git.golem15.com/jakub/sm-bm-plugin/internal/searchtest" + "gorm.io/driver/postgres" + "gorm.io/gorm" +) + +func TestMaterialsReindexRefuses(t *testing.T) { + asked := false + deps := console.MaterialsReindexDeps{ + DB: func(context.Context) (*gorm.DB, func(), error) { + asked = true + return nil, func() {}, nil + }, + Index: classes.NewMaterialIndexer(searchtest.App(t)), + } + cmds := []bonfire.Command{console.MaterialsReindexCommand(deps)} + var out bytes.Buffer + err := bonfire.Call(context.Background(), cmds, console.MaterialsReindexName, nil, &out) + if err == nil || !strings.Contains(err.Error(), "search.driver") || !strings.Contains(err.Error(), "search.typesense.api_key") { + t.Errorf("unconfigured engine: err = %v, want the configuration hint", err) + } + out.Reset() + if err := bonfire.Call(context.Background(), cmds, console.MaterialsReindexName, []string{"--if-configured"}, &out); err != nil { + t.Errorf("--if-configured: %v", err) + } + if !strings.Contains(out.String(), "not configured") { + t.Errorf("--if-configured output %q, want the notice", out.String()) + } + if asked { + t.Error("the command opened the database without an engine") + } +} + +func TestMaterialsReindexFresh(t *testing.T) { + stub := &searchtest.Stub{} + app := searchtest.App(t, searchtest.Start(t, stub)...) + gdb, err := gorm.Open(postgres.New(postgres.Config{DSN: "host=127.0.0.1"}), &gorm.Config{DryRun: true, DisableAutomaticPing: true}) + if err != nil { + t.Fatal(err) + } + sqlDB, err := gdb.DB() + if err != nil { + t.Fatal(err) + } + if err := lagoon.Publish(app, sqlDB, gdb); err != nil { + t.Fatal(err) + } + deps := console.MaterialsReindexDeps{ + DB: func(context.Context) (*gorm.DB, func(), error) { return gdb, func() {}, nil }, + Index: classes.NewMaterialIndexer(app), + } + cmds := []bonfire.Command{console.MaterialsReindexCommand(deps)} + coll := "/collections/" + searchtest.Index + var out bytes.Buffer + for run := 1; run <= 2; run++ { + before := len(stub.Requests()) + out.Reset() + if err := bonfire.Call(context.Background(), cmds, console.MaterialsReindexName, []string{"--fresh"}, &out); err != nil { + t.Fatalf("run %d: %v\n%s", run, err, out.String()) + } + var sent []string + for _, r := range stub.Requests()[before:] { + sent = append(sent, r.Method+" "+r.Path) + } + if want := []string{http.MethodDelete + " " + coll, http.MethodGet + " " + coll, http.MethodPost + " /collections"}; !slices.Equal(sent, want) { + t.Errorf("run %d sent %v, want %v", run, sent, want) + } + if !strings.Contains(out.String(), "Reindexed: 0 synced, 0 failed") { + t.Errorf("run %d output %q", run, out.String()) + } + } + // The second run dropped the collection the first one created. + if !strings.Contains(out.String(), "Dropped "+searchtest.Index) { + t.Errorf("second run output %q, want Dropped", out.String()) + } +} diff --git a/controllers/api/manage.go b/controllers/api/manage.go index 5728194..d732fbb 100644 --- a/controllers/api/manage.go +++ b/controllers/api/manage.go @@ -498,6 +498,7 @@ func (h *Handlers) UpsertMaterial(w http.ResponseWriter, r *http.Request) { writeOpaque500(w) return } + h.MaterialIndex.Sync(r.Context(), db, m.ID) status := http.StatusOK if created { status = http.StatusCreated @@ -556,6 +557,7 @@ func (h *Handlers) UpdateMaterial(w http.ResponseWriter, r *http.Request) { case taken: writeValidation(w, map[string][]string{"title": {"Another material already has this title and course."}}) default: + h.MaterialIndex.Sync(r.Context(), db, m.ID) writeManaged(w, r, db, m.ID, http.StatusOK, nil) } } @@ -659,6 +661,9 @@ func (h *Handlers) DeleteMaterial(w http.ResponseWriter, r *http.Request) { writeNotFound(w) return } + // The delete statement has no primary key on its model, so no callback + // removes the document: remove it now that the transaction committed. + h.MaterialIndex.Remove(r.Context(), db, uint(id)) // The files go with the material: rows in the transaction, blobs after it. if b := h.Bucket; b != nil && len(keys) > 0 { if bucket := b(); bucket != nil { @@ -734,6 +739,8 @@ func (h *Handlers) managedAccessChange(w http.ResponseWriter, r *http.Request, d writeOpaque500(w) return } + // An access change touches only visibility (and the author), which the + // search index does not hold, so it does not sync. writeManaged(w, r, db, m.ID, http.StatusOK, nil) } diff --git a/controllers/api/material_access.go b/controllers/api/material_access.go index e87bf46..90d0c0d 100644 --- a/controllers/api/material_access.go +++ b/controllers/api/material_access.go @@ -322,7 +322,8 @@ func (h *Handlers) UpdateMaterialAccess(w http.ResponseWriter, r *http.Request) // applyMaterialAccess writes a visibility and share change onto m inside // tx. A material with no author gets userID as author (the claim rule). // Only is_private and author_user_id are updated; the shares are replaced -// when setShares. +// when setShares. The search index holds none of these, so no caller syncs +// after it. func applyMaterialAccess(tx *gorm.DB, m *models.Material, userID uint, isPrivate *bool, setShares bool, shares []uint) error { if m.AuthorUserID == nil { id := userID diff --git a/controllers/materials_hooks.go b/controllers/materials_hooks.go new file mode 100644 index 0000000..9267664 --- /dev/null +++ b/controllers/materials_hooks.go @@ -0,0 +1,77 @@ +package controllers + +import ( + "context" + "errors" + "sync/atomic" + + "git.golem15.com/golem15/summercms/modules/cabana" + "git.golem15.com/golem15/summercms/modules/pact" + "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" +) + +var ( + _ pact.FormAfterCreate = materialsAdmin{} + _ pact.FormAfterUpdate = materialsAdmin{} + _ pact.FormAfterDelete = materialsAdmin{} +) + +// MaterialIndex keeps the materials search index current once a write +// commits (classes.MaterialIndexer). Its calls never fail the write. +type MaterialIndex interface { + SyncAfterCommit(ctx context.Context, tx *gorm.DB, id uint) + RemoveAfterCommit(ctx context.Context, tx *gorm.DB, id uint) +} + +type materialIndexHolder struct{ ix MaterialIndex } + +var materialIndex atomic.Value + +// UseMaterialIndex installs the index the materials admin syncs after a +// save or delete; nil turns the syncs off. Boot installs it, because the +// generated controller value has no field to carry it. +func UseMaterialIndex(ix MaterialIndex) { + materialIndex.Store(materialIndexHolder{ix: ix}) +} + +func installedMaterialIndex() MaterialIndex { + h, _ := materialIndex.Load().(materialIndexHolder) + return h.ix +} + +// FormAfterCreate syncs the new material once the save transaction commits. +func (materialsAdmin) FormAfterCreate(ctx context.Context, model any) error { + return materialAfterCommit(ctx, model, false) +} + +// FormAfterUpdate syncs the material once the save transaction commits. +func (materialsAdmin) FormAfterUpdate(ctx context.Context, model any) error { + return materialAfterCommit(ctx, model, false) +} + +// FormAfterDelete removes the material's document once the delete commits. +func (materialsAdmin) FormAfterDelete(ctx context.Context, model any) error { + return materialAfterCommit(ctx, model, true) +} + +// materialAfterCommit schedules the index work of an admin save on the save +// transaction. Only a model that is not a material is an error: the index +// never fails a save. +func materialAfterCommit(ctx context.Context, model any, remove bool) error { + m, ok := model.(*models.Material) + if !ok || m == nil { + return errors.New("materials admin: form model is not a material") + } + ix := installedMaterialIndex() + if ix == nil || m.ID == 0 { + return nil + } + tx, _ := cabana.TxFromContext(ctx) + if remove { + ix.RemoveAfterCommit(ctx, tx, m.ID) + } else { + ix.SyncAfterCommit(ctx, tx, m.ID) + } + return nil +} diff --git a/controllers/materials_hooks_test.go b/controllers/materials_hooks_test.go new file mode 100644 index 0000000..5813170 --- /dev/null +++ b/controllers/materials_hooks_test.go @@ -0,0 +1,81 @@ +package controllers + +import ( + "context" + "slices" + "strconv" + "testing" + + "git.golem15.com/jakub/sm-bm-plugin/models" + "gorm.io/gorm" +) + +// recordingIndex records the after-commit calls of the materials admin. +type recordingIndex struct { + calls []string + txs []*gorm.DB +} + +func (r *recordingIndex) SyncAfterCommit(_ context.Context, tx *gorm.DB, id uint) { + r.calls = append(r.calls, "sync "+itoa(id)) + r.txs = append(r.txs, tx) +} + +func (r *recordingIndex) RemoveAfterCommit(_ context.Context, tx *gorm.DB, id uint) { + r.calls = append(r.calls, "remove "+itoa(id)) + r.txs = append(r.txs, tx) +} + +func itoa(id uint) string { return strconv.FormatUint(uint64(id), 10) } + +func TestMaterialsAdminHooks(t *testing.T) { + t.Cleanup(func() { UseMaterialIndex(nil) }) + var c materialsAdmin + ctx := context.Background() + m := &models.Material{ID: 7} + + // No index installed: nothing to do, and no error. + UseMaterialIndex(nil) + for name, fn := range map[string]func(context.Context, any) error{"create": c.FormAfterCreate, "update": c.FormAfterUpdate, "delete": c.FormAfterDelete} { + if err := fn(ctx, m); err != nil { + t.Errorf("%s without an index: %v", name, err) + } + } + + rec := &recordingIndex{} + UseMaterialIndex(rec) + if err := c.FormAfterCreate(ctx, m); err != nil { + t.Fatal(err) + } + if err := c.FormAfterUpdate(ctx, m); err != nil { + t.Fatal(err) + } + if err := c.FormAfterDelete(ctx, m); err != nil { + t.Fatal(err) + } + if want := []string{"sync 7", "sync 7", "remove 7"}; !slices.Equal(rec.calls, want) { + t.Errorf("calls = %v, want %v", rec.calls, want) + } + // Outside a cabana write there is no transaction: the index then runs + // at once (lagoon.AfterCommit with a nil handle). + for i, tx := range rec.txs { + if tx != nil { + t.Errorf("call %d got a transaction outside a write", i) + } + } + + // A model of another type is an error and schedules nothing. + rec.calls = nil + for name, fn := range map[string]func(context.Context, any) error{"create": c.FormAfterCreate, "update": c.FormAfterUpdate, "delete": c.FormAfterDelete} { + if err := fn(ctx, &models.Course{ID: 7}); err == nil { + t.Errorf("%s with a course: no error", name) + } + } + var nilMaterial *models.Material + if err := c.FormAfterUpdate(ctx, nilMaterial); err == nil { + t.Error("update with a nil material: no error") + } + if len(rec.calls) != 0 { + t.Errorf("calls for a wrong model = %v", rec.calls) + } +} diff --git a/internal/searchtest/typesense.go b/internal/searchtest/typesense.go index f559a26..7a14b61 100644 --- a/internal/searchtest/typesense.go +++ b/internal/searchtest/typesense.go @@ -130,6 +130,13 @@ func (s *Stub) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } +// SetHits replaces the answer of every later search. +func (s *Stub) SetHits(hits ...Hit) { + s.mu.Lock() + defer s.mu.Unlock() + s.Hits = hits +} + // Requests returns the requests received so far. func (s *Stub) Requests() []Request { s.mu.Lock() diff --git a/material_search_postgres_test.go b/material_search_postgres_test.go new file mode 100644 index 0000000..31afe7b --- /dev/null +++ b/material_search_postgres_test.go @@ -0,0 +1,148 @@ +package bm_test + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + + userclasses "git.golem15.com/golem15/sm-user-plugin/classes" + "git.golem15.com/golem15/summercms/modules/surf" + "git.golem15.com/jakub/sm-bm-plugin/internal/searchtest" +) + +// TestMaterialSearchAgainstDatabase drives the material search end to end +// against a migrated database and a Typesense stand-in: every management +// write syncs the index, a delete removes the document, and a search whose +// candidates include a draft and another user's private material shows a +// student only the public one, counted in SQL, with no snippet of the +// hidden rows. +func TestMaterialSearchAgainstDatabase(t *testing.T) { + stub := &searchtest.Stub{} + app, plugins, gdb := migratedWith(t, searchtest.Start(t, stub)...) + handler, err := surf.Assemble(app, plugins) + if err != nil { + t.Fatal(err) + } + managerID := createUser(t, gdb, "manager@example.test") + otherID := createUser(t, gdb, "other@example.test") + studentID := createUser(t, gdb, "student@example.test") + var groupID uint + if err := gdb.Raw(` +INSERT INTO user_groups (name, code, permissions, created_at, updated_at) +VALUES ('BM materials', 'bm-materials', '{"jz.bm.access_materials":1}', NOW(), NOW()) +RETURNING id`).Scan(&groupID).Error; err != nil { + t.Fatal(err) + } + if err := gdb.Exec(`INSERT INTO users_groups (user_id, user_group_id) VALUES (?, ?)`, managerID, groupID).Error; err != nil { + t.Fatal(err) + } + secret, token, err := userclasses.MintAPIToken("usr_", managerID, "BM search", []string{"read", "write"}, nil, nil) + if err != nil { + t.Fatal(err) + } + if err := gdb.Create(token).Error; err != nil { + t.Fatal(err) + } + send := func(method, path string, payload any) (int, map[string]any) { + t.Helper() + var buf bytes.Buffer + if payload != nil { + if err := json.NewEncoder(&buf).Encode(payload); err != nil { + t.Fatal(err) + } + } + req := httptest.NewRequest(method, path, &buf) + req.Header.Set("Authorization", "Bearer "+secret) + if payload != nil { + req.Header.Set("Content-Type", "application/json") + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + var out map[string]any + if rec.Body.Len() > 0 { + _ = json.Unmarshal(rec.Body.Bytes(), &out) + } + return rec.Code, out + } + create := func(payload map[string]any) string { + t.Helper() + code, out := send(http.MethodPost, "/api/v1/bm/materials", payload) + if code != http.StatusCreated { + t.Fatalf("POST %v = %d %v", payload, code, out) + } + return id(obj(out["data"])["id"]) + } + public := create(map[string]any{"title": "Globalizacja publiczna", "type": "notatka", "body": "Tekst o **globalizacji** dla wszystkich."}) + draft := create(map[string]any{"title": "Globalizacja szkic", "type": "notatka", "body": "szkic", "published_at": nil}) + private := create(map[string]any{"title": "Globalizacja prywatna", "type": "notatka", "body": "tajne", "is_private": true, "author_user_id": otherID}) + + var upserted []string + for _, doc := range stub.Imported() { + upserted = append(upserted, doc["id"].(string)) + } + for _, want := range []string{public, draft, private} { + if !slices.Contains(upserted, want) { + t.Errorf("material %s was not upserted (upserts %v)", want, upserted) + } + } + + extra := create(map[string]any{"title": "Do usunięcia", "type": "link"}) + if code, out := send(http.MethodDelete, "/api/v1/bm/materials/"+extra, nil); code != http.StatusNoContent { + t.Fatalf("DELETE = %d %v", code, out) + } + if !slices.Contains(stub.Removed(), extra) { + t.Errorf("removed = %v, want %s", stub.Removed(), extra) + } + + mark := func(word string) string { return searchtest.HighlightStart + word + searchtest.HighlightEnd } + stub.SetHits( + searchtest.Hit{ID: private, Field: "body", Snippet: mark("tajne")}, + searchtest.Hit{ID: draft, Field: "body", Snippet: mark("szkic")}, + searchtest.Hit{ID: public, Field: "body", Snippet: "Tekst o " + mark("globalizacji") + " dla wszystkich."}, + ) + student := apiClient{t: t, h: handler, token: tokenFor(t, studentID)} + req := httptest.NewRequest(http.MethodGet, "/_bm/api/v1/materials?q=globalizacja", nil) + req.Header.Set("Authorization", "Bearer "+student.token) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("search = %d %s", rec.Code, rec.Body.String()) + } + raw := rec.Body.String() + for _, hidden := range []string{"tajne", "szkic", "Globalizacja prywatna", "Globalizacja szkic"} { + if strings.Contains(raw, hidden) { + t.Errorf("search answer leaks %q: %s", hidden, raw) + } + } + var body map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatal(err) + } + rows := list(body) + if len(rows) != 1 || id(rows[0]["id"]) != public { + t.Fatalf("rows = %v, want only %s", rows, public) + } + if total := obj(body["meta"])["total"]; total != float64(1) { + t.Errorf("meta.total = %v, want 1 (the SQL count, not the engine's found)", total) + } + snippet := obj(rows[0]["snippet"]) + if snippet["source"] != "body" { + t.Errorf("snippet = %v, want the public body snippet", rows[0]["snippet"]) + } + + // Without q the list never asks the engine. + before := len(stub.Searches()) + plain := student.ok(http.MethodGet, "/_bm/api/v1/materials") + if len(stub.Searches()) != before { + t.Error("the list without q called the engine") + } + for _, row := range list(plain) { + if _, ok := row["snippet"]; ok { + t.Errorf("row without q carries a snippet: %v", row) + } + } +} diff --git a/plugin.go b/plugin.go index 5d22da3..2195615 100644 --- a/plugin.go +++ b/plugin.go @@ -3,6 +3,7 @@ package bm import ( "context" "embed" + "errors" "io/fs" "git.golem15.com/golem15/summercms/modules/backpack" @@ -32,6 +33,7 @@ var ( _ pact.HasPermissions = (*Plugin)(nil) _ pact.HasNavigation = (*Plugin)(nil) _ pact.HasMiddleware = (*Plugin)(nil) + _ pact.HasSchedule = (*Plugin)(nil) ) //go:embed config @@ -111,17 +113,29 @@ func (p *Plugin) AdminControllers() []pact.AdminController { return generatedAdminControllers() } -// Commands returns the generated commands, bm:seed, bm:chat-sync and -// websockets:health. API tokens are managed application-wide by -// golem15.user. +// Commands returns the generated commands, bm:seed, bm:chat-sync, +// bm:materials-reindex and websockets:health. API tokens are managed +// application-wide by golem15.user. func (p *Plugin) Commands() []bonfire.Command { cmds := append(generatedCommands(), console.SeedCommand(p.withDB)) cmds = append(cmds, console.ChatSyncCommand(p.withDB, func() (string, string) { return p.chatGroup(), p.chatModeratorGroup() })) + cmds = append(cmds, console.MaterialsReindexCommand(console.MaterialsReindexDeps{DB: p.publishedDB, Index: p.materialIndex})) return append(cmds, centrifugo.Commands(p.app)...) } +// Schedule runs bm:materials-reindex every night (it exits 0 when search +// is not configured). The index drifts without it: a course rename or a +// meeting renumbering changes documents no material write syncs, a failed +// sync is only logged, and a Typesense restart can lose data (RESEARCH +// Pitfall 6). +func (p *Plugin) Schedule() []pact.ScheduledCommand { + return []pact.ScheduledCommand{ + {Command: console.MaterialsReindexName, Args: []string{"--if-configured"}, Cadence: pact.DailyAt(4, 15)}, + } +} + // AdminFS is the admin YAML the controllers read. func (p *Plugin) AdminFS() fs.FS { return adminFS } @@ -172,6 +186,32 @@ func (p *Plugin) withDB(ctx context.Context, fn func(*gorm.DB) error) error { return fn(gdb) } +// publishedDB returns the application's database published on the app: the +// one serve published or, when a console command runs, one opened from the +// config and published, because beachcomber syncs nothing without a +// published database. done closes a pool this call opened and is a no-op +// otherwise. +func (p *Plugin) publishedDB(ctx context.Context) (*gorm.DB, func(), error) { + if p.app == nil { + return nil, nil, errors.New("jz.bm: app is nil") + } + if gdb, ok := p.app.Lookup[*gorm.DB](); ok && gdb != nil { + return gdb, func() {}, nil + } + if lagoon.DSN(p.app.Config) == "" { + return nil, nil, errors.New("jz.bm: database is not configured (set SUMMER_DATABASE__DSN)") + } + sqlDB, gdb, err := lagoon.OpenFromApp(ctx, p.app) + if err != nil { + return nil, nil, err + } + if err := lagoon.Publish(p.app, sqlDB, gdb); err != nil { + _ = sqlDB.Close() + return nil, nil, err + } + return gdb, func() { _ = sqlDB.Close() }, nil +} + func init() { party.Register(&Plugin{}) } diff --git a/plugin_test.go b/plugin_test.go index c8bb9aa..032c873 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -27,8 +27,8 @@ const ( ) // testConfig is the application config the tests boot with: the HTTP -// limits, the golem15.user JWT secret and the admin secret. -func testConfig(t *testing.T) *compass.Config { +// limits, the golem15.user JWT secret and the admin secret, plus env. +func testConfig(t *testing.T, env ...string) *compass.Config { t.Helper() dir := t.TempDir() httpYAML := "body_limits:\n default_bytes: 1048576\n upload_bytes: 67108864\n" @@ -37,11 +37,11 @@ func testConfig(t *testing.T) *compass.Config { } cfg, err := compass.Open(compass.Options{ Dir: dir, - Environ: []string{ + Environ: append([]string{ "SUMMER_ENV=testing", "SUMMER_GOLEM15__USER__JWT__SECRET=" + testUserSecret, "SUMMER_ADMIN__JWT__SECRET=" + testAdminSecret, - }, + }, env...), }) if err != nil { t.Fatal(err) @@ -85,6 +85,20 @@ func TestPluginActivates(t *testing.T) { if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:seed" }) { t.Error("bm:seed is not registered") } + if !slices.ContainsFunc(p.Commands(), func(c bonfire.Command) bool { return c.Name == "bm:materials-reindex" }) { + t.Error("bm:materials-reindex is not registered") + } + var scheduled bool + for _, e := range p.Schedule() { + if e.Command == "bm:materials-reindex" && slices.Equal(e.Args, []string{"--if-configured"}) { + if h, m, daily := e.Cadence.At(); daily && h == 4 && m == 15 { + scheduled = true + } + } + } + if !scheduled { + t.Errorf("Schedule = %+v, want bm:materials-reindex --if-configured daily at 04:15", p.Schedule()) + } if got := app.Config.Int("jz.bm.materials_per_page"); got != 20 { t.Errorf("jz.bm.materials_per_page = %d, want the default 20", got) } diff --git a/postgres_test.go b/postgres_test.go index 1d2e853..a902187 100644 --- a/postgres_test.go +++ b/postgres_test.go @@ -136,13 +136,19 @@ func testDatabase(t *testing.T) (*sql.DB, string) { // migrated activates golem15.user and jz.bm, migrates a fresh database and // publishes it on the application, as serve does. func migrated(t *testing.T) (*backpack.App, []party.Plugin, *gorm.DB) { + t.Helper() + return migratedWith(t) +} + +// migratedWith is migrated with env added to the application config. +func migratedWith(t *testing.T, env ...string) (*backpack.App, []party.Plugin, *gorm.DB) { t.Helper() sqlDB, _ := testDatabase(t) gdb, err := lagoon.Use(t.Context(), sqlDB) if err != nil { t.Fatalf("lagoon.Use: %v", err) } - app := backpack.New(testConfig(t)) + app := backpack.New(testConfig(t, env...)) plugins := activate(t, app) if err := lagoon.Migrate(gdb, plugins); err != nil { t.Fatalf("lagoon.Migrate: %v", err) diff --git a/search.go b/search.go index 8685585..4a9d566 100644 --- a/search.go +++ b/search.go @@ -5,13 +5,15 @@ import ( "git.golem15.com/golem15/summercms/modules/beachcomber" _ "git.golem15.com/golem15/summercms/modules/beachcomber/typesense" "git.golem15.com/jakub/sm-bm-plugin/classes" + "git.golem15.com/jakub/sm-bm-plugin/controllers" ) // wireSearch builds the app's beachcomber service (search.driver; the // typesense engine registers itself through the blank import), installs the -// jz.bm-only Gate and the material indexer. The Gate is app-wide, so it is -// on only for contexts jz.bm marks: golem15.chatchannels' message syncs stay -// off and chat messages never reach the index. +// jz.bm-only Gate and the material indexer, which the API handlers and the +// materials admin hooks use. The Gate is app-wide, so it is on only for +// contexts jz.bm marks: golem15.chatchannels' message syncs stay off and +// chat messages never reach the index. func (p *Plugin) wireSearch(app *backpack.App) error { svc, err := beachcomber.From(app) if err != nil { @@ -19,5 +21,6 @@ func (p *Plugin) wireSearch(app *backpack.App) error { } svc.SetGate(classes.MaterialSearchGate()) p.materialIndex = classes.NewMaterialIndexer(app) + controllers.UseMaterialIndex(p.materialIndex) return nil }