Skip to content

Commit

Permalink
When dispatch is disabled via a constant, it should not be dispatched (
Browse files Browse the repository at this point in the history
…#14034)

* When dispatch is disabled via a constant, it should not be dispatched

Eg when Matomo is not installed, it would still dispatch the request in https://github.com/matomo-org/matomo/blob/3.8.1-b1/plugins/Installation/Installation.php#L111 even when `PIWIK_ENABLE_DISPATCH` is disabled. 

Will set it to WIP for now as I'm not sure if we want to have this actually merged or not.

* Update Installation.php

* Use correct exception class

* throw exception if one is given

* adding a test

* fix tests

* trying to fix test
  • Loading branch information
tsteur authored and diosmosis committed Mar 13, 2019
1 parent def7436 commit 7393e13
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 2 deletions.
2 changes: 1 addition & 1 deletion core/Application/Kernel/EnvironmentValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private function checkConfigFileExists($path, $startInstaller = false)

$message = $this->getSpecificMessageWhetherFileExistsOrNot($path);

$exception = new \Exception($message);
$exception = new NotYetInstalledException($message);

if ($startInstaller) {
$this->startInstallation($exception);
Expand Down
7 changes: 6 additions & 1 deletion plugins/Installation/Installation.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,13 @@ public function dispatch($exception = null)

$action = Common::getRequestVar('action', 'welcome', 'string');

if ($this->isAllowedAction($action)) {
if ($this->isAllowedAction($action) && (!defined('PIWIK_ENABLE_DISPATCH') || PIWIK_ENABLE_DISPATCH)) {
echo FrontController::getInstance()->dispatch('Installation', $action, array($message));
} elseif (defined('PIWIK_ENABLE_DISPATCH') && !PIWIK_ENABLE_DISPATCH) {
if ($exception && $exception instanceof \Exception) {
throw $exception;
}
return;
} else {
Piwik::exitWithErrorMessage($this->getMessageToInviteUserToInstallPiwik($message));
}
Expand Down
7 changes: 7 additions & 0 deletions plugins/Installation/tests/System/APITest.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ public function test_shouldReturnValidApiResponse_IfWrongDbInfo_formatJSON()
$this->assertContains('Access denied', $data);
}

public function test_shouldReturnEmptyResultWhenNotInstalledAndDispatchIsDisabled()
{
$url = Fixture::getTestRootUrl() . 'nodispatchnotinstalled.php';
$response = $this->sendHttpRequest($url);
$this->assertSame('', $response['data']);
}

private function getUrl()
{
return Fixture::getRootUrl() . 'tests/PHPUnit/proxy/index.php?module=API&method=API.getPiwikVersion';
Expand Down
20 changes: 20 additions & 0 deletions tests/PHPUnit/proxy/nodispatchnotinstalled.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php
/**
* Proxy to index.php, but will use the Test DB
* Used by tests/PHPUnit/System/ImportLogsTest.php and tests/PHPUnit/System/UITest.php
*/

use Piwik\Application\Environment;
use Piwik\Tests\Framework\TestingEnvironmentManipulator;
use Piwik\Tests\Framework\TestingEnvironmentVariables;

define('PIWIK_ENABLE_DISPATCH', false);

require realpath(dirname(__FILE__)) . "/includes.php";
$testEnvironment = new TestingEnvironmentVariables();
$testEnvironment->configFileLocal = PIWIK_INCLUDE_PATH . "tmp/test.config.ini.php";
$testEnvironment->save();

Environment::setGlobalEnvironmentManipulator(new TestingEnvironmentManipulator($testEnvironment));

include PIWIK_INCLUDE_PATH . '/index.php';

0 comments on commit 7393e13

Please sign in to comment.