refs #4610 simplified the fallback if async is not supported, tests and fixes for output class

core/CliMulti.php

@@ -23,10 +23,6 @@ class CliMulti {
 
     public function request($piwikUrls)
     {
-        if (!$this->supportsAsync() || 1 == count($piwikUrls)) {
-            return $this->requestNonAsyncFallback($piwikUrls);
-        }
-
         $this->start($piwikUrls);
 
         while (!$this->isFinished()) {
@@ -46,9 +42,11 @@ class CliMulti {
             $pid    = $cmdId . $index . '_cli_multi_pid';
             $output = $cmdId . $index . '_cli_multi_output';
 
-            $additionalParams = array('output' => $output, 'pid' => $pid);
+            $params   = array('output' => $output, 'pid' => $pid);
+            $command  = $this->buildCommand($url, $params);
+            $appendix = $this->supportsAsync() ? ' &' : '';
 
-            shell_exec($this->buildCommand($url, $additionalParams) . '&');
+            shell_exec($command . $appendix);
 
             $this->pids[]    = new Lock($pid);
             $this->outputs[] = new Output($output);
@@ -64,7 +62,7 @@ class CliMulti {
             $query .= '&' . http_build_query($additionalParams);
         }
 
-        $command = 'php ' . PIWIK_INCLUDE_PATH . '/core/wrapper.php -- ' . escapeshellarg($query);
+        $command = 'php ' . PIWIK_INCLUDE_PATH . '/core/CliMulti/run.php -- ' . escapeshellarg($query);
 
         return $command;
     }
@@ -74,13 +72,8 @@ class CliMulti {
         $response = array();
 
         foreach ($this->outputs as $index => $output) {
-            $url = $urls[$index];
-
-            if ($output->exists()) {
-                $response[$url] = $output->get();
-            } else {
-                $response[$url] = null;
-            }
+            $url            = $urls[$index];
+            $response[$url] = $output->get();
         }
 
         return $response;
@@ -118,15 +111,4 @@ class CliMulti {
         }
     }
 
-    private function requestNonAsyncFallback($piwikUrls)
-    {
-        $response = array();
-
-        foreach ($piwikUrls as $piwikUrl) {
-            $response[$piwikUrl] = shell_exec($this->buildCommand($piwikUrl));
-        }
-
-        return $response;
-    }
-
 }

core/CliMulti/Output.php

@@ -34,12 +34,16 @@ class Output {
 
     public function get()
     {
+        if (!$this->exists()) {
+            return null;
+        }
+
         return file_get_contents($this->tmpFile);
     }
 
     public function destroy()
     {
-       Filesystem::deleteIfExists($this->tmpFile);
+        Filesystem::deleteIfExists($this->tmpFile);
     }
 
 }

core/wrapper.php → core/CliMulti/run.php

@@ -10,7 +10,7 @@
  */
 
 if (!defined('PIWIK_INCLUDE_PATH')) {
-    define('PIWIK_INCLUDE_PATH', realpath(dirname(__FILE__) . "/.."));
+    define('PIWIK_INCLUDE_PATH', realpath(dirname(__FILE__) . "/../.."));
 }
 
 require_once PIWIK_INCLUDE_PATH . '/core/Common.php';

tests/PHPUnit/Core/CliMulti/OutputTest.php

+<?php
+/**
+ * Piwik - Open source web analytics
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+use Piwik\CliMulti\Output;
+
+/**
+ * Class OutputTest
+ * @group Core
+ */
+class OutputTest extends PHPUnit_Framework_TestCase
+{
+
+    /**
+     * @var Output
+     */
+    private $output;
+
+    public function setUp()
+    {
+        $this->output = new Output('myid');
+    }
+
+    public function tearDown()
+    {
+        $this->output->destroy();
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage The given output id has an invalid format
+     */
+    public function test_construct_shouldFail_IfInvalidOutputIdGiven()
+    {
+        new Output('../../');
+    }
+
+    public function test_exists_ShouldReturnsFalse_IfNothingWrittenYet()
+    {
+        $this->assertFalse($this->output->exists());
+    }
+
+    public function test_exists_ShouldReturnTrue_IfSomethingIsWritten()
+    {
+        $this->output->write('test');
+
+        $this->assertTrue($this->output->exists());
+
+        $this->output->destroy();
+
+        $this->assertFalse($this->output->exists());
+    }
+
+    public function test_get_shouldReturnNull_IfNothingWritten()
+    {
+        $this->assertNull($this->output->get());
+    }
+
+    public function test_get_write_shouldReturnTheActualOutput_IfExists()
+    {
+        $anyContent = 'My Actual Content';
+        $this->output->write($anyContent);
+
+        $this->assertEquals($anyContent, $this->output->get());
+    }
+
+    public function test_write_shouldNotAppend_IfWriteIsCalledTwice()
+    {
+        $anyContent = 'My Actual Content';
+        $this->output->write($anyContent);
+        $this->output->write($anyContent);
+
+        $this->assertEquals($anyContent, $this->output->get());
+    }
+
+    public function test_write_shouldSaveAnEmptyString_IfContentIsNull()
+    {
+        $this->output->write(null);
+
+        $this->assertTrue($this->output->exists());
+        $this->assertEquals('', $this->output->get());
+    }
+
+    public function test_destroy_ShouldRemove_IfAnyOutputIsWritten()
+    {
+        $this->output->write('test');
+
+        $this->assertTrue($this->output->exists());
+
+        $this->output->destroy();
+
+        $this->assertFalse($this->output->exists());
+        $this->assertNull($this->output->get());
+    }
+
+    public function test_destroy_ShouldNotFail_IfNothingIsWritten()
+    {
+        $this->output->destroy();
+
+        $this->assertFalse($this->output->exists());
+        $this->assertNull($this->output->get());
+    }
+
+    public function test_twoDifferentOutputHandles_ShouldWriteInDifferentFiles()
+    {
+        $output1 = new Output('id1');
+        $output2 = new Output('id2');
+
+        // cleanup possible earlier failed test runs
+        $output1->destroy();
+        $output2->destroy();
+
+        $output1->write('test 1');
+        $this->assertTrue($output1->exists());
+        $this->assertFalse($output2->exists());
+        $output2->write('test 2');
+
+        $this->assertEquals('test 1', $output1->get());
+        $this->assertEquals('test 2', $output2->get());
+
+        $output1->destroy();
+        $this->assertFalse($output1->exists());
+        $this->assertTrue($output2->exists());
+        $output2->destroy();
+
+        $this->assertFalse($output2->exists());
+    }
+}
\ No newline at end of file