SOLR-6039: fixed debug output when no results in response

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

solr/CHANGES.txt

@@ -138,6 +138,10 @@ Bug Fixes
 * SOLR-5090: SpellCheckComponent sometimes throws NPE if 
   "spellcheck.alternativeTermCount" is set to zero (James Dyer).
 
+* SOLR-6039: fixed debug output when no results in response
+  (Tomás Fernández Löbbe, hossman)
+
+
 Other Changes
 ---------------------
 

solr/core/src/java/org/apache/solr/handler/component/DebugComponent.java

@@ -19,15 +19,22 @@ package org.apache.solr.handler.component;
 
 import static org.apache.solr.common.params.CommonParams.FQ;
 
-import org.apache.solr.common.SolrDocumentList;
-import org.apache.solr.common.params.CommonParams;
-
 import java.io.IOException;
 import java.net.URL;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrDocumentList;
+import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
@@ -148,19 +155,20 @@ public class DebugComponent extends SearchComponent
       sreq.purpose |= ShardRequest.PURPOSE_GET_DEBUG;
       if (rb.isDebugAll()) {
         sreq.params.set(CommonParams.DEBUG_QUERY, "true");
-      } else if (rb.isDebug()) {
+      } else {
         if (rb.isDebugQuery()){
           sreq.params.add(CommonParams.DEBUG, CommonParams.QUERY);
         }
-        if (rb.isDebugTimings()){
-          sreq.params.add(CommonParams.DEBUG, CommonParams.TIMING);
-        } 
         if (rb.isDebugResults()){
           sreq.params.add(CommonParams.DEBUG, CommonParams.RESULTS);
         }
       }
     } else {
       sreq.params.set(CommonParams.DEBUG_QUERY, "false");
+      sreq.params.set(CommonParams.DEBUG, "false");
+    }
+    if (rb.isDebugTimings()) {
+      sreq.params.add(CommonParams.DEBUG, CommonParams.TIMING);
     } 
     if (rb.isDebugTrack()) {
       sreq.params.add(CommonParams.DEBUG, CommonParams.TRACK);
@@ -184,22 +192,24 @@ public class DebugComponent extends SearchComponent
     }
   }
 
