From 8911469e631651c409191113960e51462253549f Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 3 Nov 2015 12:51:29 +0100 Subject: [PATCH] some basic cleanups --- .../elasticsearch/search/suggest/Suggest.java | 2 +- .../completion/CompletionFieldStats.java | 2 ++ .../completion/CompletionSuggestParser.java | 11 ++++----- .../completion/CompletionSuggester.java | 24 +++++++++---------- .../completion/CompletionSuggestion.java | 18 ++++++++------ .../CompletionSuggestionBuilder.java | 11 ++++----- .../CompletionSuggestionContext.java | 24 ++++++++----------- 7 files changed, 44 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java b/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java index 95ce7ce70a0..db60d58953a 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/Suggest.java @@ -126,7 +126,7 @@ public class Suggest implements Iterable>(); + suggestion = new Suggestion(); break; } suggestion.readFrom(in); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionFieldStats.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionFieldStats.java index c5d39405e50..406f525e59c 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionFieldStats.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionFieldStats.java @@ -44,6 +44,7 @@ public class CompletionFieldStats { Terms terms = atomicReader.fields().terms(fieldName); if (terms instanceof CompletionTerms) { // TODO: currently we load up the suggester for reporting it's size + // nocommit - this is pretty trappy do we have any way to detect this? long fstSize = ((CompletionTerms) terms).suggester().ramBytesUsed(); if (fields != null && fields.length > 0 && Regex.simpleMatch(fields, fieldName)) { completionFields.addTo(fieldName, fstSize); @@ -52,6 +53,7 @@ public class CompletionFieldStats { } } } catch (IOException ignored) { + //nocommit - why do we ignore this exception? } } return new CompletionStats(sizeInBytes, completionFields); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestParser.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestParser.java index 9ca6c0ca8ca..6bbcd9e055d 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestParser.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestParser.java @@ -71,7 +71,7 @@ import static org.elasticsearch.search.suggest.SuggestUtils.parseSuggestContext; */ public class CompletionSuggestParser implements SuggestContextParser { - private CompletionSuggester completionSuggester; + private final CompletionSuggester completionSuggester; public CompletionSuggestParser(CompletionSuggester completionSuggester) { this.completionSuggester = completionSuggester; @@ -82,12 +82,11 @@ public class CompletionSuggestParser implements SuggestContextParser { XContentParser.Token token; ParseFieldMatcher parseFieldMatcher = mapperService.getIndexSettings().getParseFieldMatcher(); String fieldName = null; - CompletionSuggestionContext suggestion = new CompletionSuggestionContext(completionSuggester); - + final CompletionSuggestionContext suggestion = new CompletionSuggestionContext(completionSuggester, mapperService, fieldDataService); XContentParser contextParser = null; CompletionSuggestionBuilder.FuzzyOptionsBuilder fuzzyOptions = null; CompletionSuggestionBuilder.RegexOptionsBuilder regexOptions = null; - Set payloadFields = new HashSet<>(1); + final Set payloadFields = new HashSet<>(1); while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { if (token == XContentParser.Token.FIELD_NAME) { @@ -198,12 +197,10 @@ public class CompletionSuggestParser implements SuggestContextParser { suggestion.setFuzzyOptionsBuilder(fuzzyOptions); suggestion.setRegexOptionsBuilder(regexOptions); suggestion.setQueryContexts(queryContexts); - suggestion.setMapperService(mapperService); - suggestion.setFieldData(fieldDataService); suggestion.setPayloadFields(payloadFields); return suggestion; } else { - throw new ElasticsearchException("Field [" + suggestion.getField() + "] is not a completion suggest field"); + throw new IllegalArgumentException("Field [" + suggestion.getField() + "] is not a completion suggest field"); } } } diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java index ed9f0a1ddd8..8e383e9a479 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggester.java @@ -30,7 +30,6 @@ import org.apache.lucene.search.suggest.document.TopSuggestDocs; import org.apache.lucene.search.suggest.document.TopSuggestDocsCollector; import org.apache.lucene.util.*; import org.apache.lucene.util.PriorityQueue; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.text.StringText; import org.elasticsearch.index.fielddata.AtomicFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; @@ -63,29 +62,30 @@ public class CompletionSuggester extends Suggester TopSuggestDocsCollector collector = new TopDocumentsCollector(suggestionContext.getSize()); suggest(searcher, suggestionContext.toQuery(), collector); int numResult = 0; + List leaves = searcher.getIndexReader().leaves(); for (TopSuggestDocs.SuggestScoreDoc suggestScoreDoc : collector.get().scoreLookupDocs()) { TopDocumentsCollector.SuggestDoc suggestDoc = (TopDocumentsCollector.SuggestDoc) suggestScoreDoc; // collect contexts Map> contexts = Collections.emptyMap(); - if (fieldType.hasContextMappings() && !suggestDoc.getContexts().isEmpty()) { + if (fieldType.hasContextMappings() && suggestDoc.getContexts().isEmpty() == false) { contexts = fieldType.getContextMappings().getNamedContexts(suggestDoc.getContexts()); } // collect payloads final Map> payload = new HashMap<>(0); Set payloadFields = suggestionContext.getPayloadFields(); - if (!payloadFields.isEmpty()) { - int readerIndex = ReaderUtil.subIndex(suggestDoc.doc, searcher.getIndexReader().leaves()); - LeafReaderContext subReaderContext = searcher.getIndexReader().leaves().get(readerIndex); - int subDocId = suggestDoc.doc - subReaderContext.docBase; + if (payloadFields.isEmpty() == false) { + final int readerIndex = ReaderUtil.subIndex(suggestDoc.doc, leaves); + final LeafReaderContext subReaderContext = leaves.get(readerIndex); + final int subDocId = suggestDoc.doc - subReaderContext.docBase; for (String field : payloadFields) { MappedFieldType payloadFieldType = suggestionContext.getMapperService().smartNameFieldType(field); if (payloadFieldType != null) { - AtomicFieldData data = suggestionContext.getFieldData().getForField(payloadFieldType).load(subReaderContext); - ScriptDocValues scriptValues = data.getScriptValues(); + final AtomicFieldData data = suggestionContext.getIndexFieldDataService().getForField(payloadFieldType).load(subReaderContext); + final ScriptDocValues scriptValues = data.getScriptValues(); scriptValues.setNextDocId(subDocId); payload.put(field, new ArrayList<>(scriptValues.getValues())); } else { - throw new ElasticsearchException("payload field [" + field + "] does not exist"); + throw new IllegalArgumentException("payload field [" + field + "] does not exist"); } } } @@ -117,12 +117,12 @@ public class CompletionSuggester extends Suggester } // TODO: this should be refactored and moved to lucene - private static class TopDocumentsCollector extends TopSuggestDocsCollector { + private final static class TopDocumentsCollector extends TopSuggestDocsCollector { /** * Holds a list of suggest meta data for a doc */ - private static class SuggestDoc extends TopSuggestDocs.SuggestScoreDoc { + private final static class SuggestDoc extends TopSuggestDocs.SuggestScoreDoc { private List suggestScoreDocs; @@ -168,7 +168,7 @@ public class CompletionSuggester extends Suggester } } - private static class SuggestDocPriorityQueue extends PriorityQueue { + private final static class SuggestDocPriorityQueue extends PriorityQueue { public SuggestDocPriorityQueue(int maxSize) { super(maxSize); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java index a9d5a4bddb8..66c21c58162 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java @@ -46,7 +46,7 @@ import java.util.*; * } * */ -public class CompletionSuggestion extends Suggest.Suggestion { +public final class CompletionSuggestion extends Suggest.Suggestion { public static final int TYPE = 4; @@ -57,22 +57,25 @@ public class CompletionSuggestion extends Suggest.Suggestion { + private static final class OptionPriorityQueue extends org.apache.lucene.util.PriorityQueue { - public OptionPriorityQueue(int maxSize) { + private final Comparator comparator; + + OptionPriorityQueue(int maxSize, Comparator comparator) { super(maxSize); + this.comparator = comparator; } @Override protected boolean lessThan(Entry.Option a, Entry.Option b) { - int cmp = sortComparator().compare(a, b); + int cmp = comparator.compare(a, b); if (cmp != 0) { return cmp > 0; } return Lookup.CHARSEQUENCE_COMPARATOR.compare(a.getText().string(), b.getText().string()) > 0; } - public Entry.Option[] get() { + Entry.Option[] get() { int size = size(); Entry.Option[] results = new Entry.Option[size]; for (int i = size - 1; i >= 0; i--) { @@ -90,7 +93,8 @@ public class CompletionSuggestion extends Suggest.Suggestionsize entries are collected from the shard results // using a priority queue - OptionPriorityQueue priorityQueue = new OptionPriorityQueue(size); + Comparator optionComparator = sortComparator(); + OptionPriorityQueue priorityQueue = new OptionPriorityQueue(size, sortComparator()); for (Suggest.Suggestion entries : toReduce) { assert entries.getEntries().size() == 1 : "CompletionSuggestion must have only one entry"; for (Entry.Option option : entries.getEntries().get(0)) { @@ -119,7 +123,7 @@ public class CompletionSuggestion extends Suggest.Suggestion { + public final static class Entry extends Suggest.Suggestion.Entry { public Entry(Text text, int offset, int length) { super(text, offset, length); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java index b4c88e20647..fe80f70e260 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionBuilder.java @@ -43,8 +43,8 @@ import static org.elasticsearch.search.suggest.completion.context.CategoryContex public class CompletionSuggestionBuilder extends SuggestBuilder.SuggestionBuilder { private FuzzyOptionsBuilder fuzzyOptionsBuilder; private RegexOptionsBuilder regexOptionsBuilder; - private Map> queryContexts; - private String[] payloadFields; + private final Map> queryContexts = new HashMap<>(); + private final Set payloadFields = new HashSet<>(); public CompletionSuggestionBuilder(String name) { super(name, "completion"); @@ -283,7 +283,7 @@ public class CompletionSuggestionBuilder extends SuggestBuilder.SuggestionBuilde * Note: Only doc values enabled fields are supported */ public CompletionSuggestionBuilder payload(String... fields) { - this.payloadFields = fields; + Collections.addAll(this.payloadFields, fields); return this; } @@ -306,9 +306,6 @@ public class CompletionSuggestionBuilder extends SuggestBuilder.SuggestionBuilde } private CompletionSuggestionBuilder contexts(String name, ToXContent... queryContexts) { - if (this.queryContexts == null) { - this.queryContexts = new HashMap<>(2); - } List contexts = this.queryContexts.get(name); if (contexts == null) { contexts = new ArrayList<>(2); @@ -333,7 +330,7 @@ public class CompletionSuggestionBuilder extends SuggestBuilder.SuggestionBuilde if (regexOptionsBuilder != null) { regexOptionsBuilder.toXContent(builder, params); } - if (queryContexts != null) { + if (queryContexts.isEmpty() == false) { builder.startObject("contexts"); for (Map.Entry> entry : this.queryContexts.entrySet()) { builder.startArray(entry.getKey()); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionContext.java b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionContext.java index 2f4b729e1af..9f3e992d9dc 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionContext.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestionContext.java @@ -28,6 +28,7 @@ import org.elasticsearch.search.suggest.SuggestionSearchContext; import org.elasticsearch.search.suggest.completion.context.ContextMapping; import org.elasticsearch.search.suggest.completion.context.ContextMappings; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -40,13 +41,15 @@ public class CompletionSuggestionContext extends SuggestionSearchContext.Suggest private CompletionFieldMapper.CompletionFieldType fieldType; private CompletionSuggestionBuilder.FuzzyOptionsBuilder fuzzyOptionsBuilder; private CompletionSuggestionBuilder.RegexOptionsBuilder regexOptionsBuilder; - private Map> queryContexts; - private MapperService mapperService; - private IndexFieldDataService fieldData; - private Set payloadFields; + private Map> queryContexts = Collections.EMPTY_MAP; + private final MapperService mapperService; + private final IndexFieldDataService indexFieldDataService; + private Set payloadFields = Collections.EMPTY_SET; - CompletionSuggestionContext(Suggester suggester) { + CompletionSuggestionContext(Suggester suggester, MapperService mapperService, IndexFieldDataService indexFieldDataService) { super(suggester); + this.indexFieldDataService = indexFieldDataService; + this.mapperService = mapperService; } CompletionFieldMapper.CompletionFieldType getFieldType() { @@ -69,20 +72,13 @@ public class CompletionSuggestionContext extends SuggestionSearchContext.Suggest this.queryContexts = queryContexts; } - void setMapperService(MapperService mapperService) { - this.mapperService = mapperService; - } MapperService getMapperService() { return mapperService; } - void setFieldData(IndexFieldDataService fieldData) { - this.fieldData = fieldData; - } - - IndexFieldDataService getFieldData() { - return fieldData; + IndexFieldDataService getIndexFieldDataService() { + return indexFieldDataService; } void setPayloadFields(Set fields) {