Skip to content

Commit a9394c2

Browse files
danielrenauddanielrenaud
committed
MC-33394: Deliver major change Elasticsearch stories
- Fix and refactor upgrade scenario
1 parent ad0bc5f commit a9394c2

File tree

12 files changed

+130
-63
lines changed

12 files changed

+130
-63
lines changed

‎app/code/Magento/Elasticsearch/Setup/Validator.php

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,19 @@ public function __construct(ClientResolver $clientResolver)
3131
/**
3232
* Checks Elasticsearch Connection
3333
*
34-
* @param array $searchConfig
35-
* @return array
34+
* @return string[]
3635
*/
37-
public function validate(array $searchConfig = []): array
36+
public function validate(): array
3837
{
3938
$errors = [];
40-
$searchEngine = $searchConfig['search-engine'] ?? null;
4139
try {
42-
$client = $this->clientResolver->create($searchEngine);
40+
$client = $this->clientResolver->create();
4341
if (!$client->testConnection()) {
44-
$errors[] = 'Elasticsearch connection validation failed';
42+
$errors[] = 'Could not validate a connection to Elasticsearch.'
43+
. ' Verify that the Elasticsearch host and port are configured correctly.';
4544
}
4645
} catch (\Exception $e) {
47-
$errors[] = 'Elasticsearch connection validation failed: ' . $e->getMessage();
46+
$errors[] = 'Could not validate a connection to Elasticsearch. ' . $e->getMessage();
4847
}
4948
return $errors;
5049
}

‎app/code/Magento/Elasticsearch/Test/Unit/Setup/ValidatorTest.php

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public function testValidate()
5454
$this->clientResolverMock
5555
->expects($this->once())
5656
->method('create')
57-
->with(null)
5857
->willReturn($this->elasticsearchClientMock);
5958
$this->elasticsearchClientMock->expects($this->once())->method('testConnection')->willReturn(true);
6059

@@ -68,12 +67,14 @@ public function testValidateFail()
6867
$this->clientResolverMock
6968
->expects($this->once())
7069
->method('create')
71-
->with($searchEngine)
7270
->willReturn($this->elasticsearchClientMock);
7371
$this->elasticsearchClientMock->expects($this->once())->method('testConnection')->willReturn(false);
7472

75-
$expected = ['Elasticsearch connection validation failed'];
76-
$this->assertEquals($expected, $this->validator->validate(['search-engine' => $searchEngine]));
73+
$expected = [
74+
'Could not validate a connection to Elasticsearch.'
75+
. ' Verify that the Elasticsearch host and port are configured correctly.'
76+
];
77+
$this->assertEquals($expected, $this->validator->validate());
7778
}
7879

7980
public function testValidateFailException()
@@ -88,7 +89,7 @@ public function testValidateFailException()
8889
->method('testConnection')
8990
->willThrowException(new \Exception($exceptionMessage));
9091

91-
$expected = ['Elasticsearch connection validation failed: ' . $exceptionMessage];
92+
$expected = ['Could not validate a connection to Elasticsearch. ' . $exceptionMessage];
9293
$this->assertEquals($expected, $this->validator->validate());
9394
}
9495
}

‎app/code/Magento/Search/Model/SearchEngine/Validator.php

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,23 +47,20 @@ public function __construct(
4747
/**
4848
* @inheritDoc
4949
*/
50-
public function validate(array $searchConfig = []): array
50+
public function validate(): array
5151
{
5252
$errors = [];
53-
54-
$currentEngine = isset($searchConfig['search-engine'])
55-
? $searchConfig['search-engine']
56-
: $this->scopeConfig->getValue('catalog/search/engine');
57-
53+
$currentEngine = $this->scopeConfig->getValue('catalog/search/engine');
5854
if (isset($this->engineBlacklist[$currentEngine])) {
5955
$blacklistedEngine = $this->engineBlacklist[$currentEngine];
60-
$errors[] = "Search engine '{$blacklistedEngine}' is not supported. "
61-
. "Fix search configuration and try again.";
56+
$errors[] = "Your current search engine, '{$blacklistedEngine}', is not supported."
57+
. " You must install a supported search engine before upgrading."
58+
. " See the System Upgrade Guide for more information.";
6259
}
6360

6461
if (isset($this->engineValidators[$currentEngine])) {
6562
$validator = $this->engineValidators[$currentEngine];
66-
$validationErrors = $validator->validate($searchConfig);
63+
$validationErrors = $validator->validate();
6764
$errors = array_merge($errors, $validationErrors);
6865
}
6966
return $errors;

‎app/code/Magento/Search/Model/SearchEngine/ValidatorInterface.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ interface ValidatorInterface
1515
/**
1616
* Validate search engine
1717
*
18-
* @param array $searchConfig
1918
* @return string[] array of errors, empty array if validation passed
2019
*/
21-
public function validate(array $searchConfig = []): array;
20+
public function validate(): array;
2221
}

‎app/code/Magento/Search/Test/Unit/Model/SearchEngine/ValidatorTest.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,20 +56,35 @@ public function testValidateValid()
5656

5757
public function testValidateBlacklist()
5858
{
59-
$expectedErrors = ["Search engine 'Bad Engine' is not supported. Fix search configuration and try again."];
59+
$this->scopeConfigMock
60+
->expects($this->once())
61+
->method('getValue')
62+
->with('catalog/search/engine')
63+
->willReturn('badEngine');
64+
65+
$expectedErrors = [
66+
"Your current search engine, 'Bad Engine', is not supported."
67+
. " You must install a supported search engine before upgrading."
68+
. " See the System Upgrade Guide for more information."
69+
];
6070

61-
$this->assertEquals($expectedErrors, $this->validator->validate(['search-engine' => 'badEngine']));
71+
$this->assertEquals($expectedErrors, $this->validator->validate());
6272
}
6373

6474
public function testValidateInvalid()
6575
{
6676
$expectedErrors = ['Validation failed for otherEngine'];
6777

78+
$this->scopeConfigMock
79+
->expects($this->once())
80+
->method('getValue')
81+
->with('catalog/search/engine')
82+
->willReturn('otherEngine');
6883
$this->otherEngineValidatorMock
6984
->expects($this->once())
7085
->method('validate')
7186
->willReturn($expectedErrors);
7287

73-
$this->assertEquals($expectedErrors, $this->validator->validate(['search-engine' => 'otherEngine']));
88+
$this->assertEquals($expectedErrors, $this->validator->validate());
7489
}
7590
}

‎setup/src/Magento/Setup/Console/Command/UpgradeCommand.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
use Magento\Framework\Setup\Declaration\Schema\DryRunLogger;
1414
use Magento\Framework\Setup\Declaration\Schema\OperationsExecutor;
1515
use Magento\Setup\Model\InstallerFactory;
16-
use Magento\Setup\Model\SearchConfig;
16+
use Magento\Setup\Model\SearchConfigFactory;
1717
use Symfony\Component\Console\Input\ArrayInput;
1818
use Symfony\Component\Console\Input\InputInterface;
1919
use Symfony\Component\Console\Input\InputOption;
@@ -48,26 +48,26 @@ class UpgradeCommand extends AbstractSetupCommand
4848
private $appState;
4949

5050
/**
51-
* @var SearchConfig
51+
* @var SearchConfigFactory
5252
*/
53-
private $searchConfig;
53+
private $searchConfigFactory;
5454

