Skip to content

Commit 9946abf

Browse files
danielmitterdorferdanielmitterdorfer
authored
Deprecate lenient booleans (#22716)
Elasticsearch 6.0 removes support for lenient booleans (see #22000). With this commit we deprecate all usages of non-strict booleans in Elasticsearch 5.x so users can already spot improper usages. Relates #22000 Relates #22696
1 parent 23c9a87 commit 9946abf

File tree

81 files changed

+707
-353
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

81 files changed

+707
-353
lines changed

‎buildSrc/src/main/resources/checkstyle_suppressions.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,6 @@
656656
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]settings[/\\]ClusterSettingsIT.java" checks="LineLength" />
657657
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]shards[/\\]ClusterSearchShardsIT.java" checks="LineLength" />
658658
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]cluster[/\\]structure[/\\]RoutingIteratorTests.java" checks="LineLength" />
659-
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]BooleansTests.java" checks="LineLength" />
660659
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]FsBlobStoreContainerTests.java" checks="LineLength" />
661660
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]blobstore[/\\]FsBlobStoreTests.java" checks="LineLength" />
662661
<suppress files="core[/\\]src[/\\]test[/\\]java[/\\]org[/\\]elasticsearch[/\\]common[/\\]breaker[/\\]MemoryCircuitBreakerTests.java" checks="LineLength" />

‎core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import static org.elasticsearch.common.settings.Settings.writeSettingsToStream;
4444
import static org.elasticsearch.common.settings.Settings.Builder.EMPTY_SETTINGS;
4545
import static org.elasticsearch.common.xcontent.support.XContentMapValues.lenientNodeBooleanValue;
46+
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;
4647

4748
/**
4849
* Create snapshot request
@@ -366,14 +367,14 @@ public CreateSnapshotRequest source(Map<String, Object> source) {
366367
throw new IllegalArgumentException("malformed indices section, should be an array of strings");
367368
}
368369
} else if (name.equals("partial")) {
369-
partial(lenientNodeBooleanValue(entry.getValue()));
370+
partial(lenientNodeBooleanValue(entry.getValue(), name));
370371
} else if (name.equals("settings")) {
371372
if (!(entry.getValue() instanceof Map)) {
372373
throw new IllegalArgumentException("malformed settings section, should indices an inner object");
373374
}
374375
settings((Map<String, Object>) entry.getValue());
375376
} else if (name.equals("include_global_state")) {
376-
includeGlobalState = lenientNodeBooleanValue(entry.getValue());
377+
includeGlobalState = lenientNodeBooleanValue(entry.getValue(), name);
377378
}
378379
}
379380
indicesOptions(IndicesOptions.fromMap((Map<String, Object>) source, IndicesOptions.lenientExpandOpen()));

‎core/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/restore/RestoreSnapshotRequest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,16 +481,16 @@ public RestoreSnapshotRequest source(Map<String, Object> source) {
481481
throw new IllegalArgumentException("malformed indices section, should be an array of strings");
482482
}
483483
} else if (name.equals("partial")) {
484-
partial(lenientNodeBooleanValue(entry.getValue()));
484+
partial(lenientNodeBooleanValue(entry.getValue(), name));
485485
} else if (name.equals("settings")) {
486486
if (!(entry.getValue() instanceof Map)) {
487487
throw new IllegalArgumentException("malformed settings section");
488488
}
489489
settings((Map<String, Object>) entry.getValue());
490490
} else if (name.equals("include_global_state")) {
491-
includeGlobalState = lenientNodeBooleanValue(entry.getValue());
491+
includeGlobalState = lenientNodeBooleanValue(entry.getValue(), name);
492492
} else if (name.equals("include_aliases")) {
493-
includeAliases = lenientNodeBooleanValue(entry.getValue());
493+
includeAliases = lenientNodeBooleanValue(entry.getValue(), name);
494494
} else if (name.equals("rename_pattern")) {
495495
if (entry.getValue() instanceof String) {
496496
renamePattern((String) entry.getValue());

‎core/src/main/java/org/elasticsearch/action/admin/indices/flush/TransportShardFlushAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.elasticsearch.action.support.replication.ReplicationResponse;
2424
import org.elasticsearch.action.support.replication.TransportReplicationAction;
2525
import org.elasticsearch.cluster.action.shard.ShardStateAction;
26-
import org.elasticsearch.cluster.block.ClusterBlockLevel;
26+
import org.elasticsearch.cluster.metadata.IndexMetaData;
2727
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
2828
import org.elasticsearch.cluster.service.ClusterService;
2929
import org.elasticsearch.common.inject.Inject;
@@ -68,7 +68,7 @@ protected ReplicaResult shardOperationOnReplica(ShardFlushRequest request, Index
6868
}
6969

7070
@Override
71-
protected boolean shouldExecuteReplication(Settings settings) {
71+
protected boolean shouldExecuteReplication(IndexMetaData indexMetaData) {
7272
return true;
7373
}
7474
}

‎core/src/main/java/org/elasticsearch/action/admin/indices/refresh/TransportShardRefreshAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import org.elasticsearch.action.support.replication.ReplicationResponse;
2525
import org.elasticsearch.action.support.replication.TransportReplicationAction;
2626
import org.elasticsearch.cluster.action.shard.ShardStateAction;
27-
import org.elasticsearch.cluster.block.ClusterBlockLevel;
27+
import org.elasticsearch.cluster.metadata.IndexMetaData;
2828
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
2929
import org.elasticsearch.cluster.service.ClusterService;
3030
import org.elasticsearch.common.inject.Inject;
@@ -68,7 +68,7 @@ protected ReplicaResult shardOperationOnReplica(BasicReplicationRequest request,
6868
}
6969

7070
@Override
71-
protected boolean shouldExecuteReplication(Settings settings) {
71+
protected boolean shouldExecuteReplication(IndexMetaData indexMetaData) {
7272
return true;
7373
}
7474
}

‎core/src/main/java/org/elasticsearch/action/support/AutoCreateIndex.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.elasticsearch.common.Booleans;
2525
import org.elasticsearch.common.Strings;
2626
import org.elasticsearch.common.collect.Tuple;
27+
import org.elasticsearch.common.logging.DeprecationLogger;
28+
import org.elasticsearch.common.logging.Loggers;
2729
import org.elasticsearch.common.regex.Regex;
2830
import org.elasticsearch.common.settings.ClusterSettings;
2931
import org.elasticsearch.common.settings.Setting;
@@ -40,6 +42,7 @@
4042
* a write operation is about to happen in a non existing index.
4143
*/
4244
public final class AutoCreateIndex {
45+
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(AutoCreateIndex.class));
4346

