Merge pull request #23 from DevDavido/development

Bump to v1.1.0

Author: DevDavido

.github/workflows/tests.yml

@@ -16,7 +16,7 @@ jobs:
             matrix:
                 php-versions: ['7.1']
                 #php-versions: ['7.1', '7.2', '7.3', '7.4', '8.0']
-                matomo-versions: ['3.12.0', '3.13.6']
+                matomo-versions: ['3.12.0', '3.14.0']
                 node-version: [12.x]
                 #node-version: [10.x, 12.x]
 
@@ -57,7 +57,7 @@ jobs:
                   cd matomo
                   git fetch --all
                   git submodule update
-                  git checkout -q tags/${{ matrix.matomo-versions }}
+                  git checkout -f -q tags/${{ matrix.matomo-versions }}
                   [ -d ./tests/travis/.git ] || sh -c "rm -rf ./tests/travis && git clone https://github.com/matomo-org/travis-scripts.git ./tests/travis"
                   cd ./tests/travis
                   git checkout master

CHANGELOG.md

@@ -3,6 +3,16 @@
 - Added: Support for Matomo 4 🎉
 - Updated: Bumped the minimum PHP version to 7.2.5 for this new major plugin version, just as Matomo 4 itself
 
+## 1.1.0
+- Fixed: Regular users cannot login anymore if plugin is activated
+- Fixed: Renamed and fixed option to remove query strings from audited URLs which is now named group URLs and it doesn't throw SQL warnings anymore in certain (edge) cases
+- Improved: Added security information for applied no sandbox mode of Chromium
+- Improved: Increase database connection timeouts for longer running site audits
+- Improved: Increase timeout for (audit) processes to 5 minutes
+- Improved: Throw warning instead error for audited pages with HTTP 403 / 404 response
+- Improved: Added FAQ entry for missing Chromium dependencies
+- Improved: Error message during installation if directory permissions are incorrect
+
 ## 1.0.11
 - Added: Option to remove query strings from audited URLs
 - Improved: Small refactoring in settings

Exceptions/AuditFailedAuthoriseRefusedException.php

@@ -0,0 +1,27 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\PerformanceAudit\Exceptions;
+
+require PIWIK_INCLUDE_PATH . '/plugins/PerformanceAudit/vendor/autoload.php';
+
+use Exception;
+
+class AuditFailedAuthoriseRefusedException extends Exception
+{
+    /**
+     * AuditFailedAuthoriseRefusedException constructor.
+     *
+     * @param string $url
+     * @param null|string $output
+     */
+    public function __construct(string $url, $output = null)
+    {
+        parent::__construct('Audit of ' . $url . ' failed due to refused authorisation: ' . $output);
+    }
+}

Exceptions/AuditFailedNotFoundException.php

@@ -0,0 +1,27 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\PerformanceAudit\Exceptions;
+
+require PIWIK_INCLUDE_PATH . '/plugins/PerformanceAudit/vendor/autoload.php';
+
+use Exception;
+
+class AuditFailedNotFoundException extends Exception
+{
+    /**
+     * AuditFailedNotFoundException constructor.
+     *
+     * @param string $url
+     * @param null|string $output
+     */
+    public function __construct(string $url, $output = null)
+    {
+        parent::__construct('Audit of ' . $url . ' failed due to not found target: ' . $output);
+    }
+}

Exceptions/DependencyNpmMisconfigurationException.php

