Merge pull request #4 from stefna/fix-404-return-types/main

Fix returns when 404 occurs

Author: sunkan

.github/workflows/continuous-integration.yml

@@ -16,38 +16,28 @@ jobs:
       fail-fast: false
       matrix:
         php-version:
-          - "7.4"
           - "8.0"
+          - "8.1"
+          - "8.2"
         experimental:
           - false
         include:
-          - php-version: "8.1"
+          - php-version: "8.3"
             composer-options: "--ignore-platform-reqs"
             experimental: true
 
     steps:
       - name: "Checkout"
-        uses: "actions/checkout@v2"
+        uses: "actions/checkout@v4"
 
       - name: "Install PHP"
         uses: "shivammathur/setup-php@v2"
         with:
           coverage: "none"
           php-version: "${{ matrix.php-version }}"
 
-      - name: Get composer cache directory
-        id: composercache
-        run: echo "::set-output name=dir::$(composer config cache-files-dir)"
-
-      - name: Cache dependencies
-        uses: actions/cache@v2
-        with:
-          path: ${{ steps.composercache.outputs.dir }}
-          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
-          restore-keys: ${{ runner.os }}-composer-
-
       - name: "Install latest dependencies"
         run: "composer update ${{ env.COMPOSER_FLAGS }} ${{ matrix.composer-options }}"
 
       - name: "Run tests"
-        run: ./vendor/bin/phpunit -c phpunit.xml.dist
+        run: ./vendor/bin/phpunit -c phpunit.xml.dist

.github/workflows/phpstan.yml

@@ -16,31 +16,20 @@ jobs:
     strategy:
       matrix:
         php-version:
-          - "7.4"
+          - "8.0"
 
     steps:
       - name: "Checkout"
-        uses: "actions/checkout@v2"
+        uses: "actions/checkout@v4"
 
       - name: "Install PHP"
         uses: "shivammathur/setup-php@v2"
         with:
           coverage: "none"
           php-version: "${{ matrix.php-version }}"
 
-      - name: Get composer cache directory
-        id: composercache
-        run: echo "::set-output name=dir::$(composer config cache-files-dir)"
-
-      - name: Cache dependencies
-        uses: actions/cache@v2
-        with:
-          path: ${{ steps.composercache.outputs.dir }}
-          key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
-          restore-keys: ${{ runner.os }}-composer-
-
       - name: "Install latest dependencies"
         run: "composer update ${{ env.COMPOSER_FLAGS }}"
 
       - name: Run PHPStan
-        run: vendor/bin/phpstan analyse
+        run: vendor/bin/phpstan analyse

composer.json

@@ -15,9 +15,9 @@
 		"php-http/curl-client": "^1.0",
 		"php-http/message": "^1.0",
 		"guzzlehttp/psr7": "^1.0",
-		"monolog/monolog": "^1.0",
+		"monolog/monolog": "^2.0",
 		"roave/security-advisories": "dev-latest",
-		"phpunit/phpunit": "^9.0",
+		"phpunit/phpunit": "^9.0 || ^10.0",
 		"phpstan/phpstan": "^1.0",
 		"stefna/codestyle": "^1.0"
 	},
@@ -37,5 +37,10 @@
 		"stan": "./vendor/bin/phpstan",
 		"codestyle": "./vendor/bin/phpcs -n --standard=vendor/stefna/codestyle/library.xml src/",
 		"codestyle-fix": "./vendor/bin/phpcbf --colors --standard=vendor/stefna/codestyle/library.xml src/"
+	},
+	"config": {
+		"allow-plugins": {
+			"php-http/discovery": false
+		}
 	}
 }

src/Api/RestApi.php

@@ -120,7 +120,7 @@ protected function doUpdate(string $id, $data, string $className): AbstractData
 		$path = $this->getPath(self::ACTION_UPDATE, [$id]);
 		$retData = $this->client->patch($path, $data);
 
