diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 6522df2..c3529fc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -204,6 +204,30 @@ automatic** (D-021). sets status `archived` and keeps the row; `POST /api/v1/dinners/{id}/favorite` toggles `is_favorite`. Every write records the session's subject as `updated_by`; an unknown id is 404. + `GET /api/v1/dinners/{id}` returns the dinner with its recipe, read in + one transaction: `{id, name, status, origin, method, minutes_total, + difficulty, cuisine, is_favorite, photo_urls, ingredients, steps, + source, grounding_status, grounding_error, why_picked}`. `photo_urls` is + `{"400": url, "1200": url}` (`/media/photos/{photo_id}-{size}.jpg`) or + null with no photo; `ingredients` is `[{name, qty, unit, note, + raw_text}]` and `steps` the step texts, both in recipe position order and + 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 + 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 + records which created it. An unknown or archived dinner is 404. - Dinner grounding. `POST /api/v1/dinners` also enqueues a `ground_dinner` job and answers with its `grounding_job_id` without waiting for it (`GET /api/v1/jobs/{id}` reports it); if the job cannot be enqueued it diff --git a/internal/db/querier.go b/internal/db/querier.go index 00e2e22..66b6610 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -103,6 +103,13 @@ type Querier interface { // Returns sql.ErrNoRows until the row exists. GetChatSettings(ctx context.Context) (ChatSetting, error) GetDinner(ctx context.Context, id string) (Dinner, error) + // The reason of each suggestion of dinner_id that can have created it, + // none for a dinner no suggestion created. A suggestion is only ever + // created with a new dinner, and declining one archives its dinner, so a + // live dinner's creating suggestion is pending (the dinner still + // suggested) or accepted; a declined one did not create it. More than one + // row means nothing records which did. + GetDinnerSuggestionReasons(ctx context.Context, dinnerID string) ([]string, error) // Returns sql.ErrNoRows until the row exists. GetHouseholdSettings(ctx context.Context) (HouseholdSetting, error) // Returns sql.ErrNoRows until the row exists. diff --git a/internal/db/suggestions.sql.go b/internal/db/suggestions.sql.go index e7f12d7..879e606 100644 --- a/internal/db/suggestions.sql.go +++ b/internal/db/suggestions.sql.go @@ -187,6 +187,41 @@ func (q *Queries) GetActiveSuggestionBatch(ctx context.Context) (SuggestionBatch return i, err } +const getDinnerSuggestionReasons = `-- name: GetDinnerSuggestionReasons :many +SELECT reason FROM suggestions +WHERE dinner_id = ? AND status IN ('pending', 'accepted') +ORDER BY id +` + +// The reason of each suggestion of dinner_id that can have created it, +// none for a dinner no suggestion created. A suggestion is only ever +// created with a new dinner, and declining one archives its dinner, so a +// live dinner's creating suggestion is pending (the dinner still +// suggested) or accepted; a declined one did not create it. More than one +// row means nothing records which did. +func (q *Queries) GetDinnerSuggestionReasons(ctx context.Context, dinnerID string) ([]string, error) { + rows, err := q.db.QueryContext(ctx, getDinnerSuggestionReasons, dinnerID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []string + for rows.Next() { + var reason string + if err := rows.Scan(&reason); err != nil { + return nil, err + } + items = append(items, reason) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getLatestSuggestionBatch = `-- name: GetLatestSuggestionBatch :one SELECT id, "trigger", status, ai_run_id, created_at, updated_at FROM suggestion_batches ORDER BY id DESC diff --git a/internal/dinners/detail.go b/internal/dinners/detail.go new file mode 100644 index 0000000..b2673b2 --- /dev/null +++ b/internal/dinners/detail.go @@ -0,0 +1,218 @@ +package dinners + +import ( + "context" + "database/sql" + "errors" + "fmt" + "net" + "net/url" + "strconv" + "strings" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/recipes" +) + +// Detail is a dinner with its recipe, as the recipe detail screen and +// Kitchen reading mode show it. +type Detail struct { + ID string `json:"id"` + Name string `json:"name"` + Status string `json:"status"` + Origin string `json:"origin"` + Method *string `json:"method"` + MinutesTotal *int64 `json:"minutes_total"` + Difficulty *string `json:"difficulty"` + Cuisine *string `json:"cuisine"` + IsFavorite bool `json:"is_favorite"` + // PhotoURLs is the cached photo at each stored width, keyed by width + // ("400", "1200"); null when the dinner has no photo. + PhotoURLs map[string]string `json:"photo_urls"` + // Ingredients and Steps are the recipe's, in recipe order; empty when + // the dinner has no recipe. + Ingredients []Ingredient `json:"ingredients"` + Steps []string `json:"steps"` + // Source is the site of the recipe page the dinner is grounded in; + // null for a recipe entered by hand or a dinner with no recipe. + Source *Source `json:"source"` + GroundingStatus string `json:"grounding_status"` + // GroundingError is why grounding failed, one of the Reason codes; + // null unless GroundingStatus is failed. + GroundingError *string `json:"grounding_error"` + // WhyPicked is the reason of the suggestion that created the dinner + // (see GetDinnerSuggestionReasons); null for a dinner no suggestion + // created. + WhyPicked *string `json:"why_picked"` +} + +// Ingredient is one recipe line. RawText is the line as the recipe gives +// it; Name, Qty, Unit and Note are normalize_recipe's reading of it, each +// null when it has none, and Name, Qty and Unit always null for a recipe +// entered by hand (see IsManualRecipe). +type Ingredient struct { + Name *string `json:"name"` + Qty *string `json:"qty"` + Unit *string `json:"unit"` + Note *string `json:"note"` + RawText string `json:"raw_text"` +} + +// Source is the site a recipe was fetched from. It has no url: the page's +// link, pasted or offered as a candidate, may carry a personal token, and +// no stored link can be shown to be free of one, so like a dinner's +// recipe_url and a candidate's page it never leaves the server. 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), so a +// client shows "Recipe from {site}" only when SiteName is not null. +type Source struct { + SiteName *string `json:"site_name"` +} + +// Get returns dinner id with its recipe and the reason it was suggested, +// read in one read transaction. An unknown or archived dinner is +// ErrNotFound: an archived dinner has no detail to show. +func (s *Service) Get(ctx context.Context, id string) (d Detail, err error) { + tx, err := s.st.Reader.BeginTx(ctx, nil) + if err != nil { + return Detail{}, fmt.Errorf("dinners: begin read: %w", err) + } + defer func() { + err = errors.Join(err, tx.Rollback()) + }() + q := db.New(tx) + row, err := q.GetDinner(ctx, id) + if errors.Is(err, sql.ErrNoRows) { + return Detail{}, ErrNotFound + } + if err != nil { + return Detail{}, fmt.Errorf("dinners: read %s: %w", id, err) + } + if row.Status == StatusArchived { + return Detail{}, ErrNotFound + } + d = Detail{ + ID: row.ID, + Name: row.Name, + Status: row.Status, + Origin: row.Origin, + Method: nullString(row.Method), + Difficulty: nullString(row.Difficulty), + Cuisine: nullString(row.Cuisine), + IsFavorite: row.IsFavorite == 1, + Ingredients: []Ingredient{}, + Steps: []string{}, + GroundingStatus: row.GroundingStatus, + GroundingError: nullString(row.GroundingError), + } + if row.MinutesTotal.Valid { + d.MinutesTotal = &row.MinutesTotal.Int64 + } + if row.PhotoID.Valid { + d.PhotoURLs = make(map[string]string, len(recipes.PhotoSizes)) + for _, size := range recipes.PhotoSizes { + d.PhotoURLs[strconv.Itoa(size)] = photoURL(row.PhotoID.String, size) + } + } + if (row.GroundingStatus == GroundingGrounded || row.GroundingStatus == GroundingManual) && !row.RecipeID.Valid { + return Detail{}, fmt.Errorf("dinners: %s is %s with no recipe", id, row.GroundingStatus) + } + if row.RecipeID.Valid { + if err := readRecipe(ctx, q, row.RecipeID.String, &d); err != nil { + return Detail{}, fmt.Errorf("dinners: read recipe of %s: %w", id, err) + } + } + reasons, err := q.GetDinnerSuggestionReasons(ctx, id) + if err != nil { + return Detail{}, fmt.Errorf("dinners: read suggestion reason of %s: %w", id, err) + } + switch len(reasons) { + case 0: + case 1: + d.WhyPicked = &reasons[0] + default: + // Nothing records which of them created the dinner, and any + // one picked would be a guess. + return Detail{}, fmt.Errorf("dinners: %s has %d suggestions that could have created it", id, len(reasons)) + } + return d, nil +} + +// readRecipe sets d's Source, Ingredients and Steps from recipe recipeID. +func readRecipe(ctx context.Context, q *db.Queries, recipeID string, d *Detail) error { + rec, err := q.GetRecipe(ctx, recipeID) + if err != nil { + return fmt.Errorf("recipe %s: %w", recipeID, err) + } + if !IsManualRecipe(rec.SourceUrl) { + site, err := pageSiteName(rec.SiteName, rec.SourceUrl) + if err != nil { + return err + } + d.Source = &Source{SiteName: site} + } + ings, err := q.ListRecipeIngredients(ctx, recipeID) + if err != nil { + return fmt.Errorf("ingredients: %w", err) + } + for i, ing := range ings { + if i > 0 && ing.Position == ings[i-1].Position { + return duplicatePosition("ingredient", ing.Position) + } + d.Ingredients = append(d.Ingredients, Ingredient{ + Name: nullString(ing.Name), + Qty: nullString(ing.Qty), + Unit: nullString(ing.Unit), + Note: nullString(ing.Note), + RawText: ing.RawText, + }) + } + steps, err := q.ListRecipeSteps(ctx, recipeID) + if err != nil { + return fmt.Errorf("steps: %w", err) + } + for i, step := range steps { + if i > 0 && step.Position == steps[i-1].Position { + return duplicatePosition("step", step.Position) + } + d.Steps = append(d.Steps, step.Text) + } + return nil +} + +// pageSiteName is the stored site name of a recipe fetched from sourceURL +// 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 attribution with them: a recipe with no +// attribution is better than one that leaks a link. +func pageSiteName(stored sql.NullString, sourceURL string) (*string, error) { + if !stored.Valid { + return nil, nil + } + link, err := url.Parse(sourceURL) + if err != nil { + // Not wrapped: url.Parse's error quotes the link. + return nil, errors.New("source url does not parse") + } + name := stored.String + if strings.EqualFold(name, recipes.HostSiteName(link)) || net.ParseIP(name) != nil { + return nil, nil + } + if u, err := url.Parse("https://" + name + "/"); err == nil && u.Host == name && strings.Contains(name, ".") { + return nil, nil + } + return &name, nil +} + +// 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) +} diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index 54f355c..bf5154b 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -572,7 +572,12 @@ func checkUnique(ctx context.Context, q *db.Queries, self, name string, recipeUR // PhotoURL is where the API serves a list-sized copy of photo id. func PhotoURL(id string) string { - return fmt.Sprintf("/media/photos/%s-%d.jpg", id, photoListWidth) + return photoURL(id, photoListWidth) +} + +// photoURL is where the API serves photo id at stored width size. +func photoURL(id string, size int) string { + return fmt.Sprintf("/media/photos/%s-%d.jpg", id, size) } func fromRow(r db.Dinner) Dinner { diff --git a/internal/dinners/dinners_test.go b/internal/dinners/dinners_test.go index 764f6cb..16cf234 100644 --- a/internal/dinners/dinners_test.go +++ b/internal/dinners/dinners_test.go @@ -1,6 +1,7 @@ package dinners_test import ( + "database/sql" "encoding/json" "errors" "strings" @@ -74,3 +75,46 @@ 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) { + t.Parallel() + const link = "https://www.TokenHost/recipes/stew" + for _, tc := range []struct { + stored string + valid bool + want *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")}, + } { + got, err := dinners.PageSiteName(sql.NullString{String: tc.stored, Valid: tc.valid}, link) + if err != nil { + t.Fatalf("PageSiteName(%q): %v", tc.stored, err) + } + if (got == nil) != (tc.want == nil) || (got != nil && *got != *tc.want) { + t.Errorf("PageSiteName(%q) = %v, want %v", tc.stored, deref(got), deref(tc.want)) + } + } + if _, err := dinners.PageSiteName(sql.NullString{String: "Delish", Valid: true}, "https://secret-token\x7f.example/%zz"); err == nil || strings.Contains(err.Error(), "secret-token") { + t.Errorf("PageSiteName of an unparseable link: err = %v, want one not quoting the link", err) + } +} + +func deref(s *string) any { + if s == nil { + return nil + } + return *s +} diff --git a/internal/dinners/export_test.go b/internal/dinners/export_test.go index b143035..711568f 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -71,3 +71,6 @@ func (s *Service) RunGroundJob(ctx context.Context, p GroundJob) (string, error) r, err := s.runGroundJob(ctx, p, nil) return r.GroundingStatus, err } + +// PageSiteName is pageSiteName. +var PageSiteName = pageSiteName diff --git a/internal/recipes/extract.go b/internal/recipes/extract.go index 2ce6069..9de3383 100644 --- a/internal/recipes/extract.go +++ b/internal/recipes/extract.go @@ -29,7 +29,7 @@ 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 - // its host without a leading "www.". + // HostSiteName of the page's URL. SiteName string // ImageURL is the largest image, absolute; empty when the recipe has // none. @@ -232,7 +232,13 @@ func siteName(publisher any, g graph, ogSiteName string, base *url.URL) (string, if ogSiteName != "" { return ogSiteName, nil } - return strings.TrimPrefix(base.Hostname(), "www."), 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; diff --git a/internal/server/dinnerdetail_test.go b/internal/server/dinnerdetail_test.go new file mode 100644 index 0000000..f60ddaf --- /dev/null +++ b/internal/server/dinnerdetail_test.go @@ -0,0 +1,404 @@ +package server + +import ( + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "reflect" + "strings" + "testing" + + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "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" +) + +// getDetail answers GET /dinners/{id} for id, failing unless it is 200, and +// returns the body both raw (to tell null from absent) and decoded. +func getDetail(t *testing.T, h http.Handler, id string) (map[string]any, dinners.Detail) { + t.Helper() + rec, raw := callDinners(t, h, http.MethodGet, dinnersPath+"/"+id, "") + if rec.Code != http.StatusOK { + t.Fatalf("GET dinner = %d: %s", rec.Code, rec.Body) + } + var d dinners.Detail + dec := json.NewDecoder(rec.Body) + dec.DisallowUnknownFields() + if err := dec.Decode(&d); err != nil { + t.Fatalf("decode detail: %v", err) + } + return raw, d +} + +// wantNull fails unless each key is present in raw and null. +func wantNull(t *testing.T, raw map[string]any, keys ...string) { + t.Helper() + for _, k := range keys { + if v, ok := raw[k]; !ok || v != nil { + t.Errorf("%s = %v (present %v), want null", k, v, ok) + } + } +} + +// TestGetDinnerGroundedSideQuest: a Side quest grounded in a fetched page +// shows its photo at both stored sizes, its recipe's ingredients and steps +// in recipe order (not row order), its source site and the reason it was +// suggested. +func TestGetDinnerGroundedSideQuest(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t).withRecipes(t) + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + e.start(t) + rec, created := callDinners(t, e.h, http.MethodPost, dinnersPath, `{"name":"Chicken curry"}`) + if rec.Code != http.StatusCreated { + t.Fatalf("POST = %d: %s", rec.Code, rec.Body) + } + if job := waitJob(t, e.h, created["grounding_job_id"].(string)); job["status"] != "succeeded" { + t.Fatalf("grounding job = %v", job) + } + id := created["id"].(string) + + // Make it an accepted Side quest with the suggestion that created it, + // and reverse its recipe's positions so row order disagrees with + // recipe order. + var recipeID, photoID string + if err := e.st.Reader.QueryRow(`SELECT recipe_id, photo_id FROM dinners WHERE id = ?`, id).Scan(&recipeID, &photoID); err != nil { + t.Fatal(err) + } + for _, stmt := range []string{ + `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_steps SET position = 1 - position WHERE recipe_id = (SELECT recipe_id FROM dinners WHERE id = ?)`, + } { + if _, err := e.st.Writer.Exec(stmt, id); err != nil { + t.Fatal(err) + } + } + q := db.New(e.st.Writer) + now := store.Now().String() + batch, err := q.CreateSuggestionBatch(t.Context(), db.CreateSuggestionBatchParams{ID: store.NewID(), Trigger: "manual", CreatedAt: now, UpdatedAt: now}) + if err != nil { + t.Fatal(err) + } + const reason = "Like your Tikka masala, but quicker." + if err := q.CreateSuggestion(t.Context(), db.CreateSuggestionParams{ + ID: store.NewID(), DinnerID: id, BatchID: batch.ID, Name: "Chicken curry", + Reason: reason, CitedRegularIds: "[]", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } + + _, got := getDetail(t, e.h, id) + str := func(s string) *string { return &s } + minutes := int64(40) + want := dinners.Detail{ + ID: id, + Name: "Chicken curry", + Status: "quest_log", + Origin: "side_quest", + Method: str("stovetop"), + MinutesTotal: &minutes, + Difficulty: str("easy"), + IsFavorite: true, + PhotoURLs: map[string]string{ + "400": "/media/photos/" + photoID + "-400.jpg", + "1200": "/media/photos/" + photoID + "-1200.jpg", + }, + Ingredients: []dinners.Ingredient{ + {Name: str("garlic"), Qty: str("2"), Unit: str("cloves"), Note: str("minced"), RawText: "2 cloves garlic, minced"}, + {Name: str("onion"), Qty: str("1"), RawText: "1 onion"}, + }, + Steps: []string{"Fry with the garlic.", "Chop the onion."}, + Source: &dinners.Source{SiteName: str("Curry House")}, + GroundingStatus: "grounded", + WhyPicked: str(reason), + } + if !reflect.DeepEqual(got, want) { + gj, _ := json.Marshal(got) + wj, _ := json.Marshal(want) + t.Errorf("detail =\n %s\nwant\n %s", gj, wj) + } + // Each photo URL is served. + for _, u := range got.PhotoURLs { + rec := httptest.NewRecorder() + e.h.ServeHTTP(rec, signedIn(http.MethodGet, u, nil)) + if rec.Code != http.StatusOK || rec.Header().Get("Content-Type") != "image/jpeg" { + t.Errorf("GET %s = %d %s", u, rec.Code, rec.Header().Get("Content-Type")) + } + } +} + +// TestGetDinnerManualRegular: a Regular added by hand and given a recipe +// typed in has no photo, no source and no reason, and its ingredients are +// the typed lines with nothing normalized. +func TestGetDinnerManualRegular(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + id := createDinner(t, e.h, `{"name":"Family chili"}`) + 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) + } + + raw, got := getDetail(t, e.h, id) + wantNull(t, raw, "photo_urls", "source", "why_picked", "difficulty", "grounding_error") + str := func(s string) *string { return &s } + minutes := int64(240) + want := dinners.Detail{ + ID: id, + Name: "Family chili", + Status: "regular", + Origin: "regular", + Method: str("crockpot"), + MinutesTotal: &minutes, + Ingredients: []dinners.Ingredient{{RawText: "1 lb beef"}, {RawText: "1 can beans"}}, + Steps: []string{"Brown.", "Simmer."}, + GroundingStatus: "manual", + } + if !reflect.DeepEqual(got, want) { + gj, _ := json.Marshal(got) + wj, _ := json.Marshal(want) + t.Errorf("detail =\n %s\nwant\n %s", gj, wj) + } +} + +// TestGetDinnerFailedGrounding: a dinner whose grounding failed shows its +// real state — failed with its reason, no photo, no source and empty +// ingredients and steps — never a pretend recipe. +func TestGetDinnerFailedGrounding(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t).withRecipes(t) + e.start(t) + rec, created := callDinners(t, e.h, http.MethodPost, dinnersPath, `{"name":"Nothing matches this"}`) + if rec.Code != http.StatusCreated { + t.Fatalf("POST = %d: %s", rec.Code, rec.Body) + } + if job := waitJob(t, e.h, created["grounding_job_id"].(string)); job["status"] != "failed" { + t.Fatalf("grounding job = %v, want failed", job) + } + + raw, got := getDetail(t, e.h, created["id"].(string)) + wantNull(t, raw, "photo_urls", "source", "why_picked", "method", "minutes_total", "difficulty") + if ings, ok := raw["ingredients"].([]any); !ok || len(ings) != 0 { + t.Errorf("ingredients = %v, want []", raw["ingredients"]) + } + if steps, ok := raw["steps"].([]any); !ok || len(steps) != 0 { + t.Errorf("steps = %v, want []", raw["steps"]) + } + if got.GroundingStatus != "failed" || got.GroundingError == nil || *got.GroundingError != dinners.ReasonNoRecipe || + got.Name != "Nothing matches this" || got.Status != "regular" || got.Origin != "regular" { + t.Errorf("detail = %+v, want the failed Regular with no_recipe_found", got) + } +} + +// TestGetDinnerNotFound: an unknown dinner and an archived one are both +// 404, and the detail needs a session like every /api/v1 route. +func TestGetDinnerNotFound(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + id := createDinner(t, e.h, `{"name":"Old favourite"}`) + if rec, _ := callDinners(t, e.h, http.MethodDelete, dinnersPath+"/"+id, ""); rec.Code != http.StatusOK { + t.Fatalf("DELETE = %d: %s", rec.Code, rec.Body) + } + for _, target := range []string{id, store.NewID()} { + rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+target, "") + if rec.Code != http.StatusNotFound || errorCode(body) != "not_found" { + t.Errorf("GET %s = %d %v, want 404 not_found", target, rec.Code, body) + } + } + + live := createDinner(t, e.h, `{"name":"Taco night"}`) + req := httptest.NewRequest(http.MethodGet, dinnersPath+"/"+live, nil) + if rec, body := serve(t, e.h, req); rec.Code != http.StatusUnauthorized || errorCode(body) != "unauthenticated" { + t.Errorf("GET without a session = %d %v, want 401", rec.Code, body) + } +} + +// TestGetDinnerSourceHidesTokenURL: a recipe grounded in a pasted link that +// carries a token in its path or query shows its site name, but never the +// link or any part of it. +func TestGetDinnerSourceHidesTokenURL(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + const ( + pathToken = "path-token-abc123" + queryToken = "query-token-def456" + pathPage = "https://secret-path.example/u/" + pathToken + "/chicken-curry" + queryPage = "https://secret-query.example/curry?access_token=" + queryToken + ) + e.pages.Bodies[pathPage] = recipestest.RecipePage("Chicken Curry", "Curry House", "https://a.example/curry.jpg", 40) + e.pages.Bodies[queryPage] = recipestest.RecipePage("Weeknight Curry", "B Kitchen", "https://b.example/curry.jpg", 25) + 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) + for _, tc := range []struct{ dish, page, site string }{ + {"Chicken curry", pathPage, "Curry House"}, + {"Weeknight curry", queryPage, "B Kitchen"}, + } { + id := createDinner(t, e.h, `{"name":"`+tc.dish+`"}`) // runner not started: stays pending + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) + if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"recipe_url":"`+tc.page+`"}`); rec.Code != http.StatusOK { + t.Fatalf("POST ground %s = %d: %s", tc.page, rec.Code, rec.Body) + } + var stored string + if err := e.st.Reader.QueryRow(`SELECT source_url FROM recipes WHERE id = (SELECT recipe_id FROM dinners WHERE id = ?)`, id).Scan(&stored); err != nil { + t.Fatal(err) + } + if stored != tc.page { + t.Fatalf("stored source_url = %q, want %q", stored, tc.page) + } + + rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "") + if rec.Code != http.StatusOK { + t.Fatalf("GET dinner = %d: %s", rec.Code, rec.Body) + } + for _, secret := range []string{tc.page, "secret-path.example", "secret-query.example", pathToken, queryToken, "access_token", `"url"`} { + if strings.Contains(rec.Body.String(), secret) { + t.Errorf("detail of %s contains %q: %s", tc.dish, secret, rec.Body) + } + } + raw, _ := getDetail(t, e.h, id) + if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": tc.site}) { + t.Errorf("source of %s = %v, want only site_name %q", tc.dish, raw["source"], tc.site) + } + } +} + +// 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. +func TestGetDinnerSiteNameNeverFromLink(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + const ( + bareToken = "private-token-abc123" + namedToken = "named-token-def456" + bare = "https://" + bareToken + ".example.com/recipe" + named = "https://www." + namedToken + ".example.com/recipe" + ) + 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.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) + ground := func(dish, page string) (id, recipeID string) { + t.Helper() + id = createDinner(t, e.h, `{"name":"`+dish+`"}`) // runner not started: stays pending + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) + if rec, _ := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"recipe_url":"`+page+`"}`); rec.Code != http.StatusOK { + t.Fatalf("POST ground %s = %d: %s", page, rec.Code, rec.Body) + } + if err := e.st.Reader.QueryRow(`SELECT recipe_id FROM dinners WHERE id = ?`, id).Scan(&recipeID); err != nil { + t.Fatal(err) + } + return id, recipeID + } + storedSite := func(recipeID string) string { + t.Helper() + var site string + if err := e.st.Reader.QueryRow(`SELECT site_name FROM recipes WHERE id = ?`, recipeID).Scan(&site); err != nil { + t.Fatal(err) + } + return site + } + check := func(what, id string, leaks []string, want any) { + t.Helper() + rec, _ := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id, "") + if rec.Code != http.StatusOK { + t.Fatalf("GET %s = %d: %s", what, rec.Code, rec.Body) + } + for _, leak := range leaks { + if strings.Contains(strings.ToLower(rec.Body.String()), leak) { + t.Errorf("detail of %s contains %q: %s", what, leak, rec.Body) + } + } + raw, _ := getDetail(t, e.h, id) + if src, _ := raw["source"].(map[string]any); !reflect.DeepEqual(src, map[string]any{"site_name": want}) { + t.Errorf("source of %s = %v, want only site_name %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", `"url"`}, 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) + } + check("a page naming its site", namedID, []string{named, namedToken, "example.com", `"url"`}, "Curry House") + + 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) + } + check("a page named by a redirect's host", namedID, []string{redirectHost, "redirect-token", namedToken}, nil) +} + +// TestGetDinnerWhyPickedIsCreatingSuggestion: why_picked is the reason of +// the suggestion that created the dinner, never that of an earlier +// suggestion row for it that was declined. +func TestGetDinnerWhyPickedIsCreatingSuggestion(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + id := createDinner(t, e.h, `{"name":"Chicken curry"}`) + if _, err := e.st.Writer.Exec(`UPDATE dinners SET status = 'quest_log', origin = 'side_quest' WHERE id = ?`, id); err != nil { + t.Fatal(err) + } + q := db.New(e.st.Writer) + now := store.Now().String() + batch, err := q.CreateSuggestionBatch(t.Context(), db.CreateSuggestionBatchParams{ID: store.NewID(), Trigger: "manual", CreatedAt: now, UpdatedAt: now}) + if err != nil { + t.Fatal(err) + } + const reason = "Like your Tikka masala, but quicker." + // The declined row is created first, so it is the earliest by both + // created_at and id. + for _, s := range []struct{ reason, status string }{ + {"Passed on last week.", "declined"}, + {reason, "accepted"}, + } { + sid := store.NewID() + if err := q.CreateSuggestion(t.Context(), db.CreateSuggestionParams{ + ID: sid, DinnerID: id, BatchID: batch.ID, Name: "Chicken curry", + Reason: s.reason, CitedRegularIds: "[]", CreatedAt: now, UpdatedAt: now, + }); err != nil { + t.Fatal(err) + } + if n, err := q.DecideSuggestion(t.Context(), db.DecideSuggestionParams{ + Status: s.status, Now: sql.NullString{String: now, Valid: true}, ID: sid, + }); err != nil || n != 1 { + t.Fatalf("decide %s = %d, %v", s.status, n, err) + } + } + + if _, got := getDetail(t, e.h, id); got.WhyPicked == nil || *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) + } + } +} diff --git a/internal/server/dinners.go b/internal/server/dinners.go index fab52ac..5f0d54e 100644 --- a/internal/server/dinners.go +++ b/internal/server/dinners.go @@ -70,6 +70,24 @@ 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. +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")) + if err != nil { + writeDinnerError(w, r, logger, "get dinner", err) + return + } + writeJSON(w, r, logger, http.StatusOK, d) + } +} + // postDinner adds a Regular, recording the session's actor as updated_by, // and answers 201 with it and its grounding_job_id without waiting for the // job. When the job cannot be enqueued it still answers 201, with the dinner diff --git a/internal/server/dinners_test.go b/internal/server/dinners_test.go index 229b355..39a8712 100644 --- a/internal/server/dinners_test.go +++ b/internal/server/dinners_test.go @@ -86,13 +86,19 @@ func newDinnersEnv(t *testing.T) *dinnersEnv { t.Fatal(err) } e.runner = runner - g := recipestest.NewGrounder(t, st, t.TempDir(), e.search, e.pages, e.images, e.model) + // The router serves the photos the grounder caches. + dataDir := t.TempDir() + g := recipestest.NewGrounder(t, st, dataDir, e.search, e.pages, e.images, e.model) + photos, err := recipes.NewPhotoCache(e.images, st, dataDir) + if err != nil { + t.Fatal(err) + } svc, err := dinners.NewService(st, g, runner, logger) if err != nil { t.Fatal(err) } e.h, err = NewRouter(logger, "dev", nil, stubAuth{}, stubSessions{}, newTestSettings(t), newTestSearch(t), newTestAI(t), - svc, newTestSuggest(t), newTestPlan(t), newTestShopping(t), newTestGoogle(t), runner, newTestPhotos(t), fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) + svc, newTestSuggest(t), newTestPlan(t), newTestShopping(t), newTestGoogle(t), runner, photos, fstest.MapFS{"index.html": {Data: []byte(fixtureIndex)}}) if err != nil { t.Fatal(err) } diff --git a/internal/server/server.go b/internal/server/server.go index 931c1e3..ed3d05b 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -39,7 +39,7 @@ type RouteMounter interface { // /media; settings serves /api/v1/settings/household; search serves // /api/v1/settings/recipe-search and its /test; ai serves // /api/v1/settings/ai, its /api-key, /verify, /models and /reset-prompt; -// dinnerSvc serves GET and POST /api/v1/dinners, PATCH and DELETE +// dinnerSvc serves GET and POST /api/v1/dinners, GET, PATCH and DELETE // /api/v1/dinners/{id}, POST /api/v1/dinners/{id}/favorite, GET // /api/v1/dinners/{id}/recipe-candidates and POST /api/v1/dinners/{id}/ground; // suggestSvc serves POST /api/v1/suggestions/batches, GET @@ -130,6 +130,7 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Post("/settings/ai/reset-prompt", postAIResetPrompt(logger, ai)) apiV1.Get("/dinners", getDinners(logger, dinnerSvc)) apiV1.Post("/dinners", postDinner(logger, dinnerSvc)) + apiV1.Get("/dinners/{id}", getDinner(logger, dinnerSvc)) apiV1.Patch("/dinners/{id}", patchDinner(logger, dinnerSvc)) apiV1.Delete("/dinners/{id}", deleteDinner(logger, dinnerSvc)) apiV1.Post("/dinners/{id}/favorite", postDinnerFavorite(logger, dinnerSvc)) diff --git a/migrations/queries/suggestions.sql b/migrations/queries/suggestions.sql index 1068622..61aef9b 100644 --- a/migrations/queries/suggestions.sql +++ b/migrations/queries/suggestions.sql @@ -116,3 +116,14 @@ WHERE id = sqlc.arg(id) AND status = 'pending'; UPDATE dinners SET status = sqlc.arg(status), updated_by = sqlc.arg(updated_by), updated_at = sqlc.arg(updated_at) WHERE id = sqlc.arg(id) AND status = 'suggested'; + +-- name: GetDinnerSuggestionReasons :many +-- The reason of each suggestion of dinner_id that can have created it, +-- none for a dinner no suggestion created. A suggestion is only ever +-- created with a new dinner, and declining one archives its dinner, so a +-- live dinner's creating suggestion is pending (the dinner still +-- suggested) or accepted; a declined one did not create it. More than one +-- row means nothing records which did. +SELECT reason FROM suggestions +WHERE dinner_id = ? AND status IN ('pending', 'accepted') +ORDER BY id;