Bring NullEngine back

[ostrolucky]

• Algolia Search Bundle version: 4.0.0-alpha3

Description

We have difficulties to replace this functionality. Alternative you suggested requires writing implementation for interface with 11 (!!) methods. We can't use phpunit mocks, as we need to inject service into service container. We use algolia in multiple projects, this will slow down upgrade process a lot. Why was it removed?

[julienbourdeau]

Yeah I agree, the NullEngine is ❤️ It's probably going to come back before the final release.

[nunomaduro]

There is no engine in the new version, only SearchService. Maybe we can apply the following strategy:

AlgoliaSearchService.php
NullSearchService.php
SearchServiceInterface.php

What do you think @chloelbn ?

[chloelbn]

What bothers me with a NullSearchService is that all the logic regarding the manipulation of objects will have to be duplicated, while what we'd want is turn off the calls to Algolia and that's what the NullEngine was exactly doing. But that would imply reusing the Engine class again. I need to think about this a little bit more.

[ostrolucky]

With new design, (Void|Null)Client should be shipped instead. I have created following class

class TestClient implements HttpClientInterface
{
    /**
     * @var RequestInterface[]
     */
    private $requests = [];

    /**
     * {@inheritDoc}
     */
    public function sendRequest(RequestInterface $request, $timeout, $connectTimeout)
    {
        $this->requests[] = $request;

        return new Response(200, [], '{"objectIDs":[]}');
    }

    /**
     * @return mixed[][]
     */
    public function getRequestsForPath(string $path): array
    {
        $out = [];

        foreach ($this->requests as $request) {
            if (strstr($request->getUri()->getPath(), $path) === false) {
                continue;
            }

            $body = $request->getBody();
            $body->rewind();

            $out[] = json_decode($body->getContents(), true)['requests'];
        }

        return $out;
    }
}

Which additionally has feature allowing to pull algolia requests out and assert expected ones were triggered in test cases. Serves as damage control for missing #297

[chloelbn]

@ostrolucky I like your solution! If you're okay with it, I'll add it to the doc and upgrade guide. Thanks for your help!

[ostrolucky]

Definitely, feel free. Although wiring of this is not easy, because there is no service for http client nor ApiWrapper defined, so consumer needs to replace whole search.client service, including defining new services for ApiWrapper, TestClient, SearchConfig, ClusterHosts

[julienbourdeau]

I think the NullEngine is still useful when working locally. You may import data and refresh the database often for instance. You probably don't need to dispatch calls to Algolia when working on something totally unrelated.

It's also useful when working offline.

[chloelbn]

@julienbourdeau Could you tell me what would be the drawbacks of using @ostrolucky 's implementation instead of NullEngine ? I'm not sure I see them
@ostrolucky you can take advantage of the PHP Client setHttpClient method to achieve this. However while it's easy to use in a test suite, I'm still thinking about a satisfactory way of implementing this for dev environment.

[ostrolucky]

Drawback is that all users interested in this need to create class themselves and wire everything themselves. I think both me and @julienbourdeau would be interested to have NullClient in bundle/core, so it doesn't need to be copy pasted from documentation for all users interested in this feature.

As a first step to help here I have created algolia/algoliasearch-client-php#596.

[chloelbn]

@julienbourdeau is more in favor of a NullEngine, which changes the strategy.
I might be more leaning forward a NullClient as to me it may be best to turn off directly the requests and keep the logic of the others classes (DB related stuff, keep the AbstractResponses), and rely on https://www.algolia.com/doc/api-client/advanced/pass-options-to-the-http-client/php/?language=php

[chloelbn]

@ostrolucky I've opened a new issue to propose my solution, would you mind giving a look and share you thoughts?

Thanks

[chloelbn]

I'll follow what @nunomaduro originally suggested and create a NullSearchService. Thanks a lot for sharing all your suggestions everyone!

I'm closing this issue.