refactor(implementations): fetch iOS reference entries with contentful.swift [NT-3946] - #429
Conversation
…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>
The tap observer added for NT-3829 attached its UITapGestureRecognizer to its `superview`, assuming `.background()` makes the observer a sibling of a content *view*. SwiftUI draws Text and friends into a shared display list rather than one UIView per view, and places the observer in a container of its own, so that container is not reliably an ancestor of the region the user taps and the recognizer never saw the touch. No `component_click` event was emitted from the SwiftUI shell at all. Attach to the window instead, which is always an ancestor, and scope each tap to the observer's own frame — `.background()` sizes it to the tracked content. `cancelsTouchesInView`, `delaysTouchesBegan`, and `delaysTouchesEnded` are all off, so observing from the window still never competes for touches a nested interactive child needs, preserving the NT-3829 fix. Window-level recognizers need removing, so detach on window change and deinit. The nested-Button regression test added by NT-3829 only asserts the Button's own action fires, so it passed either way; the two XCUITests that assert the click event are not in CI, which runs only PreviewPanelTests for iOS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… [NT-3946] `CTEntry` was constructible by consumers only from a `Contentful.Entry`; `init(any:)` and `init(json:)` were internal, so a caller holding an entry as a dictionary — an expanded child entry, a locally built one — could not wrap it to read fields through `getField`/`hasField`. Android already exposes all three shapes (`CTEntry.from(CDAEntry)`, `from(Map)`, `from(String)`), so this closes an iOS/Android gap rather than adding new API surface. The dictionary initializer takes `[String: Any]` instead of the internal `Any` overload, matching Android's `Map` signature, and stays fail-soft; `init(json:)` keeps throwing, which is the idiomatic Swift equivalent of Android's fallback parameter. `empty` becomes public because Swift rejects an internal declaration in a public default argument, where Kotlin permits it — hence Android keeping `EMPTY` internal. Note that SDK APIs still accept entries as dictionaries, not `CTEntry`, so these initializers are for reading an entry rather than passing one back into resolution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l.swift [NT-3946] The iOS reference app fetched Contentful entries with hand-rolled HTTP: a URL string, URLSession, JSONSerialization into untyped dictionaries, and its own `includes` link resolver with a depth-10 hop budget. Reference implementations are meant to consume the public SDK surface the way customers do, and the recommended path is Contentful's official CDA SDK — which the Optimization SDK already ships an adapter for (NT-3808, #393) without any reference app using it. Both shells now fetch with contentful.swift and hand the resulting `Contentful.Entry` to the SDK's typed entry points: `OptimizedEntry(entry:)` in SwiftUI and `resolveOptimizedEntry(baseline:)` in UIKit. App-owned CDA request construction, JSON parsing, and link resolution are deleted; link resolution and cycle handling now come from contentful.swift's LinkResolver plus the SDK's `CTEntry` encoding. `isNestedContent` collapses to `entry.sys.contentTypeId`. Requests stay single-locale via `localizeResults(withLocaleCode:)` with `include(10)`, per the CDA entry contract. The preview panel's client is migrated too, mapping results back down with `CTEntry.toDictionary()` for the dictionary-shaped `PreviewContentfulClient` protocol. Expanded child entries and locally built test entries only exist as dictionaries, so the nested renderers and the dictionary initializers stay — `CTEntry` cannot be constructed from a dictionary at every SDK input boundary, since those accept dictionaries rather than `CTEntry`. contentful.swift 5.5.15 cannot express the mock server's `/contentful/` path prefix (it builds `/spaces/...` from the host root) and fetches `/locales` before its first entry request, which the mock does not serve. Rather than change test infrastructure shared with Android and the web SDKs, `MockContentfulTransport` adapts both locally. It is demo-only plumbing: a production app builds `Contentful.Client(spaceId:accessToken:)` against cdn.contentful.com and needs none of it, which its doc comment and the README both state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…[NT-3946] Fold MockContentfulTransport into MockContentfulClient as a private nested Transport. The URLProtocol shim is only meaningful as part of the client it is installed on, and keeping the mock-only plumbing in one file leaves ContentfulFetcher readable as the integration a customer would copy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lient [NT-3946] Merge ContentfulFetcher and MockPreviewContentfulClient into a single ContentfulClient that owns the contentful.swift client, exposes the app-owned entry-ID fetch as a static API, and conforms to PreviewContentfulClient so the preview panel reads through the same client. Drop the Mock prefix: the type is production-shaped code, and only its nested Transport targets the mock server. Sharing one client also avoids repeating the /locales bootstrap that contentful.swift performs per client before it can decode entries. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-3946] The mock server now serves the CDA from the host root and exposes /locales, so contentful.swift can talk to it directly. ContentfulClient no longer needs its bespoke URLProtocol transport and differs from a production integration only by host and secure = false. Requires the mock locales route and root mount. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lients [NT-3946] PreviewContentfulClient is dictionary-shaped because loadDefinitions serializes its result for the JS core, which runs the shared entry mappers. Apps that already read Contentful through contentful.swift therefore had to encode typed entries back down themselves, even though the SDK already depends on contentful.swift and exposes Contentful.Entry publicly. ContentfulSDKPreviewClient wraps an existing Contentful.Client and owns that mapping, so the panel shares the app's client configuration and session instead of opening a second connection. Choose it when the app already has a Contentful client; ContentfulHTTPPreviewClient stays for apps that do not. The iOS reference app now passes ContentfulClient.previewClient and implements no entry mapping of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-3946] PreviewPanelConfig, PreviewPanelOverlay, PreviewPanelViewController, and addFloatingButton each gain an overload taking the app's contentful.swift client and wrap it internally, so an app that already reads Contentful shares one client and implements no entry mapping. ContentfulSDKPreviewClient becomes internal because no caller needs to name it; the public PreviewContentfulClient protocol is unchanged, so custom conformers and ContentfulHTTPPreviewClient keep working. The iOS reference app drops its previewClient wrapper and shared singleton: one static ContentfulClient.client feeds both preview panel surfaces, and the duplicate fetchEntries(matching:) bridge folds into fetchEntry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ulClient [NT-3946]
The adapter is internal, so a dedicated file for one 40-line type was the
outlier in Preview/: PreviewContentfulClient.swift already holds the protocol,
both response structs, the HTTP implementation, the error enum, and the
pagination helpers. It now sits under its own MARK beside the HTTP sibling.
Keeping the type is load-bearing, not habit: Swift rejects an internal witness
for a public protocol requirement on an imported type ("must be declared
public"), so conforming Contentful.Client directly would publish
getEntries(contentType:include:skip:limit:) on every consumer's client.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6fd2402 to
e1b84aa
Compare
The base branch was changed.
|
| Source | Requirement / Code Area | Status | Notes |
|---|---|---|---|
| NT-3932, NT-3946 | Fetch iOS reference entries with contentful.swift CDA SDK | ✅ Met | The diff replaces hand-rolled HTTP with shared/ContentfulClient.swift using `contentful.swift` Client. Fetching uses `Query.where(sys: .id, .equals(id)).include(10).localizeResults(withLocaleCode:)` (lines 462-464). Both SwiftUI (`MainScreen` at line 909) and UIKit (`MainViewController` at line 1307) call `ContentfulClient.fetchEntries()` instead of the removed `ContentfulFetcher`. The `contentful.swift` v5.5.15 dependency is added to project.yml and project.pbxproj. |
| NT-3932, NT-3946 | Pass typed Contentful.Entry objects through the Optimization SDK's typed entry APIs | ✅ Met | The diff exercises the SDK's typed entry APIs. SwiftUI's ContentEntryView.swift passes `Contentful.Entry` to `OptimizedEntry(entry:)` (lines 727-728). UIKit's OptimizedEntryUIView.swift calls `client.resolveOptimizedEntry(baseline: entry)` through the typed overload (lines 1189-1193). The CTEntry bridge (line 1342-1344) encodes `Contentful.Entry` through `CDA.Entry(contentfulEntry, ancestors: [])`. Custom CDA request building and JSON parsing are removed — ContentfulFetcher.swift and MockPreviewContentfulClient.swift are deleted. |
| NT-3994, NT-3946 | Provide ContentfulSDKPreviewClient wrapper implementing PreviewContentfulClient protocol | ✅ Met | ContentfulSDKPreviewClient implements `PreviewContentfulClient` (lines 1410-1448), encoding `Contentful.Entry` through `CTEntry($0).toDictionary()`. The SDK exposes `Contentful.Client` overloads on [[STRUCT:PreviewPanelConfig]] (lines 1482-1487), [[STRUCT:PreviewPanelOverlay]] (lines 1521-1526), PreviewPanelViewController (lines 1565-1570), and `addFloatingButton` (lines 1590-1599). Both app targets pass `ContentfulClient.client` directly — no manual `CTEntry`-to-dictionary mapping needed. |
| NT-3829 | Fix SwiftUI tap tracking by attaching observer to window | ✅ Met | The SwiftUI tap tracking fix in TapTrackingModifier.swift attaches the observer to the window (lines 1645-1651) instead of the superview. The window-level recognizer scopes each tap to the observer's frame via `bounds.contains(recognizer.location(in: self))` (line 1685), with `cancelsTouchesInView` and `delaysTouchesBegan/Ended` all off (lines 1659-1661) so nested-button behavior is preserved. The updated comment (lines 1615-1625) explains why superview attachment was unreliable. |
| NT-3808 | Ship Entry to OptimizedEntry conversion path in SDK with type-safe public surface | ✅ Met | The SDK ships three entry construction paths matching Android's `CTEntry.from(...)` APIs: `init(_: Contentful.Entry)` (line 1342), `init(dictionary:fallback:)` (line 1354), and `init(json:)` (line 1362). `CTEntry.empty` is now `public static let` (line 1340). Documentation confirms metadata requirement is immutable — every initializer goes through the same `CDA.Entry` path (lines 72-79 in ios.md). Type safety improves as apps move from `as? [String: Any]` casts to `getField`/`hasField` and subscript access on `CTEntry`. |
| NT-3932 | Preserve existing loading, fallback, personalization, nested-entry, live-update, preview-panel, and tracking behavior | ✅ Met | All existing functionality is preserved. Live updates support remains with `Contentful.Entry` types in LiveUpdatesTestScreen.swift (lines 843-886) and LiveUpdatesTestViewController.swift (lines 1260-1284). Nested entry handling uses the `Source` enum pattern for both fetched and expanded entries (lines 767-786 in SwiftUI, 976-987 in UIKit). Preview panel behavior is maintained through `ContentfulSDKPreviewClient`. Tracking remains functional — `TrackingMetadata` still receives dictionary-form entries. |
| NT-3932 | Update implementation READMEs and ensure Xcode project stays synchronized with project.yml | ✅ Met | implementations/ios-sdk/README.md is updated with a new 'Contentful entry fetching' section explaining the `contentful.swift` integration and `ContentfulClient` usage (lines 295-321). project.yml declares `contentful.swift` with exact version 5.5.15 (lines 346-352) and both targets depend on it (lines 360-361, 369-370). project.pbxproj is synchronized with the package reference and product dependencies for both SwiftUI and UIKit targets (lines 220, 254-281). |
| NT-3946 | Add /locales route to mock server | 🟡 Partial | [Non-Diff Requirement] The mock server implementation is external to the provided diff. The README documents that `lib/mocks` serves `/locales` (line 320), and ContentfulClient is configured for it, but the actual server endpoint code is not shown in this diff. Validation requires inspecting the mock server implementation separately. |
| NT-3808 | Replace single-shot JSONSerialization with per-field handling in resolveOptimizedEntry | 🟡 Partial | [Non-Diff Requirement] Per-field error handling in `resolveOptimizedEntry` is internal SDK implementation not visible in the reference app diff. The change description mentions moving away from single-shot JSONSerialization, but the actual implementation of per-field logging would be in the SDK package internals, not the reference app changes shown here. |
| NT-3932, NT-3829 | Verify all existing iOS XCUITest and Android Maestro test suites pass for both UI frameworks | 🟡 Partial | [Non-Diff Requirement] Test suite validation requires runtime execution, not just source code review. The diff modifies test targets in project.pbxproj (removing `ContentfulFetcher`/`MockPreviewContentfulClient` references, adding `ContentfulClient`) and adds `CTEntryTests` coverage for the new public initializers (lines 1699-1743). Actual test execution to verify 69/69 SwiftUI tests and Android Maestro tests pass is outside the scope of diff review. |
Impact Analysis by BitoCross-Repository Impact Analysis
Code Paths AnalyzedImpact: Flow: Direct Changes (Diff Files): Repository Impact: Cross-Repository Dependencies: Database/Caching Impact: API Contract Violations: Infrastructure Dependencies: Additional Insights: Testing RecommendationsFrontend Impact: Service Integration: Data Serialization: Privacy Compliance: Backward Compatibility: OAuth Functionality: Cross-Service Communication: Reliability Testing: Additional Insights: Analysis based on known dependency patterns and edges. Actual impact may vary. |
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #af13dc
Actionable Suggestions - 1
-
implementations/ios-sdk/OptimizationApp.xcodeproj/project.pbxproj - 1
- Missing Contentful file reference entry · Line 134-134
Filtered by Review Rules
Bito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
-
implementations/ios-sdk/shared/MockPreviewContentfulClient.swift - 1
- Non-existent file referenced in diff · Line 1-73
Review Details
-
Files reviewed - 29 · Commit Range:
cd302fa..99db9e1- documentation/guides/integrating-the-optimization-ios-sdk-in-a-swiftui-app.md
- documentation/guides/integrating-the-optimization-ios-sdk-in-a-uikit-app.md
- documentation/internal/sdk-knowledge/native/ios.md
- implementations/ios-sdk/OptimizationApp.xcodeproj/project.pbxproj
- implementations/ios-sdk/README.md
- implementations/ios-sdk/project.yml
- implementations/ios-sdk/shared/Config.swift
- implementations/ios-sdk/shared/ContentfulClient.swift
- implementations/ios-sdk/shared/ContentfulFetcher.swift
- implementations/ios-sdk/shared/MockPreviewContentfulClient.swift
- implementations/ios-sdk/shared/RichText.swift
- implementations/ios-sdk/swiftui/App.swift
- implementations/ios-sdk/swiftui/Components/ContentEntryView.swift
- implementations/ios-sdk/swiftui/Components/NestedContentEntryView.swift
- implementations/ios-sdk/swiftui/Screens/LiveUpdatesTestScreen.swift
- implementations/ios-sdk/swiftui/Screens/MainScreen.swift
- implementations/ios-sdk/uikit/Components/ContentEntryUIView.swift
- implementations/ios-sdk/uikit/Components/NestedContentEntryUIView.swift
- implementations/ios-sdk/uikit/Components/OptimizedEntryUIView.swift
- implementations/ios-sdk/uikit/SceneDelegate.swift
- implementations/ios-sdk/uikit/Screens/LiveUpdatesTestViewController.swift
- implementations/ios-sdk/uikit/Screens/MainViewController.swift
- packages/ios/ContentfulOptimization/Sources/ContentfulOptimization/Contentful/CTEntry.swift
- packages/ios/ContentfulOptimization/Sources/ContentfulOptimization/Preview/PreviewContentfulClient.swift
- packages/ios/ContentfulOptimization/Sources/ContentfulOptimization/Preview/PreviewPanelConfig.swift
- packages/ios/ContentfulOptimization/Sources/ContentfulOptimization/Preview/PreviewPanelOverlay.swift
- packages/ios/ContentfulOptimization/Sources/ContentfulOptimization/Preview/PreviewPanelViewController.swift
- packages/ios/ContentfulOptimization/Sources/ContentfulOptimization/Tracking/TapTrackingModifier.swift
- packages/ios/ContentfulOptimization/Tests/ContentfulOptimizationTests/CTEntryTests.swift
-
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
3 participants
Why
The iOS reference app fetched Contentful entries with hand-rolled HTTP — a URL string,
URLSession,JSONSerializationinto untyped dictionaries, and its own link resolver. Reference implementations are meant to consume the SDK surface the way customers do, and the recommended path is Contentful's official CDA SDK. The Optimization SDK has shipped the adapter for it since NT-3808 (#393); no reference app used it.Closes NT-3946.
Decisions
Fetch with
contentful.swift, hand the typed entry to the SDK. Both shells query withQuery/include(10)/localizeResults(withLocaleCode:)and passContentful.EntrytoOptimizedEntry(entry:)(SwiftUI) andresolveOptimizedEntry(baseline:)(UIKit). App-owned request building, JSON parsing, and link resolution are deleted — the SDK'sCTEntryencoding andLinkResolverown that now, including cycle handling that replaces the old depth-10 hop budget.One client for every CDA read.
shared/ContentfulClient.swiftowns the singleContentful.Client, exposesContentfulClient.fetchEntries(ids:locale:), and conforms toPreviewContentfulClientso the preview panel reads through the same client — which is the integration that protocol documents, as opposed to the built-inContentfulHTTPPreviewClient. Sharing one client also avoids repeating the/localesbootstrapcontentful.swiftperforms per client. NoMockprefix: nothing in it is mock-specific beyond two arguments.Fix the mock server rather than shim around it in the app. See below.
Two SDK fixes ride along, both prerequisites found while migrating:
main. The NT-3829 observer attached itsUITapGestureRecognizertosuperview, but SwiftUI draws content into a shared display list and puts the observer in its own container, so that container isn't reliably an ancestor of the tapped region — the SwiftUI shell emitted nocomponent_clickevents at all. Now attached to the window and scoped to the observer's frame, with all touch-stealing options off so NT-3829's nested-button behaviour is preserved by construction.CTEntrydictionary/JSON initializers made public, matching Android'sfrom(Map)/from(String). SDK APIs still take entries as dictionaries, so these are for reading an entry, not for passing one back into resolution.Why the mock changes were necessary
An earlier revision of this PR carried a 138-line in-app
URLProtocolshim instead. Two properties oflib/mocksmade the SDK unusable against it, and neither is expressible as aContentful.Clientargument:/localesroute.contentful.swiftresolves locale fallback chains client-side, so it fetches/localesbefore its first entry query and completes that query with the locales error if it fails (Client.swift:203-217); every entry decode then hard-requires the resultingLocalizationContext. Real CDA has this endpoint and the mock never did — while the locale set sat unused inctfl-space-data.json. The handler now serves it./contentful/. The prefix exists so one port can also serve/experience/and/insights/.Contentful.Clienttakes only ahost[:port]and builds/spaces/...from the root, with nobasePath— the knobcontentful.jshas and uses (web-sdk_react/src/services/contentfulClient.ts). The CDA is now also mounted at the host root;/contentful/stays for Android, web, and React Native.Fixing the mock was the better trade because the shim was demo-only plumbing sitting in a file customers are meant to copy from, and because the
/localesgap is a genuine fidelity hole rather than an iOS quirk. The result:ContentfulClientdiffers from a production integration by exactlyhostandclientConfiguration.secure = false.While in there,
skipwas silently ignored in the content-type query (skip: 0echoed back,slice(0, limit)), so every page returned the first one and paging consumers looped over the same entries. Now honored.Validation
pnpm ios:testpnpm lint,format:check, mocks typecheck + unitSwiftUI went 67/69 → 69/69: the two failures were the pre-existing tap-tracking bug.
Two assumptions carry the least local coverage: msw's
*spanning slashes for the root mount (if wrong, iOS entry fetches 404 and the iOS jobs fail loudly), andskipnow genuinely paginating (any scenario that had encoded the old bug will change).For reviewers
fix(swift)changes get no Swift changelog entries. Happy to split.PreviewPanelTests, which is why a fully broken tap-tracking path survived onmain. Widening it is a cheap separate improvement.MockPreviewContentfulClient.kt, so the platforms now differ on that type's name. Happy to follow up for parity.PreviewContentfulClientis dictionary-shaped becauseloadDefinitionsserializes it for the JS bridge, so the app converts typed entries back down withCTEntry.toDictionary(). That mapping belongs in the SDK, which already depends on contentful.swift — filed as NT-3994.include(fixtures ship pre-resolved) andlocale(fixtures are single-locale). No consumer needs either.🤖 Generated with Claude Code