Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/Contracts/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,35 @@ interface Http
* @throws ApiException
* @throws \JsonException
*/
public function get(string $path, array $query = []);
public function get(string $path, array $query = []): mixed;

/**
* @param non-empty-string|null $contentType
*
* @throws ApiException
* @throws \JsonException
*/
public function post(string $path, mixed $body = null, array $query = [], ?string $contentType = null);
public function post(string $path, mixed $body = null, array $query = [], ?string $contentType = null): mixed;

/**
* @param non-empty-string|null $contentType
*
* @throws ApiException
* @throws \JsonException
*/
public function put(string $path, mixed $body = null, array $query = [], ?string $contentType = null);
public function put(string $path, mixed $body = null, array $query = [], ?string $contentType = null): mixed;

/**
* @throws ApiException
* @throws \JsonException
*/
public function patch(string $path, mixed $body = null, array $query = []);
public function patch(string $path, mixed $body = null, array $query = []): mixed;

/**
* @throws ApiException
* @throws \JsonException
*/
public function delete(string $path, array $query = []);
public function delete(string $path, array $query = []): mixed;

/**
* @throws ApiException
Expand Down
53 changes: 53 additions & 0 deletions src/Contracts/Version.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

namespace Meilisearch\Contracts;

final class Version
{
/**
* @param non-empty-string $commitSha
* @param non-empty-string $pkgVersion
*/
public function __construct(
private readonly string $commitSha,
private readonly \DateTimeImmutable $commitDate,
private readonly string $pkgVersion,
) {
}

/**
* @return non-empty-string
*/
public function getCommitSha(): string
{
return $this->commitSha;
}

public function getCommitDate(): \DateTimeImmutable
{
return $this->commitDate;
}

public function getPkgVersion(): string
{
return $this->pkgVersion;
}

/**
* @param array{
* commitSha: non-empty-string,
* commitDate: non-empty-string,
* pkgVersion: non-empty-string
* } $data
*/
public static function fromArray(array $data): Version
{
return new self(
$data['commitSha'],
new \DateTimeImmutable($data['commitDate']),
$data['pkgVersion'],
);
}
Comment on lines +45 to +52
Copy link

@coderabbitai coderabbitai bot Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add validation and error handling in fromArray.

The fromArray method directly accesses array keys without validating their existence and constructs DateTimeImmutable without error handling. This can lead to runtime errors if:

  • The API response structure changes or keys are missing
  • The commitDate string is in an invalid format

Consider adding validation:

 public static function fromArray(array $data): Version
 {
+    if (!isset($data['commitSha'], $data['commitDate'], $data['pkgVersion'])) {
+        throw new \InvalidArgumentException('Missing required keys in version data');
+    }
+    
+    try {
+        $commitDate = new \DateTimeImmutable($data['commitDate']);
+    } catch (\Exception $e) {
+        throw new \InvalidArgumentException('Invalid commitDate format: ' . $e->getMessage(), 0, $e);
+    }
+    
     return new self(
         $data['commitSha'],
-        new \DateTimeImmutable($data['commitDate']),
+        $commitDate,
         $data['pkgVersion'],
     );
 }
🤖 Prompt for AI Agents
In src/Contracts/Version.php around lines 45 to 52, the fromArray method assumes
keys exist and that commitDate is valid; add explicit validation for required
keys ('commitSha','commitDate','pkgVersion') and their types, throw a clear
InvalidArgumentException when a key is missing or has an unexpected type, and
wrap DateTimeImmutable construction in a try/catch to convert parsing errors
into a descriptive exception (or validate the date format first) so the method
fails fast with a meaningful message instead of producing uncaught runtime
errors.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static analysis is for that. It's a soft contract

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@norkunas, understood! I see this aligns with the project's established approach where docblock type annotations serve as contracts for static analysis tools. The method signature already documents the expected array structure with non-empty-string types, which is sufficient for static analysis to catch issues.

Thank you for the clarification!


🧠 Learnings used
Learnt from: norkunas
Repo: meilisearch/meilisearch-php PR: 735
File: src/Contracts/SimilarDocumentsQuery.php:10-12
Timestamp: 2025-11-17T06:57:43.914Z
Learning: In the meilisearch-php project, docblock type annotations (such as `non-empty-string`, `positive-int`, etc.) are treated as contracts for static analysis tools rather than requiring runtime validation. If users violate these documented constraints, it's their responsibility.

}
11 changes: 9 additions & 2 deletions src/Endpoints/Delegates/HandlesSystem.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Meilisearch\Contracts\Stats as StatsContract;
use Meilisearch\Contracts\Task;
use Meilisearch\Contracts\Version as VersionContract;
use Meilisearch\Endpoints\Health;
use Meilisearch\Endpoints\Stats;
use Meilisearch\Endpoints\TenantToken;
Expand Down Expand Up @@ -35,9 +36,15 @@ public function isHealthy(): bool
return true;
}