4447
public static final Setting<AutoCreate> AUTO_CREATE_INDEX_SETTING =
4548
new Setting<>("action.auto_create_index", "true", AutoCreate::new, Property.NodeScope, Setting.Property.Dynamic);
@@ -116,6 +119,10 @@ private AutoCreate(String value) {
116119
List<Tuple<String, Boolean>> expressions = new ArrayList<>();
117120
try {
118121
autoCreateIndex = Booleans.parseBooleanExact(value);
122+
if (Booleans.isStrictlyBoolean(value) == false) {
123+
DEPRECATION_LOGGER.deprecated("Expected a boolean [true/false] or index name pattern for setting [{}] but got [{}]",
124+
AUTO_CREATE_INDEX_SETTING.getKey(), value);
125+
}
119126
} catch (IllegalArgumentException ex) {
120127
try {
121128
String[] patterns = Strings.commaDelimitedListToStringArray(value);

‎core/src/main/java/org/elasticsearch/action/support/IndicesOptions.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,8 @@ public static IndicesOptions fromParameters(Object wildcardsString, Object ignor
195195

196196
//note that allowAliasesToMultipleIndices is not exposed, always true (only for internal use)
197197
return fromOptions(
198-
lenientNodeBooleanValue(ignoreUnavailableString, defaultSettings.ignoreUnavailable()),
199-
lenientNodeBooleanValue(allowNoIndicesString, defaultSettings.allowNoIndices()),
198+
lenientNodeBooleanValue(ignoreUnavailableString, "ignore_unavailable", defaultSettings.ignoreUnavailable()),
199+
lenientNodeBooleanValue(allowNoIndicesString, "allow_no_indices", defaultSettings.allowNoIndices()),
200200
expandWildcardsOpen,
201201
expandWildcardsClosed,
202202
defaultSettings.allowAliasesToMultipleIndices(),

‎core/src/main/java/org/elasticsearch/action/support/replication/TransportReplicationAction.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ public void handleException(TransportException exp) {
312312
} else {
313313
setPhase(replicationTask, "primary");
314314
final IndexMetaData indexMetaData = clusterService.state().getMetaData().index(request.shardId().getIndex());
315-
final boolean executeOnReplicas = (indexMetaData == null) || shouldExecuteReplication(indexMetaData.getSettings());
315+
final boolean executeOnReplicas = (indexMetaData == null) || shouldExecuteReplication(indexMetaData);
316316
final ActionListener<Response> listener = createResponseListener(primaryShardReference);
317317
createReplicatedOperation(request, new ActionListener<PrimaryResult>() {
318318
@Override
@@ -876,8 +876,8 @@ public void onFailure(Exception e) {
876876
* Indicated whether this operation should be replicated to shadow replicas or not. If this method returns true the replication phase
877877
* will be skipped. For example writes such as index and delete don't need to be replicated on shadow replicas but refresh and flush do.
878878
*/
879-
protected boolean shouldExecuteReplication(Settings settings) {
880-
return IndexMetaData.isIndexUsingShadowReplicas(settings) == false;
879+
protected boolean shouldExecuteReplication(IndexMetaData indexMetaData) {
880+
return IndexMetaData.isIndexUsingShadowReplicas(indexMetaData.getSettings()) == false;
881881
}
882882

883883
class PrimaryShardReference implements ReplicationOperation.Primary<Request, ReplicaRequest, PrimaryResult>, Releasable {

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
package org.elasticsearch.cluster.metadata;
2020

2121
import org.elasticsearch.common.Booleans;
22+
import org.elasticsearch.common.logging.DeprecationLogger;
23+
import org.elasticsearch.common.logging.Loggers;
2224
import org.elasticsearch.common.settings.Setting;
2325
import org.elasticsearch.common.settings.Setting.Property;
2426

@@ -28,12 +30,17 @@
2830
* based on the number of datanodes in the cluster. This class handles all the parsing and streamlines the access to these values.
2931
*/
3032
final class AutoExpandReplicas {
33+
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(AutoExpandReplicas.class));
34+
3135
// the value we recognize in the "max" position to mean all the nodes
3236
private static final String ALL_NODES_VALUE = "all";
3337
public static final Setting<AutoExpandReplicas> SETTING = new Setting<>(IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, "false", (value) -> {
3438
final int min;
3539
final int max;
36-
if (Booleans.parseBoolean(value, true) == false) {
40+
if (Booleans.isExplicitFalse(value)) {
41+
if (Booleans.isStrictlyBoolean(value) == false) {
42+
DEPRECATION_LOGGER.deprecated("Expected [false] for setting [{}] but got [{}]", IndexMetaData.SETTING_AUTO_EXPAND_REPLICAS, value);
43+
}
3744
return new AutoExpandReplicas(0, 0, false);
3845
}
3946
final int dash = value.indexOf('-');

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ private void initMappers(Map<String, Object> withoutType) {
215215
String fieldName = entry.getKey();
216216
Object fieldNode = entry.getValue();
217217
if (fieldName.equals("required")) {
218-
required = lenientNodeBooleanValue(fieldNode);
218+
required = lenientNodeBooleanValue(fieldNode, fieldName);
219219
}
220220
}
221221
this.routing = new Routing(required);
@@ -232,13 +232,13 @@ private void initMappers(Map<String, Object> withoutType) {
232232
String fieldName = entry.getKey();
233233
Object fieldNode = entry.getValue();
234234
if (fieldName.equals("enabled")) {
235-
enabled = lenientNodeBooleanValue(fieldNode);
235+
enabled = lenientNodeBooleanValue(fieldNode, fieldName);
236236
} else if (fieldName.equals("format")) {
237237
format = fieldNode.toString();
238238
} else if (fieldName.equals("default") && fieldNode != null) {
239239
defaultTimestamp = fieldNode.toString();
240240
} else if (fieldName.equals("ignore_missing")) {
241-
ignoreMissing = lenientNodeBooleanValue(fieldNode);
241+
ignoreMissing = lenientNodeBooleanValue(fieldNode, fieldName);
242242
}
243243
}
244244
this.timestamp = new Timestamp(enabled, format, defaultTimestamp, ignoreMissing);

‎core/src/main/java/org/elasticsearch/common/Booleans.java

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,30 +24,6 @@
2424
*/
2525
public class Booleans {
2626

27-
/**
28-
* Returns <code>false</code> if text is in <tt>false</tt>, <tt>0</tt>, <tt>off</tt>, <tt>no</tt>; else, true
29-
*/
30-
public static boolean parseBoolean(char[] text, int offset, int length, boolean defaultValue) {
31-
// TODO: the leniency here is very dangerous: a simple typo will be misinterpreted and the user won't know.
32-
// We should remove it and cutover to https://github.com/rmuir/booleanparser
33-
if (text == null || length == 0) {
34-
return defaultValue;
35-
}
36-
if (length == 1) {
37-
return text[offset] != '0';
38-
}
39-
if (length == 2) {
40-
return !(text[offset] == 'n' && text[offset + 1] == 'o');
41-
}
42-
if (length == 3) {
43-
return !(text[offset] == 'o' && text[offset + 1] == 'f' && text[offset + 2] == 'f');
44-
}
45-
if (length == 5) {
46-
return !(text[offset] == 'f' && text[offset + 1] == 'a' && text[offset + 2] == 'l' && text[offset + 3] == 's' && text[offset + 4] == 'e');
47-
}
48-
return true;
49-
}
50-
5127
/**
5228
* returns true if the a sequence of chars is one of "true","false","on","off","yes","no","0","1"
5329
*
@@ -78,6 +54,13 @@ public static boolean isBoolean(char[] text, int offset, int length) {
7854
return false;
7955
}
8056

57+
public static Boolean parseBooleanExact(String value, Boolean defaultValue) {
58+
if (Strings.hasText(value)) {
59+
return parseBooleanExact(value);
60+
}
61+
return defaultValue;
62+
}
63+
8164
/***
8265
*
8366
* @return true/false
@@ -96,6 +79,13 @@ public static Boolean parseBooleanExact(String value) {
9679
throw new IllegalArgumentException("Failed to parse value [" + value + "] cannot be parsed to boolean [ true/1/on/yes OR false/0/off/no ]");
9780
}
9881

82+
/**
83+
* @return true iff the provided value is either "true" or "false".
84+
*/
85+
public static boolean isStrictlyBoolean(String value) {
86+
return "false".equals(value) || "true".equals(value);
87+
}
88+
9989
public static Boolean parseBoolean(String value, Boolean defaultValue) {
10090
if (value == null) { // only for the null case we do that here!
10191
return defaultValue;

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242
import java.util.ArrayList;
4343
import java.util.Arrays;
4444
import java.util.EnumSet;
45-
import java.util.HashMap;
4645
import java.util.IdentityHashMap;
4746
import java.util.List;
4847
import java.util.Map;
@@ -668,15 +667,25 @@ public static Setting<Integer> intSetting(String key, int defaultValue, Property
668667
}
669668

670669
public static Setting<Boolean> boolSetting(String key, boolean defaultValue, Property... properties) {
671-
return new Setting<>(key, (s) -> Boolean.toString(defaultValue), Booleans::parseBooleanExact, properties);
670+
return new Setting<>(key, (s) -> Boolean.toString(defaultValue), (value) -> parseBoolean(key, value), properties);
672671
}
673672

674673
public static Setting<Boolean> boolSetting(String key, Setting<Boolean> fallbackSetting, Property... properties) {
675-
return new Setting<>(key, fallbackSetting, Booleans::parseBooleanExact, properties);
674+
return new Setting<>(key, fallbackSetting, (value) -> parseBoolean(key, value), properties);
676675
}
677676

678677
public static Setting<Boolean> boolSetting(String key, Function<Settings, String> defaultValueFn, Property... properties) {
679-
return new Setting<>(key, defaultValueFn, Booleans::parseBooleanExact, properties);
678+
return new Setting<>(key, defaultValueFn, (value) -> parseBoolean(key, value), properties);
679+
}
680+
681+
private static Boolean parseBoolean(String key, String value) {
682+
// let the parser handle all cases for non-proper booleans without a deprecation warning by throwing IAE
683+
boolean booleanValue = Booleans.parseBooleanExact(value);
684+
if (Booleans.isStrictlyBoolean(value) == false) {
685+
DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(Setting.class));
686+
deprecationLogger.deprecated("Expected a boolean [true/false] for setting [{}] but got [{}]", key, value);
687+
}
688+
return booleanValue;
680689
}
681690

682691
public static Setting<ByteSizeValue> byteSizeSetting(String key, ByteSizeValue value, Property... properties) {

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@
2626
import org.elasticsearch.common.io.Streams;
2727
import org.elasticsearch.common.io.stream.StreamInput;
2828
import org.elasticsearch.common.io.stream.StreamOutput;
29+
import org.elasticsearch.common.logging.DeprecationLogger;
30+
import org.elasticsearch.common.logging.Loggers;
2931
import org.elasticsearch.common.settings.loader.SettingsLoader;
3032
import org.elasticsearch.common.settings.loader.SettingsLoaderFactory;
3133
import org.elasticsearch.common.unit.ByteSizeUnit;
@@ -74,7 +76,6 @@
7476
* An immutable settings implementation.
7577
*/
7678
public final class Settings implements ToXContent {
77-
7879
public static final Settings EMPTY = new Builder().build();
7980
private static final Pattern ARRAY_PATTERN = Pattern.compile("(.*)\\.\\d+$");
8081

@@ -310,7 +311,13 @@ public Long getAsLong(String setting, Long defaultValue) {
310311
* returns the default value provided.
311312
*/
312313
public Boolean getAsBoolean(String setting, Boolean defaultValue) {
313-
return Booleans.parseBoolean(get(setting), defaultValue);
314+
String rawValue = get(setting);
315+
Boolean booleanValue = Booleans.parseBooleanExact(rawValue, defaultValue);
316+
if (rawValue != null && Booleans.isStrictlyBoolean(rawValue) == false) {
317+
DeprecationLogger deprecationLogger = new DeprecationLogger(Loggers.getLogger(Settings.class));
318+
deprecationLogger.deprecated("Expected a boolean [true/false] for setting [{}] but got [{}]", setting, rawValue);
319+
}
320+
return booleanValue;
314321
}
315322

316323
/**

‎core/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
package org.elasticsearch.common.xcontent;
2121

2222
import org.elasticsearch.common.Booleans;
23+
import org.elasticsearch.common.logging.DeprecationLogger;
24+
import org.elasticsearch.common.logging.Loggers;
2325

2426
import java.io.IOException;
2527
import java.util.Map;
@@ -30,7 +32,6 @@
3032
* but those that don't may or may not require emitting a startObject and an endObject.
3133
*/
3234
public interface ToXContent {
33-
3435
interface Params {
3536
String param(String key);
3637

@@ -65,6 +66,7 @@ public Boolean paramAsBoolean(String key, Boolean defaultValue) {
6566
};
6667

6768
class MapParams implements Params {
69+
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(MapParams.class));
6870

6971
private final Map<String, String> params;
7072

@@ -88,12 +90,16 @@ public String param(String key, String defaultValue) {
8890

8991
@Override
9092
public boolean paramAsBoolean(String key, boolean defaultValue) {
91-
return Booleans.parseBoolean(param(key), defaultValue);
93+
return paramAsBoolean(key, (Boolean) defaultValue);
9294
}
9395

9496
@Override
9597
public Boolean paramAsBoolean(String key, Boolean defaultValue) {
96-
return Booleans.parseBoolean(param(key), defaultValue);
98+
String rawParam = param(key);
99+
if (rawParam != null && Booleans.isStrictlyBoolean(rawParam) == false) {
100+
DEPRECATION_LOGGER.deprecated("Expected a boolean [true/false] for [{}] but got [{}]", key, rawParam);
101+
}
102+
return Booleans.parseBoolean(rawParam, defaultValue);
97103
}
98104
}
99105

0 commit comments

Comments
 (0)