Skip to content

Commit

Permalink
Add new pipeline helpers and deprecate old paths
Browse files Browse the repository at this point in the history
Motivation:

As part of apple#214 we need to introduce new pipeline helpers to configure
the channel pipeline with the new handlers. This PR adds those helpers
and deprecates the old paths.

Modifications:

- Add pipeline helpers with the new streamID-less initializer
- Switch old pipeline helpers which provided codecs to use the new path
  and codecs
- Duplicate SimpleClientServerTests and ConfiguringPipelineTests to add
  frame payload based counterparts
- Deprecate older helpers and multiplexer init, delete tests which don't
  hit deprecated paths (note: these still exist in the payload based
  counterparts)
- Update Bench1Conn10kRequests to be payload based

Result:

- More 'FramePayload' based Tests
- Paths for creating an 'HTTP2Frame' based stream via
  'HTTP2StreamMultiplexer' are now deprecated
  • Loading branch information
glbrntt committed Aug 4, 2020
1 parent b03b4e8 commit 58e1bbd
Show file tree
Hide file tree
Showing 17 changed files with 2,667 additions and 1,738 deletions.
35 changes: 33 additions & 2 deletions Sources/NIOHTTP2/HTTP2PipelineHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ extension Channel {
/// - inboundStreamStateInitializer: A closure that will be called whenever the remote peer initiates a new stream. This should almost always
/// be provided, especially on servers.
/// - returns: An `EventLoopFuture` containing the `HTTP2StreamMultiplexer` inserted into this pipeline, which can be used to initiate new streams.
@available(*, deprecated, renamed: "configureHTTP2Pipeline(mode:initialLocalSettings:position:targetWindowSize:inboundStreamInitializer:)")
public func configureHTTP2Pipeline(mode: NIOHTTP2Handler.ParserMode,
initialLocalSettings: [HTTP2Setting] = nioDefaultSettings,
position: ChannelPipeline.Position = .last,
Expand All @@ -115,6 +116,7 @@ extension Channel {
/// - inboundStreamStateInitializer: A closure that will be called whenever the remote peer initiates a new stream. This should almost always
/// be provided, especially on servers.
/// - returns: An `EventLoopFuture` containing the `HTTP2StreamMultiplexer` inserted into this pipeline, which can be used to initiate new streams.
@available(*, deprecated, renamed: "configureHTTP2Pipeline(mode:initialLocalSettings:position:targetWindowSize:inboundStreamInitializer:)")
public func configureHTTP2Pipeline(mode: NIOHTTP2Handler.ParserMode,
initialLocalSettings: [HTTP2Setting] = nioDefaultSettings,
position: ChannelPipeline.Position = .last,
Expand All @@ -129,6 +131,35 @@ extension Channel {
return self.pipeline.addHandlers(handlers, position: position).map { multiplexer }
}

/// Configures a `ChannelPipeline` to speak HTTP/2.
///
/// In general this is not entirely useful by itself, as HTTP/2 is a negotiated protocol. This helper does not handle negotiation.
/// Instead, this simply adds the handlers required to speak HTTP/2 after negotiation has completed, or when agreed by prior knowledge.
/// Whenever possible use this function to setup a HTTP/2 server pipeline, as it allows that pipeline to evolve without breaking your code.
///
/// - parameters:
/// - mode: The mode this pipeline will operate in, server or client.
/// - initialLocalSettings: The settings that will be used when establishing the connection. These will be sent to the peer as part of the
/// handshake.
/// - position: The position in the pipeline into which to insert these handlers.
/// - targetWindowSize: The target size of the HTTP/2 flow control window.
/// - inboundStreamInitializer: A closure that will be called whenever the remote peer initiates a new stream. This should almost always
/// be provided, especially on servers.
/// - returns: An `EventLoopFuture` containing the `HTTP2StreamMultiplexer` inserted into this pipeline, which can be used to initiate new streams.
public func configureHTTP2Pipeline(mode: NIOHTTP2Handler.ParserMode,
initialLocalSettings: [HTTP2Setting] = nioDefaultSettings,
position: ChannelPipeline.Position = .last,
targetWindowSize: Int = 65535,
inboundStreamInitializer: ((Channel) -> EventLoopFuture<Void>)?) -> EventLoopFuture<HTTP2StreamMultiplexer> {
var handlers = [ChannelHandler]()
handlers.reserveCapacity(2) // Update this if we need to add more handlers, to avoid unnecessary reallocation.
handlers.append(NIOHTTP2Handler(mode: mode, initialSettings: initialLocalSettings))
let multiplexer = HTTP2StreamMultiplexer(mode: mode, channel: self, targetWindowSize: targetWindowSize, inboundStreamInitializer: inboundStreamInitializer)
handlers.append(multiplexer)

return self.pipeline.addHandlers(handlers, position: position).map { multiplexer }
}

/// Configures a channel to perform a HTTP/2 secure upgrade.
///
/// HTTP/2 secure upgrade uses the Application Layer Protocol Negotiation TLS extension to
Expand Down Expand Up @@ -218,8 +249,8 @@ extension Channel {
targetWindowSize: Int,
_ configurator: @escaping (Channel) -> EventLoopFuture<Void>) -> EventLoopFuture<Void> {
let h2ChannelConfigurator = { (channel: Channel) -> EventLoopFuture<Void> in
channel.configureHTTP2Pipeline(mode: .server, targetWindowSize: targetWindowSize) { (streamChannel, streamID) -> EventLoopFuture<Void> in
streamChannel.pipeline.addHandler(HTTP2ToHTTP1ServerCodec(streamID: streamID)).flatMap { () -> EventLoopFuture<Void> in
channel.configureHTTP2Pipeline(mode: .server, targetWindowSize: targetWindowSize) { streamChannel -> EventLoopFuture<Void> in
streamChannel.pipeline.addHandler(HTTP2FramePayloadToHTTP1ServerCodec()).flatMap { () -> EventLoopFuture<Void> in
configurator(streamChannel)
}
}.flatMap { (_: HTTP2StreamMultiplexer) in
Expand Down
6 changes: 4 additions & 2 deletions Sources/NIOHTTP2/HTTP2StreamMultiplexer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ public final class HTTP2StreamMultiplexer: ChannelInboundHandler, ChannelOutboun
/// receiving a `HEADERS` frame from a client. For clients, these are channels created by
/// receiving a `PUSH_PROMISE` frame from a server. To initiate a new outbound channel, use
/// `createStreamChannel`.
@available(*, deprecated, message: "The signature of 'inboundStreamStateInitializer' has changed to '(Channel) -> EventLoopFuture<Void>'")
public convenience init(mode: NIOHTTP2Handler.ParserMode, channel: Channel, targetWindowSize: Int = 65535, inboundStreamStateInitializer: ((Channel, HTTP2StreamID) -> EventLoopFuture<Void>)? = nil) {
// We default to an 8kB outbound buffer size: this is a good trade off for avoiding excessive buffering while ensuring that decent
// throughput can be maintained. We use 4kB as the low water mark.
Expand Down Expand Up @@ -259,13 +260,13 @@ public final class HTTP2StreamMultiplexer: ChannelInboundHandler, ChannelOutboun
targetWindowSize: Int = 65535,
outboundBufferSizeHighWatermark: Int = 8196,
outboundBufferSizeLowWatermark: Int = 4092,
inboundStreamStateInitializer: ((Channel) -> EventLoopFuture<Void>)? = nil) {
inboundStreamInitializer: ((Channel) -> EventLoopFuture<Void>)?) {
self.init(mode: mode,
channel: channel,
targetWindowSize: targetWindowSize,
outboundBufferSizeHighWatermark: outboundBufferSizeHighWatermark,
outboundBufferSizeLowWatermark: outboundBufferSizeLowWatermark,
inboundStreamStateInitializer: .excludesStreamID(inboundStreamStateInitializer))
inboundStreamStateInitializer: .excludesStreamID(inboundStreamInitializer))
}

/// Create a new `HTTP2StreamMultiplexer`.
Expand Down Expand Up @@ -389,6 +390,7 @@ extension HTTP2StreamMultiplexer {
/// failed if an error occurs.
/// - streamStateInitializer: A callback that will be invoked to allow you to configure the
/// `ChannelPipeline` for the newly created channel.
@available(*, deprecated, message: "The signature of 'streamStateInitializer' has changed to '(Channel) -> EventLoopFuture<Void>'")
public func createStreamChannel(promise: EventLoopPromise<Channel>?, _ streamStateInitializer: @escaping (Channel, HTTP2StreamID) -> EventLoopFuture<Void>) {
self.channel.eventLoop.execute {
let streamID = self.nextStreamID()
Expand Down
10 changes: 5 additions & 5 deletions Sources/NIOHTTP2PerformanceTester/Bench1Conn10kRequests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ func setupServer(group: EventLoopGroup) throws -> Channel {

// Set the handlers that are applied to the accepted Channels
.childChannelInitializer { channel in
return channel.configureHTTP2Pipeline(mode: .server) { (streamChannel, streamID) -> EventLoopFuture<Void> in
return streamChannel.pipeline.addHandler(HTTP2ToHTTP1ServerCodec(streamID: streamID)).flatMap { () -> EventLoopFuture<Void> in
return channel.configureHTTP2Pipeline(mode: .server) { streamChannel -> EventLoopFuture<Void> in
return streamChannel.pipeline.addHandler(HTTP2FramePayloadToHTTP1ServerCodec()).flatMap { () -> EventLoopFuture<Void> in
streamChannel.pipeline.addHandler(HTTP1TestServer())
}.flatMap { () -> EventLoopFuture<Void> in
streamChannel.pipeline.addHandler(ErrorHandler())
Expand All @@ -75,8 +75,8 @@ func setupServer(group: EventLoopGroup) throws -> Channel {

func sendOneRequest(channel: Channel, multiplexer: HTTP2StreamMultiplexer) throws -> Int {
let responseReceivedPromise = channel.eventLoop.makePromise(of: Int.self)
func requestStreamInitializer(channel: Channel, streamID: HTTP2StreamID) -> EventLoopFuture<Void> {
return channel.pipeline.addHandlers([HTTP2ToHTTP1ClientCodec(streamID: streamID, httpProtocol: .https),
func requestStreamInitializer(channel: Channel) -> EventLoopFuture<Void> {
return channel.pipeline.addHandlers([HTTP2FramePayloadToHTTP1ClientCodec(httpProtocol: .https),
SendRequestHandler(host: "127.0.0.1",
request: .init(version: .init(major: 2, minor: 0),
method: .GET,
Expand All @@ -99,7 +99,7 @@ func setupClient(group: EventLoopGroup, address: SocketAddress) throws -> (Chann
channel.pipeline.addHandler(ErrorHandler())
}
.connect(to: address).flatMap { channel in
channel.configureHTTP2Pipeline(mode: .client, position: .first) { (channel, id) -> EventLoopFuture<Void> in
channel.configureHTTP2Pipeline(mode: .client, position: .first) { channel -> EventLoopFuture<Void> in
return channel.eventLoop.makeSucceededFuture(())
}.map { (channel, $0) }
}.wait()
Expand Down
4 changes: 2 additions & 2 deletions Sources/NIOHTTP2Server/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ let bootstrap = ServerBootstrap(group: group)

// Set the handlers that are applied to the accepted Channels
.childChannelInitializer { channel in
return channel.configureHTTP2Pipeline(mode: .server) { (streamChannel, streamID) -> EventLoopFuture<Void> in
return streamChannel.pipeline.addHandler(HTTP2ToHTTP1ServerCodec(streamID: streamID)).flatMap { () -> EventLoopFuture<Void> in
return channel.configureHTTP2Pipeline(mode: .server) { streamChannel -> EventLoopFuture<Void> in
return streamChannel.pipeline.addHandler(HTTP2FramePayloadToHTTP1ServerCodec()).flatMap { () -> EventLoopFuture<Void> in
streamChannel.pipeline.addHandler(HTTP1TestServer())
}.flatMap { () -> EventLoopFuture<Void> in
streamChannel.pipeline.addHandler(ErrorHandler())
Expand Down
2 changes: 2 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class LinuxMainRunnerImpl: LinuxMainRunner {
testCase(CompoundOutboundBufferTest.allTests),
testCase(ConcurrentStreamBufferTests.allTests),
testCase(ConfiguringPipelineTests.allTests),
testCase(ConfiguringPipelineWithFramePayloadStreamsTests.allTests),
testCase(ConnectionStateMachineTests.allTests),
testCase(ControlFrameBufferTests.allTests),
testCase(HPACKCodingTests.allTests),
Expand All @@ -55,6 +56,7 @@ class LinuxMainRunnerImpl: LinuxMainRunner {
testCase(IntegerCodingTests.allTests),
testCase(OutboundFlowControlBufferTests.allTests),
testCase(ReentrancyTests.allTests),
testCase(SimpleClientServerFramePayloadStreamTests.allTests),
testCase(SimpleClientServerTests.allTests),
testCase(StreamChannelFlowControllerTests.allTests),
testCase(StreamIDTests.allTests),
Expand Down
1 change: 0 additions & 1 deletion Tests/NIOHTTP2Tests/ConfiguringPipelineTests+XCTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ extension ConfiguringPipelineTests {
("testClosingParentChannelClosesStreamChannelWithTargetWindowSize", testClosingParentChannelClosesStreamChannelWithTargetWindowSize),
("testNegotiatedHTTP2BasicPipelineCommunicates", testNegotiatedHTTP2BasicPipelineCommunicates),
("testNegotiatedHTTP2BasicPipelineCommunicatesWithTargetWindowSize", testNegotiatedHTTP2BasicPipelineCommunicatesWithTargetWindowSize),
("testNegotiatedHTTP1BasicPipelineCommunicates", testNegotiatedHTTP1BasicPipelineCommunicates),
]
}
}
Expand Down
Loading

0 comments on commit 58e1bbd

Please sign in to comment.