Browse Source

Fix some warnings reported by Findbugs.

Although high-severity bugs were mostly static variables that were not final,
it also found potential NPEs and bugs like `x ^= x;` or equality checks on
objects that don't share a common interface.

Close #5571
Adrien Grand 11 years ago
parent
commit
04c16b7ba5
28 changed files with 51 additions and 68 deletions
  1. 7 6
      src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java
  2. 1 1
      src/main/java/org/elasticsearch/action/get/TransportGetAction.java
  3. 1 1
      src/main/java/org/elasticsearch/action/support/master/MasterNodeOperationRequest.java
  4. 1 1
      src/main/java/org/elasticsearch/cluster/metadata/MetaData.java
  5. 0 25
      src/main/java/org/elasticsearch/common/Names.java
  6. 1 1
      src/main/java/org/elasticsearch/common/geo/GeoDistance.java
  7. 1 1
      src/main/java/org/elasticsearch/common/lucene/Lucene.java
  8. 1 1
      src/main/java/org/elasticsearch/common/unit/DistanceUnit.java
  9. 1 1
      src/main/java/org/elasticsearch/discovery/zen/ping/ZenPing.java
  10. 1 1
      src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java
  11. 2 1
      src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java
  12. 3 3
      src/main/java/org/elasticsearch/monitor/jvm/GcNames.java
  13. 3 6
      src/main/java/org/elasticsearch/rest/action/cat/RestAllocationAction.java
  14. 1 1
      src/main/java/org/elasticsearch/rest/support/RestUtils.java
  15. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java
  16. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java
  17. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java
  18. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java
  19. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java
  20. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java
  21. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java
  22. 1 1
      src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java
  23. 1 1
      src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java
  24. 2 2
      src/main/java/org/elasticsearch/search/suggest/SuggestUtils.java
  25. 9 0
      src/main/java/org/elasticsearch/search/suggest/context/CategoryContextMapping.java
  26. 1 1
      src/main/java/org/elasticsearch/search/suggest/context/GeolocationContextMapping.java
  27. 5 5
      src/main/java/org/elasticsearch/search/suggest/term/TermSuggestion.java
  28. 1 1
      src/main/java/org/elasticsearch/transport/ConnectTransportException.java

+ 7 - 6
src/main/java/org/elasticsearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java

@@ -42,9 +42,9 @@ import java.util.Map;
 import static org.elasticsearch.action.ValidateActions.addValidationError;
 import static org.elasticsearch.common.Strings.EMPTY_ARRAY;
 import static org.elasticsearch.common.Strings.hasLength;
-import static org.elasticsearch.common.settings.ImmutableSettings.Builder.EMPTY_SETTINGS;
 import static org.elasticsearch.common.settings.ImmutableSettings.readSettingsFromStream;
 import static org.elasticsearch.common.settings.ImmutableSettings.writeSettingsToStream;
