Put more info to exception messages and remove extra logs

Signed-off-by: Martin Gaievski <[email protected]>
opensearch-project · Aug 30, 2023 · 614be2c · 614be2c
1 parent 5a555f0
commit 614be2c
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 9 deletions.
diff --git a/src/main/java/org/opensearch/neuralsearch/processor/combination/ScoreCombinationUtil.java b/src/main/java/org/opensearch/neuralsearch/processor/combination/ScoreCombinationUtil.java
@@ -100,15 +100,14 @@ protected void validateIfWeightsMatchScores(final float[] scores, final List<Flo
             return;
         }
         if (scores.length != weights.size()) {
-            log.error(
+            throw new IllegalArgumentException(
                 String.format(
                     Locale.ROOT,
                     "number of weights [%d] must match number of sub-queries [%d] in hybrid query",
                     weights.size(),
                     scores.length
                 )
             );
-            throw new IllegalArgumentException("number of weights must match number of sub-queries in hybrid query");
         }
     }
 
@@ -121,25 +120,23 @@ protected void validateIfWeightsMatchScores(final float[] scores, final List<Flo
     private void validateWeights(final List<Float> weightsList) {
         boolean isOutOfRange = weightsList.stream().anyMatch(weight -> !Range.between(0.0f, 1.0f).contains(weight));
         if (isOutOfRange) {
-            log.error(
+            throw new IllegalArgumentException(
                 String.format(
                     Locale.ROOT,
                     "all weights must be in range [0.0 ... 1.0], submitted weights: %s",
                     Arrays.toString(weightsList.toArray(new Float[0]))
                 )
             );
-            throw new IllegalArgumentException("all weights must be in range [0.0 ... 1.0]");
         }
         float sumOfWeights = weightsList.stream().reduce(0.0f, Float::sum);
         if (!DoubleMath.fuzzyEquals(1.0f, sumOfWeights, DELTA_FOR_SCORE_ASSERTION)) {
-            log.error(
+            throw new IllegalArgumentException(
                 String.format(
                     Locale.ROOT,
                     "sum of weights for combination must be equal to 1.0, submitted weights: %s",
                     Arrays.toString(weightsList.toArray(new Float[0]))
                 )
             );
-            throw new IllegalArgumentException("sum of weights for combination must be equal to 1.0");
         }
     }
 }
diff --git a/src/test/java/org/opensearch/neuralsearch/processor/ScoreCombinationIT.java b/src/test/java/org/opensearch/neuralsearch/processor/ScoreCombinationIT.java
@@ -5,6 +5,8 @@
 
 package org.opensearch.neuralsearch.processor;
 
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsString;
 import static org.opensearch.neuralsearch.TestUtils.assertHybridSearchResults;
 import static org.opensearch.neuralsearch.TestUtils.assertWeightedScores;
 import static org.opensearch.neuralsearch.TestUtils.createRandomVector;
@@ -148,7 +150,14 @@ public void testArithmeticWeightedMean_whenWeightsPassed_thenSuccessful() {
             ResponseException.class,
             () -> search(TEST_MULTI_DOC_INDEX_THREE_SHARDS_NAME, hybridQueryBuilder, null, 5, Map.of("search_pipeline", SEARCH_PIPELINE))
         );
-        assertTrue(exception1.getMessage().contains("number of weights must match number of sub-queries in hybrid query"));
+        org.hamcrest.MatcherAssert.assertThat(
+            exception1.getMessage(),
+            allOf(
+                containsString("number of weights"),
+                containsString("must match number of sub-queries"),
+                containsString("in hybrid query")
+            )
+        );
 
         // check case when number of weights is more than number of sub-queries
         // delete existing pipeline and create a new one with another set of weights
@@ -164,7 +173,14 @@ public void testArithmeticWeightedMean_whenWeightsPassed_thenSuccessful() {
             ResponseException.class,
             () -> search(TEST_MULTI_DOC_INDEX_THREE_SHARDS_NAME, hybridQueryBuilder, null, 5, Map.of("search_pipeline", SEARCH_PIPELINE))
         );
-        assertTrue(exception2.getMessage().contains("number of weights must match number of sub-queries in hybrid query"));
+        org.hamcrest.MatcherAssert.assertThat(
+            exception2.getMessage(),
+            allOf(
+                containsString("number of weights"),
+                containsString("must match number of sub-queries"),
+                containsString("in hybrid query")
+            )
+        );
     }
 
     /**

diff --git a/...est/java/org/opensearch/neuralsearch/processor/combination/ScoreCombinationUtilTests.java b/...est/java/org/opensearch/neuralsearch/processor/combination/ScoreCombinationUtilTests.java
@@ -5,6 +5,9 @@
 
 package org.opensearch.neuralsearch.processor.combination;
 
+import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.containsString;
+
 import java.util.List;
 import java.util.Map;
 
@@ -49,6 +52,13 @@ public void testWeightsValidation_whenNumberOfScoresDifferentFromNumberOfWeights
             IllegalArgumentException.class,
             () -> scoreCombinationUtil.validateIfWeightsMatchScores(new float[] { 0.6f, 0.5f }, List.of(0.4f, 0.2f, 0.4f))
         );
-        assertTrue(exception1.getMessage().contains("number of weights must match number of sub-queries in hybrid query"));
+        org.hamcrest.MatcherAssert.assertThat(
+            exception1.getMessage(),
+            allOf(
+                containsString("number of weights"),
+                containsString("must match number of sub-queries"),
+                containsString("in hybrid query")
+            )
+        );
     }
 }