diff --git a/projects/hutch/src/runtime/web/api/article-siren.test.ts b/projects/hutch/src/runtime/web/api/article-siren.test.ts index d68be3d0f..1c62ffe69 100644 --- a/projects/hutch/src/runtime/web/api/article-siren.test.ts +++ b/projects/hutch/src/runtime/web/api/article-siren.test.ts @@ -51,6 +51,7 @@ describe("toArticleSubEntity", () => { status: "unread", savedAt: "2026-03-04T10:00:00.000Z", readAt: null, + isRead: false, }, links: [ { rel: ["read"], title: "Read", href: `/queue/${ARTICLE_ID}/view` }, @@ -74,6 +75,15 @@ describe("toArticleSubEntity", () => { expect(deleteAction).toBeUndefined(); }); + it("emits isRead as an explicit boolean reflecting the read status", () => { + expect(toArticleSubEntity(makeArticle({ status: "unread" })).properties?.isRead).toBe(false); + expect( + toArticleSubEntity( + makeArticle({ status: "read", readAt: new Date("2026-03-04T12:00:00.000Z") }), + ).properties?.isRead, + ).toBe(true); + }); + it("toggles an unread item to a server-driven Mark as read with the target value", () => { const subEntity = toArticleSubEntity(makeArticle({ status: "unread" })); const updateStatus = subEntity.actions?.find((a) => a.name === "update-status"); diff --git a/projects/hutch/src/runtime/web/api/article-siren.ts b/projects/hutch/src/runtime/web/api/article-siren.ts index 200af1d3e..e283566ca 100644 --- a/projects/hutch/src/runtime/web/api/article-siren.ts +++ b/projects/hutch/src/runtime/web/api/article-siren.ts @@ -25,6 +25,10 @@ export function toArticleSubEntity(article: SavedArticle): SirenSubEntity { status: article.status, savedAt: article.savedAt.toISOString(), readAt: article.readAt?.toISOString() ?? null, + // An explicit presentational read-state so a client renders the read + // indicator from one server-authored boolean rather than re-deriving it + // from the `status` vocabulary it would otherwise have to hard-code. + isRead, }, links, actions: [ diff --git a/projects/hutch/src/runtime/web/api/collection-siren.test.ts b/projects/hutch/src/runtime/web/api/collection-siren.test.ts index dafdf3de3..f9a0d04e4 100644 --- a/projects/hutch/src/runtime/web/api/collection-siren.test.ts +++ b/projects/hutch/src/runtime/web/api/collection-siren.test.ts @@ -95,6 +95,7 @@ describe("toArticleCollectionEntity", () => { "status", "savedAt", "readAt", + "isRead", ]); }); @@ -259,6 +260,21 @@ describe("toArticleCollectionEntity", () => { expect(saveArticlesAction?.fields?.map((f) => f.name)).toEqual(["manifest", "content"]); }); + it("advertises a create-session action so a client discovers the reader session mint", () => { + const result: FindArticlesResult = { + articles: [], + total: 0, + page: 1, + pageSize: 20, + }; + + const entity = toArticleCollectionEntity(result, {}); + + const sessionAction = entity.actions?.find((a) => a.name === "create-session"); + expect(sessionAction?.href).toBe("/auth/session"); + expect(sessionAction?.method).toBe("POST"); + }); + it("includes search action with filter fields", () => { const result: FindArticlesResult = { articles: [], diff --git a/projects/hutch/src/runtime/web/api/collection-siren.ts b/projects/hutch/src/runtime/web/api/collection-siren.ts index a8f5864fa..08bb9f8ac 100644 --- a/projects/hutch/src/runtime/web/api/collection-siren.ts +++ b/projects/hutch/src/runtime/web/api/collection-siren.ts @@ -138,6 +138,17 @@ export function toArticleCollectionEntity( { name: "url", type: "url" }, ], }, + // Mints a browser session cookie from the bearer token so a client's + // in-app reader can load the cookie-authenticated reader page. Advertised + // (rather than the client hard-coding the route + method) so it can move + // without a client release; a client that doesn't open an in-app reader + // simply skips it. No fields: the route reads the bearer from the + // Authorization header. + { + name: "create-session", + href: "/auth/session", + method: "POST", + }, ], }; } diff --git a/projects/hutch/src/runtime/web/pages/queue/queue.page.ts b/projects/hutch/src/runtime/web/pages/queue/queue.page.ts index 986b331bc..290418072 100644 --- a/projects/hutch/src/runtime/web/pages/queue/queue.page.ts +++ b/projects/hutch/src/runtime/web/pages/queue/queue.page.ts @@ -297,6 +297,37 @@ const APP_BACK_LINK = { label: "← Back to queue", } as const; +/** Server-authored bridge for the iOS in-app reader: when the reader's mark-read + * htmx request completes, tell the WKWebView so the native app closes the sheet + * and reconciles its list. Guarded on the WKWebView message handler, so it is inert + * in a normal browser that renders this same reader. Keeping the htmx detail here — + * rather than injected by the app — keeps the htmx coupling on the server that owns + * htmx; the app only registers the `readplaceReader` handler and reacts to the + * `markedRead` message, with no knowledge of the front-end's event shape. */ +const READER_MARK_READ_BRIDGE_SCRIPT = ``; + /** True when the client wants the app's chromeless reader rather than the full web * shell — chosen by an explicit client signal, never a user-agent sniff. The app * appends `?platform=ios` to the `read` link it loads in its WKWebView; the @@ -468,20 +499,25 @@ export function initQueueRoutes(deps: QueueDependencies): Router { const { article: ownedArticle, state, audioEnabled } = resolved; if (isIosPlatform(req)) { + const readerBody = ReaderPage({ ...ownedArticle, content: state.content }, { + appOrigin: deps.appOrigin, + summary: state.summary, + summaryPollUrl: state.summaryPollUrl, + crawl: state.crawl, + readerPollUrl: state.readerPollUrl, + progress: state.progress, + audioEnabled, + extensionInstallUrl: undefined, + backLink: APP_BACK_LINK, + renderActions: deps.chromelessReader, + }); + assert(readerBody.scripts, "the reader page always sets its scripts"); sendComponent( req, res, - ChromelessPage(ReaderPage({ ...ownedArticle, content: state.content }, { - appOrigin: deps.appOrigin, - summary: state.summary, - summaryPollUrl: state.summaryPollUrl, - crawl: state.crawl, - readerPollUrl: state.readerPollUrl, - progress: state.progress, - audioEnabled, - extensionInstallUrl: undefined, - backLink: APP_BACK_LINK, - renderActions: deps.chromelessReader, - })), + ChromelessPage({ + ...readerBody, + scripts: readerBody.scripts + READER_MARK_READ_BRIDGE_SCRIPT, + }), ); return; } diff --git a/projects/hutch/src/runtime/web/pages/queue/queue.reader-platform.route.test.ts b/projects/hutch/src/runtime/web/pages/queue/queue.reader-platform.route.test.ts index 6b49ffeeb..6dfda3ffb 100644 --- a/projects/hutch/src/runtime/web/pages/queue/queue.reader-platform.route.test.ts +++ b/projects/hutch/src/runtime/web/pages/queue/queue.reader-platform.route.test.ts @@ -162,6 +162,23 @@ describe("Queue reader chromeless switch (GET /queue/:id/view?platform=ios)", () expect(doc.querySelector(".article-body__actions--bottom")).toBe(null); }); + it("injects the server-owned mark-read bridge for the app, absent from the browser shell", async () => { + const harness = buildHarness(); + const agent = await loginAgent(harness.server, harness.auth); + const articleId = await saveAndGetArticleId(agent, "https://example.com/app-bridge"); + + // The chromeless (app) render carries the bridge: the server owns the htmx + // detail and tells the WKWebView, so the app no longer sniffs htmx itself. + const iosText = (await agent.get(`/queue/${articleId}/view?platform=ios`)).text; + expect(iosText).toContain("window.webkit"); + expect(iosText).toContain("readplaceReader"); + expect(iosText).toContain("htmx:beforeSwap"); + + // The full web shell — served to a browser — must not carry the app bridge. + const shellText = (await agent.get(`/queue/${articleId}/view`)).text; + expect(shellText).not.toContain("readplaceReader"); + }); + it("marks the body chromeless so the reader CSS can pin the top actions", async () => { const harness = buildHarness(); const agent = await loginAgent(harness.server, harness.auth); diff --git a/projects/ios-readplace/App/AffordancePresentation.swift b/projects/ios-readplace/App/AffordancePresentation.swift index 9e1f37b1e..32ee7ed3c 100644 --- a/projects/ios-readplace/App/AffordancePresentation.swift +++ b/projects/ios-readplace/App/AffordancePresentation.swift @@ -44,6 +44,15 @@ struct AffordancePresentation { isDestructive = false removesItem = false isToolbarControl = false + case "create-session": + // Not a user control: the client invokes this bespoke to mint the reader + // session cookie (like a capture-only save), so it never renders in the + // toolbar even though it is advertised on the collection. + systemImage = "key" + tint = nil + isDestructive = false + removesItem = false + isToolbarControl = false case "update-status": systemImage = "checkmark.circle" tint = .brandSuccess @@ -127,13 +136,34 @@ extension Affordance { return fields.allSatisfy { $0.value != nil } } + /// Whether this affordance is a link carrying any structural navigation rel + /// (`self`/`root`/`prev`/`next`/`item`) — client plumbing the client follows for + /// its own pagination/identity/item resolution, never a user control. Tests every + /// rel, not just the presentation token, so a multi-rel link like + /// `["alternate", "next"]` can't slip through as a tappable control while the + /// client also follows it for paging. + var isStructuralLink: Bool { + guard case let .link(link) = invocation else { return false } + return link.rel.contains { Affordance.structuralRels.contains($0) } + } + + /// Whether the row surfaces this link as a discrete control: a semantic link that + /// is neither structural plumbing nor the `read` rel (already the row's primary + /// tap target). Keeps a future item link (e.g. `share`) rendering as a control + /// instead of being discarded, without double-rendering `read`. + var isSemanticControlLink: Bool { + guard case let .link(link) = invocation else { return false } + return !link.rel.contains { Affordance.structuralRels.contains($0) || $0 == "read" } + } + /// Whether the toolbar should surface this affordance as a control: it must be - /// both presentable in the toolbar (a structural navigation link or a - /// capture-only save is excluded by its presentation) and actually invokable + /// presentable in the toolbar (a structural navigation link or a capture-only + /// save is excluded), not carry any structural rel, and be actually invokable /// from a bare control (a field-requiring action with no server value is /// excluded). var isToolbarControl: Bool { - presentation.isToolbarControl && isInvokableByBareControl + guard !isStructuralLink else { return false } + return presentation.isToolbarControl && isInvokableByBareControl } /// Whether invoking this affordance removes the item it acts on from the @@ -154,12 +184,17 @@ extension Affordance { extension Article { /// The advertised item affordances a row surfaces as swipe and accessibility - /// controls, filtered to those a bare control can actually invoke. Like the + /// controls: every action a bare control can actually invoke, plus every semantic + /// link that is neither structural plumbing nor the `read` tap target — so a + /// future item link (e.g. `share`) renders instead of being discarded. Like the /// toolbar, the row drops a field-requiring action with no server value so a /// future such item action is never rendered as a swipe that errors on tap. The /// selection lives here, beside the symmetric toolbar rule - /// (`Affordance.isToolbarControl`) and the shared predicate it reuses - /// (`isInvokableByBareControl`), so the row's choice of controls is unit-testable - /// without standing up a view. - var rowControls: [Affordance] { affordances.filter(\.isInvokableByBareControl) } + /// (`Affordance.isToolbarControl`) and the shared predicates it reuses + /// (`isInvokableByBareControl`, `isSemanticControlLink`), so the row's choice of + /// controls is unit-testable without standing up a view. + var rowControls: [Affordance] { + affordances.filter(\.isInvokableByBareControl) + + links.compactMap(Affordance.init(link:)).filter(\.isSemanticControlLink) + } } diff --git a/projects/ios-readplace/App/AppSession.swift b/projects/ios-readplace/App/AppSession.swift index 0cb0db6de..02f76fd2d 100644 --- a/projects/ios-readplace/App/AppSession.swift +++ b/projects/ios-readplace/App/AppSession.swift @@ -28,8 +28,8 @@ final class AppSession: ObservableObject { // Defaults to an ephemeral configuration so the API/OAuth sessions keep their // cookie jar in an isolated, in-memory store rather than process-wide - // `HTTPCookieStorage.shared` — the minted `hutch_sid` reader cookie must not - // linger in the shared jar where it would outlive a sign-out. + // `HTTPCookieStorage.shared` — the minted reader session cookie must not linger + // in the shared jar where it would outlive a sign-out. // // `wipeReaderWebStore` defaults to the real WebKit deletion; it's the // OS-boundary seam tests replace with a spy. @@ -97,32 +97,30 @@ final class AppSession: ObservableObject { return readerWipe } - private static func isSessionCookie(_ cookie: HTTPCookie) -> Bool { - cookie.name == AppConfig.sessionCookieName - } - - /// Drops the minted browser session cookie (`hutch_sid`) on sign-out so it - /// doesn't linger in the API session's cookie jar for the next sign-in in the - /// same process. The jar is the configuration's own isolated store (never - /// `HTTPCookieStorage.shared`), so this clears only this app's copy. + /// Clears the API session's isolated cookie jar on sign-out so the minted + /// browser session cookie doesn't linger for the next sign-in in the same + /// process. The jar is the configuration's own isolated store (never + /// `HTTPCookieStorage.shared`), so clearing it wholesale touches only this app's + /// copy and needs no knowledge of the server's cookie name. private func clearSessionCookie() { let storage = sessionConfiguration.httpCookieStorage - for cookie in storage?.cookies ?? [] where Self.isSessionCookie(cookie) { + for cookie in storage?.cookies ?? [] { storage?.deleteCookie(cookie) } } /// Removes the reader's authenticated traces from the process-wide WebKit - /// default store on sign-out. The session cookie is deleted by name — a - /// blanket cookie wipe would also drop server-set cookies a full-shell page - /// may have put in this store (e.g. a changelog dismissal set after a - /// session-expiry redirect) — while every non-cookie data type is cleared so - /// the signed-out account's reading history doesn't stay on disk, accepting - /// that the share hint's localStorage dismissal resets with it. + /// default store on sign-out: every cookie scoped to the app's own server host + /// (whatever the server named the session cookie), so a server cookie rename + /// needs no app release, plus every non-cookie data type so the signed-out + /// account's reading history doesn't stay on disk. Scoping the cookie wipe to the + /// server host leaves cookies for other origins untouched. private static func removeReaderWebStoreData() async { let store = WKWebsiteDataStore.default() - for cookie in await store.httpCookieStore.allCookies() where isSessionCookie(cookie) { - await store.httpCookieStore.delete(cookie) + if let host = URL(string: AppConfig.serverBaseURL)?.host { + for cookie in await store.httpCookieStore.allCookies() where cookie.domain.contains(host) { + await store.httpCookieStore.delete(cookie) + } } let nonCookieTypes = WKWebsiteDataStore.allWebsiteDataTypes().subtracting([WKWebsiteDataTypeCookies]) await store.removeData(ofTypes: nonCookieTypes, modifiedSince: .distantPast) diff --git a/projects/ios-readplace/App/ItemRoute.swift b/projects/ios-readplace/App/ItemRoute.swift new file mode 100644 index 000000000..665997c96 --- /dev/null +++ b/projects/ios-readplace/App/ItemRoute.swift @@ -0,0 +1,30 @@ +import Foundation + +/// The side effect a tapped per-item control resolves to, decided purely from the +/// affordance's advertised invocation and its presentation. The row's swipe / +/// accessibility twin of `ToolbarRoute`: keeping the decision in a pure value — +/// rather than inline in the view — lets the routing (open a link, confirm a +/// destructive action, or invoke) be unit-tested without a SwiftUI host, so a +/// link-only item control is opened rather than silently dropped and a destructive +/// control never invokes without a confirmation. +enum ItemRoute: Equatable { + /// Follow a navigable link by opening its href in the in-app web view. + case open(SirenLink) + /// A destructive action (no undo) awaits an explicit confirmation before it invokes. + case confirmDestructive(SirenAction) + /// Invoke a non-destructive action immediately through the generic invoker. + case invoke(SirenAction) + + /// Routes an item affordance: a navigable link opens; an action whose + /// client-side presentation marks it destructive awaits confirmation; any other + /// action invokes immediately. Whether an action is destructive is the single + /// presentation mapping's call, never a per-name check in the view. + static func route(for affordance: Affordance) -> ItemRoute { + switch affordance.invocation { + case let .link(link): + return .open(link) + case let .action(action): + return affordance.presentation.isDestructive ? .confirmDestructive(action) : .invoke(action) + } + } +} diff --git a/projects/ios-readplace/App/LoginView.swift b/projects/ios-readplace/App/LoginView.swift index c3a0f76b8..7790fd619 100644 --- a/projects/ios-readplace/App/LoginView.swift +++ b/projects/ios-readplace/App/LoginView.swift @@ -1,10 +1,13 @@ import SwiftUI struct LoginView: View { - @EnvironmentObject private var session: AppSession + let session: AppSession /// Owned by `RootView` (which handles the OAuth deep link) so a failed Login or /// Sign up can surface here while this view is still on screen. @Binding var authErrorText: String? + /// Injected so the composition point wires the live browser-opening flow and + /// tests capture the started request; there is deliberately no internal default. + let makeFlow: @MainActor (AppSession) -> WebAuthFlow var body: some View { NavigationStack { @@ -66,17 +69,17 @@ struct LoginView: View { /// Opens `/oauth/authorize` for login in the external browser (Chrome if /// installed, to reuse its session); the result returns via the deep link. @MainActor - private func startLogin() { + func startLogin() { authErrorText = nil - makeWebAuthFlow(session: session).start(session.makeOAuth().makeNativeLoginAuthorizationRequest()) + makeFlow(session).start(session.makeOAuth().makeNativeLoginAuthorizationRequest()) } /// Opens `/oauth/authorize` for sign up in the external browser. A fresh /// `start` overwrites any prior pending record so an abandoned attempt can't /// strand stale secrets. @MainActor - private func startSignup() { + func startSignup() { authErrorText = nil - makeWebAuthFlow(session: session).start(session.makeOAuth().makeSignupAuthorizationRequest()) + makeFlow(session).start(session.makeOAuth().makeSignupAuthorizationRequest()) } } diff --git a/projects/ios-readplace/App/ReaderSheet.swift b/projects/ios-readplace/App/ReaderSheet.swift index 675d9393f..d7b3275d8 100644 --- a/projects/ios-readplace/App/ReaderSheet.swift +++ b/projects/ios-readplace/App/ReaderSheet.swift @@ -7,19 +7,19 @@ import SwiftUI /// tap keeps tapping a row instantly responsive, like following a link. struct ReaderSheet: View { let presentation: ReaderPresentation - let mintSession: () async -> HTTPCookie? + let mintSession: () async -> [HTTPCookie]? let onMarkedRead: () -> Void let onClose: () -> Void - @State private var cookie: HTTPCookie? + @State private var cookies: [HTTPCookie]? @State private var bootstrapFailed = false var body: some View { Group { - if let cookie { + if let cookies { ReaderWebView( url: presentation.readerURL, - cookie: cookie, + cookies: cookies, onMarkedRead: onMarkedRead, onClose: onClose, externalBrowser: .system @@ -32,9 +32,9 @@ struct ReaderSheet: View { } .tint(.brandAmber) .task { - guard cookie == nil, !bootstrapFailed else { return } + guard cookies == nil, !bootstrapFailed else { return } if let minted = await mintSession() { - cookie = minted + cookies = minted } else { bootstrapFailed = true } diff --git a/projects/ios-readplace/App/ReaderWebView.swift b/projects/ios-readplace/App/ReaderWebView.swift index 15b6e1cba..7d3e18715 100644 --- a/projects/ios-readplace/App/ReaderWebView.swift +++ b/projects/ios-readplace/App/ReaderWebView.swift @@ -1,4 +1,5 @@ import SwiftUI +import UIKit import WebKit /// Presents the server's authenticated reader in a WKWebView — the app acting as @@ -12,7 +13,7 @@ import WebKit /// existing precedent. struct ReaderWebView: UIViewControllerRepresentable { let url: URL - let cookie: HTTPCookie + let cookies: [HTTPCookie] let onMarkedRead: () -> Void let onClose: () -> Void /// Injected so the composition point wires the live browser and tests inject @@ -27,12 +28,10 @@ struct ReaderWebView: UIViewControllerRepresentable { let controller = UIViewController() let userContent = WKUserContentController() + // The server's chromeless reader posts the mark-read message itself; the app + // only registers the handler and reacts. It injects no script, so it holds no + // knowledge of the reader front-end's htmx internals. userContent.add(context.coordinator, name: ReaderBridge.messageName) - userContent.addUserScript(WKUserScript( - source: ReaderBridge.script, - injectionTime: .atDocumentEnd, - forMainFrameOnly: true - )) let configuration = WKWebViewConfiguration() configuration.userContentController = userContent @@ -51,10 +50,16 @@ struct ReaderWebView: UIViewControllerRepresentable { webView.uiDelegate = context.coordinator controller.view = webView - // Inject the prefetched session cookie into the web view's own store before + // Inject every prefetched session cookie into the web view's own store before // the first navigation, so the reader and its in-reader XHRs are - // authenticated from the first request. - webView.configuration.websiteDataStore.httpCookieStore.setCookie(cookie) { + // authenticated from the first request. The client forwards whatever the + // bootstrap set rather than picking one by name, so a server cookie change + // needs no app release. + Task { @MainActor in + let cookieStore = webView.configuration.websiteDataStore.httpCookieStore + for cookie in cookies { + await cookieStore.setCookie(cookie) + } webView.load(URLRequest(url: url)) } return controller @@ -136,41 +141,41 @@ struct ReaderWebView: UIViewControllerRepresentable { } } -/// The JS↔native bridge for the reader. The reader's mark-read control is an +/// Presents a dialog as a native alert over the web view, answering exactly +/// once on every path. The web view lives inside a SwiftUI sheet, so the +/// presenter is the window's topmost presented controller (the sheet's hosting +/// controller), not the root — the root is already presenting, so presenting +/// from it would silently fail and leave the page's script hanging on an +/// unanswered handler. A web view with no window (mid-dismissal) answers +/// `unpresentedAnswer` instead of presenting nowhere or crashing. +func presentWebDialog(_ dialog: WebDialog, over webView: WKWebView, answer: @escaping (Bool) -> Void) { + guard var presenter = webView.window?.rootViewController else { + answer(dialog.unpresentedAnswer) + return + } + while let presented = presenter.presentedViewController { + presenter = presented + } + let alert = UIAlertController(title: nil, message: dialog.message, preferredStyle: .alert) + for choice in dialog.choices { + alert.addAction(UIAlertAction(title: choice.title, style: choice.style) { _ in + answer(choice.answer) + }) + } + presenter.present(alert, animated: true) +} + +/// The native side of the reader's mark-read bridge. The reader's mark-read is an /// htmx form whose XHR never triggers a navigation, so a `WKNavigationDelegate` -/// can't observe it — the script listens for htmx's swap event instead. The pure -/// message parser is unit-tested; the WKWebView glue that registers and receives -/// the message is left untested (OS boundary), like `AuthWebView` before it. +/// can't observe it. Rather than the app injecting a script that sniffs the +/// front-end's htmx events, the server's chromeless reader posts the message +/// itself (the htmx coupling stays on the server that owns htmx); the app only +/// registers this handler and interprets the message. The pure message parser is +/// unit-tested; the WKWebView glue that registers and receives it is left untested +/// (OS boundary), like `AuthWebView` before it. enum ReaderBridge { static let messageName = "readplaceReader" - /// Cancels the htmx swap for a successful status-change POST (so the page it - /// redirects to never flashes into the sheet) and signals the native side, - /// which closes the sheet and drops the row. The detector keys on the protocol - /// vocabulary — a successful POST carrying the `status` field — not on the - /// request URL, so the server can move the endpoint without breaking the app. - static let script = """ - (function () { - function hasStatusField(params) { - if (!params) { return false; } - if (typeof params.has === 'function') { return params.has('status'); } - if (typeof params.get === 'function') { return params.get('status') != null; } - return Object.prototype.hasOwnProperty.call(params, 'status'); - } - function isStatusChange(detail) { - var cfg = (detail && detail.requestConfig) || {}; - var verb = (cfg.verb || '').toString().toUpperCase(); - var xhr = (detail && detail.xhr) || {}; - return verb === 'POST' && hasStatusField(cfg.parameters) && xhr.status >= 200 && xhr.status < 400; - } - document.body.addEventListener('htmx:beforeSwap', function (event) { - if (!isStatusChange(event.detail)) { return; } - event.detail.shouldSwap = false; - window.webkit.messageHandlers.readplaceReader.postMessage({ type: 'markedRead' }); - }); - })(); - """ - /// Whether a received bridge message reports a completed mark-read. Pure and /// unit-tested. static func isMarkedRead(message name: String, body: Any) -> Bool { diff --git a/projects/ios-readplace/App/ReadingListView.swift b/projects/ios-readplace/App/ReadingListView.swift index f4d316688..c8ea02326 100644 --- a/projects/ios-readplace/App/ReadingListView.swift +++ b/projects/ios-readplace/App/ReadingListView.swift @@ -77,7 +77,7 @@ struct ReadingListView: View { presentation: presentation, mintSession: { await viewModel.mintReaderSession() }, onMarkedRead: { - if let id = presentation.articleId { viewModel.readerMarkedRead(id: id) } + Task { await viewModel.readerStatusChanged() } viewModel.readerPresentation = nil }, onClose: { viewModel.readerPresentation = nil } @@ -227,13 +227,12 @@ struct ReadingListView: View { /// rendered item control resolves to an effect — a link-only affordance is opened, /// not silently dropped. private func activate(_ affordance: Affordance, on article: Article) { - guard let action = affordance.action else { - if let link = affordance.link { viewModel.open(link: link) } - return - } - if affordance.presentation.isDestructive { + switch ItemRoute.route(for: affordance) { + case let .open(link): + viewModel.open(link: link) + case .confirmDestructive: pendingDestructive = PendingDestructive(affordance: affordance, article: article) - } else { + case let .invoke(action): Task { await viewModel.invoke(action, on: article) } } } diff --git a/projects/ios-readplace/App/ReadingListViewModel.swift b/projects/ios-readplace/App/ReadingListViewModel.swift index 018e8d5e7..3c6f7fdf4 100644 --- a/projects/ios-readplace/App/ReadingListViewModel.swift +++ b/projects/ios-readplace/App/ReadingListViewModel.swift @@ -28,6 +28,10 @@ final class ReadingListViewModel: ObservableObject { @Published private(set) var collectionAffordances: [Affordance] = [ReadingListViewModel.addLinksHelp] private var nextHref: String? + /// The server-advertised `create-session` action from the loaded collection, + /// followed to mint the reader's browser session. Nil against a server that + /// hasn't advertised it, in which case the API falls back to a fixed path. + private var sessionAction: SirenAction? private var isLoadingMore = false /// Whether rows beyond the first page are loaded. A post-action adoption /// replaces the list outright only while everything on screen came from one @@ -37,10 +41,6 @@ final class ReadingListViewModel: ObservableObject { /// Whether a collection has ever been applied. Gates the foreground refresh so /// it never races the initial `.task` load with a second fetch at launch. private var hasLoadedOnce = false - /// The row the reader marked read behind the web sheet, awaiting the dismissal - /// converge; consumed by `handleWebSheetDismissal` so the drop survives exactly - /// one re-read. - private var readerDroppedId: String? private let api: ReadplaceAPI private let onSessionExpired: () -> Void @@ -139,15 +139,15 @@ final class ReadingListViewModel: ObservableObject { } } - /// Drops the row the reader just marked read — instantly, so the unread-only - /// list never shows it again behind the sheet. The reader's own POST answers - /// inside the webview where no Siren body is available, so reconciliation - /// belongs to the converge the sheet's dismissal triggers - /// (`handleWebSheetDismissal`); the id is remembered so that converge keeps - /// the row dropped even if an eventually-consistent server GET still lists it. - func readerMarkedRead(id: String) { - articles.removeAll { $0.id == id } - readerDroppedId = id + /// Reconciles the list after the reader reports a status change from inside the + /// webview. The reader's own POST answers where no Siren body is available and + /// the client cannot see which direction the toggle went, so it does not infer + /// "read" and drop a row — it re-reads the collection and adopts the server's + /// truth (a shallow list), which also brings in whatever changed elsewhere (e.g. + /// an item marked unread on the website). A deep-scrolled list holds its position + /// and reconciles on the next pull-to-refresh (`reloadAndAdopt`). + func readerStatusChanged() async { + await reloadAndAdopt(droppingId: nil) } /// Re-reads the list when the app returns to the foreground, so changes made @@ -175,12 +175,8 @@ final class ReadingListViewModel: ObservableObject { /// TokenStore and the cached UI. A live session pays one shallow /// re-read, which doubles as the same reconciliation the foreground performs; /// a deep-scrolled list still holds its position (`adopt` discards the page). - /// Carries the row the reader marked read behind this sheet, so the adopted - /// page never resurrects it (see `readerMarkedRead`). func handleWebSheetDismissal() async { - let droppedId = readerDroppedId - readerDroppedId = nil - await reloadAndAdopt(droppingId: droppedId) + await reloadAndAdopt(droppingId: nil) } /// Reconciles the visible list with the server's post-action collection. @@ -251,11 +247,12 @@ final class ReadingListViewModel: ObservableObject { } /// Mints the cookie session the reader webview needs from the current bearer. - /// Returns nil and surfaces the error when the bootstrap fails, so the reader - /// sheet can show its unavailable view instead of a blank page. - func mintReaderSession() async -> HTTPCookie? { + /// Returns the cookies the server set, or nil (surfacing the error) when the + /// bootstrap fails, so the reader sheet can show its unavailable view instead of + /// a blank page. + func mintReaderSession() async -> [HTTPCookie]? { do { - return try await api.bootstrapSession() + return try await api.bootstrapSession(action: sessionAction) } catch { handle(error) return nil @@ -292,6 +289,7 @@ final class ReadingListViewModel: ObservableObject { // the toolbar for the whole scroll. if replacing { applyToolbar(page) + sessionAction = page.action(named: "create-session") } warningText = page.warning?.message } diff --git a/projects/ios-readplace/App/ReadplaceApp.swift b/projects/ios-readplace/App/ReadplaceApp.swift index f5852e5a4..a7f27ece7 100644 --- a/projects/ios-readplace/App/ReadplaceApp.swift +++ b/projects/ios-readplace/App/ReadplaceApp.swift @@ -21,7 +21,7 @@ struct RootView: View { if session.isLoggedIn { ReadingListView(session: session) } else { - LoginView(authErrorText: $authErrorText) + LoginView(session: session, authErrorText: $authErrorText, makeFlow: makeWebAuthFlow(session:)) } } .tint(.brandAmber) diff --git a/projects/ios-readplace/App/WebDialog.swift b/projects/ios-readplace/App/WebDialog.swift index 545e1fbfc..bb9ae243a 100644 --- a/projects/ios-readplace/App/WebDialog.swift +++ b/projects/ios-readplace/App/WebDialog.swift @@ -1,5 +1,4 @@ import UIKit -import WebKit /// The native dialog a page's JS dialog call maps to. WKWebView suppresses /// `window.confirm()`/`window.alert()` unless the host implements @@ -7,8 +6,8 @@ import WebKit /// htmx's `hx-confirm`, which calls `window.confirm()`, so a suppressed dialog /// silently answers false and the button does nothing in-app. Pure and /// `Equatable` so the panel-kind → dialog mapping is unit-tested without a web -/// view, like `ReaderNavigation` before it; only `presentWebDialog` below is -/// untested UIKit glue. +/// view, like `ReaderNavigation` before it; the `UIAlertController` glue that +/// presents it is `presentWebDialog` in `ReaderWebView.swift` (OS boundary). struct WebDialog: Equatable { /// One tappable choice, carrying the boolean it answers the page's /// `confirm()` with. An alert's single OK carries one too — the alert @@ -52,27 +51,3 @@ struct WebDialog: Equatable { ) } } - -/// Presents a dialog as a native alert over the web view, answering exactly -/// once on every path. The web view lives inside a SwiftUI sheet, so the -/// presenter is the window's topmost presented controller (the sheet's hosting -/// controller), not the root — the root is already presenting, so presenting -/// from it would silently fail and leave the page's script hanging on an -/// unanswered handler. A web view with no window (mid-dismissal) answers -/// `unpresentedAnswer` instead of presenting nowhere or crashing. -func presentWebDialog(_ dialog: WebDialog, over webView: WKWebView, answer: @escaping (Bool) -> Void) { - guard var presenter = webView.window?.rootViewController else { - answer(dialog.unpresentedAnswer) - return - } - while let presented = presenter.presentedViewController { - presenter = presented - } - let alert = UIAlertController(title: nil, message: dialog.message, preferredStyle: .alert) - for choice in dialog.choices { - alert.addAction(UIAlertAction(title: choice.title, style: choice.style) { _ in - answer(choice.answer) - }) - } - presenter.present(alert, animated: true) -} diff --git a/projects/ios-readplace/App/WebPageView.swift b/projects/ios-readplace/App/WebPageView.swift index 34f533a48..f4e93f9e4 100644 --- a/projects/ios-readplace/App/WebPageView.swift +++ b/projects/ios-readplace/App/WebPageView.swift @@ -50,12 +50,14 @@ struct WebPageView: UIViewControllerRepresentable { decidePolicyFor navigationResponse: WKNavigationResponse, decisionHandler: @escaping (WKNavigationResponsePolicy) -> Void ) { - if let http = navigationResponse.response as? HTTPURLResponse, http.statusCode >= 400 { + let statusCode = (navigationResponse.response as? HTTPURLResponse)?.statusCode + switch WebResponsePolicy.decide(statusCode: statusCode) { + case .allow: + decisionHandler(.allow) + case .fail: decisionHandler(.cancel) onFail() - return } - decisionHandler(.allow) } func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) { diff --git a/projects/ios-readplace/App/WebResponsePolicy.swift b/projects/ios-readplace/App/WebResponsePolicy.swift new file mode 100644 index 000000000..efe619b1c --- /dev/null +++ b/projects/ios-readplace/App/WebResponsePolicy.swift @@ -0,0 +1,16 @@ +import Foundation + +/// Whether the in-app web view should render a navigation response or treat it as +/// a failure. WKWebView delivers a 4xx/5xx through `didFinish`, not `didFail`, so +/// the web view would otherwise paint the server's error body. Deciding here in a +/// pure value keeps the "an error status fails" policy unit-testable and out of the +/// OS-boundary delegate, which keeps only the `decisionHandler` plumbing. +enum WebResponsePolicy: Equatable { + case allow + case fail + + static func decide(statusCode: Int?) -> WebResponsePolicy { + if let statusCode, statusCode >= 400 { return .fail } + return .allow + } +} diff --git a/projects/ios-readplace/Makefile b/projects/ios-readplace/Makefile index efdcaed47..c95acb3f3 100644 --- a/projects/ios-readplace/Makefile +++ b/projects/ios-readplace/Makefile @@ -26,9 +26,13 @@ open: generate # someone builds `make ipa-staging` by hand. SIM ?= platform=iOS Simulator,name=$(shell xcrun simctl list devices available | grep -m1 -oE 'iPhone [0-9]+') SIM_DEVICE = $(shell printf '%s' '$(SIM)' | sed -E 's/.*=//') +COVERAGE_RESULT = build/TestResults.xcresult test: generate xcrun simctl bootstatus '$(SIM_DEVICE)' -b - xcodebuild test -project Readplace.xcodeproj -scheme Readplace -destination '$(SIM)' + rm -rf $(COVERAGE_RESULT) + xcodebuild test -project Readplace.xcodeproj -scheme Readplace -destination '$(SIM)' \ + -enableCodeCoverage YES -resultBundlePath $(COVERAGE_RESULT) + python3 scripts/check-coverage.py $(COVERAGE_RESULT) $(MAKE) test-staging SIM='$(SIM)' # Compile the STAGING condition and smoke-test it. The full suite can't run under diff --git a/projects/ios-readplace/README.md b/projects/ios-readplace/README.md index 76945a24f..f7a0f12b9 100644 --- a/projects/ios-readplace/README.md +++ b/projects/ios-readplace/README.md @@ -237,8 +237,12 @@ You need a Mac with **Xcode 15+** and an Apple ID (a free personal team is fine) `make test` (or `Cmd+U` in Xcode) runs the XCTest suite. The network is stubbed with a `URLProtocol`, so tests exercise the real client logic — headers, bodies, -redirects, retries — without a server. Coverage focuses on boundaries and edge -cases: +redirects, retries — without a server. `make test` then enforces a per-file +line-coverage gate (`scripts/check-coverage.py` against +`scripts/coverage-baseline.json`): pure SwiftUI views and WebKit/OS-boundary glue +are excluded, and every other source file must stay at or above its recorded +floor — a ratchet the logic files are walked toward 100%. Coverage focuses on +boundaries and edge cases: - **Siren decoding**: rich vs. minimal entities, JSON `null` image/`readAt`, read-state from `status`/`readAt`, title fallback to URL, entities without diff --git a/projects/ios-readplace/ShareExtension/ShareViewController.swift b/projects/ios-readplace/ShareExtension/ShareViewController.swift index 70800114e..cc01f8155 100644 --- a/projects/ios-readplace/ShareExtension/ShareViewController.swift +++ b/projects/ios-readplace/ShareExtension/ShareViewController.swift @@ -29,24 +29,16 @@ final class ShareViewController: UIViewController { let sharedPdf: (() async -> Data?)? = shared?.pdfProvider.map { provider in { await ShareURLExtractor.loadPDFData(provider) } } - switch await saver.run(url: shared?.url, fallbackTitle: shared?.title, sharedPdf: sharedPdf) { - case .savedWithContent: - finish(message: "Saved with content", symbol: "checkmark.circle.fill", tint: .brandSuccess) - case .savedLinkOnly: - finish(message: "Saved (link only)", symbol: "checkmark.circle.fill", tint: .brandSuccess) - case .notLoggedIn: - finish(message: "Open Readplace and sign in first.", - symbol: "person.crop.circle.badge.exclamationmark", tint: .brandWarning) - case .noLink: - finish(message: "No link found to save.", symbol: "link", tint: .brandWarning) - case .noSaveAction: - finish(message: "The server offered no save action.", - symbol: "exclamationmark.triangle.fill", tint: .brandError) - case .refused(let messages): - finish(message: messages.map(\.plainText).joined(separator: "\n"), symbol: "lock.fill", - tint: messages.contains { $0.kind == .error } ? .brandError : .brandWarning) - case .failed(let message): - finish(message: message, symbol: "exclamationmark.triangle.fill", tint: .brandError) + let outcome = await saver.run(url: shared?.url, fallbackTitle: shared?.title, sharedPdf: sharedPdf) + let status = ShareStatusPresentation(outcome: outcome) + finish(message: status.message, symbol: status.symbol, tint: uiColor(for: status.tone)) + } + + private func uiColor(for tone: ShareStatusTone) -> UIColor { + switch tone { + case .success: return .brandSuccess + case .warning: return .brandWarning + case .error: return .brandError } } diff --git a/projects/ios-readplace/Shared/AppConfig.swift b/projects/ios-readplace/Shared/AppConfig.swift index da5eb1bc3..1c4b1f641 100644 --- a/projects/ios-readplace/Shared/AppConfig.swift +++ b/projects/ios-readplace/Shared/AppConfig.swift @@ -53,18 +53,11 @@ enum AppConfig { /// advertising it as a Siren link. static let addLinksHelpPath = "/help/add-links" - /// Query item the in-app reader appends to the server `read` link so the same - /// `/queue/:id/view` route renders chromeless — bare of the web shell — with the - /// native reading list as its chrome. An explicit client-sent signal (never a - /// user-agent sniff); must match the server's `?platform=ios` switch. + /// Query item the in-app reader appends to the server `read` link so the reader + /// renders chromeless — bare of the web shell — with the native reading list as + /// its chrome. An explicit client-sent signal, never a user-agent sniff. static let readerPlatformQueryItem = URLQueryItem(name: "platform", value: "ios") - /// Name of the server's browser session cookie (`hutch_sid`). Minted from a - /// bearer token via `POST /auth/session` and injected into the in-app reader - /// webview so its cookie-authenticated pages load. Must match the server's - /// `SESSION_COOKIE_NAME`. - static let sessionCookieName = "hutch_sid" - /// Shared container so the app (which signs in) and the share extension /// (which saves) can both read the OAuth tokens. /// diff --git a/projects/ios-readplace/Shared/OAuthService.swift b/projects/ios-readplace/Shared/OAuthService.swift index db341e9a2..e5cf519e2 100644 --- a/projects/ios-readplace/Shared/OAuthService.swift +++ b/projects/ios-readplace/Shared/OAuthService.swift @@ -47,17 +47,17 @@ struct OAuthService { var nativeRedirectURI: String { AppConfig.nativeCallbackURL } /// Builds the external-browser Login `/oauth/authorize` URL: the native - /// custom-scheme callback plus `screen_hint=login`, which routes an - /// unauthenticated user to `/login` (an already-authenticated Chrome session - /// passes straight through to consent, ignoring the hint). + /// custom-scheme callback plus `screen_hint=login`, so the server shows an + /// unauthenticated user the sign-in screen (an already-authenticated Chrome + /// session passes straight through to consent, ignoring the hint). func makeNativeLoginAuthorizationRequest() -> AuthorizationRequest { makeAuthorizationRequest(redirectURI: nativeRedirectURI, screenHint: "login") } /// Builds the external-browser Sign up `/oauth/authorize` URL: the native - /// custom-scheme callback plus `screen_hint=signup`, which routes an - /// unauthenticated user to `/signup` (an already-authenticated Chrome session - /// passes straight through to consent, ignoring the hint). + /// custom-scheme callback plus `screen_hint=signup`, so the server shows an + /// unauthenticated user the sign-up screen (an already-authenticated Chrome + /// session passes straight through to consent, ignoring the hint). func makeSignupAuthorizationRequest() -> AuthorizationRequest { makeAuthorizationRequest(redirectURI: nativeRedirectURI, screenHint: "signup") } diff --git a/projects/ios-readplace/Shared/ReadplaceAPI.swift b/projects/ios-readplace/Shared/ReadplaceAPI.swift index 10f5cb912..1ed6c0ba6 100644 --- a/projects/ios-readplace/Shared/ReadplaceAPI.swift +++ b/projects/ios-readplace/Shared/ReadplaceAPI.swift @@ -96,8 +96,8 @@ final class ReadplaceAPI { // Defaults to an ephemeral configuration so the session's cookie jar is its // own isolated, in-memory store rather than process-wide `HTTPCookieStorage.shared`: - // the `hutch_sid` cookie minted by `bootstrapSession` must not linger in the - // shared jar where it would outlive the session and leak across sign-outs. + // the session cookie minted by `bootstrapSession` must not linger in the shared + // jar where it would outlive the session and leak across sign-outs. init( baseURL: String, store: TokenStore, @@ -182,41 +182,53 @@ final class ReadplaceAPI { // MARK: - Reader session - /// Mints a browser session cookie from the current bearer token via the - /// session-bootstrap endpoint and returns it. The in-app reader injects the - /// cookie so the cookie-authenticated reader page (and its in-reader XHRs) - /// load without bouncing to a sign-in page. Reuses `send()`, so a stale bearer - /// is refreshed once before the cookie is minted. - func bootstrapSession() async throws -> HTTPCookie { - guard let url = URL(string: "\(baseURL)/auth/session") else { throw APIError.decoding } + /// Mints a browser session from the current bearer token and returns every + /// cookie the response set. The in-app reader injects them so the + /// cookie-authenticated reader page (and its in-reader XHRs) load without + /// bouncing to a sign-in page. Follows the server-declared `action`'s href and + /// method when the collection advertised one (`create-session`), so the endpoint + /// can move without an app release; falls back to a fixed path only for a server + /// that hasn't advertised the action yet (an older shipped build must keep + /// working). The client never selects a cookie by name — it forwards whatever the + /// server set. Reuses `send()`, so a stale bearer is refreshed once before the + /// session is minted; a response that sets no cookie is a failed mint. + func bootstrapSession(action: SirenAction? = nil) async throws -> [HTTPCookie] { + let url: URL + let method: String + if let action { + url = try absoluteURL(action.href) + method = action.method + } else { + guard let fallback = URL(string: "\(baseURL)/auth/session") else { throw APIError.decoding } + url = fallback + method = "POST" + } var request = URLRequest(url: url) - request.httpMethod = "POST" + request.httpMethod = method let (data, http) = try await send(request) guard (200...299).contains(http.statusCode) else { throw apiError(from: data, status: http.statusCode) } - guard let cookie = sessionCookie(from: http, url: url) else { throw APIError.decoding } - return cookie + let cookies = sessionCookies(from: http, url: url) + guard !cookies.isEmpty else { throw APIError.decoding } + return cookies } - /// Reads the session cookie by name from the session's own cookie jar, which - /// the configuration isolates (an ephemeral store, not `HTTPCookieStorage.shared`) - /// so the cookie URLSession just parsed never touches the process-wide jar. - /// The cookie spec forbids folding repeated `Set-Cookie` headers into one - /// comma-joined value, so re-splitting `allHeaderFields` is unsafe once a - /// response sets more than one cookie; reading the already-parsed cookie back - /// by name sidesteps that. Falls back to the response's own `Set-Cookie` - /// header (reliable for a single cookie) for environments that don't populate - /// the store. - private func sessionCookie(from response: HTTPURLResponse, url: URL) -> HTTPCookie? { - if let stored = session.configuration.httpCookieStorage? - .cookies(for: url)? - .first(where: { $0.name == AppConfig.sessionCookieName }) { + /// Reads every cookie the bootstrap response set from the session's own cookie + /// jar, which the configuration isolates (an ephemeral store, not + /// `HTTPCookieStorage.shared`) so the cookies URLSession just parsed never touch + /// the process-wide jar. The cookie spec forbids folding repeated `Set-Cookie` + /// headers into one comma-joined value, so re-splitting `allHeaderFields` is + /// unsafe once a response sets more than one cookie; reading the already-parsed + /// cookies back from the store sidesteps that. Falls back to parsing the + /// response's own `Set-Cookie` header for environments that don't populate the + /// store. + private func sessionCookies(from response: HTTPURLResponse, url: URL) -> [HTTPCookie] { + if let stored = session.configuration.httpCookieStorage?.cookies(for: url), !stored.isEmpty { return stored } - guard let headers = response.allHeaderFields as? [String: String] else { return nil } + guard let headers = response.allHeaderFields as? [String: String] else { return [] } return HTTPCookie.cookies(withResponseHeaderFields: headers, for: url) - .first { $0.name == AppConfig.sessionCookieName } } // MARK: - Saving @@ -299,8 +311,8 @@ final class ReadplaceAPI { /// Builds a `multipart/form-data` request with a UUID boundary for /// `save-content`: the `url`, `mediaType` and (when non-empty) `title` text - /// parts, then a `content` file part whose `filename` attribute is what the - /// server keys `isFile` off — its per-part Content-Type is ignored. + /// parts, then a `content` file part. The content part carries a `filename` + /// attribute so the server treats it as a file rather than a text field. private func multipartRequest( _ url: URL, method: String, @@ -501,10 +513,9 @@ final class ReadplaceAPI { /// Re-attaches `Authorization`, `Accept` and `X-Readplace-Client` to redirected /// requests. URLSession strips `Authorization` on cross-origin redirects and may -/// drop custom headers generally; the server bounces the client from the entry +/// drop custom headers generally; the server redirects the client from the entry /// point to the collection, so the followed redirect must keep them to stay -/// authenticated and keep negotiating Siren (the client header so onboarding -/// step 1 is recorded on the post-redirect `/queue` load). +/// authenticated and keep negotiating Siren. private final class RedirectHeaderPreservingDelegate: NSObject, URLSessionTaskDelegate { func urlSession( _ session: URLSession, diff --git a/projects/ios-readplace/Shared/ShareStatusPresentation.swift b/projects/ios-readplace/Shared/ShareStatusPresentation.swift new file mode 100644 index 000000000..4b4847c82 --- /dev/null +++ b/projects/ios-readplace/Shared/ShareStatusPresentation.swift @@ -0,0 +1,54 @@ +import Foundation + +/// The visual tone of a share-sheet status, mapped to a brand colour at the +/// UIKit boundary. Kept UIKit-free so the outcome→status mapping stays a pure, +/// testable value. +enum ShareStatusTone: Equatable { + case success + case warning + case error +} + +/// What the share-sheet shell shows for a save outcome: the message text, an SF +/// Symbol name, and a tone. Lifted out of `ShareViewController` so the whole +/// mapping — including joining a refusal's messages and choosing error vs. +/// warning from their kinds — is a pure value the tests exercise directly; the +/// controller only paints it and maps the tone to a `UIColor`. +struct ShareStatusPresentation: Equatable { + let message: String + let symbol: String + let tone: ShareStatusTone + + init(outcome: SaveSharedOutcome) { + switch outcome { + case .savedWithContent: + message = "Saved with content" + symbol = "checkmark.circle.fill" + tone = .success + case .savedLinkOnly: + message = "Saved (link only)" + symbol = "checkmark.circle.fill" + tone = .success + case .notLoggedIn: + message = "Open Readplace and sign in first." + symbol = "person.crop.circle.badge.exclamationmark" + tone = .warning + case .noLink: + message = "No link found to save." + symbol = "link" + tone = .warning + case .noSaveAction: + message = "The server offered no save action." + symbol = "exclamationmark.triangle.fill" + tone = .error + case .refused(let messages): + message = messages.map(\.plainText).joined(separator: "\n") + symbol = "lock.fill" + tone = messages.contains { $0.kind == .error } ? .error : .warning + case .failed(let failureMessage): + message = failureMessage + symbol = "exclamationmark.triangle.fill" + tone = .error + } + } +} diff --git a/projects/ios-readplace/ShareExtension/ShareURLExtractor.swift b/projects/ios-readplace/Shared/ShareURLExtractor.swift similarity index 77% rename from projects/ios-readplace/ShareExtension/ShareURLExtractor.swift rename to projects/ios-readplace/Shared/ShareURLExtractor.swift index d79696374..203c4cc95 100644 --- a/projects/ios-readplace/ShareExtension/ShareURLExtractor.swift +++ b/projects/ios-readplace/Shared/ShareURLExtractor.swift @@ -18,8 +18,13 @@ enum ShareURLExtractor { } static func extract(from context: NSExtensionContext?) async -> Shared? { - guard let items = context?.inputItems as? [NSExtensionItem] else { return nil } + await extract(from: context?.inputItems as? [NSExtensionItem] ?? []) + } + /// The extraction core, taking the item list directly so a test can drive it + /// with fabricated `NSItemProvider`s instead of standing up a live extension + /// context (which no test can construct). + static func extract(from items: [NSExtensionItem]) async -> Shared? { // Scan every item before deciding: a host can deliver the PDF file and // the web URL as separate extension items, and the first item alone // would look like a URL-less share. @@ -54,19 +59,21 @@ enum ShareURLExtractor { private static func loadURL(_ provider: NSItemProvider) async -> URL? { await withCheckedContinuation { continuation in provider.loadItem(forTypeIdentifier: UTType.url.identifier, options: nil) { item, _ in - if let url = item as? URL { - continuation.resume(returning: url) - } else if let data = item as? Data, let url = URL(dataRepresentation: data, relativeTo: nil) { - continuation.resume(returning: url) - } else if let string = item as? String, let url = URL(string: string) { - continuation.resume(returning: url) - } else { - continuation.resume(returning: nil) - } + continuation.resume(returning: coerceURL(from: item)) } } } + /// Which concrete type `loadItem` hands back for a URL — `URL`, `Data`, or + /// `String` — varies by host app and OS release, so the coercion lives in a + /// synchronous seam a test can pin shape-by-shape, deterministically. + nonisolated static func coerceURL(from item: (any NSSecureCoding)?) -> URL? { + if let url = item as? URL { return url } + if let data = item as? Data, let url = URL(dataRepresentation: data, relativeTo: nil) { return url } + if let string = item as? String, let url = URL(string: string) { return url } + return nil + } + private static func loadText(_ provider: NSItemProvider) async -> String? { await withCheckedContinuation { continuation in provider.loadItem(forTypeIdentifier: UTType.plainText.identifier, options: nil) { item, _ in diff --git a/projects/ios-readplace/Shared/SirenModels.swift b/projects/ios-readplace/Shared/SirenModels.swift index e1a5b5ced..c7c833986 100644 --- a/projects/ios-readplace/Shared/SirenModels.swift +++ b/projects/ios-readplace/Shared/SirenModels.swift @@ -30,6 +30,17 @@ private extension KeyedDecodingContainer { } return elements } + + /// Decodes a single value under `key` leniently: a missing, null, or malformed + /// value yields `nil` instead of failing the surrounding decode. Used for a + /// property whose absence or evolution must degrade the one thing it feeds, not + /// blank the whole response. + func decodeLossyIfPresent( + _ type: Value.Type, + forKey key: Key + ) throws -> Value? { + (try decodeIfPresent(FailableDecodable.self, forKey: key))?.wrapped + } } // MARK: - Wire format (Siren) @@ -126,6 +137,10 @@ struct ArticleProperties: Decodable { let status: String? let savedAt: String? let readAt: String? + /// The server's explicit presentational read-state. Optional so an older server + /// that doesn't emit it still decodes; the client falls back to deriving read + /// state from `status`/`readAt` only then. + let isRead: Bool? } struct SirenEntity: Decodable { @@ -158,10 +173,24 @@ struct CollectionProperties: Decodable { let warning: SirenWarning? } +extension CollectionProperties { + private enum CodingKeys: String, CodingKey { case warning } + + /// Decodes the warning leniently so an evolving or malformed warning degrades to + /// no banner rather than failing the whole collection decode: the warning is a + /// non-fatal channel, so a change to it must never be the one thing that blanks + /// the page. + init(from decoder: Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + warning = try container.decodeLossyIfPresent(SirenWarning.self, forKey: .warning) + } +} + /// A non-fatal reason the server attaches to a collection (e.g. a URL that -/// couldn't be saved). +/// couldn't be saved). `code` is optional: the client renders only `message`, so a +/// warning whose classifier the client doesn't read still surfaces its text. struct SirenWarning: Decodable { - let code: String + let code: String? let message: String } @@ -208,9 +237,8 @@ struct ServerMessage: Decodable, Equatable { extension ServerMessage { /// How a client should present a message. The wire `type` stays a `String` so /// an unknown future value still decodes; `kind` maps it for the UI and treats - /// any unrecognized value as `.warning` (the neutral default) — mirroring the - /// server's current `"warning" | "error"` union without hard-failing on a value - /// a newer server might add. + /// any unrecognized value as `.warning` (the neutral default), so a value a + /// newer server adds never hard-fails. enum Kind { case warning case error @@ -324,6 +352,11 @@ struct Article: Identifiable, Hashable { /// per actionable entry by iterating this — it never cherry-picks an action by /// name — so a newly-advertised item action renders with no client change. let actions: [SirenAction] + /// Every navigable link the server advertised on this item, in wire order. The + /// `read` link (the row's primary tap target) is surfaced through `readHref`; + /// every other non-structural semantic link becomes a discrete control, so a + /// future item link (e.g. `share`) renders instead of being discarded. + let links: [SirenLink] /// The href of the server-declared link for reading this item, followed when /// the row is tapped. Absent ⇒ the row is not openable. This follows the /// navigable `read` link (the row's primary tap target), distinct from the @@ -337,7 +370,10 @@ struct Article: Identifiable, Hashable { extension Article { /// The item's action controls, one per advertised action the client can /// invoke (an action with a usable href). Built by iterating — not by matching - /// a known name — so the loop renders whatever the server offered. + /// a known name — so the loop renders whatever the server offered. The row also + /// surfaces the item's semantic links (see `rowControls`), which is derived in + /// the presentation layer because a link's control-worthiness is a client + /// presentation concern. var affordances: [Affordance] { actions.compactMap(Affordance.init(action:)) } /// Builds a display model from a Siren entity, or returns nil when the @@ -355,10 +391,13 @@ extension Article { excerpt = props.excerpt imageURL = props.imageUrl.flatMap(URL.init(string:)) readTimeMinutes = props.estimatedReadTimeMinutes - isRead = props.status == "read" || props.readAt != nil + // Prefer the server's explicit read-state; fall back to deriving it from the + // status vocabulary only for an older server that doesn't emit `isRead`. + isRead = props.isRead ?? (props.status == "read" || props.readAt != nil) savedAt = props.savedAt.flatMap(SirenDate.parse) actions = entity.actions ?? [] - readHref = entity.links?.first { $0.rel.contains("read") }?.href + links = entity.links ?? [] + readHref = links.first { $0.rel.contains("read") }?.href } } diff --git a/projects/ios-readplace/Shared/TokenStore.swift b/projects/ios-readplace/Shared/TokenStore.swift index db057334d..d9c1d560e 100644 --- a/projects/ios-readplace/Shared/TokenStore.swift +++ b/projects/ios-readplace/Shared/TokenStore.swift @@ -1,6 +1,6 @@ import Foundation -/// OAuth tokens issued by the server's `/oauth/token` endpoint. +/// The OAuth token pair persisted for the app and share extension. struct OAuthTokens: Equatable { let accessToken: String let refreshToken: String @@ -61,10 +61,23 @@ struct TokenStore { guard let url = Bundle.main.url(forResource: "embedded", withExtension: "mobileprovision"), let data = try? Data(contentsOf: url), + let group = parseAppGroupId(fromProvisioningProfile: data) + else { return AppConfig.appGroupId } + return group + }() + + /// Extracts the first application-group entitlement from an embedded + /// `.mobileprovision` blob, or nil when it can't be read. The profile is a + /// CMS-signed container with a plist inside; the signature isn't verified here + /// (the OS already did at install), so this just scans out the `` + /// slice and reads the entitlement. Pulled out of `resolvedAppGroupId` so the + /// parsing is unit-testable without a real `Bundle.main`. + static func parseAppGroupId(fromProvisioningProfile data: Data) -> String? { + guard let raw = String(data: data, encoding: .isoLatin1), let start = raw.range(of: "") - else { return AppConfig.appGroupId } + else { return nil } let plistString = String(raw[start.lowerBound.. SirenAction { + SirenAction(name: name, href: href, method: "POST", title: title, type: nil, fields: nil) + } + + func testNavigableLinkRoutesToOpen() { + let link = SirenLink(rel: ["share"], href: "/queue/a1/share", title: "Share") + let affordance = try! XCTUnwrap(Affordance(link: link)) + + XCTAssertEqual(ItemRoute.route(for: affordance), .open(link)) + } + + func testDestructiveActionRoutesToConfirmation() { + // `delete` is destructive per the presentation mapping, so it must confirm + // before invoking rather than acting on the tap. + let delete = action(name: "delete", href: "/queue/a1/delete", title: "Delete") + let affordance = try! XCTUnwrap(Affordance(action: delete)) + + XCTAssertEqual(ItemRoute.route(for: affordance), .confirmDestructive(delete)) + } + + func testNonDestructiveActionRoutesToInvoke() { + let markRead = action(name: "update-status", title: "Mark as read") + let affordance = try! XCTUnwrap(Affordance(action: markRead)) + + XCTAssertEqual(ItemRoute.route(for: affordance), .invoke(markRead)) + } +} diff --git a/projects/ios-readplace/Tests/KeychainTokenStorageTests.swift b/projects/ios-readplace/Tests/KeychainTokenStorageTests.swift new file mode 100644 index 000000000..0348a138f --- /dev/null +++ b/projects/ios-readplace/Tests/KeychainTokenStorageTests.swift @@ -0,0 +1,45 @@ +import XCTest +@testable import Readplace + +/// Exercises the real Keychain-backed storage against the simulator keychain. The +/// test host app carries the `group.com.readplace` app-group entitlement (which +/// doubles as the keychain access group), so generic-password items in that group +/// round-trip here — the path production uses, not the `UserDefaults` test double. +final class KeychainTokenStorageTests: XCTestCase { + private let keychain = KeychainTokenStorage(accessGroup: AppConfig.appGroupId) + + override func setUp() { + super.setUp() + for key in TokenKey.allCases { keychain.removeValue(for: key) } + } + + override func tearDown() { + for key in TokenKey.allCases { keychain.removeValue(for: key) } + super.tearDown() + } + + func testAddsReadsUpdatesAndRemovesAToken() { + XCTAssertNil(keychain.value(for: .accessToken), "starts empty") + + keychain.setValue("first", for: .accessToken) // insert path (SecItemAdd) + XCTAssertEqual(keychain.value(for: .accessToken), "first") + + keychain.setValue("second", for: .accessToken) // update path (SecItemUpdate) + XCTAssertEqual(keychain.value(for: .accessToken), "second") + + keychain.removeValue(for: .accessToken) + XCTAssertNil(keychain.value(for: .accessToken)) + } + + func testKeysAreStoredIndependently() { + keychain.setValue("acc", for: .accessToken) + keychain.setValue("ref", for: .refreshToken) + + XCTAssertEqual(keychain.value(for: .accessToken), "acc") + XCTAssertEqual(keychain.value(for: .refreshToken), "ref") + + keychain.removeValue(for: .accessToken) + XCTAssertNil(keychain.value(for: .accessToken)) + XCTAssertEqual(keychain.value(for: .refreshToken), "ref", "removing one key leaves the other") + } +} diff --git a/projects/ios-readplace/Tests/LoginFlowTests.swift b/projects/ios-readplace/Tests/LoginFlowTests.swift index ebcd3534d..98ce81547 100644 --- a/projects/ios-readplace/Tests/LoginFlowTests.swift +++ b/projects/ios-readplace/Tests/LoginFlowTests.swift @@ -23,7 +23,7 @@ final class LoginFlowTests: XCTestCase { let request = makeService(store: TestSupport.loggedInStore()).makeNativeLoginAuthorizationRequest() let components = URLComponents(url: request.url, resolvingAgainstBaseURL: false)! - XCTAssertEqual(components.host, "readplace.com") + XCTAssertEqual(components.host, URL(string: AppConfig.serverBaseURL)?.host) XCTAssertEqual(components.path, "/oauth/authorize") let items = Dictionary(uniqueKeysWithValues: (components.queryItems ?? []).map { ($0.name, $0.value ?? "") }) XCTAssertEqual(items["client_id"], "ios-app") @@ -98,8 +98,8 @@ final class LoginFlowTests: XCTestCase { XCTAssertFalse(session.isLoggedIn) XCTAssertNil( - config.httpCookieStorage?.cookies?.first { $0.name == AppConfig.sessionCookieName }, - "the minted hutch_sid cookie must not survive a forced sign-out" + config.httpCookieStorage?.cookies?.first { $0.name == "hutch_sid" }, + "the minted session cookie must not survive a forced sign-out" ) await readerWipe.value XCTAssertTrue( @@ -123,8 +123,8 @@ final class LoginFlowTests: XCTestCase { XCTAssertFalse(session.isLoggedIn) XCTAssertNil( - config.httpCookieStorage?.cookies?.first { $0.name == AppConfig.sessionCookieName }, - "the minted hutch_sid cookie must not survive sign-out" + config.httpCookieStorage?.cookies?.first { $0.name == "hutch_sid" }, + "the minted session cookie must not survive sign-out" ) XCTAssertTrue( readerWipeInvoked, @@ -161,4 +161,51 @@ final class LoginFlowTests: XCTestCase { XCTAssertEqual(queueRequest.value(forHTTPHeaderField: "Authorization"), "Bearer access-1") XCTAssertEqual(queueRequest.value(forHTTPHeaderField: "Accept"), "application/vnd.siren+json") } + + func testCallbackCarryingAnErrorParamIsDeniedWithoutExchanging() async { + let store = TokenStore(defaults: TestSupport.ephemeralDefaults()) + let session = AppSession(store: store, sessionConfiguration: TestSupport.stubbedConfiguration()) + + let result = await session.completeSignIn( + callbackURL: URL(string: "\(AppConfig.nativeCallbackURL)?error=access_denied&state=S")!, + verifier: "v", expectedState: "S", redirectURI: AppConfig.nativeCallbackURL + ) + + guard case .failure(let error) = result else { return XCTFail("expected .failure, got \(result)") } + XCTAssertEqual((error as? AuthFlowError)?.errorDescription, AuthFlowError.denied("access_denied").errorDescription) + XCTAssertFalse(session.isLoggedIn) + XCTAssertTrue(StubURLProtocol.records(path: "/oauth/token").isEmpty, "a denied authorization exchanges no code") + } + + func testCallbackWithoutACodeIsMissingCodeWithoutExchanging() async { + let store = TokenStore(defaults: TestSupport.ephemeralDefaults()) + let session = AppSession(store: store, sessionConfiguration: TestSupport.stubbedConfiguration()) + + let result = await session.completeSignIn( + callbackURL: URL(string: "\(AppConfig.nativeCallbackURL)?state=S")!, + verifier: "v", expectedState: "S", redirectURI: AppConfig.nativeCallbackURL + ) + + guard case .failure(let error) = result else { return XCTFail("expected .failure, got \(result)") } + XCTAssertEqual((error as? AuthFlowError)?.errorDescription, AuthFlowError.missingCode.errorDescription) + XCTAssertTrue(StubURLProtocol.records(path: "/oauth/token").isEmpty) + } + + func testExchangeFailureDuringSignInSurfacesAsFailureAndStaysLoggedOut() async { + let store = TokenStore(defaults: TestSupport.ephemeralDefaults()) + let session = AppSession(store: store, sessionConfiguration: TestSupport.stubbedConfiguration()) + StubURLProtocol.setHandler { _, _ in .json(400, "{\"error\":\"invalid_grant\"}") } + + let result = await session.completeSignIn( + callbackURL: URL(string: "\(AppConfig.nativeCallbackURL)?code=abc&state=S")!, + verifier: "v", expectedState: "S", redirectURI: AppConfig.nativeCallbackURL + ) + + guard case .failure(let error) = result else { return XCTFail("expected .failure, got \(result)") } + XCTAssertEqual( + (error as? OAuthError)?.errorDescription, + OAuthError.tokenExchangeFailed(status: 400).errorDescription + ) + XCTAssertFalse(session.isLoggedIn) + } } diff --git a/projects/ios-readplace/Tests/LoginViewTests.swift b/projects/ios-readplace/Tests/LoginViewTests.swift new file mode 100644 index 000000000..3d1c8a504 --- /dev/null +++ b/projects/ios-readplace/Tests/LoginViewTests.swift @@ -0,0 +1,81 @@ +import SwiftUI +import UIKit +import XCTest +@testable import Readplace + +/// The login screen's only decisions: which authorize request each button +/// starts (transposing them would send Sign up to the login screen) and that a +/// fresh attempt clears the previous attempt's error. +@MainActor +final class LoginViewTests: XCTestCase { + private final class Captured { + var requests: [AuthorizationRequest] = [] + var errorText: String? = "Authorization was denied (access_denied)." + } + + private func makeView(_ captured: Captured) -> LoginView { + LoginView( + session: AppSession( + store: TokenStore(defaults: TestSupport.ephemeralDefaults()), + sessionConfiguration: TestSupport.stubbedConfiguration() + ), + authErrorText: Binding(get: { captured.errorText }, set: { captured.errorText = $0 }), + makeFlow: { _ in WebAuthFlow(start: { captured.requests.append($0) }, complete: { _ in nil }) } + ) + } + + private func queryItems(_ request: AuthorizationRequest) -> [String: String] { + let items = URLComponents(url: request.url, resolvingAgainstBaseURL: false)?.queryItems ?? [] + return Dictionary(uniqueKeysWithValues: items.map { ($0.name, $0.value ?? "") }) + } + + func testLoginButtonClearsTheStaleErrorAndStartsALoginAuthorization() throws { + let captured = Captured() + + makeView(captured).startLogin() + + XCTAssertNil(captured.errorText, "a fresh attempt must clear the previous attempt's error") + XCTAssertEqual(captured.requests.count, 1) + let request = try XCTUnwrap(captured.requests.first) + XCTAssertEqual(queryItems(request)["screen_hint"], "login") + XCTAssertEqual(request.redirectURI, AppConfig.nativeCallbackURL) + } + + func testSignupButtonClearsTheStaleErrorAndStartsASignupAuthorization() throws { + let captured = Captured() + + makeView(captured).startSignup() + + XCTAssertNil(captured.errorText, "a fresh attempt must clear the previous attempt's error") + XCTAssertEqual(captured.requests.count, 1) + let request = try XCTUnwrap(captured.requests.first) + XCTAssertEqual(queryItems(request)["screen_hint"], "signup") + XCTAssertEqual(request.redirectURI, AppConfig.nativeCallbackURL) + } + + func testTheLoginScreenRendersWithAndWithoutAnAuthError() { + // Rendered in a real window so body coverage — including the error + // branch — is deterministic instead of depending on the app host + // happening to launch logged out. SwiftUI draws text into shared + // backing views, so the error's own pixels are not observable from + // UIKit here; mounting a live hierarchy in both states is. + let erroring = Captured() + let clean = Captured() + clean.errorText = nil + + XCTAssertGreaterThan(renderedViewCount(makeView(erroring)), 1, "the erroring login screen must mount a live view hierarchy") + XCTAssertGreaterThan(renderedViewCount(makeView(clean)), 1, "the clean login screen must mount a live view hierarchy") + } + + private func renderedViewCount(_ view: LoginView) -> Int { + let window = UIWindow(frame: UIScreen.main.bounds) + window.rootViewController = UIHostingController(rootView: view) + window.makeKeyAndVisible() + window.layoutIfNeeded() + return viewCount(in: window) + } + + private func viewCount(in view: UIView) -> Int { + view.subviews.reduce(1) { $0 + viewCount(in: $1) } + } +} diff --git a/projects/ios-readplace/Tests/OAuthServiceTests.swift b/projects/ios-readplace/Tests/OAuthServiceTests.swift index 7ee845431..b27cc0a9c 100644 --- a/projects/ios-readplace/Tests/OAuthServiceTests.swift +++ b/projects/ios-readplace/Tests/OAuthServiceTests.swift @@ -91,4 +91,46 @@ final class OAuthServiceTests: XCTestCase { let json = TestSupport.jsonObject(record.body) XCTAssertEqual(json["token"] as? String, "r1") } + + func testExchangeCodeWithAMalformedBodyThrowsMalformedResponse() async { + let store = TestSupport.loggedInStore() + StubURLProtocol.setHandler { _, _ in .json(200, "not json at all") } + do { + try await makeService(store: store).exchangeCode("c", verifier: "v", redirectURI: AppConfig.nativeCallbackURL) + XCTFail("expected .malformedResponse") + } catch { + XCTAssertEqual((error as? OAuthError)?.errorDescription, OAuthError.malformedResponse.errorDescription) + } + } + + func testExchangeCodeWithoutARefreshTokenThrowsMalformedResponse() async { + let store = TestSupport.loggedInStore() + // A 200 that carries an access token but no refresh_token, and no fallback + // (exchange has none), is malformed — the session can't be refreshed later. + StubURLProtocol.setHandler { _, _ in .json(200, Fixtures.tokenResponse(access: "a", refresh: nil)) } + do { + try await makeService(store: store).exchangeCode("c", verifier: "v", redirectURI: AppConfig.nativeCallbackURL) + XCTFail("expected .malformedResponse") + } catch { + XCTAssertEqual((error as? OAuthError)?.errorDescription, OAuthError.malformedResponse.errorDescription) + } + } + + func testRefreshWithoutAStoredRefreshTokenThrowsAndMakesNoRequest() async { + let store = TokenStore(defaults: TestSupport.ephemeralDefaults()) + do { + _ = try await makeService(store: store).refresh() + XCTFail("expected .noRefreshToken") + } catch { + XCTAssertEqual((error as? OAuthError)?.errorDescription, OAuthError.noRefreshToken.errorDescription) + } + XCTAssertTrue(StubURLProtocol.records(path: "/oauth/token").isEmpty, "no token request without a refresh token") + } + + func testRevokeWithoutAStoredTokenSkipsTheNetworkButStillClears() async { + let store = TokenStore(defaults: TestSupport.ephemeralDefaults()) + await makeService(store: store).revoke() + XCTAssertNil(store.tokens) + XCTAssertTrue(StubURLProtocol.records(path: "/oauth/revoke").isEmpty, "nothing to revoke without a token") + } } diff --git a/projects/ios-readplace/Tests/PKCETests.swift b/projects/ios-readplace/Tests/PKCETests.swift index 71c89d799..ee0d93515 100644 --- a/projects/ios-readplace/Tests/PKCETests.swift +++ b/projects/ios-readplace/Tests/PKCETests.swift @@ -23,11 +23,14 @@ final class PKCETests: XCTestCase { XCTAssertNotEqual(PKCE.makeCodeVerifier(), PKCE.makeCodeVerifier()) } - func testChallengeIsURLSafe() { + func testChallengeIsBase64URLAndCorrectLength() { let challenge = PKCE.challenge(for: PKCE.makeCodeVerifier()) - XCTAssertFalse(challenge.contains("+")) - XCTAssertFalse(challenge.contains("/")) - XCTAssertFalse(challenge.contains("=")) - XCTAssertFalse(challenge.isEmpty) + XCTAssertEqual(challenge.count, 43, "a SHA-256 digest base64url-encodes to 43 chars") + let allowed = CharacterSet(charactersIn: + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_") + XCTAssertTrue( + challenge.unicodeScalars.allSatisfy(allowed.contains), + "every character is in the base64url alphabet (no +, /, or =)" + ) } } diff --git a/projects/ios-readplace/Tests/ReadingListViewModelTests.swift b/projects/ios-readplace/Tests/ReadingListViewModelTests.swift index b8d083c8f..ee3dc8b90 100644 --- a/projects/ios-readplace/Tests/ReadingListViewModelTests.swift +++ b/projects/ios-readplace/Tests/ReadingListViewModelTests.swift @@ -738,46 +738,24 @@ final class ReadingListViewModelTests: XCTestCase { XCTAssertNil(viewModel.errorText) } - func testReaderMarkedReadDropsTheRowInstantlyWithoutItsOwnFetch() async { - // The reader's own POST already happened inside the webview, so the native - // side drops the row the moment the bridge fires — the unread-only list - // never shows it again behind the sheet — and issues no request of its own: - // reconciliation belongs to the converge the sheet's dismissal triggers. - StubURLProtocol.setHandler(markReadHandler { _ in .redirect(to: "/queue") }) - let viewModel = makeViewModel(store: TestSupport.loggedInStore()) - await viewModel.refresh() - let requestsAfterLoad = StubURLProtocol.records.count - - viewModel.readerMarkedRead(id: "a1") - - XCTAssertEqual(viewModel.articles.map(\.id), ["a2"], "the read row is gone before the sheet closes") - XCTAssertEqual( - StubURLProtocol.records.count, requestsAfterLoad, - "the reader already posted inside the webview; the drop itself fires nothing — the dismissal converge owns the re-read" - ) - } - - func testWebSheetDismissalAfterReaderMarkedReadConvergesAndKeepsTheRowDropped() async { - // Closing the reader converges with the server: the fresh page brings in an - // item marked unread on the website (w1), while the row the reader just - // marked read stays dropped even though the eventually-consistent GET still - // lists it — the dismissal converge carries the reader's confirmed drop. - let staleServerTruth = Fixtures.collection( - entitiesJSON: [ - Fixtures.article(id: "a1"), Fixtures.article(id: "a2"), Fixtures.article(id: "w1"), - ], - total: 3 + func testReaderStatusChangedConvergesWithTheServerWithoutInferringDirection() async { + // The reader's own POST already happened inside the webview, but the client + // can't see which direction the toggle went, so it does not infer "read" and + // drop a row — it re-reads the collection and adopts the server's truth, which + // no longer lists the read item (a1) and brings in an item marked unread on + // the website (w1). + let postAction = Fixtures.collection( + entitiesJSON: [Fixtures.article(id: "a2"), Fixtures.article(id: "w1")], total: 2 ) - StubURLProtocol.setHandler(markReadHandler(laterQueue: staleServerTruth) { _ in .redirect(to: "/queue") }) + StubURLProtocol.setHandler(markReadHandler(laterQueue: postAction) { _ in .redirect(to: "/queue") }) let viewModel = makeViewModel(store: TestSupport.loggedInStore()) await viewModel.refresh() - viewModel.readerMarkedRead(id: "a1") - await viewModel.handleWebSheetDismissal() + await viewModel.readerStatusChanged() XCTAssertEqual( viewModel.articles.map(\.id), ["a2", "w1"], - "the convergence load is adopted minus the read row, which an eventually-consistent GET must not resurrect" + "the server's re-read collection is adopted as truth; no row is dropped by inference" ) XCTAssertTrue( StubURLProtocol.records.allSatisfy { $0.request.httpMethod != "POST" }, @@ -957,7 +935,7 @@ final class ReadingListViewModelTests: XCTestCase { Article( id: id, url: "https://example.com/x", title: "X", siteName: nil, excerpt: nil, imageURL: nil, readTimeMinutes: nil, isRead: false, savedAt: nil, - actions: [], readHref: readHref + actions: [], links: [], readHref: readHref ) } @@ -997,10 +975,9 @@ final class ReadingListViewModelTests: XCTestCase { } let viewModel = makeViewModel(store: TestSupport.loggedInStore()) - let cookie = await viewModel.mintReaderSession() + let cookies = await viewModel.mintReaderSession() - XCTAssertEqual(cookie?.name, "hutch_sid") - XCTAssertEqual(cookie?.value, "sess-xyz") + XCTAssertEqual(cookies?.first?.value, "sess-xyz") XCTAssertNil(viewModel.errorText) } @@ -1010,9 +987,79 @@ final class ReadingListViewModelTests: XCTestCase { } let viewModel = makeViewModel(store: TestSupport.loggedInStore()) - let cookie = await viewModel.mintReaderSession() + let cookies = await viewModel.mintReaderSession() - XCTAssertNil(cookie, "a failed bootstrap mints no session, so the sheet shows its unavailable view") + XCTAssertNil(cookies, "a failed bootstrap mints no session, so the sheet shows its unavailable view") XCTAssertNotNil(viewModel.errorText) } + + // MARK: - Session expiry & warnings + + func testUnauthorizedLoadLogsOutWithoutAnErrorBanner() async { + let api = ReadplaceAPI( + baseURL: AppConfig.serverBaseURL, + store: TestSupport.loggedInStore(), + sessionConfiguration: TestSupport.stubbedConfiguration() + ) + var expired = false + let viewModel = ReadingListViewModel(api: api, onSessionExpired: { expired = true }) + // 401 everywhere: the entry-point load 401s, the single refresh 401s, and + // the load surfaces .unauthorized. + StubURLProtocol.setHandler { _, _ in .json(401, "{}") } + + await viewModel.refresh() + + XCTAssertTrue(expired, "a 401 whose refresh also fails logs the user out") + XCTAssertNil(viewModel.errorText, "a session-expiry logout is not shown as an error banner") + } + + func testCollectionWarningPopulatesWarningText() async { + let warnedQueue = """ + { + "class": ["collection", "articles"], + "properties": { "total": 1, "page": 1, "pageSize": 20, "warning": { "code": "not-saveable", "message": "Cannot save that link." } }, + "entities": [\(Fixtures.article(id: "a1"))], + "links": [{ "rel": ["self"], "href": "/queue" }, { "rel": ["root"], "href": "/queue" }], + "actions": [] + } + """ + StubURLProtocol.setHandler { request, _ in + request.url?.path == "/" ? .redirect(to: "/queue") : .json(200, warnedQueue) + } + let viewModel = makeViewModel(store: TestSupport.loggedInStore()) + + await viewModel.refresh() + + XCTAssertEqual(viewModel.warningText, "Cannot save that link.") + } + + func testMintReaderSessionFollowsTheServersCreateSessionAction() async { + let queueWithSession = """ + { + "class": ["collection", "articles"], + "properties": { "total": 1, "page": 1, "pageSize": 20 }, + "entities": [\(Fixtures.article(id: "a1"))], + "links": [{ "rel": ["self"], "href": "/queue" }, { "rel": ["root"], "href": "/queue" }], + "actions": [{ "name": "create-session", "href": "/custom/session", "method": "POST" }] + } + """ + StubURLProtocol.setHandler { request, _ in + switch request.url?.path { + case "/": return .redirect(to: "/queue") + case "/queue": return .json(200, queueWithSession) + case "/custom/session": return StubURLProtocol.Stub(status: 204, headers: ["Set-Cookie": "sess=v; Path=/"]) + default: return .json(404, "{}") + } + } + let viewModel = makeViewModel(store: TestSupport.loggedInStore()) + await viewModel.refresh() + + let cookies = await viewModel.mintReaderSession() + + XCTAssertEqual(cookies?.first?.value, "v") + XCTAssertTrue( + StubURLProtocol.records.contains { $0.request.url?.path == "/custom/session" }, + "the reader session mint follows the discovered create-session action, not a hard-coded route" + ) + } } diff --git a/projects/ios-readplace/Tests/ReadplaceAPITests.swift b/projects/ios-readplace/Tests/ReadplaceAPITests.swift index 624de3cef..c7f5eea89 100644 --- a/projects/ios-readplace/Tests/ReadplaceAPITests.swift +++ b/projects/ios-readplace/Tests/ReadplaceAPITests.swift @@ -671,10 +671,10 @@ final class ReadplaceAPITests: XCTestCase { ) } - let cookie = try await makeAPI(store: store).bootstrapSession() + let cookies = try await makeAPI(store: store).bootstrapSession() - XCTAssertEqual(cookie.name, "hutch_sid") - XCTAssertEqual(cookie.value, "sess-abc") + XCTAssertEqual(cookies.count, 1) + XCTAssertEqual(cookies.first?.value, "sess-abc") } func testBootstrapSessionRefreshesOnceWhenBearerExpired() async throws { @@ -693,16 +693,16 @@ final class ReadplaceAPITests: XCTestCase { } } - let cookie = try await makeAPI(store: store).bootstrapSession() + let cookies = try await makeAPI(store: store).bootstrapSession() - XCTAssertEqual(cookie.value, "fresh-sess") + XCTAssertEqual(cookies.first?.value, "fresh-sess") XCTAssertEqual(sessionAttempts, 2, "should retry once after refreshing the bearer") XCTAssertEqual(store.tokens?.accessToken, "fresh-access") } func testBootstrapSessionKeepsTheCookieOutOfTheSharedJar() async throws { let host = try XCTUnwrap(URL(string: AppConfig.serverBaseURL)?.host) - for stale in HTTPCookieStorage.shared.cookies?.filter({ $0.name == AppConfig.sessionCookieName }) ?? [] { + for stale in HTTPCookieStorage.shared.cookies?.filter({ $0.name == "hutch_sid" }) ?? [] { HTTPCookieStorage.shared.deleteCookie(stale) } StubURLProtocol.setHandler { _, _ in @@ -712,8 +712,24 @@ final class ReadplaceAPITests: XCTestCase { _ = try await makeAPI(store: TestSupport.loggedInStore()).bootstrapSession() XCTAssertNil( - HTTPCookieStorage.shared.cookies?.first { $0.name == AppConfig.sessionCookieName }, - "the minted hutch_sid cookie must never land in the process-wide shared jar" + HTTPCookieStorage.shared.cookies?.first { $0.name == "hutch_sid" }, + "the minted session cookie must never land in the process-wide shared jar" ) } + + func testBootstrapSessionFollowsADiscoveredActionsHrefAndMethod() async throws { + let store = TestSupport.loggedInStore() + StubURLProtocol.setHandler { request, _ in + XCTAssertEqual(request.url?.path, "/custom/session", "follows the action's href, not a hard-coded path") + XCTAssertEqual(request.httpMethod, "POST") + return StubURLProtocol.Stub(status: 204, headers: ["Set-Cookie": "sess=discovered; Path=/"]) + } + let action = SirenAction( + name: "create-session", href: "/custom/session", method: "POST", title: nil, type: nil, fields: nil + ) + + let cookies = try await makeAPI(store: store).bootstrapSession(action: action) + + XCTAssertEqual(cookies.first?.value, "discovered") + } } diff --git a/projects/ios-readplace/Tests/SaveSharedPageTests.swift b/projects/ios-readplace/Tests/SaveSharedPageTests.swift index a047df649..015e2c7ec 100644 --- a/projects/ios-readplace/Tests/SaveSharedPageTests.swift +++ b/projects/ios-readplace/Tests/SaveSharedPageTests.swift @@ -401,4 +401,29 @@ final class SaveSharedPageTests: XCTestCase { let posts = StubURLProtocol.records.filter { $0.request.httpMethod == "POST" } XCTAssertTrue(posts.isEmpty, "no save must be attempted when the server offers no save action") } + + func testFailsWhenQueueResponseIsUndecodable() async throws { + // The queue replied 200 with the negotiated Siren media type but a body + // that is not a Siren collection (a JSON array), so the journey surfaces + // the API's own decode message as .failed — and attempts no save. + let store = TestSupport.loggedInStore() + let captor = FakeHTMLCaptor(page: CapturedPage(rawHtml: "hi", title: "Captured", mediaType: nil)) + StubURLProtocol.setHandler { request, _ in + switch request.url?.path { + case "/": + return .redirect(to: "/queue") + case "/queue": + return .json(200, "[]") + default: + return .json(404, "{}") + } + } + + let saver = SaveSharedPage(store: store, api: makeAPI(store: store), captor: captor) + let outcome = await saver.run(url: URL(string: "https://example.com/post")!, fallbackTitle: nil, sharedPdf: nil) + + XCTAssertEqual(outcome, .failed("Could not read the server response.")) + let posts = StubURLProtocol.records.filter { $0.request.httpMethod == "POST" } + XCTAssertTrue(posts.isEmpty, "no save must be attempted when the queue cannot be decoded") + } } diff --git a/projects/ios-readplace/Tests/ShareStatusPresentationTests.swift b/projects/ios-readplace/Tests/ShareStatusPresentationTests.swift new file mode 100644 index 000000000..a6b9477c0 --- /dev/null +++ b/projects/ios-readplace/Tests/ShareStatusPresentationTests.swift @@ -0,0 +1,64 @@ +import XCTest +@testable import Readplace + +final class ShareStatusPresentationTests: XCTestCase { + private func present(_ outcome: SaveSharedOutcome) -> ShareStatusPresentation { + ShareStatusPresentation(outcome: outcome) + } + + private func message(type: String, body: String = "x") -> ServerMessage { + ServerMessage(type: type, content: ServerMessage.Content(type: "text/html", body: body)) + } + + func testSavedWithContentIsSuccess() { + let status = present(.savedWithContent) + XCTAssertEqual(status.message, "Saved with content") + XCTAssertEqual(status.symbol, "checkmark.circle.fill") + XCTAssertEqual(status.tone, .success) + } + + func testSavedLinkOnlyIsSuccess() { + let status = present(.savedLinkOnly) + XCTAssertEqual(status.message, "Saved (link only)") + XCTAssertEqual(status.tone, .success) + } + + func testNotLoggedInIsWarning() { + let status = present(.notLoggedIn) + XCTAssertEqual(status.message, "Open Readplace and sign in first.") + XCTAssertEqual(status.symbol, "person.crop.circle.badge.exclamationmark") + XCTAssertEqual(status.tone, .warning) + } + + func testNoLinkIsWarning() { + let status = present(.noLink) + XCTAssertEqual(status.message, "No link found to save.") + XCTAssertEqual(status.symbol, "link") + XCTAssertEqual(status.tone, .warning) + } + + func testNoSaveActionIsError() { + let status = present(.noSaveAction) + XCTAssertEqual(status.message, "The server offered no save action.") + XCTAssertEqual(status.tone, .error) + } + + func testRefusedJoinsMessagesAndIsWarningWhenNoneAreErrors() { + let status = present(.refused([message(type: "warning", body: "one"), message(type: "warning", body: "two")])) + XCTAssertEqual(status.message, "one\ntwo") + XCTAssertEqual(status.symbol, "lock.fill") + XCTAssertEqual(status.tone, .warning) + } + + func testRefusedIsErrorWhenAnyMessageIsAnError() { + let status = present(.refused([message(type: "warning"), message(type: "error")])) + XCTAssertEqual(status.tone, .error) + } + + func testFailedCarriesTheServerMessageAsError() { + let status = present(.failed("Something broke")) + XCTAssertEqual(status.message, "Something broke") + XCTAssertEqual(status.symbol, "exclamationmark.triangle.fill") + XCTAssertEqual(status.tone, .error) + } +} diff --git a/projects/ios-readplace/Tests/ShareURLExtractorTests.swift b/projects/ios-readplace/Tests/ShareURLExtractorTests.swift new file mode 100644 index 000000000..bdbb0afc2 --- /dev/null +++ b/projects/ios-readplace/Tests/ShareURLExtractorTests.swift @@ -0,0 +1,134 @@ +import UniformTypeIdentifiers +import XCTest +@testable import Readplace + +@MainActor +final class ShareURLExtractorTests: XCTestCase { + private func item(_ providers: [NSItemProvider], title: String? = nil) -> NSExtensionItem { + let item = NSExtensionItem() + item.attachments = providers + if let title { item.attributedContentText = NSAttributedString(string: title) } + return item + } + + private func urlProvider(_ string: String) -> NSItemProvider { + NSItemProvider(item: URL(string: string)! as NSURL, typeIdentifier: UTType.url.identifier) + } + + private func textProvider(_ text: String) -> NSItemProvider { + NSItemProvider(item: text as NSString, typeIdentifier: UTType.plainText.identifier) + } + + /// A PDF item provider backed by a real temp file so `loadFileRepresentation` + /// and the size check run. A `truncate`d sparse file gives a large logical size + /// without writing megabytes. + private func pdfProvider(logicalSize: Int = 64, name: String = "doc.pdf") throws -> NSItemProvider { + let dir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + let url = dir.appendingPathComponent(name) + var bytes = Data("%PDF-1.4\n".utf8) + if logicalSize > bytes.count { bytes.append(Data(count: min(logicalSize - bytes.count, 4096))) } + try bytes.write(to: url) + if logicalSize > bytes.count { + let handle = try FileHandle(forWritingTo: url) + try handle.truncate(atOffset: UInt64(logicalSize)) + try handle.close() + } + let provider = try XCTUnwrap(NSItemProvider(contentsOf: url)) + provider.suggestedName = name + return provider + } + + func testExtractsAWebURL() async { + let shared = await ShareURLExtractor.extract(from: [item([urlProvider("https://example.com/post")])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/post") + XCTAssertNil(shared?.pdfProvider) + } + + func testExtractsAURLFromPlainText() async { + let shared = await ShareURLExtractor.extract(from: [item([textProvider("read this https://example.com/p ok")])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/p") + } + + func testExtractsURLDeliveredAsData() async { + let data = URL(string: "https://example.com/d")!.dataRepresentation + let provider = NSItemProvider(item: data as NSData, typeIdentifier: UTType.url.identifier) + let shared = await ShareURLExtractor.extract(from: [item([provider])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/d") + } + + func testExtractsURLDeliveredAsString() async { + let provider = NSItemProvider(item: "https://example.com/s" as NSString, typeIdentifier: UTType.url.identifier) + let shared = await ShareURLExtractor.extract(from: [item([provider])]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/s") + } + + func testIgnoresANonWebURL() async { + let shared = await ShareURLExtractor.extract(from: [item([urlProvider("mailto:a@b.com")])]) + XCTAssertNil(shared, "a mailto link is not a web URL and there is no PDF, so nothing is saveable") + } + + func testFindsURLAndPDFAcrossSeparateItems() async throws { + let shared = await ShareURLExtractor.extract(from: [ + item([urlProvider("https://example.com/a")]), + item([try pdfProvider()]), + ]) + XCTAssertEqual(shared?.url?.absoluteString, "https://example.com/a") + XCTAssertNotNil(shared?.pdfProvider) + } + + func testTitleComesFromAttributedContentText() async { + let shared = await ShareURLExtractor.extract(from: [item([urlProvider("https://example.com/a")], title: "My Title")]) + XCTAssertEqual(shared?.title, "My Title") + } + + func testTitleFallsBackToPdfSuggestedNameWhenNoContentText() async throws { + let shared = await ShareURLExtractor.extract(from: [item([try pdfProvider(name: "report.pdf")])]) + XCTAssertNil(shared?.url) + XCTAssertNotNil(shared?.pdfProvider) + XCTAssertEqual(shared?.title, "report.pdf", "with no content text the title falls back to the PDF's suggested name") + } + + func testReturnsNilWhenNoURLAndNoPDF() async { + let shared = await ShareURLExtractor.extract(from: [item([textProvider("just words, no link")])]) + XCTAssertNil(shared) + } + + func testReturnsNilForNoItems() async { + let shared = await ShareURLExtractor.extract(from: []) + XCTAssertNil(shared) + } + + func testReturnsNilForANilExtensionContext() async { + let shared = await ShareURLExtractor.extract(from: nil) + XCTAssertNil(shared) + } + + func testReturnsNilForAnItemWithNoAttachments() async { + let shared = await ShareURLExtractor.extract(from: [NSExtensionItem()]) + XCTAssertNil(shared, "an item with no attachments carries no URL and no PDF") + } + + func testCoercesEachItemShapeAHostMayDeliver() { + XCTAssertEqual(ShareURLExtractor.coerceURL(from: URL(string: "https://example.com/u")! as NSURL)?.absoluteString, "https://example.com/u") + let data = URL(string: "https://example.com/d")!.dataRepresentation + XCTAssertEqual(ShareURLExtractor.coerceURL(from: data as NSData)?.absoluteString, "https://example.com/d") + XCTAssertEqual(ShareURLExtractor.coerceURL(from: "https://example.com/s" as NSString)?.absoluteString, "https://example.com/s") + } + + func testCoercionRejectsAnItemThatIsNotURLShaped() { + XCTAssertNil(ShareURLExtractor.coerceURL(from: NSNumber(value: 42))) + XCTAssertNil(ShareURLExtractor.coerceURL(from: nil)) + } + + func testLoadsPdfDataUnderTheCeiling() async throws { + let data = await ShareURLExtractor.loadPDFData(try pdfProvider(logicalSize: 256)) + XCTAssertEqual(data?.starts(with: Data("%PDF-".utf8)), true) + } + + func testLoadPdfDataRejectsAnOversizeFile() async throws { + let oversize = try pdfProvider(logicalSize: ReadplaceAPI.defaultMaxExternalContentBytes + 1) + let data = await ShareURLExtractor.loadPDFData(oversize) + XCTAssertNil(data, "a PDF over the extension's byte ceiling is not pulled into memory") + } +} diff --git a/projects/ios-readplace/Tests/SignupFlowTests.swift b/projects/ios-readplace/Tests/SignupFlowTests.swift index 3d308b79c..d4b682d61 100644 --- a/projects/ios-readplace/Tests/SignupFlowTests.swift +++ b/projects/ios-readplace/Tests/SignupFlowTests.swift @@ -21,7 +21,7 @@ final class SignupFlowTests: XCTestCase { let request = makeService(store: TestSupport.loggedInStore()).makeSignupAuthorizationRequest() let components = URLComponents(url: request.url, resolvingAgainstBaseURL: false)! - XCTAssertEqual(components.host, "readplace.com") + XCTAssertEqual(components.host, URL(string: AppConfig.serverBaseURL)?.host) XCTAssertEqual(components.path, "/oauth/authorize") let items = Dictionary(uniqueKeysWithValues: (components.queryItems ?? []).map { ($0.name, $0.value ?? "") }) XCTAssertEqual(items["client_id"], "ios-app") diff --git a/projects/ios-readplace/Tests/SirenDecodingTests.swift b/projects/ios-readplace/Tests/SirenDecodingTests.swift index 499c1d5a9..9c7c01c6b 100644 --- a/projects/ios-readplace/Tests/SirenDecodingTests.swift +++ b/projects/ios-readplace/Tests/SirenDecodingTests.swift @@ -2,6 +2,16 @@ import XCTest @testable import Readplace final class SirenDecodingTests: XCTestCase { + /// A fixed UTC instant, for asserting a parsed date equals a known point in + /// time rather than merely being non-nil. + static func utc(_ year: Int, _ month: Int, _ day: Int, _ hour: Int, _ minute: Int, _ second: Int) -> Date { + var calendar = Calendar(identifier: .gregorian) + calendar.timeZone = TimeZone(identifier: "UTC")! + return calendar.date(from: DateComponents( + year: year, month: month, day: day, hour: hour, minute: minute, second: second + ))! + } + private func decodeCollection(_ json: String) throws -> SirenCollection { try JSONDecoder().decode(SirenCollection.self, from: Data(json.utf8)) } @@ -27,7 +37,8 @@ final class SirenDecodingTests: XCTestCase { XCTAssertEqual(updateStatus.type, "application/x-www-form-urlencoded") XCTAssertEqual(updateStatus.fields?.first?.name, "status") XCTAssertEqual(article.readHref, "/queue/a1/view") - XCTAssertNotNil(article.savedAt) + XCTAssertEqual(article.savedAt, SirenDecodingTests.utc(2026, 5, 30, 10, 0, 0), + "the fixture's savedAt (2026-05-30T10:00:00.000Z) parses to that exact instant") } func testArticleAffordancesIterateAdvertisedActionsInWireOrder() throws { @@ -89,18 +100,34 @@ final class SirenDecodingTests: XCTestCase { XCTAssertFalse(article.isRead) } - func testStatusReadMarksAsRead() throws { + // The next two exercise the fallback derivation used only for an older server + // that doesn't emit the explicit read-state (the fixture omits `isRead`). + func testStatusReadMarksAsReadWhenServerOmitsIsRead() throws { let entity = try decodeEntity(Fixtures.article(status: "read")) let article = try XCTUnwrap(Article(entity: entity)) XCTAssertTrue(article.isRead) } - func testReadAtImpliesReadEvenWhenStatusUnread() throws { + func testReadAtImpliesReadWhenServerOmitsIsRead() throws { let entity = try decodeEntity(Fixtures.article(status: "unread", readAt: "2026-05-31T09:00:00.000Z")) let article = try XCTUnwrap(Article(entity: entity)) XCTAssertTrue(article.isRead) } + func testServerIsReadWinsOverTheStatusVocabulary() throws { + // The server says not-read even though `status` is "read": the explicit + // boolean is the client's read-state, not the status literal. + let entity = try decodeEntity(Fixtures.article(status: "read", isRead: false)) + let article = try XCTUnwrap(Article(entity: entity)) + XCTAssertFalse(article.isRead) + } + + func testServerIsReadTrueMarksReadRegardlessOfStatus() throws { + let entity = try decodeEntity(Fixtures.article(status: "unread", readAt: nil, isRead: true)) + let article = try XCTUnwrap(Article(entity: entity)) + XCTAssertTrue(article.isRead) + } + func testMinimalEntityFallsBackTitleToURL() throws { let json = """ { "properties": { "id": "x", "url": "https://example.com/x" } } @@ -315,9 +342,12 @@ final class SirenDecodingTests: XCTestCase { XCTAssertEqual(page.warning?.message, "Cannot save that link.") } - func testSirenDateParsesWithAndWithoutFractionalSeconds() { - XCTAssertNotNil(SirenDate.parse("2026-05-30T10:00:00.123Z")) - XCTAssertNotNil(SirenDate.parse("2026-05-30T10:00:00Z")) + func testSirenDateParsesWithAndWithoutFractionalSeconds() throws { + let base = SirenDecodingTests.utc(2026, 5, 30, 10, 0, 0) + XCTAssertEqual(try XCTUnwrap(SirenDate.parse("2026-05-30T10:00:00Z")), base) + let fractional = try XCTUnwrap(SirenDate.parse("2026-05-30T10:00:00.123Z")) + XCTAssertEqual(fractional.timeIntervalSince(base), 0.123, accuracy: 0.0005, + "the fractional variant is 123 ms after the whole-second instant") XCTAssertNil(SirenDate.parse("not-a-date")) XCTAssertNil(SirenDate.parse("")) } diff --git a/projects/ios-readplace/Tests/TestSupport.swift b/projects/ios-readplace/Tests/TestSupport.swift index 340825a27..1195e2fc7 100644 --- a/projects/ios-readplace/Tests/TestSupport.swift +++ b/projects/ios-readplace/Tests/TestSupport.swift @@ -16,11 +16,12 @@ enum TestSupport { return config } - /// A `hutch_sid` session cookie scoped to the server host, for seeding a - /// cookie jar before asserting sign-out clears it. - static func sessionCookie(value: String) -> HTTPCookie { + /// A session cookie scoped to the server host, for seeding a cookie jar before + /// asserting sign-out clears it. The name is arbitrary — the client no longer + /// selects the session cookie by name — so this uses a representative literal. + static func sessionCookie(value: String, name: String = "hutch_sid") -> HTTPCookie { HTTPCookie(properties: [ - .name: AppConfig.sessionCookieName, + .name: name, .value: value, .domain: URL(string: AppConfig.serverBaseURL)!.host!, .path: "/", @@ -135,7 +136,8 @@ enum Fixtures { readTime: Int? = 6, status: String = "unread", savedAt: String = "2026-05-30T10:00:00.000Z", - readAt: String? = nil + readAt: String? = nil, + isRead: Bool? = nil ) -> String { func field(_ key: String, _ value: String?) -> String { value.map { "\"\(key)\": \"\($0)\"" } ?? "\"\(key)\": null" @@ -143,6 +145,11 @@ enum Fixtures { func numField(_ key: String, _ value: Int?) -> String { value.map { "\"\(key)\": \($0)" } ?? "\"\(key)\": null" } + // Emitted only when set, so a fixture without it models an older server that + // doesn't advertise the explicit read-state. + func boolField(_ key: String, _ value: Bool?) -> String { + value.map { ", \"\(key)\": \($0)" } ?? "" + } return """ { "class": ["article"], @@ -158,7 +165,7 @@ enum Fixtures { \(numField("estimatedReadTimeMinutes", readTime)), "status": "\(status)", "savedAt": "\(savedAt)", - \(field("readAt", readAt)) + \(field("readAt", readAt))\(boolField("isRead", isRead)) }, "links": [{ "rel": ["read"], "href": "/queue/\(id)/view" }], "actions": [ diff --git a/projects/ios-readplace/Tests/TokenStoreTests.swift b/projects/ios-readplace/Tests/TokenStoreTests.swift index c84fff1b4..8ceb4202e 100644 --- a/projects/ios-readplace/Tests/TokenStoreTests.swift +++ b/projects/ios-readplace/Tests/TokenStoreTests.swift @@ -86,4 +86,53 @@ final class TokenStoreTests: XCTestCase { XCTAssertNil(target.value(for: .accessToken), "a partial legacy token is not a valid session") XCTAssertNil(legacy.string(forKey: TokenKey.accessToken.rawValue), "the stray partial token is cleared") } + + // MARK: - parseAppGroupId (embedded provisioning profile) + + private func profile(_ innerPlist: String) -> Data { + // A .mobileprovision is a CMS blob with a plist inside; the parser only + // scans out the slice, so wrapping it in arbitrary bytes + // models the real container without needing a signed profile. + Data("....signature-bytes....\(innerPlist)....trailer....".utf8) + } + + func testParsesTheFirstApplicationGroupFromAProfile() { + let plist = """ + + + Entitlements + com.apple.security.application-groups + group.com.rewritten.readplacegroup.other + + """ + XCTAssertEqual( + TokenStore.parseAppGroupId(fromProvisioningProfile: profile(plist)), + "group.com.rewritten.readplace" + ) + } + + func testReturnsNilWhenTheProfileHasNoPlist() { + XCTAssertNil(TokenStore.parseAppGroupId(fromProvisioningProfile: Data("no plist here".utf8))) + } + + func testReturnsNilWhenTheProfileHasNoAppGroupEntitlement() { + let plist = """ + + + Entitlementsapplication-identifierABCDE.com.x + + """ + XCTAssertNil(TokenStore.parseAppGroupId(fromProvisioningProfile: profile(plist))) + } + + func testReturnsNilWhenTheAppGroupArrayIsEmpty() { + let plist = """ + + + Entitlements + com.apple.security.application-groups + + """ + XCTAssertNil(TokenStore.parseAppGroupId(fromProvisioningProfile: profile(plist))) + } } diff --git a/projects/ios-readplace/Tests/WebResponsePolicyTests.swift b/projects/ios-readplace/Tests/WebResponsePolicyTests.swift new file mode 100644 index 000000000..cb7073567 --- /dev/null +++ b/projects/ios-readplace/Tests/WebResponsePolicyTests.swift @@ -0,0 +1,25 @@ +import XCTest +@testable import Readplace + +/// WKWebView reports a 4xx/5xx through `didFinish`, so the in-app web view must +/// reject an error status itself or it paints the server's error body. The policy +/// is a pure value tested at the 399/400 boundary. +final class WebResponsePolicyTests: XCTestCase { + func testNoStatusIsAllowed() { + XCTAssertEqual(WebResponsePolicy.decide(statusCode: nil), .allow) + } + + func testSuccessAndRedirectAreAllowed() { + XCTAssertEqual(WebResponsePolicy.decide(statusCode: 200), .allow) + XCTAssertEqual(WebResponsePolicy.decide(statusCode: 304), .allow) + } + + func testTheBoundaryIs400() { + XCTAssertEqual(WebResponsePolicy.decide(statusCode: 399), .allow) + XCTAssertEqual(WebResponsePolicy.decide(statusCode: 400), .fail) + } + + func testServerErrorsFail() { + XCTAssertEqual(WebResponsePolicy.decide(statusCode: 503), .fail) + } +} diff --git a/projects/ios-readplace/scripts/check-coverage.py b/projects/ios-readplace/scripts/check-coverage.py new file mode 100755 index 000000000..2bfd2f1a4 --- /dev/null +++ b/projects/ios-readplace/scripts/check-coverage.py @@ -0,0 +1,104 @@ +#!/usr/bin/env python3 +"""Enforce per-file line-coverage floors for the iOS app after `make test`. + +The rest of the repo enforces a 100% coverage gate; iOS had none. This reads the +`.xcresult` bundle `xcodebuild test -enableCodeCoverage YES` produced and fails +the build if any measured source file dropped below its recorded floor. + +Config (`coverage-baseline.json`, beside this script): + - `excluded`: files that are pure SwiftUI layout, UIKit shells, or WebKit / + OS-boundary glue — unit-uncoverable without UI tests. This is the Swift + analog of the repo's whole-file OS-boundary coverage exclusion. + - `floors`: the minimum whole-percent line coverage each remaining source file + must keep. A source file with neither an exclusion nor a floor must be 100%, + so a newly added logic file cannot land untested. Floors are a RATCHET: raise + them as coverage improves, never lower them. + +Uses only the system `python3` + `xcrun`, so the CI `ios-tests` job needs no +extra toolchain setup. +""" +import json +import subprocess +import sys +from pathlib import Path + +CONFIG_PATH = Path(__file__).with_name("coverage-baseline.json") + + +def load_report(xcresult: str) -> dict: + out = subprocess.run( + ["xcrun", "xccov", "view", "--report", "--json", xcresult], + capture_output=True, text=True, check=True, + ).stdout + return json.loads(out) + + +def measured_files(report: dict) -> dict[str, float]: + """Best line coverage per source basename across all non-test targets. + + A Shared file compiles into both the app and the extension target, so it + appears twice with identical coverage; keep the higher of the two. + """ + best: dict[str, float] = {} + for target in report.get("targets", []): + if "Tests" in target.get("name", ""): + continue + for f in target.get("files", []): + name = f.get("name", "") + if not name.endswith(".swift"): + continue + cov = float(f.get("lineCoverage", 0.0)) + best[name] = max(best.get(name, 0.0), cov) + return best + + +def main() -> int: + if len(sys.argv) != 2: + print("usage: check-coverage.py ", file=sys.stderr) + return 2 + + config = json.loads(CONFIG_PATH.read_text()) + excluded = config.get("excluded", {}) + floors = config.get("floors", {}) + + report = load_report(sys.argv[1]) + measured = measured_files(report) + + failures: list[str] = [] + slack: list[str] = [] + for name, cov in sorted(measured.items()): + if name in excluded: + continue + percent = round(cov * 100) + floor = floors.get(name, 100) + if percent < floor: + failures.append(f" {name}: {percent}% < floor {floor}%") + elif percent >= floor + 2: + # A 1% headroom is a deliberate buffer against minor coverage drift; + # only nudge a ratchet once a file is comfortably above its floor. + slack.append(f" {name}: {percent}% (floor {floor}%)") + + stale = sorted({n for n in list(excluded) + list(floors) if n not in measured}) + if stale: + print("warning: coverage config references files no longer measured — prune them:") + for n in stale: + print(f" {n}") + print() + + if failures: + print("iOS coverage gate FAILED — files below their floor:") + print("\n".join(failures)) + print("\nRaise the file's coverage, or (only with approval) its floor in " + f"{CONFIG_PATH.name}.") + return 1 + + print(f"iOS coverage gate passed: {len(measured) - len(excluded)} files at " + f"or above floor, {len(excluded)} excluded (view / OS-boundary).") + if slack: + print("Ratchet candidates (coverage now exceeds the floor — raise it):") + print("\n".join(slack)) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/projects/ios-readplace/scripts/coverage-baseline.json b/projects/ios-readplace/scripts/coverage-baseline.json new file mode 100644 index 000000000..6323ef461 --- /dev/null +++ b/projects/ios-readplace/scripts/coverage-baseline.json @@ -0,0 +1,37 @@ +{ + "_comment": "Per-file line-coverage floors for the iOS app, enforced by check-coverage.py after `make test`. `excluded` lists files that are pure SwiftUI layout, UIKit shells, or WebKit/OS-boundary glue — unit-uncoverable without UI tests (the Swift analog of the repo's whole-file OS-boundary coverage exclusion). Every other source file must stay at or above its floor; a source file with no floor must be 100%, so a new logic file cannot land untested. Floors are a RATCHET: raise them as coverage improves (the E-series PRs walk the logic files toward 100% and move ShareURLExtractor out of `excluded`), never lower them without approval.", + "excluded": { + "WebAuthOpeners.swift": "UIApplication URL-opening glue (OS boundary)", + "WebPageView.swift": "WKWebView SwiftUI wrapper (OS boundary)", + "ReaderWebView.swift": "WKWebView + JS-bridge SwiftUI wrapper (OS boundary); the pure bridge parser is covered by ReaderBridgeTests", + "HTMLCaptor.swift": "WKWebView navigation/capture state machine (OS boundary); the pure navigationResponseDecision is covered by HTMLCaptorTests", + "AddLinkInstructionsView.swift": "pure SwiftUI layout", + "ReaderSheet.swift": "pure SwiftUI layout", + "ArticleRow.swift": "pure SwiftUI layout", + "ReadingListView.swift": "pure SwiftUI layout", + "ShareViewController.swift": "UIKit share-extension shell (OS boundary)" + }, + "floors": { + "AppConfig.swift": 100, + "BrandColor.swift": 100, + "PendingAuthStore.swift": 100, + "ToolbarRoute.swift": 100, + "ReaderNavigation.swift": 100, + "ShareStatusPresentation.swift": 100, + "ShareURLExtractor.swift": 96, + "SirenModels.swift": 99, + "WebAuthFlow.swift": 97, + "PKCE.swift": 95, + "ReadplaceAPI.swift": 94, + "URLDetection.swift": 94, + "ReadingListViewModel.swift": 94, + "SaveSharedPage.swift": 93, + "LoginView.swift": 91, + "AffordancePresentation.swift": 91, + "TokenStore.swift": 98, + "OAuthService.swift": 97, + "KeychainTokenStorage.swift": 97, + "AppSession.swift": 82, + "ReadplaceApp.swift": 64 + } +}