Skip to content

Commit 05667ed

Browse files
rostyslav-hymonrostyslav-hymon
committed
MC-33151: Performance issue caused by checking if DB tables exist on every call
1 parent d69c242 commit 05667ed

File tree

13 files changed

+172
-146
lines changed

13 files changed

+172
-146
lines changed

‎app/code/Magento/Config/App/Config/Source/RuntimeConfigSource.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@
88
use Magento\Framework\App\Config\ConfigSourceInterface;
99
use Magento\Framework\App\Config\ScopeCodeResolver;
1010
use Magento\Framework\App\Config\ScopeConfigInterface;
11+
use Magento\Framework\App\DeploymentConfig;
12+
use Magento\Framework\App\ObjectManager;
1113
use Magento\Framework\DataObject;
1214
use Magento\Config\Model\ResourceModel\Config\Data\CollectionFactory;
1315
use Magento\Framework\App\Config\Scope\Converter;
16+
use Magento\Framework\DB\Adapter\TableNotFoundException;
1417

1518
/**
1619
* Class for retrieving runtime configuration from database.
@@ -34,20 +37,27 @@ class RuntimeConfigSource implements ConfigSourceInterface
3437
* @var ScopeCodeResolver
3538
*/
3639
private $scopeCodeResolver;
40+
/**
41+
* @var DeploymentConfig
42+
*/
43+
private $deploymentConfig;
3744

3845
/**
3946
* @param CollectionFactory $collectionFactory
4047
* @param ScopeCodeResolver $scopeCodeResolver
4148
* @param Converter $converter
49+
* @param DeploymentConfig|null $deploymentConfig
4250
*/
4351
public function __construct(
4452
CollectionFactory $collectionFactory,
4553
ScopeCodeResolver $scopeCodeResolver,
46-
Converter $converter
54+
Converter $converter,
55+
?DeploymentConfig $deploymentConfig = null
4756
) {
4857
$this->collectionFactory = $collectionFactory;
4958
$this->converter = $converter;
5059
$this->scopeCodeResolver = $scopeCodeResolver;
60+
$this->deploymentConfig = $deploymentConfig ?? ObjectManager::getInstance()->get(DeploymentConfig::class);
5161
}
5262

5363
/**
@@ -59,7 +69,7 @@ public function __construct(
5969
*/
6070
public function get($path = '')
6171
{
62-
$data = new DataObject($this->loadConfig());
72+
$data = new DataObject($this->deploymentConfig->isDbAvailable() ? $this->loadConfig() : []);
6373
return $data->getData($path) ?: [];
6474
}
6575

