LUCENE-2945: Fix equals/hashCode for surround query parser generated queries

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

lucene/CHANGES.txt

@@ -529,6 +529,9 @@ Bug fixes
 * LUCENE-3348: Fix thread safety hazards in IndexWriter that could
   rarely cause deletions to be incorrectly applied.  (Yonik Seeley,
   Simon Willnauer, Mike McCandless)
+  
+* LUCENE-2945: Fix hashCode/equals for surround query parser generated queries.
+  (Paul Elschot, Simon Rosenthal, gsingers via ehatcher)
 
 ======================= Lucene 3.x (not yet released) ================
 

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/BasicQueryFactory.java

@@ -45,8 +45,19 @@ public class BasicQueryFactory {
   public int getNrQueriesMade() {return queriesMade;}
   public int getMaxBasicQueries() {return maxBasicQueries;}
   
-  private synchronized void checkMax() throws TooManyBasicQueries {
-    if (queriesMade >= maxBasicQueries)
+  public String toString() {
+    return getClass().getName()
+	  + "(maxBasicQueries: " + maxBasicQueries
+	  + ", queriesMade: " + queriesMade
+	  + ")";
+  }
+
+  private boolean atMax() {
+    return queriesMade >= maxBasicQueries;
+  }
+
+  protected synchronized void checkMax() throws TooManyBasicQueries {
+    if (atMax())
       throw new TooManyBasicQueries(getMaxBasicQueries());
     queriesMade++;
   }
@@ -60,6 +71,22 @@ public class BasicQueryFactory {
     checkMax();
     return new SpanTermQuery(term);
   }
+
+  @Override
+  public int hashCode() {
+    return getClass().hashCode() ^ (atMax() ? 7 : 31*32);
+  }
+
+  /** Two BasicQueryFactory's are equal when they generate
+   *  the same types of basic queries, or both cannot generate queries anymore.
+   */
+  @Override
+  public boolean equals(Object obj) {
+    if (! (obj instanceof BasicQueryFactory))
+      return false;
+    BasicQueryFactory other = (BasicQueryFactory) obj;
+    return atMax() == other.atMax();
+  }
 }
 
 

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/ComposedQuery.java

@@ -24,36 +24,36 @@ import org.apache.lucene.search.Query;
 
 public abstract class ComposedQuery extends SrndQuery { 
   
-  public ComposedQuery(List qs, boolean operatorInfix, String opName) {
+  public ComposedQuery(List<SrndQuery> qs, boolean operatorInfix, String opName) {
     recompose(qs);
     this.operatorInfix = operatorInfix;
     this.opName = opName;
   }
   
-  protected void recompose(List queries) {
+  protected void recompose(List<SrndQuery> queries) {
     if (queries.size() < 2) throw new AssertionError("Too few subqueries"); 
     this.queries = queries;
   }
   
-  private String opName;
+  protected String opName;
   public String getOperatorName() {return opName;}
   
-  private List queries;
+  protected List<SrndQuery> queries;
   
-  public Iterator getSubQueriesIterator() {return queries.listIterator();}
+  public Iterator<SrndQuery> getSubQueriesIterator() {return queries.listIterator();}
 
   public int getNrSubQueries() {return queries.size();}
   
-  public SrndQuery getSubQuery(int qn) {return (SrndQuery) queries.get(qn);}
+  public SrndQuery getSubQuery(int qn) {return queries.get(qn);}
 
   private boolean operatorInfix; 
   public boolean isOperatorInfix() { return operatorInfix; } /* else prefix operator */
   
   public List<Query> makeLuceneSubQueriesField(String fn, BasicQueryFactory qf) {
     List<Query> luceneSubQueries = new ArrayList<Query>();
-    Iterator sqi = getSubQueriesIterator();
+    Iterator<SrndQuery> sqi = getSubQueriesIterator();
     while (sqi.hasNext()) {
-      luceneSubQueries.add( ((SrndQuery) sqi.next()).makeLuceneQueryField(fn, qf));
+      luceneSubQueries.add( (sqi.next()).makeLuceneQueryField(fn, qf));
     }
     return luceneSubQueries;
   }
@@ -77,7 +77,7 @@ public abstract class ComposedQuery extends SrndQuery {
   
   protected void infixToString(StringBuilder r) {
     /* Brackets are possibly redundant in the result. */
-    Iterator sqi = getSubQueriesIterator();
+    Iterator<SrndQuery> sqi = getSubQueriesIterator();
     r.append(getBracketOpen());
     if (sqi.hasNext()) {
       r.append(sqi.next().toString());
@@ -92,7 +92,7 @@ public abstract class ComposedQuery extends SrndQuery {
   }
 
   protected void prefixToString(StringBuilder r) {
-    Iterator sqi = getSubQueriesIterator();
+    Iterator<SrndQuery> sqi = getSubQueriesIterator();
     r.append(getOperatorName()); /* prefix operator */
     r.append(getBracketOpen());
     if (sqi.hasNext()) {
@@ -109,9 +109,9 @@ public abstract class ComposedQuery extends SrndQuery {
   @Override
   public boolean isFieldsSubQueryAcceptable() {
     /* at least one subquery should be acceptable */
-    Iterator sqi = getSubQueriesIterator();
+    Iterator<SrndQuery> sqi = getSubQueriesIterator();
     while (sqi.hasNext()) {
-      if (((SrndQuery) sqi.next()).isFieldsSubQueryAcceptable()) {
+      if ((sqi.next()).isFieldsSubQueryAcceptable()) {
         return true;
       }
     }

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceQuery.java

@@ -16,7 +16,6 @@ package org.apache.lucene.queryparser.surround.query;
  * limitations under the License.
  */
 
-
 import java.util.List;
 import java.util.Iterator;
 
@@ -39,12 +38,14 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery {
     this.ordered = ordered;
   }
 
+
   private int opDistance;
   public int getOpDistance() {return opDistance;}
   
   private boolean ordered;
   public boolean subQueriesOrdered() {return ordered;}
   
+  @Override
   public String distanceSubQueryNotAllowed() {
     Iterator<?> sqi = getSubQueriesIterator();
     while (sqi.hasNext()) {
@@ -61,31 +62,14 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery {
     }
     return null; /* subqueries acceptable */
   }
-
   
+  @Override
   public void addSpanQueries(SpanNearClauseFactory sncf) throws IOException {
     Query snq = getSpanNearQuery(sncf.getIndexReader(),
                                   sncf.getFieldName(),
                                   getWeight(),
                                   sncf.getBasicQueryFactory());
-    sncf.addSpanNearQuery(snq);
-  }
-
-  @Override
-  public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) {
-    return new Query () {
-      
-      @Override
-      public String toString(String fn) {
-        return getClass().toString() + " " + fieldName + " (" + fn + "?)";
-      }
-      
-      @Override
-      public Query rewrite(IndexReader reader) throws IOException {
-        return getSpanNearQuery(reader, fieldName, getBoost(), qf);
-      }
-      
-    };
+    sncf.addSpanQuery(snq);
   }
   
   public Query getSpanNearQuery(
@@ -93,7 +77,7 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery {
           String fieldName,
           float boost,
           BasicQueryFactory qf) throws IOException {
-    SpanQuery[] spanNearClauses = new SpanQuery[getNrSubQueries()];
+    SpanQuery[] spanClauses = new SpanQuery[getNrSubQueries()];
     Iterator<?> sqi = getSubQueriesIterator();
     int qi = 0;
     while (sqi.hasNext()) {
@@ -108,14 +92,18 @@ public class DistanceQuery extends ComposedQuery implements DistanceSubQuery {
         return SrndQuery.theEmptyLcnQuery;
       }
       
-      spanNearClauses[qi] = sncf.makeSpanNearClause();
-
+      spanClauses[qi] = sncf.makeSpanClause();
       qi++;
     }
-    
-    SpanNearQuery r = new SpanNearQuery(spanNearClauses, getOpDistance() - 1, subQueriesOrdered());
+
+    SpanNearQuery r = new SpanNearQuery(spanClauses, getOpDistance() - 1, subQueriesOrdered());
     r.setBoost(boost);
     return r;
   }
+
+  @Override
+  public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) {
+    return new DistanceRewriteQuery(this, fieldName, qf);
+  }
 }
 

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/DistanceRewriteQuery.java

@@ -0,0 +1,38 @@
+package org.apache.lucene.queryparser.surround.query;
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.IOException;
+
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.Query;
+
+class DistanceRewriteQuery extends RewriteQuery<DistanceQuery> {
+
+  DistanceRewriteQuery(
+      DistanceQuery srndQuery,
+      String fieldName,
+      BasicQueryFactory qf) {
+    super(srndQuery, fieldName, qf);
+  }
+
+  @Override
+  public Query rewrite(IndexReader reader) throws IOException {
+    return srndQuery.getSpanNearQuery(reader, fieldName, getBoost(), qf);
+  }
+}
+

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/RewriteQuery.java

@@ -0,0 +1,81 @@
+package org.apache.lucene.queryparser.surround.query;
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import java.io.IOException;
+
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.Query;
+
+abstract class RewriteQuery<SQ extends SrndQuery> extends Query {
+  protected final SQ srndQuery;
+  protected final String fieldName;
+  protected final BasicQueryFactory qf;
+
+  RewriteQuery(
+      SQ srndQuery,
+      String fieldName,
+      BasicQueryFactory qf) {
+    this.srndQuery = srndQuery;
+    this.fieldName = fieldName;
+    this.qf = qf;
+  }
+
+  @Override
+  abstract public Query rewrite(IndexReader reader) throws IOException;
+
+  @Override
+  public String toString() {
+    return toString(null);
+  }
+
+  @Override
+  public String toString(String field) {
+    return getClass().getName()
+	  + (field == null ? "" : "(unused: " + field + ")")
+	  + "(" + fieldName
+	  + ", " + srndQuery.toString()
+	  + ", " + qf.toString()
+	  + ")";
+  }
+
+  @Override
+  public int hashCode() {
+    return getClass().hashCode()
+	  ^ fieldName.hashCode()
+	  ^ qf.hashCode()
+	  ^ srndQuery.hashCode();
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj == null)
+      return false;
+    if (! getClass().equals(obj.getClass()))
+      return false;
+    RewriteQuery other = (RewriteQuery)obj;
+    return fieldName.equals(other.fieldName)
+	&& qf.equals(other.qf)
+	&& srndQuery.equals(other.srndQuery);
+  }
+
+  /** @throws UnsupportedOperationException */
+  @Override
+  public Object clone() {
+    throw new UnsupportedOperationException();
+  }
+}
+

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTerm.java

@@ -17,12 +17,9 @@ package org.apache.lucene.queryparser.surround.query;
  */
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
 
 import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.Term;
-import org.apache.lucene.search.BooleanClause;
 import org.apache.lucene.search.Query;
 
 public abstract class SimpleTerm
@@ -38,7 +35,11 @@ public abstract class SimpleTerm
   public String getFieldOperator() {return "/";}
   
   public abstract String toStringUnquoted();
-  
+
+  /** @deprecated (March 2011) Not normally used, to be removed from Lucene 4.0.
+   *   This class implementing Comparable is to be removed at the same time.
+   */
+  @Deprecated
   public int compareTo(SimpleTerm ost) {
     /* for ordering terms and prefixes before using an index, not used */
     return this.toStringUnquoted().compareTo( ost.toStringUnquoted());
@@ -70,35 +71,10 @@ public abstract class SimpleTerm
     void visitMatchingTerm(Term t)throws IOException;
   }
 
+  @Override
   public String distanceSubQueryNotAllowed() {return null;}
-
   
   @Override
-  public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) {
-    return new Query() {
-      @Override
-      public String toString(String fn) {
-        return getClass().toString() + " " + fieldName + " (" + fn + "?)";
-      }
-      
-      @Override
-      public Query rewrite(IndexReader reader) throws IOException {
-        final List<Query> luceneSubQueries = new ArrayList<Query>();
-        visitMatchingTerms( reader, fieldName,
-            new MatchingTermVisitor() {
-              public void visitMatchingTerm(Term term) throws IOException {
-                luceneSubQueries.add(qf.newTermQuery(term));
-              }
-            });
-        return  (luceneSubQueries.size() == 0) ? SrndQuery.theEmptyLcnQuery
-              : (luceneSubQueries.size() == 1) ? luceneSubQueries.get(0)
-              : SrndBooleanQuery.makeBooleanQuery(
-                  /* luceneSubQueries all have default weight */
-                  luceneSubQueries, BooleanClause.Occur.SHOULD); /* OR the subquery terms */ 
-      }
-    };
-  }
-    
   public void addSpanQueries(final SpanNearClauseFactory sncf) throws IOException {
     visitMatchingTerms(
           sncf.getIndexReader(),
@@ -109,6 +85,11 @@ public abstract class SimpleTerm
             }
           });
   }
+
+  @Override
+  public Query makeLuceneQueryFieldNoBoost(final String fieldName, final BasicQueryFactory qf) {
+    return new SimpleTermRewriteQuery(this, fieldName, qf);
+  }
 }
 
 

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SimpleTermRewriteQuery.java

@@ -0,0 +1,52 @@
+package org.apache.lucene.queryparser.surround.query;
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import java.io.IOException;
+import java.util.List;
+import java.util.ArrayList;
+
+import org.apache.lucene.index.IndexReader;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.BooleanClause;
+import org.apache.lucene.index.Term;
+
+class SimpleTermRewriteQuery extends RewriteQuery<SimpleTerm> {
+
+  SimpleTermRewriteQuery(
+      SimpleTerm srndQuery,
+      String fieldName,
+      BasicQueryFactory qf) {
+    super(srndQuery, fieldName, qf);
+  }
+
+  @Override
+  public Query rewrite(IndexReader reader) throws IOException {
+    final List<Query> luceneSubQueries = new ArrayList<Query>();
+    srndQuery.visitMatchingTerms(reader, fieldName,
+	  new SimpleTerm.MatchingTermVisitor() {
+	    public void visitMatchingTerm(Term term) throws IOException {
+	      luceneSubQueries.add(qf.newTermQuery(term));
+	    }
+	  });
+    return  (luceneSubQueries.size() == 0) ? SrndQuery.theEmptyLcnQuery
+	  : (luceneSubQueries.size() == 1) ? luceneSubQueries.get(0)
+	  : SrndBooleanQuery.makeBooleanQuery(
+	    /* luceneSubQueries all have default weight */
+	    luceneSubQueries, BooleanClause.Occur.SHOULD); /* OR the subquery terms */
+  }
+}
+

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SpanNearClauseFactory.java

@@ -64,7 +64,7 @@ import org.apache.lucene.search.spans.SpanQuery;
 import org.apache.lucene.search.spans.SpanTermQuery;
 
 
-public class SpanNearClauseFactory {
+public class SpanNearClauseFactory { // FIXME: rename to SpanClauseFactory
   public SpanNearClauseFactory(IndexReader reader, String fieldName, BasicQueryFactory qf) {
     this.reader = reader;
     this.fieldName = fieldName;
@@ -100,17 +100,16 @@ public class SpanNearClauseFactory {
     /* CHECKME: wrap in Hashable...? */
     addSpanQueryWeighted(stq, weight);
   }
-  
-  public void addSpanNearQuery(Query q) {
+
+  public void addSpanQuery(Query q) {
     if (q == SrndQuery.theEmptyLcnQuery)
       return;
-    if (! (q instanceof SpanNearQuery))
-      throw new AssertionError("Expected SpanNearQuery: " + q.toString(getFieldName()));
-    /* CHECKME: wrap in Hashable...? */
-    addSpanQueryWeighted((SpanNearQuery)q, q.getBoost());
+    if (! (q instanceof SpanQuery))
+      throw new AssertionError("Expected SpanQuery: " + q.toString(getFieldName()));
+    addSpanQueryWeighted((SpanQuery)q, q.getBoost());
   }
-  
-  public SpanQuery makeSpanNearClause() {
+
+  public SpanQuery makeSpanClause() {
     SpanQuery [] spanQueries = new SpanQuery[size()];
     Iterator<SpanQuery> sqi = weightBySpanQuery.keySet().iterator();
     int i = 0;

modules/queryparser/src/java/org/apache/lucene/queryparser/surround/query/SrndQuery.java

@@ -53,6 +53,9 @@ public abstract class SrndQuery implements Cloneable {
   
   public abstract Query makeLuceneQueryFieldNoBoost(String fieldName, BasicQueryFactory qf);
   
+  /** This method is used by {@link #hashCode()} and {@link #equals(Object)},
+   *  see LUCENE-2945.
+   */
   @Override
   public abstract String toString();
   
@@ -66,8 +69,32 @@ public abstract class SrndQuery implements Cloneable {
       throw new Error(cns);
     }
   }
-  
-/* An empty Lucene query */
+
+  /** For subclasses of {@link SrndQuery} within the package
+   *  {@link org.apache.lucene.queryparser.surround.query}
+   *  it is not necessary to override this method,
+   *  @see #toString().
+   */
+  @Override
+  public int hashCode() {
+    return getClass().hashCode() ^ toString().hashCode();
+  }
+
+  /** For subclasses of {@link SrndQuery} within the package
+   *  {@link org.apache.lucene.queryparser.surround.query}
+   *  it is not necessary to override this method,
+   *  @see #toString().
+   */
+  @Override
+  public boolean equals(Object obj) {
+    if (obj == null)
+      return false;
+    if (! getClass().equals(obj.getClass()))
+      return false;
+    return toString().equals(obj.toString());
+  }
+
+  /** An empty Lucene query */
   public final static Query theEmptyLcnQuery = new BooleanQuery() { /* no changes allowed */
     @Override
     public void setBoost(float boost) {

modules/queryparser/src/test/org/apache/lucene/queryparser/surround/query/SrndQueryTest.java

@@ -0,0 +1,52 @@
+package org.apache.lucene.queryparser.surround.query;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import junit.framework.Assert;
+
+import org.apache.lucene.queryparser.surround.parser.QueryParser;
+import org.apache.lucene.search.Query;
+import org.apache.lucene.search.QueryUtils;
+import org.apache.lucene.util.LuceneTestCase;
+import org.junit.Test;
+
+/**
+ *
+ *
+ **/
+public class SrndQueryTest extends LuceneTestCase {
+  
+  void checkEqualParsings(String s1, String s2) throws Exception {
+    String fieldName = "foo";
+    BasicQueryFactory qf = new BasicQueryFactory(16);
+    Query lq1, lq2;
+    lq1 = QueryParser.parse(s1).makeLuceneQueryField(fieldName, qf);
+    lq2 = QueryParser.parse(s2).makeLuceneQueryField(fieldName, qf);
+    QueryUtils.checkEqual(lq1, lq2);
+  }
+
+  @Test
+  public void testHashEquals() throws Exception {
+    //grab some sample queries from Test02Boolean and Test03Distance and
+    //check there hashes and equals
+    checkEqualParsings("word1 w word2", " word1  w  word2 ");
+    checkEqualParsings("2N(w1,w2,w3)", " 2N(w1, w2 , w3)");
+    checkEqualParsings("abc?", " abc? ");
+    checkEqualParsings("w*rd?", " w*rd?");
+  }
+}