Skip to content

Commit 29f63c7

Browse files
rjernstrjernst
authored
Internal: Convert empty and size checks of settings to not use getAsMap() (#22890)
With the new secure settings, methods like getAsMap() no longer work correctly as a means of checking for empty settings, or the total size. This change converts the existing uses of that method to use methods directly on Settings. Note this does not update the implementations to account for SecureSettings, as that will require a followup which changes how secure settings work.
1 parent f6d38d4 commit 29f63c7

File tree

16 files changed

+51
-47
lines changed

16 files changed

+51
-47
lines changed

‎core/src/main/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public ClusterUpdateSettingsRequest() {
5151
@Override
5252
public ActionRequestValidationException validate() {
5353
ActionRequestValidationException validationException = null;
54-
if (transientSettings.getAsMap().isEmpty() && persistentSettings.getAsMap().isEmpty()) {
54+
if (transientSettings.isEmpty() && persistentSettings.isEmpty()) {
5555
validationException = addValidationError("no settings to update", validationException);
5656
}
5757
return validationException;

‎core/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,10 @@ protected String executor() {
6767
@Override
6868
protected ClusterBlockException checkBlock(ClusterUpdateSettingsRequest request, ClusterState state) {
6969
// allow for dedicated changes to the metadata blocks, so we don't block those to allow to "re-enable" it
70-
if ((request.transientSettings().getAsMap().isEmpty() &&
71-
request.persistentSettings().getAsMap().size() == 1 &&
70+
if ((request.transientSettings().isEmpty() &&
71+
request.persistentSettings().size() == 1 &&
7272
MetaData.SETTING_READ_ONLY_SETTING.exists(request.persistentSettings())) ||
73-
(request.persistentSettings().getAsMap().isEmpty() && request.transientSettings().getAsMap().size() == 1 &&
73+
(request.persistentSettings().isEmpty() && request.transientSettings().size() == 1 &&
7474
MetaData.SETTING_READ_ONLY_SETTING.exists(request.transientSettings()))) {
7575
return null;
7676
}

‎core/src/main/java/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ protected ClusterBlockException checkBlock(UpdateSettingsRequest request, Cluste
6262
if (globalBlock != null) {
6363
return globalBlock;
6464
}
65-
if (request.settings().getAsMap().size() == 1 && IndexMetaData.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings()) || IndexMetaData.INDEX_READ_ONLY_SETTING.exists(request.settings())) {
65+
if (request.settings().size() == 1 && IndexMetaData.INDEX_BLOCKS_METADATA_SETTING.exists(request.settings()) || IndexMetaData.INDEX_READ_ONLY_SETTING.exists(request.settings())) {
6666
return null;
6767
}
6868
return state.blocks().indicesBlockedException(ClusterBlockLevel.METADATA_WRITE, indexNameExpressionResolver.concreteIndexNames(state, request));

‎core/src/main/java/org/elasticsearch/action/admin/indices/settings/put/UpdateSettingsRequest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public UpdateSettingsRequest(Settings settings, String... indices) {
7070
@Override
7171
public ActionRequestValidationException validate() {
7272
ActionRequestValidationException validationException = null;
73-
if (settings.getAsMap().isEmpty()) {
73+
if (settings.isEmpty()) {
7474
validationException = addValidationError("no settings to update", validationException);
7575
}
7676
return validationException;

‎core/src/main/java/org/elasticsearch/cluster/metadata/MetaData.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -973,15 +973,15 @@ public static void toXContent(MetaData metaData, XContentBuilder builder, ToXCon
973973
builder.field("version", metaData.version());
974974
builder.field("cluster_uuid", metaData.clusterUUID);
975975

976-
if (!metaData.persistentSettings().getAsMap().isEmpty()) {
976+
if (!metaData.persistentSettings().isEmpty()) {
977977
builder.startObject("settings");
978978
for (Map.Entry<String, String> entry : metaData.persistentSettings().getAsMap().entrySet()) {
979979
builder.field(entry.getKey(), entry.getValue());
980980
}
981981
builder.endObject();
982982
}
983983

984-
if (context == XContentContext.API && !metaData.transientSettings().getAsMap().isEmpty()) {
984+
if (context == XContentContext.API && !metaData.transientSettings().isEmpty()) {
985985
builder.startObject("transient_settings");
986986
for (Map.Entry<String, String> entry : metaData.transientSettings().getAsMap().entrySet()) {
987987
builder.field(entry.getKey(), entry.getValue());

‎core/src/main/java/org/elasticsearch/cluster/metadata/MetaDataUpdateSettingsService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ public ClusterState execute(ClusterState currentState) {
218218
closeIndices
219219
));
220220
}
221-
if (!skippedSettigns.getAsMap().isEmpty() && !openIndices.isEmpty()) {
221+
if (!skippedSettigns.isEmpty() && !openIndices.isEmpty()) {
222222
throw new IllegalArgumentException(String.format(Locale.ROOT,
223223
"Can't update non dynamic settings [%s] for open indices %s",
224224
skippedSettigns.getAsMap().keySet(),

‎core/src/main/java/org/elasticsearch/common/settings/Settings.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.elasticsearch.common.io.stream.StreamInput;
2828
import org.elasticsearch.common.io.stream.StreamOutput;
2929
import org.elasticsearch.common.logging.DeprecationLogger;
30-
import org.elasticsearch.common.logging.Loggers;
3130
import org.elasticsearch.common.settings.loader.SettingsLoader;
3231
import org.elasticsearch.common.settings.loader.SettingsLoaderFactory;
3332
import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -589,7 +588,7 @@ public static Settings readSettingsFromStream(StreamInput in) throws IOException
589588
}
590589

591590
public static void writeSettingsToStream(Settings settings, StreamOutput out) throws IOException {
592-
out.writeVInt(settings.getAsMap().size());
591+
out.writeVInt(settings.size());
593592
for (Map.Entry<String, String> entry : settings.getAsMap().entrySet()) {
594593
out.writeString(entry.getKey());
595594
out.writeOptionalString(entry.getValue());
@@ -626,7 +625,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
626625
* @return <tt>true</tt> if this settings object contains no settings
627626
*/
628627
public boolean isEmpty() {
629-
return this.settings.isEmpty();
628+
return this.settings.isEmpty(); // TODO: account for secure settings
629+
}
630+
631+
/** Returns the number of settings in this settings object. */
632+
public int size() {
633+
return this.settings.size(); // TODO: account for secure settings
630634
}
631635

632636
/**

‎core/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetSettingsAction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ public RestResponse buildResponse(GetSettingsResponse getSettingsResponse, XCont
7575
builder.startObject();
7676
for (ObjectObjectCursor<String, Settings> cursor : getSettingsResponse.getIndexToSettings()) {
7777
// no settings, jump over it to shorten the response data
78-
if (cursor.value.getAsMap().isEmpty()) {
78+
if (cursor.value.isEmpty()) {
7979
continue;
8080
}
8181
builder.startObject(cursor.key);

‎core/src/test/java/org/elasticsearch/cluster/metadata/ToAndFromJsonMetaDataTests.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
155155
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
156156
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
157157
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
158-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(3));
158+
assertThat(indexMetaData.getSettings().size(), equalTo(3));
159159
assertThat(indexMetaData.getMappings().size(), equalTo(0));
160160

161161
indexMetaData = parsedMetaData.index("test2");
@@ -164,7 +164,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
164164
assertThat(indexMetaData.primaryTerm(0), equalTo(2L));
165165
assertThat(indexMetaData.primaryTerm(1), equalTo(2L));
166166
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
167-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(5));
167+
assertThat(indexMetaData.getSettings().size(), equalTo(5));
168168
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
169169
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
170170
assertThat(indexMetaData.getMappings().size(), equalTo(0));
@@ -173,22 +173,22 @@ public void testSimpleJsonFromAndTo() throws IOException {
173173
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
174174
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
175175
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
176-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(3));
176+
assertThat(indexMetaData.getSettings().size(), equalTo(3));
177177
assertThat(indexMetaData.getMappings().size(), equalTo(1));
178178
assertThat(indexMetaData.getMappings().get("mapping1").source().string(), equalTo(MAPPING_SOURCE1));
179179

180180
indexMetaData = parsedMetaData.index("test4");
181181
assertThat(indexMetaData.getCreationDate(), equalTo(2L));
182182
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
183183
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
184-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(4));
184+
assertThat(indexMetaData.getSettings().size(), equalTo(4));
185185
assertThat(indexMetaData.getMappings().size(), equalTo(0));
186186

187187
indexMetaData = parsedMetaData.index("test5");
188188
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
189189
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
190190
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
191-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(5));
191+
assertThat(indexMetaData.getSettings().size(), equalTo(5));
192192
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
193193
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
194194
assertThat(indexMetaData.getMappings().size(), equalTo(2));
@@ -199,7 +199,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
199199
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
200200
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
201201
assertThat(indexMetaData.getCreationDate(), equalTo(2L));
202-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(6));
202+
assertThat(indexMetaData.getSettings().size(), equalTo(6));
203203
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
204204
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
205205
assertThat(indexMetaData.getMappings().size(), equalTo(0));
@@ -208,7 +208,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
208208
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
209209
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
210210
assertThat(indexMetaData.getCreationDate(), equalTo(2L));
211-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(4));
211+
assertThat(indexMetaData.getSettings().size(), equalTo(4));
212212
assertThat(indexMetaData.getMappings().size(), equalTo(2));
213213
assertThat(indexMetaData.getMappings().get("mapping1").source().string(), equalTo(MAPPING_SOURCE1));
214214
assertThat(indexMetaData.getMappings().get("mapping2").source().string(), equalTo(MAPPING_SOURCE2));
@@ -217,7 +217,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
217217
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
218218
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
219219
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
220-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(5));
220+
assertThat(indexMetaData.getSettings().size(), equalTo(5));
221221
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
222222
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
223223
assertThat(indexMetaData.getMappings().size(), equalTo(2));
@@ -231,7 +231,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
231231
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
232232
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
233233
assertThat(indexMetaData.getCreationDate(), equalTo(2L));
234-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(6));
234+
assertThat(indexMetaData.getSettings().size(), equalTo(6));
235235
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
236236
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
237237
assertThat(indexMetaData.getMappings().size(), equalTo(2));
@@ -245,7 +245,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
245245
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
246246
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
247247
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
248-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(5));
248+
assertThat(indexMetaData.getSettings().size(), equalTo(5));
249249
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
250250
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
251251
assertThat(indexMetaData.getMappings().size(), equalTo(2));
@@ -259,7 +259,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
259259
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
260260
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
261261
assertThat(indexMetaData.getCreationDate(), equalTo(-1L));
262-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(5));
262+
assertThat(indexMetaData.getSettings().size(), equalTo(5));
263263
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
264264
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
265265
assertThat(indexMetaData.getMappings().size(), equalTo(2));
@@ -277,7 +277,7 @@ public void testSimpleJsonFromAndTo() throws IOException {
277277
assertThat(indexMetaData.getNumberOfShards(), equalTo(1));
278278
assertThat(indexMetaData.getNumberOfReplicas(), equalTo(2));
279279
assertThat(indexMetaData.getCreationDate(), equalTo(2L));
280-
assertThat(indexMetaData.getSettings().getAsMap().size(), equalTo(6));
280+
assertThat(indexMetaData.getSettings().size(), equalTo(6));
281281
assertThat(indexMetaData.getSettings().get("setting1"), equalTo("value1"));
282282
assertThat(indexMetaData.getSettings().get("setting2"), equalTo("value2"));
283283
assertThat(indexMetaData.getMappings().size(), equalTo(2));

‎core/src/test/java/org/elasticsearch/common/settings/ScopedSettingsTests.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -270,21 +270,21 @@ public void testDiff() throws IOException {
270270
ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(fooBar, fooBarBaz, foorBarQuux,
271271
someGroup, someAffix)));
272272
Settings diff = settings.diff(Settings.builder().put("foo.bar", 5).build(), Settings.EMPTY);
273-
assertEquals(4, diff.getAsMap().size()); // 4 since foo.bar.quux has 3 values essentially
273+
assertEquals(4, diff.size()); // 4 since foo.bar.quux has 3 values essentially
274274
assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1));
275275
assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"a", "b", "c"});
276276

277277
diff = settings.diff(
278278
Settings.builder().put("foo.bar", 5).build(),
279279
Settings.builder().put("foo.bar.baz", 17).putArray("foo.bar.quux", "d", "e", "f").build());
280-
assertEquals(4, diff.getAsMap().size()); // 4 since foo.bar.quux has 3 values essentially
280+
assertEquals(4, diff.size()); // 4 since foo.bar.quux has 3 values essentially
281281
assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(17));
282282
assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"d", "e", "f"});
283283

284284
diff = settings.diff(
285285
Settings.builder().put("some.group.foo", 5).build(),
286286
Settings.builder().put("some.group.foobar", 17, "some.group.foo", 25).build());
287-
assertEquals(6, diff.getAsMap().size()); // 6 since foo.bar.quux has 3 values essentially
287+
assertEquals(6, diff.size()); // 6 since foo.bar.quux has 3 values essentially
288288
assertThat(diff.getAsInt("some.group.foobar", null), equalTo(17));
289289
assertNull(diff.get("some.group.foo"));
290290
assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"a", "b", "c"});
@@ -295,7 +295,7 @@ public void testDiff() throws IOException {
295295
Settings.builder().put("some.prefix.foo.somekey", 5).build(),
296296
Settings.builder().put("some.prefix.foobar.somekey", 17,
297297
"some.prefix.foo.somekey", 18).build());
298-
assertEquals(6, diff.getAsMap().size()); // 6 since foo.bar.quux has 3 values essentially
298+
assertEquals(6, diff.size()); // 6 since foo.bar.quux has 3 values essentially
299299
assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17));
300300
assertNull(diff.get("some.prefix.foo.somekey"));
301301
assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"a", "b", "c"});
@@ -314,21 +314,21 @@ public void testDiffWithAffixAndComplexMatcher() {
314314
ClusterSettings settings = new ClusterSettings(Settings.EMPTY, new HashSet<>(Arrays.asList(fooBar, fooBarBaz, foorBarQuux,
315315
someGroup, someAffix)));
316316
Settings diff = settings.diff(Settings.builder().put("foo.bar", 5).build(), Settings.EMPTY);
317-
assertEquals(1, diff.getAsMap().size());
317+
assertEquals(1, diff.size());
318318
assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(1));
319319
assertNull(diff.getAsArray("foo.bar.quux", null)); // affix settings don't know their concrete keys
320320

321321
diff = settings.diff(
322322
Settings.builder().put("foo.bar", 5).build(),
323323
Settings.builder().put("foo.bar.baz", 17).putArray("foo.bar.quux", "d", "e", "f").build());
324-
assertEquals(4, diff.getAsMap().size());
324+
assertEquals(4, diff.size());
325325
assertThat(diff.getAsInt("foo.bar.baz", null), equalTo(17));
326326
assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"d", "e", "f"});
327327

328328
diff = settings.diff(
329329
Settings.builder().put("some.group.foo", 5).build(),
330330
Settings.builder().put("some.group.foobar", 17, "some.group.foo", 25).build());
331-
assertEquals(3, diff.getAsMap().size());
331+
assertEquals(3, diff.size());
332332
assertThat(diff.getAsInt("some.group.foobar", null), equalTo(17));
333333
assertNull(diff.get("some.group.foo"));
334334
assertNull(diff.getAsArray("foo.bar.quux", null)); // affix settings don't know their concrete keys
@@ -339,7 +339,7 @@ public void testDiffWithAffixAndComplexMatcher() {
339339
Settings.builder().put("some.prefix.foo.somekey", 5).build(),
340340
Settings.builder().put("some.prefix.foobar.somekey", 17,
341341
"some.prefix.foo.somekey", 18).build());
342-
assertEquals(3, diff.getAsMap().size());
342+
assertEquals(3, diff.size());
343343
assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17));
344344
assertNull(diff.get("some.prefix.foo.somekey"));
345345
assertNull(diff.getAsArray("foo.bar.quux", null)); // affix settings don't know their concrete keys
@@ -353,7 +353,7 @@ public void testDiffWithAffixAndComplexMatcher() {
353353
.putArray("foo.bar.quux", "x", "y", "z")
354354
.putArray("foo.baz.quux", "d", "e", "f")
355355
.build());
356-
assertEquals(9, diff.getAsMap().size());
356+
assertEquals(9, diff.size());
357357
assertThat(diff.getAsInt("some.prefix.foobar.somekey", null), equalTo(17));
358358
assertNull(diff.get("some.prefix.foo.somekey"));
359359
assertArrayEquals(diff.getAsArray("foo.bar.quux", null), new String[] {"x", "y", "z"});

0 commit comments

Comments
 (0)