Replace Guzzle with Symfony HTTP client

jmleroux · jmleroux · commit a3d6de211a89 · 2023-02-17T18:20:35.000+01:00
diff --git a/Makefile b/Makefile
@@ -1,4 +1,4 @@
-DOCKER_RUN = docker-compose run --rm php
+DOCKER_RUN = docker compose run --rm php
 
 .env.local: .env
 	cp -n .env .env.local
diff --git a/src/Api/Job/JobTestMetadata.php b/src/Api/Job/JobTestMetadata.php
@@ -19,13 +19,9 @@ class JobTestMetadata
 {
     use ValidateClientVersionTrait;
 
-    /** @var Client */
-    private $client;
-
-    public function __construct(Client $client)
+    public function __construct(private readonly Client $client)
     {
-        $this->validateClientVersion($client, ['v2']);
-        $this->client = $client;
+        $this->validateClientVersion($this->client, ['v2']);
     }
 
     /**
diff --git a/src/Api/Pipeline/AllPipelines.php b/src/Api/Pipeline/AllPipelines.php
@@ -12,25 +12,21 @@
  * Retrieve pipelines of a project, optionally filtered by branch.
  *
  * @author jmleroux <jmleroux.pro@gmail.com>
- * @link https://circleci.com/docs/api/v2/#operation/listPipelinesForProject
+ * @link   https://circleci.com/docs/api/v2/#operation/listPipelinesForProject
  */
 class AllPipelines
 {
     use ValidateClientVersionTrait;
 
-    /** @var Client */
-    private $client;
-
-    public function __construct(Client $client)
+    public function __construct(private readonly Client $client)
     {
         $this->validateClientVersion($client, ['v2']);
-        $this->client = $client;
     }
 
     /**
      * @return Pipeline[]
      */
-    public function execute(string $projectSlug, ?int $maxPipelineCount, ?string $branch): array
+    public function execute(string $projectSlug, ?int $maxPipelineCount = null, ?string $branch = null): array
     {
         $pipelines = [];
 
@@ -52,7 +48,7 @@ public function execute(string $projectSlug, ?int $maxPipelineCount, ?string $br
                 unset($params['branch']);
             }
 
-            $response = json_decode((string) $this->client->get($uri, $params)->getContent());
+            $response = json_decode((string)$this->client->get($uri, $params)->getContent());
             $nextPageToken = $response->next_page_token;
 
             foreach ($response->items as $item) {
diff --git a/src/Api/Workflow/CancelWorkflow.php b/src/Api/Workflow/CancelWorkflow.php
@@ -17,13 +17,9 @@ class CancelWorkflow
 {
     use ValidateClientVersionTrait;
 
-    /** @var Client */
-    private $client;
-
-    public function __construct(Client $client)
+    public function __construct(private Client $client)
     {
         $this->validateClientVersion($client, ['v2']);
-        $this->client = $client;
     }
 
     public function execute(string $workflowId): void
diff --git a/src/Client.php b/src/Client.php
@@ -11,21 +11,16 @@
 class Client
 {
     protected HttpClientInterface $client;
-    protected string $token;
 
-    public function __construct(string $token, ?string $version = 'v1.1')
+    public function __construct(private readonly string $token, private readonly ?string $version = 'v1.1')
     {
         $baseUri = sprintf('https://circleci.com/api/%s/', $version);
         $this->client = HttpClient::create(['base_uri' => $baseUri]);
-        $this->token = $token;
     }
 
     public function getVersion(): string
     {
-        $baseUri = 'foo';
-        preg_match('#circleci.com/api/(v[\d\.]+)/#', $baseUri, $result);
-
-        return $result[1];
+        return $this->version;
     }
 
     public function get(string $url, array $params = []): ResponseInterface
diff --git a/src/ValidateClientVersionTrait.php b/src/ValidateClientVersionTrait.php
@@ -12,7 +12,6 @@
 trait ValidateClientVersionTrait
 {
     /**
-     * @param Client $client
      * @param string[]  $supportedVersions
      */
     private function validateClientVersion(Client $client, array $supportedVersions): void
diff --git a/tests/Integration/Api/Pipeline/AllPipelinesTest.php b/tests/Integration/Api/Pipeline/AllPipelinesTest.php
@@ -21,13 +21,9 @@ class AllPipelinesTest extends TestCase
 
     private Client $client;
 
-    public static function setUpBeforeClass(): void
-    {
-        MockServer::startServer();
-    }
-
     public function setUp(): void
     {
+        MockServer::startServer();
         $personaltoken = $_ENV['CIRCLECI_PERSONNAL_TOKEN'];
         $this->client = new TestClient(MockServer::getServerRoot(), $personaltoken, 'v2');
     }
@@ -69,7 +65,7 @@ public function testQueryWithBranch()
         $this->assertIsArray($pipelines);
         foreach ($pipelines as $pipeline) {
             $this->assertInstanceOf(Pipeline::class, $pipeline);
-            $this->assertEquals('master', $pipeline->vcs()->branch());
+            $this->assertEquals('feature/design-new-api', $pipeline->vcs()->branch());
         }
     }
 }
diff --git a/tests/Integration/Api/Project/BuildSummaryForBranchTest.php b/tests/Integration/Api/Project/BuildSummaryForBranchTest.php
@@ -31,9 +31,6 @@ public function testQueryOk()
         $this->assertIsInt($firstBuild->buildNum());
         $this->assertIsString($firstBuild->branch());
         $this->assertIsString($firstBuild->vcsRevision());
-        $this->assertIsString($firstBuild->committerName());
-        $this->assertIsString($firstBuild->committerEmail());
-        $this->assertIsString($firstBuild->body());
         $this->assertIsString($firstBuild->why());
         $this->assertNull($firstBuild->dontBuild());
         if (null !== $firstBuild->queuedAt()) {
@@ -60,13 +57,13 @@ public function testMaxResults()
         $personalToken = $_ENV['CIRCLECI_PERSONNAL_TOKEN'];
         $client = new Client($personalToken, 'v1.1');
         $query = new BuildSummaryForBranch($client);
-        $builds = $this->executeWithRetry($query, ['github', 'jmleroux', 'circleci-php-client', 'master', [], 8]);
-        $this->assertCount(8, $builds);
+        $builds = $this->executeWithRetry($query, ['github', 'jmleroux', 'circleci-php-client', 'master', [], 2]);
+        $this->assertCount(2, $builds);
 
-        $builds = $query->execute('github', 'jmleroux', 'circleci-php-client', 'master', [], 50);
-        $this->assertCount(50, $builds);
+        $builds = $query->execute('github', 'jmleroux', 'circleci-php-client', 'master', [], 8);
+        $this->assertCount(8, $builds);
 
-        $builds = $query->execute('github', 'jmleroux', 'circleci-php-client', 'master', ['limit' => 6], 22);
-        $this->assertCount(22, $builds);
+        $builds = $query->execute('github', 'jmleroux', 'circleci-php-client', 'master', ['limit' => 6], 8);
+        $this->assertCount(8, $builds);
     }
 }
diff --git a/tests/Integration/Api/Project/ProjectSummaryMetricsTest.php b/tests/Integration/Api/Project/ProjectSummaryMetricsTest.php
@@ -20,8 +20,7 @@ class ProjectSummaryMetricsTest extends TestCase
 {
     use ExecuteWithRetryTrait;
 
-    /** @var Client */
-    private $client;
+    private Client $client;
 
     public function setUp(): void
     {
diff --git a/tests/Integration/Api/Workflow/CancelWorkflowTest.php b/tests/Integration/Api/Workflow/CancelWorkflowTest.php
@@ -4,10 +4,10 @@
 
 namespace Jmleroux\CircleCi\Tests\Integration\Api\Workflow;
 
-use GuzzleHttp\Exception\RequestException;
 use Jmleroux\CircleCi\Api\Workflow\CancelWorkflow;
 use Jmleroux\CircleCi\Client;
 use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpClient\Exception\ClientException;
 
 class CancelWorkflowTest extends TestCase
 {
@@ -22,9 +22,8 @@ public function setUp(): void
 
     public function testQueryKo()
     {
-        $this->expectException(RequestException::class);
+        $this->expectException(ClientException::class);
         $this->expectExceptionCode(404);
-        $this->expectExceptionMessage('{"message":"Workflow not found"}');
 
         $query = new CancelWorkflow($this->client);
         $query->execute('foobar');
diff --git a/tests/Integration/Api/Workflow/WorkflowJobsTest.php b/tests/Integration/Api/Workflow/WorkflowJobsTest.php
@@ -4,6 +4,8 @@
 
 namespace Jmleroux\CircleCi\Tests\Integration\Api\Workflow;
 
+use Jmleroux\CircleCi\Api\Pipeline\AllPipelines;
+use Jmleroux\CircleCi\Api\Pipeline\PipelineWorkflows;
 use Jmleroux\CircleCi\Api\Workflow\WorkflowJobs;
 use Jmleroux\CircleCi\Client;
 use Jmleroux\CircleCi\Model\Job;
@@ -15,8 +17,7 @@ class WorkflowJobsTest extends TestCase
 {
     use ExecuteWithRetryTrait;
 
-    /** @var Client */
-    private $client;
+    private Client $client;
 
     public function setUp(): void
     {
@@ -26,8 +27,12 @@ public function setUp(): void
 
     public function testQuery()
     {
+        $query = new AllPipelines($this->client);
+        $pipelines = $query->execute('gh/jmleroux/circleci-php-client', 1);
+        $query = new PipelineWorkflows($this->client);
+        $workflows = $query->execute($pipelines[0]->id(), 1);
         $query = new WorkflowJobs($this->client);
-        $jobs = $this->executeWithRetry($query, ['78176b2f-c1e3-4c18-86e3-5a80a2f109bb']);
+        $jobs = $this->executeWithRetry($query, [$workflows[0]->id()]);
         Assert::assertIsArray($jobs);
 
         $firstJob = $jobs[0];
diff --git a/tests/Integration/ExecuteWithRetryTrait.php b/tests/Integration/ExecuteWithRetryTrait.php
@@ -4,7 +4,7 @@
 
 namespace Jmleroux\CircleCi\Tests\Integration;
 
-use GuzzleHttp\Exception\ClientException;
+use Symfony\Component\HttpClient\Exception\ClientException;
 
 /**
  * @author  JM Leroux <jmleroux.pro@gmail.com>
diff --git a/tests/Integration/TestClient.php b/tests/Integration/TestClient.php
@@ -17,8 +17,8 @@ public function __construct(
         string $token,
         ?string $version = 'v1.1'
     ) {
+        parent::__construct($token, $version);
         $baseUri = sprintf('%s/api/%s/', $mockServerBaseUrl, $version);
         $this->client = HttpClient::create(['base_uri' => $baseUri]);
-        $this->token = $token;
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-DOCKER_RUN = docker-compose run --rm php`
	`1`	`+DOCKER_RUN = docker compose run --rm php`
`2`	`2`
`3`	`3`	`.env.local: .env`
`4`	`4`	`cp -n .env .env.local`
Original file line number	Diff line number	Diff line change
`@@ -19,13 +19,9 @@ class JobTestMetadata`
`19`	`19`	`{`
`20`	`20`	`use ValidateClientVersionTrait;`
`21`	`21`
`22`		`- /** @var Client */`
`23`		`- private $client;`
`24`		`-`
`25`		`- public function __construct(Client $client)`
	`22`	`+ public function __construct(private readonly Client $client)`
`26`	`23`	`{`
`27`		`- $this->validateClientVersion($client, ['v2']);`
`28`		`- $this->client = $client;`
	`24`	`+ $this->validateClientVersion($this->client, ['v2']);`
`29`	`25`	`}`
`30`	`26`
`31`	`27`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -12,25 +12,21 @@`
`12`	`12`	`* Retrieve pipelines of a project, optionally filtered by branch.`
`13`	`13`	`*`
`14`	`14`	`* @author jmleroux <jmleroux.pro@gmail.com>`
`15`		`- * @link https://circleci.com/docs/api/v2/#operation/listPipelinesForProject`
	`15`	`+ * @link https://circleci.com/docs/api/v2/#operation/listPipelinesForProject`
`16`	`16`	`*/`
`17`	`17`	`class AllPipelines`
`18`	`18`	`{`
`19`	`19`	`use ValidateClientVersionTrait;`
`20`	`20`
`21`		`- /** @var Client */`
`22`		`- private $client;`
`23`		`-`
`24`		`- public function __construct(Client $client)`
	`21`	`+ public function __construct(private readonly Client $client)`
`25`	`22`	`{`
`26`	`23`	`$this->validateClientVersion($client, ['v2']);`
`27`		`- $this->client = $client;`
`28`	`24`	`}`
`29`	`25`
`30`	`26`	`/**`
`31`	`27`	`* @return Pipeline[]`
`32`	`28`	`*/`
`33`		`- public function execute(string $projectSlug, ?int $maxPipelineCount, ?string $branch): array`
	`29`	`+ public function execute(string $projectSlug, ?int $maxPipelineCount = null, ?string $branch = null): array`
`34`	`30`	`{`
`35`	`31`	`$pipelines = [];`
`36`	`32`
`@@ -52,7 +48,7 @@ public function execute(string $projectSlug, ?int $maxPipelineCount, ?string $br`
`52`	`48`	`unset($params['branch']);`
`53`	`49`	`}`
`54`	`50`
`55`		`- $response = json_decode((string) $this->client->get($uri, $params)->getContent());`
	`51`	`+ $response = json_decode((string)$this->client->get($uri, $params)->getContent());`
`56`	`52`	`$nextPageToken = $response->next_page_token;`
`57`	`53`
`58`	`54`	`foreach ($response->items as $item) {`
Original file line number	Diff line number	Diff line change
`@@ -17,13 +17,9 @@ class CancelWorkflow`
`17`	`17`	`{`
`18`	`18`	`use ValidateClientVersionTrait;`
`19`	`19`
`20`		`- /** @var Client */`
`21`		`- private $client;`
`22`		`-`
`23`		`- public function __construct(Client $client)`
	`20`	`+ public function __construct(private Client $client)`
`24`	`21`	`{`
`25`	`22`	`$this->validateClientVersion($client, ['v2']);`
`26`		`- $this->client = $client;`
`27`	`23`	`}`
`28`	`24`
`29`	`25`	`public function execute(string $workflowId): void`
Original file line number	Diff line number	Diff line change
`@@ -11,21 +11,16 @@`
`11`	`11`	`class Client`
`12`	`12`	`{`
`13`	`13`	`protected HttpClientInterface $client;`
`14`		`- protected string $token;`
`15`	`14`
`16`		`- public function __construct(string $token, ?string $version = 'v1.1')`
	`15`	`+ public function __construct(private readonly string $token, private readonly ?string $version = 'v1.1')`
`17`	`16`	`{`
`18`	`17`	`$baseUri = sprintf('https://circleci.com/api/%s/', $version);`
`19`	`18`	`$this->client = HttpClient::create(['base_uri' => $baseUri]);`
`20`		`- $this->token = $token;`
`21`	`19`	`}`
`22`	`20`
`23`	`21`	`public function getVersion(): string`
`24`	`22`	`{`
`25`		`- $baseUri = 'foo';`
`26`		`- preg_match('#circleci.com/api/(v[\d\.]+)/#', $baseUri, $result);`
`27`		`-`
`28`		`- return $result[1];`
	`23`	`+ return $this->version;`
`29`	`24`	`}`
`30`	`25`
`31`	`26`	`public function get(string $url, array $params = []): ResponseInterface`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,6 @@`
`12`	`12`	`trait ValidateClientVersionTrait`
`13`	`13`	`{`
`14`	`14`	`/**`
`15`		`- * @param Client $client`
`16`	`15`	`* @param string[] $supportedVersions`
`17`	`16`	`*/`
`18`	`17`	`private function validateClientVersion(Client $client, array $supportedVersions): void`
Original file line number	Diff line number	Diff line change
`@@ -21,13 +21,9 @@ class AllPipelinesTest extends TestCase`
`21`	`21`
`22`	`22`	`private Client $client;`
`23`	`23`
`24`		`- public static function setUpBeforeClass(): void`
`25`		`- {`
`26`		`- MockServer::startServer();`
`27`		`- }`
`28`		`-`
`29`	`24`	`public function setUp(): void`
`30`	`25`	`{`
	`26`	`+ MockServer::startServer();`
`31`	`27`	`$personaltoken = $_ENV['CIRCLECI_PERSONNAL_TOKEN'];`
`32`	`28`	`$this->client = new TestClient(MockServer::getServerRoot(), $personaltoken, 'v2');`
`33`	`29`	`}`
`@@ -69,7 +65,7 @@ public function testQueryWithBranch()`
`69`	`65`	`$this->assertIsArray($pipelines);`
`70`	`66`	`foreach ($pipelines as $pipeline) {`
`71`	`67`	`$this->assertInstanceOf(Pipeline::class, $pipeline);`
`72`		`- $this->assertEquals('master', $pipeline->vcs()->branch());`
	`68`	`+ $this->assertEquals('feature/design-new-api', $pipeline->vcs()->branch());`
`73`	`69`	`}`
`74`	`70`	`}`
`75`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,8 +20,7 @@ class ProjectSummaryMetricsTest extends TestCase`
`20`	`20`	`{`
`21`	`21`	`use ExecuteWithRetryTrait;`
`22`	`22`
`23`		`- /** @var Client */`
`24`		`- private $client;`
	`23`	`+ private Client $client;`
`25`	`24`
`26`	`25`	`public function setUp(): void`
`27`	`26`	`{`
Original file line number	Diff line number	Diff line change
`@@ -17,8 +17,8 @@ public function __construct(`
`17`	`17`	`string $token,`
`18`	`18`	`?string $version = 'v1.1'`
`19`	`19`	`) {`
	`20`	`+ parent::__construct($token, $version);`
`20`	`21`	`$baseUri = sprintf('%s/api/%s/', $mockServerBaseUrl, $version);`
`21`	`22`	`$this->client = HttpClient::create(['base_uri' => $baseUri]);`
`22`		`- $this->token = $token;`
`23`	`23`	`}`
`24`	`24`	`}`