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

Jump to bottom

Use Symfony HttpCache #156

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Use Symfony HttpCache #156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8e2983a
feat(http-cache): Use Symfony HttpCache
baiiko Jun 14, 2023
c3fb67d
fix: remove unuse use
baiiko Jun 14, 2023
792ead9
fix: coding style
baiiko Jun 14, 2023
41efab6
fix: coding style
baiiko Jun 14, 2023
0268b66
fix: coding style
baiiko Jun 14, 2023
0766868
fix: service
baiiko Jun 14, 2023
c2e2f0c
fix: cache dir
baiiko Jun 14, 2023
44f592e
fix: add missing dependency
baiiko Jun 14, 2023
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: 2 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@
"http-interop/http-factory-guzzle": "^1.0",
"league/pipeline": "^1.0",
"php-http/guzzle6-adapter": "^2.0",
"php-http/message-factory": "^1.1",
"psr/event-dispatcher": "^1.0",
"sylius/sylius": "^1.10",
"symfony/framework-bundle": "^5.4|^6.0",
"symfony/http-client": "^5.4|^6.0",
"symfony/lock": "^5.4|^6.0",
"symfony/property-access": "^4.3|^5.4|^6.0",
"symfony/property-info": "^5.4|^6.0",
Expand Down
23 changes: 21 additions & 2 deletions src/Client/ClientFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,21 @@

use Akeneo\Pim\ApiClient\AkeneoPimClientBuilder;
use Akeneo\Pim\ApiClient\AkeneoPimClientInterface;
use Symfony\Component\HttpClient\CachingHttpClient;
use Symfony\Component\HttpClient\HttpClient;
use Symfony\Component\HttpClient\HttplugClient;
use Symfony\Component\HttpKernel\HttpCache\Store;
use Synolia\SyliusAkeneoPlugin\Entity\ApiConfiguration;
use Synolia\SyliusAkeneoPlugin\Provider\Configuration\Api\ApiConnectionProviderInterface;

final class ClientFactory implements ClientFactoryInterface
{
private ?AkeneoPimClientInterface $akeneoClient = null;

public function __construct(private ApiConnectionProviderInterface $apiConnectionProvider)
{
public function __construct(
private ApiConnectionProviderInterface $apiConnectionProvider,
private string $cacheDir,
) {
}

public function createFromApiCredentials(): AkeneoPimClientInterface
Expand All @@ -27,6 +33,19 @@ public function createFromApiCredentials(): AkeneoPimClientInterface

$client = new AkeneoPimClientBuilder($apiConnection->getBaseUrl());

$httpClient = HttpClient::create();

$path = $this->cacheDir . '/akeneo';

if (is_dir($path) === false) {
mkdir($path);
}

$store = new Store($path);
$httpClient = new CachingHttpClient($httpClient, $store, ['default_ttl' => 3600, 'allow_revalidate' => true, 'debug' => getenv('APP_DEBUG')]);

$client->setHttpClient(new HttplugClient($httpClient));
Copy link
Member

@TheGrimmChester TheGrimmChester Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

When I try to import using your PR, I get this error:

In AkeneoPimClientBuilder.php line 103:
                                                                                                                                                                                       
  [TypeError]                                                                                                                                                                          
  Akeneo\Pim\ApiClient\AkeneoPimClientBuilder::setHttpClient(): Argument #1 ($httpClient) must be of type Akeneo\Pim\ApiClient\Client\ClientInterface, Symfony\Component\HttpClient\H  
  ttplugClient given, called in /SyliusAkeneoPlugin/src/Client/ClientFactory.php on line 47

Copy link
Contributor Author

@baiiko baiiko Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, maybe akeneo, change the contact since my pr is open ... @TheGrimmChester

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 confirm... Akeneo change that :

akeneo/api-php-client@e41246a

Copy link
Member

Choose a reason for hiding this comment

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

I think we can use the integrated akeneo cache system that creates a CachedResourceClient object to store resources.

$client = new AkeneoPimClientBuilder($apiConnection->getBaseUrl());
$client->enableCache();

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 think is not a good idea, because he use memory cache, and if you have some data, you risk to throw memory limit.

With my system, cache use is file system.

Copy link
Member

Choose a reason for hiding this comment

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

thanks @baiiko. We will check if we have to block the 11.3.0 version to benefits from the possibility to implement caching as you did. I think we should provide a way to store it to different places (like redis...) to be able to use the same cache if we are working on multiple server nodes to process the queue.

Copy link
Contributor Author

@baiiko baiiko Aug 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, is not possible, CachingHttpClient use only file system.

I use this system with your plugin and my process work faster than standard process.

Copy link
Contributor Author

@baiiko baiiko Aug 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You can talk with @oallain if necessary, he can talk with me directly :)


$this->akeneoClient = $client->buildAuthenticatedByPassword(
$apiConnection->getApiClientId(),
$apiConnection->getApiClientSecret(),
Expand Down
4 changes: 2 additions & 2 deletions src/Processor/AbstractImageProcessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
use Sylius\Component\Resource\Factory\FactoryInterface;
use Sylius\Component\Resource\Repository\RepositoryInterface;
use Symfony\Component\HttpFoundation\File\UploadedFile;
use Synolia\SyliusAkeneoPlugin\Client\ClientFactory;
use Synolia\SyliusAkeneoPlugin\Client\ClientFactoryInterface;
use Synolia\SyliusAkeneoPlugin\Entity\ProductConfiguration;
use Synolia\SyliusAkeneoPlugin\Entity\ProductConfigurationImageMapping;
use Throwable;
Expand All @@ -29,7 +29,7 @@ public function __construct(
private EntityManagerInterface $entityManager,
private FactoryInterface $productImageFactory,
protected LoggerInterface $logger,
private ClientFactory $clientFactory,
private ClientFactoryInterface $clientFactory,
) {
}

Expand Down
1 change: 1 addition & 0 deletions src/Resources/config/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ services:
public: false
bind:
$projectDir: '%kernel.project_dir%'
$cacheDir: '%kernel.cache_dir%'

Synolia\SyliusAkeneoPlugin\:
resource: '../../*'
Expand Down