jolicode / elastically

🔍 JoliCode's Elastica wrapper to bootstrap Elasticsearch PHP integrations
MIT License
248 stars 37 forks source link

Index prefixed twice #183

Closed yann-eugone closed 7 months ago

yann-eugone commented 7 months ago

Hi,

I'm in a command that is refreshing all indexes, in a foreach loop. And whenever I activate the prefix config, I get this error:

[ERROR] no such index [dev_dev_attribute] [index: dev_dev_attribute]

The index name is attribute and the prefix is %kernel.environment% (dev here). The prefix is applied twice.

Here is how I'm refreshing my indexes:

$index = $indexBuilder->createIndex($indexName);

// read from source, write documents

$indexBuilder->speedUpRefresh($index);
$indexBuilder->markAsLive($index, $indexName);
$indexBuilder->purgeOldIndices($indexName);

My best guess is that the error does not happen for each index. I said I was refreshing all my indexes, but it is not failing on the first one. It is failing on a index for which I need data from another index to build.

Basically, I get this output

[OK] attribute
     80 records saved.
[ERROR] entity
     no such index [dev_dev_attribute] [index: dev_dev_attribute]

I made the following patch to the code, but I'm not sure this is the way:

diff --git a/src/IndexNameMapper.php b/src/IndexNameMapper.php
index 7d151da..732a880 100644
--- a/src/IndexNameMapper.php
+++ b/src/IndexNameMapper.php
@@ -27,7 +27,7 @@ class IndexNameMapper

     public function getPrefixedIndex(string $name): string
     {
-        if ($this->prefix) {
+        if ($this->prefix && !str_starts_with($name, $this->prefix . '_')) {
             return sprintf('%s_%s', $this->prefix, $name);
         }
yann-eugone commented 7 months ago

Unsure this is connected, but I've seen https://github.com/jolicode/elastically/issues/48 that is talking about prefix applied multiple times, but it seems to be on another context

damienalexandre commented 7 months ago

Can you get the complete stacktrace of the error?

Your snippet seems fine to me. Are we sure your $indexName variable is "attribute"?

On "speedUpRefresh" we should not get any error.

On "markAsLive", there is a chance:

https://github.com/jolicode/elastically/blob/e016fd648d897c1c09e8714d5f4fd96559efacf8/src/IndexBuilder.php#L58-L68

And on "purgeOldIndices" there is also a chance.

yann-eugone commented 7 months ago

Here is the useful part of the stack trace

[ERROR] no such index [dev_dev_attribute] [index: dev_dev_attribute]

    Elastica\Exception\ResponseException: no such index [dev_dev_attribute] [index: dev_dev_attribute] (at
    /src/vendor/jolicode/elastically/src/Transport/HttpClientTransport.php(116))
    #0 /src/vendor/ruflin/elastica/src/Request.php(183): JoliCode\Elastically\Transport\HttpClientTransport->exec()
    #1 /src/vendor/ruflin/elastica/src/Client.php(545): Elastica\Request->send()
    #2 /src/vendor/ruflin/elastica/src/Client.php(584): Elastica\Client->request()
    #3 /src/vendor/ruflin/elastica/src/Index.php(774): Elastica\Client->requestEndpoint()
    #4 /src/vendor/ruflin/elastica/src/Index.php(299): Elastica\Index->requestEndpoint()
    #5 /src/vendor/jolicode/elastically/src/Index.php(37): Elastica\Index->getDocument()
    #6 /src/src/Elasticsearch/ElasticsearchClient.php(80): JoliCode\Elastically\Index->getModel()
    lot more, but irrelevant

The error occurs in the commented part of the snippet. At the time the error occurs, the script is not building the attribute index, it is building the entity index. But there is some kind of dependency between the two indexes : I need to know what are the attributes, in order to build the entity.

damienalexandre commented 7 months ago

The error happen in a getDocument() request, so it's not in the snippet you provided.

Could you share the missing part?

yann-eugone commented 7 months ago

There is a lot of classes involved in that part, but ultimately, it ends in that class

namespace App\Elasticsearch;

use Elastica\Exception\NotFoundException;
use JoliCode\Elastically\Client;
use JoliCode\Elastically\Index;
use JoliCode\Elastically\IndexNameMapper;
use Psr\Container\ContainerInterface;
use Symfony\Contracts\Service\ServiceSubscriberInterface;

final class ElasticsearchClient implements ServiceSubscriberInterface
{
    public function __construct(
        private readonly ContainerInterface $container,
    ) {
    }

    public static function getSubscribedServices(): array
    {
        return [
            Client::class => Client::class,
            IndexNameMapper::class => IndexNameMapper::class,
        ];
    }

    /**
     * @param class-string $class
     */
    public function index(string $class): Index
    {
        /** @var Client $client */
        $client = $this->container->get(Client::class);
        /** @var IndexNameMapper $indexNameMapper */
        $indexNameMapper = $this->container->get(IndexNameMapper::class);

        return $client->getIndex($indexNameMapper->getIndexNameFromClass($class));
    }

    /**
     * @template T of object
     *
     * @param class-string<T> $class
     *
     * @return T
     *
     * @throws NotFoundException
     */
    public function model(string $class, string $id): object
    {
        $model = $this->index($class)->getModel($id);
        \assert($model instanceof $class);

        return $model;
    }
}

Calling ElasticsearchClient::model(Attribute::class, 'attribute_code') is the beginning of the stack trace.

damienalexandre commented 7 months ago

Ok so it's kind of normal.

The error you get is because "getIndexNameForClass" return the full index name, prefix included.

https://github.com/jolicode/elastically/blob/e016fd648d897c1c09e8714d5f4fd96559efacf8/tests/IndexNameMapperTest.php#L43-L51

So when running $client->getIndex($indexNameMapper->getIndexNameFromClass($class)); you get a double prefix:

https://github.com/jolicode/elastically/blob/e016fd648d897c1c09e8714d5f4fd96559efacf8/src/Client.php#L52-L62

I think you will need something like this:


$fullIndexName = $indexNameMapper->getIndexNameFromClass($class);
$pureIndexName = $indexNameMapper->getPureIndexName($fullIndexName);

return $client->getIndex($pureIndexName);

Sorry for the inconvenience, this prefix feature doesn't have the best developer experience yet.

yann-eugone commented 7 months ago

Don't be sorry, I'm the one being sorry here, I missed the thing totally Anyway, thank you very much for your help (yeah, your snippet works), and the package