[commons-lang] branch master updated: LANG-1592: Correct implementation of RandomUtils.nextLong(long, long)

[aherbert]

This is an automated email from the ASF dual-hosted git repository.

aherbert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git


The following commit(s) were added to refs/heads/master by this push:
     new ba7505d  LANG-1592: Correct implementation of 
RandomUtils.nextLong(long, long)
ba7505d is described below

commit ba7505d9a91be02088e1309f172f3dc55823c9c0
Author: Alex Herbert <aherb...@apache.org>
AuthorDate: Tue Jul 21 08:45:03 2020 +0100

    LANG-1592: Correct implementation of RandomUtils.nextLong(long, long)
    
    The method has been changed to use exclusively the long datatype to
    generate the value.
    
    The implementation is taken from the Commons RNG project.
---
 src/changes/changes.xml                            |  1 +
 .../java/org/apache/commons/lang3/RandomUtils.java | 24 +++++++++++++++++++--
 .../org/apache/commons/lang3/RandomUtilsTest.java  | 25 ++++++++++++++++++++++
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 8c1ad67..fa59134 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -48,6 +48,7 @@ The <action> type attribute can be add,update,fix,remove.
   <release version="3.12" date="2020-MM-DD" description="New features and bug 
fixes.">
     <action issue="LANG-1591" type="update" dev="kinow" 
due-to="bhawna94">Remove redundant argument from substring call.</action>
     <action                   type="update" dev="ggregory" due-to="Enable 
Dependabot #587">Enable Dependabot #587.</action>
+    <action issue="LANG-1592" type="fix" dev="aherbert" due-to="Huang Pingcai, 
Alex Herbert">Correct implementation of RandomUtils.nextLong(long, 
long)</action>
   </release>
   <release version="3.11" date="2020-07-12" description="New features and bug 
fixes.">
     <action                   type="update" dev="chtompki" due-to="Jin 
Xu">Refine test output for FastDateParserTest</action>
diff --git a/src/main/java/org/apache/commons/lang3/RandomUtils.java 
b/src/main/java/org/apache/commons/lang3/RandomUtils.java
index 571cece..b1c7f0f 100644
--- a/src/main/java/org/apache/commons/lang3/RandomUtils.java
+++ b/src/main/java/org/apache/commons/lang3/RandomUtils.java
@@ -145,7 +145,7 @@ public class RandomUtils {
             return startInclusive;
         }
 
-        return (long) nextDouble(startInclusive, endExclusive);
+        return startInclusive + nextLong(endExclusive - startInclusive);
     }
 
     /**
@@ -156,7 +156,27 @@ public class RandomUtils {
      * @since 3.5
      */
     public static long nextLong() {
-        return nextLong(0, Long.MAX_VALUE);
+        return nextLong(Long.MAX_VALUE);
+    }
+
+    /**
+     * Generates a {@code long} value between 0 (inclusive) and the specified
+     * value (exclusive).
+     *
+     * @param n Bound on the random number to be returned.  Must be positive.
+     * @return a random {@code long} value between 0 (inclusive) and {@code n}
+     * (exclusive).
+     */
+    private static long nextLong(long n) {
+        // Extracted from o.a.c.rng.core.BaseProvider.nextLong(long)
+        long bits;
+        long val;
+        do {
+            bits = RANDOM.nextLong() >>> 1;
+            val  = bits % n;
+        } while (bits - val + (n - 1) < 0);
+
+        return val;
     }
 
     /**
diff --git a/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java 
b/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
index b05c719..0c8d609 100644
--- a/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
+++ b/src/test/java/org/apache/commons/lang3/RandomUtilsTest.java
@@ -19,6 +19,7 @@ package org.apache.commons.lang3;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -262,4 +263,28 @@ public class RandomUtilsTest {
         final double result = RandomUtils.nextDouble(0, Double.MAX_VALUE);
         assertTrue(result >= 0 && result <= Double.MAX_VALUE);
     }
+
+    /**
+     * Test a large value for long. A previous implementation using
+     * {@link RandomUtils#nextDouble(double, double)} could generate a value 
equal
+     * to the upper limit.
+     *
+     * <pre>
+     * return (long) nextDouble(startInclusive, endExclusive);
+     * </pre>
+     *
+     * <p>See LANG-1592.</p>
+     */
+    @Test
+    public void testLargeValueRangeLong() {
+        final long startInclusive = 12900000000001L;
+        final long endExclusive = 12900000000016L;
+        // Note: The method using 'return (long) nextDouble(startInclusive, 
endExclusive)'
+        // takes thousands of calls to generate an error. This size loop fails 
most
+        // of the time with the previous method.
+        final int n = (int) (endExclusive - startInclusive) * 1000;
+        for (int i = 0; i < n; i++) {
+            assertNotEquals(endExclusive, RandomUtils.nextLong(startInclusive, 
endExclusive));
+        }
+    }
 }