From aac92130fa14f7250579ae7b9bdb51d718187623 Mon Sep 17 00:00:00 2001 From: gilles Date: Mon, 4 Apr 2016 11:40:04 +0200 Subject: [PATCH 1/3] Update generator --- EventListener/CanonicalUrlListener.php | 47 ++++++++++---------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/EventListener/CanonicalUrlListener.php b/EventListener/CanonicalUrlListener.php index e6ce402..afd53f7 100755 --- a/EventListener/CanonicalUrlListener.php +++ b/EventListener/CanonicalUrlListener.php @@ -12,16 +12,13 @@ namespace CanonicalUrl\EventListener; -use Propel\Runtime\ActiveQuery\Criteria; use Symfony\Component\EventDispatcher\EventSubscriberInterface; -use Thelia\Core\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Request; use Thelia\Core\HttpFoundation\Session\Session; use Thelia\Model\ConfigQuery; use Thelia\Model\LangQuery; use CanonicalUrl\Event\CanonicalUrlEvent; use CanonicalUrl\Event\CanonicalUrlEvents; -use Thelia\Model\RewritingUrl; -use Thelia\Model\RewritingUrlQuery; /** * Class CanonicalUrlListener @@ -51,46 +48,36 @@ public function __construct(Request $request) */ public function generateUrlCanonical(CanonicalUrlEvent $event) { - $addUrlParameters = true; + if ($event->getUrl() !== null) { + return; + } $parseUrlByCurrentLocale = $this->getParsedUrlByCurrentLocale(); // Be sure to use the proper domain name $canonicalUrl = $parseUrlByCurrentLocale['scheme'] . '://' . $parseUrlByCurrentLocale['host']; - $uri = $this->request->getUri(); - - if (!empty($uri) && false !== $parse = parse_url($uri)) { - // Remove script name from path, preserving a potential subdirectory, e.g. http://somehost.com/mydir/index.php/... - $filePart = preg_replace("!/index(_dev)?\.php!", '', $parse['path']); - - $canonicalUrl .= $filePart; - - // If URL rewriting is enabled, check if our URL is rewritten. - // If it's the case, we will not add parameters to prevent duplicate content. - if (ConfigQuery::isRewritingEnable()) { - $pathList = []; + // preserving a potential subdirectory, e.g. http://somehost.com/mydir/index.php/... + $canonicalUrl .= $this->request->getBaseUrl(); - $filePart = trim($filePart, '/'); + // Remove script name from path, e.g. http://somehost.com/index.php/... + $canonicalUrl = preg_replace("!/index(_dev)?\.php!", '', $canonicalUrl); - while (! empty($filePart)) { - $pathList[] = $filePart; + $path = $this->request->getPathInfo(); - $filePart = preg_replace("!^[^/]+/?!", '', $filePart); - } + if (!empty($path) && $path != "/") { + $canonicalUrl .= $path; - // Check if we have a rewriten URL - $addUrlParameters = 0 === RewritingUrlQuery::create()->filterByUrl($pathList, Criteria::IN)->count(); - } - } - - if ($addUrlParameters) { + $canonicalUrl = rtrim($canonicalUrl, '/'); + /*if (!ConfigQuery::read('allow_slash_ended_uri', false)) { + $canonicalUrl = rtrim($canonicalUrl, '/'); + }*/ + } else { $queryString = $this->request->getQueryString(); if (! empty($queryString)) { - $canonicalUrl .= '?' . $queryString; + $canonicalUrl .= '/?' . $queryString; } - } $event->setUrl($canonicalUrl); From d542c7adc56e9e214f4a5914027b8363c426d0d0 Mon Sep 17 00:00:00 2001 From: gilles Date: Mon, 4 Apr 2016 11:39:48 +0200 Subject: [PATCH 2/3] Add Tests --- Tests/CanonicalUrlTest.php | 228 +++++++++++++++++++++++++++++++++++++ 1 file changed, 228 insertions(+) create mode 100755 Tests/CanonicalUrlTest.php diff --git a/Tests/CanonicalUrlTest.php b/Tests/CanonicalUrlTest.php new file mode 100755 index 0000000..8ed72e6 --- /dev/null +++ b/Tests/CanonicalUrlTest.php @@ -0,0 +1,228 @@ + + */ +class CanonicalUrlTest extends \PHPUnit_Framework_TestCase +{ + public function setUp() + { + /*$config = $this->getMock('Thelia\Model\ConfigQuery'); + + $config->expects($this->any()) + ->method('read') + ->with('allow_slash_ended_uri') + ->will($this->returnValue(true));*/ + } + + public function testRemoveFileIndex() + { + $this->performList('http://myhost.com/test', [ + 'http://myhost.com/index.php/test', + 'http://myhost.com/index.php/test/', + 'http://myhost.com/index.php/test?page=22&list=1', + 'http://myhost.com/index.php/test/?page=22&list=1' + ]); + } + + public function testRemoveFileIndexDev() + { + $this->performList('http://myhost.com/test', [ + 'http://myhost.com/index_dev.php/test', + 'http://myhost.com/index_dev.php/test/', + 'http://myhost.com/index_dev.php/test?page=22&list=1', + 'http://myhost.com/index_dev.php/test/?page=22&list=1' + ], $this->fakeServer( + '/var/www/web/index_dev.php', + '/index_dev.php' + )); + } + + public function testHTTPWithSubDomain() + { + $this->performList('http://mysubdomain.myhost.com/test', [ + 'http://mysubdomain.myhost.com/index.php/test/?page=22&list=1' + ]); + } + + public function testHTTPS() + { + $this->performList('https://myhost.com/test', [ + 'https://myhost.com/index.php/test/?page=22&list=1' + ]); + } + + public function testHTTPSWithSubDomain() + { + $this->performList('https://mysubdomain.myhost.com/test', [ + 'https://mysubdomain.myhost.com/index.php/test/?page=22&list=1' + ]); + } + + public function testHTTPWithSubdirectory() + { + $this->performList('http://myhost.com/web/test', [ + 'http://myhost.com/web/index.php/test', + 'http://myhost.com/web/index.php/test/', + 'http://myhost.com/web/index.php/test?page=22&list=1', + 'http://myhost.com/web/index.php/test?page=22&list=1/' + ], $this->fakeServer( + '/var/www/web/index.php', + '/web/index.php' + )); + + $this->performList('http://myhost.com/web/test', [ + 'http://myhost.com/web/index_dev.php/test', + 'http://myhost.com/web/index_dev.php/test/', + 'http://myhost.com/web/index_dev.php/test?page=22&list=1', + 'http://myhost.com/web/index_dev.php/test?page=22&list=1/' + ], $this->fakeServer( + '/var/www/web/index_dev.php', + '/web/index_dev.php' + )); + } + + public function testHTTPWithMultipleSubdirectory() + { + $this->performList('http://myhost.com/web/web2/web3/test', [ + 'http://myhost.com/web/web2/web3/index.php/test/?page=22&list=1' + ], $this->fakeServer( + '/var/www/web/web2/web3/index.php', + '/web/web2/web3/index.php' + )); + + $this->performList('http://myhost.com/web/web2/web3/test', [ + 'http://myhost.com/web/web2/web3/index_dev.php/test/?page=22&list=1' + ], $this->fakeServer( + '/var/www/web/web2/web3/index_dev.php', + '/web/web2/web3/index_dev.php' + )); + } + + public function testHTTPSWithSubdirectory() + { + $this->performList('https://myhost.com/web/test', [ + 'https://myhost.com/web/index.php/test/?page=22&list=1' + ], $this->fakeServer( + '/var/www/web/index.php', + '/web/index.php' + )); + } + + public function testHTTPSWithMultipleSubdirectory() + { + $this->performList('https://myhost.com/web/web2/web3/test', [ + 'https://myhost.com/web/web2/web3/index.php/test/?page=22&list=1' + ], $this->fakeServer( + '/var/www/web/web2/web3/index.php', + '/web/web2/web3/index.php' + )); + } + + public function testWithNoPath() + { + $this->performList('http://myhost.com/?list=22&page=1', [ + 'http://myhost.com?list=22&page=1', + 'http://myhost.com/?list=22&page=1', + 'http://myhost.com/index.php?list=22&page=1', + 'http://myhost.com/index.php/?list=22&page=1' + ]); + + $this->performList('http://myhost.com/?list=22&page=1', [ + 'http://myhost.com/index_dev.php?list=22&page=1', + 'http://myhost.com/index_dev.php/?list=22&page=1' + ], $this->fakeServer( + '/var/www/web/index_dev.php', + '/index_dev.php' + )); + } + + public function testWithNoPathAndMultipleSubdirectory() + { + $this->performList('http://myhost.com/web/?list=22&page=1', [ + 'http://myhost.com/web/index.php?list=22&page=1', + 'http://myhost.com/web/?list=22&page=1', + 'http://myhost.com/web/index.php?list=22&page=1', + 'http://myhost.com/web/index.php/?list=22&page=1' + ], $this->fakeServer( + '/var/www/web/index.php', + '/web/index.php' + )); + + $this->performList('http://myhost.com/web/?list=22&page=1', [ + 'http://myhost.com/web/index_dev.php?list=22&page=1', + 'http://myhost.com/web/index_dev.php/?list=22&page=1' + ], $this->fakeServer( + '/var/www/web/index_dev.php', + '/web/index_dev.php' + )); + } + + public function testOverrideCanonicalEvent() + { + $canonicalUrlListener = new CanonicalUrlListener(Request::create('https://myhost.com/test')); + + $event = new CanonicalUrlEvent(); + + // override canonical + $canonical = 'http://myscanonical.com'; + $event->setUrl($canonical); + + $canonicalUrlListener->generateUrlCanonical($event); + + $this->assertEquals($canonical, $event->getUrl()); + } + + /** + * @param string $scriptFileName + * @param string $scriptName + * @return array + */ + protected function fakeServer( + $scriptFileName = '/var/www/web/index.php', + $scriptName = '/index.php' + ) { + return [ + 'SCRIPT_FILENAME' => $scriptFileName, + 'SCRIPT_NAME' => $scriptName + ]; + } + + /** + * @param string $canonicalExpected canonical expected + * @param array $list array of uri + * @param array $server + */ + protected function performList($canonicalExpected, array $list, array $server = []) + { + if (empty($server)) { + $server = $this->fakeServer(); + } + + foreach ($list as $uri) { + $canonicalUrlListener = new CanonicalUrlListener( + Request::create($uri, 'GET', [], [], [], $server) + ); + + $event = new CanonicalUrlEvent(); + + $canonicalUrlListener->generateUrlCanonical($event); + + $this->assertEquals($canonicalExpected, $event->getUrl()); + } + } +} From 11b0e002117ca0dd9cd20592ab65131ff74e5836 Mon Sep 17 00:00:00 2001 From: gilles Date: Mon, 11 Apr 2016 11:46:15 +0200 Subject: [PATCH 3/3] Bump version 1.1.0 --- CHANGELOG.md | 5 +++++ Config/module.xml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f19b27..735a7fc 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 1.1.0 + +- Adds the unit tests in the case of a single domain +- Adds the case there is a subfolder + # 1.0.1 - Fix ```installer-name``` in the composer.json file diff --git a/Config/module.xml b/Config/module.xml index 6084614..0db7412 100755 --- a/Config/module.xml +++ b/Config/module.xml @@ -13,7 +13,7 @@ en_US fr_FR - 1.0.3 + 1.1.0 Gilles Bourgeat