feat: noindex paths from the route report leave the sitemaps (tracer)

The route report takes an optional noindex list of static paths. When
the key is present the stored set (golem15_seo_noindex_paths) is
replaced, and the sitemap filter leaves those paths out with the reason
noindex; a changed set asks the sitemap for a debounced rebuild through
contract.Rebuilder. A listed noindex address without a CMS record now
names sitemap_update_settings, or fix: code without the sitemap plugin.

classes/crawl/checks.go

@@ -127,6 +127,11 @@ type Env struct {
 	GenericAnchors []string
 	// Keyphrases maps an address to its SEO entry's focus keyphrase.
 	Keyphrases map[string]string
+	// SitemapTool is the tool that edits the sitemap's Additional
+	// addresses (ToolSitemapSettings while the sitemap plugin is mounted,
+	// else ""): a listed noindex address without a CMS record is one of
+	// them.
+	SitemapTool string
 }
 
 func (e *Env) record(key string) *Record {
@@ -330,6 +335,24 @@ func hasEmptyHeading(rec *Record, level int) bool {
 	return false
 }
 
+// sitemapNoindex is the finding of a listed noindex page. A CMS record's
+// page is fixed with seo_set_url_seo; an address without a record is an
+// Additional address of the sitemap (or another listed address the SEO
+// plugin cannot change), fixed by removing it from the sitemap's address
+// list: env.SitemapTool, else fix code naming that list.
+func sitemapNoindex(key, file string, rec *Record, env *Env) Finding {
+	f := Finding{Kind: KindSitemapNoindex, URL: key, Locale: locale(rec), Record: rec, Details: map[string]any{"file": file}}
+	if rec != nil {
+		f.Tool = ToolSetURLSEO
+		return f
+	}
+	f.Details["extra"] = true
+	if env != nil {
+		f.Tool = env.SitemapTool
+	}
+	return f
+}
+
 // checkSitemaps compares the sitemaps with what the crawl found:
 // sitemap_error for a listed address that does not answer 200,
 // sitemap_noindex for a listed noindex page, sitemap_blocked for a listed
@@ -352,7 +375,7 @@ func checkSitemaps(res *Result, env *Env) []Finding {
 			case p.Resp.Err != "" || p.Resp.Status != 200:
 				out = append(out, Finding{Kind: KindSitemapError, URL: key, Locale: locale(rec), Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"status": p.Resp.Status, "file": file}})
 			case env.noindex(p):
-				out = append(out, Finding{Kind: KindSitemapNoindex, URL: key, Locale: locale(rec), Record: rec, Tool: ToolSetURLSEO, Details: map[string]any{"file": file}})
+				out = append(out, sitemapNoindex(key, file, rec, env))
 			}
 		}
 		if robots != "" && res.fetcher != nil {

classes/crawl/checks_test.go

@@ -203,3 +203,50 @@ func TestEnvNoindex(t *testing.T) {
 		t.Fatal("nil env and record helpers")
 	}
 }