@@ -75,8 +85,12 @@ private function loadConfig()
7585
{
7686
try {
7787
$collection = $this->collectionFactory->create();
88+
$collection->load();
7889
} catch (\DomainException $e) {
7990
$collection = [];
91+
} catch (TableNotFoundException $exception) {
92+
// database is empty or not setup
93+
$collection = [];
8094
}
8195
$config = [];
8296
foreach ($collection as $item) {

‎app/code/Magento/Config/Test/Unit/App/Config/Source/RuntimeConfigSourceTest.php‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,48 +5,57 @@
55
*/
66
namespace Magento\Config\Test\Unit\App\Config\Source;
77

8+
use ArrayIterator;
89
use Magento\Config\App\Config\Source\RuntimeConfigSource;
10+
use Magento\Config\Model\ResourceModel\Config\Data\Collection;
911
use Magento\Config\Model\ResourceModel\Config\Data\CollectionFactory;
1012
use Magento\Framework\App\Config\Scope\Converter;
1113
use Magento\Framework\App\Config\ScopeCodeResolver;
1214
use Magento\Framework\App\Config\ScopeConfigInterface;
1315
use Magento\Framework\App\Config\Value;
16+
use Magento\Framework\App\DeploymentConfig;
17+
use Magento\Framework\DB\Adapter\TableNotFoundException;
18+
use PHPUnit\Framework\MockObject\MockObject;
19+
use PHPUnit\Framework\TestCase;
1420

1521
/**
1622
* Test Class for retrieving runtime configuration from database.
17-
* @package Magento\Config\Test\Unit\App\Config\Source
1823
*/
19-
class RuntimeConfigSourceTest extends \PHPUnit\Framework\TestCase
24+
class RuntimeConfigSourceTest extends TestCase
2025
{
2126
/**
22-
* @var CollectionFactory|\PHPUnit_Framework_MockObject_MockObject
27+
* @var CollectionFactory|MockObject
2328
*/
2429
private $collectionFactory;
2530

2631
/**
27-
* @var ScopeCodeResolver|\PHPUnit_Framework_MockObject_MockObject
32+
* @var ScopeCodeResolver|MockObject
2833
*/
2934
private $scopeCodeResolver;
3035

3136
/**
32-
* @var Converter|\PHPUnit_Framework_MockObject_MockObject
37+
* @var Converter|MockObject
3338
*/
3439
private $converter;
3540

3641
/**
37-
* @var Value|\PHPUnit_Framework_MockObject_MockObject
42+
* @var Value|MockObject
3843
*/
3944
private $configItem;
4045

4146
/**
42-
* @var Value|\PHPUnit_Framework_MockObject_MockObject
47+
* @var Value|MockObject
4348
*/
4449
private $configItemTwo;
4550

4651
/**
4752
* @var RuntimeConfigSource
4853
*/
4954
private $configSource;
55+
/**
56+
* @var DeploymentConfig|MockObject
57+
*/
58+
private $deploymentConfig;
5059

5160
public function setUp()
5261
{
@@ -68,20 +77,29 @@ public function setUp()
6877
->disableOriginalConstructor()
6978
->setMethods(['getScope', 'getPath', 'getValue', 'getScopeId'])
7079
->getMock();
80+
$this->deploymentConfig = $this->createPartialMock(DeploymentConfig::class, ['isDbAvailable']);
7181
$this->configSource = new RuntimeConfigSource(
7282
$this->collectionFactory,
7383
$this->scopeCodeResolver,
74-
$this->converter
84+
$this->converter,
85+
$this->deploymentConfig
7586
);
7687
}
7788

7889
public function testGet()
7990
{
91+
$this->deploymentConfig->method('isDbAvailable')
92+
->willReturn(true);
93+
$collection = $this->createPartialMock(Collection::class, ['load', 'getIterator']);
94+
$collection->method('load')
95+
->willReturn($collection);
96+
$collection->method('getIterator')
97+
->willReturn(new ArrayIterator([$this->configItem, $this->configItemTwo]));
8098
$scope = 'websites';
8199
$scopeCode = 'myWebsites';
82100
$this->collectionFactory->expects($this->once())
83101
->method('create')
84-
->willReturn([$this->configItem, $this->configItemTwo]);
102+
->willReturn($collection);
85103
$this->configItem->expects($this->exactly(2))
86104
->method('getScope')
87105
->willReturn(ScopeConfigInterface::SCOPE_TYPE_DEFAULT);
@@ -133,4 +151,22 @@ public function testGet()
133151
$this->configSource->get()
134152
);
135153
}
154+
155+
public function testGetWhenDbIsNotAvailable()
156+
{
157+
$this->deploymentConfig->method('isDbAvailable')->willReturn(false);
158+
$this->assertEquals([], $this->configSource->get());
159+
}
160+
161+
public function testGetWhenDbIsEmpty()
162+
{
163+
$this->deploymentConfig->method('isDbAvailable')
164+
->willReturn(true);
165+
$collection = $this->createPartialMock(Collection::class, ['load']);
166+
$collection->method('load')
167+
->willThrowException($this->createMock(TableNotFoundException::class));
168+
$this->collectionFactory->method('create')
169+
->willReturn($collection);
170+
$this->assertEquals([], $this->configSource->get());
171+
}
136172
}

‎app/code/Magento/Store/App/Config/Source/RuntimeConfigSource.php‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Magento\Framework\App\DeploymentConfig;
1010
use Magento\Framework\App\ResourceConnection;
1111
use Magento\Framework\DB\Adapter\AdapterInterface;
12+
use Magento\Framework\DB\Adapter\TableNotFoundException;
1213

1314
/**
1415
* Config source. Retrieve all configuration for scopes from db
@@ -53,15 +54,21 @@ public function __construct(
5354
*/
5455
public function get($path = '')
5556
{
56-
if ($this->canUseDatabase()) {
57-
return [
58-
'websites' => $this->getEntities('store_website', 'code'),
59-
'groups' => $this->getEntities('store_group', 'group_id'),
60-
'stores' => $this->getEntities('store', 'code'),
61-
];
57+
$data = [];
58+
try {
59+
if ($this->canUseDatabase()) {
60+
$data = [
61+
'websites' => $this->getEntities('store_website', 'code'),
62+
'groups' => $this->getEntities('store_group', 'group_id'),
63+
'stores' => $this->getEntities('store', 'code'),
64+
];
65+
}
66+
} catch (TableNotFoundException $exception) {
67+
// database is empty or not setup
68+
$data = [];
6269
}
6370

64-
return [];
71+
return $data;
6572
}
6673

6774
/**
@@ -89,14 +96,13 @@ private function getEntities($table, $keyField)
8996
$data = [];
9097
$tableName = $this->resourceConnection->getTableName($table);
9198
// Check if db table exists before fetch data
92-
if ($this->resourceConnection->getConnection()->isTableExists($tableName)) {
93-
$entities = $this->getConnection()->fetchAll(
94-
$this->getConnection()->select()->from($tableName)
95-
);
9699

97-
foreach ($entities as $entity) {
98-
$data[$entity[$keyField]] = $entity;
99-
}
100+
$entities = $this->getConnection()->fetchAll(
101+
$this->getConnection()->select()->from($tableName)
102+
);
103+
104+
foreach ($entities as $entity) {
105+
$data[$entity[$keyField]] = $entity;
100106
}
101107

102108
return $data;
@@ -109,6 +115,6 @@ private function getEntities($table, $keyField)
109115
*/
110116
private function canUseDatabase()
111117
{
112-
return $this->deploymentConfig->get('db');
118+
return $this->deploymentConfig->isDbAvailable();
113119
}
114120
}

‎app/code/Magento/Store/Model/Config/Processor/Fallback.php‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88
use Magento\Framework\App\Config\Spi\PostProcessorInterface;
99
use Magento\Framework\App\DeploymentConfig;
1010
use Magento\Framework\App\ResourceConnection;
11-
use Magento\Store\Api\Data\StoreInterface;
12-
use Magento\Store\Api\Data\WebsiteInterface;
11+
use Magento\Framework\DB\Adapter\TableNotFoundException;
1312
use Magento\Store\App\Config\Type\Scopes;
1413
use Magento\Store\Model\ResourceModel\Store;
1514
use Magento\Store\Model\ResourceModel\Store\AllStoresCollectionFactory;
@@ -85,13 +84,7 @@ public function __construct(
8584
*/
8685
public function process(array $data)
8786
{
88-
if ($this->deploymentConfig->isDbAvailable()) {//read only from db
89-
$this->storeData = $this->storeResource->readAllStores();
90-
$this->websiteData = $this->websiteResource->readAllWebsites();
91-
} else {
92-
$this->storeData = $this->scopes->get('stores');
93-
$this->websiteData = $this->scopes->get('websites');
94-
}
87+
$this->loadScopes();
9588

9689
$defaultConfig = isset($data['default']) ? $data['default'] : [];
9790
$result = [
@@ -177,4 +170,28 @@ private function getWebsiteConfig(array $websites, $id)
177170
}
178171
return [];
179172
}
173+
174+
/**
175+
* Load config from database.
176+
*
177+
* @return void
178+
*/
179+
private function loadScopes(): void
180+
{
181+
$loaded = false;
182+
try {
183+
if ($this->deploymentConfig->isDbAvailable()) {
184+
$this->storeData = $this->storeResource->readAllStores();
185+
$this->websiteData = $this->websiteResource->readAllWebsites();
186+
$loaded = true;
187+
}
188+
} catch (TableNotFoundException $exception) {
189+
// database is empty or not setup
190+
$loaded = false;
191+
}
192+
if (!$loaded) {
193+
$this->storeData = $this->scopes->get('stores');
194+
$this->websiteData = $this->scopes->get('websites');
195+
}
196+
}
180197
}

‎app/code/Magento/Store/Model/ResourceModel/Store.php‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,16 +166,10 @@ protected function _changeGroup(\Magento\Framework\Model\AbstractModel $model)
166166
*/
167167
public function readAllStores()
168168
{
169-
$stores = [];
170-
if ($this->getConnection()->isTableExists($this->getMainTable())) {
171-
$select = $this->getConnection()
172-
->select()
173-
->from($this->getTable($this->getMainTable()));
174-
175-
$stores = $this->getConnection()->fetchAll($select);
176-
}
177-
178-
return $stores;
169+
$select = $this->getConnection()
170+
->select()
171+
->from($this->getTable($this->getMainTable()));
172+
return $this->getConnection()->fetchAll($select);
179173
}
180174

181175
/**

‎app/code/Magento/Store/Model/ResourceModel/Website.php‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,12 @@ public function readAllWebsites()
4848
{
4949
$websites = [];
5050
$tableName = $this->getMainTable();
51-
if ($this->getConnection()->isTableExists($tableName)) {
52-
$select = $this->getConnection()
53-
->select()
54-
->from($tableName);
51+
$select = $this->getConnection()
52+
->select()
53+
->from($tableName);
5554

56-
foreach ($this->getConnection()->fetchAll($select) as $websiteData) {
57-
$websites[$websiteData['code']] = $websiteData;
58-
}
55+
foreach ($this->getConnection()->fetchAll($select) as $websiteData) {
56+
$websites[$websiteData['code']] = $websiteData;
5957
}
6058

6159
return $websites;

0 commit comments

Comments
 (0)