From a5f1fb51b81bce57bc0dc286501e9887f8e7c681 Mon Sep 17 00:00:00 2001 From: ZHANG Dapeng Date: Fri, 19 Nov 2021 10:12:39 -0800 Subject: [PATCH] core: have JsonUtil support parsing String value as number (#8711) As documented in https://developers.google.com/protocol-buffers/docs/proto3#json, the canonical proto-to-json converter converts int64 (Java long) values to string values in Json rather than Json numbers (Java Double). Conversely, either Json string value or number value are accepted to be converted to int64 proto value. To better support service configs defined by protobuf messages, support parsing String values as numbers in `JsonUtil`. --- .../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 | 133 ++++++++++++++++++ 4 files changed, 195 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..960800c97c0 --- /dev/null +++ b/core/src/test/java/io/grpc/internal/JsonUtilTest.java @@ -0,0 +1,133 @@ +/* + * 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("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_exponent", "2.998e8"); + map.put("key_string_minus_zero", "-0"); + + 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, "key_string_2.0")).isEqualTo(2D); + try { + 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 'key_string_2.0' is not an integer"); + } + try { + 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 'key_string_2.0' is not a long integer"); + } + + 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, "key_string_nan")).isNaN(); + try { + JsonUtil.getNumberAsInteger(map, "key_string_nan"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo( + "value 'NaN' for key 'key_string_nan' is not an integer"); + } + try { + JsonUtil.getNumberAsLong(map, "key_string_nan"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo( + "value 'NaN' for key 'key_string_nan' is not a long integer"); + } + + assertThat(JsonUtil.getNumberAsDouble(map, "key_number_5.5")).isEqualTo(5.5D); + try { + 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, "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, "key_string_six"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo( + "value 'six' for key 'key_string_six' is not a double"); + } + try { + JsonUtil.getNumberAsInteger(map, "key_string_six"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo( + "value 'six' for key 'key_string_six' is not an integer"); + } + try { + JsonUtil.getNumberAsLong(map, "key_string_six"); + fail("expecting to throw but did not"); + } catch (RuntimeException e) { + assertThat(e).hasMessageThat().isEqualTo( + "value 'six' for key 'key_string_six' is not a long integer"); + } + + 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); + 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(); + } +}