Skip to content

Commit 5105e65

Browse files
tjwiebelltjwiebell
committed
MAGETWO-63444: Remove object from cache in \Magento\Theme\Model\Theme\ThemeProvider
- ThemeProvider: Remove extraneous documentation - Theme: Fix variable name. Remove extraneous documentation - ThemeProviderTest: Modify existing tests to include process flow changes we made. Add two additional tests for call flow from the application cache. - ThemeTest: Refactor test to utilize data provider
1 parent df3201c commit 5105e65

File tree

4 files changed

+194
-90
lines changed

4 files changed

+194
-90
lines changed

‎app/code/Magento/Theme/Model/Theme.php

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ class Theme extends \Magento\Framework\Model\AbstractModel implements ThemeInter
8282
/**
8383
* @var ThemeFactory
8484
*/
85-
private $_themeModelFactory;
85+
private $themeModelFactory;
8686

8787
/**
8888
* @var ThemeInterface[]
@@ -124,8 +124,8 @@ public function __construct(
124124
$this->_imageFactory = $imageFactory;
125125
$this->_validator = $validator;
126126
$this->_customFactory = $customizationFactory;
127+
$this->themeModelFactory = $themeModelFactory ?: ObjectManager::getInstance()->get(ThemeFactory::class);
127128
$this->addData(['type' => self::TYPE_VIRTUAL]);
128-
$this->_themeModelFactory = $themeModelFactory ?: ObjectManager::getInstance()->get(ThemeFactory::class);
129129
}
130130

131131
/**
@@ -385,21 +385,7 @@ public function getInheritedThemes()
385385
}
386386

387387
/**
388-
* @param int $modelId
389-
* @param null $field
390-
* @return $this
391-
*/
392-
public function load($modelId, $field = null)
393-
{
394-
return parent::load($modelId, $field);
395-
}
396-
397-
/**
398-
* Method to convert Theme object to array. Additionally handles nested Themes that could be stored
399-
* under the parent_theme and inherited_themes keys
400-
*
401-
* @param array $keys
402-
* @return array
388+
* @inheritdoc
403389
*/
404390
public function toArray(array $keys = [])
405391
{
@@ -418,8 +404,7 @@ public function toArray(array $keys = [])
418404
}
419405

420406
/**
421-
* Method to populate Theme object from an array. Additionally handles nested Themes that could be
422-
* stored under the parent_theme and inherited_themes keys
407+
* Method to populate Theme object from an array
423408
*
424409
* @param array $data
425410
* @return Theme
@@ -441,10 +426,12 @@ public function populateFromArray(array $data)
441426
}
442427

443428
/**
444-
* @return ThemeInterface|null
429+
* Method to get a new Theme model from factory
430+
*
431+
* @return \Magento\Theme\Model\Theme
445432
*/
446433
private function getThemeInstance()
447434
{
448-
return $this->_themeModelFactory->create();
435+
return $this->themeModelFactory->create();
449436
}
450437
}

‎app/code/Magento/Theme/Model/Theme/ThemeProvider.php

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ public function getThemeByFullPath($fullPath)
8383
return $this->getThemeList()->getThemeByFullPath($fullPath);
8484
}
8585

86-
/** @var $themeCollection \Magento\Theme\Model\ResourceModel\Theme\Collection */
8786
$theme = $this->loadThemeFromCache('theme'. $fullPath);
8887
if ($theme) {
8988
$this->themes[$fullPath] = $theme;
@@ -126,7 +125,6 @@ public function getThemeById($themeId)
126125
$this->themes[$themeId] = $theme;
127126
return $theme;
128127
}
129-
/** @var $theme \Magento\Framework\View\Design\ThemeInterface */
130128
$theme = $this->themeFactory->create();
131129
$theme->load($themeId);
132130
if ($theme->getId()) {
@@ -137,8 +135,7 @@ public function getThemeById($themeId)
137135
}
138136

139137
/**
140-
* Method to load Theme model from cache. If located, the theme will be unserialized and converted
141-
* back to an object, via the populateFromArray method of the Theme object.
138+
* Method to load Theme model from cache
142139
*
143140
* @param string $cacheId
144141
* @return \Magento\Theme\Model\Theme|null
@@ -148,7 +145,6 @@ private function loadThemeFromCache($cacheId)
148145
$themeData = $this->cache->load($cacheId);
149146
if ($themeData) {
150147
$themeData = $this->serializer->unserialize($themeData);
151-
/** @var $theme \Magento\Theme\Model\Theme */
152148
$theme = $this->themeFactory->create()->populateFromArray($themeData);
153149
return $theme;
154150
}
@@ -157,8 +153,7 @@ private function loadThemeFromCache($cacheId)
157153
}
158154

