diff --git a/TODO.md b/TODO.md index 5d615b5828..40487e1b4c 100644 --- a/TODO.md +++ b/TODO.md @@ -2558,6 +2558,55 @@ on the last column via the `alsoAbout` pattern in --- +## Close the 14 mutation survivors in `src/shared/db/listing-parents.ts` (from PR #2110) + +PR #2110 mutated `src/shared/db/listing-edge-write.ts` to a 100% score. The same +run also covered `src/shared/db/listing-parents.ts`, because the assertion that +the PR changed lives in that file's mirror tests. The run found 14 survivors in +`listing-parents.ts`. The PR does not change that file, so the survivors sit +outside its own gate. The branch-level `precommit:mutation` step covers only the +sources that a branch changes. + +The survivors fall into three shapes: + +- Eleven are "did this list come back empty?" branches. No test tells the empty + arm from the full one. +- One is the sort comparator inside `listingsForLinks`, where a divide replaces + the subtraction. +- Two are fallbacks in `edgeIncompatibilityAfterChange`, where `||` replaces + `??`. Check first whether either left side can hold a falsy-but-present value. + If it cannot, the entry belongs in `equivalent-mutants/` with that proof + rather than in a test. + +``` +listingIdsWithLinks~1dqzuig ?: → arms swapped +listingIdsWithLinks~0zl9wvu > → <=, 0 → 1 +getNonStandaloneChildIds~1vmop13 ?: → arms swapped +getNonStandaloneChildIds~00bh4s4 0 → 1 +anyNonStandaloneChild~0v88xt2 > → <=, 0 → 1 +listingsForLinks~1gjwt45 - → / +listingsForLinks~14c1k8g ?: → arms swapped +listingsForLinks~1v5jl2k > → <=, 0 → 1 +edgeIncompatibilityAfterChange.children~0cip5re ?? → || +edgeIncompatibilityAfterChange.parents~1cm1r1e ?? → || +edgeIncompatibilityAfterChange~02ardat ?: → arms swapped +``` + +Starting point: `listingIdsWithLinks` is exported and pure. A table of maps — no +links, some links, all links — kills its three survivors on its own. +`getNonStandaloneChildIds` and `anyNonStandaloneChild` need a listing that is a +child, a listing that is `bookable_alone`, and the empty-input short circuit. +`listingsForLinks` is private, so reach it through the readers that hydrate the +links. Note that its `-` → `/` survivor sits in a sort comparator. That one +needs two keys whose order a divide changes. Reproduce with: + +```bash +deno task mutation --source src/shared/db/listing-parents.ts \ + --test 'test/shared/db/listing-parents/*.test.ts' --harness +``` + +--- + ## Square treats a malformed payment link as "provider not configured" _Origin: the 2026-08 refactor survey (ADMIN_SURFACE_PLAN.md)._ diff --git a/scripts/mutation/equivalent-mutants/features.txt b/scripts/mutation/equivalent-mutants/features.txt index 7b64e31108..777f80413a 100644 --- a/scripts/mutation/equivalent-mutants/features.txt +++ b/scripts/mutation/equivalent-mutants/features.txt @@ -196,3 +196,9 @@ src/features/admin/users.ts::toDisplayUser.inviteExpired~1fwmcbe false → true src/features/admin/entity-write-tab.ts::defineEditEntityPage.extraTabs~1333wen ?? → || # configuredExtraTabs is an array of tab definitions or undefined, and an array is always truthy, so || keeps exactly what ?? keeps src/features/admin/attendee-page.ts::loadEditPanel.data.returnUrl~0i4v0pb ?? → || # URLSearchParams.get gives back a string or null, and the only falsy string is the empty fallback itself src/features/admin/route-tables.ts::idParamOf.name~0rg3o3r → "mutated" # the stand-in name for a path with no "/:" is only ever fed to the guard below, which throws for every name that is not "id" and does not end with "Id", and the message it throws names the pattern rather than the name + +# Route ordering (router.ts) — two array fallbacks, and a replacement string +# whose length cancels out of the only comparison that reads it. +src/features/router.ts::routeSpecificity.paramCount~1806jrr ?? → || # String.match returns an array or null, and an array is always truthy +src/features/router.ts::compileRoutes.methodRoutes~02urywu ?? → || # Map.get returns an array or undefined, and an array is always truthy +src/features/router.ts::routeSpecificity.literalLength~0au8fma → "mutated" # literal length is only ever compared between two routes with the SAME parameter count, so replacing each parameter with a fixed word adds the same amount to both sides and the comparison is unchanged diff --git a/scripts/mutation/equivalent-mutants/shared-a-l.txt b/scripts/mutation/equivalent-mutants/shared-a-l.txt index ec6f6abbdd..c232f86f10 100644 --- a/scripts/mutation/equivalent-mutants/shared-a-l.txt +++ b/scripts/mutation/equivalent-mutants/shared-a-l.txt @@ -28,6 +28,7 @@ src/shared/site-assignment.ts::renewalDeadlineBaseMs~00cywnq ?? → || # pars src/shared/site-assignment.ts::assignSitesForEntries.site~0c03jq6 ?? → || # available.pop(): BuiltSite|undefined, and a BuiltSite object is always truthy src/shared/site-assignment.ts::sendSiteAssignmentEmail.config~11vz7t9 ?? → || # getEmailConfig(): EmailConfig|null src/shared/site-assignment.ts::sendSiteAssignmentEmail.replyTo~195nl86 ?? → || # parseEmail(): ValidEmail|null, always truthy or null +src/shared/db/listing-prices.ts::sourceRowStatements~0fyy4cr ?? → || # unit_price is number|null, so the only falsy-non-null value is 0 and 0 ?? 0 === 0 || 0 src/shared/ledger/project.ts::allBalances.add~0gr4ng6 ?? → || # allBalances: Map.get; the only falsy-non-null number is 0, and 0 ?? 0 === 0 || 0 src/shared/ledger/project.ts::balanceOf~128hnl9 ?? → || # balanceOf: same Map.get fallback; 0 ?? 0 === 0 || 0 src/shared/ledger/reconcile.ts::IDENTITY_FIELDS~0rfpt43 ?? → || # IDENTITY_FIELDS kind: string|undefined, only falsy-non-null is "" and "" ?? "" === "" || "" @@ -181,6 +182,14 @@ src/shared/band-name-generator.ts::fixArticles~1rw9fca A $1 → "A $1 mutated" # parsePositiveIntId's strict decimal-digit schema. src/shared/logistics-filter.ts::parseAgentFilter.n~03f157n → "mutated" # parsePositiveIntId's schema rejects any non-digit string identically; the "" fallback and any mutated string both parse to null +# crypto/utils.ts — the out-of-range code readers. constantTimeCodesEqual seeds +# its XOR fold with lengthA ^ lengthB and walks the longer sequence, so a reader +# is only asked past the end of one string when the lengths differ — and that +# seed already forces "not equal". Whatever the short side reads there cannot +# change the answer. +src/shared/crypto/utils.ts::constantTimeEqual~1sr6g7q 0 → 1 # only read past the end of the shorter string, where the length XOR has already decided the result +src/shared/crypto/utils.ts::constantTimeEqual~0nflsgj 0 → 1 # same reader for the other side, reached only when the lengths already differ + # crypto/der.ts — this value is written into a Uint8Array element. Adding or # subtracting 128 produces the same low eight bits, and Uint8Array discards all # higher bits. @@ -220,3 +229,23 @@ src/shared/listings-actions.ts::listingInputToEdge.listing_type~1lqe3go ?? → src/shared/listings-actions.ts::listingInputToEdge.months_per_unit~0ruet3x ?? → || # input.monthsPerUnit is a validated whole number or absent; its only falsy value 0 equals the fallback src/shared/listings-actions.ts::validateListingEdges.orphanError~1juqee2 ?? → || # input.groupIds is an array when present, and arrays are always truthy src/shared/admin-surface.ts::adminRecordPath~11evcma ?? → || # `pattern.match(/:\w+/g) ?? []`: a global match returns null when nothing matches and a non-empty array otherwise, so the fallback is reached on exactly the same input either way + +# db/listings/attendees.ts — fallbacks whose left side can never be a +# falsy-but-present value that differs from the fallback itself. +src/shared/db/listings/attendees.ts::listingAttendeeFilter.activeOnly~1bv0ikc ?? → || # activeOnly is boolean|undefined and the fallback is false, so false ?? false === false || false +src/shared/db/listings/attendees.ts::listingAttendeeFilter.kindScope~0taxexu ?? → || # kindScope is one of two non-empty words or undefined, so it is never falsy-but-present +src/shared/db/listings/attendees.ts::getListingWithAttendeeRaw.attendeeRaw~0x3k1f1 ?? → || # the row is an Attendee object or undefined, and an object is always truthy + +# db/client.ts — fallbacks whose left side can never be a falsy-but-present +# value, and a label no code ever reads. +src/shared/db/client.ts::extractUpdateColumns.addAssignment~17cdcum 0 → 1 # an "=" at the very start leaves an empty column name, which the `if (col)` guard drops, so returning early adds the same nothing +src/shared/db/client.ts::writeSqlOf~1begzdu ?? → || # the captured tail has to start with INSERT, UPDATE, DELETE, REPLACE or SELECT, so the group is never the empty string +src/shared/db/client.ts::GUARDED_CLIENT~1cajdnv guarded-db-client → "" # a symbol's description only names it in a debugger; the guard compares symbol identity, which the description cannot change +src/shared/db/client.ts::executeRead.args~0hs5u5i ?? → || # args is an array or a named-args object when present, and both are always truthy +src/shared/db/client.ts::queryOne~1hu7rck ?? → || # a result row is always an object, so the only falsy first element is the missing one the fallback is there for + +# crypto/hashing.ts — the explicit radix on the stored round count. The line +# above it now refuses any count that is not pure decimal digits, and for such a +# string an inferred radix is decimal too, so naming the radix can no longer +# change what is read. It stays because the reader should say what base it means. +src/shared/crypto/hashing.ts::verifyPassword.iterations~10pid9h 10 → 0 # the digit-only guard above rules out every string an inferred radix would read differently diff --git a/scripts/mutation/equivalent-mutants/shared-m-z.txt b/scripts/mutation/equivalent-mutants/shared-m-z.txt index 2a5ceb55e1..806fafe2bb 100644 --- a/scripts/mutation/equivalent-mutants/shared-m-z.txt +++ b/scripts/mutation/equivalent-mutants/shared-m-z.txt @@ -265,3 +265,30 @@ src/shared/payment/row-state.ts::SortedAttendeeIdsSchema~1d17kt9 Refund claim a # seeds.ts — two thresholds no seeded value can fall between. src/shared/seeds.ts::prepareAttendee.paymentId~01ncynu 0 → 1 # a seeded listing's unit price is 0 or one of the demo prices (500 and up), so no listing sits between the two thresholds src/shared/seeds.ts::createSeeds~0ghuowu 0 → 1 # the customisable listing always yields a delete plus one multi-row insert, so the list holds 0 or 2 statements, never 1 + +# superuser.ts — the cache generation's starting number. Only a CHANGE in the +# generation matters: a lookup snapshots it before its await and compares that +# snapshot with the current value, so every comparison is between two readings +# of the same counter. Starting the count anywhere gives the same answers. +src/shared/superuser.ts::%3cfile%3e~0ljtci5 0 → 1 # the generation is only ever compared against itself across an await, never against a fixed number + +# db/modifier-resolve.ts — fallbacks whose left side can never be a +# falsy-but-present value. Each map holds whole counts, arrays, or objects, +# and each option is an object or absent. +src/shared/db/modifier-resolve.ts::stockedQuantity.remaining~06sda3h ?? → || # the used-stock map holds whole counts, so its only falsy value is the 0 the fallback supplies +src/shared/db/modifier-resolve.ts::triggerQuantity~04te607 ?? → || # the add-on map holds requested whole quantities, so its only falsy value is the 0 the fallback supplies +src/shared/db/modifier-resolve.ts::triggerQuantity~18fvmyz ?? → || # the answer-quantity map holds whole counts, so its only falsy value is the 0 the fallback supplies +src/shared/db/modifier-resolve.ts::answerModifierQuantities.entries~15ao8g3 ?? → || # the argument is a record of answer ids or absent, and every record is truthy, the empty one included +src/shared/db/modifier-resolve.ts::answerModifierQuantities.modifierIds~1y1b3am ?? → || # the map holds arrays of modifier ids, and every array is truthy, the empty one included +src/shared/db/modifier-resolve.ts::answerModifierQuantities~1ju3gsl ?? → || # the running total is a whole count, so its only falsy value is the 0 the fallback supplies +src/shared/db/modifier-resolve.ts::eligibleCandidates.addOns~0b50snz ?? → || # the option is a Map or absent, and every Map is truthy +src/shared/db/modifier-resolve.ts::eligibleCandidates.answerQuantities~1uu272f ?? → || # the option is a Map or absent, and every Map is truthy +src/shared/db/modifier-resolve.ts::eligibleCandidates.ctx~0kgxcim ?? → || # the option is a pricing-context object or absent, and every object is truthy +src/shared/db/modifier-resolve.ts::oversubscribedAnswerTiers~0f5w5hu ?? → || # the used-stock map holds whole counts, so its only falsy value is the 0 the fallback supplies +src/shared/db/modifier-resolve.ts::childOnlyAddOnNameWithScopes~0vgtyif ?? → || # a modifier name is a required non-empty field, so the only falsy result is the missing one the fallback is for + +# db/modifier-resolve.ts — the early drop of an untriggered modifier. It saves +# real work on every checkout, because an unmatched promo code yields 0 for +# every code modifier. It changes no answer, because both readers drop a 0 +# again, and no negative quantity ever reaches it. +src/shared/db/modifier-resolve.ts::eligibleCandidates~1oaufcv 1 → 0 # resolveModifiers keeps only a stocked quantity of 1 or more, and an oversubscribed tier needs a quantity above the stock left, which 0 never is diff --git a/src/features/admin/catalog-transfer/import-listing.ts b/src/features/admin/catalog-transfer/import-listing.ts index 1bf7df9b57..c657536e70 100644 --- a/src/features/admin/catalog-transfer/import-listing.ts +++ b/src/features/admin/catalog-transfer/import-listing.ts @@ -6,21 +6,12 @@ import { packageGroupIdsTx, validateListingGroupMembershipsTx, } from "#db/groups/membership.ts"; -import { getGroupsById, groups, listingGroups } from "#db/groups.ts"; -import { - addParentEdgesWithPackageCheckTx, - listingParents, -} from "#db/listing-parents.ts"; +import { addParentEdgesWithPackageCheckTx } from "#db/listing-parents.ts"; import { syncListingPrices, writeListingDayCounts, } from "#db/listing-prices.ts"; -import { getListingsById, listingsTable } from "#db/listings/records.ts"; -import { - childOnlyAddOnCheckerForListings, - type ListingGroupMembership, - toListingGroupMembership, -} from "#db/modifier-resolve.ts"; +import { listingsTable } from "#db/listings/records.ts"; import { isNameTakenAnywhere, loadCatalogNameIndex, @@ -33,15 +24,9 @@ import { TransactionValidationError } from "#db/transaction.ts"; import { t } from "#i18n"; import type { ListingInput } from "#shared/catalog-fields/fields.ts"; import { isBuilderEnabled } from "#shared/config.ts"; -import { - childAddOnError, - type EdgeListing, - edgeFieldError, -} from "#shared/listing-parents-rules.ts"; import { dayPriceFieldsFromInput, generateUniqueListingSlug, - listingInputToEdge, validateListingInput, } from "#shared/listings-actions.ts"; import { errorResult, okResult, type Result } from "#shared/result.ts"; @@ -49,10 +34,7 @@ import { seenBefore } from "#shared/seen-before.ts"; import { type AdminLevel, availableDayCounts, - clampDurationDays, type DayPricedListing, - type Group, - type Listing, parseDayPrices, } from "#types"; import { type ImportedMembership, writeMembershipsTx } from "./membership.ts"; @@ -206,97 +188,6 @@ const listingDataToInput = ( } as ListingInput; }; -const listingToEdge = (listing: Listing): EdgeListing => ({ - customisable_days: listing.customisable_days, - day_prices: listing.day_prices, - duration_days: clampDurationDays(listing.duration_days), - id: listing.id, - listing_type: listing.listing_type, - months_per_unit: listing.months_per_unit, - name: listing.name, -}); - -const firstPackageGroup = async ( - groupIds: readonly number[], -): Promise => { - if (groupIds.length === 0) return null; - const byId = await getGroupsById(); - return ( - groupIds.map((id) => byId.get(id)).find((group) => group?.is_package) || - null - ); -}; - -const loadChildAddOnChecker = async ( - input: ListingInput, - groupIds: readonly number[], - byId: Awaited>, -): Promise -> | null> => { - if (groupIds.length === 0 || input.bookableAlone) return null; - const allMembership = await listingGroups.getIdsByKeys([...byId.keys()]); - const wouldBe: ListingGroupMembership[] = [ - ...[...byId.values()].map((listing) => - toListingGroupMembership(listing, allMembership), - ), - { active: true, groupIds: [...groupIds], id: 0 }, - ]; - return childOnlyAddOnCheckerForListings(wouldBe); -}; - -type ParentEdges = { - groupIds: readonly number[]; - input: ListingInput; - parentIds: readonly number[]; -}; - -const validateParentEdges = async ({ - groupIds, - input, - parentIds, -}: ParentEdges): Promise => { - if (parentIds.length === 0) return null; - const pkg = await firstPackageGroup(groupIds); - if (pkg) { - return `"${input.name}" is a member of the package "${pkg.name}", so it cannot also be an add-on child of another listing.`; - } - const [byId, nestedParentLinks, parentGroupIds, allGroups] = - await Promise.all([ - getListingsById(), - listingParents.getIdsByKeys(parentIds), - listingGroups.getIdsByKeys([...parentIds]), - groups.cache.getAll(), - ]); - const addOnChecker = await loadChildAddOnChecker(input, groupIds, byId); - const hiddenPackageIds = new Set( - allGroups - .filter((group) => group.is_package && group.hide_package_listings) - .map((group) => group.id), - ); - const childEdge = listingInputToEdge(input, 0); - for (const parentId of parentIds) { - const parent = byId.get(parentId)!; - if (nestedParentLinks.get(parentId)!.length > 0) { - return t("listings_table.children_err_parent_is_child", { - name: parent.name, - }); - } - if ( - listingGroups - .idsFor(parentGroupIds, parentId) - .some((groupId) => hiddenPackageIds.has(groupId)) - ) { - return `"${parent.name}" is a member of a hidden package, so it cannot offer add-on children.`; - } - const fieldError = edgeFieldError(listingToEdge(parent), childEdge); - if (fieldError) return fieldError; - const addOn = addOnChecker?.(0, [parentId]); - if (addOn) return childAddOnError(addOn, input.name); - } - return null; -}; - const applyImportPolicy = ( input: ListingInput, adminLevel: AdminLevel | undefined, @@ -339,12 +230,6 @@ export const importListing = async ( ); const validationError = await validateListingInput(input); if (validationError) return fail(validationError); - const edgeError = await validateParentEdges({ - groupIds: groupResolve.ids, - input, - parentIds: parentResolve.ids, - }); - if (edgeError) return fail(edgeError); const newMember: DayPricedListing = dayPriceFieldsFromInput(input); const id = await writeRowInTransaction( diff --git a/src/locales/en/listings-table.json b/src/locales/en/listings-table.json index a80d7e1e05..bfb44f8c27 100644 --- a/src/locales/en/listings-table.json +++ b/src/locales/en/listings-table.json @@ -184,6 +184,7 @@ "error.child_listing_nested": "A selected child now has its own children. Please reload and try again.", "error.listing_deleted": "This listing was deleted. Please go back and try again.", "error.parent_listing_nested": "This listing is now a child of another listing. Please reload and try again.", + "error.parent_is_already_a_child": "A listing you named as a parent is itself a child of another listing. A listing cannot be both.", "error.selected_group_deleted": "The selected group was deleted. Please try again.", "error.selected_listing_deleted": "A selected listing was deleted. Please try again.", "listings_table.duplicate_children_dropped": "Listing duplicated, but its required children weren't copied: {reason}", diff --git a/src/shared/cache-registry.ts b/src/shared/cache-registry.ts index 1a423ad2a6..79e7c95fc8 100644 --- a/src/shared/cache-registry.ts +++ b/src/shared/cache-registry.ts @@ -50,14 +50,12 @@ export const getAllCacheStats = (): CacheStat[] => * row entering or leaving always shifts the aggregates. */ -/** Verb of a mutating SQL statement */ -export type WriteVerb = "delete" | "insert" | "replace" | "update"; - -/** Context extracted from a write statement for column-gated invalidation */ +/** What a write narrows to, for column-gated invalidation. */ export type WriteInfo = { - verb: WriteVerb; - /** Lower-cased columns assigned by an UPDATE SET clause; empty for non-updates */ - columns: ReadonlySet; + /** Lower-cased columns an UPDATE assigns, or null when the write narrows + * nothing: an INSERT, DELETE or REPLACE, or an UPDATE whose SET clause the + * parser cannot read. */ + updatedColumns: ReadonlySet | null; }; /** Why cached data was cleared. Only a committed write needs primary refills. */ @@ -146,9 +144,9 @@ export const invalidateCachesForWrite = ( if (!set) return; for (const reg of set) { if ( - info.verb === "update" && + info.updatedColumns !== null && reg.whenColumns !== undefined && - !setsIntersect(info.columns, reg.whenColumns) + !setsIntersect(info.updatedColumns, reg.whenColumns) ) { continue; } @@ -157,9 +155,9 @@ export const invalidateCachesForWrite = ( }; /** Fire every cache invalidator registered against `table` (no-op if none). - * Treats the write as unconditional (INSERT semantics): always fires column-gated entries too. */ + * The write narrows nothing, so column-gated entries fire too. */ export const invalidateCachesForTable = (table: string): void => - invalidateCachesForWrite(table, { columns: new Set(), verb: "insert" }); + invalidateCachesForWrite(table, { updatedColumns: null }); /** A `dependsOn` entry accepted by `cachedTable` / `cachedEntityTable`. */ export type DependsOnEntry = diff --git a/src/shared/crypto/hashing.ts b/src/shared/crypto/hashing.ts index 24d810dfbc..e50cff8694 100644 --- a/src/shared/crypto/hashing.ts +++ b/src/shared/crypto/hashing.ts @@ -104,7 +104,12 @@ export const verifyPassword = async ( const hashStr = parts[3]; if (!iterStr || !saltStr || !hashStr) return false; + // Every other malformed stored hash returns false, so a count that is not a + // plain positive number must too. parseInt reads "0x3e8" as 0, and PBKDF2 + // throws on a count of 0. + if (!/^\d+$/.test(iterStr)) return false; const iterations = Number.parseInt(iterStr, 10); + if (iterations === 0) return false; const salt = fromBase64(saltStr); const expectedHash = fromBase64(hashStr); diff --git a/src/shared/db/client.ts b/src/shared/db/client.ts index a83d256392..09806fbc03 100644 --- a/src/shared/db/client.ts +++ b/src/shared/db/client.ts @@ -23,10 +23,7 @@ import { trackSql, } from "#db/query-log.ts"; import { lazyRef } from "#fp"; -import { - invalidateCachesForWrite, - type WriteVerb, -} from "#shared/cache-registry.ts"; +import { invalidateCachesForWrite } from "#shared/cache-registry.ts"; import { getEnv } from "#shared/env.ts"; import { namedError } from "#shared/named-error.ts"; import { proxyMembers } from "#shared/proxy-members.ts"; @@ -114,21 +111,12 @@ const invalidateForSql = (sql: string): void => { if (!match) return; const table = match[1]!.toLowerCase(); const firstWord = writeSql.trimStart().split(/\s/)[0]!.toLowerCase(); - const verb: WriteVerb = - firstWord === "delete" || firstWord === "update" || firstWord === "replace" - ? (firstWord as WriteVerb) - : "insert"; - if (verb === "update") { - const columns = extractUpdateColumns(writeSql); - if (columns === null) { - // Parse failure: fall back to unconditional (treat as INSERT-like) - invalidateCachesForWrite(table, { columns: new Set(), verb: "insert" }); - } else { - invalidateCachesForWrite(table, { columns, verb: "update" }); - } - } else { - invalidateCachesForWrite(table, { columns: new Set(), verb }); - } + // Only an UPDATE is narrowed by what it assigns. Every other write, and an + // UPDATE whose SET clause cannot be read, invalidates unconditionally. + invalidateCachesForWrite(table, { + updatedColumns: + firstWord === "update" ? extractUpdateColumns(writeSql) : null, + }); }; const createDbClient = (): Client => { diff --git a/src/shared/db/listing-edge-write.ts b/src/shared/db/listing-edge-write.ts index 4ffcdf94ea..32c482d738 100644 --- a/src/shared/db/listing-edge-write.ts +++ b/src/shared/db/listing-edge-write.ts @@ -69,7 +69,10 @@ const nestingError = ( return null; } if (hasChildren) return t("error.child_listing_nested"); - if (hasParent) return t("error.parent_listing_nested"); + // The parents contract is the catalog import. "Reload and try again" is no + // help there. The named parent was already a child before the file was read, + // so the fix is to change the file. + if (hasParent) return t("error.parent_is_already_a_child"); return null; }; diff --git a/src/shared/db/listings/attendees.ts b/src/shared/db/listings/attendees.ts index 9d1f7adedd..201fbb251a 100644 --- a/src/shared/db/listings/attendees.ts +++ b/src/shared/db/listings/attendees.ts @@ -98,7 +98,7 @@ type ListingAttendeeFilter = { }; const listingAttendeeFilter = ( - filter: boolean | ListingAttendeeFilter = false, + filter: boolean | ListingAttendeeFilter, ): Required => typeof filter === "boolean" ? { activeOnly: filter, kindScope: "attendees" } diff --git a/src/shared/db/modifier-resolve.ts b/src/shared/db/modifier-resolve.ts index 6875dcdbce..442370b54e 100644 --- a/src/shared/db/modifier-resolve.ts +++ b/src/shared/db/modifier-resolve.ts @@ -601,26 +601,6 @@ export const childOnlyAddOnNameForListings = async ( parentPageListingIds, ); -/** - * Resolve every active opt-in add-on's would-be scope once against the supplied - * in-memory listing set, returning a reusable child-only-reachability checker. - * A caller validating many parent→child edges for one new child (a catalog - * import) resolves scopes a single time rather than once per parent, so it stays - * under the request N+1 read guard. The returned checker is - * {@link childOnlyAddOnNameForListings}'s pure core over the pre-resolved scopes. - */ -export const childOnlyAddOnCheckerForListings = async ( - allListings: ListingGroupMembership[], -): Promise< - (childId: number, parentPageListingIds: readonly number[]) => string | null -> => { - const scoped = await optionalAddOnsWithScopes( - inMemoryGroupScopeResolver(allListings), - ); - return (childId, parentPageListingIds) => - childOnlyAddOnNameWithScopes(scoped, childId, parentPageListingIds); -}; - /** The post-save shape of an opt-in add-on whose child-reachability must hold: * its trigger/active state and its **already-resolved** listing scope (null = * whole order; for a group scope, every listing in the linked groups). */ diff --git a/src/shared/sentry-sdk.ts b/src/shared/sentry-sdk.ts index 73eefe541d..bfa1f4bfb2 100644 --- a/src/shared/sentry-sdk.ts +++ b/src/shared/sentry-sdk.ts @@ -5,10 +5,10 @@ * unused Sentry exports without evaluating the SDK during module load. */ -import type { ErrorEvent } from "@sentry/core"; import { createStackParser, createTransport, + type ErrorEvent, nodeStackLineParser, type Scope, } from "@sentry/core"; diff --git a/src/shared/superuser.ts b/src/shared/superuser.ts index 02e4f08ad5..28a9b32acf 100644 --- a/src/shared/superuser.ts +++ b/src/shared/superuser.ts @@ -6,7 +6,7 @@ import { getUserByUsername, onUsersInvalidated, } from "#db/users.ts"; -import { lazyRef, ttlCache } from "#fp"; +import { lazyRef, range, ttlCache } from "#fp"; import { escapeHtml } from "#jsx/escape-html.ts"; import { getEffectiveDomain } from "#shared/config.ts"; import type { EmailConfig } from "#shared/email.ts"; @@ -136,12 +136,17 @@ export const getSuperuserState = async (): Promise => { const PASSWORD_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz23456789"; +/** Each draw asks for twice the characters still needed, so one draw is almost + * always enough, and even a dozen is a freak run. A run that reaches this many + * draws has a randomness source that hands back nothing usable. */ +const MAX_PASSWORD_DRAWS = 100; + export const generateSuperuserPassword = (length = 12): string => { const alphabetLength = PASSWORD_ALPHABET.length; const maxValidByte = 256 - (256 % alphabetLength); let result = ""; - while (result.length < length) { + for (const _draw of range(0, MAX_PASSWORD_DRAWS)) { // Rejection sampling: bytes past the last whole multiple of the alphabet // are dropped so every character stays equally likely. const bytes = crypto.getRandomValues(new Uint8Array(length * 2)); @@ -150,9 +155,10 @@ export const generateSuperuserPassword = (length = 12): string => { .slice(0, length - result.length) .map((byte) => PASSWORD_ALPHABET[byte % alphabetLength]) .join(""); + if (result.length >= length) return result; } - return result; + throw new Error("Could not draw enough random characters for a password"); }; export const createActivatedSuperuser = async (opts: { diff --git a/test/features/admin/attendees-list/page.test.ts b/test/features/admin/attendees-list/page.test.ts index 3b0f125fe0..9a7552487d 100644 --- a/test/features/admin/attendees-list/page.test.ts +++ b/test/features/admin/attendees-list/page.test.ts @@ -25,6 +25,31 @@ describeWithEnv("the attendees browser page", { db: true }, () => { describe("GET /admin/attendees", () => { testRequiresAuth("/admin/attendees"); + test("offers the export at its own address", async () => { + await seedRegistrationPair(); + const html = await (await adminGet("/admin/attendees")).text(); + expect(html).toContain("/admin/attendees/csv"); + }); + + test("offers no check-in filter, which this page does not do", async () => { + await seedRegistrationPair(); + const html = await (await adminGet("/admin/attendees")).text(); + // The check-in bar links carry filter=in / filter=out when offered. + expect(html).not.toContain("filter=in"); + expect(html).not.toContain("filter=out"); + }); + + test("drops a date from the address, which this page does not use", async () => { + await seedRegistrationPair(); + const html = await ( + await adminGet("/admin/attendees?date=2026-01-01") + ).text(); + // A page that took dates would carry the chosen one through its own + // links and form fields; this one has no date control, so it forgets it. + expect(html).toContain("Alice"); + expect(html).not.toContain("2026-01-01"); + }); + test("renders the attendees page with the registration", async () => { const listing = await makeListing("Gala Night"); await createTestAttendeeDirect(listing.id, "Alice", "alice@example.com"); diff --git a/test/features/admin/catalog-transfer/import-listing/member-ids.test.ts b/test/features/admin/catalog-transfer/import-listing/member-ids.test.ts new file mode 100644 index 0000000000..c169705eb0 --- /dev/null +++ b/test/features/admin/catalog-transfer/import-listing/member-ids.test.ts @@ -0,0 +1,25 @@ +/** + * Spotting a member that vanished between resolving names and the + * transaction's own read. + */ + +import { expect } from "@std/expect"; +import { it as test } from "@std/testing/bdd"; +import { missingMemberId } from "#routes/admin/catalog-transfer/import-listing.ts"; + +const found = new Map([ + [1, {}], + [3, {}], +]); + +test("reports nothing when every member was found", () => { + expect(missingMemberId([1, 3], found)).toBeNull(); + expect(missingMemberId([], found)).toBeNull(); +}); + +test("reports the first member that is missing, not one that is present", () => { + expect(missingMemberId([1, 2, 3], found)).toBe(2); + // Two gaps: the earlier one is the one reported. + expect(missingMemberId([2, 4], found)).toBe(2); + expect(missingMemberId([4, 2], found)).toBe(4); +}); diff --git a/test/features/admin/catalog-transfer/import-listing/references.test.ts b/test/features/admin/catalog-transfer/import-listing/references.test.ts index 374a78d940..7ffd9c609a 100644 --- a/test/features/admin/catalog-transfer/import-listing/references.test.ts +++ b/test/features/admin/catalog-transfer/import-listing/references.test.ts @@ -4,6 +4,7 @@ import { execute } from "#db/client.ts"; import { assignListingsToGroup } from "#db/groups/membership.ts"; import { groups } from "#db/groups.ts"; import { listingChildren } from "#db/listing-parents.ts"; +import { t } from "#i18n"; import { importCatalog } from "#routes/admin/catalog-transfer/import.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { createTestGroup } from "#test-utils/db-helpers/groups.ts"; @@ -69,7 +70,7 @@ describeWithEnv("catalog import references", { db: true }, () => { parents: [parent.name], version: 1, }, - '"Package child" is a member of the package "Child package", so it cannot also be an add-on child of another listing.', + t("error.package_child_is_member"), ); }); @@ -87,9 +88,7 @@ describeWithEnv("catalog import references", { db: true }, () => { expect(result.ok).toBe(false); if (result.ok) throw new Error("unreachable"); - expect(result.error).toBe( - `'${parent.name}' is itself offered as a child of another listing, so it can't also be a parent.`, - ); + expect(result.error).toBe(t("error.parent_is_already_a_child")); }); test("does not treat a visible ordinary-group member as a hidden-package parent", async () => { diff --git a/test/features/admin/catalog-transfer/import.test.ts b/test/features/admin/catalog-transfer/import.test.ts index 582eaf1f38..10a3b0dab9 100644 --- a/test/features/admin/catalog-transfer/import.test.ts +++ b/test/features/admin/catalog-transfer/import.test.ts @@ -4,7 +4,6 @@ import { execute } from "#db/client.ts"; import { getGroupPackagePrices, getListingsByGroupId } from "#db/groups.ts"; import { t } from "#i18n"; import { importCatalog } from "#routes/admin/catalog-transfer/import.ts"; -import { missingMemberId } from "#routes/admin/catalog-transfer/import-listing.ts"; import { describeWithEnv } from "#test-utils/db.ts"; import { createTestListing } from "#test-utils/db-helpers/listings.ts"; @@ -94,16 +93,6 @@ describeWithEnv("catalog group import", { db: true }, () => { }); }); -test("missingMemberId finds the first gap", () => { - const found = new Map([ - [1, {}], - [3, {}], - ]); - expect(missingMemberId([1, 3], found)).toBeNull(); - expect(missingMemberId([1, 2, 3], found)).toBe(2); - expect(missingMemberId([], found)).toBeNull(); -}); - describeWithEnv("in-tx member validation", { db: true }, () => { test("rejects an import when a resolved member vanishes inside its transaction", async () => { const member = await createTestListing({ diff --git a/test/features/router.test.ts b/test/features/router.test.ts index 30b3a18c3f..cf4ec68d29 100644 --- a/test/features/router.test.ts +++ b/test/features/router.test.ts @@ -44,6 +44,32 @@ describe("route matching", () => { expect(await response?.text()).toBe("complete"); }); + test("prefers the route with more literal text, whichever order they are declared in", async () => { + // Both patterns match "/x/end" and both take one parameter, so the tie is + // broken by how much of the path is spelled out. Declaration order must + // not matter — that is the promise that lets tooling sort route files. + const headEnd: [string, () => Response] = [ + "GET /:head/end", + () => new Response("head-end"), + ]; + const xTail: [string, () => Response] = [ + "GET /x/:tail", + () => new Response("x-tail"), + ]; + for (const entries of [ + [headEnd, xTail], + [xTail, headEnd], + ]) { + const router = createRouter(Object.fromEntries(entries)); + const response = await router( + new Request("http://localhost/x/end"), + "/x/end", + "GET", + ); + expect(await response?.text()).toBe("head-end"); + } + }); + test("returns null when the method or path does not match", async () => { const router = createRouter({ "GET /known": () => new Response("ok") }); const request = new Request("http://localhost/unknown"); diff --git a/test/integration/server/catalog-transfer.test.ts b/test/integration/server/catalog-transfer.test.ts index 9ac8150673..906bcc506a 100644 --- a/test/integration/server/catalog-transfer.test.ts +++ b/test/integration/server/catalog-transfer.test.ts @@ -410,23 +410,6 @@ describeWithEnv("catalog-transfer", { db: true }, () => { expect(result.error.toLowerCase()).toContain("daily"); }); - test("rejects a listing that is both a package member and a child", async () => { - const pkg = await createTestGroup({ isPackage: true, name: "Pkg Group" }); - await createTestListing({ name: "Some Parent" }); - const result = await importCatalog({ - groups: [{ group: "Pkg Group" }], - kind: "listing", - listing: { maxAttendees: 10, name: "Torn" }, - parents: ["Some Parent"], - version: 1, - }); - expect(result.ok).toBe(false); - if (result.ok) throw new Error("unreachable"); - expect(result.error).toContain("cannot also be an add-on child"); - // Reference the created package so the binding is used. - expect(pkg.is_package).toBe(true); - }); - test("rejects a group whose members are not the same type", async () => { await createTestListing({ listingType: "standard", name: "Std" }); await createTestListing({ listingType: "daily", name: "Daily" }); @@ -494,21 +477,6 @@ describeWithEnv( expect(result.error).toContain("referenced more than once"); }); - test("rejects a parent that is itself a child (single-level nesting)", async () => { - const grandparent = await createTestListing({ name: "Grandparent" }); - const parent = await createTestListing({ name: "Middle" }); - await listingChildren.setIds(grandparent.id, [parent.id]); - const result = await importCatalog({ - kind: "listing", - listing: { maxAttendees: 1, name: "Deep Child" }, - parents: ["Middle"], - version: 1, - }); - expect(result.ok).toBe(false); - if (result.ok) throw new Error("unreachable"); - expect(result.error).toContain("offered as a child"); - }); - test("strips webhook URL and use-defaults for an editor import", async () => { const result = await importCatalog( { diff --git a/test/scripts/stripe-mock/ports.test.ts b/test/scripts/stripe-mock/ports.test.ts index ec718af42e..2daa907040 100644 --- a/test/scripts/stripe-mock/ports.test.ts +++ b/test/scripts/stripe-mock/ports.test.ts @@ -40,6 +40,7 @@ describe("stripe-mock ports and environment", () => { }); }).toThrow(Deno.errors.AddrInUse); reserved.release(); + // Releasing twice is safe: the second call has nothing left to close. reserved.release(); try { listener = Deno.listen({ diff --git a/test/shared/cache-registry.test.ts b/test/shared/cache-registry.test.ts index d2623018e1..bb3bd18671 100644 --- a/test/shared/cache-registry.test.ts +++ b/test/shared/cache-registry.test.ts @@ -72,10 +72,7 @@ describe("cache-registry", () => { describe("registerTableInvalidation / invalidateCachesForWrite", () => { test("does not fire for a table with no registrations", () => { expect(() => - invalidateCachesForWrite("untouched", { - columns: new Set(), - verb: "insert", - }), + invalidateCachesForWrite("untouched", { updatedColumns: null }), ).not.toThrow(); }); @@ -86,10 +83,7 @@ describe("cache-registry", () => { calls++; }), ); - invalidateCachesForWrite("listings", { - columns: new Set(), - verb: "insert", - }); + invalidateCachesForWrite("listings", { updatedColumns: null }); expect(calls).toBe(1); }); @@ -100,10 +94,7 @@ describe("cache-registry", () => { calls++; }), ); - invalidateCachesForWrite("attendees", { - columns: new Set(), - verb: "insert", - }); + invalidateCachesForWrite("attendees", { updatedColumns: null }); expect(calls).toBe(0); }); @@ -116,14 +107,8 @@ describe("cache-registry", () => { }, ); unregister(); - invalidateCachesForWrite("listings", { - columns: new Set(), - verb: "insert", - }); - invalidateCachesForWrite("attendees", { - columns: new Set(), - verb: "insert", - }); + invalidateCachesForWrite("listings", { updatedColumns: null }); + invalidateCachesForWrite("attendees", { updatedColumns: null }); expect(calls).toBe(0); }); @@ -134,14 +119,8 @@ describe("cache-registry", () => { calls++; }), ); - invalidateCachesForWrite("listings", { - columns: new Set(), - verb: "insert", - }); - invalidateCachesForWrite("attendees", { - columns: new Set(), - verb: "insert", - }); + invalidateCachesForWrite("listings", { updatedColumns: null }); + invalidateCachesForWrite("attendees", { updatedColumns: null }); expect(calls).toBe(2); }); @@ -158,10 +137,7 @@ describe("cache-registry", () => { secondCalls++; }), ); - invalidateCachesForWrite("listings", { - columns: new Set(), - verb: "insert", - }); + invalidateCachesForWrite("listings", { updatedColumns: null }); expect(firstCalls).toBe(1); expect(secondCalls).toBe(1); }); @@ -171,7 +147,7 @@ describe("cache-registry", () => { * times it fired for a single update touching `updatedColumns`. */ const callsForGatedUpdate = ( whenColumns: readonly string[], - updatedColumns: readonly string[], + assigned: readonly string[], ): number => { let calls = 0; track( @@ -184,8 +160,7 @@ describe("cache-registry", () => { ), ); invalidateCachesForWrite("listings", { - columns: new Set(updatedColumns), - verb: "update", + updatedColumns: new Set(assigned), }); return calls; }; @@ -198,8 +173,7 @@ describe("cache-registry", () => { }), ); invalidateCachesForWrite("listings", { - columns: new Set(["unrelated_column"]), - verb: "update", + updatedColumns: new Set(["unrelated_column"]), }); expect(calls).toBe(1); }); @@ -213,22 +187,22 @@ describe("cache-registry", () => { expect(callsForGatedUpdate([], ["name"])).toBe(0); }); - for (const verb of ["insert", "delete", "replace"] as const) { - test(`a gated dependency always fires on ${verb}, regardless of columns`, () => { - let calls = 0; - track( - registerTableInvalidation( - ["listings"], - () => { - calls++; - }, - { whenColumns: ["price"] }, - ), - ); - invalidateCachesForWrite("listings", { columns: new Set(), verb }); - expect(calls).toBe(1); - }); - } + test("a gated dependency always fires on a write that assigns nothing", () => { + // No assigned columns means the write is not an UPDATE the gate can + // narrow — a row entering or leaving always shifts the aggregates. + let calls = 0; + track( + registerTableInvalidation( + ["listings"], + () => { + calls++; + }, + { whenColumns: ["price"] }, + ), + ); + invalidateCachesForWrite("listings", { updatedColumns: null }); + expect(calls).toBe(1); + }); }); }); @@ -273,8 +247,7 @@ describe("cache-registry", () => { }), ); invalidateCachesForWrite("listings", { - columns: new Set(["unrelated"]), - verb: "update", + updatedColumns: new Set(["unrelated"]), }); expect(calls).toBe(1); }); @@ -287,8 +260,7 @@ describe("cache-registry", () => { }), ); invalidateCachesForWrite("listing_attendees", { - columns: new Set(["unrelated"]), - verb: "update", + updatedColumns: new Set(["unrelated"]), }); expect(calls).toBe(1); }); @@ -305,14 +277,12 @@ describe("cache-registry", () => { ), ); invalidateCachesForWrite("listing_prices", { - columns: new Set(["unrelated"]), - verb: "update", + updatedColumns: new Set(["unrelated"]), }); expect(calls).toBe(0); invalidateCachesForWrite("listing_prices", { - columns: new Set(["amount"]), - verb: "update", + updatedColumns: new Set(["amount"]), }); expect(calls).toBe(1); }); @@ -345,8 +315,7 @@ describe("cache-registry", () => { invalidateCachesForTable("listings"); invalidateCachesForTable("listing_attendees"); invalidateCachesForWrite("listing_prices", { - columns: new Set(["amount"]), - verb: "update", + updatedColumns: new Set(["amount"]), }); expect(calls).toBe(0); }); diff --git a/test/shared/crypto/hashing.test.ts b/test/shared/crypto/hashing.test.ts index d2c224fb3b..eb1dedbaaa 100644 --- a/test/shared/crypto/hashing.test.ts +++ b/test/shared/crypto/hashing.test.ts @@ -42,6 +42,36 @@ describe("password hashing", () => { expect(iterations).toBe(getPbkdf2Iterations()); }); + it("names the scheme it used, so a reader knows how to check it", async () => { + const hash = await hashPassword("password"); + expect(hash.startsWith("pbkdf2:")).toBe(true); + }); + + it("reads the iteration count as decimal, so a hex count never passes", async () => { + const [prefix, iterations, salt, digest] = ( + await hashPassword("password") + ).split(":"); + // The same count written as hex. Read as decimal it is 0, which PBKDF2 + // refuses outright; read as hex it would be the real count and this + // stored digest would match, letting a hand-edited hash through. + const asHex = `0x${Number(iterations).toString(16)}`; + expect( + await verifyPassword( + "password", + `${prefix}:${asHex}:${salt}:${digest}`, + ), + ).toBe(false); + }); + + it("refuses a stored hash whose round count is zero", async () => { + const [prefix, , salt, digest] = (await hashPassword("password")).split( + ":", + ); + expect( + await verifyPassword("password", `${prefix}:0:${salt}:${digest}`), + ).toBe(false); + }); + it("uses the OWASP production iteration count when the test override is off", () => { setFastPbkdf2ForTest(null); try { diff --git a/test/shared/db/admin-features.test.ts b/test/shared/db/admin-features.test.ts index 1228013231..65ca4046f6 100644 --- a/test/shared/db/admin-features.test.ts +++ b/test/shared/db/admin-features.test.ts @@ -8,12 +8,15 @@ import { execute, queryOne } from "#db/client.ts"; import { CONFIG_KEYS, settings } from "#db/settings.ts"; import { parseEnabledFeatures } from "#shared/admin-features.ts"; import { describeWithEnv } from "#test-utils/db.ts"; +import { withDbFault } from "#test-utils/db-fault.ts"; import { SEEDED_FEATURE_RECORDS, seedFeatureRecords, settingValue, } from "#test-utils/settings.ts"; +const DEFAULTS_FAULT = "test_listing_defaults_fault"; + const storedFeatures = async () => parseEnabledFeatures(await settingValue(CONFIG_KEYS.ENABLED_FEATURES)); @@ -250,6 +253,96 @@ describeWithEnv("db > admin features", { db: true, triggers: true }, () => { ).toBe(true); }); + test("refuses to turn Logistics off while a listing still uses it", async () => { + await setAdminFeatureEnabled("logistics", true); + await execute( + "INSERT INTO listings (created, max_attendees, uses_logistics) VALUES ('2026-07-15', 10, 1)", + ); + + expect(await setAdminFeatureEnabled("logistics", false)).toBe(false); + expect((await storedFeatures()).logistics).toBe(true); + }); + + test("clears the stored Logistics listing default, not just the snapshot", async () => { + await settings.update.listingDefaults({ + hidden: true, + usesLogistics: true, + }); + await setAdminFeatureEnabled("logistics", true); + const before = await settingValue(CONFIG_KEYS.LISTING_DEFAULTS); + + expect(await setAdminFeatureEnabled("logistics", false)).toBe(true); + + // The stored value itself has to move: reading it back through a fresh + // cache is what proves the default was rewritten and not only forgotten + // in memory. + expect(await settingValue(CONFIG_KEYS.LISTING_DEFAULTS)).not.toBe(before); + settings.invalidateCache(); + await settings.loadKeys([CONFIG_KEYS.LISTING_DEFAULTS]); + expect(settings.listingDefaults).toEqual({ hidden: true }); + }); + + test("tries again when the listing defaults move under the disable", async () => { + // Two different stored defaults, both real: the disable reads the first, + // and the second lands before its write. Its write is guarded on the + // value it read, so it is refused, and it must read the moved value and + // try again rather than reporting a refusal. + await settings.update.listingDefaults({ + hidden: true, + usesLogistics: true, + }); + const read = await settingValue(CONFIG_KEYS.LISTING_DEFAULTS); + await settings.update.listingDefaults({ usesLogistics: true }); + const moved = await settingValue(CONFIG_KEYS.LISTING_DEFAULTS); + await execute("UPDATE settings SET value = ? WHERE key = ?", [ + read, + CONFIG_KEYS.LISTING_DEFAULTS, + ]); + settings.invalidateCache(); + await setAdminFeatureEnabled("logistics", true); + + // Statements run in the order they are handed to the database, so this + // one lands after the disable's read of the defaults and before its + // write — the move it has to survive. + const disabling = setAdminFeatureEnabled("logistics", false); + await execute("UPDATE settings SET value = ? WHERE key = ?", [ + moved, + CONFIG_KEYS.LISTING_DEFAULTS, + ]); + + expect(await disabling).toBe(true); + expect((await storedFeatures()).logistics).toBe(false); + settings.invalidateCache(); + await settings.loadKeys([CONFIG_KEYS.LISTING_DEFAULTS]); + // The moved value is the one that was cleaned, so the retry worked from + // what it re-read rather than from the value it first saw. + expect(settings.listingDefaults).toEqual({}); + }); + + test("a write refused for another reason is not read as a defaults clash", async () => { + await settings.update.listingDefaults({ + hidden: true, + usesLogistics: true, + }); + await setAdminFeatureEnabled("logistics", true); + + await withDbFault( + `CREATE TRIGGER ${DEFAULTS_FAULT} + BEFORE UPDATE ON settings + WHEN NEW.key = '${CONFIG_KEYS.LISTING_DEFAULTS}' + BEGIN + SELECT RAISE(ABORT, 'listing defaults are not writable'); + END`, + DEFAULTS_FAULT, + async () => { + await expect( + setAdminFeatureEnabled("logistics", false), + ).rejects.toThrow("listing defaults are not writable"); + }, + ); + expect((await storedFeatures()).logistics).toBe(true); + }); + test("rejects malformed stored feature JSON after a field write", async () => { await execute( "INSERT OR REPLACE INTO settings (key, value) VALUES (?, ?)", diff --git a/test/shared/db/client.test.ts b/test/shared/db/client.test.ts index 736d7d4518..a9e0c3ee82 100644 --- a/test/shared/db/client.test.ts +++ b/test/shared/db/client.test.ts @@ -4,6 +4,7 @@ import { describe, it as test } from "@std/testing/bdd"; import { stub } from "@std/testing/mock"; import { andConditions, + DATABASE_MAX_ATTEMPTS, deleteByFieldStatement, execute, executeReturningRow, @@ -12,6 +13,7 @@ import { getDb, inPlaceholders, insert, + orIgnore, queryAll, queryBatch, queryOne, @@ -39,6 +41,40 @@ const emptyResultSet = (): ResultSet => ({ toJSON: () => ({}), }); +describe("orIgnore", () => { + test("turns a plain insert into one that skips a clash", () => { + expect( + orIgnore({ args: [1], sql: "INSERT INTO settings (key) VALUES (?)" }), + ).toEqual({ + args: [1], + sql: "INSERT OR IGNORE INTO settings (key) VALUES (?)", + }); + }); + + test("leaves a statement that does not start with an insert alone", () => { + const update = { args: [], sql: "UPDATE settings SET value = 1" }; + expect(orIgnore(update).sql).toBe(update.sql); + }); + + test("copies the arguments rather than sharing them", () => { + const original = { + args: [1], + sql: "INSERT INTO settings (key) VALUES (?)", + }; + const relaxed = orIgnore(original); + original.args.push(2); + expect(relaxed.args).toEqual([1]); + }); +}); + +describe("DATABASE_MAX_ATTEMPTS", () => { + // The refund budgets size themselves from this number, so it has to match + // what the client really does: one try, then the 50/150/350ms ladder. + test("counts the first try plus every wait on the remote ladder", () => { + expect(DATABASE_MAX_ATTEMPTS).toBe(4); + }); +}); + describe("extractUpdateColumns", () => { test("single column assignment", () => { const cols = extractUpdateColumns( diff --git a/test/shared/db/client/invalidation.test.ts b/test/shared/db/client/invalidation.test.ts index a00128bad5..dda750f05b 100644 --- a/test/shared/db/client/invalidation.test.ts +++ b/test/shared/db/client/invalidation.test.ts @@ -17,11 +17,10 @@ import { emptyResultSet } from "#test-utils/db-helpers/result-set.ts"; import { stubTransaction } from "#test-utils/db-helpers/stub-transaction.ts"; /** - * Verb- and column-driven cache invalidation: after every write, the db client - * classifies the statement's verb and (for UPDATEs) its SET columns, so a - * column-gated registration fires only when a gated column is actually - * assigned — while INSERT / DELETE / REPLACE always fire. These tests pin the - * verb classification in `invalidateForSql` (src/shared/db/client.ts) through + * Column-driven cache invalidation: after every write, the db client reads the + * SET columns of an UPDATE, so a column-gated registration fires only when a + * gated column is actually assigned — while INSERT / DELETE / REPLACE always + * fire. These tests pin `invalidateForSql` (src/shared/db/client.ts) through * the public execute() + registry API. */ describeWithEnv("db > client write invalidation", { db: true }, () => { diff --git a/test/shared/db/client/round-trip-limit.test.ts b/test/shared/db/client/round-trip-limit.test.ts index 8d3a93ea7d..f611e85af8 100644 --- a/test/shared/db/client/round-trip-limit.test.ts +++ b/test/shared/db/client/round-trip-limit.test.ts @@ -1,7 +1,7 @@ import type { Transaction } from "@libsql/client"; import { expect } from "@std/expect"; import { it as test } from "@std/testing/bdd"; -import { getDb, queryBatch, setDb } from "#db/client.ts"; +import { getDb, queryBatch, setDb, withTransaction } from "#db/client.ts"; import { runWithQueryLogContext } from "#db/query-log.ts"; import { BUNNY_SUBREQUEST_LIMIT, @@ -171,13 +171,67 @@ describeWithEnv("db > client round-trip limit", { db: true }, () => { const rollbackAtLimit = async () => { await openTx.rollback(); }; - await expect(rollbackAtLimit()).rejects.toThrow(/limit 50/); + await expect(rollbackAtLimit()).rejects.toThrow( + /limit 50.*Blocked operation: transaction rollback/, + ); return openTx; }); // Clean up the still-open transaction outside the counted scope. await tx.rollback(); }); + // A write transaction begins and commits with two database calls, so an + // allowance of two is exactly enough for the work and nothing else. + const BEGIN_AND_COMMIT = 2; + + const transactionWithin = (allowance: { + database: number; + total: number; + }): Promise => + runWithSubrequestBudget(() => + runWithQueryLogContext(() => + withSubrequestAllowance( + { external: BUNNY_SUBREQUEST_LIMIT, ...allowance }, + () => withTransaction(() => Promise.resolve("committed")), + ), + ), + ); + + test("a transaction leaves a database call spare for the rollback it may need", async () => { + // The reserve is what keeps a failure recoverable: a transaction that + // spent the last call of its allowance on the commit would have nothing + // left for the rollback that must close it. So it is refused one call + // earlier instead, and the call held back is the rollback's. + await expect( + transactionWithin({ + database: BEGIN_AND_COMMIT, + total: BUNNY_SUBREQUEST_LIMIT, + }), + ).rejects.toThrow(/allowance exceeded/); + }); + + test("the spare call is held against the total allowance too", async () => { + // Same reserve, counted the other way: an allowance that only limits the + // combined total still has to fit the rollback. + await expect( + transactionWithin({ + database: BUNNY_SUBREQUEST_LIMIT, + total: BEGIN_AND_COMMIT, + }), + ).rejects.toThrow(/allowance exceeded/); + }); + + test("one call above its work is all a transaction needs", async () => { + // One more than the work itself, on both counts, and it commits — so the + // reserve is a single call, not a wider margin. + await expect( + transactionWithin({ + database: BEGIN_AND_COMMIT + 1, + total: BEGIN_AND_COMMIT + 1, + }), + ).resolves.toBe("committed"); + }); + test("a re-set guarded client is not wrapped again (no double counting)", async () => { // Handing an already-guarded client back to setDb must not stack a second // guard on top: each statement would then count two round trips, halving diff --git a/test/shared/db/listing-edge-write.test.ts b/test/shared/db/listing-edge-write.test.ts index bb4ef1a9aa..754c0d0aad 100644 --- a/test/shared/db/listing-edge-write.test.ts +++ b/test/shared/db/listing-edge-write.test.ts @@ -135,7 +135,7 @@ describeWithEnv( missingParentError: t("catalog_transfer.parent_missing"), parentIds: [parent.id], }), - ).rejects.toThrow(t("error.parent_listing_nested")); + ).rejects.toThrow(t("error.parent_is_already_a_child")); }); test("rejects when a submitted child no longer exists", async () => { diff --git a/test/shared/db/listing-parents/parent-edges.test.ts b/test/shared/db/listing-parents/parent-edges.test.ts index 5bc05359b7..5352c9c082 100644 --- a/test/shared/db/listing-parents/parent-edges.test.ts +++ b/test/shared/db/listing-parents/parent-edges.test.ts @@ -72,7 +72,7 @@ describeWithEnv( t("catalog_transfer.parent_missing"), ), ), - ).rejects.toThrow(t("error.parent_listing_nested")); + ).rejects.toThrow(t("error.parent_is_already_a_child")); expect(await listingParents.getIds(child.id)).toEqual([]); }); diff --git a/test/shared/db/listing-prices.test.ts b/test/shared/db/listing-prices.test.ts index c6188c193e..be1f9874d1 100644 --- a/test/shared/db/listing-prices.test.ts +++ b/test/shared/db/listing-prices.test.ts @@ -318,6 +318,30 @@ describeWithEnv("listing_prices persistence", { db: true }, () => { ]); }); + test("syncListingPricesForIds rebuilds a single listing, and only it", async () => { + const only = await createTestListing({ unitPrice: 450 }); + const untouched = await createTestListing({ unitPrice: 800 }); + // The other listing's mirror is left deliberately stale. Syncing one id + // must not quietly refresh it — that is what proves the scope is honoured + // rather than the whole table being rebuilt. + await queryAll("DELETE FROM listing_prices WHERE listing_id = ?", [ + only.id, + ]); + await queryAll( + "UPDATE listing_prices SET unit_price = 1 WHERE listing_id = ?", + [untouched.id], + ); + + await syncListingPricesForIds([only.id]); + + expect(await priceRows(only.id)).toEqual([ + { price_id: "", price_type: "base", unit_price: 450 }, + ]); + expect(await priceRows(untouched.id)).toEqual([ + { price_id: "", price_type: "base", unit_price: 1 }, + ]); + }); + test("syncListingPricesForIds is a no-op for an empty id list", async () => { await syncListingPricesForIds([]); expect(await priceRows(987656)).toEqual([]); diff --git a/test/shared/db/listings/attendees.test.ts b/test/shared/db/listings/attendees.test.ts index b1fac5161e..556b797530 100644 --- a/test/shared/db/listings/attendees.test.ts +++ b/test/shared/db/listings/attendees.test.ts @@ -6,14 +6,33 @@ import { expect } from "@std/expect"; import { it as test } from "@std/testing/bdd"; +import { execute } from "#db/client.ts"; import { + getAttendeesByListingIds, + getDailyListingAttendeeDates, + getDailyListingAttendeesByDate, getListingWithAttendeeRaw, getListingWithAttendeesRaw, } from "#db/listings/attendees.ts"; import { describeWithEnv } from "#test-utils/db.ts"; +import { bookAttendee } from "#test-utils/db-helpers/attendee-payments.ts"; import { createTestAttendee } from "#test-utils/db-helpers/attendees.ts"; -import { createTestListing } from "#test-utils/db-helpers/listings.ts"; +import { + createDailyTestListing, + createTestListing, +} from "#test-utils/db-helpers/listings.ts"; import { postListingSale } from "#test-utils/ledger.ts"; +import { createServicingEvent } from "#test-utils/servicing.ts"; + +/** Book one attendee on 4 July and hand back the created row. */ +const bookedAttendee = async ( + listing: { id: number }, + name: string, +): Promise<{ id: number }> => { + const result = await bookAttendee(listing, { date: "2026-07-04", name }); + if (!result.success) throw new Error(`booking ${name} failed`); + return result.attendees[0]!; +}; describeWithEnv( "db > listings > batched listing and attendee reads", @@ -36,6 +55,116 @@ describeWithEnv( expect(result?.listing.id).toBe(listing.id); expect(result?.listing.attendee_count).toBe(1); expect(result?.attendeesRaw.length).toBe(1); + // The attendee half really is the booking, not the listing row that + // shares the same batch. + expect(result?.attendeesRaw[0]?.listing_id).toBe(listing.id); + expect(result?.attendeesRaw[0]?.quantity).toBe(1); + }); + + test("occupied dates cover every day a booking spans, once each", async () => { + const listing = await createDailyTestListing({ + maxAttendees: 100, + maximumDaysAfter: 60, + thankYouUrl: "", + }); + // A three-day stay and a one-day stay that starts on its last day, so + // the overlap proves the days are deduplicated as well as expanded. + await bookAttendee(listing, { date: "2026-06-15", durationDays: 3 }); + await bookAttendee(listing, { date: "2026-06-17", quantity: 1 }); + + expect(await getDailyListingAttendeeDates()).toEqual([ + "2026-06-15", + "2026-06-16", + "2026-06-17", + ]); + }); + + test("occupied dates ignore a listing that is not booked daily", async () => { + const listing = await createTestListing({ maxAttendees: 10 }); + await createTestAttendee(listing.id, listing.slug, "Ann", "a@test.com"); + expect(await getDailyListingAttendeeDates()).toEqual([]); + }); + + test("reads the attendees of every listing asked for, and none when asked for none", async () => { + const first = await createTestListing({ maxAttendees: 10 }); + const second = await createTestListing({ maxAttendees: 10 }); + await createTestAttendee(first.id, first.slug, "Ann", "ann@test.com"); + await createTestAttendee(second.id, second.slug, "Bo", "bo@test.com"); + + const both = await getAttendeesByListingIds([first.id, second.id]); + expect(both.length).toBe(2); + const one = await getAttendeesByListingIds([first.id]); + expect(one.map((a) => a.listing_id)).toEqual([first.id]); + expect(await getAttendeesByListingIds([])).toEqual([]); + }); + + test("asking for active lines only drops the emptied ones", async () => { + const listing = await createTestListing({ maxAttendees: 10 }); + const kept = await createTestAttendee( + listing.id, + listing.slug, + "Ann", + "ann@test.com", + ); + const emptied = await createTestAttendee( + listing.id, + listing.slug, + "Bo", + "bo@test.com", + ); + await execute( + "UPDATE listing_attendees SET quantity = 0 WHERE attendee_id = ?", + [emptied.id], + ); + + // The default keeps every line, emptied ones included. + expect((await getAttendeesByListingIds([listing.id])).length).toBe(2); + // Both ways of asking for active lines only mean the same thing. + for (const filter of [true, { activeOnly: true }] as const) { + const active = await getAttendeesByListingIds([listing.id], filter); + expect(active.map((a) => a.id)).toEqual([kept.id]); + } + // A filter object that says nothing about active lines keeps them all. + expect( + ( + await getAttendeesByListingIds([listing.id], { + kindScope: "attendees", + }) + ).length, + ).toBe(2); + }); + + test("a servicing hold is read only when the wider scope asks for it", async () => { + const listing = await createTestListing({ maxAttendees: 10 }); + await createServicingEvent({ + bookings: [{ listingId: listing.id, quantity: 1 }], + name: "Deep clean", + }); + + // The default scope is bookings made by people. + expect(await getAttendeesByListingIds([listing.id])).toEqual([]); + const wider = await getAttendeesByListingIds([listing.id], { + kindScope: "attendees-and-servicing", + }); + expect(wider.length).toBe(1); + expect(wider[0]?.listing_id).toBe(listing.id); + }); + + test("the day view skips a hold that was emptied", async () => { + const listing = await createDailyTestListing({ + maxAttendees: 100, + maximumDaysAfter: 60, + thankYouUrl: "", + }); + const booked = await bookedAttendee(listing, "Kept"); + const emptied = await bookedAttendee(listing, "Emptied"); + await execute( + "UPDATE listing_attendees SET quantity = 0 WHERE attendee_id = ?", + [emptied.id], + ); + + const onTheDay = await getDailyListingAttendeesByDate("2026-07-04"); + expect(onTheDay.map((a) => a.id)).toEqual([booked.id]); }); test("getListingWithAttendeesRaw returns null for non-existent listing", async () => { @@ -58,10 +187,18 @@ describeWithEnv( const result = await getListingWithAttendeeRaw(listing.id, attendee.id); expect(result).not.toBeNull(); expect(result?.listing.id).toBe(listing.id); - expect(result?.attendeeRaw).not.toBeNull(); + expect(result?.attendeeRaw?.id).toBe(attendee.id); + expect(result?.attendeeRaw?.listing_id).toBe(listing.id); expect(result?.listing.attendee_count).toBe(1); }); + test("getListingWithAttendeeRaw has no attendee half when the id is unknown", async () => { + const listing = await createTestListing({ maxAttendees: 5 }); + const result = await getListingWithAttendeeRaw(listing.id, 999_999); + expect(result?.listing.id).toBe(listing.id); + expect(result?.attendeeRaw).toBeNull(); + }); + test("getListingWithAttendeeRaw returns null for non-existent listing", async () => { const result = await getListingWithAttendeeRaw(999, 1); expect(result).toBeNull(); diff --git a/test/shared/db/modifier-resolve/counting.test.ts b/test/shared/db/modifier-resolve/counting.test.ts new file mode 100644 index 0000000000..f84c9b6f4b --- /dev/null +++ b/test/shared/db/modifier-resolve/counting.test.ts @@ -0,0 +1,115 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { hashEmail } from "#db/contact-preferences.ts"; +import { + buyerVisits, + getOptionalAddOns, + oversubscribedAnswerTiers, + resolveModifiers, +} from "#db/modifier-resolve.ts"; +import { checkoutItem } from "#test-utils/checkout.ts"; +import { setContactVisits } from "#test-utils/contact-preferences.ts"; +import { describeWithEnv } from "#test-utils/db.ts"; +import { + insertModifier, + insertModifierUsage, + patchModifier, +} from "#test-utils/modifiers.ts"; + +/** + * The counting edges of the pricing engine: how many visits a buyer has, how + * much stock a tier has left, and how little an add-on has to cost before the + * order needs paying for. Each test sits on a boundary where one number + * decides the answer. + */ + +describeWithEnv("modifier-resolve counting edges", { db: true }, () => { + describe("buyerVisits", () => { + test("a buyer with no history has no visits, not one", async () => { + expect(await buyerVisits("new@example.com")).toBe(0); + }); + + test("one usable detail is enough to read a count", async () => { + // The phone is missing, so the email carries the whole answer. + await setContactVisits(await hashEmail("solo@example.com"), 4); + + expect(await buyerVisits("solo@example.com")).toBe(4); + }); + + test("a detail of only spaces is nobody, so nothing is looked up", async () => { + // Spaces are not a contact detail. A count stored against them belongs + // to no buyer, and must not become this buyer's history. + await setContactVisits(await hashEmail(" "), 3); + + expect(await buyerVisits(" ")).toBe(0); + }); + }); + + describe("oversubscribedAnswerTiers", () => { + test("a tier with its stock exactly spent refuses one more", async () => { + // Four of four are gone, so nothing is left and a request for one is + // one too many. + const tier = await insertModifier({ name: "Last one", stock: 4 }); + await patchModifier(tier.id, { trigger: "answer" }); + await insertModifierUsage(tier.id, 1, 4, 0); + + expect( + await oversubscribedAnswerTiers([checkoutItem()], { + answerQuantities: new Map([[tier.id, 1]]), + }), + ).toEqual(["Last one"]); + }); + }); + + describe("resolveModifiers", () => { + test("an add-on asked for zero times does not price", async () => { + // Zero means "not chosen". Only a request of one or more prices. + const addOn = await insertModifier({ name: "Parking" }); + await patchModifier(addOn.id, { trigger: "optional" }); + + const none = await resolveModifiers([checkoutItem()], { + addOns: new Map([[addOn.id, 0]]), + }); + expect(none.map((spec) => spec.name)).toEqual([]); + + const one = await resolveModifiers([checkoutItem()], { + addOns: new Map([[addOn.id, 1]]), + }); + expect(one.map((spec) => spec.name)).toEqual(["Parking"]); + }); + }); + + describe("getOptionalAddOns", () => { + test("an add-on of one penny still sends the order to payment", async () => { + // The smallest charge there is. A free order that picks it up stops + // being free, so the checkout has to collect money. + const penny = await insertModifier({ + calcKind: "fixed", + calcValue: 0.01, + direction: "charge", + name: "Penny", + }); + await patchModifier(penny.id, { trigger: "optional" }); + + const [offered] = await getOptionalAddOns([1]); + expect(offered).toMatchObject({ + name: "Penny", + priceLabel: "+£0.01", + requiresPayment: true, + }); + }); + + test("a discount add-on never sends the order to payment", async () => { + const rebate = await insertModifier({ + calcKind: "fixed", + calcValue: 5, + direction: "discount", + name: "Rebate", + }); + await patchModifier(rebate.id, { trigger: "optional" }); + + const [offered] = await getOptionalAddOns([1]); + expect(offered).toMatchObject({ name: "Rebate", requiresPayment: false }); + }); + }); +}); diff --git a/test/shared/db/modifier-resolve/reachability.test.ts b/test/shared/db/modifier-resolve/reachability.test.ts new file mode 100644 index 0000000000..1127b77eeb --- /dev/null +++ b/test/shared/db/modifier-resolve/reachability.test.ts @@ -0,0 +1,64 @@ +import { expect } from "@std/expect"; +import { describe, it as test } from "@std/testing/bdd"; +import { reachablePageIds, scopeIsChildDeadEnd } from "#db/modifier-resolve.ts"; + +/** + * The two pure rules behind the child-only add-on block. An add-on dead-ends + * when every page that offers it belongs to a child listing with no page of + * its own. Both rules are plain data in, plain answer out, so they are tested + * here rather than through a saved listing. + */ + +describe("scopeIsChildDeadEnd", () => { + test("a whole-order add-on is never a dead end", () => { + // A null scope means the add-on loads on every page, so no listing choice + // takes it away. + expect(scopeIsChildDeadEnd(null, new Set([5]), new Set())).toBe(false); + }); + + test("a scope that names no hidden child is never a dead end", () => { + // Listing 5 keeps its own page. The add-on stays reachable there, whatever + // the parent pages hold. + expect(scopeIsChildDeadEnd([5], new Set([9]), new Set())).toBe(false); + }); + + test("a scope of one hidden child with no page left is a dead end", () => { + expect(scopeIsChildDeadEnd([5], new Set([5]), new Set())).toBe(true); + }); + + test("one live page in the scope rescues the hidden child", () => { + expect(scopeIsChildDeadEnd([5, 6], new Set([5]), new Set([6]))).toBe(false); + }); +}); + +describe("reachablePageIds", () => { + test("keeps a listing that is live and is not a hidden child", () => { + const pages = reachablePageIds([{ active: true, id: 1 }], new Set()); + expect([...pages]).toEqual([1]); + }); + + test("drops a listing that is switched off", () => { + // An inactive listing serves no public page, so it cannot carry an add-on + // even though nothing hides it. + const pages = reachablePageIds([{ active: false, id: 1 }], new Set()); + expect([...pages]).toEqual([]); + }); + + test("drops a live listing that is a hidden child", () => { + const pages = reachablePageIds([{ active: true, id: 1 }], new Set([1])); + expect([...pages]).toEqual([]); + }); + + test("keeps only the pages that pass both rules", () => { + const pages = reachablePageIds( + [ + { active: true, id: 1 }, + { active: false, id: 2 }, + { active: true, id: 3 }, + { active: false, id: 4 }, + ], + new Set([3, 4]), + ); + expect([...pages]).toEqual([1]); + }); +}); diff --git a/test/shared/pending-work.test.ts b/test/shared/pending-work.test.ts index e86434c154..b9f41f1ac0 100644 --- a/test/shared/pending-work.test.ts +++ b/test/shared/pending-work.test.ts @@ -7,6 +7,21 @@ import { runWithPendingWork, } from "#shared/pending-work.ts"; +// Waits for a fresh task, not just a microtask, so a piece of work always +// settles after the flush's own bookkeeping and each round advances exactly +// one piece. A message port is a task like a timer is, without the +// millisecond a `setTimeout` would really spend waiting. +const nextTask = (): Promise => + new Promise((resolve) => { + const channel = new MessageChannel(); + channel.port1.onmessage = () => { + channel.port1.close(); + channel.port2.close(); + resolve(); + }; + channel.port2.postMessage(null); + }); + describe("pending-work", () => { test("has a scope inside runWithPendingWork and none outside", async () => { expect(hasPendingWorkScope()).toBe(false); @@ -72,10 +87,42 @@ describe("pending-work", () => { }); }); + test("work that finishes on the last allowed round still succeeds", async () => { + // Each piece yields to the event loop before queueing the next, so the + // flush advances exactly one piece per round. That makes this the largest + // chain the round cap admits: one more and the flush refuses it. Sitting on + // the edge is what pins the size of the cap. + const TOTAL_WORK = 999; + let made = 0; + const queueAgain = (): void => { + if (made >= TOTAL_WORK) return; + made++; + addPendingWork( + (async () => { + await nextTask(); + queueAgain(); + })(), + ); + }; + await runWithPendingWork(async () => { + queueAgain(); + await flushPendingWork(); + expect(made).toBe(TOTAL_WORK); + }); + }); + test("work that queues fresh work forever fails loudly instead of spinning", async () => { + // The chain feeds itself, so it needs its own ceiling as well as the flag: + // if the assertion below throws, `finally` still lowers the flag, and if + // something stops the flush from ever throwing, this ceiling stops the + // chain rather than letting it spin the event loop forever. It sits far + // above the flush's own round cap so the real failure still happens first. + const QUEUE_CEILING = 5_000; + let queued = 0; let keepQueueing = true; const queueAgain = (): void => { - if (!keepQueueing) return; + if (!keepQueueing || queued >= QUEUE_CEILING) return; + queued++; addPendingWork( (async () => { await Promise.resolve(); @@ -85,13 +132,17 @@ describe("pending-work", () => { }; await runWithPendingWork(async () => { queueAgain(); - await expect(flushPendingWork()).rejects.toThrow( - "Pending work kept queueing more work instead of finishing", - ); - // Stop the chain and drain its tail so the scope can end cleanly. - keepQueueing = false; - await flushPendingWork(); + try { + await expect(flushPendingWork()).rejects.toThrow( + "Pending work kept queueing more work instead of finishing", + ); + } finally { + // Stop the chain and drain its tail so the scope can end cleanly. + keepQueueing = false; + await flushPendingWork(); + } }); + expect(queued).toBeLessThan(QUEUE_CEILING); }); test("flushPendingWork outside a scope is a no-op", async () => { diff --git a/test/shared/safe-fetch.test.ts b/test/shared/safe-fetch.test.ts index f7e4cb1717..e2ec7cbbcb 100644 --- a/test/shared/safe-fetch.test.ts +++ b/test/shared/safe-fetch.test.ts @@ -21,6 +21,48 @@ describe("safe-fetch", () => { expect(result.status).toBe(302); }); + // Every status the standard uses to say "go and ask over there". Written out + // rather than read from the source, so dropping one from the list has to be + // a deliberate edit here too. + for (const status of [301, 302, 303, 307, 308]) { + test(`follows a ${status} to its destination`, async () => { + const asked: string[] = []; + const result = await fetchTextFollowingSafeRedirects( + "https://example.com/start", + undefined, + (url) => { + asked.push(url); + return Promise.resolve( + asked.length === 1 + ? response(status, "https://example.com/moved") + : response(200), + ); + }, + ); + + expect(asked).toEqual([ + "https://example.com/start", + "https://example.com/moved", + ]); + expect(result.status).toBe(200); + }); + } + + test("leaves a status that is not a redirect alone", async () => { + const asked: string[] = []; + const result = await fetchTextFollowingSafeRedirects( + "https://example.com/start", + undefined, + (url) => { + asked.push(url); + return Promise.resolve(response(304, "https://example.com/moved")); + }, + ); + + expect(asked).toEqual(["https://example.com/start"]); + expect(result.status).toBe(304); + }); + test("rejects syntactically invalid redirect locations", async () => { await expect( fetchTextFollowingSafeRedirects( diff --git a/test/shared/site-pages/core.test.ts b/test/shared/site-pages/core.test.ts index 4e8b84d795..da41582a6b 100644 --- a/test/shared/site-pages/core.test.ts +++ b/test/shared/site-pages/core.test.ts @@ -210,6 +210,31 @@ describe("site-pages core", () => { expect(isReservedSlug(" Listings ")).toBe(true); expect(isReservedSlug("about-us")).toBe(false); }); + + // Every word a page may not take, written out so dropping one from the + // list has to be a deliberate edit here too. Each would otherwise shadow + // a real route or a nav label. + for (const word of [ + "admin", + "api", + "contact", + "home", + "listings", + "news", + "order", + "page", + "terms", + "ticket", + ]) { + test(`keeps "${word}" for the system`, () => { + expect(isReservedSlug(word)).toBe(true); + }); + } + + test("allows a word that merely contains a reserved one", () => { + expect(isReservedSlug("admin-notes")).toBe(false); + expect(isReservedSlug("homes")).toBe(false); + }); }); describe("buildNavModel", () => { diff --git a/test/shared/superuser.test.ts b/test/shared/superuser.test.ts index 0ba413646f..46da06ed07 100644 --- a/test/shared/superuser.test.ts +++ b/test/shared/superuser.test.ts @@ -421,6 +421,25 @@ describeWithEnv("getSuperuserState account lookup", { db: true }, () => { }); }); + test("keeps the cached account state warm as time passes", async () => { + await withAdminEmailQueryLog(async () => { + const base = Date.now(); + let offset = 0; + const clock = stub(Date, "now", () => base + offset); + try { + await expectLoggedSuperuserStateRead(); + enableQueryLog(); + // A second later is still well inside the cache's life, so this read + // must not go back to the database. + offset = 1_000; + await getSuperuserState(); + expect(getQueryLog().length).toBe(0); + } finally { + clock.restore(); + } + }); + }); + test("re-queries after a user write invalidates the cache", async () => { await withAdminEmail(async () => { // Warm the cache with the not-yet-created state. @@ -539,6 +558,63 @@ describe("generateSuperuserPassword", () => { spyCrypto.restore(); } }); + + // How many times the generator may draw before it gives up. Written out + // rather than imported: the production constant is private, and an export + // whose only reader is a test is what the dead-export check refuses. + const DRAWS_ALLOWED = 100; + + test("uses every draw it is allowed before building the password", () => { + // Every draw but the last hands back only rejected bytes, so the password + // can only be finished on the hundredth. One draw fewer and this same run + // would be refused, which is what pins how many draws are allowed. + let draws = 0; + const randomStub = stub( + crypto, + "getRandomValues", + (array: A): A => { + draws++; + // 255 sits in the rejected tail; 0 is the first alphabet character. + if (array instanceof Uint8Array) { + array.fill(draws < DRAWS_ALLOWED ? 255 : 0); + } + return array; + }, + ); + try { + expect(generateSuperuserPassword(12).length).toBe(12); + expect(draws).toBe(DRAWS_ALLOWED); + } finally { + randomStub.restore(); + } + }); + + test("randomness that never yields a usable byte fails after its last draw", () => { + // 255 sits in the rejected tail for a 58-character alphabet, so every draw + // contributes nothing and the password can never fill. Without a ceiling on + // the draws this spins the CPU forever instead of failing. Counting the + // draws pins the ceiling from the other side: the test above proves the + // hundredth draw is still allowed, this one proves there is no hundred-and- + // first. + let draws = 0; + const randomStub = stub( + crypto, + "getRandomValues", + (array: A): A => { + draws++; + if (array instanceof Uint8Array) array.fill(255); + return array; + }, + ); + try { + expect(() => generateSuperuserPassword(12)).toThrow( + "Could not draw enough random characters for a password", + ); + expect(draws).toBe(DRAWS_ALLOWED); + } finally { + randomStub.restore(); + } + }); }); // --------------------------------------------------------------------------- @@ -662,6 +738,19 @@ describe("sendSuperuserCredentialsEmail", () => { await expectEmailBodyContains("text")("https://localhost/admin/")(); }); + test("email text body opens by saying what happened", async () => { + await expectEmailBodyContains("text")( + "A superuser account has been enabled for this ticket platform.", + )(); + }); + + test("email html body escapes an apostrophe in the username", async () => { + const { body } = await sendAndCapture({ username: "o'brien" }); + const html = String(body.html); + expect(html).toContain("o'brien"); + expect(html).not.toContain("o'brien"); + }); + test("email text body contains a security warning", async () => { await expectEmailBodyContains("text")( "Store this password securely",