Skip to content

Remove autowiring #285

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

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Remove autowiring #285

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
54 changes: 43 additions & 11 deletions config/services.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,35 @@
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">

<services>
<defaults autowire="true" autoconfigure="true" />
<service id="meilisearch.engine" class="Meilisearch\Bundle\Engine">
<argument type="service" id="meilisearch.client" />
</service>

<prototype namespace="Meilisearch\Bundle\Command\" resource="../src/Command" />
<!-- After bumping to Symfony >5.1 deeprecate public services into private services -->
<service id="meilisearch.service" class="Meilisearch\Bundle\Services\MeilisearchService" public="true">
<argument /><!-- After bumping to Symfony 5.1 use type="abstract" -->
<argument type="service" id="meilisearch.engine" />
<argument type="collection" /><!-- After bumping to Symfony 5.1 use type="abstract" -->
<argument type="service" id="property_accessor" />
</service>
<service id="search.service" alias="meilisearch.service" public="true">
<deprecated package="meilisearch/search-bundle" version="0.14">The "%alias_id%" service alias is deprecated. Use "meilisearch.service" instead.</deprecated>
</service>

<service id="meilisearch.search_indexer_subscriber"
class="Meilisearch\Bundle\EventListener\DoctrineEventSubscriber"
public="true">
<service id="meilisearch.search_indexer_subscriber" class="Meilisearch\Bundle\EventListener\DoctrineEventSubscriber" public="true">
<argument type="service" id="meilisearch.service" />
</service>
<service id="search.search_indexer_subscriber" alias="meilisearch.search_indexer_subscriber">
<deprecated package="meilisearch/search-bundle" version="0.14">The "%alias_id%" service alias is deprecated. Use "meilisearch.search_indexer_subscriber" instead.</deprecated>
</service>

<service id="meilisearch.client" class="Meilisearch\Client" public="true" lazy="true">
<argument key="$url">%meili_url%</argument>
<argument key="$apiKey">%meili_api_key%</argument>
<argument key="$httpClient" type="service" id="psr18.http_client" on-invalid="ignore" />
<argument key="$clientAgents" type="collection">
<argument>%meili_symfony_version%</argument>
</argument>
<argument /><!-- url -->
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious, now we don't need to add the key=""?

Copy link
Member

Choose a reason for hiding this comment

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

In the src/DependencyInjection/MeilisearchExtension.php, I saw that you are changing to use a positional instead of a named argument. How is this better?

Copy link
Collaborator Author

@norkunas norkunas Aug 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

After PR, I left the comment about it :) #285 (comment)

<argument /><!-- api key -->
<argument type="service" id="psr18.http_client" on-invalid="ignore" /><!-- http client -->
<argument>null</argument><!-- request factory -->
<argument /><!-- client agents -->
<argument>null</argument><!-- stream factory -->
Comment on lines +30 to +35
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had to rework this as something was not right on Symfony 5.4: Symfony\Component\DependencyInjection\Exception\RuntimeException: Invalid constructor argument for service "meilisearch.client": integer expected but found string "url". Check your service definition.

</service>
<service id="search.client" alias="meilisearch.client" public="true">
<deprecated package="meilisearch/search-bundle" version="0.14">The "%alias_id%" service alias is deprecated. Use "meilisearch.client" instead.</deprecated>
Expand All @@ -34,5 +43,28 @@
<deprecated package="meilisearch/search-bundle" version="0.14">The "%alias_id%" service alias is deprecated. Use "meilisearch.client" instead.</deprecated>
</service>
<service id="Meilisearch\Bundle\SearchService" alias="meilisearch.service" />

<service id="Meilisearch\Bundle\Command\MeilisearchClearCommand">
<argument type="service" id="meilisearch.service" />
<tag name="console.command" />
</service>

<service id="Meilisearch\Bundle\Command\MeilisearchCreateCommand">
<argument type="service" id="meilisearch.service" />
<argument type="service" id="meilisearch.client" />
<tag name="console.command" />
</service>

<service id="Meilisearch\Bundle\Command\MeilisearchDeleteCommand">
<argument type="service" id="meilisearch.service" />
<tag name="console.command" />
</service>

<service id="Meilisearch\Bundle\Command\MeilisearchImportCommand">
<argument type="service" id="meilisearch.service" />
<argument type="service" id="doctrine" />
<argument type="service" id="meilisearch.client" />
<tag name="console.command" />
</service>
</services>
</container>
4 changes: 2 additions & 2 deletions src/DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ public function getConfigTreeBuilder(): TreeBuilder

$rootNode
->children()
->scalarNode('url')->end()
->scalarNode('url')->defaultValue('http://localhost:7700')->end()
->scalarNode('api_key')->end()
->scalarNode('prefix')
->defaultValue(null)
->defaultNull()
->end()
->scalarNode('nbResults')
->defaultValue(20)
Expand Down
18 changes: 7 additions & 11 deletions src/DependencyInjection/MeilisearchExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@

namespace Meilisearch\Bundle\DependencyInjection;

use Meilisearch\Bundle\Engine;
use Meilisearch\Bundle\MeilisearchBundle;
use Meilisearch\Bundle\Services\MeilisearchService;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\Loader;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
Expand Down Expand Up @@ -47,15 +44,14 @@ public function load(array $configs, ContainerBuilder $container): void
$container->removeDefinition('meilisearch.search_indexer_subscriber');
}

$engineDefinition = new Definition(Engine::class, [new Reference('meilisearch.client')]);
$container->findDefinition('meilisearch.client')
->replaceArgument(0, $config['url'])
->replaceArgument(1, $config['api_key'])
->replaceArgument(4, [MeilisearchBundle::qualifiedVersion()]);