159155
/**
160-
* Method to save Theme model to the cache. Model will be converted to an array via the toArray method
161-
* of the Theme object, then serialized.
156+
* Method to save Theme model to the cache
162157
*
163158
* @param \Magento\Theme\Model\Theme $theme
164159
* @param string $cacheId

‎app/code/Magento/Theme/Test/Unit/Model/Theme/ThemeProviderTest.php

Lines changed: 134 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -35,24 +35,33 @@ public function testGetByFullPath()
3535
false
3636
);
3737
$collectionMock = $this->getMock(\Magento\Theme\Model\ResourceModel\Theme\Collection::class, [], [], '', false);
38-
$theme = $this->getMock(\Magento\Framework\View\Design\ThemeInterface::class, [], [], '', false);
39-
$collectionMock->expects(
40-
$this->once()
41-
)->method(
42-
'getThemeByFullPath'
43-
)->with(
44-
$path
45-
)->will(
46-
$this->returnValue($theme)
47-
);
38+
39+
$theme = $this->getMock(\Magento\Theme\Model\Theme::class, [], [], '', false);
40+
$theme->expects($this->exactly(2))
41+
->method('getId')
42+
->willReturn(1);
43+
$theme->expects($this->exactly(2))
44+
->method('toArray')
45+
->willReturn(['theme_data' => 'theme_data']);
46+
47+
$collectionMock->expects($this->once())
48+
->method('getThemeByFullPath')
49+
->with($path)
50+
->willReturn($theme);
4851
$collectionFactory->expects($this->once())->method('create')->will($this->returnValue($collectionMock));
4952
$themeFactory = $this->getMock(\Magento\Theme\Model\ThemeFactory::class, [], [], '', false);
53+
$serializerMock = $this->getMock(\Magento\Framework\Serialize\Serializer\Json::class);
54+
$serializerMock->expects($this->exactly(2))
55+
->method('serialize')
56+
->with(['theme_data' => 'theme_data'])
57+
->willReturn('{"theme_data":"theme_data"}');
5058

5159
$themeProvider = $this->objectManager->getObject(
5260
\Magento\Theme\Model\Theme\ThemeProvider::class,
5361
[
5462
'collectionFactory' => $collectionFactory,
55-
'themeFactory' => $themeFactory
63+
'themeFactory' => $themeFactory,
64+
'serializer' => $serializerMock
5665
]
5766
);
5867

@@ -71,23 +80,77 @@ public function testGetByFullPath()
7180
]);
7281
\Magento\Framework\App\ObjectManager::setInstance($objectManagerMock);
7382

83+
// assertion for first time load
84+
$this->assertSame($theme, $themeProvider->getThemeByFullPath($path));
85+
// assertion for loading from local cache
86+
$this->assertSame($theme, $themeProvider->getThemeByFullPath($path));
87+
}
88+
89+
public function testGetByFullPathWithCache()
90+
{
91+
$path = 'frontend/Magento/luma';
92+
$deploymentConfig = $this->getMockBuilder(\Magento\Framework\App\DeploymentConfig::class)
93+
->disableOriginalConstructor()
94+
->getMock();
95+
$deploymentConfig->expects($this->once())
96+
->method('isDbAvailable')
97+
->willReturn(true);
98+
99+
$objectManagerMock = $this->getMock(\Magento\Framework\ObjectManagerInterface::class);
100+
$objectManagerMock->expects($this->any())
101+
->method('get')
102+
->willReturnMap([
103+
[\Magento\Framework\App\DeploymentConfig::class, $deploymentConfig],
104+
]);
105+
\Magento\Framework\App\ObjectManager::setInstance($objectManagerMock);
106+
107+
$theme = $this->getMock(\Magento\Theme\Model\Theme::class, [], [], '', false);
108+
$theme->expects($this->once())
109+
->method('populateFromArray')
110+
->willReturnSelf();
111+
$themeFactory = $this->getMock(\Magento\Theme\Model\ThemeFactory::class, [], [], '', false);
112+
$themeFactory->expects($this->once())
113+
->method('create')
114+
->willReturn($theme);
115+
116+
$serializerMock = $this->getMock(\Magento\Framework\Serialize\Serializer\Json::class);
117+
$serializerMock->expects($this->once())
118+
->method('unserialize')
119+
->with('{"theme_data":"theme_data"}')
120+
->willReturn(['theme_data' => 'theme_data']);
121+
122+
$cacheMock = $this->getMockBuilder(\Magento\Framework\App\CacheInterface::class)
123+
->disableOriginalConstructor()
124+
->getMock();
125+
$cacheMock->expects($this->once())
126+
->method('load')
127+
->with('theme' . $path)
128+
->willReturn('{"theme_data":"theme_data"}');
129+
130+
$themeProvider = $this->objectManager->getObject(
131+
\Magento\Theme\Model\Theme\ThemeProvider::class,
132+
[
133+
'themeFactory' => $themeFactory,
134+
'cache' => $cacheMock,
135+
'serializer' => $serializerMock
136+
]
137+
);
138+
139+
// assertion for load from cache
140+
$this->assertSame($theme, $themeProvider->getThemeByFullPath($path));
141+
// assertion for load from object cache
74142
$this->assertSame($theme, $themeProvider->getThemeByFullPath($path));
75143
}
76144

77145
public function testGetById()
78146
{
79147
$themeId = 755;
80-
$collectionFactory = $this->getMock(
81-
\Magento\Theme\Model\ResourceModel\Theme\CollectionFactory::class,
82-
[],
83-
[],
84-
'',
85-
false
86-
);
87148
$theme = $this->getMock(\Magento\Theme\Model\Theme::class, [], [], '', false);
88149
$theme->expects($this->once())->method('load')->with($themeId)->will($this->returnSelf());
89150
$theme->expects($this->once())->method('getId')->will($this->returnValue(1));
90-
$theme->expects($this->never())->method('__sleep');
151+
$theme->expects($this->once())
152+
->method('toArray')
153+
->willReturn(['theme_data' => 'theme_data']);
91154

92155
$themeFactory = $this->getMock(\Magento\Theme\Model\ThemeFactory::class, ['create'], [], '', false);
93156
$themeFactory->expects($this->once())->method('create')->will($this->returnValue($theme));
@@ -100,15 +163,65 @@ public function testGetById()
100163
->with('theme-by-id-' . $themeId)
101164
->willReturn(false);
102165

166+
$serializerMock = $this->getMock(\Magento\Framework\Serialize\Serializer\Json::class);
167+
$serializerMock->expects($this->once())
168+
->method('serialize')
169+
->with(['theme_data' => 'theme_data'])
170+
->willReturn('{"theme_data":"theme_data"}');
171+
103172
$themeProvider = $this->objectManager->getObject(
104173
\Magento\Theme\Model\Theme\ThemeProvider::class,
105174
[
106-
'collectionFactory' => $collectionFactory,
107175
'themeFactory' => $themeFactory,
108-
'cache' => $cacheMock
176+
'cache' => $cacheMock,
177+
'serializer' => $serializerMock
109178
]
110179
);
111180

181+
// assertion for initial load
182+
$this->assertSame($theme, $themeProvider->getThemeById($themeId));
183+
// assertion for load from object cache
184+
$this->assertSame($theme, $themeProvider->getThemeById($themeId));
185+
}
186+
187+
public function testGetByIdWithCache()
188+
{
189+
$themeId = 755;
190+
$theme = $this->getMock(\Magento\Theme\Model\Theme::class, [], [], '', false);
191+
$theme->expects($this->once())
192+
->method('populateFromArray')
193+
->with(['theme_data' => 'theme_data'])
194+
->willReturnSelf();
195+
$cacheMock = $this->getMockBuilder(\Magento\Framework\App\CacheInterface::class)
196+
->disableOriginalConstructor()
197+
->getMock();
198+
$cacheMock->expects($this->once())
199+
->method('load')
200+
->with('theme-by-id-' . $themeId)
201+
->willReturn('{"theme_data":"theme_data"}');
202+
$serializerMock = $this->getMock(\Magento\Framework\Serialize\Serializer\Json::class);
203+
$serializerMock->expects($this->once())
204+
->method('unserialize')
205+
->with('{"theme_data":"theme_data"}')
206+
->willReturn(['theme_data' => 'theme_data']);
207+
$themeFactory = $this->getMock(\Magento\Theme\Model\ThemeFactory::class, ['create'], [], '', false);
208+
$themeFactory->expects($this->once())->method('create')->will($this->returnValue($theme));
209+
$themeFactory->expects($this->once())
210+
->method('create')
211+
->willReturn($theme);
212+
213+
$themeProvider = $this->objectManager->getObject(
214+
\Magento\Theme\Model\Theme\ThemeProvider::class,
215+
[
216+
'themeFactory' => $themeFactory,
217+
'cache' => $cacheMock,
218+
'serializer' => $serializerMock
219+
]
220+
);
221+
222+
// assertion for initial load from cache
223+
$this->assertSame($theme, $themeProvider->getThemeById($themeId));
224+
// assertion for load from object cache
112225
$this->assertSame($theme, $themeProvider->getThemeById($themeId));
113226
}
114227

0 commit comments

Comments
 (0)