Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to Algolia client v2 #244

Closed
wants to merge 1 commit into from
Closed

Upgrade to Algolia client v2 #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ class TestController extends Controller
$algoliaClient = $this->get('algolia.client');
var_dump($algoliaClient->listIndexes());

$index = $algoliaClient->initIndex(
$index = $algoliaClient->index(
$this->indexManager->getFullIndexName(Post::class)
);

Expand Down
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
],
"prefer-stable": true,
"require": {
"algolia/algoliasearch-client-php": "^1.23",
"algolia/algoliasearch-client-php": "2.0.0-alpha",
"doctrine/common": "^2.5",
"symfony/serializer": "^3.4.0 || ^4.0.0",
"symfony/property-access": "^3.4.0 || ^4.0.0",
Expand Down
9 changes: 5 additions & 4 deletions src/AlgoliaSearchBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

namespace Algolia\SearchBundle;

use AlgoliaSearch\Version;
use Algolia\SearchBundle\DependencyInjection\SearchRequirementsPass;
use Algolia\AlgoliaSearch\Support\Config;
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\HttpKernel\Kernel as SfKernel;

Expand All @@ -15,7 +14,9 @@ public function boot()
{
parent::boot();

Version::addSuffixUserAgentSegment('Symfony Search Bundle', self::VERSION);
Version::addSuffixUserAgentSegment('Symfony', SfKernel::VERSION);
if (class_exists('Algolia\AlgoliaSearch\Support\Config')) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what the PHP requirement of this package is (there's none in composer.json which is bad enough), but starting with 5.5.0 there's the ::class constant that you should use instead of magic strings. This allows IDEs to find code like this when looking for usages of a class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't specify it because I'm maintaining the same as Symfony 3.4, which requires ^5.5.9. I will update the PR. For the client, I'm maintaing 5.3 so I sort of lost the habit of ::class ^^

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I would still recommend applying the PHP requirement explicitly in composer.json to be on the safe side to avoid relying on other libraries' constraints.

Config::addCustomUserAgent('Symfony Search Bundle', self::VERSION);
Config::addCustomUserAgent('Symfony', SfKernel::VERSION);
}
}
}
26 changes: 14 additions & 12 deletions src/Engine/AlgoliaEngine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
namespace Algolia\SearchBundle\Engine;

use Algolia\SearchBundle\SearchableEntityInterface;
use AlgoliaSearch\Client;
use Algolia\AlgoliaSearch\Client;

