meilisearch
Version 2: PHP 8.4, property hooks, asymetric visitibility, etc.
Description
Since Version 2 isn't out yet, I think it'd be a great time to modernize it. We could reduce the amount of boilerplate code by leverage property hooks and asymetric visibility.
Basic example
<?php
declare(strict_types=1);
namespace Meilisearch\Contracts;
class DocumentsQuery
{
/**
* @var non-negative-int|null
*/
private ?int $offset = null;
/**
* @var non-negative-int|null
*/
private ?int $limit = null;
/**
* @var non-empty-list<string>|null
*/
private ?array $fields = null;
/**
* @var list<non-empty-string|list<non-empty-string>>|null
*/
private ?array $filter = null;
private ?bool $retrieveVectors = null;
/**
* @var list<non-empty-string|int>|null
*/
private ?array $ids = null;
/**
* @var list<non-empty-string>|null
*/
private ?array $sort = null;
/**
* @param non-negative-int $offset
*
* @return $this
*/
public function setOffset(int $offset): self
{
$this->offset = $offset;
return $this;
}
/**
* @param non-negative-int $limit
*
* @return $this
*/
public function setLimit(int $limit): self
{
$this->limit = $limit;
return $this;
}
/**
* @param non-empty-list<string> $fields
*
* @return $this
*/
public function setFields(array $fields): self
{
$this->fields = $fields;
return $this;
}
/**
* Sets the filter for the DocumentsQuery.
*
* @param list<non-empty-string|list<non-empty-string>> $filter a filter expression written as an array of strings
*
* @return $this
*/
public function setFilter(array $filter): self
{
$this->filter = $filter;
return $this;
}
/**
* @param bool|null $retrieveVectors boolean value to show _vector details
*
* @return $this
*/
public function setRetrieveVectors(?bool $retrieveVectors): self
{
$this->retrieveVectors = $retrieveVectors;
return $this;
}
/**
* @param list<non-empty-string|int> $ids Array of document IDs
*
* @return $this
*/
public function setIds(array $ids): self
{
$this->ids = $ids;
return $this;
}
/**
* Checks if the $filter attribute has been set.
*
* @return bool true when filter contains at least a non-empty array
*/
public function hasFilter(): bool
{
return null !== $this->filter;
}
/**
* @param list<non-empty-string> $sort
*/
public function setSort(array $sort): self
{
$this->sort = $sort;
return $this;
}
/**
* @return array{
* offset?: non-negative-int,
* limit?: non-negative-int,
* fields?: non-empty-list<string>|non-empty-string,
* filter?: list<non-empty-string|list<non-empty-string>>,
* retrieveVectors?: bool,
* ids?: string,
* sort?: non-empty-list<string>,
* }
*/
public function toArray(): array
{
return array_filter([
'offset' => $this->offset,
'limit' => $this->limit,
'fields' => $this->fields,
'filter' => $this->filter,
'retrieveVectors' => $this->retrieveVectors,
'ids' => ($this->ids ?? []) !== [] ? implode(',', $this->ids) : null,
'sort' => $this->sort,
], static function ($item) { return null !== $item; });
}
}
NEW:
<?php
declare(strict_types=1);
namespace Meilisearch\Contracts;
class DocumentsQuery
{
/**
* @var non-negative-int|null
*/
public private(set) ?int $offset = null {
set(int $value) {
if ($value < 0) {
throw new \InvalidArgumentException('Offset must be non-negative');
}
$this->offset = $value;
}
}
/**
* @var non-negative-int|null
*/
public private(set) ?int $limit = null {
set(int $value) {
if ($value < 0) {
throw new \InvalidArgumentException('Limit must be non-negative');
}
$this->limit = $value;
}
}
/**
* @var non-empty-list<string>|null
*/
public private(set) ?array $fields = null {
set(array $value) {
if (empty($value)) {
throw new \InvalidArgumentException('Fields array cannot be empty');
}
$this->fields = $value;
}
}
/**
* @var list<non-empty-string|list<non-empty-string>>|null
*/
public private(set) ?array $filter = null;
public private(set) ?bool $retrieveVectors = null;
/**
* @var list<non-empty-string|int>|null
*/
public private(set) ?array $ids = null;
/**
* @var list<non-empty-string>|null
*/
public private(set) ?array $sort = null {
set(array $value) {
if (empty($value)) {
throw new \InvalidArgumentException('Sort array cannot be empty');
}
$this->sort = $value;
}
}
/**
* Checks if the $filter attribute has been set.
*
* @return bool true when filter contains at least a non-empty array
*/
public function hasFilter(): bool
{
return null !== $this->filter;
}
/**
* @return array{
* offset?: non-negative-int,
* limit?: non-negative-int,
* fields?: non-empty-list<string>|non-empty-string,
* filter?: list<non-empty-string|list<non-empty-string>>,
* retrieveVectors?: bool,
* ids?: string,
* sort?: non-empty-list<string>,
* }
*/
public function toArray(): array
{
return array_filter([
'offset' => $this->offset,
'limit' => $this->limit,
'fields' => $this->fields,
'filter' => $this->filter,
'retrieveVectors' => $this->retrieveVectors,
'ids' => ($this->ids ?? []) !== [] ? implode(',', $this->ids) : null,
'sort' => $this->sort,
], static function ($item) { return null !== $item; });
}
}
Usage:
$query = new DocumentsQuery();
// Direct property assignment with automatic validation
$query->offset = 10; // ✓ Valid
$query->limit = 20; // ✓ Valid
$query->fields = ['title']; // ✓ Valid
// These will throw InvalidArgumentException
$query->offset = -1; // ✗ Throws exception
$query->fields = []; // ✗ Throws exception
// Reading values directly
echo $query->offset; // 10
echo $query->hasFilter(); // false
$result = $query->toArray();
Of course, we could keep the existing setters/getters for BC and mark them as deprecated, and then really get rid of them in version 3.
But new versions are rare events, it's a natural time to leverage the new language features in PHP.
I realize that every time meilisearch releases a new endpoint or property, version 1 would need to be tweaked as well for a while. Still, I thought I'd bring it up.
Hi @tacman, and thanks for your proposal!
v2 is the ideal moment for modernizing the API. But I would prefer first having @norkunas's opinion on the matter.
In particular, this would bump the package's requirement to PHP 8.4 (v2 currently targets 8.1)
PS. After validating the scope, would you be interested in contributing?
Well currently I don't see the need change code to hooks just because it's a new language construct. In my opinion current code is perfectly fine.
Current usage:
$api->getDocuments(new DocumentsQuery()->setLimit()->setOffset());
New usage:
$query = new DocumentsQuery();
$query->offset = ..;
$query->limit = ..;
$api->getDocuments($query);
As you see, it's not possible to chain the calls anymore, so current solution is a little bit easier to use. And yeah, we now target 8.1, not the 8.4, too big jump that would be
