Skip to content

Commit 95d0bbc

Browse files
kitsunetkitsunet
committed
BUGFIX: Setting injection doesn't cause errors
Due to the order of operations with injectSettings and object injection it could cause errors to resolve the index settings early enough to override the name property. This is now resolved later. Additionally via `Index::getOriginalName()` one can now obtain the name without prefix, if necessary. BUGFIX: Index and testing overhauled
1 parent b07f718 commit 95d0bbc

File tree

4 files changed

+219
-47
lines changed

4 files changed

+219
-47
lines changed

‎Classes/Domain/Model/Index.php‎

Lines changed: 69 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -29,39 +29,45 @@ class Index
2929
* @see http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/indices-update-settings.html
3030
*/
3131
static protected $updatableSettings = [
32-
'index.number_of_replicas',
33-
'index.auto_expand_replicas',
34-
'index.blocks.read_only',
35-
'index.blocks.read',
36-
'index.blocks.write',
37-
'index.blocks.metadata',
38-
'index.refresh_interval',
39-
'index.index_concurrency',
40-
'index.codec',
41-
'index.codec.bloom.load',
42-
'index.fail_on_merge_failure',
43-
'index.translog.flush_threshold_ops',
44-
'index.translog.flush_threshold_size',
45-
'index.translog.flush_threshold_period',
46-
'index.translog.disable_flush',
47-
'index.cache.filter.max_size',
48-
'index.cache.filter.expire',
49-
'index.gateway.snapshot_interval',
50-
'index.routing.allocation.include',
51-
'index.routing.allocation.exclude',
52-
'index.routing.allocation.require',
53-
'index.routing.allocation.disable_allocation',
54-
'index.routing.allocation.disable_new_allocation',
55-
'index.routing.allocation.disable_replica_allocation',
56-
'index.routing.allocation.enable',
57-
'index.routing.allocation.total_shards_per_node',
58-
'index.recovery.initial_shards',
59-
'index.gc_deletes',
60-
'index.ttl.disable_purge',
61-
'index.translog.fs.type',
62-
'index.compound_format',
63-
'index.compound_on_flush',
64-
'index.warmer.enabled',
32+
'settings.index.number_of_replicas',
33+
'settings.index.auto_expand_replicas',
34+
'settings.index.blocks.read_only',
35+
'settings.index.blocks.read',
36+
'settings.index.blocks.write',
37+
'settings.index.blocks.metadata',
38+
'settings.index.refresh_interval',
39+
'settings.index.index_concurrency',
40+
'settings.index.codec',
41+
'settings.index.codec.bloom.load',
42+
'settings.index.fail_on_merge_failure',
43+
'settings.index.translog.flush_threshold_ops',
44+
'settings.index.translog.flush_threshold_size',
45+
'settings.index.translog.flush_threshold_period',
46+
'settings.index.translog.disable_flush',
47+
'settings.index.cache.filter.max_size',
48+
'settings.index.cache.filter.expire',
49+
'settings.index.gateway.snapshot_interval',
50+
'settings.index.routing.allocation.include',
51+
'settings.index.routing.allocation.exclude',
52+
'settings.index.routing.allocation.require',
53+
'settings.index.routing.allocation.disable_allocation',
54+
'settings.index.routing.allocation.disable_new_allocation',
55+
'settings.index.routing.allocation.disable_replica_allocation',
56+
'settings.index.routing.allocation.enable',
57+
'settings.index.routing.allocation.total_shards_per_node',
58+
'settings.index.recovery.initial_shards',
59+
'settings.index.gc_deletes',
60+
'settings.index.ttl.disable_purge',
61+
'settings.index.translog.fs.type',
62+
'settings.index.compound_format',
63+
'settings.index.compound_on_flush',
64+
'settings.index.warmer.enabled',
65+
];
66+
67+
static protected $allowedIndexCreateKeys = [
68+
'settings',
69+
'aliases',
70+
'mappings'
6571
];
6672

