Skip to content

test(test): serve CDA locales, a root mount, and skip paging from mocks [NT-3946] - #450

Merged
Felipe Mamud (fmamud) merged 1 commit into
mainfrom
nt-3946-mocks-cda-locales-root
Aug 19, 2026
Merged

test(test): serve CDA locales, a root mount, and skip paging from mocks [NT-3946]#450
Felipe Mamud (fmamud) merged 1 commit into
mainfrom
nt-3946-mocks-cda-locales-root

Conversation

@nalchevanidze

Copy link
Copy Markdown
Contributor

Extracted from #429 so it can merge first: #429's iOS work depends on these mock routes at runtime.

The mock CDA only ever had to satisfy contentful.js. The Android and iOS SDKs read the CDA with their own platform clients, and the iOS reference app is moving onto contentful.swift, which reaches the mock in ways the handlers did not support. Three gaps, and why each one blocks or degrades a native client.

Locales

contentful.swift resolves locale fallback chains client-side. Client.fetchLocalesIfNecessary requests /locales before any other endpoint, and entries cannot be decoded until it succeeds, so a stock Contentful.Client failed on its first entry query against the mock. The new route is built from the locale set already in the space fixture and honors the limit of 1000 the SDK sends.

contentful.js sends locale to the API instead and never requests the endpoint, which is why the gap stayed invisible while only web consumed the mock.

Root mount

Contentful.Client builds every request as scheme://host[:port]/spaces/…, and ClientConfiguration exposes no base-path hook, so it cannot address the /contentful/ namespace the mock server used. The same handlers are now also mounted at the host root.

contentful.js has basePath and the Android app builds its URLs from AppConfig.contentfulBaseUrl, so both keep using the prefixed mount unchanged. The iOS app can now differ from a production integration only by host and secure = false, instead of installing a URLProtocol shim.

Skip

The content-type query parsed skip and then ignored it while echoing a skip of zero back, so any page after the first returned the first page again and a paging consumer accumulated duplicates. The clients that page this way are the native ones: fetchAllEntries in the iOS SDK and the URL built by the Android PreviewContentfulClient both walk skip, and contentful.swift pages the same way.

With 45 fixture entries and a 100-item batch nothing pages today, so this is a latent fidelity gap rather than a live failure.

Scope and compatibility

Both routes are additive and no existing consumer changes mounts. The first consumer of the new behavior is the iOS reference app's move to contentful.swift in #429, which is stacked on this branch.

Per lib/mocks/AGENTS.md, the /locales route and root mount have no in-tree consumer until #429 lands. They are included here so #429 does not carry mock infrastructure changes.

Validation

  • pnpm --filter mocks typecheck — clean. Per lib/mocks/AGENTS.md this is the only direct validation for mocks.
  • pnpm lint — clean.
  • pnpm format:check — clean.
  • No consumer test run: mock behavior is validated only incidentally through a consuming test, and no in-tree consumer requests /locales or skip > 0 today. The skip change is a no-op for skip=0 callers, and web consumers keep the prefixed mount.

🤖 Generated with Claude Code

…ks [NT-3946]

The mock CDA only ever had to satisfy contentful.js. The Android and iOS SDKs read
the CDA with their own platform clients, and the iOS reference app is moving onto
contentful.swift, which reaches the mock in ways the handlers did not support.
Three gaps, and why each one blocks or degrades a native client:

Locales. contentful.swift resolves locale fallback chains client-side.
Client.fetchLocalesIfNecessary requests /locales before any other endpoint, and
entries cannot be decoded until it succeeds, so a stock Contentful.Client failed
on its first entry query against the mock. The new route is built from the locale
set already in the space fixture and honors the limit of 1000 the SDK sends.
contentful.js sends locale to the API instead and never requests the endpoint,
which is why the gap stayed invisible while only web consumed the mock.

Root mount. Contentful.Client builds every request as scheme://host[:port]/spaces/…
and ClientConfiguration exposes no base-path hook, so it cannot address the
/contentful/ namespace the mock server used. The same handlers are now also
mounted at the host root. contentful.js has basePath and the Android app builds
its URLs from AppConfig.contentfulBaseUrl, so both keep using the prefixed mount
unchanged; the iOS app can now differ from a production integration only by host
and secure = false, instead of installing a URLProtocol shim.

Skip. The content-type query parsed skip and then ignored it while echoing a skip
of zero back, so any page after the first returned the first page again and a
paging consumer accumulated duplicates. The clients that page this way are the
native ones: fetchAllEntries in the iOS SDK and the URL built by the Android
PreviewContentfulClient both walk skip, and contentful.swift pages the same way.
With 45 fixture entries and a 100-item batch nothing pages today, so this is a
latent fidelity gap rather than a live failure.

