SOLR-7530: /terms responds per field arrays in JSON by default

solr/CHANGES.txt

@@ -51,6 +51,10 @@ Upgrade Notes
 * SOLR-13323: The unused package org.apache.solr.internal.csv.writer and associated classes/tests that were easily
   confused with but not used by org.apache.solr.response.CSVWriter (or any other code) have been removed (Gus Heck)
 
+* SOLR-7530: TermsComponent's JSON response format was changed so that "terms" property carries per field arrays by default
+  regardless of distrib, terms.list, terms.ttf parameters. This affects JSON based response format but not others 
+  (Munendra S N, Mikhail Khludnev)
+
 ==================  8.2.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

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

@@ -277,7 +277,6 @@ public class TermsComponent extends SearchComponent {
         // If no lower bound was specified, use the prefix
         lowerBytes = prefixBytes;
       } else {
-        lowerBytes = new BytesRef();
         if (raw) {
           // TODO: how to handle binary? perhaps we don't for "raw"... or if the field exists
           // perhaps we detect if the FieldType is non-character and expect hex if so?
@@ -554,7 +553,7 @@ public class TermsComponent extends SearchComponent {
 
       // loop through each field we want terms from
       for (String key : fieldmap.keySet()) {
-        NamedList<Object> fieldterms = new SimpleOrderedMap<>();
+        NamedList<Object> fieldTerms = new NamedList<>();
         TermsResponse.Term[] data = null;
         if (sort) {
           data = getCountSorted(fieldmap.get(key));
@@ -567,7 +566,7 @@ public class TermsComponent extends SearchComponent {
         int cnt = 0;
         for (TermsResponse.Term tc : data) {
           if (tc.getFrequency() >= freqmin && tc.getFrequency() <= freqmax) {
-            addTermToNamedList(fieldterms, tc.getTerm(), tc.getFrequency(), tc.getTotalTermFreq(), includeTotalTermFreq);
+            addTermToNamedList(fieldTerms, tc.getTerm(), tc.getFrequency(), tc.getTotalTermFreq(), includeTotalTermFreq);
             cnt++;
           }
 
@@ -576,7 +575,7 @@ public class TermsComponent extends SearchComponent {
           }
         }
 
-        response.add(key, fieldterms);
+        response.add(key, fieldTerms);
       }
 
       return response;
@@ -621,7 +620,7 @@ public class TermsComponent extends SearchComponent {
     for (String field : fields) {
       SchemaField sf = indexSearcher.getSchema().getField(field);
       FieldType fieldType = sf.getType();
-      NamedList<Object> termsMap = new SimpleOrderedMap<>();
+      NamedList<Object> termsMap = new NamedList<>();
 
       if (fieldType.isPointField()) {
         for (String term : splitTerms) {

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

@@ -16,6 +16,7 @@
  */
 package org.apache.solr.handler.component;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -23,7 +24,17 @@ import java.util.Random;
 import java.util.stream.Stream;
 
 import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.client.solrj.ResponseParser;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.impl.BinaryResponseParser;
+import org.apache.solr.client.solrj.impl.XMLResponseParser;
+import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.response.DelegationTokenResponse;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
 import org.junit.Test;
 
 /**
@@ -89,4 +100,51 @@ public class DistributedTermsComponentTest extends BaseDistributedSearchTestCase
     }
     return super.query(q);
   }
+
+  @Override
+  protected QueryResponse query(boolean setDistribParams, SolrParams p) throws Exception {
+    QueryResponse queryResponse = super.query(setDistribParams, p);
+
+    final ModifiableSolrParams params = new ModifiableSolrParams(p);
+    // TODO: look into why passing true causes fails
+    params.set("distrib", "false");
+
+    for (ResponseParser responseParser : getResponseParsers()) {
+      final NamedList<Object> controlRsp = queryClient(controlClient, params, responseParser);
+      params.remove("distrib");
+      if (setDistribParams) {
+        setDistributedParams(params);
+      }
+
+      // query a random server
+      int which = r.nextInt(clients.size());
+      SolrClient client = clients.get(which);
+      NamedList<Object> rsp = queryClient(client, params, responseParser);
+
+      // flags needs to be called here since only terms response is passed to compare
+      // other way is to pass whole response to compare
+      assertNull(compare(rsp.findRecursive("terms"),
+          controlRsp.findRecursive("terms"), flags(handle, "terms"), handle));
+    }
+    return queryResponse;
+  }
+
+  /**
+   * Returns a {@link NamedList} containing server
+   * response deserialization is based on the {@param responseParser}
+   */
+  private NamedList<Object> queryClient(SolrClient solrClient, final ModifiableSolrParams params,
+                                        ResponseParser responseParser) throws SolrServerException, IOException {
+    QueryRequest queryRequest = new QueryRequest(params);
+    queryRequest.setResponseParser(responseParser);
+    return solrClient.request(queryRequest);
+  }
+
+  private ResponseParser[] getResponseParsers() {
+    // can't use junit parameters as this would also require RunWith
+    return new ResponseParser[]{
+        new BinaryResponseParser(), new DelegationTokenResponse.JsonMapResponseParser(),
+        new XMLResponseParser()
+    };
+  }
 }

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

@@ -328,6 +328,36 @@ public class TermsComponentTest extends SolrTestCaseJ4 {
     );
   }
 
+  @Test
+  public void testTermsWithJSON() throws Exception {
+    ModifiableSolrParams params = params(
+        "qt", "/terms", "terms", "true", "terms.fl", "standardfilt", "terms.lower", "a",
+        "terms.sort", "index", "wt", "json"
+    );
+
+    assertJQ(req(params), "/terms/standardfilt/[0]==a", "/terms/standardfilt/[1]==1");
+
+    // enable terms.ttf
+    params.set("terms.ttf", "true");
+    assertJQ(req(params), "/terms/standardfilt/[0]==a", "/terms/standardfilt/[1]/df==1",
+        "/terms/standardfilt/[1]/ttf==1");
+
+    // test the response with terms.list and terms.ttf=false
+    params.set("terms.list", "spider,snake,shark");
+    params.remove("terms.ttf");
+    assertJQ(req(params), "/terms/standardfilt/[0]==shark", "/terms/standardfilt/[1]==2",
+        "/terms/standardfilt/[2]==snake", "/terms/standardfilt/[3]==3",
+        "/terms/standardfilt/[4]==spider", "/terms/standardfilt/[5]==1"
+    );
+    // with terms.list and terms.ttf=true
+    params.set("terms.ttf", "true");
+    assertJQ(req(params),
+        "/terms/standardfilt/[0]==shark", "/terms/standardfilt/[1]/df==2", "/terms/standardfilt/[1]/ttf==2",
+        "/terms/standardfilt/[2]==snake", "/terms/standardfilt/[3]/df==3", "/terms/standardfilt/[3]/ttf==3",
+        "/terms/standardfilt/[4]==spider", "/terms/standardfilt/[5]/df==1", "/terms/standardfilt/[5]/ttf==1"
+    );
+  }
+
   @Test
   public void testDocFreqAndTotalTermFreq() throws Exception {
     SolrQueryRequest req = req(

solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java

@@ -1349,7 +1349,7 @@ public abstract class SolrTestCaseJ4 extends SolrTestCase {
    *  For example, this method changed single quoted strings into double quoted strings before
    *  the parser could natively handle them.
    *
-   * This transformation is automatically applied to JSON test srings (like assertJQ).
+   * This transformation is automatically applied to JSON test strings (like assertJQ).
    */
   public static String json(String testJSON) {
     return testJSON;