From 3137bd632cbb8b55d3b84cc4b41bce1177609d67 Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Mon, 22 Aug 2016 19:10:56 +0100 Subject: [PATCH] Move name and id out of Feature.init (towards fully pluggable Feature classes). (#50) --- .../ltr/feature/impl/FieldLengthFeature.java | 8 +++--- .../ltr/feature/impl/FieldValueFeature.java | 8 +++--- .../feature/impl/OriginalScoreFeature.java | 4 +++ .../solr/ltr/feature/impl/SolrFeature.java | 4 +++ .../solr/ltr/feature/impl/ValueFeature.java | 8 +++--- .../org/apache/solr/ltr/ranking/Feature.java | 25 +++++++++++-------- .../solr/ltr/ranking/FilterFeature.java | 22 +++++++++------- .../solr/ltr/rest/ManagedFeatureStore.java | 10 ++++++-- .../org/apache/solr/ltr/TestRerankBase.java | 5 ++-- .../solr/ltr/ranking/TestModelQuery.java | 8 +++--- .../ltr/ranking/TestReRankingPipeline.java | 3 +-- 11 files changed, 64 insertions(+), 41 deletions(-) diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldLengthFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldLengthFeature.java index 064d49e69a0d..4e6c5d686a03 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldLengthFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldLengthFeature.java @@ -73,14 +73,14 @@ private final float decodeNorm(long norm) { // positive above 127 } - public FieldLengthFeature() { - + public FieldLengthFeature(String name) { + super(name); } @Override - public void init(String name, Map params, int id) + public void init(Map params) throws FeatureException { - super.init(name, params, id); + super.init(params); if (!params.containsKey(CommonLTRParams.FEATURE_FIELD_PARAM)) { throw new FeatureException("missing param field"); } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldValueFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldValueFeature.java index 7422cea12eed..9db6b0b303a3 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldValueFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/FieldValueFeature.java @@ -55,14 +55,14 @@ protected LinkedHashMap paramsToMap() { return params; } - public FieldValueFeature() { - + public FieldValueFeature(String name) { + super(name); } @Override - public void init(String name, Map params, int id) + public void init(Map params) throws FeatureException { - super.init(name, params, id); + super.init(params); if (!params.containsKey(CommonLTRParams.FEATURE_FIELD_PARAM)) { throw new FeatureException("missing param field"); } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/OriginalScoreFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/OriginalScoreFeature.java index 80b9479e1367..7dc60acc37c5 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/OriginalScoreFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/OriginalScoreFeature.java @@ -32,6 +32,10 @@ public class OriginalScoreFeature extends Feature { + public OriginalScoreFeature(String name) { + super(name); + } + @Override protected LinkedHashMap paramsToMap() { return null; diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/SolrFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/SolrFeature.java index 1c8dec93b44c..e3e6754650d8 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/SolrFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/SolrFeature.java @@ -72,6 +72,10 @@ public void setFq(List fq) { this.fq = fq; } + public SolrFeature(String name) { + super(name); + } + @Override protected LinkedHashMap paramsToMap() { final LinkedHashMap params = new LinkedHashMap<>(3, 1.0f); diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/ValueFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/ValueFeature.java index bd2b2e332b3a..6199f19f3e12 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/ValueFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/feature/impl/ValueFeature.java @@ -79,12 +79,14 @@ protected LinkedHashMap paramsToMap() { return params; } - public ValueFeature() {} + public ValueFeature(String name) { + super(name); + } @Override - public void init(String name, Map params, int id) + public void init(Map params) throws FeatureException { - super.init(name, params, id); + super.init(params); final Object paramRequired = params.get(REQUIRED_PARAM); if (paramRequired != null) this.required = (boolean) paramRequired; diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/Feature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/Feature.java index 2a953fc5868f..abf9ab66447d 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/Feature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/Feature.java @@ -42,7 +42,7 @@ */ public abstract class Feature extends Query { - protected String name; + final protected String name; protected int id; @Deprecated @@ -50,23 +50,16 @@ public abstract class Feature extends Query { /** - * @param name - * Name of the feature * @param params * Custom parameters that the feature may use - * @param id - * Unique ID for this feature. Similar to feature name, except it can - * be used to directly access the feature in the global list of - * features. */ - public void init(String name, Map params, int id) + public void init(Map params) throws FeatureException { - this.name = name; this.params = params; - this.id = id; } - public Feature() { + public Feature(String name) { + this.name = name; } @Override @@ -142,6 +135,16 @@ public int getId() { return id; } + /** + * @param id + * Unique ID for this feature. Similar to feature name, except it can + * be used to directly access the feature in the global list of + * features. + */ + public void setId(int id) { + this.id = id; + } + protected abstract LinkedHashMap paramsToMap(); public LinkedHashMap toMap(String storeName) { diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/FilterFeature.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/FilterFeature.java index e18d42fb3253..66b3a2c2cbc1 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/FilterFeature.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/ranking/FilterFeature.java @@ -36,25 +36,19 @@ public class FilterFeature extends Feature { /** - * @param name - * Name of the feature * @param params * Custom parameters that the feature may use - * @param id - * Unique ID for this feature. Similar to feature name, except it can - * be used to directly access the feature in the global list of - * features. */ @Override - public void init(String name, Map params, int id) + public void init(Map params) throws FeatureException { - super.init(name, params, id); + super.init(params); throw new FeatureException(getClass().getCanonicalName() + " init is not supported ("+this+")"); } public FilterFeature(Feature in, Normalizer norm) { - super(); + super(null); this.in = in; this.norm = norm; } @@ -116,6 +110,16 @@ public int getId() { return in.getId(); } + /** + * @param id + * Unique ID for this feature. Similar to feature name, except it can + * be used to directly access the feature in the global list of + * features. + */ + public void setId(int id) { + in.setId(id); + } + protected LinkedHashMap paramsToMap() { return in.paramsToMap(); } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/rest/ManagedFeatureStore.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/rest/ManagedFeatureStore.java index e25b67e59689..e0ce7f3b1580 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/rest/ManagedFeatureStore.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/rest/ManagedFeatureStore.java @@ -127,8 +127,14 @@ public synchronized void addFeature(String name, String type, private Feature createFeature(String name, String type, Map params, int id) throws FeatureException { try { - final Feature f = solrResourceLoader.newInstance(type, Feature.class); - f.init(name, params, id); + final Feature f = solrResourceLoader.newInstance( + type, + Feature.class, + new String[0], // no sub packages + new Class[] { String.class }, + new Object[] { name }); + f.init(params); + f.setId(id); SolrPluginUtils.invokeSetters(f, params.entrySet()); return f; 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 44deee674615..b06a2d4ce722 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 @@ -328,10 +328,11 @@ protected List getFeatures(List names) final List features = new ArrayList<>(); int pos = 0; for (final String name : names) { - final ValueFeature f = new ValueFeature(); + final ValueFeature f = new ValueFeature(name); final Map params = new HashMap(); params.put("value", 10); - f.init(name, params, pos); + f.init(params); + f.setId(pos); features.add(f); ++pos; } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestModelQuery.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestModelQuery.java index b5d0008786c1..f02c11c8e656 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestModelQuery.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestModelQuery.java @@ -59,11 +59,12 @@ private IndexSearcher getSearcher(IndexReader r) { private static List makeFeatures(int[] featureIds) { final List features = new ArrayList<>(); for (final int i : featureIds) { - final ValueFeature f = new ValueFeature(); + final ValueFeature f = new ValueFeature("f" + i); Map params = new HashMap(); params.put("value", i); try { - f.init("f" + i, params, i); + f.init(params); + f.setId(i); } catch (final FeatureException e) { e.printStackTrace(); } @@ -75,8 +76,7 @@ private static List makeFeatures(int[] featureIds) { private static List makeNormalizedFeatures(int[] featureIds) { final List features = new ArrayList<>(); for (final int i : featureIds) { - final ValueFeature f = new ValueFeature(); - f.name = "f" + i; + final ValueFeature f = new ValueFeature("f" + i); f.setValue(i); f.id = i; final Normalizer n = new Normalizer() { diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestReRankingPipeline.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestReRankingPipeline.java index 3a09cd620b53..65bdf0c740d5 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestReRankingPipeline.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/ranking/TestReRankingPipeline.java @@ -65,8 +65,7 @@ private static List makeFieldValueFeatures(int[] featureIds, String field) { final List features = new ArrayList<>(); for (final int i : featureIds) { - final FieldValueFeature f = new FieldValueFeature(); - f.name = "f" + i; + final FieldValueFeature f = new FieldValueFeature("f" + i); f.setField(field); features.add(f); }