jolicode / elastically

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

Decorate IndexBuilder #185

Closed orangevinz closed 2 months ago

orangevinz commented 2 months ago

Thanks for Elastically !

As I'd like to implement my own pattern for indice names (in place of date('Y-m-d-His')), so I guess decorating IndexBuilder is the right way to do that. But injected arguments are private and don't have getters, which obliges me to re-declare constructor. Would it be a good idea to add getters in the IndexBuilder class ? I can write a PR if you estimate that's the good practice.

To be able to do

#[AsDecorator(decorates: IndexBuilder::class)]
class IndexBuilderDecorator
{
    public function __construct(
        #[AutowireDecorated]
        private object $inner,
    ) {}

    public function createIndex(string $indexName, array $context = []): Index
    {
        // $this->inner->getMappingProvider();
    }
}

Instead of

    App\Search\IndexBuilderDecorator:
        class: App\Search\IndexBuilderDecorator
        decorates: 'JoliCode\Elastically\IndexBuilder'
        arguments:
            - '@.inner'
            - '@elastically.default.mapping.provider'
            - '@elastically.default.client'
            - '@elastically.default.index_name_mapper'
        decoration_inner_name: 'JoliCode\Elastically\IndexBuilder.inner'
class IndexBuilderDecorator
{
    public function __construct(
        private IndexBuilder $inner,
        private MappingProviderInterface $mappingProvider,
        private Client $client,
        private IndexNameMapper $indexNameMapper
    ) {}

    public function createIndex(string $indexName, array $context = []): Index
    {
        // $this->mappingProvider
    }
}
lyrixx commented 2 months ago

Hello, Thanks for the issue.

But injected arguments are private and don't have getters, which obliges me to re-declare constructor.

This is how decoration is supposed to work. If we change from private to protected, we'll have to support this "public-ish" API forever.

So the way to go it to require what you need in your own constructor. More over, with Symfony it's super easy :)

I hope you understand the rational.