Merge pull request #5 from elkuku/error-handling

Error handling

administrator/components/com_patchtester/controllers/pull.php

@@ -19,14 +19,17 @@ class PatchtesterControllerPull extends JController
 {
 	public function apply()
 	{
-		$model = $this->getModel('pull');
+		try {
-		if ($model->apply(JRequest::getVar('pull_id'))) {
+			$model = $this->getModel('pull')
+			->apply(JRequest::getVar('pull_id'));
+
 			$msg = 'Patch successfully applied';
 			$type = 'message';
-		} else {
+		} catch (Exception $e) {
-			$msg = $model->getError();
+			$msg = $e->getMessage();
 			$type = 'error';
 		}
+
 		$this->setRedirect(JRoute::_('index.php?option=com_patchtester&view=pulls', false), $msg, $type);
 	}
 

administrator/components/com_patchtester/language/de-DE/de-DE.com_patchtester.ini

@@ -1,10 +1,8 @@
 COM_PATCHTESTER_NOT_APPLIED="Nicht angebracht"
 COM_PATCHTESTER_APPLIED="Angebracht"
 COM_PATCHTESTER_REVERT_PATCH="Patch zurücknehmen"
-COM_PATCHTESTER_APPLY_PATCH="Patch anwenden"
+COM_PATCHTESTER_APPLY_PATCH="Patch anbringen"
 COM_PATCHTESTER_TEST_THIS_PATCH="Diesen Patch testen"
-COM_PATCHTESTER_REPO_IS_GONE="Der Patch konnte nicht angebracht werden weil das Repository fehlt"
-COM_PATCHTESTER_CONFLICT="Der Patch konnte nicht angebracht werden weil er mit einem bereits angebrachten Patch in Konflikt steht."
 COM_PATCHTESTER_COMPONENT_LABEL="Patch Tester"
 COM_PATCHTESTER_COMPONENT_DESC="Patch Tester Konfiguration"
 COM_PATCHTESTER_FIELD_ORG_LABEL="Github Benutzername"
@@ -12,3 +10,10 @@ COM_PATCHTESTER_FIELD_ORG_DESC="Name des Github Kontos von welchem Pull Requests
 COM_PATCHTESTER_FIELD_REPO_LABEL="Github Repository"
 COM_PATCHTESTER_FIELD_REPO_DESC="Name des Github Repositories von welchem Pull Requests beobachtet werden sollen."
 COM_PATCHTESTER_JOOMLACODE_ISSUE="Joomlacode Tracker"
+COM_PATCHTESTER_PULL_ID="Pull ID"
+
+;messages
+COM_PATCHTESTER_REPO_IS_GONE="Der Patch konnte nicht angebracht werden weil das Repository fehlt"
+COM_PATCHTESTER_CONFLICT_S="Der Patch konnte nicht angebracht werden weil er mit einem bereits angebrachten Patch in Konflikt steht: %s"
+COM_PATCHTESTER_FILE_DELETED_DOES_NOT_EXIST_S="Die zu löschende Datei existiert nicht: %s"
+COM_PATCHTESTER_FILE_MODIFIED_DOES_NOT_EXIST_S="Die zu ändernde Datei existiert nicht: %s"

administrator/components/com_patchtester/language/en-GB/en-GB.com_patchtester.ini

@@ -3,8 +3,6 @@ COM_PATCHTESTER_APPLIED="Applied"
 COM_PATCHTESTER_REVERT_PATCH="Revert Patch"
 COM_PATCHTESTER_APPLY_PATCH="Apply Patch"
 COM_PATCHTESTER_TEST_THIS_PATCH="Test This Patch"
-COM_PATCHTESTER_REPO_IS_GONE="The patch could not be applied because the repository is missing"
-COM_PATCHTESTER_CONFLICT="The patch could not be applied because it conflicts with a previously applied patch"
 COM_PATCHTESTER_COMPONENT_LABEL="Patch Tester"
 COM_PATCHTESTER_COMPONENT_DESC="Patch Tester Configuration Values"
 COM_PATCHTESTER_FIELD_ORG_LABEL="Github Username"
@@ -13,3 +11,9 @@ COM_PATCHTESTER_FIELD_REPO_LABEL="Github Repository"
 COM_PATCHTESTER_FIELD_REPO_DESC="Name of repository on Github of which to monitor pull requests"
 COM_PATCHTESTER_JOOMLACODE_ISSUE="Joomlacode Issue"
 COM_PATCHTESTER_PULL_ID="Pull ID"
+
+;messages
+COM_PATCHTESTER_REPO_IS_GONE="The patch could not be applied because the repository is missing"
+COM_PATCHTESTER_CONFLICT_S="The patch could not be applied because it conflicts with a previously applied patch: %s"
+COM_PATCHTESTER_FILE_DELETED_DOES_NOT_EXIST_S="The file marked for deletion does not exist: %s"
+COM_PATCHTESTER_FILE_MODIFIED_DOES_NOT_EXIST_S="The file marked for modification does not exist: %s"

administrator/components/com_patchtester/models/pull.php

@@ -27,8 +27,8 @@ class PatchtesterModelPull extends JModel
 		// Load the parameters.
 		$params = JComponentHelper::getParams('com_patchtester');
 		$this->setState('params', $params);
-		$this->setState('github_user', $params->get('org'));
+		$this->setState('github_user', $params->get('org', 'joomla'));
-		$this->setState('github_repo', $params->get('repo'));
+		$this->setState('github_repo', $params->get('repo', 'joomla-cms'));
 
 		parent::populateState();
 	}
@@ -38,7 +38,9 @@ class PatchtesterModelPull extends JModel
 		$state = 0;
 		$files = array();
 
-		foreach ($patch AS $line) {
+		$lines = explode("\n", $patch);
+
+		foreach ($lines AS $line) {
 			switch ($state)
 			{
 				case 0:
@@ -83,51 +85,41 @@ class PatchtesterModelPull extends JModel
 	public function apply($id)
 	{
 		jimport('joomla.client.github');
-		jimport('joomla.client.http');
+		jimport('joomla.client.curl');
 
 		$table = JTable::getInstance('tests', 'PatchTesterTable');
-		$github = new JGithub();
+		$github = new JGithub;
 		$pull = $github->pulls->get($this->getState('github_user'), $this->getState('github_repo'), $id);
-		$patchUrl = $pull->diff_url;
-
-		$http = new JHttp;
-
-		$patch = $http->get($patchUrl)->body;
-		$patch = explode("\n", $patch);
-
 
 		if (is_null($pull->head->repo)) {
-			$this->setError(JText::_('COM_PATCHTESTER_REPO_IS_GONE'));
+			throw new Exception(JText::_('COM_PATCHTESTER_REPO_IS_GONE'));
-			return false;
 		}
 
+		$patch = JCurl::getAdapter($pull->diff_url)
+		->fetch()->body;
+
 		$files = $this->parsePatch($patch);
 
 		foreach($files AS $file) {
+			if ($file->action == 'deleted' && ! file_exists(JPATH_ROOT . '/' . $file->old)) {
+				throw new Exception(sprintf(JText::_('COM_PATCHTESTER_FILE_DELETED_DOES_NOT_EXIST_S'), $file->old));
+			}
 			if ($file->action == 'added' || $file->action == 'modified') {
-				$http = new JHttp;
-
-				$url = 'https://raw.github.com/' . $pull->head->user->login . '/' . $pull->head->repo->name . '/' .
-				$pull->head->ref . '/' . $file->new;
-
 
 				// if the backup file already exists, we can't apply the patch
-				if ($file->action != 'deleted' && file_exists(JPATH_COMPONENT . '/backups/' . md5($file->new) . '.txt')) {
+				if (file_exists(JPATH_COMPONENT . '/backups/' . md5($file->new) . '.txt')) {
-					$this->setError(JText::_('COM_PATCHTESTER_CONFLICT'));
+					throw new Exception(sprintf(JText::_('COM_PATCHTESTER_CONFLICT_S'), $file->new));
-					return false;
 				}
 
-				if (($file->action == 'deleted' || $file->action == 'modified') && !file_exists(JPATH_ROOT . '/' . $file->old)) {
+				if ($file->action == 'modified' && ! file_exists(JPATH_ROOT . '/' . $file->old)) {
-					$this->setError(JText::_('COM_PATCHTESTER_FILE_DELETED_MODIFIED_DOES_NOT_EXIST'));
+					throw new Exception(sprintf(JText::_('COM_PATCHTESTER_FILE_MODIFIED_DOES_NOT_EXIST_S'), $file->old));
-					return false;
 				}
 
-				try {
+				$url = 'https://raw.github.com/' . $pull->head->user->login . '/' . $pull->head->repo->name . '/' .
-					$file->body = $http->get($url)->body;
+					$pull->head->ref . '/' . $file->new;
-				} catch (Exception $e) {
+
-					$this->setError(JText::_('COM_PATCHTESTER_APPLY_FAILED_ERROR_RETRIEVING_FILE'));
+				$file->body = JCurl::getAdapter($url)
-					return false;
+				->fetch()->body;
-				}
 			}
 		}
 
@@ -137,34 +129,41 @@ class PatchtesterModelPull extends JModel
 		{
 			// we only create a backup if the file already exists
 			if ($file->action == 'deleted' || (file_exists(JPATH_ROOT . '/' . $file->new) && $file->action == 'modified')) {
-				JFile::copy(JPath::clean(JPATH_ROOT . '/' . $file->old), JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt');
+				if( ! JFile::copy(JPath::clean(JPATH_ROOT . '/' . $file->old), JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt')) {
+					throw new Exception(sprintf('Can not copy file %s to %s'
+					, JPATH_ROOT . '/' . $file->old, JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'));
+				}
 			}
 
 			switch ($file->action)
 			{
 				case 'modified':
 				case 'added':
-					JFile::write(JPath::clean(JPATH_ROOT . '/' . $file->new), $file->body);
+					if( ! JFile::write(JPath::clean(JPATH_ROOT . '/' . $file->new), $file->body)) {
+						throw new Exception(sprintf('Can not write the file: %s', JPATH_ROOT . '/' . $file->new));
+					}
 					break;
 
 				case 'deleted':
-					JFile::delete(JPATH::clean(JPATH_ROOT . '/' . $file->old));
+					if( ! JFile::delete(JPATH::clean(JPATH_ROOT . '/' . $file->old))) {
+						throw new Exception(sprintf('Can not delete the file: %s', JPATH_ROOT . '/' . $file->old));
+					}
 					break;
 			}
 		}
+
 		$table->pull_id = $pull->number;
 		$table->data = json_encode($files);
 		$table->patched_by = JFactory::getUser()->id;
 		$table->applied = 1;
 		$version = new JVersion;
 		$table->applied_version = $version->getShortVersion();
-		$result = $table->store();
 
-		if ($result) {
+		if ( ! $table->store()) {
-			return true;
+			throw new Exception($table->getError());
-		} else {
-			return false;
 		}
+
+		return true;
 	}
 
 	public function revert($id)
@@ -188,12 +187,21 @@ class PatchtesterModelPull extends JModel
 			switch ($file->action) {
 				case 'deleted':
 				case 'modified':
-					JFile::copy(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt', JPATH_ROOT . '/' . $file->old);
+					if ( ! JFile::copy(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt', JPATH_ROOT . '/' . $file->old)) {
-					JFile::delete(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt');
+						throw new Exception(sprintf('Can not copy file %s to %s'
+						, JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'
+						, JPATH_ROOT . '/' . $file->old));
+					}
+					if ( ! JFile::delete(JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt')) {
+						throw new Exception(sprintf('Can not delete the file: %s'
+						, JPATH_COMPONENT . '/backups/' . md5($file->old) . '.txt'));
+					}
 					break;
 
 				case 'added':
-					JFile::delete(JPath::clean(JPATH_ROOT . '/' . $file->new));
+					if ( ! JFile::delete(JPath::clean(JPATH_ROOT . '/' . $file->new))) {
+						throw new Exception(sprintf('Can not delete the file: %s', JPATH_ROOT . '/' . $file->new));
+					}
 					break;
 			}
 		}

administrator/components/com_patchtester/models/pulls.php

@@ -103,21 +103,28 @@ class PatchtesterModelPulls extends JModelList
 		$this->orderDir = $this->getState('list.direction', 'asc');
 		$search = $this->getState('filter.search');
 
-		$github = new JGithub();
+		try {
-		$pulls = $github->pulls->getAll($this->getState('github_user'), $this->getState('github_repo'));
+			$github = new JGithub();
-		usort($pulls, array($this, 'sortItems'));
+			$pulls = $github->pulls->getAll($this->getState('github_user'), $this->getState('github_repo'));
+			usort($pulls, array($this, 'sortItems'));
 
-		foreach ($pulls AS $i => &$pull)
+			foreach ($pulls AS $i => &$pull)
-		{
+			{
-			if($search && false === strpos($pull->title, $search)) {
+				if($search && false === strpos($pull->title, $search)) {
-				unset($pulls[$i]);
+					unset($pulls[$i]);
-				continue;
+					continue;
+				}
+				$matches = array();
+				preg_match('#\[\#([0-9]+)\]#', $pull->title, $matches);
+				$pull->joomlacode_issue = isset($matches[1]) ? $matches[1] : 0;
 			}
-			$matches = array();
+
-			preg_match('#\[\#([0-9]+)\]#', $pull->title, $matches);
+			return $pulls;
-			$pull->joomlacode_issue = isset($matches[1]) ? $matches[1] : 0;
+		} catch (Exception $e) {
+			JFactory::getApplication()->enqueueMessage($e->getMessage(), 'error');
+
+			return array();
 		}
-		return $pulls;
 	}
 
 	public function sortItems($a, $b)

libraries/joomla/client/curl.php

@@ -0,0 +1,193 @@
+<?php
+/**
+ * @package     Joomla.Platform
+ * @subpackage  Client
+ *
+ * @copyright   Copyright (C) 2005 - 2011 Open Source Matters, Inc. All rights reserved.
+ * @license     GNU General Public License version 2 or later; see LICENSE
+ */
+
+defined('JPATH_PLATFORM') or die;
+
+/**
+ * A cURL client class.
+ *
+ * @package     Joomla.Platform
+ * @subpackage  Client
+ * @since       ?
+ */
+class JCurl extends JURI
+{
+	/**
+	 * @var array curlOptions.
+	 */
+	protected $curlOptions = array();
+
+	/**
+	 * @var string Target path where to save the response
+	 */
+	protected $target = '';
+
+	/**
+	 * Get a cURL adapter.
+	 *
+	 * @param   string  $uri  The URI to work with.
+	 *
+	 * @throws Exception
+	 *
+	 * @return JCurl
+	 */
+	public static function getAdapter($uri = null)
+	{
+		if ( ! function_exists('curl_init'))
+		{
+			throw new Exception('cURL is not available');
+		}
+
+		return new JCurl($uri);
+	}
+
+	/**
+	 * Constructor.
+	 * You should pass a URI string to the constructor to initialise a specific URI.
+	 *
+	 * @param   string  $uri  The URI string
+	 *
+	 * @throws Exception
+	 */
+	public function __construct($uri = null)
+	{
+		if ( ! function_exists('curl_init'))
+		{
+			throw new Exception('cURL is not available');
+		}
+
+		parent::__construct($uri);
+	}
+
+	/**
+	 * Set cURL options.
+	 *
+	 * @param   array  $options  The cURL options.
+	 *
+	 * @return JCurl
+	 */
+	public function setOptions($options)
+	{
+		$this->curlOptions = (array)$options;
+
+		return $this;
+	}
+
+	/**
+	 * Read URL contents.
+	 *
+	 * @throws Exception
+	 *
+	 * @return object The cURL response.
+	 */
+	public function fetch()
+	{
+		$ch = curl_init();
+
+		curl_setopt_array($ch, $this->curlOptions);
+
+		curl_setopt($ch, CURLOPT_URL, $this->_uri);
+
+		if ( ! array_key_exists(CURLOPT_SSL_VERIFYHOST, $this->curlOptions))
+		{
+			curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, true);
+		}
+
+		if ( ! array_key_exists(CURLOPT_SSL_VERIFYPEER, $this->curlOptions))
+		{
+			curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
+		}
+
+		if ( ! array_key_exists(CURLOPT_FOLLOWLOCATION, $this->curlOptions))
+		{
+			curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true);
+		}
+
+		if ( ! array_key_exists(CURLOPT_MAXREDIRS, $this->curlOptions))
+		{
+			curl_setopt($ch, CURLOPT_MAXREDIRS, 10);
+		}
+
+		if ( ! array_key_exists(CURLOPT_TIMEOUT, $this->curlOptions))
+		{
+			curl_setopt($ch, CURLOPT_TIMEOUT, 120);
+		}
+
+		if ( ! array_key_exists(CURLOPT_RETURNTRANSFER, $this->curlOptions))
+		{
+			curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
+		}
+
+		if ($this->target)
+		{
+			// Write the response to a file
+			$fp = fopen($this->target, 'w');
+
+			if ( ! $fp)
+			{
+				throw new Exception('Can not open target file at: '.$this->target);
+			}
+
+			// Use CURLOPT_FILE to speed things up
+			curl_setopt($ch, CURLOPT_FILE, $fp);
+		}
+		else
+		{
+			// Return the response
+			if ( ! array_key_exists(CURLOPT_RETURNTRANSFER, $this->curlOptions))
+			{
+				curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
+			}
+		}
+
+		$response = curl_exec($ch);
+
+		if (curl_errno($ch))
+		{
+			throw new Exception('Curl Error: '.curl_error($ch));
+		}
+
+		$info = curl_getinfo($ch);
+
+		if (isset($info['http_code']) && $info['http_code'] != 200)
+		{
+			$response = false;
+		}
+
+		curl_close($ch);
+
+		$return = JArrayHelper::toObject($info);
+		$return->body = $response;
+
+		return $return;
+	}
+
+	/**
+	 * Save the response to a file.
+	 *
+	 * @param   string  $target  Target path
+	 *
+	 * @return boolean true on success
+	 *
+	 * @throws Exception
+	 */
+	public function saveToFile($target)
+	{
+		$this->target = $target;
+
+		$response = $this->fetch();
+
+		if (false === $response)
+		{
+			throw new Exception('File cannot be downloaded');
+		}
+
+		return true;
+	}
+}

libraries/joomla/client/github.php

@@ -11,6 +11,7 @@ defined('JPATH_PLATFORM') or die;
 
 jimport('joomla.environment.uri');
 jimport('joomla.client.http');
+jimport('joomla.client.curl');
 JLoader::register('JHttpResponse', JPATH_PLATFORM.'/joomla/client/http.php');
 jimport('joomla.client.github.githubpulls');
 jimport('joomla.client.github.githubgists');
@@ -32,7 +33,7 @@ class JGithub
 
 	/**
 	 * Authentication Method
-	 * 
+	 *
 	 * Possible values are 0 - no authentication, 1 - basic authentication, 2 - OAuth
 	 *
 	 * @var    string
@@ -43,7 +44,7 @@ class JGithub
 	protected $gists = null;
 
 	protected $issues = null;
-	
+
 	protected $pulls = null;
 
 	protected $credentials = array();
@@ -69,8 +70,6 @@ class JGithub
 		} else {
 			$this->authentication_method = JGithub::AUTHENTICATION_NONE;
 		}
-
-		$this->http = curl_init();
 	}
 
 	public function __get($name)
@@ -95,27 +94,20 @@ class JGithub
 			}
 			return $this->pulls;
 		}
-
 	}
 
 	public function sendRequest($url, $method = 'get', $data = array(), $options = array())
 	{
-		
-		$this->http = curl_init();
-
 		$curl_options = array(
-			CURLOPT_URL => 'https://api.github.com'.$url,
-			CURLOPT_RETURNTRANSFER => true,
 			CURLOPT_HEADER => false,
 			CURLOPT_FOLLOWLOCATION => false,
 			CURLOPT_USERAGENT => 'JGithub',
 			CURLOPT_CONNECTTIMEOUT => 120,
-			CURLOPT_TIMEOUT => 120,
 			CURLINFO_HEADER_OUT => true,
 			CURLOPT_HTTPHEADER => array('Content-type: application/json'),
 			CURLOPT_CAINFO => dirname(__FILE__) . '/github/cacert.pem',
 			CURLOPT_SSL_VERIFYPEER => true,
-			CURLOPT_SSL_VERIFYHOST, 2
+			CURLOPT_SSL_VERIFYHOST => 2,
 		);
 
 		switch ($this->authentication_method)
@@ -133,7 +125,6 @@ class JGithub
 				break;
 		}
 
-
 		switch ($method) {
 			case 'post':
 				$curl_options[CURLOPT_POST] = 1;
@@ -146,34 +137,32 @@ class JGithub
 				$curl_options[CURLOPT_CUSTOMREQUEST] = 'PUT';
 				$curl_options[CURLOPT_HTTPGET] = false;
 				break;
-				
+
 			case 'patch':
 				$curl_options[CURLOPT_POSTFIELDS] = json_encode($data);
+
 			case 'delete':
 				$curl_options[CURLOPT_CUSTOMREQUEST] = strtoupper($method);
 				$curl_options[CURLOPT_POST] = false;
 				$curl_options[CURLOPT_HTTPGET] = false;
-				
 				break;
 
 			case 'get':
 				$curl_options[CURLOPT_POSTFIELDS] = null;
 				$curl_options[CURLOPT_POST] = false;
 				$curl_options[CURLOPT_HTTPGET] = true;
-
 				break;
 		}
 
-		curl_setopt_array($this->http, $curl_options);
+		$curlResponse = JCurl::getAdapter('https://api.github.com'.$url)
+		->setOptions($curl_options)->fetch();
 
 		$response = new JHttpResponse;
-		$response->body = json_decode(curl_exec($this->http));
 
-		$request_data = curl_getinfo($this->http);
+		$response->code = $curlResponse->http_code;
-		$response->headers = $request_data['request_header'];
+		$response->headers = $curlResponse->request_header;
-		$response->code = $request_data['http_code'];
+		$response->body = json_decode($curlResponse->body);
 
-		curl_close($this->http);
 		return $response;
 	}
 }

patchtester/build.sh

@@ -11,6 +11,7 @@ rm -rf github
 mkdir github
 cp ../libraries/joomla/client/github.php github
 cp ../libraries/joomla/client/githubobject.php github
+cp ../libraries/joomla/client/curl.php github
 cp -r ../libraries/joomla/client/github github
 cp github.xml github
 tar jcf ../file_github.tar.bz2 github/*

patchtester/github.xml

@@ -13,6 +13,7 @@
 	<!-- Fileset definition -->
 	<fileset>
 		<files target="libraries/joomla/client">
+			<filename>curl.php</filename>
 			<filename>github.php</filename>
 			<filename>githubobject.php</filename>
 			<folder>github</folder>