ankane readme and test improvements, other tidying up

will-r
1 parent 40bee0a0
Showing 4 changed files with 38 additions and 61 deletions Show diff stats
README.md
lib/searchkick/query.rb
test/geo_shape_test.rb
test/test_helper.rb
@@ -834,7 +834,7 @@ product = Product.first
 product.similar(fields: ["name"], where: {size: "12 oz"})
 ```
-### Simple Geospatial Indexing: geo_points
+### Geospatial Searches
 If your data consists of point values, searchkick offers a useful shorthand:
@@ -883,7 +883,7 @@ City.search &quot;san&quot;, boost_by_distance: {field: :location, origin: {lat: 37, lon: 
 ```
-### Complex Geospatial Indexing: geo_shapes
+### Geo Shapes
 You can also pass through complex or varied shapes as GeoJSON objects.
@@ -911,28 +911,23 @@ end
 The `geo_shapes` hash is passed through to elasticsearch without modification. Please see the [geo_shape data type documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-shape.html) for options.
-Any geospatial data type can be used in the index or in the search. It is up to you to ensure that it is a valid geoJSON representation. The possible shapes are:
+Any geospatial data type can be held in the index or give as a search query. It is up to you to ensure that it is a valid geoJSON representation. The possible shapes are:
 * **point**: single lat/lon pair
 * **multipoint**: array of points
 * **linestring**: array of at least two lat/lon pairs
 * **multilinestring**: array of lines
-* **polygon**: an array of paths, each being an array of at least four lat/lon pairs whose first and last points are the same. Paths after the first represent exclusions.
+* **polygon**: an array of paths, each being an array of at least four lat/lon pairs whose first and last points are the same. Paths after the first represent exclusions. Elasticsearch will return an error if a polygon contains two consecutive identical points, intersects itself or is not closed.
 * **multipolygon**: array of polygons
 * **envelope**: a bounding box defined by top left and bottom right points
 * **circle**: a bounding circle defined by center point and radius
 * **geometrycollection**: an array of separate geoJSON objects possibly of various types
-See the [Elasticsearch documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-shape.html) for details. GeoJSON coordinates are usually given as an array of `[lon, lat]` points but this often causes swapping errors so searchkick can also take objects with `lon` and `lat` keys.
+See the [Elasticsearch documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/geo-shape.html) for details. GeoJSON coordinates are usually given as an array of `[lon, lat]` points but searchkick can also take objects with `lon` and `lat` keys.
-Elasticsearch is sensitive about geo_shape validity. For example it will throw an exception if a polygon contains two consecutive identical points, intersects itself or is not properly closed.
+Once a geo_shape index is established, you can include a geo_shape filter in any search. This also takes a geoJSON shape, and will return a list of items based on their overlap with that shape.
-
-### Geospatial searching
-
-Once a geo_shape index is established, you can apply a geo_shape filter to any search. This also takes a geoJSON shape, and will return a list of items based on their overlap with that shape.
-
-Intersecting with the query shape:
+Find shapes (of any kind) intersecting with the query shape:
 ```ruby
 City.search "san", where: {bounds: {geo_shape: {type: "polygon", coordinates: [[{lat: 38, lon: -123}, ...]]}}}
@@ -941,22 +936,21 @@ City.search &quot;san&quot;, where: {bounds: {geo_shape: {type: &quot;polygon&quot;, coordinates: [[
 Falling entirely within the query shape:
 ```ruby
-City.search "san", where: {relation: "within", bounds: {geo_shape: {type: "circle", coordinates: [{lat: 38, lon: -123}], radius: "1km"}}}
+City.search "san", where: {bounds: {geo_shape: {type: "circle", relation: "within", coordinates: [{lat: 38, lon: -123}], radius: "1km"}}}
 ```
 Not touching the query shape:
 ```ruby
-City.search "san", where: {relation: "disjoint", bounds: {geo_shape: {type: "envelope", coordinates: [{lat: 38, lon: -123}, {lat: 37, lon: -122}]}}}
+City.search "san", where: {bounds: {geo_shape: {type: "envelope", relation: "disjoint", coordinates: [{lat: 38, lon: -123}, {lat: 37, lon: -122}]}}}
 ```
-Envelope is a special case. For consistency, searchkick also understands coordinates given as top_left and bottom_right:
+Containing the query shape (ElasticSearch 2.2+):
 ```ruby
