LineBasedFrameDecoder.decodeLast handle more frames (#229)

### Motivation:

`LineBasedFrameDecoder.decodeLast` throws an error if there is more than
one frame’s worth of bytes remaining in the buffer.

This is in violation of the `NIOSingleStepByteToMessageDecoder`protocol
requirement that this method be called in a loop until all bytes are
decoded.

### Modifications:

* The method now only throws if the decode operation could not return a
frame and there are bytes left in the buffer (previously only the latter
criterion applied).
* Method documentation is updated.
* Test added.

### Result:

`LineBasedFrameDecoder.decodeLast` can cope with more edge cases.

Co-authored-by: Rick Newton-Rogers <rnro@apple.com>

Sources/NIOExtras/LineBasedFrameDecoder.swift

@@ -80,15 +80,20 @@ public class LineBasedFrameDecoder: ByteToMessageDecoder & NIOSingleStepByteToMe
         return .needMoreData
     }
 
-    /// Decode all remaining data.
-    /// If it is not possible to consume all the data then ``NIOExtrasErrors/LeftOverBytesError`` is reported via `context.fireErrorCaught`
+    /// Decode from a `ByteBuffer` when no more data is incoming.
+    ///
+    /// Like with `decode`, this method will be called in a loop until either `nil` is returned from the method or until the input `ByteBuffer`
+    /// has no more readable bytes. If non-`nil` is returned and the `ByteBuffer` contains more readable bytes, this method will immediately
+    /// be invoked again.
+    ///
+    /// If it is not possible to decode remaining bytes into a frame then ``NIOExtrasErrors/LeftOverBytesError`` is thrown.
     /// - Parameters:
     ///   - buffer: Buffer containing the data to decode.
     ///   - seenEOF: Has end of file been seen.
     /// - Returns: The decoded object or `nil` if we require more bytes.
     public func decodeLast(buffer: inout ByteBuffer, seenEOF: Bool) throws -> InboundOut? {
         let decoded = try self.decode(buffer: &buffer)
-        if buffer.readableBytes > 0 {
+        if decoded == nil, buffer.readableBytes > 0 {
             throw NIOExtrasErrors.LeftOverBytesError(leftOverBytes: buffer)
         }
         return decoded

Tests/NIOExtrasTests/LineBasedFrameDecoderTest.swift

@@ -22,7 +22,7 @@ class LineBasedFrameDecoderTest: XCTestCase {
     private var channel: EmbeddedChannel!
     private var decoder: LineBasedFrameDecoder!
     private var handler: ByteToMessageHandler<LineBasedFrameDecoder>!
-    
+
     override func setUp() {
         self.channel = EmbeddedChannel()
         self.decoder = LineBasedFrameDecoder()
@@ -35,7 +35,7 @@ class LineBasedFrameDecoderTest: XCTestCase {
         self.handler = nil
         _ = try? self.channel.finish()
     }
-    
+
     func testDecodeOneCharacterAtATime() throws {
         let message = "abcdefghij\r"
         // we write one character at a time
@@ -48,14 +48,14 @@ class LineBasedFrameDecoderTest: XCTestCase {
         var buffer = self.channel.allocator.buffer(capacity: 1)
         buffer.writeString("\n")
         XCTAssertTrue(try self.channel.writeInbound(buffer).isFull)
-        
+
         XCTAssertNoThrow(XCTAssertEqual("abcdefghij",
                                         (try self.channel.readInbound(as: ByteBuffer.self)?.readableBytesView).map {
             String(decoding: $0[0..<10], as: Unicode.UTF8.self)
         }))
         XCTAssertTrue(try self.channel.finish().isClean)
     }
-    
+
     func testRemoveHandlerWhenBufferIsNotEmpty() throws {
         var buffer = self.channel.allocator.buffer(capacity: 8)
         buffer.writeString("foo\r\nbar")
@@ -63,7 +63,7 @@ class LineBasedFrameDecoderTest: XCTestCase {
         var outputBuffer: ByteBuffer? = try self.channel.readInbound()
         XCTAssertEqual(3, outputBuffer?.readableBytes)
         XCTAssertEqual("foo", outputBuffer?.readString(length: 3))
-        
+
         let removeFuture = self.channel.pipeline.removeHandler(self.handler)
         (self.channel.eventLoop as! EmbeddedEventLoop).run()
         XCTAssertNoThrow(try removeFuture.wait())
@@ -72,53 +72,53 @@ class LineBasedFrameDecoderTest: XCTestCase {
                 XCTFail()
                 return
             }
-            
+
             var expectedBuffer = self.channel.allocator.buffer(capacity: 7)
             expectedBuffer.writeString("bar")
             XCTAssertEqual(error.leftOverBytes, expectedBuffer)
         }
         XCTAssertTrue(try self.channel.finish().isClean)
     }
-    
+
     func testRemoveHandlerWhenBufferIsEmpty() throws {
         var buffer = self.channel.allocator.buffer(capacity: 4)
         buffer.writeString("foo\n")
         XCTAssertTrue(try self.channel.writeInbound(buffer).isFull)
-        
+
         var outputBuffer: ByteBuffer? = try self.channel.readInbound()
         XCTAssertEqual("foo", outputBuffer?.readString(length: 3))
-        
+
         let removeFuture = self.channel.pipeline.removeHandler(self.handler)
         (self.channel.eventLoop as! EmbeddedEventLoop).run()
         XCTAssertNoThrow(try removeFuture.wait())
         XCTAssertNoThrow(try self.channel.throwIfErrorCaught())
         XCTAssertTrue(try self.channel.finish().isClean)
     }
-    
+
     func testEmptyLine() throws {
         var buffer = self.channel.allocator.buffer(capacity: 1)
         buffer.writeString("\n")
         XCTAssertTrue(try self.channel.writeInbound(buffer).isFull)
-        
+
         var outputBuffer: ByteBuffer? = try self.channel.readInbound()
         XCTAssertEqual("", outputBuffer?.readString(length: 0))
         XCTAssertTrue(try self.channel.finish().isClean)
     }
-    
+
     func testEmptyBuffer() throws {
         var buffer = self.channel.allocator.buffer(capacity: 1)
         buffer.writeString("")
         XCTAssertTrue(try self.channel.writeInbound(buffer).isEmpty)
         XCTAssertTrue(try self.channel.finish().isClean)
     }
-    
+
     func testChannelInactiveWithLeftOverBytes() throws {
         // add some data to the buffer
         var buffer = self.channel.allocator.buffer(capacity: 2)
         // read "abc" so the reader index is not 0
         buffer.writeString("hi")
         XCTAssertTrue(try self.channel.writeInbound(buffer).isEmpty)
-        
+
         try self.channel.close().wait()
         XCTAssertThrowsError(try self.channel.throwIfErrorCaught()) { error in
             guard let error = error as? NIOExtrasErrors.LeftOverBytesError else {
@@ -269,4 +269,28 @@ class LineBasedFrameDecoderTest: XCTestCase {
             }
         }
     }
+
+    func testBasicSingleStepEOFDuringDecoding() {
+        let decoder = LineBasedFrameDecoder()
+        let b2mp = NIOSingleStepByteToMessageProcessor(decoder)
+        var callCount = 0
+        XCTAssertNoThrow(try b2mp.process(buffer: ByteBuffer(string: "1\n\n2\n3\n")) { line in
+            callCount += 1
+            switch callCount {
+            case 1:
+                XCTAssertEqual(ByteBuffer(string: "1"), line)
+                XCTAssertNoThrow(try b2mp.finishProcessing(seenEOF: true) { line in
+                    print(line)
+                })
+            case 2:
+                XCTAssertEqual(ByteBuffer(string: ""), line)
+            case 3:
+                XCTAssertEqual(ByteBuffer(string: "2"), line)
+            case 4:
+                XCTAssertEqual(ByteBuffer(string: "3"), line)
+            default:
+                XCTFail("not expecting call no \(callCount)")
+            }
+        })
+    }
 }