From 996ed15137d5a5d3b13422b51e6e66572eaa85f2 Mon Sep 17 00:00:00 2001 From: Jonathan Rochkind Date: Tue, 21 Aug 2012 12:02:44 -0400 Subject: [PATCH] battle#index recovers from failed results --- app/controllers/battle_controller.rb | 48 +++++++++++++++++++++-- app/models/error.rb | 2 + test/functional/battle_controller_test.rb | 47 ++++++++++++++++++++-- 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/app/controllers/battle_controller.rb b/app/controllers/battle_controller.rb index ef1b68f..6fc6c8d 100644 --- a/app/controllers/battle_controller.rb +++ b/app/controllers/battle_controller.rb @@ -14,18 +14,26 @@ def self.contenders=(arr) before_filter :validate_choice, :only => :choice def index - if params[:q] - choices = contenders.shuffle - + if params[:q] @one = choices.pop @two = choices.pop searcher = BentoSearch::MultiSearcher.new(@one, @two) - @results = searcher.start(params[:q]).results + @results = searcher.start(params[:q]).results + + # check for failed searches and handle + ["one", "two"].each do |choice| + engine = instance_variable_get("@#{choice}") + if @results[engine].failed? + handle_failed_results(choice) + end + end end end + + def choice choice = if params["preferA"].present? params[:option_a] @@ -57,6 +65,12 @@ def choice protected + # in seperate method mainly so we can mock it in tests to choose + # exactly what engines we want. + def choices + @choices ||= contenders.shuffle + end + def validate_choice unless (params[:option_a].present? && params[:option_b].present? && params[:query].present? && @@ -68,4 +82,30 @@ def validate_choice end end + # arg 'choice' is 'one' or 'two', and is a search engine results + # that failed. We'll try to replace it with results from next in + # list, and register it's failure. Check again and recursively + # handle error again if needed. + def handle_failed_results(choice) + orig_engine = instance_variable_get("@#{choice}") + orig_results = @results[orig_engine] + + # record the error + Error.create( + :engine => orig_engine, + :error_info => orig_results.error.to_hash, + :backtrace => orig_results.error[:exception].try(:backtrace) + ) + + + @results.delete orig_engine + + new_engine = choices.pop + instance_variable_set("@#{choice}", new_engine) + + new_results = BentoSearch.get_engine(new_engine).search(params[:q]) + + @results[new_engine] = new_results + end + end diff --git a/app/models/error.rb b/app/models/error.rb index e5f555c..46232e8 100644 --- a/app/models/error.rb +++ b/app/models/error.rb @@ -1,3 +1,5 @@ class Error < ActiveRecord::Base + serialize :error_info + serialize :backtrace attr_accessible :backtrace, :engine, :error_info end diff --git a/test/functional/battle_controller_test.rb b/test/functional/battle_controller_test.rb index eeee563..41e724d 100644 --- a/test/functional/battle_controller_test.rb +++ b/test/functional/battle_controller_test.rb @@ -2,17 +2,17 @@ class BattleControllerTest < ActionController::TestCase def setup - BentoSearch.register_engine("one") do |conf| + BentoSearch.register_engine("AA") do |conf| conf.engine = "BentoSearch::MockEngine" end - BentoSearch.register_engine("two") do |conf| + BentoSearch.register_engine("BB") do |conf| conf.engine = "BentoSearch::MockEngine" end - BentoSearch.register_engine("three") do |conf| + BentoSearch.register_engine("CC") do |conf| conf.engine = "BentoSearch::MockEngine" end - BattleController.contenders = ["one", "two", "three"] + BattleController.contenders = ["AA", "BB", "CC"] end def teardown @@ -49,6 +49,45 @@ def teardown end end + test "should recover from errors when searching" do + BentoSearch.register_engine("error") do |conf| + conf.engine = "BentoSearch::MockEngine" + conf.error = {:message => "Fake error"} + end + BattleController.contenders = ["AA", "BB", "CC", "error"] + + # Fake it into 'randomly' picking error in it's first two please + @controller.extend( Module.new do + # chooses from the end, so put error at the end + def choices + @choices ||= ["AA", "BB", "error", "CC"] + end + end) + + assert_difference("Error.count", 1) do + get :index, :q => "Cancer" + end + + assert_response :success + + # proper error was saved + err = Error.last + assert_equal "error", err.engine + + + # didn't use error, replaced it with next in list, BB + assert_equal "CC", assigns["one"] + assert_equal "BB", assigns["two"] + + # and in results + assert_include assigns["results"].keys, "CC" + assert_include assigns["results"].keys, "BB" + + # and neither are errors + assert ! assigns["results"].values.find {|r| r.failed?} + + end + test "choice" do option_a = ["a", "b", "c"].sample option_b = ["1", "2", "3"].sample