diff --git a/modules/mesh-delivery-indexer/internal/index/oldsections.go b/modules/mesh-delivery-indexer/internal/index/oldsections.go index 14a6db51..91d16b98 100644 --- a/modules/mesh-delivery-indexer/internal/index/oldsections.go +++ b/modules/mesh-delivery-indexer/internal/index/oldsections.go @@ -5,9 +5,7 @@ import ( "encoding/json" "fmt" "maps" - "slices" - "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-sdk/go/delivery" "mesh-delivery-indexer/judge" @@ -44,21 +42,21 @@ func DecodeOldSections(value []byte) (OldSectionsCount, error) { return c, nil } -// countBefore counts the accepted modules using each old section as the pass starts: those whose verdict held says accepted, -// or which have none and are not fresh (acceptedBefore). A manifest is read leniently, since a module the judge now finds -// a problem in is still accepted (ADR 0579 ยง1) and still counts. -func (ix *Indexer) countBefore(ctx context.Context, raws map[string][]byte, digests map[string]string, fresh map[string]bool) judge.OldSectionCounts { - var accepted []catalogue.Manifest - for _, name := range slices.Sorted(maps.Keys(raws)) { - if !ix.acceptedBefore(ctx, digests[name], fresh) { - continue - } - if m, err := catalogue.Decode(raws[name]); err == nil { - m.Module = name - accepted = append(accepted, m) +// heldOldSections is the count the last pass wrote (the accepted set it held): a module that keeps using a section was +// accepted with it, so it is counted, and a fresh manifest changes nothing in it. Nil when none is held yet or it cannot be +// read; a failed read is said, never taken as 0 (issue 1966 D4). +func (ix *Indexer) heldOldSections(ctx context.Context) judge.OldSectionCounts { + value, _, found, err := ix.OldSections.Get(ctx, OldSectionsKey) + if err == nil && found { + var c OldSectionsCount + if c, err = DecodeOldSections(value); err == nil { + return c.Counts } } - return judge.CountOldSections(accepted) + if err != nil { + ix.logf("judge: the count of the old sections cannot be read (%v), so the zero rule is not applied in this pass", err) + } + return nil } // keepOldSections writes the counts under OldSectionsKey when they differ from the ones held. diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts.go b/modules/mesh-delivery-indexer/internal/index/verdicts.go index 03138bd2..64d5f825 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts.go @@ -75,10 +75,10 @@ func (ix *Indexer) JudgeAll(ctx context.Context, fresh map[string]bool) (Judged, digests[name] = digest } predefined := ix.predefinedList(ctx) - // The old sections end when no module is left in them (issue 1966 D4): counted from the verdicts held before this pass. + // The old sections end when no module is left in them (issue 1966 D4): the count is the accepted set the last pass held. var counts judge.OldSectionCounts if ix.OldSections != nil { - counts = ix.countBefore(ctx, raws, digests, fresh) + counts = ix.heldOldSections(ctx) } refused, unnamed := judge.ShelfCounting(raws, predefined, 0, counts) for _, p := range unnamed { diff --git a/modules/mesh-delivery-indexer/internal/index/verdicts_test.go b/modules/mesh-delivery-indexer/internal/index/verdicts_test.go index 380ca073..2e84f954 100644 --- a/modules/mesh-delivery-indexer/internal/index/verdicts_test.go +++ b/modules/mesh-delivery-indexer/internal/index/verdicts_test.go @@ -2,6 +2,8 @@ package index import ( "context" + "errors" + "fmt" "strings" "testing" "time" @@ -168,9 +170,42 @@ func TestTheIndexerJudgesEveryManifestOnce(t *testing.T) { if v := verdict(grants); v.Refused() { t.Fatalf("the foundation's manifest was refused: %+v", v) } + // 1966/AC11: the sole user of an old section changes its manifest and keeps the section: accepted, since it was accepted + // with it (the count is the accepted set held from the last pass, not recounted from fresh digests). + hold("solo", `{"module":"solo","version":"1","objects":[{"name":"o","max-mib":1}]}`) + if _, err := ix.JudgeAll(ctx, nil); err != nil { + t.Fatal(err) + } + solo2 := hold("solo", `{"module":"solo","version":"2","objects":[{"name":"o","max-mib":2}]}`) + if _, err := ix.JudgeAll(ctx, map[string]bool{solo2: true}); err != nil { + t.Fatal(err) + } + if v := verdict(solo2); v.Refused() { + t.Fatalf("the sole user of objects, changing its manifest and keeping objects, was refused: %+v", v) + } + // A count that cannot be read is never taken as 0: the zero rule is not applied in that pass and the log says so. + var logged []string + ix.Logf = func(f string, a ...any) { logged = append(logged, fmt.Sprintf(f, a...)) } + held := ix.OldSections + ix.OldSections = unreadable{held} + retry := hold("late", `{"module":"late","version":"2","logs":[{"name":"l"}]}`) + if _, err := ix.JudgeAll(ctx, map[string]bool{retry: true}); err != nil { + t.Fatal(err) + } + if v := verdict(retry); v.Refused() || !strings.Contains(strings.Join(logged, "\n"), "the zero rule is not applied") { + t.Fatalf("a count that could not be read: %+v, logged %q", v, logged) + } + ix.OldSections = held // Without the verdicts state the indexer judges nothing. ix.Verdicts = nil if j, err := ix.JudgeAll(ctx, nil); err != nil || j.Judged != 0 { t.Fatalf("without the state: %+v %v", j, err) } } + +// unreadable is a store whose reads fail. +type unreadable struct{ Store } + +func (unreadable) Get(context.Context, string) ([]byte, uint64, bool, error) { + return nil, 0, false, errors.New("the bus did not answer") +}