@@ -0,0 +1,18 @@
+<?php
+/**
+ * Matomo - free/libre analytics platform
+ *
+ * @link https://matomo.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\PerformanceAudit\Exceptions;
+
+require PIWIK_INCLUDE_PATH . '/plugins/PerformanceAudit/vendor/autoload.php';
+
+use RuntimeException;
+
+class DependencyNpmMisconfigurationException extends RuntimeException
+{
+
+}

Lighthouse.php

@@ -11,7 +11,9 @@
 require PIWIK_INCLUDE_PATH . '/plugins/PerformanceAudit/vendor/autoload.php';
 
 use Dzava\Lighthouse\Lighthouse as BaseLighthouse;
+use Piwik\Plugins\PerformanceAudit\Exceptions\AuditFailedAuthoriseRefusedException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\AuditFailedException;
+use Piwik\Plugins\PerformanceAudit\Exceptions\AuditFailedNotFoundException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\DependencyMissingException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\DependencyUnexpectedResultException;
 use UnexpectedValueException;
@@ -38,7 +40,7 @@ public function __construct()
      *
      * @param string $url
      * @return string
-     * @throws AuditFailedException
+     * @throws AuditFailedAuthoriseRefusedException|AuditFailedNotFoundException|AuditFailedException
      */
     public function audit($url)
     {
@@ -49,7 +51,7 @@ public function audit($url)
         $process->setTimeout($this->timeout)->run();
 
         if (!$process->isSuccessful()) {
-            throw new AuditFailedException($url, $process->getErrorOutput());
+            $this->throwAuditException($url, $process->getErrorOutput());
         }
 
         return $process->getOutput();
@@ -145,6 +147,19 @@ public function disableDeviceEmulation()
         return $this;
     }
 
+    /**
+     * Enable audit.
+     *
+     * @param string $audit
+     * @return self
+     */
+    public function enableAudit($audit)
+    {
+        $this->setAudit($audit, true);
+
+        return $this;
+    }
+
     /**
      * Creates the config file used during the audit.
      *
@@ -191,15 +206,21 @@ protected function setAudit($audit, $enable)
     }
 
     /**
-     * Enable audit.
+     * Throw an corresponding exception if audit fails.
      *
-     * @param string $audit
-     * @return self
+     * @param string $url
+     * @param string $errorOutput
+     * @return void
+     * @throws AuditFailedAuthoriseRefusedException|AuditFailedNotFoundException|AuditFailedException
      */
-    public function enableAudit($audit)
+    protected function throwAuditException($url, $errorOutput)
     {
-        $this->setAudit($audit, true);
+        if (stristr($errorOutput, 'Status code: 403')) {
+            throw new AuditFailedAuthoriseRefusedException($url, $errorOutput);
+        } elseif (stristr($errorOutput, 'Status code: 404')) {
+            throw new AuditFailedNotFoundException($url, $errorOutput);
+        }
 
-        return $this;
+        throw new AuditFailedException($url, $errorOutput);
     }
 }

MeasurableSettings.php

@@ -24,7 +24,7 @@ class MeasurableSettings extends BaseMeasurableSettings
     public $emulatedDevice;
 
     /** @var Setting */
-    public $hasUrlsWithoutQueryString;
+    public $hasGroupedUrls;
 
     /** @var Setting */
     public $hasExtraHttpHeader;
@@ -46,11 +46,11 @@ class MeasurableSettings extends BaseMeasurableSettings
      */
     protected function init()
     {
-        Piwik::checkUserHasSomeAdminAccess();
+        Piwik::checkUserHasSomeViewAccess();
 
         $this->runCount = $this->makeRunCountSetting();
         $this->emulatedDevice = $this->makeEmulatedDeviceSetting();
-        $this->hasUrlsWithoutQueryString = $this->makeHasUrlsWithoutQueryStringSetting();
+        $this->hasGroupedUrls = $this->makeHasGroupedUrlsSetting();
         $this->hasExtraHttpHeader = $this->makeHasExtraHttpHeaderSetting();
         $this->extraHttpHeaderKey = $this->makeExtraHttpHeaderKeySetting();
         $this->extraHttpHeaderValue = $this->makeExtraHttpHeaderValueSetting();
@@ -113,11 +113,11 @@ private function makeEmulatedDeviceSetting()
      * @return MeasurableSetting
      * @throws ValidatorException|Exception
      */
-    private function makeHasUrlsWithoutQueryStringSetting()
+    private function makeHasGroupedUrlsSetting()
     {
-        return $this->makeSetting('has_urls_without_query_string', false, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {
-            $field->title = Piwik::translate('PerformanceAudit_Settings_HasUrlsWithoutQueryString_Title');
-            $field->inlineHelp = Piwik::translate('PerformanceAudit_Settings_HasUrlsWithoutQueryString_Help');
+        return $this->makeSetting('has_grouped_urls', false, FieldConfig::TYPE_BOOL, function (FieldConfig $field) {
+            $field->title = Piwik::translate('PerformanceAudit_Settings_HasGroupedUrls_Title');
+            $field->inlineHelp = Piwik::translate('PerformanceAudit_Settings_HasGroupedUrls_Help');
             $field->uiControl = FieldConfig::UI_CONTROL_CHECKBOX;
         });
     }
@@ -218,12 +218,12 @@ public function getEmulatedDevicesList()
     }
 
     /**
-     * Returns if site has URLs without query strings for site.
+     * Returns if site has grouped URLs for site.
      *
      * @return bool
      */
-    public function hasUrlsWithoutQueryString()
+    public function hasGroupedUrls()
     {
-        return $this->getSetting('has_urls_without_query_string')->getValue();
+        return $this->getSetting('has_grouped_urls')->getValue();
     }
 }

NodeDependencyInstaller.php

@@ -12,6 +12,7 @@
 
 use Exception;
 use Piwik\Log;
+use Piwik\Plugins\PerformanceAudit\Exceptions\DependencyNpmMisconfigurationException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\DependencyOfChromeMissingException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\DependencyUnexpectedResultException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\InstallationFailedException;
@@ -121,9 +122,13 @@ private function checkDependency($executableName, $args)
 
         if (!$process->isSuccessful()) {
             $errorOutput = $process->getErrorOutput();
-            throw (stristr($errorOutput, 'libX11-xcb')) ?
-                new DependencyOfChromeMissingException() :
-                new DependencyUnexpectedResultException(ucfirst($executableName) . ' has the following unexpected output: ' . PHP_EOL . $errorOutput);
+            if (stristr($errorOutput, 'libX11-xcb')) {
+                throw new DependencyOfChromeMissingException();
+            } elseif (stristr($errorOutput, 'cache folder contains root-owned files')) {
+                throw new DependencyNpmMisconfigurationException($executableName . ' has the following issue: ' . PHP_EOL . $errorOutput);
+            }
+
+            throw new DependencyUnexpectedResultException(ucfirst($executableName) . ' has the following unexpected output: ' . PHP_EOL . $errorOutput);
         }
 
         return floatval(trim(preg_replace('/[^0-9.]/', '', $process->getOutput())));

Process.php

@@ -33,6 +33,6 @@ public function __construct($command, $env = null)
             ];
         }
 
-        parent::__construct($command, __DIR__, $env, null, 60);
+        parent::__construct($command, __DIR__, $env, null, 300);
     }
 }

README.md

@@ -80,5 +80,7 @@ If you have any questions or inquiries, you can contact `github {at} diskoboss {
 ## Security
 If you discover any security related issues, please contact `github {at} diskoboss {døt} de` instead of using the issue tracker.
 
+Please note that the performance audits will be performed with Chromium which renders Matomo-tracked web pages on your server without the sandbox mode of Chromium. You can find more information about this possible issue on the [lighthouse-cli repository](https://github.com/GoogleChrome/lighthouse-ci/tree/master/docs/recipes/docker-client#--no-sandbox-issues-explained).
+
 ## License
 Licensed under the [GPLv3 License](LICENSE.md).

Tasks.php

@@ -25,7 +25,9 @@
 use Piwik\Piwik;
 use Piwik\Plugin\Tasks as BaseTasks;
 use Piwik\Plugins\PerformanceAudit\Columns\Metrics\Audit;
+use Piwik\Plugins\PerformanceAudit\Exceptions\AuditFailedAuthoriseRefusedException;
 use Piwik\Plugins\PerformanceAudit\Exceptions\AuditFailedException;
+use Piwik\Plugins\PerformanceAudit\Exceptions\AuditFailedNotFoundException;
 use Piwik\Site;
 use Piwik\Tracker\Action;
 use Piwik\Tracker\Db\DbException;
@@ -101,6 +103,9 @@ public function auditSite(int $idSite, bool $debug = false)
             return;
         }
 
+        $this->setDatabaseTimeoutConfiguration();
+        $this->logInfo('Database timeout configuration: ' . json_encode($this->getDatabaseTimeoutConfiguration()));
+
         $this->logInfo('Performance Audit task for site ' . $idSite . ' will be started now');
         try {
             if (!$this->isInDebugMode()) {
@@ -112,8 +117,8 @@ public function auditSite(int $idSite, bool $debug = false)
             $runs = $siteSettings->getRuns();
             $emulatedDevices = $siteSettings->getEmulatedDevicesList();
 
-            if ($siteSettings->hasUrlsWithoutQueryString()) {
-                $this->removeQueryStrings($urls);
+            if ($siteSettings->hasGroupedUrls()) {
+                $this->groupUrlsByPath($urls);
             }
 
             if ($this->isInDebugMode()) {
@@ -295,16 +300,24 @@ private function getPageUrls(int $idSite, string $date)
     }
 
     /**
-     * Remove query string from each url.
+     * Group by path and remove duplicates of URLs
+     * which only differ in their query strings.
      *
      * @param array $urls
      * @return void
      */
-    private function removeQueryStrings(array &$urls)
+    private function groupUrlsByPath(array &$urls)
     {
         foreach ($urls as $key => $url) {
-            $urls[$key] = current(explode('?', $url, 2));
+            $urlBase = current(explode('?', $url, 2));
+            // Remove any URLs which differ only by query string
+            // by setting URL base as key and assign actual URL as value
+            $urls[$urlBase] = $url;
+            // Remove old entry by key
+            unset($urls[$key]);
         }
+        // Reset keys
+        $urls = array_values($urls);
 
         $this->removeUrlDuplicates($urls);
     }
@@ -320,6 +333,30 @@ private function removeUrlDuplicates(array &$urls)
         $urls = array_values(array_unique($urls, SORT_STRING));
     }
 
+    /**
+     * Set timeout configuration of the current database connection.
+     *
+     * @return void
+     * @throws Exception
+     */
+    private function setDatabaseTimeoutConfiguration()
+    {
+        // Set timeouts to maximum 1 week
+        Db::get()->exec('SET SESSION wait_timeout=604800;');
+        Db::get()->exec('SET SESSION interactive_timeout=604800;');
+    }
+
+    /**
+     * Return timeout configuration of the database.
+     *
+     * @return array
+     * @throws DbException
+     */
+    private function getDatabaseTimeoutConfiguration()
+    {
+        return Db::get()->fetchAll('SHOW VARIABLES LIKE "%timeout%"');
+    }
+
     /**
      * Perform audits for every combination of urls,
      * emulated devices and amount of runs.
@@ -355,6 +392,8 @@ private function performAudits(int $idSite, array $urls, array $emulatedDevices,
                             ))
                             ->setEmulatedDevice($emulatedDevice)
                             ->audit($url);
+                    } catch (AuditFailedAuthoriseRefusedException | AuditFailedNotFoundException $exception) {
+                        $this->logWarning($exception->getMessage());
                     } catch (AuditFailedException $exception) {
                         $this->logError($exception->getMessage());
                     }
@@ -560,6 +599,11 @@ private function storeResultsInDatabase(int $idSite, array $results)
         foreach ($siteResult as $url => $emulatedDevices) {
             foreach ($emulatedDevices as $emulatedDevice => $metrics) {
                 foreach ($metrics as $key => $values) {
+                    // Skip URL without an entry in lookup table
+                    if (!isset($actionIdLookupTable[$url])) {
+                        $this->logWarning('Entry for the following hashed URL in lookup table is missing: ' . $url);
+                        continue;
+                    }
                     [$min, $median, $max] = $values;
 
                     $result = Db::get()->query('