honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Highlighting shouldn't fail when the field to highlight is absent. 

PlainHighlighter fails with a NPE when the field to highlight is marked as
stored in the mapping but doesn't exist in a hit. This patch makes
FieldsVisitor.fields less error-prone by returning an empty list instead
of null when no matching stored field was found.

Closes #3109
This commit is contained in:
Adrien Grand 2013-05-30 10:24:34 +02:00
parent 03a86604a4
commit 5ea6c77dad
5 changed files with 48 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java
View File

@ -19,6 +19,7 @@
package org.elasticsearch.index.fieldvisitor;
import com.google.common.collect.ImmutableMap;
import org.apache.lucene.index.FieldInfo;
import org.apache.lucene.index.StoredFieldVisitor;
import org.apache.lucene.util.BytesRef;
@ -44,9 +45,6 @@ public abstract class FieldsVisitor extends StoredFieldVisitor {
protected Map<String, List<Object>> fieldsValues;
public void postProcess(MapperService mapperService) {
if (fieldsValues == null || fieldsValues.isEmpty()) {
return;
}
if (uid != null) {
DocumentMapper documentMapper = mapperService.documentMapper(uid.type());
if (documentMapper != null) {
@ -56,7 +54,7 @@ public abstract class FieldsVisitor extends StoredFieldVisitor {
}
}
// can't derive exact mapping type
for (Map.Entry<String, List<Object>> entry : fieldsValues.entrySet()) {
for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
FieldMappers fieldMappers = mapperService.indexName(entry.getKey());
if (fieldMappers == null) {
continue;
@ -69,8 +67,8 @@ public abstract class FieldsVisitor extends StoredFieldVisitor {
}
public void postProcess(DocumentMapper documentMapper) {
for (Map.Entry<String, List<Object>> entry : fieldsValues.entrySet()) {
FieldMapper fieldMapper = documentMapper.mappers().indexName(entry.getKey()).mapper();
for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
FieldMapper<?> fieldMapper = documentMapper.mappers().indexName(entry.getKey()).mapper();
if (fieldMapper == null) {
continue;
}
@ -128,7 +126,9 @@ public abstract class FieldsVisitor extends StoredFieldVisitor {
}
public Map<String, List<Object>> fields() {
return fieldsValues;
return fieldsValues != null
? fieldsValues
: ImmutableMap.<String, List<Object>>of();
}
public void reset() {

3
src/main/java/org/elasticsearch/index/get/ShardGetService.java
View File

@ -52,7 +52,6 @@ import org.elasticsearch.search.lookup.SearchLookup;
import org.elasticsearch.search.lookup.SourceLookup;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@ -322,7 +321,7 @@ public class ShardGetService extends AbstractIndexShardComponent {
}
source = fieldVisitor.source();
if (fieldVisitor.fields() != null) {
if (!fieldVisitor.fields().isEmpty()) {
fieldVisitor.postProcess(docMapper);
fields = new HashMap<String, GetField>(fieldVisitor.fields().size());
for (Map.Entry<String, List<Object>> entry : fieldVisitor.fields().entrySet()) {

2
src/main/java/org/elasticsearch/search/fetch/FetchPhase.java
View File

@ -145,7 +145,7 @@ public class FetchPhase implements SearchPhase {
fieldsVisitor.postProcess(context.mapperService());
Map<String, SearchHitField> searchFields = null;
if (fieldsVisitor.fields() != null) {
if (!fieldsVisitor.fields().isEmpty()) {
searchFields = new HashMap<String, SearchHitField>(fieldsVisitor.fields().size());
for (Map.Entry<String, List<Object>> entry : fieldsVisitor.fields().entrySet()) {
searchFields.put(entry.getKey(), new InternalSearchHitField(entry.getKey(), entry.getValue()));

11
src/main/java/org/elasticsearch/search/highlight/PlainHighlighter.java
View File

@ -18,6 +18,8 @@
*/
package org.elasticsearch.search.highlight;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import org.apache.lucene.analysis.Analyzer;
@ -58,10 +60,10 @@ public class PlainHighlighter implements Highlighter {
Encoder encoder = field.encoder().equals("html") ? Encoders.HTML : Encoders.DEFAULT;
if (!hitContext.cache().containsKey(CACHE_KEY)) {
Map<FieldMapper, org.apache.lucene.search.highlight.Highlighter> mappers = Maps.newHashMap();
Map<FieldMapper<?>, org.apache.lucene.search.highlight.Highlighter> mappers = Maps.newHashMap();
hitContext.cache().put(CACHE_KEY, mappers);
}
Map<FieldMapper, org.apache.lucene.search.highlight.Highlighter> cache = (Map<FieldMapper, org.apache.lucene.search.highlight.Highlighter>) hitContext.cache().get(CACHE_KEY);
Map<FieldMapper<?>, org.apache.lucene.search.highlight.Highlighter> cache = (Map<FieldMapper<?>, org.apache.lucene.search.highlight.Highlighter>) hitContext.cache().get(CACHE_KEY);
org.apache.lucene.search.highlight.Highlighter entry = cache.get(mapper);
if (entry == null) {
@ -98,6 +100,10 @@ public class PlainHighlighter implements Highlighter {
CustomFieldsVisitor fieldVisitor = new CustomFieldsVisitor(ImmutableSet.of(mapper.names().indexName()), false);
hitContext.reader().document(hitContext.docId(), fieldVisitor);
textsToHighlight = fieldVisitor.fields().get(mapper.names().indexName());
if (textsToHighlight == null) {
// Can happen if the document doesn't have the field to highlight
textsToHighlight = ImmutableList.of();
}
} catch (Exception e) {
throw new FetchPhaseExecutionException(context, "Failed to highlight field [" + highlighterContext.fieldName + "]", e);
}
@ -107,6 +113,7 @@ public class PlainHighlighter implements Highlighter {
lookup.setNextDocId(hitContext.docId());
textsToHighlight = lookup.source().extractRawValues(mapper.names().sourcePath());
}
assert textsToHighlight != null;
// a HACK to make highlighter do highlighting, even though its using the single frag list builder
int numberOfFragments = field.numberOfFragments() == 0 ? 1 : field.numberOfFragments();

30
src/test/java/org/elasticsearch/test/integration/search/highlight/HighlighterSearchTests.java
View File

@ -1503,4 +1503,34 @@ public class HighlighterSearchTests extends AbstractNodesTests {
}
}
@Test
public void testMissingStoredField() throws Exception {
try {
client.admin().indices().prepareDelete("test").execute().actionGet();
} catch (Exception e) {
// ignore
}
client.admin().indices().prepareCreate("test").setSettings(ImmutableSettings.settingsBuilder()
.put("index.number_of_shards", 1).put("index.number_of_replicas", 0))
.addMapping("type1", jsonBuilder().startObject().startObject("type1").startObject("properties")
.startObject("highlight_field").field("type", "string").field("store", "yes").endObject()
.endObject().endObject().endObject())
.execute().actionGet();
client.prepareIndex("test", "type1", "1")
.setSource(jsonBuilder().startObject()
.field("field", "highlight")
.endObject())
.setRefresh(true).execute().actionGet();
// This query used to fail when the field to highlight was absent
SearchResponse response = client.prepareSearch("test")
.setQuery(QueryBuilders.matchQuery("field", "highlight").type(MatchQueryBuilder.Type.BOOLEAN))
.addHighlightedField(new HighlightBuilder.Field("highlight_field")
.fragmentSize(-1).numOfFragments(1).fragmenter("simple"))
.execute().actionGet();
assertThat(response.getHits().hits()[0].highlightFields().isEmpty(), equalTo(true));
}
}