Both routes are additive and no existing consumer changes mounts. The first
consumer of the new behavior is the iOS reference app's move to contentful.swift,
which lands on top of this.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bito-code-review

bito-code-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

Code Review Agent Run #4b43d3

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: cdff76e..cdff76e
    • lib/mocks/README.md
    • lib/mocks/src/contentful-handlers.ts
    • lib/mocks/src/server.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at jared.jolton@contentful.com.

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review

Copy link
Copy Markdown

Functional Validation by Bito

SourceRequirement / Code AreaStatusNotes
NT-3946Add /locales endpoint to mock CDA to support iOS SDK's Client.fetchLocalesIfNecessary method✅ MetThe /locales endpoint is implemented in lib/mocks/src/contentful-handlers.ts via handleLocalesQuery function (lines 77-103). The endpoint reads locale data from the space fixture at lib/mocks/src/contentful/data/space/ctfl-space-data.json and honors the 1000-item limit via LOCALES_LIMIT constant. The handler route is registered at `spaces/:spaceId/environments/:environmentId/locales` and returns properly paginated responses with CORS headers.
NT-3946Mount mock CDA handlers at host root in addition to /contentful/ namespace✅ MetThe mock CDA handlers are mounted at host root in lib/mocks/src/server.ts via `...getContentfulHandlers('*/')` (line 133). This enables iOS SDK clients (contentful.swift) to address the mock server without URLProtocol shims, since their Client builds requests as `scheme://host[:port]/spaces/...` with no base-path hook. The existing namespace mount at `/contentful/` is preserved for contentful.js and Android clients.
NT-3946Fix skip parameter handling in content-type query to properly respect non-zero skip values✅ MetThe skip parameter handling is fixed in lib/mocks/src/contentful-handlers.ts within handleContentTypeQuery (lines 48-52, 61). The code now properly parses and validates the skip parameter, applies it via `filtered.slice(skip, skip + limit)` for correct pagination, and echoes back the actual skip value in the response instead of always returning 0. This prevents SDK paging consumers from silently looping over the same first page.

@bito-code-review

Copy link
Copy Markdown

Impact Analysis by Bito

Cross-Repository Impact Analysis
What Changed Impact of Change Suggested Review Actions
Added new /locales endpoint and dual-mount for Contentful CDA mock handlers - agents-api/services/api/test/mocks/contentful-handlers.mock.ts:40-44 CONSUMES optimization.lib/mocks locales pattern: The optimization mock now serves /locales endpoint with data from space fixture. The agents-api test mocks should be reviewed for consistency with the new endpoint format (sys.type, total, skip, limit, items fields).
- agents-api/packages/exo-agent/src/tools/bindings/search/utils/entry-card-client.ts:13-18 CONSUMES optimization.lib/mocks /locales: exo-agent's fetchDefaultLocale function calls the /locales endpoint. The new handler in lib/mocks will properly serve this for Swift clients and any tests that use the mock server.
- experience-packages tests/widget-testing/cypress/integration/reusable/locales-api.spec.ts CONSUMES optimization.lib/mocks /locales: Widget testing Cypress tests import testLocaleApi which tests the locales endpoint. These may use the shared mock server.
- Verify the new /locales endpoint response format (sys.type: 'Array', total, skip, limit, items) matches what agents-api test fixtures expect
- Ensure the dual-mount at both /contentful/ and / does not cause route conflicts in tests
- Check if experience-packages locales-api.spec tests need updates for the new endpoint response structure
Added skip parameter support for entries pagination in mock handlers - No cross-repo consumers found for pagination change: The entries endpoint pagination with skip was already semantically broken in the mock (always returned first page). This fix makes the mock behavior match real CDA pagination behavior. No direct cross-repo callers explicitly depend on the broken skip behavior. - Test that pagination across pages works correctly with the new skip/limit support
Code Paths Analyzed

Impact:
Enhances the mock Contentful CDA server with locale endpoint support and proper pagination, enabling Swift SDK client testing and fixing pagination behavior for all CDA mock consumers.

Flow:
Entry point: server.ts registers getContentfulHandlers twice (prefixed + root) -> contentful-handlers.ts handles requests -> /locales route -> handleLocalesQuery reads from space fixture JSON -> entries route -> properly paginated with skip parameter

Direct Changes (Diff Files):
• lib/mocks/README.md [9-23] — Added documentation explaining the dual-mount Contentful CDA endpoints: /contentful/ for contentful.js/Android clients, and / at host root for contentful.swift clients
• lib/mocks/src/contentful-handlers.ts [37-103] — Added SPACE_DATA_PATH and LOCALES_LIMIT constants; added skip parameter support for entries pagination; added handleLocalesQuery function; registered /locales HTTP handler
• lib/mocks/src/server.ts [129-133] — Added second getContentfulHandlers call at host root '*/' to serve contentful.swift SDK clients that cannot use basePath

