Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] dynamic index_class_mapping ? #173

Open
dgoosens opened this issue Aug 25, 2023 · 5 comments
Open

[Feature] dynamic index_class_mapping ? #173

dgoosens opened this issue Aug 25, 2023 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@dgoosens
Copy link

hello,

so I need to build a lot of indexes...
the naming of these indexes is dynamic, but I do use a predefined prefix av_%%indexname%%
all the indexes do use the same mapping and map to the same class

as the naming is dynamic, I can not list all of them in the configuration...
at runtime, the request tells me on which index I should use.

requests work fine...
BUT when building the resultSet at the end of \Elastica\Search::search, using the injected BuilderInterface the request fails because the class mapping is unknown

so before looking into injecting a custom BuilderInterface, I was wondering if there were ways to either use wildcards for the mapping or inject a mapping at runtime ?

thanks a lot
Dmitri

@dgoosens
Copy link
Author

hello

So I managed to handle this with a custom ResultSetBuilder that uses a custom IndexNameMapper
Creating these was rather straightforward

In my case, all I had to do was overwriting IndexNameMapper::getClassFromIndexName() to return the class I wanted
(could have gone through the trouble of matching the prefix and/or a dynamic class map)

then, when consuming the service, instead of doing

$index = $this->esClient->getIndex($indexName);
$query = .... ;
$response = $index->search($query, ['limit' => 30], Request::GET);

did

$index = $this->esClient->getIndex($indexName);
$query = .... ;

$search = $index->createSearch($query, ['limit' => 30], $myCustomResultBuilder);
$response = $search->search('', null, Request::GET);

yet.... I find this really over-complicated, just to inject a custom class mapping at runtime
I see two options to avoid this trouble:

  1. allow the injection of a class to be used by the ResultBuilder directly in the search() method
  2. allow the injection of class-mapping into the IndexNameMapper, but this is currently no possible (or I did not figure out how)

If PRs are accepted for either option, let me know...
think I should be able to deal with this

@damienalexandre
Copy link
Member

Hello, thanks for this detailed report!

This library does not support wildcard for class to index mapping as you found out.

I find this really over-complicated, just to inject a custom class mapping at runtime

You should be able to do the second one.
The ResultSetBuilder is one of the required argument of the Client:

public function __construct($config = [], callable $callback = null, LoggerInterface $logger = null, ResultSetBuilder $resultSetBuilder = null, IndexNameMapper $indexNameMapper = null)

So you can just inject your custom one when creating the Client, which depending on your framework / context may differ.

@dgoosens
Copy link
Author

hi @damienalexandre

thanks for getting back at me....
and yes, as you can see in my update, that's what I did

yet... this does not seem very efficient

so wondered if I submit a PR to enable:

  • prefix matching
  • regex matching

it would have a chance to be merged...
or if this is a deliberate choice by the devs to keep this as simple as possible ?

@damienalexandre
Copy link
Member

We tend to avoid adding options and features to keep the library easy to maintain 😬 - so my first answer would be to not accept such PR.

BUT there is room for improvement, we could do like we did for the mapping provider.

We added a MappingProviderInterface allowing users to bring their own mapping the way the want (Yaml, Php, Json... whatever).

We could add a IndexNameMapperInterface, and make it easier / clearer how to switch from one to another.

WDYT?

Cheers.

@dgoosens
Copy link
Author

We tend to avoid adding options and features to keep the library easy to maintain 😬 - so my first answer would be to not accept such PR.

BUT there is room for improvement, we could do like we did for the mapping provider.

We added a MappingProviderInterface allowing users to bring their own mapping the way the want (Yaml, Php, Json... whatever).

We could add a IndexNameMapperInterface, and make it easier / clearer how to switch from one to another.

WDYT?

Cheers.

I can do that....
not sure I will be able to deal with this immediately, but soon though

@damienalexandre damienalexandre changed the title dynamic index_class_mapping ? [Feature] dynamic index_class_mapping ? Nov 13, 2024
@damienalexandre damienalexandre added the help wanted Extra attention is needed label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants