diff --git a/includes/mollom.class.inc b/includes/mollom.class.inc index b00cbf4..17aa0e7 100644 --- a/includes/mollom.class.inc +++ b/includes/mollom.class.inc @@ -15,7 +15,7 @@ abstract class Mollom { * The Mollom API version, used in HTTP requests. */ const API_VERSION = 'v1'; - + /** * Network communication failure code: No servers could be reached. * @@ -56,6 +56,11 @@ abstract class Mollom { const REDIRECT_ERROR = 1200; /** + * The Mollom REST endpoint. + */ + public $restEndpoint = 'http://rest.mollom.com'; + + /** * The public Mollom API key to use for request authentication. * * @var string @@ -82,22 +87,6 @@ abstract class Mollom { public $oAuthStrategy = 'header'; /** - * The list of Mollom servers to communicate with, as returned by Mollom. - * - * @see Mollom::getServers() - * - * @var array - */ - public $servers = array(); - - /** - * A hard-coded list of Mollom servers to fetch the server list from. - * - * @var array - */ - public $serversInit = array('http://rest.mollom.com/v1'); - - /** * The status code of the last server response, or TRUE if the request succeeded. * * If not TRUE, then the value is either one of @@ -110,7 +99,7 @@ abstract class Mollom { * * @var int|bool|null */ - public $lastResponseCode = NULL; + public $responseCode = NULL; /** * The amount of items contained in a list response. @@ -232,92 +221,6 @@ abstract class Mollom { } /** - * Fetches Mollom servers from local configuration or retrieves a new list. - * - * @return - * An indexed array of Mollom servers, which are also assigned to - * $this->servers. - * - * @see Mollom::refreshServers() - */ - public function getServers() { - // If there is no server list yet, consult the local configuration. - if (empty($this->servers)) { - $servers = $this->loadConfiguration('servers'); - // Use the local configuration value, if any. - if (!empty($servers) && is_array($servers)) { - $this->servers = $servers; - } - // Otherwise, retrieve a new server list from Mollom. - else { - $this->servers = $this->refreshServers(); - if ($this->servers) { - $this->saveConfiguration('servers', $this->servers); - $this->log[] = array( - 'severity' => 'debug', - 'message' => 'Refreshed servers: %servers', - 'arguments' => array( - '%servers' => implode(', ', $this->servers), - ), - ); - } - } - } - return $this->servers; - } - - /** - * Returns a new server list retrieved from Mollom. - * - * The server list returned from Mollom should be stored as configuration - * value on the client-side. This method should only be called when the - * configuration value is not yet or no longer available, or when a Mollom - * server specifically asks the client to refresh its server list. - * - * @return array - * An indexed array of server URLs retrieved from Mollom. - * - * @see Mollom::getServers() - * @see Mollom::query() - * @see MollomRefreshException - * @see Mollom::REFRESH_ERROR - */ - protected function refreshServers() { - // refreshServers() cannot use query() as we need to prevent infinite - // recursion. In addition, we handle returned error codes differently here, - // since REDIRECT_ERROR, REFRESH_ERROR, and any other communication error - // requires us to skip to the next server in order to retrieve a new server - // list. We only ever abort, if we get an AUTH_ERROR, in which case there - // is a configuration error (i.e., invalid API keys). - $servers = array(); - if (empty($this->publicKey) || empty($this->privateKey)) { - return $servers; - } - - $path = 'site/' . $this->publicKey; - $expected = array('site', 'servers'); - foreach ($this->serversInit as $server) { - try { - $result = $this->handleRequest('GET', $server, $path, array(), $expected); - } - catch (MollomAuthenticationException $e) { - // Bogus configuration. Stop trying, since all servers will fail. - break; - } - catch (MollomException $e) { - // On any other error, skip to the next server. - continue; - } - - if (isset($result) && $this->lastResponseCode === TRUE) { - $servers = $result['site']['servers']; - break; - } - } - return $servers; - } - - /** * Retrieve or send data from/to Mollom servers. * * @param string $method @@ -344,99 +247,28 @@ abstract class Mollom { $this->listCount = NULL; $this->listOffset = 0; $this->listTotal = NULL; + + // Compose the REST endpoint + $server = $this->restEndpoint . '/' . self::API_VERSION; - // Retrieve server list. - // If we get no list, we will have no servers to iterate over and only the - // error logic remains. - $this->getServers(); - - // Initialize refresh variable. - $refresh = FALSE; - // Send the request to the first server; if that fails, try the other - // servers in the list. - while ($server = current($this->servers)) { - try { - $result = $this->handleRequest($method, $server, $path, $data, $expected); - } - catch (MollomRefreshException $e) { - // Prevent infinite loops. - if (!$refresh) { - $refresh = TRUE; - - // In any case, the current server list is no longer valid. - $this->servers = array(); - $this->deleteConfiguration('servers'); - - // Retrieve a fresh list of Mollom servers. - $this->servers = $this->getServers(); - // If API keys are invalid, we won't be able to get a new server list. - // To reach this, we must have had a server list (and therefore - // valid keys) before, so we do not immediately return, but trigger - // the fallback mode instead. - if (empty($this->servers)) { - break; - } - } - } - catch (MollomRedirectException $e) { - // Try the next server in the list. - $next = next($this->servers); - - $this->log[] = array( - 'severity' => 'debug', - 'message' => 'Server %server redirected to %next.', - 'arguments' => array( - '%server' => $server, - '%next' => $next ? $next : '--', - ), - ); - continue; - } - catch (MollomAuthenticationException $e) { - // This is an irrecoverable error, so don't try other servers. - break; - } - catch (MollomException $e) { - // If the resource does not exist, there is no point in trying another - // server. - if ($e->getCode() == 404) { - break; - } - // On any other known error, try the next server. - next($this->servers); - continue; - } - - // Unless we have a positive result, continue to next server. - if ($this->lastResponseCode === TRUE) { - break; - } - else { - next($this->servers); - } + try { + $result = $this->handleRequest($method, $server, $path, $data, $expected); } - - // In case all servers failed, reset the server list to enforce retrieval of - // a new list the next time. - if (current($this->servers) === FALSE) { - $this->servers = array(); - $this->deleteConfiguration('servers'); - + catch (MollomAuthenticationException $e) { $this->log[] = array( 'severity' => 'error', - 'message' => 'All servers unreachable or returning errors. The server list was emptied: %servers', + 'message' => 'Could not authenticate with %server.', 'arguments' => array( - '%servers' => implode(', ', $this->servers ? $this->servers : $this->serversInit), + '%server' => $server, ), ); } - // Otherwise, make sure to use the last server we communicated with as the - // first/default server for subsequent requests of this Mollom class - // instance, but ensure the amount of servers we can fail-over to remains - // the same. - else { - // Move current server to the start of the server list. - $this->servers = array(key($this->servers) => current($this->servers)) + $this->servers; + catch (MollomException $e) { + // If the resource does not exist, there is no point in trying another + // server. + if ($e->getCode() == 404) { + // break; + } } // Write all captured log messages. @@ -444,7 +276,7 @@ abstract class Mollom { // If there is a result (only possible with a server list) and the last // request succeeded, return the result to the caller. - if (isset($result) && $this->lastResponseCode === TRUE) { + if (isset($result) && $this->responseCode === TRUE) { // Generically handle the special case of 'list' responses. if (isset($result['list']) && is_array($result)) { // Assign list meta properties to corresponding class properties. @@ -467,18 +299,18 @@ abstract class Mollom { } // If the last request succeeded but there was a unexpected response, return // the error code. - if ($this->lastResponseCode == self::RESPONSE_ERROR) { - return $this->lastResponseCode; + if ($this->responseCode == self::RESPONSE_ERROR) { + return $this->responseCode; } // Return an authentication error, which may require special client-side // processing. - if ($this->lastResponseCode == self::AUTH_ERROR) { - return $this->lastResponseCode; + if ($this->responseCode == self::AUTH_ERROR) { + return $this->responseCode; } // Return a not found error, which always needs to be handled by the calling // code. - if ($this->lastResponseCode == 404) { - return $this->lastResponseCode; + if ($this->responseCode == 404) { + return $this->responseCode; } // In case of any kind of HTTP error (404, 0 [invalid-address], @@ -508,7 +340,7 @@ abstract class Mollom { * @return array * An associative array representing the parsed response body on success. On * any failure, a MollomException is thrown. Additionally, - * Mollom::lastResponseCode is set to TRUE on success, or to the Mollom or + * Mollom::responseCode is set to TRUE on success, or to the Mollom or * HTTP status code on failure. * * @throws MollomNetworkException @@ -518,7 +350,7 @@ abstract class Mollom { * @throws MollomResponseException * @throws MollomException * - * @see Mollom::lastResponseCode + * @see Mollom::responseCode * @see Mollom::query() * @see Mollom::httpBuildQuery() * @see Mollom::parseXML() @@ -618,7 +450,7 @@ abstract class Mollom { throw new MollomException($response->message, $response->code, NULL, $this, $arguments); } else { - $this->lastResponseCode = TRUE; + $this->responseCode = TRUE; $this->log[] = array( 'severity' => 'debug', 'message' => '@code @method @uri', @@ -1050,11 +882,11 @@ abstract class Mollom { $result = $this->updateSite($data); // If the public key could not be found, make sure to return an // authentication error. - if ($this->lastResponseCode === 404) { + if ($this->responseCode === 404) { return self::AUTH_ERROR; } - // lastResponseCode will either be TRUE, AUTH_ERROR, or NETWORK_ERROR. - return $this->lastResponseCode === TRUE ? TRUE : $this->lastResponseCode; + // responseCode will either be TRUE, AUTH_ERROR, or NETWORK_ERROR. + return $this->responseCode === TRUE ? TRUE : $this->responseCode; } /** @@ -1068,7 +900,7 @@ abstract class Mollom { */ public function deleteSite($publicKey) { $result = $this->query('POST', 'site/' . $publicKey . '/delete'); - return $this->lastResponseCode === TRUE; + return $this->responseCode === TRUE; } /** @@ -1245,7 +1077,7 @@ abstract class Mollom { return FALSE; } $this->query('POST', 'feedback', $data); - return $this->lastResponseCode === TRUE ? TRUE : FALSE; + return $this->responseCode === TRUE ? TRUE : FALSE; } /** @@ -1351,7 +1183,7 @@ abstract class Mollom { $publicKey = $this->publicKey; } $result = $this->query('POST', 'blacklist/' . $publicKey . '/' . $entryId . '/delete'); - return $this->lastResponseCode === TRUE; + return $this->responseCode === TRUE; } } @@ -1417,7 +1249,7 @@ class MollomException extends Exception { $this->mollom = $mollom; // Set the error code on the Mollom class. - $mollom->lastResponseCode = $code; + $mollom->responseCode = $code; // Log the exception. $message = array( @@ -1456,25 +1288,6 @@ class MollomAuthenticationException extends MollomException { } /** - * Mollom server refresh exception. - * - * Thrown when a Mollom server asks the client to update the server list. - */ -class MollomRefreshException extends MollomException { - protected $severity = 'debug'; -} - -/** - * Mollom server redirect exception. - * - * Thrown when a Mollom server asks the client to use next server in the server - * list. - */ -class MollomRedirectException extends MollomException { - protected $severity = 'debug'; -} - -/** * Mollom server response exception. * * Thrown when a request to a Mollom server succeeds, but the response does not diff --git a/mollom.drupal.inc b/mollom.drupal.inc index f555a9e..1611837 100644 --- a/mollom.drupal.inc +++ b/mollom.drupal.inc @@ -122,30 +122,13 @@ class MollomDrupal extends Mollom { drupal_set_message(implode('
', $output)); //, $this->log[$i]['severity']); */ } - _mollom_watchdog_multiple($messages, $this->lastResponseCode === TRUE ? WATCHDOG_DEBUG : WATCHDOG_ERROR); + _mollom_watchdog_multiple($messages, $this->responseCode === TRUE ? WATCHDOG_DEBUG : WATCHDOG_ERROR); // After writing log messages, empty the log. $this->purgeLog(); } /** - * Overrides Mollom::refreshServers(). - */ - protected function refreshServers() { - $servers = parent::refreshServers(); - // FIXME: Server list contains XML-RPC servers. - if (empty($servers)) { - return $servers; - } - return $this->serversInit; - - // Allow other modules to alter the server list. Internal use only. - drupal_alter('mollom_server_list', $servers); - - return $servers; - } - - /** * Implements Mollom::request(). */ protected function request($method, $server, $path, $query = NULL, array $headers = array()) { @@ -194,10 +177,12 @@ class MollomDrupal extends Mollom { * Drupal Mollom client implementation using production testing servers. */ class MollomDrupalTest extends MollomDrupal { + /** - * Overrides Mollom::$serversInit. + * Overrides the REST endpoint */ - public $serversInit = array('http://dev.mollom.com/v1'); + public $restEndpoint = 'http://dev.mollom.com'; + } /** diff --git a/tests/mollom.test b/tests/mollom.test index d88a56b..367566c 100644 --- a/tests/mollom.test +++ b/tests/mollom.test @@ -116,6 +116,7 @@ class MollomWebTestCase extends DrupalWebTestCase { $modules[] = 'dblog'; parent::setUp($modules); variable_set('mollom_class', $this->mollomClass); + variable_set('mollom_testing_mode', 1); // D7's new default theme Bartik is bogus in various locations, which leads // to failing tests. @@ -314,8 +315,6 @@ class MollomWebTestCase extends DrupalWebTestCase { */ protected function createKeys($public = MOLLOM_TEST_PUBLIC_KEY, $private = MOLLOM_TEST_PRIVATE_KEY) { $mollom = mollom(); - // Hardcoded servers required, or any request will fail. - $mollom->servers = $mollom->serversInit; // Dummy API keys required, or Mollom client class (rightfully) bails out. $mollom->publicKey = MOLLOM_TEST_PUBLIC_KEY; $mollom->privateKey = MOLLOM_TEST_PRIVATE_KEY; @@ -1071,7 +1070,7 @@ class MollomResponseTestCase extends MollomWebTestCase { $mollom->privateKey = $site['privateKey']; $result = $mollom->getSite(); $this->assertMollomWatchdogMessages(WATCHDOG_EMERGENCY); - $this->assertEqual($mollom->lastResponseCode, Mollom::AUTH_ERROR, 'Attempt to authenticate with deleted site keys fails.'); + $this->assertEqual($mollom->responseCode, Mollom::AUTH_ERROR, 'Attempt to authenticate with deleted site keys fails.'); // Restore keys for tearDown(). $mollom->publicKey = $this->public_key; @@ -1733,7 +1732,7 @@ class MollomBlacklistTestCase extends MollomWebTestCase { $result = $mollom->deleteBlacklistEntry(999); $this->assertMollomWatchdogMessages(WATCHDOG_EMERGENCY); $this->assertNotIdentical($result, TRUE, t('Error response for a non-existing blacklist text found.')); - $this->assertSame('Response code', $mollom->lastResponseCode, 404); + $this->assertSame('Response code', $mollom->responseCode, 404); } /**