refactor: small code refactoring

This commit is contained in:
Stefan Hausotte 2026-05-17 21:26:19 +02:00
parent 81d8bc5b4a
commit 03829084c8
4 changed files with 116 additions and 42 deletions

View file

@ -456,7 +456,7 @@ public enum AuthenticationError: LocalizedError, Sendable, Equatable {
} }
} }
public enum ServiceError: LocalizedError, Sendable { public enum ServiceError: LocalizedError, Sendable, Equatable {
case noActiveInstance case noActiveInstance
case invalidURL case invalidURL
case invalidResponse case invalidResponse
@ -502,19 +502,25 @@ public enum ServiceError: LocalizedError, Sendable {
} }
} }
private static func normalizedHTTPMessage(_ message: String) -> String? { private struct ForgejoErrorBody: Decodable {
let message: String?
}
static func normalizedHTTPMessage(_ message: String) -> String? {
let trimmed = message.trimmingCharacters(in: .whitespacesAndNewlines) let trimmed = message.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { guard !trimmed.isEmpty else {
return nil return nil
} }
guard guard
let data = trimmed.data(using: .utf8), let data = trimmed.data(using: .utf8),
let object = try? JSONSerialization.jsonObject(with: data) as? [String: Any], let body = try? JSONDecoder().decode(ForgejoErrorBody.self, from: data)
let jsonMessage = object["message"] as? String
else { else {
return trimmed return trimmed
} }
guard let jsonMessage = body.message else {
return trimmed
}
let trimmedJSONMessage = jsonMessage.trimmingCharacters(in: .whitespacesAndNewlines) let trimmedJSONMessage = jsonMessage.trimmingCharacters(in: .whitespacesAndNewlines)
return trimmedJSONMessage.isEmpty ? trimmed : trimmedJSONMessage return trimmedJSONMessage.isEmpty ? nil : trimmedJSONMessage
} }
} }

View file

@ -173,17 +173,24 @@ public final class PullRequestService: Sendable {
do { do {
_ = try await client.performRequestNoContent(url: url, method: "POST", body: jsonData, validateStatus: true) _ = try await client.performRequestNoContent(url: url, method: "POST", body: jsonData, validateStatus: true)
} catch let error as ServiceError { } catch let error as ServiceError {
if case let .httpError(statusCode, message) = error { throw Self.collapseMergeError(error)
switch statusCode { }
case 405 where message.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty: }
throw ServiceError.notMergeable
case 409 where message.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty: static func collapseMergeError(_ error: ServiceError) -> ServiceError {
throw ServiceError.mergeConflict guard case let .httpError(statusCode, message) = error else {
default: return error
throw error }
} guard message.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
} return error
throw error }
switch statusCode {
case 405:
return .notMergeable
case 409:
return .mergeConflict
default:
return error
} }
} }

View file

