docs/attachments/LUCENE-4157/LUCENE-4157_Improve_TermQueryPrefixTreeStrategy_and_move_makeQuery_impl_to_SpatialStrategy.patch

Index: lucene/spatial/src/java/org/apache/lucene/spatial/SpatialStrategy.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lucene/spatial/src/java/org/apache/lucene/spatial/SpatialStrategy.java	(revision 1353813)
+++ lucene/spatial/src/java/org/apache/lucene/spatial/SpatialStrategy.java	(revision )
@@ -21,8 +21,10 @@
 import com.spatial4j.core.query.SpatialArgs;
 import com.spatial4j.core.shape.Shape;
 import org.apache.lucene.index.IndexableField;
+import org.apache.lucene.queries.function.FunctionQuery;
 import org.apache.lucene.queries.function.ValueSource;
 import org.apache.lucene.search.Filter;
+import org.apache.lucene.search.FilteredQuery;
 import org.apache.lucene.search.Query;
 
 /**
@@ -60,13 +62,25 @@
     return new IndexableField[] { createField(fieldInfo, shape, index, store) };
   }
 
+  /**
+   * The value source yields a number that is proportional to the distance between the query shape and indexed data.
+   * @param args
+   * @param fieldInfo
+   * @return
+   */
   public abstract ValueSource makeValueSource(SpatialArgs args, T fieldInfo);
 
   /**
-   * Make a query
+   * Make a query which has a score based on the distance from the data to the query shape.
+   * The default implementation constructs a {@link FilteredQuery} based on
+   * {@link #makeFilter(com.spatial4j.core.query.SpatialArgs, SpatialFieldInfo)} and
+   * {@link #makeValueSource(com.spatial4j.core.query.SpatialArgs, SpatialFieldInfo)}.
    */
-  public abstract Query makeQuery(SpatialArgs args, T fieldInfo);
-
+  public Query makeQuery(SpatialArgs args, T fieldInfo) {
+    Filter filter = makeFilter(args, fieldInfo);
+    ValueSource vs = makeValueSource(args, fieldInfo);
+    return new FilteredQuery(new FunctionQuery(vs), filter);
+  }
   /**
    * Make a Filter
    */
Index: lucene/spatial/src/java/org/apache/lucene/spatial/prefix/TermQueryPrefixTreeStrategy.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lucene/spatial/src/java/org/apache/lucene/spatial/prefix/TermQueryPrefixTreeStrategy.java	(revision 1353813)
+++ lucene/spatial/src/java/org/apache/lucene/spatial/prefix/TermQueryPrefixTreeStrategy.java	(revision )
@@ -22,13 +22,18 @@
 import com.spatial4j.core.query.SpatialOperation;
 import com.spatial4j.core.shape.Shape;
 import org.apache.lucene.index.Term;
-import org.apache.lucene.search.*;
+import org.apache.lucene.queries.TermsFilter;
+import org.apache.lucene.search.Filter;
 import org.apache.lucene.spatial.SimpleSpatialFieldInfo;
 import org.apache.lucene.spatial.prefix.tree.Node;
 import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree;
 
 import java.util.List;
 
