jolicode / elastically

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

The indexer does not use the IndexNameMapper #168

Open Nek- opened 1 year ago

Nek- commented 1 year ago

Because the IndexNameMapper does not use the indexNameMapper, the prefix is never added to the index name. However, the IndexBuilder is using it and works with the prefix! It's highly disturbing.

https://github.com/jolicode/elastically/blob/96a52d1e99b848e97f3eb613dfa3cce0e892d64e/src/Indexer.php#L45

lyrixx commented 1 year ago

Thanks for the report. Do you want to give it a try?

BorbagUskada commented 1 year ago

What is your goto ? Using the indexNameMapper everywhere ?

Along with the issue reported by @Nek- , one consequence is that I see no way as for now to retrieve dynamically Index instance solely from a class, but maybe I am missing something.

The IndexNameMapper contains several methods to return a prefixed index name based on a class, but no way to retrieve non-prefixed index name. And since the getIndex(string $name) method of Client automatically prefix the given name, there's no way but to get double-prefixed index name, which means the getIndex has to be called with a non-dynamic non-prefixed index name (not crystal clear but I guess you get it).

I could take a look at it but I'd like to know the exact scope of it, since this seems to be a BC break.

damienalexandre commented 1 year ago

You are supposed to call Client->getIndex() to get a Index.

https://github.com/jolicode/elastically/blob/2a97fc8ee18eb5abb5b7b52e7d398fa96d1018e5/src/Client.php#L57

That means you should now the that Class FOO belong to index "yop".

If your application does not have this information, you could expose the config elsewhere:

https://github.com/jolicode/elastically/blob/2a97fc8ee18eb5abb5b7b52e7d398fa96d1018e5/tests/Symfony/config.yaml#L45-L46

Or maybe we should add a helper in the Client, like "getIndexFromObject"? Would that help?