diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 6cc873a2d5d..f0cd587556a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -79,6 +79,8 @@ New Features * SOLR-13445: Preferred replicas on nodes with same system properties as the query master (Cao Manh Dat) +* SOLR-13049: Make contrib/ltr Feature.defaultValue configurable. (Stanislav Livotov, Christine Poerschke) + ================== 8.1.0 ================== Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release. diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/Feature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/Feature.java index c6e545bd46c..dc7b3b3057a 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/Feature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/Feature.java @@ -61,6 +61,7 @@ public abstract class Feature extends Query { final protected String name; private int index = -1; private float defaultValue = 0.0f; + private Object defaultValueObject = null; final private Map params; @@ -113,8 +114,21 @@ public abstract class Feature extends Query { return defaultValue; } - public void setDefaultValue(String value){ - defaultValue = Float.parseFloat(value); + public void setDefaultValue(Object obj){ + this.defaultValueObject = obj; + if (obj instanceof String) { + defaultValue = Float.parseFloat((String)obj); + } else if (obj instanceof Double) { + defaultValue = ((Double) obj).floatValue(); + } else if (obj instanceof Float) { + defaultValue = ((Float) obj).floatValue(); + } else if (obj instanceof Integer) { + defaultValue = ((Integer) obj).floatValue(); + } else if (obj instanceof Long) { + defaultValue = ((Long) obj).floatValue(); + } else { + throw new FeatureException("Invalid type for 'defaultValue' in params for " + this); + } } @@ -184,6 +198,15 @@ public abstract class Feature extends Query { } public abstract LinkedHashMap paramsToMap(); + + protected LinkedHashMap defaultParamsToMap() { + final LinkedHashMap params = new LinkedHashMap<>(); + if (defaultValueObject != null) { + params.put("defaultValue", defaultValueObject); + } + return params; + } + /** * Weight for a feature **/ diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java index b2fc1548913..7bbfdb813af 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldLengthFeature.java @@ -56,7 +56,7 @@ public class FieldLengthFeature extends Feature { @Override public LinkedHashMap paramsToMap() { - final LinkedHashMap params = new LinkedHashMap<>(1, 1.0f); + final LinkedHashMap params = defaultParamsToMap(); params.put("field", field); return params; } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java index 78cdbaa7f98..d12795d2663 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/FieldValueFeature.java @@ -58,7 +58,7 @@ public class FieldValueFeature extends Feature { @Override public LinkedHashMap paramsToMap() { - final LinkedHashMap params = new LinkedHashMap<>(1, 1.0f); + final LinkedHashMap params = defaultParamsToMap(); params.put("field", field); return params; } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java index fc2fb909048..fa9a2e7a57d 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/OriginalScoreFeature.java @@ -47,7 +47,7 @@ public class OriginalScoreFeature extends Feature { @Override public LinkedHashMap paramsToMap() { - return null; + return defaultParamsToMap(); } @Override diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java index d9851f5936f..44b6c09ad08 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/SolrFeature.java @@ -92,7 +92,7 @@ public class SolrFeature extends Feature { @Override public LinkedHashMap paramsToMap() { - final LinkedHashMap params = new LinkedHashMap<>(3, 1.0f); + final LinkedHashMap params = defaultParamsToMap(); if (df != null) { params.put("df", df); } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/ValueFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/ValueFeature.java index f423ad9124a..80e7d8cb57a 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/ValueFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/ValueFeature.java @@ -82,7 +82,7 @@ public class ValueFeature extends Feature { @Override public LinkedHashMap paramsToMap() { - final LinkedHashMap params = new LinkedHashMap<>(2, 1.0f); + final LinkedHashMap params = defaultParamsToMap(); params.put("value", value); if (required != null) { params.put("required", required); diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java index e1eae197712..b9c39f496e9 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/TestRerankBase.java @@ -24,6 +24,7 @@ import java.nio.file.Files; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Scanner; @@ -475,4 +476,59 @@ public class TestRerankBase extends RestTestBase { scn.close(); } + protected static void doTestParamsToMap(String featureClassName, + LinkedHashMap featureParams) throws Exception { + + // start with default parameters + final LinkedHashMap paramsA = new LinkedHashMap(); + final Object defaultValue; + switch (random().nextInt(6)) { + case 0: + defaultValue = null; + break; + case 1: + defaultValue = "1.2"; + break; + case 2: + defaultValue = Double.valueOf(3.4d); + break; + case 3: + defaultValue = Float.valueOf(0.5f); + break; + case 4: + defaultValue = Integer.valueOf(67); + break; + case 5: + defaultValue = Long.valueOf(89); + break; + default: + defaultValue = null; + fail("unexpected defaultValue choice"); + break; + } + if (defaultValue != null) { + paramsA.put("defaultValue", defaultValue); + } + + // then add specific parameters + paramsA.putAll(featureParams); + + // next choose a random feature name + final String featureName = "randomFeatureName"+random().nextInt(10); + + // create a feature from the parameters + final Feature featureA = Feature.getInstance(solrResourceLoader, + featureClassName, featureName, paramsA); + + // turn the feature back into parameters + final LinkedHashMap paramsB = featureA.paramsToMap(); + + // create feature B from feature A's parameters + final Feature featureB = Feature.getInstance(solrResourceLoader, + featureClassName, featureName, paramsB); + + // check that feature A and feature B are identical + assertEquals(featureA, featureB); + } + } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java index 18729088a31..64f9778ca61 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldLengthFeature.java @@ -16,6 +16,8 @@ */ package org.apache.solr.ltr.feature; +import java.util.LinkedHashMap; + import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.ltr.TestRerankBase; import org.apache.solr.ltr.model.LinearModel; @@ -149,6 +151,12 @@ public class TestFieldLengthFeature extends TestRerankBase { assertJQ("/query" + query.toQueryString(), "/response/docs/[3]/id=='1'"); } + @Test + public void testParamsToMap() throws Exception { + final LinkedHashMap params = new LinkedHashMap(); + params.put("field", "field"+random().nextInt(10)); + doTestParamsToMap(FieldLengthFeature.class.getName(), params); + } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldValueFeature.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldValueFeature.java index 4fceddaf43a..ceaf6e6c8fd 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldValueFeature.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestFieldValueFeature.java @@ -16,6 +16,8 @@ */ package org.apache.solr.ltr.feature; +import java.util.LinkedHashMap; + import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.ltr.FeatureLoggerTestUtils; import org.apache.solr.ltr.TestRerankBase; @@ -204,5 +206,11 @@ public class TestFieldValueFeature extends TestRerankBase { } + @Test + public void testParamsToMap() throws Exception { + final LinkedHashMap params = new LinkedHashMap(); + params.put("field", "field"+random().nextInt(10)); + doTestParamsToMap(FieldValueFeature.class.getName(), params); + } } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestOriginalScoreFeature.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestOriginalScoreFeature.java index a76aa8e8402..b0af388ddca 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestOriginalScoreFeature.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestOriginalScoreFeature.java @@ -17,6 +17,7 @@ package org.apache.solr.ltr.feature; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.Map; import org.apache.solr.client.solrj.SolrQuery; @@ -152,4 +153,10 @@ public class TestOriginalScoreFeature extends TestRerankBase { } } + @Test + public void testParamsToMap() throws Exception { + final LinkedHashMap params = new LinkedHashMap(); + doTestParamsToMap(OriginalScoreFeature.class.getName(), params); + } + } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestRankingFeature.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestRankingFeature.java index 1d28dd88c68..8db1a4bceb4 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestRankingFeature.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestRankingFeature.java @@ -16,6 +16,8 @@ */ package org.apache.solr.ltr.feature; +import java.util.LinkedHashMap; + import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.ltr.TestRerankBase; import org.apache.solr.ltr.model.LinearModel; @@ -120,4 +122,11 @@ public class TestRankingFeature extends TestRerankBase { } + @Test + public void testParamsToMap() throws Exception { + final LinkedHashMap params = new LinkedHashMap(); + params.put("q", "{!func}pow(popularity,2)"); + doTestParamsToMap(SolrFeature.class.getName(), params); + } + } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestValueFeature.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestValueFeature.java index af23fdc9cfe..2b22bfee341 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestValueFeature.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/feature/TestValueFeature.java @@ -16,6 +16,8 @@ */ package org.apache.solr.ltr.feature; +import java.util.LinkedHashMap; + import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.ltr.TestRerankBase; import org.apache.solr.ltr.model.LinearModel; @@ -162,4 +164,14 @@ public class TestValueFeature extends TestRerankBase { assertJQ("/query" + query.toQueryString(), "/responseHeader/status==400"); } + @Test + public void testParamsToMap() throws Exception { + final LinkedHashMap params = new LinkedHashMap(); + params.put("value", "${val"+random().nextInt(10)+"}"); + if (random().nextBoolean()) { + params.put("required", random().nextBoolean()); + } + doTestParamsToMap(ValueFeature.class.getName(), params); + } + }