Skip to content

Commit 9292f35

Browse files
author
Oleksii Korshenko
committed
MAGETWO-70255: Fix trailing slash used in url rewrites #10043
- Merge Pull Request #10043 from ihor-sviziev/magento2:patch-6 - Merged commits: 1. c4b5874 2. 0ee7c9c 3. bddb74c 4. 30c2c79 5. fe77c14 6. ac41ad0
2 parents 1e19c93 + ac41ad0 commit 9292f35

File tree

3 files changed

+341
-5
lines changed

3 files changed

+341
-5
lines changed

‎app/code/Magento/UrlRewrite/Controller/Router.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ protected function redirect($request, $url, $code)
129129
protected function getRewrite($requestPath, $storeId)
130130
{
131131
return $this->urlFinder->findOneByData([
132-
UrlRewrite::REQUEST_PATH => trim($requestPath, '/'),
132+
UrlRewrite::REQUEST_PATH => ltrim($requestPath, '/'),
133133
UrlRewrite::STORE_ID => $storeId,
134134
]);
135135
}

‎app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
*/
66
namespace Magento\UrlRewrite\Model\Storage;
77

8+
use Magento\Framework\Api\DataObjectHelper;
89
use Magento\Framework\App\ResourceConnection;
10+
use Magento\UrlRewrite\Model\OptionProvider;
911
use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
1012
use Magento\UrlRewrite\Service\V1\Data\UrlRewriteFactory;
11-
use Magento\Framework\Api\DataObjectHelper;
1213

1314
class DbStorage extends AbstractStorage
1415
{
@@ -78,6 +79,54 @@ protected function doFindAllByData($data)
7879
*/
7980
protected function doFindOneByData($data)
8081
{
82+
if (is_array($data)
83+
&& array_key_exists(UrlRewrite::REQUEST_PATH, $data)
84+
&& is_string($data[UrlRewrite::REQUEST_PATH])
85+
) {
86+
$result = null;
87+
88+
$requestPath = $data[UrlRewrite::REQUEST_PATH];
89+
90+
$data[UrlRewrite::REQUEST_PATH] = [
91+
rtrim($requestPath, '/'),
92+
rtrim($requestPath, '/') . '/',
93+
];
94+
95+
$resultsFromDb = $this->connection->fetchAll($this->prepareSelect($data));
96+
97+
if (count($resultsFromDb) === 1) {
98+
$resultFromDb = current($resultsFromDb);
99+
$redirectTypes = [OptionProvider::TEMPORARY, OptionProvider::PERMANENT];
100+
101+
// If request path matches the DB value or it's redirect - we can return result from DB
102+
$canReturnResultFromDb = ($resultFromDb[UrlRewrite::REQUEST_PATH] === $requestPath
103+
|| in_array((int)$resultFromDb[UrlRewrite::REDIRECT_TYPE], $redirectTypes, true));
104+
105+
// Otherwise return 301 redirect to request path from DB results
106+
$result = $canReturnResultFromDb ? $resultFromDb : [
107+
UrlRewrite::ENTITY_TYPE => 'custom',
108+
UrlRewrite::ENTITY_ID => '0',
109+
UrlRewrite::REQUEST_PATH => $requestPath,
110+
UrlRewrite::TARGET_PATH => $resultFromDb[UrlRewrite::REQUEST_PATH],
111+
UrlRewrite::REDIRECT_TYPE => OptionProvider::PERMANENT,
112+
UrlRewrite::STORE_ID => $resultFromDb[UrlRewrite::STORE_ID],
113+
UrlRewrite::DESCRIPTION => null,
114+
UrlRewrite::IS_AUTOGENERATED => '0',
115+
UrlRewrite::METADATA => null,
116+
];
117+
} else {
118+
// If we have 2 results - return the row that matches request path
119+
foreach ($resultsFromDb as $resultFromDb) {
120+
if ($resultFromDb[UrlRewrite::REQUEST_PATH] === $requestPath) {
121+
$result = $resultFromDb;
122+
break;
123+
}
124+
}
125+
}
126+
127+
return $result;
128+
}
129+
81130
return $this->connection->fetchRow($this->prepareSelect($data));
82131
}
83132

‎app/code/Magento/UrlRewrite/Test/Unit/Model/Storage/DbStorageTest.php

Lines changed: 290 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,8 @@
88

99
namespace Magento\UrlRewrite\Test\Unit\Model\Storage;
1010

11-
use \Magento\UrlRewrite\Model\Storage\DbStorage;
12-
13-
use Magento\Framework\App\ResourceConnection;
1411
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
12+
use Magento\UrlRewrite\Model\Storage\DbStorage;
1513
use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
1614

1715
class DbStorageTest extends \PHPUnit_Framework_TestCase
@@ -139,6 +137,8 @@ public function testFindOneByData()
139137
->with($this->select)
140138
->will($this->returnValue(['row1']));
141139

140+
$this->connectionMock->expects($this->never())->method('fetchAll');
141+
142142
$this->dataObjectHelper->expects($this->at(0))
143143
->method('populateWithArray')
144144
->with(['urlRewrite1'], ['row1'], \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class)
@@ -151,6 +151,293 @@ public function testFindOneByData()
151151
$this->assertEquals(['urlRewrite1'], $this->storage->findOneByData($data));
152152
}
153153