+
+// TestSitemapNoindexTool pins G1 (SC-3, D-14): a listed noindex page with
+// a CMS record names seo_set_url_seo; one without a record (an Additional
+// address of the sitemap) names the sitemap's settings tool while the
+// sitemap plugin is mounted, and fix: code naming the sitemap's address
+// list otherwise.
+func TestSitemapNoindexTool(t *testing.T) {
+	res := &Result{
+		Pages: map[string]*Page{
+			"/cms":   okPage("/cms", &PageFacts{Robots: "noindex"}),
+			"/extra": okPage("/extra", &PageFacts{Robots: "noindex"}),
+		},
+		Listed: map[string]string{"/cms": "/sitemap-pages-1.xml", "/extra": "/sitemap-pages-1.xml"},
+	}
+	rec := &Record{Type: "seo-page", ID: "1"}
+	lookup := func(key string) *Record {
+		if key == "/cms" {
+			return rec
+		}
+		return nil
+	}
+	find := func(env *Env, key string) Finding {
+		t.Helper()
+		for _, f := range checkSitemaps(res, env) {
+			if f.Kind == KindSitemapNoindex && f.URL == key {
+				return f
+			}
+		}
+		t.Fatalf("no sitemap_noindex on %s", key)
+		return Finding{}
+	}
+	mounted := &Env{Lookup: lookup, SitemapTool: ToolSitemapSettings}
+	if fix, tool, _ := Route(find(mounted, "/cms")); fix != models.FixTool || tool != ToolSetURLSEO {
+		t.Fatalf("CMS record: %s %q", fix, tool)
+	}
+	extra := find(mounted, "/extra")
+	if fix, tool, _ := Route(extra); fix != models.FixTool || tool != "sitemap_update_settings" || extra.Details["extra"] != true {
+		t.Fatalf("Additional address with the sitemap mounted: %s %q %v", fix, tool, extra.Details)
+	}
+	if _, ok := find(mounted, "/cms").Details["extra"]; ok {
+		t.Fatal("a CMS record's finding is marked extra")
+	}
+	alone := &Env{Lookup: lookup}
+	if fix, tool, what := Route(find(alone, "/extra")); fix != models.FixCode || tool != "" || what != "the address list of the sitemap that names this page" {
+		t.Fatalf("Additional address without the sitemap: %s %q %q", fix, tool, what)
+	}
+}

classes/crawl/fix.go

@@ -18,6 +18,9 @@ const (
 	ToolUpdatePage     = "seo_update_page"
 	ToolCreatePage     = "seo_create_page"
 	ToolUpdateSettings = "seo_update_settings"
+	// ToolSitemapSettings is the sitemap plugin's settings tool, which
+	// edits its Additional addresses.
+	ToolSitemapSettings = "sitemap_update_settings"
 )
 
 // Record is the CMS record behind a crawled page: its content type, id
@@ -132,6 +135,7 @@ var defaultWhat = map[string]string{
 	KindGenericAnchor:    "the link text in the page template",
 	KindRedirectedLink:   "the link in the page template",
 	KindOrphanPage:       "a link to this page from the site navigation or another page",
+	KindSitemapNoindex:   "the address list of the sitemap that names this page",
 }
 
 // Fingerprint identifies a finding across crawls: the kind, the address,

classes/crawl/job.go

