fix(publicsite): send a post or page to its one address

docs/src/content/docs/reference/content-api.md

@@ -436,7 +436,8 @@ curl "https://example.com/api/content/v1/resolve?path=/categories/news"
 
 An item filed under the term through two different fields is listed
 once. Term pages paginate behind `/page/{n}` like archives, and a
-page suffix on an ordinary single item stays `404`.
+page suffix on an ordinary single item stays `404` here, while the
+built-in renderer redirects it to the item's own address.
 
 Item answers carry an `ETag` computed over the whole served answer.
 Send it back as `If-None-Match` and an unchanged answer comes back

docs/src/content/docs/self-hosting/configuration.md

@@ -643,5 +643,10 @@ listing address that ends in a slash gets a `301` redirect to the
 plain address, with the query kept. So `/page/1` goes to `/`,
 `/page/02` goes to `/page/2` and `/pages/` goes to `/pages`. Only a
 `GET` or `HEAD` is redirected, and a listing page stays where it
-was asked when other content is stored at its plain address. While
-a theme serves the site, the theme answers these addresses itself.
+was asked when other content is stored at its plain address.
+
+Each post and page has one address too. A visit with a trailing
+slash, with repeated slashes or with a page number gets the same
+`301` to the plain address, so `/hello-world/`, `//hello-world` and
+`/hello-world/page/1` all go to `/hello-world`. While a theme serves
+the site, the theme answers these addresses itself.

internal/content/resolver.go

@@ -73,6 +73,33 @@ func (r *Resolver) Resolve(ctx context.Context, raw string) (Address, error) {
 	return r.archiveAt(ctx, address)
 }
 
+// PagedItem returns the post or page an address asks a page number of, or [ErrNotFound].
+func (r *Resolver) PagedItem(ctx context.Context, raw string) (Address, error) {
+	if !storableAddress(raw) {
+		return Address{}, ErrNotFound
+	}
+	address := tidy(raw)
+	word, _, err := listingAt(address)
+	if err != nil {
+		return Address{}, err
+	}
+	if word == address || word == "" {
+		return Address{}, ErrNotFound
+	}
+	item, err := r.content.PublishedByPath(ctx, word)
+	if err != nil {
+		return Address{}, err
+	}
+	held, err := r.itemAt(ctx, item, 1)
+	if err != nil {
+		return Address{}, err
+	}
+	if held.Kind != KindItem {
+		return Address{}, ErrNotFound
+	}
+	return held, nil
+}
+
 // itemAt returns what an item answers with, which is a term page when its type lists its pointers.
 func (r *Resolver) itemAt(ctx context.Context, item Content, page int) (Address, error) {
 	listed, err := r.types.ByKey(ctx, item.Type)

internal/content/resolver_test.go

@@ -287,3 +287,51 @@ func TestResolveAnswersAnAddressNoStoreCanHoldAsNotFound(t *testing.T) {
 		t.Errorf("lookups = %d, want none for an address no store can hold", store.lookups)
 	}
 }
+
+func TestPagedItemFindsThePostOrPageBehindAPageNumber(t *testing.T) {
+	t.Parallel()
+
+	store := newFakeAddresses()
+	hello := store.hold("post", "hello-world")
+	team := store.hold("page", "pages/about/team")
+	resolver := newResolver(t, store)
+
+	for raw, want := range map[string]content.Content{
+		"/hello-world/page/1":       hello,
+		"/hello-world/page/02/":     hello,
+		"/pages/about/team/page/3":  team,
+		"//pages/about/team/page/1": team,
+	} {
+		answer, err := resolver.PagedItem(t.Context(), raw)
+		if err != nil {
+			t.Errorf("PagedItem(%q) error = %v, want nil", raw, err)
+			continue
+		}
+		if answer.Kind != content.KindItem || answer.Item.ID != want.ID {
+			t.Errorf("PagedItem(%q) = %+v, want the item at %q", raw, answer, want.Path)
+		}
+	}
+}
+
+func TestPagedItemAnswersNotFoundWithoutAnItemBehindAPageNumber(t *testing.T) {
+	t.Parallel()
+
+	store := newFakeAddresses()
+	store.hold("post", "hello-world")
+	resolver := newResolver(t, store)
+
+	for name, raw := range map[string]string{
+		"no page number":         "/hello-world",
+		"a page below the first": "/hello-world/page/0",
+		"a word for a number":    "/hello-world/page/next",
+		"the front page":         "/page/1",
+		"an archive":             "/pages/page/1",
+		"nothing stored":         "/nowhere/page/1",
+		"a NUL byte":             "/hello\x00world/page/1",
+	} {
+		_, err := resolver.PagedItem(t.Context(), raw)
+		if !errors.Is(err, content.ErrNotFound) {
+			t.Errorf("%s error = %v, want %v", name, err, content.ErrNotFound)
+		}
+	}
+}

