Skip to content

Commit 80bb08f

Browse files
committed
Replace the SearchContext with QueryShardContext when building collapsing context (#46543)
This commit replaces the `SearchContext` with the `QueryShardContext` when building collapsing conteext Collapse context is part of the `SearchContext` so it shouldn't require a `SearchContext` to create one. Relates #46523
1 parent 425b1a7 commit 80bb08f

File tree

3 files changed

+29
-43
lines changed

3 files changed

+29
-43
lines changed

server/src/main/java/org/elasticsearch/search/SearchService.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -901,7 +901,16 @@ private void parseSource(DefaultSearchContext context, SearchSourceBuilder sourc
901901
}
902902

903903
if (source.collapse() != null) {
904-
final CollapseContext collapseContext = source.collapse().build(context);
904+
if (context.scrollContext() != null) {
905+
throw new SearchContextException(context, "cannot use `collapse` in a scroll context");
906+
}
907+
if (context.searchAfter() != null) {
908+
throw new SearchContextException(context, "cannot use `collapse` in conjunction with `search_after`");
909+
}
910+
if (context.rescore() != null && context.rescore().isEmpty() == false) {
911+
throw new SearchContextException(context, "cannot use `collapse` in conjunction with `rescore`");
912+
}
913+
final CollapseContext collapseContext = source.collapse().build(queryShardContext);
905914
context.collapse(collapseContext);
906915
}
907916
}

server/src/main/java/org/elasticsearch/search/collapse/CollapseBuilder.java

+7-18
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@
3434
import org.elasticsearch.index.mapper.MappedFieldType;
3535
import org.elasticsearch.index.mapper.NumberFieldMapper;
3636
import org.elasticsearch.index.query.InnerHitBuilder;
37-
import org.elasticsearch.search.SearchContextException;
38-
import org.elasticsearch.search.internal.SearchContext;
37+
import org.elasticsearch.index.query.QueryShardContext;
3938

4039
import java.io.IOException;
4140
import java.util.ArrayList;
@@ -200,32 +199,22 @@ public int hashCode() {
200199
return result;
201200
}
202201

