From d1c976b1cac1c277923681983f8786e46146d6eb Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Fri, 2 Oct 2026 22:41:35 +0100 Subject: [PATCH 1/8] refactor(project): move the pure project-address functions to @datatug/project-address; close two #180 items in them The app's eager start-up code may not import the lazy-loaded datatug-main library statically (@nx/enforce-module-boundaries), so the hand-off trust check could not use the real trust function and kept its own copy of the trusted repository list. The id, address and trust functions (github-project-address.ts and the project URL part of nav-models.ts) are pure and have no Angular, so they move, with their specs, to libs/datatug/project-address. datatug-main keeps every import path: nav/github-project-address.ts re-exports the library and nav-models.ts re-exports the project URL functions. Two follow-ups of issue #180 land in the moved code: - tryProjectUrl/projectUrl take `page` as plain text and percent-encode every segment (a space, `%`, `?`, `#` or a non-ASCII letter used to be written as typed, or refused for a bare `%`); the result must still read back as that page. - readNewProjectFolder: the new-project form's folder field, read as the reader will accept it (traversal, a leading slash, `..`, a `-` or empty segment, `@` and the rest are refused). Co-Authored-By: Claude Sonnet 5.5 --- libs/datatug/main/package.json | 1 + .../src/lib/nav/github-project-address.ts | 272 +---------- libs/datatug/main/src/lib/nav/nav-models.ts | 457 +---------------- libs/datatug/project-address/eslint.config.js | 7 + libs/datatug/project-address/ng-package.json | 7 + libs/datatug/project-address/package.json | 6 + libs/datatug/project-address/project.json | 35 ++ .../src/lib}/github-project-address.spec.ts | 61 +++ .../src/lib/github-project-address.ts | 294 +++++++++++ .../src/lib}/project-url.spec.ts | 48 +- .../project-address/src/lib/project-url.ts | 461 ++++++++++++++++++ .../datatug/project-address/src/public_api.ts | 6 + libs/datatug/project-address/tsconfig.json | 22 + .../datatug/project-address/tsconfig.lib.json | 18 + .../project-address/tsconfig.lib.prod.json | 6 + .../project-address/tsconfig.spec.json | 21 + libs/datatug/project-address/vite.config.mts | 52 ++ tsconfig.base.json | 1 + 18 files changed, 1061 insertions(+), 714 deletions(-) create mode 100644 libs/datatug/project-address/eslint.config.js create mode 100644 libs/datatug/project-address/ng-package.json create mode 100644 libs/datatug/project-address/package.json create mode 100644 libs/datatug/project-address/project.json rename libs/datatug/{main/src/lib/nav => project-address/src/lib}/github-project-address.spec.ts (85%) create mode 100644 libs/datatug/project-address/src/lib/github-project-address.ts rename libs/datatug/{main/src/lib/nav => project-address/src/lib}/project-url.spec.ts (95%) create mode 100644 libs/datatug/project-address/src/lib/project-url.ts create mode 100644 libs/datatug/project-address/src/public_api.ts create mode 100644 libs/datatug/project-address/tsconfig.json create mode 100644 libs/datatug/project-address/tsconfig.lib.json create mode 100644 libs/datatug/project-address/tsconfig.lib.prod.json create mode 100644 libs/datatug/project-address/tsconfig.spec.json create mode 100644 libs/datatug/project-address/vite.config.mts diff --git a/libs/datatug/main/package.json b/libs/datatug/main/package.json index a00cae00..b2f1f17d 100644 --- a/libs/datatug/main/package.json +++ b/libs/datatug/main/package.json @@ -19,6 +19,7 @@ "@acrodata/code-editor": "*", "@datatug/board-models": "*", "@datatug/product-profiles": "*", + "@datatug/project-address": "*", "@sneat/auth-core": "0.27.6", "@sneat/auth-models": "0.27.6", "@sneat/auth-ui": "0.27.20", diff --git a/libs/datatug/main/src/lib/nav/github-project-address.ts b/libs/datatug/main/src/lib/nav/github-project-address.ts index 7a6cc504..c4419af9 100644 --- a/libs/datatug/main/src/lib/nav/github-project-address.ts +++ b/libs/datatug/main/src/lib/nav/github-project-address.ts @@ -1,269 +1,3 @@ -// Pure functions for the id and the trust of a project read from GitHub. No Angular, no I/O, no imports: -// design `demo-as-github-project.md` 3.3 (the id), 3.4a (one canonical spelling) and 3.6 (the trust decision). -// -// The two reader functions `parseGithubProjectId` and `buildGithubRawUrl` stay defined in -// `services/repo/github/github-project-reader.service.ts` (their one owner); they delegate here so that the -// reader service is not pulled into every file that only needs to read an address. -// -// Two readings of an id exist. `splitGithubProjectId` is lenient and never throws (the reader has always been -// tolerant of a malformed id). `readGithubProjectId` is strict: it is the only reading that a URL is built from -// and that a trust decision is made on. - -/** The store id of GitHub-stored projects (`parseStoreRef` recognises it). */ -export const GITHUB_STORE_ID = 'github.com'; - -/** The folder a two-part id (`repo@org`) has always meant. */ -export const DEFAULT_GITHUB_PROJECT_FOLDER = 'datatug'; - -/** The default branch, whatever it is called. The only spelling of it that an id or an address uses. */ -export const GITHUB_DEFAULT_BRANCH_REF = 'HEAD'; - -/** - * A GitHub project id: `repo@org`, `repo@org@folder` or `repo@org@folder@ref`. - * `folder` is the repo-relative directory holding `datatug-project.json` (`''` is the repo root); - * `ref` is present only for a branch, tag or commit that is not the default branch. - */ -export interface IGithubProjectId { - readonly repo: string; - readonly org: string; - readonly folder: string; - readonly ref?: string; -} - -/** - * Lower-cases A-Z only. `toLowerCase()` is not safe for a trust decision: the Kelvin sign U+212A lower-cases to - * the ASCII `k` (`chinooK-demo` would then equal `chinook-demo`). - */ -export function asciiLowerCase(value: string): string { - return value.replace(/[A-Z]/g, (c) => - String.fromCharCode(c.charCodeAt(0) + 32), - ); -} - -function lowerIfString(value: string): string { - // The reader has always been tolerant of a malformed id (`org` is undefined for `"abc"`): keep that. - return typeof value === 'string' ? asciiLowerCase(value) : value; -} - -/** - * Splits a project id. Owner and repo are lower-cased (GitHub treats them case-insensitively, and one project - * must have one id); folder and ref are not (they are case-sensitive on GitHub). A missing third part means the - * folder `datatug`; an empty one means the repo root; a ref of `HEAD` means no ref (the default branch). - * Never throws. - */ -export function splitGithubProjectId(projectId: string): IGithubProjectId { - const [repo, org, folder = DEFAULT_GITHUB_PROJECT_FOLDER, ref] = - projectId.split('@'); - const parts = { repo: lowerIfString(repo), org: lowerIfString(org), folder }; - return ref && ref !== GITHUB_DEFAULT_BRANCH_REF ? { ...parts, ref } : parts; -} - -/** The one id of a project: the inverse of {@link splitGithubProjectId}, in its shortest spelling. */ -export function formatGithubProjectId(project: IGithubProjectId): string { - const repo = asciiLowerCase(project.repo); - const org = asciiLowerCase(project.org); - const ref = - project.ref && project.ref !== GITHUB_DEFAULT_BRANCH_REF - ? project.ref - : undefined; - if (ref) { - return `${repo}@${org}@${project.folder}@${ref}`; - } - if (project.folder === DEFAULT_GITHUB_PROJECT_FOLDER) { - return `${repo}@${org}`; - } - return `${repo}@${org}@${project.folder}`; -} - -/** What GitHub allows as an owner (a user or an organisation): ASCII letters, digits, hyphens. */ -export const GITHUB_OWNER_PATTERN = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})$/; - -/** What GitHub allows as a repository name: ASCII letters, digits, `.`, `_`, `-`; never `.` or `..`. */ -export const GITHUB_REPO_PATTERN = /^(?!\.{1,2}$)[A-Za-z0-9._-]{1,100}$/; - -const UNSAFE_SEGMENT_CHARS = '/\\%?#'; - -/** - * One path segment of a repository path or a ref, as a person would type it (decoded): not empty, not `.` or `..`, - * no `/ \ % ? #`, no control character, no leading or trailing whitespace. A space inside is fine. - */ -export function isSafePathSegment(segment: unknown): segment is string { - if ( - typeof segment !== 'string' || - segment === '' || - segment === '.' || - segment === '..' || - segment !== segment.trim() - ) { - return false; - } - for (let i = 0; i < segment.length; i++) { - const code = segment.charCodeAt(i); - if ( - code < 0x20 || - code === 0x7f || - UNSAFE_SEGMENT_CHARS.includes(segment[i]) - ) { - return false; - } - } - return true; -} - -/** A project folder: `''` (the repo root) or `/`-separated safe segments, none of them `-` (it ends a locator) or containing `@`. */ -export function isValidGithubFolder(folder: unknown): folder is string { - return ( - folder === '' || - (typeof folder === 'string' && - folder - .split('/') - .every((s) => s !== '-' && !s.includes('@') && isSafePathSegment(s))) - ); -} - -/** A branch, tag or commit as one path segment (a `/` is not supported: use the commit). */ -export function isValidGithubRef(ref: unknown): ref is string { - return isSafePathSegment(ref) && !ref.includes('@'); -} - -export type GithubProjectIdProblem = - | 'parts' - | 'owner-or-repo' - | 'folder' - | 'ref'; - -export type GithubProjectIdReading = - | { readonly ok: true; readonly id: IGithubProjectId } - | { readonly ok: false; readonly reason: GithubProjectIdProblem }; - -/** - * The strict reading of a project id: two to four `@`-separated parts, a real owner and repo name (ASCII, no `.git` - * ending), a folder of safe segments (design 3.3: `''` is the root, a missing third part is `datatug`), and a safe - * one-segment ref (empty or `HEAD` mean the default branch and read as no ref). Owner and repo come back lower - * case. Never throws; anything that is not a valid id is a reason. - */ -export function readGithubProjectId( - projectId: unknown, -): GithubProjectIdReading { - if (typeof projectId !== 'string') { - return { ok: false, reason: 'parts' }; - } - const parts = projectId.split('@'); - if (parts.length < 2 || parts.length > 4) { - return { ok: false, reason: 'parts' }; - } - const [repo, org, folder = DEFAULT_GITHUB_PROJECT_FOLDER, ref] = parts; - if ( - !GITHUB_OWNER_PATTERN.test(org) || - !GITHUB_REPO_PATTERN.test(repo) || - asciiLowerCase(repo).endsWith('.git') - ) { - return { ok: false, reason: 'owner-or-repo' }; - } - if (!isValidGithubFolder(folder)) { - return { ok: false, reason: 'folder' }; - } - const named = ref && ref !== GITHUB_DEFAULT_BRANCH_REF ? ref : undefined; - if (named !== undefined && !isValidGithubRef(named)) { - return { ok: false, reason: 'ref' }; - } - return { - ok: true, - id: { - repo: asciiLowerCase(repo), - org: asciiLowerCase(org), - folder, - ...(named !== undefined ? { ref: named } : {}), - }, - }; -} - -/** Thrown when an id cannot be used to read a project from GitHub. */ -export class GithubProjectIdError extends Error { - constructor( - public readonly reason: GithubProjectIdProblem | 'path', - message: string, - ) { - super(message); - this.name = 'GithubProjectIdError'; - } -} - -// --------------------------------------------------------------------------- -// Trust (design 3.6) -// --------------------------------------------------------------------------- - -/** - * The projects that may run with no click. Compiled in. Exactly one entry at first: the demo repository, at the - * repository root, on its default branch. Owner and repo are lower case. `folder` is `''` for every entry in the - * first release (the demo project is at the repo root). - */ -const TRUSTED_GITHUB_PROJECTS: readonly { - readonly owner: string; - readonly repo: string; - readonly folder: string; -}[] = [{ owner: 'datatug', repo: 'chinook-demo', folder: '' }]; - -function isTrustedGithubProject(address: { - readonly owner: string; - readonly repo: string; - readonly folder: string; - readonly ref?: string; -}): boolean { - const { owner, repo, folder, ref } = address; - if (typeof owner !== 'string' || typeof repo !== 'string') { - return false; - } - if (ref !== undefined && ref !== GITHUB_DEFAULT_BRANCH_REF) { - return false; - } - const lowerOwner = asciiLowerCase(owner); - const lowerRepo = asciiLowerCase(repo); - return TRUSTED_GITHUB_PROJECTS.some( - (t) => - t.owner === lowerOwner && t.repo === lowerRepo && t.folder === folder, - ); -} - -/** What the trust decision is asked about: a store and a project id, as every page reads them. */ -export interface IProjectAddressLike { - readonly storeId: string; - readonly projectId: string; -} - -/** - * Whether a project may run with no click and read the app's own outside data hosts (design 3.6). The ONE trust - * function: it cannot be asked without the store. Pass `parseProjectUrl`'s result or a `{storeId, projectId}`. - * - * Trusted means all of: - * - the store is exactly `github.com` (a case variant, `github`, an agent or Firestore id never is); - * - the id reads strictly (`readGithubProjectId`): a real owner and repo, a folder of safe segments, a safe ref; - * - owner and repo, lower-cased (ASCII only), equal exactly an entry of the compiled-in list: never a prefix, - * never the address string; - * - the folder is empty: in the FIRST release the demo project is at the root of its repo, so a trusted project - * has no folder (this also closes every `..`, `\`, `?` or `#` trick in a folder); - * - the ref is absent or exactly `HEAD`. Any other ref (a branch, a tag, a commit SHA) is not trusted, because - * GitHub serves a fork's commits under the parent repository's address. - * - * Only own properties of the argument are read. Anything that is not such an address is not trusted. - */ -export function isTrustedProjectAddress( - address: IProjectAddressLike | { readonly ok: false }, -): boolean { - if (typeof address !== 'object' || address === null) { - return false; - } - const own = (name: string): unknown => - Object.prototype.hasOwnProperty.call(address, name) - ? (address as unknown as Record)[name] - : undefined; - if (own('storeId') !== GITHUB_STORE_ID) { - return false; - } - const reading = readGithubProjectId(own('projectId')); - if (!reading.ok) { - return false; - } - const { org, repo, folder, ref } = reading.id; - return isTrustedGithubProject({ owner: org, repo, folder, ref }); -} +// The id and trust functions of a GitHub project live in `@datatug/project-address` (pure, no Angular), so that the +// app's eager code can use them; this path stays for the reader and every other importer. +export * from '@datatug/project-address'; diff --git a/libs/datatug/main/src/lib/nav/nav-models.ts b/libs/datatug/main/src/lib/nav/nav-models.ts index fa435f6f..79604410 100644 --- a/libs/datatug/main/src/lib/nav/nav-models.ts +++ b/libs/datatug/main/src/lib/nav/nav-models.ts @@ -4,19 +4,6 @@ import { ITableFull } from '../models/definition/apis/database'; import { IEnvironmentSummary } from '../models/definition/environments'; import { IProjectSummary, IProjEnv } from '../models/definition/project'; import { IDatatugStoreBrief, IProjectBrief } from '../models/interfaces'; -import { - asciiLowerCase, - formatGithubProjectId, - GITHUB_DEFAULT_BRANCH_REF, - GITHUB_OWNER_PATTERN, - GITHUB_REPO_PATTERN, - GITHUB_STORE_ID, - IGithubProjectId, - isSafePathSegment, - isValidGithubFolder, - isValidGithubRef, - readGithubProjectId, -} from './github-project-address'; /** * Matches a bare `host:port` store id, e.g. `"localhost:8989"`. Deliberately @@ -219,433 +206,19 @@ export interface IAgentContext { port: number; } -export const getStoreId = (repo: string): string => { - return (repo || '').replace(/(https?):\/\//, '$1-'); -}; -// --------------------------------------------------------------------------- -// Project addresses (design `demo-as-github-project.md` 3.1, 3.3, 3.4, 3.4a) -// -// Two shapes of a project path exist, and these functions are the one place that knows them: -// - GitHub projects: `/project/github.com//[/tree/[/…]][/-/…]` (canonical); -// - every store, GitHub included for as long as it is accepted: `/store//project/[/…]`. -// Both are paths (a `Location.pathname`), never a path with a query string or a fragment. -// -// Contract: for every `{storeId, projectId}` and page that `projectUrl` accepts, `parseProjectUrl` reads the -// returned path back as that same project (the canonical id) and page; everything else is refused, never turned -// into the address of another project. -// --------------------------------------------------------------------------- - -const PROJECT_FILE_NAME = 'datatug-project.json'; - -/** Why an address is not accepted. The app words each one for the visitor (design 3.4a). */ -export type ProjectUrlErrorReason = - /** Not a project address at all (another route). Not an error to show. */ - | 'not-a-project-address' - /** The owner or the repo is not a name GitHub allows (this also covers `%2F`, `.git.git`, unicode look-alikes). */ - | 'invalid-owner-or-repo' - /** A segment that cannot be part of a project locator or a page: `.`/`..`, an encoded `/` or `%`, a control character, leading or trailing whitespace, `?`, `#`, an empty segment, a `-` where a page was expected. */ - | 'invalid-path-segment' - /** `@` in a directory or a ref: it separates the parts of the project id. */ - | 'at-sign-not-supported' - /** A directory segment named `-`: the first `-` ends the project locator. */ - | 'dash-directory-not-supported' - /** `blob/…` of anything but the project file: a link to a file. */ - | 'file-link' - /** `…/tree` with no ref. */ - | 'missing-ref' - /** A spelling of the GitHub store id other than `github.com` or `github` (`GitHub.com`): never a different store. */ - | 'unsupported-store-id' - /** `projectUrl` only: the id or the page has no exact address. */ - | 'not-representable'; - -export interface IProjectUrlError { - readonly ok: false; - readonly reason: ProjectUrlErrorReason; -} - -/** What `projectUrl` throws. */ -export class ProjectUrlError extends Error { - constructor(public readonly reason: ProjectUrlErrorReason) { - super(`no exact project address (${reason})`); - this.name = 'ProjectUrlError'; - } -} - -/** The parts of a GitHub project: owner and repo lower case; `folder` as in the id (`''` = repo root); `ref` only when it is not the default branch. */ -export interface IGithubProjectParts { - readonly owner: string; - readonly repo: string; - readonly folder: string; - readonly ref?: string; -} - -export interface IProjectUrlParts { - readonly ok: true; - readonly storeId: string; - readonly projectId: string; - /** What follows the project locator: `''`, or a path beginning with `/` (`/chat`), every segment checked (not empty, not `.` or `..`). Left as typed (still percent-encoded). */ - readonly rest: string; - /** `short` = `/project/github.com/…`; `legacy` = `/store//project/`. */ - readonly shape: 'short' | 'legacy'; - /** The one address of this project and page (design 3.4a), `rest` kept. */ - readonly canonicalPath: string; - /** False when the typed path is another spelling of `canonicalPath` (case, `.git`, a trailing slash, `tree/HEAD`, a `blob/` link, the legacy GitHub shape). Always true for a non-GitHub store (one shape, left as typed). */ - readonly isCanonical: boolean; - /** - * For a GitHub project: its parts, so that no caller splits the id again. `ref` is the typed branch, tag or - * commit when it is not `HEAD`; when it names the repo's default branch the caller must (with one GitHub API - * call) rewrite it to `HEAD`: it cannot be known here. - */ - readonly github?: IGithubProjectParts; -} - -function isGithubStoreId(storeId: string): boolean { - return storeId === GITHUB_STORE_ID || storeId === 'github'; -} - -function decodeSegment(raw: string): string | undefined { - try { - return decodeURIComponent(raw); - } catch { - return undefined; - } -} - -/** A project locator segment, decoded once; `undefined` when it cannot be one. */ -function decodeLocatorSegment(raw: string): string | undefined { - const decoded = decodeSegment(raw); - return isSafePathSegment(decoded) ? decoded : undefined; -} - -/** - * A page segment, as typed: it must decode, hold no control character, and (decoded, split at `/` and `\`) no - * empty, `.` or `..` piece. A `%2F` is how a host turns a name into a path, so it is judged as the path it becomes. - */ -function isSafeRestSegment(raw: string): boolean { - const decoded = decodeSegment(raw); - if (decoded === undefined || decoded === '') { - return false; - } - for (let i = 0; i < decoded.length; i++) { - const code = decoded.charCodeAt(i); - if (code < 0x20 || code === 0x7f) { - return false; - } - } - return decoded - .split(/[\\/]/) - .every((p) => p !== '' && p !== '.' && p !== '..'); -} - -/** `''`, or `/seg/seg` for checked raw segments; `undefined` when a segment is not acceptable. */ -function restOf(segments: readonly string[]): string | undefined { - if (!segments.every(isSafeRestSegment)) { - return undefined; - } - return segments.length > 0 ? '/' + segments.join('/') : ''; -} - -const encodeSegments = (segments: readonly string[]): string => - segments.map((s) => encodeURIComponent(s)).join('/'); - -/** The canonical short path of a valid (strictly read) GitHub project and a checked `rest`. */ -function shortGithubPath(project: IGithubProjectId, rest: string): string { - const base = `/project/${GITHUB_STORE_ID}/${project.org}/${project.repo}`; - // A page that starts with `tree`, `blob` or `-` would read as part of the locator: spell the tree form out. - const ambiguous = /^\/(?:tree|blob|-)(?:\/|$)/.test(rest); - if (project.ref === undefined && project.folder === '' && !ambiguous) { - return base + rest; - } - const ref = encodeURIComponent(project.ref ?? GITHUB_DEFAULT_BRANCH_REF); - const dir = project.folder - ? `/${encodeSegments(project.folder.split('/'))}` - : ''; - return `${base}/tree/${ref}${dir}${rest ? '/-' + rest : ''}`; -} - -function githubParts(project: IGithubProjectId): IGithubProjectParts { - return { - owner: project.org, - repo: project.repo, - folder: project.folder, - ...(project.ref !== undefined ? { ref: project.ref } : {}), - }; -} - -/** - * The path of a project, and of one of its pages (`page` is `'chat'`, `'queries/x'`, with or without ONE leading - * `/`), or the reason there is none. A GitHub project gets its canonical short address - * (`/project/github.com//…`); every other store `/store//project/`. - * - * The address is returned only when `parseProjectUrl` reads it back as the same project and page: an id with - * more than four parts, a folder or ref holding a `/`, a `-` folder segment, a page with an empty or `..` segment, - * a case variant of the store id (`GitHub.com`) have no exact address and are refused. A GitHub id comes back in - * its canonical spelling (`r@o@datatug` is `r@o`). - */ -export function tryProjectUrl( - ref: IProjectRef, - page?: string, -): string | IProjectUrlError { - const refused = (reason: ProjectUrlErrorReason): IProjectUrlError => ({ - ok: false, - reason, - }); - const typedPage = - typeof page === 'string' && page.startsWith('/') - ? page.slice(1) - : (page ?? ''); - const rest = typedPage ? restOf(typedPage.split('/')) : ''; - if (rest === undefined) { - return refused('invalid-path-segment'); - } - const storeId = ref?.storeId; - const projectId = ref?.projectId; - if (typeof storeId !== 'string' || typeof projectId !== 'string') { - return refused('not-representable'); - } - let candidate: string; - let expectedStoreId: string; - let expectedProjectId: string; - if (isGithubStoreId(storeId)) { - const reading = readGithubProjectId(projectId); - if (!reading.ok) { - return refused('not-representable'); - } - candidate = shortGithubPath(reading.id, rest); - expectedStoreId = GITHUB_STORE_ID; - expectedProjectId = formatGithubProjectId(reading.id); - } else { - expectedStoreId = getStoreId(storeId); - expectedProjectId = projectId; - candidate = `/store/${expectedStoreId}/project/${projectId}${rest}`; - } - const back = parseProjectUrl(candidate); - if (!back.ok) { - return refused(back.reason); - } - if ( - back.storeId !== expectedStoreId || - back.projectId !== expectedProjectId || - back.rest !== rest || - back.canonicalPath !== candidate - ) { - return refused('not-representable'); - } - return candidate; -} - -/** {@link tryProjectUrl}, throwing a {@link ProjectUrlError} when there is no exact address. */ -export function projectUrl(ref: IProjectRef, page?: string): string { - const result = tryProjectUrl(ref, page); - if (typeof result !== 'string') { - throw new ProjectUrlError(result.reason); - } - return result; -} - -/** - * Reads a project path (a pathname: no query, no fragment) in either shape into the `{storeId, projectId}` pair - * every page already reads from the route, plus what follows (`rest`), the one canonical spelling, and, for an - * address that cannot be a project, why (`ok: false`). Pure: it makes no request, so whether a named ref is the - * default branch is left to the caller (`github.ref`). - */ -export function parseProjectUrl( - path: string, -): IProjectUrlParts | IProjectUrlError { - const notOurs: IProjectUrlError = { - ok: false, - reason: 'not-a-project-address', - }; - if (typeof path !== 'string' || !path.startsWith('/')) { - return notOurs; - } - const segments = path.slice(1).split('/'); - if (segments[0] === 'project') { - return segments[1] === GITHUB_STORE_ID - ? parseShortGithubPath(path, segments) - : notOurs; - } - if ( - segments[0] === 'store' && - segments[2] === 'project' && - segments[1] && - segments[3] - ) { - return parseLegacyPath(path, segments); - } - return notOurs; -} - -/** Drops ONE trailing empty segment (a trailing slash); any further empty segment is then refused as a page segment. */ -function withoutTrailingSlash(segments: string[]): string[] { - return segments.length > 0 && segments[segments.length - 1] === '' - ? segments.slice(0, -1) - : segments; -} - -function parseLegacyPath( - path: string, - segments: string[], -): IProjectUrlParts | IProjectUrlError { - const storeId = decodeLocatorSegment(segments[1]); - const projectId = decodeLocatorSegment(segments[3]); - if (storeId === undefined || projectId === undefined) { - return { ok: false, reason: 'invalid-path-segment' }; - } - const rest = restOf(withoutTrailingSlash(segments.slice(4))); - if (rest === undefined) { - return { ok: false, reason: 'invalid-path-segment' }; - } - const lowerStoreId = asciiLowerCase(storeId); - if (!isGithubStoreId(lowerStoreId)) { - return { - ok: true, - storeId, - projectId, - rest, - shape: 'legacy', - canonicalPath: path, - isCanonical: true, - }; - } - // `GitHub.com` is neither this store nor another one: refused, so no spelling can pass for GitHub. - if (!isGithubStoreId(storeId)) { - return { ok: false, reason: 'unsupported-store-id' }; - } - const reading = readGithubProjectId(projectId); - if (!reading.ok) { - const partCount = projectId.split('@').length; - return { - ok: false, - reason: - reading.reason === 'owner-or-repo' - ? 'invalid-owner-or-repo' - : reading.reason === 'parts' - ? partCount < 2 - ? 'not-a-project-address' - : 'at-sign-not-supported' - : 'invalid-path-segment', - }; - } - const canonicalPath = shortGithubPath(reading.id, rest); - return { - ok: true, - storeId: GITHUB_STORE_ID, - projectId: formatGithubProjectId(reading.id), - rest, - shape: 'legacy', - canonicalPath, - isCanonical: canonicalPath === path, - github: githubParts(reading.id), - }; -} - -function parseShortGithubPath( - path: string, - segments: string[], -): IProjectUrlParts | IProjectUrlError { - const raw = withoutTrailingSlash(segments.slice(2)); - if (raw.length < 2) { - return { ok: false, reason: 'not-a-project-address' }; - } - const owner = decodeLocatorSegment(raw[0]); - let repo = decodeLocatorSegment(raw[1]); - if ( - repo !== undefined && - repo.length >= 4 && - asciiLowerCase(repo).endsWith('.git') - ) { - repo = repo.slice(0, -4); - } - // A repo name never ends in `.git` (so `r.git.git` is refused here), and both names are ASCII. - if ( - owner === undefined || - repo === undefined || - !GITHUB_OWNER_PATTERN.test(owner) || - !GITHUB_REPO_PATTERN.test(repo) || - asciiLowerCase(repo).endsWith('.git') - ) { - return { ok: false, reason: 'invalid-owner-or-repo' }; - } - const after = raw.slice(2); - let ref: string | undefined; - const dirs: string[] = []; - let restSegments: string[] = after; - - if (after[0] === 'tree' || after[0] === 'blob') { - const isBlob = after[0] === 'blob'; - if (after[1] === undefined) { - return { ok: false, reason: isBlob ? 'file-link' : 'missing-ref' }; - } - const refSegment = decodeLocatorSegment(after[1]); - if (refSegment === undefined) { - return { ok: false, reason: 'invalid-path-segment' }; - } - if (refSegment.includes('@')) { - return { ok: false, reason: 'at-sign-not-supported' }; - } - ref = refSegment; - let tail = after.slice(2); - restSegments = []; - if (isBlob) { - // A pasted link to the project file itself: `blob///datatug-project.json`. - if (tail[tail.length - 1] !== PROJECT_FILE_NAME) { - return { ok: false, reason: 'file-link' }; - } - tail = tail.slice(0, -1); - } else { - const dash = tail.indexOf('-'); - if (dash !== -1) { - restSegments = tail.slice(dash + 1); - tail = tail.slice(0, dash); - } - } - for (const segment of tail) { - const dir = decodeLocatorSegment(segment); - if (dir === undefined) { - return { ok: false, reason: 'invalid-path-segment' }; - } - if (dir === '-') { - return { ok: false, reason: 'dash-directory-not-supported' }; - } - if (dir.includes('@')) { - return { ok: false, reason: 'at-sign-not-supported' }; - } - dirs.push(dir); - } - } else if (after[0] === '-') { - return { ok: false, reason: 'invalid-path-segment' }; - } - - const rest = restOf(restSegments); - if (rest === undefined) { - return { ok: false, reason: 'invalid-path-segment' }; - } - const folder = dirs.join('/'); - const named = - ref !== undefined && ref !== GITHUB_DEFAULT_BRANCH_REF ? ref : undefined; - if ( - !isValidGithubFolder(folder) || - (named !== undefined && !isValidGithubRef(named)) - ) { - return { ok: false, reason: 'invalid-path-segment' }; - } - const project: IGithubProjectId = { - repo: asciiLowerCase(repo), - org: asciiLowerCase(owner), - folder, - ...(named !== undefined ? { ref: named } : {}), - }; - const canonicalPath = shortGithubPath(project, rest); - return { - ok: true, - storeId: GITHUB_STORE_ID, - projectId: formatGithubProjectId(project), - rest, - shape: 'short', - canonicalPath, - isCanonical: canonicalPath === path, - github: githubParts(project), - }; -} +// Project addresses (design `demo-as-github-project.md` 3.1, 3.3, 3.4, 3.4a): the pure functions live in +// `@datatug/project-address`, so that the app's eager code can use them; every existing import keeps its path. +export { + getStoreId, + parseProjectUrl, + projectUrl, + ProjectUrlError, + tryProjectUrl, +} from '@datatug/project-address'; +export type { + IGithubProjectParts, + IProjectUrlError, + IProjectUrlParts, + ProjectUrlErrorReason, +} from '@datatug/project-address'; diff --git a/libs/datatug/project-address/eslint.config.js b/libs/datatug/project-address/eslint.config.js new file mode 100644 index 00000000..c1a861f1 --- /dev/null +++ b/libs/datatug/project-address/eslint.config.js @@ -0,0 +1,7 @@ +const baseConfig = require('../../../eslint.config.js'); +const { sneatLibConfig } = require('../../../eslint.lib.config.js'); + +module.exports = [ + ...baseConfig, + ...sneatLibConfig(__dirname), +]; diff --git a/libs/datatug/project-address/ng-package.json b/libs/datatug/project-address/ng-package.json new file mode 100644 index 00000000..05dfbee6 --- /dev/null +++ b/libs/datatug/project-address/ng-package.json @@ -0,0 +1,7 @@ +{ + "$schema": "../../../node_modules/ng-packagr/ng-package.schema.json", + "dest": "../../../dist/libs/datatug/project-address", + "lib": { + "entryFile": "src/public_api.ts" + } +} diff --git a/libs/datatug/project-address/package.json b/libs/datatug/project-address/package.json new file mode 100644 index 00000000..a25c9fc9 --- /dev/null +++ b/libs/datatug/project-address/package.json @@ -0,0 +1,6 @@ +{ + "name": "@datatug/project-address", + "version": "0.0.1", + "description": "The pure functions that read and build the address of a project (GitHub project ids, the short project URL, and the trust decision of design demo-as-github-project.md 3.3, 3.4a and 3.6). No Angular, no I/O: the app's eager start-up code and the lazy datatug-main library both use them.", + "sideEffects": false +} diff --git a/libs/datatug/project-address/project.json b/libs/datatug/project-address/project.json new file mode 100644 index 00000000..1ad1623c --- /dev/null +++ b/libs/datatug/project-address/project.json @@ -0,0 +1,35 @@ +{ + "name": "datatug-project-address", + "$schema": "../../../node_modules/nx/schemas/project-schema.json", + "sourceRoot": "libs/datatug/project-address/src", + "prefix": "datatug", + "projectType": "library", + "tags": [], + "targets": { + "build": { + "executor": "@nx/angular:ng-packagr-lite", + "outputs": ["{workspaceRoot}/dist/{projectRoot}"], + "options": { + "project": "libs/datatug/project-address/ng-package.json", + "tsConfig": "libs/datatug/project-address/tsconfig.lib.json" + }, + "configurations": { + "production": { + "tsConfig": "libs/datatug/project-address/tsconfig.lib.prod.json" + }, + "development": {} + }, + "defaultConfiguration": "production" + }, + "test": { + "executor": "@nx/vitest:test", + "outputs": ["{workspaceRoot}/coverage/{projectRoot}"], + "options": { + "tsConfig": "libs/datatug/project-address/tsconfig.spec.json" + } + }, + "lint": { + "executor": "@nx/eslint:lint" + } + } +} diff --git a/libs/datatug/main/src/lib/nav/github-project-address.spec.ts b/libs/datatug/project-address/src/lib/github-project-address.spec.ts similarity index 85% rename from libs/datatug/main/src/lib/nav/github-project-address.spec.ts rename to libs/datatug/project-address/src/lib/github-project-address.spec.ts index d94f71ed..79bbbc66 100644 --- a/libs/datatug/main/src/lib/nav/github-project-address.spec.ts +++ b/libs/datatug/project-address/src/lib/github-project-address.spec.ts @@ -6,6 +6,7 @@ import { isValidGithubFolder, isValidGithubRef, readGithubProjectId, + readNewProjectFolder, splitGithubProjectId, } from './github-project-address'; @@ -300,3 +301,63 @@ describe('isTrustedProjectAddress (design 3.6): the one trust function', () => { ).toBe(false); }); }); + +describe('readNewProjectFolder: the new-project form\'s folder field (issue #180)', () => { + it.each([ + // [typed, folder] + ['', 'datatug'], + [' ', 'datatug'], + ['datatug', 'datatug'], + [' demo-project-1 ', 'demo-project-1'], + ['a/b', 'a/b'], + ['a/b/', 'a/b'], + ['a/b//', 'a/b'], + ['my project', 'my project'], + ['.hidden', '.hidden'], + ['a..b', 'a..b'], + ])('%j is the folder %j', (typed, folder) => { + expect(readNewProjectFolder(typed)).toEqual({ ok: true, folder }); + }); + + it.each([ + // [typed, why] + ['/', 'leading-slash'], + ['/datatug', 'leading-slash'], + [' /a', 'leading-slash'], + ['//a', 'leading-slash'], + ['..', 'invalid'], + ['.', 'invalid'], + ['../x', 'invalid'], + ['a/../b', 'invalid'], + ['a/./b', 'invalid'], + ['a/..', 'invalid'], + ['a//b', 'invalid'], + ['a\\b', 'invalid'], + ['..\\x', 'invalid'], + ['-', 'invalid'], + ['a/-/b', 'invalid'], + ['a@b', 'invalid'], + ['a%2e%2e', 'invalid'], + ['a?b', 'invalid'], + ['a#b', 'invalid'], + ['a\u0000b', 'invalid'], + ['a\tb', 'invalid'], + ])('%j is refused (%s)', (typed, reason) => { + expect(readNewProjectFolder(typed)).toEqual({ ok: false, reason }); + }); + + it('treats anything that is not text as blank', () => { + expect(readNewProjectFolder(undefined)).toEqual({ ok: true, folder: 'datatug' }); + expect(readNewProjectFolder(42)).toEqual({ ok: true, folder: 'datatug' }); + }); + + it('only ever returns a folder the reader accepts', () => { + for (const typed of ['a', 'a/b', 'a b', '.x', 'x.', 'é']) { + const reading = readNewProjectFolder(typed); + expect(reading.ok).toBe(true); + if (reading.ok) { + expect(readGithubProjectId('r@o@' + reading.folder).ok).toBe(true); + } + } + }); +}); diff --git a/libs/datatug/project-address/src/lib/github-project-address.ts b/libs/datatug/project-address/src/lib/github-project-address.ts new file mode 100644 index 00000000..cf58f785 --- /dev/null +++ b/libs/datatug/project-address/src/lib/github-project-address.ts @@ -0,0 +1,294 @@ +// Pure functions for the id and the trust of a project read from GitHub. No Angular, no I/O, no imports: +// design `demo-as-github-project.md` 3.3 (the id), 3.4a (one canonical spelling) and 3.6 (the trust decision). +// +// The two reader functions `parseGithubProjectId` and `buildGithubRawUrl` stay defined in +// `services/repo/github/github-project-reader.service.ts` (their one owner); they delegate here so that the +// reader service is not pulled into every file that only needs to read an address. +// +// Two readings of an id exist. `splitGithubProjectId` is lenient and never throws (the reader has always been +// tolerant of a malformed id). `readGithubProjectId` is strict: it is the only reading that a URL is built from +// and that a trust decision is made on. + +/** The store id of GitHub-stored projects (`parseStoreRef` recognises it). */ +export const GITHUB_STORE_ID = 'github.com'; + +/** The folder a two-part id (`repo@org`) has always meant. */ +export const DEFAULT_GITHUB_PROJECT_FOLDER = 'datatug'; + +/** The default branch, whatever it is called. The only spelling of it that an id or an address uses. */ +export const GITHUB_DEFAULT_BRANCH_REF = 'HEAD'; + +/** + * A GitHub project id: `repo@org`, `repo@org@folder` or `repo@org@folder@ref`. + * `folder` is the repo-relative directory holding `datatug-project.json` (`''` is the repo root); + * `ref` is present only for a branch, tag or commit that is not the default branch. + */ +export interface IGithubProjectId { + readonly repo: string; + readonly org: string; + readonly folder: string; + readonly ref?: string; +} + +/** + * Lower-cases A-Z only. `toLowerCase()` is not safe for a trust decision: the Kelvin sign U+212A lower-cases to + * the ASCII `k` (`chinooK-demo` would then equal `chinook-demo`). + */ +export function asciiLowerCase(value: string): string { + return value.replace(/[A-Z]/g, (c) => + String.fromCharCode(c.charCodeAt(0) + 32), + ); +} + +function lowerIfString(value: string): string { + // The reader has always been tolerant of a malformed id (`org` is undefined for `"abc"`): keep that. + return typeof value === 'string' ? asciiLowerCase(value) : value; +} + +/** + * Splits a project id. Owner and repo are lower-cased (GitHub treats them case-insensitively, and one project + * must have one id); folder and ref are not (they are case-sensitive on GitHub). A missing third part means the + * folder `datatug`; an empty one means the repo root; a ref of `HEAD` means no ref (the default branch). + * Never throws. + */ +export function splitGithubProjectId(projectId: string): IGithubProjectId { + const [repo, org, folder = DEFAULT_GITHUB_PROJECT_FOLDER, ref] = + projectId.split('@'); + const parts = { repo: lowerIfString(repo), org: lowerIfString(org), folder }; + return ref && ref !== GITHUB_DEFAULT_BRANCH_REF ? { ...parts, ref } : parts; +} + +/** The one id of a project: the inverse of {@link splitGithubProjectId}, in its shortest spelling. */ +export function formatGithubProjectId(project: IGithubProjectId): string { + const repo = asciiLowerCase(project.repo); + const org = asciiLowerCase(project.org); + const ref = + project.ref && project.ref !== GITHUB_DEFAULT_BRANCH_REF + ? project.ref + : undefined; + if (ref) { + return `${repo}@${org}@${project.folder}@${ref}`; + } + if (project.folder === DEFAULT_GITHUB_PROJECT_FOLDER) { + return `${repo}@${org}`; + } + return `${repo}@${org}@${project.folder}`; +} + +/** What GitHub allows as an owner (a user or an organisation): ASCII letters, digits, hyphens. */ +export const GITHUB_OWNER_PATTERN = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})$/; + +/** What GitHub allows as a repository name: ASCII letters, digits, `.`, `_`, `-`; never `.` or `..`. */ +export const GITHUB_REPO_PATTERN = /^(?!\.{1,2}$)[A-Za-z0-9._-]{1,100}$/; + +const UNSAFE_SEGMENT_CHARS = '/\\%?#'; + +/** + * One path segment of a repository path or a ref, as a person would type it (decoded): not empty, not `.` or `..`, + * no `/ \ % ? #`, no control character, no leading or trailing whitespace. A space inside is fine. + */ +export function isSafePathSegment(segment: unknown): segment is string { + if ( + typeof segment !== 'string' || + segment === '' || + segment === '.' || + segment === '..' || + segment !== segment.trim() + ) { + return false; + } + for (let i = 0; i < segment.length; i++) { + const code = segment.charCodeAt(i); + if ( + code < 0x20 || + code === 0x7f || + UNSAFE_SEGMENT_CHARS.includes(segment[i]) + ) { + return false; + } + } + return true; +} + +/** A project folder: `''` (the repo root) or `/`-separated safe segments, none of them `-` (it ends a locator) or containing `@`. */ +export function isValidGithubFolder(folder: unknown): folder is string { + return ( + folder === '' || + (typeof folder === 'string' && + folder + .split('/') + .every((s) => s !== '-' && !s.includes('@') && isSafePathSegment(s))) + ); +} + +export type NewProjectFolderReading = + | { readonly ok: true; readonly folder: string } + | { readonly ok: false; readonly reason: 'leading-slash' | 'invalid' }; + +/** + * The folder field of the new-project form, as the reader will accept it. A blank field means the default folder + * (`datatug`); a folder starting with `/` is refused (it would read as an absolute path); trailing slashes are + * dropped; what is left must be {@link isValidGithubFolder} and not empty, so `..`, `.`, a backslash, an empty + * segment (`a//b`), a `-` segment, `@`, `%`, `?` and `#` are all refused: a project can only be created in a + * folder that the reader (and so its address) can open. + */ +export function readNewProjectFolder(typed: unknown): NewProjectFolderReading { + const trimmed = typeof typed === 'string' ? typed.trim() : ''; + if (trimmed === '') { + return { ok: true, folder: DEFAULT_GITHUB_PROJECT_FOLDER }; + } + if (trimmed.startsWith('/')) { + return { ok: false, reason: 'leading-slash' }; + } + const folder = trimmed.replace(/\/+$/, ''); + return folder !== '' && isValidGithubFolder(folder) + ? { ok: true, folder } + : { ok: false, reason: 'invalid' }; +} + +/** A branch, tag or commit as one path segment (a `/` is not supported: use the commit). */ +export function isValidGithubRef(ref: unknown): ref is string { + return isSafePathSegment(ref) && !ref.includes('@'); +} + +export type GithubProjectIdProblem = + | 'parts' + | 'owner-or-repo' + | 'folder' + | 'ref'; + +export type GithubProjectIdReading = + | { readonly ok: true; readonly id: IGithubProjectId } + | { readonly ok: false; readonly reason: GithubProjectIdProblem }; + +/** + * The strict reading of a project id: two to four `@`-separated parts, a real owner and repo name (ASCII, no `.git` + * ending), a folder of safe segments (design 3.3: `''` is the root, a missing third part is `datatug`), and a safe + * one-segment ref (empty or `HEAD` mean the default branch and read as no ref). Owner and repo come back lower + * case. Never throws; anything that is not a valid id is a reason. + */ +export function readGithubProjectId( + projectId: unknown, +): GithubProjectIdReading { + if (typeof projectId !== 'string') { + return { ok: false, reason: 'parts' }; + } + const parts = projectId.split('@'); + if (parts.length < 2 || parts.length > 4) { + return { ok: false, reason: 'parts' }; + } + const [repo, org, folder = DEFAULT_GITHUB_PROJECT_FOLDER, ref] = parts; + if ( + !GITHUB_OWNER_PATTERN.test(org) || + !GITHUB_REPO_PATTERN.test(repo) || + asciiLowerCase(repo).endsWith('.git') + ) { + return { ok: false, reason: 'owner-or-repo' }; + } + if (!isValidGithubFolder(folder)) { + return { ok: false, reason: 'folder' }; + } + const named = ref && ref !== GITHUB_DEFAULT_BRANCH_REF ? ref : undefined; + if (named !== undefined && !isValidGithubRef(named)) { + return { ok: false, reason: 'ref' }; + } + return { + ok: true, + id: { + repo: asciiLowerCase(repo), + org: asciiLowerCase(org), + folder, + ...(named !== undefined ? { ref: named } : {}), + }, + }; +} + +/** Thrown when an id cannot be used to read a project from GitHub. */ +export class GithubProjectIdError extends Error { + constructor( + public readonly reason: GithubProjectIdProblem | 'path', + message: string, + ) { + super(message); + this.name = 'GithubProjectIdError'; + } +} + +// --------------------------------------------------------------------------- +// Trust (design 3.6) +// --------------------------------------------------------------------------- + +/** + * The projects that may run with no click. Compiled in. Exactly one entry at first: the demo repository, at the + * repository root, on its default branch. Owner and repo are lower case. `folder` is `''` for every entry in the + * first release (the demo project is at the repo root). + */ +const TRUSTED_GITHUB_PROJECTS: readonly { + readonly owner: string; + readonly repo: string; + readonly folder: string; +}[] = [{ owner: 'datatug', repo: 'chinook-demo', folder: '' }]; + +function isTrustedGithubProject(address: { + readonly owner: string; + readonly repo: string; + readonly folder: string; + readonly ref?: string; +}): boolean { + const { owner, repo, folder, ref } = address; + if (typeof owner !== 'string' || typeof repo !== 'string') { + return false; + } + if (ref !== undefined && ref !== GITHUB_DEFAULT_BRANCH_REF) { + return false; + } + const lowerOwner = asciiLowerCase(owner); + const lowerRepo = asciiLowerCase(repo); + return TRUSTED_GITHUB_PROJECTS.some( + (t) => + t.owner === lowerOwner && t.repo === lowerRepo && t.folder === folder, + ); +} + +/** What the trust decision is asked about: a store and a project id, as every page reads them. */ +export interface IProjectAddressLike { + readonly storeId: string; + readonly projectId: string; +} + +/** + * Whether a project may run with no click and read the app's own outside data hosts (design 3.6). The ONE trust + * function: it cannot be asked without the store. Pass `parseProjectUrl`'s result or a `{storeId, projectId}`. + * + * Trusted means all of: + * - the store is exactly `github.com` (a case variant, `github`, an agent or Firestore id never is); + * - the id reads strictly (`readGithubProjectId`): a real owner and repo, a folder of safe segments, a safe ref; + * - owner and repo, lower-cased (ASCII only), equal exactly an entry of the compiled-in list: never a prefix, + * never the address string; + * - the folder is empty: in the FIRST release the demo project is at the root of its repo, so a trusted project + * has no folder (this also closes every `..`, `\`, `?` or `#` trick in a folder); + * - the ref is absent or exactly `HEAD`. Any other ref (a branch, a tag, a commit SHA) is not trusted, because + * GitHub serves a fork's commits under the parent repository's address. + * + * Only own properties of the argument are read. Anything that is not such an address is not trusted. + */ +export function isTrustedProjectAddress( + address: IProjectAddressLike | { readonly ok: false }, +): boolean { + if (typeof address !== 'object' || address === null) { + return false; + } + const own = (name: string): unknown => + Object.prototype.hasOwnProperty.call(address, name) + ? (address as unknown as Record)[name] + : undefined; + if (own('storeId') !== GITHUB_STORE_ID) { + return false; + } + const reading = readGithubProjectId(own('projectId')); + if (!reading.ok) { + return false; + } + const { org, repo, folder, ref } = reading.id; + return isTrustedGithubProject({ owner: org, repo, folder, ref }); +} diff --git a/libs/datatug/main/src/lib/nav/project-url.spec.ts b/libs/datatug/project-address/src/lib/project-url.spec.ts similarity index 95% rename from libs/datatug/main/src/lib/nav/project-url.spec.ts rename to libs/datatug/project-address/src/lib/project-url.spec.ts index 5c73208e..b739d341 100644 --- a/libs/datatug/main/src/lib/nav/project-url.spec.ts +++ b/libs/datatug/project-address/src/lib/project-url.spec.ts @@ -13,7 +13,7 @@ import { projectUrl, ProjectUrlError, tryProjectUrl, -} from './nav-models'; +} from './project-url'; const SHA = '0123456789abcdef0123456789abcdef01234567'; @@ -769,6 +769,42 @@ describe('projectUrl', () => { }); }); +describe('projectUrl writes the page as plain text, percent-encoded (issue #180)', () => { + it.each([ + // [page, the written segments] + ['a b', '/a%20b'], + ['queries/a b', '/queries/a%20b'], + ['50%', '/50%25'], + ['a%20b', '/a%2520b'], // the text `a%20b`, not a space + ['x?y=1', '/x%3Fy%3D1'], + ['x#y', '/x%23y'], + ['é/ü', '/%C3%A9/%C3%BC'], + ['a\\b', '/a%5Cb'], + ['%2e%2e/x', '/%252e%252e/x'], // the text `%2e%2e`, which is not `..` + ["it's", "/it's"], + ])('page %j is written as %s, in both address shapes', (page, written) => { + const short = projectUrl({ storeId: 'github.com', projectId: 'r@o@' }, page); + expect(short).toBe('/project/github.com/o/r' + written); + const legacy = projectUrl({ storeId: 'localhost:8989', projectId: 'p' }, page); + expect(legacy).toBe('/store/localhost:8989/project/p' + written); + for (const url of [short, legacy]) { + expect(parsed(url)).toMatchObject({ rest: written, isCanonical: true }); + } + }); + + it.each(['.', '..', 'a/../b', 'a/./b', '', '/', 'a//b', 'chat/', '\u0000', 'a\\..'])( + 'still refuses the page %j', + (page) => { + if (page === '' || page === '/') { + // No page at all is the project itself. + expect(typeof tryProjectUrl({ storeId: 'github.com', projectId: 'r@o@' }, page)).toBe('string'); + return; + } + expect(tryProjectUrl({ storeId: 'github.com', projectId: 'r@o@' }, page)).toMatchObject({ ok: false }); + }, + ); +}); + describe('projectUrl refuses what it cannot write exactly (design review S2)', () => { it.each([ // [store, id, page] @@ -790,7 +826,6 @@ describe('projectUrl refuses what it cannot write exactly (design review S2)', ( ['github.com', 'r@o@', 'a//b'], ['github.com', 'r@o@', '..'], ['github.com', 'r@o@', 'a/../b'], - ['github.com', 'r@o@', '%2e%2e/x'], ['github.com', 'r@o@', '../../evil/repo'], ['github.com', 'r@o@d', 'a/./b'], ['GitHub.com', 'r@o@', undefined], @@ -896,7 +931,14 @@ describe('projectUrl refuses what it cannot write exactly (design review S2)', ( `${result} was built for ${JSON.stringify([storeId, projectId, page])} and does not parse: ${back.reason}`, ); } - const wantRest = page ? '/' + page.replace(/^\//, '') : ''; + const wantRest = page + ? '/' + + page + .replace(/^\//, '') + .split('/') + .map((segment) => encodeURIComponent(segment)) + .join('/') + : ''; const github = storeId === 'github.com' || storeId === 'github'; const same = back.rest === wantRest && diff --git a/libs/datatug/project-address/src/lib/project-url.ts b/libs/datatug/project-address/src/lib/project-url.ts new file mode 100644 index 00000000..856399ba --- /dev/null +++ b/libs/datatug/project-address/src/lib/project-url.ts @@ -0,0 +1,461 @@ +// Project addresses (design `demo-as-github-project.md` 3.1, 3.3, 3.4, 3.4a). Pure: no Angular, no I/O. +// +// This file and github-project-address.ts live in their own library so that the app's eager start-up code can use +// them (the app may not import the lazy-loaded datatug-main library statically); `nav-models.ts` of datatug-main +// re-exports them under their old names. + +import { + asciiLowerCase, + formatGithubProjectId, + GITHUB_DEFAULT_BRANCH_REF, + GITHUB_OWNER_PATTERN, + GITHUB_REPO_PATTERN, + GITHUB_STORE_ID, + IGithubProjectId, + isSafePathSegment, + isValidGithubFolder, + isValidGithubRef, + readGithubProjectId, +} from './github-project-address'; + +/** What a project address names: a store and a project id, as every page reads them. */ +export interface IProjectRef { + readonly storeId: string; + readonly projectId: string; +} + +export const getStoreId = (repo: string): string => { + return (repo || '').replace(/(https?):\/\//, '$1-'); +}; + +// --------------------------------------------------------------------------- +// Project addresses (design `demo-as-github-project.md` 3.1, 3.3, 3.4, 3.4a) +// +// Two shapes of a project path exist, and these functions are the one place that knows them: +// - GitHub projects: `/project/github.com//[/tree/[/…]][/-/…]` (canonical); +// - every store, GitHub included for as long as it is accepted: `/store//project/[/…]`. +// Both are paths (a `Location.pathname`), never a path with a query string or a fragment. +// +// Contract: for every `{storeId, projectId}` and page that `projectUrl` accepts, `parseProjectUrl` reads the +// returned path back as that same project (the canonical id) and page; everything else is refused, never turned +// into the address of another project. +// --------------------------------------------------------------------------- + +const PROJECT_FILE_NAME = 'datatug-project.json'; + +/** Why an address is not accepted. The app words each one for the visitor (design 3.4a). */ +export type ProjectUrlErrorReason = + /** Not a project address at all (another route). Not an error to show. */ + | 'not-a-project-address' + /** The owner or the repo is not a name GitHub allows (this also covers `%2F`, `.git.git`, unicode look-alikes). */ + | 'invalid-owner-or-repo' + /** A segment that cannot be part of a project locator or a page: `.`/`..`, an encoded `/` or `%`, a control character, leading or trailing whitespace, `?`, `#`, an empty segment, a `-` where a page was expected. */ + | 'invalid-path-segment' + /** `@` in a directory or a ref: it separates the parts of the project id. */ + | 'at-sign-not-supported' + /** A directory segment named `-`: the first `-` ends the project locator. */ + | 'dash-directory-not-supported' + /** `blob/…` of anything but the project file: a link to a file. */ + | 'file-link' + /** `…/tree` with no ref. */ + | 'missing-ref' + /** A spelling of the GitHub store id other than `github.com` or `github` (`GitHub.com`): never a different store. */ + | 'unsupported-store-id' + /** `projectUrl` only: the id or the page has no exact address. */ + | 'not-representable'; + +export interface IProjectUrlError { + readonly ok: false; + readonly reason: ProjectUrlErrorReason; +} + +/** What `projectUrl` throws. */ +export class ProjectUrlError extends Error { + constructor(public readonly reason: ProjectUrlErrorReason) { + super(`no exact project address (${reason})`); + this.name = 'ProjectUrlError'; + } +} + +/** The parts of a GitHub project: owner and repo lower case; `folder` as in the id (`''` = repo root); `ref` only when it is not the default branch. */ +export interface IGithubProjectParts { + readonly owner: string; + readonly repo: string; + readonly folder: string; + readonly ref?: string; +} + +export interface IProjectUrlParts { + readonly ok: true; + readonly storeId: string; + readonly projectId: string; + /** What follows the project locator: `''`, or a path beginning with `/` (`/chat`), every segment checked (not empty, not `.` or `..`). Left as typed (still percent-encoded). */ + readonly rest: string; + /** `short` = `/project/github.com/…`; `legacy` = `/store//project/`. */ + readonly shape: 'short' | 'legacy'; + /** The one address of this project and page (design 3.4a), `rest` kept. */ + readonly canonicalPath: string; + /** False when the typed path is another spelling of `canonicalPath` (case, `.git`, a trailing slash, `tree/HEAD`, a `blob/` link, the legacy GitHub shape). Always true for a non-GitHub store (one shape, left as typed). */ + readonly isCanonical: boolean; + /** + * For a GitHub project: its parts, so that no caller splits the id again. `ref` is the typed branch, tag or + * commit when it is not `HEAD`; when it names the repo's default branch the caller must (with one GitHub API + * call) rewrite it to `HEAD`: it cannot be known here. + */ + readonly github?: IGithubProjectParts; +} + +function isGithubStoreId(storeId: string): boolean { + return storeId === GITHUB_STORE_ID || storeId === 'github'; +} + +function decodeSegment(raw: string): string | undefined { + try { + return decodeURIComponent(raw); + } catch { + return undefined; + } +} + +/** A project locator segment, decoded once; `undefined` when it cannot be one. */ +function decodeLocatorSegment(raw: string): string | undefined { + const decoded = decodeSegment(raw); + return isSafePathSegment(decoded) ? decoded : undefined; +} + +/** + * A page segment, as typed: it must decode, hold no control character, and (decoded, split at `/` and `\`) no + * empty, `.` or `..` piece. A `%2F` is how a host turns a name into a path, so it is judged as the path it becomes. + */ +function isSafeRestSegment(raw: string): boolean { + const decoded = decodeSegment(raw); + if (decoded === undefined || decoded === '') { + return false; + } + for (let i = 0; i < decoded.length; i++) { + const code = decoded.charCodeAt(i); + if (code < 0x20 || code === 0x7f) { + return false; + } + } + return decoded + .split(/[\\/]/) + .every((p) => p !== '' && p !== '.' && p !== '..'); +} + +/** `''`, or `/seg/seg` for checked raw segments; `undefined` when a segment is not acceptable. */ +function restOf(segments: readonly string[]): string | undefined { + if (!segments.every(isSafeRestSegment)) { + return undefined; + } + return segments.length > 0 ? '/' + segments.join('/') : ''; +} + +const encodeSegments = (segments: readonly string[]): string => + segments.map((s) => encodeURIComponent(s)).join('/'); + +/** The canonical short path of a valid (strictly read) GitHub project and a checked `rest`. */ +function shortGithubPath(project: IGithubProjectId, rest: string): string { + const base = `/project/${GITHUB_STORE_ID}/${project.org}/${project.repo}`; + // A page that starts with `tree`, `blob` or `-` would read as part of the locator: spell the tree form out. + const ambiguous = /^\/(?:tree|blob|-)(?:\/|$)/.test(rest); + if (project.ref === undefined && project.folder === '' && !ambiguous) { + return base + rest; + } + const ref = encodeURIComponent(project.ref ?? GITHUB_DEFAULT_BRANCH_REF); + const dir = project.folder + ? `/${encodeSegments(project.folder.split('/'))}` + : ''; + return `${base}/tree/${ref}${dir}${rest ? '/-' + rest : ''}`; +} + +function githubParts(project: IGithubProjectId): IGithubProjectParts { + return { + owner: project.org, + repo: project.repo, + folder: project.folder, + ...(project.ref !== undefined ? { ref: project.ref } : {}), + }; +} + +/** + * The path of a project, and of one of its pages (`page` is `'chat'`, `'queries/x'`, with or without ONE leading + * `/`; plain text, each segment is percent-encoded: pass `'query/a b'`, never `'query/a%20b'`), or the reason there + * is none. A GitHub project gets its canonical short address + * (`/project/github.com//…`); every other store `/store//project/`. + * + * The address is returned only when `parseProjectUrl` reads it back as the same project and page: an id with + * more than four parts, a folder or ref holding a `/`, a `-` folder segment, a page with an empty or `..` segment, + * a case variant of the store id (`GitHub.com`) have no exact address and are refused. A GitHub id comes back in + * its canonical spelling (`r@o@datatug` is `r@o`). + */ +export function tryProjectUrl( + ref: IProjectRef, + page?: string, +): string | IProjectUrlError { + const refused = (reason: ProjectUrlErrorReason): IProjectUrlError => ({ + ok: false, + reason, + }); + const typedPage = + typeof page === 'string' && page.startsWith('/') + ? page.slice(1) + : (page ?? ''); + // `page` is plain text: every segment is percent-encoded here (a space, `%`, `?`, `#` or non-ASCII letter in a + // page is written, never left to be read as something else), and the result must read back as this very page. + const rest = typedPage + ? restOf(typedPage.split('/').map((s) => encodeURIComponent(s))) + : ''; + if (rest === undefined) { + return refused('invalid-path-segment'); + } + const storeId = ref?.storeId; + const projectId = ref?.projectId; + if (typeof storeId !== 'string' || typeof projectId !== 'string') { + return refused('not-representable'); + } + let candidate: string; + let expectedStoreId: string; + let expectedProjectId: string; + if (isGithubStoreId(storeId)) { + const reading = readGithubProjectId(projectId); + if (!reading.ok) { + return refused('not-representable'); + } + candidate = shortGithubPath(reading.id, rest); + expectedStoreId = GITHUB_STORE_ID; + expectedProjectId = formatGithubProjectId(reading.id); + } else { + expectedStoreId = getStoreId(storeId); + expectedProjectId = projectId; + candidate = `/store/${expectedStoreId}/project/${projectId}${rest}`; + } + const back = parseProjectUrl(candidate); + if (!back.ok) { + return refused(back.reason); + } + if ( + back.storeId !== expectedStoreId || + back.projectId !== expectedProjectId || + back.rest !== rest || + back.canonicalPath !== candidate + ) { + return refused('not-representable'); + } + return candidate; +} + +/** {@link tryProjectUrl}, throwing a {@link ProjectUrlError} when there is no exact address. */ +export function projectUrl(ref: IProjectRef, page?: string): string { + const result = tryProjectUrl(ref, page); + if (typeof result !== 'string') { + throw new ProjectUrlError(result.reason); + } + return result; +} + +/** + * Reads a project path (a pathname: no query, no fragment) in either shape into the `{storeId, projectId}` pair + * every page already reads from the route, plus what follows (`rest`), the one canonical spelling, and, for an + * address that cannot be a project, why (`ok: false`). Pure: it makes no request, so whether a named ref is the + * default branch is left to the caller (`github.ref`). + */ +export function parseProjectUrl( + path: string, +): IProjectUrlParts | IProjectUrlError { + const notOurs: IProjectUrlError = { + ok: false, + reason: 'not-a-project-address', + }; + if (typeof path !== 'string' || !path.startsWith('/')) { + return notOurs; + } + const segments = path.slice(1).split('/'); + if (segments[0] === 'project') { + return segments[1] === GITHUB_STORE_ID + ? parseShortGithubPath(path, segments) + : notOurs; + } + if ( + segments[0] === 'store' && + segments[2] === 'project' && + segments[1] && + segments[3] + ) { + return parseLegacyPath(path, segments); + } + return notOurs; +} + +/** Drops ONE trailing empty segment (a trailing slash); any further empty segment is then refused as a page segment. */ +function withoutTrailingSlash(segments: string[]): string[] { + return segments.length > 0 && segments[segments.length - 1] === '' + ? segments.slice(0, -1) + : segments; +} + +function parseLegacyPath( + path: string, + segments: string[], +): IProjectUrlParts | IProjectUrlError { + const storeId = decodeLocatorSegment(segments[1]); + const projectId = decodeLocatorSegment(segments[3]); + if (storeId === undefined || projectId === undefined) { + return { ok: false, reason: 'invalid-path-segment' }; + } + const rest = restOf(withoutTrailingSlash(segments.slice(4))); + if (rest === undefined) { + return { ok: false, reason: 'invalid-path-segment' }; + } + const lowerStoreId = asciiLowerCase(storeId); + if (!isGithubStoreId(lowerStoreId)) { + return { + ok: true, + storeId, + projectId, + rest, + shape: 'legacy', + canonicalPath: path, + isCanonical: true, + }; + } + // `GitHub.com` is neither this store nor another one: refused, so no spelling can pass for GitHub. + if (!isGithubStoreId(storeId)) { + return { ok: false, reason: 'unsupported-store-id' }; + } + const reading = readGithubProjectId(projectId); + if (!reading.ok) { + const partCount = projectId.split('@').length; + return { + ok: false, + reason: + reading.reason === 'owner-or-repo' + ? 'invalid-owner-or-repo' + : reading.reason === 'parts' + ? partCount < 2 + ? 'not-a-project-address' + : 'at-sign-not-supported' + : 'invalid-path-segment', + }; + } + const canonicalPath = shortGithubPath(reading.id, rest); + return { + ok: true, + storeId: GITHUB_STORE_ID, + projectId: formatGithubProjectId(reading.id), + rest, + shape: 'legacy', + canonicalPath, + isCanonical: canonicalPath === path, + github: githubParts(reading.id), + }; +} + +function parseShortGithubPath( + path: string, + segments: string[], +): IProjectUrlParts | IProjectUrlError { + const raw = withoutTrailingSlash(segments.slice(2)); + if (raw.length < 2) { + return { ok: false, reason: 'not-a-project-address' }; + } + const owner = decodeLocatorSegment(raw[0]); + let repo = decodeLocatorSegment(raw[1]); + if ( + repo !== undefined && + repo.length >= 4 && + asciiLowerCase(repo).endsWith('.git') + ) { + repo = repo.slice(0, -4); + } + // A repo name never ends in `.git` (so `r.git.git` is refused here), and both names are ASCII. + if ( + owner === undefined || + repo === undefined || + !GITHUB_OWNER_PATTERN.test(owner) || + !GITHUB_REPO_PATTERN.test(repo) || + asciiLowerCase(repo).endsWith('.git') + ) { + return { ok: false, reason: 'invalid-owner-or-repo' }; + } + const after = raw.slice(2); + let ref: string | undefined; + const dirs: string[] = []; + let restSegments: string[] = after; + + if (after[0] === 'tree' || after[0] === 'blob') { + const isBlob = after[0] === 'blob'; + if (after[1] === undefined) { + return { ok: false, reason: isBlob ? 'file-link' : 'missing-ref' }; + } + const refSegment = decodeLocatorSegment(after[1]); + if (refSegment === undefined) { + return { ok: false, reason: 'invalid-path-segment' }; + } + if (refSegment.includes('@')) { + return { ok: false, reason: 'at-sign-not-supported' }; + } + ref = refSegment; + let tail = after.slice(2); + restSegments = []; + if (isBlob) { + // A pasted link to the project file itself: `blob///datatug-project.json`. + if (tail[tail.length - 1] !== PROJECT_FILE_NAME) { + return { ok: false, reason: 'file-link' }; + } + tail = tail.slice(0, -1); + } else { + const dash = tail.indexOf('-'); + if (dash !== -1) { + restSegments = tail.slice(dash + 1); + tail = tail.slice(0, dash); + } + } + for (const segment of tail) { + const dir = decodeLocatorSegment(segment); + if (dir === undefined) { + return { ok: false, reason: 'invalid-path-segment' }; + } + if (dir === '-') { + return { ok: false, reason: 'dash-directory-not-supported' }; + } + if (dir.includes('@')) { + return { ok: false, reason: 'at-sign-not-supported' }; + } + dirs.push(dir); + } + } else if (after[0] === '-') { + return { ok: false, reason: 'invalid-path-segment' }; + } + + const rest = restOf(restSegments); + if (rest === undefined) { + return { ok: false, reason: 'invalid-path-segment' }; + } + const folder = dirs.join('/'); + const named = + ref !== undefined && ref !== GITHUB_DEFAULT_BRANCH_REF ? ref : undefined; + if ( + !isValidGithubFolder(folder) || + (named !== undefined && !isValidGithubRef(named)) + ) { + return { ok: false, reason: 'invalid-path-segment' }; + } + const project: IGithubProjectId = { + repo: asciiLowerCase(repo), + org: asciiLowerCase(owner), + folder, + ...(named !== undefined ? { ref: named } : {}), + }; + const canonicalPath = shortGithubPath(project, rest); + return { + ok: true, + storeId: GITHUB_STORE_ID, + projectId: formatGithubProjectId(project), + rest, + shape: 'short', + canonicalPath, + isCanonical: canonicalPath === path, + github: githubParts(project), + }; +} diff --git a/libs/datatug/project-address/src/public_api.ts b/libs/datatug/project-address/src/public_api.ts new file mode 100644 index 00000000..b696c2d5 --- /dev/null +++ b/libs/datatug/project-address/src/public_api.ts @@ -0,0 +1,6 @@ +// Public API entry for @datatug/project-address. +// +// For AI agents: when adding new lines to the file add them as comments for manual human review. + +export * from './lib/github-project-address'; +export * from './lib/project-url'; diff --git a/libs/datatug/project-address/tsconfig.json b/libs/datatug/project-address/tsconfig.json new file mode 100644 index 00000000..f4b1fb1e --- /dev/null +++ b/libs/datatug/project-address/tsconfig.json @@ -0,0 +1,22 @@ +{ + "extends": "../../../tsconfig.base.json", + "compilerOptions": { + "moduleResolution": "bundler", + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true, + "module": "esnext" + }, + "files": [], + "include": [], + "references": [ + { + "path": "./tsconfig.lib.json" + }, + { + "path": "./tsconfig.spec.json" + } + ] +} diff --git a/libs/datatug/project-address/tsconfig.lib.json b/libs/datatug/project-address/tsconfig.lib.json new file mode 100644 index 00000000..e0cb5ac5 --- /dev/null +++ b/libs/datatug/project-address/tsconfig.lib.json @@ -0,0 +1,18 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../../dist/out-tsc", + "declaration": true, + "declarationMap": true, + "inlineSources": true, + "resolveJsonModule": true, + "types": [] + }, + "exclude": [ + "src/**/*.spec.ts", + "src/test-setup.ts", + "src/**/*.test.ts", + "src/lib/__fixtures__/**" + ], + "include": ["src/**/*.ts"] +} diff --git a/libs/datatug/project-address/tsconfig.lib.prod.json b/libs/datatug/project-address/tsconfig.lib.prod.json new file mode 100644 index 00000000..46634e44 --- /dev/null +++ b/libs/datatug/project-address/tsconfig.lib.prod.json @@ -0,0 +1,6 @@ +{ + "extends": "./tsconfig.lib.json", + "compilerOptions": { + "declarationMap": false + } +} diff --git a/libs/datatug/project-address/tsconfig.spec.json b/libs/datatug/project-address/tsconfig.spec.json new file mode 100644 index 00000000..b3c904dc --- /dev/null +++ b/libs/datatug/project-address/tsconfig.spec.json @@ -0,0 +1,21 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "outDir": "../../../dist/out-tsc", + "module": "esnext", + "target": "es2016", + "types": ["node", "vitest/globals", "vitest/importMeta", "vite/client", "vitest"], + "moduleResolution": "bundler", + "resolveJsonModule": true + }, + "include": [ + "vite.config.ts", + "vite.config.mts", + "vitest.config.ts", + "vitest.config.mts", + "src/**/*.test.ts", + "src/**/*.spec.ts", + "src/**/*.d.ts", + "src/lib/__fixtures__/**/*.json" + ] +} diff --git a/libs/datatug/project-address/vite.config.mts b/libs/datatug/project-address/vite.config.mts new file mode 100644 index 00000000..b07e9e60 --- /dev/null +++ b/libs/datatug/project-address/vite.config.mts @@ -0,0 +1,52 @@ +/// +import { defineConfig } from 'vitest/config'; +import { nxViteTsPaths } from '@nx/vite/plugins/nx-tsconfig-paths.plugin'; +import { join } from 'path'; + +// Unlike libs/datatug/semantic (an Angular component library), this is a +// plain TypeScript types package with no Angular/DOM surface, so it skips +// `createBaseViteConfig` (which wires the Angular JIT plugin + happy-dom + +// Firestore/TestBed test-setup) in favour of a minimal node-environment +// config — that base config's Angular-specific pieces don't apply here. + +const dirname = new URL('.', import.meta.url).pathname; +const rootPath = process.cwd(); +const relativeToRoot = join( + dirname, + Array(dirname.replace(rootPath, '').split('/').filter(Boolean).length) + .fill('..') + .join('/'), + 'node_modules/.vite', + dirname.replace(rootPath, ''), +); +const coverageDir = join( + dirname, + Array(dirname.replace(rootPath, '').split('/').filter(Boolean).length) + .fill('..') + .join('/'), + 'coverage', + dirname.replace(rootPath, ''), +); + +export default defineConfig({ + root: dirname, + cacheDir: relativeToRoot, + plugins: [nxViteTsPaths()], + test: { + name: 'datatug-project-address', + watch: false, + globals: true, + environment: 'node', + include: ['src/**/*.spec.ts'], + reporters: ['default'], + coverage: { + enabled: true, + reportsDirectory: coverageDir, + provider: 'v8', + reporter: ['text', 'json', 'json-summary', 'html', 'lcov'], + all: true, + include: ['src/**/*.ts'], + exclude: ['src/**/*.spec.ts', 'src/**/index.ts'], + }, + }, +}); diff --git a/tsconfig.base.json b/tsconfig.base.json index 50afc738..9f9ba73e 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -25,6 +25,7 @@ "skipLibCheck": true, "paths": { "@datatug/board-models": ["./libs/datatug/board-models/src/public_api.ts"], + "@datatug/project-address": ["./libs/datatug/project-address/src/public_api.ts"], "@datatug/product-profiles": ["./libs/datatug/product-profiles/src/public_api.ts"], "@sneat/datatug-main": ["./libs/datatug/main/src/public_api.ts"], "@sneat/datatug-semantic": ["./libs/datatug/semantic/src/public_api.ts"], From f1ffcf4eb8452dcb1d994220a6ba514f7f0e6c35 Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Fri, 2 Oct 2026 22:41:47 +0100 Subject: [PATCH 2/8] fix(project): the new-project form refuses a folder the reader would refuse (#180) The folder field of the GitHub tab is checked with readNewProjectFolder before anything is created on GitHub (before a new repository too): `..`, a leading slash, a backslash, an empty or `-` segment, `@`, `%`, `?` and `#` give a form error instead of writing to a folder that cannot be opened. The checked, trimmed folder is what the create service receives. Co-Authored-By: Claude Sonnet 5.5 --- .../new-project-form.component.spec.ts | 68 +++++++++++++++++++ .../new-project/new-project-form.component.ts | 25 +++++-- 2 files changed, 89 insertions(+), 4 deletions(-) diff --git a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts index abbb963d..f5989c1d 100644 --- a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts +++ b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts @@ -351,6 +351,74 @@ describe('NewProjectFormComponent creating in a GitHub repo', () => { expect(formErrorOf(component)).toContain('name'); }); + describe('the folder field (issue #180)', () => { + const prepare = (folder: string, repo = 'datatug/demo-projects') => { + oauth.isSignedIn = true; + oauth.accessToken = 'gho_token'; + (component as unknown as { selectedRepo: { set: (v: string) => void } }).selectedRepo.set(repo); + repos.createRepo = vi.fn(() => + of({ fullName: 'datatug/my-projects', private: false, defaultBranch: 'main' }), + ); + component.newRepoName = 'my-projects'; + component.store = 'github'; + component.title = 'My project'; + component.githubFolder = folder; + }; + + it.each([ + ['/', 'leading "/"'], + ['/datatug', 'leading "/"'], + [' /datatug', 'leading "/"'], + ['..', 'cannot contain'], + ['../x', 'cannot contain'], + ['a/../b', 'cannot contain'], + ['a//b', 'cannot contain'], + ['a\\b', 'cannot contain'], + ['-', 'cannot contain'], + ['a@b', 'cannot contain'], + ['a?b', 'cannot contain'], + ])('refuses the folder %j before anything is written or created (%s)', (folder, message) => { + for (const repo of ['datatug/demo-projects', '__new__']) { + prepare(folder, repo); + + component.create(); + + expect(createProject).not.toHaveBeenCalled(); + expect(repos.createRepo).not.toHaveBeenCalled(); + expect(formErrorOf(component)).toContain(message); + } + }); + + it.each([ + ['', 'datatug'], + [' ', 'datatug'], + [' demo-project-1 ', 'demo-project-1'], + ['a/b/', 'a/b'], + ])('writes the folder %j as %j', (typed, folder) => { + prepare(typed); + + component.create(); + + expect(createProject).toHaveBeenCalledWith( + expect.objectContaining({ folder }), + 'gho_token', + ); + expect(formErrorOf(component)).toBeUndefined(); + }); + + it('also checks the folder before a new repository is created, and uses the checked folder after it', () => { + prepare(' sub/dir/ ', '__new__'); + + component.create(); + + expect(repos.createRepo).toHaveBeenCalled(); + expect(createProject).toHaveBeenCalledWith( + expect.objectContaining({ repo: 'my-projects', folder: 'sub/dir' }), + 'gho_token', + ); + }); + }); + it('still creates in the cloud store when Cloud is selected', () => { component.store = 'cloud'; component.title = 'My project'; diff --git a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts index a8bd1431..e15e4c61 100644 --- a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts +++ b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts @@ -19,6 +19,7 @@ import { IonToggle, IonToolbar, } from '@ionic/angular'; +import { readNewProjectFolder } from '@datatug/project-address'; import { STORE_ID_GITHUB_COM } from '@sneat/core'; import { ErrorLogger, IErrorLogger } from '@sneat/core'; import { IProjectContext, parseDatatugStoreRef } from '../../nav/nav-models'; @@ -235,6 +236,17 @@ export class NewProjectFormComponent implements ViewDidEnter { this.formError.set('Select a repository, or create a new one.'); return; } + // Before anything is created on GitHub: a folder that the reader (and so the project's address) would refuse + // must never be written to. + const folder = readNewProjectFolder(this.githubFolder); + if (!folder.ok) { + this.formError.set( + folder.reason === 'leading-slash' + ? 'The folder is relative to the repository root: remove the leading "/".' + : 'The folder cannot contain "..", empty or "-" folders, or any of \\ @ % ? #.', + ); + return; + } if (repo === NEW_GITHUB_REPO) { const name = this.newRepoName.trim(); if (!name) { @@ -245,7 +257,8 @@ export class NewProjectFormComponent implements ViewDidEnter { this.githubReposService .createRepo(token, name, this.makeRepoPrivate) .subscribe({ - next: (created) => this.commitGithubProject(created.fullName, token), + next: (created) => + this.commitGithubProject(created.fullName, token, folder.folder), error: (err) => { this.isCreating.set(false); this.formError.set( @@ -257,10 +270,14 @@ export class NewProjectFormComponent implements ViewDidEnter { return; } this.isCreating.set(true); - this.commitGithubProject(repo, token); + this.commitGithubProject(repo, token, folder.folder); } - private commitGithubProject(fullName: string, token: string): void { + private commitGithubProject( + fullName: string, + token: string, + folder: string, + ): void { const [org, repo] = fullName.split('/'); if (!org || !repo) { this.isCreating.set(false); @@ -269,7 +286,7 @@ export class NewProjectFormComponent implements ViewDidEnter { } this.githubProjectCreateService .createProject( - { org, repo, folder: this.githubFolder, title: this.title }, + { org, repo, folder, title: this.title }, token, ) .subscribe({ From 507a69451d6e58325cec23bc43aee41974445538 Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Fri, 2 Oct 2026 22:41:55 +0100 Subject: [PATCH 3/8] feat(project): short address for GitHub projects (G-A1b) A GitHub project opens at /project/github.com//[/tree/[/]/-/] with the existing project pages unchanged. The old /store/github.com/project/ form keeps working, with no redirect (G-A1d is held). datatug-main: - a matcher route (githubProjectMatcher) consumes the locator and supplies storeId and projectId; the project pages are its children; - a second matcher route and canMatch (github-project-address-check.ts) decide, per design 3.4a: redirect any other spelling to the canonical address (query and fragment kept), look up the default branch once when the address names a branch and redirect to the HEAD spelling when it is that branch (a refused lookup leaves the address as typed and tells the visit in a toast), and show a page that says so for each unsupported input and for a repository with no project file ("No DataTug project here", a hint for a branch name with a slash); - DatatugNavContextService reads a short address that the short route opened as the old form of the same project, so the side menu and the pages that read the nav context see the same store, project, environment and table; - the DataTug profile only: Incidentius does not match the short route. datatug-app (the holding page keeps working exactly as today): - /demo always shows the holding page; a project chat address shows it only when it arrived with a question (msg, or q), decided from what index.html kept of the query, so the same address with no question is the project's own chat page; after a reload it is the same (storage keeps the mark `asked=1` for an address that may not echo its question). The demo flag is not consulted; - isEchoTrusted is replaced by isTrustedHandoff, which parses the address and asks the one trust function (isTrustedProjectAddress): the Kelvin sign U+212A no longer passes for `k` (live on main), with the regression test (#180). Co-Authored-By: Claude Sonnet 5.5 --- apps/datatug-app/e2e/demo-handoff.spec.ts | 144 ++++- .../src/app/datatug-app-routes.spec.ts | 90 +++ .../datatug-app/src/app/datatug-app-routes.ts | 24 +- apps/datatug-app/src/app/demo-flag.spec.ts | 17 +- .../src/app/demo-handoff-asked.spec.ts | 274 +++++++++ .../datatug-app/src/app/demo-handoff-asked.ts | 141 +++++ .../src/app/demo-handoff-capture.spec.ts | 55 +- .../src/app/demo-handoff-capture.ts | 68 ++- .../src/app/demo-holding-page.component.ts | 6 +- .../src/lib/routes/datatug-routing.module.ts | 4 + ...hub-address-problem-page.component.spec.ts | 211 +++++++ .../github-address-problem-page.component.ts | 184 ++++++ .../github-project-address-check.spec.ts | 577 ++++++++++++++++++ .../routes/github-project-address-check.ts | 270 ++++++++ .../lib/routes/github-project-routes.spec.ts | 434 +++++++++++++ .../src/lib/routes/github-project-routes.ts | 107 ++++ .../nav/datatug-nav-context.service.spec.ts | 103 ++++ .../nav/datatug-nav-context.service.ts | 53 +- 18 files changed, 2699 insertions(+), 63 deletions(-) create mode 100644 apps/datatug-app/src/app/demo-handoff-asked.spec.ts create mode 100644 apps/datatug-app/src/app/demo-handoff-asked.ts create mode 100644 libs/datatug/main/src/lib/routes/github-address-problem-page.component.spec.ts create mode 100644 libs/datatug/main/src/lib/routes/github-address-problem-page.component.ts create mode 100644 libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts create mode 100644 libs/datatug/main/src/lib/routes/github-project-address-check.ts create mode 100644 libs/datatug/main/src/lib/routes/github-project-routes.spec.ts create mode 100644 libs/datatug/main/src/lib/routes/github-project-routes.ts diff --git a/apps/datatug-app/e2e/demo-handoff.spec.ts b/apps/datatug-app/e2e/demo-handoff.spec.ts index 9099b93c..cf3ddc07 100644 --- a/apps/datatug-app/e2e/demo-handoff.spec.ts +++ b/apps/datatug-app/e2e/demo-handoff.spec.ts @@ -543,6 +543,148 @@ test.describe('the hand-off holding page', () => { expect(errors).toEqual([]); }); } + + // G-A1b: the short project route and the holding page share the project chat address. With a question the + // holding page (the chat cannot run it yet, and it must not be lost); without one, the project's own chat page. + // The demo flag is off in this production build and is not consulted either way. + const TRUSTED = '/project/github.com/datatug/chinook-demo/chat'; + + test('the demo project chat address with no question is the project chat page, not the holding page, and keeps its URL', async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + const external = await stubExternal(context, (o) => o === origin); + await page.goto(TRUSTED); + await expect(page.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + await expect(page.locator('blockquote')).toHaveCount(0); + await expect(page).not.toHaveTitle('DataTug live demo'); + expect(new URL(page.url()).pathname).toBe(TRUSTED); + await page.waitForTimeout(1500); + await expect(page.locator(SENTRY_DIALOG)).toHaveCount(0); + expect(dialogRequested(external)).toBe(false); + }); + + for (const query of ['?lang=ru', '?msg=', '?msg=%20%20&utm_source=x']) { + test(`the demo project chat address with ${query} (no question) is the project chat page, and its query is stripped`, async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto(TRUSTED + query); + await expect(page.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + expect(new URL(page.url()).pathname + new URL(page.url()).search).toBe( + TRUSTED, + ); + }); + } + + test('a project page other than the chat opens the project, question or not', async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + for (const url of [ + '/project/github.com/datatug/chinook-demo', + '/project/github.com/datatug/chinook-demo/queries?msg=Hello', + '/project/github.com/datatug/chinook-demo/tree/HEAD/dir/-/chat', + ]) { + await page.goto(url); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + await page.waitForTimeout(500); + await expect(page.locator('#demo-holding-message'), url).toHaveCount(0); + } + }); + + test('a reload of a chat address that arrived with a question shows the holding page again, for the demo project and for any other repository', async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto(`${TRUSTED}?msg=${encodeURIComponent(EN_QUESTION)}`); + await expect(page.locator('blockquote')).toHaveText(EN_QUESTION, { + timeout: 20_000, + }); + await page.reload(); + await expect(page.locator('blockquote')).toHaveText(EN_QUESTION, { + timeout: 20_000, + }); + + const other = '/project/github.com/someone/else/chat'; + await page.goto(`${other}?msg=${encodeURIComponent(EN_QUESTION)}`); + await expect(page.locator('#demo-holding-message')).toHaveText( + 'This page is not available yet.', + { timeout: 20_000 }, + ); + await page.reload(); + await expect(page.locator('#demo-holding-message')).toHaveText( + 'This page is not available yet.', + { timeout: 20_000 }, + ); + expect(await page.content()).not.toContain(MARKER); + }); + + test('a fresh visit to the bare chat address after a question, in the same tab, is the project chat page', async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto(`${TRUSTED}?msg=${encodeURIComponent(EN_QUESTION)}`); + await expect(page.locator('blockquote')).toBeVisible({ timeout: 20_000 }); + await page.goto(TRUSTED); + await expect(page.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + }); + + test('a non-canonical spelling of a project address is replaced by the canonical one, query kept', async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto('/project/github.com/Datatug/Chinook-Demo/tree/HEAD/-/queries?x=1'); + await expect(page).toHaveURL( + /\/project\/github\.com\/datatug\/chinook-demo\/queries\?x=1(&|$)/, + { timeout: 20_000 }, + ); + }); + + test('the old form of a project address opens as before: no redirect', async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto('/store/github.com/project/chinook-demo@datatug@/queries'); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + await page.waitForTimeout(500); + expect(new URL(page.url()).pathname).toBe( + '/store/github.com/project/chinook-demo@datatug@/queries', + ); + }); }); test.describe('the question reaches no analytics or error report', () => { @@ -711,7 +853,7 @@ test.describe('the matrix-parameter address reaches no analytics either', () => test.describe('every other route behaves as it does on main', () => { for (const path of [ '/no-such-route-xyz?q=1', - '/project/github.com/datatug/chinook-demo', + // (`/project/github.com/datatug/chinook-demo` used to be here: since G-A1b it is the project, at its short address.) '/project/github.com/datatug/chinook-demo/chat/extra', '/demo/other?q=1', '/demo(menu:x)?q=1', // an outlet group: not a hand-off address, for the script and the router alike diff --git a/apps/datatug-app/src/app/datatug-app-routes.spec.ts b/apps/datatug-app/src/app/datatug-app-routes.spec.ts index 1eea60e8..8621a4d3 100644 --- a/apps/datatug-app/src/app/datatug-app-routes.spec.ts +++ b/apps/datatug-app/src/app/datatug-app-routes.spec.ts @@ -12,6 +12,11 @@ import { handoffOrRoot, routes, } from './datatug-app-routes'; +import { + DEMO_HANDOFF_KEY, + DEMO_HANDOFF_STASH, + resetHandoffAskedForTests, +} from './demo-handoff-asked'; import { handoffUrlMatcher } from './demo-handoff-path'; // Task 13 (S108) — see this file's own header comment in datatug-app-routes.ts. @@ -80,6 +85,8 @@ describe('DataTug app routes', () => { .filter((r) => r.matcher) .map((r) => ({ ...r, loadComponent: () => HandoffStub })), { path: '', pathMatch: 'full', component: HomeStub }, + // What the datatug-main routes make of a short project address that the hand-off route does not take. + { path: 'project/**', component: ProjectStub }, // The app shell's side menu: a named outlet with an empty path, which the router adds an empty group for. { path: '', outlet: 'menu', component: OtherStub }, { path: '**', component: OtherStub }, @@ -126,6 +133,87 @@ describe('DataTug app routes', () => { expect(result.url).toBe('/demo/other'); }); + // The short project route and the holding page share the project chat address. Which one an address gets is + // decided from the query that index.html took out of the address bar (the router never sees it), and the + // demo flag is not consulted: until the chat can run a question, the holding page is the only way not to lose it. + describe('the project chat address: the holding page only when it arrived with a question', () => { + const stash = (search: string) => { + (window as unknown as Record)[DEMO_HANDOFF_STASH] = search; + }; + beforeEach(() => { + resetHandoffAskedForTests(); + delete (window as unknown as Record)[DEMO_HANDOFF_STASH]; + window.sessionStorage.clear(); + }); + afterEach(() => { + delete (window as unknown as Record)[DEMO_HANDOFF_STASH]; + window.sessionStorage.clear(); + resetHandoffAskedForTests(); + }); + + it.each([ + '/project/github.com/datatug/chinook-demo/chat', + '/project/github.com/Datatug/Chinook-Demo/chat', + '/project/github.com/datatug/chinook-demo/tree/HEAD/-/chat', + '/project/github.com/someone/else/chat', + '/project/github.com/datatug/chinook-demo/tree/abc123/-/chat', + ])('%s: a question gets the holding page, no question gets the project', async (url) => { + for (const [search, expected] of [ + ['?msg=Hello', HandoffStub], + ['?q=Hello&lang=ru', HandoffStub], + ['?lang=ru', ProjectStub], + ['?msg=', ProjectStub], + ['?utm_source=x', ProjectStub], + ] as const) { + TestBed.resetTestingModule(); + resetHandoffAskedForTests(); + stash(search); + expect((await visit('datatug.app', url)).component, search).toBe(expected); + } + TestBed.resetTestingModule(); + resetHandoffAskedForTests(); + delete (window as unknown as Record)[DEMO_HANDOFF_STASH]; + expect((await visit('datatug.app', url)).component, 'no query').toBe(ProjectStub); + }); + + it('a reload of a question that was asked shows the holding page again; a fresh visit shows the project', async () => { + const url = '/project/github.com/datatug/chinook-demo/chat'; + window.sessionStorage.setItem(DEMO_HANDOFF_KEY, url + '?msg=Hello'); + expect((await visit('datatug.app', url)).component).toBe(HandoffStub); + }); + + it('every other address under a project is the project, question or not', async () => { + stash('?msg=Hello'); + for (const url of [ + '/project/github.com/datatug/chinook-demo', + '/project/github.com/datatug/chinook-demo/queries', + '/project/github.com/datatug/chinook-demo/tree/HEAD/dir/-/chat', + '/project/github.com/datatug/chinook-demo/chat/more', + ]) { + TestBed.resetTestingModule(); + expect((await visit('datatug.app', url)).component, url).toBe(ProjectStub); + } + }); + + it('/demo is the holding page with or without a question', async () => { + for (const search of ['?q=Hello', '?lang=ru', '']) { + TestBed.resetTestingModule(); + stash(search); + expect((await visit('datatug.app', '/demo')).component, search).toBe(HandoffStub); + } + }); + + it('at app.incidentius.com every project chat address still goes to the root, with or without a question', async () => { + for (const search of ['?msg=Hello', '?lang=ru']) { + TestBed.resetTestingModule(); + stash(search); + const result = await visit('app.incidentius.com', '/project/github.com/datatug/chinook-demo/chat'); + expect(result.component, search).toBe(HomeStub); + expect(result.url).toBe('/'); + } + }); + }); + it.each(['/demo(menu:x)', '/demo/(menu:x)', '/Demo(menu:x/y)'])( "an address with an outlet group (%s) is handled by neither profile's hand-off route", async (url) => { @@ -144,5 +232,7 @@ describe('DataTug app routes', () => { class HandoffStub {} @Component({ selector: 'sneat-stub-home', template: '' }) class HomeStub {} +@Component({ selector: 'sneat-stub-project', template: '' }) +class ProjectStub {} @Component({ selector: 'sneat-stub-other', template: '' }) class OtherStub {} diff --git a/apps/datatug-app/src/app/datatug-app-routes.ts b/apps/datatug-app/src/app/datatug-app-routes.ts index d0c4f9ed..926e4b7f 100644 --- a/apps/datatug-app/src/app/datatug-app-routes.ts +++ b/apps/datatug-app/src/app/datatug-app-routes.ts @@ -1,7 +1,8 @@ import { inject } from '@angular/core'; -import { Router, Routes, UrlTree } from '@angular/router'; +import { CanMatchFn, Router, Routes } from '@angular/router'; import { PRODUCT_PROFILE } from '@datatug/product-profiles'; import { cliChatCapability } from './cli-chat-capability'; +import { showsHoldingPage } from './demo-handoff-asked'; import { handoffUrlMatcher } from './demo-handoff-path'; // Task 13 (S108, spec/research/2026-09-09-layered-acl-reconciliation.md, @@ -18,10 +19,18 @@ import { handoffUrlMatcher } from './demo-handoff-path'; // chat's `?msg=` is the shape the hand-off is moving to. Until the live demo can answer a question, they show one // holding page instead of failing to match any route, which opens Sentry's crash-report dialog and loses the // visitor's question. No flag and no sign-in. demo-handoff-capture.ts explains how the question is taken out of -// the address bar before analytics starts, and which addresses may show it back (isEchoTrusted: `/demo` and the +// the address bar before analytics starts, and which addresses may show it back (isTrustedHandoff: `/demo` and the // demo project's own chat; any other repository gets neutral wording and no question). Registered ahead of the // root feature routes; every other path is matched exactly as before. // +// Which addresses show the holding page (demo-handoff-asked.ts decides, from what index.html kept of the query): +// - `/demo`: always. +// - `/project/github.com//[/tree//-]/chat`: only when it arrived with a question (`msg`, or +// `q`). Without one it is the chat page of that project, opened at its short address like every other page of +// it (the short project route, in the datatug-main routes). The question is the only thing that has nowhere +// else to go until the chat can run it, so it is what decides. +// - the old form of a project address, and every other short address, are never hand-offs. +// // One route with a matcher, not three `path`s: Angular's literal segments are case-sensitive, and `/Demo` must // show the page too, as must `/demo;x=1` (the router ignores matrix parameters). demo-handoff-path.ts holds the // rules, which index.html's inline script repeats so that the query is stripped for exactly these addresses. @@ -35,8 +44,15 @@ const demoHoldingPage = () => import('./demo-holding-page.component').then((m) = /** The hand-off page belongs to the DataTug product profile only. */ export const datatugProfileOnly = (): boolean => inject(PRODUCT_PROFILE).id === 'datatug'; -/** `canMatch` of the hand-off route: matches under the DataTug profile, sends every other profile to the root. */ -export const handoffOrRoot = (): boolean | UrlTree => datatugProfileOnly() || inject(Router).parseUrl('/'); +/** + * `canMatch` of the hand-off route: under the DataTug profile the holding page for `/demo` and for a project chat + * address that arrived with a question, and no match (the router goes on to the project routes) for a project chat + * address without one; under every other profile, the root. + */ +export const handoffOrRoot: CanMatchFn = (_route, segments) => + datatugProfileOnly() + ? showsHoldingPage(segments.map((segment) => segment.path)) + : inject(Router).parseUrl('/'); export const routes: Routes = [ { diff --git a/apps/datatug-app/src/app/demo-flag.spec.ts b/apps/datatug-app/src/app/demo-flag.spec.ts index 3491acf0..85acb80d 100644 --- a/apps/datatug-app/src/app/demo-flag.spec.ts +++ b/apps/datatug-app/src/app/demo-flag.spec.ts @@ -3,7 +3,9 @@ import { join } from 'node:path'; import { datatugDemoConfig as devConfig } from '../environments/environment'; import { datatugDemoConfig as prodConfig } from '../environments/environment.prod'; import { datatugDemoConfig as ssoConfig } from '../environments/environment.sso-e2e'; -import { isEchoTrusted, isHandoffPath } from './demo-handoff-capture'; +import { showsHoldingPage } from './demo-handoff-asked'; +import { isTrustedHandoff, isHandoffPath } from './demo-handoff-capture'; +import { routeSegments } from './demo-handoff-path'; import { DEMO_ENABLED_OVERRIDE_KEY, isDemoEnabled, @@ -219,22 +221,27 @@ describe('the demo flag (G-F1)', () => { '/store/x/project/y/chat', '/', ]; - const baseline = PATHS.map((p) => [isEchoTrusted(p), isHandoffPath(p)]); + const answers = (p: string) => [ + isTrustedHandoff(p), + isHandoffPath(p), + // which page a hand-off address gets is not the flag's either (it is decided by whether a question was asked) + showsHoldingPage(routeSegments(p)), + ]; + const baseline = PATHS.map(answers); it.each([[undefined], ['1'], ['0'], ['junk']])( 'the answers for every path are the same with the override at %j', (override) => { if (override !== undefined) window.localStorage.setItem(DEMO_ENABLED_OVERRIDE_KEY, override); - expect(PATHS.map((p) => [isEchoTrusted(p), isHandoffPath(p)])).toEqual( - baseline, - ); + expect(PATHS.map(answers)).toEqual(baseline); }, ); it('the trust and hand-off code does not import the flag, and the flag does not import it', () => { for (const file of [ 'demo-handoff-capture.ts', + 'demo-handoff-asked.ts', 'demo-handoff-path.ts', 'demo-holding-page.component.ts', 'datatug-app-routes.ts', diff --git a/apps/datatug-app/src/app/demo-handoff-asked.spec.ts b/apps/datatug-app/src/app/demo-handoff-asked.spec.ts new file mode 100644 index 00000000..cf6d7cb8 --- /dev/null +++ b/apps/datatug-app/src/app/demo-handoff-asked.spec.ts @@ -0,0 +1,274 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + AskedEnv, + cleanQuestion, + DEMO_HANDOFF_KEY, + DEMO_HANDOFF_STASH, + questionOfSearch, + resetHandoffAskedForTests, + searchAsksQuestion, + showsHoldingPage, +} from './demo-handoff-asked'; +import { routeSegments } from './demo-handoff-path'; + +// Which hand-off addresses show the holding page, and which are the project's own pages: the rule of +// datatug-app-routes.ts, decided from what index.html kept of the query (the router never sees it). + +const CHAT = '/project/github.com/datatug/chinook-demo/chat'; + +function env( + options: { + stash?: string; + stored?: string; + navigation?: string; + blocked?: boolean; + } = {}, +): AskedEnv { + return { + stash: + options.stash === undefined + ? {} + : { [DEMO_HANDOFF_STASH]: options.stash }, + storage: () => { + if (options.blocked) throw new Error('blocked'); + return { + getItem: (key: string) => + key === DEMO_HANDOFF_KEY ? (options.stored ?? null) : null, + } as unknown as Storage; + }, + navigationType: () => options.navigation, + }; +} + +const shows = (path: string, e?: AskedEnv): boolean => + showsHoldingPage(routeSegments(path), e); + +describe('which hand-off addresses show the holding page', () => { + beforeEach(() => resetHandoffAskedForTests()); + + describe('/demo: always, whatever the query or storage says', () => { + it.each(['/demo', '/Demo', '/demo/', '/demo;x=1'])('%s', (path) => { + expect(shows(path, env())).toBe(true); + expect(shows(path, env({ stash: '?lang=ru' }))).toBe(true); + expect(shows(path, env({ blocked: true, navigation: 'navigate' }))).toBe( + true, + ); + }); + }); + + describe('the project chat address: only when a question was asked', () => { + it.each([ + [CHAT], + ['/project/github.com/Datatug/Chinook-Demo/chat/'], + ['/project/github.com/datatug/chinook-demo/tree/HEAD/-/chat'], + ['/project/github.com/someone/else/chat'], + ['/project/github.com/datatug/chinook-demo/tree/abc123/-/chat'], + ])('%s', (path) => { + // arrived with a question (index.html stashed it) + for (const stash of [ + '?msg=Hello', + '?q=Hello', + '?msg=%20&q=Hello', + '?lang=ru&msg=Hello', + ]) { + resetHandoffAskedForTests(); + expect(shows(path, env({ stash })), stash).toBe(true); + } + // arrived with no question: the project's own chat page + for (const stash of [ + '?lang=ru', + '?msg=', + '?msg=%20%0A', + '?scenario=x', + '?other=1', + '?msg=%07', + ]) { + resetHandoffAskedForTests(); + expect(shows(path, env({ stash })), stash).toBe(false); + } + // no query at all: a fresh visit + resetHandoffAskedForTests(); + expect(shows(path, env({ navigation: 'navigate' }))).toBe(false); + resetHandoffAskedForTests(); + expect(shows(path, env())).toBe(false); + }); + + it('the mark kept for an address that may not echo its question counts as a question', () => { + expect(shows(CHAT, env({ stash: '?lang=ru&asked=1' }))).toBe(true); + expect(searchAsksQuestion('?asked=1')).toBe(true); + expect(searchAsksQuestion('?asked=0')).toBe(false); + expect(searchAsksQuestion('?asked')).toBe(false); + }); + }); + + describe('a reload or Back, from the copy kept in sessionStorage', () => { + it.each([ + ['reload', 'reload'], + ['back/forward', 'back_forward'], + ['an unknown navigation type', undefined], + ])( + 'after a %s the address that had a question shows the page again', + (_name, navigation) => { + expect( + shows(CHAT, env({ stored: CHAT + '?msg=Hello&lang=ru', navigation })), + ).toBe(true); + }, + ); + + it('also for an address that may not echo its question (the mark)', () => { + const path = '/project/github.com/someone/else/chat'; + expect( + shows( + path, + env({ stored: path + '?lang=ru&asked=1', navigation: 'reload' }), + ), + ).toBe(true); + }); + + it.each([ + ['only a language was kept', CHAT + '?lang=ru'], + ['nothing was kept', ''], + ['junk was kept', 'junk'], + [ + 'the question was asked at another address', + '/project/github.com/datatug/other/chat?msg=Hello', + ], + ['the question was asked on /demo', '/demo?q=Hello'], + ['the copy has no path', '?msg=Hello'], + ])('the page is not shown when %s', (_name, stored) => { + expect(shows(CHAT, env({ stored, navigation: 'reload' }))).toBe(false); + }); + + it('reads the path as the router does: case, a trailing slash, matrix parameters and encoding do not matter', () => { + for (const stored of [ + '/Project/GitHub.com/DATATUG/Chinook-Demo/Chat?msg=Hi', + '/project/github.com/datatug/chinook-demo/chat/?msg=Hi', + '/project/github.com/datatug/chinook-demo/chat;x=1?msg=Hi', + '/project/github.com/datatug/chinook%2Ddemo/chat?msg=Hi', + ]) { + expect(shows(CHAT, env({ stored, navigation: 'reload' })), stored).toBe( + true, + ); + } + }); + + it('a fresh visit (the document was navigated to, not reloaded) ignores what an earlier visit left in the tab', () => { + expect( + shows( + CHAT, + env({ stored: CHAT + '?msg=Hello', navigation: 'navigate' }), + ), + ).toBe(false); + }); + + it('blocked storage means the project page: the question lived only in the page', () => { + expect(shows(CHAT, env({ blocked: true, navigation: 'reload' }))).toBe( + false, + ); + }); + }); + + describe('what this page load has settled stays settled', () => { + it('after the question was taken from the stash, coming back to the address shows the page again', () => { + expect(shows(CHAT, env({ stash: '?msg=Hello' }))).toBe(true); + // the holding page has picked the stash up; the router matches the address again + expect(shows(CHAT, env({ navigation: 'navigate' }))).toBe(true); + expect( + shows('/project/github.com/datatug/chinook-demo/CHAT/', env()), + ).toBe(true); + }); + + it('an address that arrived with no question stays the project page, whatever storage later holds', () => { + expect(shows(CHAT, env({ stash: '?lang=ru' }))).toBe(false); + expect( + shows(CHAT, env({ stored: CHAT + '?msg=Hello', navigation: 'reload' })), + ).toBe(false); + }); + + it('is per address', () => { + expect(shows(CHAT, env({ stash: '?msg=Hello' }))).toBe(true); + expect( + shows( + '/project/github.com/datatug/other/chat', + env({ navigation: 'navigate' }), + ), + ).toBe(false); + }); + }); + + describe('not a hand-off address: never the holding page', () => { + it.each([ + '/', + '/project/github.com/datatug/chinook-demo', + '/project/github.com/datatug/chinook-demo/queries', + '/project/github.com/datatug/chinook-demo/chat/extra', + '/project/github.com/datatug/chinook-demo/tree/HEAD/dir/-/chat', + '/store/github.com/project/chinook-demo@datatug@/chat', + '/demo/other', + '/project/gitlab.com/datatug/chinook-demo/chat', + ])('%s', (path) => { + expect(shows(path, env({ stash: '?msg=Hello' }))).toBe(false); + }); + }); +}); + +describe('the page load it is asked about (the defaults: window, sessionStorage, navigation timing)', () => { + beforeEach(() => { + resetHandoffAskedForTests(); + window.sessionStorage.setItem(DEMO_HANDOFF_KEY, CHAT + '?msg=Hello'); + }); + afterEach(() => { + vi.restoreAllMocks(); + delete (window as unknown as Record)[DEMO_HANDOFF_STASH]; + window.sessionStorage.clear(); + resetHandoffAskedForTests(); + }); + + it('reads the stash on window', () => { + (window as unknown as Record)[DEMO_HANDOFF_STASH] = + '?q=Hello'; + expect(showsHoldingPage(routeSegments(CHAT))).toBe(true); + }); + + it('a reload (the navigation timing says so) reads sessionStorage', () => { + vi.spyOn(performance, 'getEntriesByType').mockReturnValue([ + { type: 'reload' } as PerformanceEntry, + ]); + expect(showsHoldingPage(routeSegments(CHAT))).toBe(true); + }); + + it('a navigation (the navigation timing says so) ignores sessionStorage', () => { + vi.spyOn(performance, 'getEntriesByType').mockReturnValue([ + { type: 'navigate' } as PerformanceEntry, + ]); + expect(showsHoldingPage(routeSegments(CHAT))).toBe(false); + }); + + it('a browser with no navigation timing is read as a reload: sessionStorage is used', () => { + vi.spyOn(performance, 'getEntriesByType').mockImplementation(() => { + throw new Error('unsupported'); + }); + expect(showsHoldingPage(routeSegments(CHAT))).toBe(true); + }); +}); + +describe('the question of a query string', () => { + it.each([ + ['?msg=Hello', 'Hello'], + ['?q=Hello', 'Hello'], + ['?msg=A&q=B', 'A'], + ['?msg=%20&q=B', 'B'], + ['?msg=%0A%0A%0A%0Ax%0A%0A%0A%0Ay', 'x\n\ny'], + ['?msg=a%00b', 'ab'], + ['?lang=ru', ''], + ['', ''], + ])('%j is %j', (search, question) => { + expect(questionOfSearch(search)).toBe(question); + }); + + it('cleans text like the parser of the holding page', () => { + expect(cleanQuestion(null)).toBe(''); + expect(cleanQuestion(' a\u0007b ')).toBe('ab'); + expect(cleanQuestion('a\t\nb')).toBe('a\t\nb'); + }); +}); diff --git a/apps/datatug-app/src/app/demo-handoff-asked.ts b/apps/datatug-app/src/app/demo-handoff-asked.ts new file mode 100644 index 00000000..4433c146 --- /dev/null +++ b/apps/datatug-app/src/app/demo-handoff-asked.ts @@ -0,0 +1,141 @@ +import { handoffTarget, routeSegments } from './demo-handoff-path'; + +// Whether a hand-off address arrived WITH a question, decided where the route table is read (the eager bundle), +// from what index.html's inline script kept of the query: it takes the query out of the address bar before the +// router ever sees it, so the router cannot ask the URL. This file is small on purpose (no parsing of trust, no +// storage writes, no reporting): demo-handoff-capture.ts, which is a lazy chunk, holds the rest. +// +// Which addresses show the holding page (datatug-app-routes.ts): +// - `/demo` (and `/Demo`, …): always. There is nothing else at that address. +// - the project chat address of a GitHub project (`…/chat`, `…/tree//-/chat`): only when it arrived with a +// question (`msg`, or the old `q`). The same address without one is the project's own chat page, as for every +// other page of a project opened at its short address. +// The flag of demo-flag.ts is deliberately NOT read: it is decided independently of the hand-off code (see +// demo-flag.spec.ts), and until the chat can run a question the holding page is the only way not to lose it. + +export const DEMO_HANDOFF_KEY = 'datatug.demo.handoff.v1'; +export const DEMO_HANDOFF_STASH = '__datatugHandoffSearch'; + +/** + * Kept in storage for an address that may not show its question back, in place of the question: the page was + * shown for a question that was asked, so a reload must show it again (without the question), not the chat. + */ +export const DEMO_HANDOFF_ASKED_PARAM = 'asked'; + +// Control characters other than tab and line feed have no place in a question that is shown back as text. +// eslint-disable-next-line no-control-regex +const CONTROL_CHARACTERS = /[\u0000-\u0008\u000B-\u001F\u007F]/g; +// Two or more blank lines in a row (a line of only spaces and tabs counts as blank): shown as a single blank line. +const BLANK_LINE_RUNS = /(?:[ \t]*\n){3,}/g; + +/** A question parameter as a clean text: control characters out, runs of blank lines collapsed, trimmed. */ +export function cleanQuestion(value: string | null): string { + return (value ?? '') + .replace(CONTROL_CHARACTERS, '') + .replace(BLANK_LINE_RUNS, '\n\n') + .trim(); +} + +/** The question of a query string: `msg`, else `q` (a blank `msg` does not hide a real `q`); '' when none. */ +export function questionOfSearch(search: string): string { + const params = new URLSearchParams(search); + return cleanQuestion(params.get('msg')) || cleanQuestion(params.get('q')); +} + +/** Whether this query string is a question that was asked (or the mark kept in its place for an address that may not echo it). */ +export function searchAsksQuestion(search: string): boolean { + return ( + questionOfSearch(search) !== '' || + new URLSearchParams(search).get(DEMO_HANDOFF_ASKED_PARAM) === '1' + ); +} + +export interface AskedEnv { + /** `window`, for the stash written by index.html's inline script. */ + stash: Record; + /** Throws when the browser blocks storage. */ + storage: () => Storage; + /** `PerformanceNavigationTiming.type` of this page load: 'navigate', 'reload', 'back_forward' or undefined. */ + navigationType: () => string | undefined; +} + +function navigationType(): string | undefined { + try { + const [entry] = performance.getEntriesByType( + 'navigation', + ) as PerformanceNavigationTiming[]; + return entry?.type; + } catch { + return undefined; + } +} + +function defaultEnv(): AskedEnv { + return { + stash: window as unknown as Record, + storage: () => window.sessionStorage, + navigationType, + }; +} + +/** What this page load has already been told about an address: it stays the answer for as long as the page lives. */ +const settled = new Map(); + +const keyOf = (segments: readonly string[]): string => + segments.map((s) => s.toLowerCase()).join('/'); + +/** + * Whether the hand-off address with these (decoded) segments arrived with a question, in this order: + * 1. the query index.html took out of the address bar for this page load (not yet picked up by the holding page); + * 2. what this page load already settled for the same address; + * 3. a fresh visit (the document was loaded by navigating, not reloaded) with no query: no, whatever an earlier + * visit in this tab left in storage; + * 4. a reload or back/forward: the copy of the query kept in storage for this very address. + * Blocked storage means no for 3 and 4: the question lives only in the page. + */ +export function handoffAsked( + segments: readonly string[], + env: AskedEnv = defaultEnv(), +): boolean { + const key = keyOf(segments); + const stashed = env.stash[DEMO_HANDOFF_STASH]; + if (typeof stashed === 'string') { + const asked = searchAsksQuestion(stashed); + settled.set(key, asked); + return asked; + } + const known = settled.get(key); + if (known !== undefined) return known; + if (env.navigationType() === 'navigate') return false; + try { + const raw = env.storage().getItem(DEMO_HANDOFF_KEY) ?? ''; + const at = raw.indexOf('?'); + if (at < 0) return false; + const stored = raw.slice(0, at); + return ( + stored.startsWith('/') && + keyOf(routeSegments(stored)) === key && + searchAsksQuestion(raw.slice(at)) + ); + } catch { + return false; + } +} + +/** + * Whether the holding page is the page of this hand-off address. `/demo` always; a project chat address only when + * it arrived with a question. Not a hand-off address: no. + */ +export function showsHoldingPage( + segments: readonly string[], + env?: AskedEnv, +): boolean { + const target = handoffTarget(segments); + if (!target) return false; + return target.kind === 'demo' || handoffAsked(segments, env); +} + +/** For tests: forgets what this page load settled. */ +export function resetHandoffAskedForTests(): void { + settled.clear(); +} diff --git a/apps/datatug-app/src/app/demo-handoff-capture.spec.ts b/apps/datatug-app/src/app/demo-handoff-capture.spec.ts index c2bc189e..5743c2dd 100644 --- a/apps/datatug-app/src/app/demo-handoff-capture.spec.ts +++ b/apps/datatug-app/src/app/demo-handoff-capture.spec.ts @@ -7,7 +7,7 @@ import { DEMO_HANDOFF_STASH, DEMO_QUESTION_MAX_BYTES, demoHandoff, - isEchoTrusted, + isTrustedHandoff, isHandoffPath, parseHandoffSearch, resetDemoHandoffForTests, @@ -374,7 +374,7 @@ describe('demo hand-off capture', () => { captureDemoHandoff(f.env); expect(f.replaced).toEqual([path]); expect(demoHandoff(undefined, path)?.question).toBe( - isEchoTrusted(path) ? 'Which countries' : '', + isTrustedHandoff(path) ? 'Which countries' : '', ); }); @@ -508,7 +508,7 @@ describe('demo hand-off capture', () => { }); }); - describe('which hand-offs are shown back (isEchoTrusted)', () => { + describe('which hand-offs are shown back (isTrustedHandoff)', () => { it.each([ '/demo', '/demo/', @@ -523,7 +523,7 @@ describe('demo hand-off capture', () => { '/PROJECT/GITHUB.COM/datatug/chinook-demo/CHAT', '/project/github.com/datatug/chinook-demo/chat;x=1', ])('%s is trusted', (path) => { - expect(isEchoTrusted(path)).toBe(true); + expect(isTrustedHandoff(path)).toBe(true); }); it.each([ @@ -546,7 +546,50 @@ describe('demo hand-off capture', () => { '/demo/other', '/', ])('%s is not trusted', (path) => { - expect(isEchoTrusted(path)).toBe(false); + expect(isTrustedHandoff(path)).toBe(false); + }); + + // Issue #180: `toLowerCase()` turns the Kelvin sign U+212A into the ASCII `k`, so a trust decision on the + // lower-cased text would have accepted `chinooK-demo` (live on main). The decision is on the parsed address: + // owner and repo are ASCII, lower-cased A-Z only, and compared for exact equality. + describe('look-alike letters are never trusted (the Kelvin sign U+212A)', () => { + const KELVIN = '\u212A'; + + it('is the character the old check mistook for k', () => { + expect(KELVIN.toLowerCase()).toBe('k'); + }); + + it.each([ + `/project/github.com/datatug/chinoo${KELVIN}-demo/chat`, + `/project/github.com/datatug/CHINOO${KELVIN}-DEMO/chat`, + `/project/github.com/datatug/chinoo%E2%84%AA-demo/chat`, + `/project/github.com/datatug/chinook-demo/tree/HEAD/-/chat`.replace('chinook', `chinoo${KELVIN}`), + `/project/github.com/dataTUG/chinoo${KELVIN}-demo/chat`, + `/project/github.com/${KELVIN}datatug/chinook-demo/chat`, + `/project/github.com/datatug${KELVIN}/chinook-demo/chat`, + `/project/github.com/datatug/chinook-demo${KELVIN}/chat`, + ])('%s is not trusted', (path) => { + expect(isHandoffPath(path)).toBe(true); + expect(isTrustedHandoff(path)).toBe(false); + }); + + it('drops the question of such an address, keeps the language, and still strips the address bar', () => { + const path = `/project/github.com/datatug/chinoo${KELVIN}-demo/chat`; + const f = pageLoad(path, '?msg=Hello+there&lang=ru'); + captureDemoHandoff(f.env); + expect(f.replaced).toEqual([path]); + expect(demoHandoff(undefined, path)).toEqual({ + question: '', + lang: 'ru', + truncated: false, + }); + expect(JSON.stringify([...f.store])).not.toContain('Hello'); + }); + + it('plain ASCII spellings of the same repository are still trusted', () => { + expect(isTrustedHandoff('/project/github.com/datatug/chinook-demo/chat')).toBe(true); + expect(isTrustedHandoff('/project/github.com/DataTug/Chinook-Demo/chat')).toBe(true); + }); }); const msg = '?msg=Hello+there&lang=ru'; @@ -583,7 +626,7 @@ describe('demo hand-off capture', () => { truncated: false, }); expect(JSON.stringify([...f.store])).not.toContain('Hello'); - expect(f.store.get(DEMO_HANDOFF_KEY)).toBe(path + '?lang=ru'); + expect(f.store.get(DEMO_HANDOFF_KEY)).toBe(path + '?lang=ru&asked=1'); }, ); diff --git a/apps/datatug-app/src/app/demo-handoff-capture.ts b/apps/datatug-app/src/app/demo-handoff-capture.ts index 23d4e98c..71dc9c63 100644 --- a/apps/datatug-app/src/app/demo-handoff-capture.ts +++ b/apps/datatug-app/src/app/demo-handoff-capture.ts @@ -23,19 +23,20 @@ // (`datatug/chinook-demo`, default branch) show the visitor's question. The project chat of any other repository // does not: a message chosen by whoever made the link must not be displayed inside datatug.app's pages under // someone else's repository address. For those the query is still taken out of the address bar and kept out of -// every report; only the language is kept, for the neutral page. See isEchoTrusted(). +// every report; only the language is kept, for the neutral page. See isTrustedHandoff(). +import { isTrustedProjectAddress, parseProjectUrl } from '@datatug/project-address'; +import { + DEMO_HANDOFF_ASKED_PARAM, + DEMO_HANDOFF_KEY, + DEMO_HANDOFF_STASH, + questionOfSearch, +} from './demo-handoff-asked'; import { handoffTargetOfPath, isHandoffPath } from './demo-handoff-path'; -export { isHandoffPath }; +export { isHandoffPath, DEMO_HANDOFF_KEY, DEMO_HANDOFF_STASH }; -export const DEMO_HANDOFF_KEY = 'datatug.demo.handoff.v1'; -export const DEMO_HANDOFF_STASH = '__datatugHandoffSearch'; export const DEMO_QUESTION_MAX_BYTES = 1000; -/** The demo project: the only repository whose chat address may show a message that came in a link. */ -const TRUSTED_OWNER = 'datatug'; -const TRUSTED_REPO = 'chinook-demo'; - export type DemoLang = 'en' | 'ru'; export interface DemoHandoff { @@ -49,20 +50,23 @@ export interface DemoHandoff { let current: DemoHandoff | undefined; /** - * Whether the hand-off at this path may be shown back to the visitor: `/demo`, or the chat of - * `datatug/chinook-demo` (owner and repository decoded, lower-cased, each compared for exact equality) on its - * default branch, that is with no `tree/` or with `tree/HEAD`. Anything else, however similar - * (`chinook-demo-evil`, `tree/`), is not. + * Whether the hand-off at this path may be shown back to the visitor: `/demo`, or the chat of a project the app + * trusts, decided by the one trust function (`isTrustedProjectAddress`, design 3.6) on the PARSED project address: + * owner and repository ASCII-lower-cased and compared for exact equality with the compiled-in list, on its default + * branch, that is with no `tree/` or with `tree/HEAD`. Anything else, however similar (`chinook-demo-evil`, + * `tree/`, a look-alike letter such as the Kelvin sign U+212A that `toLowerCase()` would turn into `k`), is not. */ -export function isEchoTrusted(pathname: string): boolean { +export function isTrustedHandoff(pathname: string): boolean { const target = handoffTargetOfPath(pathname); if (!target) return false; if (target.kind === 'demo') return true; - return ( - target.owner.toLowerCase() === TRUSTED_OWNER && - target.repo.toLowerCase() === TRUSTED_REPO && - (target.ref === undefined || target.ref === 'HEAD') + const project = parseProjectUrl( + `/project/github.com/${encodeURIComponent(target.owner)}/${encodeURIComponent(target.repo)}` + + (target.ref === undefined + ? '' + : `/tree/${encodeURIComponent(target.ref)}`), ); + return isTrustedProjectAddress(project); } /** What is kept for a reload: the path and the raw query string, so it is only applied to the same path. */ @@ -95,12 +99,6 @@ export function truncateToBytes( return { text, truncated: false }; } -// Control characters other than tab and line feed have no place in a question that is shown back as text. -// eslint-disable-next-line no-control-regex -const CONTROL_CHARACTERS = /[\u0000-\u0008\u000B-\u001F\u007F]/g; -// Two or more blank lines in a row (a line of only spaces and tabs counts as blank): shown as a single blank line. -const BLANK_LINE_RUNS = /(?:[ \t]*\n){3,}/g; - /** * Parses a hand-off query string. * - the question is `msg`, else `q` (a blank `msg` does not hide a real `q`), trimmed, runs of blank lines @@ -110,13 +108,10 @@ const BLANK_LINE_RUNS = /(?:[ \t]*\n){3,}/g; */ export function parseHandoffSearch(search: string): DemoHandoff { const params = new URLSearchParams(search); - const clean = (value: string | null): string => - (value ?? '') - .replace(CONTROL_CHARACTERS, '') - .replace(BLANK_LINE_RUNS, '\n\n') - .trim(); - const raw = clean(params.get('msg')) || clean(params.get('q')); - const { text, truncated } = truncateToBytes(raw, DEMO_QUESTION_MAX_BYTES); + const { text, truncated } = truncateToBytes( + questionOfSearch(search), + DEMO_QUESTION_MAX_BYTES, + ); const lang: DemoLang = (params.get('lang') ?? '').trim().toLowerCase() === 'ru' ? 'ru' : 'en'; return { question: text.trimEnd(), lang, truncated }; @@ -178,16 +173,23 @@ export function captureDemoHandoff(env: CaptureEnv = defaultEnv()): void { } if (search) { const parsed = parseHandoffSearch(search); - const trusted = isEchoTrusted(loc.pathname); + const trusted = isTrustedHandoff(loc.pathname); // Not trusted: only the language survives, for the neutral page. The question is dropped here, not hidden later. current = trusted ? parsed : { question: '', lang: parsed.lang, truncated: false }; // index.html has stored the original already; this covers a page served without that script, and removes // the question of an untrusted address from storage. + // For an address that may not echo it, what is kept is the language and a mark that a question was asked: + // after a reload the route still shows this page (neutral wording), not the project's chat. store( env, - storedFor(loc.pathname, trusted ? search : '?lang=' + parsed.lang), + storedFor( + loc.pathname, + trusted + ? search + : `?lang=${parsed.lang}&${DEMO_HANDOFF_ASKED_PARAM}=1`, + ), ); } else if (env.navigationType() === 'navigate') { // A fresh visit to the bare path must not show a question left in this tab by an earlier visit. (A reload @@ -217,7 +219,7 @@ export function demoHandoff( ): DemoHandoff | undefined { // Whatever was captured, the question is only ever returned for an address that may show it. const forPath = (handoff: DemoHandoff): DemoHandoff => - isEchoTrusted(pathname) + isTrustedHandoff(pathname) ? handoff : { question: '', lang: handoff.lang, truncated: false }; if (current) return forPath(current); diff --git a/apps/datatug-app/src/app/demo-holding-page.component.ts b/apps/datatug-app/src/app/demo-holding-page.component.ts index 99e786e9..c0353406 100644 --- a/apps/datatug-app/src/app/demo-holding-page.component.ts +++ b/apps/datatug-app/src/app/demo-holding-page.component.ts @@ -13,7 +13,7 @@ import { NavigationEnd, Router, RouterLink } from '@angular/router'; import { captureDemoHandoff, demoHandoff, - isEchoTrusted, + isTrustedHandoff, isHandoffPath, } from './demo-handoff-capture'; import { @@ -111,9 +111,9 @@ export class DemoHoldingPageComponent { // index.html's first script has already taken the query out of the address bar and stashed it; this parses it. private readonly handoff = (captureDemoHandoff(), demoHandoff()); // `/demo` and the demo project's own chat address say that the live demo opens here soon and show the question - // back. Any other repository's chat address (see isEchoTrusted) gets neutral wording: it must not claim to be + // back. Any other repository's chat address (see isTrustedHandoff) gets neutral wording: it must not claim to be // a demo, and it carries no question (the capture dropped it). - protected readonly trusted = isEchoTrusted(window.location.pathname); + protected readonly trusted = isTrustedHandoff(window.location.pathname); protected readonly lang = this.handoff?.lang ?? 'en'; protected readonly strings = DEMO_HOLDING_STRINGS[this.lang]; protected readonly question = this.handoff?.question ?? ''; diff --git a/libs/datatug/main/src/lib/routes/datatug-routing.module.ts b/libs/datatug/main/src/lib/routes/datatug-routing.module.ts index 6161c796..9114881d 100644 --- a/libs/datatug/main/src/lib/routes/datatug-routing.module.ts +++ b/libs/datatug/main/src/lib/routes/datatug-routing.module.ts @@ -6,6 +6,7 @@ import { routingParamSpaceId, routingParamStoreId, } from '../core/datatug-routing-params'; +import { githubProjectRoutes } from './github-project-routes'; import { profileHomeRedirectGuard } from './profile-home-redirect.guard'; export const datatugRoutes: Routes = [ @@ -101,6 +102,9 @@ export const datatugRoutes: Routes = [ pathMatch: 'full', redirectTo: '/', }, + // The short address of a GitHub project: `/project/github.com//…` (design + // `demo-as-github-project.md` 3.4). Matched by matchers, not paths; the old `store/…` form below is unchanged. + ...githubProjectRoutes, { path: 'store/:' + routingParamStoreId, loadChildren: () => diff --git a/libs/datatug/main/src/lib/routes/github-address-problem-page.component.spec.ts b/libs/datatug/main/src/lib/routes/github-address-problem-page.component.spec.ts new file mode 100644 index 00000000..939ffe0d --- /dev/null +++ b/libs/datatug/main/src/lib/routes/github-address-problem-page.component.spec.ts @@ -0,0 +1,211 @@ +import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { provideRouter } from '@angular/router'; +import { ToastController } from '@ionic/angular'; +import { ProjectUrlErrorReason } from '@datatug/project-address'; +import { describe, expect, it, vi } from 'vitest'; +import { + GithubAddressNotices, + GithubAddressProblem, + GithubAddressProblemState, +} from './github-project-address-check'; +import { + GithubAddressProblemPageComponent, + messageForNotFound, + messageForUnsupported, +} from './github-address-problem-page.component'; + +// The words for each thing that can be wrong with a short GitHub address (design 3.4a, "Not supported"), as text. + +describe('messageForUnsupported', () => { + it.each([ + [ + 'file-link', + 'This is a link to a file', + "Open the project's folder instead.", + ], + ['at-sign-not-supported', 'This address is not supported', '"@"'], + [ + 'dash-directory-not-supported', + 'This address is not supported', + 'named "-"', + ], + ['missing-ref', 'This address is not supported', '/tree/'], + [ + 'invalid-owner-or-repo', + 'This address is not supported', + 'datatug.app/project/github.com//', + ], + [ + 'invalid-path-segment', + 'This address is not supported', + 'datatug.app/project/github.com//', + ], + [ + 'not-a-project-address', + 'This address is not supported', + 'datatug.app/project/github.com//', + ], + [ + 'unsupported-store-id', + 'This address is not supported', + 'datatug.app/project/github.com//', + ], + [ + 'not-representable', + 'This address is not supported', + 'datatug.app/project/github.com//', + ], + ] as [ProjectUrlErrorReason, string, string][])( + '%s', + (reason, heading, text) => { + const message = messageForUnsupported(reason); + expect(message.heading).toBe(heading); + expect(message.paragraphs.join(' ')).toContain(text); + expect(message.repo).toBeUndefined(); + }, + ); +}); + +describe('messageForNotFound', () => { + const problem = ( + extra: Partial> = {}, + ) => ({ + kind: 'not-found' as const, + owner: 'o', + repo: 'r', + folder: '', + moved: false, + ...extra, + }); + + it.each([ + [problem(), 'There is no datatug-project.json at the root of o/r.'], + [ + problem({ folder: 'a/b' }), + 'There is no datatug-project.json at "a/b" in o/r.', + ], + [ + problem({ ref: 'v1' }), + 'There is no datatug-project.json at the root of o/r at "v1".', + ], + [problem({ ref: 'v1', folder: 'x' }), 'No DataTug project at "x" on "v1".'], + ])('says where no project was found', (p, text) => { + const message = messageForNotFound(p); + expect(message.heading).toBe('No DataTug project here'); + expect(message.paragraphs[0]).toBe(text); + expect(message.repo).toBe('o/r'); + }); + + it('a folder on a ref suggests a branch name with a slash, and the address to use instead', () => { + const message = messageForNotFound( + problem({ ref: 'feature', folder: 'x' }), + ); + expect(message.paragraphs[1]).toContain( + 'If the branch name contains "/", open it by its commit instead', + ); + expect(message.paragraphs[1]).toContain( + 'datatug.app/project/github.com/o/r/tree//x', + ); + }); + + it('a moved repository is said to have moved', () => { + const message = messageForNotFound(problem({ moved: true })); + expect(message.paragraphs.join(' ')).toContain('has moved or been renamed'); + expect(message.repo).toBe('o/r'); + }); +}); + +describe('GithubAddressProblemPageComponent', () => { + function render( + problem?: GithubAddressProblem, + ): ComponentFixture { + TestBed.configureTestingModule({ + imports: [GithubAddressProblemPageComponent], + providers: [provideRouter([])], + }).overrideComponent(GithubAddressProblemPageComponent, { + set: { schemas: [CUSTOM_ELEMENTS_SCHEMA] }, + }); + TestBed.inject(GithubAddressProblemState).problem.set(problem); + const fixture = TestBed.createComponent(GithubAddressProblemPageComponent); + fixture.detectChanges(); + return fixture; + } + const text = (f: ComponentFixture) => + (f.nativeElement as HTMLElement).textContent ?? ''; + + it('shows the heading and the words for an unsupported address, with a way back', () => { + const f = render({ kind: 'unsupported', reason: 'file-link' }); + const el = f.nativeElement as HTMLElement; + expect(el.querySelector('h1')?.textContent).toBe( + 'This is a link to a file', + ); + expect(text(f)).toContain("Open the project's folder instead."); + expect(el.querySelector('a[href="/"]')?.textContent).toContain( + 'Back to DataTug', + ); + expect(el.querySelector('a[href^="https://github.com/"]')).toBeNull(); + }); + + it('shows "No DataTug project here" with the repository\'s GitHub link', () => { + const f = render({ + kind: 'not-found', + owner: 'o', + repo: 'r', + folder: 'dir', + moved: false, + }); + const el = f.nativeElement as HTMLElement; + expect(el.querySelector('h1')?.textContent).toBe('No DataTug project here'); + const link = el.querySelector( + 'a[href^="https://github.com/"]', + ) as HTMLAnchorElement; + expect(link.getAttribute('href')).toBe('https://github.com/o/r'); + expect(link.textContent).toContain('Open o/r on GitHub'); + expect(link.getAttribute('rel')).toContain('noopener'); + }); + + it('shows the generic message when no problem was recorded (a page restored with no state)', () => { + const f = render(undefined); + expect(text(f)).toContain('This address is not supported'); + }); + + it('puts what came from the address on the page as text, never as markup', () => { + const f = render({ + kind: 'not-found', + owner: 'o', + repo: 'r', + ref: '', + folder: 'bold', + moved: false, + }); + const el = f.nativeElement as HTMLElement; + expect(el.querySelector('img')).toBeNull(); + expect(el.querySelector('b')).toBeNull(); + expect(text(f)).toContain('bold'); + expect(text(f)).toContain(''); + expect( + (window as unknown as Record)['__pwned'], + ).toBeUndefined(); + }); +}); + +describe('GithubAddressNotices', () => { + it('tells the visit, in a toast, that the address is kept as typed and its history is separate', async () => { + const present = vi.fn(async () => undefined); + const create = vi.fn< + (options: Record) => Promise<{ present: typeof present }> + >(async () => ({ present })); + TestBed.configureTestingModule({ + providers: [{ provide: ToastController, useValue: { create } }], + }); + TestBed.inject(GithubAddressNotices).defaultBranchUnknown('o', 'r', 'main'); + await vi.waitFor(() => expect(present).toHaveBeenCalledTimes(1)); + const message = String(create.mock.calls[0][0]['message']); + expect(message).toContain('"main"'); + expect(message).toContain('o/r'); + expect(message).toContain('kept as typed'); + expect(message).toContain('history is kept separately'); + expect(create.mock.calls[0][0]['duration']).toBeGreaterThan(0); + }); +}); diff --git a/libs/datatug/main/src/lib/routes/github-address-problem-page.component.ts b/libs/datatug/main/src/lib/routes/github-address-problem-page.component.ts new file mode 100644 index 00000000..7c210adb --- /dev/null +++ b/libs/datatug/main/src/lib/routes/github-address-problem-page.component.ts @@ -0,0 +1,184 @@ +import { + ChangeDetectionStrategy, + Component, + computed, + inject, +} from '@angular/core'; +import { RouterLink } from '@angular/router'; +import { + IonButtons, + IonContent, + IonHeader, + IonMenuButton, + IonTitle, + IonToolbar, +} from '@ionic/angular'; +import { ProjectUrlErrorReason } from '@datatug/project-address'; +import { + GithubAddressProblem, + GithubAddressProblemState, +} from './github-project-address-check'; + +/** What the visitor reads for each kind of problem with a short GitHub address (design 3.4a, "Not supported"). */ +export interface GithubAddressMessage { + readonly heading: string; + readonly paragraphs: readonly string[]; + /** A GitHub link to show, `owner/repo`. */ + readonly repo?: string; +} + +const UNSUPPORTED_HEADING = 'This address is not supported'; + +/** The words for an address that cannot be a project. Text only: nothing here is markup. */ +export function messageForUnsupported( + reason: ProjectUrlErrorReason, +): GithubAddressMessage { + switch (reason) { + case 'file-link': + return { + heading: 'This is a link to a file', + paragraphs: ["Open the project's folder instead."], + }; + case 'at-sign-not-supported': + return { + heading: UNSUPPORTED_HEADING, + paragraphs: [ + 'A folder or a branch name cannot contain "@": it separates the parts of the project id.', + ], + }; + case 'dash-directory-not-supported': + return { + heading: UNSUPPORTED_HEADING, + paragraphs: [ + 'A folder named "-" is not supported: the first "-" in an address ends the project locator.', + ], + }; + case 'missing-ref': + return { + heading: UNSUPPORTED_HEADING, + paragraphs: ['Name a branch, a tag or a commit after "/tree/".'], + }; + default: + return { + heading: UNSUPPORTED_HEADING, + paragraphs: [ + 'A project address looks like datatug.app/project/github.com//.', + ], + }; + } +} + +/** The words for an address that is a project address with no project at it. */ +export function messageForNotFound( + problem: Extract, +): GithubAddressMessage { + const repo = `${problem.owner}/${problem.repo}`; + if (problem.moved) { + return { + heading: 'No DataTug project here', + paragraphs: [ + `${repo} has moved or been renamed. Open it by its new name.`, + ], + repo, + }; + } + if (problem.ref !== undefined && problem.folder !== '') { + return { + heading: 'No DataTug project here', + paragraphs: [ + `No DataTug project at "${problem.folder}" on "${problem.ref}".`, + 'If the branch name contains "/", open it by its commit instead: datatug.app/project/github.com/' + + `${repo}/tree//${problem.folder}`, + ], + repo, + }; + } + const where = + (problem.folder === '' ? 'the root of' : `"${problem.folder}" in`) + + ` ${repo}` + + (problem.ref !== undefined ? ` at "${problem.ref}"` : ''); + return { + heading: 'No DataTug project here', + paragraphs: [`There is no datatug-project.json at ${where}.`], + repo, + }; +} + +/** + * The page for a short GitHub address that cannot open a project (design 3.4a): it says why, in words, with a way + * out, instead of a broken page. Text only; the GitHub link is built from an owner and a repo name that matched + * GitHub's own patterns. + */ +@Component({ + selector: 'sneat-datatug-github-address-problem', + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [ + RouterLink, + IonHeader, + IonToolbar, + IonButtons, + IonMenuButton, + IonTitle, + IonContent, + ], + template: ` + + + + + + DataTug + + + +
+

