A beads/cache.go => beads/cache.go +103 -0
@@ 0,0 1,103 @@
+package beads
+
+import (
+ "sync"
+ "time"
+)
+
+// --- the cache the cross-database readings share ------------------------------
+//
+// Two readings here open every database one caller may browse: /ready's
+// aggregation (ReadyAcross) and the prefix index behind cross-database issue
+// links (PrefixesAcross). Opening N stores per request is exactly what the
+// per-request browse discipline does not scale to, and both are bounded the same
+// way:
+//
+// 1. A head-hash gate. Opening a session and listing branches is cheap; reading
+// and projecting rows is not. When the head has not moved, the cached
+// projection stands and no row is read.
+// 2. A TTL (ReadyCacheTTL), so a cache can never be the reason a reader sees
+// yesterday's answer.
+// 3. A ceiling on how many databases one call opens (ReadyMaxDatabases),
+// applied by the aggregations themselves.
+//
+// The bounds are one set of numbers, named in ready.go where /ready needed them
+// first. What differs between the two readings is only the projection being
+// cached, which is what the type parameter is: a second cache with its own
+// lifetime rules is how two pages start disagreeing about how fresh "fresh" is.
+//
+// What is cached is always a projection and never an open session. An open store
+// is a file handle and a memory mapping; caching those is the thing per-request
+// opening exists to prevent.
+
+// cached is one database's cached projection plus the two facts the bounds are
+// checked against: the head it was read at, and when it was stored.
+type cached[T any] struct {
+ head string
+ at time.Time
+ value T
+}
+
+// projectionCache is a small mutex-guarded map from repository id to one cached
+// projection. The repository id is the identity it is keyed on: no two databases
+// share it and it survives a rename.
+//
+// The zero value is usable — the map is allocated on first store — so a cache
+// can be a field of a value that has no constructor.
+type projectionCache[T any] struct {
+ mu sync.Mutex
+ entries map[int]cached[T]
+}
+
+// lookup returns the cached projection for a database when it was read at the
+// same head and has not expired. Both conditions, not either: the head hash is
+// what makes it correct, the TTL is what makes it bounded.
+func (c *projectionCache[T]) lookup(id int, head string, now time.Time) (T, bool) {
+ var zero T
+ if head == "" {
+ // A database whose head cannot be named cannot be gated on one.
+ return zero, false
+ }
+ c.mu.Lock()
+ defer c.mu.Unlock()
+ e, ok := c.entries[id]
+ if !ok || e.head != head || now.Sub(e.at) >= ReadyCacheTTL {
+ return zero, false
+ }
+ return e.value, true
+}
+
+// store records a projection read at head, dropping expired entries — and, if
+// that was not enough, everything — when the map is at its ceiling. A cache is
+// not a store: over the ceiling it starts again rather than growing with the
+// instance.
+func (c *projectionCache[T]) store(id int, head string, now time.Time, value T) {
+ if head == "" {
+ return
+ }
+ c.mu.Lock()
+ defer c.mu.Unlock()
+ if c.entries == nil {
+ c.entries = make(map[int]cached[T])
+ }
+ if len(c.entries) >= readyCacheMaxEntries {
+ for k, old := range c.entries {
+ if now.Sub(old.at) >= ReadyCacheTTL {
+ delete(c.entries, k)
+ }
+ }
+ if len(c.entries) >= readyCacheMaxEntries {
+ c.entries = make(map[int]cached[T], readyCacheMaxEntries)
+ }
+ }
+ c.entries[id] = cached[T]{head: head, at: now, value: value}
+}
+
+// size reports how many entries the cache holds. It exists for the tests that
+// assert the ceiling: the bound is the point of the map, and only a count can
+// check it.
+func (c *projectionCache[T]) size() int {
+ c.mu.Lock()
+ defer c.mu.Unlock()
+ return len(c.entries)
+}
A beads/prefixes.go => beads/prefixes.go +254 -0
@@ 0,0 1,254 @@
+package beads
+
+import (
+ "context"
+ "regexp"
+ "sort"
+ "strings"
+ "time"
+
+ "sourcecraft.dev/bigbes/sr-ht-dolt/browse"
+)
+
+// --- which database owns "<prefix>-<id>" --------------------------------------
+//
+// A global-tracker issue that says "blocked by artifacts-nex.2" is naming a row
+// in another database, and without this the reader has to work out which one and
+// go there by hand. Two levels of tracker only pay for themselves if they point
+// at each other.
+//
+// This file answers the one question that needs several databases: given an
+// id-shaped string in some text, which database — of the ones this caller may
+// browse — owns it. Whether the answer becomes a link, and what that link looks
+// like, is the renderer's business; this package renders nothing.
+//
+// Authorization is not here, exactly as it is not in ReadyAcross: handing this
+// function a database is the statement that the caller may read it. A database
+// the caller may not browse is one the caller's index has never heard of, which
+// is what makes an id belonging to it plain text and not a hint that it exists.
+
+// prefixKey is the config row every beads database stores its own id prefix in
+// ("global", "artifacts", "sr-ht-dolt", …). It sits in the same config table the
+// memories do, beside compact_tier2_days and the rest of the tracker's settings.
+const prefixKey = "issue_prefix"
+
+// PrefixCache is the prefix index's projection cache: one prefix per database,
+// gated on the head hash and expiring on ReadyCacheTTL — /ready's bounds and not
+// a second set (see cache.go). The zero value is usable, so a holder that has no
+// constructor can carry one as a field.
+type PrefixCache struct {
+ projectionCache[string]
+}
+
+// PrefixIndex is prefix → the database that owns it, over the databases one
+// caller may browse. It is built per caller and must not be shared between them:
+// what it holds is precisely the set of databases that caller is allowed to know
+// exists.
+type PrefixIndex struct {
+ byPrefix map[string]ReadyDatabase
+ // Failed lists the databases that could not be read. It is here for the
+ // caller's log and for nothing else: a database that could not be read costs
+ // the ids it owns their link, and a page may not say more than that.
+ Failed []ReadyFailure
+}
+
+// Reference is one id found in a text: where it sits, what it says, and which
+// database owns it.
+type Reference struct {
+ Start, End int // byte offsets of the id within the scanned text
+ ID string // the id exactly as written
+ Database ReadyDatabase
+}
+
+// idPattern matches an id-shaped token: a prefix of one or more lowercase
+// alphanumeric segments joined by hyphens, then a hyphen, then the suffix bd
+// generates — `[0-9a-z]+` with optional dotted parts, which is the `46c.2`
+// subtask form.
+//
+// The suffix carries no hyphen, so the last hyphen of a token is always the
+// prefix/suffix boundary and the split below is unambiguous even for a prefix
+// that is itself hyphenated ("sr-ht-dolt-44n.6" is sr-ht-dolt's 44n.6, never
+// sr-ht's dolt-44n.6). The word boundaries keep a token from being found inside
+// a longer word, so "Xartifacts-46c" names nothing.
+//
+// The token being id-shaped is not what makes it an id: only a prefix the index
+// knows makes it one. Everything else is left as it was written.
+var idPattern = regexp.MustCompile(`\b[0-9a-z_]+(?:-[0-9a-z_]+)*-[0-9a-z]+(?:\.[0-9a-z]+)*\b`)
+
+// PrefixesAcross builds the index over the databases it is handed: for each, the
+// issue_prefix its config names.
+//
+// It is bounded exactly as ReadyAcross is — the same head-hash gate, the same
+// TTL, the same ceiling — because it is the same read pattern: N stores opened
+// for one request. It is cheaper per database, though: the projection is one row
+// of a dozen-row table, and the fingerprint is not asked for separately, since a
+// config table carrying issue_prefix is itself the statement that this is a bd
+// tracker. A database with no such row simply owns no prefix, and that answer is
+// cached like any other.
+//
+// now is the clock the TTL is measured against, passed in rather than read here
+// for the reason ReadyAcross takes one: this package reads no hidden clock.
+//
+// It returns no error. A database that cannot be opened or read costs the ids it
+// owns their link and nothing else; it lands in Failed for the caller's log.
+func PrefixesAcross(
+ ctx context.Context,
+ dbs []ReadyDatabase,
+ open ReadyOpener,
+ cache *PrefixCache,
+ now time.Time,
+) *PrefixIndex {
+ index := &PrefixIndex{byPrefix: map[string]ReadyDatabase{}}
+
+ if len(dbs) > ReadyMaxDatabases {
+ // The first Max in the order the caller listed them. The caller puts the
+ // database whose page this is first, so the one prefix a page cannot do
+ // without is the one the ceiling can never drop.
+ dbs = dbs[:ReadyMaxDatabases]
+ }
+
+ // A prefix two databases both claim is dropped rather than awarded to
+ // whichever was listed first: linking to one of two candidates would be a
+ // guess rendered as a fact, and the id is readable as text either way.
+ ambiguous := map[string]bool{}
+ for _, d := range dbs {
+ prefix, err := databasePrefix(ctx, d, open, cache, now)
+ if err != nil {
+ index.Failed = append(index.Failed, ReadyFailure{Database: d, Err: err})
+ continue
+ }
+ if prefix == "" {
+ continue
+ }
+ if ambiguous[prefix] {
+ continue
+ }
+ if _, taken := index.byPrefix[prefix]; taken {
+ ambiguous[prefix] = true
+ delete(index.byPrefix, prefix)
+ continue
+ }
+ index.byPrefix[prefix] = d
+ }
+ return index
+}
+
+// databasePrefix returns one database's issue prefix, from the cache when the
+// head has not moved and by reading config otherwise.
+//
+// The order is the whole point of the gate: open, list branches, and only then
+// consult the cache. Opening a session and reading the branch list is cheap;
+// reading rows is not, and on a hit no row is touched.
+func databasePrefix(
+ ctx context.Context,
+ d ReadyDatabase,
+ open ReadyOpener,
+ cache *PrefixCache,
+ now time.Time,
+) (string, error) {
+ sess, err := open(ctx, d)
+ if err != nil {
+ return "", err
+ }
+ defer sess.Close()
+
+ branches, err := sess.Branches(ctx)
+ if err != nil {
+ return "", err
+ }
+ ref := browse.DefaultBranch(branches)
+ if ref == "" {
+ // A store with no branches carries no config either: nothing to read and
+ // nothing to cache.
+ return "", nil
+ }
+ head := headHashOf(branches, ref)
+ if prefix, ok := cache.lookup(d.ID, head, now); ok {
+ return prefix, nil
+ }
+
+ prefix, err := readPrefix(ctx, sess, ref)
+ if err != nil {
+ return "", err
+ }
+ cache.store(d.ID, head, now, prefix)
+ return prefix, nil
+}
+
+// readPrefix reads the issue_prefix row out of a database's config table. A
+// missing table — this is not a bd tracker — is no prefix, the treatment every
+// optional table gets here.
+func readPrefix(ctx context.Context, sess BrowseSession, ref string) (string, error) {
+ rows, _, err := readRowsOptional(ctx, sess, ref, memoryTable)
+ if err != nil {
+ return "", err
+ }
+ if rows == nil {
+ return "", nil
+ }
+ cols := indexCols(rows.Columns)
+ for _, r := range rows.Rows {
+ if cell(cols, r, "key") != prefixKey {
+ continue
+ }
+ // Lowercased because that is the case the ids themselves are written in,
+ // and the index is looked up by what the text says.
+ return strings.ToLower(strings.TrimSpace(cell(cols, r, "value"))), nil
+ }
+ return "", nil
+}
+
+// Lookup returns the database owning prefix. A nil index — no database was
+// readable, or the caller did not build one for this page — knows no prefix,
+// which is the same answer as an unknown one: not linked.
+func (ix *PrefixIndex) Lookup(prefix string) (ReadyDatabase, bool) {
+ if ix == nil {
+ return ReadyDatabase{}, false
+ }
+ d, ok := ix.byPrefix[prefix]
+ return d, ok
+}
+
+// Prefixes lists the prefixes the index knows, in order. It exists for the
+// caller's log line and for tests.
+func (ix *PrefixIndex) Prefixes() []string {
+ if ix == nil {
+ return nil
+ }
+ out := make([]string, 0, len(ix.byPrefix))
+ for p := range ix.byPrefix {
+ out = append(out, p)
+ }
+ sort.Strings(out)
+ return out
+}
+
+// Scan returns every id in text whose prefix this index knows, in order and
+// non-overlapping.
+//
+// It answers where the ids are and nothing about how they should be rendered:
+// the caller escapes the text it was handed and wraps these ranges, which is the
+// only order in which stored text can become HTML safely.
+//
+// An id-shaped token whose prefix the index does not know yields nothing at all,
+// which is what makes an id in a database the caller may not browse
+// indistinguishable from one that matches nothing.
+func (ix *PrefixIndex) Scan(text string) []Reference {
+ if ix == nil || len(ix.byPrefix) == 0 || text == "" {
+ return nil
+ }
+ var out []Reference
+ for _, m := range idPattern.FindAllStringIndex(text, -1) {
+ token := text[m[0]:m[1]]
+ cut := strings.LastIndex(token, "-")
+ if cut <= 0 {
+ continue
+ }
+ d, ok := ix.byPrefix[token[:cut]]
+ if !ok {
+ continue
+ }
+ out = append(out, Reference{Start: m[0], End: m[1], ID: token, Database: d})
+ }
+ return out
+}
A beads/prefixes_test.go => beads/prefixes_test.go +285 -0
@@ 0,0 1,285 @@
+package beads
+
+import (
+ "errors"
+ "fmt"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ "sourcecraft.dev/bigbes/sr-ht-dolt/browse"
+)
+
+// --- fixtures ----------------------------------------------------------------
+
+// prefixTracker is a beads database whose config names an issue prefix, with the
+// settings a real tracker carries beside it — the projection has to pick one row
+// out of the table rather than read the first one.
+func prefixTracker(head, prefix string) *fakeReadyDB {
+ return &fakeReadyDB{
+ branches: []browse.Branch{{Name: "main", Head: head}},
+ tables: beadsTables(),
+ rows: map[string]*browse.RowPage{
+ "config": {
+ Columns: []string{"key", "value"},
+ Rows: [][]string{
+ {"compact_tier2_days", "30"},
+ {"kv.memory.handoff", "not a prefix"},
+ {"issue_prefix", prefix},
+ },
+ Total: 3,
+ },
+ },
+ }
+}
+
+// prefixFixture is two trackers, "global" and "artifacts", as the instance has
+// them: one cross-project tracker and one per-project.
+func prefixFixture() (*readyInstance, []ReadyDatabase) {
+ in := &readyInstance{dbs: map[int]*fakeReadyDB{
+ 1: prefixTracker("h-global", "global"),
+ 2: prefixTracker("h-artifacts", "artifacts"),
+ }}
+ return in, []ReadyDatabase{
+ {ID: 1, OwnerName: "bigbes", Name: "beads-global"},
+ {ID: 2, OwnerName: "bigbes", Name: "sourcehut-artifacts"},
+ }
+}
+
+// --- tests -------------------------------------------------------------------
+
+// The index: one prefix per database, read out of config, and nothing claimed
+// for a prefix no database named.
+func TestPrefixesAcrossIndexesTheDatabasesItIsHanded(t *testing.T) {
+ in, dbs := prefixFixture()
+
+ index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow)
+
+ assert.Equal(t, []string{"artifacts", "global"}, index.Prefixes())
+ d, ok := index.Lookup("global")
+ require.True(t, ok)
+ assert.Equal(t, "bigbes/beads-global", d.Slug())
+ d, ok = index.Lookup("artifacts")
+ require.True(t, ok)
+ assert.Equal(t, "bigbes/sourcehut-artifacts", d.Slug())
+ _, ok = index.Lookup("nosuch")
+ assert.False(t, ok, "a prefix no database named is not in the index")
+ assert.Empty(t, index.Failed)
+
+ // Every session opened is closed again: this is the read pattern the
+ // per-request browse discipline is bounded against.
+ for id, f := range in.dbs {
+ assert.Equal(t, f.opens, f.closes, "database %d: %d opens, %d closes", id, f.opens, f.closes)
+ }
+}
+
+// The head-hash gate: a second build with unmoved heads reads no rows. Asserted
+// by counting reads on the fake, never by timing.
+func TestPrefixesAcrossHeadHashGateSkipsTheRead(t *testing.T) {
+ in, dbs := prefixFixture()
+ cache := &PrefixCache{}
+
+ first := PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow)
+ require.Len(t, first.Prefixes(), 2)
+ global := in.dbs[1]
+ require.Greater(t, global.rowReads, 0, "the first build must read config")
+ reads := global.rowReads
+
+ second := PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow.Add(30*time.Second))
+
+ assert.Equal(t, reads, global.rowReads, "a second build with an unmoved head must read no rows")
+ assert.Equal(t, first.Prefixes(), second.Prefixes(), "and it is the same answer")
+ // The session is still opened and its branches listed — that is what the gate
+ // is gated on, and it is the cheap half.
+ assert.Equal(t, 2, global.opens)
+ assert.Equal(t, 2, global.closes)
+}
+
+// A head that moved is a prefix that may have been renamed: read again, even
+// well inside the TTL.
+func TestPrefixesAcrossMovedHeadForcesAReread(t *testing.T) {
+ in, dbs := prefixFixture()
+ cache := &PrefixCache{}
+ global := in.dbs[1]
+
+ PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow)
+ reads := global.rowReads
+
+ global.branches = []browse.Branch{{Name: "main", Head: "h-global-2"}}
+ PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow.Add(time.Second))
+
+ assert.Greater(t, global.rowReads, reads, "a moved head must be re-read")
+ assert.Equal(t, 1, in.dbs[2].rowReads, "the sibling's head did not move")
+}
+
+// The TTL is the same one /ready is bounded by, because it is the same cache.
+func TestPrefixesAcrossTTLExpiryForcesAReread(t *testing.T) {
+ in, dbs := prefixFixture()
+ cache := &PrefixCache{}
+ global := in.dbs[1]
+
+ PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow)
+ reads := global.rowReads
+
+ PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow.Add(ReadyCacheTTL-time.Nanosecond))
+ assert.Equal(t, reads, global.rowReads, "inside the TTL the projection stands")
+
+ PrefixesAcross(t.Context(), dbs, in.open, cache, readyNow.Add(ReadyCacheTTL))
+ assert.Greater(t, global.rowReads, reads, "at the TTL the projection is re-read")
+}
+
+// The ceiling: at most ReadyMaxDatabases stores opened per build, and the first
+// database listed — the caller puts the page's own database there — is inside
+// it.
+func TestPrefixesAcrossCeiling(t *testing.T) {
+ in := &readyInstance{dbs: map[int]*fakeReadyDB{}}
+ var dbs []ReadyDatabase
+ for i := 1; i <= ReadyMaxDatabases+3; i++ {
+ in.dbs[i] = prefixTracker(fmt.Sprintf("h%d", i), fmt.Sprintf("p%02d", i))
+ dbs = append(dbs, ReadyDatabase{ID: i, OwnerName: "alice", Name: fmt.Sprintf("db%02d", i)})
+ }
+
+ index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow)
+
+ assert.Len(t, index.Prefixes(), ReadyMaxDatabases)
+ _, ok := index.Lookup("p01")
+ assert.True(t, ok, "the first database listed is never dropped by the ceiling")
+ for i := ReadyMaxDatabases + 1; i <= ReadyMaxDatabases+3; i++ {
+ assert.Zero(t, in.dbs[i].opens, "database %d is past the ceiling", i)
+ }
+}
+
+// Two databases claiming one prefix: neither is linked. Linking to one of two
+// candidates would be a guess rendered as a fact, and the id reads as text
+// either way.
+func TestPrefixesAcrossDropsAnAmbiguousPrefix(t *testing.T) {
+ in := &readyInstance{dbs: map[int]*fakeReadyDB{
+ 1: prefixTracker("h1", "shared"),
+ 2: prefixTracker("h2", "shared"),
+ 3: prefixTracker("h3", "mine"),
+ }}
+ dbs := []ReadyDatabase{
+ {ID: 1, OwnerName: "alice", Name: "one"},
+ {ID: 2, OwnerName: "bob", Name: "two"},
+ {ID: 3, OwnerName: "carol", Name: "three"},
+ }
+
+ index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow)
+
+ _, ok := index.Lookup("shared")
+ assert.False(t, ok, "a prefix two databases claim belongs to neither")
+ assert.Equal(t, []string{"mine"}, index.Prefixes(), "the unambiguous ones are unaffected")
+}
+
+// A database that names no prefix — no config table at all, or a config without
+// the row — is simply not in the index.
+func TestPrefixesAcrossSkipsADatabaseWithNoPrefix(t *testing.T) {
+ noConfig := &fakeReadyDB{branches: []browse.Branch{{Name: "main", Head: "h1"}}, tables: beadsTables()}
+ noRow := &fakeReadyDB{
+ branches: []browse.Branch{{Name: "main", Head: "h2"}},
+ tables: beadsTables(),
+ rows: map[string]*browse.RowPage{
+ "config": {Columns: []string{"key", "value"}, Rows: [][]string{{"compact_tier2_days", "30"}}, Total: 1},
+ },
+ }
+ empty := &fakeReadyDB{} // a store with no branches: never pushed to
+ in := &readyInstance{dbs: map[int]*fakeReadyDB{1: noConfig, 2: noRow, 3: empty}}
+ dbs := []ReadyDatabase{
+ {ID: 1, OwnerName: "alice", Name: "plain"},
+ {ID: 2, OwnerName: "alice", Name: "settings-only"},
+ {ID: 3, OwnerName: "alice", Name: "fresh"},
+ }
+
+ index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow)
+
+ assert.Empty(t, index.Prefixes())
+ assert.Empty(t, index.Failed, "naming no prefix is not a failure")
+ assert.Zero(t, empty.rowReads, "a store with no branches is not read")
+}
+
+// A database that cannot be opened, or whose config cannot be read, costs the
+// ids it owns their link and nothing else.
+func TestPrefixesAcrossFailingDatabaseCostsItselfOnly(t *testing.T) {
+ in, dbs := prefixFixture()
+ in.dbs[1].openErr = errors.New("browse: open store: no such file or directory")
+
+ index := PrefixesAcross(t.Context(), dbs, in.open, &PrefixCache{}, readyNow)
+
+ require.Len(t, index.Failed, 1)
+ assert.Equal(t, "bigbes/beads-global", index.Failed[0].Database.Slug())
+ assert.ErrorContains(t, index.Failed[0].Err, "no such file")
+ assert.Equal(t, []string{"artifacts"}, index.Prefixes())
+
+ // A store that opens but cannot list its branches fails the same way, and
+ // still closes its session.
+ in2, dbs2 := prefixFixture()
+ in2.dbs[1].branchesErr = errors.New("browse: list branches: corrupt chunk")
+ index2 := PrefixesAcross(t.Context(), dbs2, in2.open, &PrefixCache{}, readyNow)
+ require.Len(t, index2.Failed, 1)
+ assert.Equal(t, []string{"artifacts"}, index2.Prefixes())
+ assert.Equal(t, 1, in2.dbs[1].closes)
+}
+
+// The pattern: what counts as an id and what does not. The suffix carries no
+// hyphen, which is what makes the split unambiguous for a hyphenated prefix.
+func TestPrefixIndexScan(t *testing.T) {
+ index := &PrefixIndex{byPrefix: map[string]ReadyDatabase{
+ "global": {ID: 1, OwnerName: "bigbes", Name: "beads-global"},
+ "artifacts": {ID: 2, OwnerName: "bigbes", Name: "sourcehut-artifacts"},
+ "sr-ht-dolt": {ID: 3, OwnerName: "bigbes", Name: "sourcehut-dolt"},
+ }}
+
+ for _, tc := range []struct {
+ name string
+ text string
+ want []string
+ }{
+ {"a plain id", "blocked by artifacts-nex", []string{"artifacts-nex"}},
+ {"the subtask form", "see artifacts-46c.2 for the bucket", []string{"artifacts-46c.2"}},
+ {"a hyphenated prefix", "sr-ht-dolt-44n.6 lands first", []string{"sr-ht-dolt-44n.6"}},
+ {"several in one line", "global-b08 needs artifacts-46c.2", []string{"global-b08", "artifacts-46c.2"}},
+ {"an unknown prefix", "nosuch-46c and other-1 are not ids", nil},
+ {"a hyphenated word", "the read-only surface", nil},
+ {"an id at the end of a sentence", "closed by artifacts-46c.", []string{"artifacts-46c"}},
+ {"an id in brackets", "(artifacts-46c) and [global-b08]", []string{"artifacts-46c", "global-b08"}},
+ {"an id inside a longer word", "xxartifacts-46c and artifactsx-1", nil},
+ {"uppercase is not an id", "ARTIFACTS-46C", nil},
+ {"a prefix on its own", "the artifacts tracker", nil},
+ {"a shorter known prefix inside a longer token", "some-global-b08", nil},
+ {"empty text", "", nil},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ var got []string
+ for _, ref := range index.Scan(tc.text) {
+ got = append(got, ref.ID)
+ assert.Equal(t, ref.ID, tc.text[ref.Start:ref.End],
+ "the offsets must name the id they carry")
+ }
+ assert.Equal(t, tc.want, got)
+ })
+ }
+
+ // Each reference names the database that owns it.
+ refs := index.Scan("global-b08 blocks artifacts-46c.2")
+ require.Len(t, refs, 2)
+ assert.Equal(t, "bigbes/beads-global", refs[0].Database.Slug())
+ assert.Equal(t, "bigbes/sourcehut-artifacts", refs[1].Database.Slug())
+}
+
+// An index that was never built knows no prefix — the same answer an unknown one
+// gets, which is what keeps "the caller may not see that database" and "there is
+// no such database" indistinguishable.
+func TestNilPrefixIndexKnowsNothing(t *testing.T) {
+ var index *PrefixIndex
+
+ assert.Nil(t, index.Scan("artifacts-46c.2"))
+ assert.Nil(t, index.Prefixes())
+ _, ok := index.Lookup("artifacts")
+ assert.False(t, ok)
+
+ // So does an empty one.
+ empty := PrefixesAcross(t.Context(), nil, nil, &PrefixCache{}, readyNow)
+ assert.Nil(t, empty.Scan("artifacts-46c.2"))
+}
M beads/ready.go => beads/ready.go +20 -58
@@ 5,7 5,6 @@ import (
"net/url"
"sort"
"strings"
- "sync"
"time"
"sourcecraft.dev/bigbes/sr-ht-dolt/browse"
@@ 26,6 25,10 @@ import (
// comment: this package renders nothing and authorizes nothing); handing it a
// database is the statement that the caller may read it.
+// The bounds every cross-database reading here is held to. They are named for
+// /ready because that is the page that needed them first; the prefix index
+// behind cross-database issue links is bounded by these same three numbers
+// rather than by a second set of its own (see cache.go).
const (
// ReadyMaxDatabases bounds how many databases one call opens. Opening N
// stores per request is exactly what the per-request browse discipline does
@@ 186,28 189,23 @@ func (f ReadyFilter) matches(c Card) bool {
return true
}
-// ReadyCache holds one projection per database, keyed by the repository id and
-// gated on the head hash. It is deliberately small: a mutex, a map, and the two
-// bounds above.
+// ReadyCache holds one ready projection per database, keyed by the repository id
+// and gated on the head hash. The gate, the TTL and the entry ceiling are the
+// shared projectionCache's (cache.go), which the prefix index is bounded by too:
+// one set of rules, one lifetime.
//
// What it holds is the projection — the ready cards — and never an open
// session. An open store is a file handle and a memory mapping; caching those is
// the thing per-request opening exists to prevent.
-//
-// The zero value is not usable; call NewReadyCache. A nil *ReadyCache is a
-// programming error and panics on first use rather than quietly disabling the
-// bound it exists to enforce.
type ReadyCache struct {
- mu sync.Mutex
- entries map[int]readyEntry
+ projectionCache[readyEntry]
}
-// readyEntry is one database's cached projection.
+// readyEntry is one database's cached ready projection. The head it was read at
+// and the time it was stored are the cache's, not this struct's.
type readyEntry struct {
- head string // the branch head the projection was read at; the gate
- ref string // the branch it was read from
- at time.Time // when it was stored; the TTL
- beads bool // the fingerprint held — a false entry is a database to skip
+ ref string // the branch it was read from
+ beads bool // the fingerprint held — a false entry is a database to skip
// commit is the head commit for the group's freshness line. It cannot go
// stale under an unmoved head, so it is cached with the cards and a cache hit
// reads no log either.
@@ 215,47 213,11 @@ type readyEntry struct {
cards []Card
}
-// NewReadyCache returns an empty cache.
+// NewReadyCache returns an empty cache. The zero value works too — the map is
+// allocated on first store — and this exists for the callers that hold one by
+// pointer.
func NewReadyCache() *ReadyCache {
- return &ReadyCache{entries: map[int]readyEntry{}}
-}
-
-// lookup returns the cached projection for a database when it was read at the
-// same head and has not expired. Both conditions, not either: the head hash is
-// what makes it correct, the TTL is what makes it bounded.
-func (c *ReadyCache) lookup(id int, head string, now time.Time) (readyEntry, bool) {
- if head == "" {
- // A database whose head cannot be named cannot be gated on one.
- return readyEntry{}, false
- }
- c.mu.Lock()
- defer c.mu.Unlock()
- e, ok := c.entries[id]
- if !ok || e.head != head || now.Sub(e.at) >= ReadyCacheTTL {
- return readyEntry{}, false
- }
- return e, true
-}
-
-// store records a projection, dropping expired entries — and, if that was not
-// enough, everything — when the map is at its ceiling.
-func (c *ReadyCache) store(id int, e readyEntry) {
- if e.head == "" {
- return
- }
- c.mu.Lock()
- defer c.mu.Unlock()
- if len(c.entries) >= readyCacheMaxEntries {
- for k, old := range c.entries {
- if e.at.Sub(old.at) >= ReadyCacheTTL {
- delete(c.entries, k)
- }
- }
- if len(c.entries) >= readyCacheMaxEntries {
- c.entries = make(map[int]readyEntry, readyCacheMaxEntries)
- }
- }
- c.entries[id] = e
+ return &ReadyCache{}
}
// ReadyAcross collects the ready set of every database it is handed, grouped by
@@ 388,11 350,11 @@ func readyProjection(
if err != nil {
return readyEntry{}, err
}
- entry := readyEntry{head: head, ref: ref, at: now, beads: Applies(tables)}
+ entry := readyEntry{ref: ref, beads: Applies(tables)}
if !entry.beads {
// Cached too: a database that is not a tracker is not one on the next
// request either, and the fingerprint is worth exactly one table listing.
- cache.store(d.ID, entry)
+ cache.store(d.ID, head, now, entry)
return entry, nil
}
@@ 402,7 364,7 @@ func readyProjection(
}
entry.cards = cards
entry.commit = readyHead(ctx, sess, ref)
- cache.store(d.ID, entry)
+ cache.store(d.ID, head, now, entry)
return entry, nil
}
M beads/ready_test.go => beads/ready_test.go +2 -5
@@ 468,10 468,7 @@ func TestParseReadyFilter(t *testing.T) {
func TestReadyCacheIsBoundedByEntryCount(t *testing.T) {
cache := NewReadyCache()
for i := 1; i <= readyCacheMaxEntries+10; i++ {
- cache.store(i, readyEntry{head: fmt.Sprintf("h%d", i), at: readyNow, beads: true})
+ cache.store(i, fmt.Sprintf("h%d", i), readyNow, readyEntry{beads: true})
}
- cache.mu.Lock()
- n := len(cache.entries)
- cache.mu.Unlock()
- assert.LessOrEqual(t, n, readyCacheMaxEntries)
+ assert.LessOrEqual(t, cache.size(), readyCacheMaxEntries)
}
M web/beads.go => web/beads.go +157 -1
@@ 2,7 2,14 @@ package web
import (
"context"
+ "fmt"
+ "html/template"
+ "log/slog"
+ "net/http"
"net/url"
+ "strings"
+
+ "go.bigb.es/auxilia/scribe"
"sourcecraft.dev/bigbes/sr-ht-dolt/beads"
"sourcecraft.dev/bigbes/sr-ht-dolt/browse"
@@ 21,7 28,30 @@ import (
// rule and the whole view model live in the beads package, which the MCP surface
// shares. This type is only the View adapter — slug, label, template, and the
// hand-off of the request's ref and query.
-type beadsView struct{}
+type beadsView struct {
+ // prefixes is the cross-database link index's cache: one issue prefix per
+ // database, gated on that database's head hash and expiring on
+ // beads.ReadyCacheTTL — /ready's bounds, shared rather than restated (see
+ // beads/cache.go).
+ //
+ // It lives on the view because the view is the one piece of per-process state
+ // this rendering has, and because a zero value is a working cache: views are
+ // registered as a bare &beadsView{}, here and in every test.
+ //
+ // The cache is keyed on the database and never on the caller, which is safe
+ // precisely because what it holds is a database's own prefix — a fact about
+ // the store, not about who may see it. Who may see it is decided per request,
+ // before a store is opened, and the index built from it belongs to that
+ // request alone.
+ prefixes beads.PrefixCache
+}
+
+// The two Data.Mode values that render one issue rather than a board. They are
+// what decides whether this request pays for the cross-database index at all.
+const (
+ beadsModeDetail = "detail"
+ beadsModeEpic = "epic"
+)
func (*beadsView) Name() string { return "beads" }
func (*beadsView) Label() string { return "Beads" }
@@ 40,3 70,129 @@ func (*beadsView) Build(ctx context.Context, sess BrowseSession, _ *core.Repo, r
}
return data, nil
}
+
+// --- cross-database issue links -----------------------------------------------
+
+// beadLinks renders one stored text with the issue ids in it linked to the
+// databases that own them. It is the envelope's .Links on the issue detail pane
+// and nil everywhere else, and a nil one renders the text with no links at all —
+// which is a whole answer, not a degraded one: an id nobody here owns is text.
+type beadLinks struct {
+ index *beads.PrefixIndex
+}
+
+// Text renders a stored text — a description, a comment body, an event summary —
+// as HTML with every recognised issue id wrapped in a link.
+//
+// It escapes first and wraps second. The result is built as a sequence of
+// escaped segments and anchors this function generated itself, and only the
+// finished whole is marked template.HTML: marking user-stored text as HTML and
+// then running a regexp over it is how a stored payload becomes a rendered one.
+// Nothing that came out of the database is ever handed to the browser unescaped,
+// including the id inside the anchor and the href built from it.
+func (l *beadLinks) Text(s string) template.HTML {
+ var refs []beads.Reference
+ if l != nil {
+ refs = l.index.Scan(s)
+ }
+ var b strings.Builder
+ b.Grow(len(s))
+ last := 0
+ for _, ref := range refs {
+ b.WriteString(template.HTMLEscapeString(s[last:ref.Start]))
+ b.WriteString(`<a href="`)
+ b.WriteString(template.HTMLEscapeString(
+ beadIssueHref(ref.Database.OwnerName, ref.Database.Name, ref.ID)))
+ b.WriteString(`">`)
+ b.WriteString(template.HTMLEscapeString(ref.ID))
+ b.WriteString(`</a>`)
+ last = ref.End
+ }
+ b.WriteString(template.HTMLEscapeString(s[last:]))
+ return template.HTML(b.String())
+}
+
+// beadIssueHref is the detail-pane URL of one issue in one database: the same
+// address beads.html writes for a dependency edge, built here because this one
+// is assembled in Go rather than by the template.
+func beadIssueHref(owner, name, id string) string {
+ return "/~" + url.PathEscape(owner) + "/" + url.PathEscape(name) +
+ "/view/beads?issue=" + url.QueryEscape(id)
+}
+
+// beadCrossLinks builds the link index for one render, and returns nil for every
+// page that is not an issue detail pane: the board carries ids in card headers
+// that are already links, and nothing else in this service renders stored prose.
+// Building the index opens a store per database, so it is asked for by the one
+// rendering that needs it and never as part of the envelope.
+//
+// The visibility rule is the /ready page's, for the same reason: enumerate with
+// ListReposForViewer (the listing rule), then ask core.Allowed/OpBrowse per
+// database (the access rule), and do both before a single store is opened. A
+// database this caller may not browse never enters the index, so an id belonging
+// to it renders as plain text — indistinguishable from one whose prefix matches
+// nothing. That indistinguishability is the point: a tooltip, a class or an
+// "unknown tracker" marker would each publish the existence of a database this
+// caller is not allowed to know about.
+//
+// A listing that fails costs the page its links and not the page. This is
+// decoration on top of an answer, exactly as the freshness line is, and it may
+// never be the reason a reader gets a 500 instead of an issue.
+func (a *app) beadCrossLinks(r *http.Request, view View, repo *core.Repo, data any) *beadLinks {
+ bv, ok := view.(*beadsView)
+ if !ok {
+ return nil
+ }
+ d, ok := data.(*beads.Data)
+ if !ok || (d.Mode != beadsModeDetail && d.Mode != beadsModeEpic) {
+ return nil
+ }
+
+ _, caller := callerOf(r.Context())
+ repos, err := a.cfg.Repos.ListReposForViewer(r.Context(), caller)
+ if err != nil {
+ slog.Error("listing databases for the issue link index failed",
+ "component", "web", scribe.Err(err))
+ return nil
+ }
+
+ // The disk path never reaches beads: it is this service's arrangement of its
+ // own storage. The opener closes over the map, so a database that was filtered
+ // out here has no path to be opened by.
+ paths := make(map[int]string, len(repos))
+ dbs := make([]beads.ReadyDatabase, 0, len(repos))
+ for _, cand := range repos {
+ if !core.Allowed(caller, cand, a.effectiveACL(r, caller, cand), core.OpBrowse) {
+ continue
+ }
+ paths[cand.ID] = cand.Path
+ entry := beads.ReadyDatabase{ID: cand.ID, OwnerName: cand.OwnerName, Name: cand.Name}
+ if cand.ID == repo.ID {
+ // The database whose page this is goes first, so the one prefix the
+ // page cannot do without — its own, the ids that used to link and must
+ // keep linking — can never be the one the ceiling drops.
+ dbs = append([]beads.ReadyDatabase{entry}, dbs...)
+ continue
+ }
+ dbs = append(dbs, entry)
+ }
+
+ open := func(ctx context.Context, d beads.ReadyDatabase) (beads.ReadySession, error) {
+ path, ok := paths[d.ID]
+ if !ok {
+ return nil, fmt.Errorf("web: no store path for database %s", d.Slug())
+ }
+ return a.cfg.Browse.Open(ctx, path)
+ }
+
+ index := beads.PrefixesAcross(r.Context(), dbs, open, &bv.prefixes, timeNow())
+
+ // A store that cannot be read is a fact about this deployment and belongs in
+ // the log with its error. The page says nothing at all: the ids that database
+ // owns simply stay text.
+ for _, f := range index.Failed {
+ slog.Warn("reading a database for the issue link index failed",
+ "component", "web", "database", f.Database.Slug(), scribe.Err(f.Err))
+ }
+ return &beadLinks{index: index}
+}
M web/beads_test.go => web/beads_test.go +298 -0
@@ 1,9 1,11 @@
package web
import (
+ "errors"
"html"
"net/http"
"net/url"
+ "regexp"
"strings"
"testing"
@@ 593,6 595,302 @@ func TestBeadsBoardHasNoCommandBlock(t *testing.T) {
}
}
+// --- cross-database issue links ------------------------------------------------
+
+// The database whose detail pane these tests render: prefix "alpha", one issue
+// whose four bodies, one comment and history name issues in other databases —
+// one visible, one this caller may not browse, one belonging to nobody — plus a
+// script tag stored in the description.
+func beadsLinkFixture() *fakeSession {
+ sess := linkTracker("h-alpha", "alpha")
+ sess.rowsByTable["issues"] = &browse.RowPage{
+ Columns: []string{"id", "title", "status", "priority", "issue_type", "assignee",
+ "created_at", "description", "design", "acceptance_criteria", "notes"},
+ Rows: [][]string{{
+ "alpha-1", "The one with references", "open", "1", "task", "alice", "2024-01-01",
+ "blocked by beta-46c.2, and by alpha-2.\nnosuch-9z is nobody's.\n<script>alert('x')</script>",
+ "the shape is beta-nex's",
+ "beta-46c.2 is closed",
+ "secret-9a1 stays a secret",
+ }},
+ Total: 1,
+ }
+ sess.rowsByTable["comments"] = &browse.RowPage{
+ Columns: []string{"issue_id", "author", "text", "created_at"},
+ Rows: [][]string{
+ {"alpha-1", "alice", "landing with beta-46c.2", "2024-01-05 09:00:00"},
+ },
+ Total: 1,
+ }
+ sess.rowsByTable["events"] = &browse.RowPage{
+ Columns: []string{"id", "issue_id", "event_type", "actor", "old_value", "new_value", "comment", "created_at"},
+ Rows: [][]string{
+ {"e1", "alpha-1", "updated", "alice", "NULL", `{"design":"the shape is beta-nex's"}`, "NULL", "2024-01-06 09:00:00"},
+ },
+ Total: 1,
+ }
+ return sess
+}
+
+// linkTracker is a beads database whose config names an issue prefix, with no
+// issues in it. It stands for the other databases on the instance: what the
+// index reads out of one is the prefix and nothing else.
+func linkTracker(head, prefix string) *fakeSession {
+ return &fakeSession{
+ branches: []browse.Branch{{Name: "main", Head: head}},
+ tables: beadsTables(),
+ rowsByTable: map[string]*browse.RowPage{
+ "issues": {Columns: []string{"id", "title", "status"}, Total: 0},
+ "dependencies": {Columns: []string{"id", "issue_id", "depends_on_issue_id", "type"},
+ Total: 0},
+ "config": {
+ Columns: []string{"key", "value"},
+ Rows: [][]string{
+ {"compact_tier2_days", "30"},
+ {"issue_prefix", prefix},
+ },
+ Total: 2,
+ },
+ },
+ }
+}
+
+// linkHarness is the instance these tests read: the current database
+// (alice/alpha), a second public one whose prefix is "beta", and a PRIVATE one
+// whose prefix is "secret" and which the caller may not browse.
+func linkHarness(t *testing.T) (h *harness, beta, secret *fakeSession) {
+ t.Helper()
+ h = newHarness(t)
+ beta = linkTracker("h-beta", "beta")
+ secret = linkTracker("h-secret", "secret")
+ addTracker(h, "alice", 1, "alpha", core.VisibilityPublic, beadsLinkFixture())
+ addTracker(h, "bob", 2, "beta", core.VisibilityPublic, beta)
+ addTracker(h, "dave", 9, "secrets", core.VisibilityPrivate, secret)
+ setViews(t, h, &beadsView{})
+ return h, beta, secret
+}
+
+// An id whose prefix names a database this caller may browse becomes a link to
+// that database's detail pane; an id in the current database keeps linking to
+// this one; an id whose prefix matches nothing is left as it was written.
+func TestBeadsDetailLinksIdsToTheDatabaseThatOwnsThem(t *testing.T) {
+ h, _, _ := linkHarness(t)
+
+ rec := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+
+ // The sibling database, in each of the long-text bodies that name it.
+ want := `<a href="/~bob/beta/view/beads?issue=beta-46c.2">beta-46c.2</a>`
+ for _, field := range []string{"Description", "Acceptance criteria"} {
+ if got := fieldBody(t, body, field); !strings.Contains(got, want) {
+ t.Errorf("the %s body did not link the sibling's id: %s", field, got)
+ }
+ }
+ // The design body's id, which is another database's too.
+ if got := fieldBody(t, body, "Design"); !strings.Contains(got,
+ `<a href="/~bob/beta/view/beads?issue=beta-nex">beta-nex</a>`) {
+ t.Errorf("the design body's id did not link: %s", got)
+ }
+ // This database's own id links where it has always linked: here.
+ if !strings.Contains(body, `<a href="/~alice/alpha/view/beads?issue=alpha-2">alpha-2</a>`) {
+ t.Errorf("an id in the current database did not link to it; body=%s", body)
+ }
+ // A prefix no database on this instance claims is not an id.
+ if !strings.Contains(body, "nosuch-9z is nobody's.") {
+ t.Errorf("an unknown prefix should be left alone; body=%s", body)
+ }
+ if strings.Contains(body, "issue=nosuch-9z") {
+ t.Errorf("an unknown prefix was linked; body=%s", body)
+ }
+}
+
+// A database this caller may not browse is not in the index, so the ids it owns
+// render as plain text — with nothing at all to distinguish them from an id that
+// matches nothing. No tooltip, no class, no marker: each of those would publish
+// the existence of a database this caller is not allowed to know about.
+func TestBeadsDetailLeavesAnInvisibleDatabaseUnlinked(t *testing.T) {
+ h, _, secret := linkHarness(t)
+
+ rec := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+
+ // The text says what it said.
+ if !strings.Contains(body, "secret-9a1 stays a secret") {
+ t.Errorf("the notes body lost its text; body=%s", body)
+ }
+ // And says nothing else: not a link, not a marker, not the database's name,
+ // its owner, or its head.
+ for _, notWant := range []string{
+ "issue=secret-9a1", ">secret-9a1</a>", "secrets", "dave", "h-secret", "unknown tracker",
+ } {
+ if strings.Contains(body, notWant) {
+ t.Errorf("the page revealed %q about a database the caller may not browse; body=%s", notWant, body)
+ }
+ }
+ if secret.opens != 0 {
+ t.Errorf("a database the caller may not browse was opened %d times", secret.opens)
+ }
+
+ // Its owner, who may browse it, gets the link — which is what makes the
+ // absence above a visibility rule and not a broken index.
+ owner := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", testCaller(9, "dave"), nil)
+ if owner.Code != http.StatusOK {
+ t.Fatalf("owner detail: got %d; body=%s", owner.Code, owner.Body.String())
+ }
+ if !strings.Contains(owner.Body.String(), `<a href="/~dave/secrets/view/beads?issue=secret-9a1">secret-9a1</a>`) {
+ t.Errorf("the owner of the private tracker should see the link; body=%s", owner.Body.String())
+ }
+}
+
+// Stored text is escaped first and linked second. A description carrying a
+// script tag reaches the page as text, and the anchors this rendering generated
+// are the only markup in the body it produced.
+func TestBeadsDetailEscapesBeforeItLinks(t *testing.T) {
+ h, _, _ := linkHarness(t)
+
+ rec := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+
+ if strings.Contains(body, "<script") {
+ t.Fatalf("a stored script tag reached the page as markup; body=%s", body)
+ }
+ if !strings.Contains(body, "<script>alert('x')</script>") {
+ t.Errorf("the stored script tag is not escaped; body=%s", body)
+ }
+
+ // Inside the rendered description, the only tags are the anchors.
+ desc := fieldBody(t, body, "Description")
+ anchor := regexp.MustCompile(`^(?:<a href="/~[a-z0-9/?=.&;-]+">|</a>)$`)
+ for _, tag := range regexp.MustCompile(`<[^>]*>`).FindAllString(desc, -1) {
+ if !anchor.MatchString(tag) {
+ t.Errorf("unexpected markup %q in the rendered description: %s", tag, desc)
+ }
+ }
+ // Escaped, the description still says exactly what was stored.
+ if got := html.UnescapeString(regexp.MustCompile(`</?a[^>]*>`).ReplaceAllString(desc, "")); got !=
+ "blocked by beta-46c.2, and by alpha-2.\nnosuch-9z is nobody's.\n<script>alert('x')</script>" {
+ t.Errorf("the rendered description does not decode to the stored text: %q", got)
+ }
+}
+
+// The ids in a comment body and in a history summary are linked too: they are
+// where a hand-off between two trackers is usually written.
+func TestBeadsDetailLinksCommentsAndHistory(t *testing.T) {
+ h, _, _ := linkHarness(t)
+
+ rec := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+
+ comment := `<div class="c-body">landing with <a href="/~bob/beta/view/beads?issue=beta-46c.2">beta-46c.2</a></div>`
+ if !strings.Contains(body, comment) {
+ t.Errorf("a comment body's id did not link; body=%s", body)
+ }
+ summary := `updated design to the shape is <a href="/~bob/beta/view/beads?issue=beta-nex">beta-nex</a>'s`
+ if !strings.Contains(body, summary) {
+ t.Errorf("a history summary's id did not link; body=%s", body)
+ }
+}
+
+// The index is bounded exactly as /ready is: a second render whose heads have
+// not moved reads no rows again. Counted on the fake, never timed.
+func TestBeadsDetailPrefixIndexIsBounded(t *testing.T) {
+ h, beta, _ := linkHarness(t)
+
+ first := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if first.Code != http.StatusOK {
+ t.Fatalf("first detail: got %d", first.Code)
+ }
+ reads := beta.rowReads
+ if reads == 0 {
+ t.Fatalf("the first render must read the sibling's config")
+ }
+
+ second := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if second.Code != http.StatusOK {
+ t.Fatalf("second detail: got %d", second.Code)
+ }
+ if beta.rowReads != reads {
+ t.Errorf("an unmoved head must cost no row reads: %d then %d", reads, beta.rowReads)
+ }
+ // The store is still opened and its branches listed — that is what the gate
+ // is gated on, and it is the cheap half.
+ if beta.opens != 2 {
+ t.Errorf("the sibling should be opened once per render, got %d", beta.opens)
+ }
+ // And the page is the same page, cache or not.
+ if first.Body.String() != second.Body.String() {
+ t.Errorf("the cached render differs from the first one")
+ }
+}
+
+// The board carries no stored prose, so it builds no index and opens no other
+// database. The index is paid for by the one rendering that needs it.
+func TestBeadsBoardBuildsNoLinkIndex(t *testing.T) {
+ h, beta, _ := linkHarness(t)
+
+ rec := h.do("GET", "/~alice/alpha/view/beads", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("board: got %d; body=%s", rec.Code, rec.Body.String())
+ }
+ if beta.opens != 0 {
+ t.Errorf("the board opened another database %d times", beta.opens)
+ }
+ if strings.Contains(rec.Body.String(), "/~bob/beta/view/beads") {
+ t.Errorf("the board linked into another database; body=%s", rec.Body.String())
+ }
+}
+
+// An index that could not be built at all costs the page its links and not the
+// page: the ids are text, exactly as they were before this existed.
+func TestBeadsDetailSurvivesAnUnbuildableIndex(t *testing.T) {
+ h, _, _ := linkHarness(t)
+ h.store.listErr = errors.New("db: list repositories: connection refused")
+
+ rec := h.do("GET", "/~alice/alpha/view/beads?issue=alpha-1", nil, nil)
+ if rec.Code != http.StatusOK {
+ t.Fatalf("detail: got %d, want 200; body=%s", rec.Code, rec.Body.String())
+ }
+ body := rec.Body.String()
+ if !strings.Contains(body, "blocked by beta-46c.2, and by alpha-2.") {
+ t.Errorf("the description is missing; body=%s", body)
+ }
+ if strings.Contains(body, "issue=beta-46c.2") {
+ t.Errorf("an index that was never built linked something; body=%s", body)
+ }
+ if strings.Contains(body, "connection refused") {
+ t.Errorf("the listing error reached the reader; body=%s", body)
+ }
+}
+
+// fieldBody returns the contents of the <pre class="field-body"> that follows
+// the named field label.
+func fieldBody(t *testing.T, body, label string) string {
+ t.Helper()
+ head := `<div class="field-label">` + label + `</div><pre class="field-body">`
+ i := strings.Index(body, head)
+ if i < 0 {
+ t.Fatalf("no %s body in the page: %s", label, body)
+ }
+ rest := body[i+len(head):]
+ j := strings.Index(rest, "</pre>")
+ if j < 0 {
+ t.Fatalf("unterminated %s body", label)
+ }
+ return rest[:j]
+}
+
func TestBeadsEpicViewRender(t *testing.T) {
h := newHarness(t)
h.store.add(&core.Repo{Name: "db", OwnerID: 1, OwnerName: "alice", Path: "/d", Visibility: core.VisibilityPublic})
M web/handlers_view.go => web/handlers_view.go +7 -0
@@ 87,6 87,12 @@ func (a *app) handleView(w http.ResponseWriter, r *http.Request) {
Views []View
Head *browse.CommitInfo
Data any
+ // Links renders the issue ids inside stored prose as links to the database
+ // that owns them. It is built by the one rendering that needs it — the
+ // beads detail pane — and is nil for every other page here, because
+ // building it opens a store per database the caller may browse. A nil one
+ // still renders text; see beadLinks.Text.
+ Links *beadLinks
}{
Page: a.page(r, view.Label()+" — "+repo.OwnerName+"/"+repo.Name),
Repo: repo,
@@ 95,6 101,7 @@ func (a *app) handleView(w http.ResponseWriter, r *http.Request) {
Views: applicableViews(a.views, tables),
Head: headCommit(r.Context(), sess, ref),
Data: data,
+ Links: a.beadCrossLinks(r, view, repo, data),
}
a.render(w, http.StatusOK, pageName(view.Template()), envelope)
}
M web/templates/beads.html => web/templates/beads.html +11 -7
@@ 261,10 261,14 @@ pre.field-body {
{{if .ClosedAt}}<tr><td class="field-label">Closed</td><td>{{.ClosedAt}}</td></tr>{{end}}
</tbody>
</table>
- {{if .Description}}<div class="field-label">Description</div><pre class="field-body">{{.Description}}</pre>{{end}}
- {{if .Design}}<div class="field-label">Design</div><pre class="field-body">{{.Design}}</pre>{{end}}
- {{if .AcceptanceCriteria}}<div class="field-label">Acceptance criteria</div><pre class="field-body">{{.AcceptanceCriteria}}</pre>{{end}}
- {{if .Notes}}<div class="field-label">Notes</div><pre class="field-body">{{.Notes}}</pre>{{end}}
+ {{/* The long-text bodies go through $.Links, which escapes the stored text and
+ then wraps the issue ids it recognises in links to the database that owns
+ them. An id whose prefix belongs to no database this viewer may browse is
+ left as the text it is. */}}
+ {{if .Description}}<div class="field-label">Description</div><pre class="field-body">{{$.Links.Text .Description}}</pre>{{end}}
+ {{if .Design}}<div class="field-label">Design</div><pre class="field-body">{{$.Links.Text .Design}}</pre>{{end}}
+ {{if .AcceptanceCriteria}}<div class="field-label">Acceptance criteria</div><pre class="field-body">{{$.Links.Text .AcceptanceCriteria}}</pre>{{end}}
+ {{if .Notes}}<div class="field-label">Notes</div><pre class="field-body">{{$.Links.Text .Notes}}</pre>{{end}}
</div>
{{if eq $.Data.Mode "epic"}}
@@ 366,7 370,7 @@ pre.field-body {
{{range $.Data.Comments}}
<div class="bead-comment">
<div class="c-head"><strong>{{.Author}}</strong> {{if .CreatedAt}}· {{.CreatedAt}}{{end}}</div>
- <div class="c-body">{{.Text}}</div>
+ <div class="c-body">{{$.Links.Text .Text}}</div>
</div>
{{end}}
{{else}}<p class="beads-empty">No comments.</p>{{end}}
@@ 384,10 388,10 @@ pre.field-body {
{{range $.Data.History}}
<li class="tl-item tl-{{.Kind}}">
<div class="tl-head">
- <strong>{{.Actor}}</strong> {{.Summary}}
+ <strong>{{.Actor}}</strong> {{$.Links.Text .Summary}}
{{if .CreatedAt}}<span class="tl-when">· {{.CreatedAt}}</span>{{end}}
</div>
- {{if .Text}}<div class="tl-body">{{.Text}}</div>{{end}}
+ {{if .Text}}<div class="tl-body">{{$.Links.Text .Text}}</div>{{end}}
</li>
{{end}}
</ul>
M web/web_test.go => web/web_test.go +7 -1
@@ 46,7 46,10 @@ type fakeStore struct {
nextID int
nextKeyID int
- createErr error
+ createErr error
+ // listErr, when set, makes ListReposForViewer fail — the metadata store
+ // unreachable, which the pages that enumerate databases have to survive.
+ listErr error
createdCalls []*core.Repo
deletedRepos []int
}
@@ 112,6 115,9 @@ func (f *fakeStore) ListReposByOwner(_ context.Context, owner string, viewer *co
// everyone, plus whatever the viewer owns or holds an ACL on. Sorted by id so a
// test that depends on the order it hands to /ready gets the same one twice.
func (f *fakeStore) ListReposForViewer(_ context.Context, viewer *core.Caller) ([]*core.Repo, error) {
+ if f.listErr != nil {
+ return nil, f.listErr
+ }
var out []*core.Repo
for _, r := range f.byID {
visible := r.Visibility == core.VisibilityPublic