Commit 587a6f639fd61ecb1fd41101d36c9a47ea144f91
Exists in
master
and in
21 other branches
Merge branch 'raise_dangerous_operations' into 1-0-take-2
Showing
9 changed files
with
61 additions
and
114 deletions
Show diff stats
.travis.yml
@@ -21,6 +21,7 @@ gemfile: | @@ -21,6 +21,7 @@ gemfile: | ||
21 | - test/gemfiles/mongoid2.gemfile | 21 | - test/gemfiles/mongoid2.gemfile |
22 | - test/gemfiles/mongoid3.gemfile | 22 | - test/gemfiles/mongoid3.gemfile |
23 | - test/gemfiles/mongoid4.gemfile | 23 | - test/gemfiles/mongoid4.gemfile |
24 | + - test/gemfiles/mongoid5.gemfile | ||
24 | matrix: | 25 | matrix: |
25 | include: | 26 | include: |
26 | - gemfile: test/gemfiles/nobrainer.gemfile | 27 | - gemfile: test/gemfiles/nobrainer.gemfile |
README.md
@@ -45,7 +45,7 @@ Add this line to your application’s Gemfile: | @@ -45,7 +45,7 @@ Add this line to your application’s Gemfile: | ||
45 | gem 'searchkick' | 45 | gem 'searchkick' |
46 | ``` | 46 | ``` |
47 | 47 | ||
48 | -For Elasticsearch 2.0, use version `0.9.2` or above. For Elasticsearch 0.90, use version `0.6.3` and [this readme](https://github.com/ankane/searchkick/blob/v0.6.3/README.md). | 48 | +For Elasticsearch 2.0, use the `elasticsearch2` branch and [this readme](https://github.com/ankane/searchkick/blob/elasticsearch2/README.md). For Elasticsearch 0.90, use version `0.6.3` and [this readme](https://github.com/ankane/searchkick/blob/v0.6.3/README.md). |
49 | 49 | ||
50 | Add searchkick to models you want to search. | 50 | Add searchkick to models you want to search. |
51 | 51 | ||
@@ -547,95 +547,11 @@ products = Product.search "peantu butta", suggest: true | @@ -547,95 +547,11 @@ products = Product.search "peantu butta", suggest: true | ||
547 | products.suggestions # ["peanut butter"] | 547 | products.suggestions # ["peanut butter"] |
548 | ``` | 548 | ``` |
549 | 549 | ||
550 | -### Aggregations | 550 | +### Facets |
551 | 551 | ||
552 | -[Aggregations](http://www.elasticsearch.org/guide/reference/api/search/facets/) provide aggregated search data. | 552 | +[Facets](http://www.elasticsearch.org/guide/reference/api/search/facets/) provide aggregated search data. |
553 | 553 | ||
554 | - | ||
555 | - | ||
556 | -```ruby | ||
557 | -products = Product.search "chuck taylor", aggs: [:product_type, :gender, :brand] | ||
558 | -products.aggs | ||
559 | -``` | ||
560 | - | ||
561 | -By default, `where` conditions apply to aggregations. | ||
562 | - | ||
563 | -```ruby | ||
564 | -Product.search "wingtips", where: {color: "brandy"}, aggs: [:size] | ||
565 | -# aggregations for brandy wingtips are returned | ||
566 | -``` | ||
567 | - | ||
568 | -Change this with: | ||
569 | - | ||
570 | -```ruby | ||
571 | -Product.search "wingtips", where: {color: "brandy"}, aggs: [:size], smart_aggs: false | ||
572 | -# aggregations for all wingtips are returned | ||
573 | -``` | ||
574 | - | ||
575 | -Set `where` conditions for each aggregation separately with: | ||
576 | - | ||
577 | -```ruby | ||
578 | -Product.search "wingtips", aggs: {size: {where: {color: "brandy"}}} | ||
579 | -``` | ||
580 | - | ||
581 | -Limit | ||
582 | - | ||
583 | -```ruby | ||
584 | -Product.search "apples", aggs: {store_id: {limit: 10}} | ||
585 | -``` | ||
586 | - | ||
587 | -#### Moving From Facets | ||
588 | - | ||
589 | -1. Replace `facets` with `aggs` in searches. **Note:** Range and stats facets are not supported at this time. | ||
590 | - | ||
591 | - ```ruby | ||
592 | - products = Product.search "chuck taylor", facets: [:brand] | ||
593 | - # to | ||
594 | - products = Product.search "chuck taylor", aggs: [:brand] | ||
595 | - ``` | ||
596 | - | ||
597 | -2. Replace the `facets` method with `aggs` for results. | ||
598 | - | ||
599 | - ```ruby | ||
600 | - products.facets | ||
601 | - # to | ||
602 | - products.aggs | ||
603 | - ``` | ||
604 | - | ||
605 | - The keys in results differ slightly. Instead of: | ||
606 | - | ||
607 | - ```json | ||
608 | - { | ||
609 | - "_type":"terms", | ||
610 | - "missing":0, | ||
611 | - "total":45, | ||
612 | - "other":34, | ||
613 | - "terms":[ | ||
614 | - {"term":14.0,"count":11} | ||
615 | - ] | ||
616 | - } | ||
617 | - ``` | ||
618 | - | ||
619 | - You get: | ||
620 | - | ||
621 | - ```json | ||
622 | - { | ||
623 | - "doc_count":45, | ||
624 | - "doc_count_error_upper_bound":0, | ||
625 | - "sum_other_doc_count":34, | ||
626 | - "buckets":[ | ||
627 | - {"key":14.0,"doc_count":11} | ||
628 | - ] | ||
629 | - } | ||
630 | - ``` | ||
631 | - | ||
632 | - Update your application to handle this. | ||
633 | - | ||
634 | -3. By default, `where` conditions apply to aggregations. This is equivalent to `smart_facets: true`. If you have `smart_facets: true`, you can remove it. If this is not desired, set `smart_aggs: false`. | ||
635 | - | ||
636 | -### Facets [deprecated] | ||
637 | - | ||
638 | -Facets have been deprecated in favor of aggregations as of Searchkick 0.9.2. See [how to upgrade](#moving-from-facets). | 554 | + |
639 | 555 | ||
640 | ```ruby | 556 | ```ruby |
641 | products = Product.search "chuck taylor", facets: [:product_type, :gender, :brand] | 557 | products = Product.search "chuck taylor", facets: [:product_type, :gender, :brand] |
@@ -777,8 +693,6 @@ City.search "san", boost_by_distance: {field: :location, origin: [37, -122], fun | @@ -777,8 +693,6 @@ City.search "san", boost_by_distance: {field: :location, origin: [37, -122], fun | ||
777 | 693 | ||
778 | Searchkick supports [Elasticsearch’s routing feature](https://www.elastic.co/blog/customizing-your-document-routing). | 694 | Searchkick supports [Elasticsearch’s routing feature](https://www.elastic.co/blog/customizing-your-document-routing). |
779 | 695 | ||
780 | -**Note:** Routing is not yet supported for Elasticsearch 2.0. | ||
781 | - | ||
782 | ```ruby | 696 | ```ruby |
783 | class Contact < ActiveRecord::Base | 697 | class Contact < ActiveRecord::Base |
784 | searchkick routing: :user_id | 698 | searchkick routing: :user_id |
lib/searchkick.rb
@@ -22,6 +22,7 @@ module Searchkick | @@ -22,6 +22,7 @@ module Searchkick | ||
22 | class MissingIndexError < StandardError; end | 22 | class MissingIndexError < StandardError; end |
23 | class UnsupportedVersionError < StandardError; end | 23 | class UnsupportedVersionError < StandardError; end |
24 | class InvalidQueryError < Elasticsearch::Transport::Transport::Errors::BadRequest; end | 24 | class InvalidQueryError < Elasticsearch::Transport::Transport::Errors::BadRequest; end |
25 | + class DangerousOperation < StandardError; end | ||
25 | 26 | ||
26 | class << self | 27 | class << self |
27 | attr_accessor :search_method_name | 28 | attr_accessor :search_method_name |
lib/searchkick/model.rb
@@ -42,6 +42,12 @@ module Searchkick | @@ -42,6 +42,12 @@ module Searchkick | ||
42 | end | 42 | end |
43 | 43 | ||
44 | def searchkick_reindex(options = {}) | 44 | def searchkick_reindex(options = {}) |
45 | + unless options[:accept_danger] | ||
46 | + if (respond_to?(:current_scope) && respond_to?(:default_scoped) && current_scope && current_scope.to_sql != default_scoped.to_sql) || | ||
47 | + (respond_to?(:queryable) && queryable != unscoped.with_default_scope) | ||
48 | + raise Searchkick::DangerousOperation, "Only call reindex on models, not relations. Pass `accept_danger: true` if this is your intention." | ||
49 | + end | ||
50 | + end | ||
45 | searchkick_index.reindex_scope(searchkick_klass, options) | 51 | searchkick_index.reindex_scope(searchkick_klass, options) |
46 | end | 52 | end |
47 | alias_method :reindex, :searchkick_reindex unless method_defined?(:reindex) | 53 | alias_method :reindex, :searchkick_reindex unless method_defined?(:reindex) |
lib/searchkick/query.rb
@@ -354,9 +354,9 @@ module Searchkick | @@ -354,9 +354,9 @@ module Searchkick | ||
354 | } | 354 | } |
355 | } | 355 | } |
356 | 356 | ||
357 | - where = {} | ||
358 | - where = (options[:where] || {}).reject { |k| k == field } unless options[:smart_aggs] == false | ||
359 | - agg_filters = where_filters(where.merge(agg_options[:where] || {})) | 357 | + agg_options.deep_merge!(where: options.fetch(:where, {}).reject { |k| k == field }) if options[:smart_aggs] == true |
358 | + agg_filters = where_filters(agg_options[:where]) | ||
359 | + | ||
360 | if agg_filters.any? | 360 | if agg_filters.any? |
361 | payload[:aggs][field] = { | 361 | payload[:aggs][field] = { |
362 | filter: { | 362 | filter: { |
test/aggs_test.rb
@@ -32,34 +32,34 @@ class AggsTest < Minitest::Test | @@ -32,34 +32,34 @@ class AggsTest < Minitest::Test | ||
32 | assert_equal(1, agg["sum_other_doc_count"]) if Gem::Version.new(Searchkick.server_version) >= Gem::Version.new("1.4.0") | 32 | assert_equal(1, agg["sum_other_doc_count"]) if Gem::Version.new(Searchkick.server_version) >= Gem::Version.new("1.4.0") |
33 | end | 33 | end |
34 | 34 | ||
35 | - def test_query_where | ||
36 | - assert_equal ({1 => 1}), store_agg(where: {in_stock: true}, aggs: [:store_id]) | 35 | + def test_where_no_smart_aggs |
36 | + assert_equal ({2 => 2}), store_agg(where: {color: "red"}, aggs: {store_id: {where: {in_stock: false}}}) | ||
37 | + assert_equal ({2 => 2}), store_agg(where: {color: "blue"}, aggs: {store_id: {where: {in_stock: false}}}) | ||
37 | end | 38 | end |
38 | 39 | ||
39 | - def test_two_wheres | ||
40 | - assert_equal ({2 => 1}), store_agg(where: {color: "red"}, aggs: {store_id: {where: {in_stock: false}}}) | 40 | + def test_smart_aggs |
41 | + assert_equal ({1 => 1}), store_agg(where: {in_stock: true}, aggs: [:store_id], smart_aggs: true) | ||
41 | end | 42 | end |
42 | 43 | ||
43 | - def test_where_override | ||
44 | - assert_equal ({}), store_agg(where: {color: "red"}, aggs: {store_id: {where: {in_stock: false, color: "blue"}}}) | ||
45 | - assert_equal ({2 => 1}), store_agg(where: {color: "blue"}, aggs: {store_id: {where: {in_stock: false, color: "red"}}}) | 44 | + def test_smart_aggs_where |
45 | + assert_equal ({2 => 1}), store_agg(where: {color: "red"}, aggs: {store_id: {where: {in_stock: false}}}, smart_aggs: true) | ||
46 | end | 46 | end |
47 | 47 | ||
48 | - def test_skip | ||
49 | - assert_equal ({1 => 1, 2 => 2}), store_agg(where: {store_id: 2}, aggs: [:store_id]) | 48 | + def test_smart_aggs_where_override |
49 | + assert_equal ({2 => 1}), store_agg(where: {color: "red"}, aggs: {store_id: {where: {in_stock: false, color: "blue"}}}, smart_aggs: true) | ||
50 | + assert_equal ({}), store_agg(where: {color: "blue"}, aggs: {store_id: {where: {in_stock: false, color: "red"}}}, smart_aggs: true) | ||
50 | end | 51 | end |
51 | 52 | ||
52 | - def test_skip_complex | ||
53 | - assert_equal ({1 => 1, 2 => 1}), store_agg(where: {store_id: 2, price: {gt: 5}}, aggs: [:store_id]) | 53 | + def test_smart_aggs_skip_agg |
54 | + assert_equal ({1 => 1, 2 => 2}), store_agg(where: {store_id: 2}, aggs: [:store_id], smart_aggs: true) | ||
54 | end | 55 | end |
55 | 56 | ||
56 | - def test_multiple | ||
57 | - assert_equal ({"store_id" => {1 => 1, 2 => 2}, "color" => {"blue" => 1, "green" => 1, "red" => 1}}), store_multiple_aggs(aggs: [:store_id, :color]) | 57 | + def test_smart_aggs_skip_agg_complex |
58 | + assert_equal ({1 => 1, 2 => 1}), store_agg(where: {store_id: 2, price: {gt: 5}}, aggs: [:store_id], smart_aggs: true) | ||
58 | end | 59 | end |
59 | 60 | ||
60 | - def test_smart_aggs_false | ||
61 | - assert_equal ({2 => 2}), store_agg(where: {color: "red"}, aggs: {store_id: {where: {in_stock: false}}}, smart_aggs: false) | ||
62 | - assert_equal ({2 => 2}), store_agg(where: {color: "blue"}, aggs: {store_id: {where: {in_stock: false}}}, smart_aggs: false) | 61 | + def test_multiple_aggs |
62 | + assert_equal ({"store_id" => {1 => 1, 2 => 2}, "color" => {"blue" => 1, "green" => 1, "red" => 1}}), store_multiple_aggs(aggs: [:store_id, :color]) | ||
63 | end | 63 | end |
64 | 64 | ||
65 | protected | 65 | protected |
test/index_test.rb
@@ -102,6 +102,18 @@ class IndexTest < Minitest::Test | @@ -102,6 +102,18 @@ class IndexTest < Minitest::Test | ||
102 | assert_raises(Searchkick::InvalidQueryError) { Product.search(query: {boom: true}) } | 102 | assert_raises(Searchkick::InvalidQueryError) { Product.search(query: {boom: true}) } |
103 | end | 103 | end |
104 | 104 | ||
105 | + def test_dangerous_reindex | ||
106 | + skip if mongoid2? || nobrainer? | ||
107 | + assert_raises(Searchkick::DangerousOperation) { Product.where(id: [1, 2, 3]).reindex } | ||
108 | + end | ||
109 | + | ||
110 | + def test_dangerous_reindex_accepted | ||
111 | + skip if nobrainer? | ||
112 | + store_names ["Product A", "Product B"] | ||
113 | + Product.where(name: "Product A").reindex(accept_danger: true) | ||
114 | + assert_search "product", ["Product A"] | ||
115 | + end | ||
116 | + | ||
105 | if defined?(ActiveRecord) | 117 | if defined?(ActiveRecord) |
106 | 118 | ||
107 | def test_transaction | 119 | def test_transaction |
@@ -110,7 +122,7 @@ class IndexTest < Minitest::Test | @@ -110,7 +122,7 @@ class IndexTest < Minitest::Test | ||
110 | raise ActiveRecord::Rollback | 122 | raise ActiveRecord::Rollback |
111 | end | 123 | end |
112 | 124 | ||
113 | - assert_search "product", [] | 125 | + assert_search "product", [], conversions: false |
114 | end | 126 | end |
115 | 127 | ||
116 | end | 128 | end |
test/test_helper.rb
@@ -22,11 +22,17 @@ def elasticsearch2? | @@ -22,11 +22,17 @@ def elasticsearch2? | ||
22 | Searchkick.server_version.starts_with?("2.") | 22 | Searchkick.server_version.starts_with?("2.") |
23 | end | 23 | end |
24 | 24 | ||
25 | -if defined?(Mongoid) | 25 | +def mongoid2? |
26 | + defined?(Mongoid) && Mongoid::VERSION.starts_with?("2.") | ||
27 | +end | ||
26 | 28 | ||
27 | - def mongoid2? | ||
28 | - Mongoid::VERSION.starts_with?("2.") | ||
29 | - end | 29 | +def nobrainer? |
30 | + defined?(NoBrainer) | ||
31 | +end | ||
32 | + | ||
33 | +if defined?(Mongoid) | ||
34 | + Mongoid.logger.level = Logger::INFO | ||
35 | + Mongo::Logger.logger.level = Logger::INFO if defined?(Mongo::Logger) | ||
30 | 36 | ||
31 | if mongoid2? | 37 | if mongoid2? |
32 | # enable comparison of BSON::ObjectIds | 38 | # enable comparison of BSON::ObjectIds |