From 3e5b46fe9681eb18a1e8dcce4068cddb02675a0e Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 21 Feb 2018 08:46:11 -0700 Subject: [PATCH 1/5] Remove Booleans use from XContent and ToXContent This removes the use of the `common.Boolean` class from two of the XContent classes, so they can be decoupled from the ES code as much as possible. Related to #28754, #28504 --- .../common/xcontent/ToXContent.java | 28 ++++++++++++++++--- .../common/xcontent/XContent.java | 11 ++++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java index 3006363a4ddd4..6e2b17e4b1b93 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java @@ -19,8 +19,6 @@ package org.elasticsearch.common.xcontent; -import org.elasticsearch.common.Booleans; - import java.io.IOException; import java.util.Map; @@ -88,12 +86,34 @@ public String param(String key, String defaultValue) { @Override public boolean paramAsBoolean(String key, boolean defaultValue) { - return Booleans.parseBoolean(param(key), defaultValue); + if (key.length() > 0) { + switch (key) { + case "true": + return true; + case "false": + return false; + default: + throw new IllegalArgumentException("Failed to parse param [" + key + "] as only [true] or [false] are allowed."); + } + } else { + return defaultValue; + } } @Override public Boolean paramAsBoolean(String key, Boolean defaultValue) { - return Booleans.parseBoolean(param(key), defaultValue); + if (key.length() > 0) { + switch (key) { + case "true": + return true; + case "false": + return false; + default: + throw new IllegalArgumentException("Failed to parse param [" + key + "] as only [true] or [false] are allowed."); + } + } else { + return defaultValue; + } } } diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java index e0307177046c0..0a882b90cc8d1 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -19,7 +19,6 @@ package org.elasticsearch.common.xcontent; -import org.elasticsearch.common.Booleans; import org.elasticsearch.common.bytes.BytesReference; import java.io.IOException; @@ -52,7 +51,15 @@ public interface XContent { */ static boolean isStrictDuplicateDetectionEnabled() { // Don't allow duplicate keys in JSON content by default but let the user opt out - return Booleans.parseBoolean(System.getProperty("es.xcontent.strict_duplicate_detection", "true")); + final String dupProp = System.getProperty("es.xcontent.strict_duplicate_detection", "true"); + switch (dupProp) { + case "true": + return true; + case "false": + return false; + default: + throw new IllegalArgumentException("Failed to parse value [" + dupProp + "] as only [true] or [false] are allowed."); + } } /** From 5c6d54574e8791a6bb732918ba9aae150d283115 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Wed, 21 Feb 2018 09:42:39 -0700 Subject: [PATCH 2/5] Fix incorrect parameter retrieval --- .../elasticsearch/common/xcontent/ToXContent.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java index 6e2b17e4b1b93..b9e12ac1d1d27 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java @@ -86,14 +86,15 @@ public String param(String key, String defaultValue) { @Override public boolean paramAsBoolean(String key, boolean defaultValue) { - if (key.length() > 0) { - switch (key) { + final String value = param(key); + if (value != null && value.length() > 0) { + switch (value) { case "true": return true; case "false": return false; default: - throw new IllegalArgumentException("Failed to parse param [" + key + "] as only [true] or [false] are allowed."); + throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); } } else { return defaultValue; @@ -102,14 +103,15 @@ public boolean paramAsBoolean(String key, boolean defaultValue) { @Override public Boolean paramAsBoolean(String key, Boolean defaultValue) { - if (key.length() > 0) { - switch (key) { + final String value = param(key); + if (value != null && value.length() > 0) { + switch (value) { case "true": return true; case "false": return false; default: - throw new IllegalArgumentException("Failed to parse param [" + key + "] as only [true] or [false] are allowed."); + throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); } } else { return defaultValue; From 429f6a4743b91b9c39799acdcc32c0a2d90c43ce Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 8 Mar 2018 11:19:56 -0700 Subject: [PATCH 3/5] Use a small helper rather than duplicating code three times --- .../common/xcontent/ToXContent.java | 48 +++++++++---------- .../common/xcontent/XContent.java | 10 +--- 2 files changed, 23 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java index b9e12ac1d1d27..ad6f7e749c90c 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java @@ -86,36 +86,12 @@ public String param(String key, String defaultValue) { @Override public boolean paramAsBoolean(String key, boolean defaultValue) { - final String value = param(key); - if (value != null && value.length() > 0) { - switch (value) { - case "true": - return true; - case "false": - return false; - default: - throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); - } - } else { - return defaultValue; - } + return parseBoolean(param(key), defaultValue); } @Override public Boolean paramAsBoolean(String key, Boolean defaultValue) { - final String value = param(key); - if (value != null && value.length() > 0) { - switch (value) { - case "true": - return true; - case "false": - return false; - default: - throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); - } - } else { - return defaultValue; - } + return parseBoolean(param(key), defaultValue); } } @@ -154,4 +130,24 @@ public Boolean paramAsBoolean(String key, Boolean defaultValue) { default boolean isFragment() { return true; } + + /** + * Parse {@code value} with values "true", "false", or null, returning the + * default value if null is used. Any other input results in an + * {@link IllegalArgumentException} being thrown. + */ + static boolean parseBoolean(String value, Boolean defaultValue) { + if (value != null && value.length() > 0) { + switch (value) { + case "true": + return true; + case "false": + return false; + default: + throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); + } + } else { + return defaultValue; + } + } } diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java index 0a882b90cc8d1..dc177afab388c 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -51,15 +51,7 @@ public interface XContent { */ static boolean isStrictDuplicateDetectionEnabled() { // Don't allow duplicate keys in JSON content by default but let the user opt out - final String dupProp = System.getProperty("es.xcontent.strict_duplicate_detection", "true"); - switch (dupProp) { - case "true": - return true; - case "false": - return false; - default: - throw new IllegalArgumentException("Failed to parse value [" + dupProp + "] as only [true] or [false] are allowed."); - } + return ToXContent.parseBoolean(System.getProperty("es.xcontent.strict_duplicate_detection", "true"), true); } /** From fd0c6eb7f55b75ecd01f7ba2e2395c7b402e5a54 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 8 Mar 2018 11:22:58 -0700 Subject: [PATCH 4/5] Small javadoc tweak --- .../java/org/elasticsearch/common/xcontent/ToXContent.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java index ad6f7e749c90c..c9bbbb19105c4 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java @@ -133,8 +133,8 @@ default boolean isFragment() { /** * Parse {@code value} with values "true", "false", or null, returning the - * default value if null is used. Any other input results in an - * {@link IllegalArgumentException} being thrown. + * default value if null or the empty string is used. Any other input + * results in an {@link IllegalArgumentException} being thrown. */ static boolean parseBoolean(String value, Boolean defaultValue) { if (value != null && value.length() > 0) { From e4bd246d90bb3d4b37f47d71eff27a645cdcba9d Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 9 Mar 2018 10:54:45 -0700 Subject: [PATCH 5/5] Factor boolean parsing into separate package-private class --- .../common/xcontent/Booleans.java | 46 +++++++++++++++++++ .../common/xcontent/ToXContent.java | 23 +--------- .../common/xcontent/XContent.java | 2 +- 3 files changed, 49 insertions(+), 22 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/xcontent/Booleans.java diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/Booleans.java b/server/src/main/java/org/elasticsearch/common/xcontent/Booleans.java new file mode 100644 index 0000000000000..21c0ea5fdd08b --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/xcontent/Booleans.java @@ -0,0 +1,46 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.common.xcontent; + +/** + * Helpers for dealing with boolean values. Package-visible only so that only XContent classes use them. + */ +final class Booleans { + /** + * Parse {@code value} with values "true", "false", or null, returning the + * default value if null or the empty string is used. Any other input + * results in an {@link IllegalArgumentException} being thrown. + */ + static boolean parseBoolean(String value, Boolean defaultValue) { + if (value != null && value.length() > 0) { + switch (value) { + case "true": + return true; + case "false": + return false; + default: + throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); + } + } else { + return defaultValue; + } + } + +} diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java index c9bbbb19105c4..f74bdec17a9f6 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/ToXContent.java @@ -86,12 +86,12 @@ public String param(String key, String defaultValue) { @Override public boolean paramAsBoolean(String key, boolean defaultValue) { - return parseBoolean(param(key), defaultValue); + return Booleans.parseBoolean(param(key), defaultValue); } @Override public Boolean paramAsBoolean(String key, Boolean defaultValue) { - return parseBoolean(param(key), defaultValue); + return Booleans.parseBoolean(param(key), defaultValue); } } @@ -131,23 +131,4 @@ default boolean isFragment() { return true; } - /** - * Parse {@code value} with values "true", "false", or null, returning the - * default value if null or the empty string is used. Any other input - * results in an {@link IllegalArgumentException} being thrown. - */ - static boolean parseBoolean(String value, Boolean defaultValue) { - if (value != null && value.length() > 0) { - switch (value) { - case "true": - return true; - case "false": - return false; - default: - throw new IllegalArgumentException("Failed to parse param [" + value + "] as only [true] or [false] are allowed."); - } - } else { - return defaultValue; - } - } } diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java index c371f3c024438..6f6ee4ffdda54 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/XContent.java @@ -49,7 +49,7 @@ public interface XContent { */ static boolean isStrictDuplicateDetectionEnabled() { // Don't allow duplicate keys in JSON content by default but let the user opt out - return ToXContent.parseBoolean(System.getProperty("es.xcontent.strict_duplicate_detection", "true"), true); + return Booleans.parseBoolean(System.getProperty("es.xcontent.strict_duplicate_detection", "true"), true); } /**