Skip to content

Commit bee987e

Browse files
committed
Merge branch '2018-03-16-es5'
2 parents d5a69df + bfe2841 commit bee987e

File tree

7 files changed

+245
-4
lines changed

7 files changed

+245
-4
lines changed

Document/Provider/AbstractDoctrineProvider.php

+5
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Doctrine\ORM\AbstractQuery;
66
use Doctrine\ORM\EntityManager;
77
use Doctrine\ORM\Query;
8+
use Sineflow\ElasticsearchBundle\Document\DocumentInterface;
89

910
/**
1011
* Base doctrine document provider
@@ -65,6 +66,7 @@ abstract public function getQuery();
6566
* Converts a Doctrine entity to Elasticsearch entity
6667
*
6768
* @param mixed $entity A doctrine entity object or data array
69+
*
6870
* @return mixed An ES document entity object or document array
6971
*/
7072
abstract protected function getAsDocument($entity);
@@ -74,6 +76,8 @@ abstract protected function getAsDocument($entity);
7476
* The returned data can be either a document entity or an array ready for direct sending to ES
7577
*
7678
* @return \Generator<DocumentInterface|array>
79+
*
80+
* @throws \Doctrine\Common\Persistence\Mapping\MappingException
7781
*/
7882
public function getDocuments()
7983
{
@@ -108,6 +112,7 @@ public function getDocuments()
108112
* The returned data can be either a document entity or an array ready for direct sending to ES
109113
*
110114
* @param int|string $id
115+
*
111116
* @return DocumentInterface|array
112117
*/
113118
public function getDocument($id)

Finder/Finder.php

+12-1
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public function __construct(
6565
* @param string $documentClass In short notation (i.e AppBundle:Product)
6666
* @param string $id
6767
* @param int $resultType
68+
*
6869
* @return mixed
6970
*/
7071
public function get($documentClass, $id, $resultType = self::RESULTS_OBJECT)
@@ -107,6 +108,7 @@ public function get($documentClass, $id, $resultType = self::RESULTS_OBJECT)
107108
* @param int $resultsType Bitmask value determining how the results are returned
108109
* @param array $additionalRequestParams Additional params to pass to the ES client's search() method
109110
* @param int $totalHits (out param) The total hits of the query response
111+
*
110112
* @return mixed
111113
*/
112114
public function find(array $documentClasses, array $searchBody, $resultsType = self::RESULTS_OBJECT, array $additionalRequestParams = [], &$totalHits = null)
@@ -157,6 +159,7 @@ public function find(array $documentClasses, array $searchBody, $resultsType = s
157159
* @param string $scrollTime The time to keep the scroll window open
158160
* @param int $resultsType Bitmask value determining how the results are returned
159161
* @param int $totalHits (out param) The total hits of the query response
162+
*
160163
* @return mixed
161164
*/
162165
public function scroll(array $documentClasses, &$scrollId, $scrollTime = self::SCROLL_TIME, $resultsType = self::RESULTS_OBJECT, &$totalHits = null)
@@ -183,6 +186,7 @@ public function scroll(array $documentClasses, &$scrollId, $scrollTime = self::S
183186
* @param array $documentClasses
184187
* @param array $searchBody
185188
* @param array $additionalRequestParams
189+
*
186190
* @return int
187191
*/
188192
public function count(array $documentClasses, array $searchBody = [], array $additionalRequestParams = [])
@@ -192,6 +196,10 @@ public function count(array $documentClasses, array $searchBody = [], array $add
192196
$params = $this->getTargetIndicesAndTypes($documentClasses);
193197

194198
if (!empty($searchBody)) {
199+
// Make sure sorting is not set in the query as it is not allowed for a count request
200+
// ES2 didn't mind, but ES5 with throw an exception
201+
unset($searchBody['sort']);
202+
195203
$params['body'] = $searchBody;
196204
}
197205

@@ -209,6 +217,7 @@ public function count(array $documentClasses, array $searchBody = [], array $add
209217
* based on the given document classes in short notation (AppBundle:Product)
210218
*
211219
* @param string[] $documentClasses
220+
*
212221
* @return array
213222
*/
214223
public function getTargetIndicesAndTypes(array $documentClasses)
@@ -271,6 +280,7 @@ public function parseResult($raw, $resultsType, array $documentClasses = null)
271280
* Returns a mapping of live indices and types to the document classes in short notation that represent them
272281
*
273282
* @param string[] $documentClasses
283+
*
274284
* @return TypesToDocumentClasses
275285
*/
276286
private function getTypesToDocumentClasses(array $documentClasses)
@@ -333,6 +343,7 @@ private function convertToNormalizedArray($data)
333343
* Verify that all types are in indices using the same connection object and return that object
334344
*
335345
* @param array $documentClasses
346+
*
336347
* @return ConnectionManager
337348
*/
338349
private function getConnection(array $documentClasses)
@@ -341,7 +352,7 @@ private function getConnection(array $documentClasses)
341352
foreach ($documentClasses as $documentClass) {
342353
$indexManagerName = $this->documentMetadataCollector->getDocumentClassIndex($documentClass);
343354
$indexManager = $this->indexManagerRegistry->get($indexManagerName);
344-
if (!is_null($connection) && $indexManager->getConnection()->getConnectionName() != $connection->getConnectionName()) {
355+
if (!is_null($connection) && $indexManager->getConnection()->getConnectionName() !== $connection->getConnectionName()) {
345356
throw new \InvalidArgumentException(sprintf('All searched types must be in indices within the same connection'));
346357
}
347358
$connection = $indexManager->getConnection();

Manager/IndexManager.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ public function rebuildIndex($deleteOld = false, $cancelExistingRebuild = false)
464464
$this->persist($document);
465465
}
466466
// Send the bulk request every X documents, so it doesn't get too big
467-
if ($i % $batchSize == 0) {
467+
if (0 === $i % $batchSize) {
468468
$this->getConnection()->commit();
469469
}
470470
$i++;

Tests/AbstractElasticsearchTestCase.php

-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ protected function populateElasticsearchWithData($indexManager, array $data)
6464
}
6565
try {
6666
$indexManager->getConnection()->commit();
67-
$indexManager->getConnection()->refresh();
6867
} catch (BulkRequestException $e) {
6968
print_r($e->getBulkResponseItems());
7069
throw $e;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php
2+
3+
namespace Functional\Document\Provider;
4+
5+
use Sineflow\ElasticsearchBundle\Document\Provider\ElasticsearchProvider;
6+
use Sineflow\ElasticsearchBundle\Tests\AbstractElasticsearchTestCase;
7+
8+
class ElasticsearchProviderTest extends AbstractElasticsearchTestCase
9+
{
10+
/**
11+
* {@inheritdoc}
12+
*/
13+
protected function getDataArray()
14+
{
15+
return [
16+
'bar' => [
17+
'AcmeBarBundle:Product' => [
18+
[
19+
'_id' => 1,
20+
'title' => 'Product 1',
21+
],
22+
[
23+
'_id' => 2,
24+
'title' => 'Product 2',
25+
],
26+
[
27+
'_id' => 3,
28+
'title' => 'Product 3',
29+
],
30+
],
31+
],
32+
];
33+
}
34+
35+
public function testGetDocument()
36+
{
37+
$esProvider = $this->getProvider();
38+
39+
$doc = $esProvider->getDocument(3);
40+
41+
$this->assertEquals([
42+
'_id' => 3,
43+
'title' => 'Product 3',
44+
], $doc);
45+
}
46+
47+
public function testGetDocuments()
48+
{
49+
$esProvider = $this->getProvider();
50+
51+
foreach ($esProvider->getDocuments() as $document) {
52+
$this->assertArrayHasKey('_id', $document);
53+
$this->assertArrayHasKey('title', $document);
54+
$ids[] = $document['_id'];
55+
}
56+
57+
// Since we're retrieving source docs ordered by _doc, they don't necessarily come in the order inserted
58+
// so we order them in order to compare
59+
sort($ids);
60+
61+
// Make sure all and exact documents were returned
62+
$this->assertEquals([1, 2, 3], $ids);
63+
}
64+
65+
private function getProvider()
66+
{
67+
return new ElasticsearchProvider(
68+
'AcmeBarBundle:Product',
69+
$this->getContainer()->get('sfes.document_metadata_collector'),
70+
$this->getIndexManager('bar'),
71+
'AcmeBarBundle:Product'
72+
);
73+
}
74+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
<?php
2+
3+
namespace ONGR\ElasticsearchBundle\Tests\Functional\Profiler;
4+
5+
use Sineflow\ElasticsearchBundle\Profiler\ElasticsearchProfiler;
6+
use Sineflow\ElasticsearchBundle\Tests\AbstractElasticsearchTestCase;
7+
use Sineflow\ElasticsearchBundle\Tests\app\fixture\Acme\BarBundle\Document\Product;
8+
use Symfony\Component\HttpFoundation\Request;
9+
use Symfony\Component\HttpFoundation\Response;
10+
11+
class ElasticsearchProfilerTest extends AbstractElasticsearchTestCase
12+
{
13+
/**
14+
* {@inheritdoc}
15+
*/
16+
protected function getDataArray()
17+
{
18+
return [
19+
'default' => [
20+
'product' => [
21+
[
22+
'_id' => 1,
23+
'title' => 'foo',
24+
],
25+
[
26+
'_id' => 2,
27+
'title' => 'bar',
28+
],
29+
[
30+
'_id' => 3,
31+
'title' => 'pizza',
32+
],
33+
],
34+
],
35+
];
36+
}
37+
38+
/**
39+
* Tests if right amount of queries are caught
40+
*/
41+
public function testGetQueryCount()
42+
{
43+
$imWithoutAliases = $this->getIndexManager('bar');
44+
$imWithoutAliases->getConnection()->setAutocommit(false);
45+
46+
// Setting up the test takes 3 queries:
47+
// 1. DELETE query to remove existing index,
48+
// 2. Internal call to GET /_aliases to check if index/alias already exists
49+
// 3. PUT request to create index
50+
$this->assertEquals(3, $this->getCollector()->getQueryCount());
51+
52+
$product = new Product();
53+
$product->title = 'tuna';
54+
$imWithoutAliases->persist($product);
55+
$imWithoutAliases->getConnection()->commit();
56+
57+
// It takes 3 more queries to insert the new document:
58+
// 1. GET /sineflow-esb-test-bar/_alias to check if more than one index should be written to
59+
// 2. POST bulk request to enter the data
60+
// 3. GET /_refresh, because of the $forceRefresh param of ->commit()
61+
$this->assertEquals(6, $this->getCollector()->getQueryCount());
62+
}
63+
64+
/**
65+
* Tests if correct time is being returned.
66+
*/
67+
public function testGetTime()
68+
{
69+
$imWithoutAliases = $this->getIndexManager('bar');
70+
$imWithoutAliases->getConnection()->setAutocommit(true);
71+
$imWithoutAliases->getRepository('AcmeBarBundle:Product')->getById(3);
72+
73+
$this->assertGreaterThan(0.0, $this->getCollector()->getTime(), 'Time should be greater than 0ms');
74+
$this->assertLessThan(1000.0, $this->getCollector()->getTime(), 'Time should be less than 1s');
75+
}
76+
77+
/**
78+
* Tests if logged data seems correct
79+
*/
80+
public function testCorrectDataLogged()
81+
{
82+
$imWithoutAliases = $this->getIndexManager('bar');
83+
$imWithoutAliases->getConnection()->setAutocommit(true);
84+
$imWithoutAliases->getRepository('AcmeBarBundle:Product')->getById(3);
85+
86+
87+
$queries = $this->getCollector()->getQueries();
88+
89+
$lastQuery = end($queries[ElasticsearchProfiler::UNDEFINED_ROUTE]);
90+
$this->checkQueryParameters($lastQuery);
91+
92+
$esHostAndPort = explode(':', $imWithoutAliases->getConnection()->getConnectionSettings()['hosts'][0]);
93+
94+
$this->assertArraySubset(
95+
[
96+
"curlRequest" => "curl -XGET 'http://".implode(':', $esHostAndPort)."/sineflow-esb-test-bar/product/3?pretty=true'",
97+
"senseRequest" => "GET /sineflow-esb-test-bar/product/3",
98+
"backtrace" => null,
99+
"scheme" => "http",
100+
"host" => $esHostAndPort[0],
101+
"port" => (int) $esHostAndPort[1],
102+
"path" => "/sineflow-esb-test-bar/product/3",
103+
],
104+
$lastQuery,
105+
'Logged data did not match expected data.'
106+
);
107+
}
108+
109+
/**
110+
* Checks query parameters that are not static.
111+
*
112+
* @param array $query
113+
*/
114+
public function checkQueryParameters($query)
115+
{
116+
$this->assertArrayHasKey('time', $query, 'Query should have time set.');
117+
$this->assertGreaterThan(0.0, $query['time'], 'Time should be greater than 0');
118+
119+
$this->assertArrayHasKey('curlRequest', $query, 'Query should have curlRequest set.');
120+
$this->assertNotEmpty($query['curlRequest'], 'curlRequest should not be empty');
121+
122+
$this->assertArrayHasKey('senseRequest', $query, 'Query should have senseRequest set.');
123+
$this->assertNotEmpty($query['senseRequest'], 'senseRequest should not be empty.');
124+
125+
$this->assertArrayHasKey('backtrace', $query, 'Query should have backtrace set.');
126+
127+
$this->assertArrayHasKey('scheme', $query, 'Query should have scheme set.');
128+
$this->assertNotEmpty($query['scheme'], 'scheme should not be empty.');
129+
130+
$this->assertArrayHasKey('host', $query, 'Query should have host set.');
131+
$this->assertNotEmpty($query['host'], 'Host should not be empty');
132+
133+
$this->assertArrayHasKey('port', $query, 'Query should have port set.');
134+
$this->assertNotEmpty($query['port'], 'port should not be empty.');
135+
136+
$this->assertArrayHasKey('path', $query, 'Query should have host path set.');
137+
$this->assertNotEmpty($query['path'], 'Path should not be empty.');
138+
}
139+
140+
/**
141+
* @return ElasticsearchProfiler
142+
*/
143+
private function getCollector()
144+
{
145+
$collector = $this->getContainer()->get('sfes.profiler');
146+
$collector->collect(new Request(), new Response());
147+
148+
return $collector;
149+
}
150+
}

composer.json

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,16 @@
2222
"require-dev": {
2323
"monolog/monolog": "~1.0",
2424
"knplabs/knp-paginator-bundle": "~2.7",
25+
"doctrine/orm": "~2.5",
2526
"mikey179/vfsStream": "~1.6",
2627
"phpunit/phpunit": "~6.0",
2728
"squizlabs/php_codesniffer": "3.*",
2829
"satooshi/php-coveralls": "~2.0"
2930
},
3031
"suggest": {
3132
"monolog/monolog": "Allows for client-level logging and tracing",
32-
"knplabs/knp-paginator-bundle": "Allows for search results to be paginated"
33+
"knplabs/knp-paginator-bundle": "Allows for search results to be paginated",
34+
"doctrine/orm": "Allows for using Doctrine as source for rebuilding indices"
3335
},
3436
"autoload": {
3537
"psr-4": { "Sineflow\\ElasticsearchBundle\\": "" }

0 commit comments

Comments
 (0)