Record site-name provenance instead of inferring it from the name's shape
CI / ci (push) Successful in 8m11s

Recipe extraction no longer falls back to the URL's host: a stored
recipes.site_name is now always a name the page gave (publisher or
og:site_name), and null when it named none. dinners.SourceOf is the one
documented rule for every fetched recipe's source: the page's name is
shown as it stands; otherwise the site is named by the link's host only
when the link itself is shown as free of secrets, else null. The recipe
detail, idea results and the recipe-candidate offer endpoint all apply
it; candidates no longer carry an extraction-time hostname fallback, so
their site_name is nullable (frontend updated to match).

Migration 0020 clears host-shaped site names (dotted host names, IP
addresses) stored before the change, in recipes and in legacy idea job
results, so already-stored rows read under the same rule.

Maestro 966066e3-0e25-4b8b-9bb0-0694c47c9106

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-11 10:02:44 -04:00
co-authored by Claude Opus 5.5
parent d182349646
commit 267d0be4a6
18 changed files with 360 additions and 141 deletions
+13 -11
View File
@@ -216,16 +216,17 @@ automatic** (D-021).
empty (`[]`) with no recipe, as for a dinner whose grounding is
`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
is 500 rather than shown out of order. `source` is `{site_name}` of the
page the recipe was fetched from, null for a manual recipe or none; it
has no `url`, as the page's link may carry a personal token.
`site_name` is the name the page gives its site (publisher or
`og:site_name`) and null otherwise: a page naming no site is stored
under its host, which may be the pasted link's — a token in a
subdomain — or a redirect's, so any stored name that could be a host
(the link's host, an IP address or a dotted host name, even a real
name like "Food.com") is withheld. The detail screen shows "Recipe
from {site}" only when `site_name` is not null. `why_picked` is the reason of the
is 500 rather than shown out of order. `source` is `{site_name, url}`
of the page the recipe was fetched from, null for a manual recipe or
none, as `dinners.SourceOf` gives it — the one rule for every fetched
recipe's source, also used by idea results and recipe candidates. `url`
is the page's link only when it is free of secrets (https, no user,
query or fragment), else null. `site_name` is the name the page gives
its site (publisher or `og:site_name`), else the link's host without
"www." when `url` is shown, else null. Extraction never stores a host
as a site name, so a stored name is always the page's; migration 0020
cleared the host-shaped names stored before that. The detail screen
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
declining a suggestion archives its dinner — null for one no suggestion
created; a dinner with more than one such suggestion is 500, as nothing
@@ -262,7 +263,8 @@ automatic** (D-021).
`GET /api/v1/dinners/{id}/recipe-candidates`
searches for the dinner's name and returns `{candidates: [{candidate_id,
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
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
+34 -37
View File
@@ -58,14 +58,14 @@ type Ingredient struct {
RawText string `json:"raw_text"`
}
// Source is the site a recipe was fetched from. SiteName is the name the
// page itself gives its site, and null when it gives none or the stored
// name could have come from a link (see pageSiteName). URL is the page's
// link only when it is verifiably free of secrets (see safeSourceURL), and
// null otherwise: a link pasted or offered as a candidate may carry a
// personal token, so a client links the credit only when URL is not null.
// Source is set for every fetched recipe, so a client always credits it:
// by SiteName, else by URL's host, else without naming the site.
// Source is the site a recipe was fetched from, as SourceOf gives it.
// SiteName is the name the page gives its site, else the host of URL when
// URL is shown, else null. URL is the page's link only when it is
// verifiably free of secrets (see safeSourceURL), and null otherwise: a
// link pasted or offered as a candidate may carry a personal token, so a
// client links the credit only when URL is not null. Source is set for
// every fetched recipe, so a client always credits it: by SiteName, else
// without naming the site.
type Source struct {
SiteName *string `json:"site_name"`
URL *string `json:"url"`
@@ -183,42 +183,39 @@ func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail)
}
// 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
// pageSiteName) and the link only when it is free of secrets (see
// safeSourceURL). It is the one way a fetched recipe's source reaches a
// client. A sourceURL that does not parse is an error, whether or not a
// site name is stored.
// site name is siteName, the name its page gave (recipes.Recipe.SiteName),
// null when the page named none. It is the one rule by which a fetched
// recipe's source reaches a client — the recipe detail, an idea and a
// recipe candidate offered for a dinner alike:
//
// - 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) {
link, err := url.Parse(sourceURL)
if err != nil {
// Not wrapped: url.Parse's error quotes the link.
return Source{}, errors.New("source url does not parse")
}
return Source{SiteName: pageSiteName(siteName, link), URL: safeSourceURL(sourceURL)}, nil
}
// pageSiteName is the stored site name of a recipe fetched from link
// when it is one the page gave, else nil. A page that names no site is
// stored under recipes.HostSiteName of the URL it was served from: the
// pasted link's host, which may hold a personal token (a private
// subdomain), or that of a page the link redirected to, which is not
// 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
src := Source{URL: safeSourceURL(sourceURL)}
switch {
case siteName.Valid && siteName.String != "":
name := siteName.String
src.SiteName = &name
case src.URL != nil:
host := strings.TrimPrefix(link.Hostname(), "www.")
src.SiteName = &host
}
name := stored.String
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
return src, nil
}
// safeSourceURL is sourceURL when it is verifiably free of secrets — an
+27 -22
View File
@@ -76,35 +76,40 @@ func TestCreateRegularNeverNamesFromLink(t *testing.T) {
check("duplicate error", dup.Error())
}
// TestPageSiteNameWithholdsHosts: a stored site name that could be a host —
// the link's own, in any case and without "www.", an IP address or a dotted
// host name — is withheld; a name the page gave, dotless or with spaces, is
// kept.
func TestPageSiteNameWithholdsHosts(t *testing.T) {
// TestSourceOfSiteName: a site name the page gave is shown as it stands,
// even one shaped like a host and even over a link that is withheld; a
// recipe whose page named none is named by its link's host without "www."
// only when the link itself is shown, and is unnamed otherwise.
func TestSourceOfSiteName(t *testing.T) {
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 {
stored string
valid bool
want *string
stored string
valid bool
link string
want, wantURL *string
}{
{"", false, nil},
{"tokenhost", true, nil},
{"TOKENHOST", true, nil},
{"private-token.example.com", true, nil},
{"Food.com", true, nil},
{"203.0.113.7", true, nil},
{"2001:db8::1", true, nil},
{"Delish", true, ptr("Delish")},
{"Bon Appétit", true, ptr("Bon Appétit")},
{"Mr. Food Test Kitchen", true, ptr("Mr. Food Test Kitchen")},
{"Delish", true, ordinary, ptr("Delish"), ptr(ordinary)},
{"Food.com", true, ordinary, ptr("Food.com"), ptr(ordinary)},
{"Bon Appétit", true, token, ptr("Bon Appétit"), nil},
{"", false, ordinary, ptr("Kitchen.example"), ptr(ordinary)},
{"", true, ordinary, ptr("Kitchen.example"), ptr(ordinary)},
{"", false, token, nil, nil},
{"", false, "https://203.0.113.7/stew", ptr("203.0.113.7"), ptr("https://203.0.113.7/stew")},
{"", false, "http://kitchen.example/stew", nil, nil},
} {
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 {
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) {
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.
+8 -2
View File
@@ -304,7 +304,9 @@ type Candidate struct {
// may carry a token.
CandidateID string `json:"candidate_id"`
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 *string `json:"photo_url"`
Minutes *int `json:"minutes"`
@@ -367,9 +369,13 @@ func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error
out := make([]Candidate, len(offers))
urls := make(map[string]string, len(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)
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 != "" {
u := PhotoURL(o.PhotoID)
out[i].PhotoURL = &u
+2 -2
View File
@@ -718,11 +718,11 @@ func TestCandidates(t *testing.T) {
}
first, second := got[0], got[1]
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 {
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 {
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
// dinners.SourceOf). A result stored now already holds it, and one stored
// 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 "".
func shownSource(sourceURL, siteName *string) (dinners.Source, error) {
var link string
+7 -14
View File
@@ -28,8 +28,10 @@ var (
type Recipe struct {
// Title is the recipe's name.
Title string
// SiteName is the publisher's name, else the page's og:site_name, else
// HostSiteName of the page's URL.
// SiteName is the name the page gives its site: the publisher's name,
// 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
// ImageURL is the largest image, absolute; empty when the recipe has
// 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 == "" {
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
}
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))
}
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)
if !ok {
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 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.")
return ogSiteName, nil
}
// 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"}`),
check: func(t *testing.T, r Recipe) {
if r.SiteName != "example.com" {
if 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 string `json:"difficulty"`
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;
@@ -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
// in, with GroundURL on its URL.
type Offer struct {
URL string
Title string
URL string
Title string
// SiteName is the name the page gives its site, "" when it names none
// (see Recipe.SiteName).
SiteName string
// 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.
+73 -26
View File
@@ -3,6 +3,7 @@ package server
import (
"database/sql"
"encoding/json"
"io/fs"
"net/http"
"net/http/httptest"
"os"
@@ -14,6 +15,7 @@ import (
"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/store"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations"
)
// getDetail answers GET /dinners/{id} for id, failing unless it is 200, and
@@ -328,14 +330,33 @@ func TestGetDinnerSourceURLOnlyWhenSafe(t *testing.T) {
}
}
// TestGetDinnerSiteNameNeverFromLink: a page that names no site is stored
// under its host, so a recipe grounded in a pasted link whose host carries a
// token has site_name null and nothing from the link in its detail; a page
// 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.
// The links carry a query, so source.url is withheld too and nothing of the
// link may appear.
func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
// siteNameMigration is migration 0020, which clears the host-shaped site
// names stored before extraction stopped storing hosts.
const siteNameMigration = "0020_site_name_from_page.sql"
// applySiteNameMigration runs siteNameMigration again over rows a test
// stored as the code before it did, as upgrading a database holding them
// does.
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()
e := newDinnersEnv(t)
const (
@@ -343,11 +364,14 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
namedToken = "named-token-def456"
bare = "https://" + bareToken + ".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[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://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) {
t.Helper()
id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending
@@ -360,15 +384,18 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
}
return id, recipeID
}
storedSite := func(recipeID string) string {
storedSite := func(recipeID string) any {
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 {
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()
rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "")
if rec.Code != http.StatusOK {
@@ -380,28 +407,48 @@ func TestGetDinnerSiteNameNeverFromLink(t *testing.T) {
}
}
raw, _ := getDetail(t, e.h, id)
if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": want, "url": nil}) {
t.Errorf("source of %s = %v, want site_name %v and url null", what, raw["source"], want)
if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, 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)
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"
if _, err := e.st.Writer.Exec(`UPDATE recipes SET site_name = ? WHERE id = ?`, redirectHost, namedRecipe); err != nil {
t.Fatal(err)
bareID, bareRecipe := ground("Chicken curry", bare)
if got := storedSite(bareRecipe); got != nil {
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
+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
// 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
// recipe page's link, and source.site_name is null unless the page itself
// named its site — a name that could be a host, the link's or a redirect's,
// is withheld (dinners.Source) — so the detail screen shows "Recipe from
// {site}" only when site_name is not null.
// not a 200 with its status: it has no detail to show. source is as
// dinners.SourceOf gives it: the recipe page's link only when it is free of
// secrets, and site_name the name the page gave its site, else the shown
// link's host, else null — so the detail screen shows "Recipe from {site}"
// only when site_name is not null.
func getDinner(logger *slog.Logger, svc *dinners.Service) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
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
// 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 {
return func(w http.ResponseWriter, r *http.Request) {
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) {
t.Parallel()
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
// 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 {
dish, link, site string
// 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
// 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{
{"Chicken Curry", cleanSourceLink, "Curry House", cleanSourceLink, "Curry House"},
{"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
// made safe is served with the sources a job stored now holds: neither a
// token-bearing link nor a name taken from its host.
// made safe, and upgraded by migration 0020, is served with the sources a
// job stored now holds: neither a token-bearing link nor a name taken from
// its host.
func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
t.Parallel()
model, normalize, search, pages, images := newQuickFakes()
@@ -162,7 +163,10 @@ func TestJobServesLegacyIdeaResultsWithOnlySafeSources(t *testing.T) {
id, kind, string(js)); err != nil {
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)
}
}
+80 -4
View File
@@ -2,6 +2,7 @@ package store
import (
"context"
"fmt"
"io/fs"
"reflect"
"slices"
@@ -176,8 +177,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,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" {
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", 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,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,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,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 {
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" {
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", 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,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,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,0020_site_name_from_page", got)
}
if !tableExists(t, s, "dinners") {
t.Error("table from 0001_init was not created")
@@ -922,3 +923,78 @@ func TestMigrateShoppingListChatSendUnconfirmed(t *testing.T) {
t.Errorf("shopping list after 0017 = %s, want %s", got, want)
}
}
// 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 — a dotted host name or an IP address —
// is cleared, so every stored name is one a page gave; any other name,
// including a dotted one with spaces, 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, want any
}{
{"private-token.example.com", nil},
{"Food.com", nil},
{"203.0.113.7", nil},
{"2001:db8::1", nil},
{"Curry House", "Curry House"},
{"Delish", "Delish"},
{"Mr. Food Test Kitchen", "Mr. Food Test Kitchen"},
{nil, 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("https://r%d.example/stew", 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}],"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}],"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)
}
}
}
+36
View File
@@ -0,0 +1,36 @@
-- 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 — a dotted host name
-- or an IP address, spelled only in host characters — is cleared here, so
-- every stored name is one a page gave. A site that names itself as a
-- domain, such as "Food.com", loses its stored name with them 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 as it was 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 site_name GLOB '*[.:]*' AND site_name NOT GLOB '*[^-A-Za-z0-9.:]*';
UPDATE jobs SET result = json_set(result, '$.ideas', (
SELECT json_group_array(
CASE
WHEN json_extract(idea.value, '$.site_name') GLOB '*[.:]*'
AND json_extract(idea.value, '$.site_name') NOT GLOB '*[^-A-Za-z0-9.:]*'
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';
+4 -2
View File
@@ -54,7 +54,9 @@ export type RecipeFix = { candidate_id: string } | { recipe_url: string } | { ma
export type RecipeCandidate = {
candidate_id: 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
minutes: number | null
}
@@ -130,7 +132,7 @@ function parseCandidate(where: string, body: unknown): RecipeCandidate {
!isRecord(body) ||
typeof body.candidate_id !== '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`)
}
@@ -344,7 +344,7 @@ describe('RegularsSettingsPage', () => {
minutes: 45,
},
{ 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([
'Use Weeknight pizza from Example Kitchen',
'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]!).getByText('Example Kitchen')).toBeInTheDocument()
+2 -2
View File
@@ -527,13 +527,13 @@ function PickCandidate({ id, onPick }: { id: string; onPick: (candidateId: strin
<button
type="button"
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)}
>
<Photo id={candidate.photo_id} name={candidate.title} radius="14px" sizes="56px" className={styles.candidatePhoto} />
<span className={styles.candidateBody}>
<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>
</button>
</li>