Remove ByteBufferAllocator constructor argument from HttpResponseEncoder

Motivation:

We can allocate the buffer on the fly if needed, no need to add the constructor argument.

Modifications:

Just allocate the buffer in the methods itself that need it.

Result:

Easier to construct encoder and cleaner code.
This commit is contained in:
Norman Maurer 2017-11-02 12:56:00 +01:00
parent 4e04a275c0
commit c22e5ead6b
5 changed files with 36 additions and 35 deletions

View File

@ -20,11 +20,8 @@ public final class HTTPResponseEncoder : ChannelOutboundHandler {
public typealias OutboundOut = IOData
private var isChunked = false
private var scratchBuffer: ByteBuffer
public init(allocator: ByteBufferAllocator = ByteBufferAllocator()) {
self.scratchBuffer = allocator.buffer(capacity: 256)
}
public init () { }
private func writeChunk(ctx: ChannelHandlerContext, chunk: IOData, promise: Promise<Void>?) {
let (mW1, mW2, mW3): (Promise<()>?, Promise<()>?, Promise<()>?)
@ -47,19 +44,20 @@ public final class HTTPResponseEncoder : ChannelOutboundHandler {
/* we don't want to copy the chunk unnecessarily and therefore call write an annoyingly large number of times */
if self.isChunked {
self.scratchBuffer.clear()
var buffer = ctx.channel!.allocator.buffer(capacity: 32)
let len = String(readableBytes, radix: 16)
self.scratchBuffer.write(string: len)
self.scratchBuffer.write(staticString: "\r\n")
ctx.write(data: self.wrapOutboundOut(.byteBuffer(self.scratchBuffer)), promise: mW1)
}
ctx.write(data: self.wrapOutboundOut(chunk), promise: mW2)
buffer.write(string: len)
buffer.write(staticString: "\r\n")
ctx.write(data: self.wrapOutboundOut(.byteBuffer(buffer)), promise: mW1)
if self.isChunked {
self.scratchBuffer.clear()
self.scratchBuffer.write(staticString: "\r\n")
ctx.write(data: self.wrapOutboundOut(.byteBuffer(self.scratchBuffer)), promise: mW3)
ctx.write(data: self.wrapOutboundOut(chunk), promise: mW2)
// Just clear the buffer as we depend on the COW semantics.
buffer.clear()
buffer.write(staticString: "\r\n")
ctx.write(data: self.wrapOutboundOut(.byteBuffer(buffer)), promise: mW3)
} else {
ctx.write(data: self.wrapOutboundOut(chunk), promise: mW2)
}
}
@ -69,27 +67,30 @@ public final class HTTPResponseEncoder : ChannelOutboundHandler {
sanitizeTransportHeaders(status: response.status, headers: &response.headers, version: response.version)
self.isChunked = response.headers["transfer-encoding"].contains("chunked")
self.scratchBuffer.clear()
response.version.write(buffer: &self.scratchBuffer)
self.scratchBuffer.write(staticString: " ")
response.status.write(buffer: &self.scratchBuffer)
self.scratchBuffer.write(staticString: "\r\n")
response.headers.write(buffer: &self.scratchBuffer)
var buffer = ctx.channel!.allocator.buffer(capacity: 256)
ctx.write(data: self.wrapOutboundOut(.byteBuffer(self.scratchBuffer)), promise: promise)
response.version.write(buffer: &buffer)
buffer.write(staticString: " ")
response.status.write(buffer: &buffer)
buffer.write(staticString: "\r\n")
response.headers.write(buffer: &buffer)
ctx.write(data: self.wrapOutboundOut(.byteBuffer(buffer)), promise: promise)
case .some(.body(let bodyPart)):
self.writeChunk(ctx: ctx, chunk: bodyPart, promise: promise)
case .some(.end(let trailers)):
switch (self.isChunked, promise) {
case (true, let p):
self.scratchBuffer.clear()
var buffer: ByteBuffer
if let trailers = trailers {
self.scratchBuffer.write(staticString: "0\r\n")
trailers.write(buffer: &self.scratchBuffer) // Includes trailing CRLF.
buffer = ctx.channel!.allocator.buffer(capacity: 256)
buffer.write(staticString: "0\r\n")
trailers.write(buffer: &buffer) // Includes trailing CRLF.
} else {
self.scratchBuffer.write(staticString: "0\r\n\r\n")
buffer = ctx.channel!.allocator.buffer(capacity: 8)
buffer.write(staticString: "0\r\n\r\n")
}
ctx.write(data: self.wrapOutboundOut(.byteBuffer(self.scratchBuffer)), promise: p)
ctx.write(data: self.wrapOutboundOut(.byteBuffer(buffer)), promise: p)
case (false, .some(let p)):
// Not chunked so we have nothing to write. However, we don't want to satisfy this promise out-of-order
// so we issue a zero-length write down the chain.

View File

@ -76,7 +76,7 @@ let bootstrap = ServerBootstrap(group: group)
// Set the handlers that are applied to the accepted Channels
.handler(childHandler: ChannelInitializer(initChannel: { channel in
return channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).then(callback: { v2 in
return channel.pipeline.add(handler: HTTPResponseEncoder()).then(callback: { v2 in
return channel.pipeline.add(handler: HTTPRequestDecoder()).then(callback: { v2 in
return channel.pipeline.add(handler: HTTPHandler())
})

View File

@ -29,7 +29,7 @@ class HTTPEncoderTests: XCTestCase {
XCTAssertFalse(try! channel.finish())
}
try! channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).wait()
try! channel.pipeline.add(handler: HTTPResponseEncoder()).wait()
var switchingResponse = HTTPResponseHead(version: HTTPVersion(major: 1, minor:1), status: status)
switchingResponse.headers = headers
try! channel.writeOutbound(data: HTTPResponsePart.head(switchingResponse))
@ -98,7 +98,7 @@ class HTTPEncoderTests: XCTestCase {
XCTAssertFalse(try! channel.finish())
}
try! channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).wait()
try! channel.pipeline.add(handler: HTTPResponseEncoder()).wait()
// This response contains neither Transfer-Encoding: chunked or Content-Length.
let response = HTTPResponseHead(version: HTTPVersion(major: 1, minor:0), status: .ok)

View File

@ -277,7 +277,7 @@ class HTTPServerClientTest : XCTestCase {
.handler(childHandler: ChannelInitializer(initChannel: { channel in
// Ensure we not read faster then we can write by adding the BackPressureHandler into the pipeline.
channel.pipeline.add(handler: HTTPRequestDecoder()).then {
channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).then {
channel.pipeline.add(handler: HTTPResponseEncoder()).then {
channel.pipeline.add(handler: httpHandler)
}
}
@ -334,7 +334,7 @@ class HTTPServerClientTest : XCTestCase {
.handler(childHandler: ChannelInitializer(initChannel: { channel in
// Ensure we not read faster then we can write by adding the BackPressureHandler into the pipeline.
channel.pipeline.add(handler: HTTPRequestDecoder()).then {
channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).then {
channel.pipeline.add(handler: HTTPResponseEncoder()).then {
channel.pipeline.add(handler: httpHandler)
}
}
@ -392,7 +392,7 @@ class HTTPServerClientTest : XCTestCase {
.option(option: ChannelOptions.Socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.handler(childHandler: ChannelInitializer(initChannel: { channel in
channel.pipeline.add(handler: HTTPRequestDecoder()).then {
channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).then {
channel.pipeline.add(handler: HTTPResponseEncoder()).then {
channel.pipeline.add(handler: httpHandler)
}
}
@ -449,7 +449,7 @@ class HTTPServerClientTest : XCTestCase {
.handler(childHandler: ChannelInitializer(initChannel: { channel in
// Ensure we not read faster then we can write by adding the BackPressureHandler into the pipeline.
channel.pipeline.add(handler: HTTPRequestDecoder()).then {
channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).then {
channel.pipeline.add(handler: HTTPResponseEncoder()).then {
channel.pipeline.add(handler: httpHandler)
}
}

View File

@ -32,7 +32,7 @@ private func serverHTTPChannel(group: EventLoopGroup, handlers: [ChannelHandler]
.option(option: ChannelOptions.Socket(SocketOptionLevel(SOL_SOCKET), SO_REUSEADDR), value: 1)
.handler(childHandler: ChannelInitializer(initChannel: { channel in
channel.pipeline.add(handler: HTTPRequestDecoder()).then {
channel.pipeline.add(handler: HTTPResponseEncoder(allocator: channel.allocator)).then {
channel.pipeline.add(handler: HTTPResponseEncoder()).then {
let futureResults = handlers.map { channel.pipeline.add(handler: $0) }
return Future<Void>.andAll(futureResults, eventLoop: channel.eventLoop)
}