Merge pull request #1016 from ruflin/reponse-refactoring

Refactor response exception and make it more BC compatible

CHANGELOG.md

@@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file based on the
 - Set `json_decode()` assoc parameter to true in `Elastica\Response` #1005
 - Add `bigintConversion` to keys passed to connection config in `Elastica\Client` #1005
 - Use POST instead of PUT to send bulk requests #1010
+- Revert getError changes in Response object and make it better BC compatible. See comment [here](https://github.com/ruflin/Elastica/commit/41a7a2075837320bc9bd3bca4150e05a1ec9a115#commitcomment-15136374).
 
 ### Added
 - Elastica\Query\MultiMatch::setFuzziness now supports being set to `AUTO` with the const `MultiMatch::FUZZINESS_AUTO`

Makefile

@@ -5,7 +5,7 @@ TARGET?=56
 
 # By default docker environment is used to run commands. To run without the predefined environment, set RUN_ENV=" " either as parameter or as environment variable
 ifndef RUN_ENV
-	RUN_ENV = docker run --entrypoint="" -v $(shell pwd):/elastica ruflin/elastica
+	RUN_ENV = docker run --workdir="/elastica" -v $(shell pwd):/elastica ruflin/elastica-dev-base
 endif
 
 .PHONY: clean

env/elastica/Docker56

@@ -8,10 +8,11 @@ RUN apt-get update && apt-get install -y \
 	git \
 	graphviz \
 	nano \
-	php5-xsl
+	php5-xsl \
+	zlib1g-dev
 	# XSL and Graphviz for PhpDocumentor
 
-RUN docker-php-ext-install sockets
+RUN docker-php-ext-install sockets zip
 
 RUN rm -r /var/lib/apt/lists/*
 
@@ -35,4 +36,4 @@ ENV PATH=/root/composer/vendor/bin:$PATH
 COPY composer.json /root/composer/
 
 # Install development tools
-RUN composer global install --prefer-source
+RUN composer global install #--prefer-source

env/elastica/composer.json

@@ -12,15 +12,14 @@
         }
     ],
     "require": {
-        "fabpot/php-cs-fixer":"1.10.*",
+        "phpdocumentor/phpdocumentor":"~2.8",
+        "fabpot/php-cs-fixer":"1.11.*",
         "mayflower/php-codebrowser":"~1.1",
-        "pdepend/pdepend":"~2.0",
+        "pdepend/pdepend":"~2.2",
         "phploc/phploc":"~2.1",
-        "phpdocumentor/template-zend":"~1.3",
-        "phpdocumentor/phpdocumentor":"~2.8",
-        "phpmd/phpmd":"~2.2",
-        "phpunit/phpunit":"~4.7",
+        "phpmd/phpmd":"~2.3",
+        "phpunit/phpunit":"~4.8",
         "sebastian/phpcpd":"~2.0",
-        "squizlabs/php_codesniffer":"~2.3"
+        "squizlabs/php_codesniffer":"~2.5"
     }
 }

lib/Elastica/Bulk/ResponseSet.php

@@ -53,6 +53,25 @@ public function getError()
         return $error;
     }
 
+    /**
+     * Returns first found error (full array).
+     *
+     * @return array|string
+     */
+    public function getFullError()
+    {
+        $error = '';
+
+        foreach ($this->getBulkResponses() as $bulkResponse) {
+            if ($bulkResponse->hasError()) {
+                $error = $bulkResponse->getFullError();
+                break;
+            }
+        }
+
+        return $error;
+    }
+
     /**
      * @return bool
      */

lib/Elastica/Query/BoolQuery.php

@@ -55,7 +55,7 @@ public function addMustNot($args)
      * @param \Elastica\Filter\AbstractFilter $filter Filter object
      *
      * @return $this
-    */
+     */
     public function addFilter(AbstractFilter $filter)
     {
         return $this->addParam('filter', $filter);

lib/Elastica/Query/Boosting.php

@@ -39,7 +39,7 @@ public function setNegativeQuery(AbstractQuery $query)
     /**
      * Set the negative_boost parameter for this Boosting Query.
      *
-     * @param Float $negativeBoost
+     * @param float $negativeBoost
      *
      * @return $this
      */

lib/Elastica/Query/MoreLikeThis.php

@@ -31,7 +31,7 @@ public function setFields(array $fields)
      * @param array $ids Document ids
      *
      * @deprecated Option "ids" deprecated as of ES 2.0.0-beta1 and will be removed in further Elastica releases. Use "like" instead.
-
+     
      * @return \Elastica\Query\MoreLikeThis Current object
      */
     public function setIds(array $ids)
@@ -57,7 +57,7 @@ public function setLike($like)
      * @param string $likeText
      *
      * @deprecated Option "like_text" deprecated as of ES 2.0.0-beta1 and will be removed at further Elastica releases. Use "like" instead.
-
+     
      * @return $this
      */
     public function setLikeText($likeText)

lib/Elastica/QueryBuilder/DSL/Query.php

@@ -199,6 +199,7 @@ public function field()
     public function filtered(AbstractQuery $query = null, AbstractFilter $filter = null)
     {
         trigger_error('Use bool() instead. Filtered query is deprecated since ES 2.0.0-beta1 and this method will be removed in further Elastica releases.', E_USER_DEPRECATED);
+
         return new Filtered($query, $filter);
     }
 

lib/Elastica/Response.php

@@ -81,18 +81,46 @@ public function __construct($responseString, $responseStatus = null)
     /**
      * Error message.
      *
-     * @return array Error object
+     * @return string Error message
      */
     public function getError()
     {
-        $error = array();
+        $error = $this->getFullError();
+
+        if (!$error) {
+            return '';
+        }
+
+        if (is_string($error)) {
+            return $error;
+        }
+
+        if (isset($error['root_cause'][0])) {
+            $error = $error['root_cause'][0];
+        }
+
+        $message = $error['reason'];
+        if (isset($error['index'])) {
+            $message .= ' [index: '.$error['index'].']';
+        }
+
+        return $message;
+    }
+
+    /**
+     * A keyed array representing any errors that occured.
+     *
+     * In case of http://localhost:9200/_alias/test the error is a string
+     *
+     * @return array|string Error data
+     */
+    public function getFullError()
+    {
         $response = $this->getData();
 
         if (isset($response['error'])) {
-            $error = $response['error'];
+            return $response['error'];
         }
-
-        return $error;
     }
 
     /**
@@ -180,7 +208,7 @@ public function isOk()
             return true;
         }
 
-        return (isset($data['ok']) && $data['ok']);
+        return isset($data['ok']) && $data['ok'];
     }
 
     /**

lib/Elastica/Type/Mapping.php

@@ -262,6 +262,7 @@ public function toArray()
      * Submits the mapping and sends it to the server.
      *
      * @param array $query Query string parameters to send with mapping
+     *
      * @return \Elastica\Response Response object
      */
     public function send(array $query = array())

test/lib/Elastica/Test/BulkTest.php

@@ -388,7 +388,7 @@ public function testErrorRequest()
             $bulk->send();
             $bulk->fail('3rd document create should produce error');
         } catch (ResponseException $e) {
-            $error = $e->getResponseSet()->getError();
+            $error = $e->getResponseSet()->getFullError();
             $this->assertContains('document_already_exists_exception', $error['type']);
             $failures = $e->getFailures();
             $this->assertInternalType('array', $failures);

test/lib/Elastica/Test/ClientTest.php

@@ -1167,7 +1167,7 @@ public function testRemoveHeader()
      */
     public function testPassBigIntSettingsToConnectionConfig()
     {
-        $client = new Client(['bigintConversion' => true]);
+        $client = new Client(array('bigintConversion' => true));
 
         $this->assertTrue($client->getConnection()->getConfig('bigintConversion'));
     }

test/lib/Elastica/Test/Cluster/SettingsTest.php

@@ -84,7 +84,7 @@ public function testSetReadOnly()
             $index1->getType('test')->addDocument($doc3);
             $this->fail('should throw read only exception');
         } catch (ResponseException $e) {
-            $error = $e->getResponse()->getError();
+            $error = $e->getResponse()->getFullError();
             $this->assertContains('cluster_block_exception', $error['type']);
             $this->assertContains('cluster read-only', $error['reason']);
         }
@@ -93,7 +93,7 @@ public function testSetReadOnly()
             $index2->getType('test')->addDocument($doc4);
             $this->fail('should throw read only exception');
         } catch (ResponseException $e) {
-            $error = $e->getResponse()->getError();
+            $error = $e->getResponse()->getFullError();
             $this->assertContains('cluster_block_exception', $error['type']);
             $this->assertContains('cluster read-only', $error['reason']);
         }

