Skip to content

Commit

Permalink
Fix the typed HTTP upgrade compiler guards (#2594)
Browse files Browse the repository at this point in the history
The compiler guards were unnecessarily complex and I also didn't cover 3 methods where we used the types.
  • Loading branch information
FranzBusch committed Nov 16, 2023
1 parent c461993 commit 702cd7c
Show file tree
Hide file tree
Showing 11 changed files with 26 additions and 14 deletions.
2 changes: 1 addition & 1 deletion Sources/NIOHTTP1/HTTPTypedPipelineSetup.swift
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
import NIOCore

// MARK: - Server pipeline configuration
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOHTTP1/NIOTypedHTTPClientUpgradeHandler.swift
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
import NIOCore

/// An object that implements `NIOTypedHTTPClientProtocolUpgrader` knows how to handle HTTP upgrade to
Expand Down
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
import DequeModule
import NIOCore

Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOHTTP1/NIOTypedHTTPServerUpgradeHandler.swift
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
import NIOCore

/// An object that implements `NIOTypedHTTPServerProtocolUpgrader` knows how to handle HTTP upgrade to
Expand Down
Expand Up @@ -11,7 +11,7 @@
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
import DequeModule
import NIOCore

Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOWebSocket/NIOWebSocketClientUpgrader.swift
Expand Up @@ -74,7 +74,7 @@ public final class NIOWebSocketClientUpgrader: NIOHTTPClientProtocolUpgrader {
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
/// A `NIOTypedHTTPClientProtocolUpgrader` that knows how to do the WebSocket upgrade dance.
///
/// This upgrader assumes that the `HTTPClientUpgradeHandler` will create and send the upgrade request.
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOWebSocket/NIOWebSocketServerUpgrader.swift
Expand Up @@ -175,7 +175,7 @@ public final class NIOWebSocketServerUpgrader: HTTPServerProtocolUpgrader, @unch
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
/// A `NIOTypedHTTPServerProtocolUpgrader` that knows how to do the WebSocket upgrade dance.
///
/// Users may frequently want to offer multiple websocket endpoints on the same port. For this
Expand Down
10 changes: 7 additions & 3 deletions Tests/NIOHTTP1Tests/HTTPClientUpgradeTests.swift
Expand Up @@ -32,7 +32,7 @@ extension EmbeddedChannel {
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
protocol TypedAndUntypedHTTPClientProtocolUpgrader: NIOHTTPClientProtocolUpgrader, NIOTypedHTTPClientProtocolUpgrader where UpgradeResult == Bool {}
#else
Expand Down Expand Up @@ -287,9 +287,13 @@ private final class RecordingHTTPHandler: ChannelInboundHandler, RemovableChanne
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
private func assertPipelineContainsUpgradeHandler(channel: Channel) {
let handler = try? channel.pipeline.syncOperations.handler(type: NIOHTTPClientUpgradeHandler.self)
let typedHandler = try? channel.pipeline.syncOperations.handler(type: NIOTypedHTTPClientUpgradeHandler<Bool>.self)

#if !canImport(Darwin) || swift(>=5.10)
let typedHandler = try? channel.pipeline.syncOperations.handler(type: NIOTypedHTTPClientUpgradeHandler<Bool>.self)
XCTAssertTrue(handler != nil || typedHandler != nil)
#else
XCTAssertTrue(handler != nil)
#endif
}

@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
Expand Down Expand Up @@ -953,7 +957,7 @@ class HTTPClientUpgradeTestCase: XCTestCase {
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
final class TypedHTTPClientUpgradeTestCase: HTTPClientUpgradeTestCase {
override func setUpClientChannel(
Expand Down
12 changes: 10 additions & 2 deletions Tests/NIOHTTP1Tests/HTTPServerUpgradeTests.swift
Expand Up @@ -36,11 +36,15 @@ extension ChannelPipeline {

@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
fileprivate func assertContainsUpgrader() {
#if !canImport(Darwin) || swift(>=5.10)
do {
_ = try self.context(handlerType: NIOTypedHTTPServerUpgradeHandler<Bool>.self).wait()
} catch {
self.assertContains(handlerType: HTTPServerUpgradeHandler.self)
}
#else
self.assertContains(handlerType: HTTPServerUpgradeHandler.self)
#endif
}

func assertContains<Handler: ChannelHandler>(handlerType: Handler.Type) {
Expand All @@ -63,6 +67,7 @@ extension ChannelPipeline {
// handler present, keep waiting
usleep(50)
} catch ChannelPipelineError.notFound {
#if !canImport(Darwin) || swift(>=5.10)
// Checking if the typed variant is present
do {
_ = try self.context(handlerType: NIOTypedHTTPServerUpgradeHandler<Bool>.self).wait()
Expand All @@ -72,6 +77,9 @@ extension ChannelPipeline {
// No upgrader, we're good.
return
}
#else
return
#endif
}
}

Expand Down Expand Up @@ -174,7 +182,7 @@ internal func assertResponseIs(response: String, expectedResponseLine: String, e
XCTAssertEqual(lines.count, 0)
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
protocol TypedAndUntypedHTTPServerProtocolUpgrader: HTTPServerProtocolUpgrader, NIOTypedHTTPServerProtocolUpgrader where UpgradeResult == Bool {}
#else
Expand Down Expand Up @@ -1557,7 +1565,7 @@ class HTTPServerUpgradeTestCase: XCTestCase {
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
final class TypedHTTPServerUpgradeTestCase: HTTPServerUpgradeTestCase {
fileprivate override func setUpTestWithAutoremoval(
Expand Down
2 changes: 1 addition & 1 deletion Tests/NIOWebSocketTests/WebSocketClientEndToEndTests.swift
Expand Up @@ -405,7 +405,7 @@ class WebSocketClientEndToEndTests: XCTestCase {
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
final class TypedWebSocketClientEndToEndTests: WebSocketClientEndToEndTests {
func setUpClientChannel(
Expand Down
2 changes: 1 addition & 1 deletion Tests/NIOWebSocketTests/WebSocketServerEndToEndTests.swift
Expand Up @@ -527,7 +527,7 @@ class WebSocketServerEndToEndTests: XCTestCase {
}
}

#if !canImport(Darwin) || (canImport(Darwin) && swift(>=5.10))
#if !canImport(Darwin) || swift(>=5.10)
@available(macOS 13, iOS 16, tvOS 16, watchOS 9, *)
final class TypedWebSocketServerEndToEndTests: WebSocketServerEndToEndTests {
override func createTestFixtures(
Expand Down

0 comments on commit 702cd7c

Please sign in to comment.