Elastica\Document constructor expects array|string, object given

[darthf1]

Hi!

According to the docs, to add a document to the index, you have to to the following:

// DTO is a PHP object
$dto = new \StdClass();

// Add a document to the queue
$indexer->scheduleIndex('beers', new Document('123', $dto));
$indexer->flush();

In the indexer, if the $data inside the Document (the $dto parameter) is an object, it is sent to the serializer:

if (\is_object($document->getData())) {
    $context = $this->client->getSerializerContext(\get_class($document->getData()));
    $document->setData($this->serializer->serialize($document->getData(), 'json', $context));
}

However, the constructor of Elastica\Document is typehinted as array|string:

/**
 * Creates a new document.
 *
 * @param string|null  $id    The document ID, if null it will be created
 * @param array|string $data  Data array
 * @param Index|string $index Index name
 */
public function __construct(?string $id = null, $data = [], $index = '')

And getData returns an array|string as well:

    /**
     * Returns the document data.
     *
     * @return array|string Document data
     */
    public function getData()

At the moment, this results in statistical analysis errors. In the future, this could lead to PHP errors when these PHPDoc's are converted to real typed properties.

[lyrixx]

👍🏼 There a many errors like this. Since elastically is a drop-in replacement of elastica but adds new features, it's hard to avoid such kind of errors.

I already fixed some of them, but the way the library is built, IMHO is not the best one. I would like to propose another approach (don't know it yet :p) but I don't have time ATM to do it.

So I think the quick win, for now, would be to extends some classes, and use them instead of elastica ones... If you wan to submit a PR, It would be awesome 💛

[laryjulien]

@lyrixx I have provided a fix for this issue, more details on PR.

[damienalexandre]

The discussion happened here https://github.com/jolicode/elastically/pull/105#issuecomment-1117011122 and I can say we are not going to fix it.

[lyrixx]

Hello, This has been solved in #150 🎉