1963 - support multiple direct_generators

[sergiuionescu]

See #1963

[ruflin]

Thanks for the contribution. Could you also add a test and a changelog entry?

Also see comment from @deguif

[sergiuionescu]

Thanks for the contribution. Could you also add a test and a changelog entry?

Also see comment from @deguif

@deguif I added the new method and deprecated the old one since it was a bit confusing to use. The toArray trick was required by \Elastica\Suggest\CandidateGenerator\DirectGenerator::toArray which is also updated.

@ruflin I updated the changelog. When do you think you could include this in a patch version?

Thanks.

[deguif]

Thanks @sergiuionescu for your work.
Some new comments related to how we handle deprecations.
Another one related to toArray change that can probably be done another way more standard.

[deguif]

Implementing NameableInterface here should do the trick and avoid customizing the toArray()

[sergiuionescu]

@deguif Thanks for suggesting the changes and align this with the future changes for the lib.

I do not see how implementing NameableInterface will do the trick here. The handling for that interface is:
$arr[$value instanceof NameableInterface ? $value->getName() : $key] = $value->toArray(); So we will still get a map with the "direct_generator" name and params, instead of a list of direct_generator's params.

We are looking to get

{
    "suggest": {
        "text": "obel prize",
        "simple_phrase": {
            "phrase": {
                "field": "title.trigram",
                "size": 1,
                "direct_generator": [
                    {
                        "field": "title.trigram",
                        "suggest_mode": "always"
                    },
                    {
                        "field": "title.reverse",
                        "suggest_mode": "always",
                        "pre_filter": "reverse",
                        "post_filter": "reverse"
                    }
                ]
            }
        }
    }
}

I think the override is the more transparent approach. Am i missing something with the NameableInterface, do you have an example where that is used to build a list?

[deguif]

You're absolutely right here.
NameableInterface is not handled correctly for list.
In this case I must admit, the first trick, overriding the toArray method of Phrase is probably the most adapted choice (and certainly kind of why it was done this way).

Overriding the toArray method of DirectGenerator to only return its parameters without its name would be a BC break.

I'm sorry for the noise my previous comment introduced regarding NameableInterface.

[sergiuionescu]

@deguif Please run the tests whenever possible, i reverted to the \Elastica\Suggest\Phrase::toArray override.

[deguif]

Yes done again, I did it this morning but Github was encountering issues.

[deguif]

Thanks @sergiuionescu

[sergiuionescu]

Thanks @sergiuionescu

Thank you @deguif , @ruflin for the quick action and support!