Minor cleanup

Signed-off-by: currantw <[email protected]>
opensearch-project · Dec 5, 2024 · cd9f94b · cd9f94b
1 parent e72aadb
commit cd9f94b
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 42 deletions.
diff --git a/core/src/main/java/org/opensearch/sql/utils/IPUtils.java b/core/src/main/java/org/opensearch/sql/utils/IPUtils.java
@@ -34,7 +34,7 @@ public class IPUtils {
 
   /**
    * Builds and returns the {@link IPAddress} represented by the given IP address range string in
-   * CIDR (classless inter-domain routing) notation. Returns {@link SemanticCheckException} if it
+   * CIDR (classless inter-domain routing) notation. Throws {@link SemanticCheckException} if it
    * does not represent a valid IP address range. Supports both IPv4 and IPv6 address ranges.
    */
   public static IPAddress toRange(String s) throws SemanticCheckException {

diff --git a/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/ip/IPFunctionTest.java
@@ -7,7 +7,6 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.when;
 import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE;
 import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE;
@@ -50,22 +49,14 @@ public class IPFunctionTest {
 
   @Test
   public void cidrmatch_invalid_arguments() {
-    Exception ex;
-
-    ex =
-        assertThrows(
-            SemanticCheckException.class,
-            () -> execute(ExprValueUtils.ipValue("INVALID"), IPv4Range));
-    assertTrue(
-        ex.getMessage().matches("IP address string 'INVALID' is not valid. Error details: .*"));
-
-    ex =
-        assertThrows(
-            SemanticCheckException.class,
-            () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID")));
-    assertTrue(
-        ex.getMessage()
-            .matches("IP address range string 'INVALID' is not valid. Error details: .*"));
+    assertThrows(
+        SemanticCheckException.class,
+        () -> execute(ExprValueUtils.ipValue("INVALID"), IPv4Range),
+        "IP address string 'INVALID' is not valid. Error details: .*");
+    assertThrows(
+        SemanticCheckException.class,
+        () -> execute(IPv4AddressWithin, ExprValueUtils.stringValue("INVALID")),
+        "IP address range string 'INVALID' is not valid. Error details: .*");
   }
 
   @Test

diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/convert/TypeCastOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/convert/TypeCastOperatorTest.java
@@ -343,9 +343,6 @@ void castToTimestamp() {
   void castToIp() {
     FunctionExpression exp;
 
-    String expectedMsg;
-    String actualMsg;
-
     final String ipv4String = "1.2.3.4";
     final String ipv6String = "2001:db7::ff00:42:8329";
     final String ipInvalidString = "INVALID";
@@ -363,10 +360,10 @@ void castToIp() {
     assertEquals(exprIpv6Value, exp.valueOf());
 
     exp = DSL.castIp(DSL.literal(ipInvalidString));
-    actualMsg = assertThrows(SemanticCheckException.class, exp::valueOf).getMessage();
-    expectedMsg =
-        String.format("IP address string '%s' is not valid. Error details: .*", ipInvalidString);
-    assertTrue(actualMsg.matches(expectedMsg));
+    assertThrows(
+        SemanticCheckException.class,
+        exp::valueOf,
+        String.format("IP address string '%s' is not valid. Error details: .*", ipInvalidString));
 
     // From IP address
     exp = DSL.castIp(DSL.literal(exprIpv4Value));
@@ -378,11 +375,10 @@ void castToIp() {
     assertEquals(exprIpv6Value, exp.valueOf());
 
     // From invalid type
-    actualMsg =
-        assertThrows(ExpressionEvaluationException.class, () -> DSL.castIp(DSL.literal(0)))
-            .getMessage();
-    expectedMsg = "cast_to_ip function expected {[IP],[STRING]}, but got [INTEGER]";
-    assertEquals(expectedMsg, actualMsg);
+    assertThrows(
+        ExpressionEvaluationException.class,
+        () -> DSL.castIp(DSL.literal(0)),
+        "cast_to_ip function expected {[IP],[STRING]}, but got [INTEGER]");
 
     // From null or missing value
     exp = DSL.castIp(DSL.literal(ExprNullValue.of()));

diff --git a/core/src/test/java/org/opensearch/sql/utils/ComparisonUtil.java b/core/src/test/java/org/opensearch/sql/utils/ComparisonUtil.java
@@ -69,8 +69,6 @@ public static int compare(ExprValue v1, ExprValue v2) {
         return v1.dateValue().compareTo(v2.dateValue());
       case TIMESTAMP:
         return v1.timestampValue().compareTo(v2.timestampValue());
-      case IP:
-        return v1.ipValue().compareTo(v2.ipValue());
       default:
         throw new ExpressionEvaluationException(
             String.format("%s instances are not comparable", v1.getClass().getSimpleName()));

diff --git a/...test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/...test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java
@@ -1907,16 +1907,15 @@ void non_literal_in_cast_should_build_script() {
   void non_cast_nested_function_should_build_script() {
     mockToStringSerializer();
     assertJsonEquals(
-        """
-                    {
-                      "script" : {
-                        "script" : {
-                          "source" : "=(integer_value, abs(+(1, 0)))",
-                          "lang" : "opensearch_query_expression"
-                        },
-                        "boost" : 1.0
-                      }
-                    }""",
+        "{\n"
+            + "  \"script\" : {\n"
+            + "    \"script\" : {\n"
+            + "      \"source\" : \"=(integer_value, abs(+(1, 0)))\",\n"
+            + "      \"lang\" : \"opensearch_query_expression\"\n"
+            + "    },\n"
+            + "    \"boost\" : 1.0\n"
+            + "  }\n"
+            + "}",
         buildQuery(
             DSL.equal(ref("integer_value", INTEGER), DSL.abs(DSL.add(literal(1), literal(0))))));
   }