From a14bc5d995f06fa2c398fa7a6bb6f98f1fd54446 Mon Sep 17 00:00:00 2001 From: Simon Leeb <52261246+sliemeobn@users.noreply.github.com> Date: Sun, 8 Dec 2024 15:52:10 +0100 Subject: [PATCH] support for non-sendable responses (#6) * support for non-sendable responses * adjusted github CI --- .github/workflows/api-breakage.yml | 22 ++++++++ .github/workflows/ci.yaml | 2 +- .gitignore | 3 +- Package.swift | 2 +- Sources/VaporElementary/HTMLResponse.swift | 33 ++++++++++-- .../HTMLResponseBodyWriter.swift | 4 +- .../NonSendableHTMLResponseTests.swift | 51 +++++++++++++++++++ 7 files changed, 108 insertions(+), 9 deletions(-) create mode 100644 .github/workflows/api-breakage.yml create mode 100644 Tests/VaporElementaryTests/NonSendableHTMLResponseTests.swift diff --git a/.github/workflows/api-breakage.yml b/.github/workflows/api-breakage.yml new file mode 100644 index 0000000..8cd479d --- /dev/null +++ b/.github/workflows/api-breakage.yml @@ -0,0 +1,22 @@ +name: API breaking changes + +on: + pull_request: + +jobs: + linux: + runs-on: ubuntu-latest + timeout-minutes: 15 + container: + image: swift:latest + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + # https://github.com/actions/checkout/issues/766 + - name: Mark the workspace as safe + run: git config --global --add safe.directory ${GITHUB_WORKSPACE} + - name: API breaking changes + run: | + swift package diagnose-api-breaking-changes origin/${GITHUB_BASE_REF} diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index de1d49b..27b82b4 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -16,7 +16,7 @@ jobs: timeout-minutes: 15 strategy: matrix: - image: ["swift:5.10", "swiftlang/swift:nightly-6.0-jammy"] + image: ["swift:5.10", "swift:6.0"] container: image: ${{ matrix.image }} diff --git a/.gitignore b/.gitignore index f129058..28dc311 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,5 @@ DerivedData/ .swiftpm/configuration/registries.json .swiftpm/xcode/package.xcworkspace/contents.xcworkspacedata .netrc -Package.resolved \ No newline at end of file +Package.resolved +.vscode/ \ No newline at end of file diff --git a/Package.swift b/Package.swift index 483777c..2843857 100644 --- a/Package.swift +++ b/Package.swift @@ -23,7 +23,7 @@ let package = Package( ], dependencies: [ .package(url: "https://github.com/vapor/vapor.git", from: "4.102.0"), - .package(url: "https://github.com/sliemeobn/elementary.git", .upToNextMajor(from: "0.3.0")), + .package(url: "https://github.com/sliemeobn/elementary.git", from: "0.4.3"), ], targets: [ .target( diff --git a/Sources/VaporElementary/HTMLResponse.swift b/Sources/VaporElementary/HTMLResponse.swift index dc05721..0d843e1 100644 --- a/Sources/VaporElementary/HTMLResponse.swift +++ b/Sources/VaporElementary/HTMLResponse.swift @@ -14,11 +14,14 @@ import Vapor /// } /// } /// } +/// +/// NOTE: For non-sendable HTML values, the resulting response body can only be written once. +/// Multiple writes will result in a runtime error. /// ``` public struct HTMLResponse: Sendable { // NOTE: The Sendable requirement on Content can probably be removed in Swift 6 using a sending parameter, and some fancy ~Copyable @unchecked Sendable box type. // We only need to pass the HTML value to the response generator body closure - private let content: any HTML & Sendable + private let value: _SendableAnyHTMLBox /// The number of bytes to write to the response body at a time. /// @@ -45,13 +48,30 @@ public struct HTMLResponse: Sendable { /// - additionalHeaders: Additional headers to be merged with predefined headers. /// - content: The `HTML` content to render in the response. public init(chunkSize: Int = 1024, additionalHeaders: HTTPHeaders = [:], @HTMLBuilder content: () -> some HTML & Sendable) { + self.init(chunkSize: chunkSize, additionalHeaders: additionalHeaders, value: .init(content())) + } + + #if compiler(>=6.0) + @available(macOS 15, *) + /// Creates a new HTMLResponse + /// + /// - Parameters: + /// - chunkSize: The number of bytes to write to the response body at a time. + /// - additionalHeaders: Additional headers to be merged with predefined headers. + /// - content: The `HTML` content to render in the response. + public init(chunkSize: Int = 1024, additionalHeaders: HTTPHeaders = [:], @HTMLBuilder content: () -> sending some HTML) { + self.init(chunkSize: chunkSize, additionalHeaders: additionalHeaders, value: .init(content())) + } + #endif + + init(chunkSize: Int, additionalHeaders: HTTPHeaders = [:], value: _SendableAnyHTMLBox) { self.chunkSize = chunkSize if additionalHeaders.contains(name: .contentType) { self.headers = additionalHeaders } else { self.headers.add(contentsOf: additionalHeaders) } - self.content = content() + self.value = value } } @@ -60,8 +80,13 @@ extension HTMLResponse: AsyncResponseEncodable { Response( status: .ok, headers: self.headers, - body: .init(asyncStream: { [content, chunkSize] writer in - try await writer.writeHTML(content, chunkSize: chunkSize) + body: .init(asyncStream: { [value, chunkSize] writer in + guard let html = value.tryTake() else { + assertionFailure("Non-sendable HTML value consumed more than once") + request.logger.error("Non-sendable HTML value consumed more than once") + throw Abort(.internalServerError) + } + try await writer.writeHTML(html, chunkSize: chunkSize) try await writer.write(.end) }) ) diff --git a/Sources/VaporElementary/HTMLResponseBodyWriter.swift b/Sources/VaporElementary/HTMLResponseBodyWriter.swift index 8c55017..8fc9768 100644 --- a/Sources/VaporElementary/HTMLResponseBodyWriter.swift +++ b/Sources/VaporElementary/HTMLResponseBodyWriter.swift @@ -1,9 +1,9 @@ import Elementary import Vapor -struct HTMLResponseBodyStreamWriter: HTMLStreamWriter { +struct HTMLResponseBodyStreamWriter: HTMLStreamWriter { let allocator: ByteBufferAllocator = .init() - var writer: any AsyncBodyStreamWriter + var writer: Writer mutating func write(_ bytes: ArraySlice) async throws { try await self.writer.writeBuffer(self.allocator.buffer(bytes: bytes)) diff --git a/Tests/VaporElementaryTests/NonSendableHTMLResponseTests.swift b/Tests/VaporElementaryTests/NonSendableHTMLResponseTests.swift new file mode 100644 index 0000000..2a3132b --- /dev/null +++ b/Tests/VaporElementaryTests/NonSendableHTMLResponseTests.swift @@ -0,0 +1,51 @@ +import Elementary +import Vapor +import VaporElementary +import XCTest +import XCTVapor + +final class NonSendableHTMLResponseTests: XCTestCase { + var app: Application! + + override func setUp() async throws { + self.app = try await Application.make(.testing) + } + + override func tearDown() async throws { + try await self.app.asyncShutdown() + } + + func testAllowsSendableValuesToBeWrittenTwice() async throws { + self.app.get { req in + let html = HTMLResponse { "Hello" } + _ = try await html.encodeResponse(for: req).body.collect(on: req.eventLoop) + return html + } + + let response = try await app.sendRequest(.GET, "/") + XCTAssertEqual(response.status, .ok) + XCTAssertEqual(String(buffer: response.body), #"Hello"#) + } + + #if compiler(>=6.0) + func testRespondsWithANonSendable() async throws { + guard #available(macOS 15.0, *) else { + throw XCTSkip("Test requires macOS 15.0") + } + self.app.get { _ in HTMLResponse { div { NonSendableHTML() } } } + + let response = try await app.sendRequest(.GET, "/") + XCTAssertEqual(response.status, .ok) + XCTAssertEqual(String(buffer: response.body), #"
Hello
"#) + } + #endif +} + +@available(*, unavailable) +extension NonSendableHTML: Sendable {} + +struct NonSendableHTML: HTML { + var content: some HTML { + "Hello" + } +}