Skip to content

Commit

Permalink
Enforce passing an explicit status code in ApiResponse
Browse files Browse the repository at this point in the history
  • Loading branch information
apfelbox committed Mar 7, 2024
1 parent 0f31d72 commit 786368c
Show file tree
Hide file tree
Showing 7 changed files with 98 additions and 9 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
3.1.0 (unreleased)
=====

* (deprecation) Deprecate passing a `bool` to the constructor of `ApiResponse`.
* (feature) Enforce passing an explicit status code to the constructor of `ApiReponse`.

3.0.2
=====

Expand Down
7 changes: 7 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
3.x to 4.0
==========

* Passing a `bool` to the constructor of `ApiResponse` was removed, pass a status code instead.
* The method `ApiResponse::withStatusCode()` was removed. Pass the status code in the constructor instead.


2.x to 3.0
==========

Expand Down
29 changes: 27 additions & 2 deletions src/Api/ApiResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,20 @@ class ApiResponse
/**
*/
public function __construct (
public readonly bool $ok,
bool|int $statusCode,
public readonly mixed $data = null,
)
{
$this->statusCode = $ok ? 200 : 400;
// @todo remove check in v4, make parameter int only and readonly.
if (\is_int($statusCode))
{
$this->statusCode = $statusCode;
}
else
{
\trigger_deprecation("21torr/rad", "3.1.0", "Passing a bool as first value to ApiResponse is deprecated. Pass the status code instead.");
$this->statusCode = $statusCode ? 200 : 400;
}
}


Expand All @@ -23,6 +32,13 @@ public function __construct (
*/
public function withStatusCode (int $statusCode) : self
{
// @todo remove method in v4
\trigger_deprecation(
"21torr/rad",
"3.1.0",
"Calling ApiResponse::withStatusCode() is deprecated, pass the status code in the constructor instead.",
);

$this->statusCode = $statusCode;
return $this;
}
Expand All @@ -36,4 +52,13 @@ public function withError (?string $error) : self
$this->error = $error;
return $this;
}


/**
*
*/
public function isOk () : bool
{
return $this->statusCode >= 200 && $this->statusCode <= 299;
}
}
2 changes: 1 addition & 1 deletion src/Api/ApiResponseNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ public function normalize (ApiResponse $apiResponse) : array
{
return \array_filter(
[
"ok" => $apiResponse->ok,
"ok" => $apiResponse->isOk(),
"data" => $apiResponse->data,
"error" => $apiResponse->error,
],
Expand Down
4 changes: 2 additions & 2 deletions tests/Api/ApiResponseNormalizerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class ApiResponseNormalizerTest extends TestCase
*/
public function testMinimal () : void
{
$apiResponse = new ApiResponse(false);
$apiResponse = new ApiResponse(400);
$normalizer = new ApiResponseNormalizer();

self::assertEquals([
Expand All @@ -27,7 +27,7 @@ public function testMinimal () : void
public function testMaximal () : void
{
$apiResponse = (new ApiResponse(
true,
200,
["o" => "hai"],
))
->withError("error message");
Expand Down
51 changes: 51 additions & 0 deletions tests/Api/ApiResponseTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php declare(strict_types=1);

namespace Tests\Torr\Rad\Api;

use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Torr\Rad\Api\ApiResponse;
use PHPUnit\Framework\TestCase;

class ApiResponseTest extends TestCase
{
use ExpectDeprecationTrait;

/**
*
*/
public function testOk () : void
{
self::assertTrue((new ApiResponse(202))->isOk());
self::assertFalse((new ApiResponse(418))->isOk());
}


/**
* @group legacy
*/
public function testLegacyBoolConstructorTrue () : void
{
$this->expectDeprecation("Since 21torr/rad 3.1.0: Passing a bool as first value to ApiResponse is deprecated. Pass the status code instead.");
new ApiResponse(true);
}

/**
* @group legacy
*/
public function testLegacyBoolConstructorFalse () : void
{
$this->expectDeprecation("Since 21torr/rad 3.1.0: Passing a bool as first value to ApiResponse is deprecated. Pass the status code instead.");
new ApiResponse(false);
}

/**
* @group legacy
*/
public function testLegacyResettingStatusCode () : void
{
$this->expectDeprecation("Since 21torr/rad 3.1.0: Calling ApiResponse::withStatusCode() is deprecated, pass the status code in the constructor instead.");

(new ApiResponse(201))
->withStatusCode(418);
}
}
8 changes: 4 additions & 4 deletions tests/Listener/ControllerResponseListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ControllerResponseListenerTest extends TestCase
*/
public function testIntegration () : void
{
$event = $this->createEvent(new ApiResponse(true));
$event = $this->createEvent(new ApiResponse(200));

$listener = new ControllerResponseListener(new ApiResponseNormalizer());
$listener->onView($event);
Expand All @@ -46,9 +46,9 @@ public function testIntegrationWithOtherResult () : void
*/
public function provideStatusCode () : iterable
{
yield [200, new ApiResponse(true)];
yield [400, new ApiResponse(false)];
yield [418, (new ApiResponse(false))->withStatusCode(418)];
yield [200, new ApiResponse(200)];
yield [400, new ApiResponse(400)];
yield [418, new ApiResponse(418)];
}

/**
Expand Down

0 comments on commit 786368c

Please sign in to comment.