diff --git a/cmd/server/main.go b/cmd/server/main.go index 146e254..01c4ee5 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -50,7 +50,8 @@ func run(cfg config.Config, logger *slog.Logger) error { ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() - if _, err := openSecretBox(cfg.SecretKey); err != nil { + box, err := openSecretBox(cfg.SecretKey) + if err != nil { return err } @@ -77,6 +78,10 @@ func run(cfg config.Config, logger *slog.Logger) error { if err != nil { return err } + search, err := recipes.NewSearchSettingsService(db, box) + if err != nil { + return err + } sessions, err := auth.NewSessionManager(db, cfg.SessionIdle, cfg.SessionMax) if err != nil { @@ -94,7 +99,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, runner, photos, frontend.Assets()) + router, err := server.NewRouter(logger, version, cfg.TrustedProxy, authFlow, sessions, settings, search, runner, photos, frontend.Assets()) if err != nil { return err } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c74d6b6..fc277a5 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -92,7 +92,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | Table | Holds | | --- | --- | | `household_settings` | One row: timezone, dinner_time (17:30), reminder_time (16:30), review_time (09:00), plan_draft day/time (Fri 09:00), suggestions day/time (Sun 08:00), chat_autosend day/time (Sat 08:00), onboarding_completed_at, updates_seen_at. | -| `recipes` | A real recipe page: source_url (unique), site_name, title, total/prep/cook minutes, yield, raw_jsonld, fetched_at. | +| `recipes` | A real recipe page: source_url (unique), site_name, title, total/prep/cook minutes (the page's JSON-LD), yield, raw_jsonld, fetched_at; and its grounding: photo_id, method, difficulty, minutes_total (normalized prep to table). | | `recipe_ingredients` | recipe_id, position, raw_text, name, qty, unit, note. | | `recipe_steps` | recipe_id, position, text. | | `photos` | source_url, local_path, width, height, sha256, fetched_at. | @@ -105,7 +105,7 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`). | `shopping_lists` | week_start (unique), plan_id, title, status (`current` / `archived`), sent_to_chat_at, frozen_at. | | `shopping_groups` / `shopping_items` | One group per dinner/day; item name, qty, needed (bool), position. | | `ai_settings` | One row: provider, base_url, model, temperature, system_prompt, api_key_ct, api_key_last4, key_verified_at. | -| `recipe_search_settings` | One row: provider (`searxng` / `brave`), base_url, api_key_ct. | +| `recipe_search_settings` | One row: provider (`searxng` / `brave`), base_url (the SearXNG instance), api_key_ct and api_key_last4 (Brave). | | `integrations_google` | One row: refresh_token_ct, calendar_id, calendar_name, connected_at, last_sync_error. | | `chat_settings` | One row: webhook_url_ct, space_label, last_test_at, last_test_status. | | `notification_prefs` | event (`plan_ready` / `list_ready` / `day_of_reminder` / `new_suggestions` / `rotation_review`), push, chat. | @@ -145,6 +145,13 @@ automatic** (D-021). with a `photos` row, with `Cache-Control: private, max-age=31536000, immutable`; anything else is 404. +- `GET`/`PATCH /api/v1/settings/recipe-search` — provider, base_url and + the Brave key (`api_key`, write-only: responses carry only + `api_key_set` and `api_key_last4`; an empty string removes it). The + result must be usable: SearXNG needs a base_url, Brave a key. + `POST /api/v1/settings/recipe-search/test` searches "chicken tikka + masala" and returns `{query, count}`, 409 `not_configured`, or 502 + `search_failed` with the provider's error. - `/health` — liveness. - Everything else serves the embedded PWA with SPA fallback. @@ -246,7 +253,7 @@ automatic** (D-021). - Recipe search (`recipes.SearchProvider`: `Search(ctx, query, n)` → `[]Candidate{URL, Title, Snippet}`, n 1–20) appends " recipe" to a query without that word. SearXNG is called at - `{MEALQUEST_SEARXNG_URL}/search?q=…&format=json&categories=general` — + `{recipe_search_settings.base_url}/search?q=…&format=json&categories=general` — directly, not through the Fetcher, since this configured endpoint may be on the internal network; Brave calls the Brave Web Search API with the key decrypted from `recipe_search_settings.api_key_ct` @@ -280,13 +287,42 @@ automatic** (D-021). hard over 10 steps or 90 minutes, easy at 5 steps or fewer and 45 minutes or less, else medium, a crockpot's minutes not counting. - Photos are cached locally (`photos`) and processed with - `golang.org/x/image`. `recipes.PhotoCache.CachePhoto` downloads through + `golang.org/x/image`. `recipes.PhotoCache.DownloadPhoto` downloads through the Fetcher's `FetchImage` (JPEG, PNG, WebP or GIF content types, 8 MB - cap, not cached), decodes the image (a GIF's first frame), refuses a - decoded image under 300 px on its short side, scales it so its long side is 1200 px and - 400 px, and stores each as JPEG quality 82 at - `$MEALQUEST_DATA_DIR/media/photos/{id}-{size}.jpg`. Images are - deduplicated by the SHA-256 of the downloaded bytes. + cap, not cached), decodes the image (a GIF's first frame) and refuses a + decoded image under 300 px on its short side, recording nothing. + `CachePhoto` then records it in the caller's transaction: it scales the + image so its long side is 1200 px and 400 px, and stores each as JPEG + quality 82 at `$MEALQUEST_DATA_DIR/media/photos/{id}-{size}.jpg`. Images + are deduplicated by the SHA-256 of the downloaded bytes; the writer's + single connection serialises transactions, so one image is never + recorded twice. Files written for a transaction that rolls back are + removed by the caller (`RemovePhotoFiles`). +- Grounding (`recipes.Grounder`) ties these together. + `Ground(ctx, dishName, query)` searches for the top 5 candidates and ranks + them all, stored and new alike: a candidate whose `source_url` is already + stored is scored on its stored recipe (title, ingredients, steps, times, + photo source) and its page is never refetched; every other candidate is + fetched and extracted. Ranking alone decides between them. It then tries + each ranked candidate best first. A stored one is returned as it is, so a + stored recipe is reused only when it ranks above every new candidate that + grounds. A new one is grounded in this order: (1) `DownloadPhoto` — + the download and validation step of photo caching — (2) + `normalize_recipe`, then (3) one **transaction** that runs `CachePhoto` + (scales and writes the photo files and records the `photos` row) and + stores the recipe, ingredients and steps (an existing `source_url` is + updated in place); files written for a rolled-back transaction are + removed. It returns `Grounded{recipe_id, + photo_id, method, minutes_total, difficulty, source_url, site_name}`. A + candidate whose photo fails or whose model output is unusable + (`llm.ErrBadOutput`) is passed over; when none grounds, the error is a + typed `*recipes.ErrUngroundable` listing every reason (no candidates, + Rank's per-candidate reasons, photo or normalize failures). Failures + that are not the candidates' — search, model provider, database — are + returned as they are. `GroundURL(ctx, url)` does the same for one pasted + URL. `GroundMany` grounds items 3 at a time and returns one result per + item, so one failure never affects its siblings. A recipe is usable only + with ingredients, an image and, if given, a positive total time. ## Scheduler diff --git a/internal/db/models.go b/internal/db/models.go index a2f8121..7879c3e 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -181,6 +181,10 @@ type Recipe struct { FetchedAt string CreatedAt string UpdatedAt string + PhotoID sql.NullString + Method sql.NullString + Difficulty sql.NullString + MinutesTotal sql.NullInt64 } type RecipeIngredient struct { @@ -197,12 +201,13 @@ type RecipeIngredient struct { } type RecipeSearchSetting struct { - ID int64 - Provider string - BaseUrl sql.NullString - ApiKeyCt []byte - CreatedAt string - UpdatedAt string + ID int64 + Provider string + BaseUrl sql.NullString + ApiKeyCt []byte + CreatedAt string + UpdatedAt string + ApiKeyLast4 sql.NullString } type RecipeStep struct { diff --git a/internal/db/recipes.sql.go b/internal/db/recipes.sql.go index 6255443..2098ef8 100644 --- a/internal/db/recipes.sql.go +++ b/internal/db/recipes.sql.go @@ -99,7 +99,7 @@ func (q *Queries) DeleteRecipeSteps(ctx context.Context, recipeID string) error } const getRecipe = `-- name: GetRecipe :one -SELECT id, source_url, site_name, title, total_minutes, prep_minutes, cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at FROM recipes +SELECT id, source_url, site_name, title, total_minutes, prep_minutes, cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at, photo_id, method, difficulty, minutes_total FROM recipes WHERE id = ? ` @@ -119,12 +119,16 @@ func (q *Queries) GetRecipe(ctx context.Context, id string) (Recipe, error) { &i.FetchedAt, &i.CreatedAt, &i.UpdatedAt, + &i.PhotoID, + &i.Method, + &i.Difficulty, + &i.MinutesTotal, ) return i, err } const getRecipeBySourceURL = `-- name: GetRecipeBySourceURL :one -SELECT id, source_url, site_name, title, total_minutes, prep_minutes, cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at FROM recipes +SELECT id, source_url, site_name, title, total_minutes, prep_minutes, cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at, photo_id, method, difficulty, minutes_total FROM recipes WHERE source_url = ? ` @@ -144,6 +148,10 @@ func (q *Queries) GetRecipeBySourceURL(ctx context.Context, sourceUrl string) (R &i.FetchedAt, &i.CreatedAt, &i.UpdatedAt, + &i.PhotoID, + &i.Method, + &i.Difficulty, + &i.MinutesTotal, ) return i, err } @@ -227,10 +235,12 @@ func (q *Queries) ListRecipeSteps(ctx context.Context, recipeID string) ([]Recip const upsertRecipe = `-- name: UpsertRecipe :one INSERT INTO recipes ( id, source_url, site_name, title, total_minutes, prep_minutes, - cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at + cook_minutes, yield, raw_jsonld, photo_id, method, difficulty, + minutes_total, fetched_at, created_at, updated_at ) VALUES ( ?, ?, ?, ?, ?, ?, - ?, ?, ?, ?, ?, ? + ?, ?, ?, ?, ?, ?, + ?, ?, ?, ? ) ON CONFLICT (source_url) DO UPDATE SET site_name = excluded.site_name, @@ -240,9 +250,13 @@ ON CONFLICT (source_url) DO UPDATE SET cook_minutes = excluded.cook_minutes, yield = excluded.yield, raw_jsonld = excluded.raw_jsonld, + photo_id = excluded.photo_id, + method = excluded.method, + difficulty = excluded.difficulty, + minutes_total = excluded.minutes_total, fetched_at = excluded.fetched_at, updated_at = excluded.updated_at -RETURNING id, source_url, site_name, title, total_minutes, prep_minutes, cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at +RETURNING id, source_url, site_name, title, total_minutes, prep_minutes, cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at, photo_id, method, difficulty, minutes_total ` type UpsertRecipeParams struct { @@ -255,6 +269,10 @@ type UpsertRecipeParams struct { CookMinutes sql.NullInt64 Yield sql.NullString RawJsonld string + PhotoID sql.NullString + Method sql.NullString + Difficulty sql.NullString + MinutesTotal sql.NullInt64 FetchedAt string CreatedAt string UpdatedAt string @@ -275,6 +293,10 @@ func (q *Queries) UpsertRecipe(ctx context.Context, arg UpsertRecipeParams) (Rec arg.CookMinutes, arg.Yield, arg.RawJsonld, + arg.PhotoID, + arg.Method, + arg.Difficulty, + arg.MinutesTotal, arg.FetchedAt, arg.CreatedAt, arg.UpdatedAt, @@ -293,6 +315,10 @@ func (q *Queries) UpsertRecipe(ctx context.Context, arg UpsertRecipeParams) (Rec &i.FetchedAt, &i.CreatedAt, &i.UpdatedAt, + &i.PhotoID, + &i.Method, + &i.Difficulty, + &i.MinutesTotal, ) return i, err } diff --git a/internal/db/settings.sql.go b/internal/db/settings.sql.go index 10c89fd..50a036e 100644 --- a/internal/db/settings.sql.go +++ b/internal/db/settings.sql.go @@ -109,7 +109,7 @@ func (q *Queries) GetIntegrationsGoogle(ctx context.Context) (IntegrationsGoogle } const getRecipeSearchSettings = `-- name: GetRecipeSearchSettings :one -SELECT id, provider, base_url, api_key_ct, created_at, updated_at FROM recipe_search_settings +SELECT id, provider, base_url, api_key_ct, created_at, updated_at, api_key_last4 FROM recipe_search_settings WHERE id = 1 ` @@ -124,6 +124,7 @@ func (q *Queries) GetRecipeSearchSettings(ctx context.Context) (RecipeSearchSett &i.ApiKeyCt, &i.CreatedAt, &i.UpdatedAt, + &i.ApiKeyLast4, ) return i, err } @@ -390,24 +391,26 @@ func (q *Queries) UpsertIntegrationsGoogle(ctx context.Context, arg UpsertIntegr const upsertRecipeSearchSettings = `-- name: UpsertRecipeSearchSettings :one INSERT INTO recipe_search_settings ( - id, provider, base_url, api_key_ct, created_at, updated_at + id, provider, base_url, api_key_ct, api_key_last4, created_at, updated_at ) VALUES ( - 1, ?, ?, ?, ?, ? + 1, ?, ?, ?, ?, ?, ? ) ON CONFLICT (id) DO UPDATE SET provider = excluded.provider, base_url = excluded.base_url, api_key_ct = excluded.api_key_ct, + api_key_last4 = excluded.api_key_last4, updated_at = excluded.updated_at -RETURNING id, provider, base_url, api_key_ct, created_at, updated_at +RETURNING id, provider, base_url, api_key_ct, created_at, updated_at, api_key_last4 ` type UpsertRecipeSearchSettingsParams struct { - Provider string - BaseUrl sql.NullString - ApiKeyCt []byte - CreatedAt string - UpdatedAt string + Provider string + BaseUrl sql.NullString + ApiKeyCt []byte + ApiKeyLast4 sql.NullString + CreatedAt string + UpdatedAt string } // Creates the single row (id 1) or replaces every column except id and @@ -417,6 +420,7 @@ func (q *Queries) UpsertRecipeSearchSettings(ctx context.Context, arg UpsertReci arg.Provider, arg.BaseUrl, arg.ApiKeyCt, + arg.ApiKeyLast4, arg.CreatedAt, arg.UpdatedAt, ) @@ -428,6 +432,7 @@ func (q *Queries) UpsertRecipeSearchSettings(ctx context.Context, arg UpsertReci &i.ApiKeyCt, &i.CreatedAt, &i.UpdatedAt, + &i.ApiKeyLast4, ) return i, err } diff --git a/internal/recipes/ground.go b/internal/recipes/ground.go new file mode 100644 index 0000000..bb52b71 --- /dev/null +++ b/internal/recipes/ground.go @@ -0,0 +1,471 @@ +package recipes + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + "strings" + "sync" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// GroundConcurrency is how many items GroundMany grounds at once. +const GroundConcurrency = 3 + +var ( + // ErrNoCandidates is a reason a dish is ungroundable: the search found + // no pages at all. + ErrNoCandidates = errors.New("recipes: search found no candidates") + // ErrTotalTimeNotPositive rejects a recipe whose page gives a total + // time of zero minutes, which normalize_recipe refuses. + ErrTotalTimeNotPositive = errors.New("recipes: recipe total time is not positive") +) + +// ErrUngroundable is returned when no recipe page could ground a dish or a +// pasted URL. Reasons says why each page tried was rejected, or why there +// were none; errors.Is matches any of them. +type ErrUngroundable struct { + // Target is the dish name or the URL. + Target string + Reasons []error +} + +func (e *ErrUngroundable) Error() string { + parts := make([]string, len(e.Reasons)) + for i, r := range e.Reasons { + parts[i] = r.Error() + } + return fmt.Sprintf("recipes: cannot ground %q: %s", e.Target, strings.Join(parts, "; ")) +} + +func (e *ErrUngroundable) Unwrap() []error { return e.Reasons } + +// Grounded is a recipe stored for a dish: the recipe and its cached photo, +// with normalize_recipe's method, minutes and difficulty. +type Grounded struct { + RecipeID string `json:"recipe_id"` + PhotoID string `json:"photo_id"` + // Method is stovetop, oven, crockpot, grill or other. + Method string `json:"method"` + // MinutesTotal is prep to table, nil when unknown. + MinutesTotal *int `json:"minutes_total"` + // Difficulty is easy, medium or hard. + Difficulty string `json:"difficulty"` + SourceURL string `json:"source_url"` + SiteName string `json:"site_name"` +} + +// SearchSource returns the household's current search provider; +// *SearchSettingsService satisfies it. +type SearchSource interface { + Provider(ctx context.Context) (SearchProvider, error) +} + +// Normalizer runs normalize_recipe on a recipe; scope is its source URL. In +// production it is skills.NormalizeRecipe with the household's +// harness.Config. +type Normalizer func(ctx context.Context, scope string, in skills.RecipeText) (skills.NormalizedRecipe, error) + +// Grounder grounds dishes in recipe pages: it searches, fetches, extracts +// and ranks candidates, caches the best one's photo, normalizes its +// ingredients and stores it all. It is safe for concurrent use. +type Grounder struct { + st *store.Store + search SearchSource + pages PageFetcher + photos *PhotoCache + normalize Normalizer +} + +// NewGrounder returns a Grounder storing recipes in st, searching with +// search, fetching pages with pages, caching photos with photos and +// normalizing with normalize. +func NewGrounder(st *store.Store, search SearchSource, pages PageFetcher, photos *PhotoCache, normalize Normalizer) (*Grounder, error) { + switch { + case st == nil: + return nil, errors.New("recipes: nil store") + case search == nil: + return nil, errors.New("recipes: nil search source") + case pages == nil: + return nil, errors.New("recipes: nil page fetcher") + case photos == nil: + return nil, errors.New("recipes: nil photo cache") + case normalize == nil: + return nil, errors.New("recipes: nil normalizer") + } + return &Grounder{st: st, search: search, pages: pages, photos: photos, normalize: normalize}, nil +} + +// Ground searches for query and grounds dishName in the best recipe found. +// The candidates are ranked as Rank does, except that a candidate already +// stored is scored on its stored recipe and never fetched. They are tried +// best first: a stored candidate is returned as it is, and otherwise the +// first whose photo can be cached and whose ingredients normalize is stored +// — the recipe, its ingredients, steps and photo in one transaction — and +// returned. A stored recipe is reused only when it ranks above every +// candidate that grounds. +// +// When no candidate grounds the dish the error is an *ErrUngroundable with +// each reason. A failure that is not the candidates' — the search, the +// model provider, the database — is returned as it is. +func (g *Grounder) Ground(ctx context.Context, dishName, query string) (Grounded, error) { + if strings.TrimSpace(dishName) == "" { + return Grounded{}, errors.New("recipes: ground: dish name is empty") + } + provider, err := g.search.Provider(ctx) + if err != nil { + return Grounded{}, fmt.Errorf("recipes: ground %q: %w", dishName, err) + } + cands, err := provider.Search(ctx, query, RankDepth) + if err != nil { + return Grounded{}, fmt.Errorf("recipes: ground %q: search: %w", dishName, err) + } + if len(cands) == 0 { + return Grounded{}, &ErrUngroundable{Target: dishName, Reasons: []error{ErrNoCandidates}} + } + cands = cands[:min(len(cands), RankDepth)] + stored := make(map[string]storedRecipe) + for _, c := range cands { + sr, ok, err := g.storedRecipe(ctx, c.URL) + if err != nil { + return Grounded{}, fmt.Errorf("recipes: ground %q: %w", dishName, err) + } + if ok { + stored[c.URL] = sr + } + } + ranked, rankErr := rank(ctx, dishName, cands, func(ctx context.Context, c Candidate) (Recipe, error) { + if sr, ok := stored[c.URL]; ok { + return sr.recipe, nil + } + return fetchRecipe(ctx, g.pages, c.URL) + }) + if err := ctx.Err(); err != nil { + return Grounded{}, err + } + var reasons []error + if rankErr != nil { + reasons = append(reasons, rankErr) + } + for _, r := range ranked { + if sr, ok := stored[r.Candidate.URL]; ok { + return sr.grounded, nil + } + gr, reason, err := g.ground(ctx, r.Candidate.URL, r.Recipe) + if err != nil { + return Grounded{}, fmt.Errorf("recipes: ground %q: %w", dishName, err) + } + if reason == nil { + return gr, nil + } + reasons = append(reasons, fmt.Errorf("%s: %w", r.Candidate.URL, reason)) + } + return Grounded{}, &ErrUngroundable{Target: dishName, Reasons: reasons} +} + +// GroundURL grounds the recipe on the page at rawURL, a pasted URL, as +// Ground does a candidate. An already stored rawURL is returned without +// fetching it. A page that cannot be fetched, holds no usable recipe, or +// whose photo or ingredients fail is an *ErrUngroundable. +func (g *Grounder) GroundURL(ctx context.Context, rawURL string) (Grounded, error) { + if gr, ok, err := g.stored(ctx, rawURL); err != nil || ok { + return gr, err + } + ungroundable := func(err error) (Grounded, error) { + if ctxErr := ctx.Err(); ctxErr != nil { + return Grounded{}, ctxErr + } + return Grounded{}, &ErrUngroundable{Target: rawURL, Reasons: []error{err}} + } + page, err := g.pages.Fetch(ctx, rawURL) + if err != nil { + return ungroundable(err) + } + r, err := Extract(page.Body, page.URL) + if err != nil { + return ungroundable(err) + } + if err := usable(r); err != nil { + return ungroundable(err) + } + gr, reason, err := g.ground(ctx, rawURL, r) + if err != nil { + return Grounded{}, fmt.Errorf("recipes: ground %s: %w", rawURL, err) + } + if reason != nil { + return ungroundable(reason) + } + return gr, nil +} + +// GroundRequest is one dish for GroundMany. +type GroundRequest struct { + DishName string + Query string +} + +// GroundResult is one GroundMany outcome: Grounded, or Err when the item +// failed. +type GroundResult struct { + Grounded Grounded + Err error +} + +// GroundMany grounds each request with Ground, GroundConcurrency at a time, +// and returns their results in request order. One item's failure does not +// affect the others. +func (g *Grounder) GroundMany(ctx context.Context, reqs []GroundRequest) []GroundResult { + results := make([]GroundResult, len(reqs)) + slots := make(chan struct{}, GroundConcurrency) + var wg sync.WaitGroup + for i, req := range reqs { + wg.Add(1) + go func() { + defer wg.Done() + select { + case slots <- struct{}{}: + case <-ctx.Done(): + results[i] = GroundResult{Err: ctx.Err()} + return + } + defer func() { <-slots }() + gr, err := g.Ground(ctx, req.DishName, req.Query) + results[i] = GroundResult{Grounded: gr, Err: err} + }() + } + wg.Wait() + return results +} + +// stored returns the grounding of the recipe stored for sourceURL, and +// false when there is none. +func (g *Grounder) stored(ctx context.Context, sourceURL string) (Grounded, bool, error) { + row, ok, err := g.storedRow(ctx, sourceURL) + if err != nil || !ok { + return Grounded{}, false, err + } + return groundedFrom(row), true, nil +} + +// storedRow returns the recipe row stored for sourceURL, and false when +// there is none. +func (g *Grounder) storedRow(ctx context.Context, sourceURL string) (db.Recipe, bool, error) { + row, err := db.New(g.st.Reader).GetRecipeBySourceURL(ctx, sourceURL) + if errors.Is(err, sql.ErrNoRows) { + return db.Recipe{}, false, nil + } + if err != nil { + return db.Recipe{}, false, fmt.Errorf("recipes: look up recipe %s: %w", sourceURL, err) + } + if !row.PhotoID.Valid || !row.Method.Valid || !row.Difficulty.Valid { + return db.Recipe{}, false, fmt.Errorf("recipes: stored recipe %s for %s has no photo, method or difficulty", row.ID, sourceURL) + } + return row, true, nil +} + +// storedRecipe is a recipe already stored: its grounding, and the recipe +// rebuilt from what was stored so it can be ranked without its page. +type storedRecipe struct { + grounded Grounded + recipe Recipe +} + +// storedRecipe returns the recipe stored for sourceURL with its photo, +// ingredients and steps, and false when there is none. +func (g *Grounder) storedRecipe(ctx context.Context, sourceURL string) (storedRecipe, bool, error) { + row, ok, err := g.storedRow(ctx, sourceURL) + if err != nil || !ok { + return storedRecipe{}, false, err + } + gr := groundedFrom(row) + q := db.New(g.st.Reader) + photo, err := q.GetPhoto(ctx, gr.PhotoID) + if err != nil { + return storedRecipe{}, false, fmt.Errorf("recipes: load photo %s of recipe %s: %w", gr.PhotoID, gr.RecipeID, err) + } + ings, err := q.ListRecipeIngredients(ctx, gr.RecipeID) + if err != nil { + return storedRecipe{}, false, fmt.Errorf("recipes: load ingredients of recipe %s: %w", gr.RecipeID, err) + } + steps, err := q.ListRecipeSteps(ctx, gr.RecipeID) + if err != nil { + return storedRecipe{}, false, fmt.Errorf("recipes: load steps of recipe %s: %w", gr.RecipeID, err) + } + r := Recipe{ + Title: row.Title, + SiteName: row.SiteName.String, + ImageURL: photo.SourceUrl, + PrepMinutes: intFrom(row.PrepMinutes), + CookMinutes: intFrom(row.CookMinutes), + TotalMinutes: intFrom(row.TotalMinutes), + Yield: row.Yield.String, + JSONLD: json.RawMessage(row.RawJsonld), + } + for _, ing := range ings { + r.Ingredients = append(r.Ingredients, ing.RawText) + } + for _, st := range steps { + r.Steps = append(r.Steps, st.Text) + } + return storedRecipe{grounded: gr, recipe: r}, true, nil +} + +// ground downloads r's photo, normalizes it and stores it as sourceURL's +// recipe. A problem with this recipe is returned as reason, so the caller +// can try another; err is a failure that is not the recipe's. +// +// The order is fixed: the photo is downloaded and validated first, so a +// recipe without a usable photo never costs a normalize_recipe call; then +// normalize_recipe runs; then save records the photo (CachePhoto) and the +// recipe in one transaction. +func (g *Grounder) ground(ctx context.Context, sourceURL string, r Recipe) (gr Grounded, reason, err error) { + photo, err := g.photos.DownloadPhoto(ctx, r.ImageURL) + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return Grounded{}, nil, ctxErr + } + return Grounded{}, fmt.Errorf("photo: %w", err), nil + } + norm, err := g.normalize(ctx, sourceURL, skills.RecipeText{ + Title: r.Title, + Ingredients: r.Ingredients, + Steps: r.Steps, + PrepMinutes: r.PrepMinutes, + CookMinutes: r.CookMinutes, + TotalMinutes: r.TotalMinutes, + }) + if errors.Is(err, llm.ErrBadOutput) { + return Grounded{}, fmt.Errorf("normalize: %w", err), nil + } + if err != nil { + return Grounded{}, nil, fmt.Errorf("normalize: %w", err) + } + if len(norm.Ingredients) != len(r.Ingredients) { + return Grounded{}, nil, fmt.Errorf("normalize: %d ingredients for %d lines", len(norm.Ingredients), len(r.Ingredients)) + } + gr, err = g.save(ctx, sourceURL, r, photo, norm) + return gr, nil, err +} + +// save stores the recipe, its ingredients and steps and its photo in one +// transaction. A photo's files written for a transaction that does not +// commit are removed. +func (g *Grounder) save(ctx context.Context, sourceURL string, r Recipe, p DownloadedPhoto, norm skills.NormalizedRecipe) (Grounded, error) { + var ( + row db.Recipe + photo db.Photo + created bool + ) + err := g.st.WithTx(ctx, func(tx *sql.Tx) error { + var err error + photo, created, err = g.photos.CachePhoto(ctx, tx, p) + if err != nil { + return err + } + q := db.New(tx) + now := store.Now().String() + row, err = q.UpsertRecipe(ctx, db.UpsertRecipeParams{ + ID: store.NewID(), + SourceUrl: sourceURL, + SiteName: sql.NullString{String: r.SiteName, Valid: r.SiteName != ""}, + Title: r.Title, + TotalMinutes: nullInt(r.TotalMinutes), + PrepMinutes: nullInt(r.PrepMinutes), + CookMinutes: nullInt(r.CookMinutes), + Yield: sql.NullString{String: r.Yield, Valid: r.Yield != ""}, + RawJsonld: string(r.JSONLD), + PhotoID: sql.NullString{String: photo.ID, Valid: true}, + Method: sql.NullString{String: norm.Method, Valid: true}, + Difficulty: sql.NullString{String: norm.Difficulty, Valid: true}, + MinutesTotal: nullInt(norm.MinutesTotal), + FetchedAt: now, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return fmt.Errorf("recipes: store recipe: %w", err) + } + if err := q.DeleteRecipeIngredients(ctx, row.ID); err != nil { + return fmt.Errorf("recipes: replace ingredients: %w", err) + } + if err := q.DeleteRecipeSteps(ctx, row.ID); err != nil { + return fmt.Errorf("recipes: replace steps: %w", err) + } + for i, line := range r.Ingredients { + ing := norm.Ingredients[i] + err := q.CreateRecipeIngredient(ctx, db.CreateRecipeIngredientParams{ + ID: store.NewID(), + RecipeID: row.ID, + Position: int64(i), + RawText: line, + Name: sql.NullString{String: ing.Name, Valid: true}, + Qty: sql.NullString{String: ing.Qty, Valid: ing.Qty != ""}, + Unit: sql.NullString{String: ing.Unit, Valid: ing.Unit != ""}, + Note: sql.NullString{String: ing.Note, Valid: ing.Note != ""}, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return fmt.Errorf("recipes: store ingredient %d: %w", i+1, err) + } + } + for i, text := range r.Steps { + err := q.CreateRecipeStep(ctx, db.CreateRecipeStepParams{ + ID: store.NewID(), + RecipeID: row.ID, + Position: int64(i), + Text: text, + CreatedAt: now, + UpdatedAt: now, + }) + if err != nil { + return fmt.Errorf("recipes: store step %d: %w", i+1, err) + } + } + return nil + }) + if err != nil { + if created { + err = errors.Join(err, g.photos.RemovePhotoFiles(photo.ID)) + } + return Grounded{}, err + } + return groundedFrom(row), nil +} + +func groundedFrom(row db.Recipe) Grounded { + gr := Grounded{ + RecipeID: row.ID, + PhotoID: row.PhotoID.String, + Method: row.Method.String, + Difficulty: row.Difficulty.String, + SourceURL: row.SourceUrl, + SiteName: row.SiteName.String, + } + if row.MinutesTotal.Valid { + m := int(row.MinutesTotal.Int64) + gr.MinutesTotal = &m + } + return gr +} + +func intFrom(v sql.NullInt64) *int { + if !v.Valid { + return nil + } + n := int(v.Int64) + return &n +} + +func nullInt(v *int) sql.NullInt64 { + if v == nil { + return sql.NullInt64{} + } + return sql.NullInt64{Int64: int64(*v), Valid: true} +} diff --git a/internal/recipes/ground_test.go b/internal/recipes/ground_test.go new file mode 100644 index 0000000..8bde81a --- /dev/null +++ b/internal/recipes/ground_test.go @@ -0,0 +1,518 @@ +package recipes + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/llm" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/skills" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +// fakeSearch is a SearchSource and SearchProvider answering each query +// from results, and failing a query in errs. With gate set, every Search +// reports its query on started and waits for gate to close. +type fakeSearch struct { + results map[string][]Candidate + errs map[string]error + started chan string + gate chan struct{} + + mu sync.Mutex + calls int + active int + maxActive int +} + +func (s *fakeSearch) Provider(context.Context) (SearchProvider, error) { return s, nil } + +func (s *fakeSearch) Search(_ context.Context, query string, n int) ([]Candidate, error) { + s.mu.Lock() + s.calls++ + s.active++ + s.maxActive = max(s.maxActive, s.active) + s.mu.Unlock() + defer func() { + s.mu.Lock() + s.active-- + s.mu.Unlock() + }() + if s.gate != nil { + s.started <- query + <-s.gate + } + if err := s.errs[query]; err != nil { + return nil, err + } + c := s.results[query] + return c[:min(n, len(c))], nil +} + +func (s *fakeSearch) callCount() int { + s.mu.Lock() + defer s.mu.Unlock() + return s.calls +} + +func (f *fakeFetcher) fetchCount() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.fetched) +} + +// groundPage is a recipe page for name with two ingredients, two steps, a +// 40-minute total and the image at https://example.com{imagePath}. +func groundPage(name, imagePath string) []byte { + return page(``, fmt.Sprintf(`{ + "@context": "https://schema.org", + "@type": "Recipe", + "name": %q, + "image": "https://example.com%s", + "recipeIngredient": ["1 onion", "2 cloves garlic, minced"], + "recipeInstructions": ["Chop the onion.", "Fry with the garlic."], + "totalTime": "PT40M", + "recipeYield": "4" + }`, name, imagePath)) +} + +// normalizedReply is a usable normalize_recipe reply for groundPage. +func normalizedReply() llm.GenerateResult { + js := `{"ingredients":[ + {"name":"onion","qty":"1","unit":"","note":""}, + {"name":"garlic","qty":"2","unit":"cloves","note":"minced"} + ],"method":"stovetop","difficulty":null,"minutes_total":40}` + return llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}} +} + +// badReply is a normalize_recipe reply that fails validation. +func badReply() llm.GenerateResult { + js := `{"ingredients":[],"method":"stovetop","difficulty":null,"minutes_total":40}` + return llm.GenerateResult{Response: llm.Response{Text: js, JSON: json.RawMessage(js)}} +} + +type groundEnv struct { + g *Grounder + st *store.Store + dataDir string + images *photoServer + pages *fakeFetcher + search *fakeSearch + llm *llm.Fake +} + +// newGroundEnv returns a Grounder over pages, images served at +// https://example.com, search and a fake model, normalizing through the +// real harness and normalize_recipe skill. +func newGroundEnv(t *testing.T, pages map[string][]byte, images map[string]photoFile, search *fakeSearch) *groundEnv { + t.Helper() + photos, st, dataDir, imgSrv := newTestPhotoCache(t, images) + model := &llm.Fake{} + guard, err := harness.NewGuard(harness.DefaultMaxCalls) + if err != nil { + t.Fatal(err) + } + cfg := harness.Config{ + Settings: llm.Settings{Provider: "anthropic", Model: "claude-test"}, + NewProvider: func(llm.Settings, string) (llm.Provider, error) { return model, nil }, + Store: st, + Guard: guard, + } + normalize := func(ctx context.Context, scope string, in skills.RecipeText) (skills.NormalizedRecipe, error) { + return skills.NormalizeRecipe(ctx, cfg, scope, in) + } + fetcher := &fakeFetcher{pages: pages} + g, err := NewGrounder(st, search, fetcher, photos, normalize) + if err != nil { + t.Fatal(err) + } + return &groundEnv{g: g, st: st, dataDir: dataDir, images: imgSrv, pages: fetcher, search: search, llm: model} +} + +func jpegFile(t *testing.T, w, h int) photoFile { + return photoFile{"image/jpeg", encodeJPEG(t, gradient(w, h))} +} + +func countRows(t *testing.T, st *store.Store, table string) int { + t.Helper() + var n int + if err := st.Reader.QueryRowContext(context.Background(), "SELECT count(*) FROM "+table).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func TestGroundHappyPath(t *testing.T) { + t.Parallel() + search := &fakeSearch{results: map[string][]Candidate{ + "chicken curry": { + {URL: "https://a.example/missing"}, + {URL: "https://b.example/curry"}, + }, + }} + env := newGroundEnv(t, + map[string][]byte{"https://b.example/curry": groundPage("Chicken Curry", "/img/curry.jpg")}, + map[string]photoFile{"/img/curry.jpg": jpegFile(t, 800, 600)}, + search) + env.llm.QueueGenerate(normalizedReply()) + + gr, err := env.g.Ground(t.Context(), "Chicken Curry", "chicken curry") + if err != nil { + t.Fatalf("Ground: %v", err) + } + if gr.RecipeID == "" || gr.PhotoID == "" { + t.Fatalf("Grounded = %+v, want recipe and photo ids", gr) + } + want := Grounded{ + RecipeID: gr.RecipeID, PhotoID: gr.PhotoID, Method: "stovetop", MinutesTotal: gr.MinutesTotal, + Difficulty: "easy", SourceURL: "https://b.example/curry", SiteName: "Example Kitchen", + } + if gr != want || gr.MinutesTotal == nil || *gr.MinutesTotal != 40 { + t.Errorf("Grounded = %+v (minutes %v), want %+v with 40 minutes", gr, gr.MinutesTotal, want) + } + + q := db.New(env.st.Reader) + rec, err := q.GetRecipe(t.Context(), gr.RecipeID) + if err != nil { + t.Fatal(err) + } + if rec.Title != "Chicken Curry" || rec.PhotoID.String != gr.PhotoID || rec.Method.String != "stovetop" || + rec.Difficulty.String != "easy" || rec.MinutesTotal.Int64 != 40 || rec.TotalMinutes.Int64 != 40 || rec.Yield.String != "4" { + t.Errorf("recipe row = %+v", rec) + } + ings, err := q.ListRecipeIngredients(t.Context(), gr.RecipeID) + if err != nil { + t.Fatal(err) + } + if len(ings) != 2 || ings[1].RawText != "2 cloves garlic, minced" || ings[1].Name.String != "garlic" || + ings[1].Qty.String != "2" || ings[1].Unit.String != "cloves" || ings[1].Note.String != "minced" || ings[0].Unit.Valid { + t.Errorf("ingredients = %+v", ings) + } + steps, err := q.ListRecipeSteps(t.Context(), gr.RecipeID) + if err != nil { + t.Fatal(err) + } + if len(steps) != 2 || steps[0].Text != "Chop the onion." || steps[1].Position != 1 { + t.Errorf("steps = %+v", steps) + } + photo, err := q.GetPhoto(t.Context(), gr.PhotoID) + if err != nil { + t.Fatal(err) + } + if photo.SourceUrl != "https://example.com/img/curry.jpg" { + t.Errorf("photo row = %+v", photo) + } + if _, err := os.Stat(filepath.Join(env.dataDir, filepath.FromSlash(photo.LocalPath))); err != nil { + t.Errorf("photo file: %v", err) + } +} + +func TestGroundUngroundable(t *testing.T) { + t.Parallel() + search := &fakeSearch{results: map[string][]Candidate{ + "mystery stew": { + {URL: "https://a.example/missing"}, + {URL: "https://b.example/no-recipe"}, + {URL: "https://c.example/tiny-photo"}, + }, + }} + env := newGroundEnv(t, + map[string][]byte{ + "https://b.example/no-recipe": page(""), + "https://c.example/tiny-photo": groundPage("Mystery Stew", "/img/tiny.jpg"), + }, + map[string]photoFile{"/img/tiny.jpg": jpegFile(t, 200, 200)}, + search) + + _, err := env.g.Ground(t.Context(), "Mystery Stew", "mystery stew") + var ungroundable *ErrUngroundable + if !errors.As(err, &ungroundable) { + t.Fatalf("Ground error = %v, want *ErrUngroundable", err) + } + if ungroundable.Target != "Mystery Stew" { + t.Errorf("Target = %q", ungroundable.Target) + } + for _, reason := range []error{ErrCandidatesUnranked, ErrNoRecipe, ErrPhotoTooSmall} { + if !errors.Is(err, reason) { + t.Errorf("error %v does not report %v", err, reason) + } + } + if n := len(env.llm.Requests()); n != 0 { + t.Errorf("%d model calls, want none before a photo is cached", n) + } + for _, table := range []string{"recipes", "recipe_ingredients", "photos"} { + if n := countRows(t, env.st, table); n != 0 { + t.Errorf("%d %s rows for an ungroundable dish", n, table) + } + } + if names := photoFiles(t, env.dataDir); len(names) != 0 { + t.Errorf("photo files stored for an ungroundable dish: %v", names) + } + + _, err = env.g.Ground(t.Context(), "Nothing At All", "nothing at all") + if !errors.As(err, &ungroundable) || !errors.Is(err, ErrNoCandidates) { + t.Errorf("Ground with no search results error = %v, want *ErrUngroundable with ErrNoCandidates", err) + } +} + +func TestGroundTriesNextCandidateAfterBadOutput(t *testing.T) { + t.Parallel() + search := &fakeSearch{results: map[string][]Candidate{ + "curry": {{URL: "https://a.example/curry"}, {URL: "https://b.example/curry"}}, + }} + env := newGroundEnv(t, + map[string][]byte{ + "https://a.example/curry": groundPage("Curry", "/img/a.jpg"), + "https://b.example/curry": groundPage("Curry", "/img/b.jpg"), + }, + map[string]photoFile{"/img/a.jpg": jpegFile(t, 640, 480), "/img/b.jpg": jpegFile(t, 480, 640)}, + search) + // The first candidate's reply and repair are unusable; the second's is + // fine. + env.llm.QueueGenerate(badReply(), badReply(), normalizedReply()) + + gr, err := env.g.Ground(t.Context(), "Curry", "curry") + if err != nil { + t.Fatalf("Ground: %v", err) + } + if gr.SourceURL != "https://b.example/curry" { + t.Errorf("grounded in %s, want the second candidate", gr.SourceURL) + } + if n := countRows(t, env.st, "photos"); n != 1 { + t.Errorf("%d photos rows, want only the grounded recipe's", n) + } +} + +func TestGroundSearchFailureIsNotUngroundable(t *testing.T) { + t.Parallel() + env := newGroundEnv(t, nil, nil, &fakeSearch{errs: map[string]error{"curry": ErrSearchRateLimited}}) + _, err := env.g.Ground(t.Context(), "Curry", "curry") + var ungroundable *ErrUngroundable + if !errors.Is(err, ErrSearchRateLimited) || errors.As(err, &ungroundable) { + t.Errorf("Ground error = %v, want ErrSearchRateLimited and not *ErrUngroundable", err) + } +} + +func TestGroundReusesStoredRecipe(t *testing.T) { + t.Parallel() + search := &fakeSearch{results: map[string][]Candidate{ + "curry": {{URL: "https://b.example/curry"}}, + "chicken curry": {{URL: "https://a.example/other"}, {URL: "https://b.example/curry"}}, + }} + env := newGroundEnv(t, + map[string][]byte{"https://b.example/curry": groundPage("Curry", "/img/curry.jpg")}, + map[string]photoFile{"/img/curry.jpg": jpegFile(t, 640, 480)}, + search) + env.llm.QueueGenerate(normalizedReply()) + + first, err := env.g.Ground(t.Context(), "Curry", "curry") + if err != nil { + t.Fatalf("first Ground: %v", err) + } + pages, images, calls := env.pages.fetchCount(), env.images.requests.Load(), len(env.llm.Requests()) + + // The stored page is gone and the model has no reply left: only the + // stored recipe can ground the dish. + delete(env.pages.pages, "https://b.example/curry") + again, err := env.g.Ground(t.Context(), "Chicken Curry", "chicken curry") + if err != nil { + t.Fatalf("second Ground: %v", err) + } + if again.RecipeID != first.RecipeID || again.PhotoID != first.PhotoID || again.Method != first.Method || + again.Difficulty != first.Difficulty || *again.MinutesTotal != *first.MinutesTotal || again.SiteName != first.SiteName { + t.Errorf("reused grounding = %+v, want %+v", again, first) + } + if env.images.requests.Load() != images || len(env.llm.Requests()) != calls { + t.Errorf("reuse fetched images %d→%d, model calls %d→%d; want none", + images, env.images.requests.Load(), calls, len(env.llm.Requests())) + } + // Ranking fetches only the candidate not stored; the stored one ranks + // on its stored recipe. + env.pages.mu.Lock() + fetched := append([]string(nil), env.pages.fetched[pages:]...) + env.pages.mu.Unlock() + if len(fetched) != 1 || fetched[0] != "https://a.example/other" { + t.Errorf("reuse fetched %v, want only https://a.example/other", fetched) + } + pages = env.pages.fetchCount() + + byURL, err := env.g.GroundURL(t.Context(), "https://b.example/curry") + if err != nil || byURL.RecipeID != first.RecipeID { + t.Errorf("GroundURL of the stored page = %+v, %v; want the stored recipe", byURL, err) + } + if env.pages.fetchCount() != pages { + t.Error("GroundURL refetched a stored page") + } + if n := countRows(t, env.st, "recipes"); n != 1 { + t.Errorf("%d recipes rows, want 1", n) + } +} + +func TestGroundStoredRecipeDoesNotOutrankRelevantOne(t *testing.T) { + t.Parallel() + search := &fakeSearch{results: map[string][]Candidate{ + "brownies": {{URL: "https://b.example/brownies"}}, + "chicken curry": {{URL: "https://b.example/brownies"}, {URL: "https://a.example/curry"}}, + }} + env := newGroundEnv(t, + map[string][]byte{ + "https://a.example/curry": groundPage("Chicken Curry", "/img/curry.jpg"), + "https://b.example/brownies": groundPage("Fudgy Brownies", "/img/brownies.jpg"), + }, + map[string]photoFile{ + "/img/curry.jpg": jpegFile(t, 640, 480), + "/img/brownies.jpg": jpegFile(t, 640, 480), + }, + search) + env.llm.QueueGenerate(normalizedReply(), normalizedReply()) + + brownies, err := env.g.Ground(t.Context(), "Brownies", "brownies") + if err != nil { + t.Fatalf("Ground brownies: %v", err) + } + + // Search lists the unrelated stored recipe first and the relevant new + // one second: ranking, not search order or storage, grounds the + // relevant one. + curry, err := env.g.Ground(t.Context(), "Chicken Curry", "chicken curry") + if err != nil { + t.Fatalf("Ground chicken curry: %v", err) + } + if curry.SourceURL != "https://a.example/curry" || curry.RecipeID == brownies.RecipeID { + t.Errorf("grounded Chicken Curry in %s (recipe %s), want https://a.example/curry, not the stored brownies %s", + curry.SourceURL, curry.RecipeID, brownies.RecipeID) + } + if n := countRows(t, env.st, "recipes"); n != 2 { + t.Errorf("%d recipes rows, want 2", n) + } +} + +func TestGroundURL(t *testing.T) { + t.Parallel() + env := newGroundEnv(t, + map[string][]byte{ + "https://b.example/pasted": groundPage("Grandma's Slow Cooker Chili", "/img/chili.jpg"), + "https://b.example/no-image": recipePage("Chili", true, false, true, true), + "https://b.example/no-time": page("", `{"@type":"Recipe","name":"Chili","image":"https://example.com/img/chili.jpg", + "recipeIngredient":["1 onion"],"totalTime":"PT0M"}`), + }, + map[string]photoFile{"/img/chili.jpg": jpegFile(t, 640, 480)}, + &fakeSearch{}) + env.llm.QueueGenerate(normalizedReply()) + + gr, err := env.g.GroundURL(t.Context(), "https://b.example/pasted") + if err != nil { + t.Fatalf("GroundURL: %v", err) + } + if gr.SourceURL != "https://b.example/pasted" || gr.Method != "crockpot" || gr.PhotoID == "" { + t.Errorf("Grounded = %+v, want the pasted URL, crockpot from the title", gr) + } + if env.search.callCount() != 0 { + t.Error("GroundURL searched") + } + + for rawURL, reason := range map[string]error{ + "https://b.example/no-image": ErrNoImage, + "https://b.example/no-time": ErrTotalTimeNotPositive, + "https://b.example/missing": nil, + } { + _, err := env.g.GroundURL(t.Context(), rawURL) + var ungroundable *ErrUngroundable + if !errors.As(err, &ungroundable) || ungroundable.Target != rawURL { + t.Errorf("GroundURL(%s) error = %v, want *ErrUngroundable", rawURL, err) + } + var status *StatusError + if reason != nil && !errors.Is(err, reason) || reason == nil && !errors.As(err, &status) { + t.Errorf("GroundURL(%s) error = %v, want reason %v", rawURL, err, reason) + } + } +} + +func TestGroundManyIsolatesFailures(t *testing.T) { + t.Parallel() + search := &fakeSearch{ + results: map[string][]Candidate{ + "curry": {{URL: "https://a.example/curry"}}, + "nothing": {{URL: "https://a.example/missing"}}, + "chili": {{URL: "https://b.example/chili"}}, + }, + errs: map[string]error{"broken": ErrSearchRefused}, + } + env := newGroundEnv(t, + map[string][]byte{ + "https://a.example/curry": groundPage("Curry", "/img/curry.jpg"), + "https://b.example/chili": groundPage("Chili", "/img/chili.jpg"), + }, + map[string]photoFile{"/img/curry.jpg": jpegFile(t, 640, 480), "/img/chili.jpg": jpegFile(t, 480, 640)}, + search) + env.llm.QueueGenerate(normalizedReply(), normalizedReply()) + + results := env.g.GroundMany(t.Context(), []GroundRequest{ + {DishName: "Curry", Query: "curry"}, + {DishName: "Nothing", Query: "nothing"}, + {DishName: "Broken", Query: "broken"}, + {DishName: "Chili", Query: "chili"}, + }) + if len(results) != 4 { + t.Fatalf("%d results, want 4", len(results)) + } + if results[0].Err != nil || results[0].Grounded.SourceURL != "https://a.example/curry" { + t.Errorf("curry = %+v", results[0]) + } + var ungroundable *ErrUngroundable + if !errors.As(results[1].Err, &ungroundable) { + t.Errorf("nothing error = %v, want *ErrUngroundable", results[1].Err) + } + if !errors.Is(results[2].Err, ErrSearchRefused) { + t.Errorf("broken error = %v, want ErrSearchRefused", results[2].Err) + } + if results[3].Err != nil || results[3].Grounded.SourceURL != "https://b.example/chili" { + t.Errorf("chili = %+v", results[3]) + } + if n := countRows(t, env.st, "recipes"); n != 2 { + t.Errorf("%d recipes rows, want 2", n) + } +} + +func TestGroundManyLimitsConcurrency(t *testing.T) { + t.Parallel() + search := &fakeSearch{started: make(chan string, 5), gate: make(chan struct{})} + env := newGroundEnv(t, nil, nil, search) + reqs := make([]GroundRequest, 5) + for i := range reqs { + reqs[i] = GroundRequest{DishName: fmt.Sprintf("Dish %d", i), Query: fmt.Sprintf("dish %d", i)} + } + done := make(chan []GroundResult) + go func() { done <- env.g.GroundMany(t.Context(), reqs) }() + + for range GroundConcurrency { + select { + case <-search.started: + case <-time.After(5 * time.Second): + t.Fatal("fewer than GroundConcurrency items started") + } + } + select { + case q := <-search.started: + t.Fatalf("%s started while %d were in flight", q, GroundConcurrency) + case <-time.After(100 * time.Millisecond): + } + close(search.gate) + results := <-done + for i, r := range results { + if !errors.Is(r.Err, ErrNoCandidates) { + t.Errorf("item %d error = %v, want ErrNoCandidates", i, r.Err) + } + } + if search.maxActive != GroundConcurrency || search.callCount() != 5 { + t.Errorf("max in flight %d of %d calls, want %d of 5", search.maxActive, search.callCount(), GroundConcurrency) + } +} diff --git a/internal/recipes/photo.go b/internal/recipes/photo.go index 107c00d..b57d223 100644 --- a/internal/recipes/photo.go +++ b/internal/recipes/photo.go @@ -19,7 +19,6 @@ import ( "path/filepath" "slices" "strconv" - "sync" "github.com/oklog/ulid/v2" "golang.org/x/image/draw" @@ -69,16 +68,12 @@ var photoDecoders = map[string]func([]byte) (image.Image, error){ // PhotoCache downloads photos through a Fetcher and stores each at every // PhotoSizes length as {id}-{size}.jpg under PhotoDir, with a photos row. // Photos are deduplicated by the SHA-256 of the downloaded bytes. A -// PhotoCache is safe for concurrent use; photos are processed one at a time. +// PhotoCache is safe for concurrent use. type PhotoCache struct { fetcher *Fetcher store *store.Store // dir is the absolute photo directory. dir string - - // mu serialises the dedupe lookup, processing and insert, so one image - // is never stored twice. - mu sync.Mutex } // NewPhotoCache returns a PhotoCache storing photos under dataDir, creating @@ -100,68 +95,88 @@ func NewPhotoCache(fetcher *Fetcher, st *store.Store, dataDir string) (*PhotoCac return &PhotoCache{fetcher: fetcher, store: st, dir: dir}, nil } -// CachePhoto downloads the image at rawURL and returns its photos row. An -// image whose bytes are already cached returns the existing row. Otherwise -// it is decoded, refused if its shorter side is under PhotoMinShortSide, -// scaled so its long side is each of PhotoSizes, encoded as JPEG and -// recorded. The row's width and height are the original image's, and its -// local_path is the largest size's file relative to the data directory. -func (c *PhotoCache) CachePhoto(ctx context.Context, rawURL string) (db.Photo, error) { +// DownloadedPhoto is an image DownloadPhoto has downloaded and decoded, +// ready for CachePhoto. +type DownloadedPhoto struct { + // url is the image's final URL, after redirects. + url string + sha256 string + img image.Image +} + +// DownloadPhoto downloads the image at rawURL and decodes it, refusing one +// whose shorter side is under PhotoMinShortSide. It records nothing, so a +// caller can try another image before opening a transaction. +func (c *PhotoCache) DownloadPhoto(ctx context.Context, rawURL string) (DownloadedPhoto, error) { page, err := c.fetcher.FetchImage(ctx, rawURL) if err != nil { - return db.Photo{}, err + return DownloadedPhoto{}, err } - digest := sha256.Sum256(page.Body) - sum := hex.EncodeToString(digest[:]) - - c.mu.Lock() - defer c.mu.Unlock() - existing, err := db.New(c.store.Reader).GetPhotoBySHA256(ctx, sum) - if err == nil { - return existing, nil - } - if !errors.Is(err, sql.ErrNoRows) { - return db.Photo{}, fmt.Errorf("recipes: look up photo: %w", err) - } - img, err := decodePhoto(page.Body) if err != nil { - return db.Photo{}, fmt.Errorf("%w: %s", err, page.URL) + return DownloadedPhoto{}, fmt.Errorf("%w: %s", err, page.URL) } + digest := sha256.Sum256(page.Body) + return DownloadedPhoto{url: page.URL, sha256: hex.EncodeToString(digest[:]), img: img}, nil +} + +// CachePhoto records p in tx, the caller's transaction, so its photos row +// commits or rolls back with the rows that refer to it. An image whose +// bytes are already recorded returns that row and created false. Otherwise +// p is scaled so its long side is each of PhotoSizes, encoded as JPEG and +// recorded, and created is true: if tx then does not commit, the caller +// removes the files with RemovePhotoFiles. The row's width and height are +// the original image's, and its local_path is the largest size's file +// relative to the data directory. +// +// The store's writer has one connection, so transactions run one at a +// time and an image is never recorded twice. +func (c *PhotoCache) CachePhoto(ctx context.Context, tx *sql.Tx, p DownloadedPhoto) (photo db.Photo, created bool, err error) { + q := db.New(tx) + existing, err := q.GetPhotoBySHA256(ctx, p.sha256) + if err == nil { + return existing, false, nil + } + if !errors.Is(err, sql.ErrNoRows) { + return db.Photo{}, false, fmt.Errorf("recipes: look up photo: %w", err) + } + id := store.NewID() - var written []string - removeWritten := func() error { - var errs []error - for _, p := range written { - errs = append(errs, os.Remove(p)) - } - return errors.Join(errs...) - } for _, size := range PhotoSizes { - p := c.path(id, size) - if err := writeJPEG(p, scaleLongSide(img, size)); err != nil { - return db.Photo{}, errors.Join(err, removeWritten()) + if err := writeJPEG(c.path(id, size), scaleLongSide(p.img, size)); err != nil { + return db.Photo{}, false, errors.Join(err, c.RemovePhotoFiles(id)) } - written = append(written, p) } now := store.Now().String() - bounds := img.Bounds() - photo, err := db.New(c.store.Writer).CreatePhoto(ctx, db.CreatePhotoParams{ + bounds := p.img.Bounds() + photo, err = q.CreatePhoto(ctx, db.CreatePhotoParams{ ID: id, - SourceUrl: page.URL, + SourceUrl: p.url, LocalPath: path.Join(PhotoDir, photoFileName(id, PhotoSizes[0])), Width: int64(bounds.Dx()), Height: int64(bounds.Dy()), - Sha256: sum, + Sha256: p.sha256, FetchedAt: now, CreatedAt: now, UpdatedAt: now, }) if err != nil { - return db.Photo{}, errors.Join(fmt.Errorf("recipes: record photo: %w", err), removeWritten()) + return db.Photo{}, false, errors.Join(fmt.Errorf("recipes: record photo: %w", err), c.RemovePhotoFiles(id)) } - return photo, nil + return photo, true, nil +} + +// RemovePhotoFiles removes the files of photo id at every PhotoSizes +// length; a file already absent is not an error. +func (c *PhotoCache) RemovePhotoFiles(id string) error { + var errs []error + for _, size := range PhotoSizes { + if err := os.Remove(c.path(id, size)); err != nil && !errors.Is(err, fs.ErrNotExist) { + errs = append(errs, err) + } + } + return errors.Join(errs...) } // OpenPhoto opens the stored file of photo id at size. An id that is not a diff --git a/internal/recipes/photo_test.go b/internal/recipes/photo_test.go index a8c08bd..dcce099 100644 --- a/internal/recipes/photo_test.go +++ b/internal/recipes/photo_test.go @@ -3,6 +3,7 @@ package recipes import ( "bytes" "context" + "database/sql" "errors" "image" "image/color" @@ -18,6 +19,7 @@ import ( "sync/atomic" "testing" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) @@ -72,6 +74,22 @@ func newTestPhotoCache(t *testing.T, files map[string]photoFile) (*PhotoCache, * return cache, st, dataDir, srv } +// cachePhoto downloads the image at rawURL and records it in its own +// transaction, as a caller of DownloadPhoto and CachePhoto does. +func cachePhoto(t *testing.T, cache *PhotoCache, rawURL string) (db.Photo, error) { + t.Helper() + p, err := cache.DownloadPhoto(t.Context(), rawURL) + if err != nil { + return db.Photo{}, err + } + var photo db.Photo + err = cache.store.WithTx(t.Context(), func(tx *sql.Tx) error { + photo, _, err = cache.CachePhoto(t.Context(), tx, p) + return err + }) + return photo, err +} + // gradient is a w×h opaque image. func gradient(w, h int) *image.RGBA { img := image.NewRGBA(image.Rect(0, 0, w, h)) @@ -167,7 +185,7 @@ func TestCachePhotoFormatsAndSizes(t *testing.T) { {"/square.png", 1500, 1500, map[int]image.Point{1200: {1200, 1200}, 400: {400, 400}}}, } for _, tc := range tests { - photo, err := cache.CachePhoto(t.Context(), "https://example.com"+tc.path) + photo, err := cachePhoto(t, cache, "https://example.com"+tc.path) if err != nil { t.Fatalf("CachePhoto %s: %v", tc.path, err) } @@ -194,7 +212,7 @@ func TestCachePhotoGIFUsesFirstFrame(t *testing.T) { cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ "/anim.gif": {"image/gif", twoFrameGIF(t, 400, 400)}, }) - photo, err := cache.CachePhoto(t.Context(), "https://example.com/anim.gif") + photo, err := cachePhoto(t, cache, "https://example.com/anim.gif") if err != nil { t.Fatalf("CachePhoto: %v", err) } @@ -220,7 +238,7 @@ func TestCachePhotoRejectsSmallGIFFirstFrame(t *testing.T) { cache, st, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ "/speck.gif": {"image/gif", b.Bytes()}, }) - if _, err := cache.CachePhoto(t.Context(), "https://example.com/speck.gif"); !errors.Is(err, ErrPhotoTooSmall) { + if _, err := cachePhoto(t, cache, "https://example.com/speck.gif"); !errors.Is(err, ErrPhotoTooSmall) { t.Errorf("CachePhoto error = %v, want ErrPhotoTooSmall", err) } if n := countPhotos(t, st); n != 0 { @@ -236,7 +254,7 @@ func TestCachePhotoFillsTransparencyWhite(t *testing.T) { cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ "/clear.png": {"image/png", encodePNG(t, image.NewNRGBA(image.Rect(0, 0, 400, 400)))}, }) - photo, err := cache.CachePhoto(t.Context(), "https://example.com/clear.png") + photo, err := cachePhoto(t, cache, "https://example.com/clear.png") if err != nil { t.Fatalf("CachePhoto: %v", err) } @@ -276,7 +294,7 @@ func TestCachePhotoRejectsTooSmall(t *testing.T) { "/edge.png": {"image/png", encodePNG(t, gradient(300, 1000))}, }) for _, p := range []string{"/narrow.png", "/short.jpg"} { - if _, err := cache.CachePhoto(t.Context(), "https://example.com"+p); !errors.Is(err, ErrPhotoTooSmall) { + if _, err := cachePhoto(t, cache, "https://example.com"+p); !errors.Is(err, ErrPhotoTooSmall) { t.Errorf("CachePhoto %s error = %v, want ErrPhotoTooSmall", p, err) } } @@ -286,7 +304,7 @@ func TestCachePhotoRejectsTooSmall(t *testing.T) { if names := photoFiles(t, dataDir); len(names) != 0 { t.Errorf("files stored for refused images: %v", names) } - if _, err := cache.CachePhoto(t.Context(), "https://example.com/edge.png"); err != nil { + if _, err := cachePhoto(t, cache, "https://example.com/edge.png"); err != nil { t.Errorf("CachePhoto with a 300 px short side: %v", err) } } @@ -297,10 +315,10 @@ func TestCachePhotoRejectsNonImages(t *testing.T) { "/page.html": {"text/html", encodePNG(t, gradient(400, 400))}, "/fake.jpg": {"image/jpeg", []byte("not a picture")}, }) - if _, err := cache.CachePhoto(t.Context(), "https://example.com/page.html"); !errors.Is(err, ErrNotImage) { + if _, err := cachePhoto(t, cache, "https://example.com/page.html"); !errors.Is(err, ErrNotImage) { t.Errorf("text/html error = %v, want ErrNotImage", err) } - if _, err := cache.CachePhoto(t.Context(), "https://example.com/fake.jpg"); !errors.Is(err, ErrPhotoFormat) { + if _, err := cachePhoto(t, cache, "https://example.com/fake.jpg"); !errors.Is(err, ErrPhotoFormat) { t.Errorf("undecodable image error = %v, want ErrPhotoFormat", err) } if n := countPhotos(t, st); n != 0 { @@ -316,11 +334,11 @@ func TestCachePhotoDeduplicatesBySHA256(t *testing.T) { "/b.jpg": {"image/jpeg", body}, "/other.jpg": {"image/jpeg", encodeJPEG(t, gradient(480, 640))}, }) - first, err := cache.CachePhoto(t.Context(), "https://example.com/a.jpg") + first, err := cachePhoto(t, cache, "https://example.com/a.jpg") if err != nil { t.Fatalf("CachePhoto a: %v", err) } - again, err := cache.CachePhoto(t.Context(), "https://example.com/b.jpg") + again, err := cachePhoto(t, cache, "https://example.com/b.jpg") if err != nil { t.Fatalf("CachePhoto b: %v", err) } @@ -330,7 +348,7 @@ func TestCachePhotoDeduplicatesBySHA256(t *testing.T) { if n := srv.requests.Load(); n != 2 { t.Errorf("%d downloads, want 2: each URL is fetched to hash it", n) } - other, err := cache.CachePhoto(t.Context(), "https://example.com/other.jpg") + other, err := cachePhoto(t, cache, "https://example.com/other.jpg") if err != nil { t.Fatalf("CachePhoto other: %v", err) } @@ -350,7 +368,7 @@ func TestOpenPhoto(t *testing.T) { cache, _, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ "/a.png": {"image/png", encodePNG(t, gradient(400, 300))}, }) - photo, err := cache.CachePhoto(t.Context(), "https://example.com/a.png") + photo, err := cachePhoto(t, cache, "https://example.com/a.png") if err != nil { t.Fatalf("CachePhoto: %v", err) } @@ -436,3 +454,42 @@ func TestFetchImageContentTypes(t *testing.T) { t.Errorf("Fetch: %v", err) } } + +func TestCachePhotoRollsBackWithTransaction(t *testing.T) { + t.Parallel() + cache, st, dataDir, _ := newTestPhotoCache(t, map[string]photoFile{ + "/a.jpg": {"image/jpeg", encodeJPEG(t, gradient(640, 480))}, + }) + p, err := cache.DownloadPhoto(t.Context(), "https://example.com/a.jpg") + if err != nil { + t.Fatal(err) + } + errAbort := errors.New("abort") + var photo db.Photo + var created bool + err = st.WithTx(t.Context(), func(tx *sql.Tx) error { + photo, created, err = cache.CachePhoto(t.Context(), tx, p) + if err != nil { + return err + } + return errAbort + }) + if !errors.Is(err, errAbort) || !created { + t.Fatalf("WithTx error = %v, created = %v; want the abort after a new photo", err, created) + } + if n := countPhotos(t, st); n != 0 { + t.Errorf("%d photos rows after rollback", n) + } + if names := photoFiles(t, dataDir); len(names) != len(PhotoSizes) { + t.Errorf("files before removal = %v, want one per size", names) + } + if err := cache.RemovePhotoFiles(photo.ID); err != nil { + t.Fatal(err) + } + if names := photoFiles(t, dataDir); len(names) != 0 { + t.Errorf("files after RemovePhotoFiles = %v", names) + } + if err := cache.RemovePhotoFiles(photo.ID); err != nil { + t.Errorf("RemovePhotoFiles of absent files: %v", err) + } +} diff --git a/internal/recipes/rank.go b/internal/recipes/rank.go index 8d3c254..69ed9ce 100644 --- a/internal/recipes/rank.go +++ b/internal/recipes/rank.go @@ -45,17 +45,26 @@ type Ranked struct { } // Rank fetches and extracts the first RankDepth candidates concurrently and -// returns those whose recipe has ingredients and an image, highest Score -// first; equal scores keep search order. +// returns those whose recipe is usable — it has ingredients and an image, +// and any total time it gives is positive — highest Score first; equal +// scores keep search order. // // Every candidate that is not ranked — its fetch failed, its page holds no -// valid recipe, or the recipe lacks ingredients or an image — is reported, -// never dropped silently: when some are ranked the error is +// valid recipe, or the recipe is not usable — is reported, never dropped +// silently: when some are ranked the error is // ErrCandidatesUnranked, returned together with the ranked ones; when none // are it is ErrNoUsableRecipe with no results. Either is joined with each // unranked candidate's URL and reason. A cancelled ctx returns only its // error. func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([]Ranked, error) { + return rank(ctx, dish, cands, func(ctx context.Context, c Candidate) (Recipe, error) { + return fetchRecipe(ctx, f, c.URL) + }) +} + +// rank is Rank with each candidate's recipe got by load rather than always +// fetched. +func rank(ctx context.Context, dish string, cands []Candidate, load func(context.Context, Candidate) (Recipe, error)) ([]Ranked, error) { if len(cands) > RankDepth { cands = cands[:RankDepth] } @@ -69,7 +78,7 @@ func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([ wg.Add(1) go func() { defer wg.Done() - r, err := rankOne(ctx, f, dish, c) + r, err := rankOne(ctx, load, dish, c) outcomes[i] = outcome{r, err} }() } @@ -97,26 +106,42 @@ func Rank(ctx context.Context, f PageFetcher, dish string, cands []Candidate) ([ return ranked, nil } -// rankOne fetches, extracts and scores one candidate, or says why it is +// rankOne loads and scores one candidate's recipe, or says why it is // rejected. -func rankOne(ctx context.Context, f PageFetcher, dish string, c Candidate) (Ranked, error) { - page, err := f.Fetch(ctx, c.URL) +func rankOne(ctx context.Context, load func(context.Context, Candidate) (Recipe, error), dish string, c Candidate) (Ranked, error) { + r, err := load(ctx, c) if err != nil { return Ranked{}, err } - r, err := Extract(page.Body, page.URL) - if err != nil { + if err := usable(r); err != nil { return Ranked{}, err } - if len(r.Ingredients) == 0 { - return Ranked{}, ErrNoIngredients - } - if r.ImageURL == "" { - return Ranked{}, ErrNoImage - } return Ranked{Candidate: c, Recipe: r, Score: harness.Similarity(r.Title, dish) + completeness(r)}, nil } +// fetchRecipe fetches the page at rawURL and extracts its recipe. +func fetchRecipe(ctx context.Context, f PageFetcher, rawURL string) (Recipe, error) { + page, err := f.Fetch(ctx, rawURL) + if err != nil { + return Recipe{}, err + } + return Extract(page.Body, page.URL) +} + +// usable says why r cannot ground a dish: it has no ingredients or no +// image, or a total time that is not positive. +func usable(r Recipe) error { + switch { + case len(r.Ingredients) == 0: + return ErrNoIngredients + case r.ImageURL == "": + return ErrNoImage + case r.TotalMinutes != nil && *r.TotalMinutes <= 0: + return ErrTotalTimeNotPositive + } + return nil +} + // completeness is a quarter for each of ingredients, image, steps and total // minutes the recipe has. func completeness(r Recipe) float64 { diff --git a/internal/recipes/search.go b/internal/recipes/search.go index 6aec8e0..dda0842 100644 --- a/internal/recipes/search.go +++ b/internal/recipes/search.go @@ -71,12 +71,12 @@ type SearchProvider interface { } // NewSearchProvider returns the provider s names. SearXNG is called at -// searxngURL, the configured MEALQUEST_SEARXNG_URL; Brave is called with the -// API key in s.ApiKeyCt, decrypted by box. -func NewSearchProvider(s db.RecipeSearchSetting, box *secretbox.Box, searxngURL string) (SearchProvider, error) { +// s.BaseUrl; Brave is called with the API key in s.ApiKeyCt, decrypted by +// box. +func NewSearchProvider(s db.RecipeSearchSetting, box *secretbox.Box) (SearchProvider, error) { switch s.Provider { case "searxng": - return NewSearXNG(searxngURL) + return NewSearXNG(s.BaseUrl.String) case "brave": if len(s.ApiKeyCt) == 0 { return nil, errors.New("recipes: Brave search has no API key") diff --git a/internal/recipes/search_test.go b/internal/recipes/search_test.go index 8bda597..f18d465 100644 --- a/internal/recipes/search_test.go +++ b/internal/recipes/search_test.go @@ -3,6 +3,7 @@ package recipes import ( "context" "crypto/rand" + "database/sql" "encoding/base64" "errors" "net/http" @@ -262,7 +263,7 @@ func TestNewSearchProvider(t *testing.T) { t.Fatal(err) } - p, err := NewSearchProvider(db.RecipeSearchSetting{Provider: "brave", ApiKeyCt: ct}, box, "") + p, err := NewSearchProvider(db.RecipeSearchSetting{Provider: "brave", ApiKeyCt: ct}, box) if err != nil { t.Fatal(err) } @@ -270,7 +271,7 @@ func TestNewSearchProvider(t *testing.T) { t.Errorf("provider = %#v, want Brave with decrypted key", p) } - p, err = NewSearchProvider(db.RecipeSearchSetting{Provider: "searxng"}, box, "http://searxng:8080") + p, err = NewSearchProvider(db.RecipeSearchSetting{Provider: "searxng", BaseUrl: sql.NullString{String: "http://searxng:8080", Valid: true}}, box) if err != nil { t.Fatal(err) } @@ -278,16 +279,13 @@ func TestNewSearchProvider(t *testing.T) { t.Errorf("provider = %#v, want SearXNG at http://searxng:8080/search", p) } - for name, tc := range map[string]struct { - s db.RecipeSearchSetting - url string - }{ - "brave without key": {db.RecipeSearchSetting{Provider: "brave"}, ""}, - "brave bad key": {db.RecipeSearchSetting{Provider: "brave", ApiKeyCt: []byte("not a token at all, far too plain")}, ""}, - "searxng without URL": {db.RecipeSearchSetting{Provider: "searxng"}, ""}, - "unknown provider": {db.RecipeSearchSetting{Provider: "google"}, ""}, + for name, s := range map[string]db.RecipeSearchSetting{ + "brave without key": {Provider: "brave"}, + "brave bad key": {Provider: "brave", ApiKeyCt: []byte("not a token at all, far too plain")}, + "searxng without URL": {Provider: "searxng"}, + "unknown provider": {Provider: "google"}, } { - if _, err := NewSearchProvider(tc.s, box, tc.url); err == nil { + if _, err := NewSearchProvider(s, box); err == nil { t.Errorf("%s: succeeded, want error", name) } } diff --git a/internal/recipes/searchsettings.go b/internal/recipes/searchsettings.go new file mode 100644 index 0000000..55d148b --- /dev/null +++ b/internal/recipes/searchsettings.go @@ -0,0 +1,245 @@ +package recipes + +import ( + "context" + "database/sql" + "errors" + "fmt" + "strings" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" +) + +const ( + // SearchTestQuery is what TestSearch searches for. + SearchTestQuery = "chicken tikka masala" + // MinBraveKeyLength is the shortest Brave API key accepted, so its last + // four characters never give away most of it. + MinBraveKeyLength = 8 +) + +// ErrSearchNotConfigured is returned when no recipe search provider has +// been chosen yet. +var ErrSearchNotConfigured = errors.New("recipes: recipe search is not configured") + +// SearchSettings is the household's recipe search configuration as the API +// shows it. The Brave API key is never part of it: only whether one is +// stored and its last four characters. +type SearchSettings struct { + // Provider is "searxng" or "brave", nil until first chosen. + Provider *string `json:"provider"` + // BaseURL is the SearXNG instance's URL, nil when not set. + BaseURL *string `json:"base_url"` + APIKeySet bool `json:"api_key_set"` + APIKeyLast4 *string `json:"api_key_last4"` + // UpdatedAt is nil until the settings are first saved. + UpdatedAt *string `json:"updated_at"` +} + +// SearchSettingsPatch changes the settings. A nil field is left as it is; +// an empty BaseURL or APIKey removes the stored one. +type SearchSettingsPatch struct { + Provider *string `json:"provider"` + BaseURL *string `json:"base_url"` + // APIKey is the Brave API key in plain text. It is stored encrypted + // and never returned. + APIKey *string `json:"api_key"` +} + +// SearchSettingsProblem is one invalid field of a SearchSettingsPatch, +// named by its JSON key. +type SearchSettingsProblem struct { + Field string + Reason string +} + +// SearchSettingsError lists every problem with a SearchSettingsPatch, or +// with the settings it would leave. +type SearchSettingsError struct { + Problems []SearchSettingsProblem +} + +func (e *SearchSettingsError) Error() string { + parts := make([]string, len(e.Problems)) + for i, p := range e.Problems { + parts[i] = p.Field + ": " + p.Reason + } + return "invalid recipe search settings: " + strings.Join(parts, "; ") +} + +// SearchSettingsService reads and changes the recipe_search_settings row +// and builds the search provider it names. Brave API keys are sealed with +// its secret box. +type SearchSettingsService struct { + st *store.Store + box *secretbox.Box + // newProvider is NewSearchProvider; tests point Brave elsewhere. + newProvider func(db.RecipeSearchSetting, *secretbox.Box) (SearchProvider, error) +} + +// NewSearchSettingsService returns the service over st, sealing keys with +// box. +func NewSearchSettingsService(st *store.Store, box *secretbox.Box) (*SearchSettingsService, error) { + if st == nil { + return nil, errors.New("recipes: nil store") + } + if box == nil { + return nil, errors.New("recipes: nil secret box") + } + return &SearchSettingsService{st: st, box: box, newProvider: NewSearchProvider}, nil +} + +// Get returns the settings; before any are saved every field is unset. +func (s *SearchSettingsService) Get(ctx context.Context) (SearchSettings, error) { + row, err := db.New(s.st.Reader).GetRecipeSearchSettings(ctx) + if errors.Is(err, sql.ErrNoRows) { + return SearchSettings{}, nil + } + if err != nil { + return SearchSettings{}, fmt.Errorf("recipes: read recipe search settings: %w", err) + } + return searchSettingsFrom(row), nil +} + +// Update applies patch and returns the settings it leaves. The first update +// must choose a provider. The result must be usable: SearXNG needs a base +// URL and Brave an API key. Every problem is reported in a +// *SearchSettingsError. +func (s *SearchSettingsService) Update(ctx context.Context, patch SearchSettingsPatch) (SearchSettings, error) { + var saved db.RecipeSearchSetting + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + q := db.New(tx) + row, err := q.GetRecipeSearchSettings(ctx) + exists := err == nil + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return fmt.Errorf("recipes: read recipe search settings: %w", err) + } + next, err := s.apply(row, exists, patch) + if err != nil { + return err + } + now := store.Now().String() + created := now + if exists { + created = row.CreatedAt + } + saved, err = q.UpsertRecipeSearchSettings(ctx, db.UpsertRecipeSearchSettingsParams{ + Provider: next.Provider, + BaseUrl: next.BaseUrl, + ApiKeyCt: next.ApiKeyCt, + ApiKeyLast4: next.ApiKeyLast4, + CreatedAt: created, + UpdatedAt: now, + }) + if err != nil { + return fmt.Errorf("recipes: save recipe search settings: %w", err) + } + return nil + }) + if err != nil { + return SearchSettings{}, err + } + return searchSettingsFrom(saved), nil +} + +// apply returns row with patch applied, validated. +func (s *SearchSettingsService) apply(row db.RecipeSearchSetting, exists bool, patch SearchSettingsPatch) (db.RecipeSearchSetting, error) { + var problems []SearchSettingsProblem + if patch.Provider != nil { + switch *patch.Provider { + case "searxng", "brave": + row.Provider = *patch.Provider + default: + problems = append(problems, SearchSettingsProblem{"provider", `must be "searxng" or "brave"`}) + } + } else if !exists { + problems = append(problems, SearchSettingsProblem{"provider", "is required until recipe search is configured"}) + } + if patch.BaseURL != nil { + row.BaseUrl = sql.NullString{} + if u := *patch.BaseURL; u != "" { + if _, err := NewSearXNG(u); err != nil { + problems = append(problems, SearchSettingsProblem{"base_url", strings.TrimPrefix(err.Error(), "recipes: ")}) + } else { + row.BaseUrl = sql.NullString{String: u, Valid: true} + } + } + } + if patch.APIKey != nil { + row.ApiKeyCt, row.ApiKeyLast4 = nil, sql.NullString{} + if key := *patch.APIKey; key != "" { + // The key is never echoed in a problem. + if _, err := NewBrave(key); err != nil { + problems = append(problems, SearchSettingsProblem{"api_key", "has a character not allowed in a header"}) + } else if len(key) < MinBraveKeyLength { + problems = append(problems, SearchSettingsProblem{"api_key", fmt.Sprintf("must be at least %d characters", MinBraveKeyLength)}) + } else { + ct, err := s.box.Encrypt([]byte(key)) + if err != nil { + return db.RecipeSearchSetting{}, fmt.Errorf("recipes: seal Brave API key: %w", err) + } + row.ApiKeyCt = ct + row.ApiKeyLast4 = sql.NullString{String: key[len(key)-4:], Valid: true} + } + } + } + if len(problems) == 0 { + switch { + case row.Provider == "searxng" && !row.BaseUrl.Valid: + problems = append(problems, SearchSettingsProblem{"base_url", "is required for SearXNG"}) + case row.Provider == "brave" && len(row.ApiKeyCt) == 0: + problems = append(problems, SearchSettingsProblem{"api_key", "is required for Brave"}) + } + } + if len(problems) > 0 { + return db.RecipeSearchSetting{}, &SearchSettingsError{Problems: problems} + } + return row, nil +} + +// Provider returns the search provider the settings name, or +// ErrSearchNotConfigured. +func (s *SearchSettingsService) Provider(ctx context.Context) (SearchProvider, error) { + row, err := db.New(s.st.Reader).GetRecipeSearchSettings(ctx) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrSearchNotConfigured + } + if err != nil { + return nil, fmt.Errorf("recipes: read recipe search settings: %w", err) + } + return s.newProvider(row, s.box) +} + +// TestSearch searches for SearchTestQuery with the configured provider and +// returns how many candidates it found, up to MaxSearchResults. +func (s *SearchSettingsService) TestSearch(ctx context.Context) (int, error) { + p, err := s.Provider(ctx) + if err != nil { + return 0, err + } + cands, err := p.Search(ctx, SearchTestQuery, MaxSearchResults) + if err != nil { + return 0, err + } + return len(cands), nil +} + +func searchSettingsFrom(row db.RecipeSearchSetting) SearchSettings { + provider, updated := row.Provider, row.UpdatedAt + return SearchSettings{ + Provider: &provider, + BaseURL: nullString(row.BaseUrl), + APIKeySet: len(row.ApiKeyCt) > 0, + APIKeyLast4: nullString(row.ApiKeyLast4), + UpdatedAt: &updated, + } +} + +func nullString(s sql.NullString) *string { + if !s.Valid { + return nil + } + return &s.String +} diff --git a/internal/recipes/searchsettings_test.go b/internal/recipes/searchsettings_test.go new file mode 100644 index 0000000..0de2ad5 --- /dev/null +++ b/internal/recipes/searchsettings_test.go @@ -0,0 +1,172 @@ +package recipes + +import ( + "crypto/rand" + "encoding/base64" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" +) + +const testBraveKey = "BSA-secret-brave-key-9f3c" + +func newTestSearchSettings(t *testing.T) *SearchSettingsService { + t.Helper() + st, err := store.Open(t.Context(), t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := st.Close(); err != nil { + t.Error(err) + } + }) + if err := store.Migrate(t.Context(), st.Writer, migrations.FS); err != nil { + t.Fatal(err) + } + key := make([]byte, secretbox.KeySize) + rand.Read(key) + box, err := secretbox.New(base64.StdEncoding.EncodeToString(key)) + if err != nil { + t.Fatal(err) + } + svc, err := NewSearchSettingsService(st, box) + if err != nil { + t.Fatal(err) + } + return svc +} + +func ptr(s string) *string { return &s } + +// noKey fails the test when v, marshalled as the API would, holds the +// Brave key. +func noKey(t *testing.T, what string, v any) { + t.Helper() + b, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(b), testBraveKey) || strings.Contains(string(b), testBraveKey[:12]) { + t.Errorf("%s holds the Brave key: %s", what, b) + } +} + +func TestSearchSettingsUpdate(t *testing.T) { + t.Parallel() + svc := newTestSearchSettings(t) + got, err := svc.Get(t.Context()) + if err != nil || got != (SearchSettings{}) { + t.Fatalf("Get before any update = %+v, %v; want every field unset", got, err) + } + if _, err := svc.Provider(t.Context()); !errors.Is(err, ErrSearchNotConfigured) { + t.Errorf("Provider before any update error = %v, want ErrSearchNotConfigured", err) + } + + for name, patch := range map[string]SearchSettingsPatch{ + "no provider": {BaseURL: ptr("http://searxng:8080")}, + "unknown provider": {Provider: ptr("google")}, + "searxng without URL": {Provider: ptr("searxng")}, + "bad URL": {Provider: ptr("searxng"), BaseURL: ptr("ftp://searxng")}, + "brave without key": {Provider: ptr("brave")}, + "short key": {Provider: ptr("brave"), APIKey: ptr("abc12")}, + "key with space": {Provider: ptr("brave"), APIKey: ptr("abc 12345678")}, + } { + var invalid *SearchSettingsError + if _, err := svc.Update(t.Context(), patch); !errors.As(err, &invalid) { + t.Errorf("%s: error = %v, want *SearchSettingsError", name, err) + } + } + + got, err = svc.Update(t.Context(), SearchSettingsPatch{Provider: ptr("searxng"), BaseURL: ptr("http://searxng:8080")}) + if err != nil { + t.Fatal(err) + } + if *got.Provider != "searxng" || *got.BaseURL != "http://searxng:8080" || got.APIKeySet || got.APIKeyLast4 != nil || got.UpdatedAt == nil { + t.Errorf("SearXNG settings = %+v", got) + } + + got, err = svc.Update(t.Context(), SearchSettingsPatch{Provider: ptr("brave"), APIKey: ptr(testBraveKey)}) + if err != nil { + t.Fatal(err) + } + if *got.Provider != "brave" || !got.APIKeySet || *got.APIKeyLast4 != "9f3c" || *got.BaseURL != "http://searxng:8080" { + t.Errorf("Brave settings = %+v", got) + } + noKey(t, "Update result", got) + got, err = svc.Get(t.Context()) + if err != nil { + t.Fatal(err) + } + noKey(t, "Get result", got) + row, err := db.New(svc.st.Reader).GetRecipeSearchSettings(t.Context()) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(row.ApiKeyCt), testBraveKey) { + t.Error("Brave key stored in plain text") + } + + if _, err := svc.Update(t.Context(), SearchSettingsPatch{APIKey: ptr("")}); err == nil { + t.Error("removing the key while Brave is chosen succeeded") + } + got, err = svc.Update(t.Context(), SearchSettingsPatch{Provider: ptr("searxng"), APIKey: ptr("")}) + if err != nil || got.APIKeySet || got.APIKeyLast4 != nil { + t.Errorf("switch to SearXNG removing the key = %+v, %v", got, err) + } +} + +func TestSearchSettingsTestSearch(t *testing.T) { + t.Parallel() + searx := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("q") != SearchTestQuery+" recipe" { + t.Errorf("SearXNG query = %q", r.URL.Query().Get("q")) + } + w.Write([]byte(`{"results":[{"url":"https://a.example/1"},{"url":"https://b.example/2"}]}`)) + })) + t.Cleanup(searx.Close) + svc := newTestSearchSettings(t) + if _, err := svc.TestSearch(t.Context()); !errors.Is(err, ErrSearchNotConfigured) { + t.Errorf("TestSearch unconfigured error = %v, want ErrSearchNotConfigured", err) + } + if _, err := svc.Update(t.Context(), SearchSettingsPatch{Provider: ptr("searxng"), BaseURL: ptr(searx.URL)}); err != nil { + t.Fatal(err) + } + if n, err := svc.TestSearch(t.Context()); err != nil || n != 2 { + t.Errorf("TestSearch = %d, %v; want 2", n, err) + } + + // Brave refuses the key; the error must not carry it. + brave := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("X-Subscription-Token") != testBraveKey { + t.Errorf("Brave token = %q", r.Header.Get("X-Subscription-Token")) + } + http.Error(w, "bad key "+testBraveKey, http.StatusUnauthorized) + })) + t.Cleanup(brave.Close) + svc.newProvider = func(s db.RecipeSearchSetting, box *secretbox.Box) (SearchProvider, error) { + p, err := NewSearchProvider(s, box) + if b, ok := p.(*Brave); ok { + b.endpoint = brave.URL + } + return p, err + } + if _, err := svc.Update(t.Context(), SearchSettingsPatch{Provider: ptr("brave"), APIKey: ptr(testBraveKey)}); err != nil { + t.Fatal(err) + } + _, err := svc.TestSearch(t.Context()) + if !errors.Is(err, ErrSearchRefused) { + t.Fatalf("TestSearch error = %v, want ErrSearchRefused", err) + } + if strings.Contains(err.Error(), testBraveKey) { + t.Errorf("TestSearch error holds the Brave key: %v", err) + } +} diff --git a/internal/server/login_test.go b/internal/server/login_test.go index 16e83d7..29f6257 100644 --- a/internal/server/login_test.go +++ b/internal/server/login_test.go @@ -83,7 +83,7 @@ func newLoginRouter(t *testing.T, issuer string) http.Handler { t.Fatal(err) } r, err := NewRouter(logger, "dev", trusted, flow, sessions, newTestSettings(t), - newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/media_test.go b/internal/server/media_test.go index 936d556..fab7903 100644 --- a/internal/server/media_test.go +++ b/internal/server/media_test.go @@ -51,7 +51,7 @@ func newMediaRouter(t *testing.T) (r http.Handler, id, orphan string) { t.Fatal(err) } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestRunner(t), + r, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestRunner(t), photos, fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) diff --git a/internal/server/origin_test.go b/internal/server/origin_test.go index bc235b9..19f5022 100644 --- a/internal/server/origin_test.go +++ b/internal/server/origin_test.go @@ -45,7 +45,7 @@ func newOriginRouter(t *testing.T, logs io.Writer) http.Handler { } logger := slog.New(forwarded.LogHandler(slog.NewJSONHandler(logs, nil))) r, err := NewRouter(logger, "dev", trusted, probeAuth{}, stubSessions{}, newTestSettings(t), - newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/recipesearch.go b/internal/server/recipesearch.go new file mode 100644 index 0000000..c216415 --- /dev/null +++ b/internal/server/recipesearch.go @@ -0,0 +1,70 @@ +package server + +import ( + "errors" + "log/slog" + "net/http" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" +) + +// searchTestResponse is the POST /settings/recipe-search/test body. +type searchTestResponse struct { + Query string `json:"query"` + Count int `json:"count"` +} + +func getRecipeSearchSettings(logger *slog.Logger, svc *recipes.SearchSettingsService) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + settings, err := svc.Get(r.Context()) + if err != nil { + logger.ErrorContext(r.Context(), "get recipe search settings", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + writeJSON(w, r, logger, http.StatusOK, settings) + } +} + +// patchRecipeSearchSettings takes the Brave API key in api_key; the +// response, like GET's, never holds it. +func patchRecipeSearchSettings(logger *slog.Logger, svc *recipes.SearchSettingsService) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + patch, err := decodePatch[recipes.SearchSettingsPatch](http.MaxBytesReader(w, r.Body, maxSettingsBody)) + if err != nil { + writeError(w, r, logger, http.StatusBadRequest, "invalid_request", err.Error()) + return + } + settings, err := svc.Update(r.Context(), patch) + var invalid *recipes.SearchSettingsError + if errors.As(err, &invalid) { + writeError(w, r, logger, http.StatusBadRequest, "invalid_settings", invalid.Error()) + return + } + if err != nil { + logger.ErrorContext(r.Context(), "update recipe search settings", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + return + } + writeJSON(w, r, logger, http.StatusOK, settings) + } +} + +// postRecipeSearchTest searches for recipes.SearchTestQuery with the saved +// settings. Recipe search not configured is 409; a search that fails is +// 502 with the provider's error, which never holds the API key. +func postRecipeSearchTest(logger *slog.Logger, svc *recipes.SearchSettingsService) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + count, err := svc.TestSearch(r.Context()) + if errors.Is(err, recipes.ErrSearchNotConfigured) { + writeError(w, r, logger, http.StatusConflict, "not_configured", "recipe search is not configured") + return + } + if err != nil { + logger.WarnContext(r.Context(), "recipe search test failed", "error", err) + writeError(w, r, logger, http.StatusBadGateway, "search_failed", err.Error()) + return + } + writeJSON(w, r, logger, http.StatusOK, searchTestResponse{Query: recipes.SearchTestQuery, Count: count}) + } +} diff --git a/internal/server/recipesearch_test.go b/internal/server/recipesearch_test.go new file mode 100644 index 0000000..384753d --- /dev/null +++ b/internal/server/recipesearch_test.go @@ -0,0 +1,124 @@ +package server + +import ( + "io" + "log/slog" + "net/http" + "net/http/httptest" + "strings" + "testing" + "testing/fstest" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" +) + +const ( + recipeSearchPath = "/api/v1/settings/recipe-search" + recipeSearchTestPath = "/api/v1/settings/recipe-search/test" + testBraveKey = "BSA-secret-brave-key-9f3c" +) + +// newRecipeSearchRouter returns a router serving search. +func newRecipeSearchRouter(t *testing.T, search *recipes.SearchSettingsService) http.Handler { + t.Helper() + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + r, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), search, newTestRunner(t), + newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + if err != nil { + t.Fatal(err) + } + return r +} + +func TestRecipeSearchSettingsAPI(t *testing.T) { + t.Parallel() + router := newRecipeSearchRouter(t, newTestSearch(t)) + var bodies []string + call := func(method, target, body string) (*httptest.ResponseRecorder, map[string]any) { + t.Helper() + var r io.Reader + if body != "" { + r = strings.NewReader(body) + } + rec, decoded := serve(t, router, signedIn(method, target, r)) + bodies = append(bodies, rec.Body.String()) + return rec, decoded + } + + rec, body := call(http.MethodGet, recipeSearchPath, "") + if rec.Code != http.StatusOK || body["provider"] != nil || body["api_key_set"] != false { + t.Errorf("GET unconfigured = %d %v", rec.Code, body) + } + rec, body = call(http.MethodPost, recipeSearchTestPath, "") + if rec.Code != http.StatusConflict || body["error"].(map[string]any)["code"] != "not_configured" { + t.Errorf("POST test unconfigured = %d %v", rec.Code, body) + } + + for _, bad := range []string{ + `{"provider":"google"}`, + `{"provider":"brave"}`, + `{"provider":"searxng","base_url":"not a url"}`, + `{"provider":"brave","api_key":null}`, + `{"provider":"brave","apikey":"x"}`, + } { + if rec, _ := call(http.MethodPatch, recipeSearchPath, bad); rec.Code != http.StatusBadRequest { + t.Errorf("PATCH %s = %d, want 400", bad, rec.Code) + } + } + + rec, body = call(http.MethodPatch, recipeSearchPath, `{"provider":"brave","api_key":"`+testBraveKey+`"}`) + if rec.Code != http.StatusOK || body["provider"] != "brave" || body["api_key_set"] != true || body["api_key_last4"] != "9f3c" { + t.Errorf("PATCH Brave = %d %v", rec.Code, body) + } + if _, ok := body["api_key"]; ok { + t.Errorf("PATCH response has an api_key field: %v", body) + } + rec, body = call(http.MethodGet, recipeSearchPath, "") + if rec.Code != http.StatusOK || body["provider"] != "brave" || body["api_key_last4"] != "9f3c" { + t.Errorf("GET Brave = %d %v", rec.Code, body) + } + + // SearXNG keeps the stored key; the test search goes to the instance. + searx := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{"results":[{"url":"https://a.example/1"},{"url":"https://b.example/2"},{"url":"https://c.example/3"}]}`)) + })) + t.Cleanup(searx.Close) + rec, body = call(http.MethodPatch, recipeSearchPath, `{"provider":"searxng","base_url":"`+searx.URL+`"}`) + if rec.Code != http.StatusOK || body["base_url"] != searx.URL || body["api_key_set"] != true { + t.Errorf("PATCH SearXNG = %d %v", rec.Code, body) + } + rec, body = call(http.MethodPost, recipeSearchTestPath, "") + if rec.Code != http.StatusOK || body["count"] != float64(3) || body["query"] != recipes.SearchTestQuery { + t.Errorf("POST test = %d %v, want count 3", rec.Code, body) + } + + searx.Close() + rec, body = call(http.MethodPost, recipeSearchTestPath, "") + if rec.Code != http.StatusBadGateway || body["error"].(map[string]any)["code"] != "search_failed" { + t.Errorf("POST test against a stopped instance = %d %v, want 502 search_failed", rec.Code, body) + } + + for _, b := range bodies { + if strings.Contains(b, testBraveKey) || strings.Contains(b, testBraveKey[:12]) { + t.Errorf("response holds the Brave key: %s", b) + } + } +} + +func TestRecipeSearchSettingsNeedCSRF(t *testing.T) { + t.Parallel() + router := newRecipeSearchRouter(t, newTestSearch(t)) + for _, method := range []string{http.MethodPatch, http.MethodPost} { + target := recipeSearchPath + if method == http.MethodPost { + target = recipeSearchTestPath + } + req := httptest.NewRequest(method, target, strings.NewReader(`{"provider":"brave","api_key":"`+testBraveKey+`"}`)) + req.AddCookie(validSessionCookie) + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Errorf("%s %s without CSRF token = %d, want 403", method, target, rec.Code) + } + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 8439dd0..84dc8ff 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -14,6 +14,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/frontend" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" ) // reservedPrefixes are owned by the API router and are never answered with the @@ -29,15 +30,19 @@ type RouteMounter interface { // has its client address and scheme resolved, honouring forwarded headers // only from a peer in trusted (see resolveOrigin). version is reported by // GET /health; auth mounts the /auth routes; sessions guards /api/v1 and -// /media; settings serves /api/v1/settings/household; runner serves +// /media; settings serves /api/v1/settings/household; search serves +// /api/v1/settings/recipe-search and its /test; runner serves // POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves // GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. // Requests to /api/v1 other than GET also need the session's CSRF token in CSRFHeader. -func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { +func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, auth RouteMounter, sessions SessionLookup, settings *household.SettingsService, search *recipes.SearchSettingsService, runner *jobs.Runner, photos PhotoFiles, assets fs.FS) (http.Handler, error) { if settings == nil { return nil, errors.New("server: nil household settings service") } + if search == nil { + return nil, errors.New("server: nil recipe search settings service") + } if runner == nil { return nil, errors.New("server: nil job runner") } @@ -72,6 +77,9 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Get("/me", me(logger)) apiV1.Get("/settings/household", getHouseholdSettings(logger, settings)) apiV1.Patch("/settings/household", patchHouseholdSettings(logger, settings)) + apiV1.Get("/settings/recipe-search", getRecipeSearchSettings(logger, search)) + apiV1.Patch("/settings/recipe-search", patchRecipeSearchSettings(logger, search)) + apiV1.Post("/settings/recipe-search/test", postRecipeSearchTest(logger, search)) apiV1.Post("/jobs", postJob(logger, runner)) apiV1.Get("/jobs/{id}", getJob(logger, runner)) media.Get("/photos/{name}", getPhoto(logger, photos)) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 9710b4c..4efead6 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -2,6 +2,8 @@ package server import ( "context" + "crypto/rand" + "encoding/base64" "encoding/json" "io" "log/slog" @@ -17,6 +19,7 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/jobs" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/secretbox" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" ) @@ -39,7 +42,7 @@ func newTestRouterWith(t *testing.T, version string, settings *household.Setting "index.html": {Data: []byte(fixtureIndex)}, "assets/index-AbC123.js": {Data: []byte(fixtureAsset)}, } - r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, runner, newTestPhotos(t), assets) + r, err := NewRouter(logger, version, nil, stubAuth{}, stubSessions{}, settings, newTestSearch(t), runner, newTestPhotos(t), assets) if err != nil { t.Fatal(err) } @@ -124,11 +127,37 @@ func newTestPhotos(t *testing.T) *recipes.PhotoCache { return photos } +// newTestSearch returns a recipe search settings service, with nothing +// configured, over a migrated temp database and a random secret box. +func newTestSearch(t *testing.T) *recipes.SearchSettingsService { + t.Helper() + key := make([]byte, secretbox.KeySize) + rand.Read(key) + box, err := secretbox.New(base64.StdEncoding.EncodeToString(key)) + if err != nil { + t.Fatal(err) + } + search, err := recipes.NewSearchSettingsService(newTestStore(t), box) + if err != nil { + t.Fatal(err) + } + return search +} + +func TestNewRouterRequiresSearchSettings(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + t.Fatal("NewRouter accepted a nil recipe search settings service") + } +} + func TestNewRouterRequiresPhotoFiles(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestRunner(t), nil, fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted nil photo files") } } @@ -137,7 +166,7 @@ func TestNewRouterRequiresSettings(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, nil, newTestSearch(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil settings service") } } @@ -146,7 +175,7 @@ func TestNewRouterRequiresJobRunner(t *testing.T) { t.Parallel() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { + if _, err := NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), nil, newTestPhotos(t), fstest.MapFS{"index.html": {}}); err == nil { t.Fatal("NewRouter accepted a nil job runner") } } diff --git a/internal/server/settings.go b/internal/server/settings.go index c29846a..b0d3b04 100644 --- a/internal/server/settings.go +++ b/internal/server/settings.go @@ -13,7 +13,8 @@ import ( "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/household" ) -// maxSettingsBody bounds a PATCH /settings/household request body. +// maxSettingsBody bounds a PATCH /settings/household or +// /settings/recipe-search request body. const maxSettingsBody = 16 << 10 // settingsPatch is the PATCH /settings/household body. Every field is @@ -52,7 +53,7 @@ func patchHouseholdSettings(logger *slog.Logger, svc *household.SettingsService) writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") return } - patch, err := decodeSettingsPatch(http.MaxBytesReader(w, r.Body, maxSettingsBody)) + patch, err := decodePatch[settingsPatch](http.MaxBytesReader(w, r.Body, maxSettingsBody)) if err != nil { writeError(w, r, logger, http.StatusBadRequest, "invalid_request", err.Error()) return @@ -72,37 +73,37 @@ func patchHouseholdSettings(logger *slog.Logger, svc *household.SettingsService) } } -// decodeSettingsPatch reads a single JSON object, refusing unknown fields, -// null values and trailing data. -func decodeSettingsPatch(body io.Reader) (settingsPatch, error) { +// decodePatch reads a single JSON object into a patch of type T, refusing +// unknown fields, null values and trailing data. +func decodePatch[T any](body io.Reader) (T, error) { + var patch T data, err := io.ReadAll(body) if err != nil { var tooLarge *http.MaxBytesError if errors.As(err, &tooLarge) { - return settingsPatch{}, fmt.Errorf("request body exceeds %d bytes", tooLarge.Limit) + return patch, fmt.Errorf("request body exceeds %d bytes", tooLarge.Limit) } - return settingsPatch{}, errors.New("cannot read request body") + return patch, errors.New("cannot read request body") } var fields map[string]json.RawMessage if err := json.Unmarshal(data, &fields); err != nil || fields == nil { - return settingsPatch{}, errors.New("request body must be a JSON object") + return patch, errors.New("request body must be a JSON object") } for name, raw := range fields { if bytes.Equal(raw, []byte("null")) { - return settingsPatch{}, fmt.Errorf("%s: must not be null", name) + return patch, fmt.Errorf("%s: must not be null", name) } } - var patch settingsPatch dec := json.NewDecoder(bytes.NewReader(data)) dec.DisallowUnknownFields() if err := dec.Decode(&patch); err != nil { var typeErr *json.UnmarshalTypeError if errors.As(err, &typeErr) { - return settingsPatch{}, fmt.Errorf("%s: want a JSON %s", typeErr.Field, typeErr.Type) + return patch, fmt.Errorf("%s: want a JSON %s", typeErr.Field, typeErr.Type) } - return settingsPatch{}, fmt.Errorf("invalid settings: %v", err) + return patch, fmt.Errorf("invalid settings: %v", err) } return patch, nil } diff --git a/internal/server/settings_test.go b/internal/server/settings_test.go index a0fe87c..e3fb3fb 100644 --- a/internal/server/settings_test.go +++ b/internal/server/settings_test.go @@ -216,7 +216,7 @@ func TestHouseholdSettingsWithRealSession(t *testing.T) { } logger := slog.New(slog.NewTextHandler(io.Discard, nil)) router, err := NewRouter(logger, "dev", nil, stubAuth{}, sessions, household.NewSettingsService(st), - newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + newTestSearch(t), newTestRunner(t), newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/store/migrate_test.go b/internal/store/migrate_test.go index fa10f79..6c00c51 100644 --- a/internal/store/migrate_test.go +++ b/internal/store/migrate_test.go @@ -175,8 +175,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding", got) } } @@ -196,8 +196,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { t.Fatal(err) } - if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status" { - t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status", got) + if got := versions(migrationRows(t, s)); got != "0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding" { + t.Errorf("versions = %q, want 0000_bootstrap,0001_init,0002_household_settings_updated_by,0003_jobs_status,0004_ai_provider_presets,0005_ai_provider_xai,0006_ai_runs_status,0007_recipe_grounding", got) } if !tableExists(t, s, "dinners") { t.Error("table from 0001_init was not created") diff --git a/migrations/0007_recipe_grounding.sql b/migrations/0007_recipe_grounding.sql new file mode 100644 index 0000000..928f1a3 --- /dev/null +++ b/migrations/0007_recipe_grounding.sql @@ -0,0 +1,18 @@ +-- 0007_recipe_grounding: a grounded recipe keeps what recipes.Ground +-- returns, so a stored source_url is reused without fetching or normalizing +-- it again: its cached photo, and normalize_recipe's method, difficulty and +-- minutes_total (prep to table; total_minutes stays the page's own JSON-LD +-- total). Nothing wrote recipes before Ground did, so the columns are added +-- nullable rather than with a default no recipe has; Ground always sets +-- photo_id, method and difficulty and refuses to reuse a row without them. +-- +-- recipe_search_settings gains api_key_last4, the last four characters of +-- the Brave API key, so the settings can show which key is stored without +-- ever returning it. + +ALTER TABLE recipes ADD COLUMN photo_id TEXT REFERENCES photos (id); +ALTER TABLE recipes ADD COLUMN method TEXT CHECK (method IN ('stovetop', 'oven', 'crockpot', 'grill', 'other')); +ALTER TABLE recipes ADD COLUMN difficulty TEXT CHECK (difficulty IN ('easy', 'medium', 'hard')); +ALTER TABLE recipes ADD COLUMN minutes_total INTEGER; + +ALTER TABLE recipe_search_settings ADD COLUMN api_key_last4 TEXT; diff --git a/migrations/queries/recipes.sql b/migrations/queries/recipes.sql index 3ed7a3c..ddb7808 100644 --- a/migrations/queries/recipes.sql +++ b/migrations/queries/recipes.sql @@ -5,10 +5,12 @@ -- DeleteRecipeIngredients/DeleteRecipeSteps then the Create queries. INSERT INTO recipes ( id, source_url, site_name, title, total_minutes, prep_minutes, - cook_minutes, yield, raw_jsonld, fetched_at, created_at, updated_at + cook_minutes, yield, raw_jsonld, photo_id, method, difficulty, + minutes_total, fetched_at, created_at, updated_at ) VALUES ( ?, ?, ?, ?, ?, ?, - ?, ?, ?, ?, ?, ? + ?, ?, ?, ?, ?, ?, + ?, ?, ?, ? ) ON CONFLICT (source_url) DO UPDATE SET site_name = excluded.site_name, @@ -18,6 +20,10 @@ ON CONFLICT (source_url) DO UPDATE SET cook_minutes = excluded.cook_minutes, yield = excluded.yield, raw_jsonld = excluded.raw_jsonld, + photo_id = excluded.photo_id, + method = excluded.method, + difficulty = excluded.difficulty, + minutes_total = excluded.minutes_total, fetched_at = excluded.fetched_at, updated_at = excluded.updated_at RETURNING *; diff --git a/migrations/queries/settings.sql b/migrations/queries/settings.sql index 171be84..6a1c6a8 100644 --- a/migrations/queries/settings.sql +++ b/migrations/queries/settings.sql @@ -70,14 +70,15 @@ WHERE id = 1; -- Creates the single row (id 1) or replaces every column except id and -- created_at. INSERT INTO recipe_search_settings ( - id, provider, base_url, api_key_ct, created_at, updated_at + id, provider, base_url, api_key_ct, api_key_last4, created_at, updated_at ) VALUES ( - 1, ?, ?, ?, ?, ? + 1, ?, ?, ?, ?, ?, ? ) ON CONFLICT (id) DO UPDATE SET provider = excluded.provider, base_url = excluded.base_url, api_key_ct = excluded.api_key_ct, + api_key_last4 = excluded.api_key_last4, updated_at = excluded.updated_at RETURNING *;