$searchDefinition = (new Definition(
MeilisearchService::class,
[new Reference($config['serializer']), $engineDefinition, $config]
));

$container->setDefinition('meilisearch.service', $searchDefinition->setPublic(true));
$container->setAlias('search.service', 'meilisearch.service')->setPublic(true);
$container->findDefinition('meilisearch.service')
->replaceArgument(0, new Reference($config['serializer']))
->replaceArgument(2, $config);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions src/Services/MeilisearchService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@
use Meilisearch\Bundle\SearchService;
use Symfony\Component\Config\Definition\Exception\Exception;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessor;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
use Symfony\Component\Serializer\Normalizer\NormalizerInterface;

final class MeilisearchService implements SearchService
{
private NormalizerInterface $normalizer;
private Engine $engine;
private Collection $configuration;
private PropertyAccessor $propertyAccessor;
private PropertyAccessorInterface $propertyAccessor;
/**
* @var list<class-string>
*/
Expand All @@ -42,12 +42,12 @@ final class MeilisearchService implements SearchService
private array $classToSerializerGroup;
private array $indexIfMapping;

public function __construct(NormalizerInterface $normalizer, Engine $engine, array $configuration)
public function __construct(NormalizerInterface $normalizer, Engine $engine, array $configuration, PropertyAccessorInterface $propertyAccessor = null)
{
$this->normalizer = $normalizer;
$this->engine = $engine;
$this->configuration = new Collection($configuration);
$this->propertyAccessor = PropertyAccess::createPropertyAccessor();
$this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Symfony defines a property accessor already, so we can inject it through DI.


$this->setSearchableEntities();
$this->setAggregatorsAndEntitiesAggregators();
Expand Down
24 changes: 12 additions & 12 deletions tests/BaseKernelTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,22 @@
use Meilisearch\Bundle\Tests\Entity\Page;
use Meilisearch\Bundle\Tests\Entity\Post;
use Meilisearch\Bundle\Tests\Entity\Tag;
use Meilisearch\Client;
use Meilisearch\Exceptions\ApiException;
use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;

abstract class BaseKernelTestCase extends KernelTestCase
{
protected EntityManagerInterface $entityManager;
protected Client $client;
protected SearchService $searchService;

protected function setUp(): void
{
self::bootKernel();

$this->entityManager = $this->get('doctrine.orm.entity_manager');
$this->client = $this->get('meilisearch.client');
$this->searchService = $this->get('meilisearch.service');

$metaData = $this->entityManager->getMetadataFactory()->getAllMetadata();
Expand All @@ -39,10 +42,7 @@ protected function setUp(): void
$this->cleanUp();
}

/**
* @param int|string|null $id
*/
protected function createPost($id = null): Post
protected function createPost(int $id = null): Post
{
$post = new Post();
$post->setTitle('Test Post');
Expand Down Expand Up @@ -83,10 +83,7 @@ protected function createSearchablePost(): SearchableEntity
);
}

/**
* @param int|string|null $id
*/
protected function createComment($id = null): Comment
protected function createComment(int $id = null): Comment
{
$post = new Post(['title' => 'What a post!']);
$comment = new Comment();
Expand All @@ -104,10 +101,7 @@ protected function createComment($id = null): Comment
return $comment;
}

/**
* @param int|string|null $id
*/
protected function createImage($id = null): Image
protected function createImage(int $id = null): Image
{
$image = new Image();
$image->setUrl('https://docs.meilisearch.com/logo.png');
Expand Down Expand Up @@ -185,6 +179,12 @@ protected function getFileName(string $indexName, string $type): string
return sprintf('%s/%s.json', $indexName, $type);
}

protected function waitForAllTasks(): void
{
$firstTask = $this->client->getTasks()->getResults()[0];
$this->client->waitForTask($firstTask['uid']);
}

private function cleanUp(): void
{
(new Collection($this->searchService->getConfiguration()->get('indices')))
Expand Down
3 changes: 0 additions & 3 deletions tests/Integration/CommandsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use Meilisearch\Bundle\Tests\Entity\DummyCustomGroups;
use Meilisearch\Bundle\Tests\Entity\DynamicSettings;
use Meilisearch\Bundle\Tests\Entity\SelfNormalizable;
use Meilisearch\Client;
use Meilisearch\Endpoints\Indexes;
use Meilisearch\Exceptions\ApiException;
use Meilisearch\Search\SearchResult;
Expand All @@ -22,7 +21,6 @@ class CommandsTest extends BaseKernelTestCase
{
private static string $indexName = 'posts';

protected Client $client;
protected Application $application;
protected Indexes $index;

Expand All @@ -34,7 +32,6 @@ protected function setUp(): void
{
parent::setUp();

$this->client = $this->get('meilisearch.client');
$this->index = $this->client->index($this->getPrefix().self::$indexName);
$this->application = new Application(self::createKernel());
}
Expand Down
6 changes: 3 additions & 3 deletions tests/Integration/DependencyInjectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ protected function getContainerExtensions(): array

public function testHasMeilisearchVersionDefinitionAfterLoad(): void
{
$this->load();
$this->load(['url' => 'http://meilisearch:7700', 'api_key' => null]);

$this->assertContainerBuilderHasServiceDefinitionWithArgument('meilisearch.client', '$clientAgents', ['%meili_symfony_version%']);
$this->assertContainerBuilderHasServiceDefinitionWithArgument('meilisearch.client', 4, [MeilisearchBundle::qualifiedVersion()]);
}

public function testHasMeilisearchVersionFromConstantAfterLoad(): void
{
$this->load();
$this->load(['url' => 'http://meilisearch:7700', 'api_key' => null]);

$this->assertContainerBuilderHasParameter('meili_symfony_version', MeilisearchBundle::qualifiedVersion());
}
Expand Down
Loading