refs #4053 coding style, prevent some possible errors

5 files changed, 94 insertions, 61 deletions

diff --git a/plugins/CorePluginsAdmin/Controller.php b/plugins/CorePluginsAdmin/Controller.php
index 4ddf05e4e8..558ab21ab7 100644
--- a/plugins/CorePluginsAdmin/Controller.php
+++ b/plugins/CorePluginsAdmin/Controller.php
@@ -36,7 +36,6 @@ class Controller extends \Piwik\Controller\Admin
         Piwik::checkUserIsSuperUser();
 
         $view = $this->configureView('@CorePluginsAdmin/' . $template);
-        $view->errorMessage = '';
 
         $pluginName = Common::getRequestVar('pluginName', '', 'string');
         $pluginName = strip_tags($pluginName);
@@ -64,7 +63,7 @@ class Controller extends \Piwik\Controller\Admin
             return $view;
         }
 
-        $marketplace  = new MarketplaceApiClient();
+        $marketplace  = new Marketplace();
         $view->plugin = $marketplace->getPluginInfo($pluginName);
 
         return $view;
@@ -94,10 +93,9 @@ class Controller extends \Piwik\Controller\Admin
         }
 
         $view = $this->configureView('@CorePluginsAdmin/pluginDetails');
-        $view->errorMessage = '';
 
         try {
-            $marketplace  = new MarketplaceApiClient();
+            $marketplace  = new Marketplace();
             $view->plugin = $marketplace->getPluginInfo($pluginName);
         } catch (\Exception $e) {
             $view->errorMessage = $e->getMessage();
@@ -188,9 +186,13 @@ class Controller extends \Piwik\Controller\Admin
     protected function configureView($template)
     {
         Piwik::checkUserIsNotAnonymous();
+
         $view = new View($template);
         $this->setBasicVariablesView($view);
         $this->displayWarningIfConfigFileNotWritable($view);
+
+        $view->errorMessage = '';
+
         return $view;
     }
 
diff --git a/plugins/CorePluginsAdmin/Marketplace.php b/plugins/CorePluginsAdmin/Marketplace.php
index 9964200e21..2f52ba6404 100644
--- a/plugins/CorePluginsAdmin/Marketplace.php
+++ b/plugins/CorePluginsAdmin/Marketplace.php
@@ -30,6 +30,13 @@ class Marketplace
         $this->client = new MarketplaceApiClient();
     }
 
