An address has one spelling, and one place that spells it

A pattern never ends in "/" now, the root aside, because the router serves "/x"
and "/x/" alike — so two spellings were two keys for one address, and a group
could not name its own root at all: declaring "" on it produced the prefix with
a trailing slash. That is why a collection root had to be declared at an
absolute path on the parent instead, which put the route outside the middleware
of the group it belonged to.

The rule was also written twice. The router composed a prefix with a leaf and
cmd/zipdoc composed the same two things to find that route's prose, so trimming
in the router alone would have filed every collection root's sentence under an
address the document does not contain — complete document, missing description,
no error anywhere. Both copies are gone; internal/addr is the one that spells it.

A test registers a trailing-slash pattern — the shape the Sentry-compatible
ingest addresses use on purpose — and asserts the route table, the registry, the
document's template, and the operation and concrete address a handler reads are
all the same string, because a service that resolves its runtime handler by name
from that pattern refuses every call if two of them disagree.

Co-authored-by: Hanzo Dev <[email protected]>

collectionroot_test.go

@@ -0,0 +1,140 @@
+package zip_test
+
+import (
+	"context"
+	"net/http/httptest"
+	"sort"
+	"testing"
+
+	"github.com/zap-proto/zip"
+)
+
+type rootIn struct{}
+type rootOut struct {
+	OK bool `json:"ok"`
+}
+
+type coll struct{}
+
+// List returns the collection.
+func (coll) List(context.Context, *rootIn) (*rootOut, error) { return &rootOut{OK: true}, nil }
+
+// Add adds to it.
+func (coll) Add(context.Context, *rootIn) (*rootOut, error) { return &rootOut{OK: true}, nil }
+
+// AN EMPTY LEAF IS THE PREFIX ITSELF. `g.Get("", h)` declares the collection
+// root at the group's own address; only an explicit "/" asks for the trailing
+// slash.
+//
+// Serving was never the problem — both spellings answer either way. The
+// DECLARED pattern is what the document, the op key and cmd/zipdoc publish, and
+// a group that could not spell its own root is why a service declared that route
+// at an absolute path on the subsystem root instead, which put it outside the
+// middleware of the group it belonged to.
+func TestGroup_AnEmptyLeafIsTheCollectionRoot(t *testing.T) {
+	app := zip.New(zip.Config{AppName: "coll", DisableStartupMessage: true})
+	g := app.Group("/v1/function")
+	g.Get("", coll{}.List)
+	g.Post("/", coll{}.Add)
+
+	var declared []string
+	for _, r := range app.Routes() {
+		declared = append(declared, r.Method+" "+r.Pattern)
+	}
+	sort.Strings(declared)
+
+	// Both spellings canonicalise to the same address; they differ only by verb.
+	want := []string{"GET /v1/function", "POST /v1/function"}
+	if len(declared) != 2 || declared[0] != want[0] || declared[1] != want[1] {
+		t.Errorf("declared %v, want %v", declared, want)
+	}
+
+	// And it still answers, at the address it now declares.
+	for _, addr := range []string{"/v1/function", "/v1/function/"} {
+		resp, err := app.Test(httptest.NewRequest("GET", addr, nil))
+		if err != nil {
+			t.Fatalf("%s: %v", addr, err)
+		}
+		_ = resp.Body.Close()
+		if resp.StatusCode != 200 {
+			t.Errorf("GET %s = %d, want 200", addr, resp.StatusCode)
+		}
+	}
+}
+
+// The root app is unaffected: with no prefix there is nothing for an empty leaf
+// to be, so it stays the root.
+func TestGroup_AnEmptyLeafAtTheRootIsStillTheRoot(t *testing.T) {
+	app := zip.New(zip.Config{AppName: "coll", DisableStartupMessage: true})
+	app.Get("", coll{}.List)
+	for _, r := range app.Routes() {
+		if r.Pattern != "/" {
+			t.Errorf("declared %q, want /", r.Pattern)
+		}
+	}
+}
+
+type ingestIn struct {
+	Project string `json:"project"`
+}
+
+type ingest struct{}
+
+// Envelope accepts a Sentry envelope for one project.
+func (ingest) Envelope(ctx context.Context, _ *ingestIn) (*rootOut, error) {
+	seenPattern, _ := zip.OpOf(ctx)
+	seenAddr, _ := zip.AddressOf(ctx)
+	ingestPattern, ingestAddr = seenPattern.Path, seenAddr
+	return &rootOut{OK: true}, nil
+}
+
+var ingestPattern, ingestAddr string
+
+// EVERY READER OF AN ADDRESS AGREES ON HOW IT IS SPELLED.
+//
+// A pattern written with a trailing slash — which the Sentry-compatible ingest
+// addresses are, deliberately — is canonicalised once, so the route table, the
+// document, the operation a handler reads and the concrete address it forwards
+// to are all the same string. A service that resolves its runtime handler BY
+// NAME from Template(pattern) is what makes this load-bearing: if registration
+// trimmed and the lookup did not, every ingest call would refuse.
+func TestGroup_EveryReaderSpellsTheAddressTheSameWay(t *testing.T) {
+	app := zip.New(zip.Config{AppName: "ingest", DisableStartupMessage: true})
+	app.Group("/v1/event/:project").Post("/envelope/", ingest{}.Envelope)
+
+	const want = "/v1/event/:project/envelope"
+
+	routes := app.Routes()
+	if len(routes) != 1 || routes[0].Pattern != want {
+		t.Fatalf("Routes() = %+v, want one at %q", routes, want)
+	}
+
+	ops := app.Manifest().Ops
+	if len(ops) != 1 || ops[0].Path != want {
+		t.Fatalf("registry = %+v, want one at %q", ops, want)
+	}
+
+	if got := zip.Template(want); got != "/v1/event/{project}/envelope" {
+		t.Errorf("Template = %q; the document's spelling of the same address", got)
+	}
+
+	// And the request serves under BOTH spellings, reporting one.
+	for _, addr := range []string{"/v1/event/abc/envelope", "/v1/event/abc/envelope/"} {
+		ingestPattern, ingestAddr = "", ""
+		resp, err := app.Test(httptest.NewRequest("POST", addr, nil))
+		if err != nil {
+			t.Fatalf("%s: %v", addr, err)
+		}
+		_ = resp.Body.Close()
+		if resp.StatusCode != 200 {
+			t.Errorf("POST %s = %d, want 200", addr, resp.StatusCode)
+			continue
+		}
+		if ingestPattern != want {
+			t.Errorf("POST %s: OpOf reported %q, want %q", addr, ingestPattern, want)
+		}
+		if ingestAddr != "/v1/event/abc/envelope" {
+			t.Errorf("POST %s: AddressOf reported %q", addr, ingestAddr)
+		}
+	}
+}

