Merge pull request #360 from basdenooijer/develop

Improvements based on SensioLabs Insight analysis

library/Solarium/Client.php

@@ -95,8 +95,8 @@ class Client extends CoreClient
      * - 1.2.4 (higher)
      *
      *
-     * @internal a string compare is used instead of version_compare because
-     *  version_compare returns false for a compare of 1.0.0 with 1.0
+     * A string compare is used instead of version_compare because
+     * version_compare returns false for a compare of 1.0.0 with 1.0
      *
      * @param string $version
      *

library/Solarium/Core/Client/Client.php

@@ -217,6 +217,23 @@ class Client extends Configurable
      */
     protected $adapter;
 
+    /**
+     * Constructor.
+     *
+     * If options are passed they will be merged with {@link $options} using
+     * the {@link setOptions()} method.
+     *
+     * If an EventDispatcher instance is provided this will be used instead of creating a new instance
+     *
+     * @param array|\Zend_Config $options
+     * @param EventDispatcher $eventDispatcher
+     */
+    public function __construct($options = null, $eventDispatcher = null)
+    {
+        $this->eventDispatcher = $eventDispatcher;
+        parent::__construct($options);
+    }
+
     /**
      * Create a endpoint instance.
      *
@@ -243,7 +260,7 @@ class Client extends Configurable
 
         if ($endpoint->getKey() !== null) {
             $this->addEndpoint($endpoint);
-            if ($setAsDefault == true) {
+            if ($setAsDefault === true) {
                 $this->setDefaultEndpoint($endpoint);
             }
         }
@@ -276,7 +293,6 @@ class Client extends Configurable
         }
 
         //double add calls for the same endpoint are ignored, but non-unique keys cause an exception
-        //@todo add trigger_error with a notice for double add calls?
         if (array_key_exists($key, $this->endpoints) && $this->endpoints[$key] !== $endpoint) {
             throw new InvalidArgumentException('An endpoint must have a unique key');
         } else {
@@ -799,6 +815,9 @@ class Client extends Configurable
     /**
      * Execute a ping query.
      *
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
+     *
      * Example usage:
      * <code>
      * $client = new Solarium\Client;
@@ -808,9 +827,6 @@ class Client extends Configurable
      *
      * @see Solarium\QueryType\Ping
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
-     *
      * @param QueryInterface|\Solarium\QueryType\Ping\Query $query
      * @param Endpoint|string|null                          $endpoint
      *
@@ -824,6 +840,9 @@ class Client extends Configurable
     /**
      * Execute an update query.
      *
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
+     *
      * Example usage:
      * <code>
      * $client = new Solarium\Client;
@@ -835,9 +854,6 @@ class Client extends Configurable
      * @see Solarium\QueryType\Update
      * @see Solarium\Result\Update
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
-     *
      * @param QueryInterface|\Solarium\QueryType\Update\Query\Query $query
      * @param Endpoint|string|null                                  $endpoint
      *
@@ -851,6 +867,9 @@ class Client extends Configurable
     /**
      * Execute a select query.
      *
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
+     *
      * Example usage:
      * <code>
      * $client = new Solarium\Client;
@@ -861,9 +880,6 @@ class Client extends Configurable
      * @see Solarium\QueryType\Select
      * @see Solarium\Result\Select
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
-     *
      * @param QueryInterface|\Solarium\QueryType\Select\Query\Query $query
      * @param Endpoint|string|null                                  $endpoint
      *
@@ -877,6 +893,9 @@ class Client extends Configurable
     /**
      * Execute a MoreLikeThis query.
      *
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
+     *
      * Example usage:
      * <code>
      * $client = new Solarium\Client;
@@ -887,9 +906,6 @@ class Client extends Configurable
      * @see Solarium\QueryType\MoreLikeThis
      * @see Solarium\Result\MoreLikeThis
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
-     *
      * @param QueryInterface|\Solarium\QueryType\MoreLikeThis\Query $query
      * @param Endpoint|string|null                                  $endpoint
      *
@@ -903,8 +919,8 @@ class Client extends Configurable
     /**
      * Execute an analysis query.
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
      *
      * @param QueryInterface|\Solarium\QueryType\Analysis\Query\Document|\Solarium\QueryType\Analysis\Query\Field $query
      * @param Endpoint|string|null                                                                                $endpoint
@@ -919,8 +935,8 @@ class Client extends Configurable
     /**
      * Execute a terms query.
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
      *
      * @param QueryInterface|\Solarium\QueryType\Terms\Query $query
      * @param Endpoint|string|null                           $endpoint
@@ -935,8 +951,8 @@ class Client extends Configurable
     /**
      * Execute a suggester query.
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
      *
      * @param QueryInterface|\Solarium\QueryType\Suggester\Query $query
      * @param Endpoint|string|null                               $endpoint
@@ -951,8 +967,8 @@ class Client extends Configurable
     /**
      * Execute an extract query.
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
      *
      * @param QueryInterface|\Solarium\QueryType\Extract\Query $query
      * @param Endpoint|string|null                             $endpoint
@@ -967,8 +983,8 @@ class Client extends Configurable
     /**
      * Execute a RealtimeGet query.
      *
-     * @internal This is a convenience method that forwards the query to the
-     *  execute method, thus allowing for an easy to use and clean API.
+     * This is a convenience method that forwards the query to the
+     * execute method, thus allowing for an easy to use and clean API.
      *
      * @param QueryInterface|\Solarium\QueryType\RealtimeGet\Query $query
      * @param Endpoint|string|null                                 $endpoint
@@ -1144,8 +1160,9 @@ class Client extends Configurable
      */
     protected function init()
     {
-        //@todo use injection
-        $this->eventDispatcher = new EventDispatcher();
+        if ($this->eventDispatcher === null) {
+            $this->eventDispatcher = new EventDispatcher();
+        }
 
         foreach ($this->options as $name => $value) {
             switch ($name) {

library/Solarium/Core/Configurable.php

@@ -108,7 +108,7 @@ class Configurable implements ConfigurableInterface
                 }
             }
 
-            if (true == $overwrite) {
+            if (true === $overwrite) {
                 $this->options = $options;
             } else {
                 $this->options = array_merge($this->options, $options);
@@ -157,9 +157,9 @@ class Configurable implements ConfigurableInterface
      * This hook is called by the constructor after saving the constructor
      * arguments in {@link $options}
      *
-     * @internal This empty implementation can optionally be implemented in
-     *  descending classes. It's not an abstract method on purpose, there are
-     *  many cases where no initialization is needed.
+     * This empty implementation can optionally be implemented in
+     * descending classes. It's not an abstract method on purpose, there are
+     * many cases where no initialization is needed.
      */
     protected function init()
     {

library/Solarium/Core/Query/AbstractRequestBuilder.php

@@ -118,7 +118,7 @@ abstract class AbstractRequestBuilder implements RequestBuilderInterface
     public function boolAttrib($name, $value)
     {
         if (null !== $value) {
-            $value = (true == $value) ? 'true' : 'false';
+            $value = (true === (bool)$value) ? 'true' : 'false';
 
             return $this->attrib($name, $value);
         } else {

library/Solarium/Plugin/Loadbalancer/Loadbalancer.php

@@ -469,7 +469,7 @@ class Loadbalancer extends AbstractPlugin
         $this->endpointExcludes = array(); // reset for each query
         $adapter = $this->client->getAdapter();
 
-        if ($this->getFailoverEnabled() == true) {
+        if ($this->getFailoverEnabled() === true) {
             $maxRetries = $this->getFailoverMaxRetries();
             for ($i = 0; $i <= $maxRetries; $i++) {
                 $endpoint = $this->getRandomEndpoint();

library/Solarium/Plugin/Loadbalancer/WeightedRandomChoice.php

@@ -110,7 +110,6 @@ class WeightedRandomChoice
         }
 
         // continue until a non-excluded value is found
-        // @todo optimize?
         $result = null;
         while (1) {
             $result = $this->values[$this->getKey()];

library/Solarium/QueryType/Extract/RequestBuilder.php

@@ -80,7 +80,7 @@ class RequestBuilder extends BaseRequestBuilder
         }
 
         // add document settings to request
-        if (($doc = $query->getDocument()) != null) {
+        if (($doc = $query->getDocument()) !== null) {
             if ($doc->getBoost() !== null) {
                 throw new RuntimeException('Extract does not support document-level boosts, use field boosts instead.');
             }

library/Solarium/QueryType/MoreLikeThis/ResponseParser.php

@@ -71,7 +71,7 @@ class ResponseParser extends SelectResponseParser
             $parseResult['interestingTerms'] = $terms;
         }
 
-        if (isset($data['match']['docs'][0]) && true == $query->getMatchInclude()) {
+        if (isset($data['match']['docs'][0]) && true === $query->getMatchInclude()) {
             $matchData = $data['match']['docs'][0];
 
             $documentClass = $query->getOption('documentclass');

library/Solarium/QueryType/MoreLikeThis/Result.php

@@ -119,7 +119,7 @@ class Result extends SelectResult
     public function getMatch()
     {
         $query = $this->getQuery();
-        if (true != $query->getMatchInclude()) {
+        if (true !== $query->getMatchInclude()) {
             throw new UnexpectedValueException('matchinclude was disabled in the MLT query');
         }
         $this->parseResponse();

library/Solarium/QueryType/Select/Query/Component/FacetSet.php

@@ -319,7 +319,6 @@ class FacetSet extends AbstractComponent
         }
 
         //double add calls for the same facet are ignored, but non-unique keys cause an exception
-        //@todo add trigger_error with a notice for double add calls?
         if (array_key_exists($key, $this->facets) && $this->facets[$key] !== $facet) {
             throw new InvalidArgumentException('A facet must have a unique key value within a query');
         } else {

library/Solarium/QueryType/Select/Query/Component/Stats/Stats.php

@@ -152,7 +152,6 @@ class Stats extends AbstractComponent
         }
 
         //double add calls for the same field are ignored, but non-unique keys cause an exception
-        //@todo add trigger_error with a notice for double add calls?
         if (array_key_exists($key, $this->fields) && $this->fields[$key] !== $field) {
             throw new InvalidArgumentException('A field must have a unique key value');
         } else {

library/Solarium/QueryType/Select/Query/Query.php

@@ -634,7 +634,6 @@ class Query extends BaseQuery
         }
 
         //double add calls for the same FQ are ignored, but non-unique keys cause an exception
-        //@todo add trigger_error with a notice for double add calls?
         if (array_key_exists($key, $this->filterQueries) && $this->filterQueries[$key] !== $filterQuery) {
             throw new InvalidArgumentException('A filterquery must have a unique key value within a query');
         } else {
@@ -792,7 +791,7 @@ class Query extends BaseQuery
         if (isset($this->components[$key])) {
             return $this->components[$key];
         } else {
-            if ($autoload == true) {
+            if ($autoload === true) {
                 if (!isset($this->componentTypes[$key])) {
                     throw new OutOfBoundsException('Cannot autoload unknown component: '.$key);
                 }

library/Solarium/QueryType/Update/Result.php

@@ -48,8 +48,8 @@ use Solarium\Core\Query\Result\QueryType as BaseResult;
  * An update query only returns a query time and status. Both are accessible
  * using the methods provided by {@link Solarium\Result\Query}.
  *
- * @internal For now this class only exists to distinguish the different result
- *  types. It might get some extra behaviour in the future.
+ * For now this class only exists to distinguish the different result
+ * types. It might get some extra behaviour in the future.
  */
 class Result extends BaseResult
 {

tests/Solarium/Tests/Core/Client/ClientTest.php

@@ -121,9 +121,16 @@ class ClientTest extends \PHPUnit_Framework_TestCase
      */
     public function testGetEventDispatcher() {
       $this->assertInstanceOf('\Symfony\Component\EventDispatcher\EventDispatcherInterface', $this->client->getEventDispatcher());
-      $event_dispatcher = $this->getMock('\Symfony\Component\EventDispatcher\EventDispatcherInterface');
-      $this->client->setEventDispatcher($event_dispatcher);
-      $this->assertSame($event_dispatcher, $this->client->getEventDispatcher());
+        $eventDispatcher = $this->getMock('\Symfony\Component\EventDispatcher\EventDispatcherInterface');
+      $this->client->setEventDispatcher($eventDispatcher);
+      $this->assertSame($eventDispatcher, $this->client->getEventDispatcher());
+    }
+
+    public function testEventDispatcherInjection()
+    {
+        $eventDispatcher = $this->getMock('\Symfony\Component\EventDispatcher\EventDispatcherInterface');
+        $client = new Client(null, $eventDispatcher);
+        $this->assertSame($eventDispatcher, $client->getEventDispatcher());
     }
 
     public function testConfigModeWithoutKeys()