Fix handling errors with empty body (#2)

wearesho-team · Jun 22, 2018 · b717029 · b717029
1 parent 282cafb
commit b717029
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 108 deletions.
diff --git a/src/Client.php b/src/Client.php
@@ -49,25 +49,20 @@ public function verify(string $response, string $remoteIp = null): Response
     /**
      * @param array $jsonResponse
      * @return Response
-     * @throws Exception
      */
     protected function createResponse(array $jsonResponse): Response
     {
-        $dateTime = \DateTime::createFromFormat(\DateTime::ATOM, $jsonResponse['challenge_ts']);
+        $dateTime = array_key_exists('challenge_ts', $jsonResponse)
+            ? \DateTime::createFromFormat(\DateTime::ATOM, $jsonResponse['challenge_ts'])
+            : null;
 
         $response = new Response(
-            $jsonResponse['success'],
             $jsonResponse['score'],
             $jsonResponse['action'],
             $dateTime,
-            $jsonResponse['hostname'],
-            $jsonResponse['error_codes'] ?? []
+            $jsonResponse['hostname']
         );
 
-        if (!$response->isSuccess()) {
-            throw new Exception($response);
-        }
-
         return $response;
     }
 
@@ -87,8 +82,15 @@ protected function parseJson(string $responseBody): array
             );
         }
 
+        if (!array_key_exists('success', $jsonResponse)) {
+            throw new \Exception('Missing required response attribute: success');
+        }
+
+        if ($jsonResponse['success'] === false) {
+            throw new Exception($jsonResponse['error-codes'] ?? []);
+        }
+
         $requiredAttributes = [
-            'success',
             'score',
             'action',
             'challenge_ts',

diff --git a/src/Exception.php b/src/Exception.php
@@ -8,30 +8,25 @@
  * Class Exception
  * @package Wearesho\ReCaptcha\V3
  */
-class Exception extends \Exception implements ExceptionInterface
+class Exception extends \Exception
 {
-    const MISSING_INPUT_SECRET = 'missing-input-secret';
-    const INVALID_INPUT_SECRET = 'invalid-input-secret';
-    const MISSING_INPUT_RESPONSE = 'missing-input-response';
-    const INVALID_INPUT_RESPONSE = 'invalid-input-response';
-    const BAD_REQUEST = 'bad-request';
+    public const MISSING_INPUT_SECRET = 'missing-input-secret';
+    public const INVALID_INPUT_SECRET = 'invalid-input-secret';
+    public const MISSING_INPUT_RESPONSE = 'missing-input-response';
+    public const INVALID_INPUT_RESPONSE = 'invalid-input-response';
+    public const BAD_REQUEST = 'bad-request';
 
-    /** @var Response */
-    protected $response;
+    /** @var array */
+    protected $errors;
 
-    public function __construct(Response $response, int $code = 0, Throwable $previous = null)
+    public function __construct(array $errors, int $code = 0, Throwable $previous = null)
     {
         $message = implode(
             ', ',
-            array_map([$this, 'getMessageByCode',], $response->getErrors())
+            array_map([$this, 'getMessageByCode',], $errors)
         );
         parent::__construct($message, $code, $previous);
-        $this->response = $response;
-    }
-
-    public function getResponse(): Response
-    {
-        return $this->response;
+        $this->errors = $errors;
     }
 
     public function getMessageByCode(string $code): string
@@ -50,4 +45,9 @@ public function getMessageByCode(string $code): string
         }
         return $code;
     }
+
+    public function getErrors(): array
+    {
+        return $this->errors;
+    }
 }
diff --git a/src/ExceptionInterface.php b/src/ExceptionInterface.php
diff --git a/src/Response.php b/src/Response.php
@@ -8,9 +8,6 @@
  */
 class Response
 {
-    /** @var bool */
-    protected $success;
-
     /** @var float */
     protected $score;
 
@@ -27,24 +24,15 @@ class Response
     protected $errors = [];
 
     public function __construct(
-        bool $success,
         float $score,
         string $action,
         \DateTime $dateTime,
-        string $hostname,
-        array $errors
+        string $hostname
     ) {
-        $this->success = $success;
         $this->score = $score;
         $this->action = $action;
         $this->dateTime = $dateTime;
         $this->hostname = $hostname;
-        $this->errors = $errors;
-    }
-
-    public function isSuccess(): bool
-    {
-        return $this->success;
     }
 
     public function getScore(): float
@@ -66,9 +54,4 @@ public function getHostname(): string
     {
         return $this->hostname;
     }
-
-    public function getErrors(): array
-    {
-        return $this->errors;
-    }
 }
diff --git a/tests/ClientTest.php b/tests/ClientTest.php
@@ -80,10 +80,6 @@ public function testParsing(): void
         );
 
         $response = $this->client->verify('response', '127.0.0.1');
