Keep candidates found at a rename's revision when clearing offers
A candidate search that read a dinner after its rename committed could store fresh offers that the rename's own clear then dropped, so a valid candidate for the renamed dinner was refused with a 400. clearOffers now takes the revision below which offers are stale: a rename passes its committed revision, so it drops only offers and in-flight searches from earlier revisions, while grounding by hand and archiving pass math.MaxInt64 and clear everything as before. A search finishing late at an earlier revision no longer replaces offers already found at a later one. Fixes MQ-103-02. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -149,7 +149,7 @@ type Service struct {
|
||||
// id, the candidates last offered by Candidates until they expire, so
|
||||
// a candidate id is resolved here and its URL never leaves the server.
|
||||
// offerSearches holds the Candidates searches running, by dinner id,
|
||||
// so clearing a dinner's offers keeps them from storing theirs.
|
||||
// so clearing a dinner's offers keeps them from storing stale ones.
|
||||
offersMu sync.Mutex
|
||||
offers map[string]*offerSet
|
||||
offerSearches map[string]map[*offerSearch]struct{}
|
||||
@@ -388,7 +388,7 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri
|
||||
}
|
||||
if renamed {
|
||||
updateCommitted()
|
||||
s.clearOffers(row.ID)
|
||||
s.clearOffers(row.ID, row.GroundingRevision)
|
||||
}
|
||||
if regrounding {
|
||||
s.stopGroundings(row.ID, row.GroundingRevision)
|
||||
@@ -426,7 +426,7 @@ func (s *Service) Archive(ctx context.Context, id, updatedBy string) (Dinner, er
|
||||
return Dinner{}, fmt.Errorf("dinners: archive %s: %w", id, err)
|
||||
}
|
||||
s.stopGroundings(row.ID, math.MaxInt64)
|
||||
s.clearOffers(row.ID)
|
||||
s.clearOffers(row.ID, math.MaxInt64)
|
||||
return fromRow(row), nil
|
||||
}
|
||||
|
||||
|
||||
@@ -49,6 +49,14 @@ func (s *Service) HasOffers(id string) bool {
|
||||
return ok
|
||||
}
|
||||
|
||||
// StoreOffers stores urls as dinner id's offers, as a Candidates search
|
||||
// that read the dinner at grounding_revision revision would on finishing.
|
||||
func (s *Service) StoreOffers(id string, urls map[string]string, revision int64) {
|
||||
keep, done := s.trackOffers(id)
|
||||
defer done()
|
||||
keep(urls, revision)
|
||||
}
|
||||
|
||||
// OfferCount is the number of dinners whose candidates the service holds.
|
||||
func (s *Service) OfferCount() int {
|
||||
s.offersMu.Lock()
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math"
|
||||
"slices"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -319,10 +320,11 @@ type offerSet struct {
|
||||
expiry *time.Timer
|
||||
}
|
||||
|
||||
// offerSearch is a Candidates search running for a dinner. It is stale
|
||||
// once the dinner's offers are cleared, and then stores none.
|
||||
// offerSearch is a Candidates search running for a dinner. It stores no
|
||||
// offers found at a grounding_revision below below, which clearing the
|
||||
// dinner's offers raises.
|
||||
type offerSearch struct {
|
||||
stale bool
|
||||
below int64
|
||||
}
|
||||
|
||||
// Candidates searches for dinner id's name and returns up to MaxCandidates
|
||||
@@ -333,7 +335,7 @@ type offerSearch struct {
|
||||
// search that fails wraps recipes.ErrSearchFailed.
|
||||
func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error) {
|
||||
// Tracked before the dinner is read: grounding, renaming or archiving
|
||||
// it after the read makes this search stale.
|
||||
// it after the read keeps this search from storing what it found.
|
||||
keep, done := s.trackOffers(id)
|
||||
defer done()
|
||||
row, err := s.get(ctx, id)
|
||||
@@ -363,8 +365,9 @@ func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error
|
||||
|
||||
// trackOffers registers a Candidates search for dinner id. It returns keep,
|
||||
// which stores urls, found for the dinner at grounding_revision revision,
|
||||
// as its offers for offerLifetime unless clearOffers has run for it since,
|
||||
// and done, to call when the search ends.
|
||||
// as its offers for offerLifetime unless clearOffers has run for it since
|
||||
// with a revision above that, or offers found at a later revision are
|
||||
// stored, and done, to call when the search ends.
|
||||
func (s *Service) trackOffers(id string) (keep func(urls map[string]string, revision int64), done func()) {
|
||||
search := &offerSearch{}
|
||||
s.offersMu.Lock()
|
||||
@@ -376,7 +379,12 @@ func (s *Service) trackOffers(id string) (keep func(urls map[string]string, revi
|
||||
keep = func(urls map[string]string, revision int64) {
|
||||
s.offersMu.Lock()
|
||||
defer s.offersMu.Unlock()
|
||||
if search.stale {
|
||||
if revision < search.below {
|
||||
return
|
||||
}
|
||||
if set := s.offers[id]; set != nil && set.revision > revision {
|
||||
// Found at a later revision, so the earlier search's offers
|
||||
// are already stale.
|
||||
return
|
||||
}
|
||||
s.dropOffers(id)
|
||||
@@ -403,15 +411,20 @@ func (s *Service) trackOffers(id string) (keep func(urls map[string]string, revi
|
||||
return keep, done
|
||||
}
|
||||
|
||||
// clearOffers drops dinner id's offers and makes every Candidates search
|
||||
// running for it stale, so no candidate offered before grounds it. Call
|
||||
// it once the dinner is grounded by hand, renamed or archived.
|
||||
func (s *Service) clearOffers(id string) {
|
||||
// clearOffers drops dinner id's offers found at a grounding_revision below
|
||||
// below and keeps every Candidates search running for it from storing
|
||||
// such offers, so none of them grounds it. Offers found at below or above
|
||||
// are kept. Call it with math.MaxInt64 once the dinner is grounded by hand
|
||||
// or archived, and with its new revision once it is renamed: a search
|
||||
// that read the rename meanwhile keeps its offers.
|
||||
func (s *Service) clearOffers(id string, below int64) {
|
||||
s.offersMu.Lock()
|
||||
defer s.offersMu.Unlock()
|
||||
s.dropOffers(id)
|
||||
if set := s.offers[id]; set != nil && set.revision < below {
|
||||
s.dropOffers(id)
|
||||
}
|
||||
for search := range s.offerSearches[id] {
|
||||
search.stale = true
|
||||
search.below = max(search.below, below)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -547,7 +560,7 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
|
||||
if err != nil {
|
||||
return Dinner{}, fmt.Errorf("%w: dinner %s: record: %w", ErrGroundFailed, id, err)
|
||||
}
|
||||
s.clearOffers(cur.ID)
|
||||
s.clearOffers(cur.ID, math.MaxInt64)
|
||||
return fromRow(row), nil
|
||||
}
|
||||
|
||||
@@ -670,7 +683,7 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u
|
||||
if err != nil {
|
||||
return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
|
||||
}
|
||||
s.clearOffers(row.ID)
|
||||
s.clearOffers(row.ID, math.MaxInt64)
|
||||
return fromRow(row), nil
|
||||
}
|
||||
|
||||
|
||||
@@ -959,6 +959,55 @@ func TestRenameSameNormalizedNameRefusesOldCandidates(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCandidatesBetweenRenameAndClearKept: a candidate search that reads
|
||||
// a dinner after its rename commits, and stores its candidates before the
|
||||
// rename clears the dinner's offers, keeps them: they ground the renamed
|
||||
// dinner, while one offered before the rename does not, even when a search
|
||||
// that read the dinner before the rename finishes after them.
|
||||
func TestCandidatesBetweenRenameAndClearKept(t *testing.T) {
|
||||
// Unstarted, so the dinner stays pending with its jobs queued.
|
||||
e := newUnstartedEnv(t)
|
||||
e.search.Results["Beef stew"] = []recipes.Candidate{{URL: stew}}
|
||||
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry")}, member)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
old := candidateID(t, e, c.ID, "Chicken Curry")
|
||||
before := e.dinner(t, c.ID)
|
||||
|
||||
var fresh string
|
||||
dinners.SetUpdateCommitted(t, func() {
|
||||
fresh = candidateID(t, e, c.ID, "Beef Stew")
|
||||
e.svc.StoreOffers(c.ID, map[string]string{old: curryA}, before.GroundingRevision)
|
||||
})
|
||||
if _, err := e.svc.Update(t.Context(), c.ID, dinners.Patch{Name: ptr("Beef stew")}, member); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if fresh == "" {
|
||||
t.Fatal("no candidate search ran between the rename and clearing offers")
|
||||
}
|
||||
if !e.svc.HasOffers(c.ID) {
|
||||
t.Fatalf("clearing offers after the rename dropped the candidates found at its revision for dinner %s", c.ID)
|
||||
}
|
||||
wantStaleCandidate(t, e, c.ID, old)
|
||||
|
||||
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
|
||||
g, err := e.svc.Ground(t.Context(), c.ID, dinners.Fix{CandidateID: ptr(fresh)}, member)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if g.GroundingStatus != dinners.GroundingGrounded || g.RecipeID == nil {
|
||||
t.Fatalf("dinner = %+v, want grounded for the new name", g)
|
||||
}
|
||||
rec, err := db.New(e.st.Reader).GetRecipe(t.Context(), *g.RecipeID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if rec.SourceUrl != stew {
|
||||
t.Errorf("recipe source = %s, want %s", rec.SourceUrl, stew)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRenameBackDuringCandidatesRefusesThem: a candidate search that read
|
||||
// a dinner as A and finishes after renames A -> B -> A have committed, but
|
||||
// before either has cleared the dinner's offers, stores candidates that
|
||||
|
||||
Reference in New Issue
Block a user