6773
/**
@@ -122,13 +128,6 @@ public function __construct(string $name, Client $client = null)
122128
public function injectSettings(array $settings): void
123129
{
124130
$this->settings = $settings;
125-
$indexSettings = $this->getSettings();
126-
if (!isset($indexSettings['prefix']) || empty($indexSettings['prefix'])) {
127-
return;
128-
}
129-
// This is obviously a side effect but can only be done after injecting settings
130-
// and it needs to be done as early as possible
131-
$this->name = $settings['prefix'] . '-' . $this->name;
132131
}
133132

134133
/**
@@ -141,7 +140,7 @@ public function findType(string $typeName): AbstractType
141140
}
142141

143142
/**
144-
* @param array <AbstractType> $types
143+
* @param array<AbstractType> $types
145144
* @return TypeGroup
146145
*/
147146
public function findTypeGroup(array $types): TypeGroup
@@ -173,11 +172,11 @@ public function exists(): bool
173172
public function request(string $method, string $path = null, array $arguments = [], $content = null, bool $prefixIndex = true): Response
174173
{
175174
if ($this->client === null) {
176-
throw new ElasticSearchException('The client of the index "' . $this->name . '" is not set, hence no requests can be done.', 1566313883);
175+
throw new ElasticSearchException('The client of the index "' . $this->prefixName() . '" is not set, hence no requests can be done.', 1566313883);
177176
}
178177
$path = ltrim($path ? trim($path) : '', '/');
179178
if ($prefixIndex === true) {
180-
$path = '/' . $this->name . '/' . $path;
179+
$path = '/' . $this->prefixName() . '/' . $path;
181180
} else {
182181
$path = '/' . ltrim($path, '/');
183182
}
@@ -191,7 +190,9 @@ public function request(string $method, string $path = null, array $arguments =
191190
*/
192191
public function create(): void
193192
{
194-
$this->request('PUT', null, [], json_encode($this->getSettings()));
193+
$indexSettings = $this->getSettings();
194+
$creationSettings = array_filter($indexSettings, static fn($key) => in_array($key, self::$allowedIndexCreateKeys, true), ARRAY_FILTER_USE_KEY);
195+
$this->request('PUT', null, [], json_encode($creationSettings));
195196
}
196197

197198
/**
@@ -206,7 +207,7 @@ protected function getSettings(): ?array
206207
}
207208

208209
$settings = Arrays::getValueByPath($this->settings, $path);
209-
return $settings !== null ? $this->dynamicIndexSettingService->process($settings, $path, $this->getName()) : $settings;
210+
return $settings !== null ? $this->dynamicIndexSettingService->process($settings, $path, $this->name) : $settings;
210211
}
211212

212213
/**
@@ -223,7 +224,9 @@ public function updateSettings(): void
223224
$updatableSettings = Arrays::setValueByPath($updatableSettings, $settingPath, $setting);
224225
}
225226
}
226-
$this->request('PUT', '/_settings', [], json_encode($updatableSettings));
227+
if ($updatableSettings !== []) {
228+
$this->request('PUT', '/_settings', [], json_encode($updatableSettings));
229+
}
227230
}
228231

229232
/**
@@ -252,6 +255,11 @@ public function refresh(): Response
252255
* @return string
253256
*/
254257
public function getName(): string
258+
{
259+
return $this->prefixName();
260+
}
261+
262+
public function getOriginalName(): string
255263
{
256264
return $this->name;
257265
}
@@ -273,4 +281,19 @@ public function setSettingsKey(string $settingsKey): void
273281
{
274282
$this->settingsKey = $settingsKey;
275283
}
284+
285+
/**
286+
* Prepends configured preset to the base index name
287+
*
288+
* @return string
289+
*/
290+
private function prefixName(): string
291+
{
292+
$indexSettings = $this->getSettings();
293+
if (!isset($indexSettings['prefix']) || empty($indexSettings['prefix'])) {
294+
return $this->name;
295+
}
296+
297+
return $indexSettings['prefix'] . '-' . $this->name;
298+
}
276299
}

‎Configuration/Testing/Settings.yaml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
Flowpack:
2+
ElasticSearch:
3+
clients:
4+
FunctionalTests:
5+
-
6+
host: localhost
7+
port: 9200
8+
scheme: 'http'
9+
username: ''
10+
password: ''
11+
realtimeIndexing:
12+
# we also use this setting for the object indexer client bundle
13+
client: FunctionalTests
14+
15+
indexes:
16+
FunctionalTests: # Configuration bundle name
17+
index_with_prefix: # The index prefix name, must be the same as in the Neos.ContentRepository.Search.elasticSearch.indexName setting
18+
prefix: 'prefix'
19+
settings:
20+
index:
21+
number_of_replicas: 1
22+
soft_deletes:
23+
enabled: true
24+
index_without_prefix:
25+
settings:
26+
index:
27+
number_of_replicas: 2
28+
soft_deletes:
29+
enabled: true

‎Tests/Functional/Domain/AbstractTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ final public function setUp(): void
4040
parent::setUp();
4141

4242
$this->clientFactory = $this->objectManager->get(ClientFactory::class);
43-
$client = $this->clientFactory->create();
43+
$client = $this->clientFactory->create("FunctionalTests");
4444
$this->testingIndex = $client->findIndex('flow_elasticsearch_functionaltests');
4545

4646
if ($this->testingIndex->exists()) {

‎Tests/Functional/Domain/IndexTest.php‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
<?php
2+
namespace Flowpack\ElasticSearch\Tests\Functional\Domain;
3+
4+
/*
5+
* This file is part of the Flowpack.ElasticSearch package.
6+
*
7+
* (c) Contributors of the Flowpack Team - flowpack.org
8+
*
9+
* This package is Open Source Software. For the full copyright and license
10+
* information, please view the LICENSE file which was distributed with this
11+
* source code.
12+
*/
13+
14+
use Flowpack\ElasticSearch\Domain\Model\Client;
15+
use Flowpack\ElasticSearch\Transfer\Response;
16+
use Neos\Flow\Tests\FunctionalTestCase;
17+
use Flowpack\ElasticSearch\Domain\Model\Index;
18+
19+
class IndexTest extends FunctionalTestCase
20+
{
21+
/**
22+
* @test
23+
*/
24+
public function indexWithoutPrefix()
25+
{
26+
$clientMock = $this->createMock(Client::class);
27+
$clientMock->method('getBundle')
28+
->willReturn('FunctionalTests');
29+
30+
$clientMock->expects($this->exactly(2))->method('request')
31+
->withConsecutive(
32+
[
33+
'PUT',
34+
'/index_without_prefix/',
35+
[],
36+
json_encode([
37+
'settings' => [
38+
'index' => [
39+
'number_of_replicas' => 2,
40+
'soft_deletes' => [
41+
'enabled' => true
42+
]
43+
]
44+
]
45+
], JSON_THROW_ON_ERROR)
46+
],
47+
// updateSettings should correctly filter soft_deletes as it's not in the allow list
48+
[
49+
'PUT',
50+
'/index_without_prefix/_settings',
51+
[],
52+
json_encode([
53+
'settings' => [
54+
'index' => [
55+
'number_of_replicas' => 2
56+
]
57+
]
58+
], JSON_THROW_ON_ERROR)
59+
]
60+
)
61+
->willReturn($this->createStub(Response::class));
62+
63+
$testObject = new Index('index_without_prefix', $clientMock);
64+
$testObject->create();
65+
$testObject->updateSettings();
66+
67+
static::assertSame('index_without_prefix', $testObject->getOriginalName());
68+
static::assertSame('index_without_prefix', $testObject->getName());
69+
}
70+
71+
/**
72+
* @test
73+
*/
74+
public function indexWithPrefix()
75+
{
76+
$clientMock = $this->createMock(Client::class);
77+
$clientMock->method('getBundle')
78+
->willReturn('FunctionalTests');
79+
80+
$clientMock->expects($this->exactly(2))->method('request')
81+
->withConsecutive(
82+
[
83+
'PUT',
84+
'/prefix-index_with_prefix/',
85+
[],
86+
json_encode([
87+
'settings' => [
88+
'index' => [
89+
'number_of_replicas' => 1,
90+
'soft_deletes' => [
91+
'enabled' => true
92+
]
93+
]
94+
]
95+
], JSON_THROW_ON_ERROR)
96+
],
97+
// updateSettings should correctly filter soft_deletes as it's not in the allow list
98+
[
99+
'PUT',
100+
'/prefix-index_with_prefix/_settings',
101+
[],
102+
json_encode([
103+
'settings' => [
104+
'index' => [
105+
'number_of_replicas' => 1
106+
]
107+
]
108+
], JSON_THROW_ON_ERROR)
109+
]
110+
)
111+
->willReturn($this->createStub(Response::class));
112+
113+
$testObject = new Index('index_with_prefix', $clientMock);
114+
$testObject->create();
115+
$testObject->updateSettings();
116+
117+
static::assertSame('index_with_prefix', $testObject->getOriginalName());
118+
static::assertSame('prefix-index_with_prefix', $testObject->getName());
119+
}
120+
}

0 commit comments

Comments
 (0)