Refactor: Eagerly parse first and second phase

3 files changed, 24 insertions, 20 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
index 30737365844..b687fac6c12 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -362,22 +362,24 @@ public class RankProfile implements Serializable, Cloneable {
         this.secondPhaseRanking = rankingExpression;
     }
 
-    /**
-     * Called by parser to store the expression string, for delayed evaluation
-     *
-     * @param exp ranking expression for second phase
-     */
-    public void setSecondPhaseRankingString(String exp) {
-        this.secondPhaseRankingString = exp;
+    public void setFirstPhaseRankingString(String expression) {
+        try {
+            this.firstPhaseRanking = parseRankingExpression("firstphase", expression);
+            this.firstPhaseRankingString = expression;
+        }
+        catch (ParseException e) {
+            throw new IllegalArgumentException("Illegal first phase ranking function", e);
+        }
     }
 
-    /**
-     * Called by parser to store the expression string, for delayed evaluation
-     *
-     * @param exp ranking expression for first phase
-     */
-    public void setFirstPhaseRankingString(String exp) {
-        this.firstPhaseRankingString = exp;
+    public void setSecondPhaseRankingString(String expression) {
+        try {
+            this.secondPhaseRanking = parseRankingExpression("secondphase", expression);
+            this.secondPhaseRankingString = expression;
+        }
+        catch (ParseException e) {
+            throw new IllegalArgumentException("Illegal second phase ranking function", e);
+        }
     }
 
     /** Returns a read-only view of the summary features to use in this profile. This is never null */
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/IncorrectRankingExpressionFileRefTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/IncorrectRankingExpressionFileRefTestCase.java
index 03fa92f5cb9..07a36832094 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/IncorrectRankingExpressionFileRefTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/IncorrectRankingExpressionFileRefTestCase.java
@@ -5,10 +5,12 @@ import com.yahoo.search.query.profile.QueryProfileRegistry;
 import com.yahoo.searchdefinition.derived.DerivedConfiguration;
 import com.yahoo.searchdefinition.parser.ParseException;
 import com.yahoo.searchlib.rankingexpression.integration.ml.ImportedModels;
+import com.yahoo.yolean.Exceptions;
 import org.junit.Test;
 
 import java.io.IOException;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -27,9 +29,9 @@ public class IncorrectRankingExpressionFileRefTestCase extends SearchDefinitionT
             new DerivedConfiguration(search, registry, new QueryProfileRegistry(), new ImportedModels()); // cause rank profile parsing
             fail("parsing should have failed");
         } catch (IllegalArgumentException e) {
-            e.printStackTrace();
-            assertTrue(e.getCause().getMessage().contains("Could not read ranking expression file"));
-            assertTrue(e.getCause().getMessage().contains("wrongending.expr.expression"));
+            String message = Exceptions.toMessageString(e);
+            assertTrue(message.contains("Could not read ranking expression file"));
+            assertTrue(message.contains("wrongending.expr.expression"));
         }
     }
 
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionValidationTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionValidationTestCase.java
index 3fe3a7c3de1..5e649c2e551 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionValidationTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionValidationTestCase.java
@@ -5,9 +5,11 @@ import com.yahoo.search.query.profile.QueryProfileRegistry;
 import com.yahoo.searchdefinition.derived.DerivedConfiguration;
 import com.yahoo.searchdefinition.parser.ParseException;
 import com.yahoo.searchlib.rankingexpression.integration.ml.ImportedModels;
+import com.yahoo.yolean.Exceptions;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 /**
@@ -29,9 +31,7 @@ public class RankingExpressionValidationTestCase extends SearchDefinitionTestCas
             fail("No exception on incorrect ranking expression " + expression);
         } catch (IllegalArgumentException e) {
             // Success
-            // TODO: Where's the "com.yahoo.searchdefinition.parser.ParseException:"  nonsense coming from?
-            assertTrue("Got unexpected error message: " + e.getCause().getMessage(),
-                       e.getCause().getMessage().startsWith("com.yahoo.searchdefinition.parser.ParseException: Could not parse ranking expression '" + expression + "'"));
+            assertTrue(Exceptions.toMessageString(e).startsWith("Illegal first phase ranking function: Could not parse ranking expression '" + expression + "' in default, firstphase.:"));
         }
     }