5555
/**
5656
* @param InstallerFactory $installerFactory
57+
* @param SearchConfigFactory $searchConfigFactory
5758
* @param DeploymentConfig $deploymentConfig
5859
* @param AppState|null $appState
59-
* @param SearchConfig|null $searchConfig
6060
*/
6161
public function __construct(
6262
InstallerFactory $installerFactory,
63+
SearchConfigFactory $searchConfigFactory,
6364
DeploymentConfig $deploymentConfig = null,
64-
AppState $appState = null,
65-
SearchConfig $searchConfig = null
65+
AppState $appState = null
6666
) {
6767
$this->installerFactory = $installerFactory;
68+
$this->searchConfigFactory = $searchConfigFactory;
6869
$this->deploymentConfig = $deploymentConfig ?: ObjectManager::getInstance()->get(DeploymentConfig::class);
6970
$this->appState = $appState ?: ObjectManager::getInstance()->get(AppState::class);
70-
$this->searchConfig = $searchConfig ?: ObjectManager::getInstance()->get(SearchConfig::class);
7171
parent::__construct();
7272
}
7373

@@ -126,9 +126,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
126126
try {
127127
$request = $input->getOptions();
128128
$keepGenerated = $input->getOption(self::INPUT_KEY_KEEP_GENERATED);
129-
$this->searchConfig->validateSearchEngine();
130129
$installer = $this->installerFactory->create(new ConsoleLogger($output));
131130
$installer->updateModulesSequence($keepGenerated);
131+
$searchConfig = $this->searchConfigFactory->create();
132+
$searchConfig->validateSearchEngine();
132133
$installer->installSchema($request);
133134
$installer->installDataFixtures($request);
134135

@@ -150,7 +151,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
150151
);
151152
}
152153
} catch (\Exception $e) {
153-
$output->writeln($e->getMessage());
154+
$output->writeln('<error>' . $e->getMessage() . '</error>');
154155
return \Magento\Framework\Console\Cli::RETURN_FAILURE;
155156
}
156157

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ public function install($request)
338338
$script[] = ['Cleaning up database...', 'cleanupDb', []];
339339
}
340340
$script[] = ['Installing database schema:', 'installSchema', [$request]];
341+
$script[] = ['Installing search configuration...', 'installSearchConfiguration', [$request]];
341342
$script[] = ['Installing user configuration...', 'installUserConfig', [$request]];
342343
$script[] = ['Enabling caches:', 'updateCaches', [true]];
343344
$script[] = ['Installing data...', 'installDataFixtures', [$request]];
@@ -1104,7 +1105,18 @@ public function installUserConfig($data)
11041105
$configModel->setDataByPath($key, $val);
11051106
$configModel->save();
11061107
}
1108+
}
11071109

1110+
/**
1111+
* Configure search engine on install
1112+
*
1113+
* @param \ArrayObject|array $data
1114+
* @return void
1115+
* @throws \Magento\Framework\Validation\ValidationException
1116+
* @throws \Magento\Setup\Exception
1117+
*/
1118+
public function installSearchConfiguration($data)
1119+
{
11081120
/** @var SearchConfig $searchConfig */
11091121
$searchConfig = $this->objectManagerProvider->get()->get(SearchConfig::class);
11101122
$searchConfig->saveConfiguration($data);

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
namespace Magento\Setup\Model;
99

10-
use Magento\Framework\ObjectManagerInterface;
1110
use Magento\Framework\Setup\Option\AbstractConfigOption;
1211
use Magento\Framework\Validation\ValidationException;
1312
use Magento\Search\Model\SearchEngine\Validator;
@@ -25,20 +24,28 @@ class SearchConfig
2524
private $searchConfigOptionsList;
2625

2726
/**
28-
* @var ObjectManagerInterface
27+
* @var Validator
2928
*/
30-
private $objectManager;
29+
private $searchValidator;
30+
31+
/**
32+
* @var CompositeInstallConfig
33+
*/
34+
private $installConfig;
3135

3236
/**
33-
* @param ObjectManagerInterface $objectManager
3437
* @param SearchConfigOptionsList $searchConfigOptionsList
38+
* @param Validator $searchValidator
39+
* @param CompositeInstallConfig $installConfig
3540
*/
3641
public function __construct(
37-
ObjectManagerInterface $objectManager,
38-
SearchConfigOptionsList $searchConfigOptionsList
42+
SearchConfigOptionsList $searchConfigOptionsList,
43+
Validator $searchValidator,
44+
CompositeInstallConfig $installConfig
3945
) {
40-
$this->objectManager = $objectManager;
4146
$this->searchConfigOptionsList = $searchConfigOptionsList;
47+
$this->searchValidator = $searchValidator;
48+
$this->installConfig = $installConfig;
4249
}
4350

4451
/**
@@ -55,26 +62,21 @@ public function saveConfiguration(array $inputOptions)
5562
$this->validateSearchEngineSelection($searchConfigOptions);
5663
}
5764
try {
58-
/** @var CompositeInstallConfig $installConfig */
59-
$installConfig = $this->objectManager->get(CompositeInstallConfig::class);
60-
$installConfig->configure($searchConfigOptions);
65+
$this->installConfig->configure($searchConfigOptions);
6166
} catch (\Exception $e) {
6267
throw new SetupException($e->getMessage());
6368
}
64-
$this->validateSearchEngine($searchConfigOptions);
69+
$this->validateSearchEngine();
6570
}
6671