internal/publicsite/canonical_address_test.go

@@ -3,6 +3,7 @@
 package publicsite_test
 
 import (
+	"context"
 	"errors"
 	"net/http"
 	"net/http/httptest"
@@ -13,8 +14,28 @@ import (
 	"github.com/google/uuid"
 
 	"github.com/gopherium/gophenberg/internal/content"
+	"github.com/gopherium/gophenberg/internal/publicsite"
 )
 
+// failingReread serves like its reader but fails every read of path after its first.
+type failingReread struct {
+	*fakeReader
+	path  string
+	err   error
+	reads int
+}
+
+// PublishedByPath lets the first read of path through and returns err for every later one.
+func (r *failingReread) PublishedByPath(ctx context.Context, path string) (content.Content, error) {
+	if path == r.path {
+		r.reads++
+		if r.reads > 1 {
+			return content.Content{}, r.err
+		}
+	}
+	return r.fakeReader.PublishedByPath(ctx, path)
+}
+
 // storedAt returns a published item of the type stored at the path.
 func storedAt(kind, title, path string) content.Content {
 	stored := publishedPost(title, path, blockMarkup, time.Now().UTC())
@@ -167,3 +188,112 @@ func TestSiteSendsNothingButTheRedirect(t *testing.T) {
 		t.Errorf("GET /page/1 body = %q, want none", recorder.Body.String())
 	}
 }
+
+func TestSiteSendsAPostOrPageToTheOneAddressItAnswersAt(t *testing.T) {
+	t.Parallel()
+
+	hello := publishedPost("Hello World", "hello-world", blockMarkup, time.Now().UTC())
+	about := storedAt("page", "About", "pages/about")
+	handler, _ := siteWith(hello, about)
+
+	for _, tc := range []struct {
+		requested string
+		want      string
+	}{
+		{"/hello-world/", "/hello-world"},
+		{"//hello-world", "/hello-world"},
+		{"/hello-world//", "/hello-world"},
+		{"/hello-world/?utm_source=feed", "/hello-world?utm_source=feed"},
+		{"/pages/about/", "/pages/about"},
+		{"/pages//about", "/pages/about"},
+		{"/hello-world/page/1", "/hello-world"},
+		{"/hello-world/page/2", "/hello-world"},
+		{"/hello-world/page/1/", "/hello-world"},
+		{"/hello-world/page/01?utm_source=feed", "/hello-world?utm_source=feed"},
+		{"/pages/about/page/1", "/pages/about"},
+	} {
+		recorder := get(t, handler, tc.requested)
+
+		if recorder.Code != http.StatusMovedPermanently {
+			t.Errorf("GET %s status = %d, want %d", tc.requested, recorder.Code, http.StatusMovedPermanently)
+		}
+		if got := recorder.Header().Get("Location"); got != tc.want {
+			t.Errorf("GET %s Location = %q, want %q", tc.requested, got, tc.want)
+		}
+	}
+}
+
+func TestSiteKeepsTheAddressesAPostOrPageAlreadyAnswersAt(t *testing.T) {
+	t.Parallel()
+
+	hello := publishedPost("Hello World", "hello-world", blockMarkup, time.Now().UTC())
+	about := storedAt("page", "About", "pages/about")
+	handler, _ := siteWith(hello, about)
+
+	for _, address := range []string{"/", "/hello-world", "/pages/about"} {
+		recorder := get(t, handler, address)
+
+		if recorder.Code != http.StatusOK {
+			t.Errorf("GET %s status = %d, want %d", address, recorder.Code, http.StatusOK)
+		}
+	}
+}
+
+func TestSiteAnswersNotFoundForAPageNumberNothingHangsFrom(t *testing.T) {
+	t.Parallel()
+
+	handler, _ := siteWith(publishedPost("Hello World", "hello-world", blockMarkup, time.Now().UTC()))
+
+	for _, address := range []string{"/hello-world/page/0", "/nowhere/page/1", "/hello-world/page/next"} {
+		recorder := get(t, handler, address)
+
+		if recorder.Code != http.StatusNotFound {
+			t.Errorf("GET %s status = %d, want %d", address, recorder.Code, http.StatusNotFound)
+		}
+	}
+}
+
+func TestSiteReportsAPostItCannotReadBehindAPageNumber(t *testing.T) {
+	t.Parallel()
+
+	reader := &failingReread{
+		fakeReader: &fakeReader{posts: []content.Content{
+			publishedPost("Hello World", "hello-world", blockMarkup, time.Now().UTC()),
+		}},
+		path: "hello-world",
+		err:  errors.New("database down"),
+	}
+	handler := publicsite.New(publicsite.Config{Content: reader, Types: fakeTypes{}})
+
+	recorder := get(t, handler, "/hello-world/page/2")
+
+	if recorder.Code != http.StatusInternalServerError {
+		t.Errorf("GET /hello-world/page/2 status = %d, want %d", recorder.Code, http.StatusInternalServerError)
+	}
+}
+
+func TestSiteRedirectsOnlyAReadOfAPostOrPage(t *testing.T) {
+	t.Parallel()
+
+	handler, _ := siteWith(publishedPost("Hello World", "hello-world", blockMarkup, time.Now().UTC()))
+
+	for _, tc := range []struct {
+		method    string
+		requested string
+		want      int
+	}{
+		{http.MethodGet, "/hello-world/", http.StatusMovedPermanently},
+		{http.MethodHead, "/hello-world/", http.StatusMovedPermanently},
+		{http.MethodPost, "/hello-world/", http.StatusOK},
+		{http.MethodHead, "/hello-world/page/1", http.StatusMovedPermanently},
+		{http.MethodPost, "/hello-world/page/1", http.StatusNotFound},
+	} {
+		recorder := httptest.NewRecorder()
+
+		handler.ServeHTTP(recorder, httptest.NewRequest(tc.method, tc.requested, nil))
+
+		if recorder.Code != tc.want {
+			t.Errorf("%s %s status = %d, want %d", tc.method, tc.requested, recorder.Code, tc.want)
+		}
+	}
+}

internal/publicsite/publicsite.go

@@ -144,29 +144,39 @@ func New(cfg Config) http.Handler {
 func (s *site) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	held, err := s.addresses.Resolve(r.Context(), r.URL.Path)
 	if errors.Is(err, content.ErrNotFound) {
+		moved, err := s.pagedItemTarget(r)
+		if err != nil {
+			serveError(w)
+			return
+		}
+		if moved != "" {
+			serveRedirect(w, r, moved)
+			return
+		}
 		s.serveNotFound(w, r)
 		return
 	}
 	if err != nil {
 		serveError(w)
 		return
 	}
-	if held.Kind == content.KindItem {
-		s.serveItem(w, r, held.Item)
-		return
-	}
 	if moved := s.redirectTarget(r, held); moved != "" {
 		serveRedirect(w, r, moved)
 		return
 	}
+	if held.Kind == content.KindItem {
+		s.serveItem(w, r, held.Item)
+		return
+	}
 	if held.Kind == content.KindArchive {
 		s.serveList(w, r, held.Type, held.Page)
 		return
 	}
 	s.serveTerm(w, r, held)
 }
 
-// redirectTarget returns the one address a read of a listing page moves to, empty when it stays where it was asked.
+// redirectTarget returns the one address a read of a post, page or listing page moves to, empty when it stays where
+// it was asked.
 func (s *site) redirectTarget(r *http.Request, held content.Address) string {
 	if r.Method != http.MethodGet && r.Method != http.MethodHead {
 		return ""
@@ -182,7 +192,22 @@ func (s *site) redirectTarget(r *http.Request, held content.Address) string {
 	return canonical
 }
 
-// canonicalAddress returns the one address a listing page answers at.
+// pagedItemTarget returns the plain address of the post or page a read asks a page number of, empty when there is none.
+func (s *site) pagedItemTarget(r *http.Request) (string, error) {
+	if r.Method != http.MethodGet && r.Method != http.MethodHead {
+		return "", nil
+	}
+	held, err := s.addresses.PagedItem(r.Context(), r.URL.Path)
+	if errors.Is(err, content.ErrNotFound) {
+		return "", nil
+	}
+	if err != nil {
+		return "", err
+	}
+	return "/" + held.Item.Path, nil
+}
+
+// canonicalAddress returns the one address a post, page or listing page answers at.
 func canonicalAddress(held content.Address) string {
 	listing := held.Item.Path
 	if held.Kind == content.KindArchive {

test/features/features/content-serving.feature

@@ -4,6 +4,7 @@ Feature: Serving content by address
   route word, and the front page always keeps its numbered pages. A listing
   page answers at one address, so page 1, a padded number and a trailing
   slash move for good to it, unless content already holds that address.
+  A post or page answers at one address too.
 
   Background:
     Given a running Gophenberg with the default content types
@@ -40,6 +41,15 @@ Feature: Serving content by address
   Scenario: A listing that moves keeps its query
     Then "/page/1?ref=feed" moves for good to "/?ref=feed"
 
+  Scenario: A post with a trailing slash moves to the address without it
+    Then "/hello-world/" moves for good to "/hello-world"
+
+  Scenario: A page asked with repeated slashes moves to its plain address
+    Then "/pages//about" moves for good to "/pages/about"
+
+  Scenario: A page number on a post moves to the post
+    Then "/hello-world/page/1?ref=feed" moves for good to "/hello-world?ref=feed"
+
   Scenario: Exact content beats pagination
     Given the published page "Page"
     And the published page "2" filed under "Page"

test/features/features/content-terms.feature

@@ -52,9 +52,9 @@ Feature: Serving term pages
     Then the answer is a term of "category"
     And it holds "Second filed" alone of 3 related items
 
-  Scenario: A page suffix on a single item answers nothing
+  Scenario: A page suffix on a single item resolves nothing
     Given the published post "Hello world"
-    Then "/hello-world/page/2" answers not found
+    When a visitor resolves "/hello-world/page/2" and finds nothing
 
   Scenario: A term page holds no validator of its own
     Given the published post "Hello world" filed under "News"

test/features/steps_content_terms_test.go

@@ -292,6 +292,7 @@ func initializeContentTerms(sc *godog.ScenarioContext) {
 	sc.Given(`^the type "([^"]*)" labeled "([^"]*)" and "([^"]*)" under "([^"]*)"$`, theTypeExists)
 	sc.When(`^a visitor resolves "([^"]*)"$`, aVisitorResolves)
 	sc.When(`^a visitor resolves "([^"]*)" one item at a time$`, aVisitorResolvesOneAtATime)
+	sc.When(`^a visitor resolves "([^"]*)" and finds nothing$`, aVisitorResolvesNothingAt)
 	sc.When(`^the administrator trashes "([^"]*)"$`, theAdministratorTrashes)
 	sc.When(`^the administrator deactivates the type "([^"]*)"$`, theTypeIsDeactivated)
 	sc.Then(`^the answer is a term of "([^"]*)"$`, theAnswerIsATermOf)
@@ -303,5 +304,4 @@ func initializeContentTerms(sc *godog.ScenarioContext) {
 	sc.Then(`^one item is related$`, oneItemIsRelated)
 	sc.Then(`^it holds "([^"]*)" alone of (\d+) related items$`, theTermPageHoldsAloneOf)
 	sc.Then(`^the answer carries no entity tag$`, theAnswerCarriesNoEntityTag)
-	sc.Then(`^"([^"]*)" answers not found$`, theAddressAnswersNotFound)
 }