Keep builds in the builds bucket and move the build rows there at start (issue 1090)

Once chain_moved records the kind, RecordBuild and every build read go through the chain (reuse key and published added); before that the tables are read as before. The serving controller copies the rows at its start, holding the lease: idempotent per key, rows counted against keys, the largest value measured, a refusal (urgent condition, nothing switched) on a count that differs or a value over the bus's largest, and a re-read when the table took a row meanwhile. ForTest attaches the chain in memory with builds moved; ForTestBefore leaves it unmoved. Build requests, gates and the other kinds are still in their tables.

internal/chain/bus.go

@@ -13,7 +13,7 @@ import (
 
 // OnTheBus opens the chain's three buckets on a connection. The controller asserts them at its start
 // (broker.EnsureControllerBuckets), so one older than this has not, and the error says so.
-func OnTheBus(ctx context.Context, conn *nats.Conn, holds func() bool) (*Chain, error) {
+func OnTheBus(ctx context.Context, conn *nats.Conn, holds Holds) (*Chain, error) {
 	api, err := jetstream.New(conn)
 	if err != nil {
 		return nil, err
@@ -85,7 +85,7 @@ func (b busBucket) Keys(ctx context.Context) ([]string, error) {
 }
 
 func (b busBucket) Watch(ctx context.Context) (<-chan Entry, error) {
-	w, err := b.kv.WatchAll(ctx)
+	w, err := b.kv.WatchAll(ctx, jetstream.IgnoreDeletes())
 	if err != nil {
 		return nil, err
 	}

internal/chain/chain.go

@@ -3,7 +3,10 @@ package chain
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"regexp"
+	"strings"
 
 	"github.com/novox/mesh-controller/internal/broker"
 )
@@ -12,9 +15,13 @@ import (
 // controller holds the lease** and writes nothing when it does not (ADR 0408 §2).
 type Bucket[T any] struct {
 	b     Backend
-	holds func() bool
+	holds Holds
 }
 
+// Holds says whether this controller may write now: nil when it holds the lease, and why not when it does not. The
+// serving controller answers from its lease, and a command from the holder's (the epoch gate).
+type Holds func(ctx context.Context) error
+
 // Kept is a record as the bucket holds it: with its revision, the one a write must find.
 type Kept[T any] struct {
 	Key      string
@@ -22,10 +29,22 @@ type Kept[T any] struct {
 	Revision uint64
 }
 
-func (s Bucket[T]) guard(value []byte) error {
-	if s.holds == nil || !s.holds() {
+// validKey is what the bus takes as a key: letters, digits and - _ / = . with no dot at either end.
+var validKey = regexp.MustCompile(`^[-/_=.a-zA-Z0-9]+$`)
+
+// ErrBadKey is a key the bus does not take: refused loudly, never rewritten into another.
+var ErrBadKey = errors.New("the bus takes no key of that spelling")
+
+func (s Bucket[T]) guard(ctx context.Context, key string, value []byte) error {
+	if s.holds == nil {
 		return ErrNotHeld
 	}
+	if err := s.holds(ctx); err != nil {
+		return fmt.Errorf("%w: %v", ErrNotHeld, err)
+	}
+	if !validKey.MatchString(key) || strings.HasPrefix(key, ".") || strings.HasSuffix(key, ".") {
+		return fmt.Errorf("%w: %q", ErrBadKey, key)
+	}
 	if len(value) > broker.ChainValueBytes {
 		return fmt.Errorf("%w: %d bytes, the most is %d", ErrTooLarge, len(value), broker.ChainValueBytes)
 	}
@@ -57,7 +76,7 @@ func (s Bucket[T]) Create(ctx context.Context, key string, v T) (uint64, error)
 	if err != nil {
 		return 0, err
 	}
-	if err := s.guard(body); err != nil {
+	if err := s.guard(ctx, key, body); err != nil {
 		return 0, err
 	}
 	return s.b.Create(ctx, key, body)
@@ -69,29 +88,39 @@ func (s Bucket[T]) Update(ctx context.Context, key string, v T, revision uint64)
 	if err != nil {
 		return 0, err
 	}
-	if err := s.guard(body); err != nil {
+	if err := s.guard(ctx, key, body); err != nil {
 		return 0, err
 	}
 	return s.b.Update(ctx, key, body, revision)
 }
 
-// All is every record, by key. An error is an error: never an empty bucket.
+// All is every record, by key, read in one pass of a watch. An error is an error: never an empty bucket.
 func (s Bucket[T]) All(ctx context.Context) (map[string]Kept[T], error) {
-	keys, err := s.b.Keys(ctx)
+	ctx, stop := context.WithCancel(ctx)
+	defer stop()
+	w, err := s.b.Watch(ctx)
 	if err != nil {
 		return nil, err
 	}
-	out := make(map[string]Kept[T], len(keys))
-	for _, key := range keys {
-		k, ok, err := s.Get(ctx, key)
-		if err != nil {
-			return nil, err
-		}
-		if ok {
-			out[key] = k
+	out := map[string]Kept[T]{}
+	for {
+		select {
+		case <-ctx.Done():
+			return nil, fmt.Errorf("reading the bucket: %w", ctx.Err())
+		case e, open := <-w:
+			if !open {
+				return nil, errors.New("the bucket stopped answering before it was read whole")
+			}
+			if e.Caught {
+				return out, nil
+			}
+			k, err := decode[T](e)
+			if err != nil {
+				return nil, err
+			}
+			out[e.Key] = k
 		}
 	}
-	return out, nil
 }
 
 // History is a key's earlier records and its current one, oldest first.
@@ -125,8 +154,8 @@ type Chain struct {
 	Assignments Bucket[Assignment]
 }
 
-// New is a chain over three backends, written only while holds says this controller holds the lease.
-func New(builds, rollouts, assignments Backend, holds func() bool) *Chain {
+// New is a chain over three backends, written only while holds says this controller may write. A nil holds writes nothing.
+func New(builds, rollouts, assignments Backend, holds Holds) *Chain {
 	return &Chain{
 		Builds:      Bucket[Build]{builds, holds},
 		Rollouts:    Bucket[Rollout]{rollouts, holds},
@@ -135,9 +164,9 @@ func New(builds, rollouts, assignments Backend, holds func() bool) *Chain {
 }
 
 // InMemory is a chain held in this process, for tests, written whenever holds says so; nil always holds.
-func InMemory(holds func() bool) *Chain {
+func InMemory(holds Holds) *Chain {
 	if holds == nil {
-		holds = func() bool { return true }
+		holds = func(context.Context) error { return nil }
 	}
 	return New(NewMemory(), NewMemory(), NewMemory(), holds)
 }

internal/chain/chain_test.go

@@ -222,14 +222,14 @@ func TestAnAssignmentNamesBuildArtifactCommitAndRollout(t *testing.T) {
 // the bucket untouched.
 func TestTheWriterWithoutTheLeaseWritesNothing(t *testing.T) {
 	ctx := context.Background()
-	held := true
+	var away error
 	builds, rollouts, assignments := NewMemory(), NewMemory(), NewMemory()
-	c := New(builds, rollouts, assignments, func() bool { return held })
+	c := New(builds, rollouts, assignments, func(context.Context) error { return away })
 	rev, err := c.Builds.Create(ctx, "b1", Build{ID: "b1"})
 	if err != nil {
 		t.Fatal(err)
 	}
-	held = false
+	away = errors.New("another controller holds the lease")
 	if _, err := c.Builds.Create(ctx, "b2", Build{ID: "b2"}); !errors.Is(err, ErrNotHeld) {
 		t.Fatalf("a create without the lease answered %v", err)
 	}
@@ -262,7 +262,7 @@ func TestTheWriterWithoutTheLeaseWritesNothing(t *testing.T) {
 func TestABucketThatIsAwayIsAnErrorNotAnEmptyOne(t *testing.T) {
 	ctx := context.Background()
 	m := NewMemory()
-	c := New(m, NewMemory(), NewMemory(), func() bool { return true })
+	c := New(m, NewMemory(), NewMemory(), func(context.Context) error { return nil })
 	if _, err := c.Builds.Create(ctx, "b1", Build{ID: "b1"}); err != nil {
 		t.Fatal(err)
 	}
@@ -310,3 +310,27 @@ func TestAWatchHandsOverTheValuesThenEachWrite(t *testing.T) {
 		t.Fatalf("third %+v", e)
 	}
 }
+
+// **A key the bus does not take is refused, never rewritten into another** (ADR 0408 §2): a write names the key loudly.
+func TestAKeyTheBusTakesNoSpellingOfIsRefused(t *testing.T) {
+	ctx := context.Background()
+	c := InMemory(nil)
+	for _, key := range []string{"", "build one", "build*", ".hidden", "trailing.", "a>b"} {
+		if _, err := c.Builds.Create(ctx, key, Build{ID: key}); !errors.Is(err, ErrBadKey) {
+			t.Errorf("the key %q answered %v", key, err)
+		}
+	}
+	for _, key := range []string{"build-1700000000000000000", "anchor.notes", "a/b_c=d"} {
+		if _, err := c.Builds.Create(ctx, key, Build{ID: key}); err != nil {
+			t.Errorf("the key %q answered %v", key, err)
+		}
+	}
+}
+
+// An empty bucket is read as empty, and the read ends.
+func TestAnEmptyBucketIsReadAsEmpty(t *testing.T) {
+	all, err := InMemory(nil).Assignments.All(context.Background())
+	if err != nil || len(all) != 0 {
+		t.Fatalf("%v %v", all, err)
+	}
+}