Improve site search detection in fragment query (matomo-org#13978)

* Improve site search detection in fragment query * adds some tests * improve fragment handling & add more tests
fdellwing · Feb 24, 2019 · c02e6ce · c02e6ce
1 parent 80cb39a
commit c02e6ce
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 3 deletions.
diff --git a/plugins/Actions/Actions/ActionSiteSearch.php b/plugins/Actions/Actions/ActionSiteSearch.php
@@ -132,8 +132,21 @@ protected function detectSiteSearchFromUrl($website, $parsedUrl)
         $keywordParameters = isset($website['sitesearch_keyword_parameters'])
             ? $website['sitesearch_keyword_parameters']
             : array();
-        $queryString = (!empty($parsedUrl['query']) ? $parsedUrl['query'] : '') . (!empty($parsedUrl['fragment']) ? $separator . $parsedUrl['fragment'] : '');
-        $parametersRaw = UrlHelper::getArrayFromQueryString($queryString);
+        $queryString = !empty($parsedUrl['query']) ? $parsedUrl['query'] : '';
+        $fragment = !empty($parsedUrl['fragment']) ? $parsedUrl['fragment'] : '';
+
+        $parsedFragment = parse_url($fragment);
+
+        // check if fragment contains a separate query (beginning with ?) otherwise assume complete fragment as query
+        if ($fragment && strpos($fragment, '?') !== false && !empty($parsedFragment['query'])) {
+            $fragmentBeforeQuery = !empty($parsedFragment['path']) ? $parsedFragment['path'] : '';
+            $fragmentQuery = $parsedFragment['query'];
+        } else {
+            $fragmentQuery = $fragment;
+            $fragmentBeforeQuery = '';
+        }
+
+        $parametersRaw = UrlHelper::getArrayFromQueryString($queryString.$separator.$fragmentQuery);
 
         // strtolower the parameter names for smooth site search detection
         $parameters = array();
@@ -183,7 +196,14 @@ protected function detectSiteSearchFromUrl($website, $parsedUrl)
                 $parsedUrl['query'] = UrlHelper::getQueryStringWithExcludedParameters(UrlHelper::getArrayFromQueryString($parsedUrl['query']), $parametersToExclude);
             }
             if (isset($parsedUrl['fragment'])) {
-                $parsedUrl['fragment'] = UrlHelper::getQueryStringWithExcludedParameters(UrlHelper::getArrayFromQueryString($parsedUrl['fragment']), $parametersToExclude);
+                $parsedUrl['fragment'] = UrlHelper::getQueryStringWithExcludedParameters(UrlHelper::getArrayFromQueryString($fragmentQuery), $parametersToExclude);
+                if ($fragmentBeforeQuery) {
+                    if ($parsedUrl['fragment']) {
+                        $parsedUrl['fragment'] = $fragmentBeforeQuery.'?'.$parsedUrl['fragment'];
+                    } else {
+                        $parsedUrl['fragment'] = $fragmentBeforeQuery;
+                    }
+                }
             }
         }
         $url = UrlHelper::getParseUrlReverse($parsedUrl);

diff --git a/plugins/Actions/tests/Integration/ActionSiteSearchTest.php b/plugins/Actions/tests/Integration/ActionSiteSearchTest.php
@@ -0,0 +1,74 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link    https://matomo.org
+ * @license https://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\Actions\tests\Unit;
+
+use Piwik\Plugins\Actions\Actions\ActionSiteSearch;
+use Piwik\Tests\Framework\Fixture;
+use Piwik\Tests\Framework\TestCase\IntegrationTestCase;
+use Piwik\Tracker\Request;
+
+/**
+ * @group Actions
+ * @group SiteSearch
+ * @group Plugins
+ */
+class ActionSiteSearchTest extends IntegrationTestCase
+{
+    public function setUp()
+    {
+        parent::setUp();
+
+        Fixture::createWebsite('2010-01-01');
+    }
+
+    /**
+     * @dataProvider getSiteSearchData
+     */
+    public function testSiteSearchDetection($website, $url, $expectedResult)
+    {
+        $action = new ActionSiteSearchPublic(new Request(['idsite' => 1]));
+        $this->assertEquals($expectedResult, $action->detectSiteSearchFromUrl($website, parse_url($url)));
+    }
+
+    public function getSiteSearchData()
+    {
+        $defaultWebsite = [
+            'idSite' => 1,
+            'sitesearch_keyword_parameters' => ['q','k'],
+            'sitesearch_category_parameters' => ['cat','cc']
+        ];
+
+        return [
+            [$defaultWebsite, 'http://example.org/index.htm?q=keyword', ['http://example.org/index.htm', 'keyword', '', false]],
+            [$defaultWebsite, 'http://example.org/index.htm#q=keyword&cat=test', ['http://example.org/index.htm', 'keyword', 'test', false]],
+            [$defaultWebsite, 'http://example.org/index.htm#&q=keyword', ['http://example.org/index.htm', 'keyword', '', false]],
+            [$defaultWebsite, 'http://example.org/index.htm#?cat=test&q=keyword', ['http://example.org/index.htm', 'keyword', 'test', false]],
+            [$defaultWebsite, 'http://example.org/index.htm#?cat=test&q=keyword&otherparam=1', ['http://example.org/index.htm#otherparam=1', 'keyword', 'test', false]],
+            [$defaultWebsite, 'http://example.org/index.htm?cat=test&otherparam=1&q=keyword', ['http://example.org/index.htm?otherparam=1', 'keyword', 'test', false]],
+            [$defaultWebsite, 'http://example.org/index.htm#anchor?cat=test&otherparam=1&q=keyword', ['http://example.org/index.htm#anchor?otherparam=1', 'keyword', 'test', false]],
+            [$defaultWebsite, 'http://example.org/index.htm?cat=test&otherparam=1&q=kw#?q=keyword', ['http://example.org/index.htm?otherparam=1', 'keyword', 'test', false]],
+            [$defaultWebsite, 'http://example.org/index.htm?k=keyword&cc=cat', ['http://example.org/index.htm', 'keyword', 'cat', false]],
+            [$defaultWebsite, '#?q=keyword', ['', 'keyword', '', false]],
+            [$defaultWebsite, 'http://example.org/index.html?a=b#?&&&q=keyword', ['http://example.org/index.html?a=b', 'keyword', '', false]],
+            [$defaultWebsite, 'http://example.org/#&?q=keyword', ['http://example.org/#&', 'keyword', '', false]],
+
+            // some invalid/incorrect urls that aren't detected as site search
+            [$defaultWebsite, 'http://example.org/index.html?a=b#?&&&?q=keyword', false],
+            [$defaultWebsite, 'http://example.org/#&?var=val?q=keyword', false],
+        ];
+    }
+}
+
+class ActionSiteSearchPublic extends ActionSiteSearch
+{
+    public function detectSiteSearchFromUrl($website, $parsedUrl) {
+        return parent::detectSiteSearchFromUrl($website, $parsedUrl);
+    }
+}
+