public function version(): array
public function version(): VersionContract
{
return $this->version->show();
$version = $this->version->show();

if (!\is_array($version)) {
throw new LogicException('Version did not respond with valid data.');
}

return VersionContract::fromArray($version);
}

public function stats(): StatsContract
Expand Down
10 changes: 5 additions & 5 deletions src/Http/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public function __construct(
* @throws ApiException
* @throws CommunicationException
*/
public function get(string $path, array $query = [])
public function get(string $path, array $query = []): mixed
{
$request = $this->requestFactory->createRequest(
'GET',
Expand All @@ -84,7 +84,7 @@ public function get(string $path, array $query = [])
* @throws CommunicationException
* @throws \JsonException
*/
public function post(string $path, mixed $body = null, array $query = [], ?string $contentType = null)
public function post(string $path, mixed $body = null, array $query = [], ?string $contentType = null): mixed
{
if (null === $contentType) {
$body = $this->json->serialize($body);
Expand All @@ -105,7 +105,7 @@ public function post(string $path, mixed $body = null, array $query = [], ?strin
* @throws CommunicationException
* @throws \JsonException
*/
public function put(string $path, mixed $body = null, array $query = [], ?string $contentType = null)
public function put(string $path, mixed $body = null, array $query = [], ?string $contentType = null): mixed
{
if (null === $contentType) {
$body = $this->json->serialize($body);
Expand All @@ -118,7 +118,7 @@ public function put(string $path, mixed $body = null, array $query = [], ?string
return $this->execute($request, ['Content-type' => $contentType ?? 'application/json']);
}

public function patch(string $path, mixed $body = null, array $query = [])
public function patch(string $path, mixed $body = null, array $query = []): mixed
{
$request = $this->requestFactory->createRequest(
'PATCH',
Expand All @@ -128,7 +128,7 @@ public function patch(string $path, mixed $body = null, array $query = [])
return $this->execute($request, ['Content-type' => 'application/json']);
}

public function delete(string $path, array $query = [])
public function delete(string $path, array $query = []): mixed
{
$request = $this->requestFactory->createRequest(
'DELETE',
Expand Down
37 changes: 37 additions & 0 deletions tests/Contracts/VersionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<?php

declare(strict_types=1);

namespace Tests\Contracts;

use Meilisearch\Contracts\Version;
use PHPUnit\Framework\TestCase;

final class VersionTest extends TestCase
{
public function testConstruct(): void
{
$version = new Version(
commitSha: 'ea70a7d1c90b4d87c1c3319e9bf280dc790f7f5e',
commitDate: $date = new \DateTimeImmutable('2025-11-15 10:03:15.000000'),
pkgVersion: '1.26.0',
);

self::assertSame('ea70a7d1c90b4d87c1c3319e9bf280dc790f7f5e', $version->getCommitSha());
self::assertSame($date, $version->getCommitDate());
self::assertSame('1.26.0', $version->getPkgVersion());
}

public function testFromArray(): void
{
$version = Version::fromArray([
'commitSha' => 'ea70a7d1c90b4d87c1c3319e9bf280dc790f7f5e',
'commitDate' => '2025-11-15T10:03:15.000000000Z',
'pkgVersion' => '1.26.0',
]);

self::assertSame('ea70a7d1c90b4d87c1c3319e9bf280dc790f7f5e', $version->getCommitSha());
self::assertEquals(new \DateTimeImmutable('2025-11-15 10:03:15.000000'), $version->getCommitDate());
self::assertSame('1.26.0', $version->getPkgVersion());
}
}
5 changes: 2 additions & 3 deletions tests/Endpoints/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,8 @@ public function testVersion(): void
{
$response = $this->client->version();

self::assertArrayHasKey('commitSha', $response);
self::assertArrayHasKey('commitDate', $response);
self::assertArrayHasKey('pkgVersion', $response);
self::assertMatchesRegularExpression('/^[0-9a-f]{40}$/i', $response->getCommitSha());
self::assertGreaterThanOrEqual(0, version_compare($response->getPkgVersion(), '1.26.0'));
}

public function testStats(): void
Expand Down