From 4f6314c59bf0830da2c99c6a820f4a3e2acb532a Mon Sep 17 00:00:00 2001 From: Mikhail Khludnev Date: Tue, 18 Jun 2019 08:36:58 +0200 Subject: [PATCH] SOLR-7530: /terms responds per field arrays in JSON by default --- solr/CHANGES.txt | 4 ++ .../handler/component/TermsComponent.java | 15 +++-- .../DistributedTermsComponentTest.java | 60 ++++++++++++++++++- .../handler/component/TermsComponentTest.java | 30 ++++++++++ .../java/org/apache/solr/SolrTestCaseJ4.java | 2 +- 5 files changed, 101 insertions(+), 10 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 061d2b30d0c..b3440dbaa5d 100644 --- a/solr/CHANGES.txt +++ b/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. diff --git a/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java b/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java index f5b03cf890b..1ddf4ca7a55 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/TermsComponent.java +++ b/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 fieldterms = new SimpleOrderedMap<>(); + NamedList 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,8 +620,8 @@ public class TermsComponent extends SearchComponent { for (String field : fields) { SchemaField sf = indexSearcher.getSchema().getField(field); FieldType fieldType = sf.getType(); - NamedList termsMap = new SimpleOrderedMap<>(); - + NamedList termsMap = new NamedList<>(); + if (fieldType.isPointField()) { for (String term : splitTerms) { Query q = fieldType.getFieldQuery(null, sf, term); @@ -636,10 +635,10 @@ public class TermsComponent extends SearchComponent { for (int i = 0; i < splitTerms.length; i++) { terms[i] = new Term(field, fieldType.readableToIndexed(splitTerms[i])); } - + TermStates[] termStates = new TermStates[terms.length]; collectTermStates(topReaderContext, termStates, terms); - + for (int i = 0; i < terms.length; i++) { if (termStates[i] != null) { String outTerm = fieldType.indexedToReadable(terms[i].bytes().utf8ToString()); diff --git a/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java index 329cd80668b..5a932ca10bd 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/DistributedTermsComponentTest.java +++ b/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; /** @@ -36,7 +47,7 @@ public class DistributedTermsComponentTest extends BaseDistributedSearchTestCase @Test public void test() throws Exception { - + Random random = random(); del("*:*"); index(id, random.nextInt(), "b_t", "snake a,b spider shark snail slug seal", "foo_i", "1"); @@ -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 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 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 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() + }; + } } diff --git a/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java b/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java index aeb7273cc9f..b50f31ffa7a 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/TermsComponentTest.java +++ b/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( diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index 66e06ef53c1..f0d1a2c2604 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/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;