diff --git a/.claude/harness/architecture.md b/.claude/harness/architecture.md new file mode 100644 index 0000000..d0e46b4 --- /dev/null +++ b/.claude/harness/architecture.md @@ -0,0 +1,83 @@ +# JavaScript Content API Library — Architecture + +Reference description of the project, its layout, the core modules, the request flow, and the CaaS storage model it reads from. Referenced from the project `CLAUDE.md`. + +## Project overview + +**fsxa-api** (the JavaScript Content API Library, a.k.a. Content API) is a published npm library — not an application. It reads content from the FirstSpirit **CaaS** (Content-as-a-Service) and from the **Navigation Service**, and maps the raw CaaS JSON into a stable, consumer-facing shape for PWAs and other frontends. It ships as CommonJS + ES5 bundles plus type declarations, and is consumed by Crownpeak PWA templates and by customer projects. + +Because it is a library, its exported surface is the product. See "Public API surface" below. + +## Layout + +| Path | Description | +|------|-------------| +| `src/modules/` | The core: both API implementations, the mapper, the query builder, the event stream, logging | +| `src/integrations/` | Adapters for hosting the proxy backend — the Express router and the framework-agnostic wrapper | +| `src/types.ts` | Both sides of every mapping: `CaaSApi_*` (raw CaaS shapes) and the mapped consumer shapes | +| `src/enums.ts` | Error message enums, content mode, proxy routes, HTTP status | +| `src/routes.ts` | Proxy route paths and request body interfaces — the contract between proxy and remote side | +| `src/testutils/` | Fixture factories used by unit tests (`createPageRef`, `createDataset`, …) | +| `src/helpers/`, `src/utils.ts` | Small pure helpers (locale discovery, navigation-map pruning, regex validation, rich-text link merging) | +| `integrationtests/` | Tests against a real CaaS tenant — see the testing guidelines | +| `proxy/` | npm workspace publishing the proxy-only entry point | +| `dev/` | Local scratch harness (`npm run dev`) for hitting a real CaaS by hand | +| `docs/superpowers/plans/` | Implementation plans written by the `writing-plans` skill | + +## The two API implementations + +Both implement the same `FSXAApi` interface (`src/types.ts`) and expose `fetchElement`, `fetchByFilter`, `fetchNavigation`, `fetchProjectProperties`: + +- **`FSXARemoteApi`** (`src/modules/FSXARemoteApi.ts`) — talks to CaaS and the Navigation Service directly. It holds the API key, so it only ever runs server-side. It owns URL construction (`buildCaaSUrl`, `buildNavigationServiceUrl`), the remote project lookup (`getRemoteConfigById`, `verifyRemoteProjectExists`), and the config: `apikey`, `caasURL`, `navigationServiceURL`, `tenantID`, `projectID`, `contentMode`, `remotes`, `maxReferenceDepth`, `customMapper`, `navigationItemFilter`, `caasItemFilter`. +- **`FSXAProxyApi`** (`src/modules/FSXAProxyApi.ts`) — same interface, but forwards each call as an HTTP POST to a backend that hosts an `FSXARemoteApi`. It carries no secrets and is the client-side implementation. + +`FSXAApiSingleton` holds one process-wide instance. The proxy backend is built with `src/integrations/express.ts` (`getExpressRouter`) or, for other frameworks, `useEndpointIntegrationWrapper` in `src/integrations/endpointIntegrationWrapper.ts`. The routes and body shapes both sides agree on live in `src/routes.ts`. + +**Consequence for any change to a fetch method:** a new parameter has to be threaded through `FSXAApi` (the interface), `FSXARemoteApi` (the implementation), `FSXAProxyApi` (serialize into the request body), `src/routes.ts` (the body interface), and the integration adapters (deserialize and validate). Changing only the remote side silently leaves proxy-mode consumers behind. + +## CaaSMapper and reference resolution + +`src/modules/CaaSMapper.ts` is the heart of the library and the file most work touches. It converts raw CaaS documents (`CaaSApi_PageRef`, `CaaSApi_Dataset`, `CaaSApi_Media`, …) into mapped items, walking the FirstSpirit document model: `PageRef` → `Page` → `Body` → `Section`, each carrying `formData` / `metaFormData` of typed input components (`CMS_INPUT_*`, `FS_REFERENCE`, `FS_DATASET`, `FS_INDEX`, `FS_CATALOG`, `CMS_INPUT_IMAGEMAP`, `Content2Section`). + +Reference resolution is **two-phase**, and understanding this is a prerequisite for editing the mapper: + +1. **Register.** While mapping, a reference is not fetched. `registerReference` / `registerReferencedItem` record the referenced id together with the path in the output object where the resolved item must later be placed, and mapping returns a placeholder string. One id can be registered at many paths. +2. **Resolve.** `resolveAllReferences` walks the registered groups and calls `resolveReferencesForGroup` per group, which chunks ids (`REFERENCED_ITEMS_CHUNK_SIZE = 30`) and fetches them via `fetchByFilter`. +3. **Denormalize.** `MappingUtils.denormalizeResolvedReferences` writes each fetched item into every path registered for it — or, in normalized mode, the caller receives `items` plus a flat `referenceMap` instead. + +Grouping is keyed by **(projectId, locale)** — `buildGroupKey` — because one CaaS filter query carries exactly one locale and one collection. Each disjoint pair therefore costs at least one request. `unifyId` namespaces ids as `projectId#uuid.locale` so items from different projects cannot collide in the cache or the reference map. + +`resolveReferenceTarget` decides where a reference is resolved. The **project** comes from the reference's own CaaS document URL (`src/modules/ReferenceUrlParser.ts`), falling back to the `remoteProject` field of a media reference and then to the surrounding document. The **locale** comes from the `remotes` entry configured for that project, or — with `useSourceLocale` — from `CaaSMapper.sourceLocale`, the locale the request was made in. The configuration is consulted before the own project id, so an entry naming the own project reads its references in another locale. A project that is neither the own one nor in `remotes` is not fetched: `registerUnresolvableReference` logs a warning and leaves the placeholder in the payload. See the README section "Resolving references across projects". + +Two guards bound the recursion: `maxReferenceDepth` (default `DEFAULT_MAX_REFERENCE_DEPTH = 2`) and `_processedItems`, which prevents re-fetching an id already handled. + +**Sharp edge:** `setLocaleFromCaasItem` mutates `this.locale` per mapped item, so it is only safe for `buildPreviewId`. Everything that keys or groups references uses the immutable `sourceLocale` instead. Do not reintroduce `this.locale` into `unifyId` or `resolveReferencesForGroup`. + +## Request flow (remote mode) + +1. The consumer calls `fetchElement` / `fetchByFilter` / `fetchNavigation` / `fetchProjectProperties`. +2. `FSXARemoteApi` builds the URL (`buildCaaSUrl`) and, for filters, translates the query via `QueryBuilder` into the CaaS filter syntax; the requested locale becomes `locale.language` + `locale.country` filters. +3. The response is handed to `CaaSMapper`, which maps documents and registers references (phase 1). +4. References are resolved per group and denormalized into the result (phases 2 and 3). +5. Optional hooks run: `customMapper` per data entry, `caasItemFilter` on mapped items, `navigationItemFilter` on navigation items. + +`CaaSEventStream` (lazily loaded via `CaaSEventStreamLazy` so `better-sse` stays out of browser bundles) is a separate path: it streams CaaS change events instead of fetching documents. + +## CaaS storage model + +A CaaS document URL is `baseURL / tenantID / collectionID / documentID`, for example: + +``` +https://enterprise-caas-api.e-spirit.cloud/enterprise-prod/3bb083df-446f-4cdd-af7e-514886c4dc20.preview.content/21f3109e-63b2-47e8-9728-5680c2decb02.en_US +``` + +- `collectionID` = `..content` +- `documentID` = `.` + +Content lives in `preview.content` / `release.content`; binaries in `preview.files` / `release.files`. The **content mode never comes from content data** — it comes only from this library's configuration. Preserve that when touching URL construction or reference parsing. + +## Public API surface + +Everything re-exported from `src/index.ts` is published: the modules listed there, all of `src/enums.ts`, all of `src/types.ts`, the helpers, the exceptions, `ROUTES`, and the integration wrappers. A rename or a narrowed type in `src/types.ts` is a breaking change for consumers even if nothing inside `src/` notices. + +`src/modules/index.ts` re-exports in a **deliberate order** to break a circular-dependency cycle. Do not reorder those export lines. diff --git a/.claude/harness/coding-guidelines.md b/.claude/harness/coding-guidelines.md new file mode 100644 index 0000000..e6f2be8 --- /dev/null +++ b/.claude/harness/coding-guidelines.md @@ -0,0 +1,101 @@ +# JavaScript Content API Library — Coding Guidelines + +Project-specific coding rules that complement the project overview in `CLAUDE.md`. Apply at write time, not only at review time. + +## Code style & quality + +- **Language**: TypeScript, `strict: true`, compiled to ES5 / CommonJS (`tsconfig.json`). No decorators in new code despite `experimentalDecorators` being on. +- **Formatting**: Prettier with `semi: false`, `singleQuote: true` (config lives in `package.json`). Committed code uses the Prettier 2 trailing-comma default — see the formatting section in `CLAUDE.md` before running Prettier at all. +- **Node**: `.nvmrc` pins 24.12.0; CI runs 24.x. `engines` still claims `>=14`, so do not use syntax or APIs newer than the declared floor in shipped code without raising `engines` deliberately. +- **No linter**: there is no ESLint and no `lint` script. Type errors are the only automated style signal, so run `npx tsc --noEmit` yourself — `npm test` will not surface a type error in a file no test imports. +- **Dependencies**: this is a library consumed in browsers. Prefer none. A new runtime dependency lands in every consumer's bundle and needs an explicit justification; heavy or server-only packages belong in `optionalDependencies` and must be lazily imported like `better-sse` is in `CaaSEventStreamLazy.ts`. + +## Never break the published surface + +**Rule:** Treat every symbol re-exported from `src/index.ts` — including every type in `src/types.ts` — as a public contract. Additive changes only: new optional fields, new optional parameters, widened return types. When behavior must change or a name must go, deprecate instead of removing. + +This library is published to npm and consumed by PWA templates and customer projects that upgrade on their own schedule. A removed field or a narrowed type breaks builds in repositories you cannot see or fix. Removing something is also a release decision, not just a code decision: `release-it` reads Conventional Commits, so a `BREAKING CHANGE:` footer ships a major version. + +**Smell:** renaming a field in a `CaaSApi_*` or mapped type "because nothing in `src/` uses the old name" — `src/` is not the consumer. + +### How to deprecate + +1. Keep the old symbol working, and keep it exported. +2. Add a `@deprecated` JSDoc tag naming the replacement and the README section that explains the migration. +3. Document the change in `README.md` under a stable heading (update notices link to those anchors). +4. Carry the deprecated path for at least one release before proposing removal — and propose it, do not do it unasked. + +If a behavior change is unavoidable even while the old API keeps compiling (a value now derived from a different source, a config field now ignored), that is still a behavior change for existing applications: document it explicitly as such, not just in the changelog. + +## Content data must never decide where a request goes + +**Rule:** No value read from a CaaS document may determine the host, tenant, content mode, or locale of an outbound request that carries the API key. A URL found in content may supply one thing: the id of the project a reference points at — and only after that id has been found in the `remotes` configuration. + +`FSXARemoteApi` holds the CaaS API key and runs server-side. A URL inside a document is editor-controlled data. If it selected the target host, a crafted document would make the server send its API key to an attacker's endpoint — a textbook SSRF with credential leak. The `remotes` allowlist is what prevents that: every request url is assembled from `caasURL`, `tenantID`, the configured project id and the configured content mode. The locale is excluded for a different reason — an editor cannot control which locale a reference url carries, so following it produces requests for documents that do not exist. + +**Smell:** `fetch(reference.url, { headers: { Authorization: apikey } })`, passing a parsed `baseUrl` into `buildCaaSUrl`, or reading `ParsedReferenceUrl.locale` to decide what to fetch. + +### Checklist when adding anything URL-derived + +1. Parse with `ReferenceUrlParser` — do not hand-roll string splitting on CaaS URLs, and do not duplicate the layout knowledge it owns. +2. Take the project id from it and nothing else. Locale comes from `remotes`, content mode and host from the api configuration. +3. Look the project id up with `getRemoteConfigById` before anything is fetched. +4. On a project that is not configured, or an unparsable URL, log a warning and leave the reference unresolved — its placeholder stays in the payload. Do not fall back to "try it anyway". + +## Mirror the change across parallel siblings + +**Rule:** When you touch a method, branch, type, or route — whether fixing a bug, refactoring, or cleaning up — immediately scan the parallel siblings with the same pattern, test files included. If the same change on a sibling is structurally identical, include it in the same pass without asking, and flag it in the commit/PR message. + +This codebase is built from mirrored pairs and families. Fixing one member ships the same bug under a different name in the others. + +The sibling groups to check: + +1. **`FSXARemoteApi` ↔ `FSXAProxyApi`** — both implement `FSXAApi`. A changed parameter or behavior almost always needs the proxy counterpart, plus the body interface in `src/routes.ts`, plus the Express router in `src/integrations/express.ts` and `endpointIntegrationWrapper.ts`, plus `parameterValidation.ts`. +2. **The `mapDataEntry` component branches** — `FS_REFERENCE`, `FS_DATASET`, `FS_INDEX`, `FS_CATALOG`, `CMS_INPUT_IMAGEMAP` all register references; a fix to one registration site usually applies to the others. +3. **The `map*` family** — `mapPageRef`, `mapDataset`, `mapGCAPage`, `mapMedia*`, `mapProjectProperties`. +4. **Raw ↔ mapped types** — a new field on a `CaaSApi_*` type usually needs its counterpart on the mapped type, and vice versa. +5. **preview ↔ release** content modes. + +**Scope:** the sweep covers every file changed on the current branch, committed and uncommitted alike. Do not leave a half-applied edit in the working tree. + +A structurally identical mirror change is bounded — apply it inline. A mirror that needs real per-sibling design judgment is separate scope — name it and leave it, rather than silently widening the diff. + +## Register-then-resolve: never fetch during mapping + +**Rule:** Mapping code (`mapDataEntry`, `map*`) must not issue a fetch for a referenced item. Register the reference and return the placeholder; let `resolveAllReferences` do the I/O. + +The two-phase design is what makes reference loading batched: one request per (projectId, locale) group of up to 30 ids instead of one request per reference. A single `await api.fetchElement(...)` inside a mapping branch turns a page with 50 images into 50 sequential round-trips, and it bypasses both `_processedItems` dedup and the `maxReferenceDepth` guard. + +**Smell:** an `await this.api.fetch…` inside a `case 'FS_…'` branch, or building a result object that already contains a resolved item rather than the id returned by `registerReferencedItem`. + +### Checklist when adding a new referencing component type + +1. Add the branch to `mapDataEntry` and register via `registerReference` with the reference's own CaaS URL, so the project is derived, not guessed. +2. Return the placeholder `registerReference` gives you. It always returns one, including for a project that is not configured — that reference is deliberately left unresolved rather than dropped. +3. Confirm the new id ends up in the right group: assert on the group key and on the resulting `fetchByFilter` calls, not just on the mapped output. +4. Add the raw shape to `src/types.ts` and a factory to `src/testutils/`. + +## Comments must add information not derivable from the code + +**Rule:** Default to writing no comments. A comment is only justified when it adds information not derivable from the code under it — a hidden constraint, a workaround for a specific bug, a non-obvious invariant, behavior that would surprise a reader. If unsure whether a comment adds information: delete it. + +This applies at write time and at all times after — write no useless comment in the first place, and remove any useless comment you come across, even one you just wrote. Comment hygiene is never deferred to a cleanup pass. It applies to test code exactly as to production code. + +Never write comments that: + +1. Restate what the code does — a well-named identifier, signature, or return type already says it. +2. Reference the current task, ticket, or caller ("added for CAAS-123", "used by the proxy") — that belongs in the commit message and the PR description. +3. Mark removed code (`// removed foo`). +4. Explain a parameter whose meaning is obvious from its type and name. + +**Judge against the code, not a snippet:** evaluate a comment by reading it together with the full declaration and body it sits on, never from a `git diff` hunk or a `grep` match in isolation. + +Conversely, keep comments that encode a decision the code cannot show. This repo has several that earn their place: why `src/modules/index.ts` fixes its export order (circular dependency), why `CaaSEventStream` is loaded lazily (browser bundle size), why a reference URL is only trusted after an origin check. Documentation of the public API is different in kind — JSDoc on exported symbols is consumer-facing and welcome. + +## Errors: reuse the enums, stay actionable + +**Rule:** User-facing failure messages belong in `FSXAApiErrors` / `CaaSMapperErrors` / `QueryBuilderErrors`, not inline in a `throw`. Before adding a case, check whether one already covers it. + +Consumers assert on these strings and match on them in their own error handling, which makes the enum values part of the public surface — editing an existing message is a breaking change for someone. Add a new member instead. + +Choose deliberately between throwing and warning. A misconfiguration the consumer must fix (missing API key, invalid locale) throws at construction or call time. Bad *content* — an unparsable reference URL, a broken reference, a component the mapper does not know — must not take down a page render: log through `this._logger` / `this.logger` and degrade, as the mapper already does for dropped references. Never use bare `console.log`; the `Logger` respects the configured `logLevel`. diff --git a/.claude/harness/testing-guidelines.md b/.claude/harness/testing-guidelines.md new file mode 100644 index 0000000..0eb6b9c --- /dev/null +++ b/.claude/harness/testing-guidelines.md @@ -0,0 +1,93 @@ +# JavaScript Content API Library — Testing Guidelines + +Project-specific testing rules that complement the project overview in `CLAUDE.md`. Apply at write time, not only at review time. + +## Test setup & resources + +- **Runner**: Jest 30 with `ts-jest`, `testEnvironment: node`. Specs live next to their subject as `*.spec.ts`; `npm test` runs `./src` only. +- **HTTP**: `jest-fetch-mock`. Enable once per file with `require('jest-fetch-mock').enableFetchMocks()` and reset in `beforeEach` with `fetchMock.resetMocks()` (see `src/modules/FSXARemoteApi.spec.ts`). +- **Mapper tests**: `CaaSMapper.spec.ts` automocks the API with `jest.mock('./FSXARemoteApi')` and builds instances through a local `createApi()` helper. Use that helper rather than constructing a fresh mock inline, so shared stubs (such as the `isTrustedReferenceUrl` default) stay in one place. +- **Fixtures**: `src/testutils/` holds a factory per shape — `createPageRef`, `createPageRefBody`, `createSection`, `createDataset`, `createDatasetReference`, `createMediaPicture`, `createMediaPictureReference`, `createImageMap`, `createReferenceUrl`, `createFetchResponse`, `generateRandomConfig`. Use and extend them; do not paste raw CaaS JSON into a spec. +- **Randomness**: `@faker-js/faker` supplies ids and words in the factories. Never assert on a faker-generated value — capture it in a variable and assert against that variable. +- **Integration tests**: `integrationtests/` runs against a real CaaS tenant using `integrationtests/.env` (template in the same folder), via `CaasTestingClient` from `integrationtests/utils.ts`. Each test file generates a random `projectID` for isolation and deletes its collection afterwards. **Never run them unless explicitly asked** — see `CLAUDE.md`. + +## Use the fixture factories, and add to them + +**Rule:** Build test input from `src/testutils/` factories. When a test needs a shape no factory covers, add or extend a factory rather than inlining a literal — and export it from `src/testutils/index.ts`. + +CaaS documents are deep and repetitive: a PageRef with a body, a section, and typed form data is fifty lines of literal. Inlined, it goes stale the moment `src/types.ts` changes, and it obscures the one field the test actually cares about. A factory takes the interesting field as a parameter and keeps the noise out of the assertion. + +**Smell:** a spec that opens with forty lines of `as any as CaaSApi_Section` before the first `expect`, when `createSection()` plus one field override would do. + +A factory parameter should make the *distinction under test* explicit. `createDatasetReference(id, remoteProjectId)` emitting a `url` only when a remote project is passed is the pattern to follow: local and remote fixtures differ in exactly the field the production code branches on. + +## Assert exact values, not lower bounds + +**Rule:** Default to `toEqual(expected)` and `toHaveLength(n)` for counts, sizes, and scalar results. `expect.any`, `toBeTruthy`, `not.toHaveLength(0)`, and `toBeGreaterThan(0)` are fallbacks, allowed only when the exact value genuinely cannot be computed from the fixture — and then justified inline. + +An exact assert is a specification. A lower bound tolerates regressions in silence: a batch count drifting from 5 to 1 still passes `toBeGreaterThan(0)`. In this codebase the fixture owns every input, so nearly every count is known at write time — including request counts, registered reference paths, and mapped array lengths. + +**Smell:** asserting `expect(fetchMock).toHaveBeenCalled()` for a change whose whole point was batching. Called once and called thirty times both pass; only the exact call count distinguishes them. + +Where a value is unordered rather than unknown, keep the assert exact on content and explicit about order: `toHaveLength(n)` plus `expect.arrayContaining([...])`. Do not silently downgrade to a partial match — object key iteration order is a real dependency and `toEqual` on a fixed array asserts it. (Registered reference paths are the live example: `mapPageRef` maps `children` before `data`, so the section path is registered before the page path.) + +## Assert on requests, not only on output + +**Rule:** For anything touching reference resolution, batching, locales, or remote projects, assert on the *calls* the mapper made — `remoteProject`, `locale`, and the id list of every `fetchByFilter` — in addition to the mapped result. + +The mapped output can be right for the wrong reason. Grouping bugs, duplicate fetches, and a locale silently taken from configuration instead of the reference URL all produce a correct-looking result while issuing the wrong requests; that is a performance and correctness regression the output cannot see. The request list is the only place where "one batch per (projectId, locale) pair" and "a shared reference is fetched once" are observable. + +**Smell:** a remote-reference test that asserts the resolved dataset appears at the right path and stops there. It passes whether the dataset was fetched once, twice, or in five separate single-id requests. + +### Pattern + +```typescript +const batches = (api.fetchByFilter as jest.Mock).mock.calls.map(([params]) => ({ + remoteProject: params.remoteProject, + locale: params.locale, + ids: params.filters[0].value, +})) + +expect(batches).toHaveLength(5) +expect(batches).toEqual( + expect.arrayContaining([ + { remoteProject: undefined, locale: 'de_DE', ids: [localDatasetId] }, + { remoteProject: 'media-project', locale: 'en_GB', ids: [mediaId] }, + ]) +) +``` + +Include `locale` in the projection even when the test is nominally about projects. Without it, the test still passes when grouping falls back to project id alone. + +## Pick inputs that prove the behavior + +**Rule:** Choose the input that *discriminates* the intended behavior from "happens to work by accident". A test passes the wrong way when the intended code path and the most plausible broken alternative produce the same observable result for your chosen input. + +This is about what you feed the system under test, not what you assert. Both failure modes look identical on a green bar. + +**Smell:** testing "references from another project are grouped separately" with a fixture whose only reference is remote. One group is the correct answer *and* the answer you get if grouping is ignored entirely. + +### Picking the discriminating input + +1. Name the two paths you must tell apart — usually the intended behavior versus the plausible bug (grouping ignored, trust check skipped, locale taken from config, dedup missing, filter inverted). +2. Pick an input where those paths differ observably. For grouping: one local reference plus remotes in at least two different projects. For dedup: the *same* reference registered from two different places. For the trust check: one URL on the configured origin and one on a foreign origin. +3. If no single input can discriminate, split into a positive and a negative case. Avoid the only-positive trap — "untrusted URLs are dropped" needs a test with an untrusted URL. +4. Sanity check by mentally deleting the production code path. If the assert still passes, the input is not discriminating. + +Recurring discrimination cases here: local versus remote references (include both); same-project-different-locale (the case that catches grouping by project id alone); a reference with no URL alongside one with a URL; preview versus release; a component type the mapper does not know mixed in with ones it does. + +## Cover both API implementations + +**Rule:** A change to a fetch method needs test coverage on both sides — `FSXARemoteApi.spec.ts` for URL construction, query translation, and response handling, and `FSXAProxyApi.spec.ts` for request-body serialization. If it adds or changes a route parameter, extend `src/integrations/express.spec.ts` and `parameterValidation.spec.ts` too. + +The two implementations satisfy one interface but share no code. Remote-only coverage lets a parameter that never reaches the proxy body ship green: proxy-mode consumers then see the parameter silently ignored, with nothing failing anywhere. + +**Smell:** a new `fetchByFilter` option with tests only in `FSXARemoteApi.spec.ts`. The proxy still posts the old body shape and no test notices. + +## Type errors are not caught by the test run + +**Rule:** Run `npx tsc --noEmit` after every change, before reporting it done. Do not treat a green `npm test` as verification on its own. + +`ts-jest` only compiles the files a test actually imports, and `noEmitOnError` is `false`. A type error in an unimported file — or in a declaration only the build's `tsc` pass sees — survives a fully green test run and then breaks `npm run build` in CI or, worse, the published `.d.ts` files. + +**Smell:** widening a signature in `src/modules/` and forgetting the mirrored declaration in `src/types.ts` (the `CustomMapper` utils are declared separately there). Tests pass; `npm run build:types` fails. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..6ff6651 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,57 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. It is an index — detailed reference lives in the linked documents. + +## Architecture + +Read [`.claude/harness/architecture.md`](./.claude/harness/architecture.md) before searching for any code — it tells you where a feature lives, how data flows through reference resolution, and how the two API implementations relate, so you know where to look. + +## Coding Guidelines + +Read [`.claude/harness/coding-guidelines.md`](./.claude/harness/coding-guidelines.md) before writing or changing any code, production or test. Its rules are mandatory — obey them both at write time and at review time. + +## Testing Guidelines + +Read [`.claude/harness/testing-guidelines.md`](./.claude/harness/testing-guidelines.md) before writing or modifying tests. Its rules are mandatory and additive on top of the coding guidelines — obey them both at write time and at review time. + +## Build & Development Commands + +```bash +# Unit tests — the default signal. Runs only ./src, no network, no credentials needed. +npm test + +# Run a single spec file, or a single test by name +npx jest src/modules/CaaSMapper.spec.ts +npx jest src/modules/CaaSMapper.spec.ts -t 'should resolve remote references' + +# Type check without emitting. Run this after every change — jest (ts-jest) does not +# fail on type errors in files no test imports. +npx tsc --noEmit + +# Build the library (types via tsc, bundles via esbuild) +npm run build + +# Local install artifact for testing against a consuming app +npm run build:local + +# Integration tests — NEVER run unless explicitly asked ("run the tests" does NOT authorize it) +npm run test:integration +``` + +The restriction on `npm run test:integration` is not stylistic: those tests read `integrationtests/.env` and write to, then delete from, a real CaaS tenant. Without that file they fail with confusing auth errors; with it they mutate a shared external system. Ask before running them, and state explicitly when you did not run them rather than implying full verification. + +`npm run test:prod` is broken — it calls `npm run lint`, which does not exist in `package.json`. Do not use it and do not "fix" it by inventing a lint script; use `npm test` plus `npx tsc --noEmit`. + +## Formatting + +Do not run `npx prettier --write` over the repository or over whole files you did not otherwise change. The committed code was formatted by Prettier 2 (`trailingComma: "es5"`); the installed Prettier 3 defaults to `"all"`, so a bare run reformats hundreds of untouched lines and buries the real diff. If you must format, scope it to the files you changed and pass the old default explicitly: + +```bash +npx prettier --write --trailing-comma es5 src/modules/CaaSMapper.ts +``` + +`lint-staged` runs `prettier --write` on staged `{src,test}/**/*.ts` at commit time, so files outside that glob (`README.md`, `integrationtests/`) are not Prettier-clean on master and must not be normalized as a side effect of an unrelated change. + +## Commits & releases + +Commit messages follow Conventional Commits and are validated by commitlint. `release-it` derives the next version and the changelog from them, so the commit type is a release decision: `fix` ships a patch, `feat` a minor, a `BREAKING CHANGE:` footer a major. Never add that footer casually — see the public API rules in the coding guidelines. diff --git a/README.md b/README.md index d5f756d..9ed83d1 100644 --- a/README.md +++ b/README.md @@ -70,22 +70,61 @@ const config = { } ``` -You can also include remote projects if you want to use remote media. - -> **_Attention_**
-> Currently the Content API can only work with the configured language of the remote media project. -> You also require a configured CAAS API key with read permissions to both projects. -> -> For this you can add another parameter called `remotes` to the config. This parameter expects an object, which requires a unique name as key and an object as value. This object must have two keys. On the one hand an `id` with the project id as the value and on the other the `locale` with the locale abbreviation. For example: +Media and datasets from other projects are resolved through the `remotes` configuration. It maps a free name to the project's `id` and to the locale that project's content is read in. You need a CAAS API key with read permissions for every project you reference — the same key is sent with every request, there is no per-project key. ```typescript const config = { ... - remotes: { media: { id: 'xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx', locale: 'en_GB' } }, + remotes: { + media1: { id: 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', locale: 'en_GB' }, + media2: { id: 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb', locale: 'de_DE' }, + datasets1: { id: 'cccccccc-cccc-cccc-cccc-cccccccccccc', locale: 'en_US' }, + datasets2: { id: 'dddddddd-dddd-dddd-dddd-dddddddddddd', useSourceLocale: true } + }, ... } ``` +### Resolving references across projects + +A reference in the CaaS carries the URL of the document it points at. The Content API reads the **project** from that URL and looks it up in your `remotes` configuration. The **locale** never comes from the URL. + +That split is deliberate. An editor cannot express which language of a referenced dataset or medium should be delivered, and the URLs FirstSpirit writes are not consistent about it: a project maintained only in `de_DE` is referenced from an `en_GB` page with URLs that sometimes say `de_DE` and sometimes `en_GB`. Which language a remote project is read in is therefore an application decision, and the application states it in `remotes`. + +#### Which locale a reference resolves in + +| Target project | Locale used | +|---|---| +| Your own project, not configured in `remotes` | the locale of the requested element | +| A configured remote | the `locale` configured for it | +| A configured remote with `useSourceLocale: true` | the locale of the requested element | +| Anything else | the reference is not resolved | + +`useSourceLocale: true` makes the configured `locale` irrelevant and may be used without one. It fits a remote project maintained in the same languages as your own: a page requested in `en_GB` then resolves its references into that project in `en_GB` too. + +"The locale of the requested element" is the locale you passed to `fetchElement` or `fetchByFilter`. It stays the same for the whole resolution tree, including references found inside an already resolved remote dataset. + +This applies to media references (`FS_REFERENCE`, `CMS_INPUT_IMAGEMAP`) and to dataset references (`FS_DATASET`, and `FS_INDEX` with the dataset data access plugin) alike. + +#### Projects that are not configured + +A reference into a project that is neither your own nor configured is **not fetched**. Its placeholder stays in the payload and the Content API logs a warning naming the project id. Add the project to `remotes` to resolve those references. + +A reference whose URL points at a different CaaS instance or tenant takes the same path: its project is not configured, so it is not fetched. Requests are always built from your `caasURL`, `tenantID` and content mode — a URL in your content can never direct a request at another host, and a release delivery cannot serve preview content. + +#### Configuration rules + +- Every entry needs an `id`. Two entries must not share one. +- Every entry needs either a `locale` or `useSourceLocale: true`. +- An entry may name your own project. References into it then resolve in that configured locale instead of the locale of the requested element. +- The name (the key) is what you pass as the `remoteProject` parameter of `fetchElement` and as the `remote` query parameter of the endpoint integration. + +#### Batching + +References are fetched per distinct project/locale pair: one CaaS request per pair, and more only when a pair holds more references than fit into a single batch of 30. Since a project resolves in exactly one locale, that is at most one batch per referenced project per 30 references. + +Datasets selected by a `Content2Section` are queried rather than referenced and expose no identifier to resolve, so cross-project datasets are not supported there. + The log level can be: `0` = Info `1` = Log diff --git a/dev/.env.template b/dev/.env.template index 9ac61b9..4110ec4 100644 --- a/dev/.env.template +++ b/dev/.env.template @@ -4,3 +4,6 @@ API_NAVIGATION_SERVICE= API_CAAS= API_PROJECT_ID= API_TENANT_ID= +; JSON map of remote projects, e.g. +; {"myRemote":{"id":"","locale":"en_GB"},"otherRemote":{"id":"","useSourceLocale":true}} +API_REMOTES= diff --git a/integrationtests/FSXAProxyApiRemoteProjects.test.ts b/integrationtests/FSXAProxyApiRemoteProjects.test.ts index 6b6e4ef..2cc9d3b 100644 --- a/integrationtests/FSXAProxyApiRemoteProjects.test.ts +++ b/integrationtests/FSXAProxyApiRemoteProjects.test.ts @@ -9,9 +9,10 @@ import { FSXAProxyApi, LogLevel, Page, + RemoteProjectConfiguration, } from '../src' import { default as expressIntegration } from '../src/integrations/express' -import { FSXARemoteApi } from '../src/modules/FSXARemoteApi' +import { FSXARemoteApi } from '../src' import { CaasTestingClient, closeServer, retryAsync, waitUntilPreconditionMet, TEST_TIMEOUTS } from './utils' import { Server } from 'http' import { faker } from '@faker-js/faker' @@ -58,7 +59,11 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { async function init( remoteProjectId: string, remoteProjectLocale: string, - differentMediaIds: boolean = false + differentMediaIds: boolean = false, + configuredRemotes: RemoteProjectConfiguration = { + media: { id: remoteProjectId, locale: remoteProjectLocale }, + }, + referenceUrlLocale: string = remoteProjectLocale ) { let remoteApi = new FSXARemoteApi({ apikey: INTEGRATION_TEST_API_KEY!, @@ -68,9 +73,7 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { 'https://your-navigationservice.e-spirit.cloud/navigation'!, projectID: randomId1, tenantID: tenantID, - remotes: { - media: { id: remoteProjectId, locale: remoteProjectLocale }, - }, + remotes: configuredRemotes, logLevel: LogLevel.INFO, enableEventStream: false, maxReferenceDepth: 10, @@ -95,14 +98,16 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { await prepareDataInCaas( remoteProjectId, remoteProjectLocale, - differentMediaIds + differentMediaIds, + referenceUrlLocale ) } async function prepareDataInCaas( remoteProjectId: string, remoteProjectLocale: string, - differentMediaIds: boolean + differentMediaIds: boolean, + referenceUrlLocale: string ) { const mediaId = faker.string.uuid() @@ -116,19 +121,30 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { pageRef = createPageRef([createPageRefBody()]) + const referenceUrlOptions = { + baseUrl: INTEGRATION_TEST_CAAS!, + tenantId: tenantID, + locale: referenceUrlLocale, + contentMode: FSXAContentMode.PREVIEW, + } + const pictureLocal = createMediaPictureReference(mediaId) const pictureRemote = createMediaPictureReference( remoteMediaId, - remoteProjectId + remoteProjectId, + referenceUrlOptions ) - // create dataset const datasetId = faker.string.uuid() dataset = createDataset(datasetId) - const datasetReference = createDatasetReference(datasetId) remoteMedia.metaFormData = { - md_dataset: datasetReference, + md_dataset: createDatasetReference(datasetId), } + const remoteDatasetReference = createDatasetReference( + datasetId, + remoteProjectId, + referenceUrlOptions + ) await caasClient.addItemsToCollection([localMedia], projectLocale) @@ -144,6 +160,7 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { pageRef.page.formData = { pt_pictureLocal: pictureLocal, pt_pictureRemote: pictureRemote, + pt_datasetRemote: remoteDatasetReference, } await caasClient.addItemsToCollection([pageRef], projectLocale) @@ -273,4 +290,66 @@ describe('FSXAProxyAPIRemoteProjects should resolve references', () => { ) }, { maxRetries: 5, delayMs: 1000 }) }, TEST_TIMEOUTS.LONG) + + it('should leave references into an unconfigured project unresolved and still deliver the page', async () => { + await init(randomId2, 'en_GB', true, {}) + + await retryAsync(async () => { + const res: Page = await proxyAPI.fetchElement({ + id: pageRef.identifier, + locale: 'de_DE', + }) + expect(typeof res.data.pt_datasetRemote).toEqual('string') + expect(res.data.pt_datasetRemote).toContain('REFERENCED-REMOTE-ITEM') + expect(typeof res.data.pt_pictureRemote).toEqual('string') + expect(res.data.pt_pictureRemote).toContain('REFERENCED-REMOTE-ITEM') + expect(localMedia.description).toEqual(res.data.pt_pictureLocal.description) + }, { maxRetries: 5, delayMs: 1000 }) + }, TEST_TIMEOUTS.LONG) + + it('should use the configured locale even when the reference url names another one', async () => { + // the remote project holds its documents in de_DE while the reference urls + // claim en_GB - the configuration wins + await init(randomId2, 'de_DE', true, { media: { id: randomId2, locale: 'de_DE' } }, 'en_GB') + + await retryAsync(async () => { + const res: Page = await proxyAPI.fetchElement({ + id: pageRef.identifier, + locale: 'de_DE', + }) + expect(remoteMedia.description).toEqual( + res.data.pt_pictureRemote.description + ) + expect(res.data.pt_datasetRemote.id).toEqual(dataset.identifier) + }, { maxRetries: 5, delayMs: 1000 }) + }, TEST_TIMEOUTS.LONG) + + it('should resolve a useSourceLocale project in the locale of the requested element', async () => { + await init(randomId2, 'de_DE', true, { media: { id: randomId2, useSourceLocale: true } }, 'en_GB') + + await retryAsync(async () => { + const res: Page = await proxyAPI.fetchElement({ + id: pageRef.identifier, + locale: 'de_DE', + }) + expect(remoteMedia.description).toEqual( + res.data.pt_pictureRemote.description + ) + expect(res.data.pt_datasetRemote.id).toEqual(dataset.identifier) + }, { maxRetries: 5, delayMs: 1000 }) + }, TEST_TIMEOUTS.LONG) + + it('Dataset references on a local page should be resolved from the remote project', async () => { + await init(randomId2, 'en_GB') + + await retryAsync(async () => { + const res: Page = await proxyAPI.fetchElement({ + id: pageRef.identifier, + locale: 'de_DE', + }) + expect(typeof res.data.pt_datasetRemote).toEqual('object') + expect(res.data.pt_datasetRemote.type).toEqual('Dataset') + expect(res.data.pt_datasetRemote.id).toEqual(dataset.identifier) + }, { maxRetries: 5, delayMs: 1000 }) + }, TEST_TIMEOUTS.LONG) }) diff --git a/src/enums.ts b/src/enums.ts index 2a18930..4e45dc6 100644 --- a/src/enums.ts +++ b/src/enums.ts @@ -13,6 +13,7 @@ export enum FSXAApiErrors { NOT_FOUND = 'Resource could not be found', MISSING_REMOTE_LOCALE = 'The specified remote project did not include a locale', MISSING_REMOTE_ID = 'The specified remote project did not include an id', + DUPLICATE_REMOTE_ID = 'Each remote project must be configured with a distinct id. [remotes]', INVALID_LOCALE = 'The specified locale is not valid, locale needs to be a string of format "xx_YY"', } diff --git a/src/modules/CaaSMapper.spec.ts b/src/modules/CaaSMapper.spec.ts index 75aaefa..1385fbd 100644 --- a/src/modules/CaaSMapper.spec.ts +++ b/src/modules/CaaSMapper.spec.ts @@ -30,25 +30,24 @@ import { RichTextElement, FetchByFilterParams, Permission, + RemoteProjectConfiguration, } from '../types' -import { createNumberEntry } from '../testutils/createNumberEntry' -import { createPageRef } from '../testutils/createPageRef' -import { createSection } from '../testutils/createSection' +import { createNumberEntry } from '../testutils' +import { createPageRef } from '../testutils' +import { createPageRefBody } from '../testutils' +import { createSection } from '../testutils' import { createDataEntry, createMediaPictureReference, mockPermissionActivity, mockPermissionGroup, -} from '../testutils/createDataEntry' -import { createProjectProperties } from '../testutils/createProjectProperties' -import { createGCAPage } from '../testutils/createGCAPage' -import { - createDataset, - createDatasetReference, -} from '../testutils/createDataset' -import { createMediaPicture } from '../testutils/createMediaPicture' -import { createMediaFile } from '../testutils/createMediaFile' -import { createImageMap } from '../testutils/createImageMap' +} from '../testutils' +import { createProjectProperties } from '../testutils' +import { createGCAPage } from '../testutils' +import { createDataset, createDatasetReference } from '../testutils' +import { createMediaPicture } from '../testutils' +import { createMediaFile } from '../testutils' +import { createImageMap } from '../testutils' import { CaaSApi_CMSInputPermission, CaaSAPI_PermissionGroup, @@ -56,7 +55,8 @@ import { Option, Reference, } from '..' -import { createFetchResponse } from '../testutils/createFetchResponse' +import { createFetchResponse } from '../testutils' +import { createReferenceUrl } from '../testutils' import { LoggerChalked } from './LoggerChalked' jest.mock('./FSXARemoteApi') @@ -64,8 +64,18 @@ jest.mock('./FSXARemoteApi') describe('CaaSMapper', () => { const createPath = () => [faker.lorem.word(), faker.lorem.word()] const createLogger = () => new LoggerChalked(LogLevel.NONE, 'Querybuilder') - const createApi = () => - jest.mocked(new (FSXARemoteApi as any)()) + const createApi = ( + remotes: RemoteProjectConfiguration = {}, + projectID = 'local-project' + ) => { + const api = jest.mocked(new (FSXARemoteApi as any)()) + Object.defineProperty(api, 'projectID', { value: projectID }) + Object.defineProperty(api, 'remotes', { value: remotes }) + Object.defineProperty(api, 'getRemoteConfigById', { + value: (id: string) => Object.values(remotes).find((e) => e.id === id), + }) + return api + } const createMapper = () => new CaaSMapper(createApi(), 'de', {}, createLogger()) let remoteProjectLocale: string | undefined = undefined @@ -101,40 +111,64 @@ describe('CaaSMapper', () => { [`${refId}.${locale}`]: [path, path2], }) }) - it('should register a remote reference and return its remote reference key', () => { - const remotes = { someName: { id: 'remoteId', locale: 'de' } } + it('should register a remote reference under its project and locale', () => { const api = createApi() - api.remotes = remotes - const mapper = new CaaSMapper(api, 'de', {}, createLogger()) + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) const refId = faker.lorem.word() const path = createPath() - const item = mapper.registerReferencedItem(refId, path, 'remoteId') + const item = mapper.registerReferencedItem( + refId, + path, + 'remote-project', + undefined, + 'en_GB' + ) + + expect(mapper._referencedItems).toEqual({}) expect(mapper._remoteReferences).toEqual({ - remoteId: { - [`${remotes.someName.id}#${refId}.${remotes.someName.locale}`]: [ - path, - ], + 'remote-project#en_GB': { + projectId: 'remote-project', + locale: 'en_GB', + references: { [`remote-project#${refId}.en_GB`]: [path] }, }, }) - expect(mapper._referencedItems).toEqual({}) expect(item).toEqual( - `[REFERENCED-REMOTE-ITEM-${remotes.someName.id}#${refId}.${remotes.someName.locale}]` + `[REFERENCED-REMOTE-ITEM-remote-project#${refId}.en_GB]` ) }) - it('should register a non-remote item if the remote project was not found', () => { + + it('should keep two locales of the same project in separate groups', () => { const api = createApi() - const locale = 'de_DE' - const mapper = new CaaSMapper(api, locale, {}, createLogger()) + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) const refId = faker.lorem.word() - const path = createPath() - const item = mapper.registerReferencedItem(refId, path, 'remoteId') - expect(mapper._remoteReferences).toEqual({}) - expect(mapper._referencedItems).toEqual({ - [`${refId}.${locale}`]: [path], - }) - expect(item).toEqual(`[REFERENCED-ITEM-${refId}.${locale}]`) + mapper.registerReferencedItem(refId, ['a'], 'p', undefined, 'en_GB') + mapper.registerReferencedItem(refId, ['b'], 'p', undefined, 'de_DE') + + expect(Object.keys(mapper._remoteReferences).sort()).toEqual([ + 'p#de_DE', + 'p#en_GB', + ]) + }) + + it('should register into any project it is given, without consulting the configuration', () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) + const refId = faker.lorem.word() + + const item = mapper.registerReferencedItem( + refId, + createPath(), + 'unconfigured-project', + undefined, + 'en_GB' + ) + + expect(item).not.toBeNull() + expect( + mapper._remoteReferences['unconfigured-project#en_GB'] + ).toBeDefined() }) }) @@ -182,6 +216,199 @@ describe('CaaSMapper', () => { }) }) + describe('resolveReferenceTarget', () => { + it('should use the configured locale, not the one in the url', () => { + const api = createApi({ + media: { id: 'remote-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ + projectId: 'remote-project', + locale: 'en_GB', + }) + + expect(mapper.resolveReferenceTarget(url, undefined, undefined)).toEqual({ + kind: 'remote', + projectId: 'remote-project', + locale: 'de_DE', + }) + }) + + it('should use the source locale when useSourceLocale is configured', () => { + const api = createApi({ + data: { id: 'remote-project', useSourceLocale: true }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ + projectId: 'remote-project', + locale: 'de_DE', + }) + + expect(mapper.resolveReferenceTarget(url, undefined, undefined)).toEqual({ + kind: 'remote', + projectId: 'remote-project', + locale: 'en_GB', + }) + }) + + it('should keep the source locale when the mapped item locale changed', () => { + const api = createApi({ + data: { id: 'remote-project', useSourceLocale: true }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + mapper.locale = 'fr_FR' + const url = createReferenceUrl({ projectId: 'remote-project' }) + + expect(mapper.resolveReferenceTarget(url, undefined, undefined)).toEqual({ + kind: 'remote', + projectId: 'remote-project', + locale: 'en_GB', + }) + }) + + it('should treat the own project as local whatever the url locale says', () => { + const mapper = new CaaSMapper(createApi(), 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ + projectId: 'local-project', + locale: 'de_DE', + }) + + expect(mapper.resolveReferenceTarget(url, undefined, undefined)).toEqual({ + kind: 'local', + }) + }) + + it('should honour a remotes entry configured for the own project', () => { + const api = createApi({ self: { id: 'local-project', locale: 'de_DE' } }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ projectId: 'local-project' }) + + expect(mapper.resolveReferenceTarget(url, undefined, undefined)).toEqual({ + kind: 'remote', + projectId: 'local-project', + locale: 'de_DE', + }) + }) + + it('should report a project that is not configured', () => { + const api = createApi({ + media: { id: 'configured-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ projectId: 'other-project' }) + + expect(mapper.resolveReferenceTarget(url, undefined, undefined)).toEqual({ + kind: 'unresolvable', + projectId: 'other-project', + }) + }) + + it('should fall back to the remoteProject field and then to the surrounding document', () => { + const api = createApi({ + media: { id: 'field-project', locale: 'de_DE' }, + data: { id: 'inherited-project', locale: 'fr_FR' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + + expect( + mapper.resolveReferenceTarget( + undefined, + 'field-project', + 'inherited-project' + ) + ).toEqual({ + kind: 'remote', + projectId: 'field-project', + locale: 'de_DE', + }) + expect( + mapper.resolveReferenceTarget(undefined, undefined, 'inherited-project') + ).toEqual({ + kind: 'remote', + projectId: 'inherited-project', + locale: 'fr_FR', + }) + expect( + mapper.resolveReferenceTarget('not-a-caas-url', undefined, undefined) + ).toEqual({ kind: 'local' }) + }) + }) + + describe('registerReference', () => { + it('should register a remote reference under the configured locale', () => { + const api = createApi({ + media: { id: 'remote-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ + projectId: 'remote-project', + locale: 'en_GB', + }) + + const placeholder = mapper.registerReference('some-uuid', ['a'], url, {}) + + expect(mapper._remoteReferences).toEqual({ + 'remote-project#de_DE': { + projectId: 'remote-project', + locale: 'de_DE', + references: { 'remote-project#some-uuid.de_DE': [['a']] }, + }, + }) + expect(placeholder).toEqual( + '[REFERENCED-REMOTE-ITEM-remote-project#some-uuid.de_DE]' + ) + }) + + it('should register a reference into the own project locally', () => { + const mapper = new CaaSMapper(createApi(), 'en_GB', {}, createLogger()) + const url = createReferenceUrl({ + projectId: 'local-project', + locale: 'de_DE', + }) + + const placeholder = mapper.registerReference('some-uuid', ['a'], url, {}) + + expect(mapper._remoteReferences).toEqual({}) + expect(mapper._referencedItems).toEqual({ 'some-uuid.en_GB': [['a']] }) + expect(placeholder).toEqual('[REFERENCED-ITEM-some-uuid.en_GB]') + }) + + it('should leave a reference into an unconfigured project unresolved and name the project', () => { + const api = createApi({ + media: { id: 'configured-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const warn = jest.spyOn(mapper.logger, 'warn') + const url = createReferenceUrl({ projectId: 'other-project' }) + + const placeholder = mapper.registerReference('some-uuid', ['a'], url, {}) + + expect(placeholder).toEqual( + '[REFERENCED-REMOTE-ITEM-other-project#some-uuid.en_GB]' + ) + expect(mapper._remoteReferences).toEqual({}) + expect(mapper._referencedItems).toEqual({}) + expect(warn.mock.calls[0][0]).toContain('other-project') + expect(warn.mock.calls[0][0]).toContain('remotes') + }) + + it('should keep the image map resolution in an unresolved placeholder', () => { + const mapper = new CaaSMapper(createApi(), 'en_GB', {}, createLogger()) + + const placeholder = mapper.registerReference( + 'some-uuid', + ['a'], + createReferenceUrl({ projectId: 'other-project' }), + {}, + { imageMapResolution: 'RES' } + ) + + expect(placeholder).toEqual( + 'IMAGEMAP___RES___other-project#some-uuid.en_GB' + ) + }) + }) + describe('mapDataEntry', () => { it('should execute the custom mapper and return its result if given', async () => { const customMapper: CustomMapper = jest @@ -603,13 +830,36 @@ describe('CaaSMapper', () => { } }) }) + it('should register the background medium against the project named in its url', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) + mapper.registerReference = jest.fn().mockReturnValue('[REF]') + const imageMap = createImageMap() + imageMap.value.media.url = createReferenceUrl({ + projectId: 'remote-project', + documentId: imageMap.value.media.identifier, + }) + const path = createPath() + + await mapper.mapImageMap(imageMap, path) + + expect(mapper.registerReference).toHaveBeenCalledWith( + imageMap.value.media.identifier, + [...path, 'media'], + imageMap.value.media.url, + { projectId: undefined }, + { + referencedProject: imageMap.value.media.remoteProject, + imageMapResolution: imageMap.value.resolution.uid, + } + ) + }) it('should work with nested formData image maps', async () => { const mapper = new CaaSMapper(createApi(), 'de', {}, createLogger()) const path = createPath() const mock = jest.spyOn(mapper, 'mapDataEntry') const entry = createImageMap() const childEntry = createImageMap() - const childEntryMediaId = `${childEntry.value.media.identifier}.de` entry.value.areas[0].link!.formData = { childEntry } await mapper.mapDataEntry(entry, path) expect(mock.mock.calls[0][0]).toEqual(entry) @@ -686,6 +936,98 @@ describe('CaaSMapper', () => { ) expect(mapper.registerReferencedItem).toHaveBeenCalled() }) + it('should register a local DatasetReference without a remote project', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) + mapper.registerReference = jest.fn().mockReturnValue('[REF]') + const entry = createDatasetReference() + const path = createPath() + + await expect(mapper.mapDataEntry(entry, path)).resolves.toBe('[REF]') + expect(mapper.registerReference).toHaveBeenCalledWith( + (entry.value as any).target.identifier, + path, + undefined, + { projectId: undefined } + ) + }) + + it('should register a DatasetReference in the locale configured for its project', async () => { + const api = createApi({ + data: { id: 'remote-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const entry = createDatasetReference(undefined, 'remote-project', { + locale: 'fr_FR', + }) + const identifier = (entry.value as any).target.identifier + const path = createPath() + + await expect(mapper.mapDataEntry(entry, path)).resolves.toEqual( + `[REFERENCED-REMOTE-ITEM-remote-project#${identifier}.de_DE]` + ) + expect(mapper._remoteReferences).toEqual({ + 'remote-project#de_DE': { + projectId: 'remote-project', + locale: 'de_DE', + references: { [`remote-project#${identifier}.de_DE`]: [path] }, + }, + }) + }) + + it('should prefer the project from the url over the surrounding document', async () => { + const api = createApi({ + data: { id: 'remote-project', locale: 'de_DE' }, + inherited: { id: 'inherited-project', locale: 'fr_FR' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const entry = createDatasetReference(undefined, 'remote-project') + + await mapper.mapDataEntry( + entry, + createPath(), + 'fr_FR', + 'inherited-project' + ) + + expect(Object.keys(mapper._remoteReferences)).toEqual([ + 'remote-project#de_DE', + ]) + }) + + it('should fall back to the surrounding document when the url is unusable', async () => { + const api = createApi({ + inherited: { id: 'inherited-project', locale: 'fr_FR' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const entry = createDatasetReference() + ;(entry.value as any).url = 'not-a-caas-url' + + await mapper.mapDataEntry( + entry, + createPath(), + 'fr_FR', + 'inherited-project' + ) + + expect(Object.keys(mapper._remoteReferences)).toEqual([ + 'inherited-project#fr_FR', + ]) + }) + + it('should leave a DatasetReference into an unconfigured project unresolved', async () => { + const api = createApi() + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const entry = createDatasetReference(undefined, 'other-project') + const identifier = (entry.value as any).target.identifier + + await expect(mapper.mapDataEntry(entry, createPath())).resolves.toEqual( + `[REFERENCED-REMOTE-ITEM-other-project#${identifier}.en_GB]` + ) + expect(mapper._remoteReferences).toEqual({}) + expect(mapper._referencedItems).toEqual({}) + }) + it('should return null if the entry is corrupted', async () => { const api = createApi() const mapper = new CaaSMapper(api, 'de', {}, createLogger()) @@ -926,31 +1268,77 @@ describe('CaaSMapper', () => { mapper.mapDataEntry(entry, createPath()) ).resolves.toBeNull() }) - it('should register a reference on Media entries', async () => { + it('should register a local Media reference without a remote project', async () => { const api = createApi() - const mapper = new CaaSMapper(api, 'de', {}, createLogger()) - mapper.registerReferencedItem = jest.fn().mockReturnValue('[REF]') + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) + mapper.registerReference = jest.fn().mockReturnValue('[REF]') const path = createPath() - const entry: CaaSApi_FSReference = { - name: faker.lorem.word(), - value: { - fsType: 'Media', - name: faker.lorem.word(), - identifier: faker.string.uuid(), - uid: faker.lorem.word(), - uidType: 'MEDIASTORE_LEAF', - url: faker.lorem.word(), - mediaType: 'PICTURE', - remoteProject: 'remote-project', - }, - fsType: 'FS_REFERENCE', - } + const entry = createMediaPictureReference() + await expect(mapper.mapDataEntry(entry, path)).resolves.toEqual('[REF]') - expect(mapper.registerReferencedItem).toHaveBeenCalledWith( + expect(mapper.registerReference).toHaveBeenCalledWith( entry.value!.identifier, path, - entry.value!.remoteProject + entry.value!.url, + { projectId: undefined }, + { referencedProject: undefined } + ) + }) + + it('should register a Media reference in the locale configured for the project named in its url', async () => { + const api = createApi({ + media: { id: 'remote-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const path = createPath() + const entry = createMediaPictureReference(undefined, 'remote-project', { + locale: 'fr_FR', + }) + + await mapper.mapDataEntry(entry, path) + + expect(mapper._remoteReferences['remote-project#de_DE']).toEqual({ + projectId: 'remote-project', + locale: 'de_DE', + references: { + [`remote-project#${entry.value!.identifier}.de_DE`]: [path], + }, + }) + }) + + it('should use the remoteProject field when the reference has no CaaS document url', async () => { + const api = createApi({ + media: { id: 'field-project', locale: 'de_DE' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const path = createPath() + const entry = createMediaPictureReference() + entry.value!.remoteProject = 'field-project' + + await expect(mapper.mapDataEntry(entry, path)).resolves.toEqual( + `[REFERENCED-REMOTE-ITEM-field-project#${ + entry.value!.identifier + }.de_DE]` ) + expect(Object.keys(mapper._remoteReferences)).toEqual([ + 'field-project#de_DE', + ]) + }) + + it('should prefer the project from the url over the remoteProject field', async () => { + const api = createApi({ + url: { id: 'url-project', locale: 'de_DE' }, + field: { id: 'field-project', locale: 'fr_FR' }, + }) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + const entry = createMediaPictureReference(undefined, 'url-project') + entry.value!.remoteProject = 'field-project' + + await mapper.mapDataEntry(entry, createPath()) + + expect(Object.keys(mapper._remoteReferences)).toEqual([ + 'url-project#de_DE', + ]) }) it('should return null and not register a reference on Media entries with a null identifier (broken reference)', async () => { const api = createApi() @@ -971,9 +1359,7 @@ describe('CaaSMapper', () => { } as any, fsType: 'FS_REFERENCE', } - await expect( - mapper.mapDataEntry(entry, path) - ).resolves.toBeNull() + await expect(mapper.mapDataEntry(entry, path)).resolves.toBeNull() expect(mapper.registerReferencedItem).not.toHaveBeenCalled() }) it('should handle PageRef & GCAPage separately', async () => { @@ -1058,7 +1444,7 @@ describe('CaaSMapper', () => { const api = createApi() const mapper = new CaaSMapper(api, 'de', {}, createLogger()) const path = createPath() - mapper.registerReferencedItem = jest + mapper.registerReference = jest .fn() .mockImplementation(($) => `REF-${$}`) const entry: CaaSApi_FSIndex = { @@ -1081,11 +1467,65 @@ describe('CaaSMapper', () => { await expect(mapper.mapDataEntry(entry, path)).resolves.toEqual([ 'REF-target-id', ]) - expect(mapper.registerReferencedItem).toHaveBeenCalledTimes(1) - expect(mapper.registerReferencedItem).toHaveBeenCalledWith( + expect(mapper.registerReference).toHaveBeenCalledTimes(1) + expect(mapper.registerReference).toHaveBeenCalledWith( 'target-id', [...path, 0], - remoteProjectId + undefined, + { projectId: undefined } + ) + }) + it('should register each record against the project named in its own url', async () => { + const api = createApi() + Object.defineProperty(api, 'projectID', { value: 'local-project' }) + const mapper = new CaaSMapper(api, 'de_DE', {}, createLogger()) + const path = createPath() + mapper.registerReference = jest + .fn() + .mockImplementation(($) => `REF-${$}`) + const entry: CaaSApi_FSIndex = { + name: faker.lorem.word(), + dapType: 'DatasetDataAccessPlugin', + value: [ + { + value: { + fsType: 'DatasetReference', + target: { identifier: 'remote-target-id' }, + url: createReferenceUrl({ projectId: 'remote-project' }), + }, + fsType: 'Record', + identifier: 'remote-record-id', + }, + { + value: { + fsType: 'DatasetReference', + target: { identifier: 'local-target-id' }, + url: createReferenceUrl({ projectId: 'local-project' }), + }, + fsType: 'Record', + identifier: 'local-record-id', + }, + ] as any, + fsType: 'FS_INDEX', + } + + await expect(mapper.mapDataEntry(entry, path)).resolves.toEqual([ + 'REF-remote-target-id', + 'REF-local-target-id', + ]) + expect(mapper.registerReference).toHaveBeenNthCalledWith( + 1, + 'remote-target-id', + [...path, 0], + (entry.value as any)[0].value.url, + { projectId: undefined } + ) + expect(mapper.registerReference).toHaveBeenNthCalledWith( + 2, + 'local-target-id', + [...path, 1], + (entry.value as any)[1].value.url, + { projectId: undefined } ) }) it("should return entries which are not of dapType 'DatasetDataAccessPlugin' as-is", async () => { @@ -1782,56 +2222,48 @@ describe('CaaSMapper', () => { }) describe('resolveAllReferences', () => { - it('should call resolveReferencesPerProject for the current project', async () => { + it('should call resolveReferencesForGroup for the local group', async () => { const mapper = new CaaSMapper(createApi(), 'de', {}, createLogger()) - mapper.resolveReferencesPerProject = jest.fn() + mapper.resolveReferencesForGroup = jest.fn() await mapper.resolveAllReferences() - expect(mapper.resolveReferencesPerProject).toHaveBeenCalledWith( - remoteProjectId, + expect(mapper.resolveReferencesForGroup).toHaveBeenCalledWith( + undefined, undefined ) }) - it('should call resolveReferencesPerProject for all remote projects', async () => { - const api = createApi() - api.remotes = { - 'remote-id1': { id: 'remote-id1', locale: 'de' }, - 'remote-id2': { id: 'remote-id2', locale: 'de' }, - 'remote-id3': { id: 'remote-id3', locale: 'de' }, + it('should call resolveReferencesForGroup for every project and locale group', async () => { + const mapper = new CaaSMapper(createApi(), 'de', {}, createLogger()) + mapper._remoteReferences = { + 'p1#en_GB': { projectId: 'p1', locale: 'en_GB', references: {} }, + 'p1#de_DE': { projectId: 'p1', locale: 'de_DE', references: {} }, } - const mapper = new CaaSMapper(api, 'de', {}, createLogger()) - mapper.resolveReferencesPerProject = jest.fn() - mapper.registerReferencedItem('id1', [], 'remote-id1') - mapper.registerReferencedItem('id2', [], 'remote-id2') - mapper.registerReferencedItem('id3', [], 'remote-id3') + mapper.resolveReferencesForGroup = jest.fn() await mapper.resolveAllReferences() - expect(mapper.resolveReferencesPerProject).toHaveBeenCalledWith( - 'remote-id1', - remoteProjectId - ) - expect(mapper.resolveReferencesPerProject).toHaveBeenCalledWith( - 'remote-id2', - remoteProjectId + expect(mapper.resolveReferencesForGroup).toHaveBeenCalledWith( + 'p1#en_GB', + undefined ) - expect(mapper.resolveReferencesPerProject).toHaveBeenCalledWith( - 'remote-id3', - remoteProjectId + expect(mapper.resolveReferencesForGroup).toHaveBeenCalledWith( + 'p1#de_DE', + undefined ) }) }) - describe('resolveReferencesPerProject', () => { + describe('resolveReferencesForGroup', () => { it('should fetch references from the api', async () => { const api = createApi() api.fetchByFilter = jest.fn().mockImplementation(async () => []) const mapper = new CaaSMapper(api, 'de', {}, createLogger()) mapper.registerReferencedItem('id1', ['root', 'id1']) mapper.registerReferencedItem('id2', ['root', 'id2']) - await mapper.resolveReferencesPerProject() + await mapper.resolveReferencesForGroup() expect(api.fetchByFilter).toHaveBeenCalled() }) it('should resolve remote media references', async () => { - const api = createApi() - api.remotes = { 'remote-id1': { id: 'remote-id1', locale: 'de' } } + const api = createApi({ + 'remote-id1': { id: 'remote-id1', locale: 'de' }, + }) const mapper = new CaaSMapper(api, 'de', {}, createLogger()) const mediaPictures = await Promise.all([ mapper.mapMediaPicture(createMediaPicture('id1')), @@ -1844,14 +2276,9 @@ describe('CaaSMapper', () => { .fn() .mockImplementation( async ({ - filters, - locale, - page, - pagesize, - additionalParams, - remoteProject, - fetchOptions, - }: FetchByFilterParams) => { + locale, + remoteProject, + }: FetchByFilterParams) => { // Unfortunately jest doesn't support mocking a func call with specific parmaters if (remoteProject !== 'remote-id1') return createFetchResponse([]) if (locale !== 'de') return createFetchResponse([]) @@ -1859,24 +2286,25 @@ describe('CaaSMapper', () => { } ) const pageRef = createPageRef() + const remoteUrl = { locale: 'de' } pageRef.page.formData = { - media1: createMediaPictureReference('id1', 'remote-id1'), - media2: createMediaPictureReference('id2', 'remote-id1'), - media3: createMediaPictureReference('id3', 'remote-id1'), + media1: createMediaPictureReference('id1', 'remote-id1', remoteUrl), + media2: createMediaPictureReference('id2', 'remote-id1', remoteUrl), + media3: createMediaPictureReference('id3', 'remote-id1', remoteUrl), } pageRef.page.metaFormData = { ...pageRef.page.metaFormData, - media4: createMediaPictureReference('id4', 'remote-id1'), + media4: createMediaPictureReference('id4', 'remote-id1', remoteUrl), } pageRef.metaFormData = { ...pageRef.metaFormData, - media5: createMediaPictureReference('id5', 'remote-id1'), + media5: createMediaPictureReference('id5', 'remote-id1', remoteUrl), } // Mapping also implicitly registers referenced items in mapper instance await mapper.mapPageRef(pageRef) - await mapper.resolveReferencesPerProject('remote-id1') + await mapper.resolveReferencesForGroup('remote-id1#de') const funArguments = api.fetchByFilter.mock.calls[0][0] @@ -1893,4 +2321,127 @@ describe('CaaSMapper', () => { ) }) }) + + describe('reference batching across projects', () => { + it('should create one batch per configured project and skip unconfigured ones', async () => { + const api = createApi({ + media: { id: 'media-project', locale: 'de_DE' }, + datasets: { id: 'dataset-project', locale: 'en_US' }, + source: { id: 'index-project-a', useSourceLocale: true }, + }) + api.fetchByFilter = jest + .fn() + .mockImplementation(async () => createFetchResponse([])) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + + const localDatasetId = 'local-dataset-id' + const remoteDatasetId = 'remote-dataset-id' + const indexDatasetIdA = 'index-dataset-a' + const indexDatasetIdB = 'index-dataset-b' + const mediaId = 'shared-media-id' + + // the very same medium, referenced from the page and from the section + const sharedImage = () => + createMediaPictureReference(mediaId, 'media-project') + + const buildIndexRecord = ( + recordId: string, + datasetId: string, + projectId: string + ) => ({ + fsType: 'Record', + identifier: recordId, + value: { + fsType: 'DatasetReference', + target: { fsType: 'Dataset', identifier: datasetId }, + url: createReferenceUrl({ projectId, documentId: datasetId }), + }, + }) + + const section = createSection() + section.formData = { + st_index: { + fsType: 'FS_INDEX', + name: 'st_index', + dapType: 'DatasetDataAccessPlugin', + value: [ + buildIndexRecord('record-a', indexDatasetIdA, 'index-project-a'), + buildIndexRecord('record-b', indexDatasetIdB, 'index-project-b'), + ], + }, + st_image: sharedImage(), + } as any as CaaSApi_DataEntries + + const pageRef = createPageRef([createPageRefBody([section])]) + pageRef.page.formData = { + pt_datasetLocal: createDatasetReference(localDatasetId), + pt_datasetRemote: createDatasetReference( + remoteDatasetId, + 'dataset-project', + { locale: 'fr_FR' } + ), + pt_image: sharedImage(), + } + + // mapping registers the references, it does not fetch anything yet + await mapper.mapPageRef(pageRef) + await mapper.resolveAllReferences() + + const batches = (api.fetchByFilter as jest.Mock).mock.calls.map( + ([params]) => ({ + remoteProject: params.remoteProject, + locale: params.locale, + ids: params.filters[0].value, + }) + ) + + expect(batches).toHaveLength(4) + expect(batches).toEqual( + expect.arrayContaining([ + { remoteProject: undefined, locale: 'en_GB', ids: [localDatasetId] }, + { + remoteProject: 'dataset-project', + locale: 'en_US', + ids: [remoteDatasetId], + }, + { remoteProject: 'media-project', locale: 'de_DE', ids: [mediaId] }, + { + remoteProject: 'index-project-a', + locale: 'en_GB', + ids: [indexDatasetIdA], + }, + ]) + ) + expect( + batches.some(({ remoteProject }) => remoteProject === 'index-project-b') + ).toBe(false) + + // the medium is registered at two paths but requested exactly once + const mediaGroup = mapper._remoteReferences['media-project#de_DE'] + const mediaPaths = mediaGroup.references[`media-project#${mediaId}.de_DE`] + expect(mediaPaths).toHaveLength(2) + expect(mediaPaths).toEqual( + expect.arrayContaining([ + ['data', 'pt_image'], + ['children', 0, 'children', 0, 'data', 'st_image'], + ]) + ) + }) + + it('should keep the local group on the requested locale when mapped items change it', async () => { + const api = createApi() + api.fetchByFilter = jest + .fn() + .mockImplementation(async () => createFetchResponse([])) + const mapper = new CaaSMapper(api, 'en_GB', {}, createLogger()) + + mapper.registerReference('local-id', ['a'], undefined, {}) + mapper.locale = 'fr_FR' + await mapper.resolveAllReferences() + + const [params] = (api.fetchByFilter as jest.Mock).mock.calls[0] + expect(params.locale).toEqual('en_GB') + expect(mapper._referencedItems).toEqual({ 'local-id.en_GB': [['a']] }) + }) + }) }) diff --git a/src/modules/CaaSMapper.ts b/src/modules/CaaSMapper.ts index 786a8e3..95a672f 100644 --- a/src/modules/CaaSMapper.ts +++ b/src/modules/CaaSMapper.ts @@ -6,6 +6,7 @@ import { CaaSApi_DataEntries, CaaSApi_DataEntry, CaaSApi_Dataset, + CaaSApi_DatasetReference, CaaSApi_GCAPage, CaaSApi_ImageMapArea, CaaSApi_ImageMapAreaCircle, @@ -45,16 +46,16 @@ import { PermissionGroup, ProjectProperties, Reference, - RemoteProjectConfigurationEntry, RichTextElement, - Section, + Section } from '../types' import { parseISO } from 'date-fns' import XMLParser from './XMLParser' import { Logger, LogLevel } from './Logger' import { FSXARemoteApi } from './FSXARemoteApi' -import { FSXAContentMode, ImageMapAreaType } from '../enums' +import { ImageMapAreaType } from '../enums' import { findResolvedReferencesByIds, getItemId } from './MappingUtils' +import { parseReferenceUrl } from './ReferenceUrlParser' const chunk = (array: T[], size: number): T[][] => { const chunks: T[][] = [] @@ -76,6 +77,21 @@ export interface ReferencedItemsInfo { [identifier: string]: NestedPath[] } +/** + * Where a reference is resolved, once its project is known and the + * configuration has supplied the locale. + */ +export type ReferenceTarget = + | { kind: 'local' } + | { kind: 'remote'; projectId: string; locale?: string } + | { kind: 'unresolvable'; projectId: string } + +export interface RemoteReferenceGroup { + projectId: string + locale?: string + references: ReferencedItemsInfo +} + export interface ResolvedReferencesInfo { [id: string]: MappedCaasItem | CaasApi_Item } @@ -90,6 +106,12 @@ export class CaaSMapper { public logger: Logger api: FSXARemoteApi locale: string | undefined + /** + * The locale of the requested element. Constant for the whole resolution + * tree, unlike `locale`, which `setLocaleFromCaasItem` moves to the locale of + * each mapped item. + */ + readonly sourceLocale: string | undefined xmlParser: XMLParser customMapper?: CustomMapper referenceDepth: number @@ -97,9 +119,8 @@ export class CaaSMapper { resolvedReferences: ResolvedReferencesInfo = {} // stores references to items of current Project _referencedItems: ReferencedItemsInfo = {} - // stores References to remote Items _remoteReferences: { - [projectId: string]: ReferencedItemsInfo + [groupKey: string]: RemoteReferenceGroup } = {} // stores items that are being or have been processed and should not be fetched // Again. They cannot be added to resolvedReferences yet, since the fetch call @@ -118,11 +139,9 @@ export class CaaSMapper { ) { this.api = api this.locale = locale + this.sourceLocale = locale this.customMapper = utils.customMapper this.xmlParser = new XMLParser(logger) - Object.values(this.api.remotes || {}).forEach( - (entry) => (this._remoteReferences[entry.id] = {}) - ) this.logger = logger this.referenceDepth = utils.referenceDepth ?? 0 this.maxReferenceDepth = @@ -145,20 +164,21 @@ export class CaaSMapper { } } + private buildGroupKey(projectId: string, locale?: string) { + return `${projectId}#${locale ?? ''}` + } + /** * unifies the two different id formats {id}.{locale} and {id} to {id}.{locale} - * if remoteProjectConfiguration is passed, prefix with project id to avoid uuid clashes + * if a remoteProjectId is passed, prefix with it to avoid uuid clashes * * @param id uuid, may be of form {id}.{locale} or {id} - * @param remoteProjectConfiguration remoteProjectConfig + * @param remoteProjectId project the item lives in, if it is not the own one + * @param remoteLocale locale the item is fetched in, if it is not the own one * @returns uuid of form {id}.{locale} or {remoteProjectId}#{id}.{locale} */ - unifyId( - id: string, - remoteProjectConfiguration?: RemoteProjectConfigurationEntry - ) { + unifyId(id: string, remoteProjectId?: string, remoteLocale?: string) { const indexOfSeparator = id.indexOf('.') - const remoteLocale = remoteProjectConfiguration?.locale let idWithLocale if (indexOfSeparator > 0) { // id has form {id}.{locale}. Override Locale if set @@ -167,11 +187,104 @@ export class CaaSMapper { : id } else { // id has form {id}. transform to {id}.{locale} - idWithLocale = `${id}.${remoteLocale || this.locale}` + idWithLocale = `${id}.${remoteLocale || this.sourceLocale}` + } + return remoteProjectId ? `${remoteProjectId}#${idWithLocale}` : idWithLocale + } + + /** + * Decides where a reference is resolved. The url identifies the project; the + * locale always comes from the `remotes` configuration or from the requested + * element, never from the url, because an editor cannot control which locale + * a reference url carries. A project that is not configured is not fetched, + * which is what keeps content data from steering a request. + * + * @param url the url on the reference, may be missing or arbitrary + * @param referencedProject the `remoteProject` field, on media references + * @param inheritedProjectId the project of the surrounding document + */ + resolveReferenceTarget( + url: string | undefined, + referencedProject: string | undefined, + inheritedProjectId: string | undefined + ): ReferenceTarget { + const parsed = parseReferenceUrl(url) + const projectId = + parsed?.projectId ?? referencedProject ?? inheritedProjectId + + if (!projectId) { + return { kind: 'local' } + } + + // an entry may name the own project, to read its references in another + // locale than the requested one + const remote = this.api.getRemoteConfigById(projectId) + if (!remote) { + return projectId === this.api.projectID + ? { kind: 'local' } + : { kind: 'unresolvable', projectId } + } + + return { + kind: 'remote', + projectId, + locale: remote.useSourceLocale ? this.sourceLocale : remote.locale, } - return remoteProjectConfiguration - ? `${remoteProjectConfiguration.id}#${idWithLocale}` - : idWithLocale + } + + /** + * Registers a reference so that `resolveAllReferences` can fetch it in a + * batch. A reference into a project that is not configured is not registered; + * its placeholder stays in the payload. + * + * @returns the placeholder string to put into the mapped output + */ + registerReference( + identifier: string, + path: NestedPath, + url: string | undefined, + inherited: { projectId?: string }, + options: { referencedProject?: string; imageMapResolution?: string } = {} + ): string { + const target = this.resolveReferenceTarget( + url, + options.referencedProject, + inherited.projectId + ) + + if (target.kind === 'unresolvable') { + return this.registerUnresolvableReference( + identifier, + path, + target.projectId, + options.imageMapResolution + ) + } + + return this.registerReferencedItem( + identifier, + path, + target.kind === 'remote' ? target.projectId : undefined, + options.imageMapResolution, + target.kind === 'remote' ? target.locale : undefined + ) + } + + private registerUnresolvableReference( + identifier: string, + path: NestedPath, + projectId: string, + imageMapResolution?: string + ): string { + this.logger.warn( + `Reference with identifier '${identifier}' points at project '${projectId}', which is not part of the 'remotes' configuration. The reference is left unresolved - add the project to 'remotes' to resolve it.`, + { path: path.join('/') } + ) + + const unifiedId = this.unifyId(identifier, projectId, this.sourceLocale) + return imageMapResolution + ? `IMAGEMAP___${imageMapResolution}___${unifiedId}` + : `[REFERENCED-REMOTE-ITEM-${unifiedId}]` } /** @@ -180,36 +293,41 @@ export class CaaSMapper { * @param identifier item identifier * @param path after fetch, items are inserted at all registered paths * @param remoteProjectId optional. If passed, the item will be fetched from the specified project + * @param imageMapResolution + * @param remoteProjectLocale optional. Locale the item is fetched in, if it differs from the own one * @returns placeholder string */ registerReferencedItem( identifier: string, path: NestedPath, remoteProjectId?: string, - imageMapResolution?: string + imageMapResolution?: string, + remoteProjectLocale?: string ): string { - const remoteData = this.getRemoteConfigForProject(remoteProjectId) - const remoteProjectKey = remoteData?.id - - if (remoteProjectId && !remoteProjectKey) { - this.logger.warn( - `Item with identifier '${identifier}' was tried to register from remoteProject '${remoteProjectId}' but no remote key was found in the config.` - ) - } - - const unifiedId = this.unifyId(identifier, remoteData) + const unifiedId = this.unifyId( + identifier, + remoteProjectId, + remoteProjectLocale + ) // Preflight check to avoid costly operations in non debug mode this.logger.logLevel === LogLevel.DEBUG && this.logger.debug('Registering Referenced Item ', { - remoteProjectKey, + remoteProjectId, + remoteProjectLocale, identifier, path: path.join('/'), }) - if (remoteProjectKey) { - this._remoteReferences[remoteProjectKey][unifiedId] = [ - ...(this._remoteReferences[remoteProjectKey][unifiedId] || []), + if (remoteProjectId) { + const groupKey = this.buildGroupKey(remoteProjectId, remoteProjectLocale) + const group = (this._remoteReferences[groupKey] ??= { + projectId: remoteProjectId, + locale: remoteProjectLocale, + references: {}, + }) + group.references[unifiedId] = [ + ...(group.references[unifiedId] || []), path, ] return imageMapResolution @@ -256,14 +374,13 @@ export class CaaSMapper { } switch (entry.fsType) { case 'CMS_INPUT_COMBOBOX': - const comboboxOption: Option | null = entry.value + return entry.value ? { - type: 'Option', - key: entry.value.identifier, - value: entry.value.label, - } + type: 'Option', + key: entry.value.identifier, + value: entry.value.label, + } : null - return comboboxOption case 'CMS_INPUT_DOM': case 'CMS_INPUT_DOMTABLE': const richTextElements: RichTextElement[] = entry.value @@ -278,44 +395,40 @@ export class CaaSMapper { case 'CMS_INPUT_NUMBER': case 'CMS_INPUT_TEXT': case 'CMS_INPUT_TEXTAREA': - const simpleValue: string | number = entry.value - return simpleValue + return entry.value case 'CMS_INPUT_RADIOBUTTON': - const radiobuttonOption: Option | null = entry.value + return entry.value ? { - type: 'Option', - key: entry.value.identifier, - value: entry.value.label, - // TODO: Remove this spread with next major release (Breaking Change!) - ...entry.value, - } + type: 'Option', + key: entry.value.identifier, + value: entry.value.label, + // TODO: Remove this spread with next major release (Breaking Change!) + ...entry.value, + } : null - return radiobuttonOption case 'CMS_INPUT_DATE': - const dateValue: Date | null = entry.value + return entry.value ? parseISO(entry.value) : null - return dateValue case 'CMS_INPUT_LINK': - const link: Link | null = entry.value + return entry.value ? { - type: 'Link', - template: entry.value.template.uid, - data: await this.mapDataEntries( - entry.value.formData, - [...path, 'data'], - remoteProjectLocale, - remoteProjectId - ), - meta: await this.mapDataEntries( - entry.value.metaFormData, - [...path, 'meta'], - remoteProjectLocale, - remoteProjectId - ), - } + type: 'Link', + template: entry.value.template.uid, + data: await this.mapDataEntries( + entry.value.formData, + [...path, 'data'], + remoteProjectLocale, + remoteProjectId + ), + meta: await this.mapDataEntries( + entry.value.metaFormData, + [...path, 'meta'], + remoteProjectLocale, + remoteProjectId + ), + } : null - return link case 'CMS_INPUT_LIST': if (!entry.value) return [] return Promise.all( @@ -371,10 +484,11 @@ export class CaaSMapper { ) return null } - return this.registerReferencedItem( + return this.registerReference( entry.value.target.identifier, path, - remoteProjectId + entry.value.url, + { projectId: remoteProjectId } ) } return null @@ -427,10 +541,12 @@ export class CaaSMapper { ) return null } - return this.registerReferencedItem( + return this.registerReference( entry.value.identifier, path, - entry.value.remoteProject || remoteProjectId + entry.value.url, + { projectId: remoteProjectId }, + { referencedProject: entry.value.remoteProject } ) } else if (['PageRef', 'GCAPage'].includes(entry.value.fsType)) { if (!entry.value.identifier) { @@ -456,8 +572,11 @@ export class CaaSMapper { if (entry.dapType === 'DatasetDataAccessPlugin') { return entry.value .map((record, index) => { + const reference = record?.value as + | CaaSApi_DatasetReference + | undefined const identifier: string | undefined = - record?.value?.target?.identifier + reference?.target?.identifier if (!identifier) { this.logger.warn( 'Skipping FS_INDEX record with a broken/null identifier', @@ -465,24 +584,24 @@ export class CaaSMapper { ) return null } - return this.registerReferencedItem( + return this.registerReference( identifier, [...path, index], - remoteProjectId + reference?.url, + { projectId: remoteProjectId } ) }) .filter(Boolean) } return entry case 'Option': - const option: Option = { + return { type: 'Option', key: entry.identifier, value: entry.label, } - return option case 'CMS_INPUT_PERMISSION': - const permission: Permission = { + return { type: 'Permission', fsType: entry.fsType, name: entry.name, @@ -497,7 +616,6 @@ export class CaaSMapper { } as PermissionActivity }), } - return permission default: return entry } @@ -512,7 +630,7 @@ export class CaaSMapper { await Promise.all( richTextElements.map(async (richTextElement, index) => { if (richTextElement.type === 'link') { - const link = { + richTextElement.data = { type: 'Link', template: richTextElement.data.type as string, data: await this.mapDataEntries( @@ -523,7 +641,6 @@ export class CaaSMapper { ), meta: {}, } - richTextElement.data = link } if (Array.isArray(richTextElement.content)) { richTextElement.content = await this.mapLinksInRichTextElements( @@ -829,11 +946,15 @@ export class CaaSMapper { let image = null if (media) { - image = this.registerReferencedItem( + image = this.registerReference( media.identifier, [...path, 'media'], - media.remoteProject, - resolution.uid + media.url, + { projectId: remoteProjectId }, + { + referencedProject: media.remoteProject, + imageMapResolution: resolution.uid, + } ) } @@ -1083,14 +1204,13 @@ export class CaaSMapper { this.resolvedReferences ) - // merge all remote references into one object - const remoteReferencesValues = Object.values(this._remoteReferences) - const remoteReferencesMerged = - remoteReferencesValues.length > 0 - ? remoteReferencesValues.reduce((result, current) => - Object.assign(result, current) - ) - : {} + // merge the references of all groups into one object + const remoteReferencesMerged = Object.values( + this._remoteReferences + ).reduce( + (result, group) => Object.assign(result, group.references), + {} + ) // return return { @@ -1103,8 +1223,8 @@ export class CaaSMapper { /** * Calls ResolveReferences for currentProject and each RemoteProject * - * @param data * @returns data + * @param filterContext */ async resolveAllReferences(filterContext?: unknown): Promise { if (this.referenceDepth >= this.maxReferenceDepth) { @@ -1115,37 +1235,37 @@ export class CaaSMapper { return } this.referenceDepth++ - const remoteIds = Object.keys(this._remoteReferences) - this.logger.debug('CaaSMapper.resolveAllReferences', { remoteIds }) + const groupKeys = Object.keys(this._remoteReferences) + this.logger.debug('CaaSMapper.resolveAllReferences', { groupKeys }) await Promise.all([ - this.resolveReferencesPerProject(undefined, filterContext), - ...remoteIds.map((remoteId) => - this.resolveReferencesPerProject(remoteId, filterContext) + this.resolveReferencesForGroup(undefined, filterContext), + ...groupKeys.map((groupKey) => + this.resolveReferencesForGroup(groupKey, filterContext) ), ]) } /** - * This method will create a filter for all referenced items that are registered inside of the referencedItems - * and fetch them in a single CaaS-Request. If remoteProjectId is set, referenced Items from the remoteProject are fetched - * After a successful fetch all references in the json structure will be replaced with the fetched and mapped item + * This method will create a filter for all referenced items of one group and + * fetch them in a single CaaS-Request per chunk. A group is one distinct + * project/locale pair, or the local project when no groupKey is given. + * After a successful fetch all references in the json structure will be + * replaced with the fetched and mapped item */ - async resolveReferencesPerProject( - remoteProjectId?: string, - filterContext?: unknown - ) { - this.logger.debug('CaaSMapper.resolveReferencesPerProject', { - remoteProjectId, + async resolveReferencesForGroup(groupKey?: string, filterContext?: unknown) { + const group = groupKey ? this._remoteReferences[groupKey] : undefined + this.logger.debug('CaaSMapper.resolveReferencesForGroup', { + groupKey, + projectId: group?.projectId, + locale: group?.locale, }) - const referencedItems = remoteProjectId - ? this._remoteReferences[remoteProjectId] - : this._referencedItems + const referencedItems = group ? group.references : this._referencedItems - // use remoteProjectLocale if provided. normal locale as standard - const remoteProjectData = this.getRemoteConfigForProject(remoteProjectId) - const remoteProjectLocale = remoteProjectData?.locale - const locale = remoteProjectLocale || this.locale + // the group carries the locale its items are fetched in + const remoteProjectId = group?.projectId + const remoteProjectLocale = group?.locale + const locale = remoteProjectLocale || this.sourceLocale const referencedIds = Object.keys(referencedItems) @@ -1162,8 +1282,8 @@ export class CaaSMapper { // skip stringification if logLevel is higher than debug this.logger.logLevel === LogLevel.DEBUG && - this.logger.debug('CaaSMapper.resolveReferencesPerProject: Id data', { - project: remoteProjectId || 'localProject', + this.logger.debug('CaaSMapper.resolveReferencesForGroup: Id data', { + group: groupKey || 'localProject', resolvedIdsArray, referencedIds, idsToFetchFromCaaS, @@ -1195,18 +1315,8 @@ export class CaaSMapper { ) } else { this.logger.debug( - 'CaaSMapper.resolveReferencesPerProject: Nothing to fetch' + 'CaaSMapper.resolveReferencesForGroup: Nothing to fetch' ) } } - - private getRemoteConfigForProject( - projectId?: string - ): RemoteProjectConfigurationEntry | undefined { - return projectId - ? Object.values(this.api.remotes || {}).find( - (entry) => entry.id === projectId - ) - : undefined - } } diff --git a/src/modules/FSXARemoteApi.spec.ts b/src/modules/FSXARemoteApi.spec.ts index fa15cf9..262b304 100644 --- a/src/modules/FSXARemoteApi.spec.ts +++ b/src/modules/FSXARemoteApi.spec.ts @@ -16,6 +16,7 @@ import { createMediaPictureReference, } from '../testutils' import { getMappedMediaPicture } from '../testutils/getMappedMediaPicture' + require('jest-fetch-mock').enableFetchMocks() describe('FSXARemoteAPI', () => { @@ -304,16 +305,12 @@ describe('FSXARemoteAPI', () => { const expectedCaaSUrl = `${config.caasURL}/${config.tenantID}/${config.projectID}.${config.contentMode}.content?${pagesizeQuery}` expect(actualCaaSUrl).toStrictEqual(expectedCaaSUrl) }) - it('should throw an error for an invalid remote project', () => { - const config = generateRandomConfig() - const remoteApi = new FSXARemoteApi(config) + it('should throw when building a url for a project that is not configured as a remote', () => { + const remoteApi = new FSXARemoteApi(generateRandomConfig()) - try { - remoteApi.buildCaaSUrl({ remoteProject: 'unknown project' }) - } catch (error: any) { - expect(error.message).toBe(FSXAApiErrors.UNKNOWN_REMOTE) - expect(error.statusCode).toBe(HttpStatus.NOT_FOUND) - } + expect(() => + remoteApi.buildCaaSUrl({ remoteProject: 'unconfigured' }) + ).toThrow(FSXAApiErrors.UNKNOWN_REMOTE) }) it('should return the correct caas url when special chars are used in id, locale, page or pagesize', () => { const specialChars = "*_'();:@&=+$,?%#[]_*'();:@&=+$,?%#[]" @@ -786,6 +783,80 @@ describe('FSXARemoteAPI', () => { items, }) }) + it('should throw when fetching from a project that is not configured as a remote', async () => { + const unconfigured = generateRandomConfig() + unconfigured.remotes = {} as any + const remoteApi = new FSXARemoteApi(unconfigured) + + await expect( + remoteApi.fetchByFilter({ + filters: [], + locale: 'en_GB', + remoteProject: 'unconfigured-project', + }) + ).rejects.toThrow(FSXAApiErrors.UNKNOWN_REMOTE) + }) + it('should fetch a configured remote project in its configured locale', async () => { + const remoteApi = new FSXARemoteApi({ + ...generateRandomConfig(), + remotes: { media: { id: 'media-project', locale: 'de_DE' } }, + }) + fetchMock.mockResponseOnce( + JSON.stringify({ _embedded: { 'rh:doc': [] } }) + ) + + await remoteApi.fetchByFilter({ + filters: [], + locale: 'en_GB', + remoteProject: 'media-project', + }) + + const url = decodeURIComponent(fetchMock.mock.calls[0][0] as string) + expect(url).toContain('/media-project.') + expect(url).toContain('{"locale.language":{"$eq":"de"}}') + expect(url).toContain('{"locale.country":{"$eq":"DE"}}') + }) + it('should fetch a useSourceLocale project in the requested locale', async () => { + const remoteApi = new FSXARemoteApi({ + ...generateRandomConfig(), + remotes: { data: { id: 'data-project', useSourceLocale: true } }, + }) + fetchMock.mockResponseOnce( + JSON.stringify({ _embedded: { 'rh:doc': [] } }) + ) + + await remoteApi.fetchByFilter({ + filters: [], + locale: 'en_GB', + remoteProject: 'data-project', + }) + + const url = decodeURIComponent(fetchMock.mock.calls[0][0] as string) + expect(url).toContain('{"locale.language":{"$eq":"en"}}') + expect(url).toContain('{"locale.country":{"$eq":"GB"}}') + }) + it('should keep the requested locale in fetchElement for a useSourceLocale project', async () => { + const remoteApi = new FSXARemoteApi({ + ...generateRandomConfig(), + remotes: { data: { id: 'data-project', useSourceLocale: true } }, + }) + const fetchByFilter = jest + .spyOn(remoteApi, 'fetchByFilter') + .mockResolvedValue({ page: 1, pagesize: 30, items: [{}] } as any) + + await remoteApi.fetchElement({ + id: 'some-id', + locale: 'en_GB', + remoteProject: 'data', + }) + + expect(fetchByFilter).toHaveBeenCalledWith( + expect.objectContaining({ + locale: 'en_GB', + remoteProject: 'data-project', + }) + ) + }) it('should return items when fetching remote items', async () => { const mainMedia = createMediaPicture( undefined, @@ -807,9 +878,11 @@ describe('FSXARemoteAPI', () => { .mockResponseOnce(JSON.stringify(firstResponse)) .mockResponseOnce(JSON.stringify(secondResponse)) + // the caller states the locale a remote project is read in; the locale + // configured in 'remotes' is no longer consulted const actualRequest = await remoteApi.fetchByFilter({ filters, - locale: 'de_DE', + locale: config.remotes.remote.locale, remoteProject: config.remotes.remote.id, }) @@ -818,14 +891,12 @@ describe('FSXARemoteAPI', () => { config.remotes.remote.locale, config.remotes.remote.id ) - const mappedReferencedMedia = getMappedMediaPicture( + mappedMainMedia.meta.fsRef = getMappedMediaPicture( referencedMedia, config.remotes.remote.locale, config.remotes.remote.id ) - mappedMainMedia.meta.fsRef = mappedReferencedMedia - expect(actualRequest).toBeDefined() expect(actualRequest).toStrictEqual({ page: 1, @@ -844,7 +915,9 @@ describe('FSXARemoteAPI', () => { remoteApi = new FSXARemoteApi(config) }) it('should trigger the fetch method with locale', () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const locale = `${faker.location.countryCode().toLowerCase()}_${faker.location.countryCode().toLowerCase()}` const initialPath = '/' remoteApi.fetchNavigation({ initialPath, locale }) @@ -855,7 +928,9 @@ describe('FSXARemoteAPI', () => { expect(actualURL).toBe(expectedURL) }) it('should trigger the fetch method with initialPath = /', () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const locale = `${faker.location.countryCode().toLowerCase()}_${faker.location.countryCode().toLowerCase()}` remoteApi.fetchNavigation({ locale }) @@ -865,7 +940,9 @@ describe('FSXARemoteAPI', () => { expect(actualURL).toBe(expectedURL) }) it('should trigger the fetch method with initialPath', () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const locale = `${faker.location.countryCode().toLowerCase()}_${faker.location.countryCode().toLowerCase()}` const initialPath = faker.lorem.words(3).split(' ').join('/') @@ -897,14 +974,18 @@ describe('FSXARemoteAPI', () => { } }) it('should return the response', async () => { - const expectedResponse = JSON.stringify(faker.helpers.fake("{{lorem.word}}")) + const expectedResponse = JSON.stringify( + faker.helpers.fake('{{lorem.word}}') + ) fetchMock.mockResponseOnce(JSON.stringify(expectedResponse)) const locale = `${faker.location.countryCode().toLowerCase()}_${faker.location.countryCode().toLowerCase()}` const actualResponse = await remoteApi.fetchNavigation({ locale }) expect(actualResponse).toEqual(expectedResponse) }) it('should throw an unknown error when ? is used in initial path', async () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const locale = `${faker.location.countryCode().toLowerCase()}_${faker.location.countryCode().toLowerCase()}` const initialPath = faker.lorem.words(3).split(' ').join('/') + '?' try { @@ -915,7 +996,9 @@ describe('FSXARemoteAPI', () => { } }) it('should throw an unknown error when # is used in initial path', async () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const locale = `${faker.location.countryCode().toLowerCase()}_${faker.location.countryCode().toLowerCase()}` const initialPath = faker.lorem.words(3).split(' ').join('/') + '#' try { @@ -926,7 +1009,9 @@ describe('FSXARemoteAPI', () => { } }) it('should trigger the fetch method with encoded params when special chars are used in locale or initial path', () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const locale = "*_'();:@&=+$,?%#[]_*'();:@&=+$,?%#[]" const initialPath = "*_'();:@&=+$,%[]" remoteApi.fetchNavigation({ initialPath, locale }) @@ -944,7 +1029,9 @@ describe('FSXARemoteAPI', () => { remoteApi = new FSXARemoteApi(config) }) it('should trigger fetchByFilter with correct params', () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const localeLanguage = faker.lorem.word(2).toLowerCase() const localeCountry = faker.lorem.word(2).toUpperCase() const locale = localeLanguage + '_' + localeCountry @@ -963,7 +1050,9 @@ describe('FSXARemoteAPI', () => { expect(actualURL).toBe(expectedURL) }) it('should trigger fetchByFilter with encoded params when special chars in locale are used', () => { - fetchMock.mockResponseOnce(JSON.stringify(faker.helpers.fake("{{lorem.word}}"))) + fetchMock.mockResponseOnce( + JSON.stringify(faker.helpers.fake('{{lorem.word}}')) + ) const localeLanguage = "*'();:@&=+$,?%#[]" const localeCountry = "*'();:@&=+$,?%#[]" const locale = localeLanguage + '_' + localeCountry diff --git a/src/modules/FSXARemoteApi.ts b/src/modules/FSXARemoteApi.ts index 984bf53..e28c42c 100644 --- a/src/modules/FSXARemoteApi.ts +++ b/src/modules/FSXARemoteApi.ts @@ -1,36 +1,27 @@ import { stringify } from 'qs' -import { - CaaSMapper, - Logger, - ReferencedItemsInfo, - ResolvedReferencesInfo, -} from '.' +import { CaaSMapper, Logger, ReferencedItemsInfo, ResolvedReferencesInfo } from '.' import { FetchResponse, ProjectProperties } from '..' import { - NavigationData, + CaasApi_Item, + CaasItemFilter, CustomMapper, - QueryBuilderQuery, - FetchNavigationParams, - FetchElementParams, FetchByFilterParams, - FSXARemoteApiConfig, + FetchElementParams, + FetchNavigationParams, FSXAApi, - CaasItemFilter, - NavigationItemFilter, - RemoteApiFilterOptions, + FSXARemoteApiConfig, MappedCaasItem, - SortParams, - CaasApi_Item, - RemoteProjectConfiguration, + NavigationData, + NavigationItemFilter, NormalizedFetchResponse, NormalizedProjectPropertyResponse, + QueryBuilderQuery, + RemoteProjectConfiguration, + RemoteProjectConfigurationEntry, + SortParams } from '../types' -import { - removeFromIdMap, - removeFromSeoRouteMap, - removeFromStructure, -} from '../utils' -import { FSXAApiErrors, FSXAContentMode, HttpStatus } from './../enums' +import { removeFromIdMap, removeFromSeoRouteMap, removeFromStructure } from '../utils' +import { FSXAApiErrors, FSXAContentMode, HttpStatus } from '../enums' import { LogLevel } from './Logger' import { denormalizeResolvedReferences } from './MappingUtils' import { ComparisonQueryOperatorEnum, QueryBuilder } from './QueryBuilder' @@ -133,7 +124,7 @@ export class FSXARemoteApi implements FSXAApi { customMapper: this._customMapper, navigationItemFilter: this._navigationItemFilter, caasItemFilter: this._caasItemFilter, - includeRevisionInMediaUrls: this._includeRevisionInMediaUrls + includeRevisionInMediaUrls: this._includeRevisionInMediaUrls, }) } @@ -148,24 +139,21 @@ export class FSXARemoteApi implements FSXAApi { } private verifyRemoteProjectExists(remoteProjectId: string) { - const remoteProjectConfig = Object.values(this._remotes) - const foundRemoteProject = remoteProjectConfig.find( - (config) => config.id === remoteProjectId - ) - if (!foundRemoteProject) { + if (!this.getRemoteConfigById(remoteProjectId)) { throw new HttpError(FSXAApiErrors.UNKNOWN_REMOTE, HttpStatus.NOT_FOUND) } } - private getRemoteConfigById(remoteProjectId: string) { - const remoteProjectConfig = Object.values(this._remotes) - const foundRemoteProject = remoteProjectConfig.find( - (config) => config.id === remoteProjectId + /** + * @param projectId the uuid of a project, as it appears on a reference url + * @returns the entry configured for that project, or undefined + */ + public getRemoteConfigById( + projectId: string + ): RemoteProjectConfigurationEntry | undefined { + return Object.values(this._remotes).find( + (config) => config.id === projectId ) - if (!foundRemoteProject) { - throw new HttpError(FSXAApiErrors.UNKNOWN_REMOTE, HttpStatus.NOT_FOUND) - } - return foundRemoteProject } /** @@ -216,7 +204,7 @@ export class FSXARemoteApi implements FSXAApi { if (filters) { let localeFilter: QueryBuilderQuery[] = [] if (locale) { - if (typeof locale !== 'string' || !locale.includes('_')) { + if (!locale.includes('_')) { this._logger.error( '[buildNavigationServiceUrl]', `Invalid locale format. Expected format: 'xx_YY' but got '${locale.toString()}'` @@ -283,6 +271,7 @@ export class FSXARemoteApi implements FSXAApi { * read the [Navigation Service documentation](https://navigationservice.e-spirit.cloud/docs/user/en/documentation.html). * @param locale value must be ISO conform, both 'en' and 'en_US' are valid." * @param initialPath can be provided when you want to access a subtree of the navigation + * @param all * @returns {string} the Navigation Service url for either a subtree of or a complete navigation */ buildNavigationServiceUrl({ @@ -429,13 +418,12 @@ export class FSXARemoteApi implements FSXAApi { const seo = removeFromSeoRouteMap(navigation.seoRouteMap, allowedRouteIds) const structure = removeFromStructure(navigation.structure, allowedRouteIds) const filteredIdMap = removeFromIdMap(navigation.idMap, allowedRouteIds) - const filteredNavigation = { + return { ...navigation, idMap: filteredIdMap, seoRouteMap: seo, structure, } - return filteredNavigation } /** @@ -448,6 +436,8 @@ export class FSXARemoteApi implements FSXAApi { * @param additionalParams optional additional URL parameters * @param remoteProject optional name of the remote project * @param fetchOptions optional object to pass additional request options (Check {@link RequestInit RequestInit}) + * @param filterContext + * @param normalized * @returns {Promise} a Promise with the mapped result */ async fetchElement({ @@ -459,17 +449,15 @@ export class FSXARemoteApi implements FSXAApi { filterContext, normalized = false, }: FetchElementParams): Promise { - if (remoteProject && !this.remotes[remoteProject]) { + const remoteConfig = remoteProject ? this.remotes[remoteProject] : undefined + if (remoteProject && !remoteConfig) { throw new HttpError(FSXAApiErrors.UNKNOWN_REMOTE, HttpStatus.NOT_FOUND) } - locale = - remoteProject && this.remotes - ? this.remotes[remoteProject]?.locale - : locale + if (remoteConfig?.locale && !remoteConfig.useSourceLocale) { + locale = remoteConfig.locale + } - const remoteProjectId = remoteProject - ? this.remotes[remoteProject]?.id - : undefined + const remoteProjectId = remoteConfig?.id const { items, @@ -513,16 +501,16 @@ export class FSXARemoteApi implements FSXAApi { * Example call: * * ```typescript - const englishMedia = await fetchByFilter({ - filters: [ - { - field: 'fsType', - value: 'Media', - operator: ComparisonQueryOperatorEnum.EQUALS, - }, - ], - "en_GB", - }) + const englishMedia = await fetchByFilter({ + filters: [ + { + field: 'fsType', + value: 'Media', + operator: ComparisonQueryOperatorEnum.EQUALS, + }, + ], + "en_GB", + }) * ``` * @param filters array of {@link QueryBuilderQuery QueryBuilderQuery} to filter you request * @param locale value must be ISO conform, both 'en' and 'en_US' are valid @@ -532,6 +520,7 @@ export class FSXARemoteApi implements FSXAApi { * @param additionalParams optional additional URL parameters * @param remoteProject optional name of the remote project * @param fetchOptions optional object to pass additional request options (Check {@link RequestInit RequestInit}) + * @param mapper * @returns the mapped and filtered response from the CaaS request, * if `additionalParams.keys` are set, the result will be unmapped, * if `data._embedded['rh:doc']` is undefined, the returning result will be the unmapped `data` object @@ -565,6 +554,14 @@ export class FSXARemoteApi implements FSXAApi { page = 1 } + const remoteConfig = remoteProjectId + ? this.getRemoteConfigById(remoteProjectId) + : undefined + const effectiveLocale = + remoteConfig?.locale && !remoteConfig.useSourceLocale + ? remoteConfig.locale + : locale + const url = this.buildCaaSUrl({ filters, additionalParams: { @@ -572,7 +569,7 @@ export class FSXARemoteApi implements FSXAApi { rep: 'hal', }, remoteProject: remoteProjectId, - locale, + locale: effectiveLocale, page, pagesize, sort, @@ -622,11 +619,9 @@ export class FSXARemoteApi implements FSXAApi { } } - const remoteProjectLocale = remoteProjectId - ? this.getRemoteConfigById(remoteProjectId).locale - : undefined + const remoteProjectLocale = remoteProjectId ? effectiveLocale : undefined - let mapperLocale = locale + let mapperLocale = effectiveLocale if (!mapperLocale && unmappedItems[0].locale) { mapperLocale = @@ -640,7 +635,7 @@ export class FSXARemoteApi implements FSXAApi { mapperLocale, { customMapper: this._customMapper, - maxReferenceDepth: this._maxReferenceDepth + maxReferenceDepth: this._maxReferenceDepth, }, new Logger(this._logLevel, 'CaaSMapper') ) @@ -1014,15 +1009,19 @@ export class FSXARemoteApi implements FSXAApi { ``` */ public set remotes(value: RemoteProjectConfiguration) { - const keys = Object.keys(value) - keys.forEach((key) => { - const { id, locale } = value[key] + const seenIds = new Set() + Object.keys(value).forEach((key) => { + const { id, locale, useSourceLocale } = value[key] if (!id) { throw new Error(FSXAApiErrors.MISSING_REMOTE_ID) } - if (!locale) { + if (!locale && !useSourceLocale) { throw new Error(FSXAApiErrors.MISSING_REMOTE_LOCALE) } + if (seenIds.has(id)) { + throw new Error(FSXAApiErrors.DUPLICATE_REMOTE_ID) + } + seenIds.add(id) }) this._remotes = value diff --git a/src/modules/ReferenceUrlParser.spec.ts b/src/modules/ReferenceUrlParser.spec.ts new file mode 100644 index 0000000..f6d6263 --- /dev/null +++ b/src/modules/ReferenceUrlParser.spec.ts @@ -0,0 +1,77 @@ +import { parseReferenceUrl } from './ReferenceUrlParser' + +describe('parseReferenceUrl', () => { + const url = + 'https://caas.example.com/my-tenant/8e2dd004-248c-4c3f-952e-f32296a9d45f.preview.content/68aec889-1ff6-47a0-b763-42d2160e7889.en_GB' + + it('should split a caas document url into its parts', () => { + expect(parseReferenceUrl(url)).toEqual({ + baseUrl: 'https://caas.example.com', + tenantId: 'my-tenant', + collectionId: '8e2dd004-248c-4c3f-952e-f32296a9d45f.preview.content', + projectId: '8e2dd004-248c-4c3f-952e-f32296a9d45f', + contentMode: 'preview', + documentId: '68aec889-1ff6-47a0-b763-42d2160e7889', + locale: 'en_GB', + }) + }) + + it('should expose the collection id as the caas names it', () => { + expect(parseReferenceUrl(url)?.collectionId).toBe( + '8e2dd004-248c-4c3f-952e-f32296a9d45f.preview.content' + ) + }) + + it('should parse release urls as well', () => { + const releaseUrl = url.replace('.preview.content', '.release.content') + expect(parseReferenceUrl(releaseUrl)?.contentMode).toBe('release') + }) + + it('should ignore query parameters', () => { + expect(parseReferenceUrl(`${url}?rep=hal`)?.documentId).toBe( + '68aec889-1ff6-47a0-b763-42d2160e7889' + ) + }) + + it('should return an undefined locale if the document segment has none', () => { + const withoutLocale = url.replace('.en_GB', '') + expect(parseReferenceUrl(withoutLocale)).toMatchObject({ + documentId: '68aec889-1ff6-47a0-b763-42d2160e7889', + locale: undefined, + }) + }) + + it('should return null for missing input', () => { + expect(parseReferenceUrl(undefined)).toBeNull() + expect(parseReferenceUrl(null)).toBeNull() + expect(parseReferenceUrl('')).toBeNull() + }) + + it('should return null for a string that is not a url', () => { + expect(parseReferenceUrl('some-media-url')).toBeNull() + }) + + it('should return null if the path does not have exactly three segments', () => { + expect( + parseReferenceUrl('https://caas.example.com/my-tenant/x.preview.content') + ).toBeNull() + expect( + parseReferenceUrl( + 'https://caas.example.com/a/my-tenant/x.preview.content/doc.en_GB' + ) + ).toBeNull() + }) + + it('should return null if the collection segment is not a caas content collection', () => { + expect( + parseReferenceUrl( + 'https://caas.example.com/my-tenant/8e2dd004.staging.content/doc.en_GB' + ) + ).toBeNull() + expect( + parseReferenceUrl( + 'https://caas.example.com/my-tenant/8e2dd004.preview/doc.en_GB' + ) + ).toBeNull() + }) +}) diff --git a/src/modules/ReferenceUrlParser.ts b/src/modules/ReferenceUrlParser.ts new file mode 100644 index 0000000..0cd894e --- /dev/null +++ b/src/modules/ReferenceUrlParser.ts @@ -0,0 +1,66 @@ +/** + * The parts of a CaaS document url as written onto references by FirstSpirit. + * Shape: {baseUrl}/{tenantId}/{projectId}.{contentMode}.content/{documentId}.{locale} + */ +export interface ParsedReferenceUrl { + baseUrl: string + tenantId: string + /** `..content`, the CaaS collection */ + collectionId: string + projectId: string + contentMode: string + documentId: string + locale?: string +} + +// {projectId}.{preview|release}.content +const CONTENT_COLLECTION_PATTERN = /^(.+)\.(preview|release)\.content$/ + +const EXPECTED_PATH_SEGMENTS = 3 + +/** + * Parses a CaaS document url into its parts. + * + * This is the only place that knows the url layout. The layout is a CaaS + * implementation detail and not a documented contract, so anything that does + * not match exactly yields null and callers must treat the reference as local. + * + * @param url the url found on a reference, may be missing or arbitrary + * @returns the parsed parts, or null if the url is absent or does not match + */ +export const parseReferenceUrl = ( + url?: string | null +): ParsedReferenceUrl | null => { + if (!url) return null + + let parsed: URL + try { + parsed = new URL(url) + } catch { + return null + } + + const segments = parsed.pathname.split('/').filter(Boolean) + if (segments.length !== EXPECTED_PATH_SEGMENTS) return null + + const [tenantId, collectionSegment, documentSegment] = segments + const collectionMatch = CONTENT_COLLECTION_PATTERN.exec(collectionSegment) + if (!collectionMatch) return null + + const separatorIndex = documentSegment.indexOf('.') + const hasLocale = separatorIndex > 0 + + return { + baseUrl: parsed.origin, + tenantId, + collectionId: collectionSegment, + projectId: collectionMatch[1], + contentMode: collectionMatch[2], + documentId: hasLocale + ? documentSegment.substring(0, separatorIndex) + : documentSegment, + locale: hasLocale + ? documentSegment.substring(separatorIndex + 1) + : undefined, + } +} diff --git a/src/testutils/createDataEntry.ts b/src/testutils/createDataEntry.ts index 508dcd8..f68ddd9 100644 --- a/src/testutils/createDataEntry.ts +++ b/src/testutils/createDataEntry.ts @@ -1,11 +1,14 @@ import { faker } from '@faker-js/faker' -import { CaaSApi_CMSInputPermission } from '..' import { CaaSApi_FSReference, CaaSApi_MediaRef, CaaSAPI_PermissionActivity, CaaSAPI_PermissionGroup, } from '../types' +import { + createReferenceUrl, + ReferenceUrlOptions, +} from './createReferenceUrl' export function createDataEntry( id = faker.string.uuid(), @@ -26,7 +29,8 @@ export function createDataEntry( export function createMediaPictureReferenceValue( id = faker.string.uuid(), - remoteProject?: string + remoteProjectId?: string, + urlOptions: Omit = {} ): CaaSApi_MediaRef { return { fsType: 'Media', @@ -35,19 +39,25 @@ export function createMediaPictureReferenceValue( uid: id, uidType: 'MEDIASTORE_LEAF', mediaType: 'PICTURE', - url: `${id}-url`, - remoteProject, + url: remoteProjectId + ? createReferenceUrl({ + ...urlOptions, + projectId: remoteProjectId, + documentId: id, + }) + : `${id}-url`, } } export function createMediaPictureReference( id = faker.string.uuid(), - remoteProject?: string + remoteProjectId?: string, + urlOptions: Omit = {} ): CaaSApi_FSReference { return { fsType: 'FS_REFERENCE', name: faker.lorem.word(), - value: createMediaPictureReferenceValue(id, remoteProject), + value: createMediaPictureReferenceValue(id, remoteProjectId, urlOptions), } } diff --git a/src/testutils/createDataset.ts b/src/testutils/createDataset.ts index 72dd8e9..fc6b9ae 100644 --- a/src/testutils/createDataset.ts +++ b/src/testutils/createDataset.ts @@ -1,6 +1,10 @@ import { faker } from '@faker-js/faker' import { CaaSApi_Dataset, CaaSApi_FSDataset } from '../types' import { createDataEntry } from './createDataEntry' +import { + createReferenceUrl, + ReferenceUrlOptions, +} from './createReferenceUrl' export const createDataset = (id?: string): CaaSApi_Dataset => { const base = createDataEntry(id) @@ -30,7 +34,11 @@ export const createDataset = (id?: string): CaaSApi_Dataset => { } } -export const createDatasetReference = (id?: string): CaaSApi_FSDataset => { +export const createDatasetReference = ( + id?: string, + remoteProjectId?: string, + urlOptions: Omit = {} +): CaaSApi_FSDataset => { const base = createDataEntry(id) return { name: faker.lorem.word(), @@ -42,6 +50,15 @@ export const createDatasetReference = (id?: string): CaaSApi_FSDataset => { identifier: base.identifier, entityType: `${base.uid}-schema`, }, + ...(remoteProjectId + ? { + url: createReferenceUrl({ + ...urlOptions, + projectId: remoteProjectId, + documentId: base.identifier, + }), + } + : {}), }, fsType: 'FS_DATASET', } diff --git a/src/testutils/createReferenceUrl.ts b/src/testutils/createReferenceUrl.ts new file mode 100644 index 0000000..008ccd3 --- /dev/null +++ b/src/testutils/createReferenceUrl.ts @@ -0,0 +1,25 @@ +import { faker } from '@faker-js/faker' + +/** + * Builds a CaaS document url in the shape FirstSpirit writes onto references. + * Only the projectId is ever interpreted by the library, the remaining parts + * exist so the url is realistic and parsable. + */ +export interface ReferenceUrlOptions { + baseUrl?: string + tenantId?: string + projectId?: string + contentMode?: string + documentId?: string + locale?: string +} + +export const createReferenceUrl = ({ + baseUrl = 'https://caas.example.com', + tenantId = 'test-tenant', + projectId = faker.string.uuid(), + contentMode = 'preview', + documentId = faker.string.uuid(), + locale = 'en_GB', +}: ReferenceUrlOptions = {}) => + `${baseUrl}/${tenantId}/${projectId}.${contentMode}.content/${documentId}.${locale}` diff --git a/src/testutils/index.ts b/src/testutils/index.ts index 4abb793..f3ceef1 100644 --- a/src/testutils/index.ts +++ b/src/testutils/index.ts @@ -11,4 +11,5 @@ export * from './createPageRef' export * from './createProjectProperties' export * from './createSection' export * from './createPageRefBody' +export * from './createReferenceUrl' export * from './createFsReference' diff --git a/src/types.ts b/src/types.ts index c543e9f..8350b42 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,6 +1,5 @@ import { FSXAContentMode, ImageMapAreaType } from './enums' import { - FSXAProxyApi, FSXARemoteApi, LogLevel, MapResponse, @@ -12,7 +11,7 @@ import { ComparisonQueryOperatorEnum, EvaluationQueryOperatorEnum, LogicalQueryOperatorEnum, -} from './modules/QueryBuilder' +} from './modules' import XMLParser from './modules/XMLParser' export interface MasterLocale { @@ -165,17 +164,16 @@ export interface CaaSApi_ImageMapAreaPoly extends CaaSApi_ImageMapArea { points: Point2D[] } -export interface CaaSApi_ImageMapMedia - extends Pick< - CaaSApi_Media, - | 'fsType' - | 'name' - | 'displayName' - | 'identifier' - | 'uid' - | 'uidType' - | 'mediaType' - > { +export interface CaaSApi_ImageMapMedia extends Pick< + CaaSApi_Media, + | 'fsType' + | 'name' + | 'displayName' + | 'identifier' + | 'uid' + | 'uidType' + | 'mediaType' +> { url: string pictureMetaData: Omit } @@ -196,21 +194,21 @@ export interface CaaSApi_CMSImageMap { } } +export interface CaaSApi_DatasetReference { + fsType: 'DatasetReference' + target: { + fsType: 'Dataset' + schema: string + entityType: string + identifier: string + } + url?: string +} + export interface CaaSApi_FSDataset { fsType: 'FS_DATASET' name: string - value: - | { - fsType: 'DatasetReference' - target: { - fsType: 'Dataset' - schema: string - entityType: string - identifier: string - } - } - | CaaSApi_DataEntry[] - | null + value: CaaSApi_DatasetReference | CaaSApi_DataEntry[] | null } export interface CaaSApi_FSButton { @@ -761,7 +759,7 @@ export type CustomMapper = ( identifier: string, path: NestedPath, remoteProjectId?: string - ) => string + ) => string | null buildPreviewId: (identifier: string, remoteProjectLocale?: string) => string buildMediaUrl: (url: string, rev?: number) => string mapDataEntries: ( @@ -983,15 +981,29 @@ export interface AppContext { fsxaApi?: FSXAApi } +/** + * Configures which other FirstSpirit projects this application resolves + * references into, and in which locale. The key is a free name, used by the + * `remoteProject` parameter of `fetchElement`; `id` is the project's uuid. + * See the README section "Resolving references across projects". + */ export type RemoteProjectConfiguration = { - [name: string]: { - id: string - locale: string - } + [name: string]: RemoteProjectConfigurationEntry } -export type RemoteProjectConfigurationEntry = - RemoteProjectConfiguration[keyof RemoteProjectConfiguration] +export type RemoteProjectConfigurationEntry = { + id: string + /** + * every reference into this project is resolved in this locale. Required + * unless `useSourceLocale` is set. + */ + locale?: string + /** + * resolve references into this project in the locale of the requested + * element, ignoring `locale`. Defaults to false. + */ + useSourceLocale?: boolean +} export interface CaasItemFilterParams extends MapResponse { filterContext?: FilterContextType