From e31ac7c1c688d561ad53a8a460a426b81b939aa9 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 11:20:59 +0100 Subject: [PATCH] feat: address-change 301 (tracer) - golem15_seo_redirects table and Redirect model - AddressChanged: old -> new 301 in the save transaction, earlier redirects pointed at the new address, self-loops dropped - the SEO tab save compares descriptor URLs with the entry URLs and answers the redirects.auto_created notice - resolve answers redirect with the absolute target for an exact source --- classes/addresses.go | 226 ++++++++++++++++++ classes/redirects.go | 58 +++++ classes/resolver.go | 47 +++- extensions.go | 24 +- lang/en/lang.yaml | 2 + lang/pl/lang.yaml | 2 + models/redirect.go | 69 ++++++ redirects_test.go | 121 ++++++++++ ...2610270006_create_golem15_seo_redirects.go | 39 +++ 9 files changed, 574 insertions(+), 14 deletions(-) create mode 100644 classes/addresses.go create mode 100644 classes/redirects.go create mode 100644 models/redirect.go create mode 100644 redirects_test.go create mode 100644 updates/202610270006_create_golem15_seo_redirects.go diff --git a/classes/addresses.go b/classes/addresses.go new file mode 100644 index 0000000..b03175c --- /dev/null +++ b/classes/addresses.go @@ -0,0 +1,226 @@ +package classes + +import ( + "context" + "strconv" + "strings" + "time" + + "git.golem15.com/golem15/sm-seo-plugin/models" + "git.golem15.com/golem15/summercms/modules/pact" + "gorm.io/gorm" +) + +// AddressChange is one record's public address change in one locale: the +// URL its SEO entry last saw (Old) and the URL its content descriptor +// answers now (New). +type AddressChange struct { + TypeKey string + ModelType string + RecordID string + Locale string + Old string + New string +} + +// AddressResult is what AddressChanged did: the redirect from the old to +// the new address, and how many earlier redirects now point straight to +// the new address. +type AddressResult struct { + Locale string + Old string + New string + Redirect *models.Redirect + Repointed int +} + +// AddressChanged applies a record's address change inside tx, the +// caller's transaction (D-18, P-20): +// 1. exact redirects whose source is the new address are deleted, so the +// record is never shadowed (this also undoes an earlier change back); +// 2. old → new is created as a 301 (origin slug), or the existing exact +// rule of the old address is pointed at the new one; +// 3. redirects whose target is the old address are pointed at the new +// one, so no chain forms (A→B, then B→C makes A→C); +// 4. redirects that now point to themselves are deleted; +// 5. the entry's URL becomes the new address. +// +// Every redirect write is recorded in history with via auto. Nothing +// happens when either address is empty or they are equal. +func (s *Service) AddressChanged(ctx context.Context, tx *gorm.DB, change AddressChange) (AddressResult, error) { + res := AddressResult{Locale: change.Locale, Old: strings.TrimSpace(change.Old), New: strings.TrimSpace(change.New)} + if res.Old == "" || res.New == "" || res.Old == res.New { + return res, nil + } + ctx = WithVia(ctx, models.ViaAuto) + now := time.Now().UTC() + + var shadowing []models.Redirect + if err := tx.Where("match = ? AND source = ?", models.RedirectExact, res.New).Find(&shadowing).Error; err != nil { + return res, err + } + for _, r := range shadowing { + if err := s.deleteRedirect(ctx, tx, r); err != nil { + return res, err + } + } + + var existing []models.Redirect + if err := tx.Where("match = ? AND source = ?", models.RedirectExact, res.Old).Limit(1).Find(&existing).Error; err != nil { + return res, err + } + var redirect models.Redirect + if len(existing) > 0 { + redirect = existing[0] + before := RedirectValues(redirect) + redirect.Target, redirect.Status, redirect.UpdatedAt = res.New, models.Status301, &now + if err := tx.Save(&redirect).Error; err != nil { + return res, err + } + if err := s.recordRedirect(ctx, tx, redirect, before, RedirectValues(redirect)); err != nil { + return res, err + } + } else { + redirect = models.NewRedirect() + redirect.Source, redirect.Target, redirect.Origin = res.Old, res.New, models.OriginSlug + redirect.CreatedAt, redirect.UpdatedAt = &now, &now + if err := tx.Create(&redirect).Error; err != nil { + return res, err + } + if err := s.recordRedirect(ctx, tx, redirect, nil, RedirectValues(redirect)); err != nil { + return res, err + } + } + res.Redirect = &redirect + + var pointing []models.Redirect + targets := []string{res.Old} + if site := s.SiteURL(); site != "" { + targets = append(targets, site+res.Old) + } + if err := tx.Where("target IN ? AND id <> ?", targets, redirect.ID).Find(&pointing).Error; err != nil { + return res, err + } + for _, r := range pointing { + before := RedirectValues(r) + r.Target, r.UpdatedAt = res.New, &now + if r.Match == models.RedirectExact && s.samePath(r.Target, r.Source) { + if err := s.deleteRedirect(ctx, tx, r); err != nil { + return res, err + } + continue + } + if err := tx.Save(&r).Error; err != nil { + return res, err + } + if err := s.recordRedirect(ctx, tx, r, before, RedirectValues(r)); err != nil { + return res, err + } + res.Repointed++ + } + + if change.ModelType != "" && change.RecordID != "" { + if err := tx.Model(&models.Entry{}). + Where("model_type = ? AND record_id = ? AND locale = ?", change.ModelType, change.RecordID, change.Locale). + Updates(map[string]any{"url": res.New, "updated_at": now}).Error; err != nil { + return res, err + } + } + return res, nil +} + +// deleteRedirect deletes r inside tx and records it in history. +func (s *Service) deleteRedirect(ctx context.Context, tx *gorm.DB, r models.Redirect) error { + if err := tx.Delete(&models.Redirect{}, r.ID).Error; err != nil { + return err + } + return s.recordRedirect(ctx, tx, r, RedirectValues(r), nil) +} + +// recordRedirect writes the history row of one redirect write. +func (s *Service) recordRedirect(ctx context.Context, tx *gorm.DB, r models.Redirect, before, after map[string]any) error { + return s.Record(ctx, tx, Change{ + Kind: models.SubjectRedirect, + SubjectID: strconv.FormatUint(uint64(r.ID), 10), + What: s.redirectWhat(r), + Before: before, + After: after, + }) +} + +// AddressChanges compares the addresses a record's content descriptor +// answers now with the URLs its SEO entries last saw, inside tx (D-18), +// and returns the changes that need a redirect: the changed locales of a +// published record, each to be applied with AddressChanged. A changed +// locale of an unpublished record only takes its new URL, without a +// redirect, and a locale without an entry gets one with its URL, so the +// next change can be detected. +func (s *Service) AddressChanges(ctx context.Context, tx *gorm.DB, typ pact.MenuItemType, id string) ([]AddressChange, error) { + reg := s.Registry() + if reg == nil || typ.Content == nil { + return nil, nil + } + rec, err := reg.Describe(ctx, typ.Key, id, s.UnprefixedLocale()) + if err != nil || rec == nil { + return nil, err + } + model := typ.Content.Model + var out []AddressChange + for _, l := range rec.Locales { + url := strings.TrimSpace(l.URL) + if url == "" { + continue + } + entry, found, err := LoadEntry(tx, model, id, l.Locale) + if err != nil { + return nil, err + } + switch { + case !found: + row := models.Entry{ModelType: model, RecordID: id, Locale: l.Locale, TypeKey: typ.Key, URL: url} + if err := tx.Create(&row).Error; err != nil { + return nil, err + } + case entry.URL == url: + case entry.URL == "" || !rec.Published: + if err := tx.Model(&models.Entry{}).Where("id = ?", entry.ID).Updates(map[string]any{"url": url, "type_key": typ.Key}).Error; err != nil { + return nil, err + } + default: + out = append(out, AddressChange{ + TypeKey: typ.Key, ModelType: model, RecordID: id, Locale: l.Locale, + Old: entry.URL, New: url, + }) + } + } + return out, nil +} + +// AddressNotices are the save notices of address changes +// (redirects.auto_created, one per changed locale), on the owner field +// named after a placeholder of the type's pattern that the save changed +// (for example slug), else on fallback. +func AddressNotices(typ pact.MenuItemType, changes []pact.RecordChange, results []AddressResult, fallback string) map[string][]pact.FormFieldNotice { + if len(results) == 0 { + return nil + } + field := fallback + for _, c := range changes { + for _, p := range typ.Placeholders { + if strings.TrimPrefix(p, ":") == c.Attribute { + field = c.Attribute + } + } + if field != fallback { + break + } + } + out := map[string][]pact.FormFieldNotice{} + for _, r := range results { + out[field] = append(out[field], pact.FormFieldNotice{ + Message: "golem15.seo::lang.redirects.auto_created", + Params: map[string]string{"old": r.Old, "new": r.New, "count": strconv.Itoa(r.Repointed)}, + }) + } + return out +} diff --git a/classes/redirects.go b/classes/redirects.go new file mode 100644 index 0000000..491bd9a --- /dev/null +++ b/classes/redirects.go @@ -0,0 +1,58 @@ +package classes + +import ( + "strings" + + "git.golem15.com/golem15/sm-seo-plugin/models" + "gorm.io/gorm" +) + +// The resolve actions of the redirect stage. +const ( + ActionRedirect = "redirect" + ActionGone = "gone" +) + +// exactRedirect returns the exact redirect whose source is path, nil when +// there is none. +func exactRedirect(db *gorm.DB, path string) (*models.Redirect, error) { + var rows []models.Redirect + if err := db.Where("match = ? AND source = ?", models.RedirectExact, path).Limit(1).Find(&rows).Error; err != nil { + return nil, err + } + if len(rows) == 0 { + return nil, nil + } + return &rows[0], nil +} + +// RedirectValues are a redirect's values for history. +func RedirectValues(r models.Redirect) map[string]any { + return map[string]any{ + "source": r.Source, + "match": r.Match, + "target": r.Target, + "status": r.Status, + "query": r.Query, + "ignore_case": r.IgnoreCase, + "trailing_slash": r.TrailingSlash, + "origin": r.Origin, + "note": r.Note, + } +} + +// redirectWhat is the history description of a redirect. +func (s *Service) redirectWhat(r models.Redirect) string { + return s.Phrase("golem15.seo::lang.history.what_redirect", map[string]string{"url": r.Source}) +} + +// samePath reports whether target names path on this site: the path +// itself, or an absolute URL on the site URL with that path. +func (s *Service) samePath(target, path string) bool { + target = strings.TrimSpace(target) + if target == path { + return true + } + site := s.SiteURL() + return site != "" && target == site+path +} diff --git a/classes/resolver.go b/classes/resolver.go index 1551816..c895efb 100644 --- a/classes/resolver.go +++ b/classes/resolver.go @@ -46,19 +46,22 @@ const ( SourceDefault = "default" ) -// Result is the effective SEO of one URL. Action is ok or not_found; Status -// is the HTTP status the frontend should answer with. URL is the absolute -// URL of the page (path and kept query on the site URL). Sources maps each -// value name (title, description, canonical, robots, og_title, -// og_description, og_image) to where it came from. +// Result is the effective SEO of one URL. Action is ok, not_found, +// redirect or gone; Status is the HTTP status the frontend should answer +// with. URL is the absolute URL of the page (path and kept query on the +// site URL). Sources maps each value name (title, description, canonical, +// robots, og_title, og_description, og_image) to where it came from. type Result struct { - Action string `json:"action"` - Status int `json:"status"` - URL string `json:"url"` - Locale string `json:"locale"` - Staging bool `json:"staging"` - SEO *SEO `json:"seo"` - Sources map[string]string `json:"-"` + Action string `json:"action"` + Status int `json:"status"` + // Location is the absolute address a redirect answer sends the visitor + // to; absent on every other action. + Location string `json:"location,omitempty"` + URL string `json:"url"` + Locale string `json:"locale"` + Staging bool `json:"staging"` + SEO *SEO `json:"seo"` + Sources map[string]string `json:"-"` // TypeKey and RecordID name the record behind the URL, "" when none. TypeKey string `json:"-"` RecordID string `json:"-"` @@ -357,6 +360,13 @@ func (r *Resolver) Resolve(ctx context.Context, rawURL string) (Result, error) { if err != nil { return Result{}, err } + redirect, err := exactRedirect(db, path) + if err != nil { + return Result{}, err + } + if redirect != nil { + return r.redirectAnswer(svc.SiteURL()+path, *redirect), nil + } key, match, rec, err := r.lookup(ctx, path) if err != nil { return Result{}, err @@ -557,6 +567,19 @@ func (r *Resolver) resolveAt(ctx context.Context, db *gorm.DB, settings models.S return res, nil } +// redirectAnswer is the answer of a matched redirect: gone for a 410, +// else redirect with the rule's status and its target absolute on the +// site URL. +func (r *Resolver) redirectAnswer(self string, rule models.Redirect) Result { + res := Result{URL: self, Staging: r.Service.Staging(), Sources: map[string]string{}} + if rule.Status == models.Status410 { + res.Action, res.Status = ActionGone, models.Status410 + return res + } + res.Action, res.Status, res.Location = ActionRedirect, rule.Status, AbsoluteURL(r.Service.SiteURL(), rule.Target) + return res +} + // rawQueryOf returns the raw query of the resolver input. func rawQueryOf(raw string) string { u, err := url.Parse(strings.TrimSpace(raw)) diff --git a/extensions.go b/extensions.go index 2892987..b45a52c 100644 --- a/extensions.go +++ b/extensions.go @@ -246,7 +246,9 @@ func (p *Plugin) loadSEOTab(ctx context.Context, key string, in pact.AdminFormLo } // saveSEOTab stores the submitted SEO tab values of a record inside the -// record's save transaction. +// record's save transaction. On an update it first compares the record's +// addresses with its entries' URLs and creates the 301 of a changed +// address (AddressChanged), answered as the redirects.auto_created notice. func (p *Plugin) saveSEOTab(ctx context.Context, key string, in pact.AdminFormSave) (pact.AdminFormSaveResult, error) { svc := p.Service() if svc == nil { @@ -279,10 +281,28 @@ func (p *Plugin) saveSEOTab(ctx context.Context, key string, in pact.AdminFormSa if len(refusals) > 0 { return pact.AdminFormSaveResult{}, refusals } + // The address diff runs before the values are written, because the + // write refreshes the entries' URLs (D-18, P-20). + var moved []classes.AddressResult + if !in.Created { + changes, err := svc.AddressChanges(ctx, in.DB, typ, in.ID) + if err != nil { + return pact.AdminFormSaveResult{}, err + } + for _, change := range changes { + res, err := svc.AddressChanged(ctx, in.DB, change) + if err != nil { + return pact.AdminFormSaveResult{}, err + } + if res.Redirect != nil { + moved = append(moved, res) + } + } + } if _, _, err := svc.SaveRecord(ctx, in.DB, classes.RecordWrite{Type: typ, ID: in.ID, Values: values}); err != nil { return pact.AdminFormSaveResult{}, err } - return pact.AdminFormSaveResult{}, nil + return pact.AdminFormSaveResult{Notices: classes.AddressNotices(typ, in.Changes, moved, "seo_status")}, nil } // localeMap reads a submitted value as locale -> text: a translatable diff --git a/lang/en/lang.yaml b/lang/en/lang.yaml index 1081f3d..d4f3c8e 100644 --- a/lang/en/lang.yaml +++ b/lang/en/lang.yaml @@ -189,3 +189,5 @@ image: snippet: action: Check duplicates invalid: The preview request is not valid. +redirects: + auto_created: The address changed, so a 301 from “:old” to “:new” was created. :count earlier redirects now point straight to the new address. diff --git a/lang/pl/lang.yaml b/lang/pl/lang.yaml index 977d6b2..07a0a10 100644 --- a/lang/pl/lang.yaml +++ b/lang/pl/lang.yaml @@ -193,3 +193,5 @@ image: snippet: action: Sprawdź duplikaty invalid: Żądanie podglądu jest nieprawidłowe. +redirects: + auto_created: Adres się zmienił, więc utworzono przekierowanie 301 z „:old” na „:new”. Wcześniejsze przekierowania (:count) prowadzą teraz bezpośrednio na nowy adres. diff --git a/models/redirect.go b/models/redirect.go new file mode 100644 index 0000000..b2bbf92 --- /dev/null +++ b/models/redirect.go @@ -0,0 +1,69 @@ +package models + +import "time" + +// The match kinds of a redirect source (D-17). +const ( + RedirectExact = "exact" + RedirectPattern = "pattern" + RedirectRegex = "regex" +) + +// The statuses of a redirect (D-16). +const ( + Status301 = 301 + Status302 = 302 + Status410 = 410 +) + +// The query string options of a redirect (D-17). +const ( + QueryIgnore = "ignore" + QueryPass = "pass" +) + +// The origins of a redirect: who created it. +const ( + OriginManual = "manual" + OriginSlug = "slug" + OriginImport = "import" + OriginMCP = "mcp" + OriginNotFound = "notfound" +) + +// Redirect is one redirect rule on golem15_seo_redirects (D-16, D-17): +// Source is a path (exact, optionally with a query), a pattern where * +// matches any text, or a regular expression; Target is a path or an +// absolute URL, empty for 410 Gone. IgnoreCase compares the source without +// letter case; TrailingSlash matches the source with and without a +// trailing slash; Query ignores the request's query string or passes it to +// the target. Origin records who created the rule. +type Redirect struct { + ID uint `gorm:"column:id;primaryKey"` + Source string `gorm:"column:source"` + Match string `gorm:"column:match"` + Target string `gorm:"column:target"` + Status int `gorm:"column:status"` + Query string `gorm:"column:query"` + IgnoreCase bool `gorm:"column:ignore_case"` + TrailingSlash bool `gorm:"column:trailing_slash"` + Origin string `gorm:"column:origin"` + Note string `gorm:"column:note"` + CreatedAt *time.Time `gorm:"column:created_at"` + UpdatedAt *time.Time `gorm:"column:updated_at"` +} + +func (Redirect) TableName() string { return "golem15_seo_redirects" } + +// MorphName names a redirect in history. +func (Redirect) MorphName() string { return `Golem15\SEO\Models\Redirect` } + +// NewRedirect is a redirect with the form's defaults: exact, 301, query +// ignored, trailing slash matched, manual. +func NewRedirect() Redirect { + return Redirect{Match: RedirectExact, Status: Status301, Query: QueryIgnore, TrailingSlash: true, Origin: OriginManual} +} + +func init() { + Register(Redirect{}) +} diff --git a/redirects_test.go b/redirects_test.go new file mode 100644 index 0000000..b178291 --- /dev/null +++ b/redirects_test.go @@ -0,0 +1,121 @@ +package seo_test + +import ( + "encoding/json" + "net/http" + "strings" + "testing" +) + +// 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) + } +} diff --git a/updates/202610270006_create_golem15_seo_redirects.go b/updates/202610270006_create_golem15_seo_redirects.go new file mode 100644 index 0000000..931c496 --- /dev/null +++ b/updates/202610270006_create_golem15_seo_redirects.go @@ -0,0 +1,39 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// createRedirects creates golem15_seo_redirects: 301, 302 and 410 rules +// with exact, pattern and regex sources (D-16, D-17), one per source and +// match. +var createRedirects = []*gormigrate.Migration{ + { + ID: "202610270006_create_golem15_seo_redirects", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(`CREATE TABLE golem15_seo_redirects ( + id BIGSERIAL PRIMARY KEY, + source TEXT NOT NULL, + match TEXT NOT NULL DEFAULT 'exact', + target TEXT NOT NULL DEFAULT '', + status INTEGER NOT NULL DEFAULT 301, + query TEXT NOT NULL DEFAULT 'ignore', + ignore_case BOOLEAN NOT NULL DEFAULT FALSE, + trailing_slash BOOLEAN NOT NULL DEFAULT TRUE, + origin TEXT NOT NULL DEFAULT 'manual', + note TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP(0) NULL, + updated_at TIMESTAMP(0) NULL, + CONSTRAINT golem15_seo_redirects_source_match UNIQUE (source, match) +)`).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(`DROP TABLE IF EXISTS golem15_seo_redirects`).Error + }, + }, +} + +func init() { + Register(createRedirects...) +}