6772
/**
6873
* Validate search engine
6974
*
70-
* @param array $config
7175
* @throws ValidationException
7276
*/
73-
public function validateSearchEngine(array $config = [])
77+
public function validateSearchEngine()
7478
{
75-
/** @var Validator $searchValidator */
76-
$searchValidator = $this->objectManager->get(Validator::class);
77-
$validationErrors = $searchValidator->validate($config);
79+
$validationErrors = $this->searchValidator->validate();
7880
if (!empty($validationErrors)) {
7981
throw new ValidationException(__(implode(PHP_EOL, $validationErrors)));
8082
}

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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\Setup\Model;
9+
10+
/**
11+
* Creates instance of Magento\Setup\Model\SearchConfig class
12+
*/
13+
class SearchConfigFactory
14+
{
15+
/**
16+
* @var ObjectManagerProvider
17+
*/
18+
private $objectManagerProvider;
19+
20+
/**
21+
* @param ObjectManagerProvider $objectManagerProvider
22+
*/
23+
public function __construct(ObjectManagerProvider $objectManagerProvider)
24+
{
25+
$this->objectManagerProvider = $objectManagerProvider;
26+
}
27+
28+
/**
29+
* Create SearchConfig instance
30+
*
31+
* @return SearchConfig
32+
* @throws \Magento\Setup\Exception
33+
*/
34+
public function create(): SearchConfig
35+
{
36+
return $this->objectManagerProvider->get()->create(SearchConfig::class);
37+
}
38+
}

‎setup/src/Magento/Setup/Test/Unit/Console/Command/UpgradeCommandTest.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Magento\Setup\Model\Installer;
1313
use Magento\Setup\Model\InstallerFactory;
1414
use Magento\Setup\Model\SearchConfig;
15+
use Magento\Setup\Model\SearchConfigFactory;
1516
use PHPUnit\Framework\MockObject\MockObject;
1617
use Symfony\Component\Console\Tester\CommandTester;
1718

@@ -74,12 +75,16 @@ protected function setUp()
7475
$this->searchConfigMock = $this->getMockBuilder(SearchConfig::class)
7576
->disableOriginalConstructor()
7677
->getMock();
78+
$searchConfigFactoryMock = $this->getMockBuilder(SearchConfigFactory::class)
79+
->disableOriginalConstructor()
80+
->getMock();
81+
$searchConfigFactoryMock->expects($this->once())->method('create')->willReturn($this->searchConfigMock);
7782

7883
$this->upgradeCommand = new UpgradeCommand(
7984
$this->installerFactoryMock,
85+
$searchConfigFactoryMock,
8086
$this->deploymentConfigMock,
81-
$this->appStateMock,
82-
$this->searchConfigMock
87+
$this->appStateMock
8388
);
8489
$this->commandTester = new CommandTester($this->upgradeCommand);
8590
}

0 commit comments

Comments
 (0)