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 1007086fd..521495300 100644 --- a/projects/hutch/src/runtime/web/api/collection-siren.test.ts +++ b/projects/hutch/src/runtime/web/api/collection-siren.test.ts @@ -258,6 +258,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 c30dbdf4b..b012541a7 100644 --- a/projects/hutch/src/runtime/web/api/collection-siren.ts +++ b/projects/hutch/src/runtime/web/api/collection-siren.ts @@ -137,6 +137,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/ios-readplace/App/AffordancePresentation.swift b/projects/ios-readplace/App/AffordancePresentation.swift index 3eacf0e55..9872abde0 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 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/ReadingListView.swift b/projects/ios-readplace/App/ReadingListView.swift index 466c243d2..6aaaded01 100644 --- a/projects/ios-readplace/App/ReadingListView.swift +++ b/projects/ios-readplace/App/ReadingListView.swift @@ -202,13 +202,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 676d78dbd..1aad5402e 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 @@ -234,7 +238,7 @@ final class ReadingListViewModel: ObservableObject { /// 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 @@ -271,6 +275,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/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/Shared/ReadplaceAPI.swift b/projects/ios-readplace/Shared/ReadplaceAPI.swift index 86ba68ba3..1ed6c0ba6 100644 --- a/projects/ios-readplace/Shared/ReadplaceAPI.swift +++ b/projects/ios-readplace/Shared/ReadplaceAPI.swift @@ -182,18 +182,29 @@ final class ReadplaceAPI { // MARK: - Reader session - /// Mints a browser session from the current bearer token via the - /// session-bootstrap endpoint 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. The client never - /// selects a cookie by name — it forwards whatever the server set — so a server - /// cookie rename needs no app release. 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() 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) diff --git a/projects/ios-readplace/Tests/AffordancePresentationTests.swift b/projects/ios-readplace/Tests/AffordancePresentationTests.swift index 77c139e2c..94c1f8d4f 100644 --- a/projects/ios-readplace/Tests/AffordancePresentationTests.swift +++ b/projects/ios-readplace/Tests/AffordancePresentationTests.swift @@ -17,10 +17,11 @@ final class AffordancePresentationTests: XCTestCase { XCTAssertTrue(presentation.isToolbarControl) } - func testCaptureOnlySavesAreNotToolbarControls() { - // iOS can't capture a page from the toolbar, so save-html/save-content render - // nowhere in the toolbar — they are reached only through the Share Sheet. - for token in ["save-html", "save-content"] { + func testMachineOnlyActionsAreNotToolbarControls() { + // iOS can't capture a page from the toolbar, so save-html/save-content are + // reached only through the Share Sheet; create-session is invoked bespoke to + // mint the reader session. None render in the toolbar. + for token in ["save-html", "save-content", "create-session"] { let presentation = AffordancePresentation(token: token) XCTAssertFalse(presentation.isToolbarControl, "\(token) must not present as a toolbar control") XCTAssertFalse(presentation.removesItem) diff --git a/projects/ios-readplace/Tests/ItemRouteTests.swift b/projects/ios-readplace/Tests/ItemRouteTests.swift new file mode 100644 index 000000000..e67822b20 --- /dev/null +++ b/projects/ios-readplace/Tests/ItemRouteTests.swift @@ -0,0 +1,35 @@ +import XCTest +@testable import Readplace + +/// The row maps each advertised item control to a side effect purely, so a +/// link-only control opens (rather than being dropped) and a destructive control +/// routes through a confirmation before invoking — decided here, not by a per-name +/// check in the view. +final class ItemRouteTests: XCTestCase { + private func action(name: String, href: String? = "/queue/a1/status", title: String? = nil) -> 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/ReadingListViewModelTests.swift b/projects/ios-readplace/Tests/ReadingListViewModelTests.swift index 84df31442..bbb63fae3 100644 --- a/projects/ios-readplace/Tests/ReadingListViewModelTests.swift +++ b/projects/ios-readplace/Tests/ReadingListViewModelTests.swift @@ -898,4 +898,34 @@ final class ReadingListViewModelTests: XCTestCase { 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 de7a224a9..c7f5eea89 100644 --- a/projects/ios-readplace/Tests/ReadplaceAPITests.swift +++ b/projects/ios-readplace/Tests/ReadplaceAPITests.swift @@ -716,4 +716,20 @@ final class ReadplaceAPITests: XCTestCase { "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/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) + } +}