diff --git a/CHANGELOG.md b/CHANGELOG.md index d83107c650..9367ec4464 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,12 +7,17 @@ All notable changes to this project will be documented in this file based on the ### Bugfixes +- Fix reading bool index settings like `\Elastica\Index\Settings::getBlocksWrite`. Elasticsearch returns all settings as strings and does not normalize bool values. + The getters now return the right bool value for whichever string representation is used like 'true', '1', 'on', 'yes'. + ### Added ### Improvements ### Deprecated +- Deprecated `\Elastica\Exception\ElasticsearchException` which is irrelevant since Elasticsearch now exposes the errors as a structured array instead of a single string. + Use `\Elastica\Exception\ResponseException::getResponse::getFullError` instead. ## [5.1.0](https://github.com/ruflin/Elastica/compare/5.0.0...5.1.0) diff --git a/lib/Elastica/Exception/ElasticsearchException.php b/lib/Elastica/Exception/ElasticsearchException.php index 5eb8e75c92..c94633d8c7 100644 --- a/lib/Elastica/Exception/ElasticsearchException.php +++ b/lib/Elastica/Exception/ElasticsearchException.php @@ -1,6 +1,8 @@ _error = $error; - // TODO: es2 improve as now an array - $this->_parseError(json_encode($error)); - parent::__construct(json_encode($error), $code); + $this->_parseError($error); + parent::__construct($error, $code); } /** diff --git a/lib/Elastica/Exception/ResponseException.php b/lib/Elastica/Exception/ResponseException.php index 33a149a5b3..e0ae67809d 100644 --- a/lib/Elastica/Exception/ResponseException.php +++ b/lib/Elastica/Exception/ResponseException.php @@ -62,9 +62,7 @@ public function getResponse() public function getElasticsearchException() { $response = $this->getResponse(); - $transfer = $response->getTransferInfo(); - $code = array_key_exists('http_code', $transfer) ? $transfer['http_code'] : 0; - return new ElasticsearchException($code, $response->getErrorMessage()); + return new ElasticsearchException($response->getStatus(), $response->getErrorMessage()); } } diff --git a/lib/Elastica/Index.php b/lib/Elastica/Index.php index f3933ca939..1b8522a8f3 100644 --- a/lib/Elastica/Index.php +++ b/lib/Elastica/Index.php @@ -283,9 +283,8 @@ public function create(array $args = [], $options = null) public function exists() { $response = $this->getClient()->request($this->getName(), Request::HEAD); - $info = $response->getTransferInfo(); - return (bool) ($info['http_code'] == 200); + return $response->getStatus() === 200; } /** diff --git a/lib/Elastica/Index/Settings.php b/lib/Elastica/Index/Settings.php index ff17482692..7690aff40a 100644 --- a/lib/Elastica/Index/Settings.php +++ b/lib/Elastica/Index/Settings.php @@ -102,6 +102,25 @@ public function get($setting = '') } } + /** + * Returns a setting interpreted as a bool. + * + * One can use a real bool, int(0), int(1) to set bool settings. + * But Elasticsearch stores and returns all settings as strings and does + * not normalize bool values. This method ensures a bool is returned for + * whichever string representation is used like 'true', '1', 'on', 'yes'. + * + * @param string $setting Setting name to return + * + * @return bool + */ + public function getBool($setting) + { + $data = $this->get($setting); + + return 'true' === $data || '1' === $data || 'on' === $data || 'yes' === $data; + } + /** * Sets the number of replicas. * @@ -111,11 +130,7 @@ public function get($setting = '') */ public function setNumberOfReplicas($replicas) { - $replicas = (int) $replicas; - - $data = ['number_of_replicas' => $replicas]; - - return $this->set($data); + return $this->set(['number_of_replicas' => (int) $replicas]); } /** @@ -127,17 +142,15 @@ public function setNumberOfReplicas($replicas) */ public function setReadOnly($readOnly = true) { - return $this->set(['blocks.write' => $readOnly]); + return $this->set(['blocks.read_only' => $readOnly]); } /** - * getReadOnly. - * * @return bool */ public function getReadOnly() { - return $this->get('blocks.write') === 'true'; // ES returns a string for this setting + return $this->getBool('blocks.read_only'); } /** @@ -145,7 +158,7 @@ public function getReadOnly() */ public function getBlocksRead() { - return (bool) $this->get('blocks.read'); + return $this->getBool('blocks.read'); } /** @@ -155,8 +168,6 @@ public function getBlocksRead() */ public function setBlocksRead($state = true) { - $state = $state ? 1 : 0; - return $this->set(['blocks.read' => $state]); } @@ -165,7 +176,7 @@ public function setBlocksRead($state = true) */ public function getBlocksWrite() { - return (bool) $this->get('blocks.write'); + return $this->getBool('blocks.write'); } /** @@ -175,8 +186,6 @@ public function getBlocksWrite() */ public function setBlocksWrite($state = true) { - $state = $state ? 1 : 0; - return $this->set(['blocks.write' => $state]); } @@ -185,13 +194,12 @@ public function setBlocksWrite($state = true) */ public function getBlocksMetadata() { - // TODO will have to be replace by block.metadata.write once https://github.com/elasticsearch/elasticsearch/pull/9203 has been fixed - // the try/catch will have to be remove too + // When blocks.metadata is enabled, reading the settings is not possible anymore. + // So when a cluster_block_exception happened it must be enabled. try { - return (bool) $this->get('blocks.write'); + return $this->getBool('blocks.metadata'); } catch (ResponseException $e) { - if (strpos($e->getMessage(), 'ClusterBlockException') !== false) { - // hacky way to test if the metadata is blocked since bug 9203 is not fixed + if ($e->getResponse()->getFullError()['type'] === 'cluster_block_exception') { return true; } @@ -200,15 +208,15 @@ public function getBlocksMetadata() } /** + * Set to true to disable index metadata reads and writes. + * * @param bool $state OPTIONAL (default = true) * * @return \Elastica\Response */ public function setBlocksMetadata($state = true) { - $state = $state ? 1 : 0; - - return $this->set(['blocks.write' => $state]); + return $this->set(['blocks.metadata' => $state]); } /** diff --git a/lib/Elastica/IndexTemplate.php b/lib/Elastica/IndexTemplate.php index 1e7efa6317..1864f97d9e 100644 --- a/lib/Elastica/IndexTemplate.php +++ b/lib/Elastica/IndexTemplate.php @@ -78,9 +78,8 @@ public function create(array $args = []) public function exists() { $response = $this->request(Request::HEAD); - $info = $response->getTransferInfo(); - return (bool) ($info['http_code'] == 200); + return $response->getStatus() === 200; } /** diff --git a/lib/Elastica/Query.php b/lib/Elastica/Query.php index 2a5531fe9b..515abdd15e 100644 --- a/lib/Elastica/Query.php +++ b/lib/Elastica/Query.php @@ -46,13 +46,13 @@ public function __construct($query = null) } /** - * Transforms a string or an array to a query object. + * Transforms the argument to a query object. * - * If query is empty, + * For example, an empty argument will return a \Elastica\Query with a \Elastica\Query\MatchAll. * * @param mixed $query * - * @throws \Elastica\Exception\NotImplementedException + * @throws InvalidException For an invalid argument * * @return self */ @@ -77,8 +77,7 @@ public static function create($query) } - // TODO: Implement queries without - throw new NotImplementedException(); + throw new InvalidException('Unexpected argument to create a query for.'); } /** diff --git a/lib/Elastica/Response.php b/lib/Elastica/Response.php index e0a44cdd62..a938626a27 100644 --- a/lib/Elastica/Response.php +++ b/lib/Elastica/Response.php @@ -27,13 +27,6 @@ class Response */ protected $_responseString = ''; - /** - * Error. - * - * @var bool Error - */ - protected $_error = false; - /** * Transfer info. * @@ -113,11 +106,11 @@ public function getError() } /** - * A keyed array representing any errors that occured. + * A keyed array representing any errors that occurred. * * In case of http://localhost:9200/_alias/test the error is a string * - * @return array|string Error data + * @return array|string|null Error data or null if there is no error */ public function getFullError() { @@ -133,13 +126,7 @@ public function getFullError() */ public function getErrorMessage() { - $error = $this->getError(); - - if (!is_string($error)) { - $error = json_encode($error); - } - - return $error; + return $this->getError(); } /** @@ -227,18 +214,15 @@ public function getData() { if ($this->_response == null) { $response = $this->_responseString; - if ($response === false) { - $this->_error = true; - } else { - try { - if ($this->getJsonBigintConversion()) { - $response = JSON::parse($response, true, 512, JSON_BIGINT_AS_STRING); - } else { - $response = JSON::parse($response); - } - } catch (JSONParseException $e) { - // leave response as is if parse fails + + try { + if ($this->getJsonBigintConversion()) { + $response = JSON::parse($response, true, 512, JSON_BIGINT_AS_STRING); + } else { + $response = JSON::parse($response); } + } catch (JSONParseException $e) { + // leave response as is if parse fails } if (empty($response)) { diff --git a/lib/Elastica/Status.php b/lib/Elastica/Status.php index b6333cb3e7..90bbc7033f 100644 --- a/lib/Elastica/Status.php +++ b/lib/Elastica/Status.php @@ -106,9 +106,8 @@ public function getIndicesWithAlias($alias) try { $response = $this->_client->request('/_alias/'.$alias); } catch (ResponseException $e) { - $transferInfo = $e->getResponse()->getTransferInfo(); // 404 means the index alias doesn't exist which means no indexes have it. - if ($transferInfo['http_code'] === 404) { + if ($e->getResponse()->getStatus() === 404) { return []; } // If we don't have a 404 then this is still unexpected so rethrow the exception. diff --git a/lib/Elastica/Type.php b/lib/Elastica/Type.php index 3acd136d47..722f3f453d 100644 --- a/lib/Elastica/Type.php +++ b/lib/Elastica/Type.php @@ -522,8 +522,7 @@ public function setSerializer($serializer) public function exists() { $response = $this->getIndex()->request($this->getName(), Request::HEAD); - $info = $response->getTransferInfo(); - return $info['http_code'] == 200; + return $response->getStatus() === 200; } } diff --git a/test/lib/Elastica/Test/Bulk/ResponseSetTest.php b/test/lib/Elastica/Test/Bulk/ResponseSetTest.php index bf0cf9c033..55297947ea 100644 --- a/test/lib/Elastica/Test/Bulk/ResponseSetTest.php +++ b/test/lib/Elastica/Test/Bulk/ResponseSetTest.php @@ -139,7 +139,7 @@ public function isOkDataProvider() */ protected function _createResponseSet(array $responseData, array $actions) { - $client = $this->createMock('Elastica\\Client', ['request']); + $client = $this->createMock('Elastica\\Client'); $client->expects($this->once()) ->method('request') diff --git a/test/lib/Elastica/Test/Exception/ElasticsearchExceptionTest.php b/test/lib/Elastica/Test/Exception/ElasticsearchExceptionTest.php deleted file mode 100644 index d11894f7eb..0000000000 --- a/test/lib/Elastica/Test/Exception/ElasticsearchExceptionTest.php +++ /dev/null @@ -1,6 +0,0 @@ -getResponse()->getFullError(); $this->assertEquals('index_already_exists_exception', $error['type']); - $this->assertEquals(400, $ex->getElasticsearchException()->getCode()); + $this->assertEquals(400, $ex->getResponse()->getStatus()); } } @@ -45,7 +45,7 @@ public function testBadType() } catch (ResponseException $ex) { $error = $ex->getResponse()->getFullError(); $this->assertEquals('mapper_parsing_exception', $error['type']); - $this->assertEquals(400, $ex->getElasticsearchException()->getCode()); + $this->assertEquals(400, $ex->getResponse()->getStatus()); } } @@ -62,7 +62,7 @@ public function testWhatever() } catch (ResponseException $ex) { $error = $ex->getResponse()->getFullError(); $this->assertEquals('index_not_found_exception', $error['type']); - $this->assertEquals(404, $ex->getElasticsearchException()->getCode()); + $this->assertEquals(404, $ex->getResponse()->getStatus()); } } } diff --git a/test/lib/Elastica/Test/Index/SettingsTest.php b/test/lib/Elastica/Test/Index/SettingsTest.php index 46680e64d7..18945296b9 100644 --- a/test/lib/Elastica/Test/Index/SettingsTest.php +++ b/test/lib/Elastica/Test/Index/SettingsTest.php @@ -199,6 +199,7 @@ public function testSetReadOnly() // Try to add doc to read only index $index->getSettings()->setReadOnly(true); $this->assertTrue($index->getSettings()->getReadOnly()); + $this->assertTrue($index->exists()); try { $type->addDocument($doc2); @@ -207,7 +208,7 @@ public function testSetReadOnly() $error = $e->getResponse()->getFullError(); $this->assertContains('cluster_block_exception', $error['type']); - $this->assertContains('index write', $error['reason']); + $this->assertContains('read-only', $error['reason']); } // Remove read only, add document diff --git a/test/lib/Elastica/Test/IndexTemplateTest.php b/test/lib/Elastica/Test/IndexTemplateTest.php index e188c44308..1378873d76 100644 --- a/test/lib/Elastica/Test/IndexTemplateTest.php +++ b/test/lib/Elastica/Test/IndexTemplateTest.php @@ -45,7 +45,7 @@ public function testDelete() $name = 'index_template1'; $response = new Response(''); /** @var \PHPUnit_Framework_MockObject_MockObject|Client $clientMock */ - $clientMock = $this->createMock('\Elastica\Client', ['request']); + $clientMock = $this->createMock('\Elastica\Client'); $clientMock->expects($this->once()) ->method('request') ->with('_template/'.$name, Request::DELETE, [], []) @@ -63,7 +63,7 @@ public function testCreate() $response = new Response(''); $name = 'index_template1'; /** @var \PHPUnit_Framework_MockObject_MockObject|Client $clientMock */ - $clientMock = $this->createMock('\Elastica\Client', ['request']); + $clientMock = $this->createMock('\Elastica\Client'); $clientMock->expects($this->once()) ->method('request') ->with('_template/'.$name, Request::PUT, $args, []) @@ -78,10 +78,9 @@ public function testCreate() public function testExists() { $name = 'index_template1'; - $response = new Response(''); - $response->setTransferInfo(['http_code' => 200]); + $response = new Response('', 200); /** @var \PHPUnit_Framework_MockObject_MockObject|Client $clientMock */ - $clientMock = $this->createMock('\Elastica\Client', ['request']); + $clientMock = $this->createMock('\Elastica\Client'); $clientMock->expects($this->once()) ->method('request') ->with('_template/'.$name, Request::HEAD, [], [])