@@ -9,6 +9,7 @@ import (
 	"git.golem15.com/golem15/sm-seo-plugin/classes"
 	"git.golem15.com/golem15/sm-seo-plugin/classes/schema"
 	"git.golem15.com/golem15/sm-seo-plugin/models"
+	"git.golem15.com/golem15/sm-sitemap-plugin/contract"
 	"git.golem15.com/golem15/summercms/modules/conga"
 	"git.golem15.com/golem15/summercms/modules/diary"
 	"git.golem15.com/golem15/summercms/modules/lagoon"
@@ -16,6 +17,20 @@ import (
 	"gorm.io/gorm"
 )
 
+// SitemapTool is ToolSitemapSettings while the application publishes the
+// sitemap plugin's contract.Listing (the sitemap plugin is mounted), else
+// "".
+func SitemapTool(svc *classes.Service) string {
+	app := svc.App()
+	if app == nil {
+		return ""
+	}
+	if _, ok := app.Lookup[contract.Listing](); ok {
+		return ToolSitemapSettings
+	}
+	return ""
+}
+
 // CrawlKind is the River kind of the crawl job.
 const CrawlKind = "golem15.seo.crawl"
 
@@ -264,7 +279,7 @@ func finish(ctx context.Context, svc *classes.Service, db *gorm.DB, row models.C
 		if err != nil {
 			return err
 		}
-		findings := Check(res, &Env{Lookup: Lookup(ctx, svc, tx), Staging: svc.Staging(), GenericAnchors: limits.GenericAnchors, Keyphrases: keyphrases})
+		findings := Check(res, &Env{Lookup: Lookup(ctx, svc, tx), Staging: svc.Staging(), GenericAnchors: limits.GenericAnchors, Keyphrases: keyphrases, SitemapTool: SitemapTool(svc)})
 		cms, err := svc.Issues(classes.WithTx(ctx, tx))
 		if err != nil {
 			return err

classes/registry.go

@@ -12,6 +12,7 @@ import (
 	"unicode/utf8"
 
 	"git.golem15.com/golem15/sm-seo-plugin/models"
+	"git.golem15.com/golem15/sm-sitemap-plugin/contract"
 	"git.golem15.com/golem15/summercms/modules/cabana"
 	"git.golem15.com/golem15/summercms/modules/lagoon"
 	"git.golem15.com/golem15/summercms/modules/pact"
@@ -817,6 +818,113 @@ func (s *Service) ReportRoutes(ctx context.Context, tx *gorm.DB, paths []string)
 	return out, nil
 }
 
+// ReportNoindex replaces the stored noindex paths (P33-58, the route
+// report's noindex key) inside tx with the valid entries of paths, each
+// normalised like a reported route (normalPagePath with the site's address
+// options; a percent-encoded path is stored decoded when its decoded form
+// is a valid address, the form the sitemap filter compares). An entry that
+// is no valid address, or that is a pattern (a "*" or a ":" parameter
+// segment), is skipped and counted. changed reports whether the stored set
+// differs from the one before; when it does, the sitemap is asked for one
+// debounced rebuild through contract.Rebuilder once tx commits (nothing
+// when no Rebuilder is published). More than MaxReportedRoutes paths is
+// ErrTooManyRoutes and stores nothing.
+func (s *Service) ReportNoindex(ctx context.Context, tx *gorm.DB, paths []string) (stored int, skipped int, changed bool, err error) {
+	if len(paths) > MaxReportedRoutes {
+		return 0, 0, false, ErrTooManyRoutes
+	}
+	settings, err := SettingsRow(tx)
+	if err != nil {
+		return 0, 0, false, err
+	}
+	opts := NormalizeOptionsOf(settings)
+	want := map[string]bool{}
+	for _, raw := range paths {
+		path, ok := noindexPath(raw, opts)
+		if !ok {
+			skipped++
+			continue
+		}
+		want[path] = true
+	}
+	var have []string
+	if err := tx.Model(&models.NoindexPath{}).Pluck("path", &have).Error; err != nil {
+		return 0, skipped, false, err
+	}
+	changed = len(have) != len(want)
+	for _, p := range have {
+		if !want[p] {
+			changed = true
+			break
+		}
+	}
+	if !changed {
+		return len(want), skipped, false, nil
+	}
+	if err := tx.Exec(`DELETE FROM golem15_seo_noindex_paths`).Error; err != nil {
+		return 0, skipped, false, err
+	}
+	now := time.Now().UTC()
+	rows := make([]models.NoindexPath, 0, len(want))
+	for p := range want {
+		rows = append(rows, models.NoindexPath{Path: p, ReportedAt: now})
+	}
+	sort.Slice(rows, func(i, j int) bool { return rows[i].Path < rows[j].Path })
+	if len(rows) > 0 {
+		if err := tx.Clauses(clause.OnConflict{DoNothing: true}).CreateInBatches(rows, 500).Error; err != nil {
+			return 0, skipped, false, err
+		}
+	}
+	lagoon.AfterCommit(ctx, tx, func(ctx context.Context, _ *gorm.DB) {
+		s.requestSitemapRebuild(ctx)
+	})
+	return len(rows), skipped, true, nil
+}
+
+// NoindexPathCount is the number of stored noindex paths.
+func NoindexPathCount(db *gorm.DB) (int, error) {
+	var n int64
+	err := db.Model(&models.NoindexPath{}).Count(&n).Error
+	return int(n), err
+}
+
+// noindexPath is a reported noindex entry as it is stored: trimmed,
+// decoded when its decoded form is a valid address, and normalised; ok is
+// false for an invalid address or a pattern.
+func noindexPath(raw string, opts NormalizeOptions) (string, bool) {
+	path := strings.TrimSpace(raw)
+	if len(path) > MaxRouteBytes || !ValidPagePath(path) || strings.Contains(path, "*") {
+		return "", false
+	}
+	for _, seg := range strings.Split(path, "/") {
+		if strings.HasPrefix(seg, ":") {
+			return "", false
+		}
+	}
+	if strings.Contains(path, "%") {
+		if dec, err := url.PathUnescape(path); err == nil && ValidPagePath(dec) {
+			path = dec
+		}
+	}
+	return normalPagePath(path, opts), true
+}
+
+// requestSitemapRebuild asks the sitemap plugin for one debounced rebuild
+// through contract.Rebuilder; nothing when none is published, a warning
+// when the request fails.
+func (s *Service) requestSitemapRebuild(ctx context.Context) {
+	if s == nil || s.app == nil {
+		return
+	}
+	rb, ok := s.app.Lookup[contract.Rebuilder]()
+	if !ok || rb == nil {
+		return
+	}
+	if err := rb.RequestRebuild(ctx); err != nil {
+		log.Warn("golem15.seo: request a sitemap rebuild", "error", err)
+	}
+}
+
 // routeTitle is a reported page's name: its last path segment with
 // hyphens and underscores as spaces and the first letter capitalised; the
 // home page is named pages.home_title.

classes/sitemap_filter.go

@@ -12,7 +12,8 @@ import (
 )
 
 // 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
+// (D-95, D-24): an address the frontend reports noindex (the route
+// report's noindex paths), 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
@@ -34,6 +35,7 @@ type filterBatch struct {
 	opts     NormalizeOptions
 	set      *RedirectSet
 	site     string
+	noindex  map[string]bool
 }
 
 // Decide answers one Decision per candidate, in order. Without a booted
@@ -107,7 +109,8 @@ func (f SitemapFilter) Groups(ctx context.Context) ([]contract.Group, error) {
 	return out, nil
 }
 
-// newFilterBatch reads the settings and the redirects once for a batch.
+// newFilterBatch reads the settings, the redirects and the noindex paths
+// of the route report once for a batch.
 func (s *Service) newFilterBatch(ctx context.Context) (*filterBatch, error) {
 	db, err := s.DB(ctx)
 	if err != nil {
@@ -121,6 +124,14 @@ func (s *Service) newFilterBatch(ctx context.Context) (*filterBatch, error) {
 	if err != nil {
 		return nil, err
 	}
+	var paths []string
+	if err := db.Model(&models.NoindexPath{}).Pluck("path", &paths).Error; err != nil {
+		return nil, err
+	}
+	noindex := make(map[string]bool, len(paths))
+	for _, p := range paths {
+		noindex[p] = true
+	}
 	return &filterBatch{
 		svc:      s,
 		resolver: &Resolver{Service: s, production: true},
@@ -129,6 +140,7 @@ func (s *Service) newFilterBatch(ctx context.Context) (*filterBatch, error) {
 		opts:     NormalizeOptionsOf(settings),
 		set:      set,
 		site:     s.SiteURL(),
+		noindex:  noindex,
 	}, nil
 }
 
@@ -140,8 +152,9 @@ func (b *filterBatch) decide(ctx context.Context, c contract.Candidate) (contrac
 
 // decideTarget is the decision of one candidate and, for a canonical or a
 // redirect, the address the sitemaps would list instead: a redirected
-// address first (it is no page), then noindex, then a canonical URL
-// elsewhere.
+// address first (it is no page), then a path the frontend's route report
+// names noindex (P33-58), then the effective SEO's noindex, then a
+// canonical URL elsewhere.
 func (b *filterBatch) decideTarget(ctx context.Context, c contract.Candidate) (contract.Decision, string, error) {
 	path, query, rawQuery, ok := b.candidatePath(c.URL)
 	if !ok {
@@ -158,6 +171,9 @@ func (b *filterBatch) decideTarget(ctx context.Context, c contract.Candidate) (c
 	if len(chain.Hops) > 0 {
 		return contract.Decision{Exclude: true, Reason: contract.ReasonRedirect}, displayURL(b.site, chain.Final), nil
 	}
+	if b.noindex[norm.Path] {
+		return contract.Decision{Exclude: true, Reason: contract.ReasonNoindex}, "", nil
+	}
 	key := ""
 	var match *pact.ContentMatchResult
 	if reg := b.svc.Registry(); reg != nil && c.RecordID != "" {