test/lib/Elastica/Test/Exception/ResponseExceptionTest.php

@@ -17,7 +17,7 @@ public function testCreateExistingIndex()
             $this->_createIndex('woo', false);
             $this->fail('Index created when it should fail');
         } catch (ResponseException $ex) {
-            $error = $ex->getResponse()->getError();
+            $error = $ex->getResponse()->getFullError();
             $this->assertEquals('index_already_exists_exception', $error['type']);
             $this->assertEquals(400, $ex->getElasticsearchException()->getCode());
         }
@@ -43,7 +43,7 @@ public function testBadType()
             )));
             $this->fail('Indexing with wrong type should fail');
         } catch (ResponseException $ex) {
-            $error = $ex->getResponse()->getError();
+            $error = $ex->getResponse()->getFullError();
             $this->assertEquals('mapper_parsing_exception', $error['type']);
             $this->assertEquals(400, $ex->getElasticsearchException()->getCode());
         }
@@ -60,7 +60,7 @@ public function testWhatever()
         try {
             $index->search();
         } catch (ResponseException $ex) {
-            $error = $ex->getResponse()->getError();
+            $error = $ex->getResponse()->getFullError();
             $this->assertEquals('index_not_found_exception', $error['type']);
             $this->assertEquals(404, $ex->getElasticsearchException()->getCode());
         }