+    public function getPluginInfo($pluginName)
+    {
+        $marketplace = new MarketplaceApiClient();
+
+        return $marketplace->getPluginInfo($pluginName);
+    }
+
     public function searchPlugins($query, $sort, $themesOnly)
     {
         if ($themesOnly) {
@@ -51,7 +58,7 @@ class Marketplace
 
     private function hasPluginUpdate($plugin)
     {
-        if (empty($plugin)) {
+        if (empty($plugin->name)) {
             return false;
         }
 
@@ -89,15 +96,14 @@ class Marketplace
         foreach ($pluginsHavingUpdate as $updatePlugin) {
             foreach ($loadedPlugins as $loadedPlugin) {
 
-                if ($loadedPlugin->getPluginName() == $updatePlugin->name) {
+                if (!empty($updatePlugin->name) && $loadedPlugin->getPluginName() == $updatePlugin->name) {
                     $updatePlugin->currentVersion = $loadedPlugin->getVersion();
-                    $updatePlugin->isActivated = $pluginManager->isPluginActivated($updatePlugin->name);
+                    $updatePlugin->isActivated    = $pluginManager->isPluginActivated($updatePlugin->name);
                     break;
-
                 }
             }
-
         }
+
         return $pluginsHavingUpdate;
     }
 
diff --git a/plugins/CorePluginsAdmin/MarketplaceApiClient.php b/plugins/CorePluginsAdmin/MarketplaceApiClient.php
index cd57e58a18..1455b611b7 100644
--- a/plugins/CorePluginsAdmin/MarketplaceApiClient.php
+++ b/plugins/CorePluginsAdmin/MarketplaceApiClient.php
@@ -36,47 +36,6 @@ class MarketplaceApiClient
         $cache->deleteAll();
     }
 
-    private function fetch($action, $params)
-    {
-        $query  = http_build_query($params);
-        $result = $this->getCachedResult($action, $query);
-
-        if (false === $result) {
-            $endpoint = $this->domain . '/api/1.0/';
-            $url      = sprintf('%s%s?%s', $endpoint, $action, $query);
-            $result   = Http::sendHttpRequest($url, 5);
-            $this->cacheResult($action, $query, $result);
-        }
-
-        $result = json_decode($result);
-
-        if (!empty($result->error)) {
-            // TODO create own exception
-            throw new \Exception($result->error);
-        }
-
-        return $result;
-    }
-
-    private function getCachedResult($action, $query)
-    {
-        $cacheKey = $this->getCacheKey($action, $query);
-
-        return $this->cache->get($cacheKey);
-    }
-
-    private function cacheResult($action, $query, $result)
-    {
-        $cacheKey = $this->getCacheKey($action, $query);
-
-        $this->cache->set($cacheKey, $result);
-    }
-
-    private function getCacheKey($action, $query)
-    {
-        return sprintf('api.1.0.%s.%s', str_replace('/', '.', $action), md5($query));
-    }
-
     public function getPluginInfo($name)
     {
         $action = sprintf('plugins/%s/info', $name);
@@ -88,14 +47,12 @@ class MarketplaceApiClient
     {
         $plugin = $this->getPluginInfo($pluginOrThemeName);
 
-        if (empty($plugin)) {
-            // TODO throw exception notExistingPlugin
-            return;
+        if (empty($plugin->versions)) {
+            return false;
         }
 
         $latestVersion = array_pop($plugin->versions);
-
-        $downloadUrl = $latestVersion->download;
+        $downloadUrl   = $latestVersion->download;
 
         $success = Http::fetchRemoteFile($this->domain . $downloadUrl, $target);
 
@@ -104,15 +61,14 @@ class MarketplaceApiClient
 
     /**
      * @param \Piwik\Plugin[] $plugins
+     * @return array|mixed
      */
     public function checkUpdates($plugins)
     {
         $params = array();
 
         foreach ($plugins as $plugin) {
-            $pluginName = $plugin->getPluginName();
-
-            $params[] = array('name' => $pluginName, 'version' => $plugin->getVersion());
+            $params[] = array('name' => $plugin->getPluginName(), 'version' => $plugin->getVersion());
         }
 
         $params = array('plugins' => $params);
@@ -128,15 +84,18 @@ class MarketplaceApiClient
 
     /**
      * @param \Piwik\Plugin[] $plugins
+     * @return array
      */
     public function getInfoOfPluginsHavingUpdate($plugins)
     {
         $hasUpdates = $this->checkUpdates($plugins);
 
         $pluginDetails = array();
+
         foreach ($hasUpdates as $pluginHavingUpdate) {
             $plugin = $this->getPluginInfo($pluginHavingUpdate->name);
-            if (!$plugin->isTheme) {
+
+            if (empty($plugin->isTheme)) {
                 $pluginDetails[] = $plugin;
             }
         }
@@ -146,6 +105,7 @@ class MarketplaceApiClient
 
     /**
      * @param \Piwik\Plugin[] $plugins
+     * @return array
      */
     public function getInfoOfThemesHavingUpdate($plugins)
     {
@@ -154,7 +114,8 @@ class MarketplaceApiClient
         $pluginDetails = array();
         foreach ($hasUpdates as $pluginHavingUpdate) {
             $plugin = $this->getPluginInfo($pluginHavingUpdate->name);
-            if ($plugin->isTheme) {
+
+            if (!empty($plugin->isTheme)) {
                 $pluginDetails[] = $plugin;
             }
         }
@@ -184,4 +145,48 @@ class MarketplaceApiClient
         return array();
     }
 
+    private function fetch($action, $params)
+    {
+        $query  = http_build_query($params);
+        $result = $this->getCachedResult($action, $query);
+
+        if (false === $result) {
+            $endpoint = $this->domain . '/api/1.0/';
+            $url      = sprintf('%s%s?%s', $endpoint, $action, $query);
+            $result   = Http::sendHttpRequest($url, 5);
+            $this->cacheResult($action, $query, $result);
+        }
+
+        $result = json_decode($result);
+
+        if (is_null($result)) {
+            throw new MarketplaceApiException('Failure during communication with marketplace, unable to read response');
+        }
+
+        if (!empty($result->error)) {
+            throw new MarketplaceApiException($result->error);
+        }
+
+        return $result;
+    }
+
+    private function getCachedResult($action, $query)
+    {
+        $cacheKey = $this->getCacheKey($action, $query);
+
+        return $this->cache->get($cacheKey);
+    }
+
+    private function cacheResult($action, $query, $result)
+    {
+        $cacheKey = $this->getCacheKey($action, $query);
+
+        $this->cache->set($cacheKey, $result);
+    }
+
+    private function getCacheKey($action, $query)
+    {
+        return sprintf('api.1.0.%s.%s', str_replace('/', '.', $action), md5($query));
+    }
+
 }
diff --git a/plugins/CorePluginsAdmin/MarketplaceApiException.php b/plugins/CorePluginsAdmin/MarketplaceApiException.php
new file mode 100644
index 0000000000..9286a6b35e
--- /dev/null
+++ b/plugins/CorePluginsAdmin/MarketplaceApiException.php
@@ -0,0 +1,20 @@
+<?php
+/**
+ * Piwik - Open source web analytics
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ *
+ * @category Piwik_Plugins
+ * @package CorePluginsAdmin
+ */
+
+namespace Piwik\Plugins\CorePluginsAdmin;
+
+/**
+ * @package CorePluginsAdmin
+ */
+class MarketplaceApiException extends \Exception
+{
+
+}
diff --git a/plugins/CorePluginsAdmin/templates/installPlugin.twig b/plugins/CorePluginsAdmin/templates/installPlugin.twig
index 8d9d6f1dda..41757a56d6 100644
--- a/plugins/CorePluginsAdmin/templates/installPlugin.twig
+++ b/plugins/CorePluginsAdmin/templates/installPlugin.twig
@@ -4,7 +4,7 @@
 
     <div style="max-width:980px;">
 
-        <h2>Installing {{ plugin.name}}</h2>
+        <h2>Installing {{ plugin.name }}</h2>
 
         {% if errorMessage %}
             <div class="ajaxError" style="">{{ errorMessage }}</div>