-  private Set<String> excludeSet = new HashSet<>(Arrays.asList("explain"));
+  private final static Set<String> EXCLUDE_SET = Collections.unmodifiableSet(new HashSet<>(Arrays.asList("explain")));
 
   @Override
   public void finishStage(ResponseBuilder rb) {
     if (rb.isDebug() && rb.stage == ResponseBuilder.STAGE_GET_FIELDS) {
       NamedList<Object> info = rb.getDebugInfo();
-      NamedList explain = new SimpleOrderedMap();
+      NamedList<Object> explain = new SimpleOrderedMap<>();
 
       Map.Entry<String, Object>[]  arr =  new NamedList.NamedListEntry[rb.resultIds.size()];
+      // Will be set to true if there is at least one response with PURPOSE_GET_DEBUG
+      boolean hasGetDebugResponses = false;
 
       for (ShardRequest sreq : rb.finished) {
-        if ((sreq.purpose & ShardRequest.PURPOSE_GET_DEBUG) == 0) continue;
         for (ShardResponse srsp : sreq.responses) {
           NamedList sdebug = (NamedList)srsp.getSolrResponse().getResponse().get("debug");
-          info = (NamedList)merge(sdebug, info, excludeSet);
+          info = (NamedList)merge(sdebug, info, EXCLUDE_SET);
-
+          if ((sreq.purpose & ShardRequest.PURPOSE_GET_DEBUG) != 0) {
+            hasGetDebugResponses = true;
             if (rb.isDebugResults()) {
               NamedList sexplain = (NamedList)sdebug.get("explain");
               for (int i = 0; i < sexplain.size(); i++) {
@@ -212,14 +222,17 @@ public class DebugComponent extends SearchComponent
             }
           }
         }
+      }
 
       if (rb.isDebugResults()) {
         explain = SolrPluginUtils.removeNulls(new SimpleOrderedMap<>(arr));
       }
 
+      if (!hasGetDebugResponses) {
         if (info == null) {
-        // No responses were received from shards. Show local query info.
           info = new SimpleOrderedMap<>();
+        }
+        // No responses were received from shards. Show local query info.
         SolrPluginUtils.doStandardQueryDebug(
                 rb.req, rb.getQueryString(),  rb.getQuery(), rb.isDebugQuery(), info);
         if (rb.isDebugQuery() && rb.getQparser() != null) {

solr/core/src/test/org/apache/solr/handler/component/DistributedDebugComponentTest.java

@@ -1,20 +1,26 @@
 package org.apache.solr.handler.component;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.io.FileUtils;
-import org.apache.lucene.util.TestUtil;
+import org.apache.commons.lang.StringUtils;
 import org.apache.solr.SolrJettyTestBase;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServer;
+import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.impl.HttpSolrServer;
 import org.apache.solr.client.solrj.request.CoreAdminRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.util.NamedList;
-import org.junit.After;
 import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
@@ -43,11 +49,6 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase {
   private static String shard2;
   private static File solrHome;
   
-  @BeforeClass
-  public static void beforeTest() throws Exception {
-    solrHome = createSolrHome();
-  }
-  
   private static File createSolrHome() throws Exception {
     File workDir = createTempDir();
     setupJettyTestHome(workDir, "collection1");
@@ -55,15 +56,10 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase {
     return workDir;
   }
 
-  @AfterClass
-  public static void afterTest() throws Exception {
   
-  }
+  @BeforeClass
-  
+  public static void createThings() throws Exception {
-  @Before
+    solrHome = createSolrHome();
-  @Override
-  public void setUp() throws Exception {
-    super.setUp();
     createJetty(solrHome.getAbsolutePath(), null, null);
     String url = jetty.getBaseUrl().toString();
     collection1 = new HttpSolrServer(url);
@@ -92,9 +88,8 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase {
     
   }
   
-  @After
+  @AfterClass
-  public void tearDown() throws Exception {
+  public static void destroyThings() throws Exception {
-    super.tearDown();
     collection1.shutdown();
     collection2.shutdown();
     collection1 = null;
@@ -151,6 +146,249 @@ public class DistributedDebugComponentTest extends SolrJettyTestBase {
     assertNull(((NamedList<Object>)track.get("GET_FIELDS")).get(shard2));
   }
   
+  @Test
+  public void testRandom() throws Exception {
+    final int NUM_ITERS = atLeast(50);
+
+    for (int i = 0; i < NUM_ITERS; i++) { 
+      SolrServer client = random().nextBoolean() ? collection1 : collection2;
+      
+      SolrQuery q = new SolrQuery();
+      q.set("distrib", "true");
+      q.setFields("id", "text");
+      
+      boolean shard1Results = random().nextBoolean();
+      boolean shard2Results = random().nextBoolean();
+      
+      String qs = "_query_with_no_results_";
+      if (shard1Results) {
+        qs += " OR batman";
+      }
+      if (shard2Results) {
+        qs += " OR superman";
+      }
+      q.setQuery(qs);
+
+      Set<String> shards = new HashSet<String>(Arrays.asList(shard1, shard2));
+      if (random().nextBoolean()) {
+        shards.remove(shard1);
+        shard1Results = false;
+      } else if (random().nextBoolean()) {
+        shards.remove(shard2);
+        shard2Results = false;
+      }
+      q.set("shards", StringUtils.join(shards, ","));
+
+
+      List<String> debug = new ArrayList<String>(10);
+
+      boolean all = false;
+      final boolean timing = random().nextBoolean();
+      final boolean query = random().nextBoolean();
+      final boolean results = random().nextBoolean();
+      final boolean track = random().nextBoolean();
+
+      if (timing) { debug.add("timing"); }
+      if (query) { debug.add("query"); }
+      if (results) { debug.add("results"); }
+      if (track) { debug.add("track"); }
+      if (debug.isEmpty()) {
+        debug.add("true");
+        all = true;
+      }
+      q.set("debug", (String[])debug.toArray(new String[debug.size()]));
+
+      QueryResponse r = client.query(q);
+      try {
+        assertDebug(r, all || track, "track");
+        assertDebug(r, all || query, "rawquerystring");
+        assertDebug(r, all || query, "querystring");
+        assertDebug(r, all || query, "parsedquery");
+        assertDebug(r, all || query, "parsedquery_toString");
+        assertDebug(r, all || query, "QParser");
+        assertDebug(r, all || results, "explain");
+        assertDebug(r, all || timing, "timing");
+      } catch (AssertionError e) {
+        throw new AssertionError(q.toString() + ": " + e.getMessage(), e);
+      }
+    }
+  }
+
+  /**
+   * Asserts that the specified debug result key does or does not exist in the 
+   * response based on the expected boolean.
+   */
+  private void assertDebug(QueryResponse response, boolean expected, String key) {
+    if (expected) {
+      assertInDebug(response, key);
+    } else {
+      assertNotInDebug(response, key);
+    }
+  }
+  /**
+   * Asserts that the specified debug result key does exist in the response and is non-null
+   */
+  private void assertInDebug(QueryResponse response, String key) {
+    assertNotNull("debug map is null", response.getDebugMap());
+    assertNotNull("debug map has null for : " + key, response.getDebugMap().get(key));
+  }
+
+  /**
+   * Asserts that the specified debug result key does NOT exist in the response
+   */
+  private void assertNotInDebug(QueryResponse response, String key) {
+    assertNotNull("debug map is null", response.getDebugMap());
+    assertFalse("debug map contains: " + key, response.getDebugMap().containsKey(key));
+  }
+
+
+  @Test
+  public void testDebugSections() throws Exception {
+    SolrQuery query = new SolrQuery();
+    query.setQuery("text:_query_with_no_results_");
+    query.set("distrib", "true");
+    query.setFields("id", "text");
+    query.set("shards", shard1 + "," + shard2);
+    verifyDebugSections(query, collection1);
+    query.setQuery("id:1 OR text:_query_with_no_results_ OR id:[0 TO 300]");
+    verifyDebugSections(query, collection1);
+    
+  }
+  
+  private void verifyDebugSections(SolrQuery query, SolrServer server) throws SolrServerException {
+    query.set("debugQuery", "true");
+    query.remove("debug");
+    QueryResponse response = server.query(query);
+    assertFalse(response.getDebugMap().isEmpty());
+    assertInDebug(response, "track");
+    assertInDebug(response, "rawquerystring");
+    assertInDebug(response, "querystring");
+    assertInDebug(response, "parsedquery");
+    assertInDebug(response, "parsedquery_toString");
+    assertInDebug(response, "QParser");
+    assertInDebug(response, "explain");
+    assertInDebug(response, "timing");
+    
+    query.set("debug", "true");
+    query.remove("debugQuery");
+    response = server.query(query);
+    assertFalse(response.getDebugMap().isEmpty());
+    assertInDebug(response, "track");
+    assertInDebug(response, "rawquerystring");
+    assertInDebug(response, "querystring");
+    assertInDebug(response, "parsedquery");
+    assertInDebug(response, "parsedquery_toString");
+    assertInDebug(response, "QParser");
+    assertInDebug(response, "explain");
+    assertInDebug(response, "timing");
+    
+    query.set("debug",  "track");
+    response = server.query(query);
+    assertFalse(response.getDebugMap().isEmpty());
+    assertInDebug(response, "track");
+    assertNotInDebug(response, "rawquerystring");
+    assertNotInDebug(response, "querystring");
+    assertNotInDebug(response, "parsedquery");
+    assertNotInDebug(response, "parsedquery_toString");
+    assertNotInDebug(response, "QParser");
+    assertNotInDebug(response, "explain");
+    assertNotInDebug(response, "timing");
+    
+    query.set("debug",  "query");
+    response = server.query(query);
+    assertFalse(response.getDebugMap().isEmpty());
+    assertNotInDebug(response, "track");
+    assertInDebug(response, "rawquerystring");
+    assertInDebug(response, "querystring");
+    assertInDebug(response, "parsedquery");
+    assertInDebug(response, "parsedquery_toString");
+    assertInDebug(response, "QParser");
+    assertNotInDebug(response, "explain");
+    assertNotInDebug(response, "timing");
+    
+    query.set("debug",  "results");
+    response = server.query(query);
+    assertFalse(response.getDebugMap().isEmpty());
+    assertNotInDebug(response, "track");
+    assertNotInDebug(response, "rawquerystring");
+    assertNotInDebug(response, "querystring");
+    assertNotInDebug(response, "parsedquery");
+    assertNotInDebug(response, "parsedquery_toString");
+    assertNotInDebug(response, "QParser");
+    assertInDebug(response, "explain");
+    assertNotInDebug(response, "timing");
+    
+    query.set("debug",  "timing");
+    response = server.query(query);
+    assertFalse(response.getDebugMap().isEmpty());
+    assertNotInDebug(response, "track");
+    assertNotInDebug(response, "rawquerystring");
+    assertNotInDebug(response, "querystring");
+    assertNotInDebug(response, "parsedquery");
+    assertNotInDebug(response, "parsedquery_toString");
+    assertNotInDebug(response, "QParser");
+    assertNotInDebug(response, "explain");
+    assertInDebug(response, "timing");
+    
+    query.set("debug",  "false");
+    response = server.query(query);
+    assertNull(response.getDebugMap());
+  }
+  
+  public void testCompareWithNonDistributedRequest() throws SolrServerException {
+    SolrQuery query = new SolrQuery();
+    query.setQuery("id:1");
+    query.setFilterQueries("id:[0 TO 10]");
+    query.set("debug",  "true");
+    query.set("distrib", "true");
+    query.setFields("id", "text");
+    query.set("shards", shard1 + "," + shard2);
+    QueryResponse distribResponse = collection1.query(query);
+    
+    // same query but not distributed
+    query.set("distrib", "false");
+    query.remove("shards");
+    QueryResponse nonDistribResponse = collection1.query(query);
+    
+    assertNotNull(distribResponse.getDebugMap().get("track"));
+    assertNull(nonDistribResponse.getDebugMap().get("track"));
+    assertEquals(distribResponse.getDebugMap().size() - 1, nonDistribResponse.getDebugMap().size());
+    
+    assertSectionEquals(distribResponse, nonDistribResponse, "explain");
+    assertSectionEquals(distribResponse, nonDistribResponse, "rawquerystring");
+    assertSectionEquals(distribResponse, nonDistribResponse, "querystring");
+    assertSectionEquals(distribResponse, nonDistribResponse, "parsedquery");
+    assertSectionEquals(distribResponse, nonDistribResponse, "parsedquery_toString");
+    assertSectionEquals(distribResponse, nonDistribResponse, "QParser");
+    assertSectionEquals(distribResponse, nonDistribResponse, "filter_qieries");
+    assertSectionEquals(distribResponse, nonDistribResponse, "parsed_filter_qieries");
+    
+    // timing should have the same sections:
+    assertSameKeys((NamedList<?>)nonDistribResponse.getDebugMap().get("timing"), (NamedList<?>)distribResponse.getDebugMap().get("timing"));
+  }
+  
+  /**
+   * Compares the same section on the two query responses
+   */
+  private void assertSectionEquals(QueryResponse distrib, QueryResponse nonDistrib, String section) {
+    assertEquals(section + " debug should be equal", distrib.getDebugMap().get(section), nonDistrib.getDebugMap().get(section));
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  private void assertSameKeys(NamedList object, NamedList object2) {
+    Iterator<Map.Entry<String,Object>> iteratorObj2 = ((NamedList)object2).iterator();
+    for (Map.Entry<String,Object> entry:(NamedList<Object>)object) {
+      assertTrue(iteratorObj2.hasNext());
+      Map.Entry<String,Object> entry2 = iteratorObj2.next();
+      assertEquals(entry.getKey(), entry2.getKey());
+      if (entry.getValue() instanceof NamedList) {
+        assertTrue(entry2.getValue() instanceof NamedList);
+        assertSameKeys((NamedList)entry.getValue(), (NamedList)entry2.getValue());
+      }
+    }
+    assertFalse(iteratorObj2.hasNext());
+  }
+
   private void assertElementsPresent(NamedList<String> namedList, String...elements) {
     for(String element:elements) {
       String value = namedList.get(element);