Jakub Zych
cb614cb1fb
AddressChanged wrote its 301 and re-pointed rules without Problems or the save-time simulation, and removed only an exact rule whose source equals the new address literally. It now removes every exact rule that matches the new address with its options, simulates the 301 and the re-pointing with the stored rules and writes nothing when that loops, stores an absolute old address on the site as its path, and skips an address that is not a path on the site.
779 lines
34 KiB
Go
779 lines
34 KiB
Go
package seo_test
|
|
|
|
import (
|
|
"bytes"
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"sort"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
"time"
|
|
|
|
"git.golem15.com/golem15/sm-seo-plugin/classes"
|
|
"git.golem15.com/golem15/sm-seo-plugin/models"
|
|
"git.golem15.com/golem15/summercms/modules/cabana"
|
|
"git.golem15.com/golem15/summercms/modules/lagoon"
|
|
"git.golem15.com/golem15/summercms/modules/pact"
|
|
"gorm.io/gorm"
|
|
)
|
|
|
|
// redirectRow is a stored redirect as the tests read it.
|
|
type redirectRow struct {
|
|
ID uint
|
|
Source string
|
|
Match string
|
|
Target string
|
|
Status int
|
|
Query string
|
|
IgnoreCase bool
|
|
TrailingSlash bool
|
|
Origin string
|
|
}
|
|
|
|
// redirects returns every stored redirect in id order.
|
|
func (e *testEnv) redirects(t *testing.T) []redirectRow {
|
|
t.Helper()
|
|
var rows []redirectRow
|
|
if err := e.db.Raw(`SELECT id, source, match, target, status, query, ignore_case, trailing_slash, origin FROM golem15_seo_redirects ORDER BY id`).Scan(&rows).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return rows
|
|
}
|
|
|
|
// redirectTo returns the target of the exact redirect of source, "" when
|
|
// there is none.
|
|
func redirectTo(rows []redirectRow, source string) string {
|
|
for _, r := range rows {
|
|
if r.Source == source && r.Match == "exact" {
|
|
return r.Target
|
|
}
|
|
}
|
|
return ""
|
|
}
|
|
|
|
// seedRedirect stores a redirect directly.
|
|
func (e *testEnv) seedRedirect(t *testing.T, source, match, target string, status int) {
|
|
t.Helper()
|
|
if err := e.db.Exec(`INSERT INTO golem15_seo_redirects (source, match, target, status, created_at, updated_at) VALUES (?, ?, ?, ?, NOW(), NOW())`,
|
|
source, match, target, status).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
// saveNotices returns the save notices of an admin save response.
|
|
func saveNotices(t *testing.T, body []byte) []struct{ Field, Message string } {
|
|
t.Helper()
|
|
var out struct {
|
|
Meta struct {
|
|
Notices []struct {
|
|
Field string `json:"field"`
|
|
Message string `json:"message"`
|
|
} `json:"notices"`
|
|
} `json:"meta"`
|
|
}
|
|
if err := json.Unmarshal(body, &out); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
var notices []struct{ Field, Message string }
|
|
for _, n := range out.Meta.Notices {
|
|
notices = append(notices, struct{ Field, Message string }{n.Field, n.Message})
|
|
}
|
|
return notices
|
|
}
|
|
|
|
// TestSlugChange301 is the plan's tracer (SC-3, D-18): an editor changes a
|
|
// published page's slug in the admin; inside the save a 301 from the old
|
|
// address to the new one is created, an earlier redirect to the old
|
|
// address is pointed straight at the new one, the editor gets the
|
|
// redirects.auto_created notice, and the resolve answer redirects the old
|
|
// address in one hop.
|
|
func TestSlugChange301(t *testing.T) {
|
|
env := bootEnv(t)
|
|
token := env.admin(t, true)
|
|
id := env.createPage(t, token, map[string]any{"title": "Alpha", "slug": "a", "published": true})
|
|
env.seedRedirect(t, "/pages/x", "exact", "/pages/a", 301)
|
|
|
|
rec := env.expect(t, http.StatusOK, http.MethodPut, adminAPI("/acme/pages/pages/"+id), token, map[string]any{
|
|
"title": "Alpha", "slug": "b", "published": true,
|
|
})
|
|
var found bool
|
|
for _, n := range saveNotices(t, rec.Body.Bytes()) {
|
|
if n.Field == "slug" && strings.Contains(n.Message, "“/pages/a”") && strings.Contains(n.Message, "“/pages/b”") && strings.Contains(n.Message, "1 earlier") {
|
|
found = true
|
|
}
|
|
}
|
|
if !found {
|
|
t.Fatalf("update notices lack redirects.auto_created on slug: %s", rec.Body.String())
|
|
}
|
|
|
|
got := env.resolve(t, "/pages/a")
|
|
if got["action"] != "redirect" || got["status"] != float64(301) || got["location"] != siteURL+"/pages/b" {
|
|
t.Fatalf("resolve /pages/a = %v, want redirect 301 to %s/pages/b", got, siteURL)
|
|
}
|
|
if got := env.resolve(t, "/pages/b"); got["action"] != "ok" {
|
|
t.Fatalf("resolve /pages/b = %v, want ok", got)
|
|
}
|
|
rows := env.redirects(t)
|
|
if target := redirectTo(rows, "/pages/x"); target != "/pages/b" {
|
|
t.Fatalf("earlier redirect /pages/x -> %q, want flattened to /pages/b (%+v)", target, rows)
|
|
}
|
|
if target := redirectTo(rows, "/pl/pages/a"); target != "/pl/pages/b" {
|
|
t.Fatalf("pl redirect /pl/pages/a -> %q, want /pl/pages/b (%+v)", target, rows)
|
|
}
|
|
for _, r := range rows {
|
|
if r.Source == "/pages/a" && r.Origin != "slug" {
|
|
t.Fatalf("address-change redirect origin = %q, want slug", r.Origin)
|
|
}
|
|
}
|
|
if got := env.resolve(t, "/pages/x"); got["location"] != siteURL+"/pages/b" {
|
|
t.Fatalf("resolve /pages/x = %v, want one hop to /pages/b", got)
|
|
}
|
|
}
|
|
|
|
// saveRedirect stores r through the service in its own transaction.
|
|
func (e *testEnv) saveRedirect(t *testing.T, r models.Redirect) (classes.RedirectCheck, error) {
|
|
t.Helper()
|
|
svc := e.seoPlugin(t).Service()
|
|
var check classes.RedirectCheck
|
|
err := e.db.Transaction(func(tx *gorm.DB) error {
|
|
c, err := svc.SaveRedirect(t.Context(), tx, &r)
|
|
check = c
|
|
return err
|
|
})
|
|
return check, err
|
|
}
|
|
|
|
// mustRedirect stores r and fails on a refusal.
|
|
func (e *testEnv) mustRedirect(t *testing.T, source, match, target string, status int, opts ...func(*models.Redirect)) classes.RedirectCheck {
|
|
t.Helper()
|
|
r := models.NewRedirect()
|
|
r.Source, r.Match, r.Target, r.Status = source, match, target, models.RedirectStatus(status)
|
|
for _, o := range opts {
|
|
o(&r)
|
|
}
|
|
check, err := e.saveRedirect(t, r)
|
|
if err != nil {
|
|
t.Fatalf("save %s -> %s: %v", source, target, err)
|
|
}
|
|
return check
|
|
}
|
|
|
|
// refusedOn asserts that saving r is refused with key on field.
|
|
func (e *testEnv) refusedOn(t *testing.T, r models.Redirect, field, key string) {
|
|
t.Helper()
|
|
_, err := e.saveRedirect(t, r)
|
|
var problems *classes.ProblemsError
|
|
if !errors.As(err, &problems) {
|
|
t.Fatalf("save %+v: err = %v, want a refusal on %s", r, err, field)
|
|
}
|
|
for _, p := range problems.Problems[field] {
|
|
if p.Key == key {
|
|
return
|
|
}
|
|
}
|
|
t.Fatalf("save %+v: problems = %+v, want %s on %s", r, problems.Problems, key, field)
|
|
}
|
|
|
|
// expectRedirect asserts that target resolves to a redirect to location.
|
|
func (e *testEnv) expectRedirect(t *testing.T, target string, status int, location string) {
|
|
t.Helper()
|
|
got := e.resolve(t, target)
|
|
if got["action"] != "redirect" || got["status"] != float64(status) || got["location"] != location {
|
|
t.Fatalf("resolve %s = %v, want redirect %d to %s", target, got, status, location)
|
|
}
|
|
}
|
|
|
|
// expectAction asserts the resolve action of target.
|
|
func (e *testEnv) expectAction(t *testing.T, target, action string) map[string]any {
|
|
t.Helper()
|
|
got := e.resolve(t, target)
|
|
if got["action"] != action {
|
|
t.Fatalf("resolve %s = %v, want %s", target, got, action)
|
|
}
|
|
return got
|
|
}
|
|
|
|
// TestRedirectEngine checks the redirect engine through the service and
|
|
// the resolve answer (D-16, D-17, D-19, T-32-16, T-32-17): matching
|
|
// options, order, statuses, refusals, loops, chains and the back-and-forth
|
|
// address change.
|
|
func TestRedirectEngine(t *testing.T) {
|
|
env := bootEnv(t)
|
|
svc := env.seoPlugin(t).Service()
|
|
ignoreCase := func(r *models.Redirect) { r.IgnoreCase = true }
|
|
noSlash := func(r *models.Redirect) { r.TrailingSlash = false }
|
|
pass := func(r *models.Redirect) { r.Query = models.QueryPass }
|
|
|
|
env.mustRedirect(t, "/old", "exact", "/new", 301)
|
|
env.expectRedirect(t, "/old", 301, siteURL+"/new")
|
|
env.expectRedirect(t, "/old/", 301, siteURL+"/new")
|
|
env.expectRedirect(t, "/old?a=1", 301, siteURL+"/new")
|
|
env.expectAction(t, "/OLD", "not_found")
|
|
|
|
env.mustRedirect(t, "/Case", "exact", "/case-new", 302, ignoreCase)
|
|
env.expectRedirect(t, "/CASE", 302, siteURL+"/case-new")
|
|
env.mustRedirect(t, "/strict", "exact", "/strict-new", 301, noSlash)
|
|
env.expectAction(t, "/strict/", "not_found")
|
|
|
|
env.mustRedirect(t, "/p-old", "exact", "/p-new", 301, pass)
|
|
env.expectRedirect(t, "/p-old?a=1&b=2", 301, siteURL+"/p-new?a=1&b=2")
|
|
env.mustRedirect(t, "/q?id=5", "exact", "/q5", 301, pass)
|
|
env.expectRedirect(t, "/q?id=5", 301, siteURL+"/q5")
|
|
env.expectAction(t, "/q?id=6", "not_found")
|
|
|
|
env.mustRedirect(t, "/w/*", "pattern", "/nw/*", 301)
|
|
env.expectRedirect(t, "/w/x/y", 301, siteURL+"/nw/x/y")
|
|
env.mustRedirect(t, `^/p/(\d+)$`, "regex", "/post/$1", 301)
|
|
env.expectRedirect(t, "/p/12", 301, siteURL+"/post/12")
|
|
env.expectAction(t, "/p/abc", "not_found")
|
|
|
|
env.mustRedirect(t, "/w/exact", "exact", "/exact-wins", 301)
|
|
env.mustRedirect(t, `^/w/.*$`, "regex", "/regex-loses", 301)
|
|
env.expectRedirect(t, "/w/exact", 301, siteURL+"/exact-wins")
|
|
env.expectRedirect(t, "/w/z", 301, siteURL+"/nw/z")
|
|
|
|
env.mustRedirect(t, "/gone", "exact", "", 410)
|
|
got := env.expectAction(t, "/gone", "gone")
|
|
if got["status"] != float64(410) || got["location"] != nil {
|
|
t.Fatalf("gone answer = %v, want 410 without a location", got)
|
|
}
|
|
|
|
newRule := func(source, match, target string, status int) models.Redirect {
|
|
r := models.NewRedirect()
|
|
r.Source, r.Match, r.Target, r.Status = source, match, target, models.RedirectStatus(status)
|
|
return r
|
|
}
|
|
env.refusedOn(t, newRule("/g2", "exact", "/x", 410), "target", "golem15.seo::lang.redirects.target_gone")
|
|
env.refusedOn(t, newRule("/g3", "exact", "", 301), "target", "golem15.seo::lang.redirects.target_required")
|
|
env.refusedOn(t, newRule("/g4", "exact", "elsewhere", 301), "target", "golem15.seo::lang.redirects.target_invalid")
|
|
env.refusedOn(t, newRule(`^/(`, "regex", "/x", 301), "source", "golem15.seo::lang.redirects.regex_invalid")
|
|
env.refusedOn(t, newRule("^/"+strings.Repeat("a", 520), "regex", "/x", 301), "source", "golem15.seo::lang.redirects.regex_invalid")
|
|
env.refusedOn(t, newRule("/old", "exact", "/other", 301), "source", "golem15.seo::lang.redirects.source_taken")
|
|
env.refusedOn(t, newRule("old", "exact", "/other", 301), "source", "golem15.seo::lang.redirects.source_invalid")
|
|
env.refusedOn(t, newRule("/a*", "exact", "/other", 301), "source", "golem15.seo::lang.redirects.source_invalid")
|
|
|
|
// Loops are refused; chains are saved with a notice (D-19).
|
|
env.mustRedirect(t, "/la", "exact", "/lb", 301)
|
|
_, err := env.saveRedirect(t, newRule("/lb", "exact", "/la", 301))
|
|
var loop *classes.LoopError
|
|
if !errors.As(err, &loop) || loop.Check.PathText() != "/lb → /la → /lb" {
|
|
t.Fatalf("B -> A after A -> B: err = %v, want a loop /lb → /la → /lb", err)
|
|
}
|
|
if !strings.Contains(svc.LoopMessage(t.Context(), loop.Check), "/lb → /la → /lb") {
|
|
t.Fatalf("loop message = %q", svc.LoopMessage(t.Context(), loop.Check))
|
|
}
|
|
_, err = env.saveRedirect(t, newRule("/grow/*", "pattern", "/grow/new/*", 301))
|
|
if !errors.As(err, &loop) {
|
|
t.Fatalf("/grow/* -> /grow/new/*: err = %v, want a loop (pattern probe)", err)
|
|
}
|
|
_, err = env.saveRedirect(t, newRule(`^/rx/(\d+)$`, "regex", "/rx/$1", 301))
|
|
if !errors.As(err, &loop) {
|
|
t.Fatalf("regex onto itself: err = %v, want a loop (regex probe)", err)
|
|
}
|
|
env.mustRedirect(t, "/cb", "exact", "/cc", 301)
|
|
check := env.mustRedirect(t, "/ca", "exact", "/cb", 301)
|
|
if !check.Chain || check.Final != "/cc" || check.Hops != 2 || check.PathText() != "/ca → /cb → /cc" {
|
|
t.Fatalf("A -> B with B -> C: check = %+v, want a chain to /cc", check)
|
|
}
|
|
check = env.mustRedirect(t, "/cz", "exact", "/ca", 301)
|
|
if !check.Chain || check.Final != "/cc" || check.Hops != 3 {
|
|
t.Fatalf("Z -> A -> B -> C: check = %+v, want a 3-hop chain to /cc", check)
|
|
}
|
|
env.expectRedirect(t, "/cz", 301, siteURL+"/cc")
|
|
check = env.mustRedirect(t, "/lone", "exact", "/somewhere", 301)
|
|
if check.Chain || check.Loop {
|
|
t.Fatalf("a single redirect: check = %+v, want neither chain nor loop", check)
|
|
}
|
|
|
|
// Slug back and forth (RESEARCH Pitfall 11): A→B, B→A, then A→C ends
|
|
// with exactly A→C and B→C.
|
|
for _, step := range [][2]string{{"/fa", "/fb"}, {"/fb", "/fa"}, {"/fa", "/fc"}} {
|
|
if err := env.db.Transaction(func(tx *gorm.DB) error {
|
|
_, err := svc.AddressChanged(t.Context(), tx, classes.AddressChange{Old: step[0], New: step[1]})
|
|
return err
|
|
}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
var back []string
|
|
for _, r := range env.redirects(t) {
|
|
if strings.HasPrefix(r.Source, "/f") {
|
|
back = append(back, r.Source+"->"+r.Target)
|
|
}
|
|
}
|
|
sort.Strings(back)
|
|
if strings.Join(back, ",") != "/fa->/fc,/fb->/fc" {
|
|
t.Fatalf("back and forth = %v, want exactly /fa->/fc and /fb->/fc", back)
|
|
}
|
|
|
|
// A capture never turns an internal target into another host
|
|
// (T-32-16); an absolute literal keeps its own host.
|
|
env.mustRedirect(t, `^/r/(.*)$`, "regex", "/$1", 301)
|
|
got = env.resolve(t, "/r//evil.example/x")
|
|
if loc, _ := got["location"].(string); got["action"] != "redirect" || !strings.HasPrefix(loc, siteURL+"/") || strings.HasPrefix(loc, siteURL+"//") {
|
|
t.Fatalf("protocol-relative capture = %v, want a path on the site", got)
|
|
}
|
|
env.mustRedirect(t, `^/ext/(.*)$`, "regex", "https://partner.example/$1", 301)
|
|
env.expectRedirect(t, "/ext/x", 301, "https://partner.example/x")
|
|
env.seedRedirect(t, `^/ext2(.*)$`, "regex", "https://partner.example$1", 301)
|
|
if got := env.resolve(t, "/ext2@evil.example"); got["action"] == "redirect" {
|
|
t.Fatalf("capture into the host = %v, want no redirect", got)
|
|
}
|
|
env.mustRedirect(t, "/qp/*", "pattern", "/nq/*", 301, pass)
|
|
got = env.resolve(t, "/qp/a?next=//evil.example")
|
|
if loc, _ := got["location"].(string); !strings.HasPrefix(loc, siteURL+"/nq/a?") {
|
|
t.Fatalf("passed query = %v, want it appended to the query only", got)
|
|
}
|
|
}
|
|
|
|
// TestExactRedirectKeys checks that the resolver's on-demand fetch of
|
|
// exact rules finds every source the in-memory matcher accepts (CR-03): a
|
|
// source stored with a trailing slash fires for both forms of the path, and
|
|
// one with upper-case letters in its query fires for that query.
|
|
func TestExactRedirectKeys(t *testing.T) {
|
|
env := bootEnv(t)
|
|
env.mustRedirect(t, "/old-page/", "exact", "/new-page", 301)
|
|
env.mustRedirect(t, "/list?Page=2", "exact", "/list-2", 301)
|
|
env.mustRedirect(t, "/Mixed/Case/", "exact", "/mixed-new", 301, func(r *models.Redirect) { r.IgnoreCase = true })
|
|
|
|
env.expectRedirect(t, "/old-page", 301, siteURL+"/new-page")
|
|
env.expectRedirect(t, "/old-page/", 301, siteURL+"/new-page")
|
|
env.expectRedirect(t, "/list?Page=2", 301, siteURL+"/list-2")
|
|
env.expectAction(t, "/list?page=2", "not_found")
|
|
env.expectRedirect(t, "/mixed/case", 301, siteURL+"/mixed-new")
|
|
|
|
// The generated column holds the same key as classes.exactKey.
|
|
for source, want := range map[string]string{
|
|
"/old-page/": "/old-page",
|
|
"/list?Page=2": "/list?Page=2",
|
|
"/Mixed/Case/": "/mixed/case",
|
|
} {
|
|
var key string
|
|
if err := env.db.Raw(`SELECT source_key FROM golem15_seo_redirects WHERE source = ?`, source).Scan(&key).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if key != want {
|
|
t.Fatalf("source_key of %q = %q, want %q", source, key, want)
|
|
}
|
|
}
|
|
env.seedRedirect(t, "/", "exact", "/home", 301)
|
|
env.seedRedirect(t, "/A/?Q=1", "exact", "/b", 301)
|
|
for source, want := range map[string]string{"/": "/", "/A/?Q=1": "/a?Q=1"} {
|
|
var key string
|
|
if err := env.db.Raw(`SELECT source_key FROM golem15_seo_redirects WHERE source = ?`, source).Scan(&key).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if key != want {
|
|
t.Fatalf("source_key of %q = %q, want %q", source, key, want)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestAddressChangeGuards checks that an address change never stores a
|
|
// redirect loop or an unusable source (WR-09): a pattern rule that sends
|
|
// the new address back to the old one stops the 301, an exact rule that
|
|
// matches the new address with its case or trailing-slash options is
|
|
// removed, an absolute old address on the site is stored as its path, and
|
|
// a new address that is not a path on the site creates nothing.
|
|
func TestAddressChangeGuards(t *testing.T) {
|
|
env := bootEnv(t)
|
|
svc := env.seoPlugin(t).Service()
|
|
change := func(old, new string) classes.AddressResult {
|
|
t.Helper()
|
|
var res classes.AddressResult
|
|
if err := env.db.Transaction(func(tx *gorm.DB) error {
|
|
var err error
|
|
res, err = svc.AddressChanged(t.Context(), tx, classes.AddressChange{Old: old, New: new})
|
|
return err
|
|
}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return res
|
|
}
|
|
|
|
env.mustRedirect(t, "/a/*", "pattern", "/b/*", 301)
|
|
if res := change("/b/x", "/a/x"); res.Redirect != nil {
|
|
t.Fatalf("/b/x -> /a/x under /a/* -> /b/*: redirect = %+v, want none (a loop)", res.Redirect)
|
|
}
|
|
if got := redirectTo(env.redirects(t), "/b/x"); got != "" {
|
|
t.Fatalf("/b/x has a redirect to %q, want none", got)
|
|
}
|
|
|
|
env.mustRedirect(t, "/Moved/", "exact", "/elsewhere", 301, func(r *models.Redirect) { r.IgnoreCase = true })
|
|
if res := change("/before", "/moved"); res.Redirect == nil {
|
|
t.Fatal("/before -> /moved: no redirect")
|
|
}
|
|
rows := env.redirects(t)
|
|
if got := redirectTo(rows, "/Moved/"); got != "" {
|
|
t.Fatalf("/Moved/ still redirects to %q; it shadows the record's new address", got)
|
|
}
|
|
env.expectRedirect(t, "/before", 301, siteURL+"/moved")
|
|
|
|
if res := change(siteURL+"/abs-old", "/abs-new"); res.Redirect == nil || res.Redirect.Source != "/abs-old" {
|
|
t.Fatalf("absolute old address: result = %+v, want a redirect from /abs-old", res)
|
|
}
|
|
env.expectRedirect(t, "/abs-old", 301, siteURL+"/abs-new")
|
|
|
|
before := env.redirectCount(t)
|
|
if res := change("/plain", "//evil.example/x"); res.Redirect != nil {
|
|
t.Fatalf("protocol-relative new address: redirect = %+v, want none", res.Redirect)
|
|
}
|
|
if got := env.redirectCount(t); got != before {
|
|
t.Fatalf("redirects = %d after a refused change, want %d", got, before)
|
|
}
|
|
}
|
|
|
|
// redirectCount is the number of stored redirects.
|
|
func (e *testEnv) redirectCount(t *testing.T) int64 {
|
|
t.Helper()
|
|
var n int64
|
|
if err := e.db.Raw(`SELECT COUNT(*) FROM golem15_seo_redirects`).Scan(&n).Error; err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return n
|
|
}
|
|
|
|
// createdID reads the id of a create response.
|
|
func createdID(t *testing.T, body []byte) string {
|
|
t.Helper()
|
|
var out struct {
|
|
Data struct {
|
|
ID json.Number `json:"id"`
|
|
} `json:"data"`
|
|
}
|
|
if err := json.Unmarshal(body, &out); err != nil || out.Data.ID == "" {
|
|
t.Fatalf("create body %s: %v", body, err)
|
|
}
|
|
return out.Data.ID.String()
|
|
}
|
|
|
|
// TestLoopRefused checks the redirects screen (SC-3, D-19, UI-SPEC C8): a
|
|
// loop is refused from the admin with redirects.loop on target and nothing
|
|
// saved, on create and on update; a chain is saved with the chain notice
|
|
// on target and marked in the list; field refusals answer 422.
|
|
func TestLoopRefused(t *testing.T) {
|
|
env := bootEnv(t)
|
|
token := env.admin(t, true)
|
|
api := adminAPI("/golem15/seo/redirects")
|
|
|
|
env.expect(t, http.StatusCreated, http.MethodPost, api, token, map[string]any{"source": "/la", "match": "exact", "target": "/lb", "status": "301"})
|
|
before := env.redirectCount(t)
|
|
rec := env.call(t, http.MethodPost, api, token, map[string]any{"source": "/lb", "match": "exact", "target": "/la", "status": "301"})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"target"`) ||
|
|
!strings.Contains(rec.Body.String(), "This redirect would create a loop: /lb → /la → /lb. Nothing was saved.") {
|
|
t.Fatalf("loop create = %d %s, want 422 redirects.loop on target", rec.Code, rec.Body.String())
|
|
}
|
|
if after := env.redirectCount(t); after != before {
|
|
t.Fatalf("redirects after a refused loop = %d, want %d", after, before)
|
|
}
|
|
|
|
other := createdID(t, env.expect(t, http.StatusCreated, http.MethodPost, api, token, map[string]any{"source": "/lc", "target": "/elsewhere", "status": "302"}).Body.Bytes())
|
|
rec = env.call(t, http.MethodPut, api+"/"+other, token, map[string]any{"source": "/lb", "match": "exact", "target": "/la", "status": "301"})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), "would create a loop") {
|
|
t.Fatalf("loop update = %d %s, want 422", rec.Code, rec.Body.String())
|
|
}
|
|
var stored redirectRow
|
|
if err := env.db.Raw(`SELECT id, source, target, status FROM golem15_seo_redirects WHERE id = ?`, other).Scan(&stored).Error; err != nil || stored.Source != "/lc" || stored.Status != 302 {
|
|
t.Fatalf("redirect after a refused update = %+v err=%v, want it unchanged", stored, err)
|
|
}
|
|
|
|
env.expect(t, http.StatusCreated, http.MethodPost, api, token, map[string]any{"source": "/cb", "target": "/cc"})
|
|
rec = env.expect(t, http.StatusCreated, http.MethodPost, api, token, map[string]any{"source": "/ca", "target": "/cb"})
|
|
chainID := createdID(t, rec.Body.Bytes())
|
|
var notice bool
|
|
for _, n := range saveNotices(t, rec.Body.Bytes()) {
|
|
if n.Field == "target" && strings.Contains(n.Message, "/ca → /cb → /cc") && strings.Contains(n.Message, "“/cc”") {
|
|
notice = true
|
|
}
|
|
}
|
|
if !notice {
|
|
t.Fatalf("chain save notices = %s, want redirects.chain_notice on target naming /cc", rec.Body.String())
|
|
}
|
|
var list struct {
|
|
Meta struct {
|
|
RowStates map[string][]string `json:"row_states"`
|
|
} `json:"meta"`
|
|
}
|
|
if err := json.Unmarshal(env.expect(t, http.StatusOK, http.MethodGet, api, token, nil).Body.Bytes(), &list); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if states := list.Meta.RowStates[chainID]; len(states) != 1 || states[0] != "negative" {
|
|
t.Fatalf("row states = %v, want the chain row %s negative", list.Meta.RowStates, chainID)
|
|
}
|
|
|
|
rec = env.call(t, http.MethodPost, api, token, map[string]any{"source": "/g", "status": "301"})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), "Enter the address to redirect to") {
|
|
t.Fatalf("missing target = %d %s, want 422 target_required", rec.Code, rec.Body.String())
|
|
}
|
|
rec = env.call(t, http.MethodPost, api, token, map[string]any{"source": `^/(`, "match": "regex", "target": "/x"})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), "This is not a valid regular expression: missing closing )") {
|
|
t.Fatalf("invalid regex = %d %s, want 422 regex_invalid with the error", rec.Code, rec.Body.String())
|
|
}
|
|
env.expect(t, http.StatusCreated, http.MethodPost, api, token, map[string]any{"source": "/gone-page", "status": "410"})
|
|
if got := env.resolve(t, "/gone-page"); got["action"] != "gone" {
|
|
t.Fatalf("410 from the admin = %v, want gone", got)
|
|
}
|
|
if rows := env.revisions(t, "redirect"); len(rows) < 4 {
|
|
t.Fatalf("redirect history rows = %d, want every admin save recorded", len(rows))
|
|
}
|
|
|
|
// The same loop is refused from MCP (a tool error with the path) and
|
|
// from a CSV import (a refused row that blocks the apply), SC-3.
|
|
t.Run("mcp", func(t *testing.T) {
|
|
agent := env.principal(t, env.admin(t, false, "golem15.seo.manage"))
|
|
before := env.redirectCount(t)
|
|
rec := env.mcp(t, http.MethodPost, "/_seo/api/v1/redirects", agent, map[string]any{"source": "/lb", "target": "/la"})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"error":"redirect_loop"`) || !strings.Contains(rec.Body.String(), `"/lb"`) {
|
|
t.Fatalf("MCP loop = %d %s, want 422 redirect_loop with the path", rec.Code, rec.Body.String())
|
|
}
|
|
rec = env.mcp(t, http.MethodPost, "/_seo/api/v1/batch", agent, map[string]any{"changes": []any{map[string]any{"op": "create_redirect", "redirect": map[string]any{"source": "/lb", "target": "/la"}}}})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"error":"redirect_loop"`) {
|
|
t.Fatalf("MCP batch loop = %d %s", rec.Code, rec.Body.String())
|
|
}
|
|
if after := env.redirectCount(t); after != before {
|
|
t.Fatalf("redirects after the MCP loop = %d, want %d", after, before)
|
|
}
|
|
})
|
|
t.Run("import", func(t *testing.T) {
|
|
agent := env.principal(t, env.admin(t, false, "golem15.seo.manage"))
|
|
before := env.redirectCount(t)
|
|
file := "source,target\n/lb,/la\n"
|
|
preview := env.mcpExpect(t, http.StatusOK, http.MethodPost, "/_seo/api/v1/import", agent, map[string]any{"dataset": "redirects", "csv": file, "delimiter": ",", "dry_run": true})
|
|
if !strings.Contains(fmt.Sprint(preview["rows"]), "Creates a redirect loop: /lb → /la → /lb") {
|
|
t.Fatalf("import preview = %v, want transfer.row_loop", preview)
|
|
}
|
|
rec := env.mcp(t, http.MethodPost, "/_seo/api/v1/import", agent, map[string]any{"dataset": "redirects", "csv": file, "delimiter": ","})
|
|
if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"error":"import_refused"`) {
|
|
t.Fatalf("import apply = %d %s, want 422 import_refused", rec.Code, rec.Body.String())
|
|
}
|
|
if after := env.redirectCount(t); after != before {
|
|
t.Fatalf("redirects after the refused import = %d, want %d", after, before)
|
|
}
|
|
})
|
|
}
|
|
|
|
// report posts a 404 report with token ("" sends none).
|
|
func (e *testEnv) report(t *testing.T, token string, body any) *httptest.ResponseRecorder {
|
|
t.Helper()
|
|
raw, err := json.Marshal(body)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
req := httptest.NewRequest(http.MethodPost, "/_seo/api/v1/notfound", bytes.NewReader(raw))
|
|
req.Header.Set("Content-Type", "application/json")
|
|
req.RemoteAddr = "198.51.100.7:4000"
|
|
if token != "" {
|
|
req.Header.Set("Authorization", "Bearer "+token)
|
|
}
|
|
rec := httptest.NewRecorder()
|
|
e.h.ServeHTTP(rec, req)
|
|
return rec
|
|
}
|
|
|
|
// notFoundRow is a 404 log row as the tests read it.
|
|
type notFoundRow struct {
|
|
ID uint
|
|
URL string
|
|
Hits int64
|
|
FirstSeen time.Time
|
|
LastHit time.Time
|
|
Referer string
|
|
}
|
|
|
|
func (e *testEnv) notFoundRow(t *testing.T, url string) notFoundRow {
|
|
t.Helper()
|
|
var row notFoundRow
|
|
if err := e.db.Raw(`SELECT id, url, hits, first_seen, last_hit, referer FROM golem15_seo_notfound WHERE url = ?`, url).Scan(&row).Error; err != nil || row.ID == 0 {
|
|
t.Fatalf("404 log row %s: %+v err=%v", url, row, err)
|
|
}
|
|
return row
|
|
}
|
|
|
|
// TestNotFoundLog checks the 404 log (D-20, P-22, UI-SPEC C9): the
|
|
// authenticated report endpoint, hits and referers, the prompted redirect
|
|
// and the 410 record actions, the Resolved state, and the bulk 410 that
|
|
// refuses a selection containing a covered address.
|
|
func TestNotFoundLog(t *testing.T) {
|
|
off := bootEnv(t)
|
|
if rec := off.report(t, "anything", map[string]string{"url": "/x"}); rec.Code != http.StatusNotFound {
|
|
t.Fatalf("report without a configured token = %d, want 404", rec.Code)
|
|
}
|
|
|
|
const secret = "notfound-test-token"
|
|
env := bootEnv(t, "SUMMER_GOLEM15__SEO__NOTFOUND_TOKEN="+secret)
|
|
if rec := env.report(t, "", map[string]string{"url": "/missing"}); rec.Code != http.StatusUnauthorized {
|
|
t.Fatalf("report without a token = %d, want 401", rec.Code)
|
|
}
|
|
if rec := env.report(t, "wrong", map[string]string{"url": "/missing"}); rec.Code != http.StatusUnauthorized {
|
|
t.Fatalf("report with a wrong token = %d, want 401", rec.Code)
|
|
}
|
|
if rec := env.report(t, secret, map[string]string{"url": "/" + strings.Repeat("a", 2100)}); rec.Code != http.StatusUnprocessableEntity {
|
|
t.Fatalf("report of a long URL = %d, want 422", rec.Code)
|
|
}
|
|
if rec := env.report(t, secret, map[string]string{"url": "/missing", "referer": "https://ref.example/a"}); rec.Code != http.StatusNoContent || rec.Header().Get("X-Robots-Tag") != "noindex" {
|
|
t.Fatalf("report = %d %v, want 204 with X-Robots-Tag", rec.Code, rec.Header())
|
|
}
|
|
first := env.notFoundRow(t, "/missing")
|
|
time.Sleep(1100 * time.Millisecond)
|
|
env.report(t, secret, map[string]string{"url": "https://www.example.test/missing", "referer": "https://ref.example/b"})
|
|
again := env.notFoundRow(t, "/missing")
|
|
if again.Hits != 2 || !again.FirstSeen.Equal(first.FirstSeen) || !again.LastHit.After(first.LastHit) || again.Referer != "https://ref.example/b" {
|
|
t.Fatalf("second report = %+v (first %+v), want hits 2, first seen kept, last hit and referer updated", again, first)
|
|
}
|
|
for _, u := range []string{"/old-a", "/old-b", "/old-c", "/old-c"} {
|
|
env.report(t, secret, map[string]string{"url": u})
|
|
}
|
|
|
|
token := env.admin(t, true)
|
|
api := adminAPI("/golem15/seo/notfound")
|
|
var list struct {
|
|
Data []struct {
|
|
ID json.Number `json:"id"`
|
|
URL string `json:"url"`
|
|
} `json:"data"`
|
|
Meta struct {
|
|
RowStates map[string][]string `json:"row_states"`
|
|
} `json:"meta"`
|
|
}
|
|
if err := json.Unmarshal(env.expect(t, http.StatusOK, http.MethodGet, api, token, nil).Body.Bytes(), &list); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if len(list.Data) != 4 || (list.Data[0].URL != "/missing" && list.Data[0].URL != "/old-c") {
|
|
t.Fatalf("404 list = %+v, want four entries, most hits first", list.Data)
|
|
}
|
|
|
|
missing := strconv.FormatUint(uint64(again.ID), 10)
|
|
rec := env.expect(t, http.StatusOK, http.MethodPost, api+"/"+missing+"/actions/redirect", token, map[string]any{"payload": "/found"})
|
|
if !strings.Contains(rec.Body.String(), "/missing") || !strings.Contains(rec.Body.String(), "/found") {
|
|
t.Fatalf("redirect action = %s, want the redirected message", rec.Body.String())
|
|
}
|
|
rows := env.redirects(t)
|
|
if target := redirectTo(rows, "/missing"); target != "/found" {
|
|
t.Fatalf("redirect from the 404 log = %+v, want /missing -> /found", rows)
|
|
}
|
|
for _, r := range rows {
|
|
if r.Source == "/missing" && (r.Origin != "notfound" || r.Status != 301) {
|
|
t.Fatalf("404 redirect = %+v, want origin notfound and 301", r)
|
|
}
|
|
}
|
|
env.expectRedirect(t, "/missing", 301, siteURL+"/found")
|
|
show := env.expect(t, http.StatusOK, http.MethodGet, api+"/"+missing, token, nil).Body.String()
|
|
if strings.Contains(show, `"actions"`) {
|
|
t.Fatalf("a resolved entry still offers actions: %s", show)
|
|
}
|
|
if rec := env.call(t, http.MethodPost, api+"/"+missing+"/actions/gone", token, map[string]any{}); rec.Code != http.StatusConflict {
|
|
t.Fatalf("gone on a resolved entry = %d %s, want 409", rec.Code, rec.Body.String())
|
|
}
|
|
hint := env.expect(t, http.StatusOK, http.MethodGet, adminAPI("/golem15/seo/notfound/partials/hint?id="+missing), token, nil).Body.String()
|
|
if !strings.Contains(hint, "Resolved by") || !strings.Contains(hint, "/missing → /found") {
|
|
t.Fatalf("hint = %s, want the resolved hint", hint)
|
|
}
|
|
|
|
oldA := strconv.FormatUint(uint64(env.notFoundRow(t, "/old-a").ID), 10)
|
|
oldB := strconv.FormatUint(uint64(env.notFoundRow(t, "/old-b").ID), 10)
|
|
oldC := strconv.FormatUint(uint64(env.notFoundRow(t, "/old-c").ID), 10)
|
|
env.expect(t, http.StatusOK, http.MethodPost, api+"/"+oldA+"/actions/gone", token, map[string]any{})
|
|
if got := env.resolve(t, "/old-a"); got["action"] != "gone" {
|
|
t.Fatalf("/old-a after gone = %v, want gone", got)
|
|
}
|
|
before := env.redirectCount(t)
|
|
rec = env.call(t, http.MethodPost, api+"/bulk/gone", token, map[string]any{"ids": []string{oldA, oldB, oldC}})
|
|
if rec.Code != http.StatusForbidden || !strings.Contains(rec.Body.String(), "A redirect for “/old-a” already exists") {
|
|
t.Fatalf("bulk gone over a covered entry = %d %s, want 403 notfound.has_rule", rec.Code, rec.Body.String())
|
|
}
|
|
if after := env.redirectCount(t); after != before {
|
|
t.Fatalf("redirects after a refused bulk = %d, want %d", after, before)
|
|
}
|
|
rec = env.expect(t, http.StatusOK, http.MethodPost, api+"/bulk/gone", token, map[string]any{"ids": []string{oldB, oldC}})
|
|
if !strings.Contains(rec.Body.String(), `"affected":2`) {
|
|
t.Fatalf("bulk gone = %s, want 2 affected", rec.Body.String())
|
|
}
|
|
if err := json.Unmarshal(env.expect(t, http.StatusOK, http.MethodGet, api, token, nil).Body.Bytes(), &list); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for _, id := range []string{missing, oldA, oldB, oldC} {
|
|
if states := list.Meta.RowStates[id]; len(states) != 1 || states[0] != "disabled" {
|
|
t.Fatalf("row states = %v, want %s resolved", list.Meta.RowStates, id)
|
|
}
|
|
}
|
|
var open []string
|
|
if err := json.Unmarshal(env.expect(t, http.StatusOK, http.MethodGet, api+"?filter[status]=open", token, nil).Body.Bytes(), &list); err == nil {
|
|
for _, d := range list.Data {
|
|
open = append(open, d.URL)
|
|
}
|
|
}
|
|
if len(open) != 0 {
|
|
t.Fatalf("open filter = %v, want none", open)
|
|
}
|
|
}
|
|
|
|
// TestAddressChangeFromEvent checks the after-commit path for writes
|
|
// outside the admin forms (D-18, D-92, D-102): a Journal-style write that
|
|
// changes a published record's slug and fires RecordSaved without a
|
|
// controller creates the 301; a save without an address change and a
|
|
// change of an unpublished record create nothing.
|
|
func TestAddressChangeFromEvent(t *testing.T) {
|
|
env := bootEnv(t)
|
|
token := env.admin(t, true)
|
|
id := env.createPage(t, token, map[string]any{"title": "Beta", "slug": "beta", "published": true})
|
|
draft := env.createPage(t, token, map[string]any{"title": "Draft", "slug": "draft", "published": false})
|
|
|
|
write := func(id string, changes ...pact.RecordChange) {
|
|
t.Helper()
|
|
if err := lagoon.Transaction(t.Context(), env.db, func(ctx context.Context, tx *gorm.DB) error {
|
|
for _, c := range changes {
|
|
if err := tx.Exec(`UPDATE acme_pages_pages SET `+c.Attribute+` = ? WHERE id = ?`, c.New, id).Error; err != nil {
|
|
return err
|
|
}
|
|
}
|
|
cabana.EmitRecordSaved(ctx, env.app, tx, pact.RecordSaved{Model: pagesModel, ID: id, Changes: changes})
|
|
return nil
|
|
}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
}
|
|
|
|
write(id, pact.RecordChange{Attribute: "title", Old: "Beta", New: "Beta 2"})
|
|
if n := env.redirectCount(t); n != 0 {
|
|
t.Fatalf("redirects after a save without an address change = %d, want 0", n)
|
|
}
|
|
|
|
write(id, pact.RecordChange{Attribute: "slug", Old: "beta", New: "gamma"})
|
|
rows := env.redirects(t)
|
|
if redirectTo(rows, "/pages/beta") != "/pages/gamma" || redirectTo(rows, "/pl/pages/beta") != "/pl/pages/gamma" {
|
|
t.Fatalf("redirects after an API slug change = %+v, want /pages/beta -> /pages/gamma in both locales", rows)
|
|
}
|
|
for _, r := range rows {
|
|
if r.Origin != "slug" || r.Status != 301 {
|
|
t.Fatalf("redirect %+v, want a 301 with origin slug", r)
|
|
}
|
|
}
|
|
env.expectRedirect(t, "/pages/beta", 301, siteURL+"/pages/gamma")
|
|
if rows := env.revisions(t, "redirect"); len(rows) == 0 || rows[0].Via != "auto" {
|
|
t.Fatalf("redirect history = %+v, want via auto", rows)
|
|
}
|
|
|
|
before := env.redirectCount(t)
|
|
write(draft, pact.RecordChange{Attribute: "slug", Old: "draft", New: "draft-2"})
|
|
if n := env.redirectCount(t); n != before {
|
|
t.Fatalf("redirects after an unpublished record's slug change = %d, want %d", n, before)
|
|
}
|
|
var url string
|
|
if err := env.db.Raw(`SELECT url FROM golem15_seo_entries WHERE record_id = ? AND locale = 'en'`, draft).Scan(&url).Error; err != nil || url != "/pages/draft-2" {
|
|
t.Fatalf("unpublished entry URL = %q err=%v, want the new address without a redirect", url, err)
|
|
}
|
|
|
|
// An admin save already applied the change: the event after it
|
|
// changes nothing more.
|
|
env.expect(t, http.StatusOK, http.MethodPut, adminAPI("/acme/pages/pages/"+id), token, map[string]any{"title": "Beta 2", "slug": "delta", "published": true})
|
|
rows = env.redirects(t)
|
|
if redirectTo(rows, "/pages/gamma") != "/pages/delta" || redirectTo(rows, "/pages/beta") != "/pages/delta" {
|
|
t.Fatalf("redirects after the admin save = %+v, want both old addresses on /pages/delta", rows)
|
|
}
|
|
}
|