+/**
+ * A basic implementation using a large {@link TermsFilter} of all the nodes from
+ * {@link SpatialPrefixTree#getNodes(com.spatial4j.core.shape.Shape, int, boolean)}.
+ */
 public class TermQueryPrefixTreeStrategy extends PrefixTreeStrategy {
 
   public TermQueryPrefixTreeStrategy(SpatialPrefixTree grid) {
@@ -37,26 +42,18 @@
 
   @Override
   public Filter makeFilter(SpatialArgs args, SimpleSpatialFieldInfo fieldInfo) {
-    return new QueryWrapperFilter( makeQuery(args, fieldInfo) );
-  }
+    final SpatialOperation op = args.getOperation();
+    if (! SpatialOperation.is(op, SpatialOperation.IsWithin, SpatialOperation.Intersects, SpatialOperation.BBoxWithin, SpatialOperation.BBoxIntersects))
+      throw new UnsupportedSpatialOperation(op);
 
-  @Override
-  public Query makeQuery(SpatialArgs args, SimpleSpatialFieldInfo fieldInfo) {
-    if (args.getOperation() != SpatialOperation.Intersects &&
-        args.getOperation() != SpatialOperation.IsWithin &&
-        args.getOperation() != SpatialOperation.Overlaps ){
-      // TODO -- can translate these other query types
-      throw new UnsupportedSpatialOperation(args.getOperation());
-    }
-    Shape qshape = args.getShape();
-    int detailLevel = grid.getMaxLevelForPrecision(qshape, args.getDistPrecision());
-    List<Node> cells = grid.getNodes(qshape, detailLevel, false);
-
-    BooleanQuery booleanQuery = new BooleanQuery();
+    Shape shape = args.getShape();
+    int detailLevel = grid.getMaxLevelForPrecision(shape, args.getDistPrecision());
+    List<Node> cells = grid.getNodes(shape, detailLevel, false);
+    TermsFilter filter = new TermsFilter();
     for (Node cell : cells) {
-      booleanQuery.add(new TermQuery(new Term(fieldInfo.getFieldName(), cell.getTokenString())), BooleanClause.Occur.SHOULD);
+      filter.addTerm(new Term(fieldInfo.getFieldName(), cell.getTokenString()));
     }
-    return booleanQuery;
+    return filter;
   }
 
 }
Index: lucene/spatial/src/java/org/apache/lucene/spatial/prefix/RecursivePrefixTreeStrategy.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lucene/spatial/src/java/org/apache/lucene/spatial/prefix/RecursivePrefixTreeStrategy.java	(revision 1353813)
+++ lucene/spatial/src/java/org/apache/lucene/spatial/prefix/RecursivePrefixTreeStrategy.java	(revision )
@@ -21,15 +21,13 @@
 import com.spatial4j.core.query.SpatialArgs;
 import com.spatial4j.core.query.SpatialOperation;
 import com.spatial4j.core.shape.Shape;
-import org.apache.lucene.queries.function.FunctionQuery;
-import org.apache.lucene.queries.function.ValueSource;
 import org.apache.lucene.search.Filter;
-import org.apache.lucene.search.FilteredQuery;
-import org.apache.lucene.search.Query;
 import org.apache.lucene.spatial.SimpleSpatialFieldInfo;
 import org.apache.lucene.spatial.prefix.tree.SpatialPrefixTree;
 
-
+/**
+ * Based on {@link RecursivePrefixTreeFilter}.
+ */
 public class RecursivePrefixTreeStrategy extends PrefixTreeStrategy {
 
   private int prefixGridScanLevel;//TODO how is this customized?
@@ -49,25 +47,17 @@
   }
 
   @Override
-  public Query makeQuery(SpatialArgs args, SimpleSpatialFieldInfo fieldInfo) {
-    Filter f = makeFilter(args, fieldInfo);
-
-    ValueSource vs = makeValueSource(args, fieldInfo);
-    return new FilteredQuery( new FunctionQuery(vs), f );
-  }
-
-  @Override
   public Filter makeFilter(SpatialArgs args, SimpleSpatialFieldInfo fieldInfo) {
     final SpatialOperation op = args.getOperation();
-    if (! SpatialOperation.is(op, SpatialOperation.IsWithin, SpatialOperation.Intersects, SpatialOperation.BBoxWithin))
+    if (! SpatialOperation.is(op, SpatialOperation.IsWithin, SpatialOperation.Intersects, SpatialOperation.BBoxWithin, SpatialOperation.BBoxIntersects))
       throw new UnsupportedSpatialOperation(op);
 
-    Shape qshape = args.getShape();
+    Shape shape = args.getShape();
 
-    int detailLevel = grid.getMaxLevelForPrecision(qshape,args.getDistPrecision());
+    int detailLevel = grid.getMaxLevelForPrecision(shape,args.getDistPrecision());
 
     return new RecursivePrefixTreeFilter(
-        fieldInfo.getFieldName(), grid,qshape, prefixGridScanLevel, detailLevel);
+        fieldInfo.getFieldName(), grid,shape, prefixGridScanLevel, detailLevel);
   }
 }