From 26acabce515f36ca3b3ff332d4b5303356a6d64b Mon Sep 17 00:00:00 2001 From: Molly Struve Date: Thu, 9 Apr 2020 17:30:27 -0500 Subject: [PATCH] [deploy] add custom sorting and scoring for feed content (#7199) --- app/services/search/feed_content.rb | 1 + .../search/query_builders/feed_content.rb | 64 +++++++++++++++++-- .../search/query_builders/query_base.rb | 20 +++--- spec/services/search/feed_content_spec.rb | 20 +++++- .../query_builders/feed_content_spec.rb | 31 ++++++--- 5 files changed, 112 insertions(+), 24 deletions(-) diff --git a/app/services/search/feed_content.rb b/app/services/search/feed_content.rb index 327973c7a..970d12e2a 100644 --- a/app/services/search/feed_content.rb +++ b/app/services/search/feed_content.rb @@ -44,6 +44,7 @@ module Search "image_url" => source["main_image"], "title" => source["title"] } + source["_score"] = hit["_score"] source.merge(timestamps_hash(hit)) end diff --git a/app/services/search/query_builders/feed_content.rb b/app/services/search/query_builders/feed_content.rb index fba9be355..19acd8406 100644 --- a/app/services/search/query_builders/feed_content.rb +++ b/app/services/search/query_builders/feed_content.rb @@ -4,9 +4,10 @@ module Search # In order for highlighting to work properly we have to search the fields we want to highlight QUERY_KEYS = { search_fields: [ - "tags.*", - "body_text", - "title", + "tags.keywords_for_search", + "tags.name^3", + "body_text^2", + "title^6", "user.name", "user.username", "organization.name", @@ -28,8 +29,12 @@ module Search ].freeze DEFAULT_PARAMS = { - sort_by: "hotness_score", - sort_direction: "desc", + sort: [ + :_score, + { score: "desc" }, + { hotness_score: "desc" }, + { comments_count: "desc" }, + ], size: 0 }.freeze @@ -64,10 +69,17 @@ module Search @params[:published] = true build_body + add_function_scoring unless sort_params_present? end private + def add_sort + return @body[:sort] = DEFAULT_PARAMS[:sort] unless sort_params_present? + + @body[:sort] = { @params[:sort_by] => @params[:sort_direction] } + end + def add_highlight_fields highlight_fields = { fields: {} } HIGHLIGHT_FIELDS.each do |field_name| @@ -124,6 +136,48 @@ module Search { range: { range_key => @params[range_key] } } end.compact end + + def query_hash(key, fields) + { + simple_query_string: { + query: key, + fields: fields, + lenient: true, + analyze_wildcard: true, + minimum_should_match: 2 + } + } + end + + def add_function_scoring + @body[:query] = { function_score: { query: @body[:query] } } + @body[:query][:function_score][:functions] = scoring_functions + end + + def scoring_functions + [ + scoring_filter(1000, 2.5), + scoring_filter(500, 2), + scoring_filter(100, 1.5), + ] + end + + def scoring_filter(score, weight) + { + filter: { + range: { + score: { + gte: score + } + } + }, + weight: weight + } + end + + def sort_params_present? + @params[:sort_by] && @params[:sort_direction] + end end end end diff --git a/app/services/search/query_builders/query_base.rb b/app/services/search/query_builders/query_base.rb index 624640a12..c9576c929 100644 --- a/app/services/search/query_builders/query_base.rb +++ b/app/services/search/query_builders/query_base.rb @@ -45,16 +45,20 @@ module Search fields = query_fields.presence || [query_key] - { - simple_query_string: { - query: "#{@params[query_key]}*", - fields: fields, - lenient: true, - analyze_wildcard: true - } - } + query_hash(@params[query_key], fields) end.compact end + + def query_hash(key, fields) + { + simple_query_string: { + query: "#{key}*", + fields: fields, + lenient: true, + analyze_wildcard: true + } + } + end end end end diff --git a/spec/services/search/feed_content_spec.rb b/spec/services/search/feed_content_spec.rb index e30daf686..b6eac22bd 100644 --- a/spec/services/search/feed_content_spec.rb +++ b/spec/services/search/feed_content_spec.rb @@ -20,14 +20,14 @@ RSpec.describe Search::FeedContent, type: :service do it "returns highlighted fields" do allow(article1).to receive(:body_text).and_return("I love ruby") - allow(article2).to receive(:body_text).and_return("Ruby Tuesday is yummy") + allow(article2).to receive(:body_text).and_return("Ruby Tuesday is love") index_documents([article1, article2]) query_params = { size: 5, search_fields: "love ruby" } feed_docs = described_class.search_documents(params: query_params) expect(feed_docs.count).to eq(2) doc_highlights = feed_docs.map { |t| t.dig("highlight", "body_text") }.flatten - expect(doc_highlights).to include("I love ruby", "Ruby Tuesday is yummy") + expect(doc_highlights).to include("I love ruby", "Ruby Tuesday is love") end it "returns fields necessary for the view" do @@ -122,6 +122,22 @@ RSpec.describe Search::FeedContent, type: :service do expect(doc_ids).to include(article2.id) end end + + context "with default sorting" do + it "sorts by Elasticsearch _score which is weighted based on article score" do + ruby_tag = create(:tag, name: "ruby") + allow(article1).to receive(:score).and_return(200) + article1.tags << ruby_tag + allow(article2).to receive(:score).and_return(1500) + article2.tags << ruby_tag + index_documents([article1, article2]) + query_params = { size: 5, search_fields: "ruby" } + + feed_docs = described_class.search_documents(params: query_params) + doc_ids = feed_docs.map { |t| t.dig("id") } + expect(doc_ids).to eq([article2.id, article1.id]) + end + end end describe "document counts" do diff --git a/spec/services/search/query_builders/feed_content_spec.rb b/spec/services/search/query_builders/feed_content_spec.rb index 455bf5b78..562b1f238 100644 --- a/spec/services/search/query_builders/feed_content_spec.rb +++ b/spec/services/search/query_builders/feed_content_spec.rb @@ -27,10 +27,14 @@ RSpec.describe Search::QueryBuilders::FeedContent, type: :service do filter = described_class.new(params) exepcted_query = [{ "simple_query_string" => { - "query" => "test*", "fields" => query_fields, "lenient" => true, "analyze_wildcard" => true + "query" => "test", + "fields" => query_fields, + "lenient" => true, + "analyze_wildcard" => true, + "minimum_should_match" => 2 } }] - expect(filter.as_hash.dig("query", "bool", "must")).to match_array(exepcted_query) + expect(search_bool_clause(filter)["must"]).to match_array(exepcted_query) end it "applies TERM_KEYS from params" do @@ -43,7 +47,7 @@ RSpec.describe Search::QueryBuilders::FeedContent, type: :service do { "terms" => { "class_name" => ["Article"] } }, { "terms" => { "published" => [true] } }, ] - expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters) + expect(search_bool_clause(filter)["filter"]).to match_array(exepcted_filters) end it "applies RANGE_KEYS from params" do @@ -54,7 +58,7 @@ RSpec.describe Search::QueryBuilders::FeedContent, type: :service do { "range" => { "published_at" => { lte: Time.current } } }, { "terms" => { "published" => [true] } }, ] - expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters) + expect(search_bool_clause(filter)["filter"]).to match_array(exepcted_filters) end end @@ -63,15 +67,15 @@ RSpec.describe Search::QueryBuilders::FeedContent, type: :service do params = { search_fields: "ruby", published_at: { lte: Time.current }, tag_names: "cfp" } filter = described_class.new(params) exepcted_query = [{ - "simple_query_string" => { "query" => "ruby*", "fields" => query_fields, "lenient" => true, "analyze_wildcard" => true } + "simple_query_string" => { "query" => "ruby", "fields" => query_fields, "lenient" => true, "analyze_wildcard" => true, "minimum_should_match" => 2 } }] exepcted_filters = [ { "range" => { "published_at" => { lte: Time.current } } }, { "terms" => { "tags.name" => ["cfp"] } }, { "terms" => { "published" => [true] } }, ] - expect(filter.as_hash.dig("query", "bool", "must")).to match_array(exepcted_query) - expect(filter.as_hash.dig("query", "bool", "filter")).to match_array(exepcted_filters) + expect(search_bool_clause(filter)["must"]).to match_array(exepcted_query) + expect(search_bool_clause(filter)["filter"]).to match_array(exepcted_filters) end end @@ -80,10 +84,10 @@ RSpec.describe Search::QueryBuilders::FeedContent, type: :service do filter = described_class.new(params) exepcted_query = [{ "simple_query_string" => { - "query" => "cfp*", "fields" => query_fields, "lenient" => true, "analyze_wildcard" => true + "query" => "cfp", "fields" => query_fields, "lenient" => true, "analyze_wildcard" => true, "minimum_should_match" => 2 } }] - expect(filter.as_hash.dig("query", "bool", "must")).to match_array(exepcted_query) + expect(search_bool_clause(filter)["must"]).to match_array(exepcted_query) end it "allows default params to be overriden" do @@ -92,5 +96,14 @@ RSpec.describe Search::QueryBuilders::FeedContent, type: :service do expect(filter.dig("sort")).to eq("published_at" => "asc") expect(filter.dig("size")).to eq(20) end + + it "correctly sets default sort" do + filter = described_class.new({}).as_hash + expect(filter.dig("sort")).to eq(described_class::DEFAULT_PARAMS[:sort]) + end + end + + def search_bool_clause(query_builder) + query_builder.as_hash.dig("query", "function_score", "query", "bool") end end