serve a declared slug as an address, leaving the key alone

The human chose the second option: a route sits beside the key rather than replacing it. So `slug` renames what a bundle is served at, in every language, and identity stays derived from the path — which is exactly what keeps ADR-0033 intact, since series membership is the directory. A series landing page can now be renamed without orphaning its chapters, and there is a test that says so. `Site` resolves routes at index time, because only it can see whether every variant agrees. Disagreement is dropped rather than resolved, as is a slug landing where another bundle already answers — the same rule colliding keys and contested aliases already follow. The key a slug moved away from stops answering, so the old address does not quietly keep working. Two bugs surfaced doing this, both older than this change: An alias naming its own bundle's former key was rejected as "an alias that names a real bundle" — which made rename-plus-alias, the entire point of ADR-0008's alias mechanism, impossible. The check now asks what a request asks: is anything actually served there. Aliases were counted per declaring *file*, so a bundle whose two language variants both listed the same alias looked like two rival claimants and lost the alias. It is a set of keys now. This one only appears with translated content, which is why no fixture had caught it since entry 4 — the real binary did, on the first multilingual rename.
2026-07-31 02:34:07 +06:00
parent e6c0673374
commit e7287a0370
10 changed files with 330 additions and 26 deletions
@@ -52,7 +52,7 @@ func serveSitemap(w http.ResponseWriter, req *http.Request, site *content.Site,
 	out.WriteString(`<?xml version="1.0" encoding="utf-8"?>` + "\n")
 	out.WriteString(`<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">` + "\n")
 	for _, entry := range site.Everything() {
-		fmt.Fprintf(&out, "<url><loc>%s</loc>", xmlEscape(content.Absolute(base, content.URL(entry.Key, entry.Lang))))
+		fmt.Fprintf(&out, "<url><loc>%s</loc>", xmlEscape(content.Absolute(base, content.URL(entry.Route, entry.Lang))))
 		if !entry.Date.IsZero() {
 			fmt.Fprintf(&out, "<lastmod>%s</lastmod>", entry.Date.Format("2006-01-02"))
 		}
@@ -0,0 +1,156 @@
+package web
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"testing/fstest"
+
+	"khosra/internal/content"
+	"khosra/internal/render"
+)
+
+func slugHandler(t *testing.T, fsys fstest.MapFS) http.Handler {
+	t.Helper()
+	bundles, err := content.Scan(fsys)
+	if err != nil {
+		t.Fatal(err)
+	}
+	r, err := render.New(nil, content.Settings{}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return Handler(content.NewSite(bundles), r, fsys, content.Settings{})
+}
+
+func TestASlugRenamesTheAddressInEveryLanguage(t *testing.T) {
+	h := slugHandler(t, fstest.MapFS{
+		"content/posts/hello-world.md":    {Data: []byte("---\ntitle: Hello\nslug: ekti-post\n---\nEnglish.\n")},
+		"content/posts/hello-world.bn.md": {Data: []byte("---\ntitle: একটি পোস্ট\n---\nবাংলা।\n")},
+	})
+	for _, path := range []string{"/posts/ekti-post/", "/bn/posts/ekti-post/"} {
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+		if rec.Code != http.StatusOK {
+			t.Errorf("GET %s = %d, want 200 — a slug renames every language (ADR-0035)", path, rec.Code)
+		}
+	}
+	// The old path is gone: the engine serves the new one only, and an author who wants both writes an alias.
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/hello-world/", nil))
+	if rec.Code != http.StatusNotFound {
+		t.Errorf("the derived path = %d, want 404", rec.Code)
+	}
+	// Canonical and hreflang must name the new address, not the key.
+	rec = httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/bn/posts/ekti-post/", nil))
+	body := rec.Body.String()
+	if !strings.Contains(body, `rel="canonical" href="/bn/posts/ekti-post/"`) {
+		t.Errorf("canonical should name the served address:\n%s", body)
+	}
+	if strings.Contains(body, "hello-world") {
+		t.Errorf("nothing the engine emits should still say hello-world:\n%s", body)
+	}
+}
+
+func TestAnAliasCanKeepTheOldPathWorking(t *testing.T) {
+	h := slugHandler(t, fstest.MapFS{
+		"content/posts/hello-world.md": {Data: []byte("---\ntitle: Hello\nslug: ekti-post\naliases: [posts/hello-world]\n---\nx\n")},
+	})
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/hello-world/", nil))
+	if rec.Code != http.StatusMovedPermanently {
+		t.Fatalf("got %d, want 301 — this is how a rename keeps its promise (ADR-0008)", rec.Code)
+	}
+	if loc := rec.Header().Get("Location"); loc != "/posts/ekti-post/" {
+		t.Errorf("Location = %q, want the new address", loc)
+	}
+}
+
+func TestListingsAndSitemapsUseTheSluggedAddress(t *testing.T) {
+	fsys := fstest.MapFS{
+		"content/posts/hello-world.md": {Data: []byte("---\ntitle: Hello\ndate: 2026-03-01\nslug: ekti-post\n---\nx\n")},
+		"content/posts/plain.md":       {Data: []byte("---\ntitle: Plain\ndate: 2026-02-01\n---\nx\n")},
+	}
+	bundles, err := content.Scan(fsys)
+	if err != nil {
+		t.Fatal(err)
+	}
+	settings := content.Settings{Base: "https://khosra.example"}
+	r, err := render.New(nil, settings, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	h := Handler(content.NewSite(bundles), r, fsys, settings)
+
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/", nil))
+	if body := rec.Body.String(); !strings.Contains(body, `href="/posts/ekti-post/"`) || strings.Contains(body, "hello-world") {
+		t.Errorf("a listing must link the address, not the key:\n%s", body)
+	}
+	rec = httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/sitemap.xml", nil))
+	if body := rec.Body.String(); !strings.Contains(body, "/posts/ekti-post/") || strings.Contains(body, "hello-world") {
+		t.Errorf("a sitemap that disagrees with what is served is worse than none:\n%s", body)
+	}
+}
+
+func TestAmbiguousOrCollidingSlugsAreDropped(t *testing.T) {
+	// Both cases keep the derived path rather than picking a winner, the same rule aliases follow.
+	h := slugHandler(t, fstest.MapFS{
+		// Variants disagree.
+		"content/posts/a.md":    {Data: []byte("---\ntitle: A\nslug: one\n---\nx\n")},
+		"content/posts/a.bn.md": {Data: []byte("---\ntitle: ক\nslug: two\n---\nx\n")},
+		// Slug lands on a bundle that already exists.
+		"content/posts/b.md":     {Data: []byte("---\ntitle: B\nslug: taken\n---\nx\n")},
+		"content/posts/taken.md": {Data: []byte("---\ntitle: Taken\n---\nx\n")},
+	})
+	for path, want := range map[string]int{
+		"/posts/a/":     http.StatusOK,       // kept its derived path
+		"/posts/one/":   http.StatusNotFound, // neither declared slug won
+		"/posts/two/":   http.StatusNotFound,
+		"/posts/b/":     http.StatusOK,
+		"/posts/taken/": http.StatusOK, // the bundle already there keeps its URL
+	} {
+		rec := httptest.NewRecorder()
+		h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
+		if rec.Code != want {
+			t.Errorf("GET %s = %d, want %d", path, rec.Code, want)
+		}
+	}
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/posts/taken/", nil))
+	if body := rec.Body.String(); !strings.Contains(body, "<h1>Taken</h1>") {
+		t.Errorf("the bundle already there must still be the one served:\n%s", body)
+	}
+}
+
+func TestASeriesSurvivesItsLandingBeingSlugged(t *testing.T) {
+	// The reason a slug does not touch the key: membership is the directory (ADR-0033), so renaming the
+	// landing page's address must not orphan its chapters.
+	h := slugHandler(t, fstest.MapFS{
+		"content/comics/the-long-monsoon/_index.md":     {Data: []byte("---\ntitle: The Long Monsoon\nslug: monsoon\n---\nx\n")},
+		"content/comics/the-long-monsoon/first-rain.md": {Data: []byte("---\ntitle: First Rain\norder: 10\n---\nx\n")},
+		"content/comics/the-long-monsoon/the-flood.md":  {Data: []byte("---\ntitle: The Flood\norder: 20\n---\nx\n")},
+	})
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/comics/monsoon/", nil))
+	if rec.Code != http.StatusOK {
+		t.Fatalf("the landing page = %d, want 200 at its new address", rec.Code)
+	}
+	body := rec.Body.String()
+	for _, want := range []string{"First Rain", "The Flood"} {
+		if !strings.Contains(body, want) {
+			t.Errorf("the archive lost %q — a slug must not orphan a series:\n%s", want, body)
+		}
+	}
+	rec = httptest.NewRecorder()
+	h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/comics/the-long-monsoon/the-flood/", nil))
+	if rec.Code != http.StatusOK {
+		t.Errorf("a chapter = %d, want 200: chapters keep their own addresses", rec.Code)
+	}
+	if body := rec.Body.String(); !strings.Contains(body, `href="/comics/monsoon/"`) {
+		t.Errorf("the chapter should link its series at the slugged address:\n%s", body)
+	}
+}
@@ -136,7 +136,10 @@ func serve(w http.ResponseWriter, req *http.Request, site *content.Site, r *rend
 	}
 	// A redirect target only exists for a path that resolves, so check the bundle before sending one:
 	// otherwise a nonexistent page answers 301 and confirms nothing.
-	b, served, found := site.Lookup(res.key, res.lang)
+	// A request path is a route: a slug may have moved a bundle there, and moved another away (ADR-0035).
+	key, live := site.KeyFor(res.key)
+	b, served, found := site.Lookup(key, res.lang)
+	found = found && live
 	if res.redirect != "" && (found || res.key == "") {
 		http.Redirect(w, req, res.redirect, http.StatusMovedPermanently)
 		return
@@ -145,7 +148,7 @@ func serve(w http.ResponseWriter, req *http.Request, site *content.Site, r *rend
 		// An alias is a promise that an old URL keeps working, so it answers a permanent redirect to the
 		// canonical one — and only for an alias that exists, so nothing can be probed by 301.
 		if canonical, isAlias := site.Alias(res.key); isAlias {
-			http.Redirect(w, req, content.URL(canonical, res.lang), http.StatusMovedPermanently)
+			http.Redirect(w, req, content.URL(site.RouteOf(canonical), res.lang), http.StatusMovedPermanently)
 			return
 		}
 		if serveListing(w, req, site, r, res) {
@@ -160,7 +163,7 @@ func serve(w http.ResponseWriter, req *http.Request, site *content.Site, r *rend
 	if series, inSeries := site.Sequence(b.Key, served); inSeries {
 		seq = &series
 	}
-	out, err := r.Bundle(b, served, site.Variants(res.key), seq)
+	out, err := r.Bundle(b, served, site.Variants(key), seq)
 	if err != nil {
 		// A render failure degrades: log it and say nothing more to the client than that it failed
 		// (conventions.md). It must never leak a template or filesystem detail.