+import static org.elasticsearch.common.settings.ImmutableSettings.Builder.EMPTY_SETTINGS;
 import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeBooleanValue;
 
 /**
@@ -104,11 +104,12 @@ public class CreateSnapshotRequest extends MasterNodeOperationRequest<CreateSnap
         }
         if (indices == null) {
             validationException = addValidationError("indices is null", validationException);
-        }
-        for (String index : indices) {
-            if (index == null) {
-                validationException = addValidationError("index is null", validationException);
-                break;
+        } else {
+            for (String index : indices) {
+                if (index == null) {
+                    validationException = addValidationError("index is null", validationException);
+                    break;
+                }
             }
         }
         if (indicesOptions == null) {

+ 1 - 1
src/main/java/org/elasticsearch/action/get/TransportGetAction.java

@@ -42,7 +42,7 @@ import org.elasticsearch.transport.TransportService;
  */
 public class TransportGetAction extends TransportShardSingleOperationAction<GetRequest, GetResponse> {
 
-    public static boolean REFRESH_FORCE = false;
+    public static final boolean REFRESH_FORCE = false;
 
     private final IndicesService indicesService;
     private final boolean realtime;

+ 1 - 1
src/main/java/org/elasticsearch/action/support/master/MasterNodeOperationRequest.java

@@ -31,7 +31,7 @@ import java.io.IOException;
  */
 public abstract class MasterNodeOperationRequest<T extends MasterNodeOperationRequest> extends ActionRequest<T> {
 
-    public static TimeValue DEFAULT_MASTER_NODE_TIMEOUT = TimeValue.timeValueSeconds(30);
+    public static final TimeValue DEFAULT_MASTER_NODE_TIMEOUT = TimeValue.timeValueSeconds(30);
 
     protected TimeValue masterNodeTimeout = DEFAULT_MASTER_NODE_TIMEOUT;
 

+ 1 - 1
src/main/java/org/elasticsearch/cluster/metadata/MetaData.java

@@ -1045,7 +1045,7 @@ public class MetaData implements Iterable<IndexMetaData> {
         int customCount1 = 0;
         for (ObjectObjectCursor<String, Custom> cursor : metaData1.customs) {
             if (customFactories.get(cursor.key).isPersistent()) {
-                if (!cursor.equals(metaData2.custom(cursor.key))) return false;
+                if (!cursor.value.equals(metaData2.custom(cursor.key))) return false;
                 customCount1++;
             }
         }

+ 0 - 25
src/main/java/org/elasticsearch/common/Names.java

@@ -24,10 +24,8 @@ import jsr166y.ThreadLocalRandom;
 
 import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.net.URL;
-import java.util.Random;
 
 /**
  *
@@ -62,29 +60,6 @@ public abstract class Names {
         }
     }
 
-    public static String randomNodeName(InputStream nodeNames) {
-        if (nodeNames == null) {
-            return null;
-        }
-        try {
-            BufferedReader reader = new BufferedReader(new InputStreamReader(nodeNames, Charsets.UTF_8));
-            int numberOfNames = Integer.parseInt(reader.readLine());
-            int number = ((new Random().nextInt(numberOfNames)) % numberOfNames) - 2; // remove 2 for last line and first line
-            for (int i = 0; i < number; i++) {
-                reader.readLine();
-            }
-            return reader.readLine();
-        } catch (Exception e) {
-            return null;
-        } finally {
-            try {
-                nodeNames.close();
-            } catch (IOException e) {
-                // ignore
-            }
-        }
-    }
-
     private Names() {
 
     }

+ 1 - 1
src/main/java/org/elasticsearch/common/geo/GeoDistance.java

@@ -212,7 +212,7 @@ public enum GeoDistance {
         GeoPoint bottomRight();
     }
 
-    public static AlwaysDistanceBoundingCheck ALWAYS_INSTANCE = new AlwaysDistanceBoundingCheck();
+    public static final AlwaysDistanceBoundingCheck ALWAYS_INSTANCE = new AlwaysDistanceBoundingCheck();
 
     private static class AlwaysDistanceBoundingCheck implements DistanceBoundingCheck {
         @Override

+ 1 - 1
src/main/java/org/elasticsearch/common/lucene/Lucene.java

@@ -54,7 +54,7 @@ public class Lucene {
 
     public static final int NO_DOC = -1;
 
-    public static ScoreDoc[] EMPTY_SCORE_DOCS = new ScoreDoc[0];
+    public static final ScoreDoc[] EMPTY_SCORE_DOCS = new ScoreDoc[0];
 
     @SuppressWarnings("deprecation")
     public static Version parseVersion(@Nullable String version, Version defaultVersion, ESLogger logger) {

+ 1 - 1
src/main/java/org/elasticsearch/common/unit/DistanceUnit.java

@@ -51,7 +51,7 @@ public enum DistanceUnit {
     // parsing would fail
     METERS(1, "m", "meters");
 
-    public static DistanceUnit DEFAULT = METERS;
+    public static final DistanceUnit DEFAULT = METERS;
 
     private double meters; 
     private final String[] names;

+ 1 - 1
src/main/java/org/elasticsearch/discovery/zen/ping/ZenPing.java

@@ -50,7 +50,7 @@ public interface ZenPing extends LifecycleComponent<ZenPing> {
 
     public static class PingResponse implements Streamable {
         
-        public static PingResponse[] EMPTY = new PingResponse[0];
+        public static final PingResponse[] EMPTY = new PingResponse[0];
 
         private ClusterName clusterName;
 

+ 1 - 1
src/main/java/org/elasticsearch/index/mapper/core/CompletionFieldMapper.java

@@ -91,7 +91,7 @@ public class CompletionFieldMapper extends AbstractFieldMapper<String> {
         public static final String CONTEXT = "context";
     }
 
-    public static Set<String> ALLOWED_CONTENT_FIELD_NAMES = Sets.newHashSet(Fields.CONTENT_FIELD_NAME_INPUT,
+    public static final Set<String> ALLOWED_CONTENT_FIELD_NAMES = Sets.newHashSet(Fields.CONTENT_FIELD_NAME_INPUT,
             Fields.CONTENT_FIELD_NAME_OUTPUT, Fields.CONTENT_FIELD_NAME_PAYLOAD, Fields.CONTENT_FIELD_NAME_WEIGHT, Fields.CONTEXT);
 
     public static class Builder extends AbstractFieldMapper.Builder<Builder, CompletionFieldMapper> {

+ 2 - 1
src/main/java/org/elasticsearch/index/mapper/geo/GeoPointFieldMapper.java

@@ -21,6 +21,7 @@ package org.elasticsearch.index.mapper.geo;
 
 import com.carrotsearch.hppc.ObjectOpenHashSet;
 import com.carrotsearch.hppc.cursors.ObjectCursor;
+import com.google.common.base.Objects;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.FieldType;
 import org.apache.lucene.index.FieldInfo;
@@ -636,7 +637,7 @@ public class GeoPointFieldMapper extends AbstractFieldMapper<GeoPoint> implement
         if (this.normalizeLon != fieldMergeWith.normalizeLon) {
             mergeContext.addConflict("mapper [" + names.fullName() + "] has different normalize_lon");
         }
-        if (this.precisionStep != fieldMergeWith.precisionStep) {
+        if (!Objects.equal(this.precisionStep, fieldMergeWith.precisionStep)) {
             mergeContext.addConflict("mapper [" + names.fullName() + "] has different precision_step");
         }
 

+ 3 - 3
src/main/java/org/elasticsearch/monitor/jvm/GcNames.java

@@ -23,9 +23,9 @@ package org.elasticsearch.monitor.jvm;
  */
 public class GcNames {
 
-    public static String YOUNG = "young";
-    public static String OLD = "old";
-    public static String SURVIVOR = "survivor";
+    public static final String YOUNG = "young";
+    public static final String OLD = "old";
+    public static final String SURVIVOR = "survivor";
 
     /**
      * Resolves the GC type by its memory pool name ({@link java.lang.management.MemoryPoolMXBean#getName()}.

+ 3 - 6
src/main/java/org/elasticsearch/rest/action/cat/RestAllocationAction.java

@@ -34,7 +34,6 @@ import org.elasticsearch.common.Table;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeValue;
-import org.elasticsearch.monitor.fs.FsStats;
 import org.elasticsearch.rest.RestChannel;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
@@ -42,8 +41,6 @@ import org.elasticsearch.rest.XContentThrowableRestResponse;
 import org.elasticsearch.rest.action.support.RestTable;
 
 import java.io.IOException;
-import java.util.Iterator;
-import java.util.Locale;
 
 import static org.elasticsearch.rest.RestRequest.Method.GET;
 
@@ -163,9 +160,9 @@ public class RestAllocationAction extends AbstractCatAction {
             table.addCell(avail < 0 ? null : new ByteSizeValue(avail));
             table.addCell(nodeStats.getFs().getTotal().getTotal());
             table.addCell(diskPercent < 0 ? null : diskPercent);
-            table.addCell(node == null ? null : node.getHostName());
-            table.addCell(node == null ? null : node.getHostAddress());
-            table.addCell(node == null ? "UNASSIGNED" : node.name());
+            table.addCell(node.getHostName());
+            table.addCell(node.getHostAddress());
+            table.addCell(node.name());
             table.endRow();
         }
 

+ 1 - 1
src/main/java/org/elasticsearch/rest/support/RestUtils.java

@@ -31,7 +31,7 @@ import java.util.Map;
  */
 public class RestUtils {
 
-    public static PathTrie.Decoder REST_DECODER = new PathTrie.Decoder() {
+    public static final PathTrie.Decoder REST_DECODER = new PathTrie.Decoder() {
         @Override
         public String decode(String value) {
             return RestUtils.decodeComponent(value);

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/InternalGeoHashGrid.java

@@ -45,7 +45,7 @@ public class InternalGeoHashGrid extends InternalAggregation implements GeoHashG
 
     public static final Type TYPE = new Type("geohash_grid", "ghcells");
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public InternalGeoHashGrid readResult(StreamInput in) throws IOException {
             InternalGeoHashGrid buckets = new InternalGeoHashGrid();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantLongTerms.java

@@ -41,7 +41,7 @@ public class SignificantLongTerms extends InternalSignificantTerms {
 
     public static final Type TYPE = new Type("significant_terms", "siglterms");
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public SignificantLongTerms readResult(StreamInput in) throws IOException {
             SignificantLongTerms buckets = new SignificantLongTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/significant/SignificantStringTerms.java

@@ -41,7 +41,7 @@ public class SignificantStringTerms extends InternalSignificantTerms {
 
     public static final InternalAggregation.Type TYPE = new Type("significant_terms", "sigsterms");
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public SignificantStringTerms readResult(StreamInput in) throws IOException {
             SignificantStringTerms buckets = new SignificantStringTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/significant/UnmappedSignificantTerms.java

@@ -38,7 +38,7 @@ public class UnmappedSignificantTerms extends InternalSignificantTerms {
     private static final Collection<Bucket> BUCKETS = Collections.emptyList();
     private static final Map<String, Bucket> BUCKETS_MAP = Collections.emptyMap();
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public UnmappedSignificantTerms readResult(StreamInput in) throws IOException {
             UnmappedSignificantTerms buckets = new UnmappedSignificantTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/DoubleTerms.java

@@ -44,7 +44,7 @@ public class DoubleTerms extends InternalTerms {
 
     public static final Type TYPE = new Type("terms", "dterms");
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public DoubleTerms readResult(StreamInput in) throws IOException {
             DoubleTerms buckets = new DoubleTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/LongTerms.java

@@ -44,7 +44,7 @@ public class LongTerms extends InternalTerms {
 
     public static final Type TYPE = new Type("terms", "lterms");
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public LongTerms readResult(StreamInput in) throws IOException {
             LongTerms buckets = new LongTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/StringTerms.java

@@ -41,7 +41,7 @@ public class StringTerms extends InternalTerms {
 
     public static final InternalAggregation.Type TYPE = new Type("terms", "sterms");
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public StringTerms readResult(StreamInput in) throws IOException {
             StringTerms buckets = new StringTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/aggregations/bucket/terms/UnmappedTerms.java

@@ -38,7 +38,7 @@ public class UnmappedTerms extends InternalTerms {
     private static final Collection<Bucket> BUCKETS = Collections.emptyList();
     private static final Map<String, Bucket> BUCKETS_MAP = Collections.emptyMap();
 
-    public static AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
+    public static final AggregationStreams.Stream STREAM = new AggregationStreams.Stream() {
         @Override
         public UnmappedTerms readResult(StreamInput in) throws IOException {
             UnmappedTerms buckets = new UnmappedTerms();

+ 1 - 1
src/main/java/org/elasticsearch/search/controller/SearchPhaseController.java

@@ -56,7 +56,7 @@ import java.util.*;
  */
 public class SearchPhaseController extends AbstractComponent {
 
-    public static Comparator<AtomicArray.Entry<? extends QuerySearchResultProvider>> QUERY_RESULT_ORDERING = new Comparator<AtomicArray.Entry<? extends QuerySearchResultProvider>>() {
+    public static final Comparator<AtomicArray.Entry<? extends QuerySearchResultProvider>> QUERY_RESULT_ORDERING = new Comparator<AtomicArray.Entry<? extends QuerySearchResultProvider>>() {
         @Override
         public int compare(AtomicArray.Entry<? extends QuerySearchResultProvider> o1, AtomicArray.Entry<? extends QuerySearchResultProvider> o2) {
             int i = o1.value.shardTarget().index().compareTo(o2.value.shardTarget().index());

+ 2 - 2
src/main/java/org/elasticsearch/search/suggest/SuggestUtils.java

@@ -44,8 +44,8 @@ import java.util.Comparator;
 import java.util.Locale;
 
 public final class SuggestUtils {
-    public static Comparator<SuggestWord> LUCENE_FREQUENCY = new SuggestWordFrequencyComparator();
-    public static Comparator<SuggestWord> SCORE_COMPARATOR = SuggestWordQueue.DEFAULT_COMPARATOR;
+    public static final Comparator<SuggestWord> LUCENE_FREQUENCY = new SuggestWordFrequencyComparator();
+    public static final Comparator<SuggestWord> SCORE_COMPARATOR = SuggestWordQueue.DEFAULT_COMPARATOR;
     
     private SuggestUtils() {
         // utils!!

+ 9 - 0
src/main/java/org/elasticsearch/search/suggest/context/CategoryContextMapping.java

@@ -212,6 +212,15 @@ public class CategoryContextMapping extends ContextMapping {
         return false;
     }
 
+    @Override
+    public int hashCode() {
+        int hashCode = fieldName.hashCode();
+        for (CharSequence seq : defaultValues) {
+            hashCode = 31 * hashCode + seq.hashCode();
+        }
+        return hashCode;
+    }
+
     private static class FieldConfig extends ContextConfig {
 
         private final String fieldname;

+ 1 - 1
src/main/java/org/elasticsearch/search/suggest/context/GeolocationContextMapping.java

@@ -575,7 +575,7 @@ public class GeolocationContextMapping extends ContextMapping {
         protected TokenStream wrapTokenStream(Document doc, TokenStream stream) {
             Collection<String> geohashes;
 
-            if(locations == null | locations.size() == 0) {
+            if (locations == null || locations.size() == 0) {
                 if(mapping.fieldName != null) {
                     IndexableField[] fields = doc.getFields(mapping.fieldName);
                     if(fields.length > 0) {

+ 5 - 5
src/main/java/org/elasticsearch/search/suggest/term/TermSuggestion.java

@@ -18,9 +18,6 @@
  */
 package org.elasticsearch.search.suggest.term;
 
-import java.io.IOException;
-import java.util.Comparator;
-
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -30,13 +27,16 @@ import org.elasticsearch.common.xcontent.XContentBuilderString;
 import org.elasticsearch.search.suggest.Suggest.Suggestion;
 import org.elasticsearch.search.suggest.Suggest.Suggestion.Entry.Option;
 
+import java.io.IOException;
+import java.util.Comparator;
+
 /**
  * The suggestion responses corresponding with the suggestions in the request.
  */
 public class TermSuggestion extends Suggestion<TermSuggestion.Entry> {
 
-    public static Comparator<Suggestion.Entry.Option> SCORE = new Score();
-    public static Comparator<Suggestion.Entry.Option> FREQUENCY = new Frequency();
+    public static final Comparator<Suggestion.Entry.Option> SCORE = new Score();
+    public static final Comparator<Suggestion.Entry.Option> FREQUENCY = new Frequency();
 
     // Same behaviour as comparators in suggest module, but for SuggestedWord
     // Highest score first, then highest freq first, then lowest term first

+ 1 - 1
src/main/java/org/elasticsearch/transport/ConnectTransportException.java

@@ -41,7 +41,7 @@ public class ConnectTransportException extends ActionTransportException {
     }
 
     public ConnectTransportException(DiscoveryNode node, String msg, String action, Throwable cause) {
-        super(node.name(), node.address(), action, msg, cause);
+        super(node == null ? null : node.name(), node == null ? null : node.address(), action, msg, cause);
         this.node = node;
     }