-        $this->assertEquals(
-            $success,
-            $response->isSuccess()
-        );
         $this->assertEquals(
             $score,
             $response->getScore()
@@ -117,7 +113,7 @@ public function testInvalidJson(): void
      * @expectedException \Exception
      * @expectedExceptionMessage Missing required response attribute: score
      */
-    public function testMissingJsonFields()
+    public function testMissingJsonFields(): void
     {
         $this->mock->append(new GuzzleHttp\Psr7\Response(200, [], json_encode([
             'success' => true,
@@ -126,6 +122,17 @@ public function testMissingJsonFields()
         $this->client->verify('response', '127.0.0.1');
     }
 
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage Missing required response attribute: success
+     */
+    public function testMissingSuccess(): void
+    {
+        $this->mock->append(new GuzzleHttp\Psr7\Response(200, [], '{}'));
+
+        $this->client->verify('response', '127.0.0.1');
+    }
+
     /**
      * @expectedException \Wearesho\ReCaptcha\V3\Exception
      * @expectedExceptionMessage The request is invalid or malformed
@@ -139,12 +146,45 @@ public function testGeneratingException(): void
                 'action' => $action = 'test',
                 'challenge_ts' => $date = date('c'),
                 'hostname' => $hostname = 'wearesho.com',
-                'error_codes' => [
+                'error-codes' => [
                     ReCaptcha\V3\Exception::BAD_REQUEST,
                 ],
             ]))
         );
 
         $this->client->verify('response', '127.0.0.1');
     }
+
+    /**
+     * @expectedException \Wearesho\ReCaptcha\V3\Exception
+     * @expectedExceptionMessage timeout-or-duplicate
+     */
+    public function testErrorWithEmptyParams(): void
+    {
+        $this->mock->append(
+            new GuzzleHttp\Psr7\Response(200, [], json_encode([
+                'success' => false,
+                'error-codes' => [
+                    'timeout-or-duplicate'
+                ],
+            ]))
+        );
+
+        $this->client->verify('response', '127.0.0.1');
+    }
+
+    /**
+     * @expectedException \Wearesho\ReCaptcha\V3\Exception
+     * @expectedExceptionMessageRegExp /^$/
+     */
+    public function testErrorWithoutErrorCodes(): void
+    {
+        $this->mock->append(
+            new GuzzleHttp\Psr7\Response(200, [], json_encode([
+                'success' => false,
+            ]))
+        );
+
+        $this->client->verify('response', '127.0.0.1');
+    }
 }
diff --git a/tests/ExceptionTest.php b/tests/ExceptionTest.php
@@ -11,31 +11,21 @@
  */
 class ExceptionTest extends TestCase
 {
-    /** @var ReCaptcha\V3\Exception */
+    /** @var ReCaptcha\V3\Exception
+     */
     protected $exception;
 
-    /** @var ReCaptcha\V3\Response */
-    protected $response;
-
-    protected function setUp(): void
+    public function setUp(): void
     {
         parent::setUp();
-        $this->response = new ReCaptcha\V3\Response(
-            false,
-            0.5,
-            'test',
-            new \DateTime,
-            'wearesho.com',
-            [
-                ReCaptcha\V3\Exception::INVALID_INPUT_RESPONSE,
-                ReCaptcha\V3\Exception::MISSING_INPUT_RESPONSE,
-                ReCaptcha\V3\Exception::INVALID_INPUT_SECRET,
-                ReCaptcha\V3\Exception::MISSING_INPUT_SECRET,
-                ReCaptcha\V3\Exception::BAD_REQUEST,
-                'some-not-documented',
-            ]
-        );
-        $this->exception = new ReCaptcha\V3\Exception($this->response);
+        $this->exception = new ReCaptcha\V3\Exception([
+            ReCaptcha\V3\Exception::INVALID_INPUT_RESPONSE,
+            ReCaptcha\V3\Exception::MISSING_INPUT_RESPONSE,
+            ReCaptcha\V3\Exception::INVALID_INPUT_SECRET,
+            ReCaptcha\V3\Exception::MISSING_INPUT_SECRET,
+            ReCaptcha\V3\Exception::BAD_REQUEST,
+            'some-not-documented',
+        ]);
     }
 
     public function testGetMessage(): void
@@ -46,11 +36,18 @@ public function testGetMessage(): void
         );
     }
 
-    public function testGetResponse(): void
+    public function testGetErrors(): void
     {
-        $this->assertEquals(
-            $this->response,
-            $this->exception->getResponse()
+        $this->assertArraySubset(
+            [
+                ReCaptcha\V3\Exception::INVALID_INPUT_RESPONSE,
+                ReCaptcha\V3\Exception::MISSING_INPUT_RESPONSE,
+                ReCaptcha\V3\Exception::INVALID_INPUT_SECRET,
+                ReCaptcha\V3\Exception::MISSING_INPUT_SECRET,
+                ReCaptcha\V3\Exception::BAD_REQUEST,
+                'some-not-documented',
+            ],
+            $this->exception->getErrors()
         );
     }
 }
diff --git a/tests/ResponseTest.php b/tests/ResponseTest.php
@@ -11,7 +11,6 @@
  */
 class ResponseTest extends TestCase
 {
-    const SUCCESS = true;
     const SCORE = 0.6;
     const ACTION = 'login';
     const TIMESTAMP = 190000;
@@ -25,22 +24,10 @@ protected function setUp(): void
     {
         parent::setUp();
         $this->response = new ReCaptcha\V3\Response(
-            static::SUCCESS,
             static::SCORE,
             static::ACTION,
             (new \DateTime())->setTimestamp(static::TIMESTAMP),
-            static::HOSTNAME,
-            [
-                static::ERROR_CODE,
-            ]
-        );
-    }
-
-    public function testIsSuccess(): void
-    {
-        $this->assertEquals(
-            static::SUCCESS,
-            $this->response->isSuccess()
+            static::HOSTNAME
         );
     }
 
@@ -75,11 +62,4 @@ public function testGetHostName(): void
             $this->response->getHostname()
         );
     }
-
-    public function testGetErrors(): void
-    {
-        $this->assertEquals([
-            static::ERROR_CODE,
-        ], $this->response->getErrors());
-    }
 }