From 9de6b2ec833b57e74ea135f3169718392296f024 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Wed, 4 Nov 2026 22:44:26 +0100 Subject: [PATCH] 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. --- README.md | 28 ++++- classes/crawl/checks.go | 25 +++- classes/crawl/checks_test.go | 47 ++++++++ classes/crawl/fix.go | 4 + classes/crawl/job.go | 17 ++- classes/registry.go | 108 +++++++++++++++++ classes/sitemap_filter.go | 24 +++- controllers/api/registry.go | 53 ++++++-- models/noindex_path.go | 15 +++ registry_test.go | 21 +++- sitemap_filter_test.go | 114 ++++++++++++++++++ ...040001_create_golem15_seo_noindex_paths.go | 28 +++++ 12 files changed, 458 insertions(+), 26 deletions(-) create mode 100644 models/noindex_path.go create mode 100644 updates/202611040001_create_golem15_seo_noindex_paths.go diff --git a/README.md b/README.md index c794d9f..17b436b 100644 --- a/README.md +++ b/README.md @@ -193,7 +193,23 @@ The **robots.txt** screen (a singleton) has the editor, the tester and **What cr **SEO › Pages** (controller `golem15.seo.pages`, after URL rules) lists the registry by address with the kind, origin, content date and the **Not on the site** state; the form takes the address and the name per locale (a locale left empty has no version of the page), the kind, the parent and the content date (the sitemap's lastmod), with the SEO tab next to it. A changed address gets its 301 like any record. -`POST /_seo/api/v1/registry/routes` takes `{"routes": ["/", "/kontakt", "/o-nas"]}` from the frontend server with `Authorization: Bearer `: `golem15.seo.route_report_token`, else `golem15.seo.notfound_token` (compared in constant time). It answers `200` with `{"inserted", "updated", "skipped", "reported_at"}`; a missing or wrong token is `401`, no token configured `404`, more than 2000 paths `422`. An unknown path becomes a page with origin `reported`, kind `page` and a name from its last segment; a known page only gets `reported_at`; a path another content type owns, or one that is no path, is skipped. A report never deletes a page or changes a value an editor set; a reported page the newest report did not name shows **Not on the site** and a warning on its form. Throttle bucket `seo-notfound`. A content type whose file group is also `pages` shares the file with the registry's pages. +`POST /_seo/api/v1/registry/routes` takes the frontend server's static routes and, optionally, the static paths it serves noindex, with `Authorization: Bearer `: `golem15.seo.route_report_token`, else `golem15.seo.notfound_token` (compared in constant time; prefer a dedicated `route_report_token`): + +```json +{"routes": ["/", "/blog", "/kontakt"], "noindex": ["/konto", "/pl/konto", "/szkic"]} +``` + +It answers `200` with the counts and `noindex`, the number of stored noindex paths: + +```json +{"inserted": 1, "updated": 3, "skipped": 0, "reported_at": "2026-11-04T20:00:00Z", "noindex": 3} +``` + +A missing or wrong token is `401`, no token configured `404`, more than 2000 entries in either list `422` `{"error": "too_many_routes", "max": 2000}` (nothing is stored). + +`noindex` lists every static path the frontend's build serves noindex (each starting with `/`, without a `:` parameter or `*`, at most 2048 bytes), the locale copies as their own prefixed paths. When the key is present (also as `[]`) the stored set (`golem15_seo_noindex_paths`) becomes exactly its valid entries, normalised like the routes; an invalid entry or a pattern is skipped and counted in `skipped`. When the key is absent the set stays as it is, so a frontend that sends no `noindex` keeps working. The sitemap filter leaves every address whose path is in the set out of the XML and HTML sitemaps with the reason `noindex` (a record's address, a registry page or an Additional address of the sitemap settings), and a changed set asks the sitemap for one debounced rebuild through `contract.Rebuilder` (with an older sitemap plugin the next scheduled rebuild picks it up). + +For the routes, an unknown path becomes a page with origin `reported`, kind `page` and a name from its last segment; a known page only gets `reported_at`; a path another content type owns, or one that is no path, is skipped. A report never deletes a page or changes a value an editor set; a reported page the newest report did not name shows **Not on the site** and a warning on its form. Throttle bucket `seo-notfound`. A content type whose file group is also `pages` shares the file with the registry's pages. ### Auto-links @@ -239,7 +255,7 @@ A crawl is a conga job (`golem15.seo.crawl`, one attempt) started from **Start c **Rendered comparison.** When `golem15.seo.crawl_chrome_path` names a Chrome or Chromium binary the operator installed on the server (none is installed by default), the crawler renders the first `golem15.seo.crawl_render_max` pages (default 200, in crawl order, so the home page and the top-level pages come first) in headless Chrome through `github.com/chromedp/chromedp`, right after each page's raw fetch, and compares the DOM after JavaScript with the raw HTML: same-host links, an H1, or at least 50 words and 20 % more text that appear only after JavaScript are one `content_js` finding with `details.check: rendered` and `details.what` naming them (up to five links). The browser is held to the crawler's limits: before Chrome starts, the site host and its `www` twin are resolved and every address checked like the crawler's dial guard (`fetchguard.IsPrivateAddr`; a private address refuses unless `golem15.seo.crawl_trusted` is on), and Chrome is pinned to those addresses through `--host-resolver-rules` while every other host resolves to nothing; every request is intercepted and continued only when it is https (http only in trusted mode) to the site host or its twin, or a `data:`/`blob:` URL, so third-party scripts, fonts, analytics and metadata addresses are never loaded. Each crawl gets a fresh temporary profile, removed when the crawl ends, with no cookies, credentials or extensions and downloads denied; one tab renders at a time, 20 s per page, the DOM is capped at 5 MiB, Chrome's own sandbox stays on, and Chrome is closed when the crawl finishes, fails or is cancelled. The path is config only (or `SUMMER_GOLEM15__SEO__CRAWL_CHROME_PATH`), never a setting or MCP value. The crawl records `render_status` `done` with the pages compared, `skipped` without a path, or `failed` with the reason (a relative path or one that is not an executable file, a private address, Chrome not starting, a page that cannot be rendered); a renderer failure never fails the crawl, and P6, `GET /_seo/api/v1/crawls/{id}` and `seo_get_crawl` (`render {status, pages, note, message}`) say which. -**Fix routes.** An issue a CMS value fixes names its tool: `seo_set_url_seo` (titles, descriptions, canonicals, sitemap membership of a record, structured data), `seo_update_robots`, `seo_create_redirect` (a missing page, a page that answers an error), `seo_create_page` (a page missing from the registry), `seo_update_settings` (publisher and site nodes), or the content type's own tool from `pact.ContentDescriptor.Tools` when the problem sits in the record's content: H1 findings prefer `Tools[headings]`, image alt text `Tools[image_alt]`, other content findings `Tools[edit]` (the registry pages name `seo_update_page`). A finding of the site's template code, or of content whose type names no fitting tool, is `fix: code` with `details.what` naming the element to change. No issue has neither. +**Fix routes.** An issue a CMS value fixes names its tool: `seo_set_url_seo` (titles, descriptions, canonicals, sitemap membership of a record, structured data), `seo_update_robots`, `seo_create_redirect` (a missing page, a page that answers an error), `seo_create_page` (a page missing from the registry), `seo_update_settings` (publisher and site nodes), `sitemap_update_settings` (a listed noindex address without a CMS record, an Additional address of the sitemap: remove it there; `details.extra` is true, and without the sitemap plugin the issue is `fix: code` naming the sitemap's address list), or the content type's own tool from `pact.ContentDescriptor.Tools` when the problem sits in the record's content: H1 findings prefer `Tools[headings]`, image alt text `Tools[image_alt]`, other content findings `Tools[edit]` (the registry pages name `seo_update_page`). A finding of the site's template code, or of content whose type names no fitting tool, is `fix: code` with `details.what` naming the element to change. No issue has neither. **Ignore and check again.** An issue's fingerprint (kind, address, locale and the `content_js` check) can be ignored (**Ignore**, bulk **Ignore issues**, `seo_ignore_issues`); it stays hidden in later crawls until **Stop ignoring** (`seo_unignore_issues`). **Check this address again** (`seo_recheck_url`) fetches the issue's address (with its canonical and hreflang targets) and replaces that address's issues; findings that depend on other pages (duplicates, orphans, link findings while the page still errors) stay until the next crawl. @@ -304,9 +320,9 @@ The element bundle (`assets/dist/seo.js` and `seo.css`) is built from `assets/sr | `OpList`, `OpShow`, `OpCreate`, `OpUpdate`, `OpDelete`, `OpExtensionSave`, `OpUpload`, `OpRecordAction`, `OpBulkAction`, `OpWidgetAction`, `OpToolbarAction` | The kinds of an admin operation. | | `RobotsTestAction` | `robots_test`, the tester's widget action. | | `SitemapPluginID` | `golem15.sitemap`, the plugin `Requires()` names. | -| `classes.SitemapFilter` | The published `contract.Filter`: `Decide` (reasons `noindex`, `canonical`, `redirect` on the production values; registry pages grouped by kind) and `Groups` (the kinds' groups with their titles per locale). | +| `classes.SitemapFilter` | The published `contract.Filter`: `Decide` (reasons `noindex` (the effective SEO, or a path the route report names noindex), `canonical`, `redirect` on the production values; registry pages grouped by kind) and `Groups` (the kinds' groups with their titles per locale). | | `classes.PageMenuItemType`, `classes.PageTypeKey`, `classes.PagesController`, `classes.PageModel`, `classes.PageGroup` | The registry's menu item type `seo-page` and its content descriptor, and the file group of a kind. | -| `classes.PageWrite`, `classes.RouteReport`, `classes.ValidPagePath`, `classes.PageMissing` | A registry write, a route report's outcome, the address check and the missing state. Service: `CheckPage`, `SavePage`, `DeletePage`, `ApplyPageTexts`, `ReportRoutes`, `RouteReportToken`, `PageAt`. | +| `classes.PageWrite`, `classes.RouteReport`, `classes.ValidPagePath`, `classes.PageMissing` | A registry write, a route report's outcome, the address check and the missing state. Service: `CheckPage`, `SavePage`, `DeletePage`, `ApplyPageTexts`, `ReportRoutes`, `ReportNoindex`, `RouteReportToken`, `PageAt`; `classes.NoindexPathCount` counts the stored noindex paths (`models.NoindexPath`). | | `classes.SiteAnswer` | `Service.Site`, the site answer. `Service.LLMSText` builds llms.txt. | | `classes.Outbound`, `classes.OutboundOf` | The resolve answer's `outbound` from the settings. | | `schema.Build`, `schema.Input`, `schema.Validate`, `schema.Normalize`, `schema.DecodeCustom`, `schema.Merge` | Package `classes/schema`: the JSON-LD graph of a page, Google's required and recommended properties per type, the structured data value and custom JSON-LD. | @@ -379,7 +395,7 @@ The non-production lock reads the application environment (`SUMMER_ENV`); it has ## Dependencies - SummerCMS v0.6.0 modules: `pact`, `party`, `backpack`, `cabana`, `signpost`, `lagoon`, `conga`, `cooler` (the site-cache purge), `surf`, `wire`, `bouncer`, `phrasebook`, `bonfire`, `compass`, `diary`. -- `git.golem15.com/golem15/sm-sitemap-plugin`, its `contract` package only (the Filter the plugin publishes and the Listing it reads); the plugin requires `golem15.sitemap`. +- `git.golem15.com/golem15/sm-sitemap-plugin`, its `contract` package only (the Filter the plugin publishes, and the Listing and Rebuilder it reads); the plugin requires `golem15.sitemap`. - `gorm.io/gorm`, `github.com/go-gormigrate/gormigrate/v2`, `gocloud.dev/blob` (the media bucket for social images), `github.com/goccy/go-yaml` (the per-type settings tabs and robots form documents). - `github.com/jimsmart/grobotstxt` v1.0.3, the robots.txt matcher (decision note `.planning/notes/2026-10-27-robots-txt-grobotstxt.md` in the framework repository). - `golang.org/x/net/html`, the crawler's HTML parser (already required by the framework). @@ -397,4 +413,4 @@ npm ci && npm run typecheck && npm test && bash scripts/check-dist.sh The element tests (`tests/elements`, vitest on happy-dom) drive the widget elements through a fake of the admin's widget host (`tests/elements/host.ts`), including a host that drops an action. -The smoke tests boot the plugin with fake content plugins (`acme.pages`, and `acme.blog` with a category segment for strict URLs) and exercise the SEO tab, the settings tabs, the resolver precedence, history, generation, the URL rule form, redirects (engine, loops and chains, address changes from the admin and from events), the 404 log, normalisation and strict URLs, robots.txt and verification files through the admin and public APIs, and the management API as an MCP client (`TestMCPSetURLSeoTracer`, `TestManagementAPI`, `TestIssues`), batches, undo and CSV (`TestBatch`, `TestUndo`, `TestCSVTransfer`, and the `mcp` and `import` cases of `TestLoopRefused`). The sitemap, the registry and the site-level endpoints are covered by `TestSitemapFilterTracer`, `TestFilterReasons`, `TestNoindexPublishedConflict`, `TestSitemapBlockedIssue`, `TestRegistry`, `TestRouteReport`, `TestSiteEndpoint`, `TestLLMSTxt`, `TestRobotsSitemapLine`, `TestSettingsLinks` and `TestPurgeAfterWrite`; the fixture boots the sitemap plugin next to the SEO plugin. The site audit is covered by `TestCrawlerTracer`, `TestCrawlerChecks` (a planted site with one page per kind and a clean control page, served to the crawler through `fetchguard.WithTransport`), `TestIssueMerge`, `TestStagingCrawl`, `TestRenderSkipped`, `TestCrawlAdmin`, `TestCrawlSchedule` and `TestCrawlTools`, with the fake content plugin `acme.crawl` naming fixing tools. Auto-links are covered by `TestAutolinksTracer` (the phrases of a page's locale in the resolve answer, never the page's own, `autolink_max` 0) and `TestAutolinks` (the screen, its refusals, Add focus keyphrases, the API and MCP); `TestLinkSuggestions` crawls a planted site and checks E10's counts and suggestions through the API and the widget action. `TestChromeRenderer` renders a page in a real headless Chrome (it runs when `SEO_TEST_CHROME` names a binary or `chromium`, `chromium-browser` or `google-chrome` is on `PATH`, and is skipped otherwise) and `TestRendererGuards` checks the refused paths and private addresses. `TestEveryAdminOperationHasTool`, `TestMCPReadToolsAreGET`, `TestIssueKindLabels` and the static parts of `TestCrawlTools` and `TestCrawlSchedule` run without Docker. +The smoke tests boot the plugin with fake content plugins (`acme.pages`, and `acme.blog` with a category segment for strict URLs) and exercise the SEO tab, the settings tabs, the resolver precedence, history, generation, the URL rule form, redirects (engine, loops and chains, address changes from the admin and from events), the 404 log, normalisation and strict URLs, robots.txt and verification files through the admin and public APIs, and the management API as an MCP client (`TestMCPSetURLSeoTracer`, `TestManagementAPI`, `TestIssues`), batches, undo and CSV (`TestBatch`, `TestUndo`, `TestCSVTransfer`, and the `mcp` and `import` cases of `TestLoopRefused`). The sitemap, the registry and the site-level endpoints are covered by `TestSitemapFilterTracer`, `TestFilterReasons`, `TestNoindexPublishedConflict`, `TestSitemapBlockedIssue`, `TestRegistry`, `TestRouteReport`, `TestNoindexPathsTracer`, `TestSiteEndpoint`, `TestLLMSTxt`, `TestRobotsSitemapLine`, `TestSettingsLinks` and `TestPurgeAfterWrite`; the fixture boots the sitemap plugin next to the SEO plugin. The site audit is covered by `TestCrawlerTracer`, `TestCrawlerChecks` (a planted site with one page per kind and a clean control page, served to the crawler through `fetchguard.WithTransport`), `TestIssueMerge`, `TestStagingCrawl`, `TestRenderSkipped`, `TestCrawlAdmin`, `TestCrawlSchedule` and `TestCrawlTools`, with the fake content plugin `acme.crawl` naming fixing tools. Auto-links are covered by `TestAutolinksTracer` (the phrases of a page's locale in the resolve answer, never the page's own, `autolink_max` 0) and `TestAutolinks` (the screen, its refusals, Add focus keyphrases, the API and MCP); `TestLinkSuggestions` crawls a planted site and checks E10's counts and suggestions through the API and the widget action. `TestChromeRenderer` renders a page in a real headless Chrome (it runs when `SEO_TEST_CHROME` names a binary or `chromium`, `chromium-browser` or `google-chrome` is on `PATH`, and is skipped otherwise) and `TestRendererGuards` checks the refused paths and private addresses. `TestEveryAdminOperationHasTool`, `TestMCPReadToolsAreGET`, `TestIssueKindLabels` and the static parts of `TestCrawlTools` and `TestCrawlSchedule` run without Docker. diff --git a/classes/crawl/checks.go b/classes/crawl/checks.go index 33b6a92..490c6be 100644 --- a/classes/crawl/checks.go +++ b/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 { diff --git a/classes/crawl/checks_test.go b/classes/crawl/checks_test.go index 91a39e3..760957e 100644 --- a/classes/crawl/checks_test.go +++ b/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) + } +} diff --git a/classes/crawl/fix.go b/classes/crawl/fix.go index 17a1dd9..f58d53f 100644 --- a/classes/crawl/fix.go +++ b/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, diff --git a/classes/crawl/job.go b/classes/crawl/job.go index 74c0382..e5f5969 100644 --- a/classes/crawl/job.go +++ b/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 diff --git a/classes/registry.go b/classes/registry.go index 956ee37..71c9b9a 100644 --- a/classes/registry.go +++ b/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. diff --git a/classes/sitemap_filter.go b/classes/sitemap_filter.go index 19c20ed..4608919 100644 --- a/classes/sitemap_filter.go +++ b/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 != "" { diff --git a/controllers/api/registry.go b/controllers/api/registry.go index 67c254b..d15050a 100644 --- a/controllers/api/registry.go +++ b/controllers/api/registry.go @@ -16,23 +16,42 @@ import ( "gorm.io/gorm" ) -// maxRouteReportBody caps the route report body: 2000 paths of 2048 bytes -// with their JSON quoting. -const maxRouteReportBody = classes.MaxReportedRoutes*(classes.MaxRouteBytes+8) + 1024 +// maxRouteReportBody caps the route report body: two lists (routes and +// noindex) of 2000 paths of 2048 bytes with their JSON quoting. +const maxRouteReportBody = 2*classes.MaxReportedRoutes*(classes.MaxRouteBytes+8) + 1024 -// routeReport is the route report body. +// routeReport is the route report body. Noindex is nil when the key is +// absent and non-nil (also empty) when it is present. type routeReport struct { - Routes []string `json:"routes"` + Routes []string `json:"routes"` + Noindex *[]string `json:"noindex"` +} + +// routeReportAnswer is the route report answer: the counts of the routes +// and the number of stored noindex paths. +type routeReportAnswer struct { + classes.RouteReport + Noindex int `json:"noindex"` } // RouteReport serves POST /_seo/api/v1/registry/routes (D-33, P33-20): the // frontend server reports its static routes with "Authorization: Bearer // ", golem15.seo.route_report_token (else notfound_token), compared // in constant time. Unknown paths become registry pages (origin reported), -// known ones get reported_at; nothing is deleted or overwritten. It answers -// 200 with the counts, 401 for a missing or wrong token, 404 when no token -// is configured (the report is off), and 422 for a malformed body or more -// than 2000 paths. +// known ones get reported_at; nothing is deleted or overwritten. +// +// The optional noindex key (P33-58) lists the static paths the frontend +// serves noindex: when it is present (also as []), the stored set becomes +// exactly its valid entries, normalised like the routes, in the same +// transaction (an invalid entry is skipped and counted in skipped); when it +// is absent the stored set is unchanged. The sitemap filter leaves those +// paths out of the sitemaps, and a changed set asks the sitemap for one +// debounced rebuild. +// +// It answers 200 with the counts and noindex, the number of stored noindex +// paths; 401 for a missing or wrong token, 404 when no token is configured +// (the report is off), and 422 for a malformed body or more than 2000 +// paths in either list, storing nothing. func RouteReport(service func() *classes.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { svc := service() @@ -56,14 +75,24 @@ func RouteReport(service func() *classes.Service) http.HandlerFunc { wire.WriteJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": errInvalidBody}) return } - if len(in.Routes) > classes.MaxReportedRoutes { + if len(in.Routes) > classes.MaxReportedRoutes || (in.Noindex != nil && len(*in.Noindex) > classes.MaxReportedRoutes) { wire.WriteJSON(w, http.StatusUnprocessableEntity, map[string]any{"error": "too_many_routes", "max": classes.MaxReportedRoutes}) return } - var out classes.RouteReport + var out routeReportAnswer err := write(r.Context(), svc, func(ctx context.Context, tx *gorm.DB) error { var err error - out, err = svc.ReportRoutes(ctx, tx, in.Routes) + out.RouteReport, err = svc.ReportRoutes(ctx, tx, in.Routes) + if err != nil { + return err + } + if in.Noindex != nil { + var skipped int + out.Noindex, skipped, _, err = svc.ReportNoindex(ctx, tx, *in.Noindex) + out.Skipped += skipped + return err + } + out.Noindex, err = classes.NoindexPathCount(tx) return err }) if err != nil { diff --git a/models/noindex_path.go b/models/noindex_path.go new file mode 100644 index 0000000..c5eac02 --- /dev/null +++ b/models/noindex_path.go @@ -0,0 +1,15 @@ +package models + +import "time" + +// NoindexPath is one static path the frontend's route report names +// noindex (P33-58) on golem15_seo_noindex_paths: Path is normalised with +// the site's address rules, ReportedAt the time of the report that stored +// it. The set is replaced by every report that sends the noindex key. +type NoindexPath struct { + Path string `gorm:"column:path;primaryKey"` + ReportedAt time.Time `gorm:"column:reported_at"` +} + +// TableName is golem15_seo_noindex_paths. +func (NoindexPath) TableName() string { return "golem15_seo_noindex_paths" } diff --git a/registry_test.go b/registry_test.go index 32231df..0452df2 100644 --- a/registry_test.go +++ b/registry_test.go @@ -297,13 +297,30 @@ func TestRouteReport(t *testing.T) { many[i] = "/p" } env.expect(t, http.StatusUnprocessableEntity, http.MethodPost, "/_seo/api/v1/registry/routes", "report-token", map[string]any{"routes": many}) + // More than 2000 noindex paths (gap-2 contract) answer 422 and store + // nothing, not even the routes. + env.expect(t, http.StatusUnprocessableEntity, http.MethodPost, "/_seo/api/v1/registry/routes", "report-token", map[string]any{"routes": []string{"/stored-not"}, "noindex": many}) + var noindexRows, notStored int64 + env.db.Raw(`SELECT COUNT(*) FROM golem15_seo_noindex_paths`).Scan(&noindexRows) + env.db.Raw(`SELECT COUNT(*) FROM golem15_seo_pages WHERE url = '/stored-not'`).Scan(¬Stored) + if noindexRows != 0 || notStored != 0 { + t.Fatalf("a refused report stored %d noindex paths and %d pages", noindexRows, notStored) + } + // An invalid noindex entry is counted in skipped; patterns are invalid. + mixed := env.routeReportBody(t, "report-token", map[string]any{"routes": []string{}, "noindex": []string{"/konto", "konto", "/p/**", "/u/:id", " /konto "}}) + if mixed["noindex"] != float64(1) || mixed["skipped"] != float64(3) { + t.Fatalf("mixed noindex = %v, want 1 stored (/konto, a duplicate kept once) and 3 skipped", mixed) + } + if err := env.db.Exec(`DELETE FROM golem15_seo_noindex_paths`).Error; err != nil { + t.Fatal(err) + } env.createRegistryPage(t, admin, map[string]any{"url": map[string]any{"en": "/kontakt"}, "title": map[string]any{"en": "Kontakt"}}) env.createPage(t, admin, map[string]any{"title": "Blog post", "slug": "post", "published": true}) out := env.reportRoutes(t, "report-token", []string{"/", "/kontakt", "/o-nas", "/pages/post", "nope", "/o-nas"}) - if out["inserted"] != float64(2) || out["updated"] != float64(1) || out["skipped"] != float64(2) { - t.Fatalf("first report = %v, want 2 inserted (/, /o-nas), 1 updated, 2 skipped", out) + if out["inserted"] != float64(2) || out["updated"] != float64(1) || out["skipped"] != float64(2) || out["noindex"] != float64(0) || len(out) != 5 { + t.Fatalf("first report = %v, want 2 inserted (/, /o-nas), 1 updated, 2 skipped, noindex 0 and reported_at", out) } about := env.pageRow(t, "/o-nas") if about.Origin != "reported" || about.Kind != "page" || about.Title != "O nas" || about.Stamp == "" { diff --git a/sitemap_filter_test.go b/sitemap_filter_test.go index bfaaf98..1458ee4 100644 --- a/sitemap_filter_test.go +++ b/sitemap_filter_test.go @@ -214,3 +214,117 @@ func TestSitemapBlockedIssue(t *testing.T) { t.Fatalf("issue = %+v", got) } } + +// routeReportBody posts a route report body as it is and decodes the 200 +// answer. +func (e *testEnv) routeReportBody(t *testing.T, token string, body map[string]any) map[string]any { + t.Helper() + rec := e.expect(t, http.StatusOK, http.MethodPost, "/_seo/api/v1/registry/routes", token, body) + var out map[string]any + if err := json.Unmarshal(rec.Body.Bytes(), &out); err != nil { + t.Fatal(err) + } + return out +} + +// pendingRebuild reports whether the sitemap's debounced rebuild is +// requested, and clears the request as the rebuild job does. +func (e *testEnv) pendingRebuild(t *testing.T) bool { + t.Helper() + var pending *string + if err := e.db.Raw(`SELECT rebuild_pending_until::text FROM golem15_sitemap_settings WHERE id = 1`).Scan(&pending).Error; err != nil { + t.Fatal(err) + } + if err := e.db.Exec(`UPDATE golem15_sitemap_settings SET rebuild_pending_until = NULL WHERE id = 1`).Error; err != nil { + t.Fatal(err) + } + return pending != nil +} + +// TestNoindexPathsTracer is plan 33-15's tracer (G1, D-24, D-33, the gap-2 +// contract): the frontend's route report names /subscriptions noindex, the +// sitemap is asked for a rebuild, and the rebuild leaves that Additional +// address out of the XML and HTML sitemaps with the reason noindex. A +// report without the key keeps the set; an empty list clears it; the same +// set again asks for no rebuild. +func TestNoindexPathsTracer(t *testing.T) { + env := bootEnv(t, "SUMMER_GOLEM15__SEO__ROUTE_REPORT_TOKEN=report-token") + if err := env.db.Exec(`UPDATE golem15_sitemap_settings SET extra_urls = '[{"url":"/subscriptions"},{"url":"/kontakt"}]', html_enabled = TRUE WHERE id = 1`).Error; err != nil { + t.Fatal(err) + } + env.rebuildSitemap(t) + listed := func() bool { + t.Helper() + return strings.Contains(env.sitemapFile(t, "sitemap-pages-1.xml"), ".test/subscriptions") + } + inHTML := func() bool { + t.Helper() + rec := env.call(t, http.MethodGet, "/_sitemap/api/v1/html/pages", "", nil) + if rec.Code != http.StatusOK { + t.Fatalf("HTML group = %d: %s", rec.Code, rec.Body.String()) + } + return strings.Contains(rec.Body.String(), "/subscriptions") + } + extraReason := func() string { + t.Helper() + var rows []string + if err := env.db.Raw(`SELECT excluded_reason FROM golem15_sitemap_urls WHERE type_key = 'extra' AND url LIKE '%/subscriptions'`).Scan(&rows).Error; err != nil { + t.Fatal(err) + } + if len(rows) != 1 { + t.Fatalf("extra rows for /subscriptions = %v", rows) + } + return rows[0] + } + if !listed() || !inHTML() { + t.Fatal("before the report the Additional address is listed") + } + env.pendingRebuild(t) + + out := env.routeReportBody(t, "report-token", map[string]any{"routes": []string{"/"}, "noindex": []string{"/subscriptions"}}) + if out["noindex"] != float64(1) { + t.Fatalf("answer = %v, want noindex 1", out) + } + if !env.pendingRebuild(t) { + t.Fatal("a changed noindex set requested no sitemap rebuild") + } + env.rebuildSitemap(t) + if listed() || inHTML() { + t.Fatal("the reported noindex address is still in the sitemaps") + } + if got := extraReason(); got != "noindex" { + t.Fatalf("excluded reason = %q, want noindex", got) + } + if !strings.Contains(env.sitemapFile(t, "sitemap-pages-1.xml"), ".test/kontakt") { + t.Fatal("an indexable Additional address left the file") + } + + // The same set again: nothing changed, no rebuild requested. + if out := env.routeReportBody(t, "report-token", map[string]any{"routes": []string{"/"}, "noindex": []string{"/subscriptions"}}); out["noindex"] != float64(1) { + t.Fatalf("same set again = %v", out) + } + if env.pendingRebuild(t) { + t.Fatal("an unchanged noindex set requested a rebuild") + } + + // A report without the key keeps the set. + if out := env.routeReportBody(t, "report-token", map[string]any{"routes": []string{"/"}}); out["noindex"] != float64(1) { + t.Fatalf("report without noindex = %v, want the stored 1", out) + } + env.rebuildSitemap(t) + if listed() { + t.Fatal("a report without the noindex key cleared the set") + } + + // An empty list clears it, and the next rebuild lists the address again. + if out := env.routeReportBody(t, "report-token", map[string]any{"routes": []string{"/"}, "noindex": []string{}}); out["noindex"] != float64(0) { + t.Fatalf("empty noindex = %v", out) + } + if !env.pendingRebuild(t) { + t.Fatal("clearing the set requested no rebuild") + } + env.rebuildSitemap(t) + if !listed() || !inHTML() || extraReason() != "" { + t.Fatal("after the empty list the Additional address is not listed again") + } +} diff --git a/updates/202611040001_create_golem15_seo_noindex_paths.go b/updates/202611040001_create_golem15_seo_noindex_paths.go new file mode 100644 index 0000000..c17de9f --- /dev/null +++ b/updates/202611040001_create_golem15_seo_noindex_paths.go @@ -0,0 +1,28 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// createNoindexPaths creates golem15_seo_noindex_paths (P33-58): the static +// paths the frontend's route report names noindex, normalised like the +// reported routes. The sitemap filter leaves them out of the sitemaps. +var createNoindexPaths = []*gormigrate.Migration{ + { + ID: "202611040001_create_golem15_seo_noindex_paths", + Migrate: func(tx *gorm.DB) error { + return tx.Exec(`CREATE TABLE golem15_seo_noindex_paths ( + path TEXT PRIMARY KEY, + reported_at TIMESTAMP(0) NOT NULL +)`).Error + }, + Rollback: func(tx *gorm.DB) error { + return tx.Exec(`DROP TABLE IF EXISTS golem15_seo_noindex_paths`).Error + }, + }, +} + +func init() { + Register(createNoindexPaths...) +}