diff --git a/docs/src/content/docs/reference/content-api.md b/docs/src/content/docs/reference/content-api.md index bff9de31..0ff34e65 100644 --- a/docs/src/content/docs/reference/content-api.md +++ b/docs/src/content/docs/reference/content-api.md @@ -439,7 +439,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 diff --git a/docs/src/content/docs/self-hosting/configuration.md b/docs/src/content/docs/self-hosting/configuration.md index b1f2069d..ec27b299 100644 --- a/docs/src/content/docs/self-hosting/configuration.md +++ b/docs/src/content/docs/self-hosting/configuration.md @@ -956,5 +956,12 @@ 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 +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`. Page 0 answers +`404`, and content stored at the address asked for stays there, so a +page stored at `/pages/about/page/1` answers at that address. While a theme serves the site, the theme answers these addresses itself. diff --git a/docs/src/content/docs/themes/writing-a-theme.md b/docs/src/content/docs/themes/writing-a-theme.md index 34dd48a1..4a31d5d0 100644 --- a/docs/src/content/docs/themes/writing-a-theme.md +++ b/docs/src/content/docs/themes/writing-a-theme.md @@ -27,9 +27,9 @@ export default defineConfig({ Leave Astro's `trailingSlash` option at its default. While your theme starts or after it crashes, the built-in renderer answers -instead and sends a listing address that ends in a slash, like -`/pages/`, to `/pages` for good. A theme that sends `/pages` back to -`/pages/` would then trap visitors in a loop. +instead and sends an address that ends in a slash, like `/pages/` or +`/hello-world/`, to the address without it for good. A theme that +sends `/pages` back to `/pages/` would then trap visitors in a loop. `src/theme.ts` declares what your theme provides: diff --git a/internal/content/resolver.go b/internal/content/resolver.go index 9095acef..3518ebfd 100644 --- a/internal/content/resolver.go +++ b/internal/content/resolver.go @@ -32,6 +32,21 @@ type Address struct { Page int } +// PagedItemError is the [ErrNotFound] of a page number asked of a post or page, naming the item. +type PagedItemError struct { + Item Content +} + +// Error returns the message of the miss it stands for. +func (e *PagedItemError) Error() string { + return ErrNotFound.Error() +} + +// Unwrap returns [ErrNotFound]. +func (e *PagedItemError) Unwrap() error { + return ErrNotFound +} + // AddressReader reads the published content an address holds. type AddressReader interface { PublishedByPath(ctx context.Context, path string) (Content, error) @@ -88,9 +103,6 @@ func (r *Resolver) itemAt(ctx context.Context, item Content, page int) (Address, if listed.PageKind == PageKindArchive { return Address{Kind: KindTerm, Type: listed, Item: item, Page: page}, nil } - if page > 1 { - return Address{}, ErrNotFound - } return Address{Kind: KindItem, Type: listed, Item: item}, nil } @@ -123,7 +135,7 @@ func (r *Resolver) archiveAt(ctx context.Context, address string) (Address, erro return r.pagedItemAt(ctx, word, page) } -// pagedItemAt returns the term page an address asks for behind the page word. +// pagedItemAt returns the term page an address asks for behind the page word, or a [PagedItemError] for a post or page. func (r *Resolver) pagedItemAt(ctx context.Context, address string, page int) (Address, error) { item, err := r.content.PublishedByPath(ctx, address) if errors.Is(err, ErrNotFound) { @@ -136,10 +148,10 @@ func (r *Resolver) pagedItemAt(ctx context.Context, address string, page int) (A if err != nil { return Address{}, err } - if held.Kind != KindTerm { - return Address{}, ErrNotFound + if held.Kind == KindTerm { + return held, nil } - return held, nil + return Address{}, &PagedItemError{Item: item} } // listingAt returns the route word an address lists and the page it asks for. diff --git a/internal/content/resolver_gaps_test.go b/internal/content/resolver_gaps_test.go index 08d44936..ef29ec99 100644 --- a/internal/content/resolver_gaps_test.go +++ b/internal/content/resolver_gaps_test.go @@ -3,12 +3,28 @@ package content_test import ( + "context" "errors" "testing" "github.com/gopherium/gophenberg/internal/content" ) +// failingKeys finds no route word and fails every read by key. +type failingKeys struct { + err error +} + +// ByRouteWord answers that no type carries the word. +func (f failingKeys) ByRouteWord(context.Context, string) (content.Type, error) { + return content.Type{}, content.ErrTypeNotFound +} + +// ByKey returns err. +func (f failingKeys) ByKey(context.Context, string) (content.Type, error) { + return content.Type{}, f.err +} + func TestResolveReportsAnAddressItCannotRead(t *testing.T) { t.Parallel() @@ -52,6 +68,20 @@ func TestResolveReportsARegistryItCannotRead(t *testing.T) { } } +func TestResolveReportsARegistryItCannotReadBehindAPageNumber(t *testing.T) { + t.Parallel() + + store := newFakeAddresses() + store.hold(content.TypePost, "a-post") + resolver := content.NewResolver(store, failingKeys{err: errStoreDown}) + + _, err := resolver.Resolve(t.Context(), "/a-post/"+content.PageWord+"/2") + + if !errors.Is(err, errStoreDown) { + t.Errorf("Resolve() error = %v, want %v", err, errStoreDown) + } +} + func TestResolveReportsAPagedAddressItCannotRead(t *testing.T) { t.Parallel() diff --git a/internal/content/resolver_test.go b/internal/content/resolver_test.go index 298084af..977227c9 100644 --- a/internal/content/resolver_test.go +++ b/internal/content/resolver_test.go @@ -356,3 +356,89 @@ func TestResolveAnswersAnAddressNoStoreCanHoldAsNotFound(t *testing.T) { t.Errorf("lookups = %d, want none for an address no store can hold", store.lookups) } } + +func TestResolveNamesThePostOrPageBehindAPageNumber(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, + } { + _, err := resolver.Resolve(t.Context(), raw) + + var paged *content.PagedItemError + if !errors.As(err, &paged) || paged.Item.ID != want.ID { + t.Errorf("Resolve(%q) error = %v, want it to name the item at %q", raw, err, want.Path) + } + } +} + +func TestResolveAnswersNotFoundWhenItNamesAnItemBehindAPageNumber(t *testing.T) { + t.Parallel() + + store := newFakeAddresses() + store.hold("post", "hello-world") + + _, err := newResolver(t, store).Resolve(t.Context(), "/hello-world/page/2") + + if !errors.Is(err, content.ErrNotFound) { + t.Errorf("Resolve() error = %v, want %v", err, content.ErrNotFound) + } + if got, want := err.Error(), content.ErrNotFound.Error(); got != want { + t.Errorf("Resolve() message = %q, want %q", got, want) + } +} + +func TestResolveNamesNothingWithoutAnItemBehindAPageNumber(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": "/nowhere", + "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.Resolve(t.Context(), raw) + + var paged *content.PagedItemError + if errors.As(err, &paged) { + t.Errorf("%s error = %v, want no item named", name, err) + } + } +} + +func TestResolveNamesNoItemOfAnInactiveTypeBehindAPageNumber(t *testing.T) { + t.Parallel() + + store := newFakeAddresses() + store.hold("retired", "retired/old") + retired := pageType() + retired.Key, retired.RouteWord, retired.Active = "retired", "retired", false + types := newFakeTypeStore() + types.types = append(types.types, retired) + resolver := content.NewResolver(store, content.NewRegistry(types)) + + _, err := resolver.Resolve(t.Context(), "/retired/old/page/1") + + var paged *content.PagedItemError + if errors.As(err, &paged) { + t.Errorf("Resolve() error = %v, want no item named", err) + } + if !errors.Is(err, content.ErrNotFound) { + t.Errorf("Resolve() error = %v, want %v", err, content.ErrNotFound) + } +} diff --git a/internal/publicsite/canonical_address_test.go b/internal/publicsite/canonical_address_test.go index aa0a3a40..5e3d6f2f 100644 --- a/internal/publicsite/canonical_address_test.go +++ b/internal/publicsite/canonical_address_test.go @@ -3,6 +3,7 @@ package publicsite_test import ( + "context" "errors" "net/http" "net/http/httptest" @@ -13,8 +14,24 @@ import ( "github.com/google/uuid" "github.com/gopherium/gophenberg/internal/content" + "github.com/gopherium/gophenberg/internal/publicsite" ) +// failingAt serves like its reader but fails every read of path. +type failingAt struct { + *fakeReader + path string + err error +} + +// PublishedByPath returns err for path and reads any other path as its reader does. +func (r *failingAt) PublishedByPath(ctx context.Context, path string) (content.Content, error) { + if path == r.path { + 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()) @@ -165,3 +182,156 @@ 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 := &failingAt{ + 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) + } + } +} + +func TestSiteSendsAPageNumberToTheStoredPathRatherThanTheSlug(t *testing.T) { + t.Parallel() + + about := storedAt("page", "About", "pages/about") + about.Slug = "about" + handler, _ := siteWith(about) + + recorder := get(t, handler, "/pages/about/page/1") + + if got := recorder.Header().Get("Location"); got != "/pages/about" { + t.Errorf("GET /pages/about/page/1 Location = %q, want %q", got, "/pages/about") + } +} + +func TestSiteSendsNothingButTheRedirectFromAPageNumber(t *testing.T) { + t.Parallel() + + handler, _ := siteWith(publishedPost("Hello World", "hello-world", blockMarkup, time.Now().UTC())) + + recorder := get(t, handler, "/hello-world/page/2") + + if recorder.Code != http.StatusMovedPermanently { + t.Fatalf("GET /hello-world/page/2 status = %d, want %d", recorder.Code, http.StatusMovedPermanently) + } + if recorder.Body.Len() != 0 { + t.Errorf("GET /hello-world/page/2 body = %q, want none", recorder.Body.String()) + } +} + +func TestSiteKeepsContentStoredBehindAPageNumberOfAPage(t *testing.T) { + t.Parallel() + + handler, _ := siteWith(storedAt("page", "About", "pages/about"), storedAt("page", "Exact", "pages/about/page/1")) + + recorder := get(t, handler, "/pages/about/page/1") + + if recorder.Code != http.StatusOK { + t.Errorf("GET /pages/about/page/1 status = %d, want %d", recorder.Code, http.StatusOK) + } + if body := recorder.Body.String(); !strings.Contains(body, "