Remove deprecated code and Factory in favor of full DI

[damienalexandre]

Dependency Injection should be the only way to deal with all our services.

For 2.0, let's get rid of all the error prone code and have only one way to use the lib - removing the Factory.

• Mapping providers should not need a context
• Mapping directory should be set in the Mapping Provider only, right now it's also needed in the Client
• Ability to change mapping filename #120 (comment)

[VincentLanglet]

Currently the factory allows to override multiple things with these options:

    public const CONFIG_BULK_SIZE = 'elastically_bulk_size';
    public const CONFIG_DENORMALIZER = 'elastically_denormalizer';
    public const CONFIG_INDEX_CLASS_MAPPING = 'elastically_index_class_mapping';
    public const CONFIG_INDEX_NAME_MAPPER = 'elastically_index_name_mapper';
    public const CONFIG_INDEX_PREFIX = 'elastically_index_prefix';
    public const CONFIG_MAPPINGS_DIRECTORY = 'elastically_mappings_directory';
    public const CONFIG_MAPPINGS_PROVIDER = 'elastically_mappings_provider';
    public const CONFIG_SERIALIZER = 'elastically_serializer';
    public const CONFIG_SERIALIZER_CONTEXT_BUILDER = 'elastically_serializer_context_builder';
    public const CONFIG_SERIALIZER_CONTEXT_PER_CLASS = 'elastically_serializer_context_per_class';

Am I wrong or some of them cannot be by the elastically configuration so far ?
I only see config for:

bulk_size
index_class_mapping
prefix
mapping_directory
mapping_provider_service
serializer. context_builder_service
serializer. context_mapping

So we're missing:

denormalizer
index_name_mapper
serializer

https://github.com/jolicode/elastically/blob/master/src/Bridge/Symfony/DependencyInjection/Configuration.php#L69

[damienalexandre]

I think you are right 👍

[lyrixx]

I don't think we need an open issue for such things.

Instead, let's do like Symfony: We keep an UPGRADE.md file up to date to track all deprecation. Then when the 2.x branch is created, we drop everything.

[VincentLanglet]

I don't think we need an open issue for such things.

Instead, let's do like Symfony: We keep an UPGRADE.md file up to date to track all deprecation. Then when the 2.x branch is created, we drop everything.

I may be wrong but to me this issue is more than just "Remove deprecated code", it's "Let's deprecate the Factory".

1. Currently the factory class is not deprecated.
2. As explained by my previous message, currently the factory class cannot be deprecated since everything is not allowed by the config.

So the step to resolve this issue would be:

• Add a serializer_service option
• Add a denormalizer_service option
• Add a index_name_mapper option
• Do more things...
• Deprecate the Factory class
• Close the issue
• In 2.x, remove the deprecated code

[lyrixx]

@VincentLanglet :

Currently the factory class is not deprecated.

I agree, let's deprecate it completely

As explained by my previous message, currently the factory class cannot be deprecated since everything is not allowed by the config.

We don't need bundle configuration for overriding a service. I'm a bit against adding options to override any part of the lib.

For exemple, the bulk_size, or the mapping_directly are values depending on the application structure / design.

The denormalizer, serializer, index_name_mapper are usually not changed. So we don't need an entry in the configuration to change them.

---

For example, in Symfony, you don't have an option to change the serializer / router, etc. if you want to do so, you decorate the existing one, or you replace it.

[VincentLanglet]

As explained by my previous message, currently the factory class cannot be deprecated since everything is not allowed by the config.

We don't need bundle configuration for overriding a service. I'm a bit against adding options to override any part of the lib.

For exemple, the bulk_size, or the mapping_directly are values depending on the application structure / design.

The denormalizer, serializer, index_name_mapper are usually not changed. So we don't need an entry in the configuration to change them.

For example, in Symfony, you don't have an option to change the serializer / router, etc. if you want to do so, you decorate the existing one, or you replace it.

Sure, I was expecting such answer.
Maybe I missing some point about services injections/decorations.

The definition of the Indexer is done this way in Elastically:

        ->set('elastically.abstract.indexer', Indexer::class)
            ->abstract()
            ->args([
                '$client' => service(Client::class),
                '$serializer' => service('serializer'),
                '$bulkMaxSize' => abstract_arg('bulk size'),
                '$bulkRequestParams' => [],
                '$contextBuilder' => abstract_arg('elastically.abstract.static_context_builder'),
            ])

If I decorate the serializer

services:
    serializer:
        class: App\NewSerializer

it will override the serializer for the whole application, not just for the elastically services.

What if I only want to change the serializer used by Elastically services ?

[lyrixx]

you need to override the service used in elastically:

service:
    elastically.my_connection.indexer:
        class: App\NewIndexer

[damienalexandre]

2.0 is soon to be released.

We didn't get any feedback or feature request about being able to set the "serializer" option for example from the Symfony configuration. I think we are going live without it.

I don't remember why we would deprecate the Factory? Isn't it a good way to build the services when outside of Symfony?