-City.search "san", where: {relation: "within", bounds: {geo_shape: {type: "envelope", top_left: {lat: 38, lon: -123}, bottom_right: {lat: 37, lon: -122}}}}
+City.search "san", where: {bounds: {geo_shape: {type: "envelope", relation: "contains", coordinates: [{lat: 38, lon: -123}, {lat: 37, lon: -122}]}}}
 ```
-
 ### Routing
 Searchkick supports [Elasticsearch’s routing feature](https://www.elastic.co/blog/customizing-your-document-routing).
@@ -809,15 +809,7 @@ module Searchkick
                   }
                 }
               when :geo_shape
-                if op_value[:type] == "envelope" && op_value[:top_left].present? && op_value[:bottom_right].present?
-                  op_value[:coordinates] = [coordinate_array(op_value[:top_left]), coordinate_array(op_value[:bottom_right])]
-                  op_value.delete(:top_left)
-                  op_value.delete(:bottom_right)
-                elsif op_value[:type] == "circle"
-                  op_value[:coordinates] = coordinate_array(op_value[:coordinates] || [])
-                else
-                  op_value[:coordinates] = (op_value[:coordinates] || []).map { |loc| coordinate_array(loc) }
-                end
+                op_value[:coordinates] = coordinate_array(op_value[:coordinates]) if op_value[:coordinates]
                 relation = op_value.delete(:relation) || 'intersects'
                 filters << {
                   geo_shape: {
@@ -943,9 +935,7 @@ module Searchkick
     end
     # Recursively descend through nesting of arrays until we reach either a lat/lon object or an array of numbers,
-    # eventually returning the same structure with all values transformed to [lon, lat]. Question: should we reverse
-    # the array order so that arguments can be given as [lat, lon], as happens elsewhere in searchkick? We are moving
-    # GeoJSON around so it seems better to stick to that specification, though the lat/lon objects are already a deviation.
+    # eventually returning the same structure with all values transformed to [lon, lat].
     #
     def coordinate_array(value)
       if value.is_a?(Hash)
-require "pp"
 require_relative "test_helper"
 class GeoShapeTest < Minitest::Test
-
-  def test_geo_shape
-    regions = [
+  def setup
+    super
+    store [
       {name: "Region A", text: "The witch had a cat", territory: "30,40,35,45,40,40,40,30,30,30,30,40"},
       {name: "Region B", text: "and a very tall hat", territory: "50,60,55,65,60,60,60,50,50,50,50,60"},
-      {name: "Region C", text: "and long ginger hair which she wore in a plait.",  territory: "10,20,15,25,20,20,20,10,10,10,10,20"},
-    ]
-    store regions, Region
+      {name: "Region C", text: "and long ginger hair which she wore in a plait",  territory: "10,20,15,25,20,20,20,10,10,10,10,20"}
+    ], Region
+  end
-    # circle
+  def test_circle
     assert_search "*", ["Region A"], {
       where: {
         territory: {
@@ -23,8 +22,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
-    # envelope
+  def test_envelope
     assert_search "*", ["Region A"], {
       where: {
         territory: {
@@ -35,21 +35,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
-    # envelope as corners
-    assert_search "*", ["Region A"], {
-      where: {
-        territory: {
-          geo_shape: {
-            type: "envelope",
-            top_left: {lat: 42.0, lon: 28.0},
-            bottom_right: {lat: 38.0, lon: 32.0}
-          }
-        }
-      }
-    }, Region
-
-    # polygon
+  def test_polygon
     assert_search "*", ["Region A"], {
       where: {
         territory: {
@@ -60,8 +48,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
-    # multipolygon
+  def test_multipolygon
     assert_search "*", ["Region A", "Region B"], {
       where: {
         territory: {
@@ -75,8 +64,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
-    # disjoint
+  def test_disjoint
     assert_search "*", ["Region B", "Region C"], {
       where: {
         territory: {
@@ -88,8 +78,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
-    # within
+  def test_within
     assert_search "*", ["Region A"], {
       where: {
         territory: {
@@ -101,8 +92,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
-    # with search
+  def test_search_math
     assert_search "witch", ["Region A"], {
       where: {
         territory: {
@@ -113,7 +105,9 @@ class GeoShapeTest &lt; Minitest::Test
         }
       }
     }, Region
+  end
+  def test_search_no_match
     assert_search "ginger hair", [], {
       where: {
         territory: {
@@ -126,20 +120,18 @@ class GeoShapeTest &lt; Minitest::Test
     }, Region
   end
-  def test_geo_shape_contains
+  def test_contains
     skip if elasticsearch_below22?
-
-    assert_search "*", ["Region A"], {
+    assert_search "*", ["Region C"], {
       where: {
         territory: {
           geo_shape: {
             type: "envelope",
             relation: "contains",
-            coordinates: [[32, 33], [33, 32]]
+            coordinates: [[12, 13], [13,12]]
           }
         }
       }
     }, Region
-
   end
 end
@@ -435,6 +435,7 @@ class Minitest::Test
     Store.destroy_all
     Animal.destroy_all
     Speaker.destroy_all
+    Region.destroy_all
   end
   protected
	@@ -435,6 +435,7 @@ class Minitest::Test		@@ -435,6 +435,7 @@ class Minitest::Test
435	Store.destroy_all	435	Store.destroy_all
436	Animal.destroy_all	436	Animal.destroy_all
437	Speaker.destroy_all	437	Speaker.destroy_all
		438	+ Region.destroy_all
438	end	439	end
439		440
440	protected	441	protected