SOLR-2920: Refactor frequent conditional use of DefaultSolrParams and AppendedSolrParams into factory methods

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1213474 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -429,6 +429,10 @@ Other Changes
 ----------------------
 * SOLR-2922: Upgrade commons-io and commons-lang to 2.1 and 2.6, respectively. (koji)
 
+* SOLR-2920: Refactor frequent conditional use of DefaultSolrParams and 
+  AppendedSolrParams into factory methods.
+  (David Smiley via hossman)
+
 ==================  3.5.0  ==================
 
 New Features

solr/core/src/java/org/apache/solr/highlight/GapFragmenter.java

@@ -22,7 +22,6 @@ import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
 import org.apache.lucene.search.highlight.Fragmenter;
 import org.apache.lucene.search.highlight.NullFragmenter;
 import org.apache.lucene.search.highlight.SimpleFragmenter;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.HighlightParams;
 import org.apache.solr.common.params.SolrParams;
 
@@ -31,9 +30,7 @@ public class GapFragmenter extends HighlightingPluginBase implements SolrFragmen
   public Fragmenter getFragmenter(String fieldName, SolrParams params )
   {
     numRequests++;
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
+    params = SolrParams.wrapDefaults(params, defaults);
     
     int fragsize = params.getFieldInt( fieldName, HighlightParams.FRAGSIZE, 100 );
     return (fragsize <= 0) ? new NullFragmenter() : new LuceneGapFragmenter(fragsize);

solr/core/src/java/org/apache/solr/highlight/HtmlFormatter.java

@@ -18,7 +18,6 @@ package org.apache.solr.highlight;
 
 import org.apache.lucene.search.highlight.Formatter;
 import org.apache.lucene.search.highlight.SimpleHTMLFormatter;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.HighlightParams;
 import org.apache.solr.common.params.SolrParams;
 
@@ -30,10 +29,8 @@ public class HtmlFormatter extends HighlightingPluginBase implements SolrFormatt
   public Formatter getFormatter(String fieldName, SolrParams params ) 
   {
     numRequests++;
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
-    
+    params = SolrParams.wrapDefaults(params, defaults);
+
     return new SimpleHTMLFormatter(
         params.getFieldParam(fieldName, HighlightParams.SIMPLE_PRE,  "<em>" ), 
         params.getFieldParam(fieldName, HighlightParams.SIMPLE_POST, "</em>"));

solr/core/src/java/org/apache/solr/highlight/RegexFragmenter.java

@@ -26,7 +26,6 @@ import org.apache.lucene.analysis.tokenattributes.OffsetAttribute;
 import org.apache.lucene.analysis.tokenattributes.PositionIncrementAttribute;
 import org.apache.lucene.search.highlight.Fragmenter;
 import org.apache.lucene.search.highlight.NullFragmenter;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.HighlightParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -66,10 +65,9 @@ public class RegexFragmenter extends HighlightingPluginBase implements SolrFragm
 
   public Fragmenter getFragmenter(String fieldName, SolrParams params )
   { 
-    numRequests++;        
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
+    numRequests++;
+    params = SolrParams.wrapDefaults(params, defaults);
+
     int fragsize  = params.getFieldInt(   fieldName, HighlightParams.FRAGSIZE,  LuceneRegexFragmenter.DEFAULT_FRAGMENT_SIZE );
     int increment = params.getFieldInt(   fieldName, HighlightParams.INCREMENT, LuceneRegexFragmenter.DEFAULT_INCREMENT_GAP );
     float slop    = params.getFieldFloat( fieldName, HighlightParams.SLOP,      LuceneRegexFragmenter.DEFAULT_SLOP );

solr/core/src/java/org/apache/solr/highlight/SimpleFragListBuilder.java

@@ -18,18 +18,18 @@
 package org.apache.solr.highlight;
 
 import org.apache.lucene.search.vectorhighlight.FragListBuilder;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.SolrParams;
 
 public class SimpleFragListBuilder extends HighlightingPluginBase implements
     SolrFragListBuilder {
 
   public FragListBuilder getFragListBuilder(SolrParams params) {
+    // NOTE: This class (currently) makes no use of params
+    // If that ever changes, it should wrap them with defaults...
+    // params = SolrParams.wrapDefaults(params, defaults)
+
     numRequests++;
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
-    
+
     return new org.apache.lucene.search.vectorhighlight.SimpleFragListBuilder();
   }
 

solr/core/src/java/org/apache/solr/highlight/SingleFragListBuilder.java

@@ -18,18 +18,18 @@
 package org.apache.solr.highlight;
 
 import org.apache.lucene.search.vectorhighlight.FragListBuilder;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.SolrParams;
 
 public class SingleFragListBuilder extends HighlightingPluginBase implements
     SolrFragListBuilder {
 
   public FragListBuilder getFragListBuilder(SolrParams params) {
+    // NOTE: This class (currently) makes no use of params
+    // If that ever changes, it should wrap them with defaults...
+    // params = SolrParams.wrapDefaults(params, defaults)
+
     numRequests++;
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
-    
+
     return new org.apache.lucene.search.vectorhighlight.SingleFragListBuilder();
   }
 

solr/core/src/java/org/apache/solr/highlight/SolrBoundaryScanner.java

@@ -18,7 +18,6 @@
 package org.apache.solr.highlight;
 
 import org.apache.lucene.search.vectorhighlight.BoundaryScanner;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.core.SolrInfoMBean;
 import org.apache.solr.util.plugin.NamedListInitializedPlugin;
@@ -28,9 +27,8 @@ public abstract class SolrBoundaryScanner extends HighlightingPluginBase impleme
 
   public BoundaryScanner getBoundaryScanner(String fieldName, SolrParams params){
     numRequests++;
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
+    params = SolrParams.wrapDefaults(params, defaults);
+
     return get(fieldName, params);
   }
   

solr/core/src/java/org/apache/solr/highlight/SolrFragmentsBuilder.java

@@ -20,7 +20,6 @@ package org.apache.solr.highlight;
 import org.apache.lucene.search.vectorhighlight.BoundaryScanner;
 import org.apache.lucene.search.vectorhighlight.FragmentsBuilder;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.HighlightParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.core.SolrInfoMBean;
@@ -40,9 +39,8 @@ public abstract class SolrFragmentsBuilder extends HighlightingPluginBase
    */
   public FragmentsBuilder getFragmentsBuilder(SolrParams params, BoundaryScanner bs) {
     numRequests++;
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
+    params = SolrParams.wrapDefaults(params, defaults);
+
     return getFragmentsBuilder( params, getPreTags( params, null ), getPostTags( params, null ), bs );
   }
   
@@ -55,9 +53,8 @@ public abstract class SolrFragmentsBuilder extends HighlightingPluginBase
   }
   
   private String[] getTags( SolrParams params, String paramName, String fieldName, String def ){
-    if( defaults != null ) {
-      params = new DefaultSolrParams( params, defaults );
-    }
+    params = SolrParams.wrapDefaults(params, defaults);
+
     String value = null;
     if( fieldName == null )
       value = params.get( paramName, def );

solr/core/src/java/org/apache/solr/search/DisMaxQParser.java

@@ -23,7 +23,6 @@ import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.Query;
 import org.apache.solr.schema.IndexSchema;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.DisMaxParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -81,7 +80,8 @@ public class DisMaxQParser extends QParser {
 
   @Override
   public Query parse() throws ParseException {
-    SolrParams solrParams = localParams == null ? params : new DefaultSolrParams(localParams, params);
+    SolrParams solrParams = SolrParams.wrapDefaults(localParams, params);
+
     queryFields = SolrPluginUtils.parseFieldBoosts(solrParams.getParams(DisMaxParams.QF));
     if (0 == queryFields.size()) {
       queryFields.put(req.getSchema().getDefaultSearchFieldName(), 1.0f);

solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParserPlugin.java

@@ -31,8 +31,6 @@ import org.apache.lucene.queryparser.classic.ParseException;
 import org.apache.lucene.queryparser.classic.QueryParser;
 import org.apache.lucene.search.*;
 import org.apache.lucene.analysis.Analyzer;
-import org.apache.lucene.analysis.TokenStream;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.DisMaxParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -42,8 +40,6 @@ import org.apache.solr.util.SolrPluginUtils;
 import org.apache.solr.analysis.*;
 
 import java.util.*;
-import java.io.Reader;
-import java.io.IOException;
 
 /**
  * An advanced multi-field query parser.
@@ -102,7 +98,7 @@ class ExtendedDismaxQParser extends QParser {
     SolrParams localParams = getLocalParams();
     SolrParams params = getParams();
     
-    SolrParams solrParams = localParams == null ? params : new DefaultSolrParams(localParams, params);
+    SolrParams solrParams = SolrParams.wrapDefaults(localParams, params);
 
     final String minShouldMatch = 
       DisMaxQParser.parseMinShouldMatch(req.getSchema(), solrParams);

solr/core/src/java/org/apache/solr/util/SolrPluginUtils.java

@@ -25,10 +25,8 @@ import org.apache.lucene.search.BooleanClause.Occur;
 import org.apache.solr.common.SolrDocument;
 import org.apache.solr.common.SolrDocumentList;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.AppendedSolrParams;
-import org.apache.solr.common.params.DefaultSolrParams;
-import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.params.UpdateParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
@@ -81,15 +79,10 @@ public class SolrPluginUtils {
                                  SolrParams appends, SolrParams invariants) {
 
       SolrParams p = req.getParams();
-      if (defaults != null) {
-        p = new DefaultSolrParams(p,defaults);
-      }
-      if (appends != null) {
-        p = new AppendedSolrParams(p,appends);
-      }
-      if (invariants != null) {
-        p = new DefaultSolrParams(invariants,p);
-      }
+      p = SolrParams.wrapDefaults(p, defaults);
+      p = SolrParams.wrapAppended(p, appends);
+      p = SolrParams.wrapDefaults(invariants, p);
+
       req.setParams(p);
   }
 

solr/core/src/test/org/apache/solr/BasicFunctionalityTest.java

@@ -34,9 +34,7 @@ import org.apache.lucene.index.IndexWriter;
 import org.apache.lucene.index.IndexableField;
 import org.apache.lucene.index.LogMergePolicy;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.AppendedSolrParams;
 import org.apache.solr.common.params.CommonParams;
-import org.apache.solr.common.params.DefaultSolrParams;
 import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.NamedList;
@@ -448,7 +446,7 @@ public class BasicFunctionalityTest extends SolrTestCaseJ4 {
     assertEquals(p.getInt("iii",5), 5);
     assertEquals(p.getFieldParam("field1","i"), "555");
 
-    req.setParams(new DefaultSolrParams(p, new MapSolrParams(m)));
+    req.setParams(SolrParams.wrapDefaults(p, new MapSolrParams(m)));
     p = req.getParams();
     assertEquals(req.getOriginalParams().get("s"), "bbb");
     assertEquals(p.get("i"), "555");
@@ -470,7 +468,7 @@ public class BasicFunctionalityTest extends SolrTestCaseJ4 {
     more.add("s", "ccc");
     more.add("ss","YYY");
     more.add("xx","XXX");
-    p = new AppendedSolrParams(p, SolrParams.toSolrParams(more));
+    p = SolrParams.wrapAppended(p, SolrParams.toSolrParams(more));
     assertEquals(3, p.getParams("s").length);
     assertEquals("bbb", p.getParams("s")[0]);
     assertEquals("aaa", p.getParams("s")[1]);

solr/solrj/src/java/org/apache/solr/client/solrj/impl/CommonsHttpSolrServer.java

@@ -47,7 +47,9 @@ import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.util.ClientUtils;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
-import org.apache.solr.common.params.*;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.ContentStream;
 import org.apache.solr.common.util.NamedList;
 import org.slf4j.Logger;
@@ -266,16 +268,8 @@ public class CommonsHttpSolrServer extends SolrServer
     ModifiableSolrParams wparams = new ModifiableSolrParams();
     wparams.set( CommonParams.WT, parser.getWriterType() );
     wparams.set( CommonParams.VERSION, parser.getVersion());
-    if( params == null ) {
-      params = wparams;
-    }
-    else {
-      params = new DefaultSolrParams( wparams, params );
-    }
-    
-    if( _invariantParams != null ) {
-      params = new DefaultSolrParams( _invariantParams, params );
-    }
+    params = SolrParams.wrapDefaults(wparams, params);
+    params = SolrParams.wrapDefaults(_invariantParams, params);
 
     int tries = _maxRetries + 1;
     try {

solr/solrj/src/java/org/apache/solr/common/params/AppendedSolrParams.java

@@ -23,6 +23,11 @@ package org.apache.solr.common.params;
  * that all of the values are returned. 
  */
 public class AppendedSolrParams extends DefaultSolrParams {
+
+  /**
+   * @deprecated (3.6) Use {@link SolrParams#wrapAppended(SolrParams, SolrParams)} instead.
+   */
+  @Deprecated
   public AppendedSolrParams(SolrParams main, SolrParams extra) {
     super(main, extra);
   }

solr/solrj/src/java/org/apache/solr/common/params/DefaultSolrParams.java

@@ -28,7 +28,12 @@ public class DefaultSolrParams extends SolrParams {
   protected final SolrParams params;
   protected final SolrParams defaults;
 
+  /**
+   * @deprecated (3.6) Use {@link SolrParams#wrapDefaults(SolrParams, SolrParams)} instead.
+   */
+  @Deprecated
   public DefaultSolrParams(SolrParams params, SolrParams defaults) {
+    assert params != null && defaults != null;
     this.params = params;
     this.defaults = defaults;
   }

solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java

@@ -32,7 +32,7 @@ import org.apache.solr.common.util.StrUtils;
  *
  */
 public abstract class SolrParams implements Serializable {
-  
+
   /** returns the String value of a param, or null if not set */
   public abstract String get(String param);
 
@@ -249,6 +249,24 @@ public abstract class SolrParams implements Serializable {
     }
   }
 
+  @SuppressWarnings({"deprecation"})
+  public static SolrParams wrapDefaults(SolrParams params, SolrParams defaults) {
+    if (params == null)
+      return defaults;
+    if (defaults == null)
+      return params;
+    return new DefaultSolrParams(params,defaults);
+  }
+
+  @SuppressWarnings({"deprecation"})
+  public static SolrParams wrapAppended(SolrParams params, SolrParams defaults) {
+    if (params == null)
+      return defaults;
+    if (defaults == null)
+      return params;
+    return new AppendedSolrParams(params,defaults);
+  }
+
   /** Create a Map<String,String> from a NamedList given no keys are repeated */
   public static Map<String,String> toMap(NamedList params) {
     HashMap<String,String> map = new HashMap<String,String>();

solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java

@@ -22,9 +22,6 @@ import java.util.Map;
 
 import org.apache.lucene.util.LuceneTestCase;
 import org.apache.solr.common.SolrException;
-import org.apache.solr.common.params.DefaultSolrParams;
-import org.apache.solr.common.params.MapSolrParams;
-import org.apache.solr.common.params.SolrParams;
 
 /**
  */
@@ -158,7 +155,7 @@ public class SolrParamTest extends LuceneTestCase
     dmap.put( "dint"               , "123"       );
     // these are defined in params
     dmap.put( "int"                , "456"       );
-    SolrParams defaults = new DefaultSolrParams( params, new MapSolrParams( dmap ) );
+    SolrParams defaults = SolrParams.wrapDefaults(params, new MapSolrParams(dmap));
   
     // in params, not in default
     assertEquals( pstr                  , defaults.get( "str"      ) );