some basic cleanups

core/src/main/java/org/elasticsearch/search/suggest/Suggest.java

@@ -126,7 +126,7 @@ public class Suggest implements Iterable<Suggest.Suggestion<? extends Entry<? ex
                 suggestion = new PhraseSuggestion();
                 break;
             default:
-                suggestion = new Suggestion<Entry<? extends Option>>();
+                suggestion = new Suggestion();
                 break;
             }
             suggestion.readFrom(in);

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);

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<String> payloadFields = new HashSet<>(1);
+        final Set<String> 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");
         }
     }
 }

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<CompletionSuggestionContext>
         TopSuggestDocsCollector collector = new TopDocumentsCollector(suggestionContext.getSize());
         suggest(searcher, suggestionContext.toQuery(), collector);
         int numResult = 0;
+        List<LeafReaderContext> leaves = searcher.getIndexReader().leaves();
         for (TopSuggestDocs.SuggestScoreDoc suggestScoreDoc : collector.get().scoreLookupDocs()) {
             TopDocumentsCollector.SuggestDoc suggestDoc = (TopDocumentsCollector.SuggestDoc) suggestScoreDoc;
             // collect contexts
             Map<String, Set<CharSequence>> 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<String, List<Object>> payload = new HashMap<>(0);
             Set<String> payloadFields = suggestionContext.getPayloadFields();
-            if (!payloadFields.isEmpty()) {
+            if (payloadFields.isEmpty() == false) {
-                int readerIndex = ReaderUtil.subIndex(suggestDoc.doc, searcher.getIndexReader().leaves());
+                final int readerIndex = ReaderUtil.subIndex(suggestDoc.doc, leaves);
-                LeafReaderContext subReaderContext = searcher.getIndexReader().leaves().get(readerIndex);
+                final LeafReaderContext subReaderContext = leaves.get(readerIndex);
-                int subDocId = suggestDoc.doc - subReaderContext.docBase;
+                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);
+                        final AtomicFieldData data = suggestionContext.getIndexFieldDataService().getForField(payloadFieldType).load(subReaderContext);
-                        ScriptDocValues scriptValues = data.getScriptValues();
+                        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<CompletionSuggestionContext>
     }
 
     // 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<TopSuggestDocs.SuggestScoreDoc> suggestScoreDocs;
 
@@ -168,7 +168,7 @@ public class CompletionSuggester extends Suggester<CompletionSuggestionContext>
             }
         }
 
-        private static class SuggestDocPriorityQueue extends PriorityQueue<SuggestDoc> {
+        private final static class SuggestDocPriorityQueue extends PriorityQueue<SuggestDoc> {
 
             public SuggestDocPriorityQueue(int maxSize) {
                 super(maxSize);

core/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java

@@ -46,7 +46,7 @@ import java.util.*;
  * }
  *
  */
-public class CompletionSuggestion extends Suggest.Suggestion<CompletionSuggestion.Entry> {
+public final class CompletionSuggestion extends Suggest.Suggestion<CompletionSuggestion.Entry> {
 
     public static final int TYPE = 4;
 
@@ -57,22 +57,25 @@ public class CompletionSuggestion extends Suggest.Suggestion<CompletionSuggestio
         super(name, size);
     }
 
-    private class OptionPriorityQueue extends org.apache.lucene.util.PriorityQueue<Entry.Option> {
+    private static final class OptionPriorityQueue extends org.apache.lucene.util.PriorityQueue<Entry.Option> {
 
-        public OptionPriorityQueue(int maxSize) {
+        private final Comparator<Suggest.Suggestion.Entry.Option> comparator;
+
+        OptionPriorityQueue(int maxSize, Comparator<Suggest.Suggestion.Entry.Option> 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.Suggestion<CompletionSuggestio
             // combine suggestion entries from participating shards on the coordinating node
             // the global top <code>size</code> entries are collected from the shard results
             // using a priority queue
-            OptionPriorityQueue priorityQueue = new OptionPriorityQueue(size);
+            Comparator<Suggest.Suggestion.Entry.Option> optionComparator = sortComparator();
+            OptionPriorityQueue priorityQueue = new OptionPriorityQueue(size, sortComparator());
             for (Suggest.Suggestion<Entry> 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<CompletionSuggestio
         return new Entry();
     }
 
-    public static class Entry extends Suggest.Suggestion.Entry<CompletionSuggestion.Entry.Option> {
+    public final static class Entry extends Suggest.Suggestion.Entry<CompletionSuggestion.Entry.Option> {
 
         public Entry(Text text, int offset, int length) {
             super(text, offset, length);

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<CompletionSuggestionBuilder> {
     private FuzzyOptionsBuilder fuzzyOptionsBuilder;
     private RegexOptionsBuilder regexOptionsBuilder;
-    private Map<String, List<ToXContent>> queryContexts;
+    private final Map<String, List<ToXContent>> queryContexts = new HashMap<>();
-    private String[] payloadFields;
+    private final Set<String> 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<ToXContent> 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<String, List<ToXContent>> entry : this.queryContexts.entrySet()) {
                 builder.startArray(entry.getKey());

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<String, List<ContextMapping.QueryContext>> queryContexts;
+    private Map<String, List<ContextMapping.QueryContext>> queryContexts = Collections.EMPTY_MAP;
-    private MapperService mapperService;
+    private final MapperService mapperService;
-    private IndexFieldDataService fieldData;
+    private final IndexFieldDataService indexFieldDataService;
-    private Set<String> payloadFields;
+    private Set<String> 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) {
+    IndexFieldDataService getIndexFieldDataService() {
-        this.fieldData = fieldData;
+        return indexFieldDataService;
-    }
-
-    IndexFieldDataService getFieldData() {
-        return fieldData;
     }
 
     void setPayloadFields(Set<String> fields) {