Skip to content

Commit 1eb9d76

Browse files
author
Alexander Akimov
authored
Merge pull request #1094 from magento-dragons/PR-2504
[DRAGONS] BugFix
2 parents 7201bc0 + 5995b40 commit 1eb9d76

File tree

7 files changed

+242
-131
lines changed

7 files changed

+242
-131
lines changed

‎app/code/Magento/Config/App/Config/Type/System.php

Lines changed: 143 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Config\App\Config\Type;
78

89
use Magento\Framework\App\Config\ConfigTypeInterface;
@@ -23,7 +24,7 @@ class System implements ConfigTypeInterface
2324
private $source;
2425

2526
/**
26-
* @var DataObject[]
27+
* @var DataObject
2728
*/
2829
private $data;
2930

@@ -64,6 +65,16 @@ class System implements ConfigTypeInterface
6465
*/
6566
private $configType;
6667

68+
/**
69+
* Key name for flag which displays whether configuration is cached or not.
70+
*
71+
* Once configuration is cached additional flag pushed to cache storage
72+
* to be able check cache existence without data load.
73+
*
74+
* @var string
75+
*/
76+
private $cacheExistenceKey;
77+
6778
/**
6879
* @param \Magento\Framework\App\Config\ConfigSourceInterface $source
6980
* @param \Magento\Framework\App\Config\Spi\PostProcessorInterface $postProcessor
@@ -92,6 +103,7 @@ public function __construct(
92103
$this->fallback = $fallback;
93104
$this->serializer = $serializer;
94105
$this->configType = $configType;
106+
$this->cacheExistenceKey = $this->configType . '_CACHE_EXISTS';
95107
}
96108

97109
/**
@@ -102,23 +114,135 @@ public function get($path = '')
102114
if ($path === null) {
103115
$path = '';
104116
}
105-
if (!$this->data) {
106-
$data = $this->cache->load($this->configType);
107-
if (!$data) {
108-
$data = $this->preProcessor->process($this->source->get());
109-
$this->data = new DataObject($data);
110-
$data = $this->fallback->process($data);
111-
$this->data = new DataObject($data);
112-
//Placeholder processing need system config - so we need to save intermediate result
113-
$data = $this->postProcessor->process($data);
114-
$this->data = new DataObject($data);
117+
if ($this->isConfigRead($path)) {
118+
return $this->data->getData($path);
119+
}
120+
121+
if (!empty($path) && $this->isCacheExists()) {
122+
return $this->readFromCache($path);
123+
}
124+
125+
$config = $this->loadConfig();
126+
$this->cacheConfig($config);
127+
$this->data = new DataObject($config);
128+
return $this->data->getData($path);
129+
}
130+
131+
/**
132+
* Check whether configuration is cached
133+
*
134+
* In case configuration cache exists method 'load' returns
135+
* value equal to $this->cacheExistenceKey
136+
*
137+
* @return bool
138+
*/
139+
private function isCacheExists()
140+
{
141+
return $this->cache->load($this->cacheExistenceKey) !== false;
142+
}
143+
144+
/**
145+
* Explode path by '/'(forward slash) separator
146+
*
147+
* In case $path string contains forward slash symbol(/) the $path is exploded and parts array is returned
148+
* In other case empty array is returned
149+
*
150+
* @param string $path
151+
* @return array
152+
*/
153+
private function getPathParts($path)
154+
{
155+
$pathParts = [];
156+
if (strpos($path, '/') !== false) {
157+
$pathParts = explode('/', $path);
158+
}
159+
return $pathParts;
160+
}
161+
162+
/**
163+
* Check whether requested configuration data is read to memory
164+
*
165+
* Because of configuration is cached partially each part can be loaded separately
166+
* Method performs check if corresponding system configuration part is already loaded to memory
167+
* and value can be retrieved directly without cache look up
168+
*
169+
*
170+
* @param string $path
171+
* @return bool
172+
*/
173+
private function isConfigRead($path)
174+
{
175+
$pathParts = $this->getPathParts($path);
176+
return !empty($pathParts) && isset($this->data[$pathParts[0]][$pathParts[1]]);
177+
}
178+
179+
/**
180+
* Load configuration from all the sources
181+
*
182+
* System configuration is loaded in 3 steps performing consecutive calls to
183+
* Pre Processor, Fallback Processor, Post Processor accordingly
184+
*
185+
* @return array
186+
*/
187+
private function loadConfig()
188+
{
189+
$data = $this->preProcessor->process($this->source->get());
190+
$this->data = new DataObject($data);
191+
$data = $this->fallback->process($data);
192+
$this->data = new DataObject($data);
193+
194+
return $this->postProcessor->process($data);
195+
}
196+
197+
/**
198+
*
199+
* Load configuration and caching it by parts.
200+
*
201+
* To be cached configuration is loaded first.
202+
* Then it is cached by parts to minimize memory usage on load.
203+
* Additional flag cached as well to give possibility check cache existence without data load.
204+
*
205+
* @param array $data
206+
* @return void
207+
*/
208+
private function cacheConfig($data)
209+
{
210+
foreach ($data as $scope => $scopeData) {
211+
foreach ($scopeData as $key => $config) {
115212
$this->cache->save(
116-
$this->serializer->serialize($this->data->getData()),
117-
$this->configType,
213+
$this->serializer->serialize($config),
214+
$this->configType . '_' . $scope . $key,
118215
[self::CACHE_TAG]
119216
);
120-
} else {
121-
$this->data = new DataObject($this->serializer->unserialize($data));
217+
}
218+
}
219+
$this->cache->save($this->cacheExistenceKey, $this->cacheExistenceKey, [self::CACHE_TAG]);
220+
}
221+
222+
/**
223+
* Read cached configuration
224+
*
225+
* Read section of system configuration corresponding to requested $path from cache
226+
* Configuration stored to internal property right after load to prevent additional
227+
* requests to cache storage
228+
*
229+
* @param string $path
230+
* @return mixed
231+
*/
232+
private function readFromCache($path)
233+
{
234+
if ($this->data === null) {
235+
$this->data = new DataObject();
236+
}
237+
238+
$result = null;
239+
$pathParts = $this->getPathParts($path);
240+
if (!empty($pathParts)) {
241+
$result = $this->cache->load($this->configType . '_' . $pathParts[0] . $pathParts[1]);
242+
if ($result !== false) {
243+
$readData = $this->data->getData();
244+
$readData[$pathParts[0]][$pathParts[1]] = $this->serializer->unserialize($result);
245+
$this->data->setData($readData);
122246
}
123247
}
124248

@@ -128,6 +252,10 @@ public function get($path = '')
128252
/**
129253
* Clean cache and global variables cache
130254
*
255+
* Next items cleared:
256+
* - Internal property intended to store already loaded configuration data
257+
* - All records in cache storage tagged with CACHE_TAG
258+
*
131259
* @return void
132260
*/
133261
public function clean()

‎app/code/Magento/Config/Test/Unit/App/Config/Type/SystemTest.php

Lines changed: 64 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Config\Test\Unit\App\Config\Type;
78

89
use Magento\Config\App\Config\Type\System;
@@ -68,8 +69,8 @@ public function setUp()
6869
$this->preProcessor = $this->getMockBuilder(PreProcessorInterface::class)
6970
->getMockForAbstractClass();
7071
$this->serializer = $this->getMockBuilder(SerializerInterface::class)
71-
->disableOriginalConstructor()
7272
->getMock();
73+
7374
$this->configType = new System(
7475
$this->source,
7576
$this->postProcessor,
@@ -80,15 +81,31 @@ public function setUp()
8081
);
8182
}
8283

