Guard recipe repairs against same-normalized renames and member edits
F1-1: GroundDinner also compares the observed raw name, so renaming "Grandma's stew" to "Easy Grandma's stew" (same normalized_name) while a link is grounding is ErrGroundConflict, not attached to the renamed dinner. F1-2: A method, minutes_total or difficulty no longer as observed before grounding is kept over the recipe's value, as GroundPendingDinner keeps values a member set while a dinner was pending. F2-1: Failing to record a grounded candidate or link on the dinner (or to read it back after a rejected write) wraps ErrGroundFailed, so the API logs reason=ground_failed with the dinner id, never the database error. Maestro task 8741cedb-1849-4e3f-8d05-f5648c77c7ee. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -254,10 +254,12 @@ automatic** (D-021).
|
||||
`dinner_archived`. A candidate or link is written only if the dinner is
|
||||
still as it was before grounding began: one renamed or grounded some
|
||||
other way meanwhile is 409 `ground_conflict`, and one archived meanwhile
|
||||
409 `dinner_archived`, either leaving the dinner unchanged. Any other
|
||||
failure grounding a candidate or link is 500, logged with a fixed reason
|
||||
and the dinner id only — never the link, which can carry a personal
|
||||
token, or dependency error text.
|
||||
409 `dinner_archived`, either leaving the dinner unchanged. A method,
|
||||
minutes or difficulty a member edits meanwhile is kept over the recipe's.
|
||||
Any other failure grounding a candidate or link, or recording it on the
|
||||
dinner, is 500, logged with a fixed reason and the dinner id only —
|
||||
never the link, which can carry a personal token, or dependency error
|
||||
text.
|
||||
- `/health` — liveness.
|
||||
- Everything else serves the embedded PWA with SPA fallback.
|
||||
|
||||
|
||||
+26
-13
@@ -251,32 +251,40 @@ UPDATE dinners
|
||||
SET recipe_id = ?1,
|
||||
recipe_url = NULL,
|
||||
photo_id = ?2,
|
||||
method = ?3,
|
||||
minutes_total = ?4,
|
||||
difficulty = ?5,
|
||||
grounding_status = ?6,
|
||||
method = CASE WHEN method IS ?3
|
||||
THEN ?4 ELSE method END,
|
||||
minutes_total = CASE WHEN minutes_total IS ?5
|
||||
THEN ?6 ELSE minutes_total END,
|
||||
difficulty = CASE WHEN difficulty IS ?7
|
||||
THEN ?8 ELSE difficulty END,
|
||||
grounding_status = ?9,
|
||||
grounding_error = NULL,
|
||||
updated_by = ?7,
|
||||
updated_at = ?8
|
||||
WHERE id = ?9
|
||||
updated_by = ?10,
|
||||
updated_at = ?11
|
||||
WHERE id = ?12
|
||||
AND status <> 'archived'
|
||||
AND normalized_name = ?10
|
||||
AND grounding_status = ?11
|
||||
AND grounding_revision = ?12
|
||||
AND recipe_id IS ?13
|
||||
AND name = ?13
|
||||
AND normalized_name = ?14
|
||||
AND grounding_status = ?15
|
||||
AND grounding_revision = ?16
|
||||
AND recipe_id IS ?17
|
||||
RETURNING id, name, normalized_name, status, origin, method, minutes_total, difficulty, cuisine, is_favorite, recipe_id, photo_id, grounding_status, updated_by, created_at, updated_at, recipe_url, grounding_error, grounding_revision
|
||||
`
|
||||
|
||||
type GroundDinnerParams struct {
|
||||
RecipeID sql.NullString
|
||||
PhotoID sql.NullString
|
||||
ObservedMethod sql.NullString
|
||||
Method sql.NullString
|
||||
ObservedMinutesTotal sql.NullInt64
|
||||
MinutesTotal sql.NullInt64
|
||||
ObservedDifficulty sql.NullString
|
||||
Difficulty sql.NullString
|
||||
GroundingStatus string
|
||||
UpdatedBy sql.NullString
|
||||
UpdatedAt string
|
||||
ID string
|
||||
ObservedName string
|
||||
ObservedNormalizedName string
|
||||
ObservedGroundingStatus string
|
||||
ObservedGroundingRevision int64
|
||||
@@ -286,21 +294,26 @@ type GroundDinnerParams struct {
|
||||
// Records a dinner's grounding by hand (grounded or manual) and clears any
|
||||
// grounding_error and pasted recipe_url: the recipe chosen replaces the
|
||||
// link. It writes only an unarchived dinner still as it was observed
|
||||
// before grounding (same normalized_name, grounding_status,
|
||||
// before grounding (same name, normalized_name, grounding_status,
|
||||
// grounding_revision and recipe_id), so a dinner renamed, archived or
|
||||
// grounded some other way meanwhile is left as it is. That dinner, or an
|
||||
// unknown id, returns sql.ErrNoRows.
|
||||
// unknown id, returns sql.ErrNoRows. A method, minutes_total or difficulty
|
||||
// no longer as observed is kept: a member edited it while it was grounding.
|
||||
func (q *Queries) GroundDinner(ctx context.Context, arg GroundDinnerParams) (Dinner, error) {
|
||||
row := q.db.QueryRowContext(ctx, groundDinner,
|
||||
arg.RecipeID,
|
||||
arg.PhotoID,
|
||||
arg.ObservedMethod,
|
||||
arg.Method,
|
||||
arg.ObservedMinutesTotal,
|
||||
arg.MinutesTotal,
|
||||
arg.ObservedDifficulty,
|
||||
arg.Difficulty,
|
||||
arg.GroundingStatus,
|
||||
arg.UpdatedBy,
|
||||
arg.UpdatedAt,
|
||||
arg.ID,
|
||||
arg.ObservedName,
|
||||
arg.ObservedNormalizedName,
|
||||
arg.ObservedGroundingStatus,
|
||||
arg.ObservedGroundingRevision,
|
||||
|
||||
@@ -70,10 +70,11 @@ type Querier interface {
|
||||
// Records a dinner's grounding by hand (grounded or manual) and clears any
|
||||
// grounding_error and pasted recipe_url: the recipe chosen replaces the
|
||||
// link. It writes only an unarchived dinner still as it was observed
|
||||
// before grounding (same normalized_name, grounding_status,
|
||||
// before grounding (same name, normalized_name, grounding_status,
|
||||
// grounding_revision and recipe_id), so a dinner renamed, archived or
|
||||
// grounded some other way meanwhile is left as it is. That dinner, or an
|
||||
// unknown id, returns sql.ErrNoRows.
|
||||
// unknown id, returns sql.ErrNoRows. A method, minutes_total or difficulty
|
||||
// no longer as observed is kept: a member edited it while it was grounding.
|
||||
GroundDinner(ctx context.Context, arg GroundDinnerParams) (Dinner, error)
|
||||
// Records the background grounding of an unarchived dinner still pending at
|
||||
// the grounding_revision it was grounded for; a dinner grounded some other
|
||||
|
||||
@@ -335,14 +335,16 @@ type ManualRecipe struct {
|
||||
// Ground grounds dinner id as f says, recording updatedBy. A candidate or
|
||||
// link is grounded with recipes.Grounder.GroundURL and sets
|
||||
// grounding_status grounded, with the recipe's photo, method, minutes and
|
||||
// difficulty. A manual recipe is stored as typed — no model runs — and
|
||||
// sets grounding_status manual with its method and minutes; it has no
|
||||
// photo, and the dinner keeps its difficulty. An invalid f is a
|
||||
// *ValidationError and an archived dinner ErrArchived. A failure grounding
|
||||
// a candidate or link wraps ErrGroundFailed; a page that cannot ground the
|
||||
// dinner is also a *recipes.ErrUngroundable. A dinner renamed, archived or
|
||||
// grounded some other way while a candidate or link was grounding is
|
||||
// ErrGroundConflict or ErrArchived. None of these changes the dinner.
|
||||
// difficulty; one a member changed while it was grounding is kept. A
|
||||
// manual recipe is stored as typed — no model runs — and sets
|
||||
// grounding_status manual with its method and minutes; it has no photo,
|
||||
// and the dinner keeps its difficulty. An invalid f is a *ValidationError
|
||||
// and an archived dinner ErrArchived. A failure grounding a candidate or
|
||||
// link, or recording it on the dinner, wraps ErrGroundFailed; a page that
|
||||
// cannot ground the dinner is also a *recipes.ErrUngroundable. A dinner
|
||||
// renamed, archived or grounded some other way while a candidate or link
|
||||
// was grounding is ErrGroundConflict or ErrArchived. None of these changes
|
||||
// the dinner.
|
||||
func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string) (Dinner, error) {
|
||||
if updatedBy == "" {
|
||||
return Dinner{}, errors.New("dinners: ground: no member to record as updated_by")
|
||||
@@ -380,27 +382,35 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
|
||||
UpdatedBy: sql.NullString{String: updatedBy, Valid: true},
|
||||
UpdatedAt: store.Now().String(),
|
||||
ID: cur.ID,
|
||||
ObservedName: cur.Name,
|
||||
ObservedNormalizedName: cur.NormalizedName,
|
||||
ObservedGroundingStatus: cur.GroundingStatus,
|
||||
ObservedGroundingRevision: cur.GroundingRevision,
|
||||
ObservedRecipeID: cur.RecipeID,
|
||||
ObservedMethod: cur.Method,
|
||||
ObservedMinutesTotal: cur.MinutesTotal,
|
||||
ObservedDifficulty: cur.Difficulty,
|
||||
})
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return Dinner{}, s.groundRejected(ctx, id)
|
||||
}
|
||||
if err != nil {
|
||||
return Dinner{}, fmt.Errorf("dinners: ground %s: %w", id, err)
|
||||
return Dinner{}, fmt.Errorf("%w: dinner %s: record: %w", ErrGroundFailed, id, err)
|
||||
}
|
||||
return fromRow(row), nil
|
||||
}
|
||||
|
||||
// groundRejected is the error for dinner id when it was not as observed
|
||||
// before grounding: ErrNotFound, ErrArchived or ErrGroundConflict.
|
||||
// before grounding: ErrNotFound, ErrArchived or ErrGroundConflict. Failing
|
||||
// to read it wraps ErrGroundFailed.
|
||||
func (s *Service) groundRejected(ctx context.Context, id string) error {
|
||||
cur, err := s.get(ctx, id)
|
||||
if err != nil {
|
||||
if errors.Is(err, ErrNotFound) {
|
||||
return err
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
|
||||
}
|
||||
if cur.Status == StatusArchived {
|
||||
return ErrArchived
|
||||
}
|
||||
@@ -486,10 +496,14 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
|
||||
UpdatedBy: sql.NullString{String: updatedBy, Valid: true},
|
||||
UpdatedAt: now,
|
||||
ID: cur.ID,
|
||||
ObservedName: cur.Name,
|
||||
ObservedNormalizedName: cur.NormalizedName,
|
||||
ObservedGroundingStatus: cur.GroundingStatus,
|
||||
ObservedGroundingRevision: cur.GroundingRevision,
|
||||
ObservedRecipeID: cur.RecipeID,
|
||||
ObservedMethod: cur.Method,
|
||||
ObservedMinutesTotal: cur.MinutesTotal,
|
||||
ObservedDifficulty: cur.Difficulty,
|
||||
})
|
||||
if err != nil {
|
||||
return fmt.Errorf("dinners: ground %s: %w", id, err)
|
||||
|
||||
@@ -1126,6 +1126,13 @@ func TestFixInFlightWhenDinnerChanges(t *testing.T) {
|
||||
}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
|
||||
return row.Name == "Chicken curry" && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
|
||||
}},
|
||||
{"renamed to the same normalized name", func(t *testing.T, e *env, id string) {
|
||||
if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Name: ptr("Easy Grandma's stew")}, member); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}, dinners.ErrGroundConflict, func(row db.Dinner) bool {
|
||||
return row.Name == "Easy Grandma's stew" && row.GroundingStatus == dinners.GroundingFailed && !row.RecipeID.Valid
|
||||
}},
|
||||
{"archived", func(t *testing.T, e *env, id string) {
|
||||
if _, err := e.svc.Archive(t.Context(), id, member); err != nil {
|
||||
t.Fatal(err)
|
||||
@@ -1170,6 +1177,40 @@ func TestFixInFlightWhenDinnerChanges(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestFixInFlightKeepsMemberEdits: a method or difficulty a member sets
|
||||
// while a link is grounding is kept; what they left alone takes the link's
|
||||
// recipe.
|
||||
func TestFixInFlightKeepsMemberEdits(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := newEnv(t)
|
||||
id := failedDinner(t, e, "Grandma's stew")
|
||||
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
|
||||
e.pages.Gate = make(chan struct{})
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := e.svc.Ground(context.Background(), id, dinners.Fix{RecipeURL: ptr(stew)}, member)
|
||||
done <- err
|
||||
}()
|
||||
e.waitFetched(t, stew)
|
||||
if _, err := e.svc.Update(t.Context(), id, dinners.Patch{Method: ptr("grill"), Difficulty: ptr("easy")}, member); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
close(e.pages.Gate)
|
||||
|
||||
if err := <-done; err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
row := e.dinner(t, id)
|
||||
if row.GroundingStatus != dinners.GroundingGrounded || !row.RecipeID.Valid {
|
||||
t.Errorf("dinner = %+v, want grounded in the link's recipe", row)
|
||||
}
|
||||
if row.Method.String != "grill" || row.Difficulty.String != "easy" || row.MinutesTotal.Int64 != 150 {
|
||||
t.Errorf("method, difficulty, minutes = %q, %q, %d; want grill and easy kept, 150 from the recipe",
|
||||
row.Method.String, row.Difficulty.String, row.MinutesTotal.Int64)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFixArchivedDinner: an archived dinner cannot be grounded by hand.
|
||||
func TestFixArchivedDinner(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -252,6 +252,33 @@ func TestPostGroundViaRecipeURLStorageFailure(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestPostGroundViaRecipeURLDinnerWriteFailure: a pasted link whose recipe
|
||||
// is stored but whose dinner cannot be written is a 500 logged by reason
|
||||
// and dinner id, never with the dependency error.
|
||||
func TestPostGroundViaRecipeURLDinnerWriteFailure(t *testing.T) {
|
||||
t.Parallel()
|
||||
e := newDinnersEnv(t).withRecipes(t)
|
||||
id := createDinner(t, e.h, `{"name":"Sunday stew"}`)
|
||||
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
|
||||
if _, err := e.st.Writer.ExecContext(t.Context(),
|
||||
`CREATE TRIGGER refuse_grounding BEFORE UPDATE OF recipe_id ON dinners WHEN NEW.recipe_id IS NOT NULL BEGIN SELECT RAISE(ABORT, 'disk full'); END`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
rec, body := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"recipe_url":"`+stewPage+`"}`)
|
||||
if rec.Code != http.StatusInternalServerError || errorCode(body) != "internal" {
|
||||
t.Errorf("POST = %d %s, want 500 internal", rec.Code, rec.Body)
|
||||
}
|
||||
var stored int
|
||||
if err := e.st.Reader.QueryRowContext(t.Context(), `SELECT COUNT(*) FROM recipes WHERE source_url = ?`, stewPage).Scan(&stored); err != nil || stored != 1 {
|
||||
t.Fatalf("recipes stored for the link = %d (%v), want 1: the failure must be the dinner write", stored, err)
|
||||
}
|
||||
logs := e.logs.String()
|
||||
if strings.Contains(logs, "disk full") || !strings.Contains(logs, "reason=ground_failed") || !strings.Contains(logs, "dinner_id="+id) {
|
||||
t.Errorf("logs = %q, want reason=ground_failed and the dinner id, without the dependency error", logs)
|
||||
}
|
||||
}
|
||||
|
||||
// TestPostGroundArchived: grounding an archived dinner by hand is a 409.
|
||||
func TestPostGroundArchived(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -85,23 +85,28 @@ RETURNING *;
|
||||
-- Records a dinner's grounding by hand (grounded or manual) and clears any
|
||||
-- grounding_error and pasted recipe_url: the recipe chosen replaces the
|
||||
-- link. It writes only an unarchived dinner still as it was observed
|
||||
-- before grounding (same normalized_name, grounding_status,
|
||||
-- before grounding (same name, normalized_name, grounding_status,
|
||||
-- grounding_revision and recipe_id), so a dinner renamed, archived or
|
||||
-- grounded some other way meanwhile is left as it is. That dinner, or an
|
||||
-- unknown id, returns sql.ErrNoRows.
|
||||
-- unknown id, returns sql.ErrNoRows. A method, minutes_total or difficulty
|
||||
-- no longer as observed is kept: a member edited it while it was grounding.
|
||||
UPDATE dinners
|
||||
SET recipe_id = sqlc.arg(recipe_id),
|
||||
recipe_url = NULL,
|
||||
photo_id = sqlc.arg(photo_id),
|
||||
method = sqlc.arg(method),
|
||||
minutes_total = sqlc.arg(minutes_total),
|
||||
difficulty = sqlc.arg(difficulty),
|
||||
method = CASE WHEN method IS sqlc.narg(observed_method)
|
||||
THEN sqlc.arg(method) ELSE method END,
|
||||
minutes_total = CASE WHEN minutes_total IS sqlc.narg(observed_minutes_total)
|
||||
THEN sqlc.arg(minutes_total) ELSE minutes_total END,
|
||||
difficulty = CASE WHEN difficulty IS sqlc.narg(observed_difficulty)
|
||||
THEN sqlc.arg(difficulty) ELSE difficulty END,
|
||||
grounding_status = sqlc.arg(grounding_status),
|
||||
grounding_error = NULL,
|
||||
updated_by = sqlc.arg(updated_by),
|
||||
updated_at = sqlc.arg(updated_at)
|
||||
WHERE id = sqlc.arg(id)
|
||||
AND status <> 'archived'
|
||||
AND name = sqlc.arg(observed_name)
|
||||
AND normalized_name = sqlc.arg(observed_normalized_name)
|
||||
AND grounding_status = sqlc.arg(observed_grounding_status)
|
||||
AND grounding_revision = sqlc.arg(observed_grounding_revision)
|
||||
|
||||
Reference in New Issue
Block a user