SOLR-10218: The Schema API commands add-field-type and replace-field-type improperly specify SimilarityFactory params

solr/CHANGES.txt

@@ -305,6 +305,9 @@ Bug Fixes
 
 * SOLR-10237: Poly-fields should work with subfields that have docValues=true (Tomás Fernández Löbbe, David Smiley)
 
+* SOLR-10218: The Schema API commands "add-field-type" and "replace-field-type" improperly specify SimilarityFactory params.
+  (Benjamin Deininger, Troy Mohl, Steve Rowe)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/rest/schema/FieldTypeXmlAdapter.java

@@ -25,7 +25,9 @@ import javax.xml.parsers.ParserConfigurationException;
 
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrException.ErrorCode;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.schema.SimilarityFactory;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
@@ -83,9 +85,30 @@ public class FieldTypeXmlAdapter {
           jsonFieldName+" not a "+jsonField.getClass().getName());
 
     Element similarity = doc.createElement("similarity");
-    appendAttrs(similarity, (Map<String,?>)jsonField);
+    Map<String,?> config = (Map<String,?>)jsonField;
+    similarity.setAttribute(SimilarityFactory.CLASS_NAME, (String)config.remove(SimilarityFactory.CLASS_NAME));
+    for (Map.Entry<String,?> entry : config.entrySet()) {
+      Object val = entry.getValue();
+      if (val != null) {
+        Element child = doc.createElement(classToXmlTag(val.getClass()));
+        child.setAttribute(CommonParams.NAME, entry.getKey());
+        child.setTextContent(entry.getValue().toString());
+        similarity.appendChild(child);
+      }
+    }
     return similarity;
   }
+
+  /** Convert types produced by noggit's ObjectBuilder (Boolean, Double, Long, String) to plugin param XML tags. */
+  protected static String classToXmlTag(Class<?> clazz) {
+    switch (clazz.getSimpleName()) {
+      case "Boolean": return "bool";
+      case "Double":  return "double";
+      case "Long":    return "long";
+      case "String":  return "str";
+    }
+    throw new SolrException(ErrorCode.BAD_REQUEST, "Unsupported object type '" + clazz.getSimpleName() + "'");
+  }
   
   @SuppressWarnings("unchecked")
   protected static Element transformAnalyzer(Document doc, Map<String,?> json, String jsonFieldName, String analyzerType) {

solr/core/src/test/org/apache/solr/rest/schema/TestBulkSchemaAPI.java

@@ -18,6 +18,7 @@ package org.apache.solr.rest.schema;
 
 import org.apache.commons.io.FileUtils;
 
+import org.apache.lucene.search.similarities.DFISimilarity;
 import org.apache.lucene.search.similarities.PerFieldSimilarityWrapper;
 import org.apache.lucene.search.similarities.BM25Similarity;
 import org.apache.lucene.misc.SweetSpotSimilarity;
@@ -42,10 +43,12 @@ import java.io.File;
 import java.io.StringReader;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Consumer;
 
 
 public class TestBulkSchemaAPI extends RestTestBase {
@@ -798,6 +801,68 @@ public class TestBulkSchemaAPI extends RestTestBase {
     assertNull(map.get("errors"));
   }
 
+  public void testSimilarityParser() throws Exception {
+    RestTestHarness harness = restTestHarness;
+
+    final float k1 = 2.25f;
+    final float b = 0.33f;
+
+    String fieldTypeName = "MySimilarityField";
+    String fieldName = "similarityTestField";
+    String payload = "{\n" +
+        "  'add-field-type' : {" +
+        "    'name' : '" + fieldTypeName + "',\n" +
+        "    'class':'solr.TextField',\n" +
+        "    'analyzer' : {'tokenizer':{'class':'solr.WhitespaceTokenizerFactory'}},\n" +
+        "    'similarity' : {'class':'org.apache.solr.search.similarities.BM25SimilarityFactory', 'k1':"+k1+", 'b':"+b+" }\n" +
+        "  },\n"+
+        "  'add-field' : {\n" +
+        "    'name':'" + fieldName + "',\n" +
+        "    'type': 'MySimilarityField',\n" +
+        "    'stored':true,\n" +
+        "    'indexed':true\n" +
+        "  }\n" +
+        "}\n";
+
+    String response = harness.post("/schema?wt=json&indent=on", json(payload));
+
+    Map map = (Map) ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
+    assertNull(response, map.get("errors"));
+
+    Map fields = getObj(harness, fieldName, "fields");
+    assertNotNull("field " + fieldName + " not created", fields);
+    
+    assertFieldSimilarity(fieldName, BM25Similarity.class,
+       sim -> assertEquals("Unexpected k1", k1, sim.getK1(), .001),
+       sim -> assertEquals("Unexpected b", b, sim.getB(), .001));
+
+    final String independenceMeasure = "Saturated";
+    final boolean discountOverlaps = false; 
+    payload = "{\n" +
+        "  'replace-field-type' : {" +
+        "    'name' : '" + fieldTypeName + "',\n" +
+        "    'class':'solr.TextField',\n" +
+        "    'analyzer' : {'tokenizer':{'class':'solr.WhitespaceTokenizerFactory'}},\n" +
+        "    'similarity' : {\n" +
+        "      'class':'org.apache.solr.search.similarities.DFISimilarityFactory',\n" +
+        "      'independenceMeasure':'" + independenceMeasure + "',\n" +
+        "      'discountOverlaps':" + discountOverlaps + "\n" +
+        "     }\n" +
+        "  }\n"+
+        "}\n";
+
+    response = harness.post("/schema?wt=json&indent=on", json(payload));
+
+    map = (Map)ObjectBuilder.getVal(new JSONParser(new StringReader(response)));
+    assertNull(response, map.get("errors"));
+    fields = getObj(harness, fieldName, "fields");
+    assertNotNull("field " + fieldName + " not created", fields);
+
+    assertFieldSimilarity(fieldName, DFISimilarity.class,
+        sim -> assertEquals("Unexpected independenceMeasure", independenceMeasure, sim.getIndependence().toString()),
+        sim -> assertEquals("Unexpected discountedOverlaps", discountOverlaps, sim.getDiscountOverlaps()));
+  }
+
   public static Map getObj(RestTestHarness restHarness, String fld, String key) throws Exception {
     Map map = getRespMap(restHarness);
     List l = (List) ((Map)map.get("schema")).get(key);
@@ -842,8 +907,11 @@ public class TestBulkSchemaAPI extends RestTestBase {
 
   /**
    * whitebox checks the Similarity for the specified field according to {@link SolrCore#getLatestSchema}
+   * 
+   * Executes each of the specified Similarity-accepting validators.
    */
-  private static void assertFieldSimilarity(String fieldname, Class<? extends Similarity> expected) {
+  @SafeVarargs
+  private static <T extends Similarity> void assertFieldSimilarity(String fieldname, Class<T> expected, Consumer<T>... validators) {
     CoreContainer cc = jetty.getCoreContainer();
     try (SolrCore core = cc.getCore("collection1")) {
       SimilarityFactory simfac = core.getLatestSchema().getSimilarityFactory();
@@ -861,7 +929,7 @@ public class TestBulkSchemaAPI extends RestTestBase {
                  mainSim instanceof PerFieldSimilarityWrapper);
       Similarity fieldSim = ((PerFieldSimilarityWrapper)mainSim).get(fieldname);
       assertEquals("wrong sim for field=" + fieldname, expected, fieldSim.getClass());
-      
+      Arrays.asList(validators).forEach(v -> v.accept((T)fieldSim));
     }
   }
 }