diff options
| author | Christian Cleberg <[email protected]> | 2026-07-15 21:33:59 -0500 |
|---|---|---|
| committer | GitHub <[email protected]> | 2026-07-15 21:33:59 -0500 |
| commit | b9ec80716ea015de5b6b31395fdc5ff03191398c (patch) | |
| tree | b1badacbdc22a2fda80bb6a94c72eecd3b596431 /Hutch/Networking/SRHTClient.swift | |
| parent | 87cf19bb18dd9423d4c1558700cc64898db07003 (diff) | |
| download | hutch-3.6.0.tar.gz hutch-3.6.0.tar.bz2 hutch-3.6.0.zip | |
Phase 1: close the write gaps (#3)v3.6.0
* refactor: collapse duplicated request paths in SRHTClient
Five request paths each repeated the token guard, header setup, status-code
handling, and a ~35-line #if DEBUG logging block. The file carried that block
five times over.
Extract makeAuthorizedRequest, send, and encodedGraphQLBody, and route execute,
executeAndCache, executeMultipartFiles, and performGraphQLRequest through them.
executeMultipart is now the single-file case of executeMultipartFiles, which it
already was byte for byte. 938 lines to 612, with one copy of the logging block.
fetchText keeps its own guard: it is a GET to an allowlisted URL and must not
run GraphQL error checks over what is usually a plain-text build log.
One behavior change falls out. executeAndCache wrote the raw response to the
cache before decoding, so a 200 carrying GraphQL errors was cached and then
thrown. Routing it through performGraphQLRequest surfaces those errors first,
so error payloads are no longer cached.
* feat: edit and delete tickets
updateTicket and deleteTicket both existed in todo.sr.ht's API but were never
called, so a ticket could be filed and its status changed but its subject and
body were frozen from the moment it was created, and it could never be removed.
Edit opens a sheet seeded with the current subject and body. The input carries
only fields that actually changed, so an edit cannot clobber a field the user
did not touch, and Save stays disabled until something differs.
Clearing the body sends an explicit null via updateValue rather than a nil
subscript assignment, which would drop the key and silently leave the old body
in place — the same trap fixed for repository descriptions in 7ffef07.
Delete is destructive and irreversible, so it sits behind a confirmation
dialog naming the ticket and pops the detail view on success.
* feat: subscribe to and unsubscribe from tickets
ticketSubscribe and ticketUnsubscribe existed in the API but were never called,
so email notifications for a ticket could only be managed on the web.
Ticket.subscription is null when the user is not subscribed, so the detail
query now reads it and the menu reflects real server state rather than
guessing. The toggle updates optimistically and reverts on failure, so the
control never claims a subscription that did not take.
Decoded into the private payload rather than TicketDetail, which is Codable and
cached — adding a field there would have changed the cached shape and touched
every optimistic-update construction site.
* feat: subscribe to and unsubscribe from trackers
trackerSubscribe and trackerUnsubscribe existed in the API but were never
called. Tracker.subscription is null when not subscribed, so the state can be
read rather than guessed.
The read is a separate uncached query. The tickets query it sits beside is
paginated and cached, and a per-user subscription has no business riding along
in page payloads or being served stale from disk.
Unsubscribe passes tickets: false, so leaving a tracker does not silently drop
subscriptions to individual tickets the user opted into.
* feat: unsubscribe from mailing lists
mailingListUnsubscribe existed in the API but was never called, so the list of
subscriptions was readable and nothing more.
Scoped to unsubscribe. MailingList has no subscription field, unlike Ticket and
Tracker, so per-list state is only knowable from the subscriptions query — which
is exactly what builds this view. Subscribing would need a list the user is by
definition not subscribed to, and sr.ht exposes no discovery API to find one
(see SCOPE.md on hub.sr.ht), so there is nowhere honest to put that action yet.
The row is removed optimistically and restored if the mutation fails. The
confirmation says plainly that Hutch cannot resubscribe, since it cannot.
* feat: manage todo and lists email preferences
updatePreferences existed on both services but was never called, so these were
web-only settings.
The two services expose preferences/updatePreferences under identical names but
with different fields — notifySelf on todo, copySelf on lists — and there is no
shared preferences service, so both are read and written side by side. They load
concurrently and one service being unreachable does not hide the other's toggle.
These are server-side and apply beyond Hutch, unlike the @AppStorage toggles
above them in Settings, so the footer says so and each toggle reverts if its
mutation fails.
* refactor: drop the memory-only cache path
Two executeCached overloads existed with different return types and semantics:
one doing stale-while-revalidate against the persistent cache with TTLs, the
other only consulting the in-memory responseCache. The second was an easy thing
to reach for by mistake, since the compiler picked it purely on argument labels.
It turned out to be dead. All 38 call sites already used the TTL-aware
overload, and the memory-only one was the sole caller of executeAndCache, so
both are removed. Its doc comment promised refresh "via the onRefresh callback",
which the signature has not had for some time.
SRHTClient is now 569 lines, down from 938 before this branch.
responseCache stays as the in-memory layer behind cachedPayload and the three
view models that read it directly.
* chore: bump to 3.6.0 and record Phase 1
MARKETING_VERSION 3.5.0 -> 3.6.0, build 87 -> 88.
* fix: decode preferences responses on the main actor
The module sets SWIFT_DEFAULT_ACTOR_ISOLATION = MainActor, so the response
types are implicitly main-actor isolated and their Decodable conformances are
too. Decoding straight from an `async let` used those conformances from a
nonisolated context, which warns today and is an error in the Swift 6 language
mode.
Move each fetch into its own method and `async let` over those instead, so
decoding stays on the main actor. This is what HomeViewModel.loadDashboard
already does, and the concurrency is unaffected — the network work still
overlaps, since execute suspends and frees the actor.
Diffstat (limited to 'Hutch/Networking/SRHTClient.swift')
| -rw-r--r-- | Hutch/Networking/SRHTClient.swift | 455 |
1 files changed, 43 insertions, 412 deletions
diff --git a/Hutch/Networking/SRHTClient.swift b/Hutch/Networking/SRHTClient.swift index 37fd6b8..94531f4 100644 --- a/Hutch/Networking/SRHTClient.swift +++ b/Hutch/Networking/SRHTClient.swift @@ -69,101 +69,8 @@ final class SRHTClient: Sendable { variables: [String: any Sendable]? = nil, responseType _: T.Type ) async throws -> T { - guard let token = tokenLock.withLock({ $0 }), !token.isEmpty else { - throw SRHTError.unauthorized - } - - // Build request - var request = URLRequest(url: service.url) - request.httpMethod = "POST" - request.setValue(Bundle.main.hutchUserAgent, forHTTPHeaderField: "User-Agent") - request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") - request.setValue("application/json", forHTTPHeaderField: "Content-Type") - - let body = GraphQLRequestBody( - query: query, - variables: variables?.mapValues { AnyCodable($0) } - ) - request.httpBody = try encoder.encode(body) - - // Execute - let (data, response): (Data, URLResponse) - do { - (data, response) = try await session.data(for: request) - } catch { - throw SRHTError.networkError(error) - } - - // Check HTTP status - if let http = response as? HTTPURLResponse { - if http.statusCode == 401 { - throw SRHTError.unauthorized - } - if !(200...299).contains(http.statusCode) { - try throwGraphQLErrorsIfPresent(in: data) - throw SRHTError.httpError(http.statusCode) - } - } - - try throwGraphQLErrorsIfPresent(in: data) - - // Decode GraphQL response envelope - let graphQLResponse: GraphQLResponse<T> - do { - graphQLResponse = try decoder.decode(GraphQLResponse<T>.self, from: data) - } catch { - #if DEBUG - let responseBody = String(data: data, encoding: .utf8) ?? "<non-utf8 response>" - let variablesDescription = String(describing: variables) - if let decodingError = error as? DecodingError { - logger.error( - """ - Decoding failed for \(String(describing: T.self), privacy: .public) - service: \(service.rawValue, privacy: .public) - query: - \(query, privacy: .public) - variables: - \(variablesDescription, privacy: .public) - decodingError: - \(String(describing: decodingError), privacy: .public) - response: - \(responseBody, privacy: .public) - """ - ) - } else { - logger.error( - """ - Decoding failed for \(String(describing: T.self), privacy: .public) - service: \(service.rawValue, privacy: .public) - query: - \(query, privacy: .public) - variables: - \(variablesDescription, privacy: .public) - error: - \(String(describing: error), privacy: .public) - response: - \(responseBody, privacy: .public) - """ - ) - } - #else - logger.error("Decoding failed for \(String(describing: T.self), privacy: .public): \(error, privacy: .public)") - #endif - throw SRHTError.decodingError(error) - } - - // Surface GraphQL-level errors - if let errors = graphQLResponse.errors, !errors.isEmpty { - throw SRHTError.graphQLErrors(errors) - } - - guard let result = graphQLResponse.data else { - throw SRHTError.decodingError( - DecodingError.dataCorrupted(.init(codingPath: [], debugDescription: "No data in response")) - ) - } - - return result + let data = try await performGraphQLRequest(service: service, query: query, variables: variables) + return try decodeGraphQLData(data, service: service, query: query, variables: variables) } func executeCached<T: Decodable>( @@ -308,132 +215,13 @@ final class SRHTClient: Sendable { file: MultipartUploadFile, responseType _: T.Type ) async throws -> T { - guard let token = tokenLock.withLock({ $0 }), !token.isEmpty else { - throw SRHTError.unauthorized - } - - let boundary = "Boundary-\(UUID().uuidString)" - - var request = URLRequest(url: service.url) - request.httpMethod = "POST" - request.setValue(Bundle.main.hutchUserAgent, forHTTPHeaderField: "User-Agent") - request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") - request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") - - // Build the operations JSON (file variable mapped to null) - let operationsBody = GraphQLRequestBody( + try await executeMultipartFiles( + service: service, query: query, - variables: variables.mapValues { AnyCodable($0) } + variables: variables, + files: [file], + responseType: T.self ) - let operationsData = try encoder.encode(operationsBody) - - // Build the map JSON: { "0": ["variables.<variablePath>"] } - let mapDict = ["0": ["variables.\(file.variablePath)"]] - let mapData = try encoder.encode(mapDict) - - // Assemble multipart body - var body = Data() - - // Part: operations - body.append("--\(boundary)\r\n") - body.append("Content-Disposition: form-data; name=\"operations\"\r\n") - body.append("Content-Type: application/json\r\n\r\n") - body.append(operationsData) - body.append("\r\n") - - // Part: map - body.append("--\(boundary)\r\n") - body.append("Content-Disposition: form-data; name=\"map\"\r\n") - body.append("Content-Type: application/json\r\n\r\n") - body.append(mapData) - body.append("\r\n") - - // Part: file - body.append("--\(boundary)\r\n") - body.append("Content-Disposition: form-data; name=\"0\"; filename=\"\(file.fileName)\"\r\n") - body.append("Content-Type: \(file.mimeType)\r\n\r\n") - body.append(file.fileData) - body.append("\r\n") - - // Closing boundary - body.append("--\(boundary)--\r\n") - - request.httpBody = body - - let (data, response): (Data, URLResponse) - do { - (data, response) = try await session.data(for: request) - } catch { - throw SRHTError.networkError(error) - } - - if let http = response as? HTTPURLResponse { - if http.statusCode == 401 { - throw SRHTError.unauthorized - } - if !(200...299).contains(http.statusCode) { - try throwGraphQLErrorsIfPresent(in: data) - throw SRHTError.httpError(http.statusCode) - } - } - - try throwGraphQLErrorsIfPresent(in: data) - - let graphQLResponse: GraphQLResponse<T> - do { - graphQLResponse = try decoder.decode(GraphQLResponse<T>.self, from: data) - } catch { - #if DEBUG - let responseBody = String(data: data, encoding: .utf8) ?? "<non-utf8 response>" - let variablesDescription = String(describing: variables) - if let decodingError = error as? DecodingError { - logger.error( - """ - Decoding failed for \(String(describing: T.self), privacy: .public) - service: \(service.rawValue, privacy: .public) - query: - \(query, privacy: .public) - variables: - \(variablesDescription, privacy: .public) - decodingError: - \(String(describing: decodingError), privacy: .public) - response: - \(responseBody, privacy: .public) - """ - ) - } else { - logger.error( - """ - Decoding failed for \(String(describing: T.self), privacy: .public) - service: \(service.rawValue, privacy: .public) - query: - \(query, privacy: .public) - variables: - \(variablesDescription, privacy: .public) - error: - \(String(describing: error), privacy: .public) - response: - \(responseBody, privacy: .public) - """ - ) - } - #else - logger.error("Decoding failed for \(String(describing: T.self), privacy: .public): \(error, privacy: .public)") - #endif - throw SRHTError.decodingError(error) - } - - if let errors = graphQLResponse.errors, !errors.isEmpty { - throw SRHTError.graphQLErrors(errors) - } - - guard let result = graphQLResponse.data else { - throw SRHTError.decodingError( - DecodingError.dataCorrupted(.init(codingPath: [], debugDescription: "No data in response")) - ) - } - - return result } func executeMultipartFiles<T: Decodable>( @@ -443,23 +231,13 @@ final class SRHTClient: Sendable { files: [MultipartUploadFile], responseType _: T.Type ) async throws -> T { - guard let token = tokenLock.withLock({ $0 }), !token.isEmpty else { - throw SRHTError.unauthorized - } - let boundary = "Boundary-\(UUID().uuidString)" - - var request = URLRequest(url: service.url) - request.httpMethod = "POST" - request.setValue(Bundle.main.hutchUserAgent, forHTTPHeaderField: "User-Agent") - request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") - request.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type") - - let operationsBody = GraphQLRequestBody( - query: query, - variables: variables.mapValues { AnyCodable($0) } + var request = try makeAuthorizedRequest( + service: service, + contentType: "multipart/form-data; boundary=\(boundary)" ) - let operationsData = try encoder.encode(operationsBody) + + let operationsData = try encodedGraphQLBody(query: query, variables: variables) let mapDict = Dictionary(uniqueKeysWithValues: files.enumerated().map { index, file in (String(index), ["variables.\(file.variablePath)"]) @@ -491,174 +269,10 @@ final class SRHTClient: Sendable { body.append("--\(boundary)--\r\n") request.httpBody = body - let (data, response): (Data, URLResponse) - do { - (data, response) = try await session.data(for: request) - } catch { - throw SRHTError.networkError(error) - } - - if let http = response as? HTTPURLResponse { - if http.statusCode == 401 { - throw SRHTError.unauthorized - } - if !(200...299).contains(http.statusCode) { - try throwGraphQLErrorsIfPresent(in: data) - throw SRHTError.httpError(http.statusCode) - } - } - - try throwGraphQLErrorsIfPresent(in: data) - - let graphQLResponse: GraphQLResponse<T> - do { - graphQLResponse = try decoder.decode(GraphQLResponse<T>.self, from: data) - } catch { - throw SRHTError.decodingError(error) - } - - if let errors = graphQLResponse.errors, !errors.isEmpty { - throw SRHTError.graphQLErrors(errors) - } - - guard let result = graphQLResponse.data else { - throw SRHTError.decodingError( - DecodingError.dataCorrupted(.init(codingPath: [], debugDescription: "No data in response")) - ) - } - - return result + let data = try await send(request) + return try decodeGraphQLData(data, service: service, query: query, variables: variables) } - // MARK: - Cached Execute - - /// Execute a query and cache the raw response data. Returns cached data - /// immediately on cache hit, then refreshes in the background via the - /// `onRefresh` callback. - func executeCached<T: Decodable>( - service: SRHTService, - query: String, - variables: [String: any Sendable]? = nil, - responseType _: T.Type, - cacheKey: String - ) async throws -> T { - // Try cache first - if let cachedData = responseCache.get(forKey: cacheKey), - let cached = try? decoder.decode(GraphQLResponse<T>.self, from: cachedData), - let data = cached.data { - return data - } - - // No cache hit — fetch normally - return try await executeAndCache( - service: service, - query: query, - variables: variables, - responseType: T.self, - cacheKey: cacheKey - ) - } - - /// Execute a query, cache the raw data, and return the decoded result. - func executeAndCache<T: Decodable>( - service: SRHTService, - query: String, - variables: [String: any Sendable]? = nil, - responseType _: T.Type, - cacheKey: String - ) async throws -> T { - guard let token = tokenLock.withLock({ $0 }), !token.isEmpty else { - throw SRHTError.unauthorized - } - - var request = URLRequest(url: service.url) - request.httpMethod = "POST" - request.setValue(Bundle.main.hutchUserAgent, forHTTPHeaderField: "User-Agent") - request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") - request.setValue("application/json", forHTTPHeaderField: "Content-Type") - - let body = GraphQLRequestBody( - query: query, - variables: variables?.mapValues { AnyCodable($0) } - ) - request.httpBody = try encoder.encode(body) - - let (data, response): (Data, URLResponse) - do { - (data, response) = try await session.data(for: request) - } catch { - throw SRHTError.networkError(error) - } - - if let http = response as? HTTPURLResponse { - if http.statusCode == 401 { - throw SRHTError.unauthorized - } - if !(200...299).contains(http.statusCode) { - try throwGraphQLErrorsIfPresent(in: data) - throw SRHTError.httpError(http.statusCode) - } - } - - // Cache the raw response data before decoding - responseCache.set(data, forKey: cacheKey) - - let graphQLResponse: GraphQLResponse<T> - do { - graphQLResponse = try decoder.decode(GraphQLResponse<T>.self, from: data) - } catch { - #if DEBUG - let responseBody = String(data: data, encoding: .utf8) ?? "<non-utf8 response>" - let variablesDescription = String(describing: variables) - if let decodingError = error as? DecodingError { - logger.error( - """ - Decoding failed for \(String(describing: T.self), privacy: .public) - service: \(service.rawValue, privacy: .public) - query: - \(query, privacy: .public) - variables: - \(variablesDescription, privacy: .public) - decodingError: - \(String(describing: decodingError), privacy: .public) - response: - \(responseBody, privacy: .public) - """ - ) - } else { - logger.error( - """ - Decoding failed for \(String(describing: T.self), privacy: .public) - service: \(service.rawValue, privacy: .public) - query: - \(query, privacy: .public) - variables: - \(variablesDescription, privacy: .public) - error: - \(String(describing: error), privacy: .public) - response: - \(responseBody, privacy: .public) - """ - ) - } - #else - logger.error("Decoding failed for \(String(describing: T.self), privacy: .public): \(error, privacy: .public)") - #endif - throw SRHTError.decodingError(error) - } - - if let errors = graphQLResponse.errors, !errors.isEmpty { - throw SRHTError.graphQLErrors(errors) - } - - guard let result = graphQLResponse.data else { - throw SRHTError.decodingError( - DecodingError.dataCorrupted(.init(codingPath: [], debugDescription: "No data in response")) - ) - } - - return result - } // MARK: - Plain-text fetch @@ -748,11 +362,9 @@ final class SRHTClient: Sendable { // MARK: - Data Helper private extension SRHTClient { - func performGraphQLRequest( - service: SRHTService, - query: String, - variables: [String: any Sendable]? - ) async throws -> Data { + /// Builds an authorized POST for `service`. Throws ``SRHTError/unauthorized`` + /// when no token is set, so callers never have to guard separately. + func makeAuthorizedRequest(service: SRHTService, contentType: String) throws -> URLRequest { guard let token = tokenLock.withLock({ $0 }), !token.isEmpty else { throw SRHTError.unauthorized } @@ -761,14 +373,14 @@ private extension SRHTClient { request.httpMethod = "POST" request.setValue(Bundle.main.hutchUserAgent, forHTTPHeaderField: "User-Agent") request.setValue("Bearer \(token)", forHTTPHeaderField: "Authorization") - request.setValue("application/json", forHTTPHeaderField: "Content-Type") - - let body = GraphQLRequestBody( - query: query, - variables: variables?.mapValues { AnyCodable($0) } - ) - request.httpBody = try encoder.encode(body) + request.setValue(contentType, forHTTPHeaderField: "Content-Type") + return request + } + /// Sends a prepared request and returns the raw body, mapping transport and + /// HTTP failures onto ``SRHTError``. sr.ht reports GraphQL errors under a 200 + /// as often as under a 4xx, so both paths check the envelope. + func send(_ request: URLRequest) async throws -> Data { let (data, response): (Data, URLResponse) do { (data, response) = try await session.data(for: request) @@ -790,6 +402,25 @@ private extension SRHTClient { return data } + func encodedGraphQLBody(query: String, variables: [String: any Sendable]?) throws -> Data { + try encoder.encode( + GraphQLRequestBody( + query: query, + variables: variables?.mapValues { AnyCodable($0) } + ) + ) + } + + func performGraphQLRequest( + service: SRHTService, + query: String, + variables: [String: any Sendable]? + ) async throws -> Data { + var request = try makeAuthorizedRequest(service: service, contentType: "application/json") + request.httpBody = try encodedGraphQLBody(query: query, variables: variables) + return try await send(request) + } + func decodeGraphQLData<T: Decodable>( _ data: Data, service: SRHTService, |