{{ message().heading }}

+
+ @for (paragraph of message().paragraphs; track $index) { +

{{ paragraph }}

+ } +
+

+ @if (message().repo; as repo) { + Open {{ repo }} on GitHub + } + Back to DataTug +

+
+ + `, + styles: ` + .problem { + max-width: 40rem; + margin: 0 auto; + } + h1 { + margin: 0.5rem 0 1rem; + font-size: 1.5rem; + outline: none; + } + .links { + display: flex; + flex-wrap: wrap; + gap: 0.5rem 1.5rem; + margin-top: 1.5rem; + } + `, +}) +export class GithubAddressProblemPageComponent { + private readonly state = inject(GithubAddressProblemState); + + protected readonly message = computed(() => { + const problem = this.state.problem(); + if (problem?.kind === 'not-found') { + return messageForNotFound(problem); + } + return messageForUnsupported( + problem?.kind === 'unsupported' + ? problem.reason + : 'not-a-project-address', + ); + }); +} diff --git a/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts b/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts new file mode 100644 index 00000000..872b7298 --- /dev/null +++ b/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts @@ -0,0 +1,577 @@ +import { TestBed } from '@angular/core/testing'; +import { of, throwError } from 'rxjs'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { DatatugStoreGithubService } from '../services/repo/datatug-store.service.github'; +import { + GITHUB_FETCH, + GithubProjectNotFoundError, +} from '../services/repo/github/github-project-reader.service'; +import { + GithubAddressCheck, + GithubAddressNotices, + GithubDefaultBranchLookup, + readShortGithubAddress, +} from './github-project-address-check'; + +// Design `demo-as-github-project.md` 3.3 (the id), 3.4a (one canonical address per project, and what is not +// supported) and the default-branch lookup, as the short project route uses them. + +const segs = (path: string): string[] => + path + .split('/') + .filter((s) => s !== '') + .map((s) => decodeURIComponent(s)); + +describe('readShortGithubAddress: the table of design 3.3', () => { + it.each([ + // [path, projectId, locator segments] + ['/project/github.com/datatug/chinook-demo', 'chinook-demo@datatug@', 4], + [ + '/project/github.com/datatug/chinook-demo/chat', + 'chinook-demo@datatug@', + 4, + ], + [ + '/project/github.com/datatug/chinook-demo/queries/a', + 'chinook-demo@datatug@', + 4, + ], + [ + '/project/github.com/o/r/tree/HEAD/demo-project-1', + 'r@o@demo-project-1', + 7, + ], + [ + '/project/github.com/o/r/tree/HEAD/demo-project-1/-/chat', + 'r@o@demo-project-1', + 8, + ], + ['/project/github.com/o/r/tree/HEAD/a/b/-/queries/x/y', 'r@o@a/b', 9], + ['/project/github.com/o/r/tree/v1.0.0', 'r@o@@v1.0.0', 6], + ['/project/github.com/o/r/tree/v1.0.0/-/chat', 'r@o@@v1.0.0', 7], + ['/project/github.com/o/r/tree/HEAD/datatug', 'r@o', 7], + ['/project/github.com/o/r/tree/HEAD/datatug/-/chat', 'r@o', 8], + ['/project/github.com/o/r/tree/feat/dir', 'r@o@dir@feat', 7], + ['/project/github.com/Datatug/Chinook-Demo', 'chinook-demo@datatug@', 4], + ['/project/github.com/o/r.git', 'r@o@', 4], + ['/project/github.com/o/r/tree/HEAD', 'r@o@', 6], + [ + '/project/github.com/o/r/blob/main/dir/datatug-project.json', + 'r@o@dir@main', + 8, + ], + // a repository called like a page + ['/project/github.com/o/chat', 'chat@o@', 4], + ['/project/github.com/o/tree', 'tree@o@', 4], + ['/project/github.com/chat/chat/chat', 'chat@chat@', 4], + ])( + '%s is the project %s, with %i locator segments', + (path, projectId, locatorLength) => { + const result = readShortGithubAddress(segs(path)); + expect(result).toMatchObject({ kind: 'project', locatorLength }); + if (result.kind === 'project') { + expect(result.parts.storeId).toBe('github.com'); + expect(result.parts.projectId).toBe(projectId); + } + }, + ); + + it('is not about any other address', () => { + for (const path of [ + '/', + '/store/github.com/project/r@o@', + '/projects/github.com/o/r', + '/Project/github.com/o/r', + '/project/GitHub.com/o/r', + '/project/gitlab.com/o/r', + '/project', + ]) { + expect(readShortGithubAddress(segs(path)), path).toEqual({ + kind: 'not-short-github', + }); + } + }); + + it.each([ + // [path, why]: the unsupported inputs of design 3.4a, and the addresses that are no project at all + ['/project/github.com/o/r/blob/main/dir/file.txt', 'file-link'], + ['/project/github.com/o/r/blob/main', 'file-link'], + ['/project/github.com/o/r/blob', 'file-link'], + ['/project/github.com/o/r/tree', 'missing-ref'], + ['/project/github.com/o/r/tree/main/a@b', 'at-sign-not-supported'], + ['/project/github.com/o/r/tree/ma@in', 'at-sign-not-supported'], + [ + '/project/github.com/o/r/blob/main/a/-/datatug-project.json', + 'dash-directory-not-supported', + ], + ['/project/github.com/o/r/-', 'invalid-path-segment'], + ['/project/github.com/o/r/tree/HEAD/a/../b', 'invalid-path-segment'], + ['/project/github.com/o/r/chat/..', 'invalid-path-segment'], + ['/project/github.com', 'not-a-project-address'], + ['/project/github.com/o', 'not-a-project-address'], + ['/project/github.com/o_x/r', 'invalid-owner-or-repo'], + ['/project/github.com/o/r.git.git', 'invalid-owner-or-repo'], + ['/project/github.com/o/chinoo\u212A-demo', 'invalid-owner-or-repo'], + ['/project/github.com/o/chinoo%E2%84%AA-demo', 'invalid-owner-or-repo'], + ])('%s is refused (%s)', (path, reason) => { + expect(readShortGithubAddress(segs(path))).toEqual({ + kind: 'refused', + reason, + }); + }); + + it('refuses a directory segment named "-": the first "-" ends the locator, so the rest reads as a page', () => { + // `…/tree/HEAD/a/-/b` is the page `b` of the project in `a`; a folder named `-` cannot be written. + expect( + readShortGithubAddress(segs('/project/github.com/o/r/tree/HEAD/a/-/b')), + ).toMatchObject({ + kind: 'project', + locatorLength: 8, + }); + }); + + it('reads an encoded slash in a page as part of one segment', () => { + const result = readShortGithubAddress([ + 'project', + 'github.com', + 'o', + 'r', + 'query', + 'a/b', + ]); + expect(result).toMatchObject({ kind: 'project', locatorLength: 4 }); + }); +}); + +describe('GithubDefaultBranchLookup', () => { + const answer = (init: () => Response | Promise) => { + const fetchFn = vi.fn< + (url: string, init: RequestInit) => Promise + >(async () => init()); + TestBed.configureTestingModule({ + providers: [{ provide: GITHUB_FETCH, useValue: fetchFn }], + }); + return { fetchFn, lookup: TestBed.inject(GithubDefaultBranchLookup) }; + }; + const json = (body: unknown, status = 200) => + new Response(typeof body === 'string' ? body : JSON.stringify(body), { + status, + }); + + it('reads default_branch from the repository, with no credentials and no redirect followed', async () => { + const { fetchFn, lookup } = answer(() => + json({ id: 1, default_branch: 'trunk' }), + ); + expect(await lookup.lookup('o', 'r')).toEqual({ + kind: 'found', + branch: 'trunk', + }); + expect(fetchFn).toHaveBeenCalledTimes(1); + const [url, init] = fetchFn.mock.calls[0]; + expect(url).toBe('https://api.github.com/repos/o/r'); + expect(init).toMatchObject({ method: 'GET', credentials: 'omit' }); + }); + + it('asks once per repository, whatever the answer', async () => { + const { fetchFn, lookup } = answer(() => json({ default_branch: 'main' })); + await Promise.all([lookup.lookup('o', 'r'), lookup.lookup('o', 'r')]); + await lookup.lookup('o', 'r'); + expect(fetchFn).toHaveBeenCalledTimes(1); + await lookup.lookup('o', 'other'); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); + + it.each([ + ['a missing repository', () => json('nf', 404), 'absent'], + ['a gone repository', () => json('gone', 410), 'absent'], + [ + 'a moved repository (a redirect)', + () => + new Response(null, { + status: 301, + headers: { location: 'https://x/' }, + }), + 'absent', + ], + ['a rate limit (403)', () => json('limit', 403), 'refused'], + ['a rate limit (429)', () => json('limit', 429), 'refused'], + ['a server error', () => json('boom', 500), 'refused'], + ['an answer that is not JSON', () => json(''), 'refused'], + ['an answer with no default_branch', () => json({ id: 1 }), 'refused'], + [ + 'an answer with a default_branch that is not text', + () => json({ default_branch: 7 }), + 'refused', + ], + ['an empty default_branch', () => json({ default_branch: '' }), 'refused'], + [ + 'a network failure', + () => Promise.reject(new TypeError('Failed to fetch')), + 'refused', + ], + ])('%s is %s', async (_name, init, kind) => { + const { lookup } = answer(init); + expect((await lookup.lookup('o', 'r')).kind).toBe(kind); + }); + + it('refuses an answer larger than the cap, checked on the bytes received', async () => { + const { lookup } = answer(() => + json({ default_branch: 'main', pad: 'x'.repeat(300 * 1024) }), + ); + expect((await lookup.lookup('o', 'r')).kind).toBe('refused'); + }); +}); + +describe('GithubAddressCheck.decide', () => { + let branch: ReturnType; + let notice: ReturnType; + let summary: ReturnType; + + const check = ( + lookupResult: unknown = { kind: 'found', branch: 'main' }, + summaryResult: () => unknown = () => of({ id: 'x' }), + ) => { + branch = vi.fn(async () => lookupResult); + notice = vi.fn(); + summary = vi.fn(summaryResult); + TestBed.configureTestingModule({ + providers: [ + { provide: GithubDefaultBranchLookup, useValue: { lookup: branch } }, + { + provide: GithubAddressNotices, + useValue: { defaultBranchUnknown: notice }, + }, + { + provide: DatatugStoreGithubService, + useValue: { getProjectSummary: summary }, + }, + ], + }); + return TestBed.inject(GithubAddressCheck); + }; + + beforeEach(() => TestBed.resetTestingModule()); + + describe('canonicalisation: every spelling of an address redirects to the one canonical address', () => { + it.each([ + // [typed, canonical] + [ + '/project/github.com/Datatug/Chinook-Demo', + '/project/github.com/datatug/chinook-demo', + ], + [ + '/project/github.com/DATATUG/CHINOOK-DEMO/chat', + '/project/github.com/datatug/chinook-demo/chat', + ], + ['/project/github.com/o/r.git', '/project/github.com/o/r'], + ['/project/github.com/o/R.GIT/chat', '/project/github.com/o/r/chat'], + ['/project/github.com/o/r/tree/HEAD', '/project/github.com/o/r'], + [ + '/project/github.com/o/r/tree/HEAD/-/chat', + '/project/github.com/o/r/chat', + ], + [ + '/project/github.com/o/r/tree/HEAD/-/queries/a', + '/project/github.com/o/r/queries/a', + ], + [ + '/project/github.com/o/r/blob/v1/dir/datatug-project.json', + '/project/github.com/o/r/tree/v1/dir', + ], + [ + '/project/github.com/o/r/blob/HEAD/dir/datatug-project.json', + '/project/github.com/o/r/tree/HEAD/dir', + ], + [ + '/project/github.com/O/r/blob/HEAD/datatug-project.json', + '/project/github.com/o/r', + ], + [ + '/project/github.com/o/r/tree/HEAD/datatug/-/chat', + '/project/github.com/o/r/tree/HEAD/datatug/-/chat', + ], + ])('%s', async (typed, canonical) => { + const decision = await check().decide(segs(typed)); + if (typed === canonical) { + expect(decision).toEqual({ kind: 'open' }); + } else { + expect(decision).toEqual({ kind: 'redirect', path: canonical }); + } + // nothing is asked of GitHub before the address is canonical + expect(branch).not.toHaveBeenCalled(); + if (typed !== canonical) { + expect(summary).not.toHaveBeenCalled(); + } + }); + + it('the redirect target is itself canonical: it opens, it does not redirect again', async () => { + for (const typed of [ + '/project/github.com/Datatug/Chinook-Demo/chat', + '/project/github.com/o/r.git', + '/project/github.com/o/r/tree/HEAD/-/chat', + '/project/github.com/o/r/blob/v1/dir/datatug-project.json', + ]) { + TestBed.resetTestingModule(); + const first = await check().decide(segs(typed)); + expect(first.kind).toBe('redirect'); + if (first.kind === 'redirect') { + TestBed.resetTestingModule(); + expect( + (await check().decide(segs(first.path))).kind, + first.path, + ).toBe('open'); + } + } + }); + }); + + describe('a branch, tag or commit: the default branch is always spelled HEAD', () => { + it.each([ + ['main', '/project/github.com/o/r/tree/main', '/project/github.com/o/r'], + [ + 'main', + '/project/github.com/o/r/tree/main/-/chat', + '/project/github.com/o/r/chat', + ], + [ + 'main', + '/project/github.com/o/r/tree/main/demo-project-1', + '/project/github.com/o/r/tree/HEAD/demo-project-1', + ], + [ + 'main', + '/project/github.com/o/r/tree/main/a/b/-/queries/x', + '/project/github.com/o/r/tree/HEAD/a/b/-/queries/x', + ], + [ + 'trunk', + '/project/github.com/o/r/tree/trunk/datatug', + '/project/github.com/o/r/tree/HEAD/datatug', + ], + ])( + '%s is the default branch: %s redirects to %s', + async (name, typed, canonical) => { + const decision = await check({ kind: 'found', branch: name }).decide( + segs(typed), + ); + expect(decision).toEqual({ kind: 'redirect', path: canonical }); + expect(branch).toHaveBeenCalledWith('o', 'r'); + expect(notice).not.toHaveBeenCalled(); + }, + ); + + it.each([ + '/project/github.com/o/r/tree/v1.0.0', + '/project/github.com/o/r/tree/v1.0.0/-/chat', + '/project/github.com/o/r/tree/develop/dir', + '/project/github.com/o/r/tree/0123456789abcdef0123456789abcdef01234567', + ])( + '%s is another version of the project: its own id, nothing to replace', + async (typed) => { + expect( + await check({ kind: 'found', branch: 'main' }).decide(segs(typed)), + ).toEqual({ kind: 'open' }); + expect(summary).toHaveBeenCalledTimes(1); + expect(notice).not.toHaveBeenCalled(); + }, + ); + + it('a ref is compared with the name as GitHub spells it, letter case included', async () => { + expect( + await check({ kind: 'found', branch: 'Main' }).decide( + segs('/project/github.com/o/r/tree/main'), + ), + ).toEqual({ kind: 'open' }); + }); + + it('the lookup is made only when the address names a ref', async () => { + const c = check(); + for (const typed of [ + '/project/github.com/o/r', + '/project/github.com/o/r/chat', + '/project/github.com/o/r/tree/HEAD/dir', + '/project/github.com/o/r/tree/HEAD/dir/-/chat', + ]) { + await c.decide(segs(typed)); + } + expect(branch).not.toHaveBeenCalled(); + }); + + it('when the lookup is refused, the address is left as typed, the visit is told, and the project still opens', async () => { + const decision = await check({ kind: 'refused' }).decide( + segs('/project/github.com/o/r/tree/main/dir'), + ); + expect(decision).toEqual({ kind: 'open' }); + expect(notice).toHaveBeenCalledTimes(1); + expect(notice).toHaveBeenCalledWith('o', 'r', 'main'); + expect(summary).toHaveBeenCalledWith('r@o@dir@main'); + }); + + it('when the repository is not there, there is nothing to warn about: the missing project is what is shown', async () => { + const decision = await check({ kind: 'absent' }, () => + throwError( + () => new GithubProjectNotFoundError('r@o@@main', 'missing'), + ), + ).decide(segs('/project/github.com/o/r/tree/main')); + expect(decision.kind).toBe('problem'); + expect(notice).not.toHaveBeenCalled(); + }); + }); + + describe('what is not supported, and what the visitor is told instead of a broken page', () => { + it.each([ + [ + 'a link to a file', + '/project/github.com/o/r/blob/main/dir/file.txt', + 'file-link', + ], + [ + '"@" in a directory', + '/project/github.com/o/r/tree/HEAD/a@b', + 'at-sign-not-supported', + ], + [ + '"@" in a ref', + '/project/github.com/o/r/tree/a@b', + 'at-sign-not-supported', + ], + [ + 'a directory named "-"', + '/project/github.com/o/r/blob/main/a/-/datatug-project.json', + 'dash-directory-not-supported', + ], + ['a tree with no ref', '/project/github.com/o/r/tree', 'missing-ref'], + [ + 'a name GitHub does not allow', + '/project/github.com/o/r!', + 'invalid-owner-or-repo', + ], + [ + 'a ".." segment', + '/project/github.com/o/r/tree/HEAD/a/../b', + 'invalid-path-segment', + ], + ['no repository', '/project/github.com/o', 'not-a-project-address'], + ])( + '%s: the page says so, and nothing is asked of GitHub', + async (_name, typed, reason) => { + const decision = await check().decide(segs(typed)); + expect(decision).toEqual({ + kind: 'problem', + problem: { kind: 'unsupported', reason }, + }); + expect(branch).not.toHaveBeenCalled(); + expect(summary).not.toHaveBeenCalled(); + }, + ); + }); + + describe('a repository with no project file at the address', () => { + it.each([ + [ + 'the root', + '/project/github.com/o/r', + { owner: 'o', repo: 'r', folder: '' }, + ], + [ + 'a folder', + '/project/github.com/o/r/tree/HEAD/dir/sub', + { owner: 'o', repo: 'r', folder: 'dir/sub' }, + ], + [ + 'a tag', + '/project/github.com/o/r/tree/v1', + { owner: 'o', repo: 'r', folder: '', ref: 'v1' }, + ], + [ + 'a first part that is a branch of the name feature/x: read as the ref `feature` and the folder `x`', + '/project/github.com/o/r/tree/feature/x', + { owner: 'o', repo: 'r', folder: 'x', ref: 'feature' }, + ], + ])('%s: "No DataTug project here"', async (_name, typed, where) => { + const decision = await check({ kind: 'found', branch: 'main' }, () => + throwError(() => new GithubProjectNotFoundError('x', 'missing')), + ).decide(segs(typed)); + expect(decision).toEqual({ + kind: 'problem', + problem: { kind: 'not-found', moved: false, ...where }, + }); + }); + + it('a renamed or moved repository (GitHub answers with a redirect) is said to have moved', async () => { + const decision = await check(undefined, () => + throwError(() => new GithubProjectNotFoundError('x', 'moved')), + ).decide(segs('/project/github.com/o/r')); + expect(decision).toMatchObject({ + kind: 'problem', + problem: { kind: 'not-found', moved: true }, + }); + }); + + it('is asked again on the next navigation, so a project created in the meantime opens', async () => { + let exists = false; + const c = check(undefined, () => + exists + ? of({ id: 'x' }) + : throwError(() => new GithubProjectNotFoundError('x', 'missing')), + ); + expect((await c.decide(segs('/project/github.com/o/r'))).kind).toBe( + 'problem', + ); + exists = true; + expect((await c.decide(segs('/project/github.com/o/r'))).kind).toBe( + 'open', + ); + }); + }); + + describe('a project that is there', () => { + it('opens, and is not asked about again on later navigations inside it', async () => { + const c = check(); + for (const typed of [ + '/project/github.com/o/r', + '/project/github.com/o/r/chat', + '/project/github.com/o/r/queries', + ]) { + expect(await c.decide(segs(typed))).toEqual({ kind: 'open' }); + } + expect(summary).toHaveBeenCalledTimes(1); + expect(summary).toHaveBeenCalledWith('r@o@'); + }); + + it('is asked about once per project id', async () => { + const c = check(); + await c.decide(segs('/project/github.com/o/r')); + await c.decide(segs('/project/github.com/o/r/tree/HEAD/dir')); + await c.decide(segs('/project/github.com/o/r/tree/v1')); + expect(summary.mock.calls.map((call) => call[0])).toEqual([ + 'r@o@', + 'r@o@dir', + 'r@o@@v1', + ]); + }); + + it.each([ + ['a rate limit', new Error('rate limited')], + ['a network error', new TypeError('Failed to fetch')], + ])( + 'opens when the read fails for another reason than "not found" (%s): the pages show their own error', + async (_name, err) => { + const c = check(undefined, () => throwError(() => err)); + expect(await c.decide(segs('/project/github.com/o/r'))).toEqual({ + kind: 'open', + }); + // an inconclusive read is not repeated on every navigation either + await c.decide(segs('/project/github.com/o/r/chat')); + expect(summary).toHaveBeenCalledTimes(1); + }, + ); + }); + + it('is not about an address outside /project/github.com', async () => { + expect( + await check().decide(segs('/store/github.com/project/r@o@')), + ).toEqual({ kind: 'open' }); + expect(summary).not.toHaveBeenCalled(); + }); +}); diff --git a/libs/datatug/main/src/lib/routes/github-project-address-check.ts b/libs/datatug/main/src/lib/routes/github-project-address-check.ts new file mode 100644 index 00000000..095fc519 --- /dev/null +++ b/libs/datatug/main/src/lib/routes/github-project-address-check.ts @@ -0,0 +1,270 @@ +import { Injectable, inject, signal } from '@angular/core'; +import { ToastController } from '@ionic/angular'; +import { firstValueFrom } from 'rxjs'; +import { + GITHUB_DEFAULT_BRANCH_REF, + GITHUB_STORE_ID, + IGithubProjectParts, + IProjectUrlParts, + parseProjectUrl, + ProjectUrlErrorReason, +} from '@datatug/project-address'; +import { DatatugStoreGithubService } from '../services/repo/datatug-store.service.github'; +import { githubGet } from '../services/repo/github/github-http'; +import { + GITHUB_FETCH, + GithubProjectNotFoundError, +} from '../services/repo/github/github-project-reader.service'; +import { + GITHUB_RESOLVE_TIMEOUT_MS, + MAX_PROJECT_FILE_BYTES, +} from '../services/repo/github/github-read-limits'; + +// What happens before a project opens at its short address (design `demo-as-github-project.md` 3.4a): +// 1. every other spelling of the address is replaced by the canonical one (a redirect, query and fragment kept); +// 2. an address that names a branch is looked up once against the repository's default branch, and when it IS the +// default branch the address is replaced by the `HEAD` spelling, so a default-branch project has one id; +// 3. an address that cannot be a project, or has no project file, shows a page that says so, instead of a broken one. +// Everything here is plain functions and small root services, so each step can be driven with fakes. + +/** The `/project/github.com/…` address of a GitHub project, read from the router's (decoded) path segments. */ +export type ShortGithubAddress = + /** Not under `/project/github.com`: not this route's business. */ + | { readonly kind: 'not-short-github' } + /** Under `/project/github.com` but not an address that can be a project. */ + | { readonly kind: 'refused'; readonly reason: ProjectUrlErrorReason } + | { + readonly kind: 'project'; + readonly parts: IProjectUrlParts; + /** How many of the segments are the project locator; the rest is the page. */ + readonly locatorLength: number; + }; + +/** + * Reads the segments as the router has them (percent-decoded) with `parseProjectUrl`, the one place that knows the + * shapes. Each segment is encoded again first, so a `%2F` or a `%` in a segment reads as the text it came from. + */ +export function readShortGithubAddress( + segments: readonly string[], +): ShortGithubAddress { + if (segments[0] !== 'project' || segments[1] !== GITHUB_STORE_ID) { + return { kind: 'not-short-github' }; + } + const parsed = parseProjectUrl( + '/' + segments.map((s) => encodeURIComponent(s)).join('/'), + ); + if (!parsed.ok) { + return { kind: 'refused', reason: parsed.reason }; + } + const pageSegments = + parsed.rest === '' ? 0 : parsed.rest.split('/').length - 1; + return { + kind: 'project', + parts: parsed, + locatorLength: segments.length - pageSegments, + }; +} + +/** What the visitor is told instead of a project. */ +export type GithubAddressProblem = + | { readonly kind: 'unsupported'; readonly reason: ProjectUrlErrorReason } + | { + readonly kind: 'not-found'; + readonly owner: string; + readonly repo: string; + /** The typed branch, tag or commit; absent for the default branch. */ + readonly ref?: string; + readonly folder: string; + /** GitHub answered with a redirect: a renamed or moved repository. */ + readonly moved: boolean; + }; + +/** The problem the page being shown is about. Set by the check, read by the page. */ +@Injectable({ providedIn: 'root' }) +export class GithubAddressProblemState { + readonly problem = signal(undefined); +} + +/** What the lookup of a repository's default branch came to. */ +export type DefaultBranchLookup = + | { readonly kind: 'found'; readonly branch: string } + /** The repository is not there (or has moved): nothing to compare with, and nothing to warn about. */ + | { readonly kind: 'absent' } + /** Refused (a rate limit), failed, timed out, or answered with something else. */ + | { readonly kind: 'refused' }; + +/** + * `GET api.github.com/repos//`, field `default_branch`; the one extra GitHub API call of design 3.4a, + * made only when an address names a branch. One answer per repository for the life of the page (a refusal too: a + * rate-limited visit must not ask again on every navigation). + */ +@Injectable({ providedIn: 'root' }) +export class GithubDefaultBranchLookup { + private readonly fetchFn = inject(GITHUB_FETCH); + private readonly answers = new Map>(); + + lookup(owner: string, repo: string): Promise { + const key = `${owner}/${repo}`; + let answer = this.answers.get(key); + if (!answer) { + answer = this.ask(owner, repo); + this.answers.set(key, answer); + } + return answer; + } + + private async ask(owner: string, repo: string): Promise { + const outcome = await githubGet( + this.fetchFn, + `https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}`, + { + maxBytes: MAX_PROJECT_FILE_BYTES, + accept: 'application/vnd.github+json', + detectMoved: true, + timeoutMs: GITHUB_RESOLVE_TIMEOUT_MS, + }, + ); + if (outcome.kind === 'missing' || outcome.kind === 'moved') { + return { kind: 'absent' }; + } + if (outcome.kind !== 'ok') { + return { kind: 'refused' }; + } + try { + const { default_branch: branch } = JSON.parse(outcome.text) as { + default_branch?: unknown; + }; + return typeof branch === 'string' && branch !== '' + ? { kind: 'found', branch } + : { kind: 'refused' }; + } catch { + return { kind: 'refused' }; + } + } +} + +/** Tells the visitor something about the address that does not stop the page: a toast. */ +@Injectable({ providedIn: 'root' }) +export class GithubAddressNotices { + private readonly toast = inject(ToastController); + + /** The default branch could not be looked up: the address is left as typed, and its history is its own. */ + defaultBranchUnknown(owner: string, repo: string, ref: string): void { + void this.toast + .create({ + message: `Could not check whether "${ref}" is the default branch of ${owner}/${repo}. This address is kept as typed, and its chat history is kept separately from the default branch's.`, + duration: 10_000, + position: 'bottom', + buttons: [{ text: 'OK', role: 'cancel' }], + }) + .then((toast) => toast.present()); + } +} + +/** Where the check stands in the navigation: the query and fragment a redirect keeps. */ +export interface NavigationParts { + readonly queryParams: Record; + readonly fragment: string | null; +} + +/** What the check decides for a navigation to a short GitHub address. */ +export type AddressDecision = + /** Not this route's address, or nothing wrong: let the project route take it. */ + | { readonly kind: 'open' } + /** Replace the address by this path (query and fragment kept). */ + | { readonly kind: 'redirect'; readonly path: string } + /** Show the page that says there is a problem. */ + | { readonly kind: 'problem'; readonly problem: GithubAddressProblem }; + +/** + * The path of the same project and page with the ref `HEAD` instead of the named one, in its canonical spelling. + * `parts` is a canonical address that names a ref, so its path has `/tree/` right after the repository, + * and the same path with `HEAD` there is an address that parses. + */ +function defaultBranchPath( + parts: IProjectUrlParts, + github: IGithubProjectParts & { readonly ref: string }, +): string { + const prefix = `/project/${GITHUB_STORE_ID}/${github.owner}/${github.repo}/tree/`; + const named = prefix + encodeURIComponent(github.ref); + const replaced = parseProjectUrl( + prefix + + GITHUB_DEFAULT_BRANCH_REF + + parts.canonicalPath.slice(named.length), + ) as IProjectUrlParts; + return replaced.canonicalPath; +} + +/** The decisions, in the order of the design. Injectable so the router glue stays a one-liner and a test fakes the network. */ +@Injectable({ providedIn: 'root' }) +export class GithubAddressCheck { + private readonly branches = inject(GithubDefaultBranchLookup); + private readonly notices = inject(GithubAddressNotices); + private readonly store = inject(DatatugStoreGithubService); + /** Projects whose first read ended in anything but "not found" (a project that was there is not asked about again). */ + private readonly probed = new Set(); + + async decide(segments: readonly string[]): Promise { + const address = readShortGithubAddress(segments); + if (address.kind === 'not-short-github') { + return { kind: 'open' }; + } + if (address.kind === 'refused') { + return { + kind: 'problem', + problem: { kind: 'unsupported', reason: address.reason }, + }; + } + const { parts } = address; + if (!parts.isCanonical) { + return { kind: 'redirect', path: parts.canonicalPath }; + } + // A project at `/project/github.com/…` is always a GitHub project: its parts are always there. + const github = parts.github as IGithubProjectParts; + if (github.ref !== undefined) { + const lookup = await this.branches.lookup(github.owner, github.repo); + if (lookup.kind === 'found' && lookup.branch === github.ref) { + return { + kind: 'redirect', + path: defaultBranchPath(parts, { ...github, ref: github.ref }), + }; + } else if (lookup.kind === 'refused') { + this.notices.defaultBranchUnknown( + github.owner, + github.repo, + github.ref, + ); + } + } + if (!this.probed.has(parts.projectId)) { + const missing = await this.probe(parts.projectId); + if (missing) { + return { + kind: 'problem', + problem: { + kind: 'not-found', + owner: github.owner, + repo: github.repo, + ...(github.ref !== undefined ? { ref: github.ref } : {}), + folder: github.folder, + moved: missing === 'moved', + }, + }; + } + this.probed.add(parts.projectId); + } + return { kind: 'open' }; + } + + /** `'missing'` or `'moved'` when GitHub says there is no project at this id; undefined otherwise, errors included. */ + private async probe( + projectId: string, + ): Promise<'missing' | 'moved' | undefined> { + try { + await firstValueFrom(this.store.getProjectSummary(projectId)); + return undefined; + } catch (err) { + return err instanceof GithubProjectNotFoundError ? err.reason : undefined; + } + } +} diff --git a/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts b/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts new file mode 100644 index 00000000..3798f5b3 --- /dev/null +++ b/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts @@ -0,0 +1,434 @@ +import { Component } from '@angular/core'; +import { TestBed } from '@angular/core/testing'; +import { + ActivatedRouteSnapshot, + provideRouter, + Router, + Routes, + UrlSegment, +} from '@angular/router'; +import { PRODUCT_PROFILE, PRODUCT_PROFILES } from '@datatug/product-profiles'; +import { of, throwError } from 'rxjs'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { DatatugStoreGithubService } from '../services/repo/datatug-store.service.github'; +import { GithubProjectNotFoundError } from '../services/repo/github/github-project-reader.service'; +import { + GithubAddressCheck, + GithubAddressNotices, + GithubAddressProblemState, + GithubDefaultBranchLookup, +} from './github-project-address-check'; +import { GithubAddressProblemPageComponent } from './github-address-problem-page.component'; +import { DatatugProjectRoutingModule } from './datatug-routing-proj'; +import { datatugRoutes } from './datatug-routing.module'; +import { + githubAddressCanMatch, + githubAddressMatcher, + githubProjectMatcher, + githubProjectRoutes, +} from './github-project-routes'; + +// The short project route (design `demo-as-github-project.md` 3.4): a matcher route with the project pages as +// children that supplies `storeId` and `projectId`, the redirects of 3.4a, and the problem page. + +@Component({ selector: 'sneat-stub-problem', template: '' }) +class ProblemStub {} +@Component({ selector: 'sneat-stub-page', template: '' }) +class PageStub {} +@Component({ selector: 'sneat-stub-legacy', template: '' }) +class LegacyStub {} +@Component({ selector: 'sneat-stub-other', template: '' }) +class OtherStub {} + +const segments = (path: string): UrlSegment[] => + path + .split('/') + .filter(Boolean) + .map((p) => new UrlSegment(decodeURIComponent(p), {})); + +describe('githubProjectMatcher', () => { + const run = (path: string) => + githubProjectMatcher(segments(path), {} as never, {} as never); + + it.each([ + // [path, consumed, storeId, projectId] + [ + '/project/github.com/datatug/chinook-demo', + 4, + 'github.com', + 'chinook-demo@datatug@', + ], + [ + '/project/github.com/datatug/chinook-demo/chat', + 4, + 'github.com', + 'chinook-demo@datatug@', + ], + [ + '/project/github.com/Datatug/Chinook-Demo/queries', + 4, + 'github.com', + 'chinook-demo@datatug@', + ], + [ + '/project/github.com/o/r/tree/HEAD/demo-project-1/-/chat', + 8, + 'github.com', + 'r@o@demo-project-1', + ], + ['/project/github.com/o/r/tree/HEAD/datatug', 7, 'github.com', 'r@o'], + [ + '/project/github.com/o/r/tree/v1.0.0/-/queries', + 7, + 'github.com', + 'r@o@@v1.0.0', + ], + ])( + '%s consumes %i segments and supplies the canonical ids', + (path, consumed, storeId, projectId) => { + const result = run(path); + expect(result?.consumed).toHaveLength(consumed); + expect(result?.posParams?.['storeId'].path).toBe(storeId); + expect(result?.posParams?.['projectId'].path).toBe(projectId); + }, + ); + + it.each([ + '/store/github.com/project/r@o@/chat', + '/project/github.com/o', + '/project/github.com/o/r/blob/main/x.txt', + '/project/github.com/o/r/tree', + '/project/gitlab.com/o/r', + '/', + ])('does not match %s', (path) => { + expect(run(path)).toBeNull(); + }); +}); + +describe('githubAddressMatcher', () => { + const run = (path: string) => + githubAddressMatcher(segments(path), {} as never, {} as never); + + it('consumes the whole of every /project/github.com address, a project or not', () => { + for (const path of [ + '/project/github.com', + '/project/github.com/o', + '/project/github.com/o/r/chat', + '/project/github.com/o/r/tree', + ]) { + expect(run(path)?.consumed, path).toHaveLength(segments(path).length); + } + }); + + it('matches nothing else', () => { + for (const path of [ + '/', + '/store/github.com/project/r@o@', + '/project/gitlab.com/o/r', + '/projects/github.com/o', + ]) { + expect(run(path), path).toBeNull(); + } + }); +}); + +describe('the routes as registered', () => { + it('are two matcher routes, ahead of the store routes in the datatug-main route table', () => { + expect(githubProjectRoutes).toHaveLength(2); + expect(githubProjectRoutes.map((r) => r.matcher)).toEqual([ + githubAddressMatcher, + githubProjectMatcher, + ]); + const at = datatugRoutes.indexOf(githubProjectRoutes[0]); + expect(at).toBeGreaterThan(-1); + expect(datatugRoutes.slice(at, at + 2)).toEqual(githubProjectRoutes); + expect(at).toBeLessThan( + datatugRoutes.findIndex((r) => r.path === 'store/:storeId'), + ); + }); + + it('the project route is lazy, has the project pages as children, and has no component of its own', () => { + const route = githubProjectRoutes[1]; + expect(route.loadChildren).toBeTypeOf('function'); + expect(route.component).toBeUndefined(); + expect(route.loadComponent).toBeUndefined(); + expect(route.path).toBeUndefined(); + }); + + it('load the problem page and the project pages lazily, and nothing else', async () => { + const [problem, project] = githubProjectRoutes; + expect(await (problem.loadComponent as () => Promise)()).toBe( + GithubAddressProblemPageComponent, + ); + expect(await (project.loadChildren as () => Promise)()).toBe( + DatatugProjectRoutingModule, + ); + }); + + it('the old form is untouched: its route is still there, with no redirect', () => { + const old = datatugRoutes.find((r) => r.path === 'store/:storeId'); + expect(old).toBeDefined(); + expect(old?.redirectTo).toBeUndefined(); + }); + + it('every other route of the table is as it was', () => { + expect( + datatugRoutes + .filter((r) => !githubProjectRoutes.includes(r)) + .map((r) => r.path), + ).toEqual([ + '', + 'incidents', + 'incidents/new', + 'incidents/:storeId/:incidentId/record', + 'incidents/:storeId/:incidentId', + 'my', + 'explore/:spaceId', + 'explore-vault', + 'signed-out', + 'store/:storeId', + 'agent', + ]); + }); +}); + +describe('navigating to a short address', () => { + let summary: ReturnType; + let branch: ReturnType; + let notice: ReturnType; + + /** The real matchers and guard; the lazy parts replaced by stubs that report what the router gave them. */ + async function visit( + url: string, + options: { + profile?: keyof typeof PRODUCT_PROFILES; + lookup?: unknown; + summaryResult?: () => unknown; + } = {}, + ) { + branch = vi.fn( + async () => options.lookup ?? { kind: 'found', branch: 'main' }, + ); + notice = vi.fn(); + summary = vi.fn(options.summaryResult ?? (() => of({ id: 'x' }))); + const routes: Routes = [ + { + ...githubProjectRoutes[0], + loadComponent: () => ProblemStub, + }, + { + ...githubProjectRoutes[1], + loadChildren: () => + [ + { path: 'chat', component: PageStub }, + { path: 'queries', component: PageStub }, + { path: '', component: PageStub }, + ] as Routes, + }, + { path: 'store/:storeId/project/:projectId/chat', component: LegacyStub }, + { path: 'store/:storeId/project/:projectId', component: LegacyStub }, + { path: '**', component: OtherStub }, + ]; + TestBed.configureTestingModule({ + providers: [ + { + provide: PRODUCT_PROFILE, + useValue: PRODUCT_PROFILES[options.profile ?? 'datatug'], + }, + { provide: GithubDefaultBranchLookup, useValue: { lookup: branch } }, + { + provide: GithubAddressNotices, + useValue: { defaultBranchUnknown: notice }, + }, + { + provide: DatatugStoreGithubService, + useValue: { getProjectSummary: summary }, + }, + provideRouter(routes), + ], + }); + const router = TestBed.inject(Router); + const ok = await router.navigateByUrl(url); + let leaf: ActivatedRouteSnapshot = router.routerState.snapshot.root; + while (leaf.firstChild) leaf = leaf.firstChild; + return { + ok, + url: router.url, + component: leaf.component, + storeId: leaf.paramMap.get('storeId'), + projectId: leaf.paramMap.get('projectId'), + problem: TestBed.inject(GithubAddressProblemState).problem(), + }; + } + + beforeEach(() => TestBed.resetTestingModule()); + + it.each([ + ['/project/github.com/datatug/chinook-demo', 'chinook-demo@datatug@'], + ['/project/github.com/datatug/chinook-demo/chat', 'chinook-demo@datatug@'], + [ + '/project/github.com/datatug/chinook-demo/queries', + 'chinook-demo@datatug@', + ], + [ + '/project/github.com/o/r/tree/HEAD/demo-project-1/-/chat', + 'r@o@demo-project-1', + ], + ['/project/github.com/o/r/tree/HEAD/datatug', 'r@o'], + ['/project/github.com/o/r/tree/v1.0.0/-/queries', 'r@o@@v1.0.0'], + ])( + '%s opens the project pages as children, with storeId and projectId %s', + async (url, projectId) => { + const result = await visit(url); + expect(result.ok).toBe(true); + expect(result.url).toBe(url); + expect(result.component).toBe(PageStub); + expect(result.storeId).toBe('github.com'); + expect(result.projectId).toBe(projectId); + }, + ); + + it('the old form opens as before, with no redirect and no question asked of GitHub', async () => { + const result = await visit( + '/store/github.com/project/chinook-demo@datatug@/chat', + ); + expect(result.url).toBe( + '/store/github.com/project/chinook-demo@datatug@/chat', + ); + expect(result.component).toBe(LegacyStub); + expect(summary).not.toHaveBeenCalled(); + expect(branch).not.toHaveBeenCalled(); + }); + + describe('every spelling redirects to the canonical address, keeping the page, the query and the fragment', () => { + it.each([ + [ + '/project/github.com/Datatug/Chinook-Demo', + '/project/github.com/datatug/chinook-demo', + ], + [ + '/project/github.com/Datatug/Chinook-Demo/chat?msg=x&lang=ru#top', + '/project/github.com/datatug/chinook-demo/chat?msg=x&lang=ru#top', + ], + [ + '/project/github.com/o/r.git/queries?order-tags-by=count', + '/project/github.com/o/r/queries?order-tags-by=count', + ], + [ + '/project/github.com/o/r/tree/HEAD/-/chat?x=1', + '/project/github.com/o/r/chat?x=1', + ], + ['/project/github.com/o/r/tree/HEAD', '/project/github.com/o/r'], + [ + '/project/github.com/o/r/blob/v1/dir/datatug-project.json', + '/project/github.com/o/r/tree/v1/dir', + ], + [ + '/project/github.com/o/r/tree/main/dir/-/chat?a=1&a=2#f', + '/project/github.com/o/r/tree/HEAD/dir/-/chat?a=1&a=2#f', + ], + ])('%s -> %s', async (typed, canonical) => { + const result = await visit(typed); + expect(result.url).toBe(canonical); + expect(result.component).toBe(PageStub); + }); + }); + + describe('what is not supported shows a page that says so, at the address as typed', () => { + it.each([ + ['/project/github.com/o/r/blob/main/dir/file.txt', 'file-link'], + ['/project/github.com/o/r/tree/HEAD/a@b', 'at-sign-not-supported'], + [ + '/project/github.com/o/r/blob/main/a/-/datatug-project.json', + 'dash-directory-not-supported', + ], + ['/project/github.com/o', 'not-a-project-address'], + ])('%s (%s)', async (url, reason) => { + const result = await visit(url); + expect(result.url).toBe(url); + expect(result.component).toBe(ProblemStub); + expect(result.problem).toEqual({ kind: 'unsupported', reason }); + expect(summary).not.toHaveBeenCalled(); + }); + + it('no project file: "No DataTug project here"', async () => { + const result = await visit('/project/github.com/o/r/tree/feature/x', { + summaryResult: () => + throwError(() => new GithubProjectNotFoundError('x', 'missing')), + }); + expect(result.component).toBe(ProblemStub); + expect(result.url).toBe('/project/github.com/o/r/tree/feature/x'); + expect(result.problem).toEqual({ + kind: 'not-found', + owner: 'o', + repo: 'r', + ref: 'feature', + folder: 'x', + moved: false, + }); + }); + }); + + it('a refused default-branch lookup leaves the address as typed, tells the visit, and opens the project', async () => { + const result = await visit('/project/github.com/o/r/tree/main/dir/-/chat', { + lookup: { kind: 'refused' }, + }); + expect(result.url).toBe('/project/github.com/o/r/tree/main/dir/-/chat'); + expect(result.component).toBe(PageStub); + expect(result.projectId).toBe('r@o@dir@main'); + expect(notice).toHaveBeenCalledWith('o', 'r', 'main'); + }); + + describe('the Incidentius profile does not have the short route', () => { + it.each([ + '/project/github.com/datatug/chinook-demo', + '/project/github.com/datatug/chinook-demo/chat', + '/project/github.com/Datatug/Chinook-Demo', + '/project/github.com/o/r/blob/main/x.txt', + ])( + '%s is as before the route existed: it matches none of these routes', + async (url) => { + const result = await visit(url, { profile: 'incidentius' }); + expect(result.component).toBe(OtherStub); + expect(result.url).toBe(url); + expect(summary).not.toHaveBeenCalled(); + expect(branch).not.toHaveBeenCalled(); + }, + ); + + it('a redirect asked for outside a navigation still keeps nothing but the path', async () => { + TestBed.configureTestingModule({ + providers: [ + { provide: PRODUCT_PROFILE, useValue: PRODUCT_PROFILES.datatug }, + { + provide: GithubAddressCheck, + useValue: { + decide: async () => ({ + kind: 'redirect', + path: '/project/github.com/o/r', + }), + }, + }, + ], + }); + const result = await TestBed.runInInjectionContext(() => + githubAddressCanMatch({} as never, segments('/project/github.com/O/r')), + ); + expect(TestBed.inject(Router).serializeUrl(result as never)).toBe( + '/project/github.com/o/r', + ); + }); + + it('the guard itself says no', async () => { + TestBed.configureTestingModule({ + providers: [ + { provide: PRODUCT_PROFILE, useValue: PRODUCT_PROFILES.incidentius }, + ], + }); + const result = await TestBed.runInInjectionContext(() => + githubAddressCanMatch({} as never, segments('/project/github.com/o/r')), + ); + expect(result).toBe(false); + }); + }); +}); diff --git a/libs/datatug/main/src/lib/routes/github-project-routes.ts b/libs/datatug/main/src/lib/routes/github-project-routes.ts new file mode 100644 index 00000000..afa5f060 --- /dev/null +++ b/libs/datatug/main/src/lib/routes/github-project-routes.ts @@ -0,0 +1,107 @@ +import { inject } from '@angular/core'; +import { + CanMatchFn, + Router, + Routes, + UrlMatcher, + UrlSegment, +} from '@angular/router'; +import { PRODUCT_PROFILE } from '@datatug/product-profiles'; +import { + GithubAddressCheck, + GithubAddressProblemState, + readShortGithubAddress, +} from './github-project-address-check'; +import { + routingParamProjectId, + routingParamStoreId, +} from '../core/datatug-routing-params'; + +// The short project route (design `demo-as-github-project.md` 3.3, 3.4, 3.4a): +// /project/github.com//[/tree/[/…]][/-/…] +// opens the same project pages as `/store/github.com/project/@@/` (which keeps working, +// with no redirect, until the held task G-A1d). A matcher consumes the project locator and supplies the two +// positional parameters every page and tracker already reads (`storeId`, `projectId`); the project pages are its +// children, unchanged. Only the DataTug product profile has it: the other profiles do not match it at all, as +// before this route existed. +// +// Two routes share the address, in this order: +// 1. the address check (github-project-address-check.ts), which matches every `/project/github.com/…` address +// and either redirects to the canonical spelling, or says there is a problem (an address that cannot be a +// project, or no project file there), or does not match so that route 2 takes the address; +// 2. the project. + +const paths = (segments: readonly UrlSegment[]): string[] => + segments.map((segment) => segment.path); + +/** Consumes the locator of a project at its short address, and supplies `storeId` and `projectId` (canonical). */ +export const githubProjectMatcher: UrlMatcher = (segments) => { + const address = readShortGithubAddress(paths(segments)); + if (address.kind !== 'project') { + return null; + } + return { + consumed: segments.slice(0, address.locatorLength), + posParams: { + [routingParamStoreId]: new UrlSegment(address.parts.storeId, {}), + [routingParamProjectId]: new UrlSegment(address.parts.projectId, {}), + }, + }; +}; + +/** Consumes every `/project/github.com/…` address, whole: whether it is a project or not is the check's to say. */ +export const githubAddressMatcher: UrlMatcher = (segments) => + readShortGithubAddress(paths(segments)).kind === 'not-short-github' + ? null + : { consumed: segments }; + +/** The short route belongs to the DataTug product profile only. */ +export const datatugProfileForShortRoute = (): boolean => + inject(PRODUCT_PROFILE).id === 'datatug'; + +/** + * `canMatch` of the address route. True when the page to show is the problem page (it has been told which problem); + * a UrlTree for the canonical spelling; false when the address is a project that the project route should open. + */ +export const githubAddressCanMatch: CanMatchFn = async (_route, segments) => { + if (!datatugProfileForShortRoute()) { + return false; + } + const router = inject(Router); + const check = inject(GithubAddressCheck); + const state = inject(GithubAddressProblemState); + const typed = router.getCurrentNavigation()?.extractedUrl; + const decision = await check.decide(paths(segments)); + switch (decision.kind) { + case 'redirect': { + const target = router.parseUrl(decision.path); + target.queryParams = { ...(typed?.queryParams ?? {}) }; + target.fragment = typed?.fragment ?? null; + return target; + } + case 'problem': + state.problem.set(decision.problem); + return true; + default: + return false; + } +}; + +export const githubProjectRoutes: Routes = [ + { + matcher: githubAddressMatcher, + canMatch: [githubAddressCanMatch], + loadComponent: () => + import('./github-address-problem-page.component').then( + (m) => m.GithubAddressProblemPageComponent, + ), + }, + { + matcher: githubProjectMatcher, + canMatch: [datatugProfileForShortRoute], + loadChildren: () => + import('./datatug-routing-proj').then( + (m) => m.DatatugProjectRoutingModule, + ), + }, +]; diff --git a/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.spec.ts b/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.spec.ts index 198b22c8..be94058e 100644 --- a/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.spec.ts +++ b/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.spec.ts @@ -219,3 +219,106 @@ describe('DatatugNavContextService — a /project/ address with no /store/ segme expect(seen['env']).toMatchObject({ id: 'local' }); }); }); + +/** + * The short address of a GitHub project (`/project/github.com//…`, design `demo-as-github-project.md` + * 3.4), once the short project route has opened it: the side menu and the pages that read the nav context see the + * same store, project, environment and table as at the old address of the same project. An address that no project + * route has opened (the hand-off holding page answers the same shape) still names nothing. + */ +describe('DatatugNavContextService — a short GitHub project address opened by the short project route', () => { + /** The router state of a page whose deepest route has these parameters. */ + const stateWith = (params: Record) => ({ + snapshot: { + root: { + firstChild: { + firstChild: { paramMap: { get: (name: string) => params[name] ?? null } }, + }, + }, + }, + }); + + function at(url: string, params?: Record) { + const logError = vi.fn(); + const watchProjectSummary = vi.fn(() => of(undefined)); + window.history.replaceState({}, '', url); + TestBed.configureTestingModule({ + providers: [ + DatatugNavContextService, + { provide: AppContextService, useValue: { currentApp: of({ appCode: 'datatug' }) } }, + { + provide: ProjectContextService, + useValue: { current: undefined, setCurrent: vi.fn(), current$: of(undefined) }, + }, + { + provide: Router, + useValue: { + events: of(), + navigate: vi.fn(), + ...(params ? { routerState: stateWith(params) } : {}), + }, + }, + { provide: ProjectService, useValue: { watchProjectSummary, getFull: vi.fn() } }, + { provide: EnvironmentService, useValue: { getEnvSummary: vi.fn(() => of(undefined)) } }, + { provide: ErrorLogger, useValue: { logError, logErrorHandler: vi.fn(() => vi.fn()) } }, + ], + }); + const service = TestBed.inject(DatatugNavContextService); + const seen: Record = {}; + service.currentProject.subscribe((p) => (seen['project'] = p)); + service.currentEnv.subscribe((e) => (seen['env'] = e)); + service.currentStoreId.subscribe((id) => (seen['store'] = id)); + return { logError, watchProjectSummary, seen }; + } + + beforeEach(() => sessionStorage.clear()); + + it.each([ + ['/project/github.com/datatug/chinook-demo', 'chinook-demo@datatug@'], + ['/project/github.com/datatug/chinook-demo/chat', 'chinook-demo@datatug@'], + ['/project/github.com/o/r/tree/HEAD/demo-project-1/-/queries?tab=shared', 'r@o@demo-project-1'], + ['/project/github.com/o/r/tree/v1.0.0/-/chat', 'r@o@@v1.0.0'], + ['http://localhost:3000/project/github.com/o/r/tree/HEAD/datatug', 'r@o'], + ])('%s is the project %s of the GitHub store', (url, projectId) => { + const { logError, watchProjectSummary, seen } = at(url, { storeId: 'github.com', projectId }); + expect(logError).not.toHaveBeenCalled(); + expect(seen['store']).toBe('github.com'); + expect(seen['project']).toMatchObject({ ref: { storeId: 'github.com', projectId } }); + expect(watchProjectSummary).toHaveBeenCalledWith({ storeId: 'github.com', projectId }); + }); + + it('reads an environment of the project from the page, as at the old address', () => { + const { seen } = at('/project/github.com/o/r/env/local', { storeId: 'github.com', projectId: 'r@o@' }); + expect(seen['store']).toBe('github.com'); + expect(seen['env']).toMatchObject({ id: 'local' }); + }); + + it('reads the same project as the old address of it', () => { + const projectId = 'chinook-demo@datatug@'; + const short = at('/project/github.com/datatug/chinook-demo/chat', { storeId: 'github.com', projectId }); + const shortProject = short.seen['project']; + TestBed.resetTestingModule(); + const old = at('/store/github.com/project/chinook-demo@datatug@/chat'); + expect(shortProject).toEqual(old.seen['project']); + expect(short.seen['store']).toEqual(old.seen['store']); + }); + + it.each([ + ['no route has opened it (the holding page answers this address)', undefined], + ['the route that opened it is for another project', { storeId: 'github.com', projectId: 'other@o@' }], + ['the route that opened it is for another store', { storeId: 'localhost:8989', projectId: 'r@o@' }], + ['the route that opened it has no project', {}], + ])('names nothing when %s', (_name, params) => { + const { logError, watchProjectSummary, seen } = at('/project/github.com/o/r/chat', params); + expect(logError).not.toHaveBeenCalled(); + expect(watchProjectSummary).not.toHaveBeenCalled(); + expect(seen['store']).toBeUndefined(); + expect(seen['project']).toBeUndefined(); + }); + + it('names nothing for a /project/ address that is not a project at all', () => { + const { logError, seen } = at('/project/github.com/o', { storeId: 'github.com', projectId: 'o@' }); + expect(logError).not.toHaveBeenCalled(); + expect(seen['project']).toBeUndefined(); + }); +}); diff --git a/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.ts b/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.ts index b19fa4c6..dfbcc678 100644 --- a/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.ts +++ b/libs/datatug/main/src/lib/services/nav/datatug-nav-context.service.ts @@ -24,6 +24,7 @@ import { IEnvDbTableContext, IProjectContext, parseDatatugStoreRef, + parseProjectUrl, populateProjectBriefFromSummaryIfMissing, } from '../../nav/nav-models'; import { ProjectContextService } from '../project/project-context.service'; @@ -37,18 +38,22 @@ const reStore = /\/store\/(.+?)($|\/)/, reEnvDb = /\/env\/\w+\/db\/(.+?)(?:\/|$)/, reTable = /\/table\/(.+?)(?:\/|$)/; +/** The path of a URL as typed (`https://host/a/b?x#y` and `/a/b?x#y` both give `/a/b`). */ +function pathOf(url: string): string { + return /^(?:[a-z][a-z0-9+.-]*:\/\/[^/?#]*)?([^?#]*)/i.exec(url)?.[1] ?? ''; +} + /** - * Whether the URL's first path segment is `project`: the hand-off address - * `/project/github.com///chat` (datatug-app's demo holding page). No route of this app starts with - * `project` (the routes that carry a store start `/store//`), so such an address names no store, project, - * environment or table, whatever its owner or repository is called: a repository called `store`, `env` or - * `table` must not be read as one. Matrix parameters, letter case and percent-encoding are ignored, as the - * router ignores them. + * Whether the URL's first path segment is `project`: the short address of a GitHub project, + * `/project/github.com//…` (design `demo-as-github-project.md` 3.4), or the hand-off address + * `/project/github.com///chat` that datatug-app's holding page answers. No route of this app starts + * with `project` other than those (the routes that carry a store start `/store//`), so such an address names + * a store, project, environment or table only when a project route has opened it (see `legacyShapeOf`), whatever its + * owner or repository is called: a repository called `store`, `env` or `table` must not be read as one. Matrix + * parameters, letter case and percent-encoding are ignored, as the router ignores them. */ function startsWithProjectSegment(url: string): boolean { - const path = - /^(?:[a-z][a-z0-9+.-]*:\/\/[^/?#]*)?([^?#]*)/i.exec(url)?.[1] ?? ''; - let first = path.split('/')[1]?.split(/[;(]/)[0] ?? ''; + let first = pathOf(url).split('/')[1]?.split(/[;(]/)[0] ?? ''; try { first = decodeURIComponent(first); } catch { @@ -266,10 +271,36 @@ export class DatatugNavContextService { //} } + /** + * The URL as the regular expressions below read it. A short GitHub address that the short project route has + * opened (the router state has its `storeId` and `projectId`, so a hand-off address that shows the holding page is + * not one) is read as the old form of the same project and page: `/store/github.com/project//`. + * Any other `/project/…` address names no store, project, environment or table: it reads as the bare root, and + * all of them clear. + */ + private legacyShapeOf(rawUrl: string): string { + if (!startsWithProjectSegment(rawUrl)) { + return rawUrl; + } + const parsed = parseProjectUrl(pathOf(rawUrl)); + if (parsed.ok) { + let leaf = this.router.routerState?.snapshot?.root; + while (leaf?.firstChild) { + leaf = leaf.firstChild; + } + if ( + leaf?.paramMap.get('projectId') === parsed.projectId && + leaf.paramMap.get('storeId') === parsed.storeId + ) { + return `/store/${parsed.storeId}/project/${parsed.projectId}${parsed.rest}`; + } + } + return '/'; + } + private processUrl(rawUrl: string): void { // console.log('DatatugNavContextService.processUrl():', url); - // An address that names no store, project, environment or table reads as the bare root: all of them clear. - const url = startsWithProjectSegment(rawUrl) ? '/' : rawUrl; + const url = this.legacyShapeOf(rawUrl); try { this.processStore(url); this.processProject(url); From 071eddf8db4b5b8fc4ed94b12cbb7a7deb3bf495 Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Fri, 2 Oct 2026 22:42:21 +0100 Subject: [PATCH 4/8] test(project): the trailing-slash row of design 3.4a Co-Authored-By: Claude Sonnet 5.5 --- .../main/src/lib/routes/github-project-routes.spec.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts b/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts index 3798f5b3..ccb6478c 100644 --- a/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts +++ b/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts @@ -334,6 +334,16 @@ describe('navigating to a short address', () => { }); }); + it.each([ + ['/project/github.com/o/r/', '/project/github.com/o/r'], + ['/project/github.com/o/r/chat/', '/project/github.com/o/r/chat'], + ])('a trailing slash (%s) is dropped by the router: the canonical address %s opens, with no second navigation', async (typed, canonical) => { + const result = await visit(typed); + expect(result.url).toBe(canonical); + expect(result.component).toBe(PageStub); + expect(result.projectId).toBe('r@o@'); + }); + describe('what is not supported shows a page that says so, at the address as typed', () => { it.each([ ['/project/github.com/o/r/blob/main/dir/file.txt', 'file-link'], From 634c02db5d4a12f4c7e0780354a96045992ec8ef Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Fri, 2 Oct 2026 23:28:05 +0100 Subject: [PATCH 5/8] fix(project): short address in any letter case, redirects without re-parsing, probe timeout, toast once (G-A1b review r1) B1: the fixed segments of a short address (project, github.com, tree and the first page) are read in any letter case, as the hand-off route and index.html read them, and are another spelling of the canonical address: the route redirects to the lower-case address, query and fragment kept, instead of matching no route (NG04002, crash dialog). Only ASCII letters are folded. Minor 3: a redirect is built from segments, not parsed from a string, so a folder with ( or ) no longer opens another project (the router read it as an outlet group). Minor 1: the refused default-branch-lookup toast is shown once per repository and ref for the life of the page. Minor 2: the project probe is raced against a named 3 s timeout (injectable timer); when GitHub does not answer the route matches and the pages show their own loading or error state, as the old form does. Co-Authored-By: Claude Sonnet 5.5 --- .../github-project-address-check.spec.ts | 160 +++++++++++++++++- .../routes/github-project-address-check.ts | 105 +++++++++--- .../lib/routes/github-project-routes.spec.ts | 71 ++++++++ .../src/lib/routes/github-project-routes.ts | 36 +++- 4 files changed, 341 insertions(+), 31 deletions(-) diff --git a/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts b/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts index 872b7298..0c2ac18d 100644 --- a/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts +++ b/libs/datatug/main/src/lib/routes/github-project-address-check.spec.ts @@ -1,5 +1,5 @@ import { TestBed } from '@angular/core/testing'; -import { of, throwError } from 'rxjs'; +import { from, NEVER, of, throwError } from 'rxjs'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { DatatugStoreGithubService } from '../services/repo/datatug-store.service.github'; import { @@ -8,6 +8,8 @@ import { } from '../services/repo/github/github-project-reader.service'; import { GithubAddressCheck, + GITHUB_PROBE_TIMEOUT_MS, + GITHUB_PROBE_TIMER, GithubAddressNotices, GithubDefaultBranchLookup, readShortGithubAddress, @@ -81,10 +83,9 @@ describe('readShortGithubAddress: the table of design 3.3', () => { '/', '/store/github.com/project/r@o@', '/projects/github.com/o/r', - '/Project/github.com/o/r', - '/project/GitHub.com/o/r', '/project/gitlab.com/o/r', '/project', + '/Project', ]) { expect(readShortGithubAddress(segs(path)), path).toEqual({ kind: 'not-short-github', @@ -130,6 +131,87 @@ describe('readShortGithubAddress: the table of design 3.3', () => { }); }); + // B1 of the review of G-A1b: the fixed segments of a short address (`project`, `github.com`, `tree`, the first + // page) match in any letter case, as the hand-off route's matcher and index.html's script do, and the address is + // another spelling of the canonical one (so it redirects), never an address that matches no route. + describe('the fixed segments in any letter case are another spelling of the canonical address', () => { + it.each([ + [ + '/Project/GitHub.com/datatug/chinook-demo/Chat', + '/project/github.com/datatug/chinook-demo/chat', + 4, + ], + [ + '/PROJECT/GITHUB.COM/datatug/chinook-demo/chat', + '/project/github.com/datatug/chinook-demo/chat', + 4, + ], + [ + '/project/github.com/datatug/chinook-demo/Chat', + '/project/github.com/datatug/chinook-demo/chat', + 4, + ], + [ + '/project/github.com/datatug/chinook-demo/Tree/HEAD/-/chat', + '/project/github.com/datatug/chinook-demo/chat', + 7, + ], + [ + '/project/github.com/o/r/TREE/v1.0.0/-/CHAT', + '/project/github.com/o/r/tree/v1.0.0/-/chat', + 7, + ], + [ + '/project/github.com/o/r/Tree/HEAD/Dir/-/Queries/Sub', + '/project/github.com/o/r/tree/HEAD/Dir/-/queries/Sub', + 8, + ], + ['/Project/github.com/o/r/Queries', '/project/github.com/o/r/queries', 4], + ['/project/GitHub.com/o/r', '/project/github.com/o/r', 4], + ])('%s -> %s', (typed, canonical, locatorLength) => { + const result = readShortGithubAddress(segs(typed)); + expect(result).toMatchObject({ kind: 'project', locatorLength }); + if (result.kind === 'project') { + expect(result.parts.isCanonical).toBe(false); + expect(result.parts.canonicalPath).toBe(canonical); + } + }); + + it('only the first page segment is lower-cased: what follows it is left as typed', () => { + const result = readShortGithubAddress( + segs('/project/github.com/o/r/queries/ABC/Def'), + ); + expect(result).toMatchObject({ kind: 'project' }); + if (result.kind === 'project') { + expect(result.parts.isCanonical).toBe(true); + expect(result.parts.rest).toBe('/queries/ABC/Def'); + } + }); + + it('a lower-case address is canonical, as before', () => { + const result = readShortGithubAddress( + segs('/project/github.com/datatug/chinook-demo/chat'), + ); + expect(result).toMatchObject({ + kind: 'project', + parts: { isCanonical: true }, + }); + }); + + it('a repository called like a fixed segment is still a repository: `Tree` and `Chat` in the repo place are names', () => { + const result = readShortGithubAddress( + segs('/project/github.com/o/Tree/Chat'), + ); + expect(result).toMatchObject({ kind: 'project', locatorLength: 4 }); + if (result.kind === 'project') { + expect(result.parts.projectId).toBe('tree@o@'); + expect(result.parts.canonicalPath).toBe( + '/project/github.com/o/tree/chat', + ); + } + }); + }); + it('reads an encoded slash in a page as part of one segment', () => { const result = readShortGithubAddress([ 'project', @@ -227,10 +309,16 @@ describe('GithubAddressCheck.decide', () => { let notice: ReturnType; let summary: ReturnType; + /** The probe's timer, stood in for: `fireTimeout()` is the moment the time is up. */ + let timerCalls: number[]; + let fireTimeout: () => void; + const check = ( lookupResult: unknown = { kind: 'found', branch: 'main' }, summaryResult: () => unknown = () => of({ id: 'x' }), ) => { + timerCalls = []; + const timeUp = new Promise((resolve) => (fireTimeout = resolve)); branch = vi.fn(async () => lookupResult); notice = vi.fn(); summary = vi.fn(summaryResult); @@ -245,6 +333,13 @@ describe('GithubAddressCheck.decide', () => { provide: DatatugStoreGithubService, useValue: { getProjectSummary: summary }, }, + { + provide: GITHUB_PROBE_TIMER, + useValue: (ms: number) => { + timerCalls.push(ms); + return timeUp; + }, + }, ], }); return TestBed.inject(GithubAddressCheck); @@ -407,6 +502,26 @@ describe('GithubAddressCheck.decide', () => { expect(summary).toHaveBeenCalledWith('r@o@dir@main'); }); + it('the toast of a refused lookup is shown once per repository and ref for the life of the page, not on every navigation (review minor 1)', async () => { + const c = check({ kind: 'refused' }); + for (const typed of [ + '/project/github.com/o/r/tree/main/dir/-/chat', + '/project/github.com/o/r/tree/main/dir/-/queries', + '/project/github.com/o/r/tree/main/other', + ]) { + expect(await c.decide(segs(typed))).toEqual({ kind: 'open' }); + } + expect(notice).toHaveBeenCalledTimes(1); + expect(notice).toHaveBeenCalledWith('o', 'r', 'main'); + await c.decide(segs('/project/github.com/o/r/tree/develop')); + await c.decide(segs('/project/github.com/o/other/tree/main')); + expect(notice.mock.calls).toEqual([ + ['o', 'r', 'main'], + ['o', 'r', 'develop'], + ['o', 'other', 'main'], + ]); + }); + it('when the repository is not there, there is nothing to warn about: the missing project is what is shown', async () => { const decision = await check({ kind: 'absent' }, () => throwError( @@ -568,6 +683,45 @@ describe('GithubAddressCheck.decide', () => { ); }); + describe('GitHub that does not answer must not leave a blank page (review minor 2)', () => { + it('the time allowed is a short, named one', () => { + expect(GITHUB_PROBE_TIMEOUT_MS).toBeGreaterThanOrEqual(2000); + expect(GITHUB_PROBE_TIMEOUT_MS).toBeLessThanOrEqual(4000); + }); + + it('a probe that is still waiting when the time is up lets the project open, and is not repeated on the next navigation', async () => { + const c = check(undefined, () => NEVER); + const deciding = c.decide(segs('/project/github.com/o/r')); + fireTimeout(); + expect(await deciding).toEqual({ kind: 'open' }); + expect(timerCalls).toEqual([GITHUB_PROBE_TIMEOUT_MS]); + expect(await c.decide(segs('/project/github.com/o/r/chat'))).toEqual({ + kind: 'open', + }); + expect(summary).toHaveBeenCalledTimes(1); + }); + + it('an answer that comes in time wins: "not found" is still shown', async () => { + const c = check(undefined, () => + throwError(() => new GithubProjectNotFoundError('x', 'missing')), + ); + expect((await c.decide(segs('/project/github.com/o/r'))).kind).toBe( + 'problem', + ); + }); + + it('an answer that comes after the time is up changes nothing', async () => { + let answer: (value: unknown) => void = () => undefined; + const late = new Promise((resolve) => (answer = resolve)); + const c = check(undefined, () => from(late)); + const deciding = c.decide(segs('/project/github.com/o/r')); + fireTimeout(); + expect((await deciding).kind).toBe('open'); + answer({ id: 'x' }); + await Promise.resolve(); + }); + }); + it('is not about an address outside /project/github.com', async () => { expect( await check().decide(segs('/store/github.com/project/r@o@')), diff --git a/libs/datatug/main/src/lib/routes/github-project-address-check.ts b/libs/datatug/main/src/lib/routes/github-project-address-check.ts index 095fc519..1c698db1 100644 --- a/libs/datatug/main/src/lib/routes/github-project-address-check.ts +++ b/libs/datatug/main/src/lib/routes/github-project-address-check.ts @@ -1,7 +1,8 @@ -import { Injectable, inject, signal } from '@angular/core'; +import { Injectable, InjectionToken, inject, signal } from '@angular/core'; import { ToastController } from '@ionic/angular'; import { firstValueFrom } from 'rxjs'; import { + asciiLowerCase, GITHUB_DEFAULT_BRANCH_REF, GITHUB_STORE_ID, IGithubProjectParts, @@ -40,28 +41,58 @@ export type ShortGithubAddress = readonly locatorLength: number; }; +/** The encoded path of an address, as the one place that knows the shapes (`parseProjectUrl`) reads it. */ +const encodedPath = (segments: readonly string[]): string => + '/' + segments.map((s) => encodeURIComponent(s)).join('/'); + +/** How many of the (already normalised) segments are the project locator, given what follows it. */ +const locatorLengthOf = ( + segments: readonly string[], + parts: IProjectUrlParts, +): number => + segments.length - (parts.rest === '' ? 0 : parts.rest.split('/').length - 1); + /** * Reads the segments as the router has them (percent-decoded) with `parseProjectUrl`, the one place that knows the * shapes. Each segment is encoded again first, so a `%2F` or a `%` in a segment reads as the text it came from. + * + * The fixed segments (`project`, `github.com`, `tree`, and the first page, which is always a lower-case name) are + * read in any letter case, as the hand-off route's matcher and index.html's script read them, and are another + * spelling of the canonical address (`isCanonical` is false: the route redirects). Only ASCII letters are folded, + * so no look-alike letter ever passes for one of them. Owner and repository are lower-cased by `parseProjectUrl`. */ export function readShortGithubAddress( segments: readonly string[], ): ShortGithubAddress { - if (segments[0] !== 'project' || segments[1] !== GITHUB_STORE_ID) { + if ( + segments.length < 2 || + asciiLowerCase(segments[0]) !== 'project' || + asciiLowerCase(segments[1]) !== GITHUB_STORE_ID + ) { return { kind: 'not-short-github' }; } - const parsed = parseProjectUrl( - '/' + segments.map((s) => encodeURIComponent(s)).join('/'), - ); + const normalised = [...segments]; + normalised[0] = 'project'; + normalised[1] = GITHUB_STORE_ID; + if (normalised[4] !== undefined && asciiLowerCase(normalised[4]) === 'tree') { + normalised[4] = 'tree'; + } + let parsed = parseProjectUrl(encodedPath(normalised)); if (!parsed.ok) { return { kind: 'refused', reason: parsed.reason }; } - const pageSegments = - parsed.rest === '' ? 0 : parsed.rest.split('/').length - 1; + let locatorLength = locatorLengthOf(normalised, parsed); + const page = normalised[locatorLength]; + if (page !== undefined && asciiLowerCase(page) !== page) { + normalised[locatorLength] = asciiLowerCase(page); + parsed = parseProjectUrl(encodedPath(normalised)) as IProjectUrlParts; + locatorLength = locatorLengthOf(normalised, parsed); + } + const respelled = normalised.some((segment, i) => segment !== segments[i]); return { kind: 'project', - parts: parsed, - locatorLength: segments.length - pageSegments, + parts: respelled ? { ...parsed, isCanonical: false } : parsed, + locatorLength, }; } @@ -195,12 +226,31 @@ function defaultBranchPath( return replaced.canonicalPath; } +/** + * How long the check waits for GitHub to say whether a project file is there, before it lets the project open and + * the pages show their own loading or error state. A host that does not answer must not leave a blank page: the + * pages' own reads give up after 40 s, the old form of the address shows its state at once. + */ +export const GITHUB_PROBE_TIMEOUT_MS = 3000; + +/** Resolves after `ms` milliseconds. A token so that a test says when the time is up. */ +export const GITHUB_PROBE_TIMER = new InjectionToken< + (ms: number) => Promise +>('GITHUB_PROBE_TIMER', { + providedIn: 'root', + factory: () => (ms) => + new Promise((resolve) => setTimeout(resolve, ms)), +}); + /** The decisions, in the order of the design. Injectable so the router glue stays a one-liner and a test fakes the network. */ @Injectable({ providedIn: 'root' }) export class GithubAddressCheck { private readonly branches = inject(GithubDefaultBranchLookup); private readonly notices = inject(GithubAddressNotices); private readonly store = inject(DatatugStoreGithubService); + private readonly timer = inject(GITHUB_PROBE_TIMER); + /** Repositories and refs the visitor has been told about (once each for the life of the page). */ + private readonly told = new Set(); /** Projects whose first read ended in anything but "not found" (a project that was there is not asked about again). */ private readonly probed = new Set(); @@ -229,11 +279,15 @@ export class GithubAddressCheck { path: defaultBranchPath(parts, { ...github, ref: github.ref }), }; } else if (lookup.kind === 'refused') { - this.notices.defaultBranchUnknown( - github.owner, - github.repo, - github.ref, - ); + const key = `${github.owner}/${github.repo}@${github.ref}`; + if (!this.told.has(key)) { + this.told.add(key); + this.notices.defaultBranchUnknown( + github.owner, + github.repo, + github.ref, + ); + } } } if (!this.probed.has(parts.projectId)) { @@ -256,15 +310,18 @@ export class GithubAddressCheck { return { kind: 'open' }; } - /** `'missing'` or `'moved'` when GitHub says there is no project at this id; undefined otherwise, errors included. */ - private async probe( - projectId: string, - ): Promise<'missing' | 'moved' | undefined> { - try { - await firstValueFrom(this.store.getProjectSummary(projectId)); - return undefined; - } catch (err) { - return err instanceof GithubProjectNotFoundError ? err.reason : undefined; - } + /** + * `'missing'` or `'moved'` when GitHub says there is no project at this id; undefined otherwise, errors included, + * and when GitHub has not answered within `GITHUB_PROBE_TIMEOUT_MS` (the project opens, and its pages show + * their own loading or error state). + */ + private probe(projectId: string): Promise<'missing' | 'moved' | undefined> { + const answer = firstValueFrom(this.store.getProjectSummary(projectId)).then( + () => undefined, + (err: unknown) => + err instanceof GithubProjectNotFoundError ? err.reason : undefined, + ); + const timeUp = this.timer(GITHUB_PROBE_TIMEOUT_MS).then(() => undefined); + return Promise.race([answer, timeUp]); } } diff --git a/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts b/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts index ccb6478c..328d5e94 100644 --- a/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts +++ b/libs/datatug/main/src/lib/routes/github-project-routes.spec.ts @@ -334,6 +334,77 @@ describe('navigating to a short address', () => { }); }); + describe('letter case: the fixed segments match in any case and redirect to the lower-case address (review B1)', () => { + it.each([ + [ + '/Project/GitHub.com/datatug/chinook-demo/Chat', + '/project/github.com/datatug/chinook-demo/chat', + ], + [ + '/PROJECT/github.com/datatug/chinook-demo/chat?lang=ru', + '/project/github.com/datatug/chinook-demo/chat?lang=ru', + ], + [ + '/project/github.com/datatug/chinook-demo/Chat?msg=x#top', + '/project/github.com/datatug/chinook-demo/chat?msg=x#top', + ], + [ + '/project/github.com/datatug/chinook-demo/Tree/HEAD/-/chat', + '/project/github.com/datatug/chinook-demo/chat', + ], + [ + '/project/github.com/datatug/chinook-demo/Tree/HEAD/-/Chat?a=1&a=2', + '/project/github.com/datatug/chinook-demo/chat?a=1&a=2', + ], + [ + '/project/github.com/datatug/chinook-demo/Queries', + '/project/github.com/datatug/chinook-demo/queries', + ], + ])('%s -> %s, through one redirect, never to a route that matches nothing', async (typed, canonical) => { + const result = await visit(typed); + expect(result.ok).toBe(true); + expect(result.url).toBe(canonical); + expect(result.component).toBe(PageStub); + expect(result.projectId).toBe('chinook-demo@datatug@'); + }); + }); + + describe('a folder with parentheses stays that folder through a redirect (review minor 3)', () => { + it('a letter-case redirect', async () => { + const result = await visit( + '/project/github.com/Acme/demo/tree/HEAD/a%28b%29/-/queries', + ); + expect(result.url).toBe( + '/project/github.com/acme/demo/tree/HEAD/a%28b%29/-/queries', + ); + expect(result.component).toBe(PageStub); + expect(result.projectId).toBe('demo@acme@a(b)'); + }); + + it('a default-branch redirect, query and fragment kept', async () => { + const result = await visit( + '/project/github.com/o/r/tree/main/a%28b%29/-/queries?x=1#f', + ); + expect(result.url).toBe( + '/project/github.com/o/r/tree/HEAD/a%28b%29/-/queries?x=1#f', + ); + expect(result.component).toBe(PageStub); + expect(result.projectId).toBe('r@o@a(b)'); + }); + + it('an unmatched closing parenthesis, and a folder that is only parentheses', async () => { + for (const folder of ['a%29b', '%28%29']) { + const result = await visit( + `/project/github.com/O/r/tree/HEAD/${folder}/-/queries`, + ); + expect(result.projectId, folder).toBe( + `r@o@${decodeURIComponent(folder)}`, + ); + TestBed.resetTestingModule(); + } + }); + }); + it.each([ ['/project/github.com/o/r/', '/project/github.com/o/r'], ['/project/github.com/o/r/chat/', '/project/github.com/o/r/chat'], diff --git a/libs/datatug/main/src/lib/routes/github-project-routes.ts b/libs/datatug/main/src/lib/routes/github-project-routes.ts index afa5f060..9cf9beca 100644 --- a/libs/datatug/main/src/lib/routes/github-project-routes.ts +++ b/libs/datatug/main/src/lib/routes/github-project-routes.ts @@ -5,6 +5,8 @@ import { Routes, UrlMatcher, UrlSegment, + UrlSegmentGroup, + UrlTree, } from '@angular/router'; import { PRODUCT_PROFILE } from '@datatug/product-profiles'; import { @@ -30,6 +32,11 @@ import { // and either redirects to the canonical spelling, or says there is a problem (an address that cannot be a // project, or no project file there), or does not match so that route 2 takes the address; // 2. the project. +// +// The fixed segments (`project`, `github.com`, `tree`, the first page) are read in any letter case and redirect to +// the lower-case address (`readShortGithubAddress`): the hand-off route and index.html read them so, and an address +// that either of them lets through must never reach the router's "no route matches" failure. A redirect is built +// from segments, not parsed from a string (`urlTreeOfPath`). const paths = (segments: readonly UrlSegment[]): string[] => segments.map((segment) => segment.path); @@ -55,6 +62,26 @@ export const githubAddressMatcher: UrlMatcher = (segments) => ? null : { consumed: segments }; +/** + * The URL tree of a canonical path, built from its segments and not parsed from the string: the router's parser + * reads `(` as the start of an outlet group, so a folder called `a(b)` would open another project. Each segment is + * decoded here and the serializer encodes it again, parentheses included. + */ +export function urlTreeOfPath( + path: string, + queryParams: Record, + fragment: string | null, +): UrlTree { + const segments = path + .split('/') + .slice(1) + .map((segment) => new UrlSegment(decodeURIComponent(segment), {})); + const primary = new UrlSegmentGroup(segments, {}); + const root = new UrlSegmentGroup([], { primary }); + primary.parent = root; + return new UrlTree(root, { ...queryParams }, fragment); +} + /** The short route belongs to the DataTug product profile only. */ export const datatugProfileForShortRoute = (): boolean => inject(PRODUCT_PROFILE).id === 'datatug'; @@ -74,10 +101,11 @@ export const githubAddressCanMatch: CanMatchFn = async (_route, segments) => { const decision = await check.decide(paths(segments)); switch (decision.kind) { case 'redirect': { - const target = router.parseUrl(decision.path); - target.queryParams = { ...(typed?.queryParams ?? {}) }; - target.fragment = typed?.fragment ?? null; - return target; + return urlTreeOfPath( + decision.path, + typed?.queryParams ?? {}, + typed?.fragment ?? null, + ); } case 'problem': state.problem.set(decision.problem); From 699329906287bf576a8a7af33d15f96428ba7136 Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Fri, 2 Oct 2026 23:28:08 +0100 Subject: [PATCH 6/8] fix(app): a bare chat visit forgets the question; msg and q leave every project address (G-A1b review r1) S1: when the page decision answers "no question" (a fresh visit, or a query with no question), the copy kept in sessionStorage and the window stash are removed, as captureDemoHandoff did on main for a bare visit. A reload now keeps showing the project chat instead of bringing the holding page back. S2: index.html's inline script takes msg and q out of the address for every path under /project/github.com (any letter case), before analytics; the rest of the query and the fragment stay, and nothing is stashed or stored for them. Hand-off addresses are handled exactly as before. e2e: the case variants of the hand-off chat address with and without a question (B1), the question then bare visit then reload, in the same tab and in a tab opened from it, for the demo project and another repository (S1), msg stripped and not reported on tree, queries and repo-root addresses (S2), and "a project page other than the chat" now asserts the page's own title instead of an ion-header the problem page also has. Co-Authored-By: Claude Sonnet 5.5 --- apps/datatug-app/e2e/demo-handoff.spec.ts | 244 +++++++++++++++++- .../src/app/demo-handoff-asked.spec.ts | 85 +++++- .../datatug-app/src/app/demo-handoff-asked.ts | 23 +- .../src/app/demo-handoff-capture.spec.ts | 96 ++++++- apps/datatug-app/src/index.html | 21 +- 5 files changed, 454 insertions(+), 15 deletions(-) diff --git a/apps/datatug-app/e2e/demo-handoff.spec.ts b/apps/datatug-app/e2e/demo-handoff.spec.ts index cf3ddc07..99470903 100644 --- a/apps/datatug-app/e2e/demo-handoff.spec.ts +++ b/apps/datatug-app/e2e/demo-handoff.spec.ts @@ -588,6 +588,9 @@ test.describe('the hand-off holding page', () => { }); } + // Review minor 11: what only a project page has. The problem page ("No DataTug project here", "This address is not + // supported") also has a header, so a header proves nothing. The stand-in for GitHub answers with `{}`, which is a + // project file the pages read; the page that opens is the project's own, with the title of its page. test('a project page other than the chat opens the project, question or not', async ({ page, context, @@ -595,20 +598,88 @@ test.describe('the hand-off holding page', () => { }) => { const origin = new URL(baseURL ?? '').origin; await stubExternal(context, (o) => o === origin); - for (const url of [ - '/project/github.com/datatug/chinook-demo', - '/project/github.com/datatug/chinook-demo/queries?msg=Hello', - '/project/github.com/datatug/chinook-demo/tree/HEAD/dir/-/chat', + for (const [url, title] of [ + ['/project/github.com/datatug/chinook-demo/queries?msg=Hello', 'Queries'], + ['/project/github.com/datatug/chinook-demo/tree/HEAD/dir/-/chat', 'Chat'], + ['/project/github.com/datatug/chinook-demo', ''], ]) { await page.goto(url); - await expect(page.locator('ion-header').first()).toBeVisible({ - timeout: 20_000, - }); + if (title) { + await expect( + page.locator('ion-title', { hasText: title }), + url, + ).toBeVisible({ timeout: 20_000 }); + } else { + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + } await page.waitForTimeout(500); await expect(page.locator('#demo-holding-message'), url).toHaveCount(0); + await expect( + page.getByRole('heading', { + name: /No DataTug project here|This address is not supported/, + }), + url, + ).toHaveCount(0); } }); + // Review B1: the fixed segments of a hand-off address match in any letter case, as they do on main. With a + // question: the holding page, as on main. Without: the project's chat at its canonical lower-case address, through + // one redirect, never the crash page (NG04002). + for (const typed of [ + '/Project/GitHub.com/datatug/chinook-demo/chat', + '/PROJECT/github.com/datatug/chinook-demo/chat?lang=ru', + '/project/github.com/datatug/chinook-demo/Chat', + '/project/github.com/datatug/chinook-demo/Tree/HEAD/-/chat', + ]) { + const [path, ownQuery] = typed.split('?'); + const join = (more: string): string => + path + '?' + [ownQuery, more].filter(Boolean).join('&'); + + test(`${typed} without a question is the project chat at ${TRUSTED}, and never the crash page`, async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + const external = await stubExternal(context, (o) => o === origin); + const errors = consoleErrors(page); + await page.goto(typed); + await expect(page.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + expect(new URL(page.url()).pathname + new URL(page.url()).search).toBe( + TRUSTED, + ); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + await page.waitForTimeout(1500); + await expect(page.locator(SENTRY_DIALOG)).toHaveCount(0); + expect(dialogRequested(external)).toBe(false); + expect(errors.filter((e) => e.includes('NG04002'))).toEqual([]); + }); + + test(`${typed} with a question shows the holding page with the question, as on main`, async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + const external = await stubExternal(context, (o) => o === origin); + const errors = consoleErrors(page); + await page.goto(join(`msg=${encodeURIComponent(EN_QUESTION)}`)); + await expect(page.locator('blockquote')).toHaveText(EN_QUESTION, { + timeout: 20_000, + }); + expect(new URL(page.url()).search).toBe(''); + await page.waitForTimeout(1500); + await expect(page.locator(SENTRY_DIALOG)).toHaveCount(0); + expect(dialogRequested(external)).toBe(false); + expect(errors).toEqual([]); + }); + } + test('a reload of a chat address that arrived with a question shows the holding page again, for the demo project and for any other repository', async ({ page, context, @@ -655,6 +726,75 @@ test.describe('the hand-off holding page', () => { await expect(page.locator('#demo-holding-message')).toHaveCount(0); }); + // Review S1: a bare visit forgets the question kept for the tab, so a reload shows the project chat. + for (const [what, path] of [ + ['the demo project', TRUSTED], + ['another repository', '/project/github.com/someone/else/chat'], + ]) { + test(`a question, then the bare chat address of ${what}, then a reload: still the project chat`, async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto(`${path}?msg=${encodeURIComponent(EN_QUESTION)}`); + await expect(page.locator('#demo-holding-message')).toBeVisible({ + timeout: 20_000, + }); + await page.goto(path); + await expect(page.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + expect( + await page.evaluate(() => sessionStorage.getItem('datatug.demo.handoff.v1')), + ).toBeNull(); + expect( + await page.evaluate( + () => (window as unknown as Record)['__datatugHandoffSearch'], + ), + ).toBeUndefined(); + + await page.reload(); + await expect(page.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + expect(await page.content()).not.toContain(MARKER); + }); + + test(`the same in a second tab opened from the first (it starts with a copy of the first tab's storage): ${what}`, async ({ + page, + context, + baseURL, + }) => { + const origin = new URL(baseURL ?? '').origin; + await stubExternal(context, (o) => o === origin); + await page.goto(`${path}?msg=${encodeURIComponent(EN_QUESTION)}`); + await expect(page.locator('#demo-holding-message')).toBeVisible({ + timeout: 20_000, + }); + const [second] = await Promise.all([ + context.waitForEvent('page'), + page.evaluate((url) => window.open(url, '_blank'), origin + path), + ]); + await expect(second.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await second.reload(); + await expect(second.locator('ion-title', { hasText: 'Chat' })).toBeVisible({ + timeout: 20_000, + }); + await expect(second.locator('#demo-holding-message')).toHaveCount(0); + // and the first tab, reloaded, still has its own question + await page.reload(); + await expect(page.locator('#demo-holding-message')).toBeVisible({ + timeout: 20_000, + }); + }); + } + test('a non-canonical spelling of a project address is replaced by the canonical one, query kept', async ({ page, context, @@ -788,6 +928,96 @@ test.describe('the question reaches no analytics or error report', () => { }); }); +test.describe('a question on a project address that is not a hand-off address is dropped, and reaches no analytics (review S2)', () => { + const HOST = 'https://handoff.datatug.test'; + + for (const path of [ + '/project/github.com/datatug/chinook-demo/tree/HEAD/dir/-/chat', + '/project/github.com/datatug/chinook-demo/queries', + '/project/github.com/datatug/chinook-demo', + '/Project/GitHub.com/datatug/chinook-demo/tree/HEAD/dir/-/Chat', + ]) { + test(`${path}?msg=…&x=1: the address bar loses msg and keeps x=1, and nothing outgoing carries it`, async ({ + page, + context, + baseURL, + }) => { + const external = await stubExternal( + context, + (o) => o === HOST || o === baseURL, + ); + await context.route(`${HOST}/**`, async (route) => { + const url = new URL(route.request().url()); + const response = await route.fetch({ + url: (baseURL ?? '') + url.pathname + url.search, + }); + await route.fulfill({ response }); + }); + await context.route( + 'https://www.googletagmanager.com/**', + async (route) => { + external.push({ + url: route.request().url(), + method: 'GET', + body: '', + }); + await route.fulfill({ + status: 200, + contentType: 'text/javascript', + body: `(function(){var dl=window.dataLayer=window.dataLayer||[]; + function report(args){new Image().src='https://www.google-analytics.com/g/collect?dl='+encodeURIComponent(location.href)+'&cmd='+encodeURIComponent(JSON.stringify(Array.prototype.slice.call(args)));} + dl.slice().forEach(report);var push=dl.push;dl.push=function(a){report(a);return push.apply(dl,arguments);};})();`, + }); + }, + ); + await page.goto( + `${HOST}${path}?msg=${encodeURIComponent(MARKER)}&q=${encodeURIComponent(MARKER)}&x=1#frag`, + ); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + await page.evaluate(() => + setTimeout(() => { + throw new Error('S2 probe: an uncaught error on a project page'); + }), + ); + await page.waitForTimeout(3000); + + const url = new URL(page.url()); + expect(url.searchParams.has('msg')).toBe(false); + expect(url.searchParams.has('q')).toBe(false); + expect(url.searchParams.get('x')).toBe('1'); + // The queries page rewrites its own address (adds its default query, drops the fragment), on the old form + // of the address too; every other page leaves the fragment where the script left it. + if (!path.endsWith('/queries')) expect(url.hash).toBe('#frag'); + expect(page.url()).not.toContain(MARKER); + // Not vacuous: the stand-in for Google Analytics saw page commands, Sentry received the probe. + const hosts = new Set( + external.map((request) => new URL(request.url).hostname), + ); + expect([...hosts].some((h) => h.includes('google-analytics.com'))).toBe( + true, + ); + expect([...hosts].some((h) => h.includes('sentry.io'))).toBe(true); + const leaked = external.filter((request) => + [request.url, request.body].some( + (text) => + text.includes(MARKER) || text.includes(encodeURIComponent(MARKER)), + ), + ); + expect(leaked.map((r) => r.url.slice(0, 120))).toEqual([]); + // nothing keeps it in the tab either + expect( + await page.evaluate( + () => + JSON.stringify(Object.entries(sessionStorage)) + + JSON.stringify(Object.entries(localStorage)), + ), + ).not.toContain(MARKER); + }); + } +}); + test.describe('the matrix-parameter address reaches no analytics either', () => { const HOST = 'https://handoff.datatug.test'; test('/demo;x=1?q=…: Google Analytics, Sentry and PostHog never receive the question', async ({ diff --git a/apps/datatug-app/src/app/demo-handoff-asked.spec.ts b/apps/datatug-app/src/app/demo-handoff-asked.spec.ts index cf6d7cb8..53e18fa4 100644 --- a/apps/datatug-app/src/app/demo-handoff-asked.spec.ts +++ b/apps/datatug-app/src/app/demo-handoff-asked.spec.ts @@ -16,6 +16,9 @@ import { routeSegments } from './demo-handoff-path'; const CHAT = '/project/github.com/datatug/chinook-demo/chat'; +/** Keys removed from sessionStorage through the env made by `env()`. */ +const removed: string[] = []; + function env( options: { stash?: string; @@ -34,6 +37,7 @@ function env( return { getItem: (key: string) => key === DEMO_HANDOFF_KEY ? (options.stored ?? null) : null, + removeItem: (key: string) => void removed.push(key), } as unknown as Storage; }, navigationType: () => options.navigation, @@ -44,7 +48,86 @@ const shows = (path: string, e?: AskedEnv): boolean => showsHoldingPage(routeSegments(path), e); describe('which hand-off addresses show the holding page', () => { - beforeEach(() => resetHandoffAskedForTests()); + beforeEach(() => { + resetHandoffAskedForTests(); + removed.length = 0; + }); + + // Review S1 and minor 6 of G-A1b: on main a fresh bare visit removed the question kept in the tab, so a reload + // shows the project chat; the answer "no question" must do the same, and the stash must not outlive it. + describe('an answer of "no question" forgets the question kept for this tab', () => { + it('a fresh visit with no query removes the stored copy, so a reload no longer shows the page', () => { + const stored = CHAT + '?msg=Hello'; + expect(shows(CHAT, env({ stored, navigation: 'navigate' }))).toBe(false); + expect(removed).toEqual([DEMO_HANDOFF_KEY]); + }); + + it('a visit with a query that is no question removes the stored copy, and the stash', () => { + const e = env({ + stash: '?lang=ru', + stored: CHAT + '?msg=Hello', + navigation: 'navigate', + }); + expect(shows(CHAT, e)).toBe(false); + expect(removed).toEqual([DEMO_HANDOFF_KEY]); + expect(DEMO_HANDOFF_STASH in e.stash).toBe(false); + }); + + it('the same for an address that may not echo its question (an untrusted repository)', () => { + const path = '/project/github.com/someone/else/chat'; + expect( + shows( + path, + env({ + stored: path + '?lang=en&asked=1', + navigation: 'navigate', + }), + ), + ).toBe(false); + expect(removed).toEqual([DEMO_HANDOFF_KEY]); + }); + + it('a question that was asked is kept, and its stash is left for the holding page', () => { + const e = env({ stash: '?msg=Hello', navigation: 'navigate' }); + expect(shows(CHAT, e)).toBe(true); + expect(removed).toEqual([]); + expect(e.stash[DEMO_HANDOFF_STASH]).toBe('?msg=Hello'); + }); + + it('a reload and Back keep it: that is how the page comes back', () => { + for (const navigation of ['reload', 'back_forward']) { + resetHandoffAskedForTests(); + expect( + shows(CHAT, env({ stored: CHAT + '?msg=Hello', navigation })), + navigation, + ).toBe(true); + } + expect(removed).toEqual([]); + }); + + it('/demo keeps its stash: the page reads the language from it', () => { + const e = env({ stash: '?lang=ru', navigation: 'navigate' }); + expect(shows('/demo', e)).toBe(true); + expect(e.stash[DEMO_HANDOFF_STASH]).toBe('?lang=ru'); + expect(removed).toEqual([]); + }); + + it('blocked storage does not throw, and the answer is still no', () => { + expect(shows(CHAT, env({ blocked: true, navigation: 'navigate' }))).toBe( + false, + ); + expect(shows(CHAT, env({ blocked: true, stash: '?lang=ru' }))).toBe( + false, + ); + }); + + it('asking again about the same address, from the same page load, does not remove anything more', () => { + const e = env({ navigation: 'navigate' }); + shows(CHAT, e); + shows(CHAT, e); + expect(removed).toEqual([DEMO_HANDOFF_KEY]); + }); + }); describe('/demo: always, whatever the query or storage says', () => { it.each(['/demo', '/Demo', '/demo/', '/demo;x=1'])('%s', (path) => { diff --git a/apps/datatug-app/src/app/demo-handoff-asked.ts b/apps/datatug-app/src/app/demo-handoff-asked.ts index 4433c146..b490ff5d 100644 --- a/apps/datatug-app/src/app/demo-handoff-asked.ts +++ b/apps/datatug-app/src/app/demo-handoff-asked.ts @@ -78,6 +78,15 @@ function defaultEnv(): AskedEnv { }; } +/** Removes the copy of the query kept for this tab; blocked storage is not an error. */ +function forgetQuestion(env: AskedEnv): void { + try { + env.storage().removeItem(DEMO_HANDOFF_KEY); + } catch { + // Storage is blocked: nothing was kept. + } +} + /** What this page load has already been told about an address: it stays the answer for as long as the page lives. */ const settled = new Map(); @@ -92,6 +101,10 @@ const keyOf = (segments: readonly string[]): string => * visit in this tab left in storage; * 4. a reload or back/forward: the copy of the query kept in storage for this very address. * Blocked storage means no for 3 and 4: the question lives only in the page. + * + * An answer of "no" that comes from 1 or 3 also forgets the question kept for this tab (the copy in storage, and + * the stash, which only the holding page reads), as `captureDemoHandoff` did on main for a bare visit: otherwise a + * reload would bring the holding page back, with a question the visitor did not ask this time. */ export function handoffAsked( segments: readonly string[], @@ -102,11 +115,19 @@ export function handoffAsked( if (typeof stashed === 'string') { const asked = searchAsksQuestion(stashed); settled.set(key, asked); + if (!asked) { + delete env.stash[DEMO_HANDOFF_STASH]; + forgetQuestion(env); + } return asked; } const known = settled.get(key); if (known !== undefined) return known; - if (env.navigationType() === 'navigate') return false; + if (env.navigationType() === 'navigate') { + settled.set(key, false); + forgetQuestion(env); + return false; + } try { const raw = env.storage().getItem(DEMO_HANDOFF_KEY) ?? ''; const at = raw.indexOf('?'); diff --git a/apps/datatug-app/src/app/demo-handoff-capture.spec.ts b/apps/datatug-app/src/app/demo-handoff-capture.spec.ts index 5743c2dd..9e52c5c2 100644 --- a/apps/datatug-app/src/app/demo-handoff-capture.spec.ts +++ b/apps/datatug-app/src/app/demo-handoff-capture.spec.ts @@ -754,15 +754,109 @@ describe('demo hand-off capture', () => { '/project/github.com/o%2Fx/r/chat', '/project/github.com/%E0%A4%A/r/chat', ]; + // Every other address under /project/github.com loses `msg` and `q` only (review S2): nothing is stashed + // or kept for it, and the rest of its query stays. + const underProject = (path: string) => + /^\/project\/github\.com(?:[/;(]|$)/i.test(path); for (const path of paths) { const result = run(path, '?q=x&lang=ru', '#h'); expect(result.stash !== undefined, path).toBe(isHandoffPath(path)); expect(result.replaced, path).toEqual( - isHandoffPath(path) ? [path + '#h'] : [], + isHandoffPath(path) + ? [path + '#h'] + : underProject(path) + ? [path + '?lang=ru#h'] + : [], ); + if (!isHandoffPath(path)) expect(result.kept, path).toEqual(new Map()); } }); + describe('every other address under /project/github.com loses msg and q, and only those (review S2)', () => { + it.each([ + // [path, search, hash, what the address becomes] + [ + '/project/github.com/o/r?msg=M&x=1', + '/project/github.com/o/r?x=1', + ], + [ + '/project/github.com/o/r/tree/HEAD/dir/-/chat?msg=M', + '/project/github.com/o/r/tree/HEAD/dir/-/chat', + ], + [ + '/project/github.com/o/r/queries?msg=M', + '/project/github.com/o/r/queries', + ], + ['/project/github.com/o/r/queries?q=M&b=2', '/project/github.com/o/r/queries?b=2'], + ['/Project/GitHub.com/o/r/Queries?a=1&MSG=keep&msg=M&q=N', '/Project/GitHub.com/o/r/Queries?a=1&MSG=keep'], + ['/project/github.com/o/r/chat/extra?msg=M&lang=ru', '/project/github.com/o/r/chat/extra?lang=ru'], + ['/project/github.com/o/r/queries?m%73g=M&x=%20y', '/project/github.com/o/r/queries?x=%20y'], + ['/project/github.com/o/r/queries?%71=M&x=1', '/project/github.com/o/r/queries?x=1'], + ['/project/github.com/o/r/queries?msg&x=1', '/project/github.com/o/r/queries?x=1'], + ['/project/github.com/o/r/queries?msg=A&msg=B&q=C', '/project/github.com/o/r/queries'], + ['/project/github.com/o/r;m=1/queries?msg=M', '/project/github.com/o/r;m=1/queries'], + ['/project/github.com/o/r/queries?x=1&&y=2&msg=M', '/project/github.com/o/r/queries?x=1&&y=2'], + ])('%s becomes %s', (path, becomes) => { + const [bare, query] = path.split('?'); + const result = run(bare, '?' + query, '#h'); + expect(result.replaced).toEqual([becomes + '#h']); + expect(result.stash).toBeUndefined(); + expect(result.kept).toEqual(new Map()); + // and what is left has neither, as the parser of the app reads a query + const left = new URLSearchParams( + result.replaced[0].split('#')[0].split('?')[1] ?? '', + ); + expect(left.has('msg')).toBe(false); + expect(left.has('q')).toBe(false); + }); + + it.each([ + '/project/github.com/o/r?x=1&MSG=1&Q=2&msgx=3&xq=4', + '/project/github.com/o/r/queries?lang=ru', + ])('%s: nothing to take out, so the address is not rewritten', (path) => { + const [bare, query] = path.split('?'); + expect(run(bare, '?' + query, '#h').replaced).toEqual([]); + }); + + it.each([ + '/no-such-route?msg=M', + '/queries?q=M', + '/store/github.com/project/p@o@/chat?msg=M', + '/project/gitlab.com/o/r?msg=M', + '/projects/github.com/o/r?msg=M', + '/demo/other?q=M', + '/', + ])('%s is left exactly as it is', (path) => { + const [bare, query] = path.split('?'); + const result = run(bare, '?' + query, '#h'); + expect(result.replaced).toEqual([]); + expect(result.stash).toBeUndefined(); + }); + + it('does nothing for an address with no query at all', () => { + expect(run('/project/github.com/o/r/queries', '').replaced).toEqual([]); + }); + + it('does not throw when the browser refuses to rewrite the address', () => { + expect(() => + run('/project/github.com/o/r', '?msg=M', '', false, true), + ).not.toThrow(); + }); + + it('a hand-off address is handled as before: the whole query goes, the question is stashed and kept', () => { + const result = run( + '/project/github.com/o/r/chat', + '?msg=M&lang=ru&x=1', + '#h', + ); + expect(result.replaced).toEqual(['/project/github.com/o/r/chat#h']); + expect(result.stash).toBe('?msg=M&lang=ru&x=1'); + expect(result.kept).toEqual( + new Map([[DEMO_HANDOFF_KEY, '/project/github.com/o/r/chat?msg=M&lang=ru&x=1']]), + ); + }); + }); + it('stashes the raw query string for the TypeScript side to parse', () => { expect(run('/demo', '?scenario=a&q=hello%20world').stash).toBe( '?scenario=a&q=hello%20world', diff --git a/apps/datatug-app/src/index.html b/apps/datatug-app/src/index.html index e38d77c0..2ad012e4 100644 --- a/apps/datatug-app/src/index.html +++ b/apps/datatug-app/src/index.html @@ -25,24 +25,35 @@ storage key as in src/app/demo-handoff-path.ts and demo-handoff-capture.ts, which parses what is kept here (their specs run this script to keep them in step): matrix parameters, a trailing slash and letter case do not change what a hand-off address is, exactly as the router matches them; a path with an outlet group `(` is not - one. Any other path is untouched. --> + one. Any other path is untouched, except that every other address under /project/github.com loses `msg` and + `q` (the question is dropped, never kept: nothing reads it there yet), the rest of its query and its fragment + staying. --> From 2ba0f42c61377da1da83c77c52b5ff8796899580 Mon Sep 17 00:00:00 2001 From: OpenVaultDB Date: Sat, 3 Oct 2026 00:30:33 +0100 Subject: [PATCH 7/8] fix(app): a question never survives in a short project address; outlet groups and matrix parameters (G-A1b review r2) R2-B1: the inline script of index.html read the path its own way (`//project/...`, `/(project/...)`), the router another, so the question stayed in the address bar and the history. Three layers now: - the short route (githubAddressCanMatch) redirects every navigation it matches, in-app ones included, to the same address without `msg` and `q`, before GitHub is asked anything; - the app's hand-off decision treats a router-matched hand-off address with a question in the navigation's query and no stash as asked, and stashes that query so the holding page captures and strips it; - the script and demo-handoff-path.ts read the path as the router does (leading and repeated empty segments, a root group is its content), and replaceState gets the origin for a `//` path (it was a cross-origin address, refused). Minor 1: a probe that timed out is not cached as "the project exists"; its late answer is kept and told on the next navigation. Minor 3: an unnamed outlet group (`a(b)`) shows the unsupported-address page. Minor 4: matrix parameters on a short address are dropped by the canonical redirect. Co-Authored-By: Claude Sonnet 5.5 --- apps/datatug-app/e2e/demo-handoff.spec.ts | 402 +++++++++++++++--- .../src/app/datatug-app-routes.spec.ts | 75 ++++ .../datatug-app/src/app/datatug-app-routes.ts | 18 +- .../src/app/demo-handoff-asked.spec.ts | 70 ++- .../datatug-app/src/app/demo-handoff-asked.ts | 37 +- .../src/app/demo-handoff-capture.spec.ts | 167 +++++++- .../src/app/demo-handoff-capture.ts | 32 +- .../src/app/demo-handoff-path.spec.ts | 76 ++++ apps/datatug-app/src/app/demo-handoff-path.ts | 51 ++- apps/datatug-app/src/index.html | 24 +- .../github-project-address-check.spec.ts | 90 +++- .../routes/github-project-address-check.ts | 58 ++- .../lib/routes/github-project-routes.spec.ts | 258 ++++++++++- .../src/lib/routes/github-project-routes.ts | 85 +++- .../src/lib/outlet-group.spec.ts | 85 ++++ .../project-address/src/lib/outlet-group.ts | 50 +++ .../datatug/project-address/src/public_api.ts | 1 + 17 files changed, 1459 insertions(+), 120 deletions(-) create mode 100644 libs/datatug/project-address/src/lib/outlet-group.spec.ts create mode 100644 libs/datatug/project-address/src/lib/outlet-group.ts diff --git a/apps/datatug-app/e2e/demo-handoff.spec.ts b/apps/datatug-app/e2e/demo-handoff.spec.ts index 99470903..3d8b3d44 100644 --- a/apps/datatug-app/e2e/demo-handoff.spec.ts +++ b/apps/datatug-app/e2e/demo-handoff.spec.ts @@ -119,6 +119,56 @@ async function shot(page: Page, name: string): Promise { await page.screenshot({ path: join(dir, `${name}.png`) }); } +const HANDOFF_HOST = 'https://handoff.datatug.test'; + +/** + * Serves the production build as if it lived on an https host that is not localhost (so the app turns its + * analytics and error reporting on, as in production), and stands in for Google Analytics with a script that + * reports what gtag.js reports: the page location and every command. + */ +async function onProductionLikeHost( + context: BrowserContext, + baseURL: string, +): Promise { + const external = await stubExternal( + context, + (o) => o === HANDOFF_HOST || o === baseURL, + ); + await context.route(`${HANDOFF_HOST}/**`, async (route) => { + const url = new URL(route.request().url()); + const response = await route.fetch({ + url: baseURL + url.pathname + url.search, + }); + await route.fulfill({ response }); + }); + await context.route( + 'https://www.googletagmanager.com/**', + async (route) => { + external.push({ url: route.request().url(), method: 'GET', body: '' }); + await route.fulfill({ + status: 200, + contentType: 'text/javascript', + body: `(function(){var dl=window.dataLayer=window.dataLayer||[]; + function report(args){new Image().src='https://www.google-analytics.com/g/collect?dl='+encodeURIComponent(location.href)+'&cmd='+encodeURIComponent(JSON.stringify(Array.prototype.slice.call(args)));} + dl.slice().forEach(report);var push=dl.push;dl.push=function(a){report(a);return push.apply(dl,arguments);};})();`, + }); + }, + ); + return external; +} + +const leaks = (external: External[]): string[] => + external + .filter((request) => + [request.url, request.body].some( + (text) => + text.includes(MARKER) || + text.includes(encodeURIComponent(MARKER)) || + text.includes(SCENARIO), + ), + ) + .map((request) => request.method + ' ' + request.url.slice(0, 120)); + const VIEWPORTS = [ { name: '390', width: 390, height: 844 }, { name: '1440', width: 1440, height: 900 }, @@ -830,54 +880,6 @@ test.describe('the hand-off holding page', () => { test.describe('the question reaches no analytics or error report', () => { const HOST = 'https://handoff.datatug.test'; - /** - * Serves the production build as if it lived on an https host that is not localhost (so the app turns its - * analytics and error reporting on, as in production), and stands in for Google Analytics with a script that - * reports what gtag.js reports: the page location and every command. - */ - async function onProductionLikeHost( - context: BrowserContext, - baseURL: string, - ): Promise { - const external = await stubExternal( - context, - (o) => o === HOST || o === baseURL, - ); - await context.route(`${HOST}/**`, async (route) => { - const url = new URL(route.request().url()); - const response = await route.fetch({ - url: baseURL + url.pathname + url.search, - }); - await route.fulfill({ response }); - }); - await context.route( - 'https://www.googletagmanager.com/**', - async (route) => { - external.push({ url: route.request().url(), method: 'GET', body: '' }); - await route.fulfill({ - status: 200, - contentType: 'text/javascript', - body: `(function(){var dl=window.dataLayer=window.dataLayer||[]; - function report(args){new Image().src='https://www.google-analytics.com/g/collect?dl='+encodeURIComponent(location.href)+'&cmd='+encodeURIComponent(JSON.stringify(Array.prototype.slice.call(args)));} - dl.slice().forEach(report);var push=dl.push;dl.push=function(a){report(a);return push.apply(dl,arguments);};})();`, - }); - }, - ); - return external; - } - - const leaks = (external: External[]): string[] => - external - .filter((request) => - [request.url, request.body].some( - (text) => - text.includes(MARKER) || - text.includes(encodeURIComponent(MARKER)) || - text.includes(SCENARIO), - ), - ) - .map((request) => request.method + ' ' + request.url.slice(0, 120)); - test('control: the same stand-in for Google Analytics does see a query that is NOT stripped (so it is not blind)', async ({ page, context, @@ -1018,6 +1020,310 @@ test.describe('a question on a project address that is not a hand-off address is } }); +test.describe('an address that the router reads another way than the inline script leaves no question behind (review r2, B1)', () => { + const HOST = HANDOFF_HOST; + const hasMarker = (text: string): boolean => { + let decoded = text; + for (let i = 0; i < 3; i++) { + try { + decoded = decodeURIComponent(decoded); + } catch { + break; + } + } + return text.includes(MARKER) || decoded.includes(MARKER); + }; + + /** Everything the page did with the address after it was loaded, and where the question could have gone. */ + async function openAndWatch( + page: Page, + context: BrowserContext, + baseURL: string, + path: string, + ready: () => Promise, + ) { + const external = await onProductionLikeHost(context, baseURL); + await page.addInitScript(() => { + const log: string[] = ((window as unknown as { __urls: string[] }).__urls = + []); + for (const name of ['pushState', 'replaceState'] as const) { + const original = history[name].bind(history); + history[name] = (state: unknown, title: string, url?: string | URL | null) => { + log.push(`${name} ${String(url)}`); + original(state, title, url); + log.push(`${name} -> ${location.href}`); + }; + } + addEventListener('popstate', () => log.push(`popstate ${location.href}`)); + }); + const requests: string[] = []; + page.on('request', (request) => { + if (!request.isNavigationRequest()) requests.push(request.url()); + }); + await page.goto(`${HOST}${path}`); + await ready(); + // Make Sentry report something, so that its breadcrumbs and request URL are on the wire too. + await page.evaluate(() => + setTimeout(() => { + throw new Error('R2 probe: an uncaught error'); + }), + ); + await page.waitForTimeout(2500); + return { external, requests }; + } + + /** + * The question is nowhere: not in the address, the history, the title, the storage, a request. The one place a + * hand-off address that may echo its question keeps it, as on main, is this tab's sessionStorage, so that a + * reload shows the holding page again (`keptForReload`). + */ + async function expectNoQuestion( + page: Page, + watched: { external: External[]; requests: string[] }, + keptForReload = false, + ) { + expect(hasMarker(page.url()), `address bar ${page.url()}`).toBe(false); + const state = await page.evaluate(() => ({ + title: document.title, + session: JSON.stringify(Object.entries(sessionStorage)), + local: JSON.stringify(Object.entries(localStorage)), + recorded: (window as unknown as { __urls: string[] }).__urls, + // every entry of the session history (the Navigation API, in Chromium) + entries: ( + window as unknown as { + navigation: { entries: () => { url: string }[] }; + } + ).navigation + .entries() + .map((entry) => entry.url), + })); + expect(state.title.includes(MARKER)).toBe(false); + expect(hasMarker(state.session), 'sessionStorage').toBe(keptForReload); + expect(hasMarker(state.local), 'localStorage').toBe(false); + expect(state.recorded.filter(hasMarker), 'history calls').toEqual([]); + expect(state.entries.filter(hasMarker), 'history entries').toEqual([]); + expect(watched.requests.filter(hasMarker), 'own requests').toEqual([]); + expect(leaks(watched.external), 'third-party requests').toEqual([]); + // Not vacuous: Google Analytics reported the page, and Sentry received the probe. + const hosts = new Set( + watched.external.map((request) => new URL(request.url).hostname), + ); + expect([...hosts].some((h) => h.includes('google-analytics.com'))).toBe(true); + expect([...hosts].some((h) => h.includes('sentry.io'))).toBe(true); + } + + const query = `?msg=${encodeURIComponent(MARKER)}&lang=ru`; + + // Hand-off addresses spelled so that the script did not recognise them: the holding page, as on main. + for (const [path, trusted] of [ + ['//project/github.com/datatug/chinook-demo/chat', true], + ['//project/github.com/acme/demo/chat', false], + ['/(project/github.com/datatug/chinook-demo/chat)', true], + ['/(project/github.com/acme/demo/chat)', false], + ['///project/github.com/datatug/chinook-demo/chat', true], + ['//project/github.com/datatug/chinook-demo/tree/HEAD/-/chat', true], + ['/(project/github.com/acme/demo/tree/HEAD/-/chat)', false], + ['/project/github.com/datatug/chinook-demo/Tree/HEAD/-/chat', true], + ['/project/github.com/acme/demo/Tree/HEAD/-/chat', false], + ] as const) { + test(`${path}${'?msg=…'}: the holding page, ${trusted ? 'with the question' : 'with neutral wording'}, and the question nowhere else`, async ({ + page, + context, + baseURL, + }) => { + const watched = await openAndWatch( + page, + context, + baseURL ?? '', + path + query, + async () => { + await expect(page.locator('#demo-holding-message')).toBeVisible({ + timeout: 20_000, + }); + }, + ); + if (trusted) { + await expect(page.locator('blockquote')).toHaveText(MARKER); + } else { + await expect(page.locator('blockquote')).toHaveCount(0); + await expect(page.locator('body')).not.toContainText(MARKER); + } + expect(new URL(page.url()).searchParams.get('lang')).toBeNull(); + await expectNoQuestion(page, watched, trusted); + }); + } + + // Every other address under /project/github.com: the project, or the page that says what is wrong with the + // address, with the question dropped. + for (const path of [ + '//project/github.com/acme/demo/queries', + '///project/github.com/acme/demo/queries', + '/(project/github.com/acme/demo/queries)', + '/project/github.com/acme/demo/queries', + '//project/github.com/acme/demo', + '/project;a=1/github.com/acme/demo/queries', + '/project/github.com/acme/demo/blob/main/x.txt', + ]) { + test(`${path}${'?msg=…&x=1'}: the address loses the question and keeps x=1, and the question is nowhere`, async ({ + page, + context, + baseURL, + }) => { + const watched = await openAndWatch( + page, + context, + baseURL ?? '', + `${path}?msg=${encodeURIComponent(MARKER)}&x=1`, + async () => { + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + }, + ); + await expect + .poll(() => new URL(page.url()).pathname) + .toMatch( + /^\/project\/github\.com\/acme\/demo(\/queries|\/blob\/main\/x\.txt)?$/, + ); + await expectNoQuestion(page, watched); + }); + } + + // The question as a matrix parameter of the path (`…;msg=Q`) is not a query: index.html's script does not look + // for it (it would have to rewrite the path), so an analytics tag that reads the address before the router has + // started could see it. The router then drops it with the canonical redirect: the address and the history keep + // nothing of it. + for (const typed of [ + '/project/github.com/acme/demo;msg=' + MARKER + '/queries', + '/project/github.com/acme/demo/queries;msg=' + MARKER, + '/project;q=' + MARKER + '/github.com/acme/demo/queries', + ]) { + test(`${typed}: the router's canonical redirect leaves it neither in the address nor in the history`, async ({ + page, + context, + baseURL, + }) => { + await onProductionLikeHost(context, baseURL ?? ''); + await page.goto(`${HOST}${typed}?x=1`); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + await expect + .poll(() => new URL(page.url()).pathname, { timeout: 20_000 }) + .toBe('/project/github.com/acme/demo/queries'); + await page.waitForTimeout(1500); + expect(new URL(page.url()).searchParams.get('x')).toBe('1'); + const state = await page.evaluate(() => ({ + title: document.title, + storage: + JSON.stringify(Object.entries(sessionStorage)) + + JSON.stringify(Object.entries(localStorage)), + entries: ( + window as unknown as { navigation: { entries: () => { url: string }[] } } + ).navigation + .entries() + .map((entry) => entry.url), + })); + expect(hasMarker(page.url())).toBe(false); + expect(state.entries.filter(hasMarker)).toEqual([]); + expect(hasMarker(state.title + state.storage)).toBe(false); + }); + } + + test('the matrix parameters of a short address are dropped by the canonical redirect', async ({ + page, + context, + baseURL, + }) => { + await onProductionLikeHost(context, baseURL ?? ''); + for (const typed of [ + '/project;a=1/github.com/acme/demo/queries', + '/project/github.com/acme/demo;b=2/queries', + '/project/github.com/acme/demo/queries;c=3', + ]) { + await page.goto(`${HOST}${typed}`); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + await expect + .poll(() => new URL(page.url()).pathname, { message: typed }) + .toBe('/project/github.com/acme/demo/queries'); + } + }); + + test('a folder with literal parentheses (as pasted from GitHub) shows the unsupported-address page, not folder a', async ({ + page, + context, + baseURL, + }) => { + await onProductionLikeHost(context, baseURL ?? ''); + await page.goto( + `${HOST}/project/github.com/Acme/demo/tree/HEAD/a(b)/-/queries`, + ); + await expect(page.getByText('This address is not supported')).toBeVisible({ + timeout: 20_000, + }); + }); + + test('a path written as one group at the root, without a question, is the project chat, not the holding page', async ({ + page, + context, + baseURL, + }) => { + await onProductionLikeHost(context, baseURL ?? ''); + await page.goto(`${HOST}/(project/github.com/acme/demo/chat)`); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + await expect(page.locator('#demo-holding-message')).toHaveCount(0); + await expect + .poll(() => new URL(page.url()).pathname) + .toBe('/project/github.com/acme/demo/chat'); + }); + + // An in-app navigation: the page is already open, and the router is asked to go to a short address with a + // question. The router's own Back/Forward handling is the way into the router that a production build allows + // from outside (it has no debugging handle); the unit specs call `router.navigateByUrl` itself. + test('a navigation inside the app to a short address with a question ends without it, in the address and the history', async ({ + page, + context, + baseURL, + }) => { + await onProductionLikeHost(context, baseURL ?? ''); + await page.goto(`${HOST}/project/github.com/acme/demo/chat`); + await expect(page.locator('ion-header').first()).toBeVisible({ + timeout: 20_000, + }); + // Back and Forward are the browser telling the router that the address changed. + await page.evaluate( + (question) => { + history.pushState( + history.state, + '', + `/project/github.com/acme/demo/queries?msg=${encodeURIComponent(question)}&x=1`, + ); + dispatchEvent(new PopStateEvent('popstate', { state: history.state })); + }, + MARKER, + ); + await expect + .poll(() => new URL(page.url()).pathname, { timeout: 20_000 }) + .toBe('/project/github.com/acme/demo/queries'); + await page.waitForTimeout(1500); + expect(new URL(page.url()).searchParams.has('msg')).toBe(false); + expect(new URL(page.url()).searchParams.get('x')).toBe('1'); + const entries = await page.evaluate(() => + ( + window as unknown as { navigation: { entries: () => { url: string }[] } } + ).navigation + .entries() + .map((entry) => entry.url), + ); + // the entry the test pushed itself carried the question; the router replaced it + expect(entries.filter(hasMarker)).toEqual([]); + }); +}); + test.describe('the matrix-parameter address reaches no analytics either', () => { const HOST = 'https://handoff.datatug.test'; test('/demo;x=1?q=…: Google Analytics, Sentry and PostHog never receive the question', async ({ diff --git a/apps/datatug-app/src/app/datatug-app-routes.spec.ts b/apps/datatug-app/src/app/datatug-app-routes.spec.ts index 8621a4d3..3f256155 100644 --- a/apps/datatug-app/src/app/datatug-app-routes.spec.ts +++ b/apps/datatug-app/src/app/datatug-app-routes.spec.ts @@ -176,6 +176,81 @@ describe('DataTug app routes', () => { expect((await visit('datatug.app', url)).component, 'no query').toBe(ProjectStub); }); + // review r2, B1: the script reads the path its own way, so a spelling of the address that only the router + // understands arrives with the question still in the navigation's query, and no stash. + describe('a question in the navigation query that the script did not take out (review r2, B1)', () => { + it.each([ + '//project/github.com/datatug/chinook-demo/chat', + '//project/github.com/acme/demo/chat', + '///project/github.com/acme/demo/chat', + '/(project/github.com/datatug/chinook-demo/chat)', + '/(project/github.com/acme/demo/chat)', + '/(project/github.com/acme/demo/tree/HEAD/-/chat)', + '//project/github.com/acme/demo/tree/HEAD/-/chat', + '/project/github.com/acme/demo/Tree/HEAD/-/chat', + '/demo', + '//demo', + '/(demo)', + ])('%s?msg=… is the holding page, and the question is stashed for it to capture', async (url) => { + const result = await visit('datatug.app', `${url}?msg=Q&lang=ru`); + expect(result.component).toBe(HandoffStub); + expect( + (window as unknown as Record)[DEMO_HANDOFF_STASH], + ).toBe('?msg=Q&lang=ru'); + }); + + it.each([ + ['?q=Q', '?q=Q'], + ['?msg=A&msg=B', '?msg=A&msg=B'], + ['?msg=a%20b%26c', '?msg=a+b%26c'], + ])('the query %s is stashed as it was asked: %s', async (typed, stashed) => { + const result = await visit( + 'datatug.app', + `//project/github.com/acme/demo/chat${typed}`, + ); + expect(result.component).toBe(HandoffStub); + expect( + (window as unknown as Record)[DEMO_HANDOFF_STASH], + ).toBe(stashed); + }); + + it.each([ + '//project/github.com/acme/demo/chat', + '/(project/github.com/acme/demo/chat)', + ])('%s with no question, or a blank one, is the project: the chat page', async (url) => { + for (const search of ['', '?msg=', '?lang=ru']) { + TestBed.resetTestingModule(); + resetHandoffAskedForTests(); + expect((await visit('datatug.app', url + search)).component, search).toBe(ProjectStub); + } + }); + + it('every other address under a project is the project, question or not: the short route drops the question', async () => { + for (const url of [ + '//project/github.com/acme/demo/queries', + '/(project/github.com/acme/demo/queries)', + '/project/github.com/acme/demo/chat/more', + ]) { + TestBed.resetTestingModule(); + resetHandoffAskedForTests(); + expect((await visit('datatug.app', url + '?msg=Q')).component, url).toBe(ProjectStub); + expect( + (window as unknown as Record)[DEMO_HANDOFF_STASH], + url, + ).toBeUndefined(); + } + }); + + it('at app.incidentius.com the same addresses go to the root, with the query dropped', async () => { + const result = await visit( + 'app.incidentius.com', + '//project/github.com/acme/demo/chat?msg=Q', + ); + expect(result.component).toBe(HomeStub); + expect(result.url).toBe('/'); + }); + }); + it('a reload of a question that was asked shows the holding page again; a fresh visit shows the project', async () => { const url = '/project/github.com/datatug/chinook-demo/chat'; window.sessionStorage.setItem(DEMO_HANDOFF_KEY, url + '?msg=Hello'); diff --git a/apps/datatug-app/src/app/datatug-app-routes.ts b/apps/datatug-app/src/app/datatug-app-routes.ts index 926e4b7f..493ee300 100644 --- a/apps/datatug-app/src/app/datatug-app-routes.ts +++ b/apps/datatug-app/src/app/datatug-app-routes.ts @@ -2,7 +2,7 @@ import { inject } from '@angular/core'; import { CanMatchFn, Router, Routes } from '@angular/router'; import { PRODUCT_PROFILE } from '@datatug/product-profiles'; import { cliChatCapability } from './cli-chat-capability'; -import { showsHoldingPage } from './demo-handoff-asked'; +import { searchOfQueryParams, showsHoldingPage } from './demo-handoff-asked'; import { handoffUrlMatcher } from './demo-handoff-path'; // Task 13 (S108, spec/research/2026-09-09-layered-acl-reconciliation.md, @@ -49,10 +49,18 @@ export const datatugProfileOnly = (): boolean => inject(PRODUCT_PROFILE).id === * address that arrived with a question, and no match (the router goes on to the project routes) for a project chat * address without one; under every other profile, the root. */ -export const handoffOrRoot: CanMatchFn = (_route, segments) => - datatugProfileOnly() - ? showsHoldingPage(segments.map((segment) => segment.path)) - : inject(Router).parseUrl('/'); +export const handoffOrRoot: CanMatchFn = (_route, segments) => { + const router = inject(Router); + if (!datatugProfileOnly()) return router.parseUrl('/'); + // The query the router has is normally empty (index.html's script took it out of the address bar before the + // router started). It is not when the script reads the path another way than the router does: then a question in + // it is as good as one the script stashed (demo-handoff-asked.ts). + return showsHoldingPage( + segments.map((segment) => segment.path), + undefined, + searchOfQueryParams(router.getCurrentNavigation()?.extractedUrl.queryParams ?? {}), + ); +}; export const routes: Routes = [ { diff --git a/apps/datatug-app/src/app/demo-handoff-asked.spec.ts b/apps/datatug-app/src/app/demo-handoff-asked.spec.ts index 53e18fa4..3ba114a0 100644 --- a/apps/datatug-app/src/app/demo-handoff-asked.spec.ts +++ b/apps/datatug-app/src/app/demo-handoff-asked.spec.ts @@ -44,8 +44,8 @@ function env( }; } -const shows = (path: string, e?: AskedEnv): boolean => - showsHoldingPage(routeSegments(path), e); +const shows = (path: string, e?: AskedEnv, routerSearch?: string): boolean => + showsHoldingPage(routeSegments(path), e, routerSearch); describe('which hand-off addresses show the holding page', () => { beforeEach(() => { @@ -53,6 +53,72 @@ describe('which hand-off addresses show the holding page', () => { removed.length = 0; }); + // Review r2, B1: an address that the router matches but index.html's script did not take the question out of + // (the script reads the path its own way) arrives with the question in the navigation's query and no stash. + describe('a question in the navigation query that the script did not take out (review r2, B1)', () => { + it('is asked: the router matched a hand-off address with a question in its query, and there is no stash', () => { + expect(shows(CHAT, env({ navigation: 'navigate' }), '?msg=Hello')).toBe( + true, + ); + }); + + it('and the query is stashed for the holding page, which captures it from there and takes it out of the address', () => { + const e = env({ navigation: 'navigate' }); + expect(shows(CHAT, e, '?msg=Hello&lang=ru')).toBe(true); + expect(e.stash[DEMO_HANDOFF_STASH]).toBe('?msg=Hello&lang=ru'); + expect(removed).toEqual([]); + }); + + it('counts q, the old name, as a question too', () => { + expect(shows(CHAT, env({ navigation: 'navigate' }), '?q=Hello')).toBe( + true, + ); + }); + + it('a blank question, or no question, is not one: the chat page, and a stored copy is forgotten as for any bare visit', () => { + for (const search of ['', '?msg=', '?msg=%20%0A', '?lang=ru&x=1']) { + resetHandoffAskedForTests(); + removed.length = 0; + const e = env({ navigation: 'navigate', stored: CHAT + '?msg=Old' }); + expect(shows(CHAT, e, search), search).toBe(false); + expect(removed, search).toEqual([DEMO_HANDOFF_KEY]); + expect(DEMO_HANDOFF_STASH in e.stash).toBe(false); + } + }); + + it('what the script stashed wins over what the router has', () => { + const e = env({ stash: '?lang=ru', navigation: 'navigate' }); + expect(shows(CHAT, e, '?msg=Hello')).toBe(false); + expect(DEMO_HANDOFF_STASH in e.stash).toBe(false); + }); + + it('is asked again for the same address later in the page, whatever was settled before (an in-app navigation)', () => { + expect(shows(CHAT, env({ navigation: 'navigate' }))).toBe(false); + expect(shows(CHAT, env({ navigation: 'navigate' }), '?msg=Later')).toBe( + true, + ); + }); + + it('/demo needs no question, and its query is stashed as well, for the holding page to capture', () => { + const e = env({ navigation: 'navigate' }); + expect(shows('/demo', e, '?q=Hello')).toBe(true); + expect(e.stash[DEMO_HANDOFF_STASH]).toBe('?q=Hello'); + const none = env({ navigation: 'navigate' }); + expect(shows('/demo', none, '')).toBe(true); + expect(DEMO_HANDOFF_STASH in none.stash).toBe(false); + }); + + it("a trusted and an untrusted repository are alike here: whether the question is shown back is the capture's to say", () => { + expect( + shows( + '/project/github.com/someone/else/chat', + env({ navigation: 'navigate' }), + '?msg=Hello', + ), + ).toBe(true); + }); + }); + // Review S1 and minor 6 of G-A1b: on main a fresh bare visit removed the question kept in the tab, so a reload // shows the project chat; the answer "no question" must do the same, and the stash must not outlive it. describe('an answer of "no question" forgets the question kept for this tab', () => { diff --git a/apps/datatug-app/src/app/demo-handoff-asked.ts b/apps/datatug-app/src/app/demo-handoff-asked.ts index b490ff5d..d25a47b6 100644 --- a/apps/datatug-app/src/app/demo-handoff-asked.ts +++ b/apps/datatug-app/src/app/demo-handoff-asked.ts @@ -102,6 +102,9 @@ const keyOf = (segments: readonly string[]): string => * 4. a reload or back/forward: the copy of the query kept in storage for this very address. * Blocked storage means no for 3 and 4: the question lives only in the page. * + * `routerSearch` is the query the router has for this navigation (review r2, B1). Normally empty: the script took + * the query out before the router started. A question in it, with no stash, is "asked" (and goes into the stash). + * * An answer of "no" that comes from 1 or 3 also forgets the question kept for this tab (the copy in storage, and * the stash, which only the holding page reads), as `captureDemoHandoff` did on main for a bare visit: otherwise a * reload would bring the holding page back, with a question the visitor did not ask this time. @@ -109,6 +112,7 @@ const keyOf = (segments: readonly string[]): string => export function handoffAsked( segments: readonly string[], env: AskedEnv = defaultEnv(), + routerSearch = '', ): boolean { const key = keyOf(segments); const stashed = env.stash[DEMO_HANDOFF_STASH]; @@ -121,6 +125,14 @@ export function handoffAsked( } return asked; } + if (searchAsksQuestion(routerSearch)) { + // The router matched this address with a question in its query that the script did not take out (it reads + // the path its own way, and an address can be spelled in ways it does not): asked. Stashed, so that the + // holding page captures it and takes it out of the address as it does for every other hand-off. + env.stash[DEMO_HANDOFF_STASH] = routerSearch; + settled.set(key, true); + return true; + } const known = settled.get(key); if (known !== undefined) return known; if (env.navigationType() === 'navigate') { @@ -149,11 +161,32 @@ export function handoffAsked( */ export function showsHoldingPage( segments: readonly string[], - env?: AskedEnv, + env: AskedEnv = defaultEnv(), + routerSearch = '', ): boolean { const target = handoffTarget(segments); if (!target) return false; - return target.kind === 'demo' || handoffAsked(segments, env); + if (target.kind === 'demo') { + if (routerSearch && typeof env.stash[DEMO_HANDOFF_STASH] !== 'string') { + env.stash[DEMO_HANDOFF_STASH] = routerSearch; + } + return true; + } + return handoffAsked(segments, env, routerSearch); +} + +/** The query of a navigation as a query string (`?a=1&b=2`; '' when it has none), as the capture parses it. */ +export function searchOfQueryParams( + queryParams: Record, +): string { + const search = new URLSearchParams(); + for (const [name, value] of Object.entries(queryParams)) { + for (const each of Array.isArray(value) ? value : [value]) { + if (each !== undefined) search.append(name, each); + } + } + const text = search.toString(); + return text === '' ? '' : `?${text}`; } /** For tests: forgets what this page load settled. */ diff --git a/apps/datatug-app/src/app/demo-handoff-capture.spec.ts b/apps/datatug-app/src/app/demo-handoff-capture.spec.ts index 9e52c5c2..bc678482 100644 --- a/apps/datatug-app/src/app/demo-handoff-capture.spec.ts +++ b/apps/datatug-app/src/app/demo-handoff-capture.spec.ts @@ -13,6 +13,12 @@ import { resetDemoHandoffForTests, truncateToBytes, } from './demo-handoff-capture'; +import { routeSegments } from './demo-handoff-path'; + +const ORIGIN = 'https://app.test'; +/** The address that is given to history.replaceState: a path that starts with `//` needs its origin (review r2). */ +const toReplaceState = (path: string): string => + (path.startsWith('//') ? ORIGIN : '') + path; const bytes = (text: string): number => new TextEncoder().encode(text).length; @@ -50,7 +56,12 @@ function pageLoad( removeItem: (key: string) => blocked === 'none' ? void store.delete(key) : thrower(), } as unknown as Storage; - const loc = { pathname, search, hash: options.hash ?? '' } as Location; + const loc = { + pathname, + search, + hash: options.hash ?? '', + origin: ORIGIN, + } as Location; const history = { state: { x: 1 }, replaceState: (_state: unknown, _title: string, url: string) => { @@ -328,6 +339,56 @@ describe('demo hand-off capture', () => { ).toBe('Hello'); }); + // review r2, B1: spellings of the address that the router reads as a hand-off address + describe('an address written another way (review r2, B1)', () => { + it.each([ + ['//project/github.com/datatug/chinook-demo/chat', true], + ['//project/github.com/acme/demo/chat', false], + ['///project/github.com/datatug/chinook-demo/tree/HEAD/-/chat', true], + ['/(project/github.com/datatug/chinook-demo/chat)', true], + ['/(project/github.com/acme/demo/chat)', false], + ['//demo', true], + ['/(demo)', true], + ])( + '%s: the query is taken out of the address (a `//` path with its origin), kept for the page, and shown back only where the address may echo it', + (path, shown) => { + const f = pageLoad(path, '?msg=Hello&lang=ru', { hash: '#x' }); + captureDemoHandoff(f.env); + expect(f.replaced).toEqual([toReplaceState(path) + '#x']); + expect(demoHandoff(undefined, path)?.question).toBe( + shown ? 'Hello' : '', + ); + expect(demoHandoff(undefined, path)?.lang).toBe('ru'); + }, + ); + + it('what the script kept under the typed path is found again under the path the router has put there since', () => { + const typed = '//project/github.com/datatug/chinook-demo/chat'; + const first = pageLoad(typed, '?msg=Hello'); + captureDemoHandoff(first.env); + resetDemoHandoffForTests(); + const reload = pageLoad( + '/project/github.com/datatug/chinook-demo/chat', + '', + { store: first.store, navigation: 'reload' }, + ); + captureDemoHandoff(reload.env); + expect( + demoHandoff( + () => reload.env.storage(), + '/project/github.com/datatug/chinook-demo/chat', + )?.question, + ).toBe('Hello'); + // another address is another address + expect( + demoHandoff( + () => reload.env.storage(), + '/project/github.com/datatug/other-demo/chat', + ), + ).toBeUndefined(); + }); + }); + it('takes the query the inline script of index.html stashed, and forgets the stash', () => { const f = pageLoad('/demo', '', { stashed: '?q=From+the+stash&lang=ru' }); captureDemoHandoff(f.env); @@ -687,7 +748,7 @@ describe('demo hand-off capture', () => { setItem: (key: string, value: string) => blocked ? thrower() : void kept.set(key, value), }; - const location = { pathname, search, hash }; + const location = { pathname, search, hash, origin: ORIGIN }; const history = { state: null, replaceState: (_s: unknown, _t: string, url: string) => { @@ -753,19 +814,44 @@ describe('demo hand-off capture', () => { '/project/github.com/o/r/tree//-/chat', '/project/github.com/o%2Fx/r/chat', '/project/github.com/%E0%A4%A/r/chat', + // review r2: spellings that the router's parser reads as the same address + '//demo', + '///demo/', + '/(demo)', + '/(Demo;x=1)/', + '/(demo//menu:x)', + '/(demo)(menu:x)', + '/(demo/other)', + '/demo//other', + '/demo///', + '//project/github.com/datatug/chinook-demo/chat', + '///project/github.com/o/r/chat/', + '/(project/github.com/o/r/chat)', + '/(PROJECT/github.com/acme/demo/tree/HEAD/-/chat)', + '/(project/github.com/o/r/chat//menu:x)', + '/(project/github.com/o/r/queries)', + '//project/github.com/o/r/queries', + '///project/github.com/o/r', + '/project//github.com/o/r/chat', + '/project/github.com/o/r//chat', + '/project/github.com/o/r/tree/HEAD/a(b)/-/chat', ]; // Every other address under /project/github.com loses `msg` and `q` only (review S2): nothing is stashed - // or kept for it, and the rest of its query stays. - const underProject = (path: string) => - /^\/project\/github\.com(?:[/;(]|$)/i.test(path); + // or kept for it, and the rest of its query stays. ("Under" as the router's parser reads the path.) + const underProject = (path: string) => { + const [first = '', second = ''] = routeSegments(path).map((s) => + s.toLowerCase(), + ); + return first === 'project' && second.split('(')[0] === 'github.com'; + }; for (const path of paths) { const result = run(path, '?q=x&lang=ru', '#h'); expect(result.stash !== undefined, path).toBe(isHandoffPath(path)); expect(result.replaced, path).toEqual( isHandoffPath(path) - ? [path + '#h'] + ? [toReplaceState(path) + '#h'] : underProject(path) - ? [path + '?lang=ru#h'] + ? [toReplaceState(path) + '?lang=ru#h'] : [], ); if (!isHandoffPath(path)) expect(result.kept, path).toEqual(new Map()); @@ -857,6 +943,73 @@ describe('demo hand-off capture', () => { }); }); + // review r2, B1: the script read these addresses differently from the router, so the question stayed in the + // address bar and the history, and went to analytics + describe('an address that the router reads as a hand-off address is one for the script too (review r2, B1)', () => { + it.each([ + '//demo', + '///demo/', + '/(demo)', + '/(Demo;x=1)', + '//project/github.com/datatug/chinook-demo/chat', + '//project/github.com/acme/demo/chat', + '///project/github.com/acme/demo/chat/', + '/(project/github.com/datatug/chinook-demo/chat)', + '/(project/github.com/acme/demo/chat)', + '/(project/github.com/acme/demo/chat)/', + '/(Project/GitHub.com/acme/demo/tree/HEAD/-/chat)', + '//project/github.com/acme/demo/tree/HEAD/-/chat', + '/project/github.com/acme/demo/Tree/HEAD/-/chat', + ])('%s: the question is stashed, kept for a reload, and out of the address bar', (path) => { + expect(isHandoffPath(path)).toBe(true); + const result = run(path, '?msg=Q&lang=ru', '#h'); + expect(result.stash).toBe('?msg=Q&lang=ru'); + expect(result.replaced).toEqual([toReplaceState(path) + '#h']); + expect(result.kept).toEqual( + new Map([[DEMO_HANDOFF_KEY, path.replace(/\/$/, '') + '?msg=Q&lang=ru']]), + ); + }); + + it.each([ + '//project/github.com/acme/demo/queries', + '///project/github.com/acme/demo/queries', + '/(project/github.com/acme/demo/queries)', + '/(project/github.com/acme/demo/queries)/', + '//project/github.com/acme/demo', + '//project/github.com/acme/demo/tree/HEAD/dir/-/chat', + '/(project/github.com/acme/demo/chat//menu:x)', + ])('%s: the question is dropped, the rest of the query stays', (path) => { + const result = run(path, '?msg=Q&x=1', '#h'); + expect(result.stash).toBeUndefined(); + expect(result.replaced).toEqual([toReplaceState(path) + '?x=1#h']); + expect(result.kept).toEqual(new Map()); + }); + + it('an address that is no project and no hand-off, written the same way, is left alone', () => { + for (const path of ['//no-such-route', '/(queries)', '///', '/(menu:x)']) { + const result = run(path, '?msg=Q'); + expect(result.stash, path).toBeUndefined(); + expect(result.replaced, path).toEqual([]); + } + }); + + it('a reload of a hand-off address written another way, after the script and before the app, shows the same question', () => { + const first = run('//project/github.com/datatug/chinook-demo/chat', '?msg=Early'); + const reload = pageLoad( + '/project/github.com/datatug/chinook-demo/chat', + '', + { store: first.kept, navigation: 'reload' }, + ); + captureDemoHandoff(reload.env); + expect( + demoHandoff( + () => reload.env.storage(), + '/project/github.com/datatug/chinook-demo/chat', + )?.question, + ).toBe('Early'); + }); + }); + it('stashes the raw query string for the TypeScript side to parse', () => { expect(run('/demo', '?scenario=a&q=hello%20world').stash).toBe( '?scenario=a&q=hello%20world', diff --git a/apps/datatug-app/src/app/demo-handoff-capture.ts b/apps/datatug-app/src/app/demo-handoff-capture.ts index 71dc9c63..631291c0 100644 --- a/apps/datatug-app/src/app/demo-handoff-capture.ts +++ b/apps/datatug-app/src/app/demo-handoff-capture.ts @@ -31,7 +31,11 @@ import { DEMO_HANDOFF_STASH, questionOfSearch, } from './demo-handoff-asked'; -import { handoffTargetOfPath, isHandoffPath } from './demo-handoff-path'; +import { + handoffTargetOfPath, + isHandoffPath, + routeSegments, +} from './demo-handoff-path'; export { isHandoffPath, DEMO_HANDOFF_KEY, DEMO_HANDOFF_STASH }; @@ -69,6 +73,16 @@ export function isTrustedHandoff(pathname: string): boolean { return isTrustedProjectAddress(project); } +/** + * This address with no query, for `history.replaceState`. A path that starts with `//` (`//project/…`, which the + * router reads as `/project/…`) is, to replaceState, an address on another host: it needs its origin. + */ +function withoutQuery(loc: Location): string { + return ( + (loc.pathname.startsWith('//') ? loc.origin : '') + loc.pathname + loc.hash + ); +} + /** What is kept for a reload: the path and the raw query string, so it is only applied to the same path. */ function storedFor(pathname: string, search: string): string { return pathname.replace(/\/$/, '') + search; @@ -166,7 +180,7 @@ export function captureDemoHandoff(env: CaptureEnv = defaultEnv()): void { // Strip first and unconditionally: nothing below may leave the question in the URL. A browser that refuses // (a SecurityError, an exotic embedding) must not stop the page from showing what it has. try { - history.replaceState(history.state, '', loc.pathname + loc.hash); + history.replaceState(history.state, '', withoutQuery(loc)); } catch { // The address keeps its query; the question is still held in memory below. } @@ -207,6 +221,17 @@ function store(env: CaptureEnv, search: string | undefined): void { } } +/** + * Whether two paths are the same address for the router: `//project/…` as typed, kept for a reload, and the + * `/project/…` the router has since replaced it with, are one. + */ +function samePath(kept: string, pathname: string): boolean { + return ( + JSON.stringify(routeSegments(kept)) === + JSON.stringify(routeSegments(pathname)) + ); +} + /** * The hand-off for the holding page: the one captured in this page load, else the one kept for a reload, else * `undefined` (a bare visit, or storage is blocked after a reload: the page then shows its no-question copy in @@ -226,8 +251,7 @@ export function demoHandoff( try { const raw = storage().getItem(DEMO_HANDOFF_KEY) ?? ''; const at = raw.indexOf('?'); - if (at < 0 || raw.slice(0, at) !== pathname.replace(/\/$/, '')) - return undefined; + if (at < 0 || !samePath(raw.slice(0, at), pathname)) return undefined; return forPath(parseHandoffSearch(raw.slice(at))); } catch { return undefined; diff --git a/apps/datatug-app/src/app/demo-handoff-path.spec.ts b/apps/datatug-app/src/app/demo-handoff-path.spec.ts index 0c8eded9..3315f529 100644 --- a/apps/datatug-app/src/app/demo-handoff-path.spec.ts +++ b/apps/datatug-app/src/app/demo-handoff-path.spec.ts @@ -58,6 +58,29 @@ const ADDRESSES = [ '/demo/(menu:x)', '/Demo(menu:x/y)', '/project/github.com/o/r/chat(menu:x)', + // review r2: other spellings of the same path, which the router's parser reads as the same address + '//demo', + '///demo', + '//demo/', + '/(demo)', + '/(demo)/', + '/(Demo;x=1)', + '/(demo//menu:x)', + '/(demo)(menu:x)', + '/(demo/other)', + '/demo//other', + '/demo///', + '//project/github.com/o/r/chat', + '///project/github.com/o/r/chat', + '/(project/github.com/o/r/chat)', + '/(project/github.com/o/r/tree/abc123/-/chat)', + '/(project/github.com/o/r/tree/abc123/-/chat//menu:x)', + '//project/github.com/o/r/tree/abc123/-/chat', + '/project//github.com/o/r/chat', + '/project/github.com/o/r//chat', + '/project/github.com/o/r/chat//', + '/project/github.com/o/r/chat(menu:x)/', + '/project/github.com/o/r/tree/HEAD/a(b)/-/chat', ]; // A browser address reaches the router through Angular's Location, which drops one trailing slash before the @@ -77,6 +100,20 @@ describe('demo-handoff-path', () => { ['/a/b;c=d/e%20f', ['a', 'b', 'e f']], ['/%E0%A4%A', ['%E0%A4%A']], ['/demo//', ['demo', '']], + // review r2: how the router's parser reads these (index.html's script does the same) + ['//demo', ['demo']], + ['///demo/', ['demo']], + ['/(demo)', ['demo']], + [ + '/(project/github.com/o/r/chat)', + ['project', 'github.com', 'o', 'r', 'chat'], + ], + ['/(/project)', ['project']], + ['/(demo)/', ['demo']], + ['/(demo)(menu:x)', ['demo']], + ['/project//github.com/o/r/chat', ['project']], + ['/a/b//c/d', ['a', 'b']], + ['/(a;x=1/B%20c)', ['a', 'B c']], ])('%j is %j', (pathname, expected) => { expect(routeSegments(pathname)).toEqual(expected); }); @@ -97,11 +134,50 @@ describe('demo-handoff-path', () => { expect(isHandoffPath('/demo')).toBe(true); expect(handoffTargetOfPath('/demo')).toEqual({ kind: 'demo' }); }); + it.each([ + '/(demo//menu:x)', + '/(project/github.com/o/r/chat//menu:x)', + '/project/github.com/o/r/tree/HEAD/a(b)/-/chat', + ])('%s', (path) => { + expect(isHandoffPath(path)).toBe(false); + }); it('and an encoded parenthesis is just a character of a segment', () => { expect(isHandoffPath('/project/github.com/a%28b/r/chat')).toBe(true); }); }); + describe('a path written another way that the router reads as a hand-off address is one (review r2)', () => { + it.each([ + ['//demo', { kind: 'demo' }], + ['///demo', { kind: 'demo' }], + ['/(demo)', { kind: 'demo' }], + ['/(Demo;x=1)/', { kind: 'demo' }], + [ + '//project/github.com/datatug/chinook-demo/chat', + { + kind: 'chat', + owner: 'datatug', + repo: 'chinook-demo', + ref: undefined, + }, + ], + [ + '/(project/github.com/acme/demo/chat)', + { kind: 'chat', owner: 'acme', repo: 'demo', ref: undefined }, + ], + [ + '/(project/github.com/acme/demo/tree/HEAD/-/chat)', + { kind: 'chat', owner: 'acme', repo: 'demo', ref: 'HEAD' }, + ], + [ + '///project/github.com/acme/demo/tree/HEAD/-/chat/', + { kind: 'chat', owner: 'acme', repo: 'demo', ref: 'HEAD' }, + ], + ])('%s', (path, target) => { + expect(handoffTargetOfPath(path)).toEqual(target); + }); + }); + describe('handoffTarget', () => { it('reads the demo address, the chat address and the tree chat address', () => { expect(handoffTarget(['demo'])).toEqual({ kind: 'demo' }); diff --git a/apps/datatug-app/src/app/demo-handoff-path.ts b/apps/datatug-app/src/app/demo-handoff-path.ts index 54ab034d..8a610015 100644 --- a/apps/datatug-app/src/app/demo-handoff-path.ts +++ b/apps/datatug-app/src/app/demo-handoff-path.ts @@ -1,4 +1,9 @@ import type { UrlMatcher, UrlSegment, UrlSegmentGroup } from '@angular/router'; +import { + hasOutletGroup, + pathHasOutletGroup, + ROOT_GROUP, +} from '@datatug/project-address'; // The one definition, in TypeScript, of which address is a hand-off address. The route table (the matcher // below), the capture of the visitor's question (demo-handoff-capture.ts) and the trust check all use it. The @@ -7,9 +12,12 @@ import type { UrlMatcher, UrlSegment, UrlSegmentGroup } from '@angular/router'; // // The rules are the router's own: matrix parameters (`/demo;x=1`) and a trailing slash do not change which route // matches, and the segments are matched after percent-decoding. The literal segments (`demo`, `project`, -// `github.com`, `chat`, `tree`, `-`) match in any letter case; `/Demo` is the same page as `/demo`. +// `github.com`, `chat`, `tree`, `-`) match in any letter case; `/Demo` is the same page as `/demo`. So do the other +// spellings that the router's parser reads as the same path: `//demo` and `///demo`, and a path written as one +// group at the root, `/(demo)` (review r2: an address the script read differently from the router left the question +// in the address bar). // -// An address with an auxiliary-outlet group (`/demo(menu:x)`, any `(` in the path) is NOT a hand-off address, for +// An address with an auxiliary-outlet group (`/demo(menu:x)`, any `(` in the path but a root group) is NOT a hand-off address, for // the script, the TypeScript and the matcher alike: the router cannot show this page for it (the outlet group // matches nothing and the navigation fails, as on main), so nothing may claim to handle it. Its query is left // alone, exactly as for any other address that is not a hand-off. @@ -34,12 +42,21 @@ function decoded(segment: string): string { } /** - * The path segments of an address as the router reads them: one trailing slash ignored, matrix parameters - * dropped, each segment percent-decoded (raw when it does not decode). + * The path segments of an address as the router reads them: one trailing slash dropped (the router's Location does + * that before the parser sees the address), a root group read as its content (`/(demo)` is `/demo`), leading + * slashes collapsed (`//demo` is `/demo`), the path cut at an empty segment inside it (`/a//b` is `/a`: the parser + * stops there), matrix parameters dropped, and each segment percent-decoded (raw when it does not decode). */ export function routeSegments(pathname: string): string[] { - const parts = pathname.split('/').slice(1); - if (parts[parts.length - 1] === '') parts.pop(); + const group = ROOT_GROUP.exec(pathname); + const path = (group ? group[1] : pathname.replace(/\/$/, '')).replace( + /^\/+/, + '', + ); + if (path === '') return []; + const parts = path.split('/'); + const empty = parts.indexOf(''); + if (empty >= 0 && empty < parts.length - 1) parts.length = empty; return parts.map((part) => decoded(part.split(';')[0])); } @@ -76,9 +93,8 @@ export function handoffTarget( export function handoffTargetOfPath( pathname: string, ): HandoffTarget | undefined { - return pathname.includes('(') - ? undefined - : handoffTarget(routeSegments(pathname)); + if (pathHasOutletGroup(pathname)) return undefined; + return handoffTarget(routeSegments(pathname)); } /** Whether this URL path is a hand-off address. */ @@ -86,23 +102,6 @@ export function isHandoffPath(pathname: string): boolean { return handoffTargetOfPath(pathname) !== undefined; } -const carriesPath = (group: UrlSegmentGroup): boolean => - group.segments.length > 0 || group.hasChildren(); - -/** - * Whether the router sees an auxiliary-outlet group in this URL: a group below this one, or beside it, that - * carries a path. The empty group the router itself adds for a named outlet whose route has an empty path (the - * app's side menu, `outlet: 'menu'`) is not one. - */ -function hasOutletGroup(group: UrlSegmentGroup | null | undefined): boolean { - if (!group) return false; - const below = Object.values(group.children); - const beside = group.parent - ? Object.values(group.parent.children).filter((other) => other !== group) - : []; - return [...below, ...beside].some(carriesPath); -} - /** * The route matcher for every hand-off address: the whole path or nothing (a matcher is not `pathMatch: 'full'`), * and nothing when the URL carries an auxiliary-outlet group. diff --git a/apps/datatug-app/src/index.html b/apps/datatug-app/src/index.html index 2ad012e4..ed4701b7 100644 --- a/apps/datatug-app/src/index.html +++ b/apps/datatug-app/src/index.html @@ -23,28 +23,32 @@ + (their specs run this script to keep them in step): matrix parameters, a trailing slash, letter case, extra + leading slashes (`//demo`) and a path written as one group at the root (`/(demo)`) do not change what a + hand-off address is, exactly as the router's parser reads them (`s` is the path as the router has it); a + path with an outlet group `(` is not one. Any other path is untouched, except that every other address under + /project/github.com loses `msg` and `q` (the question is dropped, never kept: nothing reads it there yet), the + rest of its query and its fragment staying. -->