Repository Impact:
lib/mocks contentful handlers: New locale endpoint, proper pagination, and dual-mount configuration enhance the mock server's fidelity to real CDA behavior

Cross-Repository Dependencies:
agents-api test mocks: The new /locales endpoint format should be compared with the existing agents-api mock to ensure compatibility
agents-api exo-agent locale fetching: fetchDefaultLocale in entry-card-client.ts uses /locales endpoint - the new mock handler enables proper testing for this code path
experience-packages widget tests: Tests use testLocaleApi which tests locales endpoint - may need alignment with new mock behavior

Database/Caching Impact:
• None

API Contract Violations:
• Potential mismatch: the new /locales handler returns sys.type: 'Array' in response, while the existing agents-api mock uses createCollectionFixture which may have different structure

Infrastructure Dependencies:
• Requires ctfl-space-data.json fixture file at src/contentful/data/space/ containing locales array

Additional Insights:
SDK client compatibility: Dual-mount enables contentful.swift clients (which have no basePath option) to use the mock server, expanding test coverage for Swift SDK behavior
Pagination correctness: Previous mock always returned skip: 0, causing paging consumers to loop over first page indefinitely. The fix correctly implements skip/limit pagination matching real CDA behavior

Testing Recommendations

Frontend Impact:
• Test that locale fallback chain resolution works correctly with the new /locales endpoint (Swift SDK clients require this before entry queries)
• Verify dual-mount doesn't cause route conflicts when both /contentful/ and / paths are available

Service Integration:
• Test entries pagination with various skip/limit combinations: skip=0&limit=10, skip=10&limit=10, skip=20&limit=10
• Verify /locales returns correct CDA Array response format (sys.type, total, skip, limit, items)

Data Serialization:
• Test handleLocalesQuery with missing or malformed ctfl-space-data.json file - should return 500 error gracefully
• Test locale handler with limit parameter (default 1000, respects custom values)

Privacy Compliance:
None.

Backward Compatibility:
• Verify existing tests that don't pass skip parameter still work (defaults to 0)
• Ensure dual-mount doesn't break existing /contentful/ path consumers

OAuth Functionality:
• None

Cross-Service Communication:
• Verify entry-card-client.ts fetchDefaultLocale works with new mock locales format

Reliability Testing:
• None

Additional Insights:
• Add integration test for Swift SDK client connecting to root mount /spaces/:spaceId/environments/:environmentId/locales
• Test pagination edge cases: skip beyond total items (returns empty items array), negative skip (defaults to 0), non-integer skip (defaults to 0)

Analysis based on known dependency patterns and edges. Actual impact may vary.

@bito-code-review

Copy link
Copy Markdown

✅ Review Settings Overridden

Status: Overridden Successfully

Guidelines:

  • Accepted:

    • General : Review Posture, Repo Truth And Boundaries, Domain Invariants

    • Language : typescript= TypeScript Review Evidence

Note: Extra guidelines beyond 3 general purpose guidelines and 1 language specific guideline per language are not processed. Guidelines are fetched from the source branch.

Comment thread lib/mocks/README.md
@fmamud
Felipe Mamud (fmamud) merged commit 7ee085b into main Aug 19, 2026
39 checks passed
@fmamud
Felipe Mamud (fmamud) deleted the nt-3946-mocks-cda-locales-root branch August 19, 2026 10:39
Felipe Mamud (fmamud) added a commit that referenced this pull request Aug 19, 2026
…roid's in-app synthesis

The mock CDA gained a real `/locales` route in #450 for iOS's contentful.swift
migration. Android's `LocalesInterceptor` still synthesized both `/locales` and
`/content_types` in-app because neither existed on the mock when it was written.

`/locales` is now redundant and removed outright: a real request succeeds.

`/content_types` is NOT redundant on its own — contentful.java's
`ResourceFactory.array()` unconditionally calls `RichTextFactory.resolveRichTextField`
on every array response, which unconditionally calls `ResourceUtils.ensureContentType`
for every entry, regardless of whether that entry has a Rich Text field, and throws
`CDAContentTypeNotFoundException` on a 404. The reference app's fixtures do have Rich
Text fields (RichText.kt is exercised by ContentEntryView/NestedContentEntryView), so
this path is genuinely load-bearing.

Rather than keep the in-app fixture shim, add a `/content_types` route to the mock
server that serves `ctfl-space-data.json`'s `contentTypes` array, mirroring the
`/locales` route's shape and #450's precedent of fixing the mock rather than shimming
around it in a file customers copy from. This lets Android's bundled
`content_types.json`/`ContentTypesFixture` be deleted entirely.

