Refactor CronArchive, remove need to specify URL (also did same to CliMulti), remove test skipping in ArchiveWebTest & ArchiveCronTest.

4 files changed, 12 insertions, 90 deletions

diff --git a/core/CliMulti.php b/core/CliMulti.php
index 101e9238e2..e7d59529ea 100644
--- a/core/CliMulti.php
+++ b/core/CliMulti.php
@@ -243,7 +243,7 @@ class CliMulti {
 
         $url      = $this->appendTestmodeParamToUrlIfNeeded($url);
         $query    = UrlHelper::getQueryFromUrl($url, array('pid' => $cmdId));
-        $hostname = UrlHelper::getHostFromUrl($url);
+        $hostname = Url::getHost($checkIfTrusted = false);
         $command  = $this->buildCommand($hostname, $query, $output->getPathToFile());
 
         Log::debug($command);
@@ -252,6 +252,11 @@ class CliMulti {
 
     private function executeNotAsyncHttp($url, Output $output)
     {
+        $url = SettingsPiwik::getPiwikUrl() . $url;
+        if (Config::getInstance()->General['force_ssl'] == 1) {
+            $url = str_replace("http://", "https://", $url);
+        }
+
         try {
             Log::debug("Execute HTTP API request: "  . $url);
             $response = Http::sendHttpRequestBy('curl', $url, $timeout = 0, $userAgent = null, $destinationPath = null, $file = null, $followDepth = 0, $acceptLanguage = false, $this->acceptInvalidSSLCertificate);
@@ -273,7 +278,7 @@ class CliMulti {
 
     private function appendTestmodeParamToUrlIfNeeded($url)
     {
-        $isTestMode = $url && false !== strpos($url, 'tests/PHPUnit/proxy');
+        $isTestMode = class_exists('Piwik_TestingEnvironment');
 
         if ($isTestMode && false === strpos($url, '?')) {
             $url .= "?testmode=1";
diff --git a/core/CronArchive.php b/core/CronArchive.php
index fb2ae11280..2f4eb04e4b 100644
--- a/core/CronArchive.php
+++ b/core/CronArchive.php
@@ -80,7 +80,6 @@ class CronArchive
     private $websites = array();
     private $allWebsites = array();
     private $segments = array();
-    private $piwikUrl = false;
     private $token_auth = false;
     private $validTokenAuths = array();
     private $visitsToday = 0;
@@ -227,7 +226,7 @@ class CronArchive
      * Constructor.
      *
      * @param string|false $piwikUrl The URL to the Piwik installation to initiate archiving for. If `false`,
-     *                               we determine it using the current request information.
+     *                               we determine it using the current request information. TODO: remove this param
      *
      *                               If invoked via the command line, $piwikUrl cannot be false.
      * @param string|null $processNewSegmentsFrom When to archive new segments from. See [General] process_new_segments_from
@@ -238,7 +237,6 @@ class CronArchive
         $this->logger = $logger ?: StaticContainer::get('Psr\Log\LoggerInterface');
         $this->formatter = new Formatter();
 
-        $this->initPiwikHost($piwikUrl);
         $this->initCore();
         $this->initTokenAuth();
 
@@ -266,7 +264,6 @@ class CronArchive
         $this->initStateFromParameters();
 
         $this->logInitInfo();
-        $this->checkPiwikUrlIsValid();
         $this->logArchiveTimeoutInfo();
 
         // record archiving start time
@@ -300,15 +297,6 @@ class CronArchive
         Piwik::postEvent('CronArchive.init.finish', array($this->websites->getInitialSiteIds()));
     }
 
-    public function runScheduledTasksInTrackerMode()
-    {
-        $this->initCore();
-        $this->initTokenAuth();
-        $this->logInitInfo();
-        $this->checkPiwikUrlIsValid();
-        $this->runScheduledTasks();
-    }
-
     /**
      * Main function, runs archiving on all websites with new activity
      */
@@ -597,8 +585,6 @@ class CronArchive
         return $success;
     }
 
-    // TODO: make sure core:archive + web archive is tested when host is for domain (need --dry-run parameter + some output to check for so test won't be super slow)
-
     /**
      * Returns base URL to process reports for the $idSite on a given $period
      */
@@ -1017,50 +1003,6 @@ class CronArchive
         return in_array($token_auth, $this->validTokenAuths);
     }
 
-    /**
-     * @param string|bool $piwikUrl
-     */
-    protected function initPiwikHost($piwikUrl = false)
-    {
-        // If core:archive command run as a web cron, we use the current hostname+path
-        if (empty($piwikUrl)) {
-            if (!empty(self::$url)) {
-                $piwikUrl = self::$url;
-            } else {
-                // example.org/piwik/
-                $piwikUrl = SettingsPiwik::getPiwikUrl();
-            }
-        }
-
-        if (!$piwikUrl) {
-            $this->logFatalErrorUrlExpected();
-        }
-
-        if (!\Piwik\UrlHelper::isLookLikeUrl($piwikUrl)) {
-            // try adding http:// in case it's missing
-            $piwikUrl = "http://" . $piwikUrl;
-        }
-
-        if (!\Piwik\UrlHelper::isLookLikeUrl($piwikUrl)) {
-            $this->logFatalErrorUrlExpected($piwikUrl);
-        }
-
-        // ensure there is a trailing slash
-        if ($piwikUrl[strlen($piwikUrl) - 1] != '/' && !Common::stringEndsWith($piwikUrl, 'index.php')) {
-            $piwikUrl .= '/';
-        }
-
-        if (Config::getInstance()->General['force_ssl'] == 1) {
-            $piwikUrl = str_replace('http://', 'https://', $piwikUrl);
-        }
-
-        if (!Common::stringEndsWith($piwikUrl, 'index.php')) {
-            $piwikUrl .= 'index.php';
-        }
-
-        $this->piwikUrl = $piwikUrl;
-    }
-
     private function updateIdSitesInvalidatedOldReports()
     {
         $store = new SitesToReprocessDistributedList();
@@ -1167,24 +1109,11 @@ class CronArchive
         return $websiteDayHasFinishedSinceLastRun;
     }
 
-    /**
-     *  Test that the specified piwik URL is a valid Piwik endpoint.
-     */
-    protected function checkPiwikUrlIsValid()
-    {
-        $response = $this->request("?module=API&method=API.getDefaultMetricTranslations&format=original&serialize=1");
-        $responseUnserialized = @unserialize($response);
-        if ($response === false
-            || !is_array($responseUnserialized)
-        ) {
-            $this->logFatalError("The Piwik URL {$this->piwikUrl} does not seem to be pointing to a Piwik server. Response was '$response'.");
-        }
-    }
+    // TODO: test archiving w/ curl (no climulti processes) when piwik host is unreachable, is there a useful error message?
 
     private function logInitInfo()
     {
         $this->logSection("INIT");
-        $this->logger->info("Piwik is installed at: {$this->piwikUrl}");
         $this->logger->info("Running Piwik " . Version::VERSION . " as Super User");
     }
 
@@ -1249,13 +1178,6 @@ class CronArchive
         return true;
     }
 
-    private function logFatalErrorUrlExpected($piwikUrl = false)
-    {
-        $this->logFatalError("./console core:archive expects the argument 'url' to be set to your Piwik URL, for example: --url=http://example.org/piwik/"
-            . ($piwikUrl ? "\n '$piwikUrl' supplied" : "")
-            . "\nuse --help for more information");
-    }
-
     private function getVisitsLastPeriodFromApiResponse($stats)
     {
         if (empty($stats)) {
@@ -1528,7 +1450,7 @@ class CronArchive
      */
     private function makeRequestUrl($url)
     {
-        return $this->piwikUrl . $url . self::APPEND_TO_API_REQUEST;
+        return $url . self::APPEND_TO_API_REQUEST;
     }
 
     public static function getSuperUserTokenAuths()
diff --git a/tests/PHPUnit/System/ArchiveCronTest.php b/tests/PHPUnit/System/ArchiveCronTest.php
index 1f162b10df..0413b4b594 100644
--- a/tests/PHPUnit/System/ArchiveCronTest.php
+++ b/tests/PHPUnit/System/ArchiveCronTest.php
@@ -133,9 +133,7 @@ class ArchiveCronTest extends SystemTestCase
         // run the command
         exec($cmd, $output, $result);
         if ($result !== 0 || stripos($result, "error")) {
-            $message = 'This failed once after a lunar eclipse, and it has again randomly failed.';
-            $message .= "\n\narchive cron failed: " . implode("\n", $output) . "\n\ncommand used: $cmd";
-            $this->markTestSkipped($message);
+            $this->fail("archive cron failed: " . implode("\n", $output) . "\n\ncommand used: $cmd");
         }
 
         return $output;
diff --git a/tests/PHPUnit/System/ArchiveWebTest.php b/tests/PHPUnit/System/ArchiveWebTest.php
index defddb58ae..2898613277 100644
--- a/tests/PHPUnit/System/ArchiveWebTest.php
+++ b/tests/PHPUnit/System/ArchiveWebTest.php
@@ -41,10 +41,7 @@ class ArchiveWebTest extends SystemTestCase
 
         // ignore random build issues
         if (empty($output) || strpos($output, \Piwik\CronArchive::NO_ERROR) === false) {
-            $message = "This test has failed. Because it sometimes randomly fails, we skip the test, and ignore this failure.\n";
-            $message .= "If you see this message often, or in every build, please investigate as this should only be a random and rare occurence!\n";
-            $message .= "\n\narchive web failed: " . $output . "\n\nurl used: $url";
-            $this->markTestSkipped($message);
+            $this->fail("archive web failed: " . $output . "\n\nurl used: $url");
         }
 
         if (!empty($urlTmp)) {