203-
public CollapseContext build(SearchContext context) {
204-
if (context.scrollContext() != null) {
205-
throw new SearchContextException(context, "cannot use `collapse` in a scroll context");
206-
}
207-
if (context.searchAfter() != null) {
208-
throw new SearchContextException(context, "cannot use `collapse` in conjunction with `search_after`");
209-
}
210-
if (context.rescore() != null && context.rescore().isEmpty() == false) {
211-
throw new SearchContextException(context, "cannot use `collapse` in conjunction with `rescore`");
212-
}
213-
214-
MappedFieldType fieldType = context.getQueryShardContext().fieldMapper(field);
202+
public CollapseContext build(QueryShardContext queryShardContext) {
203+
MappedFieldType fieldType = queryShardContext.fieldMapper(field);
215204
if (fieldType == null) {
216-
throw new SearchContextException(context, "no mapping found for `" + field + "` in order to collapse on");
205+
throw new IllegalArgumentException("no mapping found for `" + field + "` in order to collapse on");
217206
}
218207
if (fieldType instanceof KeywordFieldMapper.KeywordFieldType == false &&
219208
fieldType instanceof NumberFieldMapper.NumberFieldType == false) {
220-
throw new SearchContextException(context, "unknown type for collapse field `" + field +
209+
throw new IllegalArgumentException("unknown type for collapse field `" + field +
221210
"`, only keywords and numbers are accepted");
222211
}
223212

224213
if (fieldType.hasDocValues() == false) {
225-
throw new SearchContextException(context, "cannot collapse on field `" + field + "` without `doc_values`");
214+
throw new IllegalArgumentException("cannot collapse on field `" + field + "` without `doc_values`");
226215
}
227216
if (fieldType.indexOptions() == IndexOptions.NONE && (innerHits != null && !innerHits.isEmpty())) {
228-
throw new SearchContextException(context, "cannot expand `inner_hits` for collapse field `"
217+
throw new IllegalArgumentException("cannot expand `inner_hits` for collapse field `"
229218
+ field + "`, " + "only indexed field can retrieve `inner_hits`");
230219
}
231220

server/src/test/java/org/elasticsearch/search/collapse/CollapseBuilderTests.java

+12-24
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@
3737
import org.elasticsearch.index.query.InnerHitBuilder;
3838
import org.elasticsearch.index.query.InnerHitBuilderTests;
3939
import org.elasticsearch.index.query.QueryShardContext;
40-
import org.elasticsearch.search.SearchContextException;
4140
import org.elasticsearch.search.SearchModule;
42-
import org.elasticsearch.search.internal.SearchContext;
4341
import org.elasticsearch.test.AbstractSerializingTestCase;
4442
import org.junit.AfterClass;
4543
import org.junit.BeforeClass;
@@ -138,59 +136,49 @@ protected NamedXContentRegistry xContentRegistry() {
138136
return xContentRegistry;
139137
}
140138

141-
private SearchContext mockSearchContext() {
142-
SearchContext context = mock(SearchContext.class);
143-
QueryShardContext shardContext = mock(QueryShardContext.class);
144-
when(context.getQueryShardContext()).thenReturn(shardContext);
145-
when(context.scrollContext()).thenReturn(null);
146-
when(context.rescore()).thenReturn(null);
147-
when(context.searchAfter()).thenReturn(null);
148-
return context;
149-
}
150-
151139
public void testBuild() throws IOException {
152140
Directory dir = new RAMDirectory();
153141
try (IndexWriter writer = new IndexWriter(dir, newIndexWriterConfig(new MockAnalyzer(random())))) {
154142
writer.commit();
155143
}
156-
SearchContext searchContext = mockSearchContext();
144+
QueryShardContext shardContext = mock(QueryShardContext.class);
157145
try (IndexReader reader = DirectoryReader.open(dir)) {
158-
when(searchContext.getQueryShardContext().getIndexReader()).thenReturn(reader);
146+
when(shardContext.getIndexReader()).thenReturn(reader);
159147
MappedFieldType numberFieldType =
160148
new NumberFieldMapper.NumberFieldType(NumberFieldMapper.NumberType.LONG);
161149
MappedFieldType keywordFieldType =
162150
new KeywordFieldMapper.KeywordFieldType();
163151
for (MappedFieldType fieldType : new MappedFieldType[] {numberFieldType, keywordFieldType}) {
164152
fieldType.setName("field");
165153
fieldType.setHasDocValues(true);
166-
when(searchContext.getQueryShardContext().fieldMapper("field")).thenReturn(fieldType);
154+
when(shardContext.fieldMapper("field")).thenReturn(fieldType);
167155
CollapseBuilder builder = new CollapseBuilder("field");
168-
CollapseContext collapseContext = builder.build(searchContext);
156+
CollapseContext collapseContext = builder.build(shardContext);
169157
assertEquals(collapseContext.getFieldType(), fieldType);
170158

171159
fieldType.setIndexOptions(IndexOptions.NONE);
172-
collapseContext = builder.build(searchContext);
160+
collapseContext = builder.build(shardContext);
173161
assertEquals(collapseContext.getFieldType(), fieldType);
174162

175163
fieldType.setHasDocValues(false);
176-
SearchContextException exc = expectThrows(SearchContextException.class, () -> builder.build(searchContext));
164+
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> builder.build(shardContext));
177165
assertEquals(exc.getMessage(), "cannot collapse on field `field` without `doc_values`");
178166

179167
fieldType.setHasDocValues(true);
180168
builder.setInnerHits(new InnerHitBuilder());
181-
exc = expectThrows(SearchContextException.class, () -> builder.build(searchContext));
169+
exc = expectThrows(IllegalArgumentException.class, () -> builder.build(shardContext));
182170
assertEquals(exc.getMessage(),
183171
"cannot expand `inner_hits` for collapse field `field`, " +
184172
"only indexed field can retrieve `inner_hits`");
185173
}
186174
}
187175
}
188176

189-
public void testBuildWithSearchContextExceptions() {
190-
SearchContext context = mockSearchContext();
177+
public void testBuildWithExceptions() {
178+
QueryShardContext shardContext = mock(QueryShardContext.class);
191179
{
192180
CollapseBuilder builder = new CollapseBuilder("unknown_field");
193-
SearchContextException exc = expectThrows(SearchContextException.class, () -> builder.build(context));
181+
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> builder.build(shardContext));
194182
assertEquals(exc.getMessage(), "no mapping found for `unknown_field` in order to collapse on");
195183
}
196184

@@ -217,9 +205,9 @@ public Query existsQuery(QueryShardContext context) {
217205
};
218206
fieldType.setName("field");
219207
fieldType.setHasDocValues(true);
220-
when(context.getQueryShardContext().fieldMapper("field")).thenReturn(fieldType);
208+
when(shardContext.fieldMapper("field")).thenReturn(fieldType);
221209
CollapseBuilder builder = new CollapseBuilder("field");
222-
SearchContextException exc = expectThrows(SearchContextException.class, () -> builder.build(context));
210+
IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> builder.build(shardContext));
223211
assertEquals(exc.getMessage(), "unknown type for collapse field `field`, only keywords and numbers are accepted");
224212
}
225213
}

0 commit comments

Comments
 (0)