Skip to content

Commit 26acabe

Browse files
shiftedrealityshiftedreality
authored
[AWS S3] MC-38982: Add caching mechanism (#6346)
1 parent e9fa180 commit 26acabe

File tree

16 files changed

+342
-189
lines changed

16 files changed

+342
-189
lines changed

‎app/code/Magento/AwsS3/Driver/AwsS3.php‎

Lines changed: 107 additions & 71 deletions
Large diffs are not rendered by default.

‎app/code/Magento/AwsS3/Driver/AwsS3Factory.php‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@
99

1010
use Aws\S3\S3Client;
1111
use League\Flysystem\AwsS3v3\AwsS3Adapter;
12+
use League\Flysystem\Cached\CachedAdapter;
13+
use Magento\Framework\Exception\LocalizedException;
1214
use Magento\Framework\ObjectManagerInterface;
15+
use Magento\RemoteStorage\Driver\Cache\CacheFactory;
1316
use Magento\RemoteStorage\Driver\DriverException;
1417
use Magento\RemoteStorage\Driver\DriverFactoryInterface;
1518
use Magento\RemoteStorage\Driver\RemoteDriverInterface;
19+
use Magento\RemoteStorage\Model\Config;
1620

1721
/**
1822
* Creates a pre-configured instance of AWS S3 driver.
@@ -24,19 +28,54 @@ class AwsS3Factory implements DriverFactoryInterface
2428
*/
2529
private $objectManager;
2630

31+
/**
32+
* @var CacheFactory
33+
*/
34+
private $cacheFactory;
35+
36+
/**
37+
* @var Config
38+
*/
39+
private $config;
40+
2741
/**
2842
* @param ObjectManagerInterface $objectManager
43+
* @param CacheFactory $cacheFactory
44+
* @param Config $config
2945
*/
30-
public function __construct(ObjectManagerInterface $objectManager)
46+
public function __construct(ObjectManagerInterface $objectManager, CacheFactory $cacheFactory, Config $config)
3147
{
3248
$this->objectManager = $objectManager;
49+
$this->cacheFactory = $cacheFactory;
50+
$this->config = $config;
3351
}
3452

3553
/**
3654
* @inheritDoc
3755
*/
38-
public function create(array $config, string $prefix): RemoteDriverInterface
56+
public function create(): RemoteDriverInterface
3957
{
58+
try {
59+
return $this->createConfigured(
60+
$this->config->getConfig(),
61+
$this->config->getPrefix(),
62+
$this->config->getCacheAdapter(),
63+
$this->config->getCacheConfig()
64+
);
65+
} catch (LocalizedException $exception) {
66+
throw new DriverException(__($exception->getMessage()), $exception);
67+
}
68+
}
69+
70+
/**
71+
* @inheritDoc
72+
*/
73+
public function createConfigured(
74+
array $config,
75+
string $prefix,
76+
string $cacheAdapter,
77+
array $cacheConfig
78+
): RemoteDriverInterface {
4079
$config['version'] = 'latest';
4180

4281
if (empty($config['credentials']['key']) || empty($config['credentials']['secret'])) {
@@ -53,7 +92,10 @@ public function create(array $config, string $prefix): RemoteDriverInterface
5392
return $this->objectManager->create(
5493
AwsS3::class,
5594
[
56-
'adapter' => $adapter,
95+
'adapter' => $this->objectManager->create(CachedAdapter::class, [
96+
'adapter' => $adapter,
97+
'cache' => $this->cacheFactory->create($cacheAdapter, $cacheConfig)
98+
]),
5799
'objectUrl' => $client->getObjectUrl($adapter->getBucket(), $adapter->applyPathPrefix('.'))
58100
]
59101
);

‎app/code/Magento/AwsS3/Test/Unit/Driver/AwsS3Test.php‎

Lines changed: 64 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77

88
namespace Magento\AwsS3\Test\Unit\Driver;
99

10+
use League\Flysystem\AdapterInterface;
1011
use League\Flysystem\AwsS3v3\AwsS3Adapter;
11-
use League\Flysystem\Cached\CachedAdapter;
1212
use Magento\AwsS3\Driver\AwsS3;
1313
use Magento\Framework\Exception\FileSystemException;
1414
use PHPUnit\Framework\MockObject\MockObject;
@@ -37,7 +37,7 @@ class AwsS3Test extends TestCase
3737
*/
3838
protected function setUp(): void
3939
{
40-
$this->adapterMock = $this->createMock(CachedAdapter::class);
40+
$this->adapterMock = $this->getMockForAbstractClass(AdapterInterface::class);
4141
$loggerMock = $this->getMockForAbstractClass(LoggerInterface::class);
4242

4343
$this->driver = new AwsS3($this->adapterMock, $loggerMock, self::URL);
@@ -101,11 +101,6 @@ public function getAbsolutePathDataProvider(): array
101101
'test.txt',
102102
self::URL . 'test/test.txt'
103103
],
104-
[
105-
self::URL . 'media/',
106-
'media/image.jpg',
107-
self::URL . 'media/image.jpg'
108-
],
109104
[
110105
self::URL . 'media/',
111106
'/catalog/test.png',
@@ -140,6 +135,26 @@ public function getAbsolutePathDataProvider(): array
140135
self::URL . 'var/import/images/product_images/',
141136
self::URL . 'var/import/images/product_images/1.png',
142137
self::URL . 'var/import/images/product_images/1.png'
138+
],
139+
[
140+
self::URL . 'var/import/images/product_images/1.png',
141+
'',
142+
self::URL . 'var/import/images/product_images/1.png'
143+
],
144+
[
145+
self::URL . 'media/',
146+
'',
147+
self::URL . 'media/',
148+
],
149+
[
150+
self::URL . 'media/',
151+
self::URL . 'media',
152+
self::URL . 'media',
153+
],
154+
[
155+
self::URL,
156+
'',
157+
self::URL
143158
]
144159
];
145160
}
@@ -170,7 +185,7 @@ public function getRelativePathDataProvider(): array
170185
[
171186
'',
172187
'/test/test.txt',
173-
'test/test.txt'
188+
'/test/test.txt'
174189
],
175190
[
176191
self::URL,
@@ -216,14 +231,14 @@ public function isDirectoryDataProvider(): array
216231
return [
217232
[
218233
'some_directory/',
219-
'some_directory/',
234+
'some_directory',
220235
false,
221236
[],
222237
false
223238
],
224239
[
225240
'some_directory',
226-
'some_directory/',
241+
'some_directory',
227242
true,
228243
[
229244
'type' => AwsS3::TYPE_DIR
@@ -232,7 +247,7 @@ public function isDirectoryDataProvider(): array
232247
],
233248
[
234249
self::URL . 'some_directory',
235-
'some_directory/',
250+
'some_directory',
236251
true,
237252
[
238253
'type' => AwsS3::TYPE_DIR
@@ -241,7 +256,7 @@ public function isDirectoryDataProvider(): array
241256
],
242257
[
243258
self::URL . 'some_directory',
244-
'some_directory/',
259+
'some_directory',
245260
true,
246261
[
247262
'type' => AwsS3::TYPE_FILE
@@ -391,12 +406,12 @@ public function getRealPathSafetyDataProvider(): array
391406
public function testSearchDirectory(): void
392407
{
393408
$expression = '/*';
394-
$path = 'path/';
409+
$path = 'path';
395410
$subPaths = [
396-
['path' => 'path/1'],
397-
['path' => 'path/2']
411+
['path' => 'path/1', 'dirname' => self::URL],
412+
['path' => 'path/2', 'dirname' => self::URL]
398413
];
399-
$expectedResult = ['path/1', 'path/2'];
414+
$expectedResult = [self::URL . 'path/1', self::URL . 'path/2'];
400415
$this->adapterMock->expects(self::atLeastOnce())->method('has')
401416
->willReturnMap([
402417
[$path, true]
@@ -405,8 +420,10 @@ public function testSearchDirectory(): void
405420
->willReturnMap([
406421
[$path, ['type' => AwsS3::TYPE_DIR]]
407422
]);
408-
$this->adapterMock->expects(self::atLeastOnce())->method('listContents')->with($path, false)
423+
$this->adapterMock->expects(self::atLeastOnce())->method('listContents')
424+
->with($path, false)
409425
->willReturn($subPaths);
426+
410427
self::assertEquals($expectedResult, $this->driver->search($expression, $path));
411428
}
412429

@@ -415,13 +432,13 @@ public function testSearchDirectory(): void
415432
*/
416433
public function testSearchFiles(): void
417434
{
418-
$expression = "/*";
419-
$path = 'path/';
435+
$expression = '/*';
436+
$path = 'path';
420437
$subPaths = [
421-
['path' => 'path/1.jpg'],
422-
['path' => 'path/2.png']
438+
['path' => 'path/1.jpg', 'dirname' => self::URL],
439+
['path' => 'path/2.png', 'dirname' => self::URL]
423440
];
424-
$expectedResult = ['path/1.jpg', 'path/2.png'];
441+
$expectedResult = [self::URL . 'path/1.jpg', self::URL . 'path/2.png'];
425442

426443
$this->adapterMock->expects(self::atLeastOnce())->method('has')
427444
->willReturnMap([
@@ -433,6 +450,31 @@ public function testSearchFiles(): void
433450
]);
434451
$this->adapterMock->expects(self::atLeastOnce())->method('listContents')->with($path, false)
435452
->willReturn($subPaths);
453+
436454
self::assertEquals($expectedResult, $this->driver->search($expression, $path));
437455
}
456+
457+
/**
458+
* @throws FileSystemException
459+
*/
460+
public function testCreateDirectory(): void
461+
{
462+
$this->adapterMock->expects(self::exactly(2))
463+
->method('has')
464+
->willReturnMap([
465+
['test', true],
466+
['test/test2', false]
467+
]);
468+
$this->adapterMock->expects(self::once())
469+
->method('getMetadata')
470+
->willReturnMap([
471+
['test', ['type' => AwsS3::TYPE_DIR]]
472+
]);
473+
$this->adapterMock->expects(self::once())
474+
->method('createDir')
475+
->with('test/test2')
476+
->willReturn(true);
477+
478+
self::assertTrue($this->driver->createDirectory(self::URL . 'test/test2/'));
479+
}
438480
}

‎app/code/Magento/RemoteStorage/Driver/Cache/CacheFactory.php‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\RemoteStorage\Driver\Cache;
9+
10+
use League\Flysystem\Cached\CacheInterface;
11+
use League\Flysystem\Cached\Storage\Memory;
12+
use League\Flysystem\Cached\Storage\Predis;
13+
use Magento\RemoteStorage\Driver\DriverException;
14+
use Predis\Client;
15+
16+
/**
17+
* Provides cache adapters.
18+
*/
19+
class CacheFactory
20+
{
21+
public const ADAPTER_PREDIS = 'predis';
22+
public const ADAPTER_MEMORY = 'memory';
23+
24+
private const CACHE_KEY = 'storage';
25+
26+
/**
27+
* Cache for 30 days.
28+
*/
29+
private const CACHE_EXPIRATION = 30 * 86400;
30+
31+
/**
32+
* Create cache adapter.
33+
*
34+
* @param string $adapter
35+
* @param array $config
36+
* @return CacheInterface
37+
* @throws DriverException
38+
*/
39+
public function create(string $adapter, array $config = []): CacheInterface
40+
{
41+
switch ($adapter) {
42+
case self::ADAPTER_PREDIS:
43+
if (!class_exists(Client::class)) {
44+
throw new DriverException(__('Predis client is not installed'));
45+
}
46+
47+
return new Predis(new Client($config), self::CACHE_KEY, self::CACHE_EXPIRATION);
48+
case self::ADAPTER_MEMORY:
49+
return new Memory();
50+
}
51+
52+
throw new DriverException(__('Cache adapter %1 is not supported', $adapter));
53+
}
54+
}

‎app/code/Magento/RemoteStorage/Driver/DriverFactoryInterface.php‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,29 @@
1313
interface DriverFactoryInterface
1414
{
1515
/**
16-
* Creates pre-configured driver.
16+
* Creates driver from stored config.
17+
*
18+
* @return RemoteDriverInterface
19+
*
20+
* @throws DriverException
21+
*/
22+
public function create(): RemoteDriverInterface;
23+
24+
/**
25+
* Creates driver from config.
1726
*
1827
* @param array $config
1928
* @param string $prefix
29+
* @param string $cacheAdapter
30+
* @param array $cacheConfig
2031
* @return RemoteDriverInterface
2132
*
2233
* @throws DriverException
2334
*/
24-
public function create(array $config, string $prefix): RemoteDriverInterface;
35+
public function createConfigured(
36+
array $config,
37+
string $prefix,
38+
string $cacheAdapter,
39+
array $cacheConfig
40+
): RemoteDriverInterface;
2541
}

‎app/code/Magento/RemoteStorage/Driver/DriverPool.php‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020
class DriverPool extends BaseDriverPool implements DriverPoolInterface
2121
{
2222
public const PATH_DRIVER = 'remote_storage/driver';
23-
public const PATH_EXPOSE_URLS = 'remote_storage/expose_urls';
2423
public const PATH_PREFIX = 'remote_storage/prefix';
2524
public const PATH_CONFIG = 'remote_storage/config';
25+
public const PATH_CACHE = 'remote_storage/cache';
2626

2727
/**
2828
* Driver name.
@@ -79,10 +79,7 @@ public function getDriver($code = self::REMOTE): DriverInterface
7979
$driver = $this->config->getDriver();
8080

8181
if ($driver && $this->driverFactoryPool->has($driver)) {
82-
return $this->pool[$code] = $this->driverFactoryPool->get($driver)->create(
83-
$this->config->getConfig(),
84-
$this->config->getPrefix()
85-
);
82+
return $this->pool[$code] = $this->driverFactoryPool->get($driver)->create();
8683
}
8784

8885
throw new RuntimeException(__('Remote driver is not available.'));

‎app/code/Magento/RemoteStorage/Filesystem.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function getDirectoryRead($directoryCode, $driverCode = DriverPool::REMOT
6767
$code = $directoryCode . '_' . $driverCode;
6868

6969
if (!array_key_exists($code, $this->readInstances)) {
70-
$uri = $this->getUri($directoryCode) ?: '/';
70+
$uri = $this->getUri($directoryCode) ?: '';
7171

7272
$this->readInstances[$code] = $this->readFactory->create(
7373
$this->driverPool->getDriver()->getAbsolutePath('', $uri),
@@ -92,8 +92,7 @@ public function getDirectoryWrite($directoryCode, $driverCode = DriverPool::REMO
9292
$code = $directoryCode . '_' . $driverCode;
9393

9494
if (!array_key_exists($code, $this->writeInstances)) {
95-
$uri = $this->getUri($directoryCode) ?: '/';
96-
95+
$uri = $this->getUri($directoryCode) ?: '';
9796
$this->writeInstances[$code] = $this->writeFactory->create(
9897
$this->driverPool->getDriver()->getAbsolutePath('', $uri),
9998
$driverCode

0 commit comments

Comments
 (0)