@ -45,4 +45,91 @@ struct ErrorMappingTests {
#expect(AuthenticationError.certificateError.httpStatusCode == nil) #expect(AuthenticationError.certificateError.httpStatusCode == nil)
#expect(AuthenticationError.certificateError.httpErrorCategory == nil) #expect(AuthenticationError.certificateError.httpErrorCategory == nil)
} }
// MARK: - ServiceError descriptions
@Test func serviceErrorDescriptions() {
#expect(ServiceError.noActiveInstance.errorDescription == "No active Forgejo instance")
#expect(ServiceError.invalidURL.errorDescription == "Invalid URL")
#expect(ServiceError.invalidResponse.errorDescription == "Invalid response from server")
#expect(ServiceError.httpError(statusCode: 404).errorDescription == "HTTP error: 404")
#expect(ServiceError.httpError(statusCode: 500).errorDescription == "HTTP error: 500")
#expect(ServiceError.decodingFailed(detail: "missing key").errorDescription == "Decoding failed: missing key")
}
@Test func httpErrorDescriptionUsesForgejoMessageBody() {
let error = ServiceError.httpError(
statusCode: 405,
message: #"{"message":"This pull request has failing status checks"}"#,
)
#expect(error.errorDescription == "HTTP 405: This pull request has failing status checks")
}
@Test func httpErrorDescriptionFallsBackToRawBody() {
let error = ServiceError.httpError(statusCode: 500, message: "database unavailable")
#expect(error.errorDescription == "HTTP 500: database unavailable")
}
@Test func httpErrorDescriptionIgnoresWhitespaceOnlyMessage() {
let error = ServiceError.httpError(statusCode: 503, message: " \n ")
#expect(error.errorDescription == "HTTP error: 503")
}
@Test func httpErrorDescriptionFallsBackWhenJSONMessageFieldIsEmpty() {
let error = ServiceError.httpError(
statusCode: 422,
message: #"{"message":" "}"#,
)
#expect(error.errorDescription == "HTTP error: 422")
}
@Test func httpErrorDescriptionReturnsRawJSONWhenMessageFieldMissing() {
let raw = #"{"errors":["bad request"]}"#
let error = ServiceError.httpError(statusCode: 400, message: raw)
#expect(error.errorDescription == "HTTP 400: \(raw)")
}
// MARK: - Merge error collapsing
@Test func collapseMergeErrorMaps405WithEmptyBodyToNotMergeable() {
let collapsed = PullRequestService.collapseMergeError(
.httpError(statusCode: 405, message: ""),
)
#expect(collapsed == .notMergeable)
}
@Test func collapseMergeErrorMaps409WithEmptyBodyToMergeConflict() {
let collapsed = PullRequestService.collapseMergeError(
.httpError(statusCode: 409, message: " \n "),
)
#expect(collapsed == .mergeConflict)
}
@Test func collapseMergeErrorPreserves405WithBody() {
let original = ServiceError.httpError(
statusCode: 405,
message: #"{"message":"failing status checks"}"#,
)
#expect(PullRequestService.collapseMergeError(original) == original)
}
@Test func collapseMergeErrorPreserves409WithBody() {
let original = ServiceError.httpError(statusCode: 409, message: "conflict in README.md")
#expect(PullRequestService.collapseMergeError(original) == original)
}
@Test func collapseMergeErrorPassesOtherStatusCodesThrough() {
let original = ServiceError.httpError(statusCode: 500, message: "")
#expect(PullRequestService.collapseMergeError(original) == original)
}
@Test func collapseMergeErrorPassesNonHTTPErrorsThrough() {
#expect(PullRequestService.collapseMergeError(.invalidURL) == .invalidURL)
#expect(PullRequestService.collapseMergeError(.noActiveInstance) == .noActiveInstance)
}
} }

View file

@ -64,32 +64,6 @@ struct NotificationTests {
#expect(notificationSubjectNumber(from: url) == 15) #expect(notificationSubjectNumber(from: url) == 15)
} }
// MARK: - ServiceError descriptions
@Test func serviceErrorDescriptions() {
#expect(ServiceError.noActiveInstance.errorDescription == "No active Forgejo instance")
#expect(ServiceError.invalidURL.errorDescription == "Invalid URL")
#expect(ServiceError.invalidResponse.errorDescription == "Invalid response from server")
#expect(ServiceError.httpError(statusCode: 404).errorDescription == "HTTP error: 404")
#expect(ServiceError.httpError(statusCode: 500).errorDescription == "HTTP error: 500")
#expect(ServiceError.decodingFailed(detail: "missing key").errorDescription == "Decoding failed: missing key")
}
@Test func httpErrorDescriptionUsesForgejoMessageBody() {
let error = ServiceError.httpError(
statusCode: 405,
message: #"{"message":"This pull request has failing status checks"}"#,
)
#expect(error.errorDescription == "HTTP 405: This pull request has failing status checks")
}
@Test func httpErrorDescriptionFallsBackToRawBody() {
let error = ServiceError.httpError(statusCode: 500, message: "database unavailable")
#expect(error.errorDescription == "HTTP 500: database unavailable")
}
// MARK: - Full API response decoding // MARK: - Full API response decoding
@Test func decodesNotificationArray() throws { @Test func decodesNotificationArray() throws {