internal/addr/addr.go

@@ -0,0 +1,52 @@
+// Package addr spells an address, once, for everything that reads one.
+//
+// It exists because the rule was written twice — the router composed a group's
+// prefix with a leaf, and cmd/zipdoc composed the same two things to find the
+// prose for that route — and two copies of one rule drift. A pattern declared
+// under one spelling and filed under the other publishes an operation with no
+// description, which is silent: the document is complete, the sentence is
+// simply missing.
+package addr
+
+// Norm is a path in its ONE canonical spelling.
+//
+// A pattern never ends in "/" — the root aside — because the router serves "/x"
+// and "/x/" alike, so two spellings are two keys for one address: the document
+// publishes one, the op is filed under the other, and a lookup that composed the
+// pattern itself misses.
+func Norm(path string) string {
+	if path == "" {
+		return "/"
+	}
+	for len(path) > 1 && path[len(path)-1] == '/' {
+		path = path[:len(path)-1]
+	}
+	return path
+}
+
+// Join composes a group's prefix with a leaf, the way the router does.
+//
+// An EMPTY LEAF is the prefix itself: declaring "" on a group names the group's
+// own address, which is how a collection root is spelled. It used to name the
+// prefix with a trailing slash, so a group could not name its own root and a
+// service reached for an absolute path on the parent instead — which put the
+// route outside the middleware of the group it belonged to.
+func Join(prefix, path string) string {
+	if prefix == "" {
+		return Norm(path)
+	}
+	if path == "" || path == "/" {
+		return Norm(trimRight(prefix))
+	}
+	if path[0] != '/' {
+		path = "/" + path
+	}
+	return Norm(trimRight(prefix) + path)
+}
+
+func trimRight(s string) string {
+	for len(s) > 0 && s[len(s)-1] == '/' {
+		s = s[:len(s)-1]
+	}
+	return s
+}