154+
public function testFindOneByDataWithRequestPath()
155+
{
156+
$origRequestPath = 'page-one';
157+
$data = [
158+
'col1' => 'val1',
159+
'col2' => 'val2',
160+
UrlRewrite::REQUEST_PATH => $origRequestPath,
161+
];
162+
163+
$this->select->expects($this->at(1))
164+
->method('where')
165+
->with('col1 IN (?)', 'val1');
166+
167+
$this->select->expects($this->at(2))
168+
->method('where')
169+
->with('col2 IN (?)', 'val2');
170+
171+
$this->select->expects($this->at(3))
172+
->method('where')
173+
->with('request_path IN (?)', [$origRequestPath, $origRequestPath . '/']);
174+
175+
$this->connectionMock->expects($this->any())
176+
->method('quoteIdentifier')
177+
->will($this->returnArgument(0));
178+
179+
$this->connectionMock->expects($this->never())
180+
->method('fetchRow');
181+
182+
$urlRewriteRowInDb = [
183+
UrlRewrite::REQUEST_PATH => $origRequestPath,
184+
UrlRewrite::REDIRECT_TYPE => 0,
185+
];
186+
187+
$this->connectionMock->expects($this->once())
188+
->method('fetchAll')
189+
->with($this->select)
190+
->will($this->returnValue([$urlRewriteRowInDb]));
191+
192+
$this->dataObjectHelper->expects($this->at(0))
193+
->method('populateWithArray')
194+
->with(['urlRewrite1'], $urlRewriteRowInDb, \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class)
195+
->will($this->returnSelf());
196+
197+
$this->urlRewriteFactory->expects($this->at(0))
198+
->method('create')
199+
->will($this->returnValue(['urlRewrite1']));
200+
201+
$this->assertEquals(['urlRewrite1'], $this->storage->findOneByData($data));
202+
}
203+
204+
public function testFindOneByDataWithRequestPathIsDifferent()
205+
{
206+
$origRequestPath = 'page-one';
207+
$data = [
208+
'col1' => 'val1',
209+
'col2' => 'val2',
210+
UrlRewrite::REQUEST_PATH => $origRequestPath,
211+
];
212+
213+
$this->select->expects($this->at(1))
214+
->method('where')
215+
->with('col1 IN (?)', 'val1');
216+
217+
$this->select->expects($this->at(2))
218+
->method('where')
219+
->with('col2 IN (?)', 'val2');
220+
221+
$this->select->expects($this->at(3))
222+
->method('where')
223+
->with('request_path IN (?)', [$origRequestPath, $origRequestPath . '/']);
224+
225+
$this->connectionMock->expects($this->any())
226+
->method('quoteIdentifier')
227+
->will($this->returnArgument(0));
228+
229+
$this->connectionMock->expects($this->never())
230+
->method('fetchRow');
231+
232+
$urlRewriteRowInDb = [
233+
UrlRewrite::REQUEST_PATH => $origRequestPath . '/',
234+
UrlRewrite::REDIRECT_TYPE => 0,
235+
UrlRewrite::STORE_ID => 1,
236+
];
237+
238+
$this->connectionMock->expects($this->once())
239+
->method('fetchAll')
240+
->with($this->select)
241+
->will($this->returnValue([$urlRewriteRowInDb]));
242+
243+
$urlRewriteRedirect = [
244+
'request_path' => $origRequestPath,
245+
'redirect_type' => 301,
246+
'store_id' => 1,
247+
'entity_type' => 'custom',
248+
'entity_id' => '0',
249+
'target_path' => $origRequestPath . '/',
250+
'description' => null,
251+
'is_autogenerated' => '0',
252+
'metadata' => null,
253+
];
254+
255+
$this->dataObjectHelper->expects($this->at(0))
256+
->method('populateWithArray')
257+
->with(['urlRewrite1'], $urlRewriteRedirect, \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class)
258+
->will($this->returnSelf());
259+
260+
$this->urlRewriteFactory->expects($this->at(0))
261+
->method('create')
262+
->will($this->returnValue(['urlRewrite1']));
263+
264+
$this->assertEquals(['urlRewrite1'], $this->storage->findOneByData($data));
265+
}
266+
267+
public function testFindOneByDataWithRequestPathIsDifferent2()
268+
{
269+
$origRequestPath = 'page-one/';
270+
$data = [
271+
'col1' => 'val1',
272+
'col2' => 'val2',
273+
UrlRewrite::REQUEST_PATH => $origRequestPath,
274+
];
275+
276+
$this->select->expects($this->at(1))
277+
->method('where')
278+
->with('col1 IN (?)', 'val1');
279+
280+
$this->select->expects($this->at(2))
281+
->method('where')
282+
->with('col2 IN (?)', 'val2');
283+
284+
$this->select->expects($this->at(3))
285+
->method('where')
286+
->with('request_path IN (?)', [rtrim($origRequestPath, '/'), rtrim($origRequestPath, '/') . '/']);
287+
288+
$this->connectionMock->expects($this->any())
289+
->method('quoteIdentifier')
290+
->will($this->returnArgument(0));
291+
292+
$this->connectionMock->expects($this->never())
293+
->method('fetchRow');
294+
295+
$urlRewriteRowInDb = [
296+
UrlRewrite::REQUEST_PATH => rtrim($origRequestPath, '/'),
297+
UrlRewrite::REDIRECT_TYPE => 0,
298+
UrlRewrite::STORE_ID => 1,
299+
];
300+
301+
$this->connectionMock->expects($this->once())
302+
->method('fetchAll')
303+
->with($this->select)
304+
->will($this->returnValue([$urlRewriteRowInDb]));
305+
306+
$urlRewriteRedirect = [
307+
'request_path' => $origRequestPath,
308+
'redirect_type' => 301,
309+
'store_id' => 1,
310+
'entity_type' => 'custom',
311+
'entity_id' => '0',
312+
'target_path' => rtrim($origRequestPath, '/'),
313+
'description' => null,
314+
'is_autogenerated' => '0',
315+
'metadata' => null,
316+
];
317+
318+
$this->dataObjectHelper->expects($this->at(0))
319+
->method('populateWithArray')
320+
->with(['urlRewrite1'], $urlRewriteRedirect, \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class)
321+
->will($this->returnSelf());
322+
323+
$this->urlRewriteFactory->expects($this->at(0))
324+
->method('create')
325+
->will($this->returnValue(['urlRewrite1']));
326+
327+
$this->assertEquals(['urlRewrite1'], $this->storage->findOneByData($data));
328+
}
329+
330+
public function testFindOneByDataWithRequestPathIsRedirect()
331+
{
332+
$origRequestPath = 'page-one';
333+
$data = [
334+
'col1' => 'val1',
335+
'col2' => 'val2',
336+
UrlRewrite::REQUEST_PATH => $origRequestPath,
337+
];
338+
339+
$this->select->expects($this->at(1))
340+
->method('where')
341+
->with('col1 IN (?)', 'val1');
342+
343+
$this->select->expects($this->at(2))
344+
->method('where')
345+
->with('col2 IN (?)', 'val2');
346+
347+
$this->select->expects($this->at(3))
348+
->method('where')
349+
->with('request_path IN (?)', [$origRequestPath, $origRequestPath . '/']);
350+
351+
$this->connectionMock->expects($this->any())
352+
->method('quoteIdentifier')
353+
->will($this->returnArgument(0));
354+
355+
$this->connectionMock->expects($this->never())
356+
->method('fetchRow');
357+
358+
$urlRewriteRowInDb = [
359+
UrlRewrite::REQUEST_PATH => $origRequestPath . '/',
360+
UrlRewrite::TARGET_PATH => 'page-A/',
361+
UrlRewrite::REDIRECT_TYPE => 301,
362+
UrlRewrite::STORE_ID => 1,
363+
];
364+
365+
$this->connectionMock->expects($this->once())
366+
->method('fetchAll')
367+
->with($this->select)
368+
->will($this->returnValue([$urlRewriteRowInDb]));
369+
370+
$this->dataObjectHelper->expects($this->at(0))
371+
->method('populateWithArray')
372+
->with(['urlRewrite1'], $urlRewriteRowInDb, \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class)
373+
->will($this->returnSelf());
374+
375+
$this->urlRewriteFactory->expects($this->at(0))
376+
->method('create')
377+
->will($this->returnValue(['urlRewrite1']));
378+
379+
$this->assertEquals(['urlRewrite1'], $this->storage->findOneByData($data));
380+
}
381+
382+
public function testFindOneByDataWithRequestPathTwoResults()
383+
{
384+
$origRequestPath = 'page-one';
385+
$data = [
386+
'col1' => 'val1',
387+
'col2' => 'val2',
388+
UrlRewrite::REQUEST_PATH => $origRequestPath,
389+
];
390+
391+
$this->select->expects($this->at(1))
392+
->method('where')
393+
->with('col1 IN (?)', 'val1');
394+
395+
$this->select->expects($this->at(2))
396+
->method('where')
397+
->with('col2 IN (?)', 'val2');
398+
399+
$this->select->expects($this->at(3))
400+
->method('where')
401+
->with('request_path IN (?)', [$origRequestPath, $origRequestPath . '/']);
402+
403+
$this->connectionMock->expects($this->any())
404+
->method('quoteIdentifier')
405+
->will($this->returnArgument(0));
406+
407+
$this->connectionMock->expects($this->never())
408+
->method('fetchRow');
409+
410+
$urlRewriteRowInDb = [
411+
UrlRewrite::REQUEST_PATH => $origRequestPath . '/',
412+
UrlRewrite::TARGET_PATH => 'page-A/',
413+
UrlRewrite::REDIRECT_TYPE => 301,
414+
UrlRewrite::STORE_ID => 1,
415+
];
416+
417+
$urlRewriteRowInDb2 = [
418+
UrlRewrite::REQUEST_PATH => $origRequestPath,
419+
UrlRewrite::TARGET_PATH => 'page-B/',
420+
UrlRewrite::REDIRECT_TYPE => 301,
421+
UrlRewrite::STORE_ID => 1,
422+
];
423+
424+
$this->connectionMock->expects($this->once())
425+
->method('fetchAll')
426+
->with($this->select)
427+
->will($this->returnValue([$urlRewriteRowInDb, $urlRewriteRowInDb2]));
428+
429+
$this->dataObjectHelper->expects($this->at(0))
430+
->method('populateWithArray')
431+
->with(['urlRewrite1'], $urlRewriteRowInDb2, \Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class)
432+
->will($this->returnSelf());
433+
434+
$this->urlRewriteFactory->expects($this->at(0))
435+
->method('create')
436+
->will($this->returnValue(['urlRewrite1']));
437+
438+
$this->assertEquals(['urlRewrite1'], $this->storage->findOneByData($data));
439+
}
440+
154441
public function testReplace()
155442
{
156443
$urlFirst = $this->getMock(\Magento\UrlRewrite\Service\V1\Data\UrlRewrite::class, [], [], '', false);

0 commit comments

Comments
 (0)