From 17b810dfde88240d2c717e6087f9fd95a7662546 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 4 Nov 2026 13:02:39 +0100 Subject: [PATCH] feat: sitemap filter excludes noindex, canonicalised and redirected URLs (tracer) - classes.SitemapFilter implements the sitemap plugin's contract.Filter on the production values: redirected, noindex and canonicalised addresses leave the sitemaps with their reason; staging lists the same set - the plugin requires golem15.sitemap and publishes the filter in Register - go.mod requires sm-sitemap-plugin, go.work mounts the sibling checkout --- classes/resolver.go | 6 +- classes/sitemap_filter.go | 166 ++++++++++++++++++++++++++++++++++++++ fixture_test.go | 8 +- go.mod | 1 + go.work | 2 + plugin.go | 17 +++- plugin.yaml | 2 + sitemap_filter_test.go | 90 +++++++++++++++++++++ 8 files changed, 286 insertions(+), 6 deletions(-) create mode 100644 classes/sitemap_filter.go create mode 100644 sitemap_filter_test.go diff --git a/classes/resolver.go b/classes/resolver.go index 7e867f8..ad02882 100644 --- a/classes/resolver.go +++ b/classes/resolver.go @@ -103,6 +103,10 @@ type MetaTag struct { // Resolver answers the effective SEO of a URL. type Resolver struct { Service *Service + // production computes the values as production serves them: the + // non-production lock (D-22) is not applied. The sitemap filter decides + // on these, so staging lists the same addresses as production. + production bool } // Resolver returns the service's resolver. @@ -600,7 +604,7 @@ func (r *Resolver) resolveAt(ctx context.Context, db *gorm.DB, settings models.S layers = append(layers, robotsLayer{typeSetting.Robots, SourceTemplate}) } robots, robotsSource := resolveRobots(layers) - if res.Staging { + if res.Staging && !r.production { robots, robotsSource = Robots{Index: "noindex", Follow: "nofollow"}, SourceDefault } seo.Robots, src["robots"] = robots.Header(), robotsSource diff --git a/classes/sitemap_filter.go b/classes/sitemap_filter.go new file mode 100644 index 0000000..39b311e --- /dev/null +++ b/classes/sitemap_filter.go @@ -0,0 +1,166 @@ +package classes + +import ( + "context" + "net/url" + "strings" + + "git.golem15.com/golem15/sm-seo-plugin/models" + "git.golem15.com/golem15/sm-sitemap-plugin/contract" + "git.golem15.com/golem15/summercms/modules/pact" + "gorm.io/gorm" +) + +// SitemapFilter is the SEO plugin's contract.Filter for the sitemap plugin +// (D-95, D-24): an address whose effective SEO is noindex, whose canonical +// URL points elsewhere, or that redirects leaves the XML and HTML sitemaps +// with the reason noindex, canonical or redirect. It decides on the +// production values: the non-production lock (every page noindex outside +// production, D-22) never excludes an address, so staging and production +// list the same set. Service is asked on every call, because the filter is +// published in Register, before the service exists. +type SitemapFilter struct { + Service func() *Service +} + +var _ contract.Filter = SitemapFilter{} + +// filterBatch is what one Decide call reads once for all its candidates. +type filterBatch struct { + svc *Service + resolver *Resolver + db *gorm.DB + settings models.Settings + opts NormalizeOptions + set *RedirectSet + site string +} + +// Decide answers one Decision per candidate, in order. Without a booted +// service every address stays listed. +func (f SitemapFilter) Decide(ctx context.Context, batch []contract.Candidate) ([]contract.Decision, error) { + out := make([]contract.Decision, len(batch)) + var svc *Service + if f.Service != nil { + svc = f.Service() + } + if svc == nil || len(batch) == 0 { + return out, nil + } + b, err := svc.newFilterBatch(ctx) + if err != nil { + return nil, err + } + for i, c := range batch { + d, err := b.decide(ctx, c) + if err != nil { + return nil, err + } + out[i] = d + } + return out, nil +} + +// Groups lists the file groups the decisions name. +func (f SitemapFilter) Groups(context.Context) ([]contract.Group, error) { + return nil, nil +} + +// newFilterBatch reads the settings and the redirects once for a batch. +func (s *Service) newFilterBatch(ctx context.Context) (*filterBatch, error) { + db, err := s.DB(ctx) + if err != nil { + return nil, err + } + settings, err := LoadSettings(db) + if err != nil { + return nil, err + } + set, err := LoadRedirectSet(db) + if err != nil { + return nil, err + } + return &filterBatch{ + svc: s, + resolver: &Resolver{Service: s, production: true}, + db: db, + settings: settings, + opts: NormalizeOptionsOf(settings), + set: set, + site: s.SiteURL(), + }, nil +} + +// decide is the decision of one candidate: a redirected address first (it +// is no page), then noindex, then a canonical URL elsewhere. +func (b *filterBatch) decide(ctx context.Context, c contract.Candidate) (contract.Decision, error) { + path, query, rawQuery, ok := b.candidatePath(c.URL) + if !ok { + return contract.Decision{}, nil + } + norm, changed := Normalize(&url.URL{Path: path, RawQuery: rawQuery}, b.opts) + if changed { + return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, nil + } + chain, err := b.set.Follow(b.site, norm.Path, norm.RawQuery, MaxRedirectHops) + if err != nil { + return contract.Decision{}, err + } + if len(chain.Hops) > 0 { + return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, nil + } + key := "" + var match *pact.ContentMatchResult + if reg := b.svc.Registry(); reg != nil && c.RecordID != "" { + if typ, ok := reg.ContentType(c.TypeKey); ok && typ.Content != nil { + key = c.TypeKey + match = &pact.ContentMatchResult{ID: c.RecordID, Locale: c.Locale, Exact: true} + } + } + res, err := b.resolver.resolveAt(ctx, b.db, b.settings, path, query, rawQuery, key, match, nil, true) + if err != nil { + return contract.Decision{}, err + } + if res.SEO == nil { + return contract.Decision{}, nil + } + if r, err := ParseRobots(res.SEO.Robots); err == nil && r.Index == "noindex" { + return contract.Decision{Exclude: true, Reason: contract.ReasonNoindex}, nil + } + if !sameAddress(res.SEO.Canonical, res.URL) { + return contract.Decision{Exclude: true, Reason: contract.ReasonCanonical}, nil + } + return contract.Decision{}, nil +} + +// candidatePath turns a candidate's address (a path, or an absolute URL on +// the site or its www twin) into the decoded path and its query; ok is +// false for an address the plugin cannot judge (another host, malformed). +func (b *filterBatch) candidatePath(raw string) (string, url.Values, string, bool) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", nil, "", false + } + if u, err := url.Parse(raw); err == nil && u.IsAbs() && !sameSite(b.site, u.Host) { + return "", nil, "", false + } + path, query, err := parseTarget(raw) + if err != nil { + return "", nil, "", false + } + return path, query, rawQueryOf(raw), true +} + +// sameAddress reports whether two absolute URLs name the same page: the +// host without letter case, the path and the query as written. +func sameAddress(a, b string) bool { + if a == b { + return true + } + ua, errA := url.Parse(strings.TrimSpace(a)) + ub, errB := url.Parse(strings.TrimSpace(b)) + if errA != nil || errB != nil { + return false + } + return strings.EqualFold(ua.Host, ub.Host) && ua.EscapedPath() == ub.EscapedPath() && ua.RawQuery == ub.RawQuery +} diff --git a/fixture_test.go b/fixture_test.go index d575214..aa62d06 100644 --- a/fixture_test.go +++ b/fixture_test.go @@ -12,6 +12,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "slices" "strconv" "strings" "sync" @@ -21,6 +22,7 @@ import ( "time" seo "git.golem15.com/golem15/sm-seo-plugin" + _ "git.golem15.com/golem15/sm-sitemap-plugin" "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/bouncer" "git.golem15.com/golem15/summercms/modules/cabana" @@ -328,9 +330,13 @@ func bootEnv(t *testing.T, extraEnv ...string) *testEnv { return bootPlugins(t, []string{"acme.pages", "golem15.seo"}, extraEnv...) } -// bootPlugins boots the plugins ids on a fresh database. +// bootPlugins boots the plugins ids on a fresh database. golem15.seo +// requires golem15.sitemap, so the sitemap plugin is booted with it. func bootPlugins(t *testing.T, ids []string, extraEnv ...string) *testEnv { t.Helper() + if slices.Contains(ids, "golem15.seo") && !slices.Contains(ids, seo.SitemapPluginID) { + ids = append([]string{seo.SitemapPluginID}, ids...) + } sqlDB := freshDB(t) gdb, err := lagoon.Use(t.Context(), sqlDB) if err != nil { diff --git a/go.mod b/go.mod index 6d1935f..1bfbfe1 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module git.golem15.com/golem15/sm-seo-plugin go 1.27.0 require ( + git.golem15.com/golem15/sm-sitemap-plugin v0.1.2 git.golem15.com/golem15/summercms v0.6.0 github.com/go-gormigrate/gormigrate/v2 v2.1.7 github.com/goccy/go-yaml v1.19.2 diff --git a/go.work b/go.work index 9f57e58..414a95a 100644 --- a/go.work +++ b/go.work @@ -3,3 +3,5 @@ go 1.27.0 use . replace git.golem15.com/golem15/summercms => ../summercms.go + +replace git.golem15.com/golem15/sm-sitemap-plugin => ../sm-sitemap-plugin diff --git a/plugin.go b/plugin.go index 2ed506a..4be2684 100644 --- a/plugin.go +++ b/plugin.go @@ -14,6 +14,7 @@ import ( "git.golem15.com/golem15/sm-seo-plugin/classes" "git.golem15.com/golem15/sm-seo-plugin/models" "git.golem15.com/golem15/sm-seo-plugin/updates" + "git.golem15.com/golem15/sm-sitemap-plugin/contract" "git.golem15.com/golem15/summercms/modules/backpack" "git.golem15.com/golem15/summercms/modules/pact" "git.golem15.com/golem15/summercms/modules/party" @@ -46,13 +47,21 @@ type Plugin struct { func (p *Plugin) ID() string { return "golem15.seo" } -// Requires is empty: content types are found through signpost, never by -// plugin ID. -func (p *Plugin) Requires() []string { return nil } +// SitemapPluginID is the sitemap plugin the SEO plugin requires: it +// decides which addresses the sitemaps list (D-95). Content types are still +// found through signpost, never by plugin ID. +const SitemapPluginID = "golem15.sitemap" +// Requires names the sitemap plugin, whose Filter seam the SEO plugin +// implements (D-95). +func (p *Plugin) Requires() []string { return []string{SitemapPluginID} } + +// Register publishes the sitemap filter (classes.SitemapFilter): noindex, +// canonicalised and redirected addresses leave the sitemaps (D-95, D-24). +// It reads the service lazily, since Boot builds the service. func (p *Plugin) Register(app *backpack.App) error { p.app = app - return nil + return app.Publish[contract.Filter](classes.SitemapFilter{Service: p.Service}) } // Boot builds the service on the application's database, signpost registry diff --git a/plugin.yaml b/plugin.yaml index 6e75dc3..bc5937b 100644 --- a/plugin.yaml +++ b/plugin.yaml @@ -6,5 +6,7 @@ package: git.golem15.com/golem15/sm-seo-plugin name: SEO description: "The SEO plugin for SummerCMS applications: an SEO tab on every content type, per-URL rules, redirects and the 404 log, robots.txt with AI bot switches, global and per-type SEO settings, change history, and the redirect decision and effective SEO of any URL for the frontend." framework: v0.6.0 +requires: + golem15.sitemap: "" overrides: name: SEO diff --git a/sitemap_filter_test.go b/sitemap_filter_test.go new file mode 100644 index 0000000..03fa573 --- /dev/null +++ b/sitemap_filter_test.go @@ -0,0 +1,90 @@ +package seo_test + +import ( + "net/http" + "strings" + "testing" + + sitemapclasses "git.golem15.com/golem15/sm-sitemap-plugin/classes" +) + +// rebuildSitemap runs the sitemap plugin's rebuild on the test app, as its +// job and command do. +func (e *testEnv) rebuildSitemap(t *testing.T) { + t.Helper() + if _, err := sitemapclasses.Rebuild(t.Context(), e.app); err != nil { + t.Fatalf("sitemap rebuild: %v", err) + } +} + +// sitemapFile fetches one sitemap file by name; a missing file is "". +func (e *testEnv) sitemapFile(t *testing.T, name string) string { + t.Helper() + rec := e.call(t, http.MethodGet, "/_sitemap/files/"+name, "", nil) + if rec.Code == http.StatusNotFound { + return "" + } + if rec.Code != http.StatusOK { + t.Fatalf("GET %s = %d: %s", name, rec.Code, rec.Body.String()) + } + return rec.Body.String() +} + +// sitemapReason is the excluded reason the sitemap stored for a record's +// address in a locale ("" while it is listed). +func (e *testEnv) sitemapReason(t *testing.T, typeKey, id, locale string) string { + t.Helper() + var rows []string + if err := e.db.Raw(`SELECT excluded_reason FROM golem15_sitemap_urls WHERE type_key = ? AND record_id = ? AND locale = ?`, typeKey, id, locale).Scan(&rows).Error; err != nil { + t.Fatal(err) + } + if len(rows) == 0 { + t.Fatalf("no sitemap row for %s %s %s", typeKey, id, locale) + } + return rows[0] +} + +// TestSitemapFilterTracer is the plan's tracer: an editor sets a page to +// noindex in the SEO tab, the sitemap rebuild asks the SEO plugin's +// published filter, and the page leaves the sitemap file with the reason +// noindex. Staging lists the same set as production (the non-production +// lock never excludes an address). +func TestSitemapFilterTracer(t *testing.T) { + for _, envName := range []string{"production", "staging"} { + t.Run(envName, func(t *testing.T) { + env := bootEnv(t, "SUMMER_ENV="+envName) + token := env.admin(t, true) + first := env.createPage(t, token, map[string]any{"title": "First", "slug": "first", "published": true}) + second := env.createPage(t, token, map[string]any{"title": "Second", "slug": "second", "published": true}) + + env.rebuildSitemap(t) + file := env.sitemapFile(t, "sitemap-pages-1.xml") + if !strings.Contains(file, ".test/pages/first") || !strings.Contains(file, ".test/pages/second") { + t.Fatalf("before noindex the file lists both pages:\n%s", file) + } + + env.expect(t, http.StatusOK, http.MethodPut, adminAPI("/acme/pages/pages/"+first), token, map[string]any{ + "title": "First", "slug": "first", "published": true, + "seo_robots": map[string]any{"en": "noindex"}, + }) + env.rebuildSitemap(t) + file = env.sitemapFile(t, "sitemap-pages-1.xml") + if strings.Contains(file, ".test/pages/first") { + t.Fatalf("the noindex page is still listed:\n%s", file) + } + if !strings.Contains(file, ".test/pages/second") { + t.Fatalf("the indexable page left the file:\n%s", file) + } + if got := env.sitemapReason(t, "acme-page", first, "en"); got != "noindex" { + t.Fatalf("excluded reason = %q, want noindex", got) + } + if got := env.sitemapReason(t, "acme-page", second, "en"); got != "" { + t.Fatalf("second page reason = %q, want listed", got) + } + // The pl address has no noindex of its own and stays listed. + if got := env.sitemapReason(t, "acme-page", first, "pl"); got != "" { + t.Fatalf("pl address reason = %q, want listed", got) + } + }) + } +}