From e9551bd171f664cb69c18e3065a6b4a2eee7d06a Mon Sep 17 00:00:00 2001 From: David Causse Date: Wed, 27 Mar 2019 21:06:54 +0100 Subject: [PATCH] Properly handle underscores in options Since 6.1.0 all metadata except _index, _type and _id are accepted without the underscore. In 6.5 it produces a deprecation warning and in 7 they are no longer accepted. --- CHANGELOG.md | 2 + README.md | 4 +- lib/Elastica/AbstractUpdateAction.php | 89 ++++++++----------- lib/Elastica/Bulk/Action/DeleteDocument.php | 8 +- lib/Elastica/Bulk/Action/IndexDocument.php | 8 +- lib/Elastica/Document.php | 6 +- .../Bulk/Action/UpdateDocumentTest.php | 4 +- test/Elastica/BulkTest.php | 16 ++-- test/Elastica/DocumentTest.php | 29 +++--- 9 files changed, 75 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 530fb40c91..1753cecda1 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,11 @@ All notable changes to this project will be documented in this file based on the ## [Unreleased](https://github.com/ruflin/Elastica/compare/6.1.1...master) ### Backward Compatibility Breaks +* \Elastica\AbstractUpdateAction::getOptions( $fields ) no longer supports the $underscore parameter, option names must match what elasticsearch expects. ### Bugfixes * Always set the Guzzle `base_uri` to support connecting to multiple ES hosts. [#1618](https://github.com/ruflin/Elastica/pull/1618) +* Properly handle underscore prefixes in options and bulk request metadata ([cf upstream](https://github.com/elastic/elasticsearch/issues/26886). [#1621](https://github.com/ruflin/Elastica/pull/1621) ### Added diff --git a/README.md b/README.md index 4eebb59406..8ef5de841a 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ All library issues should go to the [issue tracker from github](https://github.c Compatibility ------------- -This release is compatible with all Elasticsearch 6.x releases. It was tested with version 6.6.1. +This release is compatible with all Elasticsearch 6.1 releases and onwards. It was tested with version 6.6.1. Contributing @@ -36,4 +36,4 @@ This project tries to follow Elasticsearch in terms of [End of Life](https://www | [5.x](https://github.com/ruflin/Elastica/tree/5.x) | 5.x | ^5.0 | \>=5.6 | | [3.2.3](https://github.com/ruflin/Elastica/tree/3.2.3) (unmaintained) | 2.4.0 | no | \>=5.4 | | [2.x](https://github.com/ruflin/Elastica/tree/2.x) (unmaintained) | 1.7.2 | no | \>=5.3.3 | ------------- \ No newline at end of file +------------ diff --git a/lib/Elastica/AbstractUpdateAction.php b/lib/Elastica/AbstractUpdateAction.php index 99a902f109..721d47b459 100644 --- a/lib/Elastica/AbstractUpdateAction.php +++ b/lib/Elastica/AbstractUpdateAction.php @@ -113,7 +113,7 @@ public function getIndex() */ public function setVersion($version) { - return $this->setParam('_version', (int) $version); + return $this->setParam('version', (int) $version); } /** @@ -123,7 +123,7 @@ public function setVersion($version) */ public function getVersion() { - return $this->getParam('_version'); + return $this->getParam('version'); } /** @@ -131,7 +131,7 @@ public function getVersion() */ public function hasVersion() { - return $this->hasParam('_version'); + return $this->hasParam('version'); } /** @@ -144,7 +144,7 @@ public function hasVersion() */ public function setVersionType($versionType) { - return $this->setParam('_version_type', $versionType); + return $this->setParam('version_type', $versionType); } /** @@ -154,7 +154,7 @@ public function setVersionType($versionType) */ public function getVersionType() { - return $this->getParam('_version_type'); + return $this->getParam('version_type'); } /** @@ -162,7 +162,7 @@ public function getVersionType() */ public function hasVersionType() { - return $this->hasParam('_version_type'); + return $this->hasParam('version_type'); } /** @@ -176,7 +176,7 @@ public function hasVersionType() */ public function setParent($parent) { - return $this->setParam('_parent', $parent); + return $this->setParam('parent', $parent); } /** @@ -186,7 +186,7 @@ public function setParent($parent) */ public function getParent() { - return $this->getParam('_parent'); + return $this->getParam('parent'); } /** @@ -194,7 +194,7 @@ public function getParent() */ public function hasParent() { - return $this->hasParam('_parent'); + return $this->hasParam('parent'); } /** @@ -206,7 +206,7 @@ public function hasParent() */ public function setOpType($opType) { - return $this->setParam('_op_type', $opType); + return $this->setParam('op_type', $opType); } /** @@ -216,7 +216,7 @@ public function setOpType($opType) */ public function getOpType() { - return $this->getParam('_op_type'); + return $this->getParam('op_type'); } /** @@ -224,7 +224,7 @@ public function getOpType() */ public function hasOpType() { - return $this->hasParam('_op_type'); + return $this->hasParam('op_type'); } /** @@ -236,7 +236,7 @@ public function hasOpType() */ public function setRouting($value) { - return $this->setParam('_routing', $value); + return $this->setParam('routing', $value); } /** @@ -246,7 +246,7 @@ public function setRouting($value) */ public function getRouting() { - return $this->getParam('_routing'); + return $this->getParam('routing'); } /** @@ -254,7 +254,7 @@ public function getRouting() */ public function hasRouting() { - return $this->hasParam('_routing'); + return $this->hasParam('routing'); } /** @@ -268,7 +268,7 @@ public function setFields($fields) $fields = \implode(',', $fields); } - return $this->setParam('_fields', (string) $fields); + return $this->setParam('fields', (string) $fields); } /** @@ -284,7 +284,7 @@ public function setFieldsSource() */ public function getFields() { - return $this->getParam('_fields'); + return $this->getParam('fields'); } /** @@ -292,7 +292,7 @@ public function getFields() */ public function hasFields() { - return $this->hasParam('_fields'); + return $this->hasParam('fields'); } /** @@ -302,7 +302,7 @@ public function hasFields() */ public function setRetryOnConflict($num) { - return $this->setParam('_retry_on_conflict', (int) $num); + return $this->setParam('retry_on_conflict', (int) $num); } /** @@ -310,7 +310,7 @@ public function setRetryOnConflict($num) */ public function getRetryOnConflict() { - return $this->getParam('_retry_on_conflict'); + return $this->getParam('retry_on_conflict'); } /** @@ -318,7 +318,7 @@ public function getRetryOnConflict() */ public function hasRetryOnConflict() { - return $this->hasParam('_retry_on_conflict'); + return $this->hasParam('retry_on_conflict'); } /** @@ -328,7 +328,7 @@ public function hasRetryOnConflict() */ public function setRefresh($refresh = true) { - return $this->setParam('_refresh', (bool) $refresh ? 'true' : 'false'); + return $this->setParam('refresh', (bool) $refresh ? 'true' : 'false'); } /** @@ -336,7 +336,7 @@ public function setRefresh($refresh = true) */ public function getRefresh() { - return 'true' === $this->getParam('_refresh'); + return 'true' === $this->getParam('refresh'); } /** @@ -344,7 +344,7 @@ public function getRefresh() */ public function hasRefresh() { - return $this->hasParam('_refresh'); + return $this->hasParam('refresh'); } /** @@ -354,7 +354,7 @@ public function hasRefresh() */ public function setTimeout($timeout) { - return $this->setParam('_timeout', $timeout); + return $this->setParam('timeout', $timeout); } /** @@ -362,7 +362,7 @@ public function setTimeout($timeout) */ public function getTimeout() { - return $this->getParam('_timeout'); + return $this->getParam('timeout'); } /** @@ -370,7 +370,7 @@ public function getTimeout() */ public function hasTimeout() { - return $this->hasParam('_timeout'); + return $this->hasParam('timeout'); } /** @@ -380,7 +380,7 @@ public function hasTimeout() */ public function setConsistency($timeout) { - return $this->setParam('_consistency', $timeout); + return $this->setParam('consistency', $timeout); } /** @@ -388,7 +388,7 @@ public function setConsistency($timeout) */ public function getConsistency() { - return $this->getParam('_consistency'); + return $this->getParam('consistency'); } /** @@ -396,7 +396,7 @@ public function getConsistency() */ public function hasConsistency() { - return $this->hasParam('_consistency'); + return $this->hasParam('consistency'); } /** @@ -406,7 +406,7 @@ public function hasConsistency() */ public function setReplication($timeout) { - return $this->setParam('_replication', $timeout); + return $this->setParam('replication', $timeout); } /** @@ -414,7 +414,7 @@ public function setReplication($timeout) */ public function getReplication() { - return $this->getParam('_replication'); + return $this->getParam('replication'); } /** @@ -422,7 +422,7 @@ public function getReplication() */ public function hasReplication() { - return $this->hasParam('_replication'); + return $this->hasParam('replication'); } /** @@ -455,31 +455,16 @@ public function hasUpsert() } /** - * @param array $fields if empty array all options will be returned, field names can be either with underscored either without, i.e. _percolate, routing - * @param bool $withUnderscore should option keys contain underscore prefix + * @param array $fields if empty array all options will be returned * * @return array */ - public function getOptions(array $fields = [], $withUnderscore = false) + public function getOptions(array $fields = []) { if (!empty($fields)) { - $data = []; - foreach ($fields as $field) { - $key = '_'.\ltrim($field, '_'); - if ($this->hasParam($key) && '' !== (string) $this->getParam($key)) { - $data[$key] = $this->getParam($key); - } - } - } else { - $data = $this->getParams(); - } - if (!$withUnderscore) { - foreach ($data as $key => $value) { - $data[\ltrim($key, '_')] = $value; - unset($data[$key]); - } + return \array_filter(\array_intersect_key($this->getParams(), \array_flip($fields))); } - return $data; + return \array_filter($this->getParams()); } } diff --git a/lib/Elastica/Bulk/Action/DeleteDocument.php b/lib/Elastica/Bulk/Action/DeleteDocument.php index a0d7fb74cc..dabf26a091 100644 --- a/lib/Elastica/Bulk/Action/DeleteDocument.php +++ b/lib/Elastica/Bulk/Action/DeleteDocument.php @@ -17,13 +17,13 @@ class DeleteDocument extends AbstractDocument protected function _getMetadata(AbstractUpdateAction $action): array { return $action->getOptions([ - 'index', - 'type', - 'id', + '_index', + '_type', + '_id', 'version', 'version_type', 'routing', 'parent', - ], true); + ]); } } diff --git a/lib/Elastica/Bulk/Action/IndexDocument.php b/lib/Elastica/Bulk/Action/IndexDocument.php index 1003c1bcef..b6d6daab63 100644 --- a/lib/Elastica/Bulk/Action/IndexDocument.php +++ b/lib/Elastica/Bulk/Action/IndexDocument.php @@ -30,14 +30,14 @@ public function setDocument(Document $document): AbstractDocument protected function _getMetadata(AbstractUpdateAction $action): array { return $action->getOptions([ - 'index', - 'type', - 'id', + '_index', + '_type', + '_id', 'version', 'version_type', 'routing', 'parent', 'retry_on_conflict', - ], true); + ]); } } diff --git a/lib/Elastica/Document.php b/lib/Elastica/Document.php index 1721ddb7c3..85f080b900 100644 --- a/lib/Elastica/Document.php +++ b/lib/Elastica/Document.php @@ -285,7 +285,7 @@ public function isAutoPopulate() */ public function setPipeline($pipeline) { - return $this->setParam('_pipeline', $pipeline); + return $this->setParam('pipeline', $pipeline); } /** @@ -293,7 +293,7 @@ public function setPipeline($pipeline) */ public function getPipeline() { - return $this->getParam('_pipeline'); + return $this->getParam('pipeline'); } /** @@ -301,7 +301,7 @@ public function getPipeline() */ public function hasPipeline() { - return $this->hasParam('_pipeline'); + return $this->hasParam('pipeline'); } /** diff --git a/test/Elastica/Bulk/Action/UpdateDocumentTest.php b/test/Elastica/Bulk/Action/UpdateDocumentTest.php index 496b550302..6da64da6be 100644 --- a/test/Elastica/Bulk/Action/UpdateDocumentTest.php +++ b/test/Elastica/Bulk/Action/UpdateDocumentTest.php @@ -76,7 +76,7 @@ public function testUpdateDocumentAsUpsert() $this->assertEquals('update', $action->getOpType()); $this->assertTrue($action->hasSource()); - $expected = '{"update":{"_index":"index","_type":"_doc","_id":1}}'."\n" + $expected = '{"update":{"_id":1,"_type":"_doc","_index":"index"}}'."\n" .'{"doc":{"foo":"bar"},"doc_as_upsert":true}'."\n"; $this->assertEquals($expected, $action->toString()); @@ -86,7 +86,7 @@ public function testUpdateDocumentAsUpsert() $document->setDocAsUpsert(false); $action->setDocument($document); - $expected = '{"update":{"_index":"index","_type":"_doc","_id":1}}'."\n" + $expected = '{"update":{"_id":1,"_type":"_doc","_index":"index"}}'."\n" .'{"doc":{"foo":"bar"}}'."\n"; $this->assertEquals($expected, $action->toString()); diff --git a/test/Elastica/BulkTest.php b/test/Elastica/BulkTest.php index 2b24642a31..efd0ed21b5 100644 --- a/test/Elastica/BulkTest.php +++ b/test/Elastica/BulkTest.php @@ -67,24 +67,24 @@ public function testSend() $data = $bulk->toArray(); $expected = [ - ['index' => ['_index' => $indexName, '_type' => '_doc', '_id' => 1]], + ['index' => ['_id' => 1, '_type' => '_doc', '_index' => $indexName]], ['name' => 'Mister Fantastic'], ['index' => ['_id' => 2]], ['name' => 'Invisible Woman'], - ['create' => ['_index' => $indexName, '_type' => '_doc', '_id' => 3]], + ['create' => ['_id' => 3, '_type' => '_doc', '_index' => $indexName]], ['name' => 'The Human Torch'], - ['index' => ['_index' => $indexName, '_type' => '_doc']], + ['index' => ['_type' => '_doc', '_index' => $indexName]], ['name' => 'The Thing'], ]; $this->assertEquals($expected, $data); - $expected = '{"index":{"_index":"'.$indexName.'","_type":"_doc","_id":1}} + $expected = '{"index":{"_id":1,"_type":"_doc","_index":"'.$indexName.'"}} {"name":"Mister Fantastic"} {"index":{"_id":2}} {"name":"Invisible Woman"} -{"create":{"_index":"'.$indexName.'","_type":"_doc","_id":3}} +{"create":{"_id":3,"_type":"_doc","_index":"'.$indexName.'"}} {"name":"The Human Torch"} -{"index":{"_index":"'.$indexName.'","_type":"_doc"}} +{"index":{"_type":"_doc","_index":"'.$indexName.'"}} {"name":"The Thing"} '; @@ -639,7 +639,7 @@ public function testRetry() $actions = $bulk->getActions(); $metadata = $actions[0]->getMetadata(); - $this->assertEquals(5, $metadata['_retry_on_conflict']); + $this->assertEquals(5, $metadata['retry_on_conflict']); $script = new Script(''); $script->setRetryOnConflict(5); @@ -650,7 +650,7 @@ public function testRetry() $actions = $bulk->getActions(); $metadata = $actions[0]->getMetadata(); - $this->assertEquals(5, $metadata['_retry_on_conflict']); + $this->assertEquals(5, $metadata['retry_on_conflict']); } /** diff --git a/test/Elastica/DocumentTest.php b/test/Elastica/DocumentTest.php index 9871d893c3..82b369935e 100644 --- a/test/Elastica/DocumentTest.php +++ b/test/Elastica/DocumentTest.php @@ -151,33 +151,30 @@ public function testGetOptions() $document->setParent('2'); $document->setId(1); - $options = $document->getOptions(['index', 'type', 'id', 'parent']); + $options = $document->getOptions(['_index', '_type', '_id', 'parent']); $this->assertInternalType('array', $options); $this->assertCount(3, $options); - $this->assertArrayHasKey('index', $options); - $this->assertArrayHasKey('id', $options); + $this->assertArrayHasKey('_index', $options); + $this->assertArrayHasKey('_id', $options); $this->assertArrayHasKey('parent', $options); - $this->assertEquals('index', $options['index']); - $this->assertEquals(1, $options['id']); + $this->assertEquals('index', $options['_index']); + $this->assertEquals(1, $options['_id']); $this->assertEquals('2', $options['parent']); - $this->assertArrayNotHasKey('type', $options); + $this->assertArrayNotHasKey('_type', $options); $this->assertArrayNotHasKey('op_type', $options); - $this->assertArrayNotHasKey('_index', $options); - $this->assertArrayNotHasKey('_id', $options); - $this->assertArrayNotHasKey('_parent', $options); + $this->assertArrayNotHasKey('index', $options); + $this->assertArrayNotHasKey('id', $options); - $options = $document->getOptions(['parent', 'op_type', 'percolate'], true); + $options = $document->getOptions(['parent', 'op_type', 'percolate']); $this->assertInternalType('array', $options); $this->assertCount(2, $options); - $this->assertArrayHasKey('_parent', $options); - $this->assertArrayHasKey('_op_type', $options); - $this->assertEquals('2', $options['_parent']); - $this->assertEquals('create', $options['_op_type']); + $this->assertArrayHasKey('parent', $options); + $this->assertArrayHasKey('op_type', $options); + $this->assertEquals('2', $options['parent']); + $this->assertEquals('create', $options['op_type']); $this->assertArrayNotHasKey('percolate', $options); - $this->assertArrayNotHasKey('op_type', $options); - $this->assertArrayNotHasKey('parent', $options); } /**