With both interceptor-synthesized routes gone, MockContentfulClient.kt no longer earns
its own file: consolidate the shared CDAClient and the fetch-by-ids loop into
ContentfulFetcher.kt, mirroring iOS's single ContentfulClient.swift shape.

Validated: `pnpm --filter mocks typecheck`, `pnpm lint`, `pnpm format:check` clean;
both APKs assemble; full Maestro suite passes 70/70 flows across Compose and Views.
Felipe Mamud (fmamud) added a commit that referenced this pull request Aug 19, 2026
…ntentful.java CDA SDK (NT-3947) (#439)

* feat(implementations): migrate Android reference implementation to contentful.java CDA SDK (NT-3947)

Replace hand-rolled HTTP/JSON Contentful fetching in the Android reference
implementation with Contentful's official com.contentful.java:java-sdk
CDAClient, mirroring the iOS (NT-3946) migration.

- MockContentfulClient builds the shared CDAClient against the mock server
  and synthesizes the /locales and /content_types responses CDAClient
  requires (mock server only serves /entries), bundling real content-type
  definitions so Rich Text field resolution can find them.
- ContentfulFetcher now fetches by-ID entries through CDAClient (single
  locale, include=10) and decodes them with the SDK's typed entry APIs
  instead of hand-rolled JSON parsing/link resolution.
- MockPreviewContentfulClient wraps the same CDAClient for the preview
  panel's audience/experience fetch, fixing two regressions found while
  validating against the full Maestro suite:
  - Concurrent nt_audience/nt_experience fetches against the same shared
    CDAClient could interleave and drop entries (CDAClient mutates shared
    instance state while a call is in flight); calls are now serialized
    with a Mutex.
  - The mock's content-type-filtered entries endpoint never returns an
    includes section, so contentful.java silently drops any Link field it
    can't resolve rather than keeping it as a stub, breaking the preview
    panel's audience/experience grouping; raw link stubs are now restored
    from CDAEntry.rawFields() when contentful.java drops them.

Verified with the full Compose and Views Maestro suites (35/35 passing on
both), including all preview-panel-overrides scenarios.

* refactor(implementations): favor typed CTEntry over Map in Android reference implementation

Rework the app-owned entry-rendering components (ContentEntryView,
NestedContentEntryView, ContentEntryViewBinder, NestedContentEntryViewBinder,
MainScreen, MainActivity) to accept and pass CTEntry instead of raw
Map<String, Any>, mirroring the SDK's own typed OptimizedEntry(entry: CDAEntry)
entry point. Also drops redundant nullable type arguments on getField calls.

Key CTEntry, having no structural equals/hashCode, are compared by reference:
remember/LaunchedEffect keys in ContentEntryView and NestedContentEntryView
now key on entry.toMap() (structurally comparable) instead of the entry
instance or its id, restoring correct re-resolution of merge-tag text after
identify() calls.

* fix(implementations): serve CDA content types from the mock, drop Android's in-app synthesis

The mock CDA gained a real `/locales` route in #450 for iOS's contentful.swift
migration. Android's `LocalesInterceptor` still synthesized both `/locales` and
`/content_types` in-app because neither existed on the mock when it was written.

`/locales` is now redundant and removed outright: a real request succeeds.

`/content_types` is NOT redundant on its own — contentful.java's
`ResourceFactory.array()` unconditionally calls `RichTextFactory.resolveRichTextField`
on every array response, which unconditionally calls `ResourceUtils.ensureContentType`
for every entry, regardless of whether that entry has a Rich Text field, and throws
`CDAContentTypeNotFoundException` on a 404. The reference app's fixtures do have Rich
Text fields (RichText.kt is exercised by ContentEntryView/NestedContentEntryView), so
this path is genuinely load-bearing.

Rather than keep the in-app fixture shim, add a `/content_types` route to the mock
server that serves `ctfl-space-data.json`'s `contentTypes` array, mirroring the
`/locales` route's shape and #450's precedent of fixing the mock rather than shimming
around it in a file customers copy from. This lets Android's bundled
`content_types.json`/`ContentTypesFixture` be deleted entirely.

With both interceptor-synthesized routes gone, MockContentfulClient.kt no longer earns
its own file: consolidate the shared CDAClient and the fetch-by-ids loop into
ContentfulFetcher.kt, mirroring iOS's single ContentfulClient.swift shape.

Validated: `pnpm --filter mocks typecheck`, `pnpm lint`, `pnpm format:check` clean;
both APKs assemble; full Maestro suite passes 70/70 flows across Compose and Views.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants