LUCENE-6372: Simplified and improved equals/hashcode of span queries.

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1677963 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -186,6 +186,9 @@ Other
 * LUCENE-6382: Lucene now enforces that positions never exceed the
   maximum value IndexWriter.MAX_POSITION.  (Robert Muir, Mike McCandless)
 
+* LUCENE-6372: Simplified and improved equals/hashcode of span queries.
+  (Paul Elschot via Adrien Grand)
+
 Build
 
 * LUCENE-6420: Update forbiddenapis to v1.8  (Uwe Schindler)

lucene/core/src/java/org/apache/lucene/search/payloads/PayloadNearQuery.java

@@ -20,6 +20,7 @@ package org.apache.lucene.search.payloads;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.Objects;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.Explanation;
@@ -64,8 +65,8 @@ public class PayloadNearQuery extends SpanNearQuery {
   public PayloadNearQuery(SpanQuery[] clauses, int slop, boolean inOrder,
       PayloadFunction function) {
     super(clauses, slop, inOrder);
-    fieldName = clauses[0].getField(); // all clauses must have same field
+    this.fieldName = Objects.requireNonNull(clauses[0].getField(), "all clauses must have same non null field");
-    this.function = function;
+    this.function = Objects.requireNonNull(function);
   }
 
   @Override
@@ -111,32 +112,20 @@ public class PayloadNearQuery extends SpanNearQuery {
   @Override
   public int hashCode() {
     final int prime = 31;
-    int result = super.hashCode() ^ getClass().hashCode();
+    int result = super.hashCode();
-    result = prime * result + ((fieldName == null) ? 0 : fieldName.hashCode());
+    result = prime * result + fieldName.hashCode();
-    result = prime * result + ((function == null) ? 0 : function.hashCode());
+    result = prime * result + function.hashCode();
     return result;
   }
 
   @Override
   public boolean equals(Object obj) {
-    if (this == obj)
+    if (! super.equals(obj)) {
-      return true;
-    if (!super.equals(obj))
-      return false;
-    if (getClass() != obj.getClass())
       return false;
+    }
     PayloadNearQuery other = (PayloadNearQuery) obj;
-    if (fieldName == null) {
+    return fieldName.equals(other.fieldName)
-      if (other.fieldName != null)
+          && function.equals(other.function);
-        return false;
-    } else if (!fieldName.equals(other.fieldName))
-      return false;
-    if (function == null) {
-      if (other.function != null)
-        return false;
-    } else if (!function.equals(other.function))
-      return false;
-    return true;
   }
 
   public class PayloadNearSpanWeight extends SpanWeight {

lucene/core/src/java/org/apache/lucene/search/payloads/PayloadTermQuery.java

@@ -217,16 +217,12 @@ public class PayloadTermQuery extends SpanTermQuery {
 
   @Override
   public boolean equals(Object obj) {
-    if (this == obj)
+    if (!super.equals(obj)) {
-      return true;
-    if (!super.equals(obj))
-      return false;
-    if (getClass() != obj.getClass())
       return false;
+    }
     PayloadTermQuery other = (PayloadTermQuery) obj;
-    if (includeSpanScore != other.includeSpanScore)
+    return (includeSpanScore == other.includeSpanScore)
-      return false;
+         && function.equals(other.function);
-    return function.equals(other.function);
   }
 
 }

lucene/core/src/java/org/apache/lucene/search/spans/FieldMaskingSpanQuery.java

@@ -20,6 +20,7 @@ package org.apache.lucene.search.spans;
 import java.io.IOException;
 import java.util.Map;
 import java.util.Set;
+import java.util.Objects;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.IndexReader;
@@ -79,8 +80,8 @@ public class FieldMaskingSpanQuery extends SpanQuery {
   private String field;
     
   public FieldMaskingSpanQuery(SpanQuery maskedQuery, String maskedField) {
-    this.maskedQuery = maskedQuery;
+    this.maskedQuery = Objects.requireNonNull(maskedQuery);
-    this.field = maskedField;
+    this.field = Objects.requireNonNull(maskedField);
   }
 
   @Override
@@ -141,19 +142,19 @@ public class FieldMaskingSpanQuery extends SpanQuery {
   
   @Override
   public boolean equals(Object o) {
-    if (!(o instanceof FieldMaskingSpanQuery))
+    if (! super.equals(o)) {
       return false;
+    }
     FieldMaskingSpanQuery other = (FieldMaskingSpanQuery) o;
     return (this.getField().equals(other.getField())
-            && (this.getBoost() == other.getBoost())
             && this.getMaskedQuery().equals(other.getMaskedQuery()));
 
   }
   
   @Override
   public int hashCode() {
-    return getMaskedQuery().hashCode()
+    return super.hashCode()
-      ^ getField().hashCode()
+          ^ getMaskedQuery().hashCode()
-      ^ Float.floatToRawIntBits(getBoost());
+          ^ getField().hashCode();
   }
 }

lucene/core/src/java/org/apache/lucene/search/spans/SpanFirstQuery.java

@@ -67,24 +67,4 @@ public class SpanFirstQuery extends SpanPositionRangeQuery {
     return spanFirstQuery;
   }
 
-  @Override
-  public boolean equals(Object o) {
-    if (this == o) return true;
-    if (!(o instanceof SpanFirstQuery)) return false;
-
-    SpanFirstQuery other = (SpanFirstQuery)o;
-    return this.end == other.end
-         && this.match.equals(other.match)
-         && this.getBoost() == other.getBoost();
-  }
-
-  @Override
-  public int hashCode() {
-    int h = match.hashCode();
-    h ^= (h << 8) | (h >>> 25);  // reversible
-    h ^= Float.floatToRawIntBits(getBoost()) ^ end;
-    return h;
-  }
-
-
 }

lucene/core/src/java/org/apache/lucene/search/spans/SpanMultiTermQueryWrapper.java

@@ -20,6 +20,7 @@ package org.apache.lucene.search.spans;
 import java.io.IOException;
 import java.util.Map;
 import java.util.Set;
+import java.util.Objects;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.IndexReader;
@@ -63,7 +64,7 @@ public class SpanMultiTermQueryWrapper<Q extends MultiTermQuery> extends SpanQue
    */
   @SuppressWarnings({"rawtypes","unchecked"})
   public SpanMultiTermQueryWrapper(Q query) {
-    this.query = query;
+    this.query = Objects.requireNonNull(query);
     
     MultiTermQuery.RewriteMethod method = query.getRewriteMethod();
     if (method instanceof TopTermsRewrite) {
@@ -147,12 +148,11 @@ public class SpanMultiTermQueryWrapper<Q extends MultiTermQuery> extends SpanQue
 
   @Override
   public boolean equals(Object obj) {
-    if (this == obj) return true;
+    if (! super.equals(obj)) {
-    if (!super.equals(obj)) return false;
+      return false;
-    if (getClass() != obj.getClass()) return false;
+    }
     SpanMultiTermQueryWrapper<?> other = (SpanMultiTermQueryWrapper<?>) obj;
-    if (!query.equals(other.query)) return false;
+    return query.equals(other.query);
-    return true;
   }
 
   /** Abstract class that defines how the query is rewritten. */

lucene/core/src/java/org/apache/lucene/search/spans/SpanNearPayloadCheckQuery.java

@@ -22,6 +22,7 @@ import org.apache.lucene.util.ToStringUtils;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Objects;
 
 
 /**
@@ -37,7 +38,7 @@ public class SpanNearPayloadCheckQuery extends SpanPositionCheckQuery {
    */
   public SpanNearPayloadCheckQuery(SpanNearQuery match, Collection<byte[]> payloadToMatch) {
     super(match);
-    this.payloadToMatch = payloadToMatch;
+    this.payloadToMatch = Objects.requireNonNull(payloadToMatch);
   }
 
   @Override
@@ -95,22 +96,17 @@ public class SpanNearPayloadCheckQuery extends SpanPositionCheckQuery {
 
   @Override
   public boolean equals(Object o) {
-    if (this == o) return true;
+    if (! super.equals(o)) {
-    if (!(o instanceof SpanNearPayloadCheckQuery)) return false;
+      return false;
-
+    }
     SpanNearPayloadCheckQuery other = (SpanNearPayloadCheckQuery) o;
-    return this.payloadToMatch.equals(other.payloadToMatch)
+    return this.payloadToMatch.equals(other.payloadToMatch);
-            && this.match.equals(other.match)
-            && this.getBoost() == other.getBoost();
   }
 
   @Override
   public int hashCode() {
-    int h = match.hashCode() ^ getClass().hashCode();
+    int h = super.hashCode();
-    h ^= (h << 8) | (h >>> 25);  // reversible
+    h = (h * 15) ^ payloadToMatch.hashCode();
-    //TODO: is this right?
-    h ^= payloadToMatch.hashCode();
-    h ^= Float.floatToRawIntBits(getBoost());
     return h;
   }
 }

lucene/core/src/java/org/apache/lucene/search/spans/SpanNearQuery.java

@@ -176,29 +176,23 @@ public class SpanNearQuery extends SpanQuery implements Cloneable {
   /** Returns true iff <code>o</code> is equal to this. */
   @Override
   public boolean equals(Object o) {
-    if (this == o) return true;
+    if (! super.equals(o)) {
-    if (!(o instanceof SpanNearQuery)) return false;
+      return false;
-
+    }
     final SpanNearQuery spanNearQuery = (SpanNearQuery) o;
 
-    if (inOrder != spanNearQuery.inOrder) return false;
+    return (inOrder == spanNearQuery.inOrder)
-    if (slop != spanNearQuery.slop) return false;
+        && (slop == spanNearQuery.slop)
-    if (!clauses.equals(spanNearQuery.clauses)) return false;
+        && (collectPayloads == spanNearQuery.collectPayloads)
-
+        && clauses.equals(spanNearQuery.clauses);
-    return getBoost() == spanNearQuery.getBoost();
   }
 
   @Override
   public int hashCode() {
-    int result;
+    int result = super.hashCode();
-    result = clauses.hashCode();
+    result ^= clauses.hashCode();
-    // Mix bits before folding in things like boost, since it could cancel the
-    // last element of clauses.  This particular mix also serves to
-    // differentiate SpanNearQuery hashcodes from others.
-    result ^= (result << 14) | (result >>> 19);  // reversible
-    result += Float.floatToRawIntBits(getBoost());
     result += slop;
-    result ^= (inOrder ? 0x99AFD3BD : 0);
+    int fac = 1 + (inOrder ? 8 : 4) + (collectPayloads ? 2 : 0);
-    return result;
+    return fac * result;
   }
 }

lucene/core/src/java/org/apache/lucene/search/spans/SpanOrQuery.java

@@ -131,21 +131,17 @@ public class SpanOrQuery extends SpanQuery implements Cloneable {
 
   @Override
   public boolean equals(Object o) {
-    if (this == o) return true;
+    if (! super.equals(o)) {
-    if (o == null || getClass() != o.getClass()) return false;
+      return false;
-
+    }
     final SpanOrQuery that = (SpanOrQuery) o;
-
+    return clauses.equals(that.clauses);
-    if (!clauses.equals(that.clauses)) return false;
-
-    return getBoost() == that.getBoost();
   }
 
   @Override
   public int hashCode() {
-    int h = clauses.hashCode();
+    int h = super.hashCode();
-    h ^= (h << 10) | (h >>> 23);
+    h = (h * 7) ^ clauses.hashCode();
-    h ^= Float.floatToRawIntBits(getBoost());
     return h;
   }
 

lucene/core/src/java/org/apache/lucene/search/spans/SpanPayloadCheckQuery.java

@@ -97,22 +97,17 @@ public class SpanPayloadCheckQuery extends SpanPositionCheckQuery {
 
   @Override
   public boolean equals(Object o) {
-    if (this == o) return true;
+    if (! super.equals(o)) {
-    if (!(o instanceof SpanPayloadCheckQuery)) return false;
+      return false;
-
+    }
     SpanPayloadCheckQuery other = (SpanPayloadCheckQuery)o;
-    return this.payloadToMatch.equals(other.payloadToMatch)
+    return this.payloadToMatch.equals(other.payloadToMatch);
-         && this.match.equals(other.match)
-         && this.getBoost() == other.getBoost();
   }
 
   @Override
   public int hashCode() {
-    int h = match.hashCode() ^ getClass().hashCode();
+    int h = super.hashCode();
-    h ^= (h << 8) | (h >>> 25);  // reversible
+    h = (h * 63) ^ payloadToMatch.hashCode();
-    //TODO: is this right?
-    h ^= payloadToMatch.hashCode();
-    h ^= Float.floatToRawIntBits(getBoost()) ;
     return h;
   }
 }

lucene/core/src/java/org/apache/lucene/search/spans/SpanPositionCheckQuery.java

@@ -107,15 +107,15 @@ public abstract class SpanPositionCheckQuery extends SpanQuery implements Clonea
   /** Returns true iff <code>o</code> is equal to this. */
   @Override
   public boolean equals(Object o) {
-    if (this == o) return true;
+    if (! super.equals(o)) {
-    if (o == null) return false;
+      return false;
-    if (getClass() != o.getClass()) return false;
+    }
-    final SpanPositionCheckQuery spcq = (SpanPositionCheckQuery) o;
+    SpanPositionCheckQuery spcq = (SpanPositionCheckQuery) o;
     return match.equals(spcq.match);
   }
 
   @Override
   public int hashCode() {
-    return match.hashCode() ^ getClass().hashCode();
+    return match.hashCode() ^ super.hashCode();
   }
 }

lucene/core/src/java/org/apache/lucene/search/spans/SpanPositionRangeQuery.java

@@ -85,20 +85,17 @@ public class SpanPositionRangeQuery extends SpanPositionCheckQuery {
 
   @Override
   public boolean equals(Object o) {
-    if (this == o) return true;
+    if (! super.equals(o)) {
-    if (!(o instanceof SpanPositionRangeQuery)) return false;
+      return false;
-
+    }
     SpanPositionRangeQuery other = (SpanPositionRangeQuery)o;
-    return this.end == other.end && this.start == other.start
+    return this.end == other.end && this.start == other.start;
-         && this.match.equals(other.match)
-         && this.getBoost() == other.getBoost();
   }
 
   @Override
   public int hashCode() {
-    int h = match.hashCode() ^ getClass().hashCode();
+    int h = super.hashCode() ^ end;
-    h ^= (h << 8) | (h >>> 25);  // reversible
+    h = (h * 127) ^ start;
-    h ^= Float.floatToRawIntBits(getBoost()) ^ end ^ start;
     return h;
   }
 

lucene/core/src/java/org/apache/lucene/search/spans/SpanTermQuery.java

@@ -75,12 +75,9 @@ public class SpanTermQuery extends SpanQuery {
 
   @Override
   public boolean equals(Object obj) {
-    if (this == obj)
+    if (! super.equals(obj)) {
-      return true;
-    if (!super.equals(obj))
-      return false;
-    if (getClass() != obj.getClass())
       return false;
+    }
     SpanTermQuery other = (SpanTermQuery) obj;
     return term.equals(other.term);
   }