commit 0d963e2ceec9045497bea1e4e2a939e84f36242a
author Tsz-Wo Nicholas Sze <szetszwo@apache.org> Thu Dec 07 14:19:58 2023 -0800
committer GitHub <noreply@github.com> Thu Dec 07 14:19:58 2023 -0800
tree ff74861b65191700512f0587040cab8abdc90668
parent 89a597c85e96a6f282def4466aefc17016bb7558

RATIS-1957. Preconditions incorrectly sets an exception message (#981)

ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java

diff --git a/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java b/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
index 902c1f5..36e647f 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/Preconditions.java
@@ -43,6 +43,9 @@
    */
   static void assertTrue(boolean value, Object message) {
     if (!value) {
+      if (message instanceof Supplier) {
+        message = ((Supplier<?>) message).get();
+      }
       throw new IllegalStateException(String.valueOf(message));
     }
   }
@@ -84,7 +87,7 @@
         () -> name + ": expected == " + expected + " but computed == " + computed);
   }
 
-  static void assertNull(Object object, Supplier<String> message) {
+  static void assertNull(Object object, Supplier<Object> message) {
     assertTrue(object == null, message);
   }
 
@@ -93,14 +96,14 @@
         + name + " = " + object + " != null, class = " + object.getClass());
   }
 
-  static <T> T assertNotNull(T object, Supplier<String> message) {
+  static <T> T assertNotNull(T object, Supplier<Object> message) {
     assertTrue(object != null, message);
     return object;
   }
 
   static <T> T assertNotNull(T object, String name) {
-    return assertNotNull(object, () -> name + " is expected to not be null but "
-        + name + " = " + object + " == null, class = " + object.getClass());
+    Preconditions.assertTrue(object != null, () -> name + " == null");
+    return object;
   }
 
   static <T> T assertNotNull(T object, String format, Object... args) {

ratis-test/src/test/java/org/apache/ratis/util/TestPreconditions.java

diff --git a/ratis-test/src/test/java/org/apache/ratis/util/TestPreconditions.java b/ratis-test/src/test/java/org/apache/ratis/util/TestPreconditions.java
index 7ea3cf7..884c1e5 100644
--- a/ratis-test/src/test/java/org/apache/ratis/util/TestPreconditions.java
+++ b/ratis-test/src/test/java/org/apache/ratis/util/TestPreconditions.java
@@ -22,7 +22,9 @@
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 public class TestPreconditions extends BaseTest {
@@ -50,4 +52,21 @@
 
     Preconditions.assertUnique(three, Arrays.asList(4, 5, 6));
   }
+
+  @Test(timeout = 1000)
+  public void testAssertNull() {
+    final Map<String, String> map = new HashMap<>();
+    final String key = "abc1234";
+    // putNew will call Preconditions.assertNull(..) to assert the entry does not exist in the map
+    // putNew the first time should work
+    CollectionUtils.putNew(key, key, map, () -> "m");
+    Preconditions.assertTrue(map.containsKey(key));
+
+    // putNew the second time should fail
+    final Throwable e = testFailureCase("put " + key + " again",
+        () -> CollectionUtils.putNew(key, key, map, () -> "m"),
+        IllegalStateException.class);
+    // The message should contain the key name
+    Preconditions.assertTrue(e.getMessage().contains(key));
+  }
 }