Skip to content

Commit 07a83bd

Browse files
committed
MAGETWO-63444: Remove object from cache in \Magento\Theme\Model\Theme\ThemeProvider
- ThemeProvider: Fix inheritdoc blocks. Replace docblock with inline comment. - ThemeProviderTest: Self document assertions with message parameter instead of comments.
1 parent 2b679c1 commit 07a83bd

File tree

2 files changed

+44
-23
lines changed

2 files changed

+44
-23
lines changed

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public function __construct(
7171
}
7272

7373
/**
74-
* {@inheritdoc}
74+
* @inheritdoc
7575
*/
7676
public function getThemeByFullPath($fullPath)
7777
{
@@ -100,7 +100,7 @@ public function getThemeByFullPath($fullPath)
100100
}
101101

102102
/**
103-
* {@inheritdoc}
103+
* @inheritdoc
104104
*/
105105
public function getThemeCustomizations(
106106
$area = \Magento\Framework\App\Area::AREA_FRONTEND,
@@ -113,11 +113,7 @@ public function getThemeCustomizations(
113113
}
114114

115115
/**
116-
* Get theme by id
117-
*
118-
* Checks local object cache first, application cache second, and will fall back on creating and loading
119-
* a new Theme object. Loaded Theme is only cached by ID, as virtual themes may have matching base paths.
120-
* {@inheritdoc}
116+
* @inheritdoc
121117
*/
122118
public function getThemeById($themeId)
123119
{
@@ -132,6 +128,7 @@ public function getThemeById($themeId)
132128
$theme = $this->themeFactory->create();
133129
$theme->load($themeId);
134130
if ($theme->getId()) {
131+
// We only cache by ID, as virtual themes may share the same path
135132
$this->saveThemeToCache($theme, 'theme-by-id-' . $themeId);
136133
$this->themes[$themeId] = $theme;
137134
}

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

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -112,10 +112,16 @@ public function testGetByFullPath()
112112
]);
113113
\Magento\Framework\App\ObjectManager::setInstance($objectManagerMock);
114114

115-
// Assertion for first time load
116-
$this->assertSame($this->theme, $this->themeProvider->getThemeByFullPath(self::THEME_PATH));
117-
// Assertion for loading from local cache
118-
$this->assertSame($this->theme, $this->themeProvider->getThemeByFullPath(self::THEME_PATH));
115+
$this->assertSame(
116+
$this->theme,
117+
$this->themeProvider->getThemeByFullPath(self::THEME_PATH),
118+
'Unable to load Theme'
119+
);
120+
$this->assertSame(
121+
$this->theme,
122+
$this->themeProvider->getThemeByFullPath(self::THEME_PATH),
123+
'Unable to load Theme from object cache'
124+
);
119125
}
120126

121127
public function testGetByFullPathWithCache()
@@ -155,10 +161,16 @@ public function testGetByFullPathWithCache()
155161
->with('theme' . self::THEME_PATH)
156162
->willReturn($serializedTheme);
157163

158-
// Assertion for load from cache
159-
$this->assertSame($this->theme, $this->themeProvider->getThemeByFullPath(self::THEME_PATH));
160-
// Assertion for load from object cache
161-
$this->assertSame($this->theme, $this->themeProvider->getThemeByFullPath(self::THEME_PATH));
164+
$this->assertSame(
165+
$this->theme,
166+
$this->themeProvider->getThemeByFullPath(self::THEME_PATH),
167+
'Unable to load Theme from application cache'
168+
);
169+
$this->assertSame(
170+
$this->theme,
171+
$this->themeProvider->getThemeByFullPath(self::THEME_PATH),
172+
'Unable to load Theme from object cache'
173+
);
162174
}
163175

164176
public function testGetById()
@@ -185,10 +197,16 @@ public function testGetById()
185197
->with($themeArray)
186198
->willReturn('{"theme_data":"theme_data"}');
187199

188-
// Assertion for initial load
189-
$this->assertSame($this->theme, $this->themeProvider->getThemeById(self::THEME_ID));
190-
// Assertion for load from object cache
191-
$this->assertSame($this->theme, $this->themeProvider->getThemeById(self::THEME_ID));
200+
$this->assertSame(
201+
$this->theme,
202+
$this->themeProvider->getThemeById(self::THEME_ID),
203+
'Unable to load Theme'
204+
);
205+
$this->assertSame(
206+
$this->theme,
207+
$this->themeProvider->getThemeById(self::THEME_ID),
208+
'Unable to load Theme from object cache'
209+
);
192210
}
193211

194212
public function testGetByIdWithCache()
@@ -211,10 +229,16 @@ public function testGetByIdWithCache()
211229
->method('create')
212230
->willReturn($this->theme);
213231

214-
// Assertion for initial load from cache
215-
$this->assertSame($this->theme, $this->themeProvider->getThemeById(self::THEME_ID));
216-
// Assertion for load from object cache
217-
$this->assertSame($this->theme, $this->themeProvider->getThemeById(self::THEME_ID));
232+
$this->assertSame(
233+
$this->theme,
234+
$this->themeProvider->getThemeById(self::THEME_ID),
235+
'Unable to load Theme from application cache'
236+
);
237+
$this->assertSame(
238+
$this->theme,
239+
$this->themeProvider->getThemeById(self::THEME_ID),
240+
'Unable to load Theme from object cache'
241+
);
218242
}
219243

220244
public function testGetThemeCustomizations()

0 commit comments

Comments
 (0)