Let a process consume its module's work queue, acknowledging, delaying or terminating each message (issue 1190)

2026-10-19 04:38:45 +02:00
parent 94702bcd79
commit f73ec7457a
3 changed files with 379 additions and 0 deletions
@@ -0,0 +1,127 @@
+package process
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"strings"
+	"time"
+
+	"github.com/nats-io/nats.go/jetstream"
+)
+
+// Message is one piece of work this process's queue consumer delivered, with how many times the bus has
+// delivered it. Nothing acts on its behalf: the handler decides its fate exactly once, by calling Ack,
+// NakWithDelay or Term (mesh-tools node-tools/internal/bus/seat.go's seat work Consume, with the same
+// three outcomes, offered here explicitly instead of by a handler's return value, since the caller
+// chooses its own delay before a message comes back).
+type Message struct {
+	Subject string
+	Body    []byte
+	Headers map[string]string
+	// Delivered is how many times the bus has handed this message over, this time included.
+	Delivered uint64
+
+	ack  func() error
+	nak  func(time.Duration) error
+	term func() error
+}
+
+// Ack acknowledges the message: the bus never delivers it again.
+func (m *Message) Ack() error { return m.ack() }
+
+// NakWithDelay asks for the message to be delivered again no sooner than delay, its delivery count
+// raised by one.
+func (m *Message) NakWithDelay(delay time.Duration) error { return m.nak(delay) }
+
+// Term terminates the message: the bus never delivers it again, and does not count it a dead letter the
+// way running out of deliveries does.
+func (m *Message) Term() error { return m.term() }
+
+// ProcessID is this account's own process id, the last segment of its user name <node>.<module>.<id>
+// (mesh-controller internal/broker, Principal.Username for KindProcess).
+func (a Account) ProcessID() string {
+	prefix := a.Node + "." + a.Module + "."
+	if !strings.HasPrefix(a.User, prefix) {
+		return ""
+	}
+	return strings.TrimPrefix(a.User, prefix)
+}
+
+// QueueConsumerName is the one durable consumer a process's bus.queues names it on its work-queue stream
+// (novox/hq ADR 0436 clauses 1, 2, 6; mesh-controller internal/broker/modstreams.go QueueConsumer):
+// <node>_<module>_<process id>.
+func (a Account) QueueConsumerName() string { return a.Node + "_" + a.Module + "_" + a.ProcessID() }
+
+// errNoMessage is one pull that fetched nothing within its wait: not an error to the caller, just a
+// reason to pull again.
+var errNoMessage = errors.New("no message this pull")
+
+// Consume binds this process's one durable consumer on stream (QueueConsumerName) and pulls from it, one
+// message at a time, handing each to handle until ctx ends: it then stops pulling and returns ctx.Err(),
+// never acknowledging a message still in handle's hands — only handle decides a message's fate, by
+// calling Ack, NakWithDelay or Term on it.
+func (c *Conn) Consume(ctx context.Context, stream string, handle func(*Message)) error {
+	js, err := jetstream.New(c.nc)
+	if err != nil {
+		return err
+	}
+	name := c.account.QueueConsumerName()
+	cons, err := js.Consumer(ctx, stream, name)
+	if err != nil {
+		return fmt.Errorf("binding %s's queue consumer %s on %s: %w", c.account.Module, name, stream, err)
+	}
+	return runQueue(ctx, fetchOne(cons), handle)
+}
+
+// fetchOne is one pull of at most one message, non-blocking past ctx: Fetch itself does not take a
+// context, so this selects on its message channel and ctx together, and returns promptly once ctx ends
+// even while a fetch is still outstanding.
+func fetchOne(cons jetstream.Consumer) func(ctx context.Context) (*Message, error) {
+	return func(ctx context.Context) (*Message, error) {
+		batch, err := cons.Fetch(1, jetstream.FetchMaxWait(5*time.Second))
+		if err != nil {
+			return nil, err
+		}
+		select {
+		case msg, ok := <-batch.Messages():
+			if !ok {
+				return nil, errNoMessage
+			}
+			headers := map[string]string{}
+			for k := range msg.Headers() {
+				headers[k] = msg.Headers().Get(k)
+			}
+			var delivered uint64 = 1
+			if meta, err := msg.Metadata(); err == nil && meta != nil {
+				delivered = meta.NumDelivered
+			}
+			return &Message{Subject: msg.Subject(), Body: append([]byte(nil), msg.Data()...), Headers: headers,
+				Delivered: delivered, ack: msg.Ack, nak: msg.NakWithDelay, term: msg.Term}, nil
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		}
+	}
+}
+
+// runQueue is Consume's loop, apart from the bus so a unit test can drive it with a fake next
+// (novox/hq issue 1190 AC4): it checks ctx before every pull, and stops as soon as next or the check
+// says ctx has ended, without touching the message past handing it to handle.
+func runQueue(ctx context.Context, next func(context.Context) (*Message, error), handle func(*Message)) error {
+	for {
+		if err := ctx.Err(); err != nil {
+			return err
+		}
+		msg, err := next(ctx)
+		if err != nil {
+			if ctx.Err() != nil {
+				return ctx.Err()
+			}
+			if errors.Is(err, errNoMessage) {
+				continue
+			}
+			return err
+		}
+		handle(msg)
+	}
+}
@@ -0,0 +1,181 @@
+//go:build integration
+
+package process
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/nats-io/nats.go/jetstream"
+)
+
+// mailSendsApprovedSubject is where the runtime publishes a work-queue message on mesh-mail's own stream
+// (mesh-controller internal/broker/modstreams.go ModuleStreamSubject, mail-sends' one event).
+const mailSendsApprovedSubject = "mesh.stream.mesh-mail.event.mail.send.approved"
+
+// makeQueue sets up MAIL_SENDS as a work-queue stream and binds the one durable consumer a process's
+// bus.queues names it (QueueConsumerName), both as the admin, mirroring what the controller composes
+// (mesh-controller internal/broker/modstreams.go QueueConsumer): AckWaitSeconds 30, named
+// <node>_<module>_<process id>.
+func makeQueue(t *testing.T, url string, account Account) {
+	t.Helper()
+	_, adminJetStream := adminJS(t, url)
+	if _, err := adminJetStream.CreateStream(ctxFor(t), jetstream.StreamConfig{
+		Name: "MAIL_SENDS", Subjects: []string{mailSendsApprovedSubject}, Retention: jetstream.WorkQueuePolicy,
+		Discard: jetstream.DiscardNew,
+	}); err != nil {
+		t.Fatalf("the admin could not make the work-queue stream: %v", err)
+	}
+	if _, err := adminJetStream.CreateConsumer(ctxFor(t), "MAIL_SENDS", jetstream.ConsumerConfig{
+		Durable: account.QueueConsumerName(), AckPolicy: jetstream.AckExplicitPolicy, AckWait: 30 * time.Second,
+		MaxDeliver: 5,
+	}); err != nil {
+		t.Fatalf("the admin could not make the queue consumer: %v", err)
+	}
+}
+
+func publishApproved(t *testing.T, url string, body string) {
+	t.Helper()
+	nc, _ := adminJS(t, url)
+	if err := nc.Publish(mailSendsApprovedSubject, []byte(body)); err != nil {
+		t.Fatal(err)
+	}
+	if err := nc.FlushTimeout(3 * time.Second); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// 1190/AC1: given a work-queue stream with one durable pull consumer and a process connection, when a
+// message is consumed and acknowledged, then it is gone from the stream.
+func TestConsumeAckRemovesTheMessageFromTheStream(t *testing.T) {
+	url, account := grantedSendProcess(t)
+	makeQueue(t, url, account)
+	publishApproved(t, url, `{"send":"s1"}`)
+
+	conn, err := Connect(account)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	got := make(chan *Message, 1)
+	go func() {
+		_ = conn.Consume(ctx, "MAIL_SENDS", func(m *Message) {
+			got <- m
+			cancel()
+		})
+	}()
+	var msg *Message
+	select {
+	case msg = <-got:
+	case <-time.After(8 * time.Second):
+		t.Fatal("the message was never consumed")
+	}
+	<-ctx.Done()
+	if err := msg.Ack(); err != nil {
+		t.Fatalf("Ack: %v", err)
+	}
+
+	_, adminJetStream := adminJS(t, url)
+	deadline := time.Now().Add(5 * time.Second)
+	for {
+		info, err := adminJetStream.Stream(ctxFor(t), "MAIL_SENDS")
+		if err != nil {
+			t.Fatal(err)
+		}
+		state, err := info.Info(ctxFor(t))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if state.State.Msgs == 0 {
+			return
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("the stream still holds %d messages after the ack", state.State.Msgs)
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+}
+
+// 1190/AC2: given a consumed message, when the handler asks for it to be delivered again after a delay,
+// then it comes back no sooner than the delay with its delivery count raised by one.
+func TestConsumeNakWithDelayRedeliversAfterTheDelayWithARaisedCount(t *testing.T) {
+	url, account := grantedSendProcess(t)
+	makeQueue(t, url, account)
+	publishApproved(t, url, `{"send":"s2"}`)
+
+	conn, err := Connect(account)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+
+	const delay = 2 * time.Second
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
+	deliveries := make(chan struct {
+		at        time.Time
+		delivered uint64
+	}, 2)
+	go func() {
+		_ = conn.Consume(ctx, "MAIL_SENDS", func(m *Message) {
+			deliveries <- struct {
+				at        time.Time
+				delivered uint64
+			}{time.Now(), m.Delivered}
+			if m.Delivered == 1 {
+				_ = m.NakWithDelay(delay)
+			} else {
+				_ = m.Ack()
+				cancel()
+			}
+		})
+	}()
+
+	first := <-deliveries
+	if first.delivered != 1 {
+		t.Fatalf("first delivery count: %d, want 1", first.delivered)
+	}
+	second := <-deliveries
+	if second.delivered != 2 {
+		t.Fatalf("second delivery count: %d, want 2", second.delivered)
+	}
+	if elapsed := second.at.Sub(first.at); elapsed < delay {
+		t.Fatalf("redelivered after %s, want no sooner than %s", elapsed, delay)
+	}
+}
+
+// 1190/AC3: given a consumed message, when the handler terminates it, then it is never delivered again.
+func TestConsumeTermNeverDeliversTheMessageAgain(t *testing.T) {
+	url, account := grantedSendProcess(t)
+	makeQueue(t, url, account)
+	publishApproved(t, url, `{"send":"s3"}`)
+
+	conn, err := Connect(account)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+	deliveries := make(chan *Message, 4)
+	go func() {
+		_ = conn.Consume(ctx, "MAIL_SENDS", func(m *Message) {
+			deliveries <- m
+			_ = m.Term()
+		})
+	}()
+
+	first := <-deliveries
+	if first.Delivered != 1 {
+		t.Fatalf("first delivery count: %d, want 1", first.Delivered)
+	}
+	select {
+	case again := <-deliveries:
+		t.Fatalf("a terminated message was delivered again: %+v", again)
+	case <-time.After(4 * time.Second):
+	}
+}
@@ -0,0 +1,71 @@
+package process
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+)
+
+// 1190/AC4: given the consume loop, when its context ends, then it stops pulling and returns without
+// acknowledging a message in hand.
+func TestRunQueueStopsPullingAndNeverActsOnAMessageWhenContextEnds(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+
+	acked := false
+	msg := &Message{Subject: "test", ack: func() error { acked = true; return nil },
+		nak: func(time.Duration) error { return nil }, term: func() error { return nil }}
+
+	pulls := 0
+	next := func(ctx context.Context) (*Message, error) {
+		pulls++
+		return msg, nil
+	}
+
+	handled := 0
+	done := make(chan error, 1)
+	go func() {
+		done <- runQueue(ctx, next, func(m *Message) {
+			handled++
+			// The handler does not act on the message at all: a redelivery, a termination or an
+			// acknowledgement is its choice alone, never the loop's.
+			cancel()
+		})
+	}()
+
+	select {
+	case err := <-done:
+		if !errors.Is(err, context.Canceled) {
+			t.Fatalf("runQueue returned %v, want context.Canceled", err)
+		}
+	case <-time.After(3 * time.Second):
+		t.Fatal("runQueue did not stop when its context ended")
+	}
+	if handled != 1 {
+		t.Fatalf("handle was called %d times, want 1", handled)
+	}
+	if acked {
+		t.Error("runQueue acknowledged the message itself; only the handler may")
+	}
+	if pulls != 1 {
+		t.Fatalf("next was called %d times, want exactly 1: once the handler ends the context, runQueue's "+
+			"own check stops it before pulling again", pulls)
+	}
+}
+
+// 1190/AC4: a context already ended before the loop starts pulls nothing at all.
+func TestRunQueueNeverPullsOnAnAlreadyEndedContext(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	calls := 0
+	err := runQueue(ctx, func(context.Context) (*Message, error) {
+		calls++
+		return nil, context.Canceled
+	}, func(*Message) { t.Fatal("handle was called on an already-ended context") })
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("runQueue returned %v, want context.Canceled", err)
+	}
+	if calls != 0 {
+		t.Fatalf("next was called %d times, want 0", calls)
+	}
+}