LUCENE-7723: Add hashCode and equals to DoubleValuesSource

lucene/CHANGES.txt

@@ -89,6 +89,9 @@ API Changes
 * LUCENE-7867: The deprecated Token class is now only available in the test
   framework (Alan Woodward, Adrien Grand)
 
+* LUCENE-7723: DoubleValuesSource enforces implementation of equals() and
+  hashCode() (Alan Woodward)
+
 Bug Fixes
 
 * LUCENE-7626: IndexWriter will no longer accept broken token offsets

lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java

@@ -20,9 +20,7 @@ package org.apache.lucene.search;
 import java.io.IOException;
 import java.util.Objects;
 import java.util.function.DoubleToLongFunction;
-import java.util.function.DoubleUnaryOperator;
 import java.util.function.LongToDoubleFunction;
-import java.util.function.ToDoubleBiFunction;
 
 import org.apache.lucene.index.DocValues;
 import org.apache.lucene.index.LeafReaderContext;
@@ -76,14 +74,33 @@ public abstract class DoubleValuesSource {
     return new DoubleValuesSortField(this, reverse);
   }
 
+  @Override
+  public abstract int hashCode();
+
+  @Override
+  public abstract boolean equals(Object obj);
+
+  @Override
+  public abstract String toString();
+
   /**
    * Convert to a LongValuesSource by casting the double values to longs
    */
   public final LongValuesSource toLongValuesSource() {
-    return new LongValuesSource() {
+    return new LongDoubleValuesSource(this);
+  }
+
+  private static class LongDoubleValuesSource extends LongValuesSource {
+
+    private final DoubleValuesSource inner;
+
+    private LongDoubleValuesSource(DoubleValuesSource inner) {
+      this.inner = inner;
+    }
+
     @Override
       public LongValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
-        DoubleValues in = DoubleValuesSource.this.getValues(ctx, scores);
+        DoubleValues in = inner.getValues(ctx, scores);
         return new LongValues() {
           @Override
           public long longValue() throws IOException {
@@ -99,9 +116,27 @@ public abstract class DoubleValuesSource {
 
       @Override
       public boolean needsScores() {
-        return DoubleValuesSource.this.needsScores();
+        return inner.needsScores();
       }
-    };
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      LongDoubleValuesSource that = (LongDoubleValuesSource) o;
+      return Objects.equals(inner, that.inner);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(inner);
+    }
+
+    @Override
+    public String toString() {
+      return "long(" + inner.toString() + ")";
+    }
+
   }
 
   /**
@@ -164,13 +199,38 @@ public abstract class DoubleValuesSource {
     public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) {
       return scoreExplanation;
     }
+
+    @Override
+    public int hashCode() {
+      return 0;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      return obj == this;
+    }
+
+    @Override
+    public String toString() {
+      return "scores";
+    }
   };
 
   /**
    * Creates a DoubleValuesSource that always returns a constant value
    */
   public static DoubleValuesSource constant(double value) {
-    return new DoubleValuesSource() {
+    return new ConstantValuesSource(value);
+  }
+
+  private static class ConstantValuesSource extends DoubleValuesSource {
+
+    private final double value;
+
+    private ConstantValuesSource(double value) {
+      this.value = value;
+    }
+
     @Override
     public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
       return new DoubleValues() {
@@ -196,83 +256,23 @@ public abstract class DoubleValuesSource {
       return Explanation.match((float) value, "constant(" + value + ")");
     }
 
+    @Override
+    public int hashCode() {
+      return Objects.hash(value);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      ConstantValuesSource that = (ConstantValuesSource) o;
+      return Double.compare(that.value, value) == 0;
+    }
+
     @Override
     public String toString() {
       return "constant(" + value + ")";
     }
-    };
-  }
-
-  /**
-   * Creates a DoubleValuesSource that is a function of another DoubleValuesSource
-   */
-  public static DoubleValuesSource function(DoubleValuesSource in, String description, DoubleUnaryOperator function) {
-    return new DoubleValuesSource() {
-      @Override
-      public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
-        DoubleValues inputs = in.getValues(ctx, scores);
-        return new DoubleValues() {
-          @Override
-          public double doubleValue() throws IOException {
-            return function.applyAsDouble(inputs.doubleValue());
-          }
-
-          @Override
-          public boolean advanceExact(int doc) throws IOException {
-            return inputs.advanceExact(doc);
-          }
-        };
-      }
-
-      @Override
-      public boolean needsScores() {
-        return in.needsScores();
-      }
-
-      @Override
-      public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException {
-        Explanation inner = in.explain(ctx, docId, scoreExplanation);
-        return Explanation.match((float) function.applyAsDouble(inner.getValue()), description + ", computed from:", inner, scoreExplanation);
-      }
-    };
-  }
-
-  /**
-   * Creates a DoubleValuesSource that is a function of another DoubleValuesSource and a score
-   * @param in        the DoubleValuesSource to use as an input
-   * @param description a description of the function
-   * @param function  a function of the form (source, score) == result
-   */
-  public static DoubleValuesSource scoringFunction(DoubleValuesSource in, String description, ToDoubleBiFunction<Double, Double> function) {
-    return new DoubleValuesSource() {
-      @Override
-      public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
-        DoubleValues inputs = in.getValues(ctx, scores);
-        return new DoubleValues() {
-          @Override
-          public double doubleValue() throws IOException {
-            return function.applyAsDouble(inputs.doubleValue(), scores.doubleValue());
-          }
-
-          @Override
-          public boolean advanceExact(int doc) throws IOException {
-            return inputs.advanceExact(doc);
-          }
-        };
-      }
-
-      @Override
-      public boolean needsScores() {
-        return true;
-      }
-
-      @Override
-      public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException {
-        Explanation inner = in.explain(ctx, docId, scoreExplanation);
-        return Explanation.match((float) function.applyAsDouble((double)inner.getValue(), (double)scoreExplanation.getValue()),
-                    description + ", computed from:", inner, scoreExplanation);
-      }
-    };
   }
 
   /**
@@ -312,6 +312,11 @@ public abstract class DoubleValuesSource {
           Objects.equals(decoder, that.decoder);
     }
 
+    @Override
+    public String toString() {
+      return "double(" + field + ")";
+    }
+
     @Override
     public int hashCode() {
       return Objects.hash(field, decoder);
@@ -342,9 +347,9 @@ public abstract class DoubleValuesSource {
     public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException {
       DoubleValues values = getValues(ctx, null);
       if (values.advanceExact(docId))
-        return Explanation.match((float)values.doubleValue(), "double(" + field + ")");
+        return Explanation.match((float)values.doubleValue(), this.toString());
       else
-        return Explanation.noMatch("double(" + field + ")");
+        return Explanation.noMatch(this.toString());
     }
   }
 

lucene/core/src/java/org/apache/lucene/search/LongValuesSource.java

@@ -52,6 +52,15 @@ public abstract class LongValuesSource {
    */
   public abstract boolean needsScores();
 
+  @Override
+  public abstract int hashCode();
+
+  @Override
+  public abstract boolean equals(Object obj);
+
+  @Override
+  public abstract String toString();
+
   /**
    * Create a sort field based on the value of this producer
    * @param reverse true if the sort should be decreasing
@@ -78,7 +87,17 @@ public abstract class LongValuesSource {
    * Creates a LongValuesSource that always returns a constant value
    */
   public static LongValuesSource constant(long value) {
-    return new LongValuesSource() {
+    return new ConstantLongValuesSource(value);
+  }
+
+  private static class ConstantLongValuesSource extends LongValuesSource {
+
+    private final long value;
+
+    private ConstantLongValuesSource(long value) {
+      this.value = value;
+    }
+
     @Override
     public LongValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
       return new LongValues() {
@@ -98,7 +117,25 @@ public abstract class LongValuesSource {
     public boolean needsScores() {
       return false;
     }
-    };
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(value);
+    }
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      ConstantLongValuesSource that = (ConstantLongValuesSource) o;
+      return value == that.value;
+    }
+
+    @Override
+    public String toString() {
+      return "constant(" + value + ")";
+    }
+
   }
 
   private static class FieldValuesSource extends LongValuesSource {
@@ -117,6 +154,11 @@ public abstract class LongValuesSource {
       return Objects.equals(field, that.field);
     }
 
+    @Override
+    public String toString() {
+      return "long(" + field + ")";
+    }
+
     @Override
     public int hashCode() {
       return Objects.hash(field);

lucene/core/src/test/org/apache/lucene/search/TestDoubleValuesSource.java

@@ -92,6 +92,14 @@ public class TestDoubleValuesSource extends LuceneTestCase {
     assertEquals(vs1.hashCode(), vs2.hashCode());
     DoubleValuesSource v3 = DoubleValuesSource.fromLongField("long");
     assertFalse(vs1.equals(v3));
+
+    assertEquals(DoubleValuesSource.constant(5), DoubleValuesSource.constant(5));
+    assertEquals(DoubleValuesSource.constant(5).hashCode(), DoubleValuesSource.constant(5).hashCode());
+    assertFalse((DoubleValuesSource.constant(5).equals(DoubleValuesSource.constant(6))));
+
+    assertEquals(DoubleValuesSource.SCORES, DoubleValuesSource.SCORES);
+    assertFalse(DoubleValuesSource.constant(5).equals(DoubleValuesSource.SCORES));
+
   }
 
   public void testSimpleFieldSortables() throws Exception {
@@ -184,13 +192,6 @@ public class TestDoubleValuesSource extends LuceneTestCase {
       testExplanations(q, DoubleValuesSource.fromDoubleField("double"));
       testExplanations(q, DoubleValuesSource.fromDoubleField("onefield"));
       testExplanations(q, DoubleValuesSource.constant(5.45));
-      testExplanations(q, DoubleValuesSource.function(
-          DoubleValuesSource.fromDoubleField("double"), "v * 4 + 73",
-          v -> v * 4 + 73
-      ));
-      testExplanations(q, DoubleValuesSource.scoringFunction(
-          DoubleValuesSource.fromDoubleField("double"), "v * score", (v, s) -> v * s
-      ));
     }
   }
 
@@ -227,4 +228,5 @@ public class TestDoubleValuesSource extends LuceneTestCase {
       }
     });
   }
+
 }

lucene/facet/src/test/org/apache/lucene/facet/range/TestRangeFacetCounts.java

@@ -706,19 +706,7 @@ public class TestRangeFacetCounts extends FacetTestCase {
 
   }
 
-  public void testCustomDoubleValuesSource() throws Exception {
-    Directory dir = newDirectory();
-    RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
-    
-    Document doc = new Document();
-    writer.addDocument(doc);
-    writer.addDocument(doc);
-    writer.addDocument(doc);
-
-    // Test wants 3 docs in one segment:
-    writer.forceMerge(1);
-
-    final DoubleValuesSource vs = new DoubleValuesSource() {
+  private static class PlusOneValuesSource extends DoubleValuesSource {
 
     @Override
     public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
@@ -746,7 +734,36 @@ public class TestRangeFacetCounts extends FacetTestCase {
     public Explanation explain(LeafReaderContext ctx, int docId, Explanation scoreExplanation) throws IOException {
       return Explanation.match(docId + 1, "");
     }
-    };
+
+    @Override
+    public int hashCode() {
+      return 0;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      return obj.getClass() == PlusOneValuesSource.class;
+    }
+
+    @Override
+    public String toString() {
+      return null;
+    }
+  }
+
+  public void testCustomDoubleValuesSource() throws Exception {
+    Directory dir = newDirectory();
+    RandomIndexWriter writer = new RandomIndexWriter(random(), dir);
+    
+    Document doc = new Document();
+    writer.addDocument(doc);
+    writer.addDocument(doc);
+    writer.addDocument(doc);
+
+    // Test wants 3 docs in one segment:
+    writer.forceMerge(1);
+
+    final DoubleValuesSource vs = new PlusOneValuesSource();
 
     FacetsConfig config = new FacetsConfig();
     

lucene/queries/src/java/org/apache/lucene/queries/function/FunctionScoreQuery.java

@@ -121,15 +121,6 @@ public final class FunctionScoreQuery extends Query {
           Explanation.match(boost, "boost"), expl);
     }
 
-    private Explanation scoreExplanation(LeafReaderContext context, int doc, DoubleValues scores) throws IOException {
-      if (valueSource.needsScores() == false)
-        return Explanation.match((float) scores.doubleValue(), valueSource.toString());
-      float score = (float) scores.doubleValue();
-      return Explanation.match(score, "computed from:",
-          Explanation.match(score, valueSource.toString()),
-          inner.explain(context, doc));
-    }
-
     @Override
     public Scorer scorer(LeafReaderContext context) throws IOException {
       Scorer in = inner.scorer(context);

lucene/queries/src/java/org/apache/lucene/queries/function/ValueSource.java

@@ -20,6 +20,7 @@ import java.io.IOException;
 import java.util.HashMap;
 import java.util.IdentityHashMap;
 import java.util.Map;
+import java.util.Objects;
 
 import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.search.DocIdSetIterator;
@@ -120,13 +121,23 @@ public abstract class ValueSource {
    * Expose this ValueSource as a LongValuesSource
    */
   public LongValuesSource asLongValuesSource() {
-    return new LongValuesSource() {
+    return new WrappedLongValuesSource(this);
+  }
+
+  private static class WrappedLongValuesSource extends LongValuesSource {
+
+    private final ValueSource in;
+
+    private WrappedLongValuesSource(ValueSource in) {
+      this.in = in;
+    }
+
     @Override
     public LongValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
       Map context = new IdentityHashMap<>();
       FakeScorer scorer = new FakeScorer();
       context.put("scorer", scorer);
-        final FunctionValues fv = ValueSource.this.getValues(context, ctx);
+      final FunctionValues fv = in.getValues(context, ctx);
       return new LongValues() {
 
         @Override
@@ -150,20 +161,48 @@ public abstract class ValueSource {
     public boolean needsScores() {
       return false;
     }
-    };
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      WrappedLongValuesSource that = (WrappedLongValuesSource) o;
+      return Objects.equals(in, that.in);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(in);
+    }
+
+    @Override
+    public String toString() {
+      return in.toString();
+    }
+
   }
 
   /**
    * Expose this ValueSource as a DoubleValuesSource
    */
   public DoubleValuesSource asDoubleValuesSource() {
-    return new DoubleValuesSource() {
+    return new WrappedDoubleValuesSource(this);
+  }
+
+  private static class WrappedDoubleValuesSource extends DoubleValuesSource {
+
+    private final ValueSource in;
+
+    private WrappedDoubleValuesSource(ValueSource in) {
+      this.in = in;
+    }
+
     @Override
     public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
       Map context = new HashMap<>();
       FakeScorer scorer = new FakeScorer();
       context.put("scorer", scorer);
-        FunctionValues fv = ValueSource.this.getValues(context, ctx);
+      FunctionValues fv = in.getValues(context, ctx);
       return new DoubleValues() {
 
         @Override
@@ -195,10 +234,28 @@ public abstract class ValueSource {
       FakeScorer scorer = new FakeScorer();
       scorer.score = scoreExplanation.getValue();
       context.put("scorer", scorer);
-        FunctionValues fv = ValueSource.this.getValues(context, ctx);
+      FunctionValues fv = in.getValues(context, ctx);
       return fv.explain(docId);
     }
-    };
+
+    @Override
+    public boolean equals(Object o) {
+      if (this == o) return true;
+      if (o == null || getClass() != o.getClass()) return false;
+      WrappedDoubleValuesSource that = (WrappedDoubleValuesSource) o;
+      return Objects.equals(in, that.in);
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(in);
+    }
+
+    @Override
+    public String toString() {
+      return in.toString();
+    }
+
   }
 
   //

lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreExplanations.java

@@ -59,19 +59,15 @@ public class TestFunctionScoreExplanations extends BaseExplanationTestCase {
 
   public void testExplanationsIncludingScore() throws Exception {
 
-    DoubleValuesSource scores = DoubleValuesSource.function(DoubleValuesSource.SCORES, "v * 2", v -> v * 2);
-
     Query q = new TermQuery(new Term(FIELD, "w1"));
-    FunctionScoreQuery csq = new FunctionScoreQuery(q, scores);
+    FunctionScoreQuery csq = new FunctionScoreQuery(q, DoubleValuesSource.SCORES);
 
     qtest(csq, new int[] { 0, 1, 2, 3 });
 
     Explanation e1 = searcher.explain(q, 0);
     Explanation e = searcher.explain(csq, 0);
 
-    assertEquals(e.getDetails().length, 2);
-
-    assertEquals(e1.getValue() * 2, e.getValue(), 0.00001);
+    assertEquals(e, e1);
   }
 
   public void testSubExplanations() throws IOException {

lucene/queries/src/test/org/apache/lucene/queries/function/TestFunctionScoreQuery.java

@@ -17,12 +17,18 @@
 
 package org.apache.lucene.queries.function;
 
+import java.io.IOException;
+import java.util.function.DoubleUnaryOperator;
+import java.util.function.ToDoubleBiFunction;
+
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.index.LeafReaderContext;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.BooleanQuery;
 import org.apache.lucene.search.BoostQuery;
+import org.apache.lucene.search.DoubleValues;
 import org.apache.lucene.search.DoubleValuesSource;
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.Query;
@@ -70,7 +76,7 @@ public class TestFunctionScoreQuery extends FunctionTestSetup {
   public void testScoreModifyingSource() throws Exception {
 
     DoubleValuesSource iii = DoubleValuesSource.fromIntField("iii");
-    DoubleValuesSource score = DoubleValuesSource.scoringFunction(iii, "v * s", (v, s) -> v * s);
+    DoubleValuesSource score = scoringFunction(iii, (v, s) -> v * s);
 
     BooleanQuery bq = new BooleanQuery.Builder()
         .add(new TermQuery(new Term(TEXT_FIELD, "first")), BooleanClause.Occur.SHOULD)
@@ -95,8 +101,7 @@ public class TestFunctionScoreQuery extends FunctionTestSetup {
   // check boosts with non-distributive score source
   public void testBoostsAreAppliedLast() throws Exception {
 
-    DoubleValuesSource scores
-        = DoubleValuesSource.function(DoubleValuesSource.SCORES, "ln(v + 4)", v -> Math.log(v + 4));
+    DoubleValuesSource scores = function(DoubleValuesSource.SCORES, v -> Math.log(v + 4));
 
     Query q1 = new FunctionScoreQuery(new TermQuery(new Term(TEXT_FIELD, "text")), scores);
     TopDocs plain = searcher.search(q1, 5);
@@ -111,4 +116,84 @@ public class TestFunctionScoreQuery extends FunctionTestSetup {
 
   }
 
+  public static DoubleValuesSource function(DoubleValuesSource in, DoubleUnaryOperator function) {
+    return new DoubleValuesSource() {
+      @Override
+      public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
+        DoubleValues v = in.getValues(ctx, scores);
+        return new DoubleValues() {
+          @Override
+          public double doubleValue() throws IOException {
+            return function.applyAsDouble(v.doubleValue());
+          }
+
+          @Override
+          public boolean advanceExact(int doc) throws IOException {
+            return v.advanceExact(doc);
+          }
+        };
+      }
+
+      @Override
+      public boolean needsScores() {
+        return in.needsScores();
+      }
+
+      @Override
+      public int hashCode() {
+        return 0;
+      }
+
+      @Override
+      public boolean equals(Object obj) {
+        return false;
+      }
+
+      @Override
+      public String toString() {
+        return "fn";
+      }
+    };
+  }
+
+  private static DoubleValuesSource scoringFunction(DoubleValuesSource in, ToDoubleBiFunction<Double, Double> function) {
+    return new DoubleValuesSource() {
+      @Override
+      public DoubleValues getValues(LeafReaderContext ctx, DoubleValues scores) throws IOException {
+        DoubleValues v = in.getValues(ctx, scores);
+        return new DoubleValues() {
+          @Override
+          public double doubleValue() throws IOException {
+            return function.applyAsDouble(v.doubleValue(), scores.doubleValue());
+          }
+
+          @Override
+          public boolean advanceExact(int doc) throws IOException {
+            return v.advanceExact(doc);
+          }
+        };
+      }
+
+      @Override
+      public boolean needsScores() {
+        return true;
+      }
+
+      @Override
+      public int hashCode() {
+        return 0;
+      }
+
+      @Override
+      public boolean equals(Object obj) {
+        return false;
+      }
+
+      @Override
+      public String toString() {
+        return "fn";
+      }
+    };
+  }
+
 }

lucene/suggest/src/test/org/apache/lucene/search/suggest/DocumentValueSourceDictionaryTest.java

@@ -171,6 +171,21 @@ public class DocumentValueSourceDictionaryTest extends LuceneTestCase {
       public boolean needsScores() {
         return false;
       }
+
+      @Override
+      public int hashCode() {
+        return 0;
+      }
+
+      @Override
+      public boolean equals(Object obj) {
+        return false;
+      }
+
+      @Override
+      public String toString() {
+        return null;
+      }
     };
   }