test/lib/Elastica/Test/Filter/BoolFilterTest.php

@@ -82,7 +82,7 @@ public function getTestToArrayData()
      * @group unit
      * @dataProvider getTestToArrayData()
      *
-     * @param Bool  $bool
+     * @param bool  $bool
      * @param array $expectedArray
      */
     public function testToArray(BoolFilter $bool, $expectedArray)

test/lib/Elastica/Test/Index/SettingsTest.php

@@ -231,7 +231,7 @@ public function testSetReadOnly()
             $type->addDocument($doc2);
             $this->fail('Should throw exception because of read only');
         } catch (ResponseException $e) {
-            $error = $e->getResponse()->getError();
+            $error = $e->getResponse()->getFullError();
 
             $this->assertContains('cluster_block_exception', $error['type']);
             $this->assertContains('index write', $error['reason']);
@@ -333,7 +333,7 @@ public function testNotFoundIndex()
             $settings = $index->getSettings()->get();
             $this->fail('Should throw exception because of index not found');
         } catch (ResponseException $e) {
-            $error = $e->getResponse()->getError();
+            $error = $e->getResponse()->getFullError();
             $this->assertContains('index_not_found_exception', $error['type']);
         }
     }

test/lib/Elastica/Test/IndexTest.php

@@ -26,7 +26,7 @@ public function testMapping()
         $type = $index->getType('test');
 
         $mapping = array('id' => array('type' => 'integer', 'store' => true), 'email' => array('type' => 'string', 'store' => 'no'),
-            'username' => array('type' => 'string', 'store' => 'no'), 'test' => array('type' => 'integer', 'store' => 'no'),);
+            'username' => array('type' => 'string', 'store' => 'no'), 'test' => array('type' => 'integer', 'store' => 'no'), );
         $type->setMapping($mapping);
 
         $type->addDocument($doc);
@@ -247,7 +247,7 @@ public function testExcludeFileSource()
     {
         $this->_checkPlugin('elasticsearch-mapper-attachments');
         $indexMapping = array('file' => array('type' => 'attachment'), 'text' => array('type' => 'string', 'store' => true),
-            'title' => array('type' => 'string', 'store' => true),);
+            'title' => array('type' => 'string', 'store' => true), );
 
         $indexParams = array('index' => array('number_of_shards' => 1, 'number_of_replicas' => 0));
 
@@ -669,7 +669,7 @@ public function testIndexGetMapping()
         $type = $index->getType('test');
 
         $mapping = array('id' => array('type' => 'integer', 'store' => true), 'email' => array('type' => 'string', 'store' => false),
-            'username' => array('type' => 'string', 'store' => false), 'test' => array('type' => 'integer', 'store' => false),);
+            'username' => array('type' => 'string', 'store' => false), 'test' => array('type' => 'integer', 'store' => false), );
 
         $type->setMapping($mapping);
         $index->refresh();

test/lib/Elastica/Test/Query/FilteredTest.php

@@ -9,8 +9,7 @@
 
 class FilteredTest extends BaseTest
 {
-
-    private $errors = [];
+    private $errors = array();
 
     private function setErrorHandler()
     {
@@ -27,7 +26,7 @@ private function restoreErrorHandler()
             $this->assertCount(1, $this->errors);
             $this->assertEquals(E_USER_DEPRECATED, $this->errors[0][0]);
             $this->assertEquals('Use BoolQuery instead. Filtered query is deprecated since ES 2.0.0-beta1 and this class will be removed in further Elastica releases.', $this->errors[0][1]);
-            $this->errors = [];
+            $this->errors = array();
         }
     }
 

test/lib/Elastica/Test/ResponseTest.php

@@ -131,27 +131,29 @@ public function testIsOkBulkItemsWithOkField()
     /**
      * @group unit
      */
-    public function testArrayErrorMessage()
+    public function testStringErrorMessage()
     {
         $response = new Response(json_encode(array(
-            'error' => array('a', 'b'),
+            'error' => 'a',
         )));
 
-        $this->assertEquals(json_encode(array('a', 'b')), $response->getErrorMessage());
+        $this->assertEquals('a', $response->getErrorMessage());
     }
 
     /**
      * @group unit
      */
-    public function testStringErrorMessage()
+    public function testArrayErrorMessage()
     {
         $response = new Response(json_encode(array(
-            'error' => 'a',
+            'error' => array('a', 'b'),
         )));
 
-        $this->assertEquals('a', $response->getErrorMessage());
+        $this->assertEquals(array('a', 'b'), $response->getFullError());
     }
 
+
+
     /**
      * @group unit
      */

test/lib/Elastica/Test/SearchTest.php

@@ -578,7 +578,7 @@ public function testIgnoreUnavailableOption()
         } catch (ResponseException $e) {
             $exception = $e;
         }
-        $error = $exception->getResponse()->getError();
+        $error = $exception->getResponse()->getFullError();
         $this->assertEquals('index_not_found_exception', $error['type']);
 
         $results = $search->search($query, array(Search::OPTION_SEARCH_IGNORE_UNAVAILABLE => true));