class AlgoliaEngine implements EngineInterface
{
Expand Down Expand Up @@ -58,12 +58,14 @@ public function delete($indexName)

public function search($query, $indexName, $page = 1, $nbResults = null, array $parameters = [])
{
$params = array_merge($parameters, [
'hitsPerPage' => $nbResults,
$parameters += [
'page' => $page - 1,
]);
];
if ($nbResults) {
$parameters['hitsPerPage'] = $nbResults;
}

return $this->algolia->initIndex($indexName)->search($query, $params);
return $this->algolia->index($indexName)->search($query, $parameters);
}

public function searchIds($query, $indexName, $page = 1, $nbResults = null, array $parameters = [])
Expand All @@ -75,7 +77,7 @@ public function searchIds($query, $indexName, $page = 1, $nbResults = null, arra

public function count($query, $indexName)
{
$results = $this->algolia->initIndex($indexName)->search($query);
$results = $this->algolia->index($indexName)->search($query);

return (int) $results['nbHits'];
}
Expand Down Expand Up @@ -106,8 +108,8 @@ protected function doUpdate($searchableEntities)
$result = [];
foreach ($data as $indexName => $objects) {
$result[$indexName] = $this->algolia
->initIndex($indexName)
->addObjects($objects);
->index($indexName)
->saveObjects($objects);
}

return $result;
Expand All @@ -134,10 +136,10 @@ protected function doRemove($searchableEntities)
}

$result = [];
foreach ($data as $indexName => $objects) {
foreach ($data as $indexName => $objectsIds) {
$result[$indexName] = $this->algolia
->initIndex($indexName)
->deleteObjects($objects);
->index($indexName)
->deleteObjects($objectsIds);
}

return $result;
Expand All @@ -146,7 +148,7 @@ protected function doRemove($searchableEntities)
protected function doClear($indexName)
{
return [
$indexName => $this->algolia->initIndex($indexName)->clearIndex()
$indexName => $this->algolia->clearIndex($indexName)
];
}

Expand Down
5 changes: 5 additions & 0 deletions src/IndexManagerInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ public function search($query, $className, ObjectManager $objectManager, $page =
public function rawSearch($query, $className, $page = 0, $nbResults = null, array $parameters = []);

public function count($query, $className, array $parameters = []);

/*
* TODO: shouldBeIndexed method will be added in v4.0
*/
//public function shouldBeIndexed($entity);
}
5 changes: 3 additions & 2 deletions src/Resources/config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
<tag name="doctrine_mongodb.odm.event_subscriber" connection="default" />
</service>

<service id="algolia_client" class="AlgoliaSearch\Client" public="false">
<argument key="$applicationID">%env(ALGOLIA_APP_ID)%</argument>
<service id="algolia_client" class="Algolia\AlgoliaSearch\Client" public="false">
<factory class="Algolia\AlgoliaSearch\Client" method="create"/>
<argument key="$appId ">%env(ALGOLIA_APP_ID)%</argument>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are extra spaces behind the key here.

<argument key="$apiKey">%env(ALGOLIA_API_KEY)%</argument>
</service>

Expand Down
6 changes: 3 additions & 3 deletions src/Settings/AlgoliaSettingsManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Algolia\SearchBundle\Settings;

use AlgoliaSearch\Client;
use Algolia\AlgoliaSearch\Client;
use Symfony\Component\Filesystem\Filesystem;

class AlgoliaSettingsManager implements SettingsManagerInterface
Expand All @@ -27,7 +27,7 @@ public function backup(array $params)
}

foreach ($indices as $indexName) {
$index = $this->algolia->initIndex($indexName);
$index = $this->algolia->index($indexName);
$settings = $index->getSettings();
$filename = $this->getFileName($indexName, 'settings');

Expand All @@ -48,7 +48,7 @@ public function push(array $params)
$filename = $this->getFileName($indexName, 'settings');

if (is_readable($filename)) {
$index = $this->algolia->initIndex($indexName);
$index = $this->algolia->index($indexName);
$settings = json_decode(file_get_contents($filename));
$index->setSettings($settings);

Expand Down
8 changes: 4 additions & 4 deletions tests/Engine/AlgoliaSyncEngine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public function update($searchableEntities)
$batch = $this->doUpdate($searchableEntities);

foreach ($batch as $indexName => $response) {
$this->algolia->initIndex($indexName)->waitTask($response['taskID']);
$this->algolia->waitTask($indexName, $response['taskID']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the rationale of not using $this->algolia->index($indexName)->waitTask($response['taskID']). It feels weird to have this waitTask method taking the index name as 1st parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We recently exposed waitTask in the client for 2 main reasons:

  1. To make it easier to work with multiple index endpoint (which are on the client)
  2. To make it easier to work with other API, like analytics that returns a index name and a task ID.

More details: algolia/algoliasearch-client-php#410

I thought it would best to avoid instanciating an index "just" for a waitTask but I didn't benchmark anything, I guess the performance gain would be negligable. It just felt easier.

Note that internally, Client::waitTask will instanciate an Index until we find a better solution to not duplicate the wait logic, which I don't think is possible with PHP 5.3 because:

  • Traits are not available (introduce in 5.4)
  • this is not available in closures (introduce in 5.4)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And thanks for taking the time to review 🙏 Feedback is greatly appreciated!

}

return $this->formatIndexingResponse($batch);
Expand All @@ -26,7 +26,7 @@ public function remove($searchableEntities)
$batch = $this->doRemove($searchableEntities);

foreach ($batch as $indexName => $response) {
$this->algolia->initIndex($indexName)->waitTask($response['taskID']);
$this->algolia->waitTask($indexName, $response['taskID']);
}

return $this->formatIndexingResponse($batch);
Expand All @@ -38,7 +38,7 @@ public function clear($indexName)
$batch = $this->doClear($indexName);

foreach ($batch as $indexName => $response) {
$this->algolia->initIndex($indexName)->waitTask($response['taskID']);
$this->algolia->waitTask($indexName, $response['taskID']);
}
} catch (\Exception $e) {
return false;
Expand All @@ -53,7 +53,7 @@ public function delete($indexName)
$batch = $this->doDelete($indexName);

foreach ($batch as $indexName => $response) {
$this->algolia->initIndex($indexName)->waitTask($response['taskID']);
$this->algolia->waitTask($indexName, $response['taskID']);
}
} catch (\Exception $e) {
return false;
Expand Down
7 changes: 3 additions & 4 deletions tests/TestCase/SettingsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
namespace Algolia\SearchBundle;

use Algolia\SearchBundle\Entity\Post;
use Algolia\SearchBundle\Settings\AlgoliaSettingsManager;
use Algolia\SearchBundle\Settings\SettingsManagerInterface;
use AlgoliaSearch\Client;
use Algolia\AlgoliaSearch\Client;

class SettingsTest extends BaseTest
{
Expand Down Expand Up @@ -37,7 +36,7 @@ public function testBackup()
'hitsPerPage' => 51,
'maxValuesPerFacet' => 99,
];
$index = $this->client->initIndex($this->getPrefix().'posts');
$index = $this->client->index($this->getPrefix().'posts');
$task = $index->setSettings($settingsToUpdate);
$index->waitTask($task['taskID']);

Expand All @@ -62,7 +61,7 @@ public function testPush()
'hitsPerPage' => 12,
'maxValuesPerFacet' => 100,
];
$index = $this->client->initIndex($this->getPrefix().'posts');
$index = $this->client->index($this->getPrefix().'posts');
$task = $index->setSettings($settingsToUpdate);
$index->waitTask($task['taskID']);

Expand Down