From b67827bbd2215cefeac5f4d65c23846619c8d223 Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Mon, 23 Sep 2013 02:25:22 +0000
Subject: [PATCH] refs #4053 cleanup code of marketplace integration
---
plugins/CorePluginsAdmin/Controller.php | 35 +++------
plugins/CorePluginsAdmin/Marketplace.php | 24 ++++---
.../CorePluginsAdmin/MarketplaceApiClient.php | 72 +++++++++++++------
plugins/CorePluginsAdmin/PluginInstaller.php | 46 ++++++++----
.../templates/installPlugin.twig | 13 ++--
.../templates/pluginMetadata.twig | 2 +-
.../templates/pluginOverview.twig | 25 +++----
.../CorePluginsAdmin/templates/plugins.twig | 4 +-
.../templates/themeOverview.twig | 27 +++----
.../CorePluginsAdmin/templates/themes.twig | 4 +-
10 files changed, 144 insertions(+), 108 deletions(-)
diff --git a/plugins/CorePluginsAdmin/Controller.php b/plugins/CorePluginsAdmin/Controller.php
index 13ebc522c5..706a16c8d8 100644
--- a/plugins/CorePluginsAdmin/Controller.php
+++ b/plugins/CorePluginsAdmin/Controller.php
@@ -37,13 +37,8 @@ class Controller extends \Piwik\Controller\Admin
$view = $this->configureView('@CorePluginsAdmin/' . $template);
- $pluginName = Common::getRequestVar('pluginName', '', 'string');
- $pluginName = strip_tags($pluginName);
- $nonce = Common::getRequestVar('nonce', '', 'string');
-
- if (empty($pluginName)) {
- throw new \Exception('Plugin parameter is missing');
- }
+ $pluginName = Common::getRequestVar('pluginName', null, 'string');
+ $nonce = Common::getRequestVar('nonce', null, 'string');
$view->plugin = array('name' => $pluginName);
@@ -58,7 +53,7 @@ class Controller extends \Piwik\Controller\Admin
$pluginInstaller = new PluginInstaller($pluginName);
$pluginInstaller->installOrUpdatePluginFromMarketplace();
- } catch (PluginInstallerException $e) {
+ } catch (\Exception $e) {
$view->errorMessage = $e->getMessage();
return $view;
}
@@ -85,12 +80,7 @@ class Controller extends \Piwik\Controller\Admin
public function pluginDetails()
{
- $pluginName = Common::getRequestVar('pluginName', '', 'string');
- $pluginName = strip_tags($pluginName);
-
- if (empty($pluginName)) {
- return;
- }
+ $pluginName = Common::getRequestVar('pluginName', null, 'string');
$view = $this->configureView('@CorePluginsAdmin/pluginDetails');
@@ -107,7 +97,6 @@ class Controller extends \Piwik\Controller\Admin
private function createBrowsePluginsOrThemesView($template, $themesOnly)
{
$query = Common::getRequestVar('query', '', 'string', $_POST);
- $query = strip_tags($query);
$sort = Common::getRequestVar('sort', $this->defaultSortMethod, 'string');
if (!in_array($sort, $this->validSortMethods)) {
@@ -263,13 +252,8 @@ class Controller extends \Piwik\Controller\Admin
{
Piwik::checkUserIsSuperUser();
- $pluginName = Common::getRequestVar('pluginName', '', 'string');
- $pluginName = strip_tags($pluginName);
- $nonce = Common::getRequestVar('nonce', '', 'string');
-
- if (empty($pluginName)) {
- throw new \Exception('Plugin parameter is missing');
- }
+ $pluginName = Common::getRequestVar('pluginName', null, 'string');
+ $nonce = Common::getRequestVar('nonce', null, 'string');
if (!Nonce::verifyNonce('CorePluginsAdmin.activatePlugin', $nonce)) {
throw new \Exception(Piwik_Translate('General_ExceptionNonceMismatch'));
@@ -283,11 +267,12 @@ class Controller extends \Piwik\Controller\Admin
$params = array('activated' => 1, 'pluginName' => $pluginName);
$plugin = PluginsManager::getInstance()->loadPlugin($pluginName);
+ $actionToRedirect = 'plugins';
if ($plugin->isTheme()) {
- $this->redirectToIndex('CorePluginsAdmin', 'themes', null, null, null, $params);
- } else {
- $this->redirectToIndex('CorePluginsAdmin', 'plugins', null, null, null, $params);
+ $actionToRedirect = 'themes';
}
+
+ $this->redirectToIndex('CorePluginsAdmin', $actionToRedirect, null, null, null, $params);
}
}
diff --git a/plugins/CorePluginsAdmin/Marketplace.php b/plugins/CorePluginsAdmin/Marketplace.php
index f2b9e18df5..40dfec5479 100644
--- a/plugins/CorePluginsAdmin/Marketplace.php
+++ b/plugins/CorePluginsAdmin/Marketplace.php
@@ -47,10 +47,10 @@ class Marketplace
$dateFormat = Piwik_Translate('CoreHome_ShortDateFormatWithYear');
- foreach ($plugins as $plugin) {
- $plugin->canBeUpdated = $this->hasPluginUpdate($plugin);
- $plugin->isInstalled = PluginsManager::getInstance()->isPluginLoaded($plugin->name);
- $plugin->lastUpdated = Date::factory($plugin->lastUpdated)->getLocalized($dateFormat);
+ foreach ($plugins as &$plugin) {
+ $plugin['canBeUpdated'] = $this->hasPluginUpdate($plugin);
+ $plugin['isInstalled'] = PluginsManager::getInstance()->isPluginLoaded($plugin['name']);
+ $plugin['lastUpdated'] = Date::factory($plugin['lastUpdated'])->getLocalized($dateFormat);
}
return $plugins;
@@ -58,14 +58,14 @@ class Marketplace
private function hasPluginUpdate($plugin)
{
- if (empty($plugin->name)) {
+ if (empty($plugin['name'])) {
return false;
}
- $pluginsHavingUpdate = $this->getPluginsHavingUpdate($plugin->isTheme);
+ $pluginsHavingUpdate = $this->getPluginsHavingUpdate($plugin['isTheme']);
foreach ($pluginsHavingUpdate as $pluginHavingUpdate) {
- if ($plugin->name == $pluginHavingUpdate->name) {
+ if ($plugin['name'] == $pluginHavingUpdate['name']) {
return true;
}
}
@@ -89,12 +89,14 @@ class Marketplace
$pluginsHavingUpdate = array();
}
- foreach ($pluginsHavingUpdate as $updatePlugin) {
+ foreach ($pluginsHavingUpdate as &$updatePlugin) {
foreach ($loadedPlugins as $loadedPlugin) {
- if (!empty($updatePlugin->name) && $loadedPlugin->getPluginName() == $updatePlugin->name) {
- $updatePlugin->currentVersion = $loadedPlugin->getVersion();
- $updatePlugin->isActivated = $pluginManager->isPluginActivated($updatePlugin->name);
+ if (!empty($updatePlugin['name'])
+ && $loadedPlugin->getPluginName() == $updatePlugin['name']) {
+
+ $updatePlugin['currentVersion'] = $loadedPlugin->getVersion();
+ $updatePlugin['isActivated'] = $pluginManager->isPluginActivated($updatePlugin['name']);
break;
}
}
diff --git a/plugins/CorePluginsAdmin/MarketplaceApiClient.php b/plugins/CorePluginsAdmin/MarketplaceApiClient.php
index 19c8cf636a..dc7b3841a1 100644
--- a/plugins/CorePluginsAdmin/MarketplaceApiClient.php
+++ b/plugins/CorePluginsAdmin/MarketplaceApiClient.php
@@ -11,6 +11,7 @@
namespace Piwik\Plugins\CorePluginsAdmin;
use Piwik\CacheFile;
use Piwik\Http;
+use Piwik\PluginsManager;
/**
*
@@ -18,6 +19,9 @@ use Piwik\Http;
*/
class MarketplaceApiClient
{
+ const CACHE_TIMEOUT_IN_SECONDS = 1200;
+ const HTTP_REQUEST_TIMEOUT = 30;
+
private $domain = 'http://plugins.piwik.org';
/**
@@ -27,7 +31,7 @@ class MarketplaceApiClient
public function __construct()
{
- $this->cache = new CacheFile('marketplace', 1200);
+ $this->cache = new CacheFile('marketplace', self::CACHE_TIMEOUT_IN_SECONDS);
}
public static function clearAllCacheEntries()
@@ -45,16 +49,13 @@ class MarketplaceApiClient
public function download($pluginOrThemeName, $target)
{
- $plugin = $this->getPluginInfo($pluginOrThemeName);
+ $downloadUrl = $this->getDownloadUrl($pluginOrThemeName);
- if (empty($plugin->versions)) {
+ if (empty($downloadUrl)) {
return false;
}
- $latestVersion = array_pop($plugin->versions);
- $downloadUrl = $latestVersion->download;
-
- $success = Http::fetchRemoteFile($this->domain . $downloadUrl, $target);
+ $success = Http::fetchRemoteFile($downloadUrl, $target, 0, static::HTTP_REQUEST_TIMEOUT);
return $success;
}
@@ -68,7 +69,10 @@ class MarketplaceApiClient
$params = array();
foreach ($plugins as $plugin) {
- $params[] = array('name' => $plugin->getPluginName(), 'version' => $plugin->getVersion());
+ $pluginName = $plugin->getPluginName();
+ if (!PluginsManager::getInstance()->isPluginBundledWithCore($pluginName)) {
+ $params[] = array('name' => $plugin->getPluginName(), 'version' => $plugin->getVersion());
+ }
}
$params = array('plugins' => $params);
@@ -94,9 +98,9 @@ class MarketplaceApiClient
$pluginDetails = array();
foreach ($hasUpdates as $pluginHavingUpdate) {
- $plugin = $this->getPluginInfo($pluginHavingUpdate->name);
+ $plugin = $this->getPluginInfo($pluginHavingUpdate['name']);
- if (!empty($plugin->isTheme) == $themesOnly) {
+ if (!empty($plugin['isTheme']) == $themesOnly) {
$pluginDetails[] = $plugin;
}
}
@@ -108,8 +112,8 @@ class MarketplaceApiClient
{
$response = $this->fetch('plugins', array('keywords' => $keywords, 'query' => $query, 'sort' => $sort));
- if (!empty($response->plugins)) {
- return $response->plugins;
+ if (!empty($response['plugins'])) {
+ return $response['plugins'];
}
return array();
@@ -119,8 +123,8 @@ class MarketplaceApiClient
{
$response = $this->fetch('themes', array('keywords' => $keywords, 'query' => $query, 'sort' => $sort));
- if (!empty($response->plugins)) {
- return $response->plugins;
+ if (!empty($response['plugins'])) {
+ return $response['plugins'];
}
return array();
@@ -128,24 +132,27 @@ class MarketplaceApiClient
private function fetch($action, $params)
{
+ ksort($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);
- }
+ $response = Http::sendHttpRequest($url, static::HTTP_REQUEST_TIMEOUT);
+ $result = json_decode($response, true);
- $result = json_decode($result);
+ if (is_null($result)) {
+ $message = sprintf('There was an error reading the response from the Marketplace: %s. Please try again later.',
+ substr($response, 0, 50));
+ throw new MarketplaceApiException($message);
+ }
- 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']);
+ }
- if (!empty($result->error)) {
- throw new MarketplaceApiException($result->error);
+ $this->cacheResult($action, $query, $result);
}
return $result;
@@ -170,4 +177,23 @@ class MarketplaceApiClient
return sprintf('api.1.0.%s.%s', str_replace('/', '.', $action), md5($query));
}
+ /**
+ * @param $pluginOrThemeName
+ * @throws MarketplaceApiException
+ * @return string
+ */
+ public function getDownloadUrl($pluginOrThemeName)
+ {
+ $plugin = $this->getPluginInfo($pluginOrThemeName);
+
+ if (empty($plugin['versions'])) {
+ throw new MarketplaceApiException('Plugin has no versions.');
+ }
+
+ $latestVersion = array_pop($plugin['versions']);
+ $downloadUrl = $latestVersion['download'];
+
+ return $this->domain . $downloadUrl;
+ }
+
}
diff --git a/plugins/CorePluginsAdmin/PluginInstaller.php b/plugins/CorePluginsAdmin/PluginInstaller.php
index e77b79757d..cd664b3aef 100644
--- a/plugins/CorePluginsAdmin/PluginInstaller.php
+++ b/plugins/CorePluginsAdmin/PluginInstaller.php
@@ -35,6 +35,7 @@ class PluginInstaller
$tmpPluginFolder = PIWIK_USER_PATH . self::PATH_TO_DOWNLOAD . $this->pluginName;
$this->makeSureFoldersAreWritable();
+ $this->makeSurePluginNameIsValid();
$this->downloadPluginFromMarketplace($tmpPluginZip);
$this->extractPluginFiles($tmpPluginZip, $tmpPluginFolder);
$this->makeSurePluginJsonExists($tmpPluginFolder);
@@ -53,21 +54,21 @@ class PluginInstaller
{
$this->removeFileIfExists($pluginZipTargetFile);
- try {
- $marketplace = new MarketplaceApiClient();
- $pluginDetails = $marketplace->getPluginInfo($this->pluginName);
- } catch (\Exception $e) {
- throw new PluginInstallerException($e->getMessage());
- }
-
- if (empty($pluginDetails)) {
- throw new PluginInstallerException('A plugin with this name does not exist');
- }
+ $marketplace = new MarketplaceApiClient();
try {
$marketplace->download($this->pluginName, $pluginZipTargetFile);
} catch (\Exception $e) {
- throw new PluginInstallerException('Failed to download plugin: ' . $e->getMessage());
+
+ try {
+ $downloadUrl = $marketplace->getDownloadUrl($this->pluginName);
+ $errorMessage = sprintf('Failed to download plugin from %s: %s', $downloadUrl, $e->getMessage());
+
+ } catch (\Exception $ex) {
+ $errorMessage = sprintf('Failed to download plugin: %s', $e->getMessage());
+ }
+
+ throw new PluginInstallerException($errorMessage);
}
}
@@ -94,7 +95,7 @@ class PluginInstaller
private function makeSurePluginJsonExists($tmpPluginFolder)
{
if (!file_exists($tmpPluginFolder . DIRECTORY_SEPARATOR . $this->pluginName . DIRECTORY_SEPARATOR . 'plugin.json')) {
- throw new PluginInstallerException('Plugin is not valid, missing plugin.json');
+ throw new PluginInstallerException('Plugin is not valid, it is missing the plugin.json file.');
}
}
@@ -112,9 +113,7 @@ class PluginInstaller
*/
private function removeFolderIfExists($pathExtracted)
{
- if (file_exists($pathExtracted)) {
- Filesystem::unlinkRecursive($pathExtracted, true);
- }
+ Filesystem::unlinkRecursive($pathExtracted, true);
}
/**
@@ -127,4 +126,21 @@ class PluginInstaller
}
}
+ /**
+ * @throws PluginInstallerException
+ */
+ private function makeSurePluginNameIsValid()
+ {
+ try {
+ $marketplace = new MarketplaceApiClient();
+ $pluginDetails = $marketplace->getPluginInfo($this->pluginName);
+ } catch (\Exception $e) {
+ throw new PluginInstallerException($e->getMessage());
+ }
+
+ if (empty($pluginDetails)) {
+ throw new PluginInstallerException('This plugin was not found in the Marketplace.');
+ }
+ }
+
}
diff --git a/plugins/CorePluginsAdmin/templates/installPlugin.twig b/plugins/CorePluginsAdmin/templates/installPlugin.twig
index 41757a56d6..9ef95329dd 100644
--- a/plugins/CorePluginsAdmin/templates/installPlugin.twig
+++ b/plugins/CorePluginsAdmin/templates/installPlugin.twig
@@ -18,30 +18,29 @@
<p>Unzipping theme</p>
- <p>Installing theme</p>
-
<p>You have successfully installed the theme {{ plugin.name }} {{ plugin.latestVersion }}.</p>
<p><strong><a href="{{ linkTo({'action': 'activate', 'pluginName': plugin.name, 'nonce': nonce}) }}">Activate Theme</a></strong>
+ |
+ <a href="{{ linkTo({'action': 'extend'}) }}">Back to Extend Piwik</a></p>
+
{% else %}
<p>Downloading plugin from Marketplace</p>
<p>Unzipping plugin</p>
- <p>Installing plugin</p>
-
<p>You have successfully installed the Plugin {{ plugin.name }} {{ plugin.latestVersion }}.</p>
<p><strong><a href="{{ linkTo({'action': 'activate', 'pluginName': plugin.name, 'nonce': nonce}) }}">Activate Plugin</a></strong>
+ |
+ <a href="{{ linkTo({'action': 'extend'}) }}">Back to Extend Piwik</a></p>
+
{% endif %}
</div>
- |
- <a href="{{ linkTo({'action': 'extend'}) }}">Back to Extend Piwik</a></p>
-
{% endif %}
</div>
diff --git a/plugins/CorePluginsAdmin/templates/pluginMetadata.twig b/plugins/CorePluginsAdmin/templates/pluginMetadata.twig
index f6752d0e64..831dc3b220 100644
--- a/plugins/CorePluginsAdmin/templates/pluginMetadata.twig
+++ b/plugins/CorePluginsAdmin/templates/pluginMetadata.twig
@@ -1,4 +1,4 @@
-<hr class="metadataSeparator">
+<hr class="metadataSeparator"/>
<ul class="metadata">
<li>{{ 'CorePluginsAdmin_Version'|translate }}: <strong>{{ plugin.latestVersion }}</strong></li>
<li class="even">Updated: <strong>{{ plugin.lastUpdated }}</strong></li>
diff --git a/plugins/CorePluginsAdmin/templates/pluginOverview.twig b/plugins/CorePluginsAdmin/templates/pluginOverview.twig
index d5da8de487..39a39b5052 100644
--- a/plugins/CorePluginsAdmin/templates/pluginOverview.twig
+++ b/plugins/CorePluginsAdmin/templates/pluginOverview.twig
@@ -1,21 +1,22 @@
-{% if not isSuperUser %}
-{% elseif plugin.canBeUpdated %}
- <a class="update"
- href="{{ linkTo({'action':'updatePlugin', 'pluginName': plugin.name, 'nonce': updateNonce}) }}"
- >Update</a>
-{% elseif plugin.isInstalled %}
- <span class="install">Installed</span>
-{% else %}
- <a href="{{ linkTo({'action': 'installPlugin', 'pluginName': plugin.name, 'nonce': installNonce}) }}"
- class="install">Install</a>
+{% if isSuperUser %}
+ {% if plugin.canBeUpdated %}
+ <a class="update"
+ href="{{ linkTo({'action':'updatePlugin', 'pluginName': plugin.name, 'nonce': updateNonce}) }}"
+ >Update</a>
+ {% elseif plugin.isInstalled %}
+ <span class="install">Installed</span>
+ {% else %}
+ <a href="{{ linkTo({'action': 'installPlugin', 'pluginName': plugin.name, 'nonce': installNonce}) }}"
+ class="install">Install</a>
+ {% endif %}
{% endif %}
-<h3 class="header">
+<h3 class="header" title="Click for more details">
<a href="javascript:return;" data-pluginName="{{ plugin.name }}" class="more">{{ plugin.name }}</a>
</h3>
<p class="description">{{ plugin.description }}
<br />
- <a href="javascript:return;" data-pluginName="{{ plugin.name }}" class="more">>> more</a>
+ <a href="javascript:return;" title="Click for more details" data-pluginName="{{ plugin.name }}" class="more">>> more</a>
</p>
{% if plugin.canBeUpdated %}
<p class="updateAvailableNotice">You can update this plugin to version {{ plugin.latestVersion }}</p>
diff --git a/plugins/CorePluginsAdmin/templates/plugins.twig b/plugins/CorePluginsAdmin/templates/plugins.twig
index 7b6887e915..7fa22d01cb 100644
--- a/plugins/CorePluginsAdmin/templates/plugins.twig
+++ b/plugins/CorePluginsAdmin/templates/plugins.twig
@@ -6,12 +6,14 @@
<div style="max-width:980px;">
{% if activatedPluginName %}
- <div id="feedback-success">You have successfully activated plugin {{ activatedPluginName }}</div>
+ <div id="feedback-success"><strong>Well done!</strong> You have successfully activated plugin {{ activatedPluginName }}</div>
{% endif %}
{% if pluginsHavingUpdate|length %}
<h2>{{ pluginsHavingUpdate|length }} Update(s) available</h2>
+ <p>{{ 'Update your plugins now to benefit from the latest improvements.'|translate }}</p>
+
{{ plugins.tablePluginUpdates(pluginsHavingUpdate, updateNonce, activateNonce, 0) }}
{% endif %}
diff --git a/plugins/CorePluginsAdmin/templates/themeOverview.twig b/plugins/CorePluginsAdmin/templates/themeOverview.twig
index dffb65aa25..63a5700c51 100644
--- a/plugins/CorePluginsAdmin/templates/themeOverview.twig
+++ b/plugins/CorePluginsAdmin/templates/themeOverview.twig
@@ -1,22 +1,25 @@
-{% if not isSuperUser %}
-{% elseif plugin.canBeUpdated %}
- <a href="{{ linkTo({'action':'updatePlugin', 'pluginName': plugin.name, 'nonce': updateNonce}) }}"
- class="update"
- >{{ 'CoreUpdater_UpdateTitle'|translate }}</a>
-{% elseif plugin.isInstalled %}
- <span class="install">{{ 'General_Installed'|translate }}</span>
-{% else %}
- <a href="{{ linkTo({'action': 'installPlugin', 'pluginName': plugin.name, 'nonce': installNonce}) }}"
- class="install">Install</a>
+{% if isSuperUser %}
+ {% if plugin.canBeUpdated %}
+ <a href="{{ linkTo({'action':'updatePlugin', 'pluginName': plugin.name, 'nonce': updateNonce}) }}"
+ class="update"
+ >{{ 'CoreUpdater_UpdateTitle'|translate }}</a>
+ {% elseif plugin.isInstalled %}
+ <span class="install">{{ 'General_Installed'|translate }}</span>
+ {% else %}
+ <a href="{{ linkTo({'action': 'installPlugin', 'pluginName': plugin.name, 'nonce': installNonce}) }}"
+ class="install">Install</a>
+ {% endif %}
{% endif %}
-<h3 class="header">
+<h3 class="header" title="Click for more details">
<a href="javascript:return;" data-pluginName="{{ plugin.name }}" class="more">{{ plugin.name }}</a>
</h3>
+
<p class="description">{{ plugin.description }}</p>
-<a href="javascript:return;" data-pluginName="{{ plugin.name }}" class="more"><img
+<a href="javascript:return;" data-pluginName="{{ plugin.name }}" class="more"><img title="Click for more details"
class="preview" src="{{ plugin.screenshots|first }}?w=250&h=250"/></a>
+
{% if plugin.canBeUpdated %}
<p class="updateAvailableNotice">You can update this theme to version {{ plugin.latestVersion }}</p>
{% endif %}
\ No newline at end of file
diff --git a/plugins/CorePluginsAdmin/templates/themes.twig b/plugins/CorePluginsAdmin/templates/themes.twig
index b364c0fabf..c88e3d8f5f 100644
--- a/plugins/CorePluginsAdmin/templates/themes.twig
+++ b/plugins/CorePluginsAdmin/templates/themes.twig
@@ -6,12 +6,14 @@
<div style="max-width:980px;">
{% if activatedPluginName %}
- <div id="feedback-success">You have successfully activated plugin {{ activatedPluginName }}</div>
+ <div id="feedback-success"><strong>Well done!</strong> You have successfully activated theme {{ activatedPluginName }}</div>
{% endif %}
{% if pluginsHavingUpdate|length %}
<h2>{{ pluginsHavingUpdate|length }} Update(s) available</h2>
+ <p>{{ 'Update your themes to enjoy the latest version.'|translate }}</p>
+
{{ plugins.tablePluginUpdates(pluginsHavingUpdate, updateNonce, true) }}
{% endif %}
--
GitLab