SOLR-59, copy request parameters to Solr's response

git-svn-id: https://svn.apache.org/repos/asf/incubator/solr/trunk@487199 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -154,6 +154,9 @@ Changes in runtime behavior
     not all stored fields are needed from a document (klaas, SOLR-52)   
 10. Made admin JSPs return XML and transform them with new XSL stylesheets
     (Otis Gospodnetic, SOLR-58)
+11. Request parameters are copied to a new <lst name="responseHeader"> element, which 
+    replaces the old <responseHeader>. Adding a version=2.1 parameter to the request produces 
+    the old format, for backwards compatibility (bdelacretaz and yonik, SOLR-59).
 
 Optimizations 
  1. getDocListAndSet can now generate both a DocList and a DocSet from a 

src/java/org/apache/solr/core/SolrCore.java

@@ -27,6 +27,7 @@ import org.apache.solr.schema.SchemaField;
 import org.apache.solr.search.SolrIndexSearcher;
 import org.apache.solr.update.*;
 import org.apache.solr.util.DOMUtil;
+import org.apache.solr.util.NamedList;
 import org.apache.solr.util.RefCounted;
 import org.apache.solr.util.StrUtils;
 import org.apache.solr.util.XML;
@@ -587,14 +588,34 @@ public final class SolrCore {
       log.warning("Unknown Request Handler '" + req.getQueryType() +"' :" + req);
       throw new SolrException(400,"Unknown Request Handler '" + req.getQueryType() + "'", true);
     }
+    
+    // setup response header and handle request
+    final NamedList responseHeader = new NamedList();
+    rsp.add("responseHeader", responseHeader);
     handler.handleRequest(req,rsp);
+    setResponseHeaderValues(responseHeader,req,rsp);
+
     log.info(req.getParamString()+ " 0 "+
 	     (int)(rsp.getEndTime() - req.getStartTime()));
   }
   
