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); +});