internal/zipdoc/extract.go

@@ -14,6 +14,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"github.com/zap-proto/zip/internal/addr"
 	"go/ast"
 	"go/constant"
 	"go/parser"
@@ -565,18 +566,9 @@ func isZipGroup(t types.Type) bool {
 
 // joinPath composes a prefix with a leaf the way the router does, so the key this
 // pass writes is the path zip registered.
-func joinPath(prefix, path string) string {
+// joinPath is [addr.Join] — the router's own rule, so a route this pass files
-	if path == "" {
+// prose under is the address the router serves.
-		path = "/"
+func joinPath(prefix, path string) string { return addr.Join(prefix, path) }
-	}
-	if prefix == "" {
-		return path
-	}
-	if path[0] != '/' {
-		path = "/" + path
-	}
-	return strings.TrimRight(prefix, "/") + path
-}
 
 // calleeIdent is the identifier being called, past any parens and any explicit
 // type arguments: zip.Get, zip.Get[A,B], Get and Get[A,B] all yield the Get.

internal/zipdoc/extract_test.go

@@ -282,7 +282,9 @@ func TestExtract_BothTablesLiftTheSameDocs(t *testing.T) {
 		t.Errorf("legacy response = %s", legacy.Response)
 	}
 
-	scoped := opByKey(t, p, "POST /v1/items/")
+	// The group's leaf is "/", which IS the group's own address — one canonical
+	// spelling, the same one the router declares and the document publishes.
+	scoped := opByKey(t, p, "POST /v1/items")
 	if !strings.HasPrefix(scoped.Description, "Creates a new catalog item") {
 		t.Errorf("scoped description = %q", scoped.Description)
 	}

internal/zipdoc/testdata/tables/zipdoc_gen.go

@@ -9,7 +9,7 @@ import (
 )
 
 func init() {
-	zip.Describe("github.com/zap-proto/zip/internal/zipdoc/testdata/tables POST /v1/items/", zip.Doc{
+	zip.Describe("github.com/zap-proto/zip/internal/zipdoc/testdata/tables POST /v1/items", zip.Doc{
 		Description: "Creates a new catalog item.",
 		Fields: map[string]string{
 			"ItemIn.name":  "Name of the item.",

router.go

@@ -1,7 +1,7 @@
 package zip
 
 import (
-	"strings"
+	"github.com/zap-proto/zip/internal/addr"
 
 	"github.com/zap-proto/fiber/v3"
 )
@@ -26,16 +26,7 @@ type opScope struct {
 // so a typed op's identity IS the route it registered — the document, the tool
 // name and the command all read op.Path, and a path composed by a second rule
 // would name a route that does not exist.
-func joinPath(prefix, path string) string {
+func joinPath(prefix, path string) string { return addr.Join(prefix, path) }
-	path = normPath(path)
-	if prefix == "" {
-		return path
-	}
-	if path[0] != '/' {
-		path = "/" + path
-	}
-	return strings.TrimRight(prefix, "/") + path
-}
 
 // splitChain adapts one registration chain — middleware first, the final
 // handler LAST — to fiber's variadic signature. fiber
@@ -46,12 +37,10 @@ func joinPath(prefix, path string) string {
 // normPath maps the empty leaf to the group root: Get("") on a Group("/x")
 // means "/x". fiber never matches an empty path, so
 // the normalization lives here — one place, every route method.
-func normPath(path string) string {
+// normPath and joinPath are [addr.Norm] and [addr.Join]. The rule lives in one
-	if path == "" {
+// package because cmd/zipdoc composes the same two things to find a route's
-		return "/"
+// prose, and a second copy of it drifted.
-	}
+func normPath(path string) string { return addr.Norm(path) }
-	return path
-}
 
 func splitChain(app *App, handlers []Handler) (fiber.Handler, []any) {
 	if len(handlers) == 0 {