-		/** @var T $className */
+		/** @var T */
 		return new $className($retData);
 	}
 

src/Client.php

@@ -13,6 +13,7 @@
 use Stefna\Mailchimp\Api\Campaigns\Campaigns as CampaignsApi;
 use Stefna\Mailchimp\Api\Lists\Lists as ListsApi;
 use Stefna\Mailchimp\Api\Templates\Templates;
+use Stefna\Mailchimp\Exceptions\NotFoundException;
 use function GuzzleHttp\Psr7\str;
 
 class Client
@@ -80,7 +81,7 @@ public function get(string $path, array $args = []): array
 			'GET',
 			$this->createUrl($path, $args),
 			$this->getDefaultHeaders()
-		));
+		)) ?? [];
 	}
 
 	/**
@@ -116,13 +117,17 @@ public function post(string $path, array $data = [])
 	 */
 	public function put(string $path, array $data = []): array
 	{
-		/** @var array<string, mixed> */
-		return $this->request($this->messageFactory->createRequest(
+		$ret = $this->request($this->messageFactory->createRequest(
 			'PUT',
 			$this->createUrl($path),
 			$this->getDefaultHeaders(),
 			(string)json_encode($data)
 		));
+		if ($ret === null) {
+			throw new NotFoundException('Put item not found: ' . $path);
+		}
+		/** @var array<string, mixed> */
+		return $ret;
 	}
 
 	/**
@@ -132,12 +137,16 @@ public function put(string $path, array $data = []): array
 	 */
 	public function patch(string $path, array $data = [])
 	{
-		return $this->request($this->messageFactory->createRequest(
+		$ret = $this->request($this->messageFactory->createRequest(
 			'PATCH',
 			$this->createUrl($path),
 			$this->getDefaultHeaders(),
 			(string)json_encode($data)
 		));
+		if ($ret === null) {
+			throw new NotFoundException('Patch item not found: ' . $path);
+		}
+		return $ret;
 	}
 
 	/**
@@ -207,22 +216,21 @@ public function response(ResponseInterface $response)
 				'status' => $status,
 			]);
 		}
+		if ($status === 404) {
+			return null;
+		}
 
 		if ($status > 299 && is_array($ret)) {
 			$errorMsg = $this->formatError($ret);
 			if ($this->logger) {
 				$this->logger->alert($errorMsg);
 			}
 
-			if ($status !== 404) {
-				$msg = 'Error from API';
-				if ($errorMsg && $errorMsg[0] !== '{') {
-					$msg .= ": $errorMsg";
-				}
-				throw new RuntimeException($msg, $status);
+			$msg = 'Error from API';
+			if ($errorMsg && $errorMsg[0] !== '{') {
+				$msg .= ": $errorMsg";
 			}
-			// todo replace with a not found exception
-			return null;
+			throw new RuntimeException($msg, $status);
 		}
 		if (!$jsonOk) {
 			return $contents;

src/Exceptions/NotFoundException.php

@@ -0,0 +1,7 @@
+<?php declare(strict_types=1);
+
+namespace Stefna\Mailchimp\Exceptions;
+
+final class NotFoundException extends \InvalidArgumentException
+{
+}

tests/Api/Campaigns/CampaignsTest.php

@@ -72,7 +72,7 @@ protected function getBadCreateParam2(): string
 		return 'badType';
 	}
 
-	protected function getBadDeleteId(): string
+	protected function getNotFoundId(): string
 	{
 		return 'nonExisting';
 	}

tests/Api/CollectionTestCase.php

@@ -46,6 +46,16 @@ public function testDeleteWorks(): void
 
 	public function testDeleteNotFound(): void
 	{
-		$this->checkBadDelete($this->getBadDeleteId());
+		$this->checkBadDelete($this->getNotFoundId());
+	}
+
+	public function testGetNotFound(): void
+	{
+		$this->checkGetNotFound($this->getNotFoundId());
+	}
+
+	public function testUpdateNotFound(): void
+	{
+		$this->checkPatchNotFound($this->getNotFoundId());
 	}
 }

tests/Api/Lists/ListsMembersTest.php

@@ -109,7 +109,7 @@ protected function getBadCreateParam2(): string
 		return 'badStatus';
 	}
 
-	protected function getBadDeleteId(): string
+	protected function getNotFoundId(): string
 	{
 		return '[email protected]';
 	}

tests/Api/Lists/ListsTest.php

@@ -104,7 +104,7 @@ protected function getBadCreateParam2(): ?string
 		return null;
 	}
 
-	protected function getBadDeleteId(): string
+	protected function getNotFoundId(): string
 	{
 		return 'nonExisting';
 	}

tests/Api/Templates/TemplatesTest.php

@@ -109,7 +109,7 @@ protected function getBadCreateParam2(): string
 		return 'someBadFolderId';
 	}
 
-	protected function getBadDeleteId(): string
+	protected function getNotFoundId(): string
 	{
 		return 'noTemplate';
 	}

tests/Api/TestCase.php

@@ -5,6 +5,7 @@
 use Stefna\Mailchimp\Api\CollectionRestApi;
 use Stefna\Mailchimp\Api\Request\AllInterface;
 use Stefna\Mailchimp\Api\RestApi;
+use Stefna\Mailchimp\Exceptions\NotFoundException;
 use Stefna\Mailchimp\Other\AbstractData;
 use Stefna\Mailchimp\Other\AbstractRequest;
 use Tests\Stefna\Mailchimp\AbstractTestCase;
@@ -57,7 +58,7 @@ abstract protected function getBadCreateParam1(): string;
 
 	abstract protected function getBadCreateParam2(): ?string;
 
-	abstract protected function getBadDeleteId(): string;
+	abstract protected function getNotFoundId(): string;
 
 	/**
 	 * @return array|AbstractData
@@ -156,6 +157,20 @@ protected function checkBadDelete($id): void
 		$this->assertFalse($ret);
 	}
 
+	protected function checkGetNotFound($id): void
+	{
+		$api = $this->getApi();
+		$ret = $api->get($id);
+		$this->assertNull($ret);
+	}
+
+	protected function checkPatchNotFound($id): void
+	{
+		$api = $this->getApi();
+		$this->expectException(NotFoundException::class);
+		$api->update($id, []);
+	}
+
 	protected function checkEntityDefault($entity): void
 	{
 

tests/ArrayIntersectAssocRecursiveTest.php

@@ -18,7 +18,7 @@ public function test($a, $b, $expected): void
 		$this->assertEquals($expected, TestedTestCase::array_intersect_assoc_recursive($a, $b));
 	}
 
-	public function provide(): array
+	public static function provide(): array
 	{
 		return [
 			'php.net' => [

tests/Mocks/responses/campaigns/nonExisting/get.php

@@ -0,0 +1,8 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);
+

tests/Mocks/responses/campaigns/nonExisting/patch.php

@@ -0,0 +1,7 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);

tests/Mocks/responses/lists/215f4cfac8/members/760b86a041c36c2b63b69709810bb35a/get.php

@@ -0,0 +1,7 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);

tests/Mocks/responses/lists/215f4cfac8/members/760b86a041c36c2b63b69709810bb35a/patch.php

@@ -0,0 +1,7 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);

tests/Mocks/responses/lists/nonExisting/get.php

@@ -0,0 +1,8 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);
+

tests/Mocks/responses/lists/nonExisting/patch.php

@@ -0,0 +1,7 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);

tests/Mocks/responses/templates/noTemplate/get.php

@@ -0,0 +1,8 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);
+

tests/Mocks/responses/templates/noTemplate/patch.php

@@ -0,0 +1,7 @@
+<?php
+
+return new \GuzzleHttp\Psr7\Response(
+	404,
+	['IsMock' => true],
+	''
+);