From 5820547947c339de377756c6041e27a5d19ea85a Mon Sep 17 00:00:00 2001 From: Joe Date: Thu, 6 Aug 2026 20:01:08 -0400 Subject: [PATCH] feat(plugin): named page types as the unit for per-template config + metrics; v0.34.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A route says WHETHER a path is prerendered. It could not say WHAT KIND of page it is, so everything template-shaped was keyed on the matched route's path — and a template reachable by two URL shapes became two unrelated things. Adds a top-level `pageTypes` list of named templates (`home`, `category`, `pdp`) that `ingress.routes` entries point at with `pageType`. Several routes may share one name; that is the point. metrics route_serve/route_page_age -> pagetype_serve/pagetype_age, labelled by template rather than route path. Two category routes now report as one series instead of two a reader had to know to add together. cost render_time gains the template in its previously-unused third dimension. Render cost is the fleet capacity input but was recorded as one undifferentiated distribution, so "PDPs are expensive to settle" was believable and not showable. It now joins delivered freshness on one key. cadence resolveRenderInterval precedence becomes route > pageType > stored > default. A cadence shared by several routes lives once, so the copies cannot drift with only the first match observed. job the claim payload carries the DECLARED name (never the label's fallback), so browser-side render rules can scope by template instead of restating the same URL shapes as regexes in the render fleet's own config. explain /prerender_admin reports the template, whether it is declared, and the cadence that follows from it. config a pageTypes entry no route references is an `info` finding — that state is a spelling mismatch whose only other symptom is silence. Adoption is incremental: with no `pageType` on any route the label falls back to the route path and then the route class, which is exactly what shipped before. Label cardinality stays bounded by construction — no arm derives a label from the request. Renaming route_serve/route_page_age is a clean break rather than a silent change of meaning; they shipped in v0.33.0 and no dashboard consumes them yet. --- package-lock.json | 2 +- packages/plugin/README.md | 78 +++++++- packages/plugin/package.json | 2 +- packages/plugin/src/admin/views/explain.js | 28 +++ packages/plugin/src/config.js | 15 +- packages/plugin/src/configSchema.js | 46 ++++- .../plugin/src/http_handlers/bot_request.js | 46 +++-- packages/plugin/src/resources/RenderQueue.js | 35 +++- packages/plugin/src/util/explain.js | 45 ++++- packages/plugin/src/util/ingress.js | 10 +- packages/plugin/src/util/routeClass.js | 180 +++++++++++++++-- packages/plugin/test/botServe.test.js | 62 +++--- packages/plugin/test/routeClass.test.js | 189 ++++++++++++++++++ 13 files changed, 655 insertions(+), 83 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9473882..86782cb 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9007,7 +9007,7 @@ }, "packages/plugin": { "name": "@harperfast/prerender", - "version": "0.33.0", + "version": "0.34.0", "license": "Apache-2.0", "dependencies": { "fast-xml-parser": "^5.0.9", diff --git a/packages/plugin/README.md b/packages/plugin/README.md index e112b8c..008693b 100644 --- a/packages/plugin/README.md +++ b/packages/plugin/README.md @@ -35,6 +35,9 @@ rest: true # required for the @export-ed table REST endpoints # --- options (all optional; defaults shown) --- domains: [] # indexable-host allowlist; empty = allow all hosts + # named templates; routes point at one with `pageType` — see "Page types (templates)" + pageTypes: [] # [{ name, renderInterval }] — several routes may share one name + ingress: # how incoming bot requests are parsed (see "Ingress modes" below) mode: prefix # 'prefix' (native /p/) or 'forwarded' (reverse proxy/CDN) botPathPrefix: /p/ # prefix mode: requests under this prefix are treated as bot requests @@ -44,7 +47,8 @@ rest: true # required for the @export-ed table REST endpoints forwardedProtoHeader: x-forwarded-proto defaultProtocol: https # ordered, first match wins — see "Route classes" - routes: [] # [{ match: exact|prefix|contains, path, mode: prerender|passthrough, queryParams: [...] }] + routes: [] # [{ match: exact|prefix|contains, path, mode: prerender|passthrough, + # queryParams: [...], pageType, renderInterval }] # compiled into `routes` as prepended passthrough entries; matched against the PATH excludePathPatterns: ['/search/'] # paths containing these are never auto-scheduled report: # periodic tally of paths served without prerendering @@ -131,11 +135,13 @@ rest: true # required for the @export-ed table REST endpoints enabled: true # record bot analytics at all: bot_request (ingress volume by host/bot/device), # bot_serve (outcome by source/cache-status/bot — origin offload + cache hit rate), # page_age (ms since the served page rendered — freshness at serve, cache-served only), - # route_serve (outcome by route/cache-status/device — per-route delivery, for tuning each - # route's renderInterval), and route_page_age (served age by route/cache-status/device). + # pagetype_serve (outcome by page-type/cache-status/device — per-template delivery, for + # tuning each template's renderInterval), and pagetype_age (served age by the same three). # cache-status distinguishes 'hit' (within the page's renderInterval) from 'swr' (served # from the stale-while-revalidate window because the re-render is late/in flight) — both # are cache serves; 'hit' alone is the "is the TTL being met" signal. + # render_time carries the page type as its third dimension, so render COST and delivered + # freshness join on one key — see "Page types (templates)". crawlStats: # crawl breadth: distinct URLs crawled per bot per UTC day (HyperLogLog, ~0.8% error) enabled: true # also gated by analytics.enabled above; read via GET /prerender_admin/crawl-breadth?days=7 @@ -237,6 +243,72 @@ forwarding `/blog/*`"); passthrough is the coverage backlog ("we proxy this much on purpose"). The tally is in-process, so **every worker** flushes its own line — each carries `node=` and `worker=`, and a reader sums across them. +### Page types (templates) + +A route says _whether_ a path is prerendered. A **page type** says _what kind of page it is_ — +the site's own vocabulary for its templates: `home`, `category`, `pdp`. It is the unit that +per-template settings and per-template metrics hang off. + +```yaml +pageTypes: + - { name: home, renderInterval: 7200000 } # 2h + - { name: category, renderInterval: 43200000 } # 12h + - { name: pdp, renderInterval: 172800000 } # 48h + +ingress: + routes: + - { match: exact, path: '/', pageType: home } + - { match: prefix, path: '/catalog/', pageType: category } + - { match: contains, path: '/category/', pageType: category } # same template, second URL shape + - { match: prefix, path: '/product/prd-', pageType: pdp } +``` + +**Several routes may share one name, and that is the point.** A template reachable by two URL +shapes is one template. Before page types, metrics were labelled with the matched route's _path_, +so those two category routes produced two unrelated series that only a reader holding the route +list could add back together — and a cadence set on both could silently drift, with only the +first-matching copy ever taking effect. + +Declaring a type is **optional**. A route may name a type that appears in no `pageTypes` entry; +it still labels metrics correctly. Declaring it is only needed to give the type settings, or to +share one setting across the routes that carry it. A type declared but referenced by no route is +reported as an `info` config finding — that state is almost always a spelling mismatch, and its +symptom otherwise is silence. + +**Cadence precedence** (resolved at schedule time, every cycle — no data migration): + +``` +route renderInterval > pageType renderInterval > target's stored interval > render.defaultInterval +``` + +The route level still wins so a single URL can carve itself out of its template's cadence (an +`exact` route ordered above the template's prefix) without inventing a one-member type. + +**What the name is used for:** + +| consumer | uses | +| --------------------------------- | ---------------------------------------------------------------------- | +| `pagetype_serve` / `pagetype_age` | delivery + freshness per template — the "should this TTL move" numbers | +| `render_time` | render **cost** per template — the fleet capacity input | +| `resolveRenderInterval` | one cadence for every route sharing the template | +| the queue job (`pageType`) | lets browser-side render rules scope by template name | +| `/prerender_admin` explain | the template, whether it is declared, and the cadence that follows | + +The job payload carries the **declared name only**, never the metrics fallback — a browser-side +rule scoped to a template must not fire on a route that was never declared to be one. That field +is what lets the render fleet stop re-describing the same URL shapes as regular expressions in its +own config: two pattern lists for one routing fact, in two repositories, with nothing keeping them +in step. + +**Adoption is incremental.** With no `pageType` on any route, the label falls back to the route's +path and then to the route class — exactly the values emitted before page types existed. Name one +route at a time; nothing resets. + +Metrics labels are bounded by construction: every label resolves to a configured name, a +configured path, or one of three class constants. Nothing derives a label from the request itself +— an unbounded label is how a monitoring backend gets taken down by a crawler walking a faceted +URL space. + ### Sitemaps are filtered to prerender routes A sitemap is written for search engines: it lists every indexable URL on the site, which is routinely diff --git a/packages/plugin/package.json b/packages/plugin/package.json index 53f44b7..6fe1c3c 100644 --- a/packages/plugin/package.json +++ b/packages/plugin/package.json @@ -1,6 +1,6 @@ { "name": "@harperfast/prerender", - "version": "0.33.0", + "version": "0.34.0", "type": "module", "description": "Configurable Harper plugin for prerendering pages for bots and crawlers", "license": "Apache-2.0", diff --git a/packages/plugin/src/admin/views/explain.js b/packages/plugin/src/admin/views/explain.js index 3de96ae..3c9f64b 100644 --- a/packages/plugin/src/admin/views/explain.js +++ b/packages/plugin/src/admin/views/explain.js @@ -119,6 +119,34 @@ function explanation(ctx, data) { muted(` ${data.ingress.route.source}`), ]), ], + data.eligibility.prerendered && [ + 'Page type', + el('span', null, [ + el('code', { text: data.pageType.metricLabel }), + muted( + data.pageType.name === null + ? ' no pageType on this route — reported under its path' + : data.pageType.declared + ? ' declared in pageTypes' + : ' named but not declared — metrics only, no settings' + ), + ]), + ], + data.eligibility.prerendered && [ + 'Render cadence', + el('span', null, [ + el('code', { text: duration(data.pageType.cadence.effective) }), + muted( + ` ${ + data.pageType.cadence.route !== null + ? 'from the matched route' + : data.pageType.cadence.pageType !== null + ? `from pageType ${data.pageType.name}` + : 'render.defaultInterval' + }${data.pageType.cadence.effectiveAssumesNoStoredInterval ? ', unless this target stores its own' : ''}` + ), + ]), + ], ]), verdictPills(data, page), ], diff --git a/packages/plugin/src/config.js b/packages/plugin/src/config.js index 0992ee9..fb44a0c 100644 --- a/packages/plugin/src/config.js +++ b/packages/plugin/src/config.js @@ -35,7 +35,7 @@ import { // evaluation time. The count has to come from the compiler rather than from raw config, // because the finding's whole job is to catch entries the compiler REJECTED (a typo'd // `match`), which the raw array still contains. -import { prerenderRouteCount } from './util/routeClass.js'; +import { declaredPageTypes, prerenderRouteCount, routePageTypes } from './util/routeClass.js'; // Returns the Harper logger when running inside Harper, otherwise the console. // Unit tests run outside Harper where `logger` is undefined. @@ -334,6 +334,19 @@ export const collectConfigWarnings = () => { 'check ingress.routes for entries dropped as invalid' ); } + // A `pageTypes` entry nothing points at is almost always a typo on one side of the join, and + // its symptom is silence: the type's settings simply never apply and its name never appears + // in metrics, which reads exactly like "this template gets no traffic". Info, not warn — the + // same list is legitimately shared across deployments whose route sets differ. + const unusedPageTypes = declaredPageTypes().filter((name) => !routePageTypes().has(name)); + if (unusedPageTypes.length > 0) { + add( + 'info', + 'pageTypes', + `pageTypes declared but not referenced by any route: ${unusedPageTypes.join(', ')} — their settings ` + + 'will never apply; check for a spelling mismatch with ingress.routes[].pageType' + ); + } const { staging } = config.origin; if (staging.ip) { // Mirror stagingTargetIp's gate (ip AND header AND valid ip) so the finding never diff --git a/packages/plugin/src/configSchema.js b/packages/plugin/src/configSchema.js index 4b4c7ee..46be311 100644 --- a/packages/plugin/src/configSchema.js +++ b/packages/plugin/src/configSchema.js @@ -62,6 +62,30 @@ export const configSchema = group('Prerender plugin configuration.', { { itemType: 'string' } ), + pageTypes: option( + [], + 'Named page types (templates) — the site’s own vocabulary for the kinds of page it serves: ' + + '`home`, `category`, `pdp`. Each entry is { name: string, renderInterval?: number }.\n\n' + + 'A page type is the unit that per-template settings and per-template METRICS hang off. ' + + '`ingress.routes` entries point at one with `pageType: `, and SEVERAL routes may share ' + + 'the same name — which is the point: a site whose category pages are reachable by two ' + + 'different URL shapes gets ONE set of numbers for “category” instead of two unrelated rows ' + + 'that a reader has to know to add together.\n\n' + + 'Declaring a type here is optional. It is only required to (a) set a value once for several ' + + 'routes, or (b) give the type a name in metrics; a route may name a type that is not declared ' + + 'and it still labels metrics correctly — an undeclared name is not an error, just a type with ' + + 'no settings of its own.\n\n' + + '`renderInterval` (ms) is the render cadence for every URL of this type. Full precedence, ' + + 'resolved at schedule time on each cycle: route `renderInterval` > this > the target’s stored ' + + 'interval (sitemap `changefreq` / explicit API write) > `render.defaultInterval`. The route ' + + 'level still wins so a single URL (an `exact` route) can carve itself out of its type’s ' + + 'cadence without inventing a one-member type.\n\n' + + 'Names travel: the matched type is sent to the renderer on each queue job, so browser-side ' + + 'render rules can be scoped by template name instead of re-describing the same URL shapes as ' + + 'regular expressions in a second repository.', + { itemType: 'object' } + ), + ingress: group( 'Request-ingestion model: how incoming bot requests are recognized, which paths are ' + 'prerendered, and how the target URL and device type are derived.\n\n' + @@ -97,7 +121,7 @@ export const configSchema = group('Prerender plugin configuration.', { [], 'Ordered route list (forwarded mode). Each entry is ' + "{ match: 'exact' | 'prefix' | 'contains', path: string, mode?: 'prerender' | 'passthrough', " + - 'queryParams?: string[], renderInterval?: number }.\n\n' + + 'queryParams?: string[], pageType?: string, renderInterval?: number }.\n\n' + 'FIRST MATCH WINS, so order most-specific first. That ordering is what lets a passthrough ' + 'carve-out sit inside a prerendered prefix (`/products/clearance/` above `/products/`) ' + 'without a second list and a precedence rule.\n\n' + @@ -111,12 +135,17 @@ export const configSchema = group('Prerender plugin configuration.', { 'off the proxied origin fetch and hand the visitor the wrong page.\n\n' + "A path matching NOTHING is 'unclassified': still proxied (never blocked), never cached, and " + 'counted for reporting so the gap can be fixed at the CDN or here.\n\n' + + '`pageType` (prerender routes only) names the template this route serves — see the top-level ' + + '`pageTypes`. It is what per-template settings and metrics key on, and SEVERAL routes may ' + + 'carry the same name so that one template reachable by two URL shapes reports as one thing. ' + + 'Omitted, the route reports under its own `path`, which is the pre-`pageTypes` behavior.\n\n' + '`renderInterval` (ms, prerender routes only) sets the render cadence for every URL the route ' + - "matches. Precedence: route > the target's stored interval (sitemap `` or an " + - 'explicit API write) > `render.defaultInterval` — resolved at schedule time on every cycle, so ' + - "changing it here takes effect on each URL's next render with no data migration. A per-URL " + - 'exception is an `exact` route ordered above its class (e.g. the homepage `exact /` at 2h above ' + - 'a 6h section prefix); a route that should defer to sitemap changefreq simply doesn’t set one.\n\n' + + "matches. Precedence: route > the route's `pageType` > the target's stored interval (sitemap " + + '`` or an explicit API write) > `render.defaultInterval` — resolved at schedule time ' + + "on every cycle, so changing it here takes effect on each URL's next render with no data " + + 'migration. Prefer setting a cadence on the `pageType` when several routes share it; keep it ' + + 'here for a per-URL exception (e.g. the homepage `exact /` at 2h above a 6h section prefix). A ' + + 'route that should defer to sitemap changefreq simply doesn’t set one.\n\n' + "OPERATIONAL NOTE: if the CDN edge-caches a route's responses with a fixed TTL from its own " + "property settings (not from our response headers), that TTL and the route's renderInterval " + 'must be kept aligned BY HAND — rendering much faster than the edge TTL burns renders the edge ' + @@ -391,8 +420,9 @@ export const configSchema = group('Prerender plugin configuration.', { 'How often a target is re-rendered when nothing more specific applies. Cadence is relative to ' + 'each render’s completion (not a fixed time-of-day), and a target’s first render is jittered ' + 'across its interval — so the fleet renders as a smooth stream rather than a daily herd. Full ' + - 'precedence, resolved at schedule time: matched route `renderInterval` (ingress.routes) > the ' + - 'target’s stored interval (sitemap `changefreq` / explicit API write) > this default.', + 'precedence, resolved at schedule time: matched route `renderInterval` (ingress.routes) > that ' + + 'route’s `pageType` `renderInterval` (top-level `pageTypes`) > the target’s stored interval ' + + '(sitemap `changefreq` / explicit API write) > this default.', { unit: 'ms', min: 1 } ), suppression: group( diff --git a/packages/plugin/src/http_handlers/bot_request.js b/packages/plugin/src/http_handlers/bot_request.js index f0e5f14..e2d4016 100644 --- a/packages/plugin/src/http_handlers/bot_request.js +++ b/packages/plugin/src/http_handlers/bot_request.js @@ -25,7 +25,7 @@ export async function handleBotRequest(request) { if (!target) { return { headers: {}, status: 400 }; } - const { url, cacheUrl, deviceType, routeClass, route } = target; + const { url, cacheUrl, deviceType, routeClass, pageType, pageTypeLabel, route } = target; request.botName = getBotName(request.headers); const recordBots = config.analytics.enabled && (request.botName !== 'other' || config.analytics.recordUnmatched); @@ -38,8 +38,11 @@ export async function handleBotRequest(request) { // Debug/observability info surfaced as x-harper-* response headers (only when the // debug header is present). `route` is the matched route entry, if any; `routeClass` - // decides whether this request is cached and scheduled at all. - const info = { route, routeClass }; + // decides whether this request is cached and scheduled at all. `pageType` is the declared + // template name or null; `pageTypeLabel` is the always-present metrics label for it. Both + // come from the one classification the ingress already did, so the debug headers and the + // serve metrics can never disagree about which template answered a request. + const info = { route, routeClass, pageType, pageTypeLabel }; const resource = await resolveResource({ request, url, cacheUrl, deviceType, routeClass, info }); maybeSchedule(resource, routeClass); @@ -69,19 +72,24 @@ export async function handleBotRequest(request) { // freshness = page_age, ms since the served page rendered (cache-served only, so a // render-now response doesn't drag the distribution toward zero) // -// Per-route variants of the same two signals, for tuning each route's renderInterval up or -// down independently (recordAnalytics has exactly three dimension slots — path/method/type — +// Per-PAGE-TYPE variants of the same two signals, for tuning each template's renderInterval up +// or down independently (recordAnalytics has exactly three dimension slots — path/method/type — // and bot_serve's are all taken, hence separate metrics rather than a fourth dimension): // -// route_serve = (route, cacheStatus, deviceType) counter. swr/stale share per route -// says whether that route's cadence is being DELIVERED; miss share says -// whether its corpus is even covered. -// route_page_age = (route, cacheStatus, deviceType), ms since render, cache-served only. -// Served age per route against that route's own renderInterval is the -// "should this TTL move" number. +// pagetype_serve = (pageType, cacheStatus, deviceType) counter. swr/stale share per +// template says whether that template's cadence is being DELIVERED; miss +// share says whether its corpus is even covered. +// pagetype_age = (pageType, cacheStatus, deviceType), ms since render, cache-served only. +// Served age per template against that template's own renderInterval is +// the "should this TTL move" number. // -// The route label is the matched route's path ('/', '/catalog/', '/product/prd-' — tiny, -// stable cardinality), else the route class for passthrough, else 'unrouted'. +// These replace v0.33.0's `route_serve` / `route_page_age`, which labelled by the matched +// route's PATH. That split one template across every URL shape that reaches it — a site with +// two category routes got two unrelated rows that only a reader holding the route list could +// add back together — and it is the same label the render-side cost metric needs, so the two +// halves of "is this template worth what it costs" could not be joined. `pageTypeLabel` still +// falls back to the route path when no type is declared, so an un-migrated deployment emits the +// same label values under the new metric names. // // Cost: two counter bumps per request plus two numeric samples on a cache hit // (recordAnalytics buffers in a Map and flushes on Harper's analytics timer) — no storage @@ -89,9 +97,9 @@ export async function handleBotRequest(request) { // // Exported for tests: the dimension ORDER is the contract dashboards key on. export function recordServeOutcome(resource, request, info, deviceType) { - const route = info.route?.path ?? info.routeClass ?? 'unrouted'; + const pageType = info.pageTypeLabel; server.recordAnalytics(true, 'bot_serve', info.source, info.cacheStatus, request.botName); - server.recordAnalytics(true, 'route_serve', route, info.cacheStatus, deviceType); + server.recordAnalytics(true, 'pagetype_serve', pageType, info.cacheStatus, deviceType); if (info.source === 'cache' && resource.lastCached) { // lastCached is a schema Date — guard truthiness FIRST, then coerce, exactly like the // expiresAt read above: `new Date(null)` is epoch 0 (not NaN), so an unguarded null @@ -102,7 +110,7 @@ export function recordServeOutcome(resource, request, info, deviceType) { const age = Date.now() - new Date(resource.lastCached).getTime(); if (age >= 0) { server.recordAnalytics(age, 'page_age', request.botName, deviceType); - server.recordAnalytics(age, 'route_page_age', route, info.cacheStatus, deviceType); + server.recordAnalytics(age, 'pagetype_age', pageType, info.cacheStatus, deviceType); } } } @@ -120,6 +128,8 @@ function resolveBotTarget(request) { cacheUrl: target.cacheUrl, deviceType: target.deviceType, routeClass: target.routeClass, + pageType: target.pageType, + pageTypeLabel: target.pageTypeLabel, route: target.route, }; } @@ -129,12 +139,14 @@ function resolveBotTarget(request) { // this mode — but the allowlist stays the global `url.queryParams`, so the key is // unchanged. canonicalizeUrl has already proved the URL parses by the time we classify. const cacheUrl = canonicalizeUrl(request.url.slice(config.ingress.botPathPrefix.length), config.cacheKey.queryParams); - const { routeClass, entry } = classifyPath(URL.parse(cacheUrl)?.pathname ?? '/'); + const { routeClass, pageType, pageTypeLabel, entry } = classifyPath(URL.parse(cacheUrl)?.pathname ?? '/'); return { url: new URL(cacheUrl), cacheUrl, deviceType: sanitizeDeviceType(request.headers.get(config.ingress.deviceTypeHeader)), routeClass, + pageType, + pageTypeLabel, route: entry, }; } diff --git a/packages/plugin/src/resources/RenderQueue.js b/packages/plugin/src/resources/RenderQueue.js index 044819e..aad9118 100644 --- a/packages/plugin/src/resources/RenderQueue.js +++ b/packages/plugin/src/resources/RenderQueue.js @@ -4,7 +4,7 @@ import { currentMinuteMs } from '../util/time.js'; import { QueueState } from './QueueState.js'; import { CacheKey } from '../util/cacheKey.js'; import { canonicalizeUrl } from '../util/url.js'; -import { classifyPath, queryAllowlistFor, resolveRenderInterval, PRERENDER } from '../util/routeClass.js'; +import { classifyPath, classifyUrl, queryAllowlistFor, resolveRenderInterval, PRERENDER } from '../util/routeClass.js'; import { recordUnroutedPath } from '../util/unrouted.js'; import { Target, countedStrikes } from './Target.js'; import { getDesiredPause, setDesiredPause } from '../util/queueControl.js'; @@ -225,6 +225,14 @@ export class RenderQueue extends Resource { const hasContent = result.statusCode === 200 && result.content; if (typeof result.renderTime === 'number') { + // The third dimension was unused until page types existed, and filling it with the + // template is what makes render COST attributable. Render time is the fleet's + // capacity input — demand is the sum over templates of (corpus / interval) — but + // until now it was recorded as one undifferentiated distribution, so "product pages + // are expensive to settle" was a thing you could believe and not a thing you could + // show. Sharing the label with `pagetype_serve`/`pagetype_age` is the point: cost + // per template and delivered freshness per template finally join on one key, which + // is the whole argument for moving a template's interval. server.recordAnalytics( result.renderTime, 'render_time', @@ -233,7 +241,11 @@ export class RenderQueue extends Resource { ? result.isIndexable || hasContent ? 'candidate' : 'non-candidate' - : 'unknown' + : 'unknown', + // `result.id`, NOT `cacheKey`: the latter may already have been re-pointed at a + // redirect destination above, which would bill this render to the template it + // landed on instead of the one that was scheduled and actually paid for. + classifyUrl(CacheKey.extractUrl(result.id)).pageTypeLabel ); } @@ -359,7 +371,17 @@ export class RenderQueue extends Resource { */ static async processRedirectResult(result, { redirectKey, landedOn, redirectPath, inspectedNonIndexable }) { if (typeof result.renderTime === 'number') { - server.recordAnalytics(result.renderTime, 'render_time', result.statusCode, 'redirect'); + // Same template attribution as the main path: a render that ended in a redirect still + // consumed a slot, and the template that keeps paying for redirects is the one worth + // finding. Billed to the SOURCE (`result.id`) — the job that was scheduled — not to + // wherever it landed. + server.recordAnalytics( + result.renderTime, + 'render_time', + result.statusCode, + 'redirect', + classifyUrl(CacheKey.extractUrl(result.id)).pageTypeLabel + ); } // Same status rules as processJobResult, applied BEFORE anything retires or strikes @@ -629,6 +651,13 @@ export class RenderQueue extends Resource { expiresAt, callbackOrigin: `${protocol}://${server.hostname}:${port}`, isFromSitemap: !!schedule.fromSitemap, + // The template name (or null), so browser-side render rules can be scoped by + // template instead of re-describing the same URL shapes as regular expressions in + // the render fleet's own config — two pattern lists for one routing fact, in two + // repositories, with nothing keeping them in step. Sent as the DECLARED name only: + // the metrics label's route-path fallback would make a rule scoped to a template + // fire on routes never declared to be one. Older browsers ignore the field. + pageType: classifyUrl(url).pageType, }); } diff --git a/packages/plugin/src/util/explain.js b/packages/plugin/src/util/explain.js index 8d4fcbb..4b2be1b 100644 --- a/packages/plugin/src/util/explain.js +++ b/packages/plugin/src/util/explain.js @@ -19,7 +19,7 @@ import { config } from '../config.js'; import { CacheKey } from './cacheKey.js'; import { canonicalizeUrl } from './url.js'; -import { classifyPath, isForwardedMode, PRERENDER } from './routeClass.js'; +import { classifyPath, isForwardedMode, pageTypeSettings, PRERENDER } from './routeClass.js'; import { sanitizeDeviceType } from './device_type.js'; // `source` is included deliberately: an entry folded in from `excludePathPatterns` looks @@ -27,7 +27,15 @@ import { sanitizeDeviceType } from './device_type.js'; // this" is the difference between two very different fixes. const summarizeRoute = (route) => route - ? { match: route.match, path: route.path, mode: route.mode, queryParams: route.queryParams, source: route.source } + ? { + match: route.match, + path: route.path, + mode: route.mode, + queryParams: route.queryParams, + pageType: route.pageType, + renderInterval: route.renderInterval, + source: route.source, + } : null; /** @@ -44,7 +52,8 @@ export const explainCacheKey = (rawUrl, requestedDeviceType) => { // The same classifier the read path uses, so this can never explain a key the serving path // wouldn't actually compute. - const { routeClass, queryParams: allowlist, entry: route } = classifyPath(url.pathname); + const { routeClass, pageType, pageTypeLabel, queryParams: allowlist, entry: route } = classifyPath(url.pathname); + const typeSettings = pageTypeSettings(pageType); const allowlistSource = !forwarded ? 'url.queryParams' : routeClass === PRERENDER @@ -84,6 +93,36 @@ export const explainCacheKey = (rawUrl, requestedDeviceType) => { routeClass, route: summarizeRoute(route), }, + // Which template this URL belongs to, and the cadence that follows from it. Answers the + // two questions the route block alone can't: what will this URL be REPORTED as, and why + // is it re-rendering at the rate it is. + pageType: { + // The declared name (null when the route names none) versus the label metrics + // actually carry — which falls back to the route path, then the class. Showing both + // is the point: a `null` name beside a `/catalog/` label is precisely the state where + // two routes are reporting separately and an operator meant them to be one template. + name: pageType, + metricLabel: pageTypeLabel, + declared: typeSettings !== null, + // Cadence precedence, resolved as far as config can see it. The target's STORED + // interval (sitemap changefreq / an API write) sits between `pageType` and `default` + // and is per-URL data, so it isn't knowable here — hence `effective` is the + // config-derived answer and is marked as assuming no stored value. + cadence: (() => { + const fromRoute = route?.renderInterval ?? null; + const fromType = typeSettings?.renderInterval ?? null; + const configured = fromRoute ?? fromType; + return { + route: fromRoute, + pageType: fromType, + default: config.render.defaultInterval, + effective: configured ?? config.render.defaultInterval, + // Nothing in config pins this URL's cadence, so a stored interval on the target + // would win over the default shown above. + effectiveAssumesNoStoredInterval: configured === null, + }; + })(), + }, allowlist: { used: allowlist, source: allowlistSource }, underGlobalAllowlist: { allowlist: config.cacheKey.queryParams, diff --git a/packages/plugin/src/util/ingress.js b/packages/plugin/src/util/ingress.js index a44a4a7..dc26018 100644 --- a/packages/plugin/src/util/ingress.js +++ b/packages/plugin/src/util/ingress.js @@ -28,12 +28,14 @@ const firstHeaderValue = (raw) => (raw ? raw.split(',')[0].trim() : ''); /** * Resolve a forwarded request into its prerender target: - * `{ url: URL, cacheUrl, deviceType, route, routeClass }`, or `null` when the request should - * be skipped entirely. Never throws. + * `{ url: URL, cacheUrl, deviceType, route, routeClass, pageType, pageTypeLabel }`, or `null` + * when the request should be skipped entirely. Never throws. * * `routeClass` is the single source of truth for how the handler treats this request — there * is deliberately no separate `noCache` flag that could fall out of step with it. Only * `prerender` is cached and scheduled. `route` is the matched compiled entry, or null. + * `pageType`/`pageTypeLabel` are that route's template name and its metrics label; both are + * carried from the single `classifyPath` call below so nothing downstream re-derives them. * * Skipped (`null`) means: no device-type prefix in path mode, an unusable forwarded host, or * an unclassified path in HEADER mode. That last case is a mode asymmetry worth stating. In @@ -63,7 +65,7 @@ export const resolveForwardedRequest = (request) => { path = rawPath; } - const { routeClass, queryParams, entry } = classifyPath(path); + const { routeClass, pageType, pageTypeLabel, queryParams, entry } = classifyPath(path); // See the header-mode asymmetry above. if (routeClass === UNCLASSIFIED && !fromPath) return null; @@ -91,7 +93,7 @@ export const resolveForwardedRequest = (request) => { // see util/unrouted.js for why. if (routeClass !== PRERENDER) recordUnroutedPath(routeClass, path, 'cdn'); - return { url: new URL(cacheUrl), cacheUrl, deviceType, route: entry, routeClass }; + return { url: new URL(cacheUrl), cacheUrl, deviceType, route: entry, routeClass, pageType, pageTypeLabel }; } catch (e) { // `e?.message ?? String(e)` rather than `e.message`: anything can be thrown, and a // non-Error rejection must not turn a skipped request into a TypeError in the logger. diff --git a/packages/plugin/src/util/routeClass.js b/packages/plugin/src/util/routeClass.js index 342d20b..b646581 100644 --- a/packages/plugin/src/util/routeClass.js +++ b/packages/plugin/src/util/routeClass.js @@ -1,5 +1,6 @@ /** - * Route classification — the single answer to "do we prerender this path?" + * Route classification — the single answer to "do we prerender this path?" and "what KIND of + * page is this?" * * Every path the plugin sees resolves to exactly one of three classes: * @@ -30,6 +31,22 @@ * allowlist could still do there is silently strip params from the proxied request and hand * the visitor the wrong page — with no cached entry and no `x-harper-cache-key` to explain * it. So an allowlist on a passthrough entry is rejected at compile time, not honored. + * + * PAGE TYPES (templates) are the SECOND thing a path resolves to, and the reason they live here + * rather than beside the metrics that consume them: a page type is a property of the ROUTE, and + * the route match is already computed on every request. A type is a name — `home`, `category`, + * `pdp` — that several routes may share, which is the whole point. A site whose category pages + * are reachable by two URL shapes has ONE category template and wants one set of numbers for + * it; labelling metrics by the matched route's PATH (what this module used to expose) split that + * template into two unrelated rows only a reader who knew the route list could add back + * together. Types also give per-template settings a single home, so two routes sharing a + * template cannot drift apart on cadence, and they travel to the renderer on the queue job so + * browser-side rules can be scoped by template name instead of by a second, independently + * maintained set of URL patterns in another repository. + * + * The type is deliberately NOT derived from the path (first segment, a regex, a heuristic). + * Those all re-encode routing knowledge the route list already holds, and they drift from it + * silently. Declaring `pageType` on the route keeps one list authoritative. */ import { config, getLogger } from '../config.js'; @@ -102,7 +119,66 @@ const compileEntry = (raw, source, warn) => { } } - return { match: raw.match, path: raw.path, mode, queryParams, renderInterval, source }; + // Optional template name. Like renderInterval, a bad value drops the FIELD, never the route: + // losing a name costs a metrics label, while dropping the entry would change how the path is + // SERVED. An unknown name is NOT rejected — `pageTypes` only has to declare a type that + // carries settings, so requiring a declaration here would make naming a type for metrics + // alone impossible. + let pageType = null; + if (raw.pageType !== undefined && raw.pageType !== null) { + if (mode === PASSTHROUGH) { + warn( + `ignoring pageType on passthrough route "${raw.match} ${raw.path}" — a passthrough route is never ` + + `rendered or cached, so it has no template to configure or report on` + ); + } else if (typeof raw.pageType === 'string' && raw.pageType !== '') { + pageType = raw.pageType; + } else { + warn( + `ignoring pageType on route "${raw.match} ${raw.path}" — expected a non-empty string, got ` + + `${String(raw.pageType)}` + ); + } + } + + return { match: raw.match, path: raw.path, mode, queryParams, pageType, renderInterval, source }; +}; + +/** + * Compile the top-level `pageTypes` list into a name → settings map. + * + * Last declaration of a duplicated name wins, and says so. Silently keeping the first would + * leave an operator staring at a cadence that plainly does not match the config they are + * reading. + */ +const compilePageTypes = (pageTypes) => { + const log = getLogger(); + const byName = new Map(); + + for (const raw of Array.isArray(pageTypes) ? pageTypes : []) { + if (!raw || typeof raw.name !== 'string' || raw.name === '') continue; + + let renderInterval = null; + if (raw.renderInterval !== undefined && raw.renderInterval !== null) { + if (Number.isFinite(raw.renderInterval) && raw.renderInterval > 0) { + renderInterval = raw.renderInterval; + } else { + // String(), never JSON.stringify() — the latter throws on a BigInt, and config + // compilation must not be crashable from a warning path. + log.warn?.( + `[prerender] ignoring renderInterval on pageType "${raw.name}" — expected a positive number of ` + + `milliseconds, got ${String(raw.renderInterval)}` + ); + } + } + + if (byName.has(raw.name)) { + log.warn?.(`[prerender] pageType "${raw.name}" is declared more than once — using the last declaration`); + } + byName.set(raw.name, { name: raw.name, renderInterval }); + } + + return byName; }; /** @@ -167,6 +243,32 @@ const getRoutes = () => { return compiled; }; +// Same compile-and-memoize treatment for the page-type table, tracked independently: a +// `pageTypes` edit must not force the route list to recompile, and vice versa. +let compiledTypes = null; +let compiledFromPageTypes; + +const getPageTypes = () => { + if (config.pageTypes !== compiledFromPageTypes) { + compiledTypes = compilePageTypes(config.pageTypes); + compiledFromPageTypes = config.pageTypes; + } + return compiledTypes; +}; + +/** Declared settings for a page-type name, or null when the name carries none. */ +export const pageTypeSettings = (name) => (name ? (getPageTypes().get(name) ?? null) : null); + +/** Every declared page-type name, for config reporting and the admin UI. */ +export const declaredPageTypes = () => [...getPageTypes().keys()]; + +/** Every page-type name actually referenced by a compiled route. */ +export const routePageTypes = () => { + const names = new Set(); + for (const entry of getRoutes()) if (entry.pageType) names.add(entry.pageType); + return names; +}; + /** * First matching compiled entry for `path`, or null. First match wins, so entries should be * ordered most-specific first — which is what lets a passthrough carve-out sit inside a @@ -193,12 +295,49 @@ export const prerenderRouteCount = () => { }; /** - * Classify a device-stripped path into `{ routeClass, queryParams, entry }`. + * Build the classification result every `classify*` returns, including the metrics label. + * + * The label is computed HERE rather than offered as a `pageTypeLabel(x)` helper callers apply + * themselves. Every caller that labels a metric already holds a classification, and a helper + * would have to agree with each of them about what its argument is called — the read path calls + * the matched route `route`, this module calls it `entry` — which is exactly the kind of drift + * this module exists to prevent. It is a `??` chain over values already in hand: no allocation, + * nothing worth making lazy. + * + * WHY THE FALLBACK CHAIN (name → route path → class). Every request must produce a label or the + * metric develops holes that read as traffic disappearing. Falling back to the route path + * (rather than one 'other' bucket) means a deployment that declares no `pageTypes` emits exactly + * the label values it emitted before types existed — so adoption is incremental, one route at a + * time, instead of a flag day that resets every dashboard. + * + * CARDINALITY is bounded by construction and must stay that way: every arm resolves to a + * configured name, a configured path, or one of three class constants. Nothing here may ever + * derive a label from the REQUEST (its path, query, or headers) — an unbounded metrics label is + * how a monitoring backend gets taken down by a crawler walking a faceted URL space. + */ +const classification = (routeClass, pageType, queryParams, entry) => ({ + routeClass, + pageType, + pageTypeLabel: pageType ?? entry?.path ?? routeClass, + queryParams, + entry, +}); + +/** + * Classify a device-stripped path into + * `{ routeClass, pageType, pageTypeLabel, queryParams, entry }`. * * `queryParams` is the allowlist to canonicalize this path's URL with; `entry` is the * matched compiled route (null when nothing matched) and carries `source`, so a caller can * report WHERE a classification came from. * + * `pageType` is the declared template name, or null — never a fallback — while `pageTypeLabel` + * is always a string. The two are separate on purpose. A caller labelling a metric needs a + * value for every request, so it takes the label. A caller telling the renderer which template + * it is about to render (the queue job) must send the real name or nothing: were it to send the + * label's fallback, a browser-side rule scoped to a template would fire on a route that was + * never declared to be one. + * * The field is `routeClass`, not `class`: `class` is a reserved word, so a caller could not * destructure it without renaming at every call site. * @@ -212,16 +351,16 @@ export const classifyPath = (path) => { const entry = matchRoute(path); if (!isForwardedMode()) { - return { - routeClass: entry && entry.mode === PASSTHROUGH ? PASSTHROUGH : PRERENDER, - queryParams: config.cacheKey.queryParams, - entry: entry ?? null, - }; + const routeClass = entry && entry.mode === PASSTHROUGH ? PASSTHROUGH : PRERENDER; + // Prefix mode reaches here with `entry` set only for a folded exclude (always + // passthrough), so a named type can only ever come from a real prerender route. + const pageType = routeClass === PRERENDER ? (entry?.pageType ?? null) : null; + return classification(routeClass, pageType, config.cacheKey.queryParams, entry ?? null); } - if (!entry) return { routeClass: UNCLASSIFIED, queryParams: KEEP_ALL, entry: null }; - if (entry.mode === PASSTHROUGH) return { routeClass: PASSTHROUGH, queryParams: KEEP_ALL, entry }; - return { routeClass: PRERENDER, queryParams: entry.queryParams, entry }; + if (!entry) return classification(UNCLASSIFIED, null, KEEP_ALL, null); + if (entry.mode === PASSTHROUGH) return classification(PASSTHROUGH, null, KEEP_ALL, entry); + return classification(PRERENDER, entry.pageType, entry.queryParams, entry); }; /** @@ -239,11 +378,7 @@ export const classifyUrl = (rawUrl) => { if (pathname === undefined) { // Unparseable, so unclassifiable. Keep every param exactly as an unmatched path does — // any caller that goes on to build a key from this URL fails on the URL itself first. - return { - routeClass: UNCLASSIFIED, - queryParams: isForwardedMode() ? KEEP_ALL : config.cacheKey.queryParams, - entry: null, - }; + return classification(UNCLASSIFIED, null, isForwardedMode() ? KEEP_ALL : config.cacheKey.queryParams, null); } return classifyPath(pathname); }; @@ -260,8 +395,13 @@ export const queryAllowlistFor = (rawUrl) => classifyUrl(rawUrl).queryParams; /** * The render cadence for a URL: the matched route's `renderInterval` when it sets one, else - * the target's stored interval (sitemap `` or an explicit API write), else - * `render.defaultInterval`. + * that route's `pageType` cadence, else the target's stored interval (sitemap `` or + * an explicit API write), else `render.defaultInterval`. + * + * ROUTE BEATS ITS PAGE TYPE so a single URL can carve itself out of its template's cadence + * (an `exact` route above the template's prefix) without inventing a one-member type. The + * template level is where a cadence shared by several routes belongs — set on each route + * instead, the two copies drift and only the first-matching one is ever observed. * * ROUTE BEATS STORED, deliberately. The stored interval is data written at creation time; * if it won, changing a route's cadence would apply only to targets discovered AFTER the @@ -276,8 +416,10 @@ export const queryAllowlistFor = (rawUrl) => classifyUrl(rawUrl).queryParams; * / the url-half of a cacheKey). */ export const resolveRenderInterval = (url, storedInterval) => { - const { entry } = classifyUrl(url); + const { pageType, entry } = classifyUrl(url); if (entry && entry.renderInterval !== null && entry.renderInterval !== undefined) return entry.renderInterval; + const typeInterval = pageTypeSettings(pageType)?.renderInterval; + if (typeInterval !== null && typeInterval !== undefined) return typeInterval; // Coerce before the finite check: `Long` columns can surface the stored interval as a // BigInt, which `Number.isFinite` rejects outright — without this, every such target // would silently fall back to the default cadence. `Number(null)` is 0 and diff --git a/packages/plugin/test/botServe.test.js b/packages/plugin/test/botServe.test.js index 7bb53b6..1bdb46c 100644 --- a/packages/plugin/test/botServe.test.js +++ b/packages/plugin/test/botServe.test.js @@ -8,10 +8,12 @@ import assert from 'node:assert/strict'; * - `bot_serve` dimension ORDER is (source, cacheStatus, botName). Dashboards key on the * positional path/method/type triple Harper builds from these, so reordering is a silent * breaking change. - * - `route_serve` is (route, cacheStatus, deviceType) and `route_page_age` mirrors it — - * the per-route TTL-tuning signals. Same positional contract. - * - The route label resolves route.path, then routeClass, then 'unrouted' — in that order. - * - `page_age`/`route_page_age` are recorded ONLY for a cache-served resource + * - `pagetype_serve` is (pageType, cacheStatus, deviceType) and `pagetype_age` mirrors it — + * the per-template TTL-tuning signals. Same positional contract. + * - The label is taken from `info.pageTypeLabel` VERBATIM. Resolving it (declared name → + * route path → route class) is the classifier's job and is pinned in routeClass.test.js; + * duplicating that chain here would let the two drift while both suites stayed green. + * - `page_age`/`pagetype_age` are recorded ONLY for a cache-served resource * (source === 'cache'), so render-now responses never drag the freshness distribution * toward zero. * - lastCached may arrive as a Date, a number, or a serialized string — all must yield the @@ -58,22 +60,36 @@ beforeEach(() => { const request = { botName: 'Googlebot' }; -test('bot_serve records (source, cacheStatus, botName) and route_serve records (route, cacheStatus, deviceType)', () => { - recordServeOutcome({}, request, { source: 'origin', cacheStatus: 'miss', route: { path: '/catalog/' } }, 'desktop'); +test('bot_serve records (source, cacheStatus, botName) and pagetype_serve records (pageType, cacheStatus, deviceType)', () => { + recordServeOutcome({}, request, { source: 'origin', cacheStatus: 'miss', pageTypeLabel: 'category' }, 'desktop'); assert.deepEqual(analytics, [ [true, 'bot_serve', 'origin', 'miss', 'Googlebot'], - [true, 'route_serve', '/catalog/', 'miss', 'desktop'], + [true, 'pagetype_serve', 'category', 'miss', 'desktop'], ]); }); -test('route label falls back route.path -> routeClass -> unrouted', () => { - recordServeOutcome({}, request, { source: 'origin', cacheStatus: 'miss', routeClass: 'passthrough' }, 'desktop'); - recordServeOutcome({}, request, { source: 'origin', cacheStatus: 'miss' }, 'desktop'); - assert.equal(analytics[1][2], 'passthrough'); - assert.equal(analytics[3][2], 'unrouted'); +test('the page-type label is emitted verbatim, whatever the classifier resolved it to', () => { + // A declared name, the route-path fallback, and the route-class fallback all reach this + // function the same way — as an already-resolved string. This asserts only the pass-through; + // which of the three a given request yields is routeClass.test.js's contract. + for (const label of ['pdp', '/catalog/', 'passthrough']) { + analytics = []; + recordServeOutcome({}, request, { source: 'origin', cacheStatus: 'miss', pageTypeLabel: label }, 'desktop'); + assert.equal(analytics[1][2], label); + } +}); + +test('several routes sharing one page type report under a single label', () => { + // The reason page types exist: two category routes, one row of numbers. Were the label still + // the matched route's path, these two requests would land on unrelated series. + for (let i = 0; i < 2; i++) { + recordServeOutcome({}, request, { source: 'cache', cacheStatus: 'hit', pageTypeLabel: 'category' }, 'desktop'); + } + const labels = analytics.filter(([, metric]) => metric === 'pagetype_serve').map(([, , label]) => label); + assert.deepEqual(labels, ['category', 'category']); }); -test('a cache serve also records page_age (botName, deviceType) and route_page_age (route, cacheStatus, deviceType)', () => { +test('a cache serve also records page_age (botName, deviceType) and pagetype_age (pageType, cacheStatus, deviceType)', () => { const lastCached = Date.now() - 5000; // The three shapes a schema Date reaches this code in. for (const value of [new Date(lastCached), lastCached, new Date(lastCached).toISOString()]) { @@ -81,7 +97,7 @@ test('a cache serve also records page_age (botName, deviceType) and route_page_a recordServeOutcome( { lastCached: value }, request, - { source: 'cache', cacheStatus: 'hit', route: { path: '/product/prd-' } }, + { source: 'cache', cacheStatus: 'hit', pageTypeLabel: 'pdp' }, 'mobile' ); assert.equal(analytics.length, 4); @@ -90,26 +106,26 @@ test('a cache serve also records page_age (botName, deviceType) and route_page_a assert.equal(bot, 'Googlebot'); assert.equal(device, 'mobile'); assert.ok(age >= 4000 && age <= 7000, `expected age ~5000ms, got ${age}`); - const [rAge, rMetric, rRoute, rStatus, rDevice] = analytics[3]; - assert.equal(rMetric, 'route_page_age'); - assert.equal(rRoute, '/product/prd-'); + const [rAge, rMetric, rType, rStatus, rDevice] = analytics[3]; + assert.equal(rMetric, 'pagetype_age'); + assert.equal(rType, 'pdp'); assert.equal(rStatus, 'hit'); assert.equal(rDevice, 'mobile'); assert.equal(rAge, age); } }); -test('an swr serve carries cacheStatus swr through both route metrics', () => { +test('an swr serve carries cacheStatus swr through both page-type metrics', () => { recordServeOutcome( { lastCached: Date.now() - 5000 }, request, - { source: 'cache', cacheStatus: 'swr', route: { path: '/catalog/' } }, + { source: 'cache', cacheStatus: 'swr', pageTypeLabel: 'category' }, 'desktop' ); const statuses = analytics.map(([, metric, ...dims]) => [metric, dims]); assert.deepEqual(statuses[0], ['bot_serve', ['cache', 'swr', 'Googlebot']]); - assert.deepEqual(statuses[1], ['route_serve', ['/catalog/', 'swr', 'desktop']]); - assert.equal(statuses[3][0], 'route_page_age'); + assert.deepEqual(statuses[1], ['pagetype_serve', ['category', 'swr', 'desktop']]); + assert.equal(statuses[3][0], 'pagetype_age'); assert.equal(statuses[3][1][1], 'swr'); }); @@ -118,7 +134,7 @@ test('age metrics are skipped for a non-cache source, even with lastCached prese assert.equal(analytics.length, 2); assert.deepEqual( analytics.map(([, metric]) => metric), - ['bot_serve', 'route_serve'] + ['bot_serve', 'pagetype_serve'] ); }); @@ -129,7 +145,7 @@ test('age metrics are skipped when lastCached is missing, null, or in the future recordServeOutcome({ lastCached: null }, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop'); recordServeOutcome({ lastCached: Date.now() + 60_000 }, request, { source: 'cache', cacheStatus: 'hit' }, 'desktop'); assert.equal(analytics.length, 6); - assert.ok(analytics.every(([, metric]) => metric === 'bot_serve' || metric === 'route_serve')); + assert.ok(analytics.every(([, metric]) => metric === 'bot_serve' || metric === 'pagetype_serve')); }); test('cacheServeStatus: hit before expiresAt, swr inside the window, null past it, null on NaN', () => { diff --git a/packages/plugin/test/routeClass.test.js b/packages/plugin/test/routeClass.test.js index 821861c..3b2cf7f 100644 --- a/packages/plugin/test/routeClass.test.js +++ b/packages/plugin/test/routeClass.test.js @@ -7,6 +7,9 @@ import { matchRoute, prerenderRouteCount, queryAllowlistFor, + pageTypeSettings, + declaredPageTypes, + routePageTypes, resolveRenderInterval, PASSTHROUGH, PRERENDER, @@ -275,3 +278,189 @@ test('a per-URL cadence exception is an exact route above its class prefix', () assert.equal(resolveRenderInterval(`${base}/catalog/hot-deals.jsp`, null), HOUR_MS); assert.equal(resolveRenderInterval(`${base}/catalog/girls.jsp`, null), 6 * HOUR_MS); }); + +/* ── Page types (templates) ─────────────────────────────────────────────────────────────── */ + +// The shape this whole feature exists for: one template reached by two different URL shapes. +const withPageTypes = (pageTypes, routes) => applyOptions({ pageTypes, ingress: { mode: 'forwarded', routes } }); + +const KOHLS_SHAPED = [ + { match: 'exact', path: '/', pageType: 'home' }, + { match: 'prefix', path: '/catalog/', pageType: 'category' }, + { match: 'contains', path: '/category/', pageType: 'category' }, + { match: 'prefix', path: '/product/prd-', pageType: 'pdp' }, +]; + +test('several routes share one page type, and the metrics label collapses onto it', () => { + withPageTypes([], KOHLS_SHAPED); + // Two distinct route patterns, one label — without this the two category shapes would report + // as '/catalog/' and '/category/' and no consumer could tell they were the same template. + assert.equal(classifyPath('/catalog/girls.jsp').pageTypeLabel, 'category'); + assert.equal(classifyPath('/shop/category/boys').pageTypeLabel, 'category'); + assert.equal(classifyPath('/').pageTypeLabel, 'home'); + assert.equal(classifyPath('/product/prd-1').pageTypeLabel, 'pdp'); +}); + +test('pageType is the DECLARED name only; pageTypeLabel carries the fallback', () => { + withPageTypes([], [{ match: 'prefix', path: '/catalog/' }]); + const c = classifyPath('/catalog/girls.jsp'); + // Null name, path label. The queue job sends `pageType`, so an undeclared route must not + // make a browser-side rule scoped to a template fire on it. + assert.equal(c.pageType, null); + assert.equal(c.pageTypeLabel, '/catalog/'); +}); + +test('label falls back name -> route path -> route class', () => { + withPageTypes( + [], + [ + { match: 'prefix', path: '/named/', pageType: 'pdp' }, + { match: 'prefix', path: '/bare/' }, + ] + ); + assert.equal(classifyPath('/named/x').pageTypeLabel, 'pdp'); + assert.equal(classifyPath('/bare/x').pageTypeLabel, '/bare/'); + // Nothing matched at all — the class is the label, and it is always a string. + assert.equal(classifyPath('/nothing').pageTypeLabel, UNCLASSIFIED); +}); + +test('a deployment declaring no page types emits exactly its pre-pageTypes labels', () => { + // The adoption guarantee: turning the feature on changes no label until a route names a type. + forwarded(); + assert.equal(classifyPath('/catalog/girls.jsp').pageTypeLabel, '/catalog/'); + assert.equal(classifyPath('/').pageTypeLabel, '/'); + assert.equal(classifyPath('/nothing').pageTypeLabel, UNCLASSIFIED); +}); + +test('pageType is rejected on a passthrough route (never rendered or cached)', () => { + withPageTypes([], [{ match: 'prefix', path: '/search/', mode: PASSTHROUGH, pageType: 'search' }]); + const entry = matchRoute('/search/x'); + assert.equal(entry.mode, PASSTHROUGH); + assert.equal(entry.pageType, null); + assert.equal(classifyPath('/search/x').pageType, null); + // It still labels by its PATH, exactly as it did before page types existed — knowing which + // declared passthrough is absorbing traffic is worth more than one merged 'passthrough' row. + // Only a path matching NOTHING falls all the way through to the class. + assert.equal(classifyPath('/search/x').pageTypeLabel, '/search/'); + assert.equal(classifyPath('/nothing').pageTypeLabel, UNCLASSIFIED); +}); + +test('a malformed pageType drops the FIELD, never the route', () => { + // Losing a name costs a label; dropping the entry would change how the path is SERVED. + withPageTypes( + [], + [ + { match: 'prefix', path: '/a/', pageType: '' }, + { match: 'prefix', path: '/b/', pageType: 42 }, + { match: 'prefix', path: '/c/', pageType: 'ok' }, + ] + ); + assert.equal(prerenderRouteCount(), 3); + assert.equal(matchRoute('/a/x').pageType, null); + assert.equal(matchRoute('/b/x').pageType, null); + assert.equal(matchRoute('/c/x').pageType, 'ok'); +}); + +test('a route may name a page type that is not declared — metrics only, no settings', () => { + // Declaring a type is only needed to give it SETTINGS; requiring it would make naming a + // template purely for reporting impossible. + withPageTypes([], [{ match: 'prefix', path: '/product/', pageType: 'pdp' }]); + assert.equal(classifyPath('/product/x').pageType, 'pdp'); + assert.equal(pageTypeSettings('pdp'), null); + assert.equal(resolveRenderInterval('https://www.example.com/product/x', null), config.render.defaultInterval); +}); + +test('resolveRenderInterval precedence: route > pageType > stored > default', () => { + withPageTypes( + [ + { name: 'category', renderInterval: 12 * HOUR_MS }, + { name: 'pdp', renderInterval: 48 * HOUR_MS }, + ], + [ + // An exact route carves one URL out of its template's cadence. + { match: 'exact', path: '/catalog/hot-deals.jsp', pageType: 'category', renderInterval: HOUR_MS }, + { match: 'prefix', path: '/catalog/', pageType: 'category' }, + { match: 'contains', path: '/category/', pageType: 'category' }, + { match: 'prefix', path: '/product/prd-', pageType: 'pdp' }, + { match: 'prefix', path: '/misc/' }, // no type, no cadence + ] + ); + const base = 'https://www.example.com'; + + // Route beats its page type. + assert.equal(resolveRenderInterval(`${base}/catalog/hot-deals.jsp`, 5 * HOUR_MS), HOUR_MS); + // Page type beats the stored interval, on BOTH routes that share the type — the drift this + // replaces: the same cadence copied onto two routes, where only the first match is observed. + assert.equal(resolveRenderInterval(`${base}/catalog/girls.jsp`, 5 * HOUR_MS), 12 * HOUR_MS); + assert.equal(resolveRenderInterval(`${base}/shop/category/boys`, 5 * HOUR_MS), 12 * HOUR_MS); + assert.equal(resolveRenderInterval(`${base}/product/prd-1`, 5 * HOUR_MS), 48 * HOUR_MS); + // A route with no type still falls through to stored, then default. + assert.equal(resolveRenderInterval(`${base}/misc/x`, 5 * HOUR_MS), 5 * HOUR_MS); + assert.equal(resolveRenderInterval(`${base}/misc/x`, null), config.render.defaultInterval); +}); + +test('a declared page type with no renderInterval defers to stored, then default', () => { + withPageTypes([{ name: 'pdp' }], [{ match: 'prefix', path: '/product/', pageType: 'pdp' }]); + const url = 'https://www.example.com/product/x'; + assert.equal(pageTypeSettings('pdp').renderInterval, null); + assert.equal(resolveRenderInterval(url, 6 * HOUR_MS), 6 * HOUR_MS); + assert.equal(resolveRenderInterval(url, null), config.render.defaultInterval); +}); + +test('an invalid pageType renderInterval drops the FIELD, keeping the type usable', () => { + withPageTypes([{ name: 'pdp', renderInterval: -5 }], [{ match: 'prefix', path: '/product/', pageType: 'pdp' }]); + assert.equal(pageTypeSettings('pdp').renderInterval, null); + assert.equal(classifyPath('/product/x').pageTypeLabel, 'pdp'); // still labels + assert.equal(resolveRenderInterval('https://www.example.com/product/x', null), config.render.defaultInterval); +}); + +test('a duplicated page-type name resolves to the last declaration', () => { + withPageTypes( + [ + { name: 'pdp', renderInterval: 12 * HOUR_MS }, + { name: 'pdp', renderInterval: 48 * HOUR_MS }, + ], + [{ match: 'prefix', path: '/product/', pageType: 'pdp' }] + ); + assert.equal(resolveRenderInterval('https://www.example.com/product/x', null), 48 * HOUR_MS); +}); + +test('declaredPageTypes and routePageTypes expose both sides of the join', () => { + // What the "declared but unreferenced" config finding is computed from. + withPageTypes( + [{ name: 'category' }, { name: 'orphan' }], + [ + { match: 'prefix', path: '/catalog/', pageType: 'category' }, + { match: 'prefix', path: '/x/', pageType: 'ghost' }, + ] + ); + assert.deepEqual(declaredPageTypes(), ['category', 'orphan']); + assert.deepEqual([...routePageTypes()].sort(), ['category', 'ghost']); +}); + +test('tolerates a non-array pageTypes value and junk entries', () => { + withPageTypes([null, {}, { name: '' }, { name: 'ok' }], [{ match: 'prefix', path: '/a/', pageType: 'ok' }]); + assert.deepEqual(declaredPageTypes(), ['ok']); + assert.equal(classifyPath('/a/x').pageTypeLabel, 'ok'); +}); + +test('prefix (native) mode still resolves page types from routes', () => { + // Prefix mode has no route list gating ingress, but a route that names a template must still + // label and configure it — otherwise the feature would silently be forwarded-mode-only. + applyOptions({ + pageTypes: [{ name: 'pdp', renderInterval: 48 * HOUR_MS }], + ingress: { mode: 'prefix', routes: [{ match: 'prefix', path: '/product/', pageType: 'pdp' }] }, + }); + assert.equal(classifyPath('/product/x').pageType, 'pdp'); + assert.equal(classifyPath('/product/x').pageTypeLabel, 'pdp'); + assert.equal(resolveRenderInterval('https://www.example.com/product/x', null), 48 * HOUR_MS); + // A folded exclude is passthrough and carries no template. + assert.equal(classifyPath('/other').pageType, null); +}); + +test('classifyUrl on an unparseable URL yields a label rather than undefined', () => { + // Every request must produce a label or the metric develops holes that read as lost traffic. + withPageTypes([], KOHLS_SHAPED); + assert.equal(classifyUrl('not a url').pageTypeLabel, UNCLASSIFIED); + assert.equal(classifyUrl('not a url').pageType, null); +});