Change Uint* IAE messages 75/84475/1
authorRobert Varga <robert.varga@pantheon.tech>
Sun, 15 Sep 2019 17:02:28 +0000 (19:02 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Sun, 15 Sep 2019 17:22:51 +0000 (19:22 +0200)
As we are rolling out Uint* as replacements to widened binding
representations, there is a potential useability regression in that
the excepion reported no longer contains the range of allowed
values.

This patch changes the reporting structure, so that the binding
format is retained, while also optimizing the checks a bit.

While we are in the area, fix Uint32/Uint64's failure to throw
NPE on null strings.

Change-Id: Ib482d2e632e7e4f4fbc16f047e70d8077aa9341a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
yang/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Uint16.java
yang/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Uint32.java
yang/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Uint64.java
yang/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/Uint8.java
yang/yang-common/src/main/java/org/opendaylight/yangtools/yang/common/UintConversions.java
yang/yang-common/src/test/java/org/opendaylight/yangtools/yang/common/Uint16Test.java
yang/yang-common/src/test/java/org/opendaylight/yangtools/yang/common/Uint32Test.java
yang/yang-common/src/test/java/org/opendaylight/yangtools/yang/common/Uint64Test.java
yang/yang-common/src/test/java/org/opendaylight/yangtools/yang/common/Uint8Test.java

index b4a32cd9cdf9c12258ca240970c09c4e42151520..3efddca331c640841d8464367fe8e6e4215e3ac8 100644 (file)
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.common;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
@@ -45,8 +44,8 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
 
     private static final CanonicalValueSupport<Uint16> SUPPORT = new Support();
     private static final long serialVersionUID = 1L;
-    private static final int MIN_VALUE_INT = 0;
     private static final int MAX_VALUE_INT = 65535;
+    private static final String MAX_VALUE_STR = "65535";
 
     private static final String CACHE_SIZE_PROPERTY = "org.opendaylight.yangtools.yang.common.Uint16.cache.size";
     private static final int DEFAULT_CACHE_SIZE = 256;
@@ -66,7 +65,7 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
 
     static {
         final Uint16[] c = new Uint16[CACHE_SIZE];
-        for (int i = MIN_VALUE_INT; i < c.length; ++i) {
+        for (int i = 0; i < c.length; ++i) {
             c[i] = new Uint16((short) i);
         }
         CACHE = c;
@@ -74,7 +73,7 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
 
     private static final Interner<Uint16> INTERNER = Interners.newWeakInterner();
 
-    public static final Uint16 ZERO = valueOf(MIN_VALUE_INT).intern();
+    public static final Uint16 ZERO = valueOf(0).intern();
     public static final Uint16 ONE = valueOf(1).intern();
     public static final Uint16 MAX_VALUE = valueOf(MAX_VALUE_INT).intern();
 
@@ -113,7 +112,7 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
      * @throws IllegalArgumentException if byteVal is less than zero
      */
     public static Uint16 valueOf(final byte byteVal) {
-        checkArgument(byteVal >= MIN_VALUE_INT, "Negative values are not allowed");
+        UintConversions.checkNonNegative(byteVal, MAX_VALUE_STR);
         return instanceFor(byteVal);
     }
 
@@ -126,7 +125,7 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
      * @throws IllegalArgumentException if shortVal is less than zero.
      */
     public static Uint16 valueOf(final short shortVal) {
-        checkArgument(shortVal >= MIN_VALUE_INT, "Negative values are not allowed");
+        UintConversions.checkNonNegative(shortVal, MAX_VALUE_STR);
         return instanceFor(shortVal);
     }
 
@@ -138,8 +137,7 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
      * @throws IllegalArgumentException if intVal is less than zero or greater than 65535.
      */
     public static Uint16 valueOf(final int intVal) {
-        checkArgument(intVal >= MIN_VALUE_INT && intVal <= MAX_VALUE_INT, "Value %s is outside of allowed range",
-                intVal);
+        UintConversions.checkRange(intVal, MAX_VALUE_INT, MAX_VALUE_STR);
         return instanceFor((short)(intVal & 0xffff));
     }
 
@@ -152,8 +150,7 @@ public class Uint16 extends Number implements CanonicalValue<Uint16> {
      * @throws IllegalArgumentException if intVal is less than zero or greater than 65535.
      */
     public static Uint16 valueOf(final long longVal) {
-        checkArgument(longVal >= MIN_VALUE_INT && longVal <= MAX_VALUE_INT, "Value %s is outside of allowed range",
-                longVal);
+        UintConversions.checkRange(longVal, MAX_VALUE_INT, MAX_VALUE_STR);
         return instanceFor((short)(longVal & 0xffff));
     }
 
index 727e17121bfb9eb38b76c51290cf1b810330f98a..21ddac294a334e8bd866ae17c98765ae0acc824e 100644 (file)
@@ -7,7 +7,7 @@
  */
 package org.opendaylight.yangtools.yang.common;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
 import com.google.common.collect.Interner;
@@ -45,8 +45,8 @@ public class Uint32 extends Number implements CanonicalValue<Uint32> {
 
     private static final CanonicalValueSupport<Uint32> SUPPORT = new Support();
     private static final long serialVersionUID = 1L;
-    private static final long MIN_VALUE_LONG = 0;
-    private static final long MAX_VALUE_LONG = 0xffffffffL;
+    private static final long MAX_VALUE_LONG = 4294967295L;
+    private static final String MAX_VALUE_STR = "4294967295";
 
     private static final String CACHE_SIZE_PROPERTY = "org.opendaylight.yangtools.yang.common.Uint32.cache.size";
     private static final int DEFAULT_CACHE_SIZE = 256;
@@ -113,7 +113,7 @@ public class Uint32 extends Number implements CanonicalValue<Uint32> {
      * @throws IllegalArgumentException if byteVal is less than zero
      */
     public static Uint32 valueOf(final byte byteVal) {
-        checkArgument(byteVal >= MIN_VALUE_LONG, "Negative values are not allowed");
+        UintConversions.checkNonNegative(byteVal, MAX_VALUE_STR);
         return instanceFor(byteVal);
     }
 
@@ -126,7 +126,7 @@ public class Uint32 extends Number implements CanonicalValue<Uint32> {
      * @throws IllegalArgumentException if shortVal is less than zero
      */
     public static Uint32 valueOf(final short shortVal) {
-        checkArgument(shortVal >= MIN_VALUE_LONG, "Negative values are not allowed");
+        UintConversions.checkNonNegative(shortVal, MAX_VALUE_STR);
         return instanceFor(shortVal);
     }
 
@@ -138,7 +138,7 @@ public class Uint32 extends Number implements CanonicalValue<Uint32> {
      * @throws IllegalArgumentException if intVal is less than zero
      */
     public static Uint32 valueOf(final int intVal) {
-        checkArgument(intVal >= MIN_VALUE_LONG, "Negative values are not allowed");
+        UintConversions.checkNonNegative(intVal, MAX_VALUE_STR);
         return instanceFor(intVal);
     }
 
@@ -151,8 +151,7 @@ public class Uint32 extends Number implements CanonicalValue<Uint32> {
      * @throws IllegalArgumentException if longVal is less than zero or greater than 4294967295
      */
     public static Uint32 valueOf(final long longVal) {
-        checkArgument(longVal >= MIN_VALUE_LONG && longVal <= MAX_VALUE_LONG, "Value %s is outside of allowed range",
-                longVal);
+        UintConversions.checkRange(longVal, MAX_VALUE_LONG, MAX_VALUE_STR);
         return instanceFor((int)longVal);
     }
 
@@ -228,7 +227,7 @@ public class Uint32 extends Number implements CanonicalValue<Uint32> {
      *                               {@code radix} is outside of allowed range.
      */
     public static Uint32 valueOf(final String string, final int radix) {
-        return instanceFor(Integer.parseUnsignedInt(string, radix));
+        return instanceFor(Integer.parseUnsignedInt(requireNonNull(string), radix));
     }
 
     /**
index b438419b6f354323a565064efbe87f949e7b031b..271deaddaddd9e17ff590a21e544314d6918bd23 100644 (file)
@@ -7,7 +7,7 @@
  */
 package org.opendaylight.yangtools.yang.common;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
 import com.google.common.collect.Interner;
@@ -46,7 +46,7 @@ public class Uint64 extends Number implements CanonicalValue<Uint64> {
 
     private static final CanonicalValueSupport<Uint64> SUPPORT = new Support();
     private static final long serialVersionUID = 1L;
-    private static final long MIN_VALUE_LONG = 0;
+    private static final String MAX_VALUE_STR = Long.toUnsignedString(-1);
 
     private static final String CACHE_SIZE_PROPERTY = "org.opendaylight.yangtools.yang.common.Uint64.cache.size";
     private static final int DEFAULT_CACHE_SIZE = 256;
@@ -113,7 +113,7 @@ public class Uint64 extends Number implements CanonicalValue<Uint64> {
      * @throws IllegalArgumentException if byteVal is less than zero
      */
     public static Uint64 valueOf(final byte byteVal) {
-        checkArgument(byteVal >= MIN_VALUE_LONG, "Negative values are not allowed");
+        UintConversions.checkNonNegative(byteVal, MAX_VALUE_STR);
         return instanceFor(byteVal);
     }
 
@@ -126,7 +126,7 @@ public class Uint64 extends Number implements CanonicalValue<Uint64> {
      * @throws IllegalArgumentException if shortVal is less than zero
      */
     public static Uint64 valueOf(final short shortVal) {
-        checkArgument(shortVal >= MIN_VALUE_LONG, "Negative values are not allowed");
+        UintConversions.checkNonNegative(shortVal, MAX_VALUE_STR);
         return instanceFor(shortVal);
     }
 
@@ -138,21 +138,23 @@ public class Uint64 extends Number implements CanonicalValue<Uint64> {
      * @throws IllegalArgumentException if intVal is less than zero
      */
     public static Uint64 valueOf(final int intVal) {
-        checkArgument(intVal >= MIN_VALUE_LONG, "Negative values are not allowed");
+        UintConversions.checkNonNegative(intVal, MAX_VALUE_STR);
         return instanceFor(intVal);
     }
 
     /**
-     * Returns an {@code Uint64} corresponding to a given {@code longVal}. The inverse operation is
-     * {@link #fromLongBits(long)}.
+     * Returns an {@code Uint64} corresponding to a given {@code longVal}, which is checked for range.
+     * See also {@link #longValue()} and {@link #fromLongBits(long)}.
      *
      * @param longVal long value
      * @return A Uint8 instance
      * @throws IllegalArgumentException if longVal is less than zero
      */
     public static Uint64 valueOf(final long longVal) {
-        checkArgument(longVal >= MIN_VALUE_LONG, "Negative values are not allowed");
-        return instanceFor(longVal);
+        if (longVal >= 0) {
+            return instanceFor(longVal);
+        }
+        throw new IllegalArgumentException("Invalid range: " + longVal + ", expected: [[0..18446744073709551615]].");
     }
 
     /**
@@ -208,9 +210,10 @@ public class Uint64 extends Number implements CanonicalValue<Uint64> {
      * @throws IllegalArgumentException if bigInt is less than zero or greater than 18446744073709551615
      */
     public static Uint64 valueOf(final BigInteger bigInt) {
-        checkArgument(bigInt.signum() >= 0, "Negative values not allowed");
-        checkArgument(bigInt.bitLength() <= Long.SIZE, "Value %s is outside of allowed range", bigInt);
-        return instanceFor(bigInt.longValue());
+        if (bigInt.signum() >= 0 && bigInt.bitLength() <= Long.SIZE) {
+            return instanceFor(bigInt.longValue());
+        }
+        throw new IllegalArgumentException("Invalid range: " + bigInt + ", expected: [[0..18446744073709551615]].");
     }
 
     /**
@@ -240,7 +243,7 @@ public class Uint64 extends Number implements CanonicalValue<Uint64> {
      *                               {@code radix} is outside of allowed range.
      */
     public static Uint64 valueOf(final String string, final int radix) {
-        return instanceFor(Long.parseUnsignedLong(string, radix));
+        return instanceFor(Long.parseUnsignedLong(requireNonNull(string), radix));
     }
 
     @Override
index ecd1b11b08e4bcdb5cf7d67ef0187da68d131c80..a158a045d455323e93e0750ae168074573a27b42 100644 (file)
@@ -7,7 +7,6 @@
  */
 package org.opendaylight.yangtools.yang.common;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.annotations.Beta;
@@ -43,8 +42,8 @@ public class Uint8 extends Number implements CanonicalValue<Uint8> {
 
     private static final CanonicalValueSupport<Uint8> SUPPORT = new Support();
 
-    private static final short MIN_VALUE_SHORT = 0;
     private static final short MAX_VALUE_SHORT = 255;
+    private static final String MAX_VALUE_STR = "255";
 
     private static final long serialVersionUID = 1L;
 
@@ -52,13 +51,13 @@ public class Uint8 extends Number implements CanonicalValue<Uint8> {
 
     static {
         final Uint8[] c = new Uint8[MAX_VALUE_SHORT + 1];
-        for (int i = MIN_VALUE_SHORT; i <= MAX_VALUE_SHORT; ++i) {
+        for (int i = 0; i <= MAX_VALUE_SHORT; ++i) {
             c[i] = new Uint8((byte)i);
         }
         CACHE = c;
     }
 
-    public static final Uint8 ZERO = valueOf(MIN_VALUE_SHORT);
+    public static final Uint8 ZERO = valueOf(0);
     public static final Uint8 ONE = valueOf(1);
     public static final Uint8 MAX_VALUE = valueOf(MAX_VALUE_SHORT);
 
@@ -95,7 +94,7 @@ public class Uint8 extends Number implements CanonicalValue<Uint8> {
      * @throws IllegalArgumentException if byteVal is less than zero
      */
     public static Uint8 valueOf(final byte byteVal) {
-        checkArgument(byteVal >= MIN_VALUE_SHORT, "Negative values are not allowed");
+        UintConversions.checkNonNegative(byteVal, MAX_VALUE_STR);
         return instanceFor(byteVal);
     }
 
@@ -108,8 +107,7 @@ public class Uint8 extends Number implements CanonicalValue<Uint8> {
      * @throws IllegalArgumentException if shortVal is less than zero or greater than 255.
      */
     public static Uint8 valueOf(final short shortVal) {
-        checkArgument(shortVal >= MIN_VALUE_SHORT && shortVal <= MAX_VALUE_SHORT,
-                "Value %s is outside of allowed range", shortVal);
+        UintConversions.checkRange(shortVal, MAX_VALUE_SHORT, MAX_VALUE_STR);
         return instanceFor((byte)(shortVal & 0xff));
     }
 
@@ -121,8 +119,7 @@ public class Uint8 extends Number implements CanonicalValue<Uint8> {
      * @throws IllegalArgumentException if intVal is less than zero or greater than 255.
      */
     public static Uint8 valueOf(final int intVal) {
-        checkArgument(intVal >= MIN_VALUE_SHORT && intVal <= MAX_VALUE_SHORT,
-                "Value %s is outside of allowed range", intVal);
+        UintConversions.checkRange(intVal, MAX_VALUE_SHORT, MAX_VALUE_STR);
         return instanceFor((byte)(intVal & 0xff));
     }
 
@@ -135,8 +132,7 @@ public class Uint8 extends Number implements CanonicalValue<Uint8> {
      * @throws IllegalArgumentException if intVal is less than zero or greater than 255.
      */
     public static Uint8 valueOf(final long longVal) {
-        checkArgument(longVal >= MIN_VALUE_SHORT && longVal <= MAX_VALUE_SHORT,
-                "Value %s is outside of allowed range", longVal);
+        UintConversions.checkRange(longVal, MAX_VALUE_SHORT, MAX_VALUE_STR);
         return instanceFor((byte)(longVal & 0xff));
     }
 
index 67e8f9f725c28c973599a06305c5fe5f141e08c4..a74bdc50e6f30cd86bd22e190b20d8bec911db52 100644 (file)
@@ -102,4 +102,50 @@ public final class UintConversions {
     public static Uint64 fromGuava(final UnsignedLong value) {
         return Uint64.valueOf(value);
     }
+
+    static void checkNonNegative(final byte value, final String maxValue) {
+        if (value < 0) {
+            throwIAE(value, maxValue);
+        }
+    }
+
+    static void checkNonNegative(final short value, final String maxStr) {
+        if (value < 0) {
+            throwIAE(value, maxStr);
+        }
+    }
+
+    static void checkNonNegative(final int value, final String maxStr) {
+        if (value < 0) {
+            throwIAE(value, maxStr);
+        }
+    }
+
+    static void checkRange(final short value, final short max, final String maxStr) {
+        if (value < 0 || value > max) {
+            throwIAE(value, maxStr);
+        }
+    }
+
+    static void checkRange(final int value, final int max, final String maxStr) {
+        if (value < 0 || value > max) {
+            throwIAE(value, maxStr);
+        }
+    }
+
+    static void checkRange(final long value, final long max, final String maxStr) {
+        if (value < 0 || value > max) {
+            throwIAE(value, maxStr);
+        }
+    }
+
+    private static void throwIAE(final int value, final String max) {
+        // "Invalid range: 65536, expected: [[0..65535]]."
+        throw new IllegalArgumentException("Invalid range: " + value + ", expected: [[0.." + max + "]].");
+    }
+
+    private static void throwIAE(final long value, final String max) {
+        // "Invalid range: 65536, expected: [[0..65535]]."
+        throw new IllegalArgumentException("Invalid range: " + value + ", expected: [[0.." + max + "]].");
+    }
 }
index e84347cdeef0c17d3582cae5458aaca50e0128e8..57625ecc50793a2d6461aa14c7bbb64d77ee8be4 100644 (file)
@@ -141,4 +141,9 @@ public class Uint16Test {
     public void testBigLong() {
         Uint16.valueOf(65536L);
     }
+
+    @Test(expected = NullPointerException.class)
+    public void testNullValueOfString() {
+        Uint16.valueOf((String) null);
+    }
 }
index 11d77463af9aae3d0ae7cf8346b4b5ccd3469427..81716a14e28f5608923cd4c5f2808cf0369b2736 100644 (file)
@@ -137,4 +137,9 @@ public class Uint32Test {
     public void testBigLong() {
         Uint32.valueOf(4294967296L);
     }
+
+    @Test(expected = NullPointerException.class)
+    public void testNullValueOfString() {
+        Uint32.valueOf((String) null);
+    }
 }
index 64bdf9fcf7fc792866ce9403e12ee3e0e11f4fde..707b27ea88bd000fcf318be74a66a1158ff5e0af 100644 (file)
@@ -144,4 +144,14 @@ public class Uint64Test {
     public void testBigBigInteger() {
         Uint64.valueOf(new BigInteger("0x10000000000000000"));
     }
+
+    @Test(expected = NullPointerException.class)
+    public void testNullValueOfString() {
+        Uint64.valueOf((String) null);
+    }
+
+    @Test(expected = NullPointerException.class)
+    public void testNullValueOfBigInteger() {
+        Uint64.valueOf((BigInteger) null);
+    }
 }
index d9a18276fdf073f7db1623614e913668a8d4118b..93a31556340294804ccb27c4f9c2335025c80553 100644 (file)
@@ -142,4 +142,9 @@ public class Uint8Test {
     public void testBigLong() {
         Uint8.valueOf(256L);
     }
+
+    @Test(expected = NullPointerException.class)
+    public void testNullValueOfString() {
+        Uint8.valueOf((String) null);
+    }
 }