Skip to content

Commit 6063639

Browse files
danmooney2danmooney2
authored
Merge pull request #7006 from magento-arcticfoxes/B2B-1785
B2B-1785: Cannot enable remote storage with install command when modules are not enabled
2 parents 81c9d65 + fe8d458 commit 6063639

File tree

4 files changed

+760
-19
lines changed

4 files changed

+760
-19
lines changed

‎app/code/Magento/RemoteStorage/Setup/ConfigOptionsList.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,13 @@ public function createConfig(array $options, DeploymentConfig $deploymentConfig)
141141
*/
142142
public function validate(array $options, DeploymentConfig $deploymentConfig): array
143143
{
144+
// deployment configuration existence determines readiness of object manager to resolve remote storage drivers
145+
$isDeploymentConfigExists = (bool) $deploymentConfig->getConfigData();
146+
147+
if (!$isDeploymentConfigExists) {
148+
return [];
149+
}
150+
144151
$driver = $options[self::OPTION_REMOTE_STORAGE_DRIVER] ?? DriverPool::FILE;
145152

146153
if ($driver === DriverPool::FILE) {

‎app/code/Magento/RemoteStorage/Test/Unit/Setup/ConfigOptionsListTest.php

Lines changed: 191 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,191 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\RemoteStorage\Test\Unit\Setup;
8+
9+
use Magento\Framework\App\DeploymentConfig;
10+
use Magento\Framework\Exception\LocalizedException;
11+
use Magento\RemoteStorage\Driver\DriverFactoryPool;
12+
use Magento\RemoteStorage\Driver\RemoteDriverInterface;
13+
use Magento\RemoteStorage\Setup\ConfigOptionsList;
14+
use Magento\RemoteStorage\Driver\DriverFactoryInterface;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use PHPUnit\Framework\TestCase;
17+
use Psr\Log\LoggerInterface;
18+
19+
class ConfigOptionsListTest extends TestCase
20+
{
21+
/**
22+
* @var DriverFactoryPool|MockObject
23+
*/
24+
private $driverFactoryPoolMock;
25+
26+
/**
27+
* @var LoggerInterface|MockObject
28+
*/
29+
private $loggerMock;
30+
31+
/**
32+
* @var ConfigOptionsList
33+
*/
34+
private $configOptionsList;
35+
36+
/**
37+
* @return void
38+
* @throws \Magento\Framework\Exception\FileSystemException
39+
*/
40+
protected function setUp(): void
41+
{
42+
$this->driverFactoryPoolMock = $this->getMockBuilder(DriverFactoryPool::class)
43+
->disableOriginalConstructor()
44+
->getMock();
45+
46+
$this->loggerMock = $this->getMockBuilder(LoggerInterface::class)
47+
->disableOriginalConstructor()
48+
->getMock();
49+
50+
$this->configOptionsList = new ConfigOptionsList(
51+
$this->driverFactoryPoolMock,
52+
$this->loggerMock
53+
);
54+
}
55+
56+
/**
57+
* @param array $input
58+
* @param bool $isDeploymentConfigExists
59+
* @param array $expectedOutput
60+
* @dataProvider validateDataProvider
61+
*/
62+
public function testValidate(array $input, bool $isDeploymentConfigExists, array $expectedOutput)
63+
{
64+
$deploymentConfigMock = $this->getMockBuilder(DeploymentConfig::class)
65+
->disableOriginalConstructor()
66+
->getMock();
67+
68+
$deploymentConfigMock
69+
->expects(static::once())
70+
->method('getConfigData')
71+
->willReturn($isDeploymentConfigExists);
72+
73+
$isConnectionToBeTested = $isDeploymentConfigExists && isset(
74+
$input['remote-storage-region'],
75+
$input['remote-storage-bucket']
76+
);
77+
78+
if ($isConnectionToBeTested) {
79+
$driverFactoryMock = $this->getMockBuilder(DriverFactoryInterface::class)
80+
->disableOriginalConstructor()
81+
->getMock();
82+
83+
$this->driverFactoryPoolMock
84+
->expects(static::once())
85+
->method('get')
86+
->with($input['remote-storage-driver'])
87+
->willReturn($driverFactoryMock);
88+
89+
$remoteDriverMock = $this->getMockBuilder(RemoteDriverInterface::class)
90+
->disableOriginalConstructor()
91+
->getMock();
92+
93+
$driverFactoryMock
94+
->expects(static::once())
95+
->method('createConfigured')
96+
->willReturn($remoteDriverMock);
97+
98+
$testMethodExpectation = $remoteDriverMock->expects(static::once())->method('test');
99+
100+
$isExceptionExpectedToBeCaught = (bool) count($expectedOutput);
101+
102+
if ($isExceptionExpectedToBeCaught) {
103+
$adapterErrorMessage = str_replace('Adapter error: ', '', $expectedOutput[0]);
104+
$exception = new LocalizedException(__($adapterErrorMessage));
105+
$testMethodExpectation->willThrowException($exception);
106+
$this->loggerMock->expects(static::once())->method('critical')->with($exception->getMessage());
107+
}
108+
}
109+
110+
$this->assertEquals(
111+
$expectedOutput,
112+
$this->configOptionsList->validate($input, $deploymentConfigMock)
113+
);
114+
}
115+
116+
/**
117+
* @return array
118+
*/
119+
public function validateDataProvider()
120+
{
121+
return [
122+
'Local File Storage Before Deployment Config Exists' => [
123+
[], false, [],
124+
],
125+
'Local File Storage After Deployment Config Exists' => [
126+
[], true, [],
127+
],
128+
'Remote Storage Before Deployment Config Exists' => [
129+
[
130+
'remote-storage-driver' => 'aws-s3',
131+
'remote-storage-region' => 'us-east-1',
132+
'remote-storage-bucket' => 'bucket1',
133+
],
134+
false,
135+
[],
136+
],
137+
'Remote Storage Missing Region' => [
138+
[
139+
'remote-storage-driver' => 'aws-s3',
140+
'remote-storage-bucket' => 'bucket1',
141+
],
142+
true,
143+
[
144+
'Region is required',
145+
],
146+
],
147+
'Remote Storage Missing Bucket' => [
148+
[
149+
'remote-storage-driver' => 'aws-s3',
150+
'remote-storage-region' => 'us-east-1',
151+
],
152+
true,
153+
[
154+
'Bucket is required',
155+
],
156+
],
157+
'Remote Storage Missing Region and Bucket' => [
158+
[
159+
'remote-storage-driver' => 'aws-s3',
160+
],
161+
true,
162+
[
163+
'Region is required',
164+
'Bucket is required',
165+
],
166+
],
167+
'Valid Remote Storage Config with Successful Test Connection' => [
168+
[
169+
'remote-storage-driver' => 'aws-s3',
170+
'remote-storage-region' => 'us-east-1',
171+
'remote-storage-bucket' => 'bucket1',
172+
'remote-storage-prefix' => '',
173+
],
174+
true,
175+
[],
176+
],
177+
'Valid Remote Storage With Unsuccessful Test Connection' => [
178+
[
179+
'remote-storage-driver' => 'aws-s3',
180+
'remote-storage-region' => 'us-east-1',
181+
'remote-storage-bucket' => 'bucket1',
182+
'remote-storage-prefix' => '',
183+
],
184+
true,
185+
[
186+
'Adapter error: [Message from LocalizedException]',
187+
]
188+
],
189+
];
190+
}
191+
}

‎setup/src/Magento/Setup/Model/Installer.php

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
use Magento\Framework\Setup\UpgradeSchemaInterface;
4747
use Magento\Framework\Validation\ValidationException;
4848
use Magento\PageCache\Model\Cache\Type as PageCache;
49+
use Magento\RemoteStorage\Driver\DriverException;
4950
use Magento\Setup\Console\Command\InstallCommand;
5051
use Magento\Setup\Controller\ResponseTypeInterface;
5152
use Magento\Setup\Exception;
@@ -55,6 +56,8 @@
5556
use Magento\Setup\Module\SetupFactory;
5657
use Magento\Setup\Validator\DbValidator;
5758
use Magento\Store\Model\Store;
59+
use Magento\RemoteStorage\Setup\ConfigOptionsList as RemoteStorageValidator;
60+
use ReflectionException;
5861

5962
/**
6063
* Class Installer contains the logic to install Magento application.
@@ -356,6 +359,11 @@ public function install($request)
356359
}
357360
$script[] = ['Installing database schema:', 'installSchema', [$request]];
358361
$script[] = ['Installing search configuration...', 'installSearchConfiguration', [$request]];
362+
$script[] = [
363+
'Validating remote storage configuration...',
364+
'validateRemoteStorageConfiguration',
365+
[$request]
366+
];
359367
$script[] = ['Installing user configuration...', 'installUserConfig', [$request]];
360368
$script[] = ['Enabling caches:', 'updateCaches', [true]];
361369
$script[] = ['Installing data...', 'installDataFixtures', [$request]];
@@ -385,8 +393,13 @@ public function install($request)
385393
foreach ($script as $item) {
386394
list($message, $method, $params) = $item;
387395
$this->log->log($message);
388-
// phpcs:ignore Magento2.Functions.DiscouragedFunction
389-
call_user_func_array([$this, $method], $params);
396+
try {
397+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
398+
call_user_func_array([$this, $method], $params);
399+
} catch (RuntimeException | DriverException $e) {
400+
$this->revertRemoteStorageConfiguration();
401+
throw $e;
402+
}
390403
$this->logProgress();
391404
}
392405
$this->log->logSuccess('Magento installation complete.');
@@ -1197,6 +1210,31 @@ public function installSearchConfiguration($data)
11971210
$searchConfig->saveConfiguration($data);
11981211
}
11991212

1213+
/**
1214+
* Validate remote storage on install. Since it is a deployment-based configuration, the config is already present,
1215+
* but this function confirms it can connect after Object Manager
1216+
* has all necessary dependencies loaded to do so.
1217+
*
1218+
* @param array $data
1219+
* @throws ValidationException
1220+
* @throws Exception
1221+
*/
1222+
public function validateRemoteStorageConfiguration(array $data)
1223+
{
1224+
try {
1225+
$remoteStorageValidator = $this->objectManagerProvider->get()->get(RemoteStorageValidator::class);
1226+
} catch (ReflectionException $e) { // RemoteStorage module is not available; return early
1227+
return;
1228+
}
1229+
1230+
$validationErrors = $remoteStorageValidator->validate($data, $this->deploymentConfig);
1231+
1232+
if (!empty($validationErrors)) {
1233+
$this->revertRemoteStorageConfiguration();
1234+
throw new ValidationException(__(implode(PHP_EOL, $validationErrors)));
1235+
}
1236+
}
1237+
12001238
/**
12011239
* Create data handler
12021240
*
@@ -1762,4 +1800,19 @@ private function getDisabledCacheTypes(array $cacheTypesToCheck): array
17621800

17631801
return $disabledCaches;
17641802
}
1803+
1804+
/**
1805+
* Revert remote storage configuration back to local file driver
1806+
*/
1807+
private function revertRemoteStorageConfiguration()
1808+
{
1809+
if (!$this->deploymentConfigWriter->checkIfWritable()) {
1810+
return;
1811+
}
1812+
1813+
$remoteStorageData = new ConfigData(ConfigFilePool::APP_ENV);
1814+
$remoteStorageData->set('remote_storage', ['driver' => 'file']);
1815+
$configData = [$remoteStorageData->getFileKey() => $remoteStorageData->getData()];
1816+
$this->deploymentConfigWriter->saveConfig($configData, true);
1817+
}
17651818
}

0 commit comments

Comments
 (0)