83-
/**
84-
* @param bool $isCached
85-
* @dataProvider getDataProvider
86-
*/
87-
public function testGet($isCached)
84+
public function testGetCached()
85+
{
86+
$path = 'default/dev/unsecure/url';
87+
$url = 'http://magento.test/';
88+
$data = [
89+
'unsecure' => [
90+
'url' => $url
91+
]
92+
];
93+
94+
$this->cache->expects($this->any())
95+
->method('load')
96+
->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev'])
97+
->willReturnOnConsecutiveCalls('1', serialize($data));
98+
$this->serializer->expects($this->once())
99+
->method('unserialize')
100+
->willReturn($data);
101+
$this->assertEquals($url, $this->configType->get($path));
102+
}
103+
104+
public function testGetNotCached()
88105
{
89106
$path = 'default/dev/unsecure/url';
90107
$url = 'http://magento.test/';
91-
$unserializedData = [
108+
$data = [
92109
'default' => [
93110
'dev' => [
94111
'unsecure' => [
@@ -97,58 +114,50 @@ public function testGet($isCached)
97114
]
98115
]
99116
];
100-
$serializedDataString = '{"serialized-data"}';
101117

102-
$this->cache->expects($this->once())
118+
$dataToCache = [
119+
'unsecure' => [
120+
'url' => $url
121+
]
122+
];
123+
$this->cache->expects($this->any())
103124
->method('load')
104-
->with(System::CONFIG_TYPE)
105-
->willReturn($isCached ? $unserializedData : null);
106-
107-
if ($isCached) {
108-
$this->serializer->expects($this->once())
109-
->method('unserialize')
110-
->willReturn($unserializedData);
111-
}
112-
113-
if (!$isCached) {
114-
$this->serializer->expects($this->once())
115-
->method('serialize')
116-
->willReturn($serializedDataString);
117-
$this->source->expects($this->once())
118-
->method('get')
119-
->willReturn($unserializedData);
120-
$this->fallback->expects($this->once())
121-
->method('process')
122-
->with($unserializedData)
123-
->willReturnArgument(0);
124-
$this->preProcessor->expects($this->once())
125-
->method('process')
126-
->with($unserializedData)
127-
->willReturnArgument(0);
128-
$this->postProcessor->expects($this->once())
129-
->method('process')
130-
->with($unserializedData)
131-
->willReturnArgument(0);
132-
$this->cache->expects($this->once())
133-
->method('save')
134-
->with(
135-
$serializedDataString,
136-
System::CONFIG_TYPE,
125+
->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev'])
126+
->willReturnOnConsecutiveCalls(false, false);
127+
128+
$this->serializer->expects($this->once())
129+
->method('serialize')
130+
->willReturn(serialize($dataToCache));
131+
$this->source->expects($this->once())
132+
->method('get')
133+
->willReturn($data);
134+
$this->fallback->expects($this->once())
135+
->method('process')
136+
->with($data)
137+
->willReturnArgument(0);
138+
$this->preProcessor->expects($this->once())
139+
->method('process')
140+
->with($data)
141+
->willReturnArgument(0);
142+
$this->postProcessor->expects($this->once())
143+
->method('process')
144+
->with($data)
145+
->willReturnArgument(0);
146+
$this->cache->expects($this->any())
147+
->method('save')
148+
->withConsecutive(
149+
[
150+
serialize($dataToCache),
151+
'system_defaultdev',
152+
[System::CACHE_TAG]
153+
],
154+
[
155+
'system_CACHE_EXISTS',
156+
'system_CACHE_EXISTS',
137157
[System::CACHE_TAG]
138-
);
139-
}
158+
]
159+
);
140160

141161
$this->assertEquals($url, $this->configType->get($path));
142162
}
143-
144-
/**
145-
* @return array
146-
*/
147-
public function getDataProvider()
148-
{
149-
return [
150-
[true],
151-
[false]
152-
];
153-
}
154163
}

‎app/code/Magento/Swatches/Block/Product/Renderer/Configurable.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,6 @@ protected function getConfigurableOptionsIds(array $attributeData)
392392
/**
393393
* Produce and return block's html output
394394
*
395-
* @codeCoverageIgnore
396395
* @return string
397396
*/
398397
public function toHtml()
@@ -407,7 +406,6 @@ public function toHtml()
407406
/**
408407
* Return HTML code
409408
*
410-
* @codeCoverageIgnore
411409
* @return string
412410
*/
413411
protected function _toHtml()
@@ -429,7 +427,7 @@ protected function getRendererTemplate()
429427
}
430428

431429
/**
432-
* @codeCoverageIgnore
430+
* @deprecated Now is used _toHtml() directly
433431
* @return string
434432
*/
435433
protected function getHtmlOutput()

0 commit comments

Comments
 (0)