Merge the feature branch into task/supply-archive-frozen-names (Maestro 22f80eb6 refresh)
CI / ci (push) Successful in 8m31s
CI / ci (pull_request) Successful in 8m30s

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-11 15:39:04 -04:00
co-authored by Claude Opus 5.5
24 changed files with 656 additions and 220 deletions
+35 -17
View File
@@ -95,8 +95,8 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`).
| --- | --- | | --- | --- |
| `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. | | `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 (the page's JSON-LD), yield, raw_jsonld, fetched_at; and its grounding: photo_id, method, difficulty, minutes_total (normalized prep to table). | | `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_ingredients` | recipe_id, position (unique per recipe: the recipe order), raw_text, name, qty, unit, note. |
| `recipe_steps` | recipe_id, position, text. | | `recipe_steps` | recipe_id, position (unique per recipe: the recipe order), text. |
| `photos` | source_url, local_path, width, height, sha256, fetched_at. | | `photos` | source_url, local_path, width, height, sha256, fetched_at. |
| `dinners` | The meal record and its lifecycle: name, normalized_name, status (`suggested` / `quest_log` / `tried` / `regular` / `archived`), origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, recipe_url (a pasted recipe link awaiting grounding), photo_id, grounding_status (`pending` / `grounded` / `failed` / `manual`), grounding_error (why grounding failed), grounding_revision (raised whenever a dinner is renamed, so only the latest `ground_dinner` job may write it and no candidate offered before the rename grounds it), updated_by. | | `dinners` | The meal record and its lifecycle: name, normalized_name, status (`suggested` / `quest_log` / `tried` / `regular` / `archived`), origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, recipe_url (a pasted recipe link awaiting grounding), photo_id, grounding_status (`pending` / `grounded` / `failed` / `manual`), grounding_error (why grounding failed), grounding_revision (raised whenever a dinner is renamed, so only the latest `ground_dinner` job may write it and no candidate offered before the rename grounds it), updated_by. |
| `suggestion_batches` | trigger, status, ai_run_id. | | `suggestion_batches` | trigger, status, ai_run_id. |
@@ -121,6 +121,16 @@ Columns ending in `_ct` hold AES-256-GCM ciphertext (via `secretbox`).
`recipes` holds a fetched recipe page; `dinners` holds the household's meal `recipes` holds a fetched recipe page; `dinners` holds the household's meal
record and its lifecycle, optionally linked to a recipe and a photo. record and its lifecycle, optionally linked to a recipe and a photo.
Migration `0018_recipe_item_position_unique` makes position unique per
recipe and fails, so the service refuses to start, on a database already
holding a duplicate. Before deploying it, check the database; both queries
must return no rows:
```sql
SELECT recipe_id, position FROM recipe_ingredients GROUP BY 1, 2 HAVING count(*) > 1;
SELECT recipe_id, position FROM recipe_steps GROUP BY 1, 2 HAVING count(*) > 1;
```
## Dinner lifecycle ## Dinner lifecycle
``` ```
@@ -215,17 +225,17 @@ automatic** (D-021).
raw_text}]` and `steps` the step texts, both in recipe position order and raw_text}]` and `steps` the step texts, both in recipe position order and
empty (`[]`) with no recipe, as for a dinner whose grounding is empty (`[]`) with no recipe, as for a dinner whose grounding is
`pending` or `failed`; a recipe entered by hand has only `raw_text`. `pending` or `failed`; a recipe entered by hand has only `raw_text`.
Two items at one position leave recipe order unknown, so such a recipe `source` is `{site_name, url}` of the page the recipe was fetched from,
is 500 rather than shown out of order. `source` is `{site_name}` of the null for a manual recipe or none, as `dinners.SourceOf` gives it — the
page the recipe was fetched from, null for a manual recipe or none; it one rule for every fetched recipe's source, also used by idea results
has no `url`, as the page's link may carry a personal token. and recipe candidates. `url`
`site_name` is the name the page gives its site (publisher or is the page's link only when it is free of secrets (https, no user,
`og:site_name`) and null otherwise: a page naming no site is stored query or fragment), else null. `site_name` is the name the page gives
under its host, which may be the pasted link's — a token in a its site (publisher or `og:site_name`), else the link's host without
subdomain — or a redirect's, so any stored name that could be a host "www." when `url` is shown, else null. Extraction never stores a host
(the link's host, an IP address or a dotted host name, even a real as a site name, so a stored name is always the page's; migration 0020
name like "Food.com") is withheld. The detail screen shows "Recipe cleared the host-shaped names stored before that. The detail screen
from {site}" only when `site_name` is not null. `why_picked` is the reason of the shows "Recipe from {site}" only when `site_name` is not null. `why_picked` is the reason of the
suggestion that created the dinner — its pending or accepted one, as suggestion that created the dinner — its pending or accepted one, as
declining a suggestion archives its dinner — null for one no suggestion declining a suggestion archives its dinner — null for one no suggestion
created; a dinner with more than one such suggestion is 500, as nothing created; a dinner with more than one such suggestion is 500, as nothing
@@ -262,7 +272,8 @@ automatic** (D-021).
`GET /api/v1/dinners/{id}/recipe-candidates` `GET /api/v1/dinners/{id}/recipe-candidates`
searches for the dinner's name and returns `{candidates: [{candidate_id, searches for the dinner's name and returns `{candidates: [{candidate_id,
title, site_name, photo_url, minutes}]}`, at most 3, ranked as `Ground` title, site_name, photo_url, minutes}]}`, at most 3, ranked as `Ground`
ranks them; it fetches pages but downloads no photo and runs no model, ranks them, with `site_name` as the recipe detail's (`dinners.SourceOf`,
null when the site cannot be named without leaking the link); it fetches pages but downloads no photo and runs no model,
so `photo_url` is set only for a page already stored. `candidate_id` is so `photo_url` is set only for a page already stored. `candidate_id` is
an opaque id the server resolves to the page it last offered that dinner an opaque id the server resolves to the page it last offered that dinner
(kept in memory, so a restart means fetching the candidates again) at (kept in memory, so a restart means fetching the candidates again) at
@@ -319,15 +330,22 @@ automatic** (D-021).
suggestion with the name it is suggested under, the reason and suggestion with the name it is suggested under, the reason and
`cited_regular_ids` — unless, since the job read its input, a dinner `cited_regular_ids` — unless, since the job read its input, a dinner
it repeats was added, a suggestion it repeats passed, or a Regular it it repeats was added, a suggestion it repeats passed, or a Regular it
cites renamed or archived (checked in the storing transaction). The job cites renamed or archived (checked in the storing transaction). A
notifying batch also rechecks the Side quests it holds after each round
and lets go of those no longer eligible, so a later round grounds a
spare in place of each; only a change committed between the last
recheck and the store can leave it short of 5 with spares unused. The job
publishes `{batch_id, stage, suggestion_ids, dropped}` as it goes (`suggesting`, publishes `{batch_id, stage, suggestion_ids, dropped}` as it goes (`suggesting`,
`grounding`, `done`). It fails with `no_regulars`, `ai_failed` (the `grounding`, `done`). It fails with `no_regulars`, `ai_failed` (the
skill or `normalize_recipe` failed; no Side quest is made up instead, skill or `normalize_recipe` failed; no Side quest is made up instead,
D-017), `search_not_configured` or `search_failed`, and the batch is set D-017), `search_not_configured` or `search_failed`, and the batch is set
`failed`. A failed `scheduled` or `manual` batch has stored nothing; a `failed`. A `scheduled` or `manual` batch that fails before its final
store (the AI, grounding, or the store itself) has stored nothing; a
failed `replenish` batch's Side quests stored before the failure stay failed `replenish` batch's Side quests stored before the failure stay
in the deck. If in the deck. If
recording the batch's end fails, its job fails; the job runner retries recording the batch's end fails, its job fails, even after the final
store committed: a notifying batch's Side quests and their
`new_suggestions` then stay; the job runner retries
a job's final status write with backoff until it lands, each attempt a job's final status write with backoff until it lands, each attempt
bounded at 5 seconds; at shutdown an attempt that fails or times out is bounded at 5 seconds; at shutdown an attempt that fails or times out is
not retried, and the next start fails the job as interrupted. Recording not retried, and the next start fails the job as interrupted. Recording
+2 -2
View File
@@ -220,7 +220,7 @@ func (q *Queries) GetRecipeBySourceURL(ctx context.Context, sourceUrl string) (R
const listRecipeIngredients = `-- name: ListRecipeIngredients :many const listRecipeIngredients = `-- name: ListRecipeIngredients :many
SELECT id, recipe_id, position, raw_text, name, qty, unit, note, created_at, updated_at FROM recipe_ingredients SELECT id, recipe_id, position, raw_text, name, qty, unit, note, created_at, updated_at FROM recipe_ingredients
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id ORDER BY position
` `
func (q *Queries) ListRecipeIngredients(ctx context.Context, recipeID string) ([]RecipeIngredient, error) { func (q *Queries) ListRecipeIngredients(ctx context.Context, recipeID string) ([]RecipeIngredient, error) {
@@ -260,7 +260,7 @@ func (q *Queries) ListRecipeIngredients(ctx context.Context, recipeID string) ([
const listRecipeSteps = `-- name: ListRecipeSteps :many const listRecipeSteps = `-- name: ListRecipeSteps :many
SELECT id, recipe_id, position, text, created_at, updated_at FROM recipe_steps SELECT id, recipe_id, position, text, created_at, updated_at FROM recipe_steps
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id ORDER BY position
` `
func (q *Queries) ListRecipeSteps(ctx context.Context, recipeID string) ([]RecipeStep, error) { func (q *Queries) ListRecipeSteps(ctx context.Context, recipeID string) ([]RecipeStep, error) {
+36 -53
View File
@@ -58,14 +58,14 @@ type Ingredient struct {
RawText string `json:"raw_text"` RawText string `json:"raw_text"`
} }
// Source is the site a recipe was fetched from. SiteName is the name the // Source is the site a recipe was fetched from, as SourceOf gives it.
// page itself gives its site, and null when it gives none or the stored // SiteName is the name the page gives its site, else the host of URL when
// name could have come from a link (see pageSiteName). URL is the page's // URL is shown, else null. URL is the page's link only when it is
// link only when it is verifiably free of secrets (see safeSourceURL), and // verifiably free of secrets (see safeSourceURL), and null otherwise: a
// null otherwise: a link pasted or offered as a candidate may carry a // link pasted or offered as a candidate may carry a personal token, so a
// personal token, so a client links the credit only when URL is not null. // client links the credit only when URL is not null. Source is set for
// Source is set for every fetched recipe, so a client always credits it: // every fetched recipe, so a client always credits it: by SiteName, else
// by SiteName, else by URL's host, else without naming the site. // without naming the site.
type Source struct { type Source struct {
SiteName *string `json:"site_name"` SiteName *string `json:"site_name"`
URL *string `json:"url"` URL *string `json:"url"`
@@ -157,10 +157,7 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail)
if err != nil { if err != nil {
return fmt.Errorf("ingredients: %w", err) return fmt.Errorf("ingredients: %w", err)
} }
for i, ing := range ings { for _, ing := range ings {
if i > 0 && ing.Position == ings[i-1].Position {
return duplicatePosition("ingredient", ing.Position)
}
d.Ingredients = append(d.Ingredients, Ingredient{ d.Ingredients = append(d.Ingredients, Ingredient{
Name: nullString(ing.Name), Name: nullString(ing.Name),
Qty: nullString(ing.Qty), Qty: nullString(ing.Qty),
@@ -173,52 +170,46 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail)
if err != nil { if err != nil {
return fmt.Errorf("steps: %w", err) return fmt.Errorf("steps: %w", err)
} }
for i, step := range steps { for _, step := range steps {
if i > 0 && step.Position == steps[i-1].Position {
return duplicatePosition("step", step.Position)
}
d.Steps = append(d.Steps, step.Text) d.Steps = append(d.Steps, step.Text)
} }
return nil return nil
} }
// SourceOf is the Source of a recipe fetched from sourceURL whose stored // SourceOf is the Source of a recipe fetched from sourceURL whose stored
// site name is siteName: the name only when the page gave it (see // site name is siteName, the name its page gave (recipes.Recipe.SiteName),
// pageSiteName) and the link only when it is free of secrets (see // null when the page named none. It is the one rule by which a fetched
// safeSourceURL). It is the one way a fetched recipe's source reaches a // recipe's source reaches a client — the recipe detail, an idea and a
// client. A sourceURL that does not parse is an error, whether or not a // recipe candidate offered for a dinner alike:
// site name is stored. //
// - the link is shown only when it is free of secrets (see
// safeSourceURL);
// - a site name the page gave is shown as it stands, as the page
// publishes it to everyone who reads it;
// - otherwise the site is named by the link's host without a leading
// "www.", and only when the link itself is shown, so the name tells
// nothing the shown link does not; else it is null.
//
// A host is never stored as a site name, so a stored name is always the
// page's: migration 0020 cleared the names stored before that, when a
// page naming no site was stored under its host. A sourceURL that does not
// parse is an error, whether or not a site name is stored.
func SourceOf(siteName sql.NullString, sourceURL string) (Source, error) { func SourceOf(siteName sql.NullString, sourceURL string) (Source, error) {
link, err := url.Parse(sourceURL) link, err := url.Parse(sourceURL)
if err != nil { if err != nil {
// Not wrapped: url.Parse's error quotes the link. // Not wrapped: url.Parse's error quotes the link.
return Source{}, errors.New("source url does not parse") return Source{}, errors.New("source url does not parse")
} }
return Source{SiteName: pageSiteName(siteName, link), URL: safeSourceURL(sourceURL)}, nil src := Source{URL: safeSourceURL(sourceURL)}
} switch {
case siteName.Valid && siteName.String != "":
// pageSiteName is the stored site name of a recipe fetched from link name := siteName.String
// when it is one the page gave, else nil. A page that names no site is src.SiteName = &name
// stored under recipes.HostSiteName of the URL it was served from: the case src.URL != nil:
// pasted link's host, which may hold a personal token (a private host := strings.TrimPrefix(link.Hostname(), "www.")
// subdomain), or that of a page the link redirected to, which is not src.SiteName = &host
// stored. Nothing records which a stored name is, so any name that could
// be a host — the link's own host, an IP address, or a dotted host name —
// is treated as one and withheld. A site that names itself as a domain,
// such as "Food.com", loses its name with them: a credit that does not
// name the site is better than one that leaks a link.
func pageSiteName(stored sql.NullString, link *url.URL) *string {
if !stored.Valid {
return nil
} }
name := stored.String return src, nil
if strings.EqualFold(name, recipes.HostSiteName(link)) || net.ParseIP(name) != nil {
return nil
}
if u, err := url.Parse("https://" + name + "/"); err == nil && u.Host == name && strings.Contains(name, ".") {
return nil
}
return &name
} }
// safeSourceURL is sourceURL when it is verifiably free of secrets — an // safeSourceURL is sourceURL when it is verifiably free of secrets — an
@@ -283,11 +274,3 @@ func browserHost(u *url.URL) bool {
} }
return true return true
} }
// duplicatePosition is the error for two recipe items of kind at position.
// Position is the only record of recipe order, and the id that breaks the
// tie says nothing about it, so a recipe with a duplicate is refused rather
// than shown in an order nobody can trust.
func duplicatePosition(kind string, position int64) error {
return fmt.Errorf("two %ss at position %d: recipe order is unknown", kind, position)
}
+27 -22
View File
@@ -76,35 +76,40 @@ func TestCreateRegularNeverNamesFromLink(t *testing.T) {
check("duplicate error", dup.Error()) check("duplicate error", dup.Error())
} }
// TestPageSiteNameWithholdsHosts: a stored site name that could be a host — // TestSourceOfSiteName: a site name the page gave is shown as it stands,
// the link's own, in any case and without "www.", an IP address or a dotted // even one shaped like a host and even over a link that is withheld; a
// host name — is withheld; a name the page gave, dotless or with spaces, is // recipe whose page named none is named by its link's host without "www."
// kept. // only when the link itself is shown, and is unnamed otherwise.
func TestPageSiteNameWithholdsHosts(t *testing.T) { func TestSourceOfSiteName(t *testing.T) {
t.Parallel() t.Parallel()
const link = "https://www.TokenHost/recipes/stew" const (
ordinary = "https://www.Kitchen.example/recipes/stew"
token = "https://private-token.example.com/recipes/stew?access_token=abc123"
)
for _, tc := range []struct { for _, tc := range []struct {
stored string stored string
valid bool valid bool
want *string link string
want, wantURL *string
}{ }{
{"", false, nil}, {"Delish", true, ordinary, ptr("Delish"), ptr(ordinary)},
{"tokenhost", true, nil}, {"Food.com", true, ordinary, ptr("Food.com"), ptr(ordinary)},
{"TOKENHOST", true, nil}, {"Bon Appétit", true, token, ptr("Bon Appétit"), nil},
{"private-token.example.com", true, nil}, {"", false, ordinary, ptr("Kitchen.example"), ptr(ordinary)},
{"Food.com", true, nil}, {"", true, ordinary, ptr("Kitchen.example"), ptr(ordinary)},
{"203.0.113.7", true, nil}, {"", false, token, nil, nil},
{"2001:db8::1", true, nil}, {"", false, "https://203.0.113.7/stew", ptr("203.0.113.7"), ptr("https://203.0.113.7/stew")},
{"Delish", true, ptr("Delish")}, {"", false, "http://kitchen.example/stew", nil, nil},
{"Bon Appétit", true, ptr("Bon Appétit")},
{"Mr. Food Test Kitchen", true, ptr("Mr. Food Test Kitchen")},
} { } {
src, err := dinners.SourceOf(sql.NullString{String: tc.stored, Valid: tc.valid}, link) src, err := dinners.SourceOf(sql.NullString{String: tc.stored, Valid: tc.valid}, tc.link)
if err != nil { if err != nil {
t.Fatalf("SourceOf(%q): %v", tc.stored, err) t.Fatalf("SourceOf(%q, %q): %v", tc.stored, tc.link, err)
} }
if got := src.SiteName; (got == nil) != (tc.want == nil) || (got != nil && *got != *tc.want) { if got := src.SiteName; (got == nil) != (tc.want == nil) || (got != nil && *got != *tc.want) {
t.Errorf("SourceOf(%q) site name = %v, want %v", tc.stored, deref(got), deref(tc.want)) t.Errorf("SourceOf(%q, %q) site name = %v, want %v", tc.stored, tc.link, deref(got), deref(tc.want))
}
if got := src.URL; (got == nil) != (tc.wantURL == nil) || (got != nil && *got != *tc.wantURL) {
t.Errorf("SourceOf(%q, %q) url = %v, want %v", tc.stored, tc.link, deref(got), deref(tc.wantURL))
} }
} }
// A link that does not parse is an error with or without a site name. // A link that does not parse is an error with or without a site name.
+8 -2
View File
@@ -304,7 +304,9 @@ type Candidate struct {
// may carry a token. // may carry a token.
CandidateID string `json:"candidate_id"` CandidateID string `json:"candidate_id"`
Title string `json:"title"` Title string `json:"title"`
SiteName string `json:"site_name"` // SiteName is the page's site as SourceOf names it, null when it
// cannot be named without leaking the URL.
SiteName *string `json:"site_name"`
// PhotoURL is the cached photo of a page already grounded, else null. // PhotoURL is the cached photo of a page already grounded, else null.
PhotoURL *string `json:"photo_url"` PhotoURL *string `json:"photo_url"`
Minutes *int `json:"minutes"` Minutes *int `json:"minutes"`
@@ -367,9 +369,13 @@ func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error
out := make([]Candidate, len(offers)) out := make([]Candidate, len(offers))
urls := make(map[string]string, len(offers)) urls := make(map[string]string, len(offers))
for i, o := range offers { for i, o := range offers {
src, err := SourceOf(sql.NullString{String: o.SiteName, Valid: o.SiteName != ""}, o.URL)
if err != nil {
return nil, fmt.Errorf("%w: dinner %s: candidate %d: %w", ErrCandidatesFailed, id, i+1, err)
}
cid := s.candidateID(row.ID, o.URL) cid := s.candidateID(row.ID, o.URL)
urls[cid] = o.URL urls[cid] = o.URL
out[i] = Candidate{CandidateID: cid, Title: o.Title, SiteName: o.SiteName, Minutes: o.Minutes} out[i] = Candidate{CandidateID: cid, Title: o.Title, SiteName: src.SiteName, Minutes: o.Minutes}
if o.PhotoID != "" { if o.PhotoID != "" {
u := PhotoURL(o.PhotoID) u := PhotoURL(o.PhotoID)
out[i].PhotoURL = &u out[i].PhotoURL = &u
+2 -2
View File
@@ -718,11 +718,11 @@ func TestCandidates(t *testing.T) {
} }
first, second := got[0], got[1] first, second := got[0], got[1]
wantPhoto := dinners.PhotoURL(e.dinner(t, c.ID).PhotoID.String) wantPhoto := dinners.PhotoURL(e.dinner(t, c.ID).PhotoID.String)
if first.Title != "Chicken Curry" || first.SiteName != "Curry House" || if first.Title != "Chicken Curry" || deref(first.SiteName) != "Curry House" ||
first.PhotoURL == nil || *first.PhotoURL != wantPhoto || first.Minutes == nil || *first.Minutes != 40 { first.PhotoURL == nil || *first.PhotoURL != wantPhoto || first.Minutes == nil || *first.Minutes != 40 {
t.Errorf("first = %+v, want the stored curryA with photo %s", first, wantPhoto) t.Errorf("first = %+v, want the stored curryA with photo %s", first, wantPhoto)
} }
if second.Title != "Easy Weeknight Curry" || second.SiteName != "B Kitchen" || if second.Title != "Easy Weeknight Curry" || deref(second.SiteName) != "B Kitchen" ||
second.PhotoURL != nil || second.Minutes == nil || *second.Minutes != 25 { second.PhotoURL != nil || second.Minutes == nil || *second.Minutes != 25 {
t.Errorf("second = %+v, want curryB without a photo", second) t.Errorf("second = %+v, want curryB without a photo", second)
} }
+2 -1
View File
@@ -486,7 +486,8 @@ func sourceOf(gr recipes.Grounded) (dinners.Source, error) {
// link sourceURL and site name siteName, as a client may see it (see // link sourceURL and site name siteName, as a client may see it (see
// dinners.SourceOf). A result stored now already holds it, and one stored // dinners.SourceOf). A result stored now already holds it, and one stored
// before sources were made safe holds the grounded recipe's own, with "" // before sources were made safe holds the grounded recipe's own, with ""
// for no site name, as sourceOf reads it; a null link, which only a result // for no site name, as sourceOf reads it, less any name that could be a
// host, which migration 0020 cleared; a null link, which only a result
// stored now holds, is checked as "". // stored now holds, is checked as "".
func shownSource(sourceURL, siteName *string) (dinners.Source, error) { func shownSource(sourceURL, siteName *string) (dinners.Source, error) {
var link string var link string
+7 -14
View File
@@ -28,8 +28,10 @@ var (
type Recipe struct { type Recipe struct {
// Title is the recipe's name. // Title is the recipe's name.
Title string Title string
// SiteName is the publisher's name, else the page's og:site_name, else // SiteName is the name the page gives its site: the publisher's name,
// HostSiteName of the page's URL. // else the page's og:site_name; empty when it names none. It is never
// taken from the page's URL, whose host may carry a personal token, so
// a stored site name is always one a page gave (see dinners.SourceOf).
SiteName string SiteName string
// ImageURL is the largest image, absolute; empty when the recipe has // ImageURL is the largest image, absolute; empty when the recipe has
// none. // none.
@@ -173,7 +175,7 @@ func recipeFrom(n map[string]any, g graph, base *url.URL, ogSiteName string) (Re
if r.Title = cleanText(name); !ok || r.Title == "" { if r.Title = cleanText(name); !ok || r.Title == "" {
return Recipe{}, errors.New("name is missing") return Recipe{}, errors.New("name is missing")
} }
if r.SiteName, err = siteName(n["publisher"], g, ogSiteName, base); err != nil { if r.SiteName, err = siteName(n["publisher"], g, ogSiteName); err != nil {
return Recipe{}, err return Recipe{}, err
} }
if r.ImageURL, err = imageURL(n["image"], g, base); err != nil { if r.ImageURL, err = imageURL(n["image"], g, base); err != nil {
@@ -213,7 +215,7 @@ func cleanText(s string) string {
return strings.TrimSpace(html.UnescapeString(s)) return strings.TrimSpace(html.UnescapeString(s))
} }
func siteName(publisher any, g graph, ogSiteName string, base *url.URL) (string, error) { func siteName(publisher any, g graph, ogSiteName string) (string, error) {
pubs, ok := publisher.([]any) pubs, ok := publisher.([]any)
if !ok { if !ok {
pubs = []any{publisher} pubs = []any{publisher}
@@ -229,16 +231,7 @@ func siteName(publisher any, g graph, ogSiteName string, base *url.URL) (string,
} }
} }
} }
if ogSiteName != "" { return ogSiteName, nil
return ogSiteName, nil
}
return HostSiteName(base), nil
}
// HostSiteName is the site name of a page that names none: its host
// without a leading "www.".
func HostSiteName(u *url.URL) string {
return strings.TrimPrefix(u.Hostname(), "www.")
} }
// imageURL picks the largest of the recipe's images by width × height; // imageURL picks the largest of the recipe's images by width × height;
+2 -2
View File
@@ -251,10 +251,10 @@ func TestExtractShapes(t *testing.T) {
}, },
}, },
{ {
name: "site name from host without publisher or og:site_name", name: "no site name without publisher or og:site_name, never the host",
html: page("", `{"@type":"Recipe","name":"Stew"}`), html: page("", `{"@type":"Recipe","name":"Stew"}`),
check: func(t *testing.T, r Recipe) { check: func(t *testing.T, r Recipe) {
if r.SiteName != "example.com" { if r.SiteName != "" {
t.Errorf("SiteName = %q", r.SiteName) t.Errorf("SiteName = %q", r.SiteName)
} }
}, },
+7 -3
View File
@@ -65,7 +65,9 @@ type Grounded struct {
// Difficulty is easy, medium or hard. // Difficulty is easy, medium or hard.
Difficulty string `json:"difficulty"` Difficulty string `json:"difficulty"`
SourceURL string `json:"source_url"` SourceURL string `json:"source_url"`
SiteName string `json:"site_name"` // SiteName is the name the page gives its site, "" when it names none
// (see Recipe.SiteName).
SiteName string `json:"site_name"`
} }
// SearchSource returns the household's current search provider; // SearchSource returns the household's current search provider;
@@ -200,8 +202,10 @@ func (g *Grounder) searchRanked(ctx context.Context, dishName, query string) (ra
// Offer is a ranked candidate offered for the household to ground a dish // Offer is a ranked candidate offered for the household to ground a dish
// in, with GroundURL on its URL. // in, with GroundURL on its URL.
type Offer struct { type Offer struct {
URL string URL string
Title string Title string
// SiteName is the name the page gives its site, "" when it names none
// (see Recipe.SiteName).
SiteName string SiteName string
// PhotoID is the cached photo of a candidate already stored, "" for // PhotoID is the cached photo of a candidate already stored, "" for
// one that is not: a candidate's photo is cached only when it grounds. // one that is not: a candidate's photo is cached only when it grounds.
+75 -50
View File
@@ -3,6 +3,7 @@ package server
import ( import (
"database/sql" "database/sql"
"encoding/json" "encoding/json"
"io/fs"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"os" "os"
@@ -14,6 +15,7 @@ import (
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes/recipestest"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations"
) )
// getDetail answers GET /dinners/{id} for id, failing unless it is 200, and // getDetail answers GET /dinners/{id} for id, failing unless it is 200, and
@@ -70,8 +72,8 @@ func TestGetDinnerGroundedSideQuest(t *testing.T) {
} }
for _, stmt := range []string{ for _, stmt := range []string{
`UPDATE dinners SET status = 'quest_log', origin = 'side_quest', is_favorite = 1 WHERE id = ?`, `UPDATE dinners SET status = 'quest_log', origin = 'side_quest', is_favorite = 1 WHERE id = ?`,
`UPDATE recipe_ingredients SET position = 1 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`, `UPDATE recipe_ingredients SET position = 9 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`,
`UPDATE recipe_steps SET position = 1 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`, `UPDATE recipe_steps SET position = 9 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`,
} { } {
if _, err := e.st.Writer.Exec(stmt, id); err != nil { if _, err := e.st.Writer.Exec(stmt, id); err != nil {
t.Fatal(err) t.Fatal(err)
@@ -328,14 +330,33 @@ func TestGetDinnerSourceURLOnlyWhenSafe(t *testing.T) {
} }
} }
// TestGetDinnerSiteNameNeverFromLink: a page that names no site is stored // siteNameMigration is migration 0020, which clears the host-shaped site
// under its host, so a recipe grounded in a pasted link whose host carries a // names stored before extraction stopped storing hosts.
// token has site_name null and nothing from the link in its detail; a page const siteNameMigration = "0020_site_name_from_page.sql"
// that names its site keeps that name, even on a token-bearing host; and a
// stored name shaped like another host, as a redirect leaves, is withheld. // applySiteNameMigration runs siteNameMigration again over rows a test
// The links carry a query, so source.url is withheld too and nothing of the // stored as the code before it did, as upgrading a database holding them
// link may appear. // does.
func TestGetDinnerSiteNameNeverFromLink(t *testing.T) { func applySiteNameMigration(t *testing.T, st *store.Store) {
t.Helper()
body, err := fs.ReadFile(migrations.FS, siteNameMigration)
if err != nil {
t.Fatal(err)
}
if _, err := st.Writer.ExecContext(t.Context(), string(body)); err != nil {
t.Fatalf("%s: %v", siteNameMigration, err)
}
}
// TestGetDinnerSiteNameProvenance: extraction stores only a site name the
// page gave, never the URL's host, and the detail applies one rule to what
// is stored. A name from page metadata is shown even over a token-bearing
// link; a page naming no site stores no name, and is named by its link's
// host only when the link is shown — so over a link with a token nothing
// of the link reaches the client. A name stored before extraction stopped
// storing hosts is read by the same rule once migration 0020 has cleared
// it if it could be a host.
func TestGetDinnerSiteNameProvenance(t *testing.T) {
t.Parallel() t.Parallel()
e := newDinnersEnv(t) e := newDinnersEnv(t)
const ( const (
@@ -343,11 +364,14 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
namedToken = "named-token-def456" namedToken = "named-token-def456"
bare = "https://" + bareToken + ".example.com/recipe?print=1" bare = "https://" + bareToken + ".example.com/recipe?print=1"
named = "https://www." + namedToken + ".example.com/recipe?print=1" named = "https://www." + namedToken + ".example.com/recipe?print=1"
ordinary = "https://www.kitchen.example/recipes/curry"
) )
e.pages.Bodies[bare] = recipestest.RecipePage("Chicken Curry", "", "https://a.example/curry.jpg", 40) e.pages.Bodies[bare] = recipestest.RecipePage("Chicken Curry", "", "https://a.example/curry.jpg", 40)
e.pages.Bodies[named] = recipestest.RecipePage("Weeknight Curry", "Curry House", "https://b.example/curry.jpg", 25) e.pages.Bodies[named] = recipestest.RecipePage("Weeknight Curry", "Curry House", "https://b.example/curry.jpg", 25)
e.pages.Bodies[ordinary] = recipestest.RecipePage("Green Curry", "", "https://c.example/curry.jpg", 30)
e.images.JPEGs["https://a.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 1) e.images.JPEGs["https://a.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 1)
e.images.JPEGs["https://b.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 2) e.images.JPEGs["https://b.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 2)
e.images.JPEGs["https://c.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 3)
ground := func(dish, page string) (id, recipeID string) { ground := func(dish, page string) (id, recipeID string) {
t.Helper() t.Helper()
id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending
@@ -360,15 +384,18 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
} }
return id, recipeID return id, recipeID
} }
storedSite := func(recipeID string) string { storedSite := func(recipeID string) any {
t.Helper() t.Helper()
var site string var site sql.NullString
if err := e.st.Reader.QueryRow(`SELECT site_name FROM recipes WHERE id = ?`, recipeID).Scan(&site); err != nil { if err := e.st.Reader.QueryRow(`SELECT site_name FROM recipes WHERE id = ?`, recipeID).Scan(&site); err != nil {
t.Fatal(err) t.Fatal(err)
} }
return site if !site.Valid {
return nil
}
return site.String
} }
check := func(what, id string, leaks []string, want any) { check := func(what, id string, leaks []string, want map[string]any) {
t.Helper() t.Helper()
rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "") rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
if rec.Code != http.StatusOK { if rec.Code != http.StatusOK {
@@ -380,28 +407,48 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
} }
} }
raw, _ := getDetail(t, e.h, id) raw, _ := getDetail(t, e.h, id)
if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": want, "url": nil}) { if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, want) {
t.Errorf("source of %s = %v, want site_name %v and url null", what, raw["source"], want) t.Errorf("source of %s = %v, want %v", what, raw["source"], want)
} }
} }
bareID, bareRecipe := ground("Chicken curry", bare)
if got := storedSite(bareRecipe); got != bareToken+".example.com" {
t.Fatalf("stored site_name = %q, want the link's host", got)
}
check("a page naming no site", bareID, []string{bare, bareToken, "example.com"}, nil)
namedID, namedRecipe := ground("Weeknight curry", named) namedID, namedRecipe := ground("Weeknight curry", named)
if got := storedSite(namedRecipe); got != "Curry House" { if got := storedSite(namedRecipe); got != "Curry House" {
t.Fatalf("stored site_name = %q, want the page's", got) t.Fatalf("stored site_name = %v, want the page's", got)
} }
check("a page naming its site", namedID, []string{named, namedToken, "example.com"}, "Curry House") check("a metadata-derived name over a token-bearing link", namedID, []string{named, namedToken, "example.com"},
map[string]any{"site_name": "Curry House", "url": nil})
const redirectHost = "redirect-token-ghi789.example.net" bareID, bareRecipe := ground("Chicken curry", bare)
if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, redirectHost, namedRecipe); err != nil { if got := storedSite(bareRecipe); got != nil {
t.Fatal(err) t.Fatalf("stored site_name = %v, want null: the page names no site", got)
} }
check("a page named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken}, nil) check("a hostname-derived name over a token-bearing link", bareID, []string{bare, bareToken, "example.com"},
map[string]any{"site_name": nil, "url": nil})
ordinaryID, ordinaryRecipe := ground("Green curry", ordinary)
if got := storedSite(ordinaryRecipe); got != nil {
t.Fatalf("stored site_name = %v, want null: the page names no site", got)
}
check("a hostname-derived name over an ordinary link", ordinaryID, nil,
map[string]any{"site_name": "kitchen.example", "url": ordinary})
// Rows as extraction stored them before it stopped storing hosts: the
// token-bearing link's own host, and a redirect's host under a page
// that named its site, are cleared by 0020; a page's own name stays.
const redirectHost = "redirect-token-ghi789.example.net"
for recipeID, site := range map[string]string{bareRecipe: bareToken + ".example.com", namedRecipe: redirectHost, ordinaryRecipe: "Green Kitchen"} {
if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, site, recipeID); err != nil {
t.Fatal(err)
}
}
applySiteNameMigration(t, e.st)
check("a row stored before, named by its token-bearing link's host", bareID, []string{bare, bareToken, "example.com"},
map[string]any{"site_name": nil, "url": nil})
check("a row stored before, named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken},
map[string]any{"site_name": nil, "url": nil})
check("a row stored before, named by its page", ordinaryID, nil,
map[string]any{"site_name": "Green Kitchen", "url": ordinary})
} }
// TestGetDinnerWhyPickedIsCreatingSuggestion: why_picked is the reason of // TestGetDinnerWhyPickedIsCreatingSuggestion: why_picked is the reason of
@@ -445,25 +492,3 @@ func TestGetDinnerWhyPickedIsCreatingSuggestion(t *testing.T) {
t.Errorf("why_picked = %v, want %q", got.WhyPicked, reason) t.Errorf("why_picked = %v, want %q", got.WhyPicked, reason)
} }
} }
// TestGetDinnerDuplicatePositionFails: a recipe with two ingredients or two
// steps at one position has no order anyone can trust, so its detail is a
// 500 rather than the recipe in some order.
func TestGetDinnerDuplicatePositionFails(t *testing.T) {
t.Parallel()
e := newDinnersEnv(t)
for _, table := range []string{"recipe_ingredients", "recipe_steps"} {
id := createDinner(t, e.h, `{"name":"Family chili `+table+`"}`)
if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground",
`{"manual":{"ingredients":["1 lb beef","1 can beans"],"steps":["Brown.","Simmer."],"minutes":240,"method":"crockpot"}}`); rec.Code != http.StatusOK {
t.Fatalf("POST ground = %d: %s", rec.Code, rec.Body)
}
if _, err := e.st.Writer.Exec(`UPDATE `+table+` SET position = 0 WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`, id); err != nil {
t.Fatal(err)
}
rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
if rec.Code != http.StatusInternalServerError || strings.Contains(rec.Body.String(), "Simmer.") || strings.Contains(rec.Body.String(), "1 lb beef") {
t.Errorf("GET with duplicate %s positions = %d: %s, want 500 and no recipe", table, rec.Code, rec.Body)
}
}
}
+7 -6
View File
@@ -72,11 +72,11 @@ func getDinners(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
// getDinner answers GET /dinners/{id} with the dinner, its recipe and the // getDinner answers GET /dinners/{id} with the dinner, its recipe and the
// reason it was suggested. An archived dinner is 404, like an unknown one, // reason it was suggested. An archived dinner is 404, like an unknown one,
// not a 200 with its status: it has no detail to show. source never has the // not a 200 with its status: it has no detail to show. source is as
// recipe page's link, and source.site_name is null unless the page itself // dinners.SourceOf gives it: the recipe page's link only when it is free of
// named its site — a name that could be a host, the link's or a redirect's, // secrets, and site_name the name the page gave its site, else the shown
// is withheld (dinners.Source) — so the detail screen shows "Recipe from // link's host, else null — so the detail screen shows "Recipe from {site}"
// {site}" only when site_name is not null. // only when site_name is not null.
func getDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { func getDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
d, err := svc.Get(r.Context(), chi.URLParam(r, "id")) d, err := svc.Get(r.Context(), chi.URLParam(r, "id"))
@@ -134,7 +134,8 @@ func patchDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
} }
// getDinnerCandidates answers GET /dinners/{id}/recipe-candidates with up // getDinnerCandidates answers GET /dinners/{id}/recipe-candidates with up
// to dinners.MaxCandidates recipe pages for the dinner's name. // to dinners.MaxCandidates recipe pages for the dinner's name, each
// site_name as dinners.SourceOf gives it.
func getDinnerCandidates(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc { func getDinnerCandidates(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) {
list, err := svc.Candidates(r.Context(), chi.URLParam(r, "id")) list, err := svc.Candidates(r.Context(), chi.URLParam(r, "id"))
+45
View File
@@ -184,6 +184,51 @@ func TestGetRecipeCandidates(t *testing.T) {
} }
} }
// TestGetRecipeCandidatesSiteNames: a candidate's site_name follows the
// recipe detail's rule (dinners.SourceOf), not a hostname fallback of its
// own: a name the page gave is offered as it stands, even over a link with
// a token; a page naming no site is named by its link's host only when
// the link is free of secrets, and is null over a token-bearing link, so
// nothing of that link reaches the client.
func TestGetRecipeCandidatesSiteNames(t *testing.T) {
t.Parallel()
e := newDinnersEnv(t)
const (
token = "offer-token-abc123"
named = "https://named.example/curry?access_token=" + token
bare = "https://" + token + ".example.com/curry?print=1"
ordinary = "https://www.kitchen.example/recipes/curry"
)
e.search.Results["Chicken curry"] = []recipes.Candidate{{URL: named}, {URL: bare}, {URL: ordinary}}
e.pages.Bodies[named] = recipestest.RecipePage("Named Curry", "Curry House", "https://a.example/curry.jpg", 40)
e.pages.Bodies[bare] = recipestest.RecipePage("Bare Curry", "", "https://b.example/curry.jpg", 30)
e.pages.Bodies[ordinary] = recipestest.RecipePage("Ordinary Curry", "", "https://c.example/curry.jpg", 25)
id := createDinner(t, e.h, `{"name":"Chicken curry"}`) // runner not started: stays pending
rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "")
if rec.Code != http.StatusOK {
t.Fatalf("GET = %d: %s", rec.Code, rec.Body)
}
for _, leak := range []string{token, "access_token", "example.com"} {
if strings.Contains(rec.Body.String(), leak) {
t.Errorf("candidates contain %q: %s", leak, rec.Body)
}
}
want := map[string]any{"Named Curry": "Curry House", "Bare Curry": nil, "Ordinary Curry": "kitchen.example"}
list, _ := body["candidates"].([]any)
if len(list) != len(want) {
t.Fatalf("candidates = %v, want %d", body, len(want))
}
for _, c := range list {
c := c.(map[string]any)
title, _ := c["title"].(string)
site, ok := c["site_name"]
if w, known := want[title]; !known || !ok || site != w {
t.Errorf("candidate %q site_name = %#v (present %v), want %#v", title, site, ok, w)
}
}
}
func TestGetRecipeCandidatesSearchFails(t *testing.T) { func TestGetRecipeCandidatesSearchFails(t *testing.T) {
t.Parallel() t.Parallel()
e := newDinnersEnv(t) e := newDinnersEnv(t)
+9 -5
View File
@@ -16,7 +16,7 @@ import (
) )
// sourceCase is a recipe page an idea grounds in: its link and the site // sourceCase is a recipe page an idea grounds in: its link and the site
// name its page gives (empty for none, so the host is stored instead). // name its page gives (empty for none, so no name is stored).
type sourceCase struct { type sourceCase struct {
dish, link, site string dish, link, site string
// want is the idea's source_url and site_name; nil is null. // want is the idea's source_url and site_name; nil is null.
@@ -33,7 +33,7 @@ const (
// sourceCases are a clean link, credited by its page's name, and links // sourceCases are a clean link, credited by its page's name, and links
// carrying a token in the query and the userinfo whose pages name no site, // carrying a token in the query and the userinfo whose pages name no site,
// so the stored name is the link's host. // so no name is stored and, the links being unsafe, none is shown.
var sourceCases = []sourceCase{ var sourceCases = []sourceCase{
{"Chicken Curry", cleanSourceLink, "Curry House", cleanSourceLink, "Curry House"}, {"Chicken Curry", cleanSourceLink, "Curry House", cleanSourceLink, "Curry House"},
{"Beef Stew", "https://" + queryHost + "/stew?access_token=" + queryToken, "", nil, nil}, {"Beef Stew", "https://" + queryHost + "/stew?access_token=" + queryToken, "", nil, nil},
@@ -137,8 +137,9 @@ func legacyIdea(name, link, site string) map[string]any {
} }
// A job of either idea kind whose result was stored before sources were // A job of either idea kind whose result was stored before sources were
// made safe is served with the sources a job stored now holds: neither a // made safe, and upgraded by migration 0020, is served with the sources a
// token-bearing link nor a name taken from its host. // job stored now holds: neither a token-bearing link nor a name taken from
// its host.
func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) { func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
t.Parallel() t.Parallel()
model, normalize, search, pages, images := newQuickFakes() model, normalize, search, pages, images := newQuickFakes()
@@ -162,7 +163,10 @@ func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
id, kind, string(js)); err != nil { id, kind, string(js)); err != nil {
t.Fatal(err) t.Fatal(err)
} }
checkIdeaSources(t, h, id) }
applySiteNameMigration(t, st)
for kind := range map[string]bool{ideas.KindFromIngredients: true, ideas.KindQuick: true} {
checkIdeaSources(t, h, "legacy-"+kind)
} }
} }
+175 -4
View File
@@ -2,6 +2,7 @@ package store
import ( import (
"context" "context"
"fmt"
"io/fs" "io/fs"
"reflect" "reflect"
"slices" "slices"
@@ -176,8 +177,8 @@ func TestMigrateEmbeddedMigrations(t *testing.T) {
if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil { if err := Migrate(context.Background(), s.Writer, migrations.FS); err != nil {
t.Fatal(err) 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,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0019_shopping_group_frozen_name" { 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,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique,0019_shopping_group_frozen_name,0020_site_name_from_page" {
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,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0019_shopping_group_frozen_name", got) 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,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique,0019_shopping_group_frozen_name,0020_site_name_from_page", got)
} }
} }
@@ -197,8 +198,8 @@ func TestMigrateBootstrapThenEmbeddedInitApplies(t *testing.T) {
if err := Migrate(ctx, s.Writer, migrations.FS); err != nil { if err := Migrate(ctx, s.Writer, migrations.FS); err != nil {
t.Fatal(err) 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,0007_recipe_grounding,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0019_shopping_group_frozen_name" { 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,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique,0019_shopping_group_frozen_name,0020_site_name_from_page" {
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,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0019_shopping_group_frozen_name", got) 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,0008_dinner_recipe_url,0009_dinner_grounding_error,0010_dinner_grounding_revision,0011_ground_dinner_job_revision,0012_suggestion_name,0013_week_plan_revision,0014_rotation_review_name,0015_google_reconnect_required,0016_shopping_list_untitled,0017_shopping_list_chat_send_unconfirmed,0018_recipe_item_position_unique,0019_shopping_group_frozen_name,0020_site_name_from_page", got)
} }
if !tableExists(t, s, "dinners") { if !tableExists(t, s, "dinners") {
t.Error("table from 0001_init was not created") t.Error("table from 0001_init was not created")
@@ -923,6 +924,95 @@ func TestMigrateShoppingListChatSendUnconfirmed(t *testing.T) {
} }
} }
// seedRecipeItems inserts two recipes, each with an ingredient and a step
// at position 0, then the extra statements.
func seedRecipeItems(t *testing.T, s *Store, extra ...string) {
t.Helper()
stmts := []string{
`INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES ('recipe-1', 'https://example.com/chili', 'Chili', '{}', 'f', 'c', 'u')`,
`INSERT INTO recipes (id, source_url, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES ('recipe-2', 'https://example.com/curry', 'Curry', '{}', 'f', 'c', 'u')`,
`INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-1', 'recipe-1', 0, '1 onion', 'c', 'u'), ('ing-2', 'recipe-2', 0, '1 lime', 'c', 'u')`,
`INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-1', 'recipe-1', 0, 'Chop.', 'c', 'u'), ('step-2', 'recipe-2', 0, 'Squeeze.', 'c', 'u')`,
}
for _, stmt := range append(stmts, extra...) {
if _, err := s.Writer.Exec(stmt); err != nil {
t.Fatalf("seed: %v", err)
}
}
}
// TestMigrateRecipeItemPositionUnique applies 0018 over two recipes with
// items at the same position: each recipe keeps its items, and a second
// ingredient or step at a position its recipe already uses is refused.
func TestMigrateRecipeItemPositionUnique(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0018")
seedRecipeItems(t, s)
if err := Migrate(ctx, s.Writer, migrations.FS); err != nil {
t.Fatal(err)
}
var got string
if err := s.Reader.QueryRowContext(ctx, `SELECT
(SELECT group_concat(id || '@' || recipe_id || ':' || position, ',') FROM (SELECT * FROM recipe_ingredients ORDER BY id)) || ' / ' ||
(SELECT group_concat(id || '@' || recipe_id || ':' || position, ',') FROM (SELECT * FROM recipe_steps ORDER BY id))`).Scan(&got); err != nil {
t.Fatal(err)
}
if want := "ing-1@recipe-1:0,ing-2@recipe-2:0 / step-1@recipe-1:0,step-2@recipe-2:0"; got != want {
t.Errorf("recipe items after 0018 = %s, want %s", got, want)
}
for _, tc := range []struct{ name, stmt string }{
{"second ingredient at a position", `INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-3', 'recipe-1', 0, '2 onions', 'c', 'u')`},
{"second step at a position", `INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-3', 'recipe-1', 0, 'Dice.', 'c', 'u')`},
{"ingredient moved onto another's position", `UPDATE recipe_ingredients SET recipe_id = 'recipe-1' WHERE id = 'ing-2'`},
{"step moved onto another's position", `UPDATE recipe_steps SET recipe_id = 'recipe-1' WHERE id = 'step-2'`},
} {
if _, err := s.Writer.Exec(tc.stmt); err == nil || !strings.Contains(err.Error(), "UNIQUE constraint failed") {
t.Errorf("%s: error %v, want UNIQUE constraint failed", tc.name, err)
}
}
}
// TestMigrateRecipeItemPositionUniqueRefusesDuplicate applies 0018 over a
// recipe already holding two ingredients, or two steps, at one position:
// its order is unknown, so the migration fails and is not recorded.
func TestMigrateRecipeItemPositionUniqueRefusesDuplicate(t *testing.T) {
for _, tc := range []struct{ table, stmt string }{
{"recipe_ingredients", `INSERT INTO recipe_ingredients (id, recipe_id, position, raw_text, created_at, updated_at)
VALUES ('ing-3', 'recipe-1', 0, '2 onions', 'c', 'u')`},
{"recipe_steps", `INSERT INTO recipe_steps (id, recipe_id, position, text, created_at, updated_at)
VALUES ('step-3', 'recipe-1', 0, 'Dice.', 'c', 'u')`},
} {
t.Run(tc.table, func(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0018")
seedRecipeItems(t, s, tc.stmt)
err := Migrate(ctx, s.Writer, migrations.FS)
if err == nil || !strings.Contains(err.Error(), "UNIQUE constraint failed: "+tc.table+".recipe_id, "+tc.table+".position") {
t.Fatalf("Migrate = %v, want the duplicate position refused", err)
}
var n int
if err := s.Reader.QueryRowContext(ctx, `SELECT count(*) FROM schema_migrations WHERE version = '0018_recipe_item_position_unique'`).Scan(&n); err != nil {
t.Fatal(err)
}
if n != 0 {
t.Error("0018 recorded as applied")
}
})
}
}
// TestMigrateShoppingGroupFrozenName applies 0019 over a frozen list and // TestMigrateShoppingGroupFrozenName applies 0019 over a frozen list and
// a current one: the frozen list's group takes its dinner's name now, the // a current one: the frozen list's group takes its dinner's name now, the
// current list's group has none, and a frozen name is never empty. // current list's group has none, and a frozen name is never empty.
@@ -976,3 +1066,84 @@ func TestMigrateShoppingGroupFrozenName(t *testing.T) {
t.Errorf("empty frozen name: error %v, want CHECK constraint failed", err) t.Errorf("empty frozen name: error %v, want CHECK constraint failed", err)
} }
} }
// TestMigrateSiteNameFromPage applies 0020 over recipes and idea job
// results stored when a page naming no site was stored under its host: a
// stored name that could be a host — one with no space that is dotted, an
// IP address or spelled in the recipe's own link — is cleared, so every
// stored name is one a page gave; a name with a space is kept, as is
// everything else in the rows.
func TestMigrateSiteNameFromPage(t *testing.T) {
ctx := context.Background()
s := openTestStore(t)
migrateBefore(t, s, "0020")
sites := []struct {
stored any
link string
want any
}{
{"private-token.example.com", "https://private-token.example.com/stew", nil},
{"redirect_token.example.net", "https://kitchen.example/stew", nil},
{"bücher.example", "https://kitchen.example/stew", nil},
{"Food.com", "https://www.food.com/stew", nil},
{"203.0.113.7", "https://203.0.113.7/stew", nil},
{"2001:db8::1", "https://[2001:db8::1]/stew", nil},
{"TokenHost", "https://www.tokenhost/stew", nil},
{"Delish", "https://www.delish.com/stew", nil},
{"Delish", "https://kitchen.example/stew", "Delish"},
{"Curry House", "https://curry-house.example/stew", "Curry House"},
{"Mr. Food Test Kitchen", "https://kitchen.example/stew", "Mr. Food Test Kitchen"},
{nil, "https://kitchen.example/stew", nil},
}
for i, c := range sites {
if _, err := s.Writer.ExecContext(ctx, `INSERT INTO recipes (id, source_url, site_name, title, raw_jsonld, fetched_at, created_at, updated_at)
VALUES (?, ?, ?, 'Stew', '{}', 'f', 'c', 'u')`, fmt.Sprintf("recipe-%d", i), fmt.Sprintf("%s?row=%d", c.link, i), c.stored); err != nil {
t.Fatal(err)
}
}
const (
legacy = `{"stage":"done","ideas":[{"name":"A","site_name":"query-host.example","source_url":"https://query-host.example/a?t=1"},{"name":"B","site_name":"Curry House","source_url":"https://b.example/b"},{"name":"C","site_name":null,"source_url":null},{"name":"D","site_name":"Kitchen","source_url":"https://kitchen.example/d"}],"dropped":1}`
cleaned = `{"stage":"done","ideas":[{"name":"A","site_name":null,"source_url":"https://query-host.example/a?t=1"},{"name":"B","site_name":"Curry House","source_url":"https://b.example/b"},{"name":"C","site_name":null,"source_url":null},{"name":"D","site_name":null,"source_url":"https://kitchen.example/d"}],"dropped":1}`
other = `{"ideas":[{"site_name":"host.example"}]}`
)
for _, j := range [][3]string{
{"job-ideas", "ideas_from_ingredients", legacy},
{"job-quick", "quick_ideas", legacy},
{"job-other", "ground_dinner", other},
{"job-empty", "quick_ideas", `{"stage":"searching","ideas":[]}`},
} {
if _, err := s.Writer.ExecContext(ctx, `INSERT INTO jobs (id, kind, status, payload, result, created_at, updated_at)
VALUES (?, ?, 'succeeded', '{}', ?, 'c', 'u')`, j[0], j[1], j[2]); err != nil {
t.Fatal(err)
}
}
if _, err := s.Writer.ExecContext(ctx, `INSERT INTO jobs (id, kind, status, payload, created_at, updated_at)
VALUES ('job-running', 'quick_ideas', 'running', '{}', 'c', 'u')`); err != nil {
t.Fatal(err)
}
if err := Migrate(ctx, s.Writer, migrations.FS); err != nil {
t.Fatal(err)
}
for i, c := range sites {
var got any
var title string
if err := s.Reader.QueryRowContext(ctx, `SELECT site_name, title FROM recipes WHERE id = ?`, fmt.Sprintf("recipe-%d", i)).Scan(&got, &title); err != nil {
t.Fatal(err)
}
if got != c.want || title != "Stew" {
t.Errorf("recipe stored with site_name %v: site_name %v, title %q after 0020; want %v, Stew", c.stored, got, title, c.want)
}
}
for id, want := range map[string]any{"job-ideas": cleaned, "job-quick": cleaned, "job-other": other,
"job-empty": `{"stage":"searching","ideas":[]}`, "job-running": nil} {
var got any
if err := s.Reader.QueryRowContext(ctx, `SELECT result FROM jobs WHERE id = ?`, id).Scan(&got); err != nil {
t.Fatal(err)
}
if got != want {
t.Errorf("%s result after 0020 = %v, want %v", id, got, want)
}
}
}
+4
View File
@@ -408,6 +408,8 @@ func TestSchemaUniqueConstraintsRejectDuplicates(t *testing.T) {
{"notification_outbox", []string{"event_key", "channel"}}, {"notification_outbox", []string{"event_key", "channel"}},
{"schedule_runs", []string{"schedule", "period_key"}}, {"schedule_runs", []string{"schedule", "period_key"}},
{"notification_prefs", []string{"event"}}, {"notification_prefs", []string{"event"}},
{"recipe_ingredients", []string{"recipe_id", "position"}},
{"recipe_steps", []string{"recipe_id", "position"}},
} }
s := seededStore(t) s := seededStore(t)
for _, c := range cases { for _, c := range cases {
@@ -445,6 +447,8 @@ func TestSchemaCompositeUniqueAllowsPartialOverlap(t *testing.T) {
{"same plan_date, different dinner", "rotation_reviews", "plan_date", "dinners", map[string]any{"dinner_id": rowID("dinners", 1)}}, {"same plan_date, different dinner", "rotation_reviews", "plan_date", "dinners", map[string]any{"dinner_id": rowID("dinners", 1)}},
{"same event_key, different channel", "notification_outbox", "event_key", "", map[string]any{"channel": "chat"}}, {"same event_key, different channel", "notification_outbox", "event_key", "", map[string]any{"channel": "chat"}},
{"same period_key, different schedule", "schedule_runs", "period_key", "", map[string]any{"schedule": "week_rollover"}}, {"same period_key, different schedule", "schedule_runs", "period_key", "", map[string]any{"schedule": "week_rollover"}},
{"same position, different recipe", "recipe_ingredients", "position", "recipes", map[string]any{"recipe_id": rowID("recipes", 1)}},
{"same position, different recipe", "recipe_steps", "position", "recipes", map[string]any{"recipe_id": rowID("recipes", 1)}},
} }
s := seededStore(t) s := seededStore(t)
for _, c := range cases { for _, c := range cases {
+85 -29
View File
@@ -54,7 +54,8 @@ func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) }
// no recipe grounds is dropped; any other grounding failure fails the // no recipe grounds is dropped; any other grounding failure fails the
// batch, as does suggest_side_quests failing, with no Side quest made up // batch, as does suggest_side_quests failing, with no Side quest made up
// instead. The batch ends done or failed. When recording the batch's end // instead. The batch ends done or failed. When recording the batch's end
// fails, so does the job; when it times out at shutdown, the job is left // fails, so does the job, with what the batch stored, and any
// new_suggestions it emitted, kept; when it times out at shutdown, the job is left
// running and the batch with it, for the next start to fail. // running and the batch with it, for the next start to fail.
// //
// When a batch stores its Side quests depends on whether it notifies (see // When a batch stores its Side quests depends on whether it notifies (see
@@ -64,7 +65,12 @@ func (r batchResult) PublicJSON() ([]byte, error) { return json.Marshal(r) }
// Side quest stored. So the household is told of exactly what the batch // Side quest stored. So the household is told of exactly what the batch
// added, once; and a batch that fails at any round, including a later // added, once; and a batch that fails at any round, including a later
// round's normalize_recipe, stores nothing and sends nothing, leaving no // round's normalize_recipe, stores nothing and sends nothing, leaving no
// Side quest in the deck that was never announced. A replenish batch, // Side quest in the deck that was never announced. After each round it
// rechecks every Side quest it holds (see eligible) and lets go of those
// the household's changes made ineligible, so a later round grounds a
// spare Side quest in place of each; only a change committed between the
// last recheck and the store can still leave the batch short of
// BatchSize while spare Side quests remain. A replenish batch,
// which tops up the deck as the household swipes and never notifies, // which tops up the deck as the household swipes and never notifies,
// stores each round's Side quests before the next round starts, so the // stores each round's Side quests before the next round starts, so the
// deck fills as it goes; those stored before a failure stay in the deck. // deck fills as it goes; those stored before a failure stay in the deck.
@@ -146,7 +152,13 @@ func (s *Service) fill(ctx context.Context, trigger string, job BatchJob, publis
} }
res.Dropped += len(round) - len(grounded) res.Dropped += len(round) - len(grounded)
if note != nil { if note != nil {
held = append(held, grounded...) // Recheck every held Side quest, not just this round's, so
// one the household's changes made ineligible while this
// round grounded is replaced by a later round, not dropped by
// the store once no round is left to replace it.
if held, err = s.recheck(ctx, &res, append(held, grounded...)); err != nil {
return batchResult{}, err
}
} else if err := s.storeInto(ctx, &res, nil, grounded); err != nil { } else if err := s.storeInto(ctx, &res, nil, grounded); err != nil {
return batchResult{}, err return batchResult{}, err
} }
@@ -163,6 +175,22 @@ func (s *Service) fill(ctx context.Context, trigger string, job BatchJob, publis
return res, nil return res, nil
} }
// recheck returns, in order, those of a notifying batch's held Side
// quests that store would still keep (see eligible), adding those left
// out to res.Dropped.
func (s *Service) recheck(ctx context.Context, res *batchResult, held []groundedQuest) ([]groundedQuest, error) {
kept, err := s.eligible(ctx, db.New(s.st.Reader), res.BatchID, held)
if err != nil {
return nil, err
}
res.Dropped += len(held) - len(kept)
out := make([]groundedQuest, len(kept))
for i, k := range kept {
out[i] = k.groundedQuest
}
return out, nil
}
// storeInto stores quests for res's batch with note (see store) and adds // storeInto stores quests for res's batch with note (see store) and adds
// the suggestions stored, and the Side quests left out, to res. // the suggestions stored, and the Side quests left out, to res.
func (s *Service) storeInto(ctx context.Context, res *batchResult, note *notice, quests []groundedQuest) error { func (s *Service) storeInto(ctx context.Context, res *batchResult, note *notice, quests []groundedQuest) error {
@@ -277,11 +305,11 @@ func (s *Service) ground(ctx context.Context, quests []skills.SideQuest) ([]grou
// store stores each grounded Side quest of batchID as a dinner with status // store stores each grounded Side quest of batchID as a dinner with status
// suggested and origin side_quest, grounded in its recipe, and a pending // suggested and origin side_quest, grounded in its recipe, and a pending
// suggestion citing its Regulars by id, all in one transaction, and // suggestion citing its Regulars by id, all in one transaction, and
// returns the suggestion ids. A Side quest is left out when, since // returns the suggestion ids. A Side quest eligible leaves out is not
// suggest_side_quests ran, a dinner it repeats was added, a suggestion it // stored: since suggest_side_quests ran, a dinner it repeats was added, a
// repeats passed or a Rotation check dropped, or a Regular it cites renamed // suggestion it repeats passed or a Rotation check dropped, or a Regular
// or archived. Passed names are read in the same transaction as the store, // it cites renamed or archived. eligible reads in the same transaction as
// so a Pass or drop committed before it is never missed. // the store, so a Pass or drop committed before it is never missed.
// //
// When note is not nil and a Side quest is stored, new_suggestions keyed by // When note is not nil and a Side quest is stored, new_suggestions keyed by
// note, counting and naming those stored, is emitted in the same // note, counting and naming those stored, is emitted in the same
@@ -295,28 +323,13 @@ func (s *Service) store(ctx context.Context, batchID string, note *notice, quest
var stored []string var stored []string
err := s.st.WithTx(ctx, func(tx *sql.Tx) error { err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
q := db.New(tx) q := db.New(tx)
existing, err := q.ListUnarchivedDinners(ctx) kept, err := s.eligible(ctx, q, batchID, quests)
if err != nil { if err != nil {
return fmt.Errorf("suggest: list dinners for dedupe: %w", err) return err
} }
names, err := q.ListPassedNames(ctx) for _, k := range kept {
if err != nil { gq := k.groundedQuest
return fmt.Errorf("suggest: list passed names for dedupe: %w", err) citedJSON, err := json.Marshal(k.cited)
}
regulars := map[string][]string{}
for _, row := range existing {
names = append(names, row.Dinner.Name)
if row.Dinner.Status == dinners.StatusRegular {
regulars[row.Dinner.Name] = append(regulars[row.Dinner.Name], row.Dinner.ID)
}
}
for _, gq := range quests {
cited, ok := citedIDs(gq.quest.CitedRegulars, regulars)
if !ok || repeats(gq.quest.Name, names) {
s.logger.InfoContext(ctx, "suggested dinner dropped: changed while grounding", "batch_id", batchID)
continue
}
citedJSON, err := json.Marshal(cited)
if err != nil { if err != nil {
return fmt.Errorf("suggest: encode cited Regulars: %w", err) return fmt.Errorf("suggest: encode cited Regulars: %w", err)
} }
@@ -354,7 +367,6 @@ func (s *Service) store(ctx context.Context, batchID string, note *notice, quest
}); err != nil { }); err != nil {
return fmt.Errorf("suggest: create suggestion: %w", err) return fmt.Errorf("suggest: create suggestion: %w", err)
} }
names = append(names, d.Name)
ids = append(ids, id) ids = append(ids, id)
stored = append(stored, d.Name) stored = append(stored, d.Name)
} }
@@ -372,6 +384,50 @@ func (s *Service) store(ctx context.Context, batchID string, note *notice, quest
return ids, nil return ids, nil
} }
// eligibleQuest is a Side quest store would keep, with the ids of the
// Regulars it cites.
type eligibleQuest struct {
groundedQuest
cited []string
}
// eligible returns, in order, those of batchID's quests that q's dinners
// and passed names still allow, each with the ids of the Regulars it
// cites. A Side quest is left out when a dinner it repeats is unarchived,
// a suggestion it repeats passed or a Rotation check dropped, a Regular it
// cites is renamed or archived, or an earlier Side quest of quests kept
// repeats it. store checks with this in its transaction; a notifying
// batch checks the Side quests it holds with it after each round (see
// fill).
func (s *Service) eligible(ctx context.Context, q *db.Queries, batchID string, quests []groundedQuest) ([]eligibleQuest, error) {
existing, err := q.ListUnarchivedDinners(ctx)
if err != nil {
return nil, fmt.Errorf("suggest: list dinners for dedupe: %w", err)
}
names, err := q.ListPassedNames(ctx)
if err != nil {
return nil, fmt.Errorf("suggest: list passed names for dedupe: %w", err)
}
regulars := map[string][]string{}
for _, row := range existing {
names = append(names, row.Dinner.Name)
if row.Dinner.Status == dinners.StatusRegular {
regulars[row.Dinner.Name] = append(regulars[row.Dinner.Name], row.Dinner.ID)
}
}
var kept []eligibleQuest
for _, gq := range quests {
cited, ok := citedIDs(gq.quest.CitedRegulars, regulars)
if !ok || repeats(gq.quest.Name, names) {
s.logger.InfoContext(ctx, "suggested dinner dropped: changed while grounding", "batch_id", batchID)
continue
}
names = append(names, gq.quest.Name)
kept = append(kept, eligibleQuest{groundedQuest: gq, cited: cited})
}
return kept, nil
}
// citedIDs returns the ids of the Regulars named cited, false when one // citedIDs returns the ids of the Regulars named cited, false when one
// names no Regular or more than one. // names no Regular or more than one.
func citedIDs(cited []string, regulars map[string][]string) ([]string, bool) { func citedIDs(cited []string, regulars map[string][]string) ([]string, bool) {
+68
View File
@@ -660,6 +660,74 @@ func TestBatchKeepsAtMostBatchSize(t *testing.T) {
} }
} }
// TestBatchReplacesSideQuestMadeIneligibleWhileHeld: a notifying batch
// holds each round's Side quests until its last round is done; when the
// household adds a dinner repeating one held from an earlier round while a
// later round grounds, the batch lets it go and grounds a spare Side quest
// in its place, so it still stores BatchSize and announces them once.
func TestBatchReplacesSideQuestMadeIneligibleWhileHeld(t *testing.T) {
for _, trigger := range []string{suggest.TriggerScheduled, suggest.TriggerManual} {
t.Run(trigger, func(t *testing.T) {
e := newEnv(t)
e.start(t)
// Fajita Dish B is ungroundable, so the first round holds
// BatchSize-1 and a second round grounds the next; the last is
// the spare a third round grounds in place of Fajita Dish A.
var quests, want []string
for i := range suggest.BatchSize + 2 {
name := fmt.Sprintf("Fajita Dish %c", 'A'+i)
e.search.Results[name] = nil
if i != 1 {
e.recipe(t, name)
}
quests = append(quests, sideQuest(name, "Like your Chicken Fajitas.", "Chicken Fajitas"))
if i > 1 {
want = append(want, name)
}
}
e.reply(quests...)
// The second round's search waits, so the household adds a
// dinner repeating Fajita Dish A, held from the first round,
// while that round grounds.
second := fmt.Sprintf("Fajita Dish %c", 'A'+suggest.BatchSize)
gate := make(chan struct{})
e.search.Gates = map[string]chan struct{}{second: gate}
b, key := e.startBatch(t, trigger)
deadline := time.Now().Add(10 * time.Second)
for !slices.Contains(e.search.Queries(), second) {
if time.Now().After(deadline) {
close(gate)
t.Fatalf("search for %s never ran", second)
}
time.Sleep(5 * time.Millisecond)
}
e.dinner(t, "Fajita Dish A", "regular")
close(gate)
job := e.waitJob(t, b.JobID)
if job.Status != jobs.StatusSucceeded {
t.Fatalf("batch job %s, error %v", job.Status, job.Error)
}
var res batchResult
if err := json.Unmarshal(job.Result, &res); err != nil {
t.Fatal(err)
}
if len(res.SuggestionIDs) != suggest.BatchSize || res.Dropped != 2 {
t.Errorf("batch result = %+v, want %d stored and 2 dropped", res, suggest.BatchSize)
}
if got := e.deckNames(t); !slices.Equal(got, want) {
t.Errorf("deck = %v, want %v", got, want)
}
rows := notifytest.Rows(t, e.st, notify.NewSuggestions)
title := fmt.Sprintf("%d new Side quests are in", suggest.BatchSize)
if len(rows) != 1 || rows[0].Key != key || rows[0].Payload.Title != title {
t.Errorf("new_suggestions rows = %+v, want one keyed %s titled %q", rows, key, title)
}
})
}
}
func TestDeckIsOldestFirst(t *testing.T) { func TestDeckIsOldestFirst(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.start(t) e.start(t)
@@ -0,0 +1,9 @@
-- 0018_recipe_item_position_unique: position is the only record of recipe
-- order, so a recipe never has two ingredients or two steps at one
-- position. Writers replace a recipe's items whole, numbered from 0, and
-- the detail read orders by position alone. A database already holding a
-- duplicate fails this migration rather than keep an order nobody can
-- trust.
CREATE UNIQUE INDEX recipe_ingredients_recipe_position ON recipe_ingredients (recipe_id, position);
CREATE UNIQUE INDEX recipe_steps_recipe_position ON recipe_steps (recipe_id, position);
+41
View File
@@ -0,0 +1,41 @@
-- 0020_site_name_from_page: recipes.site_name now holds only the name a
-- recipe page gives its site (its publisher's name or og:site_name), and is
-- null when the page names none; a client is credited with the link's host
-- only when dinners.SourceOf finds the link safe to show. Before this, a
-- page naming no site was stored under the host of the URL it was served
-- from, which may carry a personal token, and nothing recorded which a
-- stored name was. A stored name that could be a host is cleared here, so
-- every stored name is one a page gave: one with no space (a host has
-- none) that is dotted or holds a colon — a host name or an IP address —
-- or that appears in the recipe's own link, as a dotless host does. This
-- clears every name the read rule before it withheld, and more: a site
-- that names itself as a domain, such as "Food.com", or by a word its link
-- spells, such as "Delish" on delish.com, loses its stored name until its
-- recipe is fetched again, and is credited by its link's host meanwhile
-- when the link is safe.
--
-- Idea job results stored before sources were made safe hold the grounded
-- recipe's site name and link as they were stored, and are cleared the
-- same way: an idea's stored site name is shown as it stands. The ideas
-- array is rebuilt from a plain scan of json_each, which yields its
-- elements in array order, so the ideas keep their order (sqlc's parser
-- refuses an ORDER BY inside json_group_array).
UPDATE recipes SET site_name = NULL
WHERE instr(site_name, ' ') = 0
AND (site_name GLOB '*[.:]*' OR instr(lower(source_url), lower(site_name)) > 0);
UPDATE jobs SET result = json_set(result, '$.ideas', (
SELECT json_group_array(
CASE
WHEN instr(json_extract(idea.value, '$.site_name'), ' ') = 0
AND (json_extract(idea.value, '$.site_name') GLOB '*[.:]*'
OR instr(lower(json_extract(idea.value, '$.source_url')), lower(json_extract(idea.value, '$.site_name'))) > 0)
THEN json_set(idea.value, '$.site_name', json('null'))
ELSE json(idea.value)
END
)
FROM json_each(jobs.result, '$.ideas') AS idea
))
WHERE kind IN ('ideas_from_ingredients', 'quick_ideas')
AND json_type(result, '$.ideas') = 'array';
+2 -2
View File
@@ -48,7 +48,7 @@ INSERT INTO recipe_ingredients (
-- name: ListRecipeIngredients :many -- name: ListRecipeIngredients :many
SELECT * FROM recipe_ingredients SELECT * FROM recipe_ingredients
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id; ORDER BY position;
-- name: DeleteRecipeIngredients :exec -- name: DeleteRecipeIngredients :exec
DELETE FROM recipe_ingredients DELETE FROM recipe_ingredients
@@ -64,7 +64,7 @@ INSERT INTO recipe_steps (
-- name: ListRecipeSteps :many -- name: ListRecipeSteps :many
SELECT * FROM recipe_steps SELECT * FROM recipe_steps
WHERE recipe_id = ? WHERE recipe_id = ?
ORDER BY position, id; ORDER BY position;
-- name: DeleteRecipeSteps :exec -- name: DeleteRecipeSteps :exec
DELETE FROM recipe_steps DELETE FROM recipe_steps
+4 -2
View File
@@ -54,7 +54,9 @@ export type RecipeFix = { candidate_id: string } | { recipe_url: string } | { ma
export type RecipeCandidate = { export type RecipeCandidate = {
candidate_id: string candidate_id: string
title: string title: string
site_name: string // site_name is null when the server cannot name the page's site without
// leaking its link (dinners.SourceOf).
site_name: string | null
photo_id: string | undefined photo_id: string | undefined
minutes: number | null minutes: number | null
} }
@@ -130,7 +132,7 @@ function parseCandidate(where: string, body: unknown): RecipeCandidate {
!isRecord(body) || !isRecord(body) ||
typeof body.candidate_id !== 'string' || typeof body.candidate_id !== 'string' ||
typeof body.title !== 'string' || typeof body.title !== 'string' ||
typeof body.site_name !== 'string' (body.site_name !== null && typeof body.site_name !== 'string')
) { ) {
throw new Error(`${where}: response is not a recipe candidate`) throw new Error(`${where}: response is not a recipe candidate`)
} }
@@ -344,7 +344,7 @@ describe('RegularsSettingsPage', () => {
minutes: 45, minutes: 45,
}, },
{ candidate_id: 'https://cook.test/pizza', title: 'Sheet-pan pizza', site_name: 'Cook Test', photo_url: null, minutes: null }, { candidate_id: 'https://cook.test/pizza', title: 'Sheet-pan pizza', site_name: 'Cook Test', photo_url: null, minutes: null },
{ candidate_id: 'https://food.test/p', title: 'Pizza margherita', site_name: 'Food Test', photo_url: null, minutes: 30 }, { candidate_id: 'https://food.test/p', title: 'Pizza margherita', site_name: null, photo_url: null, minutes: 30 },
], ],
} }
@@ -372,7 +372,7 @@ describe('RegularsSettingsPage', () => {
expect(options.map((option) => option.getAttribute('aria-label'))).toEqual([ expect(options.map((option) => option.getAttribute('aria-label'))).toEqual([
'Use Weeknight pizza from Example Kitchen', 'Use Weeknight pizza from Example Kitchen',
'Use Sheet-pan pizza from Cook Test', 'Use Sheet-pan pizza from Cook Test',
'Use Pizza margherita from Food Test', 'Use Pizza margherita',
]) ])
expect(within(options[0]!).getByRole('img', { name: 'Weeknight pizza' })).toHaveAttribute('src', '/media/photos/p7-400.jpg') expect(within(options[0]!).getByRole('img', { name: 'Weeknight pizza' })).toHaveAttribute('src', '/media/photos/p7-400.jpg')
expect(within(options[0]!).getByText('Example Kitchen')).toBeInTheDocument() expect(within(options[0]!).getByText('Example Kitchen')).toBeInTheDocument()
+2 -2
View File
@@ -527,13 +527,13 @@ function PickCandidate({ id, onPick }: { id: string; onPick: (candidateId: strin
<button <button
type="button" type="button"
className={styles.candidate} className={styles.candidate}
aria-label={`Use ${candidate.title} from ${candidate.site_name}`} aria-label={candidate.site_name === null ? `Use ${candidate.title}` : `Use ${candidate.title} from ${candidate.site_name}`}
onClick={() => onPick(candidate.candidate_id)} onClick={() => onPick(candidate.candidate_id)}
> >
<Photo id={candidate.photo_id} name={candidate.title} radius="14px" sizes="56px" className={styles.candidatePhoto} /> <Photo id={candidate.photo_id} name={candidate.title} radius="14px" sizes="56px" className={styles.candidatePhoto} />
<span className={styles.candidateBody}> <span className={styles.candidateBody}>
<span className={styles.candidateTitle}>{candidate.title}</span> <span className={styles.candidateTitle}>{candidate.title}</span>
<span className={styles.candidateSite}>{candidate.site_name}</span> {candidate.site_name !== null && <span className={styles.candidateSite}>{candidate.site_name}</span>}
</span> </span>
</button> </button>
</li> </li>