Refactor readline

Author: shalvah

src/TcpClient.php

@@ -5,6 +5,7 @@
 use Clue\Redis\Protocol\Factory as ProtocolFactory;
 use Clue\Redis\Protocol\Parser\ParserInterface;
 use Knuckles\Faktory\Problems\CouldntConnect;
+use Knuckles\Faktory\Problems\ProtocolError;
 use Knuckles\Faktory\Problems\UnexpectedResponse;
 use Knuckles\Faktory\Utils\Json;
 use Psr\Log\LoggerAwareInterface;
@@ -75,7 +76,7 @@ protected function handshake()
     {
         $this->readHi();
         $this->sendHello();
-        self::checkOk($this->readLine(), operation: "Handshake");
+        $this->readLine(operation: "Handshake");
     }
 
     protected function readHi()
@@ -102,13 +103,10 @@ protected function sendHello()
         $this->send("HELLO", $workerInfo);
     }
 
-    /**
-     * Send a command and raise an error if the response is not OK.
-     */
-    public function operation($command, string ...$args): void
+    public function sendAndRead($command, string ...$args): mixed
     {
         $this->send($command, ...$args);
-        self::checkOk($this->readLine(), operation: $command);
+        return $this->readLine(operation: $command);
     }
 
     public function send($command, string ...$args): void
@@ -122,7 +120,7 @@ public function send($command, string ...$args): void
         fwrite($this->connection, $message);
     }
 
-    public function readLine(?int $skipLines = 0): mixed
+    public function readLine(?int $skipLines = 0, $operation = "Operation"): mixed
     {
         if ($this->state == State::Disconnected) {
             $this->connect();
@@ -139,16 +137,11 @@ public function readLine(?int $skipLines = 0): mixed
         $messages = $this->responseParser->pushIncoming($line);
         if (empty($messages)) return null;
 
-        return $messages[0]?->getValueNative();
-    }
+        $response = $messages[0]?->getValueNative();
+        if (is_string($response) && str_starts_with($response, "ERR"))
+            throw UnexpectedResponse::from($operation, $response);
 
-    private static function checkOk(mixed $result, $operation = "Operation")
-    {
-        if ($result !== "OK") {
-            throw UnexpectedResponse::from($operation, $result);
-        }
-
-        return true;
+        return $response;
     }
 
     public function setLogger(LoggerInterface $logger): void

tests/client_test.php

@@ -0,0 +1,24 @@
+<?php
+
+use Knuckles\Faktory\Client;
+use Knuckles\Faktory\TcpClient;
+
+function client($mockTcpClient) {
+    $client = new Client();
+    $tcpClient = new \ReflectionProperty($client, 'tcpClient');
+    $tcpClient->setValue($mockTcpClient);
+    return $client;
+}
+
+it('can push and retrieve jobs', function () {
+    $mockTcpClient = Mockery::mock(TcpClient::class);
+    $mockTcpClient->shouldReceive('send', 'PUSH');
+
+    $client = client($mockTcpClient);
+    $client->push([
+        "jid" => "123861239abnadsa",
+        "jobtype" => "SomeJobClass",
+        "args" => [1, 2, true, "hello"],
+    ]);
+    expect();
+})->skip();

tests/tcp_client_test.php

@@ -70,15 +70,15 @@
     $tcpClient->connect();
 });
 
-test('->send and ->readLine() do not raise an error when the response is not OK', function () {
+test('->readLine() raises an error when the response is an ERR', function () {
     $tcpClient = tcpClient();
-    $tcpClient->send("PUSH", "Something");
-    expect($tcpClient->readLine())->toStartWith("ERR");
+    $tcpClient->send("PUSH", "Invalid payload");
+    expect(fn() => $tcpClient->readLine())->toThrow(UnexpectedResponse::class);
 });
 
-test('->operation() raises an error when the response is not OK', function () {
+test('->sendAndRead() raises an error when the response is not OK', function () {
     expect(
-        fn() => tcpClient()->operation("PUSH", "Anything")
+        fn() => tcpClient()->sendAndRead("PUSH", "Invalid data")
     )->toThrow(UnexpectedResponse::class);
 });