From 4165209b69b23f7d0dbb4bdd59755879dcc9ac8e Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Wed, 17 Nov 2021 20:13:10 -0800 Subject: [PATCH 1/4] core: have JsonUtil support parsing String value as number --- .../io/grpc/internal/DnsNameResolver.java | 2 +- .../main/java/io/grpc/internal/JsonUtil.java | 81 +++++++++---- .../io/grpc/internal/ServiceConfigUtil.java | 6 +- .../java/io/grpc/internal/JsonUtilTest.java | 114 ++++++++++++++++++ 4 files changed, 176 insertions(+), 27 deletions(-) create mode 100644 core/src/test/java/io/grpc/internal/JsonUtilTest.java diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolver.java b/core/src/main/java/io/grpc/internal/DnsNameResolver.java index bc994a01035..5418a0bd32d 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolver.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolver.java @@ -434,7 +434,7 @@ final int getPort() { @Nullable private static final Double getPercentageFromChoice(Map serviceConfigChoice) { - return JsonUtil.getNumber(serviceConfigChoice, SERVICE_CONFIG_CHOICE_PERCENTAGE_KEY); + return JsonUtil.getNumberAsDouble(serviceConfigChoice, SERVICE_CONFIG_CHOICE_PERCENTAGE_KEY); } @Nullable diff --git a/core/src/main/java/io/grpc/internal/JsonUtil.java b/core/src/main/java/io/grpc/internal/JsonUtil.java index d80b4ed44c4..117135fe634 100644 --- a/core/src/main/java/io/grpc/internal/JsonUtil.java +++ b/core/src/main/java/io/grpc/internal/JsonUtil.java @@ -96,57 +96,92 @@ public static List getListOfStrings(Map obj, String key) { /** * Gets a number from an object for the given key. If the key is not present, this returns null. - * If the value is not a Double, throws an exception. + * If the value does not represent a double, throws an exception. */ @Nullable - public static Double getNumber(Map obj, String key) { + public static Double getNumberAsDouble(Map obj, String key) { assert key != null; if (!obj.containsKey(key)) { return null; } Object value = obj.get(key); - if (!(value instanceof Double)) { - throw new ClassCastException( - String.format("value '%s' for key '%s' in '%s' is not Double", value, key, obj)); + if (value instanceof Double) { + return (Double) value; + } + if (value instanceof String) { + try { + return Double.parseDouble((String) value); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + String.format("value '%s' for key '%s' is not a double", value, key)); + } } - return (Double) value; + throw new IllegalArgumentException( + String.format("value '%s' for key '%s' in '%s' is not a number", value, key, obj)); } /** * Gets a number from an object for the given key, casted to an integer. If the key is not - * present, this returns null. If the value is not a Double or loses precision when cast to an - * integer, throws an exception. + * present, this returns null. If the value does not represent an integer, throws an exception. */ + @Nullable public static Integer getNumberAsInteger(Map obj, String key) { - Double d = getNumber(obj, key); - if (d == null) { + assert key != null; + if (!obj.containsKey(key)) { return null; } - int i = d.intValue(); - if (i != d) { - throw new ClassCastException("Number expected to be integer: " + d); + Object value = obj.get(key); + if (value instanceof Double) { + Double d = (Double) value; + int i = d.intValue(); + if (i != d) { + throw new ClassCastException("Number expected to be integer: " + d); + } + return i; + } + if (value instanceof String) { + try { + return Integer.parseInt((String) value); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + String.format("value '%s' for key '%s' is not an integer", value, key)); + } } - return i; + throw new IllegalArgumentException( + String.format("value '%s' for key '%s' is not an integer", value, key)); } /** * Gets a number from an object for the given key, casted to an long. If the key is not - * present, this returns null. If the value is not a Double or loses precision when cast to an - * long, throws an exception. + * present, this returns null. If the value does not represent a long integer, throws an + * exception. */ public static Long getNumberAsLong(Map obj, String key) { - Double d = getNumber(obj, key); - if (d == null) { + assert key != null; + if (!obj.containsKey(key)) { return null; } - long l = d.longValue(); - if (l != d) { - throw new ClassCastException("Number expected to be long: " + d); + Object value = obj.get(key); + if (value instanceof Double) { + Double d = (Double) value; + long l = d.longValue(); + if (l != d) { + throw new ClassCastException("Number expected to be long: " + d); + } + return l; + } + if (value instanceof String) { + try { + return Long.parseLong((String) value); + } catch (NumberFormatException e) { + throw new IllegalArgumentException( + String.format("value '%s' for key '%s' is not a long integer", value, key)); + } } - return l; + throw new IllegalArgumentException( + String.format("value '%s' for key '%s' is not a long integer", value, key)); } - /** * Gets a string from an object for the given key. If the key is not present, this returns null. * If the value is not a String, throws an exception. diff --git a/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java b/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java index 294e9dbcf73..34d44f7f549 100644 --- a/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java +++ b/core/src/main/java/io/grpc/internal/ServiceConfigUtil.java @@ -113,8 +113,8 @@ static Throttle getThrottlePolicy(@Nullable Map serviceConfig) { } // TODO(dapengzhang0): check if this is null. - float maxTokens = JsonUtil.getNumber(throttling, "maxTokens").floatValue(); - float tokenRatio = JsonUtil.getNumber(throttling, "tokenRatio").floatValue(); + float maxTokens = JsonUtil.getNumberAsDouble(throttling, "maxTokens").floatValue(); + float tokenRatio = JsonUtil.getNumberAsDouble(throttling, "tokenRatio").floatValue(); checkState(maxTokens > 0f, "maxToken should be greater than zero"); checkState(tokenRatio > 0f, "tokenRatio should be greater than zero"); return new Throttle(maxTokens, tokenRatio); @@ -137,7 +137,7 @@ static Long getMaxBackoffNanosFromRetryPolicy(Map retryPolicy) { @Nullable static Double getBackoffMultiplierFromRetryPolicy(Map retryPolicy) { - return JsonUtil.getNumber(retryPolicy, "backoffMultiplier"); + return JsonUtil.getNumberAsDouble(retryPolicy, "backoffMultiplier"); } @Nullable diff --git a/core/src/test/java/io/grpc/internal/JsonUtilTest.java b/core/src/test/java/io/grpc/internal/JsonUtilTest.java new file mode 100644 index 00000000000..06d062e7aba --- /dev/null +++ b/core/src/test/java/io/grpc/internal/JsonUtilTest.java @@ -0,0 +1,114 @@ +/* + * Copyright 2021, gRPC Authors All rights reserved. + * + * Licensed 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 io.grpc.internal; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; + +import java.util.HashMap; +import java.util.Map; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link JsonUtil}. */ +@RunWith(JUnit4.class) +public class JsonUtilTest { + @Test + public void getNumber() { + Map map = new HashMap<>(); + map.put("k1", 1D); + map.put("k2", "2.0"); + map.put("k3", "3"); + map.put("k4", "NaN"); + map.put("k5", 5.5D); + map.put("k6", "six"); + + assertThat(JsonUtil.getNumberAsDouble(map, "k1")).isEqualTo(1D); + assertThat(JsonUtil.getNumberAsInteger(map, "k1")).isEqualTo(1); + assertThat(JsonUtil.getNumberAsLong(map, "k1")).isEqualTo(1L); + + assertThat(JsonUtil.getNumberAsDouble(map, "k2")).isEqualTo(2D); + try { + JsonUtil.getNumberAsInteger(map, "k2"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value '2.0' for key 'k2' is not an integer"); + } + try { + JsonUtil.getNumberAsLong(map, "k2"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value '2.0' for key 'k2' is not a long integer"); + } + + assertThat(JsonUtil.getNumberAsDouble(map, "k3")).isEqualTo(3D); + assertThat(JsonUtil.getNumberAsInteger(map, "k3")).isEqualTo(3); + assertThat(JsonUtil.getNumberAsLong(map, "k3")).isEqualTo(3L); + + assertThat(JsonUtil.getNumberAsDouble(map, "k4")).isNaN(); + try { + JsonUtil.getNumberAsInteger(map, "k4"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value 'NaN' for key 'k4' is not an integer"); + } + try { + JsonUtil.getNumberAsLong(map, "k4"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value 'NaN' for key 'k4' is not a long integer"); + } + + assertThat(JsonUtil.getNumberAsDouble(map, "k5")).isEqualTo(5.5D); + try { + JsonUtil.getNumberAsInteger(map, "k5"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("Number expected to be integer: 5.5"); + } + try { + JsonUtil.getNumberAsLong(map, "k5"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("Number expected to be long: 5.5"); + } + + try { + JsonUtil.getNumberAsDouble(map, "k6"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not a double"); + } + try { + JsonUtil.getNumberAsInteger(map, "k6"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not an integer"); + } + try { + JsonUtil.getNumberAsLong(map, "k6"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not a long integer"); + } + + assertThat(JsonUtil.getNumberAsDouble(map, "k7")).isNull(); + assertThat(JsonUtil.getNumberAsInteger(map, "k7")).isNull(); + assertThat(JsonUtil.getNumberAsLong(map, "k7")).isNull(); + } +} From 6bd85fe2b1e18f8194c06c13f34ebed6e3967a3a Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Fri, 19 Nov 2021 08:48:22 -0800 Subject: [PATCH 2/4] add more tests --- .../test/java/io/grpc/internal/JsonUtilTest.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/core/src/test/java/io/grpc/internal/JsonUtilTest.java b/core/src/test/java/io/grpc/internal/JsonUtilTest.java index 06d062e7aba..39823d70f7e 100644 --- a/core/src/test/java/io/grpc/internal/JsonUtilTest.java +++ b/core/src/test/java/io/grpc/internal/JsonUtilTest.java @@ -37,6 +37,9 @@ public void getNumber() { map.put("k4", "NaN"); map.put("k5", 5.5D); map.put("k6", "six"); + map.put("key_string_infinity", "Infinity"); + map.put("key_string_minus_infinity", "-Infinity"); + map.put("key_string_minus_zero", "-0"); assertThat(JsonUtil.getNumberAsDouble(map, "k1")).isEqualTo(1D); assertThat(JsonUtil.getNumberAsInteger(map, "k1")).isEqualTo(1); @@ -107,8 +110,15 @@ public void getNumber() { assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not a long integer"); } - assertThat(JsonUtil.getNumberAsDouble(map, "k7")).isNull(); - assertThat(JsonUtil.getNumberAsInteger(map, "k7")).isNull(); - assertThat(JsonUtil.getNumberAsLong(map, "k7")).isNull(); + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_infinity")).isPositiveInfinity(); + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_minus_infinity")).isNegativeInfinity(); + + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_minus_zero")).isZero(); + assertThat(JsonUtil.getNumberAsInteger(map, "key_string_minus_zero")).isEqualTo(0); + assertThat(JsonUtil.getNumberAsLong(map, "key_string_minus_zero")).isEqualTo(0L); + + assertThat(JsonUtil.getNumberAsDouble(map, "key_nonexistent")).isNull(); + assertThat(JsonUtil.getNumberAsInteger(map, "key_nonexistent")).isNull(); + assertThat(JsonUtil.getNumberAsLong(map, "key_nonexistent")).isNull(); } } From f76dfb48ac9e45f6556cc3e130fa470041c2efca Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Fri, 19 Nov 2021 08:51:58 -0800 Subject: [PATCH 3/4] replace meaningless keys --- .../java/io/grpc/internal/JsonUtilTest.java | 69 ++++++++++--------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/core/src/test/java/io/grpc/internal/JsonUtilTest.java b/core/src/test/java/io/grpc/internal/JsonUtilTest.java index 39823d70f7e..72091333b30 100644 --- a/core/src/test/java/io/grpc/internal/JsonUtilTest.java +++ b/core/src/test/java/io/grpc/internal/JsonUtilTest.java @@ -31,83 +31,90 @@ public class JsonUtilTest { @Test public void getNumber() { Map map = new HashMap<>(); - map.put("k1", 1D); - map.put("k2", "2.0"); - map.put("k3", "3"); - map.put("k4", "NaN"); - map.put("k5", 5.5D); - map.put("k6", "six"); + map.put("key_number_1", 1D); + map.put("key_string_2.0", "2.0"); + map.put("key_string_3", "3"); + map.put("key_string_nan", "NaN"); + map.put("key_number_5.5", 5.5D); + map.put("key_string_six", "six"); map.put("key_string_infinity", "Infinity"); map.put("key_string_minus_infinity", "-Infinity"); map.put("key_string_minus_zero", "-0"); - assertThat(JsonUtil.getNumberAsDouble(map, "k1")).isEqualTo(1D); - assertThat(JsonUtil.getNumberAsInteger(map, "k1")).isEqualTo(1); - assertThat(JsonUtil.getNumberAsLong(map, "k1")).isEqualTo(1L); + assertThat(JsonUtil.getNumberAsDouble(map, "key_number_1")).isEqualTo(1D); + assertThat(JsonUtil.getNumberAsInteger(map, "key_number_1")).isEqualTo(1); + assertThat(JsonUtil.getNumberAsLong(map, "key_number_1")).isEqualTo(1L); - assertThat(JsonUtil.getNumberAsDouble(map, "k2")).isEqualTo(2D); + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_2.0")).isEqualTo(2D); try { - JsonUtil.getNumberAsInteger(map, "k2"); + JsonUtil.getNumberAsInteger(map, "key_string_2.0"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value '2.0' for key 'k2' is not an integer"); + assertThat(e).hasMessageThat().isEqualTo( + "value '2.0' for key 'key_string_2.0' is not an integer"); } try { - JsonUtil.getNumberAsLong(map, "k2"); + JsonUtil.getNumberAsLong(map, "key_string_2.0"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value '2.0' for key 'k2' is not a long integer"); + assertThat(e).hasMessageThat().isEqualTo( + "value '2.0' for key 'key_string_2.0' is not a long integer"); } - assertThat(JsonUtil.getNumberAsDouble(map, "k3")).isEqualTo(3D); - assertThat(JsonUtil.getNumberAsInteger(map, "k3")).isEqualTo(3); - assertThat(JsonUtil.getNumberAsLong(map, "k3")).isEqualTo(3L); + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_3")).isEqualTo(3D); + assertThat(JsonUtil.getNumberAsInteger(map, "key_string_3")).isEqualTo(3); + assertThat(JsonUtil.getNumberAsLong(map, "key_string_3")).isEqualTo(3L); - assertThat(JsonUtil.getNumberAsDouble(map, "k4")).isNaN(); + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_nan")).isNaN(); try { - JsonUtil.getNumberAsInteger(map, "k4"); + JsonUtil.getNumberAsInteger(map, "key_string_nan"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value 'NaN' for key 'k4' is not an integer"); + assertThat(e).hasMessageThat().isEqualTo( + "value 'NaN' for key 'key_string_nan' is not an integer"); } try { - JsonUtil.getNumberAsLong(map, "k4"); + JsonUtil.getNumberAsLong(map, "key_string_nan"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value 'NaN' for key 'k4' is not a long integer"); + assertThat(e).hasMessageThat().isEqualTo( + "value 'NaN' for key 'key_string_nan' is not a long integer"); } - assertThat(JsonUtil.getNumberAsDouble(map, "k5")).isEqualTo(5.5D); + assertThat(JsonUtil.getNumberAsDouble(map, "key_number_5.5")).isEqualTo(5.5D); try { - JsonUtil.getNumberAsInteger(map, "k5"); + JsonUtil.getNumberAsInteger(map, "key_number_5.5"); fail("expecting to throw but did not"); } catch (RuntimeException e) { assertThat(e).hasMessageThat().isEqualTo("Number expected to be integer: 5.5"); } try { - JsonUtil.getNumberAsLong(map, "k5"); + JsonUtil.getNumberAsLong(map, "key_number_5.5"); fail("expecting to throw but did not"); } catch (RuntimeException e) { assertThat(e).hasMessageThat().isEqualTo("Number expected to be long: 5.5"); } try { - JsonUtil.getNumberAsDouble(map, "k6"); + JsonUtil.getNumberAsDouble(map, "key_string_six"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not a double"); + assertThat(e).hasMessageThat().isEqualTo( + "value 'six' for key 'key_string_six' is not a double"); } try { - JsonUtil.getNumberAsInteger(map, "k6"); + JsonUtil.getNumberAsInteger(map, "key_string_six"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not an integer"); + assertThat(e).hasMessageThat().isEqualTo( + "value 'six' for key 'key_string_six' is not an integer"); } try { - JsonUtil.getNumberAsLong(map, "k6"); + JsonUtil.getNumberAsLong(map, "key_string_six"); fail("expecting to throw but did not"); } catch (RuntimeException e) { - assertThat(e).hasMessageThat().isEqualTo("value 'six' for key 'k6' is not a long integer"); + assertThat(e).hasMessageThat().isEqualTo( + "value 'six' for key 'key_string_six' is not a long integer"); } assertThat(JsonUtil.getNumberAsDouble(map, "key_string_infinity")).isPositiveInfinity(); From ecf6c35bb14488a6d4f991d4dce8d7dde8d601b3 Mon Sep 17 00:00:00 2001 From: "Penn (Dapeng) Zhang" Date: Fri, 19 Nov 2021 08:58:22 -0800 Subject: [PATCH 4/4] add test for exponent --- core/src/test/java/io/grpc/internal/JsonUtilTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/test/java/io/grpc/internal/JsonUtilTest.java b/core/src/test/java/io/grpc/internal/JsonUtilTest.java index 72091333b30..960800c97c0 100644 --- a/core/src/test/java/io/grpc/internal/JsonUtilTest.java +++ b/core/src/test/java/io/grpc/internal/JsonUtilTest.java @@ -39,6 +39,7 @@ public void getNumber() { map.put("key_string_six", "six"); map.put("key_string_infinity", "Infinity"); map.put("key_string_minus_infinity", "-Infinity"); + map.put("key_string_exponent", "2.998e8"); map.put("key_string_minus_zero", "-0"); assertThat(JsonUtil.getNumberAsDouble(map, "key_number_1")).isEqualTo(1D); @@ -119,6 +120,7 @@ public void getNumber() { assertThat(JsonUtil.getNumberAsDouble(map, "key_string_infinity")).isPositiveInfinity(); assertThat(JsonUtil.getNumberAsDouble(map, "key_string_minus_infinity")).isNegativeInfinity(); + assertThat(JsonUtil.getNumberAsDouble(map, "key_string_exponent")).isEqualTo(2.998e8D); assertThat(JsonUtil.getNumberAsDouble(map, "key_string_minus_zero")).isZero(); assertThat(JsonUtil.getNumberAsInteger(map, "key_string_minus_zero")).isEqualTo(0);