Move name and id out of Feature.init (towards fully pluggable Featur…

…e classes). (apache#50)
mnilsson23 · Aug 22, 2016 · 3137bd6 · 3137bd6
1 parent 83f5306
commit 3137bd6
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 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
@@ -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<String,Object> params, int id)
+  public void init(Map<String,Object> 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
@@ -55,14 +55,14 @@ protected LinkedHashMap<String,Object> paramsToMap() {
     return params;
   }
 
-  public FieldValueFeature() {
-
+  public FieldValueFeature(String name) {
+    super(name);
   }
 
   @Override
-  public void init(String name, Map<String,Object> params, int id)
+  public void init(Map<String,Object> 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
@@ -32,6 +32,10 @@
 
 public class OriginalScoreFeature extends Feature {
 
+  public OriginalScoreFeature(String name) {
+    super(name);
+  }
+
   @Override
   protected LinkedHashMap<String,Object> 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
@@ -72,6 +72,10 @@ public void setFq(List<String> fq) {
     this.fq = fq;
   }
 
+  public SolrFeature(String name) {
+    super(name);
+  }
+
   @Override
   protected LinkedHashMap<String,Object> paramsToMap() {
     final LinkedHashMap<String,Object> 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
@@ -79,12 +79,14 @@ protected LinkedHashMap<String,Object> paramsToMap() {
     return params;
   }
 
-  public ValueFeature() {}
+  public ValueFeature(String name) {
+    super(name);
+  }
 
   @Override
-  public void init(String name, Map<String,Object> params, int id)
+  public void init(Map<String,Object> 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
@@ -42,31 +42,24 @@
  */
 public abstract class Feature extends Query {
 
-  protected String name;
+  final protected String name;
   protected int id;
 
   @Deprecated
   private Map<String,Object> params = LTRUtils.EMPTY_MAP;
 
 
   /**
-   * @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<String,Object>  params, int id)
+  public void init(Map<String,Object> 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<String,Object> paramsToMap();
 
   public LinkedHashMap<String,Object> 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
@@ -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<String,Object> params, int id)
+  public void init(Map<String,Object> 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<String,Object> 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
@@ -127,8 +127,14 @@ public synchronized void addFeature(String name, String type,
   private Feature createFeature(String name, String type, Map<String,Object> 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
@@ -328,10 +328,11 @@ protected List<Feature> getFeatures(List<String> names)
     final List<Feature> features = new ArrayList<>();
     int pos = 0;
     for (final String name : names) {
-      final ValueFeature f = new ValueFeature();
+      final ValueFeature f = new ValueFeature(name);
       final Map<String,Object> params = new HashMap<String,Object>();
       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
@@ -59,11 +59,12 @@ private IndexSearcher getSearcher(IndexReader r) {
   private static List<Feature> makeFeatures(int[] featureIds) {
     final List<Feature> features = new ArrayList<>();
     for (final int i : featureIds) {
-      final ValueFeature f = new ValueFeature();
+      final ValueFeature f = new ValueFeature("f" + i);
       Map<String,Object> params = new HashMap<String,Object>();
       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<Feature> makeFeatures(int[] featureIds) {
   private static List<Feature> makeNormalizedFeatures(int[] featureIds) {
     final List<Feature> 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
@@ -65,8 +65,7 @@ private static List<Feature> makeFieldValueFeatures(int[] featureIds,
       String field) {
     final List<Feature> 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);
     }