+  protected void setResponseHeaderValues(NamedList responseHeader,SolrQueryRequest req, SolrQueryResponse rsp) {
+    // TODO should check that responseHeader has not been replaced by handler
     
+    final int qtime=(int)(rsp.getEndTime() - req.getStartTime());
+    responseHeader.add("status",rsp.getException()==null ? 0 : 500);
+    responseHeader.add("QTime",qtime);
     
-
+    // Values for echoParams... false/true/all or false/explicit/all ???
+    final String EP_PARAM = "echoParams";
+    final String EXPLICIT = "explicit";
+    final String epValue = req.getParams().get(EP_PARAM); 
+    if (EXPLICIT.equals(epValue)) {
+        responseHeader.add("params", req.getOriginalParams().toNamedList());
+    } else if(epValue!=null) {
+      throw new SolrException(400,"Invalid value '" + epValue + "' for " + EP_PARAM + " parameter, use '" + EXPLICIT + "'");
+    }
+  }
 
   XmlPullParserFactory factory;
   {

src/java/org/apache/solr/request/DefaultSolrParams.java

@@ -17,6 +17,10 @@
 
 package org.apache.solr.request;
 
+import java.util.Iterator;
+
+import org.apache.solr.util.IteratorChain;
+
 /**
  * @author yonik
  * @version $Id$
@@ -40,6 +44,13 @@ public class DefaultSolrParams extends SolrParams {
     return vals!=null ? vals : defaults.getParams(param);
   }
   
+  public Iterator<String> getParameterNamesIterator() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    c.addIterator(defaults.getParameterNamesIterator());
+    c.addIterator(params.getParameterNamesIterator());
+    return c;
+  }
+
   public String toString() {
     return "{params("+params+"),defaults("+defaults+")}";
   }

src/java/org/apache/solr/request/DisMaxRequestHandler.java

@@ -346,7 +346,7 @@ public class DisMaxRequestHandler
                                        req.getStart(), req.getLimit(),
                                        flags);
       }
-      rsp.add("search-results",results.docList);
+      rsp.add("response",results.docList);
       // pre-fetch returned documents
       U.optimizePreFetchDocs(results.docList, query, req, rsp);
 

src/java/org/apache/solr/request/JSONResponseWriter.java

@@ -75,16 +75,9 @@ class JSONWriter extends TextResponseWriter {
   }
 
   public void writeResponse() throws IOException {
-    int qtime=(int)(rsp.getEndTime() - req.getStartTime());
     NamedList nl = new NamedList();
-    HashMap header = new HashMap(1);
-    header.put("qtime",qtime);
-    nl.add("header", header);
     nl.addAll(rsp.getValues());
     // give the main response a name it it doesn't have one
-    if (nl.size()>1 && nl.getVal(1) instanceof DocList && nl.getName(1)==null) {
-      nl.setName(1,"response");
-    }
     if(wrapperFunction!=null) {
         writer.write(wrapperFunction + "(");
     }

src/java/org/apache/solr/request/MapSolrParams.java

@@ -19,6 +19,7 @@ package org.apache.solr.request;
 
 import org.apache.solr.util.StrUtils;
 
+import java.util.Iterator;
 import java.util.Map;
 import java.io.IOException;
 
@@ -42,6 +43,10 @@ public class MapSolrParams extends SolrParams {
     return val==null ? null : new String[]{val};
   }
   
+  public Iterator<String> getParameterNamesIterator() {
+    return map.keySet().iterator();
+  }
+
   public Map<String,String> getMap() { return map; }
 
   public String toString() {

src/java/org/apache/solr/request/MultiMapSolrParams.java

@@ -19,6 +19,7 @@ package org.apache.solr.request;
 
 import org.apache.solr.util.StrUtils;
 
+import java.util.Iterator;
 import java.util.Map;
 import java.io.IOException;
 
@@ -55,6 +56,10 @@ public class MultiMapSolrParams extends SolrParams {
     return map.get(name);
   }
   
+  public Iterator<String> getParameterNamesIterator() {
+    return map.keySet().iterator();
+  }
+
   public Map<String,String[]> getMap() { return map; }
 
   public String toString() {

src/java/org/apache/solr/request/SolrParams.java

@@ -21,7 +21,10 @@ import org.apache.solr.util.NamedList;
 import org.apache.solr.util.StrUtils;
 
 import javax.servlet.ServletRequest;
+
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
 import java.io.IOException;
@@ -105,6 +108,8 @@ public abstract class SolrParams {
   /** returns an array of the String values of a param, or null if none */
   public abstract String[] getParams(String param);
 
+  /** returns an Iterator over the parameter names */
+  public abstract Iterator<String> getParameterNamesIterator();
 
   /** returns the value of the param, or def if not set */
   public String get(String param, String def) {
@@ -218,6 +223,23 @@ public abstract class SolrParams {
     }
     return new MapSolrParams(map);
   }
+  
+  /** Convert this to a NamedList */
+  public NamedList toNamedList() {
+    final NamedList result = new NamedList();
+    
+    for(Iterator<String> it=getParameterNamesIterator(); it.hasNext(); ) {
+      final String name = it.next();
+      final String [] values = getParams(name);
+      if(values.length==1) {
+        result.add(name,values[0]);
+      } else {
+        // currently no reason not to use the same array
+        result.add(name,values);
+      }
+    }
+    return result;
+  }
 }
 
 

src/java/org/apache/solr/request/StandardRequestHandler.java

@@ -139,7 +139,7 @@ public class StandardRequestHandler implements SolrRequestHandler, SolrInfoMBean
       // pre-fetch returned documents
       U.optimizePreFetchDocs(results.docList, query, req, rsp);
       
-      rsp.add(null,results.docList);
+      rsp.add("response",results.docList);
 
       if (null != facetInfo) rsp.add("facet_counts", facetInfo);
 

src/java/org/apache/solr/request/TextResponseWriter.java

