Service names for IndexService instances are undocumented and confusing

[ghost]

Currently MappingPass registers IndexService instances with $namespace for each document type (or index):

$container->setDefinition($namespace, $indexServiceDefinition);

$namespace is also the $class that is set in the indexes configuration property, which is e.g. App\Document\Office.

This is very confusing: it means the service name for the IndexService to manipulate offices is the name of the document class, while it isn't an instance of the document itself. This makes it look as if I'm injecting instances of Office into my other services, whilst I'm actually injecting an index service for Office.

Perhaps a BC compatible way to improve this would be to allow overriding the name and do something like $indexesOverride[$namespace]['indexServiceName'] ?? $namespace, which allows you to define custom names.

A non-BC-compatible way would be to provide a name such as es.index_services.app_document_office, es.index_services.app_document_news_posts, and so on.

Also, I had to dig into the internals of this bundle to figure out what the service names were, as the documentation still shows the approach using the manager and repositories, i.e. it doesn't appear to be documented currently.

• PHP version: 7.3
• Elasticsearch version: 6.8.4
• Bundle version: v6.0.1
• Symfony version: 4.3.6

[einorler]

I agree with you that calling the index services by their dedicated document namespaces is somewhat confusing, as it is, this was the direction that was taken for further development of the bundle. I've already begun the work on v6.1, and I will try to find the best way to handle this, I'm considering aliases, they might even be compatible with <v6.x repository names, well see.

Concerning documentation: The docs.ongr.io is not updated and still point to the documentation of the v5.x, but you should refer to the entire documentation of the bundle itself (the dedicated docs of every version of the bundle live in Resources\doc directory of the repository and are always up to date).

[ghost]

@einorler Good to know, thanks!

I see now that using the name of the of the document was probably intended because you could do something like $this->container('manager')->get(MyDoc::class) before, which now became a $this->container->get(MyDoc::class) in a similar way.

An idea would be to create a nested Symfony container called e.g. "manager" to fill the void, which would then contain all these services to keep them out of the global namespace.

I was also a bit confused as to why every type now has a separate index, but I see this is the way Elasticsearch itself is going. I'll drop a link here in case anyone stumbles upon this.

[wasinger]

The usage of document class names for IndexService instances is indeed confusing, because usually in Symfony a class name used as service identifier means that this service is an instance of the named class.

I suggest the following:

• use service identifiers in the form 'ongr.es.index.[index_alias]'
• introduce an IndexManager (or maybe called IndexRegistryService) where all IndexService instances are registered. This registry or manager service should have methods getIndexByClass(MyDoc::class) and getIndexByAlias($index_alias) that return the correspoding IndexService instance. All services and controller actions only need a parameter with the IndexManager typehint to get this manager automatically injected in Symfony 4+ and are able to fetch the needed index service by alias or document class.
• Nice to have: a console command ongr:es:index:list that displays a table of all IndexService instances with the columns 'service id', 'index alias', and 'document class'.

@einorler: Are you already actively working on something like this? If not I could submit a PR if you agree.