Cleanup RedisDataDecoder and fully implement ByteToMessageDecoder conformance

Author: Mordil

Sources/NIORedis/Coders/RedisDataDecoder.swift

@@ -5,6 +5,9 @@ import NIO
 ///
 /// See: https://redis.io/topics/protocol
 final class RedisDataDecoder: ByteToMessageDecoder {
+    /// `ByteToMessageDecoder`
+    public typealias InboundOut = RedisData
+
     /// See `ByteToMessageDecoder.cumulationBuffer`
     var cumulationBuffer: ByteBuffer?
 
@@ -16,14 +19,12 @@ final class RedisDataDecoder: ByteToMessageDecoder {
         case .notYetParsed:
             return .needMoreData
 
-        case .parsed:
+        case .parsed(let redisData):
+            ctx.fireChannelRead(wrapInboundOut(redisData))
+            buffer.moveReaderIndex(forwardBy: position)
             return .continue
         }
     }
-
-    private let encoding = String.Encoding.utf8
-
-    public typealias InboundOut = Int
 }
 
 // MARK: RESP Parsing
@@ -104,7 +105,7 @@ extension RedisDataDecoder {
         // Move the tip of the message position for recursive parsing to just after the newline
         position += expectedNewlinePosition + 1
 
-        return String(bytes: bytes[ ..<(expectedNewlinePosition - 1) ], encoding: encoding)
+        return String(bytes: bytes[ ..<(expectedNewlinePosition - 1) ], encoding: .utf8)
     }
 
     /// See https://redis.io/topics/protocol#resp-integers

Tests/NIORedisTests/Data/RedisDataDecoder+ByteToMessageDecoderTests.swift

@@ -0,0 +1,76 @@
+import NIO
+@testable import NIORedis
+import XCTest
+
+final class RedisDataDecoderByteToMessageDecoderTests: XCTestCase {
+    private let decoder = RedisDataDecoder()
+    private let allocator = ByteBufferAllocator()
+
+    func testDecoding_partial_needsMoreData() throws {
+        XCTAssertEqual(try decodeTest("+OK\r"), .needMoreData)
+        XCTAssertEqual(try decodeTest("$2\r\n"), .needMoreData)
+        XCTAssertEqual(try decodeTest("*2\r\n:1\r\n"), .needMoreData)
+        XCTAssertEqual(try decodeTest("*2\r\n*1\r\n"), .needMoreData)
+        XCTAssertEqual(try decodeTest("-ERR test\r"), .needMoreData)
+        XCTAssertEqual(try decodeTest(":2"), .needMoreData)
+    }
+
+    func testDecoding_badMessage_throws() {
+        do {
+            _ = try decodeTest("&3\r\n").0
+            XCTFail("Failed to properly throw error")
+        } catch { XCTAssertTrue(error is RedisError) }
+    }
+
+    private static let completeMessages = [
+        "+OK\r\n",
+        "$2\r\naa\r\n",
+        "*2\r\n:1\r\n:2\r\n",
+        "*2\r\n*1\r\n:1\r\n:2\r\n",
+        "-ERR test\r\n",
+        ":2\r\n"
+    ]
+
+    func testDecoding_complete_continues() throws {
+        for message in RedisDataDecoderByteToMessageDecoderTests.completeMessages {
+            XCTAssertEqual(try decodeTest(message), .continue)
+        }
+    }
+
+    func testDecoding_complete_movesReaderIndex() throws {
+        for message in RedisDataDecoderByteToMessageDecoderTests.completeMessages {
+            let messageByteSize = message.convertedToData()
+            XCTAssertEqual(try decodeTest(message).1, messageByteSize.count)
+        }
+    }
+
+    private func decodeTest(_ input: String) throws -> DecodingState {
+        var buffer = allocator.buffer(capacity: 256)
+        return try decodeTest(input, buffer: &buffer)
+    }
+
+    private func decodeTest(_ input: String) throws -> (DecodingState, Int) {
+        var buffer = allocator.buffer(capacity: 256)
+        return (try decodeTest(input, buffer: &buffer), buffer.readerIndex)
+    }
+
+    private func decodeTest(_ input: String, buffer: inout ByteBuffer) throws -> DecodingState {
+        let embeddedChannel = EmbeddedChannel()
+        defer { _ = try? embeddedChannel.finish() }
+        try embeddedChannel.pipeline.add(handler: decoder).wait()
+        let context = try embeddedChannel.pipeline.context(handler: decoder).wait()
+
+        buffer.write(string: input)
+
+        return try decoder.decode(ctx: context, buffer: &buffer)
+    }
+}
+
+extension RedisDataDecoderByteToMessageDecoderTests {
+    static var allTests = [
+        ("testDecoding_partial_needsMoreData", testDecoding_partial_needsMoreData),
+        ("testDecoding_badMessage_throws", testDecoding_badMessage_throws),
+        ("testDecoding_complete_continues", testDecoding_complete_continues),
+        ("testDecoding_complete_movesReaderIndex", testDecoding_complete_movesReaderIndex),
+    ]
+}

Tests/NIORedisTests/XCTestManifests.swift

@@ -4,7 +4,8 @@ import XCTest
 public func allTests() -> [XCTestCaseEntry] {
     return [
         testCase(NIORedisTests.allTests),
-        testCase(RedisDataDecoderParsingTests.allTests)
+        testCase(RedisDataDecoderParsingTests.allTests),
+        testCase(RedisDataDecoderByteToMessageDecoderTests.allTests),
     ]
 }
 #endif