@@ -36,6 +36,7 @@ import java.util.Set;
  * @version $Id$
  */
 public abstract class TextResponseWriter {
+  
   protected final Writer writer;
   protected final IndexSchema schema;
   protected final SolrIndexSearcher searcher;

src/java/org/apache/solr/request/XMLWriter.java

@@ -37,6 +37,9 @@ import org.apache.lucene.document.Document;
  * @version $Id$
  */
 final public class XMLWriter {
+
+  public static float CURRENT_VERSION=2.2f;
+
   //
   // static thread safe part
   //
@@ -56,9 +59,6 @@ final public class XMLWriter {
 
   public static void writeResponse(Writer writer, SolrQueryRequest req, SolrQueryResponse rsp) throws IOException {
 
-    // get total time up until now
-    int qtime=(int)(rsp.getEndTime() - req.getStartTime());
-
     String ver = req.getParam("version");
 
     writer.write(XML_START1);
@@ -77,13 +77,6 @@ final public class XMLWriter {
     else
       writer.write(XML_START2_NOSCHEMA);
 
-    writer.write("<responseHeader><status>");
-    writer.write('0');  // it's 0 (success) if we got this far...
-    writer.write("</status><QTime>");
-    writer.write(Integer.toString((int)qtime));
-    writer.write("</QTime></responseHeader>\n");
-
-    //
     // create an instance for each request to handle
     // non-thread safe stuff (indentation levels, etc)
     // and to encapsulate writer, schema, and searcher so
@@ -101,9 +94,34 @@ final public class XMLWriter {
       }
     }
 
+    // dump response values
     NamedList lst = rsp.getValues();
     int sz = lst.size();
-    for (int i=0; i<sz; i++) {
+    int start=0;
+
+    // special case the response header if the version is 2.1 or less    
+    if (xw.version<=2100 && sz>0) {
+      Object header = lst.getVal(0);
+      if (header instanceof NamedList && "responseHeader".equals(lst.getName(0))) {
+        writer.write("<responseHeader>");
+        xw.incLevel();
+        NamedList nl = (NamedList)header;
+        for (int i=0; i<nl.size(); i++) {
+          String name = nl.getName(i);
+          Object val = nl.getVal(i);
+          if ("status".equals(name) || "QTime".equals(name)) {
+            xw.writePrim(name,null,val.toString(),false);
+          } else {
+            xw.writeVal(name,val);
+          }
+        }
+        xw.decLevel();
+        writer.write("</responseHeader>");
+        start=1;
+      }
+    }
+
+    for (int i=start; i<sz; i++) {
       xw.writeVal(lst.getName(i),lst.getVal(i));
     }
 
@@ -132,7 +150,7 @@ final public class XMLWriter {
   // maybe constructed types should always indent first?
   private final int indentThreshold=0;
 
-  private final int version;
+  final int version;
 
 
   // temporary working objects...
@@ -145,7 +163,7 @@ final public class XMLWriter {
     this.writer = writer;
     this.schema = schema;
     this.searcher = searcher;
-    float ver = version==null? 2.1f : Float.parseFloat(version);
+    float ver = version==null? CURRENT_VERSION : Float.parseFloat(version);
     this.version = (int)(ver*1000);
   }
 

src/java/org/apache/solr/util/AbstractSolrTestCase.java

@@ -99,7 +99,7 @@ public abstract class AbstractSolrTestCase extends TestCase {
                         getSolrConfigFile(),
                         getSchemaFile());
     lrf = h.getRequestFactory
-      ("standard",0,20,"version","2.0");
+      ("standard",0,20,"version","2.2");
 
   }
     

src/java/org/apache/solr/util/IteratorChain.java

@@ -0,0 +1,80 @@
+/**
+ * Copyright 2006 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+/** Chain several Iterators, so that this iterates
+ *  over all of them in sequence.
+ */
+
+public class IteratorChain<E> implements Iterator<E> {
+
+  private final List<Iterator<E>> iterators = new ArrayList<Iterator<E>>();
+  private Iterator<Iterator<E>> itit;
+  private Iterator<E> current;
+ 
+  public void addIterator(Iterator<E> it) {
+    if(itit!=null) throw new RuntimeException("all Iterators must be added before calling hasNext()");
+    iterators.add(it);
+  }
+  
+  public boolean hasNext() {
+    if(itit==null) itit = iterators.iterator();
+    return recursiveHasNext();
+  }
+  
+  /** test if current iterator hasNext(), and if not try the next
+   *  one in sequence, recursively
+   */
+  private boolean recursiveHasNext() {
+    // return false if we have no more iterators
+    if(current==null) {
+      if(itit.hasNext()) {
+        current=itit.next();
+      } else {
+        return false;
+      }
+    }
+    
+    boolean result = current.hasNext();
+    if(!result) {
+      current = null;
+      result = recursiveHasNext();
+    }
+    
+    return result;
+  }
+
+  /** hasNext() must ALWAYS be called before calling this
+   *  otherwise it's a bit hard to keep track of what's happening
+   */
+  public E next() {
+    if(current==null) { 
+      throw new RuntimeException("For an IteratorChain, hasNext() MUST be called before calling next()");
+    }
+    return current.next();
+  }
+
+  public void remove() {
+    // we just need this class 
+    // to iterate in readonly mode
+    throw new UnsupportedOperationException();
+  }
+  
+}

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

@@ -58,7 +58,7 @@ public class BasicFunctionalityTest extends AbstractSolrTestCase {
   }
 
   public void testSomeStuff() throws Exception {
-
+    lrf.args.put("version","2.0");
     assertQ("test query on empty index",
             req("qlkciyopsbgzyvkylsjhchghjrdf")
             ,"//result[@numFound='0']"
@@ -582,7 +582,7 @@ public class BasicFunctionalityTest extends AbstractSolrTestCase {
     SolrQueryResponse rsp = new SolrQueryResponse();
     core.execute(req, rsp);
 
-    DocList dl = (DocList) rsp.getValues().get(null);
+    DocList dl = (DocList) rsp.getValues().get("response");
     org.apache.lucene.document.Document d = req.getSearcher().doc(dl.iterator().nextDoc());
     // ensure field is not lazy
     assertTrue( d.getFieldable("test_hlt") instanceof Field );
@@ -602,7 +602,7 @@ public class BasicFunctionalityTest extends AbstractSolrTestCase {
     SolrQueryResponse rsp = new SolrQueryResponse();
     core.execute(req, rsp);
 
-    DocList dl = (DocList) rsp.getValues().get(null);
+    DocList dl = (DocList) rsp.getValues().get("response");
     DocIterator di = dl.iterator();    
     org.apache.lucene.document.Document d = req.getSearcher().doc(di.nextDoc());
     // ensure field is lazy

src/test/org/apache/solr/ConvertedLegacyTest.java

@@ -38,6 +38,7 @@ public class ConvertedLegacyTest extends AbstractSolrTestCase {
     // these may be reused by things that need a special query
     SolrQueryRequest req = null;
     Map<String,String> args = new HashMap<String,String>();
+    lrf.args.put("version","2.0");
 
     // compact the index, keep things from getting out of hand
 

src/test/org/apache/solr/EchoParamsTest.java

@@ -0,0 +1,53 @@
+/**
+ * Copyright 2006 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr;
+
+import org.apache.solr.util.AbstractSolrTestCase;
+
+/** Test SOLR-59, echo of query parameters */
+
+public class EchoParamsTest extends AbstractSolrTestCase {
+
+  public String getSchemaFile() { return "solr/crazy-path-to-schema.xml"; }
+  public String getSolrConfigFile() { return "solr/crazy-path-to-config.xml"; }
+
+  private static final String HEADER_XPATH = "/response/lst[@name='responseHeader']";
+
+  public void testDefaultEchoParams() {
+    lrf.args.put("wt", "xml");
+    lrf.args.put("version", "2.2");    
+    assertQ(req("foo"),HEADER_XPATH + "/int[@name='status']");
+    assertQ(req("foo"),"not(//lst[@name='params'])");
+  }
+
+  public void testDefaultEchoParamsDefaultVersion() {
+    lrf.args.put("wt", "xml");
+    lrf.args.remove("version");    
+    assertQ(req("foo"),HEADER_XPATH + "/int[@name='status']");
+    assertQ(req("foo"),"not(//lst[@name='params'])");
+  }
+
+  public void testExplicitEchoParams() {
+    lrf.args.put("wt", "xml");
+    lrf.args.put("version", "2.2");
+    lrf.args.put("echoParams", "explicit");
+    assertQ(req("foo"),HEADER_XPATH + "/int[@name='status']");
+    assertQ(req("foo"),HEADER_XPATH + "/lst[@name='params']");
+    assertQ(req("foo"),HEADER_XPATH + "/lst[@name='params']/str[@name='wt'][.='xml']");
+  }
+
+}

src/test/org/apache/solr/OutputWriterTest.java

@@ -30,6 +30,7 @@ import org.apache.solr.util.TestHarness;
  * at query time.
  *
  * @author <a href='mailto:mbaranczak@epublishing.com'> Mike Baranczak </a>
+ * @author the Solr project
  */
 public class OutputWriterTest extends AbstractSolrTestCase {
     
@@ -41,12 +42,28 @@ public class OutputWriterTest extends AbstractSolrTestCase {
     public String getSolrConfigFile() { return "solr/crazy-path-to-config.xml"; }
     
     
-    public void testOriginalSolrWriter() {
+    /** responseHeader has changed in SOLR-59, check old and new variants */
+    public void testSOLR59responseHeaderVersions() {
+        // default version is 2.2, with "new" responseHeader
+        lrf.args.remove("version");
         lrf.args.put("wt", "standard");
-        assertQ(req("foo"), "//response/responseHeader/status");
-        
+        assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='status'][.='0']");
         lrf.args.remove("wt");
-        assertQ(req("foo"), "//response/responseHeader/status");
+        assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='QTime']");
+        
+        // version=2.1 reverts to old responseHeader
+        lrf.args.put("version", "2.1");
+        lrf.args.put("wt", "standard");
+        assertQ(req("foo"), "/response/responseHeader/status[.='0']");
+        lrf.args.remove("wt");
+        assertQ(req("foo"), "/response/responseHeader/QTime");
+
+        // and explicit 2.2 works as default  
+        lrf.args.put("version", "2.2");
+        lrf.args.put("wt", "standard");
+        assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='status'][.='0']");
+        lrf.args.remove("wt");
+        assertQ(req("foo"), "/response/lst[@name='responseHeader']/int[@name='QTime']");
     }
     
     public void testUselessWriter() throws Exception {

src/test/org/apache/solr/SampleTest.java

@@ -53,7 +53,7 @@ public class SampleTest extends AbstractSolrTestCase {
    * Demonstration of some of the simple ways to use the base class
    */
   public void testSimple() {
-
+    lrf.args.put("version","2.0");
     assertU("Simple assertion that adding a document works",
             adoc("id",  "4055",
                  "subject", "Hoss the Hoss man Hostetter"));
@@ -76,7 +76,7 @@ public class SampleTest extends AbstractSolrTestCase {
    * Demonstration of some of the more complex ways to use the base class
    */
   public void testAdvanced() throws Exception {
-        
+    lrf.args.put("version","2.0");        
     assertU("less common case, a complex addition with options",
             add(doc("id", "4059",
                     "subject", "Who Me?"),

src/test/org/apache/solr/util/IteratorChainTest.java

@@ -0,0 +1,102 @@
+/**
+ * Copyright 2006 The Apache Software Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
+import junit.framework.TestCase;
+
+public class IteratorChainTest extends TestCase {
+  
+  private Iterator<String> makeIterator(String marker,int howMany) {
+    final List<String> c = new ArrayList<String>();
+    for(int i = 1; i <= howMany; i++) {
+      c.add(marker + i);
+    }
+    return c.iterator();
+  }
+  
+  public void testNoIterator() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    assertFalse("Empty IteratorChain.hastNext() is false",c.hasNext());
+    assertEquals("",getString(c));
+  }
+  
+  public void testCallNextTooEarly() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    c.addIterator(makeIterator("a",3));
+    try {
+      c.next();
+      fail("Calling next() before hasNext() should throw RuntimeException");
+    } catch(RuntimeException asExpected) {
+      // we're fine
+    }
+  }
+  
+  public void testCallAddTooLate() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    c.hasNext();
+    try {
+      c.addIterator(makeIterator("a",3));
+      fail("Calling addIterator after hasNext() should throw RuntimeException");
+    } catch(RuntimeException asExpected) {
+      // we're fine
+    }
+  }
+  
+  public void testRemove() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    try {
+      c.remove();
+      fail("Calling remove should throw UnsupportedOperationException");
+    } catch(UnsupportedOperationException asExpected) {
+      // we're fine
+    }
+  }
+  
+  public void testOneIterator() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    c.addIterator(makeIterator("a",3));
+    assertEquals("a1a2a3",getString(c));
+  }
+  
+  public void testTwoIterators() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    c.addIterator(makeIterator("a",3));
+    c.addIterator(makeIterator("b",2));
+    assertEquals("a1a2a3b1b2",getString(c));
+  }
+  
+  public void testEmptyIteratorsInTheMiddle() {
+    final IteratorChain<String> c = new IteratorChain<String>();
+    c.addIterator(makeIterator("a",3));
+    c.addIterator(makeIterator("b",0));
+    c.addIterator(makeIterator("c",1));
+    assertEquals("a1a2a3c1",getString(c));
+  }
+  
+  /** dump the contents of it to a String */
+  private String getString(Iterator<String> it) {
+    final StringBuffer sb = new StringBuffer();
+    sb.append("");
+    while(it.hasNext()) {
+      sb.append(it.next());
+    }
+    return sb.toString();
+  }
+}

src/webapp/resources/admin/form.jsp

@@ -25,7 +25,7 @@
      output type specific.
   -->
 <input name="indent" type="hidden" value="on">
-<input name="version" type="hidden" value="2.1">
+<input name="version" type="hidden" value="2.2">
 
 <table>
 <tr>

src/webapp/resources/admin/index.jsp

@@ -91,7 +91,7 @@
   <td colspan=2>
 	<form name=queryForm method="GET" action="../select/">
         <textarea class="std" rows="4" cols="40" name="q"><%= defaultSearch %></textarea>
-        <input name="version" type="hidden" value="2.1">
+        <input name="version" type="hidden" value="2.2">
 	<input name="start" type="hidden" value="0">
 	<input name="rows" type="hidden" value="10">
 	<input name="indent" type="hidden" value="on">